#510 Reference Claude skills and agents from Codex#511
Conversation
📝 WalkthroughWalkthroughAdds a Codex reference layer: six skill references under .agents/skills, 21 thin agent TOML wrappers under .codex/agents, .codex/config.toml MCP entries, .codex/hooks.json automation, documentation updates, ChangesCodex Reference Layer Implementation
Sequence Diagram(s)sequenceDiagram
participant Dev
participant CodexHooks as Codex Hooks (.codex/hooks.json)
participant Cla udeHook as Claude Hook (.claude/settings.json)
participant Verifier as verify_commit_command_hook.py
participant TSBuild as Playwright TS checks (npx tsc / npm format)
Dev->>CodexHooks: run git commit (or other bash)
CodexHooks->>Verifier: if commit-like -> verify hash & run helper
Cla udeHook->>Verifier: (alternate path) run helper when matched
Verifier->>TSBuild: on allowed -> run tsc & npm format in playground
TSBuild-->>Verifier: success/failure
Verifier-->>Dev: exit 0 or 2 (blocked)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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)
Comment |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary
PR #510 adds Codex AI tooling integration: 6 symlinks under .agents/skills/, 11 thin agent TOML wrappers under .codex/agents/, a Codex config with MCP servers, and a hooks pipeline — all delegating to the authoritative .claude/skills/ and .claude/agents/ sources.
What works
- Symlinks (
.agents/skills/*): all resolve to../../.claude/skills/<name>, each target exists. - Agent TOMLs: each is a parseable TOML whose
developer_instructionscorrectly point to the matching.claude/agents/<name>.md; deep-review-next's 11-agent roster matches 11 TOML files exactly. .codex/config.toml: MCP server declarations (MCP_DOCKER,coverage-matrix,playwright,playwright-report-mcp,quality-metrics) mirror the existing.mcp.jsonat the repo root — no divergence..codex/hooks.json: PostToolUse Edit/Write hook runstsc --noEmit+ format onplaywright/typescriptfiles; PreToolUse Bash git-commit hook runs the same checks project-wide; both use|| truefallbacks appropriate for gate hooks. The PreToolUse Bash hook correctly blocksplaywright testCLI with a descriptive error redirecting to the MCP.AGENTS.mdandREADME.md: updated to document the Claude-source / Codex-reference model, the.codex/directory structure, and the substitution table for Claude-to-Codex mechanics.
Minor observations (non-blocking)
- The
|| truepattern on the PostToolUse Edit/Write hook suppresses non-zero exit codes silently. If the hook command is unreachable (e.g.,jqnot installed), the edit proceeds without the TypeScript gate firing. Consider surfacing a warning, but this is not a regression vs. the current state (no gate exists today). hooks.jsonuses 10-second timeout for worktree provisioning hooks;provision-worktree-env.shmay need more time on cold Docker pulls. Not a blocking issue.test.fixme()stubs for Codex integration gaps are not yet scaffolded (the/generate-stubsskill exists but no stubs were added). This is pre-existing and outside this PR's scope.
Verdict
The PR correctly establishes the delegating symlink + thin-wrapper pattern without duplicating skill or agent text. Configuration is internally consistent, TOML is parseable, hooks are logically sound, and documentation is updated. CI completion is the only remaining gate.
_Reviewed by proxy/@preset/minimax-minimax-m2-7-no-thinking_
Reviewed by proxy/@preset/minimax-minimax-m2-7-no-thinking
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 @.codex/agents/deep-review-architecture.toml:
- Around line 1-10: This TOML is missing the mandatory top-level name key
required for Codex agents; add a unique name field (e.g., name =
"deep-review-architecture") to the top of
.codex/agents/deep-review-architecture.toml and likewise add a distinct name
entry to every other file under .codex/agents/*.toml so each agent can be
identified and spawned by name; ensure the name strings are unique, descriptive,
and match any internal references to the agent where applicable.
In @.codex/config.toml:
- Around line 19-23: Remove the insecure flag from the MCP server args: in the
mcp_servers.playwright-report-mcp configuration remove the "--min-release-age=0"
entry from the args array so only the pinned package
"playwright-report-mcp@3.1.0" remains; ensure the args array no longer contains
"--min-release-age=0" to re-enable the default min-release-age hardening
(CISA-recommended baseline).
In @.codex/hooks.json:
- Line 44: The hook currently uses grep -q 'git commit' on the extracted CMD
variable (INPUT/CMD) which matches any occurrence of that substring; tighten the
check to only match an actual commit invocation by replacing grep -q 'git
commit' with a anchored pattern such as grep -Eq
'^[[:space:]]*git[[:space:]]+commit\b' (or use a bash regex test like [[ $CMD =~
^[[:space:]]*git[[:space:]]+commit\b ]]) so only real "git commit" commands
trigger the TypeScript/format checks; keep the rest of the command flow (INPUT,
CMD, TSC/FORMAT checks and exit codes) unchanged.
🪄 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 Plus
Run ID: 444f6584-3646-4f85-8d36-5f55ae9764df
📒 Files selected for processing (21)
.agents/skills/create-issue.agents/skills/deep-review.agents/skills/deep-review-next.agents/skills/fix-issue.agents/skills/generate-stubs.agents/skills/generate-test.codex/agents/deep-review-architecture.toml.codex/agents/deep-review-ci.toml.codex/agents/deep-review-code.toml.codex/agents/deep-review-docs.toml.codex/agents/deep-review-project-checklist.toml.codex/agents/deep-review-python.toml.codex/agents/deep-review-qa.toml.codex/agents/deep-review-security.toml.codex/agents/deep-review-simplification.toml.codex/agents/deep-review-typescript.toml.codex/agents/deep-review-unit-test.toml.codex/config.toml.codex/hooks.jsonAGENTS.mdREADME.md
There was a problem hiding this comment.
Summary
PR #511 adds Codex integration to the repository — symlinks in .agents/skills/, TOML agent wrappers in .codex/agents/, MCP server config in .codex/config.toml, and Codex hook equivalents in .codex/hooks.json. The existing .claude/skills/ and .claude/agents/ remain the source of truth. Documentation (AGENTS.md, CLAUDE.md, README.md) is updated consistently.
New scripts scripts/verify_commit_command_hook.py (714 lines, Python hook for direct git commit commands) and scripts/test_commit_hook_config.py (432 lines, unittest suite) are well-structured and properly isolated from application code.
What was reviewed
- Symlinks (
.agents/skills/*): all resolve to.claude/skills/— verified per PR test plan - Codex agent TOML wrappers (
.codex/agents/*.toml): 11 wrappers, all follow identical pattern, each references the matching.claude/agents/<name>.md - MCP config (
.codex/config.toml): mirrors MCP servers from.mcp.json(playwright, playwright-report-mcp, coverage-matrix, quality-metrics, MCP_DOCKER) - Hooks (
.codex/hooks.json): maps Claude hooks from.claude/settings.jsonto Codex equivalents — commit command hook, Edit/Write type+format hook, worktree provisioning hook, Playwright CLI block hook - Python scripts:
verify_commit_command_hook.pyhas comprehensive shell-command analysis covering all edge cases in the test suite;test_commit_hook_config.pyuses standard unittest withsubTestfor parameterized cases - Documentation: all three files updated to document the Claude-source / Codex-reference model
No blocking issues found
- No Playwright test changes in this diff
- No TypeScript changes in this diff
- Commit hook command in
.claude/settings.jsonimproves over the previous form:printf '%s'(avoids trailing newline interpretation) + hash-verified Python helper - The
commit message conventionnote added toCLAUDE.mdcorrectly states "Run commits as a directgit commit ...command" and is consistent with the new hook design
Reviewed by proxy/@preset/minimax-minimax-m2-7-no-thinking
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 `@scripts/verify_commit_command_hook.py`:
- Around line 682-695: The hook is invoking the script without an explicit mode
so main() defaults mode to "codex" and prints the wrong BLOCK_MESSAGES entry;
update the hook invocation in .claude/settings.json to pass "claude" as the
positional argument so the script’s mode variable selects
BLOCK_MESSAGES["claude"] (which contains the /deep-review reference) when main()
runs; ensure the command that pipes INPUT into the script includes the mode
argument (i.e., call python3 "$SCRIPT" claude) so command_decision and the
subsequent block handling use the correct message.
🪄 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 Plus
Run ID: 326eed18-0f13-4755-aa28-b027406c1722
📒 Files selected for processing (6)
.claude/settings.json.codex/hooks.jsonCLAUDE.mdREADME.mdscripts/test_commit_hook_config.pyscripts/verify_commit_command_hook.py
🚧 Files skipped from review as they are similar to previous changes (1)
- README.md
There was a problem hiding this comment.
Review
PR #510 adds Codex integration as a reference-onlies wrapper around the existing Claude skills and agents. The substantive changes are:
New files:
.codex/agents/*.toml— 11 thin wrappers that read.claude/agents/<name>.mdas the authoritative prompt source. Correctly structured..codex/config.toml— MCP server declarations mirroring.mcp.json. Consistent..codex/hooks.json— Codex equivalents for the supported PreToolUse/PostToolUse command hooks from.claude/settings.json. Correct.scripts/verify_commit_command_hook.py— 714-line Python hook that parses and validatesgit commitcommand shapes, allowing only direct forms (git commit -m "...") and blocking compound/dynamic/indirect forms.scripts/test_commit_hook_config.py— 444-line test suite with 200+ test cases coveringcommand_decision, error paths, and end-to-end hook execution against both hook files.
Modified files:
CLAUDE.md— Adds note to rungit commitas a direct command (consistent with the hook design).AGENTS.md/README.md— Documents the reference-onlies model. Correctly informational..claude/settings.json— Hook command updated to useverify_commit_command_hook.py.
No Playwright, TypeScript, or page object changes — the review criteria for those areas are not applicable.
Minor note: shasum (GNU coreutils) is used for the pinned-hash verification. This is standard on Linux; on macOS it requires brew install coreutils. The hash-mismatch error message is clear and self-explanatory, so this is not a blocker.
No blocking issues found. Approving.
Reviewed by proxy/@preset/minimax-minimax-m2-7-no-thinking
There was a problem hiding this comment.
🧹 Nitpick comments (2)
.claude/settings.json (2)
188-188: ⚡ Quick winConsider documenting the hash update workflow.
The pinned hash
0c6d499e65f850b8066158c2d8ce548da0756c7f3c83acd5afb2ef453d6aaa95must be manually updated wheneverscripts/verify_commit_command_hook.pychanges. If the update process isn't documented (e.g., in a comment, script, or README), maintainers may forget this step.📝 Suggested documentation approach
Add a comment above line 188 or in
scripts/verify_commit_command_hook.py:# To update the hash after modifying verify_commit_command_hook.py: # shasum -a 256 scripts/verify_commit_command_hook.py | awk '{print $1}'Alternatively, add this to your test suite or CI to fail when the script changes without updating the hash.
🤖 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 @.claude/settings.json at line 188, Add documentation explaining how to update the pinned commit-hook hash: locate the EXPECTED variable (the hardcoded hash) in the command string and add a short comment either above that line in .claude/settings.json or inside scripts/verify_commit_command_hook.py showing the exact shell command to compute the new SHA256 (e.g., the shasum/sha256sum + awk invocation) and the reminder to deliberately update EXPECTED when modifying scripts/verify_commit_command_hook.py; optionally add a CI/test note that validates the hash is kept in sync.
188-188: ⚖️ Poor tradeoffHigh cognitive complexity in a single-line hook command.
The 550+ character bash one-liner combines multiple concerns: input extraction, pre-filtering, hash computation, validation, and script invocation. While functionally correct, this complexity makes auditing and future modifications more error-prone.
Consider extracting the hook logic to a dedicated shell script (e.g.,
scripts/run_commit_hook_wrapper.sh) and calling it from the hook. This would:
- Improve readability and maintainability
- Allow easier testing of the wrapper independently
- Simplify debugging with better error messages
However, this adds another file to maintain and moves security-critical logic outside the settings file. The current approach keeps the security boundary explicit and auditable in version control.
🤖 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 @.claude/settings.json at line 188, The hook command is a long single-line that mixes input parsing, safety filtering, hash verification and invocation of verify_commit_command_hook.py; extract that logic into a dedicated wrapper script (e.g., scripts/run_commit_hook_wrapper.sh) which performs the INPUT/ CMD extraction, the same pre-filter checks, computes and compares the EXPECTED hash for "scripts/verify_commit_command_hook.py" (keeping the current EXPECTED value), and then calls "python3 scripts/verify_commit_command_hook.py claude"; update the settings.json command string to simply pipe input to that wrapper (e.g., "INPUT=$(cat); printf '%s' \"$INPUT\" | scripts/run_commit_hook_wrapper.sh") so the security-critical hash check and script invocation remain identical but the hook is readable and testable.
🤖 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.
Nitpick comments:
In @.claude/settings.json:
- Line 188: Add documentation explaining how to update the pinned commit-hook
hash: locate the EXPECTED variable (the hardcoded hash) in the command string
and add a short comment either above that line in .claude/settings.json or
inside scripts/verify_commit_command_hook.py showing the exact shell command to
compute the new SHA256 (e.g., the shasum/sha256sum + awk invocation) and the
reminder to deliberately update EXPECTED when modifying
scripts/verify_commit_command_hook.py; optionally add a CI/test note that
validates the hash is kept in sync.
- Line 188: The hook command is a long single-line that mixes input parsing,
safety filtering, hash verification and invocation of
verify_commit_command_hook.py; extract that logic into a dedicated wrapper
script (e.g., scripts/run_commit_hook_wrapper.sh) which performs the INPUT/ CMD
extraction, the same pre-filter checks, computes and compares the EXPECTED hash
for "scripts/verify_commit_command_hook.py" (keeping the current EXPECTED
value), and then calls "python3 scripts/verify_commit_command_hook.py claude";
update the settings.json command string to simply pipe input to that wrapper
(e.g., "INPUT=$(cat); printf '%s' \"$INPUT\" |
scripts/run_commit_hook_wrapper.sh") so the security-critical hash check and
script invocation remain identical but the hook is readable and testable.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 0dbe8c7b-0bad-412f-a5e5-24f8bcd070ad
📒 Files selected for processing (2)
.claude/settings.jsonscripts/test_commit_hook_config.py
1 participant
Closes #510
Summary
Test plan
Summary by CodeRabbit