fix: make the free pool actually fail over when one member is retired by sakibsadmanshajib · Pull Request #1155 · sakibsadmanshajib/hive

sakibsadmanshajib · 2026-08-25T12:02:54Z

Summary

hive-free is served by a load-balanced LiteLLM group of four free provider keys, and its whole product claim is that one exhausted or retired key fails over to the others. It does not. One member answering 404 takes the entire alias down for that request. That is what has been failing Live integration (SDK tests + smoke) on main.

Fixes #1064.

Which of the four candidate causes it was

Routing. Not credentials, not pricing, not metering.

All four members' credentials and models were verified live while diagnosing this:

Member	Upstream	Verified
`route-free-pool-free`	`dots-studio/dots-3-note-preview:free`	listed in OpenRouter's public catalog right now
`route-free-pool-groq`	`openai/gpt-oss-20b`	listed in Groq's `/v1/models` right now
`route-free-pool-groq-2`	`openai/gpt-oss-20b`, second key slot	same model, secret present
`route-free-pool-gemini`	`openai/gemini-flash-latest`	not probeable from the dev box, no key there

All four repository secrets exist. The gateway serves the group: the run's own assertion printed gateway serves route-free-pool.

Request failed, or metering silently dropped it?

Request failed. This matters because the repo has been burned by the second shape before (2026-07-31, the gateway billed nothing for three days and every failure was silent), so it was checked first rather than assumed.

The failing run's own spend report:

      model_alias       |     event_type      | events | input_tokens | output_tokens
------------------------+---------------------+--------+--------------+---------------
 hive-free              | completed           |     14 |          318 |           278
 hive-free              | error               |      1 |            0 |             0
 hive-free              | reconciled          |      2 |            0 |             0
 hive-free              | released            |      2 |            0 |             0
 hive-free              | reservation_created |     10 |            0 |             0

Fourteen metered completions on hive-free, plus one metered error. Metering is healthy and is recording both outcomes. The "zero metered completions" framing does not hold for this run: the served == 0 assertion is gated on SUITES_OUTCOME = success and never executed, because the suites had already failed.

The actual failure

Three JS tests failed, all of them the second hive-free call in their file. The first call in each file passed. Python's suite passed 14/14 because pytest runs sequentially while vitest runs test files in parallel, so only the JS run had concurrent hive-free traffic.

 ❯ tests/completions/completions.test.ts (2 tests | 1 failed)
   ✓ Completions (legacy) > returns a valid text completion via SDK  2451ms
   × Completions (legacy) > model field shows Hive alias not provider handle 128ms
     → 404 hive-free is not available.
 ❯ tests/chat-completions/chat-completions.test.ts (5 tests | 1 failed)
   × Chat Completions > model field shows Hive alias not provider handle 60004ms
     → Test timed out in 60000ms.

The customer-facing hive-free is not available. is our provider-blind collapse of this, from the run's compose-logs artifact:

litellm.NotFoundError: NotFoundError: OpenAIException - Error code: 404
No fallback model group found for original model_group=route-free-pool.
Fallbacks=[{'route-openrouter-embedding': ['route-openrouter-embedding-fallback']}].
Received Model Group=route-free-pool
Available Model Group Fallbacks=None
Cooldown Deployments=[('e644bb2c...', {'exception_received': 'litellm.NotFoundError: ...',
                       'status_code': '404', 'timestamp': ..., 'cooldown_time': 5})]

One deployment returned 404. Three were fine. The request died anyway.

Root cause

Read out of the pinned image's source (v1.98.0), not inferred:

litellm/router.py::should_retry_this_error re-raises immediately for NotFoundError, and again for any status where litellm._should_retry() is false. 404 is both:

status_code = getattr(error, "status_code", None)
if status_code is not None and not litellm._should_retry(status_code):
    if status_code not in (401, 403):
        raise error
if isinstance(error, litellm.NotFoundError):
    raise error

It is called from both async_function_with_retries and async_function_with_fallbacks, so neither the in-group retry across the other three members nor the cross-group fallback ever runs.
litellm/types/router.py::RetryPolicy has fields for BadRequest, Authentication, Timeout, RateLimit, ContentPolicyViolation and InternalServer errors, and none for NotFound. There is no configuration that lifts this.

