Skip to content

feat(hook): say why a hook is invalid - #9699

Open
dadgrand wants to merge 1 commit into
esengine:main-v2from
dadgrand:fix/hook-invalid-reason
Open

feat(hook): say why a hook is invalid#9699
dadgrand wants to merge 1 commit into
esengine:main-v2from
dadgrand:fix/hook-invalid-reason

Conversation

@dadgrand

@dadgrand dadgrand commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Summary

  • reasonix hook list --json reports "status": "invalid" for five distinct conditions without saying which one fired, so a misconfigured hook gives the operator nothing to act on.
  • The matcher branch is the clearest case: it calls hook.ValidateMatcher, which already builds a full error message, and then discards it.
  • Report the reason alongside the status. Active entries carry no reason, so existing consumers see an unchanged shape.
invalid — unknown event "NotAnEvent"
invalid — command is required
invalid — contextFile "…" is not readable
invalid — plugin hook has neither a command nor a contextFile
invalid — invalid matcher regex: error parsing regexp: missing closing ): `^(?:()$`

This matters most when porting hooks from Claude Code, which internal/hook goes to some length to support elsewhere — tool-name aliases across Claude renames (TaskAgent, BashOutputTaskOutput), PayloadFormat "claude", CLAUDE_PROJECT_DIR/CLAUDE_PLUGIN_ROOT. Claude's nested form:

{"matcher":"*","hooks":[{"type":"command","command":""}]}

leaves Command empty, so it lands on the command check and now answers command is required instead of a bare invalid.

Issues

Verification

  • go test ./... passes. gofmt -l . and go vet ./... clean. Go 1.26.2, darwin/arm64.
  • The existing TestHookMachineEntryStatusRejectsNonRegularContextFile is updated for the two-value signature and still passes unchanged in intent.
  • New TestMachineHookEntryStatusExplainsEveryRejection covers unknown event, missing command and a malformed matcher regex; TestMachineHookEntryStatusLeavesActiveEntriesUnexplained pins that an active entry emits no reason, so the JSON shape is unchanged for the common case.
  • Checked against a real config on each of the five paths, plus the Claude-style nested form.

Documentation impact

Documentation-impact: none - additive, optional field on an existing diagnostics payload; no CLI, config, provider, permission or tool behavior changes.

Cache impact

Cache-impact: none - hook list --json output never enters the provider request; no system prompt, memory prefix, tool schema, tool surface or provider serialization is touched.
Cache-guard: not applicable - no cache-sensitive path is modified.
System-prompt-review: N/A

@github-actions github-actions Bot added tui Terminal UI / CLI (internal/cli, internal/control) v2 Go rewrite (1.x) — main-v2 branch, active development labels Sep 1, 2026
`reasonix hook list --json` reports status "invalid" for five distinct
conditions without saying which one fired, so a misconfigured hook gives the
operator nothing to act on. The matcher branch is the clearest case: it calls
hook.ValidateMatcher, which already builds a full error message, and then
discards it.

Report the reason alongside the status:

  unknown event "NotAnEvent"
  command is required
  contextFile "..." is not readable
  plugin hook has neither a command nor a contextFile
  invalid matcher regex: error parsing regexp: missing closing ): `^(?:()$`

This matters most when porting hooks from Claude Code, which the hook package
goes to some length to support elsewhere (tool-name aliases, PayloadFormat
"claude", CLAUDE_PROJECT_DIR). Claude's nested form —

  {"matcher":"*","hooks":[{"type":"command","command":"..."}]}

— leaves Command empty, so it lands on the command check and now answers
"command is required" instead of a bare "invalid".

Active entries carry no reason, so existing consumers see an unchanged shape.
@dadgrand
dadgrand force-pushed the fix/hook-invalid-reason branch from 50f23c4 to bf8bd15 Compare September 1, 2026 11:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

tui Terminal UI / CLI (internal/cli, internal/control) v2 Go rewrite (1.x) — main-v2 branch, active development

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant