Skip to content

Commit 5824f5e

Browse files
committed
docs(rfc): sync workflow fix + add home-dir-resolution regression test
1 parent 55b9c7c commit 5824f5e

2 files changed

Lines changed: 137 additions & 1 deletion

File tree

.github/workflows/windows-smoke.yml

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,15 +78,17 @@ jobs:
7878
}
7979
8080
- 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.
8183
run: |
8284
./browse/dist/browse.exe --version
8385
./make-pdf/dist/pdf.exe --version
84-
./design/dist/design.exe --version
8586
8687
- name: Windows-specific unit tests
8788
run: |
8889
bun test browse/test/security.test.ts
8990
bun test browse/test/file-permissions.test.ts
91+
bun test browse/test/home-dir-resolution.test.ts
9092
bun test make-pdf/test/browseClient.test.ts
9193
bun test make-pdf/test/pdftotext.test.ts
9294
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)