mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-04 11:01:16 +01:00
fix(native): refresh prior warm controller projections (#18718)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Native remote sessions keep their controller state in a database checkpoint. > - An admission can also leave a full local copy of a prior run's state. > - The prior owner can commit more state without changing its run ID. > - Warm admission skipped a refresh when the local and saved run IDs matched. > - This pull request refreshes that prior-run copy when no retained warm owner exists. > - The next turn can use the committed state while existing ownership checks remain effective. ## Linked Issues or Issue Description **What happened?** A prior warm run can complete after an earlier admission reads its controller checkpoint. The local copy can still contain a pending command or lack the final terminal event. A later admission sees the same run ID and skips restoration. It can then hold work against that old copy even though the database checkpoint has settled. **Expected behavior** Read the canonical prior-run checkpoint before reuse when no retained in-memory owner exists. Keep current-run state with its controller. Keep unresolved canonical work and invalid checkpoint bindings blocked. **Steps to reproduce** 1. Create a full prior-run local projection while a command is pending. 2. Complete that command and record the terminal in the canonical checkpoint under the same run ID. 3. Start an ordinary remote warm continuation without a retained in-memory owner. 4. Before this change, the local command remains pending. With this change, the completed command and terminal replace the stale copy. **Paperclip version or commit** Reproduced on master `36df8933f0` before changing production code. Related work: #18018 restored per-turn checkpoints; #18343 added controller replacement recovery; #18554 restored missing remote suspension receipts. This change handles a full prior-run projection whose run ID already matches the saved supervisor. It does not change the settlement guard or MCP compatibility policy. ## What Changed - Refresh a prior-run warm controller projection even when its run ID has not changed. - Keep current-run and retained warm owner authority on the existing path. - Add five executor cases for a settled source, a pending canonical command, a current run, legacy file-backed state, and an invalid checkpoint binding. - Add an opt-in Linux continuity case with a stale full local projection and two follow-up turns. - Update execution documentation and the paired session verification recipes. ## Verification - RED before the production edit: the executor fixture retained `pending` in its local command record when the canonical record was `completed`. - GREEN on the final commit: 48 focused tests passed across executor restoration, retained warm owner rotation, per-turn restoration, and remote settlement guards. - `pnpm --filter @paperclipai/server typecheck` passed. The server TypeScript check also passed after the integration fixture was added. - `node scripts/check-maps.mjs` and `git diff --check` passed. - The unit fixture uses the real durable controller serializer and the real executor entry. It stops at current tool authority before provider/backend creation. It proves restoration and guard behavior, not full task acceptance. - Actual protocol RED/GREEN: the same opt-in continuity fixture uses real PostgreSQL, TLS, the current Linux Rust Runner, and a deterministic fake Codex backend. With only the executor restored to its parent revision, continuation fails with `prior_owner_terminal_state_indeterminate` in the settlement guard after one provider turn (20.16 seconds). The PR source completes all three turns with one provider thread and final suspension (34.77 seconds). Hard recovery permits one provider-process replacement; the healthy final follow-up starts no additional process. Both runs use the same Runner binary and owned container. - The exact PR executor source was restored after the baseline comparison, its Git blob was verified, and the worktree is clean. The disposable local container was removed. This fixture proves the controlled stale-projection recovery path; it does not establish a live model or Daytona API result. - Full current-head CI passes, including typecheck, build, general and serialized tests, Rust/Runner checks, all eight E2E shards, and the `ci / e2e` and `ci / verify` aggregate gates. Greptile is 5/5 on the current commit, with no unresolved review threads. No original task was retried and no staging sandbox lifecycle was changed. ## Risks - Admission without a retained owner performs one additional scoped checkpoint read when a full prior-run projection already exists. - The canonical checkpoint must still pass its company, task, session and record validation. A pending canonical command stays held. Current-run and retained owner state are not replaced. - The regression proves a stale local projection defect. It does not establish that a historical incident had those same local bytes. ## Model Used OpenAI Codex, GPT-6 family. The harness does not expose the exact serving model ID or context-window size. Capabilities: repository inspection, code generation, tools, and 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 - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge Co-authored-by: Paperclip <noreply@paperclip.ing>
This commit is contained in:
1 parent
db8a273562
commit
718336eb04
7 files changed
+125
-5
No files matched your search
@@ -2939,8 +2939,11 @@ loss. The marker may describe the first turn, so it cannot identify every later
|
||||
warm checkpoint. Legacy file-backed roots retain their existing checks. A
|
||||
restored checkpoint must match the normalized session before writing its local
|
||||
projection. This adds a scoped checkpoint read and local reconstruction only
|
||||
when warm admission encounters an external marker or an older projection; it
|
||||
adds no streaming checkpoint writes or recurring idle work.
|
||||
when warm admission encounters an external marker or a prior-run projection.
|
||||
A prior-run projection refreshes even when its run ID already matches the saved
|
||||
supervisor: the canonical checkpoint can have advanced after an earlier read.
|
||||
Current-run state and a retained in-memory warm owner keep their existing
|
||||
authority. This adds no streaming checkpoint writes or recurring idle work.
|
||||
|
||||
Ordinary remote `per_turn` continuations restore a valid external controller
|
||||
marker from the referenced run's company/task-bound checkpoint before reading
|
||||
|
||||
@@ -12,6 +12,11 @@ checkpoints and changed process configuration require verified suspension and
|
||||
replacement, while preserving the conversation and starting each turn once.
|
||||
Recovery can restart a provider to refresh its tool contract; process reuse is
|
||||
required for ordinary healthy follow-ups.
|
||||
The `stale-projection` case leaves a full local control projection from just
|
||||
before the prior run's terminal while its database checkpoint is complete. It
|
||||
must refresh that same-run projection before the next turn, then preserve the
|
||||
same Runner and provider history through another healthy follow-up. This is a
|
||||
controlled cache fault, not a reconstruction of a historical incident.
|
||||
Each recovery case includes another healthy turn after the recovered turn and
|
||||
asserts that it does not start another runner or provider process.
|
||||
The same-server case also starts preparation lease renewal during recovery and
|
||||
|
||||
@@ -351,6 +351,14 @@ guard. This changes no actor permission or session-reset authority. Follow the
|
||||
[session recipe](../feature-map/runs-adapters.md#session) for recovery followed by
|
||||
ordinary turns; deterministic runner checks do not establish live provider access.
|
||||
|
||||
Warm continuation without a retained controller owner refreshes a prior-run local
|
||||
projection from its company/task/session-bound canonical checkpoint, including
|
||||
later commits under the same run ID. Current-run state keeps its owner. Pending
|
||||
canonical commands and mismatched checkpoint bindings still hold admission. This
|
||||
adds no agent permission, session-reset authority, or provider replay path. The
|
||||
executor and remote continuity regressions cover this internal recovery boundary;
|
||||
live provider access remains separate evidence.
|
||||
|
||||
Sandbox export carries shallow Git boundaries through the full-bundle fallback
|
||||
and uses normal Git fetch connectivity checks. Existing host ancestry and merge
|
||||
guards remain effective. This grants no task retry or operator recovery authority;
|
||||
|
||||
@@ -102,6 +102,11 @@ The [remote continuity regression](../../doc/testing-native-remote-continuity.md
|
||||
uses a real Linux runner and deterministic provider. Live provider acceptance is
|
||||
a separate check.
|
||||
|
||||
Repeat with a full prior-run controller projection that predates its committed
|
||||
database checkpoint under the same run ID. Admission must refresh that projection
|
||||
before reuse. A pending canonical command must still hold execution; current-run
|
||||
state and a retained warm owner must keep their existing authority.
|
||||
|
||||
In a fresh sandbox workspace, fetch a different published branch with depth one,
|
||||
then save a local result and finish. Verify export imports the shallow history,
|
||||
retains host-only edits, and completes finalization without another provider turn.
|
||||
|
||||
+23
-1
@@ -150,7 +150,7 @@ describeEmbeddedPostgres("native runner restart recovery with real processes", (
|
||||
expect(next.kind).toBe("bootstrap_incomplete");
|
||||
});
|
||||
|
||||
it.skipIf(!process.env.PAPERCLIP_TEST_RUNNER_CONTAINER).each(["warm", "hard", "legacy", "changed", "same-server", "diskless", "diskless-per-turn", "diskless-per-turn-read-retry", "diskless-warm-suspended", "diskless-warm-suspended-read-retry", "diskless-live-inspection"])("preserves remote warm continuity across completed turns: %s", async (scenario) => {
|
||||
it.skipIf(!process.env.PAPERCLIP_TEST_RUNNER_CONTAINER).each(["warm", "hard", "stale-projection", "legacy", "changed", "same-server", "diskless", "diskless-per-turn", "diskless-per-turn-read-retry", "diskless-warm-suspended", "diskless-warm-suspended-read-retry", "diskless-live-inspection"])("preserves remote warm continuity across completed turns: %s", async (scenario) => {
|
||||
const hardRestart = !["warm", "same-server"].includes(scenario);
|
||||
const diskless = scenario.startsWith("diskless");
|
||||
const perTurn = scenario.startsWith("diskless-per-turn");
|
||||
@@ -364,6 +364,28 @@ describeEmbeddedPostgres("native runner restart recovery with real processes", (
|
||||
}
|
||||
expect(changed).toBe(1);
|
||||
}
|
||||
if (scenario === "stale-projection") {
|
||||
const checkpoint = await loadNativeRunnerCheckpoint(fixture.db, { companyId,
|
||||
issueId: fixture.issueId, runId: fixture.runId });
|
||||
expect(checkpoint).not.toBeNull();
|
||||
const sessions = resolve(resolvePaperclipInstanceRoot(), "runtime/paperclip-runner/durable-sessions");
|
||||
let projected = 0;
|
||||
for (const name of await readdir(sessions)) {
|
||||
const root = resolve(sessions, name), path = resolve(root, "control-plane/control-plane-state.json");
|
||||
if (!existsSync(path) || JSON.parse(await readFile(path, "utf8")).identity?.runId !== fixture.runId) continue;
|
||||
checkpoint!.restoreSessionDirectory(root);
|
||||
const state = JSON.parse(await readFile(path, "utf8"));
|
||||
const terminalIndex = state.committedEvents.findLastIndex((event: { eventType: string }) => event.eventType === "run.terminal");
|
||||
expect(terminalIndex).toBeGreaterThan(0);
|
||||
// Retain a full local projection from just before the terminal. The
|
||||
// database keeps the complete same-run checkpoint and all receipts.
|
||||
state.committedEvents = state.committedEvents.slice(0, terminalIndex);
|
||||
state.ackedSourceSeq = state.committedEvents.at(-1).sourceSeq;
|
||||
await writeFile(path, JSON.stringify(state), { mode: 0o600 });
|
||||
projected++;
|
||||
}
|
||||
expect(projected).toBe(1);
|
||||
}
|
||||
if (diskless) {
|
||||
// Preserve sandbox/provider storage and workspace dependencies. Delete
|
||||
// both native controller projections; admission must discover the source.
|
||||
|
||||
@@ -3,6 +3,7 @@ import { readIdleLocalWork, markIdleStartupComplete, markIdleIngressTracked } fr
|
||||
import * as runnerCheckpoint from "./native-runner-checkpoint.js";
|
||||
import * as stagedAttachmentRecovery from "./native-staged-attachment-recovery.js";
|
||||
import * as controllerRecovery from "./native-controller-recovery.js";
|
||||
import { isSettledRemoteWarmControlState } from "./remote-warm-session-recovery.js";
|
||||
import { DurablePrpControlPlane } from "../../vendor/paperclip-runner/index.js";
|
||||
import * as environmentTaskRuntime from "../environment-task-runtime.js";
|
||||
import * as pluginTaskRuntime from "./plugin-runner-task.js";
|
||||
@@ -36,7 +37,8 @@ import {
|
||||
import { execFile, execFileSync, spawn, spawnSync } from "node:child_process";
|
||||
import { tmpdir } from "node:os";
|
||||
import { inspect } from "node:util";
|
||||
import { dirname, join } from "node:path";
|
||||
import { basename, dirname, join } from "node:path";
|
||||
import { resolvePaperclipInstanceRoot } from "../../home-paths.js";
|
||||
import {
|
||||
activityLog,
|
||||
budgetReservations,
|
||||
@@ -14581,6 +14583,78 @@ describe("runnerd provider runtime wiring", () => {
|
||||
}
|
||||
});
|
||||
|
||||
it.each(["settled", "pending", "current-run", "legacy-file", "binding-mismatch"] as const)("refreshes a prior warm controller projection after its same-run checkpoint advances: %s", async mode => {
|
||||
const prior = scopedNativeExecution("warm-same-run-checkpoint-advance");
|
||||
prior.session.lifecyclePolicy = { mode: "warm", idleTimeoutMs: 60_000 };
|
||||
const current = { ...prior, binding: { ...prior.binding,
|
||||
runId: mode === "current-run" ? prior.binding.runId : `${prior.binding.runId}-next` } };
|
||||
const identity = { runId: prior.binding.runId, normalizedSessionId: prior.session.normalizedSessionId!,
|
||||
runnerInstanceId: "runner-warm-checkpoint-advance", environmentLeaseId: "lease-warm-checkpoint-advance",
|
||||
turnId: "prior-turn", itemId: "prior-item" };
|
||||
const root = await initializeScopedRunnerRoot(prior, identity.runnerInstanceId);
|
||||
const controller = new DurablePrpControlPlane({ stateDirectory: join(root, "control-plane"), identity,
|
||||
expectedRunnerVersion: "warm-fixture", expectedRunnerDigest: `sha256:${"a".repeat(64)}` });
|
||||
controller.queueCommand("run.prepare", { provider: prior.provider });
|
||||
const path = join(root, "control-plane/control-plane-state.json");
|
||||
const staleProjection = await readFile(path, "utf8");
|
||||
if (mode !== "pending") {
|
||||
controller.store.state.commands[0]!.status = "completed";
|
||||
controller.store.state.commands[0]!.result = { status: "completed" };
|
||||
controller.store.state.committedEvents.push({ sourceSeq: 1, sourceEventId: "warm-terminal",
|
||||
eventType: "run.terminal", priority: 0, envelope: { ...identity }, deliveryCount: 1, logicalEffectCount: 1 });
|
||||
controller.store.state.ackedSourceSeq = 1;
|
||||
}
|
||||
controller.persistRunAttachTemplate({ provider: prior.provider, normalizedSessionId: identity.normalizedSessionId });
|
||||
let saved!: import("../../vendor/paperclip-runner/index.js").DurablePrpCheckpoint;
|
||||
controller.setCheckpointPersistence(async checkpoint => { saved = checkpoint; });
|
||||
await controller.flushCheckpoint();
|
||||
// A previous admission can leave a full local projection while the owner
|
||||
// later commits its completed command and terminal under the same run ID.
|
||||
await writeFile(path, staleProjection);
|
||||
await mkdir(join(root, "runner"), { recursive: true });
|
||||
await writeFile(join(root, "runner/runner-state.json"), JSON.stringify(durableRunnerState(identity, "suspended")));
|
||||
const supervisorDirectory = join(resolvePaperclipInstanceRoot(), "runtime/paperclip-runner/sessions");
|
||||
await mkdir(supervisorDirectory, { recursive: true });
|
||||
const supervisorPath = join(supervisorDirectory, `${basename(root)}.json`);
|
||||
await writeFile(supervisorPath, JSON.stringify({ schema: "paperclip.native-session-supervisor.v1",
|
||||
snapshot: { identity: { ...prior.binding, sessionId: identity.normalizedSessionId } } }));
|
||||
const restore = vi.spyOn(runnerCheckpoint, "restoreNativeRunnerCheckpoint").mockImplementation(async (_db, binding, directory, sessionId) => {
|
||||
expect(binding).toEqual({ companyId: prior.binding.companyId, issueId: prior.binding.issueId, runId: prior.binding.runId });
|
||||
expect(sessionId).toBe(identity.normalizedSessionId);
|
||||
if (mode === "legacy-file") return null;
|
||||
if (mode === "binding-mismatch") throw new Error("native_runner_checkpoint_binding_mismatch");
|
||||
saved.restoreSessionDirectory(directory);
|
||||
return saved;
|
||||
});
|
||||
const admitted = new Error("warm resume reached current tool authority");
|
||||
state.toolAuthorityDefinitions.mockRejectedValueOnce(admitted);
|
||||
state.createBackend.mockClear();
|
||||
state.createTransport.mockClear();
|
||||
try {
|
||||
await expect(createRunnerdBackend({ db: priorNativeRunDb(prior), execution: current,
|
||||
runnerInstanceId: "next-warm-runner", runnerExecutionTarget: { kind: "remote", transport: "sandbox",
|
||||
remoteCwd: "/workspace", runner: { execute: vi.fn() } } as never })).rejects.toThrow(
|
||||
mode === "binding-mismatch" ? "native_runner_checkpoint_binding_mismatch" : admitted.message);
|
||||
const refreshed = JSON.parse(await readFile(path, "utf8"));
|
||||
const refreshedFromDatabase = mode === "settled" || mode === "pending";
|
||||
expect(refreshed.commands).toEqual([expect.objectContaining({ type: "run.prepare",
|
||||
status: mode === "settled" ? "completed" : "pending" })]);
|
||||
expect(refreshed.committedEvents).toEqual(mode === "settled"
|
||||
? [expect.objectContaining({ eventType: "run.terminal" })] : []);
|
||||
expect(isSettledRemoteWarmControlState(refreshed, identity)).toBe(mode === "settled");
|
||||
if (refreshedFromDatabase) expect(refreshed.runAttachTemplate).not.toBeNull();
|
||||
else expect(await readFile(path, "utf8")).toBe(staleProjection);
|
||||
expect(restore).toHaveBeenCalledTimes(mode === "current-run" ? 0 : 1);
|
||||
expect(state.createBackend).not.toHaveBeenCalled();
|
||||
expect(state.createTransport).not.toHaveBeenCalled();
|
||||
} finally {
|
||||
state.toolAuthorityDefinitions.mockReset().mockResolvedValue([]);
|
||||
restore.mockRestore();
|
||||
await controller.stop();
|
||||
await rm(supervisorPath, { force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it.each(["suspended", "backup", "codex", "file-projection", "legacy-file-no-checkpoint", "file-checkpoint-binding-error",
|
||||
"missing-checkpoint", "checkpoint-binding-error", "foreign-task", "foreign-marker-session",
|
||||
"missing-marker", "malformed-marker", "unknown-marker-schema", "active-owner", "foreign-prior-scope", "unsuspended-runner"] as const)(
|
||||
|
||||
@@ -5966,7 +5966,10 @@ async function restoreWarmRunnerdController(input: {
|
||||
issueId = prior.issueId;
|
||||
}
|
||||
}
|
||||
if (identity?.runId !== runId) {
|
||||
// A prior admission can leave a full projection before later commits under
|
||||
// the same run ID. Without a retained owner, refresh it from its canonical
|
||||
// checkpoint; current-run state still belongs to the active controller.
|
||||
if (identity?.runId !== runId || (!retainedRunId && runId !== input.execution.binding.runId)) {
|
||||
const restored = await restoreNativeRunnerCheckpoint(input.db,
|
||||
{ companyId: input.execution.binding.companyId, issueId, runId }, root, nativeSessionKey(input.execution));
|
||||
// A legacy file-backed root still follows its existing prior-owner checks.
|
||||
|
||||
Reference in new issue
Block a user