From 38e30b080b067e103af19138cd4cd63c353275c9 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 13 Aug 2026 03:02:21 +0000 Subject: [PATCH 1/7] ci: keep a blocking job that exercises the built bundles MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Resolving @workglow/* to src is what makes the coverage numbers mean anything, but the plugin is attached to every project unconditionally — not only to coverage runs — so with the default in force no vitest job resolves a specifier through `exports` at all. There is also no `bun test` job in the blocking workflow. So after the source-resolution change, nothing that can block a merge loads a built bundle: a `bun build` entry that silently dropped a re-export would reach main and surface only in the nightly Bun parity run, which is explicitly informational, runs on a cron, and excludes six sections. Adds test-vitest-dist: reuses the existing build-output artifact and runs the unit tier with WORKGLOW_TEST_TARGET=dist. It is in cleanup's needs list, since cleanup deletes the artifact it downloads. Scoping the plugin to coverage runs instead would not have worked: scripts/test.ts adds --coverage whenever CI is set, so in CI every run is a coverage run and would still resolve to src. Also skips --coverage for a dist-targeted run. The denominator names package sources, so such a run reported all ~1286 of them at 0% — not a measurement of anything, and it is what lets the new job reuse test:vitest:unit unchanged and produce no fragment for merge-vitest-coverage. The CLAUDE.md and vitest.config.ts notes claimed bundle integrity was covered by the nightly parity run; both now say what actually guards it. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01H797qbH356jjznKgUax63o --- .claude/CLAUDE.md | 8 ++++++ .github/workflows/test.yml | 32 +++++++++++++++++++++ scripts/testRunnerArgs.test.ts | 51 ++++++++++++++++++++++++++++++++++ vitest.config.ts | 14 ++++++---- 4 files changed, 99 insertions(+), 6 deletions(-) create mode 100644 scripts/testRunnerArgs.test.ts diff --git a/.claude/CLAUDE.md b/.claude/CLAUDE.md index e11e344a7..cd8f72107 100644 --- a/.claude/CLAUDE.md +++ b/.claude/CLAUDE.md @@ -613,6 +613,14 @@ download models or call live APIs); `WORKGLOW_TEST_TARGET=dist` turns the rewrit exercise the bundles instead, and refuses coverage there. The include/exclude globs are repo-root-relative, so coverage and `--project` cannot be combined. +The rewrite is attached to every project unconditionally, so with the default in force NO +vitest job resolves a `@workglow/*` specifier through `exports` — and the blocking workflow +runs no `bun test` job at all. The nightly Bun parity run does resolve `exports` natively, +but it is informational, never blocks a merge, runs on a cron and excludes six sections. So +the `test-vitest-dist` job in `.github/workflows/test.yml` is what keeps bundle integrity +blocking: it reuses the `build-output` artifact and runs the unit tier under +`WORKGLOW_TEST_TARGET=dist`. + ## Developing without building `bun run use-source` makes every package resolve to its source. It does **not** touch diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index d9a1520a2..77c45ab7a 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -161,6 +161,37 @@ jobs: # of files, and the shards past the first then hold none of them. - name: Run unit tests via vitest (shard ${{ matrix.shard }}) run: bun run test:vitest:unit -- --shard ${{ matrix.shard }} ${{ env.TEST_CHANGED }} + + # The one blocking job that resolves `@workglow/*` through `exports` rather + # than through the source-rewrite plugin. Every other vitest job here runs + # against `src`, and there is no `bun test` job in this workflow at all, so + # without this a `bun build` entry that silently dropped a re-export would + # reach main and surface only in the nightly parity run, which never blocks. + # Unsharded: it exists to load every bundle once, and the artifact download + # dominates its runtime either way. + test-vitest-dist: + runs-on: ubuntu-latest + needs: build + steps: + - uses: actions/checkout@v6 + with: + fetch-depth: 0 + - uses: actions/setup-node@v6 + with: + node-version: 24 + - uses: oven-sh/setup-bun@v2 + with: + bun-version: 1.4.0 + - run: bun i + - name: Download build artifacts + uses: actions/download-artifact@v8 + with: + name: build-output + path: . + - name: Run unit tests against the built bundles + env: + WORKGLOW_TEST_TARGET: dist + run: bun run test:vitest:unit -- ${{ env.TEST_CHANGED }} test-vitest-integration: runs-on: ubuntu-latest needs: build @@ -307,6 +338,7 @@ jobs: needs: [ test-vitest-unit, + test-vitest-dist, test-vitest-integration, test-vitest-rag, test-vitest-ai-provider-hft, diff --git a/scripts/testRunnerArgs.test.ts b/scripts/testRunnerArgs.test.ts new file mode 100644 index 000000000..cd1576c7f --- /dev/null +++ b/scripts/testRunnerArgs.test.ts @@ -0,0 +1,51 @@ +/** + * @license + * Copyright 2026 Steven Roussey + * SPDX-License-Identifier: Apache-2.0 + */ + +import { execFileSync } from "node:child_process"; +import { describe, expect, it } from "vitest"; +import { ROOT } from "./lib/testDiscovery"; + +/** + * Runs the test runner in dry-run mode, which prints the command it would have + * spawned as a JSON array instead of executing it. + */ +function dryRunCommand(env: Readonly>): string { + const stdout = execFileSync("bun", ["scripts/test.ts", "unit", "vitest", "--dry-run"], { + cwd: ROOT, + encoding: "utf8", + env: { ...process.env, ...env }, + }); + const line = stdout + .split("\n") + .map((entry) => entry.trim()) + .find((entry) => entry.startsWith("[")); + expect(line, `no command line in dry-run output:\n${stdout}`).toBeDefined(); + return line as string; +} + +describe("scripts/test.ts coverage flag", () => { + // The baseline the second case is measured against. Coverage is opt-in by + // name — `WORKGLOW_COVERAGE`, not `CI` — so without this the second case + // would pass on a run that was never going to collect coverage anyway. + it("asks for coverage when coverage was asked for by name", () => { + expect(dryRunCommand({ WORKGLOW_COVERAGE: "1" })).toContain('"--coverage"'); + }); + + /** + * A `dist`-targeted run exercises the built bundles, but the coverage + * denominator names `packages/*` and `providers/*` SOURCES. Collecting + * coverage there reports every source file at 0%, which is not a measurement + * of anything. + * + * This is what lets the blocking `test-vitest-dist` CI job reuse + * `test:vitest:unit` unchanged rather than needing its own invocation. + */ + it("does not ask for coverage when the run targets the built bundles", () => { + expect( + dryRunCommand({ WORKGLOW_COVERAGE: "1", WORKGLOW_TEST_TARGET: "dist" }) + ).not.toContain('"--coverage"'); + }); +}); diff --git a/vitest.config.ts b/vitest.config.ts index 40dc810e5..c31e1a640 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -148,12 +148,14 @@ const discovered = discoverTestFiles(); * package/relative import graph otherwise produces. * * `dist` turns the rewrite off, so `exports` resolution stands and the built - * bundles are what gets loaded. The nightly Bun run already exercises them — - * `bun test` resolves `exports` natively — so nothing in CI sets this; it is - * the local escape hatch for reproducing a bundle-only failure under vitest. - * Its plugin is a guard rather than nothing at all, because the tree a - * developer reaches for it on is usually a `use-source` tree, where `dist` - * re-exports `src` and the target would silently measure nothing. + * bundles are what gets loaded. The blocking `test-vitest-dist` CI job sets it: + * every other vitest job resolves to `src`, and the nightly Bun run that does + * load the bundles never blocks a merge, so without that job a `bun build` + * entry which silently dropped a re-export reaches main. It is also the local + * escape hatch for reproducing a bundle-only failure under vitest. Its plugin + * is a guard rather than nothing at all, because the tree a developer reaches + * for it on is usually a `use-source` tree, where `dist` re-exports `src` and + * the target would silently measure nothing. */ const target = resolveTestTarget(process.env.WORKGLOW_TEST_TARGET); From 9e7cc35aa1d78e9f97c1ecd868eda297c5c299c0 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 13 Aug 2026 03:54:45 +0000 Subject: [PATCH 2/7] test(scripts): pin WORKGLOW_TEST_TARGET instead of inheriting it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The coverage-flag test spawned the runner with `{...process.env, CI: "1"}` and let WORKGLOW_TEST_TARGET come from the ambient environment. The new test-vitest-dist job exports that variable for its whole step, so inside that job the source-target case inherited `dist` and became a second copy of the dist case — asserting `--coverage` is present while the runner correctly omitted it. It failed in the one job it was added to support. Both cases now state the target explicitly, so the assertions hold whatever the runner is invoked under. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01H797qbH356jjznKgUax63o --- scripts/testRunnerArgs.test.ts | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/scripts/testRunnerArgs.test.ts b/scripts/testRunnerArgs.test.ts index cd1576c7f..0608518be 100644 --- a/scripts/testRunnerArgs.test.ts +++ b/scripts/testRunnerArgs.test.ts @@ -11,12 +11,18 @@ import { ROOT } from "./lib/testDiscovery"; /** * Runs the test runner in dry-run mode, which prints the command it would have * spawned as a JSON array instead of executing it. + * + * `target` is always passed explicitly rather than left to the ambient + * environment. The `test-vitest-dist` CI job exports `WORKGLOW_TEST_TARGET` for + * its whole step, so an inherited value silently rewrites the source-target + * case into a second copy of the dist case — which is exactly how this file + * first failed, in the very job it exists to support. */ -function dryRunCommand(env: Readonly>): string { +function dryRunCommand(target: "source" | "dist"): string { const stdout = execFileSync("bun", ["scripts/test.ts", "unit", "vitest", "--dry-run"], { cwd: ROOT, encoding: "utf8", - env: { ...process.env, ...env }, + env: { ...process.env, WORKGLOW_COVERAGE: "1", WORKGLOW_TEST_TARGET: target }, }); const line = stdout .split("\n") @@ -31,7 +37,7 @@ describe("scripts/test.ts coverage flag", () => { // name — `WORKGLOW_COVERAGE`, not `CI` — so without this the second case // would pass on a run that was never going to collect coverage anyway. it("asks for coverage when coverage was asked for by name", () => { - expect(dryRunCommand({ WORKGLOW_COVERAGE: "1" })).toContain('"--coverage"'); + expect(dryRunCommand("source")).toContain('"--coverage"'); }); /** @@ -44,8 +50,6 @@ describe("scripts/test.ts coverage flag", () => { * `test:vitest:unit` unchanged rather than needing its own invocation. */ it("does not ask for coverage when the run targets the built bundles", () => { - expect( - dryRunCommand({ WORKGLOW_COVERAGE: "1", WORKGLOW_TEST_TARGET: "dist" }) - ).not.toContain('"--coverage"'); + expect(dryRunCommand("dist")).not.toContain('"--coverage"'); }); }); From b163cc9a4bd4f627dd4c6cb935d344a9b65dc4f4 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 14 Aug 2026 15:50:41 +0000 Subject: [PATCH 3/7] test: import every published entry, so the dist job is not tier-shaped MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `test-vitest-dist` is the one blocking job that resolves `@workglow/*` through `exports`, but it runs the UNIT tier only — while the source rewrite applies to EVERY vitest job. The integration/rag/provider suites previously loaded the bundles and now load src, so a bundle reachable only from an `.integration.test.ts` file lost its blocking check. Two entries lost every check: `@workglow/openrouter/ai-runtime` and `@workglow/huggingface-inference/ai-runtime`, imported only from provider-api integration files, whose section the nightly Bun parity run also excludes. Concretely: a `bun build` change dropping `registerOpenRouterInline` from `providers/openrouter/dist/ai-runtime.js` leaves the file in place, satisfies the dist-must-exist requirement, passes all of CI, and breaks consumers only after publish. `PublishedEntryImports.test.ts` makes the check total instead of tier-shaped: it enumerates every workspace manifest's `exports`, resolves each subpath under the Node conditions only (`node`/`import`/`default`, walked in declaration order the way Node does, so `types`/`browser`/`bun` are stepped over rather than entered), and dynamically imports each resulting specifier, asserting the module is non-empty. Under `WORKGLOW_TEST_TARGET=dist` that one unit-tier file loads every published bundle; under the default target it costs nothing, since it loads the same source the rest of the suite already does. Adding `workglow` to `packages/test`'s devDependencies is the larger half: it brings the meta-package's own entries and, transitively, the provider bundles those re-export. The enumeration is local rather than shared with `scripts/lib/sourceStubs`: `stubSpecsFor` returns dist targets rather than import specifiers, and `packages/test` is a `composite` project rooted at `./src`, so importing from `scripts/` would put those files in its program and break `build-types`. Anti-vacuity assertions (over 60 entries across over 20 packages, every target `./dist/**.js`) keep a mis-typed walk from passing as a short list, and both exemption maps are staleness-checked against the enumeration. Two exemptions, each with its reason: `@workglow/cli` (uncheckable — an example app `packages/test` does not depend on, so under isolated linking the specifier does not resolve from here at all) and `workglow/auto-bootstrap` (imported, but exempt from the non-empty assertion: it registers providers as a side effect and exports nothing by design). New packages default to checked. --- .claude/CLAUDE.md | 15 ++ bun.lock | 1 + packages/test/package.json | 3 +- .../test/util/PublishedEntryImports.test.ts | 172 ++++++++++++++++++ 4 files changed, 190 insertions(+), 1 deletion(-) create mode 100644 packages/test/src/test/util/PublishedEntryImports.test.ts diff --git a/.claude/CLAUDE.md b/.claude/CLAUDE.md index cd8f72107..b39204b82 100644 --- a/.claude/CLAUDE.md +++ b/.claude/CLAUDE.md @@ -621,6 +621,21 @@ the `test-vitest-dist` job in `.github/workflows/test.yml` is what keeps bundle blocking: it reuses the `build-output` artifact and runs the unit tier under `WORKGLOW_TEST_TARGET=dist`. +That job runs the UNIT tier only, which on its own would make the blocking check +**tier-shaped**: a bundle reachable only from an `.integration.test.ts` file would be +loaded by no blocking job at all. What closes that gap is +`packages/test/src/test/util/PublishedEntryImports.test.ts`, a unit-tier file that +enumerates every workspace manifest's `exports`, resolves each subpath under the Node +conditions, and dynamically imports the result, asserting the module is non-empty. Under +`WORKGLOW_TEST_TARGET=dist` that one file loads every published bundle regardless of which +tier a suite happens to exercise it from; under the default target it costs nothing, +because it loads the same sources the rest of the suite already does. Its exemption maps +are deliberately tiny and each entry states its reason, so a new package defaults to being +checked and a stale exemption fails the test rather than silently exempting nothing. +Adding a published `exports` subpath therefore needs no CI change, and importing +`workglow` — the meta-package `packages/test` devDepends on — is what pulls the provider +bundles into the sweep transitively. + ## Developing without building `bun run use-source` makes every package resolve to its source. It does **not** touch diff --git a/bun.lock b/bun.lock index 48373c1ee..bc3cf6fda 100644 --- a/bun.lock +++ b/bun.lock @@ -398,6 +398,7 @@ "pg": "catalog:", "playwright": "catalog:", "vitest": "catalog:", + "workglow": "workspace:*", }, }, "packages/test-contract": { diff --git a/packages/test/package.json b/packages/test/package.json index 611ecb753..e848c4a6e 100644 --- a/packages/test/package.json +++ b/packages/test/package.json @@ -82,6 +82,7 @@ "node-llama-cpp": "catalog:", "pg": "catalog:", "playwright": "catalog:", - "vitest": "catalog:" + "vitest": "catalog:", + "workglow": "workspace:*" } } diff --git a/packages/test/src/test/util/PublishedEntryImports.test.ts b/packages/test/src/test/util/PublishedEntryImports.test.ts new file mode 100644 index 000000000..7e4c085d4 --- /dev/null +++ b/packages/test/src/test/util/PublishedEntryImports.test.ts @@ -0,0 +1,172 @@ +/** + * @license + * Copyright 2026 Steven Roussey + * SPDX-License-Identifier: Apache-2.0 + */ + +import { readdirSync, readFileSync } from "node:fs"; +import { dirname, join, resolve } from "node:path"; +import { fileURLToPath } from "node:url"; +import { describe, expect, it } from "vitest"; + +const repoRoot = resolve(dirname(fileURLToPath(import.meta.url)), "../../../../.."); + +/** The root `workspaces` globs, in the order a manifest scan walks them. */ +const WORKSPACE_GROUPS = ["packages", "providers", "examples"] as const; + +/** + * The conditions a plain `import` from Node activates, and only those. + * + * `types` is a compiler artifact, `browser` and `bun` name runtimes this test + * is not running under, and entering either would import a bundle whose + * dependencies (DOM globals, `bun:sqlite`) do not exist here. Node's own + * algorithm is what is replicated: walk the object's keys IN ORDER and take the + * first one that names an active condition, so a `types`-first block resolves + * to whatever comes after it rather than to nothing. + */ +const NODE_CONDITIONS: ReadonlySet = new Set(["node", "import", "default"]); + +/** + * Entries no CI job can import, each with the reason it cannot. + * + * Kept deliberately small and stated per entry, so a NEW package defaults to + * being checked: the value of this file is that a published entry nobody + * imports is a bug, and a permissive default would recreate exactly the hole it + * closes. Every key is proved to still exist below, so a package that is + * renamed or deleted takes its exemption with it. + */ +const UNCHECKABLE: Readonly> = { + "@workglow/cli": + "an example app, and `packages/test` does not depend on it — under bunfig's " + + "isolated linker the specifier does not resolve from here at all, so importing " + + "it would test the linker rather than the bundle", +}; + +/** + * Entries that load but legitimately export nothing, with the reason. + * + * These are still IMPORTED — that a side-effect module evaluates cleanly is the + * whole of what it promises — only the non-empty assertion is lifted. + */ +const SIDE_EFFECT_ONLY: Readonly> = { + "workglow/auto-bootstrap": + "registers the bundled providers as an import side effect and exports nothing by design", +}; + +interface PublishedEntry { + /** The package that publishes it. */ + readonly packageName: string; + /** The specifier a consumer writes, e.g. `@workglow/util/schema`. */ + readonly specifier: string; + /** The manifest-relative target Node resolves it to under `NODE_CONDITIONS`. */ + readonly target: string; +} + +/** The target a conditional `exports` value resolves to, or `undefined`. */ +function resolveUnderNode(value: unknown): string | undefined { + if (typeof value === "string") return value; + if (typeof value !== "object" || value === null || Array.isArray(value)) return undefined; + for (const [condition, child] of Object.entries(value as Record)) { + if (!NODE_CONDITIONS.has(condition)) continue; + const resolved = resolveUnderNode(child); + if (resolved !== undefined) return resolved; + } + return undefined; +} + +/** Every published entry of every workspace package, by manifest. */ +function collectPublishedEntries(): PublishedEntry[] { + const entries: PublishedEntry[] = []; + for (const group of WORKSPACE_GROUPS) { + let directories: string[]; + try { + directories = readdirSync(join(repoRoot, group)); + } catch { + continue; + } + for (const directory of directories) { + let manifest: { name?: unknown; exports?: unknown }; + try { + manifest = JSON.parse( + readFileSync(join(repoRoot, group, directory, "package.json"), "utf8") + ); + } catch { + continue; // not a package directory + } + const { name, exports } = manifest; + if (typeof name !== "string") continue; + if (typeof exports !== "object" || exports === null || Array.isArray(exports)) continue; + for (const [subpath, value] of Object.entries(exports as Record)) { + if (!subpath.startsWith(".")) continue; // a bare condition map, not a subpath + const target = resolveUnderNode(value); + if (target === undefined) continue; // browser-only entry; nothing for Node to load + entries.push({ packageName: name, specifier: name + subpath.slice(1), target }); + } + } + } + return entries.sort((a, b) => a.specifier.localeCompare(b.specifier)); +} + +const entries = collectPublishedEntries(); +const checkable = entries.filter((entry) => !(entry.specifier in UNCHECKABLE)); + +/** + * Import every entry this repo publishes, by the specifier a consumer writes. + * + * The blocking bundle-integrity check is `test-vitest-dist`, which runs the + * UNIT tier against the built bundles. That makes the check tier-shaped: an + * entry reachable only from an `.integration.test.ts` file is exercised by no + * blocking job at all, which was true of `@workglow/openrouter/ai-runtime` and + * `@workglow/huggingface-inference/ai-runtime` — a `bun build` change that + * dropped a re-export from either bundle would have left the file in place, + * satisfied every existing check, and broken consumers only after publish. + * + * Enumerating the manifests instead of the import graph makes the check TOTAL + * rather than tier-shaped, and it costs nothing under the default target (the + * same source files the rest of the suite already loads). It is a unit-tier + * file on purpose: that is the tier `test-vitest-dist` runs. + * + * The enumeration is local rather than shared with `scripts/lib/`: + * `packages/test` is a `composite` project rooted at `./src`, so importing from + * `scripts/` would put those files in its program and break `build-types`. + */ +describe("published entry imports", () => { + it("enumerates every workspace manifest, so an empty sweep cannot pass", () => { + // Anti-vacuity: a typo in the walk (wrong group, wrong key) yields a short + // list rather than an error, and a short list passes every assertion below. + expect(entries.length).toBeGreaterThan(60); + expect(new Set(entries.map((entry) => entry.packageName)).size).toBeGreaterThan(20); + }); + + it("resolves every entry to a built file under dist", () => { + // Every published entry is a bundle. A target outside `dist` means the + // manifest ships source, and one that is not `.js` means the walk above + // landed on a `types` (or other non-runtime) condition. + const offenders = entries + .filter((entry) => !/^\.\/dist\/.+\.js$/.test(entry.target)) + .map((entry) => `${entry.specifier} -> ${entry.target}`); + expect(offenders).toEqual([]); + }); + + it("keeps every exemption pinned to an entry that still exists", () => { + // An exemption that outlives its package silently exempts nothing, and + // reads as if a real hole were still open. + const published = new Set(entries.map((entry) => entry.specifier)); + const stale = [...Object.keys(UNCHECKABLE), ...Object.keys(SIDE_EFFECT_ONLY)].filter( + (specifier) => !published.has(specifier) + ); + expect(stale).toEqual([]); + for (const reason of [...Object.values(UNCHECKABLE), ...Object.values(SIDE_EFFECT_ONLY)]) { + expect(reason.length).toBeGreaterThan(20); + } + }); + + it.each(checkable.map((entry) => entry.specifier))("%s loads", async (specifier) => { + const loaded: Record = await import(/* @vite-ignore */ specifier); + expect(loaded).toBeDefined(); + if (specifier in SIDE_EFFECT_ONLY) return; + // A bundle that lost every re-export still resolves and still evaluates; + // the export list is the only thing that says the entry point works. + expect(Object.keys(loaded).length).toBeGreaterThan(0); + }); +}); From 101ef1c511874a5f57b2bed75326b9d1dacaa314 Mon Sep 17 00:00:00 2001 From: Steven Roussey Date: Sun, 20 Sep 2026 23:29:04 +0000 Subject: [PATCH 4/7] test: check bundle identity and export parity on the dist target MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Importing every published entry proves each bundle LOADS; it does not prove the bundles agree with each other. Two entries of one package can each resolve, and still hand a consumer two different classes for one symbol — an `instanceof` across them then fails for a reason no import sweep can see. PublishedEntryIdentity pins the cross-entry identities that matter, and PublishedEntryExportParity compares each entry's named exports against its source barrel, so a re-export a `bun build` change dropped is named rather than inferred from a downstream failure. Both re-derive the workspace groups from the root manifest rather than importing the tooling's copy: `packages/test` is a composite project rooted at `./src`, so reaching into `scripts/` would pull those files into its program and break `build-types`. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01By1Tpxcr4qNBjauTK1QeNj --- .../util/PublishedEntryExportParity.test.ts | 184 ++++++++++++ .../test/util/PublishedEntryIdentity.test.ts | 269 ++++++++++++++++++ .../test/util/PublishedEntryImports.test.ts | 17 +- 3 files changed, 468 insertions(+), 2 deletions(-) create mode 100644 packages/test/src/test/util/PublishedEntryExportParity.test.ts create mode 100644 packages/test/src/test/util/PublishedEntryIdentity.test.ts diff --git a/packages/test/src/test/util/PublishedEntryExportParity.test.ts b/packages/test/src/test/util/PublishedEntryExportParity.test.ts new file mode 100644 index 000000000..7bbb203fa --- /dev/null +++ b/packages/test/src/test/util/PublishedEntryExportParity.test.ts @@ -0,0 +1,184 @@ +/** + * @license + * Copyright 2026 Steven Roussey + * SPDX-License-Identifier: Apache-2.0 + */ + +import { existsSync, readdirSync, readFileSync } from "node:fs"; +import { dirname, join, resolve } from "node:path"; +import { fileURLToPath } from "node:url"; +import { describe, expect, it } from "vitest"; + +const repoRoot = resolve(dirname(fileURLToPath(import.meta.url)), "../../../../.."); + +/** + * The workspace groups, derived from the root manifest's `workspaces` field. + * + * Duplicated derivation code rather than a duplicated list, for the reason + * `PublishedEntryImports.test.ts` spells out: `packages/test` is a `composite` + * project rooted at `./src`, so importing the tooling's own copy under + * `scripts/` would pull those files into its program and break `build-types`. + */ +const WORKSPACE_GROUPS: readonly string[] = ( + JSON.parse(readFileSync(join(repoRoot, "package.json"), "utf8")) as { workspaces: string[] } +).workspaces.map((pattern) => pattern.replace(/^\.\//, "").replace(/\/?\*.*$/, "")); + +/** + * The conditions a plain `import` from Node activates, and only those. Node's + * own algorithm: walk the keys IN ORDER and take the first active one, so a + * `types`-first block resolves to whatever follows it rather than to nothing. + */ +const NODE_CONDITIONS: ReadonlySet = new Set(["node", "import", "default"]); + +/** Source extensions a built entry can have come from, in resolution order. */ +const SOURCE_EXTENSIONS = [".ts", ".tsx"] as const; + +/** + * Entries no CI job can import, each with the reason — the same map, for the + * same entry and the same reason, as `PublishedEntryImports.test.ts`. Kept + * deliberately small so a NEW package defaults to being checked. Every key is + * proved below to still name a published entry. + */ +const UNCHECKABLE: Readonly> = { + "@workglow/cli": + "an example app, and `packages/test` does not depend on it — under bunfig's " + + "isolated linker the specifier does not resolve from here at all, so importing " + + "it would test the linker rather than the bundle", +}; + +interface EntryPair { + /** The specifier a consumer writes, e.g. `@workglow/util/schema`. */ + readonly specifier: string; + /** Absolute path of the source module the built entry was built from. */ + readonly sourcePath: string; +} + +/** The target a conditional `exports` value resolves to under Node. */ +function resolveUnderNode(value: unknown): string | undefined { + if (typeof value === "string") return value; + if (typeof value !== "object" || value === null || Array.isArray(value)) return undefined; + for (const [condition, child] of Object.entries(value as Record)) { + if (!NODE_CONDITIONS.has(condition)) continue; + const resolved = resolveUnderNode(child); + if (resolved !== undefined) return resolved; + } + return undefined; +} + +/** + * `/dist/.js` → `/src/.{ts,tsx}` — the inverse of what + * the build emits, and the same mapping `use-source` writes its stubs from. + * + * Re-derived here rather than imported from `scripts/lib/workspaceSource.ts` + * for the composite-project reason above. Deliberately returns `undefined` + * rather than guessing for a target with no counterpart (generated or copied + * build output), so such an entry is reported as skipped instead of failing. + */ +function sourceCounterpart(packageDir: string, target: string): string | undefined { + const match = /^\.\/dist\/(?.+)\.(?:js|mjs|cjs)$/.exec(target); + if (!match?.groups) return undefined; + for (const extension of SOURCE_EXTENSIONS) { + const candidate = join(packageDir, "src", `${match.groups.entry}${extension}`); + if (existsSync(candidate)) return candidate; + } + return undefined; +} + +/** Every published Node entry that has a source counterpart to compare against. */ +function collectEntryPairs(): { pairs: EntryPair[]; unmapped: string[] } { + const pairs: EntryPair[] = []; + const unmapped: string[] = []; + for (const group of WORKSPACE_GROUPS) { + let directories: string[]; + try { + directories = readdirSync(join(repoRoot, group)); + } catch { + continue; + } + for (const directory of directories) { + const packageDir = join(repoRoot, group, directory); + let manifest: { name?: unknown; exports?: unknown }; + try { + manifest = JSON.parse(readFileSync(join(packageDir, "package.json"), "utf8")); + } catch { + continue; // not a package directory + } + const { name, exports } = manifest; + if (typeof name !== "string") continue; + if (typeof exports !== "object" || exports === null || Array.isArray(exports)) continue; + for (const [subpath, value] of Object.entries(exports as Record)) { + if (!subpath.startsWith(".")) continue; // a bare condition map, not a subpath + const target = resolveUnderNode(value); + if (target === undefined) continue; // browser-only entry; nothing for Node to load + const specifier = name + subpath.slice(1); + const sourcePath = sourceCounterpart(packageDir, target); + if (sourcePath === undefined) { + unmapped.push(`${specifier} -> ${target}`); + continue; + } + pairs.push({ specifier, sourcePath }); + } + } + } + pairs.sort((a, b) => a.specifier.localeCompare(b.specifier)); + return { pairs, unmapped }; +} + +const { pairs, unmapped } = collectEntryPairs(); +const checkable = pairs.filter((pair) => !(pair.specifier in UNCHECKABLE)); + +/** + * Every published entry, imported twice — once by the specifier a consumer + * writes, once by the source file it was built from — with the export NAME sets + * compared. + * + * Why names, and why both sides: a bundle that lost a re-export still resolves + * and still evaluates cleanly, so `PublishedEntryImports.test.ts`'s "loads and + * exports something" check passes over it unchanged. That test's `> 0` bound is + * satisfied by a bundle carrying one symbol out of ninety. The export list is + * the only observable that says the entry point is intact, and the source file + * is the only available statement of what it should be. + * + * Under the default `source` target both sides resolve to the same module, so + * this passes trivially — a green source run is not a bundle check. The run + * that means something is `test-vitest-dist` (`WORKGLOW_TEST_TARGET=dist`), + * where the left side is the real bundle; hence a unit-tier file, since that is + * the tier the dist job runs. + */ +describe("published entry export parity", () => { + it("enumerates every workspace manifest, so an empty sweep cannot pass", () => { + // Anti-vacuity: a typo in the walk yields a short list rather than an + // error, and a short list passes every assertion below. + expect(pairs.length).toBeGreaterThan(60); + }); + + it("maps every published entry back to a source file", () => { + // A published entry whose target is not `./dist/.js`, or whose + // source twin is missing, is silently dropped from the sweep above — the + // same hole in a different shape. Listed here so it fails loudly instead. + expect(unmapped).toEqual([]); + }); + + it("keeps every exemption pinned to an entry that still exists", () => { + // An exemption that outlives its package silently exempts nothing, and + // reads as if a real hole were still open. + const published = new Set(pairs.map((pair) => pair.specifier)); + expect(Object.keys(UNCHECKABLE).filter((specifier) => !published.has(specifier))).toEqual([]); + for (const reason of Object.values(UNCHECKABLE)) { + expect(reason.length).toBeGreaterThan(20); + } + }); + + it.each(checkable.map((pair) => [pair.specifier, pair.sourcePath] as const))( + "%s exports the same names as its source", + async (specifier, sourcePath) => { + const [published, source] = await Promise.all([ + import(/* @vite-ignore */ specifier) as Promise>, + import(/* @vite-ignore */ sourcePath) as Promise>, + ]); + // Sorted, so the diff on failure names the missing symbols rather than + // reporting two shuffled lists as unequal. + expect(Object.keys(published).sort()).toEqual(Object.keys(source).sort()); + } + ); +}); diff --git a/packages/test/src/test/util/PublishedEntryIdentity.test.ts b/packages/test/src/test/util/PublishedEntryIdentity.test.ts new file mode 100644 index 000000000..52a20f1c9 --- /dev/null +++ b/packages/test/src/test/util/PublishedEntryIdentity.test.ts @@ -0,0 +1,269 @@ +/** + * @license + * Copyright 2026 Steven Roussey + * SPDX-License-Identifier: Apache-2.0 + */ + +import { AiProvider, getAiProviderRegistry } from "@workglow/ai"; +import { AiProvider as WorkerAiProvider } from "@workglow/ai/worker"; +import { readdirSync, readFileSync } from "node:fs"; +import { dirname, join, resolve } from "node:path"; +import { fileURLToPath } from "node:url"; +import { beforeAll, describe, expect, it } from "vitest"; + +const repoRoot = resolve(dirname(fileURLToPath(import.meta.url)), "../../../../.."); + +/** + * The workspace groups, derived from the root manifest's `workspaces` field. + * + * Duplicated derivation code rather than a duplicated list, and duplicated for + * the same reason `PublishedEntryImports.test.ts` gives: `packages/test` is a + * `composite` project rooted at `./src`, so importing the tooling's own copy + * under `scripts/` would pull those files into its program and break + * `build-types`. + */ +const WORKSPACE_GROUPS: readonly string[] = ( + JSON.parse(readFileSync(join(repoRoot, "package.json"), "utf8")) as { workspaces: string[] } +).workspaces.map((pattern) => pattern.replace(/^\.\//, "").replace(/\/?\*.*$/, "")); + +/** + * Providers whose `ai-runtime` entry cannot be registered inside a plain CI + * job, each with the reason it cannot. + * + * EMPTY, and measured to be: the local providers this was expected to need — + * `node-llama-cpp`, `huggingface-transformers`, `cactus`, + * `stable-diffusion-server` — all register cleanly with no native runtime + * present, because `register*Inline` only CONSTRUCTS the provider and hands its + * run-fn table to the registry. Every SDK, native binding and server probe is + * behind a run-fn and is reached only by an actual generation call, which this + * file never makes. Exempting them would have been a false statement that cost + * five of the sixteen candidates their coverage. + * + * The map stays as the seam for a provider that genuinely cannot register. It + * is stated per entry and must be kept small, so a NEW provider defaults to + * being checked — a permissive default would reopen exactly the hole this file + * closes. Every key is proved below to still name a candidate package, so a + * renamed or deleted provider takes its exemption with it. + */ +const NEEDS_NATIVE_RUNTIME: Readonly> = {}; + +/** + * The base classes a provider is allowed to have been built from — the ones + * `@workglow/ai` itself PUBLISHES. + * + * There are two, and that is deliberate rather than an oversight this test + * papers over. `@workglow/ai` builds `.` and `./worker` as separate `bun build` + * invocations, so each bundle carries its own copy of `AiProvider`; the worker + * entry exists precisely so a worker bundle does not drag in the full node + * entry, and the five providers with a worker runtime (`chrome-ai`, + * `tf-mediapipe`, `cactus`, `huggingface-transformers`, `node-llama-cpp`) + * extend the worker copy on purpose. Under the `source` target both specifiers + * resolve to one file and this set collapses to a single class. + * + * That published split is a real cross-entry identity seam — a consumer holding + * `AiProvider` from `@workglow/ai` and testing a Chrome AI provider with + * `instanceof` gets `false` today — but it is pre-existing, intentional, and + * belongs to the `@workglow/ai` build rather than to this check. What this file + * catches is the THIRD copy: one inlined into a provider's own `ai-runtime` + * bundle, which matches neither published class. + */ +const PUBLISHED_BASE_CLASSES = [AiProvider, WorkerAiProvider] as const; + +interface RuntimeCandidate { + /** The package that publishes both `./ai` and `./ai-runtime`. */ + readonly packageName: string; + /** The `ai-runtime` specifier a consumer writes. */ + readonly specifier: string; +} + +/** Every workspace package publishing BOTH an `./ai` and an `./ai-runtime` entry. */ +function collectRuntimeCandidates(): RuntimeCandidate[] { + const candidates: RuntimeCandidate[] = []; + for (const group of WORKSPACE_GROUPS) { + let directories: string[]; + try { + directories = readdirSync(join(repoRoot, group)); + } catch { + continue; + } + for (const directory of directories) { + let manifest: { name?: unknown; exports?: unknown }; + try { + manifest = JSON.parse( + readFileSync(join(repoRoot, group, directory, "package.json"), "utf8") + ); + } catch { + continue; // not a package directory + } + const { name, exports } = manifest; + if (typeof name !== "string") continue; + if (typeof exports !== "object" || exports === null || Array.isArray(exports)) continue; + const subpaths = Object.keys(exports as Record); + // Both halves matter: `./ai` holds the class hierarchy every consumer + // holds, `./ai-runtime` holds the registration that constructs into it. + // A package publishing only one of them cannot exhibit the split. + if (!subpaths.includes("./ai") || !subpaths.includes("./ai-runtime")) continue; + candidates.push({ packageName: name, specifier: `${name}/ai-runtime` }); + } + } + return candidates.sort((a, b) => a.packageName.localeCompare(b.packageName)); +} + +const candidates = collectRuntimeCandidates(); +const checkable = candidates.filter((c) => !(c.packageName in NEEDS_NATIVE_RUNTIME)); + +/** What one package's `register*Inline` actually put into the registry. */ +interface Registration { + readonly packageName: string; + /** The `register*Inline` export that was called. */ + readonly registrarName: string; + /** Registry keys that appeared as a result of calling it. */ + readonly providerNames: readonly string[]; +} + +const registrations: Registration[] = []; +/** + * Packages whose `ai-runtime` exports no `register*Inline` at all — skipped + * rather than failed, and reported in the anti-vacuity message below so a + * silently shrinking sweep is visible. Today that is `@workglow/mlx`, whose + * `registerMlx` deliberately returns without touching the registry until an + * mlx-lm runtime is bundled. + */ +const withoutInlineRegistrar: string[] = []; + +/** + * What this file is actually checking, and why the obvious version of it is + * vacuous. + * + * The failure mode is CLASS identity across bundle boundaries, exactly as + * `packages/task-graph/src/test-entry.ts` documents it. `registerAnthropicInline` + * constructs `new AnthropicQueuedProvider(...)` from a RELATIVE import inside the + * `ai-runtime` module graph, while that class extends a base built from + * `AiProvider` imported BY SPECIFIER. Inline `@workglow/ai` into + * `ai-runtime.js` — a bundler flag, a dropped `external`, a re-export rewritten + * from `export *` to `export { … } from` — and the constructed instance stops + * being `instanceof` the `AiProvider` every consumer holds, while every + * existing check stays green. + * + * Asserting on the SERVICE REGISTRY instead would prove nothing: the global DI + * container is stashed on `Symbol.for("@workglow/util/di/globalContainer")` + * (`packages/util/src/di/Container.ts`) precisely so duplicated bundle copies + * share one instance, and `createServiceToken` returns a plain string id. A + * duplicated `@workglow/ai` therefore resolves the SAME registry, and a `===` + * assertion on it is green by construction. + * + * Under the default `source` target this file passes trivially — every + * specifier resolves to `src`, so there is only ever one copy of every class. A + * green source run is therefore NOT a bundle check. The run that means + * something is `test-vitest-dist` (`WORKGLOW_TEST_TARGET=dist`), which is why + * this is a unit-tier file: that is the tier the dist job runs. + * + * It is affordable there because `register*Inline` needs NO API key. It + * constructs the provider and calls `registerProviderInline`, which calls + * `provider.register(...)` — registry bookkeeping and a strategy resolver, no + * network. Putting this on an integration tier instead would have cost real + * money for no extra signal, and would have been skipped entirely on fork PRs, + * where the secrets those suites gate on are unavailable. + */ +describe("published entry identity", () => { + beforeAll(async () => { + const registry = getAiProviderRegistry(); + for (const candidate of checkable) { + const loaded: Record = await import(/* @vite-ignore */ candidate.specifier); + const registrarName = Object.keys(loaded).find((key) => /^register\w+Inline$/.test(key)); + if (registrarName === undefined) { + withoutInlineRegistrar.push(candidate.packageName); + continue; + } + const before = new Set(registry.getProviders().keys()); + await (loaded[registrarName] as () => Promise)(); + registrations.push({ + packageName: candidate.packageName, + registrarName, + providerNames: [...registry.getProviders().keys()].filter((name) => !before.has(name)), + }); + } + }); + + it("enumerates the providers that publish both an ai and an ai-runtime entry", () => { + // Anti-vacuity. A typo in the walk (wrong group key, wrong subpath name) + // yields a SHORT list rather than an error, and every assertion below + // passes over a short list — including over an empty one. + expect(candidates.length).toBeGreaterThan(4); + expect(checkable.length).toBeGreaterThan(4); + }); + + it("keeps every exemption pinned to a package that still exists", () => { + // An exemption that outlives its package silently exempts nothing while + // reading as if a real hole were still open. + const known = new Set(candidates.map((c) => c.packageName)); + expect(Object.keys(NEEDS_NATIVE_RUNTIME).filter((name) => !known.has(name))).toEqual([]); + for (const reason of Object.values(NEEDS_NATIVE_RUNTIME)) { + expect(reason.length).toBeGreaterThan(20); + } + }); + + it("registers a provider from more than a handful of runtime entries", () => { + // The anti-vacuity guard that matters: the per-provider assertions below + // iterate what registration actually produced, so a run in which every + // registration silently no-opped would satisfy all of them. + const providers = getAiProviderRegistry().getProviders(); + expect( + providers.size, + `only ${providers.size} provider(s) registered from ${checkable.length} runtime entries. ` + + `Entries exporting no register*Inline: ${withoutInlineRegistrar.join(", ") || "(none)"}` + ).toBeGreaterThan(4); + }); + + it("publishes no more base classes than @workglow/ai has entry points", () => { + // Guards the allowance above from growing quietly. One class under the + // `source` target (both specifiers are one file), two under `dist` (`.` and + // `./worker` are separate bundles). A third would mean a new split nobody + // decided on. + expect(new Set(PUBLISHED_BASE_CLASSES).size).toBeLessThanOrEqual(2); + }); + + it("constructs every provider from an AiProvider class @workglow/ai publishes", () => { + // THE check. A provider built against a copy of `AiProvider` inlined into + // its own runtime bundle is a perfectly functional object that fails this + // and nothing else — no other assertion anywhere distinguishes it. + const registry = getAiProviderRegistry(); + const offenders: string[] = []; + for (const { packageName, registrarName, providerNames } of registrations) { + for (const providerName of providerNames) { + const provider = registry.getProvider(providerName); + expect( + provider, + `${packageName}: ${registrarName}() registered "${providerName}" but the registry has no such provider` + ).toBeDefined(); + if (!PUBLISHED_BASE_CLASSES.some((base) => provider instanceof base)) { + offenders.push(`${packageName} -> ${providerName} (via ${registrarName})`); + } + } + } + // Collected rather than asserted in the loop, so one bad bundle reports + // itself instead of hiding every provider sorted after it. + expect( + offenders, + `these providers are not an instanceof any AiProvider that @workglow/ai publishes, which is ` + + `the signature of @workglow/ai being INLINED into the provider's own ai-runtime bundle ` + + `instead of left external: the runtime graph built its provider on a private copy of the ` + + `base class, so every consumer's instanceof check now returns false` + ).toEqual([]); + }); + + it("registers at least one run function per registered provider", () => { + // `instanceof` alone would still pass for a bundle that lost its run-fn + // module: the provider object is intact and serves nothing. + const registry = getAiProviderRegistry(); + const empty: string[] = []; + for (const { packageName, providerNames } of registrations) { + for (const providerName of providerNames) { + if (registry.getRunFnRegistrations(providerName).length === 0) { + empty.push(`${packageName} -> ${providerName}`); + } + } + } + expect(empty).toEqual([]); + }); +}); diff --git a/packages/test/src/test/util/PublishedEntryImports.test.ts b/packages/test/src/test/util/PublishedEntryImports.test.ts index 7e4c085d4..f6d72d737 100644 --- a/packages/test/src/test/util/PublishedEntryImports.test.ts +++ b/packages/test/src/test/util/PublishedEntryImports.test.ts @@ -11,8 +11,21 @@ import { describe, expect, it } from "vitest"; const repoRoot = resolve(dirname(fileURLToPath(import.meta.url)), "../../../../.."); -/** The root `workspaces` globs, in the order a manifest scan walks them. */ -const WORKSPACE_GROUPS = ["packages", "providers", "examples"] as const; +/** + * The workspace groups, derived from the root manifest's `workspaces` field: + * `"./packages/*"` → `"packages"`. + * + * This is duplicated DERIVATION CODE, not a duplicated list. The tooling + * keeps its own list under `scripts/`, pinned against this same manifest field + * by `scripts/workspaceSource.test.ts`; this file cannot import it, for the + * reason spelled out on the `describe` below — `packages/test` is a `composite` + * project rooted at `./src`, so importing from `scripts/` would put those files + * in its program and break `build-types`. Re-deriving still beats copying the list: a group + * added to `package.json` is picked up here instead of being silently skipped. + */ +const WORKSPACE_GROUPS: readonly string[] = ( + JSON.parse(readFileSync(join(repoRoot, "package.json"), "utf8")) as { workspaces: string[] } +).workspaces.map((pattern) => pattern.replace(/^\.\//, "").replace(/\/?\*.*$/, "")); /** * The conditions a plain `import` from Node activates, and only those. From 66038cebfeb32331dfeea828b4641db9d2bbc8c2 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 20 Sep 2026 23:30:46 +0000 Subject: [PATCH 5/7] test: pin the dist-target wiring, and type-check scripts/ MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `WORKGLOW_TEST_TARGET=dist` existed only as an inline `env:` block on one CI job. Nothing in-process can notice it going missing — an unset target resolves to `source` — so a workflow edit that dropped it would leave a green job that is a duplicate of the source run. `test:vitest:dist` becomes the one definition of the target, the CI job invokes the script, and `publishPipeline.test.ts` pins both, plus the `require-green-ci` step through which `publish-all` reaches that job before anything is versioned or pushed. `scripts/` was in no CI type gate — `typecheck:budget` globs packages|providers and `typecheck:tests` globs packages/*/tsconfig.test.json — while holding the resolution logic that decides what every suite in the repo resolves a `@workglow/*` specifier to. `tsconfig.scripts.json` plus `typecheck:scripts` close that. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01By1Tpxcr4qNBjauTK1QeNj --- .github/workflows/test.yml | 8 ++-- package.json | 2 + scripts/publishPipeline.test.ts | 72 +++++++++++++++++++++++++++++++++ tsconfig.scripts.json | 14 +++++++ 4 files changed, 93 insertions(+), 3 deletions(-) create mode 100644 scripts/publishPipeline.test.ts create mode 100644 tsconfig.scripts.json diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 77c45ab7a..96e48c1df 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -188,10 +188,12 @@ jobs: with: name: build-output path: . + # Invokes the script rather than restating the variable: one definition + # of "the dist target". An inline `env:` block here is how a workflow + # edit silently demotes this job to a duplicate source run — nothing + # in-process can notice, since an unset target resolves to `source`. - name: Run unit tests against the built bundles - env: - WORKGLOW_TEST_TARGET: dist - run: bun run test:vitest:unit -- ${{ env.TEST_CHANGED }} + run: bun run test:vitest:dist -- ${{ env.TEST_CHANGED }} test-vitest-integration: runs-on: ubuntu-latest needs: build diff --git a/package.json b/package.json index 92a36752a..d75a80391 100644 --- a/package.json +++ b/package.json @@ -27,6 +27,7 @@ "build:types": "turbo run build-types --concurrency=15", "typecheck:budget": "bun scripts/typecheck-budget.ts", "typecheck:tests": "for f in packages/*/tsconfig.test.json; do echo \"typecheck $f\" && tsc -p \"$f\" || exit 1; done", + "typecheck:scripts": "tsc -p tsconfig.scripts.json", "clean": "rm -rf node_modules packages/*/node_modules packages/*/*tsbuildinfo packages/*/dist packages/*/src/**/*\\.d\\.ts packages/*/src/**/*\\.map integrations/*/node_modules integrations/*/dist integrations/*/src/**/*\\.d\\.ts integrations/*/src/**/*\\.map examples/*/node_modules examples/*/dist examples/*/src/**/*\\.d\\.ts examples/*/src/**/*\\.map .turbo */*/.turbo", "dev": "turbo run dev --concurrency=15", "docs": "typedoc", @@ -43,6 +44,7 @@ "test:bun:ai-provider-nodellama": "bun scripts/test.ts bun integration provider-nodellama", "test:bun:ai-provider-api": "bun scripts/test.ts bun integration ai provider-api", "test:vitest:unit": "bun scripts/test.ts vitest unit", + "test:vitest:dist": "WORKGLOW_TEST_TARGET=dist bun scripts/test.ts vitest unit", "test:vitest:integration": "bun scripts/test.ts vitest integration --except rag,browser,provider-hft,provider-nodellama,provider-api,provider-cactus", "test:vitest:rag": "bun scripts/test.ts vitest integration rag", "test:vitest:ai-provider": "bun scripts/test.ts vitest integration ai provider", diff --git a/scripts/publishPipeline.test.ts b/scripts/publishPipeline.test.ts new file mode 100644 index 000000000..f7ce50835 --- /dev/null +++ b/scripts/publishPipeline.test.ts @@ -0,0 +1,72 @@ +/** + * @license + * Copyright 2026 Steven Roussey + * SPDX-License-Identifier: Apache-2.0 + */ + +import { readFileSync } from "node:fs"; +import { join } from "node:path"; +import { describe, expect, it } from "vitest"; +import { ROOT } from "./lib/testDiscovery"; + +/** + * The wiring that decides whether anything ever exercises the BUILT BUNDLES. + * + * Every vitest project attaches the source-resolving plugin, so a run that does + * not ask for the `dist` target resolves `@workglow/*` to `src` and the bundles + * are never loaded at all. The `test-vitest-dist` CI job is the one caller that + * asks for it, and `publish-all` reaches it through `require-green-ci`, which + * requires this same workflow green for HEAD before anything is versioned or + * pushed. + * + * Read as TEXT, on purpose. Importing the manifest would answer "what does the + * JSON parse to", which is not the question: the question is whether the shell + * command a human reads names the script. And the workflow is YAML that this + * repo has no parser for, so treating it as text keeps the guard dependency-free + * and lets it assert the ABSENCE of a key, which a parse would have to walk the + * whole document to do. + */ +describe("dist-target wiring", () => { + const manifest = JSON.parse(readFileSync(join(ROOT, "package.json"), "utf8")) as { + scripts: Record; + }; + const workflow = readFileSync(join(ROOT, ".github/workflows/test.yml"), "utf8"); + + it("defines the dist target inside the script, not at the call site", () => { + // The variable is what makes the run mean anything, and no in-process + // assertion can notice it went missing: `resolveTestTarget(undefined)` + // returns "source", so a run stripped of it is a silent, green rerun of the + // source job. Setting it in the script is the structural fix — dropping it + // now requires deleting the script call, which the two checks below catch. + const script = manifest.scripts["test:vitest:dist"]; + expect(script).toBeDefined(); + expect(script).toContain("WORKGLOW_TEST_TARGET=dist"); + }); + + it("gates publishing on the CI run that covers the bundles", () => { + // `publish-all` runs no suite itself: it requires this workflow green for + // HEAD instead. That delegation only covers the bundles while the dist job + // is part of the workflow, and before the version bump — after `bunset`, + // HEAD names a commit no workflow has seen. + const publish = manifest.scripts["publish-all"]; + expect(publish).toBeDefined(); + expect(publish).toContain("require-green-ci"); + expect(publish!.indexOf("require-green-ci")).toBeLessThan(publish!.indexOf("bunset")); + expect(workflow).toContain("test-vitest-dist:"); + }); + + it("has CI invoke the same script rather than restate the variable", () => { + expect(workflow).toContain("bun run test:vitest:dist"); + // One definition of "the dist target". An inline `env:` block here is how + // CI and `publish-all` drift apart, and how a workflow edit silently + // demotes the bundle-integrity job to a duplicate source run. + expect(workflow).not.toMatch(/^\s*WORKGLOW_TEST_TARGET:/m); + }); + + it("keeps the dist pass out of the concurrent all-suites script", () => { + // `test:vitest:all` fans seven suites out at once; the dist pass measures + // nothing they do not already cover under the source target, so adding it + // there buys nothing and competes for the same cores. + expect(manifest.scripts["test:vitest:all"]).not.toContain("test:vitest:dist"); + }); +}); diff --git a/tsconfig.scripts.json b/tsconfig.scripts.json new file mode 100644 index 000000000..38ea4f15c --- /dev/null +++ b/tsconfig.scripts.json @@ -0,0 +1,14 @@ +{ + // `scripts/` is in no other CI type gate — `typecheck:budget` globs + // packages|providers and `typecheck:tests` globs packages/*/tsconfig.test.json + // — while it holds vitest.config.ts's resolution logic, which decides what + // every suite in the repo resolves a `@workglow/*` specifier to. + "extends": "./tsconfig.json", + "include": ["scripts/**/*.ts", "vitest.config.ts"], + "compilerOptions": { + "composite": false, + "incremental": false, + "noEmit": true, + "allowImportingTsExtensions": true + } +} From bdc511f4f52656716dfb61aee0dad090551da9d5 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 16 Aug 2026 09:49:15 +0000 Subject: [PATCH 6/7] test: declare the identity sweep's skips instead of letting them shrink it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The sweep skipped the exact regression it exists to catch. A candidate whose `ai-runtime` module exports no `register*Inline` was pushed onto `withoutInlineRegistrar` and `continue`d, contributing NO assertion, and the only place that surfaced was a message printed on failure. Sixteen workspace packages publish both `./ai` and `./ai-runtime`, and every bound was `> 4`, so eleven of the sixteen could drop their registrar and leave the file green. Measured rather than reasoned: renaming `registerAnthropicInline` and rebuilding the provider left all six assertions passing, with `@workglow/anthropic` gone from the sweep and nothing saying so. - `NO_INLINE_REGISTRAR` declares the skips, beside `NEEDS_NATIVE_RUNTIME`. Sole member today is `@workglow/mlx`, whose registrar is `registerMlx` — no `Inline` suffix — because `MlxProvider` stays unavailable until an mlx-lm runtime is bundled. `withoutInlineRegistrar` is compared for EQUALITY against its keys, so an undeclared skip fails. - The exemption-pinning test covers both maps: every key names a real candidate, every reason is longer than a word. - `MINIMUM_RUNTIME_CANDIDATES` replaces `candidates.length > 4`, and `checkable.length` is now an equality against candidates minus the declared native-runtime exemptions rather than a second floor. - The `providers.size > 4` bound becomes two statements that cannot be satisfied by a shrunken sweep: the registered package set EQUALS the checkable set minus the declared no-registrar packages, and each registration is checked for at least one provider name, collected into an offenders array so one no-opping registrar reports itself. - The base-class allowance is now two-sided and target-keyed: `dist` requires exactly 2 distinct classes (two bundles really loaded), `source` exactly 1 (the resolution plugin really attached). `<= 2` was satisfied by either, so it could not tell a real dist run from a source run mislabelled as one. This is the in-process proof that the loaded modules are bundles. With the rename still applied the new file fails, naming `@workglow/anthropic` in both the undeclared-skip check and the registered-package set; reverted and rebuilt, it passes on both targets. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Lgxtp7mQECdh7F2UT9CVwN --- .../test/util/PublishedEntryIdentity.test.ts | 131 ++++++++++++++---- 1 file changed, 106 insertions(+), 25 deletions(-) diff --git a/packages/test/src/test/util/PublishedEntryIdentity.test.ts b/packages/test/src/test/util/PublishedEntryIdentity.test.ts index 52a20f1c9..12f1a6917 100644 --- a/packages/test/src/test/util/PublishedEntryIdentity.test.ts +++ b/packages/test/src/test/util/PublishedEntryIdentity.test.ts @@ -47,6 +47,39 @@ const WORKSPACE_GROUPS: readonly string[] = ( */ const NEEDS_NATIVE_RUNTIME: Readonly> = {}; +/** + * Candidates whose `ai-runtime` entry exports no `register*Inline` at all, each + * with the reason. + * + * Such a candidate contributes NO assertion to this file: the sweep records it + * and moves on. That is correct — there is nothing to construct, so there is no + * class identity to check — but it has to be DECLARED, because the alternative + * is a sweep that shrinks in silence. Nothing stood behind it: the skip was + * surfaced only inside a message that prints on failure, and the numeric bound + * guarding it was `> 4` against sixteen candidates, so eleven providers could + * drop their registrar and leave this file green. Measured, not assumed — + * renaming `registerAnthropicInline` and rebuilding the provider left every + * assertion here passing. + * + * Today the sole member is `@workglow/mlx`, which exports `registerMlx` with no + * `Inline` suffix: `MlxProvider.isAvailable` reports `false` until an mlx-lm + * runtime is bundled, so there is nothing to register inline yet. + */ +const NO_INLINE_REGISTRAR: Readonly> = { + "@workglow/mlx": + "exports registerMlx rather than register*Inline — MlxProvider stays unavailable until an mlx-lm runtime is bundled, so it registers nothing", +}; + +/** + * The floor the enumeration has to clear for anything below to mean something. + * + * Deliberately below today's sixteen — a provider may legitimately be removed — + * but far enough above zero that a typo in the walk (wrong group key, wrong + * subpath name) fails instead of yielding a short list every assertion passes + * over. + */ +const MINIMUM_RUNTIME_CANDIDATES = 12; + /** * The base classes a provider is allowed to have been built from — the ones * `@workglow/ai` itself PUBLISHES. @@ -124,10 +157,8 @@ interface Registration { const registrations: Registration[] = []; /** * Packages whose `ai-runtime` exports no `register*Inline` at all — skipped - * rather than failed, and reported in the anti-vacuity message below so a - * silently shrinking sweep is visible. Today that is `@workglow/mlx`, whose - * `registerMlx` deliberately returns without touching the registry until an - * mlx-lm runtime is bundled. + * rather than failed, and compared below against {@link NO_INLINE_REGISTRAR} + * so a skip nobody declared fails instead of shrinking the sweep. */ const withoutInlineRegistrar: string[] = []; @@ -189,38 +220,88 @@ describe("published entry identity", () => { // Anti-vacuity. A typo in the walk (wrong group key, wrong subpath name) // yields a SHORT list rather than an error, and every assertion below // passes over a short list — including over an empty one. - expect(candidates.length).toBeGreaterThan(4); - expect(checkable.length).toBeGreaterThan(4); + expect(candidates.length).toBeGreaterThanOrEqual(MINIMUM_RUNTIME_CANDIDATES); + // An EQUALITY, not a second floor: the checked set is the candidate set + // minus exactly the declared native-runtime exemptions, so a candidate that + // falls out of the sweep for any undeclared reason fails here. + expect(checkable.length).toBe(candidates.length - Object.keys(NEEDS_NATIVE_RUNTIME).length); }); it("keeps every exemption pinned to a package that still exists", () => { // An exemption that outlives its package silently exempts nothing while - // reading as if a real hole were still open. + // reading as if a real hole were still open. Both maps, since either one + // removes a candidate from the checks below. const known = new Set(candidates.map((c) => c.packageName)); - expect(Object.keys(NEEDS_NATIVE_RUNTIME).filter((name) => !known.has(name))).toEqual([]); - for (const reason of Object.values(NEEDS_NATIVE_RUNTIME)) { - expect(reason.length).toBeGreaterThan(20); + const maps = [ + ["NEEDS_NATIVE_RUNTIME", NEEDS_NATIVE_RUNTIME], + ["NO_INLINE_REGISTRAR", NO_INLINE_REGISTRAR], + ] as const; + for (const [label, map] of maps) { + expect( + Object.keys(map).filter((name) => !known.has(name)), + label + ).toEqual([]); + for (const [name, reason] of Object.entries(map)) { + // A one-word reason is an exemption nobody can review. + expect(reason.length, `${label}["${name}"]`).toBeGreaterThan(20); + } } }); - it("registers a provider from more than a handful of runtime entries", () => { - // The anti-vacuity guard that matters: the per-provider assertions below - // iterate what registration actually produced, so a run in which every - // registration silently no-opped would satisfy all of them. - const providers = getAiProviderRegistry().getProviders(); + it("declares every candidate whose runtime entry exports no register*Inline", () => { + // The skip that was invisible: such a candidate is pushed onto this list + // and `continue`d, contributing NO assertion, and the only place it + // surfaced was a message that prints on failure. Comparing against the + // declared map means a provider that drops its registrar in a refactor + // fails here instead of quietly leaving the sweep. expect( - providers.size, - `only ${providers.size} provider(s) registered from ${checkable.length} runtime entries. ` + - `Entries exporting no register*Inline: ${withoutInlineRegistrar.join(", ") || "(none)"}` - ).toBeGreaterThan(4); + [...withoutInlineRegistrar].sort(), + "a runtime entry exporting no register*Inline is checked by nothing — declare it in " + + "NO_INLINE_REGISTRAR with the reason, or restore its registrar" + ).toEqual(Object.keys(NO_INLINE_REGISTRAR).sort()); + }); + + it("registers at least one provider from every runtime entry that has a registrar", () => { + // Replaces a `> 4` bound that fifteen registering entries cleared with + // eleven to spare. Two statements instead of a magic number: exactly the + // expected packages ran a registrar, and each one actually put something in + // the registry — the per-provider assertions below iterate what + // registration produced, so a run in which every call silently no-opped + // would satisfy all of them. + const expected = checkable + .map((candidate) => candidate.packageName) + .filter((name) => !(name in NO_INLINE_REGISTRAR)) + .sort(); + expect(registrations.map((registration) => registration.packageName).sort()).toEqual(expected); + + const registeredNothing = registrations + .filter((registration) => registration.providerNames.length < 1) + .map((registration) => `${registration.packageName} (via ${registration.registrarName})`); + // Collected rather than asserted in the loop, so one no-opping registrar + // reports itself instead of being averaged away by the fourteen that worked. + expect( + registeredNothing, + `these register*Inline calls added no provider to the registry, so every per-provider ` + + `assertion below iterates nothing for them` + ).toEqual([]); }); - it("publishes no more base classes than @workglow/ai has entry points", () => { - // Guards the allowance above from growing quietly. One class under the - // `source` target (both specifiers are one file), two under `dist` (`.` and - // `./worker` are separate bundles). A third would mean a new split nobody - // decided on. - expect(new Set(PUBLISHED_BASE_CLASSES).size).toBeLessThanOrEqual(2); + it("loads one AiProvider class per @workglow/ai entry point the target actually has", () => { + // TWO-SIDED, and this is the in-process proof that the modules under test + // are BUNDLES rather than src. `@workglow/ai` builds `.` and `./worker` as + // separate `bun build` invocations, so under `dist` there really are two + // distinct classes; under `source` both specifiers resolve through one + // underlying module and there is exactly one. The old `<= 2` was satisfied + // by either, so it could not tell a real dist run from a source run + // mislabelled as one — nor from a stubbed `dist` re-exporting src. + // + // The variable read here is the one `resolveTestTarget` already validated + // in vitest.config.ts and handed to the workers, so `=== "dist"` cannot + // silently mean "source" for a typo the way an unvalidated read would. + const target = process.env.WORKGLOW_TEST_TARGET ?? "source"; + expect(new Set(PUBLISHED_BASE_CLASSES).size, `WORKGLOW_TEST_TARGET=${target}`).toBe( + target === "dist" ? 2 : 1 + ); }); it("constructs every provider from an AiProvider class @workglow/ai publishes", () => { From ee6ee02801f92e56fd0f2ca593a238a402a64d84 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 20 Sep 2026 23:33:42 +0000 Subject: [PATCH 7/7] test: skip the parity sweep where it compares a module with itself MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Under the default `source` target the resolving plugin rewrites `import(specifier)` to exactly the path `sourceCounterpart()` computes, so both sides are the SAME module and every case asserts `X === X`. Those cases now skip, so the report distinguishes "checked" from "not applicable" rather than showing ~90 green rows that compared nothing. Nothing stops being loaded: `PublishedEntryImports.test.ts` imports every published specifier unconditionally and does carry signal under source. A skip keyed on a value that can go missing is its own hazard, so the target is handed down validated through `test.env` and an anti-vacuity case pins it to one of the two known values — otherwise broken plumbing would skip every case in every job and take `test-vitest-dist` green having compared nothing. That plumbing is vitest's, so both sweeps carry `@vitest-environment`, which is what keeps them off the Bun runner. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01By1Tpxcr4qNBjauTK1QeNj --- .../util/PublishedEntryExportParity.test.ts | 47 ++++++++++++++++--- .../test/util/PublishedEntryIdentity.test.ts | 9 ++++ vitest.config.ts | 6 +++ 3 files changed, 56 insertions(+), 6 deletions(-) diff --git a/packages/test/src/test/util/PublishedEntryExportParity.test.ts b/packages/test/src/test/util/PublishedEntryExportParity.test.ts index 7bbb203fa..02e6301d2 100644 --- a/packages/test/src/test/util/PublishedEntryExportParity.test.ts +++ b/packages/test/src/test/util/PublishedEntryExportParity.test.ts @@ -4,6 +4,15 @@ * SPDX-License-Identifier: Apache-2.0 */ +// @vitest-environment node +// +// The tag is what keeps this file off the Bun runner, which is the point of it +// here rather than any DOM: the target this sweep branches on is handed down by +// `vitest.config.ts`'s `test.env`, and `bun test` never reads that config. Under +// Bun the validated value would simply be absent, and the anti-vacuity case +// below would fail on the plumbing instead of on anything under test. `node` is +// vitest's default environment, so the tag costs nothing there. + import { existsSync, readdirSync, readFileSync } from "node:fs"; import { dirname, join, resolve } from "node:path"; import { fileURLToPath } from "node:url"; @@ -127,6 +136,16 @@ function collectEntryPairs(): { pairs: EntryPair[]; unmapped: string[] } { const { pairs, unmapped } = collectEntryPairs(); const checkable = pairs.filter((pair) => !(pair.specifier in UNCHECKABLE)); +/** + * Whether this run exercises the built bundles. + * + * The VALIDATED target handed down by `vitest.config.ts`'s `test.env`, not + * re-derived: `packages/test` is a composite program rooted at `./src` and + * cannot import `scripts/lib/*`, so a second `=== "dist"` comparison here would + * be exactly the silent-typo bug `resolveTestTarget` exists to remove. + */ +const RUNS_AGAINST_BUNDLES = process.env.WORKGLOW_TEST_TARGET === "dist"; + /** * Every published entry, imported twice — once by the specifier a consumer * writes, once by the source file it was built from — with the export NAME sets @@ -139,11 +158,18 @@ const checkable = pairs.filter((pair) => !(pair.specifier in UNCHECKABLE)); * the only observable that says the entry point is intact, and the source file * is the only available statement of what it should be. * - * Under the default `source` target both sides resolve to the same module, so - * this passes trivially — a green source run is not a bundle check. The run - * that means something is `test-vitest-dist` (`WORKGLOW_TEST_TARGET=dist`), - * where the left side is the real bundle; hence a unit-tier file, since that is - * the tier the dist job runs. + * Under the default `source` target the source-resolving plugin rewrites + * `import(specifier)` to exactly the path `sourceCounterpart()` computes, so + * both sides ARE the same module and every case asserts `X === X`. Those cases + * are therefore SKIPPED under source, so the report distinguishes "checked" + * from "not applicable" rather than showing ~90 green rows that compared + * nothing. The run that means something is `test-vitest-dist` + * (`WORKGLOW_TEST_TARGET=dist`), where the left side is the real bundle; hence + * a unit-tier file, since that is the tier the dist job runs. + * + * Skipping loses no loading: `PublishedEntryImports.test.ts` imports every + * published specifier unconditionally, and that file DOES carry signal under + * source — it is what proves each entry resolves and evaluates at all. */ describe("published entry export parity", () => { it("enumerates every workspace manifest, so an empty sweep cannot pass", () => { @@ -169,7 +195,16 @@ describe("published entry export parity", () => { } }); - it.each(checkable.map((pair) => [pair.specifier, pair.sourcePath] as const))( + it("is handed a validated target, so the skip cannot swallow the dist sweep", () => { + // If the `test.env` plumbing broke, `RUNS_AGAINST_BUNDLES` would be false + // in every job: every case below would report skipped and + // `test-vitest-dist` would go green having compared nothing at all. + expect(["source", "dist"]).toContain(process.env.WORKGLOW_TEST_TARGET); + }); + + it + .skipIf(!RUNS_AGAINST_BUNDLES) + .each(checkable.map((pair) => [pair.specifier, pair.sourcePath] as const))( "%s exports the same names as its source", async (specifier, sourcePath) => { const [published, source] = await Promise.all([ diff --git a/packages/test/src/test/util/PublishedEntryIdentity.test.ts b/packages/test/src/test/util/PublishedEntryIdentity.test.ts index 12f1a6917..736c56520 100644 --- a/packages/test/src/test/util/PublishedEntryIdentity.test.ts +++ b/packages/test/src/test/util/PublishedEntryIdentity.test.ts @@ -4,6 +4,15 @@ * SPDX-License-Identifier: Apache-2.0 */ +// @vitest-environment node +// +// The tag is what keeps this file off the Bun runner, which is the point of it +// here rather than any DOM: the target this sweep branches on is handed down by +// `vitest.config.ts`'s `test.env`, and `bun test` never reads that config. Under +// Bun the validated value would simply be absent, and the anti-vacuity case +// below would fail on the plumbing instead of on anything under test. `node` is +// vitest's default environment, so the tag costs nothing there. + import { AiProvider, getAiProviderRegistry } from "@workglow/ai"; import { AiProvider as WorkerAiProvider } from "@workglow/ai/worker"; import { readdirSync, readFileSync } from "node:fs"; diff --git a/vitest.config.ts b/vitest.config.ts index c31e1a640..a065a2906 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -212,6 +212,12 @@ const projects = listTestProjects(discovered).map((p) => { ...shared, name: p.name, root, + // The VALIDATED target, handed down rather than re-derived. A suite that + // read `process.env` itself sees `undefined` on a default run — the + // resolution `resolveTestTarget` exists to perform — so a case that skips + // unless the target is `dist` would skip in every job, reporting green + // having compared nothing. + env: { WORKGLOW_TEST_TARGET: target }, exclude: [...shared.exclude, ...bunOnly], typecheck: { ...shared.typecheck,