diff --git a/plugins/plugin-agent-orchestrator/__tests__/unit/workspace-diff.test.ts b/plugins/plugin-agent-orchestrator/__tests__/unit/workspace-diff.test.ts index 473ed45416d68..33f4e6b4aa538 100644 --- a/plugins/plugin-agent-orchestrator/__tests__/unit/workspace-diff.test.ts +++ b/plugins/plugin-agent-orchestrator/__tests__/unit/workspace-diff.test.ts @@ -1,5 +1,5 @@ import { execFileSync } from "node:child_process"; -import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { afterEach, beforeEach, describe, expect, it } from "vitest"; @@ -8,6 +8,7 @@ import { captureBaselineDirty, captureBaselineSha, captureChangeSet, + parseLsFiles, summarizeChangeSet, verifyChangedFilesOnDisk, } from "../../src/services/workspace-diff.ts"; @@ -304,3 +305,81 @@ describe("workspace-diff — unborn HEAD + untracked (#11578)", () => { expect(cs).toBeUndefined(); }); }); + +// The unborn-HEAD untracked scoop (37813124bf, #11605) could flood the +// MAX_CHANGED_FILES cap and evict the agent's real files: +// 1. a fresh scaffold that runs `npm install` BEFORE writing .gitignore has +// thousands of untracked node_modules paths (`--exclude-standard` has no +// .gitignore to honor yet), all of which entered the scoop; +// 2. agent-written tool paths were spread LAST into the changed-files union, +// and Set dedupe keeps first-occurrence order, so the flood evicted them. +describe("workspace-diff — unborn-HEAD scoop flood (#11605)", () => { + let dir: string; + + beforeEach(() => { + dir = mkdtempSync(join(tmpdir(), "wsdiff-flood-")); + git(dir, ["init", "-q"]); + git(dir, ["config", "user.email", "t@t.t"]); + git(dir, ["config", "user.name", "t"]); + // NO initial commit — HEAD is unborn. NO .gitignore — install ran first. + }); + + afterEach(() => { + rmSync(dir, { recursive: true, force: true }); + }); + + it("filters vendor install output that predates any .gitignore", async () => { + // Shell-scaffolded app (no tool paths at all) + `npm install` output. + writeFileSync(join(dir, "index.html"), "

app

\n"); + writeFileSync(join(dir, "package.json"), "{}\n"); + writeFileSync(join(dir, "server.js"), "require('http');\n"); + for (let i = 0; i < 120; i++) { + const pkg = join( + dir, + "node_modules", + `pkg-${String(i).padStart(3, "0")}`, + ); + mkdirSync(pkg, { recursive: true }); + writeFileSync(join(pkg, "index.js"), "module.exports={};\n"); + } + const cs = await captureChangeSet(dir); + expect(cs).toBeDefined(); + // ls-files sorts node_modules/* between index.html and package.json — + // without the vendor filter the cap kept index.html + 59 node_modules + // paths and evicted package.json and server.js entirely. + expect(cs?.changedFiles).toContain("index.html"); + expect(cs?.changedFiles).toContain("package.json"); + expect(cs?.changedFiles).toContain("server.js"); + expect(cs?.changedFiles.some((f) => f.startsWith("node_modules/"))).toBe( + false, + ); + }); + + it("agent-written files survive the cap ahead of a large scaffold", async () => { + // More legit untracked files than MAX_CHANGED_FILES, all sorting before + // the agent's tool-written file. + for (let i = 0; i < 70; i++) { + writeFileSync( + join(dir, `page-${String(i).padStart(2, "0")}.html`), + "

