From 5599e5a9cf4f3a932566806428e7defb84f29ab5 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Thu, 11 Jun 2026 11:36:03 -0400 Subject: [PATCH] fix(#1004): detect http-route hook registrations in installer presence check (#1032) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(#1004): detect http-route hook registrations in installer presence check referencesHook only inspected h.command and h.args, so a managed hook re-registered as a type:"http" entry (local hook-server routing) — whose identity lives only in h.url — was invisible. The installer then appended a stock command duplicate on every install/update, running the hook twice per event. Adds the h.url arm, mirroring the #976 args-form fix. Closes #1004 Co-Authored-By: Claude Opus 4.8 * chore(#1004): backfill changeset PR number to 1032 Co-Authored-By: Claude Opus 4.8 --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 --- .../1004-installer-http-hook-presence.md | 5 + bin/install.js | 17 +-- tests/install-regressions.test.cjs | 100 ++++++++++++++++++ 3 files changed, 115 insertions(+), 7 deletions(-) create mode 100644 .changeset/1004-installer-http-hook-presence.md diff --git a/.changeset/1004-installer-http-hook-presence.md b/.changeset/1004-installer-http-hook-presence.md new file mode 100644 index 000000000..3a763f4a9 --- /dev/null +++ b/.changeset/1004-installer-http-hook-presence.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 1032 +--- +**Installer no longer appends a duplicate managed hook when it is registered via an HTTP route** — a hook re-registered as a `type:"http"` entry (local hook-server routing) carries its identity only in `url`, which the installer's presence check ignored, so a stock command duplicate was appended on every install/update and the hook ran twice per event. The presence check now also inspects `h.url`. (#1004) diff --git a/bin/install.js b/bin/install.js index 168152276..f25ae7af4 100755 --- a/bin/install.js +++ b/bin/install.js @@ -11926,15 +11926,18 @@ function install(isGlobal, runtime = 'claude', options = {}) { } // Helper: detect whether a hook entry references a managed hook by name. - // Checks both the plain command string (standard form) and the args array - // (command+args / wrapped-launcher form used by windowless launchers on - // Windows and some custom PATH-less environments). Without this check the - // presence guards below only inspect h.command, so an args-form wrapper is - // invisible and a stock string-command entry is appended on every - // install/update, running the hook twice. (#976) + // Checks all three registration shapes: + // • plain command string (standard form) + // • args array (command+args / wrapped-launcher form used by windowless + // launchers on Windows and some custom PATH-less environments) (#976) + // • url field (type:"http" local-server routing form) (#1004) + // Without covering all three, an http-form or args-form entry is invisible + // and a stock string-command entry is appended on every install/update, + // running the hook twice. function referencesHook(h, hookName) { return (typeof h.command === 'string' && h.command.includes(hookName)) || - (Array.isArray(h.args) && h.args.some(a => typeof a === 'string' && a.includes(hookName))); + (Array.isArray(h.args) && h.args.some(a => typeof a === 'string' && a.includes(hookName))) || + (typeof h.url === 'string' && h.url.includes(hookName)); } // Configure SessionStart hook for update checking (skip for opencode) diff --git a/tests/install-regressions.test.cjs b/tests/install-regressions.test.cjs index ec21ab191..166981853 100644 --- a/tests/install-regressions.test.cjs +++ b/tests/install-regressions.test.cjs @@ -770,3 +770,103 @@ describe('#976 regression: installer does not duplicate managed hooks when regis ); }); }); + +// ─── #1004 — http-form hook presence detection ──────────────────────────────── +// +// Claude Code hooks support a type:"http" form where the hook identity lives +// in h.url (no command, no args). Pre-fix, referencesHook() only inspected +// h.command and h.args, so an http-form entry was invisible and a stock +// string-command entry was appended on every install, running the hook twice. +// This is the same duplicate-append failure as #976 (args-form), one shape further. + +describe('#1004 regression: installer does not duplicate managed hooks when registered in http form', () => { + let tmpDir; + let previousCwd; + + beforeEach(() => { + tmpDir = createTempDir('gsd-1004-http-form-'); + previousCwd = process.cwd(); + process.chdir(tmpDir); + + assert.strictEqual(typeof install, 'function', + 'install must be exported from bin/install.js'); + }); + + afterEach(() => { + process.chdir(previousCwd); + cleanup(tmpDir); + }); + + test('does not add a second SessionStart entry when gsd-check-update is already in http form', () => { + const targetDir = path.join(tmpDir, '.claude'); + fs.mkdirSync(targetDir, { recursive: true }); + + // Pass 1: run install with no pre-existing settings to create the + // gsd-file-manifest.json that the installer migration uses to decide + // whether a hook file is managed (kept) or foreign (removed). + // Without a manifest, migration removes any hook stubs as "unrecognized + // GSD-looking files", making fs.existsSync(checkUpdateFile) return false + // and skipping the duplicate-adding path. + install(false, 'claude'); + + // Now stub the hook files so fs.existsSync guards pass on pass 2. + // The manifest now exists, so migration classifies the stubs as + // manifest-managed and leaves them alone. + stubHooksIntoDir(targetDir, ['gsd-check-update.js']); + + // Local Claude installs read/write settings.local.json (not settings.json). + // Overwrite settings.local.json with the hook in http form. + // The GSD hook name appears only in h.url — no command, no args. + const hookUrl = 'http://127.0.0.1:18923/hooks/gsd-check-update'; + const preExistingSettings = { + hooks: { + SessionStart: [ + { + hooks: [ + { + type: 'http', + url: hookUrl, + timeout: 5, + }, + ], + }, + ], + }, + }; + fs.writeFileSync( + path.join(targetDir, 'settings.local.json'), + JSON.stringify(preExistingSettings, null, 2) + '\n', + ); + + // Pass 2: run install again — the pre-existing http-form entry must + // suppress the duplicate stock string-command registration. + const result = install(false, 'claude'); + const settings = result && result.settings; + + assert.ok(settings && settings.hooks && Array.isArray(settings.hooks.SessionStart), + 'settings.hooks.SessionStart must be an array after install'); + + // Count all hook entries (at any nesting level) that reference gsd-check-update, + // including the url arm so http-form entries are visible. + const allEntries = settings.hooks.SessionStart.flatMap(entry => + Array.isArray(entry && entry.hooks) ? entry.hooks : [] + ); + const matching = allEntries.filter(h => + (typeof h.command === 'string' && h.command.includes('gsd-check-update')) || + (Array.isArray(h.args) && h.args.some(a => typeof a === 'string' && a.includes('gsd-check-update'))) || + (typeof h.url === 'string' && h.url.includes('gsd-check-update')) + ); + + assert.strictEqual( + matching.length, + 1, + [ + 'Expected exactly 1 hook entry referencing gsd-check-update after install,', + `got ${matching.length}.`, + 'The installer added a duplicate because it could not detect the http-form registration.', + 'referencesHook() must check h.url in addition to h.command and h.args. (#1004)', + `All matching entries: ${JSON.stringify(matching)}`, + ].join(' '), + ); + }); +});