Repository navigation
fix(orchestration): let mail wake a slept agent, and make slept agents visible - #18731
brennanb2025 wants to merge 38 commits into
Conversation
Listing slept panes as resumable re-registers their terminal handle, so
`resolveTerminalHandle` now answers for a pane with no process and
`getLiveLeafForHandle` returns its PTY-less leaf instead of throwing. Both
wake branches were therefore skipped, and delivery fell to the idle-status
check — which reads the same for a slept pane as for a busy one — and
returned silently. The send promised a wake ("recipient_asleep") that never
came, for bare `term_` handles and for `run:` mailboxes alike, since a run
resolves through its coordinator handle into that same exit.
Treat "no PTY" as wake evidence before the status check: no idle edge is
ever coming from a pane with no process. A live-but-busy pane still waits
for its idle edge, and `resolveSleepingPaneWakeTarget` remains the wake
authority, so a `restoreOnTabOpenOnly` pane is still never auto-woken.
The unit tests each half already had could not catch this: listing knows
nothing about delivery, and the delivery test hands its dependency a handle
that does not resolve. The new runtime-level test exercises both halves
together — list the slept pane, address the handle the listing minted, and
assert the wake IPC, then the delivery once the pane is back and idle.
Mail arriving for an agent-slept pane resolved a wake and sent terminal:requestTabMount, but a pane agent sleep kills in a running app still has its tab mounted, so the renderer refused the mount by design and nothing happened. The mounted case needs the in-place armed cold restore instead. That path was itself unreachable: its listener subscribed to 'orca:wake-hibernated-agents-worktree' while the dispatcher fired 'orca-wake-hibernated-agents-worktree' (286c210), so the event has never been received on any build. The listener now uses the shared constant and lives in its own module. Also gate the wake on unread mail. Kill-time delivery sweeps for an empty mailbox otherwise woke the pane Orca had just put to sleep - observed live at 21:39:35 slept, 21:39:37 self-woken - which made agent sleep self-defeating as soon as waking started working. Verified end to end in a live instance with a real agent: the pane sleeps and stays asleep while idle, a send wakes it, and the agent cold restores with its prior turn intact.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 10 included reviews per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe change adds resumable slept-pane records to terminal listings and marks them as Merge Risk: 🔵 Low · up to Mail routing can expose an untranslated new recipient-status message to users. This is a bounded presentation consistency issue, but it should be localized before merge or explicitly accepted. 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
Full details: Description checkExplanation The description explains the problem, approach, behavior changes, testing, live verification, and known follow-up. It does not satisfy required template items: the Linked Issue section contains only Resolution Add a valid linked issue number, such as ✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 3
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Team
Run ID: 81d7e17a-1d54-4ddb-964d-0367ec4e0923
📒 Files selected for processing (48)
src/cli/terminal-format.tssrc/main/runtime/orca-runtime-stop-requested-pty-ids.tssrc/main/runtime/orca-runtime-sync-window-graph.tssrc/main/runtime/orca-runtime-test-fixtures.spec.tssrc/main/runtime/orca-runtime-wait-for-leaf-pty-id.tssrc/main/runtime/orca-runtime-write-orchestration-pointer-pty.tssrc/main/runtime/orchestration/mailbox-pointer-delivery.tssrc/main/runtime/orchestration/mailbox-pointer-stage.tssrc/main/runtime/orchestration/mailbox-pointer-submit.tssrc/main/runtime/orchestration/mailbox-sleeping-recipient-wake.test.tssrc/main/runtime/orchestration/sleeping-pane-wake-scheduler.test.tssrc/main/runtime/orchestration/sleeping-pane-wake-scheduler.tssrc/main/runtime/orchestration/sleeping-pane-wake-target.test.tssrc/main/runtime/orchestration/sleeping-pane-wake-target.tssrc/main/runtime/resumable-slept-pane-listing.test.tssrc/main/runtime/resumable-slept-pane-listing.tssrc/main/runtime/rpc/methods/orchestration-recipient-routing-slept.test.tssrc/main/runtime/rpc/methods/orchestration-recipient-routing.tssrc/main/runtime/rpc/methods/terminal/terminal-legacy-subscribe-snapshot.tssrc/main/runtime/rpc/methods/terminal/terminal-multiplex-subscribe-resolution.tssrc/main/runtime/rpc/methods/terminal/terminal-subscribe-method.tssrc/main/runtime/rpc/terminal-multiplex-pty-wait-capacity.test.tssrc/main/runtime/rpc/terminal-subscribe-blank-mount.test.tssrc/main/runtime/runtime-terminal-list-slept-panes.test.tssrc/main/runtime/runtime-terminal-list.tssrc/main/runtime/runtime-workspace-session-controller.tssrc/main/runtime/sleeping-pane-record-lookup.test.tssrc/main/runtime/sleeping-pane-record-lookup.tssrc/main/runtime/slept-pane-mail-wake.test.tssrc/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.tssrc/renderer/src/components/terminal-pane/wake-hibernated-agents-listener.test.tssrc/renderer/src/components/terminal-pane/wake-hibernated-agents-listener.tssrc/renderer/src/constants/terminal.tssrc/renderer/src/hooks/ipc-events/terminal-presentation-ipc-bridge.tssrc/renderer/src/lib/agent-hibernation-pane-eligibility.tssrc/renderer/src/lib/agent-hibernation-planner-supervised-runs.test.tssrc/renderer/src/lib/agent-hibernation-planner.tssrc/renderer/src/lib/agent-hibernation-supervised-runs.test.tssrc/renderer/src/lib/agent-hibernation-supervised-runs.tssrc/renderer/src/lib/mobile-terminal-tab-mount.test.tssrc/renderer/src/lib/mobile-terminal-tab-mount.tssrc/renderer/src/lib/wake-sleeping-agents-in-background.test.tssrc/renderer/src/lib/wake-sleeping-agents-in-background.tssrc/renderer/src/store/slices/agent-status-sleeping-records.tssrc/shared/agent-session-resume.tssrc/shared/pane-agent-identity-inventory.test.tssrc/shared/runtime-terminal-contracts.tssrc/shared/terminal-tab-mount-intent.ts
Included review availability: Your plan provides up to 10 included reviews per hour; 1 remains after this review.
| pane: ResumableSleptPane, | ||
| worktreesById: Map<string, ResolvedWorktree> | ||
| ): RuntimeTerminalSummary { | ||
| const worktree = worktreesById.get(pane.worktreeId) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🤖 get_repo_knowledge executed:
get_repo_knowledge stablyai/orca /tmp/coderabbit-repo-knowledge/stablyai-orca-89dc44e4/learnings
Length of output: 24301
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- target context ---'
sed -n '1,230p' src/main/runtime/orca-runtime-write-orchestration-pointer-pty.ts
printf '%s\n' '--- related definitions and callers ---'
rg -n -C 4 'runtimeWorktreeIdsEqual|collectResumableSleptPanes|worktreesById|worktreePath|branch' srcRepository: stablyai/orca
Length of output: 50370
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- target file outline ---'
wc -l src/main/runtime/orca-runtime-write-orchestration-pointer-pty.ts
sed -n '1,190p' src/main/runtime/orca-runtime-write-orchestration-pointer-pty.ts
printf '%s\n' '--- exact equality symbol locations ---'
rg -n -l 'runtimeWorktreeIdsEqual|collectResumableSleptPanes' src/main src/shared
printf '%s\n' '--- exact symbol context ---'
rg -n -C 8 'runtimeWorktreeIdsEqual|collectResumableSleptPanes' src/main/runtime src/sharedRepository: stablyai/orca
Length of output: 50370
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- worktree identity implementation ---'
sed -n '1,125p' src/main/runtime/runtime-worktree-path-identity.ts
printf '%s\n' '--- resumable pane collector ---'
sed -n '1,220p' src/main/runtime/resumable-slept-pane-listing.ts
printf '%s\n' '--- summary caller and worktree map construction ---'
rg -n -C 6 'buildSleptPaneTerminalSummary|worktreesById' src/main/runtime/orca-runtime-write-orchestration-pointer-pty.ts src/main/runtime --glob '*.ts' | head -n 260Repository: stablyai/orca
Length of output: 29901
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- shared worktree ID comparison ---'
sed -n '1,220p' src/shared/worktree/id.ts
printf '%s\n' '--- resolved worktree map implementation ---'
rg -n -C 10 'getResolvedWorktreeMap|getWorktreeMap' src/main/runtime --glob '*.ts' | head -n 220Repository: stablyai/orca
Length of output: 25720
Use an identity-aware lookup for slept-pane worktree metadata.
collectResumableSleptPanes admits equivalent IDs with runtimeWorktreeIdsEqual, but line 148 performs an exact Map lookup. A non-identical equivalent ID can therefore produce empty worktreePath and branch values. Resolve the matching ResolvedWorktree with the same equality rule and add a regression test.
There was a problem hiding this comment.
ℹ️ One rough edge worth a look, otherwise solid.
Reviewed changes
- Slept panes become visible — an additive
resumablefield onRuntimeTerminalSummarymakesterminal list(and the CLI) show auto-slept and quit-captured panes as asleep-but-resumable instead of absent, deliberately keepingconnected: falsefor its execution-host meaning. - Addressing a slept agent succeeds —
resolveBareOrchestrationRecipientresolves a slept pane through its resume record and returns an accepted, queued send with arecipient_asleepwarning rather than theterminal_not_founda nonexistent handle gets. - Inbound mail wakes an auto-slept pane — delivery requests a wake at every no-process give-up point, gated on unread mail and routed
run:→ coordinator pane,dispatch:→ assignee pane, bare handle → remembered pane; aSleepingPaneWakeSchedulerpaces and suppresses repeat wakes. - User-sleep protection —
mayBackgroundWakeSleepingAgentSessionfails closed onrestoreOnTabOpenOnly/interrupted/automaticResumeBlockedBy, enforced on both the main-process mount path and the renderer in-place wake. - Dead wake listener fixed — the in-place wake listener now subscribes to the shared event constant (the colon-for-dash typo from
286c21005b0had made it unreachable). - Sleep-eligibility guard — a pane supervising a still-
dispatched/pendingworker is no longer sleep-eligible.
The tests are real (exact-value assertions, ablation-proven), and the cross-cutting concerns — SSH host partitions, remote-wire additivity of resumable/recipient_asleep, kill-time self-wake — are all handled deliberately.
DeepSeek Pro (free via Pullfrog for OSS) | 𝕏
Independent QA (Grok) — sleep/wake on real Codex + Claude CLIsIsolated Electron on this PR worktree ( Lifecycle tested
Visible statesCodex working/done (real TUI, prior turns used as resume proof): Claude working/done: Both auto-slept (shell tab foreground; agent tabs still in the strip, processes gone): Codex reawakened with prior Claude reawakened with prior Woken Codex handled a later injected dispatch ( User-slept Claude did not auto-wake after mail: Backing checks
Gaps
Verdict: pass, with the documented remint-delivery gap. Auto-sleep, listed-as-asleep, mail-wake with real |
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
Since the prior pullfrog review (148b09ac), commit 959116f hardens the slept-agent mail wake end to end:
- Wake scheduler learns success/failure —
SleepingPaneWakeScheduler.wakenow returns a boolean; a failed wake is retried once, then parked in afailedset thatmarkGraphReadydrains viaretryPending(). The 64-entry queue cap was dropped in favor of one queued wake per distinct slept pane, and per-pane suppression now self-prunes after its TTL instead of living forever. mayBackgroundWakeSleepingPanemade fail-closed — it now returnsfalsewhen no sleeping record exists, closing the previous!record || …gap that treated a missing record as wakeable.- PTY ownership ambiguity guard —
adoptFirstPtyForLeafHandlerefuses to adopt a first PTY when the incoming graph names that PTY on two leaves, so a pending leaf handle never aliases an ambiguously owned process. - Remint-stable slept-pane dedup —
RuntimeTerminalListkeys sleeping records by leaf identity (stablePaneLeafKey) rather than the exacttabId:leafIdstring, resolving the double-list concern from the prior review. - Terminal-only slept mailbox refused —
resolveBareOrchestrationRecipientnow returnsterminal_not_found+recipient_unreachablefor a slept pane with no durablerun:/dispatch:mailbox, instead of accepting it asrecipient_asleep(which queued mail no resumed reader could consume). - Split-pane wake scoping —
paneKeythreads throughterminal:requestTabMountand the in-place wake event, so a mail wake addresses one pane in a split tab rather than the whole tab.
Tests remain exact-value and ablation-proven, with new coverage for the retry/park lifecycle, graph-ready retry, ambiguous-PTY rejection, reminted-tab dedup, and the terminal-only refusal. The prior dedup thread is addressed by the stablePaneLeafKey change.
DeepSeek Pro (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: Team
Run ID: 7b1c77cd-6f3a-49fd-9f71-0ca2fb8a468b
📒 Files selected for processing (25)
src/main/runtime/orca-runtime-bind-pty-incarnation-handle.tssrc/main/runtime/orca-runtime-stop-requested-pty-ids.tssrc/main/runtime/orca-runtime-sync-window-graph.tssrc/main/runtime/orca-runtime-wait-for-leaf-pty-id.tssrc/main/runtime/orca-runtime-write-orchestration-pointer-pty.tssrc/main/runtime/orchestration/sleeping-pane-wake-scheduler.test.tssrc/main/runtime/orchestration/sleeping-pane-wake-scheduler.tssrc/main/runtime/orchestration/sleeping-pane-wake-target.test.tssrc/main/runtime/orchestration/sleeping-pane-wake-target.tssrc/main/runtime/rpc/methods/orchestration-recipient-routing-slept.test.tssrc/main/runtime/rpc/methods/orchestration-recipient-routing.tssrc/main/runtime/runtime-terminal-list-slept-panes.test.tssrc/main/runtime/runtime-terminal-list.tssrc/main/runtime/slept-pane-mail-wake.test.tssrc/main/runtime/terminal-mobile-subscribe-tab-mount.test.tssrc/preload/api/ui-bridge-terminal-and-session-tabs.tssrc/preload/api/ui-command-event-api.tssrc/renderer/src/components/terminal-pane/pty-connection/pane-pty-binding.tssrc/renderer/src/components/terminal-pane/pty-connection/session-reconcile-dispose.tssrc/renderer/src/components/terminal-pane/wake-hibernated-agents-listener.test.tssrc/renderer/src/components/terminal-pane/wake-hibernated-agents-listener.tssrc/renderer/src/constants/terminal.tssrc/renderer/src/hooks/ipc-events/terminal-presentation-ipc-bridge.tssrc/renderer/src/lib/wake-sleeping-agents-in-background.test.tssrc/renderer/src/lib/wake-sleeping-agents-in-background.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- src/renderer/src/constants/terminal.ts
- src/main/runtime/rpc/methods/orchestration-recipient-routing-slept.test.ts
- src/main/runtime/orca-runtime-sync-window-graph.ts
- src/main/runtime/orchestration/sleeping-pane-wake-target.test.ts
- src/main/runtime/orchestration/sleeping-pane-wake-target.ts
Included review availability: Your plan provides up to 10 included reviews per hour; 2 remain after this review.
| code: 'legacy_terminal_recipient', | ||
| recipient: handle, | ||
| message: `${handle} is a live terminal-only mailbox. Delivery is not durable after that terminal closes; prefer run:<id> or dispatch:<id>.` | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if (sleptPane) { | ||
| const message = `${handle} is asleep but has no durable Run/Dispatch mailbox. Use a canonical run:<id> or dispatch:<id> address.` |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Localize the new unreachable-recipient message.
Line 121 adds a user-facing message directly in source. Move it to the localization catalog and pass handle through a named interpolation placeholder. Apply the same pattern to the new asleep-warning messages.
Based on learnings: new user-facing strings must use intent-named localization IDs with named interpolation placeholders.
Source: Learnings
There was a problem hiding this comment.
ℹ️ No critical issues — one minor suggestion inline.
Reviewed changes
Since the prior pullfrog review (959116f), commit fae2acc closes the remaining wake-recovery gaps:
- Refuses inbound-message mounts while the graph is not
ready—requestRendererTerminalTabMountForPanereturnsfalsewhengraphStatus !== 'ready', so a wake arriving during a renderer reload is reported as failure and parked for themarkGraphReady→retryPending()redrive instead of being silently swallowed bywebContents.send. - Remint-stable sleeping-record lookup — the renderer's
getSleepingRecordForPanematches a sleeping record by stable leaf id across a reminted tab, resuming when one provider session is unambiguous and declining when sessions conflict. - Manual sleep marks retained durable records lazy —
removeSleepingRecordsReplacedByManualWorktreeSleepnow setsrestoreOnTabOpenOnly: trueon a retaineddonedurable record, matching the lazy-restore semantics a freshly captured manual-sleep record already gets.
The new tests are exact-value and cover the reload race (wake parked, then redelivered after markGraphReady), the remint resume, and the conflict-decline case.
DeepSeek Pro (free via Pullfrog for OSS) | 𝕏
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
Since the prior pullfrog review (fae2acc), commit 0013746 closes the last wake-recovery gap:
- Redrives parked wakes on the reload-cancelled ready transition —
retrySleepingPaneWakesWhenGraphReadywas extracted from themarkGraphReadyoverride into a sharedprotectedhelper and is now also invoked frommarkRendererReloadCancelled, so a wake parked infailedduring a reload that ends in cancellation (e.g.did-fail-provisional-load) is redriven instead of stranded until an unrelated message arrives. - New regression test — "redrives a parked wake when a renderer graph reload is cancelled" asserts the exact wake count after cancellation, and the existing reloading test now drives
requestSleepingRecipientWakedirectly.
The prior review's single open thread is addressed. The test is exact-value and would fail without the base-class call; I confirmed the new test and the surrounding reload-cancel and wake-scheduler suites pass.
DeepSeek Pro (free via Pullfrog for OSS) | 𝕏
Independent QA (Grok) — supersedes #18731 (comment)Exact SHA This comment supersedes the earlier QA at #18731 (comment). That run sent to bare Method
Results
Unowned slept bare Same rejection for the pre-remint handle Visible statesCodex working/done (real TUI, PINEAPPLE): Slept panes still in the tab strip (shell foreground; LEMON + Claude slept): Claude reawakened by Codex reawakened with prior PINEAPPLE context (provider-native resume of the same session): Honest gaps
Verdict: partial pass. Unowned slept |
Independent QA (Grok) — supersedes #18731 (comment) and #18731 (comment)Exact SHA This comment supersedes both:
Method
Results
Visible statesCodex working/done (real TUI, PINEAPPLE): Claude working/done (real TUI, MANGO): Automatic idle sleep (shell foreground; agent tabs still listed; processes gone): Claude reawakened by Codex reawakened with prior PINEAPPLE (provider-native resume of the same session). Queued KIWI mail not consumed: Honest remaining gap
Verdict: Claude same-session resume + queued-mail consumption is now proven on this SHA. Codex native resume + prior context is proven. Codex in-session consumption of queued |
There was a problem hiding this comment.
✅ No new issues found in the incremental change.
Reviewed changes
Since the prior pullfrog review (0013746), one commit landed:
- Memoizes the status-bar runtime-target selector —
useWorktreeRuntimeTargetnow selects the stableexecutionHostIdprimitive and derives theRuntimeClientTargetobject viauseMemo, instead of returning a fresh object from the zustand selector on every render (which droveuseSyncExternalStoreinto aMaximum update depth exceededloop). - Adds a render-stability regression test —
PortsStatusSegment.render-stability.test.tsxrenders the segment under its realRecoverableRenderErrorBoundaryand asserts no render-loop crash was logged. It passes now and would fail with the pre-fix selector; confirmed passing locally.
ℹ️ This commit duplicates a fix already on main
The entire delta is byte-for-byte identical to 5412276776 (Fix status bar runtime target render loop (#18685), merged to main independently by another author). git diff main..HEAD over both files is empty, so the merge will be clean — but the branch's merge-base (79d5fb4) predates that commit, making 990e8bd a redundant cherry-pick on a branch that is 70 commits behind main. Rebasing onto current main would drop 990e8bd and leave the PR carrying only its sleep/wake work; as-is it is harmless.
DeepSeek Pro (free via Pullfrog for OSS) | 𝕏
There was a problem hiding this comment.
Important
The statusless Codex delivery path has no reliable post-wake trigger, so a single worker_done to a Codex coordinator can still strand — the silent deadlock this PR set out to fix.
Reviewed changes
- Proved a statusless Codex pane idle before delivering —
deliverForHandlenow routes arun:mailbox whose leaf has a PTY but no observed agent status through a newOrchestrationMailboxStatuslessCodexProofCoordinator, which drivesproveStatuslessCodexIdle(atui-idlewait bounded by the 5-minute default timeout, validated against the process incarnation before and after) and only then stages the pointer. - Threaded the idle proof through stage and submit —
statuslessIdleProofflows throughstageOrchestrationMailboxPointerandsubmitOrchestrationMailboxPointer, re-validated against the current leaf and process incarnation at each step so a status that turns live or a recycled PTY aborts the injection. - Extracted the shared candidate query —
getOrchestrationMailboxPointerCandidatesnow lives in its own module, used by both the pre-proof gate anddeliver.
⚠️ Statusless Codex wake has no reliable delivery trigger
The proof only runs when deliverForHandle is called, but after a mail-driven wake of a statusless Codex pane nothing re-invokes it. The three automatic delivery triggers — the graph sync (orca-runtime-sync-window-graph.ts:307-319), the OSC-title transition (orca-runtime-apply-tracked-pty-title.ts:116-118), and the prompt-submission restore (orca-runtime-serialize-agent-prompt-submission.ts:104-106) — are all gated on lastAgentStatus === 'idle' && lastAgentStatusObservedLive, which a statusless Codex never emits. The only remaining trigger is the one-shot 2-second MailPointerRepointScheduler repoint scheduled at message-arrival time; it fires mid cold-restore (wake mount settle is 10s) and hits proveStatuslessCodexIdle's fail-fast (the leaf is not yet connected/writable), with no retry. A single worker_done to a Codex coordinator therefore strands until a second message or an explicit check — the same deadlock, narrowed to a timing window. The new test (slept-pane-mail-wake.test.ts:343-396) only passes because it manually calls deliverPendingMessagesForHandle('run:run_test'), whereas the status-reporting case auto-delivers off the idle title edge.
Technical details
# Statusless Codex delivery needs a reliable post-wake trigger
## Affected sites
- src/main/runtime/orchestration/mailbox-pointer-delivery.ts:89 — `statuslessCodexProofs.runWhenProven(...)` is the only entry to the proof; it runs solely inside `deliverForHandle`.
- src/main/runtime/orca-runtime-sync-window-graph.ts:307-319 — the graph-sync delivery trigger, gated on `idle && observedLive` (never set for statusless Codex).
- src/main/runtime/orca-runtime-apply-tracked-pty-title.ts:116-118 — title-idle delivery trigger, unreachable for statusless Codex.
- src/main/runtime/orchestration/mail-pointer-repoint-scheduler.ts:8-21 — one-shot 2s repoint, fires before the ~7s Codex cold-restore completes.
## Required outcome
- After a statusless Codex pane is woken by inbound `run:` mail and becomes writable, mailbox pointer delivery must be attempted without depending on a new message arriving.
## Suggested approach (optional)
- Re-schedule the `mailPointerRepointScheduler` repoint (or an equivalent redrive) when the woken pane's graph sync marks it writable/connected while unread `run:` mail remains, or make `proveStatuslessCodexIdle` wait on pane readiness rather than failing fast when not yet `connected`/`writable`.
## Open questions for the human (optional)
- Is a 2s repoint + `tui-idle` wait intended to cover in-place (still-mounted) wakes only, with full-remount Codex wakes acknowledged as deferred until the next message? If so, that limitation should be documented rather than left implicit.DeepSeek Pro (free via Pullfrog for OSS) | 𝕏
Independent QA (Grok) — supersedes #18731 (comment), #18731 (comment), and #18731 (comment)Exact SHA This comment supersedes:
Method
Results
Visible statesCodex working/done (real TUI, STARFRUIT): Claude working/done (real TUI, HONEYDEW): Automatic idle sleep (shell foreground; agent tabs still listed; processes gone): Claude reawakened by Codex reawakened with prior STARFRUIT (provider-native resume of the same session). Queued NIGHTJAR mail not consumed; no IDs / PID evidence
Honest remaining gaps
Verdict: not a full pass. Claude same-session resume + idle-push + self-check + consume is proven. Claude requested-token reply is not. Codex native resume + prior context is proven. Codex in-session consumption of queued |
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
Since the prior pullfrog review (a793f51), commit 31aeb74 closes the statusless-Codex delivery gap that review flagged as must-address:
- Graph-sync delivery trigger for statusless Codex —
syncWindowGraphnow redrivesdeliverPendingMessagesForHandlewhen a leaf whose agent is Codex and has never emitted a status transitions to writable (becameWritable && lastAgentStatus === null && !observedLive). This is exactly the reliable post-wake trigger the prior finding asked for: it fires on the ptyId transition for both in-place and full-remount wakes, independent of a second message arriving. - Regression test drives the real trigger —
delivers queued run mail after a statusless Codex provider reattachnow spies ondeliverPendingMessagesForHandleand asserts the graph sync invokes it after a simulated Codex reattach, then still asserts the full end-to-end write anddelivered_at; it no longer calls delivery by hand. becameWritableextraction — the shared writable-transition predicate is hoisted into a variable and reused by both the status-reporting branch and the new Codex branch.
I confirmed the delivery guard (deliver still re-checks leaf.writable/leaf.ptyId at mailbox-pointer-delivery.ts:160), so the slight relaxation of the status-reporting if condition (the previousLeaf.status terms no longer require leaf.writable) is a no-op in practice — non-writable leaves short-circuit before any write. Both the slept-pane and statusless delivery suites pass (20 tests).
DeepSeek Pro (free via Pullfrog for OSS) | 𝕏
Independent QA (Grok) v5 — supersedes #18731 (comment), #18731 (comment), #18731 (comment), and #18731 (comment)Exact SHA Results
Visible statesCodex working/done (WATERMELON): Claude working/done (PAPAYA): Shell foreground after Codex auto-sleep (Claude tab still listed live at this frame): Claude native Codex native Honest blockers vs acceptance bar
Verdict: not a full pass. No product files changed. |
Independent QA (Grok) — supersedes #18731 (comment)Exact SHA This comment supersedes #18731 (comment). This run specifically checked the SHA's Codex writable-PTY redrive: mail was queued while slept, wake/remount were left to the product, and Method
Results
Visible statesCodex working/done (real TUI, POMEGRANATE): Claude working/done (real TUI, LYCHEE): Automatic idle sleep (shell foreground; agent tabs still listed; processes gone): Claude reawakened by Codex reawakened with prior POMEGRANATE (provider-native resume of the same session). Queued KESTREL31AEB mail not consumed; no IDs / PID evidence
Honest remaining gaps
Verdict: not a full pass. Claude same-session resume + idle-push + self-check of asleep-queued |
There was a problem hiding this comment.
ℹ️ No critical issues — one minor suggestion inline.
Reviewed changes
Since the prior pullfrog review (31aeb74), commit 2122edbc closes the remaining ordering gap in the statusless-Codex wake-delivery trigger:
- Redrives delivery when the Codex reattach identity lands —
registerPtynow callsdeliverPendingMessagesForHandlewhen a PTY'slaunchAgenttransitions tocodexwhile its leaf is already writable and statusless. This covers the ordering where the renderer publishes the writable graph before main receives the provider-owned reattach identity — the case the31aeb74becameWritabletrigger (gated onlaunchAgent === 'codex') could not reach, sincelaunchAgentis stillnullat graph-sync time. - Regression test reproduces the production ordering —
remountStatuslessCodexnow syncs the graph beforeregisterPtyand asserts the delivery handle, so the test fails if theregisterPtyredrive is removed.
DeepSeek Pro (free via Pullfrog for OSS) | 𝕏
There was a problem hiding this comment.
ℹ️ No new issues in this delta — the browser-safety extraction is correct, minimal, and guarded by a real regression test. Not an approval: three minor test-coverage threads from earlier reviews remain open.
Reviewed changes
Since the prior pullfrog review (b27c54b), one commit landed:
- Moved the process-table staleness constant into a browser-safe module —
PROCESS_TABLE_SNAPSHOT_MAX_STALENESS_MSnow lives insrc/shared/process-table-snapshot-timing.ts, a pure module with no Node imports.agent-completion-poll-interval.ts(a renderer module consumed byagent-completion-poll-scheduler.ts) previously imported it fromprocess-table-snapshot-reader.ts, which transitively pullsnode:child_process/node:util/node:fs/promisesand breaks the renderer'splatform: browserbundle. - Preserved the value and public surface —
process-table-snapshot-reader.tsstill re-exports the constant and setsDEFAULT_SNAPSHOT_TTL_MS = PROCESS_TABLE_SNAPSHOT_MAX_STALENESS_MS, so the 500 ms TTL is unchanged for existing importers; the cadence benchmark script follows the same import. - Added a browser-only bundle regression test —
agent-completion-poll-interval.test.tsbuilds the entry point with esbuild underplatform: browserand assertserrors: []. I ablated it: bundling the reader underplatform: browserfails with threeCould not resolve "node:…"errors, so the test genuinely guards against the Node-only import creeping back.
The three open minor threads (missing becameWritable coverage, the malformed PTY-root mock, the untested output-triggered redrive) sit in files this commit does not touch and remain open.
DeepSeek Pro (free via Pullfrog for OSS) | 𝕏
There was a problem hiding this comment.
ℹ️ No new issues in this delta — the
origin/mainmerge is clean and subsumes the browser-safe extraction from the prior commit with main's stricter fix. Not an approval: three minor test-coverage threads from earlier reviews remain open.
Reviewed changes
Since the prior pullfrog review (c939764), one merge landed:
- Merged
origin/main, which independently fixed the same blank-window bug (#18742) — main's version movesPROCESS_TABLE_SNAPSHOT_MAX_STALENESS_MSinto the pureprocess-table-snapshot.tsmodule and adds a graph-walking ratchet,renderer-node-builtin-boundary.test.ts, that walks every renderer production entry (lazy routes included) and refuses anynode:builtin. The branch'sprocess-table-snapshot-timing.tsmodule and esbuild bundle test are therefore dropped from the PR delta, superseded by main's broader guard. - Resolved the two conflicts correctly —
agent-completion-poll-interval.tsnow imports the constant from the pureprocess-table-snapshotmodule (with an explanatory comment),process-table-snapshot-reader.tsre-exports it, and the 500 ms TTL is unchanged. The boundary ratchet and poll-interval suites pass, andpnpm tc:webis clean.
The three open minor threads (missing becameWritable coverage, the malformed PTY-root mock, the untested output-triggered redrive) sit in files this merge does not touch and remain open.
DeepSeek Pro (free via Pullfrog for OSS) | 𝕏
Independent QA (Grok) — supersedes earlier QA comments on this PRExact SHA This comment supersedes earlier QA comments on #18731, including the blank-renderer FAIL on The previous SHA's renderer-blank ( Method
No QA-side Assertion table
First failed assertionClaude did not produce the phase-two token FINCH5DB9 after a genuine pointer submit and Codex independently failed the idle-push pointer after native resume of Visible statesFolder workspace fixture (non-git) before agents: Claude phase-one (Claude Code v2.1.261, LEMON5DB9, ACK-LEMON5DB9, bypass permissions): Claude automatic idle sleep (shell foreground; slept tab listed; original PID gone): Claude native resume of Claude after pointer turn: refusal of phase two; duplicate composer prompt Codex slept (shell foreground; Codex tab listed asleep; original PIDs gone): Codex native resume of IDs
VerdictFAIL. Renderer boots on this SHA (the |
There was a problem hiding this comment.
ℹ️ No new issues in this delta — the resumed-Codex wake fix is correct and ablation-proven. Not an approval: two minor test-coverage threads from earlier reviews remain open.
Reviewed changes
Since the prior pullfrog review (5db91a4), commit 2293df1 fixes mail wake for a resumed Codex pane by removing the liveness gate that rejected it:
- Dropped the
lastAgentStatusObservedLivegate from statusless-Codex delivery — a resumed Codex emits its working spinner title (flippingobservedLivetotrue) before clearing to the ready composer, so every statusless-Codex predicate (the proof candidate, theregisterPtyredrive, thebecameWritabletrigger, anddeliverForHandle) now keys onlastAgentStatus === nullalone. The relaxation stays scoped to Codex by thelaunchAgent === 'codex'check at the proof candidate, so a liveworking/permissionstatus is still rejected. - Relaxed the idle-proof revalidation —
isStatuslessIdleProofCurrentaccepts the ready-composer state (status === null) regardless of liveness, while still rejecting a live non-idle status. - Added a Codex
working → nulldelivery trigger —applyTrackedPtyTitlenow delivers pending mail when a Codex pane's title clears fromworkingto a plain cwd title (the ready composer), the signal the graph-sync and registration triggers can miss on an in-place wake. - Parameterized the reattach test over ordering —
remountStatuslessCodexnow emits a braille-spinnerworkingtitle and takes agraphBeforeRegistrationflag, driving both the title-transition andbecameWritablepaths and assertinglastAgentStatusObservedLive: true.
The reattach regression test is genuine — reverting the deliverForHandle gate fails 4 of the 12 cases, and the full suite passes (12). This also closes the prior becameWritable coverage thread (the false case registers the Codex identity before the graph goes writable).
DeepSeek Pro (free via Pullfrog for OSS) | 𝕏
Independent QA (Grok) — supersedes #18731 (comment) and earlier QAExact SHA This comment supersedes #18731 (comment) and earlier QA on this PR. No product files were edited, committed, or pushed. Screenshots are artifacts only. Method
No QA-side Assertion table
Visible statesFolder workspace fixture (non-git) before agents: Claude phase-one (Claude Code v2.1.261, LEMON2293, ACK-LEMON2293, bypass permissions): Claude automatic idle sleep (shell foreground; slept tab listed; original PID gone): Claude native resume of Codex phase-one (Codex CLI v0.153.4, YOLO, MANGO2293, ACK-MANGO2293): Codex slept (shell foreground; Codex tab listed asleep; original PIDs gone): Codex native resume of IDs
VerdictPASS. On SHA |
|
Too many files changed for review (114 files, 100 file limit). Bypass the limit by tagging |
There was a problem hiding this comment.
ℹ️ No new issues in this delta — the split-pane wake scoping is correct and well tested. Not an approval: two minor test-coverage threads from earlier reviews remain open.
Reviewed changes
Since the prior pullfrog review (2293df1), commit de55469a scopes a mail-driven wake to the addressed split pane and wakes already-mounted slept panes in place:
- Slept-pane wake scoped to the addressed split leaf — main now includes
intent: 'inbound-message'in theterminal:requestTabMountpayload (orca-runtime-wait-for-leaf-pty-id.ts), andresolveMobileTerminalTabMountattaches acoldRestorePaneKeysByTabIdrestriction for inbound-message mounts. That restriction threads through the parking/overlay/pty-dep layers (via the newbackground-terminal-cold-restore-scope.tsmerge/apply/prune set) so a mail mount cold-restores only the pane the message named instead of every slept agent in the tab. - Already-mounted slept panes wake in place —
resolveMobileTerminalTabMountnow distinguishesalready-mountedfrom "tab does not resolve", and the IPC bridge routes the former to a newwakeMountedSleptPaneInPlace, which fires a tab- and pane-scopedWAKE_HIBERNATED_AGENTS_WORKTREE_EVENT(gated onmayBackgroundWakeSleepingAgentSession) because mounting an already-mounted tab is a no-op. - Unaddressed sleeping siblings stay inert —
deferUnaddressedMailScopedColdRestore(first step ofrunDeferredSessionAttach) arms ahibernatedWakeTargetand skips the SSH attach for any slept pane outside the mail scope, so restoring the split layout does not revive siblings that received no mail. - Listener and intent plumbing —
installWakeHibernatedAgentsListenernow filters bytabId,PanePtyBindingexposespaneKey = session.cacheKey, and a sharedterminal-tab-mount-intent.tskeeps the field additive (absent = permissive, so the mobileclient-subscribepath stays unscoped).
The scoping coalesces correctly — an unscoped request dominates a mail-scoped one in both mergeBackgroundMountColdRestorePaneScopes and applyBackgroundMountColdRestorePaneRestriction — and the new tests are exact-value (assert connect/attach never fire for the inert sibling, exact merged-detail structures, per-pane listener dispatch). I confirmed the affected renderer suites pass (84 tests).
DeepSeek Pro (free via Pullfrog for OSS) | 𝕏
Independent QA (Grok) — supersedes #18731 (comment) and earlier QAExact SHA This comment supersedes #18731 (comment) and earlier QA on this PR. No product files were edited, committed, or pushed. Screenshots are artifacts only. Method
No QA-side Assertion table
Split-sibling isolation (this SHA)First proven failed assertion: both real provider sibling leaves did not auto-sleep together, so the scoped cold-restore isolation (mail to one addressed provider; sibling stays What was constructed via the product surface:
Adjacent observations (not a substitute for the failed assertion):
Visible statesFolder workspace fixture (non-git) before agents: Claude phase-one (Claude Code v2.1.261, CEDARDE55, ACK-CEDARDE55, bypass permissions): Same-split siblings (Claude left + Codex v0.153.4 YOLO right, both phase-one tokens): Codex split-leaf phase-one (MAPLEDE55, ACK-MAPLEDE55, YOLO): Codex New-tab phase-one (required New-tab path, YOLO launch command visible): Shell foreground while New-tab Codex is listed asleep ( Codex native resume of Split tab after Claude phase two (OSPREYDE55) while Codex sibling stayed on phase one: IDs
VerdictFAIL. First proven failed assertion: same-split sibling auto-sleep + scoped cold-resume isolation. Both real provider leaves of tab Codex New-tab completed the required sleep → bind-does-not-wake → durable mail |
Independent QA (Grok) — supersedes #18731 (comment) and all earlier QAExact SHA This comment supersedes #18731 (comment) and all earlier QA on this PR. No product files were edited, committed, or pushed. Screenshots are artifacts only. Method
Phase-two mail / native resume / no-QA-input pointer handling was not started, because the required both-siblings-asleep precondition failed. Assertion table
Split-sibling isolation (this SHA)First proven failed assertion: both real provider sibling leaves of tab What was constructed (New-tab first, then product split/move — not
Scoped cold-resume isolation (mail to one slept sibling; the other stays Visible statesFolder workspace fixture (non-git) before agents: Claude phase-one (Claude Code v2.1.261, CEDARR2QA, ACK-CEDAR-R2QA, bypass permissions): Codex New-tab phase-one (Codex CLI v0.153.4 YOLO, MAPLER2QA, ACK-MAPLE-R2QA): Same-split siblings after product pane move (Claude left + Codex right, original processes, both phase-one tokens): Shell tab foreground (split tab backgrounded/unmounted): After idle coordinator: Claude leaf slept ( IDs
VerdictFAIL. First proven failed assertion: same-split sibling auto-sleep + scoped cold-resume isolation. Both real provider leaves of tab This run did launch both providers through genuine New-tab |
There was a problem hiding this comment.
Reviewed changes
Since the prior pullfrog review (de55469a), commit d3662d154a adds cross-tab PTY authority transfer for terminal-pane moves:
- Added
transferExistingPtyAgentPaneAuthority— a fail-closed helper (terminal-split-existing-pty-authority.ts) that locates a unique leaf in another tab of the same worktree whoseptyIdsByLeafIdmaps to the split'sptyId, then moves that leaf's agent status and sleeping record via the existingtransferAgentPaneAuthoritystore action. - Wired it into the split handler —
installTerminalPaneMountEventsnow invokes the transfer when a non-command split adopts an existing PTY (createdPane && detail.ptyId). - Added two exact-value tests — one drives the real event routing and asserts the exact transfer args; the other drives the real store and asserts the completed-resumable-agent record survives the source tab's retirement, plus the "do not guess on two claiming leaves" case.
⚠️ The authority transfer appears to have no production trigger
transferExistingPtyAgentPaneAuthority is only reachable from a split request whose detail.ptyId is set, but I could not find a production code path that populates that field with a PTY owned by a different tab. The single production dispatch that sets ptyId (terminal-presentation-ipc-bridge.ts:172-186, the onCreateTerminal split-reveal branch) always carries a freshly spawned PTY revealed into the source leaf's own tab, so the cross-tab scan finds no owner in another tab and returns false — the transfer never fires. The other split producer (ui:splitTerminal → routeRuntimeTerminalSplitRequest) does not forward ptyId at all. If so, the move-to-existing-tab authority loss this commit's title targets is not actually fixed by it.
Technical details
# transferExistingPtyAgentPaneAuthority has no production reachable trigger
## Affected sites
- src/renderer/src/components/terminal-pane/terminal-pane-mount-events.ts:73-81 — the only call site, guarded on `createdPane && detail.ptyId`.
- src/renderer/src/components/terminal-pane/terminal-split-existing-pty-authority.ts:26-44 — scans `candidateTabId !== args.tabId` for a leaf owning `args.ptyId`; no cross-tab owner → `false`.
- src/renderer/src/hooks/ipc-events/terminal-presentation-ipc-bridge.ts:172-186 — only production dispatch that sets `detail.ptyId`; its `ptyId` is the `onCreateTerminal` payload's `ptyId`.
- src/main/runtime/orca-runtime-split-pty-backed-terminal.ts:52,101-113 — `revealSplit` forwards `ptyId: result.id` from `ptyController.spawn(...)` and `tabId: parentTabId` (same tab as the source leaf), so `detail.ptyId` is always brand-new and always in the source leaf's own tab.
- src/renderer/src/hooks/ipc-events/terminal-ui-routing-ipc-bridge.ts:91-100 — the `ui:splitTerminal` path constructs `detail` without `ptyId`; the `splitTerminal` notifier (runtime-window-lifecycle.ts:152-163) never sends `ptyId` either.
## Required outcome
- Confirm the production "move a terminal pane into an existing tab" flow that is supposed to emit a split request whose `ptyId` is another tab's existing PTY, and that it reaches `installTerminalPaneMountEvents`; or acknowledge the transfer is currently unreachable and land it together with (or after) the flow that triggers it.
## Open questions for the human (optional)
- Is there a move-to-existing-tab gesture (drag onto a sibling tab, mobile, CLI/runtime) that emits a split with an existing PTY but does not yet run through `installTerminalPaneMountEvents`? If it runs through `detachTerminalPaneToTab` instead, that path already transfers authority via `syncPaneDetachPtyOwnership` — is this commit intended to cover a second, distinct mechanism?DeepSeek Pro (free via Pullfrog for OSS) | 𝕏
Independent QA (Grok) — supersedes #18731 (comment) and all earlier QAExact SHA This comment supersedes #18731 (comment) and all earlier QA on this PR. No product files were edited, committed, or pushed. Screenshots are artifacts only. Method
Assertion table
Split-sibling isolation (this SHA)First proven failed assertion: mail to a slept Codex sibling (hard criterion 7). Both real provider leaves did auto-sleep together on the first idle cycle, and Claude-only durable mail did native-resume Claude while Codex stayed What then failed:
What this SHA did fix vs #18731 (comment): New-tab → product split/move of the existing Codex PTY kept Codex agent-status on Visible statesFolder workspace fixture (non-git) before agents: Claude phase-one (Claude Code v2.1.261, CEDARD366, ACK-CEDARD366, bypass permissions): Codex New-tab phase-one (Codex CLI v0.153.4 YOLO, MAPLED366, ACK-MAPLED366): Same-split siblings after product pane move (Claude left + Codex right, original processes, both phase-one tokens): Shell tab foreground (split tab backgrounded/unmounted): After idle coordinator: both sibling leaves slept ( Claude phase two after native Second idle wait: Claude re-slept; Codex remount stayed live (no agent-status, not selected by idle planner): IDs
VerdictFAIL. First proven failed assertion: mail to a slept Codex sibling (criterion 7). The first idle cycle did auto-sleep both real provider leaves of tab |
There was a problem hiding this comment.
ℹ️ No new issues in this delta — the cold-resume status restoration is correct, fail-closed, and well tested. Not an approval: two minor test-coverage threads from earlier commits remain open (the malformed PTY-root mock in
agent-foreground-process-confirmation.test.tsand the untested output-triggered redrive inmailbox-statusless-codex-delivery.test.ts), neither touched by this commit.
Reviewed changes
Since the prior pullfrog review (d3662d154a), commit 9953541 restores a resumed agent's completed status so a mounted cold resume can sleep again:
- New
restoreCompletedAgentStatusAfterColdResume— re-establishes the retaineddonestatus plus launch config after a sleeping-record-based resume, guarded on matching agent/provider session,isPassiveCompletedHibernationEvidence, and a non-interrupted retaineddoneentry whose worktree/agentType match. clearSleepingRecordAfterColdRestoreSpawngains aresumeConfirmedgate — the restore runs only when the resume actually succeeded (agentResumeUnavailable !== true); a dropped resume argv skips it at both call sites, so a brand-new session is never mislabeled as its resumed predecessor.- Alias / move recovery — when the sleeping record's pane key differs from the resumed pane, the completed status and launch config re-register under the new key and the retained completion is dismissed under the old one.
- Tests — a new 3-case unit suite drives the real store (exact status restore, fail-closed on provider-session mismatch, alias recovery), plus session/store fixtures and resume-banner assertions in the two cold-restore suites.
DeepSeek Pro (free via Pullfrog for OSS) | 𝕏
Independent QA (Grok) — supersedes #18731 (comment) and all earlier QAExact SHA This comment supersedes #18731 (comment) and all earlier QA on this PR. No product files were edited, committed, or pushed. Screenshots are artifacts only. Method
Assertion table
Split-sibling isolation + second sleep (this SHA)What #18731 (comment) failed: opening the split to show Claude phase two remounted Codex without republishing Codex This SHA:
Pane isolation held: Claude mail did not drive Codex; Codex mail did not duplicate Claude’s pointer. Folder workspace throughout. Visible statesFolder workspace fixture (non-git) before agents: Claude phase-one (Claude Code v2.1.261, CEDAR9953, ACK-CEDAR9953, bypass permissions): Codex New-tab phase-one (Codex CLI v0.153.4 YOLO, MAPLE9953, ACK-MAPLE9953): Same-split siblings after product pane move (Claude left + Codex right, original processes, both phase-one tokens): Shell tab foreground (split tab backgrounded/unmounted): After idle coordinator: both sibling leaves slept ( After both unmounted mail-wakes: split opened showing reawakened Claude (OSPREY9953, pointer Shell tab foreground again after the remounted split: Second idle wait: both remounted/resumed real agents auto-slept again ( IDs
VerdictPASS. First idle cycle auto-slept both real provider leaves of tab |
Final review status — CLEANReviewed exact final SHA Review and fixes
Verification
Real-agent Electron proofGrok validated actual installed Claude Code 2.1.261 and Codex CLI 0.153.4 in an isolated Orca Electron instance using Playwright CDP. This was a non-git folder workspace with genuine product launch/move behavior—no mocks, fake TUIs, forced hibernation ticks, direct wake APIs, QA-side mailbox substitution, or post-wake QA typing. Both agents performed real work, became sibling split leaves with their original PTYs, naturally auto-slept, independently woke from durable mail while the other remained asleep, consumed the mail and wrote unique results (OSPREY9953 / PUMA9953), appeared together reawakened, then naturally auto-slept a second time. Full uncropped screenshot evidence and process/session IDs: #18731 (comment) CI statusAt the time of this comment, exact-head CI has 22 successful checks, 11 expected skips, and 2 E2E jobs still running. Unit shard 7 passed all assertions and then hit a pre-existing timing-sensitive test-cleanup race: an unchanged test closes its DB before an unchanged queued Verdict: CLEAN and ready from code-review, readiness, performance, and real-agent Electron QA perspectives. |
Port the sleep/wake + statusless-Codex delivery onto #16904's durable mailbox pointer subsystem. Primary wake paths in deliverForHandle are unchanged; the post-write Enter path wakes on a refused WriteSettlement and leaves unverifiable on preserveAmbiguousWrite (loss of contact is not evidence of process death).
…nt-sleep-wake # Conflicts: # src/main/providers/agent-foreground-process.test.ts # src/main/runtime/orca-runtime-register-pty.ts # src/main/runtime/orca-runtime-sync-window-graph.ts # src/main/runtime/orchestration/mailbox-delivery-target.ts # src/main/runtime/orchestration/mailbox-pointer-submit.ts # src/renderer/src/lib/agent-hibernation-pane-eligibility.ts # src/renderer/src/store/slices/agent-status-manual-sleep-capture.test.ts # src/shared/process-table-snapshot-reader.ts
There was a problem hiding this comment.
ℹ️ No new issues in the port onto #16904 — one minor test-coverage gap inline.
Reviewed changes
Since the prior pullfrog review, the branch's only new commits are five Merge origin/main merges, and they are not inert: main had independently refactored the mailbox pointer subsystem (#16904), so the merges port the sleep/wake and statusless-Codex delivery onto the new architecture.
- Ported delivery onto
OrchestrationMailboxPointerHandleDelivery—deliverForHandle's body moves to a newmailbox-pointer-handle-delivery.ts, preserving the three wake-on-no-process sites (!terminalHandle,!leaf.ptyId, catch), andmailbox-pointer-delivery.tsdelegates to it. - Re-threaded the idle proof and wake request —
statuslessIdleProofnow flows throughdeliver/stage/submit/park, andmailbox-pointer-submit.tsgainsgetTerminalProcessIncarnationandrequestSleepingRecipientWake, waking on proven-absent and on arefusedEnter but never onunverifiable. - Reconciled the runtime triggers —
registerPtyredrives a Codex identity transition (priorLaunchAgentis captured atorca-runtime-register-pty.ts:91, before the reassignments at:115/:127),syncWindowGraphkeeps thebecameWritableCodex branch,mailbox-delivery-targetprefers a livecoordinator_pane_keyhandle, and the withdrawnautomaticResumeBlockedByfence is dropped from the sleep/wake guards (no producer remains; hydration strips it; a legacy persisted fence is deliberately ignored). - Adapted the renderer listener — new
isWakeDetail/isWakeablePaneBindingguards inwake-hibernated-agents-listener.ts, with split-panepaneKeyscoping and the user-sleep gate intact.
Two independent specialist passes (mailbox wake parity; renderer pane-scope parity) both found the port faithful. The only open item is the inline test-coverage gap below.
DeepSeek Flash (free via Pullfrog for OSS) | 𝕏
| // Refused is a positive "no byte left" verdict, so the recipient can | ||
| // be treated as gone; unverifiable must not wake (loss of contact is | ||
| // never evidence of process death — ssh-execution-boundary.md). | ||
| deps.requestSleepingRecipientWake?.(input.mailboxHandle) |
There was a problem hiding this comment.
The new refused/unverifiable wake arms are load-bearing and untested: nothing pins that a refused Enter requests a wake, and nothing pins that an unverifiable Enter (a lost SSH settlement) leaves it unrequested. The neighbouring cases already stub writePty with settledWriteStub(), so both are cheap to add. The comment also reads stronger than the verdict proves — refused includes live-pane declines (transport_queue_full, write_gate_denied, provider_refused_write), so "treated as gone" holds only because the downstream unread + durable-sleeping-record gates reject a live pane.
Technical details
# Pin the refused/unverifiable Enter wake arms
## Affected sites
- `src/main/runtime/orchestration/mailbox-pointer-submit.ts:143-148` — new wake on a `refused` Enter; the `unverifiable` arm deliberately does not wake.
- `src/main/runtime/orchestration/mailbox-pointer-submit.test.ts` — `writePty: vi.fn(settledWriteStub())` can already yield non-accepted outcomes; no case covers these arms.
## Required outcome
- A case that drives `submitOrchestrationMailboxPointer` with a `refused` Enter settlement and asserts `requestSleepingRecipientWake` fires with the mailbox handle.
- A case that drives an `unverifiable` Enter settlement and asserts it does not (the `ssh-execution-boundary` invariant: loss of contact is never evidence of process death).
## Open questions for the human (optional)
- Should the wake be reachable from a live pane's `refused` reasons (`transport_queue_full`, `write_gate_denied`), or restricted to absence-proving reasons?resolveMobileTerminalTabMount took over its last production caller; the wrapper survived only for its own tests. Retarget those tests at the resolver and delete it.













































































What this fixes
Agent sleep kills an orchestration coordinator pane while the worker it dispatched is still running. The worker finishes, its
worker_donelands in a durable mailbox, nothing can deliver it because the pane has no process, and nothing wakes the pane. The run deadlocks and every surface reports success.Reproduced end to end in a live instance: the dispatch was still
dispatchedand its worker still running when the coordinator was killed; the completion message then satread=0 delivered_at=NULLuntil the tab was clicked, at which point both queued messages delivered within the same second. The mail was deliverable throughout — only the wake was missing.Approach
Rather than trying to prevent the sleep, this makes an arriving message able to wake the pane.
Preventing the sleep was the first design and it was abandoned on evidence: the list of things that count as "still owed work" would not close. Coordinators, pending decision gates (which by construction leave no unsettled dispatch), the window right after a worker reports done, a task flipped back to
readyby gate resolution, a new run holding ready tasks — each review round found another. A fix that only works when that list is complete fails silently, which is how this bug shipped. Waking needs no such list: the message arriving is the evidence.Three parts:
orca terminal listentirely, so a sender could not tell "asleep, will read later" from "gone". It now lists with an additiveresumablemarker.connectedkeeps its existing meaning (the process really did exit), and the host-liveness vocabulary (live/unverifiable/exited) is deliberately not reused for it.orchestration sendto one is accepted and queued with arecipient_asleepwarning, instead of failing withterminal_not_found— the same answer a handle that never existed gets.Plus one cheap guard: a pane supervising an actively dispatched worker is no longer sleep-eligible. That is belt-and-braces, not the mechanism — correctness rests on the wake path.
Also fixes a pre-existing bug on main
The in-place wake this depends on was unreachable. Its listener subscribed to
orca:wake-hibernated-agents-worktreewhile the dispatcher firedorca-wake-hibernated-agents-worktree— a colon for a dash, shipped in 286c210. The event has never been received on any build, so the path its own docstring calls "the primary wake mechanism" for already-mounted slept panes was dead. The listener now uses the shared constant and lives in its own module.A second defect surfaced only once waking worked: kill-time delivery sweeps for an empty mailbox woke the pane Orca had just put to sleep (observed live — slept 21:39:35, self-woke 21:39:37), which would have made agent sleep self-defeating. The wake is now gated on unread mail.
Verification
Unit and integration tests, ablation-proven (removing the fix turns the new assertions red). Typecheck clean. Changed-code quality gate: 0 findings across native, type-aware and React Doctor over 48 files.
Verified end to end in a live instance with a real agent, on the same rig that found the bug:
connected: false, resumable: trueok: true,recipient_asleep--resumeThe user-slept protection is covered by an ablated unit test (removing the guard fails only that test) and by the code path that sets the flag, but was not exercised live — there is no CLI for the workspace-sleep gesture.
Known follow-up, not fixed here
Bare
term_direct mail wakes the pane but stays unread: the handle re-mints on wake and mailbox resolution matches only the current handle string, while the pointer push isrun:-only. Such a message therefore re-wakes the pane on every later sleep — a slow wake loop.run:anddispatch:mail delivers normally. This is pre-existing behaviour of terminal-only mailboxes and wants its own ticket.