diff --git a/doc/observability.md b/doc/observability.md index 77fe0a28860..0d55d0fe8b7 100644 --- a/doc/observability.md +++ b/doc/observability.md @@ -572,6 +572,18 @@ 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. +For `workspace_validation_failed` with `git_worktree_not_reusable`, `run_execution` +includes `workspaceValidationReason` and an allowlisted `workspaceValidationReasonCode`: +`missing_worktree`, `not_a_git_checkout`, `not_registered`, `wrong_repository_root`, +`branch_mismatch`, or `git_inspection_failed`. The last code distinguishes an +unsuccessful or truncated Git registration inspection from confirmed absence in +a complete list. Its optional `workspaceValidationInspection*` fields identify +the fixed command `worktree_list`, failure (`spawn_failed`, `nonzero_exit`, or +`output_truncated`), an allowlisted OS error code (or `unknown`), and an exit code +from 1 through 255. No paths, repository or branch names, IDs, command output, or +arbitrary messages enter these fields. Validation still blocks reuse; it does +not repair Git metadata, change the selected repository, or retry the task. + When available, the saved `workspaceRestoreDiagnostic` adds the bounded fields `workspaceRestorePhase`, `workspaceRestoreStep`, `workspaceRestoreErrorCode`, `workspaceRestoreHttpStatus`, and `workspaceRestoreExitCode` to `run_execution`. diff --git a/server/src/__tests__/managed-worktree-inspection.test.ts b/server/src/__tests__/managed-worktree-inspection.test.ts new file mode 100644 index 00000000000..7179f23b07f --- /dev/null +++ b/server/src/__tests__/managed-worktree-inspection.test.ts @@ -0,0 +1,85 @@ +import { execFile } from "node:child_process"; +import fs from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; +import { promisify } from "node:util"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { ensurePersistedExecutionWorkspaceAvailable, inspectManagedGitWorktreeBranch } from "../services/workspace-runtime.js"; + +const exec = promisify(execFile); +const roots: string[] = []; +afterEach(async () => { + vi.unstubAllEnvs(); + await Promise.all(roots.splice(0).map(root => fs.rm(root, { recursive: true, force: true }))); +}); + +async function fixture() { + const root = await fs.mkdtemp(path.join(os.tmpdir(), "worktree-inspection-")); + roots.push(root); + const repo = path.join(root, "repo"); + const worktree = path.join(root, "worktree"); + await fs.mkdir(repo); + await exec("git", ["init", repo]); + await exec("git", ["-C", repo, "-c", "user.name=Test", "-c", "user.email=test@example.invalid", "commit", "--allow-empty", "-m", "initial"]); + await exec("git", ["-C", repo, "worktree", "add", "-b", "work", worktree]); + await fs.writeFile(path.join(worktree, "retained.txt"), "retained work\n"); + return { root, repo, worktree }; +} + +async function replaceListProbe(root: string, body: string) { + const realGit = (await exec("/bin/sh", ["-c", "command -v git"])).stdout.trim(); + const bin = path.join(root, "bin"); + await fs.mkdir(bin); + await fs.writeFile(path.join(bin, "git"), `#!${process.execPath}\nconst args=process.argv.slice(2);\nif(args[0]==="worktree"&&args[1]==="list"){${body}}else{const r=require("node:child_process").spawnSync(${JSON.stringify(realGit)},args,{stdio:"inherit"});process.exitCode=r.status??1;}\n`, { mode: 0o755 }); + vi.stubEnv("PATH", `${bin}${path.delimiter}${process.env.PATH ?? ""}`); +} + +describe("managed worktree registration inspection", () => { + it("accepts a complete registration and rejects a genuine independent checkout", async () => { + const { repo, worktree, root } = await fixture(); + expect(await inspectManagedGitWorktreeBranch({ repoRoot: repo, worktreePath: worktree, expectedBranchName: "work" })).toMatchObject({ valid: true, reasonCode: null }); + const other = path.join(root, "other"); + await exec("git", ["clone", repo, other]); + expect(await inspectManagedGitWorktreeBranch({ repoRoot: repo, worktreePath: other, expectedBranchName: null })).toMatchObject({ valid: false, reasonCode: "not_registered" }); + }); + + it("distinguishes a failed Git inspection from missing registration without copying stderr", async () => { + const { root, repo, worktree } = await fixture(); + await replaceListProbe(root, 'process.stderr.write("private-token /private/repository");process.exitCode=128;'); + const result = await inspectManagedGitWorktreeBranch({ repoRoot: repo, worktreePath: worktree, expectedBranchName: "work" }); + expect(result).toMatchObject({ valid: false, reasonCode: "git_inspection_failed", inspectionDiagnostic: { command: "worktree_list", failure: "nonzero_exit", exitCode: 128 } }); + expect(JSON.stringify(result)).not.toContain("private-token"); + expect(await fs.readFile(path.join(worktree, "retained.txt"), "utf8")).toBe("retained work\n"); + }); + + it("distinguishes a missing Git executable without inventing a registration mismatch", async () => { + const { root, repo, worktree } = await fixture(); + vi.stubEnv("PATH", root); + expect(await inspectManagedGitWorktreeBranch({ repoRoot: repo, worktreePath: worktree, expectedBranchName: "work" })).toMatchObject({ + valid: false, reasonCode: "git_inspection_failed", inspectionDiagnostic: { command: "worktree_list", failure: "spawn_failed", errorCode: "ENOENT" }, + }); + }); + + it("fails closed on truncated output even if a matching registration was captured", async () => { + const { root, repo, worktree } = await fixture(); + await replaceListProbe(root, `process.stdout.write("x".repeat(300_000) + ${JSON.stringify(`\nworktree ${worktree}\n\n`)});`); + expect(await inspectManagedGitWorktreeBranch({ repoRoot: repo, worktreePath: worktree, expectedBranchName: "work" })).toMatchObject({ + valid: false, reasonCode: "git_inspection_failed", inspectionDiagnostic: { command: "worktree_list", failure: "output_truncated" }, + }); + }); + + it("retains the bounded probe failure in persisted-workspace validation without changing files", async () => { + const { root, repo, worktree } = await fixture(); + await replaceListProbe(root, 'process.stderr.write("private-provider-output");process.exitCode=126;'); + await expect(ensurePersistedExecutionWorkspaceAvailable({ + base: { baseCwd: repo, source: "project_primary", projectId: "project", workspaceId: "workspace", repoUrl: null, repoRef: "HEAD" }, + workspace: { id: "execution-workspace", mode: "isolated_workspace", strategyType: "git_worktree", cwd: worktree, providerRef: worktree, projectId: "project", projectWorkspaceId: "workspace", repoUrl: null, baseRef: "HEAD", branchName: "work" }, + issue: { id: "issue", identifier: "TEST-1", title: "Preserve workspace" }, + agent: { id: "agent", name: "Agent", companyId: "company" }, + })).rejects.toMatchObject({ code: "workspace_validation_failed", resultJson: { workspaceValidation: { + reason: "git_worktree_not_reusable", reasonCode: "git_inspection_failed", + inspectionDiagnostic: { command: "worktree_list", failure: "nonzero_exit", exitCode: 126 }, + } } }); + expect(await fs.readFile(path.join(worktree, "retained.txt"), "utf8")).toBe("retained work\n"); + }); +}); diff --git a/server/src/services/__tests__/run-failure-diagnostics.test.ts b/server/src/services/__tests__/run-failure-diagnostics.test.ts index 0cc4d104b7f..751032c9a70 100644 --- a/server/src/services/__tests__/run-failure-diagnostics.test.ts +++ b/server/src/services/__tests__/run-failure-diagnostics.test.ts @@ -7,6 +7,48 @@ const run = (overrides: Partial = {}) => ({ resultJson: null, ...overrides const collect = (error: unknown) => collectRunFailureDiagnostics(run(), { error }); describe("run failure diagnostics", () => { + it("exports only fixed workspace validation codes and bounded inspection evidence", () => { + const resultJson = { workspaceValidation: { reason: "git_worktree_not_reusable", reasonCode: "git_inspection_failed", + worktreePath: "/private/path", executionWorkspaceId: "private-id", repository: "private-url", message: "private-message", + inspectionDiagnostic: { command: "worktree_list", failure: "nonzero_exit", exitCode: 128, stderr: "private-token" } } }; + const execution = collectRunFailureDiagnostics(run({ errorCode: "workspace_validation_failed", resultJson }), {}).execution; + expect(execution).toEqual({ workspaceValidationReason: "git_worktree_not_reusable", workspaceValidationReasonCode: "git_inspection_failed", + workspaceValidationInspectionCommand: "worktree_list", workspaceValidationInspectionFailure: "nonzero_exit", workspaceValidationInspectionExitCode: 128 }); + expect(JSON.stringify(execution)).not.toContain("private"); + expect(collectRunFailureDiagnostics(run({ errorCode: "adapter_failed", resultJson }), {}).execution).toEqual({}); + }); + + it.each(["missing_worktree", "not_a_git_checkout", "not_registered", "wrong_repository_root", "branch_mismatch"])( + "exports %s without attaching unrelated probe evidence", reasonCode => { + expect(collectRunFailureDiagnostics(run({ errorCode: "workspace_validation_failed", resultJson: { workspaceValidation: { + reason: "git_worktree_not_reusable", reasonCode, inspectionDiagnostic: { command: "worktree_list", failure: "nonzero_exit", exitCode: 128 }, + } } }), {}).execution).toEqual({ workspaceValidationReason: "git_worktree_not_reusable", workspaceValidationReasonCode: reasonCode }); + }, + ); + + it.each([null, -1, 0, 256, Infinity, NaN, 1.5, "private", {}])("omits invalid Git inspection exit codes (%j)", exitCode => { + const execution = collectRunFailureDiagnostics(run({ errorCode: "workspace_validation_failed", resultJson: { workspaceValidation: { + reason: "git_worktree_not_reusable", reasonCode: "git_inspection_failed", + inspectionDiagnostic: { command: "worktree_list", failure: "nonzero_exit", exitCode }, + } } }), {}).execution; + expect(execution).not.toHaveProperty("workspaceValidationInspectionExitCode"); + expect(JSON.stringify(execution)).not.toContain("private"); + }); + + it("omits unknown validation values and contains hostile diagnostic getters", () => { + const inspect = (workspaceValidation: unknown) => collectRunFailureDiagnostics(run({ errorCode: "workspace_validation_failed", resultJson: { workspaceValidation } }), {}).execution; + expect(inspect({ reason: "private" })).toEqual({}); + expect(inspect({ reason: "git_worktree_not_reusable", reasonCode: "private" })).toEqual({ workspaceValidationReason: "git_worktree_not_reusable" }); + const base = { reason: "git_worktree_not_reusable", reasonCode: "git_inspection_failed" }; + for (const inspectionDiagnostic of [null, {}, { command: "private", failure: "nonzero_exit" }, { command: "worktree_list", failure: "private" }, + Object.defineProperty({}, "command", { get() { throw new Error("private"); } })]) { + expect(inspect({ ...base, inspectionDiagnostic })).toEqual({ workspaceValidationReason: base.reason, workspaceValidationReasonCode: base.reasonCode }); + } + const error = Object.defineProperty({ command: "worktree_list", failure: "spawn_failed" }, "errorCode", { get() { throw new Error("private"); } }); + expect(inspect({ ...base, inspectionDiagnostic: error })).toMatchObject({ workspaceValidationInspectionErrorCode: "unknown" }); + expect(inspect({ ...base, inspectionDiagnostic: { command: "worktree_list", failure: "spawn_failed", errorCode: "EACCES", exitCode: 128 } })).toMatchObject({ workspaceValidationInspectionErrorCode: "EACCES" }); + }); + it("exports bounded orphan evidence only for a process-loss failure", () => { const processLossDiagnostic = { pidRecorded: false, groupRecorded: false, localCheck: "no_identifiers", retryEligible: false, runPredatesObserver: true, observerUptimeMs: 30_000, lastOutputAgeMs: 120_000, diff --git a/server/src/services/run-failure-diagnostics.ts b/server/src/services/run-failure-diagnostics.ts index 8dc0d78a289..007cdbe868b 100644 --- a/server/src/services/run-failure-diagnostics.ts +++ b/server/src/services/run-failure-diagnostics.ts @@ -7,6 +7,7 @@ import { sanitizeWorkspaceRestoreDiagnostic } from "@paperclipai/adapter-utils/w import { redactCurrentUserText } from "../log-redaction.js"; import { redactSensitiveText, REDACTED_EVENT_VALUE } from "../redaction.js"; import { readNativeModelRejectionDiagnostic } from "./native-runtime/native-provider-failure.js"; +import { MANAGED_GIT_WORKTREE_REASON_CODES, readManagedGitInspectionDiagnostic } from "./workspace-validation-diagnostics.js"; type Run = typeof heartbeatRuns.$inferSelect; type Context = Record; @@ -147,6 +148,20 @@ export function collectRunFailureDiagnostics(run: Run, options: RunFailureReport if (Number.isFinite(durationMs) && durationMs >= 0) execution.durationMs = durationMs; } const result = run.resultJson; + const workspaceValidation = read(result, "workspaceValidation"); + if (run.errorCode === "workspace_validation_failed" && read(workspaceValidation, "reason") === "git_worktree_not_reusable") { + execution.workspaceValidationReason = "git_worktree_not_reusable"; + const reasonCode = MANAGED_GIT_WORKTREE_REASON_CODES.find(code => code === read(workspaceValidation, "reasonCode")); + if (reasonCode) execution.workspaceValidationReasonCode = reasonCode; + const diagnostic = reasonCode === "git_inspection_failed" + ? readManagedGitInspectionDiagnostic(read(workspaceValidation, "inspectionDiagnostic")) : null; + if (diagnostic) { + execution.workspaceValidationInspectionCommand = diagnostic.command; + execution.workspaceValidationInspectionFailure = diagnostic.failure; + if (diagnostic.errorCode) execution.workspaceValidationInspectionErrorCode = diagnostic.errorCode; + if (diagnostic.exitCode !== undefined) execution.workspaceValidationInspectionExitCode = diagnostic.exitCode; + } + } if (run.errorCode === "process_lost") { const diagnostic = readProcessLossDiagnostic(read(result, "processLossDiagnostic")); for (const [field, value] of Object.entries(diagnostic)) { diff --git a/server/src/services/workspace-runtime.ts b/server/src/services/workspace-runtime.ts index 052eb5071b1..f0cfe40289c 100644 --- a/server/src/services/workspace-runtime.ts +++ b/server/src/services/workspace-runtime.ts @@ -67,6 +67,11 @@ import { isRuntimeOwnedGitBranch } from "./execution-workspace-branch-ownership. import { logActivity } from "./activity-log.js"; import { readProjectWorkspaceRuntimeConfig } from "./project-workspace-runtime-config.js"; import { workspaceGitOperationScheduler } from "./workspace-git-operation-scheduler.js"; +import { + MANAGED_GIT_WORKTREE_REASON_CODES, + readManagedGitInspectionDiagnostic, + type ManagedGitInspectionDiagnostic, +} from "./workspace-validation-diagnostics.js"; import { cleanupWorktreeInstanceArtifacts, deriveWorktreeInstanceId, @@ -2606,13 +2611,8 @@ type GitWorktreeListEntry = { export type ManagedGitWorktreeBranchInspection = { valid: boolean; reason: string | null; - reasonCode: - | "missing_worktree" - | "not_a_git_checkout" - | "not_registered" - | "wrong_repository_root" - | "branch_mismatch" - | null; + reasonCode: typeof MANAGED_GIT_WORKTREE_REASON_CODES[number] | null; + inspectionDiagnostic?: ManagedGitInspectionDiagnostic; repoRoot: string | null; worktreePath: string; expectedBranchName: string | null; @@ -2735,16 +2735,33 @@ async function resolvePathForWorktreeComparison(value: string): Promise return fs.realpath(resolved).then((realPath) => path.resolve(realPath)).catch(() => resolved); } -async function listLinkedGitWorktreePaths(repoRoot: string): Promise> { - const output = await runGit(["worktree", "list", "--porcelain"], repoRoot); +async function listLinkedGitWorktreePaths(repoRoot: string): Promise< + { paths: Set; diagnostic?: never } | { paths?: never; diagnostic: ManagedGitInspectionDiagnostic } +> { + let proc: Awaited>; + try { + proc = await executeProcess({ command: "git", args: ["worktree", "list", "--porcelain"], cwd: repoRoot }); + } catch (error) { + return { diagnostic: readManagedGitInspectionDiagnostic({ + command: "worktree_list", failure: "spawn_failed", errorCode: (error as NodeJS.ErrnoException)?.code, + })! }; + } + if (proc.code !== 0) { + return { diagnostic: readManagedGitInspectionDiagnostic({ + command: "worktree_list", failure: "nonzero_exit", exitCode: proc.code, + })! }; + } + if (proc.stdoutTruncated) { + return { diagnostic: { command: "worktree_list", failure: "output_truncated" } }; + } const paths = new Set(); - for (const line of output.split("\n")) { + for (const line of proc.stdout.split("\n")) { if (!line.startsWith("worktree ")) continue; const worktree = line.slice("worktree ".length).trim(); if (!worktree) continue; paths.add(await resolvePathForWorktreeComparison(worktree)); } - return paths; + return { paths }; } export async function inspectManagedGitWorktreeBranch(input: { @@ -2783,8 +2800,18 @@ export async function inspectManagedGitWorktreeBranch(input: { }; } - const listedWorktrees = await listLinkedGitWorktreePaths(repoRoot).catch(() => null); - if (!listedWorktrees?.has(worktreePath)) { + const listedWorktrees = await listLinkedGitWorktreePaths(repoRoot); + if (listedWorktrees.diagnostic) { + return { + ...base, + valid: false, + reason: "could not inspect the complete git worktree registration list", + reasonCode: "git_inspection_failed", + repoRoot, + inspectionDiagnostic: listedWorktrees.diagnostic, + }; + } + if (!listedWorktrees.paths.has(worktreePath)) { return { ...base, valid: false, @@ -2841,6 +2868,7 @@ async function validateLinkedGitWorktree(input: { reason: string; reasonCode: Exclude; actualBranchName?: string | null; + inspectionDiagnostic?: ManagedGitInspectionDiagnostic; } > { const inspection = await inspectManagedGitWorktreeBranch({ @@ -2855,6 +2883,7 @@ async function validateLinkedGitWorktree(input: { reason: inspection.reason ?? "unknown git worktree mismatch", reasonCode: inspection.reasonCode ?? "not_a_git_checkout", actualBranchName: inspection.actualBranchName, + ...(inspection.inspectionDiagnostic ? { inspectionDiagnostic: inspection.inspectionDiagnostic } : {}), }; } @@ -2867,6 +2896,7 @@ export function formatManagedGitWorktreeBranchInspection(input: ManagedGitWorktr worktreePath: input.worktreePath, expectedBranchName: input.expectedBranchName, actualBranchName: input.actualBranchName, + ...(input.inspectionDiagnostic ? { inspectionDiagnostic: input.inspectionDiagnostic } : {}), }; } @@ -3756,6 +3786,7 @@ export async function ensurePersistedExecutionWorkspaceAvailable(input: { reasonCode: validation.reasonCode, worktreePath: reuseWorktreePath, executionWorkspaceId: input.workspace.id ?? null, + ...(validation.inspectionDiagnostic ? { inspectionDiagnostic: validation.inspectionDiagnostic } : {}), }, }, ); diff --git a/server/src/services/workspace-validation-diagnostics.ts b/server/src/services/workspace-validation-diagnostics.ts new file mode 100644 index 00000000000..73c4a452d88 --- /dev/null +++ b/server/src/services/workspace-validation-diagnostics.ts @@ -0,0 +1,37 @@ +export const MANAGED_GIT_WORKTREE_REASON_CODES = [ + "missing_worktree", "not_a_git_checkout", "not_registered", + "wrong_repository_root", "branch_mismatch", "git_inspection_failed", +] as const; + +export type ManagedGitInspectionDiagnostic = { + command: "worktree_list"; + failure: "spawn_failed" | "nonzero_exit" | "output_truncated"; + errorCode?: "EACCES" | "ENOENT" | "EIO" | "EMFILE" | "ENFILE" | "ENOMEM" | "EPERM" | "unknown"; + exitCode?: number; +}; + +function read(value: unknown, key: string): unknown { + try { + return value && typeof value === "object" ? Reflect.get(value, key) : undefined; + } catch { + return undefined; + } +} + +/** Keep command output, paths, arguments and arbitrary error properties out of reports. */ +export function readManagedGitInspectionDiagnostic(value: unknown): ManagedGitInspectionDiagnostic | null { + if (read(value, "command") !== "worktree_list") return null; + const failure = read(value, "failure"); + if (failure !== "spawn_failed" && failure !== "nonzero_exit" && failure !== "output_truncated") return null; + const result: ManagedGitInspectionDiagnostic = { command: "worktree_list", failure }; + if (failure === "spawn_failed") { + const code = read(value, "errorCode"); + result.errorCode = code === "EACCES" || code === "ENOENT" || code === "EIO" || code === "EMFILE" + || code === "ENFILE" || code === "ENOMEM" || code === "EPERM" ? code : "unknown"; + } + if (failure === "nonzero_exit") { + const code = read(value, "exitCode"); + if (typeof code === "number" && Number.isInteger(code) && code >= 1 && code <= 255) result.exitCode = code; + } + return result; +}