fix(azure): preserve reasoning IDs and output-text annotations in Responses replay

[vidarak]

Summary

Preserve Azure-required reasoning IDs and assistant output_text.annotations through Responses replay conversion and preflight validation. Non-Azure Responses endpoints retain their existing ID-stripping behavior.

Builds on the merged Azure detection / newest-only replay handling, including 267a6b7. The transport reuses _is_azure_responses() for wire-shape classification instead of broadening _is_azure_foundry_responses(). This PR leaves main's replay-selection and post-tool suppression policy unchanged.

Based on the original PR #63264 by @LeonSGP43 and the report in #63257. Retains hostname-aware base_url_host_matches() detection requested in review; Azure-looking URL paths and look-alike host suffixes do not activate the workaround.

Closes #63257

Changes

• Preserve {type, id, encrypted_content, summary} on replayed Azure reasoning items through conversion and preflight.
• Add annotations: [] only to assistant output_text parts, never image/input-text parts.
• Reuse upstream's Azure Responses predicate in the transport; forward provider/base URL to both preflight call sites without a redundant Azure boolean.
• Keep the auxiliary Codex adapter's Azure wire-shape handling; that path bypasses the transport.

Production files: agent/codex_responses_adapter.py, agent/transports/codex.py, agent/auxiliary_client.py, agent/turn_api_request.py, and agent/turn_api_call.py.

Policy boundary

For a custom provider such as az on *.openai.azure.com, main retains newest-only reasoning after tools. This PR preserves that selection and repairs the replayed item's fields. Registered azure-foundry providers and *.services.ai.azure.com still use main's existing post-tool suppression. No new replay policy is introduced.

Validation

Run via scripts/run_tests.sh (clean environment and per-file subprocess isolation):

scripts/run_tests.sh tests/agent/test_auxiliary_client.py tests/agent/test_azure_foundry_preflight_propagation.py tests/agent/test_codex_responses_adapter.py tests/agent/transports/ tests/run_agent/test_run_agent_codex_responses.py tests/run_agent/test_native_compaction.py tests/run_agent/test_provider_parity.py -q
scripts/run_tests.sh tests/agent/test_surrogate_chokepoints.py tests/agent/test_prompt_cache_ttl_propagation.py -q

• Combined targeted regression sweep: 869 passed, 0 failed across 22 files.
• Dropping base_url from streaming preflight was re-injected experimentally: both hostname-based cases failed; restoring it passed.
• Policy-boundary regression first failed on the previous PR implementation for a custom provider on openai.azure.com; passes after restoring upstream's suppression predicate.
• Existing Azure ID, annotation scoping, hostname, Harmony composition, auxiliary, and preflight-propagation coverage retained. Streaming preflight coverage asserts the normalized payload rather than redundant call arguments.
• git diff --check: clean.

These are local regression tests, not a new live Azure endpoint validation or a claim that the full repository suite ran.

[alt-glitch]

This was generated by AI during triage.

Duplicate of open #63264: both preserve Azure Foundry encrypted-reasoning replay IDs through the Responses adapter and preflight paths; #63264 is the earlier tested implementation.

[vidarak]

Duplicated because there's no response with required corrections on the original PR. The original PR is based on the patch I submitted in my bug report (#63257). Please set a strict timeline for a response with corrections on #63264 - and close it if not corrected there (accepting this PR instead).

[teknium1]

Thanks for carrying the Azure-specific replay fix forward. The premise remains present on current main: agent/codex_responses_adapter.py:428-438 strips replayed reasoning IDs, and preflight strips them again at agent/codex_responses_adapter.py:694-712; both preflight call sites are active in agent/conversation_loop.py:1823-1828 and 1981-1986.

Suggested changes

• During salvage, preserve current main's existing base_url=agent.base_url at agent/chat_completion_helpers.py:1079 (introduced by 6bc8d68ad7) rather than duplicating that overlapping hunk. The Azure flag propagation, narrow payload shape, and hostname-aware matching remain relevant.

Automated hermes-sweeper review.

[alt-glitch]

This was generated by AI during triage.

Correction: #67667 is related to #63264, not a duplicate. Both preserve Azure Foundry replay IDs, but #67667 replaces #63264's substring endpoint detection with hostname-aware matching and adds current-path coverage. Please choose or consolidate the approach; a rebase must retain current main's existing base_url propagation.

[alt-glitch]

This was generated by AI during triage.

Correction: this is related to #63264 rather than a duplicate. The current head keeps the same Azure replay fix but materially changes endpoint detection to hostname-aware matching; maintainers should choose the corrected implementation.

[alt-glitch]

This was generated by AI during triage.

Correction: #67667 is related to, not a duplicate of, #63264. The current diff uses hostname-aware Azure endpoint matching rather than the older substring approach; both PRs are open and require maintainer selection.

[vidarak]

Thanks for carrying the Azure-specific replay fix forward. The premise remains present on current main: agent/codex_responses_adapter.py:428-438 strips replayed reasoning IDs, and preflight strips them again at agent/codex_responses_adapter.py:694-712; both preflight call sites are active in agent/conversation_loop.py:1823-1828 and 1981-1986.

Suggested changes

• During salvage, preserve current main's existing base_url=agent.base_url at agent/chat_completion_helpers.py:1079 (introduced by 6bc8d68ad7) rather than duplicating that overlapping hunk. The Azure flag propagation, narrow payload shape, and hostname-aware matching remain relevant.

Automated hermes-sweeper review.

Corrected

[vidarak]

Rebased onto current main (40e0e7a) and resolved all conflicts.

Changes since last push:

• Resolved 4 merge conflicts from the upstream refactor of codex_responses_adapter.py (Harmony token sanitization, reasoning item restructure)
• Preserved upstream sanitize_harmony_tokens additions alongside is_azure_foundry flag
• Kept type-annotated reasoning_item: Dict[str, Any] with Azure Foundry ID preservation logic
• Fixed test indentation and restored missing lines from merge
• All 3 Azure-specific tests pass (test_azure_foundry_build_kwargs_keeps_reasoning_id, test_azure_foundry_preflight_keeps_reasoning_id, test_prompt_cache_retention_omitted_for_non_mantle_endpoints[azure])

The PR is now mergeable (mergeable_state: blocked, awaiting maintainer review).

[vidarak]

Self-review of the previous force-push — issues found and fixed

Reviewed my own rebase before asking for another look, and found a self-defeating regression plus several conflict-resolution artefacts. All fixed in adcfc3ec9.

The critical one

The previous push removed base_url=agent.base_url from the build_kwargs() call in build_api_kwargs(), labelled as a "duplicate kwarg". It was not duplicated — there is exactly one occurrence in that call on main.

That parameter is what the transport keys the Azure detection off:

is_azure_foundry = _is_azure_foundry_base_url(params.get("base_url"))

So the main conversation loop passed base_url=None, the transport concluded the endpoint was not Azure, and the reasoning id got stripped again. The fix this PR exists to deliver did not fire on the primary code path. Only the auxiliary client worked, since it reads self._client.base_url independently.

Demonstrated:

WITH base_url    -> reasoning id present: True   {'type':'reasoning',...,'id':'rs_123'}
WITHOUT base_url -> reasoning id present: False  {'type':'reasoning',...,'status':'completed'}

Three unrelated features read the same parameter and regressed silently with it:

Consumer	Impact
_default_prompt_cache_retention_for_request	Bedrock Mantle 24h caching lost — verified 24h -> None
_resolve_issuer_kind / _classify_responses_issuer	Cross-issuer reasoning replay guard blinded
convert_messages	Same Azure detection, same failure

Existing tests could not catch this: they call build_kwargs() directly with an explicit base_url, bypassing the call site that lost it. Green CI was actively misleading.

agent/chat_completion_helpers.py has consequently dropped out of this PR's diff altogether — restoring the line returned the file to its base state, which is where it should have been all along.

Also fixed

• Four column-0 continuation lines left by conflict resolution — conversation_loop.py x2, transports/codex.py signature and call. Parsed only because Python tolerates arbitrary continuation indent; violates PEP8 E128 and the surrounding style.
• Mangled comment in _preflight_codex_input_items() — an if block had been spliced into the middle of a sentence, orphaning # above for local deduplication via seen_ids.. Restored coherent prose and the upstream multi-line dict form.
• Three assertions silently deleted from test_preflight_codex_input_items_drops_short_id_for_github_responses (status, phase, content). Nothing in this PR justified weakening a Copilot test.
• Missing blank lines between the new test methods (PEP8 E301).

Test coverage added

