mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-21 00:05:49 +02:00
Distinguish failed Git inspection from missing worktree registration (#15525)
Apply the reviewed change for Distinguish failed Git inspection from missing worktree registration. Validation: required local typecheck and tests, passing CI, Greptile 5/5, and independent review of the exact source head. Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
1 parent
ed0c6958f8
commit
1d19f9b562
6 files changed
+235
-13
No files matched your search
@@ -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`.
|
||||
|
||||
@@ -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");
|
||||
});
|
||||
});
|
||||
@@ -7,6 +7,48 @@ const run = (overrides: Partial<Run> = {}) => ({ 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,
|
||||
|
||||
@@ -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<string, string | number | boolean>;
|
||||
@@ -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)) {
|
||||
|
||||
@@ -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<string>
|
||||
return fs.realpath(resolved).then((realPath) => path.resolve(realPath)).catch(() => resolved);
|
||||
}
|
||||
|
||||
async function listLinkedGitWorktreePaths(repoRoot: string): Promise<Set<string>> {
|
||||
const output = await runGit(["worktree", "list", "--porcelain"], repoRoot);
|
||||
async function listLinkedGitWorktreePaths(repoRoot: string): Promise<
|
||||
{ paths: Set<string>; diagnostic?: never } | { paths?: never; diagnostic: ManagedGitInspectionDiagnostic }
|
||||
> {
|
||||
let proc: Awaited<ReturnType<typeof executeProcess>>;
|
||||
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<string>();
|
||||
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<ManagedGitWorktreeBranchInspection["reasonCode"], null>;
|
||||
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 } : {}),
|
||||
},
|
||||
},
|
||||
);
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
Reference in new issue
Block a user