fix(#2089): wire adapter into install path + address all review findings

MEDIUM fixes (code review): - Wire resolveManagedHookEvents + resolveHookScripts + buildHookBusEntries from imperative-hook-bus.cts into writeCursorHooksJson — the install path is now truly descriptor-driven (reads hostBehaviors.managedHookEvents), not a hardcoded constant that happens to match the descriptor. bin/install.js passes the descriptor list via opts.managedHookEvents. - buildHookBusEntries is now consumed (was dead code); entry-building is no longer duplicated inline. - Remove try/finally from cursor-hook-bus-upgrade.test.cjs test bodies (violated CONTRIBUTING.md L342; redundant with t.after cleanup). LOW fixes: - Remove dead require('fs')/require('path') from gsd-cursor-pre-tool.js - Fix resolveManagedHookEvents docstring (all-invalid fallback behavior) - Add src/runtime-hooks-surface.cts to the AC2 source-guard file list Security review: no CRITICAL/HIGH/MEDIUM findings (3 LOW are pre-existing #777 baseline patterns, not regressions).
2026-07-09 13:17:04 -04:00
parent fad5094587
commit 45f3a2a5f9
6 changed files with 81 additions and 98 deletions
--- a/bin/install.js
+++ b/bin/install.js
@@ -10004,7 +10004,9 @@ function install(isGlobal, runtime = DEFAULT_RUNTIME, options = {}) {
    // adapter. Registers all 6 managed events (sessionStart, postToolUse, preToolUse,
    // stop, subagentStart, subagentStop) via runtime-hooks-surface.cts, which reads
    // the event list from the descriptor-driven adapter module.
-    const cursorHookResult = writeCursorHooksJson(targetDir, src, {});
+    const cursorHookResult = writeCursorHooksJson(targetDir, src, {
+      managedHookEvents: _hostBehaviors(runtime).managedHookEvents,
+    });
    if (cursorHookResult.changed) {
      console.log(`  ${green}✓${reset} Configured Cursor lifecycle hooks (sessionStart, postToolUse, preToolUse, stop, subagentStart, subagentStop)`);
    } else {
--- a/hooks/gsd-cursor-pre-tool.js
+++ b/hooks/gsd-cursor-pre-tool.js
@@ -22,9 +22,6 @@

 'use strict';

-const fs = require('fs');
-const path = require('path');
-
 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([\\/]|$)/;
--- a/src/host-integration-adapters/imperative-hook-bus.cts
+++ b/src/host-integration-adapters/imperative-hook-bus.cts
@@ -73,7 +73,9 @@ export const GSD_HOOK_MARKER = 'gsd-managed';
 * (backward-compat for descriptors predating #2089).
 *
 * Pure: no I/O, never throws. Unknown event names are silently filtered
- * (fail-closed — an unrecognized event is never registered).
+ * (fail-closed — an unrecognized event is never registered). Falls back to
+ * the full CURSOR_HOOK_EVENTS set when the descriptor is absent or all entries
+ * are unrecognized (ensures the portable-event floor is always covered).
 *
 * @param managedHookEvents - the descriptor's `hostBehaviors.managedHookEvents` array
 * @returns a deduplicated, validated array of event names
--- a/src/runtime-hooks-surface.cts
+++ b/src/runtime-hooks-surface.cts
@@ -27,6 +27,13 @@
 import fs from 'node:fs';
 import path from 'node:path';
 import os from 'node:os';
+import {
+  CURSOR_HOOK_EVENTS,
+  CURSOR_EVENT_SCRIPT_MAP,
+  resolveManagedHookEvents,
+  resolveHookScripts,
+  buildHookBusEntries,
+} from './host-integration-adapters/imperative-hook-bus.cjs';
 // eslint-disable-next-line @typescript-eslint/no-require-imports
 import shellCmdProjection = require('./shell-command-projection.cjs');
 const {
@@ -86,18 +93,13 @@ const GSD_CURSOR_SUBAGENT_START_HOOK_SCRIPT = 'gsd-cursor-subagent-start.js';
 const GSD_CURSOR_SUBAGENT_STOP_HOOK_SCRIPT = 'gsd-cursor-subagent-stop.js';
 const GSD_CURSOR_HOOK_MARKER = 'gsd-managed';

-// The full set of Cursor hook events GSD manages (AC4a upgrade, #2089).
-// Sourced from the descriptor-driven adapter module
-// (src/host-integration-adapters/imperative-hook-bus.cts). This replaces the
-// hardcoded ['sessionStart', 'postToolUse'] pair with the 6-event managed set.
-const CURSOR_MANAGED_EVENTS = [
-  'sessionStart',
-  'postToolUse',
-  'preToolUse',
-  'stop',
-  'subagentStart',
-  'subagentStop',
-];
+// The full set of Cursor hook events GSD manages — sourced from the adapter
+// (src/host-integration-adapters/imperative-hook-bus.cts) so the vocabulary
+// stays closed and first-party. Used by reconcileCursorHooksJson (the
+// reconciliation scope is always the full set). The install path
+// (writeCursorHooksJson) resolves a descriptor-driven subset via
+// resolveManagedHookEvents(opts.managedHookEvents).
+const CURSOR_MANAGED_EVENTS = CURSOR_HOOK_EVENTS;

 // ---------------------------------------------------------------------------
 // Cline / AGENTS.md constants
@@ -1035,6 +1037,7 @@ function reconcileCursorHooksJson(hooksJsonPath: string, managedEntries: CursorM
 interface WriteCursorHooksJsonOpts {
  absoluteRunner?: string | null;
  platform?: string;
+  managedHookEvents?: readonly string[];
 }

 function writeCursorHooksJson(targetDir: string, src: string, opts?: WriteCursorHooksJsonOpts): { hooksJsonPath: string; changed: boolean } {
@@ -1042,20 +1045,11 @@ function writeCursorHooksJson(targetDir: string, src: string, opts?: WriteCursor
  const hooksDir = path.join(targetDir, 'hooks');
  fs.mkdirSync(hooksDir, { recursive: true });

-  // AC4a (#2089): install all managed hook scripts, not just sessionStart/postToolUse.
-  // The event→script mapping is sourced from the descriptor-driven adapter
-  // (src/host-integration-adapters/imperative-hook-bus.cts).
-  const eventScriptMap: Record<string, string> = {
-    sessionStart: GSD_CURSOR_SESSION_HOOK_SCRIPT,
-    postToolUse: GSD_CURSOR_POST_TOOL_HOOK_SCRIPT,
-    preToolUse: GSD_CURSOR_PRE_TOOL_HOOK_SCRIPT,
-    stop: GSD_CURSOR_STOP_HOOK_SCRIPT,
-    subagentStart: GSD_CURSOR_SUBAGENT_START_HOOK_SCRIPT,
-    subagentStop: GSD_CURSOR_SUBAGENT_STOP_HOOK_SCRIPT,
-  };
-  const hookScripts = CURSOR_MANAGED_EVENTS
-    .map((ev) => eventScriptMap[ev])
-    .filter((s): s is string => Boolean(s));
+  // Descriptor-driven event resolution (#2089): the managed event set comes
+  // from the host descriptor's hostBehaviors.managedHookEvents via the pure
+  // adapter (resolveManagedHookEvents), NOT a hardcoded constant.
+  const events = resolveManagedHookEvents(opts.managedHookEvents);
+  const hookScripts = resolveHookScripts(events);
  const srcHooksDir = path.join(src, 'hooks');
  const installedScripts = new Set<string>();
  for (const script of hookScripts) {
@@ -1071,20 +1065,16 @@ function writeCursorHooksJson(targetDir: string, src: string, opts?: WriteCursor
  }

  const hookOpts: BuildHookCommandOpts = { runtime: 'cursor', platform: opts.platform || process.platform };
-  const managedEntries: CursorManagedEntries = {};
-  for (const ev of CURSOR_MANAGED_EVENTS) {
-    const script = eventScriptMap[ev];
+  const commands: Record<string, string | null> = {};
+  for (const ev of events) {
+    const script = CURSOR_EVENT_SCRIPT_MAP[ev];
    if (script && installedScripts.has(script)) {
-      const cmd = buildHookCommand(targetDir, script, hookOpts);
-      if (cmd) {
-        managedEntries[ev] = {
-          type: 'command',
-          command: cmd,
-          [GSD_CURSOR_HOOK_MARKER]: true,
-        };
-      }
+      commands[ev] = buildHookCommand(targetDir, script, hookOpts);
+    } else {
+      commands[ev] = null;
    }
  }
+  const managedEntries = buildHookBusEntries(events, commands) as CursorManagedEntries;

  const hooksJsonPath = path.join(targetDir, 'hooks.json');
  const result = reconcileCursorHooksJson(hooksJsonPath, managedEntries);
--- a/tests/cursor-hook-bus-upgrade.test.cjs
+++ b/tests/cursor-hook-bus-upgrade.test.cjs
@@ -114,68 +114,60 @@ test('all 6 hook scripts exist under hooks/', () => {
 test('reconcileCursorHooksJson writes all 6 managed events into hooks.json', (t) => {
  const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-cursor-hook-bus-'));
  t.after(() => cleanup(tmpDir));
-  try {
-    const hooksJsonPath = path.join(tmpDir, 'hooks.json');
-    const managedEntries = {};
-    for (const ev of EXPECTED_EVENTS) {
-      managedEntries[ev] = {
-        type: 'command',
-        command: `node /fake/${ev}.js`,
-        [GSD_CURSOR_HOOK_MARKER]: true,
-      };
-    }
-    const result = reconcileCursorHooksJson(hooksJsonPath, managedEntries);
-    assert.ok(result.changed, 'first write must report changed=true');
+  const hooksJsonPath = path.join(tmpDir, 'hooks.json');
+  const managedEntries = {};
+  for (const ev of EXPECTED_EVENTS) {
+    managedEntries[ev] = {
+      type: 'command',
+      command: `node /fake/${ev}.js`,
+      [GSD_CURSOR_HOOK_MARKER]: true,
+    };
+  }
+  const result = reconcileCursorHooksJson(hooksJsonPath, managedEntries);
+  assert.ok(result.changed, 'first write must report changed=true');

-    const written = JSON.parse(fs.readFileSync(hooksJsonPath, 'utf8'));
-    const hookTable = written.hooks;
-    assert.ok(hookTable && typeof hookTable === 'object');
-    for (const ev of EXPECTED_EVENTS) {
-      assert.ok(Array.isArray(hookTable[ev]),
-        `hooks.json must have a ${ev} array`);
-      assert.equal(hookTable[ev].length, 1,
-        `${ev} must have exactly 1 managed entry`);
-      assert.equal(hookTable[ev][0][GSD_CURSOR_HOOK_MARKER], true,
-        `${ev} entry must carry the GSD managed marker`);
-    }
-  } finally {
-    cleanup(tmpDir);
+  const written = JSON.parse(fs.readFileSync(hooksJsonPath, 'utf8'));
+  const hookTable = written.hooks;
+  assert.ok(hookTable && typeof hookTable === 'object');
+  for (const ev of EXPECTED_EVENTS) {
+    assert.ok(Array.isArray(hookTable[ev]),
+      `hooks.json must have a ${ev} array`);
+    assert.equal(hookTable[ev].length, 1,
+      `${ev} must have exactly 1 managed entry`);
+    assert.equal(hookTable[ev][0][GSD_CURSOR_HOOK_MARKER], true,
+      `${ev} entry must carry the GSD managed marker`);
  }
 });

 test('reconcileCursorHooksJson preserves user entries across all 6 events', (t) => {
  const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-cursor-hook-bus-'));
  t.after(() => cleanup(tmpDir));
-  try {
-    const hooksJsonPath = path.join(tmpDir, 'hooks.json');
-    // Seed with user-owned entries in two events.
-    const seed = {
-      version: 1,
-      hooks: {
-        sessionStart: [{ type: 'command', command: 'user-start.sh' }],
-        preToolUse: [{ type: 'command', command: 'user-pre.sh' }],
-      },
+  const hooksJsonPath = path.join(tmpDir, 'hooks.json');
+  // Seed with user-owned entries in two events.
+  const seed = {
+    version: 1,
+    hooks: {
+      sessionStart: [{ type: 'command', command: 'user-start.sh' }],
+      preToolUse: [{ type: 'command', command: 'user-pre.sh' }],
+    },
+  };
+  fs.writeFileSync(hooksJsonPath, JSON.stringify(seed, null, 2) + '\n');
+
+  const managedEntries = {};
+  for (const ev of EXPECTED_EVENTS) {
+    managedEntries[ev] = {
+      type: 'command',
+      command: `node /gsd/${ev}.js`,
+      [GSD_CURSOR_HOOK_MARKER]: true,
    };
-    fs.writeFileSync(hooksJsonPath, JSON.stringify(seed, null, 2) + '\n');
-
-    const managedEntries = {};
-    for (const ev of EXPECTED_EVENTS) {
-      managedEntries[ev] = {
-        type: 'command',
-        command: `node /gsd/${ev}.js`,
-        [GSD_CURSOR_HOOK_MARKER]: true,
-      };
-    }
-    reconcileCursorHooksJson(hooksJsonPath, managedEntries);
-
-    const written = JSON.parse(fs.readFileSync(hooksJsonPath, 'utf8'));
-    // sessionStart: 1 user + 1 managed
-    assert.equal(written.hooks.sessionStart.length, 2);
-    // preToolUse: 1 user + 1 managed
-    assert.equal(written.hooks.preToolUse.length, 2);
-    // postToolUse: 1 managed only
-    assert.equal(written.hooks.postToolUse.length, 1);
-  } finally {
-    cleanup(tmpDir);
  }
+  reconcileCursorHooksJson(hooksJsonPath, managedEntries);
+
+  const written = JSON.parse(fs.readFileSync(hooksJsonPath, 'utf8'));
+  // sessionStart: 1 user + 1 managed
+  assert.equal(written.hooks.sessionStart.length, 2);
+  // preToolUse: 1 user + 1 managed
+  assert.equal(written.hooks.preToolUse.length, 2);
+  // postToolUse: 1 managed only
+  assert.equal(written.hooks.postToolUse.length, 1);
 });
--- a/tests/cursor-imperative-reference.test.cjs
+++ b/tests/cursor-imperative-reference.test.cjs
@@ -118,7 +118,7 @@ test('no `runtime === "cursor"` string-equality branch remains in the install so
    .replace(/\/\*[\s\S]*?\*\//g, '')
    .replace(/\/\/[^\r\n]*/g, '')
    .replace(/`[^`]*`/g, '');
-  for (const rel of ['bin/install.js', 'src/install-engine.cts', 'src/runtime-artifact-conversion.cts']) {
+  for (const rel of ['bin/install.js', 'src/install-engine.cts', 'src/runtime-artifact-conversion.cts', 'src/runtime-hooks-surface.cts']) {
    const src = fs.readFileSync(path.join(__dirname, '..', rel), 'utf8');
    const offenders = strip(src).match(/runtime\s*[!=]==\s*'cursor'/g) || [];
    assert.deepEqual(offenders, [], `AC2: no hardcoded runtime==='cursor' branch may remain in ${rel}; found: ${offenders.join(', ')}`);