Skip to content

fix(rollback): gateway and TUI bare /rollback fall back to all-directories - #90986

Open
pierrenode wants to merge 1 commit into
NousResearch:mainfrom
pierrenode:fix/rollback-all-directories-fallback
Open

fix(rollback): gateway and TUI bare /rollback fall back to all-directories#90986
pierrenode wants to merge 1 commit into
NousResearch:mainfrom
pierrenode:fix/rollback-all-directories-fallback

Conversation

@pierrenode

Copy link
Copy Markdown
Contributor

Summary

fa89d87ea6 (#10505, reapply of PR #10633 by @nightq) fixed the classic CLI's bare /rollback: when the current directory has zero checkpoints, it now falls back to a cross-project "all directories" view instead of reporting "No checkpoints found" despite fresh checkpoints existing under a different cwd (the fix's own commit message: writes landed checkpoints under the session cwd /tmp/qa-repo while bare /rollback searched only TERMINAL_CWD's project).

Two other independent implementations of the exact same bare-/rollback-listing logic never got the fix:

Both still just call mgr.list_checkpoints(cwd) and return whatever comes back — empty if empty, with no fallback and no hint that checkpoints exist elsewhere.

Fix

Both now call CheckpointManager.list_all_checkpoints() (the method the CLI's own fix introduced) with the exact same fallback condition: only when the session's own cwd has zero checkpoints AND at least one other directory has some.

  • The gateway handler mirrors the CLI's text output exactly ("No checkpoints for {cwd} — showing all directories." + the labeled list).
  • The RPC additionally returns a new all_directories: bool field so a future UI update can label the fallback list the way the CLI does; today's TUI client already renders the (larger) checkpoint list correctly without reading that field — nothing breaks by its absence being ignored.

Verification

  • New regression tests: tests/gateway/test_rollback_command.py (end-to-end against a real CheckpointManager + real git checkpoint store, mirroring tests/gateway/test_diff_command.py's established pattern) and tests/tui_gateway/test_rollback_list_rpc.py (invokes the installed rollback.list handler via server._methods["rollback.list"], same pattern as tests/tui_gateway/test_projects_rpc.py). Both cover: falls back when own-cwd is empty, no fallback claim when nothing exists anywhere, and own-cwd checkpoints always take priority over the fallback.
  • Mutation-verify: stashed both fixes, confirmed 4/6 new assertions genuinely fail against pre-fix code (KeyError: 'all_directories', missing fallback text) while the 2 "no fallback needed" negative controls correctly still pass unchanged. Restored the fixes, all 6 pass.
  • Broader neighbor suite: tests/gateway/test_rollback_command.py tests/gateway/test_diff_command.py tests/tui_gateway/test_rollback_list_rpc.py tests/tools/test_checkpoint_manager.py tests/tools/test_rollback_all_directories.py — 60 passed, no regressions.
  • ruff check clean on all changed files.

Notes

  • Restore-by-number (/rollback <N>) is intentionally left untouched, matching the CLI's own established scope: the fallback view is informational only (it shows what exists elsewhere so the user knows to cd there), not a promise that <N> against the shown listing is directly restorable — the CLI's own fix has this same limitation.
  • While writing the RPC test I found rollback.list's response reads a checkpoint's message field via c.get("message", ""), but CheckpointManager.list_checkpoints() actually names that field reason — every checkpoint's message has always come back empty. This is a separate, pre-existing bug unrelated to the all-directories fallback and is left out of scope here.
  • Checked for competing PRs: none touch the bare-arg listing branch of _handle_rollback_command or the rollback.list RPC. fix(gateway): /rollback now undoes the conversation turn it reverts #78603 (open, "/rollback now undoes the conversation turn it reverts") touches a different branch of the same function (the successful-restore path, if result["success"]:) — no overlap.

Test plan

  • New end-to-end tests cover the gateway handler and the TUI RPC
  • Mutation-verify: fixes reverted → fallback assertions fail; negative controls unaffected; fixes restored → all pass
  • Broader tests/gateway/ + tests/tui_gateway/ + tests/tools/ checkpoint/rollback suite green
  • ruff check clean

…ories

fa89d87 (NousResearch#10505, reapply of PR NousResearch#10633 by @nightq) fixed the classic
CLI's bare /rollback: when the current directory has zero checkpoints, it
now falls back to a cross-project "all directories" view instead of
reporting "no checkpoints found" despite fresh checkpoints existing under
a different cwd. Two other independent implementations of the same bare
/rollback listing never got the fix:

- gateway/slash_commands.py::_handle_rollback_command — used by every
  chat-platform /rollback (Discord, Slack, Telegram, etc.)
- tui_gateway/methods_tools.py's rollback.list RPC — used by the Ink TUI's
  /rollback (ops.ts), and planned for Desktop per NousResearch#90029

Both now call the same CheckpointManager.list_all_checkpoints() the CLI's
fix already introduced, mirroring its exact fallback condition (session's
own cwd has zero checkpoints, but at least one other directory has some).
The RPC additionally surfaces a new all_directories boolean so a future
UI update can label the fallback list the way the CLI's text output does;
today's TUI client already renders the (larger) checkpoint list correctly
without it.
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/gateway Gateway runner, session dispatch, delivery comp/tui Terminal UI (ui-tui/ + tui_gateway/) sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state labels Aug 20, 2026
@Enough1122

Copy link
Copy Markdown
Contributor

AI code review — automated review for reference; please use your judgment.

  1. gateway/slash_commands.py:3362-3385 — Scope mismatch between listing and restoring: bare /rollback now falls back to displaying list_all_checkpoints() (numbered 1..N across directories), but /rollback <number> re-resolves against mgr.list_checkpoints(cwd) and returns "none found" when cwd is empty — exactly the situation the fallback list was shown for. Hash args fare no better since mgr.restore(cwd, target_hash) looks inside cwd's checkpoints. Why it matters: the user sees a menu whose entries cannot be acted on from this surface ("3. baseline" → /rollback 3 errors), which converts an informational fix into a dead end mid-recovery. Suggestion: remember the fallback fired (e.g., stash the last displayed list + scope flag on the session/mixin) and resolve both number and hash against it, calling restore with the checkpoint's own directory.

  2. tui_gateway/methods_tools.py:1309-1332 — The RPC now exposes all_directories: true so clients can label the view — good additive design — but the paired rollback.apply path isn't touched, so a Desktop/TUI client rendering the cross-project list will hit the same cwd-scoped rejection on restore. Suggestion: either thread the same fallback resolution into apply (keyed off the client echoing back the checkpoint's directory/hash pair) or document on the RPC result that entries are read-only unless all_directories is false.

  3. tests/gateway/test_rollback_command.py + tests/tui_gateway/test_rollback_list_rpc.py — Positive: six end-to-end cases cover fallback, true-empty, and own-directory-priority on both surfaces, run through the real installed handlers (server._methods) rather than internals; the module docstrings accurately credit the upstream CLI fix being ported. No change requested.

  4. gateway/slash_commands.py:3367 — Nit: the fallback banner interpolates the raw cwd path into a chat message; on messaging surfaces a long container path eats into per-platform length limits and reads as noise. Suggestion: consider showing only the trailing directory component (the full list header already carries scope context).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/gateway Gateway runner, session dispatch, delivery comp/tui Terminal UI (ui-tui/ + tui_gateway/) P2 Medium — degraded but workaround exists sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants