Repository navigation
fix(client): reconcile inbox rows after missed delivery events - #57427
aniruddhaadak80 wants to merge 1 commit into
Conversation
A prompt's optimistic row was guarded by its `outbox` entry until the server echoed `session.inbox.enqueued`. If the input was delivered before that echo, or the enqueue/deliver events were missed across a reconnect, the guard never cleared and every inbox snapshot kept the row as "inflight" even though the server had already promoted it. The queued card then stayed above the prompt until restart, and acting on it returned 409 because the server row was gone. Clear the optimistic guard when the admission POST succeeds (a durable acknowledgement), on delivery/cancellation, and refresh the authoritative inbox at turn boundaries and after a failed inbox action. Add a regression test where admission succeeds but the delivered event is lost.
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.
![opencode-agent[bot]](https://avatars.githubusercontent.com/in/1549082?s=60&v=4){kind=small}
There was a problem hiding this comment.
Looks good. I ran the #56748 reproduction with opencode-drive: it drops the TUI's connections right before sending a prompt. On the base, the delivered prompt stays as a pending card after idle. With this change it shows as a normal delivered message (screenshots below). The new regression test fails on the base and passes here. The packages/client suite passes (230 pass, 0 fail), and typecheck is clean in packages/client and packages/tui.
Clearing the outbox guard once the admission POST succeeds matches what compact() already does. One small side effect: an inbox read that started before the POST and finishes after it, but before the session.inbox.enqueued event, can briefly drop the pending card. The transcript message stays, and the event puts the card back, so I don't think that blocks this.
Public API is unchanged.
|
The following comment was made by an LLM, it may be inaccurate: |
|
@aniruddhaadak80 — heads up: this closed about a minute before the bot's validation review posted, and that review is positive — it reproduced #56748 on base with opencode-drive, confirmed this branch clears the stale card, and the new regression test fails on base / passes here. If you're not planning to reopen it, I've taken the fix forward in #58072 with credit to your diagnosis. One part of this branch doesn't survive the existing test suite, fwiw: clearing the outbox guard when the prompt POST settles fails the pinned "keeps the row when the response lands before the echo" test in |
A prompt admitted optimistically is pinned by the outbox guard until its inbox echo, a positive inbox read, or a rollback clears it. When the event stream drops across both the enqueued and delivered echoes, no inbox read can prove the row consumed — an empty snapshot is also what a lagging projection looks like — so the queued card outlived its turn until restart, and clicking it hit a 409.\n\nDrop the guard from evidence instead. A fetched message proves delivery: the projection writes the message in the same transaction that consumes the inbox row, so message.sync removes the matching pending row. A lifecycle conflict proves the row is not in the expected state; the session route releases the guard and re-reads the inbox, so a consumed row clears while one that already has the requested delivery stays. A successful cancel is authoritative on its own and discards the row; removePending clears the outbox entry for every removal path.\n\nClearing the guard when the prompt POST settles, as in anomalyco#57427, breaks the pinned invariant that reads may lag that response (test/cli/tui/data.test.tsx 'keeps the row when the response lands before the echo').
Issue for this PR
Closes #56748
Type of change
What does this PR do?
A queued user prompt could stay displayed above the prompt input after it had been delivered, and clicking it returned
409because the server-side inbox row was already gone.The root cause is the optimistic-admission guard. When a prompt is sent, the client admits it locally and records its ID in
outbox. That guard is meant to keep the optimistic row through an inbox read that may race the admission. It was cleared by a positive inbox read, thesession.inbox.enqueuedecho, or a rollback — but not by a successfulsession.promptresponse.If the input was delivered before the echo (or the enqueue/deliver events were missed across a reconnect), the ID stayed in
outboxforever. Every later inbox snapshot then treated the local row as still in flight, even though the server had removed it. The row was pinned until restart, which is exactly what the report describes. Image prompts make the window wider because admission takes longer, but any prompt can hit it.This change:
409from a stale card) reconciles instead of leaving the card on screen.How did you verify your code works?
New regression test in
packages/client/test/solid-inbox-race.test.ts: admission succeeds, the delivered event is never emitted, and the next inbox snapshot is empty. Without the fix the row stays pending (Received: true); with it the row is gone.packages/client: 230 pass, 4 skip, 0 fail (bun test --timeout 30000)bun run typecheckpasses inpackages/clientandpackages/tuioxlintreports 0 warnings/errors on the changed filesI could not reproduce the original Windows report directly, but the test pins the stale-state mechanism and the UI path now reconciles on the
409.Screenshots / recordings
Not a UI change. (The behavior change removes a stale card.)
Checklist