Skip to content

feat: review queue as readiness gate with Slack notifications#1240

Merged
Gkrumbach07 merged 7 commits intomainfrom
feat/review-queue-rework
Apr 7, 2026
Merged

feat: review queue as readiness gate with Slack notifications#1240
Gkrumbach07 merged 7 commits intomainfrom
feat/review-queue-rework

Conversation

@Gkrumbach07
Copy link
Copy Markdown
Contributor

@Gkrumbach07 Gkrumbach07 commented Apr 7, 2026

Summary

Rewrite the review queue from an LLM-powered session to a deterministic shell script. Now acts as a strict readiness gate — only PRs that are actually mergeable get added to the milestone and notified in Slack.

Changes

Review Queue (pr-merge-review.yml)

  • Hourly (was daily at 1pm)
  • Shell-driven (was LLM session — cheaper, faster, deterministic)
  • Strict readiness check: CI must pass (not pending), no changes requested, not draft
  • Milestone management: adds ready PRs, removes ones that fell out of readiness
  • Slack notification only for NEW additions — not every run, not every PR
  • Milestone description auto-updated with ready/not-ready lists

Amber Handler

  • Removed "PR created" Slack notifications — these were noisy
  • The review queue now handles the notification when PRs are actually ready

Slack message format

📋 2 PRs ready for review

• #1234 — Fix session timeout handling
• #1235 — Add model selector to header

Total in Review Queue: 5

Test plan

  • Manual dispatch review queue — verify milestone updated
  • Open a PR with CI passing — verify it gets added and Slack notified
  • PR with failing CI — verify NOT added to milestone
  • Draft PR — verify skipped

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Chores

    • Removed Slack prompts from PR-creation session flows
    • Review-queue schedule changed to run hourly on weekdays
    • Added concurrency controls to review-queue runs
  • New Features

    • Review queue now evaluates PR readiness (drafts, CI, reviews) and updates a milestone
    • Slack notifications for PRs newly marked as ready
    • Batch PR fixer now skips healthy PRs by checking CI, mergeability, and review states
    • PR comments and bodies now include a session footer/link when relevant

Review queue rewritten as shell-driven hourly job:
- Evaluates all open PRs for merge readiness (CI passing, no
  changes requested, not draft)
- Adds ready PRs to "Review Queue" milestone
- Removes PRs that are no longer ready
- Slack notification ONLY when new PRs are added to milestone
- Updates milestone description with ready/not-ready lists

Amber handler:
- Remove "PR created" Slack notifications (noisy)
- Review queue now handles notifications when PRs are actually ready

Flow:
1. Auto-fix creates PR → no Slack notification
2. Fixer sessions resolve CI/reviews → no notification
3. PR becomes ready (CI green, no open reviews) → review queue
   adds to milestone → Slack: "2 PRs ready for review"

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Apr 7, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Removed Slack “PR created” instructions and added session-footer insertion in PR comments; replaced updatedAt-based skipping with a per-PR needs_attention(pr_number) check in the batch fixer; replaced Ambient session with an hourly weekday inline Python review-queue job that manages the "Review Queue" milestone and posts Slack notifications.

Changes

Cohort / File(s) Summary
Issue handler & batch fixer
​.github/workflows/amber-issue-handler.yml
Removed Slack “PR created” instructions from PR-creation prompts; added instruction to append a session footer/link to PR body when commenting; removed updatedAt-based skip and gh pr list updatedAt usage; added needs_attention(pr_number) which queries CI check states, mergeable == CONFLICTING, and non-bot CHANGES_REQUESTED counts; batch loop now prints health reason or triggers fixes and logs reason.
PR merge review workflow
​.github/workflows/pr-merge-review.yml
Changed schedule to hourly weekdays (cron: '0 * * * 1-5'). Replaced Ambient session/action with an inline Python review-queue job that ensures/creates the "Review Queue" milestone, evaluates open non-draft PRs (draft, CI checks, reviews), updates milestone membership, posts Slack notifications for newly-ready PRs, logs Slack failures, adds concurrency controls, expands permissions (issues: write, pull-requests: write), and simplifies step outputs.

Sequence Diagram(s)