A retired free model is exactly what returns 404, and free model ids churn constantly on every provider in this pool. So the pool's failover premise is false against the single most likely way a free member dies.

The fix

Make the pool answer. apps/edge-api/internal/inference/retry.go is the one shared dispatch seam that every chat, completions, responses and streaming path already flows through, so the fix lands once for every pooled alias rather than for hive-free alone. It now retries a LiteLLM router-exhaustion 404.

This is sound rather than hopeful because LiteLLM cools the offending deployment down before raising: router_utils/cooldown_handlers.py::_should_cooldown_deployment ends its base case with litellm._should_retry(status) is False -> return True, so a 404 member is out of rotation on its first failure, not after allowed_fails. The next attempt therefore picks a different member, and the existing 300/800/1800 ms ladder finishes inside 2.9 s, comfortably inside the 5 s cooldown, so the dead member cannot be re-picked mid-ladder.

Scope is deliberately narrow: the retry matches on LiteLLM's message, not on a bare 404, so a customer naming a model that does not exist still gets a fast 404 and pays no retries. isRouterExhaustion404 splices the body back together after peeking, so the caller always receives a complete response; a half-consumed body would truncate the error the customer sees and the text CI classifies on.

Make the failure legible. Two additions, neither of which weakens a check:

A new step probes each member individually through LiteLLM's own /health?model=route-free-pool and names the model that is gone. One dead member is a ::warning::, because surviving one is the pool's entire purpose and failing the job on it would make that resilience worthless; the SDK suites remain the real gate. Every member dead is a hard failure, because the free tier then genuinely serves nothing. Output goes through scripts/redact-log-credentials.py like every other published dump in this job.
The upstream-refusal classifier gained a branch for the No fallback model group found signature, so this reads as "a pool member is dead" instead of falling through to "this failure is something else".

No paid fallback was added. Pointing hive-free at a paid model would silently spend real budget and break the one-alias-one-price rule; if a paid fallback is the right product answer, that is an owner decision, not one to smuggle into a CI fix.

A trap this leaves behind, documented rather than fixed

allowed_fails and cooldown_time sit under litellm_settings in deploy/litellm/config.yaml. They look misplaced and in the narrow sense they are: both are Router constructor params, so they never reach the Router, and the effective cooldown is the default 5 s, not the 30 written there. That is observed, not deduced: the failing run's log prints 'cooldown_time': 5.

Moving allowed_fails onto the Router would quietly restore this bug. With it unset, _should_cooldown_deployment takes the base case that cools a 404 member down on the first failure. Set it, and that branch is skipped for should_cooldown_based_on_allowed_fails_policy, which needs fails > allowed_fails: four failures before a dead member leaves rotation, which the retry ladder above cannot outlast. The config now says so in place, so the next tidy-up does not walk into it.

What kind of check actually closes this gap

The pool already had unit coverage, apps/control-plane/internal/routing/free_pool_router_test.go, and it passed the entire time this was broken. It parses the migration text and asserts the four rows share one litellm_model_name. That assertion was true; the shape was right and the behaviour was wrong. A test that can only see the config can never catch a failure that lives in the router's exception handling.

What closes it is a test that exercises the real function against the real upstream envelope, so this PR adds behavioural coverage over dispatchWithRetry using the verbatim 404 body from run 32830060362:

TestDispatchWithRetry_FailsOverWhenOnePoolMemberIsGone — dead member on attempt 1, 200 on attempt 2. Fails on main.
TestDispatchWithRetry_GenuineModelNotFoundIsNotRetried — bounds the blast radius; a real model-not-found stays one attempt.
TestDispatchWithRetry_DeadPoolReturnsTheUpstream404Intact — when retries cannot rescue it, the client still gets the real status and a complete body.
TestIsRouterExhaustion404LeavesTheBodyReadable — the body splice survives a payload larger than the peek window.

Plus scripts/report-free-pool-health.py --selfcheck, covering the one-dead / all-dead / missing-counts / malformed-endpoint verdicts.

The live half is closed by the new probe step, which is the only thing in CI that can name a retired model, and by the classifier branch that stops this failure shape reading as unexplained.

