Skip to content

[finding] seed-loader-state-machine-exempt.test.ts tells a future debugger to revert a flag that PR #17718 removes from SEED_OPTIONS #17938

Description

@os-sales

Filed by the domain:cli execution seat (#6024, session session_01TSf4DV7ziu4V5j73e46b7c, R73) while landing PR #17718 under a maintainer direct dispatch. ⛔ Observation-level, unassigned, no domain:* / type — routing and grading belong to central triage. The fix lands in packages/metadata-protocol.

The trap

packages/metadata-protocol/src/seed-loader-state-machine-exempt.test.ts:21 instructs a future debugger:

Revert the seedReplay flag in SEED_OPTIONS and both cases below go red.

That is a runnable instruction for falsifying the test's own premise — the most valuable kind of test comment, and the reason this is worth a card rather than a shrug. PR #17718 makes it false: SeedLoaderService.SEED_OPTIONS stops spelling the three flags inline and becomes { context: SEED_WRITE_EXECUTION_CONTEXT }, with the value imported from @objectstack/spec/kernel. After that merge there is no seedReplay flag in SEED_OPTIONS to revert; the lever moved to packages/spec/src/kernel/execution-context.zod.ts, one package away.

⇒ A debugger following the comment reaches for a flag that is not there, and the natural next move — concluding the comment is stale and ignoring it — discards a correct falsification procedure that still works, just at a different address.

Why it is filed and not ridden into PR #17718

  • It is outside that PR's declared 8-file face, and the delivering dev is long gone.
  • Adding a file to a PR that is ready and armed costs a full CI generation for a comment.
  • ⛔ Widening a PR under review to sweep up a finding it caused is exactly the shape the fencing rules refuse.

Found by the independent fable contract review of PR #17718 (5651852441), which recorded it as "a one-line follow-up, not a rider on this PR". That judgment is adopted here.

The fix

One line: point the instruction at the constant's new home — revert SEED_WRITE_EXECUTION_CONTEXT's seedReplay in packages/spec/src/kernel/execution-context.zod.ts, or say that SEED_OPTIONS now reads it from there. ⛔ No test logic changes; ⛔ no behaviour changes.

⚠️ Ordering: this is only true once PR #17718 merges. Until then the comment is still accurate, so ⛔ do not dispatch this card against main before that PR lands — verify on the merged ref first, which is the standing rule for any card whose premise is another PR's landing.

Deduplication

The trap does not exist on main yet — it is created by an unmerged PR — so a pre-existing card describing it is not possible, and a keyword sweep would be answering a question about a state no tree is in. What was checked instead: card #17178's full thread and PR #17718's comments, for anyone already carrying this follow-up. Nobody does; the review that found it explicitly declined to file it and left it to this seat.


Generated by Claude Code

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions