Skip to content

fix(review): collect GitHub Copilot reviewer threads - #3353

Merged
stranske merged 2 commits into
mainfrom
codex/bot-handler-copilot-reviewer
Sep 4, 2026
Merged

fix(review): collect GitHub Copilot reviewer threads#3353
stranske merged 2 commits into
mainfrom
codex/bot-handler-copilot-reviewer

Conversation

@stranske

@stranske stranske commented Sep 4, 2026

Copy link
Copy Markdown
Owner

Summary

  • recognize GitHub Copilot review threads under the live copilot-pull-request-reviewer login in both reusable inputs and shared handler defaults
  • keep Workflows source-template review threads in scope for the canonical manual handler
  • preserve consumer-template script parity and document/test the exact identities

Incident evidence

Handler run 33841911951 executed reusable-bot-comment-handler.yml from main at 484cac1 and reported no-unresolved-bot-comments for PR #3352. A complete exact-head GraphQL query after that scan still showed active threads PRRT_kwDOQprj9M6fLW7x and PRRT_kwDOQprj9M6fLW7_. The run log showed the allowlist omitted copilot-pull-request-reviewer and the Workflows caller ignored templates/.

This bounded successor is assigned to those exact review-debt threads. After it lands on main, rerun the handler once on #3352 and consume the terminal artifact plus a complete reviewThreads query.

Validation

  • node --test .github/scripts/tests/bot-comment-handler.test.js (23 passed)
  • python3 -m pytest -q tests/workflows/test_bot_comment_handler.py (17 passed)
  • python3 -m pytest -q tests/workflows/test_no_untrusted_interpolation.py tests/workflows/test_workflow_on_triggers_valid.py (175 passed)
  • python3 scripts/validate_template_completeness.py --strict
  • python3 scripts/validate_template_sync.py
  • /opt/homebrew/bin/actionlint on both changed workflow files
  • Ruff, Black, script parity, and git diff --check

Merge order

Closer-owned source repair. Keep ready for review with auto-merge disabled. Merge only at unchanged exact head after the seven-minute review window, passing required checks, complete zero active non-outdated review threads, and substantive review evidence. This repair must land before rerunning the #3352 handler; #3352 remains held.

Copilot AI lite review requested due to automatic review settings September 4, 2026 06:11
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 4, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-04T06:19:55.407180Z 6df141a Manual request
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@coderabbitai

coderabbitai Bot commented Sep 4, 2026

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 12 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available. Your 82 included PR review attempts over the past 7 days set your current allowance at 1 review per hour.

Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Essentials

Run ID: 5830bb6d-ac5e-4e6d-84e7-4fbaad42d964

📥 Commits

Reviewing files that changed from the base of the PR and between cfd28de and 6df141a.

📒 Files selected for processing (8)
  • .github/scripts/__tests__/bot-comment-handler.test.js
  • .github/scripts/bot-comment-handler.js
  • .github/workflows/agents-bot-comment-handler.yml
  • .github/workflows/reusable-bot-comment-handler.yml
  • docs/WORKFLOW_GUIDE.md
  • docs/bot-comment-handler.md
  • templates/consumer-repo/.github/scripts/bot-comment-handler.js
  • tests/workflows/test_bot_comment_handler.py

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

@stranske

stranske commented Sep 4, 2026

Copy link
Copy Markdown
Owner Author

MERGE HOLD — closer-owned source review-debt successor at exact head 6df141a. Do not merge or enable auto-merge until the head is unchanged, the seven-minute post-push review window has elapsed, required checks pass, the complete active non-outdated review-thread count is zero, and substantive review evidence is consumed. Ordered continuation: merge #3353 first; rerun the default-branch handler once on #3352 and verify its exact thread result; only then may #3352 pass its own merge gates and source-delta rollout.

@stranske
stranske deployed to agent-high-privilege September 4, 2026 06:11 — with GitHub Actions Active
@stranske

stranske commented Sep 4, 2026

Copy link
Copy Markdown
Owner Author

Final planned head is e2fa993. The incident run, exact omitted identity, ignored template path, and deterministic validation are recorded in the PR body.

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Keep it up!

Reviewed commit: e2fa99344d

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

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.

🟡 Changes recommended

The handler’s allowlists and related tests/docs should also account for the copilot-pull-request-reviewer[bot] variant (policy/fixtures already recognize both), otherwise some Copilot reviewer threads can still be missed by default.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR updates the bot comment handler surfaces so GitHub Copilot review threads authored by the copilot-pull-request-reviewer login are collected reliably, and so the canonical Workflows caller no longer ignores templates/ (keeping source-template review debt in scope). It also adds/updates documentation and tests to pin the expected identities and scope rules.

