diff --git a/CHANGELOG.md b/CHANGELOG.md index c63216445..82231d721 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,25 @@ # Changelog +## [1.91.218.0] - 2026-10-25 + +**`gstack-team-init required` now blocks skills when its gate can't run, instead of letting them through.** Issue #3319 (exGeni): the required-mode Skill hook is a `node -e` launcher. On a teammate's Mac or Linux machine without Node.js, the shell exits 127 with nothing on stdout. If `.claude/hooks/check-gstack.cjs` is missing, node exits 1. Claude Code treats any exit other than 0 or 2 as a non-blocking error, so in both cases the Skill call went ahead on exactly the machine the gate is for. A live Claude Code 2.1.310 session with node off PATH and no gstack install ran the skill. + +### Fixed +- **`bin/gstack-team-init`**: the check-gstack handler is registered with `"onFailure": "block"`, Claude Code's documented fail-closed switch (2.1.295+). A launcher that can't start, or exits with any code other than 0 or 2, now blocks the Skill call. The reason shown is `blocking because onFailure is "block"` plus the shell's `node: not found` (or node's `Cannot find module …check-gstack.cjs`). Exit 2 (deny) and exit 0 (allow) are unchanged. Re-running `required` upgrades an existing registration in place, and the manual-registration fallback names the field. +- The required CLAUDE.md section tells the agent what those two block reasons mean (install Node.js; the hook file is missing) and not to work around them. `required` prints that every developer needs node on PATH and Claude Code 2.1.295+, and the README says the same. + +### Compatibility +- Measured with a live `claude -p` Skill call per version. 2.1.295 and 2.1.310 block when node is missing or the hook file is gone, still deny with exit 2 when gstack is absent, and still run the skill when gstack is installed. 2.1.294 ignores the unknown field without rejecting the settings file: the hook still loads and denies as before, and a machine without node is still let through there, as it was before this change. +- Copilot CLI documents command `preToolUse` hooks as already failing closed on a crash or non-zero exit, so this change does not alter its outcome. Its handling of the extra field was not measured (no Copilot login on the test machine). + +### Tests +- `test/gstack-team-init-hook-schema.test.ts`: the handler is `{ type, command, onFailure: 'block' }`. The registered command exits 127 with no stdout when node is off PATH and 1 when check-gstack.cjs is missing, the failures only `onFailure` turns into a block. Re-running upgrades a handler registered without the field to exactly one handler with it. +- `test/team-init-hook-shell-neutral.test.ts`: through the platform's default shell, the command without node exits neither 0 nor 2 with empty stdout, and the bin registers the handler with `onFailure: 'block'`. The PowerShell case and its retries are unchanged. +- `test/team-mode.test.ts`: the legacy-upgrade assertion pins the new field. + +### Gate integrity +- One gate edit, and it tightens the gate: the required-mode Skill hook goes from fail-open to fail-closed when its launcher fails. Nothing that used to block now allows. Negative controls: with `onFailure` removed from the bin, 4 tests fail (the three new registration and upgrade tests plus the team-mode pin); with it dropped only from the in-place upgrade path, 2 fail. The live runs with gstack installed (2.1.294, 2.1.310) still run the skill, so the allow path is not broken. No threshold, budget, timeout, retry or trial count changed. + ## [1.91.217.0] - 2026-10-25 **Windows free tests now also run on every push to main.** `windows-free-tests.yml` ran on pull requests and dispatch only: 194 PR runs, 6 dispatches and 0 pushes in its last 200 runs. Merge trains dispatch only `free-tests.yml` on the train branch. So no Windows run ever saw a train's combined tree or main after a merge, and a Windows break caused by two individually green PRs only surfaced on the next unrelated PR. Found while working on GSTA-68. diff --git a/README.md b/README.md index f09877d66..213da088e 100644 --- a/README.md +++ b/README.md @@ -66,6 +66,8 @@ No vendored files in your repo, no version drift, no manual upgrades. Every Clau Swap `required` for `optional` if you'd rather nudge teammates than block them. +`required` blocks skills with a hook that runs on [Node.js](https://nodejs.org/), so every teammate needs Node.js on PATH, on every OS. With Claude Code 2.1.295 or later the hook also blocks when it can't run (no Node.js, or a missing `.claude/hooks/check-gstack.cjs`); older Claude Code lets skills through in that case. + ### OpenClaw OpenClaw spawns Claude Code sessions via ACP, so every gstack skill just works diff --git a/VERSION b/VERSION index 9df4ba64f..178012b22 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.91.217.0 +1.91.218.0 diff --git a/agents-digest/gstack-AGENTS.md b/agents-digest/gstack-AGENTS.md index a188bc268..901e289b4 100644 --- a/agents-digest/gstack-AGENTS.md +++ b/agents-digest/gstack-AGENTS.md @@ -1,4 +1,4 @@ -# gstack digest v1.91.217.0 — regenerate/re-copy after upgrading gstack +# gstack digest v1.91.218.0 — regenerate/re-copy after upgrading gstack Behavioral rules from gstack (https://github.com/garrytan/gstack), compressed for agent hosts without a full skill install. The full skills add workflows, diff --git a/bin/gstack-team-init b/bin/gstack-team-init index 3598e2672..901f5999b 100755 --- a/bin/gstack-team-init +++ b/bin/gstack-team-init @@ -102,7 +102,13 @@ Do not skip skills, ignore gstack errors, or work around missing gstack. Using gstack skills: After install, skills like /qa, /ship, /review, /investigate, and /browse are available. Use /browse for all web browsing (Aside first, the bundled gstack browser as fallback). Use the resolved install path above for gstack file paths -(default: ~/.claude/skills/gstack).' +(default: ~/.claude/skills/gstack). + +The Skill gate in .claude/settings.json runs .claude/hooks/check-gstack.cjs with +Node.js and blocks when it cannot run. If a skill is blocked with "node: not +found", tell the user to install Node.js (https://nodejs.org); if it is blocked +with "Cannot find module ... check-gstack.cjs", the hook file is missing from +this checkout. Do not work around either block.' fi # Check if CLAUDE.md already has a gstack section @@ -176,15 +182,20 @@ HOOK_EOF GENERATED+=(".claude/hooks/check-gstack.cjs") echo " + .claude/hooks/check-gstack.cjs — enforcement hook" - # Add hook to project-level settings.json + # Add hook to project-level settings.json. The command exits 0 (allow) or + # 2 (deny) only when node runs check-gstack.cjs. Without node the shell exits + # 127, and a require error exits 1; Claude Code treats any other exit as a + # non-blocking error and lets the Skill call through, so the handler is + # registered with onFailure: block (Claude Code 2.1.295+) to fail closed + # (#3319). Older Claude Code ignores the field and keeps the old behavior. HOOK_COMMAND="node -e \"require(require('path').resolve(process.env.CLAUDE_PROJECT_DIR || '.', '.claude', 'hooks', 'check-gstack.cjs'))\"" if ! command -v bun >/dev/null 2>&1; then - echo " ! bun not found — manually add this PreToolUse hook (matcher Skill|skill) to .claude/settings.json:" + echo " ! bun not found — manually add this PreToolUse hook (matcher Skill|skill, \"onFailure\": \"block\") to .claude/settings.json:" echo " $HOOK_COMMAND" elif GSTACK_SETTINGS_PATH="$SETTINGS" GSTACK_HOOK_COMMAND="$HOOK_COMMAND" bun --no-env-file --config=/dev/null -e " const fs = require('fs'); const settingsPath = process.env.GSTACK_SETTINGS_PATH; - const command = process.env.GSTACK_HOOK_COMMAND; + const handler = { type: 'command', command: process.env.GSTACK_HOOK_COMMAND, onFailure: 'block' }; let settings = {}; try { settings = JSON.parse(fs.readFileSync(settingsPath, 'utf8')); } catch {} @@ -196,7 +207,8 @@ HOOK_EOF // Claude tool name maps to, so a bare 'Skill' never fired there (#2229). // A registration an older team-init wrote (the .sh hook, matcher Skill) // is replaced in place: exactly one check-gstack handler remains, and - // unrelated hooks in the same entry are kept. + // unrelated hooks in the same entry are kept. Re-running upgrades a + // handler registered without onFailure. const MATCHER = 'Skill|skill'; const isOurs = h => h && typeof h.command === 'string' && h.command.includes('check-gstack'); let placed = false; @@ -207,12 +219,12 @@ HOOK_EOF const kept = []; for (const h of entry.hooks) { if (!isOurs(h)) kept.push(h); - else if (!placed) { kept.push({ type: 'command', command }); placed = true; } + else if (!placed) { kept.push({ ...handler }); placed = true; } } entry.hooks = kept; return kept.length > 0; }); - if (!placed) settings.hooks.PreToolUse.push({ matcher: MATCHER, hooks: [{ type: 'command', command }] }); + if (!placed) settings.hooks.PreToolUse.push({ matcher: MATCHER, hooks: [{ ...handler }] }); const tmp = settingsPath + '.tmp'; fs.writeFileSync(tmp, JSON.stringify(settings, null, 2) + '\n'); @@ -229,7 +241,7 @@ HOOK_EOF echo " - .claude/hooks/check-gstack.sh — replaced by check-gstack.cjs" fi else - echo " ! could not update .claude/settings.json — add this PreToolUse hook (matcher Skill|skill) manually:" + echo " ! could not update .claude/settings.json — add this PreToolUse hook (matcher Skill|skill, \"onFailure\": \"block\") manually:" echo " $HOOK_COMMAND" fi fi @@ -248,3 +260,9 @@ echo "" echo "Each developer then runs:" echo " git clone --depth 1 https://github.com/garrytan/gstack.git ~/.claude/skills/gstack" echo " cd ~/.claude/skills/gstack && ./setup --team" +if [ "$MODE" = "required" ]; then + echo "" + echo "The Skill gate runs on Node.js and blocks when it cannot run, so every" + echo "developer also needs node on PATH and Claude Code 2.1.295 or later." + echo "Older Claude Code ignores onFailure: without node there, skills are not blocked." +fi diff --git a/package.json b/package.json index a6ce78121..adbaada2b 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "gstack", - "version": "1.91.217", + "version": "1.91.218", "description": "Garry's Stack — Claude Code skills + fast headless browser. One repo, one install, entire AI engineering workflow.", "license": "MIT", "type": "module", diff --git a/test/gstack-team-init-hook-schema.test.ts b/test/gstack-team-init-hook-schema.test.ts index ee8cef78e..10feb4a14 100644 --- a/test/gstack-team-init-hook-schema.test.ts +++ b/test/gstack-team-init-hook-schema.test.ts @@ -84,3 +84,80 @@ describe('gstack-team-init required: PreToolUse hook schema (#2413)', () => { expect(JSON.parse(stdout.trim())).toEqual({}); }); }); + +// #3319: when the launcher itself cannot run, the hook prints nothing and exits +// with neither 0 nor 2, which Claude Code treats as a non-blocking error. The +// handler is registered with onFailure: block (Claude Code 2.1.295+) so those +// failures block the Skill call instead of letting it through. +describe('gstack-team-init required: the hook fails closed (#3319)', () => { + let repo: string; + let home: string; + let noNodeBin: string; + + beforeEach(() => { + repo = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-team-init-closed-')); + execFileSync('git', ['init', '-q'], { cwd: repo }); + home = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-team-init-closed-home-')); + noNodeBin = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-team-init-no-node-')); + }); + + afterEach(() => { + for (const dir of [repo, home, noNodeBin]) fs.rmSync(dir, { recursive: true, force: true }); + }); + + const init = () => execFileSync(TEAM_INIT, ['required'], { cwd: repo, encoding: 'utf-8', timeout: 30_000 }); + const settingsPath = () => path.join(repo, '.claude', 'settings.json'); + const ourHandlers = () => JSON.parse(fs.readFileSync(settingsPath(), 'utf-8')).hooks.PreToolUse + .flatMap((e: { hooks: { command: string }[] }) => e.hooks) + .filter((h: { command: string }) => h.command.includes('check-gstack')); + + function runRegistered(pathEnv: string): { status: number; stdout: string } { + const [{ command }] = ourHandlers(); + try { + const stdout = execSync(command, { + env: { PATH: pathEnv, HOME: home, CLAUDE_PROJECT_DIR: repo }, + encoding: 'utf-8', + stdio: ['ignore', 'pipe', 'pipe'], + timeout: 30_000, + }); + return { status: 0, stdout }; + } catch (err) { + const e = err as { status: number; stdout: string }; + return { status: e.status, stdout: e.stdout }; + } + } + + test('the check-gstack handler is registered with onFailure: block', () => { + init(); + const handlers = ourHandlers(); + expect(handlers).toHaveLength(1); + expect(handlers[0]).toEqual({ type: 'command', command: handlers[0].command, onFailure: 'block' }); + }); + + test('node missing from PATH: no output and exit 127, a failure only onFailure turns into a block', () => { + init(); + const { status, stdout } = runRegistered(noNodeBin); + expect(status).toBe(127); + expect(stdout).toBe(''); + }); + + test('check-gstack.cjs missing: no output and exit 1, a failure only onFailure turns into a block', () => { + init(); + fs.rmSync(path.join(repo, '.claude', 'hooks', 'check-gstack.cjs')); + const { status, stdout } = runRegistered(process.env.PATH!); + expect(status).toBe(1); + expect(stdout).toBe(''); + }); + + test('re-running upgrades a handler registered without onFailure, keeping one handler', () => { + init(); + const settings = JSON.parse(fs.readFileSync(settingsPath(), 'utf-8')); + const [{ command }] = ourHandlers(); + settings.hooks.PreToolUse = [{ matcher: 'Skill|skill', hooks: [{ type: 'command', command }] }]; + fs.writeFileSync(settingsPath(), JSON.stringify(settings)); + + const out = init(); + expect(ourHandlers()).toEqual([{ type: 'command', command, onFailure: 'block' }]); + expect(out).toContain('Claude Code 2.1.295 or later'); + }); +}); diff --git a/test/team-init-hook-shell-neutral.test.ts b/test/team-init-hook-shell-neutral.test.ts index 385c4003d..284de72ba 100644 --- a/test/team-init-hook-shell-neutral.test.ts +++ b/test/team-init-hook-shell-neutral.test.ts @@ -53,6 +53,17 @@ describe('required team hook: shell-neutral registration', () => { expect(command).not.toContain('%'); }); + // #3319: without node the shell's own error is all the runner gets, which is + // neither exit 0 nor 2, so the registration must carry onFailure: block. + test('without node on PATH the default shell exits neither 0 nor 2, so the handler is registered with onFailure: block', () => { + expect(TEAM_INIT).toContain("const handler = { type: 'command', command: process.env.GSTACK_HOOK_COMMAND, onFailure: 'block' };"); + const noNode = path.join(base, 'no-node-bin'); + fs.mkdirSync(noNode, { recursive: true }); + const failed = viaDefaultShell({ ...hookEnv({ HOME: elsewhere, USERPROFILE: elsewhere }), PATH: noNode, Path: noNode }); + expect(failed.stdout.trim()).toBe(''); + expect([0, 2]).not.toContain(failed.status); + }); + // `powershell -Command` reports a failing native command as exit 1, so only // the default shell sees the hook's own exit 2; the deny JSON is the same. for (const [name, runner, enabled, denyStatus] of [ diff --git a/test/team-mode.test.ts b/test/team-mode.test.ts index 1c55cc908..efd4ff0f0 100644 --- a/test/team-mode.test.ts +++ b/test/team-mode.test.ts @@ -329,7 +329,7 @@ describe('gstack-team-init', () => { const pre = JSON.parse(fs.readFileSync(path.join(tmpDir, '.claude', 'settings.json'), 'utf-8')).hooks.PreToolUse; expect(pre).toEqual([ - { matcher: 'Skill|skill', hooks: [{ type: 'command', command: 'team-audit' }, { type: 'command', command: registeredCommand() }] }, + { matcher: 'Skill|skill', hooks: [{ type: 'command', command: 'team-audit' }, { type: 'command', command: registeredCommand(), onFailure: 'block' }] }, { matcher: 'Bash', hooks: [{ type: 'command', command: 'my-own-hook' }] }, ]); expect(registeredCommand()).not.toContain('$');