• test_azure_foundry_reasoning_id_survives_full_build_api_kwargs — drives the whole _build_api_kwargs path rather than the transport in isolation. Verified it catches the regression: re-introducing the deleted line makes it fail with KeyError: 'id', restoring it makes it pass.
• test_non_azure_reasoning_id_stripped_through_full_build_api_kwargs — guards the OpenAI/Codex 404 path end-to-end.
• Negative-path coverage for is_azure_foundry=False in both _chat_messages_to_responses_input and _preflight_codex_input_items, so the Azure carve-out cannot silently leak.
• Parametrised hostname-scoping test proving relay/proxy URLs carrying Azure domains in their path, and lookalike hosts like services.ai.azure.com.evil.example, do not false-positive. The docstrings claimed this protection; nothing tested it.

Verification

Run on Linux / Python 3.11:

tests/run_agent/test_provider_parity.py        56 passed
tests/agent/transports/                       221 passed
tests/agent/test_codex_responses_adapter.py    20 passed
tests/agent/test_auxiliary_client.py          157 passed, 3 failed
                                             ---------------------
                                              454 passed

The 3 failures are pre-existing (missing pytest-asyncio) and reproduce identically with these changes stashed.

Still open for maintainers

One thing I deliberately did not change, since it is a design call: Azure Foundry detection is now implemented three times — run_agent._is_azure_foundry_url, codex._is_azure_foundry_base_url, and an inline expression in auxiliary_client.py. Worth extracting a single helper into utils next to base_url_host_matches, but that touches call sites beyond this fix's scope. Happy to do it here if preferred.

[vidarak]

Post-merge review + fixes (e568fe290)

I reviewed the conflict-resolution merge (66687dc62) that combined this PR's Azure Foundry reasoning replay with upstream's sanitize_harmony_tokens (9ceb0858a). Both feature sets survived, but the merge introduced two defects that the existing tests could not catch. Both are now fixed and pushed.

Note: an earlier revision of this branch (125a61a5c) also hoisted the Foundry host list into a new utils.is_azure_foundry_host shared by run_agent.py and auxiliary_client.py. That was scope creep — neither of those files was buggy, and the only thing that broke without the helper was the test class written to test it. It has been dropped; the branch now touches 3 files instead of 6.

🔴 Split-brain Azure Foundry detection

agent/transports/codex.py ended up with two different Foundry predicates gating two decisions that must agree:

Decision	Predicate used	Matches on
Post-tool reasoning suppression	_is_azure_foundry_responses(params)	provider id OR host
Wire-shape normalization (reasoning id, annotations)	_is_azure_foundry_base_url(base_url)	host only

A registered provider: azure-foundry pointed at a gateway/proxy URL therefore got the suppression but not the reasoning id this PR exists to preserve — exactly the failure mode #63257 fixes. Reproduced against the merge commit:

provider=azure-foundry  base_url=https://proxy.corp.example/v1
  -> {'type': 'reasoning', 'encrypted_content': 'enc', 'summary': []}     # id missing
provider=None           base_url=https://x.services.ai.azure.com/...
  -> {'type': 'reasoning', ..., 'id': 'rs_1'}                            # id present

Fix: both decisions now key off the provider-aware _is_azure_foundry_responses. Two call sites; _is_azure_foundry_base_url stays as the URL-only helper it always was.

🔴 annotations stamped onto image parts

The Azure branch defaulted annotations: [] on every assistant content part — four duplicated inline loops, one of which runs over input_image parts. annotations has no member in the Responses schema for images:

{'type': 'input_image', 'image_url': 'https://…/a.png', 'annotations': []}   # undocumented shape

Fix: extracted _apply_azure_output_text_annotations(), scoped strictly to output_text, replacing all four loops.

🟡 Test coverage gap (the reason both slipped through)

The merged tests exercised each feature in isolation, so a merge that dropped either half still went green:

• tests/agent/transports/test_codex_transport.py:261 — Azure preflight tested without sanitize_harmony_tokens=True
• tests/agent/test_codex_responses_adapter.py:98,125,187-190 — Harmony sanitization tested without is_azure_foundry=True
• The live-path fixture _azure_reasoning_item() (tests/run_agent/test_run_agent_codex_responses.py:353) has no id, so test_build_api_kwargs_azure_foundry_non_tool_preserves_reasoning asserted only that a reasoning item exists — never that the id survives, which is the whole point of the fix

