Boiling the Windows CI Lake

.github/workflows/windows-smoke.yml

@@ -0,0 +1,88 @@
+# Windows Smoke CI — Phase 1 of the phased rollout in docs/designs/WINDOWS_CI.md
+#
+# Answers one question per run: "does the code path through a Windows-critical
+# module actually run on Windows." That's deliberately a lower bar than "does
+# every test pass" — it catches the class of bugs where Linux/macOS CI runs
+# green but a Windows user immediately hits ENOENT / "browse binary not found"
+# / silent mislocations of ~/.gstack/ state.
+#
+# Coverage catch list (see RFC for full reasoning):
+#   - Build fails to produce .exe on Windows              (catches #1013 / #1024)
+#   - Binary-resolution probes wrong filename             (catches #1118 / #1094)
+#   - Shebang bash script spawn fails                     (catches #1119)
+#   - Sensitive files written without ACL restriction     (catches #1121)
+#   - { mode: 0o600 } silently ignored on Windows         (catches Pre-#1121 state)
+#
+# Miss: #1120-style home-directory fallback — no direct unit test. RFC
+# proposes adding one as a follow-on.
+name: windows-smoke
+on:
+  pull_request:
+    branches: [main]
+    paths:
+      - 'browse/**'
+      - 'make-pdf/**'
+      - 'design/**'
+      - 'scripts/**'
+      - 'bin/**'
+      - 'package.json'
+      - 'bun.lockb'
+      - '.github/workflows/windows-smoke.yml'
+  push:
+    branches: [main]
+    paths:
+      - 'browse/**'
+      - 'make-pdf/**'
+      - 'design/**'
+      - 'scripts/**'
+      - 'bin/**'
+      - 'package.json'
+      - 'bun.lockb'
+  workflow_dispatch:
+
+concurrency:
+  group: windows-smoke-${{ github.head_ref || github.ref }}
+  cancel-in-progress: true
+
+jobs:
+  smoke:
+    runs-on: windows-latest
+    timeout-minutes: 10
+    steps:
+      - uses: actions/checkout@v4
+
+      - uses: oven-sh/setup-bun@v2
+        with:
+          bun-version: latest
+
+      - name: Install dependencies
+        run: bun install --frozen-lockfile
+
+      - name: Build binaries
+        run: bun run build
+
+      - name: Assert Windows binary layout
+        shell: pwsh
+        run: |
+          $missing = @()
+          foreach ($p in @(
+            'browse/dist/browse.exe',
+            'browse/dist/find-browse.exe',
+            'browse/dist/server-node.mjs',
+            'make-pdf/dist/pdf.exe',
+            'design/dist/design.exe'
+          )) { if (-not (Test-Path $p)) { $missing += $p } }
+          if ($missing.Count -gt 0) {
+            Write-Error "Missing build artifacts: $($missing -join ', ')"
+            exit 1
+          }
+
+
+      - name: Windows-specific unit tests
+        # Single bun test invocation with all files so a failure in any
+        # file correctly fails the step. Separate invocations + default
+        # PowerShell error-handling would mask all-but-the-last failure.
+        run: bun test browse/test/security.test.ts browse/test/file-permissions.test.ts browse/test/home-dir-resolution.test.ts make-pdf/test/browseClient.test.ts make-pdf/test/pdftotext.test.ts
+
+      - name: make-pdf render smoke
+        run: bun test make-pdf/test/render.test.ts

browse/test/home-dir-resolution.test.ts

