From 022eb7c2eb53a3d2e498fb223cea218efe667c3a Mon Sep 17 00:00:00 2001 From: Devin Foley Date: Fri, 6 Nov 2026 21:18:23 -0800 Subject: [PATCH] Preserve restore diagnostics through checkout wrappers Nested saved-commit failures lost their closed diagnostic in checkout preparation and contents restoration. Preserve the normalized failure around publication hooks and retain its inner step. Keep process codes and timeout flags privately for classification without attaching raw command causes or changing generic error properties. Recognize local Git file errors before authenticated transport messages. A numeric Git exit caused by FETCH_HEAD or local configuration permissions now reports filesystem rather than rejected credentials. Real Git/Postgres regressions reproduce the lost nested commit and clone-timeout diagnostics before the fix. All seven affected suites pass (288 tests), including full expiration coverage for the controlled clone-failure seam. Layered server typechecking passes on supported Node 25.6.1. Whitespace and secrets/PII scans pass. Co-Authored-By: Paperclip --- doc/DEVELOPING.md | 4 ++ .../git-workspace-expiration.test.ts | 51 +++++++++++++++++++ ...t-workspace-restoration-diagnostic.test.ts | 4 ++ .../managed-repository-checkout.test.ts | 25 ++++++++- server/src/services/git-workspace-contents.ts | 48 +++++++++-------- .../src/services/git-workspace-restoration.ts | 25 ++++++--- .../services/managed-repository-checkout.ts | 28 +++++++++- 7 files changed, 153 insertions(+), 32 deletions(-) diff --git a/doc/DEVELOPING.md b/doc/DEVELOPING.md index 1545846ac38..aa0ecf30a0a 100644 --- a/doc/DEVELOPING.md +++ b/doc/DEVELOPING.md @@ -4539,6 +4539,10 @@ pair. Both are closed values: `step` names the failed stage (for example failure class (`remote_unavailable`, `auth_rejected`, `commit_unavailable`, `timeout`, `verification`, `filesystem`, `git_command` or `unknown`). They never contain paths, repository URLs or Git output. +Nested checkout failures retain their original restore step. The checkout +wrapper keeps process error codes and timeout flags privately for classification; +it does not attach the raw command error as a cause or add those flags to the +public error object. Both enable switches default off. Rollback disables further expiration while leaving saved recipes and restoration support intact. Recovery archives, runner diff --git a/server/src/__tests__/git-workspace-expiration.test.ts b/server/src/__tests__/git-workspace-expiration.test.ts index 5f3ea1f2c89..53a4c36776e 100644 --- a/server/src/__tests__/git-workspace-expiration.test.ts +++ b/server/src/__tests__/git-workspace-expiration.test.ts @@ -28,6 +28,21 @@ import * as activityLog from "../services/activity-log.js"; import { readGitCacheRestoration } from "../services/git-workspace-restoration.js"; import { assertReproducibleManagedCheckout, gitCacheExpirationLockLocations, readGitCacheExpiration } from "../services/git-workspace-restoration.js"; +const cloneFailure = vi.hoisted(() => ({ error: null as Error | null })); +vi.mock("node:child_process", async importOriginal => { + const actual = await importOriginal(); + const { promisify } = await import("node:util"); + const execute = promisify(actual.execFile); + const execFile = (...args: unknown[]) => Reflect.apply(actual.execFile, undefined, args); + Object.defineProperty(execFile, promisify.custom, { value: (...args: unknown[]) => { + if (cloneFailure.error && args[0] === "git" && Array.isArray(args[1]) && args[1].includes("clone")) { + return Promise.reject(cloneFailure.error); + } + return Reflect.apply(execute, undefined, args); + } }); + return { ...actual, execFile }; +}); + const gitEnv = { ...process.env, GIT_AUTHOR_NAME: "Fixture", GIT_AUTHOR_EMAIL: "fixture@example.test", GIT_COMMITTER_NAME: "Fixture", GIT_COMMITTER_EMAIL: "fixture@example.test", GIT_TERMINAL_PROMPT: "0" }; const git = (cwd: string, ...args: string[]) => execFileSync("git", args, { cwd, env: gitEnv, encoding: "utf8", stdio: ["ignore", "pipe", "pipe"] }).trim(); @@ -1012,6 +1027,42 @@ describeEmbeddedPostgres("reconstructable Git expiration", () => { expect((await f.row()).metadata?.gitCacheRestoration).toMatchObject({ commit: f.commitA }); }); + it("retains the nested saved commit failure through checkout preparation and reopening", async () => { + const f = await nestedFixture(true, false); + expect((await f.expire({ expireBase: false })).expired).toBe(true); + git(f.nestedSource, "checkout", "--orphan", "rewritten"); + await fs.writeFile(path.join(f.nestedSource, "nested.txt"), "rewritten\n"); + git(f.nestedSource, "add", "."); git(f.nestedSource, "commit", "-m", "Rewritten nested history"); + git(f.nestedSource, "push", "--force", f.nestedRemote, "rewritten:main"); + git(f.nestedRemote, "reflog", "expire", "--expire=now", "--all"); + git(f.nestedRemote, "gc", "--prune=now"); + const failed = await f.reopen(); + expect(failed).toMatchObject({ ok: false, code: "rebuild_failed", + diagnostic: { step: "saved_commit", kind: "commit_unavailable" } }); + expect(JSON.stringify(failed)).not.toContain(root); + expect(JSON.stringify(failed)).not.toContain(f.nestedA); + expect((await f.row()).metadata?.gitCacheRestoration).toMatchObject({ contents: { + checkouts: [expect.objectContaining({ commit: f.nestedA })], + } }); + }); + + it("retains a managed clone timeout through preparation and reopening", async () => { + const f = await fixture(true, false); + expect((await f.expire({ expireBase: false })).expired).toBe(true); + await fs.rename(f.baseCwd, `${f.baseCwd}.offline`); + cloneFailure.error = Object.assign(new Error("clone timed out; fixture credential must stay private"), { + killed: true, signal: "SIGTERM", code: null, + }); + let failed: Awaited>; + try { failed = await f.reopen(); } + finally { cloneFailure.error = null; } + expect(failed).toMatchObject({ ok: false, code: "rebuild_failed", + diagnostic: { step: "source_materialize", kind: "timeout" } }); + expect(JSON.stringify(failed)).not.toContain(root); + expect(JSON.stringify(failed)).not.toContain("fixture credential"); + expect(await f.reopen()).toMatchObject({ ok: true }); + }); + it("serializes concurrent reopening after deletion and publishes one transition", async () => { const f = await fixture(); let release!: () => void, entered!: () => void; diff --git a/server/src/__tests__/git-workspace-restoration-diagnostic.test.ts b/server/src/__tests__/git-workspace-restoration-diagnostic.test.ts index 04251308f35..bc92845cc87 100644 --- a/server/src/__tests__/git-workspace-restoration-diagnostic.test.ts +++ b/server/src/__tests__/git-workspace-restoration-diagnostic.test.ts @@ -15,6 +15,10 @@ describe("workspace restoration diagnostic", () => { ["fatal: unable to access 'https://example.test/repo.git/': Could not resolve host: example.test", "remote_unavailable"], ["fatal: '/srv/repo.git' does not appear to be a git repository", "remote_unavailable"], ["fatal: bad revision", "git_command"], + ["error: cannot open '.git/FETCH_HEAD': Permission denied", "filesystem"], + ["fatal: unable to access '/private/repository/.git/config': Permission denied", "filesystem"], + ["git@example.test: Permission denied (publickey).", "auth_rejected"], + ["git@example.test: Permission denied (publickey).\nerror: cannot open '.git/FETCH_HEAD': Permission denied", "filesystem"], ])("classifies %s", (stderr, kind) => { expect(classifyWorkspaceRestorationError(gitError(stderr))).toBe(kind); }); diff --git a/server/src/__tests__/managed-repository-checkout.test.ts b/server/src/__tests__/managed-repository-checkout.test.ts index 2767407786e..c00957da6f4 100644 --- a/server/src/__tests__/managed-repository-checkout.test.ts +++ b/server/src/__tests__/managed-repository-checkout.test.ts @@ -4,7 +4,7 @@ import { mkdtemp, mkdir, writeFile, readFile, readdir, rm, stat, symlink } from import { tmpdir } from "node:os"; import path from "node:path"; import { afterEach, describe, expect, it, vi } from "vitest"; -import { materializeManagedProjectWorkspace, ensureManagedRepositoriesIgnored } from "../services/managed-repository-checkout.js"; +import { materializeManagedProjectWorkspace, ensureManagedRepositoriesIgnored, readManagedRepositoryPreparationFailure } from "../services/managed-repository-checkout.js"; const execFile = promisify(execFileCallback); const directories: string[] = []; @@ -61,6 +61,29 @@ describe("atomic managed repository publication", () => { await expect(ensureManagedRepositoriesIgnored(target, { plainTaskDirectory: true })).rejects.toThrow("must not be a symlink"); expect(await readFile(path.join(source, ".git", "info", "exclude"), "utf8")).toBe(original); }); + it.each([ + { label: "timeout", code: null, killed: true, timedOut: true }, + { label: "filesystem", code: "EACCES", killed: false, timedOut: false }, + { label: "output limit", code: "ERR_CHILD_PROCESS_STDIO_MAXBUFFER", killed: true, timedOut: false }, + ])("retains safe $label metadata without adding raw command causes", async ({ code, killed, timedOut }) => { + const { source, target } = await fixture(); + const original = Object.assign(new Error("Git failed for https://user:fixture-password@example.test/repo.git"), { + code, killed, stderr: "private command output", + }); + const failure = await materializeManagedProjectWorkspace(target, { + repoUrl: "https://example.test/repo.git", localSource: source, + beforePublish: async () => { throw original; }, + }).then(() => { throw new Error("Expected preparation to fail"); }, (error: unknown) => error); + expect(failure).toBeInstanceOf(Error); + expect(readManagedRepositoryPreparationFailure(failure)).toEqual({ code, timedOut }); + expect(failure).not.toHaveProperty("code"); + expect(failure).not.toHaveProperty("cause"); + expect(failure).not.toHaveProperty("stderr"); + expect(JSON.stringify(failure)).toBe("{}"); + expect((failure as Error).message).not.toContain("fixture-password"); + expect(await stat(target).catch(() => null)).toBeNull(); + }); + it("writes the pinned receipt inside the clone before making it visible", async () => { const { source, target } = await fixture(); const result = await materializeManagedProjectWorkspace(target, { repoUrl: "https://github.com/test/source", localSource: source, beforePublish: async clone => { diff --git a/server/src/services/git-workspace-contents.ts b/server/src/services/git-workspace-contents.ts index 9e138816044..1e8e8a74774 100644 --- a/server/src/services/git-workspace-contents.ts +++ b/server/src/services/git-workspace-contents.ts @@ -10,7 +10,7 @@ import type { GitRemoteAuthProvider } from "./git-credentials.js"; import { localRepositoryPath } from "./local-repository-path.js"; import { managedProjectCheckoutName } from "./managed-project-checkout-name.js"; import { materializeManagedProjectWorkspace } from "./managed-repository-checkout.js"; -import { ensureSavedGitCommit, gitCacheCommand, GitCacheRestorationFailure } from "./git-workspace-restoration.js"; +import { ensureSavedGitCommit, gitCacheCommand, GitCacheRestorationFailure, restorationFailureAt } from "./git-workspace-restoration.js"; import { applyGitRestorePointToWorktree, ensureGitRestorePointCommit, loadGitRestorePoint, type GitRestorePointRef } from "./git-restore-point-storage.js"; import { copyVerifiedGitExcludes } from "./git-repository-excludes.js"; import { importRestorePoint } from "./git-restore-point.js"; @@ -291,30 +291,36 @@ export async function restoreGitWorkspaceContents(input: Identity & { cwd: strin } const savedRestorePoint = checkout.restorePoint && input.isolateRepositories ? await loadGitRestorePoint({ storage: input.storage ?? getStorageService(), workspace, ref: checkout.restorePoint }) : null; + let preparationFailure: GitCacheRestorationFailure | null = null; const result = await materializeManagedProjectWorkspace(destination, { repoUrl: checkout.source.repoUrl, gitObjectSource: gitObjectSource ? { cwd: gitObjectSource, commits: [checkout.commit, ...(savedRestorePoint?.manifest.prerequisites ?? [])] } : undefined, resolveGitAuth: input.resolveGitAuth, beforePublish: async temporary => { - if (gitObjectSource && checkout.restorePoint) await copyVerifiedGitExcludes(gitObjectSource, temporary); - if (savedRestorePoint) { - const { payload, manifest } = savedRestorePoint; - await ensureCheckoutPrerequisites(temporary, checkout, manifest, input.resolveGitAuth); - await importRestorePoint({ repositoryCwd: temporary, payload }); - } else if (checkout.restorePoint) await ensureGitRestorePointCommit({ storage: input.storage ?? getStorageService(), workspace, - ref: checkout.restorePoint, repositoryCwd: temporary }); - await ensureSavedGitCommit(temporary, checkout.source.repoUrl, checkout.commit, input.resolveGitAuth); - if (checkout.branchName) await gitCacheCommand(temporary, ["check-ref-format", "--branch", checkout.branchName]); - await gitCacheCommand(temporary, checkout.branchName - ? ["checkout", "-B", checkout.branchName, checkout.commit] : ["checkout", "--detach", checkout.commit]); - if (checkout.restorePoint) await applyGitRestorePointToWorktree({ storage: input.storage ?? getStorageService(), workspace, - ref: checkout.restorePoint, repositoryCwd: temporary, worktreePath: temporary, preserveImportedHistory: input.isolateRepositories }); - await gitCacheCommand(temporary, ["reflog", "expire", "--expire=now", "--all"]); - await restoreSettings(temporary, checkout.settings, input, relative); - await input.authorizeSource(checkout.source); - await recheck?.(); - if (gitObjectSource && !await isVerifiedNestedRepositoryBase(gitObjectSource, candidates!, checkout.source.repoUrl)) throw new GitCacheRestorationFailure(); - } }); + try { + if (gitObjectSource && checkout.restorePoint) await copyVerifiedGitExcludes(gitObjectSource, temporary); + if (savedRestorePoint) { + const { payload, manifest } = savedRestorePoint; + await ensureCheckoutPrerequisites(temporary, checkout, manifest, input.resolveGitAuth); + await importRestorePoint({ repositoryCwd: temporary, payload }); + } else if (checkout.restorePoint) await ensureGitRestorePointCommit({ storage: input.storage ?? getStorageService(), workspace, + ref: checkout.restorePoint, repositoryCwd: temporary }); + await ensureSavedGitCommit(temporary, checkout.source.repoUrl, checkout.commit, input.resolveGitAuth); + if (checkout.branchName) await gitCacheCommand(temporary, ["check-ref-format", "--branch", checkout.branchName]); + await gitCacheCommand(temporary, checkout.branchName + ? ["checkout", "-B", checkout.branchName, checkout.commit] : ["checkout", "--detach", checkout.commit]); + if (checkout.restorePoint) await applyGitRestorePointToWorktree({ storage: input.storage ?? getStorageService(), workspace, + ref: checkout.restorePoint, repositoryCwd: temporary, worktreePath: temporary, preserveImportedHistory: input.isolateRepositories }); + await gitCacheCommand(temporary, ["reflog", "expire", "--expire=now", "--all"]); + await restoreSettings(temporary, checkout.settings, input, relative); + await input.authorizeSource(checkout.source); + await recheck?.(); + if (gitObjectSource && !await isVerifiedNestedRepositoryBase(gitObjectSource, candidates!, checkout.source.repoUrl)) throw new GitCacheRestorationFailure(); + } catch (error) { + preparationFailure = restorationFailureAt("workspace_contents", error); + throw preparationFailure; + } + } }).catch(error => { throw preparationFailure ?? error; }); if (result.warning) throw new GitCacheRestorationFailure(); } await restoreSettings(input.cwd, input.contents.settings, input, CLAUDE_SETTINGS_PATH); - } catch { throw new GitCacheRestorationFailure(); } + } catch (error) { throw restorationFailureAt("workspace_contents", error); } } diff --git a/server/src/services/git-workspace-restoration.ts b/server/src/services/git-workspace-restoration.ts index 3726b996173..2039039f03d 100644 --- a/server/src/services/git-workspace-restoration.ts +++ b/server/src/services/git-workspace-restoration.ts @@ -8,7 +8,7 @@ import path from "node:path"; import { execFile as execFileCallback } from "node:child_process"; import { createHash } from "node:crypto"; import { resolveManagedProjectWorkspaceDir, resolvePaperclipInstanceRoot } from "../home-paths.js"; -import { materializeManagedProjectWorkspace, RepositoryRefNotFoundError } from "./managed-repository-checkout.js"; +import { materializeManagedProjectWorkspace, RepositoryRefNotFoundError, readManagedRepositoryPreparationFailure } from "./managed-repository-checkout.js"; import { readGitConnectionFailure } from "./git-connection-failure.js"; import { promisify } from "node:util"; import type { GitRemoteAuthProvider } from "./git-credentials.js"; @@ -87,7 +87,7 @@ export async function verifiedManagedGitObjectSource(workspace: Workspace, sourc } export type GitCacheRestorationStep = "recipe" | "trash_recovery" | "source_check" | "source_materialize" | "source_verify" - | "restore_point" | "saved_commit"; + | "restore_point" | "saved_commit" | "workspace_contents"; export type WorkspaceRestorationFailureKind = "remote_unavailable" | "auth_rejected" | "commit_unavailable" | "timeout" | "verification" | "filesystem" | "git_command" | "unknown"; @@ -111,14 +111,23 @@ export function classifyWorkspaceRestorationError(error: unknown): WorkspaceRest } if (error instanceof RepositoryRefNotFoundError) return "commit_unavailable"; const details = error as Error & { code?: unknown; killed?: unknown; stderr?: unknown }; - const code = typeof details.code === "string" ? details.code : null; - if (details.code === "ERR_CHILD_PROCESS_STDIO_MAXBUFFER") return "git_command"; - if (details.killed === true || details.code === "ETIMEDOUT" || error.name === "AbortError" || error.name === "TimeoutError") return "timeout"; + const preparationFailure = readManagedRepositoryPreparationFailure(error); + const commandCode = preparationFailure?.code ?? details.code; + const code = typeof commandCode === "string" ? commandCode : null; + if (commandCode === "ERR_CHILD_PROCESS_STDIO_MAXBUFFER") return "git_command"; + if (preparationFailure?.timedOut || details.killed === true || commandCode === "ETIMEDOUT" || error.name === "AbortError" || error.name === "TimeoutError") return "timeout"; if (["ENOTFOUND", "EAI_AGAIN", "ECONNREFUSED", "ECONNRESET", "EHOSTUNREACH", "ENETUNREACH"].includes(code ?? "")) return "remote_unavailable"; if (["ENOENT", "ENOTDIR", "EISDIR", "EACCES", "EPERM", "ENOSPC", "EROFS", "EMFILE", "ENFILE", "ELOOP", "ENAMETOOLONG", "EBUSY", "EIO", "EDQUOT"].includes(code ?? "")) return "filesystem"; const stderr = typeof details.stderr === "string" ? details.stderr.slice(0, 16_384) : Buffer.isBuffer(details.stderr) ? details.stderr.subarray(0, 16_384).toString("utf8") : error.message.slice(0, 16_384); - if (/authentication failed|could not read (username|password)|terminal prompts disabled|permission denied|returned error: 40[13]/i.test(stderr)) { + const lines = stderr.split(/\r?\n/).map(line => line.trim()); + if (lines.some(line => /^(?:error|fatal): (?:cannot|could not|unable to) (?:open|create|write|lock|read|stat|mkdir|unlink|remove|rename)\b.*: (?:Permission denied|Operation not permitted|Read-only file system|No space left on device)\.?$/i.test(line) + || /^(?:error|fatal): unable to access ['"](?![a-z][a-z0-9+.-]*:\/\/)[^'"\r\n]+['"]: (?:Permission denied|Operation not permitted|Read-only file system)\.?$/i.test(line))) return "filesystem"; + if (lines.some(line => /^fatal: Authentication failed for ['"][^'"\r\n]+['"]$/i.test(line) + || /^fatal: could not read (?:Username|Password) for ['"][^'"\r\n]+['"]: terminal prompts disabled$/i.test(line) + || /^fatal: unable to access ['"][^'"\r\n]+['"]: The requested URL returned error: 40[13]$/i.test(line) + || /^remote: (?:Invalid username or (?:password|token)|Permission [^\r\n]+ denied|Support for password authentication was removed|Password authentication is not supported)/i.test(line) + || /^(?:[^\s:@]+@)?[a-z0-9][a-z0-9.-]*: Permission denied \((?:publickey|password|keyboard-interactive)(?:,[a-z-]+)*\)\.?$/i.test(line))) { return "auth_rejected"; } if (/not our ref|couldn't find remote ref|no such remote ref|unadvertised object|did not send all necessary objects/i.test(stderr)) { @@ -127,11 +136,11 @@ export function classifyWorkspaceRestorationError(error: unknown): WorkspaceRest if (/could not resolve host|unable to access|could not read from remote repository|does not appear to be a git repository|repository .*not found|connection (refused|timed out|reset)/i.test(stderr)) { return "remote_unavailable"; } - if (typeof details.stderr === "string" || typeof details.code === "number") return "git_command"; + if (typeof details.stderr === "string" || typeof commandCode === "number") return "git_command"; return "unknown"; } -function restorationFailureAt(step: GitCacheRestorationStep, error: unknown) { +export function restorationFailureAt(step: GitCacheRestorationStep, error: unknown) { if (error instanceof GitCacheRestorationFailure && error.step) return error; return new GitCacheRestorationFailure({ step, kind: classifyWorkspaceRestorationError(error) }); } diff --git a/server/src/services/managed-repository-checkout.ts b/server/src/services/managed-repository-checkout.ts index cbd8ad0b1d2..73a452733fe 100644 --- a/server/src/services/managed-repository-checkout.ts +++ b/server/src/services/managed-repository-checkout.ts @@ -15,6 +15,29 @@ import { resolveRepositoryObjectCache, readRepositoryObjectCache, publishReposit const execFile = promisify(execFileCallback); const MANAGED_WORKSPACE_GIT_CLONE_TIMEOUT_MS = 10 * 60 * 1000; +type ManagedRepositoryPreparationFailure = { code: string | number | null; timedOut: boolean }; +const preparationFailures = new WeakMap(); + +export function readManagedRepositoryPreparationFailure(error: unknown): ManagedRepositoryPreparationFailure | null { + const failure = error instanceof Error ? preparationFailures.get(error) : undefined; + return failure ? { ...failure } : null; +} + +function managedRepositoryPreparationError(message: string, error: unknown): Error { + const wrapped = new Error(message); + const retained = readManagedRepositoryPreparationFailure(error); + if (retained) preparationFailures.set(wrapped, retained); + else if (error instanceof Error) { + const details = error as Error & { code?: unknown; killed?: unknown }; + const code = typeof details.code === "number" && Number.isSafeInteger(details.code) ? details.code + : typeof details.code === "string" && /^(?:E[A-Z0-9_]{1,63}|ABORT_ERR)$/.test(details.code) ? details.code : null; + const timedOut = code !== "ERR_CHILD_PROCESS_STDIO_MAXBUFFER" && (details.killed === true || code === "ETIMEDOUT" + || error.name === "AbortError" || error.name === "TimeoutError"); + if (code !== null || timedOut) preparationFailures.set(wrapped, { code, timedOut }); + } + return wrapped; +} + async function copyManagedGitObjects(cwd: string, repoUrl: string, source: { cwd: string; commits: readonly string[] }) { const git = (directory: string, args: string[]) => execFile("git", ["-C", directory, ...args], { env: { ...sanitizeRuntimeServiceBaseEnv(process.env), GIT_TERMINAL_PROMPT: "0" }, @@ -202,7 +225,7 @@ export async function materializeManagedProjectWorkspace( if (error instanceof RepositoryRefNotFoundError) throw new RepositoryRefNotFoundError(message, error.requestedRef); const connectionFailure = readGitConnectionFailure(error); if (connectionFailure) throw new GitConnectionFailureError(message, connectionFailure); - throw new Error(message); + throw managedRepositoryPreparationError(message, error); } try { @@ -216,8 +239,9 @@ export async function materializeManagedProjectWorkspace( } const reason = renameError instanceof Error ? renameError.message : String(renameError); - throw new Error( + throw managedRepositoryPreparationError( `Failed to move managed checkout into place at "${cwd}": ${reason}`, + renameError, ); } return { cwd, warning: null };