Skip to content

fix(runtime-host): keep folded steering entries decodable in the followup queue - #3532

Merged
Astro-Han merged 1 commit into
apache:mainfrom
shaokeyibb:fix/host-poison-folded-steering-placement
Aug 23, 2026
Merged

fix(runtime-host): keep folded steering entries decodable in the followup queue#3532
Astro-Han merged 1 commit into
apache:mainfrom
shaokeyibb:fix/host-poison-folded-steering-placement

Conversation

@shaokeyibb

Copy link
Copy Markdown
Contributor

Summary

A steering message the run never pulls is folded into followup at the terminal transition, but the fold only resets state — the entry keeps placement: 'current_turn'. The followup queue projection then fails decodeFollowupMessages, which requires next_turn. That throw surfaces inside createSessionContinuitySnapshot, so completeTerminalTransition fails before commitNextRoot, drainTurn's finally falls through to abandonRootReservation, the still-live entry trips #failStop(), and the Host drains mid-turn.

The fix projects followup entries by queue position: an entry in the followup queue is a next-turn message, whatever it was submitted as. Where the message was originally aimed already lives on disposition and on the durable sourceFromEntry record — which the existing fold test pins, and which this change deliberately leaves alone.

queuedSnapshot itself is unchanged because retractedSnapshot also builds on it, and a retracted steering entry legitimately carries current_turn.

Fixes #3530

Verification

New unit test a lone folded steering entry leaves the queue projection decodable folds a lone unpulled steer and round-trips the projection through decodeSessionMessageQueueProjection. Without the fix it fails on expected: 'next_turn' / actual: 'current_turn'.

End-to-end, against a real Host with the fake backend's two steering drains temporarily disabled (they are what hides this today — see the "Final stranded drain" comment in fake-backend.ts), polling queryTurn every 500 ms:

before  t=500ms  running
before  t=1000ms RuntimeHostOperationError: Runtime Host is draining

after   t=500ms  running
after   t=1000ms completed

after matches a no-steer control exactly. That scaffolding is not part of this PR; #3529 covers why the steer is never pulled in the first place.

Ran: message-coordinator (31), session-projector (5), execution-host-message (4) — all green; biome check on both changed files; npm run check:asf-headers; tsc -p packages/runtime-host.

canonical-session-projection fails 7/7 here, all EBUSY: resource busy or locked, unlink ...runtime.sqlite on teardown. Verified pre-existing: identical 7/7 against a clean origin/main build of the same file. This is a Windows-only SQLite unlink problem in the fixture, unrelated to the change.

Not run: the rest of the runtime-host suite, Desktop, and Playwright — this change is confined to the Host queue projection.

Root cause

The asymmetry that allowed it: queuedSteeringSnapshot already asserts its own placement invariant (Steering entry lost current-turn placement), and inFlightSnapshot does the same. The followup side had no guard, so a wrong placement reached the wire instead of failing loudly next to the code that produced it.

AI use

Select exactly one:

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

Tool(s) and scope: Claude Code — investigation, root-cause tracing, the test, and the fix.

Checklist

  • Tests cover the change and fail without it
  • Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

  • Yes — described under Summary above
  • No

…owup queue

A steer the run never pulls is folded into `followup` at the terminal
transition but kept its `current_turn` placement, so the queue projection
failed its own wire decode. That propagated through the session continuity
snapshot and fail-stopped the Host mid-turn: the answer stopped, no successor
Turn started, and the message was lost.

Project followup entries by queue position. Where the message was aimed stays
on `disposition` and on the durable source record, which the existing fold
test pins.

