Repository navigation
feat(orcad): supervisable server — stop requests, managed stop receipts, lock-safe lifetime (#16741 T6-3) - #24433
Conversation
976ae08 to
ff863ae
Compare
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
Important
One concurrency gap in the stale-lock reclaim: it can strand a live successor's lock record and leave two live orcad runtimes on one data root (details inline). Plus one design question on refused managed stops below.
Reviewed changes
- Stop-request files — a slot file (
.orcad-stop-request) and an instance-bound file keyed by the lock nonce; both route through the same bounded stop asSIGTERM, and the managed request is validated against version/runtimeId/instance and the live lock before it acts. --complete-managed-stop— a short-lived command that runs before the bundled handoff/preflights, writes the request only while the lock still names the instance, polls, and prints one JSON verdict line (live/unverifiable/exited) with exit codes 0/64/1.- Receipts — completed receipts written only on proven exit; refused receipts when a retire-daemon stop cannot be done safely.
- Daemon retirement — maps
requestIdleDaemonRetirement/listLiveDaemonSessionstoretiring/live/unverifiable; never guesses idle. - Instance lock — schema-checked, size-bounded record that fails closed as
orcad_instance_lock_unreadable(exit 78), plus a successor-nonce re-check and best-effort restore on the stale reclaim path. - Runtime lifetime — reverse-order cleanups via
OrcadRuntimeLifetime; any failed cleanup retains the instance lock and profile admission; failing quit handlers are now a failed teardown. - Browser startup — discovery is backgrounded so RPC readiness does not wait on it;
AbortSignalthreads through provider start and cancelled providers are cleaned, with cleanup failures propagated. - T2 deferrals — legacy-worker recovery
stop()drains active/background work and refuses new passes; headless terminal close routes through the non-throwingnotifyRendererOfHeadlessTerminalCloseand guardstabs.has(tabId)/ toleratestab_not_found.
ℹ️ A refused managed stop looks terminal for that instance
When a retireIdleDaemon stop is refused, the request file is kept as evidence and the refused receipt is written. A retry is then blocked either way: reusing the same transactionId short-circuits on the stale refused receipt (completeOrcadManagedStop reads it before doing anything), and a new transactionId is rejected by existingRequestMatches because the request path is keyed to the instance nonce, so the old request is still sitting there. Only an orcad restart (which mints a new nonce and therefore a new request path) clears it. Since the refusal case is exactly "the daemon is busy now, try when it is idle," this seems like it should be retryable in place. The tests codify the current behavior, so this may be deliberate and deferred to T6-4's cancel path — flagging to confirm.
Technical details
# Refused managed stop is not retryable within one instance lifetime
## Affected sites
- `src/main/orcad/orcad-managed-stop-completion.ts` — `readOrcadRefusedStopReceipt(request)` returns the prior refusal before the request is (re)written; a same-transaction retry returns immediately.
- `src/main/orcad/orcad-managed-stop-completion.ts` — `existingRequestMatches` returns false for a different transaction and yields `unverifiable` without writing.
- `src/main/orcad/orcad-managed-stop-admission.ts` — refusal path persists the receipt but never removes/rotates the request file.
- Tests locking in the behavior: `orcad-managed-stop.test.ts` "keeps orcad running on a busy daemon…" and "does not overwrite another transaction pending for the same instance".
## Required outcome
- Decide whether a refused retire-daemon stop is retryable while the same `orcad` is running. If yes, the request/refusal for a completed transaction must not permanently block a later request for the same instance.
## Open questions for the human
- Is retry-in-place intended to be T6-4's cancel-a-prepared-stop path, or should a later attempt with a fresh `transactionId` be allowed to replace a finished (refused) transaction's request file?deepseek-v4.1-flash (free via Pullfrog for OSS) ｜ 𝕏
| /** No-clobber restore: a third contender's newer record at the canonical path stays authoritative. */ | ||
| function restoreDisplacedLock( | ||
| claimPath: string, | ||
| lockPath: string, | ||
| claimedContents: string | null | ||
| ): void { | ||
| try { | ||
| linkSync(claimPath, lockPath) | ||
| unlinkSync(claimPath) | ||
| } catch { | ||
| if (claimedContents === null) { | ||
| return | ||
| } | ||
| try { | ||
| writeFileSync(lockPath, claimedContents, { flag: 'wx', mode: 0o600 }) | ||
| unlinkSync(claimPath) | ||
| } catch { | ||
| // A newer contender won, or restoration is unavailable; fail closed. | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
restoreDisplacedLock is best-effort and is not atomic with the renameSync that displaced the record, so a successor's live record can be permanently stranded. If another orcad reclaimed the stale record and published its own between this process's liveness read and its renameSync, the rename moves the successor's live record to claimPath; if a third starter then claims the now-absent canonical path, both linkSync and the writeFileSync(..., 'wx') fallback hit EEXIST, the catch swallows it, and the successor keeps running with no record at the canonical path while the third process owns it. That is two live runtimes on one data root, and the successor's managed stop is then refused as orcad_managed_stop_instance_lock_changed.
Technical details
# Stale reclaim can strand a live successor's lock record
## Affected sites
- `src/main/orcad/orcad-instance-lock.ts` — `renameSync(lockPath, claimPath)` (line 263) then `restoreDisplacedLock` (line 274); restore at lines 300-320 swallows both failure paths and leaves `claimPath` behind.
- `src/main/orcad/orcad-instance-lock.ts` — `publish()` at line 215 lets any newcomer create the canonical record while A holds the successor's record at `claimPath`.
- `orcad-instance-lock.test.ts:100` — the successor-race test only covers two actors, where `linkSync` finds the canonical path absent and succeeds; it cannot reach this interleaving.
## Interleaving (A = reclaimer, C = successor that republished, D = later starter)
1. A: `publish()` EEXIST, reads stale record S (nonce nS), sees it dead.
2. C: reclaims S, publishes live record c at canonical.
3. A: `renameSync(lockPath, claimA)` moves c to claimA; canonical is now absent.
4. D: `publish()` succeeds on the absent canonical path; D is live.
5. A: reads claimA (c), nonce mismatch, `restoreDisplacedLock` — `linkSync` EEXIST, `writeFileSync wx` EEXIST, swallowed; claimA is never unlinked.
6. Net: canonical = D (live); C (live) exists only at `orcad.lock.stale-<pidA>-<uuid>`.
## Required outcome
- The reclaim must not be able to remove a successor's live record from the canonical path without either restoring it or failing in a way that cannot leave two live owners. Note the window in step 3: any newcomer's `publish()` succeeds while the canonical path is absent.
## Open questions for the human
- Is a three-way simultaneous start on one data root considered in scope, or is the current two-actor guarantee sufficient for now?
There was a problem hiding this comment.
Important
Two issues in the follow-up commit's daemon-retirement rework and the managed stop listener (details inline). The rework otherwise resolves the earlier "refused stop is terminal" concern cleanly.
Reviewed changes (since 976ae08)
- Daemon retirement is now best effort — a
retireIdleDaemonrequest never blocks or vetoes the stop; the daemon is retired only when it proves idle across every generation, otherwise it stays up,releaseDaemonRetirementFencereopens admission, and the outcome is recorded in<transactionId>.retirement.jsonfor the completion receipt. - Admission fence plumbing — new
releaseFence/releaseIdleRetirementFenceon the router and adapter, plus per-generationretirementAttempted/retiredbookkeeping. - Completion path simplified — the refused-receipt short-circuit is gone; the completed receipt carries an optional
retirementcopied from the retirement record (unverifiablewhen absent). - Managed listener —
beforeManagedStopruns once before the stop and cannot prevent it; the managed listener no longer keeps a per-transaction decision set.
deepseek-v4.1-flash (free via Pullfrog for OSS) ｜ 𝕏
| releaseFence(): void { | ||
| if (this.idleRetirementPromise || this.retired) { | ||
| return | ||
| } | ||
| this.admissionClosed = false | ||
| for (const adapter of this.allAdapters()) { | ||
| adapter.releaseIdleRetirementFence() | ||
| } | ||
| } |
There was a problem hiding this comment.
The timeout path cannot actually reopen admission. retireOrcadDaemonIfIdle calls releaseFence() after withTimeout yields {state:'timed-out'}, but this guard returns early while idleRetirementPromise is still pending, and Promise.race never cancels the underlying request. So a slow-but-alive daemon that does not answer within 5s is left with admissionClosed = true (router spawn throws Terminal daemon is decommissioning) and never reopens — contradicting the PR's stated invariant that orcad reopens new-terminal admission whenever retirement did not complete. The unit test for the "no answer" case stubs releaseFence, so it asserts only that the caller invoked it, not that the real fence reopened.
Technical details
# Timed-out daemon retirement leaves terminal admission fenced
## Affected sites
- `src/main/orcad/orcad-daemon-retirement.ts:51-60` — `withTimeout(request(), timeoutMs, {state:'timed-out'})`, then `releaseFence()`; the fallback resolves while `request()` is still pending.
- `src/main/daemon/daemon-router-retirement.ts:15-23` — `releaseFence` returns early when `this.idleRetirementPromise` is non-null.
- `src/main/daemon/daemon-pty-event-subscriptions.ts:150-154` — the adapter `releaseIdleRetirementFence` has the same guard.
- `src/main/daemon/daemon-pty-router.ts:46-49` — `spawn` refuses while `retirement.admissionClosed`.
- Test gap: `src/main/orcad/orcad-daemon-retirement.test.ts` ("no answer") stubs `releaseFence: vi.fn()`; `src/main/daemon/daemon-router-retirement.test.ts:50-63` exercises `releaseFence` only after the request has settled.
## Required outcome
- When a retirement attempt times out and the daemon stays up, admission must be reopened. The pending `idleRetirementPromise` must not permanently block the release, and if the request later completes without retiring, admission must still end open.
## Open questions for the human
- If a timed-out request may still retire the daemon, should `releaseFence` be re-attempted once that promise settles (and orcad has not yet exited), rather than keyed off the caller's timeout?| orcadManagedStopRequestPath(managedStop.instance), | ||
| (path) => { | ||
| const request = validateOrcadManagedStopRequest(managedStop, path) | ||
| close() |
There was a problem hiding this comment.
close() is referenced before its const is initialized. The managed listener's consume runs synchronously from listenForRequest's initial check(), which executes while the listeners.push(...) argument is still being evaluated — before const close at line 133 exists. A managed request present at install time therefore throws ReferenceError: Cannot access 'close' before initialization, which the listener logs as a refused request; it only recovers on the next 1s poll. The tests write the managed request after listen(...), so none hit this path.
Technical details
# TDZ: `close` used before initialization in the managed listener
## Affected sites
- `src/main/orcad/orcad-stop-request-listener.ts:119` — `close()` called synchronously inside `consume`.
- `src/main/orcad/orcad-stop-request-listener.ts:133` — `const close = ...` declared after the managed listener is created.
- `src/main/orcad/orcad-stop-request-listener.ts:80` — `listenForRequest` runs `check()` synchronously on construction.
## Required outcome
- The synchronous initial check of the managed listener must be able to close the listeners without a TDZ error (hoist `close`, declare it before the managed block, or have `consume` defer to the poll instead of closing synchronously).
## Suggested approach (optional)
- Move the `close` definition above the `listeners` array; the managed `consume` can then call it synchronously.
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| return { retirement: 'retired', liveSessions: 0, reason: null } | ||
| } | ||
| // A daemon that stays must not be left refusing new terminals. | ||
| releaseFence() |
There was a problem hiding this comment.
When a daemon-retirement request takes longer than five seconds, the timeout does not cancel it. This call to releaseFence() then does nothing because the retirement request is still in progress. If that request later finishes without retiring the daemon, orcad can exit while the daemon remains running but rejects new terminals. The fence needs to be released after the request settles if the daemon did not retire.
| if (!existingRequestMatches(requestPath, request)) { | ||
| return 'unverifiable' | ||
| } | ||
| if (!writeDurableSecureJsonFile(requestPath, request)) { | ||
| throw new Error('orcad_managed_stop_request_permissions_unconfirmed') | ||
| } | ||
| for (let attempt = 0; attempt < attempts; attempt++) { | ||
| await (options.sleep ?? (() => delay(250)))() | ||
| verdict = observeInstance(request.instance, options) | ||
| if (verdict !== 'live') { |
There was a problem hiding this comment.
Two completion commands targeting the same orcad instance use the same request file. Both can pass this existing-request check before either writes, and the later write replaces the earlier transaction's request. The displaced command can then report that the instance exited even though its own request was never accepted, losing its transaction-specific retirement outcome.
| orcadManagedStopRequestPath(managedStop.instance), | ||
| (path) => { | ||
| const request = validateOrcadManagedStopRequest(managedStop, path) | ||
| close() |
There was a problem hiding this comment.
If a valid managed request already exists when the listener starts, its initial check calls close() before that const has been initialized. The resulting error is logged as a refused request, and shutdown waits for the next poll instead of handling the request immediately. This adds an avoidable delay of up to one second.
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. 🧰 Additional context used📚 Code guidelines (1)No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Advanced Run ID: 📥 CommitsReviewing files that changed from the base of the PR and between f9428f670052a510598f6a845a270100ddeb9b14 and 755c52d. 📒 Files selected for processing (1)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthroughThe changes add file-based slot and instance-bound stop requests, process-exit verdicts, and durable receipts. Instance-bound requests validate runtime identity and the current lock record. Optional daemon retirement records whether retirement succeeded or whether sessions remain verifiable. Shutdown now runs registered cleanup steps and retains the instance lock after cleanup failures. Browser startup accepts cancellation signals. The changes also add recovery-controller shutdown, watcher-disposal waiting, and terminal-close handling for missing renderer tabs. Priority: ⬇️ Low Merge Risk: 🔵 Low · up to Shutdown currently waits for requested-release work, but its test does not reliably protect that behavior against regression. Strengthen the test before relying on it; no production shutdown failure was established. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The shutdown design adds stronger instance checks, exit verification, and protection against overlapping writers. No introduced security vulnerability was established. Deployment permissions and how supervisors use completion receipts remain unverified. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
Hardening Proposals
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Advanced
Run ID: 2ec48500-e108-48a8-ab3b-4166d1de55b0
📥 Commits
Reviewing files that changed from the base of the PR and between 92cb717 and ff863ae343200f4676b6d6a1174ac5096c449688.
📒 Files selected for processing (47)
docs/reference/orcad-operations.mdsrc/main/daemon/daemon-init.tssrc/main/daemon/daemon-provider-state.tssrc/main/daemon/daemon-pty-event-subscriptions.tssrc/main/daemon/daemon-pty-router-test-fixture.tssrc/main/daemon/daemon-pty-router.tssrc/main/daemon/daemon-router-retirement.test.tssrc/main/daemon/daemon-router-retirement.tssrc/main/ipc/parcel-watcher-process-dispose.test.tssrc/main/ipc/parcel-watcher-process.tssrc/main/orcad/electron-serve-browser-process.test.tssrc/main/orcad/electron-serve-browser-process.tssrc/main/orcad/external-chromium-browser-process.test.tssrc/main/orcad/external-chromium-browser-process.tssrc/main/orcad/external-chromium-browser-session.test.tssrc/main/orcad/external-chromium-browser-session.tssrc/main/orcad/main-preflight-order.test.tssrc/main/orcad/main.tssrc/main/orcad/orcad-browser-provider.tssrc/main/orcad/orcad-browser-startup-cancellation.test.tssrc/main/orcad/orcad-browser-startup.test.tssrc/main/orcad/orcad-browser-startup.tssrc/main/orcad/orcad-completed-stop-receipt.tssrc/main/orcad/orcad-daemon-retirement.test.tssrc/main/orcad/orcad-daemon-retirement.tssrc/main/orcad/orcad-entry.tssrc/main/orcad/orcad-instance-lock.test.tssrc/main/orcad/orcad-instance-lock.tssrc/main/orcad/orcad-lifecycle-host.test.tssrc/main/orcad/orcad-lifecycle.tssrc/main/orcad/orcad-managed-stop-admission.tssrc/main/orcad/orcad-managed-stop-command.tssrc/main/orcad/orcad-managed-stop-completion.tssrc/main/orcad/orcad-managed-stop-request.tssrc/main/orcad/orcad-managed-stop.test.tssrc/main/orcad/orcad-push-startup.test.tssrc/main/orcad/orcad-runtime-lifetime.test.tssrc/main/orcad/orcad-runtime-lifetime.tssrc/main/orcad/orcad-stop-request-listener.test.tssrc/main/orcad/orcad-stop-request-listener.tssrc/main/runtime/orca-runtime-automation-operations.tssrc/main/runtime/orca-runtime-headless-terminal-close.test.tssrc/main/runtime/orca-runtime-stop-explicitly-closed-tab-ptys.tssrc/main/runtime/runtime-legacy-worker-recovery-lifetime.test.tssrc/main/runtime/runtime-legacy-worker-terminal-recovery-controller.tssrc/main/runtime/runtime-legacy-worker-terminal-recovery-runner.tssrc/shared/orcad-stop-request.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 6 remain after this review.
| - **Instance lock.** `<data-root>/orcad.lock` names the running orcad. A record that is | ||
| unreadable, malformed or over 64 KiB is never reclaimed: orcad exits 78 until an operator | ||
| removes it. A shutdown whose teardown failed keeps the lock until the process exits, so a | ||
| second orcad cannot start beside a writer that may still be running. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
#!/bin/bash
rg -nP -C8 'function resolveOrcadExitCode' src
rg -n 'orcad_instance_lock_' src --type=ts -g '!**/*.test.ts'Repository: stablyai/orca
Length of output: 2094
🏁 Script executed:
#!/bin/bash
printf '%s\n' '--- src/main/orcad/orcad-instance-lock.ts ---'
sed -n '1,70p;215,250p' src/main/orcad/orcad-instance-lock.ts
printf '%s\n' '--- docs/reference/orcad-operations.md ---'
sed -n '88,105p;168,182p' docs/reference/orcad-operations.mdRepository: stablyai/orca
Length of output: 6807
Add orcad_instance_lock_unreadable to the lock-code table.
OrcadInstanceLockError already maps to exit code 78, so the supervisor-loop concern does not apply. The table is incomplete because it omits the new unreadable-lock code that the following prose documents.
Suggested documentation fix
| `orcad_instance_lock_foreign_identity` | the lock belongs to a different identity |
+| `orcad_instance_lock_unreadable` | the lock is unreadable, malformed or larger than 64 KiB |
| `orcad_data_root_unusable` | the root cannot be created, stat'd or written || commands ??= provider.factory(host) | ||
| return Reflect.apply(Reflect.get(commands, property), commands, args) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Replace Reflect.apply/Reflect.get to fix the failing audit:anti-slop CI step.
The static analysis step fails on Line 38. The no-reflect-apply and no-reflect-get rules reject this dynamic dispatch. Use a typed indexed call on commands.
🔧 Proposed fix
commands ??= provider.factory(host)
- return Reflect.apply(Reflect.get(commands, property), commands, args)
+ const method = (commands as unknown as Record<string, (...a: unknown[]) => unknown>)[
+ property
+ ]
+ return method.apply(commands, args)If the linter also flags the cast, add a small named interface for dynamic dispatch, for example type DynamicBrowserCommands = Record<string, (...args: unknown[]) => unknown>. Keep the cast at one boundary.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| commands ??= provider.factory(host) | |
| return Reflect.apply(Reflect.get(commands, property), commands, args) | |
| commands ??= provider.factory(host) | |
| const method = (commands as unknown as Record<string, (...a: unknown[]) => unknown>)[ | |
| property | |
| ] | |
| return method.apply(commands, args) |
🧰 Tools
🪛 GitHub Actions: PR 24433 | source 58519df159906f1ebe2c99f539f153c5a04c3f7d | workflow 58519df159906f1ebe2c99f539f153c5a04c3f7d | unit full / static analysis
[error] 38-38: The audit:anti-slop step failed: oxlint reported that Reflect.apply should be replaced with a typed function call, with dynamic dispatch modeled behind a named interface.
🪛 GitHub Check: static analysis
[failure] 38-38: anti-slop(no-reflect-get)
src/main/orcad/orcad-browser-startup.ts:38:34: Replace Reflect.get with typed property access. Parse dynamic input into a named domain type before reading it.
[failure] 38-38: anti-slop(no-reflect-apply)
src/main/orcad/orcad-browser-startup.ts:38:20: Replace Reflect.apply with a typed function call. Model dynamic dispatch behind a named interface.
Sources: Linters/SAST tools, Pipeline failures
| await vi.waitFor(() => expect(ports.reconcileRequestedReleases).toHaveBeenCalledOnce()) | ||
| const settled = vi.fn() | ||
| const stopping = controller.stop().then(settled) | ||
| await Promise.resolve() |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Let shutdown continuations run before checking that shutdown remains pending.
One await Promise.resolve() does not establish that shutdown is waiting for release work. If stop() omits await Promise.all(this.backgroundWork), this test still passes: the await this.queue continuation delays settled until after the assertion.
Wait for an event-loop turn while the release promise remains pending. Then check settled before calling finish().
Proposed fix
- await Promise.resolve()
+ await new Promise<void>((resolve) => setTimeout(resolve, 0))📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| await Promise.resolve() | |
| await new Promise<void>((resolve) => setTimeout(resolve, 0)) |
ff863ae to
f9428f6
Compare
There was a problem hiding this comment.
✅ No new issues found in the changes since the last review.
Reviewed changes
Re-review of the incremental delta after the branch was rebased onto a newer main (99db2bf). Base-branch drift is filtered out; the only net change the new commit introduces is in the browser startup proxy.
- Replaced the browser proxy's unchecked member dispatch —
startOrcadBrowserProviderno longer callsReflect.apply(Reflect.get(commands, property), …). The newcallBrowserCommandlooks the member up and throws a typedBrowserError(browser_unavailable) with anUnknown browser command: <name>message when it is not a function, instead of a rawTypeErrorfrom trying to applyundefined. A test asserts that invoking a command the provider does not implement throws rather than silently dispatching.
deepseek-v4.1-flash (free via Pullfrog for OSS) ｜ 𝕏
…s and a lifetime that keeps its lock on failed teardown (#16741 T6-3) orcad stops through slot-local and instance-bound request files, so a reused PID is never signalled. A managed stop is proven by its completion command and recorded as a receipt. Optional daemon retirement is best effort: an idle daemon retires, while a busy or unverifiable one stays up with its admission fence released. Runtime teardown runs in reverse order and keeps the instance lock and profile admission when any writer fails to stop. Browser discovery no longer delays readiness. Legacy worker recovery and watcher children are drained before the final flush. Headless terminal close no longer waits on a renderer tab that does not exist. No production deployment.
f9428f6 to
755c52d
Compare
* Revert "feat(orcad): source-side dormant export of a relay-hosted SSH target (#16741 T6-8) (#24519)" This reverts commit 783101b. * Revert "feat(ssh): update, roll back, recover and stop a managed orcad server (#16741 T6-5 follow-up) (#24463)" This reverts commit 38c2d1d. * Revert "feat(ssh): deploy and pair an empty managed orcad server over SSH (#16741 T6-5) (#24453)" This reverts commit 8b76683. * Revert "fix(ssh): orcad GC honors the activation journal; readiness requires proven daemon coverage (#16741 T6 follow-up) (#24451)" This reverts commit d3f8c50. * Revert "feat(ssh): remote orcad stop by request file and journaled decommission (#16741 T6-4) (#24449)" This reverts commit 43d9b43. * Revert "feat(orcad): supervisable server: stop requests, managed stop receipts and a lifetime that keeps its lock on failed teardown (#16741 T6-3) (#24433)" This reverts commit b093d3a. * Revert "feat(ssh): crash-safe orcad activation, rollback and recovery (#16741 T6-2) (#24423)" This reverts commit 1a9ac0e. * Revert "feat(runtime): SSH access links for paired servers in a downgrade-safe sidecar (#16741 T5-1+T5-2) (#24420)" This reverts commit 99db2bf. * Revert "feat(relay): capability-gated owner reset with a durable preparation journal (#16741 T3 R1) (#24418)" This reverts commit 34a582b. * Revert "feat(ssh): track connection-manager drains, test probes and provider continuations (#16741 T2 P3+P8a) (#24407)" This reverts commit d53063d. * Revert "feat(daemon): idle retirement, session census and recovery-only provider (#16741 T2 P4b) (#24409)" This reverts commit ff212db. * Revert "feat(ssh): add pty.resumeClient and split SSH PTY process listing (#16741 T2 P5+P6) (#24414)" This reverts commit 92cb717. * Revert "feat(relay): await owned watcher and agent children on shutdown (#16741 T2 P1) (#24400)" This reverts commit 6b36e4f. * Revert "feat(session): retry failed renderer session writes and verify local folder PTYs (#16741 T2 P9) (#24406)" This reverts commit d23ecef. * Revert "feat(ssh): remote orcad primitives on the pinned Node runtime (#16741 T6-1) (#24419)" This reverts commit dd87ae5. * Revert "fix(runtime): fence runtime-environment subscriptions and status probes by identity (#16741 T5-3) (#24421)" This reverts commit ece9e4d. * Revert "feat(orcad): migration manifest and dormant-state contracts (#16741 T6-7) (#24422)" This reverts commit 3fbdaba. * Revert "feat(ssh): wire SshConnection through the work and transport close ledgers (#16741 T2 P2) (#24401)" This reverts commit 4e8edc8. * Revert "feat(profiles): carry markdown frontmatter visibility in project transfers (#16741 T2 P7) (#24405)" This reverts commit 60c9326. * Revert "fix(runtime): project the PTY incarnation onto mobile session tabs (#24413)" This reverts commit 99e0303. * Revert "feat(daemon): tag daemon stream data with the PTY incarnation id (#16741 T2 P4a) (#24402)" This reverts commit 817af76. * Revert "feat(ssh): port the SSH connection work ledger and transport close ledger (#16741 T2) (#24210)" This reverts commit c991893. * Revert "feat(relay): fence and drain file and git response streams on shutdown (#24185)" This reverts commit dc08ffe. * Revert "refactor(runtime-rpc): extract the Node WebSocket lifecycle; opt-in pinned port (#24186)" This reverts commit a789233. * Revert "feat(relay): route relay handlers through work admission; producer publication drain (#24181)" This reverts commit 0b812bd. * Revert "feat(relay): land the #16741 T1 seam (work drain, publication drain, release gate) (#24156)" This reverts commit 3aa2d3a. --------- Co-authored-by: m4air <m4air@Mac.localdomain>
ELI5
orcadis the Orca server that runs on a remote machine with no app window. To manage it safely, something outside it (a service manager, or Orca updating it over SSH) must be able to stop it, know for sure that it stopped, and restart it without damaging terminals or saved data. This PR adds those pieces:orcadto stop, using a file instead of a process ID;orcadstart while the first may still be writing.Nothing deploys
orcadyet, so users see no change.What Changed
Before:
orcadwas a signal sent to a process ID. If that ID was stale, the operating system could have reused it for an unrelated process.After:
orcad-stop-request-listener.ts). Both requests go through the same bounded stop path asSIGTERM..orcad-stop-requestbesideorcad.jsin the running slot:orcaddeletes it and stops. This is the half the client needs before T6-1's deferred switch fromSIGTERMto a stop file can land..orcad-managed-stop-request.<sha256 of the lock nonce>.orcadacts on it only when it names this process's version, runtime ID, PID, start time and lock nonce, and the instance lock still holds that record. It is kept as evidence.orcad --complete-managed-stop '<request JSON>'). It runs on the remote machine and never sends a signal.live,unverifiableorexited.exitedneeds proof: no process with that PID, or a PID whose start time shows it now belongs to a different process.exitedit writes<data-root>/orcad-stop-receipts/<transactionId>.json.orcad-daemon-retirement.ts). A request withretireIdleDaemon: truenever blocks or fails theorcadstop itself:requestIdleDaemonRetirement(feat(daemon): idle retirement, session census and recovery-only provider (#16741 T2 P4b) #24409) proves no live session across every daemon generation, the daemon is retired and the receipt recordsretirement: retired.orcadstill stops, the daemon stays up with its terminals (D7), and the receipt recordsretirement: live, with the session count from the daemon orlistLiveDaemonSessions.retirement: unverifiable. That is never read as idle.orcadreopens new-terminal admission before it exits (releaseDaemonRetirementFence, added to the daemon router and adapter). A daemon that stays is therefore never left refusing new terminals.orcadrecords it in<transactionId>.retirement.jsonbefore stopping, and the completion command copies it into the completed-stop receipt. A missing record reads asunverifiable.orcad-instance-lock.ts):orcadwrote in the meantime.orcad-runtime-lifetime.ts,orcad-lifecycle.ts,orcad-entry.ts). Every cleanup is registered when its resource is set up, and they run in reverse order: RPC, then legacy worker recovery and file watchers, then the final profile save, then the daemon disconnect, then the hooks.orcadkeeps both the instance lock and profile admission until the process exits.orcadnow waits for the file-watcher host's and watcher pool'sdisposeAndWait(feat(relay): await owned watcher and agent children on shutdown (#16741 T2 P1) #24400). Before, it never disposed watcher child processes at all.requestIdleDaemonRetirementandlistLiveDaemonSessions(feat(daemon): idle retirement, session census and recovery-only provider (#16741 T2 P4b) #24409) are used by the best-effort retirement path above.RuntimeLegacyWorkerTerminalRecoveryController.stop()cancels retries, refuses new passes, and waits for the running pass and its requested-release reconciliation.notifyRendererOfHeadlessTerminalClose, which can't fail the close.tabs.hasguard), and toleratestab_not_found. This was redone against main's rewritten close.orcad-browser-startup.tsand provider/process/sessionsignalplumbing): discovery runs in the background, so RPC readiness no longer waits for it. Stoppingorcadcancels discovery, and a provider whose cleanup failed is still reported.docs/reference/orcad-operations.mdcovers stop requests, the completion command and its exit codes (0 = verdict printed, 64 = bad invocation, 1 = failed before any verdict), the lock's fail-closed rule, and the existing 0/1/78 supervisor contract.Why
orcadthat watches it, and the instance-bound file is named after the lock nonce, so a request for an earlier instance never matches.unverifiable, neverexited(docs/reference/ssh-execution-boundary.md).validateOrcadDecommissionCompletion). Here it is self-contained, so T6-4 can build decommission on top.orcad-runtime-terminal-recovery,orcad-profile-daemon-startup,orcad-host-decommission,orcad-decommission-acceptanceandorcad-host-session-inventory;No user-visible change: this changes
orcad's behavior, butorcadhas no production deployment yet. It does not use Bun and adds no directchild_processimport.Linked Issue
Part of the Node runtime plan (Phase 3), T6 exit-gate track, porting #16741. Based on
main, because it doesn't need T6-1 (#24419) or T6-2 (#24423).Visual Proof
N/A. There's no UI change, and
orcadis not deployed to users yet.Testing
New tests:
orcad-managed-stop.test.ts, with a real lock in a temporary data root:exitedwithout proof;liveafter the attempts run out;retired,liveandunverifiableeach still complete the stop and appear on the receipt;unverifiable;retirement.orcad-stop-request-listener.test.ts, on realfs.watchand polling: a request written before or after start, managed request acceptance and refusal, close.orcad-runtime-lifetime.test.ts,orcad-lifecycle-host.test.ts: reverse order, and the lock and admission kept when a runtime writer or quit handler fails.orcad-daemon-retirement.test.ts: only an accepted retirement reads asretired. Busy, unsupported, failed or unanswered attempts never read as idle, and each one releases the admission fence.daemon-router-retirement.test.ts:releaseFencereopens admission after an incomplete retirement, and never after a completed one.orcad-stop-request-listener.test.tsalso checks that a managed stop proceeds even when its preparation fails.parcel-watcher-process-dispose.test.ts: both watcher hosts are awaited.runtime-legacy-worker-recovery-lifetime.test.ts: stop drains the running pass and the release reconciliation, and refuses queued work.orca-runtime-headless-terminal-close.test.ts: two cases that hang for 30s on main and now pass.signalcases for the provider, process and session.Updated:
main-preflight-order.test.ts: the completion command runs no preflight and starts no runtime;orcad-push-startup.test.ts: readiness doesn't wait for browser discovery, and the handle names the managed-stop instance.Regression, orcad, daemon, watcher and import-boundary tests, rerun after the retirement change: 300 files (293 passed, 7 skipped), 2,989 tests passed, 31 skipped.
Regression,
src/main/runtime: 953 files (950 passed, 3 skipped), 9,849 tests passed, 29 skipped. This ran before the retirement change, which touches no runtime code.Static checks:
node config/scripts/check-changed-code-quality.mjs origin/mainpasses;oxlintis clean;tsc --noEmitover the changed files is clean (the full typecheck was not run);pnpm-lock.yamlis unchanged.I manually tested these changes locally
Automated tests added/updated, or explained why not below
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
orcadand its own completion command. An olderorcadignores the managed-request filename.fs.watchplus a 1s poll runs on every platform. The managed-stop proofs use the existing process start-time reader.