From e109883ee2b04e01b2a856c63b9634164812c874 Mon Sep 17 00:00:00 2001 From: roboomp Date: Wed, 1 Jul 2026 22:03:23 +0000 Subject: [PATCH] fix(coding-agent/task): treated stash cleanup paths literally Failed stash-pop cleanup now invokes git clean with literal pathspecs for stash-derived untracked paths. Filenames such as `:(glob)*` are valid POSIX filenames and valid Git pathspec magic; passing them as ordinary pathspecs with `-x` could delete unrelated ignored artifacts that were never stashed and are not recoverable from the preserved stash. Extend the fallback regression with a literal `:(glob)*` stash file and an ignored `build.log` that must survive cleanup. Fixes #4175 --- packages/coding-agent/src/utils/git.ts | 15 +++++++-- .../coding-agent/test/task/worktree.test.ts | 31 ++++++++++++------- 2 files changed, 32 insertions(+), 14 deletions(-) diff --git a/packages/coding-agent/src/utils/git.ts b/packages/coding-agent/src/utils/git.ts index 8a59bfe8f..a441466ed 100644 --- a/packages/coding-agent/src/utils/git.ts +++ b/packages/coding-agent/src/utils/git.ts @@ -1635,7 +1635,7 @@ export const stash = { } if (restoredUntracked.length > 0) { try { - await clean(cwd, { includeIgnored: true, paths: restoredUntracked }); + await clean(cwd, { includeIgnored: true, literalPathspecs: true, paths: restoredUntracked }); } catch { /* best-effort cleanup — do not mask the primary conflict */ } @@ -1709,9 +1709,18 @@ export async function reset( export async function clean( cwd: string, - options: { ignoredOnly?: boolean; includeIgnored?: boolean; paths?: readonly string[]; signal?: AbortSignal } = {}, + options: { + ignoredOnly?: boolean; + includeIgnored?: boolean; + literalPathspecs?: boolean; + paths?: readonly string[]; + signal?: AbortSignal; + } = {}, ): Promise { - const args = ["clean", options.ignoredOnly ? "-fdX" : options.includeIgnored ? "-fdx" : "-fd"]; + const args = [options.literalPathspecs ? "--literal-pathspecs" : undefined, "clean"].filter( + (arg): arg is string => arg !== undefined, + ); + args.push(options.ignoredOnly ? "-fdX" : options.includeIgnored ? "-fdx" : "-fd"); if (options.paths?.length) args.push("--", ...options.paths); await runEffect(cwd, args, { signal: options.signal }); } diff --git a/packages/coding-agent/test/task/worktree.test.ts b/packages/coding-agent/test/task/worktree.test.ts index c11f3c63a..b1d12fdae 100644 --- a/packages/coding-agent/test/task/worktree.test.ts +++ b/packages/coding-agent/test/task/worktree.test.ts @@ -281,19 +281,24 @@ describe("worktree isolation helpers", () => { expect(delta.rootPatch).toContain("+downstream edit"); }); - it("removes ignored untracked files partially restored before failed stash pop exits", async () => { + it("cleans restored stash files with literal pathspecs", async () => { // Force the fallback branch: preflight would normally refuse this // pop before Git can restore anything, but mode/delete edge cases can // still pass preflight and fail during the actual stash pop. Git can // restore unrelated untracked files before reporting the tracked // conflict. If the task branch also adds an ignore rule for that - // restored path, default `git clean -fd -- ` leaves it behind; - // the fallback must clean restored ignored paths too. + // restored path, the fallback must clean the restored ignored path + // without interpreting stash-derived filenames as pathspec magic. + const magicName = ":(glob)*"; + const buildLog = path.join(repo, "build.log"); const ignoredBranch = "task/ignored-restored-untracked"; + await fs.writeFile(path.join(repo, ".gitignore"), "*.log\n"); + await runGit(repo, ["add", ".gitignore"]); + await runGit(repo, ["commit", "-q", "-m", "ignore-build-artifacts"]); await runGit(repo, ["checkout", "-q", "-b", ignoredBranch]); await Promise.all([ fs.writeFile(path.join(repo, "merged.txt"), "task branch change\n"), - fs.writeFile(path.join(repo, ".gitignore"), "note.txt\n"), + fs.writeFile(path.join(repo, ".gitignore"), `*.log\n${magicName}\n`), ]); await runGit(repo, ["add", ".gitignore", "merged.txt"]); await runGit(repo, ["commit", "-q", "-m", "task-change-ignored-note"]); @@ -301,17 +306,18 @@ describe("worktree isolation helpers", () => { try { vi.spyOn(git.patch, "canApplyText").mockResolvedValue(true); await fs.writeFile(path.join(repo, "merged.txt"), "user wip\n"); - await fs.writeFile(path.join(repo, "note.txt"), "untracked wip\n"); + await fs.writeFile(path.join(repo, magicName), "untracked wip\n"); + await fs.writeFile(buildLog, "ignored build artifact\n"); const result = await mergeTaskBranches(repo, [{ branchName: ignoredBranch, taskId: "task-1" }]); - const [status, ignoredStatus, unmerged, stashList, headContent, noteExists] = await Promise.all([ + const [status, unmerged, stashList, headContent, magicExists, buildLogExists] = await Promise.all([ runGit(repo, ["status", "--porcelain=v1"]), - runGit(repo, ["status", "--porcelain=v1", "--ignored=matching"]), runGit(repo, ["ls-files", "--unmerged"]), runGit(repo, ["stash", "list"]), fs.readFile(path.join(repo, "merged.txt"), "utf8"), - Bun.file(path.join(repo, "note.txt")).exists(), + Bun.file(path.join(repo, magicName)).exists(), + Bun.file(buildLog).exists(), ]); expect(result.merged).toEqual([ignoredBranch]); @@ -319,13 +325,16 @@ describe("worktree isolation helpers", () => { expect(result.stashConflict).toBeDefined(); expect(unmerged).toBe(""); expect(status).toBe(""); - expect(ignoredStatus).toBe(""); - expect(noteExists).toBe(false); + expect(magicExists).toBe(false); + expect(buildLogExists).toBe(true); expect(headContent).toBe("task branch change\n"); expect(stashList).toContain("omp-task-merge"); } finally { await cleanupTaskBranches(repo, [ignoredBranch]); - await fs.rm(path.join(repo, "note.txt"), { force: true }); + await Promise.all([ + fs.rm(path.join(repo, magicName), { force: true }), + fs.rm(buildLog, { force: true }), + ]); } });