Test evidence

=== RUN   TestDispatchWithRetry_FailsOverWhenOnePoolMemberIsGone
--- PASS: TestDispatchWithRetry_FailsOverWhenOnePoolMemberIsGone (0.00s)
=== RUN   TestDispatchWithRetry_GenuineModelNotFoundIsNotRetried
--- PASS: TestDispatchWithRetry_GenuineModelNotFoundIsNotRetried (0.00s)
=== RUN   TestDispatchWithRetry_DeadPoolReturnsTheUpstream404Intact
--- PASS: TestDispatchWithRetry_DeadPoolReturnsTheUpstream404Intact (0.00s)
=== RUN   TestIsRouterExhaustion404LeavesTheBodyReadable
--- PASS: TestIsRouterExhaustion404LeavesTheBodyReadable (0.00s)
PASS
ok  	github.com/sakibsadmanshajib/hive/apps/edge-api/internal/inference	0.028s

Full module, plus build and vet:

BUILD_OK
VET_OK
ok  github.com/sakibsadmanshajib/hive/apps/edge-api/internal/inference  3.161s
ok  github.com/sakibsadmanshajib/hive/apps/edge-api/internal/errors     0.065s
... (25 packages, all ok)
ok  github.com/sakibsadmanshajib/hive/apps/control-plane/internal/routing       0.185s
ok  github.com/sakibsadmanshajib/hive/apps/control-plane/internal/litellmconfig 0.181s

$ python3 scripts/report-free-pool-health.py --selfcheck
ok: report-free-pool-health verdicts and fallbacks

$ node tools/lint-litellm-config-fallbacks.mjs
lint-litellm-config-fallbacks: PASS

Redaction verified end to end on a synthetic error carrying a key-shaped value:

  DEAD MEMBER: openai/gemini-flash-latest -- Error code: 404 - models/gemini-flash-latest is not found. api_key=<redacted>

gofmt -l output is byte-identical before and after this change, so no formatting regression was introduced.

Buglog entry

{"id":"free-pool-404-no-failover","date":"2026-08-25","title":"LiteLLM aborts a load-balanced group on one member's 404 instead of failing over, taking hive-free down","error_message":"litellm.NotFoundError: NotFoundError: OpenAIException - Error code: 404No fallback model group found for original model_group=route-free-pool. Available Model Group Fallbacks=None -- surfaced to the customer as 'hive-free is not available.'","root_cause":"litellm/router.py::should_retry_this_error re-raises immediately for NotFoundError and for any status where litellm._should_retry() is false; 404 is both. It is called from async_function_with_retries and async_function_with_fallbacks, so one pool member answering 404 (what a retired free model returns) killed the request while the group's other three members were healthy. litellm/types/router.py::RetryPolicy has no NotFoundErrorRetries field, so no config lifts it. The pool's existing unit test asserts the four rows share a litellm_model_name, which stayed true throughout, so the shape was right and the behaviour was wrong.","fix":"Retry a LiteLLM router-exhaustion 404 in the shared dispatch seam (apps/edge-api/internal/inference/retry.go), matched on LiteLLM's message rather than on a bare 404 so a genuine model-not-found stays fast. Sound because LiteLLM cools the 404 deployment down on its first failure, so the next attempt picks a different member inside the 5s window. Added behavioural tests over dispatchWithRetry, a CI step that names a dead member via /health?model=, and a classifier branch for the signature.","tags":["litellm","routing","free-pool","failover","ci","edge-api","hive-free","404"]}

{"id":"litellm-router-knobs-in-wrong-config-block","date":"2026-08-25","title":"allowed_fails and cooldown_time under litellm_settings never reach the LiteLLM Router, and moving allowed_fails would break free pool failover","error_message":"config declares cooldown_time: 30 but the runtime log prints 'cooldown_time': 5 in its Cooldown Deployments line","root_cause":"allowed_fails and cooldown_time are Router constructor params (litellm/types/router.py::UpdateRouterConfig), not litellm_settings keys, so the values are silently ignored and DEFAULT_COOLDOWN_TIME_SECONDS (5) applies. The apparent misplacement is load-bearing: with allowed_fails unset on the Router, _should_cooldown_deployment takes its base case and cools a 404 member down on the first failure, which is what lets the edge retry land on a different member.","fix":"Left both settings in place deliberately and documented the trap in deploy/litellm/config.yaml, including that moving allowed_fails into router_settings would require four failures before a dead member leaves rotation and would quietly restore the free pool 404 bug.","tags":["litellm","config","cooldown","free-pool","footgun"]}

