* fix(#685): set windowsHide on all Windows child-process spawns A visible "gsd-core" console window flashed on Windows whenever a gsd-core child process spawned without `windowsHide: true`. The most visible offenders fire on every SessionStart / `/clear` (execNpm's `shell:true` npm view via the update-check worker) and on every Edit/Write/MultiEdit in a worktree (the worktree-path guard's git probe). Add `windowsHide: true` to every external-binary spawn in the runtime source: - hooks/gsd-context-monitor.js (record-session spawn) - hooks/gsd-worktree-path-guard.js (SPAWNOPT) - hooks/gsd-workflow-guard.js (git branch --show-current) - src/shell-command-projection.cts (execGit / execNpm / execTool) - src/check-command-router.cts (git log execFileSync) - src/roadmap-upgrade.cts (git status/rev-parse/reset/clean execSync) gsd-check-update.js already had it (the precedent). probeTty's tty call is POSIX-only and intentionally untouched. Adds a regression test that asserts each site plus a repo-wide completeness guard so a future external-binary spawn that omits windowsHide fails CI. No behavior change off-Windows. Closes #685 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * chore(#685): set changeset pr number to 688 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
5
.changeset/685-windowshide-spawn.md
Normal file
5
.changeset/685-windowshide-spawn.md
Normal file
@@ -0,0 +1,5 @@
|
||||
---
|
||||
type: Fixed
|
||||
pr: 688
|
||||
---
|
||||
**No more "gsd-core" console-window flash on Windows.** Every gsd-core child process now passes `windowsHide: true`: the context monitor's `record-session` spawn, the `execGit` / `execNpm` / `execTool` helpers in `shell-command-projection`, the `gsd-worktree-path-guard` and `gsd-workflow-guard` hook git probes, `check-command-router`'s `git log` call, and the `roadmap-upgrade` git status/rev-parse/reset/clean calls — matching the existing `gsd-check-update` spawn. `execNpm` (which uses `shell: true` → `cmd.exe` and runs on every SessionStart, i.e. every `/clear`) and the worktree-path guard (which runs on every Edit/Write in a worktree) were the most visible offenders. No behavior change on macOS/Linux, where the flag is ignored.
|
||||
@@ -150,7 +150,7 @@ process.stdin.on('end', () => {
|
||||
spawn(
|
||||
process.execPath,
|
||||
[gsdTools, 'state', 'record-session', '--stopped-at', stoppedAt],
|
||||
{ cwd, detached: true, stdio: 'ignore' }
|
||||
{ cwd, detached: true, stdio: 'ignore', windowsHide: true }
|
||||
).unref();
|
||||
warnData.criticalRecorded = true;
|
||||
// Persist the sentinel so subsequent debounce cycles don't re-fire
|
||||
|
||||
@@ -61,6 +61,7 @@ function currentBranch(cwd) {
|
||||
cwd,
|
||||
encoding: 'utf8',
|
||||
stdio: ['ignore', 'pipe', 'ignore'],
|
||||
windowsHide: true,
|
||||
});
|
||||
if (result.status !== 0) return '';
|
||||
return result.stdout.trim();
|
||||
|
||||
@@ -18,7 +18,7 @@ const fs = require('fs');
|
||||
const path = require('path');
|
||||
const { spawnSync } = require('child_process');
|
||||
|
||||
const SPAWNOPT = { encoding: 'utf8', stdio: ['ignore', 'pipe', 'ignore'], timeout: 2000 };
|
||||
const SPAWNOPT = { encoding: 'utf8', stdio: ['ignore', 'pipe', 'ignore'], timeout: 2000, windowsHide: true };
|
||||
|
||||
function git(args, cwd) {
|
||||
return spawnSync('git', args, { ...SPAWNOPT, cwd });
|
||||
|
||||
@@ -253,6 +253,7 @@ function recentCommitMessages(projectDir: string): string {
|
||||
cwd: projectDir,
|
||||
encoding: 'utf-8',
|
||||
maxBuffer: 4 * 1024 * 1024,
|
||||
windowsHide: true,
|
||||
});
|
||||
} catch {
|
||||
return '';
|
||||
|
||||
@@ -497,7 +497,7 @@ function applyMigration(cwd: string, plan: MigrationPlan, options: { dryRun?: bo
|
||||
// ── Real run: verify clean working tree ───────────────────────────────────
|
||||
let gitStatus: string;
|
||||
try {
|
||||
gitStatus = execSync('git status --porcelain', { cwd, encoding: 'utf8' });
|
||||
gitStatus = execSync('git status --porcelain', { cwd, encoding: 'utf8', windowsHide: true });
|
||||
} catch (err) {
|
||||
throw new Error(`git status failed: ${(err as Error).message}`);
|
||||
}
|
||||
@@ -508,7 +508,7 @@ function applyMigration(cwd: string, plan: MigrationPlan, options: { dryRun?: bo
|
||||
// Capture HEAD sha for rollback
|
||||
let headSha: string;
|
||||
try {
|
||||
headSha = execSync('git rev-parse HEAD', { cwd, encoding: 'utf8' }).trim();
|
||||
headSha = execSync('git rev-parse HEAD', { cwd, encoding: 'utf8', windowsHide: true }).trim();
|
||||
} catch (err) {
|
||||
throw new Error(`git rev-parse HEAD failed: ${(err as Error).message}`);
|
||||
}
|
||||
@@ -592,8 +592,8 @@ function applyMigration(cwd: string, plan: MigrationPlan, options: { dryRun?: bo
|
||||
} catch (err) {
|
||||
// Rollback via git reset --hard + git clean
|
||||
try {
|
||||
execSync(`git reset --hard ${headSha}`, { cwd, stdio: 'pipe' });
|
||||
execSync('git clean -fd .planning/phases/', { cwd, stdio: 'pipe' });
|
||||
execSync(`git reset --hard ${headSha}`, { cwd, stdio: 'pipe', windowsHide: true });
|
||||
execSync('git clean -fd .planning/phases/', { cwd, stdio: 'pipe', windowsHide: true });
|
||||
} catch {
|
||||
// Swallow rollback errors — surface original error
|
||||
}
|
||||
|
||||
@@ -427,6 +427,7 @@ export function execGit(args: string[], opts: { cwd?: string; env?: Record<strin
|
||||
encoding: 'utf-8',
|
||||
stdio: 'pipe',
|
||||
timeout: opts.timeout ?? 10_000,
|
||||
windowsHide: true,
|
||||
});
|
||||
return _spawnResult(result, 'git');
|
||||
}
|
||||
@@ -438,6 +439,7 @@ export function execNpm(args: string[], opts: { cwd?: string; timeout?: number }
|
||||
encoding: 'utf-8',
|
||||
stdio: ['ignore', 'pipe', 'pipe'],
|
||||
timeout: opts.timeout ?? 15_000,
|
||||
windowsHide: true,
|
||||
});
|
||||
return _spawnResult(result, 'npm');
|
||||
}
|
||||
@@ -449,6 +451,7 @@ export function execTool(program: string, args: string[], opts: { cwd?: string;
|
||||
encoding: 'utf-8',
|
||||
stdio: 'pipe',
|
||||
timeout: opts.timeout ?? 30_000,
|
||||
windowsHide: true,
|
||||
});
|
||||
return _spawnResult(result, program);
|
||||
}
|
||||
|
||||
99
tests/bug-685-windowshide-spawn.test.cjs
Normal file
99
tests/bug-685-windowshide-spawn.test.cjs
Normal file
@@ -0,0 +1,99 @@
|
||||
// allow-test-rule: source-text-is-the-product
|
||||
// These spawn/exec sites cannot be behaviourally tested for windowsHide
|
||||
// off-Windows; the source text is the runtime contract (issue #685). Without
|
||||
// windowsHide:true a detached or shell:true child allocates a visible console
|
||||
// window on Windows (the "gsd-core" flash).
|
||||
'use strict';
|
||||
|
||||
const { describe, test } = require('node:test');
|
||||
const assert = require('node:assert/strict');
|
||||
const fs = require('node:fs');
|
||||
const path = require('node:path');
|
||||
|
||||
const root = path.resolve(__dirname, '..');
|
||||
const read = (p) => fs.readFileSync(path.join(root, p), 'utf-8');
|
||||
|
||||
// Slice the exact body of one spawn site so the assertion binds that site,
|
||||
// not merely "windowsHide appears somewhere in the file".
|
||||
function regionBetween(src, startAnchor, endAnchor) {
|
||||
const i = src.indexOf(startAnchor);
|
||||
assert.notEqual(i, -1, `start anchor not found: ${startAnchor}`);
|
||||
const j = src.indexOf(endAnchor, i);
|
||||
assert.notEqual(j, -1, `end anchor not found after start: ${endAnchor}`);
|
||||
return src.slice(i, j);
|
||||
}
|
||||
|
||||
describe('bug #685: Windows spawns must set windowsHide:true (no console-window flash)', () => {
|
||||
test('gsd-context-monitor record-session spawn sets windowsHide', () => {
|
||||
const region = regionBetween(read('hooks/gsd-context-monitor.js'), "'record-session'", '.unref()');
|
||||
assert.match(region, /windowsHide:\s*true/, 'record-session spawn must set windowsHide: true');
|
||||
});
|
||||
|
||||
const cts = () => read('src/shell-command-projection.cts');
|
||||
const helpers = [
|
||||
['execGit', 'export function execGit', "_spawnResult(result, 'git')"],
|
||||
['execNpm', 'export function execNpm', "_spawnResult(result, 'npm')"],
|
||||
['execTool', 'export function execTool', '_spawnResult(result, program)'],
|
||||
];
|
||||
for (const [name, start, end] of helpers) {
|
||||
test(`shell-command-projection ${name} spawnSync sets windowsHide`, () => {
|
||||
const region = regionBetween(cts(), start, end);
|
||||
assert.match(region, /windowsHide:\s*true/, `${name} spawnSync must set windowsHide: true`);
|
||||
});
|
||||
}
|
||||
|
||||
test('gsd-worktree-path-guard SPAWNOPT sets windowsHide', () => {
|
||||
const region = regionBetween(read('hooks/gsd-worktree-path-guard.js'), 'const SPAWNOPT', '};');
|
||||
assert.match(region, /windowsHide:\s*true/, 'gsd-worktree-path-guard SPAWNOPT must set windowsHide: true');
|
||||
});
|
||||
|
||||
test('gsd-workflow-guard currentBranch spawnSync sets windowsHide', () => {
|
||||
const region = regionBetween(read('hooks/gsd-workflow-guard.js'), "spawnSync('git', ['branch'", '});');
|
||||
assert.match(region, /windowsHide:\s*true/, 'gsd-workflow-guard git-branch spawn must set windowsHide: true');
|
||||
});
|
||||
|
||||
test('check-command-router recentCommitMessages execFileSync sets windowsHide', () => {
|
||||
const region = regionBetween(read('src/check-command-router.cts'), "execFileSync('git', ['log'", '});');
|
||||
assert.match(region, /windowsHide:\s*true/, 'check-command-router git-log execFileSync must set windowsHide: true');
|
||||
});
|
||||
|
||||
test('roadmap-upgrade execSync git calls all set windowsHide', () => {
|
||||
const src = read('src/roadmap-upgrade.cts');
|
||||
const calls = src.match(/execSync\([^)]*\)/g) || [];
|
||||
assert.ok(calls.length >= 4, 'expected the roadmap-upgrade git execSync calls to be present');
|
||||
const missing = calls.filter((c) => !/windowsHide:\s*true/.test(c));
|
||||
assert.deepEqual(missing, [], `execSync without windowsHide:\n${missing.join('\n')}`);
|
||||
});
|
||||
|
||||
test('gsd-check-update spawn retains windowsHide (precedent guard)', () => {
|
||||
assert.match(read('hooks/gsd-check-update.js'), /windowsHide:\s*true/,
|
||||
'gsd-check-update.js must keep windowsHide: true');
|
||||
});
|
||||
|
||||
// Durable invariant: ANY external-binary process spawn in the runtime source
|
||||
// (hooks + src) must set windowsHide — catches future additions, not just the
|
||||
// sites known today. Handles the `{ ...CONST }` spread indirection.
|
||||
test('completeness: no external-binary spawn in runtime source omits windowsHide', () => {
|
||||
const listDir = (dir, re) =>
|
||||
fs.readdirSync(path.join(root, dir)).filter((f) => re.test(f)).map((f) => `${dir}/${f}`);
|
||||
const files = [...listDir('hooks', /\.js$/), ...listDir('src', /\.cts$/)];
|
||||
const callRe = /(?:execSync|execFileSync|spawnSync|spawn)\s*\(\s*(?:`|'|")?(?:git|npm|gh)\b|spawn\s*\(\s*process\.execPath/g;
|
||||
const offenders = [];
|
||||
for (const rel of files) {
|
||||
const src = read(rel);
|
||||
let m;
|
||||
while ((m = callRe.exec(src)) !== null) {
|
||||
const win = src.slice(m.index, m.index + 400);
|
||||
let ok = /windowsHide:\s*true/.test(win);
|
||||
if (!ok) {
|
||||
const spread = win.match(/\{\s*\.\.\.(\w+)/); // e.g. { ...SPAWNOPT, cwd }
|
||||
if (spread) {
|
||||
ok = new RegExp(`(?:const|let|var)\\s+${spread[1]}\\s*=\\s*\\{[^}]*windowsHide:\\s*true`).test(src);
|
||||
}
|
||||
}
|
||||
if (!ok) offenders.push(`${rel}: ...${src.slice(m.index, m.index + 48).replace(/\s+/g, ' ')}`);
|
||||
}
|
||||
}
|
||||
assert.deepEqual(offenders, [], `external-binary spawns missing windowsHide:\n${offenders.join('\n')}`);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user