Generated-by: Claude Code
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings August 22, 2026 19:39

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes a Runtime Host failure mode where an unpulled steering message gets folded into the followup queue at terminal transition while retaining placement: 'current_turn', causing the followup queue projection to fail protocol decode and cascading into Host drain (#3530). The change makes the followup queue projection reflect queue position (always next_turn) while preserving durable provenance (sourceFromEntry / batch.sources) about original placement.

Changes:

  • Project followup queue entries with placement: 'next_turn' via a dedicated queuedFollowupSnapshot, instead of copying the live entry placement.
  • Add a unit test that reproduces the lone-folded-steering scenario and asserts the projection round-trips through decodeSessionMessageQueueProjection while preserving durable source placement/disposition.

Required Conclusion (code-review)

  1. Is the current solution optimal for the actual problem? Yes — it fixes the defect at the projection boundary that must satisfy the existing wire decode contract, without mutating durable provenance.
  2. What production code can be deleted? none identified.
  3. What low-quality tests can be deleted or replaced? none identified.
  4. Is a deeper refactor required, and what should the final structure be? No deeper refactor required; the added queuedFollowupSnapshot keeps the intent explicit and localized alongside queuedSteeringSnapshot.
  5. Is the reviewed revision ready to merge? Yes, based on this diff’s scope and the added failing-then-passing regression test; final merge decision still requires independent human review.
  6. What residual risks or verification gaps remain? No protocol changes are introduced, but broader runtime-host suite coverage wasn’t rerun per PR notes; if there are additional consumers that implicitly relied on followup entries echoing original placement (despite the decoder contract), they should be validated in CI/human review.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
packages/runtime-host/src/server/message-coordinator.ts Ensures followup queue projection always reports placement: 'next_turn' to remain decodable even for folded steering entries.
packages/runtime-host/src/tests/message-coordinator.test.ts Adds a regression test that folds an unpulled steer and round-trips the queue projection through the protocol decoder while preserving durable provenance.

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed at head bcdbc99c with test terminal SUCCESS. No P0–P3 from this line.

The framing in the new comment is what makes this the right fix rather than a patch: placement on a queue snapshot is queue position, not origin. A steer the run never pulled is, once folded ahead of the followups at the terminal transition, a next-turn message by definition — reporting it as current_turn was the projection describing itself in terms the wire decoder rejects, and an undecodable projection takes the Host down through the continuity snapshot. Fixing the reporter rather than relaxing the decoder keeps the invariant where it belongs.

Two things I checked because they are where this class of fix usually goes wrong:

  • Provenance is not lost. disposition still says steering and the durable sourceFromEntry record still says current_turn, so the fold stays auditable. The new test asserts both sides, which is the part that makes the change safe to keep.
  • No consumer reads followup[].placement as origin. The only other place that branches on placement is the disposition derivation at message-coordinator.ts:629, which reads the input placement, not the projected snapshot. So forcing next_turn on the followup projection has no second reader to surprise.

Scope is one function and its call site, with no new state. Nothing else to raise.

* Queue position, not origin: an entry in the followup queue is a next-turn
* message by definition, including a steering entry the run never pulled and
* the terminal transition folded ahead of the followups. Where the message was
* originally aimed stays on `disposition` and on the durable

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P3] The doc comment promises a field the returned type does not have.

Where the message was originally aimed stays on disposition and on the durable {@link sourceFromEntry} record.

The durable half is true and the test pins it (batch.sources[0].disposition === 'steering'). The disposition half is not true of this snapshot:

function queuedSnapshot(entry: LiveEntry): QueuedMessageSnapshot {
  return { entryId, messageId, content, placement, state: 'queued' };  // no disposition
}

QueuedMessageSnapshot extends MessageQueueEntrySnapshotBase, which is entryId / messageId / content / placement — there is no disposition on it anywhere in protocol/message.ts. So inside the live projection, once the fold happens, a steer the run never pulled becomes indistinguishable from an ordinary followup. Origin survives only in the durable source record, which the projection consumer does not see.

Why I am still calling it P3 and not higher: nothing today reads that distinction off the projection, and it cannot have — the wire decoder rejects current_turn in followup, so any consumer depending on it would already be crashing. The behavior is correct; the comment describes a stronger guarantee than the code provides.

Why it is worth fixing anyway: this comment is doing real work — it is the thing that stops the next person from "fixing" placement back to current_turn and re-breaking #3530. A comment carrying that much load should not have a false clause in it, because the moment someone checks the clause and finds it wrong, they discount the whole comment. Dropping "on disposition and" is enough.


On the change itself — I went in expecting a problem and did not find one. The title pairs folded with decodable, and folding is usually lossy while decodability usually wants losslessness, so I checked whether this trades away information to satisfy the decoder. It does not: the projection field being corrected is queue position, which genuinely is next_turn for anything sitting in the followup queue, and the origin is kept where it is actually durable. Naming the snapshot function separately instead of branching inside queuedSnapshot also makes the asymmetry with queuedSteeringSnapshot visible at the call site.

Reviewed at 2026-08-23 12:24 UTC.

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving bcdbc99c5ae57a93dcc0ff0e5a275a5b1289342b. Required test is completed / success bound to that exact SHA (run 32594347705).

One [P3], inline, non-blocking: #3532 (comment) — the new doc comment says the original aim "stays on disposition", but QueuedMessageSnapshot has no disposition field; only the durable source record keeps it. The code is right, the comment claims more than the code delivers. Worth a one-clause edit whenever you next touch the file.

The fix is correct and the reasoning in the comment is the valuable part. Queue position and message origin are two different facts, and the bug was reporting one as the other. An entry sitting in followup is a next-turn message regardless of where it was originally aimed, and the provenance that makes the fold auditable lives in the durable source record — which the test pins explicitly rather than leaving implied.

The regression test earns its place: it drives a real lone-steer fold through beginTerminalTransition and then asserts decodeSessionMessageQueueProjection on the result, so it fails on the actual wire contract rather than on a hand-written expectation. Given that an undecodable projection takes the Host down through the session continuity snapshot (#3530), pinning the decode is the assertion that matters.

Reviewed at 2026-08-23 12:25 UTC. No P0–P2.

@Astro-Han

Copy link
Copy Markdown
Contributor

LGTM with non-blocking P3.

@Astro-Han
Astro-Han merged commit 3f94fe8 into apache:main Aug 23, 2026
1 check 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

3 participants