mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-13 15:55:46 +01:00
fix(runner): reject an out-of-order controller command instead of exiting (#20946)
## Thinking Path - Paperclip's controller and the sandbox Runner exchange numbered commands; the Runner refuses a command whose number is not exactly one past the last it processed. - Staging's most common "Runner gone" failure was a follow-up controller whose journal was behind the Runner. #20928 removed the cause on the server (a warm follow-up now launches from the journal its retired predecessor saved) and added a defence that reconciles a journal behind only by the predecessor's settled close. - The Runner still treated an out-of-order command as fatal: `durable/state.rs` returned an error and the process exited. A controller with a stale journal from any other cause (state saved by an older release, a mid-run deploy) therefore lost the Runner process and the run could only fail as "Runner gone" (`runner_remote_process_identity_unavailable`). - This pull request makes the Runner refuse such a command with a precise result instead of exiting, and makes the server rest that run with a precise code and a Retry whose next run reconciles the journal (the #20928 defence) before it opens the session. ## Linked Issues or Issue Description - Refs #20928 (the server-side root-cause fix and the journal reconciliation this relies on) - Refs #20907 (makes runnerd exit text visible in the run log) - Duplicate search: searched PRs for "controller sequence contiguous", "controller_sequence_mismatch" and "out-of-order command"; none changes the Runner's handling. ## What Changed - `runner-core/src/durable/state.rs`: a new command whose `controllerSeq` is not `lastControllerCommandSeq + 1` (behind or a gap) now yields `CommandDisposition::Reject` with `status: "rejected"` and `{ code: "controller_sequence_mismatch", message, expectedControllerSeq, receivedControllerSeq }`. The command is not executed or journaled and the last sequence is unchanged; the process keeps running. Replay of an already-processed command and the compacted-history rejection are unchanged. - `native-session-executor.ts`: a failure carrying `controller_sequence_mismatch` is classified as `native_controller_journal_behind_runner`, a permanent (terminal) failure with a Retry action that says no provider work began and that Retry reconciles the journal from the saved checkpoint or stops with a precise error. - Tests: - Rust `tests/durable_recovery.rs`: out-of-order (behind and gap) commands are rejected with the precise code, the journal is unchanged, and the next contiguous command executes. The existing `command_gaps_and_identifier_reuse_fail_closed` unit test now asserts the rejection instead of an error. - Real runnerd `src/live/runnerd-stale-controller-journal.test.ts`: a follow-up controller with a journal missing the predecessor's close now gets `"code":"controller_sequence_mismatch"`, the Runner process stays alive and its durable sequence is unchanged; with the current journal it resumes. - Server: the classification and terminal disposition of the new code. ## Verification - Red (master Runner): the Rust test panics on `expect("an out-of-order command is not fatal")`; the real-runnerd test receives `runnerd exited unexpectedly with code 1: paperclip-runnerd: controller sequence must be contiguous: expected 6, received 4`. - Green: `cargo test -p paperclip-runner-core --test durable_recovery` (4 passed); `cargo test -p paperclip-runner-core` lib suite (the only other failure, `task_memory::tests::monitor_bounds_notices`, is a timing test that passes alone); `npx vitest run src/live/runnerd-stale-controller-journal.test.ts` with a runnerd built from this branch (2 passed); server `native-session-executor.test.ts` focused tests (71 passed). - `pnpm --filter @paperclipai/paperclip-runner check:acpx-profiles`: profiles match the release manifest. No ACPX profile, attestation, pin or pack contract schema changed (`rules/acpx-profile-identity.mdc`): the change is Runner source behaviour with an existing command-result shape, so no profile version or `commandDigest` changes. ## Risks - A controller that is *ahead* of the Runner (a gap) is now rejected instead of fatal. Neither case executes the command; the Runner journal is unchanged, so this is strictly safer than exiting. - The rested run needs a Retry. Before this change the same run failed after its retries as "Runner gone" with no precise cause. - Runner source changes the Daytona image content ID; a new image is needed for chaos waves on this revision. ## Model Used - Anthropic Claude Opus 5.5 (`claude-opus-5-5`), 1M-token context, tool use (shell, file edits, tests) in Claude Code. ## 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 Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
2719145ae2
commit
d9d5958d09
5 files changed
+92
-10
No files matched your search
@@ -822,9 +822,18 @@ impl DurableState {
|
||||
.checked_add(1)
|
||||
.ok_or_else(|| DurableRunnerError::invalid("controller sequence exhausted"))?;
|
||||
if command.controller_seq != expected {
|
||||
return Err(DurableRunnerError::invalid(format!(
|
||||
"controller sequence must be contiguous: expected {expected}, received {}",
|
||||
command.controller_seq
|
||||
return Ok(CommandDisposition::Reject(command_result(
|
||||
command,
|
||||
"rejected",
|
||||
json!({
|
||||
"code": "controller_sequence_mismatch",
|
||||
"message": format!(
|
||||
"controller sequence must be contiguous: expected {expected}, received {}",
|
||||
command.controller_seq
|
||||
),
|
||||
"expectedControllerSeq": expected,
|
||||
"receivedControllerSeq": command.controller_seq,
|
||||
}),
|
||||
)));
|
||||
}
|
||||
|
||||
@@ -3287,7 +3296,11 @@ mod tests {
|
||||
let mut unknown = command("command_unknown", 1);
|
||||
unknown.command_type = "future.required.command".to_owned();
|
||||
assert!(state.begin_command(&unknown).is_err());
|
||||
assert!(state.begin_command(&command("command_2", 2)).is_err());
|
||||
assert!(matches!(
|
||||
state.begin_command(&command("command_2", 2)).unwrap(),
|
||||
CommandDisposition::Reject(result) if result.result["code"] == "controller_sequence_mismatch"
|
||||
));
|
||||
assert_eq!(state.last_controller_command_seq, 0);
|
||||
let first = command("command_1", 1);
|
||||
state.begin_command(&first).unwrap();
|
||||
state.complete_command(&first, json!({})).unwrap();
|
||||
|
||||
@@ -189,3 +189,52 @@ fn pre_fingerprint_journal_recovers_without_reexecuting_old_commands() {
|
||||
|
||||
fs::remove_dir_all(directory).expect("remove integration-test state");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn an_out_of_order_new_command_is_rejected_without_changing_the_journal() {
|
||||
let directory = temporary_directory();
|
||||
let config = config(directory.clone());
|
||||
let store = DurableStateStore::new(&directory).expect("create private state store");
|
||||
let (mut state, _) = store.load_or_create(&config).expect("create durable state");
|
||||
state.last_controller_command_seq = 12;
|
||||
|
||||
let stale = Command {
|
||||
command_id: "command_stale".to_owned(),
|
||||
controller_seq: 11,
|
||||
..command()
|
||||
};
|
||||
let disposition = state
|
||||
.begin_command(&stale)
|
||||
.expect("an out-of-order command is not fatal");
|
||||
assert!(matches!(
|
||||
&disposition,
|
||||
CommandDisposition::Reject(result)
|
||||
if result.status == "rejected"
|
||||
&& result.result["code"] == "controller_sequence_mismatch"
|
||||
&& result.result["expectedControllerSeq"] == 13
|
||||
&& result.result["receivedControllerSeq"] == 11
|
||||
));
|
||||
let gap = Command {
|
||||
command_id: "command_gap".to_owned(),
|
||||
controller_seq: 15,
|
||||
..command()
|
||||
};
|
||||
assert!(matches!(
|
||||
state.begin_command(&gap).expect("a sequence gap is not fatal"),
|
||||
CommandDisposition::Reject(result) if result.result["code"] == "controller_sequence_mismatch"
|
||||
));
|
||||
assert_eq!(state.last_controller_command_seq, 12);
|
||||
assert!(!state.processed_commands.contains_key("command_stale"));
|
||||
|
||||
let next = Command {
|
||||
command_id: "command_next".to_owned(),
|
||||
controller_seq: 13,
|
||||
..command()
|
||||
};
|
||||
assert_eq!(
|
||||
state.begin_command(&next).expect("the contiguous command"),
|
||||
CommandDisposition::Execute
|
||||
);
|
||||
|
||||
fs::remove_dir_all(directory).expect("remove integration-test state");
|
||||
}
|
||||
@@ -31,7 +31,7 @@ function kill(pid: number | null | undefined) {
|
||||
try { process.kill(pid, "SIGKILL"); } catch (error) { if ((error as NodeJS.ErrnoException).code !== "ESRCH") throw error; }
|
||||
}
|
||||
|
||||
it.each(["stale", "current"] as const)("a follow-up controller resumes the predecessor's suspended runner from a %s journal", async journal => {
|
||||
it.each(["stale", "current"] as const)("a follow-up controller on the predecessor's suspended runner with a %s journal keeps the runner and gets a precise answer", async journal => {
|
||||
const root = await mkdtemp(join(tmpdir(), "runnerd-stale-journal-"));
|
||||
const controlPath = join(root, "control-plane", "control-plane-state.json");
|
||||
const predecessor = transport(root);
|
||||
@@ -50,10 +50,11 @@ it.each(["stale", "current"] as const)("a follow-up controller resumes the prede
|
||||
const outcome = await follower.transport.request("thread/read", {})
|
||||
.then(() => "resumed", (error: unknown) => String(error));
|
||||
if (journal === "stale") {
|
||||
expect(outcome).not.toBe("resumed");
|
||||
expect(outcome).toContain("controller sequence must be contiguous");
|
||||
await follower.transport.close().catch(() => undefined);
|
||||
expect(follower.transport.runnerExitConfirmed?.()).toBe(true);
|
||||
expect(outcome).toContain('"code":"controller_sequence_mismatch"');
|
||||
expect(outcome).not.toMatch(/runnerd exited/);
|
||||
expect(follower.evidence().runnerExited).toBe(false);
|
||||
const after = JSON.parse(await readFile(join(root, "runner", "runner-state.json"), "utf8")) as { lastControllerCommandSeq: number };
|
||||
expect(after.lastControllerCommandSeq).toBe(closed.commands.length);
|
||||
} else {
|
||||
expect(outcome).toBe("resumed");
|
||||
expect(outcome).not.toContain("controller sequence must be contiguous");
|
||||
|
||||
@@ -449,6 +449,7 @@ import {
|
||||
NATIVE_PROVIDER_STARTUP_REJECTED,
|
||||
nativeRemoteStartupRejectionCode,
|
||||
nativeRunnerExitedBeforeSessionOpen,
|
||||
NATIVE_CONTROLLER_JOURNAL_BEHIND_RUNNER,
|
||||
nativeRepeatedStartupFailureAction,
|
||||
nativePreviousStartupFailureSignature,
|
||||
nativeStartupFailureSignature,
|
||||
@@ -14489,6 +14490,14 @@ describe("native session bounded recovery", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("rests a run whose Runner refused its commands as out of sequence with a precise code and a Retry", () => {
|
||||
const refused = new Error(`PRP command session.open rejected: {"commandId":"c","controllerSeq":11,"commandType":"session.open","status":"rejected","result":{"code":"controller_sequence_mismatch","message":"controller sequence must be contiguous: expected 13, received 11","expectedControllerSeq":13,"receivedControllerSeq":11}}`);
|
||||
expect(nativeSessionFailureSourceCode(refused)).toBe(NATIVE_CONTROLLER_JOURNAL_BEHIND_RUNNER);
|
||||
expect(nativeSessionFailureSourceCode(new Error(`native_session_recovery_failed: ${refused}`))).toBe(NATIVE_CONTROLLER_JOURNAL_BEHIND_RUNNER);
|
||||
expect(nativeSessionFailureDisposition(1, new Date(), NATIVE_CONTROLLER_JOURNAL_BEHIND_RUNNER)).toMatchObject({
|
||||
phase: "terminal_failure", failureCode: NATIVE_CONTROLLER_JOURNAL_BEHIND_RUNNER });
|
||||
});
|
||||
|
||||
it("records a runner proven to have exited before answering session.open as a failed fresh start", () => {
|
||||
const proven = (message: string) => Object.defineProperty(new Error(message), "runnerExitConfirmed", { value: true });
|
||||
expect(nativeRunnerExitedBeforeSessionOpen(new Error("native_session_recovery_failed", {
|
||||
|
||||
@@ -8575,7 +8575,8 @@ export function nativeSessionFailureDisposition(
|
||||
sourceFailureCode === "native_adopted_runner_authentication_timeout" ||
|
||||
sourceFailureCode === "native_provider_usage_limit" ||
|
||||
sourceFailureCode === NATIVE_PROVIDER_SESSION_CLOSED ||
|
||||
sourceFailureCode === NATIVE_PROVIDER_STARTUP_REJECTED;
|
||||
sourceFailureCode === NATIVE_PROVIDER_STARTUP_REJECTED ||
|
||||
sourceFailureCode === NATIVE_CONTROLLER_JOURNAL_BEHIND_RUNNER;
|
||||
const permissionDeclined = sourceFailureCode === "native_permission_declined";
|
||||
const exhausted = permanentFailure || permissionDeclined || attempt >= 3;
|
||||
return {
|
||||
@@ -8628,6 +8629,9 @@ export const NATIVE_PROVIDER_SESSION_CLOSED_ACTION =
|
||||
"The provider session closed after a turn start that may have reached the provider, and this run cannot reopen it. Retry starts a new run with a fresh provider conversation and the saved task context.";
|
||||
const RUNNER_PROVIDER_SESSION_CLOSED = "provider_session_closed";
|
||||
|
||||
export const NATIVE_CONTROLLER_JOURNAL_BEHIND_RUNNER = "native_controller_journal_behind_runner";
|
||||
export const NATIVE_CONTROLLER_JOURNAL_BEHIND_RUNNER_ACTION =
|
||||
"The saved controller journal was behind its Runner, so the Runner refused this run's commands and no provider work began. Retry starts a new run, which reconciles the journal from the saved checkpoint before it opens the session, or stops with a precise error if it cannot.";
|
||||
export const NATIVE_PROVIDER_STARTUP_REJECTED = "native_provider_startup_rejected";
|
||||
export const NATIVE_PROVIDER_STARTUP_REJECTED_ACTION =
|
||||
"The provider refused to start a session, so no provider work began. Read the provider message and fix its cause in the agent's environment or AI connection if needed, then Retry. Retry starts a new run with a fresh provider session.";
|
||||
@@ -8735,6 +8739,7 @@ export function nativeSessionFailureSourceCode(
|
||||
| "native_provider_usage_limit"
|
||||
| typeof NATIVE_PROVIDER_SESSION_CLOSED
|
||||
| typeof NATIVE_PROVIDER_STARTUP_REJECTED
|
||||
| typeof NATIVE_CONTROLLER_JOURNAL_BEHIND_RUNNER
|
||||
| "native_session_cleanup_quarantined"
|
||||
| "native_adopted_runner_authentication_timeout"
|
||||
| "runner_remote_provider_artifact_incompatible"
|
||||
@@ -8801,6 +8806,7 @@ export function nativeSessionFailureSourceCode(
|
||||
}
|
||||
if (runnerClosedTurnStartSession(error)) return NATIVE_PROVIDER_SESSION_CLOSED;
|
||||
const message = error instanceof Error ? error.message : String(error);
|
||||
if (/"code":"controller_sequence_mismatch"/.test(message)) return NATIVE_CONTROLLER_JOURNAL_BEHIND_RUNNER;
|
||||
if (/^(?:runner_ingress_unavailable: runnerd exited unexpectedly with code 1: )?runner_task_admission_rejected: (session_state_unavailable|account_binding_mismatch)$/.test(message)) {
|
||||
return "runner_task_admission_rejected";
|
||||
}
|
||||
@@ -12733,6 +12739,8 @@ async function executePaperclipNativeSessionAttempt(
|
||||
? NATIVE_PROVIDER_SESSION_CLOSED_ACTION
|
||||
: sourceFailureCode === NATIVE_PROVIDER_STARTUP_REJECTED
|
||||
? startupRejectedAction
|
||||
: sourceFailureCode === NATIVE_CONTROLLER_JOURNAL_BEHIND_RUNNER
|
||||
? NATIVE_CONTROLLER_JOURNAL_BEHIND_RUNNER_ACTION
|
||||
: sourceFailureCode === "native_session_cleanup_quarantined"
|
||||
? nativeCleanupRefusalAction(completedTurnCleanup)
|
||||
: sourceFailureCode === "runner_remote_provider_verification_unavailable"
|
||||
@@ -13012,6 +13020,8 @@ async function executePaperclipNativeSessionAttempt(
|
||||
? NATIVE_PROVIDER_SESSION_CLOSED_ACTION
|
||||
: sourceFailureCode === NATIVE_PROVIDER_STARTUP_REJECTED
|
||||
? startupRejectedAction
|
||||
: sourceFailureCode === NATIVE_CONTROLLER_JOURNAL_BEHIND_RUNNER
|
||||
? NATIVE_CONTROLLER_JOURNAL_BEHIND_RUNNER_ACTION
|
||||
: sourceFailureCode === "native_session_cleanup_quarantined"
|
||||
? nativeCleanupRefusalAction(completedTurnCleanup)
|
||||
: sourceFailureCode === "runner_task_admission_rejected"
|
||||
|
||||
Reference in new issue
Block a user