feat: 276 expand dashboard and web operational clients to match runtime admin capabilities#352
Merged
yacosta738 merged 27 commits intoMar 28, 2026
Conversation
…ational parity (#276) Introduce comprehensive documentation for expanding dashboard and chat web clients to match runtime admin capabilities. Includes technical design, gap analysis, phased implementation proposal, detailed specification, and actionable task breakdown for surfacing operational features via new dashboard sections, gateway endpoints, and chat enhancements.
…es config sections (#276) Wire existing gateway admin capabilities into dashboard UI: - Extend TypeScript types to match Rust AdminConfigView structs - Add WebSearchSettings, BrowserSettings, ComposioSettings, MemorySettings, and UpdateSettings components with tests - Update useConfig composable with new field defaults, mappings, and secret reset logic for 6 new config sections - Add payload builders for web-search, browser, composio, memory - Integrate new sections into App.vue - Add i18n keys for en and es locales Phase 1 of DALLAY-181 web operational parity.
…view component (#276) Add GET /web/admin/channels gateway endpoint that returns configuration status for all 15 channel types (cli, webhook, telegram, discord, slack, mattermost, imessage, matrix, signal, whatsapp, email, irc, lark, dingtalk, qq) with redacted config summaries. Dashboard side: - Add AdminChannelStatusView TypeScript type - Create ChannelsOverview component with fetch, loading/error states, and color-coded configuration indicators - Wire into App.vue with gateway URL and bearer token props - Add i18n keys for en and es locales Channel health is best-effort per ADR-5: config-derived status only, no real-time probing (channel instances not in AppState). Phase 2 of DALLAY-181 web operational parity.
…SS imports - Update all CSS files to use rgb() slash syntax for alpha transparency instead of rgba() - Switch @import statements to use url() syntax for external CSS files - Refine font-family fallbacks for consistency - Adjust media queries to use modern syntax (width <= ...) - Minor reordering and deduplication of CSS rules for clarity - Update style check script to run stylelint before recursive checks
…olicy details (#276) Phase 3 of DALLAY-181 web operational parity: Rust gateway: - Add GET /web/admin/health endpoint exposing runtime health snapshot (pid, uptime, per-component status, restart counts) - Add GET /web/admin/scheduler endpoint exposing scheduler config and task count Dashboard: - Extend SecuritySettings with autonomy policy details: require approval for medium risk, block high risk commands, auto-approve and always-ask command lists (comma-separated textareas) - Add HealthDashboard component with overall status indicator, formatted uptime, and per-component health details - Add SchedulerStatus component with enabled/disabled state, task limits, and active task count - Add TypeScript types for health snapshot and scheduler status - Update configPayload with autonomy list diff logic - Add i18n keys for en and es locales
…tbeat overview sections (#276) Phase 4 of DALLAY-181 web operational parity: Rust gateway: - Extend AdminConfigView with cost, mcp, tunnel, reliability, and heartbeat sections including secret redaction for tunnel tokens - Remove stale duplicate admin_config_view and dead view structs from gateway/mod.rs Dashboard: - Add CostOverview, McpOverview, TunnelOverview, ReliabilityOverview, and HeartbeatOverview read-only components with fetch + tests - Add TypeScript types matching all new Rust view structs - Wire all 5 components into App.vue - Add i18n keys for en and es locales
…rship conventions
…ator, and session persistence (#276) Phase 5 of DALLAY-181 web operational parity: Rust gateway: - Add POST /web/chat/stream SSE endpoint that processes messages via existing dispatch and streams response as chunk/done/error events Chat app: - Add streamMessage() with fetch + ReadableStream SSE parsing, with automatic fallback to non-streaming sendMessage on failure - Update ChatMessage with streaming cursor and error state styling - Extract Message types to types/chat.ts with status field - Add ToolApprovalCard component with approve/reject buttons - Handle 403 approval_required responses in useChat - Add HealthIndicator component with 30s polling to /health - Add session persistence via sessionStorage watch/restore - Add i18n keys for streaming, approval, and health states
Complete SDD cycle for DALLAY-181: - Sync delta specs to openspec/specs/web-operational-parity/ with 12 requirements reflecting implementation reality - Archive change to openspec/changes/archive/ with all artifacts (exploration, proposal, spec, design, tasks, verify-report, archive-report) - Verification status: PASS WITH WARNINGS (no CRITICAL issues) - Known gap: ProviderPoolsSettings component deferred
Contributor
✅ Contributor ReportUser: @yacosta738
Contributor Report evaluates based on public GitHub activity. Analysis period: 2025-03-28 to 2026-03-28 |
Contributor
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds gateway admin endpoints and SSE chat streaming; extends Rust admin views/types and handlers; implements client SSE streaming with idempotency and tool-approval UI; expands dashboard admin sections, types, payload builders, and many Vue components/tests; introduces shared CSS layers, Stylelint and workspace updates; updates build lockfiles. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ChatApp as Chat App
participant Gateway
participant Runtime
User->>ChatApp: Send message (UI)
ChatApp->>Gateway: POST /web/chat/stream (SSE, X-Request-Id)
Gateway->>Runtime: Deliver message
Runtime-->>Gateway: SSE event: chunk
Gateway-->>ChatApp: SSE event: chunk
ChatApp->>ChatApp: Append chunk (status=streaming)
alt Approval required
Runtime-->>Gateway: SSE event: tool_approval
Gateway-->>ChatApp: SSE event: tool_approval
ChatApp->>User: Render ToolApprovalCard
User->>ChatApp: Approve/reject
ChatApp->>Gateway: POST /web/chat/tool-approval
Gateway->>Runtime: Forward decision
Runtime-->>Gateway: SSE event: chunk/resume
Gateway-->>ChatApp: SSE event: chunk
end
Runtime-->>Gateway: SSE event: done (message_id, session_id)
Gateway-->>ChatApp: SSE event: done
ChatApp->>ChatApp: Set status=complete
sequenceDiagram
participant Dashboard
participant Timer as Poll Timer
participant Gateway
participant Runtime
Dashboard->>Gateway: GET /web/admin/health (Auth)
activate Gateway
Gateway->>Runtime: Query health snapshot
Runtime-->>Gateway: Return health snapshot
Gateway-->>Dashboard: JSON health response
Dashboard->>Dashboard: Render component statuses
loop every 30s
Timer->>Gateway: GET /web/admin/health
end
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 51
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
clients/web/apps/dashboard/src/composables/configPayload.spec.ts (1)
101-125: 🧹 Nitpick | 🔵 TrivialAdd test coverage for new autonomy policy fields.
The existing security section test doesn't exercise the new fields (
autonomy_require_approval_for_medium_risk,autonomy_block_high_risk_commands,autonomy_auto_approve,autonomy_always_ask). IfbuildPayloadForSectionfails to include these in the autonomy patch, this test would still pass.💡 Suggested test addition
it("includes new autonomy policy fields in security payload", () => { const snapshot = createSnapshot(); const payload = buildPayloadForSection( "security", createForm({ autonomy_require_approval_for_medium_risk: false, autonomy_auto_approve: "ls, cat, echo", }), snapshot ); expect(payload).toEqual({ autonomy: { require_approval_for_medium_risk: false, auto_approve: ["ls", "cat", "echo"], // or however the list is serialized }, }); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/web/apps/dashboard/src/composables/configPayload.spec.ts` around lines 101 - 125, The test for security payload is missing coverage for the new autonomy policy fields; update the spec to add assertions that buildPayloadForSection (used with createForm and createSnapshot) includes autonomy_require_approval_for_medium_risk, autonomy_block_high_risk_commands, autonomy_auto_approve, and autonomy_always_ask in the resulting autonomy patch. Add a new it(...) or extend the existing test to supply those autonomy_* inputs via createForm and assert the payload.autonomy contains the corresponding keys with the correct types/serializations (e.g., boolean flags and a parsed array for auto_approve) so buildPayloadForSection correctly nests and serializes the new fields.clients/web/apps/chat/src/App.spec.ts (1)
82-101:⚠️ Potential issue | 🟡 MinorAdd test coverage for successful SSE streaming with chunk/done events.
The test correctly verifies the fallback when
/web/chat/streamreturns 500. However, the SSE success path—processingchunkanddoneevents viaReadableStream—is unimplemented in the test suite. The composable logic inuseChat.ts(lines 370–420) handles these events but has no corresponding test cases in eitherApp.spec.tsoruseChat.spec.ts. Add a test that mocks successful streaming with sample chunk/done events to confirm the streaming code path works as intended.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/web/apps/chat/src/App.spec.ts` around lines 82 - 101, Add a new test in App.spec.ts (or useChat.spec.ts) that covers the successful SSE streaming path: mock fetch for the stream endpoint to resolve with a Response whose body is a ReadableStream that emits serialized SSE messages representing "chunk" events followed by a "done" event, then assert that the composable logic in useChat.ts (the stream handling between lines ~370–420) consumes those chunks and updates state as expected; specifically, create a mocked ReadableStream that enqueues JSON payloads for chunk events and a final done event, ensure the test awaits the stream completion, and verifies the resulting message content/session_id/state changes produced by useChat's handlers for "chunk" and "done".clients/web/apps/dashboard/src/composables/useConfig.ts (1)
772-815:⚠️ Potential issue | 🟠 MajorPreserve unsaved secret edits across unrelated saves.
Every successful save calls
connectGateway(), which rebuildsformfrom server state. Onlywebhook_secret_*is copied out and restored, so pending edits in the new Brave, computer-use, Composio, and Cerebro secret inputs disappear if the user saves a different section first.As per coding guidelines, "Security first, performance second" and "Look for behavioral regressions, missing tests, and contract breaks across modules."
Also applies to: 850-856
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/web/apps/dashboard/src/composables/useConfig.ts` around lines 772 - 815, processSaveResponse currently preserves only pendingWebhookSecret across the connectGateway() call, causing unsaved secret edits for Brave, computer-use, Composio, and Cerebro to be lost; capture the pending mode/value pairs for web_search_brave_api_key, browser_computer_use_api_key, composio_api_key, and memory_cerebro_auth_token before calling connectGateway() (similar to pendingWebhookSecret), call connectGateway(), and then restore each pending pair back into form (e.g., form.web_search_brave_api_key_mode/value, form.browser_computer_use_api_key_mode/value, form.composio_api_key_mode/value, form.memory_cerebro_auth_token_mode/value) if they were present so unrelated saves don’t wipe in-progress secret edits in processSaveResponse.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@clients/agent-runtime/src/gateway/admin.rs`:
- Around line 1785-1790: The response currently hardcodes task_count: 0 in
AdminSchedulerStatusView; replace this with the real runtime task count by
calling into the scheduler runtime (e.g., add or use a method like
Scheduler::task_count(&self) or .len() on the scheduler instance) and set
task_count: scheduler.task_count() (or convert to usize/u64 as needed). If a
scheduler reference is not available in this scope, obtain it from the
runtime/context where cfg is used (or thread-safe shared state such as
Arc<Mutex<Scheduler>>) and call the new accessor; alternatively, if you prefer a
quick change, make AdminSchedulerStatusView.task_count Option<u64> and return
None until the accessor is implemented. Ensure AdminSchedulerStatusView and any
serialization derive reflect the new type if you choose Option.
In `@clients/web/apps/chat/src/App.vue`:
- Around line 244-272: handleApprove and handleReject are currently UI-only
stubs; replace the local-only updates in handleApprove and handleReject with a
proper POST to the backend endpoint POST /web/chat/tool-approval, sending the
approvalId and action ("approve" or "reject"), then update messages.value based
on the successful response and handle errors (show toast/log and do not mutate
state on failure); if you cannot implement the backend round-trip now, open a
tracking issue linking these functions (handleApprove, handleReject) and the
endpoint POST /web/chat/tool-approval so the follow-up work to wire the full
approval round-trip is tracked.
- Around line 229-242: In restoreMessages, validate parsed sessionStorage data
before assigning to messages.value: after JSON.parse(raw)
(messagesStorageKey()/restoreMessages), run a type guard/filter that ensures
each item has the expected Message shape (e.g., numeric id, required string
fields like content/role or whatever Message defines) and only assign the
filtered array to messages.value; compute messageIdCounter using the filtered
array (fall back to current counter if empty) and keep the try/catch around
JSON.parse to ignore parse errors but avoid assigning malformed objects to
messages.value or using invalid ids.
- Around line 150-193: The outer catch around chat.streamMessage currently
swallows all exceptions and falls back to chat.sendMessage (causing duplicate
auth handling); change it so the catch only handles streaming-specific failures:
detect auth/credential errors (e.g., error.name/message or a sentinel like
error.isAuthError set by markCredentialInvalid in streamMessage) and rethrow
those immediately, while only proceeding to the sendMessage fallback for
non-auth streaming errors; keep updateAssistantMessage(assistantMessageId, ...)
behavior for streaming failures but avoid calling the fallback when the error
indicates credential invalidation.
In `@clients/web/apps/chat/src/components/chat/ChatMessage.vue`:
- Around line 34-35: The streaming cursor span in ChatMessage.vue is decorative
but currently has an aria-label and the streaming assistant text isn't
announced; make the span with class "streaming-cursor" purely decorative (remove
aria-label and add aria-hidden="true") and add an accessible live region for
streaming content by setting an appropriate aria-live (e.g., aria-live="polite")
and/or role="status" on the assistant message container (the element rendering
class "bubble-text" or its parent) when status === 'streaming' so screen readers
announce incremental updates; ensure the change only applies for assistant
messages and preserve visual styling.
In `@clients/web/apps/chat/src/components/chat/ToolApprovalCard.vue`:
- Around line 22-27: In ToolApprovalCard.vue update the approve/reject buttons
to set type="button" and guard against duplicate emits by using a one-shot flag
or the existing pending prop: only emit 'approve'/'reject' when not pending,
immediately set a local isSubmitting flag (or rely on pending prop) to true and
disable both buttons while true to prevent rapid double-clicks, and ensure the
click handler checks the flag before emitting (refer to the current
`@click`="$emit('approve', approvalId)" and `@click`="$emit('reject', approvalId)"
handlers and the approvalId prop).
In `@clients/web/apps/chat/src/components/HealthIndicator.spec.ts`:
- Around line 45-52: Add a new test in HealthIndicator.spec.ts that covers
non-OK HTTP responses by resolving fetch (via fetchMock) with a Response-like
object having status: 500 (and optional ok: false) and then asserting the same
disconnected behavior; specifically, replicate the existing "shows disconnected
on health check failure" flow but replace mockRejectedValueOnce with
mockResolvedValueOnce({ status: 500, ok: false }) (or the project's Response
shim) when calling mountIndicator(), await flushPromises(), and assert that
wrapper.get('[data-testid="health-status"]').classes() contains "disconnected".
In `@clients/web/apps/chat/src/components/HealthIndicator.vue`:
- Around line 21-24: Normalize props.gatewayUrl before composing the health
endpoint: ensure you strip any trailing slashes from props.gatewayUrl (and guard
against null/undefined) then append a single "/health" when calling fetch (the
string interpolation around `${props.gatewayUrl}/health`), so the request never
becomes "//health" or contains duplicate slashes; update the fetch call in
HealthIndicator.vue to use the normalized base URL (e.g., assign a local
variable like gatewayBase from props.gatewayUrl with trailing slashes removed)
and use that when building the request URL.
In `@clients/web/apps/chat/src/composables/useChat.ts`:
- Around line 399-409: The JSON.parse calls for the "done" and "error" SSE
branches can throw on malformed data; wrap the parsing in try-catch inside the
currentEvent === "done" and currentEvent === "error" branches (where doneEvent
is parsed as StreamDoneEvent and errorEvt as StreamErrorEvent) so parse failures
are handled gracefully—on parse error, log or process a fallback (e.g., log the
raw currentData and continue the stream or throw a controlled Error with a safe
message using t("chat.requestError", { text: normalizedMessage })) instead of
letting an unhandled exception bubble up; ensure setSessionReady still runs only
when a valid session_id is present after successful parse.
In `@clients/web/apps/chat/src/style.css`:
- Around line 1-2: The CSS `@import` lines currently use url(...) notation which
fails Stylelint; update the two imports (the `@import
url("@corvus/shared/app-shell.css")` and `@import url("tailwindcss")` lines) to
use quoted import syntax (e.g., `@import` "..." ) so they become plain quoted
`@import` statements without url(...) to satisfy the linter.
In `@clients/web/apps/dashboard/src/App.spec.ts`:
- Around line 240-241: Add a short inline comment above the two assertions
(expect(wrapper.findAll("input")).toHaveLength(12) and
expect(wrapper.findAll("[data-section]")).toHaveLength(7)) that documents why
those exact counts are expected (e.g., list the form fields, hidden inputs,
dynamic sections or components counted) so future maintainers can update the
numbers with context; modify the test near those assertions to include that
explanatory comment.
In `@clients/web/apps/dashboard/src/App.vue`:
- Around line 236-259: The five overview components (CostOverview, McpOverview,
TunnelOverview, ReliabilityOverview, HeartbeatOverview) each receive identical
auth props (config.baseUrl.value and config.bearerToken.value) and likely
perform redundant admin-config fetches; refactor to fetch the admin config once
in the parent (App.vue) or a shared composable (e.g., useAdminConfig) and pass
down the needed slices or derived props instead of raw auth values so children
consume the same snapshot and avoid fan-out/skew—implement a single async fetch
in App.vue (or a composable) that exposes the config object and pass specific
pieces (e.g., costConfig, mcpConfig, tunnelConfig, reliabilityConfig,
heartbeatConfig) to the respective components.
- Around line 266-268: UpdateSettings is not mounted because useConfig exposes
AdminConfigForm instead of the AdminConfigView that UpdateSettings requires; fix
by updating the useConfig hook (or adding a small adapter function) to return or
convert to AdminConfigView (or expose a getter like getAdminConfigView) so
UpdateSettings can be safely mounted in App.vue where the TODO is. Locate the
useConfig implementation and either change its return typing to include
AdminConfigView or add a conversion helper that maps AdminConfigForm ->
AdminConfigView, update UpdateSettings props/usage to consume that
AdminConfigView, and then mount UpdateSettings in App.vue using the provided
AdminConfigView from useConfig.
- Around line 226-264: The overview components (ChannelsOverview,
SchedulerStatus, CostOverview, McpOverview, TunnelOverview, ReliabilityOverview,
HeartbeatOverview, HealthDashboard) are mounting immediately and firing admin
fetches before pairing/auth is established; add a single ready/auth reactive
flag (e.g., isAuthenticated or isReady) in App.vue that becomes true only after
pairing/connection and valid config.bearerToken.value and config.baseUrl.value
are confirmed, then wrap each component tag with a v-if="isReady" (or a computed
like showOverviews() that checks config.bearerToken.value &&
config.baseUrl.value && pairing state) so they only mount/render once
authentication/connection is valid, preserving the existing prop bindings
(:gateway-url="config.baseUrl.value" and
:bearer-token="config.bearerToken.value").
In `@clients/web/apps/dashboard/src/components/config/BrowserSettings.vue`:
- Around line 35-49: The select change handler currently updates
modelValue.browser_computer_use_api_key_mode via updateField but does not clear
the stored secret; modify the change handler in BrowserSettings.vue so that
after calling updateField('browser_computer_use_api_key_mode', ...), if the new
value !== 'replace' you also call
updateField('browser_computer_use_api_key_value', '') (or null per existing
conventions) to wipe the plaintext API key from reactive state; apply the same
fix to the other analogous select that controls the related mode (the other
browser_computer_*_use_api_key_mode field) so both modes clear their
corresponding browser_computer_use_api_key_value when not 'replace'.
In `@clients/web/apps/dashboard/src/components/config/ChannelsOverview.spec.ts`:
- Around line 27-33: Update the test in ChannelsOverview.spec.ts that stubs
global fetch (vi.stubGlobal(...)) to also assert the fetch call used the
expected admin endpoint and bearer auth: after the component renders, verify the
mocked fetch was called with the URL "/web/admin/channels" and that the Request
(or fetch call args) includes an Authorization header with the expected "Bearer
..." token/format (use the same auth source used by the app under test) so the
test locks the auth contract while still returning mockChannels.
In `@clients/web/apps/dashboard/src/components/config/CostOverview.spec.ts`:
- Around line 21-51: The test for "renders cost data on successful fetch" must
also assert the authenticated request contract: after mounting via
mountComponent() and flushPromises(), verify that the global fetch
(vi.stubGlobal) was called with the admin config endpoint and that the request
init includes an Authorization header with a Bearer token (e.g.,
expect(fetch).toHaveBeenCalledWith(expect.stringMatching(/\/admin\/config/),
expect.objectContaining({ headers: expect.objectContaining({ Authorization:
expect.stringMatching(/^Bearer\s+/) }) }))). If your component reads the token
from a helper (e.g., getAuthToken) or localStorage, set/stub that value in the
test before mounting so the Authorization header is predictable, and continue to
call vi.unstubAllGlobals() at the end.
In `@clients/web/apps/dashboard/src/components/config/HealthDashboard.spec.ts`:
- Around line 21-68: The test "renders health data on successful fetch"
currently stubs fetch but doesn't assert the request contract; update the test
(which uses vi.stubGlobal("fetch", ...) and mountComponent()) to also assert
fetch was called with the admin health endpoint and an Authorization Bearer
header: after await flushPromises(), add an assertion that the fetch mock was
called (e.g., expect(fetch).toHaveBeenCalled()) and verify its call args include
the expected URL (e.g., contains "/admin/health" or the exact endpoint your
component uses) and that the options object includes headers.Authorization set
to "Bearer <expectedToken>" (if the component reads a token from storage,
set/stub that token in the test before mountComponent so you can assert the
exact value). Ensure you reference the existing fetch stub and mountComponent
when adding these assertions.
In `@clients/web/apps/dashboard/src/components/config/HeartbeatOverview.spec.ts`:
- Around line 21-58: The tests currently stub global fetch but don’t assert
Authorization header propagation; update both tests ("renders heartbeat data on
successful fetch" and "shows error on fetch failure") to verify that fetch was
called with an Authorization: Bearer <token> header by referencing the test
setup's bearerToken and the mountComponent() call; after mounting and awaiting
flushPromises(), add an expectation that vi.mocked(global.fetch) (or the fetch
spy) was called and inspect the call args (Request or init.headers) to assert
the Authorization header equals `Bearer ${bearerToken}` so the admin config
fetch contract is exercised for auth forwarding.
In `@clients/web/apps/dashboard/src/components/config/HeartbeatOverview.vue`:
- Around line 46-61: The template currently renders nothing when fetch returns
but data.config?.heartbeat is null; update HeartbeatOverview.vue to handle a
null/absent heartbeat by rendering an explicit empty-state within the same
status-grid (or an early conditional block) that displays a clear "not
configured" / "unavailable" message and consistent status visuals. Specifically,
keep the existing bindings (heartbeat.enabled and heartbeat.interval_minutes)
for the normal path but add a fallback path when heartbeat is falsy that shows a
status-item with label t("heartbeat.enabled") and a status-indicator +
status-value indicating not configured/unavailable, and optionally a second
status-item for interval showing “—” or “N/A”; ensure the conditional around the
block checks for heartbeat !== null/undefined (or add a v-else) so the
empty-state is rendered when data.config?.heartbeat is null.
- Around line 58-60: The UI currently hardcodes the unit "min" next to
heartbeat.interval_minutes in HeartbeatOverview.vue; change this to use the i18n
translation instead (e.g., call t with a key like "heartbeat.unit_min" or use a
plural/seconds formatter) so the unit is localized—update the span that renders
{{ heartbeat.interval_minutes }} min to concatenate or format via t(...) (or a
number/relative formatter) and ensure the new translation key is added to locale
files; keep the displayed number source as heartbeat.interval_minutes and only
replace the literal "min".
In `@clients/web/apps/dashboard/src/components/config/McpOverview.spec.ts`:
- Around line 21-70: The tests currently stub global fetch but never assert that
the Authorization header with the bearer token is sent; update both specs in
McpOverview.spec.ts (the "renders MCP servers on successful fetch" and "shows
error on fetch failure" tests that call mountComponent()) to assert the fetch
stub was called with an init object whose headers.Authorization contains the
expected "Bearer <token>" string (inspect
vi.getMockedFunction(fetch).mock.calls[0][1].headers or use vi.fn() call
assertions) so any dropped/misformatted Authorization header is detected.
In `@clients/web/apps/dashboard/src/components/config/McpOverview.vue`:
- Around line 59-79: The template assumes mcp.servers and server.capabilities
are always arrays; add defensive guards to avoid render-time exceptions by
checking for arrays before using .length or .join. Update the v-if and v-for
usages around mcp.servers (e.g., replace v-if="mcp.servers.length > 0" and
v-for="server in mcp.servers" with checks like Array.isArray(mcp?.servers) and
iterating over mcp.servers || []) and guard the capabilities block and join
(e.g., v-if="Array.isArray(server?.capabilities) && server.capabilities.length >
0" and use server.capabilities.join(', ') only when it's an array). Ensure the
status and command bindings still render safely when fields are missing (use
optional chaining or fallback values where referenced).
In `@clients/web/apps/dashboard/src/components/config/MemorySettings.vue`:
- Around line 67-76: The select change handler for
memory_cerebro_auth_token_mode currently only updates the mode and leaves
modelValue.memory_cerebro_auth_token_value populated; update the handler used in
the <select> (and the similar handler at the other occurrence) to, after calling
updateField('memory_cerebro_auth_token_mode', ...), check the new mode value and
if it is not 'replace' call updateField('memory_cerebro_auth_token_value', '')
to clear the plaintext token from reactive state (use the same AdminConfigForm
type casting as shown), ensuring the token is wiped whenever mode !== 'replace'.
In
`@clients/web/apps/dashboard/src/components/config/ReliabilityOverview.spec.ts`:
- Around line 37-52: Add an explicit assertion that the stubbed global fetch was
called with the expected admin URL and Authorization header to enforce the admin
auth contract: after mounting via mountComponent() and flushPromises(), assert
that the fetch mock (vi.fn() used in vi.stubGlobal("fetch")) was called at least
once and that one of its calls includes the correct admin endpoint (matching
mockConfig or the component's admin URL) and an Authorization header containing
the admin token; update the test to inspect fetch.mock.calls (or
vi.mocked(fetch).mock.calls) and assert the request init.headers.Authorization
(or headers["Authorization"]) equals the expected token to catch regressions
that drop headers or hit the wrong URL.
- Around line 53-66: The test currently calls vi.unstubAllGlobals() inside the
test body which can leave global stubs in place if the test errors; move the
global unstub cleanup into an afterEach hook so fetch is always restored. Add an
afterEach(() => vi.unstubAllGlobals()) near the top of
ReliabilityOverview.spec.ts (alongside existing setup/teardown), and remove the
per-test vi.unstubAllGlobals() calls from the "shows error on fetch failure"
test (which uses vi.stubGlobal("fetch", ...)), ensuring mountComponent and
flushPromises remain unchanged.
In `@clients/web/apps/dashboard/src/components/config/ReliabilityOverview.vue`:
- Around line 64-69: The UI currently displays only the keys of
reliability.model_fallbacks which hides the fallback targets; update the
rendering inside the <span class="status-value"> to iterate
Object.entries(reliability.model_fallbacks) and render each entry as "primary →
fallback" (e.g., map entries to `${primary} → ${fallback}` and join with ", "),
falling back to t("reliability.none") when there are no entries; keep this
change localized to the reliability.model_fallbacks usage in the
ReliabilityOverview component so the status-label remains the same.
In `@clients/web/apps/dashboard/src/components/config/SchedulerStatus.spec.ts`:
- Around line 21-47: The test "renders scheduler status on successful fetch"
stubs global fetch but doesn't assert the authenticated request; update the test
(around mountComponent and vi.stubGlobal usage) to set a known token (e.g.,
const token = "test-token") the component will use (via localStorage or the same
token-fetch helper your code uses), then assert the fetch mock was called with
the scheduler endpoint URL and an Authorization header containing "Bearer
test-token" by adding an expectation like
expect(fetch).toHaveBeenCalledWith(expect.stringContaining("/scheduler"),
expect.objectContaining({ headers: expect.objectContaining({ Authorization:
`Bearer ${token}` }) })); ensure you still call vi.unstubAllGlobals() at the
end.
In `@clients/web/apps/dashboard/src/components/config/SchedulerStatus.vue`:
- Around line 18-38: Replace the one-time onMounted(fetchSchedulerStatus) call
with a watcher so fetchSchedulerStatus runs whenever the gateway or auth
changes: import watch from Vue (add to the existing import line), remove or
keep-onMounted but add watch([() => props.gatewayUrl, () => props.bearerToken],
fetchSchedulerStatus, { immediate: true }) so fetchSchedulerStatus (the async
function) re-runs on changes to props.gatewayUrl and props.bearerToken; ensure
the watcher uses the same fetchSchedulerStatus, and remove duplicate initial
fetch if you keep onMounted.
In `@clients/web/apps/dashboard/src/components/config/SecuritySettings.spec.ts`:
- Around line 32-42: Add interaction assertions for the new autonomy controls:
locate the test in SecuritySettings.spec.ts that already mounts the component
and currently checks existence of data-testid selectors
'autonomy-require-approval-medium-risk', 'autonomy-block-high-risk-commands',
'autonomy-auto-approve', and 'autonomy-always-ask'; for each control simulate
user interaction (checkboxes: trigger('click') or setChecked; textareas/inputs:
setValue or trigger('input')) and assert the component emitted the expected
'update:modelValue' events with the updated payload (use
wrapper.find('[data-testid="..."]').trigger(...) / setValue(...) and then
expect(wrapper.emitted('update:modelValue')) to include the new values for the
corresponding fields).
In `@clients/web/apps/dashboard/src/components/config/SecuritySettings.vue`:
- Around line 103-116: The two textarea placeholders in SecuritySettings.vue are
hardcoded ("command1, command2"); replace them with the i18n translator (e.g.,
use $t("security.placeholderCommands")) for both the autonomy_auto_approve and
autonomy_always_ask textareas (refer to the textarea elements with data-testid
"autonomy-auto-approve" and "autonomy-always-ask" and the updateField handler).
Add the new "security.placeholderCommands" key to the English and Spanish
translation files (or the appropriate locale files) to maintain EN/ES parity, or
mark the key for translation if not yet available.
In `@clients/web/apps/dashboard/src/components/config/TunnelOverview.spec.ts`:
- Around line 21-63: The tests "renders tunnel data on successful fetch" and
"shows error on fetch failure" currently stub fetch but never assert the
Authorization header; update both specs to pass a known bearerToken into
mountComponent (or the component props used by mountComponent) and after
flushPromises assert that the global fetch was called with request init
containing headers.Authorization === `Bearer ${bearerToken}` (use
vi.stubGlobal("fetch", ...) and expect(fetch).toHaveBeenCalledWith(...,
expect.objectContaining({ headers: expect.objectContaining({ Authorization:
`Bearer ${bearerToken}` }) })) so the tunnel fetch auth propagation is
validated; keep assertions in the same tests and still call
vi.unstubAllGlobals() after.
In `@clients/web/apps/dashboard/src/components/config/UpdateSettings.vue`:
- Around line 25-35: The template currently treats missing booleans as "No";
update the two bindings (config.updates?.status?.update_available and
config.updates?.auto_install_enabled) to explicitly distinguish
true/false/undefined by rendering the "yes" text only when === true, the "no"
text only when === false, and an "unknown" (or placeholder like '-') when the
value is null/undefined; apply this change for both data-testid elements
(updates_update_available and updates_auto_install_enabled) so unknown state is
shown instead of defaulting to "No".
In `@clients/web/apps/dashboard/src/composables/configPayload.ts`:
- Around line 367-368: The current early-return for "updates" (if (section ===
"updates") return {}) silently produces an empty payload; either make this
section truly read-only by removing its save path in the consumer (i.e., stop
calling save for "updates") or implement a proper payload builder instead of
returning {}. Update the branch handling section === "updates" to construct and
return the real payload object (mirror the pattern used for other sections in
this module), or if intended read-only, remove any save/submit handlers that
rely on this function for "updates" so nothing is sent to the gateway.
In `@clients/web/apps/dashboard/src/style.css`:
- Around line 1-2: Replace the CSS `@import` url(...) syntax with string import
notation to satisfy the Stylelint import-notation rule; update the two imports
in style.css (the lines importing "@corvus/shared/app-shell.css" and
"tailwindcss") to use plain string form (e.g., `@import` "..." ) so Stylelint no
longer flags import-notation violations.
In `@clients/web/apps/docs/src/content/docs/es/guides/css-architecture.md`:
- Around line 1-9: The new Spanish guide "Arquitectura CSS"
(docs/es/guides/css-architecture.md) is orphaned in the docs graph causing CI to
fail; register it by adding a sidebar entry under the Spanish docs sidebar
configuration or add a persistent link to an indexed Spanish page (for example
include a link from an existing ES guide or the Spanish homepage) so the file is
reachable; ensure the title "Arquitectura CSS" and its frontmatter
(appliesTo/main, owner/team-platform) are used in the sidebar/link text so the
Docs Quality checker recognizes the page.
In `@clients/web/apps/docs/src/content/docs/guides/css-architecture.md`:
- Around line 1-9: The "CSS Architecture" guide (css-architecture.md) is
currently orphaned and missing a Spanish translation; fix by adding this guide
to the site's sidebar configuration (or add an inbound link to it from an
existing guide) so it is discoverable by the pipeline, and either add a Spanish
translation file for css-architecture.md under the ES docs directory or insert a
clear "Translation pending" notice at the top of the English guide indicating
the missing Spanish version.
- Around line 63-67: The three list items all begin with the word "Put", which
is repetitive; reword the bullets to vary sentence structure while preserving
meaning by using alternative verbs and phrasing for the entries referencing
packages/shared/theme.css, packages/shared/base.css, and
packages/shared/app-shell.css, e.g. change one or more "Put" lines to "Store",
"Place", "Keep", or "Define", and adjust the Astro/Starlight, landing-page, and
component-local style lines similarly so each bullet reads distinctly but still
directs tokens/aliases/theme variables to theme.css, resets/baseline rules to
base.css, app-frame concerns to app-shell.css, presentation rules to the owning
app, and component-specific styles inside the component.
In `@clients/web/apps/docs/src/styles/custom.css`:
- Around line 7-8: Stylelint flags the two imports using the url(...) form;
replace the url(...) imports with the linted string form so they pass
import-notation checks — change lines importing "@corvus/shared/base.css" and
"@corvus/shared/theme.css" from `@import` url("...") to the string form `@import`
"..." (update the two import statements shown).
In `@clients/web/apps/marketing/src/styles/global.css`:
- Around line 1-2: The `@import` lines in global.css use url(...) without quotes
which violates Stylelint's import-notation rule; update both imports (the lines
importing "@corvus/shared/theme.css" and "@corvus/shared/base.css") to use
quoted notation (e.g., replace url(...) usage with the quoted string form) so
the linter accepts the imports.
In `@clients/web/packages/shared/app-shell.css`:
- Around line 1-2: Stylelint rejects the url() wrapper in the `@import` rules;
update the two `@import` statements that reference "./theme.css" and "./base.css"
to use string notation (e.g., `@import` "<path>";) instead of `@import` url(...); to
satisfy Stylelint's expected syntax.
In `@gradle/libs.versions.toml`:
- Line 13: Update the AGP version referenced by the agp key in
libs.versions.toml from 8.13.2 to 9.1.0; after changing the agp = value, run a
quick compatibility check with the current Gradle (e.g., 9.5.0-milestone-1) and
Kotlin (2.3.20) toolchain, fix any breaking API changes or Gradle plugin
compatibility issues reported by the build, and update any related plugin
configuration or migration steps (AGP migrationGuide changes) to ensure the
project builds cleanly with agp set to 9.1.0.
In
`@openspec/changes/archive/2026-03-28-web-operational-parity/archive-report.md`:
- Around line 59-62: Update the final section titled "## SDD Cycle Complete" to
avoid claiming full completion: change the wording to indicate the change is
archived with known follow-ups and outstanding items (partial spec compliance,
an untested scenario, and skipped integration tests) so the archive remains
accurate — reference the "## SDD Cycle Complete" header and the footer paragraph
to rephrase it as "Archived with known follow-ups" and list the specific
outstanding items mentioned earlier in the report.
In `@openspec/changes/archive/2026-03-28-web-operational-parity/design.md`:
- Around line 77-84: Design currently lists GET /web/admin/tasks returning
AdminTaskListView but the implementation (see admin.rs) exposes GET
/web/admin/scheduler returning AdminSchedulerStatusView; update the design to
match the code (replace /web/admin/tasks and AdminTaskListView with
/web/admin/scheduler and AdminSchedulerStatusView) or explicitly document both
endpoints and their intended differences, ensuring the endpoint path and
response type in the design match the handler name and view type used in
admin.rs.
- Line 17: Update the fenced code blocks in the document to include language
identifiers: mark ASCII diagrams and sequence diagrams with ```text or
```plaintext, and mark HTTP examples (the blocks showing HTTP
requests/responses) with ```http or ```text; locate the relevant fenced blocks
by searching for the ASCII/sequence diagram blocks and the HTTP example blocks
in the document and add the appropriate language token to each opening ```
fence.
In `@openspec/changes/archive/2026-03-28-web-operational-parity/exploration.md`:
- Around line 29-39: The heading "Dashboard Coverage (~10 config sections)" is
incorrect relative to the listed components; update the heading to match the
enumerated components or expand the list accordingly — either change the heading
to "Dashboard Coverage (~7 config sections)" or add the missing config-section
entries to the list; verify you adjust the text that references the component
names (GeneralSettings.vue, SecuritySettings.vue, ObservabilitySettings.vue,
RuntimeSettings.vue, SchedulerSettings.vue, GatewaySettings.vue,
WebhookSettings.vue) so the count and list remain consistent.
In `@openspec/changes/archive/2026-03-28-web-operational-parity/proposal.md`:
- Around line 91-93: Update the proposal to use the implemented route name
`/web/admin/scheduler` instead of the stale `/web/admin/tasks`: locate the
bullet that currently reads "Add `GET /web/admin/tasks` endpoint for scheduled
task list" and change it to "Add `GET /web/admin/scheduler` endpoint for
scheduled task list"; keep the other bullets (e.g., `GET /web/admin/health` and
the skills inventory endpoint) unchanged unless their routes differ from the
implementation—verify and align any mention of the skills inventory
endpoint/component with the actual runtime enumeration route name as well.
In `@openspec/changes/archive/2026-03-28-web-operational-parity/tasks.md`:
- Around line 250-258: Update the task descriptions and acceptance criteria to
match the implemented endpoint and handler: replace references to GET
/web/admin/tasks and handle_admin_tasks with GET /web/admin/scheduler and
handle_admin_scheduler_status, and update the task bullets (3.10 and 3.11) to
enumerate the scheduler route, returned fields, and tests accordingly so
documentation aligns with the deployed implementation.
In `@openspec/changes/archive/2026-03-28-web-operational-parity/verify-report.md`:
- Around line 14-22: The summary table in verify-report.md incorrectly shows
59/59 complete while the body notes missing artifacts for tasks referencing
ProviderPoolsSettings.vue and IdentitySettings.vue; update the report so metrics
match reality by either decrementing "Tasks complete" and incrementing "Tasks
incomplete" to reflect those two missing files or mark those specific tasks as
"deviated/incomplete" in tasks.md and the verify-report text, and add a short
note that IdentitySettings functionality was folded into SecuritySettings.vue
(referencing ProviderPoolsSettings.vue, IdentitySettings.vue,
SecuritySettings.vue, and tasks.md) so auditors can trace the deviation.
In `@openspec/specs/web-operational-parity/spec.md`:
- Around line 151-163: REQ-5 in spec.md is out of sync with the implementation:
the code exposes GET /web/admin/scheduler and SchedulerStatus.vue shows
aggregate scheduler status, not a task list from GET /web/admin/tasks. Update
the spec (REQ-5) to reference the actual endpoint GET /web/admin/scheduler,
describe the returned aggregate scheduler fields (e.g., overall status, last run
timestamp/outcome, next run, enabled flag), and update the dashboard doc to
state that SchedulerStatus.vue renders aggregate scheduler status (leaving
SchedulerSettings.vue for config); alternatively, if you prefer to keep the
spec, implement GET /web/admin/tasks and add task-list rendering to
SchedulerStatus.vue—pick one consistency path and make the spec and code
references (GET /web/admin/scheduler, GET /web/admin/tasks, SchedulerStatus.vue,
SchedulerSettings.vue) match.
---
Outside diff comments:
In `@clients/web/apps/chat/src/App.spec.ts`:
- Around line 82-101: Add a new test in App.spec.ts (or useChat.spec.ts) that
covers the successful SSE streaming path: mock fetch for the stream endpoint to
resolve with a Response whose body is a ReadableStream that emits serialized SSE
messages representing "chunk" events followed by a "done" event, then assert
that the composable logic in useChat.ts (the stream handling between lines
~370–420) consumes those chunks and updates state as expected; specifically,
create a mocked ReadableStream that enqueues JSON payloads for chunk events and
a final done event, ensure the test awaits the stream completion, and verifies
the resulting message content/session_id/state changes produced by useChat's
handlers for "chunk" and "done".
In `@clients/web/apps/dashboard/src/composables/configPayload.spec.ts`:
- Around line 101-125: The test for security payload is missing coverage for the
new autonomy policy fields; update the spec to add assertions that
buildPayloadForSection (used with createForm and createSnapshot) includes
autonomy_require_approval_for_medium_risk, autonomy_block_high_risk_commands,
autonomy_auto_approve, and autonomy_always_ask in the resulting autonomy patch.
Add a new it(...) or extend the existing test to supply those autonomy_* inputs
via createForm and assert the payload.autonomy contains the corresponding keys
with the correct types/serializations (e.g., boolean flags and a parsed array
for auto_approve) so buildPayloadForSection correctly nests and serializes the
new fields.
In `@clients/web/apps/dashboard/src/composables/useConfig.ts`:
- Around line 772-815: processSaveResponse currently preserves only
pendingWebhookSecret across the connectGateway() call, causing unsaved secret
edits for Brave, computer-use, Composio, and Cerebro to be lost; capture the
pending mode/value pairs for web_search_brave_api_key,
browser_computer_use_api_key, composio_api_key, and memory_cerebro_auth_token
before calling connectGateway() (similar to pendingWebhookSecret), call
connectGateway(), and then restore each pending pair back into form (e.g.,
form.web_search_brave_api_key_mode/value,
form.browser_computer_use_api_key_mode/value, form.composio_api_key_mode/value,
form.memory_cerebro_auth_token_mode/value) if they were present so unrelated
saves don’t wipe in-progress secret edits in processSaveResponse.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: d98cf928-7b77-4fe1-a801-3889b1bc0048
⛔ Files ignored due to path filters (1)
clients/web/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (76)
clients/agent-runtime/src/gateway/admin.rsclients/agent-runtime/src/gateway/mod.rsclients/androidApp/buildscript-gradle.lockfileclients/androidApp/gradle.lockfileclients/composeApp/buildscript-gradle.lockfileclients/web/CSS_ARCHITECTURE.mdclients/web/apps/chat/src/App.spec.tsclients/web/apps/chat/src/App.vueclients/web/apps/chat/src/components/HealthIndicator.spec.tsclients/web/apps/chat/src/components/HealthIndicator.vueclients/web/apps/chat/src/components/chat/ChatMessage.vueclients/web/apps/chat/src/components/chat/ToolApprovalCard.spec.tsclients/web/apps/chat/src/components/chat/ToolApprovalCard.vueclients/web/apps/chat/src/composables/useChat.spec.tsclients/web/apps/chat/src/composables/useChat.tsclients/web/apps/chat/src/style.cssclients/web/apps/chat/src/types/chat.tsclients/web/apps/dashboard/src/App.spec.tsclients/web/apps/dashboard/src/App.vueclients/web/apps/dashboard/src/components/config/BrowserSettings.spec.tsclients/web/apps/dashboard/src/components/config/BrowserSettings.vueclients/web/apps/dashboard/src/components/config/ChannelsOverview.spec.tsclients/web/apps/dashboard/src/components/config/ChannelsOverview.vueclients/web/apps/dashboard/src/components/config/ComposioSettings.spec.tsclients/web/apps/dashboard/src/components/config/ComposioSettings.vueclients/web/apps/dashboard/src/components/config/CostOverview.spec.tsclients/web/apps/dashboard/src/components/config/CostOverview.vueclients/web/apps/dashboard/src/components/config/HealthDashboard.spec.tsclients/web/apps/dashboard/src/components/config/HealthDashboard.vueclients/web/apps/dashboard/src/components/config/HeartbeatOverview.spec.tsclients/web/apps/dashboard/src/components/config/HeartbeatOverview.vueclients/web/apps/dashboard/src/components/config/McpOverview.spec.tsclients/web/apps/dashboard/src/components/config/McpOverview.vueclients/web/apps/dashboard/src/components/config/MemorySettings.spec.tsclients/web/apps/dashboard/src/components/config/MemorySettings.vueclients/web/apps/dashboard/src/components/config/ReliabilityOverview.spec.tsclients/web/apps/dashboard/src/components/config/ReliabilityOverview.vueclients/web/apps/dashboard/src/components/config/SchedulerStatus.spec.tsclients/web/apps/dashboard/src/components/config/SchedulerStatus.vueclients/web/apps/dashboard/src/components/config/SecuritySettings.spec.tsclients/web/apps/dashboard/src/components/config/SecuritySettings.vueclients/web/apps/dashboard/src/components/config/TunnelOverview.spec.tsclients/web/apps/dashboard/src/components/config/TunnelOverview.vueclients/web/apps/dashboard/src/components/config/UpdateSettings.spec.tsclients/web/apps/dashboard/src/components/config/UpdateSettings.vueclients/web/apps/dashboard/src/components/config/WebSearchSettings.spec.tsclients/web/apps/dashboard/src/components/config/WebSearchSettings.vueclients/web/apps/dashboard/src/composables/configPayload.spec.tsclients/web/apps/dashboard/src/composables/configPayload.tsclients/web/apps/dashboard/src/composables/useConfig.tsclients/web/apps/dashboard/src/style.cssclients/web/apps/dashboard/src/test/adminConfigFormFactory.tsclients/web/apps/dashboard/src/types/admin-config.tsclients/web/apps/docs/src/content/docs/es/guides/css-architecture.mdclients/web/apps/docs/src/content/docs/guides/css-architecture.mdclients/web/apps/docs/src/styles/custom.cssclients/web/apps/marketing/src/styles/global.cssclients/web/package.jsonclients/web/packages/locales/src/en.jsonclients/web/packages/locales/src/es.jsonclients/web/packages/shared/README.mdclients/web/packages/shared/app-shell.cssclients/web/packages/shared/base.cssclients/web/packages/shared/package.jsonclients/web/packages/shared/theme.cssclients/web/stylelint.config.mjsgradle/libs.versions.tomlopenspec/changes/archive/2026-03-28-web-operational-parity/archive-report.mdopenspec/changes/archive/2026-03-28-web-operational-parity/design.mdopenspec/changes/archive/2026-03-28-web-operational-parity/exploration.mdopenspec/changes/archive/2026-03-28-web-operational-parity/proposal.mdopenspec/changes/archive/2026-03-28-web-operational-parity/specs/web-operational-parity/spec.mdopenspec/changes/archive/2026-03-28-web-operational-parity/tasks.mdopenspec/changes/archive/2026-03-28-web-operational-parity/verify-report.mdopenspec/specs/web-operational-parity/spec.mdpnpm-workspace.yaml
Contributor
There was a problem hiding this comment.
Actionable comments posted: 22
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
clients/web/packages/shared/index.ts (1)
49-51: 🧹 Nitpick | 🔵 TrivialInconsistent trailing-slash removal: regex still used here.
resolvePublicUrluses.replace(/\/$/, "")whiletrimTrailingSlasheswas added to avoid regex-based removal. Consider using the new helper for consistency and to fully mitigate S5852 concerns across the module.Proposed fix
try { - return new URL(candidate).toString().replace(/\/$/, ""); + return trimTrailingSlashes(new URL(candidate).toString()); } catch {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/web/packages/shared/index.ts` around lines 49 - 51, The try block in resolvePublicUrl still uses a regex-based trailing-slash removal (.replace(/\/$/, "")) which is inconsistent with and defeats the purpose of the new trimTrailingSlashes helper; update resolvePublicUrl to call trimTrailingSlashes on the URL string returned by new URL(candidate).toString() (and ensure trimTrailingSlashes is imported/visible to that module) so all trailing-slash trimming uses the new helper instead of a regex.
♻️ Duplicate comments (4)
clients/web/apps/dashboard/src/components/config/ComposioSettings.spec.ts (1)
110-134:⚠️ Potential issue | 🟡 MinorMake the
unchanged -> cleartest proving as well.At Line 113, the test does not seed a non-empty
composio_api_key_value, so Line 131 can pass even if the clear behavior regresses for this transition.Suggested test adjustment
- it("clears api key value when mode changes to clear", async () => { + it("clears non-empty api key value when mode changes from unchanged to clear", async () => { const wrapper = mount(ComposioSettings, { props: { modelValue: createAdminConfigForm({ composio_api_key_mode: "unchanged", + composio_api_key_value: "existing-key-789", }), disabled: false, saving: false, },As per coding guidelines, "Security first, performance second. Validate input boundaries, auth/authz implications, and secret management."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/web/apps/dashboard/src/components/config/ComposioSettings.spec.ts` around lines 110 - 134, The test in ComposioSettings.spec.ts doesn't seed a non-empty composio_api_key_value so the "unchanged -> clear" behavior can pass even if clearing regresses; update the test that mounts ComposioSettings (using createAdminConfigForm) to initialize modelValue with composio_api_key_mode: "unchanged" AND a non-empty composio_api_key_value (e.g., "secret"), then change the select to "clear" and assert the emitted update.modelValue contains composio_api_key_mode: "clear" and composio_api_key_value: "" to prove the clear transition works.clients/web/apps/dashboard/src/components/config/WebSearchSettings.spec.ts (1)
103-128:⚠️ Potential issue | 🟡 MinorAdd the
replace -> unchangedreset case.
updateSecretMode()clearsweb_search_brave_api_key_valuefor every mode other than"replace", but this spec only locks down the"clear"branch. Add the"unchanged"transition too so both reset paths stay covered.As per coding guidelines, “Look for behavioral regressions, missing tests, and contract breaks across modules.”Suggested test
it("clears brave api key value when switching from replace to clear", async () => { const wrapper = mount(WebSearchSettings, { props: { modelValue: createAdminConfigForm({ web_search_brave_api_key_mode: "replace", web_search_brave_api_key_value: "brave-key-789", }), disabled: false, saving: false, }, global: { plugins: [createI18n({ ...i18nConfig, locale: "en" })], }, }); await wrapper.get('[data-testid="web_search_brave_api_key_mode"]').setValue("clear"); const updates = wrapper.emitted("update:modelValue"); expect(updates).toHaveLength(1); expect(updates?.[0]?.[0]).toEqual( expect.objectContaining({ web_search_brave_api_key_mode: "clear", web_search_brave_api_key_value: "", }) ); }); + + it("clears brave api key value when switching from replace to unchanged", async () => { + const wrapper = mount(WebSearchSettings, { + props: { + modelValue: createAdminConfigForm({ + web_search_brave_api_key_mode: "replace", + web_search_brave_api_key_value: "brave-key-789", + }), + disabled: false, + saving: false, + }, + global: { + plugins: [createI18n({ ...i18nConfig, locale: "en" })], + }, + }); + + await wrapper.get('[data-testid="web_search_brave_api_key_mode"]').setValue("unchanged"); + + expect(wrapper.emitted("update:modelValue")?.[0]?.[0]).toEqual( + expect.objectContaining({ + web_search_brave_api_key_mode: "unchanged", + web_search_brave_api_key_value: "", + }) + ); + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/web/apps/dashboard/src/components/config/WebSearchSettings.spec.ts` around lines 103 - 128, The test currently covers the "replace -> clear" branch but misses the "replace -> unchanged" branch: update the test suite in WebSearchSettings.spec.ts (the existing it block or by adding a new it) to also simulate setting the select with data-testid "web_search_brave_api_key_mode" to "unchanged" and assert that the emitted update:modelValue contains web_search_brave_api_key_mode: "unchanged" and web_search_brave_api_key_value: "" (since updateSecretMode() clears non-"replace" values); reference updateSecretMode() and the select with data-testid "web_search_brave_api_key_mode" when locating where to add the assertion.clients/web/apps/dashboard/src/components/config/WebSearchSettings.vue (1)
24-45:⚠️ Potential issue | 🟠 MajorClose the malformed-number gap in
updateField().
type="number"inputs can still surface exponent/partial strings. This helper only normalizes values thatNumber()accepts as finite, so malformed values can still be emitted raw here and then be silently dropped or coerced whenbuildWebSearchPayload()reparses them inclients/web/apps/dashboard/src/composables/configPayload.ts:265-289.As per coding guidelines, “Validate input boundaries, auth/authz implications, and secret management.”Suggested fix
function updateField<Key extends keyof AdminConfigForm>( key: Key, value: AdminConfigForm[Key] ): void { let clamped = value; - if (key === "web_search_max_results") { - const n = Number(value); - if (Number.isFinite(n)) { - clamped = `${Math.max(1, Math.min(10, Math.round(n)))}` as AdminConfigForm[Key]; - } - } - if (key === "web_search_timeout_secs") { - const n = Number(value); - if (Number.isFinite(n)) { - clamped = `${Math.max(1, Math.round(n))}` as AdminConfigForm[Key]; - } + if (key === "web_search_max_results" || key === "web_search_timeout_secs") { + const raw = String(value).trim(); + if (!/^\d+$/.test(raw)) return; + const parsed = Number.parseInt(raw, 10); + clamped = ( + key === "web_search_max_results" + ? Math.max(1, Math.min(10, parsed)) + : Math.max(1, parsed) + ).toString() as AdminConfigForm[Key]; } emit("update:modelValue", { ...props.modelValue, [key]: clamped, });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/web/apps/dashboard/src/components/config/WebSearchSettings.vue` around lines 24 - 45, The updateField function currently emits raw non-finite numeric strings (e.g., partial/exponent input) for keys "web_search_max_results" and "web_search_timeout_secs"; change the logic in updateField so that when Number(value) is not finite you do NOT emit the raw malformed string—either return early (no emit) or set clamped to a safe normalized value such as an empty string ('' as AdminConfigForm[Key]) before emitting; update the branches that handle "web_search_max_results" and "web_search_timeout_secs" accordingly so malformed inputs are filtered here instead of relying on buildWebSearchPayload to reparse them.clients/web/apps/dashboard/src/components/config/ChannelsOverview.vue (1)
35-36: 🧹 Nitpick | 🔵 TrivialDead code branch and missing payload validation.
Per the backend (
handle_admin_channels), the response is always{ "channels": [...] }. TheArray.isArray(data)branch never executes. Remove it and add basic shape validation.Proposed fix
const data = await res.json(); - channels.value = Array.isArray(data) ? data : (data.channels ?? []); + if (!data || typeof data !== "object" || !Array.isArray(data.channels)) { + throw new Error("Invalid response shape"); + } + channels.value = data.channels;As per coding guidelines, "Look for behavioral regressions, missing tests, and contract breaks across modules."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/web/apps/dashboard/src/components/config/ChannelsOverview.vue` around lines 35 - 36, The current response handling in ChannelsOverview.vue assigns channels.value from a branch that never occurs; remove the Array.isArray(data) branch and instead validate the payload shape returned by the backend: after const data = await res.json(), check that data is an object and that Array.isArray(data.channels) is true, then set channels.value = data.channels; otherwise set channels.value = [] (and optionally log or surface an error). Update the assignment logic around channels.value to only accept the expected { channels: [...] } shape.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@clients/web/apps/chat/src/App.vue`:
- Around line 151-166: The stream handler currently overwrites the assistant
message on each SSE chunk; change the callback passed to chat.streamMessage so
it appends each chunk to the existing content instead of replacing it (use
messages.value.find(m => m.id === assistantMessageId)?.content or maintain a
local buffer to concatenate previous content + chunk before calling
updateAssistantMessage with status "streaming"), then keep the final
updateAssistantMessage call that sets status "complete" using the assembled
content; also add a new App.spec.ts test that simulates streamMessage emitting
at least two chunk events to assert that the final assistant message contains
the concatenation of both chunks and that status transitions from "streaming" to
"complete".
In `@clients/web/apps/chat/src/composables/useChat.ts`:
- Around line 321-334: The 403 webhook branch only checks for error.code ===
"approval_required" and mistakenly treats other approval-shaped errors as
invalid credentials; update the handler in useChat.ts (the response.status ===
403 block) to parse the error body with the same logic used by streamMessage() —
call parseApprovalErrorType(errorData) (or equivalent parser) and accept
returned approval types including "approval_contract" as valid approval
responses, returning the appropriate object (tool, reason, sessionId/default to
currentSessionId.value) instead of calling gateway.markCredentialInvalid() and
clearSession(); only mark credentials invalid when the parsed result is
definitively a non-approval auth error.
In `@clients/web/apps/dashboard/src/components/config/BrowserSettings.spec.ts`:
- Line 46: Remove the unnecessary async modifiers from the four test
declarations that don't await anything: the it blocks with descriptions "shows
password input when computer use api key mode is replace", "shows password input
when browser use api key mode is replace" (line ~64), "shows api key input when
browser use api key mode is configure" (line ~81), and "shows api key input when
computer use api key mode is configure" (line ~108); edit those it(...)
declarations in BrowserSettings.spec.ts to drop the async keyword so each
becomes a plain synchronous test function.
In `@clients/web/apps/dashboard/src/components/config/ChannelsOverview.vue`:
- Around line 50-51: In ChannelsOverview.vue, the status messages for loading
and error lack ARIA live regions; update the <p v-if="loading" class="helper">{{
t("channels.loading") }}</p> to include aria-live="polite" and role="status"
(and aria-atomic="true" if desired), and update the <p v-else-if="error"
class="error">{{ error }}</p> to include aria-live="assertive" and role="alert"
(or role="status" with aria-live="assertive") so the loading and error states
(variables loading and error) are announced like in CostOverview.vue.
In `@clients/web/apps/dashboard/src/components/config/ComposioSettings.spec.ts`:
- Around line 48-81: Add a regression test to ComposioSettings.spec.ts that
mounts ComposioSettings with modelValue from createAdminConfigForm where
composio_api_key_mode is "unchanged" and composio_has_api_key is true (and
composio_api_key_value is set to a non-empty string), then assert that the input
with data-testid="composio_api_key_value" does NOT render or display the secret
value; ensure the test references ComposioSettings, composio_api_key_mode,
composio_has_api_key, and data-testid="composio_api_key_value" so it fails if
the component accidentally exposes the secret in "unchanged" mode.
In `@clients/web/apps/dashboard/src/components/config/CostOverview.vue`:
- Around line 63-70: The UI currently prefixes USD values with a hardcoded "$"
in CostOverview.vue (see cost.daily_limit_usd and cost.monthly_limit_usd);
replace this with a proper currency formatter using Intl.NumberFormat (e.g.,
create a formatCurrency helper or computed method used where these values are
rendered) and pass the currency code (USD) so values are localized for number
formatting and currency symbol placement instead of string-concatenating "$".
In `@clients/web/apps/dashboard/src/components/config/HealthDashboard.vue`:
- Around line 77-90: The decorative status indicator spans (the element with
class "health-indicator" bound to overallStatus and the span with class
"component-indicator" inside the v-for over health.components) should be marked
aria-hidden="true" so screen readers ignore them; update the two spans (the one
using :class="overallStatus" and the one using :class="comp.status === 'ok' ?
'ok' : 'error'") to include aria-hidden="true" while keeping existing bindings
and attributes like :class and :data-testid intact.
- Around line 73-74: Update the two conditional paragraphs in
HealthDashboard.vue (the <p v-if="loading" class="helper">{{ t("health.loading")
}}</p> and the <p v-else-if="error" class="error">{{ error }}</p>) to include
appropriate ARIA announcements: add aria-live="polite" and role="status" for the
loading paragraph (loading variable / t("health.loading")), and add
aria-live="assertive" and role="alert" for the error paragraph (error variable),
ensuring the helper and error classes are preserved.
- Around line 64-67: The polling interval isn't reset when gatewayUrl or
bearerToken change; update the watcher around watch(() => [props.gatewayUrl,
props.bearerToken], fetchHealth, { immediate: true }) to also clear any existing
interval and recreate pollInterval after calling fetchHealth: keep a let
pollInterval variable in the outer scope, inside the watcher
clearInterval(pollInterval) if set, call fetchHealth (or rely on the immediate
option) and then assign pollInterval = setInterval(fetchHealth, 30_000); keep
the existing onUnmounted(() => clearInterval(pollInterval)) to clean up when the
component is destroyed.
In `@clients/web/apps/dashboard/src/components/config/HeartbeatOverview.vue`:
- Around line 50-51: HeartbeatOverview.vue currently renders the loading and
error <p> nodes without live-region semantics; update the loading paragraph (the
element with v-if="loading") to include aria-live="polite" and role="status" and
update the error paragraph (the element with v-else-if="error") to include
aria-live="assertive" and role="alert", keeping the existing class names (helper
and error) so styling remains the same and screen readers announce changes
consistently like CostOverview.vue.
In `@clients/web/apps/dashboard/src/components/config/McpOverview.vue`:
- Around line 19-44: The current fetchMcp function can have out-of-order
responses overwrite state when props.gatewayUrl or props.bearerToken change; fix
it by making requests cancellable and cancelling any prior request before
starting a new one: add a component-scoped AbortController (e.g.,
activeFetchController) or a monotonic request id (e.g., fetchCounter /
activeFetchId), abort/invalidate the previous controller/id at the start of
fetchMcp, create a new controller/id for this invocation, pass controller.signal
to the fetch call, and before assigning mcp.value, error.value, or loading.value
ensure the controller/id still matches the active one so stale responses cannot
overwrite the current state (reference fetchMcp, mcp, error, loading,
props.gatewayUrl, props.bearerToken).
In `@clients/web/apps/dashboard/src/components/config/MemorySettings.spec.ts`:
- Around line 84-108: The test for MemorySettings doesn't prove the token was
cleared because it starts with memory_cerebro_auth_token_mode "unchanged" and an
already-empty memory_cerebro_auth_token_value; update the test setup in
MemorySettings.spec.ts so the mounted props use createAdminConfigForm with
memory_cerebro_auth_token_mode: "replace" and a non-empty
memory_cerebro_auth_token_value (e.g., "secret") before setting the input to
"clear", then assert the emitted update for modelValue contains
memory_cerebro_auth_token_mode: "clear" and memory_cerebro_auth_token_value: ""
to verify the component actually cleared the token.
In `@clients/web/apps/dashboard/src/components/config/ReliabilityOverview.vue`:
- Around line 57-92: Replace hardcoded unit suffixes in ReliabilityOverview.vue
for the reliability fields (provider_backoff_ms, channel_initial_backoff_secs,
channel_max_backoff_secs, scheduler_poll_secs) with i18n translation keys
similar to HeartbeatOverview (e.g., use t("heartbeat.unit") or dedicated keys
like t("reliability.unit.ms") / t("reliability.unit.s") as appropriate). Update
the displayed strings for provider_backoff_ms, channel_initial_backoff_secs,
channel_max_backoff_secs, and scheduler_poll_secs to concatenate the localized
unit via t(...) instead of literal "ms" or "s", and add any new translation keys
used to the locales so the units render correctly in other languages.
- Around line 50-51: In ReliabilityOverview.vue update the status message <p>
elements so the loading paragraph (v-if="loading") has aria-live="polite" and
role="status" and the error paragraph (v-else-if="error") has
aria-live="assertive" and role="alert" to match the pattern used in
CostOverview.vue; locate the two <p> elements in the template (the ones using
v-if="loading" and v-else-if="error") and add these attributes to improve
accessibility consistency.
In `@clients/web/apps/dashboard/src/components/config/SchedulerStatus.vue`:
- Around line 71-74: The scheduler.task_count field in SchedulerStatus.vue
always shows 0 (scheduler.task_count) which is misleading; update the template
to conditionally render the status-item for task count (the span using
t("scheduler.taskCount") and scheduler.task_count) so that if
scheduler.task_count is null/0 it is either hidden or replaced with a static
"coming soon" / "not available" annotation/badge, ensuring the UI shows the real
value only when the backend provides it.
- Around line 55-58: The decorative status indicator span in SchedulerStatus.vue
(the element with class "status-indicator" and :class bound to
scheduler.enabled) should be hidden from assistive technology; add
aria-hidden="true" to that span so screen readers ignore it (consistent with
ChannelsOverview.vue) while keeping the existing class and :class binding
intact.
- Around line 50-51: Update the two conditional paragraphs in
SchedulerStatus.vue: add accessibility live region attributes to the loading and
error messages by setting aria-live="polite" (and role="status" if desired) on
the loading paragraph rendered by <p v-if="loading" ...> and
aria-live="assertive" (and role="alert" or role="status") on the error paragraph
rendered by <p v-else-if="error" ...>; keep existing Tailwind classes (e.g.,
"helper" and "error") and the translation call t("scheduler.loading")/error
binding intact so only ARIA attributes are added to the existing <p> elements.
In `@clients/web/apps/dashboard/src/components/config/TunnelOverview.vue`:
- Around line 50-52: In TunnelOverview.vue update the template conditionals to
improve A11y and show an explicit empty state: add aria-live="polite" (and
role="status" if appropriate) to the loading <p class="helper"> and
aria-live="assertive" to the error <p class="error"> so screen readers announce
changes, and replace the current v-else-if="tunnel" branch with explicit
branches that handle three cases—when loading, when error, when tunnel exists
(render .status-grid as before) and when the fetch succeeded but
data.config?.tunnel is null render an explicit empty-state <p> (e.g.,
class="helper" or a new empty-state class) with a translatable message—update
references to the component props/computed names (loading, error, tunnel or
data.config?.tunnel) and ensure Tailwind classes follow project guidelines.
In `@clients/web/apps/dashboard/src/components/config/UpdateSettings.spec.ts`:
- Around line 42-43: The test "renders fallback values when updates data is
null" is inconsistent because the fixture uses an empty object (const config:
AdminConfigView = {}) rather than a true null updates value; either rename the
test to "missing updates data" to match the fixture or change the fixture to
include updates: null so the test actually covers the null API case—locate the
UpdateSettings.spec.ts test case (the it(...) block) and update the test name or
the config fixture accordingly to ensure the test intent and input match (use
the test title "missing updates data" if you keep {} or set config.updates =
null if you want to assert null handling).
- Around line 28-35: Tests duplicate the mount + i18n setup for UpdateSettings;
extract a shared helper (e.g., a function mountWithI18n or mountUpdateSettings)
that wraps mount(UpdateSettings, { props, global: { plugins: [createI18n({
...i18nConfig, locale: "en" })] } }) and accept the config/props to pass
through, then replace each direct mount call in UpdateSettings.spec.ts with the
helper to keep fixtures focused and reduce repetition.
- Around line 37-39: Replace the brittle wrapper.text() assertions in
UpdateSettings.spec.ts with targeted selector checks: instead of
expect(wrapper.text()).toContain("1.2.3") and
expect(wrapper.text()).toContain("1.3.0"), use
wrapper.find('[data-testid="current-version"]').text() and
wrapper.find('[data-testid="available-version"]').text() (or add those
data-testid attributes to the corresponding version elements in the component)
and assert their text equals/contains the expected versions; keep the existing
button assertion using wrapper.find('button[data-testid="save"]').exists()
as-is.
In `@clients/web/apps/dashboard/src/composables/configPayload.spec.ts`:
- Around line 483-500: Add a new unit test in the same spec that uses
buildPayloadForSection (and thus exercises buildSecurityPayload) to flip the two
new security booleans by passing a form with
autonomy_require_approval_for_medium_risk and autonomy_block_high_risk_commands
set to truthy values (e.g., "on"/"true") along with the existing
auto_approve/always_ask values; assert the returned payload includes
autonomy.require_approval_for_medium_risk and autonomy.block_high_risk_commands
booleans set appropriately (true/false) so the test fails if
buildSecurityPayload does not map those form fields into the payload.
---
Outside diff comments:
In `@clients/web/packages/shared/index.ts`:
- Around line 49-51: The try block in resolvePublicUrl still uses a regex-based
trailing-slash removal (.replace(/\/$/, "")) which is inconsistent with and
defeats the purpose of the new trimTrailingSlashes helper; update
resolvePublicUrl to call trimTrailingSlashes on the URL string returned by new
URL(candidate).toString() (and ensure trimTrailingSlashes is imported/visible to
that module) so all trailing-slash trimming uses the new helper instead of a
regex.
---
Duplicate comments:
In `@clients/web/apps/dashboard/src/components/config/ChannelsOverview.vue`:
- Around line 35-36: The current response handling in ChannelsOverview.vue
assigns channels.value from a branch that never occurs; remove the
Array.isArray(data) branch and instead validate the payload shape returned by
the backend: after const data = await res.json(), check that data is an object
and that Array.isArray(data.channels) is true, then set channels.value =
data.channels; otherwise set channels.value = [] (and optionally log or surface
an error). Update the assignment logic around channels.value to only accept the
expected { channels: [...] } shape.
In `@clients/web/apps/dashboard/src/components/config/ComposioSettings.spec.ts`:
- Around line 110-134: The test in ComposioSettings.spec.ts doesn't seed a
non-empty composio_api_key_value so the "unchanged -> clear" behavior can pass
even if clearing regresses; update the test that mounts ComposioSettings (using
createAdminConfigForm) to initialize modelValue with composio_api_key_mode:
"unchanged" AND a non-empty composio_api_key_value (e.g., "secret"), then change
the select to "clear" and assert the emitted update.modelValue contains
composio_api_key_mode: "clear" and composio_api_key_value: "" to prove the clear
transition works.
In `@clients/web/apps/dashboard/src/components/config/WebSearchSettings.spec.ts`:
- Around line 103-128: The test currently covers the "replace -> clear" branch
but misses the "replace -> unchanged" branch: update the test suite in
WebSearchSettings.spec.ts (the existing it block or by adding a new it) to also
simulate setting the select with data-testid "web_search_brave_api_key_mode" to
"unchanged" and assert that the emitted update:modelValue contains
web_search_brave_api_key_mode: "unchanged" and web_search_brave_api_key_value:
"" (since updateSecretMode() clears non-"replace" values); reference
updateSecretMode() and the select with data-testid
"web_search_brave_api_key_mode" when locating where to add the assertion.
In `@clients/web/apps/dashboard/src/components/config/WebSearchSettings.vue`:
- Around line 24-45: The updateField function currently emits raw non-finite
numeric strings (e.g., partial/exponent input) for keys "web_search_max_results"
and "web_search_timeout_secs"; change the logic in updateField so that when
Number(value) is not finite you do NOT emit the raw malformed string—either
return early (no emit) or set clamped to a safe normalized value such as an
empty string ('' as AdminConfigForm[Key]) before emitting; update the branches
that handle "web_search_max_results" and "web_search_timeout_secs" accordingly
so malformed inputs are filtered here instead of relying on
buildWebSearchPayload to reparse them.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 652937b3-1e16-478b-b700-434320a1f73f
📒 Files selected for processing (26)
clients/agent-runtime/src/channels/media.rsclients/agent-runtime/src/providers/router.rsclients/web/apps/chat/src/App.spec.tsclients/web/apps/chat/src/App.vueclients/web/apps/chat/src/components/HealthIndicator.spec.tsclients/web/apps/chat/src/components/HealthIndicator.vueclients/web/apps/chat/src/composables/useChat.spec.tsclients/web/apps/chat/src/composables/useChat.tsclients/web/apps/dashboard/src/components/config/BrowserSettings.spec.tsclients/web/apps/dashboard/src/components/config/ChannelsOverview.vueclients/web/apps/dashboard/src/components/config/ComposioSettings.spec.tsclients/web/apps/dashboard/src/components/config/CostOverview.vueclients/web/apps/dashboard/src/components/config/HealthDashboard.vueclients/web/apps/dashboard/src/components/config/HeartbeatOverview.vueclients/web/apps/dashboard/src/components/config/McpOverview.spec.tsclients/web/apps/dashboard/src/components/config/McpOverview.vueclients/web/apps/dashboard/src/components/config/MemorySettings.spec.tsclients/web/apps/dashboard/src/components/config/ReliabilityOverview.vueclients/web/apps/dashboard/src/components/config/SchedulerStatus.vueclients/web/apps/dashboard/src/components/config/TunnelOverview.vueclients/web/apps/dashboard/src/components/config/UpdateSettings.spec.tsclients/web/apps/dashboard/src/components/config/WebSearchSettings.spec.tsclients/web/apps/dashboard/src/components/config/WebSearchSettings.vueclients/web/apps/dashboard/src/composables/configPayload.spec.tsclients/web/packages/shared/index.tsopenspec/specs/provider-vision-gating/spec.md
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: sonar
- GitHub Check: Cloudflare Pages
🧰 Additional context used
📓 Path-based instructions (8)
**/*
⚙️ CodeRabbit configuration file
**/*: Security first, performance second.
Validate input boundaries, auth/authz implications, and secret management.
Look for behavioral regressions, missing tests, and contract breaks across modules.
Files:
clients/web/apps/chat/src/App.spec.tsclients/web/packages/shared/index.tsclients/web/apps/dashboard/src/components/config/HeartbeatOverview.vueclients/web/apps/dashboard/src/components/config/BrowserSettings.spec.tsclients/web/apps/dashboard/src/components/config/UpdateSettings.spec.tsclients/web/apps/dashboard/src/components/config/MemorySettings.spec.tsclients/web/apps/dashboard/src/components/config/McpOverview.spec.tsclients/web/apps/dashboard/src/components/config/ComposioSettings.spec.tsclients/web/apps/chat/src/components/HealthIndicator.vueclients/web/apps/dashboard/src/components/config/TunnelOverview.vueclients/web/apps/dashboard/src/components/config/CostOverview.vueclients/web/apps/dashboard/src/components/config/SchedulerStatus.vueclients/web/apps/chat/src/components/HealthIndicator.spec.tsclients/agent-runtime/src/providers/router.rsclients/web/apps/dashboard/src/components/config/WebSearchSettings.vueclients/web/apps/chat/src/composables/useChat.spec.tsclients/web/apps/dashboard/src/components/config/HealthDashboard.vueclients/web/apps/dashboard/src/components/config/McpOverview.vueclients/agent-runtime/src/channels/media.rsclients/web/apps/dashboard/src/components/config/WebSearchSettings.spec.tsclients/web/apps/dashboard/src/components/config/ReliabilityOverview.vueclients/web/apps/chat/src/App.vueclients/web/apps/dashboard/src/composables/configPayload.spec.tsclients/web/apps/dashboard/src/components/config/ChannelsOverview.vueclients/web/apps/chat/src/composables/useChat.tsopenspec/specs/provider-vision-gating/spec.md
**/*.vue
⚙️ CodeRabbit configuration file
**/*.vue: Enforce Vue 3 Composition API with <script setup>.
Ensure accessibility (A11y) and proper use of Tailwind CSS classes.
Check for proper prop validation and emitted events documentation.
Files:
clients/web/apps/dashboard/src/components/config/HeartbeatOverview.vueclients/web/apps/chat/src/components/HealthIndicator.vueclients/web/apps/dashboard/src/components/config/TunnelOverview.vueclients/web/apps/dashboard/src/components/config/CostOverview.vueclients/web/apps/dashboard/src/components/config/SchedulerStatus.vueclients/web/apps/dashboard/src/components/config/WebSearchSettings.vueclients/web/apps/dashboard/src/components/config/HealthDashboard.vueclients/web/apps/dashboard/src/components/config/McpOverview.vueclients/web/apps/dashboard/src/components/config/ReliabilityOverview.vueclients/web/apps/chat/src/App.vueclients/web/apps/dashboard/src/components/config/ChannelsOverview.vue
clients/agent-runtime/src/providers/**/*.rs
📄 CodeRabbit inference engine (clients/agent-runtime/AGENTS.md)
Implement
Providertrait insrc/providers/and register insrc/providers/mod.rsfactory when adding a new provider
Files:
clients/agent-runtime/src/providers/router.rs
clients/agent-runtime/src/**/*.rs
📄 CodeRabbit inference engine (clients/agent-runtime/AGENTS.md)
clients/agent-runtime/src/**/*.rs: Never log secrets, tokens, raw credentials, or sensitive payloads in any logging statements
Avoid unnecessary allocations, clones, and blocking operations to maintain performance and efficiency
Files:
clients/agent-runtime/src/providers/router.rsclients/agent-runtime/src/channels/media.rs
clients/agent-runtime/**/*.rs
📄 CodeRabbit inference engine (clients/agent-runtime/AGENTS.md)
Run
cargo fmt --all -- --check,cargo clippy --all-targets -- -D warnings, andcargo testfor code validation, or document which checks were skipped and why
Files:
clients/agent-runtime/src/providers/router.rsclients/agent-runtime/src/channels/media.rs
**/*.rs
⚙️ CodeRabbit configuration file
**/*.rs: Focus on Rust idioms, memory safety, and ownership/borrowing correctness.
Flag unnecessary clones, unchecked panics in production paths, and weak error context.
Prioritize unsafe blocks, FFI boundaries, concurrency races, and secret handling.
Files:
clients/agent-runtime/src/providers/router.rsclients/agent-runtime/src/channels/media.rs
clients/agent-runtime/src/channels/**/*.rs
📄 CodeRabbit inference engine (clients/agent-runtime/AGENTS.md)
Implement
Channeltrait insrc/channels/with consistentsend,listen, andhealth_checksemantics and cover auth/allowlist/health behavior with tests
Files:
clients/agent-runtime/src/channels/media.rs
**/*.{md,mdx}
⚙️ CodeRabbit configuration file
**/*.{md,mdx}: Verify technical accuracy and that docs stay aligned with code changes.
For user-facing docs, check EN/ES parity or explicitly note pending translation gaps.
Files:
openspec/specs/provider-vision-gating/spec.md
🧠 Learnings (5)
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/src/providers/**/*.rs : Implement `Provider` trait in `src/providers/` and register in `src/providers/mod.rs` factory when adding a new provider
Applied to files:
clients/agent-runtime/src/providers/router.rs
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/src/main.rs : Preserve CLI contract unless change is intentional and documented; prefer explicit errors over silent fallback for unsupported critical paths
Applied to files:
clients/agent-runtime/src/providers/router.rsclients/agent-runtime/src/channels/media.rsopenspec/specs/provider-vision-gating/spec.md
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/src/channels/**/*.rs : Implement `Channel` trait in `src/channels/` with consistent `send`, `listen`, and `health_check` semantics and cover auth/allowlist/health behavior with tests
Applied to files:
clients/web/apps/chat/src/composables/useChat.spec.ts
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/src/{security,gateway,tools,config}/**/*.rs : Do not silently weaken security policy or access constraints; keep default behavior secure-by-default with deny-by-default where applicable
Applied to files:
clients/web/apps/dashboard/src/components/config/McpOverview.vue
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/src/{security,gateway,tools}/**/*.rs : Treat `src/security/`, `src/gateway/`, `src/tools/` as high-risk surfaces and never broaden filesystem/network execution scope without explicit policy checks
Applied to files:
clients/web/apps/dashboard/src/components/config/McpOverview.vueclients/web/apps/dashboard/src/components/config/ChannelsOverview.vue
🪛 LanguageTool
openspec/specs/provider-vision-gating/spec.md
[grammar] ~130-~130: Use a hyphen to join words.
Context: ...enario: Trait default rejects image turn on provider without an override - GIVEN...
(QB_NEW_EN_HYPHEN)
🔇 Additional comments (19)
clients/web/apps/dashboard/src/components/config/BrowserSettings.spec.ts (1)
81-106: Security-relevant test correctly added.This test verifies that switching the API key mode from "replace" to "clear" properly zeroes out
browser_computer_use_api_key_value. This addresses the security-relevant secret-clearing behavior previously flagged as untested.clients/web/apps/dashboard/src/components/config/MemorySettings.spec.ts (2)
1-47: Well-structured test setup and emission checks.Good use of
data-testidselectors, proper async handling, and the factory pattern for form creation. The emission assertions correctly verify both field preservation and update behavior.
49-82: Auth token visibility tests cover the security-relevant conditional rendering.Testing both
"replace"(shows input) and"clear"(hides input) modes ensures sensitive token fields aren't exposed when not needed.clients/web/apps/dashboard/src/components/config/UpdateSettings.spec.ts (1)
79-134: Good boolean parity coverage for update availability.Nice addition validating both
update_available: true→ “Yes” andfalse→ “No”, plusauto_install_enabledrendering in the false-path.clients/web/apps/dashboard/src/components/config/ComposioSettings.spec.ts (1)
83-108: Good fix: non-empty secret is now explicitly cleared onreplace -> clear.This test now proves the clearing path with a real non-empty starting value and guards against secret-retention regressions.
clients/agent-runtime/src/providers/router.rs (3)
156-162: Fail-closed image routing is now correctly enforced.Line 155 guards image turns before dispatch, and Line 161 returns
RouteNotImageCapableas a structured rejection.
207-228: Mock call accounting is now precise for router gating tests.Tracking
chat_callsseparately and incrementing in Line 255 makes the “providerchat()not invoked” contract verifiable.Also applies to: 249-263
539-544: Regression test now validates both error type and zero-dispatch behavior.The downcast assertion plus Line 543 call-count check gives strong coverage for router pre-dispatch rejection.
clients/agent-runtime/src/channels/media.rs (1)
46-67: Image rejection taxonomy stays explicit and machine-friendly.The variant-level canonical codes remain clear and deterministic for routing/channel error handling.
openspec/specs/provider-vision-gating/spec.md (2)
119-129: Fail-closed routing requirements are clear and testable.The router/reliable-layer constraints and REQ-5 scenarios define pre-dispatch rejection and explicitly forbid silent image stripping.
Also applies to: 213-234
8-10: Translation parity status is now explicitly documented.This clearly records pending EN/ES scope for the new spec.
As per coding guidelines, "
**/*.{md,mdx}: Verify technical accuracy and that docs stay aligned with code changes. For user-facing docs, check EN/ES parity or explicitly note pending translation gaps."clients/web/packages/shared/index.ts (2)
10-17: LGTM: Safe loop-based trailing slash removal.The implementation correctly avoids ReDoS (S5852) by using end-position scanning instead of regex.
19-40: Solid URL validation with protocol enforcement.The function properly:
- Rejects empty/whitespace input
- Catches malformed URLs
- Enforces http/https protocols
- Sanitizes by clearing search/hash and trimming pathname slashes
clients/web/apps/dashboard/src/components/config/HeartbeatOverview.vue (1)
1-45: Well-structured component with proper URL validation and reactive fetching.Uses
validateGatewayUrlfor security, watches props for reactivity, and handles all UI states (loading/error/null/data).clients/web/apps/dashboard/src/components/config/CostOverview.vue (1)
1-45: Component follows the established pattern with proper reactivity and accessibility.Good use of
validateGatewayUrl, reactivewatch, and ARIA attributes on loading/error states.clients/web/apps/dashboard/src/components/config/ReliabilityOverview.vue (1)
1-45: Solid implementation following the established admin overview pattern.Proper URL validation, reactive prop watching, and comprehensive reliability metrics display.
clients/web/apps/dashboard/src/components/config/TunnelOverview.vue (1)
1-45: Component properly validates URLs and handles optional tunnel fields.Good use of conditional rendering for optional Tailscale/ngrok fields.
clients/web/apps/dashboard/src/components/config/SchedulerStatus.vue (1)
1-45: Component follows the standard pattern with proper URL validation and reactive fetching.clients/web/apps/dashboard/src/components/config/HealthDashboard.vue (1)
1-67: Well-implemented polling health dashboard with proper cleanup.Good use of
onUnmountedto clear the interval, reactive prop watching, and computedoverallStatus.
Address second round of code review findings: Chat app: - Fix stream handler to accumulate chunks instead of overwriting - Broaden 403 handler to accept all approval-shaped errors - Add stream chunk accumulation test Dashboard components: - Add ARIA live regions to all overview loading/error states - Add aria-hidden to all decorative status indicator spans - Reset HealthDashboard polling interval on prop changes - Add AbortController to McpOverview for stale request cancellation - Replace hardcoded currency/unit strings with Intl/i18n - Show 'Not available' for zero task count in SchedulerStatus - Add explicit empty state in TunnelOverview - Reject non-finite numeric inputs in WebSearchSettings - Remove Array.isArray fallback in ChannelsOverview response Tests: - Strengthen secret clearing tests with non-empty initial values - Add replace→unchanged mode transition tests - Add security booleans payload test - Extract mountUpdateSettings helper, use data-testid selectors - Remove unnecessary async from sync test declarations Shared: - Replace regex in resolvePublicUrl with trimTrailingSlashes
Contributor
There was a problem hiding this comment.
Actionable comments posted: 11
♻️ Duplicate comments (6)
clients/web/apps/dashboard/src/components/config/McpOverview.spec.ts (1)
41-57: 🧹 Nitpick | 🔵 TrivialAdd auth-header assertion to the success test for consistency.
The refetch test (lines 87-91) correctly verifies the Authorization header, but the initial success test doesn't. For complete auth contract coverage:
Suggested addition
const wrapper = mountComponent(); await flushPromises(); + const fetchMock = vi.mocked(fetch); + expect(fetchMock).toHaveBeenCalledWith( + expect.stringContaining("/web/admin/config"), + expect.objectContaining({ headers: { Authorization: "Bearer test-token" } }) + ); + expect(wrapper.find('[data-testid="mcp-server-test-server"]').exists()).toBe(true);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/web/apps/dashboard/src/components/config/McpOverview.spec.ts` around lines 41 - 57, The success test uses vi.stubGlobal("fetch", ...) and mounts the component via mountComponent() but doesn't assert the Authorization header; add an assertion after flushPromises() that the stubbed fetch was called with an Authorization header (matching the token used in the test setup) — e.g., inspect vi.fn() call args from the stub created in vi.stubGlobal("fetch") to verify the request headers include Authorization — so that the initial success test mirrors the refetch test's header check and ensures auth contract coverage for the component that uses mockConfig and the '[data-testid="mcp-server-test-server"]' rendering.clients/web/apps/dashboard/src/components/config/ChannelsOverview.vue (1)
35-36:⚠️ Potential issue | 🟠 MajorValidate the
/web/admin/channelspayload before assigning it.
res.json()is untyped here, sodata.channels ?? []will accept any truthychannelsvalue. If that shape drifts, the template will iterate non-AdminChannelStatusViewentries and then dereferencech.channel_type/ch.configuredat runtime. Parse asunknownand require{ channels: AdminChannelStatusView[] }before updatingchannels.value.As per coding guidelines, "Look for behavioral regressions, missing tests, and contract breaks across modules."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/web/apps/dashboard/src/components/config/ChannelsOverview.vue` around lines 35 - 36, Parse the fetch response as unknown and validate its shape before assigning to channels.value: after calling res.json(), assert the result matches the expected type { channels: AdminChannelStatusView[] } (e.g., check it's an object, has a channels array, and each item contains the expected properties like channel_type and configured) and only then set channels.value = parsed.channels; otherwise leave channels.value as [] and log or handle the invalid payload. Ensure you reference the existing symbols res.json(), channels.value, and the AdminChannelStatusView shape when adding the validation.clients/web/apps/dashboard/src/components/config/ComposioSettings.spec.ts (1)
137-154:⚠️ Potential issue | 🟡 MinorProve the secret is absent from the rendered output, not just the input field.
This still passes if
composio_api_key_valueis accidentally echoed somewhere else inunchangedmode. Keep the seeded secret and add a negative text/HTML assertion so the test actually guards against leakage.As per coding guidelines, "Security first, performance second. Validate input boundaries, auth/authz implications, and secret management."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/web/apps/dashboard/src/components/config/ComposioSettings.spec.ts` around lines 137 - 154, The test "hides password input when mode is unchanged even if secret value is present" in ComposioSettings should also assert the secret isn't rendered anywhere in the component output; keep the seeded secret in the props (composio_api_key_value: "my-secret") and add a negative assertion against the rendered HTML/text (e.g., assert wrapper.html() or wrapper.text() does not contain "my-secret") in addition to the existing check that the input with data-testid="composio_api_key_value" does not exist, so the test fails if the secret is echoed elsewhere.clients/web/apps/dashboard/src/components/config/CostOverview.spec.ts (1)
21-50:⚠️ Potential issue | 🟡 MinorAssert the authenticated admin-config request in the success test.
The test currently passes even if the component stops sending the bearer token or hits the wrong route. Add a
fetchassertion for/web/admin/configplus theAuthorizationheader.As per coding guidelines, "Security first, performance second. Validate input boundaries, auth/authz implications, and secret management."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/web/apps/dashboard/src/components/config/CostOverview.spec.ts` around lines 21 - 50, The test "renders cost data on successful fetch" stubs global fetch but doesn't assert the request URL or Authorization header; update this test to assert that fetch was called with the '/web/admin/config' endpoint and that the options include an Authorization header with a Bearer token (use the same auth setup the component uses), e.g., after flushPromises add an assertion that vi's stubbed fetch was invoked for '/web/admin/config' and that the passed options object contains headers.Authorization starting with 'Bearer '; reference the test name and mountComponent/mocked fetch to locate where to add the assertion and keep vi.unstubAllGlobals at the end.clients/web/apps/dashboard/src/components/config/ChannelsOverview.spec.ts (1)
21-49:⚠️ Potential issue | 🟡 MinorAssert the endpoint and bearer header in the success-path test.
This only proves rendering. It will not catch regressions that hit the wrong admin route or drop auth entirely. Add a
fetchassertion for/web/admin/channelsplusAuthorization: Bearer test-token.As per coding guidelines, "Security first, performance second. Validate input boundaries, auth/authz implications, and secret management."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/web/apps/dashboard/src/components/config/ChannelsOverview.spec.ts` around lines 21 - 49, The test "renders channel list on successful fetch" currently stubs fetch but doesn't assert the request details; after mounting via mountComponent() and awaiting flushPromises(), add assertions that the global fetch (stubbed via vi.stubGlobal("fetch", ...)) was called with the admin endpoint "/web/admin/channels" and that the request included an Authorization header "Bearer test-token" (e.g., inspect the first call to the stubbed fetch and assert request url and headers), then keep vi.unstubAllGlobals() as cleanup.clients/web/apps/dashboard/src/components/config/CostOverview.vue (1)
49-50: 🧹 Nitpick | 🔵 TrivialConsider surfacing contract errors when
config.costis missing.The current fallback
data?.config?.cost ?? nullsilently renders nothing when the backend returns a malformed or incomplete response. For operator-facing dashboards, explicitly settingerror.valuewhenconfig.costis absent helps diagnose backend issues faster.const data = await res.json(); - cost.value = data?.config?.cost ?? null; + if (!data?.config?.cost) { + throw new Error("Invalid response: missing config.cost"); + } + cost.value = data.config.cost;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/web/apps/dashboard/src/components/config/CostOverview.vue` around lines 49 - 50, The code silently falls back to null when config.cost is missing; update the fetch handling (after const data = await res.json()) to detect missing or malformed data and set error.value with a clear message instead of just assigning cost.value to null. Specifically, check data and data.config and data.config.cost; if any are absent, assign a descriptive string to error.value (including any useful context such as the raw data or HTTP status) and only set cost.value when data.config.cost is present; reference the existing cost.value and error.value reactive refs to implement this behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@clients/web/apps/chat/src/App.spec.ts`:
- Around line 140-157: Add assertions to verify Authorization and X-Session-Id
are present on the streaming request (streamInit) just like you do for the
webhook request (webhookInit): after extracting const [, streamInit] =
fetchMock.mock.calls[4] ?? []; add expectations that (streamInit?.headers as
Record<string,string>).Authorization equals "Bearer zc_test_token" and that
(streamInit?.headers as Record<string,string>)["X-Session-Id"] equals
"11111111-1111-4111-8111-111111111111", while keeping the existing check for
(streamInit?.headers as Record<string,string>)["X-Request-Id"] to avoid
regressions.
In `@clients/web/apps/chat/src/App.vue`:
- Around line 222-237: The persistMessages function is not serializing
message.status so completion state is lost; update the serialization in
persistMessages (the mapping over messages.value used before
sessionStorage.setItem(messagesStorageKey(), ...)) to include m.status (and
ensure any deserialization code that reads sessionStorage via
messagesStorageKey() restores the status field into the message objects) so
messages retain their status after refresh.
In `@clients/web/apps/dashboard/src/components/config/BrowserSettings.spec.ts`:
- Around line 108-123: The test "hides password input when computer use api key
mode is unchanged" only asserts the input is hidden but doesn't ensure an
existing secret is not leaked; update the test for BrowserSettings to mount with
modelValue including browser_has_computer_use_api_key: true and a non-empty
browser_computer_use_api_key_value, then assert that the rendered output neither
shows the input nor contains the secret string (e.g., by checking wrapper.text()
or that specific selector does not contain the secret); keep the test name and
use the same mount setup and i18n plugin while adding these extra assertions to
cover the no-secret-leak case.
In `@clients/web/apps/dashboard/src/components/config/ComposioSettings.spec.ts`:
- Around line 83-135: Two tests in ComposioSettings.spec.ts ("clears non-empty
api key value when mode changes from replace to clear" and "clears api key value
when mode changes to clear") are duplicates; remove one of them (delete either
test block) or change the second to exercise a different path (e.g., switch from
"keep" to "clear" or "replace" to "keep") so it no longer repeats the same
transition. Update or remove the duplicate test that mounts ComposioSettings
with createAdminConfigForm(composio_api_key_mode: "replace") and
composio_api_key_value and that calls setValue("clear") on
'[data-testid="composio_api_key_mode"]' to avoid redundant assertions.
In `@clients/web/apps/dashboard/src/components/config/HealthDashboard.vue`:
- Around line 39-50: fetchHealth currently starts overlapping fetches and always
sets loading.value = true; modify fetchHealth to accept/use a shared
AbortController (create a new controller before starting a fetch and abort the
previous controller) so previous requests are cancelled when watchers or the
interval restart, and wire the controller cleanup into component unmount; only
set loading.value = true if there is no snapshot yet (i.e., when snapshot.value
is null/undefined) so refreshes don't re-show the full loading card, and ensure
you pass controller.signal into fetch; reuse the AbortController pattern from
McpOverview.vue and reference the fetchHealth function, loading/snapshot refs,
and the shared controller variable when implementing this.
- Around line 55-56: Validate the parsed response before assigning to
health.value: after const data = await res.json(), check that data.health is an
object and matches the minimal AdminHealthSnapshot shape (e.g. typeof
data.health.uptime_seconds === 'number' and
Array.isArray(data.health.components')), and only then set health.value =
data.health; otherwise set health.value = null and surface an error/empty state
(or log/report the mismatch) so callers of overallStatus and formatUptime won’t
receive malformed objects. Ensure this validation is applied where health.value
is assigned and references the same symbols (data.health, health.value,
AdminHealthSnapshot, overallStatus, formatUptime).
In `@clients/web/apps/dashboard/src/components/config/HeartbeatOverview.vue`:
- Around line 19-44: fetchHeartbeat can receive out-of-order responses; create
an AbortController per invocation and keep the current controller in an
outer-scoped variable so you can abort the previous request before starting a
new one, pass controller.signal to fetch, and treat aborts specially (do not set
error on AbortError). Ensure the previous controller is aborted at the start of
fetchHeartbeat, attach the new controller to that outer variable, and also abort
it in a component teardown (e.g., onBeforeUnmount) so in-flight requests are
cancelled when props change or the component unmounts; update error handling in
fetchHeartbeat to ignore AbortError while preserving other errors.
In `@clients/web/apps/dashboard/src/components/config/ReliabilityOverview.vue`:
- Around line 19-44: Add request cancellation to fetchReliability using
AbortController so stale fetches can't overwrite current state: create a new
AbortController for each invocation, pass controller.signal into fetch(...) when
calling requestUrl, and ensure previous requests are aborted—implement this by
converting the watcher to an inline async callback that receives onInvalidate
(watch(() => [props.gatewayUrl, props.bearerToken], async (_new, _old,
onInvalidate) => { ... })) or have fetchReliability accept an AbortSignal; call
onInvalidate(() => controller.abort()) so the prior controller is aborted on
rapid prop changes, and in the catch block ignore or handle AbortError specially
(do not set error.value when the fetch was aborted) while preserving current
assignments to reliability.value, loading.value, and error.value.
In `@clients/web/apps/dashboard/src/components/config/SchedulerStatus.vue`:
- Around line 19-44: The fetchSchedulerStatus flow needs request cancellation
like the other overview components: create an AbortController for each
invocation of fetchSchedulerStatus, pass controller.signal to fetch, and ensure
previous requests are aborted to avoid stale responses overwriting state; in the
watcher (watch(() => [props.gatewayUrl, props.bearerToken],
fetchSchedulerStatus, { immediate: true })) use the watch
invalidate/onInvalidate callback to call controller.abort() for the current
call, and also add an onBeforeUnmount cleanup to abort any remaining controller;
when handling errors, ignore AbortError (do not set error.value for aborts) and
only set error.value for real failures while still updating scheduler.value,
loading.value as before. Reference functions/identifiers: fetchSchedulerStatus,
watch(() => [props.gatewayUrl, props.bearerToken], ...), scheduler.value,
loading.value, error.value, and use AbortController and onBeforeUnmount.
- Around line 72-75: The UI currently treats scheduler.task_count === 0 as "not
available", conflating zero tasks with unavailable; update the rendering in
SchedulerStatus.vue to treat null/undefined as "not available" and display
numeric values (including 0) as-is by checking scheduler.task_count == null (or
typeof check) before showing t("scheduler.notAvailable"), and coordinate with
the backend to return null when the runtime enumeration is unsupported and 0
when there are genuinely zero tasks so the component (using scheduler.task_count
and t("scheduler.notAvailable")) can correctly distinguish the two cases.
In `@clients/web/apps/dashboard/src/components/config/UpdateSettings.spec.ts`:
- Around line 86-106: Add a symmetric unit test to cover the false case for
auto_install_enabled: using the existing mount helper (mountUpdateSettings) add
a new it block (e.g., "renders no for auto_install_enabled when false") that
passes updates.auto_install_enabled: false (with other required fields like
enabled, restart_policy, status populated similarly) and assert that
wrapper.get('[data-testid="updates_auto_install_enabled"]').text() contains
"No"; mirror the structure of the existing positive test so it integrates with
UpdateSettings.spec.ts and uses the same mountUpdateSettings helper.
---
Duplicate comments:
In `@clients/web/apps/dashboard/src/components/config/ChannelsOverview.spec.ts`:
- Around line 21-49: The test "renders channel list on successful fetch"
currently stubs fetch but doesn't assert the request details; after mounting via
mountComponent() and awaiting flushPromises(), add assertions that the global
fetch (stubbed via vi.stubGlobal("fetch", ...)) was called with the admin
endpoint "/web/admin/channels" and that the request included an Authorization
header "Bearer test-token" (e.g., inspect the first call to the stubbed fetch
and assert request url and headers), then keep vi.unstubAllGlobals() as cleanup.
In `@clients/web/apps/dashboard/src/components/config/ChannelsOverview.vue`:
- Around line 35-36: Parse the fetch response as unknown and validate its shape
before assigning to channels.value: after calling res.json(), assert the result
matches the expected type { channels: AdminChannelStatusView[] } (e.g., check
it's an object, has a channels array, and each item contains the expected
properties like channel_type and configured) and only then set channels.value =
parsed.channels; otherwise leave channels.value as [] and log or handle the
invalid payload. Ensure you reference the existing symbols res.json(),
channels.value, and the AdminChannelStatusView shape when adding the validation.
In `@clients/web/apps/dashboard/src/components/config/ComposioSettings.spec.ts`:
- Around line 137-154: The test "hides password input when mode is unchanged
even if secret value is present" in ComposioSettings should also assert the
secret isn't rendered anywhere in the component output; keep the seeded secret
in the props (composio_api_key_value: "my-secret") and add a negative assertion
against the rendered HTML/text (e.g., assert wrapper.html() or wrapper.text()
does not contain "my-secret") in addition to the existing check that the input
with data-testid="composio_api_key_value" does not exist, so the test fails if
the secret is echoed elsewhere.
In `@clients/web/apps/dashboard/src/components/config/CostOverview.spec.ts`:
- Around line 21-50: The test "renders cost data on successful fetch" stubs
global fetch but doesn't assert the request URL or Authorization header; update
this test to assert that fetch was called with the '/web/admin/config' endpoint
and that the options include an Authorization header with a Bearer token (use
the same auth setup the component uses), e.g., after flushPromises add an
assertion that vi's stubbed fetch was invoked for '/web/admin/config' and that
the passed options object contains headers.Authorization starting with 'Bearer
'; reference the test name and mountComponent/mocked fetch to locate where to
add the assertion and keep vi.unstubAllGlobals at the end.
In `@clients/web/apps/dashboard/src/components/config/CostOverview.vue`:
- Around line 49-50: The code silently falls back to null when config.cost is
missing; update the fetch handling (after const data = await res.json()) to
detect missing or malformed data and set error.value with a clear message
instead of just assigning cost.value to null. Specifically, check data and
data.config and data.config.cost; if any are absent, assign a descriptive string
to error.value (including any useful context such as the raw data or HTTP
status) and only set cost.value when data.config.cost is present; reference the
existing cost.value and error.value reactive refs to implement this behavior.
In `@clients/web/apps/dashboard/src/components/config/McpOverview.spec.ts`:
- Around line 41-57: The success test uses vi.stubGlobal("fetch", ...) and
mounts the component via mountComponent() but doesn't assert the Authorization
header; add an assertion after flushPromises() that the stubbed fetch was called
with an Authorization header (matching the token used in the test setup) — e.g.,
inspect vi.fn() call args from the stub created in vi.stubGlobal("fetch") to
verify the request headers include Authorization — so that the initial success
test mirrors the refetch test's header check and ensures auth contract coverage
for the component that uses mockConfig and the
'[data-testid="mcp-server-test-server"]' rendering.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: c427c374-a7c7-4f78-897a-529d892b386e
📒 Files selected for processing (25)
clients/web/apps/chat/src/App.spec.tsclients/web/apps/chat/src/App.vueclients/web/apps/chat/src/composables/useChat.tsclients/web/apps/dashboard/src/components/config/BrowserSettings.spec.tsclients/web/apps/dashboard/src/components/config/ChannelsOverview.spec.tsclients/web/apps/dashboard/src/components/config/ChannelsOverview.vueclients/web/apps/dashboard/src/components/config/ComposioSettings.spec.tsclients/web/apps/dashboard/src/components/config/CostOverview.spec.tsclients/web/apps/dashboard/src/components/config/CostOverview.vueclients/web/apps/dashboard/src/components/config/HealthDashboard.vueclients/web/apps/dashboard/src/components/config/HeartbeatOverview.vueclients/web/apps/dashboard/src/components/config/McpOverview.spec.tsclients/web/apps/dashboard/src/components/config/McpOverview.vueclients/web/apps/dashboard/src/components/config/MemorySettings.spec.tsclients/web/apps/dashboard/src/components/config/ReliabilityOverview.vueclients/web/apps/dashboard/src/components/config/SchedulerStatus.spec.tsclients/web/apps/dashboard/src/components/config/SchedulerStatus.vueclients/web/apps/dashboard/src/components/config/TunnelOverview.vueclients/web/apps/dashboard/src/components/config/UpdateSettings.spec.tsclients/web/apps/dashboard/src/components/config/WebSearchSettings.spec.tsclients/web/apps/dashboard/src/components/config/WebSearchSettings.vueclients/web/apps/dashboard/src/composables/configPayload.spec.tsclients/web/packages/locales/src/en.jsonclients/web/packages/locales/src/es.jsonclients/web/packages/shared/index.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: pr-checks
- GitHub Check: sonar
- GitHub Check: Cloudflare Pages
🧰 Additional context used
📓 Path-based instructions (2)
**/*
⚙️ CodeRabbit configuration file
**/*: Security first, performance second.
Validate input boundaries, auth/authz implications, and secret management.
Look for behavioral regressions, missing tests, and contract breaks across modules.
Files:
clients/web/apps/dashboard/src/components/config/CostOverview.spec.tsclients/web/apps/dashboard/src/components/config/HeartbeatOverview.vueclients/web/packages/shared/index.tsclients/web/apps/dashboard/src/components/config/UpdateSettings.spec.tsclients/web/apps/chat/src/App.spec.tsclients/web/apps/dashboard/src/components/config/ChannelsOverview.spec.tsclients/web/apps/dashboard/src/components/config/SchedulerStatus.spec.tsclients/web/apps/dashboard/src/components/config/ComposioSettings.spec.tsclients/web/apps/dashboard/src/components/config/BrowserSettings.spec.tsclients/web/apps/dashboard/src/composables/configPayload.spec.tsclients/web/apps/dashboard/src/components/config/ReliabilityOverview.vueclients/web/apps/dashboard/src/components/config/MemorySettings.spec.tsclients/web/apps/dashboard/src/components/config/McpOverview.spec.tsclients/web/apps/dashboard/src/components/config/McpOverview.vueclients/web/apps/dashboard/src/components/config/SchedulerStatus.vueclients/web/apps/dashboard/src/components/config/ChannelsOverview.vueclients/web/apps/dashboard/src/components/config/CostOverview.vueclients/web/apps/dashboard/src/components/config/TunnelOverview.vueclients/web/apps/dashboard/src/components/config/HealthDashboard.vueclients/web/apps/dashboard/src/components/config/WebSearchSettings.vueclients/web/apps/dashboard/src/components/config/WebSearchSettings.spec.tsclients/web/packages/locales/src/es.jsonclients/web/packages/locales/src/en.jsonclients/web/apps/chat/src/composables/useChat.tsclients/web/apps/chat/src/App.vue
**/*.vue
⚙️ CodeRabbit configuration file
**/*.vue: Enforce Vue 3 Composition API with <script setup>.
Ensure accessibility (A11y) and proper use of Tailwind CSS classes.
Check for proper prop validation and emitted events documentation.
Files:
clients/web/apps/dashboard/src/components/config/HeartbeatOverview.vueclients/web/apps/dashboard/src/components/config/ReliabilityOverview.vueclients/web/apps/dashboard/src/components/config/McpOverview.vueclients/web/apps/dashboard/src/components/config/SchedulerStatus.vueclients/web/apps/dashboard/src/components/config/ChannelsOverview.vueclients/web/apps/dashboard/src/components/config/CostOverview.vueclients/web/apps/dashboard/src/components/config/TunnelOverview.vueclients/web/apps/dashboard/src/components/config/HealthDashboard.vueclients/web/apps/dashboard/src/components/config/WebSearchSettings.vueclients/web/apps/chat/src/App.vue
🧠 Learnings (4)
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Include threat/risk notes and rollback strategy for security, runtime, and gateway changes; add or update tests for boundary checks and failure modes
Applied to files:
clients/web/apps/dashboard/src/composables/configPayload.spec.ts
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/src/{security,gateway,tools,config}/**/*.rs : Do not silently weaken security policy or access constraints; keep default behavior secure-by-default with deny-by-default where applicable
Applied to files:
clients/web/apps/dashboard/src/composables/configPayload.spec.tsclients/web/apps/dashboard/src/components/config/McpOverview.vue
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/src/{security,gateway,tools}/**/*.rs : Treat `src/security/`, `src/gateway/`, `src/tools/` as high-risk surfaces and never broaden filesystem/network execution scope without explicit policy checks
Applied to files:
clients/web/apps/dashboard/src/composables/configPayload.spec.tsclients/web/apps/dashboard/src/components/config/McpOverview.vueclients/web/apps/dashboard/src/components/config/ChannelsOverview.vue
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/src/channels/**/*.rs : Implement `Channel` trait in `src/channels/` with consistent `send`, `listen`, and `health_check` semantics and cover auth/allowlist/health behavior with tests
Applied to files:
clients/web/apps/dashboard/src/components/config/ChannelsOverview.vue
🔇 Additional comments (30)
clients/web/apps/dashboard/src/components/config/UpdateSettings.spec.ts (2)
1-16: LGTM: Helper function reduces test duplication.The
mountUpdateSettingshelper cleanly addresses the past review feedback by extracting the repeated mount + i18n setup. Properly typed and reusable.
19-84: LGTM: Core test scenarios are well covered.Tests 1-4 address past review feedback: targeted selectors prevent false positives, test naming matches fixtures, and the symmetric
update_available: truecase is now verified.clients/web/apps/dashboard/src/components/config/SchedulerStatus.spec.ts (2)
21-47: Tests should verify Authorization header propagation.The success-path test stubs
fetchbut doesn't assert that the bearer token is actually sent. Add an assertion to verify auth contract:const fetchMock = vi.fn().mockResolvedValue({...}); vi.stubGlobal("fetch", fetchMock); // ... mount and flush ... expect(fetchMock).toHaveBeenCalledWith( expect.stringContaining("/web/admin/scheduler"), expect.objectContaining({ headers: { Authorization: "Bearer test-token" } }) );
49-84: LGTM!Error and disabled state tests properly verify rendering behavior and clean up global stubs.
clients/web/packages/shared/index.ts (2)
10-40: Well-designed security-conscious URL utilities.The non-regex
trimTrailingSlashesavoids ReDoS (S5852), andvalidateGatewayUrlproperly enforces HTTP(S) protocol, strips trailing slashes from pathname, and clearssearch/hashto prevent credential leakage via query params. Good defensive design.
49-53: LGTM!Correctly calls
toString()on the URL object before passing totrimTrailingSlashes, matching the expected usage pattern.clients/web/packages/locales/src/es.json (1)
18-389: LGTM!Comprehensive Spanish translations for all new dashboard operational sections. Keys align with component usage, including localized units for time values (
reliability.unit.ms,reliability.unit.s,heartbeat.unit).clients/web/apps/dashboard/src/components/config/HeartbeatOverview.vue (1)
47-71: LGTM!Template properly handles all states with appropriate ARIA attributes, localized unit display via
t("heartbeat.unit"), and explicit empty-state whenheartbeatis null.clients/web/apps/dashboard/src/components/config/McpOverview.vue (2)
1-65: LGTM!Solid implementation with
AbortControllerfor request cancellation, proper cleanup on unmount, and defensiveArray.isArray()guards for server/capabilities arrays. URL validation and bearer auth handling are correct.
67-113: LGTM!Template properly renders loading/error states with ARIA attributes, includes defensive array guards, and uses
aria-hidden="true"on decorative status indicators.clients/web/apps/dashboard/src/components/config/McpOverview.spec.ts (1)
72-116: LGTM!Refetch and invalid URL tests provide good coverage of reactive behavior and URL validation error handling.
clients/web/apps/dashboard/src/components/config/ReliabilityOverview.vue (1)
47-95: LGTM!Template correctly renders model fallbacks as
primary → fallback, uses localized units (t("reliability.unit.ms/s")), and includes proper ARIA attributes on loading/error states.clients/web/apps/dashboard/src/components/config/SchedulerStatus.vue (1)
47-78: LGTM!Template properly handles all states with ARIA attributes, decorative indicator hidden from screen readers, and localized strings.
clients/web/apps/chat/src/App.vue (3)
254-282: Stub implementations noted for Phase 5B.
handleApproveandhandleRejectupdate local UI state only. The comment indicates this is intentional. Ensure follow-up work is tracked.
151-164: Stream chunk accumulation now correct.Using
streamBufferto concatenate chunks before updating the assistant message fixes the previously flagged critical issue where only the last chunk was displayed.
284-293: Debounced persistence addresses hot-path performance.The 300ms debounce prevents synchronous
sessionStorage.setItemon every streamed chunk, resolving the previously flagged performance concern.clients/web/apps/dashboard/src/composables/configPayload.spec.ts (2)
444-481: Good coverage for empty-secret validation across all secret-bearing sections.The previously requested tests for
web-search,composio, andmemoryempty-secret errors are now in place. This locks down the contract forbuildSecretUpdateacross all sections.
483-519: Autonomy risk toggle coverage added.Tests now flip
autonomy_require_approval_for_medium_riskandautonomy_block_high_risk_commands, plus validate comma-separated list parsing forauto_approve/always_ask. This addresses the previously flagged gap.clients/web/apps/dashboard/src/components/config/MemorySettings.spec.ts (1)
84-109: Test now properly validates token clearing.Starting with
memory_cerebro_auth_token_mode: "replace"and a non-empty"secret"value before switching to"clear"correctly proves the component clears the token. This addresses the previous feedback.clients/web/apps/dashboard/src/components/config/CostOverview.vue (1)
61-95: Template handles loading, error, and data states correctly with proper ARIA semantics.The
aria-live="polite"on loading androle="alert" aria-live="assertive"on error ensure screen readers announce state changes. The status grid renders cleanly whencostis populated.clients/web/apps/dashboard/src/components/config/WebSearchSettings.vue (2)
54-63: Secret mode validation guards against arbitrary values.The whitelist check before calling
updateSecretModeprevents injection of unexpected mode values. This addresses the past security concern.
28-38: Numeric clamping enforces bounds programmatically.
max_resultsis clamped to 1-10 andtimeout_secsis clamped to ≥1, preventing out-of-bounds values from reaching the model regardless of HTML attribute bypass.clients/web/apps/dashboard/src/components/config/WebSearchSettings.spec.ts (2)
103-155: Secret clearing regression tests now in place.Both
replace → clearandreplace → unchangedtransitions are tested, asserting thatweb_search_brave_api_key_valueis cleared to"". This addresses the previously flagged gap.
271-288: Invalid mode rejection test prevents silent acceptance of arbitrary values.The test confirms no
update:modelValueis emitted when an invalid mode like"bogus"is set, validating the whitelist guard in the component.clients/web/packages/locales/src/en.json (2)
18-31: Section labels extended consistently.New navigation sections (
webSearch,browser,composio,memory, etc.) follow the existing naming pattern.
281-388: New feature namespaces added with complete coverage.Each new admin feature (
webSearch,browser,composio,memory,scheduler,channels,cost,mcp,tunnel,reliability,heartbeat,updates) has its required label and status strings.clients/web/apps/chat/src/composables/useChat.ts (4)
81-105: Approval error parsing handles multiple payload shapes.
parseApprovalErrorTypecorrectly extracts error codes fromerror.code,error.type, top-leveltype, and stringerrorfields, covering the various shapes the backend might return.
321-339: 403 handling now distinguishes approval from credential errors.Using
parseApprovalErrorTypeto check forapproval_requiredorapproval_contractbefore callingmarkCredentialInvalidfixes the previously flagged issue where approvals were misclassified.
435-506: SSE parsing correctly persists frame state across reads.
buffer,currentEvent,currentData, anddoneEventare declared outside the read loop. CRLF normalization at lines 500 and 505 handles mixed line endings. The finaldecoder.decode()flush at line 504 ensures trailing data is processed.
460-478: JSON.parse calls wrapped in try-catch.Both
doneanderrorevent parsing now handle malformed JSON gracefully, throwing a controlled error message instead of crashing the stream.
Third round of review fixes: Chat: - Assert auth headers on stream request in App.spec.ts - Serialize message.status in persistMessages for session restore Dashboard: - Add AbortController to HealthDashboard, HeartbeatOverview, ReliabilityOverview, SchedulerStatus for stale request cancellation - Only show loading state on first fetch in HealthDashboard - Validate response shape in HealthDashboard and ChannelsOverview - Error on missing cost data in CostOverview instead of silent null - Distinguish null vs 0 task_count in SchedulerStatus Tests: - Assert no secret leak in BrowserSettings unchanged mode - Remove duplicate ComposioSettings clear test, add no-leak assertion - Assert fetch URL and Authorization headers in Channels, Cost, MCP - Add auto_install_enabled false test for UpdateSettings
Contributor
There was a problem hiding this comment.
Actionable comments posted: 13
♻️ Duplicate comments (4)
clients/web/apps/dashboard/src/components/config/SchedulerStatus.vue (1)
93-93:⚠️ Potential issue | 🟠 Major
task_countnull fallback is out of sync with the current API contract.
clients/web/apps/dashboard/src/types/admin-config.tsdefinestask_countasnumber, andclients/agent-runtime/src/gateway/admin.rscurrently emitsusize(0placeholder). With this contract, the null fallback here never triggers in production.Please align contract end-to-end: either make backend/type nullable when unavailable, or remove the null-specific UI/test path.
As per coding guidelines, "Security first, performance second. Validate input boundaries, auth/authz implications, and secret management. Look for behavioral regressions, missing tests, and contract breaks across modules."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/web/apps/dashboard/src/components/config/SchedulerStatus.vue` at line 93, The UI null-check for scheduler.task_count is inconsistent with the API/Type contract—task_count is defined as number in clients/web/apps/dashboard/src/types/admin-config.ts and the backend emits a usize—so remove the null-specific branch in SchedulerStatus.vue (replace the ternary that checks scheduler.task_count == null) and render the numeric value or a deterministic default (e.g., 0) instead; ensure corresponding tests and any UI copy expecting "notAvailable" are updated, and if you prefer to make the backend/type nullable instead, update clients/agent-runtime/src/gateway/admin.rs to emit an optional value and change admin-config.ts to number | null so the existing Vue fallback becomes meaningful (pick one approach and apply consistently across SchedulerStatus.vue, admin-config.ts, and admin.rs).clients/web/apps/dashboard/src/components/config/SchedulerStatus.spec.ts (1)
21-47:⚠️ Potential issue | 🟠 MajorAssert the authenticated request contract in the success-path test.
This test verifies rendering but does not assert that
fetchwas called with/web/admin/schedulerand the Bearer header, so auth regressions can slip through undetected.Suggested patch
it("renders scheduler status on successful fetch", async () => { + const token = "test-token"; const mockScheduler = { enabled: true, max_tasks: 64, max_concurrent: 4, task_count: 0, }; - vi.stubGlobal( + const fetchMock = vi.fn().mockResolvedValue({ + ok: true, + json: () => Promise.resolve({ scheduler: mockScheduler }), + }); + vi.stubGlobal( "fetch", - vi.fn().mockResolvedValue({ - ok: true, - json: () => Promise.resolve({ scheduler: mockScheduler }), - }) + fetchMock ); const wrapper = mountComponent(); await flushPromises(); + expect(fetchMock).toHaveBeenCalledWith( + expect.stringContaining("/web/admin/scheduler"), + expect.objectContaining({ + headers: expect.objectContaining({ + Authorization: `Bearer ${token}`, + }), + }) + ); expect(wrapper.find('[data-testid="scheduler-status"]').exists()).toBe(true);As per coding guidelines, "Security first, performance second. Validate input boundaries, auth/authz implications, and secret management. Look for behavioral regressions, missing tests, and contract breaks across modules."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/web/apps/dashboard/src/components/config/SchedulerStatus.spec.ts` around lines 21 - 47, The test "renders scheduler status on successful fetch" stubs global fetch but doesn't assert the authenticated request contract; update the test (around mountComponent and after flushPromises) to assert that the stubbed fetch was called with the scheduler endpoint and an Authorization Bearer header (e.g., assert fetch was called with a URL containing "/web/admin/scheduler" and the init object contains headers with an Authorization value matching /^Bearer\s+/), using the existing vi stub and expect/Matchers so auth regressions are caught; keep vi.unstubAllGlobals() as-is.clients/web/apps/dashboard/src/components/config/CostOverview.vue (1)
54-60:⚠️ Potential issue | 🟠 MajorFail fast on malformed
config.costinstead of truthy-only acceptance.Any truthy
data.config.costis accepted, so malformed payloads can silently render invalid values. Validate the object shape and numeric boundaries before assignment, then surface a contract error when invalid.Suggested validation guard
- const data = await res.json(); - if (data?.config?.cost) { - cost.value = data.config.cost; - } else { - cost.value = null; - error.value = "Cost data not available"; - } + const data: unknown = await res.json(); + const nextCost = (data as { config?: { cost?: unknown } })?.config?.cost; + const isValidCost = + typeof nextCost === "object" && + nextCost !== null && + typeof (nextCost as { enabled?: unknown }).enabled === "boolean" && + typeof (nextCost as { daily_limit_usd?: unknown }).daily_limit_usd === "number" && + (nextCost as { daily_limit_usd: number }).daily_limit_usd >= 0 && + typeof (nextCost as { monthly_limit_usd?: unknown }).monthly_limit_usd === "number" && + (nextCost as { monthly_limit_usd: number }).monthly_limit_usd >= 0 && + typeof (nextCost as { warn_at_percent?: unknown }).warn_at_percent === "number" && + (nextCost as { warn_at_percent: number }).warn_at_percent >= 0 && + (nextCost as { warn_at_percent: number }).warn_at_percent <= 100 && + typeof (nextCost as { allow_override?: unknown }).allow_override === "boolean"; + + if (!isValidCost) { + cost.value = null; + error.value = "Invalid /web/admin/config response: malformed config.cost"; + return; + } + cost.value = nextCost as AdminCostView;As per coding guidelines, "Validate input boundaries, auth/authz implications, and secret management. Look for behavioral regressions, missing tests, and contract breaks across modules."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/web/apps/dashboard/src/components/config/CostOverview.vue` around lines 54 - 60, The current assignment accepts any truthy data.config.cost; instead validate the shape and numeric boundaries before setting cost.value: after parsing res.json() inspect data?.config?.cost to ensure it's an object with the expected numeric fields (e.g., total, monthly or whichever keys your UI expects) and that those numbers are finite and within allowed ranges, and only then set cost.value = data.config.cost; otherwise set cost.value = null and error.value to a clear contract/validation error (e.g., "Invalid config.cost shape or values") so malformed payloads fail fast; update the validation logic near the res.json() handling and any helper used to parse/validate the cost structure.clients/web/apps/dashboard/src/components/config/ChannelsOverview.vue (1)
41-48:⚠️ Potential issue | 🟠 MajorHarden runtime parsing to enforce the backend contract types.
Current guards only check key presence; invalid
channel_type/configuredvalue types can still flow intochannels.valueand break UI assumptions.Suggested fix
- const data = await res.json(); - if ( - Array.isArray(data?.channels) && - data.channels.every( - (c: unknown) => - typeof c === "object" && c !== null && "channel_type" in c && "configured" in c - ) - ) { - channels.value = data.channels; + const data: unknown = await res.json(); + if ( + data && + typeof data === "object" && + Array.isArray((data as { channels?: unknown }).channels) + ) { + const raw = (data as { channels: unknown[] }).channels; + channels.value = raw.filter( + (c): c is AdminChannelStatusView => + typeof c === "object" && + c !== null && + typeof (c as { channel_type?: unknown }).channel_type === "string" && + typeof (c as { configured?: unknown }).configured === "boolean" + ); } else { channels.value = []; }As per coding guidelines, "Look for behavioral regressions, missing tests, and contract breaks across modules."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/web/apps/dashboard/src/components/config/ChannelsOverview.vue` around lines 41 - 48, The runtime type guard around data?.channels only checks for property presence and can allow wrong types through; update the check before assigning to channels.value to validate that data.channels is an array and each entry has channel_type as a string (or expected enum) and configured as a boolean (or expected shape) and any other required fields, e.g., replace the predicate used in the Array.every call to assert typeof c.channel_type === "string" and typeof c.configured === "boolean" (or call a small type-guard function like isValidChannel(c)) so only fully validated channel objects are assigned to channels.value; reference the existing data?.channels check and the channels.value assignment and tighten the per-item checks for "channel_type" and "configured".
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@clients/web/apps/dashboard/src/components/config/ChannelsOverview.spec.ts`:
- Around line 54-55: The tests stub the global fetch via vi and call
vi.unstubAllGlobals() inline (seen near vi.unstubAllGlobals() in
ChannelsOverview.spec.ts), which can be skipped if a test fails; move the
cleanup into a dedicated afterEach hook that always runs: add an afterEach block
that calls vi.unstubAllGlobals() (and explicitly restores or resets any fetch
stubs/mocks) so the global fetch is always cleaned between tests and no stub
leaks into subsequent specs.
In `@clients/web/apps/dashboard/src/components/config/ChannelsOverview.vue`:
- Around line 21-23: The loading flag is being cleared by any completed
request's finally block, causing stale requests to turn off the spinner; modify
fetchChannels to only update loading for the most recent fetch by introducing a
monotonically increasing request id (e.g., module-scoped let latestFetchId = 0),
incrementing it at the start of fetchChannels and capturing the current id in a
local const (e.g., const myId = ++latestFetchId), and then in the finally block
only set loading.value = false if myId === latestFetchId (or if the passed
AbortSignal indicates this is still the active request); reference the
fetchChannels function and the watcher that aborts old requests when applying
this change.
In `@clients/web/apps/dashboard/src/components/config/CostOverview.spec.ts`:
- Around line 20-69: Add a new spec in CostOverview tests that verifies the
contract-error path by stubbing fetch (used in mountComponent) to resolve
ok:true with a payload where config.cost is missing or malformed, then mount the
component (via mountComponent), await flushPromises, and assert that the
component renders the "Cost data not available" message (and/or shows the error
UI selector used for that branch); also ensure you clean up the global stub with
vi.unstubAllGlobals() at the end of the test.
- Around line 55-67: Move the global cleanup out of individual tests and into a
shared afterEach hook to ensure vi.unstubAllGlobals() always runs even if a test
fails: remove the per-test calls to vi.unstubAllGlobals() in the "shows error on
fetch failure" (and any other tests) and add a top-level afterEach(() =>
vi.unstubAllGlobals()) in this spec file so globals stubbed by
vi.stubGlobal("fetch", ...) are reliably restored; no behavior changes to
mountComponent or flushPromises are required.
In `@clients/web/apps/dashboard/src/components/config/CostOverview.vue`:
- Around line 35-77: The bug is that an earlier fetchCost invocation (using the
old abortController) can finish after a newer request started and its finally
block sets loading.value = false; fix by guarding state updates with a
request-order check: capture the current abortController (or a requestId) at the
start of fetchCost (or accept it as an argument) and in the catch/finally only
update loading.value, error.value, and cost.value if the captured controller (or
requestId) still equals the module-level abortController (or matches the latest
requestId); update references to fetchCost, abortController, loading.value and
the watcher to ensure the same controller/requestId is used for the comparison
so stale aborted responses cannot flip loading.
In `@clients/web/apps/dashboard/src/components/config/HealthDashboard.vue`:
- Around line 161-173: The hardcoded hex colors in the status indicator rules
(.health-indicator.ok, .component-indicator.ok, .health-indicator.error,
.component-indicator.error, .health-indicator.unknown) should be replaced with
CSS variables (e.g. var(--color-status-ok, `#22c55e`), var(--color-status-error,
`#ef4444`), var(--color-status-unknown, `#9ca3af`)) and then declare those variables
in your global/theme stylesheet (and override for dark/light themes as needed)
so status colors adapt to theming while keeping the hex values as sensible
fallbacks.
- Around line 63-72: The validation for assigning health.value allows components
to be null because typeof null === "object"; update the conditional that sets
health.value (the block checking data.health and data.health.components) to
explicitly reject null (e.g. add data.health.components !== null) or otherwise
ensure components is an object/array; this prevents
Object.values(health.value.components) in overallStatus from throwing when
components is null.
In `@clients/web/apps/dashboard/src/components/config/HeartbeatOverview.vue`:
- Around line 115-121: Replace the hardcoded hex values in the .configured and
.not-configured CSS rules with theme-aware CSS custom properties (for example
var(--color-success) and var(--color-muted) or existing Tailwind-derived
variables) so colors adapt to dark mode and brand theming; update the
.configured and .not-configured selectors to reference these custom properties
(and include sensible fallbacks like var(--color-success, `#22c55e`)) and ensure
use aligns with the app’s global CSS variables or Tailwind config for
accessibility/contrast.
- Around line 40-41: The response assigned to heartbeat.value is not validated
against the expected AdminHeartbeatView shape; add runtime guards before setting
heartbeat.value by checking that data exists and that data.config.heartbeat has
the required properties (e.g., hasOwnProperty('enabled') and
hasOwnProperty('interval_minutes')), that enabled is a boolean and
interval_minutes is a finite number (coerce/parse if necessary), and fall back
to null (or log an error) if validation fails; update the code around the
assignment to data = await res.json(); heartbeat.value = ... to perform these
checks and only assign a safe object matching the AdminHeartbeatView shape,
otherwise set heartbeat.value = null.
In `@clients/web/apps/dashboard/src/components/config/McpOverview.spec.ts`:
- Line 62: Add a global cleanup hook by importing afterEach from "vitest" and
call afterEach(() => vi.unstubAllGlobals()); then remove the explicit
vi.unstubAllGlobals() calls spread through the spec (the individual calls
currently located where vi.unstubAllGlobals() is invoked) so that
vi.unstubAllGlobals() always runs regardless of test failures; ensure the new
afterEach import and hook are placed with the other imports near the top of the
file.
In `@clients/web/apps/dashboard/src/components/config/ReliabilityOverview.vue`:
- Around line 42-47: The finally block currently unconditionally sets
loading.value = false, causing a race where an aborted older fetch clears the
loading state for a newer in-flight fetch; modify the fetch logic in
ReliabilityOverview.vue to only clear loading when the finishing request is the
active one — e.g., create a local request id or capture the AbortController
instance at request start and in the finally check that it matches the
component-level current controller (or currentRequestId) before setting
loading.value = false; ensure the catch still returns on AbortError but does not
clear loading unless the request identity matches the active request.
- Line 28: Replace the hardcoded string assignment error.value = "Invalid
gateway URL" in the ReliabilityOverview.vue component with a translated string
from useI18n() (e.g., const { t } = useI18n(); error.value =
t('errors.invalidGatewayUrl')); add the key "errors.invalidGatewayUrl" with
appropriate English and Spanish translations to the locales files en.json and
es.json so the message is localized; repeat the same change for the seven other
overview components that currently hardcode this message to ensure consistency.
In `@clients/web/apps/dashboard/src/components/config/SchedulerStatus.vue`:
- Around line 21-58: An older aborted fetch can still run its finally block and
flip loading.value to false after a newer request started; modify
fetchSchedulerStatus and the watch that creates abortController to guard state
mutations so only the latest in-flight request updates loading, error, and
scheduler. Implement a request identity check: when creating a new
AbortController in the watch, set a local token/requestId (or capture the
controller instance) and read that token at the start of fetchSchedulerStatus;
before assigning scheduler.value, error.value, or toggling loading.value in the
try/catch/finally paths, verify the token/controller still matches the current
global abortController (or that the requestId is still current) and skip updates
if it does not; apply this guard around the assignments in fetchSchedulerStatus
(including where it sets scheduler.value, error.value, and in finally) to
prevent stale UI flips.
---
Duplicate comments:
In `@clients/web/apps/dashboard/src/components/config/ChannelsOverview.vue`:
- Around line 41-48: The runtime type guard around data?.channels only checks
for property presence and can allow wrong types through; update the check before
assigning to channels.value to validate that data.channels is an array and each
entry has channel_type as a string (or expected enum) and configured as a
boolean (or expected shape) and any other required fields, e.g., replace the
predicate used in the Array.every call to assert typeof c.channel_type ===
"string" and typeof c.configured === "boolean" (or call a small type-guard
function like isValidChannel(c)) so only fully validated channel objects are
assigned to channels.value; reference the existing data?.channels check and the
channels.value assignment and tighten the per-item checks for "channel_type" and
"configured".
In `@clients/web/apps/dashboard/src/components/config/CostOverview.vue`:
- Around line 54-60: The current assignment accepts any truthy data.config.cost;
instead validate the shape and numeric boundaries before setting cost.value:
after parsing res.json() inspect data?.config?.cost to ensure it's an object
with the expected numeric fields (e.g., total, monthly or whichever keys your UI
expects) and that those numbers are finite and within allowed ranges, and only
then set cost.value = data.config.cost; otherwise set cost.value = null and
error.value to a clear contract/validation error (e.g., "Invalid config.cost
shape or values") so malformed payloads fail fast; update the validation logic
near the res.json() handling and any helper used to parse/validate the cost
structure.
In `@clients/web/apps/dashboard/src/components/config/SchedulerStatus.spec.ts`:
- Around line 21-47: The test "renders scheduler status on successful fetch"
stubs global fetch but doesn't assert the authenticated request contract; update
the test (around mountComponent and after flushPromises) to assert that the
stubbed fetch was called with the scheduler endpoint and an Authorization Bearer
header (e.g., assert fetch was called with a URL containing
"/web/admin/scheduler" and the init object contains headers with an
Authorization value matching /^Bearer\s+/), using the existing vi stub and
expect/Matchers so auth regressions are caught; keep vi.unstubAllGlobals()
as-is.
In `@clients/web/apps/dashboard/src/components/config/SchedulerStatus.vue`:
- Line 93: The UI null-check for scheduler.task_count is inconsistent with the
API/Type contract—task_count is defined as number in
clients/web/apps/dashboard/src/types/admin-config.ts and the backend emits a
usize—so remove the null-specific branch in SchedulerStatus.vue (replace the
ternary that checks scheduler.task_count == null) and render the numeric value
or a deterministic default (e.g., 0) instead; ensure corresponding tests and any
UI copy expecting "notAvailable" are updated, and if you prefer to make the
backend/type nullable instead, update clients/agent-runtime/src/gateway/admin.rs
to emit an optional value and change admin-config.ts to number | null so the
existing Vue fallback becomes meaningful (pick one approach and apply
consistently across SchedulerStatus.vue, admin-config.ts, and admin.rs).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 42c0a398-5494-4988-8149-ab40729c3e5a
📒 Files selected for processing (15)
clients/web/apps/chat/src/App.spec.tsclients/web/apps/chat/src/App.vueclients/web/apps/dashboard/src/components/config/BrowserSettings.spec.tsclients/web/apps/dashboard/src/components/config/ChannelsOverview.spec.tsclients/web/apps/dashboard/src/components/config/ChannelsOverview.vueclients/web/apps/dashboard/src/components/config/ComposioSettings.spec.tsclients/web/apps/dashboard/src/components/config/CostOverview.spec.tsclients/web/apps/dashboard/src/components/config/CostOverview.vueclients/web/apps/dashboard/src/components/config/HealthDashboard.vueclients/web/apps/dashboard/src/components/config/HeartbeatOverview.vueclients/web/apps/dashboard/src/components/config/McpOverview.spec.tsclients/web/apps/dashboard/src/components/config/ReliabilityOverview.vueclients/web/apps/dashboard/src/components/config/SchedulerStatus.spec.tsclients/web/apps/dashboard/src/components/config/SchedulerStatus.vueclients/web/apps/dashboard/src/components/config/UpdateSettings.spec.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: sonar
- GitHub Check: pr-checks
- GitHub Check: Cloudflare Pages
🧰 Additional context used
📓 Path-based instructions (2)
**/*
⚙️ CodeRabbit configuration file
**/*: Security first, performance second.
Validate input boundaries, auth/authz implications, and secret management.
Look for behavioral regressions, missing tests, and contract breaks across modules.
Files:
clients/web/apps/dashboard/src/components/config/UpdateSettings.spec.tsclients/web/apps/dashboard/src/components/config/CostOverview.spec.tsclients/web/apps/dashboard/src/components/config/HeartbeatOverview.vueclients/web/apps/dashboard/src/components/config/SchedulerStatus.spec.tsclients/web/apps/dashboard/src/components/config/ChannelsOverview.vueclients/web/apps/dashboard/src/components/config/CostOverview.vueclients/web/apps/dashboard/src/components/config/ComposioSettings.spec.tsclients/web/apps/dashboard/src/components/config/BrowserSettings.spec.tsclients/web/apps/dashboard/src/components/config/ChannelsOverview.spec.tsclients/web/apps/dashboard/src/components/config/McpOverview.spec.tsclients/web/apps/dashboard/src/components/config/SchedulerStatus.vueclients/web/apps/dashboard/src/components/config/ReliabilityOverview.vueclients/web/apps/dashboard/src/components/config/HealthDashboard.vueclients/web/apps/chat/src/App.vueclients/web/apps/chat/src/App.spec.ts
**/*.vue
⚙️ CodeRabbit configuration file
**/*.vue: Enforce Vue 3 Composition API with <script setup>.
Ensure accessibility (A11y) and proper use of Tailwind CSS classes.
Check for proper prop validation and emitted events documentation.
Files:
clients/web/apps/dashboard/src/components/config/HeartbeatOverview.vueclients/web/apps/dashboard/src/components/config/ChannelsOverview.vueclients/web/apps/dashboard/src/components/config/CostOverview.vueclients/web/apps/dashboard/src/components/config/SchedulerStatus.vueclients/web/apps/dashboard/src/components/config/ReliabilityOverview.vueclients/web/apps/dashboard/src/components/config/HealthDashboard.vueclients/web/apps/chat/src/App.vue
🧠 Learnings (1)
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/src/{security,gateway,tools}/**/*.rs : Treat `src/security/`, `src/gateway/`, `src/tools/` as high-risk surfaces and never broaden filesystem/network execution scope without explicit policy checks
Applied to files:
clients/web/apps/dashboard/src/components/config/ChannelsOverview.vue
🔇 Additional comments (27)
clients/web/apps/dashboard/src/components/config/ComposioSettings.spec.ts (2)
10-46: Good contract test for field update and save emission.This verifies both
update:modelValuepayload shape and singlesaveemission, which protects component-event integration behavior.As per coding guidelines, "Look for behavioral regressions, missing tests, and contract breaks across modules."
83-128: Secret-handling regression coverage is strong.The replace→clear flow proves key wiping with a non-empty seed, and unchanged mode verifies no secret render/leak.
As per coding guidelines, "Security first, performance second. Validate input boundaries, auth/authz implications, and secret management."
clients/web/apps/dashboard/src/components/config/BrowserSettings.spec.ts (2)
10-44: Event contract coverage looks correct.The test validates a single
update:modelValueemission on mode change and a singlesaveemission, reducing regression risk in parent integration.As per coding guidelines, "Look for behavioral regressions, missing tests, and contract breaks across modules."
81-126: Secret-mode security behavior is well-covered.These tests verify both secret clearing on replace→clear and no secret leakage in unchanged mode with a seeded secret value.
As per coding guidelines, "Security first, performance second. Validate input boundaries, auth/authz implications, and secret management."
clients/web/apps/dashboard/src/components/config/McpOverview.spec.ts (3)
50-55: Auth-header assertions now present.This addresses the prior security review feedback—fetch calls are now validated to include
Authorization: Bearer <token>.
21-63: Good coverage of auth, rendering, and reactivity.Tests verify the correct URL construction, auth headers on initial and subsequent fetches, and rendered server details including capabilities array joined as expected. Solid test structure.
Also applies to: 77-100
65-75: Error and edge-case handling well covered.Tests for network failures and invalid
gatewayUrlensure the component fails gracefully with appropriate error messages and avoids unnecessary requests.Also applies to: 102-121
clients/web/apps/dashboard/src/components/config/UpdateSettings.spec.ts (4)
9-16: LGTM!Clean helper extraction that reduces boilerplate across tests. Properly typed and configures i18n deterministically.
19-63: LGTM!Good coverage of data presence edge cases: full status data, completely missing updates, and missing nested status. Targeted selectors ensure assertions are precise.
65-106: LGTM!Boolean-to-label rendering is properly tested for both true/false states of
update_available. Combining related assertions in test 5 is reasonable.
108-127: LGTM!Symmetric test coverage for
auto_install_enabled: falseaddresses the previous feedback and completes boolean label testing.clients/web/apps/dashboard/src/components/config/HeartbeatOverview.vue (2)
19-63: AbortController integration and cleanup look solid.Proper cancellation on prop changes and unmount prevents stale response races. The AbortError handling (line 43) correctly avoids surfacing abort as a user-visible error.
66-90: Template states and accessibility are well-structured.Loading, error, empty (
!heartbeat), and data states are all handled distinctly. Thearia-live/roleattributes ensure screen reader announcements. Localized unit viat("heartbeat.unit")is correct.clients/web/apps/dashboard/src/components/config/HealthDashboard.vue (2)
1-109: Well-implemented polling and abort logic.The component correctly:
- Uses
AbortControllerto cancel in-flight requests on prop changes and unmount- Only shows loading state when no snapshot exists (avoids UI flicker on refresh)
- Validates response shape before assignment
- Properly cleans up interval and abort controller in
onUnmountedThis addresses the patterns from
McpOverview.vueand handles the edge cases raised in prior reviews.
112-143: Template accessibility and structure look good.Proper use of
aria-live,role, andaria-hiddenattributes on dynamic content and decorative elements. The conditional rendering structure is clean.clients/web/apps/chat/src/App.vue (7)
256-283: Stub implementations noted for Phase 5B.
handleApproveandhandleRejectonly update local UI state without calling the backendPOST /web/chat/tool-approvalendpoint. The comments indicate this is intentional for now.
151-164: LGTM! Chunk accumulation is correctly implemented.The streaming logic now properly accumulates chunks into
streamBufferbefore updating the assistant message, avoiding the previous overwrite bug.
165-172: LGTM! Auth errors no longer masked by fallback.Credential invalid errors are now detected and cause an early return with error status, preventing the fallback from masking auth failures or triggering duplicate requests.
285-294: LGTM! Debounced persistence avoids hot-path writes.The 300ms debounce prevents
sessionStorage.setItemfrom firing on every streamed chunk, which was previously a performance concern during SSE streaming.
296-305: LGTM! Session restoration now triggers on session ID changes.The watcher on
chat.currentSessionId.valueensures messages are restored when the session becomes available, not just on mount when the ID may be empty.
225-233: LGTM! Messagestatusis now persisted.The serialization now includes
status: m.status, so restored messages will retain their completion state after refresh.
517-520: The HealthIndicator component is secure and does not expose the bearer token. The token is used only in the fetch request Authorization header (line 40) and never appears in console logs, DOM attributes, or error messages. No action required.clients/web/apps/chat/src/App.spec.ts (2)
140-162: LGTM! Stream request now asserts all auth and session headers.The test verifies
Authorization,X-Session-Id, andX-Request-Idon the/web/chat/streamcall, plusX-Idempotency-Keyon the webhook fallback. This closes the security gap from the past review.
232-298: Test validates accumulation logic but uses synchronous mock.The
Responsebody is delivered atomically, so this tests that the SSE parser correctly accumulates multiplechunkevents—but doesn't exercise true incremental streaming (ReadableStream with chunked reads). That's acceptable for verifying the accumulation bug is fixed; consider a separate unit test inuseChat.spec.tswith a mockedReadableStreamif you want to cover backpressure/partial-read scenarios.clients/web/apps/dashboard/src/components/config/CostOverview.spec.ts (1)
43-48: Good auth contract assertion in success path.Nice coverage of both endpoint path and bearer header in the fetch call; this protects against silent auth regressions.
clients/web/apps/dashboard/src/components/config/CostOverview.vue (1)
88-90: A11y semantics for loading/error states look correct.Using
aria-liveplusrole="status"/role="alert"here is the right accessibility behavior for dynamic status updates.clients/web/apps/dashboard/src/components/config/ReliabilityOverview.vue (1)
69-71: A11y status semantics and model fallback rendering look correct.Loading/error regions now include appropriate
aria-live+role, and fallback mappings render asprimary → fallback, which improves operator visibility.Also applies to: 89-93
Fifth round of review refinements: - Move vi.unstubAllGlobals to afterEach in 4 spec files - Add fetchId guard to prevent loading race in 5 overview components - Fix typeof null check in HealthDashboard response validation - Add shape validation in HeartbeatOverview before assignment - Add missing-cost error test in CostOverview.spec - Assert fetch URL and auth headers in SchedulerStatus.spec - Remove misleading null check for task_count (type is number) - Replace 8 hardcoded error strings with i18n keys - Clean stale biome-ignore suppressions in 8 components
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
clients/web/apps/dashboard/src/components/config/McpOverview.vue (1)
20-48:⚠️ Potential issue | 🟡 MinorAdd
fetchIdguard to prevent stale responses from overwriting current state.Unlike
HeartbeatOverview.vueandSchedulerStatus.vue, this component lacks thefetchIdpattern. WhileAbortControllercancels the request, thefinallyblock at line 46 unconditionally setsloading.value = false, which can flip loading state incorrectly if an older aborted request'sfinallyexecutes after a newer request starts.Proposed fix
let abortController: AbortController | undefined; +let fetchId = 0; async function fetchMcp(signal?: AbortSignal) { + const myId = ++fetchId; loading.value = true; error.value = null; try { @@ mcp.value = data.config?.mcp ?? null; } catch (e: unknown) { if (e instanceof DOMException && e.name === "AbortError") return; error.value = e instanceof Error ? e.message : String(e); } finally { - loading.value = false; + if (myId === fetchId) { + loading.value = false; + } } }
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@clients/web/apps/dashboard/src/components/config/CostOverview.vue`:
- Around line 96-99: The decorative status indicator span in CostOverview.vue
(the element with class "status-indicator" and :class="cost.enabled ?
'configured' : 'not-configured'") should be hidden from assistive technologies;
add aria-hidden="true" to that span so screen readers ignore this purely visual
indicator, matching the approach used in SchedulerStatus.vue.
In `@clients/web/apps/dashboard/src/components/config/HeartbeatOverview.vue`:
- Around line 96-99: HeartbeatOverview.vue's visible status indicator is missing
the accessibility attribute used elsewhere; update the decorative status
indicator element (the same DOM node/class used to render the colored/pulse
status in HeartbeatOverview.vue) to include aria-hidden="true" so assistive tech
ignores this decorative element (matching SchedulerStatus.vue's treatment of its
status indicator).
In `@clients/web/apps/dashboard/src/components/config/TunnelOverview.vue`:
- Line 43: The watcher currently invokes fetchTunnel but doesn't abort
outstanding requests on component destroy; import onUnmounted and create an
AbortController (e.g., controller) that fetchTunnel uses (pass controller.signal
into fetch/axios calls), update the controller each time before starting a new
request (abort previous controller if exists), and call controller.abort()
inside an onUnmounted handler to cancel any pending request when the component
is destroyed; reference the existing watch(...) call and the fetchTunnel
function when adding the controller and onUnmounted cleanup.
- Around line 18-41: fetchTunnel can produce stale-overwrite and unvalidated
data; add an AbortController to cancel previous requests and a numeric fetchId
(incremented per call) to ignore out-of-order responses, ensure the finally
block only clears loading for the current fetchId, and validate the parsed
response shape before assigning tunnel.value (verify data.config?.tunnel
conforms to AdminTunnelView) rather than assigning it directly; use the existing
validateGatewayUrl/props.gatewayUrl and props.bearerToken to build the request,
abort previous controller when starting a new fetchTunnel, and only set
tunnel.value and loading.value when the response is from the latest fetchId.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: ca47037c-c0fc-4840-a41a-027a35356108
📒 Files selected for processing (14)
clients/web/apps/dashboard/src/components/config/ChannelsOverview.spec.tsclients/web/apps/dashboard/src/components/config/ChannelsOverview.vueclients/web/apps/dashboard/src/components/config/CostOverview.spec.tsclients/web/apps/dashboard/src/components/config/CostOverview.vueclients/web/apps/dashboard/src/components/config/HealthDashboard.vueclients/web/apps/dashboard/src/components/config/HeartbeatOverview.vueclients/web/apps/dashboard/src/components/config/McpOverview.spec.tsclients/web/apps/dashboard/src/components/config/McpOverview.vueclients/web/apps/dashboard/src/components/config/ReliabilityOverview.vueclients/web/apps/dashboard/src/components/config/SchedulerStatus.spec.tsclients/web/apps/dashboard/src/components/config/SchedulerStatus.vueclients/web/apps/dashboard/src/components/config/TunnelOverview.vueclients/web/packages/locales/src/en.jsonclients/web/packages/locales/src/es.json
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: sonar
- GitHub Check: pr-checks
- GitHub Check: Cloudflare Pages
🧰 Additional context used
📓 Path-based instructions (2)
**/*
⚙️ CodeRabbit configuration file
**/*: Security first, performance second.
Validate input boundaries, auth/authz implications, and secret management.
Look for behavioral regressions, missing tests, and contract breaks across modules.
Files:
clients/web/apps/dashboard/src/components/config/ChannelsOverview.spec.tsclients/web/apps/dashboard/src/components/config/CostOverview.spec.tsclients/web/apps/dashboard/src/components/config/SchedulerStatus.spec.tsclients/web/apps/dashboard/src/components/config/HeartbeatOverview.vueclients/web/apps/dashboard/src/components/config/McpOverview.vueclients/web/apps/dashboard/src/components/config/CostOverview.vueclients/web/apps/dashboard/src/components/config/ChannelsOverview.vueclients/web/apps/dashboard/src/components/config/SchedulerStatus.vueclients/web/apps/dashboard/src/components/config/ReliabilityOverview.vueclients/web/apps/dashboard/src/components/config/McpOverview.spec.tsclients/web/apps/dashboard/src/components/config/TunnelOverview.vueclients/web/apps/dashboard/src/components/config/HealthDashboard.vueclients/web/packages/locales/src/en.jsonclients/web/packages/locales/src/es.json
**/*.vue
⚙️ CodeRabbit configuration file
**/*.vue: Enforce Vue 3 Composition API with <script setup>.
Ensure accessibility (A11y) and proper use of Tailwind CSS classes.
Check for proper prop validation and emitted events documentation.
Files:
clients/web/apps/dashboard/src/components/config/HeartbeatOverview.vueclients/web/apps/dashboard/src/components/config/McpOverview.vueclients/web/apps/dashboard/src/components/config/CostOverview.vueclients/web/apps/dashboard/src/components/config/ChannelsOverview.vueclients/web/apps/dashboard/src/components/config/SchedulerStatus.vueclients/web/apps/dashboard/src/components/config/ReliabilityOverview.vueclients/web/apps/dashboard/src/components/config/TunnelOverview.vueclients/web/apps/dashboard/src/components/config/HealthDashboard.vue
🧠 Learnings (4)
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Inspect existing module and adjacent tests before editing; define scope boundary with one concern per PR and avoid mixed feature+refactor+infra patches
Applied to files:
clients/web/apps/dashboard/src/components/config/CostOverview.spec.tsclients/web/apps/dashboard/src/components/config/McpOverview.spec.ts
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/src/{security,gateway,tools,config}/**/*.rs : Do not silently weaken security policy or access constraints; keep default behavior secure-by-default with deny-by-default where applicable
Applied to files:
clients/web/apps/dashboard/src/components/config/McpOverview.vue
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/src/{security,gateway,tools}/**/*.rs : Treat `src/security/`, `src/gateway/`, `src/tools/` as high-risk surfaces and never broaden filesystem/network execution scope without explicit policy checks
Applied to files:
clients/web/apps/dashboard/src/components/config/McpOverview.vueclients/web/apps/dashboard/src/components/config/ChannelsOverview.vue
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/src/channels/**/*.rs : Implement `Channel` trait in `src/channels/` with consistent `send`, `listen`, and `health_check` semantics and cover auth/allowlist/health behavior with tests
Applied to files:
clients/web/apps/dashboard/src/components/config/ChannelsOverview.vue
🔇 Additional comments (15)
clients/web/apps/dashboard/src/components/config/SchedulerStatus.spec.ts (1)
1-88: LGTM!The test suite is well-structured with proper cleanup via
afterEach, validates the authenticated request contract (URL +Authorizationheader), and covers success, error, and disabled states. Prior review concerns have been addressed.clients/web/apps/dashboard/src/components/config/ChannelsOverview.spec.ts (1)
1-68: LGTM!The test suite properly validates the auth contract with URL and
Authorizationheader assertions, usesafterEachfor cleanup, and covers both success and error paths. Prior review concerns are addressed.clients/web/apps/dashboard/src/components/config/CostOverview.spec.ts (1)
1-85: LGTM!Excellent coverage: validates auth contract, tests successful rendering with formatted currency values, handles fetch failures, and verifies the "cost not available" error path for missing payload. Prior review concerns are all addressed.
clients/web/apps/dashboard/src/components/config/HeartbeatOverview.vue (1)
1-140: LGTM!Well-implemented component with proper request cancellation via
AbortController, stale-update protection viafetchId, response shape validation, accessibility attributes (aria-live,role), and localized unit display. Prior major concerns are addressed.clients/web/apps/dashboard/src/components/config/McpOverview.vue (1)
66-112: LGTM on template structure.Good defensive coding with
Array.isArray()guards, proper accessibility attributes, and clean conditional rendering.clients/web/apps/dashboard/src/components/config/SchedulerStatus.vue (1)
1-142: LGTM!Solid implementation with
AbortControllerfor cancellation,fetchIdguard for stale updates, URL validation, and proper accessibility attributes. All prior review concerns have been addressed.clients/web/apps/dashboard/src/components/config/ReliabilityOverview.vue (1)
1-144: LGTM!Comprehensive implementation addressing all prior concerns:
fetchIdguard for stale updates, URL validation with i18n error message, localized units, proper accessibility attributes, andmodel_fallbacksnow correctly displays "primary → fallback" mappings.clients/web/apps/dashboard/src/components/config/CostOverview.vue (1)
1-166: LGTM!Well-implemented component with proper
fetchIdguard for stale updates, URL validation, contract validation (surfaces "cost not available" error),Intl.NumberFormatfor currency display, and accessibility attributes on loading/error states. All prior major concerns addressed.clients/web/apps/dashboard/src/components/config/McpOverview.spec.ts (2)
25-65: Test masks potential component bug: missing filter for disabled servers.The mock data only includes servers with
enabled: true. According to the context snippet fromMcpOverview.vue:85-108, the component iterates over all servers without filtering by theenabledflag. If disabled servers should be hidden, this test won't catch that regression because the mock never includesenabled: falseentries.Consider adding a disabled server to the mock and asserting it either renders with appropriate styling or doesn't render at all (depending on intended behavior):
Proposed test data enhancement
const mockConfig = { config: { mcp: { enabled: true, servers: [ { name: "test-server", enabled: true, command: "npx test-mcp", capabilities: ["tools", "prompts"], startup_timeout_ms: 5000, call_timeout_ms: 30000, output_limit_bytes: 1048576, }, + { + name: "disabled-server", + enabled: false, + command: "npx disabled-mcp", + capabilities: [], + startup_timeout_ms: 5000, + call_timeout_ms: 30000, + output_limit_bytes: 1048576, + }, ], }, }, }; +// Then assert the expected behavior for disabled servers
1-118: LGTM on test structure and auth assertions.The test suite properly:
- Uses
afterEachfor cleanup (robust against test failures)- Validates
Authorization: Bearerheader on fetch calls- Covers refetch on prop changes and invalid URL handling
clients/web/apps/dashboard/src/components/config/ChannelsOverview.vue (1)
1-103: LGTM — well-structured component with proper safeguards.The implementation correctly addresses:
- Stale-request race via
fetchIdguard- Request cancellation with
AbortController+ cleanup on unmount- Gateway URL validation before attaching credentials
- Response shape validation before assignment
- A11y with
aria-live/roleon status messages andaria-hiddenon decorative indicatorsclients/web/apps/dashboard/src/components/config/HealthDashboard.vue (2)
1-204: LGTM on architecture — polling, cancellation, and validation are solid.The component properly:
- Resets the polling interval on prop changes
- Aborts in-flight requests before starting new ones
- Validates response shape including the
components !== nullguard- Cleans up on unmount
19-24: No action needed. The case-sensitivity concern is unfounded; the backend consistently sends lowercase status values ("ok", "error", "starting") that match the frontend's comparison logic.clients/web/packages/locales/src/en.json (1)
1-392: LGTM — locale additions are complete and consistent.All new dashboard sections and error/status keys are present. The structure mirrors the component requirements from the PR.
clients/web/packages/locales/src/es.json (1)
1-392: LGTM — Spanish translations are accurate and consistent.The translations maintain proper Spanish conventions and align with the existing localization patterns.
- Add aria-hidden to CostOverview status indicator - Add AbortController, fetchId guard, and onBeforeUnmount cleanup to TunnelOverview for stale request cancellation
|
yacosta738
deleted the
feat/276-expand-dashboard-and-web-operational-clients-to-match-runtime-admin-capabilities
branch
Merged
This was referenced Apr 6, 2026
Merged
This was referenced Apr 10, 2026
Merged
Merged
This was referenced Apr 19, 2026
Closed
This was referenced Apr 28, 2026
Merged
8 tasks
8 tasks
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.
This pull request introduces several new admin API endpoints and significantly refactors the admin configuration view to provide more detailed and structured information about system status, configuration, and runtime health. It also adds a new Server-Sent Events (SSE) streaming chat endpoint for real-time frontend consumption. The changes improve observability, configurability, and the ability to monitor and interact with the system.
Key changes include:
New API Endpoints and Features
/web/admin/channelsendpoint to report the configuration status of all communication channels, with a newAdminChannelStatusViewstruct for detailed channel info. [1] [2] [3] [4]/web/admin/schedulerendpoint to report scheduler configuration and status, including a newAdminSchedulerStatusViewstruct. [1] [2] [3] [4]/web/admin/healthendpoint to expose a runtime health snapshot for observability. [1] [2] [3] [4]/web/chat/streamendpoint for SSE-based streaming chat responses, including error and completion events, for improved frontend integration. [1] [2] [3] [4]Refactored and Expanded Admin Config View
AdminConfigViewand related structs with a new, more detailed version that includes new sections for cost controls, MCP (multi-command processor) servers, tunnel configuration, reliability settings, and heartbeat monitoring. [1] [2] [3] [4] [5]Code Cleanup and Organization
These changes collectively provide a more robust, observable, and user-friendly admin interface, and lay the groundwork for future enhancements to system monitoring and control.
Closes: #276