fix(settings): load Settings without a 404, and bound the MCP key list (#2202)

[dolho]

Stacked on #2783 (fix/2771-table-cell-markdown) → #2782 → #2781 → #2780 → #2778. Part of epic #1430.

Fixes #2202

1. The 404 on every tab

Settings read public_chat_url through the generic GET /api/settings/{key}, which answers 404 for a key nobody has written. The store already treated 404 as "unset", so nothing was broken — what was lost was the signal: a genuine failure on that call looked exactly like the ordinary case. No client-side handling can suppress the browser's own network log, which is why the fix is a route and not a try/catch.

GET /api/settings/public-chat-url answers 200 {value: null} when unset — the /mcp-url precedent, a named route for a named setting, declared above /{key} (Invariant #4, pinned by a test: declared after it, the literal path is swallowed as a setting named "public-chat-url" and 404s again, silently). The generic route's 404 is deliberately unchanged — it is the documented contract for every other key and is read outside this repo.

Writing the spec found a second key the issue never named: platform_default_model, 404ing eight times per Settings load. It needed no new route — /api/settings/feature-flags already carries the resolved value — so the page reads it from there. That is also more correct: the control now shows what the platform will actually use instead of blank.

2. The unbounded key list

Measured on the reporter's instance: 306 keys, 294 revoked (96%), ~71KB of DOM text, no filter and no bound. Agent keys accumulate structurally — one per agent, one per #1854 rotation, one per ephemeral ghost — so the page grows for the life of the instance and the keys that still work are buried in the ones that do not.

• Revoked keys hidden behind an explicit toggle that states the count.
• Search by name / prefix / agent (never the hash — asserted).
• Rendering bounded at 25 rows with "Show more", so page weight no longer scales with instance age. Client-side, per the issue's own note.
• The non-admin agent-key filter is carried through unchanged and asserted — it is an access rule wearing a filter's clothes, and widening it would disclose the fleet's agent names.

Three empties, three next actions. "No API keys" was a lie to an operator holding 294 revoked ones. But one piece of chrome: the wording is computed and only the action row branches, because three copies of the markup would have tripled this file's palette-class count.

Primitives, and paying for them

The new controls are BaseInput / BaseToggle / BaseBadge / BaseButton (#2122) rather than another hand-rolled class string. The two container borders the filter bar and footer need are paid for by converting the create form's hand-rolled name input and description textarea to BaseInput/BaseTextarea in the same component: McpKeysTab.vue raw_gray 97 → 83. Baseline edited by hand, not regenerated (a wholesale scan run deletes the refrozen block).

Verification

Against a local dev instance, with a key created and revoked to exercise the toggle (removed afterwards — the instance is back to 2 keys):

• every Settings tab: zero 404s, zero console errors, and the WS ticket mint asserted to succeed;
• the list renders 2 of 3 keys with "Show revoked (1)" and "2 active", no Revoked badge in the rows;
• the toggle reveals the revoked row and the list stays bounded;
• search narrows to a named empty state whose "Clear search" restores the list;
• the converted create form renders and binds (typed values read back) in both themes.

Negative control: with the three frontend files reverted, both halves fail — the 404 assertion lists public_chat_url, and the panel assertions cannot find the toggle.

The backend route has its own unit test (tests/unit/test_2202_public_chat_url_route.py): unset → value: null, configured → the value, the same key the writer writes, the admin gate, declaration order, and that the generic getter still 404s. Its two source-reading checks take no fixture so they run without the backend venv; they were executed locally. The route itself was verified through a stand-in on the local proxy, because the running instance is on dev — called out plainly rather than implied.

Frontend unit suite: 134 files, 2949 tests green (16 new for the list rules).

One console-error exclusion is documented in the spec: WebSocket handshake noise. The subject is Settings' own errors; the socket is a global concern present on every page and absent from any harness serving the bundle without a WS proxy. What could regress from Settings — the ticket mint — is asserted as an HTTP call instead.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Q19uRCksdn4DiRAJ55rfpZ

[dolho]

Review — /review pass

No critical findings. This is the only PR in the stack with a backend change, so the gates got the most attention.

Auth. GET /api/settings/public-chat-url runs assert_admin, matching the generic getter it replaces for this key — an addition that is not a way around the gate. It is declared above /{key} and the test pins that ordering, which matters more than it looks: declared after the catch-all, the literal path is swallowed as a setting named public-chat-url and answers 404, silently reintroducing the exact bug. Invariant #4, checked rather than remembered.

Contract preserved. The generic route still 404s for an unset key, asserted by a source-reading test. That is the right call — the 404 is depended on outside this repo (the ops agent, scripts), and widening it for one key's convenience would be a breaking change nobody asked for.

The second key. Finding platform_default_model 404ing eight times per load by writing the spec, and fixing it by reading the resolved value off /feature-flags rather than minting a second route, is the better outcome than what the issue asked for — and it makes the control show the effective default instead of blank.

[I1] The route is admin-only while /mcp-url — its stated precedent — is not (confidence 7/10)

GET /api/settings/mcp-url is get_current_user ("Any authenticated user can read this"). The new route copies that shape but keeps assert_admin. That is the conservative direction and matches the getter it replaces, so it is right — but the PR body cites /mcp-url as the precedent without noting they differ on auth. Worth one line, so a future reader does not copy the wrong half of the precedent.

[I2] The backend test mostly skips outside CI (confidence 8/10, by design but worth stating)

Four of six cases need the backend venv and skip without it; I ran the two dependency-free ones locally and they pass. So on this machine the route's behaviour is covered by CI only, and locally the route was exercised through a stand-in on the dev proxy because the running instance is on dev. The PR body says this plainly, which is the right handling — flagging it here so a reviewer knows the local evidence is frontend-side.

[I3] emptyReason is total, but the panel calls it with a fourth state it cannot express (confidence 6/10)

emptyReason({total, filtered, query, showRevoked}) returns no-match whenever a query is present, including the case where the query matches nothing and all keys are revoked. The wording then says "Searching N active keys" and offers Clear search, which is correct-but-incomplete: clearing the search still shows nothing. Minor; a combined message would be better, and the pure module is the right place for that decision.

Checked and clean

• Access rule carried through. The non-admin agent-key filter survives every new filter — filterKeys(..., {isAdmin:false, showRevoked:true, query:'scout'}) returns [] — and the spec says why it is an access rule rather than a convenience filter. That is the one thing in this diff that could have leaked (fleet agent names to any signed-in user), and it is pinned.
• Search never matches a secret-shaped field — asserted against key_hash.
• Bounded rendering is per-treatment, not per-key, so it cannot be defeated by an instance with more keys; pageOf(limit: 0) still renders one row rather than an empty list.
• Ratchet paid honestly: the new controls use the refactor: build the seven Base* primitives the design-system contract mandates (BaseButton, BaseInput, BaseSelect, BaseToggle, BaseTextarea, BaseBadge, BaseCard) #2122 primitives, and the two container borders they need are paid for by converting the create form's hand-rolled input/textarea in the same component (97 → 83). I verified the converted form still binds — typed values read back — rather than trusting the render.
• Negative control fails both halves: the 404 assertion lists public_chat_url, and the panel assertions cannot find the toggle.
• Documented exclusion (WebSocket handshake noise) is narrow, justified in the spec, and the thing that could actually regress from Settings — the ticket mint — is asserted as an HTTP call instead.

[dolho]

/review — post-rebase (stacked on fix/2771-table-cell-markdown)

Scope: CLEAN (the platform_default_model read moved to the flags payload is the same 404 class, named in the commit). Plan completion vs #2202: 5/5 done.

Rebase note: dev's #1028 split routers/settings.py into a package, so the route was ported to routers/settings/mcp_url.py beside /mcp-url and tests/unit/test_2202_public_chat_url_route.py adapted — Invariant #4 is now asserted as include_router(mcp_url.router) < include_router(generic.router) in __init__.py. Verified: assert_admin gate intact, PUBLIC_CHAT_URL_SETTING_KEY == "public_chat_url" matches the generic PUT, generic /{key} still 404s, 6/6 unit tests pass.

Critical

None.

Informational

• [I1] size="sm" on BaseInput is not a declared prop (8/10) — McpKeysTab.vue:69; BaseInput forwards $attrs to the <input>, so it lands as the HTML size attribute (invalid value → ignored). Drop it or add a real variant.
• [I2] loadFeatureFlags() is once-per-session, so a re-run of loadPlatformDefaultModel() after saving reads a stale cache (7/10) — nothing observable today (savePlatformDefaultModel keeps the local ref); loadFeatureFlags(true) or set the store value on save.
• [I3] Raw axios.get(..., { headers: authStore.authHeader }) instead of a store/api.js (6/10) — matches the surrounding idiom in Settings.vue (:2402/:2420/:2444); not blocking.
• api-endpoints.md gains no /public-chat-url line (same tier as the documented /mcp-url) — one line at sync time.

Clean

Only db.get_setting_value; route is get_current_user + assert_admin (rejects agent/connector principals); non-admin agent-key filter preserved in visibleKeys and asserted; filterKeys never searches the hash; emptyReason exhaustively handled. Suite 3231/3231.

[dolho]

merge-train (#2896, dry run): pushed 32415e066 to this branch — mechanical. The train's regression diff surfaced five failures none of this PR's own CI could see (backend-unit-test does not run for a PR against a feature branch): (1) test_2202_public_chat_url_route ×4 raised no current event loop on the CI interpreter — _call now uses asyncio.run; (2) test_1028_settings_package::test_the_mounted_route_set_is_unchanged pins the post-split route set, so the new GET /api/settings/public-chat-url is now listed in _ADDED_SINCE_SPLIT with its reason. 13/13 locally. Children re-stacked.

[vybe]

merge-train: rebased onto dev, retargeted, two mechanical follow-ups pushed. git rebase --onto origin/dev 205964978 (the pre-squash tip of #2783, merged as 29b818f). One conflict, src/frontend/raw-color-baseline.json: the _1925_note line — dev carries the corrected NavBar figures from #2781's train pass, this branch the older wording plus the new _2202_note; kept dev's note and added yours. The McpKeysTab entry (97 → 83) merged untouched. Then, from validation: dropped size="sm" on the search BaseInput (the primitive declares no size prop, so it landed on the <input> as an ignored HTML attribute), and added the GET /api/settings/public-chat-url row to docs/memory/architecture/api-endpoints.md beside its /mcp-url precedent. Locally: rawColorRatchet + loadingGateRatchet + mcpKeyList green; test_2202_public_chat_url_route + test_1028_settings_package green.

[vybe]

Addendum: the note resolution above briefly pushed the baseline without the comma after _1925_note (invalid JSON); corrected and re-pushed in the same commit — f7bccc1e4 → 997c72a12. json.load parses; rawColorRatchet green.

[vybe]

merge-train: validated (lane B), rebased onto dev, all checks green on the retargeted run.