mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-30 05:16:50 +01:00
fix: allow workspace changes after settled native bootstrap failures (#18166)
## Thinking Path > - Paperclip manages work assigned to AI agents. > - Workspace admission must wait for file persistence and execution cleanup. > - A native startup error can record a failed workspace finalization before provider work starts. > - The controller and environment can settle while that diagnostic remains. > - The workspace guard then blocks every later binding change. > - This pull request recognizes settled bootstrap failures with no output obligation. > - Real output, process ownership, and cleanup obligations continue to block changes. ## Linked Issues or Issue Description Related: #14234 handles recovery-hold regeneration for legacy failures. This change addresses the separate native workspace-finalization guard. **What happened?** A workspace change stayed blocked after two native startup failures. Both runs had explicit bootstrap receipts with `providerWorkStarted: false`. Their controllers were terminal and unowned. Their leases had completed cleanup. No provider events or workspace copyback descriptors existed. Each run still had a failed `workspace_finalize` diagnostic. Later successful turns did not remove that historical record, so selecting a task directory failed during setup on every retry. **Expected behavior** A confirmed startup failure with no provider work must not create a permanent file-persistence barrier. A real workspace export or cleanup obligation must remain fenced. **Steps to reproduce** 1. Fail native session startup before provider work, leaving a failed finalization diagnostic. 2. Settle the failed controller and its environment cleanup. 3. Schedule a different task workspace. 4. Start the next admission. Before this fix, it fails with “Previous task run still owns workspace recovery.” **Paperclip version or commit** Reproduced against the workspace guard on master at `27be85904`. **Deployment mode** Hosted application with external PostgreSQL and a plugin-managed agent computer. ## What Changed - Exempt a failed finalization diagnostic only for finished native bootstrap failures with an explicit no-work receipt and an unowned terminal coordinator without a result. - Keep provider events, process identities, restore failures, sync descriptors, copyback owners, unfinished operations, and unsettled environment cleanup as blockers. - Reuse the existing batched provider-evidence reader. - Test both pending workspace selection and legacy binding updates. Keep the original failed operation unchanged. - Document the admission invariant. ## Verification - The new reproduction failed on both workspace-change paths before the fix. - `pnpm exec vitest run server/src/__tests__/task-workspace-binding.test.ts`: 92 tests passed after the fix. - A first isolated test attempt needed the existing plugin SDK build prerequisites. Tests ran after `pnpm --filter @paperclipai/plugin-sdk ensure-build-deps`. - `pnpm -r typecheck` and `pnpm build` pass with pnpm 9.15.4 and Rust 1.97.1. The initial typecheck attempt lacked Cargo on PATH; it was corrected to use the installed toolchain. The unsharded local `pnpm test:run` did not finish during the validation window and was stopped after all PR CI test shards passed. It did not provide a completed local full-suite verdict. - Remote recovery used an audited, scoped transaction to append no-op reconciliation records for the two verified bootstrap failures. Original failures and workspace data were retained. The normal UI Retry then applied the pending selection and reached native provider execution. - Greptile reviewed commit `807d22be9e530e272d53def365a0cdd37bd142ea`: 5/5, with no review threads. All PR CI checks passed. GitHub reports no merge conflicts. ## Risks The main risk is misclassifying an unfinished workspace as a startup-only diagnostic. The exemption requires positive no-work evidence and rejects conflicting provider or output state. Existing ownership and cleanup gates run first. Provider evidence adds one batched database read only when eligible failed bootstrap diagnostics exist. There is no new background work, migration, dependency, or provider replay. Reverting this change restores the former guard. ## Model Used OpenAI Codex, GPT-6 family, with reasoning, code execution, and browser tools. This session does not expose a more specific model build ID or context-window size. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge Co-authored-by: Paperclip <noreply@paperclip.ing>
This commit is contained in:
1 parent
fd98075a22
commit
6d59f5ef7e
3 files changed
+63
-2
No files matched your search
@@ -1379,6 +1379,14 @@ termination or saved files; the existing stop-and-save worker must establish bot
|
||||
before workspace selection can proceed. Both callers hold the task, coordinator
|
||||
and source-run locks throughout retirement and preservation registration.
|
||||
|
||||
A settled native bootstrap failure does not keep a workspace change blocked
|
||||
solely because startup recorded `workspace_finalize=failed`. Admission requires
|
||||
an explicit no-provider-work bootstrap receipt, a finished failed run, an unowned
|
||||
terminal coordinator without a result, and no recorded provider events or process.
|
||||
A restore failure, sync descriptor, copyback owner, unfinished operation, or pending
|
||||
lease cleanup still blocks the change. This applies to next-admission selections
|
||||
and legacy binding updates. Historical failure records remain unchanged.
|
||||
|
||||
When a same-run retry acquires a replacement plugin task lease, the executor
|
||||
reloads the prior task's durable Runner checkpoint before checking local
|
||||
lifecycle evidence. This includes retries after a provider turn omitted its
|
||||
|
||||
@@ -13,7 +13,7 @@ import request from "supertest";
|
||||
import { taskWorkspaceRoutes } from "../routes/task-workspaces.js";
|
||||
import { issueRoutes } from "../routes/issues.js";
|
||||
import { errorHandler } from "../middleware/error-handler.js";
|
||||
import { agents, authUsers, companies, companyMemberships, completionContracts, createDb, environmentLeases, executionWorkspaceRepositories, executionWorkspaceRepositoryAccess, executionWorkspaces, heartbeatRuns, issues, nativeRunFinalizations, nativeRunResults, workspaceOperations, principalPermissionGrants, projectAccessMembers, projects, projectWorkspaces } from "@paperclipai/db";
|
||||
import { agents, authUsers, companies, companyMemberships, completionContracts, createDb, environmentLeases, executionWorkspaceRepositories, executionWorkspaceRepositoryAccess, executionWorkspaces, heartbeatRunEvents, heartbeatRuns, issues, nativeRunFinalizations, nativeRunResults, workspaceOperations, principalPermissionGrants, projectAccessMembers, projects, projectWorkspaces } from "@paperclipai/db";
|
||||
import { executionWorkspaceService } from "../services/execution-workspaces.js";
|
||||
import { authorizationService, canActorReadExecutionWorkspace } from "../services/authorization.js";
|
||||
import { assertChatExecutionDefaultsAccess } from "../services/chat-execution-defaults.js";
|
||||
@@ -418,6 +418,44 @@ const support = await getEmbeddedPostgresTestSupport();
|
||||
}
|
||||
});
|
||||
|
||||
it("allows a workspace change after a settled bootstrap failure with no provider work", async () => {
|
||||
const f = await settledNativeTask("native_continuation_requires_reconciliation");
|
||||
await db.update(heartbeatRuns).set({ status: "failed", nativePhase: "observed",
|
||||
resultJson: { executionRecovery: { kind: "bootstrap", providerWorkStarted: false } },
|
||||
runnerProfileJson: { sessionCheckpoint: { providerSessionId: "inherited-session" } },
|
||||
}).where(eq(heartbeatRuns.id, f.run.id));
|
||||
await db.insert(workspaceOperations).values({ companyId: f.companyId, issueId: f.issueId,
|
||||
heartbeatRunId: f.run.id, executionWorkspaceId: f.workspaceId, phase: "workspace_finalize", status: "failed",
|
||||
metadata: { errorMessage: "runner_state_identity_mismatch: prior_owner_terminal_state_indeterminate" } });
|
||||
await f.change();
|
||||
expect(await f.svc.inspectTaskWorkspace(f.companyId, f.issueId, f.actor)).toMatchObject({
|
||||
workspace: null, pendingSelection: null,
|
||||
});
|
||||
expect((await db.select().from(workspaceOperations).where(eq(workspaceOperations.heartbeatRunId, f.run.id)))[0])
|
||||
.toMatchObject({ status: "failed" });
|
||||
});
|
||||
|
||||
it.each(["unknown startup", "provider started", "provider event", "process identity", "sync descriptor",
|
||||
"copyback owner", "restore failure", "pending cleanup", "accepted result", "unfinished operation"] as const)(
|
||||
"keeps a bootstrap failure with %s fenced", async (obligation) => {
|
||||
const f = await settledNativeTask("native_continuation_requires_reconciliation", obligation === "accepted result");
|
||||
const recovery = { kind: "bootstrap", ...(obligation === "unknown startup" ? {} : { providerWorkStarted: obligation === "provider started" }) };
|
||||
await db.update(heartbeatRuns).set({ status: "failed", resultJson: { executionRecovery: recovery,
|
||||
...(obligation === "restore failure" ? { workspaceRestoreFailure: "restore_failed" } : {}) },
|
||||
...(obligation === "process identity" ? { processPid: 123 } : {}),
|
||||
runnerProfileJson: obligation === "sync descriptor" ? { nativeWorkspaceSync: { state: "prepared" } }
|
||||
: obligation === "copyback owner" ? { nativeWorkspaceFinalizationOwner: { token: "owner" } } : {},
|
||||
}).where(eq(heartbeatRuns.id, f.run.id));
|
||||
if (obligation === "provider event") await db.insert(heartbeatRunEvents).values({ companyId: f.companyId,
|
||||
agentId: f.run.agentId, runId: f.run.id, seq: 1, eventType: "turn.started", stream: "system", payload: {} });
|
||||
if (obligation === "pending cleanup") await db.update(environmentLeases).set({ status: "pending_cleanup" })
|
||||
.where(eq(environmentLeases.id, f.lease.id));
|
||||
await db.insert(workspaceOperations).values({ companyId: f.companyId, issueId: f.issueId,
|
||||
heartbeatRunId: f.run.id, executionWorkspaceId: f.workspaceId, phase: "workspace_finalize",
|
||||
status: obligation === "unfinished operation" ? "running" : "failed" });
|
||||
await expect(f.change()).rejects.toThrow(/workspace|Previous task run/);
|
||||
});
|
||||
|
||||
it.each(["warm release", "unsafe export omission"] as const)("accepts a settled %s receipt", async (receipt) => {
|
||||
const f = await settledNativeTask("native_retry_cancelled", receipt === "unsafe export omission");
|
||||
if (receipt === "warm release") await db.update(environmentLeases).set({ releasedAt: null, status: "retained", leasePolicy: "reuse_by_environment", cleanupStatus: "success" }).where(eq(environmentLeases.id, f.lease.id));
|
||||
|
||||
@@ -31,6 +31,8 @@ import { taskWorkspaceSelectionSchema, type TaskWorkspaceSelection, type TaskWor
|
||||
import { authorizationService, executionWorkspaceReadSqlCondition, projectReadSqlCondition, issueReadSqlCondition, type AuthorizationActor } from "./authorization.js";
|
||||
import type { SQL } from "drizzle-orm";
|
||||
import { createWorkspaceGitInspectionCache } from "./workspace-git-inspection-cache.js";
|
||||
import { nativeSessionProviderEvidence } from "./native-runtime/native-session-resume.js";
|
||||
import { hasWorkspaceRestoreFailure } from "@paperclipai/shared";
|
||||
import { notifyTerminalWorkspaceWork, TERMINAL_WORKSPACE_RETRY_MS } from "./terminal-workspace-work.js";
|
||||
import {
|
||||
deleteWorkspaceCompactionTrash,
|
||||
@@ -1401,9 +1403,22 @@ async function hasPendingTaskWorkspaceRecovery(tx: DbTransaction, input: {
|
||||
for (const operation of operations) {
|
||||
if (operation.runId && !latest.has(operation.runId)) latest.set(operation.runId, operation.status);
|
||||
}
|
||||
const bootstrapRunIds = rows.filter(({ finalization, run }) => {
|
||||
const recovery = parseObject(run.resultJson?.executionRecovery);
|
||||
return run.runtimeMode === "native" && run.status === "failed" && run.finishedAt !== null
|
||||
&& finalization.phase === "terminal_failure" && finalization.resultId === null
|
||||
&& run.processPid === null && run.processGroupId === null
|
||||
&& recovery.kind === "bootstrap" && recovery.providerWorkStarted === false
|
||||
&& !hasWorkspaceRestoreFailure(run.resultJson)
|
||||
&& run.runnerProfileJson?.nativeWorkspaceSync == null
|
||||
&& run.runnerProfileJson?.nativeWorkspaceFinalizationOwner == null
|
||||
&& latest.get(run.id) === "failed";
|
||||
}).map(({ run }) => run.id);
|
||||
const providerEvidence = await nativeSessionProviderEvidence(tx, bootstrapRunIds);
|
||||
const noWorkFailures = new Set(bootstrapRunIds.filter(id => !providerEvidence.has(id)));
|
||||
return rows.some(({ finalization, run }) => {
|
||||
const status = latest.get(run.id);
|
||||
if (status !== undefined) return status !== "succeeded";
|
||||
if (status !== undefined) return status !== "succeeded" && !noWorkFailures.has(run.id);
|
||||
return finalization.phase === "terminal_failure" && finalization.resultId !== null;
|
||||
});
|
||||
}
|
||||
|
||||
Reference in new issue
Block a user