Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions changelog.d/fixes/18134-local-backends-shutdown-drain.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
- **fix(sse):** the Codex app-server, the ChatGPT Web (Codex) runtime and the local Devin CLI bridge answer `499` (`client_disconnected`) instead of `502` while the instance is draining — a graceful restart used to be read as provider ill health, so `markAccountUnavailable` wrote `testStatus "unavailable"` into `provider_connections` (state that outlives the restart) and the combo added a model lockout, a provider circuit-breaker failure and a provider cooldown for a teardown we caused ourselves; the shared `drainAwareFailureStatus()` helper (open-sse/utils/drainStatus.ts) reads the drain flag live, applies to 5xx only and leaves a genuine upstream failure at its own status ([#18134](https://github.com/cheaper-inference/OmniRoute/pull/18134) - thanks @megamen32)
29 changes: 24 additions & 5 deletions open-sse/executors/chatgpt-web-codex.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,12 @@ import {

import { isVerifiedNativeCodexRequest } from "../config/codexIdentity.ts";
import { FORMATS } from "../translator/formats.ts";
import { buildErrorBody, sanitizeErrorMessage } from "../utils/error.ts";
import {
buildErrorBody,
errorResponse as canonicalErrorResponse,
sanitizeErrorMessage,
} from "../utils/error.ts";
import { drainAwareFailureStatus } from "../utils/drainStatus.ts";
import { createChatGptWebAdapter } from "../vendor/codex-chatgpt-web/adapters/chatgpt-web/index.ts";
import { ChatGptBrowserWorker } from "../vendor/codex-chatgpt-web/adapters/chatgpt-web/browser-worker.ts";
import {
Expand Down Expand Up @@ -408,12 +413,14 @@ export class ChatGptWebCodexExecutor extends BaseExecutor {
try {
await adapter.runTurn(parsed, incoming, (event) => events.push(event));
} catch (error) {
const message = sanitizeErrorMessage(error instanceof Error ? error.message : error);
const status = drainAwareFailureStatus(502);
events.push({
type: "error",
message: sanitizeErrorMessage(error instanceof Error ? error.message : error),
status: 502,
errorType: "provider_error",
code: "chatgpt_web_codex_turn_failed",
message,
status,
errorType: status === 499 ? "client_disconnected" : "provider_error",
code: status === 499 ? "client_disconnected" : "chatgpt_web_codex_turn_failed",
});
} finally {
try {
Expand All @@ -440,6 +447,18 @@ export class ChatGptWebCodexExecutor extends BaseExecutor {
const running = run();
const collected = await events.collect();
await running;
const drainFailure = collected.find(
(event) => event.type === "error" && event.status === 499
);
if (drainFailure?.type === "error") {
// Our own shutdown stopped the ChatGPT Web runtime under a request that
// was still in flight, so the turn was never really served. Answer 499
// (client_disconnected) with the canonical body of the repo's
// errorResponse helper, so the runtime does not read our own teardown as
// provider ill health. gracefulShutdown.cleanup() stops this runtime via
// stopChatGptWebCodexRuntime(); see open-sse/utils/drainStatus.ts.
return wrapped(canonicalErrorResponse(499, drainFailure.message), expandedBody);
}
const response = buildResponseJSON(collected, input.model, {
hideThinkingSummary: parsed.options.hideThinkingSummary,
toolNsMap: maps.namespace,
Expand Down
27 changes: 22 additions & 5 deletions open-sse/executors/codex-app-server.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import { bridgeToResponsesSSE, buildResponseJSON } from "../vendor/codex-chatgpt-web/bridge.ts";
import { AsyncEventQueue } from "../vendor/codex-chatgpt-web/event-queue.ts";
import type { AdapterEvent } from "../vendor/codex-chatgpt-web/types.ts";
import { sanitizeErrorMessage } from "../utils/error.ts";
import { errorResponse as canonicalErrorResponse, sanitizeErrorMessage } from "../utils/error.ts";
import { drainAwareFailureStatus } from "../utils/drainStatus.ts";
import { PROVIDERS } from "../config/constants.ts";
import { BaseExecutor, type ExecuteInput, type ExecutorExecuteResult } from "./base.ts";
import { CodexAppServerClient, type CodexAppServerClientOptions } from "./codex/appServerClient.ts";
Expand Down Expand Up @@ -447,12 +448,14 @@ export class CodexAppServerExecutor extends BaseExecutor {
await turnDone;
} catch (err) {
if (!terminated) {
const message = sanitizeErrorMessage(err instanceof Error ? err.message : err);
const status = drainAwareFailureStatus(502);
events.push({
type: "error",
message: sanitizeErrorMessage(err instanceof Error ? err.message : err),
status: 502,
errorType: "provider_error",
code: "codex_app_server_turn_failed",
message,
status,
errorType: status === 499 ? "client_disconnected" : "provider_error",
code: status === 499 ? "client_disconnected" : "codex_app_server_turn_failed",
});
events.close();
markTerminated();
Expand All @@ -466,6 +469,20 @@ export class CodexAppServerExecutor extends BaseExecutor {
const running = run();
const collected = await events.collect();
await running;
const drainFailure = collected.find(
(event) => event.type === "error" && event.status === 499
);
if (drainFailure?.type === "error") {
// Our own shutdown tore the app-server down under a request that was still
// in flight: the turn was never really served by the provider. Answer 499
// (client_disconnected) with the canonical body of the repo's errorResponse
// helper, so the runtime does not read our own teardown as provider ill
// health (see open-sse/utils/drainStatus.ts).
return {
response: canonicalErrorResponse(499, drainFailure.message),
url: config.url,
};
}
const response = buildResponseJSON(collected, input.model, {
toolNsMap: toolMaps.namespace,
freeformToolNames: toolMaps.freeform,
Expand Down
27 changes: 25 additions & 2 deletions open-sse/executors/devin-cli-agentic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ import fs from "node:fs";
import { randomUUID } from "node:crypto";
import { BaseExecutor, type ExecuteInput } from "./base.ts";
import { DEVIN_MODEL_CATALOG } from "../config/providers/registry/devin/catalog.ts";
import { buildErrorBody, sanitizeErrorMessage } from "../utils/error.ts";
import { buildErrorBody, errorResponse, sanitizeErrorMessage } from "../utils/error.ts";
import { drainAwareFailureStatus } from "../utils/drainStatus.ts";
import {
buildClaudeSseFrames,
buildClaudeTextResponse,
Expand Down Expand Up @@ -589,9 +590,31 @@ export class DevinCliAgenticExecutor extends BaseExecutor {
};
} catch (error) {
const bridge = error instanceof DevinAgenticBridgeError ? error : null;
const status = bridge?.status || 500;
if (drainAwareFailureStatus(status) === 499) {
// The Devin CLI is a LOCAL child process: our own shutdown kills it while
// requests are still in flight, so the spawn/ACP failure that follows is
// evidence about OUR shutdown, not about the provider's health. The account
// state the runtime writes from a 5xx (a provider connection marked
// unavailable, a model lockout, a breaker failure, a provider cooldown)
// OUTLIVES the restart, so our own teardown must not be recorded as provider
// ill health. While the instance is draining, answer 499
// (client_disconnected) with the canonical body of the repo's errorResponse
// helper (see open-sse/utils/drainStatus.ts). A 4xx of the request keeps its
// own status, and a genuine failure outside a drain keeps its 5xx.
return {
response: errorResponse(
499,
bridge?.message || (error instanceof Error ? error.message : String(error))
),
url: "devin://acp/stdio",
headers: {},
transformedBody: { model },
};
}
return {
response: new Response(JSON.stringify(errorBody(error)), {
status: bridge?.status || 500,
status,
headers: { "Content-Type": "application/json" },
}),
url: "devin://acp/stdio",
Expand Down
35 changes: 35 additions & 0 deletions open-sse/utils/drainStatus.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
/**
* Drain-aware failure status for executors whose transport we tear down ourselves.
*
* Local backends (the Codex app-server and the Devin CLI child processes, the ChatGPT
* Web runtime that `gracefulShutdown.cleanup()` stops) fail in-flight turns when OUR
* graceful shutdown disposes them. That failure is evidence about our shutdown, not
* about the provider's health, but a 5xx makes the runtime record provider ill health
* that OUTLIVES the restart (a connection marked unavailable, a model lockout, a
* breaker failure, a provider cooldown). While the instance is draining, such a 5xx is
* answered 499 (client_disconnected) instead: 499 is in neither the model-lockout
* error-code list nor the provider-breaker failure statuses (408/500/502/503/504).
*
* The engine must not import from `src/` (boundary ratchet), so the drain flag is read
* straight from the process-wide state `src/lib/gracefulShutdown.ts` owns
* (`globalThis.__omnirouteShutdown.shuttingDown`, the value its `isDraining()` returns).
* It is read live on every call, never latched: a turn that fails before the shutdown
* starts keeps its own status.
*/

type ShutdownStateView = { shuttingDown?: unknown };

/** True while the instance is draining for a graceful shutdown. */
export function isInstanceDraining(): boolean {
const state = (globalThis as { __omnirouteShutdown?: ShutdownStateView }).__omnirouteShutdown;
return state?.shuttingDown === true;
}

/**
* The status a failed local-backend turn is answered with: a 5xx becomes 499 while the
* instance is draining; a 4xx of the request and any failure outside a drain keep
* their own status.
*/
export function drainAwareFailureStatus(status: number): number {
return status >= 500 && isInstanceDraining() ? 499 : status;
}
45 changes: 45 additions & 0 deletions tests/unit/drain-status-helper.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
/**
* open-sse/utils/drainStatus.ts reads the drain flag that src/lib/gracefulShutdown.ts
* owns without importing it (the engine must not gain open-sse -> src edges). These
* tests pin the status rule and keep the two readers of the shared state in sync.
*/
import test from "node:test";
import assert from "node:assert/strict";

const { drainAwareFailureStatus, isInstanceDraining } =
await import("../../open-sse/utils/drainStatus.ts");
const { isDraining } = await import("../../src/lib/gracefulShutdown.ts");

function setDraining(shuttingDown: boolean) {
globalThis.__omnirouteShutdown = { init: false, shuttingDown, activeRequests: 0 };
}

test.afterEach(() => {
delete globalThis.__omnirouteShutdown;
});

test("a 5xx becomes 499 only while draining; a 4xx keeps its status", () => {
setDraining(false);
assert.equal(drainAwareFailureStatus(502), 502);
assert.equal(drainAwareFailureStatus(500), 500);
assert.equal(drainAwareFailureStatus(429), 429);

setDraining(true);
assert.equal(drainAwareFailureStatus(502), 499);
assert.equal(drainAwareFailureStatus(503), 499);
assert.equal(drainAwareFailureStatus(400), 400);
assert.equal(drainAwareFailureStatus(429), 429);
});

test("the engine-side reader agrees with gracefulShutdown.isDraining()", () => {
delete globalThis.__omnirouteShutdown;
assert.equal(isInstanceDraining(), false, "no shutdown state yet means not draining");

setDraining(false);
assert.equal(isInstanceDraining(), isDraining());
assert.equal(isInstanceDraining(), false);

setDraining(true);
assert.equal(isInstanceDraining(), isDraining());
assert.equal(isInstanceDraining(), true);
});
Loading
Loading