feat(chat): rewind and fork a session from a message bubble#110660
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 495360cb2b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| usageFamilyKey: params.forked ? undefined : params.currentEntry.usageFamilyKey, | ||
| usageFamilySessionIds: params.forked ? undefined : params.currentEntry.usageFamilySessionIds, |
There was a problem hiding this comment.
Preserve rotated session lineage on rewind
When rewinding a normal session that has no existing usageFamilySessionIds, this copies undefined while the row is moved to a new sessionId, so the old transcript identity drops out of the entry. The usage and cleanup paths only include the current sessionId plus entry.usageFamilySessionIds (src/gateway/server-methods/usage.ts and src/config/sessions/session-accessor.sqlite-lifecycle-state.ts), so pre-rewind usage disappears from family reports and deleting the session cannot collect the rotated-out transcript rows. Use the same lineage preservation as other same-key session-id rotations.
Useful? React with 👍 / 👎.
| const text = content | ||
| .flatMap((block) => { | ||
| const record = asRecord(block); | ||
| return record?.type === "text" && typeof record.text === "string" ? [record.text] : []; |
There was a problem hiding this comment.
Reuse transcript text extraction for editor text
For persisted user messages whose content blocks use the supported Responses/OpenAI input_text shape, this returns no editorText, so rewind/fork succeeds but the composer is blank instead of being refilled with the message. Other transcript/UI readers accept text, input_text, and output_text blocks (for example src/config/sessions/session-transcript-projection-rebuild.ts and ui/src/lib/chat/message-extract.ts), so this extraction should follow the same content contract.
Useful? React with 👍 / 👎.
| const sessionKey = state.sessionKey; | ||
| const agentParams = scopedAgentParamsForSession(state, sessionKey); | ||
| try { | ||
| const result = await state.sessions.rewind(sessionKey, entryId, agentParams); |
There was a problem hiding this comment.
Fence late message-cut results by connection
If the Gateway connection is replaced while sessions.rewind is in flight, createSessionCapability can still resolve the committed result from the old client (see ui/src/lib/sessions/index.test.ts), but this path does not capture/check the client or connectionEpoch after the await. A same-key reconnect can therefore clear the current cache, persist the old editorText, and reload the current gateway from a stale operation; guard this like createSession/isConnectionScopeCurrent before mutating state or storage.
Useful? React with 👍 / 👎.
|
Codex review: needs real behavior proof before merge. Reviewed July 18, 2026, 9:34 AM ET / 13:34 UTC. Summary PR surface: Source +1077, Tests +845, Docs +1, Other +2. Total +1925 across 35 files. Reproducibility: not applicable. This PR proposes a new session-navigation capability rather than reporting broken existing behavior. The supplied focused tests describe the intended server and UI paths, but they are not a real-session demonstration. Review metrics: 2 noteworthy metrics.
Stored data model Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Proof guidance:
Mantis proof suggestion Risk before merge
Maintainer options:
Next step before merge
Maintainer decision needed
Security Review detailsBest possible solution: Adopt the feature only if a session-state owner confirms the core product direction, then add redacted Control UI proof showing rewind and fork against a persisted SQLite session, including the refreshed transcript and composer result. Do we have a high-confidence way to reproduce the issue? Not applicable: this PR proposes a new session-navigation capability rather than reporting broken existing behavior. The supplied focused tests describe the intended server and UI paths, but they are not a real-session demonstration. Is this the best way to solve the issue? Unclear: exposing existing transcript-DAG capability through the chat UI is coherent, but making it a core persisted-session and public gateway API needs explicit maintainer confirmation of the lifecycle and permission contract. AGENTS.md: unclear because the file could not be read completely. Codex review notes: model internal, reasoning high; reviewed against 42133d0c3c3e. Label changesLabel justifications:
Evidence reviewedPR surface: Source +1077, Tests +845, Docs +1, Other +2. Total +1925 across 35 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
Review history (3 earlier review cycles)
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ed26bff77e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
| const targetKey = | ||
| action === "fork" ? buildDashboardSessionKey(current.target.agentId) : current.canonicalKey; | ||
| const result = await (action === "fork" |
There was a problem hiding this comment.
Reject locked sessions before message cuts
When the target row has current.entry.modelSelectionLocked === true (for example catalog-created sessions or durable harness-owned rows), this path still performs sessions.fork/sessions.rewind; the clone then clears modelSelectionLocked and agentHarnessId in src/config/sessions/session-accessor.sqlite-message-cut.ts:250, and rewind also rotates the transcript sessionId. Existing parent-fork and checkpoint restore paths reject locked rows, so these new RPCs let operator.write/admin callers bypass the lock and detach owner/runtime identity. Please reject locked entries before dispatching the cut.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c70135879c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| export const SessionsRewindParamsSchema = closedObject({ | ||
| sessionKey: NonEmptyString, | ||
| agentId: Type.Optional(NonEmptyString), | ||
| entryId: NonEmptyString, | ||
| }); |
There was a problem hiding this comment.
Register session-cut schemas with protocol registry
Adding these public request/result schemas without also adding them to ProtocolSchemas leaves the generated native/JSON protocol surface incomplete: repo-wide search shows SessionsRewind/SessionsFork are absent from packages/gateway-protocol/src/schema/protocol-schemas.ts and from the generated Swift/Kotlin protocol artifacts, even though the Android method enum was updated. Native clients and protocol generation therefore cannot see the new sessions.rewind/sessions.fork payload types until the registry and generated files are refreshed.
Useful? React with 👍 / 👎.
| const sourceKey = state.sessionKey; | ||
| const agentParams = scopedAgentParamsForSession(state, sourceKey); | ||
| try { | ||
| const result = await state.sessions.forkAtMessage(sourceKey, entryId, agentParams); |
There was a problem hiding this comment.
Fence late fork results by connection scope
If sessions.fork completes after the Gateway connection has been replaced, SessionCapability.forkAtMessage still returns the committed result while skipping only the refresh; this handler then persists the draft and opens result.sessionKey as long as the same source session is still selected. In a same-key reconnect, a stale fork from the retired client can therefore navigate the current pane to an old connection's branch; capture the pane connection scope and re-check it after the await before persisting or switching, like the other async pane actions do.
Useful? React with 👍 / 👎.
c701358 to
c0ea53e
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c0ea53ea73
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ? inheritSessionSelection(params.currentEntry) | ||
| : params.currentEntry; | ||
| return { | ||
| ...baseEntry, |
There was a problem hiding this comment.
Clear pending delivery state on rewind
When rewinding a channel-backed session that still has a durable pendingFinalDelivery* retry from an unsettled final reply, this spread preserves that stale delivery marker while the transcript is rotated to an earlier leaf. The pending-final machinery treats those fields as a live retry (src/config/sessions/types.ts, src/auto-reply/reply/pending-final-delivery.ts), so a later recovery/settlement can send the old final answer after the user rewound and edited the prompt; clear the pending-final fields with the other tail/runtime state for rewind.
Useful? React with 👍 / 👎.
|
QA Smoke CI failure provenance: pre-existing on The four failing QA Smoke profiles all report the startup CSS budget violation (
So a fresh dependency install currently puts |
c0ea53e to
3132203
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3132203455
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const baseEntry = params.forked | ||
| ? inheritSessionSelection(params.currentEntry) | ||
| : params.currentEntry; |
There was a problem hiding this comment.
Preserve workspace metadata for message forks
When the source session is bound to a dashboard cwd or managed worktree, this uses inheritSessionSelection() as the fork base, which keeps only model/provider selection and drops spawnedCwd/worktree. Later agent runs derive workspace and cwd from the session entry (agent-run-execution-phase.ts / resolveSessionRuntimeCwd), so a message fork from a worktree session will run in the default workspace instead of the branch the user forked from. Preserve or explicitly clone the workspace binding for forked message branches.
Useful? React with 👍 / 👎.
| if (!visibleSessionMatches(state, sessionKey, agentParams.agentId)) { | ||
| return null; | ||
| } | ||
| state.chatMessages = []; |
There was a problem hiding this comment.
Clear stale reply targets after rewind
If the user has a reply target selected and rewinds to before that message, this reloads the transcript but leaves state.chatReplyTarget intact. The next send still runs through prependReplyQuote() in chat-send.ts, so the edited prompt can quote a message that is no longer on the active transcript; clear the reply target along with the transcript state, like the clear-history path does.
Useful? React with 👍 / 👎.
| onRewindMessage: (entryId) => this.rewindToMessage(entryId), | ||
| onForkMessage: (entryId) => this.forkFromMessage(entryId), |
There was a problem hiding this comment.
Hide unavailable rewind and fork actions
These callbacks are passed even when the connected Gateway does not advertise sessions.rewind or sessions.fork (for example a write-only/limited operator token omits admin-only sessions.rewind, and older gateways omit both). ChatThread will still render the actions and only fail after the user clicks; gate each callback with isGatewayMethodAdvertised(...) before exposing it.
Useful? React with 👍 / 👎.
|
Merged via squash.
|
…#110660) * feat(chat): rewind and fork a session from a message bubble * docs(web): document chat rewind and fork bubble actions * fix(chat): lint cleanups, protocol regen, and test splits for rewind/fork CI * fix(ui): fit chat rewind disabled styles inside the startup CSS budget
What Problem This Solves
There is no way to return a session to an earlier state from the chat. Once a user message is sent, the only recovery paths are
/clear(loses everything) or whole-session fork from the Sessions page (keeps everything, including the part you wanted to undo). Claude Code desktop's per-bubble rewind shows how useful "take that back and rephrase" is; OpenClaw's transcript DAG already supported it internally (AgentSessionTree.navigateTreesemantics) but nothing exposed it.Why This Change Was Made
leafcontrol, so the operation is a pointer move in the append-only DAG, and identity rotation fences any stale liveSessionManagerfrom clobbering the repoint with a snapshot-replace.sessions.rewind(operator.admin) andsessions.fork(operator.write) follow thesessions.compaction.branch/restorelifecycle discipline: exclusive lifecycle mutation, re-validation inside the lock, and both operations are rejected while a run is active. Sessions owned by an external agent harness (upstream-linked) are rejected with a typed error. Legacy JSONL transcript storage returns a typed unsupported error; SQLite is fully supported.inheritSessionSelectionmoved fromsession-create-service.tstosession-entry-selection.tsfor reuse rather than duplication.User Impact
User-message bubbles in the Control UI chat gain a rewind hover action (with confirm popover and a "Don't ask again" option) and right-click "Rewind to here" / "Fork from here" menu items. Both are disabled with an explanatory tooltip while the agent is working. Rewind refreshes the visible thread and puts the message text back in the composer; fork opens the new session with the composer pre-filled. Rewind moves chat context only — files edited, messages sent, and other tool side effects are not reverted.
Evidence
node scripts/run-vitest.mjs src/config/sessions/session-accessor.sqlite-message-cut.test.ts src/gateway/server-methods/sessions-rewind.test.ts— leaf repoint + editor text, active message count drops, exact fork prefix, source untouched, typed errors (missing entry, non-user entry, off-active-path, external harness, unsupported storage), busy rejection for both methods.node scripts/run-vitest.mjs ui/src/pages/chat/chat-history.test.ts ui/src/pages/chat/chat-pane.test.ts ui/src/lib/sessions/index.test.ts ui/src/pages/chat/components/chat-message.test.ts ui/src/pages/chat/chat-view.test.ts ui/src/pages/chat/chat-thread.test.ts— cache invalidation + refetch + composer refill, race handling (connection replaced, late fork, newer draft preserved), entry-id eligibility (optimistic bubbles excluded), context-menu contents and busy gating, confirm popover, tooltips.pnpm tsgo:core/tsgo:ui/tsgo:test:src/tsgo:test:uigreen; oxfmt clean;pnpm ui:i18n:baselineno drift (en-only keys per contributor flow).gpt-5.6-sol, xhigh): clean, no accepted/actionable findings.origin/main.