diff --git a/doc/sandbox-work-folders.md b/doc/sandbox-work-folders.md index ca74021cc2..439d70815a 100644 --- a/doc/sandbox-work-folders.md +++ b/doc/sandbox-work-folders.md @@ -60,6 +60,13 @@ conflicting identity records, configuration drift, or a failed resume retain the old sandbox and report a recovery error instead of destroying its only copy. The provider cannot opt a new task into this compatibility mode. +Some older releases retained a sandbox without recording a workspace binding on +the task. When that binding and any explicit workspace preference are absent, +startup recovers the retained workspace from the matching task, project, agent, +responsible user, and sandbox environment. Local execution, explicit workspace +choices, and tasks that have entered scoped persistence do not use this fallback. +The normal workspace freshness and provider identity checks still apply. + Acceptance must resume representative pre-upgrade legacy and native tasks with committed, staged, unstaged, and untracked work, verify their original paths and usable continuation, and exercise their existing restore mechanism after a diff --git a/scripts/paperclip-issue-update.sh b/scripts/paperclip-issue-update.sh index f8717e31c0..4212431963 100755 --- a/scripts/paperclip-issue-update.sh +++ b/scripts/paperclip-issue-update.sh @@ -1,166 +1,5 @@ #!/usr/bin/env bash - +# Keep repository callers working; the installed runtime skill owns this helper. set -euo pipefail - -usage() { - cat <<'EOF' -Usage: - scripts/paperclip-issue-update.sh [--issue-id ID] [--status STATUS] [--comment TEXT] [--dry-run] - -Reads a multiline markdown comment from stdin when stdin is piped. This preserves -newlines when building the JSON payload for PATCH /api/issues/{issueId}. - -Examples: - scripts/paperclip-issue-update.sh --issue-id "$PAPERCLIP_TASK_ID" --status in_progress <<'MD' - Investigating formatting - - - Pulled the raw comment body - - Comparing it with the run transcript - MD - - scripts/paperclip-issue-update.sh --issue-id "$PAPERCLIP_TASK_ID" --status done --dry-run <<'MD' - Done - - - Fixed the issue update helper - MD -EOF -} - -require_command() { - if ! command -v "$1" >/dev/null 2>&1; then - printf 'Missing required command: %s\n' "$1" >&2 - exit 1 - fi -} - -issue_id="${PAPERCLIP_TASK_ID:-}" -status="" -comment_arg="" -dry_run=0 - -while [[ $# -gt 0 ]]; do - case "$1" in - --issue-id) - issue_id="${2:-}" - shift 2 - ;; - --status) - status="${2:-}" - shift 2 - ;; - --comment) - comment_arg="${2:-}" - shift 2 - ;; - --dry-run) - dry_run=1 - shift - ;; - --help|-h) - usage - exit 0 - ;; - *) - printf 'Unknown argument: %s\n' "$1" >&2 - usage >&2 - exit 1 - ;; - esac -done - -if [[ -z "$issue_id" ]]; then - printf 'Missing issue id. Pass --issue-id or set PAPERCLIP_TASK_ID.\n' >&2 - exit 1 -fi - -comment="" -if [[ -n "$comment_arg" ]]; then - comment="$comment_arg" -elif [[ ! -t 0 ]]; then - comment="$(cat)" -fi - -require_command jq - -payload="$( - jq -nc \ - --arg status "$status" \ - --arg comment "$comment" \ - ' - (if $status == "" then {} else {status: $status} end) + - (if $comment == "" then {} else {comment: $comment} end) - ' -)" - -if [[ "$dry_run" == "1" ]]; then - printf '%s\n' "$payload" - exit 0 -fi - -if [[ -z "${PAPERCLIP_API_URL:-}" || -z "${PAPERCLIP_API_KEY:-}" || -z "${PAPERCLIP_RUN_ID:-}" ]]; then - printf 'Missing PAPERCLIP_API_URL, PAPERCLIP_API_KEY, or PAPERCLIP_RUN_ID.\n' >&2 - exit 1 -fi - -# A successful PATCH always returns the updated issue JSON. An empty body or a -# connection-level failure means the write did NOT land, even when a pipeline -# exit code says otherwise, so verify the response instead of inferring success. -# Two attempts total: the shared heartbeat policy stops a control-plane write -# after two consecutive failures, so the helper must not send a third. -max_attempts=2 -attempt=1 -while :; do - http_code="" - body="" - set +e - response="$( - curl -sS -m 30 -X PATCH \ - "$PAPERCLIP_API_URL/api/issues/$issue_id" \ - -H "Authorization: Bearer $PAPERCLIP_API_KEY" \ - -H "X-Paperclip-Run-Id: $PAPERCLIP_RUN_ID" \ - -H 'Content-Type: application/json' \ - --data-binary "$payload" \ - -w '\n%{http_code}' - )" - curl_exit=$? - set -e - - if [[ "$curl_exit" -eq 0 ]]; then - http_code="${response##*$'\n'}" - body="${response%$'\n'*}" - fi - - if [[ "$curl_exit" -eq 0 && "$http_code" == 2* ]]; then - if [[ -z "$body" ]]; then - printf 'Issue update FAILED: HTTP %s with an empty response body. A real update echoes the issue JSON; treat this write as not saved.\n' "$http_code" >&2 - exit 1 - fi - if [[ -n "$status" ]]; then - returned_status="$(jq -r '.status // empty' <<<"$body" 2>/dev/null || true)" - if [[ "$returned_status" != "$status" ]]; then - printf 'Issue update FAILED: server echoed status %s instead of requested %s.\n' "${returned_status:-}" "$status" >&2 - printf '%s\n' "$body" >&2 - exit 1 - fi - fi - printf '%s\n' "$body" - exit 0 - fi - - # 4xx (other than 429) is a definitive rejection; retrying cannot change it. - if [[ "$curl_exit" -eq 0 && "$http_code" == 4* && "$http_code" != "429" ]]; then - printf 'Issue update rejected (HTTP %s).\n' "$http_code" >&2 - [[ -n "$body" ]] && printf '%s\n' "$body" >&2 - exit 1 - fi - - if (( attempt >= max_attempts )); then - printf 'Issue update FAILED after %d attempts (curl exit %s, HTTP %s). The status/comment was NOT saved — report this write as failed, do not assume it landed.\n' "$max_attempts" "$curl_exit" "${http_code:-000}" >&2 - [[ -n "$body" ]] && printf '%s\n' "$body" >&2 - exit 1 - fi - - printf 'Issue update attempt %d/%d failed (curl exit %s, HTTP %s); retrying...\n' "$attempt" "$max_attempts" "$curl_exit" "${http_code:-000}" >&2 - sleep $((attempt * 2)) - attempt=$((attempt + 1)) -done +script_dir="$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")" && pwd)" +exec bash "$script_dir/../skills/paperclip/scripts/paperclip-issue-update.sh" "$@" diff --git a/server/src/__tests__/paperclip-issue-update-helper.test.ts b/server/src/__tests__/paperclip-issue-update-helper.test.ts index 59a81de3f6..43dea763e0 100644 --- a/server/src/__tests__/paperclip-issue-update-helper.test.ts +++ b/server/src/__tests__/paperclip-issue-update-helper.test.ts @@ -2,14 +2,16 @@ import { spawn } from "node:child_process"; import http from "node:http"; import type { AddressInfo } from "node:net"; import path from "node:path"; +import fs from "node:fs/promises"; +import os from "node:os"; import { afterEach, describe, expect, it } from "vitest"; -// End-to-end coverage for scripts/paperclip-issue-update.sh: the helper must +// End-to-end coverage for the installed runtime skill helper: the helper must // only exit 0 when the server confirms the write by echoing the update, must // classify failures (retry connection-level faults and 5xx, never retry a // definitive 4xx), and must stop at two attempts total to honor the shared // bounded-write-retry rule. -const HELPER_PATH = path.resolve("scripts/paperclip-issue-update.sh"); +const HELPER_PATH = path.resolve("skills/paperclip/scripts/paperclip-issue-update.sh"); interface HelperResult { code: number | null; @@ -66,9 +68,10 @@ describe("paperclip issue update helper", () => { return { baseUrl: `http://127.0.0.1:${port}`, requests }; } - function runHelper(apiUrl: string, args: string[]): Promise { + function runHelper(apiUrl: string, args: string[], helperPath = HELPER_PATH, cwd?: string): Promise { return new Promise((resolve, reject) => { - const child = spawn("bash", [HELPER_PATH, ...args], { + const child = spawn("bash", [helperPath, ...args], { + cwd, env: { ...process.env, PAPERCLIP_API_URL: apiUrl, @@ -109,6 +112,25 @@ describe("paperclip issue update helper", () => { expect(JSON.parse(requests[0]?.body ?? "{}")).toEqual({ status: "done", comment: "closing note" }); }); + it("works after installing only the skill into a sandbox home", async () => { + const home = await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-installed-skill-")); + cleanupFns.push(() => fs.rm(home, { recursive: true, force: true })); + const skillDir = path.join(home, ".codex", "skills", "paperclip"); + await fs.cp(path.resolve("skills/paperclip"), skillDir, { recursive: true }); + const { baseUrl, requests } = await startServer((_request, _attempt, res) => { + res.writeHead(200, { "content-type": "application/json" }); + res.end(JSON.stringify({ id: "issue-1", status: "done" })); + }); + const result = await runHelper(baseUrl, doneArgs, path.join(skillDir, "scripts", "paperclip-issue-update.sh"), home); + expect(result.code).toBe(0); + expect(JSON.parse(result.stdout)).toMatchObject({ id: "issue-1", status: "done" }); + expect(requests).toHaveLength(1); + await expect(fs.access(path.join(home, "scripts"))).rejects.toThrow(); + const repositoryWrapper = await runHelper(baseUrl, doneArgs, path.resolve("scripts/paperclip-issue-update.sh"), home); + expect(repositoryWrapper.code).toBe(0); + expect(requests).toHaveLength(2); + }); + it("fails an empty 2xx body instead of treating it as success", async () => { const { baseUrl } = await startServer((_request, _attempt, res) => { res.writeHead(200, { "content-length": "0" }); diff --git a/server/src/__tests__/plugin-worker-manager.test.ts b/server/src/__tests__/plugin-worker-manager.test.ts index 03035ff2dc..420c2080e8 100644 --- a/server/src/__tests__/plugin-worker-manager.test.ts +++ b/server/src/__tests__/plugin-worker-manager.test.ts @@ -249,6 +249,10 @@ describe("plugin-worker-manager stderr failure context", () => { await expect(pendingCall).rejects.toBeInstanceOf(JsonRpcCallError); await expect(pendingCall).rejects.toMatchObject({ message: expect.stringContaining("terminated"), + cause: expect.objectContaining({ + message: "Sandbox command requested here", + stack: expect.stringContaining("plugin-worker-manager.test.ts"), + }), }); expect(unhandledRejection).not.toHaveBeenCalled(); } finally { diff --git a/server/src/__tests__/sandbox-work-folders.test.ts b/server/src/__tests__/sandbox-work-folders.test.ts index 15393464b8..d583327f10 100644 --- a/server/src/__tests__/sandbox-work-folders.test.ts +++ b/server/src/__tests__/sandbox-work-folders.test.ts @@ -10,6 +10,7 @@ import { agents, assets, companyMemberships, issueAttachments, companies, create import { createLocalDiskStorageProvider } from "../storage/local-disk-provider.js"; import { prepareSandboxWorkFolders } from "../services/sandbox-work-folders.js"; import { bindWarmSandboxWorkspace } from "../services/sandbox-workspace-binding.js"; +import { findUnboundLegacyTaskWorkspace } from "../services/legacy-sandbox-workspace.js"; import { retainUnsavedWorkFolderLease, workFolderSandboxKey } from "../services/work-folder-retention.js"; import * as activityLog from "../services/activity-log.js"; import { workFolderService } from "../services/work-folders.js"; @@ -51,6 +52,47 @@ describe("shared sandbox work-folder lifecycle", () => { for (const run of active) await run.stop().catch(() => {}); await database?.cleanup(); if (root) await fs.rm(root, { recursive: true, force: true }); }); + it.each(["codex_local", "paperclip_runner"])("recovers an unbound pre-change %s workspace only for its recorded identity", async (adapterType) => { + const task = randomUUID(), runId = randomUUID(), workspaceId = randomUUID(), leaseId = randomUUID(); + await db.insert(heartbeatRuns).values({ id: runId, companyId, agentId, responsibleUserId: "owner", + status: "succeeded", contextSnapshot: { issueId: task } }); + await db.insert(issues).values({ id: task, companyId, projectId, title: "Unbound old task", assigneeAgentId: agentId }); + await db.insert(executionWorkspaces).values({ id: workspaceId, companyId, projectId, sourceIssueId: task, + mode: "shared_workspace", strategyType: "project_primary", name: "Retained old workspace" }); + const metadata = { driver: "sandbox", agentId, reusableSandboxLease: { version: 1, companyId, agentId, + environmentId, executionWorkspaceId: workspaceId, adapterType, provider: "daytona" } }; + await db.insert(environmentLeases).values({ id: leaseId, companyId, environmentId, issueId: task, + executionWorkspaceId: workspaceId, heartbeatRunId: runId, status: "retained", leasePolicy: "reuse_by_environment", + provider: "daytona", providerLeaseId: "original-sandbox", metadata }); + const input = { companyId, issueId: task, projectId, agentId, responsibleUserId: "owner", adapterType, + executionWorkspaceId: null, executionWorkspacePreference: null, + environment: { id: environmentId, driver: "sandbox" as const, config: { reuseLease: true } } }; + expect(await findUnboundLegacyTaskWorkspace(db, input)).toBe(workspaceId); + for (const bad of [{ companyId: randomUUID() }, { issueId: randomUUID() }, { projectId: randomUUID() }, + { agentId: randomUUID() }, { responsibleUserId: null }, { responsibleUserId: "another-user" }, + { adapterType: "other-adapter" }, { executionWorkspaceId: randomUUID() }, + { executionWorkspacePreference: "create_new" }, { environment: null }, + { environment: { ...input.environment, id: randomUUID() } }, + { environment: { ...input.environment, driver: "local" as const } }, + { environment: { ...input.environment, config: { reuseLease: false } } }]) { + expect(await findUnboundLegacyTaskWorkspace(db, { ...input, ...bad })).toBeNull(); + } + await db.update(heartbeatRuns).set({ contextSnapshot: { issueId: randomUUID() } }).where(eq(heartbeatRuns.id, runId)); + expect(await findUnboundLegacyTaskWorkspace(db, input)).toBeNull(); + await db.update(heartbeatRuns).set({ contextSnapshot: { issueId: task } }).where(eq(heartbeatRuns.id, runId)); + await db.update(executionWorkspaces).set({ status: "archived" }).where(eq(executionWorkspaces.id, workspaceId)); + expect(await findUnboundLegacyTaskWorkspace(db, input)).toBeNull(); + await db.update(executionWorkspaces).set({ status: "active" }).where(eq(executionWorkspaces.id, workspaceId)); + await db.update(environmentLeases).set({ metadata: { ...metadata, + reusableSandboxLease: { ...metadata.reusableSandboxLease, executionWorkspaceId: randomUUID() } } }).where(eq(environmentLeases.id, leaseId)); + expect(await findUnboundLegacyTaskWorkspace(db, input)).toBeNull(); + await db.update(environmentLeases).set({ metadata }).where(eq(environmentLeases.id, leaseId)); + expect(await findUnboundLegacyTaskWorkspace(db, input)).toBe(workspaceId); + await db.insert(workFolderRuns).values({ runId, companyId, manifest: { version: 1, companyId, runId, + taskId: task, projectId, agentId, responsibleUserId: "owner", leaseId, sandboxKey: leaseId, + home: "/home/daytona", folders: { task: null, agent: null, user: null, project: null }, repositories: [] } }); + expect(await findUnboundLegacyTaskWorkspace(db, input)).toBeNull(); + }); it("keeps the host's warm task binding without enabling user-configurable worktrees", async () => { const task = randomUUID(), runId = randomUUID(), workspaceId = randomUUID(); await db.insert(heartbeatRuns).values({ id: runId, companyId, agentId, status: "running" }); diff --git a/server/src/__tests__/work-folder-checkpointer.test.ts b/server/src/__tests__/work-folder-checkpointer.test.ts index 3d51434310..fb6f9e51a4 100644 --- a/server/src/__tests__/work-folder-checkpointer.test.ts +++ b/server/src/__tests__/work-folder-checkpointer.test.ts @@ -31,4 +31,17 @@ describe("shared work folder checkpoint cadence", () => { expect(onError).toHaveBeenCalledWith(error); await expect(sync.stop()).rejects.toThrow("Storage unavailable"); }); + it("does not restart a failed final save during error teardown", async () => { + vi.useFakeTimers(); + const checkpoint = vi.fn().mockRejectedValueOnce(new Error("socket hang up")).mockResolvedValue(undefined); + const onError = vi.fn().mockResolvedValue(undefined); + const sync = startWorkFolderCheckpointer({ checkpoint, onError }); + const firstStop = sync.stop(); + await expect(firstStop).rejects.toThrow("socket hang up"); + expect(sync.stop()).toBe(firstStop); + await expect(sync.stop()).rejects.toThrow("socket hang up"); + await vi.advanceTimersByTimeAsync(360_000); + expect(checkpoint).toHaveBeenCalledTimes(1); + expect(onError).toHaveBeenCalledTimes(1); + }); }); diff --git a/server/src/__tests__/work-folder-transport.test.ts b/server/src/__tests__/work-folder-transport.test.ts index abdf9a388b..8f80a9f6d1 100644 --- a/server/src/__tests__/work-folder-transport.test.ts +++ b/server/src/__tests__/work-folder-transport.test.ts @@ -1,4 +1,4 @@ -import { afterEach, describe, expect, it } from "vitest"; +import { afterEach, describe, expect, it, vi } from "vitest"; import { execFile } from "node:child_process"; import { promisify } from "node:util"; import { mkdtemp, mkdir, readFile, realpath, rm, symlink, writeFile } from "node:fs/promises"; @@ -18,6 +18,39 @@ describe("sandbox work folder transport with real Node and Git", () => { roots.push(dir); return dir; } afterEach(async () => { for (const dir of roots.splice(0)) await rm(dir, { recursive: true, force: true }); }); + it("retries a lost read response without duplicating streamed bytes", async () => { + const dir = await root(); + const body = Buffer.alloc(700_000, "x"); + await writeFile(path.join(dir, "file"), body); + let lostResponse = false; + const execute = vi.fn(async (input: Parameters[0]) => { + const result = await localTestWorkFolderRunner.execute(input); + const request = JSON.parse(Buffer.from(input.args!.at(-1)!, "base64").toString()); + if (request.operation === "read" && request.offset > 0 && !lostResponse) { + lostResponse = true; + throw Object.assign(new Error("socket hang up"), { code: "ECONNRESET" }); + } + return result; + }); + const chunks = []; + for await (const chunk of workFolderTransport({ execute }).read(dir, "file", body.length)) chunks.push(chunk); + expect(lostResponse).toBe(true); + expect(Buffer.concat(chunks)).toEqual(body); + const offsets = execute.mock.calls.map(([input]) => JSON.parse(Buffer.from(input.args!.at(-1)!, "base64").toString()).offset); + expect(offsets.filter((offset) => offset === offsets[1])).toHaveLength(2); + }); + it("bounds read retries and does not replay mutations or validation failures", async () => { + const execute = vi.fn().mockRejectedValue(new Error("socket hang up")); + const retrying = workFolderTransport({ execute }); + await expect(retrying.scan("/home/daytona/task")).rejects.toThrow("socket hang up"); + expect(execute).toHaveBeenCalledTimes(3); + execute.mockClear(); + await expect(retrying.moveRoot("/old", "/new")).rejects.toThrow("socket hang up"); + expect(execute).toHaveBeenCalledTimes(1); + execute.mockReset().mockResolvedValue({ exitCode: 1, stdout: "", stderr: "symlink_not_allowed", timedOut: false }); + await expect(retrying.scan("/home/daytona/task")).rejects.toThrow("symlink_not_allowed"); + expect(execute).toHaveBeenCalledTimes(1); + }); it("streams and atomically publishes files larger than a transfer chunk", async () => { const dir = await root(); const staging = await root(); diff --git a/server/src/services/heartbeat.ts b/server/src/services/heartbeat.ts index c3f44643e0..a68a84427b 100644 --- a/server/src/services/heartbeat.ts +++ b/server/src/services/heartbeat.ts @@ -3,7 +3,7 @@ import { githubBrokerEnvironment } from "@paperclipai/adapter-utils/github-launc import { cleanupGitHubOperationLaunchers, prepareGitHubOperationLaunchers, startAdapterExecutionTargetPaperclipBridge } from "@paperclipai/adapter-utils/execution-target"; import fs from "node:fs/promises"; import { retainUnsavedWorkFolderLease, workFolderSandboxKey } from "./work-folder-retention.js"; -import { hasLegacySandboxWorkspace } from "./legacy-sandbox-workspace.js"; +import { findUnboundLegacyTaskWorkspace, hasLegacySandboxWorkspace } from "./legacy-sandbox-workspace.js"; import { prepareSandboxWorkFolders } from "./sandbox-work-folders.js"; import { bindWarmSandboxWorkspace } from "./sandbox-workspace-binding.js"; import path from "node:path"; @@ -18646,34 +18646,6 @@ export function heartbeatService( : null; const persistedNativeExecutionWorkspaceId = persistedNativeExecutionInput?.binding.executionWorkspaceId ?? null; - const requestedExecutionWorkspaceId = - persistedNativeExecutionWorkspaceId ?? - readNonEmptyString(issueRef?.executionWorkspaceId); - const existingExecutionWorkspace = requestedExecutionWorkspaceId - ? await executionWorkspacesSvc.getById(requestedExecutionWorkspaceId) - : null; - const nativeRecoveryExecutionWorkspaceId = - resolveNativeRecoveryExecutionWorkspaceBinding({ - bindingId: persistedNativeExecutionWorkspaceId, - persistedWorkspaceFound: existingExecutionWorkspace !== null, - }); - const workspaceReuseRequest = - resolveExecutionWorkspaceReuseRequestForIssue({ - issueExecutionWorkspaceId: requestedExecutionWorkspaceId, - issueExecutionWorkspacePreference: nativeRecoveryExecutionWorkspaceId - ? "reuse_existing" - : (issueRef?.executionWorkspacePreference ?? null), - existingExecutionWorkspaceStatus: - existingExecutionWorkspace?.status ?? null, - }); - const requestedShouldReuseExisting = - workspaceReuseRequest.requestedShouldReuseExisting; - const reusableExistingExecutionWorkspace = - workspaceReuseRequest.existingExecutionWorkspaceAvailable - ? existingExecutionWorkspace - : null; - const requestedReusableExecutionWorkspaceConfig = - reusableExistingExecutionWorkspace?.config ?? null; const localEnvironment = await environmentsSvc.ensureLocalEnvironment( agent.companyId, ); @@ -18786,6 +18758,42 @@ export function heartbeatService( : selectedEnvironmentId ? await environmentsSvc.getById(selectedEnvironmentId) : null; + const unboundLegacyWorkspaceId = persistedNativeExecutionWorkspaceId ? null + : await findUnboundLegacyTaskWorkspace(db, { + companyId: agent.companyId, issueId, projectId: issueRef?.projectId ?? null, + agentId: agent.id, responsibleUserId: run.responsibleUserId, + adapterType: agent.adapterType, environment: selectedEnvironmentForConfig, + executionWorkspaceId: readNonEmptyString(issueRef?.executionWorkspaceId), + executionWorkspacePreference: issueRef?.executionWorkspacePreference ?? null, + }); + const requestedExecutionWorkspaceId = + persistedNativeExecutionWorkspaceId ?? + readNonEmptyString(issueRef?.executionWorkspaceId) ?? unboundLegacyWorkspaceId; + const existingExecutionWorkspace = requestedExecutionWorkspaceId + ? await executionWorkspacesSvc.getById(requestedExecutionWorkspaceId) + : null; + const nativeRecoveryExecutionWorkspaceId = + resolveNativeRecoveryExecutionWorkspaceBinding({ + bindingId: persistedNativeExecutionWorkspaceId, + persistedWorkspaceFound: existingExecutionWorkspace !== null, + }); + const workspaceReuseRequest = + resolveExecutionWorkspaceReuseRequestForIssue({ + issueExecutionWorkspaceId: requestedExecutionWorkspaceId, + issueExecutionWorkspacePreference: nativeRecoveryExecutionWorkspaceId || unboundLegacyWorkspaceId + ? "reuse_existing" + : (issueRef?.executionWorkspacePreference ?? null), + existingExecutionWorkspaceStatus: + existingExecutionWorkspace?.status ?? null, + }); + const requestedShouldReuseExisting = + workspaceReuseRequest.requestedShouldReuseExisting; + const reusableExistingExecutionWorkspace = + workspaceReuseRequest.existingExecutionWorkspaceAvailable + ? existingExecutionWorkspace + : null; + const requestedReusableExecutionWorkspaceConfig = + reusableExistingExecutionWorkspace?.config ?? null; const sharedWorkspaceConcurrency = resolveSharedWorkspaceConcurrency({ projectPolicy: projectExecutionWorkspacePolicy, issueSettings: issueExecutionWorkspaceSettings, diff --git a/server/src/services/legacy-sandbox-workspace.ts b/server/src/services/legacy-sandbox-workspace.ts index 555f1be667..8c9d2c54d5 100644 --- a/server/src/services/legacy-sandbox-workspace.ts +++ b/server/src/services/legacy-sandbox-workspace.ts @@ -1,6 +1,6 @@ -import { and, eq, isNotNull, isNull, notExists, or, sql } from "drizzle-orm"; -import { environmentLeases, heartbeatRuns, workFolderRuns, type Db } from "@paperclipai/db"; -import type { EnvironmentLease } from "@paperclipai/shared"; +import { and, desc, eq, inArray, isNotNull, isNull, notExists, or, sql } from "drizzle-orm"; +import { environmentLeases, executionWorkspaces, heartbeatRuns, workFolderRuns, type Db } from "@paperclipai/db"; +import type { Environment, EnvironmentLease } from "@paperclipai/shared"; function record(value: unknown): Record | null { return value !== null && typeof value === "object" && !Array.isArray(value) @@ -12,6 +12,46 @@ export function hasLegacySandboxWorkspace(lease: Pick | null; +}) { + if (!input.issueId || !input.projectId || input.executionWorkspaceId || input.executionWorkspacePreference + || input.environment?.driver !== "sandbox" || input.environment.config.reuseLease !== true) return null; + const [candidate] = await db.select({ lease: environmentLeases }).from(environmentLeases) + .innerJoin(heartbeatRuns, and(eq(heartbeatRuns.id, environmentLeases.heartbeatRunId), + eq(heartbeatRuns.companyId, environmentLeases.companyId))) + .innerJoin(executionWorkspaces, and(eq(executionWorkspaces.id, environmentLeases.executionWorkspaceId), + eq(executionWorkspaces.companyId, environmentLeases.companyId))) + .where(and(eq(environmentLeases.companyId, input.companyId), eq(environmentLeases.issueId, input.issueId), + eq(environmentLeases.environmentId, input.environment.id), + eq(environmentLeases.leasePolicy, "reuse_by_environment"), + inArray(environmentLeases.status, ["retained", "released"]), + isNotNull(environmentLeases.providerLeaseId), + sql`${environmentLeases.metadata}->>'driver' = 'sandbox'`, + sql`${environmentLeases.metadata}->>'workFolderLayout' is distinct from 'scoped'`, + sql`${environmentLeases.metadata}->'reusableSandboxLease'->>'version' = '1'`, + eq(heartbeatRuns.agentId, input.agentId), + input.responsibleUserId === null ? isNull(heartbeatRuns.responsibleUserId) + : eq(heartbeatRuns.responsibleUserId, input.responsibleUserId), + eq(executionWorkspaces.projectId, input.projectId), eq(executionWorkspaces.status, "active"), + or(isNull(executionWorkspaces.sourceIssueId), eq(executionWorkspaces.sourceIssueId, input.issueId)), + notExists(db.select({ id: workFolderRuns.runId }).from(workFolderRuns).where(and( + eq(workFolderRuns.companyId, input.companyId), sql`${workFolderRuns.manifest}->>'taskId' = ${input.issueId}`))))) + .orderBy(desc(environmentLeases.createdAt), desc(environmentLeases.id)).limit(1); + if (!candidate) return null; + const lease = await bindLegacySandboxIdentity(db, candidate.lease as EnvironmentLease); + const scope = record(lease.metadata?.reusableSandboxLease); + if (scope?.version !== 2 || scope.issueId !== input.issueId || scope.responsibleUserId !== input.responsibleUserId + || scope.adapterType !== input.adapterType || scope.environmentId !== input.environment.id + || scope.executionWorkspaceId !== lease.executionWorkspaceId) return null; + // Normal workspace freshness and provider sentinel checks still run before use. + return lease.executionWorkspaceId; +} + /** Keep the old sync/restore contract even after its provider sandbox expires. */ export async function taskUsesLegacySandboxWorkspace(db: Db, companyId: string, issueId: string | null) { if (!issueId) return false; diff --git a/server/src/services/plugin-worker-manager.ts b/server/src/services/plugin-worker-manager.ts index bdcf10cb8b..29d778c8df 100644 --- a/server/src/services/plugin-worker-manager.ts +++ b/server/src/services/plugin-worker-manager.ts @@ -3292,6 +3292,10 @@ export function createPluginWorkerHandle( timeoutMs?: number, executeLogSink?: ExecuteLogSink, ): Promise { + // A worker response arrives on a different stack from its caller. Preserve + // command-call provenance so late sandbox failures identify their owner + // without recording command arguments, credentials, or worker payloads. + const caller = method === "environmentExecute" ? new Error("Sandbox command requested here") : undefined; const rpcPromise = new Promise((resolve, reject) => { if (!childProcess?.stdin?.writable) { reject( @@ -3347,7 +3351,9 @@ export function createPluginWorkerHandle( if (isJsonRpcSuccessResponse(response)) { settle(resolve, response.result as HostToWorkerMethods[M][1]); } else if ("error" in response && response.error) { - settle(reject, new JsonRpcCallError(response.error)); + const error = new JsonRpcCallError(response.error); + if (caller) error.cause = caller; + settle(reject, error); } else { settle(reject, new Error(`Unexpected response format for "${method}"`)); } diff --git a/server/src/services/work-folder-checkpointer.ts b/server/src/services/work-folder-checkpointer.ts index 0b32a4d87f..d1d9887d4a 100644 --- a/server/src/services/work-folder-checkpointer.ts +++ b/server/src/services/work-folder-checkpointer.ts @@ -6,6 +6,7 @@ export function startWorkFolderCheckpointer(input: { onError(error: unknown): Promise; }) { let active: Promise | null = null; + let finalFlush: Promise | null = null; let stopped = false; function checkpoint() { const pending = (async () => { @@ -33,6 +34,13 @@ export function startWorkFolderCheckpointer(input: { } return { flush, - async stop() { stopped = true; clearInterval(timer); await flush(); }, + stop() { + stopped = true; + clearInterval(timer); + // Success and error teardown can both call stop. Never begin another + // save after the caller has already terminalized this run on failure. + finalFlush ??= flush(); + return finalFlush; + }, }; } diff --git a/server/src/services/work-folder-transport.ts b/server/src/services/work-folder-transport.ts index d8529775e8..e6342e9d4e 100644 --- a/server/src/services/work-folder-transport.ts +++ b/server/src/services/work-folder-transport.ts @@ -1,6 +1,7 @@ import { readFile } from "node:fs/promises"; import { Readable } from "node:stream"; import { randomUUID } from "node:crypto"; +import { setTimeout as delay } from "node:timers/promises"; import path from "node:path"; import { z } from "zod"; import type { CommandManagedRuntimeRunner } from "@paperclipai/adapter-utils/command-managed-runtime"; @@ -15,11 +16,33 @@ let source: Promise | undefined; // Linux's 128 KiB single-argument limit, including a provider shell wrapper. const WRITE_CHUNK_BYTES = 48 * 1024; +function transientReadFailure(error: unknown) { + if (!(error instanceof Error)) return false; + const code = (error as Error & { code?: string }).code; + return ["ECONNRESET", "EPIPE", "EAI_AGAIN", "ECONNABORTED"].includes(code ?? "") + || error.message === "socket hang up"; +} + export function workFolderTransport(runner: CommandManagedRuntimeRunner) { async function command(input: Record): Promise { source ??= readFile(new URL("./scripts/work-folder-io.mjs", import.meta.url), "utf8"); - const result = await runner.execute({ command: "node", args: ["--input-type=module", "-e", await source, - Buffer.from(JSON.stringify(input)).toString("base64")], bypassSession: true, timeoutMs: 120_000 }); + const args = ["--input-type=module", "-e", await source, Buffer.from(JSON.stringify(input)).toString("base64")]; + const readOnly = ["home", "scan", "read"].includes(String(input.operation)); + const deadline = Date.now() + 120_000; + let result; + for (let attempt = 0; ; attempt++) { + try { + result = await runner.execute({ command: "node", args, bypassSession: true, + timeoutMs: Math.max(1, deadline - Date.now()) }); + break; + } catch (error) { + // A lost read response is safe to repeat. Staging writes, publishes and + // moves may already have happened, so never replay them here. + const waitMs = 250 * (attempt + 1); + if (!readOnly || attempt >= 2 || !transientReadFailure(error) || Date.now() + waitMs >= deadline) throw error; + await delay(waitMs); + } + } if (result.exitCode !== 0 || result.timedOut) throw new Error(`Work folder ${String(input.operation)} failed: ${result.stderr.slice(0, 1500)}`); return JSON.parse(result.stdout); } diff --git a/skills/paperclip/SKILL.md b/skills/paperclip/SKILL.md index 18f3457f54..e4b6813c93 100644 --- a/skills/paperclip/SKILL.md +++ b/skills/paperclip/SKILL.md @@ -112,7 +112,7 @@ For technical upload instructions, read `references/artifacts.md`. **Bounded write retry.** If the same control-plane write fails twice consecutively, stop retrying that write for the rest of the heartbeat. Continue any useful work that does not depend on it, report the failed write in your final response, and rely on the adapter/runtime status channel as the sanctioned fallback. Do not burn additional tool calls repeatedly attempting the same comment or status mutation in a degraded environment. -**Verify writes — never infer them.** A successful `PATCH /api/issues/{id}` always returns the updated issue JSON. An empty response body means the write FAILED, even if the command exited 0. Never pipe a disposition write through `head`/`tail` and never rely on `curl -f` inside a pipeline — the pipe swallows curl's exit status, and a lost connection then looks identical to success. Use `scripts/paperclip-issue-update.sh` (it checks the HTTP status, retries connection-level failures, and confirms the echoed `status`); if you must hand-roll curl, capture `-w '%{http_code}'` and check the response echoes your update. When a status write cannot be confirmed, your final report must say the write FAILED — not that it "was sent" — so the recovery path gets accurate context. +**Verify writes — never infer them.** A successful `PATCH /api/issues/{id}` always returns the updated issue JSON. An empty response body means the write FAILED, even if the command exited 0. Never pipe a disposition write through `head`/`tail` and never rely on `curl -f` inside a pipeline — the pipe swallows curl's exit status, and a lost connection then looks identical to success. Use this skill's bundled `scripts/paperclip-issue-update.sh`, resolved relative to the directory containing this `SKILL.md`, not your task working directory (it checks the HTTP status, retries connection-level failures, and confirms the echoed `status`); if you must hand-roll curl, capture `-w '%{http_code}'` and check the response echoes your update. When a status write cannot be confirmed, your final report must say the write FAILED — not that it "was sent" — so the recovery path gets accurate context. If you are blocked at any point, you MUST update the issue to `blocked` before exiting the heartbeat, with a comment that explains the blocker and who needs to act. @@ -132,10 +132,10 @@ Headers: X-Paperclip-Run-Id: $PAPERCLIP_RUN_ID { "status": "done", "comment": "What was done and why." } ``` -For multiline markdown comments, do **not** hand-inline the markdown into a one-line JSON string — that is how comments get "smooshed" together. Use the helper below (or an equivalent `jq --arg` pattern reading from a heredoc/file) so literal newlines survive JSON encoding: +For multiline markdown comments, do **not** hand-inline the markdown into a one-line JSON string — that is how comments get "smooshed" together. Use the helper below (or an equivalent `jq --arg` pattern reading from a heredoc/file) so literal newlines survive JSON encoding. Set `PAPERCLIP_SKILL_DIR` to the absolute directory containing the installed `SKILL.md` you just read. This helper ships with the skill and uses the injected `PAPERCLIP_*` environment variables; it does not need a Paperclip source checkout or a local credentials file. ```bash -scripts/paperclip-issue-update.sh --issue-id "$PAPERCLIP_TASK_ID" --status done <<'MD' +bash "$PAPERCLIP_SKILL_DIR/scripts/paperclip-issue-update.sh" --issue-id "$PAPERCLIP_TASK_ID" --status done <<'MD' Done - Fixed the newline-preserving issue update path diff --git a/skills/paperclip/scripts/paperclip-issue-update.sh b/skills/paperclip/scripts/paperclip-issue-update.sh new file mode 100755 index 0000000000..f8717e31c0 --- /dev/null +++ b/skills/paperclip/scripts/paperclip-issue-update.sh @@ -0,0 +1,166 @@ +#!/usr/bin/env bash + +set -euo pipefail + +usage() { + cat <<'EOF' +Usage: + scripts/paperclip-issue-update.sh [--issue-id ID] [--status STATUS] [--comment TEXT] [--dry-run] + +Reads a multiline markdown comment from stdin when stdin is piped. This preserves +newlines when building the JSON payload for PATCH /api/issues/{issueId}. + +Examples: + scripts/paperclip-issue-update.sh --issue-id "$PAPERCLIP_TASK_ID" --status in_progress <<'MD' + Investigating formatting + + - Pulled the raw comment body + - Comparing it with the run transcript + MD + + scripts/paperclip-issue-update.sh --issue-id "$PAPERCLIP_TASK_ID" --status done --dry-run <<'MD' + Done + + - Fixed the issue update helper + MD +EOF +} + +require_command() { + if ! command -v "$1" >/dev/null 2>&1; then + printf 'Missing required command: %s\n' "$1" >&2 + exit 1 + fi +} + +issue_id="${PAPERCLIP_TASK_ID:-}" +status="" +comment_arg="" +dry_run=0 + +while [[ $# -gt 0 ]]; do + case "$1" in + --issue-id) + issue_id="${2:-}" + shift 2 + ;; + --status) + status="${2:-}" + shift 2 + ;; + --comment) + comment_arg="${2:-}" + shift 2 + ;; + --dry-run) + dry_run=1 + shift + ;; + --help|-h) + usage + exit 0 + ;; + *) + printf 'Unknown argument: %s\n' "$1" >&2 + usage >&2 + exit 1 + ;; + esac +done + +if [[ -z "$issue_id" ]]; then + printf 'Missing issue id. Pass --issue-id or set PAPERCLIP_TASK_ID.\n' >&2 + exit 1 +fi + +comment="" +if [[ -n "$comment_arg" ]]; then + comment="$comment_arg" +elif [[ ! -t 0 ]]; then + comment="$(cat)" +fi + +require_command jq + +payload="$( + jq -nc \ + --arg status "$status" \ + --arg comment "$comment" \ + ' + (if $status == "" then {} else {status: $status} end) + + (if $comment == "" then {} else {comment: $comment} end) + ' +)" + +if [[ "$dry_run" == "1" ]]; then + printf '%s\n' "$payload" + exit 0 +fi + +if [[ -z "${PAPERCLIP_API_URL:-}" || -z "${PAPERCLIP_API_KEY:-}" || -z "${PAPERCLIP_RUN_ID:-}" ]]; then + printf 'Missing PAPERCLIP_API_URL, PAPERCLIP_API_KEY, or PAPERCLIP_RUN_ID.\n' >&2 + exit 1 +fi + +# A successful PATCH always returns the updated issue JSON. An empty body or a +# connection-level failure means the write did NOT land, even when a pipeline +# exit code says otherwise, so verify the response instead of inferring success. +# Two attempts total: the shared heartbeat policy stops a control-plane write +# after two consecutive failures, so the helper must not send a third. +max_attempts=2 +attempt=1 +while :; do + http_code="" + body="" + set +e + response="$( + curl -sS -m 30 -X PATCH \ + "$PAPERCLIP_API_URL/api/issues/$issue_id" \ + -H "Authorization: Bearer $PAPERCLIP_API_KEY" \ + -H "X-Paperclip-Run-Id: $PAPERCLIP_RUN_ID" \ + -H 'Content-Type: application/json' \ + --data-binary "$payload" \ + -w '\n%{http_code}' + )" + curl_exit=$? + set -e + + if [[ "$curl_exit" -eq 0 ]]; then + http_code="${response##*$'\n'}" + body="${response%$'\n'*}" + fi + + if [[ "$curl_exit" -eq 0 && "$http_code" == 2* ]]; then + if [[ -z "$body" ]]; then + printf 'Issue update FAILED: HTTP %s with an empty response body. A real update echoes the issue JSON; treat this write as not saved.\n' "$http_code" >&2 + exit 1 + fi + if [[ -n "$status" ]]; then + returned_status="$(jq -r '.status // empty' <<<"$body" 2>/dev/null || true)" + if [[ "$returned_status" != "$status" ]]; then + printf 'Issue update FAILED: server echoed status %s instead of requested %s.\n' "${returned_status:-}" "$status" >&2 + printf '%s\n' "$body" >&2 + exit 1 + fi + fi + printf '%s\n' "$body" + exit 0 + fi + + # 4xx (other than 429) is a definitive rejection; retrying cannot change it. + if [[ "$curl_exit" -eq 0 && "$http_code" == 4* && "$http_code" != "429" ]]; then + printf 'Issue update rejected (HTTP %s).\n' "$http_code" >&2 + [[ -n "$body" ]] && printf '%s\n' "$body" >&2 + exit 1 + fi + + if (( attempt >= max_attempts )); then + printf 'Issue update FAILED after %d attempts (curl exit %s, HTTP %s). The status/comment was NOT saved — report this write as failed, do not assume it landed.\n' "$max_attempts" "$curl_exit" "${http_code:-000}" >&2 + [[ -n "$body" ]] && printf '%s\n' "$body" >&2 + exit 1 + fi + + printf 'Issue update attempt %d/%d failed (curl exit %s, HTTP %s); retrying...\n' "$attempt" "$max_attempts" "$curl_exit" "${http_code:-000}" >&2 + sleep $((attempt * 2)) + attempt=$((attempt + 1)) +done