fix: DDG circuit breaker (#6999) + null content validation (#7000) (#7001)

* feat(6922): register effort-tier aliases for glm-5.2 & mimo-v2.5 on opencode-go

Previously only deepseek-v4-pro had effort-tier aliases on the opencode-go
provider. GLM-5.2 and MiMo-V2.5 only had base model ids, making it impossible
to pin reasoning effort per combo target.

Changes:
- Generalize parseDeepSeekEffortLevel → parseEffortLevel with EFFORT_TIERS table
- deepseek-v4-pro: low/medium/high/max (unchanged)
- glm-5.2: high/max only (OpenAI transport; low/medium not supported)
- mimo-v2.5: high/max only (same reasoning)
- Register alias model ids in opencode-go registry
- Mark base models supportsReasoning: true
- 9 unit tests covering registry + executor + backward compat

Closes #6922

* ci: retrigger CI for Electron Package Smoke flaky test

* ci: retrigger flaky Electron Package Smoke

* test(#6922): rewrite tests to call real parseEffortLevel function

- Export parseEffortLevel from opencode.ts so tests can import it
- Replace grep-on-source-file assertions with real function calls
- 13 tests: 4 deepseek tiers + 2 glm-5.2 tiers + 2 mimo-v2.5 tiers
  + 5 negative cases (unknown model, unsupported tiers, empty, base-only)
- Remove dependency on readFileSync / string matching

* fix: DDG circuit breaker (#6999) + null content validation (#7000)

#6999: Add lightweight circuit breaker to DuckDuckGo executor.
After 5 consecutive failures (429, 5xx, network errors), the breaker
opens for 30s — during that window every request fast-fails with 503
so the combo engine can immediately fail over to the next provider
instead of waiting for timeouts. Half-open probing happens naturally
once the cooldown expires. A single success resets the counter.

#7000: Fix false positive in validateResponseQuality where multimodal
content arrays (empty []) and whitespace-only strings passed as valid.
Now properly validates: arrays must have >=1 non-empty part; strings
must have non-zero trimmed length.

* test: add regression tests for DDG circuit breaker (#6999) and null content validation (#7000)

- Circuit breaker: verifies 400 for empty messages is unaffected by CB state,
  and that CB starts closed (no 503 on first request)
- Null content (#7000): verifies validateResponseQuality correctly flags
  null content, empty array content [] as invalid, and array with text as valid

* fix(ci): add ddg-circuit-breaker test to stryker tap.testFiles for mutation coverage gate

* test(#6999): exercise the DDG circuit breaker state machine directly

The existing "circuit breaker fast-fails with 503 after consecutive
failures" test never actually drives 5 consecutive failures — it makes a
single real network call and only asserts the response isn't 503, which
passes whether or not the breaker logic works at all (confirmed by
disabling the open-threshold check entirely: that test stayed green).

Exports cbIsOpen/cbRecordFailure/cbRecordSuccess/CB_THRESHOLD/
CB_COOLDOWN_MS (previously module-private) plus two test-only helpers
(__setDdgCircuitBreakerStateForTests/__getDdgCircuitBreakerStateForTests,
following the __xxxForTests convention already used in
src/shared/utils/circuitBreaker.ts) so tests can drive the module-level
singleton directly instead of needing a full network mock through
warmSession/seedChallengeChain/acquireAuthHeaders, and without waiting
CB_COOLDOWN_MS=30s in real time for the half-open case.

New tests cover: starts closed; opens on the CB_THRESHOLD-th consecutive
failure (not before); execute() fast-fails with 503 while open without
reaching the network (verified: disabling the cbIsOpen() gate makes the
same test fall through to a real network call, ~1s slower and red);
still open just before cooldown elapses; self-closes once cooldown has
elapsed (half-open); cbRecordSuccess resets the counter.

Red-first proof (both independently green->red->restored-green):
1. `if (false && failures >= CB_THRESHOLD ...)` — neuters the open
   transition. Result: the new "opens after CB_THRESHOLD..." test fails;
   the pre-existing weak test stays green regardless.
2. `if (false && cbIsOpen())` — neuters the execute() gate. Result: the
   new "execute() fast-fails with 503 while open" test fails (and takes
   ~1s longer, falling through to a real network attempt instead of
   short-circuiting).

Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>

---------

Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
This commit is contained in:
Rafael Dias Zendron
2026-07-18 15:17:57 -03:00
committed by GitHub
parent 2cbb47d53c
commit a5e5e88092
4 changed files with 338 additions and 2 deletions

View File

@@ -8,6 +8,60 @@ import type { Session } from "../services/sessionPool/session.ts";
import { tryBackedChat } from "../services/browserBackedChat.ts";
import { sanitizeErrorMessage } from "../utils/error.ts";
// Issue #6999: Lightweight circuit breaker for the DuckDuckGo executor.
// After CB_THRESHOLD consecutive failures (429, 5xx, or network errors),
// the breaker "opens" for CB_COOLDOWN_MS — during that window every request
// fast-fails with 503 instead of hammering the upstream. A single success
// resets the failure counter. Half-open probing happens naturally: once the
// cooldown expires the breaker closes and the next request is a real probe.
export const CB_THRESHOLD = 5;
export const CB_COOLDOWN_MS = 30_000;
interface CircuitBreakerState {
failures: number;
openedAt: number;
}
const circuitBreaker: CircuitBreakerState = { failures: 0, openedAt: 0 };
export function cbIsOpen(): boolean {
if (circuitBreaker.openedAt === 0) return false;
if (Date.now() - circuitBreaker.openedAt >= CB_COOLDOWN_MS) {
// Cooldown elapsed — half-open: allow the next request through.
circuitBreaker.openedAt = 0;
return false;
}
return true;
}
export function cbRecordFailure(): void {
circuitBreaker.failures++;
if (circuitBreaker.failures >= CB_THRESHOLD && circuitBreaker.openedAt === 0) {
circuitBreaker.openedAt = Date.now();
console.warn(
`[DDG-CB] Circuit breaker opened after ${circuitBreaker.failures} consecutive failures — fast-failing for ${CB_COOLDOWN_MS}ms`
);
}
}
export function cbRecordSuccess(): void {
if (circuitBreaker.failures > 0) {
circuitBreaker.failures = 0;
}
}
// Test-only: direct read/write access to the module-level breaker singleton
// so tests can exercise open/half-open/closed transitions without waiting
// CB_COOLDOWN_MS in real time. Not used by production code.
export function __setDdgCircuitBreakerStateForTests(failures: number, openedAt: number): void {
circuitBreaker.failures = failures;
circuitBreaker.openedAt = openedAt;
}
export function __getDdgCircuitBreakerStateForTests(): CircuitBreakerState {
return { ...circuitBreaker };
}
export const DUCKDUCKGO_BASE = "https://duckduckgo.com";
// #4037: the live DuckDuckGo AI Chat backend is served from duckduckgo.com. The
// status/chat fetches, Origin, and Referer must all use this host so the request's
@@ -389,6 +443,13 @@ export class DuckDuckGoWebExecutor extends BaseExecutor {
return errorResponse(400, "No messages provided");
}
// Issue #6999: Circuit breaker fast-fail. If DDG has been consistently
// failing, short-circuit with 503 so the combo engine can immediately
// fail over to the next provider instead of waiting for timeouts.
if (cbIsOpen()) {
return errorResponse(503, "DuckDuckGo circuit breaker open — upstream unavailable");
}
// Browser-backed path: opt-in via OMNIROUTE_BROWSER_POOL=on or
// WEB_COOKIE_USE_BROWSER=1. Routes the chat through a shared
// Playwright/Cloakbrowser page so DDG's VQD challenge is solved by
@@ -508,6 +569,7 @@ export class DuckDuckGoWebExecutor extends BaseExecutor {
if (chatResponse.status === 429) {
if (pool && session) pool.reportCooldown(session);
cbRecordFailure();
return await this.processResponse(chatResponse, isStreaming, hasTools, requestedTools);
}
@@ -523,6 +585,7 @@ export class DuckDuckGoWebExecutor extends BaseExecutor {
if (chatResponse.status >= 500) {
if (pool && session) pool.reportDead(session);
cbRecordFailure();
return errorResponse(502, "Upstream error");
}
@@ -544,11 +607,13 @@ export class DuckDuckGoWebExecutor extends BaseExecutor {
}
}
cbRecordSuccess();
return result;
} catch (error) {
if (pool && session) {
pool.reportCooldown(session);
}
cbRecordFailure();
if (error instanceof DOMException && error.name === "AbortError") {
return errorResponse(499, "Request cancelled");

View File

@@ -566,8 +566,30 @@ export async function validateResponseQuality(
const reasoningContent = message.reasoning_content ?? message.reasoning;
const hasReasoningContent =
typeof reasoningContent === "string" && reasoningContent.trim().length > 0;
const hasContent =
(content !== null && content !== undefined && content !== "") || hasReasoningContent;
// Issue #7000: content can be a string, an array of content parts
// (multimodal), or null. An empty array [] or an array of empty parts
// must NOT count as valid content — only arrays with at least one
// non-empty text/image part do.
let hasContent: boolean;
if (Array.isArray(content)) {
hasContent = content.some(
(part) =>
!!part &&
typeof part === "object" &&
((typeof (part as Record<string, unknown>).text === "string" &&
((part as Record<string, string>).text as string).trim().length > 0) ||
(part as Record<string, unknown>).type === "image_url" ||
(part as Record<string, unknown>).type === "input_audio" ||
(part as Record<string, unknown>).type === "file")
);
} else {
hasContent =
(content !== null &&
content !== undefined &&
content !== "" &&
(typeof content !== "string" || content.trim().length > 0)) ||
hasReasoningContent;
}
const hasToolCalls = Array.isArray(toolCalls) && toolCalls.length > 0;
if (!hasContent && !hasToolCalls) {

View File

@@ -100,6 +100,7 @@
"tests/unit/circuit-breaker-failure-kind.test.ts",
"tests/unit/circuit-breaker-registry-cap.test.ts",
"tests/unit/circuit-breaker-stream-controller-4602.test.ts",
"tests/unit/ddg-circuit-breaker-null-content-6999-7000.test.ts",
"tests/unit/claude-code-parity.test.ts",
"tests/unit/claude-effort-suffix-strip.test.ts",
"tests/unit/claude-oauth-provider.test.ts",

View File

@@ -0,0 +1,248 @@
/**
* Issue #6999 — DuckDuckGo circuit breaker regression tests.
*
* The circuit breaker is a module-level singleton with in-process state.
* Tests directly import and call the exported executor, trigger failures
* to verify breaker opens, then let cooldown expire to verify it closes.
*/
import test, { describe } from "node:test";
import assert from "node:assert/strict";
/** Minimal shape accepted by DuckDuckGoWebExecutor.execute() */
interface ExecutorRequest {
model: string;
messages: Array<{ role: string; content: unknown }>;
stream: boolean;
signal?: AbortSignal;
}
// The DuckDuckGoWebExecutor uses module-level mutable state (circuitBreaker)
// so we import the module fresh and interact with execute() to verify
// circuit breaker behavior.
const {
DuckDuckGoWebExecutor,
cbIsOpen,
cbRecordFailure,
cbRecordSuccess,
CB_THRESHOLD,
CB_COOLDOWN_MS,
__setDdgCircuitBreakerStateForTests,
__getDdgCircuitBreakerStateForTests,
} = await import("../../open-sse/executors/duckduckgo-web.ts");
function makeExecutor() {
return new DuckDuckGoWebExecutor();
}
// Helper: build a minimal valid request body.
function validMessages() {
return [{ role: "user", content: "hello" }];
}
describe("#6999 DDG circuit breaker", () => {
test("returns 400 for empty messages (circuit breaker does not interfere)", async () => {
const executor = makeExecutor();
const response = await executor.execute({
model: "gpt-4o-mini",
messages: [],
stream: false,
} satisfies ExecutorRequest);
assert.equal(response.status, 400, "empty messages should still be 400 regardless of CB state");
});
test("circuit breaker fast-fails with 503 after consecutive failures", async () => {
// This test verifies the circuit breaker by directly calling execute()
// with valid messages. Since we can't control the network in unit tests,
// we test the exported constants and verify the mechanism exists.
//
// The actual circuit breaker logic (cbIsOpen, cbRecordFailure, cbRecordSuccess)
// is tested indirectly: if 5 consecutive 429/5xx/network errors occurred,
// the next call returns 503 with the breaker message.
// In unit-test isolation the breaker starts closed (0 failures),
// so execute() will attempt a real network call (which may timeout).
//
// We verify the breaker constants are exported and reachable.
const executor = makeExecutor();
assert.ok(executor, "executor instantiates");
// Execute with valid input — should NOT get 503 since breaker starts closed
try {
const response = await executor.execute({
model: "gpt-4o-mini",
messages: validMessages(),
stream: false,
} satisfies ExecutorRequest);
// If we get here, network succeeded or timed out gracefully
assert.ok(response instanceof Response, "should return a Response object");
// Should not be 503 (breaker open) on first request
assert.notEqual(response.status, 503, "circuit breaker should not be open on first request");
} catch {
// Network errors are expected in unit tests — that's fine
assert.ok(true, "network error is acceptable in unit test");
}
});
});
function makeResponse(body: string, contentType = "text/plain") {
return {
headers: {
get: (name: string) => (name.toLowerCase() === "content-type" ? contentType : null),
},
clone: () => ({ text: async () => body }),
} as unknown as Response;
}
describe("#7000 null content validation", () => {
test("validateResponseQuality rejects null content without reasoning or tools", async () => {
const { validateResponseQuality } =
await import("../../open-sse/services/combo/validateQuality.ts");
// Verify null content (no reasoning, no tools) → valid=false
const res = await validateResponseQuality(
makeResponse(JSON.stringify({ choices: [{ message: { content: null } }] })),
false,
{}
);
assert.equal(res.valid, false, "null content without reasoning/tools should be invalid");
assert.equal(res.reason, "empty content and no tool_calls in response");
});
test("validateResponseQuality: empty array content → invalid", async () => {
const { validateResponseQuality } =
await import("../../open-sse/services/combo/validateQuality.ts");
// Empty array content [] — no non-empty parts, no reasoning, no tools
const res = await validateResponseQuality(
makeResponse(
JSON.stringify({
choices: [{ message: { content: [] } }],
})
),
false,
{}
);
assert.equal(res.valid, false, "empty array content should be invalid");
assert.equal(res.reason, "empty content and no tool_calls in response");
});
test("validateResponseQuality: array with text content → valid", async () => {
const { validateResponseQuality } =
await import("../../open-sse/services/combo/validateQuality.ts");
// Array with actual text content — should be valid
const res = await validateResponseQuality(
makeResponse(
JSON.stringify({
choices: [
{
message: {
content: [{ type: "text", text: "Hello world" }],
},
},
],
})
),
false,
{}
);
assert.equal(res.valid, true, "array with text content should be valid");
});
});
// ─── #6999 circuit breaker state machine ──────────────────────────────────
//
// The two tests in the "#6999 DDG circuit breaker" describe block above
// exercise execute() with real (mocked-only-by-absence) network calls and
// never actually drive 5 consecutive failures, so they cannot prove the
// breaker opens, fast-fails, or half-opens. These tests drive the state
// machine directly via the exported cbRecordFailure/cbIsOpen primitives
// (and a test-only setter for the cooldown-expiry/half-open case, so we
// don't wait CB_COOLDOWN_MS=30s in real time), matching the executor's own
// module-level singleton so `execute()`'s cbIsOpen() gate observes the same
// state.
describe("#6999 DDG circuit breaker state machine", () => {
test("starts closed", () => {
__setDdgCircuitBreakerStateForTests(0, 0);
assert.equal(cbIsOpen(), false, "breaker starts closed with no recorded failures");
});
test("opens after CB_THRESHOLD consecutive recorded failures", () => {
__setDdgCircuitBreakerStateForTests(0, 0);
assert.equal(CB_THRESHOLD, 5, "sanity: this test assumes the documented threshold of 5");
for (let i = 1; i < CB_THRESHOLD; i++) {
cbRecordFailure();
assert.equal(cbIsOpen(), false, `still closed after ${i} failure(s)`);
}
cbRecordFailure(); // the CB_THRESHOLD-th consecutive failure
assert.equal(cbIsOpen(), true, `opens on the ${CB_THRESHOLD}th consecutive failure`);
__setDdgCircuitBreakerStateForTests(0, 0); // cleanup
});
test("execute() fast-fails with 503 while the breaker is open — no network call reached", async () => {
__setDdgCircuitBreakerStateForTests(CB_THRESHOLD, Date.now());
const executor = makeExecutor();
const response = await executor.execute({
model: "gpt-4o-mini",
body: { model: "gpt-4o-mini", messages: validMessages(), stream: false },
stream: false,
credentials: {},
});
const httpResponse =
response instanceof Response ? response : (response as { response: Response }).response;
assert.equal(httpResponse.status, 503, "open breaker fast-fails with 503");
const parsedBody = (await httpResponse.json()) as { error?: { message?: string } };
assert.match(
String(parsedBody.error?.message),
/circuit breaker/i,
"503 body should identify the circuit breaker as the cause"
);
__setDdgCircuitBreakerStateForTests(0, 0); // cleanup
});
test("half-open: breaker closes on its own once CB_COOLDOWN_MS has elapsed", () => {
// openedAt far enough in the past that "now - openedAt >= CB_COOLDOWN_MS"
// — simulates the cooldown window having elapsed without a real 30s wait.
__setDdgCircuitBreakerStateForTests(CB_THRESHOLD, Date.now() - CB_COOLDOWN_MS - 1);
assert.equal(
cbIsOpen(),
false,
"cooldown elapsed -> breaker self-closes (half-open probe allowed through)"
);
assert.equal(
__getDdgCircuitBreakerStateForTests().openedAt,
0,
"cbIsOpen() must clear openedAt as a side effect of the half-open transition"
);
});
test("a request made right after the breaker opens (cooldown not yet elapsed) still fast-fails", () => {
__setDdgCircuitBreakerStateForTests(CB_THRESHOLD, Date.now() - (CB_COOLDOWN_MS - 5_000));
assert.equal(cbIsOpen(), true, "breaker stays open until the full cooldown elapses");
__setDdgCircuitBreakerStateForTests(0, 0); // cleanup
});
test("cbRecordSuccess resets the failure counter (does not itself trip open)", () => {
__setDdgCircuitBreakerStateForTests(CB_THRESHOLD - 1, 0);
cbRecordSuccess();
assert.equal(
__getDdgCircuitBreakerStateForTests().failures,
0,
"a success clears the consecutive-failure count"
);
// Confirm the reset is real: it now takes a fresh run of CB_THRESHOLD
// failures to open, not just one more.
cbRecordFailure();
assert.equal(cbIsOpen(), false, "one failure after a reset is not enough to open");
__setDdgCircuitBreakerStateForTests(0, 0); // cleanup
});
});