\n", + ); + } + writeFileSync(join(dir, "zzz-server.js"), "require('http');\n"); + const cs = await captureChangeSet(dir, undefined, ["zzz-server.js"]); + expect(cs).toBeDefined(); + // agentWritten-first: the explicit tool write leads the list and cannot + // be evicted by the cap (previously it sat at position 71 and was cut). + expect(cs?.changedFiles[0]).toBe("zzz-server.js"); + expect(cs?.changedFiles.length).toBeLessThanOrEqual(60); + expect(cs?.truncated).toBe(true); + }); + + it("parseLsFiles drops the truncated garbage tail of an over-maxBuffer listing", () => { + // A complete `git ls-files` listing always ends with a newline; output cut + // at maxBuffer (ENOBUFS) ends mid-path instead. + expect(parseLsFiles("a.txt\nb.txt\nnode_mod")).toEqual(["a.txt", "b.txt"]); + expect(parseLsFiles("a.txt\nb.txt\n")).toEqual(["a.txt", "b.txt"]); + expect(parseLsFiles("partial-only-no-newline")).toEqual([]); + expect(parseLsFiles(undefined)).toEqual([]); + }); +}); diff --git a/plugins/plugin-agent-orchestrator/src/services/workspace-diff.ts b/plugins/plugin-agent-orchestrator/src/services/workspace-diff.ts index 19cf2142de99c..589118873c6af 100644 --- a/plugins/plugin-agent-orchestrator/src/services/workspace-diff.ts +++ b/plugins/plugin-agent-orchestrator/src/services/workspace-diff.ts @@ -157,14 +157,55 @@ function parseNameStatus(out: string | undefined): string[] { return files; } -/** Parse `git ls-files --others` output (one path per line) into a path list. */ -function parseLsFiles(out: string | undefined): string[] { - return (out ?? "") +/** + * Parse `git ls-files --others` output (one path per line) into a path list. + * A complete listing always ends with a newline; when the output was cut at + * maxBuffer (ENOBUFS on a huge untracked tree) the tail is a truncated + * garbage path — drop the partial final line rather than surface junk. + */ +export function parseLsFiles(out: string | undefined): string[] { + if (!out) return []; + const complete = out.endsWith("\n") + ? out + : out.slice(0, out.lastIndexOf("\n") + 1); + return complete .split("\n") .map((line) => line.trim()) .filter((line) => line.length > 0); } +// Dependency/build directories a fresh scaffold populates BEFORE any +// .gitignore exists (`npm install` typically runs first). On an unborn HEAD +// `--exclude-standard` has no .gitignore to honor, so thousands of vendor +// paths would flood MAX_CHANGED_FILES and evict the agent's real files. +// Fallback for the unborn-HEAD untracked scoop ONLY — the born-HEAD path +// never scoops untracked files, and explicit tool-written paths are always +// kept regardless (agentWritten is unioned separately). +const UNBORN_SCOOP_VENDOR_DIRS = new Set([ + "node_modules", + ".git", + ".yarn", + ".pnpm-store", + ".venv", + "venv", + "__pycache__", + ".cache", + ".turbo", + ".next", + ".nuxt", + "dist", + "build", + "coverage", + "vendor", + "target", +]); + +function isVendorScoopPath(path: string): boolean { + return path + .split("/") + .some((segment) => UNBORN_SCOOP_VENDOR_DIRS.has(segment)); +} + /** * Resolve the base ref for the completion diff. Prefers the captured baseline * sha; otherwise HEAD — but when HEAD is unborn (a fresh repo with zero commits) @@ -279,11 +320,15 @@ export async function captureChangeSet( const untracked = unbornHead ? parseLsFiles( await git(workdir, ["ls-files", "--others", "--exclude-standard"]), - ).filter((file) => !dirtyAtSpawn.has(file)) + ).filter((file) => !dirtyAtSpawn.has(file) && !isVendorScoopPath(file)) : []; + // Agent-written paths FIRST: explicit edit/write tool calls are the + // highest-signal entries and must survive the MAX_CHANGED_FILES cap when a + // large scaffold floods `untracked`. Set dedupe keeps first-occurrence + // order, so spreading them last let the flood evict them entirely. const changedFiles = [ - ...new Set([...tracked, ...untracked, ...agentWritten]), + ...new Set([...agentWritten, ...tracked, ...untracked]), ].slice(0, MAX_CHANGED_FILES); if (changedFiles.length === 0) return undefined;