github-actions[bot] commented on code in PR #66119:
URL: https://github.com/apache/doris/pull/66119#discussion_r3662762978
##########
.github/workflows/code-review-runner.yml:
##########
@@ -493,44 +578,186 @@ jobs:
continue-on-error: true
env:
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+ OSS_AK: ${{ secrets.OSS_AK }}
+ OSS_SK: ${{ secrets.OSS_SK }}
+ OSS_ENDPOINT: oss-cn-hongkong.aliyuncs.com
+ OSS_CODEX_AUTH_STATUS_OBJECT:
oss://doris-community-ci/codex/auth-status.json
+ AUTH_UNAVAILABLE: ${{ steps.configure_auth.outputs.auth_unavailable
}}
+ CONFIGURED_ALL_QUOTA_EXHAUSTED: ${{
steps.configure_auth.outputs.all_quota_exhausted }}
REPO: ${{ github.repository }}
PR_NUMBER: ${{ inputs.pr_number }}
HEAD_SHA: ${{ inputs.head_sha }}
run: |
- GOAL_PROMPT="$(cat "$REVIEW_CONTEXT_DIR/codex_goal_prompt.txt")"
- review_started_at="$(date -u +%Y-%m-%dT%H:%M:%SZ)"
+ write_review_output() {
+ local key="$1"
+ local value="$2"
+ {
+ echo "${key}<<EOF"
+ printf '%s\n' "$value"
+ echo "EOF"
+ } >> "$GITHUB_OUTPUT"
+ }
+
+ auth_object_args=()
+ known_auth_object_args=()
+ while IFS= read -r auth_object; do
+ auth_object_args+=(--auth-object "$auth_object")
+ known_auth_object_args+=(--known-auth-object "$auth_object")
+ done < "$CODEX_AUTH_OBJECTS_FILE"
+ if [ "${#auth_object_args[@]}" -eq 0 ]; then
+ write_review_output "all_quota_exhausted" "false"
+ write_review_output "failure_reason" "No valid Codex auth account
is available for review."
+ exit 1
+ fi
- set +e
- # GitHub-hosted runners are ephemeral. Avoid workspace-write here
because
- # Codex uses bubblewrap for that mode and uid maps can be
unavailable.
- codex exec --goal "$GOAL_PROMPT" \
- --cd "$GITHUB_WORKSPACE" \
- --model "gpt-5.6-sol" \
- --config "model_reasoning_effort=xhigh" \
- --sandbox danger-full-access \
- --color never \
- --json \
- --output-last-message
"$REVIEW_CONTEXT_DIR/codex-final-message.txt" \
- > "$REVIEW_CONTEXT_DIR/codex-events.jsonl" \
- 2> >(tee "$REVIEW_CONTEXT_DIR/codex-stderr.log" >&2)
- status=$?
- set -e
+ sync_auth_state() {
+ ossutil -i "$OSS_AK" -k "$OSS_SK" -e "$OSS_ENDPOINT" cp -f \
+ "$CODEX_AUTH_STATE_FILE" "$OSS_CODEX_AUTH_STATUS_OBJECT"
+ }
+
+ sync_current_auth() {
+ if [ -s "$CODEX_HOME/auth.json" ] && jq -e '
+ .auth_mode == "chatgpt"
+ and (.tokens.access_token | type == "string" and length > 0)
+ and (.tokens.refresh_token | type == "string" and length > 0)
+ ' "$CODEX_HOME/auth.json" >/dev/null; then
+ ossutil -i "$OSS_AK" -k "$OSS_SK" -e "$OSS_ENDPOINT" cp -f \
+ "$CODEX_HOME/auth.json" "$CODEX_AUTH_OSS_OBJECT"
+ else
+ echo "The current Codex auth file is invalid; it will not be
synced to OSS."
+ fi
+ }
+
+ record_auth_result() {
+ local result="$1"
+ local http_status="$2"
+ local record_args=(
+ record
+ --state "$CODEX_AUTH_STATE_FILE"
+ --auth-object "$CODEX_AUTH_OSS_OBJECT"
+ "${known_auth_object_args[@]}"
+ --result "$result"
+ )
+ if [ -n "$http_status" ]; then
+ record_args+=(--http-status "$http_status")
+ fi
+ "$CODEX_AUTH_STATE_HELPER" "${record_args[@]}"
+ }
+
+ select_next_auth() {
+ local selection_json
+ selection_json="$("$CODEX_AUTH_STATE_HELPER" select \
+ --state "$CODEX_AUTH_STATE_FILE" \
+ "${auth_object_args[@]}")"
+ sync_auth_state
+ selected_auth_object="$(jq -r '.auth_object // empty'
<<<"$selection_json")"
+ all_quota_exhausted="$(jq -r '.all_quota_exhausted'
<<<"$selection_json")"
+ next_retry_at="$(jq -r '.next_retry_at // empty'
<<<"$selection_json")"
+ }
+
+ if [ "$AUTH_UNAVAILABLE" = "true" ]; then
+ write_review_output "all_quota_exhausted"
"$CONFIGURED_ALL_QUOTA_EXHAUSTED"
+ write_review_output "failure_reason" "No Codex auth account is
currently eligible for review."
+ exit 1
+ fi
+ GOAL_PROMPT="$(cat "$REVIEW_CONTEXT_DIR/codex_goal_prompt.txt")"
failure_reason=""
- if [ "$status" -ne 0 ]; then
- if [ -s "$REVIEW_CONTEXT_DIR/codex-events.jsonl" ]; then
- failure_reason="$(jq -r 'select(.type == "turn.failed") |
.error.message // empty' "$REVIEW_CONTEXT_DIR/codex-events.jsonl" | tail -n 1)"
+ all_quota_exhausted="${CONFIGURED_ALL_QUOTA_EXHAUSTED:-false}"
+ transient_attempt=0
+ while :; do
+ review_started_at="$(date -u +%Y-%m-%dT%H:%M:%SZ)"
+ events_file="$REVIEW_CONTEXT_DIR/codex-events.jsonl"
+ stderr_file="$REVIEW_CONTEXT_DIR/codex-stderr.log"
+
+ set +e
+ # GitHub-hosted runners are ephemeral. Avoid workspace-write here
because
+ # Codex uses bubblewrap for that mode and uid maps can be
unavailable.
+ codex exec --goal "$GOAL_PROMPT" \
+ --cd "$GITHUB_WORKSPACE" \
+ --model "gpt-5.6-sol" \
+ --config "model_reasoning_effort=xhigh" \
+ --sandbox danger-full-access \
+ --color never \
+ --json \
+ --output-last-message
"$REVIEW_CONTEXT_DIR/codex-final-message.txt" \
+ > "$events_file" \
+ 2> >(tee "$stderr_file" >&2)
+ status=$?
+ set -e
+
+ if [ "$status" -eq 0 ]; then
+ state_result="$(record_auth_result available "")"
+ sync_auth_state
+ all_quota_exhausted="$(jq -r '.all_quota_exhausted'
<<<"$state_result")"
+ break
+ fi
+
+ if [ -s "$events_file" ]; then
+ failure_reason="$(jq -r 'select(.type == "turn.failed") |
.error.message // empty' "$events_file" | tail -n 1)"
if [ -z "$failure_reason" ]; then
- failure_reason="$(jq -r 'select(.type == "error") | .message
// .error.message // empty' "$REVIEW_CONTEXT_DIR/codex-events.jsonl" | tail -n
1)"
+ failure_reason="$(jq -r 'select(.type == "error") | .message
// .error.message // empty' "$events_file" | tail -n 1)"
fi
fi
- if [ -z "$failure_reason" ] && [ -s
"$REVIEW_CONTEXT_DIR/codex-stderr.log" ]; then
- failure_reason="$(awk 'NF { line = $0 } END { print line }'
"$REVIEW_CONTEXT_DIR/codex-stderr.log")"
+ if [ -z "$failure_reason" ] && [ -s "$stderr_file" ]; then
+ failure_reason="$(awk 'NF { line = $0 } END { print line }'
"$stderr_file")"
fi
if [ -z "$failure_reason" ]; then
failure_reason="Codex exited with status $status"
fi
- fi
+
+ classification="$("$CODEX_AUTH_STATE_HELPER" classify --input
"$events_file" --input "$stderr_file")"
Review Comment:
[P1] Do not execute agent-writable control files with OSS authority
`codex exec` runs as the same runner user with `danger-full-access`, so it
can replace `$RUNNER_TEMP/codex_auth_state.py` or forge
`$CODEX_HOME/auth-status.json` before returning. The secret-bearing parent then
executes that mutable helper here (and from `record_auth_result`) and uploads
the mutable state. Consequently, merely unsetting `OSS_AK`/`OSS_SK` for the
child would still let a replacement helper inherit and use the parent's keys,
or persist forged cooldowns for healthy accounts. Please move post-agent
classification/state/auth orchestration behind a separate UID/container/job
boundary with an immutable freshly verified helper and accept only a narrowly
validated result from the agent. This is a CI secret-boundary defect, not a
Doris runtime vulnerability.
##########
.github/workflows/code-review-runner.yml:
##########
@@ -493,44 +578,186 @@ jobs:
continue-on-error: true
env:
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+ OSS_AK: ${{ secrets.OSS_AK }}
Review Comment:
[P1] Keep the OSS credentials out of the agent process
Making `OSS_AK`/`OSS_SK` step-scoped means `codex exec` and every shell tool
it launches inherit them; the generated config explicitly uses
`shell_environment_policy.inherit = "all"` and `danger-full-access`. Because
this job reviews PR-controlled source, a prompt-injected or accidental
diagnostic command can now use the installed `ossutil` to list/read/write every
object allowed by these keys—including other auth files, shared state,
memories, and sessions—rather than only the selected `auth.json`. Please
isolate the secret-bearing rotation/sync wrapper from the agent process (or use
a narrowly scoped broker/short-lived credential) and ensure the Codex process
and its parent environment cannot expose the OSS keys. This is a CI
secret-boundary defect, not a Doris runtime vulnerability under the repository
threat model.
##########
.github/workflows/code-review-runner.yml:
##########
@@ -493,44 +578,186 @@ jobs:
continue-on-error: true
env:
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+ OSS_AK: ${{ secrets.OSS_AK }}
+ OSS_SK: ${{ secrets.OSS_SK }}
+ OSS_ENDPOINT: oss-cn-hongkong.aliyuncs.com
+ OSS_CODEX_AUTH_STATUS_OBJECT:
oss://doris-community-ci/codex/auth-status.json
+ AUTH_UNAVAILABLE: ${{ steps.configure_auth.outputs.auth_unavailable
}}
+ CONFIGURED_ALL_QUOTA_EXHAUSTED: ${{
steps.configure_auth.outputs.all_quota_exhausted }}
REPO: ${{ github.repository }}
PR_NUMBER: ${{ inputs.pr_number }}
HEAD_SHA: ${{ inputs.head_sha }}
run: |
- GOAL_PROMPT="$(cat "$REVIEW_CONTEXT_DIR/codex_goal_prompt.txt")"
- review_started_at="$(date -u +%Y-%m-%dT%H:%M:%SZ)"
+ write_review_output() {
+ local key="$1"
+ local value="$2"
+ {
+ echo "${key}<<EOF"
+ printf '%s\n' "$value"
+ echo "EOF"
+ } >> "$GITHUB_OUTPUT"
+ }
+
+ auth_object_args=()
+ known_auth_object_args=()
+ while IFS= read -r auth_object; do
+ auth_object_args+=(--auth-object "$auth_object")
+ known_auth_object_args+=(--known-auth-object "$auth_object")
+ done < "$CODEX_AUTH_OBJECTS_FILE"
+ if [ "${#auth_object_args[@]}" -eq 0 ]; then
+ write_review_output "all_quota_exhausted" "false"
+ write_review_output "failure_reason" "No valid Codex auth account
is available for review."
+ exit 1
+ fi
- set +e
- # GitHub-hosted runners are ephemeral. Avoid workspace-write here
because
- # Codex uses bubblewrap for that mode and uid maps can be
unavailable.
- codex exec --goal "$GOAL_PROMPT" \
- --cd "$GITHUB_WORKSPACE" \
- --model "gpt-5.6-sol" \
- --config "model_reasoning_effort=xhigh" \
- --sandbox danger-full-access \
- --color never \
- --json \
- --output-last-message
"$REVIEW_CONTEXT_DIR/codex-final-message.txt" \
- > "$REVIEW_CONTEXT_DIR/codex-events.jsonl" \
- 2> >(tee "$REVIEW_CONTEXT_DIR/codex-stderr.log" >&2)
- status=$?
- set -e
+ sync_auth_state() {
+ ossutil -i "$OSS_AK" -k "$OSS_SK" -e "$OSS_ENDPOINT" cp -f \
+ "$CODEX_AUTH_STATE_FILE" "$OSS_CODEX_AUTH_STATUS_OBJECT"
+ }
+
+ sync_current_auth() {
+ if [ -s "$CODEX_HOME/auth.json" ] && jq -e '
+ .auth_mode == "chatgpt"
+ and (.tokens.access_token | type == "string" and length > 0)
+ and (.tokens.refresh_token | type == "string" and length > 0)
+ ' "$CODEX_HOME/auth.json" >/dev/null; then
+ ossutil -i "$OSS_AK" -k "$OSS_SK" -e "$OSS_ENDPOINT" cp -f \
+ "$CODEX_HOME/auth.json" "$CODEX_AUTH_OSS_OBJECT"
+ else
+ echo "The current Codex auth file is invalid; it will not be
synced to OSS."
+ fi
+ }
+
+ record_auth_result() {
+ local result="$1"
+ local http_status="$2"
+ local record_args=(
+ record
+ --state "$CODEX_AUTH_STATE_FILE"
+ --auth-object "$CODEX_AUTH_OSS_OBJECT"
+ "${known_auth_object_args[@]}"
+ --result "$result"
+ )
+ if [ -n "$http_status" ]; then
+ record_args+=(--http-status "$http_status")
+ fi
+ "$CODEX_AUTH_STATE_HELPER" "${record_args[@]}"
+ }
+
+ select_next_auth() {
+ local selection_json
+ selection_json="$("$CODEX_AUTH_STATE_HELPER" select \
+ --state "$CODEX_AUTH_STATE_FILE" \
+ "${auth_object_args[@]}")"
+ sync_auth_state
+ selected_auth_object="$(jq -r '.auth_object // empty'
<<<"$selection_json")"
+ all_quota_exhausted="$(jq -r '.all_quota_exhausted'
<<<"$selection_json")"
+ next_retry_at="$(jq -r '.next_retry_at // empty'
<<<"$selection_json")"
+ }
+
+ if [ "$AUTH_UNAVAILABLE" = "true" ]; then
+ write_review_output "all_quota_exhausted"
"$CONFIGURED_ALL_QUOTA_EXHAUSTED"
+ write_review_output "failure_reason" "No Codex auth account is
currently eligible for review."
+ exit 1
+ fi
+ GOAL_PROMPT="$(cat "$REVIEW_CONTEXT_DIR/codex_goal_prompt.txt")"
failure_reason=""
- if [ "$status" -ne 0 ]; then
- if [ -s "$REVIEW_CONTEXT_DIR/codex-events.jsonl" ]; then
- failure_reason="$(jq -r 'select(.type == "turn.failed") |
.error.message // empty' "$REVIEW_CONTEXT_DIR/codex-events.jsonl" | tail -n 1)"
+ all_quota_exhausted="${CONFIGURED_ALL_QUOTA_EXHAUSTED:-false}"
+ transient_attempt=0
+ while :; do
+ review_started_at="$(date -u +%Y-%m-%dT%H:%M:%SZ)"
+ events_file="$REVIEW_CONTEXT_DIR/codex-events.jsonl"
+ stderr_file="$REVIEW_CONTEXT_DIR/codex-stderr.log"
+
+ set +e
+ # GitHub-hosted runners are ephemeral. Avoid workspace-write here
because
+ # Codex uses bubblewrap for that mode and uid maps can be
unavailable.
+ codex exec --goal "$GOAL_PROMPT" \
Review Comment:
[P1] Do not replay the full review after side effects may have landed
`codex exec --goal` is not side-effect-free: the required goal submits
GitHub reviews/inline comments before its final verification and completion
turns. If that later turn gets a 503, connection reset, or credential failure,
this loop starts a fresh full goal against the pre-run thread snapshot, so it
can submit the same review again. Resetting `review_started_at` also makes the
final verifier ignore the first attempt's landed review. Please keep retries
before the submission boundary, or persist/reconcile a frozen review payload
and refresh live reviews/comments before any retry.
##########
.github/workflows/code-review-runner.yml:
##########
@@ -101,14 +115,84 @@ jobs:
OSS_CODEX_GOAL_FALLBACK_OBJECT:
oss://doris-community-ci/codex/codex-goal
- name: Configure Codex auth
+ id: configure_auth
run: |
install -m 700 -d "$RUNNER_TEMP/codex-home"
printf 'CODEX_HOME=%s\n' "$RUNNER_TEMP/codex-home" >> "$GITHUB_ENV"
- auth_object="$(printf '%s\n' \
- 'oss://doris-community-ci/codex/auth.json.1' \
- 'oss://doris-community-ci/codex/auth.json.2' \
- | shuf -n 1)"
+ auth_prefix='oss://doris-community-ci/codex/auth.json.'
+ auth_listing="$RUNNER_TEMP/codex-auth-objects.list"
+ if ! ossutil -i "$OSS_AK" -k "$OSS_SK" -e "$OSS_ENDPOINT" ls
"$auth_prefix" -s > "$auth_listing"; then
+ echo 'Failed to list Codex auth objects from OSS.'
+ exit 1
+ fi
+
+ auth_objects_file="$RUNNER_TEMP/codex-home/auth-objects.txt"
+ candidate_auth_file="$(mktemp "$RUNNER_TEMP/codex-auth.XXXXXX")"
+ trap 'rm -f "$candidate_auth_file"' EXIT
+ while IFS= read -r candidate; do
+ if ossutil -i "$OSS_AK" -k "$OSS_SK" -e "$OSS_ENDPOINT" cp -f
"$candidate" "$candidate_auth_file" \
+ && test -s "$candidate_auth_file" \
+ && jq -e '
+ .auth_mode == "chatgpt"
+ and (.tokens.access_token | type == "string" and length > 0)
+ and (.tokens.refresh_token | type == "string" and length > 0)
+ ' "$candidate_auth_file" >/dev/null; then
+ printf '%s\n' "$candidate" >> "$auth_objects_file"
+ else
+ echo "Skipping invalid Codex auth object: ${candidate##*/}"
+ fi
+ done < <(
+ awk '$0 ~
/^oss:\/\/doris-community-ci\/codex\/auth\.json\.[0-9]+$/ { print }'
"$auth_listing" | shuf
Review Comment:
[P2] Keep the persisted cursor tied to a stable account order
`next_account` is only an integer index, but this `shuf` gives that index a
different meaning on every job. For example, a run over `[A, B, C]` selects `A`
and stores `1`; the next run over `[C, A, B]` starts at index 1 and selects `A`
again. Thus the helper's round-robin test does not hold in the actual workflow,
and sequential jobs can repeatedly choose the same credential. Please sort the
discovered objects before selection (or persist the next object's identity) and
add a test that reloads state with a reordered list.
##########
.github/scripts/codex_auth_state.py:
##########
@@ -0,0 +1,293 @@
+#!/usr/bin/env python3
+"""Persist and select the rotating credentials used by code review jobs."""
+
+from __future__ import annotations
+
+import argparse
+import json
+import re
+import tempfile
+from dataclasses import dataclass
+from datetime import datetime, timedelta, timezone
+from pathlib import Path
+from typing import Any
+
+
+STATE_VERSION = 1
+AVAILABLE = "available"
+QUOTA_EXHAUSTED = "quota_exhausted"
+RATE_LIMITED = "rate_limited"
+AUTHENTICATION_FAILED = "authentication_failed"
+TRANSIENT_FAILURE = "transient_failure"
+STATUS_VALUES = {AVAILABLE, QUOTA_EXHAUSTED, RATE_LIMITED,
AUTHENTICATION_FAILED}
+QUOTA_RETRY_DELAY = timedelta(days=1)
+RATE_LIMIT_RETRY_DELAY = timedelta(hours=1)
+AUTHENTICATION_RETRY_DELAY = timedelta(days=1)
+
+
+@dataclass(frozen=True)
+class Selection:
+ auth_object: str | None
+ all_quota_exhausted: bool
+ next_retry_at: str | None
+
+
+def utc_now() -> datetime:
+ return datetime.now(timezone.utc).replace(microsecond=0)
+
+
+def parse_timestamp(value: str) -> datetime:
+ parsed = datetime.fromisoformat(value.replace("Z", "+00:00"))
+ if parsed.tzinfo is None:
+ raise ValueError(f"Timestamp must include a timezone: {value!r}")
+ return parsed.astimezone(timezone.utc).replace(microsecond=0)
+
+
+def format_timestamp(value: datetime) -> str:
+ return
value.astimezone(timezone.utc).replace(microsecond=0).isoformat().replace("+00:00",
"Z")
+
+
+def default_account_state() -> dict[str, Any]:
+ return {
+ "status": AVAILABLE,
+ "last_failure_at": None,
+ "last_http_status": None,
+ "last_success_at": None,
+ "last_recovered_at": None,
+ "retry_after": None,
+ }
+
+
+def default_state() -> dict[str, Any]:
+ return {"version": STATE_VERSION, "next_account": 0, "accounts": {}}
+
+
+def load_state(path: Path) -> dict[str, Any]:
+ if not path.exists():
+ return default_state()
+
+ state = json.loads(path.read_text())
+ if not isinstance(state, dict):
+ raise ValueError("Authentication state must be a JSON object")
+ if state.get("version") != STATE_VERSION:
+ raise ValueError(f"Unsupported authentication state version:
{state.get('version')!r}")
+ if not isinstance(state.get("next_account"), int):
+ raise ValueError("Authentication state next_account must be an
integer")
+ if not isinstance(state.get("accounts"), dict):
+ raise ValueError("Authentication state accounts must be an object")
+ return state
+
+
+def save_state(path: Path, state: dict[str, Any]) -> None:
+ with tempfile.NamedTemporaryFile("w", encoding="utf-8", dir=path.parent,
delete=False) as temporary:
+ json.dump(state, temporary, indent=2, sort_keys=True)
+ temporary.write("\n")
+ temporary_path = Path(temporary.name)
+ temporary_path.replace(path)
+
+
+def account_state(state: dict[str, Any], auth_object: str) -> dict[str, Any]:
+ accounts = state["accounts"]
+ entry = accounts.setdefault(auth_object, default_account_state())
+ if not isinstance(entry, dict):
+ raise ValueError(f"Authentication state for {auth_object!r} must be an
object")
+ status = entry.get("status", AVAILABLE)
+ if status not in STATUS_VALUES:
+ raise ValueError(f"Unknown authentication status for {auth_object!r}:
{status!r}")
+ entry.setdefault("status", AVAILABLE)
+ entry.setdefault("last_failure_at", None)
+ entry.setdefault("last_http_status", None)
+ entry.setdefault("last_success_at", None)
+ entry.setdefault("last_recovered_at", None)
+ entry.setdefault("retry_after", None)
+ return entry
+
+
+def recover_expired_accounts(state: dict[str, Any], auth_objects: list[str],
now: datetime) -> None:
+ for auth_object in auth_objects:
+ entry = account_state(state, auth_object)
+ retry_after = entry["retry_after"]
+ if retry_after is None:
+ continue
+ if parse_timestamp(retry_after) <= now:
+ entry["status"] = AVAILABLE
+ entry["retry_after"] = None
+ entry["last_recovered_at"] = format_timestamp(now)
+
+
+def next_retry_at(state: dict[str, Any], auth_objects: list[str]) -> str |
None:
+ retry_at = [
+ entry["retry_after"]
+ for auth_object in auth_objects
+ for entry in [account_state(state, auth_object)]
+ if entry["retry_after"] is not None
+ ]
+ return min(retry_at, key=parse_timestamp, default=None)
+
+
+def all_quota_exhausted(state: dict[str, Any], auth_objects: list[str]) ->
bool:
+ return bool(auth_objects) and all(
+ account_state(state, auth_object)["status"] == QUOTA_EXHAUSTED for
auth_object in auth_objects
+ )
+
+
+def select_auth_object(state: dict[str, Any], auth_objects: list[str], now:
datetime) -> Selection:
+ if not auth_objects:
+ raise ValueError("At least one auth object is required")
+
+ recover_expired_accounts(state, auth_objects, now)
+ starting_index = state["next_account"] % len(auth_objects)
+ for offset in range(len(auth_objects)):
+ index = (starting_index + offset) % len(auth_objects)
+ auth_object = auth_objects[index]
+ if account_state(state, auth_object)["status"] == AVAILABLE:
+ state["next_account"] = (index + 1) % len(auth_objects)
+ return Selection(auth_object, False, None)
+
+ return Selection(None, all_quota_exhausted(state, auth_objects),
next_retry_at(state, auth_objects))
+
+
+def record_result(
+ state: dict[str, Any], auth_object: str, result: str, now: datetime,
http_status: int | None = None
+) -> None:
+ entry = account_state(state, auth_object)
+ timestamp = format_timestamp(now)
+ if result == AVAILABLE:
+ entry["status"] = AVAILABLE
+ entry["last_success_at"] = timestamp
+ entry["retry_after"] = None
+ return
+
+ if result == QUOTA_EXHAUSTED:
+ retry_delay = QUOTA_RETRY_DELAY
+ http_status = http_status or 403
+ elif result == RATE_LIMITED:
+ retry_delay = RATE_LIMIT_RETRY_DELAY
+ http_status = http_status or 429
+ elif result == AUTHENTICATION_FAILED:
+ retry_delay = AUTHENTICATION_RETRY_DELAY
+ http_status = http_status or 401
+ elif result == TRANSIENT_FAILURE:
+ entry["status"] = AVAILABLE
+ entry["last_failure_at"] = timestamp
+ entry["last_http_status"] = http_status
+ entry["retry_after"] = None
+ return
+ else:
+ raise ValueError(f"Unsupported result: {result!r}")
+
+ entry["status"] = result
+ entry["last_failure_at"] = timestamp
+ entry["last_http_status"] = http_status
+ entry["retry_after"] = format_timestamp(now + retry_delay)
+
+
+def extract_http_status(text: str) -> int | None:
+ labelled_statuses = re.findall(
+
r"(?:http(?:\s+status)?|status(?:\s+code)?|error\s+code)\s*[:=]?\s*([1-5]\d{2})",
+ text,
+ re.IGNORECASE,
+ )
+ if labelled_statuses:
+ return int(labelled_statuses[-1])
+ matches = re.findall(r"(?<!\d)([1-5]\d{2})(?!\d)", text)
+ return int(matches[0]) if matches else None
+
+
+def classify_failure(text: str) -> str:
Review Comment:
[P1] Handle the actual Codex usage-limit error
The failure that motivated related PR #66111 does not contain an HTTP
status: run 30259990434 emitted `You've hit your usage limit ... try again at
Aug 2nd, 2026 1:27 AM.` Feeding that exact message to this helper returns
`fatal`, so the workflow takes the generic break at lines 726-727 without
recording quota or selecting another credential. This leaves the core
quota-rotation path ineffective. Please classify the structured/terminal
usage-limit error itself, honor its supplied reset time rather than the fixed
one-day cooldown, and add this exact runner message as a regression fixture.
##########
.github/workflows/code-review-runner.yml:
##########
@@ -101,14 +115,84 @@ jobs:
OSS_CODEX_GOAL_FALLBACK_OBJECT:
oss://doris-community-ci/codex/codex-goal
- name: Configure Codex auth
+ id: configure_auth
run: |
install -m 700 -d "$RUNNER_TEMP/codex-home"
printf 'CODEX_HOME=%s\n' "$RUNNER_TEMP/codex-home" >> "$GITHUB_ENV"
- auth_object="$(printf '%s\n' \
- 'oss://doris-community-ci/codex/auth.json.1' \
- 'oss://doris-community-ci/codex/auth.json.2' \
- | shuf -n 1)"
+ auth_prefix='oss://doris-community-ci/codex/auth.json.'
+ auth_listing="$RUNNER_TEMP/codex-auth-objects.list"
+ if ! ossutil -i "$OSS_AK" -k "$OSS_SK" -e "$OSS_ENDPOINT" ls
"$auth_prefix" -s > "$auth_listing"; then
+ echo 'Failed to list Codex auth objects from OSS.'
+ exit 1
+ fi
+
+ auth_objects_file="$RUNNER_TEMP/codex-home/auth-objects.txt"
+ candidate_auth_file="$(mktemp "$RUNNER_TEMP/codex-auth.XXXXXX")"
+ trap 'rm -f "$candidate_auth_file"' EXIT
+ while IFS= read -r candidate; do
+ if ossutil -i "$OSS_AK" -k "$OSS_SK" -e "$OSS_ENDPOINT" cp -f
"$candidate" "$candidate_auth_file" \
+ && test -s "$candidate_auth_file" \
+ && jq -e '
+ .auth_mode == "chatgpt"
+ and (.tokens.access_token | type == "string" and length > 0)
+ and (.tokens.refresh_token | type == "string" and length > 0)
+ ' "$candidate_auth_file" >/dev/null; then
+ printf '%s\n' "$candidate" >> "$auth_objects_file"
+ else
+ echo "Skipping invalid Codex auth object: ${candidate##*/}"
+ fi
+ done < <(
+ awk '$0 ~
/^oss:\/\/doris-community-ci\/codex\/auth\.json\.[0-9]+$/ { print }'
"$auth_listing" | shuf
+ )
+
+ if [ ! -s "$auth_objects_file" ]; then
+ echo 'No valid Codex auth object found in OSS.'
+ exit 1
+ fi
+ chmod 600 "$auth_objects_file"
+
+ auth_object_args=()
+ while IFS= read -r auth_object; do
+ auth_object_args+=(--auth-object "$auth_object")
+ done < "$auth_objects_file"
+
+ state_file="$RUNNER_TEMP/codex-home/auth-status.json"
+ set +e
+ state_stat_output="$(ossutil -i "$OSS_AK" -k "$OSS_SK" -e
"$OSS_ENDPOINT" stat "$OSS_CODEX_AUTH_STATUS_OBJECT" 2>&1)"
+ state_stat_status=$?
+ set -e
+ if [ "$state_stat_status" -eq 0 ]; then
+ ossutil -i "$OSS_AK" -k "$OSS_SK" -e "$OSS_ENDPOINT" cp -f
"$OSS_CODEX_AUTH_STATUS_OBJECT" "$state_file"
+ elif grep -Eqi 'NoSuchKey|NoSuchObject|404' <<<"$state_stat_output";
then
+ "$CODEX_AUTH_STATE_HELPER" initialize \
+ --state "$state_file" \
+ "${auth_object_args[@]}"
+ ossutil -i "$OSS_AK" -k "$OSS_SK" -e "$OSS_ENDPOINT" cp -f
"$state_file" "$OSS_CODEX_AUTH_STATUS_OBJECT"
+ echo "Created Codex auth status file in OSS."
+ else
+ echo "Unable to determine whether the Codex auth status file
exists." >&2
+ exit 1
+ fi
+
+ selection_json="$("$CODEX_AUTH_STATE_HELPER" select \
+ --state "$state_file" \
+ "${auth_object_args[@]}")"
+ ossutil -i "$OSS_AK" -k "$OSS_SK" -e "$OSS_ENDPOINT" cp -f
"$state_file" "$OSS_CODEX_AUTH_STATUS_OBJECT"
Review Comment:
[P1] Coordinate updates to the shared auth state
This is a read/modify/write of one repository-global OSS object, but
code-review runs are allowed to overlap and every sync is an unconditional `cp
-f`. Two jobs can download the same cursor, both select account A, and then a
stale success upload from one job can overwrite the other's quota cooldown; the
refreshed `auth.json` uploads have the same last-writer-wins race. Please
serialize this critical section/run set, or use per-account leases plus
conditional/versioned writes and conflict retries for both state and token
refreshes.
##########
.github/scripts/codex_auth_state.py:
##########
@@ -0,0 +1,293 @@
+#!/usr/bin/env python3
+"""Persist and select the rotating credentials used by code review jobs."""
+
+from __future__ import annotations
+
+import argparse
+import json
+import re
+import tempfile
+from dataclasses import dataclass
+from datetime import datetime, timedelta, timezone
+from pathlib import Path
+from typing import Any
+
+
+STATE_VERSION = 1
+AVAILABLE = "available"
+QUOTA_EXHAUSTED = "quota_exhausted"
+RATE_LIMITED = "rate_limited"
+AUTHENTICATION_FAILED = "authentication_failed"
+TRANSIENT_FAILURE = "transient_failure"
+STATUS_VALUES = {AVAILABLE, QUOTA_EXHAUSTED, RATE_LIMITED,
AUTHENTICATION_FAILED}
+QUOTA_RETRY_DELAY = timedelta(days=1)
+RATE_LIMIT_RETRY_DELAY = timedelta(hours=1)
+AUTHENTICATION_RETRY_DELAY = timedelta(days=1)
+
+
+@dataclass(frozen=True)
+class Selection:
+ auth_object: str | None
+ all_quota_exhausted: bool
+ next_retry_at: str | None
+
+
+def utc_now() -> datetime:
+ return datetime.now(timezone.utc).replace(microsecond=0)
+
+
+def parse_timestamp(value: str) -> datetime:
+ parsed = datetime.fromisoformat(value.replace("Z", "+00:00"))
+ if parsed.tzinfo is None:
+ raise ValueError(f"Timestamp must include a timezone: {value!r}")
+ return parsed.astimezone(timezone.utc).replace(microsecond=0)
+
+
+def format_timestamp(value: datetime) -> str:
+ return
value.astimezone(timezone.utc).replace(microsecond=0).isoformat().replace("+00:00",
"Z")
+
+
+def default_account_state() -> dict[str, Any]:
+ return {
+ "status": AVAILABLE,
+ "last_failure_at": None,
+ "last_http_status": None,
+ "last_success_at": None,
+ "last_recovered_at": None,
+ "retry_after": None,
+ }
+
+
+def default_state() -> dict[str, Any]:
+ return {"version": STATE_VERSION, "next_account": 0, "accounts": {}}
+
+
+def load_state(path: Path) -> dict[str, Any]:
+ if not path.exists():
+ return default_state()
+
+ state = json.loads(path.read_text())
+ if not isinstance(state, dict):
+ raise ValueError("Authentication state must be a JSON object")
+ if state.get("version") != STATE_VERSION:
+ raise ValueError(f"Unsupported authentication state version:
{state.get('version')!r}")
+ if not isinstance(state.get("next_account"), int):
+ raise ValueError("Authentication state next_account must be an
integer")
+ if not isinstance(state.get("accounts"), dict):
+ raise ValueError("Authentication state accounts must be an object")
+ return state
+
+
+def save_state(path: Path, state: dict[str, Any]) -> None:
+ with tempfile.NamedTemporaryFile("w", encoding="utf-8", dir=path.parent,
delete=False) as temporary:
+ json.dump(state, temporary, indent=2, sort_keys=True)
+ temporary.write("\n")
+ temporary_path = Path(temporary.name)
+ temporary_path.replace(path)
+
+
+def account_state(state: dict[str, Any], auth_object: str) -> dict[str, Any]:
+ accounts = state["accounts"]
+ entry = accounts.setdefault(auth_object, default_account_state())
+ if not isinstance(entry, dict):
+ raise ValueError(f"Authentication state for {auth_object!r} must be an
object")
+ status = entry.get("status", AVAILABLE)
+ if status not in STATUS_VALUES:
+ raise ValueError(f"Unknown authentication status for {auth_object!r}:
{status!r}")
+ entry.setdefault("status", AVAILABLE)
+ entry.setdefault("last_failure_at", None)
+ entry.setdefault("last_http_status", None)
+ entry.setdefault("last_success_at", None)
+ entry.setdefault("last_recovered_at", None)
+ entry.setdefault("retry_after", None)
+ return entry
+
+
+def recover_expired_accounts(state: dict[str, Any], auth_objects: list[str],
now: datetime) -> None:
+ for auth_object in auth_objects:
+ entry = account_state(state, auth_object)
+ retry_after = entry["retry_after"]
+ if retry_after is None:
+ continue
+ if parse_timestamp(retry_after) <= now:
+ entry["status"] = AVAILABLE
+ entry["retry_after"] = None
+ entry["last_recovered_at"] = format_timestamp(now)
+
+
+def next_retry_at(state: dict[str, Any], auth_objects: list[str]) -> str |
None:
+ retry_at = [
+ entry["retry_after"]
+ for auth_object in auth_objects
+ for entry in [account_state(state, auth_object)]
+ if entry["retry_after"] is not None
+ ]
+ return min(retry_at, key=parse_timestamp, default=None)
+
+
+def all_quota_exhausted(state: dict[str, Any], auth_objects: list[str]) ->
bool:
+ return bool(auth_objects) and all(
+ account_state(state, auth_object)["status"] == QUOTA_EXHAUSTED for
auth_object in auth_objects
+ )
+
+
+def select_auth_object(state: dict[str, Any], auth_objects: list[str], now:
datetime) -> Selection:
+ if not auth_objects:
+ raise ValueError("At least one auth object is required")
+
+ recover_expired_accounts(state, auth_objects, now)
+ starting_index = state["next_account"] % len(auth_objects)
+ for offset in range(len(auth_objects)):
+ index = (starting_index + offset) % len(auth_objects)
+ auth_object = auth_objects[index]
+ if account_state(state, auth_object)["status"] == AVAILABLE:
+ state["next_account"] = (index + 1) % len(auth_objects)
+ return Selection(auth_object, False, None)
+
+ return Selection(None, all_quota_exhausted(state, auth_objects),
next_retry_at(state, auth_objects))
+
+
+def record_result(
+ state: dict[str, Any], auth_object: str, result: str, now: datetime,
http_status: int | None = None
+) -> None:
+ entry = account_state(state, auth_object)
+ timestamp = format_timestamp(now)
+ if result == AVAILABLE:
+ entry["status"] = AVAILABLE
+ entry["last_success_at"] = timestamp
+ entry["retry_after"] = None
+ return
+
+ if result == QUOTA_EXHAUSTED:
+ retry_delay = QUOTA_RETRY_DELAY
+ http_status = http_status or 403
+ elif result == RATE_LIMITED:
+ retry_delay = RATE_LIMIT_RETRY_DELAY
+ http_status = http_status or 429
+ elif result == AUTHENTICATION_FAILED:
+ retry_delay = AUTHENTICATION_RETRY_DELAY
+ http_status = http_status or 401
+ elif result == TRANSIENT_FAILURE:
+ entry["status"] = AVAILABLE
+ entry["last_failure_at"] = timestamp
+ entry["last_http_status"] = http_status
+ entry["retry_after"] = None
+ return
+ else:
+ raise ValueError(f"Unsupported result: {result!r}")
+
+ entry["status"] = result
+ entry["last_failure_at"] = timestamp
+ entry["last_http_status"] = http_status
+ entry["retry_after"] = format_timestamp(now + retry_delay)
+
+
+def extract_http_status(text: str) -> int | None:
+ labelled_statuses = re.findall(
+
r"(?:http(?:\s+status)?|status(?:\s+code)?|error\s+code)\s*[:=]?\s*([1-5]\d{2})",
+ text,
+ re.IGNORECASE,
+ )
+ if labelled_statuses:
+ return int(labelled_statuses[-1])
+ matches = re.findall(r"(?<!\d)([1-5]\d{2})(?!\d)", text)
+ return int(matches[0]) if matches else None
+
+
+def classify_failure(text: str) -> str:
+ http_status = extract_http_status(text)
+ if http_status == 429:
+ return RATE_LIMITED
+ if http_status in {402, 403}:
+ return QUOTA_EXHAUSTED
+ if http_status == 401:
+ return AUTHENTICATION_FAILED
+ if http_status in {408, 409, 425, 500, 502, 503, 504}:
+ return TRANSIENT_FAILURE
+ if re.search(
+ r"connection reset|econnreset|eai_again|network is
unreachable|etimedout|timeout|timed? out",
+ text,
+ re.IGNORECASE,
+ ):
+ return TRANSIENT_FAILURE
+ return "fatal"
+
+
+def parse_args() -> argparse.Namespace:
+ parser = argparse.ArgumentParser(description=__doc__)
+ subcommands = parser.add_subparsers(dest="command", required=True)
+
+ commands_with_state = (
+ subcommands.add_parser("initialize"),
+ subcommands.add_parser("select"),
+ subcommands.add_parser("record"),
+ )
+ for command in commands_with_state:
+ command.add_argument("--state", type=Path, required=True)
+ command.add_argument("--now", type=parse_timestamp)
+
+ initialize = subcommands.choices["initialize"]
+ initialize.add_argument("--auth-object", action="append", required=True)
+
+ select = subcommands.choices["select"]
+ select.add_argument("--auth-object", action="append", required=True)
+
+ record = subcommands.choices["record"]
+ record.add_argument("--auth-object", required=True)
+ record.add_argument(
+ "--result",
+ choices=[AVAILABLE, QUOTA_EXHAUSTED, RATE_LIMITED,
AUTHENTICATION_FAILED, TRANSIENT_FAILURE],
+ required=True,
+ )
+ record.add_argument("--http-status", type=int)
+ record.add_argument("--known-auth-object", action="append", required=True)
+
+ classify = subcommands.add_parser("classify")
+ classify.add_argument("--input", action="append", type=Path, required=True)
+ return parser.parse_args()
+
+
+def main() -> None:
+ args = parse_args()
+ if args.command == "classify":
+ text = "\n".join(path.read_text(errors="replace") for path in
args.input)
Review Comment:
[P1] Classify only the terminal Codex failure payload
These inputs are the complete JSONL/stderr streams, including command
`aggregated_output` and therefore arbitrary reviewed source text. A prior tool
result containing `HTTP 403` followed by an unrelated fatal `turn.failed` is
classified as `quota_exhausted`; the workflow then persists a cooldown for a
healthy account and can repeat this against every credential. Please parse
JSONL structurally and inspect only the latest terminal `turn.failed`/top-level
Codex error (with a tightly scoped stderr fallback), never command output or
arbitrary bare three-digit numbers, and add a contaminated-stream regression
test.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
