chore(#1087): Windows test-portability lint + n/no-path-concat + LF normalization (#1088)

* chore(#1087): add Windows test-portability lint + DEFECT.WINDOWS-TEST-PORTABILITY Local gsd-test runs Mac+Linux only, so Windows-only test failures (Git Bash msys2 not honoring Node's chmod exec bit for PATH-executing extension-less scripts; `/` vs `\` path assertions) surface for the first time in CI's windows lanes — repeatedly (most recently PR #1084's #381 fix). Add scripts/lint-windows-test-portability.cjs: a high-signal, low-false- positive tripwire that flags any tests/**/*.test.cjs combining a chmod exec bit with a `sh -c`/`bash -c` invocation and no process.platform guard, unless annotated `// windows-portability-ok: <reason>`. Wired into lint:ci and runnable as `npm run lint:windows-test-portability`. Clean against all 721 current test files (zero pre-existing violations). Document the broader anti-pattern as DEFECT.WINDOWS-TEST-PORTABILITY in CONTEXT.md (.symptom/.examples/.detect/.fix-forward/.prevention): local gsd-test cannot substitute for the CI windows lane — watch it green before declaring a PR done. tests/lint-windows-test-portability.test.cjs covers the scanContent matrix. Closes #1087 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * chore(#1087): enable n/no-path-concat + add .gitattributes/.editorconfig (LF) Two cross-platform "free wins" complementing the Windows test-portability lint: - Enable eslint-plugin-n's n/no-path-concat (already-installed plugin) as 'error' — flags string path concatenation (the / vs \ separator class). Zero existing violations, so it's a clean ratchet, not a refactor. - Add .gitattributes (* text=auto eol=lf + binary exemptions) and .editorconfig (LF, UTF-8, final newline, trim whitespace) to normalize line endings and kill the CRLF-only-fails-on-Windows class at the source. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-11 22:04:56 -04:00
parent 7c07fce70f
commit 837991d5a2
7 changed files with 345 additions and 1 deletions
--- a/.editorconfig
+++ b/.editorconfig
@@ -0,0 +1,10 @@
+root = true
+
+[*]
+end_of_line = lf
+charset = utf-8
+insert_final_newline = true
+trim_trailing_whitespace = true
+
+[*.md]
+trim_trailing_whitespace = false
--- a/.gitattributes
+++ b/.gitattributes
@@ -0,0 +1,12 @@
+# Normalize line endings to LF on checkin; auto-detect binary (skipped).
+* text=auto eol=lf
+# Common binary types — never normalize.
+*.png binary
+*.jpg binary
+*.jpeg binary
+*.gif binary
+*.ico binary
+*.woff binary
+*.woff2 binary
+*.ttf binary
+*.pdf binary
--- a/CONTEXT.md
+++ b/CONTEXT.md
@@ -611,6 +611,12 @@ The canonical lint infrastructure adopted in ADR 452 (`docs/adr/452-eslint-lint-
 `DEFECT.TEST-SHELL-PIPELINE-NONPORTABLE.detect=test does readFileSync(md).match for a bash fence with literal \n, OR execFileSync('bash',...) gated only on a bash-presence probe; also verifying a new test with a file-scoped run instead of the full suite hides repo-wide static guards`
 `DEFECT.TEST-SHELL-PIPELINE-NONPORTABLE.fix-forward=match the fence with \r?\n and normalize the captured block to LF; gate pipeline execution on process.platform !== 'win32' && hasBash since the extraction LOGIC is platform-independent and POSIX coverage suffices; run the full suite (or the parity/lint guards) before push when adding a test file`

+`DEFECT.WINDOWS-TEST-PORTABILITY.symptom=local gsd-test runs Mac+Linux only (no Windows host); Windows-only test failures (chmod exec-bit not honored for PATH-executing extension-less scripts in Git Bash msys2; / vs \ path-separator in assertions; Git Bash msys2 shell semantics) surface ONLY in CI test (windows-latest,*) / full test (windows-latest,*) lanes, never locally`
+`DEFECT.WINDOWS-TEST-PORTABILITY.examples=PR #1084 (chmod 0o755 + bare-command execution failed on windows lane); test files that assert path.join result without normalizing to forward slashes`
+`DEFECT.WINDOWS-TEST-PORTABILITY.detect=npm run lint:windows-test-portability (tripwire: flags tests combining chmod exec-bit with sh/bash -c and no platform guard); watch CI windows matrix green before declaring a PR done`
+`DEFECT.WINDOWS-TEST-PORTABILITY.fix-forward=gate platform-specific execution with if (process.platform !== 'win32'); normalize path expectations to forward slashes with .replace(/\\/g, '/'); invoke scripts via explicit interpreter (sh <path>) rather than relying on exec-bit; annotate // windows-portability-ok: <reason> when a bypass is intentional`
+`DEFECT.WINDOWS-TEST-PORTABILITY.prevention=run lint:ci before opening a PR; treat the CI windows lane as the only true Windows signal — gsd-test (Mac/Linux only) cannot substitute for it`
+

 ---

--- a/eslint.config.mjs
+++ b/eslint.config.mjs
@@ -196,6 +196,7 @@ export default tseslint.config(
      'no-unsafe-finally': 'warn',
      // eslint-plugin-n rules
      'n/no-process-exit': 'error',
+      'n/no-path-concat': 'error',
      // Local rules — warn for now; flip to error after cleanup phases
      'local/no-source-grep': 'warn',
    },
--- a/package.json
+++ b/package.json
@@ -92,7 +92,8 @@
    "pretest:coverage": "npm run build:lib && npm run lint:skill-deps",
    "lint": "eslint . --cache --cache-location node_modules/.cache/eslint/",
    "lint:fix": "eslint . --fix",
-    "lint:ci": "npm run lint && npm run lint:skill-deps && node scripts/lint-test-file-count.cjs && node scripts/lint-command-contract.cjs && node scripts/lint-pr-check-project-dir.cjs && npm run lint:legacy-name && node scripts/lint-regression-test-names.cjs",
+    "lint:ci": "npm run lint && npm run lint:skill-deps && node scripts/lint-test-file-count.cjs && node scripts/lint-command-contract.cjs && node scripts/lint-pr-check-project-dir.cjs && npm run lint:legacy-name && node scripts/lint-regression-test-names.cjs && node scripts/lint-windows-test-portability.cjs",
+    "lint:windows-test-portability": "node scripts/lint-windows-test-portability.cjs",
    "lint:regression-names": "node scripts/lint-regression-test-names.cjs",
    "lint:descriptions": "node scripts/lint-descriptions.cjs",
    "lint:skill-deps": "node scripts/lint-skill-deps.cjs",
--- a/scripts/lint-windows-test-portability.cjs
+++ b/scripts/lint-windows-test-portability.cjs
@@ -0,0 +1,178 @@
+'use strict';
+
+/**
+ * lint-windows-test-portability.cjs — flag tests that combine chmod exec-bit
+ * with bare sh/bash -c without a platform guard.
+ *
+ * ## Why
+ *
+ * Windows Git Bash (msys2) does not honour Node's chmod exec bit for
+ * PATH-executing extension-less scripts. A test that (a) makes a fixture
+ * executable via chmodSync and (b) runs it with `sh -c`/`bash -c` will pass
+ * on Mac/Linux but fail only in the CI `test (windows-latest, *)` /
+ * `full test (windows-latest, *)` lanes, producing a hard-to-diagnose
+ * false-negative gate. See CONTEXT.md → DEFECT.WINDOWS-TEST-PORTABILITY.
+ *
+ * ## What this enforces
+ *
+ * For every file in tests/**\/*.test.cjs (recursive, excluding node_modules):
+ *   - makesExecutable: contains chmodSync?( with an exec-bit octal literal
+ *   - shellDashC: contains a sh/bash -c invocation (array form or string literal)
+ *   - guarded: contains a process.platform / os.platform() / win32 / isWindows guard
+ *   - optOut: contains the literal `windows-portability-ok`
+ *   VIOLATION = makesExecutable && shellDashC && !guarded && !optOut
+ *
+ * ## Remediation
+ *
+ * Gate the bare-command execution with `if (process.platform !== 'win32')`,
+ * or invoke via an explicit interpreter (`sh <path>`), or annotate
+ * `// windows-portability-ok: <reason>`.
+ *
+ * ## Export contract (for unit tests)
+ *
+ * When required as a module (`require.main !== module`) this file exports
+ * `scanContent(source)` → { makesExecutable, shellDashC, guarded, optOut, violation }.
+ */
+
+const fs = require('fs');
+const path = require('path');
+
+// ─── Detection regexes ──────────────────────────────────────────────────────
+
+/**
+ * Match chmod/chmodSync( calls with an octal mode literal whose exec bits are
+ * set, e.g. `fs.chmodSync(p, 0o755)` or `chmod(file, 0o111)`.
+ */
+const CHMOD_RE = /chmod(?:Sync)?\s*\([^,;]+,\s*0o([0-7]{3})\b/g;
+
+/**
+ * Array form: execFileSync/spawnSync/exec* with 'sh' or 'bash' (optionally
+ * prefixed) as the first arg and '-c' as an element of the args array.
+ * e.g. execFileSync('bash', ['-c', ...]) or spawnSync('/bin/sh', ['-c', ...])
+ */
+const SHELL_ARRAY_RE =
+  /(?:execFile(?:Sync)?|spawnSync|spawn|exec)\s*\(\s*['"`](?:\/(?:usr\/)?bin\/)?(?:bash|sh)['"`]\s*,\s*\[[^\]]*['"]-c['"]/;
+
+/**
+ * String-literal form: any string containing `bash -c` or `sh -c`.
+ */
+const SHELL_STRING_RE = /['"`][^'"`\n]*(?:bash|sh)\s+-c[^'"`\n]*['"`]/;
+
+/** Platform guard presence. */
+const GUARD_RE = /process\.platform|os\.platform\s*\(|\bwin32\b|\bisWindows\b/;
+
+/** Opt-out annotation. */
+const OPT_OUT_RE = /windows-portability-ok/;
+
+// ─── Pure scanning function (exported for unit tests) ────────────────────────
+
+/**
+ * Scan a single file's source text and return detection flags.
+ *
+ * @param {string} source - The file contents as a string.
+ * @returns {{ makesExecutable: boolean, shellDashC: boolean, guarded: boolean, optOut: boolean, violation: boolean }}
+ */
+function scanContent(source) {
+  // Reset stateful regex before use.
+  CHMOD_RE.lastIndex = 0;
+
+  let makesExecutable = false;
+  let match;
+  while ((match = CHMOD_RE.exec(source)) !== null) {
+    const oct = match[1];
+    if ((parseInt(oct, 8) & 0o111) !== 0) {
+      makesExecutable = true;
+      break;
+    }
+  }
+
+  const shellDashC = SHELL_ARRAY_RE.test(source) || SHELL_STRING_RE.test(source);
+  const guarded = GUARD_RE.test(source);
+  const optOut = OPT_OUT_RE.test(source);
+  const violation = makesExecutable && shellDashC && !guarded && !optOut;
+
+  return { makesExecutable, shellDashC, guarded, optOut, violation };
+}
+
+// ─── Filesystem walker ───────────────────────────────────────────────────────
+
+/**
+ * Recursively collect all *.test.cjs files under `dir`, excluding node_modules.
+ *
+ * @param {string} dir
+ * @param {string[]} [acc]
+ * @returns {string[]}
+ */
+function collectTestFiles(dir, acc) {
+  acc = acc || [];
+  let entries;
+  try {
+    entries = fs.readdirSync(dir, { withFileTypes: true });
+  } catch {
+    return acc;
+  }
+  for (const entry of entries) {
+    if (entry.name === 'node_modules') continue;
+    const full = path.join(dir, entry.name);
+    if (entry.isDirectory()) {
+      collectTestFiles(full, acc);
+    } else if (entry.isFile() && entry.name.endsWith('.test.cjs')) {
+      acc.push(full);
+    }
+  }
+  return acc;
+}
+
+// ─── Main ────────────────────────────────────────────────────────────────────
+
+function main() {
+  const ROOT = path.join(__dirname, '..');
+  const TESTS_DIR = path.join(ROOT, 'tests');
+
+  const files = collectTestFiles(TESTS_DIR);
+  const violations = [];
+
+  for (const file of files) {
+    let source;
+    try {
+      source = fs.readFileSync(file, 'utf8');
+    } catch {
+      continue;
+    }
+    const { violation } = scanContent(source);
+    if (violation) {
+      const rel = path.relative(ROOT, file).replace(/\\/g, '/');
+      violations.push(rel);
+    }
+  }
+
+  if (violations.length > 0) {
+    for (const rel of violations) {
+      process.stderr.write(
+        `${rel}: chmod-executable + sh/bash -c with no platform guard\n`,
+      );
+    }
+    process.stderr.write(
+      '\nWindows Git Bash does not honor Node\'s chmod exec bit for ' +
+        'PATH-executing extension-less scripts ' +
+        '(CONTEXT.md → DEFECT.WINDOWS-TEST-PORTABILITY). ' +
+        'Gate the bare-command execution with ' +
+        '`if (process.platform !== \'win32\')`, or invoke via an explicit ' +
+        'interpreter (`sh <path>`), or annotate ' +
+        '`// windows-portability-ok: <reason>`.\n',
+    );
+    process.exitCode = 1;
+  } else {
+    console.log(
+      `ok lint-windows-test-portability: ${files.length} file(s) scanned, no violations`,
+    );
+  }
+}
+
+// ─── Module boundary ─────────────────────────────────────────────────────────
+
+if (require.main === module) {
+  main();
+} else {
+  module.exports = { scanContent };
+}
--- a/tests/lint-windows-test-portability.test.cjs
+++ b/tests/lint-windows-test-portability.test.cjs
@@ -0,0 +1,136 @@
+// windows-portability-ok: fixture strings for the lint's own unit test, not real execution
+'use strict';
+
+/**
+ * Tests for scripts/lint-windows-test-portability.cjs
+ *
+ * Uses the exported `scanContent` pure function to avoid spawning real
+ * subprocesses or touching the filesystem. This keeps the test portable and
+ * prevents the lint from flagging itself (the opt-out comment above covers the
+ * chmod/bash-c fixture strings below).
+ */
+
+const { describe, test } = require('node:test');
+const assert = require('node:assert/strict');
+const { scanContent } = require('../scripts/lint-windows-test-portability.cjs');
+
+describe('lint-windows-test-portability: scanContent', () => {
+  test('(a) chmod 0o755 + bash -c with no guard => violation', () => {
+    const src = `
+      'use strict';
+      fs.chmodSync(fixture, 0o755);
+      execFileSync('bash', ['-c', 'echo hi']);
+    `;
+    const result = scanContent(src);
+    assert.strictEqual(result.makesExecutable, true, 'makesExecutable');
+    assert.strictEqual(result.shellDashC, true, 'shellDashC');
+    assert.strictEqual(result.guarded, false, 'guarded');
+    assert.strictEqual(result.optOut, false, 'optOut');
+    assert.strictEqual(result.violation, true, 'violation');
+  });
+
+  test('(b) chmod 0o755 + bash -c + process.platform guard => no violation', () => {
+    const src = `
+      'use strict';
+      fs.chmodSync(fixture, 0o755);
+      execFileSync('bash', ['-c', 'echo hi']);
+      if (process.platform !== 'win32') { runIt(); }
+    `;
+    const result = scanContent(src);
+    assert.strictEqual(result.makesExecutable, true, 'makesExecutable');
+    assert.strictEqual(result.shellDashC, true, 'shellDashC');
+    assert.strictEqual(result.guarded, true, 'guarded');
+    assert.strictEqual(result.violation, false, 'violation');
+  });
+
+  test('(c) chmod 0o644 (no exec bit) + bash -c => no violation', () => {
+    const src = `
+      'use strict';
+      fs.chmodSync(fixture, 0o644);
+      execFileSync('bash', ['-c', 'cat file']);
+    `;
+    const result = scanContent(src);
+    assert.strictEqual(result.makesExecutable, false, 'makesExecutable');
+    assert.strictEqual(result.shellDashC, true, 'shellDashC');
+    assert.strictEqual(result.violation, false, 'violation');
+  });
+
+  test('(d) chmod 0o755 + execFileSync(sh, [path]) with no -c => no violation', () => {
+    const src = `
+      'use strict';
+      fs.chmodSync(fixture, 0o755);
+      execFileSync('sh', [fixturePath]);
+    `;
+    const result = scanContent(src);
+    assert.strictEqual(result.makesExecutable, true, 'makesExecutable');
+    assert.strictEqual(result.shellDashC, false, 'shellDashC');
+    assert.strictEqual(result.violation, false, 'violation');
+  });
+
+  test('(e) violation pattern + windows-portability-ok opt-out => no violation', () => {
+    const src = `
+      // windows-portability-ok: intentional cross-platform test
+      'use strict';
+      fs.chmodSync(fixture, 0o755);
+      execFileSync('bash', ['-c', 'run']);
+    `;
+    const result = scanContent(src);
+    assert.strictEqual(result.makesExecutable, true, 'makesExecutable');
+    assert.strictEqual(result.shellDashC, true, 'shellDashC');
+    assert.strictEqual(result.optOut, true, 'optOut');
+    assert.strictEqual(result.violation, false, 'violation');
+  });
+
+  test('chmod 0o111 (pure exec bits) is detected as executable', () => {
+    const src = `fs.chmodSync(f, 0o111); spawnSync('sh', ['-c', 'x']);`;
+    const result = scanContent(src);
+    assert.strictEqual(result.makesExecutable, true, 'makesExecutable');
+    assert.strictEqual(result.shellDashC, true, 'shellDashC');
+    assert.strictEqual(result.violation, true, 'violation');
+  });
+
+  test('chmod 0o444 (read-only) is not executable', () => {
+    const src = `fs.chmodSync(f, 0o444); execFileSync('bash', ['-c', 'x']);`;
+    const result = scanContent(src);
+    assert.strictEqual(result.makesExecutable, false, 'makesExecutable');
+    assert.strictEqual(result.violation, false, 'violation');
+  });
+
+  test('string-literal sh -c form is detected', () => {
+    const src = `
+      fs.chmodSync(f, 0o755);
+      exec('sh -c "run.sh"');
+    `;
+    const result = scanContent(src);
+    assert.strictEqual(result.shellDashC, true, 'shellDashC from string literal');
+    assert.strictEqual(result.violation, true, 'violation');
+  });
+
+  test('/bin/bash prefix in array form is detected', () => {
+    const src = `
+      fs.chmodSync(f, 0o755);
+      execFileSync('/bin/bash', ['-c', 'run']);
+    `;
+    const result = scanContent(src);
+    assert.strictEqual(result.shellDashC, true, 'shellDashC with /bin/bash prefix');
+    assert.strictEqual(result.violation, true, 'violation');
+  });
+
+  test('isWindows guard suppresses violation', () => {
+    const src = `
+      const isWindows = process.platform === 'win32';
+      fs.chmodSync(f, 0o755);
+      execFileSync('bash', ['-c', 'run']);
+    `;
+    const result = scanContent(src);
+    assert.strictEqual(result.guarded, true, 'guarded via isWindows');
+    assert.strictEqual(result.violation, false, 'violation');
+  });
+
+  test('no chmod at all => no violation regardless of shell -c', () => {
+    const src = `execFileSync('bash', ['-c', 'echo hi']);`;
+    const result = scanContent(src);
+    assert.strictEqual(result.makesExecutable, false, 'makesExecutable');
+    assert.strictEqual(result.violation, false, 'violation');
+  });
+});