From 5630f1ca79b0e847dffa086ab0b9adba6d45f8da Mon Sep 17 00:00:00 2001 From: Abir Abbas Date: Wed, 22 Jul 2026 15:50:46 -0400 Subject: [PATCH] fix(codex): survive OpenAI's strict validator on --output-schema (#106) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The server behind codex exec now validates --output-schema against OpenAI strict-mode rules (probed live on codex-cli 0.144.1): every object node needs additionalProperties:false plus a full required array, every node needs a type (or $ref / anyOf), and free-form maps (dict[str, Any]), typed maps, bare Any nodes, and boolean subschemas are rejected with invalid_json_schema — which killed every schema-enforced codex role (PM, git init, ...) in seconds. Two-layer fix in the codex harness patch: - Deterministic gate: before adding --output-schema, check the written strict schema against the probed validator rules (_codex_schema_strict_expressible). Inexpressible schemas run with --output-last-message only — codex still persists the final JSON and the harness runner's local validation enforces the schema. - Reactive fallback: if the server refuses a schema anyway (its rules can tighten upstream at any time), rerun once without --output-schema and take the retry's output. The Go node needs the paired agentfield Go SDK fix (same design in the SDK's codex provider/runner) plus an SDK re-pin to pick it up. Co-Authored-By: Claude Fable 5 --- swe_af/runtime/codex_harness_patch.py | 120 +++++++++++++++-- tests/test_codex_harness_patch.py | 182 ++++++++++++++++++++++++++ 2 files changed, 294 insertions(+), 8 deletions(-) diff --git a/swe_af/runtime/codex_harness_patch.py b/swe_af/runtime/codex_harness_patch.py index 79ddbf3..ba06d0b 100644 --- a/swe_af/runtime/codex_harness_patch.py +++ b/swe_af/runtime/codex_harness_patch.py @@ -65,6 +65,81 @@ def _codex_strict_json_schema(schema: dict[str, Any]) -> dict[str, Any]: return strict +def _codex_schema_strict_expressible(schema: Any) -> bool: + """Whether a strict-rewritten schema can be sent through --output-schema. + + The server behind ``codex exec`` validates --output-schema against OpenAI + strict-mode rules (probed live on codex-cli 0.144.1): every object node + needs ``additionalProperties: false`` plus a full ``required`` array, every + node needs a ``type`` (or ``$ref`` / anyOf combinator), and free-form maps + (``dict[str, Any]``), typed maps, bare ``Any`` nodes, and boolean + subschemas are rejected with ``invalid_json_schema``. Schemas containing + such nodes must skip the flag: codex still persists its final answer via + --output-last-message and the harness runner validates locally (issue + Agent-Field/SWE-AF#106). + """ + if not isinstance(schema, dict): + return False # boolean subschemas ("default_value": true) and the like + for def_key in ("$defs", "definitions"): + defs = schema.get(def_key) + if isinstance(defs, dict): + if not all(_codex_schema_strict_expressible(value) for value in defs.values()): + return False + if isinstance(schema.get("$ref"), str): + return True # target checked via the $defs walk above + for key in ("anyOf", "oneOf", "allOf"): + branch = schema.get(key) + if isinstance(branch, list): + return all(_codex_schema_strict_expressible(item) for item in branch) + type_value = schema.get("type") + if isinstance(type_value, str): + if type_value == "object": + properties = schema.get("properties") + if not isinstance(properties, dict): + return False # free-form map — strict mode would force {} + if schema.get("additionalProperties") is not False: + return False + return all( + _codex_schema_strict_expressible(value) for value in properties.values() + ) + if type_value == "array": + items = schema.get("items") + if not isinstance(items, dict): + return False + return _codex_schema_strict_expressible(items) + return True # primitive leaf + if isinstance(type_value, list): + # e.g. ["string", "null"]; only primitive members are safely strict. + return bool(type_value) and all( + isinstance(tv, str) and tv not in ("object", "array") for tv in type_value + ) + return False # no type, no $ref, no combinator — e.g. {} for Any + + +_SCHEMA_REJECTION_MARKERS = ("invalid_json_schema", "invalid schema for response_format") + + +def _is_output_schema_rejection(output: str) -> bool: + """Whether CLI output carries the server-side strict validator's 400.""" + lower = output.lower() + return any(marker in lower for marker in _SCHEMA_REJECTION_MARKERS) + + +def _without_flag_value(cmd: list[str], flag: str) -> list[str]: + """Return cmd with one ``flag value`` pair removed.""" + out: list[str] = [] + skip_next = False + for i, arg in enumerate(cmd): + if skip_next: + skip_next = False + continue + if arg == flag and i + 1 < len(cmd): + skip_next = True + continue + out.append(arg) + return out + + def _augment_codex_error_message(message: str, detail: str) -> str: lower = f"{message}\n{detail}".lower() hints = ( @@ -178,11 +253,24 @@ async def execute_with_native_structured_output(self: Any, prompt: str, options: cmd.extend(["--sandbox", "workspace-write"]) prompt_for_codex = prompt + used_output_schema = False if cwd: schema_path = _schema.get_schema_path(cwd) output_path = _schema.get_output_path(cwd) if Path(schema_path).exists(): - cmd.extend(["--output-schema", schema_path]) + # --output-schema only when the schema survives OpenAI's strict + # validator; otherwise the server 400s the whole session with + # invalid_json_schema. --output-last-message is safe either way + # and keeps the final answer capturable for local validation. + try: + schema_expressible = _codex_schema_strict_expressible( + json.loads(Path(schema_path).read_text(encoding="utf-8")) + ) + except Exception: + schema_expressible = False + if schema_expressible: + cmd.extend(["--output-schema", schema_path]) + used_output_schema = True cmd.extend(["--output-last-message", output_path]) prompt_for_codex += ( "\n\n---\n" @@ -193,17 +281,33 @@ async def execute_with_native_structured_output(self: Any, prompt: str, options: "the output file yourself or make the output file the task." ) - try: - start = asyncio.get_running_loop().time() + async def _invoke(cmd_to_run: list[str]) -> tuple[str, str, int]: timeout_seconds = options.get("timeout_seconds") if isinstance(timeout_seconds, (int, float)) and timeout_seconds > 0: - stdout, stderr, returncode = await asyncio.wait_for( - _run_codex_cli_with_stdin(cmd, prompt_for_codex, env=merged_env, cwd=cwd), + return await asyncio.wait_for( + _run_codex_cli_with_stdin( + cmd_to_run, prompt_for_codex, env=merged_env, cwd=cwd + ), timeout=float(timeout_seconds), ) - else: - stdout, stderr, returncode = await _run_codex_cli_with_stdin( - cmd, prompt_for_codex, env=merged_env, cwd=cwd + return await _run_codex_cli_with_stdin( + cmd_to_run, prompt_for_codex, env=merged_env, cwd=cwd + ) + + try: + start = asyncio.get_running_loop().time() + stdout, stderr, returncode = await _invoke(cmd) + # Reactive fallback: if the server's strict validator refused the + # schema anyway (its rules can tighten upstream at any time), rerun + # once without --output-schema — the prompt still pins the JSON + # contract and --output-last-message still captures the answer. + if ( + returncode != 0 + and used_output_schema + and _is_output_schema_rejection(f"{stdout}\n{stderr}") + ): + stdout, stderr, returncode = await _invoke( + _without_flag_value(cmd, "--output-schema") ) duration_ms = int((asyncio.get_running_loop().time() - start) * 1000) except FileNotFoundError as exc: diff --git a/tests/test_codex_harness_patch.py b/tests/test_codex_harness_patch.py index 405b69e..50529ea 100644 --- a/tests/test_codex_harness_patch.py +++ b/tests/test_codex_harness_patch.py @@ -151,3 +151,185 @@ async def fake_exec(program: str, *args: str, **kwargs: object) -> FakeProc: assert spawned["args"][:2] == ("exec", "--json") assert spawned["stdin"] == b"prompt" assert (stdout, stderr, returncode) == ("", "", 0) + + +# The real error event codex exec --json emits when the server's strict +# validator refuses --output-schema (captured live on codex-cli 0.144.1). +_REJECTION_STDOUT = ( + '{"type":"error","message":"{\\n \\"type\\": \\"error\\",\\n \\"error\\": {\\n' + ' \\"type\\": \\"invalid_request_error\\",\\n' + ' \\"code\\": \\"invalid_json_schema\\",\\n' + ' \\"message\\": \\"Invalid schema for response_format ' + "'codex_output_schema'. Please ensure it is a valid JSON Schema.\\\",\\n" + ' \\"param\\": \\"text.format.schema\\"\\n },\\n \\"status\\": 400\\n}"}' +) + +_SUCCESS_STDOUT = ( + '{"type":"item.completed","item":{"id":"item_0","type":"agent_message",' + '"text":"{\\"ok\\":true}"}}' +) + + +def _strict(schema: dict) -> dict: + from swe_af.runtime.codex_harness_patch import _codex_strict_json_schema + + return _codex_strict_json_schema(schema) + + +def test_strict_expressible_accepts_plain_and_nested_strict_objects() -> None: + from swe_af.runtime.codex_harness_patch import _codex_schema_strict_expressible + + assert _codex_schema_strict_expressible( + _strict( + { + "type": "object", + "properties": { + "status": {"type": "string"}, + "note": {"anyOf": [{"type": "string"}, {"type": "null"}]}, + "tags": {"type": "array", "items": {"type": "string"}}, + "when": {"type": ["string", "null"]}, + }, + "$defs": { + "Sub": { + "type": "object", + "properties": {"x": {"type": "integer"}}, + } + }, + } + ) + ) + + +def test_strict_expressible_rejects_free_form_and_typed_maps_and_any() -> None: + from swe_af.runtime.codex_harness_patch import _codex_schema_strict_expressible + + base = {"type": "object", "properties": {"status": {"type": "string"}}} + + free_form = _strict({**base, "properties": {"meta": {"type": "object"}}}) + typed_map = _strict( + { + **base, + "properties": { + "meta": { + "type": "object", + "additionalProperties": {"type": "string"}, + } + }, + } + ) + bare_any = _strict({**base, "properties": {"value": {}}}) + boolean_subschema = _strict({**base, "properties": {"value": True}}) + poisoned_defs = _strict( + { + **base, + "properties": {"sub": {"$ref": "#/$defs/Sub"}}, + "$defs": {"Sub": {"type": "object"}}, + } + ) + object_in_type_list = _strict( + {**base, "properties": {"v": {"type": ["object", "null"]}}} + ) + + for schema in ( + free_form, + typed_map, + bare_any, + boolean_subschema, + poisoned_defs, + object_in_type_list, + ): + assert not _codex_schema_strict_expressible(schema) + + +def _run_patched_execute(tmp_path, monkeypatch, source_schema, run_results): + """Drive the patched CodexProvider.execute with a fake CLI runner. + + Writes the strict schema file the way the codex-native prompt suffix does, + then records every spawned command and pops results from ``run_results``. + Returns (RawResult, recorded_cmds). + """ + import asyncio + + from agentfield.harness import _schema + from agentfield.harness.providers.codex import CodexProvider + + import swe_af.runtime.codex_harness_patch as patch_mod + + apply_codex_harness_patch() + + token = active_provider.set("codex") + try: + _schema.build_prompt_suffix(source_schema, str(tmp_path)) + finally: + active_provider.reset(token) + + cmds: list[list[str]] = [] + + async def fake_run(cmd, prompt, *, env, cwd): + cmds.append(list(cmd)) + return run_results[min(len(cmds) - 1, len(run_results) - 1)] + + monkeypatch.setattr(patch_mod, "_run_codex_cli_with_stdin", fake_run) + + provider = CodexProvider.__new__(CodexProvider) + provider._bin = "codex" + raw = asyncio.run(provider.execute("prompt", {"cwd": str(tmp_path)})) + return raw, cmds + + +def test_execute_reruns_without_output_schema_on_server_rejection( + tmp_path, monkeypatch +) -> None: + """A schema the strict validator refuses server-side triggers exactly one + rerun without --output-schema, keeping --output-last-message, and the + retry's output becomes the result.""" + raw, cmds = _run_patched_execute( + tmp_path, + monkeypatch, + {"type": "object", "properties": {"ok": {"type": "boolean"}}}, + [(_REJECTION_STDOUT, "", 1), (_SUCCESS_STDOUT, "", 0)], + ) + + assert len(cmds) == 2 + assert "--output-schema" in cmds[0] + assert "--output-schema" not in cmds[1] + assert "--output-last-message" in cmds[1] + assert raw.is_error is False + assert raw.result == '{"ok":true}' + + +def test_execute_skips_output_schema_for_inexpressible_schema( + tmp_path, monkeypatch +) -> None: + """A schema strict mode cannot express (free-form dict field) must never be + sent through --output-schema — one invocation, last-message only.""" + raw, cmds = _run_patched_execute( + tmp_path, + monkeypatch, + { + "type": "object", + "properties": { + "ok": {"type": "boolean"}, + "meta": {"type": "object"}, + }, + }, + [(_SUCCESS_STDOUT, "", 0)], + ) + + assert len(cmds) == 1 + assert "--output-schema" not in cmds[0] + assert "--output-last-message" in cmds[0] + assert raw.is_error is False + + +def test_execute_does_not_rerun_on_unrelated_failure(tmp_path, monkeypatch) -> None: + raw, cmds = _run_patched_execute( + tmp_path, + monkeypatch, + {"type": "object", "properties": {"ok": {"type": "boolean"}}}, + [("", "stream error: something unrelated", 1)], + ) + + assert len(cmds) == 1 + assert "--output-schema" in cmds[0] + assert raw.is_error is True