Skip to content

Conversation

@riatzukiza
Copy link
Owner

@riatzukiza riatzukiza commented Nov 9, 2025

Summary

Testing

  • bun turbo typecheck
  • bun test packages/opencode/test/session/revert-todo.test.ts

Mirrored from sst/opencode PR anomalyco#4082

Summary by CodeRabbit

  • New Features

    • Enhanced undo functionality to preserve and restore associated todos when reverting changes.
  • Tests

    • Added comprehensive test coverage for todo management during undo operations, including multi-step revert and cleanup scenarios.

@coderabbitai
Copy link

coderabbitai bot commented Nov 9, 2025

Walkthrough

The changes extend the session revert functionality to capture and persist associated todo items. A new optional todos field is added to the Session.Info.revert schema, accompanied by implementation logic to extract todos from completed tool parts, compute revert targets based on messageID or partID matching, and synchronize the Todo store with the reverted state.

Changes

Cohort / File(s) Change Summary
Schema Extension
packages/opencode/src/session/index.ts
Adds optional todos field to Session.Info.revert, defined as Todo.Info.array().optional(), extending the revert structure to carry associated Todo items. Introduces import for Todo module.
Revert Implementation
packages/opencode/src/session/revert.ts
Implements extractTodos helper to pull Todo.Info[] from completed todowrite tool parts. Updates revert logic to compute revert targets based on messageID or partID matching. Captures todos before revert target, persists revert.todos to session state, and synchronizes Todo store via Todo.update. Adds imports for Todo and splitWhen utilities.
Integration Tests
packages/opencode/test/session/revert-todo.test.ts
New test file with writeTodoMessage helper and six regression-style tests verifying undo behavior across scenarios: reverting todo updates, deferred cleanup, multiple todo updates, non-todo messages, and todowrite parts. Tests validate session revert and cleanup operations against expected Todo state.

Sequence Diagram

sequenceDiagram
    participant User
    participant Session
    participant Revert as SessionRevert
    participant Todo
    
    User->>Session: post message with todowrite
    activate Session
    Session->>Revert: trigger revert
    activate Revert
    
    Note over Revert: Scan backwards for<br/>revert target
    Revert->>Revert: extractTodos from<br/>prior todowrite parts
    
    alt messageID match found
        Revert->>Revert: capture todosBefore
    else partID match found
        Revert->>Revert: capture todosBefore
    end
    
    Revert->>Revert: assign revert.todos
    Revert->>Todo: update with reverted todos
    deactivate Revert
    
    alt cleanup requested
        User->>Revert: trigger cleanup
        activate Revert
        Revert->>Todo: persist session.revert.todos
        Revert->>Session: clear revert state
        deactivate Revert
    end
    
    deactivate Session
    User->>Session: verify state
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Focus areas requiring attention:
    • Logic in packages/opencode/src/session/revert.ts for computing revert targets based on messageID vs. partID matching conditions
    • Todos extraction and state synchronization flow, particularly the order of operations during revert and cleanup phases
    • Test coverage in packages/opencode/test/session/revert-todo.test.ts for edge cases (e.g., deferred cleanup, multiple consecutive todo updates)

Poem

🐰 A hop, a skip, todos to flip!
Revert the past with a gentle script,
Todos remembered, state restored with care,
The session remembers what once was there! ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Fix undo restoring todo list' directly summarizes the main change: adding todo list restoration capability to the undo/revert functionality, which aligns with the PR's core objective of fixing issue anomalyco#4081.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch riatzukiza/undo-todo-revert

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 969af4d and 72e2819.

