diff --git a/doc/observability.md b/doc/observability.md index 3aaee6c9e2e..148bbb73bce 100644 --- a/doc/observability.md +++ b/doc/observability.md @@ -504,6 +504,21 @@ or `restore_failed`. Unknown values are omitted. Workspace paths and arbitrary pre-restore result data are not included. A later successful run does not, by itself, establish that an earlier failed restore recovered the workspace files. +When available, the saved `workspaceRestoreDiagnostic` adds the bounded fields +`workspaceRestorePhase`, `workspaceRestoreStep`, `workspaceRestoreErrorCode`, +`workspaceRestoreHttpStatus`, and `workspaceRestoreExitCode` to `run_execution`. +The phase is `workspace` or `asset`. The failing step is one of `git_export`, +`git_import`, `workspace_transfer`, `workspace_extract`, `directory_merge`, +`git_integration`, `index_reset`, `git_ref_cleanup`, or `asset_restore`. +Nested steps retain the most specific failing operation. Error codes come from +the restore diagnostic allowlist, with unrecognized codes reported as `unknown`; +HTTP statuses are integers from 400 through 599 and process exit codes are +integers from 1 through 255. These fields accompany a known restore failure code +only. They omit error messages, commands, paths, process output, and arbitrary +cause data. Git error wrappers preserve only these safe codes and numbers for +diagnostics, without adding the original error as a cause. This does not change +restore behavior, retries, timeouts, or recovery policy. + A caught directory-merge lock timeout also records `restoreLockOwnerState` (`alive`, `dead`, `unknown`, `missing`, or `invalid`), `restoreLockKnownLocalHolder`, and, when available, `restoreLockOwnerSameProcess`, `restoreLockOwnerPredatesProcess`, diff --git a/packages/adapter-utils/src/acpx-engine/execute.test.ts b/packages/adapter-utils/src/acpx-engine/execute.test.ts index 03a42f96455..4296a110e79 100644 --- a/packages/adapter-utils/src/acpx-engine/execute.test.ts +++ b/packages/adapter-utils/src/acpx-engine/execute.test.ts @@ -41,6 +41,8 @@ import { sessionCodec } from "./session-codec.js"; import { runChildProcess } from "../server-utils.js"; import { createPromptContextFixture } from "../test-fixtures/prompt-context.js"; import { setExpensiveWorkspaceGitExecutor } from "../git-workspace-sync.js"; +import { createWorkspaceRestoreTeardown } from "../workspace-restore-teardown.js"; +import { withWorkspaceRestoreDiagnostics, withWorkspaceRestoreStep } from "../workspace-restore-diagnostics.js"; import { resolveReferencedSourceIgnore } from "../sandbox-managed-runtime.js"; import { getActiveStepContext, @@ -3703,6 +3705,33 @@ describe("ACPX engine remote managed-home seam (PR 2: per-adapter home seed)", ( expect(path.isAbsolute(remoteAssetDir)).toBe(true); }); + it("retains bounded restore diagnostics through real settlement and result reproduction", async () => { + const { stateDir, localCwd, executionTarget } = await setupRemoteSandbox(); + const failure = Object.assign(new Error("private-restore-path and command"), { code: 1, stderr: "private-restore-stderr" }); + const { result } = await runExecutor( + { agent: "custom", agentCommand: "node ./fake-acp.js", stateDir, cwd: localCwd }, + { + authToken: "real-run-jwt", executionTarget, + prepareRemoteManagedHome: async (input) => { + const stagedRuntime = await input.stage([]); + return { + stagedRuntime, + teardown: createWorkspaceRestoreTeardown({ + stagedRuntime: { restoreWorkspace: () => withWorkspaceRestoreDiagnostics("workspace", () => + withWorkspaceRestoreStep("git_integration", async () => { throw failure; })) }, + onLog: async () => {}, startMessage: "Restoring workspace", failurePrefix: "Restore failed", + }), + }; + }, + }, + ); + expect(result.resultJson).toMatchObject({ + workspaceRestoreFailure: "restore_failed", + workspaceRestoreDiagnostic: { phase: "workspace", step: "git_integration", errorCode: "unknown", exitCode: 1 }, + }); + expect(JSON.stringify(result)).not.toContain("private-restore-"); + }); + it("test_remote_seam_teardown_fires_once_on_exit", async () => { const { stateDir, localCwd, executionTarget } = await setupRemoteSandbox(); let teardownCalls = 0; diff --git a/packages/adapter-utils/src/acpx-engine/execute.ts b/packages/adapter-utils/src/acpx-engine/execute.ts index 9f25c6ba5fa..28e5b620259 100644 --- a/packages/adapter-utils/src/acpx-engine/execute.ts +++ b/packages/adapter-utils/src/acpx-engine/execute.ts @@ -50,6 +50,7 @@ import { } from "./terminal-session-failure.js"; export type { AcpxTerminalSessionFailure } from "./terminal-session-failure.js"; import type { WorkspaceRestoreFailureCode, WorkspaceRestoreOutcome } from "../workspace-restore-merge.js"; +import type { WorkspaceRestoreDiagnostic } from "../workspace-restore-diagnostics.js"; import { classifyWorkspaceRestoreFailure, describeWorkspaceRestoreFailure, @@ -4077,7 +4078,7 @@ export function createAcpxEngineExecutor(deps: AcpxEngineExecutorOptions = {}) { // status stay exactly what the turn produced — this is a signal, not an // outcome change. let workspaceRestoreFailureField: - | { workspaceRestoreFailure: WorkspaceRestoreFailureCode } + | { workspaceRestoreFailure: WorkspaceRestoreFailureCode; workspaceRestoreDiagnostic?: WorkspaceRestoreDiagnostic } | Record = {}; // The one settlement step name whose error can be the same workspace- // restore failure the adapter teardown closure already classifies (a @@ -5410,7 +5411,10 @@ export function createAcpxEngineExecutor(deps: AcpxEngineExecutorOptions = {}) { await withAdapterExecutionPhase(ctx, "workspace_restore", () => runRuntimeSpan("sandbox.syncBack", async () => { const restoreOutcome = await syncBackManagedHome(prepared); if (!restoreOutcome.ok) { - workspaceRestoreFailureField = { workspaceRestoreFailure: restoreOutcome.code }; + workspaceRestoreFailureField = { + workspaceRestoreFailure: restoreOutcome.code, + ...(restoreOutcome.diagnostic ? { workspaceRestoreDiagnostic: restoreOutcome.diagnostic } : {}), + }; } })); } diff --git a/packages/adapter-utils/src/git-workspace-sync.test.ts b/packages/adapter-utils/src/git-workspace-sync.test.ts index d72a24cd7c2..b2231066fae 100644 --- a/packages/adapter-utils/src/git-workspace-sync.test.ts +++ b/packages/adapter-utils/src/git-workspace-sync.test.ts @@ -6,6 +6,7 @@ import { promisify } from "node:util"; import { workspacePaths } from "./workspace-manifest.js"; import { runWorkspaceGitProcess } from "./workspace-git-stream.js"; import { afterEach, describe, expect, it } from "vitest"; +import { getWorkspaceRestoreDiagnostic, withWorkspaceRestoreDiagnostics, withWorkspaceRestoreStep } from "./workspace-restore-diagnostics.js"; import { buildRemoteGitDeltaBundleScript, @@ -24,6 +25,7 @@ import { REFERENCED_SOURCE_IGNORE_MAX_ENTRY_COUNT, REFERENCED_SOURCE_IGNORE_MAX_TOTAL_BYTES, runLocalGit, + resetLocalGitIndexToHead, sanitizeGitRemoteUrl, setExpensiveWorkspaceGitExecutor, withShallowGitWorkspaceClone, @@ -722,11 +724,33 @@ describe("git workspace sync", () => { // object error (exit 128), not the no-ancestor signal (exit 1). The graft // must not fire, and the integration keeps its loud failure. const missingHead = "0123456789abcdef0123456789abcdef01234567"; - await expect(integrateImportedGitHead({ localDir: repo, importedHead: missingHead })) - .rejects.toThrow(/Failed to merge concurrent remote git histories/); + const expectedExit = await runLocalGit(repo, ["merge-tree", "--write-tree", currentHead, missingHead]) + .catch((error: { code: number }) => error.code); + const error = await withWorkspaceRestoreDiagnostics("workspace", () => withWorkspaceRestoreStep("git_integration", () => + integrateImportedGitHead({ localDir: repo, importedHead: missingHead }))).catch((error: unknown) => error); + expect(error).toBeInstanceOf(Error); + expect((error as Error).message).toMatch(/Failed to merge concurrent remote git histories/); + expect(error).not.toHaveProperty("cause"); + expect(getWorkspaceRestoreDiagnostic(error)).toEqual({ phase: "workspace", step: "git_integration", errorCode: "unknown", exitCode: expectedExit }); expect(await git(repo, ["rev-parse", "HEAD"])).toBe(currentHead); }); + it("preserves the real Git index reset exit code without attaching its raw error", async () => { + const repo = await mkdtemp(path.join(os.tmpdir(), "paperclip-git-reset-diagnostic-")); + cleanupDirs.push(repo); + await git(repo, ["init"]); + await writeFile(path.join(repo, "tracked.txt"), "base\n", "utf8"); + await git(repo, ["add", "tracked.txt"]); + await git(repo, ["-c", "user.name=Setup", "-c", "user.email=setup@paperclip.dev", "commit", "-m", "base"]); + await writeFile(path.join(repo, ".git", "index.lock"), "fixture lock\n", "utf8"); + const error = await withWorkspaceRestoreDiagnostics("workspace", () => withWorkspaceRestoreStep("index_reset", () => + resetLocalGitIndexToHead({ localDir: repo }))).catch((error: unknown) => error); + expect(error).toBeInstanceOf(Error); + expect((error as Error).message).toContain("Failed to reset local git index"); + expect(error).not.toHaveProperty("cause"); + expect(getWorkspaceRestoreDiagnostic(error)).toEqual({ phase: "workspace", step: "index_reset", errorCode: "unknown", exitCode: 128 }); + }); + describe("readReferencedSourceGitIgnoredPaths", () => { it("returns null for a directory that is not a Git work tree", async () => { const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-referenced-nogit-")); diff --git a/packages/adapter-utils/src/git-workspace-sync.ts b/packages/adapter-utils/src/git-workspace-sync.ts index 11013832bc8..ce983289bee 100644 --- a/packages/adapter-utils/src/git-workspace-sync.ts +++ b/packages/adapter-utils/src/git-workspace-sync.ts @@ -5,6 +5,7 @@ import os from "node:os"; import path from "node:path"; import { createWorkspaceManifest, workspacePaths, WorkspaceNulParser, type WorkspacePaths } from "./workspace-manifest.js"; import { runWorkspaceGitProcess } from "./workspace-git-stream.js"; +import { preserveWorkspaceRestoreErrorDiagnostic } from "./workspace-restore-diagnostics.js"; export interface GitCommandResult { stdout: string; @@ -931,9 +932,9 @@ export async function integrateImportedGitHead(input: { }); } catch (error) { const reason = error instanceof Error ? error.message : String(error); - throw new Error( + throw preserveWorkspaceRestoreErrorDiagnostic(new Error( `Failed to merge concurrent remote git histories for ${currentHead.slice(0, 12)} and ${input.importedHead.slice(0, 12)}: ${reason}`, - ); + ), error); } const mergedTreeId = mergedTree.stdout.trim().split("\n")[0]?.trim() ?? ""; if (!mergedTreeId) { @@ -990,7 +991,9 @@ export async function resetLocalGitIndexToHead(input: { (error as { stdout?: unknown }).stdout, ].filter((value): value is string => typeof value === "string" && value.trim().length > 0).join("\n") : String(error); - throw new Error(`Failed to reset local git index to HEAD after workspace restore: ${detail}`); + throw preserveWorkspaceRestoreErrorDiagnostic( + new Error(`Failed to reset local git index to HEAD after workspace restore: ${detail}`), error, + ); } const hasDiff = async (args: string[]) => { diff --git a/packages/adapter-utils/src/sandbox-managed-runtime.test.ts b/packages/adapter-utils/src/sandbox-managed-runtime.test.ts index 88144df3dfa..e84f86b5146 100644 --- a/packages/adapter-utils/src/sandbox-managed-runtime.test.ts +++ b/packages/adapter-utils/src/sandbox-managed-runtime.test.ts @@ -6,6 +6,9 @@ import path from "node:path"; import { execFile as execFileCallback, spawn } from "node:child_process"; import { promisify } from "node:util"; import { afterEach, describe, expect, it, vi } from "vitest"; +import * as gitSync from "./git-workspace-sync.js"; +import * as restoreMerge from "./workspace-restore-merge.js"; +import { getWorkspaceRestoreDiagnostic } from "./workspace-restore-diagnostics.js"; import { resetLocalGitIndexToHead, runLocalGit, @@ -3975,14 +3978,14 @@ describe("sandbox managed runtime outbound coordinator", () => { ]); } - it("records each failed restore phase without changing stable failure ordering", async () => { + it.each([false, true])("records each failed restore phase in stable order (shared error: %s)", async (sharedError) => { 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")); + control.failWith("home", sharedError ? workspaceError : Object.assign(new Error("private credential"), { status: 404 })); + control.failWith("private-asset", sharedError ? workspaceError : new Error("private asset error")); const prepared = await prepareSandboxManagedRuntime({ spec: makeSpec(remoteWorkspaceDir), adapterKey: "codex", client, workspaceLocalDir: workspaceDir, assets: [ @@ -3995,11 +3998,14 @@ describe("sandbox managed runtime outbound coordinator", () => { 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', + '[paperclip] Workspace restore diagnostic: {"phase":"workspace","step":"workspace_transfer","errorCode":"EACCES"}\n', + sharedError ? '[paperclip] Workspace restore diagnostic: {"phase":"asset","step":"asset_restore","errorCode":"EACCES"}\n' + : '[paperclip] Workspace restore diagnostic: {"phase":"asset","step":"asset_restore","errorCode":"unknown","httpStatus":404}\n', + sharedError ? '[paperclip] Workspace restore diagnostic: {"phase":"asset","step":"asset_restore","errorCode":"EACCES"}\n' + : '[paperclip] Workspace restore diagnostic: {"phase":"asset","step":"asset_restore","errorCode":"unknown"}\n', ])); expect(diagnostics.join("")).not.toContain("private"); + expect(getWorkspaceRestoreDiagnostic(workspaceError)).toEqual({ phase: "workspace", step: "workspace_transfer", errorCode: "EACCES" }); expect(control.settled).toEqual(expect.arrayContaining(["workspace", "home", "private-asset"])); }); @@ -4355,6 +4361,48 @@ describe("sandbox git-bundle export transport", () => { .filter((mapping) => path.posix.basename(mapping.sourcePath) === "git-delta.bundle"); } + it.each([ + "git_export", "git_import", "workspace_transfer", "workspace_extract", + "directory_merge", "git_integration", "index_reset", "git_ref_cleanup", + ] as const)("identifies the actual %s failure during workspace restore", async (step) => { + const { localWorkspaceDir, remoteWorkspaceDir } = await setupGitBackedWorkspace("paperclip-restore-diagnostic-"); + const client = makeTransportClient(step !== "workspace_extract", { syncOutOperations: [], readFilePaths: [] }); + const prepared = await prepareSandboxManagedRuntime({ + spec: gitSpec(remoteWorkspaceDir), adapterKey: "test-adapter", client, workspaceLocalDir: localWorkspaceDir, + }); + await commitInSandbox(remoteWorkspaceDir); + const failure = Object.assign(new Error("private-restore-command /private-restore-path"), { code: 7, stderr: "private-restore-stderr" }); + if (step === "git_export") client.run = async () => { throw failure; }; + if (step === "git_import") vi.spyOn(gitSync, "fetchGitBundleIntoLocalRef").mockRejectedValueOnce(failure); + if (step === "directory_merge") vi.spyOn(restoreMerge, "mergeDirectoryWithBaseline").mockRejectedValueOnce(failure); + if (step === "git_integration") vi.spyOn(gitSync, "integrateImportedGitHead").mockRejectedValueOnce(failure); + if (step === "index_reset") vi.spyOn(gitSync, "resetLocalGitIndexToHead").mockRejectedValueOnce(failure); + if (step === "git_ref_cleanup") vi.spyOn(gitSync, "deleteLocalGitRef").mockRejectedValueOnce(failure); + if (step === "workspace_transfer") { + const syncOut = client.syncOut!; + client.syncOut = async (operations) => { + if (operations.some((operation) => operation.files.some((file) => file.kind === "directory"))) throw failure; + return syncOut(operations); + }; + } + if (step === "workspace_extract") { + const readRemote = client.readFile; + client.readFile = async (remotePath, options) => remotePath.endsWith("workspace-download.tar") + ? Buffer.from("invalid archive") : readRemote(remotePath, options); + } + const lines: string[] = []; + try { + const thrown = await prepared.restoreWorkspace((line) => { lines.push(line); }).then(() => undefined, (error: unknown) => error); + expect(thrown).toBeInstanceOf(Error); + if (step !== "workspace_extract") expect(thrown).toBe(failure); + expect(getWorkspaceRestoreDiagnostic(thrown)).toMatchObject({ phase: "workspace", step, errorCode: "unknown" }); + if (step !== "workspace_extract") expect(getWorkspaceRestoreDiagnostic(thrown)?.exitCode).toBe(7); + const diagnostics = lines.filter((line) => line.includes("Workspace restore diagnostic:")); + expect(diagnostics).toHaveLength(1); + expect(diagnostics[0]).not.toContain("private-restore-"); + } finally { vi.restoreAllMocks(); } + }); + it("moves the bundle through one native syncOut file mapping, never through readFile", async () => { const capture: TransportCapture = { syncOutOperations: [], readFilePaths: [] }; const { localWorkspaceDir, remoteWorkspaceDir } = await setupGitBackedWorkspace("paperclip-bundle-native-"); diff --git a/packages/adapter-utils/src/sandbox-managed-runtime.ts b/packages/adapter-utils/src/sandbox-managed-runtime.ts index 10eeab606e4..abb5bdf4c30 100644 --- a/packages/adapter-utils/src/sandbox-managed-runtime.ts +++ b/packages/adapter-utils/src/sandbox-managed-runtime.ts @@ -49,7 +49,10 @@ import { type SyncOperationTask, } from "./sync-operation-schedule.js"; import type { RuntimeSpanRunner } from "./acpx-engine/startup-timing.js"; -import { withWorkspaceRestoreDiagnostics } from "./workspace-restore-diagnostics.js"; +import { + recordWorkspaceRestoreDiagnostic, withWorkspaceRestoreDiagnostics, withWorkspaceRestoreStep, + type WorkspaceRestoreDiagnostic, +} from "./workspace-restore-diagnostics.js"; const execFile = promisify(execFileCallback); const SANDBOX_WORKSPACE_HEAVY_DIR_NAMES = [ @@ -1717,6 +1720,7 @@ export async function prepareSandboxManagedRuntime(input: { // started task before it returns, so lease teardown never starts while an // outbound restore task still writes host data. const outboundTasks: Array> = []; + const outboundDiagnostics: Array = []; // The workspace restore task runs only when the run syncs the workspace. // The task exports the sandbox git history (git-backed workspace), reads @@ -1780,7 +1784,7 @@ export async function prepareSandboxManagedRuntime(input: { // the import fails on a missing prerequisite. In that case re-export // a full, self-contained bundle from the still-live sandbox rather // than discard the completed run. - const exportAndImport = async (forceFullBundle: boolean): Promise => { + const exportAndImport = async (forceFullBundle: boolean): Promise => withWorkspaceRestoreStep("git_export", async () => { await input.client.run( `sh -c ${shellQuote(buildRemoteGitDeltaBundleScript({ remoteDir: workspaceRemoteDir, @@ -1838,14 +1842,14 @@ export async function prepareSandboxManagedRuntime(input: { remoteWorkspaceStatus = remoteWorkspaceStatus === "clean" ? "clean" : "dirty"; await input.client.remove(remoteWorkspaceStatusPath).catch(() => undefined); } - return fetchGitBundleIntoLocalRef({ + return withWorkspaceRestoreStep("git_import", () => fetchGitBundleIntoLocalRef({ localDir: input.workspaceLocalDir, bundlePath: localBundlePath, exportRef, importedRef: importedRef!, baseSha: gitSnapshot.headCommit, - }); - }; + })); + }); try { importedHead = await exportAndImport(false); @@ -1885,19 +1889,19 @@ export async function prepareSandboxManagedRuntime(input: { "workspace", { sink: input.onRuntimeProgress, phase: "restore" }, ); - const syncResult = await input.client.syncOut!(operations); + const syncResult = await withWorkspaceRestoreStep("workspace_transfer", () => input.client.syncOut!(operations)); const transferredBytes = sumSyncResultBytes(syncResult); await workspaceRestore.finish(transferredBytes, transferredBytes); } else { const remoteWorkspaceTar = path.posix.join(runtimeRootDir, "workspace-download.tar"); - await input.client.run( + await withWorkspaceRestoreStep("workspace_transfer", () => input.client.run( `sh -c ${shellQuote(createRemoteTarballFromDirectoryCommand({ remoteDir: workspaceRemoteDir, archivePath: remoteWorkspaceTar, exclude: workspaceRestoreExclude, }))}`, { timeoutMs: input.spec.timeoutMs }, - ); + )); const workspaceRestore = makeTransferProgress( restoreSink, "Restoring", @@ -1905,52 +1909,56 @@ export async function prepareSandboxManagedRuntime(input: { "workspace", { sink: input.onRuntimeProgress, phase: "restore" }, ); - const archiveBytes = await input.client.readFile(remoteWorkspaceTar, workspaceRestore.options); + const archiveBytes = await withWorkspaceRestoreStep("workspace_transfer", () => input.client.readFile(remoteWorkspaceTar, workspaceRestore.options)); const archiveBuffer = toBuffer(archiveBytes); await workspaceRestore.finish(archiveBuffer.byteLength, archiveBuffer.byteLength); await input.client.remove(remoteWorkspaceTar).catch(() => undefined); const localArchivePath = path.join(tempDir, "workspace.tar"); - await fs.writeFile(localArchivePath, archiveBuffer); - await extractTarballToDirectory({ - archivePath: localArchivePath, - localDir: extractedDir, + await withWorkspaceRestoreStep("workspace_extract", async () => { + await fs.writeFile(localArchivePath, archiveBuffer); + await extractTarballToDirectory({ + archivePath: localArchivePath, + localDir: extractedDir, + }); }); } const gitHeadToIntegrate = importedHead; - const mergeBaseline = repositories.length === 0 ? baselineSnapshot! : await selectDirectorySnapshot(baselineSnapshot!, { - omit: repositories.map((repo) => repo.path), exclude: workspaceRestoreExclude, ignoredPaths: gitSnapshot?.ignoredPaths, + await withWorkspaceRestoreStep("directory_merge", async () => { + const mergeBaseline = repositories.length === 0 ? baselineSnapshot! : await selectDirectorySnapshot(baselineSnapshot!, { + omit: repositories.map((repo) => repo.path), exclude: workspaceRestoreExclude, ignoredPaths: gitSnapshot?.ignoredPaths, + }); + try { + await mergeDirectoryWithBaseline({ + baseline: mergeBaseline, + sourceDir: extractedDir, + targetDir: input.workspaceLocalDir, + beforeApply: gitHeadToIntegrate + ? async () => { + await withWorkspaceRestoreStep("git_integration", () => integrateImportedGitHead({ + localDir: input.workspaceLocalDir, + importedHead: gitHeadToIntegrate, + })); + } + : undefined, + afterApply: gitSnapshot + ? async () => { + await withWorkspaceRestoreStep("index_reset", () => resetLocalGitIndexToHead({ + localDir: input.workspaceLocalDir, + checkWorkingTreeClean: remoteWorkspaceStatus === "clean", + })); + } + : undefined, + }); + } finally { if (mergeBaseline !== baselineSnapshot) await disposeDirectorySnapshot(mergeBaseline); } }); - try { - await mergeDirectoryWithBaseline({ - baseline: mergeBaseline, - sourceDir: extractedDir, - targetDir: input.workspaceLocalDir, - beforeApply: gitHeadToIntegrate - ? async () => { - await integrateImportedGitHead({ - localDir: input.workspaceLocalDir, - importedHead: gitHeadToIntegrate, - }); - } - : undefined, - afterApply: gitSnapshot - ? async () => { - await resetLocalGitIndexToHead({ - localDir: input.workspaceLocalDir, - checkWorkingTreeClean: remoteWorkspaceStatus === "clean", - }); - } - : undefined, - }); - } finally { if (mergeBaseline !== baselineSnapshot) await disposeDirectorySnapshot(mergeBaseline); } } finally { await emitRuntimeStatus(input.onRuntimeProgress, "finalize", "Finalizing workspace"); if (importedRef) { - await deleteLocalGitRef({ localDir: input.workspaceLocalDir, ref: importedRef }); + await withWorkspaceRestoreStep("git_ref_cleanup", () => deleteLocalGitRef({ localDir: input.workspaceLocalDir, ref: importedRef! })); } } }); - }), restoreSink), + }), restoreSink, (diagnostic) => { outboundDiagnostics[0] = diagnostic; }), ); } @@ -1961,18 +1969,19 @@ export async function prepareSandboxManagedRuntime(input: { if (!asset.restore) continue; const assetRestore = asset.restore; const assetKey = asset.key; + const taskIndex = outboundTasks.length; outboundTasks.push(() => withWorkspaceRestoreDiagnostics( "asset", () => runStepSpan(`restore.asset.${assetKey}`, async () => { await withTempDir("paperclip-sandbox-restore-", async (tempDir) => { - await assetRestore({ + await withWorkspaceRestoreStep("asset_restore", () => assetRestore({ assetDir: path.posix.join(runtimeRootDir, assetKey), readFile: async (remotePath) => toBuffer(await input.client.readFile(remotePath)), tempDir, - }); + })); }); - }), restoreSink), + }), restoreSink, (diagnostic) => { outboundDiagnostics[taskIndex] = diagnostic; }), ); } @@ -1989,8 +1998,9 @@ export async function prepareSandboxManagedRuntime(input: { // Every outbound restore is a required operation: a rejection is fatal. // The workspace comes first, then each asset in order. Raise the first // rejection in stable task order, so two failures raise the earlier one. - for (const result of outboundResults) { + for (const [index, result] of outboundResults.entries()) { if (result.status === "rejected") { + recordWorkspaceRestoreDiagnostic(result.reason, outboundDiagnostics[index]); throw result.reason; } } diff --git a/packages/adapter-utils/src/workspace-restore-diagnostics.test.ts b/packages/adapter-utils/src/workspace-restore-diagnostics.test.ts index 5281c686a90..ba7768897e4 100644 --- a/packages/adapter-utils/src/workspace-restore-diagnostics.test.ts +++ b/packages/adapter-utils/src/workspace-restore-diagnostics.test.ts @@ -1,8 +1,90 @@ import { describe, expect, it, vi } from "vitest"; import { classifyWorkspaceRestoreFailure } from "./workspace-restore-merge.js"; -import { withWorkspaceRestoreDiagnostics } from "./workspace-restore-diagnostics.js"; +import { + getWorkspaceRestoreDiagnostic, preserveWorkspaceRestoreErrorDiagnostic, recordWorkspaceRestoreDiagnostic, + withWorkspaceRestoreDiagnostics, withWorkspaceRestoreStep, + type WorkspaceRestoreDiagnostic, +} from "./workspace-restore-diagnostics.js"; describe("workspace restore diagnostics", () => { + it.each([new Error("shared nested failure"), "shared primitive failure"])("selects an unlabelled nested failure without retaining a later task's step (%s)", async (error) => { + const sink = vi.fn(); + const snapshots: Array = []; + await expect(withWorkspaceRestoreDiagnostics("workspace", async () => { + await Promise.allSettled([ + withWorkspaceRestoreDiagnostics("workspace", async () => { throw error; }, sink, (value) => { snapshots[0] = value; }), + withWorkspaceRestoreDiagnostics("asset", () => withWorkspaceRestoreStep("asset_restore", async () => { throw error; }), sink, + (value) => { snapshots[1] = value; }), + ]); + recordWorkspaceRestoreDiagnostic(error, snapshots[0]); + throw error; + }, sink)).rejects.toBe(error); + expect(snapshots[1]?.step).toBe("asset_restore"); + expect(getWorkspaceRestoreDiagnostic(error)).toEqual(typeof error === "object" ? { phase: "workspace", errorCode: "unknown" } : undefined); + expect(sink).toHaveBeenCalledExactlyOnceWith('[paperclip] Workspace restore diagnostic: {"phase":"workspace","errorCode":"unknown"}\n'); + }); + + it("keeps logging when a diagnostic consumer fails", async () => { + const error = new Error("restore failed"); + const sink = vi.fn(); + await expect(withWorkspaceRestoreDiagnostics("workspace", async () => { throw error; }, sink, + () => { throw new Error("consumer failed"); })).rejects.toBe(error); + expect(sink).toHaveBeenCalledTimes(1); + }); + + it("retains a nested step and bounded Git fields across an existing wrapper without a cause", async () => { + const source = Object.assign(new Error("private command/path"), { code: 1, stderr: "private stderr" }); + const wrapper = new Error("existing wrapper"); + await expect(withWorkspaceRestoreDiagnostics("workspace", () => withWorkspaceRestoreStep("directory_merge", async () => { + try { + await withWorkspaceRestoreStep("git_integration", async () => { throw source; }); + } catch (error) { throw preserveWorkspaceRestoreErrorDiagnostic(wrapper, error); } + }))).rejects.toBe(wrapper); + expect(wrapper).not.toHaveProperty("cause"); + expect(Object.keys(wrapper)).toEqual([]); + expect(getWorkspaceRestoreDiagnostic(wrapper)).toEqual({ phase: "workspace", step: "git_integration", errorCode: "unknown", exitCode: 1 }); + }); + + it("attributes a reused error to the later failed retry step", async () => { + const error = new Error("same failure"); + await expect(withWorkspaceRestoreDiagnostics("workspace", async () => { + await withWorkspaceRestoreStep("git_import", async () => { throw error; }).catch(() => {}); + await withWorkspaceRestoreStep("git_export", async () => { throw error; }); + })).rejects.toBe(error); + expect(getWorkspaceRestoreDiagnostic(error)?.step).toBe("git_export"); + await expect(withWorkspaceRestoreDiagnostics("asset", async () => { throw error; })).rejects.toBe(error); + expect(getWorkspaceRestoreDiagnostic(error)).toEqual({ phase: "asset", errorCode: "unknown" }); + }); + + it("isolates concurrent steps even when their errors share identity", async () => { + const error = new Error("shared error"); + const sink = vi.fn(); + await Promise.allSettled([ + withWorkspaceRestoreDiagnostics("workspace", () => withWorkspaceRestoreStep("git_import", async () => { throw error; }), sink), + withWorkspaceRestoreDiagnostics("asset", () => withWorkspaceRestoreStep("asset_restore", async () => { throw error; }), sink), + ]); + expect(sink.mock.calls.map(([line]) => JSON.parse(line.split(": ")[1]))).toEqual(expect.arrayContaining([ + { phase: "workspace", step: "git_import", errorCode: "unknown" }, + { phase: "asset", step: "asset_restore", errorCode: "unknown" }, + ])); + }); + + it("starts a fresh scope for later work inherited from a completed async context", async () => { + const error = new Error("later failure"); + let release!: () => void; + const gate = new Promise((resolve) => { release = resolve; }); + let later!: Promise; + const sink = vi.fn(); + await withWorkspaceRestoreDiagnostics("workspace", async () => { + later = gate.then(() => withWorkspaceRestoreDiagnostics("asset", () => + withWorkspaceRestoreStep("asset_restore", async () => { throw error; }), sink)).catch((value) => value); + }, sink); + release(); + expect(await later).toBe(error); + expect(getWorkspaceRestoreDiagnostic(error)).toEqual({ phase: "asset", step: "asset_restore", errorCode: "unknown" }); + expect(sink).toHaveBeenCalledTimes(1); + }); + it("leaves successful restores silent", async () => { const sink = vi.fn(); expect(await withWorkspaceRestoreDiagnostics("workspace", async () => 42, sink)).toBe(42); diff --git a/packages/adapter-utils/src/workspace-restore-diagnostics.ts b/packages/adapter-utils/src/workspace-restore-diagnostics.ts index 4330b5ac25e..4b3312313ea 100644 --- a/packages/adapter-utils/src/workspace-restore-diagnostics.ts +++ b/packages/adapter-utils/src/workspace-restore-diagnostics.ts @@ -2,12 +2,39 @@ import { AsyncLocalStorage } from "node:async_hooks"; import type { RuntimeProgressSink } from "./runtime-progress.js"; type RestorePhase = "workspace" | "asset"; +const RESTORE_STEPS = new Set([ + "git_export", "git_import", "workspace_transfer", "workspace_extract", + "directory_merge", "git_integration", "index_reset", "git_ref_cleanup", "asset_restore", +] as const); +export type WorkspaceRestoreStep = typeof RESTORE_STEPS extends Set ? T : never; +export interface WorkspaceRestoreDiagnostic { + phase: RestorePhase; + step?: WorkspaceRestoreStep; + errorCode: string; + httpStatus?: number; + exitCode?: number; +} 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(); +type ErrorDiagnostic = Pick; +interface DiagnosticScope { + active: boolean; + sequence: number; + failures: Map; +} +const activeDiagnostic = new AsyncLocalStorage(); +// Never attach a raw cause to an error just to retain a numeric Git exit code. +const wrappedDiagnostics = new WeakMap(); +const restoreDiagnostics = new WeakMap(); +const diagnosticCapture = new AsyncLocalStorage>(); + +/** Keep each settlement's diagnostic receipt isolated across asynchronous work. */ +export function withWorkspaceRestoreDiagnosticCapture(operation: () => Promise): Promise { + return diagnosticCapture.run(new WeakMap(), operation); +} function readField(value: Record, key: string): unknown { try { return value[key]; } catch { return undefined; } @@ -19,22 +46,24 @@ function boundedInteger(value: unknown, minimum: number, maximum: number): numbe } /** 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" }; +function diagnostic(error: unknown): ErrorDiagnostic { + const result: ErrorDiagnostic = { 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"); + const saved = wrappedDiagnostics.get(value); + const code = saved?.errorCode !== "unknown" && saved?.errorCode !== undefined + ? saved.errorCode : 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) + const status = saved?.httpStatus ?? 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); + const exitCode = saved?.exitCode ?? boundedInteger(readField(value, "exitCode"), 1, 255) ?? boundedInteger(code, 1, 255); if (result.exitCode === undefined && exitCode !== undefined) { result.exitCode = exitCode; } @@ -43,26 +72,104 @@ function diagnostic(error: unknown): { errorCode: string; httpStatus?: number; e return result; } +/** Retain bounded fields across an existing error wrapper, without a raw cause. */ +export function preserveWorkspaceRestoreErrorDiagnostic(wrapper: T, source: unknown): T { + wrappedDiagnostics.set(wrapper, diagnostic(source)); + const scope = activeDiagnostic.getStore(); + const failure = scope?.active ? scope.failures.get(source) : undefined; + if (failure) scope!.failures.set(wrapper, failure); + return wrapper; +} + +/** Decode persisted adapter metadata; never copy arbitrary properties or text. */ +export function sanitizeWorkspaceRestoreDiagnostic(value: unknown): WorkspaceRestoreDiagnostic | undefined { + if (!value || typeof value !== "object") return undefined; + const record = value as Record; + const phase = readField(record, "phase"); + if (phase !== "workspace" && phase !== "asset") return undefined; + const step = readField(record, "step"); + const code = readField(record, "errorCode"); + const httpStatus = boundedInteger(readField(record, "httpStatus"), 400, 599); + const exitCode = boundedInteger(readField(record, "exitCode"), 1, 255); + return { + phase, + ...(typeof step === "string" && RESTORE_STEPS.has(step as WorkspaceRestoreStep) + ? { step: step as WorkspaceRestoreStep } : {}), + errorCode: typeof code === "string" && ERROR_CODES.has(code) ? code : "unknown", + ...(httpStatus !== undefined ? { httpStatus } : {}), + ...(exitCode !== undefined ? { exitCode } : {}), + }; +} + +export function getWorkspaceRestoreDiagnostic(error: unknown): WorkspaceRestoreDiagnostic | undefined { + return error && typeof error === "object" + ? sanitizeWorkspaceRestoreDiagnostic((diagnosticCapture.getStore() ?? restoreDiagnostics).get(error)) : undefined; +} + +/** Preserve the scheduler-selected task's snapshot when errors share identity. */ +export function recordWorkspaceRestoreDiagnostic(error: unknown, diagnostic: WorkspaceRestoreDiagnostic | undefined): void { + const safe = sanitizeWorkspaceRestoreDiagnostic(diagnostic); + if (error && typeof error === "object") { + const receipts = diagnosticCapture.getStore() ?? restoreDiagnostics; + if (safe) receipts.set(error, safe); + else receipts.delete(error); + } + const scope = activeDiagnostic.getStore(); + if (scope?.active) { + if (safe?.step) scope.failures.set(error, { sequence: ++scope.sequence, step: safe.step }); + else scope.failures.delete(error); + } +} + +/** Record only the failing step. Successful cleanup must not replace evidence. */ +export async function withWorkspaceRestoreStep(step: WorkspaceRestoreStep, operation: () => Promise): Promise { + const scope = activeDiagnostic.getStore(); + if (!scope?.active) return await operation(); + const started = scope.sequence; + try { + return await operation(); + } catch (error) { + // A nested step owns its failure. A caught/retried error may be thrown again + // by a later step, so identity alone cannot identify the current failure. + if (scope.active && (scope.failures.get(error)?.sequence ?? -1) <= started) { + scope.failures.set(error, { sequence: ++scope.sequence, step }); + } + throw error; + } +} + /** Add evidence without changing the thrown error, restore policy, or task ordering. */ export async function withWorkspaceRestoreDiagnostics( phase: RestorePhase, operation: () => Promise, onProgress?: RuntimeProgressSink, + onDiagnostic?: (diagnostic: WorkspaceRestoreDiagnostic) => void, ): 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 () => { + const parent = activeDiagnostic.getStore(); + const scope: DiagnosticScope = { active: true, sequence: 0, failures: new Map() }; + return await activeDiagnostic.run(scope, async () => { try { return await operation(); } catch (error) { + const step = scope.failures.get(error)?.step; + const fields = { phase, ...(step ? { step } : {}), ...diagnostic(error) }; + recordWorkspaceRestoreDiagnostic(error, fields); + if (parent?.active && step) parent.failures.set(error, { sequence: ++parent.sequence, step }); try { - await onProgress?.(`[paperclip] Workspace restore diagnostic: ${JSON.stringify({ phase, ...diagnostic(error) })}\n`); + onDiagnostic?.({ ...fields }); + } catch { /* Diagnostic consumers cannot replace a restore failure. */ } + try { + if (!parent?.active) await onProgress?.(`[paperclip] Workspace restore diagnostic: ${JSON.stringify(fields)}\n`); } catch { // A broken log sink must not replace a restore failure or relax its safety classification. } throw error; + } finally { + scope.active = false; + scope.failures.clear(); } }); } diff --git a/packages/adapter-utils/src/workspace-restore-merge.ts b/packages/adapter-utils/src/workspace-restore-merge.ts index 14c58371b35..5865d8b1c5e 100644 --- a/packages/adapter-utils/src/workspace-restore-merge.ts +++ b/packages/adapter-utils/src/workspace-restore-merge.ts @@ -6,6 +6,7 @@ import { DatabaseSync } from "node:sqlite"; import { createWorkspaceManifest, WorkspaceManifestMap, workspacePathMatcher, type PathManifest, type WorkspacePaths, type WorkspaceManifestWriter } from "./workspace-manifest.js"; import { shouldExcludePath } from "./exclude-patterns.js"; import { resolvePaperclipInstanceRootForAdapter } from "./server-utils.js"; +import type { WorkspaceRestoreDiagnostic } from "./workspace-restore-diagnostics.js"; export type SnapshotEntry = | { kind: "dir" } @@ -292,7 +293,7 @@ export type WorkspaceRestoreFailureCode = */ export type WorkspaceRestoreOutcome = | { readonly ok: true } - | { readonly ok: false; readonly code: WorkspaceRestoreFailureCode }; + | { readonly ok: false; readonly code: WorkspaceRestoreFailureCode; readonly diagnostic?: WorkspaceRestoreDiagnostic }; /** * Classifies a caught workspace-restore error into one allowlisted code. Maps diff --git a/packages/adapter-utils/src/workspace-restore-result.test.ts b/packages/adapter-utils/src/workspace-restore-result.test.ts index 98b611789c5..f3ebfa42daf 100644 --- a/packages/adapter-utils/src/workspace-restore-result.test.ts +++ b/packages/adapter-utils/src/workspace-restore-result.test.ts @@ -1,6 +1,7 @@ import { describe, expect, it } from "vitest"; import { applyWorkspaceRestoreFailure, withWorkspaceRestore } from "./workspace-restore-result.js"; import type { AdapterExecutionResult } from "./types.js"; +import { withWorkspaceRestoreDiagnostics, withWorkspaceRestoreStep } from "./workspace-restore-diagnostics.js"; const completed: AdapterExecutionResult = { exitCode: 0, signal: null, timedOut: false, @@ -10,6 +11,18 @@ const completed: AdapterExecutionResult = { const unsafe = new Error("Daytona syncOut refusing tarball link whose target escapes the extraction dir: .claude/skills/paperclip -> /tmp/private-clone/secret-key"); describe("workspace restore settlement", () => { + it("retains only the safe failed restore step alongside the original completed turn", async () => { + const error = Object.assign(new Error("private-restore-path and command"), { code: 1, stderr: "private-restore-output" }); + const result = await withWorkspaceRestore(async () => completed, () => withWorkspaceRestoreDiagnostics("workspace", () => + withWorkspaceRestoreStep("index_reset", async () => { throw error; }))); + expect(result.resultJson).toMatchObject({ + workspaceRestoreFailure: "restore_failed", + workspaceRestoreDiagnostic: { phase: "workspace", step: "index_reset", errorCode: "unknown", exitCode: 1 }, + executionBeforeRestore: { exitCode: 0, timedOut: false, errorCode: null }, + }); + expect(JSON.stringify(result)).not.toContain("private-restore-"); + }); + it("retains the completed result and safe member while excluding the unsafe target", async () => { const result = await withWorkspaceRestore(async () => completed, async () => { throw unsafe; }); expect(result).toMatchObject({ diff --git a/packages/adapter-utils/src/workspace-restore-result.ts b/packages/adapter-utils/src/workspace-restore-result.ts index 6db30e1879b..76c2fef76bc 100644 --- a/packages/adapter-utils/src/workspace-restore-result.ts +++ b/packages/adapter-utils/src/workspace-restore-result.ts @@ -1,6 +1,7 @@ import { hasWorkspaceRestoreFailure, safeWorkspaceRestorePath } from "@paperclipai/shared"; import type { AdapterExecutionResult } from "./types.js"; import { classifyWorkspaceRestoreFailure } from "./workspace-restore-merge.js"; +import { getWorkspaceRestoreDiagnostic, withWorkspaceRestoreDiagnosticCapture } from "./workspace-restore-diagnostics.js"; /** A completed model turn does not imply that required workspace files arrived. */ export function applyWorkspaceRestoreFailure(result: AdapterExecutionResult): AdapterExecutionResult { @@ -29,40 +30,44 @@ export async function withWorkspaceRestore( execute: () => Promise, restore: () => Promise, ): Promise { - let result: AdapterExecutionResult | undefined; - let executionError: unknown; - let executionThrew = false; - try { - result = await execute(); - } catch (error) { - executionError = error; - executionThrew = true; - } - try { - await restore(); - } catch (error) { - const code = classifyWorkspaceRestoreFailure(error); - // Parse the archive member only. Never expose the unsafe link target. - const member = error instanceof Error - ? /Daytona syncOut refusing tarball link whose target escapes the extraction dir: (.+?) -> /.exec(error.message)?.[1] - : null; - const relativePath = safeWorkspaceRestorePath(member); - return applyWorkspaceRestoreFailure({ - ...(result ?? { - exitCode: null, - signal: null, - timedOut: false, - errorCode: "adapter_failed", - // The server applies its ordinary execution-error redaction to this field. - errorMessage: executionError instanceof Error ? executionError.message : "Adapter execution failed.", - }), - resultJson: { - ...result?.resultJson, - workspaceRestoreFailure: code, - ...(relativePath ? { workspaceRestorePath: relativePath } : {}), - }, - }); - } - if (executionThrew) throw executionError; - return result!; + return withWorkspaceRestoreDiagnosticCapture(async () => { + let result: AdapterExecutionResult | undefined; + let executionError: unknown; + let executionThrew = false; + try { + result = await execute(); + } catch (error) { + executionError = error; + executionThrew = true; + } + try { + await restore(); + } catch (error) { + const code = classifyWorkspaceRestoreFailure(error); + const diagnostic = getWorkspaceRestoreDiagnostic(error); + // Parse the archive member only. Never expose the unsafe link target. + const member = error instanceof Error + ? /Daytona syncOut refusing tarball link whose target escapes the extraction dir: (.+?) -> /.exec(error.message)?.[1] + : null; + const relativePath = safeWorkspaceRestorePath(member); + return applyWorkspaceRestoreFailure({ + ...(result ?? { + exitCode: null, + signal: null, + timedOut: false, + errorCode: "adapter_failed", + // The server applies its ordinary execution-error redaction to this field. + errorMessage: executionError instanceof Error ? executionError.message : "Adapter execution failed.", + }), + resultJson: { + ...result?.resultJson, + workspaceRestoreFailure: code, + ...(diagnostic ? { workspaceRestoreDiagnostic: diagnostic } : {}), + ...(relativePath ? { workspaceRestorePath: relativePath } : {}), + }, + }); + } + if (executionThrew) throw executionError; + return result!; + }); } diff --git a/packages/adapter-utils/src/workspace-restore-teardown.test.ts b/packages/adapter-utils/src/workspace-restore-teardown.test.ts index 664c4dcbff8..e4fb8b2939a 100644 --- a/packages/adapter-utils/src/workspace-restore-teardown.test.ts +++ b/packages/adapter-utils/src/workspace-restore-teardown.test.ts @@ -1,6 +1,7 @@ import { describe, expect, it } from "vitest"; import { createWorkspaceRestoreTeardown } from "./workspace-restore-teardown.js"; +import { withWorkspaceRestoreDiagnostics, withWorkspaceRestoreStep } from "./workspace-restore-diagnostics.js"; // One row per adapter's two message strings, taken verbatim from // claude-local, codex-local, and gemini-local. The factory's contract must @@ -24,6 +25,32 @@ const ADAPTER_MESSAGE_PAIRS = [ ]; describe.each(ADAPTER_MESSAGE_PAIRS)("createWorkspaceRestoreTeardown ($adapter)", ({ startMessage, failurePrefix }) => { + it("keeps concurrent settlements separate when a provider reuses an error", async () => { + const error = Object.assign(new Error("private-restore-path"), { code: 1 }); + let release!: () => void; + let started!: () => void; + const gate = new Promise((resolve) => { release = resolve; }); + const waiting = new Promise((resolve) => { started = resolve; }); + const first = createWorkspaceRestoreTeardown({ + stagedRuntime: { restoreWorkspace: () => withWorkspaceRestoreDiagnostics("workspace", () => + withWorkspaceRestoreStep("git_import", async () => { throw error; }), async () => { started(); await gate; }) }, + onLog: async () => {}, startMessage, failurePrefix, + })(); + await waiting; + const second = await createWorkspaceRestoreTeardown({ + stagedRuntime: { restoreWorkspace: () => withWorkspaceRestoreDiagnostics("asset", () => + withWorkspaceRestoreStep("asset_restore", async () => { throw error; })) }, + onLog: async () => {}, startMessage, failurePrefix, + })(); + release(); + expect(await first).toEqual({ ok: false, code: "restore_failed", diagnostic: { + phase: "workspace", step: "git_import", errorCode: "unknown", exitCode: 1, + } }); + expect(second).toEqual({ ok: false, code: "restore_failed", diagnostic: { + phase: "asset", step: "asset_restore", errorCode: "unknown", exitCode: 1, + } }); + }); + it("logs the start message to stdout and returns ok on a clean restore", async () => { const logLines: Array<{ stream: "stdout" | "stderr"; chunk: string }> = []; const teardown = createWorkspaceRestoreTeardown({ diff --git a/packages/adapter-utils/src/workspace-restore-teardown.ts b/packages/adapter-utils/src/workspace-restore-teardown.ts index 7c5a298d919..5c087028d54 100644 --- a/packages/adapter-utils/src/workspace-restore-teardown.ts +++ b/packages/adapter-utils/src/workspace-restore-teardown.ts @@ -1,4 +1,5 @@ import type { RuntimeProgressSink } from "./runtime-progress.js"; +import { getWorkspaceRestoreDiagnostic, withWorkspaceRestoreDiagnosticCapture } from "./workspace-restore-diagnostics.js"; import { classifyWorkspaceRestoreFailure, describeWorkspaceRestoreFailure, @@ -33,7 +34,7 @@ export function createWorkspaceRestoreTeardown(input: { failurePrefix: string; }): () => Promise { const { stagedRuntime, onLog, startMessage, failurePrefix } = input; - return async () => { + return () => withWorkspaceRestoreDiagnosticCapture(async () => { try { await onLog("stdout", startMessage); await stagedRuntime.restoreWorkspace((line) => onLog("stdout", line)); @@ -44,8 +45,9 @@ export function createWorkspaceRestoreTeardown(input: { // filesystem path or a process id. Log only the fixed, allowlisted // diagnostic for the classified code. const code = classifyWorkspaceRestoreFailure(err); + const diagnostic = getWorkspaceRestoreDiagnostic(err); await onLog("stderr", `${failurePrefix}: ${describeWorkspaceRestoreFailure(code)}\n`); - return { ok: false, code }; + return { ok: false, code, ...(diagnostic ? { diagnostic } : {}) }; } - }; + }); } diff --git a/packages/adapters/claude-local/src/server/acp.test.ts b/packages/adapters/claude-local/src/server/acp.test.ts index ff6207fed5b..8e16e2d31a9 100644 --- a/packages/adapters/claude-local/src/server/acp.test.ts +++ b/packages/adapters/claude-local/src/server/acp.test.ts @@ -1065,10 +1065,13 @@ describe("claude_local ACP lane", () => { // 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"); + expect(result.resultJson?.workspaceRestoreDiagnostic).toEqual({ + phase: "workspace", step: "directory_merge", errorCode: "EACCES", + }); const allLogs = loggedLines.join(""); expect(allLogs).not.toContain("SENTINEL-HOST-PATH-marker"); expect(allLogs).not.toContain(localCwd); - const diagnostic = '[paperclip] Workspace restore diagnostic: {"phase":"workspace","errorCode":"EACCES"}\n'; + const diagnostic = '[paperclip] Workspace restore diagnostic: {"phase":"workspace","step":"directory_merge","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"); diff --git a/server/src/__tests__/run-failure-sentry-real-sdk.test.ts b/server/src/__tests__/run-failure-sentry-real-sdk.test.ts index 2d0ae8e8142..0ecd025d3a4 100644 --- a/server/src/__tests__/run-failure-sentry-real-sdk.test.ts +++ b/server/src/__tests__/run-failure-sentry-real-sdk.test.ts @@ -2,6 +2,12 @@ import { createRequire } from "node:module"; import { afterEach, describe, expect, it, vi } from "vitest"; import { collectRunFailureDiagnostics, sanitizeRunFailureDiagnostics } from "../services/run-failure-diagnostics.js"; import type { heartbeatRuns } from "@paperclipai/db"; +import { + preserveWorkspaceRestoreErrorDiagnostic, + withWorkspaceRestoreDiagnostics, + withWorkspaceRestoreStep, +} from "@paperclipai/adapter-utils/workspace-restore-diagnostics"; +import { createWorkspaceRestoreTeardown } from "@paperclipai/adapter-utils/workspace-restore-teardown"; // Sentry is an optional peer. When installed, exercise the real SDK with an // in-memory transport, including its context behavior without an OTel manager. @@ -205,4 +211,80 @@ describe.skipIf(!sentryPackage)("run failure context with the real Sentry SDK", } } }); + + it("reports the failing restore step without exposing the wrapped Git error", async () => { + const Sentry = sentryPackage!; + const events: Array> = []; + vi.stubEnv("SENTRY_DSN_BACKEND", "https://public@example.invalid/1"); + vi.doMock("../peer-version-check.js", () => ({ checkExactPeerVersions: () => ({ ok: true }) })); + vi.doMock("@sentry/node", () => ({ + ...Sentry, + init: (options: Record) => Sentry.init({ + ...options, + transport: () => ({ send: async () => ({}), flush: async () => true }), + beforeSend: (event: Record) => { events.push(event); return event; }, + }), + })); + vi.resetModules(); + const { sentryReady, captureRunFailure, captureException } = await import("../sentry.js"); + await sentryReady; + + const source = Object.assign(new Error("private-restore-Git command failed in /private-restore-workspace", { + cause: new Error("private-restore-provider cause"), + }), { + code: 1, statusCode: 503, stdout: "private-restore-file contents", stderr: "private-restore-Git output", + command: "private-restore-command", path: "/private-restore-workspace", response: { body: "private-restore-response" }, + }); + const wrapper = preserveWorkspaceRestoreErrorDiagnostic(new Error("private-restore-Git wrapper"), source); + expect(wrapper).not.toHaveProperty("cause"); + const logs: string[] = []; + const restore = createWorkspaceRestoreTeardown({ + stagedRuntime: { + restoreWorkspace: (onProgress) => withWorkspaceRestoreDiagnostics("workspace", () => + withWorkspaceRestoreStep("directory_merge", () => + withWorkspaceRestoreStep("git_integration", async () => { throw wrapper; })), onProgress), + }, + onLog: async (_stream, line) => { logs.push(line); }, + startMessage: "Restoring workspace\n", + failurePrefix: "Workspace restore failed", + }); + const outcome = await restore(); + expect(outcome).toEqual({ + ok: false, code: "restore_failed", + diagnostic: { phase: "workspace", step: "git_integration", errorCode: "unknown", httpStatus: 503, exitCode: 1 }, + }); + if (outcome.ok) throw new Error("Expected the restore fixture to fail"); + expect(JSON.stringify({ outcome, logs })).not.toContain("private-restore-"); + const diagnostics = sanitizeRunFailureDiagnostics(collectRunFailureDiagnostics({ + resultJson: { + workspaceRestoreFailure: outcome.code, + workspaceRestoreDiagnostic: { + ...outcome.diagnostic, + message: "private-restore-persisted message", cause: source, command: "private-restore-persisted command", + }, + }, + } as unknown as typeof heartbeatRuns.$inferSelect, {})); + captureRunFailure({ + taskId: "11111111-1111-4111-8111-111111111111", runId: "22222222-2222-4222-8222-222222222222", + errorMessage: "Workspace restore failed. Workspace files need recovery.", + errorCode: "workspace_restore_failed", agentAdapter: "fixture-adapter", runStatus: "failed", + exitCode: 1, signal: null, diagnostics, + }); + captureException(new Error("unrelated restore diagnostic fixture")); + await Sentry.flush(2000); + + expect(events).toHaveLength(2); + const restoreEvent = events.find((event) => (event.tags as Record)?.error_code === "workspace_restore_failed"); + expect(restoreEvent).toMatchObject({ contexts: { run_execution: { + workspaceRestoreFailure: "restore_failed", workspaceRestorePhase: "workspace", + workspaceRestoreStep: "git_integration", workspaceRestoreErrorCode: "unknown", + workspaceRestoreHttpStatus: 503, workspaceRestoreExitCode: 1, + } } }); + expect((restoreEvent?.exception as { values: unknown[] }).values).toHaveLength(1); + expect(JSON.stringify(events)).not.toContain("private-restore-"); + const unrelated = events.find((event) => event !== restoreEvent); + expect(unrelated).not.toHaveProperty("contexts.run_execution"); + expect(unrelated).not.toHaveProperty("contexts.run_failure"); + expect(unrelated).not.toHaveProperty("tags.error_code"); + }); }); diff --git a/server/src/services/__tests__/run-failure-diagnostics.test.ts b/server/src/services/__tests__/run-failure-diagnostics.test.ts index 4e1fe15a841..09618951cfa 100644 --- a/server/src/services/__tests__/run-failure-diagnostics.test.ts +++ b/server/src/services/__tests__/run-failure-diagnostics.test.ts @@ -55,6 +55,78 @@ describe("run failure diagnostics", () => { }, ); + it("copies only classified restore diagnostics from the saved result", () => { + const result = sanitizeRunFailureDiagnostics(collectRunFailureDiagnostics(run({ resultJson: { + workspaceRestoreFailure: "restore_failed", + workspaceRestoreDiagnostic: { + phase: "workspace", step: "git_integration", errorCode: "unknown", httpStatus: 503, exitCode: 1, + message: "private command failed", path: "/private/workspace", stdout: "private file contents", + cause: { code: "EIO", message: "private nested cause" }, + }, + } }), {})); + expect(result.execution).toEqual({ + workspaceRestoreFailure: "restore_failed", workspaceRestorePhase: "workspace", + workspaceRestoreStep: "git_integration", workspaceRestoreErrorCode: "unknown", + workspaceRestoreHttpStatus: 503, workspaceRestoreExitCode: 1, + }); + expect(JSON.stringify(result)).not.toContain("private"); + expect(result.exceptions).toEqual([]); + }); + + it("requires a known restore failure before reading its diagnostic", () => { + const diagnostic = { phase: "workspace", step: "git_import", errorCode: "EIO", exitCode: 1 }; + for (const code of [undefined, null, "private unknown classification"]) { + const result = collectRunFailureDiagnostics(run({ resultJson: { + workspaceRestoreFailure: code, workspaceRestoreDiagnostic: diagnostic, + } }), {}); + expect(result.execution).toEqual({}); + } + const resultJson = Object.defineProperty({}, "workspaceRestoreDiagnostic", { + get() { throw new Error("must not read unrelated diagnostic"); }, + }); + expect(collectRunFailureDiagnostics(run({ resultJson }), {}).execution).toEqual({}); + }); + + it.each([null, false, 1, "private payload", [], { phase: "private phase", errorCode: "EIO" }])( + "omits malformed workspace restore diagnostics (%j)", (value) => { + const result = collectRunFailureDiagnostics(run({ resultJson: { + workspaceRestoreFailure: "restore_failed", workspaceRestoreDiagnostic: value, + } }), {}); + expect(result.execution).toEqual({ workspaceRestoreFailure: "restore_failed" }); + expect(JSON.stringify(result)).not.toContain("private"); + }, + ); + + it.each([null, -1, Infinity, NaN, 1.5, 600, "private status", {}])( + "omits invalid restore status and exit values (%j)", (value) => { + const result = collectRunFailureDiagnostics(run({ resultJson: { + workspaceRestoreFailure: "restore_failed", + workspaceRestoreDiagnostic: { + phase: "asset", step: "private step", errorCode: "private code", httpStatus: value, exitCode: value, + }, + } }), {}); + expect(result.execution).toEqual({ + workspaceRestoreFailure: "restore_failed", workspaceRestorePhase: "asset", workspaceRestoreErrorCode: "unknown", + }); + }, + ); + + it("does not walk restore causes or expose errors from diagnostic getters", () => { + const diagnostic = Object.defineProperties({ phase: "workspace", errorCode: "EIO" }, { + step: { get() { throw new Error("private getter"); } }, + httpStatus: { get() { throw new Error("private getter"); } }, + exitCode: { get() { throw new Error("private getter"); } }, + cause: { get() { throw new Error("must not walk cause"); } }, + }); + const result = collectRunFailureDiagnostics(run({ resultJson: { + workspaceRestoreFailure: "restore_failed", workspaceRestoreDiagnostic: diagnostic, + } }), {}); + expect(result.execution).toEqual({ + workspaceRestoreFailure: "restore_failed", workspaceRestorePhase: "workspace", workspaceRestoreErrorCode: "EIO", + }); + expect(result.exceptions).toEqual([]); + }); + it("includes only bounded ACP activity fields, not tool names, identities, or output", () => { const result = sanitizeRunFailureDiagnostics(collectRunFailureDiagnostics(run({ resultJson: { acpLastEventAgeMs: 14_000_000, acpObservedEventCount: 9, acpPendingToolCount: 2, diff --git a/server/src/services/run-failure-diagnostics.ts b/server/src/services/run-failure-diagnostics.ts index f6711e4766b..24937ad9ad3 100644 --- a/server/src/services/run-failure-diagnostics.ts +++ b/server/src/services/run-failure-diagnostics.ts @@ -2,6 +2,7 @@ import type { heartbeatRuns } from "@paperclipai/db"; import { readRunCancellation } from "./run-cancellation.js"; import { WORKSPACE_RESTORE_FAILURE_CODES } from "@paperclipai/shared"; import { redactDiagnosticText } from "@paperclipai/adapter-utils/command-redaction"; +import { sanitizeWorkspaceRestoreDiagnostic } from "@paperclipai/adapter-utils/workspace-restore-diagnostics"; import { redactCurrentUserText } from "../log-redaction.js"; import { redactSensitiveText, REDACTED_EVENT_VALUE } from "../redaction.js"; @@ -165,7 +166,17 @@ export function collectRunFailureDiagnostics(run: Run, options: RunFailureReport if (typeof toolInventoryComplete === "boolean") execution.acpToolInventoryComplete = toolInventoryComplete; const restoreFailure = read(result, "workspaceRestoreFailure"); const restoreCode = WORKSPACE_RESTORE_FAILURE_CODES.find((code) => code === restoreFailure); - if (restoreCode) execution.workspaceRestoreFailure = restoreCode; + if (restoreCode) { + execution.workspaceRestoreFailure = restoreCode; + const diagnostic = sanitizeWorkspaceRestoreDiagnostic(read(result, "workspaceRestoreDiagnostic")); + if (diagnostic) { + execution.workspaceRestorePhase = diagnostic.phase; + execution.workspaceRestoreErrorCode = diagnostic.errorCode; + if (diagnostic.step) execution.workspaceRestoreStep = diagnostic.step; + if (diagnostic.httpStatus !== undefined) execution.workspaceRestoreHttpStatus = diagnostic.httpStatus; + if (diagnostic.exitCode !== undefined) execution.workspaceRestoreExitCode = diagnostic.exitCode; + } + } const adapter = scalars(options.adapterErrorMeta, [ "category", "phase", "errorName", "acpCode", "causeMessage", "retryable", "stackPreview", "status", "statusCode", "requestId",