Repository navigation
fix(workspace): a restored workspace paints before its terminals reconnect - #22293
brennanb2025 wants to merge 9 commits into
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. The host also made restored chats readable only when a client first asked for the tab inventory — a reply that itself waits on the renderer's runtime graph, which is gated on the same terminal reconnect. The readable sweep now starts from startup restoration behind a once-latch, still off the terminal-safety fence, and the tab projection awaits the same sweep. A stream failure payload is reported by its message instead of `[object Object]`.
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 10 included reviews per hour; 5 remain after this review. 📝 WalkthroughWalkthroughStructured session reads now make persisted chats readable on demand before Priority: ➖ Normal Merge Risk: 🟡 Moderate · up to A targeted terminal request can mount a restored pane before ownership is published, while held tabs in mounted layouts miss watcher updates. Resolve these startup issues before merging. 🚥 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: 48a5f210-7d98-45eb-b20c-d2b9eaf15599
📒 Files selected for processing (11)
src/main/runtime/orca-runtime-resolve-recovered-structured-tui-transcript.tssrc/main/runtime/orca-runtime-restore-structured-agent-session-tabs-once.tssrc/main/runtime/orca-runtime-runtime-id.tssrc/main/runtime/orca-runtime-structured-session-restore.test.tssrc/renderer/src/components/native-chat/structured-agent-session-read-transport.test.tssrc/renderer/src/components/native-chat/structured-agent-session-read-transport.tssrc/renderer/src/components/terminal-cold-activation.tssrc/renderer/src/components/terminal/background-terminal-worktree-mount.test.tssrc/renderer/src/components/terminal/background-terminal-worktree-mount.tssrc/renderer/src/components/terminal/startup-terminal-tab-hold.test.tssrc/renderer/src/components/terminal/startup-terminal-tab-hold.ts
Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review.
| mountedWorktreeIds.delete(heldWorktreeId) | ||
| } | ||
| } | ||
| applyBackgroundMountTabRestriction(restrictions, mountedWorktreeIds, worktreeId, []) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Do not widen the startup hold with an existing restriction.
applyBackgroundMountTabRestriction(..., []) preserves all tab IDs in an existing targeted restriction. If a targeted background mount admits a restored tab first, that terminal pane can mount while the startup gate is closed. The pane can then bind the PTY before restoration publishes ownership.
Replace the restriction with an empty set. Update the test at Line 218 that currently requires the unsafe preservation behavior.
Proposed fix
- applyBackgroundMountTabRestriction(restrictions, mountedWorktreeIds, worktreeId, [])
+ restrictions.set(worktreeId, new Set())📝 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.
| applyBackgroundMountTabRestriction(restrictions, mountedWorktreeIds, worktreeId, []) | |
| restrictions.set(worktreeId, new Set()) |
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
ℹ️ One scope note inline — the fix is sound for restored workspaces; nothing blocking.
Reviewed changes
- Renderer: surface mounts, terminals held.
canMountTerminalWorkspaceForStartupis renamed tocanAdmitTerminalTabsForStartupand now fences only terminal-tab admission. While closed,applyTerminalColdActivationmounts the active surface and installs a startup hold (newterminal/startup-terminal-tab-hold.ts: an empty admitted-tab restriction with no deferral entry) that only the gate-open activation plan replaces. - Main: chat readability starts at startup.
prepareStructuredAgentSessionStartupRestorationOncefires the readable-journal sweep behind a once-latch (not awaited), andrestoreStructuredAgentSessionTabsOnceawaits the same latch before projecting tabs, so readability no longer waits on the client's tab inventory. - Stream failure text.
describeReadFailuremaps a raw{ code, message }RPC payload to its message instead of[object Object]. - Tests. New hold suite exercises surface-mount-before-gate, worktree switch, no-active-worktree, and the hold/prune/reveal invariants; restore and read-transport tests updated.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
| ) { | ||
| // Why null: the gate opening must run the activation plan, which replaces the hold. | ||
| lastActivationWorktreeIdRef.current = null | ||
| holdTerminalTabsForStartup( |
There was a problem hiding this comment.
An untargeted whole-worktree background mount for the active worktree during this same startup window lifts the hold, so restored panes can still bind PTYs before ownership is published. This predates the change — background mounts add to mountedWorktreeIds independently of cold activation — so it is not a regression, but the new hold does not cover that path. Worth deciding whether it should.
Technical details
# Untargeted background mount bypasses the startup hold
## Affected sites
- `src/renderer/src/components/terminal-cold-activation.ts:58` — installs the hold, but the next closed-gate render cannot reinstall it
- `src/renderer/src/components/terminal/background-terminal-worktree-mount.ts:119-121` — `applyBackgroundMountTabRestriction(..., undefined)` hits `if (!tabIds) { restrictions.delete(worktreeId); return }`, deleting the hold
- `src/renderer/src/components/terminal/background-terminal-worktree-visibility.ts:24` — `addBackgroundMountedTerminalWorktree` then puts the worktree in `mountedWorktreeIds`, so every tab mounts (`backgroundMountTabIds == null`)
- `background-terminal-worktree-mount.ts:116-118` — the follow-up hold install early-returns on `mounted.has(W) && !existing`, so the hold is unrecoverable for the rest of the window
- Callers that pass no `tabIds`: `src/renderer/src/hooks/ipc-events/browser-automation-bootstrap-lease.ts:55` (falls back to `activeWorktreeId`), `src/renderer/src/lib/wake-sleeping-agents-in-background.ts` untargetable records
## Required outcome
- Either the startup hold survives an untargeted whole-worktree mount of the active worktree until the gate opens, or the limitation is stated where the hold is documented.
## Open questions for the human (optional)
- Is a whole-worktree background mount during the startup window intentionally exempt from the PTY fence (the caller wants everything mounted now), or should it be narrowed to a targeted mount until the gate opens?…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. Gate the early readable sweep on the structured-chat setting, as the tab projection already is, and rely on the host's own once-per-process latch instead of a second runtime-level latch that latched success when no host existed.
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: bac347a6-b638-48e5-b5bd-80404f3ebe31
📒 Files selected for processing (12)
src/main/runtime/orca-runtime-resolve-recovered-structured-tui-transcript.tssrc/main/runtime/orca-runtime-structured-session-restore.test.tssrc/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/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; 6 remain after this review.
| } | ||
| } | ||
| deferredTabIds = activationDeferredMountTabIdsByWorktreeRef.current.get(workspaceId) ?? null | ||
| deferredTabIds = selectParkedEquivalentMountTabIds( |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Synchronize held tabs when the worktree has a layout.
When a hydrated worktree has a layout, Lines 93–99 skip it before this selector runs. Lines 103–142 also exclude it when anyMountedWorktreeHasLayout is true. Its held terminal tabs remain unmounted without parked watchers for bells, titles, or completions. Allow the held worktree through both guards. Update src/renderer/src/components/terminal-parked-watcher-sync-entries.test.tsx to exercise a mounted layout; its current fixture sets anyMountedWorktreeHasLayout to false and supplies no layout.
There was a problem hiding this comment.
ℹ️ No new issues in the follow-up commit — the changes are sound. This is not an approval because a prior review thread on unchanged code remains open.
Reviewed changes
- Startup hold tracked explicitly. Replaced the shape-inferred hold (
isStartupTerminalTabHold) with aStartupTerminalTabHoldrecord (worktreeId+heldTabIds) behindstartupTerminalTabHoldRef;holdTerminalTabsForStartupnow records the held set and reuses its identity while unchanged, and the newreleaseStartupTerminalTabHoldruns before the activation plan, deleting the restriction and unmounting only when the hold was never widened. - Held tabs kept under parked-watcher coverage.
selectParkedEquivalentMountTabIdsfeeds the held set intoTerminalSplitWorkspaceSurfaces(viauseTerminalTabColdParking) and intouse-terminal-watcher-effects, covering both the has-layout and no-layout paths, so bells, titles, and completions from held terminals still land during the startup window. - Startup readable sweep gated on structured chat. The runtime-level
structuredReadableSessionRestorePromiselatch is gone; the startupvoided sweep is now behindisStructuredNativeChatEnabled(this)and relies on the host restorer's process-wide latch that the tab projection awaits. Confirmed this still runs once per process, latches the same selection, and stays off the terminal-safety fence.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
… provider answers The sweep also recovers chats owned by an agent terminal, and it finds that terminal in the runtime's PTY census. Startup restoration takes that census after the first-window barrier, which gives up waiting on the daemon after 12s. If the daemon starts slowly, a sweep started right then misses the live terminal and records it as needing manual recovery. On main the sweep only ran after legacy recovery had waited for the provider. It now waits for the local PTY provider and takes a fresh census first. The terminal fence still does not wait on it.
There was a problem hiding this comment.
ℹ️ No critical issues — one reachability note inline.
Reviewed changes
The delta since the prior Pullfrog review (3fd421c) is the single follow-up commit 4eb6b10 ("start the startup readable sweep once the local PTY provider answers"), which closes the stale-census hazard the earlier startup sweep introduced.
- Provider-gated readable sweep.
prepareStructuredAgentSessionStartupRestorationnow takes an optionallocalPtyProviderReady; the fire-and-forget sweep waits on it, takes a freshrefreshMobileSessionPtyRecords()census, then runsrestoreReadableStructuredSessions(), keeping journal parsing off the terminal-safety fence. - Bootstrap passes the provider.
app:prepareTerminalStartupRestorationforwardsstate.localPtyProviderStartupReady, so startup restoration no longer takes the census that can predate the daemon. - Tests. The startup-restore test holds the provider open and asserts no sweep before it answers, two censuses with the second before the sweep, and a new case proving no sweep while structured chat is off.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
Replaces the early startup sweep. The first subscribe or history call on a persisted chat the user still has a tab for opens its journal on demand (read phase only, never a provider process). Handoff recovery stays in the startup sweep, which runs where it did before this branch; the sweep's per-session restore is split into the read phase and the handoff phase so a chat read first still gets its handoff re-proved exactly once.
There was a problem hiding this comment.
ℹ️ No new issues in the follow-up commit — one narrow edge note inline. Approval is withheld only because a prior thread on unchanged renderer code remains open, not for anything in this delta.
Reviewed changes
The delta since the prior pullfrog review (4eb6b10) is the single commit 8284ab2 ("open a visible chat's journal on its first read"), which replaces the provider-gated early startup sweep with an on-demand read.
- On-demand journal read.
agentSession.historyandagentSession.subscribenowawait host.ensureReadable(sessionId)before reading/binding.ensureReadableopens a persisted chat's journal for a chat the user still shows, read phase only, and refuses one the persisted visible-tab index no longer lists. - Restore split into read and handoff phases.
restoreOneStructuredAgentSessionReadis nowrestoreStructuredAgentSessionReadPhase(journal open, read only) plus a second serialized handoff step; the startup sweep runs both, the on-demand read runs the read phase alone. - Startup-sweep timing reverted. The
localPtyProviderReadyparameter, the earlyvoided sweep, and the runtimerestoreReadableStructuredSessionshelper are removed; the bootstrap and tab-projection callers are back to their base shape, so the projection is the sole sweep caller at pre-PR timing. - Tests. New
structured-agent-session-readable-on-demand.test.tsandstructured-agent-session-restart-read.test.ts(real host, store reopened from disk) cover the visible/closed/unsupported/live/absent and disabled-setting cases; the restart-restore suite covers the read-vs-handoff split. I ran them plus the fullagent-session-wiresuite: 3 new files / 16 tests and 97 files / 751 tests all pass.
I also traced the load-bearing claim that the read phase is safe before startup's PTY census: ensureReadable never reaches the census-dependent handoff recovery, never calls adapter.acquire, and retrySettlement cannot reach attachJournal because onReadable installs the journal first. The claim holds.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
…while its hold starts the provider The chat pane now mounts before startup restoration builds the structured host, so its first history read raced the pane's own hold that installs it and was refused as unsupported, painting the load error. History and subscribe now install the host as hold and reveal already do. The hold's attach keeps the session's task queue for the whole provider start, so a read arriving meanwhile waited for the child. The hold now makes the session readable before it attaches, and the read phase skips the queue once the session is readable.
There was a problem hiding this comment.
ℹ️ No new issues in this delta — approval is withheld only because two prior review threads on unchanged code remain open, not for anything in these commits.
Reviewed changes
The delta since the prior pullfrog review (8284ab2) is two commits: 52e3599 ("a restored chat reads before its host is built and while its hold starts the provider") and b7bd3af ("an on-demand read skips the task queue once the chat is readable").
- Host built on first read.
agentSession.historyandagentSession.subscribenow install the structured host beforerequireHost, so a chat pane that mounts before startup restoration has built it still reads rather than painting the load error; the gate's doc now lists history and subscribe as record-addressing methods. - A hold opens the chat before starting the provider.
resumeStructuredAgentSessionForHoldrunsmakeReadable(best effort, errors routed to the event-sink error sink) between recovery resolution and attach, so a read arriving during the provider start finds the session already readable instead of queueing on the child. - Read skips the provider-start queue.
restoreStructuredAgentSessionReadPhasere-checkshasSessionafter its reconcile/resolve awaits and returns beforeserialize, so a read that raced another surface's read does not queue behind a hold's whole provider start. - Tests. New
installed: falseand hold-in-flight read cases in the restart-read suite, a "does not queue behind a provider start" case in the on-demand suite, and the reveal expectation updated to the two-step serialization.
I traced the hold ordering: the read-opened childless session is replaced by attach, whose onAttached closes the previous journal and sets hasProviderChild: true (so exactly one provider still starts), and onAttachFailed forgets the read-opened session (so a failed attach leaks nothing). Ran the four touched suites: 4 files / 32 tests pass.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
…pen a chat closed behind it The on-demand read checked the visible-tab index, then awaited reconcile and recovery before queueing. A close landing in that window hid the tab and queued first, so the read ran after it and reopened the closed chat; the startup sweep then republished it as a tab. The check is now asked again inside the task queue.
There was a problem hiding this comment.
ℹ️ No new issues in this delta — the read-vs-close race it targets is closed cleanly. Approval is withheld only because the open renderer-hold thread on unchanged code remains, not for anything in this commit.
Reviewed changes
The delta since the prior Pullfrog review (b7bd3af) is the single commit 4b09688 ("a read already past its visibility check cannot reopen a chat closed behind it").
- Read eligibility re-checked inside the serialized read.
ensureReadablenow passes atabVisibleclosure intorestoreStructuredAgentSessionReadPhaseasstillWanted, re-asked inside the task queue (after thehasSessioncheck, before the journal open). A read whose first visibility check passed and then queued behind a close's serialized eviction now returns without reopening the chat; the startup sweep keeps the default() => trueand is unaffected. - Test. New on-demand case holds
reconcileopen, hides the tab, queues the close, then resolves reconcile; it asserts the read resolvesfalse, leaves the live map empty, and never opens the journal — red without the guard.
I traced the close ordering this fix relies on: every close path (agentSession.close, closeStructuredAgentSessionChild, discardStructuredWorkerSession) writes setSessionTabVisibility(false) before host.close, and host.close serializes its eviction on the same per-session queue, so the in-queue re-check observes the hide before the read step runs. Ran the two touched suites: 2 files / 14 tests pass. This retires the prior restorer thread (PRRT_kwDORpCz1s6k9Wyy).
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Advanced
Run ID: 9fb7095b-8138-4804-b63a-dc767211f7d8
📒 Files selected for processing (12)
src/main/native-chat/agent-session-wire/structured-agent-session-host-lifetime.tssrc/main/native-chat/agent-session-wire/structured-agent-session-host.tssrc/main/native-chat/agent-session-wire/structured-agent-session-readable-on-demand.test.tssrc/main/native-chat/agent-session-wire/structured-agent-session-readable-restorer.tssrc/main/native-chat/agent-session-wire/structured-agent-session-restart-restore.test.tssrc/main/native-chat/agent-session-wire/structured-agent-session-restart-restore.tssrc/main/native-chat/agent-session-wire/structured-agent-session-reveal.test.tssrc/main/native-chat/agent-session-wire/structured-agent-session-reveal.tssrc/main/runtime/rpc/methods/structured-agent-session-gate.tssrc/main/runtime/rpc/methods/structured-agent-session-restart-read.test.tssrc/main/runtime/rpc/methods/structured-agent-session-rpc.test-fixture.tssrc/main/runtime/rpc/methods/structured-agent-session.ts
Included review availability: Your plan provides up to 10 included reviews per hour; 3 remain after this review.
| throw new Error(unreconciled.code) | ||
| } | ||
| await context.runtimeState.resolveRecovery(sessionId) | ||
| // Why first: attach keeps the session's task queue for the whole provider start, so a read | ||
| // arriving meanwhile would wait on the child. Best effort; the attach decides the hold. | ||
| await context | ||
| .makeReadable(sessionId) | ||
| .catch((error: unknown) => context.deps.onEventSinkError?.({ sessionId, error })) | ||
| await resumeHeldStructuredAgentSession({ | ||
| sessionId, | ||
| deps: context.deps, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
sed -n '168,198p' src/main/native-chat/agent-session-wire/structured-agent-session-host-lifetime.ts
sed -n '1,75p' src/main/native-chat/agent-session-wire/structured-agent-session-hold-resume.ts
sed -n '1,80p' src/main/native-chat/agent-session-wire/structured-agent-session-resume-eligibility.ts
rg -n 'createStructuredAgentSessionHolds|resumeStructuredAgentSessionForHold|makeReadable|hold' src/main/native-chat/agent-session-wireRepository: stablyai/orca
Length of output: 41768
Do not attach when makeReadable returns false.
ensureReadable returns false when the persisted session is no longer visible. The hold path discards that result and still calls resumeHeldStructuredAgentSession, which can attach the provider for the released record. This violates the hold/close contract: a closed or hidden session can regain a provider without a readable surface.
Return after the readability check, while preserving error reporting.
Suggested fix
- await context
+ const readable = await context
.makeReadable(sessionId)
.catch((error: unknown) => context.deps.onEventSinkError?.({ sessionId, error }))
+ if (readable !== true) return📝 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.
| throw new Error(unreconciled.code) | |
| } | |
| await context.runtimeState.resolveRecovery(sessionId) | |
| // Why first: attach keeps the session's task queue for the whole provider start, so a read | |
| // arriving meanwhile would wait on the child. Best effort; the attach decides the hold. | |
| await context | |
| .makeReadable(sessionId) | |
| .catch((error: unknown) => context.deps.onEventSinkError?.({ sessionId, error })) | |
| await resumeHeldStructuredAgentSession({ | |
| sessionId, | |
| deps: context.deps, | |
| throw new Error(unreconciled.code) | |
| } | |
| await context.runtimeState.resolveRecovery(sessionId) | |
| // Why first: attach keeps the session's task queue for the whole provider start, so a read | |
| // arriving meanwhile would wait on the child. Best effort; the attach decides the hold. | |
| const readable = await context | |
| .makeReadable(sessionId) | |
| .catch((error: unknown) => context.deps.onEventSinkError?.({ sessionId, error })) | |
| if (readable !== true) return | |
| await resumeHeldStructuredAgentSession({ | |
| sessionId, | |
| deps: context.deps, |
| handler: async (params, ctx, emit) => { | ||
| await ensureHostInstalled(ctx) | ||
| const host = requireHost(ctx) | ||
| // Before the stream binds, so nothing is bound across the await. Opens the journal only; | ||
| // a chat the user still shows reads without waiting for the startup sweep. | ||
| await host.ensureReadable(params.sessionId) | ||
| const subscriptionId = subscriptionIdFor(ctx, params.sessionId) | ||
| // A live stream is a surface too: it keeps a session from being evicted while it is read and | ||
| // releases that retention when the transport dies without a word. |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
sed -n '270,310p' src/main/runtime/rpc/methods/structured-agent-session.ts
sed -n '35,70p' src/main/native-chat/agent-session-wire/structured-agent-session-background-task-channel.ts
sed -n '145,195p' src/main/runtime/rpc/rpc-streaming-dispatcher.ts
rg -n 'register|cleanup|backgroundTask|subscribe' src/main/runtime/rpc src/main/native-chat/agent-session-wireRepository: stablyai/orca
Length of output: 45539
🏁 Script executed:
set -eu
printf '%s\n' '--- structured-agent-session.ts ---'
sed -n '260,315p' src/main/runtime/rpc/methods/structured-agent-session.ts
printf '%s\n' '--- background-task-channel.ts ---'
sed -n '1,100p' src/main/native-chat/agent-session-wire/structured-agent-session-background-task-channel.ts
printf '%s\n' '--- structured-agent-session-host.ts ---'
sed -n '320,375p' src/main/native-chat/agent-session-wire/structured-agent-session-host.ts
printf '%s\n' '--- rpc-streaming-dispatcher.ts ---'
sed -n '130,210p' src/main/runtime/rpc/rpc-streaming-dispatcher.ts
printf '%s\n' '--- restart-read test ---'
sed -n '130,190p' src/main/runtime/rpc/methods/structured-agent-session-restart-read.test.ts
printf '%s\n' '--- cleanup registry definitions ---'
rg -n -A16 -B8 'registerSubscriptionCleanup|registerOwnedSubscriptionCleanup|cleanupSubscription\\(' src/main/runtime src/main | head -n 260Repository: stablyai/orca
Length of output: 11890
🏁 Script executed:
set -eu
sed -n '270,310p' src/main/runtime/rpc/methods/structured-agent-session.ts
sed -n '35,70p' src/main/native-chat/agent-session-wire/structured-agent-session-background-task-channel.ts
sed -n '340,365p' src/main/native-chat/agent-session-wire/structured-agent-session-host.ts
sed -n '150,190p' src/main/runtime/rpc/rpc-streaming-dispatcher.ts
sed -n '155,182p' src/main/runtime/rpc/methods/structured-agent-session-restart-read.test.ts
rg -n -A12 -B6 'registerSubscriptionCleanup|registerOwnedSubscriptionCleanup|cleanupSubscription\(' src/main/runtime src/main | head -n 220Repository: stablyai/orca
Length of output: 30085
🏁 Script executed:
set -eu
printf '%s\n' '--- binder definition and usages ---'
rg -n -A28 -B10 'bindStructuredAgentSessionStream' src/main
printf '%s\n' '--- subscription registry definitions ---'
rg -n -A24 -B10 'registerSubscriptionCleanup|cleanupSubscription|cleanupSubscriptionsByPrefix' src/main --glob '*.ts' | head -n 260
printf '%s\n' '--- structured session stream tests ---'
rg -n -A18 -B12 'bindStructuredAgentSessionStream|streamHolder|agentSession.subscribe' src/main/runtime/rpc/methods --glob '*structured-agent-session*test.ts' | head -n 260Repository: stablyai/orca
Length of output: 42169
Remove the stream registration when host.subscribe fails.
bindStructuredAgentSessionStream registers cleanup before host.subscribe. If ensureReadable leaves no attached session, host.subscribe calls requireSession and throws. The streaming dispatcher returns the error without releasing the registration.
The cleanup closure then remains retained and can later call host.release for a subscription that never opened. Clean up the registration in the failure path.
Suggested fix
- dispose = host.subscribe({
- id: subscriptionId,
- sessionId: params.sessionId,
- emit: (event) => emit(projectTurnItemEvent(projectBackgroundTaskEvent(event, ctx), ctx)),
- ...(params.cursor ? { cursor: params.cursor } : {})
- })
+ try {
+ dispose = host.subscribe({
+ id: subscriptionId,
+ sessionId: params.sessionId,
+ emit: (event) => emit(projectTurnItemEvent(projectBackgroundTaskEvent(event, ctx), ctx)),
+ ...(params.cursor ? { cursor: params.cursor } : {})
+ })
+ } catch (error) {
+ ctx.runtime.cleanupSubscription(subscriptionId)
+ throw error
+ }… chat's journal agentSession.history and agentSession.subscribe now call host.ensureReadable before answering. The cross-version suite drives them against its own stub host, which had no such member, so every history call failed for the harness's own reason and read as "did not reach the host".
|
Closing without merging, with Brennan's approval. The terminal half (a restored workspace paints before its terminals reconnect) continues in #22810. The chat half (opening a chat's journal on first read) is redone by the native-chat redesign, at the host's session lookup rather than per read. |
…nnect (#22810) * fix(workspace): a restored workspace paints before its terminals reconnect 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. * fix(workspace): track the startup terminal hold explicitly and keep its 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. * test(workspace): cover the split surfaces' held-tab watcher wiring and fold the held branch A held worktree now falls through to the existing else, which already resets the activation marker. * fix(workspace): browser and editor panes wait for a connecting SSH host 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. * fix(workspace): a held terminal tab shows that it is restoring 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. * refactor(workspace): every pane reads one shared host-connection signal 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. * fix(workspace): the terminal names the published SSH status from the 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. * fix(startup): a degraded boot releases terminal startup restoration after 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.
ELI5
After you restart Orca, the workspace you were on comes back on screen right away — the tab strip, the chat, the browser and editor panes — instead of a blank area that only fills in once every terminal in every workspace has finished reconnecting. Terminals themselves still wait for that reconnect before they attach, so nothing spawns twice.
What Changed
Before: on restart the session was fully restored into memory within a couple of seconds, but the workspace area painted nothing at all until the entire startup chain had run — SSH reconnect, PTY reconnect for every saved terminal, legacy worker recovery. On a large profile that is tens of seconds of blank screen. Clicking around the sidebar looked like it "fixed" it only because the chain happened to finish at that moment.
After: the workspace mounts as soon as its tab model is hydrated. Chat, browser and editor panes render immediately. Terminal panes are held back (not mounted) until startup restoration has published PTY ownership, then admitted through the normal activation plan.
Two mechanisms changed:
Renderer — what the startup gate fences.
canMountTerminalWorkspaceForStartup(renamedcanAdmitTerminalTabsForStartup) used to gate mounting the whole workspace surface. It now gates only terminal-tab admission. While it is closed,terminal-cold-activationinstalls a startup hold for the active workspace: an empty admitted-tab restriction with no deferral entry, reusing the existing per-workspace restriction map that targeted background mounts and cold-activation deferral already use. The reveal pass and idle admission only act 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 tracked explicitly (which workspace, which tabs it keeps unmounted) rather than inferred from the map's shape; it is applied after the prune pass each render, and it is released on any pass where that workspace is no longer the held one — switched away from, or left for no workspace — so a workspace that only had its surface mounted returns to the unmounted state. Held tabs count as parked for the background output watchers, so bells, titles and completions from those terminals are still observed during the window, as they were before this change.Main — when a restored chat becomes readable. Even with the surface up, a restored chat pane showed "Could not load conversation", because the host only opened a persisted chat's journal (made it readable) in a startup sweep that runs when a client first asks for the tab inventory — and the renderer only asks at the very end of its startup chain. Now the chat's own first read opens it:
agentSession.subscribeandagentSession.historycall a newhost.ensureReadable(sessionId)before they bind a stream or answer. It opens the journal only; it never starts a provider process, so reading stays retain-only. It only applies to a chat the user still has a tab for (the persisted visible-tab index, when the store keeps one), so a read retry landing after a close cannot reopen the chat that was just closed; the check is asked again inside the chat's task queue, so a read already in flight when the close lands cannot reopen it either. The existing capability and setting gate still runs first. Because the pane now mounts before startup restoration has built the structured host, both reads also build it when nothing has yet, asagentSession.holdalready does — otherwise the pane's first read lost the race to its own hold and painted the load error.The pane's hold does the same read before it starts the provider process. Starting it keeps that chat's task queue for the whole start (login-shell environment, handshake), so a read arriving meanwhile would otherwise wait for the child; with the chat already readable, a read skips the queue. This is the order main already produced implicitly — the sweep made a chat readable before any pane could hold it.
The startup sweep did two jobs per chat: open the journal, and re-prove a chat that had been handed to an agent terminal (which reads the PTY census and, if the terminal is missing, marks the chat as needing manual recovery). Only the first is safe before startup has a PTY census. So the per-chat restore is split into a read phase and a handoff phase. The on-demand read runs the read phase alone; the sweep runs both, and still runs exactly where it did before this PR. A chat that was read first gets its handoff re-proved once when the sweep reaches it, and both paths go through the per-chat task queue, so a read racing the sweep opens the journal once. A chat whose journal is missing or corrupt still skips the handoff step, as before.
Also: a stream failure delivered as a raw RPC error payload rendered as
[object Object]; it now shows the payload's message.Why
The blank screen was not a slow restore — the data was there. It was one coarse flag doing two jobs: "may the surface paint" and "may a terminal pane bind a PTY". Only the second needs the reconnect chain. Splitting them along the boundary the code already had (the per-workspace admitted-tab set) keeps the PTY fence exactly where it was and adds no new state.
Alternatives considered:
hydrationSucceededearlier: would let terminal panes attach before reconnect and risk duplicate shells. Rejected.TerminalPane: a second guard next to the existing admission mechanism. Rejected in favor of reusing the restriction map.Known deviations, stated plainly: layout and surface paint from the hydrated model; terminal bodies are gated by a workspace-level admitted-tab set rather than each pane gating on its own session readiness.
Regression source
Two changes stacked, and the second is what made it visible on a native chat:
workspaceSessionReady && hydrationSucceededfencing the whole workbench mount, not just terminal PTY binding) predates this repository's visible history — it is present in the first commit of the squashed history (2026-08-27). Every workspace has always waited for the full chain; on a small profile that window is under a second, which is why it read as "still loading" rather than a bug.fd9125ea8c, 2026-08-28, Codex structured native chat restructure) lengthened that chain and put chats behind it: it madeprepareTerminalStartupRestorationawait the host's structured startup restoration (lease reconciliation over every persisted session), appendedproject-structured-session-tabsafter terminal reconnect, and made a restored chat readable only from that end-of-chain inventory sweep (previously every persisted session was resumed at startup). A restored native chat therefore could not paint or load until the last step of the chain — which is the regression reported here.Linked Issue
Reported directly (restart on a native chat left the workspace blank until the sidebar's reveal button was clicked). No ticket filed.
Visual Proof
Before — restored session, blank workspace until the chain finishes:
After — same profile, captured inside the startup window (25s delay injected before terminal reconnect for the capture;
hydrationSucceededstill false): strip, chat and held terminal tab already painted.Testing
macOS, dev build on an isolated profile with a Codex structured chat + one terminal tab, chat active at quit. Startup probed over raw CDP every 400ms (store flags + DOM markers), with a temporary 25s delay injected before terminal reconnect to make the window observable:
termSlots: 0until the gate opened at +32s, then exactly one PTY bound to the terminal tab (ptyIdsByTabIdlength 1, one daemon session).agentSession.subscriberefused → after the fix the pane shows the conversation at +9.1s; previously it showed "Could not load conversation" from +11.6s until the chain finished.922c1c2626with the same instrument: surface + chat at +8.5s (hydrationSucceededfalse),termSlots: 0until +32.9s, then one PTY.The in-app measurements above were taken with the earlier main-process mechanism (the early sweep). The on-demand read that replaced it is covered by automated tests only and has not been re-measured in the app: a real host and a store reopened from disk answer
agentSession.subscribeandagentSession.historyfor a visible chat with no sweep run and no provider started, still refuse a closed chat and a disabled setting, and removing theensureReadablecall from subscribe turns that test red with the pre-fix "no attached session" refusal. Two more tests cover the early mount: a read with no host built yet answers (red without the install), and a history read and subscribe issued while the pane's hold is blocked inside the provider start both answer (red without the hold's read-first step).Automated:
pnpm testonsrc/renderer/src/components/terminal,src/main/native-chat/agent-session-wire, the runtime structured-restore tests and the read-transport tests (617 files / 5690 tests);pnpm tc;oxlint;pnpm run check:code-quality:changed.Not tested: Linux/Windows, SSH-hosted workspaces (the hold is host-agnostic — it withholds admission, it does not touch attach — but not exercised), paired mobile/web clients.
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
The held active terminal tab shows an empty pane body during the window (no "restoring…" placeholder) — strictly better than the blank workspace it replaces, but a follow-up could render a placeholder.
Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)