feat(cli): add PreToolUse hook support for AskUserQuestion tool#66
Merged
feat(cli): add PreToolUse hook support for AskUserQuestion tool#66
Conversation
This extends the Supervisor Hook mechanism to also review AskUserQuestion tool calls, not just Stop events. When Claude Code calls AskUserQuestion, the Supervisor can now review and decide whether to allow or deny the call. Changes: - Extend HookInput structure to support all hook event types (Stop, PreToolUse) - Add HookOutput and HookSpecificOutput structures for event-specific output - Implement SupervisorResultToHookOutput converter for format translation - Detect event type from hook_event_name field (defaults to "Stop" for backward compatibility) - Add PreToolUse hook configuration with matcher "AskUserQuestion" in provider.go - Add outputHookOutput helper to output JSON in correct format The implementation maintains full backward compatibility: - StopHookInput is now an alias for HookInput - Stop events continue using existing output format (decision/reason) - PreToolUse events use new format (hookSpecificOutput with permissionDecision) - Iteration count is incremented for all event types (consistency) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Add 11 unit tests covering: - PreToolUse allow/deny decisions with feedback - Stop allow/block decisions with feedback - Empty feedback handling for both event types - JSON marshaling/unmarshaling for input/output - Backward compatibility with old Stop format Coverage increased to 33.0%. Addresses review feedback about missing unit tests.
Add comprehensive tests covering: - Edge cases: unknown event types (default to Stop format) - Event type case sensitivity (only exact "PreToolUse" match) - Integration test for PreToolUse input parsing - Full input/output validation with all fields Coverage increased from 33.0% to 36.9%. Addresses feedback about missing edge case tests and integration validation.
Based on detailed feedback, this commit adds:
1. Fixed integration tests:
- Renamed TestRunSupervisorHook_PreToolUse_InputParsing to
TestRunSupervisorHook_RecursiveCallProtection (clarifies purpose)
- Added TestRunSupervisorHook_SupervisorModeDisabled (tests early return)
2. Added provider.go validation tests (3 new tests):
- TestSwitchWithHook_PreToolUseConfiguration
- TestSwitchWithHook_PreToolUseAndStopCoexist
- TestSwitchWithHook_PreToolUseMatcherIsAskUserQuestion
3. Created end-to-end test script:
- tests/e2e_pretooluse_hook_test.sh
- 6 test scenarios covering backward compatibility, input parsing,
output format, unknown event types, hook configuration, and
iteration count
4. Created manual validation documentation:
- docs/validate-pretooluse-hook.md
- Step-by-step verification guide
- Troubleshooting section
- Performance metrics
Code coverage: 38.2% for internal/cli, 88.3% for internal/provider
Addresses all feedback points about test completeness and validation.
Major refactoring to address architectural issues in PreToolUse hook support: 1. **Centralized output types in supervisor package** - Added HookEventType constants (EventTypeStop, EventTypePreToolUse) - Added StopHookOutput and PreToolUseHookOutput types - Added OutputPreToolUseDecision() function - HookOutput is now an alias for StopHookOutput (backward compatible) 2. **Clean input type separation in cli package** - HookInputHeader: minimal struct for event type detection - StopHookInput: dedicated Stop event input - PreToolUseInput: dedicated PreToolUse event input - Removed the bloated unified HookInput struct 3. **Simplified event handling** - detectEventType(): reads stdin once and detects event type - outputDecisionByEventType(): routes to correct output function - runSupervisorReview(): extracted supervisor logic - Removed duplicate HookOutput, HookSpecificOutput types - Removed SupervisorResultToHookOutput conversion function - Removed outputHookOutput helper (use supervisor package instead) 4. **Test improvements** - Added comprehensive output_test.go for supervisor package - Updated hook_test.go to use new types - Reduced test code by 443 lines while maintaining coverage This refactoring eliminates code duplication, establishes clear ownership of types, and provides a cleaner foundation for future hook event types.
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
1 participant
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.
This extends the Supervisor Hook mechanism to also review AskUserQuestion
tool calls, not just Stop events. When Claude Code calls AskUserQuestion,
the Supervisor can now review and decide whether to allow or deny the call.
Summary
Architecture refactored for clean separation of concerns:
supervisorpackage (single source of truth)clipackageChanges
internal/supervisor/output.go- Output Types & FunctionsNew types and functions:
HookEventTypeconstants (EventTypeStop,EventTypePreToolUse)StopHookOutput- Stop event output formatPreToolUseSpecificOutput- PreToolUse specific fieldsPreToolUseHookOutput- PreToolUse event output formatOutputPreToolUseDecision()- Output PreToolUse decisionsHookOutput- Backward compatible alias forStopHookOutputinternal/cli/hook.go- Input Types & Event HandlingClean input type separation:
HookInputHeader- Minimal struct for event type detectionStopHookInput- Dedicated Stop event inputPreToolUseInput- Dedicated PreToolUse event inputEvent handling:
detectEventType()- Reads stdin and detects event typeoutputDecisionByEventType()- Routes to correct output functionrunSupervisorReview()- Extracted supervisor review logicinternal/provider/provider.go- Hook Configurationmatcher: "AskUserQuestion"Removed (cleanup)
cli.HookOutputsupervisorpackagecli.HookSpecificOutputsupervisorpackagecli.SupervisorResultToHookOutput()outputDecisionByEventType()cli.outputHookOutput()supervisor.Output*functions insteadBackward Compatibility
decision,reason)HookOutputis a type alias forStopHookOutputhook_event_nameis absent, defaults to "Stop" eventOutputDecision()function unchangedTesting
go test -race ./...)output_test.gocovers supervisor package output functionshook_test.gocovers new input types and event detectionDocumentation
SpecKit workflow documents in
specs/002-askuserquestion-hook/:spec.md- Feature specificationplan.md- Implementation plantasks.md- Task breakdownresearch.md- Technical researchdata-model.md- Data model definitioncontracts/hook-input-output.md- API contractsquickstart.md- Quick start guide