From ca3be82f71f5f3c673840824cddda004aefbe885 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Sat, 16 May 2026 12:39:02 -0400 Subject: [PATCH] fix(3597): clear residual Windows test failures + add ratchet lint guard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two more diagnostic passes (clusters J: residuals in already-touched files, K: 12 untouched files) plus a production-code path fix and a new ratchet-style lint guard. ## Test-only fixes (14 files) bug-1736, bug-2248, bug-2698 — replace inline 1s-budget rmSync with the shared cleanup() helper (5s budget, 20×250ms retries). The earlier inline maxRetries:10 / retryDelay:100 wasn't enough to absorb Windows Defender's deferred-scan handle hold on cold runners. bug-2256, skill-manifest — also override USERPROFILE alongside HOME in beforeEach/runGsdTools calls. os.homedir() reads USERPROFILE on win32, so HOME-only stubs leak the runner's real home into the SUT. bug-2784, bug-3608, enh-2500, enh-2790, few-shot-calibration, gsd-settings-advanced — CRLF tolerance: literal \n in regexes against file content (frontmatter anchors, bash-fence regex, multi-line numbered-list captures, awk-block extractors) becomes \r?\n; split('\n') becomes split(/\r?\n/). Windows checkout with autocrlf=true puts \r before every \n; .+ doesn't match \r in JS regex by default. bug-2966 — three-part fix to extractStepRun (CRLF split), awk regex (\r?\n), and conflict-marker parser (rawLine + \r$ strip). bug-2969, config — normalize separators on test assertions where the SUT correctly emits \ on win32 but the test compares against /. prompt-injection-scan — normalize relPath via replace(/\\/g, '/') before ALLOWLIST.has() lookup. ALLOWLIST keys are POSIX; path.relative returns backslashes on win32 → falsely scans allowlisted security module → trips the boundary-tag detector on its own legitimate detection code. prune-orphaned-worktrees — use the existing canonicalPath + listedWorktreePaths(repoDir).has(...) helpers instead of substring matching the raw path. git stores long-form canonical paths (runneradmin), but mkdtempSync returns 8.3 short-form (RUNNER~1) on Windows runners; plain string compare misses every entry. ## Production-code fix (1 file) get-shit-done/bin/lib/init.cjs — bug-3491 in_nested_subdir computation canonicalizes both worktreeRoot and cwd via fs.realpathSync.native + path.relative before declaring "nested." Windows runner cwd (8.3 short name) vs git's --show-toplevel (long form, forward slashes) made the raw string compare always say true even at the worktree root, breaking the "init new-project at worktree root" subtest. ## New ratchet lint guard tests/windows-test-parity-guard.test.cjs — scans tests/ for 7 anti-patterns that drove the Windows failure clusters. Each rule has a baseline count snapshot from this PR; the test fails when a new occurrence appears (count grows above baseline), ratcheting down as existing offenders are fixed. Patterns covered: G1 split('\n') after readFileSync (use /\r?\n/) G2 ```bash\n fence regex (use ```bash\r?\n) G3 ^---\n frontmatter anchor (use ^---\r?\n) G4 hardcoded "/tmp/..." literal passed to fs.* (use os.tmpdir()) G5 bare 'npm' to execFileSync without {shell:true} on win32 G6 process.env.HOME stub with no USERPROFILE G7 fs.rmSync({recursive,force}) without maxRetries Future Windows-parity regressions get caught at PR time rather than five iterations into a CI loop. Validated: holodeck (ubuntu docker) 11232/0 pass (count +8 = the 7 new ratchet tests + parent describe). Co-Authored-By: Claude Opus 4.7 (1M context) --- get-shit-done/bin/lib/init.cjs | 34 +++- .../bug-1736-local-install-commands.test.cjs | 5 +- ...bug-2248-local-install-statusline.test.cjs | 4 +- ...ug-2256-model-overrides-transport.test.cjs | 12 ++ tests/bug-2698-crlf-install.test.cjs | 4 +- .../bug-2784-update-cache-clear-path.test.cjs | 4 +- ...-2966-cherry-pick-context-missing.test.cjs | 10 +- .../bug-2969-verify-reapply-patches.test.cjs | 3 +- ...ity-update-runtime-classification.test.cjs | 2 +- tests/config.test.cjs | 5 +- ...-codebase-mapper-arch-rich-format.test.cjs | 4 +- tests/enh-2790-skill-consolidation.test.cjs | 4 +- tests/few-shot-calibration.test.cjs | 4 +- tests/gsd-settings-advanced.test.cjs | 2 +- tests/prompt-injection-scan.test.cjs | 40 +++- tests/prune-orphaned-worktrees.test.cjs | 21 +- tests/skill-manifest.test.cjs | 8 +- tests/windows-test-parity-guard.test.cjs | 183 ++++++++++++++++++ 18 files changed, 310 insertions(+), 39 deletions(-) create mode 100644 tests/windows-test-parity-guard.test.cjs diff --git a/get-shit-done/bin/lib/init.cjs b/get-shit-done/bin/lib/init.cjs index 7f06825ec..6502da042 100644 --- a/get-shit-done/bin/lib/init.cjs +++ b/get-shit-done/bin/lib/init.cjs @@ -463,7 +463,22 @@ function cmdInitNewProject(cwd, raw) { ...(() => { const info = gitWorktreeInfoInternal(cwd); const worktreeRoot = info.worktreeRoot; - const inNestedSubdir = info.inside && worktreeRoot !== null && worktreeRoot !== cwd; + // Canonicalize both sides before comparing: on Windows the runner's + // cwd may be the 8.3 short-name form (RUNNER~1) while git's + // --show-toplevel emits the long-form path with forward slashes. + // Without canonicalization, in_nested_subdir is `true` even at the + // worktree root (bug #3491). realpathSync.native handles 8.3→long + // expansion; path.resolve normalizes separators. Wrap in try so + // a missing path falls back to the original string compare. + let inNestedSubdir = info.inside && worktreeRoot !== null && worktreeRoot !== cwd; + if (inNestedSubdir) { + try { + const canonRoot = fs.realpathSync.native(worktreeRoot); + const canonCwd = fs.realpathSync.native(cwd); + const rel = path.relative(canonRoot, canonCwd); + inNestedSubdir = rel !== '' && !rel.startsWith('..'); + } catch { /* keep raw-string compare result */ } + } return { has_git: info.inside, git_worktree_root: worktreeRoot, @@ -607,7 +622,22 @@ function cmdInitIngestDocs(cwd, raw) { // Bug #3491 — see cmdInitNewProject above. Same shallow-check bug. const info = gitWorktreeInfoInternal(cwd); const worktreeRoot = info.worktreeRoot; - const inNestedSubdir = info.inside && worktreeRoot !== null && worktreeRoot !== cwd; + // Canonicalize both sides before comparing: on Windows the runner's + // cwd may be the 8.3 short-name form (RUNNER~1) while git's + // --show-toplevel emits the long-form path with forward slashes. + // Without canonicalization, in_nested_subdir is `true` even at the + // worktree root (bug #3491). realpathSync.native handles 8.3→long + // expansion; path.resolve normalizes separators. Wrap in try so + // a missing path falls back to the original string compare. + let inNestedSubdir = info.inside && worktreeRoot !== null && worktreeRoot !== cwd; + if (inNestedSubdir) { + try { + const canonRoot = fs.realpathSync.native(worktreeRoot); + const canonCwd = fs.realpathSync.native(cwd); + const rel = path.relative(canonRoot, canonCwd); + inNestedSubdir = rel !== '' && !rel.startsWith('..'); + } catch { /* keep raw-string compare result */ } + } return { has_git: info.inside, git_worktree_root: worktreeRoot, diff --git a/tests/bug-1736-local-install-commands.test.cjs b/tests/bug-1736-local-install-commands.test.cjs index 5dcf6550c..fd33861fe 100644 --- a/tests/bug-1736-local-install-commands.test.cjs +++ b/tests/bug-1736-local-install-commands.test.cjs @@ -22,6 +22,7 @@ const { execFileSync } = require('child_process'); const INSTALL_SRC = path.join(__dirname, '..', 'bin', 'install.js'); const BUILD_SCRIPT = path.join(__dirname, '..', 'scripts', 'build-hooks.js'); const { install, copyCommandsAsClaudeSkills } = require(INSTALL_SRC); +const { cleanup } = require('./helpers.cjs'); // ─── Ensure hooks/dist/ is populated before install tests ──────────────────── // With --test-concurrency=4, other install tests (bug-1834, bug-1924) run @@ -46,7 +47,9 @@ describe('#1736: local Claude install populates .claude/commands/gsd/', () => { }); afterEach(() => { - fs.rmSync(tmpDir, { recursive: true, force: true, maxRetries: 10, retryDelay: 100 }); + // Use the shared helper which has a 5s Windows-EBUSY retry budget + // (20×250ms). The inline 1s budget here was insufficient on cold runners. + cleanup(tmpDir); }); test('local install creates .claude/commands/gsd/ directory', (t) => { diff --git a/tests/bug-2248-local-install-statusline.test.cjs b/tests/bug-2248-local-install-statusline.test.cjs index 55da484dd..d35af8a4c 100644 --- a/tests/bug-2248-local-install-statusline.test.cjs +++ b/tests/bug-2248-local-install-statusline.test.cjs @@ -28,6 +28,7 @@ const { execFileSync } = require('child_process'); const INSTALL_SRC = path.join(__dirname, '..', 'bin', 'install.js'); const BUILD_SCRIPT = path.join(__dirname, '..', 'scripts', 'build-hooks.js'); const { install, finishInstall } = require(INSTALL_SRC); +const { cleanup } = require('./helpers.cjs'); // ─── Ensure hooks/dist/ is populated before install tests ──────────────────── before(() => { @@ -47,7 +48,8 @@ describe('#2248: local Claude install does not clobber profile-level statusLine' }); afterEach(() => { - fs.rmSync(tmpDir, { recursive: true, force: true, maxRetries: 10, retryDelay: 100 }); + // Use the shared 5s Windows-EBUSY retry budget instead of inline 1s. + cleanup(tmpDir); }); test('local install does not write statusLine to .claude/settings.json', (t) => { diff --git a/tests/bug-2256-model-overrides-transport.test.cjs b/tests/bug-2256-model-overrides-transport.test.cjs index fdc85d213..5e0e04727 100644 --- a/tests/bug-2256-model-overrides-transport.test.cjs +++ b/tests/bug-2256-model-overrides-transport.test.cjs @@ -19,6 +19,8 @@ const fs = require('fs'); const path = require('path'); const os = require('os'); +const isWindows = process.platform === 'win32'; + const { readGsdEffectiveModelOverrides, generateCodexAgentToml, @@ -43,17 +45,27 @@ describe('bug #2256 — readGsdEffectiveModelOverrides', () => { let projectDir; let homeDir; let origHome; + let origUserProfile; beforeEach(() => { projectDir = makeTmp('proj'); homeDir = makeTmp('home'); origHome = process.env.HOME; + // On Windows, os.homedir() reads USERPROFILE (not HOME). Tests that + // need to redirect ~ must override both — otherwise the SUT reads + // the real user's home and the fixture is invisible. + origUserProfile = process.env.USERPROFILE; process.env.HOME = homeDir; + if (isWindows) process.env.USERPROFILE = homeDir; }); afterEach(() => { if (origHome === undefined) delete process.env.HOME; else process.env.HOME = origHome; + if (isWindows) { + if (origUserProfile === undefined) delete process.env.USERPROFILE; + else process.env.USERPROFILE = origUserProfile; + } rmr(projectDir); rmr(homeDir); }); diff --git a/tests/bug-2698-crlf-install.test.cjs b/tests/bug-2698-crlf-install.test.cjs index 3e5afe648..b19699ceb 100644 --- a/tests/bug-2698-crlf-install.test.cjs +++ b/tests/bug-2698-crlf-install.test.cjs @@ -43,6 +43,7 @@ const { execFileSync } = require('child_process'); const INSTALL_SRC = path.join(__dirname, '..', 'bin', 'install.js'); const BUILD_SCRIPT = path.join(__dirname, '..', 'scripts', 'build-hooks.js'); const { install, GSD_CODEX_MARKER } = require(INSTALL_SRC); +const { cleanup } = require('./helpers.cjs'); // Ensure hooks/dist/ is populated before install tests before(() => { @@ -60,7 +61,8 @@ describe('#2698: CRLF stale gsd-update-check block is removed on Codex reinstall }); afterEach(() => { - fs.rmSync(tmpDir, { recursive: true, force: true, maxRetries: 10, retryDelay: 100 }); + // Use the shared 5s Windows-EBUSY retry budget instead of inline 1s. + cleanup(tmpDir); }); // Helper: pre-populate .codex/config.toml with a GSD marker + stale hooks block diff --git a/tests/bug-2784-update-cache-clear-path.test.cjs b/tests/bug-2784-update-cache-clear-path.test.cjs index b80a151fc..a103a22e1 100644 --- a/tests/bug-2784-update-cache-clear-path.test.cjs +++ b/tests/bug-2784-update-cache-clear-path.test.cjs @@ -61,10 +61,10 @@ describe('bug-2784: update.md cache-clear covers shared cache path', () => { const stepContent = stepMatch[0]; const bashLines = []; - const fenceRe = /```(?:bash|sh)\n([\s\S]*?)```/g; + const fenceRe = /```(?:bash|sh)\r?\n([\s\S]*?)```/g; let m; while ((m = fenceRe.exec(stepContent)) !== null) { - for (const line of m[1].split('\n')) { + for (const line of m[1].split(/\r?\n/)) { const trimmed = line.trim(); if (trimmed) bashLines.push(trimmed); } diff --git a/tests/bug-2966-cherry-pick-context-missing.test.cjs b/tests/bug-2966-cherry-pick-context-missing.test.cjs index df0021ca4..32b55c342 100644 --- a/tests/bug-2966-cherry-pick-context-missing.test.cjs +++ b/tests/bug-2966-cherry-pick-context-missing.test.cjs @@ -58,7 +58,9 @@ const WORKFLOW_PATH = path.join(__dirname, '..', '.github', 'workflows', 'releas * one for a single test isn't justified. */ function extractStepRun(workflowText, stepName) { - const lines = workflowText.split('\n'); + // CRLF-tolerant split: Windows checkout (autocrlf=true) leaves trailing + // \r on every line which downstream regex anchors don't tolerate. + const lines = workflowText.split(/\r?\n/); for (let i = 0; i < lines.length; i++) { const m = lines[i].match(/^(\s*)- name:\s*(.+?)\s*$/); if (!m || m[2] !== stepName) continue; @@ -197,7 +199,9 @@ describe('bug-2966: release-sdk hotfix cherry-pick classifies context-missing vs const blocks = []; let inHead = false; let head = ''; - for (const line of conflicted.split('\n')) { + for (const rawLine of conflicted.split(/\r?\n/)) { + // Strip residual \r so /^=======$/ matches even on CRLF content. + const line = rawLine.replace(/\r$/, ''); if (/^<<<<<<< /.test(line)) { inHead = true; head = ''; continue; } if (/^=======$/.test(line) && inHead) { inHead = false; continue; } if (/^>>>>>>> /.test(line)) { blocks.push(head); head = ''; continue; } @@ -217,7 +221,7 @@ describe('bug-2966: release-sdk hotfix cherry-pick classifies context-missing vs // exercises the exact predicate that runs in CI — not a copy. const yaml = fs.readFileSync(WORKFLOW_PATH, 'utf8'); const script = extractStepRun(yaml, 'Prepare hotfix branch'); - const awkMatch = script.match(/awk '\n([\s\S]+?)' "\$CONFLICTED"/); + const awkMatch = script.match(/awk '\r?\n([\s\S]+?)' "\$CONFLICTED"/); assert.ok(awkMatch, 'expected to find the conflict-classifying awk script in the workflow'); const awkProgram = awkMatch[1]; diff --git a/tests/bug-2969-verify-reapply-patches.test.cjs b/tests/bug-2969-verify-reapply-patches.test.cjs index fedd6e2c3..bb7d8a6c5 100644 --- a/tests/bug-2969-verify-reapply-patches.test.cjs +++ b/tests/bug-2969-verify-reapply-patches.test.cjs @@ -128,7 +128,8 @@ describe('Bug #2969: deterministic Step 5 verification gate', () => { assert.equal(status, 1); assert.equal(report.failures, 1); const r0 = report.results[0]; - assert.equal(r0.file, 'skills/discuss-phase/SKILL.md'); + // Normalize separators: on Windows the SUT emits 'skills\discuss-phase\SKILL.md'. + assert.equal(r0.file.replace(/\\/g, '/'), 'skills/discuss-phase/SKILL.md'); assert.equal(r0.status, 'fail'); assert.equal(r0.reason, REASON.FAIL_USER_LINES_MISSING); assert.ok( diff --git a/tests/bug-3608-antigravity-update-runtime-classification.test.cjs b/tests/bug-3608-antigravity-update-runtime-classification.test.cjs index 2badfbdb5..d3d9e9c12 100644 --- a/tests/bug-3608-antigravity-update-runtime-classification.test.cjs +++ b/tests/bug-3608-antigravity-update-runtime-classification.test.cjs @@ -89,7 +89,7 @@ describe('bug #3608: update.md models Antigravity as a first-class runtime', () // Extract the inference block — the if/elif ladder that maps env vars to runtime. // Match from the comment marker through the closing `fi` of the inference block. const blockMatch = content.match( - /If runtime is still unknown, infer from runtime env vars[\s\S]*?\nfi\n/, + /If runtime is still unknown, infer from runtime env vars[\s\S]*?\r?\nfi\r?\n/, ); assert.ok(blockMatch, 'env-var inference block not found'); diff --git a/tests/config.test.cjs b/tests/config.test.cjs index 5bee2988e..fbb955818 100644 --- a/tests/config.test.cjs +++ b/tests/config.test.cjs @@ -953,14 +953,15 @@ describe('config-path command (#2282)', () => { test('returns root config path when no workstream is active', () => { const result = runGsdTools('config-path', tmpDir); assert.ok(result.success, `config-path failed: ${result.error}`); - assert.ok(result.output.trim().endsWith('.planning/config.json'), `expected root config path, got: ${result.output}`); + // Normalize separators: Windows emits backslashes in the resolved path. + assert.ok(result.output.trim().replace(/\\/g, '/').endsWith('.planning/config.json'), `expected root config path, got: ${result.output}`); assert.ok(!result.output.includes('workstreams'), 'should not include workstreams in path'); }); test('returns workstream config path when GSD_WORKSTREAM is set', () => { const result = runGsdTools('config-path', tmpDir, { GSD_WORKSTREAM: 'my-stream' }); assert.ok(result.success, `config-path failed: ${result.error}`); - assert.ok(result.output.trim().includes('workstreams/my-stream/config.json'), `expected workstream config path, got: ${result.output}`); + assert.ok(result.output.trim().replace(/\\/g, '/').includes('workstreams/my-stream/config.json'), `expected workstream config path, got: ${result.output}`); }); test('config-path and config-get agree on the active path', () => { diff --git a/tests/enh-2500-codebase-mapper-arch-rich-format.test.cjs b/tests/enh-2500-codebase-mapper-arch-rich-format.test.cjs index fb35ae2b7..96ab5dc51 100644 --- a/tests/enh-2500-codebase-mapper-arch-rich-format.test.cjs +++ b/tests/enh-2500-codebase-mapper-arch-rich-format.test.cjs @@ -99,7 +99,9 @@ describe('enh-2500: gsd-codebase-mapper arch focus — rich architecture output' test('template includes data flow traces with numbered steps', () => { const hasPrimaryRequestPath = /###\s+Primary Request Path/i.test(archTemplate); - const hasThreeNumberedSteps = /^\s*1\..+\n\s*2\..+\n\s*3\./m.test(archTemplate); + // [^\n]+ + \r?\n is CRLF-tolerant: .+ doesn't match \r in JS regex by + // default, so \r before the literal \n in CRLF content kills the match. + const hasThreeNumberedSteps = /^\s*1\.[^\n]+\r?\n\s*2\.[^\n]+\r?\n\s*3\./m.test(archTemplate); const hasFileLineRefs = /\(`\[.*:(?:line|\d+)\]`\)/.test(archTemplate); assert.ok( diff --git a/tests/enh-2790-skill-consolidation.test.cjs b/tests/enh-2790-skill-consolidation.test.cjs index b9fdf66d1..52b4feaf1 100644 --- a/tests/enh-2790-skill-consolidation.test.cjs +++ b/tests/enh-2790-skill-consolidation.test.cjs @@ -18,7 +18,9 @@ const COMMANDS_DIR = path.join(__dirname, '..', 'commands', 'gsd'); */ function parseFrontmatter(filePath) { const raw = fs.readFileSync(filePath, 'utf8'); - const lines = raw.split('\n'); + // CRLF-tolerant: Windows checkouts leave \r on every line. lines.indexOf('---', 1) + // would never match because elements would be '---\r' instead of '---'. + const lines = raw.split(/\r?\n/); if (lines[0].trim() !== '---') return {}; const endIdx = lines.indexOf('---', 1); if (endIdx === -1) return {}; diff --git a/tests/few-shot-calibration.test.cjs b/tests/few-shot-calibration.test.cjs index 297230a8e..681d19c71 100644 --- a/tests/few-shot-calibration.test.cjs +++ b/tests/few-shot-calibration.test.cjs @@ -32,7 +32,7 @@ describe('few-shot calibration examples', () => { describe('frontmatter metadata', () => { test('plan-checker.md has version and component in frontmatter', () => { const content = readFile(path.join(REFS_DIR, 'plan-checker.md')); - assert.match(content, /^---\n/); + assert.match(content, /^---\r?\n/); assert.match(content, /component:\s*plan-checker/); assert.match(content, /version:\s*\d+/); assert.match(content, /last_calibrated:\s*\d{4}-\d{2}-\d{2}/); @@ -40,7 +40,7 @@ describe('few-shot calibration examples', () => { test('verifier.md has version and component in frontmatter', () => { const content = readFile(path.join(REFS_DIR, 'verifier.md')); - assert.match(content, /^---\n/); + assert.match(content, /^---\r?\n/); assert.match(content, /component:\s*verifier/); assert.match(content, /version:\s*\d+/); assert.match(content, /last_calibrated:\s*\d{4}-\d{2}-\d{2}/); diff --git a/tests/gsd-settings-advanced.test.cjs b/tests/gsd-settings-advanced.test.cjs index a563fef1e..082b83709 100644 --- a/tests/gsd-settings-advanced.test.cjs +++ b/tests/gsd-settings-advanced.test.cjs @@ -85,7 +85,7 @@ describe('gsd-settings-advanced — file scaffolding', () => { test('command frontmatter has name, description, allowed-tools', () => { const text = fs.readFileSync(COMMAND_PATH, 'utf-8'); - const fmMatch = text.match(/^---\n([\s\S]*?)\n---/); + const fmMatch = text.match(/^---\r?\n([\s\S]*?)\r?\n---/); assert.ok(fmMatch, 'command file missing frontmatter block'); const fm = fmMatch[1]; assert.match(fm, /name:\s*gsd:config/, 'frontmatter missing name (gsd:config)'); diff --git a/tests/prompt-injection-scan.test.cjs b/tests/prompt-injection-scan.test.cjs index fc1a0cc73..a17985542 100644 --- a/tests/prompt-injection-scan.test.cjs +++ b/tests/prompt-injection-scan.test.cjs @@ -101,7 +101,10 @@ describe('codebase prompt injection scan', () => { const findings = []; for (const file of agentFiles) { - const relPath = path.relative(PROJECT_ROOT, file); + // Normalize to POSIX separators so ALLOWLIST.has() works on Windows + // (path.relative returns 'get-shit-done\bin\...' on win32; allowlist + // keys are POSIX 'get-shit-done/bin/...'). + const relPath = path.relative(PROJECT_ROOT, file).replace(/\\/g, '/'); if (ALLOWLIST.has(relPath)) continue; const content = fs.readFileSync(file, 'utf-8'); @@ -131,7 +134,10 @@ describe('codebase prompt injection scan', () => { const oversized = []; for (const file of agentFiles) { - const relPath = path.relative(PROJECT_ROOT, file); + // Normalize to POSIX separators so ALLOWLIST.has() works on Windows + // (path.relative returns 'get-shit-done\bin\...' on win32; allowlist + // keys are POSIX 'get-shit-done/bin/...'). + const relPath = path.relative(PROJECT_ROOT, file).replace(/\\/g, '/'); if (ALLOWLIST.has(relPath)) continue; const content = fs.readFileSync(file, 'utf-8'); @@ -152,7 +158,10 @@ describe('codebase prompt injection scan', () => { const findings = []; for (const file of workflowFiles) { - const relPath = path.relative(PROJECT_ROOT, file); + // Normalize to POSIX separators so ALLOWLIST.has() works on Windows + // (path.relative returns 'get-shit-done\bin\...' on win32; allowlist + // keys are POSIX 'get-shit-done/bin/...'). + const relPath = path.relative(PROJECT_ROOT, file).replace(/\\/g, '/'); if (ALLOWLIST.has(relPath)) continue; const content = fs.readFileSync(file, 'utf-8'); @@ -175,7 +184,10 @@ describe('codebase prompt injection scan', () => { const findings = []; for (const file of commandFiles) { - const relPath = path.relative(PROJECT_ROOT, file); + // Normalize to POSIX separators so ALLOWLIST.has() works on Windows + // (path.relative returns 'get-shit-done\bin\...' on win32; allowlist + // keys are POSIX 'get-shit-done/bin/...'). + const relPath = path.relative(PROJECT_ROOT, file).replace(/\\/g, '/'); if (ALLOWLIST.has(relPath)) continue; const content = fs.readFileSync(file, 'utf-8'); @@ -198,7 +210,10 @@ describe('codebase prompt injection scan', () => { const findings = []; for (const file of hookFiles) { - const relPath = path.relative(PROJECT_ROOT, file); + // Normalize to POSIX separators so ALLOWLIST.has() works on Windows + // (path.relative returns 'get-shit-done\bin\...' on win32; allowlist + // keys are POSIX 'get-shit-done/bin/...'). + const relPath = path.relative(PROJECT_ROOT, file).replace(/\\/g, '/'); if (ALLOWLIST.has(relPath)) continue; const content = fs.readFileSync(file, 'utf-8'); @@ -221,7 +236,10 @@ describe('codebase prompt injection scan', () => { const findings = []; for (const file of libFiles) { - const relPath = path.relative(PROJECT_ROOT, file); + // Normalize to POSIX separators so ALLOWLIST.has() works on Windows + // (path.relative returns 'get-shit-done\bin\...' on win32; allowlist + // keys are POSIX 'get-shit-done/bin/...'). + const relPath = path.relative(PROJECT_ROOT, file).replace(/\\/g, '/'); if (ALLOWLIST.has(relPath)) continue; const content = fs.readFileSync(file, 'utf-8'); @@ -244,7 +262,10 @@ describe('codebase prompt injection scan', () => { const invisiblePattern = /[\u200B-\u200F\u2028-\u202F\uFEFF\u00AD]/; for (const file of allFiles) { - const relPath = path.relative(PROJECT_ROOT, file); + // Normalize to POSIX separators so ALLOWLIST.has() works on Windows + // (path.relative returns 'get-shit-done\bin\...' on win32; allowlist + // keys are POSIX 'get-shit-done/bin/...'). + const relPath = path.relative(PROJECT_ROOT, file).replace(/\\/g, '/'); if (ALLOWLIST.has(relPath)) continue; const content = fs.readFileSync(file, 'utf-8'); @@ -273,7 +294,10 @@ describe('codebase prompt injection scan', () => { const boundaryPattern = /<\/?(?:system|assistant|human)>/i; for (const file of allFiles) { - const relPath = path.relative(PROJECT_ROOT, file); + // Normalize to POSIX separators so ALLOWLIST.has() works on Windows + // (path.relative returns 'get-shit-done\bin\...' on win32; allowlist + // keys are POSIX 'get-shit-done/bin/...'). + const relPath = path.relative(PROJECT_ROOT, file).replace(/\\/g, '/'); if (ALLOWLIST.has(relPath)) continue; // Allow .md files to use common tags in examples/docs // But flag .js/.cjs files that embed these diff --git a/tests/prune-orphaned-worktrees.test.cjs b/tests/prune-orphaned-worktrees.test.cjs index 695963daa..2a4c5930e 100644 --- a/tests/prune-orphaned-worktrees.test.cjs +++ b/tests/prune-orphaned-worktrees.test.cjs @@ -171,12 +171,14 @@ describe('pruneOrphanedWorktrees', () => { execSync('git worktree add "' + worktreeDir + '" -b fix/stale-ref', { cwd: repoDir, stdio: 'pipe' }); assert.ok(fs.existsSync(worktreeDir), 'worktree dir should exist before manual deletion'); - // Verify it appears in git worktree list - const beforeList = execSync('git worktree list --porcelain', { cwd: repoDir, encoding: 'utf8' }); - // git worktree list --porcelain emits forward slashes on Windows even - // when path.join produced backslashes; normalize both sides for compare. - const normalizeSlashes = (p) => p.replace(/\\/g, '/'); - assert.ok(normalizeSlashes(beforeList).includes(normalizeSlashes(worktreeDir)), 'worktree should appear in list before deletion'); + // Use the canonicalPath helper so Windows 8.3 short-name (RUNNER~1) vs + // long-form (runneradmin) and slash-direction differences both collapse + // to the same key before comparison. git stores the long-form path in + // its administrative files; substring matching on the raw path fails. + // Capture the canonical key BEFORE deletion since canonicalPath calls + // realpathSync.native which fails on missing paths. + const wantedKey = canonicalPath(worktreeDir); + assert.ok(listedWorktreePaths(repoDir).has(wantedKey), 'worktree should appear in list before deletion'); // Manually delete the worktree directory (simulate orphan) fs.rmSync(worktreeDir, { recursive: true, force: true }); @@ -185,11 +187,10 @@ describe('pruneOrphanedWorktrees', () => { const pruneOrphanedWorktrees = getPruneOrphanedWorktrees(); pruneOrphanedWorktrees(repoDir); - // Assert: git worktree list no longer shows the stale entry - const afterList = execSync('git worktree list --porcelain', { cwd: repoDir, encoding: 'utf8' }); + // Assert: git worktree list no longer shows the stale entry. assert.ok( - !normalizeSlashes(afterList).includes(normalizeSlashes(worktreeDir)), - 'git worktree list still shows stale entry after prune:\n' + afterList + !listedWorktreePaths(repoDir).has(wantedKey), + 'git worktree list still shows stale entry after prune' ); }); }); diff --git a/tests/skill-manifest.test.cjs b/tests/skill-manifest.test.cjs index 1eca832a3..53d2349c0 100644 --- a/tests/skill-manifest.test.cjs +++ b/tests/skill-manifest.test.cjs @@ -52,7 +52,10 @@ describe('skill-manifest', () => { }); test('returns normalized inventory across canonical roots', () => { - const result = runGsdTools(['skill-manifest'], tmpDir, { HOME: homeDir }); + // On Windows, os.homedir() reads USERPROFILE (not HOME). The SUT scans + // global skill roots via os.homedir(), so the test must also override + // USERPROFILE to keep the fixture's homeDir visible. + const result = runGsdTools(['skill-manifest'], tmpDir, { HOME: homeDir, USERPROFILE: homeDir }); assert.ok(result.success, `Command should succeed: ${result.error || result.output}`); const manifest = JSON.parse(result.output); @@ -117,7 +120,7 @@ describe('skill-manifest', () => { }); test('writes manifest to .planning/skill-manifest.json when --write flag is used', () => { - const result = runGsdTools(['skill-manifest', '--write'], tmpDir, { HOME: homeDir }); + const result = runGsdTools(['skill-manifest', '--write'], tmpDir, { HOME: homeDir, USERPROFILE: homeDir }); assert.ok(result.success, `Command should succeed: ${result.error || result.output}`); const manifestPath = path.join(tmpDir, '.planning', 'skill-manifest.json'); @@ -131,6 +134,7 @@ describe('skill-manifest', () => { test('global roots honor runtime-home env overrides instead of hardcoded home paths', () => { const result = runGsdTools(['skill-manifest'], tmpDir, { HOME: homeDir, + USERPROFILE: homeDir, CLAUDE_CONFIG_DIR: path.join(homeDir, 'claude-custom'), CODEX_HOME: path.join(homeDir, 'codex-custom'), }); diff --git a/tests/windows-test-parity-guard.test.cjs b/tests/windows-test-parity-guard.test.cjs new file mode 100644 index 000000000..6c562911d --- /dev/null +++ b/tests/windows-test-parity-guard.test.cjs @@ -0,0 +1,183 @@ +'use strict'; + +process.env.GSD_TEST_MODE = '1'; + +/** + * Ratchet-style lint guard against Windows-test-parity regressions. + * + * PR #3649 cleared ~270 Windows-only test failures from the chunking fix + * in #3597 surfaced. Each cluster reduced to a handful of repeating + * patterns. This guard prevents the patterns from being re-introduced. + * + * Strategy: per-pattern offender list is snapshotted at the count present + * at the time of PR #3649. The test fails if a NEW file is added that + * matches the anti-pattern (count grows above the baseline). Existing + * offenders are acknowledged as technical debt that can be cleared + * incrementally without blocking this PR. + * + * When you fix an existing offender, lower the corresponding BASELINE + * count by 1. When CI breaks because BASELINE is set higher than the + * actual offender count, lower BASELINE to match (one-way ratchet down). + * + * Scope: tests/ only. Production-code Windows-compat is enforced via + * behavioural tests (see no-unconditional-win32-skip.test.cjs). + */ + +const { test, describe } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); + +const TESTS_DIR = path.join(__dirname); +const SELF = path.basename(__filename); + +// ── Baseline counts after PR #3649 batch ───────────────────────────────── +// Set these to the exact number of offending files at the time of merge. +// Each rule must not exceed its baseline; CI fails when a new offender appears. +// Decrement when an existing offender is fixed. +const BASELINE = { + splitNewlineOnFileContent: 3, + fenceRegexLiteralNewline: 2, + frontmatterAnchorLiteralNewline: 5, + hardcodedTmpToFsCall: 0, + bareNpmExecWithoutShell: 0, + stubsHomeNoUserProfile: 8, + rmSyncNoMaxRetries: 95, +}; + +function listTestFiles() { + return fs.readdirSync(TESTS_DIR) + .filter((f) => /\.(test|spec)\.cjs$/.test(f)) + .filter((f) => f !== SELF) + .map((f) => path.join(TESTS_DIR, f)); +} + +function readFileText(filePath) { + return fs.readFileSync(filePath, 'utf8'); +} + +// Strip line comments and block comments before pattern matching to avoid +// false-positives in commentary describing the very pattern we forbid. +function stripComments(text) { + return text + .replace(/\/\*[\s\S]*?\*\//g, '') + .replace(/(^|[^:])\/\/[^\n]*/g, '$1'); +} + +function countMatchingFiles(predicate) { + let count = 0; + const offenders = []; + for (const file of listTestFiles()) { + const text = stripComments(readFileText(file)); + if (predicate(text, file)) { + count += 1; + offenders.push(path.basename(file)); + } + } + return { count, offenders }; +} + +function ratchetAssert(rule, actualCount, baselineCount, offenders, guidance) { + if (actualCount > baselineCount) { + const newCount = actualCount - baselineCount; + assert.fail( + `Windows-parity guard "${rule}": ${actualCount} offenders, baseline is ${baselineCount} ` + + `(+${newCount} new). New occurrences of this anti-pattern were added. ` + + `${guidance}\n\nFull offender list (${actualCount}):\n ` + + offenders.join('\n '), + ); + } +} + +describe('Windows test-parity lint guards (ratchet baseline: PR #3649)', () => { + // ── G1 — CRLF: file-content split on literal '\n' ───────────────────── + test('split-on-newline after readFileSync (use /\\r?\\n/)', () => { + const { count, offenders } = countMatchingFiles((text) => { + return /\.readFileSync\s*\([^)]*\)[^;]*\.split\(\s*['"]\\n['"]\s*\)/.test(text); + }); + ratchetAssert( + 'splitNewlineOnFileContent', count, BASELINE.splitNewlineOnFileContent, offenders, + "Replace .split('\\n') with .split(/\\r?\\n/) so the test tolerates CRLF " + + "checkout (autocrlf=true on Windows leaves trailing \\r on every line).", + ); + }); + + // ── G2 — CRLF: ```bash|sh\n fence regex on file content ────────────── + test('markdown-fence regex with literal \\n after ```bash/sh', () => { + const { count, offenders } = countMatchingFiles((text) => { + return /\/[^/]*```(?:bash|sh)\\n[^/]*\//.test(text); + }); + ratchetAssert( + 'fenceRegexLiteralNewline', count, BASELINE.fenceRegexLiteralNewline, offenders, + "Use /```(?:bash|sh)\\r?\\n([\\s\\S]*?)```/g — Windows CRLF makes the byte after " + + "`bash` be \\r, the regex never matches, and bash-block extraction returns empty.", + ); + }); + + // ── G3 — CRLF: frontmatter regex with literal '\n' ──────────────────── + test('frontmatter regex anchors on /^---\\n/', () => { + const { count, offenders } = countMatchingFiles((text) => { + return /\/\^---\\n/.test(text); + }); + ratchetAssert( + 'frontmatterAnchorLiteralNewline', count, BASELINE.frontmatterAnchorLiteralNewline, offenders, + "Use /^---\\r?\\n/ — on Windows the byte after --- is \\r, not \\n, so the " + + "anchor fails to match and parseFrontmatter returns null/{}.", + ); + }); + + // ── G4 — POSIX-tmp: hardcoded '/tmp/' literal passed to fs.* ───────── + test('fs.* call receives a hardcoded "/tmp/..." literal', () => { + const { count, offenders } = countMatchingFiles((text) => { + return /\bfs\.[A-Za-z]+\s*\([^)]*['"]\/tmp\/[^'"]+['"][^)]*\)/.test(text); + }); + ratchetAssert( + 'hardcodedTmpToFsCall', count, BASELINE.hardcodedTmpToFsCall, offenders, + "Use os.tmpdir() — on Windows '/tmp/foo' becomes 'D:\\tmp\\foo' where D:\\tmp " + + "doesn't exist by default → ENOENT.", + ); + }); + + // ── G5 — npm.cmd: bare 'npm' to exec*Sync without shell:true ───────── + test('bare npm exec without shell-true Windows fallback', () => { + const { count, offenders } = countMatchingFiles((text) => { + const re = /\b(?:execFileSync|spawnSync)\s*\(\s*['"]npm['"]\s*,[^)]*\)/g; + const matches = text.match(re) || []; + return matches.some((m) => + !/shell\s*:\s*true/.test(m) && !/shell\s*:\s*isWindows/.test(m), + ); + }); + ratchetAssert( + 'bareNpmExecWithoutShell', count, BASELINE.bareNpmExecWithoutShell, offenders, + "On Windows npm is npm.cmd — pass {shell: process.platform === 'win32'} or " + + "use npm.cmd directly, otherwise execFileSync errors ENOENT.", + ); + }); + + // ── G6 — Test stubs HOME without USERPROFILE ───────────────────────── + test('test stubs process.env.HOME but never references USERPROFILE', () => { + const { count, offenders } = countMatchingFiles((text) => { + return /process\.env\.HOME\s*=\s*/.test(text) && !/USERPROFILE/.test(text); + }); + ratchetAssert( + 'stubsHomeNoUserProfile', count, BASELINE.stubsHomeNoUserProfile, offenders, + "On Windows os.homedir() reads USERPROFILE (not HOME). Tests redirecting ~ " + + "must override both, or the SUT sees the real user's home.", + ); + }); + + // ── G7 — rmSync cleanup without retry budget ───────────────────────── + test('test teardown rmSync without maxRetries', () => { + const { count, offenders } = countMatchingFiles((text) => { + const re = /fs\.rmSync\s*\([^)]*recursive\s*:\s*true[^)]*force\s*:\s*true[^)]*\)/g; + const matches = text.match(re) || []; + return matches.some((m) => !/maxRetries/.test(m)); + }); + ratchetAssert( + 'rmSyncNoMaxRetries', count, BASELINE.rmSyncNoMaxRetries, offenders, + "Use helpers.cleanup() (shared 5s retry budget) or pass " + + "{maxRetries: 10, retryDelay: 100} — Windows AV scanners can hold handles for " + + "seconds after a process exits, surfacing as flaky EBUSY teardown failures.", + ); + }); +});