Skip to content

feat(dev-lead): Phase 0+1 — test infrastructure and intent stub - #178

Merged
don-petry merged 12 commits into
mainfrom
dev-lead-implementation
May 15, 2026
Merged

don-petry merged 12 commits into
mainfrom
dev-lead-implementation

Conversation

@don-petry

Copy link
Copy Markdown
Collaborator

Summary

  • Phase 0 — Test infrastructure: 26 event fixtures (all valid JSON with _test_expected_intent), stub claude/gemini engines, mock gh binary, sample CI failure log (239 lines), 4 bats helpers, 7 prompt templates with <!-- VARIABLES: --> declarations, pre-flight secret-check script, prompt-coverage integration test, and test-dev-lead.yml CI workflow
  • Phase 1 — Scaffold: dev-lead.yml workflow (all 7 trigger event types, dispatch + ci-relay jobs, SHA-pinned actions) and dev-lead-intent.sh stub (anti-loop guard live from day 1; all other events emit skip/not-implemented)
  • Tests: 14/14 bats unit tests pass locally; prompt coverage integration test passes for all 7 prompts

Test plan

  • Validate event fixtures: find tests/dev-lead/fixtures/events -name '*.json' -exec jq empty {} \;
  • Run prompt coverage test: bash tests/dev-lead/integration/test_prompt_coverage.sh
  • Run bats unit tests: bats tests/dev-lead/unit/ --formatter tap
  • Verify pre-flight fails without CLAUDE_CODE_OAUTH_TOKEN: unset CLAUDE_CODE_OAUTH_TOKEN; bash scripts/dev-lead-preflight.sh; echo $? (should be 1)
  • Check anti-loop guard: set BOT_USER=donpetry-bot and run with pr_sync_dev_lead_commit.json — should emit skip/dev-lead-own-commit
  • Confirm test-dev-lead.yml and dev-lead.yml workflows appear in Actions tab

🤖 Generated with Claude Code

don-petry and others added 3 commits May 11, 2026 20:50
- review-batch.sh: non-rate-limit per-PR failures (exit code 1) no
  longer abort the session. SESSION ABORTED EARLY is now reserved for
  the rate-limit-on-fallback-engine case (exit code 2) only. All other
  failures are counted and logged; remaining candidates continue.

- review-one-pr.sh: single-review step retries up to
  SINGLE_REVIEW_MAX_RETRIES (default 2) times with a
  SINGLE_REVIEW_RETRY_DELAY_SEC (default 15s) gap before giving up.
  On exhaustion, the PR is flagged needs-human-review and the script
  exits with code 1, which the updated batch treats as a non-fatal
  per-PR failure. Raw model output and stderr are logged on each failed
  attempt for post-mortem visibility.

Root cause of run #25707852006: claude-opus-4-7 returned a verbose
non-JSON response for PR #129; the old code treated that as fatal and
skipped 35 remaining candidates.

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
…w retry

Address inline review comments on PR #133:

- Rate-limit check: after each run_agentic call, inspect both stdout
  (VERDICT_JSON.raw) and stderr (SINGLE_LOG) with is_rate_limited before
  retrying. A rate-limit match exits immediately with code 2 so
  review-batch.sh can trigger engine fallback — consistent with triage and
  deep-review tiers. Previously a rate-limited single-review would burn all
  retries and exit 1 (per-PR failure), silently leaving the batch on the
  same rate-limited engine for all remaining PRs.

- Per-attempt log files: stderr is now written to
  single-review-attempt-N.log rather than a single overwritten file, so
  no earlier-attempt errors are lost. Each attempt logs its own stderr
  inline on failure; the fallback path cats all attempt logs for
  post-mortem visibility.

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Phase 0: full test harness for the dev-lead agent —
  26 event fixtures (all valid JSON with _test_expected_intent),
  stub claude/gemini engines, mock gh binary, CI failure log sample,
  bats helpers (stub-engine, mock-gh, assert-env, prompt-vars),
  7 prompt templates with VARIABLES declarations, preflight script,
  prompt coverage integration test, and test-dev-lead.yml CI workflow.

