mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-10 12:37:55 +01:00
fix(native): prove a drained remote start had no turn when its controller only read the goal (#20137)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - When a graceful drain suspends a remote native run during its start, restart recovery may bootstrap it again only if the run's Postgres controller checkpoint proves that no provider turn exists > - That proof accepted `run.prepare`, `session.open` and session-open events > - A codex controller reads the session goal (`session.goal.get`, a read-only query) right after it opens the session and before it sends the first turn. A drain that lands in that window leaves a third command in the checkpoint > - The proof refused any checkpoint with a third command. Recovery called it `ambiguous_remote_recovery_evidence` and blocked replay, so the task waited for the board > - This pull request lets the proof accept the runner maintenance and read-only session commands that already count as maintenance in the local proof, and reads every recorded command > - The benefit is that a run drained in the first seconds of a per-turn Daytona start bootstraps again on its own ## Linked Issues or Issue Description Refs #20136 **What happened?** Daytona per-turn restart-chaos wave w-20261108T225520-s9501 (master15b987d9b8), task t06, subtask CHA-13, run `687578c9`: - A K2 graceful drain at 23:11:55.096 suspended the run 4 s into its start. - Restart recovery at 23:12:03 blocked it as `native_restart_recovery_blocked` / `ambiguous_remote_recovery_evidence`, and replay was blocked as `preserve_without_replay_v1`. - The subtask rested `blocked`. Its parent CHA-10 then failed R1. I decrypted the run's controller checkpoint with the kept instance's local key: - **commands:** `run.prepare` (completed), `session.open` (completed), `session.goal.get` (completed, written 18 ms before the drain); - **events:** `session.started`, `session.capabilities.updated`, `session.goal.snapshot` ×2; - **cursor:** `ackedSourceSeq` 4, `commandCount` 3. No turn was ever sent. `nativeRunnerCheckpointRemoteBootstrapProof` rejected the checkpoint because only the parts `command:1` and `command:2` were allowed, and `runnerdStateHasNoProviderTurn` accepted only bootstrap commands. **Expected behavior** A controller record that holds only a completed bootstrap, read-only session commands and session-open events proves that no provider turn exists. The run bootstraps again (`remote_session_opened_without_turn`) without a Retry. **Steps to reproduce** 1. Start a codex run on a remote sandbox and drain the control plane after `session.open` completes and `session.goal.get` is issued, but before `turn.start`. 2. Restart. Before this change, recovery blocks the run with `ambiguous_remote_recovery_evidence`. **Paperclip version or commit**15b987d9b8(wave). Branch rebased on current master. ## What Changed - `native-runner-checkpoint.ts` (`nativeRunnerCheckpointRemoteBootstrapProof`): - accepts up to 8 `command:N` parts and reads them all in order; - requires the stored command parts to match the cursor's `commandCount`, so no command can be missing; - keeps the 16-event bound. - `native-bootstrap-proof.ts` (`runnerdStateHasNoProviderTurn`): besides `run.prepare` and `session.open`, also accepts the existing `RUNNER_MAINTENANCE_COMMAND_TYPES`: - `runner.drain`, `runner.suspend` and `runner.shutdown`; - `session.snapshot` and `session.goal.get`. The local opened-session proof already treats these as maintenance; none of them sends provider input. - `native-runner-restart-recovery.integration.test.ts`: - a red/green test with the exact CHA-13 shape, covering both the recovery claim and the executor's restored-checkpoint proof; - a held case where a `session.goal.set` follows the read. ## Verification - Red/green: on master, the new test is blocked. With the change, recovery claims `bootstrap_incomplete` with the reason `remote_session_opened_without_turn`, and the executor's proof agrees. - Existing held cases still hold: a turn was issued, a turn event was committed, an acknowledged event is missing, a provider turn event was projected, and the new `session.goal.set` case. - `vitest run` over the native bootstrap, restart-recovery and runner-checkpoint test files and `native-session-executor.test.ts`: 9 files, 1542 tests passed (14 skipped). - `PAPERCLIP_TYPECHECK_LOW_MEMORY=1 pnpm typecheck` (`server`): passed. ## Risks - Low risk: - The added commands are the same maintenance and read-only set the local proof already accepts. - Any turn command, any non-session-open event, a missing acknowledged event, or a command count that differs from the stored parts still fails closed. - Not changed: when replay was already blocked and the saved work was later recovered, the task still waits for a board Retry. This fix keeps that block from happening for this shape. ## 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
6451d2bfbf
commit
e194e1930b
3 files changed
+43
-8
No files matched your search
@@ -56,8 +56,9 @@ export function runnerdStateHasNoProviderTurn(state: Record<string, unknown>): b
|
||||
if (record(state.bootstrapHistory).adopted === true) return false;
|
||||
if (!Array.isArray(state.commands) || !Array.isArray(state.committedEvents)) return false;
|
||||
const commands = state.commands.map(record);
|
||||
if (!commands.every(command => RUNNERD_BOOTSTRAP_COMMAND_TYPES.has(String(command.type)) &&
|
||||
(command.status === "pending" || command.status === "completed"))) return false;
|
||||
if (!commands.every(command => RUNNER_MAINTENANCE_COMMAND_TYPES.has(String(command.type)) ||
|
||||
(RUNNERD_BOOTSTRAP_COMMAND_TYPES.has(String(command.type)) &&
|
||||
(command.status === "pending" || command.status === "completed")))) return false;
|
||||
const events = state.committedEvents.map(record);
|
||||
// Every event since the first must still be here, in order, so none is hidden.
|
||||
if (typeof state.ackedSourceSeq !== "number" || !Number.isSafeInteger(state.ackedSourceSeq) ||
|
||||
|
||||
@@ -260,7 +260,8 @@ export async function loadNativeUnclaimedBootstrapCheckpoint(
|
||||
} catch { return null; }
|
||||
}
|
||||
|
||||
const BOOTSTRAP_CHECKPOINT_PARTS = new Set(["header", "cursor", "command:1", "command:2"]);
|
||||
const BOOTSTRAP_CHECKPOINT_PARTS = new Set(["header", "cursor"]);
|
||||
const MAX_BOOTSTRAP_CHECKPOINT_COMMANDS = 8;
|
||||
/** Session-open events are few; a longer journal is never a bootstrap. */
|
||||
const MAX_BOOTSTRAP_CHECKPOINT_EVENTS = 16;
|
||||
|
||||
@@ -282,13 +283,15 @@ export async function nativeRunnerCheckpointRemoteBootstrapProof(
|
||||
db: Pick<Db, "select">, binding: Binding, expected: { runnerInstanceId: string; normalizedSessionId: string },
|
||||
): Promise<NativeRemoteBootstrapProof | null> {
|
||||
try {
|
||||
const limit = BOOTSTRAP_CHECKPOINT_PARTS.size + MAX_BOOTSTRAP_CHECKPOINT_EVENTS;
|
||||
const limit = BOOTSTRAP_CHECKPOINT_PARTS.size + MAX_BOOTSTRAP_CHECKPOINT_COMMANDS + MAX_BOOTSTRAP_CHECKPOINT_EVENTS;
|
||||
const parts = await db.select({ part: nativeRunnerCheckpoints.part }).from(nativeRunnerCheckpoints).where(and(
|
||||
eq(nativeRunnerCheckpoints.companyId, binding.companyId), eq(nativeRunnerCheckpoints.issueId, binding.issueId),
|
||||
eq(nativeRunnerCheckpoints.runId, binding.runId))).limit(limit + 1);
|
||||
const eventParts = parts.map(row => row.part).filter(part => /^event:\d+$/.test(part));
|
||||
if (!parts.length || parts.length > limit
|
||||
|| parts.some(row => !BOOTSTRAP_CHECKPOINT_PARTS.has(row.part) && !/^event:\d+$/.test(row.part))) return null;
|
||||
const commandParts = parts.map(row => row.part).filter(part => /^command:\d+$/.test(part));
|
||||
if (!parts.length || parts.length > limit || eventParts.length > MAX_BOOTSTRAP_CHECKPOINT_EVENTS
|
||||
|| commandParts.length > MAX_BOOTSTRAP_CHECKPOINT_COMMANDS
|
||||
|| parts.some(row => !BOOTSTRAP_CHECKPOINT_PARTS.has(row.part) && !/^(?:event|command):\d+$/.test(row.part))) return null;
|
||||
const checkpoint = await loadNativeRunnerCheckpoint(db, binding);
|
||||
if (!checkpoint || checkpoint.predecessor !== null
|
||||
|| checkpoint.recoveryEvidence.runnerInstanceId !== expected.runnerInstanceId
|
||||
@@ -301,11 +304,13 @@ export async function nativeRunnerCheckpointRemoteBootstrapProof(
|
||||
};
|
||||
const header = read("header");
|
||||
const cursor = read("cursor");
|
||||
if (header?.schema !== "paperclip.runner.controller-checkpoint.v2" || !cursor) return null;
|
||||
if (header?.schema !== "paperclip.runner.controller-checkpoint.v2" || !cursor
|
||||
|| cursor.commandCount !== commandParts.length) return null;
|
||||
const state: Record<string, unknown> = {
|
||||
...(header.state as Record<string, unknown>),
|
||||
ackedSourceSeq: cursor.ackedSourceSeq,
|
||||
commands: ["command:1", "command:2"].map(read).filter(command => command !== undefined),
|
||||
commands: commandParts.map(part => Number(part.slice("command:".length))).sort((a, b) => a - b)
|
||||
.map(seq => read(`command:${seq}`)),
|
||||
committedEvents: eventParts.map(part => Number(part.slice("event:".length))).sort((a, b) => a - b)
|
||||
.map(seq => read(`event:${seq}`)),
|
||||
};
|
||||
|
||||
@@ -2589,6 +2589,33 @@ process.on("SIGTERM", () => child.kill("SIGTERM"));
|
||||
expect(run).toMatchObject({ status: "running", errorCode: null });
|
||||
});
|
||||
|
||||
it("restarts a drained remote session whose controller only read its goal after opening it", async () => {
|
||||
const readGoal = (core: DurablePrpControlPlane) => {
|
||||
openSession(core, ["session.started", "session.capabilities.updated", "session.goal.snapshot", "session.goal.snapshot"]);
|
||||
core.queueCommand("session.goal.get", {});
|
||||
const goal = core.store.state.commands.at(-1)!;
|
||||
goal.status = "completed";
|
||||
goal.result = { goal: null } as never;
|
||||
};
|
||||
const fixture = await seedClaimedRemoteBootstrap("SESSION-GOAL-READ", { mutate: readGoal });
|
||||
expect(await claimNativeRestartRecoveries({ db: fixture.db, controller: successor,
|
||||
restartKind: "hard", runIds: [fixture.runId] })).toEqual([expect.objectContaining({
|
||||
kind: "bootstrap_incomplete", runId: fixture.runId, controllerGeneration: 2 })]);
|
||||
const [coordinator] = await fixture.db.select().from(nativeRunFinalizations)
|
||||
.where(eq(nativeRunFinalizations.runId, fixture.runId));
|
||||
expect(coordinator.recoveryHistory.at(-1)).toMatchObject({ disposition: "bootstrap_incomplete",
|
||||
reason: "remote_session_opened_without_turn" });
|
||||
const executor = await seedClaimedRemoteBootstrap("SESSION-GOAL-READ-EXECUTOR", { mutate: readGoal });
|
||||
const root = resolve(runtimeRoot, `goal-read-${executor.runId}`);
|
||||
try {
|
||||
expect(await restoreNativeRunnerCheckpoint(executor.db, { companyId, issueId: executor.issueId, runId: executor.runId },
|
||||
root)).not.toBeNull();
|
||||
expect(runnerdStateProvesNoRemoteProviderTurn(root)).toBe(true);
|
||||
} finally {
|
||||
await rm(root, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it("lets the executor prove a drained remote session had no turn from its restored checkpoint", async () => {
|
||||
const fixture = await seedClaimedRemoteBootstrap("SESSION-OPENED-EXECUTOR", { mutate: core => openSession(core) });
|
||||
const root = resolve(runtimeRoot, `opened-${fixture.runId}`);
|
||||
@@ -2607,6 +2634,8 @@ process.on("SIGTERM", () => child.kill("SIGTERM"));
|
||||
it.each<[string, Parameters<typeof seedClaimedRemoteBootstrap>[1], { eventType: string } | null]>([
|
||||
["a turn was issued", { mutate: core => { openSession(core);
|
||||
core.queueCommand("turn.start", { prompt: "Prior work must stay held." }); } }, null],
|
||||
["a goal was set after a read", { mutate: core => { openSession(core); core.queueCommand("session.goal.get", {});
|
||||
core.queueCommand("session.goal.set", { objective: "Prior work must stay held." }); } }, null],
|
||||
["a turn event was committed", { mutate: core => openSession(core, ["session.started", "turn.started"]) }, null],
|
||||
["an acknowledged event is missing", { mutate: core => { openSession(core); core.store.state.ackedSourceSeq = 4; } }, null],
|
||||
["a provider turn event was projected", { mutate: core => openSession(core) }, { eventType: "turn.started" }],
|
||||
|
||||
Reference in new issue
Block a user