Improve code review workflow and DRY up git tools#33
Improve code review workflow and DRY up git tools#33michael-airspace wants to merge 2 commits intomainfrom
Conversation
- Rewrite code_review_instructions.md with stricter findings rules, blocker-focused review mode, and MCP tool references (deepwiki, devin, context7) - Upgrade code_reviewer to gpt-5.4 with high reasoning in config.yaml - Extract shared _commit_push_create_pr helper in mcp_server.py to DRY up git_new_branch_pr, git_push_to_branch, and git_push_create_pr - Support git_new_branch_pr on feature branches (worktree support) - Add worktree MCP setup/fix scripts Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8e6568bf4a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if current_branch not in ['main', 'master']: | ||
| return f"❌ Error: Must be on main/master branch. Currently on: {current_branch}" | ||
| # Already on a feature branch (e.g., worktree) — skip branch creation, | ||
| # commit to current branch, push, and create PR | ||
| return await _commit_push_create_pr( |
There was a problem hiding this comment.
When execute_git_new_branch_pr is called from a non-main/master branch, it now ignores the branch_name argument and always commits/pushes current_branch; this silently changes behavior from "create branch X" to "use whatever branch is checked out." In a worktree or any feature branch where current_branch != branch_name, the tool can push to and open/update a PR for the wrong branch, which is a correctness issue users are unlikely to expect from this API contract.
Useful? React with 👍 / 👎.
| jq -e '.mcpServers["dev-tools"]' "$MAIN_MCP" > /dev/null 2>&1 || exit 0 | ||
|
|
||
| # Create .mcp.json with the worktree's base64-encoded path | ||
| ENCODED_PATH=$(echo -n "$WORKTREE_PATH" | base64) |
There was a problem hiding this comment.
This uses plain base64 without disabling wrapping, but base64 --help states --wrap=COLS ... (default 76), so longer worktree paths produce embedded newlines in ENCODED_PATH; those newlines are then inserted into NEW_URL, yielding an invalid MCP URL in .mcp.json for sufficiently deep directory paths (the same pattern also appears in scripts/fix_worktree_mcp.sh).
Useful? React with 👍 / 👎.
…ng config key - git_new_branch_pr now returns an explicit error when branch_name doesn't match the current feature branch, instead of silently ignoring it - Renamed reasoning → model_reasoning_effort in code_reviewer config
1 participant
Summary
prompts/code_review_instructions.mdwith stricter findings inclusion rules (confidence >= 8/10, concrete impact required), blocker-focused default mode, and references to MCP research tools (deepwiki for public repos, devin for private repos, context7, tavily)model_reasoning_effort: highinsrc/config.yaml_commit_push_create_prshared async helper insrc/mcp_server.py, eliminating duplicated add/commit/push/PR-create logic acrossexecute_git_new_branch_pr,execute_git_push_to_branch, andexecute_git_push_create_prgit_new_branch_prnow works when already on a feature branch (with explicit branch_name validation — errors if mismatched)scripts/setup_worktree_mcp.shandscripts/fix_worktree_mcp.shfor worktree MCP configurationReview fixes (Round 1)
git_new_branch_prsilently ignoringbranch_namewhen on a feature branch — now returns explicit error on mismatchreasoning: high→model_reasoning_effort: high(correct Codex CLI config key)