Skip to content

Commit bccf630

Browse files
committed
fix(appkit): keep node: cache-path resolver out of the client-safe shared barrel
CI surfaced two failures from routing the cache-path resolvers through the shared root barrel: 1. Docs Build (+ PR Template): the root barrel `shared/src/index.ts` is client-safe (docs webpack aliases shared->src and pulls it via appkit-ui), but cache-paths imports `node:path`, so the docs client bundle died with UnhandledSchemeError. Move the resolvers off the root barrel onto a Node-only subpath and import them there from appkit's cache/serving/ choice-sink modules. tsdown owns shared's generated `exports` (devExports), so the subpath is added as a tsdown entry and keyed by its src-relative path: `shared/cli/commands/cache-paths`. 2. Unit Tests: the new bootstrap-vs-drift message assertions matched raw text, but picocolors emits ANSI codes when CI is set, splitting the strings. Strip ANSI (incl. the ESC byte) before matching. Also repoint the cache serialization test's `vi.mock` to the new subpath specifier. Verified with the gates CI runs: `CI=true pnpm test` (3725 pass), `pnpm docs:build` (client compiles, no node: error), typecheck, build, check, knip. xavier loop: iteration 6 -- CI fixes (docs node: leak + ANSI-brittle tests) Co-authored-by: Isaac Signed-off-by: Atila Fassina <atila@fassina.eu>
1 parent 992dd66 commit bccf630

10 files changed

Lines changed: 44 additions & 17 deletions

File tree

packages/appkit/src/plugins/ui-variants/choice-sink.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import fs from "node:fs/promises";
22
import path from "node:path";
3-
import { getEphemeralStateDir } from "shared";
3+
import { getEphemeralStateDir } from "shared/cli/commands/cache-paths";
44

