OpenAI's /v1/models schema carries no context field, so every client that
wants one reads a vendor dialect, and a client whose dialect is absent does
not ask — it substitutes its own default. GET /v1/models now emits the same
resolved integer under "context_window", "context_length" (the OpenRouter
spelling) and "max_model_len" (the vLLM spelling). All three are omitted
together when the window is unknown, exactly as the single field was.
Measured case: omp 18.1.15's openai-models-list discovery resolves
`max_model_len ?? context_length ?? <bundled catalog> ?? 128000` and never
reads context_window, so a 1048576-context local model reached the client as
128000, which also clamped its maxTokens.
The metadata fetcher already parses all of these dialects on the inbound
side (provider_openai.go), so this makes the outbound direction speak what
the inbound one already understands.
Claude-Session: https://claude.ai/code/session_0168ukk3uwp8PgiuRAzeLPXp
The previous round suppressed every context-caused federation error. A
server that accepted the connection and then never answered therefore
read "unknown" forever whenever federation carried the only traffic,
which is the outage shape this feature exists to surface.
Split the two causes instead. A server still waiting for a fan-out slot
when the budget expires is never dialed, so the semaphore acquire now
selects on the fan-out context and records nothing for it. A server this
gateway did dial records a failure on a deadline, because it had its
chance to answer. Only a caller hanging up stays unattributable.
Document the split rule, correct a stale comment about a second record
site, and state that "unknown" reads two ways rather than three.
Claude-Session: https://claude.ai/code/session_01EebV5jyWvYocByNQ3oss1w
targetHealthLastSweepUnixNano moves to Gateway's first field. It is
read and written only through sync/atomic's 64-bit functions, and
the sync/atomic package guarantees 64-bit alignment on ARM, x86-32,
and 32-bit MIPS only for a struct's first word — Traefik ships armv7
and 386 builds alongside amd64/arm64. Every other field keeps
golangci-lint fieldalignment's pointer-grouped ordering; verified
with the standalone fieldalignment analyzer that this is the only
field out of place and adds no struct-size padding, only a documented
+8 GC-scan-prefix cost.
docs: catches up README's admin `health` object and Target health
"State" bullet to F4 (previous commit, a0c8607) — three ways to
read `state: "unknown"`, and that consecutiveFailures/lastCheck/
lastError/latencyMs/source stay present when a target has been
observed but never succeeded.
Claude-Session: https://claude.ai/code/session_01EebV5jyWvYocByNQ3oss1w
writeTargetMetrics now returns before emitting even
llmgateway_target_healthy's own HELP/TYPE header when neither an
MCP server nor an agent is configured, mirroring
writeStoreHealthMetrics' identical behaviour when Redis is
unconfigured.
Claude-Session: https://claude.ai/code/session_01EebV5jyWvYocByNQ3oss1w
mcpFederatedToolsList's fan-out and mcpFederatedToolsCall both
derive their outbound context from r.Context(). A client cancel
(context.Canceled) or the shared fan-out deadline expiring
(context.DeadlineExceeded) is not evidence about a server's own
health, so neither is recorded any more, mirroring
recordTargetProxyHealth's own client-cancel rule. A server that is
genuinely hung is still caught by the active probe sweep or its own
per-server proxy health.
Claude-Session: https://claude.ai/code/session_01EebV5jyWvYocByNQ3oss1w
Add the targetHealth config row and a TargetHealthConfig reference
section, a Target health section under MCP and A2A describing the
passive recording rules and the active probe rules per target kind,
the health field on GET /admin/api/targets, and the
llmgateway_target_healthy gauge with its aggregation-across-replicas
note.
Claude-Session: https://claude.ai/code/session_01EebV5jyWvYocByNQ3oss1w
The Charts view could break usage down by user, group or total, but not
by model. Per-model data existed only as day-window attempt and failure
counts, which measure availability rather than delivered usage.
A served model now accumulates the same four metrics a user or group
does. The scope kind is "model" and its id is the canonical
"provider/model", so a request that fails over counts against the
provider that answered it, and one bare id served by two providers ranks
as two rows.
The request counter is written after the response, in the batch that
already writes tokens and cost. That is the first point at which the
serving model is known, so no extra round trip is needed and admission
never double counts.
GET /admin/api/usage/models ranks models for one metric and window. It
returns only models with non-zero usage, by operator requirement: the
catalog runs to hundreds of models and a padded ranking is unreadable.
The read stays one storeGetMulti over the catalog.
The Charts view gains a Models tab with its own metric picker, and the
scope picker gains the models that have traffic.
The README claimed discovery runs again on discoveryInterval "in the
background", which reads as a periodic timer. There is none. maybeRefresh
is called from ServeHTTP only, so the interval is checked per incoming
request and a refresh is spawned when it has elapsed.
That distinction is what an operator actually needs, because it has three
consequences the old wording hid: an idle gateway never refreshes at all,
a newly published upstream model takes up to discoveryInterval to appear,
and there is no manual trigger since the admin API is read-only.
The plugin runs inside the Traefik process, so declining to start a
periodic goroutine is deliberate rather than an oversight. Documented as
behaviour, with the option of lowering discoveryInterval on the providers
whose catalogues actually change. Also corrected the two provider config
table rows that carried the same implication. Documentation only.
F2: the idle timer resets on DECODED body bytes, after net/http's own
gzip decompression. A gzip'd response whose compressed bytes arrive
steadily but whose deflate stream yields nothing for the whole window
still gets aborted. The previous wording claimed a slow-but-progressing
body is always unaffected; adds a qualifying paragraph explaining the
gzip case and why fixing it (measuring wire bytes) is out of scope at
the five-minute default.
F3: reworded the "silence ends a request, never elapsed time" framing.
A streaming forwarder reads a chunk, writes it to the client, then
reads again; the write is not watched, so time blocked on a slow
client counts against the same idle window as a silent provider.
Aborting is still correct, but the log line no longer claims the
provider specifically caused it.
F5: documents that neither timeout is ever retried, and why: a
ResponseHeaderTimeout failure wraps context.DeadlineExceeded, which
Retry's own transient classification excludes; a mid-body watchdog
timeout fires after the retry mechanism has already returned.
F8: softened "Every upstream request this gateway makes" to the
routes actually covered, and added a "Federated MCP" note: the
federated /mcp endpoint's two direct calls are not wrapped by the
idle-progress watchdog, but are not unbounded either — they still get
ResponseHeaderTimeout from the shared client, plus their own 20s/120s/
5s context.WithTimeout budgets. Worst case is a bounded stall, not a
forever one.
Three blocking fixes from a second round of adversarial verification,
plus two should-fix items bundled in since they touch the same code.
storeDown detection covered only the fail-CLOSED path. failOpen
defaults to true (newConfiguredLimiter), so on the shipped default a
broken store made storeGetMulti return ok=true after silently reading
the empty in-process fallback, bypassing every storeDown guard in
metrics.go. Add limiter.configuredStoreDown, checked alongside ok in
currentUsage and providerUsage, so a fail-open read served from the
fallback is treated the same as a fail-closed refusal. The pinning
test previously hand-set failOpen=false, which meant it only ever
exercised the case that was already correct; it now leaves failOpen at
its true default.
The escaping rewrite from the previous round achieved injectivity by
copying an invalid UTF-8 byte through raw, which can make the output
itself invalid UTF-8. Measured against a real Prometheus scrape parser
this round: invalid UTF-8 in a label value is fatal, the scrape target
goes down, which is worse than the duplicate series the previous round
was fixing (measured non-fatal, dropped and logged, target stays up).
Replace an invalid byte with a deterministic \xHH marker instead,
proven injective and always valid UTF-8 by an exhaustive test over
every 1- and 2-byte input, not merely spot-checked. Also adds the fast
path a benchmark showed the byte-oriented rewrite needed: unescaped,
already-valid-UTF-8 input, the common case, returns unchanged with
zero allocations instead of paying a full copy.
llmgateway_rate_limit_rejections_total's HELP text claimed it "resets
only on restart", which stopped being true once the previous round
added generation-rotation bounding and reload-triggered eviction: a
scope resets when its user is removed, and every tracked scope resets
together in bulk the instant the tracked-scope cap is exceeded. The
HELP text and README now say so.
Adds llmgateway_limit_store_up, a per-replica gauge for this process's
own connection health to the configured limit store: with the store
flapping faster than its fail latch, every store-backed family can go
silent scrape after scrape with nothing on the endpoint itself saying
why. Also adds a test pinning that Gateway.pruneRejectionsAfterReload
runs only when authStore.maybeReload reports a real reload, never on
every request.
Fix one must-fix and one should-fix finding from re-verification of
d45125e, plus two smaller findings from the same pass.
MUST FIX: the F2 header clear ran on the first candidate attempt too,
so it ran on the default, failover-disabled path and deleted response
headers a different, earlier Traefik middleware had already staged
before this plugin ran (CORS, HSTS, request-id, and similar). Scope the
clear to i > 0, the only case that ever needs it. Add a test proving a
header staged before the gateway runs survives the default path, on
both a cache hit and a cache miss.
SHOULD FIX, coordinator ruling: drop the all-candidates cache pre-pass.
It silently served a different candidate's cached response while the
primary was perfectly healthy, which is failover with no failure. Check
each candidate's own cache entry immediately before that candidate's
own attempt instead. Request-path health, now fixed in the prior round,
is the correct mechanism for skipping a genuinely dead primary: once it
is marked unhealthy it is excluded from the candidate list entirely, so
a later request still reaches the next candidate's cache without the
dead one being dialed. Update the existing cache regression test to
drive enough requests to trip the health breaker before asserting no
repeat call, and add a dedicated test proving a healthy primary is never
bypassed for another candidate's cached response.
Guard the request-health record call with the enabled check. That state
is never read on the disabled path, so recording it there was dead work
plus a new contention point on requestHealthTracker's tracker-wide mutex
on every metered request, disabled or not. Add a test asserting the
tracker's internal state stays empty across the disabled path.
Add a test proving the 404 loud-log line is not rate-limited: drive two
404 failovers through the same Gateway in one request and assert both
lines appear, since a version that wrapped the 404 line in the same
rate limiter as the generic line would still pass every other test
(each of which only ever drives one 404 event against a fresh gate).
Document, but do not fix, that the cost guard cannot see a model
declared free via modelMeta: it resolves pricing only through the same
source real billing uses (pricing and the built-in table), which is
correct and consistent, but means an operator who declares a model free
the modelMeta way loses it as a failover candidate without realizing
why. README now says to use pricing instead for that case.
Every fix is proven by a dedicated regression test, and each was
verified by reverting the fix in place and confirming the corresponding
test fails before restoring it.
Adds a "Request timeout" section explaining the progress-based design,
the two mechanisms, the global/per-provider override rule, and error
classification. States plainly that this changes behavior for every
deployment, since no timeout existed before.
Adds requestTimeout rows to the Config and ProviderConfig reference
tables.
Flip FailoverConfig.Enabled to default false. A version upgrade with no
config change must preserve prior behaviour, and failover changes which
provider serves a request. Update tests, README, and the yaegi-check
probe to opt in explicitly.
Add a mandatory cost guard. A candidate is skipped unless its input AND
output cost per token are each no greater than the primary candidate's,
resolved through the same canonical-then-bare pricing path the real
billing code uses. Unknown pricing is never treated as free: a known
primary against an unknown candidate skips, an unknown primary against a
known non-zero candidate skips, everything else that is equal is
allowed. Not configurable.
Fix four blocking findings from adversarial review of commit 5d62479.
F1: openai-type adapters mutate the request map in place to force
stream usage reporting. Reusing one map across candidates let a later
candidate read back an earlier candidate's own injection and leak a
usage-only SSE chunk to a client that never asked for it. Every
candidate now attempts against a fresh copy of the original request.
F2: a streaming candidate that fails before writing anything still
stages SSE-only response headers. Those headers now get cleared before
every candidate attempt, so they cannot survive into a later
candidate's plain JSON response.
F3: the request-health gate used the retry-shaped isTransient
classification, which treats a non-429 4xx as a successful outcome. A
provider with a dead API key was never skipped and was retried forever.
The gate now reuses failoverEligible's own classification instead.
F4: a shared request context that an earlier candidate exhausted was
attributed to whichever candidate happened to run next, opening a
healthy provider's breaker for a timeout that was not its fault. A
candidate is no longer attempted once the shared context is already
done; the loop stops and reports the last real error instead.
Also fix three should-fix findings from the same review.
F5: a failure arriving while the request-health breaker is already open
but not yet past its cooldown window is now a no-op, instead of
resetting backoff and sliding the open window forward on every such
failure.
F6: the request-health gate now records at most one outcome per
candidate per logical request, not one per same-provider retry attempt,
so retry.attempts alone can no longer open the breaker on a single
request.
F7: every candidate's cache entry is now checked before any candidate
is attempted, so a cached failover response no longer costs a round
trip to a still-broken primary first.
F10: the generic "failing over" log line is now rate-limited to once
per 30 seconds during a sustained outage, reusing the existing logGate
type. The 404 loud-log line stays unthrottled, as ruled.
F9 (optional): skip building the failover candidate pool entirely when
failover is disabled.
Correct an overstated test comment on
TestHandleChat_Failover_DiscoveryUnhealthy_StillTried: it proves alpha
is still tried, not that ordering places it second.
Document, but do not fix here, that this plugin's HTTP client sets no
per-attempt timeout, so a provider that accepts a connection and then
hangs will never trigger failover. This is a pre-existing property of
the shared HTTP client, not specific to failover.
Deferred: F8 (surfacing request-health state on the admin dashboard)
touches admin.go, which a sibling branch is also editing concurrently;
left for a follow-up to avoid a merge conflict in a file this change
does not otherwise need.
Every fix above is proven by a dedicated regression test, and each was
verified by reverting the fix in place and confirming the corresponding
test fails before restoring it.
The metrics feature shipped in code only: no README section, no
metrics block in .traefik.yml testData, nothing under examples/. The
Go doc comments were the entire manual for a human operator or a
config-generating assistant.
Add a README Metrics section covering the served path, both auth
modes (admin bearer token, optional CIDR allowlist), the CIDR-sizing
warning with the live-reproduced SNAT failure mode, the modelLabel
cardinality trade-off, the exposed metric families, the cross-replica
aggregation contract, label escaping, and store-outage behaviour. Add
a MetricsConfig reference table matching the existing AdminConfig
one, and a metrics row in the top-level Config table.
Add metrics: {enabled: true} to .traefik.yml's testData, the
catalog-facing smoke-test config, deliberately without allowedCIDRs
so a copy-pasted example never skips the README's own sizing
guidance.
Add a metrics block to the Kubernetes Middleware example and a
VMServiceScrape resource: the target cluster runs VictoriaMetrics, not
Prometheus, so the CRD differs even though the exposition format does
not. Verified the VMServiceScrape shape (apiVersion, selector,
endpoints, path) against the victoriametrics/operator documentation
rather than assuming it. Notes that /metrics is served by Traefik
itself, not by any backend service, since this middleware intercepts
the path before a backend is ever reached.
Add failover.go with the full candidate pipeline. resolveWithCandidates
resolves a request exactly like resolve does, then adds every other
provider sharing the identical bare model id. orderedFailoverCandidates
applies a hard skip for request-path-unhealthy providers, deprioritizes
discovery-unhealthy providers without dropping them, fails open when
every candidate reads unhealthy, and caps the list at maxAttempts.
Add requestHealthTracker, a separate per-pod request-path health signal
fed by the same isTransient/isDeadlineExceeded classification
recordProviderAttempt already uses. It never widens the existing
discovery circuit breaker, per that breaker's own documented scope.
Rewrite runMeteredCall (routes_unified.go) to loop over candidates
instead of resolving once. Each attempt rebinds the attempt recorder to
its own provider, recomputes the cache key before the response is
cached, and re-injects the alias echo key an adapter may have deleted on
a prior failed attempt. Failover never runs once any byte has reached
the client, since statusTrackingWriter already tracks that state.
A failure falls through on everything except HTTP 400, matching the
brief exactly. A 404 failover logs loudly, naming both providers and the
model, since it usually signals a real configuration mistake.
Add FailoverConfig (Config.Failover): an Enabled pointer defaulting to
true, and a MaxAttempts bound between 1 and 10, defaulting to 3. A
single-provider deployment, or one with no overlapping bare model ids,
behaves exactly as before regardless of this setting.
Add a yaegi-check probe (tools/yaegi-check/main.go) driving two
providers under the real interpreter, one failing and one serving,
proving the candidate loop runs correctly interpreted.
Update README.md with a Failover section and correct the Retry
section's stale claim that no cross-provider failover exists.
Media routes (images, audio speech, audio transcriptions) do not get
failover in this change. Each has its own request-shape handling that
needs its own scoped follow-up.
Replace the alias-echo rewrite on the Anthropic passthrough branch
with a byte-preserving field replacement. The prior version decoded
the response into a Go map and re-marshaled it. This silently
corrupted a JSON integer beyond float64's exact range. It also
rewrote 1.0 to 1, HTML-escaped angle brackets and ampersands, and
re-sorted every key. A tool_use block's "input" is arbitrary
model-generated JSON. A long numeric id inside tool arguments could
be silently corrupted on a route documented as passthrough.
rewriteModelField now walks the response with a JSON decoder and
copies every field but "model" as raw bytes.
Add a test with a 20-digit integer, a 1.0 float, and a literal "<"
character inside tool_use input. It asserts the response is
byte-identical to the source apart from the model field.
Add a negative test for the anthropic-beta header allowlist. It
asserts Authorization and Cookie never reach the upstream. It also
asserts the upstream sees exactly one x-api-key value: the gateway's
own key, never the client's. A single Get() call could miss a second,
leaked value, since Set() and Add() interact in a way a naive check
would not catch. The test checks the Values() length explicitly.
Add a test case for the native passthrough route's prompt-cache
billing. It fails if cache_creation_input_tokens or
cache_read_input_tokens drop out of the billed prompt count. This
closes a production billing change that previously carried no
coverage at all.
Decouple response delivery from usage extraction on the passthrough
branch. A response body read failure, or a malformed usage field, no
longer both discard the response and misreport the failure as an
upstream connection error. A read failure still returns an error, now
correctly classified as a translation failure. A malformed usage
field no longer blocks the response at all: the client still gets
it, and the existing zero-usage estimate takes over for accounting.
Return the adapter's own usage value on a chatCompletion error in the
translated path, instead of discarding it. This has no effect while
streaming stays rejected up front. It prevents the same critical bug
from resurfacing once streaming support lands.
Normalize tool_result content to a plain string before applying the
error prefix. The prior version only prefixed a Go string. Anthropic's
array content form, which Claude Code sends, lost the is_error signal
entirely. Absent content now becomes an empty string instead of the
JSON null OpenAI rejects.
Treat "stream": null the same as an absent key. The prior fix for a
non-boolean stream value also rejected null. An SDK that serializes an
unset optional field as null must still get an ordinary non-streaming
response.
Drop a yaegi-check assertion that could not fail. The fixture
configures no cache, so the header it checked was never set regardless
of what the code under test did. A mutation pass confirmed this: it
deleted the line the assertion was meant to guard, and make
yaegi-check still reported OK.
Document that this route now intercepts POST /v1/messages
unconditionally, even under passthroughUnknown, where it previously
fell through to the next handler. Add the note to both the new README
section and Known limitations. Correct the passthrough table's claim
about response bytes to describe the actual guarantee.
The Provider health section claimed a broken provider stops being
re-probed "every single interval forever", while its own third bullet
said the opposite: under stock config the interval remained the
binding constraint and nothing was actually suppressed. Rewrite the
opening paragraph and the bullet to state the real relationship
plainly: the backoff window only ever reduces re-probe frequency once
it exceeds discoveryInterval, and the shipped defaults are chosen so
that holds.
Update every remaining reference to the old 30m maxOpenDuration default
to the new 6h value, in the BreakerConfig reference table and the
Provider health section, and explain what a misconfigured relationship
between discoveryInterval and maxOpenDuration does.
Add BreakerConfig to the full configuration reference, alongside
RetryConfig and CacheConfig, and add a breaker row to the top-level
Config table. Add a new Provider health section explaining the three
states, the failure classification, the neutral treatment of a
canceled context, and the scope limitation: the signal reflects the
discovery endpoint only and must not be the sole reason to refuse
routing to a provider.
Also document the new healthState and openUntil fields GET
/admin/api/overview returns per provider, in the same sentence that
already lists discoveryEnabled, modelRates, and modelMeta.
Preserve usage when response translation fails after a successful,
billed upstream call. The route previously discarded already-billed
tokens on that path. A client could induce unlimited free usage
against any budget via a truncated tool-call response.
Reject a streaming request with 400 instead of 501. Check it before
the rate limiter runs. A 501 made anthropic-sdk-python retry the
request, since it retries any status of 500 or higher. Each retry
also counted against the caller's rate limit before this fix.
Preserve block order when translating an Anthropic message to
OpenAI's shape. A tool_result block before other content must stay
before it. OpenAI otherwise rejects the translated conversation.
Echo the client's requested model alias in both the passthrough and
the translated response paths. The passthrough branch previously
returned the upstream's own model id instead.
Forward the anthropic-beta header to an anthropic-type upstream
through an explicit allowlist. Never forward Authorization,
x-api-key, Cookie, or hop-by-hop headers. Let the adapter's own
credential injection win regardless.
Bill Anthropic's prompt-cache tokens. anthropicUsagePayload now reads
cache_creation_input_tokens and cache_read_input_tokens too. It folds
all three into the billed prompt count. This applies to this route,
the existing outbound Anthropic path, and the native passthrough
route.
Extract the metered-request pipeline shared by the unified route and
this route into runMeteredCall. Thread an envelope writer through it
so each route answers in its own error shape. This removes the
duplication the original brief warned against. It also lets the
rate-limit and body-admission error paths answer in this route's own
shape.
Treat a non-boolean "stream" value as a streaming request. A string
or number value previously bypassed the streaming check silently.
Emit max_completion_tokens instead of max_tokens for OpenAI's
reasoning models. Those models reject max_tokens with a 400.
Preserve tool_result.is_error by prefixing string content with
"Error: ". OpenAI's tool message carries no error field of its own.
Log a warning when the thinking field is dropped instead of
discarding it silently.
Classify a response-translation failure distinctly in the log. An
operator must never see a genuine translation bug reported as an
upstream connection error.
Document the route in README.md. Cover both auth headers, the
non-streaming limitation, and which providers translate versus pass
through. Add the reviewer's yaegi-check probes to
tools/yaegi-check/main.go, adjusted for the fixes above.
Release builds send one anonymous "plugin loaded" ping per process, so
the project can see roughly how many installs exist and which versions
are in use. The payload is project name, version and timestamp. Nothing
else leaves the pod: no identifiers, no hostname, no configuration, no
provider names, no keys, no request data.
Three things keep it from firing when it should not. Builds from source
carry the devPluginVersion sentinel and stay silent, so developer
checkouts and every test run never phone home. The ping is sent only
after the plugin constructs successfully, so a rejected configuration is
never counted as an install. A sync.Once holds it to one per process,
because Traefik calls New once per route and oss-telemetry does not
deduplicate on the client side.
Operators of a release build opt out with DO_NOT_TRACK=1,
OSS_TELEMETRY_DISABLED=1, or TRAEFIK_LLMGATEWAY_DISABLE_TELEMETRY=1.
README.md documents the full payload and the opt-out; a plugin that
phones home without saying so in its own README would be indefensible.
oss-telemetry is the plugin's first non-stdlib RUNTIME dependency, which
has consequences for the Yaegi gate:
- vendor/ is no longer excluded from yaegi-check's GOPATH copy. It was
excluded on the reasoning that it held only test-only testify and
_test.go files are never interpreted. That reasoning no longer holds:
leaving vendor out would make the interpreted copy fail to resolve an
import the real plugin tree resolves fine, so the gate would stop
representing production.
- yaegi-check now stamps a fake release version into its GOPATH copy so
New's telemetry gate is PASSED rather than short-circuited, and the
vendored package is actually called under the interpreter. Import
resolution alone is not coverage; a cross-package call into interpreted
third-party code is its own risk. Telemetry is suppressed by env vars
set before anything is interpreted, so the gate never emits a real
ping.
- A new assertion reads the version back through GET /admin/api/overview.
Without it a silently failed stamp would leave the dev sentinel in
place and the harness would still print OK while covering nothing.
Verified by disabling the stamp and watching the gate fail.
shouldSendTelemetry is split out from sendStartupTelemetry so the
decision is table-testable without a network, since oss-telemetry's
endpoint is deliberately not overridable from consuming code.
Also brings the version-gated documentation up to date, since the same
README edit touches it. The latest release is v1.0.3, so the pre-1.0
experimental warning has met its own stated condition and is replaced by
a semver stability statement in README.md and dropped from
.traefik.yml's catalog summary. The catalog now serves the released tags
(verified: the v1.0.3 download endpoint returns 200), so the "not yet
published" and "analyzer has not been run" claims are corrected, the
catalog form of the static config is documented as usable, and
examples/kubernetes.yaml pins v1.0.3 instead of v0.1.0.
Round-3, 2026-08-22 review (SHOULD-6): maxInFlightBodyRequests, the new
503, and the unified routes' narrower 4MiB decode cap appeared nowhere
in the README -- a gap on a public Catalog plugin. Also corrected the
now-stale "images/generations and audio/speech share the unified
routes' 10MiB JSON cap" line: the unified chat/embeddings routes moved
to 4MiB while the media routes kept 10MiB, so they no longer share a
cap at all.
Added:
- maxInFlightBodyRequests to the top-level Config reference table.
- A new "Body-admission limit" subsection under Limits and accounting,
covering the semaphore's scope (read+decode only, not the whole
request), the 503+Retry-After behavior, the self-tuning default, the
4MiB unified cap, and the explicit note that native passthrough is not
gated by this at all (streams rather than decodes).
Independent review measured two real gaps in the round-1 model
enforcement and fixed both here, plus documentation and comment
regressions the round introduced.
MUST FIX: peekPassthroughModel's Content-Type gate was an ALLOW-list
("application/json" or skip") and fully client-controlled. The
reviewer sent "Content-Type: text/plain" with {"model":"EXPENSIVE"}
and got a 200 from a group scoped to a cheap model. This commit
replaces the whole gating approach:
1. A group with an empty Models list has nothing for allowsModel to
reject, so the request body is never read at all
(group.hasModelRestriction). This is the overwhelming majority of
traffic, including the operator's own "home" group, and removes
the memory-buffering concern entirely for it.
2. For a group WITH a restricted Models list, the Content-Type gate
is now a SKIP-list of genuinely binary types (multipart/, audio/,
image/, video/, application/octet-stream). Every other
Content-Type, including "text/plain" or none at all, is
inspected. The attacker no longer controls whether the check
runs.
3. The peek reads at most 64KiB (maxModelPeekBytes), restored via
io.MultiReader so the remainder streams straight to the upstream
instead of a second full buffer. A bounded JSON token walk
(scanTopLevelModel) finds a top-level "model" key even when the
rest of the body is truncated past the cap.
4. A restricted group whose peek window never finds "model" is
denied (403), fail closed. This also means a Gemini passthrough
request (its model lives in the URL, never the body) is always
denied for a model-restricted group; documented in
maxModelPeekBytes' own comment, the handler's doc comment, and
the README.
Matcher parity: the round-1 version glued providerName+"/" onto
whatever the body sent unconditionally, so an already-prefixed body
("openai/x") produced "openai/openai/x" and false-403'd a legitimate
client. allowsPassthroughModel now reuses modelRegistry.
splitConfiguredProvider — the same rule resolve/resolveAgainst apply
to a client-given id — to strip a leading segment only when it names
an actually configured provider, before generating candidates.
Also in this commit:
- Restored ~26 lines of doc comments fieldalignment -fix silently
dropped when it reordered the group struct (auth.go: cache,
cacheTTL) and ProviderConfig (llmgateway.go: MetadataPath, and the
Passthrough field's own new comment). Verified nothing else was
reordered by re-running fieldalignment: 0 issues.
- Documented the path.Match footgun a non-empty PassthroughPaths
inherits: "*" does not cross "/", so ["*"] denies almost all real
multi-segment passthrough traffic rather than allowing it. Empty is
the actual allow-all value. Documented on the config field, the
allowsPassthroughPath method, and the handler's own doc comment.
- Renamed the local "model" that shadowed the outer request-model
variable near extractPassthroughUsage's call site to "respModel" —
the new gating already scopes the outer one to a narrower block, so
this was no longer a live shadow, but the rename removes any future
ambiguity between the request's model and the response's.
- README: rewrote the two "Unified vs. passthrough" table cells that
described the OPPOSITE of this feature's actual behavior, added a
"Native passthrough model authorization" section, and added
providers.<name>.passthrough / groups.<name>.passthroughPaths rows
to the config tables.
New tests: the Content-Type-bypass regression (text/plain, absent,
form-urlencoded, all now 403), the matcher-parity case (already-
prefixed body allowed), fail-closed on a binary body for a restricted
group, skip-entirely for an unrestricted group, byte-identical body
forwarding for a body larger than the peek cap and for a
ContentLength<0 (chunked) request, and a scanTopLevelModel unit table
covering truncation before/after the model field, nested values, and
malformed input.
Gate re-run in full after these changes: gofmt, go vet, fieldalignment,
golangci-lint (0 issues), go test -race (ok), make yaegi-check (OK),
make integration (ok, real Traefik + Redis). Re-ran the interpreted
Yaegi harness: RTT/req unchanged at 3.00, cmds/req unchanged at 74.00.
The catalog uses a plain CommonMark renderer, not GitHub's: the
"[!WARNING]" alert syntax printed literally and the mermaid block
rendered as a wall of diagram source. Use a portable bold blockquote and
fold the diagram into <details> so plain renderers collapse it while
GitHub still draws it.
New 'Model metadata' section: the context/cost layering order, the
modelMeta config override example (mirrors the operator's own cluster
snippet), alias inheritance, the ":free" naming convention, the
metadataPath discovery capture (including the loaded-vs-max-context
volatility note — a discovery refresh can legitimately report a
different context window across reloads), the pricing-sync/built-in
table workflow, and the two GET /v1/models extension fields with a
worked example. Config reference tables gain a ModelMetaConfig
subsection and modelMeta/metadataPath rows on Config/ProviderConfig; the
Admin section notes the per-provider and per-alias modelMeta fields on
GET /admin/api/overview.
Re-read the provider/model success-rate paragraph against the FIXED
deadline behavior (v0.22 review round, round 2) per instruction — the
claim itself ("a real deadline counts as a failure") was already
accurate and needed no correction; the multi-wrap bug it was allegedly
describing never surfaced through the README, since the README
describes intended behavior, not the wrap-depth implementation detail
that broke it. Added: FOLDED-1's bound (64 in flight per process,
lossy-under-pressure by design) on the fire-and-forget write this
paragraph already described, and an explicit note that enforcement never
goes through this path.
SHOULD-3 (v0.22 review round). Documents:
- The new llmgw:prov:*/llmgw:provmodel:* counter key families under
Limits and accounting: format, minute/day-only windows and their TTLs,
and the SHOULD-1 semantics — a timed-out attempt counts as a failure, a
client-canceled one does not, both on top of retry.go's own transient
classification, reused not forked.
- Native passthrough's provider-level-only accounting limitation (no
resolved model at attempt time).
- The new adminProviderView fields (discoveryEnabled, attemptsDay/
failuresDay/attemptsMinute/failuresMinute, modelRates) under Admin's
GET /admin/api/overview description.
Rewrote the federated POST /mcp bullets to document the review round 2
fix batch: per-backend timeouts (20s tools/list, 120s tools/call),
the bare-first-with-fallback session handshake and exactly what
triggers it (JSON-RPC error or HTTP 4xx, never a network failure or
5xx), the legacy HTTP+SSE-transport exclusion (URL path ending
'/sse') and why no handshake can fix it, the loud 'no MCP server
reachable' error when every attempted server fails, non-2xx/
empty-envelope rejection, the protocol-version allowlist, and the new
405 for a disallowed method on /mcp. Extended the admin-API GET
/admin/api/targets bullet, the total-scope accounting list, and the
rejected-before-quota bullet to include the federated endpoint.
Architecture diagram gains a Federated branch node (render-checked
with mmdc).
New POST /mcp: a single JSON-RPC 2.0 endpoint aggregating every MCP
server the caller's group can reach, for clients built against the
earlier pre-plugin gateway's one aggregated endpoint (pugbot's
mcpclient, agentkit) rather than this plugin's per-server
/mcp/{name}/... proxy.
Unlike the per-server proxy, this cannot be a byte-level reverse
proxy: aggregating N servers' tools/list results, or routing one
tools/call to the single right server, both require actually parsing
the JSON-RPC envelope. The gateway answers initialize/ping locally
and, for tools/list/tools/call, issues its own outbound JSON-RPC
call(s) to the relevant backend server(s) per incoming request — no
session state persisted across separate client requests, matching the
per-server proxy's own complete absence of gateway-side session
tracking. A backend may answer bare JSON or an SSE stream; both are
accepted (parseBackendJSONRPC).
- tools/list fans out concurrently to every allowed server and merges
results, prefixing each tool "<serverName>_<toolName>" — the
underscore convention the earlier agentgateway used and that
pugbot/agentkit already persist in their own tool-id records (e.g.
brave-search_brave_web_search, verified live). A failing/unreachable
server is skipped, not a whole-call failure.
- tools/call resolves the target by LONGEST matching
"<serverName>_" prefix against the caller's allowed servers
(disambiguates a server name that prefixes another), strips it,
forwards, and relays the result/error under the CLIENT's own id. An
unresolvable prefix (unconfigured, or authz-excluded) is a JSON-RPC
invalid-params error, not a 404.
- Reuses Feature B's countTargetRequest for per-target attribution —
every backend server actually contacted increments its own scope.
- notifications/* -> 202 no body; unknown method -> JSON-RPC
method-not-found; malformed body -> JSON-RPC parse error with
id:null (spec-required, not omitted).
No new limits config surface. Tests cover prefix resolution
(table-driven, incl. the ambiguous-prefix case), SSE/JSON backend
parsing, group-restricted aggregation (a denied server is never even
contacted), call routing/prefix-stripping, backend error relay,
unreachable-server handling, unknown-prefix errors, and per-target
counter attribution. README's MCP and A2A section documents the
federation contract and naming convention; the architecture diagram
and the total-scope/rejected-before-quota notes are updated to
include the new route.
Review fix round on the group-access-display feature (Should-Fix + folded
items):
1. TestAdminUsage_GroupAccessLists now asserts at the raw-JSON level, not
just the typed adminUsageResponse struct: decodes rows into
map[string]json.RawMessage and checks the four access-list keys are
ENTIRELY ABSENT on user/total rows and an unrestricted group, PRESENT
with exact values on a restricted group. A typed-struct decode alone
cannot prove omitempty is doing its job -- a present-but-null field
decodes into the same nil []string a genuinely absent key would.
Verified by temporarily stripping the omitempty tags: the new
assertions fail (four keys read back 'null'); the old struct-only
assertions stayed green throughout, confirming the gap the review
flagged.
2. UsageView.vue's Access block now renders MCP servers and Agents too,
same Badge treatment as Providers, with muted 'All MCP servers'/
'All agents' fallbacks -- GroupConfig carries all four lists
(llmgateway.go), so the display now matches the payload instead of
dropping two of the four fields silently.
3. A configured Models entry containing *, ?, or [ (path.Match's own
special characters -- auth.go's matchesGlob) is a pattern, not a
routable id: it now renders as a plain, non-copyable Badge
(font-mono, secondary) instead of the copyable ModelChip, so a caller
can no longer copy a glob pattern into a request's model field
thinking it's a real id. Every Access label (Providers/Models/MCP
servers/Agents) also gets a title attribute (one shared GLOB_HINT
constant) making the glob-match semantics discoverable, since all
four lists are glob-matched, not just Models.
4. groupSummary's providers/models/mcpServers/agents are now independent
copies (cloneStringSlice), not aliases of the live group.* slices
authorization reads on every request -- structurally prevents a
future display-side sort/mutation from ever reordering or corrupting
enforcement data. slices.Clone was tried first and reverted: it broke
make yaegi-check (yaegi's stdlib symbol table does not resolve the
generic slices package's type parameters under interpretation), so
cloneStringSlice uses plain make/copy builtins instead -- verified
yaegi-check passes with this version.
5. README.md's usage endpoint section: fixed the stale 'Overview and
Usage views' reference (now 'Providers', matching the earlier tab
rename) and added one line naming the new providers/models/
mcpServers/agents group-access fields, matching the section's
existing terseness.
6. Renamed the 'unrestricted group carries empty/omitted lists' subtest
to 'empty or omitted' -- the slash broke -run subtest path matching.
7. The Access block now uses the same dl/dt/dd markup as the sibling
stat grid immediately below it, instead of a bespoke div/p structure
-- one convention per panel.
Review fix round (4 Important + 2 folded):
1. Version display: OverviewView.vue was missing the muted version line
the old vanilla-JS page rendered (types/api.ts already declared
overview.version) - added.
2. CSP invariant now guarded twice: webui/generate.mjs's new
assertNoInlineAssets fails the build if dist/index.html contains an
inline <script> (no src= attribute) or any <style> tag - the exact
condition the tightened admin CSP (script-src/style-src 'self', no
'unsafe-inline') depends on. admin_test.go's new
TestAdminIndexHTML_NoInlineAssets re-asserts the identical invariant
server-side, against the decoded adminIndexHTML admin.go actually
serves, so a regression is caught even if admin_assets_gen.go were
regenerated with a modified copy of generate.mjs.
3. webui/components.json's iconLibrary:"lucide"/font:"inter" fields
cannot be fixed in place - checked shadcn-vue's own schema
(PRESET_ICON_LIBRARIES/PRESET_FONTS in the installed package): no
"none"/fontawesome/system choice exists for either, and JSON (unlike
the tsconfig.*.json files touched in the previous commit) has no
comment syntax to warn inline. Documented instead: webui/README.md
gained a loud warning section naming the exact regression (already
hit twice this branch - a re-added Google Fonts import, a re-added
lucide icon import) and the two-step fix to apply after any future
shadcn-vue add/init call.
4. Real-CSP verification: the browser pass earlier in this branch ran
under vite preview, which sends no CSP header at all. New
integration/integration_test.go: TestAdminAssetCSP fetches GET
/admin against the real Traefik+Yaegi pipeline, extracts the asset
URL from its <script src="..."> tag, fetches that asset, and asserts
both responses carry the exact adminCSP string byte-for-byte.
admin.go's serveAdminAsset now also sets Content-Security-Policy
(previously JSON/page routes only) so this holds for a direct
navigation to an asset URL, not just as a same-origin sub-resource of
GET /admin; admin_test.go's TestAdminAsset_Serving updated to match.
5. (folded) UsageChart.vue stacked the "tokens" tab's two series
(tokin/tokout - two separate HTTP requests, not one atomic call) by
positional index. A window rollover landing between the two requests
could leave one series one bucket ahead, mis-stacking that refresh.
Now aligns both series by their bucket key (a sorted union of every
active series' buckets - the fixed-width, zero-padded bucket strings
sort chronologically by construction), with a missing bucket in one
series reading as 0 rather than shifting the whole series.
6. (folded) AuthGate.vue now triggers an immediate dashboard.refresh()
on successful key submit, instead of waiting up to 5s for the next
poll tick - the dashboard no longer sits blank right after auth.
Rebuilt via make admin-ui; admin_assets_gen.go regenerated.
README.md: ## Admin rewritten for the Vue rebuild (webui/, new
GET /admin/assets/{hashedname} route, tightened CSP with no more
'unsafe-inline', usage-charts description) and the stats-exclusion
directive ("Admin traffic is never counted" replaces "Poll counts
against quota"). ## Development gains `make admin-ui` plus a note
that Node is needed only for that one target.
docs/superpowers/specs/2026-08-20-llmgateway-v02-design.md §4: an
amendment block documents the Vue rewrite (new route, tightened CSP,
Charts view) following this doc's established
controller-approved-amendment convention; the Accounting bullet is
amended for the stats-exclusion operator directive, with the original
wording struck through rather than deleted.
1. README's fallback-charting story corrected: the 48h ceiling
(memoryStoreMaxTTL) only binds hour and day, not month. hour charts
its full default 48-bucket span; day shows only its most recent
~2-3 of the 35 possible buckets (its key still sits under the 48h
ceiling). month is different: enforceTTLFor's ~32-day floor wins
over the 48h ceiling there, so the CURRENT (in-progress) month's
bucket charts in full on the fallback — only past months are
unavailable, once their own key expired ~32 days after its own
creation. Aligns with limits.go's clampTTL/enforceTTLFor and the
+5d TestMemoryStore_IncrMulti_MonthCounterSurvivesPast48h... test
from the prior round.
2. limits.go's "5 call sites, all in admin_test.go" attribution
belonged to incrCounter, not incrBy — incrBy itself (memoryStore.
incrBy and redisStore.incrBy together) has 11 direct test call
sites. Fixed at both doc-comment sites (the counterStore interface
doc and memoryStore.incrBy's own doc); the no-production-caller
claim is unchanged and correct for both.
3. Appended the missing "Fix round 2" section to
.superpowers/sdd/usage-history-report.md, documenting the
enforcement-floor design (counterIncr.enforceTTL, enforceTTLFor,
newCounterIncr, clampTTL's ceiling-vs-floor precedence) that this
report's round-1 section never described. Corrected that same
section's stale "incrMulti's loop inherits the clamp for free,
since it calls incrBy" sentence — round 2 replaced that shape:
incrMulti now clamps its own entries via the shared applyIncr
primitive and no longer calls incrBy at all.
Gate: go test ./... -count=1, gofmt, vet — all green. Docs and
comments only, no behavior change.
Re-review round on 5cc6814 found round 1's 48h fallback clamp
defective: it broke TTL > window-length for the MONTH window on
memoryStore, turning tokensPerMonth/costPerMonthUSD into a rolling
~48h budget (fail-open) whenever Redis is absent or down.
1. counterIncr gains enforceTTL time.Duration — the minimum retention
window's own enforcement correctness requires (enforceTTLFor:
min->2m, hour->2h, day->25h, month->32*24h, matching each window's
pre-history-bump TTL). newCounterIncr builds every checkAndCount/
account entry through one helper so a (window, ttl) pair can never
reach incrMulti without its matching floor. memoryStore's clampTTL
now takes (ttl, enforceTTL) and applies max(min(ttl,
memoryStoreMaxTTL), enforceTTL): the 48h ceiling still bounds
min/hour/day (unaffected/still-capped as before), but the floor
wins for month, landing it back at ~32 days (1-2 live keys per
scope/metric — negligible against the day-key savings that
motivated the ceiling). redisStore ignores enforceTTL; incrBy's own
single-key path passes enforceTTL=0 (ceiling only, unchanged
behavior — it has no window to derive a floor from and has no
production caller since the batching, see (3) below).
2. limits.go's TTL-invariant comment restated precisely: the consts
still satisfy TTL > window-length exactly as applied to Redis; that
did not automatically extend to memoryStore's own effective TTL
after its ceiling, which is what round 1 broke and round 2 fixes.
README: the Limits and accounting "Fallback" bullet now says
enforcement stays correct on the fallback at every window
(including tokensPerMonth/costPerMonthUSD), and that the 48h
ceiling applies to usage-history CHARTING retention only.
LimitsConfig's window table (tokensPerDay/tokensPerMonth rows)
cross-links it. The "resets on a rebuild" bullet's stale
limiter.storeIncrBy reference is corrected to storeIncrMulti and
now distinguishes rebuild-loss from the fallback's own per-window
TTL expiry.
3. counterStore.incrBy, limiter.storeIncrBy, and limiter.incrCounter's
doc comments now state plainly they have no production caller since
the batching (perf review, 2026-08-21) and are kept for direct
counter seeding in tests — 5 call sites, all in admin_test.go's
TestAdminUsage_MathAgainstSeededCounters, correcting the prior fix
report's "dozens of tests" overclaim to the actual count.
Tests: TestClampTTL_ExactMathTable pins the ceiling-vs-floor
precedence for every window's real (ttl, enforceTTL) pair.
TestEnforceTTLFor_MatchesTheOriginalPreHistoryBumpValues pins the four
floor values directly. TestMemoryStore_IncrMulti_
MonthCounterSurvivesPast48hAndStillEnforces drives a real
TokensPerMonth budget through account/checkAndCount on the fallback
5 days in and confirms both survival and enforcement.
TestMemoryStore_IncrMulti_DayCounterStillClampsAt48h confirms day
still resets at the 48h ceiling (its own floor sits below it).
Gate: go test ./... -count=1, -race, gofmt, vet, make yaegi-check —
all green.
Review fix round (2 Important + 5 folded) on 08cfdb4/b3fb705.
1. counterStore gains incrMulti(entries []counterIncr) ([]int64, error).
redisStore.incrMulti sends every entry's INCRBY+EXPIRE pair as ONE
pipeline (ttlToSeconds extracted from incrBy, now shared);
memoryStore.incrMulti loops incrBy for interface conformance.
checkAndCount batches every scope's req:min/req:day/req:hour into one
storeIncrMulti call (down from up to 9 round trips for a 3-scope
request) and reads req:min/req:day back from the reply, no extra
GETs. account batches every scope's nonzero tok-in/tok-out/cost
entries into one call (down from up to 27). Violation math and
fail-open/fail-closed/latched semantics are unchanged — only the
round-trip count moves.
2. memoryStoreMaxTTL (48h) caps every TTL memoryStore actually applies,
clamped inside incrBy so incrMulti's loop inherits it for free. The
in-process fallback is continuity, not history: applying the real
hour/day/month TTLs there was growing live-key count ~30x, multiplying
sweepEvery's own documented "167x incrBy cliff" by the same factor.
Sweep cadence is unchanged (still 30s-gated); the clamp shrinks what
there is to sweep instead.
Folded: historyMaxSpan's margin comment corrected (~4 days, not a
month); checkAndCount's godoc names req:hour; the integration
current-hour assertion sums every point instead of reading only the
last one (an hour-rollover race the suite cannot control); a new
exact-boundary test proves tokin+tokout landing exactly on a
TokensPerDay limit still violates; README documents the history route
as unthrottled (admin-gate is its only trust boundary) and the
fallback's 48h retention ceiling.
Tests: countingIncrStore proves exactly one incrMulti call per
checkAndCount and per account, with correct per-scope/metric/window
values and unchanged violation math; a fake-RESP-server test pins
incrMulti's wire format (one pipeline, entry-ordered replies); TTL
clamp tests cover both the ceiling and the pass-through case at the
boundary.
Gates: go test ./... -count=1, -race, gofmt, vet, golangci-lint,
make yaegi-check all green. make integration: every test passes
except the pre-existing TestUsersFileHotReload flake, which is NOT
caused by this change — verified via an isolated worktree at the
prior commit (b3fb705) run under identical conditions (full suite,
same host), which fails identically. This host's Docker Desktop is
currently slow to propagate the bind-mounted users.json change into
the containers; auth.go/users_file.go are untouched by this diff.
Extends Limits and accounting and Admin for the v0.2 data-layer
change (08cfdb4): the tokin/tokout counter split and how a
TokensPerDay/TokensPerMonth limit still reads their sum, the
synthetic total scope, the stats-only hour window, the retention TTL
bump, GET /admin/api/usage/history's query parameters and error
shapes, and the admin usage API's new tokensInPerDay/tokensOutPerDay
fields and total row. One-sentence migration note: old "tok" keys
expire on their existing TTL, nothing deployed.
Micro-fixes from re-review, all text/bound-level, no behavior change to
production logic:
1. settleToBaseline's doc comment (config_hot_reload_test.go) claimed to
guard the propagation window unconditionally. Reworded: it is a real
guard on the fired path (two known rebuilds to wait out), best effort
only on the skip path, where no amount of polling can prove a rebuild
will never eventually fire — waiting is still required there because
the earlier disappear-only check was a guaranteed-zero-wait bug, not
because settling can prove safety on that path.
2. Restored the cleanup settle bound from 20s to 30s, matching the
longest propagation delay actually observed during verification.
3. README's Config hot-reload intro dropped "on every run" and now
states the same fired-vs-skip distinction the test's own Skip message
makes: zero non-200s is blip-free-reload evidence only when the watch
fires within the test's bound; otherwise it shows just that the stack
survived an unreloaded rewrite attempt.
4. auth.go's group.cacheTTL field comment still said a positive value
"overrides it for this group's cache entries only" — reworded to the
corrected write-time/shared semantics: it sets the TTL written when
that group's own request populates an entry, entries are shared
across groups, not scoped by group.
Gate: go build/vet/gofmt clean on both modules; go test ./... -count=1
green.
Review response (5 Important + 3 folded):
1. README's cacheTTL row/bullet implied per-group scoping; cacheTTL sets
the TTL written when a group's own request populates a shared cache
entry, it never scopes the entry itself. Reworded only, cacheKey
unchanged.
2. README's fallback-counter claim ("used only when redis is not
configured") was wrong: limits.go also falls back during a Redis
outage whenever failOpen is true (the default). Reworded against
limiter.storeIncrBy/storeGet.
3. Dropped the unevidenced "in-flight requests finish on the old
instance" claim; replaced with what TestConfigHotReload actually
observes (zero non-200s across the rewrite window, every run).
4. Real bug: cleanup's settle poll checked for the injected alias to
disappear, which trivially no-ops on the skip path (the alias was
never live, so "absent" was already true on the first check) —
exactly the unguarded window that caused the earlier 404 cascade.
Replaced with settleToBaseline: captures the baseline /v1/models id
set up front, then polls unconditionally (both paths) for 3
consecutive matching polls, bounded 20s. Verified against a live run
that actually hit the skip path.
5. The ~20rps probe loop shared aliceKey/"eng" (1000/min) with most of
this suite's other tests; a budget-exhaustion 429 would look identical
to a reload blip. Added a dedicated hotreload group
(requestsPerMinute: 100000) and hotreload-probe user to
dynamic.yml.tmpl; every request TestConfigHotReload issues now uses
that key.
6. examples/docker-compose.yml still shipped providers.file.filename,
contradicting the new guidance; switched to providers.file.directory.
Verified by actually running the demo stack.
7. waitForModelAliasBounded and the new settle poll now use
doJSONNonFatal (returns an error instead of calling t.Fatalf), so a
transient transport error resets that poll's retry state instead of
aborting the whole test.
8. Reworded the skip-path Skip message: zero failures there proves only
that the stack survived an unreloaded rewrite attempt, not that a
successful reload is blip-free. Re-added the old single-file path to
.gitignore alongside the new directory entry.
Gates: go build/vet/gofmt clean on integration/; root go test -race,
gofmt, vet, yaegi-check all clean; make integration run cold twice, both
exit 0, plus a verbose run that landed on the skip path (the exact
scenario item 4 fixes) with the full suite, including
TestUsersFileHotReload, passing immediately after.
README Kubernetes section: what a Middleware CR edit actually does
(Traefik builds a new instance, warm-fills discovery, swaps it in; requests
in flight finish on the old one), what survives a rebuild (Redis-backed
counters and cache — windowKey's key never carries instance identity) vs
what resets (in-memory fallback counters, down-latches, the discovery
cache), why new models via discovery need no config edit, why the users
Secret must be a whole-volume mount (subPath never receives a kubelet
update) and how the plugin's own 5s mtime poll picks up the resulting
symlink swap, and why a file-provider dynamic config needs directory:
rather than filename: in Kubernetes (ConfigMap symlink swap) and under
Docker Desktop (VirtioFS fsnotify) alike.
examples/kubernetes.yaml: annotated the existing (already-correct,
no-subPath) users Secret volume mount explaining why subPath would
silently break Secret updates, so a future edit doesn't 'simplify' it
into one.
GroupConfig gains CacheTTL (json cacheTTL, empty inherits the global
cache.ttl). Parsed and validated at construction (newAuthStore,
groupCacheTTL in cache.go): a malformed duration, a duration <= 0, or
CacheTTL set while the global cache block is not configured is a
constructor error naming the group, mirroring Cache:true's existing
"nothing to inherit from" check.
group carries the resolved time.Duration (0 means inherit).
responseCache.store gains an explicit ttl parameter; routes_unified.go
computes effectiveTTL(g.cache, grp) immediately before each store call,
so setEx's Redis-native EX seconds reflect the group's own override.
Tests: constructor-error table, empty-inherits and valid-override
resolution, effectiveTTL inheritance/override, a scripted-RESP-server
proof that store's SET...EX carries a different value per group, and an
end-to-end Gateway.ServeHTTP test with two groups asserting the actual
wire EX value (30 vs 60) differs per group.
README: GroupConfig.cacheTTL config row and a Caching-section paragraph.
Spec §5 (v0.2): Config.ModelAliases map[string]string, alias-first
resolution precedence ahead of provider-prefix splitting, alias-OR-target
authorization, lazy target resolution with a 404 naming both ids,
/v1/models listing, admin overview alias table (JSON + dashboard), and
construction-time validation (shape, prefix-shadow, explicit-model
collision, chain, empty target).
routes_unified.go/routes_media.go needed no resolution-call-site changes
- both already pass the client's raw requested id through to
registry.resolve, so aliases inherit the __alias echo and multipart
rewrite machinery for free. cache.go needed no changes either - cacheKey
already hashes requestedModel, so two aliases resolving to the same
target already produce distinct cache entries.
Adds registry_test.go validation table + precedence/authz/lazy-404/
listing coverage, a dedicated cache-key split test, chat/embeddings/
images/audio-transcriptions end-to-end tests, an extended admin overview
test, and an integration test covering the openai-type vs anthropic-type
echo asymmetry under real Traefik+Yaegi.
Task 5 re-review fixes: 1 Important + 3 folded.
1. (Important) examples/kubernetes.yaml was construction-broken on
copy-paste: the inline dashboard-admin user resolves env:LLMGW_ADMIN_KEY
at construction time regardless of admin.enabled, but the Helm env
snippet never listed it. Added it there, alongside a comment on the
admin user explaining the env var is required either way. Fixed the
two reversed above/below cross-references (kubernetes.yaml:90/:115)
and reworded the misleading "self-contained" comment on the inline
admin user. Verified with a throwaway round-trip test covering: no env
vars set fails, every documented env var set succeeds (admin.enabled
staying false as committed), and — the actual bug — every var set
EXCEPT LLMGW_ADMIN_KEY still fails even with admin disabled.
2. README: Admin section's "(disabled still 404s)" now points at the
same 404/passthroughUnknown wording the section already uses above it.
Caching section's "block absent entirely is a construction error" now
says "not enabled" — auth.go's actual check is !cfg.Cache.Enabled,
which is identically true whether the block is omitted or present with
enabled: false.
3. integration/mock/main.go: added POST /flaky/reset (handleFlakyReset),
reachable through the "flaky" provider's own native passthrough route,
resetting the atomic hit counter on demand. TestRetryFlakyRecovers now
calls it at the start of the test, removing the make integration-keep
re-run trap the old doc comment warned about instead of fixing.
dynamic.yml.tmpl's stale "test 8" comment now reads "task 5".
4. integration_test.go: added redisKeys (docker compose exec redis-cli
KEYS, mirroring flushRedis's own pattern) and a
TestResponseCache assertion that redis holds at least one
llmgw:cache:* key immediately after the miss request.
go test ./... -count=1, gofmt, vet, make yaegi-check all pass; make
integration exits 0 across two fresh full runs.
README, examples, and a comment update covering v0.2's four features:
- README: new Caching section (key semantics incl. requestedModel +
endpoint, hit/miss header, count-requests-free-tokens, group override,
redis-required) and Admin section (enable flag, admin:true users,
browser key-entry flow, sessionStorage, what's exposed/redacted,
poll-counts-against-quota, unauthenticated-shell). Retry and Image/audio
sections already existed from T3/T4; left as-is. Config reference gains
cache/admin rows at the top level, GroupConfig.cache, UserConfig.admin,
and full CacheConfig/AdminConfig tables. Known limitations gains
images/audio-not-cost-accounted and cache-shares-entries-across-users.
- examples/kubernetes.yaml: adds cache{enabled:true,ttl:5m} (this example
already configures redis) and admin{enabled:false} with a comment on
enabling it, plus an inline admin:true user gated behind that flag.
Round-trips through New() cleanly (verified with a throwaway test).
- examples/docker-compose.yml: adds retry, cache (left disabled — this
minimal demo runs no Redis), and admin (left disabled, with a comment
on enabling it). Round-trips through New() cleanly (verified the same
way).
- admin.go: pluginVersion's doc comment now says to keep it in sync with
the release tag.
lukaszraczylo