fix(merger): restrict staging to allowlist and harden git invocations

Replaces blanket `git add -A` in `commitOrAmendMergeWithFixes` with an
explicit allowlist of (squash-staged ∪ fix-agent-modified) paths, so
unrelated dirty files in the project root no longer get swept into a
task's squash commit. The in-merge fix agent now snapshots the working
tree before/after its session to capture exactly which files it touched.

Hardens the git invocations the allowlist relies on:
- All `git add` and `git checkout --ours/--theirs` calls switched from
  shell-interpolated `execAsync` to `execFile` array form, eliminating
  path-injection surface and batching per-file spawns into one call.
- `snapshotDirtyFiles` adopts `git -z` NUL-delimited parsing so paths
  with embedded spaces or specials are handled correctly.
- Long allowlist debug logs are truncated to 20 entries with an overflow
  marker.

Refused-to-stage paths emit a warn naming each file so the user can
audit what was filtered.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
gsxdsm
2026-05-04 13:17:29 -07:00
parent 840dad771d
commit 89da1de311
5 changed files with 837 additions and 20 deletions

View File

@@ -0,0 +1,586 @@
/**
* Integration tests for the merger staging allowlist (real git repos).
*
* These tests do NOT mock child_process — they run real git commands against
* temporary repositories created in the OS temp directory. This verifies the
* exact behavior of `snapshotDirtyFiles` and `commitOrAmendMergeWithFixes`
* against a real git index without the indirection of exec mocks.
*
* Test inventory:
* 1. snapshotDirtyFiles captures tracked-unstaged, staged, and untracked files
* 2. Unrelated dirty file is excluded — not staged, warn emitted
* 3. Fix-modified file is included — staged and committed
* 4. File in squash + further edited by fix agent — staged once, no error
* 5. Untracked file created by fix agent — staged and committed
* 6. Untracked file pre-existing in working tree (user WIP) — NOT staged
*/
import { describe, it, expect, beforeEach, afterEach, vi } from "vitest";
import { mkdtempSync, mkdirSync, writeFileSync, rmSync, readFileSync } from "node:fs";
import { join } from "node:path";
import { tmpdir } from "node:os";
import { execSync } from "node:child_process";
import { snapshotDirtyFiles, commitOrAmendMergeWithFixes } from "../merger.js";
import { mergerLog } from "../logger.js";
import { DEFAULT_SETTINGS } from "@fusion/core";
// ---------------------------------------------------------------------------
// Git repo helpers
// ---------------------------------------------------------------------------
/**
* Initialise a bare minimum git repo at `dir` with a single initial commit.
* Returns the SHA of that commit (used as `preAttemptHeadSha`).
*/
function initRepo(dir: string): string {
const git = (cmd: string) =>
execSync(cmd, { cwd: dir, stdio: "pipe" }).toString().trim();
git("git init");
git('git config user.email "test@example.com"');
git('git config user.name "Test"');
git('git config commit.gpgsign false');
// Create an initial commit so HEAD exists
writeFileSync(join(dir, "README.md"), "# repo\n");
git("git add README.md");
git('git commit -m "chore: initial commit"');
return git("git rev-parse HEAD");
}
/**
* Create a feature branch with one commit, then return to main and run
* `git merge --squash <branch>` so that a squash is staged but not committed.
* Returns the SHA of main's tip (which becomes `preAttemptHeadSha`).
*/
function squashBranch(dir: string, branchName: string, fileName: string, content: string): string {
const git = (cmd: string) =>
execSync(cmd, { cwd: dir, stdio: "pipe" }).toString().trim();
git(`git checkout -b ${branchName}`);
writeFileSync(join(dir, fileName), content);
git(`git add ${fileName}`);
git(`git commit -m "feat: add ${fileName}"`);
git("git checkout main");
const preAttemptSha = git("git rev-parse HEAD");
git(`git merge --squash ${branchName}`);
return preAttemptSha;
}
// ---------------------------------------------------------------------------
// Minimal stub settings / args used by commitOrAmendMergeWithFixes
// ---------------------------------------------------------------------------
const STUB_SETTINGS = {
...DEFAULT_SETTINGS,
commitAuthorEnabled: false, // skip --author flag to avoid user config issues
};
// ---------------------------------------------------------------------------
// Tests
// ---------------------------------------------------------------------------
describe("snapshotDirtyFiles", () => {
let dir: string;
beforeEach(() => {
dir = mkdtempSync(join(tmpdir(), "fn-snapshot-"));
initRepo(dir);
});
afterEach(() => {
rmSync(dir, { recursive: true, force: true });
});
it("returns empty set when working tree is clean", async () => {
const snapshot = await snapshotDirtyFiles(dir);
expect(snapshot.size).toBe(0);
});
it("captures tracked-unstaged modifications", async () => {
writeFileSync(join(dir, "README.md"), "modified\n");
const snapshot = await snapshotDirtyFiles(dir);
expect(snapshot.has("README.md")).toBe(true);
});
it("captures staged (cached) modifications", async () => {
writeFileSync(join(dir, "README.md"), "staged change\n");
execSync("git add README.md", { cwd: dir, stdio: "pipe" });
const snapshot = await snapshotDirtyFiles(dir);
expect(snapshot.has("README.md")).toBe(true);
});
it("captures untracked files", async () => {
writeFileSync(join(dir, "new-file.ts"), "export const x = 1;\n");
const snapshot = await snapshotDirtyFiles(dir);
expect(snapshot.has("new-file.ts")).toBe(true);
});
it("captures all three categories simultaneously", async () => {
// Tracked-unstaged
writeFileSync(join(dir, "README.md"), "dirty\n");
// Staged
writeFileSync(join(dir, "staged.ts"), "const s = 1;\n");
execSync("git add staged.ts", { cwd: dir, stdio: "pipe" });
// Untracked
writeFileSync(join(dir, "untracked.ts"), "const u = 2;\n");
const snapshot = await snapshotDirtyFiles(dir);
expect(snapshot.has("README.md")).toBe(true);
expect(snapshot.has("staged.ts")).toBe(true);
expect(snapshot.has("untracked.ts")).toBe(true);
});
it("returns empty set when rootDir is not a git repo (error swallowed)", async () => {
const nonRepo = mkdtempSync(join(tmpdir(), "fn-non-repo-"));
try {
const snapshot = await snapshotDirtyFiles(nonRepo);
expect(snapshot.size).toBe(0);
} finally {
rmSync(nonRepo, { recursive: true, force: true });
}
});
});
describe("commitOrAmendMergeWithFixes — staging allowlist", () => {
let dir: string;
let warnSpy: ReturnType<typeof vi.spyOn>;
beforeEach(() => {
dir = mkdtempSync(join(tmpdir(), "fn-allowlist-"));
initRepo(dir);
warnSpy = vi.spyOn(mergerLog, "warn");
});
afterEach(() => {
warnSpy.mockRestore();
rmSync(dir, { recursive: true, force: true });
});
// ── Scenario 1: Unrelated dirty file is excluded ───────────────────────
it("does not stage an unrelated dirty file and emits a warn", async () => {
const preAttemptSha = squashBranch(dir, "feat/A", "feature-a.ts", "export const a = 1;\n");
// Simulate user's unrelated WIP: a modified tracked file
writeFileSync(join(dir, "README.md"), "user WIP — should not be committed\n");
// fixModifiedFiles is empty — no fix agent ran
const result = await commitOrAmendMergeWithFixes(
dir,
"FN-TEST",
"feat/A",
"- feat: add feature-a.ts",
false,
preAttemptSha,
"", // no --author flag
undefined,
STUB_SETTINGS,
undefined,
null,
null,
new Set<string>(), // empty fixModifiedFiles
);
expect(result).toBe(true);
// The unrelated file must NOT appear in the commit
const committedFiles = execSync("git diff --name-only HEAD~1 HEAD", {
cwd: dir,
stdio: "pipe",
}).toString().trim().split("\n");
expect(committedFiles).not.toContain("README.md");
expect(committedFiles).toContain("feature-a.ts");
// Warn must have been emitted for the excluded file
const warnMessages = warnSpy.mock.calls.map((c) => String(c[0]));
expect(warnMessages.some((m) => m.includes("README.md") && m.includes("refusing to stage"))).toBe(true);
// README.md must still be dirty in the working tree
const status = execSync("git diff --name-only", { cwd: dir, stdio: "pipe" }).toString().trim();
expect(status).toContain("README.md");
});
// ── Scenario 2: Fix-modified file is included ─────────────────────────
it("stages a file that the fix agent modified", async () => {
const preAttemptSha = squashBranch(dir, "feat/B", "feature-b.ts", "export const b = 1;\n");
// Fix agent modified an additional file (tracked, unstaged)
writeFileSync(join(dir, "README.md"), "fixed by agent\n");
const result = await commitOrAmendMergeWithFixes(
dir,
"FN-TEST",
"feat/B",
"- feat: add feature-b.ts",
false,
preAttemptSha,
"",
undefined,
STUB_SETTINGS,
undefined,
null,
null,
new Set(["README.md"]), // fix agent touched this
);
expect(result).toBe(true);
const committedFiles = execSync("git diff --name-only HEAD~1 HEAD", {
cwd: dir,
stdio: "pipe",
}).toString().trim().split("\n");
expect(committedFiles).toContain("feature-b.ts");
expect(committedFiles).toContain("README.md");
// Working tree should be clean for README.md now
const status = execSync("git diff --name-only", { cwd: dir, stdio: "pipe" }).toString().trim();
expect(status).not.toContain("README.md");
});
// ── Scenario 3: Squash file further edited by fix agent ───────────────
it("stages squash file with additional fix-agent edits only once", async () => {
const preAttemptSha = squashBranch(dir, "feat/C", "feature-c.ts", "export const c = 1;\n");
// Fix agent further edits the squash file (it's tracked-unstaged after squash staged it)
writeFileSync(join(dir, "feature-c.ts"), "export const c = 2; // fixed\n");
const result = await commitOrAmendMergeWithFixes(
dir,
"FN-TEST",
"feat/C",
"- feat: add feature-c.ts",
false,
preAttemptSha,
"",
undefined,
STUB_SETTINGS,
undefined,
null,
null,
new Set(["feature-c.ts"]), // fix agent touched the same file the squash staged
);
expect(result).toBe(true);
// The committed file must contain the fix agent's content, not the squash's
const committedContent = execSync("git show HEAD:feature-c.ts", {
cwd: dir,
stdio: "pipe",
}).toString();
expect(committedContent).toContain("// fixed");
// No double-staging error should have occurred (result is true)
// Working tree should be clean
const status = execSync("git status --porcelain", { cwd: dir, stdio: "pipe" }).toString().trim();
expect(status).toBe("");
});
// ── Scenario 4: Untracked file created by fix agent ───────────────────
it("stages an untracked file created by the fix agent", async () => {
const preAttemptSha = squashBranch(dir, "feat/D", "feature-d.ts", "export const d = 1;\n");
// Fix agent created a brand-new file (untracked)
writeFileSync(join(dir, "new-fixture.ts"), "export const fixture = {};\n");
const result = await commitOrAmendMergeWithFixes(
dir,
"FN-TEST",
"feat/D",
"- feat: add feature-d.ts",
false,
preAttemptSha,
"",
undefined,
STUB_SETTINGS,
undefined,
null,
null,
new Set(["new-fixture.ts"]), // fix agent created this file
);
expect(result).toBe(true);
const committedFiles = execSync("git diff --name-only HEAD~1 HEAD", {
cwd: dir,
stdio: "pipe",
}).toString().trim().split("\n");
expect(committedFiles).toContain("new-fixture.ts");
expect(committedFiles).toContain("feature-d.ts");
});
// ── Scenario 5: Pre-existing untracked user WIP file not staged ────────
it("does not stage a pre-existing untracked user WIP file", async () => {
// Create untracked user WIP before squash (simulates pre-existing state)
writeFileSync(join(dir, "user-wip.ts"), "// WIP — do not touch\n");
const preAttemptSha = squashBranch(dir, "feat/E", "feature-e.ts", "export const e = 1;\n");
// fixModifiedFiles does not include the user's WIP file
const result = await commitOrAmendMergeWithFixes(
dir,
"FN-TEST",
"feat/E",
"- feat: add feature-e.ts",
false,
preAttemptSha,
"",
undefined,
STUB_SETTINGS,
undefined,
null,
null,
new Set<string>(), // empty — the WIP file is not fix-agent-produced
);
expect(result).toBe(true);
const committedFiles = execSync("git diff --name-only HEAD~1 HEAD", {
cwd: dir,
stdio: "pipe",
}).toString().trim().split("\n");
expect(committedFiles).not.toContain("user-wip.ts");
expect(committedFiles).toContain("feature-e.ts");
// The WIP file must still be untracked in the working tree
const porcelain = execSync("git status --porcelain", {
cwd: dir,
stdio: "pipe",
}).toString();
expect(porcelain).toContain("user-wip.ts");
// Warn must have been emitted
const warnMessages = warnSpy.mock.calls.map((c) => String(c[0]));
expect(warnMessages.some((m) => m.includes("user-wip.ts") && m.includes("refusing to stage"))).toBe(true);
});
// ── Scenario 6: Mixed — fix file included, unrelated file excluded ─────
it("stages fix-agent file but excludes a second unrelated file in the same pass", async () => {
// Commit unrelated.ts into main so it is a properly tracked file
writeFileSync(join(dir, "unrelated.ts"), "// original\n");
execSync("git add unrelated.ts", { cwd: dir, stdio: "pipe" });
execSync('git commit -m "chore: add unrelated.ts"', { cwd: dir, stdio: "pipe" });
const preAttemptSha = squashBranch(dir, "feat/F", "feature-f.ts", "export const f = 1;\n");
// Fix agent modified one file
writeFileSync(join(dir, "README.md"), "agent fix\n");
// User modified the tracked (but unrelated) file in the working tree
writeFileSync(join(dir, "unrelated.ts"), "// user WIP\n");
const result = await commitOrAmendMergeWithFixes(
dir,
"FN-TEST",
"feat/F",
"- feat: add feature-f.ts",
false,
preAttemptSha,
"",
undefined,
STUB_SETTINGS,
undefined,
null,
null,
new Set(["README.md"]), // only the agent's file is in the allowlist
);
expect(result).toBe(true);
const committedFiles = execSync("git diff --name-only HEAD~1 HEAD", {
cwd: dir,
stdio: "pipe",
}).toString().trim().split("\n");
expect(committedFiles).toContain("feature-f.ts");
expect(committedFiles).toContain("README.md");
expect(committedFiles).not.toContain("unrelated.ts");
// unrelated.ts must remain dirty
const dirty = execSync("git diff --name-only", { cwd: dir, stdio: "pipe" }).toString();
expect(dirty).toContain("unrelated.ts");
const warnMessages = warnSpy.mock.calls.map((c) => String(c[0]));
expect(warnMessages.some((m) => m.includes("unrelated.ts") && m.includes("refusing to stage"))).toBe(true);
});
});
// ---------------------------------------------------------------------------
// Embedded-space path tests — verify NUL-delimited parsing handles spaces
// ---------------------------------------------------------------------------
// ---------------------------------------------------------------------------
// Embedded-space path tests — verify NUL-delimited (-z) parsing handles spaces
//
// Note on untracked files in new subdirectories: git reports untracked entries
// at the outermost untracked directory level (e.g. `?? dir with space/`),
// not at the individual file level, when the directory itself is new. This is
// standard git behaviour regardless of -z. For that reason the untracked tests
// below use root-level files or files inside already-tracked directories,
// which are the cases that actually round-trip through `snapshotDirtyFiles`.
// ---------------------------------------------------------------------------
describe("snapshotDirtyFiles — paths with embedded spaces", () => {
let dir: string;
beforeEach(() => {
dir = mkdtempSync(join(tmpdir(), "fn-snapshot-spaces-"));
initRepo(dir);
});
afterEach(() => {
rmSync(dir, { recursive: true, force: true });
});
it("captures a root-level untracked file whose name contains spaces", async () => {
// Root-level untracked files with spaces are reported verbatim by git (no quoting in -z mode).
writeFileSync(join(dir, "my file with spaces.ts"), "export const x = 1;\n");
const snapshot = await snapshotDirtyFiles(dir);
expect(snapshot.has("my file with spaces.ts")).toBe(true);
});
it("captures a tracked-unstaged file in a subdirectory whose path contains spaces", async () => {
// First commit the file so it is tracked (git diff reports full path including spaces).
mkdirSync(join(dir, "src dir"), { recursive: true });
writeFileSync(join(dir, "src dir", "my component.ts"), "export const v = 0;\n");
execSync("git add .", { cwd: dir, stdio: "pipe" });
execSync('git commit -m "chore: add spaced file"', { cwd: dir, stdio: "pipe" });
// Now modify it without staging — git diff -z --name-only emits the full path NUL-terminated.
writeFileSync(join(dir, "src dir", "my component.ts"), "export const v = 1;\n");
const snapshot = await snapshotDirtyFiles(dir);
expect(snapshot.has("src dir/my component.ts")).toBe(true);
});
it("captures a staged (cached) file in a subdirectory whose path contains spaces", async () => {
// Create the parent so it is already tracked, then add a new file.
mkdirSync(join(dir, "path with spaces"), { recursive: true });
writeFileSync(join(dir, "path with spaces", "keeper.ts"), "export {};\n");
execSync("git add .", { cwd: dir, stdio: "pipe" });
execSync('git commit -m "chore: track dir"', { cwd: dir, stdio: "pipe" });
// Now create a new file in the tracked dir and stage it.
writeFileSync(join(dir, "path with spaces", "index.ts"), "export const i = 1;\n");
execSync("git add .", { cwd: dir, stdio: "pipe" });
const snapshot = await snapshotDirtyFiles(dir);
// git diff -z --cached --name-only reports staged files with their full path.
expect(snapshot.has("path with spaces/index.ts")).toBe(true);
});
});
describe("commitOrAmendMergeWithFixes — embedded-space paths round-trip", () => {
let dir: string;
let warnSpy: ReturnType<typeof vi.spyOn>;
beforeEach(() => {
dir = mkdtempSync(join(tmpdir(), "fn-allowlist-spaces-"));
initRepo(dir);
warnSpy = vi.spyOn(mergerLog, "warn");
});
afterEach(() => {
warnSpy.mockRestore();
rmSync(dir, { recursive: true, force: true });
});
it("stages and commits a tracked file edited by the fix agent whose path contains spaces", async () => {
// Pre-commit the spaced file so it is a tracked path.
mkdirSync(join(dir, "src components"), { recursive: true });
writeFileSync(join(dir, "src components", "my widget.ts"), "export const w = 0;\n");
execSync("git add .", { cwd: dir, stdio: "pipe" });
execSync('git commit -m "chore: add spaced component"', { cwd: dir, stdio: "pipe" });
const preAttemptSha = squashBranch(dir, "feat/G", "feature-g.ts", "export const g = 1;\n");
// Fix agent modifies the tracked spaced file (tracked-unstaged after squash).
const spacedPath = "src components/my widget.ts";
writeFileSync(join(dir, "src components", "my widget.ts"), "export const w = 1; // fixed\n");
const result = await commitOrAmendMergeWithFixes(
dir,
"FN-TEST",
"feat/G",
"- feat: add feature-g.ts",
false,
preAttemptSha,
"",
undefined,
STUB_SETTINGS,
undefined,
null,
null,
new Set([spacedPath]), // fix agent touched this tracked file
);
expect(result).toBe(true);
// Verify both the squash file and the spaced file were committed.
const committedFiles = execSync("git diff --name-only HEAD~1 HEAD", {
cwd: dir,
stdio: "pipe",
}).toString().trim().split("\n");
expect(committedFiles).toContain("feature-g.ts");
expect(committedFiles).toContain(spacedPath);
// Working tree must be clean for the spaced file.
const dirty = execSync("git diff --name-only", { cwd: dir, stdio: "pipe" }).toString();
expect(dirty).not.toContain(spacedPath);
});
it("excludes an unrelated tracked file with spaces and emits a warn", async () => {
// Commit a tracked file with spaces so it appears in git diff (not git status -z untracked).
const spacedUnrelated = "user notes/scratch.ts";
mkdirSync(join(dir, "user notes"), { recursive: true });
writeFileSync(join(dir, "user notes", "scratch.ts"), "// original\n");
execSync("git add .", { cwd: dir, stdio: "pipe" });
execSync('git commit -m "chore: add user notes"', { cwd: dir, stdio: "pipe" });
const preAttemptSha = squashBranch(dir, "feat/H", "feature-h.ts", "export const h = 1;\n");
// User edits their tracked spaced file — not in the allowlist.
writeFileSync(join(dir, "user notes", "scratch.ts"), "// user WIP\n");
const result = await commitOrAmendMergeWithFixes(
dir,
"FN-TEST",
"feat/H",
"- feat: add feature-h.ts",
false,
preAttemptSha,
"",
undefined,
STUB_SETTINGS,
undefined,
null,
null,
new Set<string>(), // empty allowlist
);
expect(result).toBe(true);
const committedFiles = execSync("git diff --name-only HEAD~1 HEAD", {
cwd: dir,
stdio: "pipe",
}).toString().trim().split("\n");
expect(committedFiles).toContain("feature-h.ts");
expect(committedFiles).not.toContain(spacedUnrelated);
// The file must still be dirty in the working tree.
const dirty = execSync("git diff --name-only", { cwd: dir, stdio: "pipe" }).toString();
expect(dirty).toContain(spacedUnrelated);
const warnMessages = warnSpy.mock.calls.map((c) => String(c[0]));
expect(
warnMessages.some((m) => m.includes(spacedUnrelated) && m.includes("refusing to stage")),
).toBe(true);
});
});

