Merge pull request #536 from fluxlabs/fix/slice-merge-loop-530

fix: prevent merge loop, auto-resolve .gsd/ conflicts, restore git.isolation (#530, #531)
This commit is contained in:
TÂCHES 2026-03-15 17:18:15 -06:00 committed by GitHub
commit 4eb5f431d4
9 changed files with 250 additions and 74 deletions

View file

@ -333,17 +333,59 @@ export function reconcileMergeState(basePath: string, ctx: ExtensionContext): bo
// Commit may already exist; non-fatal
}
} else {
// Still conflicted — abort and reset
if (hasMergeHead) {
runGit(basePath, ["merge", "--abort"], { allowFailure: true });
} else if (hasSquashMsg) {
try { unlinkSync(squashMsgPath); } catch { /* best-effort */ }
// Still conflicted — try auto-resolving .gsd/ state file conflicts (#530)
const conflictedFiles = unmerged.trim().split("\n").filter(Boolean);
const gsdConflicts = conflictedFiles.filter(f => f.startsWith(".gsd/"));
const codeConflicts = conflictedFiles.filter(f => !f.startsWith(".gsd/"));
if (gsdConflicts.length > 0 && codeConflicts.length === 0) {
// All conflicts are in .gsd/ state files — auto-resolve by accepting theirs
let resolved = true;
for (const gsdFile of gsdConflicts) {
try {
runGit(basePath, ["checkout", "--theirs", "--", gsdFile], { allowFailure: false });
runGit(basePath, ["add", "--", gsdFile], { allowFailure: false });
} catch {
resolved = false;
break;
}
}
if (resolved) {
try {
runGit(basePath, ["commit", "--no-edit"], { allowFailure: false });
ctx.ui.notify(
`Auto-resolved ${gsdConflicts.length} .gsd/ state file conflict(s) from prior merge.`,
"info",
);
} catch {
resolved = false;
}
}
if (!resolved) {
if (hasMergeHead) {
runGit(basePath, ["merge", "--abort"], { allowFailure: true });
} else if (hasSquashMsg) {
try { unlinkSync(squashMsgPath); } catch { /* best-effort */ }
}
runGit(basePath, ["reset", "--hard", "HEAD"], { allowFailure: true });
ctx.ui.notify(
"Detected leftover merge state — auto-resolve failed, cleaned up. Re-deriving state.",
"warning",
);
}
} else {
// Code conflicts present — abort and reset
if (hasMergeHead) {
runGit(basePath, ["merge", "--abort"], { allowFailure: true });
} else if (hasSquashMsg) {
try { unlinkSync(squashMsgPath); } catch { /* best-effort */ }
}
runGit(basePath, ["reset", "--hard", "HEAD"], { allowFailure: true });
ctx.ui.notify(
"Detected leftover merge state with unresolved conflicts — cleaned up. Re-deriving state.",
"warning",
);
}
runGit(basePath, ["reset", "--hard", "HEAD"], { allowFailure: true });
ctx.ui.notify(
"Detected leftover merge state with unresolved conflicts — cleaned up. Re-deriving state.",
"warning",
);
}
return true;
}

View file