sequenceDiagram
  participant Scheduler as GitHub Actions Scheduler
  participant Runner as Workflow Runner
  participant PythonJob as review-queue (Python)
  participant GH as GitHub API
  participant Slack as Slack Webhook
  Note over Scheduler,Runner: hourly on weekdays
  Scheduler->>Runner: trigger pr-merge-review workflow
  Runner->>PythonJob: run review-queue job
  PythonJob->>GH: list open non-draft PRs
  loop per PR
    PythonJob->>GH: fetch PR details (draft, mergeable, reviews, checks)
    GH-->>PythonJob: PR state, checks, reviews
    alt PR qualifies as ready
      PythonJob->>GH: add PR to "Review Queue" milestone
      PythonJob->>Slack: post newly-ready PR notification
      Slack-->>PythonJob: ack/fail
    else PR not ready
      PythonJob->>GH: remove PR from milestone (if present)
    end
  end
  PythonJob->>Runner: print summary and update milestone description
Loading

Important

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

❌ Failed checks (2 errors)

Check name Status Explanation Resolution
Performance And Algorithmic Complexity ❌ Error PR introduces N+1 API pattern (~602 calls/run), missing --paginate flags causing review truncation, and 24x frequency increase (daily to hourly) escalating quota to ~14,448 calls/day. Replace separate API calls with single GraphQL query per PR, add --paginate flag with post-processing, revert hourly execution to daily, and implement caching to skip unchanged PRs.
Security And Secret Handling ❌ Error Workflow exposes secrets.PLATFORM_HOST into PR-visible markdown (bodies, comments) and Slack messages, violating secret handling principles. Move PLATFORM_HOST to vars (non-secret variable) or strip from prompts and handle session linking server-side only.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed Title follows Conventional Commits format with 'feat' type and descriptive scope/description accurately summarizing the main change: replacing the LLM-powered review queue with deterministic workflow logic and adding Slack notifications.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Kubernetes Resource Safety ✅ Passed PR modifies only GitHub Actions workflow files (.github/workflows/), not Kubernetes resources. Kubernetes resource safety check is not applicable.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/review-queue-rework
✨ Simplify code
  • Create PR with simplified code
  • Commit simplified code in branch feat/review-queue-rework

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

Was using updatedAt which includes bot's own activity, causing
every PR to be re-processed every cycle and hitting circuit breaker
on healthy PRs.

Now checks for actual issues:
- CI failing
- Merge conflicts
- Changes requested by non-bot reviewers

Healthy PRs are skipped entirely — no session message, no retry_count bump.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.github/workflows/pr-merge-review.yml:
- Around line 72-83: The current logic treats missing or unreadable CI status as
an empty states list (states = []), allowing the PR to proceed; change this to
fail closed: after calling status_checks, if status_checks is falsy (empty) or
json.loads raises JSONDecodeError, immediately return False with a clear message
like "CI unavailable" instead of setting states = []; keep the existing parsing
flow when status_checks is present and only then evaluate the
"FAILURE"/"PENDING" checks on the resulting states variable.
- Around line 143-146: The gh(...) helper is defined as def gh(*args) but is
called with input=..., causing a TypeError; update the gh function signature to
accept an input keyword (e.g., def gh(*args, input=None)) and adjust its
implementation to treat that input value as the subprocess stdin (or translate
it into the equivalent positional "--input", "-" plus passing the JSON bytes to
stdin) so the call site that uses input=json.dumps(...) works without error.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 89a8eb3f-241e-4826-96be-0dc343aeafc1

📥 Commits

Reviewing files that changed from the base of the PR and between 1d94340 and fd84aa7.

📒 Files selected for processing (2)
  • .github/workflows/amber-issue-handler.yml
  • .github/workflows/pr-merge-review.yml

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Nitpick comments (1)
.github/workflows/amber-issue-handler.yml (1)

72-72: Pre-existing: Action pinning to SHA recommended for supply-chain security.

