Skip to content

fix(install): derive the hook list from what ships, and detect unloadable hooks - #72

Open
liveoakwag wants to merge 1 commit into
cytostack:mainfrom
liveoakwag:fix/updater-drops-hook-dependency
Open

fix(install): derive the hook list from what ships, and detect unloadable hooks#72
liveoakwag wants to merge 1 commit into
cytostack:mainfrom
liveoakwag:fix/updater-drops-hook-dependency

Conversation

@liveoakwag

Copy link
Copy Markdown

dist/hooks/ ships eleven files. init and update each copy ten, and status checks seven — three separately-written lists that had drifted apart. The file missing from all three is symbol-extractor.js, which post-write.js imports.

ESM resolves imports at load time, so from the moment of an upgrade post-write.js throws on every invocation:

Error [ERR_MODULE_NOT_FOUND]: Cannot find module '.../symbol-extractor.js'
    imported from '.../post-write.js'

Nothing is recorded in anatomy.md, memory.md or _session.json for the entire window, in every upgraded project. It fails to stderr, so there is no visible symptom — and openwolf status printed "✓ All 7 hook scripts present" throughout, because its list was the shortest of the three. Observed upgrading 1.0.4 → 2.0.1 across three projects; copying the one file in by hand fixed it immediately.

I raised this in a comment on #66 back on August 4 and called it more urgent than anything in that PR. It should have been its own PR then rather than a comment — filing it properly now.

The change

Rather than adding the name to three lists, the list is derived from the shipped directory, so it cannot drift from the build again. The literal in hook-files.ts is only a fallback for when that directory can't be read, and a test pins it to the contents of dist/hooks.

update and status additionally verify that every relative import in the installed hooks resolves to a file that is present — catching the whole class rather than this instance. status now says "present and loadable", since "present" was exactly the message that hid this.

The check is deliberately static. Importing each hook is the obvious way to prove it loads, but every hook calls main() at module scope, so an import would execute it — read stdin, write to the project. node --check is no help either: it parses without resolving imports, which is precisely why the broken hook passed a syntax check cleanly and stayed invisible. There is a test asserting the checker does not execute what it inspects.

Verification

  • node --test tests/hook-files.test.ts9/9
  • npm test35/35, 0 fail
  • npx tsc --noEmit — clean apart from the pre-existing src/daemon/cron-engine.ts(52,27) TS2503
  • git am onto pristine main (f64e737) applies cleanly, suite still 35/35
  • Control: removing symbol-extractor.js from the list again makes the suite fail, so the test reproduces the original defect

Independent of #66, #69, #70 and #71.

🤖 Generated with Claude Code

…able hooks

`dist/hooks/` ships eleven files. `init` and `update` each copied ten, and
`status` checked seven. The three lists were written out separately and had
drifted apart. The file missing from all three was `symbol-extractor.js`, which
`post-write.js` imports.

ESM resolves imports at load time, so from the moment of an upgrade
`post-write.js` threw on every invocation:

  Error [ERR_MODULE_NOT_FOUND]: Cannot find module '.../symbol-extractor.js'
      imported from '.../post-write.js'

Nothing was recorded in anatomy.md, memory.md or _session.json for the entire
window, in every upgraded project. It fails to stderr, so there is no visible
symptom - and `openwolf status` printed "✓ All 7 hook scripts present"
throughout, because its list was the shortest of the three. Observed upgrading
1.0.4 -> 2.0.1 across three projects; copying the one file in by hand fixed it
immediately.

Rather than adding the name to three lists, the list is now derived from the
shipped directory, so it cannot drift from the build again. The literal in
hook-files.ts remains only as a fallback for when that directory cannot be read,
and a test pins it to the contents of dist/hooks.

`update` and `status` additionally verify that every relative import in the
installed hooks resolves to a file that is present, which catches the whole class
rather than this instance.

That check is deliberately STATIC. Importing each hook is the obvious way to
prove it loads, but every hook calls `main()` at module scope, so an import would
execute it - read stdin, write to the project. `node --check` is no help either:
it parses without resolving imports, which is exactly why the broken hook passed
a syntax check cleanly and stayed invisible. A test asserts the check does not
execute what it inspects.

  node --test tests/hook-files.test.ts   9/9
  npm test                               35/35
  npx tsc --noEmit                       clean except the pre-existing
                                         src/daemon/cron-engine.ts TS2503,
                                         present on an unmodified checkout too

Removing `symbol-extractor.js` from the list again makes the suite fail, so the
test reproduces the original defect rather than merely passing.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant