Skip to content

fix(review): stop replay after uncertain DNS failure - #1244

Draft
seonghobae wants to merge 1 commit into
fix-review-gateway-own-free-pool-admission-and-rfrom
fix-review-dns-replay-boundary
Draft

seonghobae wants to merge 1 commit into
fix-review-gateway-own-free-pool-admission-and-rfrom
fix-review-dns-replay-boundary

Conversation

@seonghobae

@seonghobae seonghobae commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor

Issue #1106 replay boundary

A review-tagged orchestrator/free passthrough completion treated a temporary DNS exception, including one nested inside a transport exception, as proof that no provider send began. A candidate transport may already have sent an attempt before surfacing that exception. The request then advanced to another provider without an idempotency contract.

This change keeps the existing ordinary virtual-route DNS behavior. On the review-free route, DNS no longer authorizes cross-provider replay. A statusless retryable transport failure now returns nonretryable typed 502 provider_outcome_unknown. Direct local-slot admission failure and explicit provider model refusal keep their existing distinct behavior.

Exact evidence

  • Parent: PR Review gateway: admit tool requests by evidence and stop unsafe free replay #1227 b63cb4823eaac40106c215b9cd2a31e5ec495401.

  • RED: python -m pytest -q tests/test_passthrough_provider_failover.py::test_free_review_dns_failure_does_not_authorize_replay → 2 failed. Both direct and wrapped synthetic DNS cases returned the fallback model after two mocked provider calls.

  • GREEN at e5481c58abfa0de0fa6ffe487b63382facc1bc9a: the same two cases plus explicit model refusal and local-slot controls → 5 passed. Both DNS cases stop after one mocked provider call with typed nonretryable 502.

  • Bounded local run with the existing project .venv (Python 3.14, OpenAI SDK 2.54.0): /Users/seonghobae/orca/workspaces/contextual-orchestrator/fix-review-gateway-own-free-pool-admission-and-r/.venv/bin/python -m pytest -q tests/test_passthrough_provider_failover.py tests/test_provider_error_taxonomy.py tests/test_rate_limit_aware_admission.py → 184 passed, one inherited pytest config warning, process exit 0. git diff --check passed. A preliminary system-Python run used SDK 3.0.0 and failed the pinned-version assertion; it is not the project-interpreter result.

  • Local composition probe: cherry-picked this exact patch onto fix(review): reject empty text batch before submission #1243@89b7beafbde9a6952af1861e719e99daf97fa56d without conflict. Probe HEAD 230d2f6d75d9a4d80c87f7c75c3fab9d28011527 has tree 2b22bc2bfc13e7e1f384a0c17d00054c9f2d765e, matching git merge-tree. With the same project interpreter, /Users/seonghobae/orca/workspaces/contextual-orchestrator/fix-review-gateway-own-free-pool-admission-and-r/.venv/bin/python -m pytest -q tests/test_review_gateway_admission_contract_1106.py tests/test_passthrough_provider_failover.py tests/test_provider_error_taxonomy.py tests/test_cost_router_boundaries.py tests/test_batch_request_lineage.py from that probe → 262 passed, one inherited pytest config warning, exit 0. This is local composed-code evidence, not a PR-head hosted gate.

Mocked transport calls are boundary evidence, not wire delivery evidence. Keep Draft/HOLD. No merge, deploy, real provider request, company content, or broad load.

@coderabbitai

coderabbitai Bot commented Sep 24, 2026

Copy link
Copy Markdown

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant