fix(hooks): hooks-tool-dedupe fails protocol_compliance via top-level hooks block

[Brian Krabach (bkrabach)]

Root cause

Confirmed at amplifier-core 1.6.1, python/amplifier_core/loader.py:

• ModuleLoader._get_module_metadata() (loader.py:504-566) tries an explicit __amplifier_module_type__ attribute on the module first; when absent (true of every in-tree foundation hook module — none declare it), it falls back to _guess_from_naming() (loader.py:568-602).

• _guess_from_naming() does a first-match substring scan over a fixed keyword insertion order (loader.py:586-599): orchestrat, loop, provider, tool, hook, context.

• hooks-tool-dedupe contains both "tool" and "hook" as substrings. Because "tool" is checked before "hook" in that dict, the module was classified as type="tool" and validated by ToolValidator instead of HookValidator.

• ToolValidator._check_protocol_compliance (validation/tool.py:277-284) requires mount() to either populate coordinator.mount_points["tools"] or return an object with .name/.description/.execute. hooks-tool-dedupe.mount() correctly does neither (it registers a tool:post hook handler and returns a plain descriptor dict, exactly like its 5 working siblings) — so validation fails with exactly the reported error:

  ModuleValidationError: protocol_compliance: No tool was mounted and mount() did not return a Tool instance
  

Empirical proof (against amplifier-core 1.6.1's real loader):

>>> loader._guess_from_naming('hooks-tool-dedupe')
('tool', 'tools')
>>> loader._guess_from_naming('hooks-dedupe')
('hook', 'hooks')

None of the 5 working siblings (hooks-deprecation, hooks-process-guard, hooks-progress-monitor, hooks-session-naming, hooks-todo-display) contain a keyword the scan checks before "hook", which is why only this module was affected — and why it affects both composition paths that reference it (the top-level hooks: block and behaviors/agents.yaml's hooks: block): both resolve the module id through the identical loader.load() -> _validate_module() -> _get_module_metadata() call chain, so there is only one code path to fix, not two.

Fix

Renamed the module hooks-tool-dedupe -> hooks-dedupe rather than touching amplifier-core's naming heuristic — the heuristic is a documented, working fallback that every other hook module in this repo relies on correctly; this module's own name was the defect (per the discovered-root-cause guidance: prefer a rename when a name trips a core name-heuristic, over changing core).

Changed, via git mv + reference updates (no functional/behavioral changes):

• modules/hooks-tool-dedupe/ -> modules/hooks-dedupe/
• Python package amplifier_module_hooks_tool_dedupe -> amplifier_module_hooks_dedupe
• pyproject.toml: package name, entry-point name (hooks-tool-dedupe -> hooks-dedupe), wheel package list
• uv.lock: self-referential package name
• Internal ToolDedupeHook class -> DedupeHook (not part of the classification surface, but kept consistent with the module's new name)
• mount() registration name, contributor-channel name, and returned descriptor's "name" field
• behaviors/agents.yaml: module id and git source subdirectory= path
• The module's own 25 tests (imports, class name, assertions on registered/returned names)

Not a core bug in the sense of needing a core fix: the naming heuristic works correctly for every other hook module in this repo. It's a real footgun worth flagging upstream, though — see "Core follow-up" below.

Regression test

Added tests/test_hook_module_classification.py (new — no loader-classification test existed in foundation before this):

• test_hook_module_classifies_as_hook — parametrized across every hooks-* module directory (discovered dynamically), calls amplifier-core's real ModuleLoader._get_module_metadata() against each module's actual on-disk path and asserts type == "hook". This is the generic guard against a future hook module picking a name with a colliding keyword.
• test_hooks_dedupe_passes_real_validation — the specific end-to-end reproduction of the reported bug: calls the real ModuleLoader._validate_module() (the exact call site inside load() that raised ModuleValidationError) against hooks-dedupe's real on-disk module, and asserts it does not raise. This exercises the actual classification -> validator-selection -> mount() chain, unlike the module's existing tests (which call mount() directly against a fake coordinator and would not have caught this class of bug, since the bug is in classification/validator-selection before mount() is ever reached).

Smoke-test evidence (the original failure, reproduced pre-fix)

$ uv run python -c "
from amplifier_core.loader import ModuleLoader
loader = ModuleLoader()
print(loader._guess_from_naming('hooks-tool-dedupe'))
print(loader._guess_from_naming('hooks-dedupe'))
"
('tool', 'tools')   # pre-rename id: misclassified as a tool
('hook', 'hooks')   # post-rename id: classified correctly

Test results

• tests/test_hook_module_classification.py (new, 8 tests): 8 passed
• modules/hooks-dedupe/tests/ (existing 25 tests, updated for rename): 25 passed
• Full repo suite (uv run pytest tests/ -q --tb=short, matches CI's .github/workflows/ci.yml invocation): 1642 passed, 1 skipped (1634 pre-existing + 8 new)
• python_check (ruff format/lint, pyright, stub-check) on all changed files: clean

Core follow-up (not fixed here, per scope)

ModuleLoader._guess_from_naming()'s first-match-wins keyword scan (loader.py:586-599) is a latent footgun for any hook (or context/provider) module whose id happens to contain "tool", "loop", "provider", or "orchestrat" as a substring before "hook"/"context" is reached — this PR works around it for this one module by renaming, but the underlying ambiguity in amplifier-core remains. Worth an upstream fix (e.g. require modules to declare __amplifier_module_type__ explicitly, or make the scan match on hyphen-delimited tokens instead of raw substrings) tracked separately in amplifier-core.

Fixes: openai_improvement-j3t

[Brian Krabach (bkrabach)]

Root cause

amplifier-core's loader _guess_from_naming() does a first-match substring scan over module-type keywords, and checks "tool" before "hook". The module name hooks-tool-dedupe contains the substring "tool", so it was classified as a Tool module and validated against ToolValidator — which it fails, since it implements the hooks protocol, not the tool protocol.

Fix

Renamed the module hooks-tool-dedupe → hooks-dedupe. This is the smallest change that resolves the misclassification without touching amplifier-core's loader logic (out of scope for this repo/PR).

Tests added

• New regression test asserting all hooks-* modules in this repo classify as hooks via _guess_from_naming() (prevents recurrence for any future hook module whose name happens to contain another type's keyword).
• End-to-end _validate_module() reproduction test covering the renamed module.

Result: 1642 passed (1634 existing + 8 new).

Found via DTU smoke validation (protocol_compliance failure on hooks-tool-dedupe).

Merge note

Self-authored PR; self-approval is not possible for this account, so this is being merged via admin override at explicit user direction, consistent with the same documented precedent applied to foundation #317–#325 this week.