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

fix: prevent silent data loss when milestone merge fails due to dirty working tree (#1752)

Browse source 
Three bugs caused mergeMilestoneToMain to silently lose all milestone branch
commits when git merge --squash was rejected by dirty/untracked .gsd/ files:

1. nativeMergeSquash catch block returned success:true when git rejected a
   merge pre-staging ("local changes would be overwritten") because the
   --diff-filter=U conflict check found no markers. Now detects dirty tree
   rejections via stderr and returns a __dirty_working_tree__ sentinel.

2. mergeMilestoneToMain deleted the milestone branch unconditionally even
   when nothingToCommit was true (empty squash). Now throws with a clear
   error and preserves the branch to prevent data loss.

3. clearProjectRootStateFiles only removed STATE.md, auto.lock, and
   META.json but left synced milestone dirs and runtime/units from
   syncStateToProjectRoot. These untracked files blocked squash merges.
   Now cleans all untracked files in synced .gsd/ directories before merge.

Fixes #1738

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
Tom Boucher 2026-03-21 14:50:58 -04:00 • committed by GitHub
parent b78675b599
commit 2f0c078cc2
3 changed files with 203 additions and 106 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

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

@ -13,6 +13,7 @@ import {
readdirSync,
mkdirSync,
realpathSync,
rmSync,
unlinkSync,
lstatSync as lstatSyncFn,
} from "node:fs";
@ -77,6 +78,41 @@ function clearProjectRootStateFiles(basePath: string, milestoneId: string): void
/* non-fatal — file may not exist */
}
}
// Clean up entire synced milestone directory and runtime/units.
// syncStateToProjectRoot() copies these into the project root during
// execution. If they remain as untracked files when we attempt
// `git merge --squash`, git rejects the merge with "local changes would
// be overwritten", causing silent data loss (#1738).
const syncedDirs = [
join(gsdDir, "milestones", milestoneId),
join(gsdDir, "runtime", "units"),
];
for (const dir of syncedDirs) {
try {
if (existsSync(dir)) {
// Only remove files that are untracked by git — tracked files are
// managed by the branch checkout and should not be deleted.
const untrackedOutput = execFileSync(
"git",
["ls-files", "--others", "--exclude-standard", dir],
{ cwd: basePath, stdio: ["ignore", "pipe", "pipe"], encoding: "utf-8" },
).trim();
if (untrackedOutput) {
for (const f of untrackedOutput.split("\n").filter(Boolean)) {
try {
unlinkSync(join(basePath, f));
} catch {
/* non-fatal */
}
}
}
}
} catch {
/* non-fatal — git command may fail if not in repo */
}
}
}
// ─── Worktree ↔ Main Repo Sync (#1311) ──────────────────────────────────────
@ -962,6 +998,19 @@ export function mergeMilestoneToMain(
const mergeResult = nativeMergeSquash(originalBasePath_, milestoneBranch);
if (!mergeResult.success) {
// Dirty working tree — the merge was rejected before it started (e.g.
// untracked .gsd/ files left by syncStateToProjectRoot). Preserve the
// milestone branch so commits are not lost.
if (mergeResult.conflicts.includes("__dirty_working_tree__")) {
// Restore cwd so the caller is not stranded on the integration branch
process.chdir(previousCwd);
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.`,
);
}
// Check for conflicts — use merge result first, fall back to nativeConflictFiles
const conflictedFiles =
mergeResult.conflicts.length > 0
@ -1052,36 +1101,36 @@ export function mergeMilestoneToMain(
}
}
// 10. Remove worktree directory first (must happen before branch deletion)
// ONLY when a commit was actually produced — if nativeCommit returned null
// (nothing to commit), tearing down the worktree would destroy source code
// that was never merged (#1672).
// 10. Guard: if squash produced nothing to commit, the milestone branch has
// changes that were not merged. Preserve the branch and worktree so
// commits are not silently lost (#1672, #1738).
if (nothingToCommit) {
// eslint-disable-next-line no-console
console.warn(
`[GSD] Warning: squash merge of ${milestoneBranch} produced nothing to commit. ` +
"Worktree and branch preserved to prevent data loss. " +
"Inspect the worktree manually and retry.",
process.chdir(previousCwd);
throw new GSDError(
GSD_GIT_ERROR,
`Squash merge of ${milestoneBranch} produced an empty commit — milestone branch preserved to prevent data loss. ` +
`Inspect the branch manually and retry.`,
);
} else {
try {
removeWorktree(originalBasePath_, milestoneId, {
branch: null as unknown as string,
deleteBranch: false,
});
} catch {
// Best-effort -- worktree dir may already be gone
}
// 11. Delete milestone branch (after worktree removal so ref is unlocked)
try {
nativeBranchDelete(originalBasePath_, milestoneBranch);
} catch {
// Best-effort
}
}
// 12. Clear module state
// 11. Remove worktree directory first (must happen before branch deletion)
try {
removeWorktree(originalBasePath_, milestoneId, {
branch: null as unknown as string,
deleteBranch: false,
});
} catch {
// Best-effort -- worktree dir may already be gone
}
// 12. Delete milestone branch (after worktree removal so ref is unlocked)
try {
nativeBranchDelete(originalBasePath_, milestoneBranch);
} catch {
// Best-effort
}
// 13. Clear module state
originalBase = null;
nudgeGitBranchCache(previousCwd);

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

@ -850,9 +850,22 @@ export function nativeMergeSquash(basePath: string, branch: string): GitMergeRes
});
return { success: true, conflicts: [] };
} catch (err: unknown) {
// Check for conflicts — only treat as recoverable if actual conflict
// markers are present. Other failures (bad ref, corrupt repo, etc.)
// must propagate so callers don't assume the merge succeeded (#1672).
// Distinguish pre-merge rejections (dirty working tree) from actual
// content conflicts. When git rejects the merge before staging
// ("local changes would be overwritten"), there are no conflict markers
// to detect, so the old --diff-filter=U check would return an empty
// list and incorrectly report success (#1672, #1738).
const stderr =
err instanceof Error ? (err as Error & { stderr?: string }).stderr ?? err.message : String(err);
if (
stderr.includes("local changes would be overwritten") ||
stderr.includes("not possible because you have unmerged files") ||
stderr.includes("overwritten by merge")
) {
return { success: false, conflicts: ["__dirty_working_tree__"] };
}
// Check for real content conflicts
const conflictOutput = gitExec(basePath, ["diff", "--name-only", "--diff-filter=U"], true);
const conflicts = conflictOutput ? conflictOutput.split("\n").filter(Boolean) : [];
if (conflicts.length > 0) {

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

@ -181,8 +181,8 @@ async function main(): Promise<void> {
assertTrue(gitMsg.includes("- S01: Core API"), "git commit body has S01");
}
// ─── Test 3: Nothing to commit — no changes ────────────────────────
console.log("\n=== nothing to commit — no changes ===");
// ─── Test 3: Nothing to commit — preserves branch (#1738) ──────────
console.log("\n=== nothing to commit — preserves branch (#1738) ===");
{
const repo = freshRepo();
const wtPath = createAutoWorktree(repo, "M030");
@ -190,15 +190,21 @@ async function main(): Promise<void> {
// Don't add any slices/changes — milestone branch is identical to main
const roadmap = makeRoadmap("M030", "Empty milestone", []);
// Should complete without throwing
// Should throw to prevent silent branch deletion when squash is empty
let threw = false;
let errorMsg = "";
try {
const result = mergeMilestoneToMain(repo, "M030", roadmap);
assertTrue(typeof result.pushed === "boolean", "returns result even with nothing to commit");
} catch {
mergeMilestoneToMain(repo, "M030", roadmap);
} catch (err: unknown) {
threw = true;
errorMsg = err instanceof Error ? err.message : String(err);
}
assertTrue(!threw, "does not throw on nothing-to-commit");
assertTrue(threw, "throws on nothing-to-commit to preserve branch");
assertTrue(errorMsg.includes("empty commit"), "error message mentions empty commit");
// Milestone branch must still exist — not deleted
const branches = run("git branch", repo);
assertTrue(branches.includes("milestone/M030"), "milestone branch preserved when squash is empty");
// Main log unchanged (only init commit)
const mainLog = run("git log --oneline main", repo);
@ -327,18 +333,8 @@ async function main(): Promise<void> {
}
// ─── Test 7: Repo using `master` as default branch (#1668) ────────
//
// Reproduces the exact failure mode from the bug report: a repo initialised
// with `master`, no GSD preferences file setting `main_branch`, and no
// META.json (so readIntegrationBranch returns null). Before the fix,
// mergeMilestoneToMain would fall back to the hardcoded string "main",
// attempt `git checkout main`, fail, and leave the user with a broken state
// and a confusing error. After the fix, nativeDetectMainBranch detects
// `master` and the squash-merge succeeds normally.
console.log("\n=== master-branch repo — no META.json, no prefs (#1668) ===");
{
// Build a repo with `master` as the default branch (not `main`).
// Use -b master to override the system default (which may be `main`).
const dir = realpathSync(mkdtempSync(join(tmpdir(), "wt-ms-master-test-")));
tempDirs.push(dir);
run("git init -b master", dir);
@ -349,19 +345,14 @@ async function main(): Promise<void> {
writeFileSync(join(dir, ".gsd", "STATE.md"), "# State\n");
run("git add .", dir);
run("git commit -m init", dir);
// Leave the default branch as `master` — do NOT run `git branch -M main`
const defaultBranch = run("git rev-parse --abbrev-ref HEAD", dir);
assertEq(defaultBranch, "master", "repo is on master branch");
// Create a worktree for the milestone (creates milestone/M070 branch)
const wtPath = createAutoWorktree(dir, "M070");
addSliceToMilestone(dir, wtPath, "M070", "S01", "Master branch test", [
{ file: "master-feature.ts", content: "export const masterFeature = true;\n", message: "add master feature" },
]);
// No META.json written (simulates the captureIntegrationBranch failure
// described in the issue) — readIntegrationBranch will return null.
const metaFile = join(dir, ".gsd", "milestones", "M070", "M070-META.json");
assertTrue(!existsSync(metaFile), "no META.json — integration branch not captured");
@ -369,7 +360,6 @@ async function main(): Promise<void> {
{ id: "S01", title: "Master branch test" },
]);
// Should succeed: nativeDetectMainBranch detects `master` automatically.
let threw = false;
let errMsg = "";
try {
@ -378,11 +368,9 @@ async function main(): Promise<void> {
} catch (err) {
threw = true;
errMsg = err instanceof Error ? err.message : String(err);
console.error("Unexpected error:", err);
}
assertTrue(!threw, `should not throw on master-branch repo (got: ${errMsg})`);
// Verify the code landed on master and the milestone branch is gone
const finalBranch = run("git rev-parse --abbrev-ref HEAD", dir);
assertEq(finalBranch, "master", "repo is still on master after merge");
assertTrue(existsSync(join(dir, "master-feature.ts")), "feature merged to master");
@ -390,88 +378,135 @@ async function main(): Promise<void> {
assertTrue(!branches.includes("milestone/M070"), "milestone branch deleted after merge");
}
// ─── Test 8: Worktree preserved when commit is empty (#1672) ──────
console.log("\n=== worktree preserved when commit is empty (#1672) ===");
// ─── Test 8: #1738 Bug 1 — dirty working tree detected by nativeMergeSquash ──
console.log("\n=== #1738 bug 1: nativeMergeSquash detects dirty working tree ===");
{
const { nativeMergeSquash } = await import("../native-git-bridge.ts");
const repo = freshRepo();
run("git checkout -b milestone/M070", repo);
writeFileSync(join(repo, "feature.ts"), "export const feature = true;\n");
run("git add .", repo);
run('git commit -m "add feature"', repo);
run("git checkout main", repo);
writeFileSync(join(repo, "feature.ts"), "// local dirty version\n");
const result = nativeMergeSquash(repo, "milestone/M070");
assertEq(result.success, false, "merge reports failure on dirty working tree");
assertTrue(
result.conflicts.includes("__dirty_working_tree__"),
"conflicts include __dirty_working_tree__ sentinel",
);
run("git checkout -- . 2>/dev/null || true", repo);
run("rm -f feature.ts", repo);
}
// ─── Test 9: #1738 Bug 2 — branch preserved on empty squash commit ──
console.log("\n=== #1738 bug 2: branch preserved when squash commit empty ===");
{
const repo = freshRepo();
const wtPath = createAutoWorktree(repo, "M080");
// Do NOT add any slices/changes — milestone branch is identical to main.
// This simulates the WSL stat-cache bug where autoCommitCurrentBranch
// skips commits, leaving the milestone branch identical to main.
// Make no changes — squash will produce nothing to commit
const roadmap = makeRoadmap("M080", "Empty milestone", []);
// Capture console.warn to verify the warning is emitted
const warnings: string[] = [];
const origWarn = console.warn;
console.warn = (...args: unknown[]) => {
warnings.push(args.map(String).join(" "));
};
let threw = false;
try {
mergeMilestoneToMain(repo, "M080", roadmap);
} finally {
console.warn = origWarn;
} catch (err: unknown) {
threw = true;
const msg = err instanceof Error ? err.message : String(err);
assertTrue(msg.includes("empty commit"), "#1738 error says empty commit");
assertTrue(msg.includes("preserved"), "#1738 error says branch preserved");
}
assertTrue(threw, "#1738 throws to prevent silent data loss");
// Milestone branch must still exist (not deleted)
const branches = run("git branch", repo);
assertTrue(
branches.includes("milestone/M080"),
"milestone branch preserved when nothing was committed (#1672)",
);
// A warning should have been emitted
assertTrue(
warnings.some((w) => w.includes("nothing to commit")),
"emits warning about empty merge (#1672)",
"#1738 milestone branch NOT deleted on empty squash",
);
}
// ─── Test 9: Worktree removed when commit succeeds (#1672) ──────
console.log("\n=== worktree removed when commit succeeds (#1672) ===");
// ─── Test 10: #1738 Bug 3 — clearProjectRootStateFiles cleans synced dirs ──
console.log("\n=== #1738 bug 3: synced .gsd/ dirs cleaned before merge ===");
{
const repo = freshRepo();
const wtPath = createAutoWorktree(repo, "M090");
addSliceToMilestone(repo, wtPath, "M090", "S01", "Teardown test", [
{ file: "teardown.ts", content: "export const teardown = true;\n", message: "add teardown file" },
addSliceToMilestone(repo, wtPath, "M090", "S01", "Sync test", [
{ file: "sync-test.ts", content: "export const sync = true;\n", message: "add sync-test" },
]);
const roadmap = makeRoadmap("M090", "Teardown verification", [
{ id: "S01", title: "Teardown test" },
]);
mergeMilestoneToMain(repo, "M090", roadmap);
// Milestone branch must be deleted
const branches = run("git branch", repo);
assertTrue(
!branches.includes("milestone/M090"),
"milestone branch deleted after successful commit (#1672)",
// Simulate syncStateToProjectRoot: create untracked .gsd/ milestone files
const msDir = join(repo, ".gsd", "milestones", "M090", "slices", "S01");
mkdirSync(msDir, { recursive: true });
writeFileSync(join(msDir, "S01-PLAN.md"), "# synced plan\n");
writeFileSync(
join(repo, ".gsd", "milestones", "M090", "M090-ROADMAP.md"),
"# synced roadmap\n",
);
// Worktree directory must be removed
const worktreeDir = join(repo, ".gsd", "worktrees", "M090");
assertTrue(!existsSync(worktreeDir), "worktree directory removed after successful commit (#1672)");
const runtimeDir = join(repo, ".gsd", "runtime", "units");
mkdirSync(runtimeDir, { recursive: true });
writeFileSync(join(runtimeDir, "unit-001.json"), '{"stale": true}');
// File should be on main
assertTrue(existsSync(join(repo, "teardown.ts")), "teardown.ts merged to main (#1672)");
}
const roadmap = makeRoadmap("M090", "Sync cleanup test", [
{ id: "S01", title: "Sync test" },
]);
// ─── Test 10: nativeMergeSquash throws on non-conflict failures (#1672) ─
console.log("\n=== nativeMergeSquash throws on non-conflict failures (#1672) ===");
{
const repo = freshRepo();
// Merge a nonexistent branch — a non-conflict failure that must throw
let threw = false;
try {
nativeMergeSquash(repo, "nonexistent-branch");
} catch {
const result = mergeMilestoneToMain(repo, "M090", roadmap);
assertTrue(
result.commitMessage.includes("feat(M090)"),
"#1738 merge succeeds after cleaning synced dirs",
);
} catch (err: unknown) {
threw = true;
console.error("#1738 bug 3 regression:", err);
}
assertTrue(threw, "nativeMergeSquash throws on nonexistent branch (#1672)");
assertTrue(!threw, "#1738 merge does not fail on synced .gsd/ files");
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 ===");
{
const repo = freshRepo();
const wtPath = createAutoWorktree(repo, "M100");
addSliceToMilestone(repo, wtPath, "M100", "S01", "E2E test", [
{ file: "e2e.ts", content: "export const e2e = true;\n", message: "add e2e" },
]);
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",
);
const branches = run("git branch", repo);
assertTrue(
branches.includes("milestone/M100"),
"#1738 e2e: milestone branch preserved on dirty tree rejection",
);
}
} finally {