From e04e757672c56545ca458d6e10ee2b6333e5f34a Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Sun, 7 Jun 2026 19:16:32 -0400 Subject: [PATCH 1/8] feat(#776): Gemini hook events (BeforeAgent/AfterAgent/BeforeModel) + hooksConfig.enabled check (#829) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(#776): Gemini hook events (BeforeAgent/AfterAgent/BeforeModel) + hooksConfig.enabled check Register three new Gemini-CLI hook events on install: - BeforeAgent: fires before agent planning; wired to gsd-context-monitor - AfterAgent: fires after final response generation; wired to gsd-context-monitor - BeforeModel: fires before each LLM call (per-turn); wired to gsd-context-monitor All three reuse gsd-context-monitor.js (no new hook files). Uninstall cleanup loop extended to remove the new events. Non-array guard added for robustness against malformed settings. Also detect hooksConfig.enabled:false in Gemini settings and emit a clear warning — without this check, all registered hooks silently do nothing. Co-Authored-By: Claude Opus 4.8 * chore: update changeset pr: 829 Co-Authored-By: Claude Opus 4.8 * docs(#776): document Gemini hook events Add hook coverage table to the Gemini CLI section of install-on-your-runtime.md, covering the three new events (BeforeAgent/AfterAgent/BeforeModel wired to gsd-context-monitor) plus a callout for the hooksConfig.enabled:false silent failure mode detected by the installer. Co-Authored-By: Claude Opus 4.8 --------- Co-authored-by: Claude Opus 4.8 --- .changeset/776-gemini-hook-events.md | 5 + bin/install.js | 73 +++- docs/how-to/install-on-your-runtime.md | 15 + scripts/lint-test-file-count.allowlist.json | 1 + ...nh-776-install-gemini-hook-events.test.cjs | 406 ++++++++++++++++++ 5 files changed, 497 insertions(+), 3 deletions(-) create mode 100644 .changeset/776-gemini-hook-events.md create mode 100644 tests/enh-776-install-gemini-hook-events.test.cjs diff --git a/.changeset/776-gemini-hook-events.md b/.changeset/776-gemini-hook-events.md new file mode 100644 index 000000000..7f03043df --- /dev/null +++ b/.changeset/776-gemini-hook-events.md @@ -0,0 +1,5 @@ +--- +type: Added +pr: 829 +--- +Gemini installs now register three additional hook events — `BeforeAgent`, `AfterAgent`, and `BeforeModel` — wired to `gsd-context-monitor.js` for per-turn context headroom tracking. Previously only `SessionStart`, `BeforeTool`, and `AfterTool` were registered. The installer also detects `hooksConfig.enabled: false` in the user's Gemini `settings.json` and emits a clear warning, surfacing the silent failure mode where all hooks are registered but never execute. (#776) diff --git a/bin/install.js b/bin/install.js index 8a88ad2de..c706ff23c 100755 --- a/bin/install.js +++ b/bin/install.js @@ -8239,9 +8239,10 @@ function uninstall(isGlobal, runtime = 'claude') { // Remove GSD hooks from settings — per-hook granularity to preserve // user hooks that share an entry with a GSD hook (#1755 followup). // Includes the 3 Qwen-only events added in #788 (SubagentStop, Stop, - // PreCompact) — safe to iterate for all runtimes; non-Qwen installs - // simply find no entries and skip. - for (const eventName of ['SessionStart', 'PostToolUse', 'AfterTool', 'PreToolUse', 'BeforeTool', 'SubagentStop', 'Stop', 'PreCompact']) { + // PreCompact) and the 3 Gemini-only events added in #776 (BeforeAgent, + // AfterAgent, BeforeModel) — safe to iterate for all runtimes; non-Qwen + // and non-Gemini installs simply find no entries and skip. + for (const eventName of ['SessionStart', 'PostToolUse', 'AfterTool', 'PreToolUse', 'BeforeTool', 'SubagentStop', 'Stop', 'PreCompact', 'BeforeAgent', 'AfterAgent', 'BeforeModel']) { if (settings.hooks && settings.hooks[eventName]) { const before = JSON.stringify(settings.hooks[eventName]); settings.hooks[eventName] = settings.hooks[eventName] @@ -11215,8 +11216,74 @@ function install(isGlobal, runtime = 'claude', options = {}) { } } // ── end Qwen-only extended hook events ──────────────────────────────────── + + // ── Gemini-only extended hook events (#776) ─────────────────────────────── + // Gemini CLI exposes several hook events beyond BeforeTool/AfterTool that + // gsd previously did not register. Three high-value events are added here: + // + // BeforeAgent — fires after user submits a prompt, before the agent + // plans. Wire gsd-context-monitor for context headroom + // awareness at prompt time. + // AfterAgent — fires once per turn after the model generates its final + // response. Wire gsd-context-monitor to track headroom + // after each agent turn completes. + // BeforeModel — fires before each LLM call (per-turn, not per-session). + // Wire gsd-context-monitor for per-turn context injection + // — more precise than session-start-only injection. + // + // All three reuse gsd-context-monitor.js — no new hook files needed. + // The `decision:"deny"` retry capability of AfterAgent is intentionally + // left to the hook script to implement when triggered (gsd-context-monitor + // exits 0 / advisory-only today; an active quality gate is a follow-on). + // + // Note: BeforeToolSelection is NOT wired. That event does not map to a + // gsd hook use case at this time; deferred to a follow-on issue. + // + // Guard: isGemini is defined at the top of install() (line ~8696). + if (isGemini) { + for (const geminiEvent of ['BeforeAgent', 'AfterAgent', 'BeforeModel']) { + if (!Array.isArray(settings.hooks[geminiEvent])) { + settings.hooks[geminiEvent] = []; + } + const alreadyHasContextMonitor = settings.hooks[geminiEvent].some(entry => + entry.hooks && entry.hooks.some(h => h.command && h.command.includes('gsd-context-monitor')) + ); + if (!alreadyHasContextMonitor && fs.existsSync(contextMonitorFile) && contextMonitorCommand) { + settings.hooks[geminiEvent].push({ + hooks: [ + { + type: 'command', + command: contextMonitorCommand, + timeout: 10 + } + ] + }); + console.log(` ${green}✓${reset} Configured ${geminiEvent} context monitor hook (Gemini)`); + } else if (!alreadyHasContextMonitor && !fs.existsSync(contextMonitorFile)) { + console.warn(` ${yellow}⚠${reset} Skipped ${geminiEvent} hook — gsd-context-monitor.js not found at target`); + } + } + } + // ── end Gemini-only extended hook events ────────────────────────────────── } + // ── Gemini hooksConfig.enabled check (#776) ─────────────────────────────── + // Detect `hooksConfig.enabled: false` in the already-loaded settings object + // and emit a clear warning. When this field is false the Gemini CLI silently + // disables ALL hook execution — gsd hooks are registered but will never run. + // The check is read-only (warning only; we do not mutate hooksConfig). + // Note: we use the in-memory `settings` object (already read from disk and + // cleaned up by validateHookFields/cleanupOrphanedHooks above) rather than + // re-reading settings.json, avoiding a TOCTOU window between the two reads. + if (isGemini && settings && settings.hooksConfig && settings.hooksConfig.enabled === false) { + console.warn( + ` ${yellow}⚠${reset} Warning: hooksConfig.enabled is false in your Gemini settings.json.\n` + + ` gsd-core hooks are registered but will NOT run until you set\n` + + ` hooksConfig.enabled: true in ${path.join(targetDir, 'settings.json')}.` + ); + } + // ── end hooksConfig.enabled check ──────────────────────────────────────── + // Compute the update-banner hook command alongside the others so // installAllRuntimes can register it at finalize time when the user opts // in (#2795). Computed here (not in finishInstall) so the same buildHookCommand diff --git a/docs/how-to/install-on-your-runtime.md b/docs/how-to/install-on-your-runtime.md index 6d10fbbf5..595d45f0b 100644 --- a/docs/how-to/install-on-your-runtime.md +++ b/docs/how-to/install-on-your-runtime.md @@ -107,6 +107,21 @@ The installer also enriches the generated TOML commands with two native Gemini c GEMINI_CONFIG_DIR=~/.gemini-alt npx @opengsd/gsd-core@latest --gemini --global ``` +**Hook coverage** + +GSD registers the following hook events automatically on install: + +| Event | Hook | Purpose | +|---|---|---| +| `SessionStart` | `gsd-check-update.js`, `gsd-session-state.sh` | Update check, session orientation | +| `BeforeTool` | `gsd-prompt-guard.js`, `gsd-read-guard.js`, `gsd-workflow-guard.js`, `gsd-worktree-path-guard.js`, `gsd-validate-commit.sh` | Prompt guard, read-before-edit, workflow + worktree safety, commit validation | +| `AfterTool` | `gsd-context-monitor.js`, `gsd-read-injection-scanner.js`, `gsd-phase-boundary.sh`, `gsd-graphify-update.sh` | Context monitoring, read-time scan, phase boundary detection | +| `BeforeAgent` | `gsd-context-monitor.js` | Context headroom awareness before the agent begins planning each prompt | +| `AfterAgent` | `gsd-context-monitor.js` | Context headroom tracking after each agent turn's final response | +| `BeforeModel` | `gsd-context-monitor.js` | Per-turn context injection before each LLM call | + +> **`hooksConfig.enabled: false` warning.** If your Gemini `settings.json` contains `hooksConfig.enabled: false`, the Gemini CLI silently disables all hook execution — GSD hooks are registered but will never run. The installer detects this and emits a warning. To enable hooks, set `hooksConfig.enabled: true` in `~/.gemini/settings.json` (or the directory matching your `GEMINI_CONFIG_DIR`). + --- ### Gemini CLI — native extension install (#775) diff --git a/scripts/lint-test-file-count.allowlist.json b/scripts/lint-test-file-count.allowlist.json index abec29430..d5859805f 100644 --- a/scripts/lint-test-file-count.allowlist.json +++ b/scripts/lint-test-file-count.allowlist.json @@ -121,6 +121,7 @@ "install": { "files": [ "bug-410-install-defaults-test-mode-guard.test.cjs", + "enh-776-install-gemini-hook-events.test.cjs", "install-minimal-hooks.test.cjs", "install-path-detection.test.cjs", "install-regressions.test.cjs", diff --git a/tests/enh-776-install-gemini-hook-events.test.cjs b/tests/enh-776-install-gemini-hook-events.test.cjs new file mode 100644 index 000000000..23a259cf4 --- /dev/null +++ b/tests/enh-776-install-gemini-hook-events.test.cjs @@ -0,0 +1,406 @@ +'use strict'; + +process.env.GSD_TEST_MODE = '1'; + +/** + * Enhancement #776: Adopt new Gemini hook events + detect hooksConfig.enabled:false. + * + * Gemini CLI exposes several hook events beyond BeforeTool/AfterTool that gsd + * previously did not register. This suite asserts that a Gemini install + * registers the 3 new high-value events: + * - BeforeAgent — fires before the agent plans (context headroom tracking) + * - AfterAgent — fires after final response generation (context tracking) + * - BeforeModel — fires before each LLM call (per-turn context awareness) + * + * All three are wired to gsd-context-monitor.js — the same hook used for + * AfterTool — so context headroom warnings surface at these lifecycle moments. + * + * Also asserts: + * - Claude Code installs do NOT gain these Gemini-only events (strict scope guard). + * - Reinstalls are idempotent (no hook duplication). + * - Uninstall removes the new event registrations. + * - hooksConfig.enabled:false warning is emitted during a Gemini install. + * + * Source: https://github.com/google-gemini/gemini-cli/blob/main/docs/hooks/reference.md + */ + +const { test, describe, beforeEach, afterEach } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); + +const { install, uninstall, validateHookFields } = require('../bin/install.js'); +const { createTempDir, cleanup } = require('./helpers.cjs'); + +// ─── Helpers ───────────────────────────────────────────────────────────────── + +/** Extract all hook commands registered under `eventName` from settings. */ +function hooksForEvent(settings, eventName) { + if (!settings || !settings.hooks || !Array.isArray(settings.hooks[eventName])) return []; + return settings.hooks[eventName].flatMap(entry => + (entry && Array.isArray(entry.hooks) ? entry.hooks : []) + .map(h => h && h.command) + .filter(Boolean) + ); +} + +// Stub JS hook files that the installer checks with fs.existsSync() so hook +// registration guards pass even when hooks/dist/ isn't built. +// +// For Gemini, the installer migration baseline includes 'hooks/' in its surface +// list (unlike Qwen which excludes it). Pre-install stubs placed in .gemini/hooks/ +// are classified as 'bundled-gsd-hook' and auto-removed by the migration before +// registration can succeed. The workaround: run a first install (which writes the +// gsd-file-manifest.json), then add the stubs, then run install again. On the +// second install the manifest marks the hook files as managed, so migration keeps +// them and registration guards (fs.existsSync) pass. +const HOOKS_SRC = path.join(__dirname, '..', 'hooks'); +const STUB_HOOKS = [ + 'gsd-context-monitor.js', + 'gsd-prompt-guard.js', + 'gsd-check-update.js', +]; + +function stubHooksIntoTarget(targetDir) { + const hooksDest = path.join(targetDir, 'hooks'); + fs.mkdirSync(hooksDest, { recursive: true }); + for (const hookFile of STUB_HOOKS) { + const src = path.join(HOOKS_SRC, hookFile); + const dest = path.join(hooksDest, hookFile); + if (fs.existsSync(src)) { + fs.copyFileSync(src, dest); + } else { + // Minimal stub so existsSync passes + fs.writeFileSync(dest, '#!/usr/bin/env node\n// stub\n'); + } + try { fs.chmodSync(dest, 0o755); } catch { /* Windows */ } + } +} + +/** + * Two-pass Gemini install. + * + * Pass 1: install() with no hook stubs — writes gsd-file-manifest.json. + * Migration runs on an empty hooks/ so nothing gets auto-removed. + * Pass 2: stub hooks into the target dir (now manifest-tracked on next scan), + * then run install() again. Migration now classifies the hooks as + * managed-unchanged and preserves them; registration guards pass. + * + * Returns the settings from pass 2. + */ +function twoPassGeminiInstall(tmpDir) { + const targetDir = path.join(tmpDir, '.gemini'); + fs.mkdirSync(targetDir, { recursive: true }); + + // Pass 1 — no hook stubs yet; writes the manifest + const result1 = install(false, 'gemini'); + persistSettings(result1.settingsPath, result1.settings); + + // Inject stubs so the registration guards (fs.existsSync) pass on pass 2 + stubHooksIntoTarget(targetDir); + + // Pass 2 — manifest exists, migration keeps stubs, registration succeeds + process.chdir(tmpDir); + const result2 = install(false, 'gemini'); + return result2; +} + +/** + * Persist in-memory settings to disk, simulating what finishInstall() does + * (finishInstall is not exported). Required for tests that call install() + * twice and need the second call to read the first call's hook registrations. + */ +function persistSettings(settingsPath, settings) { + fs.mkdirSync(path.dirname(settingsPath), { recursive: true }); + fs.writeFileSync(settingsPath, JSON.stringify(validateHookFields(settings), null, 2) + '\n', 'utf8'); +} + +// ─── Suite 1: Gemini — new events are registered ───────────────────────────── + +describe('enh-776: Gemini install registers 3 new hook events', () => { + let tmpDir; + let previousCwd; + let settings; + + beforeEach(() => { + tmpDir = createTempDir('gsd-776-gemini-'); + previousCwd = process.cwd(); + process.chdir(tmpDir); + + // Two-pass install: first pass writes manifest, second pass with stubs + // so registration guards (fs.existsSync) pass. + const result = twoPassGeminiInstall(tmpDir); + settings = result.settings; + }); + + afterEach(() => { + process.chdir(previousCwd); + cleanup(tmpDir); + }); + + test('install returns a settings object (not null)', () => { + assert.ok(settings !== null && typeof settings === 'object', + 'Gemini install must return a non-null settings object'); + }); + + test('BeforeAgent event is registered with at least one hook', () => { + const cmds = hooksForEvent(settings, 'BeforeAgent'); + assert.ok(cmds.length > 0, + `Expected BeforeAgent hooks; got hooks: ${JSON.stringify(settings && settings.hooks)}`); + }); + + test('AfterAgent event is registered with at least one hook', () => { + const cmds = hooksForEvent(settings, 'AfterAgent'); + assert.ok(cmds.length > 0, + `Expected AfterAgent hooks; got hooks: ${JSON.stringify(settings && settings.hooks)}`); + }); + + test('BeforeModel event is registered with at least one hook', () => { + const cmds = hooksForEvent(settings, 'BeforeModel'); + assert.ok(cmds.length > 0, + `Expected BeforeModel hooks; got hooks: ${JSON.stringify(settings && settings.hooks)}`); + }); + + test('BeforeAgent / AfterAgent / BeforeModel all use gsd-context-monitor', () => { + for (const event of ['BeforeAgent', 'AfterAgent', 'BeforeModel']) { + const cmds = hooksForEvent(settings, event); + assert.ok( + cmds.some(c => c.includes('gsd-context-monitor')), + `Event ${event} should use gsd-context-monitor; got commands: ${JSON.stringify(cmds)}` + ); + } + }); +}); + +// ─── Suite 2: Non-Gemini installs do NOT get the new events ────────────────── +// +// Two runtimes are particularly important to guard: +// Claude — the canonical non-Gemini runtime +// Antigravity — shares Gemini-style BeforeTool/AfterTool naming and uses +// isGemini-adjacent logic; a future accidental +// `isGemini || isAntigravity` change must be caught here. + +describe('enh-776: Claude install does NOT register Gemini-only hook events', () => { + let tmpDir; + let previousCwd; + let settings; + + beforeEach(() => { + tmpDir = createTempDir('gsd-776-claude-'); + previousCwd = process.cwd(); + process.chdir(tmpDir); + + const result = install(false, 'claude'); + settings = result && result.settings; + }); + + afterEach(() => { + process.chdir(previousCwd); + cleanup(tmpDir); + }); + + test('Claude install does not register BeforeAgent', () => { + const cmds = hooksForEvent(settings, 'BeforeAgent'); + assert.strictEqual(cmds.length, 0, + `Claude should NOT have BeforeAgent; got: ${JSON.stringify(cmds)}`); + }); + + test('Claude install does not register AfterAgent', () => { + const cmds = hooksForEvent(settings, 'AfterAgent'); + assert.strictEqual(cmds.length, 0, + `Claude should NOT have AfterAgent; got: ${JSON.stringify(cmds)}`); + }); + + test('Claude install does not register BeforeModel', () => { + const cmds = hooksForEvent(settings, 'BeforeModel'); + assert.strictEqual(cmds.length, 0, + `Claude should NOT have BeforeModel; got: ${JSON.stringify(cmds)}`); + }); +}); + +describe('enh-776: Antigravity install does NOT register Gemini-only hook events', () => { + let tmpDir; + let previousCwd; + let settings; + + beforeEach(() => { + tmpDir = createTempDir('gsd-776-antigravity-'); + previousCwd = process.cwd(); + process.chdir(tmpDir); + + const result = install(false, 'antigravity'); + settings = result && result.settings; + }); + + afterEach(() => { + process.chdir(previousCwd); + cleanup(tmpDir); + }); + + test('Antigravity install does not register BeforeAgent', () => { + const cmds = hooksForEvent(settings, 'BeforeAgent'); + assert.strictEqual(cmds.length, 0, + `Antigravity should NOT have BeforeAgent; got: ${JSON.stringify(cmds)}`); + }); + + test('Antigravity install does not register AfterAgent', () => { + const cmds = hooksForEvent(settings, 'AfterAgent'); + assert.strictEqual(cmds.length, 0, + `Antigravity should NOT have AfterAgent; got: ${JSON.stringify(cmds)}`); + }); + + test('Antigravity install does not register BeforeModel', () => { + const cmds = hooksForEvent(settings, 'BeforeModel'); + assert.strictEqual(cmds.length, 0, + `Antigravity should NOT have BeforeModel; got: ${JSON.stringify(cmds)}`); + }); +}); + +// ─── Suite 3: Idempotency — persisted reinstall does not duplicate hooks ────── + +describe('enh-776: Gemini install is idempotent across persisted reinstalls', () => { + let tmpDir; + let previousCwd; + + beforeEach(() => { + tmpDir = createTempDir('gsd-776-idem-'); + previousCwd = process.cwd(); + process.chdir(tmpDir); + }); + + afterEach(() => { + process.chdir(previousCwd); + cleanup(tmpDir); + }); + + test('re-running after persisted first install does not duplicate hook entries', () => { + // Two-pass to get hooks installed. + const result2 = twoPassGeminiInstall(tmpDir); + const s2 = result2.settings; + + // Assert hooks ARE registered after pass 2 (guards against false-pass where + // hooks never registered and idempotency passes trivially at count=0). + for (const event of ['BeforeAgent', 'AfterAgent', 'BeforeModel']) { + const cmds = hooksForEvent(s2, event); + assert.strictEqual(cmds.length, 1, + `Event ${event} should have exactly 1 hook after two-pass install; got ${cmds.length}: ${JSON.stringify(cmds)}`); + } + + persistSettings(result2.settingsPath, s2); + + // Third install: reads the persisted settings.json — dedup guards apply + process.chdir(tmpDir); + const result3 = install(false, 'gemini'); + const s3 = result3.settings; + + for (const event of ['BeforeAgent', 'AfterAgent', 'BeforeModel']) { + const cmds = hooksForEvent(s3, event); + assert.strictEqual(cmds.length, 1, + `Event ${event} should have exactly 1 hook command after idempotent reinstall; got ${cmds.length}: ${JSON.stringify(cmds)}`); + } + }); +}); + +// ─── Suite 4: Uninstall removes the new event registrations ────────────────── + +describe('enh-776: Gemini uninstall removes new hook event entries', () => { + let tmpDir; + let previousCwd; + + beforeEach(() => { + tmpDir = createTempDir('gsd-776-uninstall-'); + previousCwd = process.cwd(); + process.chdir(tmpDir); + + // Two-pass install and persist so uninstall has a settings.json to clean + const result = twoPassGeminiInstall(tmpDir); + persistSettings(result.settingsPath, result.settings); + }); + + afterEach(() => { + process.chdir(previousCwd); + cleanup(tmpDir); + }); + + test('settings.json hook entries are removed on uninstall', () => { + uninstall(false, 'gemini'); + const settingsPath = path.join(tmpDir, '.gemini', 'settings.json'); + if (!fs.existsSync(settingsPath)) return; // file removed entirely is fine + const settings = JSON.parse(fs.readFileSync(settingsPath, 'utf8')); + for (const event of ['BeforeAgent', 'AfterAgent', 'BeforeModel']) { + const cmds = hooksForEvent(settings, event); + assert.strictEqual(cmds.length, 0, + `After uninstall, ${event} should have 0 hooks; got: ${JSON.stringify(cmds)}`); + } + }); +}); + +// ─── Suite 5: hooksConfig.enabled:false warning ─────────────────────────────── + +describe('enh-776: hooksConfig.enabled:false warning during Gemini install', () => { + let tmpDir; + let previousCwd; + let stderrLines; + let originalWarn; + + beforeEach(() => { + tmpDir = createTempDir('gsd-776-hookscfg-'); + previousCwd = process.cwd(); + process.chdir(tmpDir); + + const targetDir = path.join(tmpDir, '.gemini'); + fs.mkdirSync(targetDir, { recursive: true }); + + // Capture console.warn output + stderrLines = []; + originalWarn = console.warn; + console.warn = (...args) => { stderrLines.push(args.join(' ')); }; + }); + + afterEach(() => { + console.warn = originalWarn; + process.chdir(previousCwd); + cleanup(tmpDir); + }); + + test('emits a warning when hooksConfig.enabled is false', () => { + // Write a settings.json with hooksConfig.enabled: false BEFORE install + // (the check in install() reads the existing settings.json on disk). + const targetDir = path.join(tmpDir, '.gemini'); + const settingsPath = path.join(targetDir, 'settings.json'); + fs.writeFileSync(settingsPath, JSON.stringify({ hooksConfig: { enabled: false } }, null, 2) + '\n', 'utf8'); + + install(false, 'gemini'); + const warnText = stderrLines.join('\n'); + assert.ok( + warnText.includes('hooksConfig.enabled is false'), + `Expected hooksConfig.enabled warning; got console.warn output:\n${warnText}` + ); + }); + + test('does NOT emit the hooksConfig warning when hooksConfig.enabled is true', () => { + const targetDir = path.join(tmpDir, '.gemini'); + const settingsPath = path.join(targetDir, 'settings.json'); + fs.writeFileSync(settingsPath, JSON.stringify({ hooksConfig: { enabled: true } }, null, 2) + '\n', 'utf8'); + + install(false, 'gemini'); + const warnText = stderrLines.join('\n'); + assert.ok( + !warnText.includes('hooksConfig.enabled is false'), + `Should NOT warn when hooksConfig.enabled is true; got:\n${warnText}` + ); + }); + + test('does NOT emit the hooksConfig warning when hooksConfig is absent', () => { + const targetDir = path.join(tmpDir, '.gemini'); + const settingsPath = path.join(targetDir, 'settings.json'); + fs.writeFileSync(settingsPath, JSON.stringify({}, null, 2) + '\n', 'utf8'); + + install(false, 'gemini'); + const warnText = stderrLines.join('\n'); + assert.ok( + !warnText.includes('hooksConfig.enabled is false'), + `Should NOT warn when hooksConfig is absent; got:\n${warnText}` + ); + }); +}); From 1040fb792edc709d0ae92e6d43a87287803e3eb3 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Sun, 7 Jun 2026 19:22:48 -0400 Subject: [PATCH 2/8] feat(#777): register Cursor-native hooks (.cursor/hooks.json) for session-start/post-tool parity (#831) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(#777): register Cursor-native hooks (.cursor/hooks.json) for session-start/post-tool parity - Add gsd-cursor-session-start.js: injects STATE.md presence reminder (or new-project nudge) into Cursor sessions via the sessionStart hook event - Add gsd-cursor-post-tool.js: emits an additional_context nudge when write-class tool calls touch .planning/ files (postToolUse hook event) - Add 'cursor-hooks-json' installSurface to runtime-config-adapter-registry; writeCursorHooksJson/reconcileCursorHooksJson write the canonical { version: 1, hooks: { sessionStart, postToolUse } } JSON shape with idempotent reconciliation that preserves user-owned hook entries - Hook scripts are copied with /gsd:→gsd- rewrite so installed files contain no colon-form slash-command refs (bug-376 invariant) - 20 new tests in tests/cursor-hooks.test.cjs cover all reconciler paths, entry helpers, removal, runtime adapter surface, and hook script behavior - Update CONTEXT.md, ARCHITECTURE.md, installer-migrations.md, and 000-first-time-baseline.cts to include Cursor hooks.json surface Co-Authored-By: Claude Opus 4.8 * fix(#777): build hooks/dist on demand in bug-376 test for scoped/windows CI hooks/dist is gitignored and only produced by `npm run build:hooks`. The CI scoped (ubuntu-latest/node-22) and windows (windows-latest/node-24) test jobs do NOT run build:hooks before executing tests, so bug-376's prerequisite suite was failing with "hooks/dist not found" on both legs. Add ensureHooksDist() helper (mirrors bug-3357 pattern) that builds hooks/dist on demand in the before() hooks of prerequisite and Suite 3. Also add ensureHooksDist() call to Suite 3's before() so the snapshot step is also hermetic. Co-Authored-By: Claude Opus 4.8 --------- Co-authored-by: Claude Opus 4.8 --- .changeset/eager-herons-forage.md | 5 + CONTEXT.md | 2 +- bin/install.js | 315 +++++++++++- docs/ARCHITECTURE.md | 2 +- docs/INVENTORY-MANIFEST.json | 2 + docs/INVENTORY.md | 4 +- docs/installer-migrations.md | 2 +- hooks/gsd-cursor-post-tool.js | 75 +++ hooks/gsd-cursor-session-start.js | 52 ++ hooks/managed-hooks-registry.cjs | 2 + scripts/build-hooks.js | 3 + src/installer-migration-report.cts | 2 + .../000-first-time-baseline.cts | 2 +- src/runtime-config-adapter-registry.cts | 5 +- ...g-376-claude-js-hook-gsd-rewriter.test.cjs | 66 ++- tests/cursor-hooks.test.cjs | 462 ++++++++++++++++++ .../runtime-config-adapter-registry.test.cjs | 9 +- 17 files changed, 985 insertions(+), 25 deletions(-) create mode 100644 .changeset/eager-herons-forage.md create mode 100644 hooks/gsd-cursor-post-tool.js create mode 100644 hooks/gsd-cursor-session-start.js create mode 100644 tests/cursor-hooks.test.cjs diff --git a/.changeset/eager-herons-forage.md b/.changeset/eager-herons-forage.md new file mode 100644 index 000000000..f1adf4731 --- /dev/null +++ b/.changeset/eager-herons-forage.md @@ -0,0 +1,5 @@ +--- +type: Added +pr: 777 +--- +Cursor now receives GSD lifecycle hooks via `.cursor/hooks.json` — a sessionStart hook injects the current workflow state as context at session start, and a postToolUse hook nudges the agent to update `.planning/` after write-class operations, bringing Cursor to baseline hook parity with Gemini and Claude Code. diff --git a/CONTEXT.md b/CONTEXT.md index ae909288b..b6641f085 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -122,7 +122,7 @@ Module owning the per-runtime mapping from artifact kind to filesystem placement Projects a pure, typed install plan for a given runtime by composing artifact placements (Runtime Artifact Layout Module), command text (Shell Command Projection Module), and per-runtime config intentions — with no filesystem IO or format-specific serialization. Runtime-specific adapters consume the plan and execute concrete file mutations and config rendering. See ADR-58. ### Runtime Config Adapter Registry -Module owning the explicit per-runtime config-mutation dispatch table for the installer. `resolveRuntimeConfigIntent(runtime)` projects a typed config intent — `installSurface` (`settings-json` | `codex-toml` | `copilot-instructions` | `cline-rules` | `profile-marker-only`), `writesSharedSettings` (the `finishInstall` shared-settings write gate), and `finishPermissionWriter` (`opencode` | `kilo` | none) — that `bin/install.js` dispatches on instead of inline `runtime === '...'` branching. Owns adapter selection only: it performs no filesystem IO and does not execute config mutations (the install/finishInstall handlers and the per-runtime writers do that). Unknown runtimes fail loudly with a `TypeError`, guarded by an `Object.hasOwn` own-property check so prototype-chain keys (`__proto__`, `constructor`) also throw. Realizes the adapter-selection half of the Runtime Install Policy Module boundary. Source: `gsd-core/bin/lib/runtime-config-adapter-registry.cjs`. See ADR-58, #60. +Module owning the explicit per-runtime config-mutation dispatch table for the installer. `resolveRuntimeConfigIntent(runtime)` projects a typed config intent — `installSurface` (`settings-json` | `codex-toml` | `copilot-instructions` | `cline-rules` | `cursor-hooks-json` | `profile-marker-only`), `writesSharedSettings` (the `finishInstall` shared-settings write gate), and `finishPermissionWriter` (`opencode` | `kilo` | none) — that `bin/install.js` dispatches on instead of inline `runtime === '...'` branching. Owns adapter selection only: it performs no filesystem IO and does not execute config mutations (the install/finishInstall handlers and the per-runtime writers do that). Unknown runtimes fail loudly with a `TypeError`, guarded by an `Object.hasOwn` own-property check so prototype-chain keys (`__proto__`, `constructor`) also throw. Realizes the adapter-selection half of the Runtime Install Policy Module boundary. Source: `gsd-core/bin/lib/runtime-config-adapter-registry.cjs`. See ADR-58, #60. ### Claude Code Plugin Manifest Module Module owning the projection of gsd-core's artifact surfaces (`commands`, `agents`, hooks) onto the Claude Code plugin contract (`.claude-plugin/plugin.json` + `hooks/hooks.json`) — the plugin-contract sibling of the Runtime Artifact Layout Module (which projects the same surfaces onto filesystem placements). Defined mapping: `name`=`binName` (drives the `/gsd-core:` command namespace), `repository`/`homepage`=`repoUrl` (Package Identity Module), `version`/`description`/`license` from `package.json` (`version` is required for `claude plugin validate --strict`), `commands`=`./commands/gsd/`, agents via Claude Code's default `agents/` discovery (the explicit string form is schema-rejected), `hooks`=`./hooks/hooks.json`. The hook projection carries ONLY the always-on subset of the Installer Module's Claude `settings.json` wiring (check-update, context-monitor, prompt-guard, read-guard, worktree-path-guard, read-injection-scanner) via `${CLAUDE_PLUGIN_ROOT}`; config-gated opt-in hooks are excluded because a static manifest cannot honor per-project config gates, and plugin-shipped agents cannot carry hook frontmatter (so all plugin-path hook wiring lives in hooks.json). Additive — the file-copy path (Runtime Artifact Layout / Install Policy / Installer Modules) is unchanged. Conformance is validated by `claude plugin validate --strict` plus the in-repo drift-guard `tests/issue-766-plugin-manifest.test.cjs`. _Avoid_: "the plugin API", "the plugin file" (when you mean the seam). See ADR-766 and Runtime Artifact Layout Module. diff --git a/bin/install.js b/bin/install.js index c706ff23c..a3c0408eb 100755 --- a/bin/install.js +++ b/bin/install.js @@ -190,6 +190,19 @@ const GSD_COPILOT_SESSION_HOOK_PWSH = `{ '{"additionalContext":"${GSD_COPILOT_SESSION_MSG_PRESENT}"}' } ` + `else { '{"additionalContext":"${GSD_COPILOT_SESSION_MSG_ABSENT}"}' }`; +// #777 — Cursor CLI lifecycle hook constants. +// Cursor reads hook configs from /.cursor/hooks.json (local) or +// ~/.cursor/hooks.json (global) with the shape { version: 1, hooks: { : [...] } }. +// Events use camelCase: sessionStart, postToolUse, preToolUse, etc. +// A `command` hook entry runs an external script. GSD registers two managed hooks: +// sessionStart → gsd-cursor-session-start.js (context injection) +// postToolUse → gsd-cursor-post-tool.js (STATE.md update monitor) +// Cursor docs: https://cursor.com/docs/hooks +const GSD_CURSOR_SESSION_HOOK_SCRIPT = 'gsd-cursor-session-start.js'; +const GSD_CURSOR_POST_TOOL_HOOK_SCRIPT = 'gsd-cursor-post-tool.js'; +// Marker comment embedded in managed hook entries so GSD can find+remove them. +const GSD_CURSOR_HOOK_MARKER = 'gsd-managed'; + // GSD-managed files under hooks/lib/ (helpers required by gsd-*.sh hooks). // git-cmd.js does not start with "gsd-" (shared classifier for #3129), gsd-graphify-rebuild.sh does. const GSD_HOOK_LIB_FILES = ['git-cmd.js', 'gsd-graphify-rebuild.sh']; @@ -5738,6 +5751,248 @@ function writeClineArtifacts(targetDir, isGlobalInstall) { return written; } +// ── Cursor hooks.json reconciler (issue #777) ──────────────────────────────── +// +// Cursor v2.4+ supports a hooks.json lifecycle hook system. GSD registers two +// managed command hooks: +// sessionStart → gsd-cursor-session-start.js (context injection) +// postToolUse → gsd-cursor-post-tool.js (STATE.md update monitor) +// +// hooks.json schema: +// { "version": 1, "hooks": { "": [ { "type": "command", "command": "" } ] } } +// +// Location: +// Global: ~/.cursor/hooks.json +// Local: /.cursor/hooks.json +// +// GSD entries are identified by a top-level `"gsd-managed": true` field on +// each hook entry. Non-GSD entries are preserved. The reconciler is idempotent +// (safe to re-run) and preserves user-owned entries in the file. +// +// References: https://cursor.com/docs/hooks + +/** + * Build a managed Cursor hook entry for a given hook script path. + * + * @param {string} scriptPath - Absolute path to the hook script + * @returns {object} Cursor hook entry object + */ +function buildCursorHookEntry(scriptPath) { + return { + type: 'command', + command: scriptPath.replace(/\\/g, '/'), + [GSD_CURSOR_HOOK_MARKER]: true, + }; +} + +/** + * Return true if a Cursor hook entry is GSD-managed. + * Detection: presence of the GSD_CURSOR_HOOK_MARKER sentinel field. + * + * @param {object} entry - A hooks array element from hooks.json + * @returns {boolean} + */ +function isManagedCursorHookEntry(entry) { + return Boolean(entry && typeof entry === 'object' && entry[GSD_CURSOR_HOOK_MARKER]); +} + +/** + * Reconcile the GSD-managed entries in a Cursor hooks.json file. + * + * Supports both known hooks.json shapes: + * 1) { "version": 1, "hooks": { "sessionStart": [...], "postToolUse": [...] } } + * 2) { "sessionStart": [...], "postToolUse": [...] } (no wrapper object) + * + * Managed entries (those with GSD_CURSOR_HOOK_MARKER) are removed then + * re-added if managedEntries is non-null/non-empty. User-owned entries are + * preserved. File is written atomically only when content changes. + * + * @param {string} hooksJsonPath - Absolute path to the hooks.json file + * @param {{ sessionStart?: object|null, postToolUse?: object|null }|null} managedEntries + * Map from event name to the new hook entry to register (or null to remove). + * Pass null for the whole param to remove all managed entries. + * @returns {{ changed: boolean, wrote: boolean, path: string }} + */ +function reconcileCursorHooksJson(hooksJsonPath, managedEntries) { + let parsed = {}; + let currentContent = null; + + if (fs.existsSync(hooksJsonPath)) { + const raw = fs.readFileSync(hooksJsonPath, 'utf8'); + currentContent = raw; + if (raw.trim()) { + try { + parsed = JSON.parse(raw); + } catch (err) { + throw new Error(`Cursor hooks.json parse failed: ${err && err.message ? err.message : String(err)}`); + } + } + } + if (!parsed || typeof parsed !== 'object' || Array.isArray(parsed)) parsed = {}; + + // Cursor's canonical hooks.json schema is { "version": 1, "hooks": { ... } }. + // GSD always writes (and migrates to) the nested shape so Cursor reads it correctly. + // The flat shape { "sessionStart": [...] } is accepted on read for backwards compat + // with manually-written files, but the output always uses the nested form. + const hasNestedHooksObject = + parsed.hooks && typeof parsed.hooks === 'object' && !Array.isArray(parsed.hooks); + if (!hasNestedHooksObject) { + // Migrate flat shape (or empty {}) to nested: lift event keys into hooks:{}. + const eventKeys = ['sessionStart', 'postToolUse']; + const lifted = {}; + for (const k of eventKeys) { + if (Array.isArray(parsed[k])) { + lifted[k] = parsed[k]; + delete parsed[k]; + } + } + parsed.hooks = lifted; + } + if (!parsed.version) parsed.version = 1; + const hookTable = parsed.hooks; + + // Events GSD manages. + const MANAGED_EVENTS = ['sessionStart', 'postToolUse']; + const entries = managedEntries || {}; + + for (const event of MANAGED_EVENTS) { + const existing = Array.isArray(hookTable[event]) ? hookTable[event] : []; + // Strip all prior GSD-managed entries for this event. + const userOwned = existing.filter((e) => !isManagedCursorHookEntry(e)); + const newEntry = entries[event] || null; + if (newEntry) { + hookTable[event] = [...userOwned, newEntry]; + } else { + // Remove-only: keep user entries, or delete the key if it would be empty. + if (userOwned.length > 0) { + hookTable[event] = userOwned; + } else { + delete hookTable[event]; + } + } + } + + // hookTable is parsed.hooks (always nested now); no reassignment needed. + // Write only if content changed or if we're creating the file for the first time. + const nextContent = `${JSON.stringify(parsed, null, 2)}\n`; + const changed = currentContent !== nextContent; + const shouldWrite = changed && (currentContent !== null || Object.keys(parsed).length > 0); + if (shouldWrite) { + atomicWriteFileSync(hooksJsonPath, nextContent, 'utf8'); + } + + return { changed: changed, wrote: shouldWrite, path: hooksJsonPath }; +} + +/** + * #777 — Write GSD-managed Cursor lifecycle hooks into /hooks.json. + * + * Both managed hook scripts (gsd-cursor-session-start.js, gsd-cursor-post-tool.js) + * are copied from the GSD hooks/ source to /hooks/ first, so the + * hooks.json entries never reference a script that wasn't installed. + * + * @param {string} targetDir - The Cursor config dir (global: ~/.cursor; local: .cursor) + * @param {string} src - The GSD install source root (for copying hook scripts) + * @param {{ absoluteRunner?: string|null }} opts + * @returns {{ hooksJsonPath: string, changed: boolean }} + */ +function writeCursorHooksJson(targetDir, src, opts) { + opts = opts || {}; + const hooksDir = path.join(targetDir, 'hooks'); + fs.mkdirSync(hooksDir, { recursive: true }); + + // Copy the two GSD-managed hook scripts from the GSD source hooks/ directory. + // Apply the same /gsd:/gi → gsd- rewrite used by copyWithPathReplacement for Cursor + // JS files, so the installed hook scripts contain no /gsd: colon refs (bug-376 2b). + // Track which scripts were successfully installed so we never register a hook entry + // that references a script that wasn't copied (dangling command guard). + const hookScripts = [GSD_CURSOR_SESSION_HOOK_SCRIPT, GSD_CURSOR_POST_TOOL_HOOK_SCRIPT]; + const srcHooksDir = path.join(src, 'hooks'); + const installedScripts = new Set(); + for (const script of hookScripts) { + const srcPath = path.join(srcHooksDir, script); + const destPath = path.join(hooksDir, script); + if (fs.existsSync(srcPath)) { + let content = fs.readFileSync(srcPath, 'utf8'); + // Rewrite /gsd: → gsd- so installed hook scripts are consistent + // with the Cursor convention (no colon-form slash commands in agent context). + content = content.replace(/gsd:/gi, 'gsd-'); + fs.writeFileSync(destPath, content); + try { fs.chmodSync(destPath, 0o755); } catch { /* Windows: ignore chmod */ } + installedScripts.add(script); + } + } + + // Build command strings using the same buildHookCommand helper used by other runtimes. + // buildHookCommand resolves the node runner + emits "" "/hooks/". + const hookOpts = { runtime: 'cursor', platform: opts.platform || process.platform }; + // buildHookCommand('gsd-cursor-session-start.js', ...): sessionStart → context injection + // Only register the hook entry if the script was actually installed (dangling guard). + const sessionStartCmd = installedScripts.has('gsd-cursor-session-start.js') + ? buildHookCommand(targetDir, 'gsd-cursor-session-start.js', hookOpts) + : null; + // buildHookCommand('gsd-cursor-post-tool.js', ...): postToolUse → STATE.md update monitor + const postToolCmd = installedScripts.has('gsd-cursor-post-tool.js') + ? buildHookCommand(targetDir, 'gsd-cursor-post-tool.js', hookOpts) + : null; + + // Build managed entries; skip events whose command couldn't be resolved (e.g. no node). + const managedEntries = {}; + if (sessionStartCmd) { + managedEntries.sessionStart = { + type: 'command', + command: sessionStartCmd, + [GSD_CURSOR_HOOK_MARKER]: true, + }; + } + if (postToolCmd) { + managedEntries.postToolUse = { + type: 'command', + command: postToolCmd, + [GSD_CURSOR_HOOK_MARKER]: true, + }; + } + + const hooksJsonPath = path.join(targetDir, 'hooks.json'); + const result = reconcileCursorHooksJson(hooksJsonPath, managedEntries); + return { hooksJsonPath, changed: result.changed }; +} + +/** + * Remove all GSD-managed Cursor lifecycle hook entries from hooks.json. + * User-owned entries are preserved. If the file becomes empty, it is removed. + * + * @param {string} targetDir - The Cursor config dir + * @returns {{ changed: boolean }} + */ +function removeCursorHooksJson(targetDir) { + const hooksJsonPath = path.join(targetDir, 'hooks.json'); + if (!fs.existsSync(hooksJsonPath)) return { changed: false }; + const result = reconcileCursorHooksJson(hooksJsonPath, null); + // If the resulting file has no meaningful hook content, remove it. + // A file is "empty" if it contains only the scaffolding (version, empty hooks + // object, or a bare {}) with no user-authored hook entries. + if (result.changed) { + try { + const contentRaw = fs.readFileSync(hooksJsonPath, 'utf8'); + const parsed = JSON.parse(contentRaw); + // reconcileCursorHooksJson always writes the nested { version, hooks:{} } shape. + // The file is "empty" when there are no remaining hook events with entries. + const hookTable = (parsed.hooks && typeof parsed.hooks === 'object' && !Array.isArray(parsed.hooks)) + ? parsed.hooks + : {}; + const hasAnyEvents = Object.keys(hookTable).some( + (k) => Array.isArray(hookTable[k]) && hookTable[k].length > 0, + ); + if (!hasAnyEvents) { + fs.unlinkSync(hooksJsonPath); + return { changed: true }; + } + } catch { /* best-effort: leave the file */ } + } + return { changed: result.changed }; +} + /** * #786 — Build the GSD-managed GitHub Copilot lifecycle hook config object. * @@ -7804,6 +8059,8 @@ const GSD_UNINSTALL_HOOKS = [ 'gsd-check-update.js', 'gsd-check-update.cmd', 'gsd-context-monitor.js', + 'gsd-cursor-session-start.js', + 'gsd-cursor-post-tool.js', 'gsd-prompt-guard.js', 'gsd-read-guard.js', 'gsd-read-injection-scanner.js', @@ -8039,6 +8296,33 @@ function uninstall(isGlobal, runtime = 'claude') { } } + // 1b-cursor. Non-layout Cursor side-effects (issue #777): remove GSD-managed + // hook entries from hooks.json and clean up the managed hook scripts. + if (isCursor) { + const hooksJsonCleanup = removeCursorHooksJson(targetDir); + if (hooksJsonCleanup.changed) { + removedCount++; + console.log(` ${green}✓${reset} Removed GSD-managed Cursor hooks from hooks.json`); + } + // Remove the managed hook scripts (session-start + post-tool). + const hooksDir = path.join(targetDir, 'hooks'); + for (const script of [GSD_CURSOR_SESSION_HOOK_SCRIPT, GSD_CURSOR_POST_TOOL_HOOK_SCRIPT]) { + const p = path.join(hooksDir, script); + try { + if (fs.existsSync(p)) { + fs.unlinkSync(p); + removedCount++; + } + } catch { /* best-effort */ } + } + // Prune hooks/ if empty. + try { + if (fs.existsSync(hooksDir) && fs.readdirSync(hooksDir).length === 0) { + fs.rmdirSync(hooksDir); + } + } catch { /* best-effort */ } + } + // 1c. Claude local: remove commands/gsd/ (primary local install location). // The layout's _removeGsdEntries uses the 'gsd-' prefix which applies to // flat command dirs (OpenCode/Kilo). Claude local files use no prefix inside @@ -10137,8 +10421,10 @@ function install(isGlobal, runtime = 'claude', options = {}) { } // Gate hooks/lib/ install on the same runtimes that receive hooks (see line ~8702). - // Codex/Copilot/Cursor/Windsurf/Trae/Cline skip hooks entirely, so they must not - // receive the hooks/lib/ helpers either — otherwise the Codex comment downstream + // Codex/Copilot/Cursor/Windsurf/Trae/Cline do not use the shared hooks/lib/ helpers + // (Cursor uses standalone .js hook scripts registered via hooks.json; Codex uses + // hooks.json directly; the others skip hooks entirely), so they must not receive + // the hooks/lib/ helpers — otherwise the Codex comment downstream // ("we deliberately do *not* copy hooks/lib/ for Codex") is contradicted in practice. const hooksLibSrc = path.join(src, 'hooks', 'lib'); if (!isCodex && !isCopilot && !isCursor && !isWindsurf && !isTrae && !isCline && fs.existsSync(hooksLibSrc)) { @@ -10653,8 +10939,23 @@ function install(isGlobal, runtime = 'claude', options = {}) { return { settingsPath: null, settings: null, statuslineCommand: null, updateBannerCommand: null, runtime, configDir: targetDir }; } + if (configIntent.installSurface === 'cursor-hooks-json') { + // #777: Cursor v2.4+ supports hooks.json. Register sessionStart + postToolUse. + // Hook scripts are copied to /hooks/ and referenced by hooks.json. + const cursorHookResult = writeCursorHooksJson(targetDir, src, {}); + if (cursorHookResult.changed) { + console.log(` ${green}✓${reset} Configured Cursor lifecycle hooks (sessionStart, postToolUse)`); + } else { + console.log(` ${green}✓${reset} Cursor lifecycle hooks already up to date`); + } + // Re-run the manifest pass so the hook scripts + hooks.json are hash-tracked. + writeManifest(targetDir, runtime, { mode: _effectiveInstallMode }); + persistActiveProfileMarker(); + return { settingsPath: null, settings: null, statuslineCommand: null, updateBannerCommand: null, runtime, configDir: targetDir }; + } + if (configIntent.installSurface === 'profile-marker-only') { - // Cursor/Windsurf/Trae use skills — no config.toml, no settings.json hooks needed + // Windsurf/Trae use skills — no config.toml, no settings.json hooks needed persistActiveProfileMarker(); return { settingsPath: null, settings: null, statuslineCommand: null, updateBannerCommand: null, runtime, configDir: targetDir }; } @@ -12262,6 +12563,14 @@ module.exports = { buildClinePreToolUseHook, writeClineArtifacts, mergeGsdAgentsMd, + GSD_CURSOR_SESSION_HOOK_SCRIPT, + GSD_CURSOR_POST_TOOL_HOOK_SCRIPT, + GSD_CURSOR_HOOK_MARKER, + buildCursorHookEntry, + isManagedCursorHookEntry, + reconcileCursorHooksJson, + writeCursorHooksJson, + removeCursorHooksJson, stripGsdFromAgentsMd, GSD_AGENTS_MD_MARKER, GSD_AGENTS_MD_CLOSE_MARKER, diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 0c555656f..b2b1b4e08 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -802,7 +802,7 @@ The migration-specific ownership and source snapshots live in | Codex | `~/.codex` | `./.codex` | `skills/gsd-*/SKILL.md` | `agents/` source markdown plus per-agent TOML | `config.toml` `[agents.gsd-*]`, `[features].hooks` (canonical; legacy alias `codex_hooks` is recognized and migrated forward on reinstall, #3566), and hook tables | | GitHub Copilot | `~/.copilot` | `./.github` | `skills/gsd-*/SKILL.md`, `copilot-instructions.md`, and `AGENTS.md` (repo root, local) | `.agent.md` files | Self-contained `sessionStart` hook (`hooks/gsd-session.json`, inline `command` type); no statusline | | Antigravity | auto-detected: `~/.gemini/antigravity`, `~/.gemini/antigravity-ide`, or `~/.gemini/antigravity-cli` | `./.agent` | `skills/gsd-*/SKILL.md` | `agents/gsd-*.md` | Gemini-style `settings.json` hook entries when installed by GSD | -| Cursor | `~/.cursor` | `./.cursor` | `skills/gsd-*/SKILL.md` | `agents/gsd-*.md` | Rule references under `rules/`; no GSD hooks | +| Cursor | `~/.cursor` | `./.cursor` | `skills/gsd-*/SKILL.md` | `agents/gsd-*.md` | Rule references under `rules/`; `hooks.json` with sessionStart context injection and postToolUse STATE.md monitor (#777) | | Windsurf | `~/.codeium/windsurf` | `./.windsurf` | `skills/gsd-*/SKILL.md` | `agents/gsd-*.md` | Rule references under `rules/`; no GSD hooks | | Augment Code | `~/.augment` | `./.augment` | `skills/gsd-*/SKILL.md` | `agents/gsd-*.md` | No GSD hooks or statusline | | Trae | `~/.trae` | `./.trae` | `skills/gsd-*/SKILL.md` | `agents/gsd-*.md` | Rule references under `rules/`; no GSD hooks | diff --git a/docs/INVENTORY-MANIFEST.json b/docs/INVENTORY-MANIFEST.json index 046b487c2..d3def7871 100644 --- a/docs/INVENTORY-MANIFEST.json +++ b/docs/INVENTORY-MANIFEST.json @@ -360,6 +360,8 @@ "gsd-check-update-worker.js", "gsd-check-update.js", "gsd-context-monitor.js", + "gsd-cursor-post-tool.js", + "gsd-cursor-session-start.js", "gsd-graphify-update.sh", "gsd-phase-boundary.sh", "gsd-prompt-guard.js", diff --git a/docs/INVENTORY.md b/docs/INVENTORY.md index bb422cd18..d7bc3964b 100644 --- a/docs/INVENTORY.md +++ b/docs/INVENTORY.md @@ -471,7 +471,7 @@ Full listing: `gsd-core/bin/lib/*.cjs`. --- -## Hooks (14 shipped) +## Hooks (16 shipped) Full listing: `hooks/`. @@ -482,6 +482,8 @@ Full listing: `hooks/`. | `gsd-check-update.js` | `SessionStart` | Background check for new GSD versions | | `gsd-check-update-worker.js` | (worker) | Background worker helper for check-update | | `gsd-update-banner.js` | `SessionStart` | Opt-in banner surfacing update availability when GSD statusline isn't used (PR #2795) | +| `gsd-cursor-session-start.js` | Cursor `sessionStart` | Cursor-native context injection at session start (issue #777) | +| `gsd-cursor-post-tool.js` | Cursor `postToolUse` | Cursor-native STATE.md update monitor after tool calls (issue #777) | | `gsd-prompt-guard.js` | `PreToolUse` | Scans `.planning/` writes for prompt-injection patterns (advisory) | | `gsd-workflow-guard.js` | `PreToolUse` | Detects file edits outside GSD workflow context (advisory, opt-in) | | `gsd-read-guard.js` | `PreToolUse` | Advisory guard preventing Edit/Write on unread files | diff --git a/docs/installer-migrations.md b/docs/installer-migrations.md index 0cacbc98b..2d8c33197 100644 --- a/docs/installer-migrations.md +++ b/docs/installer-migrations.md @@ -370,7 +370,7 @@ for the new shape before changing migration behavior. | Codex | Skills in `skills/gsd-*/SKILL.md`; agents as source markdown plus per-agent TOML in `agents/`; `[agents.gsd-*]` and hooks in `config.toml` | Global `CODEX_HOME` or `~/.codex`; local `./.codex` | GSD owns generated skills, generated agent TOML, `agents.gsd-*` config sections, `[features].hooks` when added by GSD (canonical; legacy alias `codex_hooks` is recognized and migrated forward, #3566), and GSD hook entries | [Codex config schema](https://developers.openai.com/codex/config-schema.json), [Codex developer docs](https://developers.openai.com/codex/); docs not versioned, checked 2026-05-15; installer compatibility sentinel: Codex 0.130.0 features.hooks key (legacy `codex_hooks` recognized) | | GitHub Copilot | Skills in `skills/gsd-*/SKILL.md`; agents as `.agent.md`; repository instructions in `copilot-instructions.md` | Global `COPILOT_CONFIG_DIR`, `COPILOT_HOME`, or `~/.copilot`; local `./.github` | GSD owns generated skill/agent files and GSD-authored instruction files; no hook/statusline ownership | [Repository custom instructions](https://docs.github.com/en/copilot/how-tos/configure-custom-instructions/add-repository-instructions), [Copilot CLI custom instructions](https://docs.github.com/en/copilot/how-tos/copilot-cli/add-custom-instructions); GitHub Docs product docs, checked 2026-05-11 | | Antigravity | Skills in `skills/gsd-*/SKILL.md`; agents in `agents/`; Gemini-style `settings.json` hooks when installed by GSD | Global `ANTIGRAVITY_CONFIG_DIR` or `~/.gemini/antigravity`; local `./.agent` | GSD owns generated skills/agents/hooks and GSD settings entries only | Public Antigravity install/config docs for this file layout were not stable or complete as of 2026-05-11; installer compatibility therefore uses GSD's Gemini-compatible settings policy, documented shim baseline. | -| Cursor | Skills in `skills/gsd-*/SKILL.md`; agents in `agents/`; rule references under `rules/` | Global `CURSOR_CONFIG_DIR` or `~/.cursor`; local `./.cursor` | GSD owns generated skills/agents and GSD rule files or references; no hook/statusline ownership | [Cursor rules](https://docs.cursor.com/context/rules); docs not versioned, checked 2026-05-11 | +| Cursor | Skills in `skills/gsd-*/SKILL.md`; agents in `agents/`; rule references under `rules/`; lifecycle hooks via `hooks.json` (sessionStart + postToolUse, #777) | Global `CURSOR_CONFIG_DIR` or `~/.cursor`; local `./.cursor` | GSD owns generated skills/agents, GSD rule files or references, and GSD-managed `hooks.json` entries (sentinel `gsd-managed:true`); no statusline ownership | [Cursor rules](https://docs.cursor.com/context/rules); [Cursor hooks](https://docs.cursor.com/context/hooks); docs not versioned, checked 2026-06-07 | | Windsurf | Skills in `skills/gsd-*/SKILL.md`; agents in `agents/`; rule references under `rules/` | Global `WINDSURF_CONFIG_DIR` or `~/.codeium/windsurf`; local `./.windsurf` | GSD owns generated skills/agents and GSD rule files or references; no hook/statusline ownership | Windsurf public rule docs were source-limited in search results as of 2026-05-11; installer targets the common workspace rules convention `./.windsurf/rules` and must be rechecked before migrations rewrite rules | | Augment Code | Skills in `skills/gsd-*/SKILL.md`; agents in `agents/` | Global `AUGMENT_CONFIG_DIR` or `~/.augment`; local `./.augment` | GSD owns generated skills/agents only; no hook/statusline ownership | [Augment Agent Skills](https://docs.augmentcode.com/cli/skills), [Augment IDE skills](https://docs.augmentcode.com/using-augment/skills); IDE skills public beta in VS Code 0.789.0+, checked 2026-05-11 | | Trae | Skills in `skills/gsd-*/SKILL.md`; agents in `agents/`; rule references under `rules/` | Global `TRAE_CONFIG_DIR` or `~/.trae`; local `./.trae` | GSD owns generated skills/agents and GSD rule files or references; no hook/statusline ownership | Public Trae docs expose AI settings and `.rules` announcements, but no stable skills/config API was found as of 2026-05-11; migrations must treat this row as source-limited | diff --git a/hooks/gsd-cursor-post-tool.js b/hooks/gsd-cursor-post-tool.js new file mode 100644 index 000000000..7f7dec769 --- /dev/null +++ b/hooks/gsd-cursor-post-tool.js @@ -0,0 +1,75 @@ +#!/usr/bin/env node +// gsd-hook-version: {{GSD_VERSION}} +// gsd-cursor-post-tool.js — Cursor postToolUse hook (issue #777) +// +// Cursor invokes this script after each tool call completes. +// Protocol: JSON from Cursor on stdin; JSON response on stdout. +// +// Input schema (cursor postToolUse): +// { tool_name, tool_input, tool_output, duration, +// conversation_id, generation_id, model, hook_event_name, +// cursor_version, workspace_roots, user_email, transcript_path } +// +// Output schema (cursor postToolUse): +// { additional_context?: string } ← injected as context after the tool use +// +// Behaviour: +// - After a write-class tool that targets .planning/, reminds the agent +// to keep STATE.md current. +// - Fails open: any error silently exits 0. +// +// Cursor docs: https://cursor.com/docs/hooks + +'use strict'; + +const WRITE_TOOL_RE = /write|edit|replace|create|delete|remove|append|apply|patch|insert|mkdir/i; +const PATH_KEY_RE = /^(path|file|file_?path|filepath|target_?path|target|dir|directory|uri|filename)$/i; +const PLANNING_PATH_RE = /(^|[\\/])\.planning([\\/]|$)/; + +let raw = ''; +const stdinTimeout = setTimeout(() => { + // Timeout guard: exit silently rather than hanging. + process.exit(0); +}, 10000); + +process.stdin.setEncoding('utf8'); +process.stdin.on('data', (chunk) => { raw += chunk; }); +process.stdin.on('end', () => { + clearTimeout(stdinTimeout); + try { + let input; + try { input = JSON.parse(raw || '{}'); } catch { process.stdout.write(JSON.stringify({})); return; } + + const toolName = String( + input.tool_name || input.toolName || '' + ).toLowerCase(); + + const isWrite = WRITE_TOOL_RE.test(toolName); + if (!isWrite) { process.stdout.write(JSON.stringify({})); return; } + + // Collect only PATH-bearing field values (not free-form content). + const paths = []; + const walk = (v, depth) => { + if (depth > 5 || paths.length > 64) return; + if (Array.isArray(v)) { for (const x of v) walk(x, depth + 1); return; } + if (v && typeof v === 'object') { + for (const k of Object.keys(v)) { + const val = v[k]; + if (typeof val === 'string' && PATH_KEY_RE.test(k)) paths.push(val); + else walk(val, depth + 1); + } + } + }; + walk(input.tool_input || input.toolInput || {}, 0); + + if (paths.some((p) => PLANNING_PATH_RE.test(p))) { + process.stdout.write(JSON.stringify({ + additional_context: + 'GSD: .planning/ artifact updated — ensure STATE.md reflects the latest phase and progress.', + })); + return; + } + } catch { /* fall through to empty response */ } + + process.stdout.write(JSON.stringify({})); +}); diff --git a/hooks/gsd-cursor-session-start.js b/hooks/gsd-cursor-session-start.js new file mode 100644 index 000000000..dbc8c97ef --- /dev/null +++ b/hooks/gsd-cursor-session-start.js @@ -0,0 +1,52 @@ +#!/usr/bin/env node +// gsd-hook-version: {{GSD_VERSION}} +// gsd-cursor-session-start.js — Cursor sessionStart hook (issue #777) +// +// Cursor invokes this script at the start of each agent session. +// Protocol: JSON from Cursor on stdin; JSON response on stdout. +// +// Input schema (cursor sessionStart): +// { session_id, is_background_agent, composer_mode, conversation_id, +// generation_id, model, hook_event_name, cursor_version, +// workspace_roots, user_email, transcript_path } +// +// Output schema (cursor sessionStart): +// { additional_context?: string } ← injected into the session as context +// +// Behaviour: +// - If .planning/STATE.md is present, injects a brief state reminder. +// - If absent, nudges the user toward /gsd:new-project. +// - Fails open: any error silently exits 0 so a hook bug never wedges Cursor. +// +// Cursor docs: https://cursor.com/docs/hooks + +'use strict'; + +const fs = require('fs'); +const path = require('path'); + +const MSG_PRESENT = + 'GSD: .planning/STATE.md is present — review the current phase and any blockers before acting.'; +const MSG_ABSENT = + 'GSD: no .planning/ workflow found — run /gsd:new-project to start a tracked workflow.'; + +let raw = ''; +const stdinTimeout = setTimeout(() => { + // Timeout guard: exit silently rather than hanging. + process.exit(0); +}, 10000); + +process.stdin.setEncoding('utf8'); +process.stdin.on('data', (chunk) => { raw += chunk; }); +process.stdin.on('end', () => { + clearTimeout(stdinTimeout); + try { + const statePath = path.join(process.cwd(), '.planning', 'STATE.md'); + const statePresent = fs.existsSync(statePath); + const msg = statePresent ? MSG_PRESENT : MSG_ABSENT; + process.stdout.write(JSON.stringify({ additional_context: msg })); + } catch { + // Fail open — never block a Cursor session because of a GSD hook error. + process.stdout.write(JSON.stringify({})); + } +}); diff --git a/hooks/managed-hooks-registry.cjs b/hooks/managed-hooks-registry.cjs index e29c7bb3c..77aa4982b 100644 --- a/hooks/managed-hooks-registry.cjs +++ b/hooks/managed-hooks-registry.cjs @@ -19,6 +19,8 @@ const MANAGED_HOOKS = [ 'gsd-check-update-worker.js', 'gsd-check-update.js', 'gsd-context-monitor.js', + 'gsd-cursor-post-tool.js', + 'gsd-cursor-session-start.js', 'gsd-graphify-update.sh', 'gsd-phase-boundary.sh', 'gsd-prompt-guard.js', diff --git a/scripts/build-hooks.js b/scripts/build-hooks.js index 3072a5e50..c566a18cd 100644 --- a/scripts/build-hooks.js +++ b/scripts/build-hooks.js @@ -30,6 +30,9 @@ const HOOKS_TO_COPY = [ // so require('./managed-hooks-registry.cjs') resolves in the installed hooks/ dir. 'managed-hooks-registry.cjs', 'gsd-context-monitor.js', + // Cursor lifecycle hooks (issue #777): sessionStart context injection + postToolUse monitor + 'gsd-cursor-session-start.js', + 'gsd-cursor-post-tool.js', 'gsd-prompt-guard.js', 'gsd-read-guard.js', 'gsd-read-injection-scanner.js', diff --git a/src/installer-migration-report.cts b/src/installer-migration-report.cts index 536de56f1..3e41512fa 100644 --- a/src/installer-migration-report.cts +++ b/src/installer-migration-report.cts @@ -30,6 +30,8 @@ export const BUNDLED_GSD_HOOK_FILES: ReadonlySet = Object.freeze(new Set 'hooks/gsd-check-update-worker.js', 'hooks/gsd-check-update.js', 'hooks/gsd-context-monitor.js', + 'hooks/gsd-cursor-post-tool.js', + 'hooks/gsd-cursor-session-start.js', 'hooks/gsd-graphify-update.sh', 'hooks/gsd-phase-boundary.sh', 'hooks/gsd-prompt-guard.js', diff --git a/src/installer-migrations/000-first-time-baseline.cts b/src/installer-migrations/000-first-time-baseline.cts index c81f30e6a..a7c68ace2 100644 --- a/src/installer-migrations/000-first-time-baseline.cts +++ b/src/installer-migrations/000-first-time-baseline.cts @@ -23,7 +23,7 @@ const RUNTIME_SURFACES: Record = { kilo: ['gsd-core', 'command', 'skills', 'agents'], copilot: ['gsd-core', 'skills', 'agents'], antigravity: ['gsd-core', 'skills', 'agents'], - cursor: ['gsd-core', 'skills', 'agents'], + cursor: ['gsd-core', 'skills', 'agents', 'hooks', 'hooks.json'], windsurf: ['gsd-core', 'skills', 'agents', 'rules'], augment: ['gsd-core', 'skills', 'agents'], trae: ['gsd-core', 'skills', 'agents', 'rules'], diff --git a/src/runtime-config-adapter-registry.cts b/src/runtime-config-adapter-registry.cts index 3bb690471..68d1b4d90 100644 --- a/src/runtime-config-adapter-registry.cts +++ b/src/runtime-config-adapter-registry.cts @@ -11,6 +11,7 @@ * 'codex-toml' → early-return after writing codex.toml. * 'copilot-instructions' → early-return after writing .github/copilot-instructions.md. * 'cline-rules' → early-return after writing .clinerules. + * 'cursor-hooks-json' → early-return after writing .cursor/hooks.json (issue #777). * 'profile-marker-only' → early-return after writing only the profile marker. * - `writesSharedSettings` is the finishInstall writeSettings gate: * false for codex / copilot / kilo / cursor / windsurf / trae / cline (legacy exclusion list). @@ -30,6 +31,7 @@ type ConfigInstallSurface = | 'codex-toml' | 'copilot-instructions' | 'cline-rules' + | 'cursor-hooks-json' | 'profile-marker-only'; type FinishPermissionWriter = 'opencode' | 'kilo' | null; @@ -64,7 +66,7 @@ const REGISTRY: Record> = Object.freeze({ codex: Object.freeze({ installSurface: 'codex-toml', writesSharedSettings: false, finishPermissionWriter: null } as const), copilot: Object.freeze({ installSurface: 'copilot-instructions', writesSharedSettings: false, finishPermissionWriter: null } as const), cline: Object.freeze({ installSurface: 'cline-rules', writesSharedSettings: false, finishPermissionWriter: null } as const), - cursor: Object.freeze({ installSurface: 'profile-marker-only', writesSharedSettings: false, finishPermissionWriter: null } as const), + cursor: Object.freeze({ installSurface: 'cursor-hooks-json', writesSharedSettings: false, finishPermissionWriter: null } as const), windsurf: Object.freeze({ installSurface: 'profile-marker-only', writesSharedSettings: false, finishPermissionWriter: null } as const), trae: Object.freeze({ installSurface: 'profile-marker-only', writesSharedSettings: false, finishPermissionWriter: null } as const), }); @@ -82,6 +84,7 @@ const INSTALL_SURFACES: ReadonlyArray = Object.freeze([ 'codex-toml', 'copilot-instructions', 'cline-rules', + 'cursor-hooks-json', 'profile-marker-only', ]); diff --git a/tests/bug-376-claude-js-hook-gsd-rewriter.test.cjs b/tests/bug-376-claude-js-hook-gsd-rewriter.test.cjs index 12d66b7b1..22ea002be 100644 --- a/tests/bug-376-claude-js-hook-gsd-rewriter.test.cjs +++ b/tests/bug-376-claude-js-hook-gsd-rewriter.test.cjs @@ -32,6 +32,20 @@ const { cleanup } = require('./helpers.cjs'); 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 BUILD_HOOKS_SCRIPT = path.join(REPO_ROOT, 'scripts', 'build-hooks.js'); + +/** + * Ensure hooks/dist is populated before any suite that reads it. + * hooks/dist/ is gitignored and only produced by `npm run build:hooks`. + * In CI the scoped/windows test jobs do NOT run build:hooks before running + * tests, so the first test that needs hooks/dist would fail. This mirrors + * the pattern used in bug-3357-codex-legacy-hooks-json-migration.test.cjs. + */ +function ensureHooksDist() { + if (!fs.existsSync(HOOKS_DIST_DIR) || fs.readdirSync(HOOKS_DIST_DIR).filter(f => f.endsWith('.js')).length === 0) { + execFileSync(process.execPath, [BUILD_HOOKS_SCRIPT], { stdio: 'pipe' }); + } +} // --------------------------------------------------------------------------- // Helpers @@ -92,6 +106,12 @@ function colonRefs(content) { // Prerequisite: hooks/dist must exist (built by `npm run build:hooks`) // --------------------------------------------------------------------------- describe('bug #376 — prerequisite: hooks/dist is present', () => { + before(() => { + // hooks/dist is gitignored; build it on demand so this test is + // deterministic in CI scoped/windows jobs that don't pre-run build:hooks. + ensureHooksDist(); + }); + test('hooks/dist directory exists (run npm run build:hooks if missing)', () => { assert.ok( fs.existsSync(HOOKS_DIST_DIR), @@ -189,11 +209,13 @@ describe('bug #376 — Suite 1: Claude install rewrites /gsd: → /gsd- in hook // --------------------------------------------------------------------------- // 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/gsd-core/*.js etc). We verify that Cursor's -// installed .js files under .cursor/ have no /gsd: colon refs. +// Note: Cursor installs its own hooks (gsd-cursor-session-start.js and +// gsd-cursor-post-tool.js) via the cursor-hooks-json installSurface (issue #777). +// It does NOT install the bundled Claude-style hooks/dist files (no gsd-session-state.sh +// etc.). The Cursor /gsd: rewrite applies in `copyWithPathReplacement` to JS files +// under the agent/skill tree (.cursor/gsd-core/*.js etc). We verify that Cursor's +// installed .js files under .cursor/ have no /gsd: colon refs, and that the hooks/ +// directory contains only the Cursor-specific managed hooks. // --------------------------------------------------------------------------- describe('bug #376 — Suite 2: Cursor install still rewrites /gsd: → /gsd- (regression)', () => { let tmpDir; @@ -242,15 +264,32 @@ describe('bug #376 — Suite 2: Cursor install still rewrites /gsd: → /gsd- (r ); }); - 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.). + test('2c: Cursor install creates a hooks/ directory with only Cursor-specific managed hooks', () => { + // Since issue #777, Cursor installs gsd-cursor-session-start.js and + // gsd-cursor-post-tool.js into /hooks/. These are Cursor-native + // hooks — NOT the bundled Claude-style hooks (no gsd-session-state.sh etc.). + // Verify: hooks/ exists AND does NOT contain any Claude-bundled hooks. const hooksDir = path.join(tmpDir, '.cursor', 'hooks'); - assert.strictEqual( + assert.ok( fs.existsSync(hooksDir), - false, - 'Cursor install must NOT create a hooks/ directory — Cursor skips the hooks install step', + 'Cursor install must create a hooks/ directory for its managed hook scripts (#777)', + ); + const CLAUDE_BUNDLED_HOOKS = ['gsd-session-state.sh', 'gsd-context-monitor.js', 'gsd-statusline.js']; + for (const hook of CLAUDE_BUNDLED_HOOKS) { + assert.strictEqual( + fs.existsSync(path.join(hooksDir, hook)), + false, + `Cursor hooks/ must NOT contain Claude-bundled hook ${hook} — only Cursor-native hooks are installed`, + ); + } + // The two Cursor-specific managed hooks must be present. + assert.ok( + fs.existsSync(path.join(hooksDir, 'gsd-cursor-session-start.js')), + 'gsd-cursor-session-start.js must be installed in .cursor/hooks/ (#777)', + ); + assert.ok( + fs.existsSync(path.join(hooksDir, 'gsd-cursor-post-tool.js')), + 'gsd-cursor-post-tool.js must be installed in .cursor/hooks/ (#777)', ); }); }); @@ -262,6 +301,9 @@ describe('bug #376 — Suite 3: hooks/ source files are unchanged by install', ( let snapshotBefore; before(() => { + // Ensure hooks/dist is built before snapshotting; it may be absent in CI + // scoped/windows jobs that don't pre-run build:hooks (#777 fix). + ensureHooksDist(); // Snapshot hooks/dist JS files before any install in this suite snapshotBefore = {}; if (fs.existsSync(HOOKS_DIST_DIR)) { diff --git a/tests/cursor-hooks.test.cjs b/tests/cursor-hooks.test.cjs new file mode 100644 index 000000000..fcbd0fc34 --- /dev/null +++ b/tests/cursor-hooks.test.cjs @@ -0,0 +1,462 @@ +/** + * Tests for Cursor hooks.json lifecycle hook registration (issue #777). + * + * Cursor v2.4+ supports a hooks.json system with events including sessionStart + * and postToolUse. GSD registers two managed command hooks so Cursor users get + * baseline parity with Claude Code and Gemini. + * + * Test plan: + * T1 reconcileCursorHooksJson — creates new hooks.json with both events + * T2 reconcileCursorHooksJson — idempotent (re-run writes same content) + * T3 reconcileCursorHooksJson — preserves user-owned entries in sessionStart + * T4 reconcileCursorHooksJson — preserves user-owned entries in postToolUse + * T5 reconcileCursorHooksJson — remove-only (managedEntries=null) strips managed, keeps user + * T6 reconcileCursorHooksJson — handles nested { version, hooks: {...} } shape + * T7 reconcileCursorHooksJson — handles flat (no version) shape + * T8 reconcileCursorHooksJson — corrupted JSON throws descriptive error + * T9 isManagedCursorHookEntry — returns true for GSD-marked entries + * T10 isManagedCursorHookEntry — returns false for user entries + * T11 buildCursorHookEntry — emits correct shape with marker + * T12 removeCursorHooksJson — removes hooks.json when it becomes empty + * T13 removeCursorHooksJson — preserves file when user entries remain + * T14 runtime-config-adapter — cursor now has 'cursor-hooks-json' surface + * T15 INSTALL_SURFACES — 'cursor-hooks-json' in the valid surfaces list + * T16 Hook scripts exist in hooks/ + * T17 Hook script content — sessionStart script emits JSON with additional_context + * T18 Hook script content — postToolUse script emits JSON {} for non-write tools + * T19 GSD_CURSOR_HOOK_MARKER constant is exported + * T20 GSD_CURSOR_SESSION_HOOK_SCRIPT / GSD_CURSOR_POST_TOOL_HOOK_SCRIPT constants + */ + +// allow-test-rule: source-text-is-the-product +// Hook script text IS what Cursor loads. Testing script content tests the deployed contract. + +'use strict'; + +process.env.GSD_TEST_MODE = '1'; + +const { test, describe } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); +const os = require('node:os'); +const { createTempDir, cleanup } = require('./helpers.cjs'); + +const { + reconcileCursorHooksJson, + isManagedCursorHookEntry, + buildCursorHookEntry, + removeCursorHooksJson, + GSD_CURSOR_HOOK_MARKER, + GSD_CURSOR_SESSION_HOOK_SCRIPT, + GSD_CURSOR_POST_TOOL_HOOK_SCRIPT, +} = require('../bin/install.js'); + +const { + resolveRuntimeConfigIntent, + INSTALL_SURFACES, +} = require('../gsd-core/bin/lib/runtime-config-adapter-registry.cjs'); + +// --------------------------------------------------------------------------- +// Helpers +// --------------------------------------------------------------------------- + +function makeHooksJson(obj) { + return JSON.stringify(obj, null, 2) + '\n'; +} + +function readHooksJson(dir) { + const p = path.join(dir, 'hooks.json'); + if (!fs.existsSync(p)) return null; + return JSON.parse(fs.readFileSync(p, 'utf8')); +} + +function managedEntry(command) { + return { type: 'command', command, [GSD_CURSOR_HOOK_MARKER]: true }; +} + +function userEntry(command) { + return { type: 'command', command }; +} + +// --------------------------------------------------------------------------- +// T1: Creates new hooks.json with both events +// --------------------------------------------------------------------------- +describe('reconcileCursorHooksJson', () => { + test('T1: creates new hooks.json with sessionStart and postToolUse', (t) => { + const dir = createTempDir(); + t.after(() => cleanup(dir)); + + const hooksJsonPath = path.join(dir, 'hooks.json'); + const result = reconcileCursorHooksJson(hooksJsonPath, { + sessionStart: managedEntry('/node /path/gsd-cursor-session-start.js'), + postToolUse: managedEntry('/node /path/gsd-cursor-post-tool.js'), + }); + + assert.equal(result.wrote, true); + assert.equal(result.changed, true); + + const parsed = readHooksJson(dir); + assert.ok(parsed, 'hooks.json must exist'); + + // Cursor requires the canonical nested { "version": 1, "hooks": { ... } } shape. + assert.equal(parsed.version, 1, 'hooks.json must have version: 1'); + assert.ok(parsed.hooks && typeof parsed.hooks === 'object', 'hooks.json must have top-level hooks object'); + const hookTable = parsed.hooks; + assert.ok(Array.isArray(hookTable.sessionStart), 'sessionStart must be an array'); + assert.ok(Array.isArray(hookTable.postToolUse), 'postToolUse must be an array'); + assert.equal(hookTable.sessionStart.length, 1); + assert.equal(hookTable.postToolUse.length, 1); + assert.equal(hookTable.sessionStart[0][GSD_CURSOR_HOOK_MARKER], true); + assert.equal(hookTable.postToolUse[0][GSD_CURSOR_HOOK_MARKER], true); + }); + + // --------------------------------------------------------------------------- + // T2: Idempotent + // --------------------------------------------------------------------------- + test('T2: idempotent — second run produces same content (no write)', (t) => { + const dir = createTempDir(); + t.after(() => cleanup(dir)); + + const hooksJsonPath = path.join(dir, 'hooks.json'); + const entries = { + sessionStart: managedEntry('/node /gsd-cursor-session-start.js'), + postToolUse: managedEntry('/node /gsd-cursor-post-tool.js'), + }; + + reconcileCursorHooksJson(hooksJsonPath, entries); + const result2 = reconcileCursorHooksJson(hooksJsonPath, entries); + + assert.equal(result2.wrote, false, 'second run must not write (idempotent)'); + assert.equal(result2.changed, false, 'content must not change on second run'); + }); + + // --------------------------------------------------------------------------- + // T3: Preserves user-owned entries in sessionStart + // --------------------------------------------------------------------------- + test('T3: preserves user-owned entries in sessionStart', (t) => { + const dir = createTempDir(); + t.after(() => cleanup(dir)); + + const hooksJsonPath = path.join(dir, 'hooks.json'); + // Pre-populate with user-owned entry. + fs.writeFileSync(hooksJsonPath, makeHooksJson({ + version: 1, + hooks: { + sessionStart: [userEntry('/my-team/hook.sh')], + }, + })); + + reconcileCursorHooksJson(hooksJsonPath, { + sessionStart: managedEntry('/node /gsd-cursor-session-start.js'), + postToolUse: managedEntry('/node /gsd-cursor-post-tool.js'), + }); + + const parsed = readHooksJson(dir); + const hookTable = parsed.hooks && typeof parsed.hooks === 'object' ? parsed.hooks : parsed; + assert.ok(Array.isArray(hookTable.sessionStart)); + // Both user and GSD entries must survive. + assert.equal(hookTable.sessionStart.length, 2, 'user + GSD entry must coexist'); + const userStays = hookTable.sessionStart.some((e) => e.command === '/my-team/hook.sh'); + const gsdAdded = hookTable.sessionStart.some((e) => e[GSD_CURSOR_HOOK_MARKER]); + assert.ok(userStays, 'user-owned entry must be preserved'); + assert.ok(gsdAdded, 'GSD managed entry must be present'); + }); + + // --------------------------------------------------------------------------- + // T4: Preserves user-owned entries in postToolUse + // --------------------------------------------------------------------------- + test('T4: preserves user-owned entries in postToolUse', (t) => { + const dir = createTempDir(); + t.after(() => cleanup(dir)); + + const hooksJsonPath = path.join(dir, 'hooks.json'); + fs.writeFileSync(hooksJsonPath, makeHooksJson({ + hooks: { + postToolUse: [userEntry('/user/post-tool.sh')], + }, + })); + + reconcileCursorHooksJson(hooksJsonPath, { + sessionStart: managedEntry('/node /gsd-cursor-session-start.js'), + postToolUse: managedEntry('/node /gsd-cursor-post-tool.js'), + }); + + const parsed = readHooksJson(dir); + const hookTable = parsed.hooks && typeof parsed.hooks === 'object' ? parsed.hooks : parsed; + assert.ok(Array.isArray(hookTable.postToolUse)); + assert.equal(hookTable.postToolUse.length, 2); + assert.ok(hookTable.postToolUse.some((e) => e.command === '/user/post-tool.sh')); + }); + + // --------------------------------------------------------------------------- + // T5: Remove-only (managedEntries=null) strips managed, keeps user entries + // --------------------------------------------------------------------------- + test('T5: remove-only (null managedEntries) strips GSD entries, preserves user', (t) => { + const dir = createTempDir(); + t.after(() => cleanup(dir)); + + const hooksJsonPath = path.join(dir, 'hooks.json'); + fs.writeFileSync(hooksJsonPath, makeHooksJson({ + version: 1, + hooks: { + sessionStart: [ + userEntry('/user/session.sh'), + managedEntry('/node /gsd-cursor-session-start.js'), + ], + postToolUse: [ + managedEntry('/node /gsd-cursor-post-tool.js'), + ], + }, + })); + + reconcileCursorHooksJson(hooksJsonPath, null); + + const parsed = readHooksJson(dir); + const hookTable = parsed.hooks && typeof parsed.hooks === 'object' ? parsed.hooks : parsed; + // sessionStart user entry survives; postToolUse key should be absent. + assert.ok(Array.isArray(hookTable.sessionStart), 'sessionStart must remain (user entry)'); + assert.equal(hookTable.sessionStart.length, 1, 'only user entry remains'); + assert.equal(hookTable.sessionStart[0].command, '/user/session.sh'); + assert.equal(hookTable.postToolUse, undefined, 'postToolUse key must be removed (was GSD-only)'); + }); + + // --------------------------------------------------------------------------- + // T6: Handles nested { version, hooks: {...} } shape + // --------------------------------------------------------------------------- + test('T6: handles nested { version, hooks } shape', (t) => { + const dir = createTempDir(); + t.after(() => cleanup(dir)); + + const hooksJsonPath = path.join(dir, 'hooks.json'); + fs.writeFileSync(hooksJsonPath, makeHooksJson({ version: 1, hooks: {} })); + + reconcileCursorHooksJson(hooksJsonPath, { + sessionStart: managedEntry('/node /gsd-cursor-session-start.js'), + }); + + const parsed = readHooksJson(dir); + assert.ok(parsed.hooks, 'top-level hooks key must be preserved'); + assert.equal(parsed.version, 1, 'version must be preserved'); + assert.ok(Array.isArray(parsed.hooks.sessionStart)); + }); + + // --------------------------------------------------------------------------- + // T7: Handles flat (no version) shape + // --------------------------------------------------------------------------- + test('T7: handles flat shape (no wrapper object)', (t) => { + const dir = createTempDir(); + t.after(() => cleanup(dir)); + + const hooksJsonPath = path.join(dir, 'hooks.json'); + fs.writeFileSync(hooksJsonPath, makeHooksJson({ sessionStart: [] })); + + reconcileCursorHooksJson(hooksJsonPath, { + sessionStart: managedEntry('/node /gsd-cursor-session-start.js'), + }); + + const parsed = readHooksJson(dir); + // Flat input is migrated to nested canonical shape: { version: 1, hooks: { ... } }. + assert.equal(parsed.version, 1, 'migrated flat shape must get version: 1'); + assert.ok(parsed.hooks && typeof parsed.hooks === 'object', 'migrated shape must have hooks object'); + assert.ok(Array.isArray(parsed.hooks.sessionStart), 'sessionStart must be in hooks object after migration'); + }); + + // --------------------------------------------------------------------------- + // T8: Corrupted JSON throws descriptive error + // --------------------------------------------------------------------------- + test('T8: throws descriptive error for corrupted hooks.json', (t) => { + const dir = createTempDir(); + t.after(() => cleanup(dir)); + + const hooksJsonPath = path.join(dir, 'hooks.json'); + fs.writeFileSync(hooksJsonPath, '{ not valid json }'); + + assert.throws( + () => reconcileCursorHooksJson(hooksJsonPath, { sessionStart: managedEntry('/cmd') }), + (err) => { + assert.match(err.message, /Cursor hooks\.json parse failed/i); + return true; + } + ); + }); +}); + +// --------------------------------------------------------------------------- +// T9-T11: Entry helpers +// --------------------------------------------------------------------------- +describe('isManagedCursorHookEntry / buildCursorHookEntry', () => { + test('T9: isManagedCursorHookEntry returns true for GSD-marked entry', () => { + const entry = { type: 'command', command: '/x', [GSD_CURSOR_HOOK_MARKER]: true }; + assert.equal(isManagedCursorHookEntry(entry), true); + }); + + test('T10: isManagedCursorHookEntry returns false for user-owned entry', () => { + const entry = { type: 'command', command: '/user/hook.sh' }; + assert.equal(isManagedCursorHookEntry(entry), false); + assert.equal(isManagedCursorHookEntry(null), false); + assert.equal(isManagedCursorHookEntry({}), false); + }); + + test('T11: buildCursorHookEntry emits correct shape', () => { + const entry = buildCursorHookEntry('/usr/local/bin/node /path/to/hook.js'); + assert.equal(entry.type, 'command'); + assert.equal(entry[GSD_CURSOR_HOOK_MARKER], true); + assert.ok(typeof entry.command === 'string'); + // Forward slashes only. + assert.ok(!entry.command.includes('\\'), 'command must use forward slashes'); + }); +}); + +// --------------------------------------------------------------------------- +// T12-T13: removeCursorHooksJson +// --------------------------------------------------------------------------- +describe('removeCursorHooksJson', () => { + test('T12: removes hooks.json when it becomes empty after GSD removal', (t) => { + const dir = createTempDir(); + t.after(() => cleanup(dir)); + + const hooksJsonPath = path.join(dir, 'hooks.json'); + // GSD-only file. + fs.writeFileSync(hooksJsonPath, makeHooksJson({ + version: 1, + hooks: { + sessionStart: [managedEntry('/node /gsd-cursor-session-start.js')], + postToolUse: [managedEntry('/node /gsd-cursor-post-tool.js')], + }, + })); + + const result = removeCursorHooksJson(dir); + assert.equal(result.changed, true); + // File should be removed (was GSD-only). + assert.equal(fs.existsSync(hooksJsonPath), false, 'empty hooks.json must be removed'); + }); + + test('T13: preserves hooks.json when user entries remain', (t) => { + const dir = createTempDir(); + t.after(() => cleanup(dir)); + + const hooksJsonPath = path.join(dir, 'hooks.json'); + fs.writeFileSync(hooksJsonPath, makeHooksJson({ + version: 1, + hooks: { + sessionStart: [ + managedEntry('/node /gsd-cursor-session-start.js'), + userEntry('/user/session.sh'), + ], + }, + })); + + removeCursorHooksJson(dir); + + assert.ok(fs.existsSync(hooksJsonPath), 'hooks.json must remain (user entries present)'); + const parsed = readHooksJson(dir); + const hookTable = parsed.hooks && typeof parsed.hooks === 'object' ? parsed.hooks : parsed; + assert.equal(hookTable.sessionStart.length, 1); + assert.equal(hookTable.sessionStart[0].command, '/user/session.sh'); + }); +}); + +// --------------------------------------------------------------------------- +// T14: runtime-config-adapter — cursor now has 'cursor-hooks-json' surface +// --------------------------------------------------------------------------- +test('T14: cursor runtime has installSurface cursor-hooks-json', () => { + const intent = resolveRuntimeConfigIntent('cursor'); + assert.equal(intent.installSurface, 'cursor-hooks-json'); + assert.equal(intent.writesSharedSettings, false); + assert.equal(intent.finishPermissionWriter, null); +}); + +// --------------------------------------------------------------------------- +// T15: INSTALL_SURFACES includes 'cursor-hooks-json' +// --------------------------------------------------------------------------- +test('T15: INSTALL_SURFACES includes cursor-hooks-json', () => { + assert.ok( + INSTALL_SURFACES.includes('cursor-hooks-json'), + "'cursor-hooks-json' must be in INSTALL_SURFACES" + ); +}); + +// --------------------------------------------------------------------------- +// T16: Hook script files exist in hooks/ +// --------------------------------------------------------------------------- +test('T16: Cursor hook script files exist in hooks/', () => { + const hooksDir = path.join(__dirname, '..', 'hooks'); + const sessionStart = path.join(hooksDir, GSD_CURSOR_SESSION_HOOK_SCRIPT); + const postTool = path.join(hooksDir, GSD_CURSOR_POST_TOOL_HOOK_SCRIPT); + assert.ok(fs.existsSync(sessionStart), `${GSD_CURSOR_SESSION_HOOK_SCRIPT} must exist in hooks/`); + assert.ok(fs.existsSync(postTool), `${GSD_CURSOR_POST_TOOL_HOOK_SCRIPT} must exist in hooks/`); +}); + +// --------------------------------------------------------------------------- +// T17: sessionStart script emits JSON with additional_context on stdin close +// --------------------------------------------------------------------------- +test('T17: gsd-cursor-session-start.js emits JSON with additional_context', (t, done) => { + const hooksDir = path.join(__dirname, '..', 'hooks'); + const scriptPath = path.join(hooksDir, GSD_CURSOR_SESSION_HOOK_SCRIPT); + const { execFile } = require('child_process'); + + const input = JSON.stringify({ session_id: 'test-123', composer_mode: 'agent' }); + const child = execFile(process.execPath, [scriptPath], { + timeout: 10000, + cwd: os.tmpdir(), // no .planning/ dir here — should get MSG_ABSENT + }, (err, stdout) => { + if (err && !stdout) { done(err); return; } + let parsed; + try { parsed = JSON.parse(stdout); } catch (e) { done(new Error(`stdout not valid JSON: ${stdout}`)); return; } + assert.ok('additional_context' in parsed, 'output must have additional_context field'); + assert.ok(typeof parsed.additional_context === 'string', 'additional_context must be a string'); + assert.ok(parsed.additional_context.length > 0, 'additional_context must not be empty'); + done(); + }); + child.stdin.write(input); + child.stdin.end(); +}); + +// --------------------------------------------------------------------------- +// T18: postToolUse script emits {} for non-write tools +// --------------------------------------------------------------------------- +test('T18: gsd-cursor-post-tool.js emits {} for non-write tool names', (t, done) => { + const hooksDir = path.join(__dirname, '..', 'hooks'); + const scriptPath = path.join(hooksDir, GSD_CURSOR_POST_TOOL_HOOK_SCRIPT); + const { execFile } = require('child_process'); + + const input = JSON.stringify({ + tool_name: 'Read', + tool_input: { path: '/some/file.js' }, + tool_output: 'contents', + duration: 42, + }); + + const child = execFile(process.execPath, [scriptPath], { + timeout: 10000, + cwd: os.tmpdir(), + }, (err, stdout) => { + if (err && !stdout) { done(err); return; } + let parsed; + try { parsed = JSON.parse(stdout); } catch (e) { done(new Error(`stdout not valid JSON: ${stdout}`)); return; } + // Non-write tool → empty response (no additional_context). + assert.ok(typeof parsed === 'object', 'output must be an object'); + // additional_context should be absent for non-write, non-planning tool. + assert.equal(parsed.additional_context, undefined, 'no additional_context for non-write tool'); + done(); + }); + child.stdin.write(input); + child.stdin.end(); +}); + +// --------------------------------------------------------------------------- +// T19: GSD_CURSOR_HOOK_MARKER is exported and is a non-empty string +// --------------------------------------------------------------------------- +test('T19: GSD_CURSOR_HOOK_MARKER is exported and is a non-empty string', () => { + assert.equal(typeof GSD_CURSOR_HOOK_MARKER, 'string'); + assert.ok(GSD_CURSOR_HOOK_MARKER.length > 0); +}); + +// --------------------------------------------------------------------------- +// T20: Script name constants are exported and correct +// --------------------------------------------------------------------------- +test('T20: GSD_CURSOR_SESSION_HOOK_SCRIPT and GSD_CURSOR_POST_TOOL_HOOK_SCRIPT are exported', () => { + assert.equal(GSD_CURSOR_SESSION_HOOK_SCRIPT, 'gsd-cursor-session-start.js'); + assert.equal(GSD_CURSOR_POST_TOOL_HOOK_SCRIPT, 'gsd-cursor-post-tool.js'); +}); diff --git a/tests/runtime-config-adapter-registry.test.cjs b/tests/runtime-config-adapter-registry.test.cjs index a288a6ccf..10ed2dc68 100644 --- a/tests/runtime-config-adapter-registry.test.cjs +++ b/tests/runtime-config-adapter-registry.test.cjs @@ -31,7 +31,7 @@ const EXPECTED_TABLE = [ { runtime: 'codex', installSurface: 'codex-toml', writesSharedSettings: false, finishPermissionWriter: null }, { runtime: 'copilot', installSurface: 'copilot-instructions', writesSharedSettings: false, finishPermissionWriter: null }, { runtime: 'cline', installSurface: 'cline-rules', writesSharedSettings: false, finishPermissionWriter: null }, - { runtime: 'cursor', installSurface: 'profile-marker-only', writesSharedSettings: false, finishPermissionWriter: null }, + { runtime: 'cursor', installSurface: 'cursor-hooks-json', writesSharedSettings: false, finishPermissionWriter: null }, { runtime: 'windsurf', installSurface: 'profile-marker-only', writesSharedSettings: false, finishPermissionWriter: null }, { runtime: 'trae', installSurface: 'profile-marker-only', writesSharedSettings: false, finishPermissionWriter: null }, ]; @@ -160,8 +160,8 @@ describe('installSurface correctness', () => { assert.strictEqual(resolveRuntimeConfigIntent('cline').installSurface, 'cline-rules'); }); - test('cursor -> "profile-marker-only"', () => { - assert.strictEqual(resolveRuntimeConfigIntent('cursor').installSurface, 'profile-marker-only'); + test('cursor -> "cursor-hooks-json"', () => { + assert.strictEqual(resolveRuntimeConfigIntent('cursor').installSurface, 'cursor-hooks-json'); }); test('windsurf -> "profile-marker-only"', () => { @@ -236,10 +236,11 @@ describe('INSTALL_SURFACES export', () => { 'codex-toml', 'copilot-instructions', 'cline-rules', + 'cursor-hooks-json', 'profile-marker-only', ]); - test('INSTALL_SURFACES contains exactly the 5 surface strings', () => { + test('INSTALL_SURFACES contains exactly the 6 surface strings', () => { assert.deepStrictEqual(new Set(INSTALL_SURFACES), EXPECTED_SURFACES); }); }); From 5b4880522ba2cc833f0269a18870045bd7f02b8e Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Sun, 7 Jun 2026 19:36:28 -0400 Subject: [PATCH 3/8] docs(#832): add how-to guide for minimal install / skill profiles (#835) Add docs/how-to/install-minimal-and-add-skills.md covering the --minimal / --core-only / --profile=core install, the core/standard/full profiles, and growing the surface live via /gsd:surface or on reinstall. Register it in the docs/README.md How-to guides index. Closes #832 Co-authored-by: Claude Opus 4.8 --- docs/README.md | 1 + docs/how-to/install-minimal-and-add-skills.md | 127 ++++++++++++++++++ 2 files changed, 128 insertions(+) create mode 100644 docs/how-to/install-minimal-and-add-skills.md diff --git a/docs/README.md b/docs/README.md index 79d835975..5d773af02 100644 --- a/docs/README.md +++ b/docs/README.md @@ -16,6 +16,7 @@ Language versions: [English](README.md) · [Português (pt-BR)](pt-BR/README.md) ## How-to guides - [Install on your runtime](how-to/install-on-your-runtime.md) — runtime-specific install steps for all 15 supported runtimes +- [Install a minimal GSD and add skills later](how-to/install-minimal-and-add-skills.md) — install only the core skills, then grow the surface with profiles and `/gsd:surface` - [Discuss a phase](how-to/discuss-a-phase.md) — capture implementation decisions before planning begins - [Plan a phase](how-to/plan-a-phase.md) — run research, decompose work, and verify plan quality - [Execute a phase](how-to/execute-a-phase.md) — run plans in parallel waves with fresh-context subagents diff --git a/docs/how-to/install-minimal-and-add-skills.md b/docs/how-to/install-minimal-and-add-skills.md new file mode 100644 index 000000000..e30537af7 --- /dev/null +++ b/docs/how-to/install-minimal-and-add-skills.md @@ -0,0 +1,127 @@ +# How to install a minimal GSD and add skills later + +Install GSD Core with a small skill footprint to keep cold-start context low, then grow the surface — live or on reinstall — only when you need more. Use this when context budget matters: large existing projects, constrained models, or runtimes where every description token counts. + +**What you need:** A supported runtime and the standard installer prerequisites (Node.js 18+ and npx). If you have not installed GSD at all yet, read [Install on your runtime](install-on-your-runtime.md) first — this guide covers the *profile* choice that layers on top of any runtime install. + +--- + +## Install the minimal profile + +To install only the core main-loop skills, add `--minimal` to the installer: + +```bash +npx @opengsd/gsd-core@latest --claude --global --minimal +``` + +`--minimal` has two aliases — use whichever reads best to you; they are identical: + +```bash +npx @opengsd/gsd-core@latest --claude --global --core-only +npx @opengsd/gsd-core@latest --claude --global --profile=core +``` + +A minimal install gives you the eight skills needed to run the core phase loop: + +- `new-project` +- `discuss-phase` +- `plan-phase` +- `execute-phase` +- `phase` +- `help` +- `update` +- `surface` + +No sub-agents are installed, and the skill-description tokens the model carries at cold start drop to roughly 130, against roughly 1,200 for a full install. The chosen profile is recorded in the `.gsd-profile` marker in your runtime config directory and is reapplied automatically every time you run `/gsd-update`, so you stay minimal across upgrades until you decide otherwise. + +> Do not combine `--minimal` with `--profile=` — the installer treats that as a conflict and exits. + +--- + +## Choose a profile + +If `core` is too small, pick a wider profile instead. Pass it with `--profile=`: + +| Profile | What you get | Approx. description tokens | +|---------|--------------|--------------------------| +| `core` | The eight core-loop skills above. No agents. | ~130 desc tokens | +| `standard` | Everything in `core` plus common management skills — `review`, `config`, `progress`, `resume-work`, `pause-work`, `workspace` — and the sub-agents those skills need. | ~700 desc tokens | +| `full` | Every skill and every sub-agent. This is the default when you pass no profile flag. | ~1,200 desc tokens | + +```bash +# Standard: the core loop plus everyday management commands +npx @opengsd/gsd-core@latest --claude --global --profile=standard +``` + +If you want a named profile plus one extra cluster, compose them with a comma. The installer writes the union of both: + +```bash +# Core loop plus the audit/review skills, nothing else +npx @opengsd/gsd-core@latest --claude --global --profile=core,audit +``` + +--- + +## See what is installed and what is available + +From inside your runtime, list the current surface, the disabled clusters, and the token cost of each: + +```bash +/gsd:surface list +``` + +The skills are grouped into clusters you can toggle as a unit: + +`core_loop`, `audit_review`, `milestone`, `research_ideate`, `workspace_state`, `docs`, `ui`, `ai_eval`, `ns_meta`, `utility` + +--- + +## Add skills later without reinstalling + +If you installed minimal and now need more, you do not have to re-run the installer. `/gsd:surface` changes the live surface and persists the change in a separate `.gsd-surface.json` file, leaving your install-time profile marker untouched. + +To switch to a wider profile in place: + +```bash +/gsd:surface profile standard +``` + +To turn on just one cluster while keeping your base profile: + +```bash +/gsd:surface enable audit_review +``` + +To turn a cluster back off, or to discard all your live changes and return to the profile you installed: + +```bash +/gsd:surface disable utility +/gsd:surface reset +``` + +Surface changes take effect in your next session — restart the runtime to pick them up. + +--- + +## Add skills by reinstalling + +`/gsd:surface` is the right tool for occasional, reversible adjustments. If you have decided you want the wider surface permanently, change the install-time profile instead so every future `/gsd-update` keeps it: + +```bash +# Re-run the installer without --minimal to record `full` as your profile +npx @opengsd/gsd-core@latest --claude --global + +# ...or pin a specific profile +npx @opengsd/gsd-core@latest --claude --global --profile=standard +``` + +Running `/gsd-update` re-reads the `.gsd-profile` marker and reinstalls at that profile, so a one-off reinstall at a new profile is all you need — subsequent updates follow it automatically. + +--- + +## Related + +- [Install on your runtime](install-on-your-runtime.md) +- [Update GSD](update-gsd.md) +- [Configuration](../CONFIGURATION.md) +- [Docs index](../README.md) From 40d48c0508dc3c2ec6d886ea6c7145309fd48bb3 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Sun, 7 Jun 2026 20:22:07 -0400 Subject: [PATCH 4/8] feat(#815): add /gsd-update --next to install the @next RC channel (#839) Adds an opt-in --next (alias --rc) flag to /gsd-update targeting the @next RC dist-tag (ADR #660), with a {latest,next} allowlist enforced at three layers, channel-aware version check + banner, and byte-for-byte unchanged default @latest behavior. Closes #815 --- .changeset/update-next-rc-channel.md | 5 ++ commands/gsd/update.md | 5 +- docs/COMMANDS.md | 2 + docs/FEATURES.md | 1 + docs/USER-GUIDE.md | 12 ++++ docs/how-to/update-gsd.md | 20 ++++++ gsd-core/bin/check-latest-version.cjs | 62 +++++++++++++++++-- gsd-core/workflows/help/modes/full.md | 3 +- gsd-core/workflows/update.md | 37 +++++++++-- tests/bug-2992-check-latest-version.test.cjs | 59 ++++++++++++++++++ ...3130-update-npx-robust-invocation.test.cjs | 19 +++--- tests/issue-815-update-next-channel.test.cjs | 44 +++++++++++++ 12 files changed, 249 insertions(+), 20 deletions(-) create mode 100644 .changeset/update-next-rc-channel.md create mode 100644 tests/issue-815-update-next-channel.test.cjs diff --git a/.changeset/update-next-rc-channel.md b/.changeset/update-next-rc-channel.md new file mode 100644 index 000000000..bfcbea65d --- /dev/null +++ b/.changeset/update-next-rc-channel.md @@ -0,0 +1,5 @@ +--- +type: Added +pr: 839 +--- +Added `/gsd-update --next` (alias `--rc`) to install or refresh from the `@next` RC dist-tag (ADR #660). A new `parse_update_channel` workflow step resolves the channel from `$ARGUMENTS`; the version check and all three npx install invocations thread `$TAG` instead of hardcoding `@latest`. When `--next` is used the version-comparison output gains a `Channel: next (RC)` banner so the user knows they are leaving the stable line; omitting the flag keeps `@latest` behavior byte-for-byte unchanged. `check-latest-version.cjs` gains `ALLOWED_TAGS`, `buildViewArgs`, and `resolveTag` exports, with an allowlist guard (enforced at both the CLI and function boundary) that rejects any dist-tag other than `latest`/`next`. (#815) diff --git a/commands/gsd/update.md b/commands/gsd/update.md index dacd1b0d3..516709a16 100644 --- a/commands/gsd/update.md +++ b/commands/gsd/update.md @@ -1,7 +1,7 @@ --- name: gsd:update description: Update GSD to latest version with changelog display -argument-hint: "[--sync | --reapply]" +argument-hint: "[--sync | --reapply | --next | --rc]" allowed-tools: - Read - Write @@ -31,6 +31,7 @@ Routes to the update workflow which handles: - **--sync**: Sync managed GSD skills across runtime roots so multi-runtime users stay aligned after an update. Runs the sync-skills workflow (--from, --to, --dry-run, --apply flags supported). - **--reapply**: Reapply local modifications after a GSD update. Uses three-way comparison (pristine baseline, user-modified backup, newly installed version) to merge user customizations back. Runs the reapply-patches workflow. +- **--next** (alias **--rc**): Target the `@next` RC dist-tag instead of `@latest` so you can install or refresh a release candidate (e.g. `1.4.0-rc.1`) through the normal update flow — scope/runtime detection, changelog preview, custom-file backup, and cache clearing all still apply. Omitting it keeps targeting `@latest` (no change). See ADR #660 for the RC channel. - **(no flag)**: Standard update — check for new version, show changelog, install. @@ -38,7 +39,7 @@ Routes to the update workflow which handles: Parse the first token of $ARGUMENTS: - If it is `--sync`: strip the flag, execute the sync-skills workflow (passing remaining args for --from/--to/--dry-run/--apply). - If it is `--reapply`: strip the flag, execute the reapply-patches workflow. -- Otherwise: execute the update workflow end-to-end. +- Otherwise (including `--next` / `--rc`): execute the update workflow end-to-end, passing `$ARGUMENTS` through so the workflow's parse_update_channel step can select the release channel. diff --git a/docs/COMMANDS.md b/docs/COMMANDS.md index ffc8bf6d5..50c636ca8 100644 --- a/docs/COMMANDS.md +++ b/docs/COMMANDS.md @@ -1148,11 +1148,13 @@ Update GSD with changelog preview, and optionally sync skills or reapply local p |------|-------------| | `--sync` | Sync skills from the GSD registry after updating | | `--reapply` | Restore local modifications (patches) after updating | +| `--next` / `--rc` | Target the `@next` RC dist-tag instead of `@latest` (installs or refreshes a release candidate, e.g. `1.4.0-rc.1`; see ADR #660) | ```bash /gsd-update # Check for updates and install /gsd-update --sync # Update and sync skills /gsd-update --reapply # Update and reapply local patches +/gsd-update --next # Install from the @next RC dist-tag ``` --- diff --git a/docs/FEATURES.md b/docs/FEATURES.md index 2ab41eb79..7c96d5895 100644 --- a/docs/FEATURES.md +++ b/docs/FEATURES.md @@ -904,6 +904,7 @@ continues. Drift detection cannot fail verification. - REQ-UPDATE-03: System MUST be runtime-aware and target the correct directory - REQ-UPDATE-04: System MUST back up locally modified files to `gsd-local-patches/` - REQ-UPDATE-05: `/gsd-update --reapply` MUST restore local modifications after update +- REQ-UPDATE-06: `/gsd-update --next` (alias `--rc`) MUST target the `@next` RC dist-tag for version check and install; omitting the flag MUST keep `@latest` behavior unchanged (ADR #660) --- diff --git a/docs/USER-GUIDE.md b/docs/USER-GUIDE.md index c4f114a76..7f5d9d373 100644 --- a/docs/USER-GUIDE.md +++ b/docs/USER-GUIDE.md @@ -830,6 +830,18 @@ Set `commit_docs: false` during `/gsd-new-project` or via `/gsd-settings`. Add ` Since v1.17, the installer backs up locally modified files to `gsd-local-patches/`. Run `/gsd-update --reapply` to merge your changes back. +### Install or Refresh a Release Candidate + +To install or refresh GSD from the `@next` RC dist-tag (the pre-release channel established by ADR #660), run: + +```bash +/gsd-update --next +# or equivalently: +/gsd-update --rc +``` + +The same scope/runtime detection, changelog preview, custom-file backup, and cache clearing apply. Omitting `--next`/`--rc` keeps targeting `@latest` (stable channel, no change). Only the `@latest` and `@next` channels are supported — no arbitrary dist-tag can be passed. + ### Cannot Update via npm See [docs/manual-update.md](manual-update.md) for a step-by-step manual update procedure. diff --git a/docs/how-to/update-gsd.md b/docs/how-to/update-gsd.md index 01887fc1c..aeaf5884c 100644 --- a/docs/how-to/update-gsd.md +++ b/docs/how-to/update-gsd.md @@ -35,14 +35,34 @@ Restart your runtime after the update to pick up new commands and agents. |------|--------------| | `--sync` | After updating, sync skills from the GSD registry | | `--reapply` | After updating, merge locally modified GSD files back in from `gsd-local-patches/` | +| `--next` / `--rc` | Target the `@next` RC dist-tag instead of `@latest` (installs or refreshes a release candidate; see ADR #660) | ```bash /gsd-update --sync # Update and sync skills /gsd-update --reapply # Update and reapply local patches +/gsd-update --next # Install from the @next RC dist-tag ``` --- +## Install or refresh a release candidate + +GSD publishes release candidates on the `@next` npm dist-tag (established by ADR #660). To install or refresh from that channel: + +```bash +/gsd-update --next +# or equivalently: +/gsd-update --rc +``` + +The full update flow applies — scope/runtime detection, changelog preview, custom-file backup, and cache clearing all run normally. The only difference is that `check-latest-version.cjs` resolves the `@next` tag and npx installs from `@opengsd/gsd-core@next`. + +Only `latest` and `next` are supported channels; no arbitrary dist-tag can be passed (the script enforces an allowlist and exits with code 2 on an invalid tag). + +Omitting `--next`/`--rc` keeps targeting `@latest` (stable channel, no change in behavior). + +--- + ## Reviewing the changelog before updating `/gsd-update` always shows the changelog diff between your installed version and the latest *before* it asks for confirmation. You do not need to visit GitHub separately. The output looks like: diff --git a/gsd-core/bin/check-latest-version.cjs b/gsd-core/bin/check-latest-version.cjs index d5f889e53..e00ff58c0 100755 --- a/gsd-core/bin/check-latest-version.cjs +++ b/gsd-core/bin/check-latest-version.cjs @@ -37,18 +37,64 @@ const CHECK_REASON = Object.freeze({ const SEMVER_RE = /^\d+\.\d+\.\d+(?:[-+][0-9A-Za-z.-]+)?$/; +// #815: the one RC channel ADR #660 sanctions, plus the stable default. +// An allowlist (not a free string) keeps a typo from silently resolving +// `npm view` to an empty or foreign dist-tag. +const ALLOWED_TAGS = Object.freeze(['latest', 'next']); + +/** + * Build the `npm view` args for a dist-tag. `latest` keeps the bare package + * spec so the default invocation is byte-for-byte identical to before tag + * support existed (#815); any other allowlisted tag appends `@` so + * `npm view @opengsd/gsd-core@next version` resolves the RC channel (#660). + */ +function buildViewArgs(tag = 'latest') { + if (!ALLOWED_TAGS.includes(tag)) { + throw new RangeError(`invalid dist-tag '${tag}'; allowed: ${ALLOWED_TAGS.join(', ')}`); + } + const spec = tag === 'latest' ? PACKAGE_NAME : `${PACKAGE_NAME}@${tag}`; + return ['view', spec, 'version']; +} + +/** + * Resolve the requested dist-tag from argv. Defaults to `latest` (no flag => + * no behavior change). Restricted to ALLOWED_TAGS so a typo can't silently + * resolve to an empty/foreign tag (#815 alternative 1). + */ +function resolveTag(argv) { + let val; + const eq = argv.find((a) => typeof a === 'string' && a.startsWith('--tag=')); + if (eq !== undefined) { + val = eq.slice('--tag='.length); + } else { + const i = argv.indexOf('--tag'); + if (i === -1) return 'latest'; + val = argv[i + 1]; + } + if (!val || !ALLOWED_TAGS.includes(val)) { + throw new RangeError( + `invalid --tag '${val || ''}'; allowed: ${ALLOWED_TAGS.join(', ')}`, + ); + } + return val; +} + /** * Pure-ish: takes an injected spawn function so tests don't actually run npm. * In production, defaults to execNpm() from the shell-projection seam. */ function checkLatestVersion(opts = {}) { + const tag = opts.tag || 'latest'; + if (!ALLOWED_TAGS.includes(tag)) { + throw new RangeError(`invalid dist-tag '${tag}'; allowed: ${ALLOWED_TAGS.join(', ')}`); + } // Default path routes through the shell-projection seam (execNpm owns the // Windows shell-flag policy and timeout default). The injection point // remains spawnSync-shaped for test compatibility — the adapter below // translates { exitCode } → { status } so the consumer logic is unchanged. // Bounded at 15s so a hung registry doesn't block /gsd-update (#2993 CR). const defaultSpawn = () => { - const r = execNpm(['view', PACKAGE_NAME, 'version'], { timeout: 15_000 }); + const r = execNpm(buildViewArgs(tag), { timeout: 15_000 }); return { status: r.exitCode, stdout: r.stdout, @@ -90,8 +136,16 @@ function checkLatestVersion(opts = {}) { } function main() { - const json = process.argv.includes('--json'); - const r = checkLatestVersion(); + const argv = process.argv.slice(2); + const json = argv.includes('--json'); + let tag; + try { + tag = resolveTag(argv); + } catch (e) { + process.stderr.write(`check-latest-version: ${e.message}\n`); + return 2; + } + const r = checkLatestVersion({ tag }); if (json) { process.stdout.write(JSON.stringify(r) + '\n'); } else if (r.ok) { @@ -104,4 +158,4 @@ function main() { if (require.main === module) runMain(main); -module.exports = { checkLatestVersion, CHECK_REASON, PACKAGE_NAME }; +module.exports = { checkLatestVersion, CHECK_REASON, PACKAGE_NAME, ALLOWED_TAGS, buildViewArgs, resolveTag }; diff --git a/gsd-core/workflows/help/modes/full.md b/gsd-core/workflows/help/modes/full.md index 1efe4f401..07bb71757 100644 --- a/gsd-core/workflows/help/modes/full.md +++ b/gsd-core/workflows/help/modes/full.md @@ -550,11 +550,12 @@ Usage: `/gsd:help --full` Usage: `/gsd:help debug` Usage: `/gsd:help --brief debug` -**`/gsd:update [--sync] [--reapply]`** +**`/gsd:update [--sync] [--reapply] [--next | --rc]`** Update GSD to latest version with changelog preview. - `--sync` — sync managed GSD skills across runtime roots (replaces the former `gsd-sync-skills`) - `--reapply` — reapply local modifications after an update (replaces the former `gsd-reapply-patches`) +- `--next` (alias `--rc`) — install/refresh from the `@next` RC dist-tag instead of `@latest` (ADR #660); omit for the stable channel - Shows installed vs latest version comparison - Displays changelog entries for versions you've missed diff --git a/gsd-core/workflows/update.md b/gsd-core/workflows/update.md index 77692917e..4ff98df1c 100644 --- a/gsd-core/workflows/update.md +++ b/gsd-core/workflows/update.md @@ -75,6 +75,25 @@ If multiple runtime installs are detected and the invoking runtime cannot be det **If VERSION file missing (version resolves to `0.0.0`):** report the installed version as Unknown and proceed to install (treated as `0.0.0` for comparison). + +Determine the release channel from `$ARGUMENTS`. This selects which npm dist-tag the entire update flow targets — `latest` (stable) by default, or `next` (the RC channel established by ADR #660) when the user opts in with `--next`/`--rc`: + +```bash +case " $ARGUMENTS " in + *" --next "*|*" --rc "*) + TAG="next" + CHANNEL_LABEL="next (RC)" + ;; + *) + TAG="latest" + CHANNEL_LABEL="latest (stable)" + ;; +esac +``` + +`TAG` is restricted to `latest`/`next` by `check-latest-version.cjs` (it rejects any other value with exit 2), so no arbitrary dist-tag can leak through. Omitting `--next`/`--rc` reproduces the prior behavior exactly: `TAG=latest`. + + Check npm for latest version via the deterministic script. **Do NOT run `npm view` or `npm search` directly** — the package name must come from the script, not from a free choice at execution time. (#2992: LLM-driven prescriptions of npm package names produced wrong-package queries; moving the package name into a script constant closes that gap.) @@ -91,7 +110,7 @@ if [ -z "$GSD_DIR" ]; then LATEST_VERSION="" LATEST_REASON="no_install_detected" else - LATEST_RESULT="$(node "$GSD_DIR/gsd-core/bin/check-latest-version.cjs" --json 2>/dev/null)" + LATEST_RESULT="$(node "$GSD_DIR/gsd-core/bin/check-latest-version.cjs" --json --tag "$TAG" 2>/dev/null)" LATEST_STATUS=$? # #2993 CR: when node is missing or the script doesn't exist, LATEST_RESULT # is empty and piping it to `jq` produces a parse error on stderr while @@ -114,7 +133,7 @@ fi ```text Couldn't check for updates (reason: {LATEST_REASON}, exit: {LATEST_STATUS}). -To update manually: `npx -y --package=@opengsd/gsd-core@latest -- gsd-core --global` +To update manually: `npx -y --package=@opengsd/gsd-core@{TAG} -- gsd-core --global` ``` Exit. @@ -123,6 +142,14 @@ Exit. Compare installed vs latest: +**Only when `TAG=next`** (the user passed `--next`/`--rc`), prepend a channel banner so they know they are leaving the stable line — add this line immediately after the `**Latest:**` line in whichever output block renders: + +**Channel:** {CHANNEL_LABEL} + +On the default stable channel (`TAG=latest`), do NOT add a channel line — the output must match the prior stable behavior exactly. + +When `TAG=next`, the "latest" value is the release candidate published under `@next` (e.g. `1.4.0-rc.1`). Apply standard semver precedence for prereleases (`1.4.0-rc.1` is newer than `1.3.1` but older than the final `1.4.0`). Do NOT treat an `-rc.N` suffix as a dev install or as "behind" — offer it as an available update. + **If installed == latest:** ``` ## GSD Update @@ -327,17 +354,17 @@ RUNTIME_FLAG="--$TARGET_RUNTIME" **If LOCAL install:** ```bash -npx -y --package=@opengsd/gsd-core@latest -- gsd-core "$RUNTIME_FLAG" --local +npx -y --package=@opengsd/gsd-core@"$TAG" -- gsd-core "$RUNTIME_FLAG" --local ``` **If GLOBAL install:** ```bash -npx -y --package=@opengsd/gsd-core@latest -- gsd-core "$RUNTIME_FLAG" --global +npx -y --package=@opengsd/gsd-core@"$TAG" -- gsd-core "$RUNTIME_FLAG" --global ``` **If UNKNOWN install:** ```bash -npx -y --package=@opengsd/gsd-core@latest -- gsd-core --claude --global +npx -y --package=@opengsd/gsd-core@"$TAG" -- gsd-core --claude --global ``` Capture output. If install fails, show error and exit. diff --git a/tests/bug-2992-check-latest-version.test.cjs b/tests/bug-2992-check-latest-version.test.cjs index adb41e724..51a6dc303 100644 --- a/tests/bug-2992-check-latest-version.test.cjs +++ b/tests/bug-2992-check-latest-version.test.cjs @@ -93,3 +93,62 @@ describe('Bug #2992: error paths', () => { assert.deepEqual(r, { ok: true, version: '1.40.0-rc.1', reason: CHECK_REASON.OK }); }); }); + +describe('Issue #815: --next dist-tag support', () => { + const { buildViewArgs, resolveTag, ALLOWED_TAGS } = require( + path.join(ROOT, 'gsd-core', 'bin', 'check-latest-version.cjs'), + ); + + test('ALLOWED_TAGS is the sanctioned channel allowlist (latest, next)', () => { + assert.deepEqual([...ALLOWED_TAGS].sort(), ['latest', 'next']); + }); + + test('buildViewArgs() defaults to the bare latest spec (byte-for-byte unchanged)', () => { + assert.deepEqual(buildViewArgs(), ['view', '@opengsd/gsd-core', 'version']); + assert.deepEqual(buildViewArgs('latest'), ['view', '@opengsd/gsd-core', 'version']); + }); + + test('buildViewArgs("next") targets the @next dist-tag', () => { + assert.deepEqual(buildViewArgs('next'), ['view', '@opengsd/gsd-core@next', 'version']); + }); + + test('resolveTag defaults to latest when no --tag flag', () => { + assert.equal(resolveTag(['--json']), 'latest'); + }); + + test('resolveTag reads --tag next', () => { + assert.equal(resolveTag(['--json', '--tag', 'next']), 'next'); + }); + + test('resolveTag rejects an unknown tag (typo guard)', () => { + assert.throws(() => resolveTag(['--tag', 'nightly']), /invalid --tag 'nightly'/); + }); + + test('resolveTag rejects --tag with no value', () => { + assert.throws(() => resolveTag(['--tag']), /invalid --tag ''/); + }); + + test('checkLatestVersion accepts an RC under the next tag', () => { + const r = checkLatestVersion({ tag: 'next', spawn: () => ({ status: 0, stdout: '1.4.0-rc.1\n', stderr: '' }) }); + assert.deepEqual(r, { ok: true, version: '1.4.0-rc.1', reason: CHECK_REASON.OK }); + }); + + test('buildViewArgs rejects a tag outside the allowlist (exported-API guard)', () => { + assert.throws(() => buildViewArgs('nightly'), /invalid dist-tag 'nightly'/); + }); + + test('checkLatestVersion rejects an out-of-allowlist tag even with an injected spawn', () => { + assert.throws( + () => checkLatestVersion({ tag: 'nightly', spawn: () => ({ status: 0, stdout: '9.9.9\n', stderr: '' }) }), + /invalid dist-tag 'nightly'/, + ); + }); + + test('resolveTag handles the --tag=next equals form', () => { + assert.equal(resolveTag(['--json', '--tag=next']), 'next'); + }); + + test('resolveTag rejects an unknown --tag=value equals form (no silent fallback)', () => { + assert.throws(() => resolveTag(['--tag=nightly']), /invalid --tag 'nightly'/); + }); +}); diff --git a/tests/bug-3130-update-npx-robust-invocation.test.cjs b/tests/bug-3130-update-npx-robust-invocation.test.cjs index ba0ea78d9..3ae34d453 100644 --- a/tests/bug-3130-update-npx-robust-invocation.test.cjs +++ b/tests/bug-3130-update-npx-robust-invocation.test.cjs @@ -4,18 +4,20 @@ // Regression guard for bug #3130. // // Two failure modes were observed with the pre-fix npx invocation form: -// 1. Cache-stale: bare `npx -y @opengsd/gsd-core@latest` hits npx's local -// cache and may pull an older version instead of @latest. +// 1. Cache-stale: bare `npx -y @opengsd/gsd-core@` hits npx's local +// cache and may pull an older version instead of the target tag. // 2. Token-routing: Bash-tool wrappers misroute the `@` token in -// `@opengsd/gsd-core@latest`, causing npm to error with -// "Unknown command: @opengsd/gsd-core@latest". +// `@opengsd/gsd-core@`, causing npm to error with +// "Unknown command: @opengsd/gsd-core@". // // The robust form is: -// npx -y --package=@opengsd/gsd-core@latest -- gsd-core $ARGS +// npx -y --package=@opengsd/gsd-core@"$TAG" -- gsd-core $ARGS // // `--package=` forces a fresh registry fetch, bypassing the npx cache. // `--` clearly delineates npx flags from the run-command, preventing // Bash-tool @-token misrouting. +// `$TAG` is a shell variable (latest by default, next under --next/--rc), +// set by the parse_update_channel step (#815). const { test } = require('node:test'); const assert = require('node:assert/strict'); @@ -28,9 +30,9 @@ const UPDATE_WF = path.join(ROOT, 'gsd-core', 'workflows', 'update.md'); const src = fs.readFileSync(UPDATE_WF, 'utf8'); test('bug #3130: update.md contains no bare npx invocations (cache-stale form)', () => { - // Any occurrence of `npx -y @opengsd/gsd-core@latest` without `--package=` + // Any occurrence of `npx -y @opengsd/gsd-core@` without `--package=` // is the stale form that triggers the two failure modes. - const stale = (src.match(/npx -y @opengsd\/gsd-core@latest[^\n]*/g) || []); + const stale = (src.match(/npx -y @opengsd\/gsd-core@\S+[^\n]*/g) || []); assert.deepEqual( stale, [], @@ -40,7 +42,8 @@ test('bug #3130: update.md contains no bare npx invocations (cache-stale form)', test('bug #3130: update.md has >=3 robust npx invocations (--package= + -- separator)', () => { // Three sibling invocations: local, global, and unknown/fallback. - const robust = (src.match(/npx -y --package=@opengsd\/gsd-core@latest -- gsd-core/g) || []); + // The tag is now a $TAG variable (latest by default, next under --next/--rc). + const robust = (src.match(/npx -y --package=@opengsd\/gsd-core@\S+ -- gsd-core/g) || []); assert.ok( robust.length >= 3, `Expected >=3 robust npx invocations in update.md, found ${robust.length}`, diff --git a/tests/issue-815-update-next-channel.test.cjs b/tests/issue-815-update-next-channel.test.cjs new file mode 100644 index 000000000..5ad08ec4f --- /dev/null +++ b/tests/issue-815-update-next-channel.test.cjs @@ -0,0 +1,44 @@ +'use strict'; +// allow-test-rule: reads product workflow/command markdown to verify the --next RC channel contract — not a source-grep test + +// Issue #815: `/gsd-update --next` (alias `--rc`) must thread the @next dist-tag +// through the whole update flow (version check + install) while leaving the +// default @latest path unchanged. + +const { test } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); + +const ROOT = path.join(__dirname, '..'); +const WF = fs.readFileSync(path.join(ROOT, 'gsd-core', 'workflows', 'update.md'), 'utf8'); +const CMD = fs.readFileSync(path.join(ROOT, 'commands', 'gsd', 'update.md'), 'utf8'); + +test('issue #815: workflow parses --next/--rc into a TAG channel', () => { + assert.match(WF, /--next/); + assert.match(WF, /--rc/); + assert.match(WF, /TAG="next"/); + assert.match(WF, /TAG="latest"/); +}); + +test('issue #815: version check threads the tag through check-latest-version.cjs', () => { + // The script path is double-quoted in the shell command, so the line is: + // node "$GSD_DIR/gsd-core/bin/check-latest-version.cjs" --json --tag "$TAG" + // The closing " on the script path sits between .cjs and --json. + assert.match(WF, /check-latest-version\.cjs"? --json --tag "\$TAG"/); +}); + +test('issue #815: install uses the selected tag, not a hardcoded @latest', () => { + const robust = WF.match(/npx -y --package=@opengsd\/gsd-core@"\$TAG" -- gsd-core/g) || []; + assert.ok(robust.length >= 3, `expected >=3 tag-parameterized npx invocations, found ${robust.length}`); + assert.doesNotMatch(WF, /--package=@opengsd\/gsd-core@latest -- gsd-core/, + 'install lines must not hardcode @latest once --next exists'); + assert.doesNotMatch(WF, /--package=@opengsd\/gsd-core@(?:latest|next|beta|canary|rc) -- gsd-core/, + 'install lines must use the $TAG variable, never a hardcoded dist-tag literal'); +}); + +test('issue #815: command documents --next/--rc and routes it to the update workflow', () => { + assert.match(CMD, /--next/); + assert.match(CMD, /--rc/); + assert.match(CMD, /argument-hint:.*--next/); +}); From 988024c1a36fcdd41093e0402742b2081ede37c4 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Sun, 7 Jun 2026 20:28:45 -0400 Subject: [PATCH 5/8] fix(#837): three-dot diff in ci-test-scope so docs-only PRs skip the heavy matrix (#841) CI test-scope detection diffed changed files with a two-dot `git diff --name-only base head`, where base is the moving tip of `next`. A PR branch cut from a slightly older `next` surfaced every product file `next` had gained since the merge-base, flipping product_changed/full_matrix and running the full Windows/macOS matrix + coverage on docs-only PRs. Switch to a three-dot `git diff --name-only base...head` (vs the merge-base), matching GitHub's PR "Files changed" semantics. Add a regression test that builds a stale-base topology, plus a guard test pinning `fetch-depth: 0` on the `changes` job (required for the merge-base to be locally available). Closes #837 Co-authored-by: Claude Opus 4.8 --- scripts/ci-test-scope.cjs | 7 ++- tests/ci-test-scope.test.cjs | 104 +++++++++++++++++++++++++++++++++++ 2 files changed, 110 insertions(+), 1 deletion(-) diff --git a/scripts/ci-test-scope.cjs b/scripts/ci-test-scope.cjs index 1028377a4..3e9435e6b 100644 --- a/scripts/ci-test-scope.cjs +++ b/scripts/ci-test-scope.cjs @@ -289,7 +289,12 @@ function changedFiles(args) { if (!args.base || !args.head) { throw new Error('--base/--head or --files is required'); } - const stdout = execFileSync('git', ['diff', '--name-only', args.base, args.head], { + // Three-dot diff (merge-base...head) matches GitHub's PR "Files changed" semantics. + // A two-dot `git diff base head` would surface every file `next` gained after this + // branch's merge-base, mis-flagging product_changed/full_matrix on docs-only PRs cut + // from a slightly stale base (#837). The `changes` job checks out with fetch-depth: 0, + // so the merge-base is always available. + const stdout = execFileSync('git', ['diff', '--name-only', `${args.base}...${args.head}`], { encoding: 'utf8', }); return splitFiles(stdout); diff --git a/tests/ci-test-scope.test.cjs b/tests/ci-test-scope.test.cjs index a242cee4e..d6d456028 100644 --- a/tests/ci-test-scope.test.cjs +++ b/tests/ci-test-scope.test.cjs @@ -5,6 +5,8 @@ const assert = require('node:assert/strict'); const { spawnSync } = require('child_process'); const path = require('path'); const fs = require('fs'); +const os = require('node:os'); +const { cleanup } = require('./helpers.cjs'); const ROOT = path.join(__dirname, '..'); const SCRIPT = path.join(ROOT, 'scripts', 'ci-test-scope.cjs'); @@ -207,6 +209,74 @@ describe('ci-test-scope.cjs', () => { assert.deepStrictEqual(result.targeted_tests, ['unit'], 'targeted_tests must be [\'unit\'] when code changed but no rule matched'); }); + + test('three-dot diff: docs-only PR on a stale base ignores product commits next gained after the merge-base', () => { + // Reproduces #837: a docs-only PR branched from a slightly older `next`. + // After the branch point, `next` advances with a PRODUCT commit. A two-dot + // `git diff base head` would surface that product file (flipping product_changed/ + // full_matrix true); a three-dot `git diff base...head` (vs the merge-base, which is + // GitHub's PR semantics) must see ONLY the docs change. + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'ci-scope-837-')); + try { + const git = (...a) => { + const r = spawnSync('git', a, { cwd: tmp, encoding: 'utf8' }); + assert.strictEqual(r.status, 0, `git ${a.join(' ')} failed: ${r.stderr}`); + return r.stdout.trim(); + }; + git('init', '-q'); + git('config', 'user.email', 'test@example.com'); + git('config', 'user.name', 'Test'); + git('config', 'commit.gpgsign', 'false'); + + // merge-base: a docs file + a product file (package.json) + fs.mkdirSync(path.join(tmp, 'tests'), { recursive: true }); // existingTests() reads tests/ + fs.mkdirSync(path.join(tmp, 'docs'), { recursive: true }); + fs.writeFileSync(path.join(tmp, 'docs', 'a.md'), 'base\n'); + fs.writeFileSync(path.join(tmp, 'package.json'), '{"name":"x","version":"1.0.0"}\n'); + git('add', '-A'); + git('commit', '-qm', 'merge-base'); + const baseBranch = git('rev-parse', '--abbrev-ref', 'HEAD'); + + // PR branch (head): docs-only change + git('checkout', '-q', '-b', 'feature'); + fs.writeFileSync(path.join(tmp, 'docs', 'a.md'), 'base\nnew docs line\n'); + git('add', '-A'); + git('commit', '-qm', 'docs: add line'); + const head = git('rev-parse', 'HEAD'); + + // base advances (next gains a PRODUCT commit after the merge-base) + git('checkout', '-q', baseBranch); + fs.writeFileSync(path.join(tmp, 'package.json'), '{"name":"x","version":"2.0.0"}\n'); + git('add', '-A'); + git('commit', '-qm', 'chore: bump version on next'); + const base = git('rev-parse', 'HEAD'); + + const r = spawnSync(process.execPath, [SCRIPT, '--base', base, '--head', head], { + cwd: tmp, + encoding: 'utf8', + }); + assert.strictEqual(r.status, 0, `script failed: stderr=${r.stderr}\nstdout=${r.stdout}`); + const result = JSON.parse(r.stdout); + + assert.deepStrictEqual( + result.changed_files, + ['docs/a.md'], + `expected three-dot diff to see only the docs file, got: ${JSON.stringify(result.changed_files)}`, + ); + assert.strictEqual( + result.product_changed, + false, + `docs-only PR must not set product_changed even on a stale base, got: ${JSON.stringify(result)}`, + ); + assert.strictEqual( + result.full_matrix, + false, + `docs-only PR must not set full_matrix even on a stale base, got: ${JSON.stringify(result)}`, + ); + } finally { + cleanup(tmp); + } + }); }); describe('ci-test-scope superset invariant (#494)', () => { @@ -335,6 +405,40 @@ describe('INERT_WORKFLOWS allowlist integrity guard', () => { }); }); +describe('test.yml changes job contract (#837)', () => { + // ci-test-scope.cjs uses a three-dot `git diff base...head`, which requires the + // merge-base commit to be locally present. The `changes` job in test.yml guarantees + // this via `fetch-depth: 0` on its checkout step. This test pins that contract so + // any future reduction of fetch-depth fails CI loudly (#837). + test('changes job checkout step sets fetch-depth: 0 (required for three-dot diff merge-base)', () => { + const workflowPath = path.join(WORKFLOWS_DIR, 'test.yml'); + const text = fs.readFileSync(workflowPath, 'utf8'); + const lines = text.split('\n'); + + // Locate the `changes:` job (two-space-indented top-level job key). + const jobStart = lines.findIndex(l => /^ {2}changes:\s*$/.test(l)); + assert.ok(jobStart !== -1, 'Could not find ` changes:` job in test.yml'); + + // Find the next top-level job key at the same two-space indentation to bound the region. + let jobEnd = lines.length; + for (let i = jobStart + 1; i < lines.length; i++) { + if (/^ {2}[A-Za-z0-9_-]+:\s*$/.test(lines[i])) { + jobEnd = i; + break; + } + } + + const changesJobText = lines.slice(jobStart, jobEnd).join('\n'); + + assert.ok( + /fetch-depth:\s*0/.test(changesJobText), + 'changes job checkout must set `fetch-depth: 0` so the three-dot `git diff base...head` ' + + 'in ci-test-scope.cjs can resolve the merge-base locally (#837). ' + + 'Reducing fetch-depth breaks the three-dot diff and causes incorrect scope detection.', + ); + }); +}); + describe('code_changed=false implies clean output invariant', () => { // Fix 1: when code_changed is false, full_matrix, targeted_tests, windows_tests // must ALL be empty/false — even if a docs path coincidentally From d32b8db635ea53ad269b3d736dda13b50cc8d276 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Sun, 7 Jun 2026 20:35:43 -0400 Subject: [PATCH 6/8] feat(#836): no-LLM duplicate-issue detection + challenge + 1-day auto-close (#843) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(#836): no-LLM duplicate-issue detection + challenge + 1-day auto-close Adds a deterministic (no-LLM) duplicate-issue governance lifecycle: - scripts/issue-dedupe.cjs: pure, unit-tested module (tokenize, Sørensen–Dice title similarity, scoreCandidates, renderChallengeComment, shouldClose) with fail-safe destructive-action guards. - duplicate-check.yml (issues:opened): scores new-issue title against open issues, posts a challenge comment + applies the pending `possible-duplicate` label on a clear match. - duplicate-sweep.yml (daily cron): closes possible-duplicate issues whose challenge comment is >24h old with no human reply and no 👎 veto; honors exempt labels; re-checks the label immediately before close (TOCTOU guard); strips the label on close to avoid reopen loops. - remove-duplicate-label.yml (issue_comment:created): clears the label and applies needs-maintainer-review when any human responds. - bug_report.yml / docs_issue.yml: add the required "I searched existing issues" preflight checkbox so all five forms force a pre-search attestation. - docs/agents/triage-labels.md: document the label + lifecycle. Co-Authored-By: Claude Opus 4.8 * chore(#836): add changeset fragment for duplicate-issue detection Co-Authored-By: Claude Opus 4.8 --------- Co-authored-by: Claude Opus 4.8 --- .changeset/eager-birds-squeak.md | 5 + .github/ISSUE_TEMPLATE/bug_report.yml | 8 + .github/ISSUE_TEMPLATE/docs_issue.yml | 8 + .github/workflows/duplicate-check.yml | 54 ++ .github/workflows/duplicate-sweep.yml | 91 +++ .github/workflows/remove-duplicate-label.yml | 39 ++ docs/agents/triage-labels.md | 10 + scripts/issue-dedupe.cjs | 278 +++++++++ tests/issue-dedupe.test.cjs | 624 +++++++++++++++++++ 9 files changed, 1117 insertions(+) create mode 100644 .changeset/eager-birds-squeak.md create mode 100644 .github/workflows/duplicate-check.yml create mode 100644 .github/workflows/duplicate-sweep.yml create mode 100644 .github/workflows/remove-duplicate-label.yml create mode 100644 scripts/issue-dedupe.cjs create mode 100644 tests/issue-dedupe.test.cjs diff --git a/.changeset/eager-birds-squeak.md b/.changeset/eager-birds-squeak.md new file mode 100644 index 000000000..1963ad041 --- /dev/null +++ b/.changeset/eager-birds-squeak.md @@ -0,0 +1,5 @@ +--- +type: Added +pr: 843 +--- +Issues are now checked for duplicates when opened: a no-LLM title-similarity check posts a challenge comment and applies a `possible-duplicate` label when a new issue closely matches existing open ones. Flagged issues that go unanswered for 24h are auto-closed as duplicates (reply, or react 👎 to the bot comment, to keep one open); a reply clears the label and routes to `needs-maintainer-review`. (#836) diff --git a/.github/ISSUE_TEMPLATE/bug_report.yml b/.github/ISSUE_TEMPLATE/bug_report.yml index c1623c685..f1bfa3727 100644 --- a/.github/ISSUE_TEMPLATE/bug_report.yml +++ b/.github/ISSUE_TEMPLATE/bug_report.yml @@ -13,6 +13,14 @@ body: > 2. Redact usernames, paths, and API keys (e.g., replace `/Users/yourname/` with `/Users/REDACTED/`) > 3. Or run your logs through an anonymizer — we recommend **[presidio-anonymizer](https://microsoft.github.io/presidio/)** (open-source, local-only) or **[scrub](https://github.com/dssg/scrub)** before pasting + - type: checkboxes + id: preflight + attributes: + label: Pre-submission checklist + options: + - label: I have searched existing issues and this bug has not already been reported + required: true + - type: input id: version attributes: diff --git a/.github/ISSUE_TEMPLATE/docs_issue.yml b/.github/ISSUE_TEMPLATE/docs_issue.yml index b40577b30..ce1e324a4 100644 --- a/.github/ISSUE_TEMPLATE/docs_issue.yml +++ b/.github/ISSUE_TEMPLATE/docs_issue.yml @@ -8,6 +8,14 @@ body: value: | Help us improve the docs. Point us to what's wrong or missing. + - type: checkboxes + id: preflight + attributes: + label: Pre-submission checklist + options: + - label: I have searched existing issues and this documentation problem has not already been reported + required: true + - type: dropdown id: type attributes: diff --git a/.github/workflows/duplicate-check.yml b/.github/workflows/duplicate-check.yml new file mode 100644 index 000000000..0931c4575 --- /dev/null +++ b/.github/workflows/duplicate-check.yml @@ -0,0 +1,54 @@ +name: Duplicate check + +on: + issues: + types: [opened] + +concurrency: + group: ${{ github.workflow }}-${{ github.event.issue.number }} + cancel-in-progress: true + +permissions: + issues: write + contents: read + +jobs: + detect: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + - uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 + with: + script: | + const dedupe = require(`${process.env.GITHUB_WORKSPACE}/scripts/issue-dedupe.cjs`); + const issue = context.payload.issue; + if (issue.pull_request) return; + const existing = (issue.labels || []).map((l) => (typeof l === 'string' ? l : l.name)); + if (existing.includes(dedupe.POSSIBLE_DUPLICATE_LABEL)) return; + const open = await github.paginate(github.rest.issues.listForRepo, { + owner: context.repo.owner, + repo: context.repo.repo, + state: 'open', + per_page: 100, + }); + const candidates = open + .filter((i) => !i.pull_request && i.number !== issue.number) + .map((i) => ({ number: i.number, title: i.title })); + const matches = dedupe.scoreCandidates(issue.title, candidates, { excludeNumber: issue.number }); + if (!matches.length) { + core.info('No similar open issues found.'); + return; + } + await github.rest.issues.createComment({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: issue.number, + body: dedupe.renderChallengeComment(matches, { windowHours: dedupe.DEFAULT_WINDOW_HOURS }), + }); + await github.rest.issues.addLabels({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: issue.number, + labels: [dedupe.POSSIBLE_DUPLICATE_LABEL], + }); + core.info(`Flagged #${issue.number} as possible duplicate of: ${matches.map((m) => '#' + m.number).join(', ')}`); diff --git a/.github/workflows/duplicate-sweep.yml b/.github/workflows/duplicate-sweep.yml new file mode 100644 index 000000000..083c32118 --- /dev/null +++ b/.github/workflows/duplicate-sweep.yml @@ -0,0 +1,91 @@ +name: Duplicate auto-close sweep + +on: + schedule: + - cron: '0 7 * * *' + workflow_dispatch: + +permissions: + issues: write + contents: read + +jobs: + sweep: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + - uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 + with: + script: | + const dedupe = require(`${process.env.GITHUB_WORKSPACE}/scripts/issue-dedupe.cjs`); + const { owner, repo } = context.repo; + const issues = await github.paginate(github.rest.issues.listForRepo, { + owner, + repo, + state: 'open', + labels: dedupe.POSSIBLE_DUPLICATE_LABEL, + per_page: 100, + }); + const now = Date.now(); + for (const issue of issues) { + if (issue.pull_request) continue; + const comments = await github.paginate(github.rest.issues.listComments, { + owner, + repo, + issue_number: issue.number, + per_page: 100, + }); + const challenges = comments.filter((c) => dedupe.isChallengeComment(c.body)); + const challenge = challenges[challenges.length - 1]; + let challengeComment = null; + let laterUserComments = 0; + if (challenge) { + const challengeAt = new Date(challenge.created_at).getTime(); + laterUserComments = comments.filter( + (c) => new Date(c.created_at).getTime() > challengeAt && c.user && c.user.type !== 'Bot', + ).length; + const reactions = await github.paginate(github.rest.reactions.listForIssueComment, { + owner, + repo, + comment_id: challenge.id, + per_page: 100, + }); + const downvoted = reactions.some((r) => r.content === '-1'); + challengeComment = { createdAt: challenge.created_at, downvoted }; + } + const decision = dedupe.shouldClose({ + now, + labels: issue.labels, + challengeComment, + laterUserComments, + windowHours: dedupe.DEFAULT_WINDOW_HOURS, + }); + core.info(`#${issue.number}: ${decision.reason}`); + if (!decision.close) continue; + const fresh = await github.rest.issues.get({ owner, repo, issue_number: issue.number }); + if (fresh.data.state !== 'open') continue; + const freshLabels = (fresh.data.labels || []).map((l) => (typeof l === 'string' ? l : l.name)); + if (!freshLabels.includes(dedupe.POSSIBLE_DUPLICATE_LABEL)) { + core.info(`#${issue.number}: possible-duplicate cleared since snapshot, skipping close`); + continue; + } + await github.rest.issues.createComment({ + owner, + repo, + issue_number: issue.number, + body: `Closing as a likely duplicate — no response within ${dedupe.DEFAULT_WINDOW_HOURS}h of the duplicate check. If this was a mistake, comment and a maintainer will reopen it.`, + }); + await github.rest.issues.update({ + owner, + repo, + issue_number: issue.number, + state: 'closed', + state_reason: 'duplicate', + }); + await github.rest.issues.removeLabel({ + owner, + repo, + issue_number: issue.number, + name: dedupe.POSSIBLE_DUPLICATE_LABEL, + }).catch((e) => core.info(`removeLabel after close: ${e.message}`)); + } diff --git a/.github/workflows/remove-duplicate-label.yml b/.github/workflows/remove-duplicate-label.yml new file mode 100644 index 000000000..af71abd9e --- /dev/null +++ b/.github/workflows/remove-duplicate-label.yml @@ -0,0 +1,39 @@ +name: Clear possible-duplicate on response + +on: + issue_comment: + types: [created] + +permissions: + issues: write + contents: read + +jobs: + clear: + if: ${{ !github.event.issue.pull_request }} + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + - uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 + with: + script: | + const dedupe = require(`${process.env.GITHUB_WORKSPACE}/scripts/issue-dedupe.cjs`); + const { owner, repo } = context.repo; + const comment = context.payload.comment; + const issue = context.payload.issue; + if (comment.user && comment.user.type === 'Bot') return; + const existing = (issue.labels || []).map((l) => (typeof l === 'string' ? l : l.name)); + if (!existing.includes(dedupe.POSSIBLE_DUPLICATE_LABEL)) return; + await github.rest.issues.removeLabel({ + owner, + repo, + issue_number: issue.number, + name: dedupe.POSSIBLE_DUPLICATE_LABEL, + }).catch((e) => core.info(`removeLabel: ${e.message}`)); + await github.rest.issues.addLabels({ + owner, + repo, + issue_number: issue.number, + labels: [dedupe.HUMAN_REVIEW_LABEL], + }); + core.info(`Cleared possible-duplicate on #${issue.number}; routed to ${dedupe.HUMAN_REVIEW_LABEL}.`); diff --git a/docs/agents/triage-labels.md b/docs/agents/triage-labels.md index 04b40b648..a78db6082 100644 --- a/docs/agents/triage-labels.md +++ b/docs/agents/triage-labels.md @@ -9,6 +9,7 @@ Maps the five canonical triage roles to the actual label strings in `open-gsd/gs | `ready-for-agent` | `confirmed` | Bug verified + fully specified — AFK agent can pick up | | `ready-for-human` | `approved-enhancement` / `approved-feature` | Enhancement/feature approved by maintainer — human codes it | | `wontfix` | `wontfix` | Will not be actioned | +| `possible-duplicate` | `possible-duplicate` | Applied by the Duplicate check workflow when a new issue's title closely matches existing open issues. The reporter (or a maintainer) replies justifying why it is not a duplicate within 24h, or the Duplicate auto-close sweep closes it. A reply clears this label and applies needs-maintainer-review for human adjudication. React 👎 to the bot comment to veto auto-close. | ## Notes on this repo's label model @@ -17,3 +18,12 @@ Maps the five canonical triage roles to the actual label strings in `open-gsd/gs - There is no separate "ready-for-human" vs "ready-for-agent" distinction for enhancements — both flow through the same `approved-*` labels. If the work requires human judgment (design decisions, external access), note it in the issue body. - `needs-triage` is removed when any other state label is applied. - `needs-reproduction` is used instead of the generic `needs-info` — be specific in triage comments about what reproduction steps or information are missing. + +## Duplicate detection lifecycle + +The `possible-duplicate` label is managed by three GitHub Actions workflows that together form a self-service deduplication loop: + +1. **Detect on open** — When an issue is opened, `duplicate-check.yml` scores its title against all other open issues using Dice-coefficient similarity. If any match clears the threshold, the bot posts a challenge comment listing the similar issues and applies `possible-duplicate`. +2. **Challenge comment + reporter window** — The reporter (or a maintainer) has `DEFAULT_WINDOW_HOURS` (24h) to reply explaining why the issue is not a duplicate. Reacting 👎 to the bot comment also signals the reporter objects to auto-close. +3. **Daily sweep auto-close** — `duplicate-sweep.yml` runs at 07:00 UTC daily. For each open issue with `possible-duplicate`, it checks whether the window has elapsed, whether the reporter replied, and whether a 👎 reaction exists. Issues with exempt labels (`priority: critical`, `pinned`, `confirmed-bug`, `confirmed`, `fix-pending`) are never auto-closed. Issues that pass the close check receive a closing comment and are closed with `state_reason: duplicate`. +4. **Reporter reply clears label** — `remove-duplicate-label.yml` fires on every new non-bot comment. If the issue still carries `possible-duplicate`, it removes that label and applies `needs-maintainer-review` (the value of `HUMAN_REVIEW_LABEL` in `scripts/issue-dedupe.cjs`), routing the issue to a maintainer for manual adjudication. diff --git a/scripts/issue-dedupe.cjs b/scripts/issue-dedupe.cjs new file mode 100644 index 000000000..fc2204bd8 --- /dev/null +++ b/scripts/issue-dedupe.cjs @@ -0,0 +1,278 @@ +#!/usr/bin/env node +'use strict'; + +// --------------------------------------------------------------------------- +// Constants +// --------------------------------------------------------------------------- + +const POSSIBLE_DUPLICATE_LABEL = 'possible-duplicate'; +const HUMAN_REVIEW_LABEL = 'needs-maintainer-review'; +const CHALLENGE_MARKER = ''; +const DEFAULT_WINDOW_HOURS = 24; +const DEFAULT_THRESHOLD = 0.6; +const DEFAULT_MAX_CANDIDATES = 5; +const MIN_TOKEN_LENGTH = 3; + +const EXEMPT_LABELS = [ + 'priority: critical', + 'pinned', + 'confirmed-bug', + 'confirmed', + 'fix-pending', + 'needs-maintainer-review', +]; + +const STOPWORDS = new Set([ + 'the', 'a', 'an', 'and', 'or', 'but', 'if', 'then', 'is', 'are', 'was', + 'be', 'to', 'of', 'in', 'on', 'for', 'with', 'as', 'at', 'by', 'from', + 'this', 'that', 'it', 'its', 'not', 'no', 'when', 'what', 'why', 'how', + 'does', 'do', 'doing', 'did', 'can', 'will', 'would', 'should', + 'i', 'we', 'you', 'your', 'my', 'me', + 'issue', 'bug', 'error', 'problem', 'feature', 'request', + 'help', 'support', 'please', 'question', + 'after', 'before', 'into', 'only', 'then', 'than', 'them', 'they', + 'use', 'used', 'using', +]); + +// --------------------------------------------------------------------------- +// tokenize(title) -> string[] +// +// Lowercase the title, replace any non-[a-z0-9] run with a space, split on +// whitespace, drop tokens shorter than MIN_TOKEN_LENGTH, drop STOPWORDS, and +// dedupe while preserving stable first-occurrence order. +// +// Non-string, null, or empty input returns []. Must not throw on any input. +// --------------------------------------------------------------------------- + +function tokenize(title) { + if (typeof title !== 'string' || !title) return []; + + const normalized = title.toLowerCase().replace(/[^a-z0-9]+/g, ' ').trim(); + if (!normalized) return []; + + const seen = new Set(); + const result = []; + + for (const token of normalized.split(' ')) { + if (!token || token.length < MIN_TOKEN_LENGTH) continue; + if (STOPWORDS.has(token)) continue; + if (seen.has(token)) continue; + seen.add(token); + result.push(token); + } + + return result; +} + +// --------------------------------------------------------------------------- +// diceSimilarity(aTokens, bTokens) -> number 0..1 +// +// Sørensen–Dice over the token sets: 2 * |A ∩ B| / (|A| + |B|). +// Both inputs are treated as sets (duplicates ignored). Empty either side -> 0. +// Identical sets -> 1. +// --------------------------------------------------------------------------- + +function diceSimilarity(aTokens, bTokens) { + const setA = new Set(aTokens); + const setB = new Set(bTokens); + + if (setA.size === 0 || setB.size === 0) return 0; + + let intersection = 0; + for (const token of setA) { + if (setB.has(token)) intersection += 1; + } + + return (2 * intersection) / (setA.size + setB.size); +} + +// --------------------------------------------------------------------------- +// scoreCandidates(newTitle, candidates, opts) -> [{number, title, score}] +// +// opts: { threshold=DEFAULT_THRESHOLD, limit=DEFAULT_MAX_CANDIDATES, excludeNumber } +// +// Tokenizes newTitle once. If no tokens -> []. Filters out null/garbage +// candidates, those missing a number, and the excluded number. Scores each +// using diceSimilarity. Keeps score >= threshold. Sorts DESC by score, +// tie-break ASC by number. Caps to limit. +// --------------------------------------------------------------------------- + +function scoreCandidates(newTitle, candidates, opts) { + const threshold = (opts && opts.threshold != null) ? opts.threshold : DEFAULT_THRESHOLD; + const limit = (opts && opts.limit != null) ? opts.limit : DEFAULT_MAX_CANDIDATES; + const excludeNumber = opts && opts.excludeNumber; + + const newTokens = tokenize(newTitle); + if (newTokens.length === 0) return []; + + const scored = []; + + const safeCandidates = Array.isArray(candidates) ? candidates : []; + for (const candidate of safeCandidates) { + if (!candidate || typeof candidate !== 'object') continue; + if (!(typeof candidate.number === 'number' && Number.isFinite(candidate.number))) continue; + if (candidate.number === excludeNumber) continue; + + const score = diceSimilarity(newTokens, tokenize(candidate.title)); + if (score < threshold) continue; + + scored.push({ number: candidate.number, title: candidate.title, score }); + } + + scored.sort((a, b) => { + if (Math.abs(a.score - b.score) > 1e-9) return b.score - a.score; + return a.number - b.number; + }); + + return scored.slice(0, limit); +} + +// --------------------------------------------------------------------------- +// renderChallengeComment(candidates, opts) -> string +// +// opts: { windowHours=DEFAULT_WINDOW_HOURS } +// +// Deterministic. Must start with CHALLENGE_MARKER on its own line. Must list +// each candidate as a line with #, title, and percentage similarity. +// Must mention windowHours and the 👎 veto. +// --------------------------------------------------------------------------- + +function renderChallengeComment(candidates, opts) { + const windowHours = (opts && opts.windowHours != null) ? opts.windowHours : DEFAULT_WINDOW_HOURS; + + const lines = [CHALLENGE_MARKER, '']; + + lines.push('**Possible duplicate detected.** This issue may already be reported:'); + lines.push(''); + + for (const candidate of candidates) { + const pct = Math.round(candidate.score * 100); + lines.push(`- #${candidate.number} — ${candidate.title} (similarity ${pct}%)`); + } + + lines.push(''); + lines.push( + `If this is **not** a duplicate, react with 👎 on this comment to veto and keep the issue open. ` + + `If no response is received within ${windowHours} hours, this issue may be closed as a duplicate.`, + ); + + return lines.join('\n'); +} + +// --------------------------------------------------------------------------- +// isChallengeComment(body) -> boolean +// +// True iff body is a string containing CHALLENGE_MARKER. +// --------------------------------------------------------------------------- + +function isChallengeComment(body) { + if (typeof body !== 'string') return false; + return body.includes(CHALLENGE_MARKER); +} + +// --------------------------------------------------------------------------- +// hasExemptLabel(labels) -> boolean +// +// labels may be an array of strings or array of {name}. True if any name is +// in EXEMPT_LABELS. +// --------------------------------------------------------------------------- + +function hasExemptLabel(labels) { + if (!Array.isArray(labels)) return false; + const exemptSet = new Set(EXEMPT_LABELS); + for (const label of labels) { + const name = typeof label === 'string' ? label : (label && label.name); + if (name && exemptSet.has(name)) return true; + } + return false; +} + +// --------------------------------------------------------------------------- +// toMs(value) -> number +// +// Coerce a Date, ISO string, or ms-number to milliseconds since epoch. +// --------------------------------------------------------------------------- + +function toMs(value) { + if (value instanceof Date) return value.getTime(); + if (typeof value === 'string') return new Date(value).getTime(); + return Number(value); +} + +// --------------------------------------------------------------------------- +// shouldClose(input) -> {close: boolean, reason: string} +// +// input: { now, labels, challengeComment, laterUserComments, windowHours=DEFAULT_WINDOW_HOURS } +// +// Decision order (returns first match): +// 1. hasExemptLabel(labels) -> {close:false, reason:'exempt-label'} +// 2. !challengeComment -> {close:false, reason:'no-challenge-comment'} +// 3. challengeComment.downvoted -> {close:false, reason:'vetoed'} +// 4. laterUserComments > 0 -> {close:false, reason:'reporter-responded'} +// 5. ageHours < windowHours -> {close:false, reason:'within-window'} +// 6. else -> {close:true, reason:'duplicate-no-response'} +// --------------------------------------------------------------------------- + +function shouldClose(input) { + const { + now, + labels = [], + challengeComment, + laterUserComments = 0, + } = input; + const windowHours = (input.windowHours != null) ? input.windowHours : DEFAULT_WINDOW_HOURS; + + if (hasExemptLabel(labels)) { + return { close: false, reason: 'exempt-label' }; + } + + if (!challengeComment) { + return { close: false, reason: 'no-challenge-comment' }; + } + + if (challengeComment.downvoted) { + return { close: false, reason: 'vetoed' }; + } + + if (laterUserComments > 0) { + return { close: false, reason: 'reporter-responded' }; + } + + const nowMs = toMs(now); + const createdMs = toMs(challengeComment.createdAt); + + if (!Number.isFinite(nowMs) || !Number.isFinite(createdMs)) { + return { close: false, reason: 'invalid-timestamp' }; + } + + const ageHours = (nowMs - createdMs) / 3600000; + + if (ageHours < windowHours) { + return { close: false, reason: 'within-window' }; + } + + return { close: true, reason: 'duplicate-no-response' }; +} + +// --------------------------------------------------------------------------- +// Exports +// --------------------------------------------------------------------------- + +module.exports = { + POSSIBLE_DUPLICATE_LABEL, + HUMAN_REVIEW_LABEL, + CHALLENGE_MARKER, + DEFAULT_WINDOW_HOURS, + DEFAULT_THRESHOLD, + DEFAULT_MAX_CANDIDATES, + MIN_TOKEN_LENGTH, + EXEMPT_LABELS, + STOPWORDS, + tokenize, + diceSimilarity, + scoreCandidates, + renderChallengeComment, + isChallengeComment, + hasExemptLabel, + shouldClose, +}; diff --git a/tests/issue-dedupe.test.cjs b/tests/issue-dedupe.test.cjs new file mode 100644 index 000000000..eb4695a9b --- /dev/null +++ b/tests/issue-dedupe.test.cjs @@ -0,0 +1,624 @@ +'use strict'; + +const { describe, test } = require('node:test'); +const assert = require('node:assert/strict'); + +const { + POSSIBLE_DUPLICATE_LABEL, + HUMAN_REVIEW_LABEL, + CHALLENGE_MARKER, + DEFAULT_WINDOW_HOURS, + DEFAULT_THRESHOLD, + DEFAULT_MAX_CANDIDATES, + MIN_TOKEN_LENGTH, + EXEMPT_LABELS, + STOPWORDS, + tokenize, + diceSimilarity, + scoreCandidates, + renderChallengeComment, + isChallengeComment, + hasExemptLabel, + shouldClose, +} = require('../scripts/issue-dedupe.cjs'); + +// --------------------------------------------------------------------------- +// Constants +// --------------------------------------------------------------------------- + +describe('issue-dedupe constants', () => { + test('POSSIBLE_DUPLICATE_LABEL is correct string', () => { + assert.equal(POSSIBLE_DUPLICATE_LABEL, 'possible-duplicate'); + }); + + test('HUMAN_REVIEW_LABEL is correct string', () => { + assert.equal(HUMAN_REVIEW_LABEL, 'needs-maintainer-review'); + }); + + test('CHALLENGE_MARKER is an HTML comment string', () => { + assert.ok(typeof CHALLENGE_MARKER === 'string'); + assert.ok(CHALLENGE_MARKER.startsWith('')); + }); + + test('DEFAULT_WINDOW_HOURS is 24', () => { + assert.equal(DEFAULT_WINDOW_HOURS, 24); + }); + + test('DEFAULT_THRESHOLD is 0.6', () => { + assert.equal(DEFAULT_THRESHOLD, 0.6); + }); + + test('DEFAULT_MAX_CANDIDATES is 5', () => { + assert.equal(DEFAULT_MAX_CANDIDATES, 5); + }); + + test('MIN_TOKEN_LENGTH is 3', () => { + assert.equal(MIN_TOKEN_LENGTH, 3); + }); + + test('EXEMPT_LABELS includes required entries', () => { + assert.ok(Array.isArray(EXEMPT_LABELS)); + for (const label of ['priority: critical', 'pinned', 'confirmed-bug', 'needs-maintainer-review']) { + assert.ok(EXEMPT_LABELS.includes(label), `EXEMPT_LABELS must contain "${label}"`); + } + }); + + test('STOPWORDS is a Set containing common words', () => { + assert.ok(STOPWORDS instanceof Set, 'STOPWORDS must be a Set'); + for (const word of ['the', 'and', 'bug', 'issue', 'please']) { + assert.ok(STOPWORDS.has(word), `STOPWORDS must contain "${word}"`); + } + }); +}); + +// --------------------------------------------------------------------------- +// tokenize +// --------------------------------------------------------------------------- + +describe('tokenize', () => { + test('lowercases, strips punctuation, drops stopwords, dedupes', () => { + // 'CLI', 'crashes', '--flag', 'input' survive; 'Bug', 'on', 'with' are stopwords + const tokens = tokenize('Bug: CLI crashes on --flag with input!!!'); + assert.ok(Array.isArray(tokens)); + assert.ok(tokens.includes('cli')); + assert.ok(tokens.includes('crashes')); + assert.ok(tokens.includes('flag')); + assert.ok(tokens.includes('input')); + // stopwords must be absent + assert.ok(!tokens.includes('bug')); + assert.ok(!tokens.includes('on')); + assert.ok(!tokens.includes('with')); + // all lowercase + for (const t of tokens) assert.equal(t, t.toLowerCase()); + }); + + test('drops tokens shorter than MIN_TOKEN_LENGTH', () => { + // 'ab' is 2 chars, 'xy' is 2 chars — both under MIN_TOKEN_LENGTH=3 + const tokens = tokenize('ab xy hello'); + assert.ok(!tokens.includes('ab')); + assert.ok(!tokens.includes('xy')); + assert.ok(tokens.includes('hello')); + }); + + test('deduplicates tokens (stable first-occurrence order)', () => { + const tokens = tokenize('crash crash crash happened happened'); + assert.equal(tokens.filter((t) => t === 'crash').length, 1); + assert.equal(tokens.filter((t) => t === 'happened').length, 1); + // first occurrence order: crash before happened + assert.ok(tokens.indexOf('crash') < tokens.indexOf('happened')); + }); + + test('emoji-only title returns empty array and does not throw', () => { + const tokens = tokenize('🔥🔥'); + assert.ok(Array.isArray(tokens)); + assert.equal(tokens.length, 0); + }); + + test('empty string returns empty array', () => { + assert.deepEqual(tokenize(''), []); + }); + + test('null returns empty array', () => { + assert.deepEqual(tokenize(null), []); + }); + + test('non-string number returns empty array', () => { + assert.deepEqual(tokenize(123), []); + }); + + test('all-stopword title returns empty array', () => { + const tokens = tokenize('the and or but if'); + assert.deepEqual(tokens, []); + }); + + test('complex real-world title with mixed punctuation and short words', () => { + const tokens = tokenize('Bug: CLI crashes on --flag with空 input!!!'); + // non-ascii gets stripped along with punctuation — '空' becomes empty + // remaining meaningful tokens should be present + assert.ok(tokens.includes('cli')); + assert.ok(tokens.includes('crashes')); + assert.ok(tokens.includes('flag')); + assert.ok(tokens.includes('input')); + }); + + test('title with only very short words returns empty array', () => { + // 'to', 'be', 'or', 'it' are all either stopwords or too short (<3 chars) + const tokens = tokenize('to be or it'); + assert.deepEqual(tokens, []); + }); +}); + +// --------------------------------------------------------------------------- +// diceSimilarity +// --------------------------------------------------------------------------- + +describe('diceSimilarity', () => { + test('identical token arrays return 1', () => { + assert.equal(diceSimilarity(['foo', 'bar', 'baz'], ['foo', 'bar', 'baz']), 1); + }); + + test('disjoint token arrays return 0', () => { + assert.equal(diceSimilarity(['foo', 'bar'], ['baz', 'qux']), 0); + }); + + test('partial overlap returns expected dice coefficient', () => { + // A = {a,b,c}, B = {b,c,d}: intersection = {b,c} -> 2*2/(3+3) = 4/6 ≈ 0.6667 + const score = diceSimilarity(['a', 'b', 'c'], ['b', 'c', 'd']); + assert.ok(Math.abs(score - (4 / 6)) < 0.001, `Expected ~0.6667, got ${score}`); + }); + + test('empty first array returns 0', () => { + assert.equal(diceSimilarity([], ['foo', 'bar']), 0); + }); + + test('empty second array returns 0', () => { + assert.equal(diceSimilarity(['foo', 'bar'], []), 0); + }); + + test('both empty returns 0', () => { + assert.equal(diceSimilarity([], []), 0); + }); + + test('single common token: A={x}, B={x} -> 1', () => { + assert.equal(diceSimilarity(['x'], ['x']), 1); + }); + + test('single token each, different: 0', () => { + assert.equal(diceSimilarity(['x'], ['y']), 0); + }); + + test('treats token arrays as sets (duplicates in input do not inflate score)', () => { + // Even if caller somehow passes duplicates, score should still be well-formed + // A={foo,bar}, B={foo,bar}: expect 1 even with duplicates in input + const score = diceSimilarity(['foo', 'foo', 'bar'], ['foo', 'bar', 'bar']); + // The function works on sets internally; result should be 1 + assert.equal(score, 1); + }); +}); + +// --------------------------------------------------------------------------- +// scoreCandidates +// --------------------------------------------------------------------------- + +describe('scoreCandidates', () => { + const candidates = [ + { number: 10, title: 'CLI crashes on startup with bad config' }, + { number: 11, title: 'Something entirely different here' }, + { number: 12, title: 'CLI crashes with segfault on startup' }, + { number: 13, title: 'Feature request add dark mode' }, + ]; + + test('excludeNumber excludes self from results', () => { + const results = scoreCandidates('CLI crashes on startup with bad config', candidates, { excludeNumber: 10 }); + assert.ok(!results.some((r) => r.number === 10)); + }); + + test('threshold filters out low-scoring candidates', () => { + const results = scoreCandidates('CLI crashes on startup', candidates, { threshold: 0.9 }); + // Only very close matches should survive + for (const r of results) { + assert.ok(r.score >= 0.9, `Score ${r.score} for #${r.number} is below threshold`); + } + }); + + test('results are sorted descending by score, ascending by number on tie', () => { + const results = scoreCandidates('CLI crashes startup config', candidates, { threshold: 0 }); + for (let i = 1; i < results.length; i++) { + const prev = results[i - 1]; + const curr = results[i]; + if (Math.abs(prev.score - curr.score) < 0.0001) { + // tie-break: ascending number + assert.ok(prev.number < curr.number, 'Tie-break should be ascending by number'); + } else { + assert.ok(prev.score >= curr.score, 'Results should be sorted descending by score'); + } + } + }); + + test('limit caps the number of results', () => { + const manyCandidates = Array.from({ length: 20 }, (_, i) => ({ + number: i + 1, + title: `CLI crashes startup config issue ${i}`, + })); + const results = scoreCandidates('CLI crashes startup config', manyCandidates, { threshold: 0, limit: 3 }); + assert.ok(results.length <= 3); + }); + + test('all-stopword newTitle returns empty array', () => { + const results = scoreCandidates('the and or but', candidates, { threshold: 0 }); + assert.deepEqual(results, []); + }); + + test('null candidate entries are tolerated (filtered out)', () => { + const messyCandidates = [null, undefined, { number: 1, title: 'CLI crashes badly' }, { title: 'no number' }, { number: 2, title: null }]; + let results; + assert.doesNotThrow(() => { + results = scoreCandidates('CLI crashes', messyCandidates, { threshold: 0 }); + }); + // entry without number should be skipped; entry with null title should survive (tokenize(null) = []) + assert.ok(!results.some((r) => r.number === undefined || r.number === null)); + }); + + test('each result has number, title, and score fields', () => { + const results = scoreCandidates('CLI crashes on startup', candidates, { threshold: 0 }); + for (const r of results) { + assert.ok('number' in r, 'result must have number'); + assert.ok('title' in r, 'result must have title'); + assert.ok('score' in r, 'result must have score'); + assert.ok(typeof r.score === 'number'); + assert.ok(r.score >= 0 && r.score <= 1); + } + }); + + test('defaults: threshold=DEFAULT_THRESHOLD, limit=DEFAULT_MAX_CANDIDATES', () => { + // With default opts, results should respect DEFAULT_THRESHOLD + const results = scoreCandidates('CLI crashes', candidates); + for (const r of results) { + assert.ok(r.score >= DEFAULT_THRESHOLD); + } + assert.ok(results.length <= DEFAULT_MAX_CANDIDATES); + }); + + test('candidate with missing number is skipped', () => { + const withNoNumber = [{ title: 'CLI crashes badly' }, { number: 5, title: 'CLI crashes badly' }]; + const results = scoreCandidates('CLI crashes', withNoNumber, { threshold: 0 }); + assert.ok(!results.some((r) => r.number === undefined)); + assert.ok(results.some((r) => r.number === 5)); + }); + + test('default threshold 0.6 excludes near-miss ~0.545, but explicit 0.5 includes it', () => { + // newTitle tokens: ['cli', 'crashes', 'startup'] (3 tokens — 'on' is stopword) + // candidate tokens: ['cli', 'crashes', 'startup', 'mode', 'display', 'render', 'timeout'] (7 tokens) + // intersection = 3, dice = 2*3/(3+7) = 6/10 = 0.6 exactly — adjust to get below 0.6 + // newTitle tokens: ['crashes', 'startup'] (2 tokens after removing 'cli' via excludeNumber not applicable here) + // Use a 4-token new title and 7-token candidate with 3 shared for 6/11 ≈ 0.545 + // newTitle: 'CLI crashes startup config' → tokens: ['cli','crashes','startup','config'] (4 tokens) + // candidate: 'CLI crashes startup mode display render timeout' → tokens: ['cli','crashes','startup','mode','display','render','timeout'] (7 tokens) + // intersection = {cli,crashes,startup} = 3; dice = 2*3/(4+7) = 6/11 ≈ 0.5454 + const nearMissCandidate = [{ number: 99, title: 'CLI crashes startup mode display render timeout' }]; + const scoreVal = 6 / 11; // ≈ 0.5454 + assert.ok(scoreVal < 0.6, 'sanity: near-miss score is below 0.6'); + assert.ok(scoreVal > 0.5, 'sanity: near-miss score is above 0.5'); + + // Default threshold (0.6) should exclude it + const withDefault = scoreCandidates('CLI crashes startup config', nearMissCandidate); + assert.equal(withDefault.length, 0, 'default threshold 0.6 must exclude ~0.545 score'); + + // Explicit threshold 0.5 should include it + const withLower = scoreCandidates('CLI crashes startup config', nearMissCandidate, { threshold: 0.5 }); + assert.equal(withLower.length, 1, 'explicit threshold 0.5 must include ~0.545 score'); + assert.ok(withLower[0].score > 0.5 && withLower[0].score < 0.6); + }); +}); + +// --------------------------------------------------------------------------- +// renderChallengeComment +// --------------------------------------------------------------------------- + +describe('renderChallengeComment', () => { + const sampleCandidates = [ + { number: 42, title: 'CLI crashes on startup', score: 0.83 }, + { number: 7, title: 'Segfault when starting CLI', score: 0.66 }, + ]; + + test('output starts with CHALLENGE_MARKER on its own line', () => { + const output = renderChallengeComment(sampleCandidates, {}); + const firstLine = output.split('\n')[0]; + assert.equal(firstLine.trim(), CHALLENGE_MARKER); + }); + + test('output contains each candidate number prefixed with #', () => { + const output = renderChallengeComment(sampleCandidates, {}); + assert.ok(output.includes('#42')); + assert.ok(output.includes('#7')); + }); + + test('output contains each candidate title', () => { + const output = renderChallengeComment(sampleCandidates, {}); + assert.ok(output.includes('CLI crashes on startup')); + assert.ok(output.includes('Segfault when starting CLI')); + }); + + test('output includes percentage similarity rounded correctly', () => { + const output = renderChallengeComment(sampleCandidates, {}); + // 0.83 -> 83%, 0.66 -> 66% + assert.ok(output.includes('83%'), 'Expected 83% in output'); + assert.ok(output.includes('66%'), 'Expected 66% in output'); + }); + + test('output mentions the windowHours', () => { + const output = renderChallengeComment(sampleCandidates, { windowHours: 48 }); + assert.ok(output.includes('48'), 'Output must mention windowHours=48'); + }); + + test('output includes 👎 veto character', () => { + const output = renderChallengeComment(sampleCandidates, {}); + assert.ok(output.includes('👎'), 'Output must include the 👎 veto emoji'); + }); + + test('output is deterministic (same input produces same output)', () => { + const a = renderChallengeComment(sampleCandidates, {}); + const b = renderChallengeComment(sampleCandidates, {}); + assert.equal(a, b); + }); + + test('isChallengeComment(renderChallengeComment(...)) is true', () => { + const output = renderChallengeComment(sampleCandidates, {}); + assert.equal(isChallengeComment(output), true); + }); + + test('uses DEFAULT_WINDOW_HOURS when windowHours not provided', () => { + const output = renderChallengeComment(sampleCandidates, {}); + assert.ok(output.includes(String(DEFAULT_WINDOW_HOURS))); + }); +}); + +// --------------------------------------------------------------------------- +// isChallengeComment +// --------------------------------------------------------------------------- + +describe('isChallengeComment', () => { + test('returns true when body contains CHALLENGE_MARKER', () => { + assert.equal(isChallengeComment(`${CHALLENGE_MARKER}\nsome content`), true); + }); + + test('returns false when body does not contain CHALLENGE_MARKER', () => { + assert.equal(isChallengeComment('just a normal comment'), false); + }); + + test('returns false for empty string', () => { + assert.equal(isChallengeComment(''), false); + }); + + test('returns false for non-string', () => { + assert.equal(isChallengeComment(null), false); + assert.equal(isChallengeComment(undefined), false); + assert.equal(isChallengeComment(42), false); + }); +}); + +// --------------------------------------------------------------------------- +// hasExemptLabel +// --------------------------------------------------------------------------- + +describe('hasExemptLabel', () => { + test('returns true for string label matching an exempt label', () => { + assert.equal(hasExemptLabel(['priority: critical']), true); + }); + + test('returns true for object label {name} matching an exempt label', () => { + assert.equal(hasExemptLabel([{ name: 'pinned' }]), true); + }); + + test('returns true for confirmed-bug', () => { + assert.equal(hasExemptLabel(['confirmed-bug']), true); + }); + + test('returns true for needs-maintainer-review', () => { + assert.equal(hasExemptLabel(['needs-maintainer-review']), true); + }); + + test('returns false for non-exempt string label', () => { + assert.equal(hasExemptLabel(['bug']), false); + }); + + test('returns false for empty array', () => { + assert.equal(hasExemptLabel([]), false); + }); + + test('returns false for non-exempt object label', () => { + assert.equal(hasExemptLabel([{ name: 'enhancement' }]), false); + }); + + test('returns true when one of multiple labels is exempt', () => { + assert.equal(hasExemptLabel(['bug', 'priority: critical', 'enhancement']), true); + }); + + test('returns true for mixed string and object labels', () => { + assert.equal(hasExemptLabel(['bug', { name: 'pinned' }]), true); + }); +}); + +// --------------------------------------------------------------------------- +// shouldClose +// --------------------------------------------------------------------------- + +describe('shouldClose', () => { + const BASE_NOW = new Date('2026-01-01T12:00:00Z'); + const CHALLENGE_CREATED_RECENT = new Date('2026-01-01T11:30:00Z'); // 30 min ago + const CHALLENGE_CREATED_OLD = new Date('2026-01-01T09:00:00Z'); // 3 hours ago, >DEFAULT + const CHALLENGE_CREATED_25H = new Date('2025-12-31T11:00:00Z'); // 25 hours ago + + const baseChallenge = { + createdAt: CHALLENGE_CREATED_OLD, + downvoted: false, + }; + + test('exempt-label short-circuits even when overdue', () => { + const result = shouldClose({ + now: BASE_NOW, + labels: ['priority: critical'], + challengeComment: { ...baseChallenge, createdAt: CHALLENGE_CREATED_25H }, + laterUserComments: 0, + }); + assert.equal(result.close, false); + assert.equal(result.reason, 'exempt-label'); + }); + + test('no challenge comment returns no-challenge-comment', () => { + const result = shouldClose({ + now: BASE_NOW, + labels: [], + challengeComment: null, + laterUserComments: 0, + }); + assert.equal(result.close, false); + assert.equal(result.reason, 'no-challenge-comment'); + }); + + test('no challenge comment with undefined challengeComment returns no-challenge-comment', () => { + const result = shouldClose({ + now: BASE_NOW, + labels: [], + laterUserComments: 0, + }); + assert.equal(result.close, false); + assert.equal(result.reason, 'no-challenge-comment'); + }); + + test('downvoted challenge returns vetoed', () => { + const result = shouldClose({ + now: BASE_NOW, + labels: [], + challengeComment: { ...baseChallenge, createdAt: CHALLENGE_CREATED_25H, downvoted: true }, + laterUserComments: 0, + }); + assert.equal(result.close, false); + assert.equal(result.reason, 'vetoed'); + }); + + test('laterUserComments > 0 returns reporter-responded', () => { + const result = shouldClose({ + now: BASE_NOW, + labels: [], + challengeComment: { ...baseChallenge, createdAt: CHALLENGE_CREATED_25H }, + laterUserComments: 1, + }); + assert.equal(result.close, false); + assert.equal(result.reason, 'reporter-responded'); + }); + + test('within window (1h age, 24h window) returns within-window', () => { + const result = shouldClose({ + now: BASE_NOW, + labels: [], + challengeComment: { ...baseChallenge, createdAt: CHALLENGE_CREATED_RECENT }, + laterUserComments: 0, + }); + assert.equal(result.close, false); + assert.equal(result.reason, 'within-window'); + }); + + test('age > windowHours, no reply, not vetoed, not exempt -> close:true duplicate-no-response', () => { + const result = shouldClose({ + now: BASE_NOW, + labels: [], + challengeComment: { ...baseChallenge, createdAt: CHALLENGE_CREATED_25H }, + laterUserComments: 0, + }); + assert.equal(result.close, true); + assert.equal(result.reason, 'duplicate-no-response'); + }); + + test('createdAt as ISO string works', () => { + const result = shouldClose({ + now: BASE_NOW, + labels: [], + challengeComment: { ...baseChallenge, createdAt: CHALLENGE_CREATED_25H.toISOString() }, + laterUserComments: 0, + }); + assert.equal(result.close, true); + assert.equal(result.reason, 'duplicate-no-response'); + }); + + test('createdAt as Date object works', () => { + const result = shouldClose({ + now: BASE_NOW, + labels: [], + challengeComment: { ...baseChallenge, createdAt: CHALLENGE_CREATED_25H }, + laterUserComments: 0, + }); + assert.equal(result.close, true); + assert.equal(result.reason, 'duplicate-no-response'); + }); + + test('createdAt as ms-number works', () => { + const result = shouldClose({ + now: BASE_NOW, + labels: [], + challengeComment: { ...baseChallenge, createdAt: CHALLENGE_CREATED_25H.getTime() }, + laterUserComments: 0, + }); + assert.equal(result.close, true); + assert.equal(result.reason, 'duplicate-no-response'); + }); + + test('now as ms-number works', () => { + const result = shouldClose({ + now: BASE_NOW.getTime(), + labels: [], + challengeComment: { ...baseChallenge, createdAt: CHALLENGE_CREATED_25H }, + laterUserComments: 0, + }); + assert.equal(result.close, true); + assert.equal(result.reason, 'duplicate-no-response'); + }); + + test('custom windowHours is respected', () => { + // Challenge created 3 hours ago; with windowHours=1 it should be overdue + const threeHoursAgo = new Date(BASE_NOW.getTime() - 3 * 3600000); + const result = shouldClose({ + now: BASE_NOW, + labels: [], + challengeComment: { ...baseChallenge, createdAt: threeHoursAgo }, + laterUserComments: 0, + windowHours: 1, + }); + assert.equal(result.close, true); + assert.equal(result.reason, 'duplicate-no-response'); + }); + + test('exempt check still short-circuits when within window', () => { + const result = shouldClose({ + now: BASE_NOW, + labels: ['confirmed'], + challengeComment: { ...baseChallenge, createdAt: CHALLENGE_CREATED_RECENT }, + laterUserComments: 0, + }); + assert.equal(result.close, false); + assert.equal(result.reason, 'exempt-label'); + }); + + test('createdAt as unparseable string returns invalid-timestamp (fail-safe)', () => { + const result = shouldClose({ + now: BASE_NOW, + labels: [], + challengeComment: { ...baseChallenge, createdAt: 'not-a-date' }, + laterUserComments: 0, + }); + assert.equal(result.close, false); + assert.equal(result.reason, 'invalid-timestamp'); + }); + + test('now as NaN returns invalid-timestamp (fail-safe)', () => { + const result = shouldClose({ + now: NaN, + labels: [], + challengeComment: { ...baseChallenge, createdAt: CHALLENGE_CREATED_25H }, + laterUserComments: 0, + }); + assert.equal(result.close, false); + assert.equal(result.reason, 'invalid-timestamp'); + }); +}); From 1b6bd66f2c00aaccbf6c6f8bf7b50fe7a7f76588 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Sun, 7 Jun 2026 20:36:11 -0400 Subject: [PATCH 7/8] feat(#770): register Claude Code lifecycle hooks (SubagentStop/Stop/PreCompact/FileChanged) (#821) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(#770): register Claude Code lifecycle hooks (SubagentStop/Stop/PreCompact/FileChanged) Wire three new context-tracking events (SubagentStop, Stop, PreCompact) to gsd-context-monitor so context-headroom warnings surface at model-stop and subagent-finalisation moments — not just on PostToolUse. Add a new FileChanged hook (gsd-config-reload.js) that hot-reloads .planning/config.json context mid-session when the user edits it, injecting a config summary as hookSpecificOutput.additionalContext. Updates plugin manifest hooks.json, managed-hooks-registry, installer-migration-report allowlist, and shell-command-projection cleanup tables. Tests: 21 new assertions in enh-770-claude-hook-events.test.cjs; enh-788 and issue-766 test suites updated. Closes #770 Co-Authored-By: Claude Opus 4.8 * docs(#770): document newly-registered Claude Code lifecycle hooks Add a Hook coverage table to the Claude Code npm installer section of docs/how-to/install-on-your-runtime.md describing SubagentStop, Stop, PreCompact, and the new FileChanged (gsd-config-reload.js) hook that hot-reloads .planning/config.json mid-session. Also fixes the changeset frontmatter (adds type: Added + pr: 821) so docs-lint can consume the fragment. Co-Authored-By: Claude Opus 4.8 * fix(#770): add gsd-config-reload.js to INVENTORY.md and regenerate manifest The feat commit added hooks/gsd-config-reload.js but did not bump the Hooks count in docs/INVENTORY.md (14→15) or add the new row, and did not regenerate docs/INVENTORY-MANIFEST.json. Both inventory-counts and inventory-manifest-sync tests failed across the full CI matrix. Co-Authored-By: Claude Opus 4.8 * fix(#770): make lifecycle-hook tests deterministic on scoped runner Replace the shared hooks/dist/ ensemble setup (ensureHooksDist / teardownHooksDist) in the Claude hook tests with per-test isolation: pre-populate each test's own tmpDir/.claude/hooks/ with stub files and pass installerMigrations:[] to install() so the first-time-baseline migration does not remove the stubs before the copy step can run. Root cause: hooks/dist/ is gitignored and absent on a fresh npm ci. ensureHooksDist() created it and teardownHooksDist() deleted it, but with --test-concurrency=4 both test files ran concurrently as separate Node.js worker processes sharing the same filesystem. One file's afterEach teardown deleted hooks/dist/ while the other file's install() was copying from it, producing an ENOENT (reproduced 2/10 runs locally). The additional issue: even with pre-placed stubs surviving the copy race, the 000-first-time-baseline migration classified hooks/gsd-*.js as bundled-gsd-hook artifacts, auto-removed them, and the copy step never re-ran (hooks/dist/ absent) — leaving contextMonitorFile missing and all hook registrations silently skipped (the 'got: []' symptom). Fix: pre-populate targetDir/hooks/ per-test (isolated temp dir) AND pass installerMigrations:[] so the baseline scan is skipped. The Qwen suites already used this pattern correctly; the Claude suites are aligned to it. Co-Authored-By: Claude Opus 4.8 * fix(#770): ship gsd-config-reload.js by adding it to build-hooks HOOKS_TO_COPY The #770 feature added hooks/gsd-config-reload.js and registered it in MANAGED_HOOKS, the installer, INVENTORY, and the test EXPECTED_ALL_HOOKS list — but never added it to scripts/build-hooks.js HOOKS_TO_COPY. As a result the hook was never copied into hooks/dist/ during the build, so: - the hook would never ship to users (real production bug — the FileChanged config-reload feature was dead-on-arrival), and - install-minimal-hooks.test.cjs #1755 ("all expected hooks are copied from hooks/dist/ to target", ".js hooks are executable after copy", "manifest contains .js hook entries") failed on any environment with a clean checkout (no pre-existing hooks/dist/): coverage, full test macos-22/macos-24, test ubuntu-24. The failures were masked locally only by a stale hooks/dist/ left from a prior build (build-hooks copies into dist without clearing it). On CI's fresh `npm ci` there is no dist, so the omission surfaced. Fix: add 'gsd-config-reload.js' to HOOKS_TO_COPY so build-hooks stages it into hooks/dist/ alongside the other JS hooks. Verified by removing hooks/dist/ and rerunning the full suite green (0 fail). Co-Authored-By: Claude Opus 4.8 * fix(#770): make config prototype-pollution beforeEach deterministic on scoped runner Root cause: the #663 and alert-#26 prototype-pollution describe blocks seeded .planning/config.json in beforeEach via a bare runGsdTools('config-ensure-section') whose result was discarded. That command runs in a spawned gsd-tools child; on the scoped CI lane (--test-concurrency=4, config.test.cjs scheduled alongside the heavy install/tarball suites that #770 pulled into the targeted set) the child can be transiently killed under resource pressure (non-zero exit, empty stderr — an OS-level kill, not an app error). The swallowed failure left config.json absent, so the first subtest's readConfig() threw ENOENT opening /.planning/config.json. Only 1 of 4 subtests failed, confirming a per-invocation transient, not a deterministic miss; the full suite schedules files differently so config.test.cjs did not collide with those heavy neighbors → passed there. Fix: add ensureConfigReady(tmpDir) which retries config-ensure-section on ANY failure or missing file and throws a clear diagnostic if it still cannot create config.json, then use it in both prototype-pollution beforeEach blocks. Setup is now deterministic under load; the #663/alert-#26 security assertions are unchanged. Co-Authored-By: Claude Opus 4.8 --------- Co-authored-by: Claude Opus 4.8 --- .changeset/770-claude-lifecycle-hooks.md | 6 + CONTEXT.md | 2 +- bin/install.js | 90 +++-- docs/INVENTORY-MANIFEST.json | 1 + docs/INVENTORY.md | 3 +- docs/how-to/install-on-your-runtime.md | 16 + hooks/gsd-config-reload.js | 133 ++++++++ hooks/hooks.json | 29 ++ hooks/managed-hooks-registry.cjs | 1 + scripts/build-hooks.js | 4 + src/installer-migration-report.cts | 1 + src/shell-command-projection.cts | 2 + tests/config.test.cjs | 47 ++- tests/enh-770-claude-hook-events.test.cjs | 379 ++++++++++++++++++++++ tests/enh-788-qwen-hook-events.test.cjs | 36 +- tests/helpers/install-shared.cjs | 1 + tests/issue-766-plugin-manifest.test.cjs | 10 +- 17 files changed, 715 insertions(+), 46 deletions(-) create mode 100644 .changeset/770-claude-lifecycle-hooks.md create mode 100644 hooks/gsd-config-reload.js create mode 100644 tests/enh-770-claude-hook-events.test.cjs diff --git a/.changeset/770-claude-lifecycle-hooks.md b/.changeset/770-claude-lifecycle-hooks.md new file mode 100644 index 000000000..c69857eba --- /dev/null +++ b/.changeset/770-claude-lifecycle-hooks.md @@ -0,0 +1,6 @@ +--- +type: Added +pr: 821 +--- + +Added: register newly-available Claude Code lifecycle hooks — SubagentStop, Stop, PreCompact (all wired to gsd-context-monitor for context-headroom warnings), and FileChanged (matcher: `config.json`, wired to new gsd-config-reload.js hook that hot-reloads `.planning/config.json` context mid-session). Also updates hooks/hooks.json (plugin manifest) and managed-hooks-registry for drift-guard coverage (#770). diff --git a/CONTEXT.md b/CONTEXT.md index b6641f085..871b64625 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -125,7 +125,7 @@ Projects a pure, typed install plan for a given runtime by composing artifact pl Module owning the explicit per-runtime config-mutation dispatch table for the installer. `resolveRuntimeConfigIntent(runtime)` projects a typed config intent — `installSurface` (`settings-json` | `codex-toml` | `copilot-instructions` | `cline-rules` | `cursor-hooks-json` | `profile-marker-only`), `writesSharedSettings` (the `finishInstall` shared-settings write gate), and `finishPermissionWriter` (`opencode` | `kilo` | none) — that `bin/install.js` dispatches on instead of inline `runtime === '...'` branching. Owns adapter selection only: it performs no filesystem IO and does not execute config mutations (the install/finishInstall handlers and the per-runtime writers do that). Unknown runtimes fail loudly with a `TypeError`, guarded by an `Object.hasOwn` own-property check so prototype-chain keys (`__proto__`, `constructor`) also throw. Realizes the adapter-selection half of the Runtime Install Policy Module boundary. Source: `gsd-core/bin/lib/runtime-config-adapter-registry.cjs`. See ADR-58, #60. ### Claude Code Plugin Manifest Module -Module owning the projection of gsd-core's artifact surfaces (`commands`, `agents`, hooks) onto the Claude Code plugin contract (`.claude-plugin/plugin.json` + `hooks/hooks.json`) — the plugin-contract sibling of the Runtime Artifact Layout Module (which projects the same surfaces onto filesystem placements). Defined mapping: `name`=`binName` (drives the `/gsd-core:` command namespace), `repository`/`homepage`=`repoUrl` (Package Identity Module), `version`/`description`/`license` from `package.json` (`version` is required for `claude plugin validate --strict`), `commands`=`./commands/gsd/`, agents via Claude Code's default `agents/` discovery (the explicit string form is schema-rejected), `hooks`=`./hooks/hooks.json`. The hook projection carries ONLY the always-on subset of the Installer Module's Claude `settings.json` wiring (check-update, context-monitor, prompt-guard, read-guard, worktree-path-guard, read-injection-scanner) via `${CLAUDE_PLUGIN_ROOT}`; config-gated opt-in hooks are excluded because a static manifest cannot honor per-project config gates, and plugin-shipped agents cannot carry hook frontmatter (so all plugin-path hook wiring lives in hooks.json). Additive — the file-copy path (Runtime Artifact Layout / Install Policy / Installer Modules) is unchanged. Conformance is validated by `claude plugin validate --strict` plus the in-repo drift-guard `tests/issue-766-plugin-manifest.test.cjs`. _Avoid_: "the plugin API", "the plugin file" (when you mean the seam). See ADR-766 and Runtime Artifact Layout Module. +Module owning the projection of gsd-core's artifact surfaces (`commands`, `agents`, hooks) onto the Claude Code plugin contract (`.claude-plugin/plugin.json` + `hooks/hooks.json`) — the plugin-contract sibling of the Runtime Artifact Layout Module (which projects the same surfaces onto filesystem placements). Defined mapping: `name`=`binName` (drives the `/gsd-core:` command namespace), `repository`/`homepage`=`repoUrl` (Package Identity Module), `version`/`description`/`license` from `package.json` (`version` is required for `claude plugin validate --strict`), `commands`=`./commands/gsd/`, agents via Claude Code's default `agents/` discovery (the explicit string form is schema-rejected), `hooks`=`./hooks/hooks.json`. The hook projection carries ONLY the always-on subset of the Installer Module's Claude `settings.json` wiring (check-update, context-monitor, prompt-guard, read-guard, worktree-path-guard, read-injection-scanner) via `${CLAUDE_PLUGIN_ROOT}`; config-gated opt-in hooks are excluded because a static manifest cannot honor per-project config gates, and plugin-shipped agents cannot carry hook frontmatter (so all plugin-path hook wiring lives in hooks.json). `hooks.json` covers all seven Claude Code lifecycle events: SessionStart, PreToolUse, PostToolUse, SubagentStop, Stop, PreCompact (all wired to context-monitor for context-headroom awareness), and FileChanged (matcher: `config.json` → config-reload, injects `additionalContext` when `.planning/config.json` changes mid-session). Additive — the file-copy path (Runtime Artifact Layout / Install Policy / Installer Modules) is unchanged. Conformance is validated by `claude plugin validate --strict` plus the in-repo drift-guard `tests/issue-766-plugin-manifest.test.cjs`. _Avoid_: "the plugin API", "the plugin file" (when you mean the seam). See ADR-766 and Runtime Artifact Layout Module. ### Gemini Extension Package The repo-root `gemini-extension.json` + `GEMINI.md` pair that projects gsd-core onto the Gemini CLI extension contract, enabling one-step lifecycle management via `gemini extensions install ` / `update` / `remove` (and `gemini extensions link ` for dev). The Gemini-CLI sibling of the Claude Code Plugin Manifest Module — same additive idea, different runtime package format. Defined mapping: `name`=`binName` (`gsd-core`; lowercase-dashes per Gemini's extension naming rule), `version` tracks `package.json` (Gemini's `gemini extensions update` keys off the manifest `version` field), `description` (required by the manifest schema), `contextFileName`=`GEMINI.md` (the extension's context payload, loaded into every Gemini session). Intentionally minimal: no `mcpServers` (gsd-core ships no MCP server). Slash-command / agent / hook projection into the extension (which would require committing the Gemini-format TOML/agent conversions the Installer Module produces at `--gemini` install time) is deferred — the manual `npx gsd-core --gemini` path remains the way to install the `/gsd:*` commands, and is unchanged (additive, no breaking change). Conformance is guarded by the in-repo drift test `tests/issue-775-gemini-extension.test.cjs` (manifest validity, `version`↔`package.json` parity, `contextFileName` existence, `files[]` publication). _Avoid_: "the Gemini plugin" (Gemini calls them extensions, not plugins). See #775, ADR-766, Claude Code Plugin Manifest Module, and Runtime Artifact Layout Module. diff --git a/bin/install.js b/bin/install.js index a3c0408eb..0ae9bdcf6 100755 --- a/bin/install.js +++ b/bin/install.js @@ -8058,6 +8058,7 @@ const GSD_UNINSTALL_HOOKS = [ 'gsd-statusline.js', 'gsd-check-update.js', 'gsd-check-update.cmd', + 'gsd-config-reload.js', 'gsd-context-monitor.js', 'gsd-cursor-session-start.js', 'gsd-cursor-post-tool.js', @@ -8523,10 +8524,12 @@ function uninstall(isGlobal, runtime = 'claude') { // Remove GSD hooks from settings — per-hook granularity to preserve // user hooks that share an entry with a GSD hook (#1755 followup). // Includes the 3 Qwen-only events added in #788 (SubagentStop, Stop, - // PreCompact) and the 3 Gemini-only events added in #776 (BeforeAgent, - // AfterAgent, BeforeModel) — safe to iterate for all runtimes; non-Qwen - // and non-Gemini installs simply find no entries and skip. - for (const eventName of ['SessionStart', 'PostToolUse', 'AfterTool', 'PreToolUse', 'BeforeTool', 'SubagentStop', 'Stop', 'PreCompact', 'BeforeAgent', 'AfterAgent', 'BeforeModel']) { + // PreCompact, also registered for Claude in #770), the 3 Gemini-only + // events added in #776 (BeforeAgent, AfterAgent, BeforeModel), and the + // Claude-only FileChanged event added in #770 — safe to iterate for all + // runtimes; installs that don't register these events simply find no + // entries and skip. + for (const eventName of ['SessionStart', 'PostToolUse', 'AfterTool', 'PreToolUse', 'BeforeTool', 'SubagentStop', 'Stop', 'PreCompact', 'BeforeAgent', 'AfterAgent', 'BeforeModel', 'FileChanged']) { if (settings.hooks && settings.hooks[eventName]) { const before = JSON.stringify(settings.hooks[eventName]); settings.hooks[eventName] = settings.hooks[eventName] @@ -11118,6 +11121,9 @@ function install(isGlobal, runtime = 'claude', options = {}) { const readInjectionScannerCommand = isGlobal ? buildHookCommand(targetDir, 'gsd-read-injection-scanner.js', hookOpts) : localCmd('gsd-read-injection-scanner.js'); + const configReloadCommand = isGlobal + ? buildHookCommand(targetDir, 'gsd-config-reload.js', hookOpts) + : localCmd('gsd-config-reload.js'); // #3002 CR: when resolveNodeRunner() returns null, every dependent JS-hook // command is null too. Emit one warning here so the operator sees the cause @@ -11472,36 +11478,33 @@ function install(isGlobal, runtime = 'claude', options = {}) { console.warn(` ${yellow}⚠${reset} Skipped phase boundary hook — Bash executable path unavailable (#3393)`); } - // ── Qwen-only extended hook events (#788) ──────────────────────────────── - // Qwen Code exposes 15 hook events — a superset of Claude Code. Three - // additional events are registered for Qwen installs: + // ── Extended hook events: SubagentStop / Stop / PreCompact (#788 + #770) ── + // Claude Code (since #770) and Qwen Code (since #788) both support these + // three lifecycle events. Wire gsd-context-monitor so agents get context- + // headroom warnings at subagent completion, model stop, and pre-compaction + // (the most critical moment to surface headroom info). + // // SubagentStop — subagent lifecycle completion (context headroom tracking) // Stop — model stop / final-response moment (context headroom) // PreCompact — fires before conversation compaction (most critical // moment to surface context headroom warnings) // - // Wire gsd-context-monitor to all three — the same hook already used for - // PostToolUse — so no new hook files are needed. - // // Note: UserPromptSubmit is NOT wired here. That event carries the raw // user prompt text, not a tool invocation, so gsd-prompt-guard (which // exits unless tool_name is Write/Edit) would be a silent no-op. A // dedicated handler for UserPromptSubmit is deferred to a follow-on issue. - // - // Guard: isQwen is defined at the top of install() (line ~8254). - if (isQwen) { - // SubagentStop, Stop, PreCompact — route through the context monitor so - // agents get context-headroom warnings at subagent completion, model stop, - // and pre-compaction (the most critical moment to surface headroom info). - for (const qwenEvent of ['SubagentStop', 'Stop', 'PreCompact']) { - if (!settings.hooks[qwenEvent]) { - settings.hooks[qwenEvent] = []; + if (isQwen || runtime === 'claude') { + const runtimeLabel = isQwen ? 'Qwen Code' : 'Claude Code'; + // SubagentStop, Stop, PreCompact — route through the context monitor. + for (const event of ['SubagentStop', 'Stop', 'PreCompact']) { + if (!settings.hooks[event]) { + settings.hooks[event] = []; } - const alreadyHasContextMonitor = settings.hooks[qwenEvent].some(entry => + const alreadyHasContextMonitor = settings.hooks[event].some(entry => entry.hooks && entry.hooks.some(h => h.command && h.command.includes('gsd-context-monitor')) ); if (!alreadyHasContextMonitor && fs.existsSync(contextMonitorFile) && contextMonitorCommand) { - settings.hooks[qwenEvent].push({ + settings.hooks[event].push({ hooks: [ { type: 'command', @@ -11510,13 +11513,13 @@ function install(isGlobal, runtime = 'claude', options = {}) { } ] }); - console.log(` ${green}✓${reset} Configured ${qwenEvent} context monitor hook (Qwen Code)`); + console.log(` ${green}✓${reset} Configured ${event} context monitor hook (${runtimeLabel})`); } else if (!alreadyHasContextMonitor && !fs.existsSync(contextMonitorFile)) { - console.warn(` ${yellow}⚠${reset} Skipped ${qwenEvent} hook — gsd-context-monitor.js not found at target`); + console.warn(` ${yellow}⚠${reset} Skipped ${event} hook — gsd-context-monitor.js not found at target`); } } } - // ── end Qwen-only extended hook events ──────────────────────────────────── + // ── end SubagentStop / Stop / PreCompact events ──────────────────────────── // ── Gemini-only extended hook events (#776) ─────────────────────────────── // Gemini CLI exposes several hook events beyond BeforeTool/AfterTool that @@ -11566,6 +11569,45 @@ function install(isGlobal, runtime = 'claude', options = {}) { } } // ── end Gemini-only extended hook events ────────────────────────────────── + + // ── FileChanged hook: hot-reload gsd config on .planning/config.json edits ─ + // Claude Code fires FileChanged when a watched file changes on disk. Wire + // gsd-config-reload.js to reload the gsd config context whenever the user + // edits .planning/config.json mid-session, eliminating the need to restart. + // + // The matcher "config.json" watches for changes to any file named config.json + // (Claude Code matches by filename, not full path). The hook exits silently + // when the changed file is not the gsd config. + // + // Scoped to Claude Code only: Qwen Code's FileChanged support is not yet + // verified; extend in a follow-on if empirically confirmed. + if (runtime === 'claude') { + if (!settings.hooks.FileChanged) { + settings.hooks.FileChanged = []; + } + const configReloadFile = path.join(targetDir, 'hooks', 'gsd-config-reload.js'); + const alreadyHasConfigReload = settings.hooks.FileChanged.some(entry => + entry.hooks && entry.hooks.some(h => h.command && h.command.includes('gsd-config-reload')) + ); + if (!alreadyHasConfigReload && fs.existsSync(configReloadFile) && configReloadCommand) { + settings.hooks.FileChanged.push({ + matcher: 'config.json', + hooks: [ + { + type: 'command', + command: configReloadCommand, + timeout: 8 + } + ] + }); + console.log(` ${green}✓${reset} Configured FileChanged config-reload hook (Claude Code)`); + } else if (!alreadyHasConfigReload && !fs.existsSync(configReloadFile)) { + console.warn(` ${yellow}⚠${reset} Skipped FileChanged hook — gsd-config-reload.js not found at target`); + } else if (!alreadyHasConfigReload && !configReloadCommand) { + console.warn(` ${yellow}⚠${reset} Skipped FileChanged hook — Node executable path unavailable`); + } + } + // ── end FileChanged hook ──────────────────────────────────────────────────── } // ── Gemini hooksConfig.enabled check (#776) ─────────────────────────────── diff --git a/docs/INVENTORY-MANIFEST.json b/docs/INVENTORY-MANIFEST.json index d3def7871..5d7854d75 100644 --- a/docs/INVENTORY-MANIFEST.json +++ b/docs/INVENTORY-MANIFEST.json @@ -359,6 +359,7 @@ "hooks": [ "gsd-check-update-worker.js", "gsd-check-update.js", + "gsd-config-reload.js", "gsd-context-monitor.js", "gsd-cursor-post-tool.js", "gsd-cursor-session-start.js", diff --git a/docs/INVENTORY.md b/docs/INVENTORY.md index d7bc3964b..ebce42500 100644 --- a/docs/INVENTORY.md +++ b/docs/INVENTORY.md @@ -471,7 +471,7 @@ Full listing: `gsd-core/bin/lib/*.cjs`. --- -## Hooks (16 shipped) +## Hooks (17 shipped) Full listing: `hooks/`. @@ -489,6 +489,7 @@ Full listing: `hooks/`. | `gsd-read-guard.js` | `PreToolUse` | Advisory guard preventing Edit/Write on unread files | | `gsd-read-injection-scanner.js` | `PostToolUse` | Scans tool Read results for prompt-injection patterns (v1.36+, PR #2201) | | `gsd-worktree-path-guard.js` | `PreToolUse` | Hard-blocks Edit/Write/MultiEdit with absolute paths outside the worktree root (PR #579, #260) | +| `gsd-config-reload.js` | `FileChanged` | Hot-reloads GSD config context when `.planning/config.json` changes mid-session (#770) | | `gsd-session-state.sh` | `PostToolUse` | Session-state tracking for shell-based runtimes | | `gsd-validate-commit.sh` | `PostToolUse` | Commit validation for conventional-commit enforcement | | `gsd-phase-boundary.sh` | `PostToolUse` | Phase-boundary detection for workflow transitions | diff --git a/docs/how-to/install-on-your-runtime.md b/docs/how-to/install-on-your-runtime.md index 595d45f0b..b2f53f5d5 100644 --- a/docs/how-to/install-on-your-runtime.md +++ b/docs/how-to/install-on-your-runtime.md @@ -42,6 +42,22 @@ Skills land in `~/.claude/`. Commands appear as `/gsd-*` slash commands in your CLAUDE_CONFIG_DIR=~/.claude-alt npx @opengsd/gsd-core@latest --claude --global ``` +**Hook coverage** + +GSD registers the following Claude Code hook events automatically on install: + +| Event | Hook | Purpose | +|---|---|---| +| `SessionStart` | `gsd-check-update.js`, `gsd-session-state.sh` | Update check, session orientation | +| `PostToolUse` | `gsd-context-monitor.js`, `gsd-read-injection-scanner.js`, `gsd-phase-boundary.sh`, `gsd-graphify-update.sh` | Context monitoring, read-time scan, phase boundary detection | +| `PreToolUse` | `gsd-prompt-guard.js`, `gsd-read-guard.js`, `gsd-workflow-guard.js`, `gsd-worktree-path-guard.js`, `gsd-validate-commit.sh` | Prompt guard, read-before-edit, workflow + worktree safety, commit validation | +| `SubagentStop` | `gsd-context-monitor.js` | Context headroom tracking after subagent completion | +| `Stop` | `gsd-context-monitor.js` | Context headroom tracking before model stop | +| `PreCompact` | `gsd-context-monitor.js` | Context awareness before conversation compaction | +| `FileChanged` (matcher: `config.json`) | `gsd-config-reload.js` | Hot-reloads `.planning/config.json` context mid-session when you edit your GSD config — no session restart required | + +The `FileChanged` hook is always-on and a no-op when `.planning/config.json` does not exist in the project. Editing that file while a session is running injects an `additionalContext` summary of the new configuration so the agent picks up model overrides, workflow toggles, and hook settings immediately. + --- ### Claude Code — native plugin install diff --git a/hooks/gsd-config-reload.js b/hooks/gsd-config-reload.js new file mode 100644 index 000000000..184bebf73 --- /dev/null +++ b/hooks/gsd-config-reload.js @@ -0,0 +1,133 @@ +#!/usr/bin/env node +// gsd-hook-version: {{GSD_VERSION}} +// gsd-config-reload.js — FileChanged hook: hot-reload GSD config context +// Fires when .planning/config.json is modified, created, or deleted. +// +// When the user edits .planning/config.json mid-session, this hook reads the +// updated config and injects a summary as additionalContext so the agent knows +// the new configuration without requiring a session restart. +// +// Input (from Claude Code): +// { session_id, cwd, hook_event_name: "FileChanged", +// file_path: "/abs/path/.planning/config.json", event: "change"|"add"|"unlink" } +// +// Output: +// { hookSpecificOutput: { hookEventName: "FileChanged", additionalContext: "..." } } +// or exits 0 silently (if config absent, unreadable, or event is "unlink"). +// +// Enabled for all Claude Code installs. This hook is always-on — it is a +// no-op when .planning/config.json is absent (ENOENT → exit 0). + +const fs = require('fs'); +const path = require('path'); + +let input = ''; +// Timeout guard: if stdin does not close within 8s exit silently rather than +// hanging until Claude Code kills the process and reports "hook error". +const stdinTimeout = setTimeout(() => process.exit(0), 8000); +process.stdin.setEncoding('utf8'); +process.stdin.on('data', chunk => (input += chunk)); +process.stdin.on('end', () => { + clearTimeout(stdinTimeout); + try { + const data = JSON.parse(input); + const event = data.event; // "change" | "add" | "unlink" + const filePath = data.file_path || ''; + const cwd = data.cwd || process.cwd(); + + // Only handle the GSD planning config — verify both basename and that the + // resolved path is .planning/config.json relative to cwd. The hook + // matcher ('config.json') fires on any watched config.json; this guard + // ensures an unrelated config.json in node_modules/ or elsewhere does not + // inject spurious additionalContext. + const basename = path.basename(filePath); + if (basename !== 'config.json') { + process.exit(0); + } + const expectedPath = path.resolve(cwd, '.planning', 'config.json'); + if (path.resolve(filePath) !== expectedPath) { + process.exit(0); + } + + // On unlink (deletion) emit a brief notice and exit + if (event === 'unlink') { + process.stdout.write(JSON.stringify({ + hookSpecificOutput: { + hookEventName: 'FileChanged', + additionalContext: + 'GSD config (.planning/config.json) was deleted. ' + + 'Falling back to built-in defaults for this session.', + }, + })); + process.exit(0); + } + + // Read the updated config file + let config; + try { + const raw = fs.readFileSync(filePath, 'utf8'); + config = JSON.parse(raw); + } catch (e) { + if (e && e.code === 'ENOENT') process.exit(0); + // Malformed JSON — inform the agent without crashing + process.stdout.write(JSON.stringify({ + hookSpecificOutput: { + hookEventName: 'FileChanged', + additionalContext: + 'GSD config (.planning/config.json) was modified but could not be parsed. ' + + 'Check the file for JSON syntax errors.', + }, + })); + process.exit(0); + } + + // Build a concise summary of key config fields the agent cares about + const lines = ['GSD config reloaded (.planning/config.json updated):']; + + if (config.runtime) lines.push(` runtime: ${config.runtime}`); + if (config.mode) lines.push(` mode: ${config.mode}`); + + // hooks section (opt-in toggles agents act on) + if (config.hooks && typeof config.hooks === 'object') { + const hookKeys = Object.entries(config.hooks) + .filter(([, v]) => v !== undefined) + .map(([k, v]) => `${k}=${v}`) + .join(', '); + if (hookKeys) lines.push(` hooks: { ${hookKeys} }`); + } + + // workflow section (key toggles) + if (config.workflow && typeof config.workflow === 'object') { + const wfKeys = Object.entries(config.workflow) + .filter(([, v]) => v !== undefined) + .map(([k, v]) => `${k}=${v}`) + .join(', '); + if (wfKeys) lines.push(` workflow: { ${wfKeys} }`); + } + + // model overrides (agents use these) + if (config.models && typeof config.models === 'object') { + const modelKeys = Object.entries(config.models) + .filter(([, v]) => v !== undefined) + .map(([k, v]) => `${k}=${v}`) + .join(', '); + if (modelKeys) lines.push(` models: { ${modelKeys} }`); + } + + if (lines.length === 1) { + // No notable fields — still confirm the reload happened + lines.push(' (no notable keys changed)'); + } + + const additionalContext = lines.join('\n'); + process.stdout.write(JSON.stringify({ + hookSpecificOutput: { + hookEventName: 'FileChanged', + additionalContext, + }, + })); + } catch (e) { + // Silent fail — never block the session on a config reload error + process.exit(0); + } +}); diff --git a/hooks/hooks.json b/hooks/hooks.json index 9d99579dc..09c8dc0aa 100644 --- a/hooks/hooks.json +++ b/hooks/hooks.json @@ -35,6 +35,35 @@ { "type": "command", "command": "node \"${CLAUDE_PLUGIN_ROOT}/hooks/gsd-read-injection-scanner.js\"", "timeout": 5 } ] } + ], + "SubagentStop": [ + { + "hooks": [ + { "type": "command", "command": "node \"${CLAUDE_PLUGIN_ROOT}/hooks/gsd-context-monitor.js\"", "timeout": 10 } + ] + } + ], + "Stop": [ + { + "hooks": [ + { "type": "command", "command": "node \"${CLAUDE_PLUGIN_ROOT}/hooks/gsd-context-monitor.js\"", "timeout": 10 } + ] + } + ], + "PreCompact": [ + { + "hooks": [ + { "type": "command", "command": "node \"${CLAUDE_PLUGIN_ROOT}/hooks/gsd-context-monitor.js\"", "timeout": 10 } + ] + } + ], + "FileChanged": [ + { + "matcher": "config.json", + "hooks": [ + { "type": "command", "command": "node \"${CLAUDE_PLUGIN_ROOT}/hooks/gsd-config-reload.js\"", "timeout": 8 } + ] + } ] } } diff --git a/hooks/managed-hooks-registry.cjs b/hooks/managed-hooks-registry.cjs index 77aa4982b..12715ed38 100644 --- a/hooks/managed-hooks-registry.cjs +++ b/hooks/managed-hooks-registry.cjs @@ -18,6 +18,7 @@ const MANAGED_HOOKS = [ 'gsd-check-update-worker.js', 'gsd-check-update.js', + 'gsd-config-reload.js', 'gsd-context-monitor.js', 'gsd-cursor-post-tool.js', 'gsd-cursor-session-start.js', diff --git a/scripts/build-hooks.js b/scripts/build-hooks.js index c566a18cd..121a73afb 100644 --- a/scripts/build-hooks.js +++ b/scripts/build-hooks.js @@ -33,6 +33,10 @@ const HOOKS_TO_COPY = [ // Cursor lifecycle hooks (issue #777): sessionStart context injection + postToolUse monitor 'gsd-cursor-session-start.js', 'gsd-cursor-post-tool.js', + // Claude Code FileChanged hook (#770) — hot-reloads gsd config when + // .planning/config.json changes mid-session. Must ship to dist so the + // installer can copy it to the target hooks/ dir and register FileChanged. + 'gsd-config-reload.js', 'gsd-prompt-guard.js', 'gsd-read-guard.js', 'gsd-read-injection-scanner.js', diff --git a/src/installer-migration-report.cts b/src/installer-migration-report.cts index 3e41512fa..81e5b8d59 100644 --- a/src/installer-migration-report.cts +++ b/src/installer-migration-report.cts @@ -29,6 +29,7 @@ const VALID_CHOICES: ReadonlyArray = ['keep', 'remove']; export const BUNDLED_GSD_HOOK_FILES: ReadonlySet = Object.freeze(new Set([ 'hooks/gsd-check-update-worker.js', 'hooks/gsd-check-update.js', + 'hooks/gsd-config-reload.js', 'hooks/gsd-context-monitor.js', 'hooks/gsd-cursor-post-tool.js', 'hooks/gsd-cursor-session-start.js', diff --git a/src/shell-command-projection.cts b/src/shell-command-projection.cts index 309872381..4de9d9639 100644 --- a/src/shell-command-projection.cts +++ b/src/shell-command-projection.cts @@ -145,6 +145,7 @@ export function projectManagedHookCommand({ absoluteRunner, scriptPath, runtime const MANAGED_HOOK_BASENAMES_BY_SURFACE: Record> = { 'settings-json': new Set([ 'gsd-check-update.js', + 'gsd-config-reload.js', 'gsd-statusline.js', 'gsd-context-monitor.js', 'gsd-prompt-guard.js', @@ -161,6 +162,7 @@ const MANAGED_HOOK_BASENAMES_BY_SURFACE: Record> = { const MANAGED_HOOK_COMMAND_BASENAMES_BY_SURFACE: Record> = { 'settings-json': new Set([ 'gsd-check-update.js', + 'gsd-config-reload.js', 'gsd-statusline.js', 'gsd-context-monitor.js', 'gsd-prompt-guard.js', diff --git a/tests/config.test.cjs b/tests/config.test.cjs index 1180f5222..7457767e2 100644 --- a/tests/config.test.cjs +++ b/tests/config.test.cjs @@ -39,6 +39,37 @@ async function runConfigEnsureSectionWithRetry(tmpDir, attempts = 4) { return last; } +/** + * Seed `.planning/config.json` for a test and guarantee it lands on disk + * before the test body runs. + * + * `config-ensure-section` is invoked through a spawned `gsd-tools.cjs` child. + * On the scoped CI lane (`--test-concurrency=4`, config.test.cjs scheduled + * alongside the heavy install/tarball suites) that child can be transiently + * killed under resource pressure — surfacing as a non-zero exit with empty + * stderr (an OS-level kill, not a gsd-tools application error; see the + * `runGsdTools` catch). A bare `runGsdTools('config-ensure-section')` in + * `beforeEach` swallows that failure, leaving config.json absent so the first + * subtest's `readConfig()` throws a confusing ENOENT (#770 scoped-lane flake). + * + * This retries on ANY failure or missing file (not just the EPERM/EBUSY class + * `runConfigEnsureSectionWithRetry` covers) and throws a clear diagnostic if it + * still cannot create the file, so setup is deterministic under load. + */ +async function ensureConfigReady(tmpDir, attempts = 5) { + const configPath = path.join(tmpDir, '.planning', 'config.json'); + let last; + for (let i = 0; i < attempts; i += 1) { + last = runGsdTools('config-ensure-section', tmpDir); + if (last.success && fs.existsSync(configPath)) return last; + if (i < attempts - 1) await delay(150 * (i + 1)); + } + throw new Error( + `config-ensure-section failed to create ${configPath} after ${attempts} attempts: ` + + `${(last && last.error) || 'unknown error'}`, + ); +} + // ─── config-ensure-section ─────────────────────────────────────────────────── describe('config-ensure-section command', () => { @@ -1106,10 +1137,12 @@ describe('config-path command (#2282)', () => { describe('config-set prototype-pollution guard (#663)', () => { let tmpDir; - beforeEach(() => { + beforeEach(async () => { tmpDir = createTempProject(); - // Initialise config so there is a config.json to write to. - runGsdTools('config-ensure-section', tmpDir); + // Initialise config so there is a config.json to write to. Retry + assert + // so a transient config-ensure-section child failure under scoped-lane load + // cannot leave config.json absent (#770). + await ensureConfigReady(tmpDir); }); afterEach(() => { @@ -1163,10 +1196,12 @@ describe('config-set prototype-pollution guard (#663)', () => { describe('config-set prototype-pollution guard via dynamic-key prefixes (alert #26)', () => { let tmpDir; - beforeEach(() => { + beforeEach(async () => { tmpDir = createTempProject(); - // Initialise config so there is a config.json to write to. - runGsdTools('config-ensure-section', tmpDir); + // Initialise config so there is a config.json to write to. Retry + assert + // so a transient config-ensure-section child failure under scoped-lane load + // cannot leave config.json absent (#770). + await ensureConfigReady(tmpDir); }); afterEach(() => { diff --git a/tests/enh-770-claude-hook-events.test.cjs b/tests/enh-770-claude-hook-events.test.cjs new file mode 100644 index 000000000..6352bfa98 --- /dev/null +++ b/tests/enh-770-claude-hook-events.test.cjs @@ -0,0 +1,379 @@ +'use strict'; + +process.env.GSD_TEST_MODE = '1'; + +/** + * Enhancement #770: Register Claude Code lifecycle hooks (SubagentStop / Stop / + * PreCompact / FileChanged). + * + * Claude Code now supports the same SubagentStop, Stop, and PreCompact events + * that were wired for Qwen Code in #788. This suite asserts: + * + * 1. Claude Code installs register SubagentStop, Stop, and PreCompact, each + * wired to gsd-context-monitor.js (same as Qwen). + * 2. Claude Code installs register a FileChanged hook for .planning/config.json + * wired to gsd-config-reload.js (new hook; hot-reloads gsd config). + * 3. All four registrations are idempotent (reinstall does not duplicate). + * 4. Uninstall removes all four event registrations. + * 5. The gsd-config-reload.js hook script exists in hooks/ and has the + * expected structure (reads on stdin, emits additionalContext or exits 0). + * 6. The hooks/hooks.json plugin manifest includes the new events. + * + * Source: https://code.claude.com/docs/en/hooks + */ + +const { test, describe, beforeEach, afterEach } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); + +const { install, uninstall, validateHookFields } = require('../bin/install.js'); +const { createTempDir, cleanup } = require('./helpers.cjs'); + +// ─── Helpers ───────────────────────────────────────────────────────────────── + +/** Extract all hook commands registered under `eventName` from settings. */ +function hooksForEvent(settings, eventName) { + if (!settings || !settings.hooks || !Array.isArray(settings.hooks[eventName])) return []; + return settings.hooks[eventName].flatMap(entry => + (entry && Array.isArray(entry.hooks) ? entry.hooks : []) + .map(h => h && h.command) + .filter(Boolean) + ); +} + +/** Extract all matchers registered under `eventName` from settings. */ +function matchersForEvent(settings, eventName) { + if (!settings || !settings.hooks || !Array.isArray(settings.hooks[eventName])) return []; + return settings.hooks[eventName] + .map(entry => entry && entry.matcher) + .filter(Boolean); +} + +const HOOKS_SRC = path.join(__dirname, '..', 'hooks'); +// Hooks the installer existsSync-checks before registering; must be present +// in targetDir/hooks/ so the registration guards pass. +const STUB_HOOKS = [ + 'gsd-context-monitor.js', + 'gsd-prompt-guard.js', + 'gsd-check-update.js', + 'gsd-config-reload.js', +]; + +/** + * Pre-populate targetDir/hooks/ with stub hook files so the installer's + * fs.existsSync guards pass even when hooks/dist/ is absent (e.g. CI without + * a build step). Each test suite passes its own per-test tmpDir/.claude path + * so stubs are isolated to that test's temp directory — no shared filesystem + * state, no cross-test races. + * + * When hooks/dist/ DOES exist (local dev with npm run build:hooks), the + * installer copies real files over these stubs during install() — that is + * fine and correct. + */ +function stubHooksIntoTarget(targetDir) { + const hooksDest = path.join(targetDir, 'hooks'); + fs.mkdirSync(hooksDest, { recursive: true }); + for (const hookFile of STUB_HOOKS) { + const src = path.join(HOOKS_SRC, hookFile); + const dest = path.join(hooksDest, hookFile); + if (fs.existsSync(src)) { + fs.copyFileSync(src, dest); + } else { + // Minimal stub so existsSync passes + fs.writeFileSync(dest, '#!/usr/bin/env node\n// stub\n'); + } + try { fs.chmodSync(dest, 0o755); } catch { /* Windows */ } + } +} + +function persistSettings(settingsPath, settings) { + fs.mkdirSync(path.dirname(settingsPath), { recursive: true }); + fs.writeFileSync(settingsPath, JSON.stringify(validateHookFields(settings), null, 2) + '\n', 'utf8'); +} + +// ─── Suite 1: Claude — new context monitor events are registered ────────────── + +describe('enh-770: Claude install registers SubagentStop / Stop / PreCompact context hooks', () => { + let tmpDir; + let previousCwd; + let settings; + + beforeEach(() => { + tmpDir = createTempDir('gsd-770-claude-ctx-'); + previousCwd = process.cwd(); + process.chdir(tmpDir); + stubHooksIntoTarget(path.join(tmpDir, '.claude')); + + const result = install(false, 'claude', { installerMigrations: [] }); + settings = result && result.settings; + }); + + afterEach(() => { + process.chdir(previousCwd); + cleanup(tmpDir); + }); + + test('install returns a settings object (not null)', () => { + assert.ok(settings !== null && typeof settings === 'object', + 'Claude install must return a non-null settings object'); + }); + + test('SubagentStop event is registered with at least one hook', () => { + const cmds = hooksForEvent(settings, 'SubagentStop'); + assert.ok(cmds.length > 0, + `Expected SubagentStop hooks; got hooks: ${JSON.stringify(settings && settings.hooks)}`); + }); + + test('Stop event is registered with at least one hook', () => { + const cmds = hooksForEvent(settings, 'Stop'); + assert.ok(cmds.length > 0, + `Expected Stop hooks; got hooks: ${JSON.stringify(settings && settings.hooks)}`); + }); + + test('PreCompact event is registered with at least one hook', () => { + const cmds = hooksForEvent(settings, 'PreCompact'); + assert.ok(cmds.length > 0, + `Expected PreCompact hooks; got hooks: ${JSON.stringify(settings && settings.hooks)}`); + }); + + test('SubagentStop / Stop / PreCompact all use gsd-context-monitor', () => { + for (const event of ['SubagentStop', 'Stop', 'PreCompact']) { + const cmds = hooksForEvent(settings, event); + assert.ok( + cmds.some(c => c.includes('gsd-context-monitor')), + `Event ${event} should use gsd-context-monitor; got commands: ${JSON.stringify(cmds)}` + ); + } + }); +}); + +// ─── Suite 2: Claude — FileChanged hook for config hot-reload ───────────────── + +describe('enh-770: Claude install registers FileChanged hook for .planning/config.json', () => { + let tmpDir; + let previousCwd; + let settings; + + beforeEach(() => { + tmpDir = createTempDir('gsd-770-filechanged-'); + previousCwd = process.cwd(); + process.chdir(tmpDir); + stubHooksIntoTarget(path.join(tmpDir, '.claude')); + + const result = install(false, 'claude', { installerMigrations: [] }); + settings = result && result.settings; + }); + + afterEach(() => { + process.chdir(previousCwd); + cleanup(tmpDir); + }); + + test('FileChanged event is registered with at least one hook', () => { + const cmds = hooksForEvent(settings, 'FileChanged'); + assert.ok(cmds.length > 0, + `Expected FileChanged hooks; got hooks: ${JSON.stringify(settings && settings.hooks)}`); + }); + + test('FileChanged hook uses gsd-config-reload', () => { + const cmds = hooksForEvent(settings, 'FileChanged'); + assert.ok( + cmds.some(c => c.includes('gsd-config-reload')), + `FileChanged should use gsd-config-reload; got commands: ${JSON.stringify(cmds)}` + ); + }); + + test('FileChanged hook has a matcher targeting .planning/config.json', () => { + const matchers = matchersForEvent(settings, 'FileChanged'); + assert.ok( + matchers.some(m => m && m.includes('config.json')), + `FileChanged matcher should target config.json; got matchers: ${JSON.stringify(matchers)}` + ); + }); +}); + +// ─── Suite 3: Idempotency ───────────────────────────────────────────────────── + +describe('enh-770: Claude install is idempotent for the new hook events', () => { + let tmpDir; + let previousCwd; + + beforeEach(() => { + tmpDir = createTempDir('gsd-770-idem-'); + previousCwd = process.cwd(); + process.chdir(tmpDir); + stubHooksIntoTarget(path.join(tmpDir, '.claude')); + }); + + afterEach(() => { + process.chdir(previousCwd); + cleanup(tmpDir); + }); + + test('re-running after persisted first install does not duplicate context monitor hooks', () => { + const result1 = install(false, 'claude', { installerMigrations: [] }); + persistSettings(result1.settingsPath, result1.settings); + + process.chdir(tmpDir); + const result2 = install(false, 'claude', { installerMigrations: [] }); + const s2 = result2.settings; + + for (const event of ['SubagentStop', 'Stop', 'PreCompact']) { + const cmds = hooksForEvent(s2, event); + assert.strictEqual(cmds.length, 1, + `Event ${event} should have exactly 1 hook after idempotent reinstall; got ${cmds.length}: ${JSON.stringify(cmds)}`); + } + }); + + test('re-running after persisted first install does not duplicate FileChanged hook', () => { + const result1 = install(false, 'claude', { installerMigrations: [] }); + persistSettings(result1.settingsPath, result1.settings); + + process.chdir(tmpDir); + const result2 = install(false, 'claude', { installerMigrations: [] }); + const s2 = result2.settings; + + const cmds = hooksForEvent(s2, 'FileChanged'); + assert.strictEqual(cmds.length, 1, + `FileChanged should have exactly 1 hook after idempotent reinstall; got ${cmds.length}: ${JSON.stringify(cmds)}`); + }); +}); + +// ─── Suite 4: Uninstall removes registrations ───────────────────────────────── + +describe('enh-770: Uninstall removes new hook event entries', () => { + let tmpDir; + let previousCwd; + + beforeEach(() => { + tmpDir = createTempDir('gsd-770-uninstall-'); + previousCwd = process.cwd(); + process.chdir(tmpDir); + stubHooksIntoTarget(path.join(tmpDir, '.claude')); + + const result = install(false, 'claude', { installerMigrations: [] }); + persistSettings(result.settingsPath, result.settings); + }); + + afterEach(() => { + process.chdir(previousCwd); + cleanup(tmpDir); + }); + + test('settings.json hook entries are removed on uninstall', () => { + uninstall(false, 'claude', { installerMigrations: [] }); + const settingsPath = path.join(tmpDir, '.claude', 'settings.json'); + if (!fs.existsSync(settingsPath)) return; // file removed entirely is fine + const settings = JSON.parse(fs.readFileSync(settingsPath, 'utf8')); + for (const event of ['SubagentStop', 'Stop', 'PreCompact', 'FileChanged']) { + const cmds = hooksForEvent(settings, event); + assert.strictEqual(cmds.length, 0, + `After uninstall, ${event} should have 0 hooks; got: ${JSON.stringify(cmds)}`); + } + }); +}); + +// ─── Suite 5: gsd-config-reload.js hook script exists and has correct shape ─── + +describe('enh-770: gsd-config-reload.js hook script', () => { + const reloadScript = path.join(__dirname, '..', 'hooks', 'gsd-config-reload.js'); + + test('gsd-config-reload.js exists in hooks/', () => { + assert.ok(fs.existsSync(reloadScript), + `gsd-config-reload.js must exist at ${reloadScript}`); + }); + + test('gsd-config-reload.js contains the gsd-hook-version stamp', () => { + // allow-test-rule: runtime-contract-is-the-product — the stamp template token + // IS the product surface that the installer must find and replace with the + // real version at copy time; asserting its presence is required. + const content = fs.readFileSync(reloadScript, 'utf8'); + assert.ok( + content.includes('gsd-hook-version'), + 'gsd-config-reload.js must contain the gsd-hook-version stamp for installer stamping' + ); + }); + + test('gsd-config-reload.js reads from stdin and emits JSON output', () => { + // allow-test-rule: runtime-contract-is-the-product — the stdin-read and + // JSON-emit pattern IS the hook contract; asserting its presence is required. + const content = fs.readFileSync(reloadScript, 'utf8'); + assert.ok( + content.includes('process.stdin') && content.includes('JSON.stringify'), + 'gsd-config-reload.js must read stdin and emit JSON output per hook protocol' + ); + }); + + test('gsd-config-reload.js targets the FileChanged hook event', () => { + // allow-test-rule: runtime-contract-is-the-product — the hookEventName is + // the protocol surface; asserting its presence verifies the contract. + const content = fs.readFileSync(reloadScript, 'utf8'); + assert.ok( + content.includes('FileChanged'), + 'gsd-config-reload.js must reference FileChanged in its hookSpecificOutput' + ); + }); +}); + +// ─── Suite 6: hooks.json plugin manifest includes new events ────────────────── + +describe('enh-770: hooks/hooks.json plugin manifest includes new hook events', () => { + const hooksJsonPath = path.join(__dirname, '..', 'hooks', 'hooks.json'); + + test('hooks.json exists', () => { + assert.ok(fs.existsSync(hooksJsonPath), `hooks.json must exist at ${hooksJsonPath}`); + }); + + test('hooks.json contains SubagentStop event', () => { + // allow-test-rule: runtime-contract-is-the-product — hooks.json IS the + // plugin manifest surface that Claude Code reads at plugin load time. + const content = JSON.parse(fs.readFileSync(hooksJsonPath, 'utf8')); + assert.ok( + content.hooks && content.hooks.SubagentStop, + 'hooks.json must contain SubagentStop' + ); + }); + + test('hooks.json contains Stop event', () => { + // allow-test-rule: runtime-contract-is-the-product — hooks.json IS the + // plugin manifest surface that Claude Code reads at plugin load time. + const content = JSON.parse(fs.readFileSync(hooksJsonPath, 'utf8')); + assert.ok( + content.hooks && content.hooks.Stop, + 'hooks.json must contain Stop' + ); + }); + + test('hooks.json contains PreCompact event', () => { + // allow-test-rule: runtime-contract-is-the-product — hooks.json IS the + // plugin manifest surface that Claude Code reads at plugin load time. + const content = JSON.parse(fs.readFileSync(hooksJsonPath, 'utf8')); + assert.ok( + content.hooks && content.hooks.PreCompact, + 'hooks.json must contain PreCompact' + ); + }); + + test('hooks.json contains FileChanged event', () => { + // allow-test-rule: runtime-contract-is-the-product — hooks.json IS the + // plugin manifest surface that Claude Code reads at plugin load time. + const content = JSON.parse(fs.readFileSync(hooksJsonPath, 'utf8')); + assert.ok( + content.hooks && content.hooks.FileChanged, + 'hooks.json must contain FileChanged' + ); + }); +}); + +// ─── Suite 7: managed-hooks-registry includes gsd-config-reload.js ─────────── + +describe('enh-770: managed-hooks-registry includes gsd-config-reload.js', () => { + test('MANAGED_HOOKS array includes gsd-config-reload.js', () => { + const { MANAGED_HOOKS } = require('../hooks/managed-hooks-registry.cjs'); + assert.ok( + MANAGED_HOOKS.includes('gsd-config-reload.js'), + `MANAGED_HOOKS must include gsd-config-reload.js; got: ${JSON.stringify(MANAGED_HOOKS)}` + ); + }); +}); diff --git a/tests/enh-788-qwen-hook-events.test.cjs b/tests/enh-788-qwen-hook-events.test.cjs index 39d2e0d05..c391a9b88 100644 --- a/tests/enh-788-qwen-hook-events.test.cjs +++ b/tests/enh-788-qwen-hook-events.test.cjs @@ -52,6 +52,7 @@ const STUB_HOOKS = [ 'gsd-context-monitor.js', 'gsd-prompt-guard.js', 'gsd-check-update.js', + 'gsd-config-reload.js', // Added in #770 ]; function stubHooksIntoTarget(targetDir) { @@ -148,11 +149,21 @@ describe('enh-788: Qwen install registers 3 new hook events', () => { ); } }); + + test('FileChanged is NOT registered for Qwen (Claude-only event)', () => { + // gsd-config-reload / FileChanged is a Claude Code-only registration. + // Qwen does not support the FileChanged hook event at all. + const cmds = hooksForEvent(settings, 'FileChanged'); + assert.strictEqual(cmds.length, 0, + `FileChanged should NOT be registered for Qwen; got: ${JSON.stringify(cmds)}`); + }); }); -// ─── Suite 2: Claude install does NOT get the new events ───────────────────── +// ─── Suite 2: Claude install DOES get the context events (since #770) ─────── +// Note: Prior to #770, these were Qwen-only events. #770 extended them to +// Claude Code. This suite is updated to match the new expected behavior. -describe('enh-788: Claude install does NOT register Qwen-only hook events', () => { +describe('enh-788 (updated by #770): Claude install registers context lifecycle events', () => { let tmpDir; let previousCwd; let settings; @@ -161,8 +172,9 @@ describe('enh-788: Claude install does NOT register Qwen-only hook events', () = tmpDir = createTempDir('gsd-788-claude-'); previousCwd = process.cwd(); process.chdir(tmpDir); + stubHooksIntoTarget(path.join(tmpDir, '.claude')); - const result = install(false, 'claude'); + const result = install(false, 'claude', { installerMigrations: [] }); settings = result && result.settings; }); @@ -171,22 +183,22 @@ describe('enh-788: Claude install does NOT register Qwen-only hook events', () = cleanup(tmpDir); }); - test('Claude install does not register SubagentStop', () => { + test('Claude install registers SubagentStop (since #770)', () => { const cmds = hooksForEvent(settings, 'SubagentStop'); - assert.strictEqual(cmds.length, 0, - `Claude should NOT have SubagentStop; got: ${JSON.stringify(cmds)}`); + assert.ok(cmds.length > 0, + `Claude should have SubagentStop since #770; got: ${JSON.stringify(cmds)}`); }); - test('Claude install does not register Stop', () => { + test('Claude install registers Stop (since #770)', () => { const cmds = hooksForEvent(settings, 'Stop'); - assert.strictEqual(cmds.length, 0, - `Claude should NOT have Stop; got: ${JSON.stringify(cmds)}`); + assert.ok(cmds.length > 0, + `Claude should have Stop since #770; got: ${JSON.stringify(cmds)}`); }); - test('Claude install does not register PreCompact', () => { + test('Claude install registers PreCompact (since #770)', () => { const cmds = hooksForEvent(settings, 'PreCompact'); - assert.strictEqual(cmds.length, 0, - `Claude should NOT have PreCompact; got: ${JSON.stringify(cmds)}`); + assert.ok(cmds.length > 0, + `Claude should have PreCompact since #770; got: ${JSON.stringify(cmds)}`); }); }); diff --git a/tests/helpers/install-shared.cjs b/tests/helpers/install-shared.cjs index 14f131fbd..93f1dc977 100644 --- a/tests/helpers/install-shared.cjs +++ b/tests/helpers/install-shared.cjs @@ -26,6 +26,7 @@ const EXPECTED_SH_HOOKS = [ const EXPECTED_ALL_HOOKS = [ 'gsd-check-update.js', + 'gsd-config-reload.js', 'gsd-context-monitor.js', 'gsd-prompt-guard.js', 'gsd-read-guard.js', diff --git a/tests/issue-766-plugin-manifest.test.cjs b/tests/issue-766-plugin-manifest.test.cjs index 870e4aa23..3854499bd 100644 --- a/tests/issue-766-plugin-manifest.test.cjs +++ b/tests/issue-766-plugin-manifest.test.cjs @@ -114,9 +114,15 @@ describe('B: hooks/hooks.json', () => { ); }); - test('every event name is one of: SessionStart, PreToolUse, PostToolUse', (t) => { + test('every event name is a known Claude Code lifecycle event', (t) => { if (!hooksConfig) { t.skip('hooks.json could not be parsed'); return; } - const validEvents = new Set(['SessionStart', 'PreToolUse', 'PostToolUse']); + // Complete set of Claude Code hook events as of #770 (SubagentStop, Stop, + // PreCompact, FileChanged added in #770; prior set was SessionStart, + // PreToolUse, PostToolUse from #766). + const validEvents = new Set([ + 'SessionStart', 'PreToolUse', 'PostToolUse', + 'SubagentStop', 'Stop', 'PreCompact', 'FileChanged', + ]); for (const eventName of Object.keys(hooksConfig.hooks)) { assert.ok(validEvents.has(eventName), `Unknown hook event: "${eventName}"`); } From 543e51e71f5e5a79d58e6e4522a957445323e7b1 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Sun, 7 Jun 2026 21:43:30 -0400 Subject: [PATCH 8/8] fix(#844): sync runtime manifest versions on npm version bump (#845) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(#844): sync runtime manifest versions on npm version bump The release workflow bumps package.json via `npm version` but never stamped the runtime-integration manifests that must track it (.claude-plugin/plugin.json #766, gemini-extension.json #775), so the first RC/finalize whose version diverged from the -dev stream failed the test suite before tagging/publishing. Add scripts/sync-manifest-versions.cjs (single VERSIONED_MANIFESTS registry) wired to a `version` npm lifecycle hook that stamps + stages the manifests on every `npm version` — covering all four release bump sites and local bumps with no workflow edits. A regression guard test fails if any repo JSON whose version matches package.json is not registered, forcing future version-bearing manifests into the sync. Co-Authored-By: Claude Opus 4.8 * docs(#844): add changeset for manifest version sync fix Co-Authored-By: Claude Opus 4.8 --------- Co-authored-by: Claude Opus 4.8 --- .changeset/curious-seals-howl.md | 5 + VERSIONING.md | 18 ++ package.json | 1 + scripts/sync-manifest-versions.cjs | 119 +++++++++ .../issue-844-manifest-version-sync.test.cjs | 236 ++++++++++++++++++ 5 files changed, 379 insertions(+) create mode 100644 .changeset/curious-seals-howl.md create mode 100644 scripts/sync-manifest-versions.cjs create mode 100644 tests/issue-844-manifest-version-sync.test.cjs diff --git a/.changeset/curious-seals-howl.md b/.changeset/curious-seals-howl.md new file mode 100644 index 000000000..aee2587f9 --- /dev/null +++ b/.changeset/curious-seals-howl.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 845 +--- +**Release version bumps now keep runtime manifest versions in sync** — `.claude-plugin/plugin.json` and `gemini-extension.json` are stamped to match `package.json` on every `npm version`, unblocking RC/finalize releases. New version-bearing manifests must be registered in `scripts/sync-manifest-versions.cjs` (enforced by a regression test). diff --git a/VERSIONING.md b/VERSIONING.md index c568db28a..7c3a53493 100644 --- a/VERSIONING.md +++ b/VERSIONING.md @@ -124,6 +124,24 @@ Branch names map to commit types: | `docs/` | `docs:` | none | | `refactor/` | `refactor:` | none | +## Manifest Version Sync + +Certain runtime-integration manifests carry a `version` field that must always +match `package.json`: + +- `.claude-plugin/plugin.json` — Claude Code plugin manifest (issue #766) +- `gemini-extension.json` — Gemini CLI extension manifest (issue #775) + +The `version` npm lifecycle script (`scripts/sync-manifest-versions.cjs --stage`) +stamps these files automatically on every `npm version` call, and stages them so +they are included in the release commit alongside `package.json`. + +To add a new manifest that must track the package version, register its path in +the `VERSIONED_MANIFESTS` array in `scripts/sync-manifest-versions.cjs`. A +regression test (`tests/issue-844-manifest-version-sync.test.cjs`) enforces this: +it scans all committed JSON files for a matching `version` field and fails if any +are missing from the registry. + ## Publishing Commands (Reference) ```bash diff --git a/package.json b/package.json index 8eac5d16e..64e82d54d 100644 --- a/package.json +++ b/package.json @@ -83,6 +83,7 @@ "generate:identity": "node scripts/generate-package-identity.cjs", "prepack": "npm run build:lib", "prepare": "npm run build:lib", + "version": "node scripts/sync-manifest-versions.cjs --stage", "prepublishOnly": "npm run build:lib && npm run build:hooks", "pretest": "npm run build:lib && npm run lint:skill-deps", "pretest:coverage": "npm run build:lib && npm run lint:skill-deps", diff --git a/scripts/sync-manifest-versions.cjs b/scripts/sync-manifest-versions.cjs new file mode 100644 index 000000000..f110b492e --- /dev/null +++ b/scripts/sync-manifest-versions.cjs @@ -0,0 +1,119 @@ +#!/usr/bin/env node +'use strict'; + +/** + * sync-manifest-versions.cjs + * + * Stamps the package.json version into every runtime-integration manifest whose + * top-level `version` field MUST track the package version. Called automatically + * by the `version` npm lifecycle script so that `npm version X.Y.Z` keeps all + * registered manifests in sync. + * + * Usage: + * node scripts/sync-manifest-versions.cjs # stamp + report + * node scripts/sync-manifest-versions.cjs --stage # stamp + git-stage manifests + * node scripts/sync-manifest-versions.cjs --check # report drift, exit 1 if any + */ + +const fs = require('fs'); +const path = require('path'); +const { execFileSync } = require('child_process'); + +const ROOT = path.resolve(__dirname, '..'); + +// Single source of truth: runtime-integration manifests whose top-level `version` +// MUST track package.json. Add a new manifest here so `npm version` keeps it in +// sync — the regression guard test (issue 844) fails if you forget. +const VERSIONED_MANIFESTS = [ + '.claude-plugin/plugin.json', + 'gemini-extension.json', +]; + +function readJson(p) { + return JSON.parse(fs.readFileSync(p, 'utf8')); +} + +function getPackageVersion(root) { + const r = root || ROOT; + return readJson(path.join(r, 'package.json')).version; +} + +// Stamp `version` into each registered manifest, preserving field order and +// 2-space + trailing-newline formatting. Returns the list of changed rel paths. +function syncManifestVersions(opts) { + const root = (opts && opts.root) || ROOT; + const v = (opts && opts.version) != null ? opts.version : getPackageVersion(root); + const changed = []; + for (const rel of VERSIONED_MANIFESTS) { + const abs = path.join(root, rel); + const manifest = readJson(abs); + if (manifest.version !== v) { + manifest.version = v; + fs.writeFileSync(abs, JSON.stringify(manifest, null, 2) + '\n'); + changed.push(rel); + } + } + return changed; +} + +// Registered manifests whose version != package version. +function findDrift(opts) { + const root = (opts && opts.root) || ROOT; + const v = (opts && opts.version) != null ? opts.version : getPackageVersion(root); + const drift = []; + for (const rel of VERSIONED_MANIFESTS) { + const found = readJson(path.join(root, rel)).version; + if (found !== v) drift.push({ manifest: rel, found, expected: v }); + } + return drift; +} + +// Best-effort outside git; fail-closed inside a work tree so a release never +// ships a stale manifest that the working-tree test already accepted. +function stageManifests(opts) { + const root = (opts && opts.root) || ROOT; + let insideWorkTree = false; + try { + insideWorkTree = execFileSync('git', ['rev-parse', '--is-inside-work-tree'], { + cwd: root, + stdio: ['ignore', 'pipe', 'ignore'], + }).toString().trim() === 'true'; + } catch {} + if (!insideWorkTree) { + console.warn('sync-manifest-versions: not a git work tree; skipping staging.'); + return; + } + try { + execFileSync('git', ['add', '--', ...VERSIONED_MANIFESTS], { cwd: root, stdio: ['ignore', 'ignore', 'pipe'] }); + } catch (err) { + const detail = err && err.stderr ? err.stderr.toString().trim() : (err && err.message) || 'unknown error'; + throw new Error(`sync-manifest-versions: failed to git-add manifests inside a work tree: ${detail}`); + } +} + +module.exports = { VERSIONED_MANIFESTS, syncManifestVersions, findDrift, getPackageVersion, stageManifests }; + +if (require.main === module) { + const args = process.argv.slice(2); + const version = getPackageVersion(); + if (args.includes('--check')) { + const drift = findDrift({ version }); + if (drift.length) { + for (const d of drift) { + console.error('Manifest ' + d.manifest + ' version ' + d.found + ' != package.json ' + d.expected); + } + console.error('Run `node scripts/sync-manifest-versions.cjs` to fix.'); + process.exitCode = 1; + } else { + console.log('All ' + VERSIONED_MANIFESTS.length + ' versioned manifests in sync at ' + version + '.'); + } + } else { + const changed = syncManifestVersions({ version }); + if (changed.length) { + console.log('Stamped ' + version + ' into: ' + changed.join(', ')); + } else { + console.log('Versioned manifests already at ' + version + '.'); + } + if (args.includes('--stage')) stageManifests(); + } +} diff --git a/tests/issue-844-manifest-version-sync.test.cjs b/tests/issue-844-manifest-version-sync.test.cjs new file mode 100644 index 000000000..109e8756b --- /dev/null +++ b/tests/issue-844-manifest-version-sync.test.cjs @@ -0,0 +1,236 @@ +'use strict'; + +/** + * Regression tests for issue #844: manifest version sync. + * + * Verifies that `scripts/sync-manifest-versions.cjs` correctly stamps the + * package.json version into every registered runtime-integration manifest, + * and that all currently-tracked manifests are in sync. + * + * Key deliverable: the regression guard (test d) asserts that any committed + * JSON file with a top-level `version` field matching package.json is + * registered in VERSIONED_MANIFESTS — forcing explicit opt-in for future + * manifests. + */ + +const { test, describe } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('fs'); +const os = require('os'); +const path = require('path'); +const { execFileSync } = require('child_process'); + +const ROOT = path.resolve(__dirname, '..'); +const helpers = require(path.join(__dirname, 'helpers.cjs')); +const { + VERSIONED_MANIFESTS, + syncManifestVersions, + getPackageVersion, + stageManifests, +} = require(path.join(ROOT, 'scripts', 'sync-manifest-versions.cjs')); + +// ─── A: RED→GREEN repro via temp fixture ───────────────────────────────────── +describe('A: syncManifestVersions — temp fixture', () => { + + let tmpRoot; + + test('setup: create temp fixture with stale manifests', () => { + tmpRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-844-')); + + // Write tmp package.json + fs.writeFileSync( + path.join(tmpRoot, 'package.json'), + JSON.stringify({ name: 'x', version: '9.9.9-test.0' }, null, 2) + '\n' + ); + + // Copy real manifests into tmp, stamped at OLD version + for (const rel of VERSIONED_MANIFESTS) { + const realAbs = path.join(ROOT, rel); + const manifest = JSON.parse(fs.readFileSync(realAbs, 'utf8')); + manifest.version = '0.0.0'; + + const destAbs = path.join(tmpRoot, rel); + const destDir = path.dirname(destAbs); + if (!fs.existsSync(destDir)) fs.mkdirSync(destDir, { recursive: true }); + fs.writeFileSync(destAbs, JSON.stringify(manifest, null, 2) + '\n'); + } + }); + + test('pre-sync: at least one manifest has stale version', () => { + assert.ok(tmpRoot, 'tmpRoot must be set by setup test'); + const pkgVersion = getPackageVersion(tmpRoot); + let anyStale = false; + for (const rel of VERSIONED_MANIFESTS) { + const m = JSON.parse(fs.readFileSync(path.join(tmpRoot, rel), 'utf8')); + if (m.version !== pkgVersion) { anyStale = true; break; } + } + assert.ok(anyStale, 'At least one manifest should be stale before sync (version 0.0.0 != 9.9.9-test.0)'); + }); + + test('syncManifestVersions stamps all manifests to package.json version', () => { + assert.ok(tmpRoot, 'tmpRoot must be set by setup test'); + const changed = syncManifestVersions({ root: tmpRoot }); + assert.ok(changed.length > 0, 'syncManifestVersions should report at least one changed file'); + + const pkgVersion = getPackageVersion(tmpRoot); + assert.equal(pkgVersion, '9.9.9-test.0'); + + for (const rel of VERSIONED_MANIFESTS) { + const abs = path.join(tmpRoot, rel); + const m = JSON.parse(fs.readFileSync(abs, 'utf8')); + assert.equal( + m.version, + '9.9.9-test.0', + `${rel} version should be 9.9.9-test.0 after sync` + ); + } + }); + + test('non-version fields are preserved after sync', () => { + assert.ok(tmpRoot, 'tmpRoot must be set by setup test'); + // Read from the real manifests to know what non-version fields should exist + for (const rel of VERSIONED_MANIFESTS) { + const real = JSON.parse(fs.readFileSync(path.join(ROOT, rel), 'utf8')); + const tmp = JSON.parse(fs.readFileSync(path.join(tmpRoot, rel), 'utf8')); + // Check that every non-version key from the real manifest exists in tmp + for (const key of Object.keys(real)) { + if (key === 'version') continue; + assert.ok( + Object.prototype.hasOwnProperty.call(tmp, key), + `${rel}: field "${key}" should be preserved after sync` + ); + } + } + }); + + test('each synced file ends with a single trailing newline', () => { + assert.ok(tmpRoot, 'tmpRoot must be set by setup test'); + for (const rel of VERSIONED_MANIFESTS) { + const raw = fs.readFileSync(path.join(tmpRoot, rel), 'utf8'); + assert.ok(raw.endsWith('\n'), `${rel} must end with a trailing newline`); + assert.ok(!raw.endsWith('\n\n'), `${rel} must not end with a double newline`); + } + }); + + test('second syncManifestVersions call is idempotent (returns [])', () => { + assert.ok(tmpRoot, 'tmpRoot must be set by setup test'); + const changed = syncManifestVersions({ root: tmpRoot }); + assert.deepEqual(changed, [], 'Second sync call should return [] (already in sync)'); + }); + + test('cleanup: remove temp fixture', () => { + if (tmpRoot) { + helpers.cleanup(tmpRoot); + tmpRoot = null; + } + }); +}); + +// ─── B: Registry-in-sync: real manifests match package.json ────────────────── +describe('B: real manifests match package.json version', () => { + + const pkgVersion = getPackageVersion(ROOT); + + for (const rel of VERSIONED_MANIFESTS) { + test(`${rel} version === ${pkgVersion}`, () => { + const abs = path.join(ROOT, rel); + assert.ok(fs.existsSync(abs), `${rel} must exist at ${abs}`); + const m = JSON.parse(fs.readFileSync(abs, 'utf8')); + assert.equal( + m.version, + pkgVersion, + `${rel} version (${m.version}) must match package.json version (${pkgVersion}). ` + + 'Run `node scripts/sync-manifest-versions.cjs` to fix.' + ); + }); + } +}); + +// ─── C: Regression guard — all version-bearing JSON files are registered ────── +describe('C: regression guard — version-bearing JSON files must be registered', () => { + + // package.json is the version source; package-lock.json is npm-managed. + // Both inherently track the version without the sync script. + const ALLOWED = new Set([...VERSIONED_MANIFESTS, 'package.json', 'package-lock.json']); + + // Semver-ish: matches X.Y.Z with optional pre-release/build metadata. + const SEMVER = /^\d+\.\d+\.\d+(?:[-+].+)?$/; + + // Paths to exclude from the guard + const EXCLUDED_PREFIXES = ['tests/', 'node_modules/', '.changeset/', 'docs/']; + + test('every committed JSON with a semver top-level version is registered or explicitly allowed', (t) => { + // Enumerate committed JSON files via git (no pathspec to avoid recursion quirks; + // filter to .json in JS instead). + let lines; + try { + const out = execFileSync('git', ['ls-files'], { cwd: ROOT }); + lines = out.toString().split('\n').filter((f) => f.endsWith('.json')); + } catch (err) { + t.skip('git unavailable: ' + err.message); + return; + } + + for (const rel of lines) { + if (ALLOWED.has(rel)) continue; + if (EXCLUDED_PREFIXES.some(prefix => rel.startsWith(prefix))) continue; + + const abs = path.join(ROOT, rel); + let parsed; + try { + parsed = JSON.parse(fs.readFileSync(abs, 'utf8')); + } catch (_) { + continue; // skip invalid JSON (shouldn't exist, but be safe) + } + + if ( + parsed && + typeof parsed === 'object' && + !Array.isArray(parsed) && + typeof parsed.version === 'string' && + SEMVER.test(parsed.version) + ) { + assert.ok( + ALLOWED.has(rel), + `${rel} has a semver top-level "version" but is not registered in ` + + 'scripts/sync-manifest-versions.cjs VERSIONED_MANIFESTS (nor an npm-managed file). ' + + "Register it so 'npm version' keeps it in sync (issue #844)." + ); + } + } + }); +}); + +// ─── E: stageManifests in a non-git dir must not throw ─────────────────────── +describe('E: stageManifests — non-git dir is a no-op, not a throw', () => { + + test('stageManifests({root}) with a non-git tempdir warns and returns without throwing', () => { + const tmpRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-844-nogit-')); + try { + // Write a minimal package.json so getPackageVersion doesn't error if called + fs.writeFileSync( + path.join(tmpRoot, 'package.json'), + JSON.stringify({ name: 'x', version: '0.0.0' }, null, 2) + '\n' + ); + // Must not throw even though tmpRoot is not a git repo + assert.doesNotThrow(() => { + stageManifests({ root: tmpRoot }); + }, 'stageManifests must not throw outside a git work tree'); + } finally { + helpers.cleanup(tmpRoot); + } + }); +}); + +// ─── D: CLI --check exits 0 when in sync ───────────────────────────────────── +describe('D: CLI --check exits 0 when manifests are in sync', () => { + + test('node scripts/sync-manifest-versions.cjs --check exits 0', () => { + // Will throw if exit code != 0 + execFileSync( + process.execPath, + [path.join(ROOT, 'scripts', 'sync-manifest-versions.cjs'), '--check'], + { cwd: ROOT } + ); + }); +});