mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-10 12:37:55 +01:00
fix(runner): read an uncommitted turn start as empty history when the event window is complete (#20107)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - A native run must survive repeated control-plane restarts; each
restore reconciles the turn its checkpoint names as active
> - That reconciliation reads the turn's history. When the provider has
no paginated history, the runnerd transport rebuilds it from the
committed runner events
> - After back-to-back hard restarts, the checkpoint can name an
accepted turn whose start was never committed. The runner was killed
after journaling `turn.start` and before the provider emitted anything
> - The rebuild treated every missing start as lost history and failed
with `codex_history_incomplete`, so the task needed a Retry
> - This pull request treats a missing start as empty history when the
committed window is provably complete
> - The benefit is that a run survives back-to-back restores, as it
already survives one
## Linked Issues or Issue Description
Refs #20074
**What happened?**
In restart-chaos wave w-20261108T200620-s9701 (local, K4/K6 + M1,
back-to-back K4 restores), three runs failed on the second restore. Two
were Claude ACPX C1 tasks; the third was an M1 parent with its subtask.
They failed with `native_session_recovery_failed:
HarnessReconciliationError: codex_history_read_failed: thread/items/list
requires supported paginated history; Error: codex_history_incomplete:
requested turn start is outside the retained runner event window`, and
each needed a Retry.
The sequence:
1. The first restore lost the in-flight turn and started a continuation
turn. ACPX echoes the requested turn id, so the checkpoint named it as
active.
2. Six seconds later a second K4 SIGKILL killed the runner. The runner
had journaled that `turn.start` (now `execution_indeterminate`), but the
provider had emitted nothing, so no `turn.started` was committed and the
runner outbox was empty.
3. On the second restore, `thread/items/list` for that turn hit the
guard, although the committed window was complete: source sequences
1–25, nothing evicted.
**Expected behavior**
The second restore treats the never-started turn like the first restore
treated its lost turn. The runner reports it lost on restore, and
recovery starts a continuation with no Retry.
**Steps to reproduce**
1. Have a checkpoint name an accepted turn whose `turn.started` was
never committed. For example, kill the runner after `turn.start` and
before the provider emits anything.
2. Restore the session. Before this change, recovery fails with
`codex_history_incomplete`.
**Paperclip version or commit**
master at 88835e624a plus #20094 (the wave). Branch rebased on current
master.
## What Changed
- `runnerd-codex-transport.ts`: a new
`committedEventWindowComplete(state)`. It is true only when the
committed events are exactly source sequences 1 to `ackedSourceSeq`:
none missing, none duplicated, none evicted.
- In the `thread/items/list` fallback, a turn with no committed start
returns empty history when the window is complete. Otherwise it still
throws `codex_history_incomplete`.
## Verification
- Red/green connected test in `runnerd-restart-continuation.test.ts`:
"starts a new turn when the checkpoint names an accepted turn whose
start was never committed". It uses the real runnerd and the fake Codex
server, and holds the provider start with `--turn-started-gate`.
- Without the change, it fails with the exact production error.
- With the change, the run completes on a continuation turn.
- Unit test: `committedEventWindowComplete` is false for a gap, a
missing first event, duplicates and no state.
- `npx vitest run` over every Codex, restart and recovery test file in
`packages/paperclip-runner`: 45 files, 1158 tests passed.
- `PAPERCLIP_TYPECHECK_LOW_MEMORY=1 pnpm typecheck`
(`packages/paperclip-runner`): passed.
## Risks
- Low risk. The empty history applies only when every acknowledged event
is still retained, so no event of that turn can have existed and been
dropped. Any evicted or gapped window keeps failing closed.
- Known gap, recorded as a follow-up: if eviction did remove events of a
turn the checkpoint still names, recovery still fails closed. The
planned fix is to never evict events of an unresolved checkpointed turn.
## Model Used
- Claude Opus 5.5 (`claude-opus-5-5`), via Claude Code with tool use
(shell, file edits, test execution).
## 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
83a229ae8c
commit
a88e23fa34
3 files changed
+111
-1
No files matched your search
@@ -69,6 +69,7 @@ import {
|
||||
resolveRunnerdCodexSkillInputs,
|
||||
withCodexCollaborationRuntimeInstructions,
|
||||
controllerAcceptedTurnStartId,
|
||||
committedEventWindowComplete,
|
||||
} from "./runnerd-codex-transport.js";
|
||||
import {
|
||||
defaultCapabilityRunnerdBinary,
|
||||
@@ -78,6 +79,18 @@ import {
|
||||
assignedRuntimeContext,
|
||||
} from "../../test/support/runnerd-transport.js";
|
||||
|
||||
describe("committedEventWindowComplete", () => {
|
||||
const events = (...seqs: number[]) => seqs.map(sourceSeq => ({ sourceSeq }));
|
||||
it("is complete only when every acknowledged event from the first one is retained", () => {
|
||||
expect(committedEventWindowComplete({ committedEvents: events(1, 2, 3), ackedSourceSeq: 3 })).toBe(true);
|
||||
expect(committedEventWindowComplete({ committedEvents: [], ackedSourceSeq: 0 })).toBe(true);
|
||||
expect(committedEventWindowComplete({ committedEvents: events(1, 3, 4), ackedSourceSeq: 4 })).toBe(false);
|
||||
expect(committedEventWindowComplete({ committedEvents: events(2, 3), ackedSourceSeq: 3 })).toBe(false);
|
||||
expect(committedEventWindowComplete({ committedEvents: events(1, 2, 2), ackedSourceSeq: 3 })).toBe(false);
|
||||
expect(committedEventWindowComplete(undefined)).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe("controllerAcceptedTurnStartId", () => {
|
||||
// Shapes copied from the RC-59 durable controller journal.
|
||||
const attach = { commandId: "command_prp_00000001", controllerSeq: 1, type: "run.attach", status: "completed", result: {} };
|
||||
|
||||
@@ -802,6 +802,14 @@ async function awaitProviderDrainBarrier(input: {
|
||||
return false;
|
||||
}
|
||||
|
||||
export function committedEventWindowComplete(state: { committedEvents: readonly { sourceSeq: number }[]; ackedSourceSeq: number } | undefined): boolean {
|
||||
if (!state || state.committedEvents.length !== state.ackedSourceSeq) return false;
|
||||
const seen = new Set(state.committedEvents.map(event => event.sourceSeq));
|
||||
if (seen.size !== state.ackedSourceSeq) return false;
|
||||
for (let seq = 1; seq <= state.ackedSourceSeq; seq += 1) if (!seen.has(seq)) return false;
|
||||
return true;
|
||||
}
|
||||
|
||||
function providerTurnIsActiveFromCommittedEvents(
|
||||
events: readonly { eventType: string }[],
|
||||
): boolean {
|
||||
@@ -4319,7 +4327,9 @@ class DurablePrpCodexTransport implements CodexAppServerTransport {
|
||||
const item = record(rehydrateRunnerdItemNotification(payload, this.#threadId, observedTurn).item);
|
||||
if (typeof item.id === "string") items.set(item.id, { turnId: observedTurn, item });
|
||||
}
|
||||
if (!observedStart) throw new Error("codex_history_incomplete: requested turn start is outside the retained runner event window");
|
||||
if (!observedStart && !committedEventWindowComplete(this.#core?.store.state)) {
|
||||
throw new Error("codex_history_incomplete: requested turn start is outside the retained runner event window");
|
||||
}
|
||||
data = [...items.values()];
|
||||
// Runner authority wins over schema-shaped prose in an assistant item.
|
||||
if (semanticResultItem) data.push(semanticResultItem);
|
||||
|
||||
@@ -597,3 +597,90 @@ it("starts a new turn when the lost turn's start reached the runner only after t
|
||||
await rm(root, { recursive: true, force: true });
|
||||
}
|
||||
}, 45_000);
|
||||
|
||||
it("starts a new turn when the checkpoint names an accepted turn whose start was never committed", async () => {
|
||||
const root = await mkdtemp(join(tmpdir(), "runner-restart-uncommitted-start-"));
|
||||
const identity = { runId: "run-uncommitted-start", sessionId: "session-uncommitted-start", companyId: "company-uncommitted-start", issueId: "issue-uncommitted-start", agentId: "agent-uncommitted-start" };
|
||||
const execution: NativeExecutionInputV1 = {
|
||||
schema: "paperclip.native-execution-input.v1",
|
||||
binding: { runId: identity.runId, companyId: identity.companyId, issueId: identity.issueId, agentId: identity.agentId, executionWorkspaceId: "workspace-uncommitted-start" },
|
||||
session: { normalizedSessionId: identity.sessionId, driverKind: "codex_app_server", protocolVersion: 1, lifecyclePolicy: { mode: "per_turn", idleTimeoutMs: null } },
|
||||
task: { identifier: "TEST-6", title: "Finish checks", description: null, prompt: "Watch the already running checks and finish the request.", workMode: "standard" },
|
||||
workspace: { cwd: root, repoUrl: null, repoRef: null, branchName: null },
|
||||
provider: { kind: "codex", model: null },
|
||||
completionContract: { id: "contract-uncommitted-start", sha256: "digest", schemaVersion: "paperclip.completion-contract.v1", contract: {
|
||||
revision: "1", objective: "Finish checks", criteria: [{ id: "checks", requirement: "Check the outcome" }],
|
||||
} },
|
||||
interactionResponses: [], credentialBindings: [],
|
||||
};
|
||||
const options = {
|
||||
runnerBinary: defaultCapabilityRunnerdBinary(),
|
||||
codexCommand: resolve(import.meta.dirname, "../../runner/target/debug/fake-codex-app-server"),
|
||||
codexArgs: ["--state-file", join(root, "fake-state.json"), "--call-log", join(root, "calls.log"), "--durable-turn-ids", "--hold-first-durable-turn", "--turn-started-gate", join(root, "release-turn-started")],
|
||||
stateDirectory: root,
|
||||
turnStartTimeoutMs: 8_000,
|
||||
prpIdentity: { runnerInstanceId: "runner-uncommitted-start", environmentLeaseId: "lease-uncommitted-start", runId: identity.runId, normalizedSessionId: identity.sessionId, turnId: "turn-uncommitted-start", itemId: "item-uncommitted-start" },
|
||||
};
|
||||
const first = createRunnerdCodexTransport(options);
|
||||
let restored: ReturnType<typeof createRunnerdCodexTransport> | undefined;
|
||||
const alive = (pid: number | null) => {
|
||||
if (!pid) return false;
|
||||
try { process.kill(pid, 0); return true; } catch { return false; }
|
||||
};
|
||||
const kill = (pid: number | null) => {
|
||||
if (!pid) return;
|
||||
try { process.kill(pid, "SIGKILL"); } catch (error) { if ((error as NodeJS.ErrnoException).code !== "ESRCH") throw error; }
|
||||
};
|
||||
try {
|
||||
const backend = createRunnerdNativeSessionBackend(execution, { runnerInstanceId: "runner-uncommitted-start", transportFactory: () => first.transport });
|
||||
const session = await backend.openSession({ identity, workingDirectory: root });
|
||||
const opened = await session.snapshot();
|
||||
const starting = session.startTurn({ message: { role: "user", text: execution.task.prompt } }).catch((error: unknown) => error);
|
||||
const controlPath = join(root, "control-plane", "control-plane-state.json");
|
||||
await vi.waitFor(async () => {
|
||||
const control = JSON.parse(await readFile(controlPath, "utf8"));
|
||||
expect(control.commands.find((command: { type: string }) => command.type === "turn.start")).toMatchObject({ status: "completed" });
|
||||
}, { timeout: 8_000, interval: 25 });
|
||||
const owner = first.evidence();
|
||||
const checkpoint = { ...opened, activeTurnId: "provider-turn-1" };
|
||||
await first.detachControllerForRestart();
|
||||
await starting;
|
||||
expect(await readFile(join(root, "runner", "runner-state.json"), "utf8")).not.toContain("turn.started");
|
||||
kill(owner.runnerPid);
|
||||
kill(owner.providerPid);
|
||||
await vi.waitFor(() => { expect(alive(owner.runnerPid)).toBe(false); expect(alive(owner.providerPid)).toBe(false); });
|
||||
await writeFile(join(root, "release-turn-started"), "");
|
||||
const providerState = JSON.parse(await readFile(join(root, "fake-state.json"), "utf8"));
|
||||
providerState.activeTurnId = null;
|
||||
await writeFile(join(root, "fake-state.json"), JSON.stringify(providerState));
|
||||
restored = createRunnerdCodexTransport({ ...options, resumeDynamicTools: [], resumeActiveTurnId: "provider-turn-1" });
|
||||
const recovering = createRunnerdNativeSessionBackend(execution, { runnerInstanceId: "runner-uncommitted-start", transportFactory: () => restored!.transport });
|
||||
const events: PrpEvent[] = [];
|
||||
const completion = await executeNativeSession({
|
||||
input: execution, backend: recovering, persistedSession: checkpoint, resumeInterruptedTurn: true,
|
||||
runnerInstanceId: "runner-uncommitted-start", controlPlaneInstanceId: "controller-uncommitted-start", timeoutMs: 15_000,
|
||||
controlPlane: {
|
||||
async openRun() {},
|
||||
async checkpointSession() {},
|
||||
async appendEvent(event) { if (!("sourceSeq" in event)) throw new Error("Expected a durable PRP event"); events.push(event); return { cursor: events.length, highestContiguousSourceSeq: event.sourceSeq, disposition: "committed" }; },
|
||||
async replayEvents(request) {
|
||||
const source = events.filter(event => event.sourceInstanceId === request.sourceInstanceId);
|
||||
return { events: source.filter(event => event.sourceSeq > request.afterSourceSeq), highestContiguousSourceSeq: source.at(-1)?.sourceSeq ?? 0 };
|
||||
},
|
||||
async completeRun() {},
|
||||
},
|
||||
});
|
||||
expect(completion).toMatchObject({ providerSessionId: checkpoint.providerSessionId, turnId: "provider-turn-2", terminal: { runTerminalState: "succeeded" } });
|
||||
expect(events.find(event => event.eventType === "session.failed")).toBeUndefined();
|
||||
expect((await readFile(join(root, "calls.log"), "utf8")).match(/^turn\/start$/gm)).toHaveLength(2);
|
||||
} finally {
|
||||
await restored?.transport.close().catch(() => undefined);
|
||||
for (const bundle of [first, restored]) {
|
||||
if (!bundle) continue;
|
||||
const evidence = bundle.evidence();
|
||||
kill(evidence.runnerPid);
|
||||
kill(evidence.providerPid);
|
||||
}
|
||||
await rm(root, { recursive: true, force: true });
|
||||
}
|
||||
}, 45_000);
|
||||
Reference in new issue
Block a user