fix(scripts): validate WORKGLOW_TEST_TARGET and rank the unresolved-specifier causes - #759
Merged
sroussey merged 1 commit intoAug 13, 2026
Conversation
…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
Coverage Report
File CoverageNo changed files found. |
sroussey
merged commit Aug 13, 2026
4eb47c7
into
claude/coverage-dist-bundle-fix-ew0vj8
12 checks passed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #741 (
claude/coverage-dist-bundle-fix-ew0vj8). Two independent fixes to the machinery that PR introduced.(a)
WORKGLOW_TEST_TARGETis validated, not guessedvitest.config.ts:72andscripts/test.ts:122each 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) orDistturnstest-vitest-distinto a byte-identical rerun oftest-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_TARGETSandresolveTestTarget(raw)now live inscripts/lib/workspaceSource.tsand 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:(b) The unresolved-specifier diagnostic stops guessing at
distunresolvedWorkspaceMessageoffered exactly two causes — "never built" or "stale dist" — chosen fromhasBuiltEntries(owner.dir). But resolution fails from the importer'snode_modules, andbunfig.tomlsetslinker = "isolated". An importing workspace package that does not declare the owner, or a subpath the owner'sexportsnever published, fails while the owner'sdistis completely populated. The message then confidently tells the reader tobun run build— advice that changes nothing, pointing at a directory that is fine.The causes are now ranked:
package.jsonand re-runbun i;exports→ missing from the manifest, not from the build output;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).WorkspacePackagegains the parsedexportsand a set of every declared dependency — both read from the manifestlistWorkspacePackagesalready 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).importerDeclaresDependencyisboolean | undefined, whereundefinedmeans "the importer is outside a workspace, so the question does not apply" rather than "no".What the tests catch
scripts/workspaceSource.test.ts:undefinedand""→"source"; each ofTEST_TARGETSround-trips;"dist ","Dist","1"each throw with the raw value in the message. RED before — the export did not exist.isolatedand 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.undefinedis not read asfalse.Verification
bun scripts/test.ts scripts vitest— 4 files, 29 tests, all passing (the 5th is abun:testfile 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 extendedWorkspacePackage.WORKGLOW_TEST_TARGET=distrun of the scripts project — still green.npx prettier --checkon all four changed files — clean.scripts/is not covered by any packagetsconfig. 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 untypedresolveIdparams that follow from it, and two unrelated ones intestDiscovery.test.ts/vitest.config.ts). No new type error is introduced, but there is also no clean typecheck baseline forscripts/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-distruns only the unit tier (bun run test:vitest:unit). A package entry reached solely by an integration test is therefore never loaded fromdistby any blocking job.@workglow/duckdbis the clean example: its only test-side importer ispackages/test/src/test/storage-tabular/DuckDbTabularStorage.integration.test.ts(theworkglowmeta-package barrel also re-exports it, but nothing inpackages/testimports that barrel).@workglow/electron,@workglow/playwrightand@workglow/bun-webvieware reached only frompackages/test/src/test/browser/*.integration.test.ts.test:vitest:integrationexcludesbrowserby name, and grepping.github/workflows/test.ymlforbrowserreturns 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
exportssubpath. 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 needWORKGLOW_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