v0.1.78

Per-(provider, model) unavailability marking, Retry-After aware

Built live against a real production incident: coding-premium's muse-spark-1.3 tier was overloaded upstream (503 service_overloaded/500/504, intermittently hanging up to 90s). A real curl against the live upstream confirmed it sends a standard Retry-After: 60 header on its 503 — this gateway read the error body but never the response headers at all, so that signal was silently discarded.

Operator's own framing, which drove the design: the request is the unit, not the provider. If a target fails, mark it unavailable; the next real request is itself the retry (honoring the vendor's own Retry-After, or a configurable default backoff otherwise) — if it fails too, it falls through to the next tier exactly like any other failed attempt, invisibly to whoever's waiting on it. No separate background prober.

New internal/unavail package

A simpler, always-on sibling to the existing per-(alias,tier) circuit breaker:

• Keyed by (provider, upstream_model) directly — a target's health is a fact about the vendor/model, not about any one alias's fallback chain, so a failure discovered via one alias immediately protects every other alias routing to the same target.
• No error-rate window, no consecutive-failure counter, no half-open probe-admission race.
• Honors the upstream's own Retry-After exactly when present (seconds or HTTP-date, RFC 9110 §10.2.3); absent that, a configurable default backoff doubles from the target's last mark, capped (built-in default 200ms → ~25.6s over 8 doublings).
• Redis-shared across replicas, fails open if Redis is unreachable.

Also

• providers.Error.RetryAfter, threaded through every non-2xx upstream call site (chat, audio, model-catalog listing).
• Vendor X-RateLimit-* headers logged when present (observability only, no behavior tied to them yet).
• Wired into executePlan alongside the existing breaker, reusing countsAgainstTier's classification so both mechanisms agree on what's attributable to a target.

Test plan

• internal/unavail: 7 unit tests, -race clean. Two genuine bugs caught and fixed by these tests before reaching executePlan: Retry-After wrongly clamped by the default max, and a unit-scaling bug that silently reset the backoff every other mark.
• internal/providers: Retry-After parsing (including the exact 60 confirmed live), httpError population, rate-limit-header logging.
• internal/httpapi: 3 executePlan wiring tests (skip-then-retry, Retry-After-over-default, cross-alias sharing) — surgical-revert proven.
• Full suite green with -race.

Shipped via #161, chart bump #162.

Full Changelog: v0.1.77...v0.1.78

v0.1.74

Frontend — 3 of 62 confirmed findings (14-16 of 62) — campaign complete

Re-derived from current code by a fresh investigation fork. Bundled into one PR since all three are small and land in the same subsystem — the last one in this 62-finding whole-project review campaign.

Numeric fields in modalForm had no native HTML5 validation

The generic field renderer never emitted type/min/max. Confirmed affected: editProvider's max_concurrency and all 5 numeric fields in editPrice. This is the frontend counterpart of an Admin API backend bug fixed earlier this campaign (negative max_concurrency silently became "unlimited", now a 400): the backend rejects it, but the form gave zero client-side signal. Added a type:"number" branch with min/max/step, extracted the whole field renderer into its own renderFormField function (same reason staleGuard already is one) for testability.

Copy-token button had no clipboard failure handling

navigator.clipboard is genuinely undefined on a plain-HTTP origin (TLS is opt-in here, not default) — confirmed live. Added a guard plus .catch(), both showing a clear toast.

"Import prices" silently no-op'd with zero providers configured

Now shows a toast naming the reason.

• Test: renderformfield.test.mjs (new, 4 unit tests, mirrors staleguard.test.mjs)
• Surgical-revert proof: disabling the new branch fails 1 of 6 total JS tests
• Live-verified against the running dev stack with a real browser (Playwright + system Chrome): navigator.clipboard confirmed undefined on this deployment's actual non-localhost HTTP origin, the fallback toast fired exactly as designed; typing -1 into max_concurrency and clicking Save is genuinely blocked by the browser's own native constraint validation, modal stays open.

Shipped via #151, chart bump #152.

---

🎉 Campaign complete: all 62 confirmed findings fixed

This release closes the whole-project review campaign started earlier this session: 10 Critical + 26 Important + every re-derived Minor finding across all 8 subsystems (Auth, DLP/capture, Protocol translation, Limits/pricing, Admin API, Store/config, Routing/fallback, Frontend) — all fixed, tested with a real failing-then-passing proof, surgical-revert-verified, and shipped through the full branch → PR → CI → merge → chart bump → tag → release → image-verification pipeline, one finding (or small same-subsystem batch) at a time, from v0.1.48 through this release.

