mirror of
https://github.com/garrytan/gstack.git
synced 2026-09-09 14:38:59 +02:00
fix(setup): Windows re-runs refresh installed skills for codex/factory/opencode hosts
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 <noreply@anthropic.com>
This commit is contained in:
Garry Tan
co-authored by
Claude Fable 5
Claude Fable 5
parent
663aca3b05
commit
52006feac4
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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 });
|
||||
}
|
||||
});
|
||||
},
|
||||
);
|
||||
Reference in New Issue
Block a user