From c8b502c3e1762a22aae39a93026d9d6dbedcfc1d Mon Sep 17 00:00:00 2001 From: Dongkeun Lee Date: Fri, 21 Aug 2026 11:35:41 -0400 Subject: [PATCH] fix(crosscheck): parse fenced Azure Pi verdicts --- bin/fm-crosscheck-azure-model-guest.sh | 154 ++++++++++++++++++-- bin/fm-crosscheck.py | 14 ++ docs/azure-crosscheck.md | 9 +- tests/fm-crosscheck-azure.test.sh | 187 ++++++++++++++++++++++++- tests/fm-crosscheck.test.sh | 45 ++++++ 5 files changed, 388 insertions(+), 21 deletions(-) diff --git a/bin/fm-crosscheck-azure-model-guest.sh b/bin/fm-crosscheck-azure-model-guest.sh index 3ad768a3ab2..58e59d9b3e6 100755 --- a/bin/fm-crosscheck-azure-model-guest.sh +++ b/bin/fm-crosscheck-azure-model-guest.sh @@ -237,27 +237,153 @@ case "$HARNESS" in python3 - "$BASE/pi-events.jsonl" "$RESULT" <<'PY' import json import pathlib +import re import sys source = pathlib.Path(sys.argv[1]) destination = pathlib.Path(sys.argv[2]) -final = None -ended = False -for line in source.read_text(encoding="utf-8").splitlines(): - event = json.loads(line) - if event.get("type") == "turn_end": - message = event.get("message") or {} - if message.get("role") == "assistant" and message.get("stopReason") == "stop": + +# BEGIN PI_VERDICT_BODY_CONTRACT +PI_FENCED_BLOCK_RE = re.compile( + r"```[A-Za-z0-9_+.-]*[ \t]*\r?\n(?P.*?)\r?\n?```", + re.DOTALL, +) + + +def pi_verdict_body(final_text: str) -> str: + """Return the JSON body of a Pi reviewer's final assistant text. + + An UNTERMINATED fence anywhere in the message refuses outright, before the + block count is even consulted. That ordering is the whole safety property. + A truncated verdict fence contributes ZERO complete blocks, so a model that + emitted any complete fence earlier in the same message - a draft, an + example, a quoted snippet - left the count at exactly one, and this + returned THAT EARLIER BLOCK as the verdict while silently discarding the + truncated real one. `stopReason` is `stop` in that shape (the exact live + condition seen on attempt 3), and the parse SUCCEEDS on the wrong block, so + nothing else downstream catches it: a superseded draft gets certified as + the review. That is strictly worse than the failure it replaced, which at + least failed loudly. + """ + + stripped = final_text.strip() + # An odd number of fence markers means one was opened and never closed. + # Refusing on the marker count rather than on "no complete block found" + # is what makes a preceding complete fence unable to rescue a truncated + # one; returning the raw text sends it to the parser, which fails. + if stripped.count("```") % 2: + return stripped + blocks = PI_FENCED_BLOCK_RE.findall(stripped) + if len(blocks) != 1: + return stripped + # The block must be the ONLY JSON-bearing content in the message. An even + # fence count is not enough on its own: a COMPLETE example fence followed + # by a truncated BARE verdict also counts one block, and unwrapping there + # would certify the example and discard the real answer. Prose carries no + # braces, so this still tolerates a wrapper while refusing every shape + # where a second candidate verdict exists. + remainder = PI_FENCED_BLOCK_RE.sub("", stripped, count=1) + if "{" in remainder or "}" in remainder: + return stripped + return blocks[0].strip() +# END PI_VERDICT_BODY_CONTRACT + + +turn_count = 0 +attempt_turn_count = 0 +agent_ended = False +final_text = None +final_stop_reason = None +final_error = None +for line_number, line in enumerate(source.read_text(encoding="utf-8").splitlines(), start=1): + if not line.strip(): + continue + try: + event = json.loads(line) + except (json.JSONDecodeError, ValueError, RecursionError) as exc: + raise SystemExit( + f"model guest: Pi returned malformed JSON events at line {line_number}: {exc}" + ) + if not isinstance(event, dict): + raise SystemExit( + f"model guest: Pi returned a non-object event at line {line_number}" + ) + event_type = event.get("type") + if event_type == "turn_end": + if agent_ended: + raise SystemExit("model guest: Pi emitted a turn after agent completion") + turn_count += 1 + attempt_turn_count += 1 + final_text = None + final_stop_reason = None + final_error = None + message = event.get("message") + if isinstance(message, dict) and message.get("role") == "assistant": + stop_reason = message.get("stopReason") + if isinstance(stop_reason, str): + final_stop_reason = stop_reason + error_message = message.get("errorMessage") + if isinstance(error_message, str) and error_message.strip(): + final_error = error_message.strip() content = message.get("content") if isinstance(content, str): - final = content + final_text = content elif isinstance(content, list): - final = "".join(part.get("text", "") for part in content if isinstance(part, dict)) - elif event.get("type") == "agent_end": - ended = True -if not ended or not final: - raise SystemExit("model guest: Pi stopped without a successful verdict") -value = json.loads(final) + text_parts = [ + part["text"] + for part in content + if isinstance(part, dict) + and part.get("type") == "text" + and isinstance(part.get("text"), str) + ] + if text_parts: + final_text = "".join(text_parts) + elif event_type == "agent_end": + if agent_ended: + raise SystemExit("model guest: Pi emitted duplicate agent completion") + agent_ended = True + elif event_type == "auto_retry_start": + if not agent_ended: + raise SystemExit( + "model guest: Pi announced a retry while its agent was still running" + ) + if attempt_turn_count == 0: + raise SystemExit( + "model guest: Pi announced a retry after an attempt that executed no turn" + ) + if final_stop_reason == "stop": + raise SystemExit( + "model guest: Pi announced a retry after a successful assistant turn" + ) + agent_ended = False + attempt_turn_count = 0 + final_text = None + final_stop_reason = None + final_error = None +if turn_count == 0: + raise SystemExit("model guest: Pi completed without executing a turn") +if not agent_ended: + raise SystemExit("model guest: Pi stopped before agent completion") +if attempt_turn_count == 0: + raise SystemExit("model guest: Pi final attempt completed without executing a turn") +if final_stop_reason != "stop": + raise SystemExit( + "model guest: Pi final assistant turn did not stop successfully: " + f"stopReason={final_stop_reason!r}" + + (f": {final_error[:500]}" if final_error else "") + ) +if final_text is None or not final_text.strip(): + raise SystemExit("model guest: Pi completed without a verdict artifact") +body = pi_verdict_body(final_text) +try: + value = json.loads(body) +except (json.JSONDecodeError, ValueError, RecursionError) as exc: + raise SystemExit( + f"model guest: Pi returned a malformed verdict artifact: {exc}; " + f"final assistant text began {body[:240]!r}" + ) +if not isinstance(value, dict): + raise SystemExit("model guest: Pi verdict artifact must be an object") destination.write_text(json.dumps(value, sort_keys=True, separators=(",", ":")) + "\n", encoding="utf-8") PY ;; diff --git a/bin/fm-crosscheck.py b/bin/fm-crosscheck.py index 4dd87b2f8ca..268b9e06b22 100755 --- a/bin/fm-crosscheck.py +++ b/bin/fm-crosscheck.py @@ -3795,6 +3795,7 @@ def pi_reviewer_command() -> list[str]: # never closes its fence, so it yields zero complete blocks, falls through to # the bare text, and still fails to parse - and `stopReason` refuses it one # step earlier regardless. Both remain pinned by tests. +# BEGIN PI_VERDICT_BODY_CONTRACT PI_FENCED_BLOCK_RE = re.compile( r"```[A-Za-z0-9_+.-]*[ \t]*\r?\n(?P.*?)\r?\n?```", re.DOTALL, @@ -3837,10 +3838,12 @@ def pi_verdict_body(final_text: str) -> str: if "{" in remainder or "}" in remainder: return stripped return blocks[0].strip() +# END PI_VERDICT_BODY_CONTRACT def pi_review_result(output: str) -> tuple[dict[str, Any], int]: turn_count = 0 + attempt_turn_count = 0 agent_ended = False final_text: str | None = None final_stop_reason: str | None = None @@ -3869,6 +3872,7 @@ def pi_review_result(output: str) -> tuple[dict[str, Any], int]: if agent_ended: tool_fail("Pi reviewer emitted a turn after agent completion") turn_count += 1 + attempt_turn_count += 1 final_text = None final_stop_reason = None final_error = None @@ -3908,11 +3912,21 @@ def pi_review_result(output: str) -> tuple[dict[str, Any], int]: # refused above, so the original defense stands. if not agent_ended: tool_fail("Pi reviewer announced a retry while its agent was still running") + if attempt_turn_count == 0: + tool_fail("Pi reviewer announced a retry after an attempt that executed no turn") + if final_stop_reason == "stop": + tool_fail("Pi reviewer announced a retry after a successful assistant turn") agent_ended = False + attempt_turn_count = 0 + final_text = None + final_stop_reason = None + final_error = None if turn_count == 0: tool_fail("Pi reviewer completed without executing a turn") if not agent_ended: tool_fail("Pi reviewer stopped before agent completion") + if attempt_turn_count == 0: + tool_fail("Pi reviewer final attempt completed without executing a turn") if final_stop_reason != "stop": tool_fail( "Pi reviewer final assistant turn did not stop successfully: " diff --git a/docs/azure-crosscheck.md b/docs/azure-crosscheck.md index 4adb8c1e999..1e4e5a7dd9e 100644 --- a/docs/azure-crosscheck.md +++ b/docs/azure-crosscheck.md @@ -83,9 +83,10 @@ The primary review family is a registered cross-family lane, driven by Pi as tha For that profile the packaged compartment credential is the api-key `models.json` (not a codex `auth.json`), pinned to exactly `https://api.fireworks.ai/inference/v1` - chat completions only; any other baseUrl, including a Responses API surface, refuses before staging. The archive gate also refuses a model-level `compat` that is not the lane's pinned compat, so a credential cannot weaken the truncated-verdict refusal on its way into a compartment. pi gives model-level `baseUrl`/`api` fields precedence over the provider level, so the inspection, the archive gate, and the model guest all refuse a model entry carrying either field; the pinned provider level owns both. `effective_provider_host` is model-aware: a cross-family review derives its own lane's host, today `api.fireworks.ai`, as its single egress host and refuses a conflicting configured `provider_host`, while the codex-family fallback keeps its `chatgpt.com` derivation. -The executing identity is the non-secret Foundry resource/deployment binding (an api key names no upstream account); the api key and anything derived from it never enter identity, ledger, or output. +The executing identity is the non-secret provider-slot, endpoint, and model binding (an api key names no upstream account); the api key and anything derived from it never enter identity, ledger, or output. The interim claude reviewer lane is retired end to end: no `api.anthropic.com` host derivation, no `.credentials.json` packaging or boot copy, and no claude launch branch in the model guest. -Honest limit, corrected twice. This Azure-compartment lane does not run today, and until 2026-08-21 both reasons given for that were wrong. It is not that the image lacks `pi` (stale), and it is not ONLY that the lane is switched off. `$FM_HOME/config/crosscheck-azure.json` carries `"enabled": false`, set by an operator on 2026-08-20 - but flipping that flag would NOT have restored the codex-family path, because a second, independent, in-code blocker sat behind it: the archive gate derived the executing-account identity a second time and returned the BARE account id while the admitted identity carried a `codex:` / `openai-codex:` prefix, so `archived_identity != reviewer_account_identity` was structurally always true. A live run refused there at 04:43Z, before any billable resource. NO codex-family compartment review has ever run; every one of the resource group's historical `fm-crosscheck-model-*` deployments is on the cross-family path, which passed only because both sides there read one shared value. Fixed 2026-08-21, and the FIRST attempt at that fix was incomplete in a way worth recording: it made the host reader and the host archive gate share one derivation, but the model guest carried a THIRD copy that still derived the bare account id, so the refusal simply moved from staging into a booted, paid VM. All three now derive the prefixed identity; the guest cannot import the others because it ships self-contained onto the VM, so `model_guest_executing_account_unit` EXECUTES the guest's own credential block against the host readers to prove they agree, and is red on either the two- or the three-derivation form. The executable cross-family lane today is still the local Pi reviewer. +The Pi model guest applies the same byte-pinned verdict-body parser as the local gate: a bare JSON object or exactly one complete fenced JSON object with brace-free surrounding prose is accepted, while unterminated fences, multiple complete blocks, brace-bearing remainder, malformed JSON, and non-objects fail closed. The guest independently retains the terminal stream contract too: at least one turn, a final assistant `stop`, and exactly one completed agent are required. Pi's explicit `auto_retry_start` may open a continuation only after a completed attempt executed a turn and did not stop successfully; the continuation starts with empty terminal state and must execute its own turn before completing. `model_guest_pi_verdict_unit` executes the exact shipped heredoc and byte-compares its pure fence parser to the host copy so the self-contained image artifact cannot drift back to `json.loads(final)`. +Honest limit, corrected three times. A live 2026-08-21 cross-family attempt for PR #285 provisioned a model compartment and reached the guest, but it did not return a valid verdict: Pi produced one fenced JSON object and the guest's then-naive `json.loads(final)` refused it. Until that attempt, both reasons given for the lane not running were wrong. It is not that the image lacks `pi` (stale), and it is not ONLY that the lane is switched off. `$FM_HOME/config/crosscheck-azure.json` carries `"enabled": false`, set by an operator on 2026-08-20 - but flipping that flag would NOT have restored the codex-family path, because a second, independent, in-code blocker sat behind it: the archive gate derived the executing-account identity a second time and returned the BARE account id while the admitted identity carried a `codex:` / `openai-codex:` prefix, so `archived_identity != reviewer_account_identity` was structurally always true. A live run refused there at 04:43Z, before any billable resource. NO codex-family compartment review has ever run; every one of the resource group's historical `fm-crosscheck-model-*` deployments is on the cross-family path, which passed only because both sides there read one shared value. Fixed 2026-08-21, and the FIRST attempt at that fix was incomplete in a way worth recording: it made the host reader and the host archive gate share one derivation, but the model guest carried a THIRD copy that still derived the bare account id, so the refusal simply moved from staging into a booted, paid VM. All three now derive the prefixed identity; the guest cannot import the others because it ships self-contained onto the VM, so `model_guest_executing_account_unit` EXECUTES the guest's own credential block against the host readers to prove they agree, and is red on either the two- or the three-derivation form. A compartment review has still not completed; the only cross-family verdicts accepted so far came from the local Pi reviewer. The earlier reading of this limit said the built image carries no `pi` binary and needed a rebake. That was measured on 2026-08-16 against gallery version `1.0.1786915905`, whose source managed image `img-fm7c799d-ccm-1.0.0` was built on 2026-08-13 from the pre-Pi declaration and carries no `pi-tarball-sha256` tag (M29 in the owner's mutation ledger, `firstmate-azure-full-completion-mutation-ledger.md`, which lives outside this repository rather than in it). It was already stale when it was written here: `model_image_id` has named `1.0.1787092687` since 2026-08-18T22:45Z. That current version was published 2026-08-18T22:38:08Z from managed image `img-fm7c799d-ccm-1.0.1787091895`, which carries `pi-tarball-sha256` `a69a1859...` and `node-tarball-sha256` `d60acfe0...`, matching `docs/azure-crosscheck/model-image-closure.json` for `pi-coding-agent` 0.84.1 and Node v22.23.2. Only a build from the Pi-carrying declaration writes those tags, its Image Builder run succeeded, and that declaration asserts `/usr/local/bin/pi --version` against the tracked version twice under `set -eu`, before and after the credential purge, so a build that reached distribution cannot have omitted `pi`. What remains unproven is a Pi review actually completing on this image, which is a separate claim from the binary being present. Both readings were guesses about an image that admission never inspected. It does now: the harness attestation guard described under Operator setup reads `pi-tarball-sha256` and `node-tarball-sha256` off the configured image before any model VM exists, so the next time this question is asked the lane answers it from the image rather than from a document, and a wrong `model_image_id` is refused for free instead of discovered on a paid VM. @@ -181,9 +182,7 @@ That loses exactly the numbers a failed compartment review would be most useful **This is a known gap with a follow-up that must land before any compartment timing is relied on: the lane must be stamped at its START.** It was previously described as bound to an image rebake; there is no rebake to wait for, so the follow-up is gated only on the lane being switched on. `docs/crosscheck.md` owns that follow-up, including why stamping `execution_mode` earlier refuses the record instead of fixing it. -These numbers do not exist yet. -The compartment lane is disabled in the operator home, by `"enabled": false` in `$FM_HOME/config/crosscheck-azure.json` rather than by any missing image capability, so no run has executed these phases since they were instrumented. -`bin/fm-crosscheck.sh timings ` shows `-` in the `create`, `stage`, `boot`, and `collect` columns for every local-lane run, which is the honest reading: that lane did not do that work. +No accepted numbers exist yet. The failed 2026-08-21 compartment attempt executed these phases but did not produce the complete Azure reviewer identity record this ledger boundary requires, so its measurements were discarded as described above. The operator-home file still defaults the lane off with `"enabled": false`; that live attempt used the explicit Azure execution-mode opt-in. `bin/fm-crosscheck.sh timings ` therefore still shows `-` in the `create`, `stage`, `boot`, and `collect` columns for local-lane runs, which is the honest reading: those runs did not do that work. ## Operator setup diff --git a/tests/fm-crosscheck-azure.test.sh b/tests/fm-crosscheck-azure.test.sh index 25293c59f32..17fca3a7251 100755 --- a/tests/fm-crosscheck-azure.test.sh +++ b/tests/fm-crosscheck-azure.test.sh @@ -376,6 +376,188 @@ PY pass "the model guest derives the host's executing account, refuses foreign ones, and dispatches every registered lane" } +model_guest_pi_verdict_unit() { + python3 - "$MODEL_GUEST" "$CORE" <<'PY' \ + || fail "model guest Pi verdict parser contract failed" +import json +from pathlib import Path +import subprocess +import sys +import tempfile + +guest_path, core_path = map(Path, sys.argv[1:3]) +guest_source = guest_path.read_text(encoding="utf-8") +core_source = core_path.read_text(encoding="utf-8") + +# The guest is self-contained inside its image-pinned run command. Keep its +# pure fenced-body parser byte-identical to the host contract so neither copy +# can drift weaker while still passing a hand-reimplemented test. +contract_start = "# BEGIN PI_VERDICT_BODY_CONTRACT\n" +contract_end = "# END PI_VERDICT_BODY_CONTRACT" + + +def contract(source): + start = source.index(contract_start) + end = source.index(contract_end, start) + len(contract_end) + return source[start:end] + + +assert contract(guest_source) == contract(core_source), ( + "the model guest fenced-body parser drifted from the host contract" +) + +# EXECUTE the exact verdict heredoc shipped to the paid VM. Testing a helper +# that merely resembles this block would not catch the live json.loads(final) +# failure that prompted this regression. +marker = 'python3 - "$BASE/pi-events.jsonl" "$RESULT" <<\'PY\'\n' +start = guest_source.index(marker) + len(marker) +end = guest_source.index("\nPY\n", start) +guest_block = guest_source[start:end] +assert "PI_VERDICT_BODY_CONTRACT" in guest_block, "extracted the wrong guest block" + + +def assistant(text, stop_reason="stop", error=None): + message = { + "role": "assistant", + "stopReason": stop_reason, + "content": [{"type": "text", "text": text}], + } + if error is not None: + message["errorMessage"] = error + return {"type": "turn_end", "message": message} + + +def events(*items): + return "\n".join(json.dumps(item) for item in items) + "\n" + + +def run_guest(stream): + with tempfile.TemporaryDirectory() as temporary: + root = Path(temporary) + source = root / "pi-events.jsonl" + result = root / "result.json" + script = root / "guest-verdict.py" + source.write_text(stream, encoding="utf-8") + script.write_text(guest_block, encoding="utf-8") + completed = subprocess.run( + [sys.executable, str(script), str(source), str(result)], + capture_output=True, + text=True, + timeout=30, + ) + value = json.loads(result.read_text(encoding="utf-8")) if result.is_file() else None + return completed, value + + +verdict = json.dumps({"verdict": "clear"}) +for label, text in ( + ("bare object", verdict), + ("json fence", f"```json\n{verdict}\n```"), + ("plain fence with harmless prose", f"Here is the verdict:\n```\n{verdict}\n```\nDone."), +): + completed, value = run_guest(events(assistant(text), {"type": "agent_end"})) + assert completed.returncode == 0, (label, completed.stdout, completed.stderr) + assert value == {"verdict": "clear"}, (label, value) + +for label, text in ( + ("unterminated fence", f"```json\n{verdict}"), + ("multiple complete fences", f"```json\n{verdict}\n```\n```json\n{verdict}\n```"), + ( + "complete example then truncated fenced candidate", + f"Example:\n```json\n{verdict}\n```\nFinal:\n```json\n{{\"verdict\":\"block", + ), + ( + "complete example then truncated bare candidate", + f"Example:\n```json\n{verdict}\n```\n{{\"verdict\":\"block", + ), + ( + "complete example then complete bare candidate", + f"Example:\n```json\n{verdict}\n```\n{{\"verdict\":\"blocking\"}}", + ), + ("malformed object", '{"verdict":'), + ("non-object", '[{"verdict":"clear"}]'), +): + completed, value = run_guest(events(assistant(text), {"type": "agent_end"})) + assert completed.returncode != 0, (label, completed.stdout, completed.stderr, value) + assert value is None, (label, value) + +# The parser admits only the terminal successful assistant turn of a completed +# agent. Earlier successful text cannot survive a later failed turn, and an +# agent_end is neither optional nor repeatable. Pi's explicit retry boundary +# remains the one allowed way to continue after an agent_end. +terminal_failures = ( + ("no turn", events({"type": "agent_end"})), + ("no agent end", events(assistant(verdict))), + ("truncated stop", events(assistant(verdict, "length"), {"type": "agent_end"})), + ( + "later failed turn", + events(assistant(verdict), assistant("", "error", "provider failed"), {"type": "agent_end"}), + ), + ( + "turn after completion", + events(assistant(verdict), {"type": "agent_end"}, assistant(verdict)), + ), + ( + "duplicate completion", + events(assistant(verdict), {"type": "agent_end"}, {"type": "agent_end"}), + ), +) +for label, stream in terminal_failures: + completed, value = run_guest(stream) + assert completed.returncode != 0, (label, completed.stdout, completed.stderr, value) + assert value is None, (label, value) + +retry_terminal_failures = ( + ( + "successful attempt retried into an empty attempt", + events( + assistant(verdict), + {"type": "agent_end"}, + {"type": "auto_retry_start"}, + {"type": "agent_end"}, + ), + "retry after a successful assistant turn", + ), + ( + "failed attempt retried into an empty attempt", + events( + assistant("", "error", "provider failed"), + {"type": "agent_end"}, + {"type": "auto_retry_start"}, + {"type": "agent_end"}, + ), + "final attempt completed without executing a turn", + ), + ( + "empty completed attempt opened a retry", + events( + {"type": "agent_end"}, + {"type": "auto_retry_start"}, + ), + "retry after an attempt that executed no turn", + ), +) +for label, stream, expected in retry_terminal_failures: + completed, value = run_guest(stream) + combined = completed.stdout + completed.stderr + assert completed.returncode != 0, (label, combined, value) + assert expected in combined, (label, combined) + assert value is None, (label, value) + +completed, value = run_guest(events( + assistant("", "error", "retryable"), + {"type": "agent_end"}, + {"type": "auto_retry_start"}, + assistant(f"```json\n{verdict}\n```"), + {"type": "agent_end"}, +)) +assert completed.returncode == 0, (completed.stdout, completed.stderr) +assert value == {"verdict": "clear"}, value +print("GUEST Pi verdict parser is byte-bound to the host and fails closed") +PY + pass "the exact model guest accepts one fenced verdict and preserves terminal-turn safety" +} + cross_family_provider_host_unit() { python3 - "$ADAPTER" "$CORE" <<'PY' || fail "model-aware provider host derivation failed" import importlib.util @@ -410,7 +592,7 @@ for lane in module.CROSS_FAMILY_LANES.values(): ) # The model decides the host: every cross-family review derives its own exact -# Foundry host, refuses a conflicting configured host, and the codex-family +# pinned provider host, refuses a conflicting configured host, and the codex-family # fallback keeps its existing derivation. The retired claude harness derives # nothing. for lane in module.CROSS_FAMILY_LANES.values(): @@ -492,7 +674,7 @@ else: "a cross-family ledger record with a foreign provider host validated" ) PY - pass "the reviewer model derives the exact Foundry host and the claude host lane is retired" + pass "the reviewer model derives the exact provider host and the claude host lane is retired" } cross_family_credential_lane_unit() { @@ -1937,6 +2119,7 @@ adapter_mode_unit cross_family_provider_host_unit cross_family_credential_lane_unit model_guest_executing_account_unit +model_guest_pi_verdict_unit identity_outcome_unit account_and_cleanup_identity_unit bridge_security_unit diff --git a/tests/fm-crosscheck.test.sh b/tests/fm-crosscheck.test.sh index 319f0057d2e..949b309c276 100755 --- a/tests/fm-crosscheck.test.sh +++ b/tests/fm-crosscheck.test.sh @@ -1971,6 +1971,51 @@ verdict, turn_count = module.pi_review_result(stream([ assert verdict == {"verdict": "clear"}, verdict assert turn_count == 2, turn_count +# A retry is a continuation of a failed attempt, never a way to reopen a +# successfully completed review. The refusal occurs at the retry boundary, +# before a later empty agent_end could reuse the successful verdict. +try: + module.pi_review_result(stream([ + {"type": "agent_start"}, + assistant_turn(verdict_text, "stop"), + {"type": "agent_end", "messages": []}, + {"type": "auto_retry_start"}, + {"type": "agent_end", "messages": []}, + ])) +except module.CrosscheckToolError as exc: + assert "retry after a successful assistant turn" in str(exc), str(exc) +else: + raise AssertionError("a successful attempt was reopened as a retry") + +# Opening a valid retry clears all terminal state and starts a new per-attempt +# turn count. An empty final attempt therefore cannot inherit either a verdict +# or the provider error from the completed failed attempt. +try: + module.pi_review_result(stream([ + {"type": "agent_start"}, + assistant_turn("", "error", RATE_LIMIT), + {"type": "agent_end", "messages": []}, + {"type": "auto_retry_start"}, + {"type": "agent_end", "messages": []}, + ])) +except module.CrosscheckToolError as exc: + assert "final attempt completed without executing a turn" in str(exc), str(exc) + assert "RateLimitReached" not in str(exc), str(exc) +else: + raise AssertionError("an empty retry attempt inherited stale terminal state") + +# The completed attempt that earns a retry must itself have executed a turn. +try: + module.pi_review_result(stream([ + {"type": "agent_start"}, + {"type": "agent_end", "messages": []}, + {"type": "auto_retry_start"}, + ])) +except module.CrosscheckToolError as exc: + assert "retry after an attempt that executed no turn" in str(exc), str(exc) +else: + raise AssertionError("an empty completed attempt opened a retry") + # Exhausted retries surface the PROVIDER error, never the retry mechanics. try: module.pi_review_result(stream([