Repository navigation
Conversation
Withdraw undelivered inbox prompts on undo/revert, and fall back to staged revert when cancel races with promotion. Fixes message-dialog revert as well. Fixes anomalyco#39736
github-actions
Bot
added
needs:compliance
|
Thanks for updating your PR! It now meets our contributing guidelines. 👍 |
Call out the compliance template so agent sessions fill the body before opening PRs.
![opencode-agent[bot]](https://avatars.githubusercontent.com/in/1549082?s=60&v=4){kind=small}
There was a problem hiding this comment.
This fixes Revert from the message actions while a turn is running. I checked it with a simulated model in the TUI: on the base, Revert fills the prompt, shows Session is busy and keeps the turn running. With this PR it interrupts, waits, reverts and then restores the prompt. I couldn't reproduce the /undo failure from #39736 on the base: after a steer and an interrupt, /undo already reverts the steer there, because the base withdraws pending prompts itself. So for /undo the new part is only the fallback for when the prompt was delivered at the same moment as the cancel, and I couldn't trigger that case. Unit tests and the tui and app type checks pass.
The public API is unchanged; this only changes client-side undo and revert.
Concerns (inline): /undo no longer goes through mutatePending, so it loses that function's guard against firing twice and its error toast. The delivered-or-not check treats any error from message.get as "withdrawn". Also, #54163 makes the same cancel-then-message.get change for /undo in the shared data layer (data.session.undo, run in the session's send chain). It also handles a cancel that reaches the server before the prompt's own request. The two will conflict in index.tsx, so it would be good to agree with that PR on one shared helper instead of a separate TUI-only undo.ts.
Fixes #57383
Related: #54163, #52537
There was a problem hiding this comment.
The only new commit adds a section to the root AGENTS.md. The undo code is unchanged, so the two earlier points are still open: errors from message.get are treated as "withdrawn", and the pending branch of /undo skips mutatePending. The fix still behaves as I tested before: Revert during a running turn now interrupts, reverts and only then restores the prompt.
The public API is unchanged.
The new AGENTS.md section is a repository-wide rule for agents writing PR descriptions, which has nothing to do with this fix. Please drop it from this PR and propose it on its own if you want it.
Keep /undo pending cancel on mutatePending for single-flight and toasts. Treat only MessageNotFoundError from message.get as withdrawn.
|
Addressed the two inline review points in 53ebc0f:
On overlap with #54163: happy to converge on |
|
Also dropped the unrelated |
There was a problem hiding this comment.
Both earlier points are addressed. Only "message not found" from message.get now counts as withdrawn, and other errors show a toast and leave the prompt alone, in both the TUI and the app. /undo cancels through mutatePending again, so it keeps the guard against firing twice and the failure toast, and the prompt is not restored if the cancel fails. The AGENTS.md change is reverted too.
I ran the Revert-while-a-reply-streams steps again with a simulated model. The turn is interrupted, the message is reverted and the prompt comes back, while on the base nothing is reverted. Steer, interrupt, then /undo still works. The unit tests (7) and the tui and app type checks pass.
The public API is unchanged. I found nothing new.
Move cancel-then-probe undo into data.session.undo / input.withdraw so cancel cannot overtake the prompt POST, matching the approach in anomalyco#54163. Wire TUI /undo, message Revert, queued undo, and app revert through it.
|
Follow-up on the remaining soft concern from the first review (overlap with #54163): In 8e24280, undo moved into the shared client data layer as
|
There was a problem hiding this comment.
Moving undo into the shared data layer (data.session.undo / data.session.input.withdraw on the per-session send chain) works as intended in the TUI. With a simulated model, Revert from Message Actions during a streaming turn now interrupts, reverts and restores the prompt. The new TUI tests (4) and the tui type check pass.
There is one blocker: this commit removes recall from createSessionRevert in packages/app/src/session/revert.ts, but packages/app/src/session/composer/region.tsx (line 222) still calls revert.recall(item). The app type check now fails with TS2339, and the app would crash when it tries to recall a prompt. This looks like a leftover from merging dev, which added recall in #57466; please restore it.
Public API: data.session.undo(...) and data.session.input.withdraw(...) are new methods on the data store in @opencode/client (solid/data.ts). This is backwards compatible.
This is now essentially the same approach as #54163, which is still open, so maintainers will need to pick one of the two.
Keep recall on createSessionRevert; region.tsx still calls it when recalling a prompt into the composer.
|
Restored |
There was a problem hiding this comment.
The recall blocker is fixed: the helper is back in createSessionRevert, and the app type check passes again. I found nothing new in this commit.
Public API: unchanged from my last review. data.session.undo(...) and data.session.input.withdraw(...) are new, backwards-compatible methods on the data store in @opencode/client.
Issue for this PR
Fixes #39736
Type of change
What does this PR do?
Undo and message-dialog Revert could call
session.revert.stagewith an admitted inbox ID that is not insession_messageyet (common after interrupt, before delivery). The server then returnsMessage not found.Undo now lives in the shared Solid data layer as
data.session.undo/data.session.input.withdraw, on the per-session send chain so cancel cannot overtake the prompt POST (same approach as #54163). Withdraw probessession.message.getand treats onlyMessageNotFoundErroras withdrawn; otherwise it interrupts, waits, and stages. TUI/undo, message Revert, queued-prompt undo, and app revert all use that path.How did you verify your code works?
bun test test/session-undo.test.tsxinpackages/tui— 4 passed (admission overtake, delivered race, rejected admission, queued already-delivered)Screenshots / recordings
N/A — behavior/state fix, no UI chrome change.
Checklist
Added by the opencode bot (AI-generated).
Fixes #57383
Related: #54163, #52537