fix: clean up SQUASH_MSG after squash-merge and guard worktree teardown against uncommitted changes (#1868)

Three changes to prevent data loss and persistent doctor errors in the
worktree merge-back lifecycle:

1. After nativeCommit in mergeMilestoneToMain, explicitly delete
   .git/SQUASH_MSG. The native libgit2 path and git commit -F - on
   some versions do not auto-remove it, causing doctor to report
   corrupt_merge_state on every run.

2. Before worktree removal (step 11), check for uncommitted changes
   and force a final auto-commit if dirty. This prevents code files
   written by task agents from being destroyed by git worktree remove.

3. Invalidate the nativeHasChanges 10-second cache before the
   post-unit auto-commit in auto-post-unit.ts. A stale false result
   causes autoCommit to skip staging entirely.

Fixes #1853

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
Tom Boucher 2026-03-21 17:24:53 -04:00 committed by GitHub
parent 99032444eb
commit 747e29b9b4
3 changed files with 95 additions and 11 deletions

View file

@ -59,6 +59,7 @@ import { existsSync, unlinkSync } from "node:fs";
import { join } from "node:path";
import { uncheckTaskInPlan } from "./undo.js";
import { atomicWriteSync } from "./atomic-write.js";
import { _resetHasChangesCache } from "./native-git-bridge.js";
/** Throttle STATE.md rebuilds — at most once per 30 seconds */
const STATE_REBUILD_MIN_INTERVAL_MS = 30_000;
@ -156,6 +157,13 @@ export async function postUnitPreVerification(pctx: PostUnitContext, opts?: PreV
}
}
// Invalidate the nativeHasChanges cache before auto-commit (#1853).
// The cache has a 10-second TTL and is keyed by basePath. A stale
// `false` result causes autoCommit to skip staging entirely, leaving
// code files only in the working tree where they are destroyed by
// `git worktree remove --force` during teardown.
_resetHasChangesCache();
const commitMsg = autoCommitCurrentBranch(s.basePath, s.currentUnit.type, s.currentUnit.id, taskContext);
if (commitMsg) {
ctx.ui.notify(`Committed: ${commitMsg.split("\n")[0]}`, "info");

View file

@ -31,6 +31,7 @@ import { gsdRoot } from "./paths.js";
import {
createWorktree,
removeWorktree,
resolveGitDir,
worktreePath,
} from "./worktree-manager.js";
import {
@ -1142,6 +1143,16 @@ export function mergeMilestoneToMain(
const commitResult = nativeCommit(originalBasePath_, commitMessage);
const nothingToCommit = commitResult === null;
// 8a. 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
// reports `corrupt_merge_state` on every subsequent run.
try {
const squashMsgPath = join(resolveGitDir(originalBasePath_), "SQUASH_MSG");
if (existsSync(squashMsgPath)) unlinkSync(squashMsgPath);
} catch { /* best-effort */ }
// 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
@ -1217,6 +1228,30 @@ export function mergeMilestoneToMain(
// 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
// 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`.
if (existsSync(worktreeCwd)) {
try {
const dirtyCheck = nativeWorkingTreeStatus(worktreeCwd);
if (dirtyCheck) {
debugLog("mergeMilestoneToMain", {
phase: "pre-teardown-dirty",
worktreeCwd,
status: dirtyCheck.slice(0, 200),
});
nativeAddAllWithExclusions(worktreeCwd, RUNTIME_EXCLUSION_PATHS);
nativeCommit(worktreeCwd, "chore: pre-teardown auto-commit of uncommitted worktree changes");
}
} catch (e) {
debugLog("mergeMilestoneToMain", {
phase: "pre-teardown-commit-error",
error: String(e),
});
}
}
// 11. Remove worktree directory first (must happen before branch deletion)
try {
removeWorktree(originalBasePath_, milestoneId, {

View file

@ -639,21 +639,17 @@ async function main(): Promise<void> {
{ file: "base.ts", content: "export const base = true;\n", message: "add base" },
]);
// Detach HEAD, then reset branch ref forward independently to create
// divergence (branch ref is NOT an ancestor of worktree HEAD).
run("git checkout --detach HEAD", wtPath);
writeFileSync(join(wtPath, "detached-work.ts"), "export const detached = true;\n");
run("git add .", wtPath);
run('git commit -m "detached work"', wtPath);
// Now advance the branch ref on a different path (via the main repo)
run("git checkout milestone/M150", repo);
writeFileSync(join(repo, "diverged-work.ts"), "export const diverged = true;\n");
run("git add .", repo);
run('git commit -m "diverged work on branch"', repo);
run("git checkout main", repo);
// Move back to worktree cwd
process.chdir(wtPath);
const roadmap = makeRoadmap("M150", "Diverged milestone", [
@ -669,16 +665,61 @@ async function main(): Promise<void> {
errMsg = err instanceof Error ? err.message : String(err);
}
assertTrue(threw, "throws when worktree HEAD diverged from branch ref (#1846)");
assertTrue(
errMsg.includes("diverged"),
"error message mentions divergence (#1846)",
);
assertTrue(errMsg.includes("diverged"), "error message mentions divergence (#1846)");
// Branch must be preserved — no data loss
const branches = run("git branch", repo);
assertTrue(branches.includes("milestone/M150"), "milestone branch preserved on divergence (#1846)");
}
// ─── Test 16: #1853 Bug 1 — SQUASH_MSG cleaned up after squash-merge ──
console.log("\n=== #1853 bug 1: SQUASH_MSG cleaned up after successful squash-merge ===");
{
const repo = freshRepo();
const wtPath = createAutoWorktree(repo, "M160");
addSliceToMilestone(repo, wtPath, "M160", "S01", "SQUASH_MSG cleanup test", [
{ file: "squash-cleanup.ts", content: "export const cleanup = true;\n", message: "add squash-cleanup" },
]);
const roadmap = makeRoadmap("M160", "SQUASH_MSG cleanup", [
{ id: "S01", title: "SQUASH_MSG cleanup test" },
]);
const squashMsgPath = join(repo, ".git", "SQUASH_MSG");
writeFileSync(squashMsgPath, "leftover squash message\n");
assertTrue(existsSync(squashMsgPath), "SQUASH_MSG planted before merge");
const result = mergeMilestoneToMain(repo, "M160", roadmap);
assertTrue(result.commitMessage.includes("feat(M160)"), "merge commit created");
assertTrue(
branches.includes("milestone/M150"),
"milestone branch preserved on divergence (#1846)",
!existsSync(squashMsgPath),
"#1853: SQUASH_MSG must not persist after successful squash-merge",
);
}
// ─── Test 17: #1853 Bug 2 — uncommitted worktree code survives teardown ──
console.log("\n=== #1853 bug 2: uncommitted worktree changes committed before teardown ===");
{
const repo = freshRepo();
const wtPath = createAutoWorktree(repo, "M170");
addSliceToMilestone(repo, wtPath, "M170", "S01", "Teardown safety test", [
{ file: "safe-file.ts", content: "export const safe = true;\n", message: "add safe file" },
]);
writeFileSync(join(wtPath, "uncommitted-agent-code.ts"), "export const lost = true;\n");
const roadmap = makeRoadmap("M170", "Teardown safety", [
{ id: "S01", title: "Teardown safety test" },
]);
const result = mergeMilestoneToMain(repo, "M170", roadmap);
assertTrue(result.commitMessage.includes("feat(M170)"), "merge commit created");
assertTrue(
existsSync(join(repo, "uncommitted-agent-code.ts")),
"#1853: uncommitted worktree code must survive teardown",
);
}