This is an automated email from the ASF dual-hosted git repository.
hello-stephen pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/master by this push:
new 1fc02707cb1 [fix](ci) Verify completed reviews before recovering
wrap-up capacity errors (#68599)
1fc02707cb1 is described below
commit 1fc02707cb1c1cb1d9ea942cfc575bda9c184b93
Author: shuke <[email protected]>
AuthorDate: Tue Sep 29 11:24:50 2026 +0800
[fix](ci) Verify completed reviews before recovering wrap-up capacity
errors (#68599)
Related PR: #68567
Problem Summary:
The review of #66227 in run 36422537817 submitted and verified review
5339520198 with one P1 finding, then hit model capacity during wrap-up.
The
runner waited five minutes and failed its duplicate-review guard. The
existing status publisher also treated any successful execution as a
passing
review without considering P0/P1 findings.
Declare the complete final review through a trusted helper loaded from
the
workflow SHA. Bind its summary, full inline set, and reconfirmed
existing
blockers to a per-invocation marker and target head. Independently
verify
GitHub delivery before accepting completion, including after a terminal
capacity error. Read back ambiguous submissions without reposting.
Preserve
failure for incomplete, foreign, stale, cancelled, or unverifiable work.
Separate execution completion from the PR verdict: completed P0/P1
reviews
block code-review, while completed reviews without P0/P1 pass. Keep
authorized
local PASS/skip overrides. Retain the real failed model turn in
telemetry and
record verified completion separately. Six five-minute retries remain
for
unfinished capacity failures.
---
.github/scripts/emit_litefuse_otel_io.py | 10 +
.github/scripts/resolve_code_review_status.py | 13 +
.github/scripts/run_review_with_resume.py | 30 +-
.github/scripts/submit_review.py | 217 ++++++++++++++
.github/scripts/test_emit_litefuse_otel_io.py | 20 ++
.github/scripts/test_resolve_code_review_status.py | 27 ++
.github/scripts/test_review_auth_quarantine.py | 59 +++-
.github/scripts/test_run_review_with_resume.py | 38 ++-
.github/scripts/test_submit_review.py | 311 +++++++++++++++++++++
.github/workflows/code-review-runner.yml | 74 ++---
10 files changed, 745 insertions(+), 54 deletions(-)
diff --git a/.github/scripts/emit_litefuse_otel_io.py
b/.github/scripts/emit_litefuse_otel_io.py
index 53ef46949af..4da5db39950 100644
--- a/.github/scripts/emit_litefuse_otel_io.py
+++ b/.github/scripts/emit_litefuse_otel_io.py
@@ -379,6 +379,16 @@ def build_ingestion_payload(args, input_text, output_text,
events):
key: value for key, value in trace_metadata.items() if value not in
(None, "")
}
+ # Keep the model turn's real failure/usage while exposing the independently
+ # verified delivery and blocking verdict as a separate trace outcome.
+ completion = next((event for event in reversed(events)
+ if event.get("type") == "review.completed"), None)
+ if completion is not None:
+ trace_metadata["review_completion"] = {
+ key: value for key, value in completion.items()
+ if key not in ("type", "_line_number")
+ }
+
completed_items = [
event
for event in events
diff --git a/.github/scripts/resolve_code_review_status.py
b/.github/scripts/resolve_code_review_status.py
index cc81807e34e..a2d54302577 100644
--- a/.github/scripts/resolve_code_review_status.py
+++ b/.github/scripts/resolve_code_review_status.py
@@ -72,10 +72,23 @@ def resolve_status(
latest_by_context[context] = item
approved_contexts: set[tuple[int, str]] = set()
+ blocked_contexts: set[tuple[int, str]] = set()
for context, item in latest_by_context.items():
match = SOURCE_CONTEXT_RE.fullmatch(context)
if match is not None and item["state"] == "success":
approved_contexts.add((int(match.group(1)),
match.group(2).casefold()))
+ elif (match is not None and item["state"] == "failure"
+ and context.startswith("code-review/source/automated/")):
+ blocked_contexts.add((int(match.group(1)),
match.group(2).casefold()))
+
+ # Preserve the authorized local PASS / skip override. Otherwise a completed
+ # automated review with P0/P1 blocks, rather than appearing to still be
running.
+ blocked = sorted((open_contexts - approved_contexts) & blocked_contexts)
+ if blocked:
+ number, base_sha = blocked[0]
+ return Resolution(
+ "failure", f"Review completed with P0/P1 for PR #{number} at
{base_sha[:12]}.",
+ )
missing = sorted(open_contexts - approved_contexts)
if missing:
diff --git a/.github/scripts/run_review_with_resume.py
b/.github/scripts/run_review_with_resume.py
index a6176cb3616..43fe87a4079 100755
--- a/.github/scripts/run_review_with_resume.py
+++ b/.github/scripts/run_review_with_resume.py
@@ -32,6 +32,8 @@ import uuid
from datetime import datetime, timezone
from pathlib import Path
+from submit_review import RUN_FILE, RESULT_FILE, SUBMISSION_FILE,
verify_completion, write_json
+
# Six same-session capacity retries, each after a fixed five-minute wait.
RETRY_DELAYS = (300,) * 6
CAPACITY_MESSAGE = "Selected model is at capacity. Please try a different
model."
@@ -353,6 +355,12 @@ def run_review(args, reaper=None):
goal_prompt = (context / "codex_goal_prompt.txt").read_text()
deadline = time.monotonic() + args.budget_seconds
started_at = datetime.now(timezone.utc).strftime("%Y-%m-%dT%H:%M:%SZ")
+ run = {
+ "repository": args.repository, "pr_number": args.pr_number,
+ "head_sha": args.head_sha, "base_sha": args.base_sha,
+ "started_at": started_at, "token": str(uuid.uuid4()), "deadline":
deadline,
+ }
+ write_json(context / RUN_FILE, run)
thread_id = None
model = args.model
fallback_model = getattr(args, "fallback_model", None)
@@ -458,9 +466,7 @@ def run_review(args, reaper=None):
)
model = fallback_model
continue
- if message is not None and (
- message != CAPACITY_MESSAGE or capacity_retry ==
len(RETRY_DELAYS)
- ):
+ if message is not None and message != CAPACITY_MESSAGE:
return fail(message)
current_id = session_id(events)
if thread_id and current_id != thread_id:
@@ -468,8 +474,24 @@ def run_review(args, reaper=None):
"Codex resumed a different session; refusing further
attempts"
)
thread_id = current_id
- if message is None:
+ if message is None or (context / SUBMISSION_FILE).exists():
+ # Final submission declares all review work complete. Only an
+ # exact GitHub readback can recover a subsequent capacity
error;
+ # another run's review or a partially posted review cannot
pass.
+ result = verify_completion(context, run, remaining)
+ result["recovered_after_capacity"] = message ==
CAPACITY_MESSAGE
+ write_json(context / RESULT_FILE, result)
+ with aggregate.open("a") as handle:
+ handle.write(json.dumps({"type": "review.completed",
**result}) + "\n")
+ print(
+ f"Verified final review {result['review_id']}: "
+ f"P0={result['p0']}, P1={result['p1']}, "
+
f"recovered_after_capacity={result['recovered_after_capacity']}",
+ file=sys.stderr, flush=True,
+ )
return 0
+ if capacity_retry == len(RETRY_DELAYS):
+ return fail(message)
require_rollout(Path(os.environ["CODEX_HOME"]), thread_id,
args.cwd)
# Check parser support without authenticating or starting a model
request.
subprocess.run(
diff --git a/.github/scripts/submit_review.py b/.github/scripts/submit_review.py
new file mode 100644
index 00000000000..edd8fe9cf18
--- /dev/null
+++ b/.github/scripts/submit_review.py
@@ -0,0 +1,217 @@
+#!/usr/bin/env python3
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+"""Submit and verify a final review bound to this runner invocation."""
+
+import argparse
+import hashlib
+import json
+import re
+import subprocess
+import time
+from collections import Counter
+from pathlib import Path
+
+
+SUBMISSION_FILE = "review-submission.json"
+RUN_FILE = "review-run.json"
+RESULT_FILE = "review-result.json"
+PRIORITY = re.compile(r"^\[P([0-3])\]\s+\S")
+
+
+def write_json(path, value):
+ temporary = path.with_suffix(".tmp")
+ temporary.write_text(json.dumps(value, indent=2) + "\n")
+ temporary.replace(path)
+
+
+def priority(body):
+ match = PRIORITY.match(body)
+ if match is None:
+ raise ValueError("Every finding must start with [P0], [P1], [P2] or
[P3]")
+ return int(match.group(1))
+
+
+def validate_submission(value):
+ if not isinstance(value, dict) or set(value) != {
+ "body", "comments", "existing_blocking_comment_ids"
+ }:
+ raise ValueError("Final review requires body, comments and
existing_blocking_comment_ids")
+ if not isinstance(value["body"], str) or not value["body"].strip():
+ raise ValueError("Final review summary is empty")
+ if not isinstance(value["comments"], list):
+ raise ValueError("comments must be an array")
+ for comment in value["comments"]:
+ if not isinstance(comment, dict) or set(comment) != {"path",
"position", "body"}:
+ raise ValueError("Each finding requires path, position and body")
+ if not isinstance(comment["path"], str) or not comment["path"]:
+ raise ValueError("Finding path is empty")
+ if type(comment["position"]) is not int or comment["position"] <= 0:
+ raise ValueError("Finding position must be a positive diff
position")
+ if not isinstance(comment["body"], str):
+ raise ValueError("Finding body must be text")
+ priority(comment["body"])
+ ids = value["existing_blocking_comment_ids"]
+ if not isinstance(ids, list) or any(type(i) is not int or i <= 0 for i in
ids):
+ raise ValueError("Existing blocking comment IDs must be positive
integers")
+ if len(set(ids)) != len(ids):
+ raise ValueError("Existing blocking comment IDs must be unique")
+ return value
+
+
+def api(path, remaining, *, payload=None, paginated=False):
+ command = ["gh", "api", path]
+ if paginated:
+ command += ["--paginate", "--slurp"]
+ if payload is not None:
+ command += ["--method", "POST", "--input", "-"]
+ result = subprocess.run(
+ command, input=json.dumps(payload) if payload is not None else None,
+ capture_output=True, text=True, check=True, timeout=min(30,
remaining()),
+ )
+ value = json.loads(result.stdout)
+ return [item for page in value for item in page] if paginated else value
+
+
+def check_target(run, remaining):
+ root = f"repos/{run['repository']}/pulls/{run['pr_number']}"
+ pr = api(root, remaining)
+ if (pr["state"] != "open" or pr["head"]["sha"] != run["head_sha"]
+ or pr["base"]["sha"] != run["base_sha"]):
+ raise ValueError("PR base/head or open state changed; final review is
stale")
+ return root
+
+
+def review_payload(submission, run, root, remaining):
+ counts = [0, 0, 0, 0]
+ for comment in submission["comments"]:
+ counts[priority(comment["body"])] += 1
+ # Previously reported findings are not posted again. The reviewer must
+ # explicitly reconfirm which existing P0/P1 findings still apply to this
head.
+ ids = submission["existing_blocking_comment_ids"]
+ if ids:
+ comments = {c["id"]: c for c in api(f"{root}/comments", remaining,
paginated=True)}
+ for comment_id in ids:
+ comment = comments.get(comment_id)
+ if comment is None or comment.get("in_reply_to_id") is not None:
+ raise ValueError(f"Existing blocking finding {comment_id} is
missing or a reply")
+ level = priority(comment["body"])
+ if level > 1:
+ raise ValueError(f"Existing finding {comment_id} is not P0/P1")
+ counts[level] += 1
+ # The marker binds the entire declared final submission to this invocation,
+ # including the deduplicated findings. A parallel run cannot satisfy it.
+ digest = hashlib.sha256(json.dumps(submission,
sort_keys=True).encode()).hexdigest()
+ marker = f"<!-- code-review-completion:{run['token']}:{digest} -->"
+ body = submission["body"]
+ if ids:
+ body += "\n\nExisting P0/P1 findings confirmed for this head: " + ",
".join(
+
f"https://github.com/{run['repository']}/pull/{run['pr_number']}#discussion_r{i}"
+ for i in ids
+ )
+ body += f"\n\n{marker}"
+ return {
+ "commit_id": run["head_sha"],
+ "event": "REQUEST_CHANGES" if counts[0] + counts[1] else "COMMENT",
+ "body": body,
+ "comments": submission["comments"],
+ }, counts, marker
+
+
+def verify_completion(context, run, remaining, *, submit=False,
submission=None):
+ """Never infer completion from an unowned review or a local result file."""
+ path = context / SUBMISSION_FILE
+ if submission is None:
+ if not path.exists():
+ raise ValueError("No final review submission was declared")
+ submission = json.loads(path.read_text())
+ submission = validate_submission(submission)
+ if path.exists() and json.loads(path.read_text()) != submission:
+ raise ValueError("Final submission already declared; refusing a
different review")
+ root = check_target(run, remaining)
+ payload, counts, marker = review_payload(submission, run, root, remaining)
+ reviews = api(f"{root}/reviews", remaining, paginated=True)
+ matching = [r for r in reviews if marker in (r.get("body") or "")]
+ if not matching and submit:
+ if any(r.get("user", {}).get("login") == "github-actions[bot]"
+ and r.get("commit_id") == run["head_sha"]
+ and (r.get("submitted_at") or "") >= run["started_at"] for r in
reviews):
+ raise ValueError("Another bot review was submitted during this
run; refusing duplicate submission")
+ if path.exists():
+ raise ValueError("Final submission was attempted but cannot be
verified; refusing to repost")
+ # Persist the complete intent before POST: if the response is lost, the
+ # runner can read back this exact review without repeating the write.
+ # Exclusive creation also prevents two accidental simultaneous helper
+ # invocations from both posting after their initial read found no
review.
+ with path.open("x") as handle:
+ json.dump(submission, handle)
+ api(f"{root}/reviews", remaining, payload=payload)
+ matching = [r for r in api(f"{root}/reviews", remaining,
paginated=True)
+ if marker in (r.get("body") or "")]
+ if len(matching) != 1:
+ raise ValueError("Could not verify exactly one final review from this
invocation")
+ review = matching[0]
+ expected_state = "CHANGES_REQUESTED" if payload["event"] ==
"REQUEST_CHANGES" else "COMMENTED"
+ if (review.get("user", {}).get("login") != "github-actions[bot]"
+ or review.get("commit_id") != run["head_sha"]
+ or (review.get("submitted_at") or "") < run["started_at"]
+ or review.get("state") != expected_state
+ or review.get("body") != payload["body"]):
+ raise ValueError("Final review author, head, timestamp, state or
summary did not match")
+ # Replies may arrive while the model wraps up. They are not part of the
+ # declared inline set and must not invalidate an otherwise complete review.
+ comments = [c for c in api(f"{root}/reviews/{review['id']}/comments",
remaining, paginated=True)
+ if c.get("in_reply_to_id") is None]
+ expected = Counter((c["path"], c["position"], c["body"]) for c in
payload["comments"])
+ actual = Counter((c["path"], c.get("position"), c["body"]) for c in
comments)
+ if actual != expected or any(
+ c.get("user", {}).get("login") != "github-actions[bot]"
+ or c.get("commit_id") != run["head_sha"] for c in comments
+ ):
+ raise ValueError("Final review inline comments were not completely
verified")
+ # Recheck after the API reads so a moving PR cannot pass an obsolete
review.
+ check_target(run, remaining)
+ return {
+ "state": "failure" if counts[0] + counts[1] else "success",
+ "p0": counts[0], "p1": counts[1], "review_id": review["id"],
+ "review_url": review["html_url"],
+ }
+
+
+def main():
+ parser = argparse.ArgumentParser(description=__doc__)
+ parser.add_argument("--context-dir", type=Path, required=True)
+ parser.add_argument("--input-file", type=Path, required=True)
+ args = parser.parse_args()
+ run = json.loads((args.context_dir / RUN_FILE).read_text())
+
+ def remaining():
+ seconds = run["deadline"] - time.monotonic()
+ if seconds <= 0:
+ raise TimeoutError("Review's shared time budget was exhausted")
+ return seconds
+
+ result = verify_completion(
+ args.context_dir, run, remaining, submit=True,
+ submission=json.loads(args.input_file.read_text()),
+ )
+ print(json.dumps(result))
+
+
+if __name__ == "__main__":
+ main()
diff --git a/.github/scripts/test_emit_litefuse_otel_io.py
b/.github/scripts/test_emit_litefuse_otel_io.py
index db2def32208..866b3c09ead 100644
--- a/.github/scripts/test_emit_litefuse_otel_io.py
+++ b/.github/scripts/test_emit_litefuse_otel_io.py
@@ -172,6 +172,26 @@ class LitefuseOtelExporterTest(unittest.TestCase):
["doris-ai-review", "codex-jsonl"],
)
+ def test_verified_completion_preserves_capacity_turn_for_diagnostics(self):
+ args = SimpleNamespace(
+ repository="apache/doris", workflow="Code Review", run_id="123",
pr_number="123",
+ head_sha="a" * 40, base_sha="b" * 40, reasoning_effort="xhigh",
+ max_json_chars=20000, max_context_json_chars=0,
trace_name="review",
+ session_id="123", environment="test", model="gpt-6-sol",
+ )
+ completion = {"state": "failure", "p0": 0, "p1": 1, "review_id": 99,
+ "recovered_after_capacity": True}
+ _, payload, _ = MODULE.build_ingestion_payload(args, "review", "", [
+ {"type": "turn.failed", "error": {"message": "capacity"}},
+ {"type": "review.completed", **completion},
+ ])
+ trace = payload["batch"][0]["body"]
+ self.assertEqual(completion, trace["metadata"]["review_completion"])
+ turn = next(event["body"] for event in payload["batch"]
+ if event["body"].get("name") == "codex.turn")
+ self.assertEqual("failed", turn["output"]["status"])
+ self.assertEqual("ERROR", turn["level"])
+
def test_bounds_and_shrinks_failed_turn_status_message(self):
args = SimpleNamespace(
repository="apache/doris",
diff --git a/.github/scripts/test_resolve_code_review_status.py
b/.github/scripts/test_resolve_code_review_status.py
index 113e7cca1bf..c9576ab5085 100644
--- a/.github/scripts/test_resolve_code_review_status.py
+++ b/.github/scripts/test_resolve_code_review_status.py
@@ -110,6 +110,33 @@ class ResolveCodeReviewStatusTest(unittest.TestCase):
result = resolve_status([], [], head_sha=HEAD_SHA)
self.assertEqual("pending", result.state)
+ def test_completed_blocking_review_is_failure(self) -> None:
+ result = resolve_status([pull(123)], [status(1, source="automated",
state="failure")], head_sha=HEAD_SHA)
+ self.assertEqual("failure", result.state)
+ self.assertIn("P0/P1", result.description)
+
+ def test_local_or_skip_success_can_override_blocking_review(self) -> None:
+ for source in ("local", "skip"):
+ with self.subTest(source=source):
+ result = resolve_status([pull(123)], [status(1,
source="automated", state="failure"),
+ status(2, source=source)], head_sha=HEAD_SHA)
+ self.assertEqual("success", result.state)
+
+ def test_stale_blocking_result_is_not_reused(self) -> None:
+ for pulls in ([pull(123, base=OTHER_BASE_SHA)], [pull(124)],
[pull(123, head=OTHER_HEAD_SHA)]):
+ with self.subTest(pulls=pulls):
+ result = resolve_status(pulls, [status(1, source="automated",
state="failure")], head_sha=HEAD_SHA)
+ self.assertEqual("pending", result.state)
+
+ def test_latest_automated_result_replaces_blocker(self) -> None:
+ result = resolve_status([pull(123)], [status(1, source="automated",
state="failure"),
+ status(2, source="automated")], head_sha=HEAD_SHA)
+ self.assertEqual("success", result.state)
+
+ def test_shared_head_blocker_takes_precedence_over_pending(self) -> None:
+ result = resolve_status([pull(123), pull(124)], [status(1,
source="automated", state="failure")], head_sha=HEAD_SHA)
+ self.assertEqual("failure", result.state)
+
def test_rejects_malformed_api_data(self) -> None:
with self.assertRaisesRegex(ResolutionError, "base SHA"):
resolve_status([pull(123, base="short")], [], head_sha=HEAD_SHA)
diff --git a/.github/scripts/test_review_auth_quarantine.py
b/.github/scripts/test_review_auth_quarantine.py
index cc9b509cba7..3d8e4e53e97 100755
--- a/.github/scripts/test_review_auth_quarantine.py
+++ b/.github/scripts/test_review_auth_quarantine.py
@@ -103,6 +103,13 @@ import os
from pathlib import Path
import sys
+if os.environ.get("FAKE_FINAL_REVIEW") == "1":
+ import json, subprocess
+ request = Path(os.environ["REVIEW_CONTEXT_DIR"], "final-input.json")
+ request.write_text(json.dumps({"body": "Complete review", "comments": [],
"existing_blocking_comment_ids": []}))
+ subprocess.run([sys.executable, str(Path(os.environ["RUNNER_TEMP"],
"submit_review.py")),
+ "--context-dir", os.environ["REVIEW_CONTEXT_DIR"],
"--input-file", str(request)],
+ check=True, stdout=subprocess.DEVNULL)
print('{"type":"thread.started","thread_id":"0199a213-81c0-7800-8aa1-bbab2a035a53"}')
print(os.environ.get("FAKE_CODEX_EVENTS", ""))
print(os.environ.get("FAKE_CODEX_STDERR", ""), file=sys.stderr, flush=True)
@@ -118,10 +125,31 @@ import os
from pathlib import Path
import sys
-if sys.argv[1] == "api" and
any("/contents/.github/scripts/run_review_with_resume.py?ref=" in arg for arg
in sys.argv):
- sys.stdout.write(Path(os.environ["FAKE_REVIEW_HELPER"]).read_text())
+helper = next((arg.split("/contents/.github/scripts/", 1)[1].split("?ref=",
1)[0]
+ for arg in sys.argv if "/contents/.github/scripts/" in arg),
None)
+if sys.argv[1] == "api" and helper:
+
sys.stdout.write(Path(os.environ["FAKE_REVIEW_HELPER"]).with_name(helper).read_text())
elif sys.argv[1] == "api":
- print(json.dumps([[{"submitted_at": "2099-01-01T00:00:00Z", "commit_id":
os.environ["HEAD_SHA"]}]]))
+ root = f"repos/{os.environ['REPO']}/pulls/{os.environ['PR_NUMBER']}"
+ path = sys.argv[2]
+ saved = Path(os.environ["REVIEW_CONTEXT_DIR"], "fake-review.json")
+ if path == root:
+ print(json.dumps({"state": "open", "head": {"sha":
os.environ["HEAD_SHA"]},
+ "base": {"sha": os.environ["BASE_SHA"]}}))
+ elif path == root + "/reviews":
+ if "POST" in sys.argv:
+ payload = json.load(sys.stdin)
+ review = {"id": 99, "body": payload["body"], "commit_id":
payload["commit_id"],
+ "user": {"login": "github-actions[bot]"}, "state":
"COMMENTED",
+ "submitted_at": "2099-01-01T00:00:00Z", "html_url":
"https://example.test/review/99"}
+ saved.write_text(json.dumps(review))
+ print(json.dumps(review))
+ else:
+ print(json.dumps([[json.loads(saved.read_text())] if
saved.exists() else []]))
+ elif path.endswith("/reviews/99/comments"):
+ print("[[]]")
+ else:
+ raise AssertionError(path)
else:
Path(os.environ["FAKE_COMMENT_FILE"]).write_text(sys.argv[-1])
'''
@@ -388,12 +416,35 @@ class ReviewAuthQuarantineTest(unittest.TestCase):
def test_success_is_not_quarantined_even_with_earlier_stderr_error(self):
_, outputs = self.run_step(
- "Run automated code review", FAKE_CODEX_STATUS="0",
+ "Run automated code review", FAKE_CODEX_STATUS="0",
FAKE_FINAL_REVIEW="1",
FAKE_CODEX_EVENTS=json.dumps({"type": "turn.completed", "usage":
{}}),
FAKE_CODEX_STDERR='{"code":"refresh_token_reused"}',
)
self.assertNotIn("auth_invalid_reason", outputs)
+ def
test_capacity_after_submission_is_success_without_auth_quarantine(self):
+ _, outputs = self.run_step(
+ "Run automated code review", FAKE_FINAL_REVIEW="1",
FAKE_CODEX_STATUS="1",
+ FAKE_CODEX_EVENTS=json.dumps({"type": "turn.failed", "error": {
+ "message": "Selected model is at capacity. Please try a
different model."
+ }}),
+ )
+ self.assertIn("review_state=success", outputs)
+ self.assertNotIn("auth_invalid_reason", outputs)
+ self.assertNotIn("failure_reason", outputs)
+
+ def test_sync_separates_execution_failure_from_blocking_findings(self):
+ # Exercise the real workflow shell; record POST args instead of
writing a status.
+ for execution, verdict, expected in (("success", "success", "success"),
+ ("success", "failure", "failure"), ("failure", "success",
"pending")):
+ with self.subTest(execution=execution, verdict=verdict):
+ result = subprocess.run(["bash", "-eo", "pipefail", "-c",
+ 'gh() { printf "%s\\n" "$@"; };\n' +
re.sub(r"\$\{\{.*?\}\}", "test", step_script("Sync Code Review check for
current head"))],
+ env={**self.env, "JOB_STATUS": execution,
"REVIEW_CONTEXT_OUTCOME": "success",
+ "REVIEW_OUTCOME": execution, "REVIEW_STATE": verdict,
"REVIEW_P0": "0", "REVIEW_P1": "1"},
+ capture_output=True, text=True, check=True)
+ self.assertIn(f"state={expected}", result.stdout)
+
def test_failure_comment_reports_marker_write_outcome(self):
for outcome in ("success", "failure"):
with self.subTest(outcome=outcome):
diff --git a/.github/scripts/test_run_review_with_resume.py
b/.github/scripts/test_run_review_with_resume.py
index 352f29b423c..ea06e64284e 100755
--- a/.github/scripts/test_run_review_with_resume.py
+++ b/.github/scripts/test_run_review_with_resume.py
@@ -99,6 +99,11 @@ class ResumeReviewTest(unittest.TestCase):
mock.patch.object(runner, "check_resume_target")
)
self.help = self.enterContext(mock.patch.object(runner.subprocess,
"run"))
+ self.verification = self.enterContext(mock.patch.object(
+ runner, "verify_completion", return_value={
+ "state": "success", "p0": 0, "p1": 0, "review_id": 123,
+ }
+ ))
def write_rollout(self, *, thread_id=THREAD, cwd=None):
(self.sessions / f"rollout-2026-09-07-{thread_id}.jsonl").write_text(
@@ -155,6 +160,30 @@ class ResumeReviewTest(unittest.TestCase):
self.target_check.assert_not_called()
self.assertEqual([], self.sleeps)
+ def test_completed_turn_without_verified_delivery_fails(self):
+ self.verification.side_effect = ValueError("No final review submission
was declared")
+ self.assertEqual(1, self.execute([{"events": [thread_event(),
completed()], "status": 0}]))
+ self.assertIn("No final review submission", self.last_error())
+ self.assertFalse((self.context / "review-result.json").exists())
+
+ def test_partial_submission_at_capacity_does_not_resume_or_pass(self):
+ (self.context / "review-submission.json").write_text("{}")
+ self.verification.side_effect = ValueError("Final review inline
comments were not completely verified")
+ self.assertEqual(1, self.execute([{"events": [thread_event(),
failed()]}]))
+ self.assertEqual([], self.sleeps)
+ self.target_check.assert_not_called()
+ self.assertFalse((self.context / "review-result.json").exists())
+
+ def test_only_capacity_can_recover_after_submission(self):
+ (self.context / "review-submission.json").write_text("{}")
+ self.assertEqual(1, self.execute([{"events": [thread_event(),
failed("Other failure")]}]))
+ self.verification.assert_not_called()
+
+ def test_cancellation_after_submission_is_still_failure(self):
+ (self.context / "review-submission.json").write_text("{}")
+ self.assertEqual(1, self.execute([{"events": [thread_event(),
failed()], "status": 143}]))
+ self.verification.assert_not_called()
+
def test_unsupported_model_falls_back_before_review_work(self):
self.args.model = "gpt-6-sol"
self.args.fallback_model = "gpt-5.6-sol"
@@ -768,9 +797,9 @@ HELPER
}
"""
for minutes, setup, expected in (
- (default_minutes, 12, 7068),
- ("150", 30, 8850),
- ("120", 7201, -121),
+ (default_minutes, 12, 7056),
+ ("150", 30, 8820),
+ ("120", 7201, -7322),
):
with (
self.subTest(minutes=minutes, setup=setup),
@@ -1097,6 +1126,9 @@ else:
),
mock.patch.object(runner, "RETRY_DELAYS", (0, 0, 0)),
mock.patch.object(runner, "check_resume_target"),
+ mock.patch.object(runner, "verify_completion", return_value={
+ "state": "success", "p0": 0, "p1": 0, "review_id": 123,
+ }),
):
self.assertEqual(0, runner.run_review(args))
self.assertEqual("2", (root / "request-count").read_text())
diff --git a/.github/scripts/test_submit_review.py
b/.github/scripts/test_submit_review.py
new file mode 100644
index 00000000000..f40ad82d05b
--- /dev/null
+++ b/.github/scripts/test_submit_review.py
@@ -0,0 +1,311 @@
+#!/usr/bin/env python3
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+import copy
+import io
+import json
+import subprocess
+import tempfile
+import threading
+import unittest
+from contextlib import redirect_stderr
+from concurrent.futures import ThreadPoolExecutor
+from pathlib import Path
+from types import SimpleNamespace
+from unittest import mock
+
+import run_review_with_resume as runner
+import submit_review as submitter
+from resolve_code_review_status import resolve_status
+
+
+class FinalReviewTest(unittest.TestCase):
+ def setUp(self):
+ temp = tempfile.TemporaryDirectory()
+ self.addCleanup(temp.cleanup)
+ self.context = Path(temp.name)
+ self.run = {
+ "repository": "apache/doris", "pr_number": "123", "head_sha": "a"
* 40,
+ "base_sha": "b" * 40, "token": "unique-run", "started_at":
"2026-09-29T00:00:00Z",
+ }
+ self.root = "repos/apache/doris/pulls/123"
+ self.pr = {"state": "open", "number": 123,
+ "head": {"sha": self.run["head_sha"]}, "base": {"sha":
self.run["base_sha"]}}
+ self.reviews = []
+ self.comments = []
+ self.existing = []
+ self.posts = []
+ self.api = self.enterContext(mock.patch.object(submitter, "api",
side_effect=self.fake_api))
+ self.remaining = lambda: 60
+
+ def submission(self, priority=1):
+ return {"body": "All checkpoints reviewed.", "comments": [] if
priority is None else [
+ {"path": "file.py", "position": 12, "body": f"[P{priority}] Fix
the verified issue"}
+ ], "existing_blocking_comment_ids": []}
+
+ def fake_api(self, path, remaining, *, payload=None, paginated=False):
+ if path == self.root:
+ return copy.deepcopy(self.pr)
+ if path == self.root + "/reviews":
+ if payload is None:
+ return copy.deepcopy(self.reviews)
+ self.posts.append(copy.deepcopy(payload))
+ self.reviews.append({
+ "id": 99, "user": {"login": "github-actions[bot]"},
+ "commit_id": payload["commit_id"], "submitted_at":
"9999-01-01T00:00:00Z",
+ "body": payload["body"], "html_url":
"https://github.com/apache/doris/pull/123#pullrequestreview-99";,
+ "state": "CHANGES_REQUESTED" if payload["event"] ==
"REQUEST_CHANGES" else "COMMENTED",
+ })
+ self.comments = [dict(c, user={"login": "github-actions[bot]"},
+ commit_id=payload["commit_id"]) for c in
payload["comments"]]
+ return copy.deepcopy(self.reviews[-1])
+ if path == self.root + "/reviews/99/comments":
+ return copy.deepcopy(self.comments)
+ if path == self.root + "/comments":
+ return copy.deepcopy(self.existing)
+ self.fail(f"Unexpected API path {path}")
+
+ def submit(self, submission=None):
+ return submitter.verify_completion(self.context, self.run,
self.remaining, submit=True,
+ submission=submission or
self.submission())
+
+ def verify(self):
+ return submitter.verify_completion(self.context, self.run,
self.remaining)
+
+ def test_submits_once_and_verifies_same_complete_review(self):
+ result = self.submit()
+ self.assertEqual("failure", result["state"])
+ self.assertEqual(1, result["p1"])
+ self.assertEqual(result, self.submit())
+ self.assertEqual(result, self.verify())
+ self.assertEqual(1, len(self.posts))
+ self.assertEqual(self.run["head_sha"], self.posts[0]["commit_id"])
+
+ def test_minor_or_empty_review_passes_without_requesting_changes(self):
+ for level in (None, 2, 3):
+ with self.subTest(priority=level):
+ self.reviews.clear()
+ (self.context /
submitter.SUBMISSION_FILE).unlink(missing_ok=True)
+ result = self.submit(self.submission(level))
+ self.assertEqual("success", result["state"])
+ self.assertEqual("COMMENT", self.posts[-1]["event"])
+
+ def
test_existing_confirmed_blocker_is_counted_without_duplicate_inline(self):
+ self.existing = [{"id": 101, "body": "[P0] Previously reported
blocker"}]
+ submission = self.submission(None)
+ submission["existing_blocking_comment_ids"] = [101]
+ result = self.submit(submission)
+ self.assertEqual((1, 0, "failure"), (result["p0"], result["p1"],
result["state"]))
+ self.assertEqual([], self.posts[0]["comments"])
+ self.assertIn("#discussion_r101", self.posts[0]["body"])
+
+ def test_invalid_existing_blocker_cannot_silently_pass(self):
+ submission = self.submission(None)
+ submission["existing_blocking_comment_ids"] = [101]
+ for comments in ([], [{"id": 101, "body": "[P2] Minor"}],
+ [{"id": 101, "body": "[P1] Reply", "in_reply_to_id":
1}]):
+ with self.subTest(comments=comments),
self.assertRaises(ValueError):
+ self.existing = comments
+ self.submit(submission)
+ self.assertEqual([], self.posts)
+
+ def test_missing_intent_cannot_use_other_reviews_or_result_file(self):
+ (self.context /
submitter.RESULT_FILE).write_text('{"state":"success"}')
+ with self.assertRaisesRegex(ValueError, "No final review submission"):
+ self.verify()
+ self.api.assert_not_called()
+
+ def
test_parallel_run_cannot_satisfy_completion_or_cause_duplicate_post(self):
+ self.submit()
+ self.run["token"] = "another-run"
+ with self.assertRaisesRegex(ValueError, "exactly one"):
+ self.verify()
+ with self.assertRaisesRegex(ValueError, "Another bot review"):
+ self.submit()
+ self.assertEqual(1, len(self.posts))
+
+ def test_simultaneous_helper_invocations_post_only_once(self):
+ barrier = threading.Barrier(2, timeout=5)
+ original_open = Path.open
+
+ def concurrent_open(path, mode="r", *args, **kwargs):
+ if path.name == submitter.SUBMISSION_FILE and mode == "x":
+ barrier.wait()
+ return original_open(path, mode, *args, **kwargs)
+
+ with mock.patch.object(Path, "open", concurrent_open),
ThreadPoolExecutor(2) as pool:
+ futures = [pool.submit(self.submit) for _ in range(2)]
+ outcomes = []
+ for future in futures:
+ try:
+ outcomes.append(future.result())
+ except FileExistsError:
+ outcomes.append(None)
+ self.assertEqual(1, sum(outcome is not None for outcome in outcomes))
+ self.assertEqual(1, len(self.posts))
+ self.assertEqual("failure", self.verify()["state"])
+
+ def test_reply_during_wrap_up_does_not_invalidate_final_inline_set(self):
+ self.submit()
+ self.comments.append({"in_reply_to_id": 1, "body": "Thanks", "path":
"file.py",
+ "user": {"login": "human"}})
+ self.assertEqual("failure", self.verify()["state"])
+
+ def test_frozen_submission_cannot_change_on_retry(self):
+ self.submit()
+ with self.assertRaisesRegex(ValueError, "different review"):
+ self.submit(self.submission(None))
+ self.assertEqual(1, len(self.posts))
+
+ def test_partial_or_modified_inline_delivery_fails(self):
+ self.submit()
+ original = copy.deepcopy(self.comments)
+ for comments in ([], original + original,
+ [dict(original[0], position=13)],
+ [dict(original[0], body="[P1] Different")],
+ [dict(original[0], user={"login": "other"})],
+ [dict(original[0], commit_id="c" * 40)]):
+ with self.subTest(comments=comments),
self.assertRaisesRegex(ValueError, "inline"):
+ self.comments = comments
+ self.verify()
+
+ def test_changed_review_identity_or_summary_cannot_pass(self):
+ self.submit()
+ original = copy.deepcopy(self.reviews[0])
+ for changes in ({"commit_id": "c" * 40}, {"user": {"login": "human"}},
+ {"submitted_at": "2020-01-01T00:00:00Z"}, {"state":
"DISMISSED"},
+ {"body": original["body"] + " changed"}):
+ with self.subTest(changes=changes),
self.assertRaisesRegex(ValueError, "did not match"):
+ self.reviews[0] = dict(original, **changes)
+ self.verify()
+
+ def test_pr_move_or_close_rejects_before_submission(self):
+ for change in ({"state": "closed"}, {"head": {"sha": "c" * 40}},
+ {"base": {"sha": "c" * 40}}):
+ original = copy.deepcopy(self.pr)
+ with self.subTest(change=change),
self.assertRaisesRegex(ValueError, "stale"):
+ self.pr.update(change)
+ self.submit()
+ self.pr = original
+ self.assertEqual([], self.posts)
+
+ def test_pr_move_during_readback_does_not_pass(self):
+ self.submit()
+ original_api = self.fake_api
+
+ def move_after_comments(path, *args, **kwargs):
+ result = original_api(path, *args, **kwargs)
+ if path.endswith("/reviews/99/comments"):
+ self.pr["head"]["sha"] = "c" * 40
+ return result
+
+ self.api.side_effect = move_after_comments
+ with self.assertRaisesRegex(ValueError, "stale"):
+ self.verify()
+
+ def test_lost_post_response_recovers_by_readback_without_duplicate(self):
+ original_api = self.fake_api
+
+ def lost_response(path, *args, **kwargs):
+ result = original_api(path, *args, **kwargs)
+ if kwargs.get("payload") is not None:
+ raise subprocess.TimeoutExpired("gh", 30)
+ return result
+
+ self.api.side_effect = lost_response
+ with self.assertRaises(subprocess.TimeoutExpired):
+ self.submit()
+ self.api.side_effect = original_api
+ self.assertEqual("failure", self.verify()["state"])
+ self.submit()
+ self.assertEqual(1, len(self.posts))
+
+ def
test_capacity_after_submission_completes_execution_and_gates_findings(self):
+ # Reproduce #66227: final write/readback, then capacity (also on the
last
+ # allowed attempt). Run the real verifier and SHA-wide status resolver.
+ for level in (None, 0, 1, 2):
+ with self.subTest(priority=level), tempfile.TemporaryDirectory()
as tmp:
+ context = Path(tmp)
+ (context / "codex_goal_prompt.txt").write_text("Review the PR")
+ args = SimpleNamespace(context_dir=context, cwd=context, **{
+ k: self.run[k] for k in ("repository", "pr_number",
"head_sha", "base_sha")
+ }, model="gpt-6-sol", effort="xhigh", budget_seconds=60)
+ self.reviews.clear()
+
+ def attempt(command, events, stderr, timeout, reaper=None):
+ run = json.loads((context /
submitter.RUN_FILE).read_text())
+ submitter.verify_completion(context, run, self.remaining,
submit=True,
+
submission=self.submission(level))
+ events.write_text(json.dumps({"type": "thread.started",
"thread_id":
+ "0199a213-81c0-7800-8aa1-bbab2a035a53"}) + "\n" +
json.dumps({
+ "type": "turn.failed", "error": {"message":
runner.CAPACITY_MESSAGE}}) + "\n")
+ stderr.write_text("")
+ return 1
+
+ with mock.patch.object(runner, "run_attempt",
side_effect=attempt), \
+ mock.patch.object(runner, "RETRY_DELAYS", ()), \
+ mock.patch.object(runner, "check_resume_target") as
guard, \
+ mock.patch.object(runner.time, "sleep") as sleep,
redirect_stderr(io.StringIO()):
+ self.assertEqual(0, runner.run_review(args))
+ guard.assert_not_called()
+ sleep.assert_not_called()
+ result = json.loads((context /
submitter.RESULT_FILE).read_text())
+ self.assertTrue(result["recovered_after_capacity"])
+ state = "failure" if level in (0, 1) else "success"
+ self.assertEqual(state, result["state"])
+ resolution = resolve_status([self.pr], [{"id": 1, "state":
result["state"],
+ "context":
f"code-review/source/automated/pr-123/base-{self.run['base_sha']}"}],
+ head_sha=self.run["head_sha"])
+ self.assertEqual(state, resolution.state)
+
+ def test_unconfirmed_post_is_never_repeated(self):
+ submission = self.submission()
+ submitter.write_json(self.context / submitter.SUBMISSION_FILE,
submission)
+ with self.assertRaisesRegex(ValueError, "refusing to repost"):
+ self.submit(submission)
+ self.assertEqual([], self.posts)
+
+ def test_api_unavailable_is_not_completion(self):
+ self.submit()
+ self.api.side_effect = subprocess.CalledProcessError(1, "gh")
+ with self.assertRaises(subprocess.CalledProcessError):
+ self.verify()
+
+ def test_schema_requires_explicit_severity_and_all_fields(self):
+ for value in ({"body": "summary"}, dict(self.submission(),
comments="no"),
+ dict(self.submission(),
existing_blocking_comment_ids=[1, 1]),
+ dict(self.submission(), comments=[{"path": "f",
"position": 1, "body": "Bug"}]),
+ dict(self.submission(), comments=[{"path": "f",
"position": 0, "body": "[P1] Bug"}])):
+ with self.subTest(value=value), self.assertRaises(ValueError):
+ self.submit(value)
+ self.assertEqual([], self.posts)
+
+
+class GitHubAPITest(unittest.TestCase):
+ def test_read_all_pages_and_bound_timeout(self):
+ with mock.patch.object(submitter.subprocess, "run",
return_value=SimpleNamespace(
+ stdout='[[{"id":1}], [{"id":2}]]'
+ )) as command:
+ self.assertEqual([{"id": 1}, {"id": 2}], submitter.api("reviews",
lambda: 7, paginated=True))
+ self.assertEqual(7, command.call_args.kwargs["timeout"])
+ self.assertIn("--paginate", command.call_args.args[0])
+ self.assertIn("--slurp", command.call_args.args[0])
+
+
+if __name__ == "__main__":
+ unittest.main()
diff --git a/.github/workflows/code-review-runner.yml
b/.github/workflows/code-review-runner.yml
index 55c4f69e0ed..099430613d5 100644
--- a/.github/workflows/code-review-runner.yml
+++ b/.github/workflows/code-review-runner.yml
@@ -739,15 +739,14 @@ jobs:
## Final response format
- After completing the review, you MUST provide a final summary
opinion based on the rules defined in AGENTS.md and the code-review skill. The
summary must include conclusions for each applicable critical checkpoint.
- - If the overall quality of PR is good and there are no critical
blocking issues (even if there are some tolerable minor issues), submit an
opinion on approval using: gh pr review PLACEHOLDER_PR_NUMBER --comment --body
"<summary>"
- - Note that when submitting review comments in this way, the
content will not be escaped, so you need to input multi-line text with line
breaks directly, rather than using `\n`.
- - If issues found, submit a review with inline comments plus a
comprehensive summary body. Use GitHub Reviews API to ensure comments are
inline:
- - Inline comment bodies may include GitHub suggested changes
blocks when you can propose a precise patch.
- - Prefer suggested changes for small, self-contained fixes (for
example typos, trivial refactors, or narrowly scoped code corrections).
- - Do not force suggested changes for broad, architectural, or
multi-file issues; explain those normally.
- - Build a JSON array of comments like: [{ "path": "<file>",
"position": <diff_position>, "body": "..." }]
- - Submit via: gh api
repos/PLACEHOLDER_REPO/pulls/PLACEHOLDER_PR_NUMBER/reviews --input <json_file>
- - The JSON file should contain:
{"event":"REQUEST_CHANGES","body":"<summary>","comments":[...]}
+ - After the required convergence rounds and final sweep, declare the
COMPLETE final review in a JSON file with exactly these fields:
+ {"body":"<comprehensive
summary>","comments":[{"path":"<file>","position":<diff_position>,"body":"[P1]
<finding title and explanation>"}],"existing_blocking_comment_ids":[]}
+ - Every new finding must be inline and start with [P0], [P1], [P2]
or [P3]. Suggested changes are welcome for precise, self-contained fixes.
+ - Include ALL accepted new findings in comments. Use an empty array
when there are none. In existing_blocking_comment_ids, list the numeric IDs of
every already-reported P0/P1 inline finding you independently confirmed still
applies to this head. Do not duplicate those findings. Explain their
disposition in the summary; use an empty array only if none still apply.
+ - Submit only after all review completion criteria are satisfied,
using the trusted helper: python3 "$RUNNER_TEMP/submit_review.py" --context-dir
PLACEHOLDER_CONTEXT_DIR --input-file <json_file>
+ - The helper pins the head, adds this invocation's completion
marker, submits the review and verifies the entire summary and inline comment
set. P0/P1 results request changes; other completed reviews are comments. This
is the ONLY final GitHub submission path. Do not submit reviews or inline
comments directly with gh.
+ - If the helper reports an error, inspect it and verify the outcome
before proceeding. Do not change or delete its review-run.json or
review-submission.json files. A retry with the identical JSON reads back a
submitted review instead of posting it again.
+ - Mark the goal complete only after the helper reports the verified
review ID. A completed review with P0/P1 is still a completed execution; the
workflow applies the separate blocking verdict.
PROMPT
sed -i "s|PLACEHOLDER_REPO|${REPO}|g"
"$REVIEW_CONTEXT_DIR/review_prompt.txt"
@@ -859,16 +858,17 @@ jobs:
HELPER_REF: ${{ github.workflow_sha || github.sha }}
run: |
review_step_started_at=$SECONDS
- review_started_at="$(date -u +%Y-%m-%dT%H:%M:%SZ)"
+ for script in run_review_with_resume.py submit_review.py; do
+ gh api \
+ -H "Accept: application/vnd.github.raw" \
+
"repos/${REPO}/contents/.github/scripts/${script}?ref=${HELPER_REF}" \
+ > "$RUNNER_TEMP/$script"
+ done
helper="$RUNNER_TEMP/run_review_with_resume.py"
- gh api \
- -H "Accept: application/vnd.github.raw" \
-
"repos/${REPO}/contents/.github/scripts/run_review_with_resume.py?ref=${HELPER_REF}"
\
- > "$helper"
# Share the step's budget; reserve two minutes for process cleanup
and
- # the existing GitHub review verification. Helper download counts
too.
+ # GitHub review verification. Both helper downloads count too.
budget_seconds=$((REVIEW_TIMEOUT_MINUTES * 60 - (SECONDS -
review_step_started_at) - 120))
set +e
# GitHub-hosted runners are ephemeral. Avoid workspace-write here
because
@@ -935,32 +935,6 @@ jobs:
fi
fi
- if [ -z "$failure_reason" ]; then
- reviews_file="$REVIEW_CONTEXT_DIR/pr_reviews_after_codex.json"
- reviews_api_ok=false
- review_verified=false
- for attempt in 1 2 3 4 5 6; do
- if gh api --paginate --slurp
"repos/${REPO}/pulls/${PR_NUMBER}/reviews" > "$reviews_file"; then
- reviews_api_ok=true
- if jq -e --arg started_at "$review_started_at" --arg head_sha
"$HEAD_SHA" '
- (add // [])
- | map(select((.submitted_at // "") >= $started_at and
(.commit_id // "") == $head_sha))
- | length > 0
- ' "$reviews_file" >/dev/null; then
- review_verified=true
- break
- fi
- fi
- sleep 5
- done
-
- if [ "$review_verified" != "true" ] && [ "$reviews_api_ok" !=
"true" ]; then
- failure_reason="Codex completed, but the workflow could not
verify pull request reviews through GitHub API."
- elif [ "$review_verified" != "true" ]; then
- failure_reason="Codex completed, but no new pull request review
was submitted for the current head SHA."
- fi
- fi
-
if [ -n "$failure_reason" ]; then
{
echo "failure_reason<<EOF"
@@ -970,6 +944,12 @@ jobs:
exit 1
fi
+ # Execution succeeded even when verified findings block the PR check.
+ # This result is written only after the runner's own API readback.
+ jq -er 'select(.state == "success" or .state == "failure") |
+ "review_state=\(.state)\np0=\(.p0)\np1=\(.p1)"' \
+ "$REVIEW_CONTEXT_DIR/review-result.json" >> "$GITHUB_OUTPUT"
+
- name: Record invalid Codex auth
id: auth_invalid_record
if: ${{ always() && steps.auth.outcome == 'success' &&
steps.review.outputs.auth_invalid_reason == 'refresh_token_reused' }}
@@ -1104,6 +1084,9 @@ jobs:
REPO: ${{ github.repository }}
REVIEW_CONTEXT_OUTCOME: ${{ steps.review_context.outcome }}
REVIEW_OUTCOME: ${{ steps.review.outcome }}
+ REVIEW_STATE: ${{ steps.review.outputs.review_state }}
+ REVIEW_P0: ${{ steps.review.outputs.p0 }}
+ REVIEW_P1: ${{ steps.review.outputs.p1 }}
run: |
state="pending"
summary="Automated review did not pass for PR #${PR_NUMBER} at
${BASE_SHA:0:12}."
@@ -1111,8 +1094,13 @@ jobs:
if [ "$JOB_STATUS" = "success" ] && \
[ "$REVIEW_CONTEXT_OUTCOME" = "success" ] && \
[ "$REVIEW_OUTCOME" = "success" ]; then
- state="success"
- summary="Automated review passed for PR #${PR_NUMBER} at
${BASE_SHA:0:12}."
+ case "$REVIEW_STATE" in
+ success|failure)
+ state="$REVIEW_STATE"
+ summary="Review completed for PR #${PR_NUMBER}:
P0=${REVIEW_P0}, P1=${REVIEW_P1}."
+ ;;
+ *) echo "::error::Missing verified review verdict"; exit 1 ;;
+ esac
fi
source_context="code-review/source/automated/pr-${PR_NUMBER}/base-${BASE_SHA}"
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
