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
Open
Conversation
…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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1817 (opened by @catchingknives).
I verified everything against a clean clone at
58381b3before 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 ahookskey.Here's what this PR changes:
1. Registered the 6 orphan hooks
VersionDrift,ModelRungGuard,TimeContext,AtlasEventCapture,KnowledgeWriteGuard, andSpendAuditorwere absent fromhooks/hooks.jsonand imported by no sibling dispatcher, meaning they ran zero times on every install.Rather than deleting any—especially
AtlasEventCaptureandTimeContext, 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:
AtlasEventCaptureneeded a newBashmatcher bucket underPostToolUse—this is the only structural addition to the manifest, as everything else joined existing groups.SpendAuditorunblocksReflect.ts’swithin_budgetcheck, sinceSpendAuditoris the sole writer forspend-audit.jsonl.2. Fixed raw
{{PRINCIPAL_NAME}}leaking to model contextSince 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 ingetPrincipalName()fromlib/identitywrapped in a generic fallback so it honors its contract to never fail a prompt.Tested end-to-end against a synthetic off-pin transcript:
…do not ask {{PRINCIPAL_NAME}} to change /model.…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
HookHealerso we don't run into dead hooks again. (Note: The 154 changed lines inHookHealerinclude a refactor of the settings walk to return(event, command)pairs parsed once and shared with the existing exec-bit lane, changing thedirectExecPaths()signature. The new check is cheap: just one directory listing plus one read per hook file.)TRIGGER:headers against actual registrations to catch partial wiring.Ran four sanity checks against simulated installs:
4. Docs note (intentionally not included in this PR)
I avoided editing
hooks/README.mdandHookSystem.mdin this PR since upstream releases overwrite repo-level edits. However, for when you sync upstream:.hook.tsfiles on disk (51 with.sh), 36 registered directly, 14 dispatcher-imported (missingISAStaleWriteGuard,ISAFoldGate,LoopDetectorfrom the docs dispatcher list).KnowledgeWriteGuardwas never registered on2026-07-29as claimed in the docs—this PR is its first actual registration.