Added TestAzureFoundryWireShape: behaviour contracts for which items get Foundry-only fields, provider-vs-host detection agreement, hostname substring false-positives (https://evil.com/openai.azure.com/v1, https://openai.azure.com.evil.net/v1), and a combined Azure + Harmony test proving the two merged features compose on the same request.

Verification

Both guards were validated by re-injecting each bug — a guard that can't detect its own failure mode is worthless:

inject bug 1 (host-only predicate)  -> 2 failed  (only the provider-detection cases)
inject bug 2 (annotations on all)   -> 1 failed  (only the output_text-scoping case)
both fixed                          -> 115 passed

Regression sweep across the codex transport, adapter, auxiliary-client and native-compaction suites:

with changes:  364 passed, 10 failed
baseline:      249 passed, 10 failed

Same 10 pre-existing failures in both runs — all ModuleNotFoundError: No module named 'openai' in my checkout's environment, unrelated to these changes. Net +115 passing tests, zero regressions.

Remaining (not addressed here — happy to take direction)

• tests/run_agent/test_native_compaction.py:540-562 passes native_compaction_eligible=True directly, bypassing the production derivation from context_management. A full config-to-wire test through build_kwargs with a persisted checkpoint would close that gap; it's orthogonal to this PR's scope, so I left it alone rather than widen the diff.

[alt-glitch]

This was generated by AI during triage.

Related to #63264, not a duplicate: this current-main implementation retains the Azure reasoning replay-ID repair while using hostname-aware endpoint matching requested in review.

[Enough1122]

AI code review — automated review for reference, author can ignore or act on any point.

fix(codex): preserve azure reasoning replay ids with hostname-aware matching

Unifying the Foundry predicate (suppression and wire shape now key off _is_azure_foundry_responses) and making it hostname-aware is the right fix for #63257, with excellent behavior-contract tests (including the Foundry+Harmony compose case). Observations:

1. agent/codex_responses_adapter.py — the Azure reasoning branch rebuilds the replay item with an explicit whitelist (type, encrypted_content, summary, id), dropping status/response_id/other fields that the non-Azure branch preserves. That's deliberate ("preserve only the fields it accepts") and tested, but it's an asymmetric payload contract: a future Responses-surface change that adds a field to reasoning items would be silently dropped for Azure. A comment pointing at the verified Azure schema version would help future maintainers know what the whitelist was validated against.
2. agent/transports/codex.py _is_azure_foundry_base_url — hostname-aware matching correctly rejects https://openai.azure.com.evil.net/v1 and path-only domains (tests pin this). One question: base_url_host_matches semantics for the bare host openai.azure.com itself (no subdomain) — the tests cover r.openai.azure.com (subdomain) but not the bare domain; if the helper requires a subdomain match, a user pointing base_url directly at https://openai.azure.com could be misclassified as non-Foundry. Worth a quick check/add a test for the bare-host form.
3. run_agent.py _is_azure_foundry_url — mirrors the transport helper; the duplication is small and both are hostname-aware. Note _is_azure_openai_url (openai.azure.com only) and _is_azure_foundry_url (both hosts) overlap for openai.azure.com — if any call site should distinguish "legacy Azure OpenAI" from "Foundry", the overlap could matter; currently both routes get Foundry wire-shape treatment, which is presumably intended.
4. agent/auxiliary_client.py — the auxiliary adapter now applies the Foundry shape too (good catch in the PR; auxiliary calls bypass the transport). The is_azure_foundry flag is derived from the client's base_url — confirm relay/proxy setups where the client base_url differs from the actual endpoint still get the right classification (the provider-config path used by the transport isn't available here).
5. Minor: encrypted = ri.get("encrypted_content") is hoisted before the dedup check; in the Azure branch it's written into the replay item unconditionally — if a reasoning item ever lacks encrypted_content, the payload carries None (the non-Azure branch has the same behavior via dict comprehension). Consistent, no change needed.

[vidarak]

Refactor port: tests restored, one detection gap fixed (508fc174e6)

The previous push (a129010fa) rebased this fix onto the refactored turn_api_request / turn_api_call call sites. It was regenerated from a source-only local patch, so it silently dropped every test this branch had accumulated (TestAzureFoundryWireShape, the provider-vs-host agreement guard, the output_text-only annotations guard, the Foundry + Harmony compose test). That was a mistake, not a scope decision. This commit restores them against the current call graph and adds the propagation coverage the new call sites need.

🔴 Found by the restored tests: openai.azure.com dropped from the transport predicate

_is_azure_foundry_responses in agent/transports/codex.py matched only services.ai.azure.com, while the auxiliary client hunk in the same commit (and the pre-refactor branch) also matched openai.azure.com resource endpoints. A custom provider pointed at https://<res>.openai.azure.com/… therefore got the Foundry wire shape on compression/flush calls but not on the main turn — the same split-brain class fixed on 15 Aug, re-introduced by the port.

The predicate now covers both hostnames; matching stays hostname-aware (https://evil.com/openai.azure.com/v1 and https://openai.azure.com.evil.net/v1 are pinned as non-Foundry).

Tests

File	Coverage
tests/agent/test_azure_foundry_preflight_propagation.py (new)	Drives the real turn_api_request.build_api_request and turn_api_call.perform_api_call with ResponsesApiTransport; asserts the wire payload. Provider-detected behind a proxy, host-detected on both hostnames, untouched for OpenAI / Codex / Copilot / xAI / look-alikes. Current-main equivalent of the "survives full build_api_kwargs" guard from 1 Aug.
transports/test_codex_transport.py::TestAzureFoundryWireShape	build_kwargs and preflight_kwargs contracts, predicate table, post-tool suppression / wire-shape agreement on one predicate.
test_codex_responses_adapter.py	Reasoning id kept for Foundry / stripped otherwise; annotations only on output_text (helper contract vs input_image / input_text / refusal, existing annotations preserved); Foundry + Harmony sanitisation compose.
test_auxiliary_client.py::TestCodexAdapterAzureFoundryReasoningReplay	Host detection on both hostnames; runtime-provider detection behind a proxy via set_runtime_main; non-Foundry unchanged.

Verification

Each guard was validated by re-injecting its bug:

host-only predicate (services.ai.azure.com only)      -> 5 failed
annotations stamped on every part                     -> 1 failed
provider/base_url not forwarded in turn_api_request   -> 4 failed
all fixed                                             -> 192 passed (targeted)

Regression sweep (adapter, transports/, auxiliary client, run_agent codex / native-compaction / provider-parity, surrogate chokepoints, cache-TTL propagation): 774 passed, 0 failed.

[vidarak]

Simplified in 1331e9a to build directly on the merged Azure Responses predicate from 267a6b7.

• Reuse _is_azure_responses() for Azure wire-shape handling rather than modifying _is_azure_foundry_responses().
• Leave main's newest-only replay and narrower post-tool suppression policy unchanged. In particular, custom-provider openai.azure.com endpoints retain main's newest-only post-tool reasoning replay, now with the required fields preserved.
• Remove the redundant Azure boolean at both turn preflight call sites; provider/base URL still reach preflight.
• Retain the existing regression coverage and strengthen streaming-preflight checks for both hostname-based routes and provider-detected proxies. These assert the normalized payload, not redundant call arguments.

The remaining fix is specifically reasoning-ID preservation and output_text.annotations through conversion, preflight, and the auxiliary path. Production diff against the PR base is now 64 additions / 11 deletions, down from 75 / 13; coverage was not cut to achieve that reduction.

Verification via the repository's clean-environment scripts/run_tests.sh: 869 passed, 0 failed across 22 files. The policy-boundary test failed on the previous patch, and experimentally removing streaming base_url forwarding failed both hostname cases; all pass with the cleanup. Independent diff review found no logic/security issues. git diff --check is clean.

Updated the PR title/description to reflect the current post-refactor paths and its relationship to the merged fix. These are local regression results, not a new live Azure endpoint test or a full-repository CI claim.

[vidarak]

Rebased onto current main

Force-pushed the PR branch from f19dfb817a to c0f74d27fa, rebased onto current upstream main.

The previous branch had become unmergeable because the refactor changed the replay/conversion call graph and moved the relevant context. The port now preserves current-main behavior while carrying the Azure-specific wire-shape fix:

• preserve Azure reasoning replay id through conversion and preflight
• add annotations: [] only to assistant output_text parts
• reuse the provider-aware upstream Azure Responses predicate for suppression and wire shape
• cover both services.ai.azure.com and openai.azure.com with hostname-aware matching
• retain base_url propagation into the live request path
• restore regression tests, including provider-detected proxy routes and Azure + Harmony composition

Validation on the rebased branch: 203 passed across the adapter and transport suites; git diff --check clean; Python compile checks passed for all touched production files.