fix(cli): decide duplicate-serve before macOS aborts pre-JS (STA-4336) - #14768
fix(cli): decide duplicate-serve before macOS aborts pre-JS (STA-4336)#14768Jinwoo-H wants to merge 15 commits into
Conversation
On macOS the Electron main constructs NSApplication before any JS runs, and _RegisterApplication calls abort() when Launch Services is unreachable. The "one Orca per userData profile" rule lives inside that main, so a duplicate `orca serve` never reaches it: the process SIGABRTs pre-JS and a supervisor that retries loops on it. Each abort also wedges the next GUI app launch for ~45s, so the loop degrades launches machine-wide. Move the decision to the CLI, before the exec: - `serve` preflights the profile and returns the existing exit code 3 without spawning, printing the RPC failure envelope for --json/--recipe-json. - `open` watches the detached launch and reports a classified runtime_open_failed instead of a 15s "no window" timeout. - `isProcessRunning` treats EPERM as alive, so another user's serve still counts as the owner. - The macOS signal diagnostic now names _RegisterApplication and says retrying cannot help. Adds a byte-identical oracle (config/scripts/run-macos-launch-abort-oracle.mjs) that denies the Launch Services mach services with sandbox-exec and grades six entrypoints, plus a reliability gate for the invariant.
…ackaged build The CLI spawns ORCA_APP_EXECUTABLE with no profile switch, and only ORCA_DEV_USER_DATA_PATH redirects userData — which a packaged main ignores. Run the CLI against a wrapper that pins --user-data-dir instead, so the packaged arm cannot reach the real profile. The wrapper sits at a .app/Contents/MacOS/ path so the CLI's bundle-shape branch is unchanged. Records the four-arm red/green/red evidence: packaged 1.4.177, latest main 92fb276 (already contains #12212), and the candidate with the fix reverted are each 4/6 red; the candidate is 6/6 green.
An unreachable owner is believed on its recorded pid alone, so a pid the OS recycled would refuse every serve on that profile forever with no way out of the message. Records the concurrent-serve race in the gate's known gaps.
|
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:
📝 WalkthroughWalkthroughThe change adds duplicate serving-profile detection with structured and human-readable refusal responses. Detached launches now expose early exit information, and 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 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: 3
🧹 Nitpick comments (3)
src/shared/single-instance-exit-code.ts (1)
1-3: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueShorten the implementation comments.
Keep each comment to one brief, non-obvious reason. Move incident history and detailed behavior explanations to documentation or tests.
src/shared/single-instance-exit-code.ts#L1-L3: reduce the rationale to one line.src/cli/runtime/status.ts#L110-L112: reduce the EPERM rationale to one line.src/cli/runtime/serving-profile-owner.ts#L10-L17: reduce the startup-boundary rationale to one line.src/cli/runtime/serving-profile-owner.ts#L31-L34: reduce the JSON-output rationale to one line.src/cli/runtime/launch.ts#L80-L83: reduce the launch-failure rationale to one line.src/cli/runtime/launch.ts#L109-L112: reduce the duplicate-refusal rationale to one line.src/cli/runtime/serve-duplicate-refusal.test.ts#L13-L18: reduce the test scenario rationale to one line.src/cli/runtime/serve-duplicate-refusal.test.ts#L45-L47: reduce the fixture rationale to one line.src/cli/runtime/serve-duplicate-refusal.test.ts#L68-L69: reduce the JSON-output rationale to one line.src/cli/runtime/launch.test.ts#L72-L73: reduce the async setup rationale to one line.src/cli/runtime/launch.test.ts#L93-L95: reduce the isolated-profile rationale to one line.As per coding guidelines: “Comments must be concise, non-obvious, and brief—prefer one line.”
Source: Coding guidelines
config/scripts/run-macos-launch-abort-oracle.mjs (1)
387-399: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winEscape single quotes when you build the wrapper script.
executableWrapperScriptplaceselectronandprofileinside single-quoted/bin/shwords. A single quote in the--electronvalue ends the quoting, and the rest of the path is then interpreted as shell code. The wrapper then either fails with a confusing syntax error or runs unintended commands.♻️ Proposed escaping
+function shellSingleQuote(value) { + return `'${value.replaceAll("'", `'\\''`)}'` +} + export function executableWrapperScript(electron, profile) { - return `#!/bin/sh\nexec '${electron}' '--user-data-dir=${profile}' "$@"\n` + return `#!/bin/sh\nexec ${shellSingleQuote(electron)} ${shellSingleQuote(`--user-data-dir=${profile}`)} "$@"\n` }config/scripts/run-macos-launch-abort-oracle.test.mjs (1)
3-3: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPin the duplicated exit code to the shared constant.
run-macos-launch-abort-oracle.mjsduplicates the value3because it must run against packaged builds with no repo on disk. This test always runs inside the repo, so it can prove the copy still matches. Without that assertion, a change toSINGLE_INSTANCE_ALREADY_RUNNING_EXIT_CODEleaves the oracle grading duplicate scenarios against a stale exit code and every duplicate scenario fails for the wrong reason.💚 Proposed drift pin
+import { SINGLE_INSTANCE_ALREADY_RUNNING_EXIT_CODE } from '../../src/shared/single-instance-exit-code' + +it('keeps the duplicated exit code aligned with the shared constant', () => { + expect(ALREADY_RUNNING_EXIT_CODE).toBe(SINGLE_INSTANCE_ALREADY_RUNNING_EXIT_CODE) +})Run the following script to confirm the exported name and path of the shared constant:
#!/bin/bash # Confirm the shared single-instance exit-code export. fd -t f 'single-instance-exit-code*' src rg -nP -C3 'ALREADY_RUNNING_EXIT_CODE' src
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: b8638a72-f398-43b7-989e-997735689bd0
📒 Files selected for processing (16)
config/reliability-gates.jsoncconfig/scripts/run-macos-launch-abort-oracle.mjsconfig/scripts/run-macos-launch-abort-oracle.test.mjssrc/cli/runtime/client.tssrc/cli/runtime/launch.test.tssrc/cli/runtime/launch.tssrc/cli/runtime/open-launch-failure.test.tssrc/cli/runtime/serve-duplicate-refusal.test.tssrc/cli/runtime/serve-signal-exit-diagnostic.test.tssrc/cli/runtime/serve-signal-exit-diagnostic.tssrc/cli/runtime/serving-profile-owner.test.tssrc/cli/runtime/serving-profile-owner.tssrc/cli/runtime/status.test.tssrc/cli/runtime/status.tssrc/main/startup/single-instance-lock.tssrc/shared/single-instance-exit-code.ts
Cross-model review found two defects in the refusal added here: - An unreachable owner was believed on its recorded pid alone. Once the OS recycled that pid the claim was permanent, so `orca serve` would exit 3 forever against a profile with no runtime. The belief is now bounded to a startup window; a reachable owner is still definitive at any age. - `--recipe-json` refusals wrote an RPC failure envelope to recipe stdout, which carries a strict result schema — the serve path already diverts even schema-valid non-orca-server results to stderr. The refusal now goes to stderr and exit 3 carries the signal. Also adopts the repo's "only ESRCH proves death" pid rule, replaces the environment-dependent pid-1 EPERM test with a controlled one, and renames serve-signal-exit-diagnostic to launch-exit-diagnostic now that it classifies both serve and open.
process.kill throws a TypeError with no errno for a pid the OS will not accept — a non-integer or out-of-range value in a corrupt orca-runtime.json. The "only ESRCH proves death" rule read that as alive, which would refuse serve on a profile with nothing running.
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: f0eae234-58fe-481c-acaf-c3ed475ce077
📒 Files selected for processing (5)
config/reliability-gates.jsoncsrc/cli/runtime/launch.tssrc/cli/runtime/serve-duplicate-refusal.test.tssrc/cli/runtime/serving-profile-owner.test.tssrc/cli/runtime/serving-profile-owner.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- src/cli/runtime/serving-profile-owner.test.ts
- src/cli/runtime/serve-duplicate-refusal.test.ts
- config/reliability-gates.jsonc
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 954edbaa-5e46-4ac3-91a2-72dd4eac6f12
📒 Files selected for processing (11)
config/reliability-gates.jsoncsrc/cli/runtime/client.tssrc/cli/runtime/launch-exit-diagnostic.test.tssrc/cli/runtime/launch-exit-diagnostic.tssrc/cli/runtime/launch.tssrc/cli/runtime/serve-duplicate-refusal.test.tssrc/cli/runtime/serve-update-supervisor.tssrc/cli/runtime/serving-profile-owner.test.tssrc/cli/runtime/serving-profile-owner.tssrc/cli/runtime/status.test.tssrc/cli/runtime/status.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- src/cli/runtime/client.ts
- src/cli/runtime/status.ts
- config/reliability-gates.jsonc
…STA-4336) `orca serve` refused a profile whenever metadata named a live pid, so a crashed runtime whose pid the OS recycled refused every serve on that profile forever. Bounding that belief to a startup window traded one defect for another: a healthy serve older than the window stopped refusing, and a supervisor retrying into it on a Launch-Services-broken macOS host is exactly the pre-JS SIGABRT loop this fixes. Ownership is now only ever asserted on a live runtime answering for itself — an RPC reply, or failing that a bounded 250ms connect to the endpoint it published. The runtime binds its transports before writing metadata, so an accepted connect proves a live owner even while it is too busy to answer RPC; a crash leaves the socket path behind with nothing to accept on it. The recorded pid names the owner in the message and never establishes one. Also stop discarding detached spawn errors: a command that never starts emits `error` and no `exit`, so `orca open` waited out its full 15s window and then blamed a missing window for a process that was never created.
CodeRabbit round 1 — responses1. Confirmed by reading the code: 2. The analysis is right: Two things bound how much this matters here, both measured rather than assumed:
Following the app's real lifetime means tracking an |
A single quote in an `--electron` or profile path closed the quoting in the generated /bin/sh wrapper and ran the rest as shell code. Verified end to end: a profile of `/tmp/o'brien; touch /tmp/PWNED` now reaches the wrapper as one argument. Also compresses the rationales AGENTS.md asks to keep to one line.
There was a problem hiding this comment.
Actionable comments posted: 4
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 3c34af32-67bc-40de-be2b-3ad774e17c61
📒 Files selected for processing (32)
.github/CONTRIBUTING.md.github/pull_request_template.md.github/workflows/pullfrog.ymlAGENTS.mdREADME.mdconfig/electron-builder.config.cjsconfig/reliability-gates.jsoncdocs/STYLEGUIDE.mddocs/reference/git-compatibility.mddocs/reference/headless-linux-server.mddocs/reference/linux-glibc-compatibility.mddocs/reference/windows-setup-shell.mddocs/reference/worktree-scan-fingerprint.mdskill-guides/orca-emulator-android.mdskill-guides/orca-emulator.mdskill-guides/orca-per-workspace-env.mdsrc/cli/runtime/launch-exit-diagnostic.test.tssrc/cli/runtime/launch-exit-diagnostic.tssrc/cli/runtime/launch.test.tssrc/cli/runtime/launch.tssrc/cli/runtime/runtime-listener-probe.test.tssrc/cli/runtime/runtime-listener-probe.tssrc/cli/runtime/serve-duplicate-refusal.test.tssrc/cli/runtime/serving-profile-owner.test.tssrc/cli/runtime/serving-profile-owner.tssrc/cli/runtime/status.test.tstests/e2e/fixtures/terminal-emoji-table.mdtests/e2e/ssh-config-host-picker.PLAN.mdtests/tools/daemon-relocation-spike/README.mdtests/tools/repro-watcher-crash-7547/fixed-child.cjstests/tools/repro-watcher-crash-7547/run.cjstests/tools/win-update-e2e/README.md
💤 Files with no reviewable changes (1)
- .github/CONTRIBUTING.md
🚧 Files skipped from review as they are similar to previous changes (4)
- src/cli/runtime/launch-exit-diagnostic.test.ts
- src/cli/runtime/status.test.ts
- config/reliability-gates.jsonc
- src/cli/runtime/launch-exit-diagnostic.ts
|
|
||
| # Style | ||
|
|
||
| ## Concise/Brief Non-obviosu comments ONLY |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Correct the heading typo.
Line [11] contains Non-obviosu. Change it to Non-obvious.
| const owner = await findServingProfileOwner( | ||
| (await getCliStatus(userDataPath)).result, | ||
| // Why: re-reading is deliberate. A runtime that removed its metadata in the | ||
| // meantime is shutting down and a fresh serve should proceed; one that | ||
| // rewrote it is newer, and that is the endpoint worth probing. | ||
| tryReadMetadata(userDataPath) | ||
| ) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Make duplicate detection atomic.
Two concurrent serve commands can both observe no owner before either child publishes metadata. Both commands then spawn Electron. The losing process can still hit the post-NSApplication single-instance path and abort on macOS.
Acquire a per-profile pre-launch lock or reservation before this check. Transfer or release it deterministically during owner startup. Add a concurrent-start test that asserts one launch and one exit code 3.
🧰 Tools
🪛 ast-grep (0.45.1)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { spawn as spawnProcess, type SpawnOptions } from 'node:child_process'
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/cli/runtime/launch.ts (1)
87-95: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueKeep rationale comments to one line.
src/cli/runtime/launch.ts#L87-L95: Compress each// Why:rationale into one line.src/cli/runtime/launch.ts#L117-L126: Compress the ownership-refusal rationale into one line.
As per coding guidelines: “BE CONCISE. 1 LINE if possible.”Source: Coding guidelines
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 9d86287b-0b2f-4c88-b09d-04e92a6abaef
📒 Files selected for processing (9)
config/scripts/run-macos-launch-abort-oracle.mjsconfig/scripts/run-macos-launch-abort-oracle.test.mjssrc/cli/runtime/launch.test.tssrc/cli/runtime/launch.tssrc/cli/runtime/runtime-listener-probe.tssrc/cli/runtime/serve-duplicate-refusal.test.tssrc/cli/runtime/serving-profile-owner.tssrc/cli/runtime/status.tssrc/shared/single-instance-exit-code.ts
🚧 Files skipped from review as they are similar to previous changes (8)
- src/shared/single-instance-exit-code.ts
- src/cli/runtime/status.ts
- src/cli/runtime/serve-duplicate-refusal.test.ts
- src/cli/runtime/serving-profile-owner.ts
- src/cli/runtime/runtime-listener-probe.ts
- config/scripts/run-macos-launch-abort-oracle.test.mjs
- src/cli/runtime/launch.test.ts
- config/scripts/run-macos-launch-abort-oracle.mjs
The listener probe returned a boolean, so every outcome that was not a completed connect — a timed-out connect, EACCES, any unexpected errno — read as "nobody owns this profile" and let serve spawn a second GUI Electron main. On a host where Launch Services is unreachable that is the pre-JS SIGABRT this change exists to prevent, so the ambiguous case was failing open into the bug. Measured on macOS: a runtime that stops unlinks its socket (ENOENT, 0.08ms) and one that is killed leaves a path that refuses (ECONNREFUSED, 0.16ms). Every way an owner really goes away is definitive and immediate, so those two codes now free the profile and nothing else does. Same rule `orca status` already applies to pids, where only ESRCH proves the process is gone. The refusal names the endpoint-held case separately and points at the one manual escape hatch it needs, since it is the only refusal a user may have to clear. Oracle candidate-w7: 6/6 green.
A consumed `once('error')` leaves the socket with no listener, and a second
error event on a destroyed socket then throws uncaught — verified: the process
dies. The probe already ignores anything after it settles, so hold the listener.
…wrote A `pnpm format` run reflowed three checked-in generator outputs (markdown table padding and emphasis markers), which the generator then no longer reproduced. These are build artifacts, not hand-edited files.
…(STA-4336) orca-runtime.json is parsed with no validation, so the probe fed whatever the file held straight to createConnection. A missing or blank endpoint throws synchronously (ERR_MISSING_ARGS / ERR_SOCKET_BAD_PORT) and would fail serve outright, and a numeric one is read as a TCP port — the probe would dial 127.0.0.1:42 and could be answered by something that is not Orca. Junk metadata leaves nothing to probe, so it frees the profile rather than refusing forever; the in-process single-instance lock stays the backstop. Also covers the ORCA_OPEN_COMMAND launch branch, which the exit watcher already handled but no test exercised.
CodeRabbit round 2 — responses1.
The synchronous throws happen inside the promise executor, so they rejected the probe and failed 2. Changing the existing test's env would have dropped the 3. Two The suggested fix — a per-profile reservation before the exec — needs a lock that the OS releases when its holder dies. Node has no What this PR does cover is the shape that actually crash-loops: a supervisor restarting 4. Oracle re-run after this round: |
…ta rule (STA-4336)
A repo-wide `pnpm format` had reflowed 18 files this change never touches. Restores them to main so the diff is only the CLI launch routing.
…10 (STA-4336) The fix-disabled arm is rebuilt at the branch's merge base, so the red and green runs are the same machine and the same harness minutes apart.
1 participant
Fixes STA-4336. Closes #14541.
The bug
On macOS the Electron main constructs
NSApplicationbefore any JavaScript runs, and HIServices_RegisterApplicationcallsabort()when Launch Services is unreachable (no GUI login session, an SSH/sandbox-execcontext, a wedgedlsd). Orca's "one Orca per userData profile" rule lives inside that main, so a duplicateorca servenever reaches it — the process SIGABRTs before the V8 isolate exists and a supervisor that retries loops on it. Upstream: electron/electron#52815.Each abort also leaves Launch Services unable to register the next GUI app for ~45s, machine-wide (measured on macOS 25.5), so the retry loop degrades launches for everything else on the machine — not just Orca.
PR #12212 is already merged, and latest main still reproduces: it fixed the in-process path, but the decision has to move before the exec to help.
The fix
Move the duplicate decision into the CLI, ahead of the launch:
servepreflights the profile and returns the existing exit code 3 without spawning, printing the RPC failure envelope for--json(recipe stdout is a strict schema channel, so its refusal goes to stderr and exit 3 carries the signal).ENOENT(a runtime that unlinked its socket on shutdown) orECONNREFUSED(a path a killed runtime left behind). Measured on macOS, both land in under 1ms, so every way an owner really goes away is immediate and unambiguous. Every other outcome — the 250ms cap expiring,EACCES, an unexpected errno — is treated as an owner, because it proves nothing and guessing "free" is what spawns the second main that aborts pre-JS. That refusal names the one manual escape hatch (deleteorca-runtime.json) since it is the only one a user may have to clear by hand.openwatches the detached launch and reports a classifiedruntime_open_failedat the first failed exit instead of waiting out its 15s "no window" timeout.orca statustreatsEPERMas alive (another user's serve on a shared host is still running) but no longer reads a pid the OS rejects outright as one.errorand noexitthere, soorca openused to wait out its full window and blame a missing window for a process that was never created._RegisterApplicationand states that retrying cannot help.Exit 3 is the existing
SINGLE_INSTANCE_ALREADY_RUNNING_EXIT_CODEthe Electron main already returns and that the documented systemd unit keysRestartPreventExitStatus=off, so a supervisor now stops instead of looping. The constant moved tosrc/shared/so the CLI can reuse it without importing a main-process module.Falsifiable invariant
Evidence
config/scripts/run-macos-launch-abort-oracle.mjsdenies exactly the four Launch Services mach services with asandbox-execseatbelt profile that is otherwise(allow default), so a difference in outcome can only come from Launch Services being unreachable. It grades six entrypoints and is byte-identical across arms — point--electron/--cliat any build.92fb276040(already contains #12212)c0a775454a= this branch with the fix removedThe third arm is a fresh build of
c0a775454a, this branch's merge base. The branch diff against that commit is exactly this change and nothing else, so one build serves as both the refreshed main arm and the fix-disabled arm — and it ran on the same machine, through the same harness, minutes before the green arm below.Red detail (
disabled-c0a7754, matchingmain-92fb2760scenario for scenario):This branch (
candidate-w10, final head):Both unsandboxed scenarios stay green on every arm, which is the point: the in-process rule is correct, it just sits behind the boundary that aborts.
The packaged arm is profile-safe because the harness points the CLI at a wrapper that execs the build under test with
--user-data-dirpinned to the run profile — a packaged main ignoresORCA_DEV_USER_DATA_PATH. The wrapper lives at a<name>.app/Contents/MacOS/path so the CLI's bundle-shape branch (serve-update handoff) takes the same path a real bundle would.Coverage
New reliability gate
cli-launch.macos-pre-js-abort-refusalwith the invariant, oracle, four-arm red/green evidence, runtime budget, and promotion criteria. Deterministic vitest coverage for the refusal wiring, the JSON envelope, owner classification, theEPERMownership case, theopenclassification, and the diagnostic wording runs on every platform shard; thesandbox-execscenarios are macOS-only and local, because PR CI unit shards are Linux.Scope notes
orca opengets strictly faster on this path (fails at the first launch exit instead of 15s).Relation to #14542
#14542 was treated as an untrusted hypothesis and was not used as a base; this branch was written against the oracle above and its byte-identical arms.
Do not merge — review only for now.
Review round 1 (cross-model, GPT-5.6 via
codex exec)Three independent reviewers — adversarial, performance, elegance — ran against
git diff 5b7f44278a..HEAD. Confirmed findings and what changed:orca serveexits 3 forever with no runtimefindServingProfileOwnernow disbelieves an unreachable owner more thanSTARTING_OWNER_TRUST_WINDOW_MS(120s) pastmetadata.startedAt. A reachable owner stays definitive at any age. The refusal message also namesorca-runtime.jsonso the manual escape hatch is always visible.--recipe-jsonrefusal wrote an RPC envelope to recipe stdout, which carries a strict result schema (the serve path already diverts even schema-valid non-orca-serverresults to stderr)--jsonwrites the stdout envelope. Exit 3 carries the signal. Regression test asserts stdout is untouched.orca openon a packaged macOS build goes through/usr/bin/open, which exits 0 as soon as Launch Services accepts — so the new exit watcher cannot see a later Electron abortknownGaps. Verified separately that a Launch-Services-denied seatbelt does not make/usr/bin/openfail, because the requested app is launched outside the caller's sandbox. The classification covers the direct-exec path; packagedopenstill falls back to the 15s no-window timeout.open-duplicateaccepts eitherruntime_open_failedordesktop_activation_blockedand never exercises/usr/bin/openknownGapsnow names the/usr/bin/openlimitation explicitly.process.kill(pid, 0)errno handlingprocess.killthrow aTypeErrorwith no errno, which read as alive. Now requires a numericerrnobefore believing an owner.orca statusEPERM test pinned pid 1process.killfor EPERM and ESRCH, plus a real non-integer-pid case.serve-signal-exit-diagnostic.tsnow classifies both serve and openlaunch-exit-diagnostic.ts._RegisterApplicationthough the parent only observes a signalservepath (the same readorca statusalready does), oneonce('exit')listener.orca openon the direct-exec path is ~14.6s faster.performanceBudgetcorrected — it previously claimed "no IPC", but the preflight does use the existing local RPC socket.serving-profile-owner.ts, route throughreportCliError"--jsonconsumers and the oracle assert on, in exchange for line count.Oracle re-run after every round: 6/6 green (
candidate-w3,candidate-w4).Review round 2
CodeRabbit (GitHub):
spawnDetacheddiscardedchild.once('error'), so a command that never starts leftfailedExit()null andorca openwaited out its 15s windowfailedExit()now carriesspawnError, andopenLaunchExitErrorreports it without the Launch Services guidance — nothing ran, so crash reports andmach-lookupwould send the user to the wrong place. Mutation-checked: reverting the handler or dropping the branch fails the new tests.openlifecycle is not followed/usr/bin/openfail (the app launches outside the caller's sandbox), so this needs machine-wide LS breakage rather than the sandboxed-agent case; andorca servenever goes through/usr/bin/open. Following the real lifetime means trackingNSRunningApplicationoropen -W, which changes detach semantics for every packaged launch. ExplicitknownGapsentry./bin/shwords without escaping/tmp/o'brien; touch /tmp/PWNEDnow reaches the wrapper as one argument, no injection.orca statusEPERM test pinned pid 1process.killwith faithful errnos, plus a real non-integer-pid case). CodeRabbit marked it addressed.Self-review that changed the design. Round 1's 120s trust window fixed the permanent false refusal but reopened the headline failure mode in a narrower form:
metadata.startedAtis written once at runtime construction and never refreshed, so a healthy serve older than 120s whose RPC went quiet for one second stopped refusing — and a supervisor retrying into it on a Launch-Services-broken host is exactly the pre-JS SIGABRT loop this PR exists to stop (a SIGABRT exit is not exit 3, soRestartPreventExitStatus=3does not catch it). Replacing the window with the listener probe removes both defects and deletes the heuristic rather than tuning it. Verified myself, not assumed:runtime-rpc.tsstarts its transports beforewriteMetadata(), and every laterwriteMetadata()call site mutates only the websocket entry, always registering the live transport first. The local endpoint in metadata was always bound before the write.findTransport(metadata, 'unix', 'named-pipe')cannot return a websocket, so the probe is local-only and never reachable off-box.Mutation testing. Every new assertion was checked by breaking the production code it covers: trusting the pid instead of the listener (2 fail), dropping the listening tier (5 fail), accepting on connect error (2 fail), discarding spawn errors (1 fail), dropping the
spawnErrordiagnostic branch (1 fail). None pass vacuously.Oracle re-run after every round: 6/6 green (
candidate-w3…candidate-w6).Review round 3
Self-review found a fail-open in round 2's own fix. The listener probe returned a boolean, so only a completed connect meant "owner" and everything else — a timed-out connect,
EACCES, any unexpected errno — meant "this profile is free, go spawn". On a host where Launch Services is unreachable, that is the pre-JS SIGABRT this PR exists to prevent: the ambiguous case was failing open into the bug.I measured the real outcomes rather than reasoning about them (macOS, local unix sockets):
connectENOENTSIGKILLed, socket file still on diskECONNREFUSEDEACCESEvery way an owner really goes away is definitive and sub-millisecond, so the probe became a tri-state and only those two codes free the profile. This is the same rule
orca statusalready applies one layer up, where onlyESRCHproves a pid is gone. The 250ms cap is now a ceiling nobody pays rather than a silent verdict.Also hardened: the probe held
once('error'), so a second error event on a socket destroyed mid-connect would have had no listener and taken the CLI down with an uncaught exception (verified — the process dies). It holdson('error')now and ignores anything after it settles.Supervisor re-entry checked.
superviseForegroundServedoes loop and respawn, but only when the handoff file readsinstall-requestedand names the exiting child's pid. A pre-JS abort runs no JavaScript and so writes no handoff, which means it exits throughserveSignalExitErrorand its classified_RegisterApplicationdiagnostic instead. The CLI cannot become the retry loop itself.Mutation testing of the new branches (all killed): timeout frees the profile → 1 failed; any connect error frees it → 1 failed;
unproventreated as not-an-owner → 1 failed.Cross-model review was unavailable for this round. Two
codex execpasses were started and both wedged in gateway reconnect loops without producing a single line of review — 56 minutes and 14 minutes. A one-line liveness prompt then failed to return in 180s, confirming the gateway is down rather than the prompts being at fault; both were stopped. Round 3 is therefore my own adversarial pass plus CodeRabbit, and I have said so rather than implying a second model signed off.Oracle re-run on the redesign:
candidate-w76/6 green (duplicate serve 97ms, duplicate serve with a reachable owner 87ms,--recipe-json101ms, all exit 3 without spawning;openclassified in 604ms; fresh sandboxed serve failed once with the diagnostic).Review round 4 (CodeRabbit)
tryReadMetadatais an unvalidatedJSON.parse, so the file's contents reachedcreateConnectiondirectly. Measured:undefinedthrowsERR_MISSING_ARGS,''/' 'throwERR_SOCKET_BAD_PORT— both inside the promise executor, so they failedorca serveoutright — and42does not throw at all, it is read as a TCP port and dials127.0.0.1:42, where something that is not Orca could accept and produce a false refusal. The probe now requires a non-blank string endpoint; anything else frees the profile, because refusing forever on junk metadata is the permanent-refusal failure mode, and the in-process lock is still the backstop./usr/bin/openbranch untested (Minor) — added, as a new case rather than by repurposing theORCA_APP_EXECUTABLEtest.serveruns racing on an ownerless profile can both spawn. A pre-launch reservation needs a lock the OS releases on holder death; Node has noflock, so a pure-JS lock is a file plus a staleness rule — which is precisely the unsound time window round 2 deleted, and a stale lock refuses serve permanently. The shape that actually crash-loops (a supervisor restarting against a profile that already has an owner) is sequential and is fixed here.Base merged with
main(c0a775454a) — two CI failures on the previous head were stale-base, not this change:static analysiswas fixed on main by #14755, and the failing test shard was three generatedskill-guides/*.mdartifacts that a repo-widepnpm formathad reflowed out of sync with their generator. The artifacts are restored.Oracle at the final head:
candidate-w96/6 green.Review round 5
pnpm formatrun —README.md,AGENTS.md,docs/,.github/,config/electron-builder.config.cjs, and severaltests/tools/fixtures. All restored tomain, so the diff is now exactly the 20 files that implement and cover the fix. (CodeRabbit'sAGENTS.mdtypo note went away with them; that line is main's, not this branch's.)c0a775454aand re-graded: same four failures, same harness, same machine, minutes beforecandidate-w10went 6/6 green.Oracle at the final head:
candidate-w106/6 green.