Skip to content

Commit 24abf11

Browse files
committed
fix(ci): block BM Bossbot approval on unresolved review threads; theme PR images on PR content
Two BM Bossbot fixes: 1. Unresolved review threads now block approval. The review prompt only sees metadata+diff, so an approve verdict said nothing about outstanding feedback — #932 merged with two open Codex P2 threads. finalize now counts unresolved reviewThreads via GraphQL and fails the status when any remain. A new recheck command, triggered by pull_request_review / review_comment / review_thread events, flips the status to failure when feedback arrives after approval and restores a previously earned approval for the same head SHA once every thread is resolved. Review and recheck jobs use separate job-level concurrency groups so neither cancels the other. 2. PR images depict the PR's content, not the review outcome. The image prompt previously used the BM Bossbot review summary (verdict/status) as its only source material, so every image rendered an APPROVED stamp. The prompt now sources the PR title and the author's own description (managed bot blocks stripped) and explicitly bans approval stamps, verdict wording, badges, checkmarks, and Bossbot branding. Theme selection seeds on PR number+title for stability across re-reviews. Signed-off-by: phernandez <paul@basicmachines.co>
1 parent ca9a4d9 commit 24abf11

5 files changed

Lines changed: 564 additions & 64 deletions

File tree

.github/workflows/bm-bossbot.yml

Lines changed: 52 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,27 +11,42 @@ name: BM Bossbot
1111
pr_number:
1212
description: Pull request number to review
1313
required: true
14+
# Review-thread activity re-evaluates the approval status without re-running
15+
# the full LLM review: new feedback flips the gate to failure, resolving the
16+
# last thread restores a previously earned approval for the same head SHA.
17+
pull_request_review:
18+
types:
19+
- submitted
20+
pull_request_review_comment:
21+
types:
22+
- created
23+
pull_request_review_thread:
24+
types:
25+
- resolved
26+
- unresolved
1427

1528
permissions:
1629
contents: read
1730
pull-requests: write
1831
statuses: write
1932
issues: read
2033

21-
concurrency:
22-
group: bm-bossbot-${{ github.event.workflow_run.pull_requests[0].number || inputs.pr_number }}
23-
cancel-in-progress: true
24-
2534
env:
2635
FORCE_JAVASCRIPT_ACTIONS_TO_NODE24: "true"
2736
BM_BOSSBOT_STATUS_CONTEXT: "BM Bossbot Approval"
2837

2938
jobs:
3039
review:
3140
name: BM Bossbot Review
41+
# Job-level concurrency (not workflow-level): thread-recheck events for the
42+
# same PR must never cancel an in-flight review run, and vice versa.
43+
concurrency:
44+
group: bm-bossbot-review-${{ github.event.workflow_run.pull_requests[0].number || inputs.pr_number }}
45+
cancel-in-progress: true
3246
if: |
3347
github.event_name == 'workflow_dispatch' ||
3448
(
49+
github.event_name == 'workflow_run' &&
3550
github.event.workflow_run.conclusion == 'success' &&
3651
github.event.workflow_run.pull_requests[0].number != ''
3752
)
@@ -298,8 +313,10 @@ jobs:
298313
run: |
299314
set -euo pipefail
300315
gh pr view "${PR_NUMBER}" --repo "${GITHUB_REPOSITORY}" --json body --jq '.body // ""' > "${RUNNER_TEMP}/bm-bossbot-pr-body.md"
316+
pr_title="$(gh pr view "${PR_NUMBER}" --repo "${GITHUB_REPOSITORY}" --json title --jq '.title // ""')"
301317
uv run --script scripts/generate_pr_infographic.py \
302318
--pr-number "${PR_NUMBER}" \
319+
--pr-title "${pr_title}" \
303320
--pr-body-file "${RUNNER_TEMP}/bm-bossbot-pr-body.md" \
304321
--provenance-output "${RUNNER_TEMP}/bm-bossbot-image-provenance.md" \
305322
--output "docs/assets/infographics/pr-${PR_NUMBER}.webp"
@@ -375,3 +392,34 @@ jobs:
375392
Path(output_path).write_text(body, encoding="utf-8")
376393
PY
377394
gh pr edit "${PR_NUMBER}" --repo "${GITHUB_REPOSITORY}" --body-file "${updated_body}"
395+
396+
recheck:
397+
name: BM Bossbot Thread Recheck
398+
if: |
399+
github.event_name == 'pull_request_review' ||
400+
github.event_name == 'pull_request_review_comment' ||
401+
github.event_name == 'pull_request_review_thread'
402+
runs-on: ubuntu-latest
403+
# Job-level concurrency: collapse bursts of thread events for one PR while
404+
# staying isolated from the review job's group so neither cancels the other.
405+
concurrency:
406+
group: bm-bossbot-recheck-${{ github.event.pull_request.number }}
407+
cancel-in-progress: true
408+
steps:
409+
- name: Checkout trusted base ref
410+
uses: actions/checkout@v6
411+
with:
412+
ref: ${{ github.event.repository.default_branch }}
413+
fetch-depth: 1
414+
415+
- name: Set up uv
416+
uses: astral-sh/setup-uv@v3
417+
418+
- name: Re-evaluate approval from review-thread state
419+
env:
420+
GITHUB_TOKEN: ${{ github.token }}
421+
run: |
422+
uv run --script scripts/bm_bossbot_status.py recheck \
423+
--pr-number "${{ github.event.pull_request.number }}" \
424+
--repo "${GITHUB_REPOSITORY}" \
425+
--run-url "${GITHUB_SERVER_URL}/${GITHUB_REPOSITORY}/actions/runs/${GITHUB_RUN_ID}"

scripts/bm_bossbot_status.py

Lines changed: 170 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,66 @@ def pull_request_event(
9797
)
9898

9999

100+
def count_unresolved_review_threads(*, token: str, repo: str, number: int) -> int:
101+
"""Count unresolved review threads (e.g. open Codex inline comments) on a PR.
102+
103+
Review threads are the canonical 'outstanding feedback' signal: bot reviewers
104+
submit COMMENTED reviews that never flip reviewDecision, so thread resolution
105+
is the only deterministic way to know feedback was addressed.
106+
"""
107+
owner, _, name = repo.partition("/")
108+
if not owner or not name:
109+
raise SystemExit(f"Invalid repository: {repo}")
110+
111+
query = """
112+
query($owner: String!, $name: String!, $number: Int!, $cursor: String) {
113+
repository(owner: $owner, name: $name) {
114+
pullRequest(number: $number) {
115+
reviewThreads(first: 100, after: $cursor) {
116+
pageInfo { hasNextPage endCursor }
117+
nodes { isResolved }
118+
}
119+
}
120+
}
121+
}
122+
"""
123+
124+
unresolved = 0
125+
cursor: str | None = None
126+
while True:
127+
response = _github_request(
128+
method="POST",
129+
path="/graphql",
130+
token=token,
131+
payload={
132+
"query": query,
133+
"variables": {"owner": owner, "name": name, "number": number, "cursor": cursor},
134+
},
135+
)
136+
if not isinstance(response, Mapping) or response.get("errors"):
137+
raise SystemExit(f"GitHub GraphQL reviewThreads query failed: {response}")
138+
try:
139+
threads = response["data"]["repository"]["pullRequest"]["reviewThreads"]
140+
nodes = threads["nodes"]
141+
page_info = threads["pageInfo"]
142+
except (KeyError, TypeError):
143+
raise SystemExit(
144+
"GitHub GraphQL reviewThreads response was missing expected fields"
145+
) from None
146+
unresolved += sum(1 for node in nodes if not node.get("isResolved"))
147+
if not page_info.get("hasNextPage"):
148+
return unresolved
149+
cursor = page_info.get("endCursor")
150+
151+
152+
def unresolved_threads_result(count: int) -> ApprovalResult:
153+
return ApprovalResult(
154+
False,
155+
"failure",
156+
f"BM Bossbot found {count} unresolved review thread(s)",
157+
)
158+
159+
100160
def validate_review(payload: Mapping[str, Any], *, expected_head_sha: str) -> ApprovalResult:
101161
required = {
102162
"reviewed_head_sha",
@@ -245,6 +305,19 @@ def finalize_review(
245305
review = {}
246306

247307
result = validate_review(review, expected_head_sha=event.head_sha)
308+
# Trigger: the LLM review approved, but reviewers (human or bot, e.g. Codex
309+
# inline comments) still have unresolved threads on the PR.
310+
# Why: the review prompt only sees metadata+diff, never review threads, so an
311+
# approve verdict says nothing about outstanding feedback (#932 merged
312+
# with two open P2 threads because of exactly this gap).
313+
# Outcome: unresolved threads turn an approve into a failure status; the
314+
# recheck command restores approval once all threads are resolved.
315+
if result.approved:
316+
unresolved = count_unresolved_review_threads(
317+
token=token, repo=event.repo, number=event.number
318+
)
319+
if unresolved > 0:
320+
result = unresolved_threads_result(unresolved)
248321
current_body = get_pull_request_body(token=token, repo=event.repo, number=event.number)
249322
updated_body = upsert_summary_block(current_body, render_summary(review, result))
250323
update_pull_request_body(token=token, repo=event.repo, number=event.number, body=updated_body)
@@ -262,6 +335,89 @@ def finalize_review(
262335
return result
263336

264337

338+
def get_pull_request_head_sha(*, token: str, repo: str, number: int) -> str:
339+
response = _github_request(
340+
method="GET",
341+
path=f"/repos/{repo}/pulls/{number}",
342+
token=token,
343+
)
344+
if not isinstance(response, Mapping):
345+
raise SystemExit("GitHub API response for pull request was invalid")
346+
head = response.get("head")
347+
head_sha = _string(head.get("sha")) if isinstance(head, Mapping) else ""
348+
if not head_sha:
349+
raise SystemExit("GitHub API response was missing pull request head SHA")
350+
return head_sha
351+
352+
353+
def head_sha_was_approved(*, token: str, repo: str, sha: str) -> bool:
354+
"""Return whether a full BM Bossbot review previously approved this head SHA.
355+
356+
Commit statuses are append-only history, so the approval record survives a
357+
later thread-failure status for the same SHA.
358+
"""
359+
response = _github_request(
360+
method="GET",
361+
path=f"/repos/{repo}/commits/{sha}/statuses?per_page=100",
362+
token=token,
363+
)
364+
if not isinstance(response, list):
365+
raise SystemExit("GitHub API response for commit statuses was invalid")
366+
return any(
367+
isinstance(status, Mapping)
368+
and status.get("context") == STATUS_CONTEXT
369+
and status.get("state") == "success"
370+
and status.get("description") == APPROVED_DESCRIPTION
371+
for status in response
372+
)
373+
374+
375+
def recheck_threads(
376+
*,
377+
repo: str,
378+
number: int,
379+
run_url: str,
380+
token_env: str,
381+
) -> None:
382+
"""Re-evaluate the approval status when review threads change.
383+
384+
Trigger: pull_request_review / review_comment / review_thread events.
385+
Why: the full review runs once per head SHA after Tests; feedback that
386+
arrives later (or gets resolved later) must move the gate without
387+
re-running the LLM review.
388+
Outcome: unresolved threads flip the status to failure; once every thread
389+
is resolved, a previously earned approval for the same head SHA is
390+
restored. Without a prior approval the status is left untouched so a
391+
pending/failed review cannot be upgraded by thread resolution alone.
392+
"""
393+
token = _token(token_env)
394+
head_sha = get_pull_request_head_sha(token=token, repo=repo, number=number)
395+
unresolved = count_unresolved_review_threads(token=token, repo=repo, number=number)
396+
397+
if unresolved > 0:
398+
result = unresolved_threads_result(unresolved)
399+
elif head_sha_was_approved(token=token, repo=repo, sha=head_sha):
400+
result = ApprovalResult(True, "success", APPROVED_DESCRIPTION)
401+
else:
402+
typer.echo(
403+
f"All review threads resolved but no prior approval exists for {head_sha}; "
404+
"leaving status unchanged"
405+
)
406+
return
407+
408+
set_commit_status(
409+
token=token,
410+
repo=repo,
411+
sha=head_sha,
412+
payload=build_status_payload(
413+
state=result.state,
414+
description=result.description,
415+
target_url=run_url,
416+
),
417+
)
418+
typer.echo(f"Marked {STATUS_CONTEXT} {result.state} for {head_sha} ({result.description})")
419+
420+
265421
def _github_request(
266422
*,
267423
method: str,
@@ -378,6 +534,20 @@ def finalize(
378534
raise typer.Exit(1)
379535

380536

537+
@app.command("recheck")
538+
def recheck(
539+
pr_number: Annotated[int, typer.Option("--pr-number", min=1, help="Pull request number.")],
540+
run_url: Annotated[str, typer.Option("--run-url", help="Workflow run URL.")],
541+
repo: Annotated[str, typer.Option("--repo", help="owner/name repository.")],
542+
token_env: Annotated[
543+
str,
544+
typer.Option("--token-env", help="Environment variable containing a GitHub token."),
545+
] = "GITHUB_TOKEN",
546+
) -> None:
547+
"""Re-evaluate BM Bossbot Approval from current review-thread state."""
548+
recheck_threads(repo=repo, number=pr_number, run_url=run_url, token_env=token_env)
549+
550+
381551
def main() -> None:
382552
app()
383553

0 commit comments

Comments
 (0)