Lines 72, 205, 257, and 306 use mutable tags (ambient-code/ambient-action@v0.0.5) instead of commit SHAs. Per coding guidelines, pin to SHA for supply-chain security. This is pre-existing and outside the scope of the current PR changes (104–105, 290–291, 541–610). Consider addressing in a dedicated security hardening PR.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/amber-issue-handler.yml at line 72, The workflow uses the
mutable tag ambient-code/ambient-action@v0.0.5 in multiple steps; replace each
occurrence of ambient-code/ambient-action@v0.0.5 with the corresponding commit
SHA for that release (pin to a full commit hash) to satisfy supply-chain
security guidelines, ensure you update every instance of the
ambient-code/ambient-action usage, and make this change in a dedicated
security-hardening PR rather than the current feature PR.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.github/workflows/amber-issue-handler.yml:
- Around line 541-542: BOT_USERS is declared but never used; replace the
hardcoded "github-actions[bot]" in the jq filter with a dynamic filter built
from the BOT_USERS set (or remove BOT_USERS if you prefer not to parameterize).
Update the code that constructs the jq expression (the place currently filtering
by "github-actions[bot]") to iterate BOT_USERS and join them into the filter
string used in the jq invocation so all bot usernames in BOT_USERS are matched,
and ensure any quoting/escaping matches how the existing jq call expects its
argument; alternatively, delete the BOT_USERS constant if you choose to keep the
single hardcoded username.
- Around line 565-567: The variable reviews_json is assigned from the gh("api",
f"repos/{REPO}/pulls/{pr_number}/reviews", "--jq", json.dumps([{"state":
".state", "user": ".user.login"}])) call but never used; remove this dead
assignment or replace subsequent logic to use reviews_json consistently. Locate
the redundant assignment to reviews_json and either delete it, or refactor the
subsequent review-checking code to consume reviews_json (the later gh("api",
f"repos/{REPO}/pulls/{pr_number}/reviews", ...) usage) so there is a single
source of truth for the reviews query. Ensure no duplicate API calls remain and
update any variable names accordingly.

---

Nitpick comments:
In @.github/workflows/amber-issue-handler.yml:
- Line 72: The workflow uses the mutable tag ambient-code/ambient-action@v0.0.5
in multiple steps; replace each occurrence of ambient-code/ambient-action@v0.0.5
with the corresponding commit SHA for that release (pin to a full commit hash)
to satisfy supply-chain security guidelines, ensure you update every instance of
the ambient-code/ambient-action usage, and make this change in a dedicated
security-hardening PR rather than the current feature PR.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 962ac067-cffb-4c3a-8084-f7716f2b9f75

📥 Commits

Reviewing files that changed from the base of the PR and between fd84aa7 and 26aca4b.

📒 Files selected for processing (1)
  • .github/workflows/amber-issue-handler.yml

…ove dead code

- Review queue: fail closed when CI status is empty/unknown
- Review queue: fix gh() to accept input_text kwarg for stdin
- Amber handler: remove dead reviews_json assignment
- Amber handler: use BOT_LOGINS list in jq filter instead of hardcoded string

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
.github/workflows/amber-issue-handler.yml (1)

72-72: 🛠️ Refactor suggestion | 🟠 Major

Pin action to commit SHA instead of tag.

ambient-code/ambient-action@v0.0.5 uses a mutable tag. Same issue at lines 205, 257, and 306.

♻️ Proposed fix
-        uses: ambient-code/ambient-action@v0.0.5
+        uses: ambient-code/ambient-action@<full-commit-sha>  # v0.0.5

