feat: add Copilot and Codex hook parity - #1788
Conversation
- enable new Codex feature flags in managed config - register dcg in Codex and Copilot pre-tool hooks - let shared GitHub blockers parse Copilot hook input Co-authored-by: Codex <noreply@openai.com>
Co-authored-by: Codex <noreply@openai.com>
Co-authored-by: Codex <noreply@openai.com>
Co-authored-by: Codex <noreply@openai.com>
Co-authored-by: Codex <noreply@openai.com>
Co-authored-by: Codex <noreply@openai.com>
Co-authored-by: Codex <noreply@openai.com>
|
📝 WalkthroughSummary by CodeRabbit
WalkthroughThis PR extends the dotfiles configuration system to support GitHub Copilot alongside existing Claude Code and Codex integration. It generalizes hook scripts to handle multiple JSON input formats from different tools, adds Copilot-specific activation and configuration, enables new Codex feature flags, and provides comprehensive test coverage for cross-tool compatibility. ChangesCopilot Integration and Multi-Tool Hook Harmonization
Possibly Related PRs
Suggested Labels
Poem
🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request extends the hook system—including rtk-rewrite, security, and shared blockers—to support GitHub Copilot and Codex input formats, and introduces a new corc fish function for remote control. Feedback highlights the need to handle non-object inputs in the rtk-rewrite hook to prevent crashes, the importance of maintaining numeric fallbacks in the auto-switch script, and a recommendation to use command -s instead of which for more reliable command resolution in fish scripts.
| ORIGINAL_INPUT=$(echo "$INPUT" | jq -c ' | ||
| ( | ||
| .tool_input | ||
| // .tool.input | ||
| // .toolArgs | ||
| // .toolInput | ||
| // {} | ||
| ) | if type == "string" then (fromjson? // {}) else . end | ||
| ') | ||
| UPDATED_INPUT=$(echo "$ORIGINAL_INPUT" | jq --arg cmd "$REWRITTEN" '.command = $cmd') | ||
| IS_COPILOT_INPUT=$(echo "$INPUT" | jq -r 'has("toolName") and has("toolArgs")') |
There was a problem hiding this comment.
There are two issues in this block:
- The
ORIGINAL_INPUTextraction (lines 90-98) is missing.commandas a fallback source. This causes other top-level fields to be lost when the input is a simple command object (common in Codex), contradicting the goal of preserving original fields. - The
IS_COPILOT_INPUTcheck (line 100) will cause the script to crash if the input is a JSON string (e.g.,"ls"), ashas()only works on objects/arrays. Sinceset -eis active (line 32), the script will terminate prematurely.
Adding a type check and including .command in the fallback chain resolves both issues.
| ORIGINAL_INPUT=$(echo "$INPUT" | jq -c ' | |
| ( | |
| .tool_input | |
| // .tool.input | |
| // .toolArgs | |
| // .toolInput | |
| // {} | |
| ) | if type == "string" then (fromjson? // {}) else . end | |
| ') | |
| UPDATED_INPUT=$(echo "$ORIGINAL_INPUT" | jq --arg cmd "$REWRITTEN" '.command = $cmd') | |
| IS_COPILOT_INPUT=$(echo "$INPUT" | jq -r 'has("toolName") and has("toolArgs")') | |
| ORIGINAL_INPUT=$(echo "$INPUT" | jq -c ' | |
| ( | |
| .tool_input | |
| // .tool.input | |
| // .toolArgs | |
| // .toolInput | |
| // (if type == "object" and has("command") then . else {} end) | |
| ) | if type == "string" then (fromjson? // {}) else . end | |
| ') | |
| UPDATED_INPUT=$(echo "$ORIGINAL_INPUT" | jq --arg cmd "$REWRITTEN" '.command = $cmd') | |
| IS_COPILOT_INPUT=$(echo "$INPUT" | jq -r 'type == "object" and has("toolName") and has("toolArgs")') |
References
- Scripts should handle failures gracefully to avoid premature termination, especially when set -e is active or when used in critical execution paths.
| ORIGINAL_INPUT=$(echo "$INPUT" | jq -c ' | ||
| ( | ||
| .tool_input | ||
| // .tool.input | ||
| // .toolArgs | ||
| // .toolInput | ||
| // {} | ||
| ) | if type == "string" then (fromjson? // {}) else . end | ||
| ') | ||
| UPDATED_INPUT=$(echo "$ORIGINAL_INPUT" | jq --arg cmd "$REWRITTEN" '.command = $cmd') | ||
| IS_COPILOT_INPUT=$(echo "$INPUT" | jq -r 'has("toolName") and has("toolArgs")') |
There was a problem hiding this comment.
There are two issues in this block:
- The
ORIGINAL_INPUTextraction (lines 90-98) is missing.commandas a fallback source. This causes other top-level fields to be lost when the input is a simple command object (common in Codex), contradicting the goal of preserving original fields. - The
IS_COPILOT_INPUTcheck (line 100) will cause the script to crash if the input is a JSON string (e.g.,"ls"), ashas()only works on objects/arrays. Sinceset -eis active (line 32), the script will terminate prematurely.
Adding a type check and including .command in the fallback chain resolves both issues.
| ORIGINAL_INPUT=$(echo "$INPUT" | jq -c ' | |
| ( | |
| .tool_input | |
| // .tool.input | |
| // .toolArgs | |
| // .toolInput | |
| // {} | |
| ) | if type == "string" then (fromjson? // {}) else . end | |
| ') | |
| UPDATED_INPUT=$(echo "$ORIGINAL_INPUT" | jq --arg cmd "$REWRITTEN" '.command = $cmd') | |
| IS_COPILOT_INPUT=$(echo "$INPUT" | jq -r 'has("toolName") and has("toolArgs")') | |
| ORIGINAL_INPUT=$(echo "$INPUT" | jq -c ' | |
| ( | |
| .tool_input | |
| // .tool.input | |
| // .toolArgs | |
| // .toolInput | |
| // (if type == "object" and has("command") then . else {} end) | |
| ) | if type == "string" then (fromjson? // {}) else . end | |
| ') | |
| UPDATED_INPUT=$(echo "$ORIGINAL_INPUT" | jq --arg cmd "$REWRITTEN" '.command = $cmd') | |
| IS_COPILOT_INPUT=$(echo "$INPUT" | jq -r 'type == "object" and has("toolName") and has("toolArgs")') |
References
- Scripts should handle failures gracefully to avoid premature termination, especially when set -e is active or when used in critical execution paths.
|
|
||
| # Require at least 2 managed accounts | ||
| ACCOUNT_COUNT=$(cswap --list 2>/dev/null | grep -c '^\s*[0-9]' || echo 0) | ||
| ACCOUNT_COUNT=$(cswap --list 2>/dev/null | grep -c '^\s*[0-9]' || true) |
There was a problem hiding this comment.
Changing || echo 0 to || true reduces the robustness of the script. While grep -c typically outputs 0 even when no matches are found (exiting with status 1), using || true means that if grep fails to produce any output (e.g., due to an internal error or being killed), ACCOUNT_COUNT will be assigned an empty string. This will cause a syntax error in the subsequent numeric comparison [ "$ACCOUNT_COUNT" -lt 2 ]. Reverting to || echo 0 ensures a valid numeric fallback.
| ACCOUNT_COUNT=$(cswap --list 2>/dev/null | grep -c '^\s*[0-9]' || true) | |
| ACCOUNT_COUNT=$(cswap --list 2>/dev/null | grep -c '^\s*[0-9]' || echo 0) |
| # keeps its reference to the old inode and is unaffected. | ||
| # Usage: corc [<codex remote-control args...>] | ||
|
|
||
| set -l codex_real (realpath (which codex)) |
There was a problem hiding this comment.
Using which in scripts is generally discouraged as its behavior varies across systems; command -s in fish is more reliable. Additionally, if codex is not found in the PATH, command -s will return nothing, causing realpath to fail with an error. It's safer to verify the command exists before attempting to resolve its path.
set -l codex_path (command -s codex)
if test -z "$codex_path"
echo "Error: codex command not found" >&2
return 1
end
set -l codex_real (realpath "$codex_path")
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@config/copilot/config.json`:
- Around line 11-15: The pre-tool hook that runs "command -v dcg >/dev/null 2>&1
&& dcg" is missing a matcher and thus runs for every tool; restrict it to
shell-only tools by adding a matcher entry to that hook (e.g., add a "matcher"
key that allows only shell/bash tools) so the command-type hook only executes
for shell tool invocations and does not run for non-shell tools.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: fd2e008d-9ee7-48aa-a651-22d68ea7df27
📒 Files selected for processing (24)
config/claude/hooks/auto-switch.shconfig/claude/hooks/rtk-rewrite.shconfig/codex/config.tomlconfig/codex/config.tpl.tomlconfig/codex/hooks.jsonconfig/codex/hooks/rtk-rewrite.shconfig/codex/hooks/security.shconfig/copilot/activate.shconfig/copilot/config.jsonconfig/copilot/default.nixconfig/default.nixconfig/shared/hooks/block-gh-settings.shconfig/shared/hooks/block-git-push.shhome-manager/programs/fish/default.nixhome-manager/programs/fish/functions/_corc_function.fishspec/activate_config_spec.shspec/block_gh_settings_spec.shspec/block_git_push_spec.shspec/codex_rtk_rewrite_spec.shspec/codex_security_spec.shspec/coverage_spec.shspec/fish/_corc_function_test.fishspec/rtk_rewrite_spec.shspec/sync_rtk_rewrite_spec.sh
| { | ||
| "type": "command", | ||
| "command": "command -v dcg >/dev/null 2>&1 && dcg", | ||
| "timeout": 5 | ||
| }, |
There was a problem hiding this comment.
Restrict dcg to shell tools here too.
This is the only pre-tool hook in this chain without a matcher, so Copilot will run it for every tool call. The stack contract for this PR keeps dcg in the Bash-only path on the Codex side, so this currently breaks the parity goal and can make non-shell tools pay the hook cost or get blocked unexpectedly.
💡 Proposed fix
{
"type": "command",
+ "matcher": "bash|shell|Bash",
"command": "command -v dcg >/dev/null 2>&1 && dcg",
"timeout": 5
},📝 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.
| { | |
| "type": "command", | |
| "command": "command -v dcg >/dev/null 2>&1 && dcg", | |
| "timeout": 5 | |
| }, | |
| { | |
| "type": "command", | |
| "matcher": "bash|shell|Bash", | |
| "command": "command -v dcg >/dev/null 2>&1 && dcg", | |
| "timeout": 5 | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@config/copilot/config.json` around lines 11 - 15, The pre-tool hook that runs
"command -v dcg >/dev/null 2>&1 && dcg" is missing a matcher and thus runs for
every tool; restrict it to shell-only tools by adding a matcher entry to that
hook (e.g., add a "matcher" key that allows only shell/bash tools) so the
command-type hook only executes for shell tool invocations and does not run for
non-shell tools.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
3 issues found across 24 files
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="config/codex/hooks/security.sh">
<violation number="1" location="config/codex/hooks/security.sh:12">
P1: Unsafe jq field access can abort the security hook when `.tool` is not an object. Guard the `.tool.name` lookup so mixed input shapes don't crash the hook.</violation>
<violation number="2" location="config/codex/hooks/security.sh:19">
P1: The first command-path in the jq fallback chain is not type-safe; it can fail fast and skip all fallback parsing.</violation>
</file>
<file name="config/shared/hooks/block-git-push.sh">
<violation number="1" location="config/shared/hooks/block-git-push.sh:17">
P1: This jq path can throw on valid inputs (e.g. when `tool` is a string), causing the hook to exit early under `set -euo pipefail` instead of reading fallback command fields.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| esac | ||
|
|
||
| command=$(echo "$input" | jq -r ' | ||
| .tool.input.command |
There was a problem hiding this comment.
P1: The first command-path in the jq fallback chain is not type-safe; it can fail fast and skip all fallback parsing.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At config/codex/hooks/security.sh, line 19:
<comment>The first command-path in the jq fallback chain is not type-safe; it can fail fast and skip all fallback parsing.</comment>
<file context>
@@ -1,18 +1,29 @@
+esac
+
+command=$(echo "$input" | jq -r '
+ .tool.input.command
+ // .tool_input.command
+ // (.toolArgs | if type == "object" then .command else empty end)
</file context>
|
|
||
| command=$(echo "$input" | jq -r '.tool_input.command // empty' 2>/dev/null) | ||
| # Only process shell commands when the hook input includes a tool name. | ||
| tool_name=$(echo "$input" | jq -r '.tool.name // .tool_name // .toolName // empty' 2>/dev/null) |
There was a problem hiding this comment.
P1: Unsafe jq field access can abort the security hook when .tool is not an object. Guard the .tool.name lookup so mixed input shapes don't crash the hook.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At config/codex/hooks/security.sh, line 12:
<comment>Unsafe jq field access can abort the security hook when `.tool` is not an object. Guard the `.tool.name` lookup so mixed input shapes don't crash the hook.</comment>
<file context>
@@ -1,18 +1,29 @@
-
-command=$(echo "$input" | jq -r '.tool_input.command // empty' 2>/dev/null)
+# Only process shell commands when the hook input includes a tool name.
+tool_name=$(echo "$input" | jq -r '.tool.name // .tool_name // .toolName // empty' 2>/dev/null)
+case "$tool_name" in
+"" | Bash | bash | shell) ;;
</file context>
| tool_name=$(echo "$input" | jq -r '.tool.name // .tool_name // .toolName // empty' 2>/dev/null) | |
| tool_name=$(echo "$input" | jq -r '(.tool | if type == "object" then .name else empty end) // .tool_name // .toolName // empty' 2>/dev/null) |
| # Extract command (works for both Claude and Codex input formats) | ||
| command=$(echo "$input" | jq -r '.tool_input.command // .command // empty' 2>/dev/null) | ||
| # Extract command (works for Claude, Codex, and Copilot hook input formats) | ||
| command=$(echo "$input" | jq -r '.tool.input.command // .tool_input.command // .toolArgs.command // .toolInput.command // .command // empty' 2>/dev/null) |
There was a problem hiding this comment.
P1: This jq path can throw on valid inputs (e.g. when tool is a string), causing the hook to exit early under set -euo pipefail instead of reading fallback command fields.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At config/shared/hooks/block-git-push.sh, line 17:
<comment>This jq path can throw on valid inputs (e.g. when `tool` is a string), causing the hook to exit early under `set -euo pipefail` instead of reading fallback command fields.</comment>
<file context>
@@ -13,8 +13,8 @@ ALLOWED_REPOS=(
-# Extract command (works for both Claude and Codex input formats)
-command=$(echo "$input" | jq -r '.tool_input.command // .command // empty' 2>/dev/null)
+# Extract command (works for Claude, Codex, and Copilot hook input formats)
+command=$(echo "$input" | jq -r '.tool.input.command // .tool_input.command // .toolArgs.command // .toolInput.command // .command // empty' 2>/dev/null)
[[ -z $command ]] && exit 0
</file context>
| command=$(echo "$input" | jq -r '.tool.input.command // .tool_input.command // .toolArgs.command // .toolInput.command // .command // empty' 2>/dev/null) | |
| command=$(echo "$input" | jq -r '.tool.input.command? // .tool_input.command? // .toolArgs.command? // .toolInput.command? // .command? // empty' 2>/dev/null) |
![indent-zero[bot]](https://avatars.githubusercontent.com/in/1426746?s=60&v=4){kind=small}
| # Extract command (works for both Claude and Codex input formats) | ||
| command=$(echo "$input" | jq -r '.tool_input.command // .command // empty' 2>/dev/null) | ||
| # Extract command (works for Claude, Codex, and Copilot hook input formats) | ||
| command=$(echo "$input" | jq -r '.tool.input.command // .tool_input.command // .toolArgs.command // .toolInput.command // .command // empty' 2>/dev/null) |
There was a problem hiding this comment.
Copilot string toolArgs not blocked: this only reads .toolArgs.command (object form). When Copilot sends toolArgs as a JSON-encoded string — the variant config/codex/hooks/security.sh and config/codex/hooks/rtk-rewrite.sh explicitly handle via fromjson — jq errors with Cannot index string with string "command", the error is swallowed by 2>/dev/null, command is empty, and [[ -z $command ]] && exit 0 lets the push through.
Reproduce:
echo '{"toolName":"shell","toolArgs":"{\"command\":\"git push origin main\"}"}' \
| jq -r '.tool.input.command // .tool_input.command // .toolArgs.command // .toolInput.command // .command // empty' 2>/dev/null
# -> empty
Replace the extraction with the same union expression used in config/codex/hooks/rtk-rewrite.sh:34-43 (object + string branches). Same fix needed in config/shared/hooks/block-gh-settings.sh:10.
| CONFIG_JSON="$1" | ||
|
|
||
| mkdir -p ~/.copilot | ||
| cp -f "$CONFIG_JSON" ~/.copilot/config.json |
There was a problem hiding this comment.
Wipes Copilot-written state on every activation: config/copilot/default.nix:3 calls out that "Copilot CLI mutates config.json", which is why a Nix symlink isn't used — but cp -f then unconditionally overwrites the runtime file every home-manager switch, blowing away whatever Copilot wrote (auth tokens, banner, model prefs, etc.). The new spec/activate_config_spec.sh "replaces existing config with the managed config" case explicitly tests-and-locks this: a pre-existing "banner": "never" and an extra hook are gone after activation.
If preserving Copilot's mutations matters, merge instead of overwrite, e.g.:
jq -s '.[0] * .[1]' ~/.copilot/config.json "$CONFIG_JSON" > ~/.copilot/config.json.new \
&& mv -f ~/.copilot/config.json.new ~/.copilot/config.json
(or restrict the merge to just hooks and disableAllHooks). If wholesale overwrite is intended, the default.nix comment should reflect that.
| tool_search = true | ||
| tool_search_always_defer_mcp_tools = true | ||
| tool_suggest = true | ||
| terminal_resize_reflow = true |
There was a problem hiding this comment.
Out of alphabetical order — the rest of [features] is sorted, so this should go above tool_call_mcp_elicitation. Same fix needed in config/codex/config.tpl.toml.
| }, | ||
| { | ||
| "type": "command", | ||
| "command": "command -v dcg >/dev/null 2>&1 && dcg", |
There was a problem hiding this comment.
Missing matcher breaks Copilot/Codex parity: this dcg hook (and the block-git-push.sh / block-gh-settings.sh entries below) has no matcher field, so Copilot will run it for every tool call — file reads, MCP, etc. — not just shell. The Codex chain in config/codex/hooks.json:22-65 keeps the entire group under "matcher": "Bash", so on Codex dcg only fires for Bash. Add "matcher": "bash|shell|Bash" here (and to the two block-* entries) to match the sibling rtk/security entries and the Codex chain.
{
"type": "command",
"matcher": "bash|shell|Bash",
"command": "command -v dcg >/dev/null 2>&1 && dcg",
"timeout": 5
}
…1795) * fix: add Copilot toolArgs/modifiedArgs support to rtk-rewrite hook Tests added in #1788 check that rtk-rewrite.sh reads Copilot-format input (.toolArgs as object or stringified JSON) and emits .modifiedArgs output, but the hook itself was never updated, so the Shell workflow has been failing on every push to main. #1794 then added two new copilot hook files without updating the coverage list, adding a 5th failure. - Read CMD from .tool_input.command OR .toolArgs.command (object) OR .toolArgs | fromjson | .command (string). - Emit modifiedArgs alongside hookSpecificOutput when input used toolArgs, preserving original fields (e.g. timeout). - Mirror to claude/codex/copilot copies (sync_rtk_rewrite_spec.sh enforces byte equality between claude and codex). - Add config/copilot/hooks/{rtk-rewrite,security}.sh to coverage_spec.sh. Note: scripts/sync-rtk-rewrite.sh fetches from upstream and overwrites all three local copies — next sync will wipe this fix. Documented inline. * fix(hooks): tolerant fromjson, exclude copilot from shfmt, preserve sync Entire-Checkpoint: 9d826b214b88 * refactor(sync): drop patch file, make sync drift-check-only Entire-Checkpoint: 8c976510b69a
1 participant
Changes
Testing
Generated with Codex.
Summary by cubic
Adds Copilot pre‑tool hooks with parity to Codex/Claude for RTK rewrite, security checks, and GitHub push/settings blockers. Also enables new Codex feature flags, registers dcg in both chains, and adds a stable fish corc wrapper for remote‑control.
New Features
Bug Fixes
Written for commit 7e1604c. Summary will update on new commits.