`dispatchClaudeTurn`'s error path shifted `dispatchWaiters[0]`, which is only
this call's waiter when nothing else is in flight. With two sends queued, a
broken pipe on the SECOND resolved the FIRST with null - reporting a delivered
message as "delivery unconfirmed" while the send that actually failed stayed
queued for its full ack budget, and the first send's replay then settled a
waiter nobody awaited.
Same mis-adoption class this PR is hardening: settle a dispatch only on
evidence that belongs to it. `waitForReplay` now hands its waiter back so the
error path can splice that one, mirroring what the timeout path already did.
Also raises the tests' ack budget to 5s. vi.waitFor's first successful poll
lands ~50ms after the waiter is armed, so a 100ms budget left almost nothing
for the frame and could expire under parallel load.
Found by the review loop on #15657.
The frame carried `isReplay: true`, so the gate short-circuited and
`claudeUserFrameText` never ran - the content branch the test exists for was
uncovered. Verified by mutation: making claudeUserFrameText return null when no
part carries text (exactly the regression the comment warns about) left the
test green.
Drop the marker so the frame takes the fallback, and give this one case a 5s ack
budget: it does real file I/O before the timer starts, so 100ms could expire
under parallel load before the frame arrives.
The same mutation now fails the test.
Found by the review loop on #15657.
The shape fallback ran on every frame without `isReplay`, so an injected turn
whose leading text is not on the hand-maintained allowlist in
harness-injected-user-turns.ts would still steal the send identity - and an
injected turn upserts a body, so it overwrites the user's own bubble before the
real replay appends a second one.
The launch args always request replays, so once this session has seen the CLI
stamp `isReplay`, a frame without it cannot be the echo. Latch that and switch
the fallback off; it now only ever runs on a CLI that never stamps the marker,
where the alternative is every send timing out into a false
"delivery unconfirmed" and being sent twice on retry.
Also corrects the string-content comment: `readClaudeMessageEnvelope` tolerates
a plain-string `content` but builds an EMPTY envelope from it, so such an echo
settles the dispatch without upserting a body - the same as an image-only echo,
not the reconciliation the old wording implied.
Found by the review loop on #15657.
"Not a tool result" was too weak. Claude publishes harness-injected turns -
interruption notices, system reminders, local-command output - in the same
top-level `user` shape with a null parent_tool_use_id, and those are the WORSE
half of the class: unlike a tool result they do build a journal body, so
adopting one upserts harness text over the user's own bubble and still leaves
the real replay to append as a second row.
`--replay-user-messages` stamps `isReplay` on the genuine echo, so trust that
when present. Keep a shape test as the fallback for a CLI that omits the field,
now excluding harness turns via the repo's existing
`isKnownHarnessInjectedUserTurnText` rather than a second enumeration.
Also accept string `content`. `readClaudeMessageEnvelope` already tolerates it,
so rejecting it was a new way for a send to go unacknowledged - which times out
into a false "delivery unconfirmed" and sends the message twice on retry.
Found by the review loop on #15657.
Requiring `claudeMessageBody(...) !== null` inherited that function's content
model: it builds blocks only for text and URL images, while dispatch
base64-encodes local image paths. An image-only send was therefore refused its
OWN replay, ran out the 10s ack timeout, and reported a delivered message as
`unknown` - which offers retry, and retry sends it to Claude a second time. A
duplicate render traded for a duplicate send.
Reject the tool-result shape directly instead. That still keeps a tool-result
turn from stealing the identity, without making confirmation depend on which
content parts the translator happens to model.
Measured against the real CLI while checking the timeout concern: a queued
send's replay is published at the FIRST tool boundary after it is queued, 2-3ms
after that boundary's tool result (1004ms queued -> 4772ms tool_result ->
4774ms replay, across a 22s three-tool turn). So rejecting tool results costs no
time - whenever there was a frame to adopt, the right one follows immediately -
and a turn with no boundary inside the timeout times out identically either way.
Found by the review loop on #15657.
Claude publishes a top-level `user` turn for tool results as well as for the
`--replay-user-messages` echo of the prompt Orca dispatched. Both carry
`type: "user"` and a null `parent_tool_use_id`, so resolveClaudeReplayWaiter
adopted whichever arrived first.
A send issued while a turn is running is dispatched with tools in flight, so
the tool-result turn takes the waiter. Dispatch then records that uuid as the
provider identity, the journal aliases the submission onto an item nothing
ever upserts, and the real replay lands as its own user item - the user's
message renders twice.
Gate the waiter on the same body claudeMessageBody builds, so the identity
dispatch adopts is always the one the echo upserts into. An unrecognized
frame leaves the send `unknown` ("delivery unconfirmed"), never duplicated.
Both frames in the regression test are captured verbatim from
`claude -p --input-format stream-json --output-format stream-json
--replay-user-messages`; the previous double invented a `user` frame with no
message body at all, which the CLI never emits.
Brennan Benson