As per coding guidelines: "Pin action versions to SHA."

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/amber-issue-handler.yml at line 72, Replace the mutable
tag usage of the GitHub Action "ambient-code/ambient-action@v0.0.5" with the
corresponding immutable commit SHA; locate every occurrence of the string
ambient-code/ambient-action@v0.0.5 (appears multiple times in the workflow) and
update each to ambient-code/ambient-action@<commit-sha> where <commit-sha> is
the exact 40-character commit hash for the action release you want to pin.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.github/workflows/pr-merge-review.yml:
- Around line 88-97: The CHANGES_REQUESTED count currently includes bot reviews;
update the gh API query that sets reviews (the gh(...) call) to exclude
reviewers whose .user.login is in BOT_LOGINS so bots don't block milestone
addition. Specifically, change the jq filter used in the gh call to select only
reviews with .state == "CHANGES_REQUESTED" and whose .user.login is not in the
BOT_LOGINS list (or compare against a comma-separated BOT_LOGINS string), then
parse that length into changes_requested as before; keep the variable names
reviews and changes_requested intact.
- Line 160: The milestone link string is missing the milestone ID so the URL
will not resolve; update the f-string in the lines.append call that builds the
message (the line containing f"\n_Total in
<https://github.com/{REPO}/milestone|Review Queue>: {len(ready_prs)}_") to
include the milestone number (e.g. use {milestone_number} or {milestone.id}
depending on your variable) so the URL becomes
https://github.com/{REPO}/milestone/{milestone_number} while keeping the visible
text "Review Queue".

---

Outside diff comments:
In @.github/workflows/amber-issue-handler.yml:
- Line 72: Replace the mutable tag usage of the GitHub Action
"ambient-code/ambient-action@v0.0.5" with the corresponding immutable commit
SHA; locate every occurrence of the string ambient-code/ambient-action@v0.0.5
(appears multiple times in the workflow) and update each to
ambient-code/ambient-action@<commit-sha> where <commit-sha> is the exact
40-character commit hash for the action release you want to pin.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 58f5f4bc-f7cd-43ab-b0e7-651b91afd42f

📥 Commits

Reviewing files that changed from the base of the PR and between 26aca4b and 828043c.

📒 Files selected for processing (2)
  • .github/workflows/amber-issue-handler.yml
  • .github/workflows/pr-merge-review.yml

Ambient Code Bot and others added 2 commits April 7, 2026 10:40
- PR body: visible session link at bottom (above the fold, not just
  hidden in frontmatter HTML comment)
- PR comments: small italic footer linking to the session

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Circuit breaker should not fire on healthy PRs. Agent now only
increments retry_count when it had to fix a CI failure, conflict,
or review comment. Healthy PRs just get last_action updated.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
.github/workflows/amber-issue-handler.yml (1)

72-72: ⚠️ Potential issue | 🟡 Minor

Pin action to SHA per coding guidelines.

ambient-code/ambient-action@v0.0.5 at lines 72, 210, 264, 318 uses version tags instead of SHA pinning. Tags can be reassigned; replace with the commit SHA to ensure immutability.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/amber-issue-handler.yml at line 72, The workflow uses the
mutable tag ambient-code/ambient-action@v0.0.5 (appearing multiple times) which
must be pinned to a specific commit SHA; replace each occurrence of
ambient-code/ambient-action@v0.0.5 with ambient-code/ambient-action@<COMMIT_SHA>
where <COMMIT_SHA> is the full commit hash from the ambient-action repository
for the desired release, ensuring all instances are updated to the exact same
SHA to guarantee immutability.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.github/workflows/amber-issue-handler.yml:
- Around line 555-588: The needs_attention function currently treats an empty CI
`states` list as "healthy" and skips PRs with no CI; add an explicit check after
parsing `states` (the variable produced by json.loads) to handle the no-CI case
by returning False, "CI unknown" (matching is_ready_to_review behavior). Update
the code in needs_attention (function name: needs_attention) to: after the
try/except that sets `states`, add `if not states: return False, "CI unknown"`,
ensuring PRs with no CI are classified consistently.

---

Outside diff comments:
In @.github/workflows/amber-issue-handler.yml:
- Line 72: The workflow uses the mutable tag ambient-code/ambient-action@v0.0.5
(appearing multiple times) which must be pinned to a specific commit SHA;
replace each occurrence of ambient-code/ambient-action@v0.0.5 with
ambient-code/ambient-action@<COMMIT_SHA> where <COMMIT_SHA> is the full commit
hash from the ambient-action repository for the desired release, ensuring all
instances are updated to the exact same SHA to guarantee immutability.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: ffc41f63-9ff7-4611-a7c0-6c731d59dfa5

📥 Commits

Reviewing files that changed from the base of the PR and between 828043c and a3b6099.

📒 Files selected for processing (1)
  • .github/workflows/amber-issue-handler.yml

Comment on lines +555 to +588
def needs_attention(pr_number):
"""Check if a PR has actionable issues that need the fixer's attention.
Returns (needs_work, reason) tuple."""
# Check CI status
checks_json = gh("pr", "checks", str(pr_number), "--repo", REPO,
"--json", "name,state",
"--jq", "[.[] | .state] | unique")
try:
states = json.loads(checks_json) if checks_json else []
except json.JSONDecodeError:
states = []

if "FAILURE" in states:
return True, "CI failing"

# Check for merge conflicts
mergeable = gh("pr", "view", str(pr_number), "--repo", REPO,
"--json", "mergeable", "--jq", ".mergeable")
if mergeable == "CONFLICTING":
return True, "merge conflicts"

# Check for changes_requested from non-bot users
bot_filter = " and ".join([f'.user.login != "{b}"' for b in BOT_LOGINS])
try:
reviews_raw = gh("api", f"repos/{REPO}/pulls/{pr_number}/reviews",
"--jq", f'[.[] | select(.state == "CHANGES_REQUESTED" and {bot_filter})] | length')
changes_requested = int(reviews_raw) if reviews_raw else 0
except (ValueError, TypeError):
changes_requested = 0

if changes_requested > 0:
return True, "changes requested"

return False, "healthy"
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.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check if any ambient-code:managed PRs currently have no CI checks
gh pr list --repo "$GITHUB_REPOSITORY" --state open --label "ambient-code:managed" --json number --jq '.[].number' | while read -r pr; do
  checks=$(gh pr checks "$pr" --repo "$GITHUB_REPOSITORY" --json name 2>/dev/null | jq 'length')
  if [ "$checks" = "0" ] || [ -z "$checks" ]; then
    echo "PR #$pr has no CI checks"
  fi
done

Repository: ambient-code/platform

Length of output: 246


🏁 Script executed:

# Find and inspect the is_ready_to_review function to compare CI handling
fd -e yml -e yaml | head -20

Repository: ambient-code/platform

Length of output: 1233


🏁 Script executed:

# Look at the actual amber-issue-handler.yml file around the flagged lines
wc -l .github/workflows/amber-issue-handler.yml

Repository: ambient-code/platform

Length of output: 110


🏁 Script executed:

# Search for is_ready_to_review to see how it differs from needs_attention
rg "is_ready_to_review" .github/workflows/ -A 20

Repository: ambient-code/platform

Length of output: 3498


Add handling for PRs with no CI checks.

When states is empty (no CI runs), needs_attention() returns (False, "healthy"), causing the batch-fixer to skip. Meanwhile, is_ready_to_review() in pr-merge-review.yml explicitly returns (False, "CI unknown"), keeping the PR out of the Review Queue.

Result: PRs without CI are neither fixed nor queued for review. Consider either:

  • Adding an explicit check in needs_attention() to handle missing CI (e.g., if not states: return False, "CI unknown"), or
  • Documenting why this edge case is acceptable for the project's workflow
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/amber-issue-handler.yml around lines 555 - 588, The
needs_attention function currently treats an empty CI `states` list as "healthy"
and skips PRs with no CI; add an explicit check after parsing `states` (the
variable produced by json.loads) to handle the no-CI case by returning False,
"CI unknown" (matching is_ready_to_review behavior). Update the code in
needs_attention (function name: needs_attention) to: after the try/except that
sets `states`, add `if not states: return False, "CI unknown"`, ensuring PRs
with no CI are classified consistently.

- Filter out bot reviews (github-actions, ambient-code, ambient-bot)
  when checking for CHANGES_REQUESTED — consistent with amber handler
- Fix milestone URL to include milestone number

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.github/workflows/amber-issue-handler.yml:
- Around line 103-110: This workflow is rendering a secret-scoped value
($PLATFORM_HOST from secrets.PLATFORM_HOST) into PR-visible markdown; replace
all occurrences in .github/workflows/amber-issue-handler.yml (including the
blocks around lines referenced: 103-110, 241-242, 298-305, 652-653) so the
runtime value is not output directly: reference a non-secret repo/org-level
variable (e.g., repository or environment variable exposed via
vars.PLATFORM_HOST or an inputs/default placeholder), or render a static
placeholder URL instead, and ensure the action builds the Session link using a
non-secret value or redacts the host before inserting it into PR body/comments
so secrets.* are never exposed in PR-visible markdown.
- Around line 581-588: The reviews counting currently treats every historical
review as equal; update the gh call and post-processing so only each reviewer's
latest non-bot review is considered: add "--paginate" to the gh("api",
f"repos/{REPO}/pulls/{pr_number}/reviews", ...) invocation, then change the jq
expression used with bot_filter to group by reviewer (user.login), sort by
submitted_at descending, pick the first non-bot entry per user, and count those
with state == "CHANGES_REQUESTED"; finally set changes_requested from that count
(instead of counting all CHANGES_REQUESTED entries). Alternatively, replace the
REST call with a GraphQL query that returns reviewDecision or
latestOpinionatedReviews and derive changes_requested from that result.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 70cdd95c-74ac-48be-ab6c-e7efcc453ed6

📥 Commits

Reviewing files that changed from the base of the PR and between a3b6099 and 2ac1605.

📒 Files selected for processing (1)
  • .github/workflows/amber-issue-handler.yml

Comment on lines +581 to +588
reviews_raw = gh("api", f"repos/{REPO}/pulls/{pr_number}/reviews",
"--jq", f'[.[] | select(.state == "CHANGES_REQUESTED" and {bot_filter})] | length')
changes_requested = int(reviews_raw) if reviews_raw else 0
except (ValueError, TypeError):
changes_requested = 0

if changes_requested > 0:
return True, "changes requested"
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.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

repo="${GITHUB_REPOSITORY:-$(gh repo view --json nameWithOwner --jq '.nameWithOwner')}"

gh pr list --repo "$repo" --state open --label "ambient-code:managed" --json number --jq '.[].number' |
while read -r pr; do
  reviews="$(gh api --paginate "repos/$repo/pulls/$pr/reviews" | jq -cs 'add')"
  python - "$pr" <<'PY' <<<"$reviews"
import json, sys

pr = sys.argv[1]
bots = {"github-actions[bot]", "ambient-code[bot]", "ambient-bot"}
reviews = json.load(sys.stdin)

historical = any(
    r.get("state") == "CHANGES_REQUESTED"
    and r.get("user", {}).get("login") not in bots
    for r in reviews
)

latest = {}
for r in sorted(reviews, key=lambda r: r.get("submitted_at") or ""):
    login = r.get("user", {}).get("login")
    if login and login not in bots:
        latest[login] = r.get("state")

active = any(state == "CHANGES_REQUESTED" for state in latest.values())
if historical and not active:
    print(f"PR #{pr} has stale CHANGES_REQUESTED history but no active latest non-bot changes request")
PY
done

Repository: ambient-code/platform

Length of output: 1796


🏁 Script executed:

fd -t f amber-issue-handler.yml

Repository: ambient-code/platform

Length of output: 47


🏁 Script executed:

ls -la .github/workflows/ | head -30

Repository: ambient-code/platform

Length of output: 1922


🏁 Script executed:

sed -n '570,600p' .github/workflows/amber-issue-handler.yml

Repository: ambient-code/platform

Length of output: 1482


🏁 Script executed:

sed -n '1,100p' .github/workflows/amber-issue-handler.yml | grep -A 30 "def gh"

Repository: ambient-code/platform

Length of output: 47


🏁 Script executed:

grep -n "def gh" .github/workflows/amber-issue-handler.yml

Repository: ambient-code/platform

Length of output: 93


🏁 Script executed:

sed -n '434,460p' .github/workflows/amber-issue-handler.yml

Repository: ambient-code/platform

Length of output: 1437


🌐 Web query:

GitHub API pulls reviews endpoint historical all reviews paginated

💡 Result:

Use the REST endpoint List reviews for a pull request:

  • Endpoint: GET /repos/{owner}/{repo}/pulls/{pull_number}/reviews (returns reviews in chronological order) [1]
  • Pagination params: per_page (default 30, max 100) and page (default 1) [1]
  • How to fetch all (historical) reviews: request pages until GitHub stops returning a Link header with rel="next" (GitHub’s standard REST pagination mechanism) [2]

Example:

# page 1
curl -i -H "Accept: application/vnd.github+json" \
  -H "Authorization: Bearer $TOKEN" \
  "https://api.github.com/repos/OWNER/REPO/pulls/123/reviews?per_page=100&page=1"

# page 2, 3, ... until there's no rel="next" in the Link header

If you also need the inline comments for each review, fetch them per review id:

  • GET /repos/{owner}/{repo}/pulls/{pull_number}/reviews/{review_id}/comments [1]

Sources: GitHub REST “Pull request reviews” docs [1], GitHub REST pagination guide (Link headers / per_page / page iteration) [2].


🏁 Script executed:

sed -n '575,595p' .github/workflows/amber-issue-handler.yml

Repository: ambient-code/platform

Length of output: 1081


🌐 Web query:

GitHub CLI gh api command automatic pagination default behavior

💡 Result:

gh api does not automatically paginate by default. Without any flags, it makes a single request and returns only the first page that the GitHub API returns for that endpoint. To fetch all pages, you must opt in with --paginate. [1]

Notes:

  • --paginate tells gh to “make additional HTTP requests to fetch all pages of results.” [1]
  • If you want a single combined JSON array/object across pages (instead of page-by-page output), use --slurp together with --paginate. [1]
  • Page size is still controlled by the API parameters (e.g., -F per_page=100 for many REST endpoints); --paginate just follows the pagination links/cursors to retrieve subsequent pages. [1]

Sources
[1] GitHub CLI manual: gh api (flags --paginate, --slurp) (cli.github.com)

Citations:


🌐 Web query:

GitHub API pull request review decision latest state each reviewer

💡 Result:

To get the latest review state per reviewer and the PR’s overall review decision, you have two practical options:

1) GraphQL API (best: server computes “latest per user”)

