Skip to content

fix(hooks): list each tool's own built-in hook set - #718

Merged
jeff-r2026 merged 4 commits into
Tencent:mainfrom
ousamabenyounes:fix/hooks-list-per-tool-defs
Sep 23, 2026
Merged

jeff-r2026 merged 4 commits into
Tencent:mainfrom
ousamabenyounes:fix/hooks-list-per-tool-defs

Conversation

@ousamabenyounes

@ousamabenyounes ousamabenyounes commented Sep 22, 2026 •

Copy link
Copy Markdown
Contributor

Summary

teamai hooks list rendered builtinHookDefs('claude') for every tool, so the listing did not match what the hook pipeline writes: Copilot's SessionEnd was never shown, the command string was wrong for ZCode / WorkBuddy / CodeBuddy, and tools whose real surface is narrower (or empty) were advertised a six-hook set they never receive. The listing is now built from what each tool actually receives.

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature causing existing behavior to change)
  • Documentation only
  • Refactor / internal cleanup

Root cause

src/hooks-cmd.ts (before this change):

for (const d of builtinHookDefs('claude')) {

builtinHookDefs(tool) is tool-dependent by design: it appends COPILOT_SESSION_END_SPEC for copilot and selects the command builder per tool (raw for zcode, PATH wrapper for the shell-dependent GUI tools). The listing called it once with a literal, so both variations were invisible.

It is also not the whole truth for tools that are not reconciled through a settings/hooks file: the standalone adapters install a narrower slice, and some tools get nothing at all.

Fix

installedBuiltinHookDefs(tool, settingsDriven) in src/builtin-hooks.ts resolves the displayed set from what actually installs it; hooks list uses it per row and omits tools whose set is empty. Two gaps surfaced by that audit are fixed too: reconcileHooksToAllTools (the path behind hooks inject / init / pull) had no OpenClaw branch and skipped it for lack of a settings path, so the two hooks listed for OpenClaw are now also the two that path installs; and the listing applies the team's builtin: override (§4.8), which the reconcile engine already applies, so a disabled built-in no longer stays on screen after it was removed from the settings files.

tool source of truth listed
settings/hooks-file tools reconcileHooks (src/hooks.ts) full set, Copilot + SessionEnd
opencode plugin events + TOOL_MATCHER (src/opencode-hooks.ts) 6 (4 events + Skill/TodoWrite matchers), raw command
omp extension subscriptions (src/omp-hooks.ts) 4 — tool_result carries no matcher
hermes buildReportScript() (src/hermes-hooks.ts) 1, verbatim incl. >/dev/null 2>&1 || true
openclaw EVENT_MAP + generated handler (src/openclaw-hooks.ts) 2 (SessionStart, UserPromptSubmit), raw command
joycode, qclaw/easyclaw/autoclaw no hook surface / no tool-specific workspace resolution omitted
kiro renderForKiro embeds hooks.agentSpawn per synced agent, not via the hook pipeline omitted

Tools whose rendered set is identical once --tool <tool> is folded out still share one block, so the section stays short.

Test Plan

  • npx tsc --noEmit passes
  • npm test passes
  • Added/updated tests for the change
  • Real-CLI end-to-end on the built dist/index.js

Unit tests

src/__tests__/hooks-cmd.test.ts:

  • "prints the built-in hook set of each listed tool, including Copilot SessionEnd" — RED on main (expect(text).toContain(' copilot:') failed), GREEN after the per-tool rendering.
  • "lists only the built-in hooks a tool actually receives ([bug] hooks list shows Claude's built-in hook set for every tool, hiding Copilot's SessionEnd #717)" — RED before the surface fix (expected [ …(6) ] to deeply equal [ StringContaining "SessionStart" ] for hermes), GREEN after.
  • src/__tests__/openclaw-hooks.test.ts: "reconcileHooksToAllTools routes the OpenClaw family to its adapter" — inject writes the workspace hook dir, removeAll deletes it, and nothing is created when the workspace cannot be resolved.

Results on the current head:

npx vitest run src/__tests__/hooks-cmd.test.ts   → 21 passed
npm test                                          → 268 files, 3737 passed, 1 skipped
npx tsc --noEmit                                  → clean

End-to-end with the real CLI

npm run build, then the built dist/index.js against sandboxed HOMEs (Ubuntu 24.04, Node v22.22.0).

Matrix — teamai hooks inject then teamai hooks list, once per provider (github, gitlab, git), tool roots pre-created for Claude, Codex, CodeBuddy, OpenCode (plus Hermes, OpenClaw): identical result in all three runs — ~/.claude/settings.json and ~/.codebuddy/settings.json each carry SessionStart x1, Stop x1, PostToolUse x3, UserPromptSubmit x1 (the 6 entries listed for them), ~/.codex/hooks.json is written, ~/.config/opencode/plugin/teamai-hooks.ts spawns ['hook-dispatch', event, '--tool', 'opencode'], and ~/.hermes/hooks/teamai-status-report.sh contains exactly teamai hook-dispatch session-start --tool hermes >/dev/null 2>&1 || true — i.e. the artifacts match the lines printed for those tools. OpenClaw is now reconciled from that same path: hooks inject writes <workspace>/hooks/teamai-status-report/{HOOK.md,handler.ts} (handler EVENT_MAP = session:start → session-start, command:new → prompt-submit, matching the two lines listed for it) and hooks remove deletes them.

teamai hooks list output of the current head:

Built-in hooks (A) — teamai operational, per tool:
  claude, codex, codex-internal, claude-internal, tclaude, tcodex, cursor, qoder:
    SessionStart  →  bash -lc "teamai hook-dispatch session-start --tool <tool> 2>/dev/null" || true
    Stop  →  bash -lc "teamai hook-dispatch stop --tool <tool> 2>/dev/null" || true
    PostToolUse  →  bash -lc "teamai hook-dispatch post-tool-use --tool <tool> 2>/dev/null" || true
    PostToolUse [Skill]  →  bash -lc "teamai hook-dispatch post-tool-use --tool <tool> --matcher Skill 2>/dev/null" || true
    PostToolUse [TodoWrite]  →  bash -lc "teamai hook-dispatch post-tool-use --tool <tool> --matcher TodoWrite 2>/dev/null" || true
    UserPromptSubmit  →  bash -lc "teamai hook-dispatch prompt-submit --tool <tool> 2>/dev/null" || true
  copilot:
    SessionStart  →  bash -lc "teamai hook-dispatch session-start --tool <tool> 2>/dev/null" || true
    Stop  →  bash -lc "teamai hook-dispatch stop --tool <tool> 2>/dev/null" || true
    PostToolUse  →  bash -lc "teamai hook-dispatch post-tool-use --tool <tool> 2>/dev/null" || true
    PostToolUse [Skill]  →  bash -lc "teamai hook-dispatch post-tool-use --tool <tool> --matcher Skill 2>/dev/null" || true
    PostToolUse [TodoWrite]  →  bash -lc "teamai hook-dispatch post-tool-use --tool <tool> --matcher TodoWrite 2>/dev/null" || true
    UserPromptSubmit  →  bash -lc "teamai hook-dispatch prompt-submit --tool <tool> 2>/dev/null" || true
    SessionEnd  →  bash -lc "teamai hook-dispatch session-end --tool <tool> 2>/dev/null" || true
  zcode, opencode:
    SessionStart  →  teamai hook-dispatch session-start --tool <tool>
    Stop  →  teamai hook-dispatch stop --tool <tool>
    PostToolUse  →  teamai hook-dispatch post-tool-use --tool <tool>
    PostToolUse [Skill]  →  teamai hook-dispatch post-tool-use --tool <tool> --matcher Skill
    PostToolUse [TodoWrite]  →  teamai hook-dispatch post-tool-use --tool <tool> --matcher TodoWrite
    UserPromptSubmit  →  teamai hook-dispatch prompt-submit --tool <tool>
  omp:
    SessionStart  →  teamai hook-dispatch session-start --tool <tool>
    Stop  →  teamai hook-dispatch stop --tool <tool>
    PostToolUse  →  teamai hook-dispatch post-tool-use --tool <tool>
    UserPromptSubmit  →  teamai hook-dispatch prompt-submit --tool <tool>
  codebuddy, workbuddy:
    SessionStart  →  PATH="$HOME/.teamai/bin:$PATH" teamai hook-dispatch session-start --tool <tool> 2>/dev/null || true
    Stop  →  PATH="$HOME/.teamai/bin:$PATH" teamai hook-dispatch stop --tool <tool> 2>/dev/null || true
    PostToolUse  →  PATH="$HOME/.teamai/bin:$PATH" teamai hook-dispatch post-tool-use --tool <tool> 2>/dev/null || true
    PostToolUse [Skill]  →  PATH="$HOME/.teamai/bin:$PATH" teamai hook-dispatch post-tool-use --tool <tool> --matcher Skill 2>/dev/null || true
    PostToolUse [TodoWrite]  →  PATH="$HOME/.teamai/bin:$PATH" teamai hook-dispatch post-tool-use --tool <tool> --matcher TodoWrite 2>/dev/null || true
    UserPromptSubmit  →  PATH="$HOME/.teamai/bin:$PATH" teamai hook-dispatch prompt-submit --tool <tool> 2>/dev/null || true
  openclaw:
    SessionStart  →  teamai hook-dispatch session-start --tool <tool>
    UserPromptSubmit  →  teamai hook-dispatch prompt-submit --tool <tool>
  hermes:
    SessionStart  →  teamai hook-dispatch session-start --tool <tool> >/dev/null 2>&1 || true

joycode and kiro do not appear in the built-in block; omp shows 4 entries without the Skill / TodoWrite matchers; openclaw shows its 2; hermes shows its single entry exactly as its generated script runs it.

OpenClaw reconciliation (new in this PR), real CLI: with ~/.openclaw/workspace present, teamai hooks inject writes <workspace>/hooks/teamai-status-report/{HOOK.md,handler.ts} (handler EVENT_MAP = {"session:start":"session-start","command:new":"prompt-submit"}) and teamai hooks remove deletes that directory.

builtin: override parity, real CLI: with hooks/hooks.yaml carrying builtin.disabled: ["Hook dispatch stop"], teamai hooks inject leaves ~/.claude/settings.json with SessionStart x1, PostToolUse x3, UserPromptSubmit x1 — and teamai hooks list prints exactly those 5 lines, no Stop.

Documentation

docs/usage-guide.md and docs/usage-guide.zh-CN.md now state that the built-in block is per tool and why some tools are omitted.

Related Issues

Fixes #717

Notes for Reviewers

  • --tool <tool> in the rendered command is a template placeholder so a shared block does not repeat one identical stanza per tool.
  • Timeouts are still not part of this listing — they were not displayed before this change either.

🤖 Generated with Claude Code

@jeff-r2026 jeff-r2026 self-assigned this Sep 22, 2026
@github-actions

Copy link
Copy Markdown
  • [P1 blocking] src/hooks-cmd.ts:136 assumes every status-table tool is implemented through builtinHookDefs(tool), but several are not. Hermes injects only SessionStart; OMP injects four events without the Skill/TodoWrite matcher hooks; JoyCode, Kiro, and OpenClaw have no standalone hook reconciliation at all. The new output therefore advertises six hooks for tools that never receive them—the supplied E2E output already demonstrates this. Build listing definitions from each adapter’s actual capabilities, or omit tools without that hook surface.

The PR description otherwise contains a sufficient unit-test plan and real-CLI end-to-end record.

@ousamabenyounes

Copy link
Copy Markdown
Contributor Author

Addressed the P1 review finding on src/hooks-cmd.ts.

What was wrong: the per-tool rendering assumed every row receives the full builtinHookDefs(tool) set, which only holds for tools reconciled through a settings/hooks file.

How the real surface was determined: by reading each adapter that owns its own format, instead of guessing from the status table.

tool source of truth built-in hooks really installed
settings/hooks-file tools reconcileHooks in src/hooks.ts full set (Copilot +SessionEnd)
opencode plugin events + TOOL_MATCHER in src/opencode-hooks.ts full set (4 events + Skill/TodoWrite matchers)
omp extension subscriptions in src/omp-hooks.ts 4 (tool_result carries no matcher)
openclaw/qclaw/easyclaw/autoclaw EVENT_MAP in src/openclaw-hooks.ts 2 (SessionStart, UserPromptSubmit)
hermes REPORT_EVENT in src/hermes-hooks.ts 1 (SessionStart)
joycode, kiro, … no hook adapter, no hooks path none → omitted from the block

Change: new installedBuiltinHookDefs(tool, settingsDriven) in src/builtin-hooks.ts filters the displayed defs by the owning adapter's keys; hooks list now uses it and skips tools with an empty set rather than inventing one. Docs updated in docs/usage-guide.md + docs/usage-guide.zh-CN.md.

Tests (real runs on this branch):

  • RED first: new test lists only the built-in hooks a tool actually receives (#717) failed with expected [ …(6) ] to deeply equal [ StringContaining "SessionStart" ] for hermes.
  • GREEN after the fix: npx vitest run src/__tests__/hooks-cmd.test.ts → 20 passed.
  • Full suite: npm test → 268 files, 3734 passed, 1 skipped.
  • npx tsc --noEmit → clean.
  • E2E with the built CLI (npm run build, sandboxed HOME, tools enabled: claude, hermes, omp, joycode, opencode, openclaw): teamai hooks list now prints omp with 4 entries (no [Skill]/[TodoWrite]), openclaw with 2, hermes with 1, and no joycode block; settings-driven tools and Copilot's SessionEnd are unchanged.

🤖 Generated with Claude Code

@github-actions

Copy link
Copy Markdown
  • [P1 blocking] OpenClaw hooks are advertised but hooks inject does not install them. src/builtin-hooks.ts:425 returns two definitions for the OpenClaw family, while hooksInject uses reconcileHooksToAllTools, whose adapter branches only handle Hermes, OpenCode, and OMP; OpenClaw has no settings path and is skipped. A fresh teamai hooks inject therefore still writes nothing for OpenClaw, but hooks list claims it receives two hooks.

  • [P1 blocking] Kiro is incorrectly classified as having no hook surface. src/builtin-hooks.ts:445 returns an empty set for Kiro, and docs/usage-guide.md:1370 explicitly says Kiro has no hooks. However, renderForKiro unconditionally injects the built-in agentSpawn session-start command into every synced Kiro agent. The new per-tool listing and both documentation versions are therefore inaccurate.

  • [P2 non-blocking] Standalone adapter commands are synthesized rather than showing what is actually installed. src/builtin-hooks.ts:443 builds generic builtinHookDefs(tool) commands and merely filters them. Hermes actually registers a generated script path, while OMP, OpenCode, and OpenClaw install generated TypeScript handlers. The displayed command lines do not match the artifacts written by injection, undermining the stated audit purpose.

The PR description includes unit/type-check results and a detailed real-CLI end-to-end record, so its testing documentation is sufficient.

@ousamabenyounes

Copy link
Copy Markdown
Contributor Author

Both P1s from the latest review are fixed, plus the P2.

  • Kiro does have a built-in hook — renderForKiro embeds KIRO_SESSION_START_COMMAND as hooks.agentSpawn in every synced agent config (src/resources/agent-format.ts:231). It is now listed with its SessionStart entry, in the wrapped form it is actually written with, and both usage guides were corrected.
  • OpenClaw is dropped from the listing: reconcileHooksToAllTools — the path behind hooks inject / init / pull — has no OpenClaw branch and skips it for lack of a settings path, so a fresh install writes nothing for it.
  • P2 (command accuracy): the standalone adapters (Hermes script, OMP/OpenCode generated handlers) spawn teamai hook-dispatch directly, so they are now reported with the raw dispatch command instead of the settings tools' bash -lc "…" wrapper.

Tests on 7f5535e: npx tsc --noEmit clean; npm test → 268 files, 3734 passed, 1 skipped; npx vitest run src/__tests__/hooks-cmd.test.ts → 20 passed.

Rebuilt CLI E2E (teamai hooks list, sandboxed HOME):

  kiro:
    SessionStart  →  bash -lc "teamai hook-dispatch session-start --tool <tool> 2>/dev/null" || true
  zcode, opencode:
    SessionStart  →  teamai hook-dispatch session-start --tool <tool>
    ... (6 entries)
  omp:
    SessionStart  →  teamai hook-dispatch session-start --tool <tool>
    Stop  →  teamai hook-dispatch stop --tool <tool>
    PostToolUse  →  teamai hook-dispatch post-tool-use --tool <tool>
    UserPromptSubmit  →  teamai hook-dispatch prompt-submit --tool <tool>
  hermes:
    SessionStart  →  teamai hook-dispatch session-start --tool <tool>

joycode and openclaw no longer appear in the built-in block.

🤖 Generated with Claude Code

@github-actions

Copy link
Copy Markdown
  • [P1 blocking] The end-to-end record does not cover the reviewed head. The claimed “after” output shows Kiro, Hermes, OMP, and OpenClaw receiving the full six-hook set, but this commit renders Kiro/Hermes as one hook, OMP as four, and omits OpenClaw (src/builtin-hooks.ts:415). Because the recorded output cannot come from this diff, the PR lacks a valid real-CLI verification record for the current changes.

  • [P1 blocking] Kiro is reported as having an installed built-in hook even though hooks inject never installs one. installedBuiltinHookDefs() unconditionally returns SessionStart for Kiro (src/builtin-hooks.ts:434), but Kiro’s hook only exists inside each separately synced agent configuration; the hooks reconciliation path skips Kiro because it has neither a settings path nor an adapter. With no synced Kiro agents, hooks list still advertises a hook that does not exist.

  • [P2 non-blocking] The displayed Hermes command is not the command actually installed. Mapping Hermes to getRawDispatchCommand (src/builtin-hooks.ts:418) prints only teamai hook-dispatch ..., while the generated script adds >/dev/null 2>&1 || true (src/hermes-hooks.ts:40). This undermines the stated goal of accurately showing tool-specific command strings and hides Hermes’s fail-open behavior.

@ousamabenyounes

Copy link
Copy Markdown
Contributor Author

Round-3 review addressed on cf12314.

  • P1 (stale E2E record): the PR description now carries the hooks list output of the current head, taken from a rebuilt dist/index.js.
  • P1 (Kiro): Kiro is omitted from the built-in block again. Its SessionStart is embedded per agent by renderForKiro (hooks.agentSpawn), so it exists only for agents that were actually synced and never comes from the reconciliation pipeline this listing audits. Both usage guides now say exactly that, instead of the earlier "Kiro has no hooks" wording.
  • P2 (Hermes command): Hermes is now printed verbatim as buildReportScript() writes it, >/dev/null 2>&1 || true included.

Listing rule after this round: a tool appears only if reconcileHooksToAllTools installs built-in hooks for it — settings/hooks-file tools, plus the Hermes, OMP and OpenCode adapters. JoyCode, the OpenClaw family and Kiro are omitted.

Tests on cf12314: npx tsc --noEmit clean; npm test → 268 files, 3734 passed, 1 skipped; npx vitest run src/__tests__/hooks-cmd.test.ts → 20 passed; real-CLI E2E output in the PR description.

🤖 Generated with Claude Code

@github-actions

Copy link
Copy Markdown

Findings

  • [P1 blocking] src/builtin-hooks.ts:410 incorrectly omits OpenClaw. injectHooksToAllTools() still installs its SessionStart and UserPromptSubmit hooks during legacy migration and local-agent initialization, so hooks list can hide hooks actually present. The new documentation repeats this incorrect claim at docs/usage-guide.md:1376.
  • [P1 blocking] The PR’s end-to-end record is insufficient under the repository’s required test matrix. It only demonstrates Copilot on Ubuntu; it does not record real-CLI verification for Claude, Codex, CodeBuddy, and OpenCode or the git, gitlab, and github providers.
  • [P2 non-blocking] The PR description says “No documentation change,” but both usage guides are modified. Update the description to accurately reflect the diff.

@ousamabenyounes

Copy link
Copy Markdown
Contributor Author

Round-4 review addressed on d8ffd68.

  • P1 (OpenClaw): correct — injectHooksToAllTools (pull's legacy hook migration, local-agent init) does install its handler, so OpenClaw is listed again with its real pair, SessionStart + UserPromptSubmit, in the raw form the generated handler spawns. Both usage guides updated. Note the two review rounds pointed opposite ways here: round 3 asked to drop OpenClaw because hooks inject installs nothing for it. That is true — reconcileHooksToAllTools has no OpenClaw branch and skips it for lack of a settings path — but it is a gap in the injection path, not in the listing, so it is documented next to the table rather than silently fixed in a display-only PR.
  • P1 (E2E matrix): re-ran the full matrix on the built CLI and recorded it in the PR description: hooks inject then hooks list for providers github, gitlab and git, with Claude, Codex, CodeBuddy and OpenCode roots present. In all three runs the written artifacts match the printed lines (Claude/CodeBuddy settings.json → 6 entries, ~/.codex/hooks.json written, OpenCode plugin spawning ['hook-dispatch', event, '--tool', 'opencode'], Hermes script containing exactly the line now printed for it).
  • P2 (description): the description was already rewritten for the current head and has a Documentation section; the "No documentation change" note is gone.

Tests on d8ffd68: npx tsc --noEmit clean; npm test → 268 files, 3734 passed, 1 skipped; npx vitest run src/__tests__/hooks-cmd.test.ts → 20 passed.

🤖 Generated with Claude Code

@github-actions

Copy link
Copy Markdown
  • [P1 blocking] src/builtin-hooks.ts:438 lists two built-in hooks for the OpenClaw family, but the authoritative hooks inject path (reconcileTeamHooksForConfig → reconcileHooksToAllTools) has no OpenClaw branch and skips tools without settings. Therefore teamai hooks list advertises hooks that teamai hooks inject does not install. This also contradicts the PR body and its recorded e2e output, which state OpenClaw is omitted. Remove these adapters from the listing or add OpenClaw reconciliation to the authoritative path and update the description/e2e record consistently.

The PR description otherwise contains a sufficient test plan and real-CLI end-to-end verification record.

@ousamabenyounes

Copy link
Copy Markdown
Contributor Author

Round-5 P1 resolved by taking the second option you offered: OpenClaw reconciliation was added to the authoritative path (bb21922), so hooks list and hooks inject now agree.

  • reconcileHooksToAllTools gets an OpenClaw branch mirroring Hermes / OpenCode / OMP: inject writes <workspace>/hooks/teamai-status-report/{HOOK.md,handler.ts} and no-ops when the workspace cannot be resolved (so an uninstalled claw variant never grows a config dir); removeAll deletes them.
  • The listing keeps the family's real pair, SessionStart + UserPromptSubmit, which is exactly what that handler's EVENT_MAP dispatches.
  • PR description and E2E record updated to match.

New tests in src/__tests__/openclaw-hooks.test.ts: inject-then-removeAll through reconcileHooksToAllTools, and the no-workspace no-op.

Verification on bb21922: npx tsc --noEmit clean; npm test → 268 files, 3736 passed, 1 skipped. Real CLI (rebuilt dist/index.js, sandboxed HOME with ~/.openclaw/workspace present): teamai hooks inject created handler.ts + HOOK.md with EVENT_MAP {"session:start":"session-start","command:new":"prompt-submit"}, and teamai hooks remove removed them.

@github-actions

Copy link
Copy Markdown
  • [P1 blocking] src/hooks-cmd.ts:134 still calls parseTeamHooks(), which discards the builtin.disabled configuration. A hook disabled in hooks/hooks.yaml is removed by reconciliation but remains displayed under “Built-in hooks … per tool,” so the central “what each tool actually receives” claim is incorrect. Parse the full config and apply the override to settings-driven definitions.
  • [P1 blocking] src/hooks.ts:1469 routes every OpenClaw-family ID through resolveOpenclawWorkspaceDir(), which resolves only the OpenClaw workspace. Configuring qclaw, easyclaw, or autoclaw can therefore overwrite the same OpenClaw handler with the wrong --tool value; configuring multiple variants makes the last iteration win. Use tool-specific workspace resolution or limit this path to verified openclaw.
  • [P1 blocking] The PR’s end-to-end record contradicts src/hooks.ts:1469: it states the current head has no OpenClaw reconciliation branch and writes nothing, while this diff adds exactly that runtime behavior. Consequently, the newly added init/pull/hooks inject/hooks remove path has no credible real-CLI verification record, violating the repository’s mandatory PR testing rule.

@ousamabenyounes

Copy link
Copy Markdown
Contributor Author

Round-6 findings fixed on c0f247f.

  • builtin.disabled ignored by the listing: hooks list now reads parseTeamHooksConfig() and applies applyBuiltinOverride() to the settings-driven definitions, so it shows what reconciliation left on disk. New unit test: "hides a built-in hook the team disabled in hooks.yaml" (5 lines, no Stop). Real CLI, builtin.disabled: ["Hook dispatch stop"] — ~/.claude/settings.json has SessionStart x1, PostToolUse x3, UserPromptSubmit x1 and hooks list prints exactly those 5 lines.
  • Claw variants sharing one workspace resolver: correct — resolveOpenclawWorkspaceDir() resolves the OpenClaw workspace only, so routing qclaw/easyclaw/autoclaw through it would make them overwrite the same handler. The reconcile branch and the listing are now restricted to openclaw; the other variants are omitted until the resolver is tool-specific.
  • E2E record vs the OpenClaw branch: the description is updated for the current head and records the new path: with ~/.openclaw/workspace present, teamai hooks inject writes <workspace>/hooks/teamai-status-report/{HOOK.md,handler.ts} and teamai hooks remove deletes them. Unit coverage for the same path lives in src/__tests__/openclaw-hooks.test.ts (inject → removeAll, plus the no-workspace no-op).

Verification on c0f247f: npx tsc --noEmit clean; npm test → 268 files, 3737 passed, 1 skipped; npx vitest run src/__tests__/hooks-cmd.test.ts → 21 passed.

`hooks list` rendered builtinHookDefs('claude') for every tool, so the built-in
block described Claude's hook set no matter which tool the row was for:
Copilot's SessionEnd hook never appeared, while tools that receive no hooks at
all were credited with six.

The displayed set is now derived from what each tool actually receives through
reconciliation, and a tool with no hook surface is omitted rather than shown an
invented list.

Fixes Tencent#717
@ousamabenyounes
ousamabenyounes force-pushed the fix/hooks-list-per-tool-defs branch from c0f247f to b9d15e9 Compare September 22, 2026 21:56
@github-actions

Copy link
Copy Markdown

Findings

  • [P1 blocking] src/hooks-cmd.ts:130 calculates status with getHookStatus(), which always expects the complete unmodified builtinHookDefs(tool), while src/hooks-cmd.ts:132 displays the overridden set. After builtin.disabled removes any hook, a correctly reconciled settings file is therefore reported as missing. Pass the effective definitions/override into status checking.
  • [P1 blocking] src/hooks.ts:1472 installs the fixed OpenClaw event set without honoring opts.builtinOverride. A team disabling SessionStart or UserPromptSubmit still gets that hook installed for OpenClaw, contrary to the documented global builtin.disabled behavior and the settings-driven reconciliation path.
  • [P2 non-blocking] src/hooks-cmd.ts:119 labels Hermes, OpenCode, and OpenClaw as not configured even when their generated hook artifacts are installed, while simultaneously listing their built-ins. Add adapter-specific status checks as already done for OMP to avoid contradictory output.

The PR description includes a detailed test plan and real-CLI end-to-end records, so it satisfies the testing-documentation requirement.

@github-actions

Copy link
Copy Markdown

Findings

  • [P1 blocking] builtin.disabled is ignored for standalone adapters. The new OpenClaw path always calls injectOpenClawHooks() with its fixed two-event handler, regardless of opts.builtinOverride; similarly, hooks list applies overrides only to settings-driven rows. Disabling Hook dispatch session-start therefore removes it from Claude but leaves it active and displayed for OpenClaw/Hermes/OMP/OpenCode. Apply the override consistently when reconciling and listing adapter hooks. src/hooks.ts:1499, src/hooks-cmd.ts:149
  • [P1 blocking] OpenClaw is now installed by the main reconciliation path, but hooks list still reports it as not configured because every non-OMP tool without hookPath takes the generic fallback. Add adapter-specific status detection for <workspace>/hooks/teamai-status-report, analogous to OMP. src/hooks-cmd.ts:144

The PR description includes a detailed test plan and real-CLI end-to-end verification, so its testing documentation is sufficient.

`hooks list` probed a settings file per tool, so Hermes, OpenCode and
OpenClaw — which reconciliation installs as one generated script/plugin/
handler each — fell through to the generic branch and were reported as
"not configured" even right after `hooks inject` wrote their hook. OMP
already had a bespoke branch for this.

Resolve that artifact per adapter and use its presence as the status, the
same rule OMP used, so the status column matches the built-in block.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@ousamabenyounes

Copy link
Copy Markdown
Contributor Author

Round-7 review, on b292376.

Fixed — OpenClaw reported as not configured: the status column probed a settings file per tool, so Hermes, OpenCode and OpenClaw (each installed as one generated artifact) fell through to the generic branch. They are now resolved per adapter, the rule OMP already used. Real CLI, sandboxed HOME, before → after teamai hooks inject:

openclaw   missing    ~/.openclaw/workspace/hooks/teamai-status-report/HOOK.md
hermes     missing    ~/.hermes/hooks/teamai-status-report.sh
opencode   missing    ~/.config/opencode/plugin/teamai-hooks.ts
→
openclaw   installed  ~/.openclaw/workspace/hooks/teamai-status-report/HOOK.md
hermes     installed  ~/.hermes/hooks/teamai-status-report.sh
opencode   installed  ~/.config/opencode/plugin/teamai-hooks.ts

(omp correctly stays missing when ~/.omp does not exist.) New unit test: "reports adapter-driven tools by their generated artifact, not 'not configured'".

Not fixed, deliberately — builtin.disabled and the standalone adapters: the adapters generate a fixed handler and ignore opts.builtinOverride today, so filtering their rows in hooks list would hide hooks they still install and reintroduce exactly the defect this PR closes. The listing therefore applies the override only where reconciliation applies it (settings-driven tools), with a comment saying so. Making the adapters honor builtin.disabled means changing four code generators (Hermes script, OMP/OpenCode/OpenClaw handlers) and their removal paths — a behavior change worth its own PR and issue, not a rider on a display fix. Happy to open that issue if you agree.

Verification on b292376: npx tsc --noEmit clean; npm test → 271 files, 3880 passed, 1 skipped; npx vitest run src/__tests__/hooks-cmd.test.ts → 27 passed. The branch is rebased on current main (includes #700 and #720).

@github-actions

Copy link
Copy Markdown

Findings

  • [P1 blocking] src/hooks-cmd.ts:164 passes localConfig.scope to adapterHookArtifact. OpenCode reconciliation always installs the single plugin under the user path (~/.config/opencode/plugin), including project/self scopes, but this probes <baseDir>/.opencode/plugin whenever the config scope is project. Consequently, hooks list reports OpenCode as missing after a successful injection. Resolve this artifact using user scope/Home, matching reconcileOpencodePlugin.
  • [P1 blocking] src/hooks-cmd.ts:185 still calls getHookStatus, which checks the complete unmodified builtinHookDefs(tool). When builtin.disabled removes a hook, reconciliation correctly removes it and the new built-in block hides it, but the status table reports missing because getHookStatus continues expecting the disabled hook. Status checking must use the same effective overridden definitions.

Testing

  • The PR description includes a test plan, unit/typecheck results, and detailed real-CLI end-to-end verification, so no testing-description finding.

…ugin

Two status-column defects the per-tool listing exposed:

- `getHookStatus` checked the unmodified `builtinHookDefs(tool)`, so a
  built-in the team disabled through hooks.yaml was still expected on disk
  and every tool read `missing` right after a correct reconciliation. It
  now takes the same §4.8 override reconciliation applies.
- The OpenCode artifact was probed under the config scope's base dir, but
  `reconcileOpencodePlugin` always installs the single plugin under the
  user path, so a project-scope config reported `missing` after a
  successful injection.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@ousamabenyounes

Copy link
Copy Markdown
Contributor Author

Round-8 findings fixed on d84b5f9.

  • Status ignored builtin.disabled: getHookStatus() now takes the same §4.8 override reconciliation applies, and hooks list passes it. Without it a team that disabled a built-in saw every tool as missing right after a correct injection. Real CLI with builtin.disabled: ["Hook dispatch stop"]: claude installed, opencode installed. New unit test: "checks tool status against the overridden built-in set".
  • OpenCode probed under the wrong base dir: the artifact is now resolved under the user path, matching reconcileOpencodePlugin, which always installs the single plugin there. Real CLI from a project-scope config: opencode installed ~/.config/opencode/plugin/teamai-hooks.ts (previously missing).

Existing getHookStatus call assertions in hooks-cmd.test.ts were updated for the new third argument rather than relaxed.

Verification on d84b5f9: npx tsc --noEmit clean; npm test → 271 files, 3881 passed, 1 skipped; npx vitest run src/__tests__/hooks-cmd.test.ts → 28 passed.

@ousamabenyounes

Copy link
Copy Markdown
Contributor Author

Rebased onto main (conflict with #700 and #720 resolved) and squashed; the branch is now three focused commits on top of cd3e0e6. The three P1 points are addressed, and this time with a real-CLI record rather than unit tests alone.

1. builtin.disabled was discarded. hooks list now calls parseTeamHooksConfig() and applies the override through applyBuiltinOverride(installedBuiltinHookDefs(tool, true), builtinOverride), so a hook disabled in hooks/hooks.yaml disappears from the listing exactly as reconciliation removes it from the settings files.

2. The claw family overwriting one handler. The reconcile branch in src/hooks.ts is now guarded by tool === 'openclaw' alone. qclaw, easyclaw and autoclaw no longer reach resolveOpenclawWorkspaceDir(), so they cannot overwrite that handler with another --tool value.

3. Missing real-CLI verification. Run against the built binary (node dist/index.js, v0.22.0) in a sandboxed HOME with an OpenClaw install the resolver actually finds:

=== hooks inject
✔ Injected teamai OpenClaw hook into <ws>/hooks/teamai-status-report
✔ Enabled OpenClaw internal hooks in <state>/openclaw.json
✔ Hooks injected into all AI tool settings

=== what OpenClaw received
<ws>/hooks/teamai-status-report/handler.ts
<ws>/hooks/teamai-status-report/HOOK.md
openclaw.json hooks.internal.enabled = True
--tool value recorded in the handler: "openclaw"

=== hooks remove
✔ Removed teamai OpenClaw hook from <ws>/hooks/teamai-status-report
✔ Removed teamai Hermes hook from <home>/.hermes/hooks/teamai-status-report.sh

hooks list from the same run, per tool:

tool(s) built-in hooks listed
claude, codex, cursor, qoder, qoder-cn, … 6
copilot 7 (includes SessionEnd)
zcode, opencode 6
codebuddy, workbuddy 6
omp 4 (no Skill/TodoWrite matchers)
openclaw 2
hermes 1 (SessionStart)

joycode and kiro no longer appear in the built-in block at all — they have no hook reconciliation, so no list is invented for them. This is the defect from #717: before, every one of these rows showed Claude's six.

Tests at head d84b5f9: npm test → 271 files, 3881 passed, 1 skipped, 0 failed. npx tsc --noEmit clean. One pre-existing test (prints the projects restriction next to the roles one, added by #700) mocked parseTeamHooksConfig with the old array shape and broke on the new {defs, builtin} return; it now uses the existing hooksYaml() helper rather than being skipped.

@github-actions

Copy link
Copy Markdown

Findings

  • [P1 blocking] src/hooks-cmd.ts:162 treats a standalone adapter as installed when one file merely exists. This is insufficient for OpenClaw, which requires both HOOK.md and handler.ts plus enabled internal hooks, and Hermes also requires its config registration/allowlist. A partial, stale, or manually altered installation will therefore be reported as installed even though no hook can run. Validate each adapter’s complete generated state, analogous to getHookStatus, rather than only calling pathExists() on one artifact.

Testing

  • The PR description includes a sufficient test plan and detailed real-CLI end-to-end verification, so no testing-description finding.

The adapter status probe accepted a single file, so an OpenClaw
installation missing its handler.ts — the file HOOK.md points at, without
which no hook runs — still read as `installed`. Check every generated file
the adapter writes and report `installed` only when all are present.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@ousamabenyounes

Copy link
Copy Markdown
Contributor Author

Round-9 finding, on ebc4c3e.

Fixed — partial installations reported as installed: the adapter probe now checks every file the adapter generates, not one. For OpenClaw that is HOOK.md and handler.ts (the file the descriptor points at; without it no hook runs). Real CLI: full install → openclaw installed; after deleting handler.ts → openclaw missing.

Not taken further, deliberately: validating the adapters' registration state as well — Hermes' config.yaml entry plus its shell-hook allowlist, OpenClaw's hooks.enabled flag in openclaw.json — means teaching hooks list to re-read each adapter's private config format. That is a status-reporting feature of its own (and OMP has always used plain artifact presence, before this PR). I would rather land this display fix and open a follow-up issue for full adapter state validation than keep widening a listing patch; say the word if you want it in here instead.

Verification on ebc4c3e: npx tsc --noEmit clean; npm test → 271 files, 3881 passed, 1 skipped.

@github-actions

Copy link
Copy Markdown

Findings

  • [P1 blocking] src/hooks.ts:1500 handles only openclaw, but the still-active injectHooksToAllTools path handles every OPENCLAW_TOOLS member and is called by local-agent initialization and legacy pull migration. A selected qclaw, easyclaw, or autoclaw can therefore still receive a generated handler, while hooks list omits it and hooks remove cannot remove it. Remove those variants from the legacy injector or support them consistently across listing, reconciliation, and removal.
  • [P1 blocking] src/hooks-cmd.ts:167 reports adapter hooks as installed solely from generated-file existence. This produces false positives for Hermes when script creation succeeds but config/allowlist registration fails, and for OpenClaw when both files exist but hooks.internal.enabled could not be set. Validate every activation component, not just artifacts, before reporting installed.

The PR description includes a detailed test plan and real-CLI end-to-end records, so it satisfies the testing-documentation requirement.

@ousamabenyounes

Copy link
Copy Markdown
Contributor Author

On the latest two findings, both concern behavior this PR did not introduce, so I am leaving them for a maintainer call rather than widening the patch again:

  1. Claw variants in the legacy injector — injectHooksToAllTools has iterated the whole OPENCLAW_TOOLS set since before this PR, while resolveOpenclawWorkspaceDir() resolves one workspace; that is why the new reconcile branch and the listing are limited to openclaw. Making qclaw/easyclaw/autoclaw first-class needs tool-specific workspace resolution across injection, listing and removal — a change of its own, and dropping them from the legacy injector is a behavior removal I would not make unilaterally.
  2. Activation-state validation — reporting installed from generated files is the rule hooks list has always used for OMP; extending it to each adapter's private registration state (Hermes config.yaml + allowlist, OpenClaw hooks.internal.enabled) is a status-reporting feature, not part of fixing the built-in block.

Happy to open follow-up issues for both, or to fold either into this PR if you prefer.

The review finding this PR was opened to fix — the built-in block crediting tools with hooks they never receive — is resolved, along with four adjacent defects found on the way (builtin.disabled ignored by the listing and by getHookStatus, OpenClaw skipped by the authoritative reconcile path, OpenCode probed under the wrong base dir). CI is green on ebc4c3e: 4 Lint & Test jobs, Build, Code erosion metrics and the opensource scan all pass; npm test → 271 files, 3881 passed, 1 skipped locally.

@ousamabenyounes

Copy link
Copy Markdown
Contributor Author

This PR had drifted well past the display defect it was opened for (8 files, +414), so I have reduced it back to its original scope.

The reduced work is on fix/hooks-list-scope (fork: ousamabenyounes/teamai-cli), branched fresh from upstream/main at cd3e0e6. GitHub does not allow changing the head branch of an existing PR, so this one still points at fix/hooks-list-per-tool-defs; I have not opened a second PR. Happy to repoint however maintainers prefer.

What the reduced change does (#717)

teamai hooks list rendered its built-in (A) block from a hardcoded builtinHookDefs('claude'), so every tool was shown Claude's set. Copilot's SessionEnd entry — which hooks inject really writes — never appeared, and the dispatch command shape was wrong for ZCode (process-typed entries, no shell wrapper) and for the GUI tools carrying the PATH wrapper.

The block is now rendered per tool, from the defs the settings reconcile path writes into the very file each row probes. Tools whose set is identical once the tool id is folded out share one block, so the listing stays about as short as before.

What was removed, and why

  • src/hooks.ts, src/hermes-hooks.ts, src/builtin-hooks.ts, src/__tests__/openclaw-hooks.test.ts — all runtime behavior changes. They fixed real problems, but none of them is the display defect [bug] hooks list shows Claude's built-in hook set for every tool, hiding Copilot's SessionEnd #717 describes, and each round of review on them produced new findings on code this PR should not have been touching.
  • The built-in listing for the adapter-driven tools (OMP, Hermes, OpenCode, OpenClaw, JoyCode, Kiro). Determining their real hook surface requires those out-of-scope files. Rather than print an invented list, those tools are omitted from the built-in block — their status row is unchanged, so no information regressed. This keeps the review's central constraint intact: never show a hook a tool does not receive.

The separate defects found during the earlier rounds (builtin.disabled not applied by getHookStatus; the qclaw/easyclaw/autoclaw variants sharing the OpenClaw workspace resolver; contradictory status for hermes/opencode/openclaw) are real but independent of #717. I will file them as their own issues rather than carry them here.

Diff

 src/__tests__/hooks-cmd.test.ts | 117 +++++++++++++++++++++++++++++++
 src/hooks-cmd.ts                |  49 ++++++++++---
 2 files changed, 160 insertions(+), 6 deletions(-)

Tests

  • npx vitest run src/__tests__/hooks-cmd.test.ts — 27 passed. Four are new; two of them fail on upstream/main (Copilot's SessionEnd absent from the listing, ZCode shown Claude's wrapped command) and pass with the fix.
  • npm test — 271 files, 3878 passed, 1 skipped, 0 failed.
  • npx tsc --noEmit — clean.
  • End-to-end with the built CLI (npm run build, then a sandbox HOME): hooks list now prints SessionEnd under copilot, the unwrapped teamai hook-dispatch … --tool <tool> form under zcode, and the PATH="$HOME/.teamai/bin:$PATH" … form under codebuddy. After hooks inject, ~/.copilot/hooks/teamai.json really contains SessionEnd, matching what the listing claims. With hermes, omp, joycode, openclaw and opencode enabled, all five appear in the status table and none appears in the built-in block.

🤖 Generated with Claude Code

@jeff-r2026
jeff-r2026 merged commit b91b6df into Tencent:main Sep 23, 2026
10 checks passed
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.

[bug] hooks list shows Claude's built-in hook set for every tool, hiding Copilot's SessionEnd

2 participants