Add the /undo and /redo picker, with the cascade warning (part 2 of #944) - #1344
Open
KazenDev wants to merge 2 commits into
Open
Add the /undo and /redo picker, with the cascade warning (part 2 of #944)#1344KazenDev wants to merge 2 commits into
KazenDev wants to merge 2 commits into
Conversation
Snapshot capture, restore, and the per-chat journal. No UI yet: the picker, the command registry entry, and the send-message hook come in the follow-up. Three data-loss paths closed, each with a test that fails without it: - a failed `git add` no longer hands back the tree of a stale index - every entry is anchored (`refs/freebuff/undo/<chat>/<hash>`) so the cleanup job's prune cannot collect a snapshot the journal still lists - reverting checks the content, not just the tree: `git checkout` deletes a file whose blob it cannot read, and `ls-tree` still lists it The journal is the root set: refs are released when the entry is dropped, and `sweepAnchors` collects what a deleted chat left behind. Verified: 20 tests pass, the CLI typecheck has no new errors, and the full suite matches its baseline (+20 pass, same 61 pre-existing failures).
…odebuffAI#944) The UI half: the picker, the commands, and the send-path hook. The snapshot engine (part 1) is CodebuffAI#1343 and is not touched here. Two review points from CodebuffAI#944, each with a test that fails without its fix: - The picker warned about the cascade only in a code comment. The files panel now says how many newer turns go with the selected one, and lists the union of the files they touch rather than the selected turn's alone. - recordUndoEntry ran in the finally even when a newer turn had started (an interrupt releases the chain lock first), so its diff picked up that turn's files. A turn now stamps itself when it starts and records only while its stamp is still the newest for that chat. Verified: 16 new tests, the CLI suite at 3069 pass with the same 74 pre-existing failures as the port alone, and a typecheck with no undo errors.
This was referenced Sep 13, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Second half of #944 (the
/undoand/redofeature): the picker, the commands, and the two review points that only exist on this side of the split — the race in thefinally, and the warning the picker owes the user.Stacked on #1343. The four engine files (
undo-snapshot.ts,undo-store.tsand their two test files) also appear in this diff and are unchanged here — they belong to that PR, which the review asked for first. Read #1343 first; the diff that belongs to this PR is the other 17 files. When #1343 lands, this rebases ontomainand those four files drop out of the diff on their own.Not a rebase of #944: the history rewrite destroyed its base commit, so the UI half was re-applied on current
mainby a three-way merge against the original commit.What this is
17 files, +1302/-16. Of the three files added here that are not new logic (
undo-history-screen.tsx,command-result-block.tsx,undo-history-store.ts), 569 lines are the original UI, carried over.cli/src/components/undo-history-screen.tsxcli/src/components/blocks/command-result-block.tsxcli/src/state/undo-history-store.tscli/src/app.tsxcli/src/commands/command-registry.tscli/src/hooks/use-send-message.tscli/src/utils/settings.tscli/src/data/slash-commands.tscli/src/chat.tsxcli/src/types/chat.tscli/src/components/message-with-agents.tsxcli/src/utils/message-history.tscli/src/components/message-block.tsxcli/src/utils/active-run.tscli/src/state/undo-guards.tscli/src/state/__tests__/undo-guards.test.tscli/src/components/__tests__/undo-history-screen.test.tsxOf the seven files
mainhad moved, three merged clean and four conflicted; all seven conflicts were the same shape — both sides added something at the same point — and were resolved by keeping both. The one that needed a decision wasuse-send-message.ts: the pre-turn snapshot has to be captured beforeclient.run(), so the two sides could not just be concatenated.1. The picker now says how far back the undo reaches
The review: "jumping back via
/undoreverts everything newer … users need to be warned about clearly in the picker (right now it's only in a code comment, not obviously surfaced in the UI copy for multi-turn jumps)."It was only in a code comment. Now the files panel carries the notice for the focused row, before
Enter:Frame rendered by the component test, trimmed to the panel.
Two things had to be true for that line to be honest, and neither was:
newerTurnCountreturns it; the same call gives the redo picker its counterpart ("⚠ Also discards 2 newer redos"), because a redo jump invalidates the newer redo entries the same way.undoToRecordactually reverts (filesTakenBack), and for a redo the turns it brings back (filesRestored).shared.ts, touched by three turns, is listed once.This is not a nicety in this feature. Claude Code's
/rewindis the same shape — restore the picked checkpoint, reverting everything after it — and its blast radius is the single most-reported complaint about it: #64615 (open) asks for exactly "a blast-radius preview (file count + list …) before applying a code revert", after #50897 and #27387 were closed as not planned; one user patched the binary to move the destructive option off the default. The same tracker has #34368, "Rewind offers to undo file deletions it cannot actually restore" — the hole fix 3 of #1343 closes here.2. A turn that was replaced no longer records
The review: "
recordUndoEntryin thefinallyblock runs regardless of whether the run was superseded by a newer one; interleaved concurrent turns could record entries against the wrong snapshot state."Reproduced path:
client.run().Escthe chain lock is released before A'sfinallyfinishes — the code says so in the block below this one.finallycallspatchSnapshot, which diffs the worktree as it is now: B's files are in that diff./undoon that entry reverts it, and reports it as the change the user selected.The fix is a stamp, not a lock: the turn stamps itself when it captures its snapshot (
beginUndoTurn), and records only while its stamp is still the newest for that chat (isLatestUndoTurn) — checked before the diff, and again after it, which is the await where a newer turn can appear. Past that point the turn is dropped rather than recorded wrong, which matches the best-effort rule the rest of the feature already follows: a failure disables undo for that turn instead of breaking the chat.Stamps are per chat, so switching chats mid-run cannot invalidate a turn, and a turn that started in another chat still records into its own. In memory only —
undo.jsondoes not change shape.Where each point of the review is answered
recordUndoEntryruns even if the turn was replacedNot in this PR, on purpose
• Create new.py→/undo→new.pydeleted → the next turn "the earlier write may not have completed — let me create it again"). Restoring the conversation, staging that cut so/redocan lift it, and offering code only vs code and conversation the way Claude Code and OpenCode do is the next PR, stacked on this one — and that is also where the removed prompt goes back into the input for editing, so the default here is expected to change.Enter, butEnterstill runs it. Claude Code's report asks for both a preview and a confirmation ("continue? (y/N)"); gating a destructive action is a product decision with its own UX, so it is a follow-up rather than a silent addition here.↺/🗑distinction appears only in the confirmation, after the fact. That is a redesign of the row, and it belongs with the confirmation gate above.skippedcount Add the /undo snapshot engine, anchored so it survives cleanup (part 1 of #944) #1343 deferred to this half is still not reported. That note says the counter has to come out ofrevertFiles, which is an engine file this PR claims not to touch. Adding it here would make that claim false; it stays tracked, and the summary keeps being silent rather than wrong.Undid 3 change(s) back to: …— the pluralisation fix is an edit toundo-store.ts, same reason.finally's ordering withreleaseRunOwnershipchanged. The stamp answers the race the review named; whether an interrupted turn should record at all is a separate question.Verification
Public CI does not run this suite. Both columns are the same command, on the same machine, against the worktree with the steps above removed and re-added — not against memory:
bun test cli/src)diffafter normalising timings is empty)undo+16 pass and +2 files is exactly the 16 tests added here. The 74 failures are pre-existing and unrelated (release wrapper, model picker, prompts).
The UI assertions fail without the change: with the picker reverted to the ported version, 3 of its 5 tests go red —
The two that stay green either way are the absence assertions ("says nothing when there is nothing newer"), which is expected.
How to try it
bun test cli/src/components/__tests__/undo-history-screen.test.tsx \ cli/src/state/__tests__/undo-guards.test.ts bun run --cwd cli typecheckManual walkthrough, from #944's validation comment:
/undoneeds a git root (the snapshot repo borrows its objects), and the picker only opens once the journal has an entry — and a turn that changed no files leaves none./undo— the picker opens. The newest entry shows nothing extra; move down with the arrows and the notice appears ("⚠ Also reverts 2 newer changes") while the file panel grows to the union of the turns that go with it.Enterreverts the selected turn and every newer one, and the confirmation shows the diff stat./redo— its picker warns "⚠ Also discards N newer redos" for the same reason./undostill lists the history.