Skip to content
Closed
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
22 changes: 22 additions & 0 deletions .github/workflows/sync-fork.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
name: Sync Fork

on:
schedule:
- cron: "0 */6 * * *"
workflow_dispatch:

jobs:
sync:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
with:
ref: main
fetch-depth: 0

- name: Sync upstream
run: |
git remote add upstream https://github.com/diegosouzapw/OmniRoute.git
git fetch upstream main
git merge upstream/main --no-edit
git push origin main
19 changes: 16 additions & 3 deletions open-sse/services/combo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,11 @@ const COMBO_BAD_REQUEST_FALLBACK_PATTERNS = [
// Used to detect 503 responses from handleNoCredentials so combo can fallback.
const ALL_ACCOUNTS_RATE_LIMITED_PATTERNS = [/unavailable/i, /service temporarily unavailable/i];

function isAllAccountsRateLimitedResponse(status: number, contentType: string | null, errorText: string): boolean {
function isAllAccountsRateLimitedResponse(
status: number,
contentType: string | null,
errorText: string
): boolean {
if (status !== 503) return false;
if (!contentType?.includes("application/json")) return false;
return ALL_ACCOUNTS_RATE_LIMITED_PATTERNS.some((p) => p.test(errorText));
Expand Down Expand Up @@ -168,7 +172,9 @@ async function validateResponseQuality(
try {
json = JSON.parse(text);
} catch {
if (text.startsWith("data:")) return { valid: true };
if (text.startsWith("data:") || text.startsWith("event:") || text.includes("\ndata:")) {
return { valid: true };
}
return { valid: false, reason: "response is not valid JSON" };
}

Expand Down Expand Up @@ -1522,6 +1528,8 @@ export async function handleComboChat({
target: toRecordedTarget(target),
});
recordedAttempts++;
if (!lastStatus) lastStatus = 422;
lastError = lastError || `quality check failed: ${quality.reason}`;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

To maintain consistency with the error handling logic in the !result.ok branch (see line 1705), lastError should be updated with the latest failure reason rather than preserving the first one. The current use of the logical OR operator (||) deviates from the established pattern where lastStatus captures the first status code while lastError captures the most recent error message.

Suggested change
lastError = lastError || `quality check failed: ${quality.reason}`;
lastError = `quality check failed: ${quality.reason}`;

if (i > 0) fallbackCount++;
break; // move to next model
}
Expand Down Expand Up @@ -1900,6 +1908,8 @@ async function handleRoundRobinCombo({
target: toRecordedTarget(target),
});
recordedAttempts++;
if (!lastStatus) lastStatus = 422;
lastError = lastError || `quality check failed: ${quality.reason}`;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

To maintain consistency with the error handling logic in the !result.ok branch (see line 2053), lastError should be updated with the latest failure reason rather than preserving the first one. The current use of the logical OR operator (||) deviates from the established pattern where lastStatus captures the first status code while lastError captures the most recent error message.

Suggested change
lastError = lastError || `quality check failed: ${quality.reason}`;
lastError = `quality check failed: ${quality.reason}`;

if (offset > 0) fallbackCount++;
break; // move to next model
}
Expand Down Expand Up @@ -2001,7 +2011,10 @@ async function handleRoundRobinCombo({
}

if (isAllAccountsRateLimited) {
log.info("COMBO", `All accounts rate-limited for ${modelStr}, falling back to next model`);
log.info(
"COMBO",
`All accounts rate-limited for ${modelStr}, falling back to next model`
);
} else if (!shouldFallback && !comboBadRequestFallback) {
log.warn("COMBO-RR", `${modelStr} failed (no fallback)`, { status: result.status });
recordComboRequest(combo.name, modelStr, {
Expand Down
94 changes: 94 additions & 0 deletions tests/unit/combo-routing-engine.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1927,3 +1927,97 @@ test("handleComboChat aborts combo when 503 response does NOT contain the unavai
result.status === 503
);
});

test("handleComboChat returns 422 (not ALL_ACCOUNTS_INACTIVE) when all models fail quality check", async () => {
const calls = [];

const result = await handleComboChat({
body: {},
combo: {
name: "quality-check-laststatus",
strategy: "priority",
models: ["model-a", "model-b"],
config: { maxRetries: 0 },
},
handleSingleModel: async (_body: any, modelStr: any) => {
calls.push(modelStr);
// Return 200 with invalid JSON — passes the ok check but fails validateResponseQuality
return new Response("{not valid json", {
status: 200,
headers: { "content-type": "text/plain" },
});
},
isModelAvailable: async () => true,
log: createLog(),
settings: null,
allCombos: null,
relayOptions: null,
});

const payload = await result.json();
// Before the fix: lastStatus was never set on quality-check failure path,
// so the code fell through to the `!lastStatus` branch returning ALL_ACCOUNTS_INACTIVE.
// After the fix: lastStatus is set to 422 and lastError is populated.
assert.equal(result.status, 422);
assert.ok(payload.error?.message?.includes("quality check failed"));
assert.deepEqual(calls, ["model-a", "model-b"]);
});

test("handleComboChat accepts SSE responses starting with event: line as valid", async () => {
const result = await handleComboChat({
body: {},
combo: {
name: "quality-sse-event-prefix",
strategy: "priority",
models: ["model-a"],
config: { maxRetries: 0 },
},
handleSingleModel: async () =>
new Response('event: message_start\ndata: {"type":"message_start"}\n\n', {
status: 200,
headers: { "content-type": "text/event-stream" },
}),
isModelAvailable: async () => true,
log: createLog(),
settings: null,
allCombos: null,
relayOptions: null,
});

// Before the fix: validateResponseQuality only checked text.startsWith("data:"),
// so SSE starting with "event:" was rejected as "response is not valid JSON".
// After the fix: text.startsWith("event:") and text.includes("\ndata:") also pass.
assert.equal(result.ok, true);
});

test("handleComboChat accepts SSE responses with event and data on separate lines", async () => {
const calls = [];
const result = await handleComboChat({
body: {},
combo: {
name: "quality-sse-newline-data",
strategy: "priority",
models: ["model-a", "model-b"],
config: { maxRetries: 0 },
},
handleSingleModel: async (_body: any, modelStr: any) => {
calls.push(modelStr);
if (modelStr === "model-a") {
// SSE with event: prefix before data: — Anthropic-style
return new Response(
'event: message_start\ndata: {"type":"message_start","message":{"id":"msg_1"}}\n\nevent: content_block_start\ndata: {"type":"content_block_start"}\n\n',
{ status: 200, headers: { "content-type": "text/event-stream" } }
);
}
return okResponse();
},
isModelAvailable: async () => true,
log: createLog(),
settings: null,
allCombos: null,
relayOptions: null,
});

assert.equal(result.ok, true);
assert.deepEqual(calls, ["model-a"]);
});