Repository navigation
mcpx: /v1/chat/completions panics on schema-built settings (proxy.* paths undeclared) #2726
Description
Found while dispositioning Codex findings on PR #2711 (task 3271). Pre-existing on main, unrelated to #1975.
Claim: handleChatCompletions (pkgs/mcpx/internal/daemon/routes_proxy.go) panics with settings: no such setting proxy.maxTools whenever the daemon has a non-nil settings set built from the registry schema.
Evidence:
- The handler reads three undeclared paths:
s.set.Int("proxy.maxTools")(:63),s.set.String("proxy.upstreamUrl")(:137),s.set.String("proxy.apiKey")(:148). All three lines pre-date mcpx: optional bearer token for /v1 and /mcp #1975 (untouched since migration commit a03494f). - No
proxy.*path is declared anywhere underpkgs/mcpx/internal/settings/(checked everyPath:declaration; onlytransport.*,upstream.*,retention.*, etc. exist). - Typed readers panic on unknown paths by design (
internal/settings/value.go:raw()panicssettings: no such setting). - Reproduced: a
settings.New(settings.Registry())set +handleChatCompletionspanics atproxy.maxTools(observed in a new test for mcpx: optional bearer-token auth for /v1 and /mcp (#1975) #2711, which works around it viaWithOverrides). - Every production set construction found (
daemon/server.go:212,cli/*) usessettings.Registry(), so a real daemon serving/v1/chat/completionsshould hit this. Existing unit tests dodge it by using&Server{reg: reg}with a nil set.
Suggested fix (triage to confirm production impact first): declare the proxy.* family in the settings registry (with env bindings, matching the OPENAI_* fallbacks), or make the handler tolerant of undeclared paths. Not fixed in #2711 — out of scope there.
Acceptance criteria
- Production impact is confirmed or refuted with evidence: a daemon built from the registry schema serving
POST /v1/chat/completionseither reproduces thesettings: no such setting proxy.maxToolspanic or is shown not to reach the undeclared reads. - The fix lands: the
proxy.*settings family (proxy.maxTools,proxy.upstreamUrl,proxy.apiKey) is declared in the settings registry with env bindings matching theOPENAI_*fallbacks, or the handler no longer panics on undeclared paths — with the choice justified on this issue. - A regression test exercises
handleChatCompletionswith a schema-built settings set (not a nil set) and passes. - Existing mcpx settings/daemon tests still pass in CI.
Metadata
Metadata
Assignees
Labels
Package/app: mcpx (pkgs/ or apps/ in this repo)Small: expected under 2 hours of workSomething isn't working (prefixed form of the classic bug label)Intake review done: acceptance criteria, links, and normalized labels addedImportant and time-sensitiveFiled by the new-issue-creator jobQueue packet kind: verify, reproduce, and route this item
Activity
dezren39 commentedon Oct 30, 2026
Intake review
🤖 Agent note · handed off
Related: PR #2711 / issue #1975 (where this was found while dispositioning Codex findings — explicitly out of scope there, pre-existing since migration commit a03494f). Also in the same family as the settings-registry failures in #2716 / #2697 (undeclared/missing settings paths on main), though those are test failures, not this panic.
Still live? Yes — the evidence is file:line on current main (
routes_proxy.go:63,137,148reading undeclaredproxy.*paths; noproxy.*declaration underinternal/settings/; typed readers panic by design atvalue.go), plus a reproduction with a schema-built set. No merged PR claims this fix.Notes / risks: Every production set construction found uses
settings.Registry(), so the route is very likely broken for real daemons while unit tests (nil set) stay green — a coverage-shape problem as much as a missing declaration. If triage instead makes the handler tolerant of undeclared paths, that weakens the registry's panic-by-design contract for every other caller; declaring the family is the narrower fix.Suggested approach: Confirm production reachability first (one schema-built daemon request), then declare
proxy.*with env bindings matching the existingOPENAI_*fallbacks, and add the handler test with a real registry-built set.Labels normalised this pass: priority/high (a public daemon route panics on the production settings construction), effort/s, status/needs-triage, meta/enriched (kind/bug and app/mcpx were already present). An
## Acceptance criteriasection was appended to the body (original preserved verbatim above it).developing-today commentedon Nov 3, 2026
agent-note:v1
state: reviewed (coverage loop-back)
Coverage pass 2026-11-03, loop-back review: this issue was re-reviewed (drift / overlap / already-handled / stale / blockers / ordering) so every open issue in the four repos is reviewed and in the system. Verdict: still live; in the system as task 7352 (triage, queued, priority 50) in repo-health-code-pilot. Labels audited and synced. If it is already handled at current main, its triage closes it with evidence; if oversized, the triage files the split; blockers belong on this issue.
Found via: coverage loop-back 2026-11-03
dezren39 commentedon Nov 3, 2026
Triage findings — task 7352 (lane code-g)
Verdict: production impact CONFIRMED for the pre-fix code, but the fix has already landed on main — this issue is already done.
Verified fresh against
origin/main@6aeb038a0cd64b5057912edac71d4c2744a8b5d6(2026-11-03 17:39 -0600):proxy.maxTools/proxy.upstreamUrl/proxy.apiKeythrough the typed readers (Set.Int/Set.String), whoseraw()panicssettings: no such setting <path>on undeclared paths (pkgs/mcpx/internal/settings/value.go), none of the three paths is declared in the registry, and production construction builds the set from the schema (pkgs/mcpx/internal/daemon/server.go:settings.New(settings.Registry())).fix(mcpx): bound chat proxy upstream calls, for mcpx(daemon): chat-completions proxy has no upstream timeout and buffers the whole upstream response in memory — proxyToUpstream uses http.DefaultClient and an uncapped TeeReader buffer #2114), commita98c6941f3e5fdcf88670284d4b353e9599f774bon main (2026-11-03 15:41 -0600) — after this issue was filed (2026-10-30) and after its last review note (2026-11-03 16:47 UTC).pkgs/mcpx/internal/daemon/routes_proxy.gonow reads all three via the tolerants.set.Value(path)with anokcheck (:92-93, :168-169, :181-182), falling back to defaults /OPENAI_*env vars exactly as before. The one remaining typed read in the handler,proxy.timeout(:280-281), IS declared (pkgs/mcpx/internal/settings/proxy.go, added by the same commit).TestProxyUpstreamTimeoutReturnsBadGateway(routes_proxy_test.go) driveshandleChatCompletionswith a schema-built set (settings.New(settings.Registry()), onlyproxy.timeoutoverridden) — the three undeclared reads take theok=falsepath with no panic and the handler returns 502 as asserted. PR mcpx(daemon): bound chat proxy upstream calls (#2114) #3380 also addedpkgs/mcpx/internal/e2e/proxy_test.goandsettings/proxy_test.go.Residual (cosmetic, not blocking): the NOTE comment in
routes_proxy_test.go(~:109) still says a schema-built set "panics on the typed readers in handleChatCompletions — filed as a follow-up"; that text is stale since a98c694 and can be corrected whenever that test is next touched.Closing as already done per the evidence above. Queue task 7352 completes
already_done.