From 52006feac487b857ff0295db95a6d7cf9c7e21ee Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Sun, 16 Aug 2026 09:08:13 -0700 Subject: [PATCH] fix(setup): Windows re-runs refresh installed skills for codex/factory/opencode hosts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On Windows (Git Bash / MSYS2, no Developer Mode), _link_or_copy installs REAL directory copies. The install guards in link_codex_skill_dirs, link_factory_skill_dirs, link_opencode_skill_dirs, and create_agents_sidecar only ran the copy when the target was a symlink or missing — true on the first install, never again. Every subsequent ./setup after a git pull reported 'gstack ready (codex).' and exited 0 while silently refreshing nothing: users ran stale SKILL.md forever. (link_claude_skill_dirs already handled this; the other hosts never got the treatment.) Fix: all five guard sites bypass the symlink-or-missing check when IS_WINDOWS=1 — _link_or_copy rm -rf's the destination first, so the real-dir copy refreshes in place. Unix behavior is unchanged (symlinks still pass the guard via -L and serve updates without re-copying). The new bash-fixture test drives the REAL extracted functions through the install → upstream change → re-run cycle under IS_WINDOWS=1 (v1 must become v2), pins the sidecar-skip behavior, checks the Unix path stayed a symlink, and statically asserts the bypass at all five sites so factory/opencode can't regress. Registered in the Windows-safe curated list (KNOWN_WINDOWS_SAFE) so it actually runs on the windows-latest CI lane — the 'bin/' pattern hit is a fixture path segment, not a shebang spawn. Fixes #2444 Co-Authored-By: Claude Fable 5 --- scripts/test-free-shards.ts | 10 ++ setup | 26 +++- test/setup-windows-rerun-refresh.test.ts | 188 +++++++++++++++++++++++ 3 files changed, 219 insertions(+), 5 deletions(-) create mode 100644 test/setup-windows-rerun-refresh.test.ts diff --git a/scripts/test-free-shards.ts b/scripts/test-free-shards.ts index 4c0bc021e..a164af2a3 100755 --- a/scripts/test-free-shards.ts +++ b/scripts/test-free-shards.ts @@ -255,6 +255,16 @@ export const KNOWN_WINDOWS_INCOMPATIBLE: Array<{ file: string; reason: string }> // pattern hit is a false positive — the point of these files is Windows // coverage, so auto-excluding them defeats the regression tests they carry. const KNOWN_WINDOWS_SAFE: Array<{ file: string; reason: string }> = [ + { + file: 'test/setup-windows-rerun-refresh.test.ts', + // Trips the "spawns bin/ shebang script" pattern via path.join(..., 'bin', + // 'tool.sh') fixture paths, but every spawn goes through spawnSync('bash', + // ['-c', ...]) — Git Bash executes it fine on windows-latest. This file IS + // the #2444 Windows regression coverage (IS_WINDOWS=1 copy-refresh path); + // excluding it here would keep the bug class unexercised on the one + // platform it bites. + reason: 'bin/ hits are fixture path segments; spawns bash explicitly — the IS_WINDOWS=1 refresh path must run on windows-latest', + }, { file: 'browse/test/file-permissions.test.ts', // Trips the POSIX-mode-bitmask pattern, but every `mode & 0o777` assertion diff --git a/setup b/setup index def88155a..b4cbed373 100755 --- a/setup +++ b/setup @@ -942,7 +942,11 @@ link_codex_skill_dirs() { [ "$skill_name" = "gstack" ] && continue target="$skills_dir/$skill_name" # Create or update symlink - if [ -L "$target" ] || [ ! -e "$target" ]; then + # #2444: on Windows the installed target is a REAL directory copy, so + # the symlink-or-missing guard skipped every re-run and SKILL.md never + # refreshed after `git pull`. IS_WINDOWS bypasses the guard — + # _link_or_copy rm -rf's the destination first, refreshing the copy. + if [ "$IS_WINDOWS" -eq 1 ] || [ -L "$target" ] || [ ! -e "$target" ]; then _link_or_copy "$skill_dir" "$target" linked+=("$skill_name") fi @@ -968,7 +972,9 @@ create_agents_sidecar() { local src="$SOURCE_GSTACK_DIR/$asset" local dst="$agents_gstack/$asset" if [ -d "$src" ] || [ -f "$src" ]; then - if [ -L "$dst" ] || [ ! -e "$dst" ]; then + # #2444: IS_WINDOWS bypass — real-dir copies never match -L, so re-runs + # skipped the refresh. _link_or_copy rm -rf's the destination first. + if [ "$IS_WINDOWS" -eq 1 ] || [ -L "$dst" ] || [ ! -e "$dst" ]; then _link_or_copy "$src" "$dst" fi fi @@ -979,7 +985,9 @@ create_agents_sidecar() { local src="$SOURCE_GSTACK_DIR/$file" local dst="$agents_gstack/$file" if [ -f "$src" ]; then - if [ -L "$dst" ] || [ ! -e "$dst" ]; then + # #2444: IS_WINDOWS bypass — real-dir copies never match -L, so re-runs + # skipped the refresh. _link_or_copy rm -rf's the destination first. + if [ "$IS_WINDOWS" -eq 1 ] || [ -L "$dst" ] || [ ! -e "$dst" ]; then _link_or_copy "$src" "$dst" fi fi @@ -1175,7 +1183,11 @@ link_factory_skill_dirs() { skill_name="$(basename "$skill_dir")" [ "$skill_name" = "gstack" ] && continue target="$skills_dir/$skill_name" - if [ -L "$target" ] || [ ! -e "$target" ]; then + # #2444: on Windows the installed target is a REAL directory copy, so + # the symlink-or-missing guard skipped every re-run and SKILL.md never + # refreshed after `git pull`. IS_WINDOWS bypasses the guard — + # _link_or_copy rm -rf's the destination first, refreshing the copy. + if [ "$IS_WINDOWS" -eq 1 ] || [ -L "$target" ] || [ ! -e "$target" ]; then _link_or_copy "$skill_dir" "$target" linked+=("$skill_name") fi @@ -1207,7 +1219,11 @@ link_opencode_skill_dirs() { skill_name="$(basename "$skill_dir")" [ "$skill_name" = "gstack" ] && continue target="$skills_dir/$skill_name" - if [ -L "$target" ] || [ ! -e "$target" ]; then + # #2444: on Windows the installed target is a REAL directory copy, so + # the symlink-or-missing guard skipped every re-run and SKILL.md never + # refreshed after `git pull`. IS_WINDOWS bypasses the guard — + # _link_or_copy rm -rf's the destination first, refreshing the copy. + if [ "$IS_WINDOWS" -eq 1 ] || [ -L "$target" ] || [ ! -e "$target" ]; then _link_or_copy "$skill_dir" "$target" linked+=("$skill_name") fi diff --git a/test/setup-windows-rerun-refresh.test.ts b/test/setup-windows-rerun-refresh.test.ts new file mode 100644 index 000000000..24703a463 --- /dev/null +++ b/test/setup-windows-rerun-refresh.test.ts @@ -0,0 +1,188 @@ +/** + * Windows re-run refresh (#2444). + * + * On Windows, _link_or_copy installs REAL directory copies (no Developer + * Mode symlinks). The skill-linking guards `[ -L "$target" ] || [ ! -e + * "$target" ]` in link_codex_skill_dirs / link_factory_skill_dirs / + * link_opencode_skill_dirs / create_agents_sidecar therefore skipped every + * re-run: `./setup --host codex` reported "gstack ready" but never refreshed + * an already-installed SKILL.md after `git pull`. The fix bypasses the guard + * when IS_WINDOWS=1 — _link_or_copy rm -rf's the destination first, so the + * copy refreshes in place. + * + * The behavior fixture drives the REAL link_codex_skill_dirs / + * create_agents_sidecar functions (extracted from setup) against a fake + * install tree; the static block pins the bypass at all five guard sites so + * factory/opencode can't silently regress. + */ +import { describe, test, expect } from 'bun:test'; +import { spawnSync } from 'child_process'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; + +const ROOT = path.resolve(import.meta.dir, '..'); +const SETUP_SRC = fs.readFileSync(path.join(ROOT, 'setup'), 'utf-8'); + +function extractFn(name: string): string { + const start = SETUP_SRC.indexOf(`${name}() {`); + const end = SETUP_SRC.indexOf('\n}\n', start); + if (start < 0 || end < 0) throw new Error(`Could not locate ${name}() in setup`); + return SETUP_SRC.slice(start, end + 2); +} + +const WINDOWS_BYPASS = '[ "$IS_WINDOWS" -eq 1 ] || [ -L '; + +describe('setup: Windows re-run refresh — static guard sites (#2444)', () => { + test('all five install guards carry the IS_WINDOWS bypass', () => { + const sites = SETUP_SRC.split(WINDOWS_BYPASS).length - 1; + expect(sites).toBe(5); + }); + + test.each([ + 'link_codex_skill_dirs', + 'link_factory_skill_dirs', + 'link_opencode_skill_dirs', + 'create_agents_sidecar', + ])('%s bypasses the symlink-or-missing guard on Windows', (fn) => { + expect(extractFn(fn)).toContain(WINDOWS_BYPASS); + }); +}); + +interface RunResult { + status: number | null; + stdout: string; + stderr: string; +} + +/** Run the extracted installer functions against a fake tree. */ +function runInstaller( + isWindows: '0' | '1', + fns: string[], + invocation: string, + extraVars = '', +): RunResult { + const script = [ + 'set -e', + `IS_WINDOWS=${isWindows}`, + extraVars, + extractFn('_link_or_copy'), + ...fns.map(extractFn), + invocation, + ].join('\n'); + const r = spawnSync('bash', ['-c', script], { encoding: 'utf-8', timeout: 15_000 }); + return { status: r.status, stdout: r.stdout, stderr: r.stderr }; +} + +describe('setup: Windows re-run refresh — behavior fixture (#2444)', () => { + test('IS_WINDOWS=1: link_codex_skill_dirs refreshes an already-installed skill', () => { + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-rerun-')); + try { + const fake = path.join(tmp, 'gstack'); + const skills = path.join(tmp, 'skills'); + const demo = path.join(fake, '.agents', 'skills', 'gstack-demo'); + fs.mkdirSync(demo, { recursive: true }); + fs.mkdirSync(skills, { recursive: true }); + fs.writeFileSync(path.join(demo, 'SKILL.md'), 'v1-original\n'); + + // First run: installs the copy. + let r = runInstaller('1', ['link_codex_skill_dirs'], `link_codex_skill_dirs "${fake}" "${skills}"`); + expect(r.status).toBe(0); + const installed = path.join(skills, 'gstack-demo', 'SKILL.md'); + expect(fs.readFileSync(installed, 'utf-8')).toBe('v1-original\n'); + expect(fs.lstatSync(path.join(skills, 'gstack-demo')).isSymbolicLink()).toBe(false); + + // Upstream ships a change (the git pull). + fs.writeFileSync(path.join(demo, 'SKILL.md'), 'v2-UPDATED\n'); + + // Second run: pre-#2444 this was a silent no-op on Windows. + r = runInstaller('1', ['link_codex_skill_dirs'], `link_codex_skill_dirs "${fake}" "${skills}"`); + expect(r.status).toBe(0); + expect(fs.readFileSync(installed, 'utf-8')).toBe('v2-UPDATED\n'); + } finally { + fs.rmSync(tmp, { recursive: true, force: true }); + } + }); + + test('IS_WINDOWS=1: create_agents_sidecar refreshes copied runtime assets', () => { + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-rerun-sidecar-')); + try { + const fake = path.join(tmp, 'gstack'); + fs.mkdirSync(path.join(fake, 'bin'), { recursive: true }); + fs.writeFileSync(path.join(fake, 'bin', 'tool.sh'), 'v1\n'); + fs.writeFileSync(path.join(fake, 'ETHOS.md'), 'ethos-v1\n'); + + const vars = `SOURCE_GSTACK_DIR="${fake}"`; + let r = runInstaller('1', ['create_agents_sidecar'], `create_agents_sidecar "${fake}"`, vars); + expect(r.status).toBe(0); + const sidecarBin = path.join(fake, '.agents', 'skills', 'gstack', 'bin', 'tool.sh'); + const sidecarEthos = path.join(fake, '.agents', 'skills', 'gstack', 'ETHOS.md'); + expect(fs.readFileSync(sidecarBin, 'utf-8')).toBe('v1\n'); + expect(fs.readFileSync(sidecarEthos, 'utf-8')).toBe('ethos-v1\n'); + + fs.writeFileSync(path.join(fake, 'bin', 'tool.sh'), 'v2\n'); + fs.writeFileSync(path.join(fake, 'ETHOS.md'), 'ethos-v2\n'); + + r = runInstaller('1', ['create_agents_sidecar'], `create_agents_sidecar "${fake}"`, vars); + expect(r.status).toBe(0); + expect(fs.readFileSync(sidecarBin, 'utf-8')).toBe('v2\n'); + expect(fs.readFileSync(sidecarEthos, 'utf-8')).toBe('ethos-v2\n'); + } finally { + fs.rmSync(tmp, { recursive: true, force: true }); + } + }); + + test('IS_WINDOWS=1: the gstack sidecar dir is still skipped by the skill loop', () => { + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-rerun-skip-')); + try { + const fake = path.join(tmp, 'gstack'); + const skills = path.join(tmp, 'skills'); + const sidecar = path.join(fake, '.agents', 'skills', 'gstack'); + fs.mkdirSync(sidecar, { recursive: true }); + fs.mkdirSync(skills, { recursive: true }); + fs.writeFileSync(path.join(sidecar, 'SKILL.md'), 'sidecar\n'); + + const r = runInstaller('1', ['link_codex_skill_dirs'], `link_codex_skill_dirs "${fake}" "${skills}"`); + expect(r.status).toBe(0); + expect(fs.existsSync(path.join(skills, 'gstack'))).toBe(false); + } finally { + fs.rmSync(tmp, { recursive: true, force: true }); + } + }); +}); + +// On real Windows, `ln -snf` under Git Bash silently produces copies, so the +// Unix-mode symlink assertions are meaningless there — the same skip the +// _link_or_copy behavior matrix uses (test/setup-windows-fallback.test.ts). +describe.skipIf(process.platform === 'win32')( + 'setup: Unix path unchanged by the #2444 bypass', + () => { + test('IS_WINDOWS=0: installs a symlink and re-runs still refresh through it', () => { + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-rerun-unix-')); + try { + const fake = path.join(tmp, 'gstack'); + const skills = path.join(tmp, 'skills'); + const demo = path.join(fake, '.agents', 'skills', 'gstack-demo'); + fs.mkdirSync(demo, { recursive: true }); + fs.mkdirSync(skills, { recursive: true }); + fs.writeFileSync(path.join(demo, 'SKILL.md'), 'v1-original\n'); + + let r = runInstaller('0', ['link_codex_skill_dirs'], `link_codex_skill_dirs "${fake}" "${skills}"`); + expect(r.status).toBe(0); + const target = path.join(skills, 'gstack-demo'); + expect(fs.lstatSync(target).isSymbolicLink()).toBe(true); + + // A symlink serves updates without any re-run at all… + fs.writeFileSync(path.join(demo, 'SKILL.md'), 'v2-UPDATED\n'); + expect(fs.readFileSync(path.join(target, 'SKILL.md'), 'utf-8')).toBe('v2-UPDATED\n'); + + // …and the re-run keeps it a symlink (guard still passes via -L). + r = runInstaller('0', ['link_codex_skill_dirs'], `link_codex_skill_dirs "${fake}" "${skills}"`); + expect(r.status).toBe(0); + expect(fs.lstatSync(target).isSymbolicLink()).toBe(true); + } finally { + fs.rmSync(tmp, { recursive: true, force: true }); + } + }); + }, +);