Uh oh!
There was an error while loading. Please reload this page.
test(e2e): C7 /v1/responses dispatch + provider mismatch (#151) - #163
Merged
Conversation
Second endpoint covered from #151's C7 row. /v1/responses is OpenAI's newer endpoint (introduced 2024) and is the recommended path for new integrations — rapidly displacing /v1/chat/completions in new code. Prior to this file the gateway had **zero** e2e coverage on /v1/responses. Two user journeys pinned, both derived from the gateway's own published contract in `docs/api-proxy.md` §4.6: > Native OpenAI Responses API. OpenAI Models only — non-OpenAI > providers return 400. Case 1 — happy path on OpenAI provider: Caller POSTs /v1/responses with the OpenAI Responses-shape body (`{model, input}`). Gateway dispatches to upstream's /v1/responses (NOT /v1/chat/completions — a regression that mis-routed via chat would surface as `object: "chat.completion"` on the response). Caller receives the upstream's Responses-shape body byte-for-byte: - `object === "response"` (distinct envelope from chat) - `output[0].type === "message"`, `output[0].role === "assistant"` - `output[0].content[0].type === "output_text"` per spec - Reply text byte-for-byte - Usage uses `input_tokens` / `output_tokens` / `total_tokens` (different field names from chat's `prompt_tokens` / `completion_tokens` — a regression that translated through chat-completions field names would mismatch) Upstream-side: gateway hit `/v1/responses` exactly once with `Bearer sk-mock`, body has display name → upstream model_name translation, caller's input reaches upstream verbatim. Case 2 — provider mismatch (anthropic Model on /v1/responses): Per docs §4.6, non-OpenAI providers must return 400. Caller sees 400 with OpenAI-shape error envelope (`error.type` / `error.message` non-empty); upstream MUST NOT be hit (the whole point of the restriction is that OpenAI-Responses-shape doesn't translate to Anthropic Messages today). References: - OpenAI Responses API spec <https://platform.openai.com/docs/api-reference/responses> - Gateway's /v1/responses contract: `docs/api-proxy.md` §4.6 - OpenAI error envelope spec <https://platform.openai.com/docs/guides/error-codes/api-errors> Note on SDK: the OpenAI Node SDK 4.65 (the version pinned by this project) doesn't yet expose `client.responses.create()` — `client.responses` was added in a later SDK release. The test uses raw `fetch` to hit `/v1/responses` directly. This still exercises the real gateway end-to-end; the SDK-call layer is just a thin convenience wrapper for the same wire shape. Refs #151
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Free Run ID: 📒 Files selected for processing (1)
Note 🎁 Summarized by CodeRabbit FreeYour organization has reached its limit of developer seats under the Pro Plan. For new users, CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please add seats to your subscription by visiting https://app.coderabbit.ai/login.If you believe this is a mistake and have available seats, please assign one to the pull request author through the subscription management page using the link above. Comment |
There was a problem hiding this comment.
Pull request overview
Adds first end-to-end coverage for the gateway’s POST /v1/responses proxy surface, validating both correct dispatch for OpenAI-backed models and correct early rejection for non-OpenAI providers per docs/api-proxy.md §4.6.
Changes:
- Introduces an e2e test that pins
/v1/responsesdispatch to the upstream/v1/responsespath and validates key OpenAI Responses envelope/usage fields. - Introduces an e2e test that pins the “OpenAI-only” restriction by asserting a
400OpenAI-shaped error envelope and that the upstream is not contacted for an Anthropic model.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+248
to
+252
| // Readiness gate: poll until the gateway returns the | ||
| // documented 400, not a model-not-found 400 from snapshot lag. | ||
| // Disambiguate by checking the error envelope is fully formed | ||
| // (a snapshot-lag 400 might have a different message style). | ||
| await waitConfigPropagation(async () => { |
Comment on lines
+76
to
+80
| test("OpenAI provider: caller receives upstream Responses body byte-for-byte", async (ctx) => { | ||
| if (!etcdReachable || !app || !admin) { | ||
| ctx.skip(); | ||
| return; | ||
| } |
Audit (per CLAUDE.md §8) found one HIGH, two MEDIUM, three LOW. Resolutions: HIGH H1 (error.type left loose despite a documented value in the gateway's own contract): Pinned `error.type === "invalid_request_error"` per docs §2 status→type table (400 → invalid_request_error). The published table makes this unambiguous; loose "non-empty string" let regressions to "service_unavailable", "provider_error", or any new vocabulary slip through. Same pinning convention body-edges-e2e and error-envelope-normalization-e2e use. MEDIUM M1 (misleading probe comment): Probe comment claimed "model not found 400" — but per docs §2, model_not_found is mapped to 404, NOT 400. Rewrote comment to reflect the actual disambiguation (404 = snapshot lag, 400 + invalid_request_error = §4.6 OpenAI-only refusal). MEDIUM M2 (provider matrix coverage gap): Promoted the non-OpenAI provider case from a single anthropic test to a parametrized matrix covering all three non-OpenAI providers per docs §6 (anthropic, gemini, deepseek). Critical for gemini and deepseek specifically: their bridges DO speak OpenAI wire shape upstream, so a regression that "just dispatched anyway" instead of refusing per §4.6 would silently 200 from the upstream-compat layer, billing the caller and breaking the published contract. LOW L1 (id round-trip not pinned): Pinned `body.id === "resp_e2e_01"` byte-for-byte. A regression that re-issued ids during gateway-side normalization would break SDK paginators and webhook callbacks that key off response id. LOW L2-L3 (created_at, streaming/tools/multi-turn coverage): Not addressed (created_at is a minor add; streaming/tools/ multi-turn deferred to follow-up rows). All 32 e2e tests still pass locally (was 30; this PR now ships 4 cases — 1 happy path + 3 non-OpenAI mismatch).
Uh oh!
There was an error while loading. Please reload this page.
This was referenced May 9, 2026
moonming added a commit
that referenced
this pull request
May 9, 2026
Closes#157 (test-infra concern, not a product bug). Issue: the e2e suite's `guardrail-keyword-e2e.test.ts` has flaked three times on CI in the past 24 hours (#157 first occurrence, plus reruns required on PR #165 and #167). Failure mode is the same: `waitConfigPropagation: condition not met within 5s`. Root cause: vitest is configured with `maxForks: 4` (per `vitest.config.ts`), so up to 4 test files run in parallel, each spawning its own `aisix` binary against a SHARED etcd. Each binary opens watches and writes resources via the admin API concurrently with the others. Under that load, etcd watch dispatch latency for the LAST resource in a multi-resource batch (e.g. a Guardrail rule following Model + ApiKey + ProviderKey writes) can exceed the 5s budget. The 5s budget was sized when the suite had ~9 files. The suite is now 20+ files (#161, #163, #165, #167 added embeddings, responses, passthrough, rerank, images). The growth in parallelism load wasn't matched by a budget bump. Fix: raise the deadline to 10s. This: - Eliminates the recurring rerun churn on feature PRs - Preserves the "fail loudly on a genuinely stuck snapshot" property — 10s is still a generous floor; a real bug where propagation hangs indefinitely would still fail clearly - Doesn't change the happy-path latency at all (the helper polls every 50ms and returns as soon as the condition is met — bumping the deadline only affects the sad path) This is a test-infra-only change; no product behavior is affected. The ≤500ms spec target for in-process propagation is unchanged; this is purely the CI test harness's wait budget for slow runners under concurrent load. If 10s proves insufficient as the suite grows further, the next escalation would be to reduce `maxForks` from 4 to 2 (slower wall time, less etcd pressure) — tracked in #157 as a fallback.
moonming added a commit
that referenced
this pull request
May 9, 2026
… (#169) * test(harness): raise waitConfigPropagation budget 5s → 10s (#157) Closes#157 (test-infra concern, not a product bug). Issue: the e2e suite's `guardrail-keyword-e2e.test.ts` has flaked three times on CI in the past 24 hours (#157 first occurrence, plus reruns required on PR #165 and #167). Failure mode is the same: `waitConfigPropagation: condition not met within 5s`. Root cause: vitest is configured with `maxForks: 4` (per `vitest.config.ts`), so up to 4 test files run in parallel, each spawning its own `aisix` binary against a SHARED etcd. Each binary opens watches and writes resources via the admin API concurrently with the others. Under that load, etcd watch dispatch latency for the LAST resource in a multi-resource batch (e.g. a Guardrail rule following Model + ApiKey + ProviderKey writes) can exceed the 5s budget. The 5s budget was sized when the suite had ~9 files. The suite is now 20+ files (#161, #163, #165, #167 added embeddings, responses, passthrough, rerank, images). The growth in parallelism load wasn't matched by a budget bump. Fix: raise the deadline to 10s. This: - Eliminates the recurring rerun churn on feature PRs - Preserves the "fail loudly on a genuinely stuck snapshot" property — 10s is still a generous floor; a real bug where propagation hangs indefinitely would still fail clearly - Doesn't change the happy-path latency at all (the helper polls every 50ms and returns as soon as the condition is met — bumping the deadline only affects the sad path) This is a test-infra-only change; no product behavior is affected. The ≤500ms spec target for in-process propagation is unchanged; this is purely the CI test harness's wait budget for slow runners under concurrent load. If 10s proves insufficient as the suite grows further, the next escalation would be to reduce `maxForks` from 4 to 2 (slower wall time, less etcd pressure) — tracked in #157 as a fallback. * test(harness): also reduce maxForks 4→2 (#157 fallback escalation) After the timeout bump landed, #169's own CI run still flaked with the same `condition not met within 10s` error — meaning etcd watch dispatch latency genuinely exceeds 10s under maxForks=4 with the current suite size (20+ files). The audit on #169 had flagged this as the "product-side hypothesis still open" — confirmed. Apply the documented fallback from #157: cut maxForks from 4 to 2. This halves the concurrent-watcher count against the shared etcd and brings dispatch latency back inside the budget. Trade-off: wall time grows ~1.5-2× per CI run (locally measured ~14s @ maxForks=4 vs ~30s @ maxForks=2, so faster than expected because the extra parallelism wasn't fully utilized anyway — contention dominated). Net for CI is "predictable green" vs "fast but constantly-rerunning". The 10s `waitConfigPropagation` budget from the prior commit stays in place as belt-and-suspenders — even with maxForks=2 the original 5s would still be tight on slow runners. Combined this PR now does: 1. waitConfigPropagation deadline 5s → 10s (`harness/admin.ts`) 2. maxForks 4 → 2 (`vitest.config.ts`) The product-side hypothesis ("etcd watch dispatch degrades non-linearly with concurrent watchers") is now strongly supported and worth a separate product-side investigation tracked in #157 follow-up.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Second endpoint covered from #151's C7 row. `/v1/responses` is OpenAI's newer endpoint (introduced 2024) and the recommended path for new integrations — rapidly displacing `/v1/chat/completions`. Prior to this PR the gateway had zero e2e coverage on `/v1/responses`.
All 4 cases derived directly from the gateway's published contract in `docs/api-proxy.md` §4.6 + §2 status→type table.
What's pinned
The non-OpenAI matrix is parametrized across all three non-OpenAI providers per docs §6 (anthropic, gemini, deepseek). Critical for gemini and deepseek specifically: their bridges do speak OpenAI wire shape upstream, so a regression that "just dispatched anyway" instead of refusing per §4.6 would silently 200 from the upstream-compat layer, billing the caller and breaking the published contract.
Why these matter
Three regression modes that were unverified before:
Source-blind discipline
Every assertion derives from external contracts:
No internal Rust paths or struct field names referenced.
Note on SDK
The OpenAI Node SDK 4.65 (this project's pinned version) doesn't yet expose `client.responses.create()` — that method was added in a later SDK release. The test uses raw `fetch` to hit `/v1/responses` directly. This still exercises the real gateway end-to-end; the SDK call layer is a thin convenience wrapper for the same wire shape.
Independent audit
Per CLAUDE.md §8, an independent audit agent reviewed the initial commit (6255e19). Resolution log:
Test plan
Refs #151.