refactor(#1059): phase 5f-1 — extract standalone hook-surface writers into a module — ADR-857/1016 (#1064)

* refactor(#1059): phase 5f-1 — extract standalone hook-surface writers into a module

Extract the structurally-isolated hook-surface writer functions (cline/cursor/
copilot/codex-hooks-json + buildHookCommand + atomicWriteFileSync + node/bash runner
resolvers) out of bin/install.js into a new src/runtime-hooks-surface.cts module
(-693 LOC from install.js). Behavior-preserving: install.js requires + re-exports
the moved functions (module.exports surface preserved); no descriptor reads, no
behavior change. Prerequisite for the descriptor-drive (5f-2), mirroring ADR-3660's
artifactLayout extract→drive split.

Review caught + fixed 3 coupling issues: (HIGH) the module's atomicWriteFileSync
dropped the shared __atomicWrittenTmps temp-tracking → now ONE shared set (module
owns it, install.js aliases it, both cleanups read it); (drift) buildHookCommand
called resolveNodeRunner(opts) vs the original resolveNodeRunner() → reverted; two
source-grep tests (workflow-guard, sh-hook-paths) that scanned install.js for the
moved functions → made behavioral/non-vacuous; duplicate runner resolvers consolidated.

Settings-json hook block (~648 LOC) deferred to 5f-1b; descriptor-drive to 5f-2.
New-module checklist done. ~62 hook test files green; gsd-test 17592/0.

Closes #1059

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* chore(#1059): reconcile CLI Modules count after merging next (uat-predicate)

Merging current next (which added uat-predicate.cjs via #247) alongside this
branch's runtime-hooks-surface.cjs put the filesystem at 107 bin/lib modules, but
both sides had independently bumped the INVENTORY headline 105→106 so the merge
under-counted. Set "CLI Modules (107 shipped)" + regenerate INVENTORY-MANIFEST.json.
Both module rows already present. Fixes inventory-counts.test.cjs (the only CI red).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
Tom Boucher
2026-06-11 17:21:04 -04:00
committed by GitHub
parent 9223f2f4c8
commit 58bfae9d6a
10 changed files with 1365 additions and 920 deletions

1
.gitignore vendored
View File

@@ -130,6 +130,7 @@ build/
/gsd-core/bin/lib/planning-workspace.cjs
/gsd-core/bin/lib/runtime-artifact-layout.cjs
/gsd-core/bin/lib/runtime-config-adapter-registry.cjs
/gsd-core/bin/lib/runtime-hooks-surface.cjs
/gsd-core/bin/lib/command-routing-hub.cjs
/gsd-core/bin/lib/core.cjs
/gsd-core/bin/lib/core-utils.cjs

View File

@@ -175,6 +175,9 @@ A Capability whose integration shape brings its own external process, service, o
`RULESET.CAPABILITY.step-additive-gate-blocks=a `step` hook is purely additive (invoke skill + produce artifacts, NEVER halts the host); host-blocking preconditions are `gate`s (blocking:true, onError:halt); runtime/mode context (auto/chain vs manual) self-gates IN THE SKILL, not via `when` (config-only). §5.6 = plan:pre step (ui-phase; skill self-gates on frontend+pipeline, auto-fires only in pipelines) + a NEW plan:pre gate (frontend-and-no-UI-SPEC → halt, when:workflow.ui_safety_gate); the loop.render-hooks dispatch template handles steps AND gates. Resolves #1022.`
### Runtime Hooks Surface Module
Standalone hook-surface writer module extracted from `bin/install.js` as ADR-857 phase 5f-1 (behavior-preserving relocation, no logic change). Owns: Cline rules-body/agents-md/pre-tool-use hook generation (`buildClineRulesBody`, `buildClineAgentsMdBody`, `buildClinePreToolUseHook`, `mergeGsdAgentsMd`, `writeClineArtifacts`); Cursor `hooks.json` lifecycle (`buildCursorHookEntry`, `isManagedCursorHookEntry`, `reconcileCursorHooksJson`, `writeCursorHooksJson`, `removeCursorHooksJson`); Copilot session-hook config (`buildCopilotHookConfig`, `writeCopilotHookConfig`); Codex hook-block and event management (`buildCodexHookBlock`, `rewriteLegacyCodexHookBlock`, `reconcileCodexHooksJsonEvent`, `reconcileCodexHooksJsonSessionStart`, `ensureCodexHooksJsonSessionStart`, `ensureCodexHooksJsonEvent`, `removeCodexHooksJsonEvent`, `removeCodexHooksJsonSessionStart`, `buildCodexHookWindowsShimIR`); and shared hook command helpers (`buildHookCommand`, `rewriteLegacyManagedNodeHookCommands`, `normalizeNodePath`, `resolveNodeRunner`). `bin/install.js` delegates to this module via thin wrappers and re-exports its functions unchanged so existing tests require no modification. Source: `src/runtime-hooks-surface.cts`. Built output: `gsd-core/bin/lib/runtime-hooks-surface.cjs`.
### 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` | `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.

File diff suppressed because it is too large Load Diff

View File

@@ -378,6 +378,7 @@ Node.js CLI utility (`gsd-tools.cjs`) with domain modules split across `gsd-core
| `graphify-command-router.cjs` | ADR-959 capability command router — first real capability command cutover (phase 4d-impl-2); extracted from the `case 'graphify':` arm in `gsd-tools.cjs`; dispatches build/query/status/diff subcommands; discovered via `commandFamilies` in the capability registry |
| `audit-command-router.cjs` | ADR-959 capability command router (phase 4d-impl-3); extracted from the `case 'audit-uat':` and `case 'audit-open':` arms in `gsd-tools.cjs`; `routeAuditUat` → `uat.cjs:cmdAuditUat`, `routeAuditOpen` → `audit.cjs:{auditOpenArtifacts,formatAuditReport}`; discovered via `commandFamilies` in the capability registry |
| `intel-command-router.cjs` | ADR-959 capability command router (phase 4d-impl-4, last first-party cutover); extracted from the `case 'intel':` arm in `gsd-tools.cjs`; `routeIntelCommand` → all 9 intel subcommands via lazy `require('./intel.cjs')`; preserves non-raw `timeAgo` transform on `status.files[*].updated_at`; discovered via `commandFamilies` in the capability registry |
| `runtime-hooks-surface.cjs` | Standalone hook-surface writer module (ADR-857 phase 5f-1); owns Cline rules/agents-md/pre-tool-use hook generation, Cursor `hooks.json` reconciliation, Copilot session-hook config, and Codex hook-block management; extracted verbatim from `bin/install.js` with no logic change. |
---

View File

@@ -343,6 +343,7 @@
"runtime-artifact-layout.cjs",
"runtime-config-adapter-registry.cjs",
"runtime-homes.cjs",
"runtime-hooks-surface.cjs",
"runtime-name-policy.cjs",
"runtime-slash.cjs",
"schema-detect.cjs",

View File

@@ -371,7 +371,7 @@ The `gsd-planner` agent is decomposed into a core agent plus reference modules t
---
## CLI Modules (106 shipped)
## CLI Modules (107 shipped)
Full listing: `gsd-core/bin/lib/*.cjs`.
@@ -453,6 +453,7 @@ Full listing: `gsd-core/bin/lib/*.cjs`.
| `roadmap.cjs` | ROADMAP.md parsing, phase extraction, plan progress |
| `runtime-artifact-layout.cjs` | Runtime artifact layout module — resolves the artifact directory shapes (commands, agents, skills) for each supported runtime; single source of truth for per-runtime artifact placement (#3663) |
| `runtime-config-adapter-registry.cjs` | Explicit runtime config adapter registry — resolves per-runtime config-mutation install intent (install surface, shared-settings gate, finish-phase permission writer); see ADR-58. |
| `runtime-hooks-surface.cjs` | Runtime hooks surface module — standalone hook-surface writer functions extracted from bin/install.js (ADR-857 phase 5f-1); owns Cline/Cursor/Copilot/Codex hook artifact generation and reconciliation. |
| `runtime-name-policy.cjs` | Runtime name normalization policy — canonical token sanitization for runtime identifiers used in path construction and display |
| `runtime-homes.cjs` | Canonical runtime → global config/skills directory mapping; first-class support for all 15 runtimes including Hermes nested layout and Cline rules-based exclusion (#3126) |
| `runtime-slash.cjs` | Runtime-aware slash-command formatter — single source of truth for emitting `/gsd-<cmd>` (skills-based runtimes) and `$gsd-<cmd>` (codex) in user-facing output and persisted artifacts (#3584) |

View File

@@ -95,6 +95,7 @@ export default tseslint.config(
'gsd-core/bin/lib/planning-workspace.cjs',
'gsd-core/bin/lib/runtime-artifact-layout.cjs',
'gsd-core/bin/lib/runtime-config-adapter-registry.cjs',
'gsd-core/bin/lib/runtime-hooks-surface.cjs',
'gsd-core/bin/lib/command-routing-hub.cjs',
'gsd-core/bin/lib/core.cjs',
'gsd-core/bin/lib/core-utils.cjs',

File diff suppressed because it is too large Load Diff

View File

@@ -29,12 +29,25 @@ const path = require('path');
const INSTALL_SRC = path.join(__dirname, '..', 'bin', 'install.js');
// buildHookCommand was extracted to gsd-core/bin/lib/runtime-hooks-surface.cjs
// (ADR-857 phase 5f-1) and re-exported via install.js. Import through install.js
// so the test exercises the same public surface that the rest of the codebase uses.
const INSTALL = require(path.join(__dirname, '..', 'bin', 'install.js'));
const { buildHookCommand } = INSTALL;
const SH_HOOKS = [
{ name: 'gsd-validate-commit.sh', commandVar: 'validateCommitCommand' },
{ name: 'gsd-session-state.sh', commandVar: 'sessionStateCommand' },
{ name: 'gsd-phase-boundary.sh', commandVar: 'phaseBoundaryCommand' },
];
// Use a fixed configDir that is unambiguously absolute so the assertions below
// are not accidentally satisfied by a relative path in the output.
const TEST_CONFIG_DIR = '/test-home/.claude';
// Force a non-Windows platform so resolveBashRunner reliably returns 'bash'
// (Windows candidates need filesystem probing; platform:linux is hermetic).
const HOOK_OPTS = { platform: 'linux', runtime: 'claude' };
describe('bugs #2045 #2046: .sh hook paths must be absolute and quoted', () => {
let src;
@@ -44,57 +57,52 @@ describe('bugs #2045 #2046: .sh hook paths must be absolute and quoted', () => {
src = '';
}
// ── Test 1: buildHookCommand supports .sh files ──────────────────────────
// ── Test 1: buildHookCommand supports .sh files (BEHAVIORAL) ─────────────
describe('buildHookCommand', () => {
test('returns a bash command for .sh hookName', () => {
// Extract buildHookCommand from source and verify it branches on .sh
const fnStart = src.indexOf('function buildHookCommand(');
assert.ok(fnStart !== -1, 'buildHookCommand function not found in install.js');
// Behavioral: call the exported function and assert on the returned string.
// buildHookCommand was extracted to runtime-hooks-surface.cjs; source-grep
// on install.js no longer works (the wrapper body just delegates).
assert.equal(typeof buildHookCommand, 'function',
'buildHookCommand must be exported from install.js');
// Find the closing brace of the function (scan for the balanced brace)
let depth = 0;
let fnEnd = fnStart;
for (let i = fnStart; i < src.length; i++) {
if (src[i] === '{') depth++;
else if (src[i] === '}') {
depth--;
if (depth === 0) { fnEnd = i + 1; break; }
}
}
const fnBody = src.slice(fnStart, fnEnd);
const cmd = buildHookCommand(TEST_CONFIG_DIR, 'gsd-validate-commit.sh', HOOK_OPTS);
assert.ok(typeof cmd === 'string' && cmd.length > 0,
'buildHookCommand must return a non-empty string for .sh hooks');
assert.ok(
fnBody.includes('.sh') || fnBody.includes('bash'),
'buildHookCommand must handle .sh files by using "bash" as the runner. ' +
'The function body must contain ".sh" or "bash" for branching logic.'
cmd.includes('bash'),
'buildHookCommand must use "bash" as the runner for .sh hooks. ' +
`Got: ${cmd}`
);
});
test('buildHookCommand produces bash runner for .sh and node runner for .js', () => {
const fnStart = src.indexOf('function buildHookCommand(');
assert.ok(fnStart !== -1, 'buildHookCommand function not found in install.js');
assert.equal(typeof buildHookCommand, 'function',
'buildHookCommand must be exported from install.js');
let depth = 0;
let fnEnd = fnStart;
for (let i = fnStart; i < src.length; i++) {
if (src[i] === '{') depth++;
else if (src[i] === '}') {
depth--;
if (depth === 0) { fnEnd = i + 1; break; }
}
}
const fnBody = src.slice(fnStart, fnEnd);
// Must still produce "node" for .js (existing behavior)
// .sh hook must contain "bash"
const shCmd = buildHookCommand(TEST_CONFIG_DIR, 'gsd-validate-commit.sh', HOOK_OPTS);
assert.ok(
fnBody.includes('node'),
'buildHookCommand must still produce a "node" command for .js hooks'
typeof shCmd === 'string' && shCmd.includes('bash'),
'buildHookCommand must produce a "bash" command for .sh hooks. ' +
`Got: ${shCmd}`
);
// Must produce "bash" for .sh
// .js hook must contain "node" (absolute path will include the word "node")
const jsCmd = buildHookCommand(TEST_CONFIG_DIR, 'gsd-something.js', HOOK_OPTS);
assert.ok(
fnBody.includes('bash'),
'buildHookCommand must produce a "bash" command for .sh hooks'
typeof jsCmd === 'string' && jsCmd.includes('node'),
'buildHookCommand must produce a "node" command for .js hooks. ' +
`Got: ${jsCmd}`
);
// Non-vacuousness guard: the two commands must be DIFFERENT so that if
// buildHookCommand stops branching on .sh the test actually fails.
assert.notEqual(
shCmd.split('"')[0], // runner token before the first quoted path
jsCmd.split('"')[0],
'buildHookCommand must use different runners for .sh vs .js hooks'
);
});
});

View File

@@ -12,14 +12,24 @@
*
* Also tests the broader anti-pattern: every hook in gsdHooks that is a JS
* PreToolUse/PostToolUse hook should have a corresponding registration block.
* Hooks owned by the runtime-hooks-surface module (Cursor) are validated
* behaviorally rather than by source-scan of install.js.
*/
const { describe, test } = require('node:test');
const assert = require('node:assert/strict');
const fs = require('fs');
const path = require('path');
const { createTempDir, cleanup } = require('./helpers.cjs');
const INSTALL_JS = path.join(__dirname, '..', 'bin', 'install.js');
// Hooks whose registration lives in runtime-hooks-surface module, not install.js.
// These are excluded from the install.js source-scan and validated behaviorally.
const MODULE_OWNED_HOOKS = new Set([
'gsd-cursor-session-start.js',
'gsd-cursor-post-tool.js',
]);
describe('workflow-guard hook registration (#1767)', () => {
test('install.js constructs a command path variable for gsd-workflow-guard.js', () => {
const content = fs.readFileSync(INSTALL_JS, 'utf-8');
@@ -76,13 +86,16 @@ describe('workflow-guard hook registration (#1767)', () => {
});
describe('hook registration completeness anti-pattern guard', () => {
test('every JS hook in gsdHooks has a command construction in install.js', () => {
test('every JS hook in gsdHooks (except module-owned) has a command construction in install.js', () => {
const content = fs.readFileSync(INSTALL_JS, 'utf-8');
// Use the typed export instead of source-grep regex (branch #455: retire source-grep)
const { GSD_UNINSTALL_HOOKS } = require('../bin/install.js');
assert.ok(Array.isArray(GSD_UNINSTALL_HOOKS), 'GSD_UNINSTALL_HOOKS must be exported from install.js');
const jsHooks = GSD_UNINSTALL_HOOKS.filter(h => h.endsWith('.js'));
// Cursor hooks (gsd-cursor-*.js) are registered by writeCursorHooksJson in
// runtime-hooks-surface module, not by direct buildHookCommand calls in install.js.
// They are validated behaviorally in the describe block below.
const jsHooks = GSD_UNINSTALL_HOOKS.filter(h => h.endsWith('.js') && !MODULE_OWNED_HOOKS.has(h));
const missing = [];
for (const hook of jsHooks) {
@@ -100,10 +113,122 @@ describe('hook registration completeness anti-pattern guard', () => {
assert.strictEqual(
missing.length, 0,
[
'Every JS hook in gsdHooks must have a command construction in install.js.',
'Missing registration for:',
'Every JS hook in gsdHooks (excluding module-owned cursor hooks) must have a command',
'construction in install.js. Missing registration for:',
...missing.map(h => ` - ${h}`),
].join('\n')
);
});
});
describe('atomic write temp-tracking — shared Set parity guard', () => {
// FIX 1 verification: atomic writes from the runtime-hooks-surface module
// must register their temp paths into the SAME __atomicWrittenTmps Set that
// install.js's _cleanTmpFiles() uses. If the module had its own separate Set,
// temp files from Cursor/Codex writes would never be cleaned up.
test('writeCursorHooksJson atomic write registers temp path in shared __atomicWrittenTmps', (t) => {
const targetDir = createTempDir('atomic-track-target-');
const srcDir = createTempDir('atomic-track-src-');
t.after(() => {
cleanup(targetDir);
cleanup(srcDir);
});
const srcHooksDir = path.join(srcDir, 'hooks');
fs.mkdirSync(srcHooksDir, { recursive: true });
fs.writeFileSync(path.join(srcHooksDir, 'gsd-cursor-session-start.js'), '// stub\n');
fs.writeFileSync(path.join(srcHooksDir, 'gsd-cursor-post-tool.js'), '// stub\n');
// Capture the Set BEFORE the call so we can diff it after.
const hooksSurface = require('../gsd-core/bin/lib/runtime-hooks-surface.cjs');
const tmpsSet = hooksSurface.__atomicWrittenTmps;
assert.ok(tmpsSet instanceof Set, '__atomicWrittenTmps must be a Set');
const sizeBefore = tmpsSet.size;
const { writeCursorHooksJson } = require('../bin/install.js');
writeCursorHooksJson(targetDir, srcDir, {});
// After the write, the Set must have grown: the hooks.json temp path
// was recorded so _cleanTmpFiles can clean it if it lingers.
assert.ok(
tmpsSet.size > sizeBefore,
`__atomicWrittenTmps must grow after writeCursorHooksJson (was ${sizeBefore}, now ${tmpsSet.size})`
);
// Every new entry must be an absolute path matching the tmp pattern.
for (const tmp of tmpsSet) {
if (tmp.startsWith(targetDir)) {
// This is the entry we added; it must look like hooks.json.tmp-<pid>-<n>
assert.ok(
/hooks\.json\.tmp-\d+-\d+$/.test(tmp),
`Registered temp path must match hooks.json.tmp-<pid>-<n> pattern, got: ${tmp}`
);
}
}
});
});
describe('cursor hook registration — behavioral guard (module-owned hooks)', () => {
// The cursor hook scripts are registered by writeCursorHooksJson inside
// runtime-hooks-surface. This test calls the function directly and asserts
// that both managed entries appear in hooks.json. Deleting either cursor
// hook registration from the module will cause this test to fail.
test('writeCursorHooksJson writes both gsd-cursor-session-start and gsd-cursor-post-tool to hooks.json', (t) => {
// Create a temp target dir (simulated ~/.cursor) and a src dir with stub scripts.
const targetDir = createTempDir('cursor-guard-target-');
const srcDir = createTempDir('cursor-guard-src-');
t.after(() => {
cleanup(targetDir);
cleanup(srcDir);
});
// Stub the hook scripts in srcDir/hooks/ so writeCursorHooksJson can copy them.
const srcHooksDir = path.join(srcDir, 'hooks');
fs.mkdirSync(srcHooksDir, { recursive: true });
fs.writeFileSync(path.join(srcHooksDir, 'gsd-cursor-session-start.js'), '// stub\n');
fs.writeFileSync(path.join(srcHooksDir, 'gsd-cursor-post-tool.js'), '// stub\n');
const { writeCursorHooksJson, GSD_CURSOR_HOOK_MARKER } = require('../bin/install.js');
const result = writeCursorHooksJson(targetDir, srcDir, {});
assert.ok(result, 'writeCursorHooksJson must return a result');
const hooksJsonPath = path.join(targetDir, 'hooks.json');
assert.ok(fs.existsSync(hooksJsonPath), 'hooks.json must be created in targetDir');
const parsed = JSON.parse(fs.readFileSync(hooksJsonPath, 'utf8'));
const hookTable = (parsed.hooks && typeof parsed.hooks === 'object') ? parsed.hooks : parsed;
// Both cursor hook entries must be present.
assert.ok(
Array.isArray(hookTable.sessionStart) && hookTable.sessionStart.length > 0,
'hooks.json must contain a sessionStart entry (gsd-cursor-session-start.js)'
);
assert.ok(
Array.isArray(hookTable.postToolUse) && hookTable.postToolUse.length > 0,
'hooks.json must contain a postToolUse entry (gsd-cursor-post-tool.js)'
);
// Both entries must be GSD-managed (not user-authored stubs).
assert.equal(
hookTable.sessionStart[0][GSD_CURSOR_HOOK_MARKER], true,
'sessionStart entry must carry the GSD managed marker'
);
assert.equal(
hookTable.postToolUse[0][GSD_CURSOR_HOOK_MARKER], true,
'postToolUse entry must carry the GSD managed marker'
);
// Both commands must reference the correct script filenames.
const sessionCmd = hookTable.sessionStart[0].command || '';
const postToolCmd = hookTable.postToolUse[0].command || '';
assert.ok(
sessionCmd.includes('gsd-cursor-session-start.js'),
`sessionStart command must reference gsd-cursor-session-start.js, got: ${sessionCmd}`
);
assert.ok(
postToolCmd.includes('gsd-cursor-post-tool.js'),
`postToolUse command must reference gsd-cursor-post-tool.js, got: ${postToolCmd}`
);
});
});