From 8c7e86e73b5e7916a138979bdac176cd004a7ada Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Fri, 6 Nov 2026 14:33:09 -0600 Subject: [PATCH 1/5] fix(workspaces): let non-git workspace directories pass close readiness (#19553) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Execution workspaces hold the files that agents work in, and a person can close a workspace when the work ends > - Before close, Paperclip checks "close readiness" so that it does not destroy unsaved git work > - The check assumed that every existing workspace path is a git repository > - A shared or `local_fs` workspace can be a plain directory. For that directory, `git rev-parse` fails, and the check reported "Paperclip could not verify the workspace git status" as a blocking reason > - So a person could never close these workspaces > - This pull request treats a path with no `.git` marker as a workspace with no git state > - The benefit is that plain-directory workspaces can close, and real git errors still block close ## Linked Issues or Issue Description **What happened?** `GET /api/execution-workspaces/:id/close-readiness` returned `state: "blocked"` for a `shared_workspace` with provider `local_fs`. The workspace directory existed but was not a git repository. The only blocking reason was "Paperclip could not verify the workspace git status. Retry before destructive cleanup." **Steps to reproduce** 1. Create a project workspace and an execution workspace (`shared_workspace`, `local_fs`) whose `cwd` is a plain directory that exists. 2. Request close readiness for the execution workspace. 3. See `state: "blocked"` and `isDestructiveCloseAllowed: false`. **Expected behavior** The workspace is ready to close. It has no git state to lose. ## What Changed - `inspectGitCloseReadiness` in `server/src/services/execution-workspaces.ts`: when `git rev-parse --show-toplevel` fails, the provider is not `git_worktree`, and no `.git` entry exists on the path or on an ancestor, return `git: null` with `statusInspectionSucceeded: true`. - Added `hasGitMarkerInPathOrAncestors`. The check uses the filesystem, not git error text, so it does not depend on the git locale. It uses `lstat`, so a dangling `.git` symlink still counts as a marker. Any error other than `ENOENT` counts as a possible marker, so the check fails closed. - For a plain directory, the result is a git readiness object with `repoRoot: null`, the same shape as the missing-path case. `createdByRuntime` stays on it, so close readiness still plans `remove_local_directory` for a runtime-created directory. - Added two regression tests in `server/src/__tests__/execution-workspaces-service.test.ts`: - A plain directory is `ready_with_warnings` with `git: null` and no blocking reasons. This test failed before the fix (`state: "blocked"`). - A directory with a broken `.git` pointer is still `blocked` with the git-status reason. - A directory with a dangling `.git` symlink is still `blocked`. This test failed before the second commit. - A runtime-created plain directory plans `remove_local_directory`. This test failed before the second commit. - A plain directory nested under an ancestor with an unreadable `.git` marker is still `blocked`. This test covers the ancestor walk. ## Verification - Red/green: the new non-git test failed on `master` (`state: "blocked"`) and passes with the fix. The broken-git test passes on both. - `cd server && npx vitest run src/__tests__/execution-workspaces-service.test.ts src/__tests__/execution-workspaces-routes.test.ts`: 2 files, 101 tests pass. The five close-readiness tests also pass after the third commit. - `pnpm verify` selected 2,223 of 2,991 test files because this service has many importers. I did not run that near-full suite locally. Required CI supplies that coverage. - `tsc -p server/tsconfig.json` shows no errors in the changed files. ## Risks - Low risk. The change only affects paths where git inspection already failed. - With `git: null` removed, the result uses the same `repoRoot: null` shape as a missing path. Existing code already handles that shape. - `git_worktree` workspaces still fail closed. A real repository that git cannot read (corrupt, missing git dir, ownership refusal) still has a `.git` marker, so it still blocks. - The terminal-workspace sweep also uses this inspection. With no repo root there is no HEAD SHA, so delivery cannot be "merged", and the sweep still skips these workspaces. Only an explicit close by a person proceeds. - A `.git` on an ancestor across a filesystem boundary makes git report "not a repository" while the marker check finds it. That case stays blocked, which is the safe result. ## Model Used - Anthropic Claude Opus 5.5 (`claude-opus-5-5`) in Claude Code, with tool use (shell, file edit, test runs). ## 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 --- .../execution-workspaces-service.test.ts | 123 ++++++++++++++++++ server/src/services/execution-workspaces.ts | 48 +++++++ 2 files changed, 171 insertions(+) diff --git a/server/src/__tests__/execution-workspaces-service.test.ts b/server/src/__tests__/execution-workspaces-service.test.ts index cb1dde02123..9e114d6ee68 100644 --- a/server/src/__tests__/execution-workspaces-service.test.ts +++ b/server/src/__tests__/execution-workspaces-service.test.ts @@ -2419,6 +2419,129 @@ describeEmbeddedPostgres("executionWorkspaceService.getCloseReadiness", () => { ])); }); + async function seedSharedLocalWorkspace(cwd: string, options: { metadata?: Record } = {}) { + const companyId = randomUUID(); + const projectId = randomUUID(); + const projectWorkspaceId = randomUUID(); + const executionWorkspaceId = randomUUID(); + await db.insert(companies).values({ + id: companyId, + name: "Paperclip", + issuePrefix: "PAP", + requireBoardApprovalForNewAgents: false, + }); + await db.insert(projects).values({ + id: projectId, + companyId, + name: "Workspaces", + status: "in_progress", + executionWorkspacePolicy: { enabled: true }, + }); + await db.insert(projectWorkspaces).values({ + id: projectWorkspaceId, + companyId, + projectId, + name: "Primary", + sourceType: "local_path", + isPrimary: true, + cwd, + }); + await db.insert(executionWorkspaces).values({ + id: executionWorkspaceId, + companyId, + projectId, + projectWorkspaceId, + mode: "shared_workspace", + strategyType: "project_primary", + name: "Shared workspace", + status: "active", + providerType: "local_fs", + cwd, + metadata: options.metadata ?? null, + }); + return { executionWorkspaceId }; + } + + it("treats an existing workspace directory that is not a git repository as ready", async () => { + const workspaceDir = await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-non-git-workspace-")); + tempDirs.add(workspaceDir); + await fs.writeFile(path.join(workspaceDir, "notes.txt"), "plain files\n", "utf8"); + const { executionWorkspaceId } = await seedSharedLocalWorkspace(workspaceDir); + + const readiness = await svc.getCloseReadiness(executionWorkspaceId); + + expect(readiness).toMatchObject({ + state: "ready_with_warnings", + isDestructiveCloseAllowed: true, + git: { repoRoot: null, hasDirtyTrackedFiles: false, hasUntrackedFiles: false }, + }); + expect(readiness?.blockingReasons).toEqual([]); + expect(readiness?.warnings.some((warning) => warning.startsWith("Could not inspect git status"))).toBe(false); + }); + + it("plans removal of a runtime-created plain workspace directory", async () => { + const parentDir = await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-non-git-runtime-")); + tempDirs.add(parentDir); + const workspaceDir = path.join(parentDir, "task"); + await fs.mkdir(workspaceDir); + const { executionWorkspaceId } = await seedSharedLocalWorkspace(workspaceDir, { + metadata: { createdByRuntime: true }, + }); + await db.update(projectWorkspaces).set({ cwd: path.join(parentDir, "primary") }) + .where(eq(projectWorkspaces.cwd, workspaceDir)); + + const readiness = await svc.getCloseReadiness(executionWorkspaceId); + + expect(readiness?.blockingReasons).toEqual([]); + expect(readiness?.git).toMatchObject({ repoRoot: null, createdByRuntime: true }); + expect(readiness?.plannedActions.map((action) => action.kind)).toContain("remove_local_directory"); + }); + + it("still blocks close when an ancestor directory holds an unreadable .git marker", async () => { + const parentDir = await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-ancestor-git-workspace-")); + tempDirs.add(parentDir); + await fs.writeFile(path.join(parentDir, ".git"), `gitdir: ${path.join(parentDir, "missing-git-dir")}\n`, "utf8"); + const workspaceDir = path.join(parentDir, "nested", "task"); + await fs.mkdir(workspaceDir, { recursive: true }); + const { executionWorkspaceId } = await seedSharedLocalWorkspace(workspaceDir); + + const readiness = await svc.getCloseReadiness(executionWorkspaceId); + + expect(readiness).toMatchObject({ state: "blocked", isDestructiveCloseAllowed: false }); + expect(readiness?.blockingReasons).toContain( + "Paperclip could not verify the workspace git status. Retry before destructive cleanup.", + ); + }); + + it("still blocks close when the workspace .git marker is a dangling symlink", async () => { + const workspaceDir = await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-dangling-git-workspace-")); + tempDirs.add(workspaceDir); + await fs.symlink(path.join(workspaceDir, "unmounted-repo", ".git"), path.join(workspaceDir, ".git")); + const { executionWorkspaceId } = await seedSharedLocalWorkspace(workspaceDir); + + const readiness = await svc.getCloseReadiness(executionWorkspaceId); + + expect(readiness).toMatchObject({ state: "blocked", isDestructiveCloseAllowed: false }); + expect(readiness?.blockingReasons).toContain( + "Paperclip could not verify the workspace git status. Retry before destructive cleanup.", + ); + }); + + it("still blocks close when a workspace git repository cannot be inspected", async () => { + const workspaceDir = await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-broken-git-workspace-")); + tempDirs.add(workspaceDir); + // A worktree-style .git pointer whose git dir is missing: git refuses to read it. + await fs.writeFile(path.join(workspaceDir, ".git"), `gitdir: ${path.join(workspaceDir, "missing-git-dir")}\n`, "utf8"); + const { executionWorkspaceId } = await seedSharedLocalWorkspace(workspaceDir); + + const readiness = await svc.getCloseReadiness(executionWorkspaceId); + + expect(readiness).toMatchObject({ state: "blocked", isDestructiveCloseAllowed: false }); + expect(readiness?.blockingReasons).toContain( + "Paperclip could not verify the workspace git status. Retry before destructive cleanup.", + ); + }); + it("clears matching environment selections transactionally without touching other workspaces", async () => { const companyId = randomUUID(); const projectId = randomUUID(); diff --git a/server/src/services/execution-workspaces.ts b/server/src/services/execution-workspaces.ts index 4a4601844bf..3b29a35aa28 100644 --- a/server/src/services/execution-workspaces.ts +++ b/server/src/services/execution-workspaces.ts @@ -521,6 +521,31 @@ async function pathExists(value: string | null | undefined) { } } +/** + * Returns false only when no `.git` entry exists on the path or any ancestor. + * Uses lstat so a dangling `.git` symlink still counts, and treats any error + * other than ENOENT as "may exist" so unreadable locations fail closed. + */ +async function hasGitMarkerInPathOrAncestors(start: string) { + let current: string; + try { + current = await fs.realpath(start); + } catch { + return true; + } + while (true) { + try { + await fs.lstat(path.join(current, ".git")); + return true; + } catch (error) { + if ((error as NodeJS.ErrnoException | null)?.code !== "ENOENT") return true; + } + const parent = path.dirname(current); + if (parent === current) return false; + current = parent; + } +} + async function runGit(args: string[], cwd: string) { return await execFileAsync("git", ["-C", cwd, ...args], { cwd }); } @@ -963,6 +988,29 @@ async function inspectGitCloseReadiness(workspace: ExecutionWorkspace): Promise< try { repoRoot = (await runGit(["rev-parse", "--show-toplevel"], workspacePath)).stdout.trim() || null; } catch (error) { + // A plain directory (e.g. a local_fs shared workspace) has no git state to + // lose. Only treat it that way when no `.git` exists on the path, so a + // broken or unreadable repository still fails closed. + if (workspace.providerType !== "git_worktree" && !(await hasGitMarkerInPathOrAncestors(workspacePath))) { + return { + git: { + repoRoot: null, + workspacePath, + branchName: workspace.branchName, + baseRef: workspace.baseRef, + hasDirtyTrackedFiles: false, + hasUntrackedFiles: false, + dirtyEntryCount: 0, + untrackedEntryCount: 0, + aheadCount: null, + behindCount: null, + isMergedIntoBase: null, + createdByRuntime, + }, + warnings, + statusInspectionSucceeded: true, + }; + } warnings.push( `Could not inspect git status for "${workspacePath}": ${error instanceof Error ? error.message : String(error)}`, ); From 02214b1a3c914ce25fd8c52d7b354cc74bd117eb Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Fri, 6 Nov 2026 14:36:26 -0600 Subject: [PATCH 2/5] fix(evals): treat a job-less TestOps plan row as unassigned during registration (#19601) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Runner E2E cases run from Allure TestOps plans through `runner-full-stack-e2e.yml` > - #19549 added a sibling "Reliability — staging soak" plan and a one-shot registration workflow > - The first live `register` dispatch created the seven cases and the plan, then stopped with `plan_job_conflict` > - Live TestOps lists a new plan's uncovered cases as one row without a job, and the code read that row as another job > - This pull request treats a job-less row as "not assigned" > - The benefit is that a second `register` dispatch can assign the plan to the standing GitHub job and finish registration ## Linked Issues or Issue Description Refs #19549 **What happened?** The `runner-e2e-testops-registration.yml` `register` run on master failed with `reliability_testops_registration_plan_job_conflict`. It had already created the seven cases and the plan. `GET /api/testplan//job` for that plan returned `[{"testCasesCount":7}]`. The row has no `job` field. **Expected behavior** Registration treats a plan with only a job-less row as unassigned. It assigns the plan to the standing Product regression GitHub job and reads the assignment back. **Steps to reproduce** 1. Dispatch `runner-e2e-testops-registration.yml` on master with `operation=register` when no Reliability plan exists. 2. The new plan reports one job-less row. 3. Registration fails with `plan_job_conflict` before the assignment step. **Paperclip version or commit** master at 744056ecc8 and later. ## What Changed - `tests/runner-e2e/reliability-testops-registration.mjs`: add `jobAssignments()`. It keeps the array check and ignores rows without a `job`. Inspect, the pre-assignment conflict check and the post-assignment read-back use it. - `tests/runner-e2e/reliability-testops.test.mjs`: the fake TestOps now returns the live job-less row for a new plan. A new test reproduces the half-registered plan and checks that `register` assigns the standing job and that the receipt becomes complete. ## Verification - `node --test tests/runner-e2e/reliability-testops.test.mjs tests/runner-e2e/product-regression-testops.test.mjs`: 28/28 pass. - Red before the fix: with the old registration module, 4 tests fail, including the new one. Green after: 13/13 in the reliability file. - Live check after merge: dispatch `register` again. The cases and plan already exist, so it only assigns the plan to the standing job. ## Risks - Low risk. The change touches only the one-shot registration helper and its tests. It does not change provider cells, launch selection or reporting. - A plan assigned to another job still fails with `plan_job_conflict`. The existing test still covers that case. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`) in Claude Code with tool use (shell, browser read of the TestOps API response), standard context. ## 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 --- .../reliability-testops-registration.mjs | 19 +++++++++++++------ tests/runner-e2e/reliability-testops.test.mjs | 16 ++++++++++++++++ 2 files changed, 29 insertions(+), 6 deletions(-) diff --git a/tests/runner-e2e/reliability-testops-registration.mjs b/tests/runner-e2e/reliability-testops-registration.mjs index c9da1096cec..42d1c9706a2 100644 --- a/tests/runner-e2e/reliability-testops-registration.mjs +++ b/tests/runner-e2e/reliability-testops-registration.mjs @@ -94,8 +94,7 @@ export async function inspectReliability(api) { if (candidates.length !== 1 || !positive(candidates[0].job.id)) fail('standing_job'); const job = await api(`/api/job/${candidates[0].job.id}`); if (job?.id !== candidates[0].job.id || job.type !== 'github' || !workflowUrls.has(job.url)) fail('standing_job'); - const assignments = plan ? await api(`/api/testplan/${plan.id}/job`) : []; - if (!Array.isArray(assignments)) fail('plan_job'); + const assignments = plan ? jobAssignments(await api(`/api/testplan/${plan.id}/job`)) : []; return { caseIds: Object.fromEntries(found), plan, jobId: job.id, standingParameters: Array.isArray(candidates[0].job.parameters) ? candidates[0].job.parameters : [], assignments }; } @@ -134,8 +133,8 @@ export async function registerReliability(api) { plan = await api(`/api/testplan/${plan.id}`); } assertPlan(plan, ids); - let assignments = await api(`/api/testplan/${plan.id}/job`); - if (!Array.isArray(assignments) || assignments.length > 1 || assignments.some(row => row?.job?.id !== before.jobId)) fail('plan_job_conflict'); + let assignments = jobAssignments(await api(`/api/testplan/${plan.id}/job`)); + if (assignments.length > 1 || assignments.some(row => row?.job?.id !== before.jobId)) fail('plan_job_conflict'); if (!assignments.length) { await api(`/api/testplan/${plan.id}/assign`, 'POST', { jobId: before.jobId, selection: { inverted: false, manual: false, leafsInclude: ids } }); created.assignment = true; @@ -145,13 +144,21 @@ export async function registerReliability(api) { await api(`/api/testplan/${plan.id}/jobparameter`, 'POST', { jobId: before.jobId, parameters: before.standingParameters }); created.parameters = true; } - assignments = await api(`/api/testplan/${plan.id}/job`); - if (!Array.isArray(assignments) || assignments.length !== 1 || assignments[0]?.job?.id !== before.jobId + assignments = jobAssignments(await api(`/api/testplan/${plan.id}/job`)); + if (assignments.length !== 1 || assignments[0]?.job?.id !== before.jobId || assignments[0].testCasesCount !== ids.length || (before.standingParameters.length && !same(assignments[0].job.parameters ?? [], before.standingParameters))) fail('plan_job'); return { caseIds, plan, jobId: before.jobId, created }; } +// TestOps lists a plan's cases that no job covers as a job-less row, e.g. +// `[{ testCasesCount: 7 }]` for a freshly created plan. That row is not a job +// assignment; only rows naming a job are. +function jobAssignments(rows) { + if (!Array.isArray(rows)) fail('plan_job'); + return rows.filter(row => row?.job != null); +} + export function registrationReceipt(operation, state, env) { const caseIds = Object.fromEntries(reliabilityDefinitions.filter(row => state.caseIds[row.name]).map(row => [row.name, state.caseIds[row.name]])); const complete = Object.keys(caseIds).length === reliabilityDefinitions.length && positive(state.plan?.id) diff --git a/tests/runner-e2e/reliability-testops.test.mjs b/tests/runner-e2e/reliability-testops.test.mjs index c10727bd54c..4cda542b8c3 100644 --- a/tests/runner-e2e/reliability-testops.test.mjs +++ b/tests/runner-e2e/reliability-testops.test.mjs @@ -196,6 +196,8 @@ function fakeTestOps({ existing = [], plan = null, assignments = [], standing = if (method === 'GET' && url.pathname === '/api/testplan') return { content: state.plan ? [{ id: state.plan.id, name: state.plan.name }] : [], totalPages: 1 }; if (method === 'POST' && url.pathname === '/api/testplan') { state.plan = { id: 77, projectId: 1, name: body.name, treeSelection: body.treeSelection, testCasesCount: body.treeSelection.leafsInclude.length }; + // Live TestOps lists a new plan's uncovered cases as one job-less row. + state.assignments = [{ testCasesCount: body.treeSelection.leafsInclude.length }]; return { id: 77 }; } if (method === 'GET' && url.pathname === '/api/testplan/77') return state.plan; @@ -293,6 +295,20 @@ test('registration refuses a standing plan without exactly one trusted job or a await assert.rejects(registerReliability(api), /plan_job_conflict/); }); +test('a plan TestOps reports only as a job-less row is unassigned, so registration assigns the standing job', async () => { + const { api, state } = fakeTestOps(); + await registerReliability(api); + // Reproduces staging plan 135: created, then the run stopped before assignment. + state.assignments = [{ testCasesCount: 7 }]; + const inspected = await inspectReliability(api); + assert.deepEqual(inspected.assignments, []); + assert.equal(registrationReceipt('inspect', inspected, env).complete, false); + const repaired = await registerReliability(api); + assert.equal(repaired.created.assignment, true); + assert.deepEqual(state.assignments.map(row => row.job?.id), [50]); + assert.equal(registrationReceipt('register', repaired, { GITHUB_RUN_ID: '1', GITHUB_SHA: workflowSha }).complete, true); +}); + test('the workflows expose the reliability plan without new provider or reporting authority', async () => { const full = parse(await readFile(new URL('../../.github/workflows/runner-full-stack-e2e.yml', import.meta.url), 'utf8')); assert.deepEqual(full.on.workflow_dispatch.inputs.testops_plan.options, ['product-regression', 'reliability']); From dc1adb7209bf8077983fdde8c1e8ab18405403b7 Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Fri, 6 Nov 2026 14:38:29 -0600 Subject: [PATCH 3/5] fix(tasks): advertise a native resume only while the scheduler would claim it (#19523) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - When a native run cannot reattach to its provider after a control-plane restart, the native coordinator resumes the same run by itself after about 30 seconds, up to three attempts > - The run projection shows this correctly as "Retry scheduled" with only `inspect_run` permitted > - The task interruption shown on the issue read only system-recovery evidence and `scheduledRetryAt`. It never read the coordinator's `nextAttemptAt`, so it showed `state: failed, canRetry: true` and offered a manual Retry while the automatic resume was still pending > - The two surfaces disagree, and a user click can race the automatic resume. That is one way a Retry gets parked behind "execution checks" > - #19503 made the task interruption read the coordinator's pending resume time. Review found that the coordinator also pushes that time forward while it defers an ineligible candidate, so a reassigned or blocked task could hide Retry for good > - This pull request advertises the resume only while the scheduler's synchronous admission checks pass > - The benefit is one consistent story: while the automatic resume is pending, the task shows it with its time and no competing Retry. If recovery ends, exactly one Retry returns ## Linked Issues or Issue Description Follow-up to #19503. Refs #19487, #19497. **What happened?** A native run failed with `native_session_interrupted` after a restart. Its finalization row had `phase: retryable_failure` and `nextAttemptAt` 30 seconds later. The run projection said "Retry scheduled". The issue interruption said `state: failed, canRetry: true, nextRetryAt: null`. **What did you expect?** While the automatic resume is pending, the task shows that it is resuming, with the time, and no Retry. When the resume fails for good or never runs, exactly one Retry that works on the first click. **Steps to reproduce** 1. Run a native task and restart the control plane so the native attach fails. 2. Read `GET /api/issues/:id` while the coordinator's resume is pending. 3. Observe `interruption.canRetry: true` while `execution.phase` is `retry_scheduled`. ## What Changed - `task-interruption-presentation.ts`: the pending-resume read now also requires the task to be in a resumable status, assigned to the run's agent, not owned by another execution or checkout run, the run's question not cancelled, and the agent invokable (not paused, terminated or pending approval). These mirror `readNativeSessionResumptionAgent`. - `native-session-resumption-statuses.ts`: one shared status set used by both the scheduler admission and the read model. - No time cap: provider cooldowns and remote backoff schedules stay visible as a pending resume. ## Verification - `task-interruption-query.test.ts` (embedded Postgres): a reassigned task and a blocked task show one Retry; a paused agent asks to be resumed; in each case `readNativeSessionResumptionAgent` also refuses the candidate, so the read model and the scheduler agree. A resume an hour out (provider cooldown) stays `retry_scheduled` with no Retry. Red on master for the reassigned case. - All 57 presentation and query tests pass. Server typecheck passes. ## Risks - Low risk. Read model only. - A review-assignment resume (reviewer is not the assignee) shows Retry instead of the pending resume. That is the safe fallback. - Deferrals for a pause hold, dependency readiness or a competing run are not mirrored. Those gates also hold a manual Retry, so no competing action appears. - The `POST /issues/:id/retry` endpoint is unchanged. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, via Claude Code with tool use (shell, file editing, embedded Postgres tests). ## 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 --- .../__tests__/task-interruption-query.test.ts | 67 +++++++++++++++++++ .../native-finalization-reconciler.ts | 3 +- .../native-session-resumption-admission.ts | 3 +- .../native-session-resumption-statuses.ts | 4 ++ .../task-interruption-presentation.ts | 39 ++++++++++- 5 files changed, 113 insertions(+), 3 deletions(-) create mode 100644 server/src/services/native-runtime/native-session-resumption-statuses.ts diff --git a/server/src/__tests__/task-interruption-query.test.ts b/server/src/__tests__/task-interruption-query.test.ts index df854e62152..f0352841235 100644 --- a/server/src/__tests__/task-interruption-query.test.ts +++ b/server/src/__tests__/task-interruption-query.test.ts @@ -4,6 +4,7 @@ import { createRunSecretRedactionRegistry } from "../services/run-secret-redacti import { afterAll, beforeAll, describe, expect, it, vi } from "vitest"; import { agents, agentWakeupRequests, companies, createDb, applyPendingMigrations, heartbeatRuns, issues, nativeRunFinalizations, EMBEDDED_POSTGRES_TEST_TIMEOUT_MS } from "@paperclipai/db"; import { taskInterruptionPresentation, taskInterruptionPresentations } from "../services/task-interruption-presentation.js"; +import { readNativeSessionResumptionAgent } from "../services/native-runtime/native-session-resumption-admission.js"; import { getEmbeddedPostgresTestSupport, startEmbeddedPostgresTestDatabase } from "./helpers/embedded-postgres.js"; import { resolveExternalTestDatabaseUrl } from "./helpers/external-test-database.js"; import { captureInterruptionQuery, seedInterruptionFixture } from "./helpers/task-interruption-fixture.js"; @@ -39,6 +40,49 @@ describe.skipIf(!support.supported)("task interruption latest run query", () => await taskInterruptionPresentation(db, issue)]) { expect(presentation).toMatchObject({ state: "retry_scheduled", canRetry: false, nextRetryAt: resumeAt.toISOString() }); } + // The coordinator also pushes nextAttemptAt while it defers an ineligible + // candidate. Those deferrals, or one far past the failure, never hide Retry. + const otherAgentId = randomUUID(); + await db.insert(agents).values({ id: otherAgentId, companyId, name: "Other agent", adapterType: "paperclip_runner" }); + const admission = async () => { + const [currentIssue] = await db.select().from(issues).where(eq(issues.id, issue.id)); + const [currentRun] = await db.select().from(heartbeatRuns).where(eq(heartbeatRuns.id, runId)); + const [coordinator] = await db.select().from(nativeRunFinalizations).where(eq(nativeRunFinalizations.runId, runId)); + return readNativeSessionResumptionAgent(db, { issue: currentIssue!, run: currentRun!, coordinator: coordinator! }); + }; + expect(await admission()).not.toBeNull(); + const otherRunId = randomUUID(); + await db.insert(heartbeatRuns).values({ id: otherRunId, companyId, agentId: otherAgentId, status: "cancelled", + finishedAt: new Date(Date.now() - 60_000), createdAt: new Date(Date.now() - 60 * 60_000) }); + const terminatedManagerId = randomUUID(); + await db.insert(agents).values({ id: terminatedManagerId, companyId, name: "Former manager", + adapterType: "paperclip_runner", status: "terminated" }); + for (const [table, change, restore] of [ + ["issue", { assigneeAgentId: otherAgentId }, { assigneeAgentId: agentId }], + ["issue", { status: "blocked" }, { status: "in_progress" }], + ["issue", { executionRunId: otherRunId }, { executionRunId: null }], + ["issue", { checkoutRunId: otherRunId }, { checkoutRunId: null }], + ["run", { contextSnapshot: { nativeQuestionCancellation: { requestId: "q" } } }, { contextSnapshot: {} }], + // Invokability is more than status: an idle agent under a terminated manager is refused. + ["agent", { reportsTo: terminatedManagerId }, { reportsTo: null }], + ] as const) { + if (table === "issue") await db.update(issues).set(change).where(eq(issues.id, issue.id)); + else if (table === "run") await db.update(heartbeatRuns).set(change).where(eq(heartbeatRuns.id, runId)); + else await db.update(agents).set(change).where(eq(agents.id, agentId)); + // The scheduler would defer this candidate, so the task offers Retry. + if (table !== "run") expect(await admission()).toBeNull(); + expect((await taskInterruptionPresentations(db, companyId, [issue])).get(issue.id)) + .toMatchObject({ state: "failed", canRetry: true }); + if (table === "issue") await db.update(issues).set(restore).where(eq(issues.id, issue.id)); + else if (table === "run") await db.update(heartbeatRuns).set(restore).where(eq(heartbeatRuns.id, runId)); + else await db.update(agents).set(restore).where(eq(agents.id, agentId)); + } + expect(await admission()).not.toBeNull(); + // A provider cooldown can schedule the resume an hour out; it stays visible. + const cooldown = new Date(Date.now() + 60 * 60_000); + await db.update(nativeRunFinalizations).set({ nextAttemptAt: cooldown }).where(eq(nativeRunFinalizations.runId, runId)); + expect((await taskInterruptionPresentations(db, companyId, [issue])).get(issue.id)) + .toMatchObject({ state: "retry_scheduled", canRetry: false, nextRetryAt: cooldown.toISOString() }); // Exhausted recovery is terminal: exactly one Retry returns. await db.update(nativeRunFinalizations).set({ phase: "terminal_failure", nextAttemptAt: null }) .where(eq(nativeRunFinalizations.runId, runId)); @@ -46,6 +90,29 @@ describe.skipIf(!support.supported)("task interruption latest run query", () => .toMatchObject({ state: "failed", canRetry: true }); }); + it("decides several pending native resumes in one list read", async () => { + const companyId = randomUUID(), agentId = randomUUID(), otherAgentId = randomUUID(); + await db.insert(companies).values({ id: companyId, name: "Resuming list", issuePrefix: `L${companyId.slice(0, 7)}` }); + await db.insert(agents).values([{ id: agentId, companyId, name: "Resuming agent", adapterType: "paperclip_runner" }, + { id: otherAgentId, companyId, name: "Other agent", adapterType: "paperclip_runner" }]); + const resumeAt = new Date(Date.now() + 30_000); + const list = []; + for (let index = 0; index < 6; index++) { + const [issue] = await db.insert(issues).values({ companyId, title: `Resume ${index}`, status: "in_progress", + assigneeAgentId: index % 2 ? otherAgentId : agentId }).returning(); + const runId = randomUUID(); + await db.insert(heartbeatRuns).values({ id: runId, companyId, agentId, nativeIssueId: issue.id, scopeKind: "issue", + status: "failed", runtimeMode: "native", errorCode: "native_session_interrupted", finishedAt: new Date() }); + await db.insert(nativeRunFinalizations).values({ companyId, issueId: issue.id, runId, phase: "retryable_failure", + attempt: 1, nextAttemptAt: resumeAt }); + list.push(issue); + } + const result = await taskInterruptionPresentations(db, companyId, list); + list.forEach((issue, index) => expect(result.get(issue.id)).toMatchObject(index % 2 + ? { state: "failed", canRetry: true } + : { state: "retry_scheduled", canRetry: false, nextRetryAt: resumeAt.toISOString() })); + }); + it("redacts workspace preparation errors on task detail and list reads", async () => { vi.stubEnv("PAPERCLIP_SECRETS_MASTER_KEY", "a".repeat(64)); try { diff --git a/server/src/services/native-runtime/native-finalization-reconciler.ts b/server/src/services/native-runtime/native-finalization-reconciler.ts index ba43627916a..5d97112d267 100644 --- a/server/src/services/native-runtime/native-finalization-reconciler.ts +++ b/server/src/services/native-runtime/native-finalization-reconciler.ts @@ -1,4 +1,5 @@ import { notifyLiveRunRecovery } from "../live-run-recovery-notifications.js"; +import { NATIVE_SESSION_RESUMPTION_ISSUE_STATUSES } from "./native-session-resumption-statuses.js"; import { notifyExternalAgentSettlement } from "../../modules/external-agents/index.js"; import { pendingNativeRunProjectionCondition } from "./native-run-projection.js"; import { nativeStatusReassessmentSourceIsSuperseded, reassessNativeStatusFromSource } from "./native-status-reassessment.js"; @@ -158,7 +159,7 @@ export async function claimNativeSessionResumptions(input: { if (await retireSupersededNativeRetry(tx as unknown as Db, { run, coordinator, issueExecutionRunId: issue.executionRunId, now, })) return false; - if (!["todo", "in_progress", "in_review"].includes(issue.status)) return false; + if (!(NATIVE_SESSION_RESUMPTION_ISSUE_STATUSES as readonly string[]).includes(issue.status)) return false; const row = { run, coordinator }; if ( row.run.runtimeMode !== "native" || diff --git a/server/src/services/native-runtime/native-session-resumption-admission.ts b/server/src/services/native-runtime/native-session-resumption-admission.ts index 6d27fff6f82..e768328c05a 100644 --- a/server/src/services/native-runtime/native-session-resumption-admission.ts +++ b/server/src/services/native-runtime/native-session-resumption-admission.ts @@ -8,6 +8,7 @@ import { getNativeReviewAssignment } from "./native-review-participant.js"; import { appendHeartbeatRunEvent } from "../heartbeat-run-events.js"; import { notifyStatusCardIssueChange } from "../status-card-notifications.js"; import { normalizeAgentNameKey } from "../agent-name-key.js"; +import { NATIVE_SESSION_RESUMPTION_ISSUE_STATUSES } from "./native-session-resumption-statuses.js"; export async function restoreNativeSessionExecution(db: Db, input: { issue: Pick; @@ -48,7 +49,7 @@ export async function readNativeSessionResumptionAgent(db: Db, input: { const { issue, run, coordinator } = input; if (issue.companyId !== run.companyId || issue.id !== run.nativeIssueId || coordinator.companyId !== run.companyId || coordinator.issueId !== issue.id || coordinator.runId !== run.id || - !["todo", "in_progress", "in_review"].includes(issue.status) || + !(NATIVE_SESSION_RESUMPTION_ISSUE_STATUSES as readonly string[]).includes(issue.status) || (issue.executionRunId && issue.executionRunId !== run.id) || (issue.checkoutRunId && issue.checkoutRunId !== run.id)) return null; const [agent] = await db.select().from(agents).where(and( diff --git a/server/src/services/native-runtime/native-session-resumption-statuses.ts b/server/src/services/native-runtime/native-session-resumption-statuses.ts new file mode 100644 index 00000000000..9c19a2a9dc6 --- /dev/null +++ b/server/src/services/native-runtime/native-session-resumption-statuses.ts @@ -0,0 +1,4 @@ +/** Task statuses from which a native same-run resume may be claimed. The task + * interruption read model shares this set with resumption admission so it + * never advertises a resume the scheduler would refuse. */ +export const NATIVE_SESSION_RESUMPTION_ISSUE_STATUSES = ["todo", "in_progress", "in_review"] as const; diff --git a/server/src/services/task-interruption-presentation.ts b/server/src/services/task-interruption-presentation.ts index c24db5150b4..73c7682f9be 100644 --- a/server/src/services/task-interruption-presentation.ts +++ b/server/src/services/task-interruption-presentation.ts @@ -216,10 +216,15 @@ export async function taskInterruptionPresentations(db: Db, companyId: string, i or ${heartbeatRuns.errorCode} = 'workspace_validation_failed' then ${heartbeatRuns.error} else null end`, errorCode: heartbeatRuns.errorCode, livenessState: heartbeatRuns.livenessState, scheduledRetryAt: heartbeatRuns.scheduledRetryAt, scheduledRetryReason: heartbeatRuns.scheduledRetryReason, + // Candidate only: the coordinator also pushes nextAttemptAt forward while + // it defers an ineligible run. The scheduler's own admission check below + // decides whether this resume is real. nativeResumeAt: sql`(select ${nativeRunFinalizations.nextAttemptAt} from ${nativeRunFinalizations} where ${nativeRunFinalizations.companyId} = ${heartbeatRuns.companyId} and ${nativeRunFinalizations.runId} = ${heartbeatRuns.id} and ${heartbeatRuns.runtimeMode} = 'native' and ${heartbeatRuns.status} = 'failed' - and ${nativeRunFinalizations.resultId} is null and ${nativeRunFinalizations.phase} = 'retryable_failure')`.mapWith(value => value == null ? null : new Date(value)), + and ${nativeRunFinalizations.resultId} is null and ${nativeRunFinalizations.phase} = 'retryable_failure' + and ${heartbeatRuns.contextSnapshot}->'nativeQuestionCancellation' is null + and ${heartbeatRuns.runnerProfileJson}->'legacyWorkspaceRecoveryMigration' is null)`.mapWith(value => value == null ? null : new Date(value)), // The run whose resultJson supplied providerFailure. A scheduled retry // shows its predecessor's reason, so redaction must use that run's registry. providerFailureRunId: sql`case when ${heartbeatRuns.resultJson}->'providerFailure' is null @@ -279,6 +284,38 @@ export async function taskInterruptionPresentations(db: Db, companyId: string, i inArray(approvals.status, ["pending", "revision_requested"]))), db.select({ status: companies.status, pauseReason: companies.pauseReason }).from(companies).where(eq(companies.id, companyId)), ]); + // Advertise a pending resume only when the scheduler would admit it, using + // its own check, so the task never hides Retry behind a deferred candidate + // and never offers Retry beside a resume that will run. Rare rows only. + // Candidates are failed native runs inside an active coordinator retry, so + // there are few. Their rows load in three batched reads; admission itself + // runs a few bounded reads per candidate, four at a time. + const resumeCandidates = runs.filter(run => run.nativeResumeAt); + if (resumeCandidates.length) { + const { readNativeSessionResumptionAgent } = await import("./native-runtime/native-session-resumption-admission.js"); + const candidateIds = resumeCandidates.map(run => run.id); + const [fullRuns, coordinators] = await Promise.all([ + db.select().from(heartbeatRuns).where(and(eq(heartbeatRuns.companyId, companyId), inArray(heartbeatRuns.id, candidateIds))), + db.select().from(nativeRunFinalizations).where(and(eq(nativeRunFinalizations.companyId, companyId), + inArray(nativeRunFinalizations.runId, candidateIds))), + ]); + const issueIds = [...new Set(fullRuns.flatMap(run => run.nativeIssueId ? [run.nativeIssueId] : []))]; + const issueRows = issueIds.length ? await db.select().from(issues).where(and(eq(issues.companyId, companyId), + inArray(issues.id, issueIds))) : []; + const runById = new Map(fullRuns.map(run => [run.id, run])); + const coordinatorByRun = new Map(coordinators.map(row => [row.runId, row])); + const issueById = new Map(issueRows.map(row => [row.id, row])); + for (let start = 0; start < resumeCandidates.length; start += 4) { + await Promise.all(resumeCandidates.slice(start, start + 4).map(async candidate => { + const run = runById.get(candidate.id); + const issue = run?.nativeIssueId ? issueById.get(run.nativeIssueId) : undefined; + const coordinator = coordinatorByRun.get(candidate.id); + const admitted = run && issue && coordinator + && await readNativeSessionResumptionAgent(db, { issue, run, coordinator }).catch(() => null); + if (!admitted) candidate.nativeResumeAt = null; + })); + } + } const runIds = runs.map(run => run.id); const actions = runIds.length ? await db.selectDistinctOn([issueRecoveryActions.sourceIssueId]).from(issueRecoveryActions).where(and( eq(issueRecoveryActions.companyId, companyId), inArray(issueRecoveryActions.sourceIssueId, ids), From d728be8fbffc95a903ffd58bc527c7e8f1a1c704 Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Fri, 6 Nov 2026 14:43:22 -0600 Subject: [PATCH 4/5] fix(native): re-bootstrap a Daytona runner restarted before session.open (#19583) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Native runs on Daytona start a `paperclip-runnerd` process in a sandbox. The local control plane keeps the runner's controller state in an encrypted Postgres checkpoint. > - When the control plane restarts, restart recovery classifies each live native run and decides to reattach, resume, bootstrap again, or hold the task. > - A restart chaos wave killed the control plane twice while a Daytona runner was connecting. The run was claimed and the sandbox runner was launched, but `session.open` never completed. No provider evidence existed. > - Recovery waited correctly for the old 60 s controller lease to expire. Then the classifier had no rule for a claimed remote bootstrap. It returned `ambiguous_remote_recovery_evidence` and blocked the task as "Workspace repair required". > - The controller writes each command to the Postgres checkpoint before it sends it. After the lease expires, that checkpoint is the complete record of what reached the sandbox runner. > - This pull request lets recovery prove from that checkpoint that no provider work exists, and then start the same-run bootstrap again. > - The benefit is that a restart during Daytona runner startup continues automatically instead of stopping the task. ## Linked Issues or Issue Description **What happened?** A restart during Daytona remote runner startup (native ACPX Claude, two SIGKILLs about 20 s apart) left the task blocked. The run history showed `remote_controller_lease_live` twice, then `ambiguous_remote_recovery_evidence` with disposition `blocked`. The task showed "Workspace repair required" (`native_restart_recovery_blocked`) and did not continue. **Expected behavior** When the durable controller record proves that the runner received only bootstrap commands, recovery continues the run with a fresh bootstrap. Any evidence of provider work keeps the hold. **Steps to reproduce** 1. Start a native run on a Daytona environment. 2. Kill the control plane after the sandbox runner connects and before `session.open` completes. 3. Restart the control plane and wait for the old controller lease to expire. 4. See that restart recovery blocks the task with `ambiguous_remote_recovery_evidence`. Root cause: `server/src/services/native-runtime/native-restart-recovery.ts` `classifyNativeRunnerRecoveryEvidence`, remote branch. It admitted only an unclaimed bootstrap (`remote_execution_never_claimed`, #16349) or a run with a provider checkpoint. A claimed bootstrap with no provider evidence fell through to `ambiguous_remote_recovery_evidence`. The decrypted checkpoint of the failed run held `connectionCount: 1`, `run.prepare` delivered once and pending, `session.open` pending and never delivered, no committed events, and `ackedSourceSeq: 0`. Refs #16349, #19524 ## What Changed - `native-bootstrap-proof.ts`: move the incomplete-bootstrap predicates from the executor into a small pure module. Restart recovery and the executor now use the same proof. No behavior change. - `native-runner-checkpoint.ts`: add `nativeRunnerCheckpointProvesIncompleteBootstrap`. It reads only a checkpoint with the bootstrap parts (header, cursor, `command:1`, `command:2`), checks the run, runner instance, and session identity, and applies the shared proof. A remote runner journal is not on this host, so the proof uses the controller record alone. - `native-restart-recovery.ts`: for a classic sandbox run whose owner lease has expired, with a durable checkpoint, no session checkpoint, no provider evidence, and no staged attachment, claim `bootstrap_incomplete` with the reason `remote_bootstrap_proven_incomplete` when the checkpoint proof holds. At three provider attempts, block with `execution_recovery_budget_exhausted`. - `native-session-executor.ts`: add `restoreRemoteBootstrapCheckpointForRecovery`. For a `bootstrap_incomplete` recovery on a sandbox target, it restores the Postgres checkpoint over the local external-state marker. The existing incomplete-bootstrap proof then checks the restored state before it quarantines the root and bootstraps again. - `doc/run-log-events.md`: document the new disposition reason. ## Verification - Red, then green: `cd server && npx vitest run src/services/native-runtime/native-runner-restart-recovery.integration.test.ts -t "remote bootstrap"`. Before the fix, the proven case and the budget case returned `blocked` / `ambiguous_remote_recovery_evidence`. - New cases: the proven claimed bootstrap gets `bootstrap_incomplete` (attempt kept, generation +1, run still `running`, no recovery action). The hold stays for a queued `turn.start`, an acknowledged event, adopted state, no checkpoint, and a spent budget. The executor helper restores the checkpoint only for a bootstrap recovery on a classic sandbox and only over a marker for the same run. - Regression set: `native-runner-restart-recovery.integration`, `native-session-executor`, `native-restart-recovery`, `native-bootstrap-retry.integration`, `heartbeat/recovery`: 1383 passed, 14 skipped. - Live Daytona replay of the failing chaos task (double SIGKILL during remote runner startup), 3 runs: REPLAY_RESULTS ## Risks - A fresh bootstrap acquires a new sandbox. The old sandbox keeps its lease until run settlement releases all run leases. The old runner gets no turn command, because only the controller issues one and the new controller never connects to it. - If `session.open` reached the old runner, the old provider session can exist without a turn. No provider work or tool call can happen in it. - Any checkpoint shape other than the bootstrap-only shape keeps the previous fail-closed hold. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, via Claude Code with tool use (shell, file edits, GitHub CLI) and extended reasoning. ## 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 --- doc/run-log-events.md | 8 + .../native-runtime/native-bootstrap-proof.ts | 68 +++++++++ .../native-runtime/native-restart-recovery.ts | 25 +++- .../native-runner-checkpoint.ts | 48 ++++++ ...unner-restart-recovery.integration.test.ts | 137 ++++++++++++++++++ .../native-runtime/native-session-executor.ts | 76 +++------- 6 files changed, 308 insertions(+), 54 deletions(-) create mode 100644 server/src/services/native-runtime/native-bootstrap-proof.ts diff --git a/doc/run-log-events.md b/doc/run-log-events.md index 29e7669f97e..9ac6842d450 100644 --- a/doc/run-log-events.md +++ b/doc/run-log-events.md @@ -347,6 +347,14 @@ For a prepared remote workspace interrupted before its first executor claim, that ledger records `remote_execution_never_claimed` with a `bootstrap_incomplete` disposition. The provider attempt remains zero during classification; the executor increments it before starting provider work. +A claimed remote bootstrap (the sandbox runner was launched and may have +connected) records `remote_bootstrap_proven_incomplete` with the same +disposition. Recovery admits it only after the previous controller lease has +expired and the Postgres controller checkpoint holds just `run.prepare` and an +unfinished `session.open`, with no committed or acknowledged event. The +executor restores that checkpoint and checks the same proof before it +bootstraps again. After three provider attempts it records +`execution_recovery_budget_exhausted` instead. For a local Codex, OpenCode, or ACPX turn lost while runnerd was stopped, the canonical `turn.failed` event retains `error.code: "provider_turn_lost_on_restore"` and diff --git a/server/src/services/native-runtime/native-bootstrap-proof.ts b/server/src/services/native-runtime/native-bootstrap-proof.ts new file mode 100644 index 00000000000..786db4d3034 --- /dev/null +++ b/server/src/services/native-runtime/native-bootstrap-proof.ts @@ -0,0 +1,68 @@ +/** + * Pure proofs over durable runner state that a bootstrap never reached + * provider work. Restart recovery and the executor share them, so the claim + * and the executor's own check accept exactly the same shapes. + */ + +export const RUNNERD_CONTROL_PLANE_STATE_SCHEMAS: ReadonlySet = new Set([ + "paperclip.runner.durable.control-plane-state.v1", + "paperclip.runner.durable.control-plane-state.v2", +]); + +const RUNNERD_BOOTSTRAP_COMMAND_TYPES = new Set(["run.prepare", "session.open"]); + +function record(value: unknown): Record { + return value && typeof value === "object" && !Array.isArray(value) + ? (value as Record) + : {}; +} + +/** The runner never connected, so every bootstrap command is still undelivered. */ +export function runnerdStateIsUnconnectedBootstrap(state: Record): boolean { + const commands = Array.isArray(state.commands) ? state.commands.map(record) : []; + const committedEvents = Array.isArray(state.committedEvents) ? state.committedEvents : []; + return RUNNERD_CONTROL_PLANE_STATE_SCHEMAS.has(String(state.schema)) && + state.connectionCount === 0 && committedEvents.length === 0 && + commands.every(command => command.status === "pending" && + RUNNERD_BOOTSTRAP_COMMAND_TYPES.has(String(command.type))); +} + +/** + * A provider turn can start only after session.open completes and the + * controller issues a turn command. A root whose controller journal stops + * before that point (run.prepare may be acknowledged, session.open never is, + * no event was committed or acknowledged) and whose runner journal emitted no + * event either holds no provider work, so restart recovery may quarantine it + * and bootstrap again. Any other shape stays fail-closed. + * + * `runnerJournal` is `undefined` when the runner journal is not on this host + * (never written locally, or a remote runner), `null` when it is unreadable. + */ +export function runnerdStateIsIncompleteBootstrap( + state: Record, + runnerJournal?: Record | null, +): boolean { + if (runnerdStateIsUnconnectedBootstrap(state)) return true; + if (!RUNNERD_CONTROL_PLANE_STATE_SCHEMAS.has(String(state.schema))) return false; + if (typeof state.connectionCount !== "number" || !Number.isSafeInteger(state.connectionCount) || + state.connectionCount < 0) return false; + if (!Array.isArray(state.committedEvents) || state.committedEvents.length !== 0) return false; + if (state.ackedSourceSeq !== 0) return false; + if (state.warmTransition != null || state.completedWarmTransition != null || + state.stagedRunAttachment != null) return false; + if (record(state.bootstrapHistory).adopted === true) return false; + if (!Array.isArray(state.commands)) return false; + const commands = state.commands.map(record); + if (!commands.every(command => RUNNERD_BOOTSTRAP_COMMAND_TYPES.has(String(command.type)) && + (command.status === "pending" || + (command.type === "run.prepare" && command.status === "completed")))) return false; + // The controller cannot see runner events that were emitted but never + // committed. When the runner journal exists it must agree that none were. + if (runnerJournal === undefined) return true; + if (runnerJournal === null) return false; + const processed = Object.values(record(runnerJournal.processedCommands)).map(record); + return Array.isArray(runnerJournal.outbox) && runnerJournal.outbox.length === 0 && + runnerJournal.nextSourceSeq === 1 && runnerJournal.ackedSourceSeq === 0 && + processed.every(command => RUNNERD_BOOTSTRAP_COMMAND_TYPES.has(String(command.commandType)) && + (command.commandType === "run.prepare" || command.status !== "completed")); +} diff --git a/server/src/services/native-runtime/native-restart-recovery.ts b/server/src/services/native-runtime/native-restart-recovery.ts index 1baceb4e514..d750a83a141 100644 --- a/server/src/services/native-runtime/native-restart-recovery.ts +++ b/server/src/services/native-runtime/native-restart-recovery.ts @@ -11,7 +11,8 @@ import { notifyCancelledWorkspacePreservation } from "../cancelled-workspace-pre import { notifyInstructionCollection } from "../instruction-collection-notifications.js"; import { notifyExecutionRunWork } from "../execution-control-notifications.js"; import { preserveNativeFailedWorkspace } from "./native-failed-workspace-recovery.js"; -import { hasNativeRunnerCheckpoint, loadNativeRunnerCheckpointHeader } from "./native-runner-checkpoint.js"; +import { hasNativeRunnerCheckpoint, loadNativeRunnerCheckpointHeader, + nativeRunnerCheckpointProvesIncompleteBootstrap } from "./native-runner-checkpoint.js"; import { resolveNativeStagedAttachmentRecovery, type NativeStagedAttachmentRecovery } from "./native-staged-attachment-recovery.js"; import { readNativeWorkspaceSyncReference } from "./native-workspace-sync.js"; import { activeNativeExecutionRunIds, NATIVE_PREPARATION_FENCING_VERSION } from "./native-active-runs.js"; @@ -340,6 +341,8 @@ export function classifyNativeRunnerRecoveryEvidence(input: { runnerGroupAlive: boolean; remoteSandbox?: boolean; unclaimedBootstrap?: boolean; + /** The durable controller checkpoint of a claimed remote bootstrap proves no provider work. */ + remoteBootstrapProvenIncomplete?: boolean; processStartMatches: boolean; knownProviderProcessAlive?: boolean; knownProviderProcessIdentityAmbiguous?: boolean; @@ -368,6 +371,12 @@ export function classifyNativeRunnerRecoveryEvidence(input: { if (input.unclaimedBootstrap === true && !input.hasCheckpoint && !input.hasProviderEvidence) { return { claimKind: "bootstrap_incomplete", reason: "remote_execution_never_claimed" }; } + if (input.remoteBootstrapProvenIncomplete === true && !input.hasCheckpoint && !input.hasProviderEvidence) { + // Each same-run bootstrap is another provider attempt; keep restarts bounded. + if ((input.providerAttempt ?? 0) >= 3) + return { claimKind: null, reason: "execution_recovery_budget_exhausted" }; + return { claimKind: "bootstrap_incomplete", reason: "remote_bootstrap_proven_incomplete" }; + } return input.hasCheckpoint && input.checkpointIdentityMatches === true && input.hasProviderEvidence @@ -1017,9 +1026,23 @@ export async function claimNativeRestartRecoveries(input: { unclaimedBootstrap = !executionEvent; } + // A claimed remote bootstrap (runner launched, perhaps connected) has no + // provider evidence when its controller dies before session.open + // completes. The owner's lease has expired above, and the controller + // writes every command to Postgres before sending it, so that checkpoint + // is the complete record of what reached the sandbox runner. When it + // proves no provider work, bootstrap again instead of holding the task. + const remoteBootstrapProvenIncomplete = !unclaimedBootstrap && remoteWorkspace !== null && !pluginTask + && Boolean(durableAuthority) && stagedAttachment === null && !hasCheckpoint && !hasProviderEvidence + && Boolean(row.run.runnerInstanceId && row.run.nativeSessionId) + && await nativeRunnerCheckpointProvesIncompleteBootstrap(tx, { + companyId: row.run.companyId, issueId: row.coordinator.issueId, runId: row.run.id, + }, { runnerInstanceId: row.run.runnerInstanceId!, normalizedSessionId: row.run.nativeSessionId! }); + const classification = classifyNativeRunnerRecoveryEvidence({ remoteSandbox: remoteSandbox || Boolean(pluginTask && durableAuthority), unclaimedBootstrap, + remoteBootstrapProvenIncomplete, runnerPidAlive, runnerGroupAlive, processStartMatches: exactRunnerIdentity, diff --git a/server/src/services/native-runtime/native-runner-checkpoint.ts b/server/src/services/native-runtime/native-runner-checkpoint.ts index 9e7340553d5..f72e9123c6c 100644 --- a/server/src/services/native-runtime/native-runner-checkpoint.ts +++ b/server/src/services/native-runtime/native-runner-checkpoint.ts @@ -5,6 +5,7 @@ import { inspectDurablePrpCheckpointHeader, inspectUnclaimedBootstrapCheckpoint, import { createLocalEncryptedCipher } from "../../secrets/local-encrypted-provider.js"; import { legacyControllerBootId } from "../legacy-controller-lease.js"; import { assertNativeSourceCheckpointConsumer, lockSourceRecoveryCoordinators } from "./native-source-recovery-ownership.js"; +import { runnerdStateIsIncompleteBootstrap } from "./native-bootstrap-proof.js"; type Binding = { companyId: string; issueId: string; runId: string }; export const NATIVE_CHECKPOINT_HEADER_INSPECTION_MAX_BYTES = 1024 * 1024; @@ -248,6 +249,53 @@ export async function loadNativeUnclaimedBootstrapCheckpoint( } catch { return null; } } +const BOOTSTRAP_CHECKPOINT_PARTS = new Set(["header", "cursor", "command:1", "command:2"]); + +/** + * A remote runner's controller state lives only in this checkpoint, and every + * command is written here before the controller sends it. Once the previous + * controller's lease has expired, the checkpoint is therefore a complete record + * of what reached the sandbox runner. Returns true only when that record holds + * the bootstrap commands alone (run.prepare, an unfinished session.open) with + * no committed or acknowledged event, so no provider turn can exist. The + * sandbox runner journal is not on this host; the proof uses the controller + * record alone, as for a local root whose runner never journaled. + */ +export async function nativeRunnerCheckpointProvesIncompleteBootstrap( + db: Pick, binding: Binding, expected: { runnerInstanceId: string; normalizedSessionId: string }, +): Promise { + try { + 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(BOOTSTRAP_CHECKPOINT_PARTS.size + 1); + if (!parts.length || parts.length > BOOTSTRAP_CHECKPOINT_PARTS.size + || parts.some(row => !BOOTSTRAP_CHECKPOINT_PARTS.has(row.part))) return false; + const checkpoint = await loadNativeRunnerCheckpoint(db, binding); + if (!checkpoint || checkpoint.predecessor !== null + || checkpoint.recoveryEvidence.runnerInstanceId !== expected.runnerInstanceId + || checkpoint.recoveryEvidence.normalizedSessionId !== expected.normalizedSessionId) return false; + // The decoder above validated these canonical v2 records. Rebuild the + // controller state in memory; the part check admits no event records. + const read = (part: string) => { + const value = checkpoint.records.get(part); + return value === undefined ? undefined : JSON.parse(value) as Record; + }; + const header = read("header"); + const cursor = read("cursor"); + if (header?.schema !== "paperclip.runner.controller-checkpoint.v2" || !cursor) return false; + const state: Record = { + ...(header.state as Record), + ackedSourceSeq: cursor.ackedSourceSeq, + commands: ["command:1", "command:2"].map(read).filter(command => command !== undefined), + committedEvents: [], + }; + return (state.identity as { runId?: unknown } | undefined)?.runId === binding.runId + && runnerdStateIsIncompleteBootstrap(state); + } catch { + return false; + } +} + export async function restoreNativeRunnerCheckpoint(db: Db, binding: Binding, root: string, normalizedSessionId?: string) { const checkpoint = await loadNativeRunnerCheckpoint(db, binding); if (!checkpoint) return null; diff --git a/server/src/services/native-runtime/native-runner-restart-recovery.integration.test.ts b/server/src/services/native-runtime/native-runner-restart-recovery.integration.test.ts index 5241fe2b5b5..a9cc8f325cc 100644 --- a/server/src/services/native-runtime/native-runner-restart-recovery.integration.test.ts +++ b/server/src/services/native-runtime/native-runner-restart-recovery.integration.test.ts @@ -60,7 +60,10 @@ import { detachNativeSessionsForRestart, executePaperclipNativeSession, NativeControllerDetachedForRestartError, + restoreRemoteBootstrapCheckpointForRecovery, + runnerdStateProvesIncompleteBootstrap, } from "./native-session-executor.js"; +import type { AdapterExecutionTarget } from "@paperclipai/adapter-utils/execution-target"; import { listExpiredNativeRunnerAuthorities, loadNativeRunnerCheckpoint, @@ -2220,6 +2223,140 @@ describeEmbeddedPostgres("native runner restart recovery with real processes", ( }, ); + // RC-29: a restart while a Daytona runner was connecting. The executor had + // claimed the run and launched the sandbox runner, which authenticated, but + // session.open never completed. The controller's Postgres checkpoint is the + // only record of what the runner received. + async function seedClaimedRemoteBootstrap(label: string, options: { + attempt?: number; + mutate?: (core: DurablePrpControlPlane) => void; + checkpoint?: boolean; + } = {}) { + const fixture = await seedRun(`REMOTE-CLAIMED-${label}`); + const { runId, issueId, native } = fixture; + const remote = remotePreparedWorkspace(); + await fixture.db.update(heartbeatRuns).set({ + runnerProfileJson: { ...fixture.preparedProfile, nativeWorkspaceSync: remote }, + processPid: 443, processStartedAt: new Date(Date.now() - 30_000), + }).where(eq(heartbeatRuns.id, runId)); + await fixture.db.insert(heartbeatRunEvents).values([ + { companyId, runId, agentId, seq: 900, stream: "system", level: "info", eventType: "native.process_start_requested" }, + { companyId, runId, agentId, seq: 901, stream: "system", level: "info", eventType: "native.process_identity_recorded", + payload: { targetKind: "remote", processPid: 443 } }, + ]); + const bootId = randomUUID(); + const leaseOwner = `${bootId}:1:${randomUUID()}`; + await fixture.db.update(nativeRunFinalizations).set({ + attempt: options.attempt ?? 1, controllerGeneration: 1, controllerBootId: bootId, controllerPid: 2_000_000_001, + controllerProcessStartedAt: new Date(Date.now() - 60_000), leaseOwner, leaseExpiresAt: new Date(Date.now() + 60_000), + }).where(eq(nativeRunFinalizations.runId, runId)); + if (options.checkpoint !== false) { + const identity = { runnerInstanceId: native.runnerInstanceId, environmentLeaseId: runId, runId, + normalizedSessionId: native.normalizedSessionId, turnId: `turn-${runId}`, itemId: `item-${runId}` }; + const core = new DurablePrpControlPlane({ stateDirectory: resolve(runtimeRoot, runId, "control-plane"), identity, + expectedRunnerVersion: "0.3.0", expectedRunnerDigest: `sha256:${"a".repeat(64)}` }); + try { + const template = { provider: { provider: "acpx", command: "fixture" }, authorizedTools: {} }; + core.persistRunAttachTemplate(template); + core.queueCommand("run.prepare", template); + core.queueCommand("session.open", { reuse: "same_session" }); + core.issueBootstrapTicket(); + // The runner authenticated once and received run.prepare. + core.store.state.connectionCount = 1; + core.store.state.freshBootstraps = 1; + core.store.state.commandDeliveryCounts = { [core.store.state.commands[0]!.commandId]: 1 }; + options.mutate?.(core); + core.setCheckpointPersistence(nativeRunnerCheckpointWriter(fixture.db, { + companyId, issueId, runId, authorityRunId: runId, bootId, leaseOwner, generation: 1, + })); + await core.flushCheckpoint(); + } finally { + await core.stop(); + } + } + // The killed controller can no longer renew its lease. + await fixture.db.update(nativeRunFinalizations).set({ leaseExpiresAt: new Date(Date.now() - 1_000) }) + .where(eq(nativeRunFinalizations.runId, runId)); + return fixture; + } + + it("restarts a claimed remote bootstrap whose durable checkpoint proves no provider work", async () => { + const fixture = await seedClaimedRemoteBootstrap("PROVEN"); + const dispositions = await claimNativeRestartRecoveries({ db: fixture.db, controller: successor, + restartKind: "hard", runIds: [fixture.runId] }); + expect(dispositions).toEqual([expect.objectContaining({ kind: "bootstrap_incomplete", runId: fixture.runId, + providerAttempt: 1, controllerGeneration: 2 })]); + const [coordinator] = await fixture.db.select().from(nativeRunFinalizations) + .where(eq(nativeRunFinalizations.runId, fixture.runId)); + expect(coordinator).toMatchObject({ phase: "observed", recoveryState: "bootstrap_incomplete", failureCode: null }); + expect(coordinator.recoveryHistory.at(-1)).toMatchObject({ disposition: "bootstrap_incomplete", + reason: "remote_bootstrap_proven_incomplete" }); + const [run] = await fixture.db.select().from(heartbeatRuns).where(eq(heartbeatRuns.id, fixture.runId)); + expect(run).toMatchObject({ status: "running", errorCode: null, processPid: null }); + expect(await fixture.db.select().from(issueRecoveryActions) + .where(eq(issueRecoveryActions.sourceIssueId, fixture.issueId))).toHaveLength(0); + }); + + it("lets the executor prove the remote bootstrap from its Postgres checkpoint, not the local marker", async () => { + const fixture = await seedClaimedRemoteBootstrap("EXECUTOR"); + const execution = buildNativeExecutionInput({ + companyId, agentId, runId: fixture.runId, + issue: { id: fixture.issueId, identifier: null, title: "Restart bootstrap", description: null, workMode: "standard" }, + taskPrompt: "Restart bootstrap", provider: "codex", + workspace: { id: randomUUID(), cwd: runtimeRoot, repoUrl: null, repoRef: null, branchName: null }, + normalizedSessionId: fixture.native.normalizedSessionId, + runtimeContext: nativeRuntimeContextFixture(), + completionContract: { id: randomUUID(), sha256: `sha256:${"a".repeat(64)}`, schemaVersion: "paperclip.run-result.v1", + contract: { revision: "1", objective: "Restart bootstrap", criteria: [{ id: "result", requirement: "Complete work" }] } }, + }); + const root = resolve(runtimeRoot, `marker-${fixture.runId}`); + await mkdir(resolve(root, "control-plane"), { recursive: true, mode: 0o700 }); + const writeMarker = (runId: string) => writeFile(resolve(root, "control-plane", "control-plane-state.json"), JSON.stringify({ + schema: "paperclip.runner.external-control-plane-state.v1", + identity: { runnerInstanceId: fixture.native.runnerInstanceId, environmentLeaseId: fixture.runId, runId, + normalizedSessionId: fixture.native.normalizedSessionId, turnId: `turn-${runId}`, itemId: `item-${runId}` }, + })); + const claim = { kind: "bootstrap_incomplete" as const, runId: fixture.runId, leaseOwner: "successor", + controllerGeneration: 2, providerAttempt: 1, restartKind: "hard" as const, recoveryRequestId: null }; + const target = { kind: "remote", transport: "sandbox", remoteCwd: "/workspace" } as AdapterExecutionTarget; + try { + await writeMarker(fixture.runId); + expect(runnerdStateProvesIncompleteBootstrap(root)).toBe(false); + // Only a bootstrap recovery on a classic sandbox restores the checkpoint. + for (const other of [{ restartRecovery: undefined, runnerExecutionTarget: target }, + { restartRecovery: claim, runnerExecutionTarget: { ...target, runnerTask: vi.fn() } as AdapterExecutionTarget }]) { + await restoreRemoteBootstrapCheckpointForRecovery({ db: fixture.db, execution, root, ...other }); + expect(runnerdStateProvesIncompleteBootstrap(root)).toBe(false); + } + // A marker for another run never receives this run's checkpoint. + await writeMarker(randomUUID()); + await restoreRemoteBootstrapCheckpointForRecovery({ db: fixture.db, execution, root, + restartRecovery: claim, runnerExecutionTarget: target }); + expect(runnerdStateProvesIncompleteBootstrap(root)).toBe(false); + await writeMarker(fixture.runId); + await restoreRemoteBootstrapCheckpointForRecovery({ db: fixture.db, execution, root, + restartRecovery: claim, runnerExecutionTarget: target }); + expect(runnerdStateProvesIncompleteBootstrap(root)).toBe(true); + } finally { + await rm(root, { recursive: true, force: true }); + } + }); + + it.each<[string, Parameters[1], string]>([ + ["a turn was issued", { mutate: core => { core.queueCommand("turn.start", { prompt: "Prior work must stay held." }); } }, + "ambiguous_remote_recovery_evidence"], + ["an event was acknowledged", { mutate: core => { core.store.state.ackedSourceSeq = 1; } }, + "ambiguous_remote_recovery_evidence"], + ["the state was adopted", { mutate: core => { core.store.state.bootstrapHistory!.adopted = true; } }, + "ambiguous_remote_recovery_evidence"], + ["no checkpoint exists", { checkpoint: false }, "ambiguous_remote_recovery_evidence"], + ["the bootstrap budget is spent", { attempt: 3 }, "execution_recovery_budget_exhausted"], + ])("keeps a claimed remote bootstrap held when %s", async (label, options, reason) => { + const fixture = await seedClaimedRemoteBootstrap(label.replace(/\W+/g, "-"), options); + expect(await claimNativeRestartRecoveries({ db: fixture.db, controller: successor, + restartKind: "hard", runIds: [fixture.runId] })).toEqual([{ kind: "blocked", runId: fixture.runId, reason }]); + }); + it.each([ { attempt: 2, driverKind: "codex_app_server" }, { attempt: 3, driverKind: "codex_app_server" }, diff --git a/server/src/services/native-runtime/native-session-executor.ts b/server/src/services/native-runtime/native-session-executor.ts index 5d2321c14e6..c7fddc8388b 100644 --- a/server/src/services/native-runtime/native-session-executor.ts +++ b/server/src/services/native-runtime/native-session-executor.ts @@ -85,6 +85,8 @@ import { createNativeToolTrace, type NativeToolTrace } from "./native-tool-trace import { createNativeProviderFailureObservation } from "./native-provider-failure.js"; import { nativeStartupRetryExhaustedAction, nativeStartupRetryWait, planNativeStartupRetry } from "./native-startup-retry.js"; import { admitUnclaimedNativeBootstrapRetry } from "./native-bootstrap-retry.js"; +import { RUNNERD_CONTROL_PLANE_STATE_SCHEMAS, runnerdStateIsIncompleteBootstrap, + runnerdStateIsUnconnectedBootstrap } from "./native-bootstrap-proof.js"; import { createNativeDeliverableTextObservation } from "./native-deliverable-text.js"; import { createNativeGitHubAccess, type NativeGitHubAccess } from "./native-github-access.js"; import { resolveGitHubOperationCredentials } from "../github-operation-credentials.js"; @@ -522,10 +524,6 @@ const ACPX_RUNTIME_CHECKPOINT_EXCLUDED_ENTRIES = [ "home/.npm", "data/pnpm/store", ] as const; -const RUNNERD_CONTROL_PLANE_STATE_SCHEMAS = new Set([ - "paperclip.runner.durable.control-plane-state.v1", - "paperclip.runner.durable.control-plane-state.v2", -]); const RUNNERD_STATE_SCHEMA = "paperclip.runner.durable.state.v1"; const CODEX_PROVIDER_STATE_SCHEMA = "paperclip.runner.codex-provider-state.v1"; const ACPX_PROVIDER_STATE_SCHEMA = "paperclip.runner.acpx-provider-state.v3"; @@ -5486,6 +5484,7 @@ async function migrateRunnerdStateRootForExecution(input: { if (input.restartRecovery?.kind === "reattach_remote_runner" && !existsSync(scoped)) { throw new Error("runner_state_identity_mismatch: remote_reattach_root_missing"); } + await restoreRemoteBootstrapCheckpointForRecovery({ ...input, root: scoped }); if (existsSync(scoped)) { if (!isSafeNativeStateDirectory(scoped)) { throw new Error("runner_state_directory_unsafe"); @@ -5952,55 +5951,6 @@ function localProcessDefinitelyGone(value: unknown, group = false): boolean { } } -const RUNNERD_BOOTSTRAP_COMMAND_TYPES = new Set(["run.prepare", "session.open"]); - -/** The runner never connected, so every bootstrap command is still undelivered. */ -function runnerdStateIsUnconnectedBootstrap(state: Record): boolean { - const commands = Array.isArray(state.commands) ? state.commands.map(record) : []; - const committedEvents = Array.isArray(state.committedEvents) ? state.committedEvents : []; - return RUNNERD_CONTROL_PLANE_STATE_SCHEMAS.has(String(state.schema)) && - state.connectionCount === 0 && committedEvents.length === 0 && - commands.every(command => command.status === "pending" && - RUNNERD_BOOTSTRAP_COMMAND_TYPES.has(String(command.type))); -} - -/** - * A provider turn can start only after session.open completes and the - * controller issues a turn command. A root whose controller journal stops - * before that point (run.prepare may be acknowledged, session.open never is, - * no event was committed or acknowledged) and whose runner journal emitted no - * event either holds no provider work, so restart recovery may quarantine it - * and bootstrap again. Any other shape stays fail-closed. - */ -function runnerdStateIsIncompleteBootstrap( - state: Record, - runnerJournal?: Record | null, -): boolean { - if (runnerdStateIsUnconnectedBootstrap(state)) return true; - if (!RUNNERD_CONTROL_PLANE_STATE_SCHEMAS.has(String(state.schema))) return false; - if (typeof state.connectionCount !== "number" || !Number.isSafeInteger(state.connectionCount) || - state.connectionCount < 0) return false; - if (!Array.isArray(state.committedEvents) || state.committedEvents.length !== 0) return false; - if (state.ackedSourceSeq !== 0) return false; - if (state.warmTransition != null || state.completedWarmTransition != null || - state.stagedRunAttachment != null) return false; - if (record(state.bootstrapHistory).adopted === true) return false; - if (!Array.isArray(state.commands)) return false; - const commands = state.commands.map(record); - if (!commands.every(command => RUNNERD_BOOTSTRAP_COMMAND_TYPES.has(String(command.type)) && - (command.status === "pending" || - (command.type === "run.prepare" && command.status === "completed")))) return false; - // The controller cannot see runner events that were emitted but never - // committed. When the runner journal exists it must agree that none were. - if (runnerJournal === undefined) return true; - if (runnerJournal === null) return false; - const processed = Object.values(record(runnerJournal.processedCommands)).map(record); - return Array.isArray(runnerJournal.outbox) && runnerJournal.outbox.length === 0 && - runnerJournal.nextSourceSeq === 1 && runnerJournal.ackedSourceSeq === 0 && - processed.every(command => RUNNERD_BOOTSTRAP_COMMAND_TYPES.has(String(command.commandType)) && - (command.commandType === "run.prepare" || command.status !== "completed")); -} - /** `undefined` when the runner never journaled, `null` when unreadable. */ function readRunnerdBootstrapJournal(root: string): Record | null | undefined { const runnerRoot = resolve(root, "runner"); @@ -6029,6 +5979,26 @@ function readRunnerdControlPlaneState(root: string): Record { ); } +/** + * A remote runner's controller state lives in Postgres; its local root holds + * only a fail-closed marker. Before a `bootstrap_incomplete` recovery checks + * that root, restore the exact checkpoint so the same incomplete-bootstrap + * proof applies. A missing checkpoint leaves the marker, which stays fail-closed. + */ +export async function restoreRemoteBootstrapCheckpointForRecovery(input: { + db: Db; execution: NativeExecutionInput; root: string; + restartRecovery?: NativeRestartRecoveryClaim; + runnerExecutionTarget?: AdapterExecutionTarget | null; +}): Promise { + const target = input.runnerExecutionTarget; + if (input.restartRecovery?.kind !== "bootstrap_incomplete" || target?.kind !== "remote" + || target.transport !== "sandbox" || target.runnerTask || !existsSync(input.root) + || readRunnerdDurableIdentity(input.root) !== null) return; + const marker = readRunnerdDurableIdentity(input.root, true); + if (!marker || marker.runId !== input.execution.binding.runId) return; + await restoreNativeRunnerCheckpoint(input.db, input.execution.binding, input.root, nativeSessionKey(input.execution)); +} + export function runnerdStateProvesIncompleteBootstrap(root: string): boolean { try { return runnerdStateIsIncompleteBootstrap(readRunnerdControlPlaneState(root), readRunnerdBootstrapJournal(root)); From 4d554f08218b04df49ec0f2a0d5c5dfdfaf65660 Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Fri, 6 Nov 2026 14:52:38 -0600 Subject: [PATCH 5/5] test(runner): set default inputs in workflow target fixture (#19602) ## Thinking Path > - Paperclip manages work performed by AI agents. > - Its Runner support tests verify workflow authorization and target pinning. > - The workflow now passes two Boolean inputs to its shell guard. > - The target fixture runs that guard with `set -u` but omits those inputs. > - Five cases stop on an unset variable before they reach their assertions. > - This change supplies the workflow's normal false defaults in the fixture. > - The production guard and all target-pinning assertions remain unchanged. ## Linked Issues or Issue Description Refs #19537, #19576. **What happened?** The required Runner support suite fails five `workflow-target.test.ts` cases with `IMAGE_ONLY: unbound variable`. The real workflow supplies `IMAGE_ONLY` and `TESTOPS_REQUESTED`. The test fixture does not supply either value. **Expected behavior** The fixture uses the normal workflow defaults and reaches the target authorization assertions. **Steps to reproduce** Run `pnpm exec vitest run --config tests/runner-e2e/vitest.config.ts workflow-target.test.ts` on master `4938a930d66846094810cbed45d2fd33c0d14c2d`. ## What Changed - Set `IMAGE_ONLY` and `TESTOPS_REQUESTED` to `false` in the target fixture's child environment. ## Verification - RED: the unchanged current-master fixture had five failures and four passes. Bash stopped on `IMAGE_ONLY: unbound variable` before target authorization ran (220ms). - GREEN: all nine cases pass after the one-line fixture repair (168ms). Existing checks still reject a custom bounded target and preserve the queued commit for scheduled and bounded manual runs. - Local evidence uses the actual test and workflow files with installed Vitest 5.0.3 and a temporary focused config. It omits the unrelated provider-bundle global setup. No provider, external service, or paid workflow runs. - The [original required CI failure](https://github.com/paperclipai/paperclip/actions/runs/42288042001/job/128134318743) has the same five Bash failures. Its evidence is retained; no unchanged retry was used. - Whitespace checks pass. Exact head `f1d178ad483a49f8dd087a1c6d22bf43c06a39e4` has 56 successful checks, three expected skips, one neutral advisory status and successful Snyk. No check is pending or failed. The [full Runner support suite](https://github.com/paperclipai/paperclip/actions/runs/42289536842/job/128139308510), typecheck, build and [aggregate verification](https://github.com/paperclipai/paperclip/actions/runs/42289536842/job/128145908574) pass. Greptile and Paperclip Review both rate this exact head 5/5, with no findings or unresolved threads. - Environment: macOS arm64, Node 26.4.0, Vitest 5.0.3. ## Risks Low risk. This changes only fixture inputs for normal target selection. It does not change the production workflow guard, branch policy, credentials, paid execution, timeouts or assertions. Product documentation needs no update for test-only default inputs. ## Model Used OpenAI Codex, GPT-6 family, with repository analysis, code editing and tool execution. The exact runtime model ID and context window are not exposed in this session. ## 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 references) - [x] My branch name describes the change and contains no internal Paperclip ticket id - [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 (not needed: fixture-only default inputs) - [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 --- tests/runner-e2e/workflow-target.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/runner-e2e/workflow-target.test.ts b/tests/runner-e2e/workflow-target.test.ts index a4e1c57998b..d02c01d647b 100644 --- a/tests/runner-e2e/workflow-target.test.ts +++ b/tests/runner-e2e/workflow-target.test.ts @@ -16,6 +16,7 @@ async function resolveTarget(event: string, cadence: string, branch = "master") 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" }, });