From 9eef1b791b4cdb6cfab8f34fb7edfa209635a9d5 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Wed, 2 Sep 2026 01:26:37 -0400 Subject: [PATCH] fix(#3981): give blocking guards a host-stall-proof timeout budget (#4175) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * test(#3981): blocking-guard timeout budget and 5→120 migration Fresh registration must carry a host-stall-proof 120 s budget on the six blocking PreToolUse guards; existing managed timeout:5 entries are migrated; non-managed entries and advisory budgets are untouched. * fix(#3981): give blocking guards a host-stall-proof timeout budget Claude Code treats a timed-out hook as non-blocking, so the 5 s budget on the six blocking PreToolUse guards silently dropped the gates exactly when the host stalled under load. Registers them at 120 s (covers every observed stall, max 84.3 s) and migrates existing managed timeout:5 entries in place, context-monitor-backfill shape. Advisory hook budgets are unchanged. * chore(#3981): backfill changeset pr number --------- Co-authored-by: sim --- .changeset/brave-eagles-greet.md | 5 + src/runtime-hooks-surface.cts | 44 +++++++-- tests/install-minimal-hooks.test.cjs | 135 +++++++++++++++++++++++++++ 3 files changed, 178 insertions(+), 6 deletions(-) create mode 100644 .changeset/brave-eagles-greet.md diff --git a/.changeset/brave-eagles-greet.md b/.changeset/brave-eagles-greet.md new file mode 100644 index 000000000..74cadb10d --- /dev/null +++ b/.changeset/brave-eagles-greet.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 4175 +--- +**Blocking guards no longer silently disable themselves when the host stalls** — the six blocking PreToolUse guards are registered (and migrated on existing installs) with a 120 s timeout instead of 5 s; Claude Code treats a timed-out hook as non-blocking, so the old budget dropped the gate exactly under load. (#3981) diff --git a/src/runtime-hooks-surface.cts b/src/runtime-hooks-surface.cts index 8f8208658..31ef90ce9 100644 --- a/src/runtime-hooks-surface.cts +++ b/src/runtime-hooks-surface.cts @@ -664,6 +664,7 @@ function resolveBashRunner(opts?: BashRunnerOpts): string | null { interface HookEntry { command?: string; args?: unknown[]; + timeout?: number; } interface HookGroup { @@ -2048,6 +2049,37 @@ function applySettingsJsonHooks(settings: any, opts: ApplySettingsJsonHooksOpts) settings.hooks.SessionStart = []; } + // #3981: Claude Code treats a timed-out hook as NON-blocking — the tool + // call continues through the normal permission flow. The blocking + // PreToolUse guards therefore need a budget a host stall cannot exceed, + // not one sized to the hook's own ~0.1 s runtime. Observed stalls reached + // 84.3 s; 120 s is the top of the issue's prescribed 60–120 range and + // returns every observed verdict. Registration below uses this constant, + // and the migration pass right here raises existing managed entries. + const BLOCKING_GUARD_TIMEOUT_S = 120; + const blockingGuardNames = [ + 'gsd-prompt-guard', + 'gsd-workflow-guard', + 'gsd-worktree-path-guard', + 'gsd-agent-isolation-guard', + 'gsd-write-guard', + 'gsd-validate-commit', + ]; + for (const entries of Object.values(settings.hooks as Record)) { + if (!Array.isArray(entries)) continue; + for (const entry of entries) { + if (!entry || !Array.isArray(entry.hooks)) continue; + for (const h of entry.hooks) { + if ( + blockingGuardNames.some((name) => referencesHook(h as Record, name)) && + h.timeout === 5 + ) { + h.timeout = BLOCKING_GUARD_TIMEOUT_S; + } + } + } + } + const hasGsdUpdateHook = settings.hooks.SessionStart.some((entry: HookGroup) => entry.hooks && entry.hooks.some((h: HookEntry) => referencesHook(h as Record, 'gsd-check-update')) ); @@ -2138,7 +2170,7 @@ function applySettingsJsonHooks(settings: any, opts: ApplySettingsJsonHooksOpts) { type: 'command', command: promptGuardCommand, - timeout: 5 + timeout: BLOCKING_GUARD_TIMEOUT_S } ] }); @@ -2224,7 +2256,7 @@ function applySettingsJsonHooks(settings: any, opts: ApplySettingsJsonHooksOpts) { type: 'command', command: workflowGuardCommand, - timeout: 5 + timeout: BLOCKING_GUARD_TIMEOUT_S } ] }); @@ -2251,7 +2283,7 @@ function applySettingsJsonHooks(settings: any, opts: ApplySettingsJsonHooksOpts) { type: 'command', command: worktreePathGuardCommand, - timeout: 5 + timeout: BLOCKING_GUARD_TIMEOUT_S } ] }); @@ -2283,7 +2315,7 @@ function applySettingsJsonHooks(settings: any, opts: ApplySettingsJsonHooksOpts) { type: 'command', command: agentIsolationGuardCommand, - timeout: 5 + timeout: BLOCKING_GUARD_TIMEOUT_S } ] }); @@ -2312,7 +2344,7 @@ function applySettingsJsonHooks(settings: any, opts: ApplySettingsJsonHooksOpts) { type: 'command', command: writeGuardCommand, - timeout: 5 + timeout: BLOCKING_GUARD_TIMEOUT_S } ] }); @@ -2339,7 +2371,7 @@ function applySettingsJsonHooks(settings: any, opts: ApplySettingsJsonHooksOpts) { type: 'command', command: validateCommitCommand, - timeout: 5 + timeout: BLOCKING_GUARD_TIMEOUT_S } ] }); diff --git a/tests/install-minimal-hooks.test.cjs b/tests/install-minimal-hooks.test.cjs index b8040e6c2..086c81966 100644 --- a/tests/install-minimal-hooks.test.cjs +++ b/tests/install-minimal-hooks.test.cjs @@ -3019,3 +3019,138 @@ describe('#3023 pi shared-hooks bundle avoids the host-reserved hooks/ directory }); } }); + +// ─── #3981: blocking PreToolUse guards must not fail open on a host stall ──── + +describe('bug #3981: blocking-guard timeout budget + migration', () => { + let targetDir; + beforeEach(() => { targetDir = createTempDir('gsd-3981-'); }); + afterEach(() => { cleanup(targetDir); }); + + // Local to this describe: the #1754 helpers above are scoped to their own + // describe, so re-require the seam and rebuild the runner here. + const { applySettingsJsonHooks } = require('../gsd-core/bin/lib/runtime-hooks-surface.cjs'); + const { captureConsole } = require('./helpers.cjs'); + function runApplySettingsJsonHooks(dir, presentHooks) { + fs.mkdirSync(path.join(dir, 'hooks'), { recursive: true }); + for (const hook of presentHooks) { + fs.writeFileSync(path.join(dir, 'hooks', hook), '// stub\n'); + } + const settings = {}; + const localCmd = (hookFile) => `node ${path.join(dir, 'hooks', hookFile)}`; + const localShellCmd = (hookFile) => `bash ${path.join(dir, 'hooks', hookFile)}`; + captureConsole(() => { + applySettingsJsonHooks(settings, { + runtime: 'claude', + isGlobal: false, + targetDir: dir, + postToolEvent: 'PostToolUse', + hookEvents: 'claude', + extendedHookEvents: [], + hooksSurface: 'settings-json', + updateCheckCommand: localCmd('gsd-check-update.js'), + contextMonitorCommand: localCmd('gsd-context-monitor.js'), + promptGuardCommand: localCmd('gsd-prompt-guard.js'), + readGuardCommand: localCmd('gsd-read-guard.js'), + readInjectionScannerCommand: localCmd('gsd-read-injection-scanner.js'), + configReloadCommand: null, + hookOpts: { portableHooks: false, runtime: 'claude' }, + localCmd, + localShellCmd, + }); + }); + return { settings }; + } + + const BLOCKING_GUARDS = [ + 'gsd-prompt-guard.js', + 'gsd-workflow-guard.js', + 'gsd-worktree-path-guard.js', + 'gsd-agent-isolation-guard.js', + 'gsd-write-guard.js', + 'gsd-validate-commit.sh', + ]; + + test('blocking PreToolUse guards register with a host-stall-proof timeout (#3981)', () => { + const { settings } = runApplySettingsJsonHooks(targetDir, ['gsd-prompt-guard.js']); + const entry = (settings.hooks.PreToolUse || []).find((e) => + (e.hooks || []).some((h) => h.command && h.command.includes('gsd-prompt-guard.js'))); + assert.ok(entry, 'prompt guard should be registered on a fresh install'); + const h = entry.hooks.find((x) => x.command.includes('gsd-prompt-guard.js')); + assert.equal(h.timeout, 120, + `Claude Code treats a timed-out hook as non-blocking, so a 5 s budget silently disables the gate under host stalls; expected 120, got ${h.timeout}`); + }); + + test('managed timeout:5 blocking-guard entries are migrated to 120 (#3981)', () => { + // Same shape as the context-monitor backfill: raising the source constant + // alone never reaches an existing settings.json, because registration + // skips entries whose command is already referenced. + for (const guard of BLOCKING_GUARDS) { + const settings = { + hooks: { + PreToolUse: [{ + matcher: 'Write|Edit', + hooks: [{ type: 'command', command: `node ${path.join(targetDir, 'hooks', guard)}`, timeout: 5 }], + }], + }, + }; + const localCmd = (hookFile) => `node ${path.join(targetDir, 'hooks', hookFile)}`; + captureConsole(() => { + applySettingsJsonHooks(settings, { + runtime: 'claude', + isGlobal: false, + targetDir, + postToolEvent: 'PostToolUse', + hookEvents: 'claude', + extendedHookEvents: [], + hooksSurface: 'settings-json', + updateCheckCommand: null, + contextMonitorCommand: null, + promptGuardCommand: null, + readGuardCommand: null, + readInjectionScannerCommand: null, + configReloadCommand: null, + hookOpts: { portableHooks: false, runtime: 'claude' }, + localCmd, + localShellCmd: localCmd, + }); + }); + const h = settings.hooks.PreToolUse[0].hooks[0]; + assert.equal(h.timeout, 120, + `existing managed ${guard} entry at timeout:5 must be migrated to 120, got ${h.timeout}`); + } + }); + + test('non-managed timeout:5 entries are left alone (#3981)', () => { + const mine = { type: 'command', command: 'node /usr/local/bin/my-own-hook.js', timeout: 5 }; + const settings = { hooks: { PreToolUse: [{ matcher: 'Write', hooks: [mine] }] } }; + const localCmd = (hookFile) => `node ${path.join(targetDir, 'hooks', hookFile)}`; + captureConsole(() => { + applySettingsJsonHooks(settings, { + runtime: 'claude', isGlobal: false, targetDir, + postToolEvent: 'PostToolUse', hookEvents: 'claude', extendedHookEvents: [], + hooksSurface: 'settings-json', updateCheckCommand: null, contextMonitorCommand: null, + promptGuardCommand: null, readGuardCommand: null, readInjectionScannerCommand: null, + configReloadCommand: null, hookOpts: { portableHooks: false, runtime: 'claude' }, + localCmd, localShellCmd: localCmd, + }); + }); + assert.equal(mine.timeout, 5, 'the migration must only touch entries referencing managed GSD guards'); + }); + + test('advisory hook budgets are unchanged (#3981)', () => { + const { settings } = runApplySettingsJsonHooks(targetDir, ['gsd-read-guard.js', 'gsd-context-monitor.js']); + const events = Object.values(settings.hooks).flat(); + const findTimeout = (basename) => { + for (const entry of events) { + if (!Array.isArray(entry.hooks)) continue; + for (const h of entry.hooks) { + if (h.command && h.command.includes(basename)) return h.timeout; + } + } + return undefined; + }; + assert.equal(findTimeout('gsd-read-guard.js'), 5, 'advisory read guard keeps its 5 s budget'); + assert.equal(findTimeout('gsd-context-monitor.js'), 10, 'context monitor keeps its 10 s budget'); + }); +});