stablyai / stablyai/orca

[Bug]: worker-release can never reclaim a settled worker whose PTY vanished (retained/identity_unproven on every retry)

Open
#19,166 2 comments 0 reactions 0 assignees View on GitHub
Dominant language
TypeScript
Stars
71.3k
Forks
4.7k
Avg merge
14h 54m
Merged PRs (30d)
520

Description

## Symptom

A coordinator tried to reclaim the terminals of 8 workers that had already reported
`worker_done`. `worker-list` showed all of them as `terminalState: "reclaimable"`.
**7 of the 8 `worker-release` calls failed**, always with the same receipt:

```json
{"dispatchId":"","state":"retained","reason":"identity_unproven",
"processAction":"none","archive":null}
```

The only one that released cleanly (`processAction: "closed_agent_terminal"`) was the most
recent one — the only one whose pane was still alive. A planned host restart had happened in
between, and several of the dispatches were from earlier sprints.

**Retrying does not help.** The same receipt comes back indefinitely. There is no CLI path out
of the state.

Version: **1.4.197**, Windows 11. Reproduced against source at the same tag.

## How to reproduce

1. Start workers with `orca orchestration worker-start`, let them report `worker_done`.
2. Restart the host (or otherwise let the PTYs die) while the dispatches are still `owned`.
3. `orca orchestration worker-list --run --json` — the dispatches show
`terminalState: "reclaimable"`.
4. `orca orchestration worker-release --dispatch --json` — `retained` /
`identity_unproven`, forever.

## Root cause

Two defects; only the first is what wedges the state.

### 1. The identity gate preempts the branch that handles a vanished PTY

In `src/main/runtime/rpc/methods/orchestration-worker-release-completion.ts`,
`completeWorkerTerminalReleaseOnce` runs its gates in this order:

| Line | Gate |
|---|---|
| :131 | `if (!workerTerminalLeaseIsCurrent(...))` -> `retained` / `identity_unproven` |
| :141 | `if (observation.status === 'missing' \|\| 'unattached')` -> `release_unknown` |

`workerTerminalLeaseIsCurrent` (:262-283) requires a live dispatch authority:

```ts
const authority = runtime.getOrchestrationDispatchAuthority(resource.terminal_handle)
return Boolean(... && authority && ...)
```

and `getOrchestrationDispatchAuthority` (`orca-runtime.ts:16354-16370`) returns **`null`** as
soon as the handle no longer resolves to a live, connected PTY:

```ts
try { ptyId = this.getLivePtyForHandle(h)?.pty.ptyId ?? this.resolveLiveLeafForHandle(h)?.ptyId ?? null }
catch { return null }
if (!ptyId) { return null }
const pty = this.ptysById.get(ptyId)
if (!pty?.connected) { return null }
```

A restart takes the PTYs, so the authority is `null`, the lease reads stale, and the method
returns at :131. **The `missing` branch at :141 is unreachable for any handle that no longer
resolves** — which is the only condition under which it would mean anything. `release_unknown`
is dead code along this path.

And there is no way out. The four candidate routes, checked one by one:

- **Retrying `worker-release`:** `requestWorkerTerminalRelease`
(`worker-terminal-release.ts:78-98`) does not look at `retained_reason`; it resets
`release_state='requested'`, `retained_reason=NULL`, and lands on the same gate.
`--retry-request` is RPC idempotency, it re-proves nothing.
- **The startup reconciler (`mode: 'recovery'`):** it iterates
`listWorkerTerminalReleaseBacklog`, which filters `release_state IN ('requested','releasing')`
(`worker-terminal-listing.ts:79`). A resource already reverted to `'retained'` is not in the
backlog. And even if it were, `mode` only changes behavior at :142 — *after* the gate.
- **`worker-retain`:** only rewrites `retained_reason` to `user_requested`.
- **`worker-terminal-recovery.ts:23`:** only looks at workers in
`starting/ready/start_unknown/stopping/stop_unknown`.

**The probe that would resolve this already exists, wired to the wrong arm of the `if`.**
`orchestration-worker-release.ts:57-79` does `inspectTerminalProcessIncarnationLiveness` +
`settleDeadWorkerTerminalRelease` — it proves the process is gone and settles the accounting —
but only under `disposition === 'retained'`. A `succeeded` worker holding an `owned` resource
always comes back as `disposition: 'requested'` (`worker-terminal-release.ts:88-98`), so it
never reaches the probe. Only `stopped`/`abandoned` workers take that arm (:58-60): the probe
was written for workers that never completed.

### 2. `worker-list` advertises `reclaimable` without asking the runtime

`deriveWorkerTerminalListState` (`worker-terminal-ownership.ts:81-111`) decides from **three DB
columns and zero runtime calls**:

