diff --git a/src/health-diagnostic-rules/worktree-health.cts b/src/health-diagnostic-rules/worktree-health.cts index d96e3d66c..4ca349b5a 100644 --- a/src/health-diagnostic-rules/worktree-health.cts +++ b/src/health-diagnostic-rules/worktree-health.cts @@ -13,7 +13,7 @@ * pre-migration source still names the split-off stale-worktree site * 'W017' — this batch is what actually applies the W027 split). * - * KNOWN GAPS (found while building, reported rather than papered over — see + * KNOWN GAP (found while building, reported rather than papered over — see * this batch's dispatch report for full detail): * * 1. W020's original THREE conditions were git_timed_out / git_list_failed / @@ -31,25 +31,10 @@ * with the discarded `reason` field — an snapshot-field enhancement * outside this rule-file batch's scope, flagged here rather than guessed * around. - * 2. W027's original exclusion of the active session's own worktree - * (`verify.cts:2233-2242`, comparing `finding.path` against - * `process.cwd()`) happens at the `cmdValidateHealth` call site, NOT - * inside `inspectWorktreeHealth`/`worktree-safety.cts`. Confirmed by - * direct read: `inspectWorktreeHealth` (`src/worktree-safety.cts:352-397`) - * performs no cwd comparison, and `listLinkedWorktreePaths` - * (`src/worktree-safety.cts:321-338`) only drops the FIRST `git worktree - * list` entry (assumed main worktree) via `.slice(1)` — it does not know - * which entry, if any, is the ACTIVE session's cwd, which is commonly a - * LINKED (non-first) worktree in this repo's own multi-worktree workflow. - * A `Rule.check(snapshot)` has no ambient `process.cwd()` access (§8.1 - * rule 1 forbids it), and `PlanningSnapshot` carries no - * "active worktree path" field to filter against. This is a REAL, - * unclosed gap: `checkW027` below reports every 'stale' finding, - * INCLUDING the active session's own worktree, which is a behavior - * change from the pre-migration code. Closing it precisely requires - * either a new snapshot field carrying the active worktree path/cwd, or - * moving the exclusion into `inspectWorktreeHealth` itself — both are - * snapshot/owner changes outside this rule-file batch's scope. + * + * W027 restores the pre-migration active-worktree exclusion + * (`verify.cts:2233-2242`) via `PlanningSnapshot.cwd` — see `checkW027`'s own + * comment below for the mechanism. * * Design: .gsd/phase/refactor-3309-health-diagnostic-rule-table/40-design.md * @@ -58,6 +43,8 @@ * gsd-core/bin/lib/health-diagnostic-rules/worktree-health.cjs (gitignored). */ +import path from 'node:path'; + // eslint-disable-next-line @typescript-eslint/no-require-imports -- type-only; erased at compile time, no runtime require emitted import type planningSnapshotMod = require('../planning-snapshot.cjs'); @@ -65,7 +52,7 @@ type PlanningSnapshot = ReturnType` -// template, mirroring the split the brief specifies. +// `finding.kind === 'stale'` — age-based. Excludes the active session's own +// worktree, restored via `snapshot.cwd` (see module doc, gap 2 — RESOLVED): +// a 'stale' finding is skipped when `snapshot.cwd` equals the finding's path +// or is nested under it, the exact comparison `verify.cts:2238-2241` made +// against `process.cwd()`. Per this batch's brief: the interpolated command +// (with the real path) lives in `message`; `remedy.args.command` stays a +// static `` template, mirroring the split the brief specifies. function checkW027(snapshot: PlanningSnapshot): Diagnostic[] { const diagnostics: Diagnostic[] = []; + const activeCwd = snapshot.cwd; for (const finding of snapshot.worktreeHealth.value) { if (finding.kind !== 'stale') continue; + const normalizedWorktree = path.resolve(finding.path); + const isActiveWorktree = + activeCwd === normalizedWorktree || activeCwd.startsWith(normalizedWorktree + path.sep); + if (isActiveWorktree) continue; diagnostics.push({ code: 'W027', severity: SEVERITY.WARNING, message: `Stale git worktree: ${finding.path} (last modified ${finding.ageMinutes} minutes ago). Run: git worktree remove ${finding.path} --force`, - remedy: { - action: REMEDY_ACTION.ADVISE, - risk: REMEDY_RISK.NONE, - args: { command: 'git worktree remove --force' }, - }, + remedy: adviseRemedy('git worktree remove --force'), }); } return diagnostics; diff --git a/src/planning-snapshot.cts b/src/planning-snapshot.cts index 30219461a..b8be41e8d 100644 --- a/src/planning-snapshot.cts +++ b/src/planning-snapshot.cts @@ -97,6 +97,13 @@ interface PhaseSnapshot { } interface PlanningSnapshot { + // The resolved absolute `cwd` this snapshot was built for — `cwd` is + // already `buildPlanningSnapshot`'s own input, not a new ambient read, so + // exposing it is a "parsed value" per §8.1 rule 2, not §8.1 rule 1 ambient + // I/O. Backs W027's active-worktree exclusion + // (`src/health-diagnostic-rules/worktree-health.cts`), the one pre-migration + // behavior (`verify.cts:2233-2242`) that genuinely needed the caller's cwd. + cwd: string; milestone: ReturnType; phaseDirs: ReturnType; phases: { value: PhaseSnapshot[]; scope: Scope }; @@ -668,6 +675,7 @@ function buildPlanningSnapshot(cwd: string): PlanningSnapshot { const stateFields = buildStateFields(paths.state); return { + cwd: path.resolve(cwd), milestone, phaseDirs, phases: { diff --git a/tests/health-diagnostic-rules/worktree-health.test.cjs b/tests/health-diagnostic-rules/worktree-health.test.cjs index 998d99167..18a07356a 100644 --- a/tests/health-diagnostic-rules/worktree-health.test.cjs +++ b/tests/health-diagnostic-rules/worktree-health.test.cjs @@ -306,17 +306,13 @@ describe('W027 — stale git worktree', () => { assert.deepEqual(ruleFor('W027').check(snapshot), []); }); - // GAP (documented in the rule module's own header comment, gap 2): the - // pre-migration `process.cwd()` exclusion of the ACTIVE session's own - // worktree cannot be reproduced here — `Rule.check(snapshot)` has no - // ambient cwd access, and `PlanningSnapshot` carries no "which entry is - // the active worktree" field. This test recreates the real-world shape the - // module doc calls out: the active session's cwd is a LINKED (non-first) - // `git worktree list` entry, not the main repo root — `buildPlanningSnapshot(cwd)` - // is called with `cwd` itself listed as entry index 1 (not the dropped - // index-0 "main" entry) and made stale. W027 fires for it anyway, - // demonstrating the gap rather than silently passing. - test('GAP: fires for the active session\'s own (stale) worktree — no cwd-based exclusion is possible from snapshot alone', (t) => { + // Regression proof (restores `verify.cts:2233-2242`'s pre-migration + // behavior via `PlanningSnapshot.cwd`, see the rule module's own header + // comment): the active session's cwd is a LINKED (non-first) `git worktree + // list` entry, not the main repo root — `buildPlanningSnapshot(cwd)` is + // called with `cwd` itself listed as entry index 1 (not the dropped + // index-0 "main" entry) and made stale. W027 must NOT fire for it. + test('excludes the active session\'s own (stale) worktree — matches snapshot.cwd exactly', (t) => { const cwd = createTempDir('gsd-3309-w027-3-'); t.after(() => cleanup(cwd)); fs.mkdirSync(planningDirOf(cwd), { recursive: true }); @@ -327,10 +323,52 @@ describe('W027 — stale git worktree', () => { mockGitWorktreeListOk(t, buildPorcelain(['/fake/main-repo', cwd])); const snapshot = buildPlanningSnapshot(cwd); + assert.equal(snapshot.cwd, path.resolve(cwd), 'snapshot.cwd must be the resolved active cwd'); + + const diagnostics = ruleFor('W027').check(snapshot); + assert.deepEqual( + diagnostics, + [], + 'the active worktree must be excluded from stale-worktree diagnostics, matching pre-migration behavior', + ); + }); + + test('excludes the active session\'s own (stale) worktree when cwd is NESTED under the worktree path — mirrors verify.cts:2239-2241\'s startsWith check', (t) => { + const cwd = createTempDir('gsd-3309-w027-4-'); + t.after(() => cleanup(cwd)); + const worktreePath = path.join(cwd, 'wt-active'); + const nestedCwd = path.join(worktreePath, 'sub', 'dir'); + fs.mkdirSync(nestedCwd, { recursive: true }); + fs.mkdirSync(planningDirOf(nestedCwd), { recursive: true }); + fs.utimesSync(worktreePath, new Date(), new Date(Date.now() - 2 * 60 * 60 * 1000)); + + mockGitWorktreeListOk(t, buildPorcelain(['/fake/main-repo', worktreePath])); + + const snapshot = buildPlanningSnapshot(nestedCwd); const diagnostics = ruleFor('W027').check(snapshot); - assert.equal(diagnostics.length, 1, 'the active worktree is NOT excluded — this is the documented gap'); + assert.deepEqual( + diagnostics, + [], + 'a worktree that is an ancestor of the active cwd must also be excluded', + ); + }); + + test('a DIFFERENT stale worktree (not the active cwd, not an ancestor of it) still fires', (t) => { + const cwd = createTempDir('gsd-3309-w027-5-'); + t.after(() => cleanup(cwd)); + fs.mkdirSync(planningDirOf(cwd), { recursive: true }); + + const otherStalePath = path.join(cwd, 'wt-other-stale'); + fs.mkdirSync(otherStalePath, { recursive: true }); + fs.utimesSync(otherStalePath, new Date(), new Date(Date.now() - 2 * 60 * 60 * 1000)); + + mockGitWorktreeListOk(t, buildPorcelain(['/fake/main-repo', otherStalePath])); + + const snapshot = buildPlanningSnapshot(cwd); + const diagnostics = ruleFor('W027').check(snapshot); + + assert.equal(diagnostics.length, 1, 'a stale worktree distinct from the active cwd must still be flagged'); assert.equal(diagnostics[0].code, 'W027'); - assert.ok(diagnostics[0].message.includes(cwd)); }); });