fix(gsd): remove force-staging of .gsd/milestones/ through symlinks (#2247) (#2249)

smartStage() was using git hash-object + update-index to bypass .gitignore
and force-stage .gsd/milestones/ files when .gsd is a symlink. This
contradicts the external state design (symlink = state lives outside repo)
and the documented deprecation of commit_docs.

Remove the force-add block, finish the commit_docs deprecation in
auto-prompts (always emit "do not commit"), and clean up the commitDocs
parameter from all call sites. The deprecation warning in
preferences-validation remains so users are told to remove the setting.

Closes #2247

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
TÂCHES 2026-03-23 11:00:02 -06:00 committed by GitHub
parent e7e22d5eca
commit 61c9e62d37
10 changed files with 23 additions and 115 deletions

View file

@ -975,11 +975,7 @@ export async function buildPlanSlicePrompt(
const executorContextConstraints = formatExecutorConstraints();
const outputRelPath = relSliceFile(base, mid, sid, "PLAN");
const prefs = loadEffectiveGSDPreferences();
const commitDocsEnabled = prefs?.preferences?.git?.commit_docs !== false;
const commitInstruction = commitDocsEnabled
? `Commit the plan files only: \`git add --force ${relSlicePath(base, mid, sid)}/ .gsd/DECISIONS.md .gitignore && git commit -m "docs(${sid}): add slice plan"\`. Do not stage .gsd/STATE.md or other runtime files — the system manages those.`
: "Do not commit — planning docs are not tracked in git for this project.";
const commitInstruction = "Do not commit — .gsd/ planning docs are managed externally and not tracked in git.";
return loadPrompt("plan-slice", {
workingDirectory: base,
milestoneId: mid, sliceId: sid, sliceTitle: sTitle,
@ -1477,11 +1473,7 @@ export async function buildReassessRoadmapPrompt(
// Non-fatal — captures module may not be available
}
const reassessPrefs = loadEffectiveGSDPreferences();
const reassessCommitDocsEnabled = reassessPrefs?.preferences?.git?.commit_docs !== false;
const reassessCommitInstruction = reassessCommitDocsEnabled
? `Commit: \`docs(${mid}): reassess roadmap after ${completedSliceId}\`. Stage only the .gsd/milestones/ files you changed — do not stage .gsd/STATE.md or other runtime files.`
: "Do not commit — planning docs are not tracked in git for this project.";
const reassessCommitInstruction = "Do not commit — .gsd/ planning docs are managed externally and not tracked in git.";
return loadPrompt("reassess-roadmap", {
workingDirectory: base,

View file

@ -167,22 +167,19 @@ export async function bootstrapAutoSession(
// ensureGitignore checks for git-tracked .gsd/ files and skips the
// ".gsd" pattern if the project intentionally tracks .gsd/ in git.
const gitPrefs = loadEffectiveGSDPreferences()?.preferences?.git;
const commitDocs = gitPrefs?.commit_docs;
const manageGitignore = gitPrefs?.manage_gitignore;
ensureGitignore(base, { commitDocs, manageGitignore });
ensureGitignore(base, { manageGitignore });
if (manageGitignore !== false) untrackRuntimeFiles(base);
// Bootstrap .gsd/ if it doesn't exist
const gsdDir = join(base, ".gsd");
if (!existsSync(gsdDir)) {
mkdirSync(join(gsdDir, "milestones"), { recursive: true });
if (commitDocs !== false) {
try {
nativeAddAll(base);
nativeCommit(base, "chore: init gsd");
} catch {
/* nothing to commit */
}
try {
nativeAddAll(base);
nativeCommit(base, "chore: init gsd");
} catch {
/* nothing to commit */
}
}
@ -487,7 +484,7 @@ export async function bootstrapAutoSession(
// Capture integration branch
if (s.currentMilestoneId) {
if (getIsolationMode() !== "none") {
captureIntegrationBranch(base, s.currentMilestoneId, { commitDocs });
captureIntegrationBranch(base, s.currentMilestoneId);
}
setActiveMilestoneId(base, s.currentMilestoneId);
}

View file

@ -109,7 +109,6 @@ export interface LoopDeps {
captureIntegrationBranch: (
basePath: string,
mid: string,
opts?: { commitDocs?: boolean },
) => void;
getIsolationMode: () => string;
getCurrentBranch: (basePath: string) => string;

View file

@ -261,9 +261,7 @@ export async function runPreDispatch(
if (mid) {
if (deps.getIsolationMode() !== "none") {
deps.captureIntegrationBranch(s.basePath, mid, {
commitDocs: prefs?.git?.commit_docs,
});
deps.captureIntegrationBranch(s.basePath, mid);
}
deps.resolver.enterMilestone(mid, ctx.ui);
} else {

View file

@ -9,8 +9,8 @@
*/
import { execFileSync, execSync } from "node:child_process";
import { existsSync, lstatSync, mkdirSync, readFileSync, readdirSync, writeFileSync } from "node:fs";
import { join, relative } from "node:path";
import { existsSync, mkdirSync, readFileSync, writeFileSync } from "node:fs";
import { join } from "node:path";
import { gsdRoot } from "./paths.js";
import { GIT_NO_PROMPT_ENV } from "./git-constants.js";
import { loadEffectiveGSDPreferences } from "./preferences.js";
@ -245,7 +245,6 @@ export function writeIntegrationBranch(
basePath: string,
milestoneId: string,
branch: string,
_options?: { commitDocs?: boolean },
): void {
// Don't record slice branches as the integration target
if (SLICE_BRANCH_RE.test(branch)) return;
@ -486,80 +485,11 @@ export class GitServiceImpl {
// git add -A already skips it and the exclusions are harmless no-ops.
const allExclusions = [...RUNTIME_EXCLUSION_PATHS, ...extraExclusions];
nativeAddAllWithExclusions(this.basePath, allExclusions);
// Force-add .gsd/milestones/ when .gsd is a symlink (#2104).
// When .gsd is a symlink (external state projects), ensureGitignore adds
// `.gsd` to .gitignore. The nativeAddAllWithExclusions call above falls
// back to plain `git add -A` (symlink pathspec rejection), which respects
// .gitignore and silently skips new .gsd/milestones/ files.
//
// `git add -f` also fails with "beyond a symbolic link", so we use
// `git hash-object -w` + `git update-index --add --cacheinfo` to bypass
// the symlink restriction entirely. This stages each milestone artifact
// individually by hashing the file content and updating the index directly.
const gsdPath = join(this.basePath, ".gsd");
const milestonesDir = join(gsdPath, "milestones");
try {
if (
existsSync(gsdPath) &&
lstatSync(gsdPath).isSymbolicLink() &&
existsSync(milestonesDir)
) {
this._forceAddMilestoneArtifacts(milestonesDir);
}
} catch {
// Non-fatal: if force-add fails, the commit proceeds without these files.
// This matches existing behavior where milestone artifacts were silently
// omitted — but now we at least attempt to include them.
}
}
/** Tracks whether runtime file cleanup has run this session. */
private _runtimeFilesCleanedUp = false;
/**
* Recursively collect all files under a directory.
* Returns paths relative to `basePath` (e.g. ".gsd/milestones/M009/SUMMARY.md").
*/
private _collectFiles(dir: string): string[] {
const files: string[] = [];
for (const entry of readdirSync(dir, { withFileTypes: true })) {
const full = join(dir, entry.name);
if (entry.isDirectory()) {
files.push(...this._collectFiles(full));
} else if (entry.isFile()) {
files.push(relative(this.basePath, full));
}
}
return files;
}
/**
* Stage milestone artifacts through a symlinked .gsd directory (#2104).
*
* `git add` (even with `-f`) refuses to stage files "beyond a symbolic link".
* This method bypasses that restriction by hashing each file with
* `git hash-object -w` and inserting the blob into the index with
* `git update-index --add --cacheinfo 100644 <hash> <path>`.
*/
private _forceAddMilestoneArtifacts(milestonesDir: string): void {
const files = this._collectFiles(milestonesDir);
for (const filePath of files) {
const hash = execFileSync("git", ["hash-object", "-w", filePath], {
cwd: this.basePath,
stdio: ["ignore", "pipe", "pipe"],
encoding: "utf-8",
env: GIT_NO_PROMPT_ENV,
}).trim();
execFileSync("git", ["update-index", "--add", "--cacheinfo", "100644", hash, filePath], {
cwd: this.basePath,
stdio: ["ignore", "pipe", "pipe"],
encoding: "utf-8",
env: GIT_NO_PROMPT_ENV,
});
}
}
/**
* Stage files (smart staging) and commit.
* Returns the commit message string on success, or null if nothing to commit.

View file

@ -137,7 +137,7 @@ export function hasGitTrackedGsdFiles(basePath: string): boolean {
*/
export function ensureGitignore(
basePath: string,
options?: { manageGitignore?: boolean; commitDocs?: boolean },
options?: { manageGitignore?: boolean },
): boolean {
// If manage_gitignore is explicitly false, do not touch .gitignore at all
if (options?.manageGitignore === false) return false;

View file

@ -1411,16 +1411,14 @@ async function main(): Promise<void> {
rmSync(repo, { recursive: true, force: true });
}
// ─── autoCommit: symlinked .gsd stages new milestone artifacts (#2104) ──
// ─── autoCommit: symlinked .gsd does NOT stage milestone artifacts (#2247) ──
console.log("\n=== autoCommit: symlinked .gsd stages new milestone artifacts (#2104) ===");
console.log("\n=== autoCommit: symlinked .gsd does NOT stage milestone artifacts (#2247) ===");
{
// Reproduction: when .gsd is a symlink (external state project),
// autoCommit silently fails to stage NEW .gsd/milestones/ files because:
// 1. nativeAddAllWithExclusions falls back to plain `git add -A` (symlink)
// 2. `.gsd` is in .gitignore → new .gsd/ files are invisible to `git add`
// The fix: smartStage() force-adds .gsd/milestones/ after the normal staging.
// When .gsd is a symlink (external state project), .gsd/ files live outside
// the repo by design. smartStage() must NOT force-stage them into git — the
// .gitignore exclusion is correct and intentional.
const repo = initTempRepo();
// Create an external .gsd directory and symlink it into the repo
@ -1449,12 +1447,8 @@ async function main(): Promise<void> {
const committed = run("git show --name-only HEAD", repo);
assertTrue(committed.includes("src/feature.ts"), "symlink autoCommit: source file committed");
assertTrue(committed.includes(".gsd/milestones/M009/M009-SUMMARY.md"),
"symlink autoCommit: new M009-SUMMARY.md is committed (not silently dropped)");
assertTrue(committed.includes(".gsd/milestones/M009/S01-SUMMARY.md"),
"symlink autoCommit: new S01-SUMMARY.md is committed");
assertTrue(committed.includes(".gsd/milestones/M009/T01-VERIFY.json"),
"symlink autoCommit: new T01-VERIFY.json is committed");
assertTrue(!committed.includes(".gsd/milestones/"),
"symlink autoCommit: .gsd/milestones/ files are NOT staged (external state stays external)");
try { rmSync(repo, { recursive: true, force: true }); } catch {}
try { rmSync(externalGsd, { recursive: true, force: true }); } catch {}

View file

@ -139,11 +139,10 @@ function makeDeps(
captureIntegrationBranch: (
basePath: string,
mid: string | undefined,
opts?: { commitDocs?: boolean },
) => {
calls.push({
fn: "captureIntegrationBranch",
args: [basePath, mid, opts],
args: [basePath, mid],
});
},
...overrides,

View file

@ -63,7 +63,6 @@ export interface WorktreeResolverDeps {
captureIntegrationBranch: (
basePath: string,
mid: string,
opts?: { commitDocs?: boolean },
) => void;
}

View file

@ -57,13 +57,13 @@ export function setActiveMilestoneId(basePath: string, milestoneId: string | nul
* record when the user starts from a different branch (#300). Always a no-op
* if on a GSD slice branch.
*/
export function captureIntegrationBranch(basePath: string, milestoneId: string, options?: { commitDocs?: boolean }): void {
export function captureIntegrationBranch(basePath: string, milestoneId: string): void {
// In a worktree, the base branch is implicit (worktree/<name>).
// Writing it to META.json would leave stale metadata after merge back to main.
if (detectWorktreeName(basePath)) return;
const svc = getService(basePath);
const current = svc.getCurrentBranch();
writeIntegrationBranch(basePath, milestoneId, current, options);
writeIntegrationBranch(basePath, milestoneId, current);
}
// ─── Pure Utility Functions (unchanged) ────────────────────────────────────