fix(dev-lead): add chatgpt-codex-connector[bot] to TRUSTED_BOTS + fix pre-existing test failures - #291
Conversation
When copilot_chat passes the full prompt via -p, the prompt text itself may contain "graphql" causing gh stubs with *"graphql"* patterns to match copilot invocations instead of actual gh api graphql calls. Check \$1 (the subcommand) first so copilot calls are handled before pattern matching on the full argument string. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ 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: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (12)
✨ 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 |
|
![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 updates the test suite to simulate GitHub Copilot interactions via the gh CLI, adds support for the chatgpt-codex-connector[bot], and improves environment variable parsing in tests. Review feedback highlighted the need for more robust argument checking in CLI stubs to avoid false positives caused by prompt text and suggested centralizing the _get_env helper to reduce code duplication.
| export COPILOT_GITHUB_TOKEN="stub-token" | ||
| cat > "$STUB_BIN_DIR/gh" << 'GHEOF' | ||
| #!/usr/bin/env bash | ||
| case "$*" in | ||
| *"issues/42/comments"*) echo "[]" ;; | ||
| *"pr checkout"*) exit 0 ;; | ||
| *"pr comment"*) echo "COMMENT_POSTED: $*"; exit 0 ;; | ||
| *"run view"*) echo "log output" ;; | ||
| *"copilot"*) echo "rate limit exceeded"; exit 1 ;; | ||
| *) echo "{}" ;; | ||
| esac | ||
| GHEOF |
There was a problem hiding this comment.
The gh stub uses pattern matching on the full argument string ($*) to detect copilot calls. As correctly identified in other parts of this PR (e.g., test_fix_reviews.bats), the prompt text passed via the -p flag can contain arbitrary strings like issues/42/comments, which would cause a false positive match in the preceding cases. It is safer to check the first argument ($1) specifically for the copilot command before falling back to pattern matching on the full argument string.
export COPILOT_GITHUB_TOKEN="stub-token"
cat > "$STUB_BIN_DIR/gh" << 'GHEOF'
#!/usr/bin/env bash
case "$1" in
copilot) echo "rate limit exceeded"; exit 1 ;;
esac
case "$*" in
*"issues/42/comments"*) echo "[]" ;;
*"pr checkout"*) exit 0 ;;
*"pr comment"*) echo "COMMENT_POSTED: $*"; exit 0 ;;
*"run view"*) echo "log output" ;;
*) echo "{}" ;;
esac
GHEOF
| *"copilot"*) | ||
| echo "hit your limit"; exit 1 ;; |
There was a problem hiding this comment.
This stub implementation is inconsistent with the more robust pattern introduced elsewhere in this file (e.g., at line 177). Matching *"copilot"* against the full argument string is risky because the prompt text might contain patterns that match other cases (like pulls/ at line 226). Please use the $1 check at the top of the stub to reliably identify the command.
| *"copilot"*) | ||
| echo "rate limit exceeded"; exit 1 ;; |
There was a problem hiding this comment.
Similar to the issue at line 228, this stub should use a $1 check for copilot at the top of the script to avoid false positives from the prompt text matching other patterns in the case block.
| # Handle multiline heredoc format: KEY<<DELIM\nvalue\nDELIM | ||
| local delim_line delimiter | ||
| delim_line=$(grep "^${key}<<" "$GITHUB_ENV" 2>/dev/null | head -1) | ||
| if [ -n "$delim_line" ]; then | ||
| delimiter="${delim_line#*<<}" | ||
| awk -v start="${key}<<${delimiter}" -v end="${delimiter}" \ | ||
| 'found && $0==end{exit} found{print} $0==start{found=1}' "$GITHUB_ENV" | ||
| return | ||
| fi |
There was a problem hiding this comment.
The _get_env helper logic for handling multiline heredocs is duplicated across test_intent_ci.bats and test_intent_issue.bats, but remains un-updated in test_intent_reviews.bats. While functional, this duplication makes maintenance harder. Consider centralizing this helper if the test framework allows, or at least ensuring consistency across all intent test files to avoid confusing behavior when inspecting INTENT_CONTEXT.
There was a problem hiding this comment.
Pull request overview
Updates dev-lead’s trust configuration and test suite so review/review-comment events from chatgpt-codex-connector[bot] route to fix-reviews, while also repairing several previously failing unit tests/fixtures and Copilot rate-limit stubbing.
Changes:
- Add
chatgpt-codex-connector[bot]to the defaultTRUSTED_BOTSlist (and add unit tests/fixtures to cover codex review + review-comment events). - Fix unit test fixture data to include required
author_associationfields for human-trigger scenarios. - Update bats helpers/stubs to correctly parse multiline
GITHUB_ENVheredocs and to stubgh copilot(withCOPILOT_GITHUB_TOKEN) in rate-limit scenarios.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
.github/workflows/dev-lead.yml |
Adds codex bot to default TRUSTED_BOTS for the main dev-lead workflow. |
tests/dev-lead/unit/test_intent_reviews.bats |
Extends trusted bot list for unit tests and adds new codex routing test cases. |
tests/dev-lead/unit/test_intent_issue.bats |
Fixes _get_env helper to parse multiline heredoc entries from GITHUB_ENV. |
tests/dev-lead/unit/test_intent_ci.bats |
Same _get_env heredoc parsing fix for CI intent tests. |
tests/dev-lead/unit/test_fix_reviews.bats |
Adjusts rate-limit tests to stub gh copilot correctly and sets COPILOT_GITHUB_TOKEN. |
tests/dev-lead/unit/test_fix_ci.bats |
Updates GH stubs to simulate Copilot rate-limit behavior via gh copilot. |
tests/dev-lead/unit/test_engine_writer.bats |
Aligns Copilot writer tests with the gh copilot invocation path and token requirement. |
tests/dev-lead/unit/test_engine_fallback.bats |
Aligns Copilot fallback test with gh copilot and rate-limit text detection. |
tests/dev-lead/fixtures/events/pr_review_comment_human_trigger.json |
Adds missing comment.author_association for human-trigger routing correctness. |
tests/dev-lead/fixtures/events/issue_comment_human_trigger.json |
Adds missing comment.author_association for human-trigger routing correctness. |
tests/dev-lead/fixtures/events/pr_review_comment_codex.json |
New fixture for codex PR review-comment event. |
tests/dev-lead/fixtures/events/pr_review_codex_commented.json |
New fixture for codex PR review (COMMENTED) event. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| env: | ||
| BOT_USER: ${{ vars.BOT_USER || 'donpetry-bot' }} | ||
| TRUSTED_BOTS: ${{ vars.TRUSTED_BOTS || 'copilot-pull-request-reviewer[bot],gemini-code-assist[bot],sonarqubecloud[bot],coderabbitai[bot]' }} | ||
| TRUSTED_BOTS: ${{ vars.TRUSTED_BOTS || 'copilot-pull-request-reviewer[bot],gemini-code-assist[bot],sonarqubecloud[bot],coderabbitai[bot],chatgpt-codex-connector[bot]' }} |
… test suite (#291) Fixes #290 - Adds chatgpt-codex-connector[bot] to the TRUSTED_BOTS default so codex review comments trigger fix-reviews intent - Adds test fixtures and cases for codex bot events - Fixes 18 pre-existing test failures: human-trigger fixture missing author_association, _get_env heredoc parsing, copilot rate-limit stubs using wrong binary
… test suite (#291) Fixes #290 - Adds chatgpt-codex-connector[bot] to the TRUSTED_BOTS default so codex review comments trigger fix-reviews intent - Adds test fixtures and cases for codex bot events - Fixes 18 pre-existing test failures: human-trigger fixture missing author_association, _get_env heredoc parsing, copilot rate-limit stubs using wrong binary
… test suite (#291) Fixes #290 - Adds chatgpt-codex-connector[bot] to the TRUSTED_BOTS default so codex review comments trigger fix-reviews intent - Adds test fixtures and cases for codex bot events - Fixes 18 pre-existing test failures: human-trigger fixture missing author_association, _get_env heredoc parsing, copilot rate-limit stubs using wrong binary
… test suite (#291) Fixes #290 - Adds chatgpt-codex-connector[bot] to the TRUSTED_BOTS default so codex review comments trigger fix-reviews intent - Adds test fixtures and cases for codex bot events - Fixes 18 pre-existing test failures: human-trigger fixture missing author_association, _get_env heredoc parsing, copilot rate-limit stubs using wrong binary
… test suite (#291) Fixes #290 - Adds chatgpt-codex-connector[bot] to the TRUSTED_BOTS default so codex review comments trigger fix-reviews intent - Adds test fixtures and cases for codex bot events - Fixes 18 pre-existing test failures: human-trigger fixture missing author_association, _get_env heredoc parsing, copilot rate-limit stubs using wrong binary
… test suite (#291) Fixes #290 - Adds chatgpt-codex-connector[bot] to the TRUSTED_BOTS default so codex review comments trigger fix-reviews intent - Adds test fixtures and cases for codex bot events - Fixes 18 pre-existing test failures: human-trigger fixture missing author_association, _get_env heredoc parsing, copilot rate-limit stubs using wrong binary
… test suite (#291) Fixes #290 - Adds chatgpt-codex-connector[bot] to the TRUSTED_BOTS default so codex review comments trigger fix-reviews intent - Adds test fixtures and cases for codex bot events - Fixes 18 pre-existing test failures: human-trigger fixture missing author_association, _get_env heredoc parsing, copilot rate-limit stubs using wrong binary
… test suite (#291) Fixes #290 - Adds chatgpt-codex-connector[bot] to the TRUSTED_BOTS default so codex review comments trigger fix-reviews intent - Adds test fixtures and cases for codex bot events - Fixes 18 pre-existing test failures: human-trigger fixture missing author_association, _get_env heredoc parsing, copilot rate-limit stubs using wrong binary
Fixes #290
Summary
chatgpt-codex-connector[bot]was not in theTRUSTED_BOTSdefault list, sodev-lead-intent.shwas classifying itspull_request_review_commentevents asskip/no-trigger-or-untrustedinstead offix-reviewschatgpt-codex-connector[bot]to theTRUSTED_BOTSdefault indev-lead.ymlauthor_associationon comment objects causing human-trigger tests to fail)_get_envhelper bug intest_intent_ci.batsandtest_intent_issue.bats—INTENT_CONTEXTis written in GitHub Actions multiline heredoc format (KEY<<DELIM) but the helper only parsedKEY=valueformatcopilotstubs but the engine usesgh copilot; updated to setCOPILOT_GITHUB_TOKENand stubgh copilotcalls correctly; usecase "$1"to guard copilot dispatch before pattern-matching on full args (prompt text may contain "graphql")All 116 unit tests now pass (was 18 failing before this PR).
Test plan
pull_request_review codex COMMENTED → fix-reviewsandpull_request_review_comment codex → fix-reviewschatgpt-codex-connector[bot](confirmed from PR feat: implement issue #232 — Phase 1: Issue Triage gh-aw workflow #284 review API)🤖 Generated with Claude Code