Backport rc Tier-1 fixes to main#1104
Merged
Merged
Conversation
📝 WalkthroughWalkthroughThis PR propagates a human-readable SSH remote command string from SSH command builders through the entire spawn pipeline (wrappers, IPC handlers, process manager, cue lifecycle) to a new "Remote Command" tile in the Process Monitor detail view. It also introduces a ChangesSSH Remote Command in Process Monitor
Terminal Restart and Persistent Terminal Lifecycle
Session List Auto-Running Filter
CSS Color Validation Refactor
Supporting Migrations and Cleanup
Sequence DiagramssequenceDiagram
rect rgba(100, 149, 237, 0.5)
Note over buildSshCommand,wrapSpawnWithSsh: SSH Builder Layer
buildSshCommand->>buildSshCommand: buildShellCommand(cmd, args) → remoteCommandLine
buildSshCommand-->>wrapSpawnWithSsh: SshCommandResult { remoteCommandLine }
wrapSpawnWithSsh-->>caller: SshSpawnWrapResult { sshRemoteCommand }
end
rect rgba(144, 238, 144, 0.5)
Note over caller,ProcessDetailView: Spawn Pipeline → Renderer
caller->>ChildProcessSpawner: spawn({ sshRemoteCommand })
ChildProcessSpawner-->>IPC: ManagedProcess { sshRemoteCommand }
IPC-->>Renderer: getActiveProcesses → CueProcessEntry { sshRemoteCommand }
Renderer->>buildProcessTree: populate ProcessNode { sshRemoteCommand }
buildProcessTree-->>ProcessMonitor: ProcessNode { sshRemoteCommand }
ProcessMonitor->>ProcessDetailView: detailView { sshRemoteCommand }
ProcessDetailView->>ProcessDetailView: render "Remote Command" tile
end
stateDiagram-v2
[*] --> Spawning: spawnPtyForTab
Spawning --> Running: PTY spawned
Spawning --> SpawnFailed: error
SpawnFailed --> ExitedPersistent: isPersistent → mark exited + write restart notice
SpawnFailed --> Closed: isScratch → closeTerminalTab('pty-exit')
Running --> PTYExited: PTY exit event
PTYExited --> ExitedPersistent: isPersistent → write restart notice
PTYExited --> Closed: isScratch + rapid exit → toast + closeTerminalTab('pty-exit')
PTYExited --> Closed: isScratch + normal exit → closeTerminalTab('pty-exit')
ExitedPersistent --> Spawning: restartTerminalTab (kill PTY + reset state)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
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 docstrings
🧪 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 backports four isolated fixes from
Confidence Score: 4/5Safe to merge; all three source fixes are self-contained and the full test suite passes. The only substantive concern is that The terminal-tab and left-bar fixes are straightforward and well-tested. The SSH remote-command feature threads a new optional string field through many layers without touching existing behavior. The one concrete issue is the src/main/utils/ssh-command-builder.ts - the Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[PTY process exits] --> B{tab.pid != 0?}
B -->|No - spawn failure already handled| Z[Skip duplicate handling]
B -->|Yes| C{startupCommand OR isRemoteSession?}
C -->|Yes - persistent| E[Write exit notice to terminal\nKeep tab for restart]
C -->|No - scratch tab| F[closeTerminalTab with reason=pty-exit]
A --> G{age less than 2s?}
G -->|Yes| H[notifySpawnFailure toast]
J[User clicks Restart button] --> K[restartTerminalTab in tabStore]
K --> L[Kill any lingering PTY]
K --> M[Reset tab: pid=0, state=idle]
M --> N[Select tab, set inputMode=terminal]
N --> O[spawn effect fires because pid===0 and state is idle]
O --> P[spawnPtyForTab called]
P --> Q{Spawn success?}
Q -->|No and persistent| R[handleSpawnFailure: mark exited, write message]
Q -->|No and scratch| S[closeTerminalTab with reason=spawn-failure]
Q -->|Yes| T[onTabPidChange with real PID]
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
A[PTY process exits] --> B{tab.pid != 0?}
B -->|No - spawn failure already handled| Z[Skip duplicate handling]
B -->|Yes| C{startupCommand OR isRemoteSession?}
C -->|Yes - persistent| E[Write exit notice to terminal\nKeep tab for restart]
C -->|No - scratch tab| F[closeTerminalTab with reason=pty-exit]
A --> G{age less than 2s?}
G -->|Yes| H[notifySpawnFailure toast]
J[User clicks Restart button] --> K[restartTerminalTab in tabStore]
K --> L[Kill any lingering PTY]
K --> M[Reset tab: pid=0, state=idle]
M --> N[Select tab, set inputMode=terminal]
N --> O[spawn effect fires because pid===0 and state is idle]
O --> P[spawnPtyForTab called]
P --> Q{Spawn success?}
Q -->|No and persistent| R[handleSpawnFailure: mark exited, write message]
Q -->|No and scratch| S[closeTerminalTab with reason=spawn-failure]
Q -->|Yes| T[onTabPidChange with real PID]
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+383
to
+385
| {/* Remote Command — full width. Shown only for SSH spawns: the actual agent | ||
| invocation running on the remote host, displayed above the local SSH | ||
| command line below so it's clear what is executing remotely. */} |
There was a problem hiding this comment.
Both the new "Remote Command" comment (line 383) and the edited "Command Line" comment (line 414) contain — (em-dash). CLAUDE.md requires: "NEVER use em-dashes anywhere ... If you catch an em-dash in anything you produce or edit, replace it." Replace both — with - (spaced hyphen).
Context Used: CLAUDE.md (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
![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)
src/renderer/components/SessionList/SessionList.tsx (1)
720-726:⚠️ Potential issue | 🟠 Major | ⚡ Quick winInclude auto-running worktree children in unread-only filtering.
Line 720-726 omits AUTO-state checks, so an auto-running worktree child can still disappear in unread mode even though its parent is kept visible.
Suggested fix
const worktreeChildren = showUnreadAgentsOnly ? allWorktreeChildren.filter( (child) => child.id === activeSessionId || child.aiTabs?.some((tab) => tab.hasUnread) || - child.state === 'busy' + child.state === 'busy' || + activeBatchSessionIds.includes(child.id) ) : allWorktreeChildren;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/renderer/components/SessionList/SessionList.tsx` around lines 720 - 726, The filter logic in the worktreeChildren filtering block is missing a check for the 'auto' state, which causes auto-running worktree children to be hidden when showUnreadAgentsOnly is true even though their parent remains visible. Add an additional condition to the filter within the allWorktreeChildren.filter call to also include children where the state equals 'auto', alongside the existing check for child.state === 'busy'. This ensures auto-running children are retained in unread-only filtering mode.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/renderer/components/TabBar/TerminalTabItem.tsx`:
- Around line 392-398: The restart button in the TerminalTabItem component
relies only on the title attribute for accessibility, which is insufficient for
screen readers. Add an aria-label attribute to the button element with an
appropriate accessible name like "Restart terminal" to provide an explicit
accessible name alongside the existing title attribute. This ensures assistive
technologies can properly announce the button's purpose.
In `@src/renderer/stores/tabStore.ts`:
- Around line 622-632: The restartTerminalTab function resets a tab to idle and
then kills the PTY process, but if the kill operation emits an exit event after
the reset completes, the exit event listener can revert the tab back to exited
state, preventing the spawn effect from running. Add a guard mechanism (such as
a restart generation/token or one-shot exit suppression) associated with the
restarted tab/session ID to prevent stale exit events from reverting the
restart. This guard should be set when restartTerminalTab is called and checked
in the exit event listener to ignore exit events that occur during the restart
sequence.
---
Outside diff comments:
In `@src/renderer/components/SessionList/SessionList.tsx`:
- Around line 720-726: The filter logic in the worktreeChildren filtering block
is missing a check for the 'auto' state, which causes auto-running worktree
children to be hidden when showUnreadAgentsOnly is true even though their parent
remains visible. Add an additional condition to the filter within the
allWorktreeChildren.filter call to also include children where the state equals
'auto', alongside the existing check for child.state === 'busy'. This ensures
auto-running children are retained in unread-only filtering mode.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c3c2f2ac-d794-4bbb-b54b-275922936d66
📒 Files selected for processing (28)
src/__tests__/main/cue/cue-spawn-builder.test.tssrc/__tests__/main/utils/ssh-command-builder.test.tssrc/__tests__/renderer/components/ProcessMonitor/ProcessDetailView.test.tsxsrc/__tests__/renderer/components/TerminalView.test.tsxsrc/__tests__/renderer/hooks/useSessionCategories.test.tssrc/__tests__/renderer/stores/tabStore.test.tssrc/__tests__/renderer/utils/terminalTabHelpers.test.tssrc/main/cue/cue-process-lifecycle.tssrc/main/cue/cue-spawn-builder.tssrc/main/group-chat/group-chat-moderator.tssrc/main/group-chat/spawnGroupChatAgent.tssrc/main/ipc/handlers/process.tssrc/main/process-listeners/exit-listener.tssrc/main/process-manager/spawners/ChildProcessSpawner.tssrc/main/process-manager/types.tssrc/main/utils/ssh-command-builder.tssrc/main/utils/ssh-spawn-wrapper.tssrc/renderer/components/ProcessMonitor/ProcessDetailView.tsxsrc/renderer/components/ProcessMonitor/ProcessMonitor.tsxsrc/renderer/components/ProcessMonitor/processTree.tssrc/renderer/components/ProcessMonitor/types.tssrc/renderer/components/SessionList/SessionList.tsxsrc/renderer/components/TabBar/TerminalTabItem.tsxsrc/renderer/components/TerminalView.tsxsrc/renderer/hooks/session/useSessionCategories.tssrc/renderer/hooks/tabs/internal/useUnifiedTabHandlers.tssrc/renderer/stores/tabStore.tssrc/renderer/utils/terminalTabHelpers.ts
Comment on lines
+392
to
+398
| <button | ||
| onClick={handleRestartClick} | ||
| className="p-0.5 rounded hover:bg-white/10 transition-colors shrink-0" | ||
| title="Restart terminal" | ||
| > | ||
| <RotateCw className="w-3 h-3" style={{ color: theme.colors.accent }} /> | ||
| </button> |
There was a problem hiding this comment.
Add an accessible name to the restart icon button.
The new icon-only restart button should expose an explicit accessible name (do not rely only on title).
Suggested fix
{tab.state === 'exited' && (
<button
onClick={handleRestartClick}
className="p-0.5 rounded hover:bg-white/10 transition-colors shrink-0"
title="Restart terminal"
+ aria-label="Restart terminal"
>
<RotateCw className="w-3 h-3" style={{ color: theme.colors.accent }} />
</button>
)}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <button | |
| onClick={handleRestartClick} | |
| className="p-0.5 rounded hover:bg-white/10 transition-colors shrink-0" | |
| title="Restart terminal" | |
| > | |
| <RotateCw className="w-3 h-3" style={{ color: theme.colors.accent }} /> | |
| </button> | |
| <button | |
| onClick={handleRestartClick} | |
| className="p-0.5 rounded hover:bg-white/10 transition-colors shrink-0" | |
| title="Restart terminal" | |
| aria-label="Restart terminal" | |
| > | |
| <RotateCw className="w-3 h-3" style={{ color: theme.colors.accent }} /> | |
| </button> |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/renderer/components/TabBar/TerminalTabItem.tsx` around lines 392 - 398,
The restart button in the TerminalTabItem component relies only on the title
attribute for accessibility, which is insufficient for screen readers. Add an
aria-label attribute to the button element with an appropriate accessible name
like "Restart terminal" to provide an explicit accessible name alongside the
existing title attribute. This ensures assistive technologies can properly
announce the button's purpose.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/__tests__/shared/cssColor.test.ts (1)
24-34: ⚡ Quick winAdd malformed functional-notation regression tests.
The suite should include explicit invalid
rgb()/hsl()shapes to lock the rejection behavior and prevent permissive regex regressions.Suggested test additions
describe('functional notations', () => { it('accepts rgb() and rgba()', () => { expect(isValidCssColor('rgb(26, 26, 46)')).toBe(true); expect(isValidCssColor('rgba(189, 147, 249, 0.2)')).toBe(true); }); it('accepts hsl() and hsla()', () => { expect(isValidCssColor('hsl(262, 83%, 58%)')).toBe(true); expect(isValidCssColor('hsla(262, 83%, 58%, 0.5)')).toBe(true); }); + + it('rejects malformed functional notation', () => { + expect(isValidCssColor('rgb(,,)')).toBe(false); + expect(isValidCssColor('rgba(255, 255)')).toBe(false); + expect(isValidCssColor('hsl(eee)')).toBe(false); + }); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/__tests__/shared/cssColor.test.ts` around lines 24 - 34, Add regression tests within the 'functional notations' describe block to verify that isValidCssColor properly rejects malformed rgb()/rgba()/hsl()/hsla() formats. Create additional test cases that check invalid variations such as rgb values outside the 0-255 range, missing parameters, incorrect separator usage, invalid alpha values, malformed percentage syntax, and other common mistakes. These tests should use expect().toBe(false) to ensure the validation function correctly rejects these invalid formats and prevent future permissive regex regressions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/shared/cssColor.ts`:
- Around line 18-19: The `RGB_COLOR` and `HSL_COLOR` regex patterns are too
permissive and accept malformed strings like `rgb(,,)` and `hsl(eee)` due to
their loose character class matching with the plus quantifier. Tighten these
regex patterns to properly validate the structure of functional color notation
by requiring properly formatted numeric or percentage values (potentially with
units like deg) separated by commas or slashes, rather than just matching any
sequence of allowed characters. The patterns should reject sequences of only
commas, spaces, or invalid characters to ensure only valid color values pass
validation.
---
Nitpick comments:
In `@src/__tests__/shared/cssColor.test.ts`:
- Around line 24-34: Add regression tests within the 'functional notations'
describe block to verify that isValidCssColor properly rejects malformed
rgb()/rgba()/hsl()/hsla() formats. Create additional test cases that check
invalid variations such as rgb values outside the 0-255 range, missing
parameters, incorrect separator usage, invalid alpha values, malformed
percentage syntax, and other common mistakes. These tests should use
expect().toBe(false) to ensure the validation function correctly rejects these
invalid formats and prevent future permissive regex regressions.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 66396f79-127c-4d8c-af51-a4ab7521047f
📒 Files selected for processing (3)
src/__tests__/shared/cssColor.test.tssrc/renderer/components/CustomThemeBuilder.tsxsrc/shared/cssColor.ts
Comment on lines
+18
to
+19
| const RGB_COLOR = /^rgba?\(\s*[0-9.,%\s/]+\)$/i; | ||
| const HSL_COLOR = /^hsla?\(\s*[0-9.,%\s/deg]+\)$/i; |
There was a problem hiding this comment.
Tighten functional color regexes to reject malformed values.
RGB_COLOR/HSL_COLOR currently accept malformed strings like rgb(,,) and hsl(eee). That lets invalid imported theme files pass validation and then fail at render time.
Proposed fix
-const RGB_COLOR = /^rgba?\(\s*[0-9.,%\s/]+\)$/i;
-const HSL_COLOR = /^hsla?\(\s*[0-9.,%\s/deg]+\)$/i;
+const NUMBER = String.raw`\d+(?:\.\d+)?`;
+const PERCENT = String.raw`${NUMBER}%`;
+const ALPHA = String.raw`(?:0|1|0?\.\d+|${PERCENT})`;
+
+const RGB_CHANNEL = String.raw`\d{1,3}%?`;
+const RGB_COLOR = new RegExp(
+ `^rgba?\\(\\s*${RGB_CHANNEL}\\s*,\\s*${RGB_CHANNEL}\\s*,\\s*${RGB_CHANNEL}(?:\\s*,\\s*${ALPHA})?\\s*\\)$`,
+ 'i'
+);
+
+const HUE = String.raw`${NUMBER}(?:deg|grad|rad|turn)?`;
+const HSL_COLOR = new RegExp(
+ `^hsla?\\(\\s*${HUE}\\s*,\\s*${PERCENT}\\s*,\\s*${PERCENT}(?:\\s*,\\s*${ALPHA})?\\s*\\)$`,
+ 'i'
+);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const RGB_COLOR = /^rgba?\(\s*[0-9.,%\s/]+\)$/i; | |
| const HSL_COLOR = /^hsla?\(\s*[0-9.,%\s/deg]+\)$/i; | |
| const NUMBER = String.raw`\d+(?:\.\d+)?`; | |
| const PERCENT = String.raw`${NUMBER}%`; | |
| const ALPHA = String.raw`(?:0|1|0?\.\d+|${PERCENT})`; | |
| const RGB_CHANNEL = String.raw`\d{1,3}%?`; | |
| const RGB_COLOR = new RegExp( | |
| `^rgba?\\(\\s*${RGB_CHANNEL}\\s*,\\s*${RGB_CHANNEL}\\s*,\\s*${RGB_CHANNEL}(?:\\s*,\\s*${ALPHA})?\\s*\\)$`, | |
| 'i' | |
| ); | |
| const HUE = String.raw`${NUMBER}(?:deg|grad|rad|turn)?`; | |
| const HSL_COLOR = new RegExp( | |
| `^hsla?\\(\\s*${HUE}\\s*,\\s*${PERCENT}\\s*,\\s*${PERCENT}(?:\\s*,\\s*${ALPHA})?\\s*\\)$`, | |
| 'i' | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/shared/cssColor.ts` around lines 18 - 19, The `RGB_COLOR` and `HSL_COLOR`
regex patterns are too permissive and accept malformed strings like `rgb(,,)`
and `hsl(eee)` due to their loose character class matching with the plus
quantifier. Tighten these regex patterns to properly validate the structure of
functional color notation by requiring properly formatted numeric or percentage
values (potentially with units like deg) separated by commas or slashes, rather
than just matching any sequence of allowed characters. The patterns should
reject sequences of only commas, spaces, or invalid characters to ensure only
valid color values pass validation.
The spec env spreads process.env, so an ambient MAESTRO_CLAUDE_BIN (leaked when the suite runs inside a maestro/claude agent, e.g. the pre-push hook) bled into specs and tripped the 'maestro-p disabled -> undefined' assertion. Clear it per test via vi.stubEnv so the suite asserts only what the builder injects.
The unread filter only kept busy or unread-tab agents, so an AUTO-badge agent idling between prompts was dropped. Thread activeBatchSessionIds (the same source driving the AUTO badge) into useSessionCategories and treat batch-running agents and their worktree children as visible.
Surface the actual agent invocation running on an SSH remote in the Process Details modal, displayed above the local SSH wrapper (which is relabeled "SSH Command"). Previously only the opaque `ssh ... /bin/bash --norc --noprofile -s` wrapper was visible. The bare invocation is captured at the SSH command builders (buildSshCommand, buildSshCommandWithStdin) as `remoteCommandLine`, stripped of the PATH bootstrap, cd/env prefix, and exec wrapper, then threaded through ProcessConfig/ManagedProcess, getActiveProcesses, group-chat spawns, and Cue runs to ProcessDetailData. Local processes are unchanged.
…add close diagnostics Terminal tabs with a startup command or running under an SSH/remote session were silently destroyed when their PTY exited (e.g. a dropped SSH transport), discarding the tab and its config. Such "persistent" tabs are now kept as a restartable exited husk with a visible notice and a one-click restart; plain scratch shells still auto-close on exit. Adds structured logging at every terminal close and PTY-exit decision so a vanished terminal can be explained from the logs: - exit-listener.ts: "Terminal PTY process exited" at the main-process source - TerminalView.tsx: "Terminal PTY exited" with keep/close decision; spawn-failure log - tabStore.ts: TerminalCloseReason threaded through closeTerminalTab - useUnifiedTabHandlers.ts: same log shape on the bulk-close path Adds restartTerminalTab helper + store action and tests.
isValidColor relied on a jsdom DOM round-trip (new Option().style.color), which is non-deterministic across environments: it accepts valid colors locally but intermittently rejects them under load on CI, causing the CustomThemeBuilder import tests to fail (a pre-existing flake on main). Replace it with a pure, environment-independent validator in a new shared module (isValidCssColor) covering hex, rgb/rgba, hsl/hsla, and CSS named colors, plus a dedicated unit test.
…on bloat File-preview tabs stored the full file in tab.content; the viewer only renders a truncated slice and re-reads from disk on activation, so the full bytes were never needed at rest. A 500k-line .jsonl persisted 363MB in one tab, ballooning maestro-sessions.json to 531MB and OOM-ing the main process at startup (it JSON.parses the whole store on boot). prepareSessionForPersistence now drops content (and stale editContent) for any preview tab over 256KB, mirroring the existing fileTree and filePreviewHistory stripping. Small files still persist so unsaved edit state survives a restart.
Backfill the never-configured state (enableMaestroP === undefined) only. An explicit false means the user deliberately picked the API token source; forcing Adaptive Mode back on would silently revert their choice to Dynamic on the next launch. The previous !== true check wrongly caught both.
A push that only deletes refs has no new commits to lint or test. Detect the all-zero local sha git feeds on stdin and exit early so branch deletions don't trigger the full validate:push suite.
The CustomThemeBuilder import tests failed on CI (a pre-existing flake on main) for two independent reasons, both fixed here: 1. Import validation required the optional bgTitleBar color key. The ThemeColors type marks bgTitleBar as optional and the UI falls back to bgMain, but handleImport listed it in the required-key check, so any theme exported without it (and the test fixture) was rejected. It is now excluded from the required set while still being value-validated when present. 2. Color validation used a DOM round-trip (new Option().style.color), which is non-deterministic: a sibling test spies on document.createElement, and the round-trip then rejected every color when tests ran together. Replaced with a pure, environment-independent validator in a new shared module (isValidCssColor) covering hex, rgb/rgba, hsl/hsla, and CSS named colors. Adds a unit test for isValidCssColor and regression tests covering import with a missing optional bgTitleBar and an invalid bgTitleBar value.
pedramamini
force-pushed
the
backport/rc-fixes-tier1
branch
from
June 17, 2026 03:19
35e8e45 to
4b15231
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/renderer/components/TerminalView.tsx (1)
328-333:⚠️ Potential issue | 🟠 Major | ⚡ Quick winSpawn the active tab after an in-place restart.
When the already active exited tab is restarted,
restartTerminalTabkeeps the same tab ID and only resetspid/stateto0/idle; this effect won’t rerun because its deps only observeactiveTab?.id. Includepidandstateso the active tab actually respawns after restart.🐛 Proposed fix
useEffect(() => { if (!activeTab || activeTab.pid !== 0 || activeTab.state === 'exited') { return; } spawnPtyForTab(activeTab); - }, [activeTab?.id, spawnPtyForTab]); + }, [activeTab?.id, activeTab?.pid, activeTab?.state, spawnPtyForTab]);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/renderer/components/TerminalView.tsx` around lines 328 - 333, The useEffect hook in the TerminalView component has an incomplete dependency array that only watches activeTab?.id and spawnPtyForTab. When restartTerminalTab resets the pid and state properties to 0 and 'idle' while keeping the same tab ID, the effect doesn't rerun because the ID hasn't changed. Add activeTab?.pid and activeTab?.state to the dependency array so the effect will properly trigger when these properties are reset during a restart, ensuring spawnPtyForTab is called to respawn the terminal.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/renderer/hooks/utils/useDebouncedPersistence.ts`:
- Around line 168-171: The size check in the cleanedFilePreviewTabs mapping only
verifies tab.content.length against MAX_PERSISTED_PREVIEW_CONTENT, but does not
account for tab.editContent which can also be very large. Modify the condition
to also check if tab.editContent exists and its length exceeds
MAX_PERSISTED_PREVIEW_CONTENT, then clear both content and editContent when
either exceeds the size limit to prevent large session startup pressure.
---
Outside diff comments:
In `@src/renderer/components/TerminalView.tsx`:
- Around line 328-333: The useEffect hook in the TerminalView component has an
incomplete dependency array that only watches activeTab?.id and spawnPtyForTab.
When restartTerminalTab resets the pid and state properties to 0 and 'idle'
while keeping the same tab ID, the effect doesn't rerun because the ID hasn't
changed. Add activeTab?.pid and activeTab?.state to the dependency array so the
effect will properly trigger when these properties are reset during a restart,
ensuring spawnPtyForTab is called to respawn the terminal.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ebd5c54c-2601-40a6-b44a-9787ddf1a33b
📒 Files selected for processing (37)
.husky/pre-pushsrc/__tests__/helpers/mockTheme.tssrc/__tests__/main/cue/cue-spawn-builder.test.tssrc/__tests__/main/stores/migrations/adaptive-mode-default.test.tssrc/__tests__/main/utils/ssh-command-builder.test.tssrc/__tests__/renderer/components/CustomThemeBuilder.test.tsxsrc/__tests__/renderer/components/ProcessMonitor/ProcessDetailView.test.tsxsrc/__tests__/renderer/components/TerminalView.test.tsxsrc/__tests__/renderer/hooks/useSessionCategories.test.tssrc/__tests__/renderer/stores/tabStore.test.tssrc/__tests__/renderer/utils/terminalTabHelpers.test.tssrc/__tests__/shared/cssColor.test.tssrc/main/cue/cue-process-lifecycle.tssrc/main/cue/cue-spawn-builder.tssrc/main/group-chat/group-chat-moderator.tssrc/main/group-chat/spawnGroupChatAgent.tssrc/main/ipc/handlers/process.tssrc/main/process-listeners/exit-listener.tssrc/main/process-manager/spawners/ChildProcessSpawner.tssrc/main/process-manager/types.tssrc/main/stores/migrations/adaptive-mode-default.tssrc/main/utils/ssh-command-builder.tssrc/main/utils/ssh-spawn-wrapper.tssrc/renderer/components/CustomThemeBuilder.tsxsrc/renderer/components/ProcessMonitor/ProcessDetailView.tsxsrc/renderer/components/ProcessMonitor/ProcessMonitor.tsxsrc/renderer/components/ProcessMonitor/processTree.tssrc/renderer/components/ProcessMonitor/types.tssrc/renderer/components/SessionList/SessionList.tsxsrc/renderer/components/TabBar/TerminalTabItem.tsxsrc/renderer/components/TerminalView.tsxsrc/renderer/hooks/session/useSessionCategories.tssrc/renderer/hooks/tabs/internal/useUnifiedTabHandlers.tssrc/renderer/hooks/utils/useDebouncedPersistence.tssrc/renderer/stores/tabStore.tssrc/renderer/utils/terminalTabHelpers.tssrc/shared/cssColor.ts
✅ Files skipped from review due to trivial changes (2)
- src/tests/helpers/mockTheme.ts
- src/main/process-listeners/exit-listener.ts
🚧 Files skipped from review as they are similar to previous changes (18)
- src/renderer/components/ProcessMonitor/ProcessMonitor.tsx
- src/tests/shared/cssColor.test.ts
- src/renderer/components/ProcessMonitor/processTree.ts
- src/tests/renderer/utils/terminalTabHelpers.test.ts
- src/renderer/components/ProcessMonitor/ProcessDetailView.tsx
- src/tests/main/cue/cue-spawn-builder.test.ts
- src/tests/main/utils/ssh-command-builder.test.ts
- src/main/cue/cue-process-lifecycle.ts
- src/renderer/components/SessionList/SessionList.tsx
- src/renderer/hooks/session/useSessionCategories.ts
- src/main/process-manager/types.ts
- src/tests/renderer/stores/tabStore.test.ts
- src/renderer/utils/terminalTabHelpers.ts
- src/main/ipc/handlers/process.ts
- src/tests/renderer/hooks/useSessionCategories.test.ts
- src/shared/cssColor.ts
- src/renderer/stores/tabStore.ts
- src/tests/renderer/components/TerminalView.test.tsx
Comment on lines
+168
to
+171
| const cleanedFilePreviewTabs = (session.filePreviewTabs || []).map((tab) => | ||
| tab.content && tab.content.length > MAX_PERSISTED_PREVIEW_CONTENT | ||
| ? { ...tab, content: '', editContent: undefined } | ||
| : tab |
There was a problem hiding this comment.
Include editContent in the oversize guard.
At Line 169, only tab.content.length is checked. A tab can still persist a very large editContent while content is small, which bypasses this cap and can reintroduce large-session startup pressure.
Suggested fix
-const cleanedFilePreviewTabs = (session.filePreviewTabs || []).map((tab) =>
- tab.content && tab.content.length > MAX_PERSISTED_PREVIEW_CONTENT
- ? { ...tab, content: '', editContent: undefined }
- : tab
-);
+const cleanedFilePreviewTabs = (session.filePreviewTabs || []).map((tab) => {
+ const contentLength = tab.content.length;
+ const editContentLength = tab.editContent?.length ?? 0;
+ const exceedsCap =
+ contentLength > MAX_PERSISTED_PREVIEW_CONTENT ||
+ editContentLength > MAX_PERSISTED_PREVIEW_CONTENT;
+
+ return exceedsCap ? { ...tab, content: '', editContent: undefined } : tab;
+});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const cleanedFilePreviewTabs = (session.filePreviewTabs || []).map((tab) => | |
| tab.content && tab.content.length > MAX_PERSISTED_PREVIEW_CONTENT | |
| ? { ...tab, content: '', editContent: undefined } | |
| : tab | |
| const cleanedFilePreviewTabs = (session.filePreviewTabs || []).map((tab) => { | |
| const contentLength = tab.content?.length ?? 0; | |
| const editContentLength = tab.editContent?.length ?? 0; | |
| const exceedsCap = | |
| contentLength > MAX_PERSISTED_PREVIEW_CONTENT || | |
| editContentLength > MAX_PERSISTED_PREVIEW_CONTENT; | |
| return exceedsCap ? { ...tab, content: '', editContent: undefined } : tab; | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/renderer/hooks/utils/useDebouncedPersistence.ts` around lines 168 - 171,
The size check in the cleanedFilePreviewTabs mapping only verifies
tab.content.length against MAX_PERSISTED_PREVIEW_CONTENT, but does not account
for tab.editContent which can also be very large. Modify the condition to also
check if tab.editContent exists and its length exceeds
MAX_PERSISTED_PREVIEW_CONTENT, then clear both content and editContent when
either exceeds the size limit to prevent large session startup pressure.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Backports three isolated, tested bug fixes from `0.15.0-rc` to `main`, plus one supporting test-isolation fix.
What's included
Dropped during backport
Validation
Cherry-picked cleanly with no conflicts.
Summary by CodeRabbit
Release Notes
New Features
Improvements
bgTitleBar.Tests