mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-08 22:13:59 +01:00
fix(chat): retry fully recorded setup failures before dispatch (#18882)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Connected tasks retain their exact external request across failures. > - Task Retry must preserve that request and prove that old execution cannot continue. > - A workspace setup failure can settle before any provider dispatch. > - The chat retry guard accepted one specific validation failure but rejected other fully recorded setup failures. > - This pull request reuses the same strict server settlement proof for those failures. > - The benefit is one safe continuation through the existing queue, with all source and execution gates intact. ## Linked Issues or Issue Description Refs #18803, #18845, #18852, #18853 and #18860. These changes preserve workspace writers, bind completed preservation, or repair other preparation and native continuation paths. This change repairs the separate connected-chat guard for fully recorded legacy setup failures. Search also found #18876, which repairs issue scope during deferred wake promotion and does not repair this guard. **What happened?** A connected task failed during workspace reservation before provider dispatch. Its source remained authorized. Task Retry saved an intent and chat action, but the chat worker rejected that request as `failed_chat_retry_not_authorized`. No successor was created. The old failure had a bootstrap marker, a dispatch-only Runner profile, the matching server error and durable measured settlement events, and no runtime, process, output, result or environment lease. **Expected behavior** A deliberate board Retry can continue the exact saved input after the complete never-dispatched proof passes. An unfinished earlier writer, changed source, accepted result or unknown execution state must still prevent execution. **Steps to reproduce** 1. Admit an authorized request through a restricted chat endpoint. 2. Settle setup before provider dispatch, for example while an earlier workspace writer is unfinished. 3. Keep the exact server error and durable measurement events, a dispatch-only Runner profile, and no execution resources. 4. Press task Retry after the independent writer and ordinary admission gates permit work. 5. The original guard saves the Retry action but refuses to create a successor. **Paperclip version or commit** Reproduced on master at `a00109aa35`. The branch is rebased on current master. **Deployment mode** Hosted task diagnosis. Local proof uses real Express HTTP routes, chat authorization, heartbeat queue admission and disposable embedded PostgreSQL on macOS arm64, Node 26.4.0. Provider work is deliberately prevented by occupied fixture capacity. ## What Changed - Extend only canonical task Retry's existing strict pre-provider preparation proof to generic server-settled setup failures. - Keep exact result shape, dispatch profile, controller, runtime, process, output, event and resource checks. No diagnostic message allowlist is added. - Exercise GitHub and Telegram HTTP Retry with the original sender and saved input. Keep the failed run unchanged and create at most one queued successor. - Cover provider events, retained native state/results, modified source, unknown result metadata, live controllers, process identity, source leases and an independent unfinished writer. - Document the guarded contract and correct the obsolete access-map sentence. No new endpoint or agent permission is added. ## Verification - Red before repair: both setup causes failed through the real HTTP/chat/queue path because the durable Retry had no successor. An initial retained-native negative fixture had an invalid foreign-key setup; that fixture was corrected before the final run. - Green: 37 focused integration cases pass, including 22 new GitHub/Telegram cases and 15 existing pre-provider source and execution guards. - Current head `1bf69d20e02a049248ec378f087505563daeb9a0`: all 37 focused integration cases and full repository `pnpm -r typecheck` pass after the master rebase. The full `pnpm build` also passes. - Map structure, module boundaries and `git diff --check` pass. - Full local `pnpm test:run` is running; all six chat shards passed and the remaining server suites are progressing. This is not a full-suite pass claim. - Greptile and Paperclip Review both reviewed this exact head at 5/5 with no actionable findings or review threads. - Initial CI passed all jobs except the unchanged Runner package suite. Its `runtime-host.test.ts` approve-reads case failed before its intended assertions because a managed credential home could not acquire its loopback-port quorum. The exact case passes locally. A wider 121-case check hit the same lease error in a different Pi case (120 passed); all 121 pass with one worker. The exact competing port owner is unknown. One unchanged retry of only failed CI jobs passed, including the Runner package suite and aggregate `ci / verify`. All current-head required CI gates are green; the original failure remains in workflow history. - The dependency setup initially missed the embedded PostgreSQL package's dylib symlinks. Running its own postinstall helper fixed the local setup before assertions. This was not a product test failure. - This change has not been deployed. The original hosted task has not been replayed with the fix. ## Risks - Never-dispatched proof must remain strict. A bootstrap marker or empty process fields alone cannot authorize continuation. The regression cases retain contrary-state holds. - The inherited session display hint remains history and supplies no resume authority. - Current connection, principal, source integrity, pause, budget, approval, dependency and workspace-owner gates remain in their existing owners. - No schema, provider profile, trust policy, model or credential permission changes are required. ## Model Used OpenAI GPT-6 through Codex, with reasoning, code editing, tool use and PostgreSQL/browser verification. This session does not expose a more specific deployment model ID or context-window size. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge Co-authored-by: Paperclip <noreply@paperclip.ing>
This commit is contained in:
1 parent
d8c1b9d4b5
commit
c5251b4c0e
4 files changed
+111
-5
No files matched your search
@@ -64502,6 +64502,107 @@ describeEmbeddedPostgres("chat channel control-plane integration", () => {
|
||||
return { ...f, input, result };
|
||||
}
|
||||
|
||||
it.each((["github", "telegram"] as const).flatMap(provider =>
|
||||
(["writer_setup", "other_setup", "late_event", "retained_native", "accepted_result", "source_changed", "untrusted_metadata", "live_controller", "process", "environment", "prior_writer"] as const).map(mode => ({ provider, mode }))))(
|
||||
"retries a fully recorded never-dispatched $provider setup through HTTP: $mode", async ({ provider, mode }) => {
|
||||
const f = provider === "github" ? await currentGitHubTaskRetryFixture() : await failedChatRetryFixture("telegram", undefined, true);
|
||||
const { issueRoutes } = await import("../routes/issues.js");
|
||||
const companyId = f.fixture.companyId, issueId = f.issue.id;
|
||||
const app = express();
|
||||
app.use(express.json());
|
||||
app.use((req, _res, next) => { req.actor = boardActor(companyId); next(); });
|
||||
app.use("/api", issueRoutes(db, createStorageService().storage, { chatRunRetries: f.service }));
|
||||
app.use(errorHandler);
|
||||
try {
|
||||
if (provider === "telegram") {
|
||||
await db.update(agents).set({ adapterType: "paperclip_runner", status: "running",
|
||||
runtimeConfig: { heartbeat: { wakeOnDemand: true, maxConcurrentRuns: 1 } } })
|
||||
.where(eq(agents.id, f.fixture.assignedAgentId));
|
||||
await db.insert(heartbeatRuns).values({ companyId, agentId: f.fixture.assignedAgentId,
|
||||
status: "running", startedAt: new Date() });
|
||||
f.wakeup.mockImplementation(heartbeatService(db).wakeup);
|
||||
}
|
||||
await db.delete(heartbeatRunEvents).where(eq(heartbeatRunEvents.runId, f.runId));
|
||||
await db.delete(nativeRunFinalizations).where(eq(nativeRunFinalizations.runId, f.runId));
|
||||
const diagnostic = mode === "other_setup" ? "A different preparation failure" : "The workspace still has an unfinished writer or save operation.";
|
||||
await db.update(heartbeatRuns).set({ runtimeMode: "legacy", nativeIssueId: null, nativePhase: null,
|
||||
errorCode: "setup_failed", error: diagnostic, sessionIdBefore: "inherited-display-hint",
|
||||
runnerProfileJson: { adapterDispatch: { adapterType: "paperclip_runner" } },
|
||||
usageJson: { ledgerScope: { issueId, projectId: null, billingCode: null } },
|
||||
resultJson: { stopReason: "adapter_failed", executionRecovery: { kind: "bootstrap", providerWorkStarted: false },
|
||||
timeoutFired: false, timeoutSource: "config", timeoutConfigured: false, effectiveTimeoutSec: 0,
|
||||
...(mode === "untrusted_metadata" ? { providerOutput: "Untrusted adapter data" } : {}),
|
||||
},
|
||||
...(mode === "live_controller" ? { controllerLeaseExpiresAt: new Date(Date.now() + 60_000) } : {}),
|
||||
...(mode === "process" ? { processPid: 999999999, processGroupId: 999999999 } : {}),
|
||||
}).where(eq(heartbeatRuns.id, f.runId));
|
||||
await db.insert(heartbeatRunEvents).values([
|
||||
{ companyId, agentId: f.fixture.assignedAgentId, runId: f.runId, seq: 1,
|
||||
eventType: "error", stream: "system", level: "error", message: diagnostic },
|
||||
{ companyId, agentId: f.fixture.assignedAgentId, runId: f.runId, seq: 2,
|
||||
eventType: "lifecycle", stream: "system", message: "task.run.measured",
|
||||
payload: { phase: "task.run.measured", outcome: "failed", durableSettlement: true, durationMs: 5 } },
|
||||
]);
|
||||
if (mode === "late_event") await db.insert(heartbeatRunEvents).values({ companyId,
|
||||
agentId: f.fixture.assignedAgentId, runId: f.runId, seq: 3, eventType: "session.started",
|
||||
sourceInstanceId: randomUUID(), payload: { prpEvent: { sourceKind: "runner" } } });
|
||||
if (mode === "retained_native") {
|
||||
await db.update(heartbeatRuns).set({ nativeIssueId: issueId }).where(eq(heartbeatRuns.id, f.runId));
|
||||
await db.insert(nativeRunFinalizations).values({ companyId, issueId, runId: f.runId, phase: "terminal_failure" });
|
||||
}
|
||||
if (mode === "environment") await db.insert(environmentLeases).values({ companyId, issueId,
|
||||
heartbeatRunId: f.runId, provider: "daytona", providerLeaseId: randomUUID(), status: "active" });
|
||||
if (mode === "prior_writer") {
|
||||
const previousId = randomUUID();
|
||||
await db.insert(heartbeatRuns).values({ id: previousId, companyId, nativeIssueId: issueId,
|
||||
agentId: f.fixture.assignedAgentId, status: "failed", runtimeMode: "native", nativePhase: "terminal_failure",
|
||||
createdAt: new Date(Date.now() - 60_000), finishedAt: new Date(Date.now() - 30_000) });
|
||||
await db.insert(environmentLeases).values({ companyId, issueId, heartbeatRunId: previousId,
|
||||
provider: "daytona", providerLeaseId: randomUUID(), status: "pending_cleanup", cleanupStatus: "failed" });
|
||||
await db.insert(issueRecoveryActions).values({ companyId, sourceIssueId: issueId,
|
||||
kind: "active_run_watchdog", status: "resolved", ownerType: "system", outcome: "blocked",
|
||||
cause: "native_continuation_requires_reconciliation", fingerprint: randomUUID(), nextAction: "Previous writer remains unknown",
|
||||
evidence: { runId: previousId, automaticRecovery: { replay: "blocked", actionOutcome: "unknown" } } });
|
||||
}
|
||||
if (mode === "source_changed") await db.update(issueComments).set({ body: "Edited after admission", updatedAt: new Date() })
|
||||
.where(eq(issueComments.id, String(f.action.payload.commentId)));
|
||||
if (mode === "accepted_result") {
|
||||
const contractId = randomUUID(), resultId = randomUUID();
|
||||
await db.insert(completionContracts).values({ id: contractId, companyId, issueId, revision: 1,
|
||||
schemaVersion: "paperclip.completion-contract.v1", policyVersion: "test", risk: "standard",
|
||||
completionAuthority: "server_arbiter", incompleteCriteriaPolicy: "preserve_non_terminal", contractJson: {},
|
||||
canonicalSha256: contractId, createdByActorType: "system", createdByActorId: "test" });
|
||||
await db.update(heartbeatRuns).set({ nativeIssueId: issueId, completionContractId: contractId })
|
||||
.where(eq(heartbeatRuns.id, f.runId));
|
||||
await db.insert(nativeRunResults).values({ id: resultId, companyId, issueId, runId: f.runId,
|
||||
completionContractId: contractId, serverFingerprint: resultId, schemaStatus: "accepted", canonicalSha256: resultId,
|
||||
resultJson: { terminal: { runTerminalState: "succeeded" }, summary: "Existing output stays history" } });
|
||||
}
|
||||
const original = (await readHeartbeatRunsById(f.runId))[0];
|
||||
const response = await request(app).post(`/api/issues/${issueId}/retry`).send({ observedRunId: f.runId,
|
||||
idempotencyKey: randomUUID() });
|
||||
if (mode === "writer_setup" || mode === "other_setup") {
|
||||
expect(response.status, JSON.stringify(response.body)).toBe(202);
|
||||
await f.service.processFailedChatRunRetry(response.body.chatActionId);
|
||||
await expect.poll(async () => (await readAgentWakeupRequestsById(response.body.requestId))[0]?.runId).toBeTruthy();
|
||||
const [intent] = await readAgentWakeupRequestsById(response.body.requestId);
|
||||
const [successor] = await readHeartbeatRunsById(intent.runId!);
|
||||
expect(successor).toMatchObject({ status: "queued", retryOfRunId: f.runId,
|
||||
contextSnapshot: { taskRetryRequestId: intent.id, wakeCommentIds: [f.action.payload.commentId] } });
|
||||
expect((await readHeartbeatRunsByCompanyId(companyId)).filter(run => run.retryOfRunId === f.runId)).toHaveLength(1);
|
||||
} else {
|
||||
expect([202, 409], JSON.stringify(response.body)).toContain(response.status);
|
||||
if (response.status === 202) {
|
||||
const outcome = await f.service.processFailedChatRunRetry(response.body.chatActionId);
|
||||
expect(outcome.status).not.toBe("queued");
|
||||
}
|
||||
expect((await readHeartbeatRunsByCompanyId(companyId)).filter(run => run.retryOfRunId === f.runId)).toHaveLength(0);
|
||||
}
|
||||
expect((await readHeartbeatRunsById(f.runId))[0]).toEqual(original);
|
||||
} finally { await f.service.shutdown(); }
|
||||
},
|
||||
);
|
||||
|
||||
it("retries saved GitHub input after a successful run without replaying that run", async () => {
|
||||
const f = await successfulGitHubQueuedInputFixture();
|
||||
try {
|
||||
|
||||
@@ -218,16 +218,19 @@ export function createChatFailedRunRetry(db: Db, options: ChatFailedRunRetryOpti
|
||||
const failedRetry = run.errorCode === "chat_failed_run_retry_not_authorized" && run.error === retryDiagnostic;
|
||||
const validation = run.resultJson?.workspaceValidation as Record<string, unknown> | undefined;
|
||||
const recovery = run.resultJson?.executionRecovery as Record<string, unknown> | undefined;
|
||||
const workspacePreparation = allowWorkspacePreparation && run.errorCode === "workspace_validation_failed" &&
|
||||
const workspaceValidation = run.errorCode === "workspace_validation_failed" &&
|
||||
validation?.reason === "isolated_task_directory_binding_mismatch" &&
|
||||
typeof validation.issueId === "string" && validation.issueId === run.contextSnapshot?.issueId &&
|
||||
(run.issueId === null || run.issueId === validation.issueId) &&
|
||||
(run.contextSnapshot?.taskId === undefined || run.contextSnapshot.taskId === validation.issueId) &&
|
||||
Object.keys(validation).every(key => ["reason", "issueId"].includes(key)) &&
|
||||
Object.keys(validation).every(key => ["reason", "issueId"].includes(key));
|
||||
const setupPreparation = run.errorCode === "setup_failed" && typeof run.error === "string" &&
|
||||
run.error.trim().length > 0 && validation === undefined;
|
||||
const workspacePreparation = allowWorkspacePreparation && (workspaceValidation || setupPreparation) &&
|
||||
recovery?.kind === "bootstrap" && recovery.providerWorkStarted === false &&
|
||||
Object.keys(recovery).every(key => ["kind", "providerWorkStarted"].includes(key)) &&
|
||||
run.resultJson?.stopReason === "adapter_failed" &&
|
||||
Object.keys(run.resultJson).every(key => ["executionRecovery", "workspaceValidation", "stopReason",
|
||||
Object.keys(run.resultJson).every(key => ["executionRecovery", ...(workspaceValidation ? ["workspaceValidation"] : []), "stopReason",
|
||||
"effectiveTimeoutSec", "timeoutConfigured", "timeoutFired", "timeoutSource"].includes(key));
|
||||
const diagnostic = failedRetry ? retryDiagnostic : workspacePreparation ? run.error : "reviewed_chat_execution_binding_not_authorized";
|
||||
const dispatchOnlyProfile = failedRetry || workspacePreparation;
|
||||
|
||||
Reference in new issue
Block a user