Skip to content

fix(bootstrap): pin the registered set, correct the overwrite claim, regenerate bun.lock - #780

Merged
sroussey merged 3 commits into
claude/libs-issues-triage-prs-mh6x2o-574from
claude/optimistic-goldberg-hxoj5s-bootstrap
Aug 14, 2026
Merged

fix(bootstrap): pin the registered set, correct the overwrite claim, regenerate bun.lock#780
sroussey merged 3 commits into
claude/libs-issues-triage-prs-mh6x2o-574from
claude/optimistic-goldberg-hxoj5s-bootstrap

Conversation

@sroussey

Copy link
Copy Markdown
Collaborator

Three MEDIUM defects from review of the @workglow/bootstrap extraction PR. Commit order B1 → B2 → B3, lockfile last.

Targets claude/libs-issues-triage-prs-mh6x2o-574, not main.


1. Nothing pinned the registered set, and self-registration masks a dropped line

The extraction preserved all 14 calls, but no test asserted the resulting set — and on this code base the obvious test cannot fail. Every register*Defaults module also self-registers on the global registry at module scope (registerLoggerDefaults(); at the bottom of LoggerRegistry.ts, and thirteen siblings), and registerAllDefaults.ts imports all of them. Delete registerTabularStorageDefaults(registry); from the body and bootstrapWorkglow() still yields a fully populated global registry by import side effect; the whole suite passes.

The damage lands on createOrchestrationContext(), whose registry gets nothing it was not explicitly handed: a task run with { registry: ctx.registry } and a format: "storage:tabular" input then receives the raw id string instead of the repository. No error, wrong value.

So the fixtures are asserted against a bare new ServiceRegistry(new Container()) — the only place the assertion means anything — and the anti-vacuity case comes first: a bare registry has none of the 15 tokens before the call, so nothing below can pass on an already-populated one. Exact fixtures for the tokens, the 7 resolver prefixes and the 6 compactor prefixes, failing in both directions. (The sets were enumerated from the source rather than taken from the review: image registers a resolver but no compactor, which is why the two lists differ by one.)

Ordering gets a source-text guard, because the runtime cannot see it: getInputResolvers self-heals — handed a registry with no INPUT_RESOLVERS map it registers one and carries on — so moving a primitive container after its nine consumers produces an identical registry and a green suite, until a pre-registered map is silently replaced by the healed one.

Both directions verified by hand:

mutation this file the three existing bootstrap tests
delete registerTabularStorageDefaults(registry); 5 of 6 fail all pass
move registerInputResolverDefaults to the end only the ordering case fails all pass

2. The README promised earlier registrations survive; for the resolver half they do not

Both the README and the registerAllDefaults JSDoc said defaults register with registerIfAbsent, so an earlier explicit registration "is never overwritten". That holds for the registry.registerIfAbsent(TOKEN, ...) half and not for the other: registerInputResolver / registerInputCompactor are an unconditional resolvers.set(formatPrefix, fn) — last writer wins — and nine of the fourteen calls register one of each.

The half-truth is worse than a plain error would have been, because the example the README picked to illustrate it — a custom storage backend, a factory token — is exactly the case that does survive. A consumer following it registers registerInputResolver("model", myCatalogResolver) during plugin init, calls bootstrapWorkglow() at startup, and every format: "model" input silently resolves from the built-in MODEL_REPOSITORY instead of their catalog. No warning, nothing in the registry to inspect.

Both documents now state the split and the rule it implies — custom resolvers go in AFTER bootstrapWorkglow() — and the README shows the wrong order and the right one side by side.

The executable half, four cases: a pre-registered factory token survives; a pre-registered resolver is overwritten; a pre-registered compactor is overwritten; the documented order keeps the custom resolver without disturbing its neighbours.


3. packages/bootstrap was the only workspace whose lockfile version was not updated

bun.lock recorded 0.3.38 for that workspace while the other 41 read 0.3.39, and packages/bootstrap/CHANGELOG.md was still headed ## 0.3.38. bun install --frozen-lockfile — any consumer CI, any reproducible install — fails outright on that; a plain bun i succeeds and rewrites bun.lock, so every developer gets an unexplained dirty lockfile on first install and repo CI (bare bun i) never reports it.

bun install touched exactly the one line, which is the whole lockfile diff here — no hand-editing, and no extra churn to report. bun install --frozen-lockfile now exits 0.

WorkspaceVersions.test.ts gains the cross-check that would have caught it: it read manifests only, so the straggler it was added for slipped past. It now parses bun.lock (JSONC — trailing commas stripped) and compares the version recorded per workspace against that workspace's manifest, naming any entry that disagrees. It asserts more than 40 entries first, so a degraded parse or a moved workspaces key fails loudly instead of comparing an empty set and passing vacuously. Verified red against the stale lockfile — it printed packages/bootstrap/package.json: bun.lock 0.3.38 vs manifest 0.3.39.


Verification

  • bun scripts/test.ts util vitest — 57 files, 778 passed / 10 skipped.
  • bun install --frozen-lockfile — exits 0 (Checked 1022 installs across 1098 packages (no changes)).
  • npx tsc -b packages/test — clean.
  • Each guard confirmed red against the defect it closes, and green after.

The LOW finding about the stale vitest.setup.ts comment was outside this task's scope and is untouched.


🤖 Generated with Claude Code


Generated by Claude Code

Extracting `registerAllDefaults` preserved all 14 calls, but nothing asserted
the resulting set — and on this code base the obvious test cannot fail. Every
`register*Defaults` module ALSO self-registers on the GLOBAL registry at module
scope (`registerLoggerDefaults();` at the bottom of `LoggerRegistry.ts`, and
thirteen siblings), and `registerAllDefaults.ts` imports all of them. Delete
`registerTabularStorageDefaults(registry);` from the body and
`bootstrapWorkglow()` still yields a fully populated global registry by import
side effect; the whole suite passes.

The damage lands on `createOrchestrationContext()`, whose registry gets nothing
it was not explicitly handed: a task run with `{ registry: ctx.registry }` and a
`format: "storage:tabular"` input then receives the raw id string instead of the
repository. No error, wrong value.

So the fixtures are asserted against a bare `new ServiceRegistry(new
Container())`, the only place the assertion means anything, and the
anti-vacuity case comes first — a bare registry has none of the 15 tokens
before the call, so the rest cannot pass on an already-populated one. Exact
fixtures for the tokens, the 7 resolver prefixes and the 6 compactor prefixes
(`image` resolves a data URI it cannot round-trip, so it registers no
compactor), failing in both directions.

Ordering gets a source-text guard instead, because the runtime cannot see it:
`getInputResolvers` SELF-HEALS — handed a registry with no `INPUT_RESOLVERS`
map it registers one and carries on — so moving a primitive container after its
nine consumers produces an identical registry and a green suite, until a
pre-registered map is silently replaced by the healed one.

Verified both directions by hand: deleting the tabular call fails 5 of these 6
cases and none of the three existing bootstrap tests; moving
`registerInputResolverDefaults` to the end fails only the ordering case.
…okens

The README and the `registerAllDefaults` JSDoc both said defaults register with
`registerIfAbsent`, so an earlier explicit registration "is never overwritten".
That holds for the `registry.registerIfAbsent(TOKEN, ...)` half and not for the
other one: `registerInputResolver` / `registerInputCompactor` are an
unconditional `resolvers.set(formatPrefix, fn)` — last writer wins — and nine of
the fourteen calls register one of each.

The half-truth is worse than a plain error would have been, because the example
the README picked to illustrate it (a custom storage backend, a factory token)
is exactly the case that DOES survive. A consumer following it registers
`registerInputResolver("model", myCatalogResolver)` during plugin init, calls
`bootstrapWorkglow()` at startup, and every `format: "model"` input silently
resolves from the built-in MODEL_REPOSITORY instead of their catalog — no
warning, and nothing in the registry to inspect.

Both documents now state the split and the rule it implies: custom resolvers go
in AFTER `bootstrapWorkglow()` / `registerAllDefaults()`. The README shows the
wrong order and the right one side by side.

Four cases make it executable rather than prose: a pre-registered factory token
survives, a pre-registered resolver and compactor are both overwritten, and the
documented order keeps the custom resolver without disturbing its neighbours.
…3.39

The version bump reached `packages/bootstrap/package.json` but not the
lockfile: `bun.lock` still recorded `0.3.38` for that workspace while the other
41 read `0.3.39`, and `packages/bootstrap/CHANGELOG.md` was still headed
`## 0.3.38`.

`bun install --frozen-lockfile` — any consumer CI, any reproducible install —
fails outright on that. A plain `bun i` succeeds and rewrites `bun.lock`, so
every developer instead gets an unexplained dirty lockfile on first install,
and repo CI (which runs bare `bun i`) never reports it.

`bun install` touched exactly the one line, which is the whole diff here.
`bun install --frozen-lockfile` now exits 0.

`WorkspaceVersions.test.ts` gains the cross-check that would have caught it: it
read manifests only, so the straggler it was added for slipped past. It now
parses `bun.lock` (JSONC — trailing commas stripped) and compares the version
recorded per workspace against that workspace's manifest, naming any entry that
disagrees. It asserts more than 40 entries first, so a degraded parse or a
moved `workspaces` key fails loudly instead of comparing an empty set and
passing vacuously.
@sroussey
sroussey merged commit b9e5469 into claude/libs-issues-triage-prs-mh6x2o-574 Aug 14, 2026
10 of 11 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.

1 participant