The `hive-free` alias is served by a load-balanced LiteLLM group of four free provider keys, and its whole product claim is that one exhausted or retired key fails over to the others. It does not. One member answering 404 takes the entire alias down for that request, which is what has been failing the Live integration job on main. Root cause, read out of the pinned image's source (v1.98.0) rather than inferred: * `litellm/router.py::should_retry_this_error` re-raises immediately for `NotFoundError`, and again for any status where `litellm._should_retry()` is false. 404 is both. It is called from `async_function_with_retries` AND from `async_function_with_fallbacks`, so neither the in-group retry across the other three members nor the cross-group fallback ever runs. * `litellm/types/router.py::RetryPolicy` has no `NotFoundErrorRetries` field, so no configuration lifts this. * A retired free model is exactly what returns 404, and free model ids churn constantly on every provider in the pool. The edge now retries a LiteLLM router-exhaustion 404 in the one shared dispatch seam every chat, completions, responses and streaming path already flows through. This is sound rather than hopeful because LiteLLM cools the offending deployment down before raising (`_should_cooldown_deployment` returns true on the first failure when `_should_retry(status)` is false), so the next attempt picks a different member, and the whole ladder finishes inside the cooldown window. The retry is matched on LiteLLM's message, not on a bare 404, so a customer naming a model that does not exist still gets a fast 404 and pays no retries. Also makes the failure legible instead of generic: * A new step probes each pool member individually through LiteLLM's own per-group health endpoint and names any model that is gone. One dead member is a warning, since surviving it is the pool's entire purpose; every member dead is a hard failure, since the free tier then serves nothing. * The upstream-refusal classifier gained a branch for the router exhaustion signature, so it reads as "a pool member is dead" rather than falling through to "something else". `allowed_fails` and `cooldown_time` in the LiteLLM config look misplaced under `litellm_settings` and are now documented as deliberately left there: moving `allowed_fails` onto the Router would require four failures before a dead member leaves rotation and would quietly restore this bug. Fixes #1064

coderabbitai · 2026-08-25T12:03:05Z

Warning

Review limit reached

Next included review available in 17 minutes.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 410b9e05-4fc1-4a6d-b248-232abab8a342

📥 Commits

Reviewing files that changed from the base of the PR and between d369b3c and b3a5fa5.

📒 Files selected for processing (5)

.github/workflows/ci.yml
apps/edge-api/internal/inference/retry.go
apps/edge-api/internal/inference/retry_test.go
deploy/litellm/config.yaml
scripts/report-free-pool-health.py

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

The Live integration job is gated on the run-live-integration label, which was added after the first push, so the initial run predates it. This empty commit re-triggers the workflow so the free pool fix is exercised against real providers before merge.

chatgpt-codex-connector · 2026-08-25T12:19:09Z

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

sakibsadmanshajib · 2026-08-25T12:38:05Z

Verified live, twice

Live integration (SDK tests + smoke) passed on two consecutive runs of this branch (32845556683, 32846939118). The whole CI run is green: 26 checks pass, 4 skip, 0 fail.

The three JS tests that this issue was about now pass:

== sdk-tests-js  (exit 0)

The suites now reach the token meter's success branch, which was never exercised on the failing runs because it is gated on SUITES_OUTCOME = success:

 hive-free              | completed           |     18 |          403 |           551
 hive-free              | reconciled          |      2 |            0 |             0
 hive-free              | reservation_created |     10 |            0 |             0

live-integration metered 1654 tokens across the configured aliases (ceiling 6000)
18 completion(s) served by 'hive-free'

Eighteen metered completions on hive-free, and no error event at all — the failing run had hive-free | error | 1. Only the three permitted aliases were billed, so the model knob still reaches the suites and nothing leaked onto a paid route it did not choose.

