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 a2638f38eac [fix](build) Preserve GitHub API failure diagnostics for
code review (#69386)
a2638f38eac is described below
commit a2638f38eacd4037a41d44a4e290cbeb2d13f393
Author: shuke <[email protected]>
AuthorDate: Thu Oct 22 10:44:22 2026 +0800
[fix](build) Preserve GitHub API failure diagnostics for code review
(#69386)
Problem Summary:
An empty or malformed JSON error response can make `gh api` fail with
only `unexpected end of JSON input`, hiding the HTTP status and GitHub
request ID. This prevents distinguishing transient server failures from
rejected requests in the review pipeline.
Add a shared diagnostic helper for review startup, status
synchronization and aggregation, helper downloads, submission, and head
polling. Each failure records HTTP status, allowlisted response headers,
bounded redacted response/output excerpts, exit code or timeout, and run
context. Failure artifacts are uploaded even before PR checkout or when
the caller handles the error. Load the helper from the immutable
workflow commit before the first API request; distinguish aggregate
artifacts by head SHA.
Outgoing request headers, bodies, and complete command arguments are
excluded. Credential values and fields are redacted before truncation.
Response excerpts depend on gh's debug format and are not complete wire
captures.
Normal stdout, existing request arguments, permissions, review verdicts,
submission deduplication, and retry behavior are preserved.
---
.github/scripts/review-github-diagnostics.md | 48 ++++
.github/scripts/review_github_api.py | 214 ++++++++++++++++
.github/scripts/run_review_with_resume.py | 8 +-
.github/scripts/submit_review.py | 17 +-
.github/scripts/test_prepare_review_agents.py | 1 +
.github/scripts/test_review_auth_quarantine.py | 4 +-
.../scripts/test_review_convergence_workflow.py | 5 +-
.github/scripts/test_review_github_api.py | 284 +++++++++++++++++++++
.github/scripts/test_run_review_with_resume.py | 14 +-
.github/scripts/test_submit_review.py | 8 +-
.github/workflows/code-review-aggregate-status.yml | 38 ++-
.github/workflows/code-review-runner.yml | 52 +++-
.github/workflows/code-review-sync-result.yml | 80 +++++-
13 files changed, 721 insertions(+), 52 deletions(-)
diff --git a/.github/scripts/review-github-diagnostics.md
b/.github/scripts/review-github-diagnostics.md
new file mode 100644
index 00000000000..3a22febb521
--- /dev/null
+++ b/.github/scripts/review-github-diagnostics.md
@@ -0,0 +1,48 @@
+# GitHub request diagnostics for code review
+
+The runner, status aggregator and result-sync jobs load review_github_api.py
+from the immutable workflow commit before their first GitHub API request. The
+copy in RUNNER_TEMP survives the subsequent PR checkout and is shared by the
+submission helper and head watchdog.
+
+Use python3 "$REVIEW_GH_HELPER" api ... in these workflow steps. Python helpers
+use run_gh with captured text output, check=True and their existing timeout;
+use decode_json when decoding its result. Request arguments, normal stdout,
+write deduplication, permission decisions and retry policy remain unchanged.
+
+On a failure, the helper prints a short diagnostic and saves one JSON record
+under REVIEW_GITHUB_DIAGNOSTICS_DIR. Records include the command exit status or
+timeout, the last HTTP method/path/status, GitHub request ID, rate-limit and
+retry headers, gh version, and bounded response/stdout/stderr excerpts. A
+successful gh exit followed by a Python JSON decode error is also recorded.
+A transport failure without a response has status: null; it is not inferred
+to be an HTTP error.
+
+The helper captures GH_DEBUG=api in memory because gh can discard a malformed
+error body and its HTTP status while parsing it. Outgoing headers, request
+bodies and argv are never archived. Response headers use an allowlist.
+Credential values and common credential fields are redacted before excerpts
+are truncated. The response body is gh's formatted debug excerpt, not exact
+wire bytes; gh may itself limit or omit it. Debug format changes may leave
+fields unavailable. Hard termination of the helper can prevent recording.
+
+Each failure uses a separate file so later cleanup errors do not overwrite it.
+An always() artifact step runs after cleanup, including when an API failure
+is caught by an optional step. Artifacts are named
+github-api-errors-<job>-<run>-<attempt>; reusable aggregation jobs include the
+head SHA to distinguish current-head and previous-head executions. Diagnostic
+upload/storage failures do not replace the original request failure.
+
+The existing failed-code-review-* submission artifacts remain available.
+These additions do not change the failure notification's content or posting
+arguments, or its existing failure classification.
+
+Local validation:
+
+~~~sh
+RUN_GH_HTTP_TESTS=1 python3 -m unittest discover -s .github/scripts -p
'test_*.py'
+~~~
+
+The opt-in HTTP test uses real gh only against a loopback server, with fake
+tokens and a temporary config; it never sends a request to GitHub. Linux-only
+process-supervision tests require a Linux host.
diff --git a/.github/scripts/review_github_api.py
b/.github/scripts/review_github_api.py
new file mode 100644
index 00000000000..fc2a014fa37
--- /dev/null
+++ b/.github/scripts/review_github_api.py
@@ -0,0 +1,214 @@
+#!/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.
+
+"""Capture gh HTTP failures without changing stdout, exit status, or retry
policy."""
+
+import json
+import os
+import re
+import subprocess
+import sys
+import time
+import uuid
+from datetime import datetime, timezone
+from pathlib import Path
+from urllib.parse import urlsplit
+
+LIMIT = 65536
+HEADERS = {
+ "content-type", "content-length", "date", "x-github-request-id",
+ "retry-after", "x-ratelimit-limit", "x-ratelimit-remaining",
+ "x-ratelimit-reset", "x-ratelimit-resource",
+}
+
+
+def redact(value):
+ if isinstance(value, bytes):
+ value = value.decode("utf-8", errors="replace")
+ value = value or ""
+ # Artifacts do not inherit Actions' secret masker. Never store the request
+ # headers or body; also redact credentials echoed by an error response.
+ for name, secret in os.environ.items():
+ if
re.search(r"(?:TOKEN|SECRET|PASSWORD|(?:^|_)KEY|(?:^|_)AK|(?:^|_)SK)$", name)
and secret:
+ for encoded in (secret, json.dumps(secret)[1:-1]):
+ value = value.replace(encoded, "[REDACTED]")
+ value = re.sub(r"\b(?:gh[pousr]_[A-Za-z0-9]+|github_pat_[A-Za-z0-9_]+)\b",
"[REDACTED]", value)
+ value = re.sub(
+
r'(?i)(["\']?(?:access_token|refresh_token|id_token|token|api_key|secret|'
+
r'client_secret|password|authorization|cookie|set-cookie)["\']?\s*[:=]\s*)'
+ r'("(?:\\.|[^"\\])*"|[^\r\n,}]+)', r'\1"[REDACTED]"', value,
+ )
+ value = re.sub(r"(https?://)[^/\s:@]+:[^/@\s]+@", r"\1[REDACTED]@", value)
+ value =
re.sub(r"(?i)([?&](?:token|access_token|signature|sig|key)=)[^&\s\"']+",
r"\1[REDACTED]", value)
+ return value
+
+
+def excerpt(value):
+ value = redact(value)
+ return value[:LIMIT] + ("\n[truncated]" if len(value) > LIMIT else "")
+
+
+def split_trace(stderr):
+ """Keep only the final HTTP exchange; discard every outgoing header/body.
+
+ GH_DEBUG=api writes HTTP diagnostics to stderr, independently of gh api's
+ response parser. Its body is a formatted, size-limited debug excerpt, not
+ a byte-for-byte copy. Unknown debug formatting is discarded, never dumped.
+ """
+ text = stderr.decode("utf-8", errors="replace") if isinstance(stderr,
bytes) else (stderr or "")
+ response = {"method": None, "path": None, "status": None, "headers": {},
+ "body_excerpt": "", "body_source": "gh_debug",
"request_count": 0}
+ ordinary = []
+ mode = "ordinary"
+ body = []
+ body_chars = 0
+ for line in text.splitlines():
+ if line.startswith("* Request at "):
+ count = response["request_count"] + 1
+ response = {**response, "method": None, "path": None, "status":
None,
+ "headers": {}, "body_excerpt": "", "request_count":
count}
+ body = []
+ body_chars = 0
+ response.pop("body_truncated", None)
+ mode = "request"
+ elif line.startswith("> "):
+ match = re.match(r"> ([A-Z]+) (\S+) HTTP/", line)
+ if match:
+ response["method"] = match[1]
+ response["path"] = redact(urlsplit(match[2]).path)
+ mode = "request"
+ elif re.match(r"< HTTP/\S+ \d{3}", line):
+ response["status"] = int(line.split()[2])
+ response["headers"] = {}
+ body = []
+ body_chars = 0
+ mode = "headers"
+ elif mode == "headers" and line.startswith("< "):
+ name, sep, value = line[2:].partition(":")
+ if sep and name.lower() in HEADERS:
+ response["headers"][name.lower()] = excerpt(value.strip())
+ elif line.startswith("* Request took "):
+ mode = "ordinary"
+ elif mode == "headers" and not line:
+ mode = "body"
+ elif mode == "body":
+ # Bound retained response content even if debug output grows.
+ line = redact(line) # Redact before truncation can split a secret.
+ if body_chars < LIMIT:
+ response["body_truncated"] = len(line) + body_chars > LIMIT
+ body.append(line[:LIMIT - body_chars])
+ body_chars += len(line) + 1
+ else:
+ response["body_truncated"] = True
+ elif mode == "ordinary" and not line.startswith("* "):
+ ordinary.append(line)
+ response["body_excerpt"] = excerpt("\n".join(body).strip())
+ if mode == "request":
+ # A transport failure has no response; do not mistake request data for
+ # its body. subprocess timeout/type still identifies the failure.
+ response["body_excerpt"] = ""
+ return response, excerpt("\n".join(ordinary).strip())
+
+
+def record_failure(command, error, stdout="", stderr="", *, diagnostic=None):
+ response, ordinary = split_trace(stderr) if diagnostic is None else
(diagnostic, excerpt(stderr))
+ record = {
+ "time": datetime.now(timezone.utc).isoformat(),
+ "run_id": os.environ.get("GITHUB_RUN_ID"),
+ "run_attempt": os.environ.get("GITHUB_RUN_ATTEMPT"),
+ "job": os.environ.get("GITHUB_JOB"),
+ "gh_version": os.environ.get("REVIEW_GH_VERSION"),
+ "error_type": type(error).__name__,
+ "returncode": getattr(error, "returncode", None),
+ "timeout_seconds": getattr(error, "timeout", None),
+ "http": response,
+ "stderr": ordinary,
+ "stdout_excerpt": excerpt(stdout),
+ }
+ # Do not record argv: -f/--body/--input can contain credentials or entire
+ # reviews. Endpoint metadata comes only from the outgoing request line.
+ summary = (
+ f"GitHub request failed: {response['method'] or 'unknown'}
{response['path'] or 'unknown'}; "
+ f"HTTP={response['status']};
request_id={response['headers'].get('x-github-request-id', 'unknown')}; "
+ f"{record['error_type']}; exit={record['returncode']};
timeout={record['timeout_seconds']}"
+ )
+ record["summary"] = redact(summary)
+ directory = os.environ.get("REVIEW_GITHUB_DIAGNOSTICS_DIR")
+ if directory:
+ try:
+ path = Path(directory)
+ path.mkdir(parents=True, exist_ok=True, mode=0o700)
+ data = json.dumps(record, ensure_ascii=True, indent=2) + "\n"
+ # Unique files support concurrent helper calls without append
races.
+ filename = path / f"{time.time_ns()}-{uuid.uuid4().hex}.json"
+ with open(filename, "x", opener=lambda p, f: os.open(p, f, 0o600))
as handle:
+ handle.write(data)
+ except OSError as exc:
+ print(f"Could not persist GitHub diagnostic
({type(exc).__name__})", file=sys.stderr)
+ print(record["summary"], file=sys.stderr, flush=True)
+ return ordinary, record
+
+
+def run_gh(command, **kwargs):
+ """subprocess.run contract for captured text gh calls, with failure
evidence."""
+ env = dict(kwargs.pop("env", os.environ))
+ env.update(GH_DEBUG="api", NO_COLOR="1")
+ try:
+ result = subprocess.run(command, env=env, **kwargs)
+ except (subprocess.CalledProcessError, subprocess.TimeoutExpired) as error:
+ ordinary, record = record_failure(command, error, error.stdout,
error.stderr)
+ error.stderr = ordinary + "\n" + record["summary"]
+ # Preserve the original return code/timeout and stdout for callers.
+ raise
+ except OSError as error:
+ record_failure(command, error)
+ raise
+ response, ordinary = split_trace(result.stderr)
+ result.stderr = ordinary
+ result.github_response = response
+ return result
+
+
+def decode_json(result, command):
+ try:
+ return json.loads(result.stdout)
+ except json.JSONDecodeError as error:
+ record_failure(command, error, result.stdout, result.stderr,
+ diagnostic=result.github_response)
+ raise
+
+
+def main():
+ command = ["gh", *sys.argv[1:]]
+ try:
+ result = run_gh(command, capture_output=True, text=True, check=True)
+ except subprocess.CalledProcessError as error:
+ sys.stdout.write(error.stdout or "")
+ print(error.stderr, file=sys.stderr)
+ return error.returncode if error.returncode >= 0 else 128 -
error.returncode
+ except OSError as error:
+ print(f"Could not execute gh ({type(error).__name__})",
file=sys.stderr)
+ return 1
+ sys.stdout.write(result.stdout)
+ if result.stderr:
+ print(result.stderr, file=sys.stderr)
+ return 0
+
+
+if __name__ == "__main__":
+ sys.exit(main())
diff --git a/.github/scripts/run_review_with_resume.py
b/.github/scripts/run_review_with_resume.py
index 7f8bbf1589d..e02c0d9dd78 100644
--- a/.github/scripts/run_review_with_resume.py
+++ b/.github/scripts/run_review_with_resume.py
@@ -33,6 +33,7 @@ import uuid
from datetime import datetime, timezone
from pathlib import Path
+from review_github_api import decode_json, run_gh
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.
@@ -78,11 +79,12 @@ class HeadWatchdog:
# Include API time in the existing shared budget and polling interval.
self.next_check = time.monotonic() + HEAD_POLL_SECONDS
try:
- response = subprocess.run(
- ["gh", "api",
f"repos/{self.args.repository}/pulls/{self.args.pr_number}"],
+ command = ["gh", "api",
f"repos/{self.args.repository}/pulls/{self.args.pr_number}"]
+ response = run_gh(
+ command,
check=True, capture_output=True, text=True, timeout=timeout,
)
- check_head(json.loads(response.stdout), self.args.head_sha)
+ check_head(decode_json(response, command), self.args.head_sha)
except (ValueError, OSError, subprocess.SubprocessError) as exc:
# An unavailable API is not evidence of a new commit. Strict target
# checks before resume/submission still apply independently.
diff --git a/.github/scripts/submit_review.py b/.github/scripts/submit_review.py
index dc48b1b8ed3..945f08e5158 100644
--- a/.github/scripts/submit_review.py
+++ b/.github/scripts/submit_review.py
@@ -30,6 +30,8 @@ from collections import Counter
from datetime import datetime, timezone
from pathlib import Path
+from review_github_api import decode_json, redact, run_gh
+
SUBMISSION_FILE = "review-submission.json"
REJECTION_FILE = "review-submission-rejected.json"
@@ -96,16 +98,7 @@ def validate_submission(value):
def diagnostic_text(value):
- if isinstance(value, bytes):
- value = value.decode("utf-8", errors="replace")
- value = value or ""
- # These are the credentials consumed by gh. Artifacts do not inherit the
- # GitHub Actions log masker, so remove them before persisting API output.
- for name in ("GH_TOKEN", "GITHUB_TOKEN", "GH_ENTERPRISE_TOKEN",
"GITHUB_ENTERPRISE_TOKEN"):
- token = os.environ.get(name)
- if token:
- value = value.replace(token, "[REDACTED]")
- value = re.sub(r"\b(?:gh[pousr]_[A-Za-z0-9]+|github_pat_[A-Za-z0-9_]+)\b",
"[REDACTED]", value)
+ value = redact(value)
return value[:65536] + ("\n[truncated after 65536 characters]" if
len(value) > 65536 else "")
@@ -135,14 +128,14 @@ def api(path, remaining, *, payload=None,
paginated=False):
command += ["--method", "POST", "--input", "-"]
method = "POST" if payload is not None else "GET"
try:
- result = subprocess.run(
+ result = run_gh(
command, input=json.dumps(payload) if payload is not None else
None,
capture_output=True, text=True, check=True, timeout=min(30,
remaining()),
)
except (subprocess.CalledProcessError, subprocess.TimeoutExpired) as error:
raise GitHubAPIError(path, method, error, error.stdout, error.stderr)
from None
try:
- value = json.loads(result.stdout)
+ value = decode_json(result, command)
except json.JSONDecodeError as error:
raise GitHubAPIError(path, method, error, result.stdout,
result.stderr) from None
return [item for page in value for item in page] if paginated else value
diff --git a/.github/scripts/test_prepare_review_agents.py
b/.github/scripts/test_prepare_review_agents.py
index 3c1be37a183..5ad7b8a2bbc 100644
--- a/.github/scripts/test_prepare_review_agents.py
+++ b/.github/scripts/test_prepare_review_agents.py
@@ -187,6 +187,7 @@ class ReviewGuidanceTest(unittest.TestCase):
sed.write_text('#!/bin/bash\nif [ "$1" = -i ]; then shift; exec
/usr/bin/sed -i "" "$@"; fi\nexec /usr/bin/sed "$@"\n')
sed.chmod(0o755)
env = {**os.environ, "PATH": f"{bin_dir}:{os.environ['PATH']}",
+ "REVIEW_GH_HELPER": str(SCRIPTS / "review_github_api.py"),
"REPO": "apache/doris", "PR_NUMBER": "66227", "HEAD_SHA":
self.head,
"BASE_SHA": self.base, "HELPER_REF": self.trusted,
"REVIEW_CONTEXT_DIR": str(self.context), "REVIEW_CONTEXT_REL":
self.context.name,
diff --git a/.github/scripts/test_review_auth_quarantine.py
b/.github/scripts/test_review_auth_quarantine.py
index 3d8e4e53e97..d458b6dbf3b 100755
--- a/.github/scripts/test_review_auth_quarantine.py
+++ b/.github/scripts/test_review_auth_quarantine.py
@@ -183,12 +183,14 @@ class ReviewAuthQuarantineTest(unittest.TestCase):
"REPO": "example/repo", "PR_NUMBER": "1", "HEAD_SHA": "a" * 40,
"GITHUB_WORKSPACE": str(self.root),
"BASE_SHA": "b" * 40, "HELPER_REF": "test-pin",
"REVIEW_TIMEOUT_MINUTES": "120",
+ "REVIEW_GH_HELPER":
str(Path(__file__).resolve().with_name("review_github_api.py")),
"FAKE_REVIEW_HELPER":
str(Path(__file__).resolve().with_name("run_review_with_resume.py")),
})
self.new_runner()
def new_runner(self):
runner = Path(tempfile.mkdtemp(dir=self.root))
+ shutil.copyfile(self.env["REVIEW_GH_HELPER"], runner /
"review_github_api.py")
context = runner / "review-context"
context.mkdir()
(context / "codex_goal_prompt.txt").write_text("Fake review")
@@ -439,7 +441,7 @@ class ReviewAuthQuarantineTest(unittest.TestCase):
("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"))],
+ 'python3() { shift; 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)
diff --git a/.github/scripts/test_review_convergence_workflow.py
b/.github/scripts/test_review_convergence_workflow.py
index 97953d01b77..88620f1c453 100644
--- a/.github/scripts/test_review_convergence_workflow.py
+++ b/.github/scripts/test_review_convergence_workflow.py
@@ -67,8 +67,9 @@ class ReviewConvergenceWorkflowTest(unittest.TestCase):
source_script = step_script("Sync Code Review check for current head")
# Replace the only external mutation with an argument capture. Run the
# actual Bash/jq blocks, then feed their source into the real resolver.
- capture = """gh() {
- python3 -c 'import json, sys; print(json.dumps(sys.argv[1:]))' "$@"
+ capture = """python3() {
+ shift
+ command python3 -c 'import json, sys; print(json.dumps(sys.argv[1:]))' "$@"
}
"""
for converged, p0, p1, expected in (
diff --git a/.github/scripts/test_review_github_api.py
b/.github/scripts/test_review_github_api.py
new file mode 100644
index 00000000000..1836e491773
--- /dev/null
+++ b/.github/scripts/test_review_github_api.py
@@ -0,0 +1,284 @@
+#!/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 http.server
+import io
+import json
+import os
+import re
+import shutil
+import subprocess
+import sys
+import threading
+import tempfile
+import unittest
+from contextlib import redirect_stderr
+from pathlib import Path
+from unittest import mock
+
+import review_github_api as diagnostic
+
+
+def trace(status=502, body='{"message":',
path="/repos/apache/doris/statuses/abc"):
+ return (
+ "* Request at 2026-10-21\n"
+ "* Request to https://api.github.com"; + path + "\n"
+ "> POST " + path + " HTTP/2.0\n"
+ "> Authorization: token request-secret\n"
+ "> Cookie: request-cookie\n\n"
+ '{"private_request":"DO_NOT_PERSIST_REQUEST_PAYLOAD"}\n\n'
+ f"< HTTP/2.0 {status} Example\n"
+ "< Content-Type: application/json\n"
+ "< X-Github-Request-Id: REQUEST-123\n"
+ "< X-Ratelimit-Remaining: 0\n"
+ "< Retry-After: 10\n"
+ "< Set-Cookie: response-cookie\n"
+ "< Authorization: Bearer response-secret\n\n"
+ + body + "\n* Request took 1ms\n"
+ )
+
+
+class GitHubDiagnosticsTest(unittest.TestCase):
+ def setUp(self):
+ temp = tempfile.TemporaryDirectory()
+ self.addCleanup(temp.cleanup)
+ self.directory = Path(temp.name)
+ self.env = self.enterContext(mock.patch.dict(os.environ, {
+ "REVIEW_GITHUB_DIAGNOSTICS_DIR": temp.name,
+ "GH_TOKEN": "env-github-secret",
+ "LANGFUSE_SECRET_KEY": "env-telemetry-secret",
+ }))
+ self.logs = self.enterContext(redirect_stderr(io.StringIO()))
+ self.command = ["gh", "api", "repos/apache/doris/statuses/abc",
+ "-X", "POST", "-f", "sensitive=COMMAND_SECRET"]
+
+ def records(self):
+ return [json.loads(p.read_text()) for p in
sorted(self.directory.glob("*.json"))]
+
+ def test_malformed_http_error_retains_status_headers_body_and_exit(self):
+ error = subprocess.CalledProcessError(1, self.command, "", trace() +
"unexpected end of JSON input\n")
+ with mock.patch.object(diagnostic.subprocess, "run",
side_effect=error) as run:
+ with self.assertRaises(subprocess.CalledProcessError) as result:
+ diagnostic.run_gh(self.command, capture_output=True,
text=True, check=True, timeout=10)
+ self.assertIs(error, result.exception)
+ self.assertEqual(1, run.call_count) # Never replay a write for
diagnostics.
+ self.assertEqual("api", run.call_args.kwargs["env"]["GH_DEBUG"])
+ record, = self.records()
+ self.assertEqual(502, record["http"]["status"])
+ self.assertEqual('{"message":', record["http"]["body_excerpt"])
+ self.assertEqual("REQUEST-123",
record["http"]["headers"]["x-github-request-id"])
+ self.assertEqual("10", record["http"]["headers"]["retry-after"])
+ self.assertEqual(1, record["returncode"])
+ self.assertIn("unexpected end of JSON input", error.stderr)
+ data = json.dumps(record)
+ for secret in ("COMMAND_SECRET", "request-secret", "request-cookie",
+ "response-cookie", "response-secret",
"DO_NOT_PERSIST_REQUEST_PAYLOAD"):
+ self.assertNotIn(secret, data)
+ self.assertEqual(0o600,
next(self.directory.glob("*.json")).stat().st_mode & 0o777)
+
+ def
test_success_stdout_and_pagination_are_unchanged_and_not_archived(self):
+ output = '[[{"id":1}],[{"id":2}]]\n'
+ response = subprocess.CompletedProcess(self.command, 0, output,
trace(200, '{"id":1}')+trace(200, '{"id":2}'))
+ with mock.patch.object(diagnostic.subprocess, "run",
return_value=response):
+ actual = diagnostic.run_gh(self.command, check=True,
capture_output=True, text=True)
+ self.assertEqual(output, actual.stdout)
+ self.assertEqual("", actual.stderr)
+ self.assertEqual(2, actual.github_response["request_count"])
+ self.assertEqual([], self.records())
+ self.assertEqual("", self.logs.getvalue())
+
+ def test_later_page_failure_has_last_request_not_previous_body(self):
+ stderr = trace(200, '{"previous":"large successful body"}') +
trace(503, "")
+ error = subprocess.CalledProcessError(1, self.command, "partial
stdout", stderr+"unexpected end of JSON input")
+ with mock.patch.object(diagnostic.subprocess, "run",
side_effect=error):
+ with self.assertRaises(subprocess.CalledProcessError):
+ diagnostic.run_gh(self.command, check=True)
+ http = self.records()[0]["http"]
+ self.assertEqual((503, 2, ""), (http["status"], http["request_count"],
http["body_excerpt"]))
+
+ def test_timeout_keeps_partial_response_and_does_not_retry(self):
+ error = subprocess.TimeoutExpired(self.command, 10, output=b"",
stderr=trace(502).encode())
+ with mock.patch.object(diagnostic.subprocess, "run",
side_effect=error) as run:
+ with self.assertRaises(subprocess.TimeoutExpired):
+ diagnostic.run_gh(self.command, check=True, timeout=10)
+ record, = self.records()
+ self.assertEqual(10, record["timeout_seconds"])
+ self.assertEqual(502, record["http"]["status"])
+ self.assertEqual(1, run.call_count)
+
+ def
test_transport_failure_without_response_does_not_claim_http_status(self):
+ stderr = trace().split("< HTTP/",1)[0] + "* Request took 1ms\nPost:
connection reset by peer\n"
+ error = subprocess.CalledProcessError(1, self.command, "", stderr)
+ with mock.patch.object(diagnostic.subprocess, "run",
side_effect=error):
+ with self.assertRaises(subprocess.CalledProcessError):
+ diagnostic.run_gh(self.command, check=True)
+ record, = self.records()
+ self.assertIsNone(record["http"]["status"])
+ self.assertEqual("", record["http"]["body_excerpt"])
+ self.assertIn("connection reset", record["stderr"])
+
+ def
test_json_decode_failure_keeps_success_status_and_raw_output_excerpt(self):
+ response = subprocess.CompletedProcess(self.command, 0, "{",
trace(201, "{"))
+ with mock.patch.object(diagnostic.subprocess, "run",
return_value=response):
+ result = diagnostic.run_gh(self.command, check=True)
+ with self.assertRaises(json.JSONDecodeError):
+ diagnostic.decode_json(result, self.command)
+ record, = self.records()
+ self.assertEqual(201, record["http"]["status"])
+ self.assertEqual("JSONDecodeError", record["error_type"])
+ self.assertEqual("{", record["stdout_excerpt"])
+
+ def test_credential_echoes_are_redacted_in_artifacts_and_summary(self):
+ body =
'{"access_token":"not-in-env","password":"another-secret","message":"env-github-secret
env-telemetry-secret ghp_ExampleToken"}'
+ error = subprocess.CalledProcessError(1, self.command, body,
trace(403,body)+"env-github-secret")
+ with mock.patch.object(diagnostic.subprocess, "run",
side_effect=error):
+ with self.assertRaises(subprocess.CalledProcessError):
+ diagnostic.run_gh(self.command, check=True)
+ data = json.dumps(self.records()) + self.logs.getvalue()
+ for secret in
("not-in-env","another-secret","env-github-secret","env-telemetry-secret","ghp_ExampleToken"):
+ self.assertNotIn(secret, data)
+
+ def test_excerpt_is_bounded_and_paths_drop_sensitive_queries(self):
+ http, _ = diagnostic.split_trace(trace(500, "x"*100000,
"/repos/a/b?token=secret"))
+ self.assertEqual("/repos/a/b", http["path"])
+ self.assertLessEqual(len(http["body_excerpt"]), diagnostic.LIMIT + 20)
+
+ def test_later_failure_cannot_overwrite_earlier_evidence(self):
+ for status in (502,403):
+ error = subprocess.CalledProcessError(1, self.command, "",
trace(status))
+ with mock.patch.object(diagnostic.subprocess, "run",
side_effect=error):
+ with self.assertRaises(subprocess.CalledProcessError):
+ diagnostic.run_gh(self.command, check=True)
+ self.assertEqual([502,403], [r["http"]["status"] for r in
self.records()])
+
+ def test_artifact_write_failure_does_not_replace_original_error(self):
+ error = subprocess.CalledProcessError(7, self.command, "", trace())
+ with mock.patch.object(diagnostic.subprocess, "run",
side_effect=error), \
+ mock.patch.object(Path, "mkdir", side_effect=PermissionError):
+ with self.assertRaises(subprocess.CalledProcessError) as result:
+ diagnostic.run_gh(self.command, check=True)
+ self.assertEqual(7, result.exception.returncode)
+ self.assertIn("Could not persist", self.logs.getvalue())
+
+ def test_unknown_trace_format_is_not_dumped_as_request_data(self):
+ http, ordinary = diagnostic.split_trace("* Request at now\nSECRET
REQUEST CONTENT\n")
+ self.assertEqual("", http["body_excerpt"])
+ self.assertEqual("", ordinary)
+
+
+ def test_redaction_precedes_body_truncation(self):
+ prefix = "x" * (diagnostic.LIMIT - 4)
+ http, _ = diagnostic.split_trace(trace(500, prefix +
"env-github-secret"))
+ self.assertNotIn("env-", http["body_excerpt"])
+
+ def test_json_escaped_credentials_are_fully_redacted(self):
+ raw = json.dumps({"password": 'a"b,secret', "id_token": "ID_SECRET",
"api_key": "KEY_SECRET"})
+ output = diagnostic.redact(raw)
+ for secret in ("secret", "ID_SECRET", "KEY_SECRET"):
+ self.assertNotIn(secret, output)
+ json.loads(output)
+
+ def test_cli_signal_exit_uses_shell_convention(self):
+ error = subprocess.CalledProcessError(-15, self.command, "",
"terminated")
+ with mock.patch.object(diagnostic, "run_gh", side_effect=error):
+ self.assertEqual(143, diagnostic.main())
+
+
[email protected](os.environ.get("RUN_GH_HTTP_TESTS") == "1" and
shutil.which("gh"),
+ "opt-in real gh loopback test")
+class GitHubHTTPLoopbackTest(unittest.TestCase):
+ def test_real_cli_failures_keep_http_evidence_without_reposting(self):
+ requests = []
+ class Handler(http.server.BaseHTTPRequestHandler):
+ def log_message(self, *_):
+ pass
+
+ def do_POST(self):
+
requests.append(json.loads(self.rfile.read(int(self.headers["Content-Length"]))))
+ status = int(self.path.rsplit("/", 1)[1])
+ body = b'{"id":1}' if status == 201 else b''
+ self.send_response(status)
+ self.send_header("Content-Type", "application/json")
+ self.send_header("Content-Length", str(len(body)))
+ self.send_header("X-GitHub-Request-Id", "loopback-id")
+ self.send_header("Retry-After", "12")
+ self.end_headers()
+ self.wfile.write(body)
+
+ server = http.server.ThreadingHTTPServer(("127.0.0.1", 0), Handler)
+ thread = threading.Thread(target=server.serve_forever, daemon=True)
+ thread.start()
+ try:
+ with tempfile.TemporaryDirectory() as tmp:
+ env = {"PATH": os.environ["PATH"], "GH_TOKEN": "fake-only",
+ "GH_ENTERPRISE_TOKEN": "fake-only", "GH_CONFIG_DIR":
tmp,
+ "GH_NO_UPDATE_NOTIFIER": "1", "GH_PROMPT_DISABLED": "1",
+ "REVIEW_GITHUB_DIAGNOSTICS_DIR": tmp}
+ helper = Path(__file__).with_name("review_github_api.py")
+ for status in (201,422,502,503):
+ result = subprocess.run([
+ sys.executable, str(helper), "api",
+
f"http://127.0.0.1:{server.server_port}/statuses/{status}";,
+ "-X", "POST", "-f", "state=pending",
+ ], env=env, capture_output=True, text=True, timeout=10)
+ self.assertEqual(0 if status == 201 else 1,
result.returncode)
+ if status == 201:
+ self.assertEqual('{"id":1}', result.stdout)
+ self.assertEqual("", result.stderr)
+ else:
+ self.assertEqual("", result.stdout)
+ self.assertIn("unexpected end of JSON input",
result.stderr)
+ self.assertIn(f"HTTP={status}", result.stderr)
+ records = [json.loads(p.read_text()) for p in
sorted(Path(tmp).glob("*.json"))]
+ self.assertEqual([422,502,503], [r["http"]["status"] for r in
records])
+
self.assertTrue(all(r["http"]["headers"]["x-github-request-id"] ==
"loopback-id" for r in records))
+ self.assertEqual([{"state":"pending"}]*4, requests)
+ self.assertNotIn("fake-only", json.dumps(records))
+ finally:
+ server.shutdown()
+ server.server_close()
+ thread.join()
+
+
+class WorkflowDiagnosticsWiringTest(unittest.TestCase):
+ def test_every_job_installs_before_api_and_uploads_after_cleanup(self):
+ workflows = Path(__file__).resolve().parents[1] / "workflows"
+ for filename, jobs in (
+ ("code-review-runner.yml", ["code-review"]),
+ ("code-review-aggregate-status.yml", ["aggregate"]),
+ ("code-review-sync-result.yml", ["accept-skill-review",
"skip-on-comment"]),
+ ):
+ text = (workflows / filename).read_text()
+ for job in jobs:
+ block = text.split("\n " + job + ":", 1)[1]
+ block = re.split(r"\n [\w-]+:", block, maxsplit=1)[0]
+ self.assertLess(block.index("Install GitHub API diagnostics"),
+ block.index('python3 "$REVIEW_GH_HELPER"'))
+ upload = block.split(" - name: Preserve GitHub API
failure diagnostics",1)[1]
+ self.assertIn("if: always()", upload)
+ self.assertIn("continue-on-error: true", upload)
+ self.assertIn("review-github-diagnostics/*.json", upload)
+ self.assertNotIn(" - name:", upload)
+ self.assertNotIn("gh api ", block)
+ aggregate = (workflows /
"code-review-aggregate-status.yml").read_text()
+ # Two reusable aggregate jobs can run for current and previous heads.
+ self.assertIn("github-api-errors-aggregate-${{ inputs.head_sha }}-",
aggregate)
+
+
+if __name__ == "__main__":
+ unittest.main()
diff --git a/.github/scripts/test_run_review_with_resume.py
b/.github/scripts/test_run_review_with_resume.py
index 6651d65b145..313a740129e 100644
--- a/.github/scripts/test_run_review_with_resume.py
+++ b/.github/scripts/test_run_review_with_resume.py
@@ -814,7 +814,7 @@ class HeadWatchdogTest(unittest.TestCase):
self.enterContext(mock.patch.object(runner.time, "monotonic",
side_effect=lambda: self.clock))
self.sleep = self.enterContext(mock.patch.object(runner.time, "sleep",
side_effect=self.advance))
self.api = self.enterContext(mock.patch.object(runner.subprocess,
"run"))
- self.api.return_value = SimpleNamespace(stdout=json.dumps({"head":
{"sha": "a" * 40}}))
+ self.api.return_value = SimpleNamespace(stderr="",
stdout=json.dumps({"head": {"sha": "a" * 40}}))
self.stderr = self.enterContext(redirect_stderr(io.StringIO()))
self.watchdog = runner.HeadWatchdog(self.args, lambda: 1000 -
self.clock)
@@ -873,8 +873,8 @@ class HeadWatchdogTest(unittest.TestCase):
def test_backoff_polls_and_stops_before_another_model_request(self):
self.api.side_effect = [
- SimpleNamespace(stdout=json.dumps({"head": {"sha": "a" * 40}})),
- SimpleNamespace(stdout=json.dumps({"head": {"sha": "c" * 40}})),
+ SimpleNamespace(stderr="", stdout=json.dumps({"head": {"sha": "a"
* 40}})),
+ SimpleNamespace(stderr="", stdout=json.dumps({"head": {"sha": "c"
* 40}})),
]
with self.assertRaises(runner.ReviewSuperseded):
self.watchdog.wait(300)
@@ -883,7 +883,7 @@ class HeadWatchdogTest(unittest.TestCase):
def test_backoff_api_time_counts_toward_wait(self):
def slow_api(*args, **kwargs):
self.clock += 5
- return SimpleNamespace(stdout=json.dumps({"head": {"sha": "a" *
40}}))
+ return SimpleNamespace(stderr="", stdout=json.dumps({"head":
{"sha": "a" * 40}}))
self.api.side_effect = slow_api
self.watchdog.wait(125)
self.assertEqual(125, self.clock)
@@ -1017,7 +1017,9 @@ class WorkflowBudgetTest(unittest.TestCase):
)
# A shell gh stub writes the downloaded helper and advances Bash's
elapsed
# clock without waiting minutes. The helper reports the actual CLI
args.
- gh = """gh() {
+ gh = """python3() {
+ if [ "$1" != "$REVIEW_GH_HELPER" ]; then command python3 "$@"; return; fi
+ shift
SECONDS=$((SECONDS + SETUP_SECONDS))
cat <<'HELPER'
import json, sys
@@ -1081,7 +1083,7 @@ class ResumeTargetTest(unittest.TestCase):
def check(self, pages):
def api(command, **kwargs):
data = pages if "--paginate" in command else self.pr
- return SimpleNamespace(stdout=json.dumps(data))
+ return SimpleNamespace(stderr="", stdout=json.dumps(data))
with mock.patch.object(runner.subprocess, "run", side_effect=api):
runner.check_resume_target(self.args, "2026-09-07T11:00:00Z",
lambda: 500)
diff --git a/.github/scripts/test_submit_review.py
b/.github/scripts/test_submit_review.py
index 2d1bdc4fb27..9d52ae05907 100644
--- a/.github/scripts/test_submit_review.py
+++ b/.github/scripts/test_submit_review.py
@@ -166,8 +166,10 @@ class FinalReviewTest(unittest.TestCase):
self.assertEqual("failure", self.verify()["state"])
self.assertEqual(1, len(self.posts))
diagnostic = json.loads((self.context /
submitter.API_ERRORS_FILE).read_text())
- self.assertEqual((30, "partial response", "timed out"),
- (diagnostic["timeout_seconds"], diagnostic["stdout"],
diagnostic["stderr"]))
+ self.assertEqual((30, "partial response"),
+ (diagnostic["timeout_seconds"], diagnostic["stdout"]))
+ self.assertTrue(diagnostic["stderr"].startswith("timed out"))
+ self.assertIn("timeout=30", diagnostic["stderr"])
def test_runner_failure_keeps_original_post_error(self):
(self.context / "codex_goal_prompt.txt").write_text("Review the PR")
@@ -626,7 +628,7 @@ class ReviewWorkflowTest(unittest.TestCase):
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}]]'
+ stderr="", 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"])
diff --git a/.github/workflows/code-review-aggregate-status.yml
b/.github/workflows/code-review-aggregate-status.yml
index 89812d76e31..f0524d064ca 100644
--- a/.github/workflows/code-review-aggregate-status.yml
+++ b/.github/workflows/code-review-aggregate-status.yml
@@ -21,13 +21,33 @@ jobs:
pull-requests: read
statuses: write
steps:
+ # Keep the trusted helper outside the PR checkout; capture the first API
failure too.
+ - name: Checkout trusted GitHub API diagnostics
+ timeout-minutes: 2
+ uses: actions/checkout@v4
+ with:
+ ref: ${{ github.workflow_sha || github.sha }}
+ persist-credentials: false
+ path: .review-api-source
+ sparse-checkout: .github/scripts/review_github_api.py
+ sparse-checkout-cone-mode: false
+
+ - name: Install GitHub API diagnostics
+ timeout-minutes: 1
+ run: |
+ install -m 700
.review-api-source/.github/scripts/review_github_api.py
"$RUNNER_TEMP/review_github_api.py"
+ gh --version
+ printf 'REVIEW_GH_VERSION=%s\n' "$(gh --version | sed -n '1p')" >>
"$GITHUB_ENV"
+ printf 'REVIEW_GH_HELPER=%s\n' "$RUNNER_TEMP/review_github_api.py"
>> "$GITHUB_ENV"
+ printf 'REVIEW_GITHUB_DIAGNOSTICS_DIR=%s\n'
"$RUNNER_TEMP/review-github-diagnostics" >> "$GITHUB_ENV"
+
- name: Mark aggregate status as pending
env:
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
HEAD_SHA: ${{ inputs.head_sha }}
REPO: ${{ github.repository }}
run: |
- gh api "repos/${REPO}/statuses/${HEAD_SHA}" \
+ python3 "$REVIEW_GH_HELPER" api "repos/${REPO}/statuses/${HEAD_SHA}"
\
-X POST \
-f state='pending' \
-f context='code-review' \
@@ -54,12 +74,12 @@ jobs:
status_pages="$RUNNER_TEMP/code-review-status-pages.json"
statuses="$RUNNER_TEMP/code-review-statuses.json"
- gh api --paginate --slurp \
+ python3 "$REVIEW_GH_HELPER" api --paginate --slurp \
-H 'Accept: application/vnd.github+json' \
"repos/${REPO}/pulls?state=open&per_page=100" > "$pull_pages"
jq '[.[][]]' "$pull_pages" > "$pulls"
- gh api --paginate --slurp \
+ python3 "$REVIEW_GH_HELPER" api --paginate --slurp \
-H 'Accept: application/vnd.github+json' \
"repos/${REPO}/commits/${HEAD_SHA}/statuses?per_page=100" >
"$status_pages"
jq '[.[][]]' "$status_pages" > "$statuses"
@@ -77,9 +97,19 @@ jobs:
REPO: ${{ github.repository }}
STATE: ${{ steps.resolution.outputs.state }}
run: |
- gh api "repos/${REPO}/statuses/${HEAD_SHA}" \
+ python3 "$REVIEW_GH_HELPER" api "repos/${REPO}/statuses/${HEAD_SHA}"
\
-X POST \
-f state="$STATE" \
-f context='code-review' \
-f description="$DESCRIPTION" \
-f target_url="${{ github.server_url }}/${{ github.repository
}}/actions/runs/${{ github.run_id }}"
+
+ - name: Preserve GitHub API failure diagnostics
+ if: always()
+ continue-on-error: true
+ timeout-minutes: 2
+ uses: actions/upload-artifact@v4
+ with:
+ name: github-api-errors-aggregate-${{ inputs.head_sha }}-${{
github.run_id }}-${{ github.run_attempt }}
+ path: ${{ runner.temp }}/review-github-diagnostics/*.json
+ if-no-files-found: ignore
diff --git a/.github/workflows/code-review-runner.yml
b/.github/workflows/code-review-runner.yml
index 3f61b66a669..0db0b373032 100644
--- a/.github/workflows/code-review-runner.yml
+++ b/.github/workflows/code-review-runner.yml
@@ -232,6 +232,26 @@ jobs:
)
)
steps:
+ # Keep the trusted helper outside the PR checkout; capture the first API
failure too.
+ - name: Checkout trusted GitHub API diagnostics
+ timeout-minutes: 2
+ uses: actions/checkout@v4
+ with:
+ ref: ${{ github.workflow_sha || github.sha }}
+ persist-credentials: false
+ path: .review-api-source
+ sparse-checkout: .github/scripts/review_github_api.py
+ sparse-checkout-cone-mode: false
+
+ - name: Install GitHub API diagnostics
+ timeout-minutes: 1
+ run: |
+ install -m 700
.review-api-source/.github/scripts/review_github_api.py
"$RUNNER_TEMP/review_github_api.py"
+ gh --version
+ printf 'REVIEW_GH_VERSION=%s\n' "$(gh --version | sed -n '1p')" >>
"$GITHUB_ENV"
+ printf 'REVIEW_GH_HELPER=%s\n' "$RUNNER_TEMP/review_github_api.py"
>> "$GITHUB_ENV"
+ printf 'REVIEW_GITHUB_DIAGNOSTICS_DIR=%s\n'
"$RUNNER_TEMP/review-github-diagnostics" >> "$GITHUB_ENV"
+
- name: Resolve review inputs
id: review_inputs
timeout-minutes: 2
@@ -255,7 +275,7 @@ jobs:
MANAGE_STATUS="$INPUT_MANAGE_STATUS"
elif [ "$EVENT_NAME" = "issue_comment" ]; then
PR_NUMBER="$ISSUE_NUMBER"
- PR_JSON="$(gh api "repos/${REPO}/pulls/${PR_NUMBER}")"
+ PR_JSON="$(python3 "$REVIEW_GH_HELPER" api
"repos/${REPO}/pulls/${PR_NUMBER}")"
HEAD_SHA="$(jq -er '.head.sha' <<<"$PR_JSON")"
BASE_SHA="$(jq -er '.base.sha' <<<"$PR_JSON")"
REVIEW_FOCUS="$(printf '%s' "${COMMENT_BODY#'/review'}" |
@@ -271,7 +291,7 @@ jobs:
test -n "$BASE_SHA"
if [ "$MANAGE_STATUS" = "true" ]; then
- PR_JSON="${PR_JSON:-$(gh api "repos/${REPO}/pulls/${PR_NUMBER}")}"
+ PR_JSON="${PR_JSON:-$(python3 "$REVIEW_GH_HELPER" api
"repos/${REPO}/pulls/${PR_NUMBER}")}"
LIVE_HEAD_SHA="$(jq -er '.head.sha' <<<"$PR_JSON")"
LIVE_BASE_SHA="$(jq -er '.base.sha' <<<"$PR_JSON")"
if [ "$LIVE_HEAD_SHA" != "$HEAD_SHA" ] || [ "$LIVE_BASE_SHA" !=
"$BASE_SHA" ]; then
@@ -303,7 +323,7 @@ jobs:
PR_NUMBER: ${{ steps.review_inputs.outputs.pr_number }}
REPO: ${{ github.repository }}
run: |
- gh api "repos/${REPO}/statuses/${HEAD_SHA}" \
+ python3 "$REVIEW_GH_HELPER" api "repos/${REPO}/statuses/${HEAD_SHA}"
\
-X POST \
-f state='pending' \
-f context='code-review' \
@@ -311,7 +331,7 @@ jobs:
-f target_url="${{ github.server_url }}/${{ github.repository
}}/actions/runs/${{ github.run_id }}"
source_context="code-review/source/automated/pr-${PR_NUMBER}/base-${BASE_SHA}"
- gh api "repos/${REPO}/statuses/${HEAD_SHA}" \
+ python3 "$REVIEW_GH_HELPER" api "repos/${REPO}/statuses/${HEAD_SHA}"
\
-X POST \
-f state='pending' \
-f context="$source_context" \
@@ -560,7 +580,7 @@ jobs:
MAX_THREADS=30
MAX_BODY_CHARS=1200
- if gh api --paginate --slurp
repos/${REPO}/pulls/${PR_NUMBER}/comments >
"$REVIEW_CONTEXT_DIR/pr_review_comments_pages.json"
2>"$REVIEW_CONTEXT_DIR/pr_review_comments_error.log" \
+ if python3 "$REVIEW_GH_HELPER" api --paginate --slurp
repos/${REPO}/pulls/${PR_NUMBER}/comments >
"$REVIEW_CONTEXT_DIR/pr_review_comments_pages.json"
2>"$REVIEW_CONTEXT_DIR/pr_review_comments_error.log" \
&& jq 'add | sort_by((.in_reply_to_id // .id), .id)'
"$REVIEW_CONTEXT_DIR/pr_review_comments_pages.json" >
"$REVIEW_CONTEXT_DIR/pr_review_comments.json"
2>>"$REVIEW_CONTEXT_DIR/pr_review_comments_error.log" \
&& jq -r '
def shorten($limit):
@@ -639,7 +659,7 @@ jobs:
exit 1
fi
- live_pr="$(gh api "repos/${REPO}/pulls/${PR_NUMBER}")"
+ live_pr="$(python3 "$REVIEW_GH_HELPER" api
"repos/${REPO}/pulls/${PR_NUMBER}")"
live_head_sha="$(jq -r '.head.sha' <<<"$live_pr")"
live_base_sha="$(jq -r '.base.sha' <<<"$live_pr")"
live_base_ref="$(jq -er '.base.ref' <<<"$live_pr")"
@@ -670,7 +690,7 @@ jobs:
git diff --shortstat "$diff_range"
helper="$RUNNER_TEMP/prepare_review_agents.py"
- gh api \
+ python3 "$REVIEW_GH_HELPER" api \
-H "Accept: application/vnd.github.raw" \
"repos/${REPO}/contents/.github/scripts/prepare_review_agents.py?ref=${HELPER_REF}"
\
> "$helper"
@@ -875,7 +895,7 @@ jobs:
review_step_started_at=$SECONDS
for script in run_review_with_resume.py submit_review.py; do
- gh api \
+ python3 "$REVIEW_GH_HELPER" api \
-H "Accept: application/vnd.github.raw" \
"repos/${REPO}/contents/.github/scripts/${script}?ref=${HELPER_REF}" \
> "$RUNNER_TEMP/$script"
@@ -1083,7 +1103,7 @@ jobs:
else
error_msg="${REVIEW_FAILURE_REASON:-Review step was
$REVIEW_OUTCOME (possibly timeout or cancelled)}"
fi
- gh pr comment "$PR_NUMBER" --body "$(cat <<EOF
+ python3 "$REVIEW_GH_HELPER" pr comment "$PR_NUMBER" --body "$(cat
<<EOF
Codex automated review failed and did not complete.
Error: ${error_msg}
@@ -1155,7 +1175,7 @@ jobs:
fi
source_context="code-review/source/automated/pr-${PR_NUMBER}/base-${BASE_SHA}"
- gh api "repos/${REPO}/statuses/${HEAD_SHA}" \
+ python3 "$REVIEW_GH_HELPER" api "repos/${REPO}/statuses/${HEAD_SHA}"
\
-X POST \
-f state="${state}" \
-f context="$source_context" \
@@ -1184,7 +1204,7 @@ jobs:
fi
helper="$RUNNER_TEMP/emit_litefuse_otel_io.py"
- gh api \
+ python3 "$REVIEW_GH_HELPER" api \
-H "Accept: application/vnd.github.raw" \
"repos/${REPO}/contents/.github/scripts/emit_litefuse_otel_io.py?ref=${HELPER_REF}"
\
> "$helper"
@@ -1301,6 +1321,16 @@ jobs:
OSS_SK: ${{ secrets.OSS_SK }}
OSS_ENDPOINT: oss-cn-hongkong.aliyuncs.com
+ - name: Preserve GitHub API failure diagnostics
+ if: always()
+ continue-on-error: true
+ timeout-minutes: 2
+ uses: actions/upload-artifact@v4
+ with:
+ name: github-api-errors-${{ github.job }}-${{ github.run_id }}-${{
github.run_attempt }}
+ path: ${{ runner.temp }}/review-github-diagnostics/*.json
+ if-no-files-found: ignore
+
aggregate-status:
needs: code-review
if: >-
diff --git a/.github/workflows/code-review-sync-result.yml
b/.github/workflows/code-review-sync-result.yml
index a45a9b06041..547d60bb897 100644
--- a/.github/workflows/code-review-sync-result.yml
+++ b/.github/workflows/code-review-sync-result.yml
@@ -23,6 +23,26 @@ jobs:
github.event.issue.pull_request != null &&
contains(github.event.comment.body, '<!-- doris-repo-review:v1:begin
-->')
steps:
+ # Keep the trusted helper outside the PR checkout; capture the first API
failure too.
+ - name: Checkout trusted GitHub API diagnostics
+ timeout-minutes: 2
+ uses: actions/checkout@v4
+ with:
+ ref: ${{ github.workflow_sha || github.sha }}
+ persist-credentials: false
+ path: .review-api-source
+ sparse-checkout: .github/scripts/review_github_api.py
+ sparse-checkout-cone-mode: false
+
+ - name: Install GitHub API diagnostics
+ timeout-minutes: 1
+ run: |
+ install -m 700
.review-api-source/.github/scripts/review_github_api.py
"$RUNNER_TEMP/review_github_api.py"
+ gh --version
+ printf 'REVIEW_GH_VERSION=%s\n' "$(gh --version | sed -n '1p')" >>
"$GITHUB_ENV"
+ printf 'REVIEW_GH_HELPER=%s\n' "$RUNNER_TEMP/review_github_api.py"
>> "$GITHUB_ENV"
+ printf 'REVIEW_GITHUB_DIAGNOSTICS_DIR=%s\n'
"$RUNNER_TEMP/review-github-diagnostics" >> "$GITHUB_ENV"
+
# A local PASS is a creation-time credential. Later edits, deletion, or
# permission changes do not revoke it; edited events are intentionally
ignored.
- name: Authorize review comment creator
@@ -39,7 +59,7 @@ jobs:
exit 0
fi
- if ! permission_info="$(gh api
"repos/${REPO}/collaborators/${COMMENT_AUTHOR}/permission")"; then
+ if ! permission_info="$(python3 "$REVIEW_GH_HELPER" api
"repos/${REPO}/collaborators/${COMMENT_AUTHOR}/permission")"; then
echo "Review comment ignored: cannot verify ${COMMENT_AUTHOR}'s
repository permission."
echo "authorized=false" >> "$GITHUB_OUTPUT"
exit 0
@@ -80,7 +100,7 @@ jobs:
comment_file="$RUNNER_TEMP/doris-repo-review-comment.md"
jq -r '.comment.body' "$GITHUB_EVENT_PATH" > "$comment_file"
- pr_info="$(gh api "repos/${REPO}/pulls/${PR_NUMBER}")"
+ pr_info="$(python3 "$REVIEW_GH_HELPER" api
"repos/${REPO}/pulls/${PR_NUMBER}")"
head_sha="$(jq -r '.head.sha' <<<"$pr_info")"
live_base_sha="$(jq -r '.base.sha' <<<"$pr_info")"
pr_state="$(jq -r '.state' <<<"$pr_info")"
@@ -96,7 +116,7 @@ jobs:
echo "valid=false" >> "$GITHUB_OUTPUT"
exit 0
fi
- if ! base_compare="$(gh api \
+ if ! base_compare="$(python3 "$REVIEW_GH_HELPER" api \
"repos/${REPO}/compare/${reviewed_base_sha}...${live_base_sha}")"; then
echo "Review comment ignored: cannot compare its base with the
current PR base."
echo "valid=false" >> "$GITHUB_OUTPUT"
@@ -107,9 +127,9 @@ jobs:
reviewed_base_committed_at=""
live_base_committed_at=""
else
- if ! reviewed_base_committed_at="$(gh api \
+ if ! reviewed_base_committed_at="$(python3 "$REVIEW_GH_HELPER" api
\
"repos/${REPO}/commits/${reviewed_base_sha}" --jq
'.commit.committer.date')" || \
- ! live_base_committed_at="$(gh api \
+ ! live_base_committed_at="$(python3 "$REVIEW_GH_HELPER" api \
"repos/${REPO}/commits/${live_base_sha}" --jq
'.commit.committer.date')"; then
echo "Review comment ignored: cannot resolve base commit times."
echo "valid=false" >> "$GITHUB_OUTPUT"
@@ -148,20 +168,30 @@ jobs:
PR_NUMBER: ${{ github.event.issue.number }}
run: |
source_context="code-review/source/local/pr-${PR_NUMBER}/base-${BASE_SHA}"
- gh api "repos/${REPO}/statuses/${HEAD_SHA}" \
+ python3 "$REVIEW_GH_HELPER" api "repos/${REPO}/statuses/${HEAD_SHA}"
\
-X POST \
-f state='pending' \
-f context='code-review' \
-f description="Recalculating code review for PR #${PR_NUMBER}." \
-f target_url="$COMMENT_URL"
- gh api "repos/${REPO}/statuses/${HEAD_SHA}" \
+ python3 "$REVIEW_GH_HELPER" api "repos/${REPO}/statuses/${HEAD_SHA}"
\
-X POST \
-f state='success' \
-f context="$source_context" \
-f description="Local review passed for PR #${PR_NUMBER} at
${BASE_SHA:0:12}." \
-f target_url="$COMMENT_URL"
+ - name: Preserve GitHub API failure diagnostics
+ if: always()
+ continue-on-error: true
+ timeout-minutes: 2
+ uses: actions/upload-artifact@v4
+ with:
+ name: github-api-errors-${{ github.job }}-${{ github.run_id }}-${{
github.run_attempt }}
+ path: ${{ runner.temp }}/review-github-diagnostics/*.json
+ if-no-files-found: ignore
+
aggregate-after-skill-review:
needs: accept-skill-review
if: needs.accept-skill-review.outputs.accepted == 'true'
@@ -185,6 +215,26 @@ jobs:
!startsWith(github.event.comment.body, '/review') &&
contains(github.event.comment.body, 'skip buildall')
steps:
+ # Keep the trusted helper outside the PR checkout; capture the first API
failure too.
+ - name: Checkout trusted GitHub API diagnostics
+ timeout-minutes: 2
+ uses: actions/checkout@v4
+ with:
+ ref: ${{ github.workflow_sha || github.sha }}
+ persist-credentials: false
+ path: .review-api-source
+ sparse-checkout: .github/scripts/review_github_api.py
+ sparse-checkout-cone-mode: false
+
+ - name: Install GitHub API diagnostics
+ timeout-minutes: 1
+ run: |
+ install -m 700
.review-api-source/.github/scripts/review_github_api.py
"$RUNNER_TEMP/review_github_api.py"
+ gh --version
+ printf 'REVIEW_GH_VERSION=%s\n' "$(gh --version | sed -n '1p')" >>
"$GITHUB_ENV"
+ printf 'REVIEW_GH_HELPER=%s\n' "$RUNNER_TEMP/review_github_api.py"
>> "$GITHUB_ENV"
+ printf 'REVIEW_GITHUB_DIAGNOSTICS_DIR=%s\n'
"$RUNNER_TEMP/review-github-diagnostics" >> "$GITHUB_ENV"
+
- name: Check user permission and mark review as success
id: skip
env:
@@ -193,7 +243,7 @@ jobs:
COMMENT_USER_ID: ${{ github.event.comment.user.id }}
PR_NUMBER: ${{ github.event.issue.number }}
run: |
- PR_INFO=$(gh api repos/${REPO}/pulls/${PR_NUMBER})
+ PR_INFO=$(python3 "$REVIEW_GH_HELPER" api
repos/${REPO}/pulls/${PR_NUMBER})
HEAD_SHA=$(echo "${PR_INFO}" | jq -r '.head.sha')
BASE_SHA=$(echo "${PR_INFO}" | jq -r '.base.sha')
TARGET_BRANCH=$(echo "${PR_INFO}" | jq -r '.base.ref')
@@ -221,14 +271,14 @@ jobs:
echo "COMMENT_USER_ID ${COMMENT_USER_ID} is allowed to skip code
review for ${TARGET_BRANCH}."
source_context="code-review/source/skip/pr-${PR_NUMBER}/base-${BASE_SHA}"
- gh api repos/${REPO}/statuses/${HEAD_SHA} \
+ python3 "$REVIEW_GH_HELPER" api repos/${REPO}/statuses/${HEAD_SHA} \
-X POST \
-f state="pending" \
-f context='code-review' \
-f description="Recalculating code review for PR #${PR_NUMBER}." \
-f target_url="${{ github.server_url }}/${{ github.repository
}}/actions/runs/${{ github.run_id }}"
- gh api repos/${REPO}/statuses/${HEAD_SHA} \
+ python3 "$REVIEW_GH_HELPER" api repos/${REPO}/statuses/${HEAD_SHA} \
-X POST \
-f state="success" \
-f context="$source_context" \
@@ -238,6 +288,16 @@ jobs:
echo "accepted=true" >> "$GITHUB_OUTPUT"
echo "head_sha=$HEAD_SHA" >> "$GITHUB_OUTPUT"
+ - name: Preserve GitHub API failure diagnostics
+ if: always()
+ continue-on-error: true
+ timeout-minutes: 2
+ uses: actions/upload-artifact@v4
+ with:
+ name: github-api-errors-${{ github.job }}-${{ github.run_id }}-${{
github.run_attempt }}
+ path: ${{ runner.temp }}/review-github-diagnostics/*.json
+ if-no-files-found: ignore
+
aggregate-after-skip:
needs: skip-on-comment
if: needs.skip-on-comment.outputs.accepted == 'true'
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
