Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
125 changes: 125 additions & 0 deletions config/reliability-gates.jsonc
Original file line number Diff line number Diff line change
Expand Up @@ -18698,6 +18698,131 @@
"Default config paths are shared with GNOME on a disposable hosted CI runner; nested mode refuses non-GitHub-Actions execution."
],
"demotionRule": "Keep experimental on unexplained failures; retain exact bytes and participation checks without retries, skips, or longer deadlines."
},
{
"id": "ssh-liveness.relay-owned-exit-certification",
"title": "Only the owning relay certifies that one of its PTYs exited",
"maturity": "experimental",
"protection": "partial",
"owner": "terminal-runtime",
"layer": "relay-handler-and-provider-service-composition",
"surfaces": [
"SSH legacy worker terminal recovery",
"relay PTY lifecycle",
"orchestration Task reuse after a worker exits"
],
"platforms": ["macos", "linux", "windows"],
"providers": ["ssh"],
"coveredPlatforms": ["macos"],
"coveredProviders": ["ssh"],
"coverageNotes": "Deterministic service-level composition of the real PtyHandler, the real SshPtyProvider, the real provider registry and runtime-controller readback, the real recovery candidate, and an in-memory orchestration DB. node-pty, the process-group kill and notification delivery are injected; the platform is pinned to linux inside the handler harness, so the run is macOS-hosted but platform-neutral. No live SSH server, real profile, kernel D-state process or app UI is exercised.",
"motivatingLinks": [
"https://github.com/stablyai/orca/pull/19347",
"https://github.com/stablyai/orca/pull/16904"
],
"invariant": "A PTY id is certified exited only by the relay that owned the record and watched the process end: node-pty's onExit, or a pid probe that answered ESRCH. Every other disappearance of a record - a shutdown that stopped waiting for an uninterruptible child, the torn-down-record sweep, an evicted or forgotten observation, an id this relay never held, a revive in flight, a record mid-teardown, and a host too old to answer - is unverifiable, and recovery defers instead of retiring the worker.",
"oracle": "The relay probe answers live/exited/unverifiable for each of those states directly, and binds the answer to one incarnation: admission under a reused id clears the prior observation, and only the incarnation that currently owns the id may record one, including a callback that arrives after teardown removed the record. The provider maps only live and exited, and maps a JSON-RPC -32601 rejection to null with no capability handshake. At the recovery seam, an observed exit converges to an exited resolution that releases the Task, while the shutdown-timeout case (with and without a sibling kill failure) and the old-relay case produce no resolution and a deferred dispatch, with a real orchestration DB refusing Task reuse.",
"commands": [
"ORCA_BACKGROUND_LAUNCH=1 node_modules/.bin/vitest run --config config/vitest.config.ts src/relay/pty-handler-liveness-probe.test.ts src/relay/pty-handler-ssh-exit-certification.test.ts src/relay/pty-handler-ssh-recovery-convergence.test.ts src/main/providers/ssh-pty-liveness-probe.test.ts src/main/providers/ssh-pty-provider-liveness-readback.test.ts src/main/runtime/runtime-legacy-worker-terminal-exit-certification.test.ts"
],
"testFiles": [
"src/relay/pty-handler-liveness-probe.test.ts",
"src/relay/pty-handler-ssh-exit-certification.test.ts",
"src/relay/pty-handler-ssh-recovery-convergence.test.ts",
"src/main/providers/ssh-pty-liveness-probe.test.ts",
"src/main/providers/ssh-pty-provider-liveness-readback.test.ts",
"src/main/runtime/runtime-legacy-worker-terminal-exit-certification.test.ts"
],
"assertionRefs": [
{
"file": "src/relay/pty-handler-liveness-probe.test.ts",
"assertions": [
"separates a real exit from a teardown removal in the same shutdown",
"certifies an exit node-pty reported after the shutdown wait had given up",
"stays unverifiable after the listing sweeps a torn-down record away",
"stays unverifiable while a revive holds the id, and answers for the new record after",
"does not certify a revived process with the exit of the one it replaced",
"does not let a superseded incarnation certify the id once both records are gone",
"evicts observations in the order it made them, not the order it was asked",
"forgets every observation when the relay restarts, even under the same mint epoch",
"certifies a long-lived resident whose owner entry was evicted",
"withholds a late observation once both the record and its owner entry are gone"
]
},
{
"file": "src/relay/pty-handler-ssh-recovery-convergence.test.ts",
"assertions": [
"recovers a worker after the connected relay observed its physical exit while the client was absent",
"recovers a worker whose exit the relay observed after its shutdown wait gave up",
"does not retire a revived worker with the exit of the process it replaced",
"does not certify an unexited shutdown record; sibling kill failure=%s"
]
},
{
"file": "src/relay/pty-handler-ssh-exit-certification.test.ts",
"assertions": [
"does not certify exit after shutdown bookkeeping removes an unexited PTY",
"keeps missing shutdown records unverifiable when another failed kill leaves the relay serving",
"publishes nothing that names the generation which minted its PTY ids",
"leaves a shutdown-removed id unverifiable for a client that infers from listings"
]
},
{
"file": "src/main/providers/ssh-pty-liveness-probe.test.ts",
"assertions": ["maps the owner verdict %s", "fails closed on %s"]
},
{
"file": "src/main/providers/ssh-pty-provider-liveness-readback.test.ts",
"assertions": ["exposes the readback the liveness rule asks the owning provider for"]
},
{
"file": "src/main/runtime/runtime-legacy-worker-terminal-exit-certification.test.ts",
"assertions": [
"certifies an SSH exit the owning relay observed",
"defers an SSH candidate the owning relay will not certify",
"defers when the host is too old to know the readback, without a capability handshake"
]
}
],
"evidenceRuns": [
{
"date": "2026-09-07",
"runner": "local",
"platform": "macos",
"command": "ORCA_BACKGROUND_LAUNCH=1 node_modules/.bin/vitest run --config config/vitest.config.ts src/relay/pty-handler-liveness-probe.test.ts src/relay/pty-handler-ssh-exit-certification.test.ts src/relay/pty-handler-ssh-recovery-convergence.test.ts src/main/providers/ssh-pty-liveness-probe.test.ts src/main/providers/ssh-pty-provider-liveness-readback.test.ts src/main/runtime/runtime-legacy-worker-terminal-exit-certification.test.ts",
"result": "passed",
"durationSeconds": 1.27,
"summary": "6 files, 58 tests passed. The convergence file fails 1/3 without the owner readback; thirteen mutations of the probe, its incarnation binding, the published capabilities, the bounded map and the forwarder each fail at least one of these tests."
}
],
"runtimeBudget": {
"p95Seconds": 30,
"scope": "Six unit/service-level files; the ledger-eviction and owner-eviction cases each drive 4,096+ spawn-and-exit cycles through the real handler and reads the oldest entry before the overflow and dominates the run at roughly 0.8 s."
},
"flakeHistory": {
"status": "not-started",
"evidence": "New with PR #19347. Deterministic: fake timers, injected node-pty, no real SSH, no network. No soak history yet."
},
"redGreenEvidence": {
"status": "complete",
"evidence": "Red: the archived round-3 reproduction failed 1/3 at head 0be1813781 with an empty resolution list, and the round-2 reproduction failed 2/4 against the removed client-side epoch inference. The round-4 review reproduced three more at head 03af416c38 - a revived id certified from its predecessor's exit, an exit observed after disposal lost forever, and an old client's inference restored by a published mint epoch. Green at this head. Twelve independent mutations (record in the shared reap, dropped disposed check, dropped pending-revive check, absent-record-as-exited, provider unknown-as-exited, provider rejection-as-exited, read-reordered ledger eviction, dropped admission invalidation, dropped incarnation guard, incarnation guard weakened to an empty pool, recording moved back below the disposed guard, republished mint epoch) each fail a checked-in test."
},
"performanceBudget": {
"required": false,
"evidence": "One exact-id RPC per candidate, replacing a full-inventory scan plus a second capability read; no polling or caching added."
},
"promotionCriteria": [
"Run a live SSH host journey where a supervised worker exits while the client is disconnected and the Task is reused after reconnection.",
"Cover Windows and Linux relay hosts, and an old-relay host binary that genuinely lacks pty.probeLiveness.",
"Collect 100 soak runs across 14 days with no unexplained flakes."
],
"knownGaps": [
"No live SSH server, real relay process restart, or kernel-uninterruptible child; the shutdown-timeout and restart cases are simulated at the handler seam.",
"The observed-exit ledger is in-memory and per relay process; a relay that restarts between the observation and the question answers unverifiable forever for that id, and only manual stop/abandon releases the worker.",
"A disposed record left in the pool is not reachable from any current teardown, so that guard, and the twice-superseded-id ordering built on top of it, are exercised by constructing the state rather than by a production path.",
"Local and daemon providers keep their existing readback; this gate makes no claim about them."
],
"demotionRule": "Any failure that shows an exit certified without an observation is a P0 revert of the probe, not a test relaxation; unverifiable answers that merely defer stay experimental while the live SSH journey is missing."
}
]
}
92 changes: 92 additions & 0 deletions fix-reports/19347-round2.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
# PR #19347 round 2 — SSH exit certification moved to its owner