📒 Files selected for processing (3)
  • packages/opencode/src/session/index.ts (2 hunks)
  • packages/opencode/src/session/revert.ts (4 hunks)
  • packages/opencode/test/session/revert-todo.test.ts (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
packages/opencode/src/session/index.ts (1)
packages/sdk/js/src/gen/types.gen.ts (1)
  • Todo (564-581)
packages/opencode/test/session/revert-todo.test.ts (3)
packages/sdk/js/src/gen/types.gen.ts (1)
  • Todo (564-581)
packages/opencode/src/session/index.ts (3)
  • Info (38-76)
  • Info (77-77)
  • messages (273-287)
packages/opencode/src/project/instance.ts (1)
  • Instance (14-57)
packages/opencode/src/session/revert.ts (3)
packages/opencode/src/session/message-v2.ts (4)
  • Part (285-300)
  • Part (301-301)
  • Info (342-344)
  • Info (345-345)
packages/sdk/js/src/gen/types.gen.ts (3)
  • Part (886-896)
  • Todo (564-581)
  • Session (528-555)
packages/opencode/src/session/index.ts (2)
  • Info (38-76)
  • Info (77-77)
🔇 Additional comments (15)
packages/opencode/src/session/index.ts (2)

20-20: LGTM: Clean import addition.

The Todo import is properly added to support the new revert todos field.


70-70: LGTM: Schema extension follows existing patterns.

The optional todos field in the revert schema aligns well with the existing optional fields and enables persistence of todo state during undo operations.

packages/opencode/src/session/revert.ts (6)

43-43: LGTM: Correct capture of previous todo state.

The todosBefore variable correctly captures the most recent todos before the revert target is encountered. This ensures undo operations restore the previous plan state.

Also applies to: 67-72


56-66: LGTM: Improved target identification logic.

The separation of matchesMessage and matchesPart conditions properly handles both message-level and part-level undo operations. The partID derivation on line 61 correctly handles the edge case where a part should only be used as the target if there are preceding text or tool parts.


132-133: LGTM: Cleanup correctly persists reverted todo state.

The cleanup operation properly restores todos from the revert snapshot, ensuring the todo list reflects the undone state after cleanup completes.


11-12: LGTM: Clean import additions.

The Todo and splitWhen imports are properly added to support the new todo revert functionality.


17-23: No changes needed. The metadata structure is guaranteed by design.

The extractTodos function is already protected by the preceding filter on line 19 (if (part.tool !== "todowrite")). The TodoWriteTool always returns metadata: { todos: params.todos } (line 18 in todo.ts), where params.todos is validated as z.array(Todo.Info). The optional chaining on line 22 (metadata?.todos) safely returns undefined if the property is missing—it won't fail at runtime. No runtime validation is needed.


81-82: No action needed—the implementation is correct.

The code correctly captures and restores the todo state from before the target message. Todos are managed exclusively through the "todowrite" tool in message history and stored only within the optional revert object (not as a persistent session property). When undoing the first todo-setting operation, there is no prior state to restore, so defaulting to an empty array is the correct behavior. The concern about todos initialized elsewhere is not applicable since the schema shows todos only exist within the revert state.

packages/opencode/test/session/revert-todo.test.ts (7)

13-54: LGTM: Well-structured test helper.

The writeTodoMessage helper accurately simulates todowrite tool operations by:

  • Creating proper message and part structures
  • Setting metadata.todos matching the expected format
  • Updating both the part storage and Todo store

This provides a clean abstraction for test scenarios.


63-113: LGTM: Comprehensive test for basic undo behavior.

This test properly verifies that undoing a message with updated todos restores the previous todo state, testing both the revert and cleanup phases.


115-153: LGTM: Tests intermediate UI state.

This test correctly verifies that the todo state is reverted immediately upon calling revert(), before cleanup is triggered. This is important for UI responsiveness.


155-208: LGTM: Tests multiple sequential updates.

This test validates the critical behavior of reverting to the immediately previous todo state when multiple updates exist, confirming the "last before target" logic works correctly.


210-266: LGTM: Verifies non-todo message handling.

This test ensures that undoing a message without todos doesn't inadvertently affect the todo list, which is important for maintaining correct state.


268-318: LGTM: Tests part-level undo.

This test verifies that undoing a specific todowrite part (rather than an entire message) correctly reverts to the previous todo state. This completes the test coverage for both message-level and part-level undo operations.


1-319: Excellent test coverage overall.

The test suite comprehensively covers:

  • Basic undo/redo with todos
  • Immediate vs. post-cleanup state
  • Multiple sequential updates
  • Non-todo messages (no false positives)
  • Part-level undo

All tests include proper cleanup via Session.remove, preventing test pollution.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants