Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds end-to-end terminal title support: normalization utilities and tests, transport-level cached titles with websocket bidirectional sync and replay-suppression, a registry getTitle accessor, UI dropdown using runtime titles, and server-side session title storage and broadcast. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as Desktop UI
participant Reg as Runtime Registry
participant Trans as WS Transport
participant WS as WebSocket
participant Srv as Host Service
UI->>Reg: getTitle(terminalId, instanceId)
Reg-->>UI: title | null
UI->>Trans: connect / open
Trans->>WS: send {type:"replay", title?}
WS->>Srv: deliver replay/title on connect
Srv->>Srv: normalize & set session.title
Srv->>WS: broadcast {type:"title", title} (except source)
WS->>Trans: incoming {type:"title", title} / replay
Trans->>Trans: apply replay-suppression, update cached title
Trans->>Reg: notify runtime listeners
UI->>Trans: terminal title change (OSC9/xterm)
Trans->>Trans: normalize, suppress no-op, check replay suppression
Trans->>WS: send {type:"title", title} (if allowed)
WS->>Srv: deliver client title
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Greptile SummaryThis PR adds terminal session title tracking by using The architecture is well thought out and cleanup (dispose guards, listener unsubscription) is handled correctly throughout. Two P2 observations: (1) the renderer-side Confidence Score: 5/5Safe to merge; all findings are P2 style/design observations with no correctness or data-integrity impact. No P0 or P1 issues found. The title flash during replay is transient and self-correcting, and the duplicate normalization is a no-op for the server-message path. Core lifecycle management (dispose guards, listener cleanup, useSyncExternalStore subscribe/unsubscribe) is implemented correctly. No files require special attention; terminal-ws-transport.ts has the minor dual-source title concern noted in comments.
|
| Filename | Overview |
|---|---|
| packages/host-service/src/terminal/title-tracker.ts | New class using @xterm/headless to parse OSC 0/2 and OSC 9;3 sequences for server-side title tracking; well-structured with double-dispose guard and correct normalization. |
| packages/host-service/src/terminal/title-tracker.test.ts | Good test coverage for OSC 0, OSC 2, split-chunk parsing, OSC 9;3, and normalization; missing a test for empty/null title clearing but not critical. |
| packages/host-service/src/terminal/terminal.ts | Integrates TerminalTitleTracker into the session lifecycle; sends current title on (re)connect, broadcasts changes, and disposes correctly on exit and session dispose. |
| apps/desktop/src/renderer/lib/terminal/terminal-ws-transport.ts | Adds title field and dual title sources (server messages + renderer xterm onTitleChange); cleanup on disconnect/dispose is correct, but the renderer-side onTitleChange can produce a brief stale title flash during replay playback. |
| apps/desktop/src/renderer/lib/terminal/terminal-runtime-registry.ts | Adds getTitle accessor; straightforward delegation to the transport; correctly uses getEntry (not getOrCreateEntry) to avoid phantom entries. |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/TerminalPane/components/TerminalSessionDropdown/TerminalSessionDropdown.tsx | Uses useSyncExternalStore correctly with memoized subscribe/snapshot; title priority (runtimeTitle > session.title > "Terminal") is sensible; non-current sessions fall back to server-side session.title. |
Sequence Diagram
sequenceDiagram
participant PTY as PTY Process
participant TT as TerminalTitleTracker (host-service)
participant Session as TerminalSession
participant WS as WebSocket
participant Transport as TerminalTransport (renderer)
participant XTerm as XTerm Renderer
participant UI as TerminalSessionDropdown
Note over PTY,UI: Session active — live title change
PTY->>Session: onData(rawData)
Session->>TT: write(rawData)
TT->>TT: HeadlessTerminal.write() [async]
Session->>WS: broadcast { type:data }
WS->>Transport: message { type:data }
Transport->>XTerm: terminal.write(data)
XTerm-->>Transport: onTitleChange(new title)
Transport->>Transport: setTitle(new title)
Transport->>UI: notifyStateListeners()
TT-->>Session: onTitleChange(new title) [async]
Session->>WS: broadcast { type:title }
WS->>Transport: message { type:title }
Transport->>Transport: setTitle(new title) [no-op]
Note over PTY,UI: Client reconnects to existing session
Transport->>WS: WebSocket connect
WS->>Session: onConnect
Session->>WS: send { type:title, current }
WS->>Transport: message { type:title }
Transport->>Transport: setTitle(current)
Transport->>UI: notifyStateListeners()
Session->>WS: send { type:replay, buffer }
WS->>Transport: message { type:replay }
Transport->>XTerm: terminal.write(replayData)
XTerm-->>Transport: onTitleChange(intermediate) stale flash
Transport->>Transport: setTitle(intermediate)
XTerm-->>Transport: onTitleChange(current)
Transport->>Transport: setTitle(current)
Prompt To Fix All With AI
This is a comment left during a code review.
Path: apps/desktop/src/renderer/lib/terminal/terminal-ws-transport.ts
Line: 201-204
Comment:
**Stale title flash during replay playback**
When reconnecting to a session, the server sends the current title *before* the replay (`sendMessage(ws, { type: "title", title: ... })`), which correctly pre-populates `transport.title`. However, the `onTitleChange` listener on the renderer xterm then fires for every OSC title sequence found in the replay buffer, including historical intermediate titles (e.g., `vim` that has since exited). This temporarily overrides the correct title before the final replay sequence restores it, causing a brief stale title flash visible in the dropdown trigger.
Consider skipping `onTitleChange` updates while the replay is being processed, or only accepting title updates from the server `title` message and dropping the renderer `onTitleChange` entirely (since the server's `TerminalTitleTracker` already provides authoritative state).
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: apps/desktop/src/renderer/lib/terminal/terminal-ws-transport.ts
Line: 46-51
Comment:
**`normalizeTitle` is redundant for server-sent messages**
`normalizeTitle` is applied in the client transport to titles that arrive from the server via `{ type: "title" }` messages. The host-service already normalizes through `TerminalTitleTracker`'s own `normalizeTitle` before broadcasting, so client-side re-normalization on server-sent values is a no-op in practice. The only case that benefits is the renderer-xterm `onTitleChange` path (which produces raw, unnormalized strings).
Consider moving the `normalizeTitle` call inside the `onTitleChange` handler only, and accepting the server message value directly, to avoid confusion and make the data flow clearer.
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "Add terminal session titles" | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Kitenite
force-pushed
the
terminal-session-naming
branch
from
5a3a18b to
15e4b8a
Compare
🚀 Preview Deployment🔗 Preview Links
Preview updates automatically with new commits |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/desktop/src/renderer/lib/terminal/terminal-ws-transport.ts (1)
207-221:⚠️ Potential issue | 🟡 MinorConsider whether
disconnect()should also cleartransport.title.
disconnect()resetsconnectionState,currentUrl,_terminal, and the data subscription, but leavestransport.titlepopulated with the last value seen on the previous connection. Any UI that consumesgetTitle()(e.g. the session dropdown) will keep showing the stale title for a disconnected session. If the intent is for an explicit disconnect to put the transport back into a "fresh" state, you likely want to clear the title here as well; if stale-title-after-disconnect is intentional (e.g. so a brief reconnect doesn't blank the label), a short comment would help.♻️ Optional reset
cancelReconnect(transport); if (transport.socket) { transport.socket.close(); transport.socket = null; } transport.currentUrl = null; transport._terminal = null; transport._reconnectAttempt = 0; setConnectionState(transport, "disconnected"); transport.onDataDisposable?.dispose(); transport.onDataDisposable = null; transport.onTitleDisposable?.dispose(); transport.onTitleDisposable = null; + setTitle(transport, null);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/lib/terminal/terminal-ws-transport.ts` around lines 207 - 221, The disconnect function leaves transport.title set which causes UI consumers of getTitle() to show a stale label after disconnect; update disconnect (the function named disconnect in terminal-ws-transport.ts that manipulates transport.currentUrl, transport._terminal, transport._reconnectAttempt, and disposables) to also clear transport.title (set to null or empty string) so the transport returns to a fresh state on explicit disconnect; if preserving title across transient disconnects was intentional, add a short comment in disconnect explaining that choice instead.
🧹 Nitpick comments (3)
packages/host-service/src/terminal/terminal.ts (1)
420-426: Fragile forward-reference:titleTrackercallback closes oversessionbefore its declaration.Today this works because
TerminalTitleTracker's constructor only registers parser/event handlers and never invokesonTitleChangesynchronously — by the timepty.onDatafirst fires,sessionhas been assigned. But the callback still captures the TDZ binding, so any future change that causes the tracker to emit a title during construction (e.g. seeding from a buffer, a self-test write, etc.) would throwReferenceError: Cannot access 'session' before initialization. Worth assigning the field after construction to keep the dataflow more obvious:♻️ Avoid TDZ-binding by wiring the callback after the session object exists
- let session: TerminalSession; - const titleTracker = new TerminalTitleTracker((title) => { - session.title = title; - broadcastMessage(session, { type: "title", title }); - }); - - session = { + const session: TerminalSession = { terminalId, workspaceId, pty, sockets: new Set(), buffer: [], bufferBytes: 0, createdAt, exited: false, exitCode: 0, exitSignal: 0, listed, title: null, - titleTracker, + // Assigned immediately below; non-null by the time anyone reads it. + titleTracker: undefined as unknown as TerminalTitleTracker, shellReadyState: shellSupportsReady ? "pending" : "unsupported", shellReadyResolve, shellReadyPromise, shellReadyTimeoutId: null, scanState: createScanState(), }; + session.titleTracker = new TerminalTitleTracker((title) => { + session.title = title; + broadcastMessage(session, { type: "title", title }); + }); sessions.set(terminalId, session);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/host-service/src/terminal/terminal.ts` around lines 420 - 426, The titleTracker callback currently closes over session before session is assigned, risking a TDZ ReferenceError; fix by ensuring session is created before wiring the tracker callback: first construct the session object (the TerminalSession) and assign its properties, then instantiate TerminalTitleTracker or call a setter to register the onTitleChange callback that calls broadcastMessage(session, ...) and updates session.title; alternatively instantiate titleTracker without a callback and call titleTracker.setOnTitleChange(...) (or similar) after session exists to avoid the forward-reference.packages/host-service/src/terminal/title-tracker.ts (2)
60-60: Minor:slice(0, MAX_TITLE_LENGTH)may split a surrogate pair.
String.prototype.sliceoperates on UTF-16 code units, so a title containing emoji or other supplementary-plane characters could be cut mid-pair at index 120, leaving an unpaired surrogate that renders as�. Since the upstream sanitization already iterates code points viaArray.from, you could truncate the same way:♻️ Code-point-safe truncation
- const normalized = withoutControlCharacters.replace(/\s+/g, " ").trim(); - - if (normalized.length === 0) return null; - return normalized.slice(0, MAX_TITLE_LENGTH); + const normalized = withoutControlCharacters.replace(/\s+/g, " ").trim(); + + if (normalized.length === 0) return null; + const codePoints = Array.from(normalized); + return codePoints.length > MAX_TITLE_LENGTH + ? codePoints.slice(0, MAX_TITLE_LENGTH).join("") + : normalized;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/host-service/src/terminal/title-tracker.ts` at line 60, The current return uses normalized.slice(0, MAX_TITLE_LENGTH) which can cut UTF-16 surrogate pairs; change the truncation to be code-point-aware by converting normalized into an array of code points (e.g., via Array.from or the spread operator), slice that array to MAX_TITLE_LENGTH, and join it back before returning so emoji and other supplementary characters are not split; update the return in title-tracker (the normalized / MAX_TITLE_LENGTH logic) accordingly.
52-61: Consider extractingnormalizeTitle+MAX_TITLE_LENGTHto a shared module.The exact same
normalizeTitleimplementation andMAX_TITLE_LENGTH = 120constant are duplicated inapps/desktop/src/renderer/lib/terminal/terminal-ws-transport.ts(lines 56-69). Drift between these two copies could cause the server's broadcast title and the renderer's xterm-derived title to differ in subtle ways (e.g. one truncates at 120 and the other at 100), leading to UI inconsistencies. Consider hoisting both into@superset/shared(or similar) so server and renderer share a single source of truth for title sanitization.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/host-service/src/terminal/title-tracker.ts` around lines 52 - 61, Duplicate title sanitization logic (normalizeTitle and MAX_TITLE_LENGTH = 120) exists in title-tracker.ts and terminal-ws-transport.ts; extract both into a shared module (e.g., `@superset/shared/terminalTitle`) that exports MAX_TITLE_LENGTH and normalizeTitle, replace the local implementations in TitleTracker (normalizeTitle) and the renderer transport with imports from that module, keep the exact behavior and export names to avoid API changes, and update any imports/usages so both server and renderer reference the single shared implementation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/host-service/package.json`:
- Line 56: The dependency "@xterm/headless" in package.json is pinned to an
outdated beta ("6.1.0-beta.197"); update the version string to either the stable
"6.0.0" (preferred for a long-lived daemon) or to the latest beta
"6.1.0-beta.203" if you need beta features—edit the "@xterm/headless" entry to
the chosen version and run your install/lockfile update (e.g., npm/yarn/pnpm
install) to refresh lockfile and ensure tests/build succeed.
In `@packages/host-service/src/terminal/terminal.ts`:
- Around line 631-632: Summary: Sending the authoritative title via
sendMessage(...) before replayBuffer(...) causes renderer flicker because
replayed OSC title sequences re-fire onTitleChange and overwrite the displayed
title briefly. Fix: move the sendMessage(ws, { type: "title", title:
existing.title }) call to after replayBuffer(existing, ws) so the authoritative
title is sent last (or alternatively add a replay-in-flight flag that suppresses
renderer onTitleChange until replayBuffer completes); update references in code
to sendMessage and replayBuffer (and transport.title/onTitleChange handlers)
accordingly so the final title wins the race.
---
Outside diff comments:
In `@apps/desktop/src/renderer/lib/terminal/terminal-ws-transport.ts`:
- Around line 207-221: The disconnect function leaves transport.title set which
causes UI consumers of getTitle() to show a stale label after disconnect; update
disconnect (the function named disconnect in terminal-ws-transport.ts that
manipulates transport.currentUrl, transport._terminal,
transport._reconnectAttempt, and disposables) to also clear transport.title (set
to null or empty string) so the transport returns to a fresh state on explicit
disconnect; if preserving title across transient disconnects was intentional,
add a short comment in disconnect explaining that choice instead.
---
Nitpick comments:
In `@packages/host-service/src/terminal/terminal.ts`:
- Around line 420-426: The titleTracker callback currently closes over session
before session is assigned, risking a TDZ ReferenceError; fix by ensuring
session is created before wiring the tracker callback: first construct the
session object (the TerminalSession) and assign its properties, then instantiate
TerminalTitleTracker or call a setter to register the onTitleChange callback
that calls broadcastMessage(session, ...) and updates session.title;
alternatively instantiate titleTracker without a callback and call
titleTracker.setOnTitleChange(...) (or similar) after session exists to avoid
the forward-reference.
In `@packages/host-service/src/terminal/title-tracker.ts`:
- Line 60: The current return uses normalized.slice(0, MAX_TITLE_LENGTH) which
can cut UTF-16 surrogate pairs; change the truncation to be code-point-aware by
converting normalized into an array of code points (e.g., via Array.from or the
spread operator), slice that array to MAX_TITLE_LENGTH, and join it back before
returning so emoji and other supplementary characters are not split; update the
return in title-tracker (the normalized / MAX_TITLE_LENGTH logic) accordingly.
- Around line 52-61: Duplicate title sanitization logic (normalizeTitle and
MAX_TITLE_LENGTH = 120) exists in title-tracker.ts and terminal-ws-transport.ts;
extract both into a shared module (e.g., `@superset/shared/terminalTitle`) that
exports MAX_TITLE_LENGTH and normalizeTitle, replace the local implementations
in TitleTracker (normalizeTitle) and the renderer transport with imports from
that module, keep the exact behavior and export names to avoid API changes, and
update any imports/usages so both server and renderer reference the single
shared implementation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 86e089ec-ea9e-45ca-8440-151651f8b0cd
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (7)
apps/desktop/src/renderer/lib/terminal/terminal-runtime-registry.tsapps/desktop/src/renderer/lib/terminal/terminal-ws-transport.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/TerminalPane/components/TerminalSessionDropdown/TerminalSessionDropdown.tsxpackages/host-service/package.jsonpackages/host-service/src/terminal/terminal.tspackages/host-service/src/terminal/title-tracker.test.tspackages/host-service/src/terminal/title-tracker.ts
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
3 issues found across 8 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/host-service/src/terminal/title-tracker.test.ts">
<violation number="1" location="packages/host-service/src/terminal/title-tracker.test.ts:8">
P2: Use xterm's completion callback instead of a fixed timeout here; otherwise the test can read `currentTitle` before parsing finishes and become flaky.</violation>
</file>
<file name="packages/host-service/src/terminal/terminal.ts">
<violation number="1" location="packages/host-service/src/terminal/terminal.ts:631">
P2: Send replay data before the explicit `title` message; otherwise buffered OSC title sequences can override the current server title on reconnect.</violation>
</file>
<file name="apps/desktop/src/renderer/lib/terminal/terminal-ws-transport.ts">
<violation number="1" location="apps/desktop/src/renderer/lib/terminal/terminal-ws-transport.ts:202">
P2: The `onTitleChange` handler fires for every OSC title sequence replayed from the buffer during reconnection, temporarily overwriting the correct server-sent title with stale historical values (e.g., from a `vim` session that has since exited). This causes a brief but visible flash of incorrect titles in the session dropdown. Consider suppressing `onTitleChange` updates while replay data is being processed, or ignoring renderer-side title changes entirely since the server's `TerminalTitleTracker` already provides the authoritative title state.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/host-service/src/terminal/terminal.ts (1)
420-445: Forward reference vialet session+ closure is OK, but worth a// session is assigned synchronously below before any tracker callback can firenote.The
TerminalTitleTrackercallback closes oversessionbefore its assignment. This is safe in practice because the tracker only emits viawrite()(driven later bypty.onData), andsession = {…}runs synchronously in this same function before any data flows. A short comment would prevent a future refactor from accidentally invoking the callback synchronously during construction (which would hit the TDZ).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/host-service/src/terminal/terminal.ts` around lines 420 - 445, Add a short clarifying comment above the TerminalTitleTracker instantiation to note that the callback closes over the yet-to-be-assigned variable session but is safe because session is synchronously assigned immediately after; reference the TerminalTitleTracker callback and the session variable (and that data emissions come later via pty.onData/write) so future refactors won't accidentally call the callback synchronously and hit the temporal dead zone.packages/host-service/src/terminal/title-tracker.ts (1)
9-25: LGTM — and a small note on thecols: 2choice.OSC handling here is well-scoped: xterm's
onTitleChangecovers OSC 0/1/2, and the manual OSC 9 handler returningfalsefor non-"3;"payloads correctly defers other ConEmu OSC 9 subcodes (progress, notify, etc.) to default xterm handling. Dedup + idempotent dispose are clean.Minor:
cols: 2will cause printable PTY bytes that flow through the tracker to wrap aggressively in the headless buffer. Since OSC parsing is independent of the grid andscrollback: 0bounds memory, this is functionally fine, but a comment explaining the rationale ("only the parser is used; grid size is intentionally minimal") would help future readers avoid second-guessing the dimensions.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/host-service/src/terminal/title-tracker.ts` around lines 9 - 25, Add a brief inline comment near the HeadlessTerminal construction (in the constructor of TitleTracker) explaining why cols: 2 and rows: 1 are chosen—e.g., that only the parser is used so the grid is intentionally minimal and scrollback: 0 bounds memory—to avoid future confusion about aggressive wrapping; reference the HeadlessTerminal instantiation and the parser.registerOscHandler(9, ...) usage so reviewers see the link between minimal grid sizing and OSC-only parsing.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/shared/src/terminal-title.ts`:
- Line 15: The final UTF-16 slice can split a surrogate pair; replace the
unit-based truncation on normalized (the return using normalized.slice(0,
MAX_TERMINAL_TITLE_LENGTH)) with a code-point-aware truncation: convert
normalized into an array of code points (e.g., via Array.from or spread), take
the first MAX_TERMINAL_TITLE_LENGTH items and join them back before returning so
surrogate pairs (and non‑BMP characters) are not broken — update the return site
that currently references normalized.slice and keep the
MAX_TERMINAL_TITLE_LENGTH constant and variable names as-is.
---
Nitpick comments:
In `@packages/host-service/src/terminal/terminal.ts`:
- Around line 420-445: Add a short clarifying comment above the
TerminalTitleTracker instantiation to note that the callback closes over the
yet-to-be-assigned variable session but is safe because session is synchronously
assigned immediately after; reference the TerminalTitleTracker callback and the
session variable (and that data emissions come later via pty.onData/write) so
future refactors won't accidentally call the callback synchronously and hit the
temporal dead zone.
In `@packages/host-service/src/terminal/title-tracker.ts`:
- Around line 9-25: Add a brief inline comment near the HeadlessTerminal
construction (in the constructor of TitleTracker) explaining why cols: 2 and
rows: 1 are chosen—e.g., that only the parser is used so the grid is
intentionally minimal and scrollback: 0 bounds memory—to avoid future confusion
about aggressive wrapping; reference the HeadlessTerminal instantiation and the
parser.registerOscHandler(9, ...) usage so reviewers see the link between
minimal grid sizing and OSC-only parsing.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 119e1a87-efa5-4616-8e3d-77cb226fb63f
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (9)
apps/desktop/src/renderer/lib/terminal/terminal-runtime-registry.tsapps/desktop/src/renderer/lib/terminal/terminal-ws-transport.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/TerminalPane/components/TerminalSessionDropdown/TerminalSessionDropdown.tsxpackages/host-service/package.jsonpackages/host-service/src/terminal/terminal.tspackages/host-service/src/terminal/title-tracker.test.tspackages/host-service/src/terminal/title-tracker.tspackages/shared/package.jsonpackages/shared/src/terminal-title.ts
✅ Files skipped from review due to trivial changes (3)
- packages/shared/package.json
- packages/host-service/package.json
- packages/host-service/src/terminal/title-tracker.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/TerminalPane/components/TerminalSessionDropdown/TerminalSessionDropdown.tsx
# Conflicts: # apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/TerminalPane/components/TerminalSessionDropdown/TerminalSessionDropdown.tsx
There was a problem hiding this comment.
1 issue found across 8 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/TerminalPane/components/TerminalSessionDropdown/TerminalSessionDropdown.tsx">
<violation number="1">
P2: Non-current sessions now always fall back to "Terminal" when no pane location exists, which drops per-session titles for detached sessions in the session dropdown.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
2 issues found across 8 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/desktop/src/renderer/lib/terminal/terminal-ws-transport.ts">
<violation number="1" location="apps/desktop/src/renderer/lib/terminal/terminal-ws-transport.ts:194">
P2: Resetting `transport.title` on disconnect bypasses `titleListeners`, so title subscribers may not re-render and can show stale session titles.</violation>
</file>
<file name="packages/shared/src/terminal-title-scanner.ts">
<violation number="1" location="packages/shared/src/terminal-title-scanner.ts:99">
P2: Byte-limit guard uses `string.length` instead of actual byte length, so oversized non-ASCII OSC payloads can bypass the intended cap.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
|
You're iterating quickly on this pull request. To help protect your rate limits, cubic has paused automatic reviews on new pushes for now—when you're ready for another review, comment |
1 participant
Summary
@superset/shared/terminal-titleso host and renderer use the same display rules.Why renderer xterm is the right path
onTitleChangeinstead of scanning terminal bytes themselves.listSessions, but detached sessions do not incur continuous parser CPU while no renderer is attached.9;3TUI naming tokens are handled through the renderer xterm parser hook, not a custom raw-byte scanner.Performance notes
@xterm/headlessdependency or host-side xterm parser was added for this feature.Validation
bun test packages/shared/src/terminal-title.test.ts packages/host-service/src/terminal/env.test.tsbun run --cwd packages/shared typecheckbun run --cwd packages/host-service typecheckbun run --cwd apps/desktop typecheckSummary by CodeRabbit
New Features
Tests