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.
Critical: attempts now counts retries performed AFTER the first try
(default 1 -> two tries total), not the total try count. The prior
reading made retry:{enabled:true}'s own documented default perform zero
retries. (*retryPolicy).totalTries() is the single conversion point,
clamping any attempts<1 to 1 so do()'s loop bound can never compute <=0
and skip calling call() entirely (no (nil,nil) return path).
Backoff waits are now ctx-aware: sleepFn func(time.Duration) is replaced
by waitFn func(context.Context, time.Duration) bool, whose production
implementation (defaultWaitFn) selects between time.After(d) and
ctx.Done(). A context canceled mid-wait aborts the remaining retries
immediately instead of blocking out the full backoff.
Also: classify a request-build failure (invalid method/URL) as
non-transient via errRequestBuildFailed, since retrying an identically
malformed request only wastes attempts; drop the drain cap to a
dedicated 64KiB constant instead of the 32MiB response cap; fix a
t.Fatal call from a non-test goroutine in the mid-stream-death test;
drop the misleading omitempty on Config.Retry (a struct value is never
'empty' to encoding/json); document the Retry-After >2s-falls-back-to-
backoff behavior and cover it with a direct table test.
README: add the retry config table, a RetryConfig field reference, and
a Retry section documenting the semantics. examples/kubernetes.yaml
gains a retry block alongside its other optional sections.
Gate: go test -race, gofmt, vet, yaegi-check, golangci-lint (incl a
fieldalignment fix on two new test structs) all clean. 295 subtests
pass, 0 failed. integration/ untouched.
I2 (controller ruling): docs/superpowers/specs/2026-08-19-traefik-llmgateway-design.md
described limits as "user overrides group" / "user limit if set, else
group limit, else unlimited" — stale relative to the actual, already-correct
implementation (buildLimitScopes/checkAndCount evaluate every configured
scope and return the first violation). Amended §2 and §7 to state both
scopes are enforced together and the tighter bound wins.
I4 (manifest hedge): .traefik.yml's groups.default was a bare "{}",
the shape empirically reproduced as a decode failure outside this
repo's own (necessarily simplified) yaegi-check harness. Changed to
"providers: []" — same zero semantics, no longer a bare empty object.
manifest_test.go's JSON mirror and drift-check token updated to match;
make yaegi-check still passes (its YAML-subset parser already handles
an inline "[]" under a nested map key).
M9 (pricing provenance): pricing.go's built-in table comment replaced
"verify before release" with a real source note (each provider's own
published list prices, recorded 2026-08, approximate). README's limits
section gained the matching caveat and points to the "pricing" config
override for billing-grade accuracy.
Doc nits: README's Kubernetes section now says the example manifest
applies cleanly once Traefik's CRDs (Middleware, IngressRoute) are
installed, rather than claiming kubectl apply -f works unconditionally.
README's Security notes section no longer claims "no auth-event logging
at all" (see the paired fix: commit) — it now describes what is logged
and that key material never appears.
This commit and its sibling fix: commit are the final wave closing out
the whole-branch review findings for docs/superpowers/sdd/2026-08-20-
traefik-llmgateway.
Gates: go test ./... -count=1 (-race) green, gofmt -l . empty, go vet
clean, make yaegi-check OK, golangci-lint 0 issues.
Fixes from review, verified against source before rewriting:
- README.md Security notes: "auth events are logged" was false — grepped
auth.go, zero logging calls exist anywhere in it. A lookup miss is a
plain 401 envelope with no log line at all, not "logged without key
material". Also corrected the adjacent providerHTTPError claim: the
struct retains the upstream response body as a field (surfaced
deliberately to the client under error.upstream); only its Error()
string omits the body, and nothing in this codebase currently logs a
providerHTTPError via a bare %v anyway.
- README.md mermaid diagram: the previous version showed a single global
Auth gate ahead of routing. ServeHTTP (llmgateway.go) actually dispatches
on path+method first and calls identify() separately inside each
recognized-route branch; an unrecognized route (passthroughUnknown/404
fallback) never calls identify() at all. Redrawn Route -> per-branch
Auth, with the unauthenticated fallback branching directly off Route.
Also fixed the 401 node's error type label from the nonexistent
"invalid_api_key" to the actual envelope type, "authentication_error"
(llmgateway.go, routes_unified.go). Re-rendered with mmdc before and
after the fix.
- README.md + examples/kubernetes.yaml: narrowed the bare-{} empty-group
claim. Re-verified this session with an isolated, minimal repro
(Traefik file provider only, no docker-compose configs: mechanism
involved) that a bare `default: {}` still fails plugin construction
with the file provider specifically: "expected a map, got 'string'".
Scoped the claim to what is actually verified (file provider, twice)
and explicitly flagged Docker/ECS label providers and the Kubernetes
CRD provider as untested, rather than asserting the file provider is
unaffected or that the CRD provider shares its decode path — the
latter was an unverified inference in the prior text, now hedged.
- examples/kubernetes.yaml: the IngressRoute referenced an undefined
Service (llmgateway-dummy-backend), so the manifest did not actually
apply as advertised. Added the missing Deployment + Service (traefik/
whoami, the same image already exercised end-to-end in
examples/docker-compose.yml this session). Re-validated: 6-document
YAML parses, Middleware's plugin config still round-trips through the
real Config struct and New() (throwaway test, deleted after).
- README.md Known limitations: dropped the standalone path-traversal
bullet — it duplicated content already stated in the "request rejected
before it reaches a provider" bullet immediately above it.
NOTE for the reviewer: the ruling on the bare-{} finding stated "file
provider observed working" and named Docker/ECS label providers as the
affected ones. I re-tested this directly (twice, including a standalone
repro outside the compose demo) and the file provider fails on a bare
{} exactly as originally reported — I could not reproduce "file provider
unaffected" and have not tested the label providers at all. Narrowed the
doc wording to only what I verified rather than asserting either
provider's behaviour without evidence; flagging the discrepancy in case
the review had independent grounds I don't have visibility into.
Gates: go test ./... -count=1, gofmt -l ., go vet ./..., make
yaegi-check — all clean. Also re-ran go test -race, golangci-lint (incl.
gosec), and fieldalignment, matching the standard this task already set;
all clean.
- README.md: what-it-is, quickstart (localPlugins today, catalog form
once published), full config reference table for every Config/
ProviderConfig/GroupConfig/LimitsConfig/UsersConfig/UserConfig/
RedisConfig/ModelPricing/TargetConfig/AgentConfig field, model routing,
limits and accounting semantics, unified vs passthrough differences,
MCP/A2A, security notes, known limitations (streaming buffering under
Yaegi, quota-on-reject nuances, identity-encoding, path-traversal
rejects), Kubernetes deployment notes, and development/catalog-
submission status. Every claim grounded against the actual source read
this session; two draft claims (user-limits "replace" group limits, an
unqualified "go-redis is documented as unsuitable in resp.go/
providers.go") were caught and corrected against the real
buildLimitScopes semantics and an actual grep before committing.
- examples/kubernetes.yaml: Secret (users.json) + Middleware CR +
IngressRoute + a Helm values snippet. The Middleware's plugin config
round-trips through the real Config struct and New() (verified via a
throwaway, since-deleted test), not just YAML-syntax-checked.
- examples/docker-compose.yml: minimal localPlugins-based demo (traefik +
whoami + inline file-provider config via compose's inline configs.content).
Brought up and exercised end-to-end against real OpenAI this session:
auth 401/200, /v1/models, /v1/chat/completions (upstream 401 envelope
with a fake key, proving passthrough+key-injection+wrapping all work),
and the unknown-route 404 envelope. Found and fixed a real bug in the
process: a bare "{}" empty group value fails Traefik's own dynamic-
config decode with "expected a map, got 'string'"; fixed to an
explicit empty list and documented the gotcha in both example files and
the README.
- LICENSE: MIT, copyright 2026 Lukasz Raczylo.
lukaszraczylo