singularity/singularity-forge
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions 1

fix(gsd): auto-stash dirty files before squash merge and surface dirty filenames in error (#2298)

Browse source 
* fix: auto-stash dirty files before squash merge and surface dirty filenames in error

Two bugs in mergeMilestoneToMain caused milestone completion to fail when
the project root had pre-existing dirty tracked files:

Bug 1 — No auto-stash: clearProjectRootStateFiles only removes untracked
.gsd/ files. Any tracked dirty file elsewhere (e.g. .planning/work-state.json
with stash conflict markers) caused `git merge --squash` to reject with
"local changes would be overwritten". Fixed by adding a stash/pop wrapper
around the squash merge — dirty files are stashed before merge and restored
after commit. Stash is also popped on all error paths so local work is never
lost.

Bug 2 — Misleading error message: nativeMergeSquash discarded the filenames
from git stderr and the caller hardcoded blame on .gsd/ regardless of which
files were actually dirty. Fixed by parsing tab-indented filenames from git
stderr into a new `dirtyFiles` field on GitMergeResult, and surfacing them
in the error message.

Closes #2151

* ci: re-trigger CI (derive-state-db perf assertion is nondeterministic on slow runners)

* review: move #2151 tests to node:test format in separate file

Per review feedback, moved Tests 20 and 21 from the script-style
auto-worktree-milestone-merge.test.ts into a new auto-stash-merge.test.ts
using node:test's test() function and assert module.
This commit is contained in:
mastertyko 2026-03-24 14:17:26 +01:00 • committed by GitHub
parent e9e36f9568
commit 865dae2462
4 changed files with 227 additions and 34 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

92
src/resources/extensions/gsd/auto-worktree.ts
View file

@ -1098,7 +1098,32 @@ export function mergeMilestoneToMain(
}
}
// 7. Squash merge — auto-resolve .gsd/ state file conflicts (#530)
// 7. Stash any pre-existing dirty files so the squash merge is not
// blocked by unrelated local changes (#2151). clearProjectRootStateFiles
// only removes untracked .gsd/ files; tracked dirty files elsewhere (e.g.
// .planning/work-state.json with stash conflict markers) are invisible to
// that cleanup but will cause `git merge --squash` to reject.
let stashed = false;
try {
const status = execFileSync("git", ["status", "--porcelain"], {
cwd: originalBasePath_,
stdio: ["ignore", "pipe", "pipe"],
encoding: "utf-8",
}).trim();
if (status) {
execFileSync(
"git",
["stash", "push", "--include-untracked", "-m", `gsd: pre-merge stash for ${milestoneId}`],
{ cwd: originalBasePath_, stdio: ["ignore", "pipe", "pipe"], encoding: "utf-8" },
);
stashed = true;
}
} catch {
// Stash failure is non-fatal — proceed without stash and let the merge
// report the dirty tree if it fails.
}
// 8. Squash merge — auto-resolve .gsd/ state file conflicts (#530)
const mergeResult = nativeMergeSquash(originalBasePath_, milestoneBranch);
if (!mergeResult.success) {
@ -1106,12 +1131,27 @@ export function mergeMilestoneToMain(
// untracked .gsd/ files left by syncStateToProjectRoot). Preserve the
// milestone branch so commits are not lost.
if (mergeResult.conflicts.includes("__dirty_working_tree__")) {
// Pop stash before throwing so local work is not lost.
if (stashed) {
try {
execFileSync("git", ["stash", "pop"], {
cwd: originalBasePath_,
stdio: ["ignore", "pipe", "pipe"],
encoding: "utf-8",
});
} catch { /* stash pop conflict is non-fatal */ }
}
// Restore cwd so the caller is not stranded on the integration branch
process.chdir(previousCwd);
// Surface the actual dirty filenames from git stderr instead of
// generically blaming .gsd/ (#2151).
const fileList = mergeResult.dirtyFiles?.length
? `Dirty files:\n${mergeResult.dirtyFiles.map((f) => ` ${f}`).join("\n")}`
: `Check \`git status\` in the project root for details.`;
throw new GSDError(
GSD_GIT_ERROR,
`Squash merge of ${milestoneBranch} rejected: working tree has dirty or untracked files that conflict with the merge. ` +
`Clean the project root .gsd/ directory and retry.`,
`Squash merge of ${milestoneBranch} rejected: working tree has dirty or untracked files ` +
`that conflict with the merge. ${fileList}`,
);
}
@ -1147,6 +1187,16 @@ export function mergeMilestoneToMain(
// If there are still non-.gsd conflicts, escalate
if (codeConflicts.length > 0) {
// Pop stash before throwing so local work is not lost (#2151).
if (stashed) {
try {
execFileSync("git", ["stash", "pop"], {
cwd: originalBasePath_,
stdio: ["ignore", "pipe", "pipe"],
encoding: "utf-8",
});
} catch { /* stash pop conflict is non-fatal */ }
}
throw new MergeConflictError(
codeConflicts,
"squash",
@ -1158,11 +1208,11 @@ export function mergeMilestoneToMain(
// No conflicts detected — possibly "already up to date", fall through to commit
}
// 8. Commit (handle nothing-to-commit gracefully)
// 9. Commit (handle nothing-to-commit gracefully)
const commitResult = nativeCommit(originalBasePath_, commitMessage);
const nothingToCommit = commitResult === null;
// 8a. Clean up SQUASH_MSG left by git merge --squash (#1853).
// 9a. Clean up SQUASH_MSG left by git merge --squash (#1853).
// git only removes SQUASH_MSG when the commit reads it directly (plain
// `git commit`). nativeCommit uses `-F -` (stdin) or libgit2, neither
// of which trigger git's SQUASH_MSG cleanup. If left on disk, doctor
@ -1172,7 +1222,23 @@ export function mergeMilestoneToMain(
if (existsSync(squashMsgPath)) unlinkSync(squashMsgPath);
} catch { /* best-effort */ }
// 8b. Safety check (#1792): if nothing was committed, verify the milestone
// 9a-ii. Restore stashed files now that the merge+commit is complete (#2151).
// Pop after commit so stashed changes do not interfere with the squash merge
// or the commit content. Conflict on pop is non-fatal — the stash entry is
// preserved and the user can resolve manually with `git stash pop`.
if (stashed) {
try {
execFileSync("git", ["stash", "pop"], {
cwd: originalBasePath_,
stdio: ["ignore", "pipe", "pipe"],
encoding: "utf-8",
});
} catch {
// Stash pop conflict is non-fatal — stash entry persists for manual resolution.
}
}
// 9b. Safety check (#1792): if nothing was committed, verify the milestone
// work is already on the integration branch before allowing teardown.
// Compare only non-.gsd/ paths — .gsd/ state files diverge normally and
// are auto-resolved during the squash merge.
@ -1197,7 +1263,7 @@ export function mergeMilestoneToMain(
}
}
// 8c. Detect whether any non-.gsd/ code files were actually merged (#1906).
// 9c. Detect whether any non-.gsd/ code files were actually merged (#1906).
// When a milestone only produced .gsd/ metadata (summaries, roadmaps) but no
// real code, the user sees "milestone complete" but nothing changed in their
// codebase. Surface this so the caller can warn the user.
@ -1218,7 +1284,7 @@ export function mergeMilestoneToMain(
}
}
// 9. Auto-push if enabled
// 10. Auto-push if enabled
let pushed = false;
if (prefs.auto_push === true && !nothingToCommit) {
const remote = prefs.remote ?? "origin";
@ -1264,11 +1330,11 @@ export function mergeMilestoneToMain(
}
}
// 10. Guard removed — step 8b (#1792) now handles this with a smarter check:
// 11. Guard removed — step 9b (#1792) now handles this with a smarter check:
// throws only when the milestone has unanchored code changes, passes
// through when the code is genuinely already on the integration branch.
// 10a. Pre-teardown safety net (#1853): if the worktree still has uncommitted
// 11a. Pre-teardown safety net (#1853): if the worktree still has uncommitted
// changes (e.g. nativeHasChanges cache returned stale false, or auto-commit
// silently failed), force one final commit so code is not destroyed by
// `git worktree remove --force`.
@ -1292,7 +1358,7 @@ export function mergeMilestoneToMain(
}
}
// 11. Remove worktree directory first (must happen before branch deletion)
// 12. Remove worktree directory first (must happen before branch deletion)
try {
removeWorktree(originalBasePath_, milestoneId, {
branch: null as unknown as string,
@ -1302,14 +1368,14 @@ export function mergeMilestoneToMain(
// Best-effort -- worktree dir may already be gone
}
// 12. Delete milestone branch (after worktree removal so ref is unlocked)
// 13. Delete milestone branch (after worktree removal so ref is unlocked)
try {
nativeBranchDelete(originalBasePath_, milestoneBranch);
} catch {
// Best-effort
}
// 13. Clear module state
// 14. Clear module state
originalBase = null;
nudgeGitBranchCache(previousCwd);

13
src/resources/extensions/gsd/native-git-bridge.ts
View file

@ -58,6 +58,8 @@ interface GitBatchInfo {
interface GitMergeResult {
success: boolean;
conflicts: string[];
/** Filenames extracted from git stderr when a dirty working tree blocks the merge (#2151). */
dirtyFiles?: string[];
}
// ─── Native Module Loading ──────────────────────────────────────────────────
@ -863,7 +865,16 @@ export function nativeMergeSquash(basePath: string, branch: string): GitMergeRes
stderr.includes("not possible because you have unmerged files") ||
stderr.includes("overwritten by merge")
) {
return { success: false, conflicts: ["__dirty_working_tree__"] };
// Extract filenames from git stderr so callers can report which files
// are dirty instead of generically blaming .gsd/ (#2151).
// Git lists them as tab-indented lines between the "would be overwritten"
// header and the "Please commit" footer.
const dirtyFiles = stderr
.split("\n")
.filter((line) => line.startsWith("\t"))
.map((line) => line.trim())
.filter(Boolean);
return { success: false, conflicts: ["__dirty_working_tree__"], dirtyFiles };
}
// Check for real content conflicts

121
src/resources/extensions/gsd/tests/auto-stash-merge.test.ts Normal file
View file

@ -0,0 +1,121 @@
/**
* auto-stash-merge.test.ts — Regression tests for #2151.
*
* Tests that mergeMilestoneToMain auto-stashes dirty files before squash merge,
* and that nativeMergeSquash returns dirty filenames from git stderr.
*/
import test from "node:test";
import assert from "node:assert/strict";
import { mkdtempSync, mkdirSync, writeFileSync, rmSync, existsSync, readFileSync, realpathSync } from "node:fs";
import { join } from "node:path";
import { tmpdir } from "node:os";
import { execSync } from "node:child_process";
import { createAutoWorktree, mergeMilestoneToMain } from "../auto-worktree.ts";
import { nativeMergeSquash } from "../native-git-bridge.ts";
function run(cmd: string, cwd: string): string {
return execSync(cmd, { cwd, stdio: ["ignore", "pipe", "pipe"], encoding: "utf-8" }).trim();
}
function createTempRepo(): string {
const dir = realpathSync(mkdtempSync(join(tmpdir(), "wt-autostash-test-")));
run("git init", dir);
run("git config user.email test@test.com", dir);
run("git config user.name Test", dir);
writeFileSync(join(dir, "README.md"), "# test\n");
mkdirSync(join(dir, ".gsd"), { recursive: true });
writeFileSync(join(dir, ".gsd", "STATE.md"), "# State\n");
run("git add .", dir);
run("git commit -m init", dir);
run("git branch -M main", dir);
return dir;
}
function makeRoadmap(milestoneId: string, title: string, slices: Array<{ id: string; title: string }>): string {
const sliceLines = slices.map(s => `- [x] **${s.id}: ${s.title}**`).join("\n");
return `# ${milestoneId}: ${title}\n\n## Slices\n${sliceLines}\n`;
}
function addSliceToMilestone(
repo: string, wtPath: string, milestoneId: string,
sliceId: string, sliceTitle: string,
commits: Array<{ file: string; content: string; message: string }>,
): void {
const normalizedPath = wtPath.replaceAll("\\", "/");
const worktreeName = normalizedPath.split("/").pop() || milestoneId;
const sliceBranch = `slice/${worktreeName}/${sliceId}`;
run(`git checkout -b "${sliceBranch}"`, wtPath);
for (const c of commits) {
writeFileSync(join(wtPath, c.file), c.content);
run("git add .", wtPath);
run(`git commit -m "${c.message}"`, wtPath);
}
const milestoneBranch = `milestone/${milestoneId}`;
run(`git checkout "${milestoneBranch}"`, wtPath);
run(`git merge --no-ff "${sliceBranch}" -m "merge ${sliceId}: ${sliceTitle}"`, wtPath);
}
test("#2151 bug 1: auto-stash unblocks merge when unrelated files are dirty", () => {
const repo = createTempRepo();
try {
const wtPath = createAutoWorktree(repo, "M200");
addSliceToMilestone(repo, wtPath, "M200", "S01", "Stash test", [
{ file: "stash-test.ts", content: "export const stash = true;\n", message: "add stash test" },
]);
// Dirty an unrelated tracked file in the project root — this previously
// blocked the squash merge with "local changes would be overwritten".
writeFileSync(join(repo, "README.md"), "# modified locally\n");
const roadmap = makeRoadmap("M200", "Auto-stash test", [
{ id: "S01", title: "Stash test" },
]);
// Should succeed — the dirty README.md is auto-stashed before merge.
const result = mergeMilestoneToMain(repo, "M200", roadmap);
assert.ok(result.commitMessage.includes("feat(M200)"), "merge succeeds with dirty unrelated file");
assert.ok(existsSync(join(repo, "stash-test.ts")), "milestone code merged to main");
// Verify the dirty file was restored (stash popped).
const readmeContent = readFileSync(join(repo, "README.md"), "utf-8");
assert.equal(readmeContent, "# modified locally\n", "stash popped — dirty file restored after merge");
} finally {
rmSync(repo, { recursive: true, force: true });
}
});
test("#2151 bug 2: nativeMergeSquash returns dirty filenames", async () => {
const { nativeMergeSquash } = await import("../native-git-bridge.ts");
const repo = createTempRepo();
try {
run("git checkout -b milestone/M210", repo);
writeFileSync(join(repo, "overlap.ts"), "export const overlap = true;\n");
run("git add .", repo);
run('git commit -m "add overlap"', repo);
run("git checkout main", repo);
// Create the same file as a dirty local change
writeFileSync(join(repo, "overlap.ts"), "// local dirty version\n");
const result = nativeMergeSquash(repo, "milestone/M210");
assert.equal(result.success, false, "merge reports failure");
assert.ok(
result.conflicts.includes("__dirty_working_tree__"),
"conflicts include __dirty_working_tree__ sentinel",
);
assert.ok(
Array.isArray(result.dirtyFiles) && result.dirtyFiles.length > 0,
"dirtyFiles array is populated",
);
assert.ok(
result.dirtyFiles!.includes("overlap.ts"),
"dirtyFiles includes the actual dirty file name",
);
} finally {
run("git checkout -- . 2>/dev/null || true", repo);
rmSync(repo, { recursive: true, force: true });
}
});

35
src/resources/extensions/gsd/tests/auto-worktree-milestone-merge.test.ts
View file

@ -463,8 +463,11 @@ async function main(): Promise<void> {
assertTrue(existsSync(join(repo, "sync-test.ts")), "sync-test.ts on main after merge");
}
// ─── Test 11: #1738 Bug 1+2 — dirty tree merge preserves branch end-to-end ──
console.log("\n=== #1738 e2e: dirty tree rejection preserves branch ===");
// ─── Test 11: #1738 Bug 1+2 → #2151: dirty tree auto-stashed, merge succeeds ──
// Before #2151, a conflicting dirty file in the project root would cause
// the squash merge to reject. Now auto-stash moves it out of the way,
// the merge succeeds, and the user's local file goes to the stash.
console.log("\n=== #2151: dirty tree auto-stashed, merge succeeds ===");
{
const repo = freshRepo();
const wtPath = createAutoWorktree(repo, "M100");
@ -473,31 +476,21 @@ async function main(): Promise<void> {
{ file: "e2e.ts", content: "export const e2e = true;\n", message: "add e2e" },
]);
// Create a conflicting local file — previously blocked the merge.
writeFileSync(join(repo, "e2e.ts"), "// conflicting local file\n");
const roadmap = makeRoadmap("M100", "E2E dirty tree", [
{ id: "S01", title: "E2E test" },
]);
let threw = false;
let errorMsg = "";
try {
mergeMilestoneToMain(repo, "M100", roadmap);
} catch (err: unknown) {
threw = true;
errorMsg = err instanceof Error ? err.message : String(err);
}
assertTrue(threw, "#1738 e2e: throws on dirty working tree");
assertTrue(
errorMsg.includes("dirty") || errorMsg.includes("untracked") || errorMsg.includes("overwritten"),
"#1738 e2e: error identifies dirty tree cause",
);
// With auto-stash (#2151), the merge should succeed.
const result = mergeMilestoneToMain(repo, "M100", roadmap);
assertTrue(result.commitMessage.includes("feat(M100)"), "#2151: merge succeeds after auto-stash");
const branches = run("git branch", repo);
assertTrue(
branches.includes("milestone/M100"),
"#1738 e2e: milestone branch preserved on dirty tree rejection",
);
// The milestone code should be on main.
assertTrue(existsSync(join(repo, "e2e.ts")), "#2151: e2e.ts merged to main");
const content = readFileSync(join(repo, "e2e.ts"), "utf-8");
assertEq(content, "export const e2e = true;\n", "#2151: merged content is from milestone branch");
}
// ─── Test 12: Throw on unanchored code changes after empty commit (#1792) ─
@ -771,6 +764,8 @@ async function main(): Promise<void> {
assertTrue(existsSync(join(repo, "real-code.ts")), "real-code.ts merged to main");
}
// Tests 20 and 21 for #2151 are in auto-stash-merge.test.ts (node:test format).
} finally {
process.chdir(savedCwd);
for (const d of tempDirs) {