mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-12 22:56:58 +01:00
fix(runner): settle saved handoffs after verified shutdown (#20336)
## Thinking Path > - Paperclip manages AI agents and their work. > - An agent can save a question or approval handoff before its provider stops. > - A missing provider terminal notification does not prove that the saved work failed. > - Runner already verifies provider exit, event drain, tool settlement and suspension. > - This change uses that proof to settle the same saved handoff. > - Unproven shutdown holds continuation and explains that the work is saved. > - Recovery must never repeat the completed work or allow overlapping execution. ## Linked Issues or Issue Description Refs #20330. That PR extends the Codex terminal drain window. This change handles the separate case where no terminal arrives but the owned shutdown can prove stopped execution. **What happened?** A native run saved a human handoff, accepted cancellation, and later stopped its provider. The missing terminal notification still made the attempt fail and show “Couldn’t finish this attempt.” **Expected behavior** Keep the saved handoff. Accept its yielded result after either an authoritative provider terminal or verified shutdown of the exact execution. If shutdown remains uncertain, retain the handoff and explain the hold without offering a duplicate Retry. **Steps to reproduce** Save a governed wait, acknowledge interruption, and omit the provider terminal. The new regression uses the real Runner binary and a controlled Codex process. The old runtime rejects the handoff instead of asking the existing shutdown owner to settle it. **Deployment mode** Native Runner with Codex. Local verification used macOS arm64 and Node 26.4.0. Hosted sandbox recovery has not been qualified. ## What Changed - Add an optional driver shutdown-settlement operation. Runner returns proof only after its existing stop, drain, tool settlement and suspension barrier passes. - Bind the receipt to the run, normalized session and turn. Persist it with the saved wait and validate it during recovery. - Complete the same yielded handoff after verified shutdown. Retire the stopped session from warm reuse. Do not invent a provider terminal or complete usage. - Retain a named shutdown-pending condition under the existing bounded same-run recovery policy. Preserve actual provider failures and integrity failures. - Show saved-work shutdown guidance in task and inbox presentation. Keep genuine approvals authoritative and disable duplicate Retry. - Document the completion and restart contract. The checkpoint field is additive; older checkpoints retain the terminal-or-shutdown path. ## Verification - Red: the new runtime regression failed against the original implementation because verified shutdown was never requested. Server presentation and error-classification regressions also failed before the fix. - Red: both inbox shutdown states showed generic text before the inbox change. - Green: Runner runtime, shutdown and harness-backend suites pass 301 tests. Tests cover missing terminals, failed stop proof, wrong identity, cancellation, late proof, genuine provider failure, and restart after receipt persistence. Recovery completes once without another model turn. - Green: the native executor and task-presentation suites pass 1,371 tests. Execution projection, inbox and blocker presentation pass 125 tests. Two test-authoring mistakes were corrected: an integrity-error text expectation and an assertion for a nonexistent projection field. Their initial failures remain in local logs. - Runner TypeScript and full server layered type checks pass. Token gates and diff whitespace checks pass. - Live provider acceptance: real Runner and Codex with gpt-6.1-sol, one disposable local workspace, one tool call, and one deliberately dropped terminal notification. The saved file, bound settlement receipt, one accepted yielded result, one provider turn, confirmed provider exit and suspended Runner all passed. The tool handler and persistence port were controlled. This does not claim a full server or hosted journey. - The full browser question-and-answer cell stopped before execution because OPENAI_API_KEY was absent. No model work ran in that attempt. Hosted sandbox recovery and the full browser continuation journey remain unverified. - `pnpm verify` returned exit 2 because the execution-spec change requires broad CI. Required CI will supply that coverage; a full local repository suite was not repeated. - Review correction in `8553da244e`: a failed trailing-event write could hide a provider failure. A new regression reproduced red, then passed after all ordinary drain errors retained the named pending condition. No settlement receipt or result can be accepted after that error. The live missing-terminal check passed again on the corrected runtime. - The real PostgreSQL control-plane regression and three existing governed-wait controls pass. The new case persists one accepted yielded result, a bound shutdown receipt, and the original pending human question without a provider terminal. - First CI revision `7c2cb9b1`: test-program typecheck found a missing `PrpEvent` annotation in the new fixture. The annotation is fixed and the Runner test-program typecheck passes. Runner support also timed out in the unchanged `registered roots remain protected even when equal to genuine public static bytes` case after 30 seconds. One unchanged local diagnostic passed in 3.69 seconds overall. The hosted timeout cause remains unknown; no timeout, fixture, or assertion was weakened. [Original failed support job](https://github.com/paperclipai/paperclip/actions/runs/42670851821/job/129346900133). - Rebased onto the merged #20330 and current recovery changes. The combined branch passes 1,402 executor/presentation tests. Both prompt and 18-second provider terminals still pass. A warm session with no terminal passes after the full 30-second drain and verified stop/suspension. Its initial overly broad assertion rejected a nonterminal turn event; the corrected assertion rejects every terminal type. The rebased CI gate was cancelled before any runner or step started; its cause is unconfirmed and it supplies no validation credit. Final revision `b49ec5a144` starts fresh CI and review. - Final-head CI attempt 1 failed before creating jobs. GitHub reports an internal server error (correlation `2c576b07-b67f-4179-a69f-74f7d99cd926`), followed by empty runner-selection outputs for the aggregate jobs. One unchanged-commit retry was requested; the initial admission failure remains recorded. [CI run](https://github.com/paperclipai/paperclip/actions/runs/42673159868). - **Current blocker:** final head `b49ec5a144e94e41e96e2508a79bd9aec91fe45a` has a fresh 5/5 Greptile review and zero unresolved threads, but required CI is not complete. GitHub reports an active [Actions and Checks API incident](https://www.githubstatus.com/incidents/tp8cghg7tlc4). Attempt 2 ran only part of the full selected graph. Build, canary, Rust, sandbox providers, admitted browser shards and other completed test jobs passed. The E2E aggregate correctly failed because `policy` and `e2e_shards` had `abandoned` results. Server shard 5/12 received a runner shutdown signal at 18:36:55 UTC and its test step was cancelled; its job still reports in progress. The shutdown cause is not established. The separate review workflow had the same pre-execution internal-error class and passed one bounded retry. No further retry was submitted during the incident. Resume with the missing/cancelled coverage after GitHub recovers; do not count the absent typecheck, Runner or support jobs as passes. [Failed coverage gate](https://github.com/paperclipai/paperclip/actions/runs/42673159868/job/129354318236), [interrupted server shard](https://github.com/paperclipai/paperclip/actions/runs/42673159868/job/129350854174). ## Risks - A missing terminal can use one additional bounded shutdown interval. Unknown shutdown still blocks continuation. - Provider terminal and usage accounting keep their separate meaning. The receipt proves stopped execution and settled tool outcomes, not model success. - Recovery rejects a foreign receipt, a later turn, an active turn with a settled receipt, and an actual failed provider terminal. - There is no database migration. Older checkpoint shapes remain readable. Drivers without this optional proof retain a named pending state. - This PR does not deploy the fix or repair an existing hosted task. ## Model Used OpenAI Codex, GPT-6 family, with reasoning, repository tools and local code execution. The exact serving model ID and context window are not exposed in this session. Live test inference used gpt-6.1-sol. ## 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 - [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
a67b936885
commit
0e325e8d52
22 files changed
+413
-44
No files matched your search
@@ -95,6 +95,7 @@ import {
|
||||
NativeProviderSideEffectPendingError,
|
||||
NativeProviderStateIncompatibleError,
|
||||
NativeProviderTerminalFailure,
|
||||
NativeGovernedWaitSettlementPendingError,
|
||||
NativeSessionProtocolIntegrityError,
|
||||
} from "../../vendor/paperclip-runner/index.js";
|
||||
import * as issueServiceModule from "../issues.js";
|
||||
@@ -20768,6 +20769,19 @@ describe("runnerd provider runtime wiring", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("reconciles a saved governed wait with a bounded same-run settlement schedule", () => {
|
||||
const now = new Date("2026-11-09T12:00:00Z");
|
||||
const code = nativeSessionFailureSourceCode(new NativeGovernedWaitSettlementPendingError());
|
||||
expect(code).toBe("native_governed_wait_settlement_pending");
|
||||
expect(nativeSessionFailureDisposition(1, now, code)).toEqual({
|
||||
phase: "retryable_failure", failureCode: code, nextAttemptAt: new Date(now.getTime() + 30_000),
|
||||
});
|
||||
expect(nativeSessionFailureDisposition(3, now, code)).toEqual({
|
||||
phase: "terminal_failure", failureCode: code, nextAttemptAt: null,
|
||||
});
|
||||
expect(requiresExecutionReconciliation(code)).toBe(true);
|
||||
});
|
||||
|
||||
// Terminal wrapping must never turn an authorization/integrity failure into a safe replacement.
|
||||
it.each([
|
||||
"tool_binding_mismatch",
|
||||
|
||||
@@ -175,6 +175,7 @@ import {
|
||||
NativeProviderSideEffectPendingError,
|
||||
NativeProviderStateIncompatibleError,
|
||||
NativeProviderTerminalFailure,
|
||||
NativeGovernedWaitSettlementPendingError,
|
||||
NativeRunnerSourceReconciliationRequiredError,
|
||||
NativeSessionCleanupQuarantinedError,
|
||||
NativeSessionCloseUnrecoverableError,
|
||||
@@ -8001,7 +8002,7 @@ export function nativeSessionFailureDisposition(
|
||||
phase: exhausted
|
||||
? ("terminal_failure" as const)
|
||||
: ("retryable_failure" as const),
|
||||
failureCode: permanentFailure || permissionDeclined
|
||||
failureCode: permanentFailure || permissionDeclined || sourceFailureCode === "native_governed_wait_settlement_pending"
|
||||
? sourceFailureCode!
|
||||
: exhausted
|
||||
? ("native_session_retry_exhausted" as const)
|
||||
@@ -8040,6 +8041,7 @@ export function nativeSessionFailureSourceCode(
|
||||
):
|
||||
| "native_permission_declined"
|
||||
| "native_provider_terminal_failed"
|
||||
| "native_governed_wait_settlement_pending"
|
||||
| typeof NATIVE_PROVIDER_STATE_INCOMPATIBLE
|
||||
| typeof NATIVE_PROVIDER_SIDE_EFFECT_PENDING
|
||||
| "native_provider_approval_required"
|
||||
@@ -8072,6 +8074,7 @@ export function nativeSessionFailureSourceCode(
|
||||
| "local_runtime_api_listener_unavailable"
|
||||
| "native_session_interrupted" {
|
||||
if (error instanceof NativePermissionDeclinedError) return "native_permission_declined";
|
||||
if (error instanceof NativeGovernedWaitSettlementPendingError) return "native_governed_wait_settlement_pending";
|
||||
if (error instanceof NativeProviderStateIncompatibleError) return NATIVE_PROVIDER_STATE_INCOMPATIBLE;
|
||||
if (error instanceof NativeProviderSideEffectPendingError) return NATIVE_PROVIDER_SIDE_EFFECT_PENDING;
|
||||
if (error instanceof NativeProviderTerminalFailure) {
|
||||
@@ -11897,6 +11900,8 @@ async function executePaperclipNativeSessionAttempt(
|
||||
? "The computer provider rejected access. Restore the environment's credentials or permissions before retrying. Automatic retries are stopped."
|
||||
: sourceFailureCode === "native_provider_terminal_failed"
|
||||
? "The provider session is permanently unusable. Verify stopped execution, completed actions, and task context before starting a linked continuation."
|
||||
: sourceFailureCode === "native_governed_wait_settlement_pending"
|
||||
? "The work and waiting state are saved. Confirm shutdown of the retained execution and settle that same handoff without repeating provider work."
|
||||
: sourceFailureCode === "native_provider_model_rejected" && !recoveryEvidence.providerSessionEstablished
|
||||
? NATIVE_STARTUP_MODEL_REJECTED_ACTION
|
||||
: sourceFailureCode === NATIVE_PROVIDER_STATE_INCOMPATIBLE
|
||||
|
||||
@@ -327,7 +327,7 @@ describe("PaperclipControlPlanePort conformance", () => {
|
||||
await rm(outputStorageRoot, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
it.each(["settles", "checkpoint timeout", "unscoped terminal"] as const)("retains a durably recorded governed terminal while its postcommit observer is pending: %s", async mode => {
|
||||
it.each(["settles", "checkpoint timeout", "unscoped terminal", "verified shutdown"] as const)("settles a durable governed wait through its recorded terminal or shutdown: %s", async mode => {
|
||||
const identity = { ...CONTROL_PLANE_CONFORMANCE_OPEN.identity, issueId: randomUUID(), runId: randomUUID(), sessionId: randomUUID() };
|
||||
const runnerId = randomUUID();
|
||||
const fixtureContractId = randomUUID();
|
||||
@@ -383,9 +383,11 @@ describe("PaperclipControlPlanePort conformance", () => {
|
||||
async capabilities() { return { resume: false, typedEvents: true, steering: false, interruption: true, structuredResult: true }; },
|
||||
async startTurn() { starts++; return { turnId: "park-turn" }; },
|
||||
cancel() { return { cleanup: Promise.resolve() }; },
|
||||
async *events() { yield event(1, "item.completed", { kind: "dynamicToolCall" }); terminalEmitted = true; yield event(2, "turn.interrupted", { status: "interrupted", error: null }); },
|
||||
async settleCancelledTurn() { return { runId: identity.runId, normalizedSessionId: identity.sessionId, turnId: "park-turn" }; },
|
||||
async *events() { yield event(1, "item.completed", { kind: "dynamicToolCall" }); terminalEmitted = true;
|
||||
if (mode !== "verified shutdown") yield event(2, "turn.interrupted", { status: "interrupted", error: null }); },
|
||||
async snapshot() { return { backendKind: "mock", sessionId: identity.sessionId, identity,
|
||||
activeTurnId: terminalEmitted ? null : "park-turn", cursor: terminalEmitted ? "2" : "1" }; },
|
||||
activeTurnId: terminalEmitted ? null : "park-turn", cursor: terminalEmitted && mode !== "verified shutdown" ? "2" : "1" }; },
|
||||
async result() { return null; }, async close() {},
|
||||
}; },
|
||||
};
|
||||
@@ -409,6 +411,19 @@ describe("PaperclipControlPlanePort conformance", () => {
|
||||
const execution = executeNativeSession(options);
|
||||
const outcome = execution.then(value => ({ value }), error => ({ error }));
|
||||
try {
|
||||
if (mode === "verified shutdown") {
|
||||
expect(await outcome).toMatchObject({ value: { result: yielded, terminal: { runTerminalState: "succeeded" } } });
|
||||
expect(starts).toBe(1);
|
||||
expect(await db.select().from(nativeRunResults).where(eq(nativeRunResults.runId, identity.runId))).toHaveLength(1);
|
||||
expect((await port.loadSessionCheckpoint())?.governedWait?.settlement).toEqual({
|
||||
runId: identity.runId, normalizedSessionId: identity.sessionId, turnId: "park-turn",
|
||||
});
|
||||
expect((await db.select().from(heartbeatRunEvents).where(eq(heartbeatRunEvents.runId, identity.runId)))
|
||||
.some(row => row.eventType.startsWith("turn."))).toBe(false);
|
||||
expect((await db.select().from(issueThreadInteractions).where(eq(issueThreadInteractions.id, interaction.id)))[0])
|
||||
.toMatchObject({ status: "pending", result: null, effectiveResolverPolicy: "human_only", sourceRunId: identity.runId });
|
||||
return;
|
||||
}
|
||||
await Promise.race([recorded, outcome.then(result => { if ("error" in result) throw result.error; throw new Error("run settled before recording terminal"); })]);
|
||||
const rows = await db.select().from(heartbeatRunEvents).where(eq(heartbeatRunEvents.runId, identity.runId));
|
||||
expect(rows.some(row => row.eventType === "turn.interrupted")).toBe(true);
|
||||
|
||||
Reference in new issue
Block a user