Skip to content

Commit a2c1417

Browse files
scarsonscarson
committed
demo: windows-smoke catches garrytan#1120 with home-dir regression test on pre-fix source
1 parent e23ff28 commit a2c1417

2 files changed

Lines changed: 230 additions & 0 deletions

File tree

‎.github/workflows/windows-smoke.yml‎

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
# Windows Smoke CI — Phase 1 of the phased rollout in docs/designs/WINDOWS_CI.md
2+
#
3+
# Answers one question per run: "does the code path through a Windows-critical
4+
# module actually run on Windows." That's deliberately a lower bar than "does
5+
# every test pass" — it catches the class of bugs where Linux/macOS CI runs
6+
# green but a Windows user immediately hits ENOENT / "browse binary not found"
7+
# / silent mislocations of ~/.gstack/ state.
8+
#
9+
# Coverage catch list (see RFC for full reasoning):
10+
# - Build fails to produce .exe on Windows (catches #1013 / #1024)
11+
# - Binary-resolution probes wrong filename (catches #1118 / #1094)
12+
# - Shebang bash script spawn fails (catches #1119)
13+
# - Sensitive files written without ACL restriction (catches #1121)
14+
# - { mode: 0o600 } silently ignored on Windows (catches Pre-#1121 state)
15+
#
16+
# Miss: #1120-style home-directory fallback — no direct unit test. RFC
17+
# proposes adding one as a follow-on.
18+
name: windows-smoke
19+
on:
20+
pull_request:
21+
branches: [main]
22+
paths:
23+
- 'browse/**'
24+
- 'make-pdf/**'
25+
- 'design/**'
26+
- 'scripts/**'
27+
- 'bin/**'
28+
- 'package.json'
29+
- 'bun.lockb'
30+
- '.github/workflows/windows-smoke.yml'
31+
push:
32+
branches: [main]
33+
paths:
34+
- 'browse/**'
35+
- 'make-pdf/**'
36+
- 'design/**'
37+
- 'scripts/**'
38+
- 'bin/**'
39+
- 'package.json'
40+
- 'bun.lockb'
41+
workflow_dispatch:
42+
43+
concurrency:
44+
group: windows-smoke-${{ github.head_ref || github.ref }}
45+
cancel-in-progress: true
46+
47+
jobs:
48+
smoke:
49+
runs-on: windows-latest
50+
timeout-minutes: 10
51+
steps:
52+
- uses: actions/checkout@v4
53+
54+
- uses: oven-sh/setup-bun@v2
55+
with:
56+
bun-version: latest
57+
58+
- name: Install dependencies
59+
run: bun install --frozen-lockfile
60+
61+
- name: Build binaries
62+
run: bun run build
63+
64+
- name: Assert Windows binary layout
65+
shell: pwsh
66+
run: |
67+
$missing = @()
68+
foreach ($p in @(
69+
'browse/dist/browse.exe',
70+
'browse/dist/find-browse.exe',
71+
'browse/dist/server-node.mjs',
72+
'make-pdf/dist/pdf.exe',
73+
'design/dist/design.exe'
74+
)) { if (-not (Test-Path $p)) { $missing += $p } }
75+
if ($missing.Count -gt 0) {
76+
Write-Error "Missing build artifacts: $($missing -join ', ')"
77+
exit 1
78+
}
79+
80+
- name: Smoke-test binaries
81+
# design.exe's build is smoked by the layout assertion above; it
82+
# doesn't implement --version, so we don't invoke it here.
83+
run: |
84+
./browse/dist/browse.exe --version
85+
./make-pdf/dist/pdf.exe --version
86+
87+
- name: Windows-specific unit tests
88+
run: |
89+
bun test browse/test/security.test.ts
90+
bun test browse/test/file-permissions.test.ts
91+
bun test browse/test/home-dir-resolution.test.ts
92+
bun test make-pdf/test/browseClient.test.ts
93+
bun test make-pdf/test/pdftotext.test.ts
94+
95+
- name: make-pdf render smoke
96+
run: bun test make-pdf/test/render.test.ts

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