Phase 1: dev-lead.yml trigger workflow (all 7 event types, dispatch + ci-relay
  jobs) and dev-lead-intent.sh stub (anti-loop guard live; all other events
  emit skip/not-implemented).  14/14 bats unit tests pass.

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings May 15, 2026 03:35
@coderabbitai

coderabbitai Bot commented May 15, 2026 •

Copy link
Copy Markdown

Warning

Rate limit exceeded

@don-petry has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 55 minutes and 11 seconds before requesting another review.

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 @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 09b41db5-1956-46b7-a00d-80bbab0d5f8c

📥 Commits

Reviewing files that changed from the base of the PR and between c7c39e8 and 29296b2.

📒 Files selected for processing (61)
  • .github/workflows/dev-lead.yml
  • .github/workflows/test-dev-lead.yml
  • prompts/dev-lead/fix-bot-comment.md
  • prompts/dev-lead/fix-ci.md
  • prompts/dev-lead/fix-issue.md
  • prompts/dev-lead/fix-reviews.md
  • prompts/dev-lead/human-pr.md
  • prompts/dev-lead/human.md
  • prompts/dev-lead/rebase.md
  • scripts/dev-lead-fix-ci.sh
  • scripts/dev-lead-fix-issue.sh
  • scripts/dev-lead-fix-reviews.sh
  • scripts/dev-lead-intent.sh
  • scripts/dev-lead-preflight.sh
  • scripts/engine.sh
  • scripts/review-batch.sh
  • scripts/review-one-pr.sh
  • tests/dev-lead/fixtures/engines/stub-claude
  • tests/dev-lead/fixtures/engines/stub-gemini
  • tests/dev-lead/fixtures/events/check_run_dev_lead_self.json
  • tests/dev-lead/fixtures/events/check_run_failure.json
  • tests/dev-lead/fixtures/events/check_run_failure_fork.json
  • tests/dev-lead/fixtures/events/check_run_failure_no_pr.json
  • tests/dev-lead/fixtures/events/check_run_success.json
  • tests/dev-lead/fixtures/events/issue_comment_coderabbit.json
  • tests/dev-lead/fixtures/events/issue_comment_dev_lead_bot.json
  • tests/dev-lead/fixtures/events/issue_comment_human_no_trigger.json
  • tests/dev-lead/fixtures/events/issue_comment_human_trigger.json
  • tests/dev-lead/fixtures/events/issue_comment_rebase_sentinel.json
  • tests/dev-lead/fixtures/events/issue_comment_sonarqube.json
  • tests/dev-lead/fixtures/events/issues_labeled_claude.json
  • tests/dev-lead/fixtures/events/issues_labeled_dev_lead.json
  • tests/dev-lead/fixtures/events/issues_labeled_other.json
  • tests/dev-lead/fixtures/events/pr_opened_dependabot.json
  • tests/dev-lead/fixtures/events/pr_opened_fork.json
  • tests/dev-lead/fixtures/events/pr_opened_human.json
  • tests/dev-lead/fixtures/events/pr_review_comment_copilot.json
  • tests/dev-lead/fixtures/events/pr_review_comment_human_no_trigger.json
  • tests/dev-lead/fixtures/events/pr_review_comment_human_trigger.json
  • tests/dev-lead/fixtures/events/pr_review_copilot_approved.json
  • tests/dev-lead/fixtures/events/pr_review_copilot_commented.json
  • tests/dev-lead/fixtures/events/pr_review_gemini_changes.json
  • tests/dev-lead/fixtures/events/pr_review_human_owner.json
  • tests/dev-lead/fixtures/events/pr_sync_dev_lead_commit.json
  • tests/dev-lead/fixtures/events/repository_dispatch_ci_failure.json
  • tests/dev-lead/fixtures/logs/ci_failure_sample.txt
  • tests/dev-lead/fixtures/stubs/gh
  • tests/dev-lead/helpers/assert-env.bash
  • tests/dev-lead/helpers/mock-gh.bash
  • tests/dev-lead/helpers/prompt-vars.bash
  • tests/dev-lead/helpers/stub-engine.bash
  • tests/dev-lead/integration/test_prompt_coverage.sh
  • tests/dev-lead/unit/test_engine_fallback.bats
  • tests/dev-lead/unit/test_engine_writer.bats
  • tests/dev-lead/unit/test_fix_ci.bats
  • tests/dev-lead/unit/test_fix_issue.bats
  • tests/dev-lead/unit/test_fix_reviews.bats
  • tests/dev-lead/unit/test_intent_ci.bats
  • tests/dev-lead/unit/test_intent_issue.bats
  • tests/dev-lead/unit/test_intent_reviews.bats
  • tests/dev-lead/unit/test_intent_stub.bats
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch dev-lead-implementation

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Comment thread .github/workflows/test-dev-lead.yml Fixed
Comment thread .github/workflows/test-dev-lead.yml Fixed
Comment thread .github/workflows/test-dev-lead.yml Fixed

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request introduces the foundational components for the Dev-Lead Agent, including a suite of task-specific prompt templates (e.g., for fixing CI failures, addressing bot comments, and handling rebases) and supporting scripts. Key additions include dev-lead-intent.sh for event classification and dev-lead-preflight.sh for secret validation. The PR also improves existing review workflows by adding retry logic and rate-limit detection to review-one-pr.sh and making review-batch.sh more resilient to individual PR failures. Feedback identifies a misleading variable name in the intent script and suggests avoiding hardcoded paths in test stubs to ensure better portability across different environments.

Comment thread scripts/dev-lead-intent.sh Outdated
exit 0
fi
# Also skip if the commit message starts with fix(ci): or fix(dev-lead):
head_commit_msg=$(jq -r '.pull_request.head.sha // empty' "$EVENT_PATH" 2>/dev/null || true)

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 variable name head_commit_msg is misleading as it is being assigned the commit SHA (.pull_request.head.sha) rather than the commit message. Additionally, the variable is currently unused. If this was intended to be a placeholder for a commit message check, it should be corrected or removed, especially since the subsequent comment notes that the message is not available in the event payload.

Suggested change
head_commit_msg=$(jq -r '.pull_request.head.sha // empty' "$EVENT_PATH" 2>/dev/null || true)
head_sha=$(jq -r '.pull_request.head.sha // empty' "$EVENT_PATH" 2>/dev/null || true)

Comment thread tests/dev-lead/fixtures/stubs/gh Outdated
Comment on lines +29 to +30
if command -v /usr/bin/gh >/dev/null 2>&1; then
/usr/bin/gh "$@" 2>/dev/null || echo "{}"

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

Hardcoding the path to /usr/bin/gh makes the test stub less portable across different operating systems (e.g., macOS) or environments where gh is installed in a different location. It is better to avoid hardcoded absolute paths in test stubs to ensure they work consistently across developer machines and CI environments.

coderabbitai[bot]
coderabbitai Bot previously approved these changes May 15, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR lays down the Phase 0 test infrastructure and Phase 1 scaffolding for a new "dev-lead" agent: event fixtures, prompt templates, stub engine binaries, bats helpers, a pre-flight secret check, and the dev-lead.yml GitHub Actions workflow with a stub dev-lead-intent.sh classifier whose only fully-implemented logic is an anti-loop guard. The PR also makes unrelated tweaks to the existing PR-review pipeline (retry/rate-limit handling in review-one-pr.sh and softer failure handling in review-batch.sh).

