feat(dashboard): add secure web admin configuration surface#75
Merged
Conversation
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
corvus-plugins-edge | d20e17d | Feb 24 2026, 04:56 PM |
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a new Vue 3 dashboard app and client pairing/webhook flows, and implements gateway admin HTTP endpoints (/web/admin GET/PUT/OPTIONS) with auth/origin guards, redacted views, partial-patch updates, validation, persistence, restart detection, and tests. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant DashboardUI as "Dashboard (UI)"
participant Browser
participant Gateway
participant ConfigStore as "Config Store / Runtime"
User->>DashboardUI: Enter pairing code
DashboardUI->>Browser: POST /pair (X-Pairing-Code)
Browser->>Gateway: /pair request
Gateway->>ConfigStore: Validate pairing code
ConfigStore-->>Gateway: Return token
Gateway-->>Browser: { paired: true, token }
Browser-->>DashboardUI: store token
User->>DashboardUI: Open admin UI
DashboardUI->>Browser: GET /web/admin/options & /web/admin/config (Authorization)
Browser->>Gateway: forward requests
Gateway->>ConfigStore: Read config & options
ConfigStore-->>Gateway: Redacted view + options
Gateway-->>Browser: Response
Browser-->>DashboardUI: Populate form
User->>DashboardUI: Submit config changes
DashboardUI->>Browser: PUT /web/admin/config (Authorization, payload)
Browser->>Gateway: forward update
Gateway->>ConfigStore: Validate, apply patch, persist
alt restart required
ConfigStore-->>Gateway: restart_required fields
Gateway-->>Browser: 409 Conflict (restart_required)
else success
ConfigStore-->>Gateway: Persisted view
Gateway-->>Browser: Updated redacted view
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
✅ Contributor ReportUser: @yacosta738
Contributor Report evaluates based on public GitHub activity. Analysis period: 2025-02-24 to 2026-02-24 |
![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: 14
🧹 Nitpick comments (4)
clients/web/apps/dashboard/src/style.css (1)
24-29:min-height: 100%on#appmay not reliably fill the viewport.
min-height: 100%percentage resolution requires the parent to have a definite height, not merely anothermin-height. Withoutheight: 100%onhtml, the chain breaks and#appmay not stretch to fill the viewport in some browsers. Using100dvh(or100vhas a fallback) directly onbodyand#appis more robust.♻️ Proposed fix
html, body, `#app` { margin: 0; - min-height: 100%; + min-height: 100vh; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/web/apps/dashboard/src/style.css` around lines 24 - 29, The CSS rule using min-height: 100% on `#app` can fail because percentage heights need a definite parent height; update the stylesheet so html has height: 100% and set body and `#app` to use a viewport-based min-height (prefer 100dvh with a 100vh fallback) to reliably fill the viewport across browsers; modify the selectors html, body, and `#app` accordingly (ensure you change the existing html, body, `#app` block and the `#app` rule to use height: 100% on html and min-height: 100dvh; min-height: 100vh on body and `#app`).clients/web/apps/dashboard/src/components/ui/input/Input.vue (1)
4-8: Consider narrowingtypeprop to valid HTML input types
type?: stringaccepts arbitrary strings. Narrowing it prevents accidental misuse and improves IDE autocomplete.♻️ Proposed refinement
- type?: string; + type?: "text" | "password" | "email" | "number" | "search" | "tel" | "url" | "date" | "time" | "datetime-local" | "month" | "week" | "color" | "file" | "range" | "checkbox" | "radio" | "hidden";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/web/apps/dashboard/src/components/ui/input/Input.vue` around lines 4 - 8, The prop "type" in defineProps currently accepts any string; narrow it to valid HTML input types by replacing its type annotation (the "type" prop in Input.vue's defineProps) with the DOM union type HTMLInputTypeAttribute (or an explicit union of allowed literals like 'text'|'password'|'email' etc.) so IDE autocomplete and type-safety are enabled; update any usages in the component (e.g., where "type" is passed to the <input>) to match the narrowed type and fix any resulting type errors.clients/web/apps/dashboard/src/App.vue (2)
102-107:gatewayPortandwebhookPortshare the same default (3000).The two values are serialized as distinct payload keys (
gateway.portandwebhook.port). If the gateway and webhook listener bind to the same interface, having them default to the same port will cause a startup conflict. Confirm whether these are expected to share a port (same server) or if the webhook default should differ (e.g.,3001).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/web/apps/dashboard/src/App.vue` around lines 102 - 107, The default ports for gatewayPort and webhookPort both use "3000", which will conflict since they serialize to distinct keys (gateway.port and webhook.port); decide whether they should share a port (same server) or run separately and, if separate, change the webhookPort default to a different value (e.g., "3001") in the App.vue setup where gatewayPort and webhookPort are defined so the webhook listener won’t collide with the gateway listener.
181-183: All catch blocks silently discard the error object.The bare
catch { }pattern means network errors, JSON parse errors, and unexpected server responses are indistinguishable at the console. Add at minimum aconsole.errorto preserve the original error for debugging:♻️ Suggested pattern (apply to all three catch sites)
- } catch { + } catch (err) { + console.error("[pairGateway]", err); errorMessage.value = t("auth.unauthorized"); }Also applies to: 264-265, 353-354
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/web/apps/dashboard/src/App.vue` around lines 181 - 183, The bare catch blocks (the ones that currently do "} catch { errorMessage.value = t('auth.unauthorized'); }" and the two similar instances later) discard the original error; change each to "catch (err)" and log the error (e.g., console.error(err)) before setting user-facing state so the real network/parse/server error is preserved for debugging; update all three catch sites that set errorMessage.value to t("auth.unauthorized") to follow this pattern.
🤖 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/mod.rs`:
- Around line 1066-1307: handle_admin_update_config only mutates and persists
Config but does not update the live AppState (so changes to pairing, webhook
secret, rate_limiter, trust_forwarded_headers,
default_model/default_temperature, etc. do not take effect); after a successful
cfg.save() in handle_admin_update_config, update the corresponding in-memory
fields on State(AppState) (or store those mutable runtime fields behind
Arc<Mutex<...>> and refresh them) — specifically refresh pairing flag, recompute
and set webhook_secret_hash, rebuild/replace the rate_limiter, apply
trust_forwarded_headers, and update runtime model/temperature values (or
alternatively reject patches that require process restart with a clear error) so
the runtime reflects persisted changes immediately and security-sensitive
changes are not silently weakened.
In `@clients/web/apps/chat/src/locales/es.json`:
- Line 31: The string for the localization key "unauthorizedError" mixes English
and Spanish ("Haz pairing con /pair"); update that value to use consistent
Spanish terminology (use "emparejamiento") and keep the route and token
reference, e.g., replace the phrase "Haz pairing con /pair" with "Realiza el
emparejamiento con /pair" so the "unauthorizedError" entry is fully consistent.
In `@clients/web/apps/dashboard/README.md`:
- Around line 3-9: The README Spanish text misses diacritics: replace
"administracion" with "administración", "Edicion" with "Edición" and
"configuracion" with "configuración" in the Dashboard description (the lines
containing the phrases "Panel de administracion...", "Edicion de configuracion
via API `GET/PUT /web/admin/config`"); update those exact word occurrences so
the document reads "Panel de administración..." and "Edición de
configuración..." while preserving the surrounding phrasing and markup.
In `@clients/web/apps/dashboard/src/App.vue`:
- Line 369: Replace all hard-coded section headings and the webhook secret
helper text in the template with i18n lookups (use $t('...') in templates or the
v-t directive) instead of literal strings; specifically update each <h2> element
(currently rendering "Auth" and the other section headings) to use $t with new
keys (e.g. $t('auth.heading')) and replace the webhook-secret helper text that
currently mixes "configurado"/"no configurado" with a single i18n key (e.g.
$t('webhook.secretStatus', { status })) and add those keys to the locale files;
ensure the component imports/uses the existing vue-i18n instance and that all
replacements use consistent keys matching your locale JSON.
- Around line 281-286: The URL safety check currently only runs when
webhookSecretMode.value === "replace", but authHeaders() always adds the
Authorization header; update the guard so it runs whenever a bearer token would
be sent (i.e., when webhookSecretMode.value !== "clear") by changing the
condition from webhookSecretMode.value === "replace" to webhookSecretMode.value
!== "clear" (or alternatively check whether authHeaders() includes an
Authorization header) and keep using normalizeBaseUrl() and
isUrlSafeForSecrets() to set errorMessage.value and abort saving in saveConfig.
- Around line 189-199: connectGateway currently calls authHeaders() and sends
the Authorization bearer token without validating the gateway URL; add the same
URL-safety guard used in pairGateway (isUrlSafeForSecrets) after
normalizeBaseUrl() and before calling authHeaders() or making the fetch: if
isUrlSafeForSecrets(gatewayBaseUrl) is false, do not include authHeaders (omit
Authorization) and surface an error/status instead; modify connectGateway to
compute headers conditionally (e.g., const headers =
isUrlSafeForSecrets(gatewayBaseUrl) ? authHeaders() : {}) and use that headers
object in the fetch call, referencing connectGateway, authHeaders,
isUrlSafeForSecrets, and normalizeBaseUrl to locate the change.
- Line 117: The computed `canSave` currently returns true when not saving and a
token exists, but it also needs to block while connection is populating; update
the `canSave` computed (the const named canSave) to include the loading guard
(e.g., ensure it checks `!loading.value`) so it only becomes true when not
saving, not loading, and a non-empty trimmed `bearerToken.value` exists; locate
the `canSave` computed in App.vue and add the `!loading.value` condition
alongside `!saving.value` and `!!bearerToken.value.trim()`.
- Around line 295-333: The payload currently uses Number.parseFloat/parseInt
directly which converts empty or invalid strings to NaN and JSON.stringify turns
NaN into null; update the build of payload to validate and normalize numeric
inputs first (e.g., implement helpers like parseFloatSafe / parseIntSafe that
return a number only for valid numeric strings and undefined otherwise) and use
those helpers for fields referenced in payload (default_temperature,
autonomy.max_actions_per_hour, autonomy.max_cost_per_day_cents,
scheduler.max_tasks, scheduler.max_concurrent, gateway.port, webhook.port); when
a helper returns undefined, either omit the key from the payload or set it to a
sensible default so the server does not receive null.
In `@clients/web/apps/dashboard/src/components/ui/button/Button.vue`:
- Around line 11-18: The template applies parent classes twice by including
($attrs.class as string) in the :class array while also using v-bind="$attrs";
remove the explicit ($attrs.class as string) entry from the :class binding so
class merging is left to Vue's v-bind behavior and preserve the existing 'btn'
and `btn--${props.variant ?? 'default'}` entries (refer to the <template> button
element, v-bind="$attrs", :class and props.variant symbols).
In `@clients/web/apps/dashboard/src/components/ui/input/Input.vue`:
- Around line 16-24: The template in Input.vue duplicates parent classes by
including ($attrs.class as string) in :class while also using v-bind="$attrs";
remove the duplicated extraction and change the :class binding to only include
the local class (e.g., :class="['form-input']") so that v-bind="$attrs" handles
merging parent classes automatically, keeping the existing v-bind="$attrs",
:type, :value and `@input` handler unchanged.
In `@clients/web/apps/dashboard/src/locales/es.json`:
- Around line 2-47: Update the Spanish locale values to include missing accents:
change "app.title" subtitle value "Configuracion segura del gateway" to
"Configuración segura del gateway"; "auth.pairingCode" to "Código de
emparejamiento"; "auth.loadError" to "No se pudo cargar la configuración
admin."; "form.saveSuccess" to "Configuración guardada."; "form.saveError" to
"No se pudo guardar la configuración."; "form.autonomyLevel" to "Nivel de
autonomía"; "form.maxActionsPerHour" to "Máx acciones por hora";
"form.maxCostPerDayCents" to "Máx costo por día (centavos)";
"form.workspaceOnly" to "Sólo workspace"; and "form.allowPublicBind" to
"Permitir bind público" (update the string values for these keys in the JSON).
In `@clients/web/apps/dashboard/src/style.css`:
- Line 17: The CSS font-family value uses a capitalized BlinkMacSystemFont which
violates Stylelint's value-keyword-case rule; update the font-family entry so
the token BlinkMacSystemFont is lowercased to blinkmacsystemfont (preserve
surrounding quotes/commas and other font names) so the line containing "Inter
Variable", "Inter", system-ui, -apple-system, BlinkMacSystemFont, "Segoe UI",
sans-serif; becomes compliant.
In `@clients/web/apps/dashboard/vite.config.js`:
- Around line 1-14: This file (the JS Vite config that exports default
defineConfig and sets plugins: [vue()] plus the "@" alias) must be deleted
because it silently overrides vite.config.ts and is excluded from
formatting/linting; remove the entire vite.config.js file so Vite will load
vite.config.ts and project style/biome checks apply.
In `@clients/web/README.md`:
- Around line 54-58: Update the "dashboard" README section to document the
required authentication and credentials for accessing the admin endpoints:
explicitly state that GET/PUT /web/admin/config require authentication, list the
supported auth mechanism(s) (e.g., bearer token, API key, admin secret), how to
obtain/set the token/secret, where to configure it for local development (env
var name or config file), and any scope/roles required; reference the endpoint
names GET/PUT /web/admin/config and the "dashboard" section so readers can find
the details quickly.
---
Nitpick comments:
In `@clients/web/apps/dashboard/src/App.vue`:
- Around line 102-107: The default ports for gatewayPort and webhookPort both
use "3000", which will conflict since they serialize to distinct keys
(gateway.port and webhook.port); decide whether they should share a port (same
server) or run separately and, if separate, change the webhookPort default to a
different value (e.g., "3001") in the App.vue setup where gatewayPort and
webhookPort are defined so the webhook listener won’t collide with the gateway
listener.
- Around line 181-183: The bare catch blocks (the ones that currently do "}
catch { errorMessage.value = t('auth.unauthorized'); }" and the two similar
instances later) discard the original error; change each to "catch (err)" and
log the error (e.g., console.error(err)) before setting user-facing state so the
real network/parse/server error is preserved for debugging; update all three
catch sites that set errorMessage.value to t("auth.unauthorized") to follow this
pattern.
In `@clients/web/apps/dashboard/src/components/ui/input/Input.vue`:
- Around line 4-8: The prop "type" in defineProps currently accepts any string;
narrow it to valid HTML input types by replacing its type annotation (the "type"
prop in Input.vue's defineProps) with the DOM union type HTMLInputTypeAttribute
(or an explicit union of allowed literals like 'text'|'password'|'email' etc.)
so IDE autocomplete and type-safety are enabled; update any usages in the
component (e.g., where "type" is passed to the <input>) to match the narrowed
type and fix any resulting type errors.
In `@clients/web/apps/dashboard/src/style.css`:
- Around line 24-29: The CSS rule using min-height: 100% on `#app` can fail
because percentage heights need a definite parent height; update the stylesheet
so html has height: 100% and set body and `#app` to use a viewport-based
min-height (prefer 100dvh with a 100vh fallback) to reliably fill the viewport
across browsers; modify the selectors html, body, and `#app` accordingly (ensure
you change the existing html, body, `#app` block and the `#app` rule to use height:
100% on html and min-height: 100dvh; min-height: 100vh on body and `#app`).
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
clients/web/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml,!**/pnpm-lock.yaml
📒 Files selected for processing (26)
clients/agent-runtime/src/gateway/mod.rsclients/web/README.mdclients/web/apps/chat/README.mdclients/web/apps/chat/src/App.spec.tsclients/web/apps/chat/src/App.vueclients/web/apps/chat/src/components/chat/ChatMessage.vueclients/web/apps/chat/src/locales/es.jsonclients/web/apps/dashboard/README.mdclients/web/apps/dashboard/components.jsonclients/web/apps/dashboard/index.htmlclients/web/apps/dashboard/package.jsonclients/web/apps/dashboard/src/App.spec.tsclients/web/apps/dashboard/src/App.vueclients/web/apps/dashboard/src/components/ui/button/Button.vueclients/web/apps/dashboard/src/components/ui/input/Input.vueclients/web/apps/dashboard/src/i18n.tsclients/web/apps/dashboard/src/locales/es.jsonclients/web/apps/dashboard/src/main.tsclients/web/apps/dashboard/src/style.cssclients/web/apps/dashboard/tsconfig.app.jsonclients/web/apps/dashboard/tsconfig.jsonclients/web/apps/dashboard/tsconfig.node.jsonclients/web/apps/dashboard/vite.config.jsclients/web/apps/dashboard/vite.config.tsclients/web/build.gradle.ktsclients/web/package.json
Deploying corvus-plugins with
|
| Latest commit: |
d20e17d
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://df4e819c.corvus-plugins.pages.dev |
| Branch Preview URL: | https://feature-dashboard.corvus-plugins.pages.dev |
Deploying corvus with
|
| Latest commit: |
d20e17d
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://7477a966.corvus-42x.pages.dev |
| Branch Preview URL: | https://feature-dashboard.corvus-42x.pages.dev |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
clients/web/apps/dashboard/src/App.vue (1)
309-377:⚠️ Potential issue | 🟠 MajorBlock unsafe URLs even when clearing the webhook secret.
authHeaders()still sendsAuthorizationwhenever a token exists (andcanSaveenforces that), but the guard skips whenwebhookSecretMode === "clear". That allows bearer tokens to be sent to an unsafe base URL. The guard should apply wheneverAuthorizationmay be sent (or alternatively omitAuthorizationfor unsafe URLs).🔒 Suggested fix
- if (webhookSecretMode.value !== "clear" && !isUrlSafeForSecrets(gatewayBaseUrl)) { + if (!isUrlSafeForSecrets(gatewayBaseUrl)) { errorMessage.value = t("auth.insecureUrlError"); saving.value = false; return; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/web/apps/dashboard/src/App.vue` around lines 309 - 377, The guard that checks isUrlSafeForSecrets(gatewayBaseUrl) currently skips when webhookSecretMode.value === "clear", which still allows authHeaders() (and thus Authorization) to be sent to an unsafe gatewayBaseUrl; update the condition so the URL safety check runs whenever authHeaders() may attach a token (e.g., when canSave or authHeaders() would include Authorization) — specifically modify the existing guard around gatewayBaseUrl and isUrlSafeForSecrets to not exempt webhookSecretMode === "clear" (or alternatively strip Authorization in authHeaders() when isUrlSafeForSecrets returns false for gatewayBaseUrl) so that requests built for new URL in fetch(new URL("/web/admin/config", gatewayBaseUrl).toString(), ...) never carry credentials to unsafe hosts.
🤖 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/mod.rs`:
- Around line 341-357: The function restart_required_updates should also detect
changes to the default_provider so UI updates that field but runtime continues
using the old provider aren't misleading; in restart_required_updates (which
takes &Config and &AdminConfigUpdateRequest) add logic analogous to the
default_model/temperature checks to compare patch.default_provider (after
trimming/normalizing as needed) with cfg.default_provider and push
"default_provider" into the returned Vec when they differ, ensuring provider
changes are marked restart-required instead of silently mutating the config.
---
Duplicate comments:
In `@clients/web/apps/dashboard/src/App.vue`:
- Around line 309-377: The guard that checks isUrlSafeForSecrets(gatewayBaseUrl)
currently skips when webhookSecretMode.value === "clear", which still allows
authHeaders() (and thus Authorization) to be sent to an unsafe gatewayBaseUrl;
update the condition so the URL safety check runs whenever authHeaders() may
attach a token (e.g., when canSave or authHeaders() would include Authorization)
— specifically modify the existing guard around gatewayBaseUrl and
isUrlSafeForSecrets to not exempt webhookSecretMode === "clear" (or
alternatively strip Authorization in authHeaders() when isUrlSafeForSecrets
returns false for gatewayBaseUrl) so that requests built for new URL in
fetch(new URL("/web/admin/config", gatewayBaseUrl).toString(), ...) never carry
credentials to unsafe hosts.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (9)
clients/agent-runtime/src/gateway/mod.rsclients/web/README.mdclients/web/apps/chat/src/locales/es.jsonclients/web/apps/dashboard/README.mdclients/web/apps/dashboard/src/App.vueclients/web/apps/dashboard/src/components/ui/button/Button.vueclients/web/apps/dashboard/src/components/ui/input/Input.vueclients/web/apps/dashboard/src/locales/es.jsonclients/web/apps/dashboard/src/style.css
🚧 Files skipped from review as they are similar to previous changes (3)
- clients/web/apps/dashboard/src/style.css
- clients/web/README.md
- clients/web/apps/dashboard/src/components/ui/input/Input.vue
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/mod.rs`:
- Around line 404-417: The comparison logic incorrectly clamps incoming max-keys
to >=1 using max(1), preventing a client from sending 0 to mean "use default"
and causing false restart detections; update the checks in the update path (the
blocks referencing gateway.rate_limit_max_keys and gateway.idempotency_max_keys
and the idempotency_ttl_secs/idempotency_max_keys group) to use the same
normalization used elsewhere (call or apply
normalize_max_keys/normalize_idempotency_max_keys behavior) or explicitly treat
0 as "use cfg value" before comparing to cfg.gateway.*; ensure you reference
normalize_max_keys (or the equivalent helper) and the gateway.* fields so the
comparison uses normalized values rather than max(1).
- Around line 341-463: The restart_required_updates function currently compares
only gateway/webhook/default_* fields and misses config sections built at
startup (autonomy/runtime/memory/observability), so add explicit comparisons for
those patch fields: inside restart_required_updates, detect when patch.autonomy,
patch.runtime, patch.memory, or patch.observability are Some(...) and compare
their relevant subfields to cfg.autonomy / cfg.runtime / cfg.memory /
cfg.observability (use the same pattern as for gateway/webhook: handle
Option/trim/max/defaults and AdminSecretUpdate variants where applicable) and
push the corresponding string names (e.g., "autonomy.policy", "runtime.engine",
"memory.limit", "observability.metrics", or the real field names from
AdminConfigUpdateRequest) into fields so these startup-only changes are flagged
as restart-required; keep using fields.sort_unstable() and fields.dedup() as
already present.
In `@clients/web/apps/dashboard/src/App.vue`:
- Around line 309-321: When building secretPayload, ensure webhookSecretMode ===
"replace" requires a non-empty trimmed webhookSecretValue; if the value is empty
after trim, set errorMessage.value to the appropriate validation message (e.g.,
t("auth.emptyWebhookSecret")), set saving.value = false and return so we don’t
send a replace with an empty secret. Update the branch that creates
secretPayload (references: webhookSecretMode, webhookSecretValue, secretPayload,
saving.value, errorMessage.value) to perform this guard and direct users to
choose "clear" if they intend to remove the secret.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
clients/agent-runtime/src/gateway/mod.rsclients/web/apps/dashboard/src/App.vue
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
clients/web/apps/dashboard/src/App.vue (1)
137-139: Trim whitespace innormalizeBaseUrlfor resilience.
Users often paste URLs with leading/trailing spaces; trimming here avoidsnew URL()failures downstream.🧹 Proposed tweak
function normalizeBaseUrl(): string { - return baseUrl.value.replace(/\/$/, ""); + return baseUrl.value.trim().replace(/\/$/, ""); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/web/apps/dashboard/src/App.vue` around lines 137 - 139, normalizeBaseUrl currently only strips a trailing slash which fails if baseUrl.value has leading/trailing whitespace; update the function (normalizeBaseUrl) to trim whitespace from baseUrl.value first (e.g., call .trim() on baseUrl.value) before removing the trailing slash so downstream new URL() calls won't error on pasted urls with spaces.
🤖 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/mod.rs`:
- Around line 305-324: admin_requires_auth currently returns None when
state.pairing.require_pairing() is false, which leaves /web/admin/* unprotected;
change the logic so admin endpoints always require a valid bearer token: remove
the early short-circuit on require_pairing() and always extract and validate the
token via extract_bearer_token(...) and state.pairing.is_authenticated(...),
returning StatusCode::UNAUTHORIZED with the same JSON if invalid; alternatively,
if you prefer a configurable option, introduce a new flag (e.g.,
pairing.admin_requires_pairing defaulting to true) and check that flag instead
of require_pairing() in admin_requires_auth so admin protection remains
secure-by-default.
---
Nitpick comments:
In `@clients/web/apps/dashboard/src/App.vue`:
- Around line 137-139: normalizeBaseUrl currently only strips a trailing slash
which fails if baseUrl.value has leading/trailing whitespace; update the
function (normalizeBaseUrl) to trim whitespace from baseUrl.value first (e.g.,
call .trim() on baseUrl.value) before removing the trailing slash so downstream
new URL() calls won't error on pasted urls with spaces.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
clients/agent-runtime/src/gateway/mod.rsclients/web/apps/dashboard/src/App.vueclients/web/apps/dashboard/src/locales/es.json
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (4)
clients/agent-runtime/src/gateway/mod.rs (1)
1077-1079: Consider combining the two/web/admin/configroutes.Axum 0.8 supports
method_routerchaining on a single route path, which avoids registering the same path twice and makes the routing slightly more explicit.♻️ Optional refactor
- .route("/web/admin/config", get(handle_admin_get_config)) - .route("/web/admin/config", put(handle_admin_update_config)) + .route("/web/admin/config", get(handle_admin_get_config).put(handle_admin_update_config))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/agent-runtime/src/gateway/mod.rs` around lines 1077 - 1079, The two separate route registrations for "/web/admin/config" should be combined into a single route call to use Axum's method-router chaining: replace the two .route("/web/admin/config", get(handle_admin_get_config)) and .route("/web/admin/config", put(handle_admin_update_config)) entries with one .route("/web/admin/config", get(handle_admin_get_config).put(handle_admin_update_config)) (keep the existing .route("/web/admin/options", get(handle_admin_options)) unchanged); this removes duplicate path registration and makes the HTTP-method-to-handler mapping explicit.clients/web/apps/dashboard/src/App.vue (3)
380-403:saveConfigdiscards the 409 Conflict body, hiding restart-required field feedback.The backend returns a structured
{ restart_required: true, fields: [...] }payload on 409, but thethrow new Error(${response.status})at Line 387 discards the body. The user only sees a generic "save error" with no indication of which fields need a restart. Parsing the 409 response would let the dashboard display actionable guidance.♻️ Suggested improvement
if (!response.ok) { - throw new Error(`${response.status}`); + const errorBody = await response.json().catch(() => null); + if (response.status === 409 && errorBody?.restart_required) { + const fieldList = (errorBody.fields as string[])?.join(", ") ?? ""; + errorMessage.value = t("form.restartRequired", { fields: fieldList }); + saving.value = false; + return; + } + throw new Error(`${response.status}`); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/web/apps/dashboard/src/App.vue` around lines 380 - 403, The current saveConfig error path throws a generic Error on any non-OK response which discards the response body (so the 409 payload with restart_required and fields is lost); change the PUT response handling in the saveConfig block to specifically handle response.status === 409 by await response.json(), extract restart_required and fields, and set appropriate reactive state (e.g., restartRequired.value and restartFields.value or populate a detailed errorMessage) instead of throwing; for other non-OK statuses keep the existing generic error flow (or throw) and preserve existing updates to statusMessage, webhookSecretExists, webhookSecretValue, webhookSecretMode, errorMessage, and saving.value.
338-378: Payload always sendsruntime.kind,memory_backend,observability.backend— changes trigger 409.These fields are restart-required on the server. Because
saveConfigalways includes them in the payload, if the server's current value differs from what the dashboard loaded (e.g., another admin changed it in the meantime), the save fails with a 409 that the user can't resolve from the dashboard. Consider either omitting unchanged fields from the payload or re-fetching config on 409 to detect drift.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/web/apps/dashboard/src/App.vue` around lines 338 - 378, The payload builder always includes restart-required fields (e.g., runtime.kind, memory_backend, observability.backend) which causes 409s when those values drift; update the logic that builds payload (the const payload block) to only include restart-required keys when their current value differs from the originally loaded config (compare provider.value, model.value, memoryBackend.value, runtimeKind.value, observabilityBackend.value, etc. against a stored initialConfig object) and/or implement 409 handling in saveConfig to re-fetch the latest config and surface drift to the user; in short, omit unchanged restart-required fields from payload by checking against initialConfig before spreading them in payload, and add a retry-case on HTTP 409 in saveConfig that refetches config and notifies the user.
188-204: AllpairGatewayerrors surface as "unauthorized", masking network failures.If the gateway is unreachable (DNS failure, connection refused, timeout), the user sees the
auth.unauthorizedmessage, which is misleading. Consider differentiating network errors from HTTP 403/401 responses.♻️ Suggested improvement
} catch (err) { console.error("pairGateway failed", err); - errorMessage.value = t("auth.unauthorized"); + const msg = err instanceof Error ? err.message : ""; + if (msg === "403" || msg === "Missing token") { + errorMessage.value = t("auth.unauthorized"); + } else if (msg === "429") { + errorMessage.value = t("auth.pairingRateLimitError"); + } else { + errorMessage.value = t("auth.loadError"); + } } finally {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/web/apps/dashboard/src/App.vue` around lines 188 - 204, The catch-all currently maps every failure to auth.unauthorized; update the error handling in the pairGateway flow so network/connection errors are shown differently from HTTP 401/403 responses: when you throw on non-ok responses (inside the block that currently does throw new Error(`${response.status}`)), throw a distinguishable error (e.g., include a prefix like "HTTP_" + response.status or set error.name), then in the catch block inspect the error (check err.name === "AbortError" or err instanceof TypeError or err.message startsWith("HTTP_")/parseInt for status) and set errorMessage.value to t("auth.networkError") for network/timeout/DNS failures and t("auth.unauthorized") only for HTTP 401/403; keep updating bearerToken.value, pairingCode.value, statusMessage.value and loading.value as before.
🤖 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/mod.rs`:
- Around line 1296-1509: The handler handle_admin_update_config currently
mutates the locked shared cfg in-place and returns early on validation errors,
leaving a partially-updated in-memory Config; instead, clone the config first
(let cfg_clone = cfg.clone() or read without mutating), perform all mutations
and validations against that local mutable copy (use
restart_required_updates(&cfg_clone, &patch) and apply changes to cfg_clone),
and only when all validations pass acquire the lock again to replace the shared
state with the updated cfg_clone and call cfg.save() (or the existing
persistence path); ensure all references to cfg in the function that perform
validation/mutation (e.g., memory_backend handling, observability.backend,
runtime.kind, plugins.install_policy, gateway/webhook sections) are applied to
the cloned config until the final commit.
- Around line 483-492: The idempotency TTL compare currently normalizes a sent 0
to cfg.gateway.idempotency_ttl_secs.max(1), causing a spurious diff when the
stored cfg.gateway.idempotency_ttl_secs is 0; change the logic in the gateway
idempotency check so that a client-sent ttl of 0 is treated as a no-op (i.e.,
use the existing stored cfg.gateway.idempotency_ttl_secs for comparison or skip
the field entirely) instead of forcing max(1), so the comparison in the block
around gateway.idempotency_ttl_secs matches the update path behavior that
ignores 0.
- Around line 345-543: The restart_required_updates function is missing checks
for scheduler and plugins changes; update restart_required_updates to compare
patch.scheduler.* against cfg.scheduler.* and patch.plugins.* against
cfg.plugins.* and push the corresponding names when different: check
scheduler.enabled, scheduler.max_tasks, scheduler.max_concurrent (compare
Option<bool>/Option<usize>/Option<usize> values in patch.scheduler to
cfg.scheduler) and check plugins.enabled and plugins.install_policy (compare
Option<bool>/Option<...> and Option<...> in patch.plugins to cfg.plugins),
trim/normalize values the same way other sections do, then push
"scheduler.enabled", "scheduler.max_tasks", "scheduler.max_concurrent",
"plugins.enabled", and "plugins.install_policy" into fields when they differ.
---
Nitpick comments:
In `@clients/agent-runtime/src/gateway/mod.rs`:
- Around line 1077-1079: The two separate route registrations for
"/web/admin/config" should be combined into a single route call to use Axum's
method-router chaining: replace the two .route("/web/admin/config",
get(handle_admin_get_config)) and .route("/web/admin/config",
put(handle_admin_update_config)) entries with one .route("/web/admin/config",
get(handle_admin_get_config).put(handle_admin_update_config)) (keep the existing
.route("/web/admin/options", get(handle_admin_options)) unchanged); this removes
duplicate path registration and makes the HTTP-method-to-handler mapping
explicit.
In `@clients/web/apps/dashboard/src/App.vue`:
- Around line 380-403: The current saveConfig error path throws a generic Error
on any non-OK response which discards the response body (so the 409 payload with
restart_required and fields is lost); change the PUT response handling in the
saveConfig block to specifically handle response.status === 409 by await
response.json(), extract restart_required and fields, and set appropriate
reactive state (e.g., restartRequired.value and restartFields.value or populate
a detailed errorMessage) instead of throwing; for other non-OK statuses keep the
existing generic error flow (or throw) and preserve existing updates to
statusMessage, webhookSecretExists, webhookSecretValue, webhookSecretMode,
errorMessage, and saving.value.
- Around line 338-378: The payload builder always includes restart-required
fields (e.g., runtime.kind, memory_backend, observability.backend) which causes
409s when those values drift; update the logic that builds payload (the const
payload block) to only include restart-required keys when their current value
differs from the originally loaded config (compare provider.value, model.value,
memoryBackend.value, runtimeKind.value, observabilityBackend.value, etc. against
a stored initialConfig object) and/or implement 409 handling in saveConfig to
re-fetch the latest config and surface drift to the user; in short, omit
unchanged restart-required fields from payload by checking against initialConfig
before spreading them in payload, and add a retry-case on HTTP 409 in saveConfig
that refetches config and notifies the user.
- Around line 188-204: The catch-all currently maps every failure to
auth.unauthorized; update the error handling in the pairGateway flow so
network/connection errors are shown differently from HTTP 401/403 responses:
when you throw on non-ok responses (inside the block that currently does throw
new Error(`${response.status}`)), throw a distinguishable error (e.g., include a
prefix like "HTTP_" + response.status or set error.name), then in the catch
block inspect the error (check err.name === "AbortError" or err instanceof
TypeError or err.message startsWith("HTTP_")/parseInt for status) and set
errorMessage.value to t("auth.networkError") for network/timeout/DNS failures
and t("auth.unauthorized") only for HTTP 401/403; keep updating
bearerToken.value, pairingCode.value, statusMessage.value and loading.value as
before.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
clients/agent-runtime/src/gateway/mod.rsclients/web/apps/dashboard/src/App.vue
Comment on lines
+345
to
+543
| fn restart_required_updates(cfg: &Config, patch: &AdminConfigUpdateRequest) -> Vec<&'static str> { | ||
| let mut fields = Vec::new(); | ||
|
|
||
| if let Some(provider) = patch.default_provider.as_ref() { | ||
| let provider = provider.trim(); | ||
| let next = (!provider.is_empty()).then_some(provider); | ||
| let current = cfg.default_provider.as_deref(); | ||
| if next != current { | ||
| fields.push("default_provider"); | ||
| } | ||
| } | ||
|
|
||
| if let Some(model) = patch.default_model.as_ref() { | ||
| let model = model.trim(); | ||
| let next = (!model.is_empty()).then_some(model); | ||
| let current = cfg.default_model.as_deref(); | ||
| if next != current { | ||
| fields.push("default_model"); | ||
| } | ||
| } | ||
|
|
||
| if let Some(temperature) = patch.default_temperature { | ||
| if temperature != cfg.default_temperature { | ||
| fields.push("default_temperature"); | ||
| } | ||
| } | ||
|
|
||
| if let Some(memory_backend) = patch.memory_backend.as_ref() { | ||
| let backend = memory_backend.trim().to_ascii_lowercase(); | ||
| if backend != cfg.memory.backend { | ||
| fields.push("memory_backend"); | ||
| } | ||
| } | ||
|
|
||
| if let Some(observability) = patch.observability.as_ref() { | ||
| if let Some(backend) = observability.backend.as_ref() { | ||
| let backend = backend.trim().to_ascii_lowercase(); | ||
| if backend != cfg.observability.backend { | ||
| fields.push("observability.backend"); | ||
| } | ||
| } | ||
|
|
||
| if let Some(endpoint) = observability.otel_endpoint.as_ref() { | ||
| let endpoint = endpoint.trim(); | ||
| let next = (!endpoint.is_empty()).then_some(endpoint); | ||
| let current = cfg.observability.otel_endpoint.as_deref(); | ||
| if next != current { | ||
| fields.push("observability.otel_endpoint"); | ||
| } | ||
| } | ||
|
|
||
| if let Some(service_name) = observability.otel_service_name.as_ref() { | ||
| let service_name = service_name.trim(); | ||
| let next = (!service_name.is_empty()).then_some(service_name); | ||
| let current = cfg.observability.otel_service_name.as_deref(); | ||
| if next != current { | ||
| fields.push("observability.otel_service_name"); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if let Some(runtime) = patch.runtime.as_ref() { | ||
| if let Some(kind) = runtime.kind.as_ref() { | ||
| let kind = kind.trim().to_ascii_lowercase(); | ||
| if kind != cfg.runtime.kind { | ||
| fields.push("runtime.kind"); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if let Some(autonomy) = patch.autonomy.as_ref() { | ||
| if let Some(level) = autonomy.level { | ||
| if level != cfg.autonomy.level { | ||
| fields.push("autonomy.level"); | ||
| } | ||
| } | ||
|
|
||
| if let Some(workspace_only) = autonomy.workspace_only { | ||
| if workspace_only != cfg.autonomy.workspace_only { | ||
| fields.push("autonomy.workspace_only"); | ||
| } | ||
| } | ||
|
|
||
| if let Some(max_actions_per_hour) = autonomy.max_actions_per_hour { | ||
| if max_actions_per_hour != cfg.autonomy.max_actions_per_hour { | ||
| fields.push("autonomy.max_actions_per_hour"); | ||
| } | ||
| } | ||
|
|
||
| if let Some(max_cost_per_day_cents) = autonomy.max_cost_per_day_cents { | ||
| if max_cost_per_day_cents != cfg.autonomy.max_cost_per_day_cents { | ||
| fields.push("autonomy.max_cost_per_day_cents"); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if let Some(gateway) = patch.gateway.as_ref() { | ||
| if let Some(port) = gateway.port { | ||
| if port != cfg.gateway.port { | ||
| fields.push("gateway.port"); | ||
| } | ||
| } | ||
| if let Some(host) = gateway.host.as_ref() { | ||
| if host.trim() != cfg.gateway.host { | ||
| fields.push("gateway.host"); | ||
| } | ||
| } | ||
| if let Some(require_pairing) = gateway.require_pairing { | ||
| if require_pairing != cfg.gateway.require_pairing { | ||
| fields.push("gateway.require_pairing"); | ||
| } | ||
| } | ||
| if let Some(allow_public_bind) = gateway.allow_public_bind { | ||
| if allow_public_bind != cfg.gateway.allow_public_bind { | ||
| fields.push("gateway.allow_public_bind"); | ||
| } | ||
| } | ||
| if let Some(pair_limit) = gateway.pair_rate_limit_per_minute { | ||
| if pair_limit != cfg.gateway.pair_rate_limit_per_minute { | ||
| fields.push("gateway.pair_rate_limit_per_minute"); | ||
| } | ||
| } | ||
| if let Some(webhook_limit) = gateway.webhook_rate_limit_per_minute { | ||
| if webhook_limit != cfg.gateway.webhook_rate_limit_per_minute { | ||
| fields.push("gateway.webhook_rate_limit_per_minute"); | ||
| } | ||
| } | ||
| if let Some(trust_forwarded_headers) = gateway.trust_forwarded_headers { | ||
| if trust_forwarded_headers != cfg.gateway.trust_forwarded_headers { | ||
| fields.push("gateway.trust_forwarded_headers"); | ||
| } | ||
| } | ||
| if let Some(max_keys) = gateway.rate_limit_max_keys { | ||
| let normalized = normalize_max_keys(max_keys, cfg.gateway.rate_limit_max_keys); | ||
| if normalized != cfg.gateway.rate_limit_max_keys { | ||
| fields.push("gateway.rate_limit_max_keys"); | ||
| } | ||
| } | ||
| if let Some(ttl) = gateway.idempotency_ttl_secs { | ||
| let normalized_ttl = if ttl == 0 { | ||
| cfg.gateway.idempotency_ttl_secs.max(1) | ||
| } else { | ||
| ttl | ||
| }; | ||
| if normalized_ttl != cfg.gateway.idempotency_ttl_secs { | ||
| fields.push("gateway.idempotency_ttl_secs"); | ||
| } | ||
| } | ||
| if let Some(max_keys) = gateway.idempotency_max_keys { | ||
| let normalized = normalize_max_keys(max_keys, cfg.gateway.idempotency_max_keys); | ||
| if normalized != cfg.gateway.idempotency_max_keys { | ||
| fields.push("gateway.idempotency_max_keys"); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if let Some(webhook) = patch.webhook.as_ref() { | ||
| if let Some(port) = webhook.port { | ||
| let current_port = cfg.channels_config.webhook.as_ref().map_or(3000, |w| w.port); | ||
| if port != current_port { | ||
| fields.push("webhook.port"); | ||
| } | ||
| } | ||
|
|
||
| if let Some(secret) = webhook.secret.as_ref() { | ||
| match secret { | ||
| AdminSecretUpdate::Unchanged => {} | ||
| AdminSecretUpdate::Clear => { | ||
| if cfg | ||
| .channels_config | ||
| .webhook | ||
| .as_ref() | ||
| .and_then(|w| w.secret.as_ref()) | ||
| .map(|value| !value.trim().is_empty()) | ||
| .unwrap_or(false) | ||
| { | ||
| fields.push("webhook.secret"); | ||
| } | ||
| } | ||
| AdminSecretUpdate::Replace { value } => { | ||
| let next = value.trim(); | ||
| let current = cfg | ||
| .channels_config | ||
| .webhook | ||
| .as_ref() | ||
| .and_then(|w| w.secret.as_deref()) | ||
| .unwrap_or(""); | ||
| if next != current { | ||
| fields.push("webhook.secret"); | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| fields.sort_unstable(); | ||
| fields.dedup(); | ||
| fields | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check how scheduler and plugins config values are consumed at runtime.
echo "=== Scheduler config structure and usage ==="
rg -nP --type=rust 'struct.*Scheduler|pub scheduler:' | head -20
echo ""
echo "=== Scheduler field access (reading config) ==="
rg -nP --type=rust 'scheduler\.(enabled|max_tasks|max_concurrent)' | head -30
echo ""
echo "=== Plugins config structure and usage ==="
rg -nP --type=rust 'struct.*Plugin|pub plugins:' | head -20
echo ""
echo "=== Plugins field access (reading config) ==="
rg -nP --type=rust 'plugins\.(enabled|install_policy)' | head -30
echo ""
echo "=== Config access patterns in scheduler/plugins code ==="
rg -nP --type=rust 'lock\(\)|read\(\)' src/ | grep -E '(scheduler|plugin)' | head -20
echo ""
echo "=== Check if scheduler/plugins cache config at startup ==="
rg -nP --type=rust 'fn new|fn init|fn start' src/scheduler/ src/plugins/ 2>/dev/null | head -20Repository: dallay/corvus
Length of output: 4753
🏁 Script executed:
#!/bin/bash
# Check how scheduler is initialized and how it accesses config
echo "=== Scheduler initialization/spawn ==="
rg -nP --type=rust 'spawn.*scheduler|new.*scheduler|fn.*scheduler' clients/agent-runtime/src/cron/scheduler.rs | head -10
echo ""
echo "=== How scheduler is created (look for function signature) ==="
head -120 clients/agent-runtime/src/cron/scheduler.rs | tail -50
echo ""
echo "=== Check if scheduler holds config reference or snapshot ==="
rg -nP --type=rust 'struct.*Scheduler' clients/agent-runtime/src/cron/scheduler.rs -A 10
echo ""
echo "=== How plugins are initialized ==="
rg -nP --type=rust 'fn new|impl.*Plugin' clients/agent-runtime/src/plugins/mod.rs -A 5 | head -40
echo ""
echo "=== Check how scheduler is spawned in main/runtime initialization ==="
rg -nP --type=rust 'tokio::spawn|spawn.*SchedulerService' clients/agent-runtime/src/ | head -20
echo ""
echo "=== Check how plugins are accessed after initialization ==="
rg -nP --type=rust 'config.plugins' clients/agent-runtime/src/ --type=rust | grep -v 'gateway/mod.rs' | head -20Repository: dallay/corvus
Length of output: 6630
🏁 Script executed:
#!/bin/bash
# Look at scheduler loop and how config is accessed
echo "=== Scheduler main loop (start of file to understand structure) ==="
head -150 clients/agent-runtime/src/cron/scheduler.rs
echo ""
echo "=== Search for main scheduler loop/entry point ==="
rg -nP --type=rust 'async.*fn|loop.*{' clients/agent-runtime/src/cron/scheduler.rs | head -20
echo ""
echo "=== Check main.rs to see how scheduler is spawned ==="
grep -n -A 10 'scheduler\|cron' clients/agent-runtime/src/main.rs | head -50
echo ""
echo "=== Check how Arc<Mutex<Config>> is used with scheduler ==="
rg -nP --type=rust 'Arc.*Config|Mutex.*Config' clients/agent-runtime/src/cron/scheduler.rs
echo ""
echo "=== Check GatewayServer initialization and config updates ==="
rg -nP --type=rust 'impl.*GatewayServer|fn new' clients/agent-runtime/src/gateway/mod.rs | head -10Repository: dallay/corvus
Length of output: 8127
🏁 Script executed:
#!/bin/bash
# Check how daemon initializes and manages scheduler
echo "=== Look for daemon initialization ==="
grep -n -B 5 -A 15 'fn daemon\|async fn daemon' clients/agent-runtime/src/main.rs | head -60
echo ""
echo "=== Check if scheduler is respawned on config updates ==="
rg -nP --type=rust 'scheduler.*spawn|restart.*scheduler' clients/agent-runtime/src/
echo ""
echo "=== Look for daemon struct and config field ==="
rg -nP --type=rust 'struct.*Daemon|pub config:' clients/agent-runtime/src/daemon/mod.rs | head -20
echo ""
echo "=== Check daemon loop for config reloading ==="
head -200 clients/agent-runtime/src/daemon/mod.rs | tail -150Repository: dallay/corvus
Length of output: 5702
🏁 Script executed:
#!/bin/bash
# Final verification: check how gateway interacts with config Arc<Mutex> and daemon
echo "=== How is Arc<Mutex<Config>> managed in gateway ==="
rg -nP --type=rust 'Arc.*Mutex.*config|config_guard|config.lock' clients/agent-runtime/src/gateway/mod.rs | head -15
echo ""
echo "=== Check gateway update pattern ==="
rg -nP --type=rust 'fn.*update_config|AdminConfigUpdateRequest' clients/agent-runtime/src/gateway/mod.rs -A 5 | head -40
echo ""
echo "=== Verify daemon doesn't use Arc<Mutex<Config>> for supervisor ==="
rg -nP --type=rust 'pub async fn daemon|Arc.*Mutex' clients/agent-runtime/src/daemon/mod.rs | head -20Repository: dallay/corvus
Length of output: 1892
Add scheduler and plugins fields to restart detection.
restart_required_updates omits scheduler.enabled, scheduler.max_tasks, scheduler.max_concurrent, plugins.enabled, and plugins.install_policy. These fields must be included because the scheduler and plugins components receive a config snapshot at daemon startup via config.clone() in the supervisor, not a reference to the shared Arc<Mutex<Config>>. The gateway updates the Arc, but the scheduler's snapshot is never refreshed—changes to these fields will silently not take effect without a full daemon restart.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@clients/agent-runtime/src/gateway/mod.rs` around lines 345 - 543, The
restart_required_updates function is missing checks for scheduler and plugins
changes; update restart_required_updates to compare patch.scheduler.* against
cfg.scheduler.* and patch.plugins.* against cfg.plugins.* and push the
corresponding names when different: check scheduler.enabled,
scheduler.max_tasks, scheduler.max_concurrent (compare
Option<bool>/Option<usize>/Option<usize> values in patch.scheduler to
cfg.scheduler) and check plugins.enabled and plugins.install_policy (compare
Option<bool>/Option<...> and Option<...> in patch.plugins to cfg.plugins),
trim/normalize values the same way other sections do, then push
"scheduler.enabled", "scheduler.max_tasks", "scheduler.max_concurrent",
"plugins.enabled", and "plugins.install_policy" into fields when they differ.
Comment on lines
+483
to
+492
| if let Some(ttl) = gateway.idempotency_ttl_secs { | ||
| let normalized_ttl = if ttl == 0 { | ||
| cfg.gateway.idempotency_ttl_secs.max(1) | ||
| } else { | ||
| ttl | ||
| }; | ||
| if normalized_ttl != cfg.gateway.idempotency_ttl_secs { | ||
| fields.push("gateway.idempotency_ttl_secs"); | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
idempotency_ttl_secs normalization uses max(1) on the current config value.
When ttl == 0, the code computes cfg.gateway.idempotency_ttl_secs.max(1). If the stored config value is 0, the normalized becomes 1, which differs from the stored 0, triggering a false restart-required flag. Meanwhile, the update path (Line 1464-1468) silently ignores 0. The semantics are: "sending 0 is a no-op", but the restart check says "this differs from current". This inconsistency only manifests when the persisted config itself holds 0.
🔧 Suggested fix — match the update path's no-op semantics
if let Some(ttl) = gateway.idempotency_ttl_secs {
- let normalized_ttl = if ttl == 0 {
- cfg.gateway.idempotency_ttl_secs.max(1)
- } else {
- ttl
- };
- if normalized_ttl != cfg.gateway.idempotency_ttl_secs {
+ // 0 is treated as "keep current" in the update path — skip restart detection.
+ if ttl != 0 && ttl != cfg.gateway.idempotency_ttl_secs {
fields.push("gateway.idempotency_ttl_secs");
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@clients/agent-runtime/src/gateway/mod.rs` around lines 483 - 492, The
idempotency TTL compare currently normalizes a sent 0 to
cfg.gateway.idempotency_ttl_secs.max(1), causing a spurious diff when the stored
cfg.gateway.idempotency_ttl_secs is 0; change the logic in the gateway
idempotency check so that a client-sent ttl of 0 is treated as a no-op (i.e.,
use the existing stored cfg.gateway.idempotency_ttl_secs for comparison or skip
the field entirely) instead of forcing max(1), so the comparison in the block
around gateway.idempotency_ttl_secs matches the update path behavior that
ignores 0.
Comment on lines
+1296
to
+1509
| let mut cfg = state.config.lock(); | ||
|
|
||
| let restart_required_fields = restart_required_updates(&cfg, &patch); | ||
| if !restart_required_fields.is_empty() { | ||
| return ( | ||
| StatusCode::CONFLICT, | ||
| Json(serde_json::json!({ | ||
| "error": "One or more requested config changes require a gateway restart to take effect.", | ||
| "restart_required": true, | ||
| "fields": restart_required_fields, | ||
| })), | ||
| ); | ||
| } | ||
|
|
||
| if let Some(provider) = patch.default_provider { | ||
| let provider = provider.trim(); | ||
| cfg.default_provider = (!provider.is_empty()).then(|| provider.to_string()); | ||
| } | ||
|
|
||
| if let Some(model) = patch.default_model { | ||
| let model = model.trim(); | ||
| cfg.default_model = (!model.is_empty()).then(|| model.to_string()); | ||
| } | ||
|
|
||
| if let Some(temperature) = patch.default_temperature { | ||
| if !(0.0..=2.0).contains(&temperature) { | ||
| return ( | ||
| StatusCode::BAD_REQUEST, | ||
| Json(serde_json::json!({ | ||
| "error": "default_temperature must be in range [0.0, 2.0]" | ||
| })), | ||
| ); | ||
| } | ||
| cfg.default_temperature = temperature; | ||
| } | ||
|
|
||
| if let Some(memory_backend) = patch.memory_backend { | ||
| let backend = memory_backend.trim().to_ascii_lowercase(); | ||
| if !validate_memory_backend(&backend) { | ||
| return ( | ||
| StatusCode::BAD_REQUEST, | ||
| Json(serde_json::json!({ | ||
| "error": "Invalid memory_backend. Allowed: sqlite, lucid, surreal-graphs, markdown, surreal, none" | ||
| })), | ||
| ); | ||
| } | ||
| cfg.memory.backend = backend; | ||
| } | ||
|
|
||
| if let Some(observability_patch) = patch.observability { | ||
| if let Some(backend) = observability_patch.backend { | ||
| let backend = backend.trim().to_ascii_lowercase(); | ||
| if !validate_observability_backend(&backend) { | ||
| return ( | ||
| StatusCode::BAD_REQUEST, | ||
| Json(serde_json::json!({ | ||
| "error": "Invalid observability.backend. Allowed: none, log, prometheus, otel" | ||
| })), | ||
| ); | ||
| } | ||
| cfg.observability.backend = backend; | ||
| } | ||
|
|
||
| if let Some(endpoint) = observability_patch.otel_endpoint { | ||
| let endpoint = endpoint.trim(); | ||
| cfg.observability.otel_endpoint = (!endpoint.is_empty()).then(|| endpoint.to_string()); | ||
| } | ||
|
|
||
| if let Some(service_name) = observability_patch.otel_service_name { | ||
| let service_name = service_name.trim(); | ||
| cfg.observability.otel_service_name = | ||
| (!service_name.is_empty()).then(|| service_name.to_string()); | ||
| } | ||
| } | ||
|
|
||
| if let Some(runtime_patch) = patch.runtime { | ||
| if let Some(kind) = runtime_patch.kind { | ||
| let kind = kind.trim().to_ascii_lowercase(); | ||
| if !validate_runtime_kind(&kind) { | ||
| return ( | ||
| StatusCode::BAD_REQUEST, | ||
| Json(serde_json::json!({ | ||
| "error": "Invalid runtime.kind. Allowed: native, docker" | ||
| })), | ||
| ); | ||
| } | ||
| cfg.runtime.kind = kind; | ||
| } | ||
| } | ||
|
|
||
| if let Some(autonomy_patch) = patch.autonomy { | ||
| if let Some(level) = autonomy_patch.level { | ||
| cfg.autonomy.level = level; | ||
| } | ||
| if let Some(workspace_only) = autonomy_patch.workspace_only { | ||
| cfg.autonomy.workspace_only = workspace_only; | ||
| } | ||
| if let Some(max_actions_per_hour) = autonomy_patch.max_actions_per_hour { | ||
| cfg.autonomy.max_actions_per_hour = max_actions_per_hour; | ||
| } | ||
| if let Some(max_cost_per_day_cents) = autonomy_patch.max_cost_per_day_cents { | ||
| cfg.autonomy.max_cost_per_day_cents = max_cost_per_day_cents; | ||
| } | ||
| } | ||
|
|
||
| if let Some(scheduler_patch) = patch.scheduler { | ||
| if let Some(enabled) = scheduler_patch.enabled { | ||
| cfg.scheduler.enabled = enabled; | ||
| } | ||
| if let Some(max_tasks) = scheduler_patch.max_tasks { | ||
| cfg.scheduler.max_tasks = max_tasks.max(1); | ||
| } | ||
| if let Some(max_concurrent) = scheduler_patch.max_concurrent { | ||
| cfg.scheduler.max_concurrent = max_concurrent.max(1); | ||
| } | ||
| } | ||
|
|
||
| if let Some(plugins_patch) = patch.plugins { | ||
| if let Some(enabled) = plugins_patch.enabled { | ||
| cfg.plugins.enabled = enabled; | ||
| } | ||
| if let Some(install_policy) = plugins_patch.install_policy { | ||
| let install_policy = install_policy.trim(); | ||
| if install_policy != "pin-manual" { | ||
| return ( | ||
| StatusCode::BAD_REQUEST, | ||
| Json(serde_json::json!({ | ||
| "error": "Invalid plugins.install_policy. Allowed: pin-manual" | ||
| })), | ||
| ); | ||
| } | ||
| cfg.plugins.install_policy = install_policy.to_string(); | ||
| } | ||
| } | ||
|
|
||
| if let Some(gateway_patch) = patch.gateway { | ||
| if let Some(port) = gateway_patch.port { | ||
| cfg.gateway.port = port; | ||
| } | ||
| if let Some(host) = gateway_patch.host { | ||
| let host = host.trim(); | ||
| if host.is_empty() { | ||
| return ( | ||
| StatusCode::BAD_REQUEST, | ||
| Json(serde_json::json!({"error": "gateway.host cannot be empty"})), | ||
| ); | ||
| } | ||
| cfg.gateway.host = host.to_string(); | ||
| } | ||
| if let Some(require_pairing) = gateway_patch.require_pairing { | ||
| cfg.gateway.require_pairing = require_pairing; | ||
| } | ||
| if let Some(allow_public_bind) = gateway_patch.allow_public_bind { | ||
| cfg.gateway.allow_public_bind = allow_public_bind; | ||
| } | ||
| if let Some(limit) = gateway_patch.pair_rate_limit_per_minute { | ||
| cfg.gateway.pair_rate_limit_per_minute = limit; | ||
| } | ||
| if let Some(limit) = gateway_patch.webhook_rate_limit_per_minute { | ||
| cfg.gateway.webhook_rate_limit_per_minute = limit; | ||
| } | ||
| if let Some(trust_forwarded_headers) = gateway_patch.trust_forwarded_headers { | ||
| cfg.gateway.trust_forwarded_headers = trust_forwarded_headers; | ||
| } | ||
| if let Some(max_keys) = gateway_patch.rate_limit_max_keys { | ||
| cfg.gateway.rate_limit_max_keys = | ||
| normalize_max_keys(max_keys, cfg.gateway.rate_limit_max_keys); | ||
| } | ||
| if let Some(ttl_secs) = gateway_patch.idempotency_ttl_secs { | ||
| if ttl_secs != 0 { | ||
| cfg.gateway.idempotency_ttl_secs = ttl_secs; | ||
| } | ||
| } | ||
| if let Some(max_keys) = gateway_patch.idempotency_max_keys { | ||
| cfg.gateway.idempotency_max_keys = | ||
| normalize_max_keys(max_keys, cfg.gateway.idempotency_max_keys); | ||
| } | ||
| } | ||
|
|
||
| if let Some(webhook_patch) = patch.webhook { | ||
| if webhook_patch.port.is_some() || webhook_patch.secret.is_some() { | ||
| if cfg.channels_config.webhook.is_none() { | ||
| cfg.channels_config.webhook = Some(crate::config::schema::WebhookConfig { | ||
| port: 3000, | ||
| secret: None, | ||
| }); | ||
| } | ||
|
|
||
| if let Some(webhook) = cfg.channels_config.webhook.as_mut() { | ||
| if let Some(port) = webhook_patch.port { | ||
| webhook.port = port; | ||
| } | ||
|
|
||
| if let Some(secret_mode) = webhook_patch.secret { | ||
| match secret_mode { | ||
| AdminSecretUpdate::Unchanged => {} | ||
| AdminSecretUpdate::Clear => webhook.secret = None, | ||
| AdminSecretUpdate::Replace { value } => { | ||
| let trimmed = value.trim(); | ||
| if trimmed.is_empty() { | ||
| return ( | ||
| StatusCode::BAD_REQUEST, | ||
| Json(serde_json::json!({ | ||
| "error": "webhook.secret replace value cannot be empty" | ||
| })), | ||
| ); | ||
| } | ||
| webhook.secret = Some(trimmed.to_string()); | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Partial in-memory config mutation on midway validation failure.
handle_admin_update_config acquires the config lock and mutates fields sequentially. If a validation check (e.g., validate_memory_backend at Line 1334, validate_observability_backend at Line 1348, validate_runtime_kind at Line 1374, or plugins.install_policy at Line 1419) fails and the handler returns BAD_REQUEST, all fields that were already applied to cfg remain mutated in the shared, in-memory Config. Since cfg.save() is never called, the on-disk config stays intact, but subsequent GET /web/admin/config reads will return a partially-mutated view — and a retry from the dashboard will compare against the already-mutated values.
Clone the config before mutation so the shared state is only updated on full success.
🐛 Proposed fix (outline)
- let mut cfg = state.config.lock();
+ let mut cfg_guard = state.config.lock();
+ let mut cfg = cfg_guard.clone();
let restart_required_fields = restart_required_updates(&cfg, &patch);
if !restart_required_fields.is_empty() {
@@ // ... all the mutation + validation code stays the same ...
let updated_view = admin_config_view(&cfg);
match cfg.save() {
- Ok(()) => (
+ Ok(()) => {
+ *cfg_guard = cfg;
+ (
StatusCode::OK,
Json(serde_json::json!({"updated": true, "config": updated_view})),
- ),
+ )
+ }
Err(error) => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@clients/agent-runtime/src/gateway/mod.rs` around lines 1296 - 1509, The
handler handle_admin_update_config currently mutates the locked shared cfg
in-place and returns early on validation errors, leaving a partially-updated
in-memory Config; instead, clone the config first (let cfg_clone = cfg.clone()
or read without mutating), perform all mutations and validations against that
local mutable copy (use restart_required_updates(&cfg_clone, &patch) and apply
changes to cfg_clone), and only when all validations pass acquire the lock again
to replace the shared state with the updated cfg_clone and call cfg.save() (or
the existing persistence path); ensure all references to cfg in the function
that perform validation/mutation (e.g., memory_backend handling,
observability.backend, runtime.kind, plugins.install_policy, gateway/webhook
sections) are applied to the cloned config until the final commit.
- Gateway: fix idempotency TTL comparison bug - Gateway: add restart detection for scheduler and plugins fields - Gateway: combine GET/PUT route registrations - Gateway: fix race condition by cloning config before restart check - Gateway: update shared config after successful save - Dashboard: track initial config snapshot for change detection - Dashboard: send only changed fields instead of full payload - Dashboard: handle 409 conflict with restart-required fields - Dashboard: differentiate network errors from HTTP errors - Dashboard: add Spanish i18n for new messages
This was referenced Mar 2, 2026
Merged
This was referenced Mar 18, 2026
This was referenced Apr 9, 2026
Merged
Merged
6 tasks
This was referenced Apr 11, 2026
Merged
This was referenced Apr 19, 2026
This was referenced Apr 29, 2026
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 significant improvements to the Corvus Chat web app, focusing on real gateway integration, enhanced authentication and error handling, and updates to documentation and tests. The chat frontend now interacts with real backend endpoints for pairing and messaging, supports bearer token authentication, and provides better user feedback on errors. Additionally, the codebase is streamlined by removing unused icon imports and inlining SVGs. Documentation is updated to reflect the new dashboard app and command changes.
Gateway Integration and Authentication:
/pairfor pairing and/webhookfor sending messages, replacing previous local/mock implementations. The pairing process retrieves a bearer token, which is then used for authenticating subsequent webhook requests. [1] [2] [3]Error Handling and User Feedback:
Testing Improvements:
UI and Code Cleanup:
Documentation and Monorepo Updates:
README.mdfiles to document the new dashboard app, its commands, and the real gateway integration in chat. Added build/test commands for the dashboard. [1] [2] [3] [4] [5] [6] [7]Summary by CodeRabbit
New Features
Bug Fixes / Improvements