Repository navigation
fix(ssh): stop reporting a confirmed kill when the SSH provider is gone - #14977
Conversation
A detached relay PTY is designed to outlive the provider that addressed it (it ignores SIGHUP and ships with an unlimited grace), so "the SSH provider is no longer registered" is lost contact, never evidence the remote process stopped. Both stop primitives in the PTY controller returned `true` from that branch, and every caller downstream reported the fabricated success: the CLI printed "PTY killed.", worker-stop settled the dispatch as stopped, and — because the stop "succeeded" — the unstopped-PTY gate never ran, so worktree removal walked straight past a live remote agent. `kill`/`stopAndWait` now still tombstone the local lease but report an unconfirmed stop and record why, using the three-verdict vocabulary the worktree teardown gate already spoke (`live` / `unverifiable` / `exited`), promoted out of that module into `src/shared/pty-liveness-verdict.ts`. The close receipt, the CLI wording, worker-stop and the removal gate all read that verdict instead of inferring an exit from silence. The same rule fixes the mirror-image defect: the aggregate inventory only enumerates registered providers, so a dropped relay clears `connected` for every remote PTY at once. The sweep now separates the provider answering "absent" (an exit) from no provider being able to answer (lost contact), so worker-stop stops claiming `exited` from a disconnect. The `connected` wire field is unchanged in meaning and shape.
|
Warning Review limit reached
Next review available in: 12 minutes Limit details: You’ve used all 10 included reviews currently available under your plan. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (46)
🚧 Files skipped from review as they are similar to previous changes (42)
Included review availability: Your plan includes up to 10 reviews per rolling hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe change adds shared PTY liveness verdicts for exited, live, and unverifiable states. SSH PTY shutdown records unverifiable liveness when its provider is unavailable or termination cannot be confirmed. Runtime inventory reconciliation and terminal-close receipts preserve verdicts and reasons. Worker and federation orchestration expose liveness details and unknown stop results. Worktree teardown blocks removal for unverifiable stops. Agent pane operations and CLI output report the resulting PTY state. 🚥 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 |
The federation host runs its own copy of the worker observation and stop logic, with the same two defects: `inspectRemoteAttachment` read a dropped relay's `connected: false` as `exited`, and `federationStop` settled the dispatch as stopped from a close it never confirmed — relaying a fabricated success all the way home to the coordinator. Two guards also had to move so the honest verdict does not become a new refusal. `federationRead` gated on `status !== 'running'`, which would have rejected a connected terminal the moment a stop lost contact with it; it now gates on `status === 'exited'`, which is equivalent for every pre-existing status given the two guards beside it. Local `workerStop` likewise still attempts the close when the verdict is `unverifiable` — losing contact is a reason to report the outcome honestly, never a reason to stop trying. The show observations now carry the reason alongside the status, so a bare `unverifiable` is actionable. Both are new optional fields.
c7aa737 to
f48ccaf
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/runtime/rpc/methods/orchestration-worker-stop.ts (1)
62-69: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winUnknown worker-stop receipts discard close diagnostics.
src/main/runtime/rpc/methods/orchestration-worker-stop.ts#L62-L69: passremote.closethroughunknownReceipt.src/main/runtime/rpc/methods/orchestration-worker-stop.ts#L122-L129: passclosethroughunknownReceipt.src/main/runtime/rpc/methods/orchestration-worker-stop-liveness-verdict.test.ts#L88-L99: assert the returned close verdict and reason.
🧹 Nitpick comments (2)
src/main/runtime/orca-runtime.ts (1)
17538-17577: 🗄️ Data Integrity & Integration | 🔵 Trivial | 💤 Low valueRemove or support the unreachable
liveclose verdict branch.
ptyLivenessVerdictByPtyIdrecords onlyunverifiableverdicts and clears entries. The separateliveverdict inunstopped-pty-verification.tsdoes not populate this map. ThereforedescribeTerminalClosecannot returnptyStopVerdict: 'live'. Remove the branch, or add a runtime writer and a terminal-close test if live close receipts are required.src/shared/pty-liveness-verdict.ts (1)
1-8: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueKeep liveness rationale comments concise.
Use one brief sentence explaining that provider silence does not prove remote PTY exit, and remove repeated implementation context from the module and related test comments.
Source: Coding guidelines
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: e1c9333c-e943-493e-bd31-71034983b3b7
📥 Commits
Reviewing files that changed from the base of the PR and between d670355 and c7aa737443d7dd957310aeae4203c43d81494c22.
📒 Files selected for processing (19)
src/cli/terminal-format.test.tssrc/cli/terminal-format.tssrc/main/ipc/pty-daemon-controller-teardown.test.tssrc/main/ipc/pty-ssh-stop-verdict.test.tssrc/main/ipc/pty.tssrc/main/runtime/orca-runtime-terminal-close-continuity.test.tssrc/main/runtime/orca-runtime.tssrc/main/runtime/pty-inventory-liveness-verdict.test.tssrc/main/runtime/rpc/methods/orchestration-federation-control.tssrc/main/runtime/rpc/methods/orchestration-federation-liveness-verdict.test.tssrc/main/runtime/rpc/methods/orchestration-worker-control.tssrc/main/runtime/rpc/methods/orchestration-worker-observation.tssrc/main/runtime/rpc/methods/orchestration-worker-stop-liveness-verdict.test.tssrc/main/runtime/rpc/methods/orchestration-worker-stop.tssrc/main/runtime/unstopped-pty-verification.tssrc/main/runtime/worktree-teardown-unstopped-pty.test.tssrc/main/runtime/worktree-teardown.tssrc/shared/pty-liveness-verdict.tssrc/shared/runtime-types.ts
Included review availability: Your plan includes up to 10 reviews per rolling hour; 1 remains after this review.
| const outcome = await call('orchestration.federationRead', { | ||
| dispatchId: DISPATCH_ID | ||
| }).catch((error: unknown) => error) | ||
|
|
||
| expect(outcome).not.toMatchObject({ code: 'worker_identity_changed' }) | ||
| }) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Require a successful terminal read.
This test catches every error. An unrelated readTerminal failure satisfies the assertion.
Mock a successful runtime.readTerminal result. Then await a resolved federation-read response and assert that readTerminal received HANDLE. This verifies that output remains available.
| const stopped = (await call('orchestration.workerStop', { dispatch: dispatch.id })) as { | ||
| state: string | ||
| processAction: string | ||
| lastError: string | ||
| } | ||
| // Losing contact is a reason to report honestly, never to stop trying. | ||
| expect(closeTerminal).toHaveBeenCalledWith('term_worker') | ||
| expect(stopped.processAction).toBe('closed_agent_terminal') | ||
| expect(stopped.state).toBe('stop_unknown') | ||
| expect(stopped.lastError).toContain('could not be confirmed stopped') | ||
| expect(stopped.lastError).not.toContain('exited') | ||
| }) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Assert the returned close verdict.
This test does not inspect close.ptyStopVerdict or close.ptyStopReason. It passes when orchestration.workerStop drops the structured diagnostic.
After the receipt contract is fixed, assert both fields on stopped.close.
|
CI green on |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/shared/pty-liveness-verdict.ts (1)
24-33: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse the shared stop-receipt formatter.
describeUnconfirmedAgentStopduplicates implementations in the agent-pane, worker-stop, federation-control, and worker-release modules. Import this helper there and remove the copies.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 0968b788-4c5e-45d5-a9b5-42592bcbb082
📥 Commits
Reviewing files that changed from the base of the PR and between c7aa737443d7dd957310aeae4203c43d81494c22 and 03a785a.
📒 Files selected for processing (40)
src/cli/terminal-format.test.tssrc/cli/terminal-format.tssrc/main/ipc/pty-daemon-controller-teardown.test.tssrc/main/ipc/pty-ssh-stop-verdict.test.tssrc/main/ipc/pty.tssrc/main/runtime/claude-agent-teams-service.test.tssrc/main/runtime/claude-agent-teams-tmux-dispatcher.tssrc/main/runtime/claude-agent-teams-types.tssrc/main/runtime/orca-runtime-process-incarnation-liveness.test.tssrc/main/runtime/orca-runtime-terminal-close-continuity.test.tssrc/main/runtime/orca-runtime-terminal-split-authority.test.tssrc/main/runtime/orca-runtime.tssrc/main/runtime/orchestration/db.tssrc/main/runtime/orchestration/worker-terminal-process-liveness.tssrc/main/runtime/pty-inventory-liveness-verdict.test.tssrc/main/runtime/rpc/methods/orchestration-federation-control.tssrc/main/runtime/rpc/methods/orchestration-federation-liveness-verdict.test.tssrc/main/runtime/rpc/methods/orchestration-federation-output.test.tssrc/main/runtime/rpc/methods/orchestration-federation-setup.test.tssrc/main/runtime/rpc/methods/orchestration-federation.test.tssrc/main/runtime/rpc/methods/orchestration-worker-control.tssrc/main/runtime/rpc/methods/orchestration-worker-observation.tssrc/main/runtime/rpc/methods/orchestration-worker-output.test.tssrc/main/runtime/rpc/methods/orchestration-worker-output.tssrc/main/runtime/rpc/methods/orchestration-worker-release-completion.tssrc/main/runtime/rpc/methods/orchestration-worker-release-liveness-verdict.test.tssrc/main/runtime/rpc/methods/orchestration-worker-release-recovery.test.tssrc/main/runtime/rpc/methods/orchestration-worker-release.test.tssrc/main/runtime/rpc/methods/orchestration-worker-release.tssrc/main/runtime/rpc/methods/orchestration-worker-stop-capability.test.tssrc/main/runtime/rpc/methods/orchestration-worker-stop-liveness-verdict.test.tssrc/main/runtime/rpc/methods/orchestration-worker-stop.tssrc/main/runtime/rpc/methods/orchestration-workers-recovery.test.tssrc/main/runtime/unstopped-pty-verification.tssrc/main/runtime/worktree-teardown-unstopped-pty.test.tssrc/main/runtime/worktree-teardown.test.tssrc/main/runtime/worktree-teardown.tssrc/shared/orchestration-worker-output.tssrc/shared/protocol-version.tssrc/shared/pty-liveness-verdict.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- src/cli/terminal-format.ts
- src/cli/terminal-format.test.ts
- src/main/runtime/rpc/methods/orchestration-worker-observation.ts
- src/main/runtime/rpc/methods/orchestration-federation-control.ts
Included review availability: Your plan includes up to 10 reviews per rolling hour; 5 remain after this review.
| let stopped = false | ||
| if (this.ptyController?.stopAndWait) { | ||
| try { | ||
| stopped = await this.ptyController.stopAndWait(ptyId, { deadlineMs }) | ||
| } catch (error) { | ||
| this.markPtyLivenessUnverifiable( | ||
| ptyId, | ||
| error instanceof Error ? error.message : String(error) | ||
| ) | ||
| } | ||
| if (!stopped) { | ||
| const verdict = this.getPtyLivenessVerdict(ptyId) | ||
| const providerAlreadyRetiredPty = | ||
| verdict?.status === 'unverifiable' && | ||
| verdict.reason === SSH_PROVIDER_UNREGISTERED_REASON | ||
| if (!providerAlreadyRetiredPty) { | ||
| this.ptyController.kill(ptyId) | ||
| if (!verdict || verdict.status === 'live') { | ||
| this.markPtyLivenessUnverifiable( | ||
| ptyId, | ||
| 'a follow-up stop was issued but its outcome could not be verified' | ||
| ) | ||
| } | ||
| } | ||
| } | ||
| } else { | ||
| stopped = this.ptyController?.kill(ptyId) ?? false | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Guard this.ptyController against becoming null after the stopAndWait await.
At line 28553, this.ptyController.kill(ptyId) runs without a null check or try/catch, after await this.ptyController.stopAndWait(...) at line 28540. setPtyController accepts null and can run while this await is pending (for example during a renderer detach). If this.ptyController becomes null in that window, this.ptyController.kill(ptyId) throws a TypeError.
No caller of stopExplicitlyClosedTabPtys (for example closeTerminal) wraps the call in a try/catch, so the exception would propagate out of the terminal-close RPC path instead of leaving the terminal closed with an unverified-stop receipt.
The rejected-pty cleanup path in splitPtyBackedTerminal (around lines 29066-29071) already guards the equivalent this.ptyController.kill(...) call with a try/catch for exactly this reason. Apply the same guard here, or capture this.ptyController once at the top of the loop iteration and use optional chaining for both calls.
🛡️ Proposed fix
if (!providerAlreadyRetiredPty) {
- this.ptyController.kill(ptyId)
+ try {
+ this.ptyController?.kill(ptyId)
+ } catch {
+ // Best-effort fallback kill; the pty already carries an unverified-stop verdict below.
+ }
if (!verdict || verdict.status === 'live') {📝 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.
| let stopped = false | |
| if (this.ptyController?.stopAndWait) { | |
| try { | |
| stopped = await this.ptyController.stopAndWait(ptyId, { deadlineMs }) | |
| } catch (error) { | |
| this.markPtyLivenessUnverifiable( | |
| ptyId, | |
| error instanceof Error ? error.message : String(error) | |
| ) | |
| } | |
| if (!stopped) { | |
| const verdict = this.getPtyLivenessVerdict(ptyId) | |
| const providerAlreadyRetiredPty = | |
| verdict?.status === 'unverifiable' && | |
| verdict.reason === SSH_PROVIDER_UNREGISTERED_REASON | |
| if (!providerAlreadyRetiredPty) { | |
| this.ptyController.kill(ptyId) | |
| if (!verdict || verdict.status === 'live') { | |
| this.markPtyLivenessUnverifiable( | |
| ptyId, | |
| 'a follow-up stop was issued but its outcome could not be verified' | |
| ) | |
| } | |
| } | |
| } | |
| } else { | |
| stopped = this.ptyController?.kill(ptyId) ?? false | |
| } | |
| let stopped = false | |
| if (this.ptyController?.stopAndWait) { | |
| try { | |
| stopped = await this.ptyController.stopAndWait(ptyId, { deadlineMs }) | |
| } catch (error) { | |
| this.markPtyLivenessUnverifiable( | |
| ptyId, | |
| error instanceof Error ? error.message : String(error) | |
| ) | |
| } | |
| if (!stopped) { | |
| const verdict = this.getPtyLivenessVerdict(ptyId) | |
| const providerAlreadyRetiredPty = | |
| verdict?.status === 'unverifiable' && | |
| verdict.reason === SSH_PROVIDER_UNREGISTERED_REASON | |
| if (!providerAlreadyRetiredPty) { | |
| try { | |
| this.ptyController?.kill(ptyId) | |
| } catch { | |
| // Best-effort fallback kill; the pty already carries an unverified-stop verdict below. | |
| } | |
| if (!verdict || verdict.status === 'live') { | |
| this.markPtyLivenessUnverifiable( | |
| ptyId, | |
| 'a follow-up stop was issued but its outcome could not be verified' | |
| ) | |
| } | |
| } | |
| } | |
| } else { | |
| stopped = this.ptyController?.kill(ptyId) ?? false | |
| } |
Live SSH QA evidence (previously captured)This evidence was captured at After
Evidence limits: SSH |
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
…7-closeout # Conflicts: # src/main/runtime/orchestration/db.ts
Final review verdict: release-readyThe review enforced one invariant throughout: losing contact with a process-owning host is not positive evidence that the process exited. The implementation now preserves
Failure diagnosis and evidenceThe failures at The PR body records the red-first evidence before the implementation: failures in Final-head CIFinal head: Observed at that exact head: 47 passing checks, 4 intentional skips, 0 non-passing; mergeable/CLEAN. PR Checks run 32000317068 completed with 43 successes and 1 skip, including green Node 24/26 shards 3, 9, and 13 and a green QA boundaryThe visible live SSH evidence was captured only at The following QA legs did not run: Electron UI, local/native SSH, WSL SSH, Windows SSH, folder-workspace SSH, live federation, live worker-release, and live agent-team respawn. SSH Verdict: release-ready on the observed evidence above. Not merged. |
The Known gaps section was accurate when written and becomes actively misleading as its fixes land: it told a reader to go fix restoreRequired, the missing unverifiable verdict, and the absent terminal-list host field, three things now addressed by #14974, #14977 and #14973. Mark the section as dated, require verification against current code before acting on any entry, name the PR per entry, and move landed items out. Also correct the two body claims that the landed fixes invalidated. The rules above are durable; only this section rots.
…bulary (#14971) * docs(ssh): state the SSH execution boundary and pin the liveness vocabulary Nothing under docs/ described how work splits between the client and an SSH host, so agents and humans inferred it from error strings and got it wrong: loss of contact was repeatedly reported as process death, which orphaned live remote agents and cold-started duplicates over the same worktree. Pins the vocabulary to the incumbent live/unverifiable/exited verdict from unstopped-pty-verification so no synonym is introduced, records the one real discriminator (all of a host's terminals drop together on link loss; one alone means process exit), and lists the outstanding gaps with citations. Tracked via the docs allow-list and linked from AGENTS.md, per the convention in .gitignore. * docs(ssh): cite the live restoreRequired site after it moved The throw now lives in reattachSshPtySessionForSpawn; ssh-pty-provider.ts no longer contains it. Caught by the worker fixing it, against a newer main than the audit ran on. * docs(ssh): require host evidence for liveness verdicts * docs(ssh): keep boundary references stable * docs(ssh): fence liveness evidence to its host identity * docs(ssh): state replay and environment boundaries precisely * docs(ssh): correct replay and platform boundary claims * docs(ssh): describe headless runtime continuity accurately * docs(ssh): distinguish authority from client metadata * docs(ssh): describe pending fixes accurately * docs(ssh): date the gap list and name the PR that closes each entry The Known gaps section was accurate when written and becomes actively misleading as its fixes land: it told a reader to go fix restoreRequired, the missing unverifiable verdict, and the absent terminal-list host field, three things now addressed by #14974, #14977 and #14973. Mark the section as dated, require verification against current code before acting on any entry, name the PR per entry, and move landed items out. Also correct the two body claims that the landed fixes invalidated. The rules above are durable; only this section rots. * docs(ssh): make the boundary doc a durable ruleset, not an incident record The Known gaps section was 18 of 93 lines enumerating specific defects from one investigation, several already fixed by sibling PRs in the same batch. A reference doc that needs a 'this section rots' warning is telling you the section belongs somewhere else; those entries belong in issues. Replace the six-row table of currently-lying signals with the method that outlands any particular bug: ask whether the owning host produced the signal, whether every PTY on the target went quiet together, whether the termination event matches the current incarnation and generation, and whether a returned status is actually a claim. Same for artifacts - state what ls-remote and a PR head each do and do not prove, rather than listing which command is currently wrong. Nothing here goes stale when the open fixes land.
…ne (stablyai#14977) * fix(ssh): stop reporting a confirmed kill when the SSH provider is gone A detached relay PTY is designed to outlive the provider that addressed it (it ignores SIGHUP and ships with an unlimited grace), so "the SSH provider is no longer registered" is lost contact, never evidence the remote process stopped. Both stop primitives in the PTY controller returned `true` from that branch, and every caller downstream reported the fabricated success: the CLI printed "PTY killed.", worker-stop settled the dispatch as stopped, and — because the stop "succeeded" — the unstopped-PTY gate never ran, so worktree removal walked straight past a live remote agent. `kill`/`stopAndWait` now still tombstone the local lease but report an unconfirmed stop and record why, using the three-verdict vocabulary the worktree teardown gate already spoke (`live` / `unverifiable` / `exited`), promoted out of that module into `src/shared/pty-liveness-verdict.ts`. The close receipt, the CLI wording, worker-stop and the removal gate all read that verdict instead of inferring an exit from silence. The same rule fixes the mirror-image defect: the aggregate inventory only enumerates registered providers, so a dropped relay clears `connected` for every remote PTY at once. The sweep now separates the provider answering "absent" (an exit) from no provider being able to answer (lost contact), so worker-stop stops claiming `exited` from a disconnect. The `connected` wire field is unchanged in meaning and shape. * fix(orchestration): apply the same honesty to the federation stop path The federation host runs its own copy of the worker observation and stop logic, with the same two defects: `inspectRemoteAttachment` read a dropped relay's `connected: false` as `exited`, and `federationStop` settled the dispatch as stopped from a close it never confirmed — relaying a fabricated success all the way home to the coordinator. Two guards also had to move so the honest verdict does not become a new refusal. `federationRead` gated on `status !== 'running'`, which would have rejected a connected terminal the moment a stop lost contact with it; it now gates on `status === 'exited'`, which is equivalent for every pre-existing status given the two guards beside it. Local `workerStop` likewise still attempts the close when the verdict is `unverifiable` — losing contact is a reason to report the outcome honestly, never a reason to stop trying. The show observations now carry the reason alongside the status, so a bare `unverifiable` is actionable. Both are new optional fields. * fix(ssh): preserve unconfirmed stop verdicts across consumers * fix(ssh): use canonical live verdict wording * fix(ssh): refuse wrong-host teardown verification * test(orchestration): confirm worker release teardown * fix(orchestration): negotiate honest worker stop receipts * fix(agent-teams): fence uncertain teammate respawns * fix(ssh): avoid duplicate missing-provider teardown * fix(orchestration): preserve archives across release retries * fix(ssh): preserve verdicts across synthetic kill exits * fix(ssh): preserve liveness evidence across teardown * fix(agent-teams): replace panes only after confirmed stop * fix(ssh): distinguish host exits from relay loss * fix(ssh): narrow concurrent inventory verdicts * fix(orchestration): serve archives after uncertain release * fix(orchestration): expose unverifiable read liveness * test(ssh): align liveness assertions with verdicts * fix(ssh): preserve host scope across inventory failures
…bulary (stablyai#14971) * docs(ssh): state the SSH execution boundary and pin the liveness vocabulary Nothing under docs/ described how work splits between the client and an SSH host, so agents and humans inferred it from error strings and got it wrong: loss of contact was repeatedly reported as process death, which orphaned live remote agents and cold-started duplicates over the same worktree. Pins the vocabulary to the incumbent live/unverifiable/exited verdict from unstopped-pty-verification so no synonym is introduced, records the one real discriminator (all of a host's terminals drop together on link loss; one alone means process exit), and lists the outstanding gaps with citations. Tracked via the docs allow-list and linked from AGENTS.md, per the convention in .gitignore. * docs(ssh): cite the live restoreRequired site after it moved The throw now lives in reattachSshPtySessionForSpawn; ssh-pty-provider.ts no longer contains it. Caught by the worker fixing it, against a newer main than the audit ran on. * docs(ssh): require host evidence for liveness verdicts * docs(ssh): keep boundary references stable * docs(ssh): fence liveness evidence to its host identity * docs(ssh): state replay and environment boundaries precisely * docs(ssh): correct replay and platform boundary claims * docs(ssh): describe headless runtime continuity accurately * docs(ssh): distinguish authority from client metadata * docs(ssh): describe pending fixes accurately * docs(ssh): date the gap list and name the PR that closes each entry The Known gaps section was accurate when written and becomes actively misleading as its fixes land: it told a reader to go fix restoreRequired, the missing unverifiable verdict, and the absent terminal-list host field, three things now addressed by stablyai#14974, stablyai#14977 and stablyai#14973. Mark the section as dated, require verification against current code before acting on any entry, name the PR per entry, and move landed items out. Also correct the two body claims that the landed fixes invalidated. The rules above are durable; only this section rots. * docs(ssh): make the boundary doc a durable ruleset, not an incident record The Known gaps section was 18 of 93 lines enumerating specific defects from one investigation, several already fixed by sibling PRs in the same batch. A reference doc that needs a 'this section rots' warning is telling you the section belongs somewhere else; those entries belong in issues. Replace the six-row table of currently-lying signals with the method that outlands any particular bug: ask whether the owning host produced the signal, whether every PTY on the target went quiet together, whether the termination event matches the current incarnation and generation, and whether a returned status is actually a claim. Same for artifacts - state what ls-remote and a PR head each do and do not prove, rather than listing which command is currently wrong. Nothing here goes stale when the open fixes land.
…bulary (stablyai#14971) * docs(ssh): state the SSH execution boundary and pin the liveness vocabulary Nothing under docs/ described how work splits between the client and an SSH host, so agents and humans inferred it from error strings and got it wrong: loss of contact was repeatedly reported as process death, which orphaned live remote agents and cold-started duplicates over the same worktree. Pins the vocabulary to the incumbent live/unverifiable/exited verdict from unstopped-pty-verification so no synonym is introduced, records the one real discriminator (all of a host's terminals drop together on link loss; one alone means process exit), and lists the outstanding gaps with citations. Tracked via the docs allow-list and linked from AGENTS.md, per the convention in .gitignore. * docs(ssh): cite the live restoreRequired site after it moved The throw now lives in reattachSshPtySessionForSpawn; ssh-pty-provider.ts no longer contains it. Caught by the worker fixing it, against a newer main than the audit ran on. * docs(ssh): require host evidence for liveness verdicts * docs(ssh): keep boundary references stable * docs(ssh): fence liveness evidence to its host identity * docs(ssh): state replay and environment boundaries precisely * docs(ssh): correct replay and platform boundary claims * docs(ssh): describe headless runtime continuity accurately * docs(ssh): distinguish authority from client metadata * docs(ssh): describe pending fixes accurately * docs(ssh): date the gap list and name the PR that closes each entry The Known gaps section was accurate when written and becomes actively misleading as its fixes land: it told a reader to go fix restoreRequired, the missing unverifiable verdict, and the absent terminal-list host field, three things now addressed by stablyai#14974, stablyai#14977 and stablyai#14973. Mark the section as dated, require verification against current code before acting on any entry, name the PR per entry, and move landed items out. Also correct the two body claims that the landed fixes invalidated. The rules above are durable; only this section rots. * docs(ssh): make the boundary doc a durable ruleset, not an incident record The Known gaps section was 18 of 93 lines enumerating specific defects from one investigation, several already fixed by sibling PRs in the same batch. A reference doc that needs a 'this section rots' warning is telling you the section belongs somewhere else; those entries belong in issues. Replace the six-row table of currently-lying signals with the method that outlands any particular bug: ask whether the owning host produced the signal, whether every PTY on the target went quiet together, whether the termination event matches the current incarnation and generation, and whether a returned status is actually a claim. Same for artifacts - state what ls-remote and a PR head each do and do not prove, rather than listing which command is currently wrong. Nothing here goes stale when the open fixes land.
…ne (stablyai#14977) * fix(ssh): stop reporting a confirmed kill when the SSH provider is gone A detached relay PTY is designed to outlive the provider that addressed it (it ignores SIGHUP and ships with an unlimited grace), so "the SSH provider is no longer registered" is lost contact, never evidence the remote process stopped. Both stop primitives in the PTY controller returned `true` from that branch, and every caller downstream reported the fabricated success: the CLI printed "PTY killed.", worker-stop settled the dispatch as stopped, and — because the stop "succeeded" — the unstopped-PTY gate never ran, so worktree removal walked straight past a live remote agent. `kill`/`stopAndWait` now still tombstone the local lease but report an unconfirmed stop and record why, using the three-verdict vocabulary the worktree teardown gate already spoke (`live` / `unverifiable` / `exited`), promoted out of that module into `src/shared/pty-liveness-verdict.ts`. The close receipt, the CLI wording, worker-stop and the removal gate all read that verdict instead of inferring an exit from silence. The same rule fixes the mirror-image defect: the aggregate inventory only enumerates registered providers, so a dropped relay clears `connected` for every remote PTY at once. The sweep now separates the provider answering "absent" (an exit) from no provider being able to answer (lost contact), so worker-stop stops claiming `exited` from a disconnect. The `connected` wire field is unchanged in meaning and shape. * fix(orchestration): apply the same honesty to the federation stop path The federation host runs its own copy of the worker observation and stop logic, with the same two defects: `inspectRemoteAttachment` read a dropped relay's `connected: false` as `exited`, and `federationStop` settled the dispatch as stopped from a close it never confirmed — relaying a fabricated success all the way home to the coordinator. Two guards also had to move so the honest verdict does not become a new refusal. `federationRead` gated on `status !== 'running'`, which would have rejected a connected terminal the moment a stop lost contact with it; it now gates on `status === 'exited'`, which is equivalent for every pre-existing status given the two guards beside it. Local `workerStop` likewise still attempts the close when the verdict is `unverifiable` — losing contact is a reason to report the outcome honestly, never a reason to stop trying. The show observations now carry the reason alongside the status, so a bare `unverifiable` is actionable. Both are new optional fields. * fix(ssh): preserve unconfirmed stop verdicts across consumers * fix(ssh): use canonical live verdict wording * fix(ssh): refuse wrong-host teardown verification * test(orchestration): confirm worker release teardown * fix(orchestration): negotiate honest worker stop receipts * fix(agent-teams): fence uncertain teammate respawns * fix(ssh): avoid duplicate missing-provider teardown * fix(orchestration): preserve archives across release retries * fix(ssh): preserve verdicts across synthetic kill exits * fix(ssh): preserve liveness evidence across teardown * fix(agent-teams): replace panes only after confirmed stop * fix(ssh): distinguish host exits from relay loss * fix(ssh): narrow concurrent inventory verdicts * fix(orchestration): serve archives after uncertain release * fix(orchestration): expose unverifiable read liveness * test(ssh): align liveness assertions with verdicts * fix(ssh): preserve host scope across inventory failures
…bulary (stablyai#14971) * docs(ssh): state the SSH execution boundary and pin the liveness vocabulary Nothing under docs/ described how work splits between the client and an SSH host, so agents and humans inferred it from error strings and got it wrong: loss of contact was repeatedly reported as process death, which orphaned live remote agents and cold-started duplicates over the same worktree. Pins the vocabulary to the incumbent live/unverifiable/exited verdict from unstopped-pty-verification so no synonym is introduced, records the one real discriminator (all of a host's terminals drop together on link loss; one alone means process exit), and lists the outstanding gaps with citations. Tracked via the docs allow-list and linked from AGENTS.md, per the convention in .gitignore. * docs(ssh): cite the live restoreRequired site after it moved The throw now lives in reattachSshPtySessionForSpawn; ssh-pty-provider.ts no longer contains it. Caught by the worker fixing it, against a newer main than the audit ran on. * docs(ssh): require host evidence for liveness verdicts * docs(ssh): keep boundary references stable * docs(ssh): fence liveness evidence to its host identity * docs(ssh): state replay and environment boundaries precisely * docs(ssh): correct replay and platform boundary claims * docs(ssh): describe headless runtime continuity accurately * docs(ssh): distinguish authority from client metadata * docs(ssh): describe pending fixes accurately * docs(ssh): date the gap list and name the PR that closes each entry The Known gaps section was accurate when written and becomes actively misleading as its fixes land: it told a reader to go fix restoreRequired, the missing unverifiable verdict, and the absent terminal-list host field, three things now addressed by stablyai#14974, stablyai#14977 and stablyai#14973. Mark the section as dated, require verification against current code before acting on any entry, name the PR per entry, and move landed items out. Also correct the two body claims that the landed fixes invalidated. The rules above are durable; only this section rots. * docs(ssh): make the boundary doc a durable ruleset, not an incident record The Known gaps section was 18 of 93 lines enumerating specific defects from one investigation, several already fixed by sibling PRs in the same batch. A reference doc that needs a 'this section rots' warning is telling you the section belongs somewhere else; those entries belong in issues. Replace the six-row table of currently-lying signals with the method that outlands any particular bug: ask whether the owning host produced the signal, whether every PTY on the target went quiet together, whether the termination event matches the current incarnation and generation, and whether a returned status is actually a claim. Same for artifacts - state what ls-remote and a PR head each do and do not prove, rather than listing which command is currently wrong. Nothing here goes stale when the open fixes land.
The invariant
Orca's SSH execution boundary has two rules. This PR enforces the second one:
The vocabulary is the one the worktree teardown gate already spoke —
live/unverifiable/exited— promoted out ofunstopped-pty-verification.tsintosrc/shared/pty-liveness-verdict.tsso every surface uses the same three words. No synonyms were introduced.The defect
src/main/ipc/pty.ts— bothkillandstopAndWaitreturnedtruewhen the SSH provider lookup threw. The in-code justification ("an absent SSH provider means there is no live target left to await") is false: a detached relay PTY is designed to outlive the provider that addressed it — it ignores SIGHUP (src/relay/relay.ts:1361-1364) and the shipped grace default is 0 (unlimited).Every caller downstream reported that fabricated success:
pty.tskill/stopAndWaitreturn truereturn false+ recordunverifiablewith the reason; the local lease is still tombstoned exactly as beforeorca-runtime.tscloseTerminal{ ptyKilled: true }{ ptyKilled: false, ptyStopVerdict: 'unverifiable', ptyStopReason }via a singledescribeTerminalClosereceipt builder used by all four return sitescli/terminal-format.ts"Closed terminal X. PTY killed.""Closed terminal X. The PTY was not confirmed stopped: its SSH provider is no longer registered."orchestration-worker-stop.tsprocessAction: 'closed_agent_terminal'+settleWorkerStopmarkWorkerStopUnknownwith "could not be confirmed stopped"; the dispatch is not settled as stoppedworktree-teardown.tsfailedPtyIds→ gate skipped → removal proceeded past a live remote agentunverifiableverdict short-circuits the re-list so removal is refused with the--forcehintSecond-order bypass, also fixed
Even with the honest
false, the gate would still have answeredexited:verifyUnstoppedPtysre-lists the client's own provider, which is silent about a host it cannot reach. An empty list read as "they exited."unverifiableStopVerdictnow consults the recorded verdict first — silence from a provider that does not own the PTY cannot overturn lost contact. Force Delete remains the documented escape hatch.Related defect (false death)
orchestration-worker-observation.tsclassifiedterminal.connected === falseas'exited'. But the aggregate inventory only iterates registered providers (pty.ts:5750-5753), so one dropped relay clearsconnectedfor every remote PTY at once — and worker-stop then asserted "The recorded worker process is exited; no terminal was closed."The sweep now distinguishes the two cases by what the provider actually answered:
hasPty === false→ observed absence →exited(today's behavior, unchanged)hasPty === null→ nobody could be asked →unverifiablehasPtyabsent entirely → no new claim either wayThe observation gains an
'unverifiable'status. Its three consumers degrade safely:worker-controlshow passes it through for display,worker-controlread maps anything non-exitedto'running', andrelease-completiononly branches onidentity_changed.Explicitly NOT changed
connectedfield's meaning or shape. Other clients read it; that is a wire-visible change owned by a separate PR. The verdict is carried internally and surfaced only through new optional fields.src/main/providers/ssh-pty-provider.tsrestoreRequiredhandling and the terminal-list schema — owned by other workers.Wire compatibility
Per
docs/reference/remote-wire-compatibility.md:RuntimeTerminalClose.ptyStopVerdict/.ptyStopReasonare new optional fields (Rule 1 — safe). Absent means today's answer, so a client reading onlyptyKilledis unaffected.'exited'deliberately never appears inptyStopVerdict— that is whatptyKilledreports.'unverifiable'value inobservation.statusand the rewordedlastErrorstrings. No client switches onobservation.status— it is rendered, not branched on — andlastErroris already free text.Red-first evidence
Every test below was written and run before the fix. Exact failure output:
src/main/ipc/pty-ssh-stop-verdict.test.ts—2 failed | 1 passedsrc/cli/terminal-format.test.ts—2 failed | 2 passedsrc/main/runtime/rpc/methods/orchestration-worker-stop-liveness-verdict.test.ts—1 failed | 1 passedsrc/main/runtime/worktree-teardown-unstopped-pty.test.ts—1 failed | 14 passed(removal completed successfully past a PTY nothing had confirmed stopped)
src/main/runtime/pty-inventory-liveness-verdict.test.ts— red obtained by reverting the sweep-classification hunk:Mutation checks
Each key assertion was re-run against a deliberately broken implementation to prove it binds:
pty.tskill catch back toreturn trueexpected true to be false— 1 failedworktree-teardown.tsdrop theunverifiableStopVerdictshort-circuitpromise resolved "{ runtimeStopped: +0, …(2) }" instead of rejecting— 1 failedorchestration-worker-observation.tsdrop theunverifiablebranch- "status": "unverifiable"/+ "status": "exited"— 1 failedterminal-format.tsdrop the unverifiable wordingexpected 'Closed terminal term_remote.' to be 'Closed terminal term_remote. The PTY …'— 1 failedorca-runtime.tsdescribeTerminalClosenever reads the verdictexpected { …(3) } to match object { ptyKilled: false, …(2) }— 1 failedThe lint gate was verified live the same way (injecting
const unusedProbe: any = 1produced two oxlint errors), since oxlint is silent when clean.Non-regression
Every suite carries a passing counterpart proving the confirmed path is untouched:
stopAndWaitstill resolvestrue, still callsshutdown(..., { immediate: true }), and never records a verdict;ptyKilled: truestill prints"Closed terminal X. PTY killed.", and a confirmed-kill receipt carries no verdict fields at all;'exited'with the exact message"The recorded worker process is exited; no terminal was closed.";hasPty === falsestill records no doubt.Two incumbent tests in
pty-daemon-controller-teardown.test.tsassertedkill(...) === truefor exactly the branch this PR corrects. Their real subject — that the SSH lease is tombstoned and the runtime is told — is unchanged and still asserted; only the fabricated return value flipped.Verification
src/main/ipc/pty*—541 passed, plus 10 pre-existing failures in MiMo/Pi/OMP env-overlay, login-shell and serializer suites. Proven pre-existing by revertingsrc/main/ipc/pty.tsto the base commit and re-running: they fail identically (expected "vi.fn()" to be called 1 times, but got 0 times). Untouched by this PR.src/main/runtime/orca-runtime*—1482 passed | 1 skippedsrc/main/runtime/rpc/methods/orchestration*—333 passedsrc/main/runtime/worktree*,src/main/ipc/worktrees*—347 passedsrc/cli/—791 passedtsc --noEmit -p config/tsconfig.node.jsonand-p config/tsconfig.cli.json— clean. (tsconfig.web.jsonreports 16 pre-existing TS6307 project-membership errors, none in files this PR touches.)oxlint+oxfmtclean on all 16 changed files.worktree-teardown.tscrossed the 300-line cap during this work; rather than disablemax-lines,unverifiableStopVerdictwas moved tounstopped-pty-verification.ts, where the other verdict decisions already live.Note
The verdict map is bounded at 256 entries with oldest-first eviction, and entries are cleared on any positive evidence — a reconnect, a provider reporting absence, or a real wait status from the owning host.
Follow-up commit: the federation host had the same two defects
Reviewing the call sites turned up a second, parallel copy of this logic on the federation host (
orchestration-federation-control.ts), serving remote coordinators. It had both defects:inspectRemoteAttachmentread a dropped relay'sconnected: falseasexited— the same false death, relayed home.federationStopcalledsettleRemoteAttachmentStopunconditionally, so a close it never confirmed was reported to the coordinator asstate: 'stopped'— the same false success, one hop further away.Both now follow the local rule.
federationStoproutes an unconfirmed close tomarkRemoteAttachmentStopUnknownand still returnsprocessAction: 'closed_agent_terminal'(the close was attempted) with the honestlastError.Two guards had to move so honesty does not become a new refusal
Adding an
unverifiablestatus to a union that other code gates on is exactly where a fix like this turns into a regression. Two sites gated onstatus !== 'running':federationReadwould have refused to read output from a connected terminal the moment a stop lost contact with it. It now gates onstatus === 'exited', which is provably equivalent for every pre-existing status —unattached/missingalready imply!observation.terminal, andidentity_changedalready implies!observation.exact— while leaving an unverifiable terminal readable.workerStopwould have skipped the close attempt entirely and gone straight to an unknown receipt, leaving the terminal open. It now proceeds to the close forunverifiableas well: losing contact is a reason to report the outcome honestly, never a reason to stop trying to stop the worker. The close receipt's verdict then decides the outcome.Both are covered and mutation-verified below. The
unverifiablereason is now published alongside the status in both show observations (new optional field) so the status is actionable rather than a mystery.Red-first / mutation evidence for the follow-up
src/main/runtime/rpc/methods/orchestration-federation-liveness-verdict.test.tsis new (5 tests). Each production branch was verified by breaking it:federationStopsettles an unconfirmed closeexpected 'stopped' not to be 'stopped'— 1 failedfederationShowignores the verdictexpected { …(5) } to match object { observation: { …(3) } }— 1 failedfederationReadgate back to!== 'running'expected OrchestrationError: Remote Dispatch ctx_f… to not match object { code: 'worker_identity_changed' }— 1 failedworkerStoprefuses to try when unverifiableexpected "closeTerminal" to be called with arguments: [ 'term_worker' ]— 1 failedNon-regression in the same file: a confirmed close still settles as
stoppedwithprocessAction: 'closed_agent_terminal', and a locally observed exit still reportsexited.The new federation assertions live in their own file rather than in
orchestration-federation.test.ts, because adding them there pushed that file to 808 lines against an 800-linemax-linescap. No lint rule was disabled to make room — the same reasonunverifiableStopVerdictmoved tounstopped-pty-verification.tsin the first commit.orchestrationsuites:338 passed(27 files).tsc --noEmitclean on both the node and CLI projects. CI was green on the first commit; this second one is pushed on top.Live SSH QA — 2026-08-16
Validated HEAD
3127133768fcb0f910b317aacafb9ff8672732a4against an isolated Ubuntu 20.04 SSH target. After disconnecting the owning provider, the real CLI close receipt reportedptyKilled: false,ptyStopVerdict: unverifiable, and reasonits SSH provider is no longer registered; an independent SSH probe confirmed the same remote PID remained live after the attempt.