All contributors have signed the CLA. Thank you! ✅
_{Posted by the CLA Assistant Lite bot.}

Greptile Summary

This PR fixes a bug where the keyword-detector hook was being invoked within claude-code-hooks even when keyword-detector was in the disabled_hooks configuration. The fix properly respects the disabled_hooks configuration for the keyword detector.

Key changes:

• Added keywordDetectorDisabled boolean config option to control keyword detection within claude-code-hooks
• Modified keyword detection logic in chat.message hook to check this flag before running
• Wired up the disabled state from src/index.ts by passing !isHookEnabled("keyword-detector")

Architecture insight:
The codebase has two separate keyword-related hooks:

1. keyword-detector hook (standalone) - runs separately in src/index.ts
2. Keyword detection inside claude-code-hooks - was missing the disabled check

This fix ensures both respect the same disabled_hooks configuration.

Confidence Score: 5/5

• This PR is safe to merge with minimal risk
• The fix is well-scoped, addressing a clear bug with a simple conditional check. The implementation follows existing patterns (disabledHooks config option), passes type checking and build tests, and the logic is straightforward with no edge cases or potential side effects.
• No files require special attention

Important Files Changed

Sequence Diagram

sequenceDiagram
    participant User
    participant Plugin as Plugin (src/index.ts)
    participant ClaudeCodeHooks as ClaudeCodeHooks Hook
    participant KeywordDetector as Keyword Detector

    User->>Plugin: Load plugin with disabled_hooks config
    Plugin->>Plugin: isHookEnabled("keyword-detector") = false
    Plugin->>ClaudeCodeHooks: createClaudeCodeHooksHook(ctx, {<br/>keywordDetectorDisabled: true})
    
    User->>Plugin: Submit message with keywords
    Plugin->>ClaudeCodeHooks: chat.message hook
    
    alt keywordDetectorDisabled = true
        ClaudeCodeHooks->>ClaudeCodeHooks: Skip keyword detection
        Note over ClaudeCodeHooks: keywordMessages = []
    else keywordDetectorDisabled = false
        ClaudeCodeHooks->>KeywordDetector: detectKeywordsWithType()
        KeywordDetector-->>ClaudeCodeHooks: detected keywords
        ClaudeCodeHooks->>ClaudeCodeHooks: Add keyword messages
    end
    
    ClaudeCodeHooks->>ClaudeCodeHooks: Combine keyword + hook messages
    ClaudeCodeHooks-->>Plugin: Inject messages to prompt

I have read the CLA Document and I hereby sign the CLA

@sisyphus-dev-ai review

Code Review: PR #562

Summary

This PR successfully fixes the bug where keyword-detector was being invoked within claude-code-hooks even when disabled via disabled_hooks configuration. The implementation is clean, type-safe, and follows existing codebase patterns.

✅ What's Good

1. Correct Logic Implementation

• The wiring in src/index.ts is correct: keywordDetectorDisabled: !isHookEnabled("keyword-detector")
• Safe default behavior: if keywordDetectorDisabled is undefined, keyword detection runs (enabled by default)
• Proper TypeScript typing with optional field

2. Consistent Patterns

• Follows the existing disabledHooks pattern in PluginConfig
• Matches the codebase style for conditional hook execution
• Clean array initialization pattern

3. Edge Case Handling

• ✅ undefined → keyword detection enabled (safe default)
• ✅ true → keyword detection disabled
• ✅ false → keyword detection enabled

💡 Suggestions for Improvement

1. Test Coverage (Recommended)
While there are tests for the standalone keyword-detector hook (src/hooks/keyword-detector/index.test.ts), there's no test coverage for this new integration. Consider adding a test in src/hooks/claude-code-hooks/:

// src/hooks/claude-code-hooks/index.test.ts (new file)
describe('claude-code-hooks keyword detection', () => {
  test('should respect keywordDetectorDisabled config', async () => {
    // Test that keyword detection is skipped when keywordDetectorDisabled=true
  })
  
  test('should run keyword detection when keywordDetectorDisabled=false', async () => {
    // Test that keyword detection runs when enabled
  })
})

2. Minor Log Information Loss (Optional)
The PR changed from logging keyword types to just keywordCount:

• Before: types: detectedKeywords.map((k) => k.type)
• After: keywordCount: keywordMessages.length

This reduces observability slightly. Consider keeping both:

if (keywordMessages.length > 0) {
  log("[claude-code-hooks] Detected keywords", {
    sessionID: input.sessionID,
    keywordCount: keywordMessages.length,
    types: detectedKeywords.map((k) => k.type),
  })
}

However, the current implementation is also fine since you can't access detectedKeywords outside the conditional block without refactoring.

3. Documentation (Nice-to-have)
Consider updating src/hooks/AGENTS.md to document that claude-code-hooks respects the keyword-detector disabled_hooks setting.

🎯 Verdict

APPROVED ✅

This PR is ready to merge. The fix is correct, type-safe, and solves the reported issue. The suggestions above are optional improvements, not blockers.

Confidence Score: 5/5

• Logic is correct and follows existing patterns
• Safe default behavior
• No breaking changes
• Already validated by Greptile and cubic-dev-ai

Great work @popododo0720! 🎉

recheck

Applied the review suggestion: restored types array in the keyword detection log for better observability.

log("[claude-code-hooks] Detected keywords", {
  sessionID: input.sessionID,
  keywordCount: keywordMessages.length,
  types: detectedKeywords.map((k) => k.type),  // restored
})