Query the PR’s overall decision via PullRequest.reviewDecision (values include APPROVED, CHANGES_REQUESTED, REVIEW_REQUIRED) and the latest review per user via latestOpinionatedReviews. [1][2][3]

Example:

query($owner:String!, $repo:String!, $number:Int!) {
  repository(owner:$owner, name:$repo) {
    pullRequest(number:$number) {
      reviewDecision
      latestOpinionatedReviews(first: 100) {
        nodes {
          author { login }
          state
          submittedAt
          url
        }
      }
    }
  }
}

state is a PullRequestReviewState (e.g. APPROVED, CHANGES_REQUESTED, COMMENTED, DISMISSED, PENDING). [3]

2) REST API (you compute “latest per user” yourself)

Call “List reviews for a pull request”, then group by user.login and pick the most recent relevant review (typically by submitted_at, optionally ignoring state == "PENDING"). The REST review objects include user and state (e.g. APPROVED, CHANGES_REQUESTED, DISMISSED). [4]

Endpoint:

  • GET /repos/{owner}/{repo}/pulls/{pull_number}/reviews [4]

Sources

[1] GitHub Community discussion referencing PullRequest.reviewDecision field [7]
[2] GitHub GraphQL objects docs (GHES) snippet: latestOpinionatedReviews = “latest reviews per user”, and reviewDecision field [2]
[3] GitHub GraphQL enum docs: PullRequestReviewDecision + PullRequestReviewState values [3]
[4] GitHub REST docs: Pull request reviews endpoint and review object fields (includes user, state, submitted_at) [1]


