Skip to content

test: make publish-all exercise the bundles it ships, and gate the dist target - #817

Merged
sroussey merged 1 commit into
claude/coverage-dist-bundle-fix-ew0vj8from
claude/optimistic-goldberg-d74u5n-publish-tests-bundles
Aug 16, 2026
Merged

test: make publish-all exercise the bundles it ships, and gate the dist target#817
sroussey merged 1 commit into
claude/coverage-dist-bundle-fix-ew0vj8from
claude/optimistic-goldberg-d74u5n-publish-tests-bundles

Conversation

@sroussey

Copy link
Copy Markdown
Collaborator

Stacked fix onto #741 (claude/coverage-dist-bundle-fix-ew0vj8), which is the base of this PR — not main. Review/merge #741 first.

HIGH — publish-all no longer tests the artifacts it publishes

Before #741, every vitest run resolved @workglow/* through exports into dist, so publish-all's rebuild && format && test vitest exercised the bundles it was about to version-bump and push. #741 attaches the source-resolution plugin to every project unconditionally under the default target, so that step now exercises src only — and WORKGLOW_TEST_TARGET=dist existed solely as an inline env: block on one CI job. There was no script exposing the dist target at all, and publish-all was not updated. The three new PublishedEntry*.test.ts files document themselves as vacuous under the source target.

  • test:vitest:distWORKGLOW_TEST_TARGET=dist bun scripts/test.ts vitest unit. Unit tier only, matching what the CI job runs: the integration/rag/provider tiers want API keys, databases and multi-GB model downloads, which is not what a bundle-wiring check needs. The variable is set inside the script, not by the caller — that is what makes it impossible for a workflow edit to silently drop it.
  • publish-all runs it after rebuild/format and before bunset, so the bundles under test are the bundles about to be tagged and pushed. The existing bun run test vitest stays.
  • test-vitest-dist in test.yml drops its inline env: block and calls bun run test:vitest:dist. One definition of "the dist target", shared by CI and publish.
  • Deliberately not added to test:vitest:all — that fans seven suites out concurrently and merges coverage fragments; the dist pass emits none (scripts/test.ts skips --coverage when targeting dist).

MEDIUM — the dist run had no self-verification and ignored the source-stub sentinel