Full Changelog: v0.1.73...v0.1.74

v0.1.73

Routing/fallback — 3 of 62 confirmed findings (11-13 of 62)

Re-derived from current code by a fresh investigation fork. Bundled into one PR since all three are small and land in the same subsystem.

Unregistered target fell through to a generic, unclassified error

runChat/runStream silently fell back on a plain error when a resolved target's provider wasn't in the live registry — a real gap: a Postgres row with enabled=true can still fail to build into the in-memory Registry (e.g. a malformed Vertex config), logged only at startup/reload time. If every tier hit this, the client just got a generic 502 with nothing telling an operator the alias was silently broken. Now logged at the moment a real request hits the gap, naming the alias and provider.

Lazy-resolve race in the BERT model pool

modelpool.Pool.Scan's if !p.resolved.Load() { p.Resolve() } let several concurrent Scans against a never-Start()-ed pool each kick off their own independent Resolve — same shape as the already-fixed model-catalog cache finding. A Start()-ed pool is unaffected. Guarded with sync.Once, matching this codebase's existing lazy-start idiom.

Disabled vs. nonexistent provider gave the identical message

passthroughTarget filtered on enabled = true, so a disabled provider and a nonexistent one both got "not found" — misleading, not a leak (the key already had to know the name). Now distinguishes "is disabled" from "not found".

• Test: TestRunChatLogsUnregisteredTarget, TestScanConcurrentLazyResolveDedupes (-race clean), TestPassthroughDisabledProviderDistinctFromNotFound
• Surgical-revert proof: each reverted genuinely — including a real self-caught false positive: the passthrough test's first version named its fixture provider "passthrough-disabled-<ts>", and reverting the fix still passed because the provider's own name contained the word "disabled" the assertion was checking for. Renamed the fixture and added negative assertions before shipping.

Shipped via #149, chart bump #150.

★ Routing/fallback subsystem: every finding (Critical/Important/Minor) now fully closed.

Next up: Frontend's 2 uncatalogued Minor findings — the LAST subsystem in the full 62-finding campaign.

Full Changelog: v0.1.72...v0.1.73

v0.1.72

Store/config — 3 of 62 confirmed findings (8-10 of 62)

Re-derived from current code by a fresh investigation fork. Bundled into one PR since all three are small and land in the same subsystem.

Multi-role policy merge depended on incidental row order

EffectivePolicy's "first non-empty limits wins" rule scanned roles_policy with no ORDER BY — and that table has no precedence/priority column at all. Which role's limits applied to a user holding two roles with different non-empty limits depended on whatever order an unordered scan happened to return — a VACUUM, a changed query plan, or a Postgres version bump could silently flip it. Added ORDER BY role: this does not invent a real precedence model, it only makes the existing rule reproducible instead of accidental.

N+1 query in role-triggered snapshot rebuild

RebuildKeySnapshotsRole re-queried each affected user's roles individually even though its own initial query already knew every affected user's id. Extracted a shared applyKeySnapshot helper and had the role query return roles alongside each id, eliminating the per-user re-query.

Duplicate OIDC_ROLE_MAP key silently resolved last-wins

parseRoleMap resolved a repeated idp_role key via plain map assignment with zero validation or log line. Operator-typed config, not attacker-reachable, but an easy typo to make invisible — now logs a warning naming the key and both values. Behavior unchanged.

