Skip to content

test(combos): await the fallback lease release on both passthrough relays #7360

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
lidge-jun merged 1 commit into from
Oct 19, 2026
Merged

test(combos): await the fallback lease release on both passthrough relays #7360

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 25 additions & 7 deletions tests/server/server-combo-zero-output-failover.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import { clearKeyCooldowns } from "../../src/providers/key-failover";
import { clearCodexUpstreamHealth } from "../../src/codex/routing";
import { clearRequestLogsForTests, type RequestLogContext } from "../../src/server/request-log";
import { activeRegistryMetrics, getActiveTurnCount, tryAdmitTurn } from "../../src/server/lifecycle";
import { setRelayPlatformForTests } from "../../src/server/responses/passthrough-delivery";
import {
clearResponseStateForTests,
flushResponseState,
Expand Down Expand Up @@ -262,8 +263,12 @@ describe("combo zero-output bare Responses error failover", () => {
expect(activeRegistryMetrics().activeTurns!.releaseMisses).toBe(misses);
});

// Both passthrough relay shapes, on every OS: "linux" selects tee, "win32" plus an explicit
// eager-relay mode selects the eager bounded relay that Windows uses by default.
for (const relay of ["tee", "eager"] as const)
for (const exit of ["complete", "cancel", "abort"] as const)
test(`accepted fallback keeps its turn lease until stream ${exit}`, async () => {
test(`accepted fallback keeps its turn lease until stream ${exit} (${relay} relay)`, async () => {
setRelayPlatformForTests(relay === "eager" ? "win32" : "linux");
const encoder = new TextEncoder();
const frame = (event: unknown) => encoder.encode(`data: ${JSON.stringify(event)}\n\n`);
const a = serve(() => new Response(frame({ type: "response.failed", response: {
Expand All @@ -277,13 +282,16 @@ describe("combo zero-output bare Responses error failover", () => {
controller.close();
};
} }), { headers: { "content-type": "text/event-stream" } }));
const config = comboConfig({ a: provider("openai-responses", baseUrl(a), "key-a"),
b: provider("openai-responses", baseUrl(b), "key-b") });
const config = { ...comboConfig({ a: provider("openai-responses", baseUrl(a), "key-a"),
b: provider("openai-responses", baseUrl(b), "key-b") }), streamMode: "eager-relay" as const };
const caller = new AbortController();
const lease = tryAdmitTurn(undefined, caller.signal);
if (!lease) throw new Error("test turn admission unavailable");
let releases = 0;
lease.attach({ release() { releases++; } });
// Await the release itself: a relay may legitimately settle it after the client's cancel.
let released!: () => void;
const releasedSignal = new Promise<void>(resolve => { released = resolve; });
lease.attach({ release() { releases++; released(); } });
const misses = activeRegistryMetrics().activeTurns!.releaseMisses;
try {
const response = await handleResponses(new Request("http://localhost/v1/responses", {
Expand All @@ -293,18 +301,28 @@ describe("combo zero-output bare Responses error failover", () => {
expect(response.status).toBe(200);
expect(getActiveTurnCount()).toBe(1);
expect(releases).toBe(0);
if (exit === "cancel") await response.body!.cancel();
else if (exit === "abort") {
if (exit === "cancel") {
await response.body!.cancel();
if (relay === "eager") {
// #44: after a client cancel the eager relay keeps draining upstream, still admitted,
// until a terminal or its bounded drain window. A request abort releases at once; a
// body cancel alone waits for the drain, which an upstream terminal ends here.
expect(getActiveTurnCount()).toBe(1);
expect(releases).toBe(0);
}
try { finish(); } catch { /* the tee relay already cancelled the upstream body */ }

@chatgpt-codex-connector chatgpt-codex-connector Bot Oct 19, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Verify tee cancellation before completing upstream

In the tee/cancel row, calling finish() before checking the release lets the test pass even if cancelling the response no longer releases the lease: the injected terminal will independently finish inspection and unregister the turn. Only the eager row is supposed to need that terminal, so the tee row should assert the release after response.body.cancel() without completing upstream.

Useful? React with 👍 / 👎.

} else if (exit === "abort") {
caller.abort();
await response.text().catch(() => "");
} else {
finish();
expect(await response.text()).toContain("held fallback");
}
await releasedSignal;

@chatgpt-codex-connector chatgpt-codex-connector Bot Oct 19, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Reset the relay override from afterEach

When a relay regression prevents the release callback, this promise remains pending until Bun times out the test; Bun then runs afterEach without resuming this async function, so the lexical finally never clears setRelayPlatformForTests. Because the existing afterEach does not reset that module-global override, all subsequent passthrough tests in the worker can run under the forced platform and produce cascading or misleading failures. Clear the override in afterEach as well.

Useful? React with 👍 / 👎.

expect(getActiveTurnCount()).toBe(0);
expect(releases).toBe(1);
expect(activeRegistryMetrics().activeTurns!.releaseMisses).toBe(misses);
} finally { lease.release(); }
} finally { setRelayPlatformForTests(undefined); lease.release(); }
expect(releases).toBe(1);
});

Expand Down
Loading