Skip to content

fix: register six orphaned hooks, render runtime {{PRINCIPAL_NAME}}, add orphan check lane to HookHealer (#1817) - #1821

Open
firaslamouchi21 wants to merge 1 commit into
danielmiessler:mainfrom
firaslamouchi21:main
Open

fix: register six orphaned hooks, render runtime {{PRINCIPAL_NAME}}, add orphan check lane to HookHealer (#1817) #1821
firaslamouchi21 wants to merge 1 commit into
danielmiessler:mainfrom
firaslamouchi21:main

Conversation

@firaslamouchi21

Copy link
Copy Markdown

Fixes #1817 (opened by @catchingknives).

I verified everything against a clean clone at 58381b3 before touching anything, and all three defects reproduced right away: six hooks were dead on disk, raw template placeholders were getting passed to live model context, and neither settings file had a hooks key.

Here's what this PR changes:

1. Registered the 6 orphan hooks

VersionDrift, ModelRungGuard, TimeContext, AtlasEventCapture, KnowledgeWriteGuard, and SpendAuditor were absent from hooks/hooks.json and imported by no sibling dispatcher, meaning they ran zero times on every install.

Rather than deleting any—especially AtlasEventCapture and TimeContext, which were flagged in the issue as potential design questions—I registered all six based on the exact events, positions, and timeouts already outlined in the docs. Tearing out docs-sanctioned subsystem features felt like a maintainer call, and registering them keeps things aligned with the documented behavior (and is easily reverted if you'd rather cut them).

Mechanical details:

  • AtlasEventCapture needed a new Bash matcher bucket under PostToolUse—this is the only structural addition to the manifest, as everything else joined existing groups.
  • Registering SpendAuditor unblocks Reflect.ts’s within_budget check, since SpendAuditor is the sole writer for spend-audit.jsonl.

2. Fixed raw {{PRINCIPAL_NAME}} leaking to model context

Since template substitution is just an install-time pass on disk, runtime strings generated on the fly never got rendered.

  • PromptProcessing.hook.ts (lines 708, 721): Swapped the raw token for the ${PRINCIPAL_NAME} constant already in scope at line 682. This was leaking on live system prompts on every install.
  • ModelRungGuard.hook.ts (line 168): Pulled in getPrincipalName() from lib/identity wrapped in a generic fallback so it honors its contract to never fail a prompt.

Tested end-to-end against a synthetic off-pin transcript:

  • Before: …do not ask {{PRINCIPAL_NAME}} to change /model.
  • After: …do not ask Daniel to change /model.

Grepping all 51 hook files for the literal token outside comments now returns zero hits.

3. Added an orphan check lane to HookHealer (1.1.0)

Added a lightweight check to HookHealer so we don't run into dead hooks again. (Note: The 154 changed lines in HookHealer include a refactor of the settings walk to return (event, command) pairs parsed once and shared with the existing exec-bit lane, changing the directExecPaths() signature. The new check is cheap: just one directory listing plus one read per hook file.)

  • Warning only: It never auto-registers hooks, since deciding when a hook fires should be a human call.
  • Per-event, not per-filename: Keys registrations by event, so a hook wired to one event while its header declares three doesn't read as correctly wired. A second lane compares explicit TRIGGER: headers against actual registrations to catch partial wiring.
  • Dispatcher-aware: Explicitly ignores the 14 dispatcher-imported hooks so it doesn't trigger false positives.

Ran four sanity checks against simulated installs:

  • Pre-fix manifest → flags the exact 6 orphans.
  • Post-fix manifest → exit code 0, completely silent.
  • Un-registering a hook → single targeted warning.
  • Event mismatch → catches header vs manifest mismatch properly.

4. Docs note (intentionally not included in this PR)

I avoided editing hooks/README.md and HookSystem.md in this PR since upstream releases overwrite repo-level edits. However, for when you sync upstream:

  • The 4 hooks documented as "live" are now actually live, making the existing README tables and wiring diagrams accurate as written.
  • Disk count breakdown for reference: 50 .hook.ts files on disk (51 with .sh), 36 registered directly, 14 dispatcher-imported (missing ISAStaleWriteGuard, ISAFoldGate, LoopDetector from the docs dispatcher list).
  • KnowledgeWriteGuard was never registered on 2026-07-29 as claimed in the docs—this PR is its first actual registration.

…o the model (danielmiessler#1817)

Turns out six of our hooks (ModelRungGuard, AtlasEventCapture, KnowledgeWriteGuard, SpendAuditor, TimeContext, and VersionDrift) were just sitting on disk doing absolutely nothing. They weren't registered in the manifest and weren't imported anywhere, so they ran zero times on every install despite what the docs claimed. Wired them up using the exact events, positions, and timeouts specified in the docs.

Also fixed a sneaky issue where raw {{PRINCIPAL_NAME}} tokens were reaching the model. Since template substitution only runs at install time, runtime strings in PromptProcessing (lines 708, 721) and ModelRungGuard (line 168) were sending unrendered placeholders directly into live system prompts. PromptProcessing now uses the constant already in scope, and ModelRungGuard pulls from getPrincipalName() with a generic fallback so it never fails a prompt.

Finally, updated HookHealer (1.1.0) with an orphan check lane so we don't hit this again. It warns if a hook isn't registered per event, ignores the 14 dispatcher hooks on purpose, and reports zero issues against this fix.
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.

Six hook scripts ship registered nowhere, four documented as live; unrendered {{PRINCIPAL_NAME}} reaches the model from two hooks

1 participant