Repository navigation
feat(webui): connection profiles with status-driven header - #689
Conversation
Replace the always-visible base URL / API key inputs with a connection indicator (status dot + profile name + server version) and a ConnectionPanel: multiple saved profiles (localStorage, migrated from the legacy config), one-click switch with full session-state cleanup, and a test-connection gate that validates both reachability (/healthz) and the API key (authenticated /sessions probe) before saving. Review round: 1 P1 (stale session list from old backend after profile switch — sessionsSeqRef guard) + 2 P2 (unauthenticated healthz gave false-green tests; in-flight test results unlocked Save for edited values) all fixed. 128 vitest, tsc clean.
📝 WalkthroughWalkthroughThe Web UI now supports persisted connection profiles. Users can switch, add, edit, test, and delete profiles. Header health checks use abort signals. App session loading resets and rejects stale responses when the active backend changes. ChangesConnection profile management
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant Header
participant ConnectionPanel
participant App
participant Backend
User->>Header: open connection control
Header->>ConnectionPanel: display profile store
User->>ConnectionPanel: test or select profile
ConnectionPanel->>Backend: check health and authenticated sessions
Backend-->>ConnectionPanel: return connection result
ConnectionPanel->>App: submit profile store change
App->>Backend: reload session list
Backend-->>App: return sessions
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (7)
coding-agent/webui/app/src/components/ConnectionPanel.tsx (3)
260-280: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd accessible names to the form inputs.
The three inputs carry only
placeholderand, for two of them,title. The API key input has neithertitlenor a label. Screen readers announce the placeholder inconsistently across browsers, and the placeholder disappears once the user types. Addaria-labelto each input.♿ Proposed labels
<input className={inputCls} type="password" placeholder="X-API-Key" + aria-label="API key" value={apiKey} onChange={(e) => onFieldChange(setApiKey)(e.target.value)} />Note:
ConnectionPanel.test.tsxline 85 andHeader.test.tsxline 184 query the API key input by placeholder text, so keep theplaceholderattribute.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@coding-agent/webui/app/src/components/ConnectionPanel.tsx` around lines 260 - 280, Update the three inputs in the ConnectionPanel form to include explicit aria-label values describing the profile name, server base URL, and API key fields. Preserve the existing placeholder attributes, values, and change handlers so current behavior and placeholder-based tests remain unchanged.
139-142: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider allowing metadata-only edits without a connection test.
canSaverequirestest.state === "ok"or a failed test plussaveAnyway. In edit mode a user who only renames a profile must run a network test against the server. If that server is currently offline, the user must also select "save anyway", which suggests the profile is broken.One option: allow save without a test when
mode.kind === "edit"and neitherbaseUrlnorapiKeychanged from the stored profile.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@coding-agent/webui/app/src/components/ConnectionPanel.tsx` around lines 139 - 142, The canSave condition in ConnectionPanel should allow edit-mode metadata-only saves when both baseUrl and apiKey remain unchanged from the stored profile, without requiring a successful connection test or saveAnyway. Preserve the existing validation and test requirements for new profiles and edits that change connection settings.
109-137: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAdd a timeout and abort to the connection test.
testConnectioncallsclient.health()andclient.listSessions()with noAbortSignaland no timeout. If the target host does not answer (for example a wrong port that drops packets), the fetch can stay pending for the browser default period. The button stays disabled and shows "Testing…" with no way to cancel. The pending promise also survives panel close, because nothing aborts it.
AgentClient.health(signal?)already accepts a signal (coding-agent/webui/app/src/lib/api.tslines 60-61).listSessions()does not, so a full fix needs either a signal parameter there orAbortSignal.timeoutwired through a client that supports it.♻️ Proposed timeout for the health probe
const testConnection = async () => { setTest({ state: "testing" }); const started = performance.now(); const snapshot: TestSnapshot = { baseUrl: baseUrl.trim(), apiKey }; const client = new AgentClient({ baseUrl: snapshot.baseUrl, apiKey: snapshot.apiKey }); + const ctrl = new AbortController(); + const timer = setTimeout(() => ctrl.abort(), 10_000); try { - const h = await client.health(); + const h = await client.health(ctrl.signal);Clear
timerin afinallyblock, and extendlistSessionsto accept a signal so the authenticated probe is bounded too.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@coding-agent/webui/app/src/components/ConnectionPanel.tsx` around lines 109 - 137, Update testConnection to use an AbortController with a timeout covering both client.health and client.listSessions, and abort the request when the panel unmounts or the timeout expires. Extend AgentClient.listSessions to accept and forward an optional AbortSignal, clear the timeout in a finally block, and preserve the existing snapshot checks and error reporting.coding-agent/webui/app/src/components/Header.tsx (2)
168-190: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAlign the ARIA pattern between the trigger and the panel.
The trigger uses
aria-expanded, which describes a disclosure widget.ConnectionPanelusesrole="dialog"(coding-agent/webui/app/src/components/ConnectionPanel.tsxline 178) withoutaria-modal, focus move on open, or focus return on close. Screen reader users receive mixed signals about the widget type.Pick one pattern. For a dropdown, drop
role="dialog"and keeparia-expandedwitharia-controlspointing at the panel id. For a dialog, move focus into the panel on open and restore focus to the trigger on close.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@coding-agent/webui/app/src/components/Header.tsx` around lines 168 - 190, Use the dropdown pattern for the connection control: update the trigger in Header’s connection panel block to include aria-controls referencing a stable panel id, and update ConnectionPanel to remove role="dialog" while retaining the existing aria-expanded behavior. Ensure the panel uses the same id so the trigger and controlled content are consistently associated.
83-104: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winThe health state can stay stale until the client changes.
The effect depends only on
client. It runs once per client identity. If the server stops responding after a successful check, the indicator keeps showing green and the previous version. Users read this control as live connection status.Consider a periodic re-check, for example a 30-second interval inside the same effect, with the existing abort logic reused for cleanup. Alternatively re-check when the window regains focus.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@coding-agent/webui/app/src/components/Header.tsx` around lines 83 - 104, The health-check effect keyed by client identity does not refresh after a successful check, leaving the indicator stale. Update the effect containing the health request to periodically re-run the check, such as every 30 seconds, while reusing the existing AbortController and cleanup logic so timers and in-flight requests are canceled when the effect is disposed.coding-agent/webui/app/src/lib/profiles.ts (1)
60-69: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueTrim
baseUrlandnameinsanitizeProfile.
sanitizeProfiletestsp.baseUrl.trim()but stores the raw value. A persisted profile with surrounding whitespace stays untrimmed, andAgentClient.url()only strips trailing slashes.addProfiletrims, so the two entry points differ.♻️ Proposed normalization
return { id: typeof p.id === "string" && p.id ? p.id : generateId(), - name: typeof p.name === "string" && p.name.trim() ? p.name : deriveName(p.baseUrl), - baseUrl: p.baseUrl, + name: typeof p.name === "string" && p.name.trim() ? p.name.trim() : deriveName(p.baseUrl), + baseUrl: p.baseUrl.trim(), apiKey: typeof p.apiKey === "string" ? p.apiKey : "", };Note:
profiles.test.tsline 73 assertsstore.profilesequals the stored objects verbatim, so that expectation stays valid only while fixtures contain no whitespace.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@coding-agent/webui/app/src/lib/profiles.ts` around lines 60 - 69, Update sanitizeProfile to store a trimmed baseUrl after validating it, and trim valid name values before returning the normalized profile. Preserve deriveName behavior for missing or blank names, and keep addProfile and persisted-profile loading consistent through the same normalization.coding-agent/webui/app/src/App.tsx (1)
66-80: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
loadProfiles()runs twice at startup.Line 75 calls
loadProfiles()insideloadConfig, and line 108 calls it again as theuseStateinitializer. Each call reads and parseslocalStorage, and the first call may seed and persist a new store.If the seed write fails,
saveProfilesswallows the error (coding-agent/webui/app/src/lib/profiles.tslines 131-135). The second call then seeds again and produces a differentidthroughgenerateId().config.baseUrlandconfig.apiKeystill match, because both seeds derive from the same legacy config ordefaultBaseUrl(), sohandleProfilesChangebehaves correctly. The duplicated work and the divergent-id path are still avoidable.Load the store once and derive both states from it.
♻️ Proposed single load
-function loadConfig(): Config { +function loadConfig(store: ProfileStore): Config { const defaults: Config = { baseUrl: defaultBaseUrl(), @@ // The active connection profile owns baseUrl/apiKey; the legacy config's // copies are only migration input for the profiles store. - const active = activeProfile(loadProfiles()); + const active = activeProfile(store); if (active) {Then inside
App:- const [config, setConfig] = useState<Config>(loadConfig); - const [profiles, setProfiles] = useState<ProfileStore>(loadProfiles); + const [profiles, setProfiles] = useState<ProfileStore>(loadProfiles); + const [config, setConfig] = useState<Config>(() => loadConfig(profiles));Note: a
useStateinitializer cannot read another state value declared in the same render in that form. Use a singleuseMemo-free helper that returns both values, or compute the store in a module-level lazy initializer shared by bothuseStatecalls.Also applies to: 108-108
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@coding-agent/webui/app/src/App.tsx` around lines 66 - 80, Update the startup initialization around loadConfig and the App useState initializer so loadProfiles executes only once and both the initial config and profiles state derive from that same loaded store. Use a shared helper returning both values, or an equivalent module-level lazy initializer, while preserving activeProfile’s baseUrl/apiKey override and avoiding repeated seeding or parsing.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@coding-agent/webui/app/src/App.tsx`:
- Around line 66-80: Update the startup initialization around loadConfig and the
App useState initializer so loadProfiles executes only once and both the initial
config and profiles state derive from that same loaded store. Use a shared
helper returning both values, or an equivalent module-level lazy initializer,
while preserving activeProfile’s baseUrl/apiKey override and avoiding repeated
seeding or parsing.
In `@coding-agent/webui/app/src/components/ConnectionPanel.tsx`:
- Around line 260-280: Update the three inputs in the ConnectionPanel form to
include explicit aria-label values describing the profile name, server base URL,
and API key fields. Preserve the existing placeholder attributes, values, and
change handlers so current behavior and placeholder-based tests remain
unchanged.
- Around line 139-142: The canSave condition in ConnectionPanel should allow
edit-mode metadata-only saves when both baseUrl and apiKey remain unchanged from
the stored profile, without requiring a successful connection test or
saveAnyway. Preserve the existing validation and test requirements for new
profiles and edits that change connection settings.
- Around line 109-137: Update testConnection to use an AbortController with a
timeout covering both client.health and client.listSessions, and abort the
request when the panel unmounts or the timeout expires. Extend
AgentClient.listSessions to accept and forward an optional AbortSignal, clear
the timeout in a finally block, and preserve the existing snapshot checks and
error reporting.
In `@coding-agent/webui/app/src/components/Header.tsx`:
- Around line 168-190: Use the dropdown pattern for the connection control:
update the trigger in Header’s connection panel block to include aria-controls
referencing a stable panel id, and update ConnectionPanel to remove
role="dialog" while retaining the existing aria-expanded behavior. Ensure the
panel uses the same id so the trigger and controlled content are consistently
associated.
- Around line 83-104: The health-check effect keyed by client identity does not
refresh after a successful check, leaving the indicator stale. Update the effect
containing the health request to periodically re-run the check, such as every 30
seconds, while reusing the existing AbortController and cleanup logic so timers
and in-flight requests are canceled when the effect is disposed.
In `@coding-agent/webui/app/src/lib/profiles.ts`:
- Around line 60-69: Update sanitizeProfile to store a trimmed baseUrl after
validating it, and trim valid name values before returning the normalized
profile. Preserve deriveName behavior for missing or blank names, and keep
addProfile and persisted-profile loading consistent through the same
normalization.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 017bdfd2-fa59-4ad6-92ed-520f83c43f5c
📒 Files selected for processing (9)
coding-agent/webui/app/src/App.test.tsxcoding-agent/webui/app/src/App.tsxcoding-agent/webui/app/src/components/ConnectionPanel.test.tsxcoding-agent/webui/app/src/components/ConnectionPanel.tsxcoding-agent/webui/app/src/components/Header.test.tsxcoding-agent/webui/app/src/components/Header.tsxcoding-agent/webui/app/src/lib/api.tscoding-agent/webui/app/src/lib/profiles.test.tscoding-agent/webui/app/src/lib/profiles.ts
Header 的 URL/key 裸输入框 → 连接指示器(状态点+档案名+版本号)+ 连接档案面板:
评审一轮:1 P1 + 2 P2 已修;128 vitest 全绿,tsc 干净
Summary by CodeRabbit