fix: restore missing revert actions and chat layout after resize#4157
fix: restore missing revert actions and chat layout after resize#41571-minute-to-midnight wants to merge 13 commits into
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
ApprovabilityVerdict: Needs human review 2 blocking correctness issues found. This PR introduces a new shadow checkpoint system and rewrites conversation revert logic across server and client. Multiple high-severity unresolved review comments identify potential bugs in rollback counting and atomicity of workspace/conversation state, warranting careful human review. You can customize Macroscope's approvability policy. Learn more. |
There was a problem hiding this comment.
The Effect service conventions require catching statically known tagged failures with Effect.catchTags({ ... }) — including when handling only a single tag — and explicitly prohibit Effect.catchTag. This PR introduces two new catchTag call sites. Pre-existing catchTag usages elsewhere in the repository are out of scope for this review.
Posted via Macroscope — Effect Service Conventions
There was a problem hiding this comment.
One Effect error-convention violation: raw git stderr is copied into VcsProcessExitError.detail, which is interpolated into the caller-visible message. Prefer a bounded, normalized detail and capture the raw output only as stderrLength, matching the canonical VcsProcessExitError.fromProcessExit factory.
Posted via Macroscope — Effect Service Conventions
Resolve symbolic workspace aliases before choosing between the owned Git repository and the shadow checkpoint store. This keeps /var and /private/var representations from splitting checkpoint state on macOS. Co-authored-by: codex <codex@users.noreply.github.com>
733a3d8 to
1bafb7b
Compare
| if (nextEntry.message.role === "user") { | ||
| break; | ||
| } |
There was a problem hiding this comment.
🟡 Medium components/ChatView.logic.ts:59
deriveRevertTurnCountByUserMessageId skips a user message that has no assistant reply yet whenever it encounters the next user message, so the earlier user message is never assigned a revertTurnCount. This happens during steering, which appends another user message to the same running turn before any assistant message exists (the code explicitly supports this). After the turn completes, the first user message's revert button stays permanently disabled even though reverting to that earlier index is valid. The inner loop should continue scanning past subsequent user messages instead of breaking, or otherwise account for steering-appended messages.
| if (nextEntry.message.role === "user") { | |
| break; | |
| } | |
| if (nextEntry.message.role === "user") { | |
| continue; | |
| } |
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/web/src/components/ChatView.logic.ts around lines 59-61:
`deriveRevertTurnCountByUserMessageId` skips a user message that has no assistant reply yet whenever it encounters the next user message, so the earlier user message is never assigned a `revertTurnCount`. This happens during steering, which appends another user message to the same running turn before any assistant message exists (the code explicitly supports this). After the turn completes, the first user message's revert button stays permanently disabled even though reverting to that earlier index is valid. The inner loop should continue scanning past subsequent user messages instead of breaking, or otherwise account for steering-appended messages.
| data-timeline-layout-generation={timelineLayoutGeneration} | ||
| > | ||
| <LegendList<MessagesTimelineRow> | ||
| key={timelineLayoutGeneration} |
There was a problem hiding this comment.
🟡 Medium chat/MessagesTimeline.tsx:530
When the viewport width settles after a resize, the list remounts via key={timelineLayoutGeneration} with initialScrollAtEnd still set, so the scroll position jumps to the conversation end even if the user was reading older messages. The remount resets the LegendList to its initial scroll position instead of preserving the current offset. Consider saving the current scroll offset before remounting and restoring it afterward (for example, via initialScrollIndex or a post-mount scrollToOffset), or gate initialScrollAtEnd so it only applies on the first mount.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/web/src/components/chat/MessagesTimeline.tsx around line 530:
When the viewport width settles after a resize, the list remounts via `key={timelineLayoutGeneration}` with `initialScrollAtEnd` still set, so the scroll position jumps to the conversation end even if the user was reading older messages. The remount resets the `LegendList` to its initial scroll position instead of preserving the current offset. Consider saving the current scroll offset before remounting and restoring it afterward (for example, via `initialScrollIndex` or a post-mount `scrollToOffset`), or gate `initialScrollAtEnd` so it only applies on the first mount.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 4 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 1bafb7b. Configure here.
| return; | ||
| // Refresh the workspace entry index so the @-mention file picker reflects | ||
| // the reverted filesystem state before conversation history is rewound. | ||
| yield* workspaceEntries.refresh(sessionRuntime.value.cwd); |
There was a problem hiding this comment.
Provider rollback over-counts steering
High Severity
The revert logic calculates rolledBackTurns using the count of user messages, which can overstate the number of provider turns to roll back. This mismatch, especially with multiple user messages in a single provider turn, may cause the provider to truncate its history excessively, leading to an inconsistent state.
Reviewed by Cursor Bugbot for commit 1bafb7b. Configure here.
| return; | ||
| // Refresh the workspace entry index so the @-mention file picker reflects | ||
| // the reverted filesystem state before conversation history is rewound. | ||
| yield* workspaceEntries.refresh(sessionRuntime.value.cwd); |
There was a problem hiding this comment.
Workspace reverts before rollback
High Severity
During checkpoint revert, the workspace is restored and refreshed before provider rollback and thread.revert.complete. If rollbackConversation or later steps fail, the handler records failure but leaves files already reverted while conversation history and projections stay unchanged, diverging workspace and chat.
Reviewed by Cursor Bugbot for commit 1bafb7b. Configure here.
| continue; | ||
| } | ||
| if (turn.checkpointTurnCount === null) { | ||
| latestTurnId = turn.turnId; |
There was a problem hiding this comment.
Null checkpoint overwrites latest turn
Medium Severity
After thread.reverted, the SQL projection loop assigns latestTurnId for every retained turn with a null checkpointTurnCount, overwriting a higher checkpoint turn chosen earlier. Persisted latestTurnId can point at the wrong turn versus the in-memory projector and retained checkpoints.
Reviewed by Cursor Bugbot for commit 1bafb7b. Configure here.
| config.stateDir, | ||
| "checkpoints", | ||
| NodeCrypto.createHash("sha256").update(NodePath.resolve(cwd)).digest("hex"), | ||
| ); |
There was a problem hiding this comment.
Shadow store splits symlink paths
High Severity
Shadow checkpoint storage keys the bare repo with sha256(NodePath.resolve(cwd)) only. The same workspace reached via different path strings (symlinks, relative vs absolute) gets separate shadow repositories, so captures and reverts can miss refs and fail closed or restore the wrong snapshot.
Reviewed by Cursor Bugbot for commit 1bafb7b. Configure here.


