Repository navigation
Conversation
![opencode-agent[bot]](https://avatars.githubusercontent.com/in/1549082?s=60&v=4){kind=small}
There was a problem hiding this comment.
This fixes the stale queued card from #56748. I ran the issue's drive script (event stream dropped while the prompt is sent, simulated model). On the base, the delivered prompt stays a grey pending card after idle. With this PR it turns into a normal delivered message. The client tests, the TUI data.test.tsx and the type checks of both packages pass. Using a fetched message as proof of delivery is sound: on the server, a message row exists only after its inbox row was consumed, under the same ID.
Public API: adds data.session.pending.discard(sessionID, inboxID) to the Solid data layer in @opencode/client (backwards compatible). The server API, events and SDK types are unchanged.
One concern, in the inline comment: a 409 from steer/queue doesn't always mean the row is gone, so discarding on every ConflictError can hide a prompt that is still queued.
Related: #56735
| // The server rejects an update only when the row is already gone — | ||
| // delivered or cancelled while its echo was lost — which a stale queued | ||
| // card still renders. Drop it instead of leaving it until a restart. | ||
| if (typeof error === "object" && error !== null && "_tag" in error && error._tag === "ConflictError") { |
There was a problem hiding this comment.
A 409 here doesn't always mean the row is gone. projectDelivery in packages/core/src/session/inbox.ts only updates WHERE delivery = from, so steering a row that is already steer (or queueing one that is already queue) also returns InboxConflictError → ConflictError while the row still exists. That can happen in the same stream-drop case this PR targets: a delivery-change echo is lost, the card still shows the old mode, and the user clicks again. The card then disappears even though the prompt is still queued, until a later inbox re-read brings it back.
Could the conflict path drop the outbox guard and re-read the inbox (pending.sync), instead of removing the row outright? A consumed row then clears, and a live one stays.
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').
4a9c350 to
71c354b
Compare
|
Good catch on the 409 path — you're right that Added Re-verified: client suite 232 pass, TUI |
There was a problem hiding this comment.
The conflict concern is addressed. On a 409, the route now releases the outbox guard and re-reads the inbox instead of discarding the row, so a consumed row clears and a live one stays with the server's delivery mode. I reran the #56748 script on the new head and the delivered prompt no longer lingers as a pending card. The client tests, the TUI data.test.tsx and both packages' type checks pass.
Public API: adds data.session.pending.discard(sessionID, inboxID) and data.session.pending.release(inboxID) to the @opencode/client Solid data layer (backwards compatible). The server API, events and SDK types are unchanged.
No further concerns.
Issue for this PR
Closes #56748
Type of change
What does this PR do?
A queued prompt that got delivered while the event stream was down stayed rendered above the prompt input until you restarted the TUI; clicking it failed with a 409 because the server-side inbox row was already consumed. The local row is pinned by the
outboxguard, and only its inbox echo, a positive inbox read, or a rollback ever cleared it — but an empty inbox snapshot is also what a lagging projection looks like (that's a pinned invariant, see "keeps the row when the response lands before the echo" inpackages/tui/test/cli/tui/data.test.tsx), so absence alone can never prove the row consumed.So instead of dropping the guard on timing, the fix drops it on evidence:
message.syncremoves the matching pending row once the fetched messages contain its id. The server writes that message in the same transaction that consumes the inbox row, so a fetched message is proof of delivery — and the TUI already re-reads messages when the event stream reconnects, which is exactly when the stale card should clear.data.session.pending.release()unpins a row without removing it. On an inbox-mutationConflictErrorthe session route releases the guard and re-reads the inbox, so a consumed row clears while one that already has the requested delivery (a re-click after a lost delivery-change echo) stays.data.session.pending.discard()removes the row outright after a successful cancel, which is authoritative even when the row was already gone.removePendingclears theoutboxentry too, so every removal path fully unpins.One thing worth passing on: my first attempt was the straightforward version — clear the guard when the prompt POST settles, which is what the closed #57427 did — and it fails the lag test named above. Reads really can lag that response, so settlement cannot authorize dropping the row; positive evidence can.
How did you verify your code works?
Three regression tests in
packages/client/test/solid-inbox-race.test.ts: one walks prompt → both echoes lost → empty inbox read (row correctly kept) → message listing containing the delivered message (row drops); one pinsreleaseletting a read keep a live conflicted row and drop a consumed one; one pinsdiscardafter a successful cancel.packages/client: 232 pass, 0 failpackages/tui: 1537 pass, 1 pre-existing failure unrelated to this change (go-upsell-art"the art stops every frame" also fails on cleandev, verified by stashing);test/cli/tui/data.test.tsx50 passbun run check: 34/34 greenScreenshots / recordings
Not a UI change — the change removes a stale card; the reporter's before screenshots are already in the issue.
Checklist
Added by the opencode bot (AI-generated).
Related: #56735
Public API: adds data.session.pending.discard() and data.session.pending.release() to the @opencode/client Solid data layer (backwards compatible)