Lines changed: 134 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,134 @@
1+
/**
2+
* Regression test for PR #1120 — home-directory fallback.
3+
*
4+
* Background: before #1120, gstack source code constructed paths to
5+
* `~/.gstack/` state using `path.join(process.env.HOME || '/tmp', ...)`.
6+
* On Windows, `HOME` is unset by default (Windows uses `USERPROFILE`),
7+
* so the fallback `'/tmp'` triggered — producing literal `\tmp\.gstack\...`
8+
* paths that don't exist on disk. Any Windows user running gstack from
9+
* cmd.exe, PowerShell, or an IDE subprocess without Git Bash's env
10+
* inheritance hit this. #1120 replaced every occurrence with
11+
* `os.homedir()`, which on Node reads `USERPROFILE` on Windows and
12+
* `HOME` on POSIX.
13+
*
14+
* This test enforces the replacement is permanent. If a future change
15+
* reintroduces the `process.env.HOME || '/tmp'` pattern (or its close
16+
* variants) anywhere under `browse/src/` or `design/`, the test fails
17+
* and surfaces the exact file and line.
18+
*
19+
* Also: tests that `os.homedir()` itself returns a real path with
20+
* `HOME` unset — the contract the fix relies on.
21+
*/
22+
23+
import { describe, test, expect } from 'bun:test';
24+
import * as fs from 'fs';
25+
import * as os from 'os';
26+
import * as path from 'path';
27+
28+
// ─── os.homedir() contract ──────────────────────────────────
29+
30+
describe('os.homedir() — the fix relies on this', () => {
31+
test('returns a real path even when HOME is unset', () => {
32+
const savedHome = process.env.HOME;
33+
delete process.env.HOME;
34+
try {
35+
const h = os.homedir();
36+
expect(h).toBeTruthy();
37+
expect(h.length).toBeGreaterThan(0);
38+
// Sanity-check: on Windows the path should start with a drive letter;
39+
// on POSIX it should start with '/'.
40+
if (process.platform === 'win32') {
41+
expect(/^[A-Z]:\\/.test(h)).toBe(true);
42+
} else {
43+
expect(h.startsWith('/')).toBe(true);
44+
}
45+
} finally {
46+
if (savedHome !== undefined) process.env.HOME = savedHome;
47+
}
48+
});
49+
});
50+
51+
// ─── Static regression scan ─────────────────────────────────
52+
53+
/**
54+
* Recursively collect every `.ts` file under a given directory.
55+
* Skips node_modules, dist, .git, and anything under a `.claude/` subdir.
56+
*/
57+
function tsFilesUnder(root: string): string[] {
58+
const out: string[] = [];
59+
if (!fs.existsSync(root)) return out;
60+
const stack = [root];
61+
while (stack.length) {
62+
const dir = stack.pop()!;
63+
for (const entry of fs.readdirSync(dir, { withFileTypes: true })) {
64+
const p = path.join(dir, entry.name);
65+
if (entry.isDirectory()) {
66+
if (entry.name === 'node_modules' || entry.name === 'dist' ||
67+
entry.name === '.git' || entry.name === '.claude') continue;
68+
stack.push(p);
69+
} else if (entry.isFile() && entry.name.endsWith('.ts')) {
70+
out.push(p);
71+
}
72+
}
73+
}
74+
return out;
75+
}
76+
77+
describe('home-directory resolution pattern (regression for #1120)', () => {
78+
// Pattern we banned in #1120:
79+
// process.env.HOME || '/tmp'
80+
// process.env.HOME || ''
81+
// process.env.HOME || "~"
82+
// process.env.HOME! (non-null assertion)
83+
// process.env.HOME || process.env.USERPROFILE || '/tmp'
84+
// All of these evaluate wrong on Windows when HOME is unset.
85+
const bannedPatterns: RegExp[] = [
86+
/process\.env\.HOME\s*\|\|\s*['"]\/tmp['"]/,
87+
/process\.env\.HOME\s*\|\|\s*['"]['"]/,
88+
/process\.env\.HOME\s*\|\|\s*['"]~['"]/,
89+
/process\.env\.HOME!/,
90+
/process\.env\.HOME\s*\|\|\s*process\.env\.USERPROFILE/,
91+
];
92+
93+
test('no source file in browse/src or design/ reintroduces the banned fallback', () => {
94+
// Resolve from the repo root. bun test runs from the repo root by default,
95+
// but guard against the worktree layout just in case.
96+
const cwd = process.cwd();
97+
const roots = [
98+
path.join(cwd, 'browse', 'src'),
99+
path.join(cwd, 'design'),
100+
];
101+
102+
const offenders: { file: string; line: number; text: string; pattern: string }[] = [];
103+
for (const root of roots) {
104+
for (const file of tsFilesUnder(root)) {
105+
// Skip this very test file — it embeds the banned patterns as regex literals.
106+
if (file.endsWith('home-dir-resolution.test.ts')) continue;
107+
const lines = fs.readFileSync(file, 'utf-8').split(/\r?\n/);
108+
for (let i = 0; i < lines.length; i++) {
109+
for (const pat of bannedPatterns) {
110+
if (pat.test(lines[i])) {
111+
offenders.push({
112+
file: path.relative(cwd, file),
113+
line: i + 1,
114+
text: lines[i].trim(),
115+
pattern: pat.source,
116+
});
117+
}
118+
}
119+
}
120+
}
121+
}
122+
123+
if (offenders.length > 0) {
124+
const report = offenders
125+
.map(o => ` ${o.file}:${o.line} matches /${o.pattern}/ → ${o.text}`)
126+
.join('\n');
127+
throw new Error(
128+
`Found ${offenders.length} reintroduction(s) of the #1120 banned fallback ` +
129+
`pattern. Use os.homedir() instead. Matches:\n${report}`,
130+
);
131+
}
132+
expect(offenders).toEqual([]);
133+
});
134+
});

0 commit comments

Comments
 (0)