@ -307,7 +307,7 @@ export function mergeMilestoneToMain(
}
const commitMessage = subject + body;
// 7. Squash merge
// 7. Squash merge — auto-resolve .gsd/ state file conflicts (#530)
try {
execSync(`git merge --squash ${milestoneBranch}`, {
cwd: originalBasePath_,
@ -315,7 +315,7 @@ export function mergeMilestoneToMain(
encoding: "utf-8",
});
} catch (mergeErr) {
// Check for real conflicts
// Check for conflicts — auto-resolve .gsd/ state files, escalate the rest
try {
const conflictOutput = execSync("git diff --name-only --diff-filter=U", {
cwd: originalBasePath_,
@ -324,7 +324,44 @@ export function mergeMilestoneToMain(
}).trim();
if (conflictOutput) {
const conflictedFiles = conflictOutput.split("\n").filter(Boolean);
throw new MergeConflictError(conflictedFiles, "squash", milestoneBranch, mainBranch);
// Separate .gsd/ state file conflicts from real code conflicts.
// GSD state files (STATE.md, completed-units.json, auto.lock, etc.)
// diverge between branches during normal operation — always prefer the
// milestone branch version since it has the latest execution state.
const gsdConflicts = conflictedFiles.filter(f => f.startsWith(".gsd/"));
const codeConflicts = conflictedFiles.filter(f => !f.startsWith(".gsd/"));
// Auto-resolve .gsd/ conflicts by accepting the milestone branch version
if (gsdConflicts.length > 0) {
for (const gsdFile of gsdConflicts) {
try {
execFileSync("git", ["checkout", "--theirs", "--", gsdFile], {
cwd: originalBasePath_,
stdio: ["ignore", "pipe", "pipe"],
encoding: "utf-8",
});
execFileSync("git", ["add", "--", gsdFile], {
cwd: originalBasePath_,
stdio: ["ignore", "pipe", "pipe"],
encoding: "utf-8",
});
} catch {
// If checkout --theirs fails, try removing the file from the merge
// (it's a runtime file that shouldn't be committed anyway)
execFileSync("git", ["rm", "--force", "--", gsdFile], {
cwd: originalBasePath_,
stdio: ["ignore", "pipe", "pipe"],
encoding: "utf-8",
});
}
}
}
// If there are still non-.gsd conflicts, escalate
if (codeConflicts.length > 0) {
throw new MergeConflictError(codeConflicts, "squash", milestoneBranch, mainBranch);
}
}
} catch (diffErr) {
if (diffErr instanceof MergeConflictError) throw diffErr;

View file

@ -156,6 +156,18 @@ const unitRecoveryCount = new Map<string, number>();
/** Persisted completed-unit keys — survives restarts. Loaded from .gsd/completed-units.json. */
const completedKeySet = new Set<string>();
/**
* Resolve whether auto-mode should use worktree isolation.
* Returns true for worktree mode (default), false for branch mode.
* Branch mode works directly in the project root useful for repos
* with git submodules where worktrees don't work well (#531).
*/
function shouldUseWorktreeIsolation(): boolean {
const prefs = loadEffectiveGSDPreferences()?.preferences?.git;
if (prefs?.isolation === "branch") return false;
return true; // default: worktree
}
/** Crash recovery prompt — set by startAuto, consumed by first dispatchNextUnit */
let pendingCrashRecovery: string | null = null;
@ -463,7 +475,8 @@ export async function startAuto(
// ── Auto-worktree: re-enter worktree on resume if not already inside ──
// Skip if already inside a worktree (manual /worktree) to prevent nesting.
if (currentMilestoneId && originalBasePath && !isInAutoWorktree(basePath) && !detectWorktreeName(basePath) && !detectWorktreeName(originalBasePath)) {
// Skip entirely in branch isolation mode (#531).
if (currentMilestoneId && shouldUseWorktreeIsolation() && originalBasePath && !isInAutoWorktree(basePath) && !detectWorktreeName(basePath) && !detectWorktreeName(originalBasePath)) {
try {
const existingWtPath = getAutoWorktreePath(originalBasePath, currentMilestoneId);
if (existingWtPath) {
@ -640,7 +653,7 @@ export async function startAuto(
return p.endsWith(worktreesSuffix);
};
if (currentMilestoneId && !detectWorktreeName(base) && !isUnderGsdWorktrees(base)) {
if (currentMilestoneId && shouldUseWorktreeIsolation() && !detectWorktreeName(base) && !isUnderGsdWorktrees(base)) {
try {
const existingWtPath = getAutoWorktreePath(base, currentMilestoneId);
if (existingWtPath) {
@ -1680,9 +1693,9 @@ async function dispatchNextUnit(
return;
}
// NOTE: Slice merge happens AFTER the complete-slice unit finishes,
// not here at dispatch time. See the merge logic at the top of
// dispatchNextUnit where we check if the previous unit was complete-slice.
// Branchless architecture: all work commits sequentially on the milestone
// branch — no per-slice branches or slice-level merges. Milestone merge
// happens when phase === "complete" (see mergeMilestoneToMain above).
// Write lock AFTER newSession so we capture the session file path.
// Pi appends entries incrementally via appendFileSync, so on crash the

View file

@ -1,6 +1,9 @@
import { execSync } from "node:child_process";
// GSD Dispatch Guard — prevents out-of-order slice dispatch
// Copyright (c) 2026 Jeremy McSpadden <jeremy@fluxlabs.net>
import { readFileSync } from "node:fs";
import { readdirSync } from "node:fs";
import { relMilestoneFile, milestonesDir } from "./paths.js";
import { resolveMilestoneFile, milestonesDir } from "./paths.js";
import { parseRoadmapSlices } from "./roadmap-slices.js";
import { extractMilestoneSeq, milestoneIdSort } from "./guided-flow.js";
@ -12,19 +15,29 @@ const SLICE_DISPATCH_TYPES = new Set([
"complete-slice",
]);
function readTrackedFileFromBranch(base: string, branch: string, relPath: string): string | null {
/**
* Read a roadmap file from disk (working tree) rather than from a git branch.
*
* Prior implementation used `git show <branch>:<path>` which read committed
* state on a specific branch. This caused false-positive blockers when work
* was committed on a milestone/worktree branch but the integration branch
* (main) hadn't been updated yet the guard would see prior slices as
* incomplete on main even though they were done in the working tree (#530).
*
* Reading from disk always reflects the latest state, regardless of which
* branch is checked out or whether changes have been committed.
*/
function readRoadmapFromDisk(base: string, milestoneId: string): string | null {
try {
return execSync(`git show ${branch}:${relPath}`, {
cwd: base,
stdio: ["ignore", "pipe", "pipe"],
encoding: "utf-8",
}).trim();
const absPath = resolveMilestoneFile(base, milestoneId, "ROADMAP");
if (!absPath) return null;
return readFileSync(absPath, "utf-8").trim();
} catch {
return null;
}
}
export function getPriorSliceCompletionBlocker(base: string, mainBranch: string, unitType: string, unitId: string): string | null {
export function getPriorSliceCompletionBlocker(base: string, _mainBranch: string, unitType: string, unitId: string): string | null {
if (!SLICE_DISPATCH_TYPES.has(unitType)) return null;
const [targetMid, targetSid] = unitId.split("/");
@ -50,17 +63,15 @@ export function getPriorSliceCompletionBlocker(base: string, mainBranch: string,
}
for (const mid of milestoneIds) {
const roadmapRel = relMilestoneFile(base, mid, "ROADMAP");
if (!roadmapRel) continue;
const roadmapContent = readTrackedFileFromBranch(base, mainBranch, roadmapRel);
// Read from disk (working tree) — always has the latest state
const roadmapContent = readRoadmapFromDisk(base, mid);
if (!roadmapContent) continue;
const slices = parseRoadmapSlices(roadmapContent);
if (mid !== targetMid) {
const incomplete = slices.find(slice => !slice.done);
if (incomplete) {
return `Cannot dispatch ${unitType} ${unitId}: earlier slice ${mid}/${incomplete.id} is not complete on ${mainBranch}.`;
return `Cannot dispatch ${unitType} ${unitId}: earlier slice ${mid}/${incomplete.id} is not complete.`;
}
continue;
}
@ -70,7 +81,7 @@ export function getPriorSliceCompletionBlocker(base: string, mainBranch: string,
const incomplete = slices.slice(0, targetIndex).find(slice => !slice.done);
if (incomplete) {
return `Cannot dispatch ${unitType} ${unitId}: earlier slice ${targetMid}/${incomplete.id} is not complete on ${mainBranch}.`;
return `Cannot dispatch ${unitType} ${unitId}: earlier slice ${targetMid}/${incomplete.id} is not complete.`;
}
}

View file

@ -35,6 +35,11 @@ export interface GitPreferences {
commit_type?: string;
main_branch?: string;
merge_strategy?: "squash" | "merge";
/** Controls auto-mode git isolation strategy.
* - "worktree": (default) creates a milestone worktree for isolated work
* - "branch": works directly in the project root (for submodule-heavy repos)
*/
isolation?: "worktree" | "branch";
}
export const VALID_BRANCH_NAME = /^[a-zA-Z0-9_\-\/.]+$/;

View file

@ -1022,11 +1022,15 @@ export function validatePreferences(preferences: GSDPreferences): {
errors.push("git.main_branch must be a valid branch name (alphanumeric, _, -, /, .)");
}
}
// Deprecated: isolation and merge_to_main are ignored (branchless architecture).
// Emit warnings so users know to remove them from preferences.
if (g.isolation !== undefined) {
warnings.push("git.isolation is deprecated — worktree isolation is now always enabled. Remove this setting.");
const validIsolation = new Set(["worktree", "branch"]);
if (typeof g.isolation === "string" && validIsolation.has(g.isolation)) {
git.isolation = g.isolation as "worktree" | "branch";
} else {
errors.push("git.isolation must be one of: worktree, branch");
}
}
// Deprecated: merge_to_main is ignored (branchless architecture).
if (g.merge_to_main !== undefined) {
warnings.push("git.merge_to_main is deprecated — milestone-level merge is now always used. Remove this setting.");
}

View file

@ -247,6 +247,49 @@ async function main(): Promise<void> {
assertEq(result.pushed, false, "pushed is false without discoverable prefs");
}
// ─── Test 5: Auto-resolve .gsd/ state file conflicts (#530) ───────
console.log("\n=== auto-resolve .gsd/ state file conflicts ===");
{
const repo = freshRepo();
const wtPath = createAutoWorktree(repo, "M050");
// Add a slice with real work
addSliceToMilestone(repo, wtPath, "M050", "S01", "Conflict test", [
{ file: "feature.ts", content: "export const feature = true;\n", message: "add feature" },
]);
// Modify .gsd/STATE.md on the milestone branch (simulates auto-mode state updates)
writeFileSync(join(wtPath, ".gsd", "STATE.md"), "# State\n\n## Updated on milestone branch\n");
run("git add .", wtPath);
run('git commit -m "chore: update state on milestone branch"', wtPath);
// Now modify .gsd/STATE.md on main too (simulates divergence)
run("git checkout main", repo);
writeFileSync(join(repo, ".gsd", "STATE.md"), "# State\n\n## Updated on main\n");
run("git add .", repo);
run('git commit -m "chore: update state on main"', repo);
// Go back to worktree for the merge
process.chdir(wtPath);
const roadmap = makeRoadmap("M050", "Conflict resolution", [
{ id: "S01", title: "Conflict test" },
]);
// Merge should succeed despite .gsd/STATE.md conflict — auto-resolved
let threw = false;
try {
const result = mergeMilestoneToMain(repo, "M050", roadmap);
assertTrue(result.commitMessage.includes("feat(M050)"), "merge commit created despite .gsd conflict");
} catch (err) {
threw = true;
}
assertTrue(!threw, "auto-resolves .gsd/ state file conflicts without throwing");
// Feature file should be on main
assertTrue(existsSync(join(repo, "feature.ts")), "feature.ts merged to main");
}
} finally {
process.chdir(savedCwd);
for (const d of tempDirs) {

View file

@ -1,14 +1,13 @@
// GSD Dispatch Guard Tests
// Copyright (c) 2026 Jeremy McSpadden <jeremy@fluxlabs.net>
import { mkdtempSync, mkdirSync, rmSync, writeFileSync } from "node:fs";
import { execSync } from "node:child_process";
import { join } from "node:path";
import { tmpdir } from "node:os";
import { getPriorSliceCompletionBlocker } from "../dispatch-guard.ts";
import { createTestContext } from './test-helpers.ts';
const { assertEq, report } = createTestContext();
function run(command: string, cwd: string): void {
execSync(command, { cwd, stdio: ["ignore", "pipe", "pipe"] });
}
const { assertEq, assertTrue, report } = createTestContext();
const repo = mkdtempSync(join(tmpdir(), "gsd-dispatch-guard-"));
try {
@ -33,18 +32,14 @@ try {
"",
].join("\n"));
run("git init -b main", repo);
run("git config user.email test@example.com", repo);
run("git config user.name Test", repo);
run("git add .", repo);
run("git commit -m init", repo);
// dispatch-guard now reads from disk, not git — no need for git init/commit
assertEq(
getPriorSliceCompletionBlocker(repo, "main", "plan-slice", "M003/S01"),
"Cannot dispatch plan-slice M003/S01: earlier slice M002/S02 is not complete on main.",
"blocks first slice of next milestone when prior milestone is incomplete on main",
"Cannot dispatch plan-slice M003/S01: earlier slice M002/S02 is not complete.",
"blocks first slice of next milestone when prior milestone is incomplete",
);
// Complete M002 on disk
writeFileSync(join(repo, ".gsd", "milestones", "M002", "M002-ROADMAP.md"), [
"# M002: Previous",
"",
@ -53,15 +48,14 @@ try {
"- [x] **S02: Done** `risk:low` `depends:[S01]`",
"",
].join("\n"));
run("git add .", repo);
run("git commit -m complete-m002", repo);
assertEq(
getPriorSliceCompletionBlocker(repo, "main", "execute-task", "M003/S02/T01"),
"Cannot dispatch execute-task M003/S02/T01: earlier slice M003/S01 is not complete on main.",
"blocks later slice in same milestone when an earlier slice is incomplete on main",
"Cannot dispatch execute-task M003/S02/T01: earlier slice M003/S01 is not complete.",
"blocks later slice in same milestone when an earlier slice is incomplete",
);
// Complete M003/S01 on disk
writeFileSync(join(repo, ".gsd", "milestones", "M003", "M003-ROADMAP.md"), [
"# M003: Current",
"",
@ -70,13 +64,11 @@ try {
"- [ ] **S02: Second** `risk:low` `depends:[S01]`",
"",
].join("\n"));
run("git add .", repo);
run("git commit -m complete-m003-s01", repo);
assertEq(
getPriorSliceCompletionBlocker(repo, "main", "execute-task", "M003/S02/T01"),
null,
"allows dispatch when all earlier slices are complete on main",
"allows dispatch when all earlier slices are complete on disk",
);
assertEq(
@ -84,6 +76,28 @@ try {
null,
"does not affect non-slice dispatch types",
);
// Verify disk-based reads work without any git repo (#530)
const noGitRepo = mkdtempSync(join(tmpdir(), "gsd-dispatch-guard-nogit-"));
try {
mkdirSync(join(noGitRepo, ".gsd", "milestones", "M001"), { recursive: true });
writeFileSync(join(noGitRepo, ".gsd", "milestones", "M001", "M001-ROADMAP.md"), [
"# M001: Test",
"",
"## Slices",
"- [x] **S01: Done** `risk:low` `depends:[]`",
"- [ ] **S02: Pending** `risk:low` `depends:[S01]`",
"",
].join("\n"));
assertEq(
getPriorSliceCompletionBlocker(noGitRepo, "main", "plan-slice", "M001/S02"),
null,
"allows dispatch for S02 when S01 is complete (no git repo needed)",
);
} finally {
rmSync(noGitRepo, { recursive: true, force: true });
}
} finally {
rmSync(repo, { recursive: true, force: true });
}

View file

@ -1,7 +1,5 @@
/**
* preferences-git.test.ts Validates that deprecated git.isolation and
* git.merge_to_main preference fields produce deprecation warnings.
*/
// GSD Git Preferences Tests — validates git.isolation and git.merge_to_main handling
// Copyright (c) 2026 Jeremy McSpadden <jeremy@fluxlabs.net>
import { createTestContext } from "./test-helpers.ts";
import { validatePreferences } from "../preferences.ts";
@ -9,18 +7,27 @@ import { validatePreferences } from "../preferences.ts";
const { assertEq, assertTrue, report } = createTestContext();
async function main(): Promise<void> {
console.log("\n=== git.isolation deprecated ===");
console.log("\n=== git.isolation ===");
// Any value produces a deprecation warning
// Valid values are accepted without warnings
{
const { warnings } = validatePreferences({ git: { isolation: "worktree" } });
assertTrue(warnings.length > 0, "isolation: worktree — produces deprecation warning");
assertTrue(warnings[0].includes("deprecated"), "isolation: worktree — warning mentions deprecated");
const { preferences, warnings, errors } = validatePreferences({ git: { isolation: "worktree" } });
assertEq(errors.length, 0, "isolation: worktree — no errors");
assertEq(warnings.length, 0, "isolation: worktree — no warnings");
assertEq(preferences.git?.isolation, "worktree", "isolation: worktree — value preserved");
}
{
const { warnings } = validatePreferences({ git: { isolation: "branch" } });
assertTrue(warnings.length > 0, "isolation: branch — produces deprecation warning");
assertTrue(warnings[0].includes("deprecated"), "isolation: branch — warning mentions deprecated");
const { preferences, warnings, errors } = validatePreferences({ git: { isolation: "branch" } });
assertEq(errors.length, 0, "isolation: branch — no errors");
assertEq(warnings.length, 0, "isolation: branch — no warnings");
assertEq(preferences.git?.isolation, "branch", "isolation: branch — value preserved");
}
// Invalid values produce errors
{
const { errors } = validatePreferences({ git: { isolation: "invalid" } });
assertTrue(errors.length > 0, "isolation: invalid — produces error");
assertTrue(errors[0].includes("worktree, branch"), "isolation: invalid — error mentions valid values");
}
// Undefined passes through without warning
@ -51,14 +58,14 @@ async function main(): Promise<void> {
assertEq(preferences.git?.merge_to_main, undefined, "merge_to_main: undefined — not set");
}
console.log("\n=== both deprecated fields together ===");
console.log("\n=== isolation + deprecated merge_to_main together ===");
{
const { warnings } = validatePreferences({
git: { isolation: "worktree", merge_to_main: "slice" },
const { warnings, errors } = validatePreferences({
git: { isolation: "branch", merge_to_main: "slice" },
});
assertEq(warnings.length, 2, "both deprecated fields — 2 warnings");
assertTrue(warnings.some(w => w.includes("isolation")), "one warning mentions isolation");
assertTrue(warnings.some(w => w.includes("merge_to_main")), "one warning mentions merge_to_main");
assertEq(errors.length, 0, "branch isolation + deprecated merge_to_main — no errors");
assertEq(warnings.length, 1, "branch isolation + deprecated merge_to_main — 1 warning (merge_to_main only)");
assertTrue(warnings[0].includes("merge_to_main"), "warning mentions merge_to_main");
}
report();