mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-09 16:02:36 +01:00
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The native Runner path runs an agent turn through a runnerd process that the control plane owns > - A restart of the control plane must keep the task going without a click, if that is safe > - A graceful shutdown closes the HTTP listener first and then drains the heartbeat runs > - A native run that was between provider dispatch and runnerd backend setup in that gap failed with `local_runtime_api_listener_unavailable`, so the drain did not see it as running and did not suspend it for the next boot > - This pull request makes that failure a restart detach: the run stays running with its claim, the drain suspends it, and the next controller adopts it > - The benefit is that a SIGTERM in this window no longer spends a failure and a same-run retry, and the task continues by itself ## Linked Issues or Issue Description Refs #19530 (the same restart window, before provider dispatch). **What happened?** In a restart chaos replay (ACPX Claude, local runner), a restart-recovered run got SIGTERM while `createRunnerdBackendWithinSessionClaim` built the runnerd backend. `resolveLocalRuntimeApiUrl()` threw `local_runtime_api_listener_unavailable` because shutdown had already closed the listener. The run settled `failed`. The drain then logged `restartSuspendedRunIds: []`. The next boot used the one bootstrap retry. A second SIGTERM hit the same window, and the task stopped at a manual Retry. **Expected behavior** A run that this process cannot continue because it is shutting down is handed to the next controller. It is not settled as failed. **Steps to reproduce** 1. Start a native Runner task and restart the server (K1) so that startup recovery resumes the run. 2. Send SIGTERM while the resumed run sets up its runnerd backend. 3. See the run fail with `local_runtime_api_listener_unavailable` and the drain report no suspended runs. ## What Changed - `server/src/runtime-api.ts`: `resolveLocalRuntimeApiUrl()` throws the new `LocalRuntimeApiListenerClosedError` when this process had a listener and closed it (the `listening` flag is false or the `close` event fired). The error message stays `local_runtime_api_listener_unavailable`, so the existing classifiers do not change. A listener that never started still throws the plain error. - `server/src/services/native-runtime/native-session-executor.ts`: for a local Runner, the startup failure handler treats `LocalRuntimeApiListenerClosedError` like a restart detach. It stops lease renewal, keeps the claim and throws `NativeControllerDetachedForRestartError`. Heartbeat already keeps such a run `running`. A remote Runner keeps the same-run bootstrap retry from #19462, because startup recovery has no process evidence to adopt a claimed remote startup (`native-runner-restart-preparation.integration.test.ts` still covers that path). - `doc/execution-semantics.md`: describe the new rule. - Tests: a heartbeat regression that closes the real runtime API listener before backend setup, then drains and restarts; a unit test for the closed versus never-started listener; two `vi.mock` factories keep the real module exports. ## Verification - Red before the fix: `pnpm exec vitest run src/__tests__/heartbeat-native-restart-suspension.test.ts -t listener` failed with `restartSuspendedRunIds: []` and the run `failed` (`local_runtime_api_listener_unavailable`, `bootstrap_retry`). - Green after the fix: the same test passes. The drain suspends the run, the run stays `running` with `awaiting_runner_reattach`, and the next controller claims it and reaches the backend once. - `pnpm exec vitest run src/__tests__/heartbeat-native-restart-suspension.test.ts src/http/server.test.ts src/__tests__/server-startup-feedback-export.test.ts src/services/native-runtime/native-session-executor.test.ts src/services/native-runtime/native-recovered-owner-followup.test.ts`: 1333 tests pass. One timing test in `native-session-executor.test.ts` failed once under host load average ~30 and passed alone. - `pnpm exec tsc --noEmit -p server/tsconfig.json`: pass. - Live, restart chaos harness, local ACPX Claude (`claude-sonnet-5`), `--rc06-masked`, this branch ata35ec8b845(before the remote-scope commit5dc63f24fb, which does not change the local path). The original replay schedule no longer reached the window, because the task now ends before the second fault. So I moved the faults earlier: K1 at 10 s, then K2 as soon as the recovered server is ready. - K1 + K2, 2 runs: both runs PASS with **R1 pass**. Each drain logged `restartSuspendedRunIds: [<run>]` (before: `[]`). The next boot classified the run `resume_dead_runner` and the same run completed. No `local_runtime_api_listener_unavailable` failure. - K1 + K2 + K2, 2 runs: each drain suspended the run. The fourth boot blocked the run with `native_restart_recovery_blocked`, because three restart claims used the three-attempt recovery budget. Retry then completed the task (R3 pass). That budget count is a separate open item. This pull request does not change it. ## Risks - Low. The new path applies only after this process closed its own listener, which happens only during shutdown. The run then waits for the drain or the next boot, as for the other restart detaches. - If shutdown does not reach the drain (for example SIGKILL after the listener closes), the run keeps its live claim until the claim expires. Then startup recovery handles it as a hard restart. This is the same as any run that is running at a hard kill. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, through Claude Code with tool use (shell, file edits, GitHub CLI). Extended reasoning on. ## 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 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
84 lines
4.7 KiB
TypeScript
84 lines
4.7 KiB
TypeScript
import { spawnSync } from "node:child_process";
|
|
import { mkdtemp, readFile, rm, access } from "node:fs/promises";
|
|
import os from "node:os";
|
|
import path from "node:path";
|
|
import { describe, expect, it } from "vitest";
|
|
|
|
const queuedSha = "a".repeat(40);
|
|
const advancedSha = "b".repeat(40);
|
|
|
|
async function resolveTarget(event: string, cadence: string, branch = "master") {
|
|
const workflow = await readFile(new URL("../../.github/workflows/runner-full-stack-e2e.yml", import.meta.url), "utf8");
|
|
const step = workflow.split(" - name: Resolve requested repository branch to an immutable commit\n")[1]!.split("\n - name:")[0]!;
|
|
const script = step.split(" run: |\n")[1]!.replace(/^ {10}/gm, "");
|
|
const directory = await mkdtemp(path.join(os.tmpdir(), "eval-workflow-target-"));
|
|
try {
|
|
const output = path.join(directory, "output");
|
|
const result = spawnSync("bash", ["-c", `gh() { printf '%s\\n' "$MOCK_BRANCH_SHA"; }\n${script}`], {
|
|
encoding: "utf8", env: { ...process.env, EVENT_NAME: event, EVAL_CADENCE: cadence,
|
|
IMAGE_ONLY: "false", TESTOPS_REQUESTED: "false",
|
|
TARGET_BRANCH: branch, DEFAULT_BRANCH: "master", GITHUB_SHA: queuedSha,
|
|
MOCK_BRANCH_SHA: advancedSha, GITHUB_OUTPUT: output, REPOSITORY: "paperclipai/paperclip",
|
|
IMAGE_ONLY: "false", TESTOPS_REQUESTED: "false" },
|
|
});
|
|
return { status: result.status, stderr: result.stderr,
|
|
output: await readFile(output, "utf8").catch(() => "") };
|
|
} finally { await rm(directory, { recursive: true, force: true }); }
|
|
}
|
|
|
|
describe("paid workflow target authorization", () => {
|
|
it.each([["schedule", "manual"], ["workflow_dispatch", "daily"], ["workflow_dispatch", "weekly"]])(
|
|
"pins %s/%s to the queued revision when the branch advances", async (event, cadence) => {
|
|
const result = await resolveTarget(event, cadence);
|
|
expect(result.status, result.stderr).toBe(0);
|
|
expect(result.output).toContain(`sha=${queuedSha}\n`);
|
|
expect(result.output).not.toContain(advancedSha);
|
|
});
|
|
|
|
it("rejects custom bounded targets before emitting a commit", async () => {
|
|
const result = await resolveTarget("workflow_dispatch", "daily", "development");
|
|
expect(result.status).toBe(1);
|
|
expect(result.output).toBe("");
|
|
expect(result.stderr).toContain("default-branch commit");
|
|
});
|
|
|
|
it("still resolves explicit manual targets at authorization time", async () => {
|
|
const result = await resolveTarget("workflow_dispatch", "manual", "development");
|
|
expect(result.status, result.stderr).toBe(0);
|
|
expect(result.output).toContain(`sha=${advancedSha}\n`);
|
|
expect(result.output).toContain("ref=refs/heads/development\n");
|
|
});
|
|
});
|
|
|
|
describe("qualification image dependency correction", () => {
|
|
it.each([true, false])("applies the workflow correction only to its own source: %s", async sameSource => {
|
|
const workflow = await readFile(new URL("../../.github/workflows/docker-runner-check.yml", import.meta.url), "utf8");
|
|
const step = workflow.split(" - name: Apply reviewed qualification lock corrections\n")[1]!.split("\n - ")[0]!;
|
|
const script = step.split(" run: |\n")[1]!.replace(/^ {10}/gm, "");
|
|
const directory = await mkdtemp(path.join(os.tmpdir(), "eval-image-lock-target-"));
|
|
try {
|
|
const marker = path.join(directory, "called");
|
|
const result = spawnSync("bash", ["-c", `node() { touch "$CALLED"; }\n${script}`], {
|
|
cwd: directory, encoding: "utf8", env: { ...process.env, GITHUB_SHA: queuedSha,
|
|
TARGET_SHA: sameSource ? queuedSha : advancedSha, CALLED: marker },
|
|
});
|
|
expect(result.status, result.stderr).toBe(0);
|
|
expect(await access(marker).then(() => true, () => false)).toBe(sameSource);
|
|
} finally { await rm(directory, { recursive: true, force: true }); }
|
|
});
|
|
});
|
|
|
|
describe("release image capacity prerequisite", () => {
|
|
it.each([34, 35])("checks the unchanged disk threshold for every image route: %s GiB", async available => {
|
|
const workflow = await readFile(new URL("../../.github/workflows/docker-runner-check.yml", import.meta.url), "utf8");
|
|
const step = workflow.split(" - name: Prepare hosted image build capacity\n")[1]!.split("\n - ")[0]!;
|
|
expect(step).toContain("if: inputs.build_eval_image || inputs.publish_eval_image\n");
|
|
expect(step).not.toContain("release_run_id");
|
|
const script = step.split(" run: |\n")[1]!.replace(/^ {10}/gm, "");
|
|
const result = spawnSync("bash", ["-c", `sudo() { test "$*" = "rm -rf /usr/share/dotnet /usr/local/lib/android /opt/ghc"; }; df() { printf 'Avail\\n%sG\\n' "$AVAILABLE"; }; ${script}`], {
|
|
encoding: "utf8", env: { PATH: process.env.PATH, AVAILABLE: String(available) },
|
|
});
|
|
expect(result.status, result.stderr).toBe(available >= 35 ? 0 : 1);
|
|
});
|
|
});
|