Skip to content

feat(doctor): hook-config validation in /doctor#227

Merged
emal-avala merged 1 commit intomainfrom
feat/doctor-hook-validation
Apr 23, 2026
Merged

feat(doctor): hook-config validation in /doctor#227
emal-avala merged 1 commit intomainfrom
feat/doctor-hook-validation

Conversation

@emal-avala
Copy link
Copy Markdown
Member

Summary

A misconfigured hook silently does nothing at runtime — a shell hook with an empty command string fails its subprocess; an HTTP hook with a malformed URL fails its request. Users only notice when the expected side-effect (audit log, Slack ping, backup) doesn't happen. /doctor is the right place to surface these early, before they become production-debugging ghost stories.

Adds a hooks:* section to the /doctor output that:

  • Skips entirely when no hooks are configured (no noise)
  • Pass: emits one hooks:count line when all hooks are valid
  • Fail (per broken entry):
    • Shell action with empty / whitespace-only command
    • HTTP action with a URL that reqwest::Url can't parse
  • Warn (per HTTP action with a method string that isn't one of GET/POST/PUT/PATCH/DELETE): those get silently treated as POST by the dispatcher, which is almost certainly not what the user wrote

Ships with a new private is_valid_http_method helper that matches the exact verb set the dispatcher actually recognizes.

Example output

  ! git:repo: Not inside a git repository
  ✓ hooks:count: 3 hook(s) configured, all valid

or when broken:

  ✗ hooks:entry:0: Hook #0 (PostToolUse): url "not a url" is malformed: relative URL without a base
  ! hooks:entry:1: Hook #1 (SessionStart): method "FETCH" is not a recognized HTTP verb
  ✗ hooks:count: 1/3 hook(s) valid (2 broken)

Test plan

  • cargo clippy --workspace --tests --no-deps -- -D warnings — clean
  • cargo test -p agent-code-lib --lib services::diagnostics::tests — 16 pass (9 prior + 7 new)

7 new tests:

  1. is_valid_http_method accepts common verbs (case-insensitive)
  2. is_valid_http_method rejects typos (GETS, HEAD, empty, POSTT)
  3. No hooks configured → no hooks:* checks emitted
  4. N valid hooks → single hooks:count Pass
  5. Empty shell command → Fail entry + Fail count
  6. Malformed URL → Fail entry
  7. Unknown HTTP method → Warn entry

A misconfigured hook silently does nothing at runtime — a shell hook
with an empty command string fails its subprocess, and an HTTP hook
with a malformed URL fails its request. Users only notice when the
expected side-effect (audit log, Slack ping, backup) doesn't
happen. /doctor is the right place to surface these early, before
they become production-debugging ghost stories.

Adds a hooks:* section to the /doctor output that:

- Skips entirely when no hooks are configured (no noise)
- Emits one hooks:count Pass line when all hooks are valid
- Emits one hooks:entry:<i> Fail per broken entry:
  - Shell action with empty (or whitespace-only) command
  - HTTP action with a URL that reqwest::Url can't parse
- Emits one hooks:entry:<i> Warn per HTTP action with a method
  string that isn't one of GET/POST/PUT/PATCH/DELETE — those get
  silently treated as POST by the dispatcher, which is almost
  certainly not what the user wrote

Ships with a new private is_valid_http_method helper that matches
the exact verb set the dispatcher actually recognizes.

7 new tests:
- is_valid_http_method accepts common verbs (case-insensitive)
- is_valid_http_method rejects typos (GETS, HEAD, empty, POSTT)
- No hooks configured -> no hooks:* checks emitted
- N valid hooks -> single hooks:count Pass
- Empty shell command -> Fail entry + Fail count
- Malformed URL -> Fail entry
- Unknown HTTP method -> Warn entry
@chatgpt-codex-connector
Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@emal-avala emal-avala merged commit 2e0cf30 into main Apr 23, 2026
14 checks passed
@emal-avala emal-avala deleted the feat/doctor-hook-validation branch April 23, 2026 21:44
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.

1 participant