55
/**
66
* Filename of the JSONL choices file. The agent skill discovers this file at

packages/appkit/src/type-generator/cache.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import crypto from "node:crypto";
22
import fs from "node:fs/promises";
33
import path from "node:path";
4-
import { getCommittedCacheDir } from "shared";
4+
import { getCommittedCacheDir } from "shared/cli/commands/cache-paths";
55
import { createLogger } from "../logging/logger";
66
import type { MetricSchema } from "./mv-registry/types";
77

packages/appkit/src/type-generator/serving/cache.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import crypto from "node:crypto";
22
import fs from "node:fs/promises";
33
import path from "node:path";
4-
import { getCommittedCacheDir } from "shared";
4+
import { getCommittedCacheDir } from "shared/cli/commands/cache-paths";
55
import { createLogger } from "../../logging/logger";
66

77
const logger = createLogger("type-generator:serving:cache");

packages/appkit/src/type-generator/tests/cache-serialization.test.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,11 @@ vi.mock("node:fs/promises", () => ({
1414
},
1515
}));
1616

17-
// Mock getCommittedCacheDir to return a consistent test path
18-
vi.mock("shared", () => ({
17+
// Mock getCommittedCacheDir to return a consistent test path.
18+
// cache.ts imports it from the Node-only "shared/cli/commands/cache-paths"
19+
// subpath (kept out of the client-safe "shared" root barrel), so the mock must
20+
// target that exact specifier.
21+
vi.mock("shared/cli/commands/cache-paths", () => ({
1922
getCommittedCacheDir: () => "/test/app/.appkit",
2023
}));
2124

packages/appkit/src/type-generator/tests/index.test.ts

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,12 @@ import {
1111
} from "vitest";
1212
import type { DatabricksStatementExecutionResponse } from "../types";
1313

14+
// picocolors emits ANSI color codes when CI is set; strip them (ESC + the
15+
// bracketed SGR sequence) so message-content assertions match regardless of
16+
// the color environment.
17+
// biome-ignore lint/suspicious/noControlCharactersInRegex: matching the ESC (\x1b) byte is the point.
18+
const stripAnsi = (s: string): string => s.replace(/\x1b\[[0-9;]*m/g, "");
19+
1420
const mocks = vi.hoisted(() => ({
1521
generateQueriesFromDescribe: vi.fn(),
1622
getWarehouseState: vi.fn(),
@@ -304,11 +310,12 @@ describe("generateFromEntryPoint — query failure handling", () => {
304310
expect(err).toBeInstanceOf(TypegenFatalError);
305311
// This is the critical assertion: the message must reflect the state
306312
// BEFORE generation, not after (when the cache file now exists).
307-
expect((err as Error).message).toMatch(/No committed type cache found/i);
308-
expect((err as Error).message).toMatch(/generate-types --wait/i);
309-
expect((err as Error).message).toMatch(/commit \.appkit\//i);
313+
const message = stripAnsi((err as Error).message);
314+
expect(message).toMatch(/No committed type cache found/i);
315+
expect(message).toMatch(/generate-types --wait/i);
316+
expect(message).toMatch(/commit \.appkit\//i);
310317
// Must NOT say "missing or stale" (the drift message):
311-
expect((err as Error).message).not.toMatch(/missing or stale/i);
318+
expect(message).not.toMatch(/missing or stale/i);
312319
}
313320
});
314321

@@ -334,13 +341,12 @@ describe("generateFromEntryPoint — query failure handling", () => {
334341
} catch (err) {
335342
expect(err).toBeInstanceOf(TypegenFatalError);
336343
// Cache existed at start → report drift (stale/missing key).
337-
expect((err as Error).message).toMatch(/missing or stale/i);
338-
expect((err as Error).message).toMatch(/Regenerate with/i);
339-
expect((err as Error).message).toMatch(/generate-types --wait/i);
344+
const message = stripAnsi((err as Error).message);
345+
expect(message).toMatch(/missing or stale/i);
346+
expect(message).toMatch(/Regenerate with/i);
347+
expect(message).toMatch(/generate-types --wait/i);
340348
// Must NOT say "No committed type cache found" (the bootstrap message):
341-
expect((err as Error).message).not.toMatch(
342-
/No committed type cache found/i,
343-
);
349+
expect(message).not.toMatch(/No committed type cache found/i);
344350
}
345351
});
346352
});

packages/appkit/tsconfig.json

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,9 @@
77
"@/*": ["src/*"],
88
"@tools/*": ["../../tools/*"],
99
"shared": ["../../packages/shared/src"],
10+
"shared/cli/commands/cache-paths": [
11+
"../../packages/shared/src/cli/commands/cache-paths"
12+
],
1013
"@databricks/lakebase": ["../../packages/lakebase/src"]
1114
}
1215
},

packages/shared/package.json

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,10 @@
1515
"development": "./src/cli/index.ts",
1616
"default": "./dist/cli/index.js"
1717
},
18+
"./cli/commands/cache-paths": {
19+
"development": "./src/cli/commands/cache-paths.ts",
20+
"default": "./dist/cli/commands/cache-paths.js"
21+
},
1822
"./package.json": "./package.json"
1923
},
2024
"scripts": {
@@ -35,6 +39,7 @@
3539
"exports": {
3640
".": "./dist/index.js",
3741
"./cli": "./dist/cli/index.js",
42+
"./cli/commands/cache-paths": "./dist/cli/commands/cache-paths.js",
3843
"./package.json": "./package.json"
3944
}
4045
},

packages/shared/src/index.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
export * from "./agent";
22
export * from "./cache";
3-
export * from "./cli/commands/cache-paths";
43
export * from "./execute";
54
export * from "./genie";
65
export * from "./plugin";

packages/shared/tsdown.config.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,15 @@ import { defineConfig } from "tsdown";
22

33
export default defineConfig({
44
name: "shared",
5-
entry: ["src/index.ts", "src/cli/index.ts"],
5+
entry: [
6+
"src/index.ts",
7+
"src/cli/index.ts",
8+
// Node-only path resolvers, exposed as the "shared/cache-paths" subpath so
9+
// appkit can import them without pulling node: builtins into the
10+
// client-safe root barrel. Must be a tsdown entry or the exports plugin
11+
// (exports.devExports below) drops the "./cache-paths" export on rebuild.
12+
"src/cli/commands/cache-paths.ts",
13+
],
614
outDir: "dist",
715
minify: false,
816
format: "esm",

tsconfig.json

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,9 @@
1616
"@databricks/appkit": ["packages/appkit/*"],
1717
"@databricks/appkit-ui": ["packages/appkit-ui/*"],
1818
"shared": ["packages/shared/src"],
19+
"shared/cli/commands/cache-paths": [
20+
"packages/shared/src/cli/commands/cache-paths"
21+
],
1922
"@tools/*": ["tools/*"]
2023
}
2124
},

0 commit comments

Comments
 (0)