fix: preserve pre-turn spawn identity - #3
Conversation
📝 WalkthroughWalkthroughThe change adds ChangesInitial user message identity
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai review |
|
@codex review |
✅ Action performedReview finished.
|
|
P2 live gate is now PASS before merge, superseding the earlier pending statement in the PR body.
The durable native user messages intentionally have |
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
| function matchesMessageIdentity( | ||
| snapshot: NativeThreadSnapshot, | ||
| messageId: string, | ||
| ): boolean { | ||
| const observedMessageIds = [ | ||
| snapshot.latestUserMessageId, | ||
| snapshot.latestTurn?.userMessageId, | ||
| ].filter((candidate): candidate is string => candidate !== undefined); | ||
| return ( | ||
| observedMessageIds.length > 0 && | ||
| observedMessageIds.every((candidate) => candidate === messageId) | ||
| ); | ||
| } |
There was a problem hiding this comment.
🟠 High src/facade.ts:273
matchesMessageIdentity requires latestUserMessageId and latestTurn.userMessageId to be equal, but during ambiguous send recovery the new user message can be durable while latestTurn still reflects the previous completed turn. In that projection window latestUserMessageId matches the retried command but latestTurn.userMessageId identifies the prior turn, so the .every() check returns false and rejects a dispatch that actually succeeded, surfacing the ambiguous error instead of a recovered receipt. Consider not treating the prior turn's userMessageId as conflicting evidence for send — for example, matching when any candidate equals messageId rather than requiring all to agree.
| function matchesMessageIdentity( | |
| snapshot: NativeThreadSnapshot, | |
| messageId: string, | |
| ): boolean { | |
| const observedMessageIds = [ | |
| snapshot.latestUserMessageId, | |
| snapshot.latestTurn?.userMessageId, | |
| ].filter((candidate): candidate is string => candidate !== undefined); | |
| return ( | |
| observedMessageIds.length > 0 && | |
| observedMessageIds.every((candidate) => candidate === messageId) | |
| ); | |
| } | |
| function matchesMessageIdentity( | |
| snapshot: NativeThreadSnapshot, | |
| messageId: string, | |
| ): boolean { | |
| const observedMessageIds = [ | |
| snapshot.latestUserMessageId, | |
| snapshot.latestTurn?.userMessageId, | |
| ].filter((candidate): candidate is string => candidate !== undefined); | |
| return observedMessageIds.some((candidate) => candidate === messageId); | |
| } |
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @src/facade.ts around lines 273-285:
`matchesMessageIdentity` requires `latestUserMessageId` and `latestTurn.userMessageId` to be equal, but during ambiguous `send` recovery the new user message can be durable while `latestTurn` still reflects the previous completed turn. In that projection window `latestUserMessageId` matches the retried command but `latestTurn.userMessageId` identifies the prior turn, so the `.every()` check returns `false` and rejects a dispatch that actually succeeded, surfacing the ambiguous error instead of a recovered receipt. Consider not treating the prior turn's `userMessageId` as conflicting evidence for `send` — for example, matching when any candidate equals `messageId` rather than requiring all to agree.
|
Follow-up to the first live receipt: the final adversarial evaluator found and blocked an ambiguous-send recovery edge case. It is now fixed test-first in Fresh exact-head live gate is PASS:
A fresh independent evaluator is reproducing its former failing ambiguous-send case against this exact head before merge. |
1 participant
Summary
Why
A live production-adapter spawn was accepted and executed by T3, but the immediate read model had
latestTurn: nullwhile the initial user message was already durable. The facade rejected that valid intermediate snapshot astransport_unavailable.Verification
Live gate
This fixes the boundary found by the real isolated T3 RPC proof. The full two-turn live proof will be rerun after merge; this PR does not claim that gate is complete.
Note
Medium Risk
Changes fail-closed identity checks on spawn and ambiguous send recovery; incorrect matching could accept wrong dispatches or reject valid ones, but scope is limited to reconciliation paths with new regression tests.
Overview
Fixes false
transport_unavailablewhen spawn succeeds but the read model still haslatestTurn: nullwhile the initial user message is already visible.The native adapter now exposes
latestUserMessageIdonNativeThreadSnapshot(newest user message from orchestration detail, including before a provider turn exists). Spawn and ambiguous-sendreconciliation usematchesMessageIdentity, which treatslatestUserMessageIdandlatestTurn.userMessageIdas identity sources: at least one must be present and any present values must match the dispatchedmessageId. Conflicting sources are rejected instead of accepted.Tests cover the pre-turn spawn window, conflicting identity on send, and adapter projection for unassigned turns.
Reviewed by Cursor Bugbot for commit 1adf16e. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit
Note
Preserve pre-turn spawn identity using
latestUserMessageIdin thread snapshotslatestUserMessageIdtoNativeThreadSnapshotby scanning messages in reverse for the most recent user-role message, independent of whether a provider turn has been assigned.matchesMessageIdentityhelper in facade.ts that checks bothlatestUserMessageIdandlatestTurn.userMessageId, requiring all present sources to agree.latestTurn?.userMessageIdcomparisons in spawn and send reconciliation withmatchesMessageIdentity, so identity is accepted when either source matches and fails closed when sources conflict.latestUserMessageIdandlatestTurn.userMessageIdare both present but disagree, where previously onlylatestTurn.userMessageIdwas checked.Macroscope summarized 1adf16e.