mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-08 21:35:36 +01:00
fix(native): let a Stop before the provider session opens continue once (#19206)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - A remote task restores its saved workspace into a sandbox before the agent starts. > - When that restore fails, the run records only `remote_workspace_operation_failed: operation=restore exit=1`. > - The run then fails as `setup_failed`. No retry starts, and a `blocked` task gets no recovery action. > - An operator cannot tell which step failed, and nobody owns the next step. > - This pull request makes the sandbox scripts name their failed step in a closed grammar, and it gives a restore failure a bounded retry or a named board recovery action. > - The benefit is a visible cause and a clear owner. Saved work stays protected. ## Linked Issues or Issue Description Refs #19152 and #19187. Those changes name copy-back (save) failures. Refs #18852 and #19195. Those changes added the bounded bootstrap retry that this change reuses. **What happened?** A board reply woke an agent on a `blocked` parent task. The run failed during setup 13 seconds later. The only run event was `remote_workspace_operation_failed: operation=restore exit=1`. The run had no workspace operation row. The task stayed `blocked`, and its recovery actions showed `active: null`. The same opaque family also appeared for workspace preparation (`exit=1` only). **Expected behavior** The run names which restore step failed, without command output or paths. A failure before provider work uses a bounded retry when a retry can help. Otherwise, and when the retry budget is spent, the board gets a named recovery action. The saved version is never changed or overwritten. **Steps to reproduce** 1. Save a remote workspace, then start a new run on a different allocation. 2. Put a folder at the published workspace path in the sandbox before restore. 3. Observe that the base code records only `operation=restore exit=1`, then `setup_failed`, with no recovery action for a `blocked` task. **Paperclip version or commit** Base: `87dfba6818`. **Deployment mode** Legacy runtime with sandbox (Daytona) remote workspace storage. ## What Changed - The prepare, repository, and restore sandbox scripts track their current stage. On failure they print `remote_workspace_<op>_failed: stage=<stage>` plus closed fields: `reason=` (the script's own snake_case refusal), `archive=` (the archive helper's fixed reason), `code=` (errno), `disk=full`, `git=` (a fixed label chosen from Git output), and the existing `exit`/`signal`/`program` fields. - The server accepts that text only when it matches a strict grammar for the same operation. Other output is dropped, as before. - Provider refusals during the restore upload and restore command are recorded by closed labels, like copy-back. - A failed saved-workspace restore raises `RemoteWorkspaceRestoreError` with the server step (`snapshot_lookup`, `snapshot_download`, `sandbox_restore`). It is retryable only when the owner re-reads the same writer, epoch, and saved version with no unsaved work, and the sandbox did not name a refusal reason. - Setup failure handling classifies a pre-dispatch legacy restore failure as `remote_workspace_restore_failed`. A retryable failure uses the existing bounded bootstrap retry. A non-retryable or exhausted failure records a board-owned recovery action with the redacted cause, when stranded-work recovery does not scan the task status (for example `blocked`). - Stranded-work recovery gets a named notice for this code. Generic continuation recovery does not grant it a second budget, and legacy reconciliation does not invent provider actions for it. - `doc/execution-semantics.md` documents the contract. ## Verification - Red: the new transport tests failed on the base code with `remote_workspace_operation_failed: operation=restore exit=1` (and the same for prepare and repository). The new completion tests failed because the run became `setup_failed` with no retry and no recovery action. - Green: `npx vitest run src/services/remote-workspace-transport.test.ts src/services/heartbeat/run-completion.test.ts src/services/remote-workspaces.integration.test.ts src/__tests__/heartbeat-retry-scheduling.test.ts src/services/legacy-execution-recovery.test.ts src/services/recovery src/services/remote-workspace-save-concurrency.test.ts src/services/native-runtime/native-workspace-finalizer-failure.test.ts src/__tests__/native-workspace-finalization-work.test.ts` passes (563 tests). - The transport tests use real sandbox commands. They prove an occupied folder reports `stage=inspect_root reason=occupied_root` and keeps its files. They prove a wrong manifest reports `stage=extract archive=manifest_mismatch`. They prove a missing branch reports `stage=checkout_branch`, and a changed origin reports `stage=verify_origin`. Forged output with a path is dropped. - The real-PostgreSQL cold-restore test proves an occupied root gives a non-retryable `sandbox_restore` failure and keeps the unknown files. - The completion tests cover retried, exhausted, refused, post-dispatch, and `in_progress` cases. - `tsc --noEmit` for the server reports no errors in the changed files. ## Risks - A retry keeps the same restore path. If the cause is deterministic but unnamed, the run uses its two retries before the board action. Named refusals skip the retries. - The board action is recorded only for statuses that stranded-work recovery does not scan, and only when no other action is active. This prevents a second recovery identity for one failure. - Script stderr grammar is new. Old sandboxes run the script that the server sends, so no runner update is necessary. - No migration, deployment, or credential change. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, through Claude Code with tool use, code execution, and browser tools for read-only staging evidence. Context window and reasoning mode are host-managed. ## 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 <noreply@anthropic.com>
This commit is contained in:
1 parent
833d068741
commit
beb73381ca
6 files changed
+322
-13
No files matched your search
@@ -353,6 +353,10 @@ type NativeSessionStartup = {
|
||||
executionLeaseRenewal?: NativeExecutionLeaseRenewal;
|
||||
/** A fresh claim whose preparation failed before any provider turn. */
|
||||
unstartedClaim?: { leaseOwner: string; attempt: number };
|
||||
/** A provider session handle was published; a turn may have followed it. */
|
||||
sessionPublished?: boolean;
|
||||
/** Restart recovery reattaches a provider that may already have worked. */
|
||||
providerWorkMayPrecede?: boolean;
|
||||
executionContext?: NativeExecutionContext;
|
||||
settled: Promise<void>;
|
||||
settle: () => void;
|
||||
@@ -7739,6 +7743,8 @@ export function cancelNativeSession(
|
||||
scope?: "turn" | "run" | "issue";
|
||||
replacementAccepted?: boolean;
|
||||
cancellationRequestId?: string;
|
||||
/** Internal: the startup owner fenced this Stop before any provider turn. */
|
||||
beforeProviderTurn?: boolean;
|
||||
},
|
||||
): Promise<{
|
||||
dispatched: boolean;
|
||||
@@ -7754,6 +7760,8 @@ export async function cancelNativeSession(
|
||||
scope?: "turn" | "run" | "issue";
|
||||
replacementAccepted?: boolean;
|
||||
cancellationRequestId?: string;
|
||||
/** Internal: the startup owner fenced this Stop before any provider turn. */
|
||||
beforeProviderTurn?: boolean;
|
||||
},
|
||||
): Promise<
|
||||
| boolean
|
||||
@@ -7779,7 +7787,8 @@ export async function cancelNativeSession(
|
||||
async function performNativeCancellation(
|
||||
runId: string,
|
||||
reason: string,
|
||||
options?: { db: Db; scope?: "turn" | "run" | "issue"; replacementAccepted?: boolean; cancellationRequestId?: string },
|
||||
options?: { db: Db; scope?: "turn" | "run" | "issue"; replacementAccepted?: boolean; cancellationRequestId?: string;
|
||||
beforeProviderTurn?: boolean },
|
||||
) {
|
||||
const runStop = (options?.scope ?? "run") === "run";
|
||||
let decision: NativeStatusDecision | null = null;
|
||||
@@ -8098,6 +8107,11 @@ async function performNativeCancellation(
|
||||
const startup = runStop && !activeNativeSessions.has(runId) ? nativeSessionStartups.get(runId) : undefined;
|
||||
let settleStartupCancellation: (() => void) | undefined;
|
||||
let settleActiveCancellation: (() => void) | undefined;
|
||||
// Proof for the acknowledgement: this Stop reached startup before it
|
||||
// published a provider handle. Publication then fences that handle, so no
|
||||
// provider turn can follow. A reattached provider may already have worked.
|
||||
const beforeProviderTurn = runStop && (options?.beforeProviderTurn === true || Boolean(startup
|
||||
&& startup.sessionPublished !== true && startup.providerWorkMayPrecede !== true));
|
||||
if (startup) {
|
||||
startup.stopRequested = true;
|
||||
startup.executionContext?.abort(new Error("native_session_cancelled"));
|
||||
@@ -8298,6 +8312,7 @@ async function performNativeCancellation(
|
||||
decisionId,
|
||||
acknowledgementAuditId,
|
||||
acknowledgedAt: new Date().toISOString(),
|
||||
...(beforeProviderTurn ? { beforeProviderTurn: true } : {}),
|
||||
},
|
||||
},
|
||||
updatedAt: new Date(),
|
||||
@@ -8479,6 +8494,7 @@ export async function executePaperclipNativeSession(input: {
|
||||
let settle!: () => void;
|
||||
const startup: NativeSessionStartup = {
|
||||
priorRestartDetach: nativeRunsDetachingForRestart.get(runId),
|
||||
providerWorkMayPrecede: Boolean(input.restartRecovery),
|
||||
settled: new Promise<void>(resolve => { settle = resolve; }),
|
||||
settle: () => settle(),
|
||||
promise: new Promise<ActiveNativeSession | null>(resolve => { resolveStartup = resolve; }),
|
||||
@@ -10340,6 +10356,9 @@ async function executePaperclipNativeSessionWithinScope(
|
||||
const active: ActiveNativeSession = { session, cancelRequested: false };
|
||||
activeNativeSessions.set(input.execution.binding.runId, active);
|
||||
const startup = nativeSessionStartups.get(input.execution.binding.runId);
|
||||
const stopBeforeProviderTurn = startup?.stopRequested === true && startup.sessionPublished !== true
|
||||
&& startup.providerWorkMayPrecede !== true;
|
||||
if (startup) startup.sessionPublished = true;
|
||||
startup?.resolve(active);
|
||||
if (startup?.stopRequested) {
|
||||
// Publication wakes the Stop caller; wait for its durable ACK
|
||||
@@ -10358,7 +10377,8 @@ async function executePaperclipNativeSessionWithinScope(
|
||||
)).limit(1);
|
||||
const pendingStop = record(record(stopRun?.resultJson).nativeCancellation);
|
||||
if (pendingStop.scope === "run" && pendingStop.dispatchState === "pending") {
|
||||
await cancelNativeSession(input.execution.binding.runId, "Stop requested during native startup", { db: input.db, scope: "run" });
|
||||
await cancelNativeSession(input.execution.binding.runId, "Stop requested during native startup",
|
||||
{ db: input.db, scope: "run", beforeProviderTurn: stopBeforeProviderTurn });
|
||||
}
|
||||
} catch (error) {
|
||||
// Best effort: the in-memory Stop below still fences this
|
||||
|
||||
Reference in new issue
Block a user