What the new probe revealed, and what it changes about the diagnosis

free pool members: 4 healthy, 0 unhealthy

All four members answer. So the 404 that took the alias down was an intermittent blip from a member that is alive, not a permanently retired model. That is a sharper diagnosis than the one this PR was opened with, and it makes the fix more clearly right rather than less: the pool was not surviving even a transient single-member 404, which is the cheapest possible failure to absorb and exactly what a four-member pool exists to absorb.

It also means the earlier failures were load-shaped, which fits the evidence: in every failing file the first hive-free call passed and the second failed, and only the JS suite failed because vitest runs test files in parallel while pytest is sequential.

The probe step keeps its value either way. Free model ids do get retired, and when one does, this step now names it instead of leaving a generic hive-free is not available. with nothing pointing at which of the four models went away.

sakibsadmanshajib · 2026-08-25T16:17:12Z

Post-merge safety audit (independent, PR merged before review completed)

Verdict: no double-billing bug, no stranded-hold bug. Read against the actual merged code on origin/main (34c90fd54), not the PR diff description.

Why double-billing cannot happen here

dispatchWithRetry (apps/edge-api/internal/inference/retry.go) is called exactly once per request from each of the three call sites (orchestrator.go:228, stream.go:236, stream_responses.go:167), and in every caller the reservation (CreateReservation) and the usage attempt (StartAttempt) are created once, before that single call. All internal retries (429/5xx, and now the new router-exhaustion 404) happen inside that one call and are invisible to accounting: only the final *http.Response it returns is normalized/settled. A 200 is returned immediately (if !retryableStatuses[...] && !isRouterExhaustion404(resp) { return resp, nil }) and is never retried again, so a response that already succeeded upstream cannot trigger a second dispatch even if downstream JSON parsing later fails — that failure path releases the hold, it does not re-dispatch.

The 404 addition in this PR only changes what counts as retryable; it does not touch reservation/finalize/release logic at all. Confirmed via git diff ed66a46d8 34c90fd549 --stat: the only files touched are retry.go, retry_test.go, .github/workflows/ci.yml, deploy/litellm/config.yaml, scripts/report-free-pool-health.py. orchestrator.go, stream.go, stream_responses.go, accounting_client.go are byte-identical to before.

Why holds cannot strand

The single-terminal-state pattern (finalized bool + defer-gated release/settle on a fresh background context, independent of the request context) predates this PR and is unmodified by it. dispatchWithRetry returning an error (including ctx.Err() on client disconnect mid-ladder) is handled the same way a pre-existing 429/5xx failure always was: releaseReservationBackground fires from the defer, on context.Background() with its own accountingTimeout, gated on the release actually reaching control-plane before finalized is set. Nothing about the new 404 path bypasses this.

Streaming: retry cannot fire after bytes reach the client

Both stream.go and stream_responses.go call dispatchWithRetry strictly before w.WriteHeader/flusher ever runs (explicit in-code comment: "safe: no bytes have been written to the client yet at this point"). Confirmed by reading the function bodies directly; the SSE relay loop starts only after dispatchWithRetry has already returned a resolved response.

Retry ladder vs. LiteLLM cooldown — independently re-verified against the pinned image source (not just the PR's citation)

Pulled litellm==1.98.0 from PyPI and fetched router_utils/cooldown_handlers.py + router.py at tag v1.98.0 directly from BerriAI/litellm on GitHub:

litellm._should_retry(404) returns falsy (only 408/409/429/5xx return True), so _should_cooldown_deployment's base case (elif litellm._should_retry(status_code=...) is False: return True) puts a 404-erroring deployment into cooldown on its first failure, not after allowed_fails.
DEFAULT_COOLDOWN_TIME_SECONDS = 5 (env-overridable, defaults to 5), matching the 'cooldown_time': 5 the original failing CI run actually logged.
should_retry_this_error re-raises immediately for NotFoundError and for any non-401/403 status _should_retry rejects, confirming neither in-group retry nor cross-group fallback runs for a 404.

The 2.9s worst-case retry ladder is comfortably inside the 5s cooldown window, so a dead pool member cannot be re-selected mid-ladder, provided allowed_fails/cooldown_time stay off the Router (already documented as a footgun in deploy/litellm/config.yaml by this same PR).

One real, non-blocking gap found (opened separately)

The retry is scoped by matching LiteLLM's error message text ("no fallback model group found"), not a status code, and there is no test that would fail if that wording changes on a LiteLLM version bump — the fixture in retry_test.go is a hardcoded copy of one historical error body, not a live check against the pinned image. Dependabot is configured to bump the Docker image in /deploy/docker monthly, which is exactly the path that would carry a reworded error message in. If that happens, the fix silently reverts to the original bug (pool stops failing over on 404) with no unit test going red — only a live-traffic symptom recurring. See the filed issue below for detail and a cheap mitigation.

No action needed on this PR itself; it does not need to be reverted.

…lization boundary (#1169) ## Summary The deploy of merge commit 2df3afd (PR #1163) failed its SDK replay against the demo box (run 32879931588): ``` tests/chat-completions/chat-completions.test.ts:36 "Chat Completions > returns a valid chat completion via SDK" AssertionError: expected 'object' to be 'string' Expected: "string" Received: "object" ``` **Correction to the field named in the incident report.** The failing assertion is `typeof response.choices[0].message.content`, not `response.model` as first reported. Line 36 of the test is: ```ts expect(typeof response.choices[0].message.content).toBe("string"); ``` `response.model` is asserted separately at line 51 in the very next test in the same file (`model field shows Hive alias not provider handle`), which passed. `typeof null === "object"` in JavaScript, so "Received: object" for a `content` assertion means the wire value was a bare `null`, not an actual object. ## Root cause **Not PR #1163.** That PR touches only `apps/edge-api/internal/anthropic/`, adding request-side fields (`Thinking`, `TopK`, `User`, `ParallelToolCalls`, `ThinkingBlocks`, `ThinkingConfig`) to that package's own `OAIRequest`/`OAIMessage` structs. Those structs are private to the `anthropic` package: `grep -rn "anthropic\." apps/edge-api/internal/inference/*.go` returns no hits outside a comment. The OpenAI-compatible chat-completions path (`apps/edge-api/internal/inference`) has its own separate `ChatCompletionResponse`/`ChatCompletionMessage` types and never touches the anthropic package's types. No shared struct, no cross-package marshaling effect. **The real cause is the free-pool router**, PR #1115 (`hive-free`, four load-balanced provider members: OpenRouter `dots-3-note-preview:free`, Google `gemini-flash-latest`, and two Groq `gpt-oss-20b` keys) plus its retry fix PR #1155 (`dispatchWithRetry`, which lets a request land on any member on retry). The replay's `HIVE_TEST_MODEL` is `hive-free` (`deploy-demo-box.yml`), so this plain "Say hello" / `max_tokens: 256` request could land on any of the four. The failing test ran for 50.8s versus 1-7s for its four sibling tests in the same run, consistent with a reasoning-capable member (Groq's `gpt-oss-20b` or Gemini's thinking-capable flash model) spending its entire token budget on hidden reasoning before hitting the length limit, returning `finish_reason: "length"` with `message.content` omitted from the JSON entirely. Go's `ChatCompletionMessage.Content` is `*string`. Unmarshaling a response missing that key leaves it `nil`; `normalizeChatCompletion` in `apps/edge-api/internal/inference/chat_completions.go` re-marshaled that nil pointer straight back out as JSON `null`, which every OpenAI SDK (this test uses the real `openai` npm package) treats as an unconditional string. This exact class of failure was already documented in `deploy-demo-box.yml`'s own comments for a different alias (`deepseek-v4-flash`: "returned message.content as object/null/string across probes"), which is why `HIVE_TOOLS_MODEL` was pinned away from it. `hive-free` was never given the same treatment because its tool-free smoke test uses no `max_tokens` pressure test and had not yet hit a reasoning-heavy member in CI. **This gateway is provider-blind by design.** No pool member's response shape should leak to a client through the OpenAI-compatible surface. OpenAI's own contract makes `content` nullable *only* alongside `tool_calls`/`function_call`; every other case must be a string. That is where this fix lives. ## Fix `normalizeChatCompletion` (`apps/edge-api/internal/inference/chat_completions.go`) now coerces a `nil`, tool-free `message.content` to an empty string before marshaling the response, via a new `coerceNullContent` helper. A genuine tool-call message with `content: null` is left untouched, since that shape is spec-correct per OpenAI and is exercised by the existing "passes tools through" replay test. ## Why no existing test caught this pre-merge `packages/sdk-tests` only runs against a live deployed box (`sdk-replay` job, post-merge), never in CI unit/integration tests, because it needs a real LiteLLM + provider round trip. The Go unit-test suite for `normalizeChatCompletion` (`usage_clamp_test.go`) had fixtures for zero-completion-token clamping but none exercising an upstream response with `content` entirely absent from the JSON. Added two new unit tests in `chat_completions_null_content_test.go` that assert on the actual marshaled wire bytes (not just the Go struct), which would have caught this before any live deploy: - `TestNormalizeChatCompletion_NullContentCoercedToEmptyString`: reproduces the missing-`content` upstream shape and asserts the outgoing JSON has `"content":""`, never `null`. - `TestNormalizeChatCompletion_NullContentPreservedWithToolCalls`: asserts a genuine tool-call message's `content: null` is left alone. ## Buglog entry To be appended to `.wolf/buglog.jsonl` on `main` via a separate buglog-only PR, per `.claude/rules/openwolf.md`: ```json {"date":"2026-08-25","error_message":"AssertionError: expected 'object' to be 'string' at tests/chat-completions/chat-completions.test.ts:36 (response.choices[0].message.content)","root_cause":"hive-free free-pool member (PR #1115/#1155) returned message.content omitted (null) after burning its max_tokens budget on hidden reasoning; normalizeChatCompletion re-marshaled the nil *string as JSON null instead of coercing it, leaking a non-OpenAI-contract shape to every SDK client","fix":"apps/edge-api/internal/inference/chat_completions.go: normalizeChatCompletion coerces nil, tool-free message.content to an empty string; tool-call messages with null content are left untouched per the OpenAI contract","tags":["free-pool","chat-completions","normalization","sdk-replay","hive-free"]} ``` ## Test plan - [x] `go build ./apps/edge-api/...` - [x] `go vet ./apps/edge-api/...` - [x] New unit tests pass: `go test ./apps/edge-api/internal/inference/... -run 'NullContent|NormalizeChatCompletion' -v` - [x] Full `apps/edge-api` suite green, no regressions: `go test ./apps/edge-api/... -count=1 -short` - [ ] Next deploy's `sdk-replay` job green against the live demo box (this PR cannot verify that itself; the orchestrator confirms on merge) Reproduction: static, from the actual failing-run log (`gh run view --job 97907530300 --log`) plus static code reading. Live reproduction against the demo box was not attempted directly (no API key available to this agent; another agent was separately reported to be investigating an unrelated Cloudflare Tunnel issue on the same box). The box itself answered a plain reachability probe (`401` on `/v1/models` with no auth) during this session, so it was not down at the time of investigation.

…1177) ## Summary Live verification of every capability DEMO.md claims, against the actually deployed box (chat-hive, console-hive, api-hive, control-hive), run today after the 2026-08-25 Cloudflare regional-edge false alarm cleared. Full capability matrix and methodology in `docs/proof/demo-readiness-verify-2026-08-25/log.md`. **Confirmed fixed, DEMO.md corrected (was stale):** - Artifacts (#1110, fixed by PR #1141): `/artifacts` renders a real empty-state index today, not the "spins forever" DEMO.md described. No sidebar entry yet (tracked by #943 item 4, not new). - In-chat credits (#1063, fixed by PR #1119): a "You've used N credits today, N remaining" strip sits above the composer, matching the console Billing balance exactly. **Confirmed still broken, unchanged:** - Knowledge nav (#1109): clicking it still does nothing (URL unchanged). Direct `/knowledge` now answers an honest 404 instead of the originally-reported silent bounce home, a minor symptom shift, not a fix. **Verified today's merges, all landed after this session started:** - Cache-aware billing (#1157) and Anthropic `cache_control` passthrough (#1152): shipped and tested, but unexercised live. The catalog has no Anthropic model today, and a direct `usage_events` query shows zero cache-bearing requests since deploy. - Free pool failover (#1155) and the null-content coercion fix (#1169): no regressions in the trailing 24h of live traffic (zero error-status `usage_events` rows across 356 requests), though neither fix's specific trigger recurred live to re-test directly. - External uptime probe (#1166): confirmed running on its 15-minute schedule, all green. Added a T-1 checklist note pointing at it, since today's regional Cloudflare maintenance window is exactly the scenario it exists to catch. **Corrected a claim broader than the two named stale items:** the "not demoable: multi user isolation (#947, #948, #949 family)" line was itself stale. All three were fixed 2026-08-23 (PRs #960, #1067, #1091, #1096). One residual, #1056 (two Knowledge by-id/files routes still short-circuit on `role == admin`), is still open, so the line now says that precisely instead of citing three closed issues. ## New issues filed None. Every genuinely broken thing found already has an open tracking issue (#1109, #1056, #943). ## Verification - `node tools/lint-no-token-in-proof-captures.mjs` passes against the new proof log. - Live session obtained via the standard admin one-time-token mint (`docs/live-test-auth.md`), read-only against the demo fixture account, no password touched, no message sent, no key minted, no task submitted. - Screenshots posted separately to the PR via `scripts/post-pr-visual-proof.sh` (permanent GitHub Release, per `.wolf/decisions.md` D-042). ## Test plan - [x] `node tools/lint-no-token-in-proof-captures.mjs` - [x] Manual read of the rendered DEMO.md for internal consistency Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

sakibsadmanshajib added the run-live-integration label Aug 25, 2026

sakibsadmanshajib marked this pull request as ready for review August 25, 2026 12:19

sakibsadmanshajib merged commit 34c90fd into main Aug 25, 2026
42 of 43 checks passed

sakibsadmanshajib deleted the fix/free-pool-live branch August 25, 2026 14:24

sakibsadmanshajib mentioned this pull request Aug 25, 2026

ci: assert pinned LiteLLM still contains the free-pool retry marker when the image bumps #1160

Closed

This was referenced Aug 25, 2026

retry.go's router-exhaustion 404 match has no drift detection against a LiteLLM version bump #1162

Open

fix: coerce null chat completion content to empty string at the normalization boundary #1169

Merged

This was referenced Aug 25, 2026

free pool: decide max_tokens semantics across reasoning and non-reasoning members #1171

Closed

docs: correct DEMO.md against live verification of the deployed box #1177

Merged

Navigation Menu

Repository navigation

fix: make the free pool actually fail over when one member is retired - #1155

fix: make the free pool actually fail over when one member is retired#1155
sakibsadmanshajib merged 2 commits into
mainfrom
fix/free-pool-live

sakibsadmanshajib commented Aug 25, 2026

Uh oh!

coderabbitai Bot commented Aug 25, 2026 •

edited

Loading

Review limit reached

Uh oh!

chatgpt-codex-connector Bot commented Aug 25, 2026

Uh oh!

sakibsadmanshajib commented Aug 25, 2026

Uh oh!

Uh oh!

sakibsadmanshajib commented Aug 25, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

sakibsadmanshajib commented Aug 25, 2026

Summary

Which of the four candidate causes it was

Request failed, or metering silently dropped it?

The actual failure

Root cause

The fix

A trap this leaves behind, documented rather than fixed

What kind of check actually closes this gap

Test evidence

Buglog entry

Uh oh!

coderabbitai Bot commented Aug 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review limit reached

Uh oh!

chatgpt-codex-connector Bot commented Aug 25, 2026

Uh oh!

sakibsadmanshajib commented Aug 25, 2026

Verified live, twice

What the new probe revealed, and what it changes about the diagnosis

Uh oh!

Uh oh!

sakibsadmanshajib commented Aug 25, 2026

Post-merge safety audit (independent, PR merged before review completed)

Why double-billing cannot happen here

Why holds cannot strand

Streaming: retry cannot fire after bytes reach the client

Retry ladder vs. LiteLLM cooldown — independently re-verified against the pinned image source (not just the PR's citation)

One real, non-blocking gap found (opened separately)

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

coderabbitai Bot commented Aug 25, 2026 •

edited

Loading