@@ -0,0 +1,134 @@
+/**
+ * Regression test for PR #1120 — home-directory fallback.
+ *
+ * Background: before #1120, gstack source code constructed paths to
+ * `~/.gstack/` state using `path.join(process.env.HOME || '/tmp', ...)`.
+ * On Windows, `HOME` is unset by default (Windows uses `USERPROFILE`),
+ * so the fallback `'/tmp'` triggered — producing literal `\tmp\.gstack\...`
+ * paths that don't exist on disk. Any Windows user running gstack from
+ * cmd.exe, PowerShell, or an IDE subprocess without Git Bash's env
+ * inheritance hit this. #1120 replaced every occurrence with
+ * `os.homedir()`, which on Node reads `USERPROFILE` on Windows and
+ * `HOME` on POSIX.
+ *
+ * This test enforces the replacement is permanent. If a future change
+ * reintroduces the `process.env.HOME || '/tmp'` pattern (or its close
+ * variants) anywhere under `browse/src/` or `design/`, the test fails
+ * and surfaces the exact file and line.
+ *
+ * Also: tests that `os.homedir()` itself returns a real path with
+ * `HOME` unset — the contract the fix relies on.
+ */
+
+import { describe, test, expect } from 'bun:test';
+import * as fs from 'fs';
+import * as os from 'os';
+import * as path from 'path';
+
+// ─── os.homedir() contract ──────────────────────────────────
+
+describe('os.homedir() — the fix relies on this', () => {
+  test('returns a real path even when HOME is unset', () => {
+    const savedHome = process.env.HOME;
+    delete process.env.HOME;
+    try {
+      const h = os.homedir();
+      expect(h).toBeTruthy();
+      expect(h.length).toBeGreaterThan(0);
+      // Sanity-check: on Windows the path should start with a drive letter;
+      // on POSIX it should start with '/'.
+      if (process.platform === 'win32') {
+        expect(/^[A-Z]:\\/.test(h)).toBe(true);
+      } else {
+        expect(h.startsWith('/')).toBe(true);
+      }
+    } finally {
+      if (savedHome !== undefined) process.env.HOME = savedHome;
+    }
+  });
+});
+
+// ─── Static regression scan ─────────────────────────────────
+
+/**
+ * Recursively collect every `.ts` file under a given directory.
+ * Skips node_modules, dist, .git, and anything under a `.claude/` subdir.
+ */
+function tsFilesUnder(root: string): string[] {
+  const out: string[] = [];
+  if (!fs.existsSync(root)) return out;
+  const stack = [root];
+  while (stack.length) {
+    const dir = stack.pop()!;
+    for (const entry of fs.readdirSync(dir, { withFileTypes: true })) {
+      const p = path.join(dir, entry.name);
+      if (entry.isDirectory()) {
+        if (entry.name === 'node_modules' || entry.name === 'dist' ||
+            entry.name === '.git' || entry.name === '.claude') continue;
+        stack.push(p);
+      } else if (entry.isFile() && entry.name.endsWith('.ts')) {
+        out.push(p);
+      }
+    }
+  }
+  return out;
+}
+
+describe('home-directory resolution pattern (regression for #1120)', () => {
+  // Pattern we banned in #1120:
+  //    process.env.HOME || '/tmp'
+  //    process.env.HOME || ''
+  //    process.env.HOME || "~"
+  //    process.env.HOME!            (non-null assertion)
+  //    process.env.HOME || process.env.USERPROFILE || '/tmp'
+  // All of these evaluate wrong on Windows when HOME is unset.
+  const bannedPatterns: RegExp[] = [
+    /process\.env\.HOME\s*\|\|\s*['"]\/tmp['"]/,
+    /process\.env\.HOME\s*\|\|\s*['"]['"]/,
+    /process\.env\.HOME\s*\|\|\s*['"]~['"]/,
+    /process\.env\.HOME!/,
+    /process\.env\.HOME\s*\|\|\s*process\.env\.USERPROFILE/,
+  ];
+
+  test('no source file in browse/src or design/ reintroduces the banned fallback', () => {
+    // Resolve from the repo root. bun test runs from the repo root by default,
+    // but guard against the worktree layout just in case.
+    const cwd = process.cwd();
+    const roots = [
+      path.join(cwd, 'browse', 'src'),
+      path.join(cwd, 'design'),
+    ];
+
+    const offenders: { file: string; line: number; text: string; pattern: string }[] = [];
+    for (const root of roots) {
+      for (const file of tsFilesUnder(root)) {
+        // Skip this very test file — it embeds the banned patterns as regex literals.
+        if (file.endsWith('home-dir-resolution.test.ts')) continue;
+        const lines = fs.readFileSync(file, 'utf-8').split(/\r?\n/);
+        for (let i = 0; i < lines.length; i++) {
+          for (const pat of bannedPatterns) {
+            if (pat.test(lines[i])) {
+              offenders.push({
+                file: path.relative(cwd, file),
+                line: i + 1,
+                text: lines[i].trim(),
+                pattern: pat.source,
+              });
+            }
+          }
+        }
+      }
+    }
+
+    if (offenders.length > 0) {
+      const report = offenders
+        .map(o => `  ${o.file}:${o.line}  matches /${o.pattern}/  →  ${o.text}`)
+        .join('\n');
+      throw new Error(
+        `Found ${offenders.length} reintroduction(s) of the #1120 banned fallback ` +
+        `pattern. Use os.homedir() instead. Matches:\n${report}`,
+      );
+    }
+    expect(offenders).toEqual([]);
+  });
+});