Changes:

  • Extend bot-author allowlists (workflow input defaults + shared handler defaults) to include copilot-pull-request-reviewer.
  • Update the canonical agents-bot-comment-handler.yml caller to stop ignoring templates/.
  • Document and test the updated identities/scope behavior.
File summaries
File Description
tests/workflows/test_bot_comment_handler.py Adds assertions covering the updated reusable defaults and canonical ignored-path behavior.
templates/consumer-repo/.github/scripts/bot-comment-handler.js Updates consumer-template handler defaults to recognize the Copilot reviewer login.
docs/WORKFLOW_GUIDE.md Notes Copilot reviewer identity support in the reusable handler description.
docs/bot-comment-handler.md Documents the Copilot pull-request reviewer identity as a default bot author.
.github/workflows/reusable-bot-comment-handler.yml Adds copilot-pull-request-reviewer to bot_authors defaults for both dispatch + call triggers.
.github/workflows/agents-bot-comment-handler.yml Removes templates/ from ignored_paths so template review threads remain in scope.
.github/scripts/bot-comment-handler.js Updates shared handler defaults to recognize copilot-pull-request-reviewer.
.github/scripts/__tests__/bot-comment-handler.test.js Extends unit coverage for the default allowlist recognizing canonical review bots.
Review details

Suppressed comments (3)

.github/scripts/bot-comment-handler.js:14

  • The allowlist only includes copilot-pull-request-reviewer, but this repo’s reviewer policy recognizes both copilot-pull-request-reviewer and copilot-pull-request-reviewer[bot] (see config/consumer_sync_review_policy.json). Since normalizeLogin() only lowercases, any threads authored by the [bot] variant would be missed by isBotAuthor() / collection.
const DEFAULT_BOT_AUTHORS = Object.freeze([
  'copilot[bot]',
  'copilot-pull-request-reviewer',
  'github-actions[bot]',
  'coderabbitai[bot]',
  'chatgpt-codex-connector',
  'chatgpt-codex-connector[bot]',
]);

templates/consumer-repo/.github/scripts/bot-comment-handler.js:14

  • Same allowlist gap as the Workflows copy: if GitHub emits copilot-pull-request-reviewer[bot] for some review threads, it won’t match because logins are only lowercased/trimmed. Including the [bot] variant here keeps consumer template behavior aligned with the repo’s reviewer policy.
const DEFAULT_BOT_AUTHORS = Object.freeze([
  'copilot[bot]',
  'copilot-pull-request-reviewer',
  'github-actions[bot]',
  'coderabbitai[bot]',
  'chatgpt-codex-connector',
  'chatgpt-codex-connector[bot]',
]);

.github/workflows/reusable-bot-comment-handler.yml:74

  • Same default allowlist issue for the workflow_call input: include copilot-pull-request-reviewer[bot] alongside copilot-pull-request-reviewer so callers that don’t override bot_authors still collect all Copilot reviewer threads.
        default: >-
          Copilot,copilot[bot],copilot-pull-request-reviewer,
          github-actions[bot],coderabbitai[bot],
          chatgpt-codex-connector,chatgpt-codex-connector[bot]
  • Files reviewed: 8/8 changed files
  • Comments generated: 5
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread .github/scripts/__tests__/bot-comment-handler.test.js
Comment thread .github/workflows/reusable-bot-comment-handler.yml
Comment thread docs/WORKFLOW_GUIDE.md Outdated
Comment thread docs/bot-comment-handler.md
Comment thread tests/workflows/test_bot_comment_handler.py
@stranske

stranske commented Sep 4, 2026

Copy link
Copy Markdown
Owner Author

Fallback-review disposition: Cursor returned approve with no P0/P1 blockers on e2fa993. Its valid P2 noted the fleet already recognizes the copilot-pull-request-reviewer[bot] identity; exact head 6df141a now adds that variant to both workflow defaults, both shared script copies, tests, and docs. The reported stacked-branch risk came from the reviewer's stale local main ref; live PR evidence shows base cfd28de and this two-commit branch only. Focused tests, template sync/completeness, actionlint, Ruff, Black, parity, and diff checks pass.

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Hooray!

Reviewed commit: 6df141a9e0

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

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.

🟢 Approval recommended

The changes are narrow, consistent across workflow defaults/scripts/templates, and are backed by updated Node/Python regressions that align with the stated incident.

Review details
  • Files reviewed: 8/8 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

@agents-workflows-bot

Copy link
Copy Markdown
Contributor

Workflow source needed

PR #3353 needs either a linked GitHub issue or one valid non-issue Workflow Source before PR metadata automation can manage it safely.

Please do one of:

  • Add <!-- meta:issue:123 --> or a normal Closes #123 / Related to #123 line.
  • Check one Workflow Source option in the PR body.
  • Add a hidden marker such as <!-- workflow-source:local_request -->, <!-- workflow-source:manual_remote -->, <!-- workflow-source:review_followup -->, <!-- workflow-source:sync_campaign -->, or <!-- workflow-source:dependabot -->.
  • Add a workflow source label such as workflow:source-direct-pr, workflow:source-local-request, workflow:source-review-followup, workflow:source-sync, or workflow:no-automation.

Once a valid source is present, this warning will not be reposted.

@agents-workflows-bot

agents-workflows-bot Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Automated Status Summary

Head SHA: 0c8bf2d
Latest Runs: ⏳ pending — Gate
Required contexts: summary
Required: core tests (3.12): ⏳ pending, core tests (3.13): ⏳ pending, docker smoke: ⏳ pending, gate: ⏳ pending

Workflow / Job Result Logs
(no jobs reported) ⏳ pending

Coverage Overview

  • Coverage history entries: 1

Coverage Trend

Metric Value
Current 79.29%
Baseline 85.00%
Delta -5.71%
Minimum 70.00%
Status ✅ Pass

Top Coverage Hotspots (lowest coverage)

File Coverage Missing
scripts/issue_dedup_smoke.py 0.0% 4
scripts/runner_lib/__main__.py 0.0% 3
scripts/repo_review_round2_runner.py 25.1% 344
scripts/repo_review_round1_runner.py 39.0% 133
scripts/prune_agent_stubs.py 39.7% 26
tools/ensure_workflow_timeout_variables.py 42.1% 74
scripts/sync_label_docs.py 42.9% 64
tools/discover_model_catalog.py 44.8% 55
scripts/repo_review_backlog_scan.py 45.3% 116
scripts/repo_review_body_writer.py 45.8% 86
tools/codex_session_analyzer.py 47.9% 59
scripts/create_verifier_labels.py 48.3% 58
tools/ci_failure_triage.py 49.7% 113
scripts/validate_template_sync.py 52.1% 36
scripts/select_consumer_sync_phase.py 53.0% 62

Low Coverage Files (<50.0%)

File Coverage Missing
scripts/issue_dedup_smoke.py 0.0% 4
scripts/runner_lib/__main__.py 0.0% 3
scripts/repo_review_round2_runner.py 25.1% 344
scripts/repo_review_round1_runner.py 39.0% 133
scripts/prune_agent_stubs.py 39.7% 26
tools/ensure_workflow_timeout_variables.py 42.1% 74
scripts/sync_label_docs.py 42.9% 64
tools/discover_model_catalog.py 44.8% 55
scripts/repo_review_backlog_scan.py 45.3% 116
scripts/repo_review_body_writer.py 45.8% 86
tools/codex_session_analyzer.py 47.9% 59
scripts/create_verifier_labels.py 48.3% 58
tools/ci_failure_triage.py 49.7% 113

Updated automatically; will refresh on subsequent CI/Docker completions.


Keepalive checklist

Scope

No scope information available

Tasks

  • No tasks defined

Acceptance criteria

  • No acceptance criteria defined

@stranske
stranske deployed to agent-high-privilege September 4, 2026 06:26 — with GitHub Actions Active
@stranske
stranske merged commit 0c8bf2d into main Sep 4, 2026
61 checks passed
@stranske
stranske deleted the codex/bot-handler-copilot-reviewer branch September 4, 2026 06:48
@stranske stranske added the verify:compare Compare multiple LLM evaluations label Sep 4, 2026
@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Provider Comparison Report

Provider Summary

Provider Model Verdict Confidence Summary
openai gpt-5.6-terra PASS 94% The change consistently recognizes both copilot-pull-request-reviewer and its [bot] variant in the shared handler defaults and both reusable-workflow input defaults. The consumer template handl...
anthropic claude-sonnet-5 PASS 78% The PR correctly adds the 'copilot-pull-request-reviewer' and its '[bot]' variant to the DEFAULT_BOT_AUTHORS list in the shared handler script (and its template mirror for parity), updates both ins...
📋 Full Provider Details (click to expand)