use-source writes stubs carrying SOURCE_STUB_SENTINEL (scripts/lib/sourceStubs.ts) and nothing here read it. Under a stubbed dist, @workglow/ai and @workglow/ai/worker collapse onto one source module: every cross-entry instanceof succeeds trivially and export-name parity compares a file with itself. The run stays green and covers nothing.

  • containsSourceStubSentinel(file) — a node-portable (node:fs) sentinel probe. The existing isSourceStub uses Bun.file and is therefore Bun-only, unusable from vitest.config.ts, which Vite loads under node. The async helper now delegates to it, so publish-workspaces.ts keeps working unchanged and the two can never disagree about what a stub is.
  • assertNoSourceStubs(packages) — walks each package's exports dist targets and throws one aggregated error naming every offender, pointing at bun run use-dist. A developer who ran use-source has stubbed all 39 packages; failing on the first would take 39 fix-and-rerun cycles to learn one fact.
  • vitest.config.ts keeps the resolved target in a const and calls it at config load under dist. Failing there rather than in a test is deliberate: it kills the whole run before any suite can pass vacuously, and it covers the export-parity sweep too, which a test-shaped guard could only protect by being imported into every file it defends.
  • shared.env = { WORKGLOW_TEST_TARGET: target } hands the validated value to the workers. packages/test is a composite program rooted at ./src and cannot import scripts/lib/* (which is why the three PublishedEntry files duplicate their derivations), so re-implementing the === "dist" comparison there would reintroduce exactly the silent-typo bug resolveTestTarget exists to remove.

Note on what cannot be asserted. No in-process check can distinguish "the job meant source" from "the env block was deleted": resolveTestTarget(undefined) returns "source", so a target-keyed assertion just flips branches and stays green. The only fix is structural — the variable is now set by test:vitest:dist itself, so deleting it requires deleting the script call, which the wiring guard below catches.

MEDIUM — scripts/ is outside every CI type gate

typecheck:budget globs packages|providers and typecheck:tests globs packages/*/tsconfig.test.json, so nothing typechecked scripts/ — now home to vitest.config.ts's resolution logic, and already carrying real errors.

  • tsconfig.scripts.json at the root (not scripts/tsconfig.json, which risks being swept up by a future */tsconfig.json glob): include: ["scripts/**/*.ts", "vitest.config.ts"], composite: false, incremental: false, noEmit: true, allowImportingTsExtensions: true. The last is required rather than cosmetic — workspaceSource.ts imports ./workspaceGroups.ts and vitest.config.ts imports ./scripts/lib/*.ts with explicit extensions (Vite's native config loader needs them), which is TS5097 without it; legal here because noEmit is set.
  • typecheck:scripts (tsgo, the same binary typecheck:tests uses) plus a step in the typecheck-budget job beside "Typecheck co-located tests".

The first run surfaced four real errors, all fixed here:

error fix
TS2741 ×3 — WorkspacePackage gained a required readonly publishes in #741; three annotated literals in scripts/workspaceSource.test.ts omit it added publishes: true
TS2345 — scripts/testDiscovery.test.ts annotates a .map callback as { test: { root: string } }, but TestProjectConfiguration also admits a glob string read the root defensively instead
TS2307 — import type { Plugin } from "vite" with no root vite devDependency (it exists only nested under vitest) added vite to root devDependencies
TS2769 — coverage.root see below

coverage.root was inert. Vitest 4 neither types it nor reads it: BaseCoverageProvider derives the roots it globs from ctx.config.root / the resolved project configs, never from the coverage block (this.roots = ctx.config.project?.length ? ... : [ctx.config.root]). Removing it is behaviour-neutral. What actually makes the repo-relative include globs work from a package directory is pm.isMatch(..., { contains: true }), which the comment now says.

Out of scope, noted for a follow-up: eslint.config.js also scopes to {packages,providers,examples}/**, so scripts/ is unlinted as well.

Tests

scripts/publishPipeline.test.ts (new) — a dependency-free wiring guard reading the raw root package.json and .github/workflows/test.yml as text, because the question is whether the shell command a human reads names the script, and because asserting the absence of a YAML key is what a text read does well:

  • test:vitest:dist exists and its command contains WORKGLOW_TEST_TARGET=dist
  • publish-all contains test:vitest:dist, before bunset
  • test.yml contains bun run test:vitest:dist and no bare WORKGLOW_TEST_TARGET: YAML key
  • test:vitest:all does not contain it

Measured on the base commit: 3 of 4 fail, the fourth passing only because the script did not exist.

scripts/workspaceSource.test.ts — three cases driving assertNoSourceStubs over mkdtempSync fixtures: a dist file containing the sentinel throws and names the file and use-dist; a real-looking bundle and an exports entry that was never built both pass; two stubbed packages are both named in one message.

Verification (actual output)

$ bun run use-source && bun run test:vitest:dist
Wrote 280 stub(s) across 39 package(s). Imports now hit src/.
…/packages/workglow/dist/xai.js
    at assertNoSourceStubs (…/vitest.config.ts.timestamp-….mjs:418:8)
error: script "test:vitest:dist" exited with code 1        # fails at config load, naming every stub

$ bun run use-dist
Tasks:    81 successful, 81 total

$ bun run test:vitest:dist
Test Files  537 passed | 3 skipped (540)
     Tests  6272 passed | 71 skipped (6343)

$ bun run typecheck:scripts     # EXIT=0
$ bun run typecheck:budget      # typecheck-budget: OK (38 packages within budget)
$ bun run typecheck:tests       # EXIT=0

Generated by Claude Code

@github-actions

Copy link
Copy Markdown

Coverage Report

Status Category Percentage Covered / Total
🔵 Lines 71.81% 29669 / 41311
🔵 Statements 70.62% 31706 / 44891
🔵 Functions 72.33% 6136 / 8483
🔵 Branches 61.18% 16006 / 26162
File CoverageNo changed files found.
Generated in workflow #3167 for commit 5c5071c by the Vitest Coverage Report Action

…st target

Every vitest project now attaches the source-resolving plugin unconditionally,
so under the default target nothing resolves `@workglow/*` through `exports`.
`publish-all`'s test step therefore stopped touching the bundles it is about to
version-bump and push, and `WORKGLOW_TEST_TARGET=dist` existed only as an inline
`env:` block on one CI job.

- `test:vitest:dist` is the one definition of the dist target, unit tier only
  (matching the CI job; the other tiers want keys, databases and model
  downloads). The variable is set INSIDE the script, so dropping it now means
  deleting the script call.
- `publish-all` runs it after `rebuild`/`format` and before `bunset`.
- The `test-vitest-dist` job invokes the script instead of restating the
  variable.

The dist run also had no precondition. `use-source` writes stubs carrying
`SOURCE_STUB_SENTINEL` and nothing read it, so under a stubbed dist
`@workglow/ai` and `@workglow/ai/worker` collapse onto one source module: every
cross-entry `instanceof` succeeds trivially and export-name parity compares a
file with itself. `assertNoSourceStubs` now runs at CONFIG LOAD under the dist
target, which kills the whole run before any suite can pass vacuously.
`containsSourceStubSentinel` is the node-portable half of the existing Bun-only
`isSourceStub`, since vitest.config.ts is loaded by Vite under node.

`shared.env` hands the validated target down to the workers: `packages/test` is
a composite program rooted at ./src and cannot import scripts/lib/*, so
re-deriving it there would reintroduce the silent-comparison bug
`resolveTestTarget` exists to remove.

`scripts/` was in no CI type gate — `typecheck:budget` globs packages|providers
and `typecheck:tests` globs packages/*/tsconfig.test.json — while now holding
vitest.config.ts's resolution logic. `tsconfig.scripts.json` + `typecheck:scripts`
close that, and the first run surfaced four real errors, all fixed here:
three `WorkspacePackage` literals missing the required `publishes`, a
`TestProjectConfiguration` callback annotation that ignores the union's string
variant, a missing root `vite` devDependency behind
`import type { Plugin } from "vite"`, and a `coverage.root` key that vitest 4
neither types nor reads (its provider derives coverage roots from the resolved
project configs), so it was inert.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Lgxtp7mQECdh7F2UT9CVwN
@sroussey
sroussey force-pushed the claude/optimistic-goldberg-d74u5n-publish-tests-bundles branch from 5c5071c to 7014948 Compare August 16, 2026 16:37
@sroussey
sroussey merged commit 7014948 into claude/coverage-dist-bundle-fix-ew0vj8 Aug 16, 2026
11 of 12 checks passed
@sroussey
sroussey deleted the claude/optimistic-goldberg-d74u5n-publish-tests-bundles branch August 24, 2026 18:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants