feat: support Gemini CLI with improved security, UX, and xargs wrapping - #16
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 7 minutes and 15 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughRenames and rekeys hooks for Gemini ( Changes
Sequence DiagramsequenceDiagram
participant CLI as Client (Gemini/Claude)
participant Hook as Hook System
participant Validator as validate-cloud-command.py
participant Response as Response
rect rgba(100,150,200,0.5)
Note over CLI,Hook: Gemini flow
CLI->>Hook: run_shell_command (BeforeTool)
Hook->>Validator: invoke with hook_event_name: "BeforeTool" + tool_input
Validator->>Validator: detect platform == "gemini"
alt blocked
Validator->>Response: {decision: "deny", reason, systemMessage}
Response-->>CLI: Gemini deny JSON
else allowed
Validator->>Response: exit 0, empty stdout
Response-->>CLI: allow execution
end
end
rect rgba(200,100,100,0.5)
Note over CLI,Hook: Claude flow
CLI->>Hook: PreToolUse / SessionStart
Hook->>Validator: invoke with Claude payload
Validator->>Validator: detect platform == "claude"
alt blocked
Validator->>Response: {hookSpecificOutput: {permissionDecision: "deny", permissionDecisionReason}}
Response-->>CLI: Claude deny JSON
else allowed
Validator->>Response: no intervention
Response-->>CLI: allow execution
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
skills/gemini/SKILL.md (1)
19-19:⚠️ Potential issue | 🟡 MinorSkill text still references old hook/tool names.
The body mentions “Bash” and
PreToolUse, but current hook config usesrun_shell_commandandBeforeTool.🛠️ Suggested text update
-DeployShield is active. All Bash commands are validated before execution. +DeployShield is active. All shell commands are validated before execution. ... -This skill uses the core DeployShield validator via a PreToolUse hook. +This skill uses the core DeployShield validator via a BeforeTool hook.Also applies to: 33-33
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@skills/gemini/SKILL.md` at line 19, The documentation line still refers to "Bash" and the old hook name `PreToolUse`; update the skill text to reference the current names instead — replace "Bash" with "shell" or "run_shell_command" and change `PreToolUse` to `BeforeTool` (and similarly update the occurrence at line 33) so the description matches the current hook/config (`run_shell_command` and `BeforeTool`) and reads clearly with the new terminology.hooks/scripts/validate-cloud-command.py (1)
5-5:⚠️ Potential issue | 🟡 MinorUpdate stale hook-event wording in module docstring.
Line 5 still references
PreToolUse, which no longer matches the active hook event naming.🛠️ Suggested doc fix
-Reads PreToolUse hook JSON from stdin. +Reads BeforeTool hook JSON from stdin.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hooks/scripts/validate-cloud-command.py` at line 5, The module docstring in hooks/scripts/validate-cloud-command.py still references the old hook event name 'PreToolUse'; update the module-level docstring to use the project's current hook event name (replace the literal 'PreToolUse' with the active hook event identifier/casing used elsewhere in the codebase or hook registry) so the wording matches the rest of the project.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/superpowers/plans/2026-03-29-gemini-cli-compatibility.md`:
- Line 7: Fix the hyphenation in the "Architecture" line by making "Gemini CLI
specific" a proper compound adjective (e.g., "Gemini-CLI-specific" or use an
en‑dash "Gemini CLI–specific"), and correct the heading level jump at line 13 so
headings follow a consistent incremental hierarchy (adjust the heading at line
13 to match the surrounding levels rather than skipping levels). Ensure the
manifest/command phrasing in the Architecture paragraph and the heading
hierarchy are consistent across the doc.
In `@gemini-extension.json`:
- Line 9: The manifest field contextFileName references "GEMINI.md" but that
file is missing, causing context loading to fail; either add a new GEMINI.md
document with the intended context content to the repo or remove/replace the
contextFileName entry in gemini-extension.json (the "contextFileName" key) so it
no longer points to a nonexistent file; ensure whichever path/name you use
matches an actual file committed to the repository.
---
Outside diff comments:
In `@hooks/scripts/validate-cloud-command.py`:
- Line 5: The module docstring in hooks/scripts/validate-cloud-command.py still
references the old hook event name 'PreToolUse'; update the module-level
docstring to use the project's current hook event name (replace the literal
'PreToolUse' with the active hook event identifier/casing used elsewhere in the
codebase or hook registry) so the wording matches the rest of the project.
In `@skills/gemini/SKILL.md`:
- Line 19: The documentation line still refers to "Bash" and the old hook name
`PreToolUse`; update the skill text to reference the current names instead —
replace "Bash" with "shell" or "run_shell_command" and change `PreToolUse` to
`BeforeTool` (and similarly update the occurrence at line 33) so the description
matches the current hook/config (`run_shell_command` and `BeforeTool`) and reads
clearly with the new terminology.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c1e71219-38a1-42cf-94d3-b61c63c89f95
📒 Files selected for processing (6)
commands/deployshield-status.tomldocs/superpowers/plans/2026-03-29-gemini-cli-compatibility.mdgemini-extension.jsonhooks/hooks.jsonhooks/scripts/validate-cloud-command.pyskills/gemini/SKILL.md
|
|
||
| **Goal:** Fix DeployShield's integration with Gemini CLI by updating hook names, matchers, and extension manifest structure. | ||
|
|
||
| **Architecture:** Add parallel support for Gemini CLI specific hook events (`BeforeTool`) and tool names (`run_shell_command`) alongside existing Claude Code hooks. Normalize the extension manifest and command definitions for Gemini discovery. |
There was a problem hiding this comment.
Resolve markdown/style lint warnings in the plan doc.
There is a hyphenation issue at Line 7 and a heading level jump at Line 13.
🛠️ Suggested doc lint fix
-**Architecture:** Add parallel support for Gemini CLI specific hook events (`BeforeTool`) and tool names (`run_shell_command`) alongside existing Claude Code hooks. Normalize the extension manifest and command definitions for Gemini discovery.
+**Architecture:** Add parallel support for Gemini CLI-specific hook events (`BeforeTool`) and tool names (`run_shell_command`) alongside existing Claude Code hooks. Normalize the extension manifest and command definitions for Gemini discovery.
-### Task 1: Update Extension Manifest
+## Task 1: Update Extension ManifestAlso applies to: 13-13
🧰 Tools
🪛 LanguageTool
[grammar] ~7-~7: Use a hyphen to join words.
Context: ...e:** Add parallel support for Gemini CLI specific hook events (BeforeTool) and ...
(QB_NEW_EN_HYPHEN)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/superpowers/plans/2026-03-29-gemini-cli-compatibility.md` at line 7, Fix
the hyphenation in the "Architecture" line by making "Gemini CLI specific" a
proper compound adjective (e.g., "Gemini-CLI-specific" or use an en‑dash "Gemini
CLI–specific"), and correct the heading level jump at line 13 so headings follow
a consistent incremental hierarchy (adjust the heading at line 13 to match the
surrounding levels rather than skipping levels). Ensure the manifest/command
phrasing in the Architecture paragraph and the heading hierarchy are consistent
across the doc.
| "commands": [ | ||
| "commands/deployshield-status.md" | ||
| ] | ||
| "contextFileName": "GEMINI.md" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Ensure the referenced context file exists (case-sensitive check).
fd -HI '^GEMINI\.md$' .
# 2) Confirm the manifest points to the same filename.
rg -n --fixed-strings '"contextFileName": "GEMINI.md"' gemini-extension.jsonRepository: matanryngler/deployshield
Length of output: 103
Add missing GEMINI.md file or remove the contextFileName reference.
The manifest references GEMINI.md at line 9, but this file does not exist in the repository. Context loading will fail at runtime unless the file is added or the reference is removed.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@gemini-extension.json` at line 9, The manifest field contextFileName
references "GEMINI.md" but that file is missing, causing context loading to
fail; either add a new GEMINI.md document with the intended context content to
the repo or remove/replace the contextFileName entry in gemini-extension.json
(the "contextFileName" key) so it no longer points to a nonexistent file; ensure
whichever path/name you use matches an actual file committed to the repository.
34f4133 to
152fc89
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/test_integration.py (1)
169-184: Good test coverage for Gemini format, but consider adding a negative case.The tests correctly verify:
- Blocked commands return the Gemini-specific JSON schema (
decision,reason,systemMessage)- Allowed commands exit cleanly with no output
Consider adding a test that verifies the Claude format is not present in Gemini responses (e.g., asserting
"hookSpecificOutput" not in result) to catch accidental format mixing.💡 Optional: Add negative assertion
def test_gemini_block_format(self): """Verify that a blocked command returns Gemini-specific JSON format.""" code, out = run_validator_gemini("terraform apply") assert code == 0 result = json.loads(out) assert "decision" in result assert "reason" in result assert "systemMessage" in result assert result["decision"] == "deny" + # Ensure Claude format is not present + assert "hookSpecificOutput" not in result🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/test_integration.py` around lines 169 - 184, Add a negative assertion to the Gemini tests to ensure Claude-specific fields are not mixed into Gemini output: in TestGeminiCompatibility.test_gemini_block_format (which calls run_validator_gemini and parses result), after checking "decision","reason","systemMessage" and result["decision"] == "deny", assert that the Claude-specific key "hookSpecificOutput" (or any Claude-only fields) is not present (e.g., "hookSpecificOutput" not in result); you can also add a similar negative assertion in test_gemini_allow_format to ensure the empty output does not contain Claude fields.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@tests/test_integration.py`:
- Around line 169-184: Add a negative assertion to the Gemini tests to ensure
Claude-specific fields are not mixed into Gemini output: in
TestGeminiCompatibility.test_gemini_block_format (which calls
run_validator_gemini and parses result), after checking
"decision","reason","systemMessage" and result["decision"] == "deny", assert
that the Claude-specific key "hookSpecificOutput" (or any Claude-only fields) is
not present (e.g., "hookSpecificOutput" not in result); you can also add a
similar negative assertion in test_gemini_allow_format to ensure the empty
output does not contain Claude fields.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c514ec81-fb0d-4c88-8eb1-ec1e0f8eedfe
📒 Files selected for processing (5)
docs/superpowers/plans/2026-03-29-gemini-support.mdhooks/hooks.jsonhooks/scripts/validate-cloud-command.pyskills/gemini/SKILL.mdtests/test_integration.py
🚧 Files skipped from review as they are similar to previous changes (2)
- skills/gemini/SKILL.md
- hooks/hooks.json
4342e80 to
1586566
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
hooks/claude-hooks.json (1)
20-20: Keep the SessionStart policy summary in one source of truth.This inline message is already drifting from the actual validator rules in
hooks/scripts/validate-cloud-command.py:check_npm()also blocksunpublish, andcheck_gem()blocksyank, but the text only mentionspublish/gem. Generating this payload from a shared constant or small script will keep Claude’s onboarding text in sync as the policy evolves.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hooks/claude-hooks.json` at line 20, The SessionStart message in claude-hooks.json is duplicated and out of sync with the validator logic; instead of hardcoding the payload string, generate it from a single source of truth used by hooks/scripts/validate-cloud-command.py (e.g., reuse or export the policy list/constants that check_npm() and check_gem() rely on) and have the claude hook build hookSpecificOutput.hookEventName/ contextForAgent from that generator; update the hook to call that script or import the shared constants so entries like npm unpublish and gem yank are included automatically and the onboarding text always matches the validator rules.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.claude-plugin/plugin.json:
- Line 27: Update the "hooks" value in plugin.json to use a relative path that
starts with ./ so it conforms to Claude plugin spec: change the "hooks" field
value from "hooks/claude-hooks.json" to "./hooks/claude-hooks.json" (edit the
"hooks" entry in .claude-plugin/plugin.json).
In `@hooks/scripts/validate-cloud-command.py`:
- Around line 1490-1508: The deny function currently embeds the raw cmd into
reason and systemMessage, which can leak secrets and control characters;
instead, replace uses of cmd in both the reason string and the gemini
systemMessage with a sanitized summary (e.g., a constant like
"<redacted-command>" or a sanitized version produced by a helper that strips
ANSI/control chars, newlines, and truncates to a safe length) and ensure you use
that sanitized variable in both reason and systemMessage (referencing deny,
reason, systemMessage and the cmd parameter); do not include the raw cmd,
tokens, or unescaped characters in any output.
- Around line 1585-1587: The current implicit platform detection sets platform =
"gemini" if hook_event == "BeforeTool" else "claude" and silently defaults
unknown hook_event values to Claude; change this to validate hook_event
explicitly: read hook_event (variable hook_event), if it equals "BeforeTool" set
platform="gemini", if it equals the documented Claude event (or an explicitly
passed platform parameter) set platform="claude", otherwise log an error and
fail fast (raise/exit) so unknown/unsupported hook_event values are rejected;
alternatively, accept an explicit platform parameter from configuration and
prefer that over auto-detection to make intent clear.
---
Nitpick comments:
In `@hooks/claude-hooks.json`:
- Line 20: The SessionStart message in claude-hooks.json is duplicated and out
of sync with the validator logic; instead of hardcoding the payload string,
generate it from a single source of truth used by
hooks/scripts/validate-cloud-command.py (e.g., reuse or export the policy
list/constants that check_npm() and check_gem() rely on) and have the claude
hook build hookSpecificOutput.hookEventName/ contextForAgent from that
generator; update the hook to call that script or import the shared constants so
entries like npm unpublish and gem yank are included automatically and the
onboarding text always matches the validator rules.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: aa909f19-a87f-41bc-9624-578c5bf8b2f0
📒 Files selected for processing (5)
.claude-plugin/plugin.jsongemini-extension.jsonhooks/claude-hooks.jsonhooks/hooks.jsonhooks/scripts/validate-cloud-command.py
💤 Files with no reviewable changes (1)
- gemini-extension.json
🚧 Files skipped from review as they are similar to previous changes (1)
- hooks/hooks.json
There was a problem hiding this comment.
🧹 Nitpick comments (1)
hooks/scripts/validate-cloud-command.py (1)
1618-1622: Validate--session-startplatform values explicitly.Right now, unknown values silently fall back to Claude output. Rejecting invalid values makes misconfiguration obvious.
Suggested patch
def get_session_start_message(platform: str = "claude") -> None: @@ - if platform == "gemini": + if platform == "gemini": result = {"additionalContext": msg} - else: + elif platform == "claude": result = { "hookSpecificOutput": { "hookEventName": "SessionStart", "contextForAgent": msg, } } + else: + sys.stderr.write(f"Error: Unsupported platform for session start: {platform}\n") + sys.exit(1) @@ def main() -> None: @@ if len(sys.argv) > 1 and sys.argv[1] == "--session-start": platform = "claude" if len(sys.argv) > 2: platform = sys.argv[2] get_session_start_message(platform) returnAlso applies to: 1515-1518
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hooks/scripts/validate-cloud-command.py` around lines 1618 - 1622, Validate the platform argument passed to the --session-start handler instead of silently defaulting to "claude": when parsing sys.argv in the block that sets platform and calls get_session_start_message(platform), check that platform is one of the allowed values (e.g., "claude", "gpt", or whatever supported names your codebase uses), and if it is not, print a clear error message and exit with non-zero status; update both occurrences (the block around get_session_start_message and the similar block at the earlier lines) to perform this explicit whitelist check before calling get_session_start_message.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@hooks/scripts/validate-cloud-command.py`:
- Around line 1618-1622: Validate the platform argument passed to the
--session-start handler instead of silently defaulting to "claude": when parsing
sys.argv in the block that sets platform and calls
get_session_start_message(platform), check that platform is one of the allowed
values (e.g., "claude", "gpt", or whatever supported names your codebase uses),
and if it is not, print a clear error message and exit with non-zero status;
update both occurrences (the block around get_session_start_message and the
similar block at the earlier lines) to perform this explicit whitelist check
before calling get_session_start_message.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 38d873de-7b36-41ae-9f53-50477030abe5
📒 Files selected for processing (5)
.claude-plugin/plugin.jsonhooks/claude-hooks.jsonhooks/hooks.jsonhooks/scripts/validate-cloud-command.pytests/test_integration.py
✅ Files skipped from review due to trivial changes (2)
- .claude-plugin/plugin.json
- hooks/claude-hooks.json
🚧 Files skipped from review as they are similar to previous changes (2)
- hooks/hooks.json
- tests/test_integration.py
Summary
This PR adds full support for Gemini CLI as a first-class platform for DeployShield while maintaining 100% backward compatibility with Claude Code. It introduces a dual-platform hook architecture, improved security via command sanitization, and enhanced UX with colorful terminal blocking messages.
Key Changes
hooks/hooks.jsonis now Gemini-specific, eliminating the "Invalid hook event name" warning.hooks/claude-hooks.jsonhandles Claude hooks, with.claude-plugin/plugin.jsonupdated to point to it.hook_event_name(BeforeTool/PreToolUse) and fails fast for unsupported values.--session-startflag in the validator dynamically generates onboarding messages based on the actual guarded providers.xargsas a command wrapper (alongsidesudoandenv).Verification
claude-hooks.json.uv run pytest).Summary by CodeRabbit
New Features
Tests
Documentation