diff --git a/doc/observability.md b/doc/observability.md index 148bbb73bc..d8c4b0e4a2 100644 --- a/doc/observability.md +++ b/doc/observability.md @@ -514,9 +514,21 @@ 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 +only. They omit error messages, raw command lines, 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 +diagnostics, without adding the original error as a cause. +For `git_integration`, optional `workspaceRestoreGitCommand` identifies the fixed +command family: `rev_parse`, `symbolic_ref`, `merge_base`, `merge_tree`, +`commit_tree`, `update_ref`, or `log`. `workspaceRestoreGitFailureKind` is +`merge_conflict`, `invalid_object`, `ref_conflict`, `permission_denied`, or +`unknown`. A merge conflict requires an uninterrupted `merge-tree --write-tree` +exit of 1 with a completed tree ID in stdout; exit 1 alone is ambiguous. +Object and ref classifications require recognized Git diagnostics; +permission denial requires an OS `EACCES` or `EPERM` code. Unrecognized or +localized messages remain `unknown`. Up to 16 KiB of stderr is inspected only +in memory; no arguments, stderr, paths, repository URLs, filenames, or ref names +enter these fields. Handled probes and successful retries emit no diagnostic. +This does not change restore behavior, retries, timeouts, or recovery policy. A caught directory-merge lock timeout also records `restoreLockOwnerState` diff --git a/doc/run-log-events.md b/doc/run-log-events.md index 20e7a395e1..1757c04fe1 100644 --- a/doc/run-log-events.md +++ b/doc/run-log-events.md @@ -327,6 +327,17 @@ 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. +The optional `step` identifies the failed restore operation. For +`phase=workspace` and `step=git_integration`, `gitCommand` identifies one fixed +command family (`rev_parse`, `symbolic_ref`, `merge_base`, `merge_tree`, +`commit_tree`, `update_ref`, or `log`). `gitFailureKind` is `merge_conflict`, +`invalid_object`, `ref_conflict`, `permission_denied`, or `unknown`; it is a +bounded diagnostic clue, not a new recovery or retry decision. Only supported +exit/OS codes and recognized Git messages produce a specific classification. +No command arguments, stderr, filenames, repository URLs, or ref names are saved. +The same closed fields persist in `workspaceRestoreDiagnostic` and are +revalidated before projection into an enabled Sentry failure report. + ## Codex resume usage snapshot The native runner retains a bounded local `harness.diagnostic` event with code diff --git a/packages/adapter-utils/src/git-workspace-sync.test.ts b/packages/adapter-utils/src/git-workspace-sync.test.ts index 5af489d9a0..c1029239a1 100644 --- a/packages/adapter-utils/src/git-workspace-sync.test.ts +++ b/packages/adapter-utils/src/git-workspace-sync.test.ts @@ -6,7 +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 { getWorkspaceRestoreDiagnostic, withWorkspaceRestoreDiagnostics, withWorkspaceRestoreStep, withWorkspaceRestoreGitCommand } from "./workspace-restore-diagnostics.js"; import { buildRemoteGitDeltaBundleScript, @@ -893,10 +893,93 @@ exit 0 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(getWorkspaceRestoreDiagnostic(error)).toEqual({ phase: "workspace", step: "git_integration", errorCode: "unknown", exitCode: expectedExit, + gitCommand: "merge_tree", gitFailureKind: "invalid_object" }); expect(await git(repo, ["rev-parse", "HEAD"])).toBe(currentHead); }); + it("identifies a real merge conflict without changing the host tip or copying Git output", async () => { + const repo = await mkdtemp(path.join(os.tmpdir(), "paperclip-git-conflict-diagnostic-")); + cleanupDirs.push(repo); + await git(repo, ["init", "-b", "host"]); + await git(repo, ["config", "user.name", "Test"]); + await git(repo, ["config", "user.email", "test@paperclip.dev"]); + await writeFile(path.join(repo, "private-filename.txt"), "base\n"); + await git(repo, ["add", "."]); + await git(repo, ["commit", "-m", "base"]); + const base = await git(repo, ["rev-parse", "HEAD"]); + await writeFile(path.join(repo, "private-filename.txt"), "host change\n"); + await git(repo, ["commit", "-am", "host"]); + const currentHead = await git(repo, ["rev-parse", "HEAD"]); + await git(repo, ["checkout", "-b", "imported", base]); + await writeFile(path.join(repo, "private-filename.txt"), "remote change\n"); + await git(repo, ["commit", "-am", "remote"]); + const importedHead = await git(repo, ["rev-parse", "HEAD"]); + await git(repo, ["checkout", "host"]); + const logs: string[] = []; + const error = await withWorkspaceRestoreDiagnostics("workspace", () => withWorkspaceRestoreStep("directory_merge", () => + withWorkspaceRestoreStep("git_integration", () => integrateImportedGitHead({ localDir: repo, importedHead }))), + async (line) => { logs.push(line); }).catch(error => error); + expect(error).toBeInstanceOf(Error); + expect(error.message).toContain("Failed to merge concurrent remote git histories"); + expect(error).not.toHaveProperty("cause"); + expect(getWorkspaceRestoreDiagnostic(error)).toEqual({ phase: "workspace", step: "git_integration", errorCode: "unknown", + exitCode: 1, gitCommand: "merge_tree", gitFailureKind: "merge_conflict" }); + expect(await git(repo, ["rev-parse", "HEAD"])).toBe(currentHead); + expect(await readFile(path.join(repo, "private-filename.txt"), "utf8")).toBe("host change\n"); + expect(logs).toHaveLength(1); + expect(logs[0]).not.toMatch(/private-filename|host change|remote change/); + for (const privateValue of [repo, currentHead, importedHead]) expect(logs[0]).not.toContain(privateValue); + }); + + it("labels a failed locked ref transaction without changing its branch or tip", async () => { + const repo = await mkdtemp(path.join(os.tmpdir(), "paperclip-git-transaction-diagnostic-")); + cleanupDirs.push(repo); + await git(repo, ["init", "-b", "private-branch"]); + await git(repo, ["config", "user.name", "Test"]); + await git(repo, ["config", "user.email", "test@paperclip.dev"]); + await git(repo, ["commit", "--allow-empty", "-m", "base"]); + const base = await git(repo, ["rev-parse", "HEAD"]); + await git(repo, ["commit", "--allow-empty", "-m", "advance"]); + const importedHead = await git(repo, ["rev-parse", "HEAD"]); + await git(repo, ["reset", "--hard", base]); + // Git owns this lock. A failed restore must leave it alone and keep its + // existing failure behavior; diagnostic collection grants no cleanup rights. + await writeFile(path.join(repo, ".git", "HEAD.lock"), "private-lock"); + const error = await withWorkspaceRestoreDiagnostics("workspace", () => withWorkspaceRestoreStep("git_integration", () => + integrateImportedGitHead({ localDir: repo, importedHead, baseline: { headCommit: base, branchName: "private-branch" } }))) + .catch(error => error); + expect(error).toMatchObject({ code: 128 }); + const diagnostic = getWorkspaceRestoreDiagnostic(error); + expect(diagnostic).toEqual({ phase: "workspace", step: "git_integration", errorCode: "unknown", exitCode: 128, + gitCommand: "update_ref", gitFailureKind: "unknown" }); + expect(await git(repo, ["rev-parse", "HEAD"])).toBe(base); + expect(await git(repo, ["symbolic-ref", "--short", "HEAD"])).toBe("private-branch"); + expect(await readFile(path.join(repo, ".git", "HEAD.lock"), "utf8")).toBe("private-lock"); + for (const privateValue of [repo, base, importedHead, "private-branch"]) expect(JSON.stringify(diagnostic)).not.toContain(privateValue); + }); + + it("recognizes a real expected-old ref mismatch without copying the ref or commit IDs", async () => { + const repo = await mkdtemp(path.join(os.tmpdir(), "paperclip-git-ref-diagnostic-")); + cleanupDirs.push(repo); + await git(repo, ["init", "-b", "private-branch"]); + await git(repo, ["config", "user.name", "Test"]); + await git(repo, ["config", "user.email", "test@paperclip.dev"]); + await git(repo, ["commit", "--allow-empty", "-m", "base"]); + const base = await git(repo, ["rev-parse", "HEAD"]); + await git(repo, ["commit", "--allow-empty", "-m", "advance"]); + const current = await git(repo, ["rev-parse", "HEAD"]); + const error = await withWorkspaceRestoreDiagnostics("workspace", () => withWorkspaceRestoreStep("git_integration", () => + withWorkspaceRestoreGitCommand("update_ref", () => runLocalGit(repo, ["update-ref", "refs/heads/private-branch", base, base])))) + .catch(error => error); + expect(error).toMatchObject({ code: 128 }); + const diagnostic = getWorkspaceRestoreDiagnostic(error); + expect(diagnostic).toEqual({ phase: "workspace", step: "git_integration", errorCode: "unknown", exitCode: 128, + gitCommand: "update_ref", gitFailureKind: "ref_conflict" }); + expect(await git(repo, ["rev-parse", "HEAD"])).toBe(current); + for (const privateValue of [repo, base, current, "private-branch"]) expect(JSON.stringify(diagnostic)).not.toContain(privateValue); + }); + 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); diff --git a/packages/adapter-utils/src/git-workspace-sync.ts b/packages/adapter-utils/src/git-workspace-sync.ts index 57bbbd06c4..fcd6bd763d 100644 --- a/packages/adapter-utils/src/git-workspace-sync.ts +++ b/packages/adapter-utils/src/git-workspace-sync.ts @@ -5,7 +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"; +import { preserveWorkspaceRestoreErrorDiagnostic, withWorkspaceRestoreGitCommand, type WorkspaceRestoreGitCommand } from "./workspace-restore-diagnostics.js"; export interface GitCommandResult { stdout: string; @@ -810,6 +810,14 @@ export function buildRemoteGitDeltaBundleScript(input: { ].filter(Boolean).join("\n"); } +// Labels are fixed at the integration call sites, never derived from Git arguments. +function runIntegrationGit( + command: WorkspaceRestoreGitCommand, + ...args: Parameters +): Promise { + return withWorkspaceRestoreGitCommand(command, () => runLocalGit(...args)); +} + /** * Preserve imported work whose history does not connect to the local one. * @@ -831,11 +839,11 @@ export async function createUnrelatedHistoryGraftCommit(input: { importedHead: string; syncLabel: string; }): Promise { - const importedTree = (await runLocalGit(input.localDir, ["rev-parse", `${input.importedHead}^{tree}`], { + const importedTree = (await runIntegrationGit("rev_parse", input.localDir, ["rev-parse", `${input.importedHead}^{tree}`], { timeout: 10_000, maxBuffer: 16 * 1024, })).stdout.trim(); - const importedMessage = (await runLocalGit(input.localDir, ["log", "-1", "--format=%B", input.importedHead], { + const importedMessage = (await runIntegrationGit("log", input.localDir, ["log", "-1", "--format=%B", input.importedHead], { timeout: 10_000, maxBuffer: 256 * 1024, })).stdout; @@ -844,8 +852,8 @@ export async function createUnrelatedHistoryGraftCommit(input: { "", `(${input.syncLabel} graft ${input.importedHead.slice(0, 12)}: imported history shares no ancestor with ${input.currentHead.slice(0, 12)})`, ].join("\n"); - const graftCommit = await runLocalGit( - input.localDir, + const graftCommit = await runIntegrationGit( + "commit_tree", input.localDir, [...GIT_SYNC_COMMIT_IDENTITY_ARGS, "commit-tree", importedTree, "-p", input.currentHead, "-m", message], { timeout: 60_000, @@ -865,7 +873,7 @@ async function updateLocalGitHead(input: { // Git holds HEAD.lock (and the branch lock when attached) until commit/abort, // so a checkout cannot redirect the write after this check. --no-deref also // prevents a detached write from following a newly attached branch. - await new Promise((resolve, reject) => { + await withWorkspaceRestoreGitCommand("update_ref", () => new Promise((resolve, reject) => { let identityError: unknown; let prepared = false; let output = ""; @@ -886,7 +894,7 @@ async function updateLocalGitHead(input: { prepared = true; void (async () => { try { - const branchName = (await runLocalGit(input.localDir, ["symbolic-ref", "--quiet", "--short", "HEAD"], { + const branchName = (await runIntegrationGit("symbolic_ref", input.localDir, ["symbolic-ref", "--quiet", "--short", "HEAD"], { timeout: 10_000, }).catch((error) => { if (error.code === 1) return { stdout: "" }; @@ -909,7 +917,7 @@ async function updateLocalGitHead(input: { "prepare", "", ].join("\n")); - }); + })); } export async function integrateImportedGitHead(input: { @@ -925,8 +933,8 @@ export async function integrateImportedGitHead(input: { for (let attempt = 0; attempt < 5; attempt += 1) { const snapshot = { - headCommit: (await runLocalGit(input.localDir, ["rev-parse", "HEAD"])).stdout.trim(), - branchName: (await runLocalGit(input.localDir, ["symbolic-ref", "--quiet", "--short", "HEAD"]).catch((error) => { + headCommit: (await runIntegrationGit("rev_parse", input.localDir, ["rev-parse", "HEAD"])).stdout.trim(), + branchName: (await runIntegrationGit("symbolic_ref", input.localDir, ["symbolic-ref", "--quiet", "--short", "HEAD"]).catch((error) => { if (error.code === 1) return { stdout: "" }; throw error; })).stdout.trim() || null, @@ -943,7 +951,7 @@ export async function integrateImportedGitHead(input: { // (timeout, missing object, repository error) must keep failing the // integration instead of silently rewriting the tip. let noCommonAncestor = false; - const mergeBase = await runLocalGit(input.localDir, ["merge-base", currentHead, input.importedHead], { + const mergeBase = await runIntegrationGit("merge_base", input.localDir, ["merge-base", currentHead, input.importedHead], { timeout: 10_000, maxBuffer: 16 * 1024, }).catch((error: unknown) => { @@ -999,7 +1007,7 @@ export async function integrateImportedGitHead(input: { let mergedTree; try { - mergedTree = await runLocalGit(input.localDir, ["merge-tree", "--write-tree", currentHead, input.importedHead], { + mergedTree = await runIntegrationGit("merge_tree", input.localDir, ["merge-tree", "--write-tree", currentHead, input.importedHead], { timeout: 60_000, maxBuffer: 256 * 1024, }); @@ -1014,8 +1022,8 @@ export async function integrateImportedGitHead(input: { throw new Error("Failed to compute a merged git tree for workspace restore."); } - const mergeCommit = await runLocalGit( - input.localDir, + const mergeCommit = await runIntegrationGit( + "commit_tree", input.localDir, [ ...GIT_SYNC_COMMIT_IDENTITY_ARGS, "commit-tree", diff --git a/packages/adapter-utils/src/workspace-restore-diagnostics.test.ts b/packages/adapter-utils/src/workspace-restore-diagnostics.test.ts index ba7768897e..39e82e1929 100644 --- a/packages/adapter-utils/src/workspace-restore-diagnostics.test.ts +++ b/packages/adapter-utils/src/workspace-restore-diagnostics.test.ts @@ -2,7 +2,7 @@ import { describe, expect, it, vi } from "vitest"; import { classifyWorkspaceRestoreFailure } from "./workspace-restore-merge.js"; import { getWorkspaceRestoreDiagnostic, preserveWorkspaceRestoreErrorDiagnostic, recordWorkspaceRestoreDiagnostic, - withWorkspaceRestoreDiagnostics, withWorkspaceRestoreStep, + sanitizeWorkspaceRestoreDiagnostic, withWorkspaceRestoreDiagnostics, withWorkspaceRestoreStep, withWorkspaceRestoreGitCommand, type WorkspaceRestoreDiagnostic, } from "./workspace-restore-diagnostics.js"; @@ -164,3 +164,136 @@ describe("workspace restore diagnostics", () => { expect(sink).toHaveBeenCalledTimes(3); }); }); + +describe("Git integration diagnostic privacy and attribution", () => { + async function capture(command: Parameters[0], error: unknown) { + let receipt: WorkspaceRestoreDiagnostic | undefined; + const originalProperties = error && typeof error === "object" ? Object.getOwnPropertyDescriptors(error) : undefined; + await expect(withWorkspaceRestoreDiagnostics("workspace", () => withWorkspaceRestoreStep("git_integration", () => + withWorkspaceRestoreGitCommand(command, async () => { throw error; })), undefined, + (value) => { receipt = value; })).rejects.toBe(error); + if (originalProperties) expect(Object.getOwnPropertyDescriptors(error)).toEqual(originalProperties); + return receipt; + } + + it.each([ + ["merge_tree", { code: 1, stdout: "a".repeat(40) + "\nprivate-file", stderr: "private-conflict-body" }, "merge_conflict"], + ["merge_tree", { code: 1 }, "unknown"], + ["merge_tree", { code: 1, stderr: `merge-tree: ${"a".repeat(40)} - not something we can merge\n` }, "invalid_object"], + ["merge_tree", { code: 1, signal: "SIGTERM" }, "unknown"], + ["merge_tree", { code: 1, killed: true }, "unknown"], + ["merge_tree", { code: 128, stderr: "fatal: Not a valid object name private-object" }, "invalid_object"], + ["merge_tree", { code: 128, stderr: `merge-tree: ${"a".repeat(40)} - not something we can merge\n` }, "invalid_object"], + ["merge_base", { code: 128, stderr: "fatal: Not a valid commit name private-object" }, "invalid_object"], + ["rev_parse", { code: 128, stderr: "fatal: bad object private-object" }, "invalid_object"], + ["update_ref", { code: 128, stderr: `fatal: update_ref failed for ref 'private-ref': cannot lock ref 'private-ref': is at ${"a".repeat(40)} but expected ${"b".repeat(40)}\n` }, "ref_conflict"], + ["update_ref", { code: 128, stderr: "fatal: Unable to create 'private-path.lock': File exists." }, "unknown"], + ["update_ref", { code: "EACCES", stderr: "private-path" }, "permission_denied"], + ["commit_tree", { code: "EPERM" }, "permission_denied"], + ["commit_tree", { code: 1, stderr: "Permission denied private-path" }, "unknown"], + ["symbolic_ref", { code: 1 }, "unknown"], + ["merge_base", { code: 1 }, "unknown"], + ["log", { code: 128, stderr: "localized or unrecognized private-text" }, "unknown"], + ["log", { code: 128, stderr: "x".repeat(16 * 1024) + "\nfatal: bad object private-object" }, "unknown"], + ["merge_tree", "private-string", "unknown"], + ] as const)("classifies only supported %s evidence (%j)", async (command, error, kind) => { + const receipt = await capture(command, error); + expect(receipt).toMatchObject({ gitCommand: command, gitFailureKind: kind }); + expect(JSON.stringify(receipt)).not.toContain("private-"); + expect(Object.keys(receipt!).sort()).toEqual([ + "phase", "step", "errorCode", "gitCommand", "gitFailureKind", + ...(typeof error === "object" && typeof error.code === "number" ? ["exitCode"] : []), + ].sort()); + }); + + it("treats throwing stderr getters as unknown and never replaces the original error", async () => { + const error = Object.defineProperty(new Error("private-original"), "stderr", { get() { throw new Error("private-getter"); } }); + expect(await capture("commit_tree", error)).toEqual({ phase: "workspace", step: "git_integration", + errorCode: "unknown", gitCommand: "commit_tree", gitFailureKind: "unknown" }); + }); + + it("retains a nested command across wrappers and nested task diagnostics", async () => { + const source = Object.assign(new Error("private-source"), { code: 1, stdout: "a".repeat(40) + "\n" }); + const wrapper = new Error("private-wrapper"); + const sink = vi.fn(); + await expect(withWorkspaceRestoreDiagnostics("workspace", () => withWorkspaceRestoreStep("directory_merge", () => + withWorkspaceRestoreDiagnostics("workspace", () => withWorkspaceRestoreStep("git_integration", async () => { + try { await withWorkspaceRestoreGitCommand("merge_tree", async () => { throw source; }); } + catch (error) { throw preserveWorkspaceRestoreErrorDiagnostic(wrapper, error); } + }), sink)), sink)).rejects.toBe(wrapper); + expect(getWorkspaceRestoreDiagnostic(wrapper)).toEqual({ phase: "workspace", step: "git_integration", + errorCode: "unknown", exitCode: 1, gitCommand: "merge_tree", gitFailureKind: "merge_conflict" }); + expect(wrapper).not.toHaveProperty("cause"); + expect(sink).toHaveBeenCalledTimes(1); + expect(sink.mock.calls[0][0]).not.toContain("private-"); + }); + + it("keeps a nested identity probe failure instead of relabelling it as its ref transaction", async () => { + const error = Object.assign(new Error("private-probe"), { code: 128, stderr: "private-probe-output" }); + await expect(withWorkspaceRestoreDiagnostics("workspace", () => withWorkspaceRestoreStep("git_integration", () => + withWorkspaceRestoreGitCommand("update_ref", () => + withWorkspaceRestoreGitCommand("symbolic_ref", async () => { throw error; }))))).rejects.toBe(error); + expect(getWorkspaceRestoreDiagnostic(error)).toEqual({ phase: "workspace", step: "git_integration", + errorCode: "unknown", exitCode: 128, gitCommand: "symbolic_ref", gitFailureKind: "unknown" }); + }); + + it("does not retain a handled command's label on a later failed step or retry", async () => { + const error = Object.assign(new Error("same error"), { code: 1 }); + await expect(withWorkspaceRestoreDiagnostics("workspace", async () => { + await withWorkspaceRestoreStep("git_integration", () => + withWorkspaceRestoreGitCommand("merge_tree", async () => { throw error; })).catch(() => {}); + await withWorkspaceRestoreStep("index_reset", async () => { throw error; }); + })).rejects.toBe(error); + expect(getWorkspaceRestoreDiagnostic(error)).toEqual({ phase: "workspace", step: "index_reset", errorCode: "unknown", exitCode: 1 }); + await expect(withWorkspaceRestoreDiagnostics("workspace", () => withWorkspaceRestoreStep("git_integration", async () => { + await withWorkspaceRestoreGitCommand("merge_tree", async () => { throw error; }).catch(() => {}); + await withWorkspaceRestoreGitCommand("update_ref", async () => { throw error; }); + }))).rejects.toBe(error); + expect(getWorkspaceRestoreDiagnostic(error)).toMatchObject({ gitCommand: "update_ref", gitFailureKind: "unknown" }); + }); + + it("retains the selected parallel task's command when two errors share identity", async () => { + const error = Object.assign(new Error("shared"), { code: 1, stdout: "a".repeat(40) + "\n" }); + const snapshots: WorkspaceRestoreDiagnostic[] = []; + await expect(withWorkspaceRestoreDiagnostics("workspace", async () => { + await Promise.allSettled((["merge_tree", "update_ref"] as const).map((command, index) => + withWorkspaceRestoreDiagnostics("workspace", () => withWorkspaceRestoreStep("git_integration", () => + withWorkspaceRestoreGitCommand(command, async () => { throw error; })), undefined, + (receipt) => { snapshots[index] = receipt; }))); + recordWorkspaceRestoreDiagnostic(error, snapshots[0]); + throw error; + })).rejects.toBe(error); + expect(snapshots.map(value => value.gitCommand)).toEqual(["merge_tree", "update_ref"]); + expect(getWorkspaceRestoreDiagnostic(error)).toMatchObject({ gitCommand: "merge_tree", gitFailureKind: "merge_conflict" }); + }); + + it("leaves successful or handled commands silent and preserves values", async () => { + const sink = vi.fn(); + const value = {}; + expect(await withWorkspaceRestoreDiagnostics("workspace", () => withWorkspaceRestoreStep("git_integration", async () => { + await withWorkspaceRestoreGitCommand("symbolic_ref", async () => { throw { code: 1 }; }).catch(() => {}); + return withWorkspaceRestoreGitCommand("update_ref", async () => value); + }), sink)).toBe(value); + expect(sink).not.toHaveBeenCalled(); + const error = new Error("outside restore"); + await expect(withWorkspaceRestoreGitCommand("log", async () => { throw error; })).rejects.toBe(error); + expect(getWorkspaceRestoreDiagnostic(error)).toBeUndefined(); + }); + + it.each([ + { phase: "asset", step: "git_integration", gitCommand: "merge_tree" }, + { phase: "workspace", step: "index_reset", gitCommand: "merge_tree" }, + { phase: "workspace", step: "git_integration", gitCommand: "private-command" }, + ])("does not decode Git metadata outside the closed integration contract (%j)", (value) => { + const diagnostic = sanitizeWorkspaceRestoreDiagnostic({ ...value, gitFailureKind: "merge_conflict" }); + expect(diagnostic).not.toHaveProperty("gitCommand"); + expect(diagnostic).not.toHaveProperty("gitFailureKind"); + }); + + it("revalidates persisted command and failure labels without copying extra fields", () => { + expect(sanitizeWorkspaceRestoreDiagnostic({ phase: "workspace", step: "git_integration", gitCommand: "log", + gitFailureKind: "private-reason", stderr: "private-text", args: ["private-args"] })).toEqual({ + phase: "workspace", step: "git_integration", errorCode: "unknown", gitCommand: "log", gitFailureKind: "unknown", + }); + }); +}); diff --git a/packages/adapter-utils/src/workspace-restore-diagnostics.ts b/packages/adapter-utils/src/workspace-restore-diagnostics.ts index 4b3312313e..afb8d659c9 100644 --- a/packages/adapter-utils/src/workspace-restore-diagnostics.ts +++ b/packages/adapter-utils/src/workspace-restore-diagnostics.ts @@ -7,12 +7,22 @@ const RESTORE_STEPS = new Set([ "directory_merge", "git_integration", "index_reset", "git_ref_cleanup", "asset_restore", ] as const); export type WorkspaceRestoreStep = typeof RESTORE_STEPS extends Set ? T : never; +const GIT_COMMANDS = new Set([ + "rev_parse", "symbolic_ref", "merge_base", "merge_tree", "commit_tree", "update_ref", "log", +] as const); +export type WorkspaceRestoreGitCommand = typeof GIT_COMMANDS extends Set ? T : never; +const GIT_FAILURE_KINDS = new Set([ + "merge_conflict", "invalid_object", "ref_conflict", "permission_denied", "unknown", +] as const); +type GitFailureKind = typeof GIT_FAILURE_KINDS extends Set ? T : never; export interface WorkspaceRestoreDiagnostic { phase: RestorePhase; step?: WorkspaceRestoreStep; errorCode: string; httpStatus?: number; exitCode?: number; + gitCommand?: WorkspaceRestoreGitCommand; + gitFailureKind?: GitFailureKind; } const ERROR_CODES = new Set([ "ENOENT", "EACCES", "EPERM", "ENOSPC", "EIO", "EXDEV", "ENOTDIR", "EISDIR", @@ -23,7 +33,12 @@ type ErrorDiagnostic = Pick; + failures: Map; } const activeDiagnostic = new AsyncLocalStorage(); // Never attach a raw cause to an error just to retain a numeric Git exit code. @@ -45,6 +60,47 @@ function boundedInteger(value: unknown, minimum: number, maximum: number): numbe ? value : undefined; } +/** Inspect bounded stderr only in memory; never persist Git text or arguments. */ +function gitFailureKind(command: WorkspaceRestoreGitCommand, error: unknown): GitFailureKind { + if (!error || typeof error !== "object") return "unknown"; + const value = error as Record; + const code = readField(value, "code"); + if (code === "EACCES" || code === "EPERM") return "permission_denied"; + if (readField(value, "killed") || readField(value, "signal")) return "unknown"; + const stderr = readField(value, "stderr"); + const text = typeof stderr === "string" ? stderr.slice(0, 16 * 1024) : ""; + if (command === "update_ref" && /cannot lock ref '[^'\r\n]+': is at [a-f0-9]{40,64} but expected [a-f0-9]{40,64}(?:\s|$)/m.test(text)) { + return "ref_conflict"; + } + if (/^fatal: (?:bad object |Not a valid object name |not a valid object name |Not a valid commit name )/m.test(text) + || (command === "merge_tree" && /^merge-tree: [a-f0-9]{40,64} - not something we can merge\s*$/m.test(text))) { + return "invalid_object"; + } + // merge-tree --write-tree prints the merged tree before reporting conflicts. + // Exit 1 alone is insufficient: some Git versions also use it for bad objects. + const stdout = readField(value, "stdout"); + if (command === "merge_tree" && code === 1 && typeof stdout === "string" + && /^(?:[a-f0-9]{40}|[a-f0-9]{64})\r?\n/.test(stdout.slice(0, 66))) return "merge_conflict"; + return "unknown"; +} + +/** Annotate the original thrown error in its restore scope; never change it. */ +export async function withWorkspaceRestoreGitCommand(command: WorkspaceRestoreGitCommand, operation: () => Promise): Promise { + const scope = activeDiagnostic.getStore(); + if (!scope?.active || !GIT_COMMANDS.has(command)) return await operation(); + const started = scope.sequence; + try { + return await operation(); + } catch (error) { + // A ref transaction can fail in its nested branch-identity probe. Retain + // that more specific command, while later retries get fresh attribution. + if (scope.active && (scope.failures.get(error)?.sequence ?? -1) <= started) scope.failures.set(error, { + sequence: ++scope.sequence, gitCommand: command, gitFailureKind: gitFailureKind(command, error), + }); + throw error; + } +} + /** Only fixed codes and bounded numbers may enter the company-readable run log. */ function diagnostic(error: unknown): ErrorDiagnostic { const result: ErrorDiagnostic = { errorCode: "unknown" }; @@ -91,6 +147,10 @@ export function sanitizeWorkspaceRestoreDiagnostic(value: unknown): WorkspaceRes const code = readField(record, "errorCode"); const httpStatus = boundedInteger(readField(record, "httpStatus"), 400, 599); const exitCode = boundedInteger(readField(record, "exitCode"), 1, 255); + const gitCommand = readField(record, "gitCommand"); + const gitFailureKind = readField(record, "gitFailureKind"); + const hasGitCommand = phase === "workspace" && step === "git_integration" + && typeof gitCommand === "string" && GIT_COMMANDS.has(gitCommand as WorkspaceRestoreGitCommand); return { phase, ...(typeof step === "string" && RESTORE_STEPS.has(step as WorkspaceRestoreStep) @@ -98,6 +158,11 @@ export function sanitizeWorkspaceRestoreDiagnostic(value: unknown): WorkspaceRes errorCode: typeof code === "string" && ERROR_CODES.has(code) ? code : "unknown", ...(httpStatus !== undefined ? { httpStatus } : {}), ...(exitCode !== undefined ? { exitCode } : {}), + ...(hasGitCommand ? { + gitCommand: gitCommand as WorkspaceRestoreGitCommand, + gitFailureKind: typeof gitFailureKind === "string" && GIT_FAILURE_KINDS.has(gitFailureKind as GitFailureKind) + ? gitFailureKind as GitFailureKind : "unknown", + } : {}), }; } @@ -116,7 +181,10 @@ export function recordWorkspaceRestoreDiagnostic(error: unknown, diagnostic: Wor } const scope = activeDiagnostic.getStore(); if (scope?.active) { - if (safe?.step) scope.failures.set(error, { sequence: ++scope.sequence, step: safe.step }); + if (safe?.step) scope.failures.set(error, { + sequence: ++scope.sequence, step: safe.step, + ...(safe.gitCommand ? { gitCommand: safe.gitCommand, gitFailureKind: safe.gitFailureKind } : {}), + }); else scope.failures.delete(error); } } @@ -131,8 +199,13 @@ export async function withWorkspaceRestoreStep(step: WorkspaceRestoreStep, op } 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 }); + if (scope.active) { + const failure = scope.failures.get(error); + if (failure && failure.sequence > started) { + failure.step ??= step; + } else { + scope.failures.set(error, { sequence: ++scope.sequence, step }); + } } throw error; } @@ -154,10 +227,14 @@ export async function withWorkspaceRestoreDiagnostics( try { return await operation(); } catch (error) { - const step = scope.failures.get(error)?.step; - const fields = { phase, ...(step ? { step } : {}), ...diagnostic(error) }; + const failure = scope.failures.get(error); + const step = failure?.step; + const fields = { phase, ...(step ? { step } : {}), ...diagnostic(error), + ...(phase === "workspace" && step === "git_integration" && failure?.gitCommand + ? { gitCommand: failure.gitCommand, gitFailureKind: failure.gitFailureKind } : {}), + }; recordWorkspaceRestoreDiagnostic(error, fields); - if (parent?.active && step) parent.failures.set(error, { sequence: ++parent.sequence, step }); + if (parent?.active && step) parent.failures.set(error, { ...failure, sequence: ++parent.sequence, step }); try { onDiagnostic?.({ ...fields }); } catch { /* Diagnostic consumers cannot replace a restore failure. */ } diff --git a/packages/adapter-utils/src/workspace-restore-result.test.ts b/packages/adapter-utils/src/workspace-restore-result.test.ts index f3ebfa42da..3c3adb9df1 100644 --- a/packages/adapter-utils/src/workspace-restore-result.test.ts +++ b/packages/adapter-utils/src/workspace-restore-result.test.ts @@ -1,7 +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"; +import { withWorkspaceRestoreDiagnostics, withWorkspaceRestoreStep, withWorkspaceRestoreGitCommand } from "./workspace-restore-diagnostics.js"; const completed: AdapterExecutionResult = { exitCode: 0, signal: null, timedOut: false, @@ -23,6 +23,22 @@ describe("workspace restore settlement", () => { expect(JSON.stringify(result)).not.toContain("private-restore-"); }); + it("persists only bounded Git evidence without replacing the completed model result", async () => { + const error = Object.assign(new Error("private-git-command /private/workspace"), { + code: 1, stdout: "a".repeat(40) + "\nprivate-git-file contents", stderr: "private-git-refs", + }); + const result = await withWorkspaceRestore(async () => completed, () => withWorkspaceRestoreDiagnostics("workspace", () => + withWorkspaceRestoreStep("git_integration", () => withWorkspaceRestoreGitCommand("merge_tree", async () => { throw error; })))); + expect(result).toMatchObject({ + errorCode: "workspace_restore_failed", exitCode: completed.exitCode, summary: completed.summary, usage: completed.usage, + resultJson: { requestId: "request", workspaceRestoreFailure: "restore_failed", + workspaceRestoreDiagnostic: { phase: "workspace", step: "git_integration", errorCode: "unknown", exitCode: 1, + gitCommand: "merge_tree", gitFailureKind: "merge_conflict" }, + executionBeforeRestore: { exitCode: 0, timedOut: false, errorCode: null } }, + }); + expect(JSON.stringify(result)).not.toContain("private-git-"); + }); + 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/server/src/__tests__/run-failure-sentry-real-sdk.test.ts b/server/src/__tests__/run-failure-sentry-real-sdk.test.ts index 0ecd025d3a..fd51830402 100644 --- a/server/src/__tests__/run-failure-sentry-real-sdk.test.ts +++ b/server/src/__tests__/run-failure-sentry-real-sdk.test.ts @@ -6,6 +6,7 @@ import { preserveWorkspaceRestoreErrorDiagnostic, withWorkspaceRestoreDiagnostics, withWorkspaceRestoreStep, + withWorkspaceRestoreGitCommand, } from "@paperclipai/adapter-utils/workspace-restore-diagnostics"; import { createWorkspaceRestoreTeardown } from "@paperclipai/adapter-utils/workspace-restore-teardown"; @@ -232,17 +233,20 @@ describe.skipIf(!sentryPackage)("run failure context with the real Sentry SDK", 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", + code: 1, statusCode: 503, stdout: "a".repeat(40) + "\nprivate-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); + const wrapper = new Error("private-restore-Git wrapper"); 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), + withWorkspaceRestoreStep("git_integration", async () => { + try { await withWorkspaceRestoreGitCommand("merge_tree", async () => { throw source; }); } + catch (error) { throw preserveWorkspaceRestoreErrorDiagnostic(wrapper, error); } + })), onProgress), }, onLog: async (_stream, line) => { logs.push(line); }, startMessage: "Restoring workspace\n", @@ -251,7 +255,8 @@ describe.skipIf(!sentryPackage)("run failure context with the real Sentry SDK", const outcome = await restore(); expect(outcome).toEqual({ ok: false, code: "restore_failed", - diagnostic: { phase: "workspace", step: "git_integration", errorCode: "unknown", httpStatus: 503, exitCode: 1 }, + diagnostic: { phase: "workspace", step: "git_integration", errorCode: "unknown", httpStatus: 503, exitCode: 1, + gitCommand: "merge_tree", gitFailureKind: "merge_conflict" }, }); if (outcome.ok) throw new Error("Expected the restore fixture to fail"); expect(JSON.stringify({ outcome, logs })).not.toContain("private-restore-"); @@ -279,6 +284,7 @@ describe.skipIf(!sentryPackage)("run failure context with the real Sentry SDK", workspaceRestoreFailure: "restore_failed", workspaceRestorePhase: "workspace", workspaceRestoreStep: "git_integration", workspaceRestoreErrorCode: "unknown", workspaceRestoreHttpStatus: 503, workspaceRestoreExitCode: 1, + workspaceRestoreGitCommand: "merge_tree", workspaceRestoreGitFailureKind: "merge_conflict", } } }); expect((restoreEvent?.exception as { values: unknown[] }).values).toHaveLength(1); expect(JSON.stringify(events)).not.toContain("private-restore-"); diff --git a/server/src/services/__tests__/run-failure-diagnostics.test.ts b/server/src/services/__tests__/run-failure-diagnostics.test.ts index 09618951cf..98a585aa7c 100644 --- a/server/src/services/__tests__/run-failure-diagnostics.test.ts +++ b/server/src/services/__tests__/run-failure-diagnostics.test.ts @@ -60,6 +60,7 @@ describe("run failure diagnostics", () => { workspaceRestoreFailure: "restore_failed", workspaceRestoreDiagnostic: { phase: "workspace", step: "git_integration", errorCode: "unknown", httpStatus: 503, exitCode: 1, + gitCommand: "merge_tree", gitFailureKind: "merge_conflict", message: "private command failed", path: "/private/workspace", stdout: "private file contents", cause: { code: "EIO", message: "private nested cause" }, }, @@ -68,11 +69,25 @@ describe("run failure diagnostics", () => { workspaceRestoreFailure: "restore_failed", workspaceRestorePhase: "workspace", workspaceRestoreStep: "git_integration", workspaceRestoreErrorCode: "unknown", workspaceRestoreHttpStatus: 503, workspaceRestoreExitCode: 1, + workspaceRestoreGitCommand: "merge_tree", workspaceRestoreGitFailureKind: "merge_conflict", }); expect(JSON.stringify(result)).not.toContain("private"); expect(result.exceptions).toEqual([]); }); + it.each([ + { phase: "asset", step: "git_integration", gitCommand: "merge_tree", gitFailureKind: "merge_conflict" }, + { phase: "workspace", step: "git_import", gitCommand: "merge_tree", gitFailureKind: "merge_conflict" }, + { phase: "workspace", step: "git_integration", gitCommand: "private-command", gitFailureKind: "private-output" }, + ])("omits unrelated or unrecognized persisted Git labels (%j)", (diagnostic) => { + const result = collectRunFailureDiagnostics(run({ resultJson: { + workspaceRestoreFailure: "restore_failed", workspaceRestoreDiagnostic: diagnostic, + } }), {}); + expect(result.execution).not.toHaveProperty("workspaceRestoreGitCommand"); + expect(result.execution).not.toHaveProperty("workspaceRestoreGitFailureKind"); + expect(JSON.stringify(result)).not.toContain("private-"); + }); + 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"]) { diff --git a/server/src/services/run-failure-diagnostics.ts b/server/src/services/run-failure-diagnostics.ts index 24937ad9ad..5a83450794 100644 --- a/server/src/services/run-failure-diagnostics.ts +++ b/server/src/services/run-failure-diagnostics.ts @@ -175,6 +175,8 @@ export function collectRunFailureDiagnostics(run: Run, options: RunFailureReport if (diagnostic.step) execution.workspaceRestoreStep = diagnostic.step; if (diagnostic.httpStatus !== undefined) execution.workspaceRestoreHttpStatus = diagnostic.httpStatus; if (diagnostic.exitCode !== undefined) execution.workspaceRestoreExitCode = diagnostic.exitCode; + if (diagnostic.gitCommand) execution.workspaceRestoreGitCommand = diagnostic.gitCommand; + if (diagnostic.gitFailureKind) execution.workspaceRestoreGitFailureKind = diagnostic.gitFailureKind; } } const adapter = scalars(options.adapterErrorMeta, [