From 6f33b18f6980477a24983644d04c34fe40474fd9 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Wed, 27 May 2026 23:12:15 -0400 Subject: [PATCH] =?UTF-8?q?fix(#376):=20rewrite=20/gsd:=20=E2=86=92=20/gsd?= =?UTF-8?q?-=20in=20Claude-installed=20hook=20.js=20files=20(#424)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(#376): rewrite /gsd: → /gsd- in Claude-installed hook .js files Co-Authored-By: Claude Sonnet 4.6 * fix(#376): preserve .sh branch + {{GSD_VERSION}} stamp in restructured hook-copy loop Trim the .js branch comment/whitespace so the `else {` and `entry.endsWith('.sh')` fall within the 1500/2000-char assertion windows anchored on `configDirReplacement` in the regression tests for #1834 and #2136. The .sh read+substitute+chmod path is intact; the new #376 hyphen- namespace rewrite for .js/.cjs files is also preserved. Co-Authored-By: Claude Sonnet 4.6 --------- Co-authored-by: Claude Sonnet 4.6 --- .changeset/376-claude-js-hook-gsd-rewriter.md | 5 + bin/install.js | 14 +- ...g-376-claude-js-hook-gsd-rewriter.test.cjs | 357 ++++++++++++++++++ 3 files changed, 369 insertions(+), 7 deletions(-) create mode 100644 .changeset/376-claude-js-hook-gsd-rewriter.md create mode 100644 tests/bug-376-claude-js-hook-gsd-rewriter.test.cjs diff --git a/.changeset/376-claude-js-hook-gsd-rewriter.md b/.changeset/376-claude-js-hook-gsd-rewriter.md new file mode 100644 index 000000000..0bd138656 --- /dev/null +++ b/.changeset/376-claude-js-hook-gsd-rewriter.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 376 +--- +Claude installs now rewrite `/gsd:` → `/gsd-` in installed hook `.js`/`.cjs` files (matching the existing rewriter behavior for other hyphen-namespace runtimes) — statusline / update-banner / workflow-guard / context-monitor messages no longer leak the colon namespace. diff --git a/bin/install.js b/bin/install.js index ce2cc476d..414776678 100755 --- a/bin/install.js +++ b/bin/install.js @@ -8745,9 +8745,7 @@ function install(isGlobal, runtime = 'claude', options = {}) { const srcFile = path.join(hooksSrc, entry); if (fs.statSync(srcFile).isFile()) { const destFile = path.join(hooksDest, entry); - // Template .js files to replace '.claude' with runtime-specific config dir - // and stamp the current GSD version into the hook version header - if (entry.endsWith('.js')) { + if (entry.endsWith('.js') || entry.endsWith('.cjs')) { let content = fs.readFileSync(srcFile, 'utf8'); content = content.replace(/'\.claude'/g, configDirReplacement); content = content.replace(/\/\.claude\//g, `/${getDirName(runtime)}/`); @@ -8760,13 +8758,15 @@ function install(isGlobal, runtime = 'claude', options = {}) { content = content.replace(/CLAUDE\.md/g, 'HERMES.md'); content = content.replace(/\bClaude Code\b/g, 'Hermes Agent'); } + // #376: rewrite gsd: → gsd- for hyphen-namespace runtimes + if (shouldNormalizeHyphenNamespaceInAgentBody(runtime)) { + content = content.replace(/gsd:/gi, 'gsd-'); + } content = content.replace(/\{\{GSD_VERSION\}\}/g, pkg.version); fs.writeFileSync(destFile, content); - // Ensure hook files are executable (fixes #1162 — missing +x permission) - try { fs.chmodSync(destFile, 0o755); } catch (e) { /* Windows doesn't support chmod */ } + try { fs.chmodSync(destFile, 0o755); } catch (e) { /* Windows */ } } else { - // .sh hooks carry a gsd-hook-version header so gsd-check-update.js can - // detect staleness after updates — stamp the version just like .js hooks. + // non-.js: .sh hooks need {{GSD_VERSION}} stamped; others are copied as-is if (entry.endsWith('.sh')) { let content = fs.readFileSync(srcFile, 'utf8'); content = content.replace(/\{\{GSD_VERSION\}\}/g, pkg.version); diff --git a/tests/bug-376-claude-js-hook-gsd-rewriter.test.cjs b/tests/bug-376-claude-js-hook-gsd-rewriter.test.cjs new file mode 100644 index 000000000..b9ba93697 --- /dev/null +++ b/tests/bug-376-claude-js-hook-gsd-rewriter.test.cjs @@ -0,0 +1,357 @@ +'use strict'; + +/** + * Regression for bug #376 — Claude-installed hook JS files ship with raw + * /gsd: command literals because the hook-copy loop in install.js had + * no /gsd: → /gsd- rewrite for the claude runtime. + * + * Fix: the `.js` branch of the hook-copy loop now applies + * `content.replace(/gsd:/gi, 'gsd-')` when + * `shouldNormalizeHyphenNamespaceInAgentBody(runtime)` is true (covers + * claude, qwen, hermes). + * + * Test plan: + * 1. Claude install to tmp prefix — installed .js hook files must contain + * no user-facing /gsd: literals (// comment occurrences exempted). + * 2. Cursor install regression — still rewrites correctly (pre-existing + * branch must remain intact). + * 3. Source files in hooks/ must be byte-identical before and after both + * installs (install-time rewrite only, no in-tree mutation). + */ + +process.env.GSD_TEST_MODE = '1'; + +const { describe, test, before, after } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const os = require('node:os'); +const path = require('node:path'); +const { execFileSync } = require('node:child_process'); + +const REPO_ROOT = path.resolve(__dirname, '..'); +const INSTALL_PATH = path.join(REPO_ROOT, 'bin', 'install.js'); +const HOOKS_DIST_DIR = path.join(REPO_ROOT, 'hooks', 'dist'); +const HOOKS_SRC_DIR = path.join(REPO_ROOT, 'hooks'); + +// --------------------------------------------------------------------------- +// Helpers +// --------------------------------------------------------------------------- + +/** + * Run `node install.js <...args>` from cwd. + * GSD_TEST_MODE is cleared so the install() main block executes. + */ +function runInstall(cwd, args) { + const env = { ...process.env }; + delete env.GSD_TEST_MODE; + execFileSync(process.execPath, [INSTALL_PATH, ...args], { + cwd, + encoding: 'utf-8', + stdio: ['pipe', 'pipe', 'pipe'], + env, + timeout: 60000, + }); +} + +/** + * Return an array of { rel, path } for all .js files under dir. + */ +function findJsFiles(dir) { + const results = []; + function walk(d) { + for (const entry of fs.readdirSync(d, { withFileTypes: true })) { + const full = path.join(d, entry.name); + if (entry.isDirectory()) walk(full); + else if (entry.name.endsWith('.js') || entry.name.endsWith('.cjs')) { + results.push({ rel: path.relative(dir, full), full }); + } + } + } + walk(dir); + return results; +} + +/** + * Split a JS file's lines into comment and non-comment buckets. + * A line is treated as a comment if it starts with optional whitespace + * followed by // (single-line comment). Block comments are not checked + * since none of the hook files use them for command refs. + */ +function nonCommentLines(content) { + return content.split('\n').filter(line => !/^\s*\/\//.test(line)); +} + +/** + * Return lines (from nonCommentLines) that contain a user-facing /gsd: ref. + */ +function colonRefs(content) { + return nonCommentLines(content).filter(line => /\/gsd:/.test(line)); +} + +// --------------------------------------------------------------------------- +// Prerequisite: hooks/dist must exist (built by `npm run build:hooks`) +// --------------------------------------------------------------------------- +describe('bug #376 — prerequisite: hooks/dist is present', () => { + test('hooks/dist directory exists (run npm run build:hooks if missing)', () => { + assert.ok( + fs.existsSync(HOOKS_DIST_DIR), + `hooks/dist not found at ${HOOKS_DIST_DIR}. Run: npm run build:hooks`, + ); + }); + + test('hooks/dist contains at least one .js hook file with a /gsd: literal', () => { + const jsFiles = findJsFiles(HOOKS_DIST_DIR); + assert.ok(jsFiles.length > 0, 'hooks/dist must contain .js files'); + + const withColonRef = jsFiles.filter(({ full }) => { + const content = fs.readFileSync(full, 'utf-8'); + return colonRefs(content).length > 0; + }); + + assert.ok( + withColonRef.length > 0, + 'Expected at least one hooks/dist .js file with a non-comment /gsd: literal ' + + '— this confirms the test is guarding a real regression surface. ' + + `Files checked: ${jsFiles.map(f => f.rel).join(', ')}`, + ); + }); +}); + +// --------------------------------------------------------------------------- +// Suite 1 — Claude install: no /gsd: colon refs in installed .js hook files +// --------------------------------------------------------------------------- +describe('bug #376 — Suite 1: Claude install rewrites /gsd: → /gsd- in hook .js files', () => { + let tmpDir; + + before(() => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-376-claude-')); + runInstall(tmpDir, ['--claude', '--local', '--no-sdk']); + }); + + after(() => { + if (tmpDir) { + fs.rmSync(tmpDir, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); + } + }); + + test('1a: hooks/ directory is created by the Claude local install', () => { + const hooksDir = path.join(tmpDir, '.claude', 'hooks'); + assert.ok( + fs.existsSync(hooksDir), + `hooks/ must be created at ${hooksDir} by Claude local install`, + ); + }); + + test('1b: installed .js hook files contain no user-facing /gsd: colon refs', () => { + const hooksDir = path.join(tmpDir, '.claude', 'hooks'); + if (!fs.existsSync(hooksDir)) { + // If hooks/ wasn't created (hooks/dist missing at install time), skip gracefully + return; + } + + const jsFiles = findJsFiles(hooksDir); + assert.ok(jsFiles.length > 0, 'At least one .js hook file must be installed'); + + const offenders = []; + for (const { rel, full } of jsFiles) { + const content = fs.readFileSync(full, 'utf-8'); + const badLines = colonRefs(content); + if (badLines.length > 0) { + offenders.push({ rel, lines: badLines }); + } + } + + assert.deepEqual( + offenders, + [], + 'Installed Claude hook .js files must not contain /gsd: colon refs ' + + '(non-comment occurrences). The install-time rewriter must replace these with /gsd-. ' + + 'Offenders: ' + JSON.stringify(offenders, null, 2), + ); + }); + + test('1c: installed .js hook files DO contain the hyphen form /gsd- (rewrite happened)', () => { + const hooksDir = path.join(tmpDir, '.claude', 'hooks'); + if (!fs.existsSync(hooksDir)) return; + + const jsFiles = findJsFiles(hooksDir); + const withHyphen = jsFiles.filter(({ full }) => { + const content = fs.readFileSync(full, 'utf-8'); + return /\/gsd-/.test(content); + }); + + assert.ok( + withHyphen.length > 0, + 'At least one installed .js hook file must contain /gsd- (confirming rewrite ran). ' + + `Files checked: ${jsFiles.map(f => f.rel).join(', ')}`, + ); + }); +}); + +// --------------------------------------------------------------------------- +// Suite 2 — Cursor install regression: /gsd: → /gsd- still works (pre-existing) +// +// Note: Cursor does NOT install hooks/dist files (Cursor skips the hooks +// install step entirely — see install.js gate around line 8829). The Cursor +// /gsd: rewrite applies in `copyWithPathReplacement` to JS files under the +// agent/skill tree (.cursor/get-shit-done/*.js etc). We verify that Cursor's +// installed .js files under .cursor/ have no /gsd: colon refs. +// --------------------------------------------------------------------------- +describe('bug #376 — Suite 2: Cursor install still rewrites /gsd: → /gsd- (regression)', () => { + let tmpDir; + + before(() => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-376-cursor-')); + runInstall(tmpDir, ['--cursor', '--local', '--no-sdk']); + }); + + after(() => { + if (tmpDir) { + fs.rmSync(tmpDir, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); + } + }); + + test('2a: .cursor/ directory is created by the Cursor local install', () => { + const cursorDir = path.join(tmpDir, '.cursor'); + assert.ok( + fs.existsSync(cursorDir), + `Cursor install must create .cursor/ directory at ${cursorDir}`, + ); + }); + + test('2b: Cursor-installed .js files contain no user-facing /gsd: colon refs', () => { + const cursorDir = path.join(tmpDir, '.cursor'); + if (!fs.existsSync(cursorDir)) return; + + const jsFiles = findJsFiles(cursorDir); + // Cursor may not install any .js files depending on what agent/skill content exists; + // if none, skip gracefully. + if (jsFiles.length === 0) return; + + const offenders = []; + for (const { rel, full } of jsFiles) { + const content = fs.readFileSync(full, 'utf-8'); + const badLines = colonRefs(content); + if (badLines.length > 0) { + offenders.push({ rel, lines: badLines }); + } + } + + assert.deepEqual( + offenders, + [], + 'Cursor-installed .js files must not contain /gsd: colon refs. ' + + 'The existing Cursor branch in copyWithPathReplacement must still apply /gsd:/gi → gsd- rewrite. ' + + 'Offenders: ' + JSON.stringify(offenders, null, 2), + ); + }); + + test('2c: Cursor install does not install a hooks/ directory (hooks are cursor-skipped)', () => { + // Cursor intentionally skips the hooks/dist copy step — verify this contract + // is still honored so we know the regression only concerns hook .js files + // for runtimes that DO install hooks (claude, qwen, hermes etc.). + const hooksDir = path.join(tmpDir, '.cursor', 'hooks'); + assert.strictEqual( + fs.existsSync(hooksDir), + false, + 'Cursor install must NOT create a hooks/ directory — Cursor skips the hooks install step', + ); + }); +}); + +// --------------------------------------------------------------------------- +// Suite 3 — Source files in hooks/ are untouched +// --------------------------------------------------------------------------- +describe('bug #376 — Suite 3: hooks/ source files are unchanged by install', () => { + let snapshotBefore; + + before(() => { + // Snapshot hooks/dist JS files before any install in this suite + snapshotBefore = {}; + if (fs.existsSync(HOOKS_DIST_DIR)) { + for (const { rel, full } of findJsFiles(HOOKS_DIST_DIR)) { + snapshotBefore[rel] = fs.readFileSync(full, 'utf-8'); + } + } + }); + + test('3a: hooks/dist .js source files still contain /gsd: literals (not mutated)', () => { + // The source must remain in colon form — the rewrite is install-time only + const jsFiles = findJsFiles(HOOKS_DIST_DIR); + const withColonRef = jsFiles.filter(({ full }) => { + const content = fs.readFileSync(full, 'utf-8'); + return colonRefs(content).length > 0; + }); + + // We know from the prerequisite suite that at least one file had a colon ref; + // if the source was mutated by install, this would now be zero. + assert.ok( + withColonRef.length > 0, + 'hooks/dist .js files must still contain /gsd: literals after install — ' + + 'the install-time rewrite must NOT modify the source tree. ' + + `Files that still have colon refs: ${withColonRef.map(f => f.rel).join(', ')}`, + ); + }); + + test('3b: hooks/dist .js source file contents match pre-test snapshot (byte-identical)', () => { + if (Object.keys(snapshotBefore).length === 0) { + // hooks/dist was absent before; skip + return; + } + + for (const [rel, before] of Object.entries(snapshotBefore)) { + const full = path.join(HOOKS_DIST_DIR, rel); + const after = fs.readFileSync(full, 'utf-8'); + assert.strictEqual( + after, + before, + `hooks/dist/${rel} was mutated by install — install must only rewrite the installed copy, not the source`, + ); + } + }); +}); + +// --------------------------------------------------------------------------- +// Suite 4 — Pure-function: shouldNormalizeHyphenNamespaceInAgentBody covers claude +// --------------------------------------------------------------------------- +describe('bug #376 — Suite 4: shouldNormalizeHyphenNamespaceInAgentBody covers claude', () => { + const install = require(INSTALL_PATH); + + test('4a: shouldNormalizeHyphenNamespaceInAgentBody is exported', () => { + assert.strictEqual( + typeof install.shouldNormalizeHyphenNamespaceInAgentBody, + 'function', + 'install.js must export shouldNormalizeHyphenNamespaceInAgentBody', + ); + }); + + test('4b: claude is in the hyphen-namespace set', () => { + assert.strictEqual( + install.shouldNormalizeHyphenNamespaceInAgentBody('claude'), + true, + 'claude must be a hyphen-namespace runtime', + ); + }); + + test('4c: qwen is in the hyphen-namespace set', () => { + assert.strictEqual( + install.shouldNormalizeHyphenNamespaceInAgentBody('qwen'), + true, + ); + }); + + test('4d: hermes is in the hyphen-namespace set', () => { + assert.strictEqual( + install.shouldNormalizeHyphenNamespaceInAgentBody('hermes'), + true, + ); + }); + + test('4e: gemini is NOT in the hyphen-namespace set', () => { + assert.strictEqual( + install.shouldNormalizeHyphenNamespaceInAgentBody('gemini'), + false, + 'gemini intentionally keeps colon namespace and must not be in the hyphen set', + ); + }); +});