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

fix(auto): verify merge anchored before worktree teardown (#1829)

Browse source 
* fix(auto): verify merge anchored before worktree teardown

Fixes #1792

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: replace unconditional empty-commit throw with anchor check

Step 10's unconditional throw on nothingToCommit conflicted with step
8b's smarter anchor check (#1792). Step 8b correctly distinguishes
"genuinely safe empty" (code already on main) from "data loss risk"
(unanchored code changes). Updated tests accordingly.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
Tom Boucher 2026-03-21 15:23:50 -04:00 • committed by GitHub
parent 77d88407ab
commit a7ad0caf9f
2 changed files with 109 additions and 30 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

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

@ -56,6 +56,7 @@ import {
nativeRmForce,
nativeBranchDelete,
nativeBranchExists,
nativeDiffNumstat,
} from "./native-git-bridge.js";
// ─── Module State ──────────────────────────────────────────────────────────
@ -932,7 +933,8 @@ function autoCommitDirtyState(cwd: string): boolean {
* 9. Clear originalBase
*
* On merge conflict: throws MergeConflictError.
* On "nothing to commit" after squash: handles gracefully (no error).
* On "nothing to commit" after squash: safe only if milestone work is already
* on the integration branch. Throws if unanchored code changes would be lost.
*/
export function mergeMilestoneToMain(
originalBasePath_: string,
@ -1064,6 +1066,31 @@ export function mergeMilestoneToMain(
const commitResult = nativeCommit(originalBasePath_, commitMessage);
const nothingToCommit = commitResult === null;
// 8b. 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.
if (nothingToCommit) {
const numstat = nativeDiffNumstat(
originalBasePath_,
mainBranch,
milestoneBranch,
);
const codeChanges = numstat.filter(
(entry) => !entry.path.startsWith(".gsd/"),
);
if (codeChanges.length > 0) {
// Milestone has unanchored code changes — abort teardown.
process.chdir(previousCwd);
throw new GSDError(
GSD_GIT_ERROR,
`Squash merge produced nothing to commit but milestone branch "${milestoneBranch}" ` +
`has ${codeChanges.length} code file(s) not on "${mainBranch}". ` +
`Aborting worktree teardown to prevent data loss.`,
);
}
}
// 9. Auto-push if enabled
let pushed = false;
if (prefs.auto_push === true && !nothingToCommit) {
@ -1107,17 +1134,9 @@ export function mergeMilestoneToMain(
}
}
// 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) {
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.`,
);
}
// 10. Guard removed — step 8b (#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.
// 11. Remove worktree directory first (must happen before branch deletion)
try {

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

@ -182,7 +182,7 @@ async function main(): Promise<void> {
}
// ─── Test 3: Nothing to commit — preserves branch (#1738) ──────────
console.log("\n=== nothing to commit — preserves branch (#1738) ===");
console.log("\n=== nothing to commit — safe when no code changes (#1738, #1792) ===");
{
const repo = freshRepo();
const wtPath = createAutoWorktree(repo, "M030");
@ -190,7 +190,8 @@ async function main(): Promise<void> {
// Don't add any slices/changes — milestone branch is identical to main
const roadmap = makeRoadmap("M030", "Empty milestone", []);
// Should throw to prevent silent branch deletion when squash is empty
// Should NOT throw — milestone branch is identical to main, nothing to lose.
// The anchor check (#1792) verifies no code files differ and passes through.
let threw = false;
let errorMsg = "";
try {
@ -199,12 +200,7 @@ async function main(): Promise<void> {
threw = true;
errorMsg = err instanceof Error ? err.message : String(err);
}
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");
assertTrue(!threw, `safe empty milestone should not throw (got: ${errorMsg})`);
// Main log unchanged (only init commit)
const mainLog = run("git log --oneline main", repo);
@ -412,22 +408,17 @@ async function main(): Promise<void> {
// Make no changes — squash will produce nothing to commit
const roadmap = makeRoadmap("M080", "Empty milestone", []);
// With the #1792 anchor check, empty milestones with no code changes
// are safe to proceed — no data to lose.
let threw = false;
let errMsg = "";
try {
mergeMilestoneToMain(repo, "M080", roadmap);
} 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");
errMsg = err instanceof Error ? err.message : String(err);
}
assertTrue(threw, "#1738 throws to prevent silent data loss");
const branches = run("git branch", repo);
assertTrue(
branches.includes("milestone/M080"),
"#1738 milestone branch NOT deleted on empty squash",
);
assertTrue(!threw, `empty milestone with no code changes should not throw (got: ${errMsg})`);
}
// ─── Test 10: #1738 Bug 3 — clearProjectRootStateFiles cleans synced dirs ──
@ -509,6 +500,75 @@ async function main(): Promise<void> {
);
}
// ─── Test 12: Throw on unanchored code changes after empty commit (#1792) ─
console.log("\n=== throw on unanchored code changes after empty commit (#1792) ===");
{
const repo = freshRepo();
const wtPath = createAutoWorktree(repo, "M120");
addSliceToMilestone(repo, wtPath, "M120", "S01", "Critical feature", [
{ file: "critical.ts", content: "export const critical = true;\n", message: "add critical feature" },
]);
// Simulate: merge then revert — git considers branch "already merged"
// but code is NOT on main (reverted).
run(`git merge milestone/M120 --no-ff -m "merge M120"`, repo);
run("git revert HEAD --no-edit -m 1", repo);
const roadmap = makeRoadmap("M120", "Critical milestone", [
{ id: "S01", title: "Critical feature" },
]);
let threw = false;
let errMsg = "";
try {
mergeMilestoneToMain(repo, "M120", roadmap);
} catch (err) {
threw = true;
errMsg = err instanceof Error ? err.message : String(err);
}
assertTrue(threw, "throws when milestone has unanchored code changes (#1792)");
assertTrue(
errMsg.includes("code file(s) not on"),
"error message mentions unanchored code files (#1792)",
);
const branches = run("git branch", repo);
assertTrue(
branches.includes("milestone/M120"),
"milestone branch preserved when code is unanchored (#1792)",
);
}
// ─── Test 13: Safe teardown when nothing-to-commit and work already on main (#1792) ─
console.log("\n=== safe teardown — nothing to commit, work already on main (#1792) ===");
{
const repo = freshRepo();
const wtPath = createAutoWorktree(repo, "M130");
addSliceToMilestone(repo, wtPath, "M130", "S01", "Already landed", [
{ file: "landed.ts", content: "export const landed = true;\n", message: "add landed feature" },
]);
run("git merge --squash milestone/M130", repo);
run('git commit -m "pre-land milestone work"', repo);
const roadmap = makeRoadmap("M130", "Pre-landed milestone", [
{ id: "S01", title: "Already landed" },
]);
let threw = false;
let errMsg = "";
try {
mergeMilestoneToMain(repo, "M130", roadmap);
} catch (err) {
threw = true;
errMsg = err instanceof Error ? err.message : String(err);
}
assertTrue(!threw, `safe nothing-to-commit should not throw (got: ${errMsg})`);
assertTrue(existsSync(join(repo, "landed.ts")), "landed.ts present on main");
}
} finally {
process.chdir(savedCwd);
for (const d of tempDirs) {