mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-01 06:24:41 +01:00
C7: required team mode also blocks Copilot CLI's lowercase skill tool
Copilot CLI applies Claude-format matchers by tool name; its skill tool is `skill`, which no Claude tool name maps to, so the 'Skill' matcher never fired and Copilot ran protected skills without gstack. The hook entry now matches 'Skill|skill', an entry an older team-init wrote is upgraded in place, and the deny is emitted in both decision formats (Copilot reads the top-level permissionDecision, Claude Code reads hookSpecificOutput) with exit 0 so both parse it. Test migration: the settings test pinned matcher 'Skill'. Co-authored-by: neallee <neallee@gmail.com>
This commit is contained in:
1 parent
68cabd06a6
commit
8e4bb7db13
2 files changed
+48
-9
No files matched your search
+13
-7
@@ -151,8 +151,10 @@ Install it:
|
||||
|
||||
Then restart your AI coding tool.
|
||||
MSG
|
||||
echo '{"hookSpecificOutput":{"hookEventName":"PreToolUse","permissionDecision":"deny","permissionDecisionReason":"gstack is required but not installed. See stderr for install instructions."}}'
|
||||
exit 2
|
||||
# Claude Code reads hookSpecificOutput; Copilot CLI reads the top-level
|
||||
# fields (#2229). Exit 0 so both parse the decision instead of an error.
|
||||
echo '{"permissionDecision":"deny","permissionDecisionReason":"gstack is required but not installed. See stderr for install instructions.","hookSpecificOutput":{"hookEventName":"PreToolUse","permissionDecision":"deny","permissionDecisionReason":"gstack is required but not installed. See stderr for install instructions."}}'
|
||||
exit 0
|
||||
fi
|
||||
|
||||
echo '{}'
|
||||
@@ -173,15 +175,19 @@ HOOK_EOF
|
||||
if (!settings.hooks) settings.hooks = {};
|
||||
if (!settings.hooks.PreToolUse) settings.hooks.PreToolUse = [];
|
||||
|
||||
// Dedup
|
||||
const exists = settings.hooks.PreToolUse.some(entry =>
|
||||
entry.matcher === 'Skill' &&
|
||||
// Claude Code's skill tool is Skill; Copilot CLI's is skill, which no
|
||||
// Claude tool name maps to, so a bare 'Skill' never fired there (#2229).
|
||||
// An entry an older team-init wrote is upgraded in place, never duplicated.
|
||||
const MATCHER = 'Skill|skill';
|
||||
const ours = settings.hooks.PreToolUse.filter(entry =>
|
||||
(entry.matcher === 'Skill' || entry.matcher === MATCHER) &&
|
||||
entry.hooks && entry.hooks.some(h => h.command && h.command.includes('check-gstack'))
|
||||
);
|
||||
for (const entry of ours) entry.matcher = MATCHER;
|
||||
|
||||
if (!exists) {
|
||||
if (ours.length === 0) {
|
||||
settings.hooks.PreToolUse.push({
|
||||
matcher: 'Skill',
|
||||
matcher: MATCHER,
|
||||
hooks: [{
|
||||
type: 'command',
|
||||
command: '\"\$CLAUDE_PROJECT_DIR/.claude/hooks/check-gstack.sh\"'
|
||||
|
||||
+35
-2
@@ -2,7 +2,7 @@ import { describe, test, expect, beforeEach, afterEach } from 'bun:test';
|
||||
import * as fs from 'fs';
|
||||
import * as path from 'path';
|
||||
import * as os from 'os';
|
||||
import { execSync } from 'child_process';
|
||||
import { execSync, spawnSync } from 'child_process';
|
||||
import { createHash } from 'crypto';
|
||||
import { runBashScript } from './helpers/bash-script';
|
||||
|
||||
@@ -248,10 +248,43 @@ describe('gstack-team-init', () => {
|
||||
expect(fs.existsSync(settingsPath)).toBe(true);
|
||||
const settings = JSON.parse(fs.readFileSync(settingsPath, 'utf-8'));
|
||||
expect(settings.hooks.PreToolUse).toHaveLength(1);
|
||||
expect(settings.hooks.PreToolUse[0].matcher).toBe('Skill');
|
||||
// C7 (#2229): Copilot CLI's skill tool is lowercase `skill`; Claude-format
|
||||
// matchers fire when an alternation token equals the runtime tool name.
|
||||
expect(settings.hooks.PreToolUse[0].matcher.split('|').sort()).toEqual(['Skill', 'skill']);
|
||||
expect(settings.hooks.PreToolUse[0].hooks[0].command).toContain('check-gstack');
|
||||
});
|
||||
|
||||
test('required: a Skill-only entry from an older team-init is upgraded, not duplicated (C7)', () => {
|
||||
const settingsPath = path.join(tmpDir, '.claude', 'settings.json');
|
||||
fs.mkdirSync(path.dirname(settingsPath), { recursive: true });
|
||||
fs.writeFileSync(settingsPath, JSON.stringify({ hooks: { PreToolUse: [
|
||||
{ matcher: 'Bash', hooks: [{ type: 'command', command: 'my-own-hook' }] },
|
||||
{ matcher: 'Skill', hooks: [{ type: 'command', command: '"$CLAUDE_PROJECT_DIR/.claude/hooks/check-gstack.sh"' }] },
|
||||
] } }));
|
||||
run(`${TEAM_INIT} required`, { cwd: tmpDir });
|
||||
run(`${TEAM_INIT} required`, { cwd: tmpDir });
|
||||
const pre = JSON.parse(fs.readFileSync(settingsPath, 'utf-8')).hooks.PreToolUse;
|
||||
expect(pre.map((e: { matcher: string }) => e.matcher)).toEqual(['Bash', 'Skill|skill']);
|
||||
});
|
||||
|
||||
test('required: the hook denies in both Claude Code and Copilot CLI decision formats (C7)', () => {
|
||||
run(`${TEAM_INIT} required`, { cwd: tmpDir });
|
||||
const home = mkTmpDir();
|
||||
try {
|
||||
const r = spawnSync('bash', [path.join(tmpDir, '.claude', 'hooks', 'check-gstack.sh')], {
|
||||
encoding: 'utf-8', timeout: 30_000, env: { PATH: process.env.PATH ?? '/usr/bin:/bin', HOME: home },
|
||||
});
|
||||
expect(r.status).toBe(0);
|
||||
const out = JSON.parse(r.stdout);
|
||||
expect(out.permissionDecision).toBe('deny');
|
||||
expect(out.permissionDecisionReason).toContain('gstack is required');
|
||||
expect(out.hookSpecificOutput).toMatchObject({ hookEventName: 'PreToolUse', permissionDecision: 'deny' });
|
||||
expect(r.stderr).toContain('BLOCKED: gstack is not installed');
|
||||
} finally {
|
||||
fs.rmSync(home, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
test('idempotent: running twice does not duplicate CLAUDE.md section', () => {
|
||||
run(`${TEAM_INIT} optional`, { cwd: tmpDir });
|
||||
run(`${TEAM_INIT} optional`, { cwd: tmpDir });
|
||||
|
||||
Reference in new issue
Block a user