Keep Claude session titles on terminal tabs#6971
Conversation
|
Ready to review this PR? Stage has broken it down into 12 individual chapters for you: Chapters generated by Stage for commit b7b88f4 on Jul 1, 2026 2:15am UTC. |
📝 WalkthroughWalkthroughThis change introduces title provenance tracking for terminal tabs, distinguishing "authoritative" titles derived from OSC 1 escape sequences versus "legacy" fallback titles from OSC 2/window titles. OSC parsing now returns structured title updates with a target (icon/window/both), consumed by a new reducer that determines acceptance precedence. This flows through PTY transports, pane connection logic, and a new accepted-pane-title store map, with cleanup wired into tab/worktree/project removal and hibernation. Session hydration, persistence, and runtime/mobile sync were updated to carry and respect this provenance. A separate, unrelated change refactors CodexRestartChip's store selector usage and collapse-state synchronization, adding a render test. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (8)
src/renderer/src/components/CodexRestartChip.tsx (1)
97-101: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument why collapse state is synchronized from
noticeKey.This effect/current-state split is carrying a non-obvious invariant: avoid render-time state updates while still re-expanding on notice changes. A short comment here would make the render-loop fix much harder to accidentally undo.
Suggested note
const [collapseState, setCollapseState] = useState(() => createCodexRestartOverlayCollapseState(noticeKey) ) + // Keep notice-key resets out of render so new notices re-expand cleanly + // without reintroducing render-time state updates. useEffect(() => { setCollapseState((prev) => getCodexRestartOverlayCollapseState(prev, noticeKey)) }, [noticeKey])As per coding guidelines,
**/*.{ts,tsx,js,jsx}changes driven by a non-obvious constraint should include a short comment explaining why the code behaves that way.Source: Coding guidelines
src/renderer/src/components/codex-restart-chip-render.test.tsx (1)
31-59: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a regression case that exercises the new
noticeKeypath.This test only covers the
restartNotice === undefinedbranch, so it never hits the newuseEffect(...[noticeKey])sync or the collapse-reset behavior. A second case that renders with a notice, collapses it, then swaps to a different notice would actually guard the render-loop fix.src/renderer/src/store/slices/tabs-hydration.ts (1)
76-98: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a short why-comment for the
titleSource→labelSourcebackfill.This lookup through
session.tabsByWorktreeandtab.entityIdis migration-only behavior, so the intent is hard to infer during future refactors. A 1-2 line note that this preserves terminal title provenance for older unified sessions would make the contract much safer to maintain.As per coding guidelines, “When writing or modifying code driven by a design doc or non-obvious constraint, add a comment explaining why the code behaves the way it does.”
Source: Coding guidelines
src/renderer/src/lib/session-write-subscriber.ts (1)
34-40: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument the legacy-source normalization.
Treating
undefinedas'legacy-window-fallback'is the key persistence invariant here, but that’s not obvious from the code alone. A short why-comment near the normalization would make it clear that source-only migrations must persist while decorative title-frame churn stays ignored.As per coding guidelines, “When writing or modifying code driven by a design doc or non-obvious constraint, add a comment explaining why the code behaves the way it does.”
Also applies to: 84-93
Source: Coding guidelines
src/shared/terminal-tab-title-reducer.test.ts (1)
7-27: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover the authoritative-first path too.
This only exercises blank-title handling. The reducer's main contract is that a later
windowupdate stops being accepted after the firsticon/bothtitle, and that branch is still untested here.src/shared/tab-title-resolution.ts (1)
4-46: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a comment explaining the authoritative-title precedence rule.
Inserting
authoritativeTitle/authoritativeLabelbetweencustomTitle/customLabelandquickCommandLabelis a core, non-obvious design decision of this PR (OSC 0/1 titles outrank quick-command and generated labels, but not a manual custom title; OSC 2 legacy titles rank below both). A short inline comment would help future readers understand why authoritative titles sit at this specific point in the chain rather than, e.g., beforecustomTitleor aftergeneratedTitle.📝 Suggested comment
+ // Why: OSC 0/1 (authoritative) titles should override quick-command/generated + // fallback labels but still yield to an explicit manual custom title. OSC 2 + // (legacy) titles fall through to the same priority as the raw live title. const authoritativeTitle = resolveAuthoritativeTitle(tab.title, tab.titleSource)As per coding guidelines,
**/*.{ts,tsx,js,jsx}: "When writing or modifying code driven by a design doc or non-obvious constraint, add a comment explaining why the code behaves the way it does."Source: Coding guidelines
src/renderer/src/runtime/web-session-tabs-sync.ts (1)
498-506: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCentralize the authoritative-only
titleSourcerule.This same “emit provenance only for authoritative visible titles” decision is implemented here and again in
src/renderer/src/runtime/sync-runtime-graph.ts. Keeping it inline in both paths makes the mobile publisher and the web-session applier easy to drift. Please extract a shared helper for this rule, or at minimum add a one-line why comment here if you want to keep it local. As per coding guidelines, “When writing or modifying code driven by a design doc or non-obvious constraint, add a comment explaining why the code behaves the way it does.”Source: Coding guidelines
src/renderer/src/components/terminal-pane/pane-visible-tab-title-sync.ts (1)
13-31: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a short why-comment for the precedence rule.
This helper is now the shared boundary for the non-obvious “accepted pane title wins; raw runtime title is only a legacy fallback” contract, but that rationale is only implicit in the code. A 1-2 line comment here would make future changes much safer.
Suggested edit
export function resolvePaneVisibleTabTitle( state: PaneVisibleTitleState, tabId: string, paneId: number ): PaneVisibleTabTitle | null { + // Why: accepted pane titles already passed the OSC 0/1 vs OSC 2 precedence + // reducer, so only fall back to raw runtime titles when no accepted title exists. const acceptedTitle = state.acceptedPaneTabTitlesByTabId[tabId]?.[paneId] if (acceptedTitle?.title.trim()) { return acceptedTitle }As per coding guidelines, "When writing or modifying code driven by a design doc or non-obvious constraint, add a comment explaining why the code behaves the way it does."
Source: Coding guidelines
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 0de162be-70a0-408a-836e-5bdc6255f984
📒 Files selected for processing (44)
src/renderer/src/components/CodexRestartChip.tsxsrc/renderer/src/components/codex-restart-chip-render.test.tsxsrc/renderer/src/components/terminal-pane/TerminalPane.tsxsrc/renderer/src/components/terminal-pane/pane-visible-tab-title-sync.test.tssrc/renderer/src/components/terminal-pane/pane-visible-tab-title-sync.tssrc/renderer/src/components/terminal-pane/pty-connection-types.tssrc/renderer/src/components/terminal-pane/pty-connection.test.tssrc/renderer/src/components/terminal-pane/pty-connection.tssrc/renderer/src/components/terminal-pane/pty-dispatcher.tssrc/renderer/src/components/terminal-pane/pty-transport.test.tssrc/renderer/src/components/terminal-pane/pty-transport.tssrc/renderer/src/components/terminal-pane/remote-runtime-pty-transport.test.tssrc/renderer/src/components/terminal-pane/remote-runtime-pty-transport.tssrc/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.tssrc/renderer/src/hooks/useIpcEvents.tssrc/renderer/src/lib/session-write-subscriber.test.tssrc/renderer/src/lib/session-write-subscriber.tssrc/renderer/src/runtime/sync-runtime-graph-key-reuse.test.tssrc/renderer/src/runtime/sync-runtime-graph.test.tssrc/renderer/src/runtime/sync-runtime-graph.tssrc/renderer/src/runtime/web-session-tabs-sync.test.tssrc/renderer/src/runtime/web-session-tabs-sync.tssrc/renderer/src/store/slices/repos.tssrc/renderer/src/store/slices/runtime-pane-title-sort-epoch.test.tssrc/renderer/src/store/slices/store-cascades.test.tssrc/renderer/src/store/slices/tabs-hydration.tssrc/renderer/src/store/slices/tabs.tssrc/renderer/src/store/slices/terminal-helpers.test.tssrc/renderer/src/store/slices/terminal-helpers.tssrc/renderer/src/store/slices/terminal-orphan-helpers.tssrc/renderer/src/store/slices/terminals-hydration.test.tssrc/renderer/src/store/slices/terminals.tssrc/renderer/src/store/slices/worktrees.test.tssrc/renderer/src/store/slices/worktrees.tssrc/shared/agent-detection.test.tssrc/shared/agent-detection.tssrc/shared/osc-title-extraction.tssrc/shared/runtime-types.tssrc/shared/tab-title-resolution.test.tssrc/shared/tab-title-resolution.tssrc/shared/terminal-tab-title-reducer.test.tssrc/shared/terminal-tab-title-reducer.tssrc/shared/types.tssrc/shared/workspace-session-schema.ts
Co-authored-by: Orca <help@stably.ai>
e6b6c3f to
cb9f7d7
Compare
Co-authored-by: Orca <help@stably.ai>
Co-authored-by: Orca <help@stably.ai>
This makes terminal tab labels follow Claude/iTerm2-style OSC title behavior automatically: OSC 0/1 are treated as authoritative tab titles, while OSC 2 is kept as a legacy fallback for panes that have not observed OSC 0/1. Manual custom tab titles still win.
Boundary: this does not turn OSC 2 into the primary visible tab title protocol. OSC 2 continues to update raw runtime/window-title state and only supplies visible labels for legacy producers until a pane observes OSC 0/1.
Discord context: https://discord.com/channels/1492228674081263786/1517463258641076256
Linear: STA-728
Design approach
Design review outcome
/tmp/orca-sync-terminal-titles-design.md.Implementation and deviations
terminal-tab-title-reducerand propagatedtitleSourcethrough runtime/session schemas.CodexRestartChiprender loop discovered during Electron validation. That fix is included because it blocked validating this UI path.Completeness verification
Broad app consistency
Risks, ambiguities, and non-goals
Performance audit
Code review loop
Brennan's PR process validation
Claude Visible Titlewithauthoritative-tab, while raw runtime pane title showedShell Window Title.Legacy Fallback Titlewithlegacy-window-fallback.OSC1_UI_REMOTE; raw pane title showedOSC2_UI_IGNORED.OSC2_ONLY_REMOTE_NO_PROMPTaslegacy-window-fallback.Screenshot evidence
/tmp/orca-title-validation-run/screenshots/osc1-authoritative-not-overwritten.png/tmp/orca-title-validation-run/screenshots/osc2-legacy-fallback.png/tmp/orca-title-validation-run/screenshots/manual-custom-title-wins.png/tmp/orca-title-ssh-validation-evidence-1782868350/ssh-osc1-authoritative.png/tmp/orca-title-ssh-validation-evidence-1782868350/ssh-osc2-legacy-fallback.png/tmp/orca-title-ssh-validation-evidence-1782868350/remote-and-relay-evidence.txtStatic checks and focused tests
pnpm exec vitest run --config config/vitest.config.tsfocused title/runtime batches: latest reported 20 files / 695 tests passed.pty-connection.test.tsandweb-session-tabs-sync.test.ts, 224 tests passed.pnpm build:relaypassed during SSH validation.pnpm run typecheckpassed.pnpm run lintpassed.git diff --checkpassed.Post-PR stabilization
verifyandwayland terminal input.Made with Orca 🐋