• Test: TestEffectivePolicyMultiRoleLimitsAreDeterministic, TestRebuildKeySnapshotsRoleAvoidsPerUserRoleQuery, TestParseRoleMapLogsDuplicateKey (all new)
• Surgical-revert proof: each of the three reverts genuinely broke its own test (wrong role's limits won; QueryRow count went from 0 back to 3; removing the warning removed the only use of log/slog in the file, failing the build)

Shipped via #146, chart bump #147.

Next up: Routing/fallback's 3 uncatalogued Minor findings — fresh investigation fork, same methodology.

Full Changelog: v0.1.71...v0.1.72

v0.1.71

Admin API — 3 of 62 confirmed findings (5-7 of 62)

Re-derived from current code by a fresh investigation fork (the original review's scratchpad detail is gone). Bundled into one PR since all three are small and land in the same subsystem.

Negative max_concurrency silently became "unlimited"

handleAdminPutProvider clamped max_concurrency < 0 to 0 — but 0 means unlimited (a nil semaphore lets every Acquire succeed). A fat-fingered or buggy admin request silently got the single most permissive outcome instead of a validation error. Now rejected as a 400.

?limit on the captures list had no upper bound

handleAuditListCaptures validated limit > 0 but never capped it, unlike the established clampHours pattern (capped at 168) used for this package's analogous usage-series endpoints. Now capped at 500.

Model-catalog cache had no single-flight lock

handleAdminProviderModels's TTL cache let N concurrent requests racing an empty (or just-expired) cache each independently call the upstream model-list API. Now deduped via golang.org/x/sync/singleflight, the same package this codebase already uses for Vertex token resolution.

• Test: TestPutProviderRejectsNegativeMaxConcurrency, TestAuditListCapturesClampsLimit, TestProviderModelsConcurrentRequestsDedupe (-race clean, 5 concurrent requests against a lister blocked until released)
• Surgical-revert proof: each of the three reverts genuinely broke its own test (nil-DB panic past validation; ?limit=999999999 passed through unclamped; 5 independent upstream calls instead of 1)

Shipped via #144, chart bump #145.

Next up: Store/config's 4 uncatalogued Minor findings (one, a migration PK-change edge case, is already noted as low-priority/deferred — the rest need a fresh investigation fork the same way).

Full Changelog: v0.1.70...v0.1.71

v0.1.70

Protocol translation — 4 of 62 confirmed findings (subsystem's Minor findings now fully closed)

Documented the Anthropic/OpenAI temperature-scale mismatch

Anthropic documents temperature as 0–1, OpenAI as 0–2. The IR passes the raw value through unchanged (verified via code inspection — no rescaling in internal/anthropic, internal/openai, or internal/llm). Since every real upstream in this codebase is OpenAI-shaped, an Anthropic client's temperature: 1 (its own documented maximum) reaches the upstream as OpenAI's midpoint, not its maximum.

The investigation fork that surfaced this framed it as a documentation gap, not a specific code fix — there's no "correct" rescaling to apply without a real Anthropic-speaking upstream to calibrate against. Added to docs/translation.md's existing "Cross-protocol caveats" section. Docs-only change, no logic touched.

Shipped via #142, chart bump #143.

★ Protocol translation subsystem: every finding (Critical/Important/Minor) now fully closed.

Next up: Admin API's 4 uncatalogued Minor findings — spawning a fresh investigation fork to re-derive them from current code, the same methodology used for Auth, DLP/capture, and Protocol translation's Minor findings this campaign.

Full Changelog: v0.1.69...v0.1.70

v0.1.69

Protocol translation — 3 of 62 confirmed findings

Anthropic ingress now requires a positive max_tokens

Anthropic's real Messages API makes max_tokens a required, positive field (unlike OpenAI's optional max_tokens) — a request missing it, or sending 0/negative, gets a clean 400 from the real API. DecodeMessagesRequest validated model and messages as required in the same function but silently accepted a missing/invalid max_tokens, omitting it from the IR entirely and letting the upstream model run with no cap at all — diverging from real Anthropic API behavior and masking what should be a client-side bug behind a silent gateway default.

Fixed by validating max_tokens <= 0 as a 400 alongside the existing model/messages checks.

• Test: TestDecodeRequiresMaxTokens (new) — missing, 0, and negative max_tokens all rejected
• Surgical-revert proof: validation forced to a no-op → all 3 cases failed with "want a max_tokens error, got ". Fix restored, test passes.

Shipped via #140, chart bump #141.

Next up: Protocol translation's finding #4 (temperature-scale documentation gap — Anthropic 0–1 vs OpenAI 0–2, passed through unchanged and undocumented; likely a docs/translation.md note rather than a full fix-ship cycle), then Admin API's Minor findings.

Full Changelog: v0.1.68...v0.1.69

v0.1.68

Protocol translation — 2 of 62 confirmed findings fixed

n accepts semantic 1.0, not just literal "1"

DecodeChatRequest validated the OpenAI n field by comparing raw JSON bytes against the literal string "1". A semantically-identical single-choice request sent as "n":1.0 has wire bytes "1.0", so it was wrongly rejected. Now decodes n to a *float64 and compares the numeric value.

• Test: TestDecodeChatRequestAcceptsFloatOne, TestDecodeChatRequestRejectsNonNumericN (internal/openai)
• Surgical-revert proof: byte-comparison check reinstated → TestDecodeChatRequestAcceptsFloatOne failed ("n:1.0 must pass (semantically n=1), got n is not supported"). Fix restored, test passes.

stream_options.include_usage honored

stream_options was an "owned" wire key (never leaked into Extra), but chatRequestWire had no field to capture it — every streaming client got an unrequested terminal usage chunk regardless of preference. Now threaded through llm.ChatRequest.IncludeStreamUsage into openaiSink, gating the usage chunk; defaults to false when stream_options is omitted, matching OpenAI's documented default. This gateway's own billing (runStream) is unaffected — it captures usage from the upstream call directly, independent of what the sink forwards to the client.

• Test: TestDecodeChatRequestStreamOptions (internal/openai), TestOpenaiSinkSuppressesUsageChunkByDefault, TestOpenaiSinkForwardsUsageChunkWhenRequested (internal/httpapi)
• Surgical-revert proof: decode forced to always return false → TestDecodeChatRequestStreamOptions failed ("IncludeStreamUsage = false, want true"). Fix restored, test passes.

Shipped via #138, chart bump #139.

Next up: finding #3 (Anthropic max_tokens not enforced as required/positive) and #4 (temperature-scale documentation gap), then Admin API's Minor findings.

Full Changelog: v0.1.67...v0.1.68

v0.1.67 — scanRows logs corrupt JSON instead of discarding it

DLP/capture-M4 — silently discarded JSON corruption in admin capture views

scanRows is shared by every admin-facing capture view (List, Get, ReviewQueue) and silently discarded any json.Unmarshal failure on all three JSON columns (detected, secondpass_labels, gold_labels). Its sibling PendingForSecondPass handles the identical corruption on the same detected column by at least logging a warning. Since these columns are jsonb (Postgres validates JSON syntax at write time), the realistic failure mode isn't invalid syntax — it's valid JSON in the wrong shape for []dlp.Finding (a stray object instead of an array, from a future encoding bug or a hand-edited row). Before this fix, that would render as a silently-empty finding/label list in the admin UI with zero diagnostic trail.

Fix: scanRows now logs the same "capture: corrupt <column> JSON" warning PendingForSecondPass already does, naming the column and the row id, for all three columns.

Test plan:

• TestScanRowsLogsCorruptJSON (real Postgres): seeds a row, directly UPDATEs secondpass_labels to a syntactically-valid-but-wrong-shape JSON value, calls the real Get, confirms it still succeeds and that a warning naming both the corruption and the row's id was logged
• Verified load-bearing via surgical revert — genuinely failed (no log output); reverted before committing
• Full gate green: gofmt, go build, go vet, full test suite

PRs: #135 (fix), #137 (chart bump — skips 0.1.66, already claimed by another in-flight PR on this repo)

Thirty-ninth of 62 confirmed findings — closes DLP/capture's last Minor finding. DLP/capture subsystem is now fully closed: every Critical, Important, and Minor finding fixed. Next up: re-deriving Protocol translation's uncatalogued Minor findings from current code.

🤖 Generated with Claude Code

v0.1.65 — drain response bodies before close

DLP/capture-M3 + Limits-M1 — response bodies weren't drained, blocking connection reuse

One underlying bug class, two independent occurrences, fixed together: both internal/dlp.ModelScan (on a non-2xx sidecar response) and internal/webhook's deliver (on every response, success or failure — it never read the body at all) closed the response body without reading it first. Go's http.Transport cannot safely reuse a connection whose body wasn't read to EOF, so a sidecar/webhook endpoint returning errors (or just responding normally) forced a fresh TCP connection — and over TLS, a fresh handshake — per call instead of pooling it. ModelScan runs on every message the BERT layer is enabled for; deliver runs on every DLP-incident webhook fan-out.

Fix: io.Copy(io.Discard, resp.Body) before Close in both places. webhook's fix is factored into a new doDeliver(hc, req) taking the client as a parameter, so the drain behavior is testable against a plain client — deliver itself still always uses the real SSRF-guarded client.

Test plan:

• TestModelScanDrainsBodyOnNonSuccessStatus, TestDoDeliverDrainsBodyAllowingConnectionReuse: write a large (10MB) response body, make several sequential requests over a keep-alive client, count actual accepted TCP connections via the test server's ConnState hook
• Verified load-bearing via surgical revert on both — both genuinely failed; reverted before committing
• A genuine data race surfaced while writing these tests (caught by -race, not a false alarm): httptest.NewServer starts serving immediately, racing against setting Config.ConnState afterward. Fixed with NewUnstartedServer + manual Start(). Verified clean across 15+ repeated -race runs
• Full gate green: gofmt, go build, go vet, full test suite

PRs: #133 (fix), #134 (chart bump)

Thirty-eighth of 62 confirmed findings — closes DLP/capture's third Minor finding and Limits/pricing's only Minor finding in one PR. Next up: DLP/capture-M4 (capture/store.go's scanRows silently discards JSON-unmarshal errors).

🤖 Generated with Claude Code