Count only each reviewer's latest non-bot review state, not all historical entries. Add --paginate and group by reviewer.

The current code counts every CHANGES_REQUESTED entry in the reviews history. If reviewer A requests changes, then approves, the old entry still gets counted, marking the PR unhealthy. Additionally, without --paginate, the command only fetches the first ~30 reviews; if the PR has more reviewers, recent reviews on later pages are skipped.

Use one of:

  1. GraphQL (simpler): Query reviewDecision + latestOpinionatedReviews to get GitHub's computed latest state per reviewer.
  2. REST + grouping: Add --paginate, then post-process to group reviews by reviewer and take the most recent non-bot entry per user before checking for CHANGES_REQUESTED.
Current problematic snippet
reviews_raw = gh("api", f"repos/{REPO}/pulls/{pr_number}/reviews",
                "--jq", f'[.[] | select(.state == "CHANGES_REQUESTED" and {bot_filter})] | length')
changes_requested = int(reviews_raw) if reviews_raw else 0
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/amber-issue-handler.yml around lines 581 - 588, The
reviews counting currently treats every historical review as equal; update the
gh call and post-processing so only each reviewer's latest non-bot review is
considered: add "--paginate" to the gh("api",
f"repos/{REPO}/pulls/{pr_number}/reviews", ...) invocation, then change the jq
expression used with bot_filter to group by reviewer (user.login), sort by
submitted_at descending, pick the first non-bot entry per user, and count those
with state == "CHANGES_REQUESTED"; finally set changes_requested from that count
(instead of counting all CHANGES_REQUESTED entries). Alternatively, replace the
REST call with a GraphQL query that returns reviewDecision or
latestOpinionatedReviews and derive changes_requested from that result.

- PLATFORM_HOST is a URL rendered in PR bodies/comments — should be a
  repo variable, not a secret (secrets get masked in logs)
- Explicit handling for PRs with no CI checks in needs_attention

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@Gkrumbach07 Gkrumbach07 force-pushed the feat/review-queue-rework branch from b305d22 to f30f85c Compare April 7, 2026 16:31
@Gkrumbach07 Gkrumbach07 merged commit c994ec5 into main Apr 7, 2026
8 checks passed
@Gkrumbach07 Gkrumbach07 deleted the feat/review-queue-rework branch April 7, 2026 16:33
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.

1 participant