fix(runner): accept the acknowledgement of an indeterminate suspend replay (#20714)

## Thinking Path

> - Paperclip is the open source app people use to manage AI agents for
work.
> - A native run talks to a Runner process (runnerd). The controller in
the server sends commands. The Runner journals each command before it
acts and replies with a result.
> - `runner.suspend` and `runner.shutdown` are terminal commands. The
Runner waits for a `command_result_ack` before it exits. It compares
that acknowledgement with the receipt that it journaled.
> - A Runner that is killed inside `runner.suspend` journals the command
but not its result. The next Runner replays the command as
`indeterminate`. It sends it on the wire as `failed` with
`execution_indeterminate`, and it still treats it as terminal.
> - The controller acknowledged the `failed` status that it received.
The Runner compared that with its journal status `indeterminate` and
exited with "terminal command result acknowledgement changed its durable
identity". The controller also lifted its terminal fence for a failed
suspend and sent the next command to a Runner that was about to exit.
> - This pull request makes both sides agree on the status of that one
receipt, and keeps the fence.
> - The benefit is that a freeze or restart during a suspend no longer
burns the same-run retries and leaves the task at a manual Retry.

## Linked Issues or Issue Description

**What happened?**

A restart-chaos task (local Runner, Claude, a 28-second freeze of the
server during a live turn) needed a manual Retry. Retry worked.

1. The turn completed. The server then sent `runner.suspend`. The server
froze before the Runner reported the result.
2. The close found the Runner not durably suspended (`commandStatus:
pending`, `runnerLifecycle: ready`). The run failed with
`native_runner_checkpoint_owner_lost`, and same-run recovery stopped the
old runnerd. The suspend was journaled but not finished.
3. Attempt 2 started a new runnerd on the same state. The welcome
delivered the stale suspend. The Runner replayed it as `indeterminate`
and sent `failed`. The controller acknowledged `failed`. The Runner
exited with code 1: "terminal command result acknowledgement changed its
durable identity". The controller had recorded the suspend as `failed`.
4. Attempt 3 found the fence for a suspend that the controller no longer
listed as pending. The Runner reconciled it and exited with code 0. The
server reported "runnerd exited unexpectedly with code 0", and
`native_session_retry_exhausted` stopped automatic recovery.

**Expected behavior**

The controller acknowledges exactly the receipt that the Runner sent.
The Runner accepts that acknowledgement, records `suspended`, and the
next attempt starts from a clean suspended Runner.

**Steps to reproduce**

1. Start a native run with a local Runner. Let the turn complete so that
the server sends `runner.suspend`.
2. Freeze the server (SIGSTOP) for about 30 seconds before the suspend
result arrives, so that recovery stops the Runner inside the suspend.
3. Let same-run recovery start a new Runner. It exits with "terminal
command result acknowledgement changed its durable identity". The unit
tests in this PR reproduce the same sequence without a live run.

**Paperclip version or commit**

`master` at 200c6e40e2. The failing wave ran on 9296c74e8d plus #20656.

**Root cause**

`command_result_envelope` changes the journal status `indeterminate` to
`failed` on the wire. `wait_for_terminal_result_ack` compared the
acknowledgement with the journal status, not with the status that it
sent. The controller also treated a `failed` suspend as a refused
suspend (the Runner keeps serving), but the Runner treats an
indeterminate suspend as terminal.

## What Changed

- Runner (`runner.rs`): a new `wire_command_status` gives the status
that goes on the wire. `wait_for_terminal_result_ack` accepts the
acknowledgement when its status is the wire status (`failed`) or the
journal status (`indeterminate`). Command id, type, and controller
sequence must still match exactly.
- Controller (`durable-prp-control-plane.ts`): for a result with
`execution_indeterminate`, the `command_result_ack` names
`indeterminate`. Runners that shipped before this change accept only
that status. The controller keeps the terminal-lifecycle fence for an
indeterminate suspend, so it does not send the next command to a Runner
that is exiting. The command stays `failed` in the controller journal,
as before.
- Runner Rust test
`indeterminate_suspend_replay_accepts_the_acknowledged_wire_status`: it
seeds a journaled suspend, starts the Runner against a fake controller
that acknowledges `failed` and then `indeterminate`, and checks that the
Runner finishes as `suspended` with no fence. Before the fix it failed
with the production message.
- Controller test: it checks the `indeterminate` acknowledgement and
that the next queued command is not delivered on that connection.
- `docs/durable-recovery.md`: it describes this case.

## Verification

- Red, then green: `cargo test -p paperclip-runner-core --lib
indeterminate_suspend_replay` failed with `DurableRunnerError("terminal
command result acknowledgement changed its durable identity")` before
the Runner change and passes after it.
- Red, then green: `npx vitest run
src/control-plane/durable-prp-control-plane.test.ts -t "indeterminate
suspension"` failed (`status: "failed"`, expected `"indeterminate"`)
before the controller change and passes after it.
- `cargo test -p paperclip-runner-core --lib durable`: 152 passed.
- `npx vitest run src/control-plane/durable-prp-control-plane.test.ts`:
215 passed.
- `npx vitest run src/live/runnerd-codex-transport-settlement.test.ts
src/live/runnerd-codex-transport-suspension.test.ts`: 30 passed. These
use the real runnerd binary built from `master`, so they also cover the
new controller with an old Runner.
- `npx tsc --noEmit -p packages/paperclip-runner`: clean.
- `pnpm verify` returned exit 2 (broad input). Required CI supplies the
broad coverage.
- Not run: a chaos wave. The unit tests reproduce the exact sequence at
the Runner and controller boundary.

## Risks

- Version mix. Old Runner with new server: the acknowledgement says
`indeterminate`, which is the old Runner's journal status, so it is
accepted. New Runner with old server: the acknowledgement says `failed`,
which is the new Runner's wire status, so it is accepted. New with new:
accepted.
- No command runs again. The indeterminate suspend is never executed
again; only its acknowledgement changes. The Runner still refuses a
different command id, type, or controller sequence.
- The Runner change takes effect only after a new Runner image. The
controller change alone removes the failure for Runners that already
shipped.
- Remaining gap: the attempt that delivers the stale suspend still ends
when that Runner exits as suspended. The next same-run attempt then
starts from a clean suspended Runner. A later change can restart the
Runner inside the same attempt.

> For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and
discuss it in `#dev` before opening the PR. Feature PRs that overlap
with planned core work may need to be redirected — check the roadmap
first. See `CONTRIBUTING.md`.

## Model Used

- Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, in Claude Code with
tool use (shell, file edits, test runs). Extended reasoning.

## Checklist

- [x] I have included a thinking path that traces from project context
to this change
- [x] I have specified the model used (with version and capability
details)
- [x] I have checked ROADMAP.md and confirmed this PR does not duplicate
planned core work
- [x] I have searched GitHub for duplicate or related PRs and linked
them above
- [x] I have either (a) linked existing issues with `Fixes: #` / `Closes
#` / `Refs #` OR (b) described the issue in-PR following the relevant
issue template
- [x] I have not referenced internal/instance-local Paperclip issues or
links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip`
URLs)
- [x] My branch name describes the change (e.g. `docs/...`, `fix/...`)
and contains no internal Paperclip ticket id or instance-derived details
- [x] I have run tests locally and they pass
- [x] I have added or updated tests where applicable
- [x] I have updated relevant documentation to reflect my changes
- [x] I have considered and documented any risks above
- [ ] All Paperclip CI gates are green
- [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups
- [x] I will address all Greptile and reviewer comments before
requesting merge

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude <noreply@anthropic.com>
This commit is contained in:
DottaandClaude authored and GitHub committed 2026-11-10 22:53:53 -06:00
1 parent 156212cfd5
commit e719d42557
6 files changed
+236 -9

No files matched your search

@@ -233,6 +233,16 @@ Attaching starts no turn, and the runner admits the new command only for a
settled session, so the run continues instead of failing on the old command. settled session, so the run continues instead of failing on the old command.
Warm transitions and staged attachments keep their exact replay paths. Warm transitions and staged attachments keep their exact replay paths.
A restart can kill the runner inside `runner.suspend` in the same way. The
next runner replays it from the welcome as `execution_indeterminate`, sends it
as `failed`, and still treats it as terminal: it records `suspended` and exits
once the controller acknowledges the result. The controller therefore keeps the
terminal fence for that result (it lifts the fence only for a suspension the
runner refused), and its acknowledgement names the runner's journal status
`indeterminate`. Current runners accept either `indeterminate` or `failed`; older
runners accept only `indeterminate` and otherwise exit with "terminal command
result acknowledgement changed its durable identity".
A kill during that attachment's provider launch can also leave a `spawned` A kill during that attachment's provider launch can also leave a `spawned`
startup fence over a `prepared` state that holds a saved thread. The next runner startup fence over a `prepared` state that holds a saved thread. The next runner
retires that fence once the recorded provider process and its group have exited, retires that fence once the recorded provider process and its group have exited,
@@ -1752,13 +1752,18 @@ fn wait_for_terminal_result_ack(
"terminal command result acknowledgement payload is required", "terminal command result acknowledgement payload is required",
) )
})?; })?;
// The controller acknowledges the status it received. An
// indeterminate journal entry travels as `failed`, so either
// spelling names this exact receipt.
let status = payload.get("status").and_then(Value::as_str);
if payload.get("commandId").and_then(Value::as_str) if payload.get("commandId").and_then(Value::as_str)
!= Some(result.command_id.as_str()) != Some(result.command_id.as_str())
|| payload.get("commandType").and_then(Value::as_str) || payload.get("commandType").and_then(Value::as_str)
!= Some(result.command_type.as_str()) != Some(result.command_type.as_str())
|| payload.get("controllerSeq").and_then(Value::as_u64) || payload.get("controllerSeq").and_then(Value::as_u64)
!= Some(result.controller_seq) != Some(result.controller_seq)
|| payload.get("status").and_then(Value::as_str) != Some(result.status.as_str()) || (status != Some(result.status.as_str())
&& status != Some(wire_command_status(result)))
{ {
return Err(DurableRunnerError::invalid( return Err(DurableRunnerError::invalid(
"terminal command result acknowledgement changed its durable identity", "terminal command result acknowledgement changed its durable identity",
@@ -2551,18 +2556,24 @@ fn send_outbox(
Ok(()) Ok(())
} }
/// "indeterminate" is an internal crash-recovery journal state. On the wire it
/// is a failed command with the preserved execution_indeterminate reason so the
/// controller can settle the command and continue replay.
fn wire_command_status(result: &StoredCommandResult) -> &str {
if result.status == "indeterminate" {
"failed"
} else {
result.status.as_str()
}
}
fn command_result_envelope( fn command_result_envelope(
state: &DurableState, state: &DurableState,
result: &StoredCommandResult, result: &StoredCommandResult,
protocol_version: u64, protocol_version: u64,
) -> Value { ) -> Value {
let mut payload = json!(result); let mut payload = json!(result);
// "indeterminate" is an internal crash-recovery journal state. On the payload["status"] = json!(wire_command_status(result));
// wire it is a failed command with the preserved execution_indeterminate
// reason so the controller can settle the command and continue replay.
if result.status == "indeterminate" {
payload["status"] = json!("failed");
}
json!({ json!({
"protocol": PROTOCOL, "protocol": PROTOCOL,
"version": protocol_version, "version": protocol_version,
@@ -0,0 +1,142 @@
/// A runnerd killed inside `runner.suspend` leaves it journaled but unfinished.
/// The next process replays it as `indeterminate`, sends it as `failed`, and the
/// controller acknowledges the `failed` it received. That acknowledgement names
/// the same receipt: the runner finishes the suspension instead of exiting with
/// "terminal command result acknowledgement changed its durable identity".
#[test]
fn indeterminate_suspend_replay_accepts_the_acknowledged_wire_status() {
for acknowledged in ["failed", "indeterminate"] {
indeterminate_suspend_replay_is_acknowledged(acknowledged);
}
}
fn indeterminate_suspend_replay_is_acknowledged(acknowledged_status: &'static str) {
struct NeverExecutes;
impl super::super::CommandExecutor for NeverExecutes {
fn execute(
&mut self,
command: &Command,
) -> Result<super::super::CommandExecution, DurableRunnerError> {
panic!("an indeterminate {} must never execute again", command.command_type);
}
}
let listener = TcpListener::bind("127.0.0.1:0").unwrap();
let port = listener.local_addr().unwrap().port();
let mut config = config(port);
config.max_runtime = Duration::from_secs(5);
let directory = std::env::temp_dir().join(format!(
"paperclip-runner-indeterminate-terminal-ack-{}-{acknowledged_status}",
std::process::id()
));
let _ = std::fs::remove_dir_all(&directory);
config.state_dir = directory.clone();
let suspend = json!({
"schema": "paperclip.prp.command.v1",
"commandId": "command_suspend",
"controllerSeq": 1,
"type": "runner.suspend",
"issuedAt": "2026-11-11T01:34:11.000Z",
"payload": {},
});
{
// The prior process journaled the suspend before its effect and died.
let store = super::super::DurableStateStore::new(&directory).unwrap();
let (mut state, _) = store.load_or_create(&config).unwrap();
let command: Command = serde_json::from_value(suspend.clone()).unwrap();
assert_eq!(
state.begin_command(&command).unwrap(),
super::super::CommandDisposition::Execute
);
store.save(&state).unwrap();
}
let state = test_state(&config);
let expires = current_unix_ms().unwrap() + 60_000;
let server_config = config.clone();
let server_state = state.clone();
let server = thread::spawn(move || {
let (stream, _) = listener.accept().unwrap();
let mut socket = accept(stream).unwrap();
let mut secure = server_authenticate(
&mut socket,
&server_config,
&server_state,
ServerCredential {
token: "bootstrap-secret",
kind: "bootstrap",
lease_id: None,
expires_at_unix_ms: expires,
revocation_epoch: 0,
},
);
send_secure(
&mut socket,
&mut secure,
&server_config,
&welcome(
&server_state,
"connection_1",
Some("lease-secret"),
expires,
0,
vec![suspend],
),
);
let result = receive_secure(&mut socket, &mut secure, &server_config);
assert_eq!(result["kind"], "command_result");
assert_eq!(result["payload"]["commandId"], "command_suspend");
assert_eq!(result["payload"]["status"], "failed");
assert_eq!(
result["payload"]["result"]["code"],
"execution_indeterminate"
);
send_secure(
&mut socket,
&mut secure,
&server_config,
&control(
&server_state,
"connection_1",
"command_result_ack",
json!({
"commandId": "command_suspend",
"commandType": "runner.suspend",
"controllerSeq": 1,
"status": acknowledged_status,
}),
),
);
// Acknowledge any terminal suffix until the runner closes.
while let Ok(frame) = receive_plain(&mut socket, server_config.max_frame_bytes) {
let message = secure.decrypt(&frame, true).unwrap();
if message["kind"] == "event" {
send_secure(
&mut socket,
&mut secure,
&server_config,
&control(
&server_state,
"connection_1",
"ack",
json!({"ackedSourceSeq": message["payload"]["sourceSeq"]}),
),
);
}
}
});
super::super::run_durable_runner(
config.clone(),
BootstrapTicket::new("bootstrap-secret".to_owned()).unwrap(),
NeverExecutes,
)
.unwrap();
server.join().unwrap();
let store = super::super::DurableStateStore::new(&directory).unwrap();
let (state, _) = store.load_or_create(&config).unwrap();
assert_eq!(state.lifecycle, "suspended");
assert!(state.pending_terminal_delivery.is_none());
std::fs::remove_dir_all(directory).unwrap();
}
@@ -1764,6 +1764,7 @@ mod tests {
#[cfg(unix)] #[cfg(unix)]
include!("tests/hosted_retention.rs"); include!("tests/hosted_retention.rs");
include!("tests/ack_release_poll.rs"); include!("tests/ack_release_poll.rs");
include!("tests/indeterminate_terminal_ack.rs");
use std::net::{IpAddr, Ipv4Addr, TcpListener}; use std::net::{IpAddr, Ipv4Addr, TcpListener};
use std::path::PathBuf; use std::path::PathBuf;
@@ -5377,6 +5377,54 @@ describe("DurablePrpControlPlane", () => {
} }
}); });
it("acknowledges an indeterminate suspension in the Runner's journal status and keeps its fence", async () => {
// A runnerd killed inside runner.suspend journals it as indeterminate. The
// next runnerd replays it from the welcome as `failed` with
// execution_indeterminate and then exits as suspended. Shipped Runners
// compare the acknowledgement with their journal status and exited with
// "terminal command result acknowledgement changed its durable identity".
const root = mkdtempSync(resolve(tmpdir(), "paperclip-prp-indeterminate-suspend-"));
const controlPlane = new DurablePrpControlPlane({
stateDirectory: root,
identity,
expectedRunnerVersion,
expectedRunnerDigest,
});
try {
await controlPlane.start();
const stale = controlPlane.queueCommand("runner.suspend", {}, "command-suspend-stale");
const next = controlPlane.queueCommand("session.snapshot", {}, "command-snapshot-next");
const client = await authenticate(controlPlane, controlPlane.issueBootstrapTicket());
sendSecure(client!, {
protocol: "paperclip.runner", version: 1, kind: "command_result",
payload: {
commandId: stale.commandId, commandType: stale.type, controllerSeq: stale.controllerSeq,
status: "failed",
result: {
code: "execution_indeterminate",
message: "runner recovered after journaling this command; it will not execute twice",
},
},
});
await expect(receiveSecure(client!)).resolves.toMatchObject({
kind: "command_result_ack",
payload: {
commandId: "command-suspend-stale",
commandType: "runner.suspend",
controllerSeq: stale.controllerSeq,
status: "indeterminate",
},
});
expect(controlPlane.getCommand(stale.commandId)?.status).toBe("failed");
// The Runner is leaving as suspended; nothing may follow on this connection.
expect(controlPlane.store.state.commandDeliveryCounts[next.commandId]).toBeUndefined();
client?.socket.destroy();
} finally {
await controlPlane.stop();
rmSync(root, { recursive: true, force: true });
}
});
it("acknowledges terminal command results after persisting them", async () => { it("acknowledges terminal command results after persisting them", async () => {
const root = mkdtempSync(resolve(tmpdir(), "paperclip-prp-terminal-ack-")); const root = mkdtempSync(resolve(tmpdir(), "paperclip-prp-terminal-ack-"));
const controlPlane = new DurablePrpControlPlane({ const controlPlane = new DurablePrpControlPlane({
@@ -84,6 +84,15 @@ export function isRunnerRecoveryMaintenanceCommand(type: string): boolean {
return ["session.snapshot", "session.goal.get", "runner.drain", "runner.suspend", "runner.shutdown", "turn.stop"].includes(type); return ["session.snapshot", "session.goal.get", "runner.drain", "runner.suspend", "runner.shutdown", "turn.stop"].includes(type);
} }
/** The Runner journaled this command before its effect, then died. It sends
* that `indeterminate` entry as `failed` with `execution_indeterminate`, never
* executes it again, and still treats a lifecycle command as terminal. */
function isIndeterminateCommandResult(command: DurableRecoveryCoreCommand): boolean {
if (command.status === "indeterminate") return true;
const detail = command.result?.result;
return command.status === "failed" && isRecord(detail) && detail.code === "execution_indeterminate";
}
const commandTypes = new Set([ const commandTypes = new Set([
"run.prepare", "run.prepare",
"run.attach", "run.attach",
@@ -4102,7 +4111,8 @@ export class DurablePrpControlPlane {
* A rejected runner.suspend changes no provider state, and the runner keeps * A rejected runner.suspend changes no provider state, and the runner keeps
* serving this connection. Lift the terminal-lifecycle fence it installed * serving this connection. Lift the terminal-lifecycle fence it installed
* so the controller's next command (a fresh suspend) can be delivered. * so the controller's next command (a fresh suspend) can be delivered.
* Completed suspensions and every shutdown keep the fence. * Completed suspensions and every shutdown keep the fence. So does an
* indeterminate suspension: the Runner treats it as terminal and exits.
*/ */
#releaseServingLifecycle( #releaseServingLifecycle(
connection: AuthorityConnection, connection: AuthorityConnection,
@@ -4111,6 +4121,7 @@ export class DurablePrpControlPlane {
if ( if (
command.type === "runner.suspend" && command.type === "runner.suspend" &&
(command.status === "failed" || command.status === "rejected") && (command.status === "failed" || command.status === "rejected") &&
!isIndeterminateCommandResult(command) &&
connection.terminalLifecycleCommandId === command.commandId connection.terminalLifecycleCommandId === command.commandId
) { ) {
connection.terminalLifecycleCommandId = null; connection.terminalLifecycleCommandId = null;
@@ -4159,7 +4170,11 @@ export class DurablePrpControlPlane {
commandId: command.commandId, commandId: command.commandId,
commandType: command.type, commandType: command.type,
controllerSeq: command.controllerSeq, controllerSeq: command.controllerSeq,
status: command.status, // A Runner journals a command it may have started before dying as
// `indeterminate` and sends it as `failed`. Runners compare this
// acknowledgement with their journal, so name that journal status;
// current Runners accept either spelling.
status: isIndeterminateCommandResult(command) ? "indeterminate" : command.status,
}, },
), ),
); );