## The defect

`probeSshPtyLiveness` inferred "absent from `pty.listProcesses` and minted by this relay's
generation, therefore exited". The implication is false. The relay has removal paths that never
observe the process end: `disposePtyForRelayShutdown` waits `IMMEDIATE_PTY_EXIT_TIMEOUT_MS` for a
physical exit, catches the timeout, and deletes the record anyway; `disposePtys` settles with
`Promise.allSettled`, so another PTY's failed kill rejects the aggregate while that record is

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🔎 Supported by static analysis

🏁 Script executed:

#!/bin/bash
set -euo pipefail
rg -n -C 12 '\bdisposePtys\b|Promise\.allSettled' src

Repository: stablyai/orca

Length of output: 50370


🏁 Script executed:

#!/bin/bash
set -euo pipefail
printf '%s\n' '--- report ---'
sed -n '1,18p' fix-reports/19347-round2.md
printf '%s\n' '--- implementation ---'
sed -n '3210,3248p' src/relay/pty-handler.ts

Repository: stablyai/orca

Length of output: 2398


Correct the Promise.allSettled failure description.

Promise.allSettled fulfills with rejected result entries. disposePtys explicitly throws rejected.reason after settlement, so state that disposePtys rejects rather than the Promise.allSettled aggregate.

already gone; `relay-grace-lifecycle` then defers shutdown and the relay keeps serving under the
same mint epoch. A newly connected provider asked, was told `false`, and recovery queued a live
worker as exited.

