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 d8bbb991656 [fix](build) Quarantine invalidated Codex review refresh
tokens (#69737)
d8bbb991656 is described below
commit d8bbb991656a1b87a4792f120ee80470b0b36ca0
Author: shuke <[email protected]>
AuthorDate: Wed Oct 28 16:55:28 2026 +0800
[fix](build) Quarantine invalidated Codex review refresh tokens (#69737)
Related PR: #67635
Problem Summary:
Code Review Runner only detects and records `refresh_token_reused`.
Revoked sessions now fail with `refresh_token_invalidated` in Codex's
Rust `TokenErrorDetail` stderr, while the terminal error is only
`workspace routing discovery unauthorized (401)`. The invalid-auth
recording step is skipped, so subsequent reviews keep selecting the
failed credential. For example, [run
40643700741](https://github.com/apache/doris/actions/runs/40643700741)
reproduces this sequence.
Recognize both permanent refresh failures in terminal events and Codex
stderr, including `error_code: Some(...)`. Pass the detected reason
through the quarantine condition, marker JSON, and failure notification.
Preserve the existing per-refresh-token hash marker so replacing a token
restores eligibility and late failures cannot quarantine its
replacement.
Generic 401/`token_revoked`, tool-output quotations, and successful
reviews remain excluded from permanent quarantine. Only the workflow and
its offline tests change. The tests also supply the required convergence
fields in the fake final submission so existing success-path assertions
can execute.
---
.github/scripts/test_review_auth_quarantine.py | 131 +++++++++++++++++++++++--
.github/workflows/code-review-runner.yml | 53 ++++++----
2 files changed, 158 insertions(+), 26 deletions(-)
diff --git a/.github/scripts/test_review_auth_quarantine.py
b/.github/scripts/test_review_auth_quarantine.py
index d458b6dbf3b..6557a6af9df 100755
--- a/.github/scripts/test_review_auth_quarantine.py
+++ b/.github/scripts/test_review_auth_quarantine.py
@@ -39,6 +39,13 @@ from pathlib import Path
WORKFLOW = Path(__file__).resolve().parents[1] /
"workflows/code-review-runner.yml"
PREFIX = "oss://doris-community-ci/codex/"
+INVALIDATED_MESSAGE = "Your session has ended. Please log in again."
+INVALIDATED_STDERR = (
+ '2026-10-28T06:26:31.373554Z ERROR codex_login::auth::manager: '
+ 'Failed to refresh token status=401 Unauthorized detail=TokenErrorDetail {
'
+ 'error_code: Some("refresh_token_invalidated"), '
+ 'error_message: Some("Your session has ended. Please log in again."), .. }'
+)
REUSED_MESSAGE = (
"Your access token could not be refreshed because your refresh token was
already used. "
"Please log out and sign in again."
@@ -106,7 +113,9 @@ import sys
if os.environ.get("FAKE_FINAL_REVIEW") == "1":
import json, subprocess
request = Path(os.environ["REVIEW_CONTEXT_DIR"], "final-input.json")
- request.write_text(json.dumps({"body": "Complete review", "comments": [],
"existing_blocking_comment_ids": []}))
+ request.write_text(json.dumps({"body": "Complete review", "comments": [],
+ "existing_blocking_comment_ids": [],
"converged": True,
+ "rounds": 1, "incomplete_reason": ""}))
subprocess.run([sys.executable, str(Path(os.environ["RUNNER_TEMP"],
"submit_review.py")),
"--context-dir", os.environ["REVIEW_CONTEXT_DIR"],
"--input-file", str(request)],
check=True, stdout=subprocess.DEVNULL)
@@ -209,10 +218,13 @@ class ReviewAuthQuarantineTest(unittest.TestCase):
context = Path(tempfile.mkdtemp(dir=self.env["RUNNER_TEMP"]))
(context / "codex_goal_prompt.txt").write_text("Fake review")
self.env["REVIEW_CONTEXT_DIR"] = str(context)
+ return self.run_script(step_script(name), expected=expected, **env)
+
+ def run_script(self, script, expected=0, **env):
output = Path(self.env["GITHUB_OUTPUT"])
output.write_text("")
result = subprocess.run(
- ["bash", "--noprofile", "--norc", "-eo", "pipefail", "-c",
step_script(name)],
+ ["bash", "--noprofile", "--norc", "-eo", "pipefail", "-c", script],
env={**self.env, **env}, cwd=self.root, capture_output=True,
text=True, timeout=15, check=False,
)
self.assertEqual(result.returncode, expected, result.stdout +
result.stderr)
@@ -239,7 +251,8 @@ class ReviewAuthQuarantineTest(unittest.TestCase):
def configure(self, expected=0, **env):
return self.run_step("Configure Codex auth", expected=expected, **env)
- def fail_review(self, events=None, stderr="", expected_invalid=True,
**env):
+ def fail_review(self, events=None, stderr="", expected_invalid=True,
+ expected_reason="refresh_token_reused", **env):
if events is None:
events = [{"type": "turn.failed", "error": {"message":
REUSED_MESSAGE}}]
logs, outputs = self.run_step(
@@ -247,11 +260,106 @@ class ReviewAuthQuarantineTest(unittest.TestCase):
FAKE_CODEX_EVENTS="\n".join(json.dumps(event) for event in events),
FAKE_CODEX_STDERR=stderr, **env,
)
- self.assertEqual("auth_invalid_reason=refresh_token_reused\n" in
outputs, expected_invalid, outputs)
+ self.assertEqual(f"auth_invalid_reason={expected_reason}\n" in
outputs, expected_invalid, outputs)
return logs, outputs
- def record(self, expected=0, **env):
- return self.run_step("Record invalid Codex auth", expected=expected,
**env)
+ def record(self, expected=0, reason="refresh_token_reused", **env):
+ return self.run_step("Record invalid Codex auth", expected=expected,
+ AUTH_INVALID_REASON=reason, **env)
+
+ def classify_failure(self, events=(), stderr="", status=1):
+ # Execute the real workflow's post-run shell independently of Linux
+ # process supervision, so classification and OSS tests also run on
macOS.
+ context = Path(self.env["REVIEW_CONTEXT_DIR"])
+ (context / "codex-events.jsonl").write_text(
+ "\n".join(json.dumps(event) for event in events))
+ (context / "codex-stderr.log").write_text(stderr)
+ (context / "review-result.json").write_text(json.dumps({
+ "state": "success", "p0": 0, "p1": 0, "converged": True, "rounds":
1,
+ }))
+ script = step_script("Run automated code review")
+ script = script[script.index('failure_reason=""'):]
+ _, outputs = self.run_script(script, expected=int(status != 0),
status=str(status))
+ return next((line.split("=", 1)[1] for line in outputs.splitlines()
+ if line.startswith("auth_invalid_reason=")), "")
+
+ def
test_invalidated_stderr_is_quarantined_and_replacement_is_eligible(self):
+ self.put_auth()
+ self.put_auth(3, "refresh-other")
+ self.configure()
+ # The terminal message hides the permanent refresh failure on stderr.
+ reason = self.classify_failure(events=[{
+ "type": "turn.failed", "error": {"message": "workspace routing
discovery unauthorized (401)"},
+ }], stderr=INVALIDATED_STDERR)
+ self.assertEqual(reason, "refresh_token_invalidated")
+ self.record(reason=reason)
+ marker = json.loads(self.marker().read_text())
+ self.assertEqual(marker["reason"], reason)
+ self.assertNotIn("refresh-old", self.marker().read_text())
+ self.new_runner()
+ logs, _ = self.configure()
+ self.assertIn("Skipping Codex auth auth.json.2", logs)
+ self.assertEqual(self.env["CODEX_AUTH_OSS_OBJECT"], PREFIX +
"auth.json.3")
+ self.put_auth(refresh="refresh-new")
+ self.new_runner()
+ self.configure()
+ self.assertEqual(self.env["CODEX_AUTH_OSS_OBJECT"], PREFIX +
"auth.json.2")
+ self.assertFalse(self.marker(refresh="refresh-new").exists())
+
+ def test_permanent_auth_codes_in_terminal_events_and_stderr(self):
+ for code, message in (("refresh_token_reused", REUSED_MESSAGE),
+ ("refresh_token_invalidated",
INVALIDATED_MESSAGE)):
+ samples = [
+ ([{"type": "turn.failed", "error": {"code": code, "message":
"Refresh failed"}}], ""),
+ ([{"type": "error", "code": code, "message": "Refresh
failed"}], ""),
+ ([{"type": "error", "message": code}], ""),
+ ([{"type": "error", "message": message}], ""),
+ ([], json.dumps({"error": {"code": code}})),
+ ([], INVALIDATED_STDERR.replace("refresh_token_invalidated",
code)),
+ ]
+ for events, stderr in samples:
+ with self.subTest(code=code, events=events, stderr=stderr):
+ self.assertEqual(self.classify_failure(events, stderr),
code)
+
+ def test_auth_like_tool_output_and_generic_401_do_not_quarantine(self):
+ for message in ("401 Unauthorized", "workspace routing discovery
unauthorized (401)",
+ "token_revoked", "Request timed out", "You've hit your
usage limit"):
+ with self.subTest(message=message):
+ self.assertEqual(self.classify_failure(events=[
+ {"type": "item.completed", "item": {"type":
"command_execution",
+ "aggregated_output": INVALIDATED_STDERR +
INVALIDATED_MESSAGE}},
+ {"type": "turn.failed", "error": {"message": message}},
+ ], stderr="ERROR codex_models_manager::manager: auth error
code: token_revoked"), "")
+
+ def test_success_does_not_quarantine_prior_invalidated_stderr(self):
+ self.assertEqual(self.classify_failure(
+ events=[{"type": "error", "code": "refresh_token_invalidated"},
+ {"type": "turn.completed"}], stderr=INVALIDATED_STDERR,
status=0), "")
+
+ def test_invalidated_marker_write_failure_and_notification(self):
+ self.put_auth()
+ self.configure()
+ self.record(expected=1, reason="refresh_token_invalidated",
+ FAKE_OSS_FAIL="cp:" + PREFIX + self.marker().name)
+ self.assertFalse(self.marker().exists())
+ for outcome in ("success", "failure"):
+ with self.subTest(outcome=outcome):
+ self.run_step(
+ "Comment PR on review failure", AUTH_FAILURE_REASON="",
+ AUTH_INVALID_REASON="refresh_token_invalidated",
AUTH_INVALID_RECORD_OUTCOME=outcome,
+ REVIEW_FAILURE_REASON=INVALIDATED_MESSAGE,
REVIEW_CONTEXT_OUTCOME="success",
+ REVIEW_OUTCOME="failure", USAGE_LIMIT_RETRY_AFTER_EPOCH="",
+ RUN_URL="https://example.test/run";,
+ )
+ body = Path(self.env["FAKE_COMMENT_FILE"]).read_text()
+ self.assertEqual("excluded from future reviews" in body,
outcome == "success")
+ self.assertEqual("may still be selected" in body, outcome ==
"failure")
+
+ def test_unknown_auth_reason_cannot_write_marker(self):
+ self.put_auth()
+ self.configure()
+ self.record(expected=1, reason="401 Unauthorized")
+ self.assertFalse(self.marker().exists())
def test_reused_token_is_persisted_and_skipped_on_next_run(self):
self.put_auth()
@@ -343,7 +451,7 @@ class ReviewAuthQuarantineTest(unittest.TestCase):
}))
_, outputs = self.configure(expected=1)
self.assertIn("Earliest usage-limit retry", outputs)
- self.assertIn("reused refresh tokens must be replaced", outputs)
+ self.assertIn("invalid refresh tokens must be replaced", outputs)
def test_marker_lookup_failure_does_not_select_token(self):
self.put_auth()
@@ -388,6 +496,11 @@ class ReviewAuthQuarantineTest(unittest.TestCase):
def test_codex_stderr_reused_code(self):
self.fail_review(events=[], stderr='Token refresh failed: 401
Unauthorized\n{\n "error": {\n "code": "refresh_token_reused"\n }\n}')
+ def
test_codex_stderr_invalidated_code_survives_runner_failure_summary(self):
+ self.fail_review(events=[{
+ "type": "error", "message": "workspace routing discovery
unauthorized (401)",
+ }], stderr=INVALIDATED_STDERR,
expected_reason="refresh_token_invalidated")
+
def test_unrelated_failures_are_not_quarantined(self):
for message in ("401 Unauthorized", "We're experiencing high demand",
"Request timed out", "You've hit your usage limit"):
with self.subTest(message=message):
@@ -465,7 +578,9 @@ class ReviewAuthQuarantineTest(unittest.TestCase):
def
test_marker_step_runs_after_failed_review_with_explicit_detection(self):
step = workflow_step("Record invalid Codex auth")
self.assertIn("always() && steps.auth.outcome == 'success'", step)
- self.assertIn("steps.review.outputs.auth_invalid_reason ==
'refresh_token_reused'", step)
+ self.assertIn("contains(fromJSON(", step)
+ self.assertIn('"refresh_token_reused","refresh_token_invalidated"',
step)
+ self.assertIn("steps.review.outputs.auth_invalid_reason)", step)
self.assertNotIn("continue-on-error: true", step)
diff --git a/.github/workflows/code-review-runner.yml
b/.github/workflows/code-review-runner.yml
index 0db0b373032..24e21cc2524 100644
--- a/.github/workflows/code-review-runner.yml
+++ b/.github/workflows/code-review-runner.yml
@@ -471,7 +471,7 @@ jobs:
exit 1
fi
if grep -Fxq "$invalid_object" "$invalid_listing"; then
- echo "Skipping Codex auth ${candidate##*/}: refresh token was
already used."
+ echo "Skipping Codex auth ${candidate##*/}: refresh token is
marked invalid."
rm -f "$candidate_auth"
continue
fi
@@ -484,12 +484,12 @@ jobs:
done < <(shuf "$auth_candidates")
if [ -z "$auth_object" ]; then
- auth_failure_reason="No eligible Codex review credentials: all are
usage-limited or have a reused refresh token."
+ auth_failure_reason="No eligible Codex review credentials: all are
usage-limited or have an invalid refresh token."
if [ -n "$earliest_retry_after_epoch" ]; then
retry_at="$(date -u -d "@$earliest_retry_after_epoch"
+%Y-%m-%dT%H:%M:%SZ)"
auth_failure_reason="$auth_failure_reason Earliest usage-limit
retry is $retry_at."
fi
- auth_failure_reason="$auth_failure_reason Credentials with reused
refresh tokens must be replaced."
+ auth_failure_reason="$auth_failure_reason Credentials with invalid
refresh tokens must be replaced."
echo "failure_reason=$auth_failure_reason" >> "$GITHUB_OUTPUT"
echo "::error::$auth_failure_reason"
exit 1
@@ -951,16 +951,28 @@ jobs:
failure_reason="Codex exited with status $status"
fi
- # Only terminal auth errors and Codex's own stderr identify an
invalid
- # token. A tool result quoting this error must not quarantine
credentials.
- if grep -Fqi "Your access token could not be refreshed because
your refresh token was already used." <<<"$failure_reason" || \
- grep -Fxq "refresh_token_reused" <<<"$failure_reason" || \
- { jq -r 'select(.type == "turn.failed" or .type == "error") |
.error.code // .code // empty' \
- "$REVIEW_CONTEXT_DIR/codex-events.jsonl" | grep -Fx
"refresh_token_reused" >/dev/null; } || \
- grep -Eq
'"code"[[:space:]]*:[[:space:]]*"refresh_token_reused"'
"$REVIEW_CONTEXT_DIR/codex-stderr.log"; then
- echo "auth_invalid_reason=refresh_token_reused" >>
"$GITHUB_OUTPUT"
- echo "::warning::Codex refresh token was already used; recording
this credential version as invalid."
- fi
+ # Only permanent refresh failures quarantine the selected token.
+ # Access-token 401/token_revoked alone can still recover by
refresh;
+ # tool results quoting auth errors are not authentication evidence.
+ for auth_invalid_reason in refresh_token_reused
refresh_token_invalidated; do
+ case "$auth_invalid_reason" in
+ refresh_token_reused)
+ auth_invalid_message="Your access token could not be
refreshed because your refresh token was already used." ;;
+ refresh_token_invalidated)
+ auth_invalid_message="Your session has ended. Please log in
again." ;;
+ esac
+ if grep -Fqi "$auth_invalid_message" <<<"$failure_reason" || \
+ grep -Fxq "$auth_invalid_reason" <<<"$failure_reason" || \
+ { jq -r 'select(.type == "turn.failed" or .type == "error") |
.error.code // .code // empty' \
+ "$REVIEW_CONTEXT_DIR/codex-events.jsonl" | grep -Fx
"$auth_invalid_reason" >/dev/null; } || \
+ grep -Eq
"\"code\"[[:space:]]*:[[:space:]]*\"${auth_invalid_reason}\""
"$REVIEW_CONTEXT_DIR/codex-stderr.log" || \
+ grep -Eq "ERROR codex_login::auth(::manager)?: Failed to
refresh token .*error_code: Some\(\"${auth_invalid_reason}\"\)" \
+ "$REVIEW_CONTEXT_DIR/codex-stderr.log"; then
+ echo "auth_invalid_reason=$auth_invalid_reason" >>
"$GITHUB_OUTPUT"
+ echo "::warning::Codex refresh failed ($auth_invalid_reason);
recording this credential version as invalid."
+ break
+ fi
+ done
usage_limit_retry_after_epoch=""
if grep -Fqi "You've hit your usage limit" <<<"$failure_reason";
then
@@ -1017,20 +1029,25 @@ jobs:
- name: Record invalid Codex auth
id: auth_invalid_record
- if: ${{ always() && steps.auth.outcome == 'success' &&
steps.review.outputs.auth_invalid_reason == 'refresh_token_reused' }}
+ if: ${{ always() && steps.auth.outcome == 'success' &&
contains(fromJSON('["refresh_token_reused","refresh_token_invalidated"]'),
steps.review.outputs.auth_invalid_reason) }}
timeout-minutes: 5
env:
+ AUTH_INVALID_REASON: ${{ steps.review.outputs.auth_invalid_reason }}
OSS_AK: ${{ secrets.OSS_AK }}
OSS_SK: ${{ secrets.OSS_SK }}
OSS_ENDPOINT: oss-cn-hongkong.aliyuncs.com
run: |
+ case "$AUTH_INVALID_REASON" in
+ refresh_token_reused|refresh_token_invalidated) ;;
+ *) echo "::error::Unsupported Codex auth invalidation reason.";
exit 1 ;;
+ esac
# Bind the marker to the selected snapshot, so a late failing job
cannot
# invalidate a replacement uploaded by another job or an operator.
refresh_token_hash="$(<"$RUNNER_TEMP/codex-auth-refresh-token.sha256")"
[[ "$refresh_token_hash" =~ ^[0-9a-f]{64}$ ]]
invalid_file="$(mktemp "$RUNNER_TEMP/codex-auth-invalid.XXXXXX")"
- jq -n --arg refresh_token_sha256 "$refresh_token_hash" \
- '{version: 1, state: "auth_invalid", reason:
"refresh_token_reused", refresh_token_sha256: $refresh_token_sha256}' \
+ jq -n --arg refresh_token_sha256 "$refresh_token_hash" --arg reason
"$AUTH_INVALID_REASON" \
+ '{version: 1, state: "auth_invalid", reason: $reason,
refresh_token_sha256: $refresh_token_sha256}' \
> "$invalid_file"
# A separate object per token is idempotent and cannot be
overwritten by
@@ -1083,8 +1100,8 @@ jobs:
recovery_msg="Please inspect the workflow logs and rerun the review
after the underlying issue is resolved."
if [ -n "$AUTH_FAILURE_REASON" ]; then
error_msg="$AUTH_FAILURE_REASON"
- elif [ "$AUTH_INVALID_REASON" = "refresh_token_reused" ]; then
- error_msg="${REVIEW_FAILURE_REASON:-The selected Codex refresh
token was already used.}"
+ elif [ "$AUTH_INVALID_REASON" = "refresh_token_reused" ] || [
"$AUTH_INVALID_REASON" = "refresh_token_invalidated" ]; then
+ error_msg="${REVIEW_FAILURE_REASON:-The selected Codex refresh
token is invalid.}"
if [ "$AUTH_INVALID_RECORD_OUTCOME" = "success" ]; then
recovery_msg="This credential version is excluded from future
reviews. Replace its refresh token to restore eligibility. Please trigger
/review again; another configured account may be available."
else
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
