fix(3579): ship graphify hook + lib/ helper through build-hooks + install (#3640)
* fix(3579): ship graphify hook + lib/ helper through build-hooks + install `scripts/build-hooks.js` `HOOKS_TO_COPY` did not include `gsd-graphify-update.sh` (added in #3347 / PR #3557), so it never landed in `hooks/dist/` and `bin/install.js` — which `readdirSync`s the dist — never copied it to `~/.claude/hooks/`. The hook's detached rebuild helper at `hooks/lib/gsd-graphify-rebuild.sh` was also silently dropped because both build-hooks.js (flat allowlist) and bin/install.js (readdir + isFile filter) only walked top-level files. The published tarball gap (Gap 3 in the issue body) is not reproduced on origin/main — `npm pack --dry-run --json` shows both source files are present today. Only Gaps 1 and 2 are in scope. Changes: - Add `gsd-graphify-update.sh` to `HOOKS_TO_COPY`. - Add `HOOKS_SUBDIRS_TO_COPY = ['lib']` and copy whitelisted hook subdirectories (`hooks/<dir>/*` → `hooks/dist/<dir>/*`) in build-hooks.js, with the same syntax-check + atomic-rename path the top-level loop uses. - `bin/install.js`: when copying `hooks/dist/`, recurse one level into any directory entry so subdir files (e.g. `lib/gsd-graphify-rebuild.sh`) land at the mirrored target path the hook's REBUILD_SCRIPT lookup expects. Top-level if/else structure for files is unchanged. Regression test `tests/bug-3579-graphify-hook-publish.test.cjs`: - Drift guard: every top-level `hooks/*.sh` must appear in `HOOKS_TO_COPY`. Generalizes beyond graphify so the next .sh hook added cannot regress. - After build: `hooks/dist/gsd-graphify-update.sh` AND `hooks/dist/lib/gsd-graphify-rebuild.sh` exist. - After install: both files land at the target, and no "Missing expected hook: gsd-graphify-update.sh" warning is emitted. Fixes #3579 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(3579): replace source-grep drift guard with filesystem-behavior assertion The Gap-1 drift guard read scripts/build-hooks.js as text and regex-parsed the HOOKS_TO_COPY literal, which tripped lint-no-source-grep and is brittle under refactors. Replace with a behavior-based assertion: run the build, then for every top-level hooks/*.sh assert hooks/dist/<name> exists. Strictly stronger — catches both the original allowlist gap and any future regression that silently drops a hook for any other reason. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Tom Boucher
5
.changeset/fix-3579-graphify-hook-publish.md
Normal file
5
.changeset/fix-3579-graphify-hook-publish.md
Normal file
@@ -0,0 +1,5 @@
|
||||
---
|
||||
type: Fixed
|
||||
pr: 3579
|
||||
---
|
||||
**Graphify auto-update hook now ships to install targets** — `gsd-graphify-update.sh` was missing from `scripts/build-hooks.js` `HOOKS_TO_COPY`, so it never landed in `hooks/dist/` and the installer's flat-readdir loop never copied it to `~/.claude/hooks/`. The hook's detached rebuild helper at `hooks/lib/gsd-graphify-rebuild.sh` was also dropped because both `build-hooks.js` and `bin/install.js` only walked top-level files. Both gaps are fixed: the allowlist now includes the hook, `build-hooks.js` copies whitelisted hook subdirectories into `hooks/dist/`, and `bin/install.js` mirrors hook subdirs to the target. Added a coverage drift guard so every top-level `hooks/*.sh` must be listed in `HOOKS_TO_COPY` going forward (#3579).
|
||||
@@ -8793,6 +8793,27 @@ function install(isGlobal, runtime = 'claude', options = {}) {
|
||||
fs.copyFileSync(srcFile, destFile);
|
||||
}
|
||||
}
|
||||
} else if (fs.statSync(srcFile).isDirectory()) {
|
||||
// #3579: recurse one level into hook subdirs (lib/ etc.). The
|
||||
// graphify auto-update hook's rebuild helper lives at
|
||||
// hooks/dist/lib/gsd-graphify-rebuild.sh and must land at the
|
||||
// mirrored target path so the hook's REBUILD_SCRIPT lookup resolves.
|
||||
const subDest = path.join(hooksDest, entry);
|
||||
fs.mkdirSync(subDest, { recursive: true });
|
||||
const subEntries = fs.readdirSync(srcFile);
|
||||
for (const subEntry of subEntries) {
|
||||
const subSrcFile = path.join(srcFile, subEntry);
|
||||
if (!fs.statSync(subSrcFile).isFile()) continue;
|
||||
const subDestFile = path.join(subDest, subEntry);
|
||||
if (subEntry.endsWith('.sh')) {
|
||||
let content = fs.readFileSync(subSrcFile, 'utf8');
|
||||
content = content.replace(/\{\{GSD_VERSION\}\}/g, pkg.version);
|
||||
fs.writeFileSync(subDestFile, content);
|
||||
try { fs.chmodSync(subDestFile, 0o755); } catch (e) { /* Windows */ }
|
||||
} else {
|
||||
fs.copyFileSync(subSrcFile, subDestFile);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
if (verifyInstalled(hooksDest, 'hooks')) {
|
||||
|
||||
@@ -36,9 +36,18 @@ const HOOKS_TO_COPY = [
|
||||
// Community hooks (bash, opt-in via .planning/config.json hooks.community)
|
||||
'gsd-session-state.sh',
|
||||
'gsd-validate-commit.sh',
|
||||
'gsd-phase-boundary.sh'
|
||||
'gsd-phase-boundary.sh',
|
||||
// Graphify auto-update hook (#3347 / PR #3557 / #3579). Opt-in via
|
||||
// .planning/config.json graphify.auto_update; off by default.
|
||||
'gsd-graphify-update.sh'
|
||||
];
|
||||
|
||||
// Subdirectories under hooks/ whose contents must also ship to dist. Each
|
||||
// entry is copied as `hooks/<dir>/*` → `hooks/dist/<dir>/*` so detached
|
||||
// helpers (e.g. hooks/lib/gsd-graphify-rebuild.sh) resolve from the hook's
|
||||
// installed runtime path. See #3579.
|
||||
const HOOKS_SUBDIRS_TO_COPY = ['lib'];
|
||||
|
||||
// Sync millisecond sleep using Atomics.wait on a throwaway SharedArrayBuffer.
|
||||
// Used between Windows rename retries; this script is sync end-to-end so
|
||||
// setTimeout would not work. Total worst-case backoff across MAX_ATTEMPTS
|
||||
@@ -169,6 +178,37 @@ function build() {
|
||||
renameAtomicWithRetry(stagedDest, dest, hook);
|
||||
}
|
||||
|
||||
// Copy whitelisted hook subdirectories (e.g. hooks/lib/) into dist so the
|
||||
// installer's readdir-and-isFile loop in bin/install.js sees them and
|
||||
// detached hook helpers resolve from the installed runtime path (#3579).
|
||||
for (const subdir of HOOKS_SUBDIRS_TO_COPY) {
|
||||
const srcDir = path.join(HOOKS_DIR, subdir);
|
||||
if (!fs.existsSync(srcDir)) continue;
|
||||
const destDir = path.join(DIST_DIR, subdir);
|
||||
fs.mkdirSync(destDir, { recursive: true });
|
||||
const entries = fs.readdirSync(srcDir, { withFileTypes: true });
|
||||
for (const ent of entries) {
|
||||
if (!ent.isFile()) continue;
|
||||
const srcFile = path.join(srcDir, ent.name);
|
||||
const destFile = path.join(destDir, ent.name);
|
||||
if (ent.name.endsWith('.js')) {
|
||||
const syntaxError = validateSyntax(srcFile);
|
||||
if (syntaxError) {
|
||||
console.error(`\x1b[31m✗ ${subdir}/${ent.name}: SyntaxError — ${syntaxError}\x1b[0m`);
|
||||
hasErrors = true;
|
||||
continue;
|
||||
}
|
||||
}
|
||||
console.log(`\x1b[32m✓\x1b[0m Copying ${subdir}/${ent.name}...`);
|
||||
const stagedDest = path.join(STAGE_DIR, `${subdir}__${ent.name}.${Date.now()}`);
|
||||
fs.copyFileSync(srcFile, stagedDest);
|
||||
if (ent.name.endsWith('.sh')) {
|
||||
try { fs.chmodSync(stagedDest, 0o755); } catch (e) { /* Windows */ }
|
||||
}
|
||||
renameAtomicWithRetry(stagedDest, destFile, `${subdir}/${ent.name}`);
|
||||
}
|
||||
}
|
||||
|
||||
// Best-effort cleanup of this process's own staging dir. Since STAGE_DIR
|
||||
// is per-PID (`.dist-staging-<pid>/`), no other builder touches it — so
|
||||
// rmSync with recursive:true is safe and leaves no race window.
|
||||
|
||||
150
tests/bug-3579-graphify-hook-publish.test.cjs
Normal file
150
tests/bug-3579-graphify-hook-publish.test.cjs
Normal file
@@ -0,0 +1,150 @@
|
||||
'use strict';
|
||||
|
||||
/**
|
||||
* Regression tests for #3579 — graphify auto-update hook (#3347 / PR #3557)
|
||||
* was dead-on-arrival in 1.50.0-canary.x because:
|
||||
*
|
||||
* Gap 1: scripts/build-hooks.js HOOKS_TO_COPY did not include
|
||||
* gsd-graphify-update.sh, so it never landed in hooks/dist/ — the
|
||||
* installer's bin/install.js readdir loop then never copied it to
|
||||
* ~/.claude/hooks/.
|
||||
* Gap 2: build-hooks.js (flat allowlist) and bin/install.js (readdir +
|
||||
* isFile filter) never copied hooks/lib/gsd-graphify-rebuild.sh.
|
||||
* Without the helper the hook resolves rebuild script → not found →
|
||||
* exit 0 — feature silently dead.
|
||||
*
|
||||
* Beyond these two gaps the issue body lists a Gap 3 (npm tarball missing
|
||||
* the source files). Inspection of `npm pack --dry-run --json` on origin/main
|
||||
* shows both files are now present in the tarball, so the tarball-side
|
||||
* regression is not reproduced; only Gaps 1 & 2 are in scope here.
|
||||
*
|
||||
* Test strategy — three layers, each independent:
|
||||
* 1. build-hooks.js HOOKS_TO_COPY includes every top-level .sh under hooks/
|
||||
* (allowlist-coverage drift guard). This generalizes beyond graphify so
|
||||
* the next .sh added cannot drift back into the gap.
|
||||
* 2. After running scripts/build-hooks.js, hooks/dist/gsd-graphify-update.sh
|
||||
* and hooks/dist/lib/gsd-graphify-rebuild.sh both exist.
|
||||
* 3. After installing into a temp config dir, both files land at
|
||||
* hooks/gsd-graphify-update.sh and hooks/lib/gsd-graphify-rebuild.sh
|
||||
* and the installer does not emit the "Missing expected hook" warning
|
||||
* for gsd-graphify-update.sh.
|
||||
*/
|
||||
|
||||
const { test, describe, before, after } = 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 { execFileSync } = require('node:child_process');
|
||||
|
||||
const REPO_ROOT = path.resolve(__dirname, '..');
|
||||
const HOOKS_DIR = path.join(REPO_ROOT, 'hooks');
|
||||
const DIST_DIR = path.join(HOOKS_DIR, 'dist');
|
||||
const BUILD_SCRIPT = path.join(REPO_ROOT, 'scripts', 'build-hooks.js');
|
||||
const INSTALL_SCRIPT = path.join(REPO_ROOT, 'bin', 'install.js');
|
||||
|
||||
// ─── Coverage guard ─────────────────────────────────────────────────────────
|
||||
|
||||
describe('#3579 Gap 1: build-hooks.js packages every top-level hooks/*.sh into dist', () => {
|
||||
// Behavior-based drift guard: rather than parsing the HOOKS_TO_COPY literal
|
||||
// out of scripts/build-hooks.js as text (a source-grep that breaks under
|
||||
// harmless refactors and fails to catch any other reason a file might get
|
||||
// dropped on the floor), we run the actual build and assert the actual
|
||||
// filesystem outcome: every top-level hooks/*.sh has a corresponding file
|
||||
// in hooks/dist/. This catches the original gap (missing allowlist entry)
|
||||
// AND any future regression that silently drops a hook for any other
|
||||
// reason (e.g. a copy that swallows errors, a syntax-validator bug, etc.).
|
||||
before(() => {
|
||||
execFileSync(process.execPath, [BUILD_SCRIPT], { encoding: 'utf-8', stdio: 'pipe' });
|
||||
});
|
||||
|
||||
test('every top-level hooks/*.sh is emitted to hooks/dist/ by the build', () => {
|
||||
const topLevelSh = fs
|
||||
.readdirSync(HOOKS_DIR, { withFileTypes: true })
|
||||
.filter((e) => e.isFile() && e.name.endsWith('.sh'))
|
||||
.map((e) => e.name);
|
||||
|
||||
assert.ok(topLevelSh.length > 0, 'expected at least one top-level hooks/*.sh in source');
|
||||
|
||||
const missing = topLevelSh.filter(
|
||||
(sh) => !fs.existsSync(path.join(DIST_DIR, sh))
|
||||
);
|
||||
assert.deepStrictEqual(
|
||||
missing,
|
||||
[],
|
||||
`every top-level hooks/*.sh must be emitted to hooks/dist/ by scripts/build-hooks.js; missing from dist: ${JSON.stringify(missing)}`
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
// ─── build-hooks emits dist/ files ──────────────────────────────────────────
|
||||
|
||||
describe('#3579 Gap 1 + Gap 2: build-hooks.js populates dist with graphify hook + lib helper', () => {
|
||||
before(() => {
|
||||
execFileSync(process.execPath, [BUILD_SCRIPT], { encoding: 'utf-8', stdio: 'pipe' });
|
||||
});
|
||||
|
||||
test('hooks/dist/gsd-graphify-update.sh exists after build', () => {
|
||||
assert.ok(
|
||||
fs.existsSync(path.join(DIST_DIR, 'gsd-graphify-update.sh')),
|
||||
'expected hooks/dist/gsd-graphify-update.sh to exist after build (Gap 1)'
|
||||
);
|
||||
});
|
||||
|
||||
test('hooks/dist/lib/gsd-graphify-rebuild.sh exists after build', () => {
|
||||
assert.ok(
|
||||
fs.existsSync(path.join(DIST_DIR, 'lib', 'gsd-graphify-rebuild.sh')),
|
||||
'expected hooks/dist/lib/gsd-graphify-rebuild.sh to exist after build (Gap 2)'
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
// ─── install lands the files at the target ──────────────────────────────────
|
||||
|
||||
describe('#3579: installer deploys graphify hook + lib helper to target', () => {
|
||||
let tmpDir;
|
||||
let installStdout;
|
||||
|
||||
before(() => {
|
||||
execFileSync(process.execPath, [BUILD_SCRIPT], { encoding: 'utf-8', stdio: 'pipe' });
|
||||
tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-3579-install-'));
|
||||
installStdout = execFileSync(
|
||||
process.execPath,
|
||||
[INSTALL_SCRIPT, '--claude', '--global', '--yes', '--no-sdk'],
|
||||
{
|
||||
encoding: 'utf-8',
|
||||
stdio: 'pipe',
|
||||
env: { ...process.env, CLAUDE_CONFIG_DIR: tmpDir },
|
||||
}
|
||||
);
|
||||
});
|
||||
|
||||
after(() => {
|
||||
if (tmpDir) {
|
||||
try { fs.rmSync(tmpDir, { recursive: true, force: true }); } catch { /* ignore */ }
|
||||
}
|
||||
});
|
||||
|
||||
test('hooks/gsd-graphify-update.sh present at install target', () => {
|
||||
const dest = path.join(tmpDir, 'hooks', 'gsd-graphify-update.sh');
|
||||
assert.ok(fs.existsSync(dest), `expected ${dest} to exist after install`);
|
||||
});
|
||||
|
||||
test('hooks/lib/gsd-graphify-rebuild.sh present at install target', () => {
|
||||
const dest = path.join(tmpDir, 'hooks', 'lib', 'gsd-graphify-rebuild.sh');
|
||||
assert.ok(fs.existsSync(dest), `expected ${dest} to exist after install`);
|
||||
});
|
||||
|
||||
test('installer does not warn about missing gsd-graphify-update.sh', () => {
|
||||
assert.ok(
|
||||
!installStdout.includes('Missing expected hook: gsd-graphify-update.sh'),
|
||||
`installer output must not warn about missing graphify hook; got:\n${installStdout}`
|
||||
);
|
||||
assert.ok(
|
||||
!installStdout.includes(
|
||||
'Skipped graphify auto-update hook — gsd-graphify-update.sh not found'
|
||||
),
|
||||
`installer must not skip graphify hook configuration; got:\n${installStdout}`
|
||||
);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user