mirror of
https://github.com/garrytan/gstack.git
synced 2026-05-02 11:45:20 +02:00
822e843a60
* fix: headed browser no longer auto-shuts down after 15 seconds The parent-process watchdog in server.ts polls the spawning CLI's PID every 15s and self-terminates if it is gone. The connect command in cli.ts exits with process.exit(0) immediately after launching the server, so the watchdog would reliably kill the headed browser within ~15s. This contradicted the idle timer's own design: server.ts:745 explicitly skips headed mode because "the user is looking at the browser. Never auto-die." The watchdog had no such exemption. Two-layer fix: 1. CLI layer: connect handler always sets BROWSE_PARENT_PID=0 (was only pass-through for pair-agent subprocesses). The user owns the headed browser lifecycle; cleanup happens via browser disconnect event or $B disconnect. 2. CLI layer: startServer() honors caller's BROWSE_PARENT_PID=0 in the headless spawn path too. Lets CI, non-interactive shells, and Claude Code Bash calls opt into persistent servers across short-lived CLI invocations. 3. Server layer: defense-in-depth. Watchdog now also skips when BROWSE_HEADED=1, so even if a future launcher forgets PID=0, headed browsers won't die. Adds log lines when the watchdog is disabled so lifecycle debugging is easier. Four community contributors diagnosed variants of this bug independently. Thanks for the clear analyses and reproductions. Closes #1020 (rocke2020) Closes #1018 (sanghyuk-seo-nexcube) Closes #1012 (rodbland2021) Closes #986 (jbetala7) Closes #1006 Closes #943 Co-Authored-By: rocke2020 <noreply@github.com> Co-Authored-By: sanghyuk-seo-nexcube <noreply@github.com> Co-Authored-By: rodbland2021 <noreply@github.com> Co-Authored-By: jbetala7 <noreply@github.com> Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix: disconnect handler runs full cleanup before exiting When the user closed the headed browser window, the disconnect handler in browser-manager.ts called process.exit(2) directly, bypassing the server's shutdown() function entirely. That meant: - sidebar-agent daemon kept polling a dead server - session state wasn't saved - Chromium profile locks (SingletonLock, SingletonSocket, SingletonCookie) weren't cleaned — causing "profile in use" errors on next $B connect - state file at .gstack/browse.json was left stale Now the disconnect handler calls onDisconnect(), which server.ts wires up to shutdown(2). Full cleanup runs first, then the process exits with code 2 — preserving the existing semantic that distinguishes user-close (exit 2) from crashes (exit 1). shutdown() now accepts an optional exitCode parameter (default 0) so the SIGTERM/SIGINT paths and the disconnect path can share cleanup code while preserving their distinct exit codes. Surfaced by Codex during /plan-eng-review of the watchdog fix. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix: pre-existing test flakiness in relink.test.ts The 23 tests in this file all shell out to gstack-config + gstack-relink (bash scripts doing subprocess work). Under parallel bun test load, those subprocess spawns contend with other test suites and each test can drift ~200ms past Bun's 5s default timeout, causing 5+ flaky timeouts per run in the gate-tier ship gate. Wrap the `test` import to default the per-test timeout to 15s. Explicit per-test timeouts (third arg) still win, so individual tests can lower it if needed. No behavior change — only gives subprocess-heavy tests more headroom under parallel load. Noticed by /ship pre-flight test run. Unrelated to the main PR fix but blocking the gate, so fixing as a separate commit per the test ownership protocol. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix: SIGTERM/SIGINT shutdown exit code regression Node's signal listeners receive the signal name ('SIGTERM' / 'SIGINT') as the first argument. When shutdown() started accepting an optional exitCode parameter in the prior disconnect-cleanup commit, the bare `process.on('SIGTERM', shutdown)` registration started silently calling shutdown('SIGTERM'). The string passed through to process.exit(), Node coerced it to NaN, and the process exited with code 1 instead of 0. Wrap both listeners so they call shutdown() with no args — signal name never leaks into the exitCode slot. Surfaced by /ship's adversarial subagent. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix: onDisconnect async rejection leaves process running The disconnect handler calls this.onDisconnect() without awaiting it, but server.ts wires the callback to shutdown(2) — which is async. If that promise rejects, the rejection drops on the floor as an unhandled rejection, the browser is already disconnected, and the server keeps running indefinitely with no browser attached. Add a sync try/catch for throws and a .catch() chain for promise rejections. Both fall back to process.exit(2) so a dead browser never leaves a live server. Also widen the callback type from `() => void` to `() => void | Promise<void>` to match the actual runtime shape of the wired shutdown(2) call. Surfaced by /ship's adversarial subagent. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix: honor BROWSE_PARENT_PID=0 with trailing whitespace The strict string compare `process.env.BROWSE_PARENT_PID === '0'` meant any stray newline or whitespace (common from shell `export` in a pipe or heredoc) would fail the check and re-enable the watchdog against the caller's intent. Switch to parseInt + === 0, matching the server's own parseInt at server.ts:760. Handles '0', '0\n', ' 0 ', and unset correctly; non-numeric values (parseInt returns NaN, NaN === 0 is false) fail safe — watchdog stays active, which is the safe default for unexpected input. Surfaced by /ship's adversarial subagent. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix: preserve bun:test sub-APIs in relink test wrapper The previous commit wrapped bun:test's `test` to bump the per-test timeout default to 15s but cast the wrapper `as typeof _bunTest` without copying the sub-properties (`.only`, `.skip`, `.each`, `.todo`, `.failing`, `.if`) from the original. The cast was a lie: the wrapper was a plain function, not the full callable with those chained properties attached. The file doesn't use any of them today, but a future test.only or test.skip would fail with a cryptic "undefined is not a function." Object.assign the original _bunTest's properties onto the wrapper so sub-APIs chain correctly forever. Surfaced by /ship's adversarial subagent. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore: bump version and changelog (v0.18.1.0) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test: regression tests for parent-process watchdog End-to-end tests in browse/test/watchdog.test.ts that prove the three invariants v0.18.1.0 depends on. Each test spawns the real server.ts (not a mock), so any future change that breaks the watchdog logic fails here — the thing /ship's adversarial review flagged as missing. 1. BROWSE_PARENT_PID=0 disables the watchdog Spawns server with PID=0, reads stdout, confirms the "watchdog disabled (BROWSE_PARENT_PID=0)" log line appears and "Parent process ... exited" does NOT. ~2s. 2. BROWSE_HEADED=1 disables the watchdog (server-side guard) Spawns server with BROWSE_HEADED=1 and a bogus parent PID (999999). Proves BROWSE_HEADED takes precedence over a present PID — if the server-side defense-in-depth regresses, the watchdog would try to poll 999999 and fire on the "dead parent." ~2s. 3. Default headless mode: watchdog fires when parent dies The regression guard for the original orphan-prevention behavior. Spawns a real `sleep 60` parent and a server watching its PID, then kills the parent and waits up to 25s for the server to exit. The watchdog polls every 15s so first tick is 0-15s after death, plus shutdown() cleanup. ~18s. Total runtime: ~21s for all 3 tests. They catch the class of bug this branch exists to fix: "does the process live or die when it should?" Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: rocke2020 <noreply@github.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
526 lines
22 KiB
TypeScript
526 lines
22 KiB
TypeScript
import { describe, test as _bunTest, expect, beforeEach, afterEach } from 'bun:test';
|
|
import { execSync } from 'child_process';
|
|
import * as fs from 'fs';
|
|
import * as path from 'path';
|
|
import * as os from 'os';
|
|
|
|
// Every test in this file shells out to gstack-config + gstack-relink (bash scripts
|
|
// invoking subprocess work). Under parallel bun test load, subprocess spawn contends
|
|
// with other suites and each test can drift ~200ms past the 5s default. Bump to 15s.
|
|
// Object.assign preserves test.only / test.skip / test.each / test.todo sub-APIs.
|
|
const test = Object.assign(
|
|
((name: any, fn: any, timeout?: number) =>
|
|
_bunTest(name, fn, timeout ?? 15_000)) as typeof _bunTest,
|
|
_bunTest,
|
|
);
|
|
|
|
const ROOT = path.resolve(import.meta.dir, '..');
|
|
const BIN = path.join(ROOT, 'bin');
|
|
|
|
let tmpDir: string;
|
|
let skillsDir: string;
|
|
let installDir: string;
|
|
|
|
function run(cmd: string, env: Record<string, string> = {}, expectFail = false): string {
|
|
try {
|
|
return execSync(cmd, {
|
|
cwd: ROOT,
|
|
env: { ...process.env, GSTACK_STATE_DIR: tmpDir, ...env },
|
|
encoding: 'utf-8',
|
|
timeout: 10000,
|
|
stdio: ['pipe', 'pipe', 'pipe'],
|
|
}).trim();
|
|
} catch (e: any) {
|
|
if (expectFail) return (e.stderr || e.stdout || '').toString().trim();
|
|
throw e;
|
|
}
|
|
}
|
|
|
|
// Create a mock gstack install directory with skill subdirs
|
|
function setupMockInstall(skills: string[]): void {
|
|
installDir = path.join(tmpDir, 'gstack-install');
|
|
skillsDir = path.join(tmpDir, 'skills');
|
|
fs.mkdirSync(installDir, { recursive: true });
|
|
fs.mkdirSync(skillsDir, { recursive: true });
|
|
|
|
// Copy the real gstack-config and gstack-relink to the mock install
|
|
const mockBin = path.join(installDir, 'bin');
|
|
fs.mkdirSync(mockBin, { recursive: true });
|
|
fs.copyFileSync(path.join(BIN, 'gstack-config'), path.join(mockBin, 'gstack-config'));
|
|
fs.chmodSync(path.join(mockBin, 'gstack-config'), 0o755);
|
|
if (fs.existsSync(path.join(BIN, 'gstack-relink'))) {
|
|
fs.copyFileSync(path.join(BIN, 'gstack-relink'), path.join(mockBin, 'gstack-relink'));
|
|
fs.chmodSync(path.join(mockBin, 'gstack-relink'), 0o755);
|
|
}
|
|
if (fs.existsSync(path.join(BIN, 'gstack-patch-names'))) {
|
|
fs.copyFileSync(path.join(BIN, 'gstack-patch-names'), path.join(mockBin, 'gstack-patch-names'));
|
|
fs.chmodSync(path.join(mockBin, 'gstack-patch-names'), 0o755);
|
|
}
|
|
|
|
// Create mock skill directories with proper frontmatter
|
|
for (const skill of skills) {
|
|
fs.mkdirSync(path.join(installDir, skill), { recursive: true });
|
|
fs.writeFileSync(
|
|
path.join(installDir, skill, 'SKILL.md'),
|
|
`---\nname: ${skill}\ndescription: test\n---\n# ${skill}`
|
|
);
|
|
}
|
|
}
|
|
|
|
beforeEach(() => {
|
|
tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-relink-test-'));
|
|
});
|
|
|
|
afterEach(() => {
|
|
fs.rmSync(tmpDir, { recursive: true, force: true });
|
|
});
|
|
|
|
describe('gstack-relink (#578)', () => {
|
|
// Test 11: prefixed symlinks when skill_prefix=true
|
|
test('creates gstack-* symlinks when skill_prefix=true', () => {
|
|
setupMockInstall(['qa', 'ship', 'review']);
|
|
// Set config to prefix mode (pass install/skills env so auto-relink uses mock install)
|
|
run(`${path.join(installDir, 'bin', 'gstack-config')} set skill_prefix true`, {
|
|
GSTACK_INSTALL_DIR: installDir,
|
|
GSTACK_SKILLS_DIR: skillsDir,
|
|
});
|
|
// Run relink with env pointing to the mock install
|
|
const output = run(`${path.join(installDir, 'bin', 'gstack-relink')}`, {
|
|
GSTACK_INSTALL_DIR: installDir,
|
|
GSTACK_SKILLS_DIR: skillsDir,
|
|
});
|
|
// Verify gstack-* symlinks exist
|
|
expect(fs.existsSync(path.join(skillsDir, 'gstack-qa'))).toBe(true);
|
|
expect(fs.existsSync(path.join(skillsDir, 'gstack-ship'))).toBe(true);
|
|
expect(fs.existsSync(path.join(skillsDir, 'gstack-review'))).toBe(true);
|
|
expect(output).toContain('gstack-');
|
|
});
|
|
|
|
// Test 12: flat symlinks when skill_prefix=false
|
|
test('creates flat symlinks when skill_prefix=false', () => {
|
|
setupMockInstall(['qa', 'ship', 'review']);
|
|
run(`${path.join(installDir, 'bin', 'gstack-config')} set skill_prefix false`, {
|
|
GSTACK_INSTALL_DIR: installDir,
|
|
GSTACK_SKILLS_DIR: skillsDir,
|
|
});
|
|
const output = run(`${path.join(installDir, 'bin', 'gstack-relink')}`, {
|
|
GSTACK_INSTALL_DIR: installDir,
|
|
GSTACK_SKILLS_DIR: skillsDir,
|
|
});
|
|
expect(fs.existsSync(path.join(skillsDir, 'qa'))).toBe(true);
|
|
expect(fs.existsSync(path.join(skillsDir, 'ship'))).toBe(true);
|
|
expect(fs.existsSync(path.join(skillsDir, 'review'))).toBe(true);
|
|
expect(output).toContain('flat');
|
|
});
|
|
|
|
// REGRESSION: unprefixed skills must be real directories, not symlinks (#761)
|
|
// Claude Code auto-prefixes skills nested under a parent dir symlink.
|
|
// e.g., `qa -> gstack/qa` gets discovered as "gstack-qa", not "qa".
|
|
// The fix: create real directories with SKILL.md symlinks inside.
|
|
test('unprefixed skills are real directories with SKILL.md symlinks, not dir symlinks', () => {
|
|
setupMockInstall(['qa', 'ship', 'review', 'plan-ceo-review']);
|
|
run(`${path.join(installDir, 'bin', 'gstack-config')} set skill_prefix false`, {
|
|
GSTACK_INSTALL_DIR: installDir,
|
|
GSTACK_SKILLS_DIR: skillsDir,
|
|
});
|
|
run(`${path.join(installDir, 'bin', 'gstack-relink')}`, {
|
|
GSTACK_INSTALL_DIR: installDir,
|
|
GSTACK_SKILLS_DIR: skillsDir,
|
|
});
|
|
for (const skill of ['qa', 'ship', 'review', 'plan-ceo-review']) {
|
|
const skillPath = path.join(skillsDir, skill);
|
|
const skillMdPath = path.join(skillPath, 'SKILL.md');
|
|
// Must be a real directory, NOT a symlink
|
|
expect(fs.lstatSync(skillPath).isDirectory()).toBe(true);
|
|
expect(fs.lstatSync(skillPath).isSymbolicLink()).toBe(false);
|
|
// Must contain a SKILL.md that IS a symlink
|
|
expect(fs.existsSync(skillMdPath)).toBe(true);
|
|
expect(fs.lstatSync(skillMdPath).isSymbolicLink()).toBe(true);
|
|
// The SKILL.md symlink must point to the source skill's SKILL.md
|
|
const target = fs.readlinkSync(skillMdPath);
|
|
expect(target).toContain(skill);
|
|
expect(target).toEndWith('/SKILL.md');
|
|
}
|
|
});
|
|
|
|
// Same invariant for prefixed mode
|
|
test('prefixed skills are real directories with SKILL.md symlinks, not dir symlinks', () => {
|
|
setupMockInstall(['qa', 'ship']);
|
|
run(`${path.join(installDir, 'bin', 'gstack-config')} set skill_prefix true`, {
|
|
GSTACK_INSTALL_DIR: installDir,
|
|
GSTACK_SKILLS_DIR: skillsDir,
|
|
});
|
|
run(`${path.join(installDir, 'bin', 'gstack-relink')}`, {
|
|
GSTACK_INSTALL_DIR: installDir,
|
|
GSTACK_SKILLS_DIR: skillsDir,
|
|
});
|
|
for (const skill of ['gstack-qa', 'gstack-ship']) {
|
|
const skillPath = path.join(skillsDir, skill);
|
|
const skillMdPath = path.join(skillPath, 'SKILL.md');
|
|
expect(fs.lstatSync(skillPath).isDirectory()).toBe(true);
|
|
expect(fs.lstatSync(skillPath).isSymbolicLink()).toBe(false);
|
|
expect(fs.lstatSync(skillMdPath).isSymbolicLink()).toBe(true);
|
|
}
|
|
});
|
|
|
|
// Upgrade: old directory symlinks get replaced with real directories
|
|
test('upgrades old directory symlinks to real directories', () => {
|
|
setupMockInstall(['qa', 'ship']);
|
|
// Simulate old behavior: create directory symlinks (the old pattern)
|
|
fs.symlinkSync(path.join(installDir, 'qa'), path.join(skillsDir, 'qa'));
|
|
fs.symlinkSync(path.join(installDir, 'ship'), path.join(skillsDir, 'ship'));
|
|
// Verify they start as symlinks
|
|
expect(fs.lstatSync(path.join(skillsDir, 'qa')).isSymbolicLink()).toBe(true);
|
|
|
|
run(`${path.join(installDir, 'bin', 'gstack-config')} set skill_prefix false`, {
|
|
GSTACK_INSTALL_DIR: installDir,
|
|
GSTACK_SKILLS_DIR: skillsDir,
|
|
});
|
|
run(`${path.join(installDir, 'bin', 'gstack-relink')}`, {
|
|
GSTACK_INSTALL_DIR: installDir,
|
|
GSTACK_SKILLS_DIR: skillsDir,
|
|
});
|
|
|
|
// After relink: must be real directories, not symlinks
|
|
expect(fs.lstatSync(path.join(skillsDir, 'qa')).isSymbolicLink()).toBe(false);
|
|
expect(fs.lstatSync(path.join(skillsDir, 'qa')).isDirectory()).toBe(true);
|
|
expect(fs.lstatSync(path.join(skillsDir, 'qa', 'SKILL.md')).isSymbolicLink()).toBe(true);
|
|
});
|
|
|
|
// FIRST INSTALL: --no-prefix must create ONLY flat names, zero gstack-* pollution
|
|
test('first install --no-prefix: only flat names exist, zero gstack-* entries', () => {
|
|
setupMockInstall(['qa', 'ship', 'review', 'plan-ceo-review', 'gstack-upgrade']);
|
|
// Simulate first install: no saved config, pass --no-prefix equivalent
|
|
run(`${path.join(installDir, 'bin', 'gstack-config')} set skill_prefix false`, {
|
|
GSTACK_INSTALL_DIR: installDir,
|
|
GSTACK_SKILLS_DIR: skillsDir,
|
|
});
|
|
run(`${path.join(installDir, 'bin', 'gstack-relink')}`, {
|
|
GSTACK_INSTALL_DIR: installDir,
|
|
GSTACK_SKILLS_DIR: skillsDir,
|
|
});
|
|
// Enumerate everything in skills dir
|
|
const entries = fs.readdirSync(skillsDir);
|
|
// Expected: qa, ship, review, plan-ceo-review, gstack-upgrade (its real name)
|
|
expect(entries.sort()).toEqual(['gstack-upgrade', 'plan-ceo-review', 'qa', 'review', 'ship']);
|
|
// No gstack-qa, gstack-ship, gstack-review, gstack-plan-ceo-review
|
|
const leaked = entries.filter(e => e.startsWith('gstack-') && e !== 'gstack-upgrade');
|
|
expect(leaked).toEqual([]);
|
|
});
|
|
|
|
// FIRST INSTALL: --prefix must create ONLY gstack-* names, zero flat-name pollution
|
|
test('first install --prefix: only gstack-* entries exist, zero flat names', () => {
|
|
setupMockInstall(['qa', 'ship', 'review', 'plan-ceo-review', 'gstack-upgrade']);
|
|
run(`${path.join(installDir, 'bin', 'gstack-config')} set skill_prefix true`, {
|
|
GSTACK_INSTALL_DIR: installDir,
|
|
GSTACK_SKILLS_DIR: skillsDir,
|
|
});
|
|
run(`${path.join(installDir, 'bin', 'gstack-relink')}`, {
|
|
GSTACK_INSTALL_DIR: installDir,
|
|
GSTACK_SKILLS_DIR: skillsDir,
|
|
});
|
|
const entries = fs.readdirSync(skillsDir);
|
|
// Expected: gstack-qa, gstack-ship, gstack-review, gstack-plan-ceo-review, gstack-upgrade
|
|
expect(entries.sort()).toEqual([
|
|
'gstack-plan-ceo-review', 'gstack-qa', 'gstack-review', 'gstack-ship', 'gstack-upgrade',
|
|
]);
|
|
// No unprefixed qa, ship, review, plan-ceo-review
|
|
const leaked = entries.filter(e => !e.startsWith('gstack-'));
|
|
expect(leaked).toEqual([]);
|
|
});
|
|
|
|
// FIRST INSTALL: non-TTY (no saved config, piped stdin) defaults to flat names
|
|
test('non-TTY first install defaults to flat names via relink', () => {
|
|
setupMockInstall(['qa', 'ship']);
|
|
// Don't set any config — simulate fresh install
|
|
// gstack-relink reads config; on fresh install config returns empty → defaults to false
|
|
run(`${path.join(installDir, 'bin', 'gstack-relink')}`, {
|
|
GSTACK_INSTALL_DIR: installDir,
|
|
GSTACK_SKILLS_DIR: skillsDir,
|
|
});
|
|
const entries = fs.readdirSync(skillsDir);
|
|
// Should be flat names (relink defaults to false when config returns empty)
|
|
expect(entries.sort()).toEqual(['qa', 'ship']);
|
|
});
|
|
|
|
// SWITCH: prefix → no-prefix must clean up ALL gstack-* entries
|
|
test('switching prefix to no-prefix removes all gstack-* entries completely', () => {
|
|
setupMockInstall(['qa', 'ship', 'review', 'plan-ceo-review', 'gstack-upgrade']);
|
|
// Start in prefix mode
|
|
run(`${path.join(installDir, 'bin', 'gstack-config')} set skill_prefix true`, {
|
|
GSTACK_INSTALL_DIR: installDir,
|
|
GSTACK_SKILLS_DIR: skillsDir,
|
|
});
|
|
run(`${path.join(installDir, 'bin', 'gstack-relink')}`, {
|
|
GSTACK_INSTALL_DIR: installDir,
|
|
GSTACK_SKILLS_DIR: skillsDir,
|
|
});
|
|
let entries = fs.readdirSync(skillsDir);
|
|
expect(entries.filter(e => !e.startsWith('gstack-'))).toEqual([]);
|
|
|
|
// Switch to no-prefix
|
|
run(`${path.join(installDir, 'bin', 'gstack-config')} set skill_prefix false`, {
|
|
GSTACK_INSTALL_DIR: installDir,
|
|
GSTACK_SKILLS_DIR: skillsDir,
|
|
});
|
|
run(`${path.join(installDir, 'bin', 'gstack-relink')}`, {
|
|
GSTACK_INSTALL_DIR: installDir,
|
|
GSTACK_SKILLS_DIR: skillsDir,
|
|
});
|
|
entries = fs.readdirSync(skillsDir);
|
|
// Only flat names + gstack-upgrade (its real name)
|
|
expect(entries.sort()).toEqual(['gstack-upgrade', 'plan-ceo-review', 'qa', 'review', 'ship']);
|
|
const leaked = entries.filter(e => e.startsWith('gstack-') && e !== 'gstack-upgrade');
|
|
expect(leaked).toEqual([]);
|
|
});
|
|
|
|
// SWITCH: no-prefix → prefix must clean up ALL flat entries
|
|
test('switching no-prefix to prefix removes all flat entries completely', () => {
|
|
setupMockInstall(['qa', 'ship', 'review', 'gstack-upgrade']);
|
|
// Start in no-prefix mode
|
|
run(`${path.join(installDir, 'bin', 'gstack-config')} set skill_prefix false`, {
|
|
GSTACK_INSTALL_DIR: installDir,
|
|
GSTACK_SKILLS_DIR: skillsDir,
|
|
});
|
|
run(`${path.join(installDir, 'bin', 'gstack-relink')}`, {
|
|
GSTACK_INSTALL_DIR: installDir,
|
|
GSTACK_SKILLS_DIR: skillsDir,
|
|
});
|
|
let entries = fs.readdirSync(skillsDir);
|
|
expect(entries.filter(e => e.startsWith('gstack-') && e !== 'gstack-upgrade')).toEqual([]);
|
|
|
|
// Switch to prefix
|
|
run(`${path.join(installDir, 'bin', 'gstack-config')} set skill_prefix true`, {
|
|
GSTACK_INSTALL_DIR: installDir,
|
|
GSTACK_SKILLS_DIR: skillsDir,
|
|
});
|
|
run(`${path.join(installDir, 'bin', 'gstack-relink')}`, {
|
|
GSTACK_INSTALL_DIR: installDir,
|
|
GSTACK_SKILLS_DIR: skillsDir,
|
|
});
|
|
entries = fs.readdirSync(skillsDir);
|
|
// Only gstack-* names
|
|
expect(entries.sort()).toEqual([
|
|
'gstack-qa', 'gstack-review', 'gstack-ship', 'gstack-upgrade',
|
|
]);
|
|
const leaked = entries.filter(e => !e.startsWith('gstack-'));
|
|
expect(leaked).toEqual([]);
|
|
});
|
|
|
|
// Test 13: cleans stale symlinks from opposite mode
|
|
test('cleans up stale symlinks from opposite mode', () => {
|
|
setupMockInstall(['qa', 'ship']);
|
|
// Create prefixed symlinks first
|
|
run(`${path.join(installDir, 'bin', 'gstack-config')} set skill_prefix true`, {
|
|
GSTACK_INSTALL_DIR: installDir,
|
|
GSTACK_SKILLS_DIR: skillsDir,
|
|
});
|
|
run(`${path.join(installDir, 'bin', 'gstack-relink')}`, {
|
|
GSTACK_INSTALL_DIR: installDir,
|
|
GSTACK_SKILLS_DIR: skillsDir,
|
|
});
|
|
expect(fs.existsSync(path.join(skillsDir, 'gstack-qa'))).toBe(true);
|
|
|
|
// Switch to flat mode
|
|
run(`${path.join(installDir, 'bin', 'gstack-config')} set skill_prefix false`, {
|
|
GSTACK_INSTALL_DIR: installDir,
|
|
GSTACK_SKILLS_DIR: skillsDir,
|
|
});
|
|
run(`${path.join(installDir, 'bin', 'gstack-relink')}`, {
|
|
GSTACK_INSTALL_DIR: installDir,
|
|
GSTACK_SKILLS_DIR: skillsDir,
|
|
});
|
|
|
|
// Flat symlinks should exist, prefixed should be gone
|
|
expect(fs.existsSync(path.join(skillsDir, 'qa'))).toBe(true);
|
|
expect(fs.existsSync(path.join(skillsDir, 'gstack-qa'))).toBe(false);
|
|
});
|
|
|
|
// Test 14: error when install dir missing
|
|
test('prints error when install dir missing', () => {
|
|
const output = run(`${BIN}/gstack-relink`, {
|
|
GSTACK_INSTALL_DIR: '/nonexistent/path/gstack',
|
|
GSTACK_SKILLS_DIR: '/nonexistent/path/skills',
|
|
}, true);
|
|
expect(output).toContain('setup');
|
|
});
|
|
|
|
// Test: gstack-upgrade does NOT get double-prefixed
|
|
test('does not double-prefix gstack-upgrade directory', () => {
|
|
setupMockInstall(['qa', 'ship', 'gstack-upgrade']);
|
|
run(`${path.join(installDir, 'bin', 'gstack-config')} set skill_prefix true`, {
|
|
GSTACK_INSTALL_DIR: installDir,
|
|
GSTACK_SKILLS_DIR: skillsDir,
|
|
});
|
|
run(`${path.join(installDir, 'bin', 'gstack-relink')}`, {
|
|
GSTACK_INSTALL_DIR: installDir,
|
|
GSTACK_SKILLS_DIR: skillsDir,
|
|
});
|
|
// gstack-upgrade should keep its name, NOT become gstack-gstack-upgrade
|
|
expect(fs.existsSync(path.join(skillsDir, 'gstack-upgrade'))).toBe(true);
|
|
expect(fs.existsSync(path.join(skillsDir, 'gstack-gstack-upgrade'))).toBe(false);
|
|
// Regular skills still get prefixed
|
|
expect(fs.existsSync(path.join(skillsDir, 'gstack-qa'))).toBe(true);
|
|
});
|
|
|
|
// Test 15: gstack-config set skill_prefix triggers relink
|
|
test('gstack-config set skill_prefix triggers relink', () => {
|
|
setupMockInstall(['qa', 'ship']);
|
|
// Run gstack-config set which should auto-trigger relink
|
|
run(`${path.join(installDir, 'bin', 'gstack-config')} set skill_prefix true`, {
|
|
GSTACK_INSTALL_DIR: installDir,
|
|
GSTACK_SKILLS_DIR: skillsDir,
|
|
});
|
|
// If relink was triggered, symlinks should exist
|
|
expect(fs.existsSync(path.join(skillsDir, 'gstack-qa'))).toBe(true);
|
|
expect(fs.existsSync(path.join(skillsDir, 'gstack-ship'))).toBe(true);
|
|
});
|
|
});
|
|
|
|
describe('upgrade migrations', () => {
|
|
const MIGRATIONS_DIR = path.join(ROOT, 'gstack-upgrade', 'migrations');
|
|
|
|
test('migrations directory exists', () => {
|
|
expect(fs.existsSync(MIGRATIONS_DIR)).toBe(true);
|
|
});
|
|
|
|
test('all migration scripts are executable and parse without syntax errors', () => {
|
|
const scripts = fs.readdirSync(MIGRATIONS_DIR).filter(f => f.endsWith('.sh'));
|
|
expect(scripts.length).toBeGreaterThan(0);
|
|
for (const script of scripts) {
|
|
const fullPath = path.join(MIGRATIONS_DIR, script);
|
|
// Must be executable
|
|
const stat = fs.statSync(fullPath);
|
|
expect(stat.mode & 0o111).toBeGreaterThan(0);
|
|
// Must parse without syntax errors (bash -n is a syntax check, doesn't execute)
|
|
const result = execSync(`bash -n "${fullPath}" 2>&1`, { encoding: 'utf-8', timeout: 5000 });
|
|
// bash -n outputs nothing on success
|
|
}
|
|
});
|
|
|
|
test('migration filenames follow v{VERSION}.sh pattern', () => {
|
|
const scripts = fs.readdirSync(MIGRATIONS_DIR).filter(f => f.endsWith('.sh'));
|
|
for (const script of scripts) {
|
|
expect(script).toMatch(/^v\d+\.\d+\.\d+\.\d+\.sh$/);
|
|
}
|
|
});
|
|
|
|
test('v0.15.2.0 migration runs gstack-relink', () => {
|
|
const content = fs.readFileSync(path.join(MIGRATIONS_DIR, 'v0.15.2.0.sh'), 'utf-8');
|
|
expect(content).toContain('gstack-relink');
|
|
});
|
|
|
|
test('v0.15.2.0 migration fixes stale directory symlinks', () => {
|
|
setupMockInstall(['qa', 'ship', 'review']);
|
|
// Simulate old state: directory symlinks (pre-v0.15.2.0 pattern)
|
|
fs.symlinkSync(path.join(installDir, 'qa'), path.join(skillsDir, 'qa'));
|
|
fs.symlinkSync(path.join(installDir, 'ship'), path.join(skillsDir, 'ship'));
|
|
fs.symlinkSync(path.join(installDir, 'review'), path.join(skillsDir, 'review'));
|
|
// Set no-prefix mode (suppress auto-relink so symlinks stay intact for the test)
|
|
run(`${path.join(installDir, 'bin', 'gstack-config')} set skill_prefix false`, {
|
|
GSTACK_SETUP_RUNNING: '1',
|
|
});
|
|
// Verify old state: symlinks
|
|
expect(fs.lstatSync(path.join(skillsDir, 'qa')).isSymbolicLink()).toBe(true);
|
|
|
|
// Run the migration (it calls gstack-relink internally)
|
|
run(`bash ${path.join(MIGRATIONS_DIR, 'v0.15.2.0.sh')}`, {
|
|
GSTACK_INSTALL_DIR: installDir,
|
|
GSTACK_SKILLS_DIR: skillsDir,
|
|
});
|
|
|
|
// After migration: real directories with SKILL.md symlinks
|
|
for (const skill of ['qa', 'ship', 'review']) {
|
|
const skillPath = path.join(skillsDir, skill);
|
|
expect(fs.lstatSync(skillPath).isSymbolicLink()).toBe(false);
|
|
expect(fs.lstatSync(skillPath).isDirectory()).toBe(true);
|
|
expect(fs.lstatSync(path.join(skillPath, 'SKILL.md')).isSymbolicLink()).toBe(true);
|
|
}
|
|
});
|
|
});
|
|
|
|
describe('gstack-patch-names (#620/#578)', () => {
|
|
// Helper to read name: from SKILL.md frontmatter
|
|
function readSkillName(skillDir: string): string | null {
|
|
const content = fs.readFileSync(path.join(skillDir, 'SKILL.md'), 'utf-8');
|
|
const match = content.match(/^name:\s*(.+)$/m);
|
|
return match ? match[1].trim() : null;
|
|
}
|
|
|
|
test('prefix=true patches name: field in SKILL.md', () => {
|
|
setupMockInstall(['qa', 'ship', 'review']);
|
|
run(`${path.join(installDir, 'bin', 'gstack-config')} set skill_prefix true`, {
|
|
GSTACK_INSTALL_DIR: installDir,
|
|
GSTACK_SKILLS_DIR: skillsDir,
|
|
});
|
|
run(`${path.join(installDir, 'bin', 'gstack-relink')}`, {
|
|
GSTACK_INSTALL_DIR: installDir,
|
|
GSTACK_SKILLS_DIR: skillsDir,
|
|
});
|
|
// Verify name: field is patched with gstack- prefix
|
|
expect(readSkillName(path.join(installDir, 'qa'))).toBe('gstack-qa');
|
|
expect(readSkillName(path.join(installDir, 'ship'))).toBe('gstack-ship');
|
|
expect(readSkillName(path.join(installDir, 'review'))).toBe('gstack-review');
|
|
});
|
|
|
|
test('prefix=false restores name: field in SKILL.md', () => {
|
|
setupMockInstall(['qa', 'ship']);
|
|
// First, prefix them
|
|
run(`${path.join(installDir, 'bin', 'gstack-config')} set skill_prefix true`, {
|
|
GSTACK_INSTALL_DIR: installDir,
|
|
GSTACK_SKILLS_DIR: skillsDir,
|
|
});
|
|
run(`${path.join(installDir, 'bin', 'gstack-relink')}`, {
|
|
GSTACK_INSTALL_DIR: installDir,
|
|
GSTACK_SKILLS_DIR: skillsDir,
|
|
});
|
|
expect(readSkillName(path.join(installDir, 'qa'))).toBe('gstack-qa');
|
|
// Now switch to flat mode
|
|
run(`${path.join(installDir, 'bin', 'gstack-config')} set skill_prefix false`, {
|
|
GSTACK_INSTALL_DIR: installDir,
|
|
GSTACK_SKILLS_DIR: skillsDir,
|
|
});
|
|
run(`${path.join(installDir, 'bin', 'gstack-relink')}`, {
|
|
GSTACK_INSTALL_DIR: installDir,
|
|
GSTACK_SKILLS_DIR: skillsDir,
|
|
});
|
|
// Verify name: field is restored to unprefixed
|
|
expect(readSkillName(path.join(installDir, 'qa'))).toBe('qa');
|
|
expect(readSkillName(path.join(installDir, 'ship'))).toBe('ship');
|
|
});
|
|
|
|
test('gstack-upgrade name: not double-prefixed', () => {
|
|
setupMockInstall(['qa', 'gstack-upgrade']);
|
|
run(`${path.join(installDir, 'bin', 'gstack-config')} set skill_prefix true`, {
|
|
GSTACK_INSTALL_DIR: installDir,
|
|
GSTACK_SKILLS_DIR: skillsDir,
|
|
});
|
|
run(`${path.join(installDir, 'bin', 'gstack-relink')}`, {
|
|
GSTACK_INSTALL_DIR: installDir,
|
|
GSTACK_SKILLS_DIR: skillsDir,
|
|
});
|
|
// gstack-upgrade should keep its name, NOT become gstack-gstack-upgrade
|
|
expect(readSkillName(path.join(installDir, 'gstack-upgrade'))).toBe('gstack-upgrade');
|
|
// Regular skill should be prefixed
|
|
expect(readSkillName(path.join(installDir, 'qa'))).toBe('gstack-qa');
|
|
});
|
|
|
|
test('SKILL.md without frontmatter is a no-op', () => {
|
|
setupMockInstall(['qa']);
|
|
// Overwrite qa SKILL.md with no frontmatter
|
|
fs.writeFileSync(path.join(installDir, 'qa', 'SKILL.md'), '# qa\nSome content.');
|
|
run(`${path.join(installDir, 'bin', 'gstack-config')} set skill_prefix true`, {
|
|
GSTACK_INSTALL_DIR: installDir,
|
|
GSTACK_SKILLS_DIR: skillsDir,
|
|
});
|
|
// Should not crash
|
|
run(`${path.join(installDir, 'bin', 'gstack-relink')}`, {
|
|
GSTACK_INSTALL_DIR: installDir,
|
|
GSTACK_SKILLS_DIR: skillsDir,
|
|
});
|
|
// Content should be unchanged (no name: to patch)
|
|
const content = fs.readFileSync(path.join(installDir, 'qa', 'SKILL.md'), 'utf-8');
|
|
expect(content).toBe('# qa\nSome content.');
|
|
});
|
|
});
|