View File

@@ -74,7 +74,38 @@ vi.mock("node:child_process", async () => {
}
});
});
return { execSync: execSyncFn, exec: execFn, spawn: spawnFn };
// execFile(file, args, opts, cb) — reassemble a shell-equivalent command and
// delegate to execSyncFn so the same mock infrastructure handles both exec and execFile.
const execFileFn: any = vi.fn((file: any, args: any, opts: any, cb: any) => {
// Normalize overloads: (file, args, cb) or (file, args, opts, cb)
const callback = typeof opts === "function" ? opts : cb;
const options = typeof opts === "function" ? {} : opts;
const cmd = [file, ...(Array.isArray(args) ? args : [])].join(" ");
try {
const out = execSyncFn(cmd, { stdio: ["pipe", "pipe", "pipe"], ...options });
const stdout = out === undefined ? "" : out.toString();
if (typeof callback === "function") callback(null, stdout, "");
} catch (err: any) {
if (typeof callback === "function") {
callback(err, err?.stdout?.toString?.() ?? "", err?.stderr?.toString?.() ?? "");
}
}
});
execFileFn[promisify.custom] = (file: any, args?: any, opts?: any) =>
new Promise((resolve, reject) => {
execFileFn(file, args, opts, (err: any, stdout: any, stderr: any) => {
if (err) {
err.stdout = stdout;
err.stderr = stderr;
reject(err);
} else {
resolve({ stdout, stderr });
}
});
});
return { execSync: execSyncFn, exec: execFn, execFile: execFileFn, spawn: spawnFn };
});
vi.mock("node:fs", () => ({
@@ -1205,8 +1236,8 @@ describe("push-after-merge", () => {
if (cmdStr.includes("git diff --name-only --diff-filter=U")) {
return hasConflicts ? "pnpm-lock.yaml" as any : "" as any;
}
if (cmdStr.startsWith('git checkout --ours "pnpm-lock.yaml"')) return Buffer.from("");
if (cmdStr.startsWith('git add "pnpm-lock.yaml"')) {
if (cmdStr.includes("checkout --ours") && cmdStr.includes("pnpm-lock.yaml")) return Buffer.from("");
if (cmdStr.includes("git add") && cmdStr.includes("pnpm-lock.yaml")) {
hasConflicts = false;
return Buffer.from("");
}
@@ -1235,7 +1266,7 @@ describe("push-after-merge", () => {
expect(result.pushed).toBe(true);
expect(
mockedExecSync.mock.calls.some((call) => String(call[0]).startsWith('git checkout --ours "pnpm-lock.yaml"')),
mockedExecSync.mock.calls.some((call) => String(call[0]).includes("checkout --ours") && String(call[0]).includes("pnpm-lock.yaml")),
).toBe(true);
expect(
mockedExecSync.mock.calls.some((call) => String(call[0]).startsWith("GIT_EDITOR=true git rebase --continue")),

View File

@@ -79,6 +79,35 @@ vi.mock("node:child_process", () => {
});
});
// execFile(file, args, opts, cb) — assemble a command string and delegate to
// execSyncFn so the same mock infrastructure covers execFile-based git calls.
const execFileFn: any = vi.fn((file: any, args: any, opts: any, cb: any) => {
const callback = typeof opts === "function" ? opts : cb;
const options = typeof opts === "function" ? undefined : opts;
const cmd = [file, ...(Array.isArray(args) ? args : [])].join(" ");
try {
const out = execSyncFn(cmd, options);
const stdout = out === undefined ? "" : out.toString();
if (typeof callback === "function") callback(null, stdout, "");
} catch (err: any) {
if (typeof callback === "function") {
callback(err, err?.stdout?.toString?.() ?? "", err?.stderr?.toString?.() ?? "");
}
}
});
execFileFn[promisify.custom] = (file: any, args?: any, opts?: any) =>
new Promise((resolve, reject) => {
execFileFn(file, args, opts, (err: any, stdout: any, stderr: any) => {
if (err) {
err.stdout = stdout;
err.stderr = stderr;
reject(err);
} else {
resolve({ stdout, stderr });
}
});
});
// spawn() is used by the merger's verification runner. Route it through the
// same execSyncFn mock so a single mockedExecSync.mockImplementation controls
// both git calls (execSync) and verification commands (spawn). Throwing from
@@ -104,7 +133,7 @@ vi.mock("node:child_process", () => {
return child;
});
return { execSync: execSyncFn, exec: execFn, spawn: spawnFn };
return { execSync: execSyncFn, exec: execFn, execFile: execFileFn, spawn: spawnFn };
});
vi.mock("node:fs", () => ({
existsSync: vi.fn().mockReturnValue(true),

View File

@@ -1,8 +1,9 @@
/* eslint-disable @typescript-eslint/no-explicit-any */
import { execSync, exec } from "node:child_process";
import { execSync, exec, execFile } from "node:child_process";
import { promisify } from "node:util";
const execAsync = promisify(exec);
const execFileAsync = promisify(execFile);
import {
runVerificationCommand as runVerificationCommandShared,
summarizeVerificationOutput,
@@ -355,6 +356,57 @@ export function throwIfAborted(signal: AbortSignal | undefined, taskId: string):
throw new MergeAbortedError(`Merge aborted for ${taskId}: engine shutdown requested`);
}
/**
* Return the union of all dirty paths in `rootDir`:
* - tracked files modified vs the index (`git diff --name-only`)
* - staged but not yet committed (`git diff --cached --name-only`)
* - untracked files (`git status --porcelain` lines starting with `??`)
*
* Errors are swallowed and an empty set is returned so callers are never
* blocked by a failing porcelain query.
*
* All three git queries use NUL-delimited output (`-z`) so paths with
* embedded spaces or special characters are parsed correctly without quoting.
*/
export async function snapshotDirtyFiles(rootDir: string): Promise<Set<string>> {
const paths = new Set<string>();
try {
const [unstagedOut, stagedOut, porcelainOut] = await Promise.all([
execFileAsync("git", ["diff", "-z", "--name-only"], { cwd: rootDir, encoding: "utf-8" }).then(
(r) => r.stdout,
() => "",
),
execFileAsync("git", ["diff", "-z", "--cached", "--name-only"], { cwd: rootDir, encoding: "utf-8" }).then(
(r) => r.stdout,
() => "",
),
execFileAsync("git", ["status", "-z", "--porcelain"], { cwd: rootDir, encoding: "utf-8" }).then(
(r) => r.stdout,
() => "",
),
]);
for (const entry of unstagedOut.split("\0")) {
const p = entry.trim();
if (p) paths.add(p);
}
for (const entry of stagedOut.split("\0")) {
const p = entry.trim();
if (p) paths.add(p);
}
// Untracked files: entries beginning with `?? ` (3-char prefix, no quoting in -z mode)
for (const entry of porcelainOut.split("\0")) {
if (!entry.startsWith("?? ")) continue;
const p = entry.slice(3);
if (p) paths.add(p);
}
} catch {
// Best-effort — an empty snapshot is safe: the allowlist logic will simply
// not add any fix-agent files, which is conservative.
}
return paths;
}
function rethrowIfMergeAborted(error: unknown): void {
if (error instanceof Error && error.name === "MergeAbortedError") {
throw error;
@@ -367,8 +419,7 @@ function rethrowIfMergeAborted(error: unknown): void {
* this helper normalises all three cases.
*/
function execSyncText(command: string, options: Parameters<typeof execSync>[1]): string {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const output: any = execSync(command, options);
const output = execSync(command, options);
if (output == null) return "";
if (typeof output === "string") return output.trim();
return (output as Buffer).toString("utf-8").trim();
@@ -551,6 +602,12 @@ async function runVerificationCommand(
* Attempt an in-merge verification fix by spawning an AI agent on the main branch.
* Returns true if verification passes after the fix, false otherwise.
* Never throws — errors are caught and logged, and the function returns false.
*
* @param fixModifiedFiles - Mutable set that this function populates with every
* path that changed during the fix agent's run (post-snapshot minus
* pre-snapshot). The caller passes this set across all fix attempts so that
* `commitOrAmendMergeWithFixes` can build an allowlist that covers every file
* the fix agent touched, regardless of how many retries were needed.
*/
async function attemptInMergeVerificationFix(
store: TaskStore,
@@ -568,7 +625,11 @@ async function attemptInMergeVerificationFix(
fixAttemptNumber?: number,
_testCommand?: string,
_buildCommand?: string,
fixModifiedFiles?: Set<string>,
): Promise<boolean> {
// Snapshot the working tree before doing anything so the diff reflects only
// what the fix agent touched, not pre-existing dirty state.
const preFixSnapshot = await snapshotDirtyFiles(rootDir);
try {
mergerLog.log(`${taskId}: spawning in-merge verification fix agent`);
@@ -702,6 +763,17 @@ ${failureContext.output.slice(0, VERIFICATION_LOG_MAX_CHARS)}
});
await accumulateSessionTokenUsage(store, taskId, session);
// Compute which paths the fix agent introduced or modified, then
// accumulate them into the caller's mutable set.
const postFixSnapshot = await snapshotDirtyFiles(rootDir);
if (fixModifiedFiles) {
for (const p of postFixSnapshot) {
if (!preFixSnapshot.has(p)) {
fixModifiedFiles.add(p);
}
}
}
// Re-run deterministic verification command after the fix attempt.
await store.logEntry(
taskId,
@@ -731,6 +803,19 @@ ${failureContext.output.slice(0, VERIFICATION_LOG_MAX_CHARS)}
}
} catch (err: unknown) {
rethrowIfMergeAborted(err);
// Even on failure, try to surface any paths the agent partially touched.
if (fixModifiedFiles) {
try {
const postFixSnapshot = await snapshotDirtyFiles(rootDir);
for (const p of postFixSnapshot) {
if (!preFixSnapshot.has(p)) {
fixModifiedFiles.add(p);
}
}
} catch {
// Best-effort only
}
}
const errorMessage = err instanceof Error ? err.message : String(err);
mergerLog.warn(`${taskId}: in-merge fix agent error: ${errorMessage}`);
await store.logEntry(taskId, "In-merge verification fix agent encountered an error", errorMessage);
@@ -892,10 +977,16 @@ async function buildDeterministicMergeMessage(params: {
* branch's actual step commits, so consumers of mergeDetails never see a
* hallucinated body that talks about files that aren't in the diff.
*
* Only files that are part of the squash or that the fix agent explicitly
* modified are staged. Any other dirty files in the working tree are left
* untouched and a warning is emitted for each one.
*
* Returns true on a successful commit/amend. Never throws — errors are logged
* and the function returns false (callers decide whether to abort the merge).
*
* @internal Exported for integration tests only — not part of the public API.
*/
async function commitOrAmendMergeWithFixes(
export async function commitOrAmendMergeWithFixes(
rootDir: string,
taskId: string,
branch: string,
@@ -908,19 +999,80 @@ async function commitOrAmendMergeWithFixes(
signal?: AbortSignal,
aiSummary?: string | null,
aiSubject?: string | null,
fixModifiedFiles: ReadonlySet<string> = new Set(),
): Promise<boolean> {
try {
// Stage everything (squash state + verification fixes the agent left
// unstaged). FN-2152 still applies: filter out any submodule gitlinks
// before committing.
const { stdout: unstagedFiles } = await execAsync("git diff --name-only", {
// Build an allowlist of paths we are permitted to stage.
// Allowlist = (already staged by squash) (unstaged ∩ fixModifiedFiles)
// We also handle untracked files created by the fix agent.
//
// FN-2152 still applies: the submodule-gitlink filter below removes any
// gitlinks that slip through (nested worktrees, etc.).
// 1. Read currently-staged files (squash produced these) for diagnostic logging.
const { stdout: squashStagedOut } = await execAsync("git diff --cached --name-only", {
cwd: rootDir,
encoding: "utf-8",
});
if (unstagedFiles.trim().length > 0) {
await execAsync("git add -A", { cwd: rootDir });
const squashStaged = new Set(squashStagedOut.split("\n").map((l) => l.trim()).filter(Boolean));
// 2. What is currently unstaged (tracked, modified-but-not-staged).
const { stdout: unstagedOut } = await execAsync("git diff --name-only", {
cwd: rootDir,
encoding: "utf-8",
});
const unstaged = new Set(unstagedOut.split("\n").map((l) => l.trim()).filter(Boolean));
// 3. Untracked files created by the fix agent (NUL-delimited, no quoting needed).
const { stdout: porcelainOut } = await execFileAsync("git", ["status", "-z", "--porcelain"], {
cwd: rootDir,
encoding: "utf-8",
});
const untracked = new Set<string>();
for (const entry of porcelainOut.split("\0")) {
if (!entry.startsWith("?? ")) continue;
const p = entry.slice(3);
if (p) untracked.add(p);
}
// 4. Stage each unstaged path that the fix agent touched (batched, no shell).
const unstagedToStage: string[] = [];
for (const p of unstaged) {
if (fixModifiedFiles.has(p)) {
unstagedToStage.push(p);
} else {
mergerLog.warn(
`${taskId}: refusing to stage unrelated working-tree change: ${p} (not part of squash or in-merge fix)`,
);
}
}
if (unstagedToStage.length > 0) {
await execFileAsync("git", ["add", "--", ...unstagedToStage], { cwd: rootDir });
}
// 5. Stage untracked files created by the fix agent (batched, no shell).
const untrackedToStage: string[] = [];
for (const p of untracked) {
if (fixModifiedFiles.has(p)) {
untrackedToStage.push(p);
} else {
mergerLog.warn(
`${taskId}: refusing to stage unrelated working-tree change: ${p} (not part of squash or in-merge fix)`,
);
}
}
if (untrackedToStage.length > 0) {
await execFileAsync("git", ["add", "--", ...untrackedToStage], { cwd: rootDir });
}
// Fix 3: cap long path lists to avoid unreadable single-line logs.
const cap = (arr: string[], n = 20) =>
arr.length <= n ? arr.join(", ") : `${arr.slice(0, n).join(", ")} ... (+${arr.length - n} more)`;
mergerLog.log(
`${taskId}: staging allowlist — squash: [${cap([...squashStaged])}], fixModified: [${cap([...fixModifiedFiles])}]`,
);
const { stdout: staged } = await execAsync("git diff --cached --raw", {
cwd: rootDir,
encoding: "utf-8",
@@ -1357,8 +1509,8 @@ export async function classifyConflict(filePath: string, cwd: string): Promise<C
*/
export async function resolveWithOurs(filePath: string, cwd: string): Promise<void> {
try {
await execAsync(`git checkout --ours "${filePath}"`, { cwd });
await execAsync(`git add "${filePath}"`, { cwd });
await execFileAsync("git", ["checkout", "--ours", "--", filePath], { cwd });
await execFileAsync("git", ["add", "--", filePath], { cwd });
mergerLog.log(`Auto-resolved ${filePath} using --ours`);
} catch (error) {
throw new Error(`Failed to auto-resolve ${filePath} with ours: ${error}`);
@@ -1371,8 +1523,8 @@ export async function resolveWithOurs(filePath: string, cwd: string): Promise<vo
*/
export async function resolveWithTheirs(filePath: string, cwd: string): Promise<void> {
try {
await execAsync(`git checkout --theirs "${filePath}"`, { cwd });
await execAsync(`git add "${filePath}"`, { cwd });
await execFileAsync("git", ["checkout", "--theirs", "--", filePath], { cwd });
await execFileAsync("git", ["add", "--", filePath], { cwd });
mergerLog.log(`Auto-resolved ${filePath} using --theirs`);
} catch (error) {
throw new Error(`Failed to auto-resolve ${filePath} with theirs: ${error}`);
@@ -1385,7 +1537,7 @@ export async function resolveWithTheirs(filePath: string, cwd: string): Promise<
*/
export async function resolveTrivialWhitespace(filePath: string, cwd: string): Promise<void> {
try {
await execAsync(`git add "${filePath}"`, { cwd });
await execFileAsync("git", ["add", "--", filePath], { cwd });
mergerLog.log(`Auto-resolved ${filePath} (trivial whitespace)`);
} catch (error) {
throw new Error(`Failed to auto-resolve ${filePath} trivial conflict: ${error}`);
@@ -3134,6 +3286,9 @@ export async function aiMergeTask(
if (failedResult) {
let fixSuccess = false;
// Accumulate all paths the fix agent touches across retries so
// commitOrAmendMergeWithFixes can build a precise allowlist.
const verificationFixModifiedFiles = new Set<string>();
for (let fixAttempt = 1; fixAttempt <= maxFixRetries; fixAttempt++) {
const fixAttemptStartedAt = Date.now();
mergerLog.log(`${taskId}: in-merge verification fix attempt ${fixAttempt}/${maxFixRetries}`);
@@ -3161,6 +3316,7 @@ export async function aiMergeTask(
fixAttempt,
effectiveTestCommand,
effectiveBuildCommand,
verificationFixModifiedFiles,
);
const fixAttemptDurationMs = Date.now() - fixAttemptStartedAt;
@@ -3206,6 +3362,7 @@ export async function aiMergeTask(
options.signal,
aiMergeSummary,
aiMergeSubject,
verificationFixModifiedFiles,
);
if (!finalized) {
// Phantom-merge guard: refused to fabricate a commit. Reset
@@ -3246,6 +3403,9 @@ export async function aiMergeTask(
const fixType = effectiveBuildCommand ? "build" as const : "test" as const;
let fixSuccess = false;
// Accumulate all paths the fix agent touches across retries so
// commitOrAmendMergeWithFixes can build a precise allowlist.
const buildFixModifiedFiles = new Set<string>();
for (let fixAttempt = 1; fixAttempt <= maxFixRetries; fixAttempt++) {
const fixAttemptStartedAt = Date.now();
mergerLog.log(`${taskId}: in-merge verification fix attempt ${fixAttempt}/${maxFixRetries}`);
@@ -3273,6 +3433,7 @@ export async function aiMergeTask(
fixAttempt,
effectiveTestCommand,
effectiveBuildCommand,
buildFixModifiedFiles,
);
const fixAttemptDurationMs = Date.now() - fixAttemptStartedAt;
@@ -3313,6 +3474,7 @@ export async function aiMergeTask(
options.signal,
aiMergeSummary,
aiMergeSubject,
buildFixModifiedFiles,
);
if (!finalized) {
// Phantom-merge guard: the verification fix passed but no