Summary
This fixes two user-facing issues:
Revert was unavailable in most threads
The revert icon would appear in some conversations but be completely absent in many others. This made the feature feel inconsistent and often impossible to use.
The fix keeps the revert action available on user messages and makes checkpoint matching work across current, resumed, imported, and older threads. While a turn is still running, the action remains visible but disabled with an explanation.
Reverting to a message now restores the conversation to that point and restores the corresponding workspace files. This also works in folders that are not Git repositories: T3 stores its own snapshots outside the workspace and does not create a
.gitdirectory in the user's project.Implementation details:
GitVcsDriverand failing with exit code 128Additional revert failure screenshots
Resizing the window could permanently squish the conversation
Opening an external desktop window, such as KDE Konsole beside T3 Code, temporarily narrowed the app as expected. After closing that window, the T3 layout expanded again but the conversation rows remained collapsed into one-character-wide strips. Switching to another thread and back was previously required to reset them.
The virtualized message list was retaining cached row measurements from the narrow layout. The fix refreshes the list after resizing settles, covering element resize notifications, desktop window and visual viewport changes, and compositor transitions that do not reliably emit the same event sequence.
Validation
t3,@t3tools/web, and@t3tools/client-runtimehelloremoves a file created by the next turn and removes every subsequent messagebackend readyandmain window createdin an isolated smoke testDevelopment note
ChatGPT 5.6 Sol was used to debug these issues, implement the fixes, and run the focused verification. The packaged result was then manually tested and confirmed by the reporter.
Note
Restore missing revert actions and fix chat layout after viewport resize
thread.revertedto truncate at the Nth user message boundary (by index) rather than by checkpoint-derived turn IDs, preserving uncheckpointed predecessor messages and always retaining system messages across the server projector, client reducer, and orchestration pipeline.deriveRevertTurnCountByUserMessageIdinChatView.logic.tsto map user messages to revert indices by matching checkpoints viaassistantMessageIdorturnIdfallback, including non-streaming assistant messages.MessagesTimeline.tsxwith context-aware disabled state and tooltip, instead of being hidden when unavailable.MessagesTimelineremountingLegendListafter viewport width changes settle (debounced 120ms) to reset cached measurements and correct layout after resize.CheckpointStoreto support non-Git workspaces via a per-workspace shadow bare Git repository, and fixes checkpoint restore in unborn repositories inGitVcsDriver.checkpoint.revert.failedactivity is recorded.Macroscope summarized 1bafb7b.
Note
High Risk
Changes checkpoint capture/restore, revert semantics, and Git detection across server and client; incorrect behavior could corrupt workspace files or desync conversation from disk state.
Overview
Restores revert-to-message across more thread types and fixes a chat layout bug after the window narrows and widens again.
Checkpoints and revert:
CheckpointStorenow uses a T3-managed shadow Git repo under app state when the workspace does not own a Git root (non-Git folders, nested projects).isGitRepositoryonly returns true for the canonical repo root.CheckpointReactorno longer skips checkpoint work for non-Git cwd; revert fails closed on missing filesystem snapshots when checkpoint summaries exist, but still allows conversation-only rollback for legacy threads. Revert retention onthread.revertedis rewritten to cut at the Nth user message (conversation index), not checkpoint turn IDs—server projector, projection pipeline, and clientthreadReducerstay aligned.UI:
deriveRevertTurnCountByUserMessageIdcentralizes revert targets (checkpoints, turn IDs, conversation order). Revert stays visible on user messages (disabled until the turn can revert).MessagesTimelineremounts the virtualized list after viewport width changes so rows do not stay one-character wide.Reviewed by Cursor Bugbot for commit 1bafb7b. Bugbot is set up for automated code reviews on this repo. Configure here.