Changes:

  • New scripts/dev-lead-intent.sh stub + scripts/dev-lead-preflight.sh, wired into a new .github/workflows/dev-lead.yml (dispatch + ci-relay jobs) and a test-dev-lead.yml CI workflow.
  • Comprehensive Phase 0 test scaffolding under tests/dev-lead/ (26 event fixtures, 4 bats helpers, mock gh, stub claude/gemini, prompt-coverage integration test, bats unit tests) plus 7 prompt templates under prompts/dev-lead/.
  • Unrelated changes in scripts/review-one-pr.sh (single-review retry with rate-limit fallback) and scripts/review-batch.sh (per-PR continue instead of session abort on generic failures).

Reviewed changes

Copilot reviewed 49 out of 49 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
.github/workflows/dev-lead.yml New agent workflow with dispatch + ci-relay jobs; contains a context-property bug for GITHUB_EVENT_PATH and a gh api --field JSON-payload issue.
.github/workflows/test-dev-lead.yml CI workflow that validates event fixtures, runs bats unit tests, and prompt-coverage check.
scripts/dev-lead-intent.sh Phase 1 stub classifier — implements anti-loop guard; everything else emits skip/not-implemented; contains dead head_commit_msg lines.
scripts/dev-lead-preflight.sh Pre-flight secret checker emitting GITHUB_STEP_SUMMARY rows.
scripts/review-one-pr.sh Adds retry loop + rate-limit detection around single-review; off-topic for this PR.
scripts/review-batch.sh Switches generic per-PR failures from session-fatal to continue; off-topic for this PR.
prompts/dev-lead/*.md 7 new prompt templates with <!-- VARIABLES: --> declarations.
tests/dev-lead/unit/test_intent_stub.bats Bats unit tests covering stub event routing, anti-loop, env/output writing, pre-flight.
tests/dev-lead/integration/test_prompt_coverage.sh Verifies all prompts exist and that every ${VAR} is declared.
tests/dev-lead/helpers/*.bash Bats helpers for stub engines, mock gh, env assertions, prompt-var coverage.
tests/dev-lead/fixtures/** 26 event JSON fixtures, mock gh binary, sample CI log, stub claude/gemini executables.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread .github/workflows/dev-lead.yml Outdated
Comment on lines +69 to +71
env:
GITHUB_EVENT_NAME: ${{ github.event_name }}
GITHUB_EVENT_PATH: ${{ github.event_path }}
Comment thread scripts/dev-lead-intent.sh Outdated
Comment on lines +76 to +80
# Also skip if the commit message starts with fix(ci): or fix(dev-lead):
head_commit_msg=$(jq -r '.pull_request.head.sha // empty' "$EVENT_PATH" 2>/dev/null || true)
# Note: full commit message check would require a git fetch; check via
# event context only (pusher commits are not embedded in the event payload).
# Future phases can add a git-based check after checkout.
Comment thread .github/workflows/dev-lead.yml Outdated
Comment on lines +208 to +222
gh api \
--method POST \
"repos/${{ github.repository }}/dispatches" \
--field event_type=dev-lead-ci-failure \
--field client_payload="{
\"pr_number\": ${{ steps.check-pr.outputs.pr_number }},
\"head_sha\": \"${{ github.event.check_run.head_sha }}\",
\"repo\": \"${{ github.repository }}\",
\"checks\": [{
\"name\": \"${{ github.event.check_run.name }}\",
\"conclusion\": \"${{ github.event.check_run.conclusion }}\",
\"details_url\": \"${{ github.event.check_run.details_url }}\",
\"app_slug\": \"${{ github.event.check_run.app.slug }}\"
}]
}"
Comment thread scripts/review-batch.sh
Comment on lines +106 to 109
# Per-PR failure — log and continue. Remaining candidates still run.
# Accumulated failures surface in the session summary.
failed=$((failed + 1))
echo "::error::Review failed for $pr_url (exit code $rc)"
Comment thread scripts/review-one-pr.sh
Comment on lines +319 to +324
SINGLE_REVIEW_MAX_RETRIES="${SINGLE_REVIEW_MAX_RETRIES:-2}"
SINGLE_REVIEW_RETRY_DELAY_SEC="${SINGLE_REVIEW_RETRY_DELAY_SEC:-15}"

single_attempt=1
single_ok=false
while [ "$single_attempt" -le "$SINGLE_REVIEW_MAX_RETRIES" ]; do

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ad922ed948

ℹ️ 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".

Comment thread .github/workflows/dev-lead.yml Outdated
--method POST \
"repos/${{ github.repository }}/dispatches" \
--field event_type=dev-lead-ci-failure \
--field client_payload="{

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Build repository_dispatch with a JSON payload

For the ci-relay path that actually reaches this step, this gh api --field client_payload="{...}" passes the whole object as a string, not as a nested JSON object; the gh api docs say --field only does scalar conversions and nested parameters require key[subkey]=value syntax or a pre-built body via --input. The repository_dispatch endpoint expects client_payload to be an object, so CI-failure relays will fail instead of triggering the dispatch job.

Useful? React with 👍 / 👎.

Comment thread .github/workflows/dev-lead.yml Outdated
exit 0
fi
# Require PR head repo matches this repo (not a fork)
head_repo=$(echo "$CHECK_RUN_PRS" | jq -r '.[0].head.repo.full_name')

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Read check-run PR repo data from fields GitHub sends

For real check_run payloads, the associated pull_requests[].head.repo object contains fields like id, url, and name, not full_name (fork pushes are already represented by an empty pull_requests array). This makes head_repo evaluate to null, so same-repository failed checks are treated as forks and should_relay is never set, disabling the CI-failure relay.

Useful? React with 👍 / 👎.

Comment thread scripts/review-batch.sh
Comment on lines 105 to 109
*)
# Other failure — session-fatal. A systemic problem (model degraded,
# prompt regression, malformed verdicts) shouldn't burn the queue.
# Per-PR failure — log and continue. Remaining candidates still run.
# Accumulated failures surface in the session summary.
failed=$((failed + 1))
echo "::error::Review failed for $pr_url (exit code $rc)"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Return failure when any PR review fails

When review-one-pr.sh exits non-zero for a non-rate-limit error, this branch now only increments failed and logs an annotation; because the script only exits 1 when session_aborted is set, the batch finishes with exit 0 even after malformed model output, gh errors, or the new single-review retry exhaustion. In the scheduled review workflow that makes failed review runs look green, so keep processing the remaining candidates but exit non-zero after the summary when failed > 0.

Useful? React with 👍 / 👎.

Comment on lines +14 to +15
check_run:
types: [completed]

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Use an event that fires for Actions CI failures

When the failed check is produced by GitHub Actions, this check_run trigger will not run at all: GitHub suppresses check_run workflow triggers for check suites created by GitHub Actions to prevent recursion. Since the new relay is intended to catch CI failures like the github-actions fixtures, those failures will never be dispatched to dev-lead-ci-failure; use an event such as workflow_run or poll PR check rollups from a trigger that actually fires.

Useful? React with 👍 / 👎.

Comment thread .github/workflows/dev-lead.yml Outdated
Comment on lines +64 to +65
- name: Pre-flight checks
run: bash scripts/dev-lead-preflight.sh

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Classify skip events before requiring secrets

For fork PRs and other events the intent script is expected to skip, this pre-flight runs first and requires CLAUDE_CODE_OAUTH_TOKEN; pull_request runs from forks do not receive repository secrets, so a harmless fork PR hits this step and fails the check instead of reaching the classifier's fork/skip path. Move the pre-flight after intent classification and guard it with env.INTENT_TYPE != 'skip' so skipped events stay green without secrets.

Useful? React with 👍 / 👎.

don-petry and others added 2 commits May 14, 2026 22:45
Phase 2: run_writer/run_writer_with_fallback in engine.sh, full intent
routing in dev-lead-intent.sh, dev-lead-fix-ci.sh handler, workflow wiring.

Phase 3: dev-lead-fix-reviews.sh handles fix-reviews, fix-bot-comment,
human, human-pr, rebase intents. Full review routing in intent classifier.

Phase 5: dev-lead-fix-issue.sh handles the issue intent with dedup guard
and branch/PR creation.

Phase 6: run_writer_with_fallback with claude→gemini→copilot fallback chain.

Tests: 77 unit tests across 8 new .bats files, all passing. Updated Phase 1
stub tests to reflect actual Phase 2+ routing behavior.

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4497ec7ccf

ℹ️ 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".

return 0
fi

run_writer_with_fallback "$prompt_file"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Persist review fixes back to the PR branch

For fix-reviews, fix-bot-comment, human, and human-pr intents, this path only invokes the writer and then exits; there is no gh pr checkout, commit/push wrapper, or post-run push in this script, while the prompts tell the engine not to push because the workflow will handle it. On review/comment events the initial checkout is not guaranteed to be the PR head branch, so any generated fixes either remain only on the runner or are committed to the wrong/detached checkout instead of updating the PR.

Useful? React with 👍 / 👎.

commenter=$(jq -r '.comment.user.login // empty' "$EVENT_PATH" 2>/dev/null || true)
comment_body=$(jq -r '.comment.body // empty' "$EVENT_PATH" 2>/dev/null || true)
pr_number=$(jq -r '.issue.number // empty' "$EVENT_PATH" 2>/dev/null || true)
author_assoc=$(jq -r '.issue.author_association // empty' "$EVENT_PATH" 2>/dev/null || true)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Authorize comment triggers using the commenter

For PR issue comments, this reads .issue.author_association, which describes the PR/issue author rather than the user who wrote the new comment. On a PR opened by an OWNER/MEMBER/COLLABORATOR, any outside commenter can include @dev-lead and satisfy this check, causing the write-capable human path to run under repository secrets; use the comment's association instead.

Useful? React with 👍 / 👎.

exit 0
fi

context=$(printf '{"pr_number":%s}' "${pr_number:-0}")

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Pass the triggering comment into the action

This context only preserves the PR number for issue_comment events, but the downstream fix-bot-comment and human prompts expect ACTOR plus COMMENT_BODY or USER_INSTRUCTION; the workflow never supplies those values and dev-lead-fix-reviews.sh defaults them to empty. As a result, a trusted bot comment or human @dev-lead request reaches the writer with a blank bot/comment instruction, so the agent cannot address the actual requested change.

Useful? React with 👍 / 👎.

review_state=$(jq -r '.review.state // empty' "$EVENT_PATH" 2>/dev/null || true)
pr_number=$(jq -r '.pull_request.number // empty' "$EVENT_PATH" 2>/dev/null || true)
head_sha=$(jq -r '.pull_request.head.sha // empty' "$EVENT_PATH" 2>/dev/null || true)
author_assoc=$(jq -r '.pull_request.author_association // empty' "$EVENT_PATH" 2>/dev/null || true)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Authorize PR reviews using the reviewer

This uses the PR author's association to decide whether a submitted review is trusted, not the association of the user who submitted the review. On an internal PR authored by an OWNER/MEMBER/COLLABORATOR, an outside user can submit a non-bot review and satisfy the human-pr branch, running the write-capable action with repository secrets; read the review/comment author's association instead.

Useful? React with 👍 / 👎.

Comment thread scripts/engine.sh

# Map rate-limit to exit code 2 for caller to detect
local output_check=""
if [ "$rc" -ne 0 ] && output_check=$(cat /tmp/dev-lead-writer-stderr 2>/dev/null) && is_rate_limited "$output_check"; then

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Capture writer stderr before checking rate limits

The fallback path only maps failures to exit 2 when /tmp/dev-lead-writer-stderr contains rate-limit text, but none of the writer invocations redirect stderr to that file. When Claude or Gemini hits a real quota/429 and exits nonzero, the message goes to the Actions log and this check sees an empty/missing file, so run_writer_with_fallback treats it as a hard failure instead of trying the next engine.

Useful? React with 👍 / 👎.

comment_body=$(jq -r '.comment.body // empty' "$EVENT_PATH" 2>/dev/null || true)
pr_number=$(jq -r '.pull_request.number // empty' "$EVENT_PATH" 2>/dev/null || true)
head_sha=$(jq -r '.pull_request.head.sha // empty' "$EVENT_PATH" 2>/dev/null || true)
author_assoc=$(jq -r '.pull_request.author_association // empty' "$EVENT_PATH" 2>/dev/null || true)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Authorize review comments using the commenter

This has the same trust inversion for PR review comments: .pull_request.author_association is the PR author's relationship, not the user who wrote the new review comment. On an internal PR, an outside commenter can add @dev-lead to an inline comment and pass this check, causing the write-capable human action to run with repository secrets; use the comment author's association instead.

Useful? React with 👍 / 👎.

exit 0
fi
context=$(printf '{"pr_number":%s,"head_sha":"%s"}' "${pr_number:-0}" "${head_sha:-}")
emit_intent "human-pr" "pr-synchronize" "$context"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Keep fork PR synchronizations in the skip path

The opened/reopened branch skips forks, but the synchronize branch does not repeat that check, so every update pushed to a fork PR by a non-bot author is classified as human-pr. In pull_request runs from forks, secrets and write tokens are withheld, so these harmless fork updates proceed to preflight/action setup and fail instead of staying green like fork opens.

Useful? React with 👍 / 👎.

contents: write
pull-requests: write
issues: write
checks: read

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Grant Actions read permission for CI log collection

The CI fixer collects failed logs with gh run view --log-failed, but this workflow sets an explicit permissions block without actions: read. GitHub's workflow syntax says unspecified scopes become none once any permission is specified, and the actions scope is what allows working with GitHub Actions, so runs that fall back to GITHUB_TOKEN cannot read workflow logs and the fixer receives only empty/no log context.

Useful? React with 👍 / 👎.

Comment thread scripts/engine.sh
local saved="$REVIEW_ENGINE"
export REVIEW_ENGINE="$engine"
local rc=0
run_writer "$prompt_file" "$model" || rc=$?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Recompute the model when switching fallback engines

model is captured once from the initially sourced engine configuration, then reused after REVIEW_ENGINE is changed in the fallback loop. If Claude is primary and rate-limited, the Gemini fallback is invoked with a Claude model id such as claude-sonnet-4-6 (and vice versa), so the fallback path fails even when the alternate engine is available.

Useful? React with 👍 / 👎.

fi

git add -A
git commit -m "fix(ci): auto-fix for $(echo "$CHECKS_JSON" | jq -r '.[0].name // "CI failure"') [skip ci-relay]"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Configure a git author before wrapper commits

This direct git commit runs in a fresh GitHub-hosted checkout, but the workflow never configures user.name or user.email before reaching it. When the writer leaves uncommitted changes for the wrapper, the commit step fails with the standard missing identity error, so CI fixes cannot be pushed unless the model happened to configure git itself.

Useful? React with 👍 / 👎.

@sonarqubecloud

Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
C Security Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 29296b22d2

ℹ️ 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".

Comment on lines +247 to +250
# Rebase sentinel check (highest priority, before bot-skip)
if echo "$comment_body" | grep -qF "<!-- auto-rebase-conflict:"; then
context=$(printf '{"pr_number":%s}' "${pr_number:-0}")
emit_intent "rebase" "rebase-conflict-sentinel" "$context"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Require trust before honoring rebase sentinels

For PR issue comments, this recognizes the rebase sentinel before any self/trusted-actor check, so any outside commenter who posts <!-- auto-rebase-conflict: ... --> on an internal PR is classified as rebase and reaches the write-capable handler with repository secrets. Gate this sentinel on the expected bot/trusted actor before emitting the intent.

Useful? React with 👍 / 👎.

Comment on lines +64 to +65
git add -A
git commit -m "feat: implement issue #${ISSUE_NUMBER} — ${ISSUE_TITLE}"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Configure a git author for issue implementation commits

When an issue is labeled dev-lead, this wrapper creates a branch and commits in a fresh Actions checkout, but the workflow never sets user.name/user.email. If the writer leaves uncommitted changes as expected, this git commit fails with the standard missing identity error, so the issue implementation never gets pushed or opened as a PR unless the model happened to configure git itself.

Useful? React with 👍 / 👎.

Comment on lines +26 to +27
6. Commit with a message referencing the issue: `feat: implement ${ISSUE_TITLE} (closes #${ISSUE_NUMBER})`
7. Open a pull request referencing the issue

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Leave issue commits and PR creation to the wrapper

The issue handler already creates the branch before invoking the writer and only pushes/opens the PR after it sees an uncommitted diff, but this prompt tells the writer to commit and open a pull request itself. If the engine follows those instructions, the wrapper reaches its clean-worktree path and exits as "No changes made" (or risks a duplicate PR), so issue-labeled runs do not reliably produce the single managed PR the script is designed to create.

Useful? React with 👍 / 👎.

Comment on lines +93 to +94
export PR_NUMBER="${PR_NUMBER:-}" PR_URL="https://github.com/${REPO}/pull/${PR_NUMBER:-}"
export REPO BASE_REF="${BASE_REF:-main}" HEAD_REF="${HEAD_REF:-}" CONFLICTING_FILES="${CONFLICTING_FILES:-}"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Populate the PR head ref before rebase pushes

For rebase sentinel runs, the intent context only supplies the PR number and this handler defaults HEAD_REF to an empty string before rendering the rebase prompt. The prompt's final push command is therefore git push --force-with-lease origin ${HEAD_REF} with no branch name, so the agent can complete the rebase locally but then fail to update the PR branch.

Useful? React with 👍 / 👎.

Comment on lines +36 to +39
- Prefer keeping PR changes over base branch changes when there is a semantic conflict
- Never silently drop code from either side — if both sides add code to the same location, merge them intelligently
- Do not squash or otherwise rewrite the PR commit history beyond rebasing
- If a conflict cannot be resolved safely, abort the rebase and comment on the PR explaining why

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Preserve the application-conflict abort rule

When the auto-rebase sentinel is caused by conflicts outside workflow files, this new prompt directs the writer to resolve semantic conflicts and continue the rebase. The existing rebase design in docs/dev-lead/spec.md says application-code conflicts should be aborted for human resolution, so this can force-push guessed conflict resolutions into PR branches instead of leaving those conflicts to the author.

Useful? React with 👍 / 👎.

Comment on lines +284 to +287
case "$label_name" in
dev-lead|claude)
context=$(printf '{"issue_number":%s}' "${issue_number:-0}")
emit_intent "issue" "issue-labeled-${label_name}" "$context"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Skip PR labels in the issue handler

Because pull requests also emit issues label events, adding the dev-lead or claude label to an existing PR reaches this branch and is treated as an issue implementation request. That sends the PR number to dev-lead-fix-issue.sh, which creates a new implementation branch/PR for the pull request instead of acting on the labeled PR or skipping it; check .issue.pull_request before emitting the issue intent.

Useful? React with 👍 / 👎.

Comment on lines +192 to +197
if is_trusted_bot "$reviewer"; then
# Bot review: only route non-APPROVED states
if [ "$review_state" = "APPROVED" ]; then
emit_skip "bot-approved"
else
emit_intent "fix-reviews" "bot-review-${review_state}" "$context"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Normalize review states before skipping approvals

For real pull_request_review webhook payloads in Actions, review.state is lowercase (for example approved), while this check only skips uppercase APPROVED. A trusted bot approval therefore falls into the fix-reviews path and runs the write-capable review fixer even though approvals are explicitly meant to be ignored.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants