mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-29 21:07:40 +01:00
fix: preserve native cleanup quarantine across failed-run retry surfaces
Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
1 parent
4a26f10be7
commit
c81357e4dc
8 files changed
+156
-4
No files matched your search
@@ -1007,6 +1007,19 @@ describe("agent live run routes", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("refuses a quarantined native retry before queuing another failed attempt", async () => {
|
||||
const fixture = createFailedChatRetryDb(false);
|
||||
mockHeartbeatService.getRun.mockResolvedValue({ ...selectedRun,
|
||||
runtimeMode: "native", errorCode: "native_session_cleanup_quarantined" });
|
||||
const res = await requestApp(await createApp(fixture.db, {
|
||||
type: "board", userId: "operator", source: "session", companyIds: ["company-1"],
|
||||
}), url => request(url).post(`/api/agents/${routeAgentId}/wakeup`).send(retryBody));
|
||||
expect(res.status).toBe(409);
|
||||
expect(res.body).toMatchObject({ error: expect.stringContaining("cleanup and reconciliation") });
|
||||
expect(mockHeartbeatService.wakeup).not.toHaveBeenCalled();
|
||||
expect(mockChatRunRetries.prepareFailedChatRunRetry).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("retries a task for an operator without agent-creation permission", async () => {
|
||||
const fixture = createFailedChatRetryDb(false);
|
||||
mockHeartbeatService.getRun.mockResolvedValue({ ...selectedRun, contextSnapshot: {
|
||||
|
||||
@@ -5778,6 +5778,11 @@ export function agentRoutes(
|
||||
if (!["failed", "timed_out"].includes(failedRun.status)) {
|
||||
throw conflict("Only a failed run can be retried.");
|
||||
}
|
||||
if (failedRun.runtimeMode === "native" && failedRun.errorCode === "native_session_cleanup_quarantined") {
|
||||
throw conflict("The stopped native session requires cleanup and reconciliation before a new attempt.", {
|
||||
code: "native_session_cleanup_quarantined",
|
||||
});
|
||||
}
|
||||
const failedContext = asRecord(failedRun.contextSnapshot) ?? {};
|
||||
const issueId =
|
||||
typeof failedContext.issueId === "string"
|
||||
|
||||
@@ -37,6 +37,18 @@ const project = (
|
||||
) => projectExecution(r, c, pending, undefined, now);
|
||||
|
||||
describe("execution truth projection", () => {
|
||||
it("keeps cleanup quarantine visible without a coordinator or recovery row", () => {
|
||||
const stopped = run({ id: "stopped", status: "failed", errorCode: "native_session_cleanup_quarantined" });
|
||||
expect(projectExecution(stopped, undefined, [], undefined, now)).toMatchObject({
|
||||
phase: "recovery_needed", cause: "native_session_cleanup_quarantined", recoveryOwner: "board",
|
||||
permittedActions: ["inspect_run", "inspect_recovery"],
|
||||
});
|
||||
expect(projectExecution(stopped, coordinator({ phase: "retryable_failure" }), [], undefined, now).phase).toBe("recovery_needed");
|
||||
expect(projectExecution({ ...stopped, errorCode: "provider_frame_too_large" }, undefined, [], undefined, now).phase).toBe("failed");
|
||||
expect(projectExecution(stopped, undefined, [], { status: "resolved", cause: "native_session_cleanup_quarantined", nextAction: "Verify prior execution",
|
||||
evidence: { executionReconciliation: { providerStopped: true }, continuationRunId: "successor" } }, now))
|
||||
.toMatchObject({ phase: "completed", successorRunId: "successor", cause: null });
|
||||
});
|
||||
it("shows subscription contention as a resource wait without failed provider attempts", () => {
|
||||
expect(projectExecution(run({ runtimeMode: "legacy", status: "scheduled_retry", scheduledRetryReason: "ai_connection_busy",
|
||||
scheduledRetryAttempt: 12, contextSnapshot: { failureRetriesBeforeAiConnectionWait: 0 } }), undefined, [], undefined, now))
|
||||
|
||||
@@ -239,10 +239,15 @@ export function projectExecution(
|
||||
projection.nextAction = "Waiting for the live workspace holder to finish; the scheduled check will revalidate ownership.";
|
||||
return set("retry_scheduled", "Waiting for workspace");
|
||||
}
|
||||
if (
|
||||
// Cleanup can fail before a finalization coordinator exists. The missing
|
||||
// row must not turn a quarantined native session into an ordinary Retry.
|
||||
const cleanupQuarantined = run.runtimeMode === "native" &&
|
||||
["failed", "timed_out"].includes(run.status) &&
|
||||
run.errorCode === "native_session_cleanup_quarantined";
|
||||
if (!cleanupQuarantined && (
|
||||
coordinator?.phase === "retryable_failure" ||
|
||||
run.status === "scheduled_retry"
|
||||
) {
|
||||
)) {
|
||||
projection.recoveryOwner = "agent";
|
||||
return set(
|
||||
projection.retryAt && new Date(projection.retryAt) > now
|
||||
@@ -253,7 +258,7 @@ export function projectExecution(
|
||||
: "Reconnecting",
|
||||
);
|
||||
}
|
||||
if (coordinator?.phase === "terminal_failure" || recoveryAction) {
|
||||
if (coordinator?.phase === "terminal_failure" || recoveryAction || cleanupQuarantined) {
|
||||
if (
|
||||
coordinator?.failureCode === "native_provider_terminal_failed" &&
|
||||
!detail.replacementDenied &&
|
||||
@@ -271,6 +276,9 @@ export function projectExecution(
|
||||
text(detail.replacementDenied) ??
|
||||
projection.cause;
|
||||
projection.nextAction = recoveryAction?.nextAction ?? projection.nextAction;
|
||||
if (cleanupQuarantined && !projection.nextAction) {
|
||||
projection.nextAction = "Verify the stopped session and its saved work before starting a new attempt.";
|
||||
}
|
||||
projection.permittedActions.push("inspect_recovery");
|
||||
return set("recovery_needed", "Recovery needed");
|
||||
}
|
||||
|
||||
@@ -0,0 +1,90 @@
|
||||
# Native onboarding and Agent Chat qualification — 21 September 2026
|
||||
|
||||
This record distinguishes live behavior from the existence of an eval. No native
|
||||
onboarding default or production prompt was changed. All paid work ran on isolated
|
||||
GitHub Actions workers with real providers, Chromium, Paperclip, and public APIs.
|
||||
|
||||
## Native onboarding
|
||||
|
||||
[Campaign 35656761484](https://github.com/paperclipai/paperclip/actions/runs/35656761484)
|
||||
on master `846336e5a0d3003e1b938e8c983aa5a65bda81ff`: **26/26 passed**,
|
||||
with **26/26 cleanup passes**. The 13 cases ran on both native Codex and native
|
||||
Claude: interviews, clear/ambiguous requests, plain messages, explicit plans,
|
||||
ordinary-task isolation, plan acceptance, card/reply acceptance, acceptance during
|
||||
active work, clarification, revision, and rejection. The observed completed models
|
||||
were `gpt-5.6-sol` and `claude-sonnet-5`; some Codex first-response cases stop on an
|
||||
answerable native question before model metadata is emitted.
|
||||
|
||||
The real wizard creates the agent. The existing fixture switches its runtime
|
||||
through the public API before the first provider run, preserving the wizard's
|
||||
model choice, persona, skills, and task. This qualifies the pre-default native
|
||||
first-task process. It does not qualify native selection in the onboarding UI or
|
||||
production rollout, neither of which is enabled by this change.
|
||||
|
||||
## Initial new-case campaign
|
||||
|
||||
[Campaign 35657128077](https://github.com/paperclipai/paperclip/actions/runs/35657128077)
|
||||
on `3ca6d25e0d20dd36e74c0868872802856e5503fc` retained all six failures:
|
||||
|
||||
- Active reassignment, both providers: the fixture sent an instructions bundle to
|
||||
the general agent PATCH endpoint. It did not update the worker's managed file,
|
||||
so the worker finished before the intended wait. The active boundary was never
|
||||
exercised. Use the instructions-file endpoint and verify its persisted content.
|
||||
- Answer quality, both providers: the prompt did not specify whether the blocker
|
||||
field should contain only its reference or also its description, and did not
|
||||
explicitly exclude the current chat from the active-run count. Codex returned
|
||||
the reference plus its correct description; Claude correctly counted this chat
|
||||
and explained that no other work was active. Claude also put the explanation
|
||||
outside the JSON. These attempts do not establish a factual reasoning failure.
|
||||
The fixture now specifies the JSON shape, reference-only field, and task scope.
|
||||
- Worker crash, both providers: after the verified worker loss, Retry was visible
|
||||
and accepted. The second run immediately failed with
|
||||
`native_session_cleanup_quarantined`. The saved plan and original message
|
||||
survived, but no usable recovered answer was produced. This is a real recovery
|
||||
boundary, not a model failure. Reported billing is incomplete for these crashed
|
||||
runs; absent usage must not be described as zero cost.
|
||||
|
||||
The fault helper now uses a Linux pidfd, checks the command's exact run ID and
|
||||
start ticks, and signals the owned handle. Tests reject changed identity and dead
|
||||
handles. Linux CI also exercises a real disposable child. macOS does not run this
|
||||
fault case; the supported qualification host is Linux.
|
||||
|
||||
## Recovery correction
|
||||
|
||||
A quarantined failure can precede creation of its coordinator row. The execution
|
||||
projection now preserves `recovery_needed` even without that row or with a stale
|
||||
retryable coordinator. The generic failed-run retry endpoint refuses this exact
|
||||
native quarantine instead of admitting another doomed run. An explicitly
|
||||
reconciled successor still clears the historical projection correctly.
|
||||
|
||||
The worker-crash eval remains **non-passing** when recovery is unavailable. It
|
||||
records that saved work survives, verifies the unavailable Retry UI and HTTP 409,
|
||||
and reports the missing usable recovery. Passing a guard test is not recovery
|
||||
qualification. Selecting verified cleanup plus a fresh attempt versus exact-session
|
||||
resume remains a product decision; neither is silently enabled here.
|
||||
|
||||
## Answer-quality review
|
||||
|
||||
Review method: inspect retained user comments, agent comments, question/confirmation
|
||||
cards, and durable task outcomes. This is an agent-authored semantic review, not an
|
||||
independent LLM-judge score and not a claim about all possible questions.
|
||||
|
||||
The 26 onboarding recordings show focused questions for ambiguous requests,
|
||||
concrete proposals for clear requests, preservation of the supplied day/fee/audience,
|
||||
and respect for revision and rejection. The first status answers in the new
|
||||
campaign correctly identify the resolved budget issue, current venue blocker,
|
||||
deferred printing, and unknown attendance count. The second-turn counterfactual
|
||||
questions were not reached in that initial campaign.
|
||||
|
||||
Two user-facing limitations remain worth deciding:
|
||||
|
||||
- One Claude acceptance reply exposed internal wording: “per the confirmation
|
||||
proposal mode.” This conflicts with the existing skill's wording preference.
|
||||
- Five of the six Claude acceptance journeys left a future-tense handoff as the
|
||||
latest parent-thread reply, even though the child later completed. The saved
|
||||
child output and task status passed. Codex posted completion follow-ups in the
|
||||
corresponding journeys. Decide whether a final parent-thread completion notice
|
||||
should be a required outcome; the current onboarding oracle does not require it.
|
||||
|
||||
Production instructions were preserved. These observations must not be hidden by
|
||||
changing prompts solely to make the benchmark green.
|
||||
@@ -930,3 +930,10 @@ runtime switch occurs after the real wizard creates its first agent and before
|
||||
any provider work. It preserves the wizard's model, persona, skills, and task.
|
||||
This tests the native first-task process in advance of the UI/default rollout;
|
||||
it does not certify a native option in the wizard, which is not offered yet.
|
||||
|
||||
Current proof and remaining decisions are recorded in
|
||||
[the 21 September qualification report](QUALIFICATION-2026-09-21.md). In particular,
|
||||
worker loss currently quarantines both providers. The crash eval retains a red
|
||||
qualification result when no usable recovery exists, while also verifying that
|
||||
quarantine preserves the plan and rejects a misleading generic Retry. A passing
|
||||
quarantine guard is not a recovered workflow.
|
||||
@@ -975,7 +975,7 @@ export const runnerSuites: readonly RunnerSuiteFixture[] = [
|
||||
profiles: runnerProfiles.filter(profile => ["runner-codex", "runner-acpx-claude"].includes(profile.id))
|
||||
.map(profile => productionStoryProfile(defaultPermissionProfile(profile))),
|
||||
environments: [localEnvironment], tasks: chatQualificationTasks, expectedMatrixSize: 6,
|
||||
definitionMetadata: { version: 3, permissions: "production-defaults", instructions: "production", crashBoundary: "verified-native-worker-pid-at-file-wait", recovery: "user-visible-retry", answerGrading: "exact-grounded-propositions-plus-separate-semantic-review", scheduling: "explicit-only" },
|
||||
definitionMetadata: { version: 4, permissions: "production-defaults", instructions: "production", crashBoundary: "verified-native-worker-pid-at-file-wait", recovery: "user-visible-retry", answerGrading: "exact-grounded-propositions-plus-separate-semantic-review", scheduling: "explicit-only" },
|
||||
},
|
||||
...(process.env.PAPERCLIP_RUNNER_E2E_CONNECTION_REVIEWS === "1" ? [connectionReviewSuite] : []),
|
||||
{
|
||||
|
||||
@@ -150,6 +150,23 @@ export async function runWorkerCrash(context: Context) {
|
||||
const failed = await input.api.get<Row>(`/api/heartbeat-runs/${boundary.id}`);
|
||||
await input.capture("worker-failed", "Worker loss before user Retry", "worker-failed.png");
|
||||
await writeFile(wait.gate, reference);
|
||||
if (failed.errorCode === "native_session_cleanup_quarantined") {
|
||||
// Preserve the red qualification result, but verify the stop is honest:
|
||||
// the UI/API must not offer an attempt which cannot pass cleanup admission.
|
||||
await input.page.reload({ waitUntil: "domcontentloaded" });
|
||||
await expect(input.page.getByTestId("task-chat-composer-input")).toBeVisible();
|
||||
await expect(input.page.getByRole("button", { name: "Retry", exact: true })).toHaveCount(0);
|
||||
const refused = await input.api.request.post(`/api/agents/${input.fixtures.agent.id}/wakeup`, {
|
||||
data: { failedRunId: failed.id, reason: "retry_failed_run" },
|
||||
});
|
||||
expect(refused.status()).toBe(409);
|
||||
expect(await context.allRuns()).toHaveLength(1);
|
||||
expect(await input.api.get(`/api/issues/${context.issue().id}/documents/plan`)).toEqual(planBefore);
|
||||
await input.evidence("chat-worker-quarantine.json", { failed, retryStatus: refused.status(), savedPlanPreserved: true,
|
||||
usableRecovery: false, classification: "product recovery boundary; no provider retry was admitted" });
|
||||
await input.capture("worker-quarantined", "Worker recovery requires reconciliation", "worker-quarantined.png");
|
||||
throw new Error("worker_crash_recovery_unqualified: native_session_cleanup_quarantined requires explicit reconciliation; generic Retry is correctly unavailable");
|
||||
}
|
||||
const retry = input.page.getByRole("button", { name: "Retry", exact: true });
|
||||
await expect(retry).toHaveCount(1, { timeout: 60_000 });
|
||||
await retry.click();
|
||||
|
||||
Reference in new issue
Block a user