Repository navigation
fix(runtime): answer startup terminal queries for background-created terminals - #22384
Conversation
…terminals A runtime-created terminal (orchestration worker-start, `orca terminal create`) has no renderer pane until the user opens its tab. Main only answers terminal queries for PTYs the renderer marked hidden, so on a fresh app sitting on the landing screen nothing answered the agent's startup cursor-position query. Muse waits ~2s per unanswered CPR and then exits 0 with no output, which surfaced as `agent_readiness: timeout`. Background runtime spawns now carry initiallyHidden, mirroring the renderer's hidden-at-spawn path: fresh daemon sessions are marked before byte zero, the committed id is marked and paced as backgrounded, and the mark is released on failure, reattach, or adoption. A pane that later mounts visible unmarks and restores from the model snapshot as before.
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes — the full one-commit diff (f62c583), plus the runtime/CLI spawn path it threads through.
- Background runtime spawns now start hidden —
orca-runtime-create-terminal.tsspreadsBACKGROUND_TERMINAL_SPAWN_FLAGS(initiallyHidden: true) into the background-branchptyController.spawn, so main owns delivery and answers startup queries until a renderer view mounts. - New runtime hidden-delivery module (
spawn-hidden-delivery.ts) — marks a fresh daemon session hidden before byte zero, re-affirms it after commit (withsyncPtyBackgroundedDelivery(id, 'spawn')andcloseStartupQueryAuthorityForPty), and releases the mark for reattach / stable-pane adoption / adopted agent session. - Controller plumbing — optional
transitionSpawnHiddenRendererPtyDeliveryState/syncPtyBackgroundedDeliverydeps wired inregister-handlers.ts;preSpawnHiddenMarkIdand theinitiallyHiddenarg added to spawn state and the controller contract. - Tests — new
pty-runtime-hidden-at-spawn-mark.test.tscovers the pre-spawn mark, failure release, reattach skip, unflagged spawn, and an end-to-endESC[6n→ESC[1;1Rreply for a PTY with no renderer view;cli-terminal-create-host-session-binding.test.tsassertsinitiallyHidden: true.
I verified the new suite and the related existing hidden-delivery suites pass, and pnpm run typecheck:node is clean. The deliberate divergence from the IPC twin (ipc/spawn-commit-persist.ts marks hidden even for reattach/adopted; this path releases instead) reads as correct — the runtime has no renderer request, so it must not hide a session that may already back a visible pane. Marking after sendPtySpawnedToRenderer is safe because main applies the mark synchronously in the same tick, before any renderer IPC can be handled.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. Warning Review limit reachedNext included review available in 16 minutes. View limit detailsLimit details: You’ve used all 10 included reviews currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughBackground terminal creation now passes Priority: ➖ Normal Merge Risk: 🔵 Low · up to Opening a terminal during startup can leave its output hidden until another visibility transition. Fix the handoff before merging, or explicitly accept this bounded risk. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Advanced
Run ID: e47a890f-4593-4620-8516-eb0233ffec1a
📒 Files selected for processing (11)
src/main/ipc/pty-runtime-hidden-at-spawn-mark.test.tssrc/main/ipc/pty/register-handlers.tssrc/main/ipc/pty/runtime/controller-deps.tssrc/main/ipc/pty/runtime/spawn-execute.tssrc/main/ipc/pty/runtime/spawn-hidden-delivery.tssrc/main/ipc/pty/runtime/spawn-state.tssrc/main/ipc/pty/runtime/spawn.tssrc/main/runtime/cli-terminal-create-host-session-binding.test.tssrc/main/runtime/orca-runtime-create-terminal-dependencies.tssrc/main/runtime/orca-runtime-create-terminal.tssrc/main/runtime/runtime-pty-controller-contract.ts
Included review availability: Your plan provides up to 10 included reviews per hour; 6 remain after this review.
…eloads A runtime background spawn has no renderer pane to report visibility or re-mark it hidden, so it synced as foregrounded (no backpressure thinning) and a reload/crash gate reset cleared its hidden mark, leaving startup queries unanswered. Track runtime-owned hidden marks: they survive renderer-scoped resets, count as known-hidden for backgrounded pacing until a visible report, and are released by a renderer unmark or PTY teardown.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to GitHub limitations.
🟡 Minor · Avoid re-marking a PTY that is already visible. · spawn-hidden-delivery.ts:29-43
src/main/ipc/pty/runtime/spawn-hidden-delivery.ts:29-43
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winAvoid re-marking a PTY that is already visible.
When an initially hidden daemon spawn becomes visible before
commitRuntimeSpawnHiddenDeliveryruns, the commit callback can reapply the hidden delivery gate.pty:setRendererPtyVisibleupdatesvisibleRendererPtys, but it does not clear the hidden gate.shouldDropHiddenRendererPtyDataalso ignores visible state, so output can be dropped from the opened terminal until another unhide transition occurs. Query authority is already closed by the visibility listener.Suggested fix
import { closeStartupQueryAuthorityForPty } from '../provider/registry' +import { visibleRendererPtys } from '../delivery/visibility-state' import type { RuntimePtySpawnState } from './spawn-state' @@ - markRuntimeSpawnHidden(ctx, id) - if (ctx.preSpawnHiddenMarkId !== id) { - releaseRuntimeSpawnPreSpawnHiddenMark(ctx) + if (visibleRendererPtys.has(id)) { + releaseRuntimeSpawnPreSpawnHiddenMark(ctx) + } else { + markRuntimeSpawnHidden(ctx, id) + if (ctx.preSpawnHiddenMarkId !== id) { + releaseRuntimeSpawnPreSpawnHiddenMark(ctx) + } } ctx.deps.syncPtyBackgroundedDelivery?.(id, 'spawn')
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Advanced
Run ID: 6c92fa25-c5f6-4292-b864-c1e82e3c9b37
📒 Files selected for processing (5)
src/main/ipc/pty-hidden-delivery-gate.test.tssrc/main/ipc/pty-hidden-delivery-gate.tssrc/main/ipc/pty-runtime-hidden-at-spawn-mark.test.tssrc/main/ipc/pty/delivery/producer-sync.tssrc/main/ipc/pty/runtime/spawn-hidden-delivery.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/main/ipc/pty-runtime-hidden-at-spawn-mark.test.ts
- src/main/ipc/pty/runtime/spawn-hidden-delivery.ts
Included review availability: Your plan provides up to 10 included reviews per hour; 2 remain after this review.
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes — the new commit d2eaedf (range-diff against the prior pullfrog review at f62c583), plus the renderer visible-handoff path it depends on.
- Tracked runtime-owned hidden marks —
pty-hidden-delivery-gate.tsgainsruntimeOwnedHiddenRendererPtys;resetRendererScopedHiddenPtyDeliveryStatere-adds those PTYs after a reload/crash reset, so a pane-less background spawn keeps answering startup queries when no renderer party exists to re-mark it. - Released on unmark or teardown —
unmarkHiddenRendererPty(renderer visible mount) andclearHiddenRendererPtyDeliveryState(wired intoclearProviderPtyState) both drop the runtime-owned flag, so the mark cannot outlive the PTY or a renderer view that took ownership. - Paced as background —
syncPtyBackgroundedDeliverynow counts a runtime-owned PTY with no visible report as known-hidden, so the daemon pacer keeps thinning a background spawn's flood until a renderer reports visible. - Tests — the gate suite adds survival across a scoped reset plus release via unmark/teardown; the spawn suite adds reload survival, pacing-until-visible, and renderer-handover (no resurrection) cases. I ran these and the related hidden-delivery/lifecycle/claims suites (7 files, pass);
pnpm run typecheck:nodeandpnpm run check:code-quality:changedare clean.
I traced the renderer handoff: a mounting visible pane's first syncHiddenRendererPtyDelivery calls declareRendererPtyDeliveryVisible, which sends setHiddenRendererPty(id, false), so a surviving runtime-owned mark is cleared as soon as a view owns it. That also means the visible report still wins for pacing, and the !visibleRendererPtys.has(id) guard keeps the two paths consistent.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
|
Re CodeRabbit's outside-diff comment "Avoid re-marking a PTY that is already visible" (spawn-hidden-delivery.ts:29-43): valid, fixed in 1806b86. A view can mount visible while the provider spawn is still pending. That mount sends |
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes — the new commit 1806b86 (range-diff against the prior pullfrog review at d2eaedf), plus the renderer visible-handoff path it depends on.
- Skip re-hiding a visible-during-spawn PTY —
commitRuntimeSpawnHiddenDeliverynow returns early whenvisibleRendererPtys.has(id): it releases the pre-spawn mark and callssyncPtyBackgroundedDelivery(id, 'spawn')instead of re-marking hidden, so a pane that mounted visible before spawn committed does not have its bytes dropped. - Test — the runtime hidden-at-spawn suite gains a case that reports
visible: trueplussetHiddenRendererPty(false)whilespawn()is gated, then assertsisHiddenRendererPty(result.id)is false and a subsequent chunk is delivered aspty:data. It fails without the guard.
The visible case skips closeStartupQueryAuthorityForPty, which is safe because the pty:setRendererPtyVisible listener already closed authority for that id. Ran pty-runtime-hidden-at-spawn-mark.test.ts: 9/9 pass.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
1 participant
ELI5
Some terminal programs ask the terminal a question when they start, such as "where is the cursor?", and wait for the answer. When Orca creates a terminal in the background (an orchestration worker, or
orca terminal create) before you've opened any worktree, nobody answered. Muse Code gives up and quits when that happens. Orca now answers those questions until the terminal is shown on screen.What Changed
orca orchestration worker-start --agent muse …failed withagent_readiness: timeoutafter 60 s. Muse printed nothing and exited 0.ready / input_acceptedin about 4.6 s. The worker runs its task and reportsworker_done.terminal-model-query-authority.ts).initiallyHidden), the same way the renderer's existing hidden-at-spawn path works:src/main/ipc/pty/runtime/spawn-hidden-delivery.ts.orca-runtime-create-terminal.tspasses the flag.Why
We captured Muse 1.3 in a bare terminal. It sends colour, device-attribute and cursor-position queries, waits about 2 s on each unanswered cursor-position query, then exits 0 after about 6.4 s. Answering the cursor-position query is enough for it to start. Codex in the same bare terminal keeps running and just draws late, so Muse is the agent this breaks. But every agent in a background terminal got no answers, so the right fix is to answer, not a Muse-specific workaround. Reusing the existing hidden-at-spawn path means the handover when you open the tab behaves exactly like it already does for hidden panes the renderer creates.
Linked Issue
Found while validating #22383 (Muse orchestration workers, #19823).
Visual Proof
Before: fresh app on the landing screen (no worktree opened). The worker never came up, and
worker-startreturned:After: same fresh-app setup, still on the landing screen when the worker starts:
worker-startreturned"state": "ready", "stage": "input_accepted". Opening the worker's tab afterwards shows Muse rendered cleanly, with no stray^[[1;1Rreply characters. It did the task and sentworker_done, and the footer readsmuse-spark-1.3 · minimal:Testing
I manually tested these changes locally
Automated tests added/updated, or explained why not below
Manual test (macOS): in two separately launched dev apps, each with a fresh profile and still on the landing screen,
worker-start --agent musereachedreadyin about 4.6 s.hello.txtwas created andworker_donesettled the task.Automated tests:
src/main/ipc/pty-runtime-hidden-at-spawn-mark.test.tshas 5 cases. One checks that main answersESC[6nwithESC[1;1Rfor a terminal no renderer has opened, and it fails without the flag. Thesrc/main/ipcsuites (4,146 tests),tc:nodeand the changed-code quality gate pass.AI Disclosure
Anthropic Claude assisted with implementation, validation, and this pull request description.
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
Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (tc:node and the ipc suites pass locally; CI covers the rest)