```
ownership_state === 'owned'
&& release_state not in {released, unknown, requested, releasing, retained}
&& workerState in ['succeeded','failed']
```

against what release actually requires (`orchestration-worker-release-completion.ts:272-282`),
which includes `getOrchestrationDispatchAuthority(handle) != null` — i.e. a live, connected PTY.
The CLI advertises as reclaimable exactly what its own release cannot reclaim.

One nuance we measured: **the lie is single-shot.** After the failed attempt `release_state` is
`'retained'` and :99 classifies it `retained` from then on. But it has already sent the
coordinator down a dead end.

## A green test protects an unreachable path

`orchestration-worker-release.test.ts:483` — *"returns release_unknown when the terminal no
longer resolves, then completes a retry"* — mocks `showTerminal` to reject with
`terminal_handle_stale` and expects `release_unknown`. **It passes.**

It passes because its `setup()` mocks `getOrchestrationDispatchAuthority` **by handle string**
(:48-58), so the authority stays alive while only `showTerminal` fails. In the real runtime both
derive from the same live-PTY lookup: **that pairing cannot occur.** The test asserts a behavior
the runtime never produces, which is why the defect has stayed green across versions.

## Suggested fix

Move the probe that already exists into the shared completion path — the one both the RPC method
and the recovery reconciler route through. When identity cannot be proven, revert to `retained`
as before, and **only if the recorded process is proven `'exited'`** settle the accounting.
`unverifiable` and `live` keep retaining, so `docs/reference/ssh-execution-boundary.md` holds:
loss of contact is not evidence of death. Nothing is ever closed on this path —
`processAction` is `'none'`.

```diff
@@ -108,35 +108,14 @@ async function completeWorkerTerminalReleaseOnce(
const { runtime, db, dispatchId, resource } = args
const worker = db.getWorkerDispatch(dispatchId)
if (!worker || worker.agent_terminal_handle !== resource.terminal_handle) {
- const retained = db.revertWorkerTerminalReleaseToRetained(resource.id, 'identity_unproven')
- return {
- dispatchId,
- state: 'retained',
- reason: 'identity_unproven',
- processAction: 'none',
- archive: archiveSummary(retained)
- }
+ return retainUnlessProvenDead(args)
}
const observation = await inspectWorkerTerminal(runtime, db, dispatchId)
if (observation.status === 'identity_changed') {
- const retained = db.revertWorkerTerminalReleaseToRetained(resource.id, 'identity_unproven')
- return {
- dispatchId,
- state: 'retained',
- reason: 'identity_unproven',
- processAction: 'none',
- archive: archiveSummary(retained)
- }
+ return retainUnlessProvenDead(args)
}
if (!workerTerminalLeaseIsCurrent(runtime, db, dispatchId, resource)) {
- const retained = db.revertWorkerTerminalReleaseToRetained(resource.id, 'identity_unproven')
- return {
- dispatchId,
- state: 'retained',
- reason: 'identity_unproven',
- processAction: 'none',
- archive: archiveSummary(retained)
- }
+ return retainUnlessProvenDead(args)
}
if (observation.status === 'missing' || observation.status === 'unattached') {
if (args.mode === 'recovery') {
@@ -202,14 +181,7 @@ async function completeWorkerTerminalReleaseOnce(
}
}
if (!workerTerminalLeaseIsCurrent(runtime, db, dispatchId, releasing)) {
- const retained = db.revertWorkerTerminalReleaseToRetained(resource.id, 'identity_unproven')
- return {
- dispatchId,
- state: 'retained',
- reason: 'identity_unproven',
- processAction: 'none',
- archive: archiveSummary(retained)
- }
+ return retainUnlessProvenDead(args)
}

try {
@@ -261,6 +233,48 @@ async function completeWorkerTerminalReleaseOnce(
}
}

+// A vanished PTY drops its dispatch authority, so the lease check rejects it before the
+// 'missing' branch can run; settle the accounting only when the recorded process is proven gone.
+async function retainUnlessProvenDead(
+ args: WorkerTerminalReleaseArgs
+): Promise {
+ const { runtime, db, dispatchId, resource } = args
+ const retained = db.revertWorkerTerminalReleaseToRetained(resource.id, 'identity_unproven')
+ const processIncarnation = retained.process_incarnation
+ // Why 'owned': a takeover can flip the row between the reconciler's backlog snapshot and here,
+ // and settleDeadWorkerTerminalRelease only refuses 'released'.
+ if (
+ processIncarnation &&
+ retained.ownership_state === 'owned' &&
+ (await runtime.inspectTerminalProcessIncarnationLiveness(
+ processIncarnation,
+ retained.host_scope
+ )) === 'exited'
+ ) {
+ const reconciled = db.settleDeadWorkerTerminalRelease({
+ requestingDispatchId: dispatchId,
+ resourceId: resource.id,
+ processIncarnation
+ })
+ if (reconciled.disposition === 'released') {
+ runtime.notifyMessageArrived(`dispatch:${dispatchId}`, 'status')
+ return {
+ dispatchId,
+ state: 'released',
+ processAction: 'none',
+ archive: archiveSummary(reconciled.resource)
+ }
+ }
+ }
+ return {
+ dispatchId,
+ state: 'retained',
+ reason: 'identity_unproven',
+ processAction: 'none',
+ archive: archiveSummary(retained)
+ }
+}
+
function workerTerminalLeaseIsCurrent(
runtime: OrcaRuntimeService,
db: OrchestrationDb,
```

