mirror of
https://github.com/garrytan/gstack.git
synced 2026-10-31 22:14:25 +01:00
Without node on PATH the node -e launcher exits 127, and with check-gstack.cjs missing it exits 1; Claude Code treats both as non-blocking and lets the Skill call through. Register the handler with onFailure: block (Claude Code 2.1.295+), upgrade existing registrations in place, and tell the agent and the developer what the two block reasons mean. Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This commit is contained in:
1 parent
2d27b482d2
commit
d73b547066
9 files changed
+140
-12
No files matched your search
@@ -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.
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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,
|
||||
|
||||
+26
-8
@@ -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
|
||||
+1
-1
@@ -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",
|
||||
|
||||
@@ -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');
|
||||
});
|
||||
});
|
||||
@@ -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 [
|
||||
|
||||
@@ -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('$');
|
||||
|
||||
Reference in new issue
Block a user