Skip to content

fix(scripts): validate WORKGLOW_TEST_TARGET and rank the unresolved-specifier causes - #759

Merged
sroussey merged 1 commit into
claude/coverage-dist-bundle-fix-ew0vj8from
claude/optimistic-goldberg-4xvngr-dist-target-guards
Aug 13, 2026
Merged

fix(scripts): validate WORKGLOW_TEST_TARGET and rank the unresolved-specifier causes#759
sroussey merged 1 commit into
claude/coverage-dist-bundle-fix-ew0vj8from
claude/optimistic-goldberg-4xvngr-dist-target-guards

Conversation

@sroussey

Copy link
Copy Markdown
Collaborator

Stacked on #741 (claude/coverage-dist-bundle-fix-ew0vj8). Two independent fixes to the machinery that PR introduced.

(a) WORKGLOW_TEST_TARGET is validated, not guessed

vitest.config.ts:72 and scripts/test.ts:122 each read the variable and each treated any value other than the literal "dist" as source mode — silently, and independently of the other.

The failure that buys: a workflow edit to "dist " (trailing space) or Dist turns test-vitest-dist into a byte-identical rerun of test-vitest-unit. It goes green, spends ~15 minutes of runner time, and provides zero coverage of the built bundles — which is the entire reason that job exists. Nothing anywhere says so; the only symptom is a job that always passes.

TEST_TARGETS and resolveTestTarget(raw) now live in scripts/lib/workspaceSource.ts and both call sites route through them, so they can no longer disagree. Unset and whitespace-only mean "source" (the default a caller who set nothing is asking for). Everything else throws, naming the offending value. Deliberately no lowercasing and no prefix matching: reinterpreting a typo is the failure being fixed, not a fix for it.

Confirmed at runtime — WORKGLOW_TEST_TARGET="Dist" npx vitest run … now fails at config load with:

Error: WORKGLOW_TEST_TARGET="Dist" is not a known test target. Expected one of: source, dist (or unset for "source").

(b) The unresolved-specifier diagnostic stops guessing at dist

unresolvedWorkspaceMessage offered exactly two causes — "never built" or "stale dist" — chosen from hasBuiltEntries(owner.dir). But resolution fails from the importer's node_modules, and bunfig.toml sets linker = "isolated". An importing workspace package that does not declare the owner, or a subpath the owner's exports never published, fails while the owner's dist is completely populated. The message then confidently tells the reader to bun run build — advice that changes nothing, pointing at a directory that is fine.

The causes are now ranked:

  1. undeclared dependency under isolated linking → add the owner to the importer's package.json and re-run bun i;
  2. subpath absent from the owner's exports → missing from the manifest, not from the build output;
  3. otherwise the existing built/stale pair, now prefixed "The remaining likely cause".

Both new inputs are passed in as data, keeping the message a pure function of its inputs (the convention the function already used for distHasBuiltEntries). WorkspacePackage gains the parsed exports and a set of every declared dependency — both read from the manifest listWorkspacePackages already parses — and the plugin resolves the importer by walking up to the nearest workspace dir (importerPackageOf, deepest match, so a nested workspace is not attributed to an ancestor). importerDeclaresDependency is boolean | undefined, where undefined means "the importer is outside a workspace, so the question does not apply" rather than "no".

What the tests catch

scripts/workspaceSource.test.ts:

  • (a) undefined and """source"; each of TEST_TARGETS round-trips; "dist ", "Dist", "1" each throw with the raw value in the message. RED before — the export did not exist.
  • (b) an undeclared-dependency case whose message contains isolated and the importer package name and does not contain the stale-dist wording. Impossible to express against the old signature: the file fails to typecheck first, then fails on content.
  • (b) an undeclared-subpath case, and an "importer outside any workspace" case proving undefined is not read as false.
  • (b) the two existing built/stale assertions stay green on the fallback path.

Verification

  • bun scripts/test.ts scripts vitest — 4 files, 29 tests, all passing (the 5th is a bun:test file the runner filters).
  • npx vitest run --project test packages/test/src/test/util/ — 8 files / 41 tests, exercising the plugin end to end with the extended WorkspacePackage.
  • WORKGLOW_TEST_TARGET=dist run of the scripts project — still green.
  • npx prettier --check on all four changed files — clean.
  • Typecheck: scripts/ is not covered by any package tsconfig. Checked against an ad-hoc config extending the root one; it reports the same six pre-existing errors with and without this change (Cannot find module 'vite' from the root under isolated linking, the untyped resolveId params that follow from it, and two unrelated ones in testDiscovery.test.ts / vitest.config.ts). No new type error is introduced, but there is also no clean typecheck baseline for scripts/ to point at.

Out of scope, and worth doing next: the coverage-shrink smoke test

The third finding is not in this PR. Recording what was verified, because the gap is larger than it looks:

  • test-vitest-dist runs only the unit tier (bun run test:vitest:unit). A package entry reached solely by an integration test is therefore never loaded from dist by any blocking job. @workglow/duckdb is the clean example: its only test-side importer is packages/test/src/test/storage-tabular/DuckDbTabularStorage.integration.test.ts (the workglow meta-package barrel also re-exports it, but nothing in packages/test imports that barrel).
  • @workglow/electron, @workglow/playwright and @workglow/bun-webview are reached only from packages/test/src/test/browser/*.integration.test.ts. test:vitest:integration excludes browser by name, and grepping .github/workflows/test.yml for browser returns nothing — no CI job runs that tier at all. Those three packages have zero blocking coverage of any kind, dist or source.

Recommendation: a unit-tier smoke test that imports every workspace exports subpath. It runs inside the existing dist job, costs seconds rather than minutes, and needs no new secrets. Prefer it over a second dist job running the integration tier: that job would need WORKGLOW_SECRETS_PASSPHRASE, which is unavailable on fork PRs, so the guard would silently degrade to nothing exactly where review matters most.


Generated by Claude Code

…pecifier causes

Two guards on the dist-targeted test path.

`WORKGLOW_TEST_TARGET` had two independent readers, each treating
anything that was not literally "dist" as source, silently. A workflow
edited to "dist " or Dist turned test-vitest-dist into a byte-identical
rerun of test-vitest-unit: green, a full runner slot spent, no bundle
coverage. `resolveTestTarget` is now the single reader — unset and empty
mean source, everything else throws naming the value, with no
lowercasing and no prefix matching.

`unresolvedWorkspaceMessage` offered only "never built" or "stale dist",
but resolution fails from the IMPORTER's node_modules: under isolated
linking an undeclared workspace dependency, or a subpath absent from the
owner's exports, fails while the owner's dist is fully populated. The
causes are now ranked, so the built/stale pair is offered last and
labelled as the remaining likely cause.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RomTUtZSTgUbFCYqFs4pcu
@github-actions

Copy link
Copy Markdown

Coverage Report

Status Category Percentage Covered / Total
🔵 Lines 70.3% 29043 / 41312
🔵 Statements 69.15% 31051 / 44900
🔵 Functions 70.14% 5896 / 8405
🔵 Branches 60.23% 15871 / 26348
File CoverageNo changed files found.
Generated in workflow #3032 for commit 12043ba by the Vitest Coverage Report Action

@sroussey
sroussey merged commit 4eb47c7 into claude/coverage-dist-bundle-fix-ew0vj8 Aug 13, 2026
12 checks passed
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