diff --git a/src/cli/hook-files.ts b/src/cli/hook-files.ts new file mode 100644 index 00000000..5fe2b647 --- /dev/null +++ b/src/cli/hook-files.ts @@ -0,0 +1,89 @@ +import * as fs from "node:fs"; +import * as path from "node:path"; + +/** + * Which hook files an install consists of. + * + * This used to be a literal list, written out three times — in `init`, in `update`, and in + * `status` — and the three had drifted apart. `dist/hooks/` ships eleven files; `init` and + * `update` each copied ten, and `status` checked seven. The missing one was + * `symbol-extractor.js`, which `post-write.js` imports. + * + * The consequence was total and silent: ESM resolves imports at load time, so `post-write.js` + * threw `ERR_MODULE_NOT_FOUND` on *every* invocation from the moment of the upgrade. Nothing + * was recorded in `anatomy.md`, `memory.md` or `_session.json` for the entire window, in every + * upgraded project, and it fails to stderr so there is no visible symptom. `openwolf status` + * reported "✓ All 7 hook scripts present" throughout, because its list was the shortest of + * the three. + * + * Reading the shipped directory removes the possibility of drift rather than correcting one + * instance of it: whatever the build emits is what gets installed and what gets checked. The + * literal below is only a fallback for when the shipped directory cannot be read. + */ +export const HOOK_FILES = [ + "anatomy-lock.js", + "anatomy-store.js", + "post-read.js", + "post-write.js", + "pre-read.js", + "pre-write.js", + "precompact.js", + "session-start.js", + "shared.js", + "stop.js", + "symbol-extractor.js", +] as const; + +/** The hook files actually shipped in `sourceDir`, or the fallback list if it cannot be read. */ +export function shippedHookFiles(sourceDir?: string): string[] { + if (sourceDir) { + try { + const found = fs + .readdirSync(sourceDir) + .filter((f) => f.endsWith(".js") && !f.endsWith(".map")) + .sort(); + if (found.length > 0) return found; + } catch { + // fall through to the literal + } + } + return [...HOOK_FILES]; +} + +/** + * Every relative import in `hooksDir`'s hooks resolves to a file that is present. + * + * Returns a list of human-readable problems; empty means healthy. + * + * Deliberately a STATIC check rather than importing each hook. Importing them would be the + * obvious way to prove they load, but every hook calls `main()` at module scope — so an + * import would execute it, read stdin, and write to the project. `node --check` is no help + * either: it parses without resolving imports, which is precisely why a missing dependency + * passed a syntax check cleanly and stayed invisible. + */ +export function verifyHookImports(hooksDir: string): string[] { + const problems: string[] = []; + let files: string[]; + try { + files = fs.readdirSync(hooksDir).filter((f) => f.endsWith(".js")); + } catch { + return [`hooks directory is unreadable: ${hooksDir}`]; + } + for (const file of files) { + let src: string; + try { + src = fs.readFileSync(path.join(hooksDir, file), "utf-8"); + } catch { + problems.push(`${file}: unreadable`); + continue; + } + // `import ... from "./x.js"` and `export ... from "./x.js"`, single or double quoted. + for (const m of src.matchAll(/(?:import|export)[^;]*?from\s*["'](\.[^"']+)["']/g)) { + const target = path.resolve(hooksDir, m[1]); + if (!fs.existsSync(target)) { + problems.push(`${file} imports ${m[1]}, which is not installed`); + } + } + } + return problems; +} diff --git a/src/cli/init.ts b/src/cli/init.ts index 8d862e6c..abf97776 100644 --- a/src/cli/init.ts +++ b/src/cli/init.ts @@ -11,6 +11,7 @@ import { registerProject, getRegisteredProjects } from "./registry.js"; import { resolveAgents, detectInstalledAgents } from "../agents/index.js"; import { installSkills } from "../agents/skills.js"; import { newStore, importFromMarkdown, saveStore, loadStore, STORE_FILE, sha256 as storeSha256 } from "../hooks/anatomy-store.js"; +import { shippedHookFiles, verifyHookImports } from "./hook-files.js"; const __filename = fileURLToPath(import.meta.url); const __dirname = path.dirname(__filename); @@ -582,18 +583,8 @@ function copyHookScripts(wolfDir: string): void { } } - const hookFiles = [ - "session-start.js", - "pre-read.js", - "pre-write.js", - "post-read.js", - "post-write.js", - "precompact.js", - "stop.js", - "shared.js", - "anatomy-store.js", - "anatomy-lock.js", - ]; + // Derived from what is actually shipped, so a new hook dependency cannot be left behind. + const hookFiles = shippedHookFiles(sourceDir); let copiedAny = false; if (sourceDir) { diff --git a/src/cli/status.ts b/src/cli/status.ts index 0fb04c35..e5ef784b 100644 --- a/src/cli/status.ts +++ b/src/cli/status.ts @@ -2,6 +2,7 @@ import * as fs from "node:fs"; import * as path from "node:path"; import { findProjectRoot } from "../scanner/project-root.js"; import { readJSON, readText } from "../utils/fs-safe.js"; +import { HOOK_FILES, verifyHookImports } from "./hook-files.js"; export async function statusCommand(): Promise { const projectRoot = findProjectRoot(); @@ -35,17 +36,24 @@ export async function statusCommand(): Promise { } // Hook scripts check - const hookFiles = [ - "session-start.js", "pre-read.js", "pre-write.js", - "post-read.js", "post-write.js", "stop.js", "shared.js", - ]; + // Was a seven-name subset, so it reported "✓ All 7 hook scripts present" while an + // uninstalled dependency made post-write.js fail to load on every invocation. + const hookFiles = [...HOOK_FILES]; const hooksDir = path.join(wolfDir, "hooks"); let hooksMissing = 0; for (const file of hookFiles) { if (!fs.existsSync(path.join(hooksDir, file))) hooksMissing++; } if (hooksMissing === 0) { - console.log(` ✓ All ${hookFiles.length} hook scripts present`); + // Present is not the same as loadable: an unresolvable import kills the hook at load + // time, silently, and "all present" is exactly the message that hid it. + const problems = verifyHookImports(hooksDir); + if (problems.length > 0) { + console.log(` ✗ ${problems.length} hook(s) cannot load — unresolved imports:`); + for (const p of problems) console.log(` - ${p}`); + } else { + console.log(` ✓ All ${hookFiles.length} hook scripts present and loadable`); + } } else { console.log(` ✗ Missing ${hooksMissing} hook scripts`); } diff --git a/src/cli/update.ts b/src/cli/update.ts index b7191f55..2ae9d7ae 100644 --- a/src/cli/update.ts +++ b/src/cli/update.ts @@ -10,6 +10,7 @@ import * as fs from "node:fs"; import * as path from "node:path"; import { fileURLToPath } from "node:url"; +import { shippedHookFiles, verifyHookImports } from "./hook-files.js"; import { getRegisteredProjects, registerProject, type RegisteredProject } from "./registry.js"; import { readJSON, writeJSON, readText, writeText, safeCopyFile } from "../utils/fs-safe.js"; import { ensureDir } from "../utils/paths.js"; @@ -450,11 +451,9 @@ function copyHookScripts(wolfDir: string): void { } } - const hookFiles = [ - "session-start.js", "pre-read.js", "pre-write.js", - "post-read.js", "post-write.js", "precompact.js", "stop.js", "shared.js", - "anatomy-store.js", "anatomy-lock.js", - ]; + // Derived from what is actually shipped, so the copy list cannot drift from the build + // again. It had: this list omitted symbol-extractor.js, which post-write.js imports. + const hookFiles = shippedHookFiles(sourceDir); if (sourceDir) { for (const file of hookFiles) { @@ -468,6 +467,15 @@ function copyHookScripts(wolfDir: string): void { // Always ensure package.json with type:module const hooksPkgPath = path.join(hooksDir, "package.json"); fs.writeFileSync(hooksPkgPath, JSON.stringify({ type: "module" }, null, 2) + "\n", "utf-8"); + + // A hook with an unresolvable import throws at LOAD time, on every invocation, straight to + // stderr — so the failure is both total and invisible. Say so here rather than let a user + // discover it weeks later from an empty anatomy.md. + const problems = verifyHookImports(hooksDir); + if (problems.length > 0) { + console.log(` ✗ ${problems.length} hook dependency problem(s) — hooks will fail to load:`); + for (const p of problems) console.log(` - ${p}`); + } } function replaceOpenWolfHooks( diff --git a/tests/hook-files.test.ts b/tests/hook-files.test.ts new file mode 100644 index 00000000..d3ec97d4 --- /dev/null +++ b/tests/hook-files.test.ts @@ -0,0 +1,94 @@ +import { test, describe } from "node:test"; +import * as assert from "node:assert"; +import * as fs from "node:fs"; +import * as os from "node:os"; +import * as path from "node:path"; + +import { HOOK_FILES, shippedHookFiles, verifyHookImports } from "../src/cli/hook-files.ts"; + +const tmpDir = () => fs.mkdtempSync(path.join(os.tmpdir(), "wolf-hookfiles-")); + +describe("the install list is derived from what ships", () => { + test("every hook in dist/hooks is in the fallback list", () => { + // The regression that started this: dist shipped 11 hooks, the copy lists named 10, and + // the omitted one was a dependency of post-write.js. Pin the literal to the build. + const shipped = path.resolve(import.meta.dirname, "../dist/hooks"); + if (!fs.existsSync(shipped)) return; // not built in this checkout; the next test covers the logic + const onDisk = fs.readdirSync(shipped).filter((f) => f.endsWith(".js") && !f.endsWith(".map")).sort(); + const missing = onDisk.filter((f) => !(HOOK_FILES as readonly string[]).includes(f)); + assert.deepEqual(missing, [], `shipped but not in HOOK_FILES: ${missing.join(", ")}`); + }); + + test("shippedHookFiles reads the directory rather than a hardcoded list", () => { + const dir = tmpDir(); + for (const f of ["a.js", "b.js", "brand-new-dependency.js", "notes.md", "x.js.map"]) + fs.writeFileSync(path.join(dir, f), "", "utf-8"); + const found = shippedHookFiles(dir); + assert.deepEqual(found, ["a.js", "b.js", "brand-new-dependency.js"].sort()); + assert.ok(found.includes("brand-new-dependency.js"), "a newly added hook must be picked up"); + fs.rmSync(dir, { recursive: true, force: true }); + }); + + test("falls back to the literal when the directory is unreadable or empty", () => { + assert.deepEqual(shippedHookFiles(path.join(os.tmpdir(), "does-not-exist-xyz")), [...HOOK_FILES]); + assert.deepEqual(shippedHookFiles(undefined), [...HOOK_FILES]); + const empty = tmpDir(); + assert.deepEqual(shippedHookFiles(empty), [...HOOK_FILES]); + fs.rmSync(empty, { recursive: true, force: true }); + }); + + test("symbol-extractor.js is in the list", () => { + // The specific omission. Named explicitly so a future edit cannot quietly drop it again. + assert.ok((HOOK_FILES as readonly string[]).includes("symbol-extractor.js")); + }); +}); + +describe("an uninstalled dependency is detected instead of failing silently", () => { + test("reports a hook whose import is not installed", () => { + const dir = tmpDir(); + fs.writeFileSync(path.join(dir, "post-write.js"), + 'import { extractSymbols } from "./symbol-extractor.js";\nmain();\n', "utf-8"); + const problems = verifyHookImports(dir); + assert.equal(problems.length, 1, JSON.stringify(problems)); + assert.match(problems[0], /post-write\.js imports \.\/symbol-extractor\.js/); + fs.rmSync(dir, { recursive: true, force: true }); + }); + + test("silent once the dependency is installed", () => { + const dir = tmpDir(); + fs.writeFileSync(path.join(dir, "post-write.js"), + 'import { extractSymbols } from "./symbol-extractor.js";\n', "utf-8"); + fs.writeFileSync(path.join(dir, "symbol-extractor.js"), "export function extractSymbols() {}\n", "utf-8"); + assert.deepEqual(verifyHookImports(dir), []); + fs.rmSync(dir, { recursive: true, force: true }); + }); + + test("bare specifiers are left alone — only relative imports are ours to install", () => { + const dir = tmpDir(); + fs.writeFileSync(path.join(dir, "h.js"), + 'import * as fs from "node:fs";\nimport cron from "node-cron";\n', "utf-8"); + assert.deepEqual(verifyHookImports(dir), []); + fs.rmSync(dir, { recursive: true, force: true }); + }); + + test("catches re-exports too, and both quote styles", () => { + const dir = tmpDir(); + fs.writeFileSync(path.join(dir, "h.js"), "export { x } from './gone.js';\n", "utf-8"); + const problems = verifyHookImports(dir); + assert.equal(problems.length, 1, JSON.stringify(problems)); + assert.match(problems[0], /gone\.js/); + fs.rmSync(dir, { recursive: true, force: true }); + }); + + test("does not execute the hooks it checks", () => { + // Importing each hook would be the obvious way to prove it loads — but every hook calls + // main() at module scope, so an import would run it. This must stay a static check. + const dir = tmpDir(); + const sentinel = path.join(dir, "SIDE_EFFECT"); + fs.writeFileSync(path.join(dir, "h.js"), + `import * as fs from "node:fs";\nfs.writeFileSync(${JSON.stringify(sentinel)}, "ran");\n`, "utf-8"); + verifyHookImports(dir); + assert.ok(!fs.existsSync(sentinel), "verifyHookImports executed a hook"); + fs.rmSync(dir, { recursive: true, force: true }); + }); +});