Files
doris/.github/scripts/review-github-diagnostics.md
shuke cb41339ffd [fix](ci) Retry transient review GET failures within bounded budgets (#69510)
Related PR: #69469

Problem Summary: A single transient GitHub response currently aborts the
review pipeline. [Input resolution for
#69322](https://github.com/apache/doris/actions/runs/39794013905/job/119876748768)
stopped on a GET 503 despite `Retry-After: 2`; [aggregate resolution for
#69438](https://github.com/apache/doris/actions/runs/39760934956/job/119764632659)
stopped on a paginated GET 504 after emitting partial output.

The shared helper now retries recognized body-free REST GETs on
502/503/504, for at most three attempts. Each attempt has a 30-second
limit; attempts and waits share a 90-second budget or the caller's
shorter timeout. It respects Retry-After, records every failed attempt,
discards failed pagination output, and returns only a successful
attempt's complete output. Exhaustion still fails. Writes, GraphQL,
unknown argument forms, other HTTP errors, transport/timeouts and JSON
parsing errors are not retried.

Input resolution can also fail before publishing its PR-number output.
Failure notices now fall back to the authorized trigger's numeric PR
identity, explicitly select the repository, and name the
input-resolution failure in both the notice and terminal error. This
fallback does not permit status publication for an unresolved head/base.

This change can merge independently. It includes the same explicit
`--repo` argument as #69469; a local combination with #69469 and #69504
merged cleanly and passed the macOS suite.
2026-11-02 14:23:14 +08:00

5.5 KiB

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 and permission decisions remain unchanged.

Body-free REST GETs using the pipeline's known gh api arguments retry HTTP 502/503/504 at most twice. Each attempt is limited to 30 seconds; all attempts and waits share 90 seconds, or the caller's shorter timeout. Retry-After (seconds or an HTTP date) is respected; a malformed header or a wait exceeding the remaining budget stops retries. Each failed attempt keeps its diagnostic. Only a successful attempt's complete stdout is returned; failed pagination output is discarded before restarting the full read. Exhaustion still fails.

Writes, GraphQL, fields/input, unknown argument forms, transport/timeouts, JSON parse failures and other HTTP statuses retain single-attempt behavior. The failure notification can use the authorized event/input PR number when input resolution failed before producing outputs; this does not authorize status writes for an unresolved head/base.

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. GraphQL errors returned with HTTP 200 are recorded as failures as well. 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---; 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:

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.

Staged final review submission

When a review has inline findings, submit_review.py creates one PENDING review, adds each finding with a separate addPullRequestReviewThread mutation, verifies the full draft comment set, then submits the review once. This avoids asking the bulk REST create-review endpoint to resolve all targeted file diffs in one request. The input still uses path/position/body; line and side are derived from the authoritative local pr.diff before any write. A summary-only review keeps the direct REST submission path.

The invocation marker, frozen head/base guards, exclusive review-submission.json intent and complete final readback remain required. A pending review, partial comment set or local journal never proves delivery. No write is automatically retried. A lost final-submit response may be recovered by exact readback of the already submitted review, without another submission event.

failed-code-review-* now also includes review-draft.json: the current stage, owned review ID, confirmed thread IDs and current finding index/location. This is diagnostic progress, not a resume instruction. Before publication, an error triggers best-effort deletion only of the uniquely identified pending draft with this invocation's exact body, bot author and head. Foreign or modified drafts and submitted reviews are never deleted.

An ambiguous publish attempt is not deleted or repeated. If it remains PENDING, or cleanup cannot finish because of an API outage, exhausted budget or process termination, inspect the journal and GitHub state before manually clearing the owned draft and starting a new run. Never remove the submission intent to force a retry. The pipeline must fail rather than report partial delivery.

The motivating #68499 experiment accepted all five original findings in one draft via sequential GraphQL mutations, while bulk REST creation rejected the same set with 422. That experiment used a personal account and deleted the draft; formal publication and the Actions identity still require live rollout validation. The exact GitHub internal diff limit is not established.