Verified against source at 1.4.197: `tsc --noEmit` clean, `oxlint` clean, the four
`orchestration-worker-release*` test files 52/52 green, and a new test file covering the pairing
that actually happens (dispatch authority `null` *and* `showTerminal` rejecting) is **red without
the patch**. It is a separate file on purpose: `orchestration-worker-release.test.ts` sits at 792
countable lines against a `max-lines` ceiling of 800.

## What we did NOT isolate — please read before taking the diff

- **The liveness probe is fail-open on an inventory that answers.**
`classifyWorkerTerminalProcessIncarnation` (`worker-terminal-process-liveness.ts:39-62`) returns
`'exited'` when the recorded session is simply absent from the list, and that is deliberate
(`orca-runtime-process-incarnation-liveness.test.ts:77-88` asserts an empty list gives
`'exited'`). The patch does not introduce that semantics — the `stopped`/`abandoned` arm already
relies on it — but it **extends it to `succeeded` workers**, which is new. Every way of failing
to obtain the inventory answers `'unverifiable'` first (unparseable host scope, no
`ptyController.listProcesses`, `listProcesses` throwing, the `PTY_CONTROLLER_LIST_TIMEOUT_MS`
timeout), and the local daemon inventory rethrows rather than returning an empty list
(`daemon-pty-session-inventory.ts:80`). The one gap we could not close by test is the cold-start
provider swap: `listProcessesFromRuntimeController` does **not** await
`getLocalPtyProviderStartupPromise`, unlike `probePtyLivenessFromRuntimeController`
(`operations.ts:71`) and the `pty:listSessions` handler (`inspect.ts:92`), so a pre-swap provider
could fabricate an absence. In practice the reconciler is triggered *by* terminal rediscovery,
downstream of that swap — but if you want the guarantee, the fix belongs in
`listProcessesFromRuntimeController`, not in the release path.
- **Defect 2 (`worker-list` advertising `reclaimable`) is described but not patched.** The diff
above changes release only. With release fixed the advertisement becomes true rather than
misleading, which is why we left it alone — but the derivation still never consults the runtime.
- **SSH and WSL host scopes are reasoned about, not exercised.** Our reproduction is a local
Windows host.

## Refuted hypotheses

- *"It is a stale `retained_reason` that a retry clears."* No: `requestWorkerTerminalRelease`
clears it and the call lands on the same gate. Byte-identical receipt on every retry.
- *"The startup reconciler will settle it eventually."* No: the backlog query excludes
`'retained'`, and `mode` is only consulted after the gate that rejects it.
- *"`worker-retain` or `worker-terminal-recovery` offer a way back."* No: the first only relabels
the reason, the second only looks at unsettled workers.

## Not a duplicate

Full-text search for `identity_unproven` returns 8 issues; none covers this path.

| # | Why it is not this |
|---|---|
| **16927** (PR, open) *fix: settle exited worker release when tab is absent* | Closest, and points the **other** way (avoid closing a replacement PTY). Does not reorder the gates or reach the probe. Not in this build: `expectedProcessIncarnation` has 0 occurrences in `src/`. |
| 15920 | `release can end in tab_not_found` — startup path, not post-restart |
| 18737 | release reporting success **without** closing — the opposite |
| 13047 | terminals left open after settle, but via `legacy_ambiguous` |
| 17004, 16522, 15580, 17006, 17892, 18827 | unrelated to the release path |

Contributor guide

Open the contributing guide

Research direction

Start in src/main/runtime/rpc/methods/orchestration-worker-release-completion.ts and trace the shared completion path through orchestration-worker-release.ts and worker-terminal-release.ts. Run the four orchestration-worker-release test files, including the existing stale-terminal case, then add coverage for null dispatch authority with a rejected showTerminal lookup. Done means the dead recorded process can be settled while unverifiable or live processes remain retained, without regressing the existing tests.

Written by the indexing model from the issue text.

Assessment

Tech stack
typescript
Domain
backend, cli
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
58/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.