openai

  • Model: gpt-5.6-terra
  • Verdict: PASS
  • Confidence: 94%
  • Scores:
    • Correctness: 10.0/10
    • Completeness: 10.0/10
    • Quality: 9.0/10
    • Testing: 9.0/10
    • Risks: 9.0/10
  • Summary: The change consistently recognizes both copilot-pull-request-reviewer and its [bot] variant in the shared handler defaults and both reusable-workflow input defaults. The consumer template handler remains synchronized with the source handler. Removing templates/ from the canonical manual handler's ignored paths keeps Workflows source-template changes in scope. Documentation and test coverage were updated for the exact identities, including JavaScript allowlist coverage and workflow-level assertions. The changes are small, readable, backward-compatible, and introduce no meaningful security or performance risk.

anthropic

  • Model: claude-sonnet-5
  • Verdict: PASS
  • Confidence: 78%
  • Scores:
    • Correctness: 8.0/10
    • Completeness: 8.0/10
    • Quality: 8.0/10
    • Testing: 7.0/10
    • Risks: 7.0/10
  • Summary: The PR correctly adds the 'copilot-pull-request-reviewer' and its '[bot]' variant to the DEFAULT_BOT_AUTHORS list in the shared handler script (and its template mirror for parity), updates both instances of the reusable workflow's default author list inputs, and extends the JS test suite to cover the new identities. Docs are updated accordingly. An additional change removes 'templates/' from ignored_paths in agents-bot-comment-handler.yml, which is plausible given the parity requirement but not explicitly called out in the task list — a minor scope/documentation gap. CI signals available (selftest-ci.yml success, pr-11-ci-smoke.yml success) support the implementation working correctly, though the in-progress gate workflow and limited diff context for the Python test additions leave some testing depth unconfirmed. Overall the implementation is correct, targeted, and low-risk, satisfying the stated acceptance criteria closely enough to warrant a PASS with moderate confidence.
  • Concerns:
    • The removal of 'templates/' from ignored_paths in agents-bot-comment-handler.yml is not explicitly listed in the task bullets but is a reasonable side-effect of preserving consumer-template script parity; its broader impact (template-only changes now trigger the bot comment handler) isn't discussed in the PR description.
    • Python test file only gained +10 lines; without seeing the actual diff content it's hard to confirm it fully exercises both new identity strings and their expected classification (allowlisted vs bot-detection paths).
    • Exact pytest/node test counts stated in acceptance criteria (23/17/175 passed) cannot be independently verified from the diff alone; reliance is placed on CI summaries (selftest-ci.yml, pr-11-ci-smoke.yml) which report success but don't show raw counts.
    • pr-00-gate.yml was still in_progress at time of review with several jobs pending; while none failed, full completion wasn't observed before merge evaluation.

Agreement

  • Verdict: PASS (all providers)
  • Quality: scores within 1 point (avg 8.5/10, range 8.0-9.0)

Disagreement

Dimension openai anthropic
Correctness 10.0/10 8.0/10
Completeness 10.0/10 8.0/10
Testing 9.0/10 7.0/10
Risks 9.0/10 7.0/10

Unique Insights

  • openai: The change consistently recognizes both copilot-pull-request-reviewer and its [bot] variant in the shared handler defaults and both reusable-workflow input defaults. The consumer template handler remains synchronized with the source handler. Removing templates/ from the canonical manual han...
  • anthropic: The removal of 'templates/' from ignored_paths in agents-bot-comment-handler.yml is not explicitly listed in the task bullets but is a reasonable side-effect of preserving consumer-template script parity; its broader impact (template-only changes now trigger the bot comment handler) isn't discussed in the PR description.; Python test file only gained +10 lines; without seeing the actual diff content it's hard to confirm it fully exercises both new identity strings and their expected classification (allowlisted vs bot-detection paths).; Exact pytest/node test counts stated in acceptance criteria (23/17/175 passed) cannot be independently verified from the diff alone; reliance is placed on CI summaries (selftest-ci.yml, pr-11-ci-smoke.yml) which report success but don't show raw counts.; pr-00-gate.yml was still in_progress at time of review with several jobs pending; while none failed, full completion wasn't observed before merge evaluation.

🔍 LangSmith Traces

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

Labels

verify:compare Compare multiple LLM evaluations

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants