feat(providers): configurable timeouts, graceful rate-limit retry, non-answer detection - #167
Conversation
…n-answer detection
Three failure modes showed up in one ask-all fan-out and none of them were
reachable from config.
Timeouts. models.<id>.timeout was the only key that governed an MCP-path call.
The composition root passed no timeout to codex/gemini/grok, so req.timeoutMs
was structurally undefined and each provider sat on its hardcoded default;
providers.<name>.timeout did not exist in the schema, and
providers.openrouter.defaults.timeout was read only by the standalone bridge.
Adds providers.defaults.timeout (one knob for every provider) plus
providers.<name>.timeout, with OpenRouter reusing the defaults block it already
owns instead of a second key. The ladder resolves in resolveProviders, so the
composition root just reads the resolved value:
req.timeoutMs > models.<id>.timeout > providers.<name>.timeout
> providers.defaults.timeout > adapter built-in
Response body was not covered by the timeout. Both HTTP bridges cleared the
abort timer in `finally` before `await res.text()`, leaving the body read
unbounded. That is why deepseek-v4-pro returned at 250s in the same fan-out
where three siblings died at exactly 180,00x ms, and how a parse failure was
able to burn 38 minutes. The controller now stays armed through the body, and a
body-read abort classifies as timeout rather than network. The Grok Files API
upload had no abort signal at all; it is now bounded by GROK_UPLOAD_TIMEOUT_MS.
Rate limits were classified retryable but never retried, and Retry-After was
read nowhere. Both bridges now parse it onto the error, toErrorResult forwards
it, and callProvider retries once for network | rate-limit | empty, waiting the
hint (default 2s, clamped to 30s). timeout stays non-retryable: the call may
already have burned tokens, and a slow-but-good answer should not be discarded.
Gemini counted any non-empty stdout on exit 0 as a full answer, so an 8-second
agy run printing only "I will begin by finding the repository directory..."
reached the caller as a real opinion - and inside consensus parseReview turned
it into verdict:null, silently blocking convergence with no diagnostic. A clean
exit shorter than GEMINI_MIN_ANSWER_CHARS (default 80, 0 disables) now fails as
errorKind "empty" and is retried once. The floor applies only on the normal
clean-exit path; after a soft timeout a short recovered answer still beats a
hard timeout, so the drain branch is unchanged.
Also: a SIGKILL'd codex now reports timeout from the kill flag instead of a
stderr substring match. A killed codex usually writes nothing, so it classified
as unknown - or as auth, since "author" contains "auth" - and an unknown can
never trip the consensus circuit breaker.
The five ask-* slash commands capped at 180000-300000ms, below the new server
ceiling, so /ask-gpt gave up while the provider was still allowed to run.
33 new tests: timeout precedence per provider and through the config layer, the
retry ladder and Retry-After clamp, body-read timeouts against a stalling mock
server, and the Gemini floor plus its escape hatch.
Only the final result was logged, so a rate-limit or empty stub that succeeded on retry left no trace in debug.jsonl - and that log is exactly how provider health gets diagnosed. A retried call now emits two provider_result rows. Found in multi-model review of #167.
Three findings from the multi-model review of #167. The timeout knob missed half the surface. /ask-grok, /ask-gemini and /ask-openrouter call the STANDALONE bridges, not the unified server, and those read their timeout from the call argument only - so they stayed pinned to 180s/300s/180s no matter what providers.defaults.timeout said. Each bridge now resolves the configured ceiling itself: configuredTimeout() in the Grok and Gemini bridges, and providers.openrouter.timeout added to the OpenRouter bridge's pick chain. A non-abort body failure on an OK status was misclassified. A mid-stream socket drop ("TypeError: terminated") was swallowed, leaving an empty body that then failed JSON.parse as `parse` - non-retryable, so the new network retry never ran. It now surfaces as `network`. On an error status the body is only diagnostic, so the status is kept and the body reported empty. The answer floor now applies on the drain path too. Exempting it made validity timing-dependent: the same stub failed when agy was fast and passed when it was slow. Three reviewers flagged this independently. A sub-floor stub recovered after a soft timeout falls through to the hard timeout, which is the honest result. Also: parseRetryAfterMs drops an overflowing delta-seconds instead of forwarding Infinity, which would serialize to null and break the finite-ms contract. 9 new tests (644 total).
Multi-model review (
|
| Delegate | Result |
|---|---|
| Codex (GPT) | Review - 3 HIGH, 2 MEDIUM |
Gemini (gemini-3.6-flash-high) |
Review - 2 critical, 3 recommendations |
| OpenRouter / deepseek-v4-pro | Review - 1 HIGH. Returned at 772,426 ms on a 180s ceiling |
Grok (grok-4.5) |
143s of tool narration, no review - the non-answer class this PR is about |
| OpenRouter / qwen3-7-max | timeout at 180,012 ms |
| OpenRouter / kimi-k3 | timeout at 180,003 ms |
| OpenRouter / glm-5-2 | timeout at 180,002 ms |
Three models dying at exactly 180,00x ms while a fourth ran 12.9 minutes past the same ceiling is the original incident, live, on the installed build.
Acted on
Standalone bridges ignored the configured timeout (Codex, HIGH). /ask-grok, /ask-gemini and /ask-openrouter call the standalone bridges, not the unified server. Only the unified server's composition root was wired, so those three stayed pinned to their built-ins - the knob missed half the surface it claims to cover. Each bridge now resolves the ceiling itself.
Non-abort body failure misclassified (Codex, HIGH). A mid-stream socket drop on an OK status was swallowed, leaving an empty body that failed JSON.parse as the non-retryable parse - so the retry never ran. Now network. Pre-existing, but the new retry made it matter.
Answer floor exempted the drain path (Codex + Gemini + deepseek, independently). It made validity timing-dependent: the same stub failed when agy was fast and passed when it was slow. My original rationale - a short recovered answer beats a hard timeout - doesn't survive that framing. Floor now applies on both paths.
Retried calls hid the first attempt (Gemini, MEDIUM). Only the final result was logged, so a rate-limit that succeeded on retry vanished from debug.jsonl - the log used to diagnose exactly this. Two rows now.
Overflowing Retry-After (Codex, MEDIUM). A 400-digit header became Infinity, serializing to null. Dropped instead.
Declined, with reasons
Lower the 80-char floor to 10-15 (Gemini). The preamble that motivated this is 51 characters - a 15-char floor would not have caught it. GEMINI_MIN_ANSWER_CHARS covers usage that skews short.
The config change breaks fan-out (Gemini). config.providers is only read by keyed lookup (core/registry.js:117, server/mcp/index.js:291); nothing enumerates the map, and the registry filters from the constructed provider list. Emitting entries with enabled: true is inert. Gemini's sandbox denied filesystem access, so it couldn't check. Codex, which could, independently confirmed it is selection-neutral.
The letter-guard rejects numeric dates (Gemini). RFC 9110 defines Retry-After as delta-seconds or IMF-fixdate, and IMF-fixdate always contains letters. A bare ISO date is out of spec, and falls back to the 2s default rather than misparsing as year 1.5.
Retrying empty can double-charge (Codex + Gemini). Accepted as a tradeoff: one retry against an 8-second stub, versus surfacing the stub as a real opinion.
npm run check: 644 pass, 0 fail. Fixes in b93b629 and 5e26011.
1 participant
Why
Three failure modes showed up in one
/ask-allfan-out, and none of them were reachable from config:{"tool":"ask-one","provider":"grok","model":"grok-4.5","ms":718,"isError":true,"errorKind":"rate-limit"} {"tool":"ask-one","provider":"openrouter:qwen3-7-max","ms":180006,"isError":true,"errorKind":"timeout"} {"tool":"ask-one","provider":"openrouter:kimi-k3","ms":180002,"isError":true,"errorKind":"timeout"} {"tool":"ask-one","provider":"openrouter:glm-5-2","ms":180007,"isError":true,"errorKind":"timeout"} {"tool":"ask-one","provider":"codex","ms":600016,"isError":true,"errorKind":"auth"}Separately, Gemini returned in 8s with
"I will begin by finding the repository directory..."and nothing else — and that counted as a successful answer.What changed
Timeouts are configurable for every provider
models.<id>.timeoutwas the only key that governed an MCP-path call. The composition root passed no timeout to codex/gemini/grok, soreq.timeoutMswas structurallyundefinedand each provider sat on its hardcoded default.providers.<name>.timeoutdid not exist in the schema, andproviders.openrouter.defaults.timeoutwas read only by the standalone bridge.New
providers.defaults.timeoutcovers all four at once;providers.<name>.timeoutoverrides one, with OpenRouter reusing thedefaultsblock it already owns rather than gaining a second key.The ladder resolves in
resolveProviders(the config SSOT), so the composition root just reads the resolved value. Read at startup, so a change needs an MCP restart.The timeout did not cover the response body
Both HTTP bridges cleared the abort timer in
finallybeforeawait res.text(), leaving the body read unbounded. This is whydeepseek-v4-prosucceeded at 250,346 ms in the same fan-out where three siblings died at exactly 180,00x ms, and how aparsefailure was able to burn 2,295,013 ms (38 min).The controller now stays armed through the body, and a body-read abort classifies as
timeoutrather thannetwork. The Grok Files API upload had no abort signal at all; it is now bounded byGROK_UPLOAD_TIMEOUT_MS(120s,0disables).Rate limits are handled instead of dropped
rate-limitwas markedretryable: truebutcallProviderretriednetworkonly (exact string match), andRetry-Afterwas read nowhere in the repo. Both bridges now parse it onto the error,toErrorResultforwards it, andcallProviderretries once fornetwork | rate-limit | empty, waiting the hint (default 2s, clamped to 30s so a hostile value cannot stall a fan-out).timeoutstays non-retryable on purpose: the call may already have burned tokens, and a slow-but-good answer should not be discarded.A provider stub is an error, not an answer
server/gemini/index.jsaccepted any non-empty stdout on exit 0 as a full answer, so an 8s preamble reached the caller as a real opinion. Inside consensus,parseReviewturned it intoverdict: null, silently blocking convergence with no diagnostic. (parseOpinion/validateOpinionwould have caught it, but they are referenced only by tests —wellFormed: falsenever reaches a caller.)A clean exit shorter than
GEMINI_MIN_ANSWER_CHARS(default 80,0disables) now fails aserrorKind: "empty"and is retried once. The floor applies only on the normal clean-exit path — after a soft timeout, a short recovered answer still beats a hard timeout, so the drain branch is unchanged.A character count is a proxy for "announced intent instead of answering"; the upgrade path is a structured-output check once
parseOpinionis wired in.Two smaller fixes
timeoutfrom the kill flag instead of a stderr substring match. A killed codex usually writes nothing, so it classified asunknown— or asauth, since"author"contains"auth"— and anunknowncan never trip the consensus circuit breaker.ask-*slash commands capped at 180000–300000 ms, below the new server ceiling, so/ask-gptgave up at 180s while the codex provider was still allowed to run to 600s.Tests
33 new, 633 total, 0 failing:
providers.defaultsnever leaks in as a provider entryrate-limitandemptyretry once,timeout/auth/parse/configdo not, a second failure is returned as-isRetry-Afterparsing in both forms, plus the case thatDate.parse("-5")succeeds as a year — which would have clamped a garbage header to 0 and meant "retry immediately"=0escape hatch, and that output above the floor still succeedsThe existing bridge fixtures print 11-char sentinels to assert plumbing rather than answer length, so
startBridgedisables the floor by default and the floor tests opt back in.Verification
npm run check— 633 pass, 0 fail, typecheck cleannpm run sync:check— persona, host artifacts, command fallbacks all up to datenpm run prepack --workspace=server/mcp— bundle buildsconfig.default.jsonand a real user config both resolve throughvalidateConfigwith the expected per-provider ceilingsNot verified here: a live
/ask-allafter an MCP restart. Constructor args are not hot-reloaded, so that needs/reload-pluginsfirst.Docs
README, SETUP, TECHNICAL, CLAUDE.md, AGENTS.md, plus
config.schema.jsonandconfig.default.json. Generated host artifacts regenerated viascripts/sync-hosts.js.