diff --git a/.changeset/sharp-seals-run.md b/.changeset/sharp-seals-run.md new file mode 100644 index 000000000..60093f7f9 --- /dev/null +++ b/.changeset/sharp-seals-run.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 3793 +--- +**The stale-worktree health check no longer flags the worktree you are currently in on Windows** — paths that differ only by drive-letter or folder casing (as-typed vs git's canonical spelling) are now recognized as the same directory on Windows, while case-sensitive comparison is preserved on macOS/Linux. (#3663) diff --git a/CONTEXT.md b/CONTEXT.md index 16841a96a..6e1bf3f5b 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -131,7 +131,7 @@ Leaf module owning the `SEVERITY`/`REMEDY_ACTION`/`REMEDY_RISK` enums and `Diagn Module owning the frozen rule-table contract for `validate health`, per ADR-3180 §8.2/§8.3/§8.5 (Phase 11, #3309). Exposes three frozen enums — `SEVERITY` (`error`/`warning`/`info`), `REMEDY_ACTION` (the six real repair actions harvested from `cmdValidateHealth`'s existing `--repair` implementation — `createConfig`, `resetConfig`, `regenerateState`, `addNyquistKey`, `addAiIntegrationPhaseKey`, `backfillMilestones` — plus `advise`, the non-repairable payload every non-actionable finding's fix text becomes), and `REMEDY_RISK` (`none`/`destructive`) — plus the `Diagnostic`/`Remedy`/`Rule` shapes every rule's `check(snapshot: PlanningSnapshot) → Diagnostic[]` signature and every finding's `remedy` conform to. `RULES: Rule[]` is the rule table, fully wired: the static concatenation of the 33 rules exported by the eight Health Diagnostic Rule Groups files below (extracted from `cmdValidateHealth`, `src/verify.cts:1616-2577`; the count is locked by `tests/health-diagnostic.test.cjs`'s frozen RULES assertion and by `scripts/lint-health-diagnostic-rule-table.cjs`, so update all three together). `evaluateRules(snapshot) → Diagnostic[]` runs every rule in `RULES` against one `PlanningSnapshot` and flattens the results, throwing on any two rules sharing a `code` — defense in depth beside the static 1:1 lint guard (§8.2 rule 1, `scripts/lint-health-diagnostic-rule-table.cjs`). `applyRepairs(cwd, diagnostics, repair, backfill) → {applied, refused, details}` is the `--repair`/`--backfill` dispatcher: a `DESTRUCTIVE` remedy (`resetConfig`/`regenerateState` — health.md's own published table: "loses custom settings" / "loses session history") is reported but never executed by `--repair`, a deliberate, disclosed breaking change (§8.3 rule 3) from `cmdValidateHealth`'s current unconditional application; `backfillMilestones` alone among the `NONE`-risk actions is requested by `--backfill` without `--repair`, mirroring `cmdValidateHealth`'s existing gate (`src/verify.cts:2504`). Per-action repair handlers (`runRepairAction`) are REAL, ported behavior-preserving from `verify.cts:2405-2553`'s repair switch — `createConfig`/`resetConfig` (write the default config.json payload), `regenerateState` (backs up and regenerates STATE.md), `addNyquistKey`/`addAiIntegrationPhaseKey` (add a missing `workflow.*` key), `backfillMilestones` (synthesize missing MILESTONES.md entries from archive snapshots) — not stubs. `applied` records only a repair that actually SUCCEEDED (`outcome.success === true`); a thrown or `{success: false}` attempt is recorded in `details` with `success: false` but is never pushed to `applied`. Source of truth: `gsd-core/bin/lib/health-diagnostic.cjs` (generated from `src/health-diagnostic.cts`). Design: `.gsd/phase/refactor-3309-health-diagnostic-rule-table/40-design.md`. ### Health Diagnostic Rule Groups -Directory `src/health-diagnostic-rules/` (Phase 11, #3309, ADR-3180 §8.2/§8.3/§8.5) owning the 33 rules migrated off `cmdValidateHealth` (the count actually summed from each group's exported `RULES` array and locked by `tests/health-diagnostic.test.cjs`'s "RULES" describe block; it was stale at 31 through W028 and is corrected here alongside W029, #3586), split into eight files — one per subject-area group from the design doc's "Rule table organization" table — each exporting a `RULES: Rule[]` conforming to the Health Diagnostic Module's frozen `Rule` shape. `src/health-diagnostic.cts` concatenates all eight into the single `RULES` table `evaluateRules` runs; no group re-derives its own `Diagnostic`/`Remedy` shapes. Groups: `root-existence.cts` (root `.planning/` + PROJECT.md existence, E002-E004/W001), `state-consistency.cts` (STATE.md vs config/ROADMAP/disk, W002/W011/W021/W026 — W024's state_head freshness check is a disclosed gap, deliberately not migrated), `config-validation.cts` (config.json shape, W003/W004/W022/E005/W008/W012-W016, plus W029 — the tracked-but-gitignored `.planning/` contradiction, #3586), `phase-structure.cts` (phase directory structure, W005/W023/I001/W009), `agent-install.cts` (agent-installation completeness, W010), `roadmap-disk-consistency.cts` (ROADMAP-vs-disk phase matching via the shared `matchPhaseDirs` matcher, W006/W007), `worktree-health.cts` (worktree health, W020/W017/W027), `milestone-archive-hygiene.cts` (milestone archive + root hygiene, W018/W019). Every rule is a behavior-preserving port of one `addIssue` call site in `cmdValidateHealth` (`src/verify.cts`), reading only the parsed `PlanningSnapshot` fields the Planning Snapshot Module already computes — never raw `.planning/` I/O. Source of truth: `gsd-core/bin/lib/health-diagnostic-rules/*.cjs` (generated from `src/health-diagnostic-rules/*.cts`). +Directory `src/health-diagnostic-rules/` (Phase 11, #3309, ADR-3180 §8.2/§8.3/§8.5) owning the 33 rules migrated off `cmdValidateHealth` (the count actually summed from each group's exported `RULES` array and locked by `tests/health-diagnostic.test.cjs`'s "RULES" describe block; it was stale at 31 through W028 and is corrected here alongside W029, #3586), split into eight files — one per subject-area group from the design doc's "Rule table organization" table — each exporting a `RULES: Rule[]` conforming to the Health Diagnostic Module's frozen `Rule` shape (worktree-health.cts additionally exports `isActiveWorktreePath(activeCwd, worktreePath, platform?)`, the #3663 win32-only path-casing comparison key helper W027's active-worktree exclusion delegates to — a pure predicate, not a rule). `src/health-diagnostic.cts` concatenates all eight into the single `RULES` table `evaluateRules` runs; no group re-derives its own `Diagnostic`/`Remedy` shapes. Groups: `root-existence.cts` (root `.planning/` + PROJECT.md existence, E002-E004/W001), `state-consistency.cts` (STATE.md vs config/ROADMAP/disk, W002/W011/W021/W026 — W024's state_head freshness check is a disclosed gap, deliberately not migrated), `config-validation.cts` (config.json shape, W003/W004/W022/E005/W008/W012-W016, plus W029 — the tracked-but-gitignored `.planning/` contradiction, #3586), `phase-structure.cts` (phase directory structure, W005/W023/I001/W009), `agent-install.cts` (agent-installation completeness, W010), `roadmap-disk-consistency.cts` (ROADMAP-vs-disk phase matching via the shared `matchPhaseDirs` matcher, W006/W007), `worktree-health.cts` (worktree health, W020/W017/W027), `milestone-archive-hygiene.cts` (milestone archive + root hygiene, W018/W019). Every rule is a behavior-preserving port of one `addIssue` call site in `cmdValidateHealth` (`src/verify.cts`), reading only the parsed `PlanningSnapshot` fields the Planning Snapshot Module already computes — never raw `.planning/` I/O. Source of truth: `gsd-core/bin/lib/health-diagnostic-rules/*.cjs` (generated from `src/health-diagnostic-rules/*.cts`). ### Planning Workspace Module Module owning `.planning` path resolution, active workstream pointer policy (`session-scoped > shared`), pointer self-heal behavior, and planning lock semantics for workstream-aware execution. diff --git a/src/health-diagnostic-rules/worktree-health.cts b/src/health-diagnostic-rules/worktree-health.cts index 8c40c6c4e..8300d895d 100644 --- a/src/health-diagnostic-rules/worktree-health.cts +++ b/src/health-diagnostic-rules/worktree-health.cts @@ -35,7 +35,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 +import shellCmdProjection = require('../shell-command-projection.cjs'); // 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'); @@ -141,15 +142,35 @@ function checkW017(snapshot: PlanningSnapshot): Diagnostic[] { // (with the real path) lives in `message`; `remedy.args.command` stays a // static `` template, mirroring the split the brief specifies. +// ─── #3663 — path-comparison provenance helper ───────────────────────────── +// +// snapshot.cwd is raw process-cwd-derived — path.resolve() normalizes +// separators and relative segments but NOT casing, and a process launched via +// a differently-cased path echoes that spelling back. finding.path is +// `git worktree list`-derived, which self-normalizes to the canonical +// on-disk casing (forward slashes). Comparing those two spellings strictly +// misclassifies the ACTIVE worktree as stale on win32 — so the comparison +// folds case ONLY on win32 (case-insensitive filesystem, via the Shell +// Command Projection seam's toComparablePathKey — the platform-conditional +// fold policy lives there, not per call site) and stays case-sensitive on +// POSIX, where differently-cased paths are genuinely different directories. +// No realpath resolution — that would change symlink matching behavior. +function isActiveWorktreePath( + activeCwd: string, + worktreePath: string, + platform: string = process.platform, +): boolean { + const active = shellCmdProjection.toComparablePathKey(activeCwd, platform); + const worktree = shellCmdProjection.toComparablePathKey(worktreePath, platform); + return active === worktree || active.startsWith(worktree + '/'); +} + 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; + if (isActiveWorktreePath(activeCwd, finding.path)) continue; diagnostics.push({ code: 'W027', severity: SEVERITY.WARNING, @@ -194,4 +215,4 @@ const RULES: Rule[] = [ }, ]; -export = { RULES }; +export = { RULES, isActiveWorktreePath }; diff --git a/src/shell-command-projection.cts b/src/shell-command-projection.cts index 0aa40e58f..e007dd21c 100644 --- a/src/shell-command-projection.cts +++ b/src/shell-command-projection.cts @@ -56,6 +56,21 @@ export function posixNormalize(p: string): string { return p.replace(/\\/g, '/'); } +/** + * #3663 — comparison key for a path that must equal another path regardless + * of spelling: separator form (forward slashes via posixNormalize), relative + * segments and trailing separators (path.resolve + strip), and — ONLY on + * win32's case-insensitive filesystem — letter casing. POSIX stays + * case-sensitive: differently-cased paths are genuinely different + * directories there. This module owns the platform-conditional fold so the + * policy lives at the seam instead of accreting per-call-site copies (the + * class init.cts's normalizeForCompare/toComparableRaw predate). + */ +export function toComparablePathKey(p: string, platform: string = process.platform): string { + const normalized = posixNormalize(path.resolve(p)).replace(/\/+$/g, ''); + return platform === 'win32' ? normalized.toLowerCase() : normalized; +} + /** * Return true when a managed hook command must be prefixed with PowerShell's * call operator so a quoted executable token is invokable by the target diff --git a/tests/health-diagnostic-rules/worktree-health.test.cjs b/tests/health-diagnostic-rules/worktree-health.test.cjs index 60dc090e9..a9e62536b 100644 --- a/tests/health-diagnostic-rules/worktree-health.test.cjs +++ b/tests/health-diagnostic-rules/worktree-health.test.cjs @@ -502,3 +502,99 @@ describe('W027 — stale git worktree', () => { }); }); }); + +// ─── W027 — #3663: path-casing provenance mismatch (win32 fold, posix strict) ── +// +// snapshot.cwd is raw process-cwd-derived (no casing normalization) while +// finding.path is git-worktree-list-derived (canonical on-disk casing, forward +// slashes). On win32 the two can spell the same directory differently, which +// made checkW027 flag the ACTIVE worktree as stale. The fold happens ONLY on +// win32 — POSIX comparison stays case-sensitive (criterion 2 in the issue's +// triage brief: differently-cased paths are genuinely different directories +// on case-sensitive filesystems). + +describe('W027 — #3663 isActiveWorktreePath casing/separator normalization', () => { + test('isActiveWorktreePath folds casing on win32', () => { + assert.equal( + worktreeHealth.isActiveWorktreePath('C:\\Repos\\P\\.claude\\worktrees\\Wt', 'c:/repos/p/.claude/worktrees/Wt', 'win32'), + true, + 'the same directory spelled with different casing must match on win32', + ); + }); + + test('isActiveWorktreePath folds casing for nested cwd on win32', () => { + assert.equal( + worktreeHealth.isActiveWorktreePath('c:/REPOS/p/.claude/worktrees/wt/sub/dir', 'C:\\Repos\\p\\.claude\\worktrees\\WT', 'win32'), + true, + 'a cwd nested under the worktree (differently cased) must match on win32', + ); + }); + + test('isActiveWorktreePath stays case-sensitive on posix', () => { + assert.equal( + worktreeHealth.isActiveWorktreePath('/tmp/A/wt', '/tmp/a/wt', 'linux'), + false, + 'differently-cased paths are different directories on case-sensitive filesystems', + ); + }); + + test('isActiveWorktreePath matches exact paths on posix', () => { + assert.equal(worktreeHealth.isActiveWorktreePath('/tmp/a/wt', '/tmp/a/wt', 'linux'), true); + assert.equal(worktreeHealth.isActiveWorktreePath('/tmp/a/wt/sub', '/tmp/a/wt', 'linux'), true); + assert.equal(worktreeHealth.isActiveWorktreePath('/tmp/a/wtx', '/tmp/a/wt', 'linux'), false, 'boundary segment wtx must not prefix-match wt'); + }); + + test('isActiveWorktreePath does not fold distinct worktrees together on win32', () => { + assert.equal( + worktreeHealth.isActiveWorktreePath('C:\\Repos\\p\\.claude\\worktrees\\alpha', 'C:\\Repos\\p\\.claude\\worktrees\\beta', 'win32'), + false, + ); + }); + + test('isActiveWorktreePath normalizes separators and trailing slashes', () => { + assert.equal( + worktreeHealth.isActiveWorktreePath('C:/Repos/p/wt/', 'C:\\Repos\\p\\wt', 'win32'), + true, + 'mixed separators and a trailing slash must not break the comparison', + ); + }); + + test('isActiveWorktreePath keeps the segment boundary under win32 folding', () => { + assert.equal( + worktreeHealth.isActiveWorktreePath('C:\\Repos\\p\\wt-alphabeta', 'C:\\Repos\\p\\wt-alpha', 'win32'), + false, + 'a sibling worktree whose name extends another (wt-alphabeta vs wt-alpha) must not prefix-match', + ); + }); + + test('isActiveWorktreePath treats a drive root as the ancestor it is', () => { + // 'C:/' strips to 'c:'; the + '/' guard makes the comparison + // startsWith('c:/') — the drive root contains everything on it, which is + // correct ancestor semantics (and unreachable via git worktree list). + assert.equal(worktreeHealth.isActiveWorktreePath('C:\\Repos\\p\\wt', 'C:/', 'win32'), true); + assert.equal(worktreeHealth.isActiveWorktreePath('/srv/app', '/', 'linux'), true); + }); + + test('W027 still fires for differently-cased paths on posix (case-sensitive pin)', (t) => { + const cwd = createTempDir('gsd-3663-w027-posix-case-'); + t.after(() => cleanup(cwd)); + fs.mkdirSync(planningDirOf(cwd), { recursive: true }); + + // A stale finding whose path differs from snapshot.cwd ONLY by casing. + // The variant flips a letter of the FIXED fixture prefix — deriving it + // from the random mkdtemp suffix would flake (1/62 the suffix is already + // 'X') or pass vacuously (60/62 it differs by a different character, not + // case). On POSIX the two spellings are different directories, so the + // stale finding MUST still be reported — folding here would be a + // criterion-2 regression. + const stalePath = cwd.replace('gsd-3663-w027-posix-case-', 'gsd-3663-w027-posix-Case-'); + fs.mkdirSync(stalePath, { recursive: true }); + fs.utimesSync(stalePath, new Date(), new Date(Date.now() - 2 * 60 * 60 * 1000)); + mockGitWorktreeListOk(t, buildPorcelain(['/fake/main-repo', stalePath])); + + const snapshot = buildPlanningSnapshot(cwd); + const diagnostics = ruleFor('W027').check(snapshot); + assert.equal(diagnostics.length, 1, 'differently-cased path is a distinct worktree on posix and must still be flagged'); + assert.equal(diagnostics[0].code, 'W027'); + }); +});