Skip to content

fix(dev-lead): add chatgpt-codex-connector[bot] to TRUSTED_BOTS + fix pre-existing test failures #291

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
don-petry merged 3 commits into from
May 19, 2026
Merged

fix(dev-lead): add chatgpt-codex-connector[bot] to TRUSTED_BOTS + fix pre-existing test failures #291

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e223d20
fix(reviews): address review comments [skip ci-relay]
don-petry May 19, 2026
4544bea
fix(reviews): address review comments [skip ci-relay]
don-petry May 19, 2026
229c300
fix(test): use case \$1 to guard copilot calls in gh stubs
don-petry May 19, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/dev-lead.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ concurrency:

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]' }}
TRIGGER_PHRASES: ${{ vars.TRIGGER_PHRASES || '@dev-lead' }}
DEV_LEAD_ENGINE: ${{ vars.DEV_LEAD_ENGINE || 'claude' }}
DEV_LEAD_DRY_RUN: ${{ vars.DEV_LEAD_DRY_RUN || 'false' }}
Expand Down
3 changes: 2 additions & 1 deletion tests/dev-lead/fixtures/events/issue_comment_human_trigger.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@
"user": {
"login": "donpetry",
"type": "User"
}
},
"author_association": "OWNER"
},
"repository": { "full_name": "petry-projects/.github-private" },
"sender": { "login": "donpetry", "type": "User" }
Expand Down
30 changes: 30 additions & 0 deletions tests/dev-lead/fixtures/events/pr_review_codex_commented.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
{
"_test_expected_intent": "fix-reviews",
"action": "submitted",
"review": {
"id": 3050,
"state": "COMMENTED",
"body": "Please address the naming inconsistencies noted inline.",
"user": {
"login": "chatgpt-codex-connector[bot]",
"type": "Bot"
}
},
"pull_request": {
"number": 284,
"title": "feat: implement issue #232",
"state": "open",
"author_association": "OWNER",
"head": {
"sha": "aaa111bbb222",
"ref": "feat/issue-232",
"repo": { "full_name": "petry-projects/.github-private" }
},
"base": {
"ref": "main",
"repo": { "full_name": "petry-projects/.github-private" }
}
},
"repository": { "full_name": "petry-projects/.github-private" },
"sender": { "login": "chatgpt-codex-connector[bot]", "type": "Bot" }
}
31 changes: 31 additions & 0 deletions tests/dev-lead/fixtures/events/pr_review_comment_codex.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
{
"_test_expected_intent": "fix-reviews",
"action": "created",
"comment": {
"id": 2050,
"body": "This logic has a potential off-by-one error on line 17.",
"path": "src/index.js",
"line": 17,
"user": {
"login": "chatgpt-codex-connector[bot]",
"type": "Bot"
}
},
"pull_request": {
"number": 284,
"title": "feat: implement issue #232",
"state": "open",
"author_association": "OWNER",
"head": {
"sha": "aaa111bbb222",
"ref": "feat/issue-232",
"repo": { "full_name": "petry-projects/.github-private" }
},
"base": {
"ref": "main",
"repo": { "full_name": "petry-projects/.github-private" }
}
},
"repository": { "full_name": "petry-projects/.github-private" },
"sender": { "login": "chatgpt-codex-connector[bot]", "type": "Bot" }
}
3 changes: 2 additions & 1 deletion tests/dev-lead/fixtures/events/pr_review_comment_human_trigger.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@
"user": {
"login": "donpetry",
"type": "User"
}
},
"author_association": "OWNER"
},
"pull_request": {
"number": 59,
Expand Down
17 changes: 10 additions & 7 deletions tests/dev-lead/unit/test_engine_fallback.bats
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,16 +92,19 @@ STUBEOF
}

@test "fallback: all rate-limited → returns 2" {
# All engines exit 2
# All engines exit 2 (rate-limited)
_make_stub "claude" 2
_make_stub "gemini" 2
# copilot falls back to claude internally, so we only need claude and gemini
# Create a copilot stub that also fails with 2
cat > "$STUB_BIN_DIR/copilot" <<'STUBEOF'
# copilot uses `gh copilot`; provide token + gh stub returning rate-limit text
export COPILOT_GITHUB_TOKEN="stub-token"
cat > "$STUB_BIN_DIR/gh" <<'GHEOF'
#!/usr/bin/env bash
exit 2
STUBEOF
chmod +x "$STUB_BIN_DIR/copilot"
case "$*" in
*"copilot"*) echo "rate limit exceeded"; exit 1 ;;
*) echo "{}" ;;
esac
GHEOF
chmod +x "$STUB_BIN_DIR/gh"
_source_engine "claude"

run run_writer_with_fallback "$TEST_PROMPT"
Expand Down
25 changes: 20 additions & 5 deletions tests/dev-lead/unit/test_engine_writer.bats
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,14 +91,21 @@ _source_engine() {
}

@test "writer: copilot falls back to claude in run_writer (internal)" {
# When REVIEW_ENGINE=copilot, run_writer internally falls back to claude
# When REVIEW_ENGINE=copilot, run_writer calls copilot_chat → gh copilot
_source_engine "copilot"
export STUB_ENGINE_EXIT=0
export DEV_LEAD_DRY_RUN=false
export COPILOT_GITHUB_TOKEN="stub-token"
cat > "$STUB_BIN_DIR/gh" <<'GHEOF'
#!/usr/bin/env bash
case "$*" in
*"copilot"*) exit 0 ;;
*) echo "{}" ;;
esac
GHEOF
chmod +x "$STUB_BIN_DIR/gh"

run run_writer "$TEST_PROMPT"

# Should succeed because the internal claude stub is present
[ "$status" -eq 0 ]
}

Expand Down Expand Up @@ -187,8 +194,16 @@ exit 1
STUB
chmod +x "$STUB_BIN_DIR/$engine"
done
# Add copilot stub (falls back to claude internally)
cp "$STUB_BIN_DIR/claude" "$STUB_BIN_DIR/copilot"
# copilot uses `gh copilot`; provide token + gh stub returning rate-limit text
export COPILOT_GITHUB_TOKEN="stub-token"
cat > "$STUB_BIN_DIR/gh" << 'GHEOF'
#!/usr/bin/env bash
case "$*" in
*"copilot"*) echo "rate limit exceeded"; exit 1 ;;
*) echo "{}" ;;
esac
GHEOF
chmod +x "$STUB_BIN_DIR/gh"

run run_writer_with_fallback "$TEST_PROMPT"

Expand Down
10 changes: 8 additions & 2 deletions tests/dev-lead/unit/test_fix_ci.bats
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -300,13 +300,15 @@ STUB
chmod +x "$STUB_BIN_DIR/$engine"
done

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
Comment on lines +303 to 314

@gemini-code-assist gemini-code-assist Bot May 19, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

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

Expand All @@ -322,6 +324,7 @@ GHEOF

@test "fix-ci: rate-limited: does not count toward exhaustion threshold" {
# PR already has rate-limited markers but zero status=failed markers
export COPILOT_GITHUB_TOKEN="stub-token"
cat > "$STUB_BIN_DIR/gh" << 'GHEOF'
#!/usr/bin/env bash
case "$*" in
Expand All @@ -332,13 +335,14 @@ case "$*" in
*"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
chmod +x "$STUB_BIN_DIR/gh"

# Engine also rate-limits on this run
for engine in claude gemini copilot; do
for engine in claude gemini; do
cat > "$STUB_BIN_DIR/$engine" << 'STUB'
#!/usr/bin/env bash
echo "rate limit exceeded"
Expand Down Expand Up @@ -406,6 +410,7 @@ GHEOF

@test "fix-ci: rate-limited dedup: existing rate-limited marker for same SHA skips duplicate post" {
# Simulate an existing rate-limited marker for HEAD_SHA (same sha)
export COPILOT_GITHUB_TOKEN="stub-token"
cat > "$STUB_BIN_DIR/gh" << 'GHEOF'
#!/usr/bin/env bash
case "$*" in
Expand All @@ -415,12 +420,13 @@ case "$*" in
*"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
chmod +x "$STUB_BIN_DIR/gh"

for engine in claude gemini copilot; do
for engine in claude gemini; do
cat > "$STUB_BIN_DIR/$engine" << 'STUB'
#!/usr/bin/env bash
echo "rate limit exceeded"
Expand Down
35 changes: 28 additions & 7 deletions tests/dev-lead/unit/test_fix_reviews.bats
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,9 +159,10 @@ teardown() {
export INTENT_TYPE="fix-reviews"
export DEV_LEAD_DRY_RUN="false"
export HEAD_SHA="ddd444eee555"
export COPILOT_GITHUB_TOKEN="stub-token"

# All engines rate-limited
for engine in claude gemini copilot; do
# claude and gemini engines rate-limited
for engine in claude gemini; do
cat > "$STUB_BIN_DIR/$engine" << 'STUB'
#!/usr/bin/env bash
echo "rate limit exceeded"
Expand All @@ -172,6 +173,10 @@ STUB

cat > "$STUB_BIN_DIR/gh" << 'GHEOF'
#!/usr/bin/env bash
# copilot must be checked first — its -p prompt text may contain "graphql"
case "$1" in
copilot) echo "rate limit exceeded"; exit 1 ;;
esac
ARGS="$*"
case "$ARGS" in
*"graphql"*)
Expand All @@ -198,9 +203,10 @@ GHEOF
export HEAD_SHA="ddd444eee555"
export ACTOR="donpetry"
export USER_INSTRUCTION="Please fix the failing tests"
export COPILOT_GITHUB_TOKEN="stub-token"

# All engines rate-limited
for engine in claude gemini copilot; do
# claude and gemini engines rate-limited
for engine in claude gemini; do
cat > "$STUB_BIN_DIR/$engine" << 'STUB'
#!/usr/bin/env bash
echo "hit your limit"
Expand All @@ -219,6 +225,8 @@ case "$ARGS" in
echo "COMMENT_POSTED: $ARGS"; exit 0 ;;
*"pulls/"*)
echo '{"head":{"sha":"ddd444eee555"}}' ;;
*"copilot"*)
echo "hit your limit"; exit 1 ;;
Comment on lines +228 to +229

@gemini-code-assist gemini-code-assist Bot May 19, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

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.

*) echo "{}" ;;
esac
GHEOF
Expand All @@ -245,7 +253,8 @@ GHEOF
comment_count_file=$(mktemp)
echo "0" > "$comment_count_file"

for engine in claude gemini copilot; do
export COPILOT_GITHUB_TOKEN="stub-token"
for engine in claude gemini; do
cat > "$STUB_BIN_DIR/$engine" << 'STUB'
#!/usr/bin/env bash
echo "quota exceeded"
Expand All @@ -256,6 +265,10 @@ STUB

cat > "$STUB_BIN_DIR/gh" << GHEOF
#!/usr/bin/env bash
# copilot must be checked first — its -p prompt text may contain "graphql"
case "\$1" in
copilot) echo "quota exceeded"; exit 1 ;;
esac
ARGS="\$*"
case "\$ARGS" in
*"graphql"*)
Expand Down Expand Up @@ -287,8 +300,9 @@ GHEOF
export DEV_LEAD_DRY_RUN="false"
export HEAD_SHA="ddd444eee555"
export COMMENT_BODY="SonarQube found issues"
export COPILOT_GITHUB_TOKEN="stub-token"

for engine in claude gemini copilot; do
for engine in claude gemini; do
cat > "$STUB_BIN_DIR/$engine" << 'STUB'
#!/usr/bin/env bash
echo "rate limit exceeded"
Expand All @@ -305,6 +319,8 @@ case "$ARGS" in
echo "[]" ;;
*"pr comment"*)
echo "COMMENT_POSTED"; exit 0 ;;
*"copilot"*)
echo "rate limit exceeded"; exit 1 ;;
Comment on lines +322 to +323

@gemini-code-assist gemini-code-assist Bot May 19, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

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.

*) echo "{}" ;;
esac
GHEOF
Expand All @@ -320,10 +336,15 @@ GHEOF
export INTENT_TYPE="fix-reviews"
export DEV_LEAD_DRY_RUN="false"
export HEAD_SHA="ddd444eee555"
export COPILOT_GITHUB_TOKEN="stub-token"

# Returns existing rate-limited marker for this sha+intent
cat > "$STUB_BIN_DIR/gh" << 'GHEOF'
#!/usr/bin/env bash
# copilot must be checked first — its -p prompt text may contain "graphql"
case "$1" in
copilot) echo "rate limit exceeded"; exit 1 ;;
esac
ARGS="$*"
case "$ARGS" in
*"graphql"*)
Expand All @@ -337,7 +358,7 @@ esac
GHEOF
chmod +x "$STUB_BIN_DIR/gh"

for engine in claude gemini copilot; do
for engine in claude gemini; do
cat > "$STUB_BIN_DIR/$engine" << 'STUB'
#!/usr/bin/env bash
echo "rate limit exceeded"
Expand Down
9 changes: 9 additions & 0 deletions tests/dev-lead/unit/test_intent_ci.bats
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,15 @@ teardown() {

_get_env() {
local key="$1"
# 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
Comment on lines +23 to +31

@gemini-code-assist gemini-code-assist Bot May 19, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

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.

grep "^${key}=" "$GITHUB_ENV" | cut -d= -f2- | head -1
}

Expand Down
9 changes: 9 additions & 0 deletions tests/dev-lead/unit/test_intent_issue.bats
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,15 @@ teardown() {

_get_env() {
local key="$1"
# 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
grep "^${key}=" "$GITHUB_ENV" | cut -d= -f2- | head -1
}

Expand Down
Loading