feat: advanced hooks, LLM gates, and 7 new skills for v3.2.0#38
Merged
feat: advanced hooks, LLM gates, and 7 new skills for v3.2.0#38
Conversation
New skills (7): permission-tuner, compact-guard, cost-tracker, auto-setup, mcp-audit, llm-gate, file-watcher — covering permission optimization, context preservation, cost awareness, project auto-configuration, MCP overhead analysis, AI-powered quality gates, and reactive file watching. New agents (2): permission-analyst and cost-analyst for automated analysis of denial patterns and token spend. Agent optimizations: added omitClaudeMd to 6 read-only agents (planner, reviewer, scout, context-engineer, permission-analyst, cost-analyst) to skip project context loading and save tokens. Hook upgrades: - 6 new hook events: PermissionDenied, Setup, WorktreeCreate, WorktreeRemove, CwdChanged, TaskCreated (24 total) - Added `if` conditionals to PreToolUse hooks for precise filtering without spawning processes - Added `type: "prompt"` hooks for LLM-powered commit message validation and secret detection using Haiku - Consolidated git commit/push hooks into single matcher with multiple hooks - Scoped FileChanged matcher to specific config files New commands (5): /permission-tuner, /compact-guard, /cost-tracker, /auto-setup, /mcp-audit New scripts (6): permission-denied.js, worktree-create.js, worktree-remove.js, cwd-changed.js, setup-hook.js, task-created.js Totals: 24 skills, 8 agents, 21 commands, 29 scripts, 24 hook events
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds many new agent/skill/command docs, refactors hook configuration with new lifecycle events and matcher logic, and introduces six Node.js hook-handler scripts; several agent markdown files had Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant HookEngine as "Hook Engine"
participant LLM as "LLM Validator"
participant Script as "Script Handler"
participant Tool as "Tool (e.g., Bash)"
Client->>HookEngine: request to run tool / write / file change
HookEngine->>HookEngine: evaluate matchers (PreToolUse, Write, FileChanged, etc.)
alt Prompt hook matched
HookEngine->>LLM: send prompt (model haiku, await JSON)
LLM-->>HookEngine: {ok: true|false, reason}
HookEngine->>Client: allow or block tool based on result
else Script hook matched
HookEngine->>Script: pipe stdin JSON to script (pre-commit / setup / permission-denied ...)
Script-->>HookEngine: stdout (payload) + stderr diagnostics
HookEngine->>Tool: proceed to execute tool (if not blocked)
Tool-->>Client: tool output
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (11)
commands/cost-tracker.md (1)
28-30: Add language specifier for consistency.Same as other command docs—add
bashorshellto the fenced code block.📝 Proposed fix
-``` +```bash /cost-tracker</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@commands/cost-tracker.mdaround lines 28 - 30, Update the fenced code block
in commands/cost-tracker.md that shows "/cost-tracker" to include a language
specifier (e.g., bash or shell) for consistency with other command docs; locate
the code fence containing "/cost-tracker" and change the opening triple
backticks to include the language token (for example, replacewithbash).</details> </blockquote></details> <details> <summary>commands/auto-setup.md (1)</summary><blockquote> `27-29`: **Add language specifier for consistency.** Add `bash` or `shell` to match the pattern used elsewhere. <details> <summary>📝 Proposed fix</summary> ```diff -``` +```bash /auto-setup ``` ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@commands/auto-setup.mdaround lines 27 - 29, The markdown code block
containing the CLI path "/auto-setup" is missing a language specifier; update
the fenced block to include a shell language tag (e.g., changetobash)
so it matches other examples and enables proper syntax highlighting for the
snippet that shows /auto-setup.</details> </blockquote></details> <details> <summary>scripts/cwd-changed.js (1)</summary><blockquote> `22-33`: **Project type detection logic is duplicated across scripts.** This same detection pattern (`package.json` → node, `Cargo.toml` → rust, etc.) appears in `setup-hook.js` and `file-changed.js`. Consider extracting to a shared utility in the future to ensure consistency if detection rules change. The current implementation is correct; this is optional refactoring for maintainability. <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@scripts/cwd-changed.js` around lines 22 - 33, The project-type detection logic (the ternary chain that checks package.json → Cargo.toml → go.mod → pyproject.toml and sets PRO_WORKFLOW_PROJECT_TYPE when process.env.CLAUDE_ENV_FILE is present) is duplicated across cwd-changed.js, setup-hook.js and file-changed.js; extract that detection into a single shared utility function (e.g., detectProjectType(dir) that returns 'node'|'rust'|'go'|'python'|null) and replace the inline ternary chain in each script to call detectProjectType(newCwd) (or the equivalent dir arg), then have the caller append to process.env.CLAUDE_ENV_FILE only when the returned type is non-null and handle fs.appendFileSync errors as before. ``` </details> </blockquote></details> <details> <summary>scripts/worktree-remove.js (1)</summary><blockquote> `17-23`: **Consider validating that parsed JSON is an array.** If `worktrees.json` somehow contains valid JSON that isn't an array (e.g., `{}`), `worktrees.filter()` will throw. The inner catch swallows this, which is fine for not blocking the hook, but the file won't be updated—leaving stale entries. <details> <summary>🛡️ Proposed defensive fix</summary> ```diff if (fs.existsSync(worktreeLog)) { try { let worktrees = JSON.parse(fs.readFileSync(worktreeLog, 'utf8')); + if (!Array.isArray(worktrees)) worktrees = []; worktrees = worktrees.filter(w => w.worktree_path !== input.worktree_path); fs.writeFileSync(worktreeLog, JSON.stringify(worktrees, null, 2)); } catch (e) { /* ignore */ } } ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@scripts/worktree-remove.js` around lines 17 - 23, The code reading worktrees.json may parse non-array JSON and then call worktrees.filter, causing a thrown error that is silently ignored; update the block that reads and writes worktrees (referencing worktreeLog and input.worktree_path) to validate that the parsed value is an Array before using .filter — if it's not an array, treat it as an empty array (or replace the file with an empty array) and then write the updated array back to worktreeLog; keep the try/catch but ensure you only call filter on a validated array so the file gets corrected when corrupted. ``` </details> </blockquote></details> <details> <summary>scripts/worktree-create.js (1)</summary><blockquote> `17-19`: **Add array validation for consistency with worktree-remove.js.** Same concern as in `worktree-remove.js`: if the file contains valid JSON that isn't an array, `worktrees.push()` on line 21 will throw. The outer catch handles this gracefully, but the entry won't be logged. <details> <summary>🛡️ Proposed defensive fix</summary> ```diff if (fs.existsSync(worktreeLog)) { - try { worktrees = JSON.parse(fs.readFileSync(worktreeLog, 'utf8')); } catch (e) { worktrees = []; } + try { + worktrees = JSON.parse(fs.readFileSync(worktreeLog, 'utf8')); + if (!Array.isArray(worktrees)) worktrees = []; + } catch (e) { worktrees = []; } } ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@scripts/worktree-create.js` around lines 17 - 19, When reading worktreeLog into worktrees in scripts/worktree-create.js, validate that the parsed JSON is an array before using worktrees.push: in the try block where you parse fs.readFileSync(worktreeLog, 'utf8'), check Array.isArray(parsed) and only assign worktrees = parsed when true, otherwise set worktrees = []; this mirrors the defensive handling in worktree-remove.js and prevents a non-array JSON file from causing a runtime error on worktrees.push. ``` </details> </blockquote></details> <details> <summary>commands/mcp-audit.md (1)</summary><blockquote> `29-31`: **Add language specifier to fenced code block.** The code block is missing a language identifier. Since this is a shell command invocation, use `bash` or `shell` for consistency with other command documentation. <details> <summary>📝 Proposed fix</summary> ```diff -``` +```bash /mcp-audit ``` ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@commands/mcp-audit.mdaround lines 29 - 31, Add a language specifier to the
fenced code block that contains the shell command "/mcp-audit" so syntax
highlighting is consistent; update the block fromtobash (or ```shell)
and leave the content "/mcp-audit" unchanged, ensuring the fenced block now
reads as a bash/shell code block for proper rendering.</details> </blockquote></details> <details> <summary>skills/cost-tracker/SKILL.md (1)</summary><blockquote> `35-35`: **Keep model name casing consistent.** Use “Haiku” (capitalized) to match the rest of the document and related skill docs. <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@skills/cost-tracker/SKILL.md` at line 35, Update the table row containing "Model selection | 3-10x difference | Use haiku for simple tasks" to use consistent casing by replacing the lowercase token "haiku" with "Haiku" so the entry reads "Model selection | 3-10x difference | Use Haiku for simple tasks". ``` </details> </blockquote></details> <details> <summary>skills/permission-tuner/SKILL.md (1)</summary><blockquote> `29-32`: **Permission extraction command is fragile for larger configs.** `grep -A 20 "permissions"` can truncate the block and miss rules. Prefer structured JSON extraction to avoid partial analysis. <details> <summary>Suggested doc update</summary> ```diff -cat .claude/settings.json 2>/dev/null | grep -A 20 "permissions" -cat ~/.claude/settings.json 2>/dev/null | grep -A 20 "permissions" +jq '.permissions' .claude/settings.json 2>/dev/null +jq '.permissions' ~/.claude/settings.json 2>/dev/null ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@skills/permission-tuner/SKILL.md` around lines 29 - 32, The documentation uses the fragile shell snippet grep -A 20 "permissions" which can truncate larger JSON configs; update the SKILL.md example to use a structured JSON extractor (e.g., jq) instead of grep so the entire "permissions" key/object is reliably retrieved and pretty-printed for both occurrences of the current two-line example (the lines that run cat ... | grep -A 20 "permissions"). Replace the grep-based examples with instructions showing how to parse and output the "permissions" field from JSON in a robust manner and note fallback/empty-file behavior. ``` </details> </blockquote></details> <details> <summary>skills/auto-setup/SKILL.md (1)</summary><blockquote> `30-33`: **Node.js quality-gate defaults should align with repo-standard gates.** Using `npm run typecheck` and `npm test -- --related` keeps generated setup aligned with the expected workflow in this repo. <details> <summary>Suggested doc update</summary> ```diff - "typecheck": "npx tsc --noEmit", - "test": "npm test -- --changed --passWithNoTests", + "typecheck": "npm run typecheck", + "test": "npm test -- --related", ``` </details> Based on learnings: After ANY code edit, run quality gates: (1) lint with `npm run lint`, (2) typecheck with `npm run typecheck`, (3) run related tests with `npm test -- --related`. <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@skills/auto-setup/SKILL.md` around lines 30 - 33, Update the package scripts so the repo-quality gate defaults match the project standard: replace the recursive "lint" script value ("lint": "npm run lint") with a real linter invocation (e.g., "lint": "eslint . --ext .js,.ts") and change the "test" script from "npm test -- --changed --passWithNoTests" to use the related-tests flag ("test": "npm test -- --related --passWithNoTests"); keep the "typecheck" script as "npx tsc --noEmit" and ensure documentation instructs running "npm run lint", "npm run typecheck", then "npm test -- --related" after edits. ``` </details> </blockquote></details> <details> <summary>hooks/hooks.json (2)</summary><blockquote> `39-51`: **Secret detection only covers Write, not Edit operations.** The LLM-powered secret scanner triggers on `Write` but not `Edit`. While `Write` typically handles new file creation, secrets could also be introduced through `Edit` operations on existing files. Consider whether the same protection should apply to both tools, or document the intentional scope difference. <details> <summary>💡 Option to extend coverage to Edit</summary> ```diff { - "matcher": "Write", + "matcher": "tool == \"Edit\" || tool == \"Write\"", "hooks": [ { "type": "prompt", ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@hooks/hooks.json` around lines 39 - 51, The secret-scanning hook currently only matches "Write" operations (matcher: "Write") so edits can bypass detection; update hooks.json to also cover "Edit" by either changing the matcher to include both operations or adding a sibling hook object with matcher: "Edit" using the same prompt/model/timeout/statusMessage payload (reference the existing "matcher": "Write", the prompt block and model "haiku") so that the LLM-powered secret detection runs for file edits as well. ``` </details> --- `299-310`: **Consider adding lock files if dependency change detection is intended.** The matcher includes dependency manifests (`package.json`, `Cargo.toml`, `go.mod`, `pyproject.toml`) but excludes their corresponding lock files (`package-lock.json`, `pnpm-lock.yaml`, `Cargo.lock`, `go.sum`, `poetry.lock`). If the goal is to detect dependency changes, lock file modifications often indicate actual dependency updates while manifest changes might be metadata-only. This may be intentional to reduce noise—lock files change frequently. If so, the current scope is reasonable. <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@hooks/hooks.json` around lines 299 - 310, The matcher in the "FileChanged" hook currently only lists manifests (package.json, Cargo.toml, go.mod, pyproject.toml); update the matcher string in the hook object (key "matcher" under "FileChanged") to also include common lock files such as package-lock.json, pnpm-lock.yaml, yarn.lock, Cargo.lock, go.sum, and poetry.lock so dependency changes detected via lockfile updates will trigger the hook (or explicitly document/confirm why lockfiles are excluded if you want to keep the current behavior). ``` </details> </blockquote></details> </blockquote></details> <details> <summary>🤖 Prompt for all review comments with AI agents</summary>Verify each finding against the current code and only fix it if needed.
Inline comments:
In@agents/permission-analyst.md:
- Around line 15-16: The analyst prompt expects both approved and denied counts
but the pipeline only persists denials (scripts/permission-denied.js), so add
explicit approval telemetry ingestion and storage and update the prompt to
reference that source; specifically, extend or create a counterpart (e.g.,
scripts/permission-approved.js or add a
persistApprovalRecord/ingestPermissionTelemetry path alongside
persistDenialRecord) to record approval events, update any ingestion/ETL that
aggregates session logs to emit both approved and denied counters, and revise
agents/permission-analyst.md (including the sections around lines 15-16 and
49-50) to read from the new approval data source so the agent can report
concrete "Approved [N] times" and "Denied [N] times" values.In
@commands/compact-guard.md:
- Around line 33-35: The unlabeled fenced code block containing "/compact-guard"
should be marked with a language for markdownlint (MD040); update the
triple-backtick fence that wraps the line "/compact-guard" to include a language
label (e.g., changetobash) so the fenced block becomes a bash code
block.In
@commands/permission-tuner.md:
- Around line 28-30: The markdown code fence for the "/permission-tuner" example
lacks a language specifier causing MD040; update the fenced code block around
"/permission-tuner" (the block containing the text "/permission-tuner") to
include a language such as "bash" (i.e., change the openingtobash) so
the block becomes a fenced bash code block.In
@scripts/permission-denied.js:
- Around line 18-30: The parsed denials value may be a non-array JSON (e.g., {})
causing denials.push to throw; after reading/parsing denialsFile (the JSON.parse
call that assigns denials), validate that denials is an Array (e.g., using
Array.isArray) and if not replace it with an empty array before pushing the new
entry object (entry) and trimming/saving back to denialsFile; ensure the
fallback behavior matches the existing logic that keeps the last 500 entries.In
@scripts/setup-hook.js:
- Around line 49-50: The catch block currently does console.log(data || '{}')
which can print non-JSON text; change the error path in the catch (err) handling
so it never emits arbitrary data — always emit a valid JSON fallback (e.g.,
JSON.stringify({}) or the literal '{}' ) to stdout and send the parse error to
stderr via console.error; update the catch surrounding the parsing logic that
references data and err to output only the safe JSON fallback to stdout and log
err to stderr.- Line 40: The recent-window filter currently only checks that Date.now() - new
Date(d.timestamp).getTime() < ONE_WEEK_MS, which lets future timestamps pass;
update the predicate used in the denials.filter (the recent variable) to
calculate the timestamp via new Date(d.timestamp).getTime(), compute delta =
Date.now() - ts, and require both delta >= 0 and delta < ONE_WEEK_MS (i.e.,
exclude future timestamps) so recent only includes items with timestamps <= now
and within ONE_WEEK_MS.In
@skills/compact-guard/SKILL.md:
- Line 42: In SKILL.md replace the phrase "top priority file" with the
hyphenated compound adjective "top-priority file" so the line reads "Re-read the
top-priority file (the one you're actively editing)"; update any other
occurrences of that exact phrase in the document to the hyphenated form for
grammatical consistency.In
@skills/file-watcher/SKILL.md:
- Around line 20-32: The docs claim CwdChanged (and SessionStart) can return
hookSpecificOutput.watchPaths but the current CwdChanged implementation
(scripts/cwd-changed.js) only echoes input and writes env vars; update the
implementation to emit the same structure the docs show by having the CwdChanged
handler include a hookSpecificOutput object with a watchPaths array (absolute
paths) when appropriate, i.e., modify the CwdChanged export to build and print
JSON containing
{"hookSpecificOutput":{"hookEventName":"CwdChanged","watchPaths":[...]}}, or
alternatively update the SKILL.md text to explicitly state that CwdChanged
currently does not register watchers—choose the implementation change if you
want behavior to match docs, otherwise update the documentation to match the
existing CwdChanged function behavior.In
@skills/permission-tuner/SKILL.md:
- Line 3: Update the top-level description field (the "description:" line) to
accurately reflect both behaviors: it should state that the skill analyzes
permission denial patterns and generates optimized alwaysAllow and alwaysDeny
rules; also update any summary in the output section and any mentions near the
alwaysDeny logic (referenced around the alwaysDeny lines) so the description and
output documentation are consistent about proposing both allow and deny
optimizations.
Nitpick comments:
In@commands/auto-setup.md:
- Around line 27-29: The markdown code block containing the CLI path
"/auto-setup" is missing a language specifier; update the fenced block to
include a shell language tag (e.g., changetobash) so it matches other
examples and enables proper syntax highlighting for the snippet that shows
/auto-setup.In
@commands/cost-tracker.md:
- Around line 28-30: Update the fenced code block in commands/cost-tracker.md
that shows "/cost-tracker" to include a language specifier (e.g., bash or shell)
for consistency with other command docs; locate the code fence containing
"/cost-tracker" and change the opening triple backticks to include the language
token (for example, replacewithbash).In
@commands/mcp-audit.md:
- Around line 29-31: Add a language specifier to the fenced code block that
contains the shell command "/mcp-audit" so syntax highlighting is consistent;
update the block fromtobash (or ```shell) and leave the content
"/mcp-audit" unchanged, ensuring the fenced block now reads as a bash/shell code
block for proper rendering.In
@hooks/hooks.json:
- Around line 39-51: The secret-scanning hook currently only matches "Write"
operations (matcher: "Write") so edits can bypass detection; update hooks.json
to also cover "Edit" by either changing the matcher to include both operations
or adding a sibling hook object with matcher: "Edit" using the same
prompt/model/timeout/statusMessage payload (reference the existing "matcher":
"Write", the prompt block and model "haiku") so that the LLM-powered secret
detection runs for file edits as well.- Around line 299-310: The matcher in the "FileChanged" hook currently only
lists manifests (package.json, Cargo.toml, go.mod, pyproject.toml); update the
matcher string in the hook object (key "matcher" under "FileChanged") to also
include common lock files such as package-lock.json, pnpm-lock.yaml, yarn.lock,
Cargo.lock, go.sum, and poetry.lock so dependency changes detected via lockfile
updates will trigger the hook (or explicitly document/confirm why lockfiles are
excluded if you want to keep the current behavior).In
@scripts/cwd-changed.js:
- Around line 22-33: The project-type detection logic (the ternary chain that
checks package.json → Cargo.toml → go.mod → pyproject.toml and sets
PRO_WORKFLOW_PROJECT_TYPE when process.env.CLAUDE_ENV_FILE is present) is
duplicated across cwd-changed.js, setup-hook.js and file-changed.js; extract
that detection into a single shared utility function (e.g.,
detectProjectType(dir) that returns 'node'|'rust'|'go'|'python'|null) and
replace the inline ternary chain in each script to call
detectProjectType(newCwd) (or the equivalent dir arg), then have the caller
append to process.env.CLAUDE_ENV_FILE only when the returned type is non-null
and handle fs.appendFileSync errors as before.In
@scripts/worktree-create.js:
- Around line 17-19: When reading worktreeLog into worktrees in
scripts/worktree-create.js, validate that the parsed JSON is an array before
using worktrees.push: in the try block where you parse
fs.readFileSync(worktreeLog, 'utf8'), check Array.isArray(parsed) and only
assign worktrees = parsed when true, otherwise set worktrees = []; this mirrors
the defensive handling in worktree-remove.js and prevents a non-array JSON file
from causing a runtime error on worktrees.push.In
@scripts/worktree-remove.js:
- Around line 17-23: The code reading worktrees.json may parse non-array JSON
and then call worktrees.filter, causing a thrown error that is silently ignored;
update the block that reads and writes worktrees (referencing worktreeLog and
input.worktree_path) to validate that the parsed value is an Array before using
.filter — if it's not an array, treat it as an empty array (or replace the file
with an empty array) and then write the updated array back to worktreeLog; keep
the try/catch but ensure you only call filter on a validated array so the file
gets corrected when corrupted.In
@skills/auto-setup/SKILL.md:
- Around line 30-33: Update the package scripts so the repo-quality gate
defaults match the project standard: replace the recursive "lint" script value
("lint": "npm run lint") with a real linter invocation (e.g., "lint": "eslint .
--ext .js,.ts") and change the "test" script from "npm test -- --changed
--passWithNoTests" to use the related-tests flag ("test": "npm test -- --related
--passWithNoTests"); keep the "typecheck" script as "npx tsc --noEmit" and
ensure documentation instructs running "npm run lint", "npm run typecheck", then
"npm test -- --related" after edits.In
@skills/cost-tracker/SKILL.md:
- Line 35: Update the table row containing "Model selection | 3-10x difference |
Use haiku for simple tasks" to use consistent casing by replacing the lowercase
token "haiku" with "Haiku" so the entry reads "Model selection | 3-10x
difference | Use Haiku for simple tasks".In
@skills/permission-tuner/SKILL.md:
- Around line 29-32: The documentation uses the fragile shell snippet grep -A 20
"permissions" which can truncate larger JSON configs; update the SKILL.md
example to use a structured JSON extractor (e.g., jq) instead of grep so the
entire "permissions" key/object is reliably retrieved and pretty-printed for
both occurrences of the current two-line example (the lines that run cat ... |
grep -A 20 "permissions"). Replace the grep-based examples with instructions
showing how to parse and output the "permissions" field from JSON in a robust
manner and note fallback/empty-file behavior.</details> <details> <summary>🪄 Autofix (Beta)</summary> Fix all unresolved CodeRabbit comments on this PR: - [ ] <!-- {"checkboxId": "4b0d0e0a-96d7-4f10-b296-3a18ea78f0b9"} --> Push a commit to this branch (recommended) - [ ] <!-- {"checkboxId": "ff5b1114-7d8c-49e6-8ac1-43f82af23a33"} --> Create a new PR with the fixes </details> --- <details> <summary>ℹ️ Review info</summary> <details> <summary>⚙️ Run configuration</summary> **Configuration used**: defaults **Review profile**: CHILL **Plan**: Pro **Run ID**: `c3130f4a-a155-4e0b-b2e8-945e4aeefeb5` </details> <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 7b8e34f8801ae90c6346028e373e0d10a740e771 and 03eae2a77177c876e88ac74e079f6051696cbe02. </details> <details> <summary>📒 Files selected for processing (25)</summary> * `agents/context-engineer.md` * `agents/cost-analyst.md` * `agents/permission-analyst.md` * `agents/planner.md` * `agents/reviewer.md` * `agents/scout.md` * `commands/auto-setup.md` * `commands/compact-guard.md` * `commands/cost-tracker.md` * `commands/mcp-audit.md` * `commands/permission-tuner.md` * `hooks/hooks.json` * `scripts/cwd-changed.js` * `scripts/permission-denied.js` * `scripts/setup-hook.js` * `scripts/task-created.js` * `scripts/worktree-create.js` * `scripts/worktree-remove.js` * `skills/auto-setup/SKILL.md` * `skills/compact-guard/SKILL.md` * `skills/cost-tracker/SKILL.md` * `skills/file-watcher/SKILL.md` * `skills/llm-gate/SKILL.md` * `skills/mcp-audit/SKILL.md` * `skills/permission-tuner/SKILL.md` </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
agents/permission-analyst.md
Outdated
Comment on lines
+15
to
+16
| 2. Check session logs for permission approval/denial patterns | ||
| 3. Categorize operations by risk level (safe/medium/dangerous) |
There was a problem hiding this comment.
Approval-count reporting is underspecified against available telemetry.
The current pipeline persists denial records only (scripts/permission-denied.js), but this prompt requires explicit approved/denied pattern counts. Without a concrete approval data source, the agent can produce unreliable “Approved [N] times” outputs.
Also applies to: 49-50
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@agents/permission-analyst.md` around lines 15 - 16, The analyst prompt
expects both approved and denied counts but the pipeline only persists denials
(scripts/permission-denied.js), so add explicit approval telemetry ingestion and
storage and update the prompt to reference that source; specifically, extend or
create a counterpart (e.g., scripts/permission-approved.js or add a
persistApprovalRecord/ingestPermissionTelemetry path alongside
persistDenialRecord) to record approval events, update any ingestion/ETL that
aggregates session logs to emit both approved and denied counters, and revise
agents/permission-analyst.md (including the sections around lines 15-16 and
49-50) to read from the new approval data source so the agent can report
concrete "Approved [N] times" and "Denied [N] times" values.
scripts/setup-hook.js
Outdated
Comment on lines
+49
to
+50
| } catch (err) { | ||
| console.log(data || '{}'); |
There was a problem hiding this comment.
Parse-failure fallback can emit invalid JSON to stdout.
When parsing fails, console.log(data || '{}') can output arbitrary non-JSON text, which is unsafe for hook chains expecting JSON.
Proposed fix
} catch (err) {
- console.log(data || '{}');
+ console.error('[ProWorkflow] Invalid hook payload JSON');
+ console.log('{}');
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| } catch (err) { | |
| console.log(data || '{}'); | |
| } catch (err) { | |
| console.error('[ProWorkflow] Invalid hook payload JSON'); | |
| console.log('{}'); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/setup-hook.js` around lines 49 - 50, The catch block currently does
console.log(data || '{}') which can print non-JSON text; change the error path
in the catch (err) handling so it never emits arbitrary data — always emit a
valid JSON fallback (e.g., JSON.stringify({}) or the literal '{}' ) to stdout
and send the parse error to stderr via console.error; update the catch
surrounding the parsing logic that references data and err to output only the
safe JSON fallback to stdout and log err to stderr.
skills/file-watcher/SKILL.md
Outdated
Comment on lines
+20
to
+32
| Claude Code's `SessionStart` and `CwdChanged` hooks can return `watchPaths` to register file watchers: | ||
|
|
||
| ```json | ||
| { | ||
| "hookSpecificOutput": { | ||
| "hookEventName": "SessionStart", | ||
| "watchPaths": [ | ||
| "/absolute/path/to/.env", | ||
| "/absolute/path/to/package.json" | ||
| ] | ||
| } | ||
| } | ||
| ``` |
There was a problem hiding this comment.
Documented CwdChanged watch registration behavior does not match current implementation.
This section implies active watch-path registration, but scripts/cwd-changed.js currently echoes input and writes env vars only; it does not emit hookSpecificOutput.watchPaths. Users following this guide may expect watchers that never get registered.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@skills/file-watcher/SKILL.md` around lines 20 - 32, The docs claim CwdChanged
(and SessionStart) can return hookSpecificOutput.watchPaths but the current
CwdChanged implementation (scripts/cwd-changed.js) only echoes input and writes
env vars; update the implementation to emit the same structure the docs show by
having the CwdChanged handler include a hookSpecificOutput object with a
watchPaths array (absolute paths) when appropriate, i.e., modify the CwdChanged
export to build and print JSON containing
{"hookSpecificOutput":{"hookEventName":"CwdChanged","watchPaths":[...]}}, or
alternatively update the SKILL.md text to explicitly state that CwdChanged
currently does not register watchers—choose the implementation change if you
want behavior to match docs, otherwise update the documentation to match the
existing CwdChanged function behavior.
- Validate JSON shape with Array.isArray before array ops (permission-denied.js)
- Exclude future timestamps from recent denials filter (setup-hook.js)
- Always emit valid JSON '{}' on parse failure (setup-hook.js)
- Add fenced code language for markdownlint MD040 (compact-guard.md, permission-tuner.md)
- Fix compound adjective: "top-priority file" (compact-guard SKILL.md)
- Update permission-tuner description to mention alwaysDeny rules
- Clarify file-watcher docs: watchPaths is API capability, cwd-changed.js does env injection
- Fix permission-analyst to reference actual denial log path, remove phantom approval data
- Version badge: v2.0 → v3.2 - Component counts: 24 skills, 8 agents, 24 hooks, 21 commands, 29 scripts - Added 7 new hook events to hooks diagram (PermissionDenied, WorktreeCreate, WorktreeRemove, CwdChanged, TaskCreated, LLM Gate) - Added 3 new agent cards (Context Engineer, Permission Analyst, Cost Analyst) - Expanded skills table with 7 new v3.2 skills highlighted in coral - Updated all section titles to reflect new counts
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.
Summary
type: "prompt"hooks for commit message validation and secret detection using HaikuomitClaudeMd: trueon 6 read-only agents to skip context loadingifconditionals for targeted filtering without spawning processesWhat's new
Key features
LLM Gates — First plugin to use
type: "prompt"hooks for AI-powered quality gates. Commit messages are validated against conventional commits format, and file writes are scanned for hardcoded secrets — all using Haiku for fast, cheap verification.Permission Tuner — Analyzes permission denial patterns and generates optimized alwaysAllow/alwaysDeny rules, categorized by risk level (safe/medium/dangerous). Reduces prompt fatigue.
Compact Guard — Protects context through compaction cycles. Key insight: compaction only restores 5 files with 5K tokens each within a 50K budget. This skill helps you prepare.
Cost Tracker — Session cost awareness with budget benchmarks by task type and optimization strategies.
Auto Setup — Detects project type (Node/Python/Rust/Go) and configures quality gates automatically.
MCP Audit — Analyzes MCP server overhead (each server adds ALL tool descriptions to every API request) and recommends servers to disable.
File Watcher — Configures reactive workflows using FileChanged/CwdChanged hooks with environment variable injection via CLAUDE_ENV_FILE.
Test plan
git commitwith bad message formatomitClaudeMd: truedoesn't break read-only agents/Summary by CodeRabbit
New Features
Enhancements
Documentation