feat(webui): provider management UI with keep-key semantics and hot-apply wiring - #37
Merged
Merged
Conversation
Ticket 03 of the model-providers workstream. Adds the settings-modal panel that lists, edits, adds, deletes and test-connects providers, and wires the 'providers.updated' SSE frame into the store so the model selector refreshes without a page reload. Server side - keep-existing-key convention: an incoming 'auth.apiKey === ""' on PUT means 'do not change the existing key for this provider id'. Implemented as a pure helper (lib/providers-config.js# applyKeepKeyConvention) called from handlePutProviders before validation. Existing key is preserved; non-empty value replaces. New providers with empty keys fail the normal byok validation. Cross-branch API note: convention lives on this branch only. - 6 new lib tests + 2 new route tests pin the contract end-to-end (round-trip through PUT, masked-only responses, etc.). Webapp side - /api/providers + /api/providers/test typed wrappers in lib/api.ts. - 'providers.updated' added to NAMED_EVENTS in lib/sse.ts with a new 'providers-updated' SseAction. Store carries a providersRevision counter that bumps on the frame; composer re-fetches /api/models on revision change so the selector picks up new groups without reload. - New 'providers' section in the settings modal (alongside general / appearance / connection). Left rail lists providers with preset/custom badge, protocol, auth type, has-key state, per-row test summary. Right pane edits auth (byok or coding-plan), baseURL, models list (id / label / contextLimit / thinking levels multi / modalities multi / add / remove). API key field is write-only: the masked value is the placeholder, never the controlled value. - Per-protocol minimal probe inline: success latency, or mapped error (INVALID_KEY / BAD_PROTOCOL / PROBE_FAILED / timeout). - Preset one-click enable degrades gracefully when the sibling branch's endpoints are not yet mounted (404 -> hide section). - Pure helpers in lib/provider-management.ts (validation, draftToWire, describeTestOutcome, draftFromView, newDraftProvider). Tested in isolation: 37 webapp tests covering keep-existing sentinel, sentinel-not-leaked-into-controlled-field, model-row validation, enum value pinning, and the SSE frame parser. Inventory regenerated via 'node scripts/source-inventory.mjs --write' (reviewed 3 new file paths: provider-management.tsx, provider-management.ts, provider-management.test.ts).
…ayer scoping)
Acceptance round-2 hardening for ticket 03's PUT convention.
Two gaps closed:
1. ABSENT auth.apiKey. The v2 normaliser coerces a missing apiKey
to the empty string before validation, so a PUT body that omits
the field used to silently wipe a stored credential AND drop the
model array (the malformed branch also failed the inner models
shape on some paths). The convention now treats absent-field as
the sentinel — same as the explicit empty string. Test pins
this end-to-end through handlePutProviders.
2. ENV-LAYER SCOPING. The convention's 'previous key' lookup used to
read the merged catalogue. That meant editing a provider whose
key is sourced from the env or cwd layer would materialise the
deployment secret into the user-level file — once written there,
the deployment layer can no longer rotate it. The convention now
reads the user-level file only via a new loadUserLevelProviders()
helper. The merged view still wins for the engine
(loadProvidersConfig priority order), so visible behaviour for the
operator is unchanged: env-defined keys still win at read time
even after the user edits.
DELIBERATE KEY CLEARING is impossible by design: every wire shape
that lacks an explicit apiKey inherits the stored key. Rotating a
credential means PUTting the new value. Documented in the API note
and in the editor placeholder ('Stored as-is — leave blank (or
omit the field) to keep the existing key. Clearing is not supported.').
Tests (3x confirmed 0 fail):
- providers-config.test.js: 55 (added: absent-field copies key;
absent-field keeps empty on new; absent-auth entirely; deliberate
clear remains impossible; loadUserLevelProviders missing /
user-only / env-invisible).
- providers.check.mjs: 19 (added: absent-field PUT preserves key
end-to-end; env-layer key NOT materialised to user-level file).
- webapp typecheck + repo typecheck clean; full pnpm test:webapp
253 pass; full pnpm test 1494 pass / 0 fail.
…h preset routes) PR #36 added preset provider templates alongside the existing provider routes, conflicting with this branch's edits to packages/webui/server/routes/providers.js (the keep-existing-key convention in handlePutProviders). Resolution: - routes/providers.js imports: union of both sides. The keep-key helpers (applyKeepKeyConvention, loadUserLevelProviders) from this branch plus normaliseProvider (used by handleEnablePreset) from main are all imported from lib/providers-config.js. - The new handleGetPresets / handleEnablePreset handlers from main coexist with this branch's handlePutProviders / handleTestProvider intact — both sides' logic survives. - routes/providers.js exports: re-export the preset handlers from this single module so the Hono app can mount them. Gates (post-merge): - app-hono.test.js: OWNED_ROUTES includes both sides; app.get / app.put / app.post calls for /api/providers, /api/providers/test, /api/providers/presets, /api/providers/preset/:id/enable all present (auto-merged clean). - providers-config + providers.check: 74 pass / 0 fail × 3. - app-hono: 85 pass / 0 fail. - full pnpm test:webapp: 253 pass / 0 fail. - full pnpm test: 1545 pass / 0 fail / 2 skipped (no regressions). - pnpm webapp:typecheck + pnpm typecheck: clean. - node scripts/source-inventory.mjs: clean.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Ticket 03 of the provider-configuration feature: the management UI, plus one API-hardening contract that the UI depends on.
provider-management.tsx): provider list (preset/custom badge, enabled toggle, protocol, auth, has-key), editor (auth-type switch, baseURL, per-model rows with thinkingLevels/modalities multi-select), test-connection with structured errors, soft-delete custom providers. 47 bilingual i18n keys.auth.apiKeyempty OR absent means "keep the existing key" (applyKeepKeyConvention, user-layer-only source so env secrets never persist to operator disk); deliberate key-clear is documented as impossible (delete-and-recreate). Masked keys never round-trip into the form.providers.updatedSSE frame →providersRevisionin the store → composer re-fetches/api/models— the model selector updates WITHOUT page refresh (live-verified end to end).Acceptance (independent agent, 2 rounds + hardening confirm)
Round 1 PASS-WITH-CONCERNS — the sentinel had a bypass: a PUT with
auth.apiKeyfield ABSENT skipped convention+validation and silently wiped the stored key and models (live-proven). Round 2 hardening: absent = keep (probe re-verified live: disk key byte-identical), env-layer scoping restricted to the user file (env secrets zero-leak to disk), no-deliberate-clear documented. Pre-existing-failure claims refuted twice in the good direction (full suite 0 fail). Gates: 74/74 focused ×3 · test:webapp 253/253 · full suite 1494/0 fail · typechecks 0 · check:source ✓.Known non-blocking: editing an env-layer provider's label is shadowed at merge time by the env layer's scalar (pre-existing merge semantics, noted).
Full
pnpm verifydeferred to CI. Note: this branch and #36 both touch routes/providers.js (different regions); CI verifies the merge.