Reproduced at `1e89522a9d` with the reviewer's test: 2 failed / 2 passed.

## Census of record removals in `PtyHandler`

Every removal funnels through `removePty`, which has three callers plus the shared reap.

| Site | Path | Observed the process end? |
| --- | --- | --- |
| node-pty `onExit` handler | the process reported its own exit code | Yes |
| `reapPtyProvenExited` | pid probed, answered ESRCH from the owning host | Yes |
| `reapExitedPty(_, 'record-torn-down')` from `listProcesses` | tidies a record already disposed elsewhere | No |
| `disposePtyForRelayShutdown` | SIGKILL sent, exit wait timed out, record deleted regardless | No |

The two "yes" rows now write the id into `observedPtyExitIds`. Nothing else does, and the write
sits at the line that made the observation rather than inside the shared reap, so the reap's
bookkeeping caller cannot inherit it.

## What the relay answers

`pty.probeLiveness` takes an exact id and answers from the handler's own state.

- A live record: `live`, or `exited` if probing its pid reaps it.
- A record mid-teardown, or an id with a revive in flight: `unknown`.
- Any other id: `exited` only if it is in the ledger, otherwise `unknown`.

Fail-closed by construction. A future removal path that forgets to record itself yields `unknown`,
which defers; it cannot yield a false death certificate. Eviction from the bounded ledger has the
same safe direction.

Two gates were considered and dropped as dead code, each confirmed by a surviving mutation that
changed no observable behaviour. A mint-epoch comparison is subsumed because ledger keys are whole
ids that already embed the generation. A blanket "a shutdown has been fenced" refusal masked the
ledger in the only path where teardown runs, made that path's correctness untestable, and withheld
exits the relay really did observe.

## Boundary

**Invariant.** Only the execution host can distinguish a process that ended from bookkeeping that
stopped tracking it. A mint epoch proves who allocated an id; it says nothing about why that
owner's record disappeared.

**Owner.** `PtyHandler`. It holds the records, the physical-exit path, the pid probe, the shutdown
fence, and pending revives.

**Restores or compensates.** Restores. The previous version compensated at the client, rebuilding a
verdict from two projections that do not carry the missing evidence. The client now asks and
forwards; `SshPtyProvider.probePtyLiveness` maps three statuses to `true` / `false` / `null` and
does nothing else.

**Alternative rejected.** Keeping the inference and excluding the shutdown case client-side. It
needs the client to know which removals were bookkeeping, which is precisely the knowledge only the
owner has, and the next removal path would silently reopen the hole.

**Compatibility.** A relay too old to know the method answers JSON-RPC `-32601`, which arrives as a
rejection and maps to `null`, so no capability negotiation is needed and no discovery round trip is
spent. Old provider against a new relay is unaffected: the handler is additive. The `ptyIdMintEpoch`
capability and the shared id module stay because the relay still mints through them.

## Mutation results

Eight mutations against the relay verdict and the two suites that drive it.

| Mutation | Result |
| --- | --- |
| Certify any id absent from the map | 9 failed |
| Record the shutdown teardown as an observed exit | 4 failed |
| Drop the node-pty `onExit` record | 2 failed |
| Drop the pid-probe record | 1 failed |
| Answer live instead of reaping a dead pid | 1 failed |
| Record the torn-down sweep as an observed exit | survives |
| Treat a disposed record as live | survives |
| Drop the pending-revive gate | survives |

The three survivors guard states no current path constructs, all in the safe direction. A disposed
record is never left in the map: all three `disposeManagedPty` callers remove it in the same
synchronous block, so neither the torn-down sweep nor the disposed branch can be driven. An id in
the ledger cannot be revived, because revive requires `process.kill(pid, 0)` to succeed and a
recorded id's process is gone. The revive gate is kept anyway because that coupling lives in
another function.
Loading
Loading