diff --git a/doc/run-log-events.md b/doc/run-log-events.md index 7a7833dbd8..df12d118b6 100644 --- a/doc/run-log-events.md +++ b/doc/run-log-events.md @@ -193,6 +193,18 @@ the successor to the original failed run. Repair does not reset the automatic retry budget. Transient failures retain the existing bounded retry policy. Archive confinement remains required. +Sandbox restore tasks also write a `Workspace restore diagnostic` line to the +run log on failure. `phase` is `workspace` or `asset`, so a failed staged-asset +copy-back (such as credentials) can be distinguished from workspace restoration. The line +contains only an allowlisted OS/transport `errorCode` (otherwise `unknown`), an +optional numeric HTTP error status, and an optional bounded process exit code. +Up to four nested causes are inspected. Messages, URLs, filesystem paths, asset +names, credentials, and response bodies are excluded. Every failed outbound +task emits its own diagnostic; nested repository failures are logged once by +the enclosing workspace task. The original error and restore safety policy are +unchanged. These lines stay in the instance run log and its configured durable +storage, and are not new first-party telemetry events. + ## Codex resume usage snapshot The native runner retains a bounded local `harness.diagnostic` event with code diff --git a/packages/adapter-utils/src/sandbox-managed-runtime.test.ts b/packages/adapter-utils/src/sandbox-managed-runtime.test.ts index 05f8cec2ea..e04d5a4ee2 100644 --- a/packages/adapter-utils/src/sandbox-managed-runtime.test.ts +++ b/packages/adapter-utils/src/sandbox-managed-runtime.test.ts @@ -3862,6 +3862,34 @@ describe("sandbox managed runtime outbound coordinator", () => { ]); } + it("records each failed restore phase without changing stable failure ordering", async () => { + const { workspaceDir, remoteWorkspaceDir, dirOf } = await makeOutboundDirs(["home", "private-asset"]); + const { control, gate } = makeOutboundControl(); + const client = makeGatedOutboundClient(true, gate); + const workspaceError = Object.assign(new Error("private workspace path"), { code: "EACCES" }); + control.failWith("workspace", workspaceError); + control.failWith("home", Object.assign(new Error("private credential"), { status: 404 })); + control.failWith("private-asset", new Error("private asset error")); + const prepared = await prepareSandboxManagedRuntime({ + spec: makeSpec(remoteWorkspaceDir), adapterKey: "codex", client, workspaceLocalDir: workspaceDir, + assets: [ + makeControlledAsset("home", dirOf("home"), control, gate), + makeControlledAsset("private-asset", dirOf("private-asset"), control, gate), + ], + }); + const lines: string[] = []; + await expect(prepared.restoreWorkspace((line) => { lines.push(line); })).rejects.toBe(workspaceError); + const diagnostics = lines.filter((line) => line.includes("Workspace restore diagnostic:")); + expect(diagnostics).toHaveLength(3); + expect(diagnostics).toEqual(expect.arrayContaining([ + '[paperclip] Workspace restore diagnostic: {"phase":"workspace","errorCode":"EACCES"}\n', + '[paperclip] Workspace restore diagnostic: {"phase":"asset","errorCode":"unknown","httpStatus":404}\n', + '[paperclip] Workspace restore diagnostic: {"phase":"asset","errorCode":"unknown"}\n', + ])); + expect(diagnostics.join("")).not.toContain("private"); + expect(control.settled).toEqual(expect.arrayContaining(["workspace", "home", "private-asset"])); + }); + it("with concurrency permitted, an asset restore starts while the workspace restore is held open", async () => { const { workspaceDir, remoteWorkspaceDir, dirOf } = await makeOutboundDirs(["home"]); const { control, gate } = makeOutboundControl(); diff --git a/packages/adapter-utils/src/sandbox-managed-runtime.ts b/packages/adapter-utils/src/sandbox-managed-runtime.ts index e065e05463..7fd3ab0d97 100644 --- a/packages/adapter-utils/src/sandbox-managed-runtime.ts +++ b/packages/adapter-utils/src/sandbox-managed-runtime.ts @@ -45,6 +45,7 @@ import { type SyncOperationTask, } from "./sync-operation-schedule.js"; import type { RuntimeSpanRunner } from "./acpx-engine/startup-timing.js"; +import { withWorkspaceRestoreDiagnostics } from "./workspace-restore-diagnostics.js"; const execFile = promisify(execFileCallback); const SANDBOX_WORKSPACE_HEAVY_DIR_NAMES = [ @@ -1674,7 +1675,7 @@ export async function prepareSandboxManagedRuntime(input: { // tasks never share scratch state. if (syncWorkspace) { outboundTasks.push(() => - runStepSpan("restore.workspace", async () => { + withWorkspaceRestoreDiagnostics("workspace", () => runStepSpan("restore.workspace", async () => { // Each repository owns its Git history and merge. The parent baseline also // records child files so restart recovery has their original merge inputs. for (const repository of repositories) { @@ -1900,7 +1901,7 @@ export async function prepareSandboxManagedRuntime(input: { } } }); - }), + }), restoreSink), ); } @@ -1912,15 +1913,17 @@ export async function prepareSandboxManagedRuntime(input: { const assetRestore = asset.restore; const assetKey = asset.key; outboundTasks.push(() => - runStepSpan(`restore.asset.${assetKey}`, async () => { - await withTempDir("paperclip-sandbox-restore-", async (tempDir) => { - await assetRestore({ - assetDir: path.posix.join(runtimeRootDir, assetKey), - readFile: async (remotePath) => toBuffer(await input.client.readFile(remotePath)), - tempDir, + withWorkspaceRestoreDiagnostics( + "asset", + () => runStepSpan(`restore.asset.${assetKey}`, async () => { + await withTempDir("paperclip-sandbox-restore-", async (tempDir) => { + await assetRestore({ + assetDir: path.posix.join(runtimeRootDir, assetKey), + readFile: async (remotePath) => toBuffer(await input.client.readFile(remotePath)), + tempDir, + }); }); - }); - }), + }), restoreSink), ); } diff --git a/packages/adapter-utils/src/workspace-restore-diagnostics.test.ts b/packages/adapter-utils/src/workspace-restore-diagnostics.test.ts new file mode 100644 index 0000000000..5281c686a9 --- /dev/null +++ b/packages/adapter-utils/src/workspace-restore-diagnostics.test.ts @@ -0,0 +1,84 @@ +import { describe, expect, it, vi } from "vitest"; +import { classifyWorkspaceRestoreFailure } from "./workspace-restore-merge.js"; +import { withWorkspaceRestoreDiagnostics } from "./workspace-restore-diagnostics.js"; + +describe("workspace restore diagnostics", () => { + it("leaves successful restores silent", async () => { + const sink = vi.fn(); + expect(await withWorkspaceRestoreDiagnostics("workspace", async () => 42, sink)).toBe(42); + expect(sink).not.toHaveBeenCalled(); + }); + + it("records bounded cause fields without copying messages or arbitrary codes", async () => { + const sink = vi.fn(); + const error = Object.assign(new Error("private path and credential"), { + code: "private-code", url: "https://private.invalid/secret", body: "private body", + cause: Object.assign(new Error("private cause"), { code: "ECONNRESET", statusCode: 503, exitCode: 7 }), + }); + await expect(withWorkspaceRestoreDiagnostics("asset", async () => { throw error; }, sink)).rejects.toBe(error); + expect(sink).toHaveBeenCalledExactlyOnceWith( + '[paperclip] Workspace restore diagnostic: {"phase":"asset","errorCode":"ECONNRESET","httpStatus":503,"exitCode":7}\n', + ); + }); + + it.each([null, "private string", { code: "token", status: "401", exitCode: Infinity }, { status: 200, exitCode: -1 }])( + "omits unrecognized diagnostic values (%j)", async (error) => { + const sink = vi.fn(); + await expect(withWorkspaceRestoreDiagnostics("asset", async () => { throw error; }, sink)).rejects.toBe(error); + expect(sink).toHaveBeenCalledExactlyOnceWith( + '[paperclip] Workspace restore diagnostic: {"phase":"asset","errorCode":"unknown"}\n', + ); + }, + ); + + it("bounds cause traversal even when it cycles", async () => { + const error = Object.assign(new Error("private"), { cause: null as unknown, code: "ENOENT" }); + error.cause = error; + const sink = vi.fn(); + await expect(withWorkspaceRestoreDiagnostics("asset", async () => { throw error; }, sink)).rejects.toBe(error); + expect(sink.mock.calls[0]?.[0]).toContain('"errorCode":"ENOENT"'); + }); + + it.each(["EACCES", "WORKSPACE_RESTORE_UNSAFE_ARCHIVE"])("preserves %s when logging fails", async (code) => { + const error = Object.assign(new Error("private"), { code }); + const classification = classifyWorkspaceRestoreFailure(error); + await expect(withWorkspaceRestoreDiagnostics("workspace", async () => { throw error; }, async () => { throw new Error("sink failed"); })) + .rejects.toBe(error); + expect(classifyWorkspaceRestoreFailure(error)).toBe(classification); + }); + + it("keeps the restore error even if a provider diagnostic getter throws", async () => { + const error = Object.defineProperty(Object.assign(new Error("original"), { + statusCode: 503, cause: { code: "ECONNRESET" }, + }), "code", { get() { throw new Error("getter failed"); } }); + const sink = vi.fn(); + await expect(withWorkspaceRestoreDiagnostics("asset", async () => { throw error; }, sink)).rejects.toBe(error); + expect(sink).toHaveBeenCalledExactlyOnceWith( + '[paperclip] Workspace restore diagnostic: {"phase":"asset","errorCode":"ECONNRESET","httpStatus":503}\n', + ); + }); + + it("uses valid fallback statuses when preferred fields are invalid", async () => { + const error = { status: "failed", statusCode: 503, exitCode: "failed", code: 7 }; + const sink = vi.fn(); + await expect(withWorkspaceRestoreDiagnostics("workspace", async () => { throw error; }, sink)).rejects.toBe(error); + expect(sink).toHaveBeenCalledExactlyOnceWith( + '[paperclip] Workspace restore diagnostic: {"phase":"workspace","errorCode":"unknown","httpStatus":503,"exitCode":7}\n', + ); + }); + + it("logs nested failures once while preserving independent concurrent and later diagnostics", async () => { + const error = Object.assign(new Error("nested failure"), { code: "ENOENT" }); + const sink = vi.fn(); + const fail = async () => { await Promise.resolve(); throw error; }; + const results = await Promise.allSettled([ + withWorkspaceRestoreDiagnostics("workspace", () => withWorkspaceRestoreDiagnostics("workspace", fail, sink), sink), + withWorkspaceRestoreDiagnostics("asset", fail, sink), + ]); + expect(results).toEqual([{ status: "rejected", reason: error }, { status: "rejected", reason: error }]); + expect(sink).toHaveBeenCalledTimes(2); + expect(sink.mock.calls.map(([line]) => JSON.parse(line.split(": ")[1]).phase).sort()).toEqual(["asset", "workspace"]); + await expect(withWorkspaceRestoreDiagnostics("workspace", fail, sink)).rejects.toBe(error); + expect(sink).toHaveBeenCalledTimes(3); + }); +}); diff --git a/packages/adapter-utils/src/workspace-restore-diagnostics.ts b/packages/adapter-utils/src/workspace-restore-diagnostics.ts new file mode 100644 index 0000000000..4330b5ac25 --- /dev/null +++ b/packages/adapter-utils/src/workspace-restore-diagnostics.ts @@ -0,0 +1,68 @@ +import { AsyncLocalStorage } from "node:async_hooks"; +import type { RuntimeProgressSink } from "./runtime-progress.js"; + +type RestorePhase = "workspace" | "asset"; +const ERROR_CODES = new Set([ + "ENOENT", "EACCES", "EPERM", "ENOSPC", "EIO", "EXDEV", "ENOTDIR", "EISDIR", + "ECONNRESET", "ECONNREFUSED", "ETIMEDOUT", "EPIPE", "ENOTFOUND", "EAI_AGAIN", + "ABORT_ERR", "UND_ERR_CONNECT_TIMEOUT", "UND_ERR_SOCKET", +]); +const activeDiagnostic = new AsyncLocalStorage(); + +function readField(value: Record, key: string): unknown { + try { return value[key]; } catch { return undefined; } +} + +function boundedInteger(value: unknown, minimum: number, maximum: number): number | undefined { + return typeof value === "number" && Number.isInteger(value) && value >= minimum && value <= maximum + ? value : undefined; +} + +/** Only fixed codes and bounded numbers may enter the company-readable run log. */ +function diagnostic(error: unknown): { errorCode: string; httpStatus?: number; exitCode?: number } { + const result: { errorCode: string; httpStatus?: number; exitCode?: number } = { errorCode: "unknown" }; + let current = error; + // SDKs wrap transport errors in a cause. Bound traversal, including cycles. + for (let depth = 0; depth < 4 && current && typeof current === "object"; depth++) { + const value = current as Record; + const code = readField(value, "code"); + if (result.errorCode === "unknown" && typeof code === "string" && ERROR_CODES.has(code)) { + result.errorCode = code; + } + const status = boundedInteger(readField(value, "status"), 400, 599) + ?? boundedInteger(readField(value, "statusCode"), 400, 599); + if (result.httpStatus === undefined && status !== undefined) { + result.httpStatus = status; + } + const exitCode = boundedInteger(readField(value, "exitCode"), 1, 255) ?? boundedInteger(code, 1, 255); + if (result.exitCode === undefined && exitCode !== undefined) { + result.exitCode = exitCode; + } + current = readField(value, "cause"); + } + return result; +} + +/** Add evidence without changing the thrown error, restore policy, or task ordering. */ +export async function withWorkspaceRestoreDiagnostics( + phase: RestorePhase, + operation: () => Promise, + onProgress?: RuntimeProgressSink, +): Promise { + // A nested repository restore propagates to its enclosing workspace task. + // That task owns the diagnostic. Independent parallel tasks retain their own + // async scopes, so two failed tasks still produce two diagnostic lines. + if (activeDiagnostic.getStore()) return await operation(); + return await activeDiagnostic.run(true, async () => { + try { + return await operation(); + } catch (error) { + try { + await onProgress?.(`[paperclip] Workspace restore diagnostic: ${JSON.stringify({ phase, ...diagnostic(error) })}\n`); + } catch { + // A broken log sink must not replace a restore failure or relax its safety classification. + } + throw error; + } + }); +} diff --git a/packages/adapters/claude-local/src/server/acp.test.ts b/packages/adapters/claude-local/src/server/acp.test.ts index ffb0747364..91009bf010 100644 --- a/packages/adapters/claude-local/src/server/acp.test.ts +++ b/packages/adapters/claude-local/src/server/acp.test.ts @@ -1044,14 +1044,16 @@ describe("claude_local ACP lane", () => { }), ); - // Fail-open: the restore miss never changes the run's exit code or - // status, and it surfaces as one allowlisted code — never the raw error. + // Preserve the execution's exit code while reporting the restore failure. + // Only a fixed diagnostic may contain the errno, never the raw error. expect(result.exitCode).toBe(0); expect(result.resultJson?.workspaceRestoreFailure).toBe("restore_permission_denied"); const allLogs = loggedLines.join(""); expect(allLogs).not.toContain("SENTINEL-HOST-PATH-marker"); expect(allLogs).not.toContain(localCwd); - expect(allLogs).not.toContain("EACCES"); + const diagnostic = '[paperclip] Workspace restore diagnostic: {"phase":"workspace","errorCode":"EACCES"}\n'; + expect(loggedLines.filter((line) => line.includes("Workspace restore diagnostic:"))).toEqual([diagnostic]); + expect(loggedLines.filter((line) => line !== diagnostic).join("")).not.toContain("EACCES"); expect(allLogs).toContain("permission denied"); } finally { await fs.chmod(localCwd, 0o700).catch(() => undefined);