Repository navigation
fix(workspace): a restored workspace paints before its terminals reconnect - #22810
Conversation
…nnect After a restart the workspace content area stayed blank — no tab strip, no pane — until the whole startup chain (SSH reconnect, PTY reconnect, legacy worker recovery) had finished, even though the restored tab model had been in the store for seconds. Only terminal panes need that chain, because a pane binds a PTY on mount. The workbench now mounts the active workspace from its hydrated tab model, and holds its terminal tabs unadmitted (an empty admitted-tab restriction with no deferral entry) until startup restoration has published PTY ownership; the activation plan then replaces the hold. Chat, browser and editor panes mount immediately. Carried from #22293 (922c1c2), renderer terminal files only.
…ts tabs watched The startup hold was inferred from the restriction map's shape and released only when a new hold was installed, so a workspace left for no workspace mid-startup stayed mounted with no terminal panes and no background watchers. Track the held workspace explicitly, release it on any pass where it is no longer held, apply it after prune, and count held tabs as parked so their bells, titles, and completions are still observed during the window. Carried from #22293 (3fd421c), renderer terminal files only.
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. 📝 WalkthroughWalkthroughThe change separates active-worktree surface mounting from terminal-tab admission during startup. It adds startup hold state and helpers, applies the hold during cold activation, and uses held or activation-deferred tab IDs for mount selection and watcher synchronization. Visible tabs that are not admitted show a restoring placeholder. The change also adds worktree host connection state and uses it to control SSH browser preparation and editor file retries. Terminal pane host state now uses the shared host resolver, and degraded startup marks terminal restoration ready after a non-cancelled recovery pass. Priority: ⬇️ Low Merge Risk: 🔵 Low · up to After switching runtime environments, an affected browser route or editor file may remain in an error state instead of retrying. Include the environment in the connection identity before merging, or accept this bounded risk. Security Architecture ReviewSecurity architecture risk: 🟡 Moderate · up to Restored workspaces will appear sooner while terminals reconnect, and host connection changes will trigger browser and editor work. Existing target and file-owner checks limit the apparent exposure, but interrupted startup and file-read enforcement need further verification. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
Hardening Proposals
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Advanced
Run ID: 9f04e587-79b4-4588-8357-d09b350e63a2
📒 Files selected for processing (12)
src/renderer/src/components/TerminalSplitWorkspaceSurfaces.tsxsrc/renderer/src/components/terminal-cold-activation.tssrc/renderer/src/components/terminal-parked-watcher-sync-entries.test.tsxsrc/renderer/src/components/terminal-workspace-surface-ids.test.tsxsrc/renderer/src/components/terminal/background-terminal-worktree-mount.test.tssrc/renderer/src/components/terminal/background-terminal-worktree-mount.tssrc/renderer/src/components/terminal/cold-activation-deferral-stranding.test.tssrc/renderer/src/components/terminal/initial-terminal-structured-launch.test.tsxsrc/renderer/src/components/terminal/startup-terminal-tab-hold.test.tssrc/renderer/src/components/terminal/startup-terminal-tab-hold.tssrc/renderer/src/components/use-terminal-parking-foundation.tssrc/renderer/src/components/use-terminal-watcher-effects.ts
Included review availability: Your plan provides up to 10 included reviews per hour; 4 remain after this review.
…d fold the held branch A held worktree now falls through to the existing else, which already resets the activation marker.
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
ℹ️ No critical issues — one rough edge worth a look.
Reviewed changes
- Split the startup gate from surface mounting:
canMountTerminalWorkspaceForStartup→canAdmitTerminalTabsForStartup, so the workspace surface mounts from the hydrated tab model while terminal panes stay fenced. - New startup hold (
startup-terminal-tab-hold.ts): installs an empty restriction for the active workspace, tracks it explicitly, releases it on switch-away / no-workspace / gate-open, and preserves a hold widened by a targeted background mount. - Held tabs treated as parked:
selectParkedEquivalentMountTabIdsfeeds both the surface prop and the top-level watcher effect so held tabs keep watcher coverage. - Carried from #22293:
activationDeferralPlanRevisionas an admission-effect dep, plus the updated mount/watcher/cold-activation tests.
ℹ️ A held workspace mounted without an effective layout can fall out of watcher coverage
holdTerminalTabsForStartup adds the held workspace to mountedWorktreeIdsRef unconditionally, but the two watcher-coverage paths assume a mounted workspace either has an effective layout (the pane layer covers it) or is not mounted at all. A held workspace that is mounted, has no effective layout, while some other mounted workspace does have one produces a sync entry with an empty parked set and no deferred set; because TerminalSplitWorkspaceSurfaces skips layout-less workspaces and TerminalLegacyWorkspaceSurface is suppressed while any mounted workspace has a layout, no pane layer runs for it. Its held tabs then have neither a pane nor a watcher, and syncParkedTerminalTabWatchersForWorkspaces disposes any watcher a previous pass had started.
Technical details
# Held workspace mounted without a layout loses watcher coverage
## Affected sites
- `src/renderer/src/components/terminal/startup-terminal-tab-hold.ts:56` — adds the held workspace to `mountedWorktreeIdsRef` with no layout precondition.
- `src/renderer/src/components/use-terminal-watcher-effects.ts:93-99` — early `continue` when `anyMountedWorktreeHasLayout && mounted.has(workspaceId) && getEffectiveLayoutForWorktree(workspaceId)`.
- `src/renderer/src/components/use-terminal-watcher-effects.ts:103` — mounted branch only runs when `!anyMountedWorktreeHasLayout`.
- `src/renderer/src/components/use-terminal-watcher-effects.ts:143` — background branch only runs when `!mounted.has(workspaceId)`.
- `src/renderer/src/components/TerminalSplitWorkspaceSurfaces.tsx:49-51` — layout-less workspaces are skipped.
- `src/renderer/src/components/TerminalLegacyWorkspaceSurface.tsx:11` — legacy panes suppressed while any mounted workspace has a layout.
## Required outcome
- While a hold is active, a held workspace's tabs must be covered by a parked watcher (or a mounted pane) in every pass, independent of whether that workspace currently has an effective layout.
## Open questions for the human
- Is a held workspace without an effective layout actually reachable? Groups are hydrated with the tab model and `ensureWorktreeRootGroup` runs in an effect, so the window may be a single commit; I could not construct an end-to-end startup sequence where another mounted workspace already has a layout during it. If it is reachable, the mounted branch at `use-terminal-watcher-effects.ts:103` could also run for a held workspace whose own `getEffectiveLayoutForWorktree` is falsy.ℹ️ Nitpicks
- The new
terminal-parked-watcher-sync-entriescase "watches the tabs a startup hold keeps unmounted on the visible workspace" leavesgetEffectiveLayoutForWorktree: () => undefinedandanyMountedWorktreeHasLayout: false, so it exercises the no-layout branch rather than the real hold (layout present) path; the title reads as if it covers the visible-with-layout case. - Pre-existing, not a regression: a whole-worktree background mount (
requestBackgroundTerminalWorktreeMountwithouttabIds) deletes the hold's empty restriction, and once the workspace is inmountedWorktreeIdsRefthe hold cannot re-install it (background-terminal-worktree-mount.ts:116), so panes can still bind during the window. Worth a one-line caveat in the hold's comment if "nothing spawns twice" is meant literally.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
Since the prior review at 51f891f, commit f43a9bd folds the held branch and adds a surface-wiring test:
- Folded the held branch in
applyTerminalColdActivation:if (renderedActiveWorktreeId && !startupHeldWorktreeId) { …plan… } else { lastActivationWorktreeIdRef.current = null }. Behavior-preserving — the held and no-active cases both still null the ref so the gate-open pass runs the plan, and the plan body is unchanged. - New test
terminal-split-workspace-surfaces-startup-hold.test.tsx: pins thatTerminalSplitWorkspaceSurfaceshands the held workspace itsheldTabIds, a deferred workspace its deferral set, and other workspacesnull, plus the released case. Verified the new/changed suites pass (16 tests).
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
A restored SSH workspace now mounts its panes before the host reconnects. Derive one per-worktree host phase from the published SSH state, treating a target startup restoration has not dialed yet as connecting. The browser route holds its prepare until the host connects and re-derives a failed route on connect or a new connection generation; the editor shows the connecting state instead of a dropped connection and reloads once the host connects.
While startup restoration holds terminal panes unmounted, the visible tab's slot was blank. Fill the slot of a visible held tab, including one created during the hold, with a restoring state until its pane mounts.
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
Since the prior pullfrog review at f43a9bd, two commits widen the change set beyond the terminal hold:
- New
worktree-host-connection-phase.ts: derives a per-worktreelocal | connecting | connected | unavailablephase plus connection generation from the published SSH/runtime state, and addsselectRuntimeAwareSshConnectionGeneration. - Browser routes wait for a connecting SSH host (
use-ssh-workspace-browser-route.ts): a route whose host isconnectingreportspreparinginstead of probing, a fresh connection/reconnection generation re-derives a route that failed or was held, and a ready route is exempt so a reconnect does not unmount the page. - Editor file loads wait for a connecting SSH host (
useEditorPanelFileContentLoader.ts,useEditorPanelFileLoadRetry.ts): a read that fails through a still-connecting target is reclassified as owner-not-ready (machine code dropped), and the retry effect pauses while connecting, then reloads once on the connected/reconnected epoch, including re-arming an exhausted budget. - Held terminal tab placeholder (
TerminalRestoringPlaceholder.tsx,TerminalPaneOverlayLayer.tsx,TerminalLegacyTerminalPanes.tsx): the visible held tab now paints a "Restoring terminal…" slot instead of nothing in both the overlay and legacy pane layers. - Tests: new host-phase selector suite, browser-route and editor-loader/retry host-connection suites, held-placeholder rendering suite, and the accompanying i18n entries.
ℹ️ The description no longer matches the change set
The PR body still frames the change as terminal-pane-only ("the chat, browser and editor panes render immediately") and lists "SSH-hosted workspaces" under Not tested, but the two new commits change exactly that: SSH browser and editor panes now hold until the host connects, and no manual SSH startup run is recorded.
Technical details
# Stale PR description vs. SSH-host behavior changes
## Affected sites
- PR body `## ELI5` / `## What Changed` / `## Testing` — describe the change as terminal-pane-only and state "Not tested: SSH-hosted workspaces", which the two new commits contradict.
- `src/renderer/src/components/browser-pane/use-ssh-workspace-browser-route.ts:68-101` — SSH browser routes now hold at `preparing` while `phase === 'connecting'`.
- `src/renderer/src/components/editor/useEditorPanelFileLoadRetry.ts:83-137` — SSH editor loads now pause retries while connecting.
## Required outcome
- Update the ELI5 / What Changed / Testing sections to cover the SSH browser/editor gating and the held-tab placeholder, and correct the "SSH-hosted workspaces" not-tested line (or record a manual SSH run).
## Open questions for the human
- Was an SSH-hosted startup run done for the new gating, or is the unit coverage intended to stand in for it?DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
The terminal host state now reads its SSH status from the same per-worktree signal as the browser route and the editor, instead of resolving it on its own. The signal carries the resolved status (an undialed target during startup restoration reads as connecting), the owning environment, and a connection epoch that changes on every reconnect, so no pane re-derives the no-entry case or the reconnect generation. A nested target whose runtime cannot be seen is reported as unverifiable rather than unavailable.
…shared signal The shared host signal now carries the published status separately from its phase. Only the phase reads a target startup restoration has not dialed yet as connecting, so the terminal's reconnect overlay and error ownership keep reporting what the host published, while the browser and editor still wait. Tests pin that window for the terminal and pin that a host the client cannot verify never holds the browser or editor.
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
Since the prior pullfrog review at 0c40d85, commit f56bbf9 refactors the SSH host signal into one shared reading:
- One
WorktreeHostConnectionshape —worktree-host-connection-phase.tsnow returns{ phase, targetId, environmentId, status, connectedEpoch }and dropsconnectionGeneration;resolveWorktreeSshTargetbecomesresolveWorktreeHostConnection, andselectTargetStatuskeeps the pre-existing "not dialed yet reads connecting" startup fallback. unverifiablephase added — a nested runtime target whose owning runtime is unreachable or not hydrated now readsunverifiable(where it used to readunavailable). Traced every consumer: all only branch onphase === 'connecting', so the rename is behavior-neutral.- Terminal pane reads the shared status —
terminal-pane-host-state.tsuseshost.statusinstead of callingselectRuntimeAwareSshStatusdirectly, so a direct target startup restoration has not dialed yet readsconnectingrather thandisconnected. Deliberate, and pinned by a new test. - Browser route and editor retry read
connectedEpoch— both consume the shared reconnect identity instead of re-deriving it; the epoch string format changes fromconnected:${gen}to${targetId}:${gen}(internal only), and the mocks/fixture were updated to the new shape. selectRuntimeAwareSshConnectionGenerationadded and re-exported throughruntime-environment-ssh.ts.
Verified locally: the 4 affected suites (21 tests) pass, pnpm tc:web is clean, and check:code-quality:changed reports 0 new findings.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
Since the prior pullfrog review at f56bbf9, commit 5a0ac95 corrects how the shared host signal names the raw SSH status:
- Renamed
status→publishedStatusonWorktreeHostConnectionand extractedderivePhase, so the shared reading exposes both the phase (which still reads an undialed startup target asconnecting) and the raw published status. Traced the derivation againstf56bbf9: equivalent for every phase, including theenvironmentId === nullstartup fallback (local status defaults to'disconnected', nevernull, so the null short-circuit only applies to nested targets). - The terminal names the published status —
terminal-pane-host-state.tsreadshost.publishedStatusrather than the phase-derived status, so an undialed direct target reads'disconnected'(as onmain) instead of the'connecting'the prior commit briefly introduced. Deliberate, and the pane is held during startup so the overlay is only read once the gate opens. - Tests pin the divergence and the
unverifiablepath —terminal-pane-host-state.test.tsasserts phaseconnectingalongside statusdisconnected; the browser-route and editor-retry suites assert anunverifiablehost prepares / keeps the owner-not-ready poll rather than waiting asconnecting.
Verified locally at 5a0ac95: the 4 directly affected suites pass (24 tests), pnpm tc:web is clean, and check:code-quality:changed reports 0 new findings.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
…fter reconnect The degraded recovery path's successful reconnect set workspaceSessionReady but never terminalStartupRestorationReady; only its failure branches did. Every reader waiting on restoration then waited for the whole session: no fallback terminal, no structured tab sync, the activation gate timing out, and an SSH target never dialed at startup reading as connecting forever. Release the flag once the reconnect finishes, unless a newer startup pass has taken over.
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
Since the prior pullfrog review at 5a0ac95, commit 849d58d releases terminal startup restoration on a degraded boot:
- Degraded recovery publishes
terminalStartupRestorationReady(startup-degraded-recovery.ts): after the recovery path's ownreconnectPersistedTerminalsand legacy-worker recovery succeed, the flag flips so restoration readers (agent activation gate, parked-terminal watchers, structured-session-tabs sync, and the host-connection startup fallback) stop waiting. Guarded byisCancelled()so a cancelled pass cannot stomp a newer one. - New
startup-degraded-recovery.test.ts: pins the flag flip on a successful degraded reconnect and its suppression when the pass is cancelled mid-reconnect.
The fix closes a gap between the two degraded paths: the reconnectStarted branch already set the flag through forceWorkspaceSessionReady(), while the path that ran its own reconnect did not, leaving every reader gated on the flag closed for good. The new set does not touch hydrationSucceeded, so degraded no-save semantics are unchanged.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Advanced
Run ID: 78372f01-7860-4c02-8673-379121b1850e
📒 Files selected for processing (11)
src/renderer/src/components/browser-pane/use-ssh-workspace-browser-route.host-connection.test.tsxsrc/renderer/src/components/browser-pane/use-ssh-workspace-browser-route.tssrc/renderer/src/components/editor/local-host-test-fixture.tssrc/renderer/src/components/editor/useEditorPanelFileLoadRetry.host-connection.test.tsxsrc/renderer/src/components/editor/useEditorPanelFileLoadRetry.tssrc/renderer/src/components/terminal-pane/terminal-pane-host-state.test.tssrc/renderer/src/components/terminal-pane/terminal-pane-host-state.tssrc/renderer/src/lib/worktree-host-connection-phase.test.tssrc/renderer/src/lib/worktree-host-connection-phase.tssrc/renderer/src/startup/startup-degraded-recovery.test.tssrc/renderer/src/startup/startup-degraded-recovery.ts
Included review availability: Your plan provides up to 10 included reviews per hour; 2 remain after this review.
| targetId, | ||
| environmentId, | ||
| publishedStatus, | ||
| connectedEpoch: publishedStatus === 'connected' ? `${targetId}:${generation ?? ''}` : null |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
sed -n '75,120p' src/renderer/src/lib/worktree-host-connection-phase.ts
sed -n '45,150p' src/renderer/src/components/browser-pane/use-ssh-workspace-browser-route.ts
sed -n '65,155p' src/renderer/src/components/editor/useEditorPanelFileLoadRetry.ts
sed -n '50,90p' src/renderer/src/store/slices/runtime-environment-ssh-selectors.tsRepository: stablyai/orca
Length of output: 11328
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- target resolver file outline/search ---'
rg -n "getExplicitRuntimeEnvironmentIdForWorktree|isRuntimeOwnedSshTargetId|resolveWorktreeHostConnection|connectedEpoch" src/renderer/src/lib src/renderer/src/components src/renderer/src/store
printf '%s\n' '--- resolver imports and surrounding source ---'
sed -n '1,110p' src/renderer/src/lib/worktree-host-connection-phase.ts
printf '%s\n' '--- environment ownership definitions ---'
rg -n -C 6 "function getExplicitRuntimeEnvironmentIdForWorktree|const getExplicitRuntimeEnvironmentIdForWorktree|export .*getExplicitRuntimeEnvironmentIdForWorktree|runtimeEnvironmentId|environmentId.*worktree|worktree.*environment" src/renderer/src
printf '%s\n' '--- browser reset continuation ---'
sed -n '1,190p' src/renderer/src/components/browser-pane/use-ssh-workspace-browser-route.ts
printf '%s\n' '--- editor effect continuation ---'
sed -n '1,220p' src/renderer/src/components/editor/useEditorPanelFileLoadRetry.tsRepository: stablyai/orca
Length of output: 45650
🏁 Script executed:
rg -n -C 8 "getExplicitRuntimeEnvironmentIdForWorktree|connectedEpoch|selectRuntimeAwareSshConnectionGeneration" src/renderer/src && sed -n '1,120p' src/renderer/src/lib/worktree-host-connection-phase.ts && sed -n '1,190p' src/renderer/src/components/browser-pane/use-ssh-workspace-browser-route.ts && sed -n '1,180p' src/renderer/src/components/editor/useEditorPanelFileLoadRetry.tsRepository: stablyai/orca
Length of output: 42092
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- worktree runtime owner ---'
sed -n '1,180p' src/renderer/src/lib/worktree-runtime-owner.ts
printf '%s\n' '--- runtime SSH selectors and generation references ---'
rg -n -C 5 "connectionGeneration|sshStateByEnvironment|runtimeEnvironmentId|environmentId" src/renderer/src/store/slices/runtime-environment-ssh* src/renderer/src/runtime src/renderer/src/lib/worktree-runtime-owner.test.ts
printf '%s\n' '--- focused owner tests ---'
sed -n '160,380p' src/renderer/src/lib/worktree-runtime-owner.test.ts
printf '%s\n' '--- host connection tests ---'
sed -n '130,230p' src/renderer/src/lib/worktree-host-connection-phase.test.tsRepository: stablyai/orca
Length of output: 45662
🏁 Script executed:
sed -n '1,180p' src/renderer/src/lib/worktree-runtime-owner.ts; rg -n -C 5 "connectionGeneration|sshStateByEnvironment|runtimeEnvironmentId|environmentId" src/renderer/src/store/slices/runtime-environment-ssh* src/renderer/src/runtime src/renderer/src/lib/worktree-runtime-owner.test.ts; sed -n '160,380p' src/renderer/src/lib/worktree-runtime-owner.test.ts; sed -n '130,230p' src/renderer/src/lib/worktree-host-connection-phase.test.tsRepository: stablyai/orca
Length of output: 45662
🏁 Script executed:
sed -n '90,260p' src/renderer/src/store/slices/runtime-environment-ssh.ts
rg -n -C 4 "runtimeOwnerEnvironmentId" src/renderer/src/lib src/renderer/src/store src/renderer/src/runtime shared | head -n 500
sed -n '1,240p' src/renderer/src/lib/worktree-operation-route.tsRepository: stablyai/orca
Length of output: 42011
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- SSH state type ---'
rg -n -C 8 "type SshConnectionState|interface SshConnectionState|connectionGeneration" src shared 2>/dev/null | head -n 500
printf '%s\n' '--- environment SSH state producers ---'
rg -n -C 10 "setEnvironmentSshConnectionState\\(" src | head -n 500
printf '%s\n' '--- connection state construction ---'
rg -n -C 8 "connectionGeneration:" src shared 2>/dev/null | head -n 500Repository: stablyai/orca
Length of output: 42156
🏁 Script executed:
set -e
printf '%s\n' '--- SSH state type ---'
rg -n -C 8 "type SshConnectionState|interface SshConnectionState|connectionGeneration" src shared 2>/dev/null | head -n 500
printf '%s\n' '--- environment SSH state producers ---'
rg -n -C 10 "setEnvironmentSshConnectionState\\(" src | head -n 500
printf '%s\n' '--- connection state construction ---'
rg -n -C 8 "connectionGeneration:" src shared 2>/dev/null | head -n 500Repository: stablyai/orca
Length of output: 41744
🏁 Script executed:
set -e
rg -n -C 12 "export (type|interface) SshConnectionState|type SshConnectionState|interface SshConnectionState|connectionGeneration" src/main src/renderer src/preload | grep -E "ssh-types|SshConnectionState|connectionGeneration|providerEpoch" | head -n 300
rg -n -C 10 "connectionGeneration" src/main/ssh src/main/runtime src/renderer/src/ssh src/renderer/src/runtime/runtime-environment-ssh-state.ts 2>/dev/null | head -n 400Repository: stablyai/orca
Length of output: 42798
Include environmentId in connectedEpoch.
When a worktree moves from runtime environment A to B, both environments can publish the same SSH target ID and generation. The current epoch then stays unchanged. The browser does not prepare the failed route again, and the editor does not reload an exhausted owner-unreachable file.
Suggested fix
- connectedEpoch: publishedStatus === 'connected' ? `${targetId}:${generation ?? ''}` : null
+ connectedEpoch:
+ publishedStatus === 'connected'
+ ? `${environmentId ?? 'local'}:${targetId}:${generation ?? ''}`
+ : null📝 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.
| connectedEpoch: publishedStatus === 'connected' ? `${targetId}:${generation ?? ''}` : null | |
| connectedEpoch: | |
| publishedStatus === 'connected' | |
| ? `${environmentId ?? 'local'}:${targetId}:${generation ?? ''}` | |
| : null |
Review status: ready for review at
|
|
Ready to merge. This is my view as coordinator of this PR set.
|
ELI5
When you restart Orca, the workspace you were on now shows up right away: its tabs, editor, browser and chat panes. Before, the whole area stayed blank until every saved terminal had reconnected.
Terminals still wait for their saved sessions to come back, so nothing spawns twice. While they wait, a terminal tab says "Restoring terminal…" instead of sitting blank.
On a workspace hosted over SSH, the browser and editor now wait politely while the SSH connection comes up and load by themselves once it does. Before, a slow reconnect could leave the browser stuck on "SSH connection unavailable" until you clicked Retry. That also fixes the same stuck states after an SSH connection drops and comes back mid-session.
This PR carries the terminal half of #22293 onto current main, plus what live testing showed the early paint needs. #22293's chat half is left out because the native-chat redesign replaces it.
What Changed
Before: after a restart the saved tab model was in memory within a few seconds, but the workspace area painted nothing until the whole startup chain had run: SSH reconnect, reconnecting every saved terminal, and recovering legacy worker terminals. On a large profile or a slow SSH host that is a long blank screen.
After:
Mechanism, all in the renderer:
canMountTerminalWorkspaceForStartupis renamedcanAdmitTerminalTabsForStartup. It used to gate mounting the whole workspace surface; now it gates only whether terminal tabs may mount their panes.terminal-cold-activationinstalls a startup hold on the active workspace (startup-terminal-tab-hold.ts): an empty admitted-tab set, with no deferral entry, in the per-workspace restriction map that background mounts and cold-activation deferral already use. Reveal and idle admission act only on workspaces with a deferral entry, so nothing admits a held tab until the gate opens and the activation plan replaces the hold. The hold is released on any pass where that workspace is no longer the held one (switching away, or leaving for no workspace mid-startup).TerminalRestoringPlaceholderin the same slot a pane uses. Only the startup hold produces that case, and the real pane replaces it in the same render.selectWorktreeHostConnectionPhase(src/renderer/src/lib/worktree-host-connection-phase.ts):local | connecting | connected | unavailable | unverifiable, the status as published, and a connection identity that changes on every reconnect. It is derived from the SSH state already in the store; nothing new is stored. The terminal host state, the SSH browser route and the editor's file loading all read it; none re-derives SSH status itself.connecting. That ends by itself when restoration finishes. Only the phase applies this rule; the terminal keeps showing the status as published, so terminal behaviour is unchanged.unverifiable, not down. No pane waits on it.terminalStartupRestorationReadyis now released, as the other degraded outcomes already did. Without it, anything waiting on restoration waited for the whole session. That includes the new signal's "connecting" rule, the fallback terminal for an empty worktree, and structured-session tab sync.Why
The blank screen was not a slow restore; the data was already there. One flag was doing two jobs: "may the surface paint" and "may a terminal pane bind a process". Only the second needs the reconnect chain. Splitting them along the per-workspace admitted-tab set, which the code already has, keeps the process fence exactly where it was.
Painting early exposed panes that had never had to handle a host that was not ready yet. The browser route asked the SSH host once and kept the failure. The editor told the user to click Reconnect while the app was already connecting. Fixing each pane with its own check would give several answers to one question, so every pane now reads one derived signal. Each pane waits while connecting and re-derives on connect instead of keeping a failure. The same fix covers a mid-session drop, which had the same stuck states on main.
Alternatives considered:
hydrationSucceededearlier. This would let terminal panes attach before reconnect and risk duplicate shells.tab.ptyId, which startup reconnect writes later, so every attach and spawn path would have to re-derive on its arrival. That reaches much further into the riskiest code for the same outcome; the hold changes no attach code.Differences from the common pattern
In the common pattern, each terminal pane attaches to its own restored session, with its own loading and error state. Non-terminal panes that depend on a remote host wait while the connection comes up and refresh when it becomes ready.
Linked Issue
Carries the terminal half of #22293, which was reported directly: after a restart on a native chat, the workspace stayed blank until the sidebar's reveal button was clicked. No ticket was filed.
Visual Proof
Isolated dev profiles. A temporary delay before terminal reconnect (local) or a paused SSH container (SSH) makes the startup window observable.
Before (main), local, during the startup window: the workspace area is blank.
After, local, during the startup window: tabs and editor painted, and the held terminal says it is restoring (light and dark).
After, local, once the gate opens: the same shell (original PID) with its scrollback.
SSH, before the host-connection fix: the early-mounted browser stayed on "SSH connection unavailable" after SSH was back.
SSH, after: "Connecting through the SSH host" while SSH is down, then the page loads by itself.
SSH, editor while connecting: connecting copy instead of "Click Reconnect", then the file loads by itself.
SSH, host never returns: every pane ends on a truthful error with Retry.
Testing
Manual, macOS, background-launched dev builds on isolated profiles, probed over CDP.
Local, with a 25 s delay injected before terminal reconnect:
echo $$printed the original shells, and scrollback was intact. A "+" terminal spawned exactly one shell. Across four restarts the daemon held exactly one shell per terminal.SSH, against a throwaway Linux container, paused before relaunch so SSH hangs:
Automated:
pnpm tc:web,oxlint,check:code-quality:changed,check:react-doctor:changedand the localization verifiers are clean. The new string is in all six locales.Startup timing, before vs after (does the early paint delay terminals?):
f5d2ce5de7and this PR849d58d918, on a second Mac. Each build had its own profile, seeded identically: 3 workspaces, 10 terminals with 3000 lines of scrollback each, an editor tab and a browser tab. Each restart quits normally, so the terminal daemon keeps its shells, and relaunches. There were 12 restarts per build, alternated, after one discarded warm-up each. All 24 reattached to the same terminal sessions.Not tested:
Linux and Windows desktop builds; a packaged build.
Native chat on an SSH workspace (the test host had no agent CLI).
The legacy no-layout rendering path live (covered by unit tests only).
I manually tested these changes locally
Automated tests added/updated, or explained why not below
AI Disclosure
Review
Agent skill upstream boundary
docs/reference/agent-skill-sharing-upstream-boundary.mdand copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.Notes
Author X handle: @BrennanKB5
Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)