fix(runtime): decide the tool call event's ledger lane at push time

[UncertaintyDeterminesYou4ndMe]

Rebased onto main (8bb7c8e1d). #2233 landed the same orphan fix in a
different shape while this was open, so the scope has changed — see
What this is now.

Summary

A pre-dispatch tool refusal used to kill the turn. The call event's lane was
guessed when the event was built: an operationId claims the T1 dispatch
protocol, so AgentRun.acceptMappedEvent skips the generic projection of that
function_call and waits for commitToolPrepared — which sits after every
refusal return. The call fact was never persisted, writeSyntheticToolResult
put the refusal on the generic lane with nothing to attach to, and the ledger
refused the orphan_response (#2234).

#2233 closed that by predicting the refusals instead: every guard hoisted into a
preflightRejected boolean read before the operationId is assigned.

What this is now

The lane is decided where it is known. pushCallEvent('preflight' | 'dispatch')
pushes at most once, every refusal routes through one refuseBeforeDispatch
helper that keeps call and result together on the generic lane, and only
prepareDurableToolAttempt asks for T1. The decision is the code path taken,
so prediction and guard cannot drift apart — which is the failure mode the
enumeration leaves open: a new refusal path, or a guard that grows a condition
its hoisted twin does not, silently restores the orphan.

Per review, #2233's preflightRejected, rejectedBeforeClientBoundary and early
slot reservation are removed; the boundary read and the reservation go back to
the guards they belong to. Everything else #2233 restored is untouched.

Two things #2233 does not cover, both falling out of the same failure:

• A rejected append still cost the run its terminal write. It latched the
  RuntimeEvent store unavailable, and commitTerminalRun returns early on an
  unavailable store — a run left at running with no terminal event.
  ToolLedgerRejectionError now marks a refusal of one bad candidate against a
  healthy store, and only that skips the latch.
• agent_swarm's selector refusal. It said only Provide exactly one of subagent_id or legacy profile. — naming neither which of the two mistakes it
  was nor a single value that would be accepted, while the field list an args
  violation appends is the top-level one, not items[n] where the violation
  actually was.

Review responses

P1 — rebase. Done, and taken as recommended: your model in the operationId
region, #2233's predicate dropped. git merge-tree is clean.

P2 — the conflated error class. You are right, and the failure you describe
is reachable: assertWorkspaceToolLedgerHealthy refuses well-formed writes
because of damage elsewhere, so "the store is healthy" — the whole premise of
skipping the latch — is false there. Split into ToolLedgerCorruptionError,
which stays on the fail-closed path.

A second adversarial round on the split found three test defects and one false
rationale, all fixed in the current head (see the comment below for the full
account):

• The corrupt-ledger test I originally cited as pinning the split was vacuous —
  it passed with latching deleted outright, because its double refused every
  append. Rewritten so the double refuses exactly what production refuses
  (tool-bearing events only); it now fails if the latch is removed.
• Neither error class was pinned at its producer, and both messages are
  byte-identical to the plain Error strings they replaced — a regression to
  throw new Error(...) would have kept every suite green while the latch
  exemption silently died. Two sqlite-runtime-store.test.ts cases now assert
  the class (and code/eventId) where the throws happen, including corruption
  detected across sessions.
• The fail-closed rationale in my comment — a corrupt ledger means "nothing this
  run emits next can be trusted to land" — is false: the health scan only runs
  for tool-bearing events, so a damaged ledger refuses tool facts and would have
  taken the terminal event. The latch is what keeps it out, which is bug(runtime): every pre-dispatch tool refusal kills the turn — the synthetic result lands as an orphan_response the ledger rejects #2234's
  shape for an already-damaged workspace. Behaviour deliberately unchanged here
  (it predates this PR, and changing it is a call about what fail-closed should
  mean); the comment now says what is actually true, the test pins the price
  explicitly, and the decision is tracked in bug(runtime): a corrupt ledger costs a run its terminal fact via the latch, not via the corruption #2313.

P2 — coverage. pre-dispatch-refusal-ledger.test.ts is now table-driven over
seven paths: schema rejection, exclusive-step admission, both loop gates
(repeated identical call, repeated ambiguous Computer Use target),
deferred-not-loaded, boundary-read failure, and client-capability non-bypass.
Each asserts the scanner is clean and that the refused call carries no dispatch
fact. Forced onto the tagged lane, all seven produce orphan_response. Still
uncovered and called out in the file: the subagent cap, which needs five
settlements held open at once and exercises the slot-release path rather than
the lane.

P3. "Both set" now has its own case, asserting it does not read as the
"neither set" sentence. And you are right about the headline claim — on current
main #2233 already fixed the symptom, so these tests guard the refactor rather
than reproduce a live bug. The PR body no longer says otherwise.

Verification

• @maka/core 788 pass, @maka/runtime 3171 pass / 0 fail, @maka/storage 694
  pass with two unrelated failures pre-existing on this machine
  (package-import.test.js asserts an empty stderr and Node 22.17 prints the
  node:sqlite ExperimentalWarning; root-authority-dependency.test.js fails
  identically with this branch's changes stashed).
• biome clean; tsc clean across core / storage / runtime.
• Each fix was re-confirmed load-bearing by reverting it: the seven refusal rows
  go orphan, the terminal-write case loses its terminal event, deleting the
  latch fails the corrupt-ledger test, and regressing either error class to a
  plain Error with the identical message fails its storage test.
• Not run: the desktop e2e suites.

Review focus

The call event's queue.push now happens later than either previous shape — at
the refusal, or at prepareDurableToolAttempt. The only await between
appendMessage(callMsg) and the latest possible push is readExecutionBoundary()
on client_capability tools; nothing in that window emits an event referencing
the toolUseId or reads the ledger back. The tool context's operationId now
reads pushedCallEvent?.operationId — the id the emitted fact actually carries.

Adjacent, deliberately not in scope: subagent-tools.ts carries the identical
child-selector superRefine with the old thin message, so agent_spawn gives a
model the same dead end. One call, no items[n] path — a follow-up.

[Astro-Han]

Thanks for the fix — I traced the causal chain against the source and it all checks out: the one-shot lane guess, the commitToolPrepared placement after every pre-dispatch refusal, the orphan response the ledger rejects, and the latch swallowing the terminal write. The push-time lane model is the right architecture — lane decided where it's known, refusal pair kept together on the generic lane, T1 asked only by the durable path. That's structurally stronger than a construction-time enumeration, and the 1,300+ tests are green. The merge state, though, needs a decision:

P1 — the branch cannot merge: it conflicts with #2233, which already merged the same fix in a different shape.
git merge-tree main pr-2240 conflicts in agent-run.ts and tool-runtime.ts. #2233 ("restore desktop Computer Use capability", merged 8/5) added a construction-time preflightRejected enumeration that predicts all eight refusals — the same orphan fix, but without your push-time machinery, the error-class refactor, or the latch fix. On current main the schema-rejection call is already untagged, which is why your ledger test passes there.

On rebase, my recommendation is to keep your model in the operationId region and drop #2233's predicate (~40 lines: preflightRejected, rejectedBeforeClientBoundary, the early slot reservation) — the enumeration is exactly the "guess at construction time" your PR criticizes: any future refusal path or drift between predicate and actual check silently re-introduces the orphan. But keep everything else #2233 carries (client-capability boundary reads, slot reservation/release, the Computer Use restore) — only the operationId condition should move to your push-time form.

P2 — ToolLedgerRejectionError now conflates two states your latch semantics treat differently. A rejected append means "store healthy, event malformed" (your comment's rationale), but assertWorkspaceToolLedgerHealthy throws the same class for existing ledger corruption (sqlite-runtime-store.ts:2165). With the new latch rule keyed on that type, a corrupt ledger skips latching, commitTerminalRun can fail before the header commit, and the run row ends non-terminal again — the exact shape you're eliminating, via a path no test covers. A separate error type (or an explicit comment at the throw site) would close it.

P2 — the ledger tests pin 1 of the 8 refusal paths. Only schema-rejection is covered end to end (plus the exclusive-step case already on main). The two loop gates, deferred-not-loaded, and boundary-read-failure have no ledger-level test on any branch — and those four are exactly the ones that were broken pre-fix (tagged T1 → orphan). A table-driven variant of pre-dispatch-refusal-ledger.test.ts would cover them cheaply; the harness already supports it (readExecutionBoundary override etc.).

P3 (optional): the refusal-text tests only assert the "neither set" wording — "both set" is untested; and the PR body's "reproduces the production issue byte for byte" is only true against your merge-base — on current main #2233 already fixed the symptom, so the headline test guards the refactor (still valuable, worth a sentence in the body).

None of this questions the fix itself — design and implementation both check out. Happy to re-review once it's rebased.

[UncertaintyDeterminesYou4ndMe]

Thanks — all four land. Rebased onto 8bb7c8e1d; git merge-tree is clean.

P1. Taken as recommended: push-time model in the operationId region,
#2233's preflightRejected / rejectedBeforeClientBoundary / early slot
reservation dropped, boundary read and reservation returned to the guards they
belong to, everything else #2233 restored left alone. One thing worth flagging
since it is not a pure revert: tool.impl's context takes operationId, which
no longer exists as a scope variable — it now reads pushedCallEvent?.operationId,
the id the emitted fact actually carries rather than the candidate.

P2, the error class. You are right and I had it wrong in the comment itself:
I wrote "the store is healthy" as the rationale for skipping the latch, which is
simply false at sqlite-runtime-store.ts:2165 — that site refuses well-formed
writes because of damage elsewhere. Split into ToolLedgerCorruptionError, which
is not exempt and keeps failing closed, with a test that pins the difference:
corrupt ledger → latched → terminal write refused outright, rather than the
silent skip that produced this bug.

Worth noting my own adversarial review pass raised this same site and talked
itself out of it — on the grounds that the canonical store rethrows, so the run
ends failed rather than completed. That argument is true and beside the
point: it answers "what status does the header get" when the question was
"does the terminal fact land". Your framing caught what mine missed.

P2, coverage. Now table-driven over seven paths — schema rejection,
exclusive-step admission, both loop gates, deferred-not-loaded, boundary-read
failure, client-capability non-bypass. Forced onto the tagged lane, all seven
produce orphan_response, so the table is load-bearing rather than decorative.
The subagent cap is still uncovered — it needs five settlements held open
concurrently and exercises the slot-release path more than the lane — and the
file says so rather than leaving the gap implicit.

P3. "Both set" has its own case now, asserting it does not read as the
"neither set" sentence. And you are right about the headline claim: on current
main #2233 already fixed the symptom, so these tests guard the refactor rather
than reproduce a live bug. The PR body says that now instead of the old claim.

Ready for another look.

[UncertaintyDeterminesYou4ndMe]

Force-pushed ab7a8f7f4 → 31f67c2ed. Same change, four fixes from a second adversarial review round I ran after verifying the branch against a packaged build. One of them corrects something I told you in the last round, so it goes first.

The corrupt-ledger test I cited to you was vacuous

In my P2 reply I said the split came "with a test that pins it". That test pinned nothing. I proved it this round by deleting the latch outright (agent-run.ts, if (!(error instanceof ToolLedgerRejectionError)) → if (false)): the test still passed. It also passed with the exemption removed (if (true)). It could not see the latch at all.

The mechanism: the TinyAgentRunStore double threw ToolLedgerCorruptionError on every append, so the second recordRuntimeEvents rejected identically whether or not the store was latched — and the second assert.rejects had no validator anyway.

The fix is not more assertions on the same double; the double itself over-modelled the store. Production gates the workspace health scan behind isToolLedgerBearingEvent, so a corrupt ledger refuses tool facts only. The double now does the same, which makes the terminal event a write the "corrupt" store would accept — and "the terminal event did not land" becomes the one assertion that separates latch from no-latch. Revert-proven: with latching deleted, the rewritten test fails.

Which exposed a false rationale

My comment justified corruption staying fail-closed with "nothing this run emits next can be trusted to land." That is false, per the same gate: a run's terminal RuntimeEvent bears no tool fact, so a damaged ledger would have taken it. The latch, not the corruption, is what costs the run its terminal fact — #2234's shape, standing for any workspace with pre-existing ledger damage (the health scan has no WHERE; one damaged operation anywhere in the workspace file triggers it for every run in it).

I have not changed the behaviour. It predates this PR — before the split, every append failure latched — and whether a run that cannot write tool facts should still get to say it ended is a real decision, not a drive-by. The comment now states the true trade-off, the test pins the price explicitly (it asserts the terminal event does not land, so deciding #2313 either way must change it), and #2313 has the write-up.

The split had no test at its producer

Both new classes were only ever thrown by test doubles in the runtime suite. Nothing asserted SqliteRuntimeStore itself throws them — and both messages are byte-identical to the plain Error strings they replaced, so throw new Error(...) regressed cleanly against every message-matching assertion while the latch exemption silently died. Two cases in sqlite-runtime-store.test.ts now pin class, code, and eventId at the throw sites — the corruption one seeds damage in a different session via raw SQL to also pin the workspace-wide reach. Revert-proven: regressing either throw to a plain Error with the identical message fails its test.

Small

The client-capability blocked by the execution boundary row asserted expect: /./, which matches anything. Now /require the Bypass execution boundary/.

---

Verification on 31f67c2ed: runtime 3171 pass / 0 fail, storage 694 pass (two failures pre-existing on my machine — the Node 22.17 node:sqlite ExperimentalWarning vs empty-stderr assertion, and root-authority-dependency.test.js, which fails identically with this branch stashed), core 788 pass, biome + tsc clean. Every fix above was confirmed load-bearing by reverting it and watching the specific test fail.

Also filed from the packaged-build verification, all adjacent to but outside this PR: #2310 (a transition refusal that names no exit left a genuinely-completed task pending forever — same species as the agent_swarm message, one layer up), #2311 (stop-button cancel stamps traceWriteError via the sealed-run refusal), #2313 (above).

[Astro-Han]

Thanks for the rewrite — this is exactly the shape we hoped for, and I verified it end to end. The push-time lane model is correct by construction: at-most-once push via the closure flag, all 8 refusal paths route through refuseBeforeDispatch (I enumerated every pre-dispatch exit in executeTool), T1 is claimed only by prepareDurableToolAttempt, and #2233's predicate, hoisted boundary read, and early slot reservation are fully gone (grep-confirmed) — with the boundary read and reservation back in their guards in a strictly safer order (an appendMessage failure can no longer leak a reserved slot). The error classification is precise: the latch exemption is scoped exactly to ToolLedgerRejectionError (CorruptionError extends Error directly, so it keeps failing closed), and both directions are pinned — the rejection test fails if the exemption regresses, and the corruption test's final assertion is explicitly the one that fails if the latch goes away. The 7-path suite genuinely reproduces the production orphan_response on the pre-#2233 bug code (verified empirically), the swarm message fixes are real (both new tests fail on the parent), the canonical-store usage is behaviorally significant, and the full runtime/core suites are green. The deliberate corruption-latch behavior is honestly tracked in #2313 with a pinning test — good call.

Four optional notes, none blocking:

• The PR body's "byte-identical messages" claim holds for ToolLedgerRejectionError but not ToolLedgerCorruptionError — that message actually changed from Tool ledger transition rejected: … to Tool ledger is corrupt: … (tool-ledger-scanner.ts:60-64). No consumer matches the old string (the pins carry the weight), but the claim overstates.
• The 7-path suite passes on the pre-fix parent too (9/9) — fix(runtime-host): restore desktop Computer Use capability #2233's build-time predicate enumerated the paths correctly, so the suite is drift-protection over enumerated outcomes, not a pin on the push-time mechanism itself. Worth one sentence in the PR body so the claim matches what the tests can enforce. And while the harness is fresh, the 8th path (subagent cap, tool-runtime.ts:1242) is the only refusal without a lane assertion — the delegated swarm test asserts only refusal text — so a cap regression tagging T1 would ship green. One more row in REFUSAL_PATHS would close it.
• pushCallEvent's at-most-once guard is unreachable today (every refusal site early-returns before the dispatch push) and unpinned — either accept it as defensive or drop the claim.
• The conversation-copy corruption gate (sqlite-runtime-store.ts:419-422) still throws a plain Error; harmless at import time, but worth throwing ToolLedgerCorruptionError for consistency if that path ever routes through the runtime latch.

Merging now — this closes #2234 properly.