From 28b6470ab751f2d246239f7b830c0f67d1ddb694 Mon Sep 17 00:00:00 2001 From: Matthew Grossman Date: Wed, 4 Nov 2026 21:14:35 -0800 Subject: [PATCH 1/4] fix(ci): expose compatibility review failure diagnostics Signed-off-by: Matthew Grossman --- .agents/skills/watch-github-actions/SKILL.md | 7 +- CI.md | 8 + tasks/scripts/codex_compatibility.py | 146 ++++++++- tasks/scripts/codex_compatibility_report.py | 9 + tasks/scripts/codex_compatibility_test.py | 302 +++++++++++++++++++ tasks/test.toml | 6 + 6 files changed, 469 insertions(+), 9 deletions(-) create mode 100644 tasks/scripts/codex_compatibility_test.py diff --git a/.agents/skills/watch-github-actions/SKILL.md b/.agents/skills/watch-github-actions/SKILL.md index 6435a119ea..0c2c7d8f86 100644 --- a/.agents/skills/watch-github-actions/SKILL.md +++ b/.agents/skills/watch-github-actions/SKILL.md @@ -235,7 +235,12 @@ For `Codex Compatibility Review`, inspect its job summary and the candidate and baseline SHAs before interpreting findings. `complete` describes execution and coverage, not compatibility: also read the assessment and findings. An `error`, incomplete coverage, or missing artifact means no clean review is -available. Findings are advisory. The review targets breaking changes with high +available. Findings are advisory. For failed execution, inspect `report.json`'s `diagnostics` +or the execution diagnostics in the job log/summary: phase, exception type, +last/final SDK code and upstream HTTP status, and retry counts. SDK error text +and transcripts are deliberately omitted. Retry events alone do not imply failure +if the review ultimately completed. Context-resolution merge conflicts are not +transient SDK failures. The review targets breaking changes with high reasoning effort. Risk acceptance needs a linked author or verified-maintainer comment or review, not a PR description, general approval, or bot report. The full report keeps all findings and prior-comment links. `comment.md` includes diff --git a/CI.md b/CI.md index e9384acf15..b851843273 100644 --- a/CI.md +++ b/CI.md @@ -144,6 +144,14 @@ Each run retains `report.json` and `report.md` for 90 days in `compatibility-review-run--attempt-`. The report records both revisions, the train, tool/model metadata, coverage gaps, and actionable findings. Raw agent transcripts and PR discussions are not published. +The report's `diagnostics` object and job log record the last execution phase, +elapsed seconds, exception type, turn status, retry/error notification counts, +and the last and final structured SDK error codes and upstream HTTP statuses +when available. A merge conflict is identified separately during context +resolution. SDK messages, additional details, stderr, and rejected review output +are not retained because they can contain source or credentials. A successful +review can still have retry notifications; use `final_error` and the execution +status to distinguish recovery from failure. The qualification summary links this separate advisory evidence without waiting for it; compatibility review remains missing from the blocking RFC profile during this rollout. diff --git a/tasks/scripts/codex_compatibility.py b/tasks/scripts/codex_compatibility.py index 99d64698e2..33fbd8c07a 100644 --- a/tasks/scripts/codex_compatibility.py +++ b/tasks/scripts/codex_compatibility.py @@ -10,6 +10,8 @@ import re import subprocess import tempfile +import time +from contextlib import aclosing from importlib.metadata import version from pathlib import Path @@ -17,11 +19,106 @@ import openai_codex # ty: ignore[unresolved-import] from check_proto_compatibility import git, train_policy from codex_compatibility_report import validate_review, write_report +from openai_codex.generated.v2_all import ( # ty: ignore[unresolved-import] + AgentMessageThreadItem, + ErrorNotification, + ItemCompletedNotification, + MessagePhase, + TurnCompletedNotification, +) ASSETS = Path(__file__).resolve().parents[2] / ".github" / "prompts" MODEL = "openai/openai/gpt-6.1-sol" ENDPOINT = "https://inference-api.nvidia.com/v1" +# Error messages/details may contain source, tool output, or credentials. Publish +# only protocol-defined states, never arbitrary strings from the SDK or model. +ERROR_CODES = frozenset( + { + "contextWindowExceeded", + "sessionBudgetExceeded", + "usageLimitExceeded", + "rateLimitExceeded", + "flexUnavailable", + "serverOverloaded", + "cyberPolicy", + "misalignmentPolicyViolation", + "tooManyDenials", + "internalServerError", + "unauthorized", + "badRequest", + "threadRollbackFailed", + "sandboxError", + "other", + "httpConnectionFailed", + "responseStreamConnectionFailed", + "responseStreamDisconnected", + "responseTooManyFailedAttempts", + "activeTurnNotSteerable", + } +) + + +def public_turn_error(error) -> dict: + """Extract only allowlisted protocol fields, not message/additionalDetails.""" + info = error.codex_error_info + value = info.model_dump(mode="json", by_alias=True) if info is not None else None + code = value if isinstance(value, str) else None + status = None + if isinstance(value, dict) and len(value) == 1: + code, details = next(iter(value.items())) + if code in ERROR_CODES and isinstance(details, dict): + status = details.get("httpStatusCode", details.get("http_status_code")) + return { + "code": code if code in ERROR_CODES else "unknown", + "http_status": status if type(status) is int and 100 <= status <= 599 else None, + } + + +async def collect_review(thread, prompt: str, schema: dict, diagnostics: dict) -> str: + """Consume the public turn stream before the SDK erases structured errors.""" + diagnostics["phase"] = "turn_start" + turn = await thread.turn(prompt, output_schema=schema) + diagnostics["phase"] = "turn_stream" + completed = None + final_response = None + fallback_response = None + async with aclosing(turn.stream()) as stream: + async for event in stream: + payload = event.payload + if isinstance(payload, ErrorNotification): + diagnostics["error_events"] += 1 + diagnostics["retry_events"] += int(payload.will_retry) + diagnostics["last_error"] = public_turn_error(payload.error) + elif isinstance(payload, TurnCompletedNotification): + completed = payload.turn + status = completed.status.value + diagnostics["turn_status"] = ( + status + if status in ("completed", "failed", "interrupted", "inProgress") + else "unknown" + ) + if completed.error is not None: + diagnostics["final_error"] = public_turn_error(completed.error) + elif isinstance(payload, ItemCompletedNotification): + item = payload.item.root + if isinstance(item, AgentMessageThreadItem): + if item.phase == MessagePhase.final_answer: + final_response = item.text + elif item.phase is None: + fallback_response = item.text + if completed is None: + diagnostics["reason"] = "missing_completion" + raise ValueError("Codex turn ended without a completion event.") + if completed.status.value != "completed": + diagnostics["reason"] = "unsuccessful_turn" + raise ValueError("Codex did not complete the compatibility review.") + response = final_response if final_response is not None else fallback_response + if not response: + diagnostics["reason"] = "missing_final_response" + raise ValueError("Codex completed without a final compatibility report.") + return response + def resolve_candidate(tag: str, expected_sha: str) -> dict: if not re.fullmatch(r"v0\.\d+\.\d+(?:-pre\.\d+)?", tag): @@ -66,9 +163,10 @@ def resolve_branch(base_ref: str, source_sha: str, pr_number: int) -> dict: } -async def invoke_codex(context: dict, directory: Path) -> dict: +async def invoke_codex(context: dict, directory: Path, diagnostics: dict) -> dict: # Git objects expose both revisions without loading candidate-side agent # instructions, configuration, skills, or hooks into the review workspace. + diagnostics["phase"] = "clone_source" subprocess.run( [ "git", @@ -109,16 +207,17 @@ async def invoke_codex(context: dict, directory: Path) -> dict: 'shell_environment_policy.filters={PATH="include", HOME="include", GH_TOKEN="include", ' 'SSL_CERT_FILE="include", SSL_CERT_DIR="include"}', ) + diagnostics["phase"] = "load_prompt" prompt = (ASSETS / "compatibility-review.md").read_text() prompt += "\nReview context:\n" + json.dumps(context) schema = json.loads((ASSETS / "compatibility-review.schema.json").read_text()) codex = openai_codex.AsyncCodex( openai_codex.CodexConfig(cwd=str(directory), config_overrides=settings) ) - # Raw agent output can include inspected source or tool output. Publish only - # the validated final report, never the transcript or credential environment. + # Retain structured failure states, but never SDK messages or transcripts. try: async with asyncio.timeout(1800): + diagnostics["phase"] = "thread_start" thread = await codex.thread_start( cwd=str(directory), model=MODEL, @@ -126,16 +225,19 @@ async def invoke_codex(context: dict, directory: Path) -> dict: approval_mode=openai_codex.ApprovalMode.deny_all, ephemeral=True, ) - result = await thread.run(prompt, output_schema=schema) + response = await collect_review(thread, prompt, schema, diagnostics) except (openai_codex.CodexError, RuntimeError, ValueError) as error: + diagnostics["error_type"] = type(error).__name__ + if isinstance(error, openai_codex.JsonRpcError) and type(error.code) is int: + diagnostics["rpc_code"] = error.code raise ValueError( f"Codex SDK review failed ({type(error).__name__}); no assessment is available." ) from None finally: await codex.close() - if result.status.value != "completed" or not result.final_response: - raise ValueError("Codex did not complete the compatibility review.") - review = json.loads(result.final_response) + diagnostics["phase"] = "decode_report" + review = json.loads(response) + diagnostics["phase"] = "validate_report" validate_review(review) return review @@ -143,6 +245,12 @@ async def invoke_codex(context: dict, directory: Path) -> dict: def run_review( tag: str, expected_sha: str, output: Path, base_ref: str, pr_number: int ) -> dict: + started = time.monotonic() + diagnostics = { + "phase": "resolve_context", + "error_events": 0, + "retry_events": 0, + } result = { "schema_version": 3, "advisory": True, @@ -150,6 +258,7 @@ def run_review( "context": {}, "review": None, "error": "", + "diagnostics": diagnostics, "model": MODEL, "reasoning_effort": "high", "codex_version": "", @@ -164,8 +273,10 @@ def run_review( if base_ref else resolve_candidate(tag, expected_sha) ) + diagnostics["phase"] = "credentials" if not os.environ.get("NVIDIA_INFERENCE_API_KEY"): raise ValueError("NVIDIA inference credential is missing.") + diagnostics["phase"] = "tool_versions" result["codex_version"] = version("openai-codex-cli-bin") result["codex_sdk_version"] = version("openai-codex") result["context"]["repository"] = os.environ.get("GITHUB_REPOSITORY", "") @@ -174,25 +285,40 @@ def run_review( prefix="openshell-compatibility-" ) as temporary: result["review"] = asyncio.run( - invoke_codex(result["context"], Path(temporary)) + invoke_codex(result["context"], Path(temporary), diagnostics) ) result["status"] = ( "incomplete" if result["review"]["unreviewed_surfaces"] else "complete" ) + diagnostics["phase"] = "complete" except TimeoutError: + diagnostics["error_type"] = "TimeoutError" + diagnostics["reason"] = "timeout" result["error"] = "Compatibility review exceeded its time limit." except subprocess.CalledProcessError as error: + diagnostics["error_type"] = "CalledProcessError" + diagnostics["exit_code"] = error.returncode + if ( + isinstance(error.cmd, (list, tuple)) + and tuple(error.cmd[:2]) == ("git", "merge-tree") + and error.returncode == 1 + ): + diagnostics["reason"] = "merge_conflict" result["error"] = ( f"Review tooling failed (exit {error.returncode}); no assessment is available." ) except json.JSONDecodeError: + diagnostics["error_type"] = "JSONDecodeError" result["error"] = "Codex did not return a valid JSON report." except ValueError as error: + diagnostics.setdefault("error_type", "ValueError") result["error"] = str(error) except OSError as error: + diagnostics["error_type"] = type(error).__name__ result["error"] = ( f"Review input, tool, or output unavailable ({type(error).__name__})." ) + diagnostics["elapsed_seconds"] = round(time.monotonic() - started, 1) write_report(output, result) return result @@ -213,6 +339,10 @@ def main() -> None: args.candidate, args.source_sha, args.output_dir, args.base_ref, args.pr_number ) verdict = result["review"]["verdict"] if result["review"] else "unavailable" + print( + "Compatibility review execution diagnostics: " + + json.dumps(result["diagnostics"]) + ) has_findings = bool( result["review"] and any(not f["previous_report_url"] for f in result["review"]["findings"]) diff --git a/tasks/scripts/codex_compatibility_report.py b/tasks/scripts/codex_compatibility_report.py index 1c959ef909..c9a436c31a 100644 --- a/tasks/scripts/codex_compatibility_report.py +++ b/tasks/scripts/codex_compatibility_report.py @@ -179,6 +179,15 @@ def write_report(output: Path, result: dict) -> None: f"- Baseline: `{context.get('baseline_tag', 'unresolved')}` " f"(`{context.get('baseline_sha', 'unresolved')}`)", ] + if diagnostics := result.get("diagnostics"): + details += [ + "", + "Execution diagnostics (fixed states and numeric metadata only):", + "", + "```json", + json.dumps(diagnostics, indent=2), + "```", + ] if review: details.append( f"- Reviewed: {', '.join(review['reviewed_surfaces']) or 'none'}" diff --git a/tasks/scripts/codex_compatibility_test.py b/tasks/scripts/codex_compatibility_test.py new file mode 100644 index 0000000000..e38039cd3b --- /dev/null +++ b/tasks/scripts/codex_compatibility_test.py @@ -0,0 +1,302 @@ +# SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +"""Exercise the pinned SDK's public notification types without inference calls.""" + +import json +import subprocess +import sys +from pathlib import Path +from types import SimpleNamespace +from unittest.mock import AsyncMock + +import openai_codex +import pytest +from openai_codex.generated.v2_all import ( + ErrorNotification, + ItemCompletedNotification, + TurnCompletedNotification, + TurnError, +) + +sys.path.insert(0, str(Path(__file__).resolve().parent)) + +import codex_compatibility as compatibility +from codex_compatibility_report import SURFACES + +SENSITIVE = "PRIVATE_source_token_tool_output" +REVIEW = { + "verdict": "no_unaddressed_breaks", + "summary": "No changes to public contracts.", + "reviewed_surfaces": list(SURFACES), + "unreviewed_surfaces": [], + "findings": [], +} + + +def turn_error(info): + return TurnError( + message=SENSITIVE, + additionalDetails=SENSITIVE, + codexErrorInfo=info, + ) + + +def completed(status="completed", error=None): + return TurnCompletedNotification( + threadId=SENSITIVE, + turn={"id": "turn", "status": status, "error": error, "items": []}, + ) + + +def message(text, phase="final_answer"): + return ItemCompletedNotification( + threadId=SENSITIVE, + turnId="turn", + completedAtMs=0, + item={"type": "agentMessage", "id": "message", "text": text, "phase": phase}, + ) + + +class FakeTurn: + def __init__(self, events): + self.events = events + self.closed = False + + async def stream(self): + try: + for payload in self.events: + if isinstance(payload, Exception): + raise payload + yield SimpleNamespace(payload=payload) + finally: + self.closed = True + + +@pytest.fixture +def runner(monkeypatch, tmp_path): + """Run the entire report pipeline with fake SDK I/O, not fake SDK types.""" + monkeypatch.setenv("NVIDIA_INFERENCE_API_KEY", SENSITIVE) + monkeypatch.setattr(compatibility, "version", lambda _name: "0.160.0") + monkeypatch.setattr( + compatibility, "resolve_branch", lambda *_args: {"source_sha": "a" * 40} + ) + monkeypatch.setattr(compatibility.subprocess, "run", lambda *_args, **_kwargs: None) + + def run(events, start_error=None): + turn = FakeTurn(events) + thread = SimpleNamespace(turn=AsyncMock(return_value=turn)) + codex = SimpleNamespace( + thread_start=AsyncMock(return_value=thread, side_effect=start_error), + close=AsyncMock(), + ) + monkeypatch.setattr( + compatibility.openai_codex, "AsyncCodex", lambda _config: codex + ) + result = compatibility.run_review("candidate", "a" * 40, tmp_path, "base", 1) + codex.close.assert_awaited_once() + # Nothing unvalidated, including SDK messages or IDs, reaches artifacts. + for artifact in tmp_path.iterdir(): + assert SENSITIVE not in artifact.read_text() + assert json.loads((tmp_path / "report.json").read_text()) == result + return result, turn + + return run + + +@pytest.mark.parametrize( + ("info", "code", "status"), + [ + ("rateLimitExceeded", "rateLimitExceeded", None), + ("contextWindowExceeded", "contextWindowExceeded", None), + ( + {"httpConnectionFailed": {"httpStatusCode": 429}}, + "httpConnectionFailed", + 429, + ), + ( + {"responseStreamConnectionFailed": {"httpStatusCode": 502}}, + "responseStreamConnectionFailed", + 502, + ), + ( + {"responseStreamDisconnected": {"httpStatusCode": 200}}, + "responseStreamDisconnected", + 200, + ), + ( + {"responseTooManyFailedAttempts": {"httpStatusCode": 503}}, + "responseTooManyFailedAttempts", + 503, + ), + (None, "unknown", None), + ( + {"httpConnectionFailed": {"httpStatusCode": 999}}, + "httpConnectionFailed", + None, + ), + ], +) +def test_failed_turn_retains_only_structured_error(runner, info, code, status): + error = turn_error(info) + notification = ErrorNotification( + threadId=SENSITIVE, turnId="turn", error=error, willRetry=False + ) + result, turn = runner([notification, completed("failed", error)]) + assert turn.closed + assert result["status"] == "error" + assert result["review"] is None + diagnostics = result["diagnostics"] + assert diagnostics["phase"] == "turn_stream" + assert diagnostics["final_error"] == {"code": code, "http_status": status} + assert diagnostics["last_error"] == diagnostics["final_error"] + assert diagnostics["error_events"] == 1 + assert diagnostics["retry_events"] == 0 + assert diagnostics["turn_status"] == "failed" + assert diagnostics["elapsed_seconds"] >= 0 + + +def test_retry_can_recover_and_final_answer_wins(runner): + notification = ErrorNotification( + threadId=SENSITIVE, + turnId="turn", + error=turn_error("serverOverloaded"), + willRetry=True, + ) + result, turn = runner( + [ + notification, + message(SENSITIVE, "commentary"), + message(json.dumps(REVIEW)), + message(SENSITIVE, None), + completed(), + ] + ) + assert turn.closed + assert result["status"] == "complete" + assert result["review"] == REVIEW + diagnostics = result["diagnostics"] + assert diagnostics["phase"] == "complete" + assert diagnostics["turn_status"] == "completed" + assert diagnostics["retry_events"] == 1 + assert "final_error" not in diagnostics + + +def test_unphased_final_answer_is_supported(runner): + result, _turn = runner([message(json.dumps(REVIEW), None), completed()]) + assert result["review"] == REVIEW + + +def test_empty_final_answer_does_not_fall_back_to_earlier_text(runner): + result, _turn = runner( + [message(json.dumps(REVIEW), None), message(""), completed()] + ) + assert result["status"] == "error" + assert result["diagnostics"]["reason"] == "missing_final_response" + + +def test_incomplete_coverage_remains_incomplete(runner): + review = { + **REVIEW, + "verdict": "needs_review", + "reviewed_surfaces": list(SURFACES[1:]), + "unreviewed_surfaces": [SURFACES[0]], + } + result, _turn = runner([message(json.dumps(review)), completed()]) + assert result["status"] == "incomplete" + assert result["review"] == review + assert result["diagnostics"]["phase"] == "complete" + + +@pytest.mark.parametrize("status", ["failed", "interrupted"]) +def test_unsuccessful_turn_without_error_metadata(runner, status): + result, _turn = runner([completed(status)]) + assert result["status"] == "error" + assert result["diagnostics"]["turn_status"] == status + assert "final_error" not in result["diagnostics"] + + +@pytest.mark.parametrize( + ("events", "phase", "error_type"), + [ + ([], "turn_stream", "ValueError"), + ([completed()], "turn_stream", "ValueError"), + ([RuntimeError(SENSITIVE)], "turn_stream", "RuntimeError"), + ([TimeoutError()], "turn_stream", "TimeoutError"), + ([message(SENSITIVE), completed()], "decode_report", "JSONDecodeError"), + ( + [message(json.dumps({"summary": SENSITIVE})), completed()], + "validate_report", + "ValueError", + ), + ], +) +def test_failure_phase_and_no_unvalidated_output(runner, events, phase, error_type): + result, turn = runner(events) + assert turn.closed + assert result["status"] == "error" + assert result["diagnostics"]["phase"] == phase + assert result["diagnostics"]["error_type"] == error_type + + +def test_rpc_failure_retains_numeric_code_not_message_or_data(runner): + result, _turn = runner( + [], openai_codex.JsonRpcError(-32602, SENSITIVE, {"data": SENSITIVE}) + ) + assert result["diagnostics"]["phase"] == "thread_start" + assert result["diagnostics"]["rpc_code"] == -32602 + assert result["diagnostics"]["error_type"] == "JsonRpcError" + + +def test_merge_conflict_does_not_publish_subprocess_output(monkeypatch, tmp_path): + def conflict(*_args): + raise subprocess.CalledProcessError( + 1, ["git", "merge-tree", "--write-tree"], output=SENSITIVE, stderr=SENSITIVE + ) + + monkeypatch.setattr(compatibility, "resolve_branch", conflict) + result = compatibility.run_review("candidate", "a" * 40, tmp_path, "base", 1) + assert result["diagnostics"]["phase"] == "resolve_context" + assert result["diagnostics"]["reason"] == "merge_conflict" + assert result["diagnostics"]["exit_code"] == 1 + for artifact in tmp_path.iterdir(): + assert SENSITIVE not in artifact.read_text() + + +def test_unknown_error_codes_fail_closed(): + info = SimpleNamespace( + model_dump=lambda **_kwargs: {SENSITIVE: {"httpStatusCode": 429}} + ) + error = SimpleNamespace(codex_error_info=info) + assert compatibility.public_turn_error(error) == { + "code": "unknown", + "http_status": None, + } + + +@pytest.mark.parametrize( + "status,exit_code", [("complete", 0), ("error", 1), ("incomplete", 1)] +) +def test_cli_exit_behavior_and_diagnostics( + monkeypatch, capsys, tmp_path, status, exit_code +): + monkeypatch.setattr( + sys, + "argv", + ["codex_compatibility.py", "candidate", "--output-dir", str(tmp_path)], + ) + monkeypatch.setattr( + compatibility, + "run_review", + lambda *_args: { + "status": status, + "review": REVIEW if status != "error" else None, + "diagnostics": { + "phase": "complete" if status == "complete" else "turn_stream" + }, + }, + ) + with pytest.raises(SystemExit) as caught: + compatibility.main() + assert caught.value.code == exit_code + assert "Compatibility review execution diagnostics:" in capsys.readouterr().out diff --git a/tasks/test.toml b/tasks/test.toml index 4309ad95b7..bc101646f0 100644 --- a/tasks/test.toml +++ b/tasks/test.toml @@ -20,6 +20,7 @@ depends = [ "test:qualification-summary", "test:codex-security-release-range", "test:codex-security-diagnostics", + "test:codex-compatibility", "test:docs-website", "test:docs-nav", "test:maintainer-approval", @@ -92,6 +93,11 @@ description = "Test public Codex Security diagnostics and sensitive-data filteri run = "uv run --no-project --with pytest pytest tasks/scripts/codex_security_diagnostics_test.py" hide = true +["test:codex-compatibility"] +description = "Test compatibility review execution diagnostics without inference calls" +run = "uv run --no-project --with pytest --with openai-codex==0.160.0 pytest tasks/scripts/codex_compatibility_test.py" +hide = true + ["test:maintainer-approval"] description = "Test the maintainer approval gate and change-alert tools" run = "uv run --no-project --with pytest pytest -o \"python_files=*_test.py\" tasks/scripts/check_maintainer_approval_test.py tasks/scripts/alert_maintainer_change_test.py" From f035b7175e7b0118e7250832c19a6c430734f98b Mon Sep 17 00:00:00 2001 From: Matthew Grossman Date: Wed, 4 Nov 2026 21:17:00 -0800 Subject: [PATCH 2/4] fix(ci): simplify compatibility error reporting Signed-off-by: Matthew Grossman --- .agents/skills/watch-github-actions/SKILL.md | 7 +- CI.md | 10 +- tasks/scripts/codex_compatibility.py | 158 ++-------- tasks/scripts/codex_compatibility_report.py | 9 - tasks/scripts/codex_compatibility_test.py | 316 +++---------------- tasks/test.toml | 6 - 6 files changed, 59 insertions(+), 447 deletions(-) diff --git a/.agents/skills/watch-github-actions/SKILL.md b/.agents/skills/watch-github-actions/SKILL.md index 0c2c7d8f86..6435a119ea 100644 --- a/.agents/skills/watch-github-actions/SKILL.md +++ b/.agents/skills/watch-github-actions/SKILL.md @@ -235,12 +235,7 @@ For `Codex Compatibility Review`, inspect its job summary and the candidate and baseline SHAs before interpreting findings. `complete` describes execution and coverage, not compatibility: also read the assessment and findings. An `error`, incomplete coverage, or missing artifact means no clean review is -available. Findings are advisory. For failed execution, inspect `report.json`'s `diagnostics` -or the execution diagnostics in the job log/summary: phase, exception type, -last/final SDK code and upstream HTTP status, and retry counts. SDK error text -and transcripts are deliberately omitted. Retry events alone do not imply failure -if the review ultimately completed. Context-resolution merge conflicts are not -transient SDK failures. The review targets breaking changes with high +available. Findings are advisory. The review targets breaking changes with high reasoning effort. Risk acceptance needs a linked author or verified-maintainer comment or review, not a PR description, general approval, or bot report. The full report keeps all findings and prior-comment links. `comment.md` includes diff --git a/CI.md b/CI.md index b851843273..dd0c3d4388 100644 --- a/CI.md +++ b/CI.md @@ -144,14 +144,8 @@ Each run retains `report.json` and `report.md` for 90 days in `compatibility-review-run--attempt-`. The report records both revisions, the train, tool/model metadata, coverage gaps, and actionable findings. Raw agent transcripts and PR discussions are not published. -The report's `diagnostics` object and job log record the last execution phase, -elapsed seconds, exception type, turn status, retry/error notification counts, -and the last and final structured SDK error codes and upstream HTTP statuses -when available. A merge conflict is identified separately during context -resolution. SDK messages, additional details, stderr, and rejected review output -are not retained because they can contain source or credentials. A successful -review can still have retry notifications; use `final_error` and the execution -status to distinguish recovery from failure. +SDK failures retain the exception message in the report, with known inference +and GitHub credentials redacted and the message limited to 1,000 characters. The qualification summary links this separate advisory evidence without waiting for it; compatibility review remains missing from the blocking RFC profile during this rollout. diff --git a/tasks/scripts/codex_compatibility.py b/tasks/scripts/codex_compatibility.py index 33fbd8c07a..e39b205eac 100644 --- a/tasks/scripts/codex_compatibility.py +++ b/tasks/scripts/codex_compatibility.py @@ -10,8 +10,6 @@ import re import subprocess import tempfile -import time -from contextlib import aclosing from importlib.metadata import version from pathlib import Path @@ -19,106 +17,11 @@ import openai_codex # ty: ignore[unresolved-import] from check_proto_compatibility import git, train_policy from codex_compatibility_report import validate_review, write_report -from openai_codex.generated.v2_all import ( # ty: ignore[unresolved-import] - AgentMessageThreadItem, - ErrorNotification, - ItemCompletedNotification, - MessagePhase, - TurnCompletedNotification, -) ASSETS = Path(__file__).resolve().parents[2] / ".github" / "prompts" MODEL = "openai/openai/gpt-6.1-sol" ENDPOINT = "https://inference-api.nvidia.com/v1" -# Error messages/details may contain source, tool output, or credentials. Publish -# only protocol-defined states, never arbitrary strings from the SDK or model. -ERROR_CODES = frozenset( - { - "contextWindowExceeded", - "sessionBudgetExceeded", - "usageLimitExceeded", - "rateLimitExceeded", - "flexUnavailable", - "serverOverloaded", - "cyberPolicy", - "misalignmentPolicyViolation", - "tooManyDenials", - "internalServerError", - "unauthorized", - "badRequest", - "threadRollbackFailed", - "sandboxError", - "other", - "httpConnectionFailed", - "responseStreamConnectionFailed", - "responseStreamDisconnected", - "responseTooManyFailedAttempts", - "activeTurnNotSteerable", - } -) - - -def public_turn_error(error) -> dict: - """Extract only allowlisted protocol fields, not message/additionalDetails.""" - info = error.codex_error_info - value = info.model_dump(mode="json", by_alias=True) if info is not None else None - code = value if isinstance(value, str) else None - status = None - if isinstance(value, dict) and len(value) == 1: - code, details = next(iter(value.items())) - if code in ERROR_CODES and isinstance(details, dict): - status = details.get("httpStatusCode", details.get("http_status_code")) - return { - "code": code if code in ERROR_CODES else "unknown", - "http_status": status if type(status) is int and 100 <= status <= 599 else None, - } - - -async def collect_review(thread, prompt: str, schema: dict, diagnostics: dict) -> str: - """Consume the public turn stream before the SDK erases structured errors.""" - diagnostics["phase"] = "turn_start" - turn = await thread.turn(prompt, output_schema=schema) - diagnostics["phase"] = "turn_stream" - completed = None - final_response = None - fallback_response = None - async with aclosing(turn.stream()) as stream: - async for event in stream: - payload = event.payload - if isinstance(payload, ErrorNotification): - diagnostics["error_events"] += 1 - diagnostics["retry_events"] += int(payload.will_retry) - diagnostics["last_error"] = public_turn_error(payload.error) - elif isinstance(payload, TurnCompletedNotification): - completed = payload.turn - status = completed.status.value - diagnostics["turn_status"] = ( - status - if status in ("completed", "failed", "interrupted", "inProgress") - else "unknown" - ) - if completed.error is not None: - diagnostics["final_error"] = public_turn_error(completed.error) - elif isinstance(payload, ItemCompletedNotification): - item = payload.item.root - if isinstance(item, AgentMessageThreadItem): - if item.phase == MessagePhase.final_answer: - final_response = item.text - elif item.phase is None: - fallback_response = item.text - if completed is None: - diagnostics["reason"] = "missing_completion" - raise ValueError("Codex turn ended without a completion event.") - if completed.status.value != "completed": - diagnostics["reason"] = "unsuccessful_turn" - raise ValueError("Codex did not complete the compatibility review.") - response = final_response if final_response is not None else fallback_response - if not response: - diagnostics["reason"] = "missing_final_response" - raise ValueError("Codex completed without a final compatibility report.") - return response - def resolve_candidate(tag: str, expected_sha: str) -> dict: if not re.fullmatch(r"v0\.\d+\.\d+(?:-pre\.\d+)?", tag): @@ -163,10 +66,9 @@ def resolve_branch(base_ref: str, source_sha: str, pr_number: int) -> dict: } -async def invoke_codex(context: dict, directory: Path, diagnostics: dict) -> dict: +async def invoke_codex(context: dict, directory: Path) -> dict: # Git objects expose both revisions without loading candidate-side agent # instructions, configuration, skills, or hooks into the review workspace. - diagnostics["phase"] = "clone_source" subprocess.run( [ "git", @@ -207,17 +109,16 @@ async def invoke_codex(context: dict, directory: Path, diagnostics: dict) -> dic 'shell_environment_policy.filters={PATH="include", HOME="include", GH_TOKEN="include", ' 'SSL_CERT_FILE="include", SSL_CERT_DIR="include"}', ) - diagnostics["phase"] = "load_prompt" prompt = (ASSETS / "compatibility-review.md").read_text() prompt += "\nReview context:\n" + json.dumps(context) schema = json.loads((ASSETS / "compatibility-review.schema.json").read_text()) codex = openai_codex.AsyncCodex( openai_codex.CodexConfig(cwd=str(directory), config_overrides=settings) ) - # Retain structured failure states, but never SDK messages or transcripts. + # Raw agent output can include inspected source or tool output. Publish only + # the validated final report, never the transcript or credential environment. try: async with asyncio.timeout(1800): - diagnostics["phase"] = "thread_start" thread = await codex.thread_start( cwd=str(directory), model=MODEL, @@ -225,19 +126,26 @@ async def invoke_codex(context: dict, directory: Path, diagnostics: dict) -> dic approval_mode=openai_codex.ApprovalMode.deny_all, ephemeral=True, ) - response = await collect_review(thread, prompt, schema, diagnostics) + result = await thread.run(prompt, output_schema=schema) except (openai_codex.CodexError, RuntimeError, ValueError) as error: - diagnostics["error_type"] = type(error).__name__ - if isinstance(error, openai_codex.JsonRpcError) and type(error.code) is int: - diagnostics["rpc_code"] = error.code + message = str(error) + for key in ( + "NVIDIA_INFERENCE_API_KEY", + "GH_TOKEN", + "GITHUB_TOKEN", + "CODEX_SECURITY_API_KEY", + ): + if secret := os.environ.get(key): + message = message.replace(secret, "[REDACTED]") + message = " ".join(message.split())[:1000] raise ValueError( - f"Codex SDK review failed ({type(error).__name__}); no assessment is available." + f"Codex SDK review failed ({type(error).__name__}): {message}; no assessment is available." ) from None finally: await codex.close() - diagnostics["phase"] = "decode_report" - review = json.loads(response) - diagnostics["phase"] = "validate_report" + if result.status.value != "completed" or not result.final_response: + raise ValueError("Codex did not complete the compatibility review.") + review = json.loads(result.final_response) validate_review(review) return review @@ -245,12 +153,6 @@ async def invoke_codex(context: dict, directory: Path, diagnostics: dict) -> dic def run_review( tag: str, expected_sha: str, output: Path, base_ref: str, pr_number: int ) -> dict: - started = time.monotonic() - diagnostics = { - "phase": "resolve_context", - "error_events": 0, - "retry_events": 0, - } result = { "schema_version": 3, "advisory": True, @@ -258,7 +160,6 @@ def run_review( "context": {}, "review": None, "error": "", - "diagnostics": diagnostics, "model": MODEL, "reasoning_effort": "high", "codex_version": "", @@ -273,10 +174,8 @@ def run_review( if base_ref else resolve_candidate(tag, expected_sha) ) - diagnostics["phase"] = "credentials" if not os.environ.get("NVIDIA_INFERENCE_API_KEY"): raise ValueError("NVIDIA inference credential is missing.") - diagnostics["phase"] = "tool_versions" result["codex_version"] = version("openai-codex-cli-bin") result["codex_sdk_version"] = version("openai-codex") result["context"]["repository"] = os.environ.get("GITHUB_REPOSITORY", "") @@ -285,40 +184,25 @@ def run_review( prefix="openshell-compatibility-" ) as temporary: result["review"] = asyncio.run( - invoke_codex(result["context"], Path(temporary), diagnostics) + invoke_codex(result["context"], Path(temporary)) ) result["status"] = ( "incomplete" if result["review"]["unreviewed_surfaces"] else "complete" ) - diagnostics["phase"] = "complete" except TimeoutError: - diagnostics["error_type"] = "TimeoutError" - diagnostics["reason"] = "timeout" result["error"] = "Compatibility review exceeded its time limit." except subprocess.CalledProcessError as error: - diagnostics["error_type"] = "CalledProcessError" - diagnostics["exit_code"] = error.returncode - if ( - isinstance(error.cmd, (list, tuple)) - and tuple(error.cmd[:2]) == ("git", "merge-tree") - and error.returncode == 1 - ): - diagnostics["reason"] = "merge_conflict" result["error"] = ( f"Review tooling failed (exit {error.returncode}); no assessment is available." ) except json.JSONDecodeError: - diagnostics["error_type"] = "JSONDecodeError" result["error"] = "Codex did not return a valid JSON report." except ValueError as error: - diagnostics.setdefault("error_type", "ValueError") result["error"] = str(error) except OSError as error: - diagnostics["error_type"] = type(error).__name__ result["error"] = ( f"Review input, tool, or output unavailable ({type(error).__name__})." ) - diagnostics["elapsed_seconds"] = round(time.monotonic() - started, 1) write_report(output, result) return result @@ -339,10 +223,6 @@ def main() -> None: args.candidate, args.source_sha, args.output_dir, args.base_ref, args.pr_number ) verdict = result["review"]["verdict"] if result["review"] else "unavailable" - print( - "Compatibility review execution diagnostics: " - + json.dumps(result["diagnostics"]) - ) has_findings = bool( result["review"] and any(not f["previous_report_url"] for f in result["review"]["findings"]) diff --git a/tasks/scripts/codex_compatibility_report.py b/tasks/scripts/codex_compatibility_report.py index c9a436c31a..1c959ef909 100644 --- a/tasks/scripts/codex_compatibility_report.py +++ b/tasks/scripts/codex_compatibility_report.py @@ -179,15 +179,6 @@ def write_report(output: Path, result: dict) -> None: f"- Baseline: `{context.get('baseline_tag', 'unresolved')}` " f"(`{context.get('baseline_sha', 'unresolved')}`)", ] - if diagnostics := result.get("diagnostics"): - details += [ - "", - "Execution diagnostics (fixed states and numeric metadata only):", - "", - "```json", - json.dumps(diagnostics, indent=2), - "```", - ] if review: details.append( f"- Reviewed: {', '.join(review['reviewed_surfaces']) or 'none'}" diff --git a/tasks/scripts/codex_compatibility_test.py b/tasks/scripts/codex_compatibility_test.py index e38039cd3b..f2d7d4d8c4 100644 --- a/tasks/scripts/codex_compatibility_test.py +++ b/tasks/scripts/codex_compatibility_test.py @@ -1,10 +1,7 @@ # SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. # SPDX-License-Identifier: Apache-2.0 -"""Exercise the pinned SDK's public notification types without inference calls.""" - -import json -import subprocess +import asyncio import sys from pathlib import Path from types import SimpleNamespace @@ -12,291 +9,52 @@ import openai_codex import pytest -from openai_codex.generated.v2_all import ( - ErrorNotification, - ItemCompletedNotification, - TurnCompletedNotification, - TurnError, -) sys.path.insert(0, str(Path(__file__).resolve().parent)) import codex_compatibility as compatibility -from codex_compatibility_report import SURFACES - -SENSITIVE = "PRIVATE_source_token_tool_output" -REVIEW = { - "verdict": "no_unaddressed_breaks", - "summary": "No changes to public contracts.", - "reviewed_surfaces": list(SURFACES), - "unreviewed_surfaces": [], - "findings": [], -} - - -def turn_error(info): - return TurnError( - message=SENSITIVE, - additionalDetails=SENSITIVE, - codexErrorInfo=info, - ) - - -def completed(status="completed", error=None): - return TurnCompletedNotification( - threadId=SENSITIVE, - turn={"id": "turn", "status": status, "error": error, "items": []}, - ) - - -def message(text, phase="final_answer"): - return ItemCompletedNotification( - threadId=SENSITIVE, - turnId="turn", - completedAtMs=0, - item={"type": "agentMessage", "id": "message", "text": text, "phase": phase}, - ) - - -class FakeTurn: - def __init__(self, events): - self.events = events - self.closed = False - - async def stream(self): - try: - for payload in self.events: - if isinstance(payload, Exception): - raise payload - yield SimpleNamespace(payload=payload) - finally: - self.closed = True - - -@pytest.fixture -def runner(monkeypatch, tmp_path): - """Run the entire report pipeline with fake SDK I/O, not fake SDK types.""" - monkeypatch.setenv("NVIDIA_INFERENCE_API_KEY", SENSITIVE) - monkeypatch.setattr(compatibility, "version", lambda _name: "0.160.0") - monkeypatch.setattr( - compatibility, "resolve_branch", lambda *_args: {"source_sha": "a" * 40} - ) - monkeypatch.setattr(compatibility.subprocess, "run", lambda *_args, **_kwargs: None) - - def run(events, start_error=None): - turn = FakeTurn(events) - thread = SimpleNamespace(turn=AsyncMock(return_value=turn)) - codex = SimpleNamespace( - thread_start=AsyncMock(return_value=thread, side_effect=start_error), - close=AsyncMock(), - ) - monkeypatch.setattr( - compatibility.openai_codex, "AsyncCodex", lambda _config: codex - ) - result = compatibility.run_review("candidate", "a" * 40, tmp_path, "base", 1) - codex.close.assert_awaited_once() - # Nothing unvalidated, including SDK messages or IDs, reaches artifacts. - for artifact in tmp_path.iterdir(): - assert SENSITIVE not in artifact.read_text() - assert json.loads((tmp_path / "report.json").read_text()) == result - return result, turn - - return run @pytest.mark.parametrize( - ("info", "code", "status"), - [ - ("rateLimitExceeded", "rateLimitExceeded", None), - ("contextWindowExceeded", "contextWindowExceeded", None), - ( - {"httpConnectionFailed": {"httpStatusCode": 429}}, - "httpConnectionFailed", - 429, - ), - ( - {"responseStreamConnectionFailed": {"httpStatusCode": 502}}, - "responseStreamConnectionFailed", - 502, - ), - ( - {"responseStreamDisconnected": {"httpStatusCode": 200}}, - "responseStreamDisconnected", - 200, - ), - ( - {"responseTooManyFailedAttempts": {"httpStatusCode": 503}}, - "responseTooManyFailedAttempts", - 503, - ), - (None, "unknown", None), - ( - {"httpConnectionFailed": {"httpStatusCode": 999}}, - "httpConnectionFailed", - None, - ), - ], + "error_type", [RuntimeError, ValueError, openai_codex.CodexError] ) -def test_failed_turn_retains_only_structured_error(runner, info, code, status): - error = turn_error(info) - notification = ErrorNotification( - threadId=SENSITIVE, turnId="turn", error=error, willRetry=False - ) - result, turn = runner([notification, completed("failed", error)]) - assert turn.closed - assert result["status"] == "error" - assert result["review"] is None - diagnostics = result["diagnostics"] - assert diagnostics["phase"] == "turn_stream" - assert diagnostics["final_error"] == {"code": code, "http_status": status} - assert diagnostics["last_error"] == diagnostics["final_error"] - assert diagnostics["error_events"] == 1 - assert diagnostics["retry_events"] == 0 - assert diagnostics["turn_status"] == "failed" - assert diagnostics["elapsed_seconds"] >= 0 - - -def test_retry_can_recover_and_final_answer_wins(runner): - notification = ErrorNotification( - threadId=SENSITIVE, - turnId="turn", - error=turn_error("serverOverloaded"), - willRetry=True, - ) - result, turn = runner( - [ - notification, - message(SENSITIVE, "commentary"), - message(json.dumps(REVIEW)), - message(SENSITIVE, None), - completed(), - ] - ) - assert turn.closed - assert result["status"] == "complete" - assert result["review"] == REVIEW - diagnostics = result["diagnostics"] - assert diagnostics["phase"] == "complete" - assert diagnostics["turn_status"] == "completed" - assert diagnostics["retry_events"] == 1 - assert "final_error" not in diagnostics - - -def test_unphased_final_answer_is_supported(runner): - result, _turn = runner([message(json.dumps(REVIEW), None), completed()]) - assert result["review"] == REVIEW - - -def test_empty_final_answer_does_not_fall_back_to_earlier_text(runner): - result, _turn = runner( - [message(json.dumps(REVIEW), None), message(""), completed()] - ) - assert result["status"] == "error" - assert result["diagnostics"]["reason"] == "missing_final_response" - - -def test_incomplete_coverage_remains_incomplete(runner): - review = { - **REVIEW, - "verdict": "needs_review", - "reviewed_surfaces": list(SURFACES[1:]), - "unreviewed_surfaces": [SURFACES[0]], - } - result, _turn = runner([message(json.dumps(review)), completed()]) - assert result["status"] == "incomplete" - assert result["review"] == review - assert result["diagnostics"]["phase"] == "complete" - - -@pytest.mark.parametrize("status", ["failed", "interrupted"]) -def test_unsuccessful_turn_without_error_metadata(runner, status): - result, _turn = runner([completed(status)]) - assert result["status"] == "error" - assert result["diagnostics"]["turn_status"] == status - assert "final_error" not in result["diagnostics"] - - -@pytest.mark.parametrize( - ("events", "phase", "error_type"), - [ - ([], "turn_stream", "ValueError"), - ([completed()], "turn_stream", "ValueError"), - ([RuntimeError(SENSITIVE)], "turn_stream", "RuntimeError"), - ([TimeoutError()], "turn_stream", "TimeoutError"), - ([message(SENSITIVE), completed()], "decode_report", "JSONDecodeError"), +@pytest.mark.parametrize("long_message", [False, True]) +def test_sdk_error_message_is_retained_redacted_and_bounded( + monkeypatch, tmp_path, error_type, long_message +): + secrets = ["inference-secret", "gh-secret", "github-secret", "codex-secret"] + for key, secret in zip( ( - [message(json.dumps({"summary": SENSITIVE})), completed()], - "validate_report", - "ValueError", + "NVIDIA_INFERENCE_API_KEY", + "GH_TOKEN", + "GITHUB_TOKEN", + "CODEX_SECURITY_API_KEY", ), - ], -) -def test_failure_phase_and_no_unvalidated_output(runner, events, phase, error_type): - result, turn = runner(events) - assert turn.closed - assert result["status"] == "error" - assert result["diagnostics"]["phase"] == phase - assert result["diagnostics"]["error_type"] == error_type - - -def test_rpc_failure_retains_numeric_code_not_message_or_data(runner): - result, _turn = runner( - [], openai_codex.JsonRpcError(-32602, SENSITIVE, {"data": SENSITIVE}) - ) - assert result["diagnostics"]["phase"] == "thread_start" - assert result["diagnostics"]["rpc_code"] == -32602 - assert result["diagnostics"]["error_type"] == "JsonRpcError" - - -def test_merge_conflict_does_not_publish_subprocess_output(monkeypatch, tmp_path): - def conflict(*_args): - raise subprocess.CalledProcessError( - 1, ["git", "merge-tree", "--write-tree"], output=SENSITIVE, stderr=SENSITIVE - ) - - monkeypatch.setattr(compatibility, "resolve_branch", conflict) - result = compatibility.run_review("candidate", "a" * 40, tmp_path, "base", 1) - assert result["diagnostics"]["phase"] == "resolve_context" - assert result["diagnostics"]["reason"] == "merge_conflict" - assert result["diagnostics"]["exit_code"] == 1 - for artifact in tmp_path.iterdir(): - assert SENSITIVE not in artifact.read_text() - - -def test_unknown_error_codes_fail_closed(): - info = SimpleNamespace( - model_dump=lambda **_kwargs: {SENSITIVE: {"httpStatusCode": 429}} - ) - error = SimpleNamespace(codex_error_info=info) - assert compatibility.public_turn_error(error) == { - "code": "unknown", - "http_status": None, - } + secrets, + strict=True, + ): + monkeypatch.setenv(key, secret) + message = "HTTP 502\nresponse stream disconnected " + " ".join(secrets) + if long_message: + message += "x" * 2000 + thread = SimpleNamespace(run=AsyncMock(side_effect=error_type(message))) + codex = SimpleNamespace( + thread_start=AsyncMock(return_value=thread), close=AsyncMock() + ) + monkeypatch.setattr(compatibility.openai_codex, "AsyncCodex", lambda _config: codex) + monkeypatch.setattr(compatibility.subprocess, "run", lambda *_args, **_kwargs: None) + with pytest.raises(ValueError) as caught: + asyncio.run(compatibility.invoke_codex({}, tmp_path)) -@pytest.mark.parametrize( - "status,exit_code", [("complete", 0), ("error", 1), ("incomplete", 1)] -) -def test_cli_exit_behavior_and_diagnostics( - monkeypatch, capsys, tmp_path, status, exit_code -): - monkeypatch.setattr( - sys, - "argv", - ["codex_compatibility.py", "candidate", "--output-dir", str(tmp_path)], - ) - monkeypatch.setattr( - compatibility, - "run_review", - lambda *_args: { - "status": status, - "review": REVIEW if status != "error" else None, - "diagnostics": { - "phase": "complete" if status == "complete" else "turn_stream" - }, - }, + error = str(caught.value) + assert ( + f"Codex SDK review failed ({error_type.__name__}): HTTP 502 response stream disconnected" + in error ) - with pytest.raises(SystemExit) as caught: - compatibility.main() - assert caught.value.code == exit_code - assert "Compatibility review execution diagnostics:" in capsys.readouterr().out + assert all(secret not in error for secret in secrets) + assert error.count("[REDACTED]") == 4 + detail = error.split(": ", 1)[1].removesuffix("; no assessment is available.") + assert len(detail) <= 1000 + assert "\n" not in error + codex.close.assert_awaited_once() diff --git a/tasks/test.toml b/tasks/test.toml index bc101646f0..4309ad95b7 100644 --- a/tasks/test.toml +++ b/tasks/test.toml @@ -20,7 +20,6 @@ depends = [ "test:qualification-summary", "test:codex-security-release-range", "test:codex-security-diagnostics", - "test:codex-compatibility", "test:docs-website", "test:docs-nav", "test:maintainer-approval", @@ -93,11 +92,6 @@ description = "Test public Codex Security diagnostics and sensitive-data filteri run = "uv run --no-project --with pytest pytest tasks/scripts/codex_security_diagnostics_test.py" hide = true -["test:codex-compatibility"] -description = "Test compatibility review execution diagnostics without inference calls" -run = "uv run --no-project --with pytest --with openai-codex==0.160.0 pytest tasks/scripts/codex_compatibility_test.py" -hide = true - ["test:maintainer-approval"] description = "Test the maintainer approval gate and change-alert tools" run = "uv run --no-project --with pytest pytest -o \"python_files=*_test.py\" tasks/scripts/check_maintainer_approval_test.py tasks/scripts/alert_maintainer_change_test.py" From ebea7e1bdd83a04f8847cbf4641794f39eb35f01 Mon Sep 17 00:00:00 2001 From: Matthew Grossman Date: Wed, 4 Nov 2026 21:23:24 -0800 Subject: [PATCH 3/4] fix(ci): pass through SDK errors without filtering Signed-off-by: Matthew Grossman --- CI.md | 3 +-- tasks/scripts/codex_compatibility.py | 12 +-------- tasks/scripts/codex_compatibility_test.py | 32 +++-------------------- 3 files changed, 6 insertions(+), 41 deletions(-) diff --git a/CI.md b/CI.md index dd0c3d4388..ff278592e7 100644 --- a/CI.md +++ b/CI.md @@ -144,8 +144,7 @@ Each run retains `report.json` and `report.md` for 90 days in `compatibility-review-run--attempt-`. The report records both revisions, the train, tool/model metadata, coverage gaps, and actionable findings. Raw agent transcripts and PR discussions are not published. -SDK failures retain the exception message in the report, with known inference -and GitHub credentials redacted and the message limited to 1,000 characters. +SDK failures retain the exception message in the report. The qualification summary links this separate advisory evidence without waiting for it; compatibility review remains missing from the blocking RFC profile during this rollout. diff --git a/tasks/scripts/codex_compatibility.py b/tasks/scripts/codex_compatibility.py index e39b205eac..59f893f11c 100644 --- a/tasks/scripts/codex_compatibility.py +++ b/tasks/scripts/codex_compatibility.py @@ -128,18 +128,8 @@ async def invoke_codex(context: dict, directory: Path) -> dict: ) result = await thread.run(prompt, output_schema=schema) except (openai_codex.CodexError, RuntimeError, ValueError) as error: - message = str(error) - for key in ( - "NVIDIA_INFERENCE_API_KEY", - "GH_TOKEN", - "GITHUB_TOKEN", - "CODEX_SECURITY_API_KEY", - ): - if secret := os.environ.get(key): - message = message.replace(secret, "[REDACTED]") - message = " ".join(message.split())[:1000] raise ValueError( - f"Codex SDK review failed ({type(error).__name__}): {message}; no assessment is available." + f"Codex SDK review failed ({type(error).__name__}): {error}; no assessment is available." ) from None finally: await codex.close() diff --git a/tasks/scripts/codex_compatibility_test.py b/tasks/scripts/codex_compatibility_test.py index f2d7d4d8c4..985b841cd1 100644 --- a/tasks/scripts/codex_compatibility_test.py +++ b/tasks/scripts/codex_compatibility_test.py @@ -18,25 +18,8 @@ @pytest.mark.parametrize( "error_type", [RuntimeError, ValueError, openai_codex.CodexError] ) -@pytest.mark.parametrize("long_message", [False, True]) -def test_sdk_error_message_is_retained_redacted_and_bounded( - monkeypatch, tmp_path, error_type, long_message -): - secrets = ["inference-secret", "gh-secret", "github-secret", "codex-secret"] - for key, secret in zip( - ( - "NVIDIA_INFERENCE_API_KEY", - "GH_TOKEN", - "GITHUB_TOKEN", - "CODEX_SECURITY_API_KEY", - ), - secrets, - strict=True, - ): - monkeypatch.setenv(key, secret) - message = "HTTP 502\nresponse stream disconnected " + " ".join(secrets) - if long_message: - message += "x" * 2000 +def test_sdk_error_message_is_retained(monkeypatch, tmp_path, error_type): + message = "HTTP 502: response stream disconnected" thread = SimpleNamespace(run=AsyncMock(side_effect=error_type(message))) codex = SimpleNamespace( thread_start=AsyncMock(return_value=thread), close=AsyncMock() @@ -47,14 +30,7 @@ def test_sdk_error_message_is_retained_redacted_and_bounded( with pytest.raises(ValueError) as caught: asyncio.run(compatibility.invoke_codex({}, tmp_path)) - error = str(caught.value) - assert ( - f"Codex SDK review failed ({error_type.__name__}): HTTP 502 response stream disconnected" - in error + assert str(caught.value) == ( + f"Codex SDK review failed ({error_type.__name__}): {message}; no assessment is available." ) - assert all(secret not in error for secret in secrets) - assert error.count("[REDACTED]") == 4 - detail = error.split(": ", 1)[1].removesuffix("; no assessment is available.") - assert len(detail) <= 1000 - assert "\n" not in error codex.close.assert_awaited_once() From 64a1423f063d29e871b5e73649facc660397b90e Mon Sep 17 00:00:00 2001 From: Matthew Grossman Date: Wed, 4 Nov 2026 21:24:45 -0800 Subject: [PATCH 4/4] chore(ci): remove compatibility error test file Signed-off-by: Matthew Grossman --- tasks/scripts/codex_compatibility_test.py | 36 ----------------------- 1 file changed, 36 deletions(-) delete mode 100644 tasks/scripts/codex_compatibility_test.py diff --git a/tasks/scripts/codex_compatibility_test.py b/tasks/scripts/codex_compatibility_test.py deleted file mode 100644 index 985b841cd1..0000000000 --- a/tasks/scripts/codex_compatibility_test.py +++ /dev/null @@ -1,36 +0,0 @@ -# SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. -# SPDX-License-Identifier: Apache-2.0 - -import asyncio -import sys -from pathlib import Path -from types import SimpleNamespace -from unittest.mock import AsyncMock - -import openai_codex -import pytest - -sys.path.insert(0, str(Path(__file__).resolve().parent)) - -import codex_compatibility as compatibility - - -@pytest.mark.parametrize( - "error_type", [RuntimeError, ValueError, openai_codex.CodexError] -) -def test_sdk_error_message_is_retained(monkeypatch, tmp_path, error_type): - message = "HTTP 502: response stream disconnected" - thread = SimpleNamespace(run=AsyncMock(side_effect=error_type(message))) - codex = SimpleNamespace( - thread_start=AsyncMock(return_value=thread), close=AsyncMock() - ) - monkeypatch.setattr(compatibility.openai_codex, "AsyncCodex", lambda _config: codex) - monkeypatch.setattr(compatibility.subprocess, "run", lambda *_args, **_kwargs: None) - - with pytest.raises(ValueError) as caught: - asyncio.run(compatibility.invoke_codex({}, tmp_path)) - - assert str(caught.value) == ( - f"Codex SDK review failed ({error_type.__name__}): {message}; no assessment is available." - ) - codex.close.assert_awaited_once()