mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-06 20:34:57 +02:00
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - The native runner can execute a task inside a Daytona sandbox. > - The sandbox can keep running when the Paperclip controller restarts. > - Recovery treated sandbox process IDs as local process IDs and selected the wrong recovery path. > - Live verification also found races between startup, shutdown, and queued task cleanup. > - This pull request verifies the existing remote owner and orders those transitions. > - Users can continue the same task and provider session after a controller restart. ## Linked Issues or Issue Description **What happened?** The Daytona `recover-controller` cases failed with `runner_state_identity_mismatch`. Remote process IDs can be absent on the controller or collide with unrelated local processes. Recovery then looked for remote state in the local runner directory. Later turns could also start before the previous executor released its sandbox resources. **Expected behavior** Reconnect to the original sandbox and authenticated runner. Preserve the task, provider session, and queued comments. Reject a replacement sandbox or mismatched identity. Do not start another provider during reattachment. **Steps to reproduce** Run the `everyday-workflows` `recover-controller` case for `runner-codex` or `runner-acpx-claude` in Daytona. The browser creates a Python tool, requests a revision, restarts the controller during execution, and queues another revision. It then downloads and tests the final ZIP. Related: #13682 is the preceding operational fix. #13291 addresses legacy sandbox conversation recovery, a different execution path. #13666 includes broader run-capacity work; this change guards cleanup of an existing native task executor. ## What Changed - Add remote runner recovery without interpreting sandbox PIDs on the controller. - Verify the original provider lease, remote workspace, durable state, process marker, and authenticated PRP authority before adoption. - Compare the process marker with live Linux boot identity and start ticks to reject PID reuse. Read virtual proc files through the guaranteed Node runtime; unavailable proof blocks adoption without blocking a fresh launch. - Make the E2E supervisor own the actual server process so forced restart cannot leave a late database closer behind. - Scope the chat delivery lease test to its own fixture instead of draining other tests’ pending deliveries. - Preserve provider-attempt counts and recorded evidence during reattachment. - Serialize an idle-session checkpoint with admission of the next native turn. - Wait for an in-progress startup to acknowledge restart detachment. Fail after a bounded deadline if it cannot. - Keep a queued comment waiting until the previous native task executor releases its resources. Allow unrelated tasks to continue. - Update the Daytona image's resolved lock digest to match current dependency manifests. - Add classifier, ownership, process, startup, checkpoint, and queued-admission regression tests. Document recovery behavior. ## Verification - 415 focused tests passed across native execution, restart recovery, workspace synchronization, queued admission, and real-process restart tests. The final Node-based fingerprint change passed all 375 native-session tests. - Runner harness unit tests: 394 passed. Chat integration shard 2: 335 passed after fixture isolation. - The exact fingerprint command succeeded twice in a disposable Daytona sandbox and returned the same identity; the sandbox was deleted. - 11 real-process restart integration tests passed, including absent and colliding remote PIDs. - Repository typecheck and final build passed. Broad local checks found machine-dependent database startup and timing failures; focused retries passed. The final-revision PR pipeline is green. One unrelated browser shard hit a five-second blank-page timeout on the first run and passed its targeted retry. - Final-revision local headed browser E2E: `everyday-workflows.runner-acpx-claude.daytona.recover-controller` passed on attempt 1 in 4.7 minutes, **40/40 checks**. Manual browser inspection confirmed Done, all three ZIPs, and delivery of the queued follow-up. All three runs succeeded using the same provider session. The harness downloaded and independently tested the final artifact. - Final-revision Daytona campaign: https://github.com/paperclipai/paperclip/actions/runs/35463999611 — **Codex passed first attempt (4.8 minutes); ACPX Claude passed first attempt (6.1 minutes)**. Campaign aggregation/publication is finishing; both test jobs succeeded. - Greptile reviewed `beb08d8493b3286f5bb988dead369ff8c96a395d`: **5/5**, no open findings. - Staging browser verification is pending selection of a disposable staging instance and removal of a Chrome extension UI block. ## Risks - Recovery now depends on the original sandbox remaining available. A replacement or mismatched identity still blocks adoption. - Shutdown waits up to 30 seconds for a native startup to reach a safe detach point. An unfinished startup returns a clear failure instead of a false detach receipt. - Queued native work on the same task waits for cleanup. Unrelated tasks remain eligible. - The image digest update rebuilds the Daytona runtime image. No database migration or public API change is included. ## Model Used OpenAI Codex, GPT-6, with repository inspection, code execution, and browser tools. The runtime does not expose the exact deployed model ID or context-window size. ## 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>
409 lines
13 KiB
TypeScript
409 lines
13 KiB
TypeScript
import { runnerE2ETypeScriptProcessArgs } from "./web-server-command.js";
|
|
import { qualifyLegacyClaudeCli } from "./legacy-claude-cli.js";
|
|
import { spawn, type ChildProcess } from "node:child_process";
|
|
import { createWriteStream } from "node:fs";
|
|
import { mkdir, readFile, rename, writeFile } from "node:fs/promises";
|
|
import path from "node:path";
|
|
import { prepareRunnerE2EServerConfig } from "./server-config.js";
|
|
import {
|
|
assertIsolatedServerEnvironment,
|
|
buildPaperclipServerEnvironment,
|
|
runnerE2EServerControlPaths,
|
|
} from "./harness-env.js";
|
|
|
|
function required(name: string) {
|
|
const value = process.env[name]?.trim();
|
|
if (!value) throw new Error(`${name} is required`);
|
|
return value;
|
|
}
|
|
|
|
const logPath = required("PAPERCLIP_RUNNER_E2E_SERVER_LOG");
|
|
const temporaryRoot = required("PAPERCLIP_RUNNER_E2E_TEMP_ROOT");
|
|
const paperclipHome = required("PAPERCLIP_HOME");
|
|
const configPath = required("PAPERCLIP_CONFIG");
|
|
const port = required("PAPERCLIP_RUNNER_E2E_PORT");
|
|
const repositoryRoot = path.resolve(import.meta.dirname, "../..");
|
|
const paperclipCli = path.join(repositoryRoot, "tests/runner-e2e/server-entry.ts");
|
|
const {
|
|
controlDirectory,
|
|
restartRequestPath,
|
|
restartAcknowledgementPath: restartAckPath,
|
|
} = runnerE2EServerControlPaths(temporaryRoot);
|
|
const restartTimeoutMs = 180_000;
|
|
const gracefulStopTimeoutMs = 30_000;
|
|
const serverEnvironment = buildPaperclipServerEnvironment(process.env, {
|
|
NODE_ENV: "test",
|
|
PORT: port,
|
|
// Keep provider caches attempt-private without changing Playwright's browser
|
|
// cache lookup in the parent process.
|
|
XDG_CACHE_HOME: path.join(temporaryRoot, "xdg-cache"),
|
|
PAPERCLIP_HOME: paperclipHome,
|
|
PAPERCLIP_CONFIG: configPath,
|
|
PAPERCLIP_INSTANCE_ID: required("PAPERCLIP_INSTANCE_ID"),
|
|
PAPERCLIP_AGENT_JWT_SECRET: required("PAPERCLIP_AGENT_JWT_SECRET"),
|
|
PAPERCLIP_DECISION_SIGNING_SECRET: required(
|
|
"PAPERCLIP_DECISION_SIGNING_SECRET",
|
|
),
|
|
PAPERCLIP_TOOL_ACTION_SIGNING_SECRET: required(
|
|
"PAPERCLIP_TOOL_ACTION_SIGNING_SECRET",
|
|
),
|
|
BETTER_AUTH_SECRET: required("BETTER_AUTH_SECRET"),
|
|
PAPERCLIP_BIND: "loopback",
|
|
PAPERCLIP_BIND_HOST: "127.0.0.1",
|
|
PAPERCLIP_DEPLOYMENT_MODE: "local_trusted",
|
|
PAPERCLIP_DEPLOYMENT_EXPOSURE: "private",
|
|
SERVE_UI: "true",
|
|
PAPERCLIP_STORAGE_PROVIDER: "local_disk",
|
|
PAPERCLIP_STORAGE_LOCAL_DIR: path.join(temporaryRoot, "storage"),
|
|
PAPERCLIP_SECRETS_PROVIDER: "local_encrypted",
|
|
PAPERCLIP_SECRETS_STRICT_MODE: "true",
|
|
PAPERCLIP_DB_BACKUP_ENABLED: "false",
|
|
PAPERCLIP_DB_BACKUP_DIR: path.join(temporaryRoot, "backups"),
|
|
// Onboarding normally opens the app after listen. Browser ownership belongs
|
|
// to Playwright in this harness, so never create a developer desktop tab.
|
|
PAPERCLIP_OPEN_ON_LISTEN: "false",
|
|
});
|
|
assertIsolatedServerEnvironment(serverEnvironment, {
|
|
temporaryRoot,
|
|
paperclipHome,
|
|
configPath,
|
|
});
|
|
const definedServerEnvironment = Object.fromEntries(
|
|
Object.entries(serverEnvironment).filter(
|
|
(entry): entry is [string, string] => entry[1] !== undefined,
|
|
),
|
|
);
|
|
|
|
await Promise.all([
|
|
mkdir(path.dirname(logPath), { recursive: true }),
|
|
mkdir(controlDirectory, { recursive: true, mode: 0o700 }),
|
|
]);
|
|
const log = createWriteStream(logPath, { flags: "a", mode: 0o600 });
|
|
const expectedStops = new WeakSet<ChildProcess>();
|
|
const childErrors = new WeakMap<ChildProcess, Error>();
|
|
let child: ChildProcess | null = null;
|
|
let unexpectedChildFailure: Error | null = null;
|
|
let shutdownSignal: NodeJS.Signals | null = null;
|
|
let activeRestartRequestId: string | null = null;
|
|
|
|
function appendLog(message: string) {
|
|
process.stderr.write(message);
|
|
log.write(message);
|
|
}
|
|
|
|
function shutdownRequested() {
|
|
return shutdownSignal !== null;
|
|
}
|
|
|
|
function childExited(candidate: ChildProcess) {
|
|
return candidate.exitCode !== null || candidate.signalCode !== null;
|
|
}
|
|
|
|
function describeChildExit(candidate: ChildProcess) {
|
|
const spawnError = childErrors.get(candidate);
|
|
if (spawnError) return `server spawn failed: ${spawnError.message}`;
|
|
return `server exited code=${String(candidate.exitCode)} signal=${String(candidate.signalCode)}`;
|
|
}
|
|
|
|
function startServer() {
|
|
if (shutdownRequested()) {
|
|
throw new Error("Refusing to start Paperclip after wrapper shutdown");
|
|
}
|
|
const candidate = spawn(
|
|
process.execPath,
|
|
runnerE2ETypeScriptProcessArgs(repositoryRoot, paperclipCli, ["onboard", "--yes", "--run"]),
|
|
{
|
|
cwd: repositoryRoot,
|
|
env: definedServerEnvironment,
|
|
stdio: ["ignore", "pipe", "pipe"],
|
|
// Stay in the launcher-created process group. That lets the launcher stop
|
|
// Playwright, this wrapper, Paperclip, embedded Postgres, and runner children
|
|
// as one verified tree even if graceful web-server shutdown stalls.
|
|
detached: false,
|
|
},
|
|
);
|
|
child = candidate;
|
|
|
|
candidate.stdout?.on("data", (chunk) => {
|
|
process.stdout.write(chunk);
|
|
log.write(chunk);
|
|
});
|
|
candidate.stderr?.on("data", (chunk) => {
|
|
process.stderr.write(chunk);
|
|
log.write(chunk);
|
|
});
|
|
candidate.once("error", (error) => {
|
|
childErrors.set(candidate, error);
|
|
if (!expectedStops.has(candidate) && !shutdownRequested()) {
|
|
unexpectedChildFailure = new Error(
|
|
`Paperclip server spawn failed: ${error.message}`,
|
|
);
|
|
}
|
|
});
|
|
candidate.once("exit", () => {
|
|
appendLog(`\n${describeChildExit(candidate)}\n`);
|
|
if (!expectedStops.has(candidate) && !shutdownRequested()) {
|
|
unexpectedChildFailure = new Error(
|
|
`Paperclip server stopped unexpectedly: ${describeChildExit(candidate)}`,
|
|
);
|
|
}
|
|
});
|
|
|
|
// A shutdown may arrive in the synchronous interval around spawn. Never let
|
|
// that race create an unowned replacement server.
|
|
if (shutdownSignal) {
|
|
expectedStops.add(candidate);
|
|
try {
|
|
candidate.kill(shutdownSignal);
|
|
} catch {
|
|
// The process may have failed during spawn.
|
|
}
|
|
}
|
|
return candidate;
|
|
}
|
|
|
|
function delay(milliseconds: number) {
|
|
return new Promise<void>((resolve) => setTimeout(resolve, milliseconds));
|
|
}
|
|
|
|
async function waitForExit(candidate: ChildProcess, timeoutMs: number) {
|
|
if (childExited(candidate) || childErrors.has(candidate)) return true;
|
|
return await new Promise<boolean>((resolve) => {
|
|
let settled = false;
|
|
const finish = (exited: boolean) => {
|
|
if (settled) return;
|
|
settled = true;
|
|
clearTimeout(timeout);
|
|
candidate.off("exit", onExit);
|
|
candidate.off("error", onError);
|
|
resolve(exited);
|
|
};
|
|
const onExit = () => finish(true);
|
|
const onError = () => finish(true);
|
|
const timeout = setTimeout(() => finish(false), timeoutMs);
|
|
candidate.once("exit", onExit);
|
|
candidate.once("error", onError);
|
|
});
|
|
}
|
|
|
|
async function stopServer(
|
|
candidate: ChildProcess,
|
|
signal: NodeJS.Signals = "SIGTERM",
|
|
) {
|
|
expectedStops.add(candidate);
|
|
if (childExited(candidate) || childErrors.has(candidate)) return;
|
|
try {
|
|
candidate.kill(signal);
|
|
} catch {
|
|
if (childExited(candidate) || childErrors.has(candidate)) return;
|
|
throw new Error("Could not signal the Paperclip server to stop");
|
|
}
|
|
if (await waitForExit(candidate, gracefulStopTimeoutMs)) return;
|
|
|
|
appendLog(
|
|
`\nPaperclip did not stop within ${gracefulStopTimeoutMs}ms; sending SIGKILL\n`,
|
|
);
|
|
try {
|
|
candidate.kill("SIGKILL");
|
|
} catch {
|
|
if (childExited(candidate) || childErrors.has(candidate)) return;
|
|
throw new Error("Could not force the Paperclip server to stop");
|
|
}
|
|
if (!(await waitForExit(candidate, 5_000))) {
|
|
throw new Error("Paperclip server did not exit after SIGKILL");
|
|
}
|
|
}
|
|
|
|
async function waitForHealth(candidate: ChildProcess) {
|
|
const deadline = Date.now() + restartTimeoutMs;
|
|
const healthUrl = `http://127.0.0.1:${port}/api/health`;
|
|
while (Date.now() < deadline) {
|
|
if (shutdownRequested()) {
|
|
throw new Error("Wrapper shutdown interrupted the Paperclip restart");
|
|
}
|
|
if (childErrors.has(candidate) || childExited(candidate)) {
|
|
throw new Error(
|
|
`Replacement Paperclip server could not start: ${describeChildExit(candidate)}`,
|
|
);
|
|
}
|
|
try {
|
|
const response = await fetch(healthUrl, {
|
|
signal: AbortSignal.timeout(1_000),
|
|
});
|
|
if (response.ok) return;
|
|
} catch {
|
|
// The replacement process may still be booting.
|
|
}
|
|
await delay(250);
|
|
}
|
|
throw new Error(
|
|
`Replacement Paperclip server did not become healthy within ${restartTimeoutMs}ms`,
|
|
);
|
|
}
|
|
|
|
async function waitForHealthToStop() {
|
|
const healthUrl = `http://127.0.0.1:${port}/api/health`;
|
|
const deadline = Date.now() + gracefulStopTimeoutMs;
|
|
while (Date.now() < deadline) {
|
|
if (shutdownRequested()) {
|
|
throw new Error("Wrapper shutdown interrupted the Paperclip restart");
|
|
}
|
|
try {
|
|
await fetch(healthUrl, { signal: AbortSignal.timeout(500) });
|
|
} catch {
|
|
return;
|
|
}
|
|
await delay(100);
|
|
}
|
|
throw new Error(
|
|
"The old Paperclip server remained healthy after its launcher exited",
|
|
);
|
|
}
|
|
|
|
interface RestartRequest {
|
|
requestId: string;
|
|
}
|
|
|
|
async function readRestartRequest(): Promise<RestartRequest | null> {
|
|
let encoded: string;
|
|
try {
|
|
encoded = await readFile(restartRequestPath, "utf8");
|
|
} catch (error) {
|
|
if ((error as NodeJS.ErrnoException).code === "ENOENT") return null;
|
|
throw error;
|
|
}
|
|
let value: unknown;
|
|
try {
|
|
value = JSON.parse(encoded);
|
|
} catch {
|
|
// The writer may not have completed its atomic replacement yet.
|
|
return null;
|
|
}
|
|
if (!value || typeof value !== "object" || Array.isArray(value)) return null;
|
|
const requestId = (value as { requestId?: unknown }).requestId;
|
|
if (
|
|
typeof requestId !== "string" ||
|
|
!/^[A-Za-z0-9._:-]{1,200}$/.test(requestId)
|
|
) {
|
|
return null;
|
|
}
|
|
return { requestId };
|
|
}
|
|
|
|
async function writeRestartAck(
|
|
requestId: string,
|
|
status: "ready" | "failed",
|
|
message?: string,
|
|
) {
|
|
const temporaryAckPath = `${restartAckPath}.${process.pid}.tmp`;
|
|
await writeFile(
|
|
temporaryAckPath,
|
|
`${JSON.stringify({
|
|
requestId,
|
|
status,
|
|
completedAt: new Date().toISOString(),
|
|
...(message ? { message } : {}),
|
|
})}\n`,
|
|
{ encoding: "utf8", mode: 0o600 },
|
|
);
|
|
await rename(temporaryAckPath, restartAckPath);
|
|
}
|
|
|
|
async function restartServer(requestId: string) {
|
|
activeRestartRequestId = requestId;
|
|
appendLog(`\nRestart request ${requestId}: stopping Paperclip\n`);
|
|
const previous = child;
|
|
if (!previous) throw new Error("No Paperclip server is available to restart");
|
|
await stopServer(previous);
|
|
if (child === previous) child = null;
|
|
// Do not mistake an orphaned old server for a healthy replacement. The port
|
|
// must stop answering before the next launcher is allowed to start.
|
|
await waitForHealthToStop();
|
|
if (shutdownRequested()) {
|
|
throw new Error("Wrapper shutdown interrupted the Paperclip restart");
|
|
}
|
|
|
|
appendLog(`Restart request ${requestId}: starting Paperclip\n`);
|
|
const replacement = startServer();
|
|
await waitForHealth(replacement);
|
|
if (shutdownRequested()) {
|
|
throw new Error("Wrapper shutdown interrupted the Paperclip restart");
|
|
}
|
|
await writeRestartAck(requestId, "ready");
|
|
appendLog(`Restart request ${requestId}: Paperclip is healthy\n`);
|
|
activeRestartRequestId = null;
|
|
}
|
|
|
|
for (const signal of ["SIGINT", "SIGTERM", "SIGHUP"] as const) {
|
|
process.on(signal, () => {
|
|
if (shutdownSignal) return;
|
|
shutdownSignal = signal;
|
|
if (!child) return;
|
|
expectedStops.add(child);
|
|
try {
|
|
child.kill(signal);
|
|
} catch {
|
|
// The Paperclip process may already have exited.
|
|
}
|
|
});
|
|
}
|
|
|
|
async function supervise() {
|
|
const executionIds: string[] = JSON.parse(process.env.PAPERCLIP_RUNNER_E2E_EXECUTION_IDS ?? "[]");
|
|
if (executionIds.some(id => id.includes(".legacy-claude.local."))) {
|
|
definedServerEnvironment.PATH = await qualifyLegacyClaudeCli(temporaryRoot, definedServerEnvironment);
|
|
}
|
|
const databaseReservation = await prepareRunnerE2EServerConfig({
|
|
temporaryRoot,
|
|
configPath,
|
|
serverPort: Number(port),
|
|
});
|
|
// Postgres needs the socket itself; release immediately before child spawn.
|
|
await databaseReservation?.close();
|
|
startServer();
|
|
let lastRestartRequestId: string | null = null;
|
|
while (!shutdownRequested()) {
|
|
if (unexpectedChildFailure) throw unexpectedChildFailure;
|
|
const request = await readRestartRequest();
|
|
if (request && request.requestId !== lastRestartRequestId) {
|
|
lastRestartRequestId = request.requestId;
|
|
await restartServer(request.requestId);
|
|
}
|
|
await delay(200);
|
|
}
|
|
|
|
const running = child;
|
|
if (running) await stopServer(running, shutdownSignal ?? "SIGTERM");
|
|
}
|
|
|
|
let exitCode = 0;
|
|
try {
|
|
await supervise();
|
|
} catch (error) {
|
|
exitCode = 1;
|
|
const message = error instanceof Error ? error.message : String(error);
|
|
appendLog(`\nPaperclip E2E server supervisor failed: ${message}\n`);
|
|
if (activeRestartRequestId) {
|
|
try {
|
|
await writeRestartAck(activeRestartRequestId, "failed", message);
|
|
} catch (ackError) {
|
|
appendLog(
|
|
`Failed to write restart acknowledgement: ${ackError instanceof Error ? ackError.message : String(ackError)}\n`,
|
|
);
|
|
}
|
|
}
|
|
const running = child;
|
|
if (running) {
|
|
try {
|
|
await stopServer(running);
|
|
} catch (stopError) {
|
|
appendLog(
|
|
`Failed to stop Paperclip after supervisor failure: ${stopError instanceof Error ? stopError.message : String(stopError)}\n`,
|
|
);
|
|
}
|
|
}
|
|
}
|
|
|
|
await new Promise<void>((resolve) => log.end(resolve));
|
|
process.exitCode = exitCode;
|