test(exports): close the gaps the export guards left — resolve shim targets, compare both directions, compile the browser condition - #837
Merged
sroussey merged 5 commits intoAug 17, 2026
Conversation
`duplicateBrowserEntryViolations` exempts an identical browser/node pair whose
specifiers are all BARE, on the ground that the split happens one layer down.
That is a claim about the TARGET, and nothing resolved it — so a shim, its
`browser` block, its bundle and its declarations could all exist while
resolving browser consumers to the NODE build, with every rule in this file
silent.
That shape is in the tree today. Of the nine `packages/workglow` shims,
deepseek / ollama / openai / openrouter / xai do split, but
`@workglow/anthropic/ai`, `@workglow/google-gemini/ai`,
`@workglow/huggingface-inference/ai` and `@workglow/huggingface-transformers/ai`
expose only `{types, import}` — no `browser` condition at all. `@workglow/mlx`
has none either, so the reviewer's scenario (a new shim shipping a `browser`
condition that resolves browser consumers to the node build) is reachable now,
not hypothetical.
`inertBareShimViolations` asks the question the exemption assumes the answer
to. Its probe is deliberately NOT `"browser" in exports`: `providers/chrome-ai`
and `providers/tf-mediapipe` declare a `browser` block that names the NODE
target — honest there, since each ships one bundle — and a presence test would
call that a split. `subpathSplit` compares the declared implementations against
`nodeImportTarget` instead, so a split exists only when a browser consumer
lands somewhere a node consumer does not. ONE splitting specifier keeps the
shim; a pair naming no bare specifier belongs to the sibling rule; and an
`unknown` target is REPORTED rather than skipped, for the same reason an
underivable dist stem is — a target the rule could not resolve is where an
inert shim would hide.
The pair scan is hoisted to a module-level `entryPairs` since two tests now
read it, and its count assertion is loosened to a lower bound: the exact number
moves whenever a shim or provider entry lands, and the surviving shims are the
negative case this rule must keep passing.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HJRf3YFa8DjmjsZvXz8xDT
`@workglow/anthropic/ai`, `@workglow/google-gemini/ai`,
`@workglow/huggingface-inference/ai` and `@workglow/huggingface-transformers/ai`
expose only `{types, import}` — no `browser` condition at all — so the meta-
package's browser shims for them, their `browser` blocks, their bundles and
their declarations were four artifacts byte-identical to the node ones, and the
`types` repoint was a no-op.
Resolution-neutral, verified by hand: today a browser consumer of
`workglow/anthropic` resolves `dist/anthropic.browser.*` →
`export * from "@workglow/anthropic/ai"` → (no browser block) →
`providers/anthropic/dist/ai.*`. After removal it resolves `dist/anthropic.*` →
the same bare specifier → the same module, one hop earlier.
The four subpaths now carry the two-key shape `./tf-mediapipe` and
`./chrome-ai` already use. No build-script change is needed —
`packages/workglow/build.ts` globs `src/*.ts` — and nothing else imports the
deleted files, including `src/{common,browser,node}.ts`.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HJRf3YFa8DjmjsZvXz8xDT
…oviders `providers/llamacpp-server` and `providers/stable-diffusion-server` have no platform-specific source — `src/ai/` contains no `.browser.ts` — and their `src/ai.browser.ts` was `export * from "./ai"`. A relative specifier is resolved once by the file's own path, so the browser bundle was the same module graph and the split was nominal: two extra bundles and two extra declaration files kept equal only by construction. Both `.browser.ts` entry files, both `"browser"` blocks and the `watch-browser` / `build-browser` scripts are gone, and the two aggregators now match `providers/anthropic` verbatim. Verified guard by guard: `browserSplitViolations` reaches its empty-block branch only when `findBrowserBlock` returns something and then continues at the source-entry probe, so removing BOTH halves keeps it green; `buildEntryViolations` now sees only `./dist/ai.js` and `./dist/ai-runtime.js`, both named by `build-code`; and `ExportBarrelParity`'s pair count is unchanged, since those four were already filtered out by `isSiblingReExport`. That helper's doc named these two packages as the shape it wants; `providers/openrouter/src/ai/runtime.browser.ts` is the one live instance left, so it now names that. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HJRf3YFa8DjmjsZvXz8xDT
The check computed only `node.surface \ browser.surface` while its title claimed symmetry, so a name the BROWSER barrel exports and the node one does not was invisible — the same defect mirrored, and the one that hurts a browser consumer, who compiles against a symbol the node build cannot supply. Re-running the guard's own parser with the direction reversed found exactly one real case: `providers/ollama/src/ai/common/Ollama_JobRunFns.browser.ts` re-exporting `getClient`, `getModelName` and `loadOllamaSDK` from `./Ollama_Client.browser`. That line is DEAD, not intentional, so it is deleted rather than pinned: `runtime.browser.ts` already does `export * from "./common/Ollama_Client.browser"`, so all three names reach `@workglow/ollama/ai-runtime` regardless; `ai/index.browser.ts` never re-exports the run-fns module, so `@workglow/ollama/ai` never saw them; and nothing imports them from that path (only `OLLAMA_RUN_FNS` is imported from it). Pinning would have installed a permanent exemption for a redundant line, against this file's own stated philosophy. `INTENTIONAL_BROWSER_ONLY` therefore ships empty and documented, exactly as `ALLOWED_MISMATCHES` does, with the deleted line recorded and a warning not to add an entry to silence drift. The staleness test loops both maps. Three inline fixtures exercise the comparison directly, since no pair in the tree differs either way. The first is the go-red proof: the old one-direction check returned `[]` for exactly that input. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HJRf3YFa8DjmjsZvXz8xDT
Every browser-split guard in the repo reads TEXT: `ExportTypesPairing` walks `exports` maps, `ExportBarrelParity` diffs `export` statements. An `export *` dropped from BOTH halves of a pair keeps barrel parity, keeps the manifest self-consistent, builds clean, and surfaces only when a downstream browser app upgrades and gets `TS2305`. Nothing in CI resolved a provider subpath under `customConditions: ["browser"]` — `examples/web/tsconfig.json` sets the condition but references no affected provider. `packages/test/src/browser-conditions/browserConditionResolution.types.ts` is a type-only fixture that never runs; the assertion is that it RESOLVES and COMPILES. Positives name a symbol each split subpath must still export; the negatives are what make them mean anything — `@ts-expect-error` on the node-only `_testOnly`, so a `customConditions` that silently stopped applying turns every one into `TS2578` rather than passing against the node declarations. Verified by deleting `customConditions` from the program: seven `TS2578`. It compiles as its own program (`tsconfig.browser-conditions.json`, `noEmit`), never as part of `packages/test`'s `build-types`, which runs under node conditions where every negative control would fail the build — `packages/test/tsconfig.json` excludes the directory for that reason. `workglow` joins `packages/test`'s devDependencies because the fixture also checks the meta-package's five splitting shims, the two-hop case no other program covers. The subpath list is not hand-maintained: a guard derives the expected set from the `providers/*` and `packages/workglow` manifests via `subpathSplit` — every subpath that really splits, no exemption list — so a new provider shipping a browser split fails until the fixture names it. A second guard pins both halves of the wiring, since a script nothing calls and a workflow step naming a script that does not exist each disable the check silently. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HJRf3YFa8DjmjsZvXz8xDT
sroussey
marked this pull request as ready for review
August 17, 2026 20:08
Coverage Report
File CoverageNo changed files found. |
sroussey
merged commit Aug 17, 2026
ad3209a
into
claude/wonderful-turing-rjtcnx-ai-types
11 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 #717 (targets its head branch, the way #813 was), addressing review findings on that PR's guards. Three gaps, each of which let a real defect through, plus the four dead artifacts the first two found.
1. A bare shim's target was never resolved (
9cfee82)duplicateBrowserEntryViolationsexempts an identical browser/node entry pair whose specifiers are all BARE, on the ground that the split happens one layer down. Nothing resolved that claim — so a shim, itsbrowserblock, its bundle and its declarations could all exist while resolving browser consumers to the NODE build, with every rule in the file silent.inertBareShimViolationsasks the question. Its probe is deliberately not"browser" in exports:providers/chrome-aiandproviders/tf-mediapipedeclare abrowserblock naming the very same target as the default branch — honest, since each ships one bundle.subpathSplitcompares the declared browser implementations againstnodeImportTarget, so a split exists only when a browser consumer lands somewhere a node consumer does not. One splitting specifier keeps the shim; anunknowntarget is reported rather than skipped.2. Four
packages/workglowshims were exactly that shape (67dbdd9)Of the nine vendor shims, deepseek / ollama / openai / openrouter / xai do split.
@workglow/anthropic/ai,@workglow/google-gemini/ai,@workglow/huggingface-inference/aiand@workglow/huggingface-transformers/aiexpose only{types, import}— nobrowsercondition at all — so those four shims, theirbrowserblocks, their bundles and their declarations were artifacts byte-identical to the node ones.Resolution-neutral: today a browser consumer of
workglow/anthropicresolvesdist/anthropic.browser.js→export * from "@workglow/anthropic/ai"→ (no browser block) →providers/anthropic/dist/ai.js. After removal it resolvesdist/anthropic.js→ the same bare specifier → the same module, one hop earlier. The four subpaths now carry the two-key shape./tf-mediapipeand./chrome-aialready use.3. Two providers shipped a nominal browser build (
88e0d81)providers/llamacpp-serverandproviders/stable-diffusion-serverhave no platform-specific source, and theirsrc/ai.browser.tswasexport * from "./ai"— a relative specifier, so the browser bundle was the same module graph. Both entry files, bothbrowserblocks and thewatch-browser/build-browserscripts are gone; the two aggregators now matchproviders/anthropicverbatim. Neither package imports a node builtin, and both bundles were byte-identical to the node ones apart from the debug id.4. The barrel-parity check ran in one direction only (
6203303)It computed
node.surface \ browser.surfacewhile its title claimed symmetry, so a name the BROWSER barrel exports and the node one does not was invisible — the same defect mirrored, and the one that hurts a browser consumer, who compiles against a symbol the node build cannot supply.Re-running the guard's own parser reversed found exactly one case:
providers/ollama/src/ai/common/Ollama_JobRunFns.browser.tsre-exportinggetClient,getModelNameandloadOllamaSDKfrom./Ollama_Client.browser. That line was dead, not intentional —runtime.browser.tsalready doesexport * from "./common/Ollama_Client.browser", so all three names reach@workglow/ollama/ai-runtimeregardless;ai/index.browser.tsnever re-exports the run-fns module; and onlyOLLAMA_RUN_FNSis imported from that path. Deleted rather than pinned.INTENTIONAL_BROWSER_ONLYships empty, likeALLOWED_MISMATCHES.5. Nothing in CI ever compiled the browser condition (
a4b11c8)Every guard above reads text. An
export *dropped from BOTH halves of a pair keeps barrel parity, keeps the manifest self-consistent, builds clean, and surfaces only when a downstream browser app upgrades and getsTS2305.customConditions: ["browser"]appeared once in the repo, inexamples/web/tsconfig.json, which references no affected provider.packages/test/src/browser-conditions/browserConditionResolution.types.tsis a type-only fixture that never runs; the assertion is that it RESOLVES and COMPILES. Positives name a symbol each split subpath must still export. The negatives are what make them mean anything:@ts-expect-erroron the node-only_testOnly, so acustomConditionsthat silently stopped applying turns every one intoTS2578instead of quietly passing against the node declarations.It compiles as its own program (
tsconfig.browser-conditions.json,noEmit), never as part ofpackages/test'sbuild-types, which runs under node conditions where every negative control would fail the build —packages/test/tsconfig.jsonexcludes the directory for that reason.workglowjoinspackages/test's devDependencies (the onebun.lockline) because the fixture also covers the meta-package's five splitting shims, the two-hop case no other program checks.The subpath list is derived, not hand-maintained: a guard reads the expected set out of the
providers/*andpackages/workglowmanifests throughsubpathSplit— every subpath that really splits, no exemption list — so a new provider shipping a browser split fails until the fixture names it. A second guard pins both halves of the wiring, since a script nothing calls and a workflow step naming a script that does not exist each disable the check silently.Verified
Run in this worktree, all observed:
bun run build:types— 41/41 successful.bun run build— 84/84 successful. Clean rebuild (build-clean+build-package) ofproviders/llamacpp-server,providers/stable-diffusion-serverandpackages/workglow: successful, anddistnow holds zero browser artifacts for the two providers and noanthropic/google-gemini/hf-inference/hf-transformersbrowser artifacts for the meta-package.bun run typecheck:browser— exit 0.bun run typecheck:budget— OK, 38 packages within budget.bun run typecheck:tests— clean.bun scripts/test.ts util vitest— 55 files, 830 passed / 10 skipped.bun scripts/test.ts --check-sections— every test file reachable.bunx eslinton the changed/new test sources — clean.prettier --checkon every touched file — clean, exceptdocs/technical/19-build-system.md, which reports the same pre-existing warning on the base branch's copy (verified by checking out the base file and running prettier on it); the added paragraph itself is byte-identical to prettier's output.Go-red proofs:
customConditionsfromtsconfig.browser-conditions.json→ sevenTS2578: Unused '@ts-expect-error' directive, i.e. the negative controls are live and the program really does compile the fixture.@workglow/xai/aiimport → the derived-coverage guard fails withexpected [ '@workglow/xai/ai' ] to deeply equal [].dist/ai.browser.jsvsdist/ai.jsforproviders/llamacpp-serverdiffered only indebugId; the fourpackages/workglowshim bundles were identical to their node counterparts.Not verified: the full repo test suite (only the
utilsection plus the two typecheck programs), and no downstream browser app was built against the result — the compiled fixture is the evidence for that path.🤖 Generated with Claude Code
https://claude.ai/code/session_01HJRf3YFa8DjmjsZvXz8xDT
Generated by Claude Code