feat(hooks): add if field to markitdown and deny-vendor-write hooks

[amondnet]

Summary

• Add if field (permission rule syntax filter) to markitdown PreToolUse hook — only fires on binary doc extensions (*.pptx, *.docx, *.xlsx, *.xls, *.ppt, *.doc) instead of every Read call
• Add if field to deny-vendor-write PreToolUse hook — only fires on edits targeting vendor/ or sources/ paths instead of every Edit/Write call
• Reduces unnecessary hook process spawns for improved performance

Test plan

• Verify markitdown hook still shows warning when reading .docx/.pptx/.xlsx files
• Verify markitdown hook does NOT fire on .ts/.md/.json reads
• Verify deny-vendor-write still blocks edits to vendor/ and sources/ paths
• Verify deny-vendor-write does NOT fire on normal file edits
• claude plugin validate plugins/markitdown passes

---

Summary by cubic

Added an if filter to the markitdown PreToolUse hook so it only runs on binary document reads, reducing unnecessary hook spawns. Dropped the planned deny-vendor-write filter due to permission rule limits (no alternation).

• New Features

  • markitdown PreToolUse now runs only for Read on: .pptx, .docx, .xlsx, .xls, .ppt, .doc

• Refactors

  • Split markitdown into six single-pattern entries since permission rules don’t support alternation.

^{Written for commit 5250912. Summary will update on new commits.}

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
claude-code-plugins	Ready	Preview, Comment	Mar 28, 2026 2:52pm

[gemini-code-assist]

Code Review

This pull request introduces the if field to several hooks to improve performance by filtering tool-call invocations before spawning processes. It also includes a new track for this feature with a plan and specification. Several critical issues were identified in the implementation: the added if fields in .claude/settings.json and plugins/markitdown/hooks/hooks.json contain syntax errors (missing commas) that break JSON parsing. Additionally, the glob patterns used for vendor/sources filtering are too broad and should be refined to target specific directories. There is also a consistency issue in the specification regarding PDF support that does not match the actual implementation.

[cubic-dev-ai]

1 issue found across 6 files

Confidence score: 4/5

• This PR is likely safe to merge with minimal risk: the reported issue is moderate-low severity (4/10) and appears limited to hook filtering behavior rather than core runtime functionality.
• In .claude/settings.json, the *vendor* / *sources* patterns are substring matches, so the hook may still trigger on unrelated files and reduce the expected performance improvement.
• Pay close attention to .claude/settings.json - tighten the filter patterns to target actual vendor/sources directories and avoid unintended hook executions.

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name=".claude/settings.json">

<violation number="1" location=".claude/settings.json:10">
P2: The new hook filter is too broad: `*vendor*`/`*sources*` match substrings rather than vendor/sources directories, so the hook can still spawn on unrelated edits and miss the intended performance reduction.

(Based on your team's feedback about narrowing hook `if` filters to avoid unnecessary process overhead.) [FEEDBACK_USED]</violation>
</file>

Architecture diagram

sequenceDiagram
    participant Claude as Claude Code Engine
    participant Engine as Hook Evaluator
    participant Script as External Hook Script
    participant FS as File System / Tool

    Note over Claude,FS: Tool Invocation Flow (e.g., Read, Edit, Write)

    Claude->>Engine: Trigger PreToolUse Hook
    
    Note over Engine: NEW: Evaluate "if" field<br/>(Permission Rule Syntax)

    alt Pattern Matches (e.g., Read(*.docx) or Edit(*vendor*))
        Engine->>Script: CHANGED: Spawn process and execute script
        Script-->>Engine: Return exit code/validation
        
        alt Hook Allowed
            Engine-->>Claude: Continue tool execution
            Claude->>FS: Perform File Operation
            FS-->>Claude: Operation Result
        else Hook Denied
            Engine-->>Claude: Block tool execution (Error)
        end

    else Pattern Does NOT Match (e.g., Read(*.ts) or Edit(src/))
        Note over Engine,Script: NEW: Skip process spawn (Performance Gain)
        Engine-->>Claude: Continue tool execution
        Claude->>FS: Perform File Operation
        FS-->>Claude: Operation Result
    end

Loading

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.}

[amondnet]

All review feedback addressed:

1. Settings hook if removed — permission rule syntax (gitignore-based) doesn't support | alternation, making 4 tools × 2 paths = 8 entries impractical. The script already handles filtering efficiently.
2. Markitdown hook split into 6 entries — one per extension (Read(*.pptx), Read(*.docx), etc.). The suggested Read(*.pptx|*.docx) syntax is invalid since gitignore patterns have no alternation operator.
3. Spec fixed — removed *.pdf reference (not handled by check-binary-doc.sh)
4. Plan updated — T-2 descoped, added syntax limitation discovery

[cubic-dev-ai]

1 issue found across 4 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name=".please/docs/tracks/active/hooks-if-field-20260328/plan.md">

<violation number="1" location=".please/docs/tracks/active/hooks-if-field-20260328/plan.md:47">
P3: The T-2 descoping update is not propagated through the rest of the plan, leaving contradictory guidance in the same document.</violation>
</file>

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.}