feat: implement comprehensive auto-update system with centralized management and safety features#142
Merged
Merged
Conversation
…agement and safety features
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (8)
📜 Recent 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)
🧰 Additional context used📓 Path-based instructions (2)**/*.vue⚙️ CodeRabbit configuration file
Files:
**/*⚙️ CodeRabbit configuration file
Files:
🔇 Additional comments (9)
📝 WalkthroughSummary by CodeRabbitRelease Notes
WalkthroughAdds a centralized update subsystem (UpdateManager) with public status/policy contracts, install detection and install/state machine, process-safe locking, JSONL history, CLI/admin/dashboard surfaces, daemon supervision and channel notification/confirmation flows, plus config/schema extensions and tests. Changes
Sequence Diagram(s)sequenceDiagram
participant CLI
participant Daemon
participant Admin
participant UpdateManager
participant StateStore
participant Installer
participant Notifier
CLI->>UpdateManager: request status/check/install/confirm
Daemon->>UpdateManager: periodic check trigger
Admin->>UpdateManager: query status/policy
UpdateManager->>StateStore: acquire process-safe lock / read snapshot
UpdateManager->>Installer: detect method -> execute install (if requested)
Installer-->>UpdateManager: install result (Success/Failed/Busy)
UpdateManager->>StateStore: persist state & append JSONL history
UpdateManager->>Notifier: emit audit events / send notifications
Notifier-->>CLI: surface result
Notifier-->>Admin: update status/policy view
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ 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 |
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
Contributor
✅ Contributor ReportUser: @yacosta738
Contributor Report evaluates based on public GitHub activity. Analysis period: 2025-03-05 to 2026-03-05 |
![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: 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 `@openspec/changes/enhance-auto-update-system/design.md`:
- Around line 418-433: The phase ordering in the Phased Implementation Plan
conflicts with the dependency contract in tasks.md: swap the two phase blocks so
"Phase 1" is the Policy model + multi-surface visibility block and "Phase 2" is
the Safety + command foundation block, renumber the headings and any in-text
references to Phase 1/Phase 2 accordingly (update the "Phase 1: Safety + command
foundation" and "Phase 2: Policy model + multi-surface visibility" headings and
their bullet lists), and then re-run a quick cross-check against tasks.md to
ensure the sequence now matches the required policy/model/env precedence.
In `@openspec/changes/enhance-auto-update-system/exploration.md`:
- Around line 1-54: Change the top-level heading from "## Exploration: Enhance
Auto-Update System" to a single H1 ("# Exploration: Enhance Auto-Update System")
and ensure there is a blank line before and after each section heading (e.g.,
"Current State", "Key Touchpoints", "Risks", "Open Questions and Assumptions",
"Recommended Scope Boundaries", "Ready for Proposal") so the markdown satisfies
MD041/MD022; update the heading tokens in this file (the title string
"Exploration: Enhance Auto-Update System" and the section headings) to follow
the single H1 + blank-line-around-headings rule.
In `@openspec/changes/enhance-auto-update-system/specs/update-system/spec.md`:
- Around line 102-140: Add an explicit compatibility scenario for the existing
channel-initiated nonce-confirm flow to the CLI contract: document a new
scenario titled something like "Scenario: `update confirm <nonce>`
compatibility" that states GIVEN a channel-initiated install provides a nonce,
WHEN the user or automated agent runs `update confirm <nonce>`, THEN the CLI
accepts the nonce, completes the install handshake, and returns deterministic
success/failure semantics and audit-able history entries; ensure it also notes
this may be advanced/internal and that `update status`/`update history` reflect
actions from nonce-confirmed installs. Reference the exact command token `update
confirm <nonce>` and ensure the scenario is inserted alongside the other
`update` scenarios so implementers know not to regress this flow.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 2f1e3a5b-15fc-4621-996c-3c29b54a2c81
📒 Files selected for processing (5)
openspec/changes/enhance-auto-update-system/design.mdopenspec/changes/enhance-auto-update-system/exploration.mdopenspec/changes/enhance-auto-update-system/proposal.mdopenspec/changes/enhance-auto-update-system/specs/update-system/spec.mdopenspec/changes/enhance-auto-update-system/tasks.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). (3)
- GitHub Check: pr-checks
- GitHub Check: sonar
- GitHub Check: Cloudflare Pages
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{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/changes/enhance-auto-update-system/specs/update-system/spec.mdopenspec/changes/enhance-auto-update-system/tasks.mdopenspec/changes/enhance-auto-update-system/proposal.mdopenspec/changes/enhance-auto-update-system/exploration.mdopenspec/changes/enhance-auto-update-system/design.md
**/*
⚙️ 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:
openspec/changes/enhance-auto-update-system/specs/update-system/spec.mdopenspec/changes/enhance-auto-update-system/tasks.mdopenspec/changes/enhance-auto-update-system/proposal.mdopenspec/changes/enhance-auto-update-system/exploration.mdopenspec/changes/enhance-auto-update-system/design.md
🧠 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:
openspec/changes/enhance-auto-update-system/specs/update-system/spec.mdopenspec/changes/enhance-auto-update-system/proposal.mdopenspec/changes/enhance-auto-update-system/design.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/{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:
openspec/changes/enhance-auto-update-system/design.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/main.rs : Preserve CLI contract unless change is intentional and documented; prefer explicit errors over silent fallback for unsupported critical paths
Applied to files:
openspec/changes/enhance-auto-update-system/design.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/{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:
openspec/changes/enhance-auto-update-system/design.md
🪛 LanguageTool
openspec/changes/enhance-auto-update-system/tasks.md
[grammar] ~43-~43: Use a hyphen to join words.
Context: ... Implement GREEN admin API and dashboard type updates in `clients/agent-runtime/s...
(QB_NEW_EN_HYPHEN)
openspec/changes/enhance-auto-update-system/proposal.md
[uncategorized] ~84-~84: The official name of this software platform is spelled with a capital “H”.
Context: ...se metadata and checksum artifacts from .github/workflows/_publish.yml. - Existing ins...
(GITHUB)
openspec/changes/enhance-auto-update-system/design.md
[style] ~379-~379: The double modal “requires managed” is nonstandard (only accepted in certain dialects). Consider “to be managed”.
Context: ...gRestarthandling when policy requires managed restart | |clients/agent-runtime/src/...
(NEEDS_FIXED)
🪛 markdownlint-cli2 (0.21.0)
openspec/changes/enhance-auto-update-system/exploration.md
[warning] 1-1: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
[warning] 3-3: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 14-14: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 25-25: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 33-33: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 41-41: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 54-54: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
Deploying corvus with
|
| Latest commit: |
1f3e934
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://c3de6a40.corvus-42x.pages.dev |
| Branch Preview URL: | https://fix-update-command.corvus-42x.pages.dev |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
openspec/changes/archive/2026-03-04-agent-runtime-mission-layer/verify-report.md (1)
56-63:⚠️ Potential issue | 🟠 MajorResolve pass/fail ambiguity for threshold breach.
The report says coverage is
7.1%against a configured threshold of60%, but the final verdict isPASS WITH WARNINGS. If threshold is gating, this should be a fail; if non-gating, explicitly state that policy in this report to avoid a contract mismatch.Proposed doc fix
-### Verdict - -PASS WITH WARNINGS - -All mission-layer spec scenarios are now verified as compliant by passing runtime evidence; only -coverage threshold remains below the configured baseline. +### Verdict + +FAIL (coverage threshold not met) + +All mission-layer spec scenarios are compliant by runtime evidence; however, coverage is 7.1% +against the configured 60% threshold, so verification does not meet archive gating criteria.As per coding guidelines, "
**/*.{md,mdx}: Verify technical accuracy and that docs stay aligned with code changes."Also applies to: 139-142
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openspec/changes/archive/2026-03-04-agent-runtime-mission-layer/verify-report.md` around lines 56 - 63, The coverage section shows "Coverage: 7.1% line / threshold: 60% -> ⚠️ Below threshold" but still emits the verdict "PASS WITH WARNINGS"; update the verify-report.md text so that when coverage < rules.verify.coverage_threshold (configured at openspec/config.yaml) the report emits an explicit FAIL verdict (or, if the policy is intentionally non-gating, add a clear statement that the threshold is advisory and will not fail the run). Change the lines around the "Coverage: 7.1%..." block and the later similar block (currently showing "PASS WITH WARNINGS" at lines ~139-142) to either (A) replace "PASS WITH WARNINGS" with "FAIL: coverage below threshold (7.1% < 60%)" or (B) add a sentence that the coverage threshold is advisory and non-gating, referencing the configured key and the output of `make test-coverage`/clients/composeApp/build/reports/kover/html/index.html so consumers know the intended policy.openspec/changes/archive/2026-03-03-agent-loop/tasks.md (1)
5-67:⚠️ Potential issue | 🟠 MajorDocumentation misalignment: task.md marked complete without corresponding runtime code changes in this PR.
The modified
openspec/changes/archive/2026-03-03-agent-loop/tasks.mdmarks allAgentLoopimplementation phases (1–5) and remediation tasks as complete (lines 5–67). However, the PR diff shows no modifications to the referenced runtime files (clients/agent-runtime/src/agent/unified_loop.rs,dispatcher.rs,channels/mod.rs,gateway/mod.rs,main.rs) or their corresponding tests. All these files exist in the codebase but remain unchanged in this PR.Either revert the task.md completion claims to reflect the actual PR scope (spec/design doc updates only), or include the runtime implementation and test changes required to justify the checklist. Documentation must stay auditable and aligned with actual code changes.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openspec/changes/archive/2026-03-03-agent-loop/tasks.md` around lines 5 - 67, The tasks.md checklist incorrectly marks AgentLoop work complete without corresponding runtime changes; either revert the completed checkboxes in openspec/changes/archive/2026-03-03-agent-loop/tasks.md to reflect this PR is spec-only, or add the missing runtime commits that implement and test the referenced symbols (AgentLoop/LoopConfig/LoopEvent in clients/agent-runtime/src/agent/unified_loop.rs, Dispatcher risk classification/ApprovalRequired in clients/agent-runtime/src/agent/dispatcher.rs, event-to-channel mapping in clients/agent-runtime/src/channels/mod.rs, SSE mapping in clients/agent-runtime/src/gateway/mod.rs, and CLI wiring in clients/agent-runtime/src/main.rs) and their unit/integration tests so the checklist items accurately match the code changes.openspec/changes/archive/2026-03-04-web-agent-config/design.md (1)
96-152:⚠️ Potential issue | 🟠 MajorAPI contract examples are out of sync with backend implementation.
This section documents
api_keys?: Record<string, SecretUpdate>and a RustSecretUpdatemap onAdminConfigUpdateRequest, but current gateway contract evidence showsAdminConfigUpdateRequestdoes not includeapi_keysand usesAdminSecretUpdateon specific nested fields. Please update this section to match actual request/serde shapes to avoid client breakage.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openspec/changes/archive/2026-03-04-web-agent-config/design.md` around lines 96 - 152, The docs' TypeScript/Rust API examples are out of sync with the gateway serde shapes: remove or update the documented api_keys: Record<string, SecretUpdate> and align the secret types to the actual backend shape (which uses AdminSecretUpdate on specific nested fields rather than a flat api_keys map). Edit the Frontend Types and Backend Types sections so the TypeScript interfaces and Rust structs match the real request/serde shapes—update SecretMode/SecretUpdate to match the backend enum/struct names or replace them with AdminSecretUpdate where appropriate, and ensure AdminConfigUpdateRequest lists the nested fields that actually carry secret updates (the same nested fields referenced in the backend, e.g., runtime/gateway/observability if they contain AdminSecretUpdate) so clients will serialize to the same shape the server deserializes.openspec/changes/archive/2026-03-03-support-mcps-agent-runtime/verify-report.md (1)
145-203:⚠️ Potential issue | 🟡 MinorMark pre-addendum verdict as explicitly superseded.
This report currently contains both FAIL and PASS outcomes. Please add an explicit “superseded by addendum (2026-03-03)” note near Line 147 to avoid ambiguous interpretation by reviewers and tooling.
As per coding guidelines
**/*.{md,mdx}: Verify technical accuracy and that docs stay aligned with code changes.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openspec/changes/archive/2026-03-03-support-mcps-agent-runtime/verify-report.md` around lines 145 - 203, The Verdict section currently shows both FAIL and PASS; update the document so the original FAIL is explicitly marked as superseded by the addendum: insert a clear note like “superseded by addendum (2026-03-03)” immediately adjacent to the Verdict header or right after the initial "FAIL" line (before the "Implementation..." paragraph), making it clear that the Verification Addendum (2026-03-03) supersedes the prior verdict.openspec/changes/archive/2026-03-03-agent-loop/specs/agent-loop/spec.md (1)
57-61:⚠️ Potential issue | 🟠 MajorTimeout scenario is stricter than current runtime behavior.
Line 60 requires a timeout error event for both iteration and timeout overruns, but runtime emits distinct errors (
"iteration budget exceeded"vs"timeout exceeded"). Line 61 requires safe session resource release, butAgentLoopcurrently has no explicit session-resource cleanup path before returning on timeout (clients/agent-runtime/src/agent/unified_loop.rs). Please either narrow this scenario wording or add matching runtime cleanup/evidence.As per coding guidelines
**/*.{md,mdx}: Verify technical accuracy and that docs stay aligned with code changes.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openspec/changes/archive/2026-03-03-agent-loop/specs/agent-loop/spec.md` around lines 57 - 61, The spec's timeout scenario is stricter than runtime: it mandates a single timeout error event and session cleanup, but the runtime distinguishes "iteration budget exceeded" vs "timeout exceeded" and AgentLoop (clients/agent-runtime/src/agent/unified_loop.rs) lacks explicit session-resource cleanup on abort. Update either the spec or the runtime: either narrow the spec text to state that the system MUST emit a timeout or iteration-budget error (preserving distinct messages) and SHOULD attempt best-effort session cleanup, or modify AgentLoop/unified_loop.rs to unify the error path (emit a single standardized timeout error string) and add an explicit session resource release/cleanup routine invoked on both iteration and time abort paths (ensure cleanup helper name like release_session_resources or AgentLoop::cleanup_session is called before returning and that emitted errors include "iteration budget exceeded" or "timeout exceeded" consistently).openspec/changes/archive/2026-03-03-agent-loop/design.md (1)
119-151:⚠️ Potential issue | 🟠 Major
LoopEventcontract in docs diverges from implemented API.The design block documents typed payload variants (
ToolCall,ToolResult, etc.), but currentclients/agent-runtime/src/agent/unified_loop.rsdefines these asStringpayload variants. Please align the interface section with the actual exported type shape (or clearly label this block as future/target contract).As per coding guidelines
**/*.{md,mdx}: Verify technical accuracy and that docs stay aligned with code changes.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openspec/changes/archive/2026-03-03-agent-loop/design.md` around lines 119 - 151, The documented LoopEvent variants in the design.md (LoopEvent: ToolDispatchStarted, ToolDispatchCompleted, ApprovalRequired, etc.) do not match the implemented API which uses String payloads; update the docs to reflect the actual exported shapes (e.g., show ToolDispatchStarted(String), ToolDispatchCompleted(String), ApprovalRequired(String), LLMProgress(String), Complete(String), Error(String) or whatever concrete string-typed payload the runtime uses) or explicitly mark this section as a future/target contract and add a TODO noting the current implementation differs; ensure you reference the same types used by the runtime (LoopEvent, AgentLoop, run, resume, ToolCall, ToolResult, ApprovalRequest, FinalResponse, LoopError) so readers can map doc variants to the real API.
♻️ Duplicate comments (1)
openspec/changes/enhance-auto-update-system/design.md (1)
460-473:⚠️ Potential issue | 🟠 MajorPhase plan still drifts from
tasks.mdexecution contract.This design puts multi-surface visibility in Phase 1, while
openspec/changes/enhance-auto-update-system/tasks.mdschedules that work in Phase 3 after CLI/locking foundations. Please align phase content and numbering to avoid implementation and acceptance drift. As per coding guidelines: "**/*.{md,mdx}: Verify technical accuracy and that docs stay aligned with code changes."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openspec/changes/enhance-auto-update-system/design.md` around lines 460 - 473, The Phase headings and numbered items in "Phase 1: Policy model + multi-surface visibility" and "Phase 2: Safety + command foundation" drift from the execution order in openspec/changes/enhance-auto-update-system/tasks.md; update design.md so phase names, numbering, and included work match the tasks.md plan (e.g., move multi-surface visibility to the same phase where CLI/locking foundations are scheduled), ensuring the same sequence for items like "unify canonical payload fan-out", "update status|check|install command surface", and "lock manager + atomic state writes"; reference and align the exact section titles used here when editing so reviewers can verify consistency between design.md and tasks.md.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@openspec/changes/archive/2026-03-03-agent-loop/proposal.md`:
- Around line 48-60: The ordered list in the phased rollout section uses
incremental numbering; update each list item so every ordered-list prefix is
"1." to satisfy markdownlint MD029 — locate the lines starting with "Baseline
Contract Phase", "Convergence Phase", "Alignment Phase", and "Hardening Phase"
in the diff and change their numeric prefixes to "1." while preserving the
existing text and indentation.
In `@openspec/changes/archive/2026-03-03-agent-loop/verify-report.md`:
- Around line 59-64: The Timeout Aborts compliance mark is missing evidence for
session resource cleanup; update the verification by either (A) downgrading the
"Timeout Aborts" row to partial compliance or (B) adding concrete
test/implementation evidence that session resources are safely released when
timeouts occur: add tests that assert cleanup of session resources and handles
(e.g., file descriptors, in-memory state, timers) in the same modules
referenced—clients/agent-runtime/src/agent/unified_loop.rs::test_agent_loop_emits_timeout_error,
clients/agent-runtime/tests/cli_loop_events_e2e.rs::cli_non_preview_timeout_abort_is_session_scoped,
and
clients/agent-runtime/src/gateway/mod.rs::tests::webhook_non_preview_timeout_aborts_with_session_scope—or
point to existing cleanup functions invoked on timeout in
unified_loop/unified_entrypoint so the Compliance summary is accurate.
In `@openspec/changes/archive/2026-03-03-support-mcps-agent-runtime/design.md`:
- Around line 296-303: The ordered list under the Rollout Plan uses incremental
numeric prefixes; change each list item so they all start with "1." to satisfy
MD029 (use 1./1./1. style). Specifically update the lines that begin with "1.
Phase 1: Config + discovery scaffolding", "2. Phase 2: Identity + dispatch +
policy", "3. Phase 3: Hardening", and "4. Verification gates" to each start with
"1." while preserving the text for each phase and bullets.
In `@openspec/changes/archive/2026-03-03-support-mcps-agent-runtime/tasks.md`:
- Around line 32-33: Replace the malformed markdown emphasis token "*
*Acceptance:**" with the correct "**Acceptance:**" in the document: search for
the literal substring "* *Acceptance:**" (and similar malformed variants) in
openspec/changes/archive/2026-03-03-support-mcps-agent-runtime/tasks.md and
update each occurrence (notably around the previously flagged ranges) so the
bullet headers render consistently as "**Acceptance:**".
In
`@openspec/changes/archive/2026-03-04-agent-runtime-mission-layer/verify-report.md`:
- Around line 49-53: The markdown has a broken list item split into a stray "-"
line and wrapped lines; fix verify-report.md by merging the fragmented lines
into a single well-formed bullet that contains the command and its result (e.g.,
one bullet like "`cargo test -p corvus --test legacy_loop_guard --test
mission_lifecycle_integration --test mission_governance_integration --test
mission_security_parity --test mission_config_toggle --test
mission_entrypoint_parity` -> ✅ passed (25 passed / 0 failed)" and similarly
ensure the `cargo test -p corvus
concurrent_transition_attempts_are_serialized_with_single_winner` evidence is a
single bullet showing "✅ passed"); ensure there are no standalone "-" lines and
wrapping is preserved within each list item.
In `@openspec/changes/archive/2026-03-04-web-agent-config/design.md`:
- Around line 17-23: The documentation uses two different composable
names—useConfigStore.ts (in design.md) and useConfig.ts (in related docs/tasks);
pick one canonical name (e.g., useConfig.ts) and make the references consistent:
rename the file if necessary and update all mentions (design.md, task
descriptions, example imports/usages) to that chosen symbol so implementation,
docs, and task references match; ensure any import paths, examples, and CI/docs
links are updated to the canonical composable name (useConfig.ts or
useConfigStore.ts) across the repo.
In
`@openspec/changes/archive/2026-03-04-web-agent-config/specs/agent-config/spec.md`:
- Around line 15-19: Update the scenario contract that currently states "POST"
to use the correct HTTP method "PUT /web/admin/config" so it matches the
verified admin config update flow; change the step that reads WHEN a POST
request is sent to instead specify a PUT to /web/admin/config carrying the full
JSON payload for config.toml, ensuring the rest of the scenario (validation,
persisting to config.toml, and returning 200 with AdminConfigView) remains
unchanged.
- Around line 7-10: The spec currently requires AdminConfigUpdateRequest to
support "all fields defined in the system's `config.toml`" which overstates
scope; update the text to require support for "all admin-editable fields"
instead and adjust the following sentence to state that the backend MUST
securely accept updates to admin-editable server settings, agent identity
details, and LLM provider configurations; search for the phrase
"AdminConfigUpdateRequest" and the exact string "all fields defined in the
system's `config.toml`" and replace it with "all admin-editable fields" and
similarly update the subsequent sentence to reference "admin-editable" fields to
keep the spec aligned with hidden/non-editable sections.
In `@openspec/changes/archive/2026-03-04-web-agent-config/verify-report.md`:
- Around line 1-8: The document's headings skip levels (MD001); update the
heading hierarchy in verify-report.md so it starts with a single top-level
heading (e.g., change "## Verification Report" to "# Verification Report") and
ensure subsequent headings only increase by one level at a time (for example use
"## Completeness" rather than jumping from # to ###). Verify all other headings
in the file follow sequential levels and adjust them accordingly.
In `@openspec/changes/enhance-auto-update-system/design.md`:
- Around line 378-397: Replace the free-form String fields with explicit
enum-backed types and document their serialized representations and casing/serde
strategy: change install_method and restart_policy related fields in
UpdateStatusView (effective_install_method, detected_install_method,
install_method_source) and UpdatePolicyView (restart_policy) to specific enums
(e.g., InstallMethod, InstallMethodSource, RestartPolicy) and add serde
attributes (e.g., #[serde(rename_all = "snake_case")]/#[serde(rename = "...")]
to guarantee canonical string values; update the design.md snippet to list every
enum variant and its exact serialized string and casing, and note that
clients/web/apps/dashboard/src/types/admin-config.ts must mirror these exact
strings/types when parsing.
---
Outside diff comments:
In `@openspec/changes/archive/2026-03-03-agent-loop/design.md`:
- Around line 119-151: The documented LoopEvent variants in the design.md
(LoopEvent: ToolDispatchStarted, ToolDispatchCompleted, ApprovalRequired, etc.)
do not match the implemented API which uses String payloads; update the docs to
reflect the actual exported shapes (e.g., show ToolDispatchStarted(String),
ToolDispatchCompleted(String), ApprovalRequired(String), LLMProgress(String),
Complete(String), Error(String) or whatever concrete string-typed payload the
runtime uses) or explicitly mark this section as a future/target contract and
add a TODO noting the current implementation differs; ensure you reference the
same types used by the runtime (LoopEvent, AgentLoop, run, resume, ToolCall,
ToolResult, ApprovalRequest, FinalResponse, LoopError) so readers can map doc
variants to the real API.
In `@openspec/changes/archive/2026-03-03-agent-loop/specs/agent-loop/spec.md`:
- Around line 57-61: The spec's timeout scenario is stricter than runtime: it
mandates a single timeout error event and session cleanup, but the runtime
distinguishes "iteration budget exceeded" vs "timeout exceeded" and AgentLoop
(clients/agent-runtime/src/agent/unified_loop.rs) lacks explicit
session-resource cleanup on abort. Update either the spec or the runtime: either
narrow the spec text to state that the system MUST emit a timeout or
iteration-budget error (preserving distinct messages) and SHOULD attempt
best-effort session cleanup, or modify AgentLoop/unified_loop.rs to unify the
error path (emit a single standardized timeout error string) and add an explicit
session resource release/cleanup routine invoked on both iteration and time
abort paths (ensure cleanup helper name like release_session_resources or
AgentLoop::cleanup_session is called before returning and that emitted errors
include "iteration budget exceeded" or "timeout exceeded" consistently).
In `@openspec/changes/archive/2026-03-03-agent-loop/tasks.md`:
- Around line 5-67: The tasks.md checklist incorrectly marks AgentLoop work
complete without corresponding runtime changes; either revert the completed
checkboxes in openspec/changes/archive/2026-03-03-agent-loop/tasks.md to reflect
this PR is spec-only, or add the missing runtime commits that implement and test
the referenced symbols (AgentLoop/LoopConfig/LoopEvent in
clients/agent-runtime/src/agent/unified_loop.rs, Dispatcher risk
classification/ApprovalRequired in
clients/agent-runtime/src/agent/dispatcher.rs, event-to-channel mapping in
clients/agent-runtime/src/channels/mod.rs, SSE mapping in
clients/agent-runtime/src/gateway/mod.rs, and CLI wiring in
clients/agent-runtime/src/main.rs) and their unit/integration tests so the
checklist items accurately match the code changes.
In
`@openspec/changes/archive/2026-03-03-support-mcps-agent-runtime/verify-report.md`:
- Around line 145-203: The Verdict section currently shows both FAIL and PASS;
update the document so the original FAIL is explicitly marked as superseded by
the addendum: insert a clear note like “superseded by addendum (2026-03-03)”
immediately adjacent to the Verdict header or right after the initial "FAIL"
line (before the "Implementation..." paragraph), making it clear that the
Verification Addendum (2026-03-03) supersedes the prior verdict.
In
`@openspec/changes/archive/2026-03-04-agent-runtime-mission-layer/verify-report.md`:
- Around line 56-63: The coverage section shows "Coverage: 7.1% line /
threshold: 60% -> ⚠️ Below threshold" but still emits the verdict "PASS WITH
WARNINGS"; update the verify-report.md text so that when coverage <
rules.verify.coverage_threshold (configured at openspec/config.yaml) the report
emits an explicit FAIL verdict (or, if the policy is intentionally non-gating,
add a clear statement that the threshold is advisory and will not fail the run).
Change the lines around the "Coverage: 7.1%..." block and the later similar
block (currently showing "PASS WITH WARNINGS" at lines ~139-142) to either (A)
replace "PASS WITH WARNINGS" with "FAIL: coverage below threshold (7.1% < 60%)"
or (B) add a sentence that the coverage threshold is advisory and non-gating,
referencing the configured key and the output of `make
test-coverage`/clients/composeApp/build/reports/kover/html/index.html so
consumers know the intended policy.
In `@openspec/changes/archive/2026-03-04-web-agent-config/design.md`:
- Around line 96-152: The docs' TypeScript/Rust API examples are out of sync
with the gateway serde shapes: remove or update the documented api_keys:
Record<string, SecretUpdate> and align the secret types to the actual backend
shape (which uses AdminSecretUpdate on specific nested fields rather than a flat
api_keys map). Edit the Frontend Types and Backend Types sections so the
TypeScript interfaces and Rust structs match the real request/serde
shapes—update SecretMode/SecretUpdate to match the backend enum/struct names or
replace them with AdminSecretUpdate where appropriate, and ensure
AdminConfigUpdateRequest lists the nested fields that actually carry secret
updates (the same nested fields referenced in the backend, e.g.,
runtime/gateway/observability if they contain AdminSecretUpdate) so clients will
serialize to the same shape the server deserializes.
---
Duplicate comments:
In `@openspec/changes/enhance-auto-update-system/design.md`:
- Around line 460-473: The Phase headings and numbered items in "Phase 1: Policy
model + multi-surface visibility" and "Phase 2: Safety + command foundation"
drift from the execution order in
openspec/changes/enhance-auto-update-system/tasks.md; update design.md so phase
names, numbering, and included work match the tasks.md plan (e.g., move
multi-surface visibility to the same phase where CLI/locking foundations are
scheduled), ensuring the same sequence for items like "unify canonical payload
fan-out", "update status|check|install command surface", and "lock manager +
atomic state writes"; reference and align the exact section titles used here
when editing so reviewers can verify consistency between design.md and tasks.md.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 72048ad2-78e8-4148-845a-da4e51e8b8b0
📒 Files selected for processing (24)
openspec/changes/archive/2026-03-03-agent-loop/archive-report.mdopenspec/changes/archive/2026-03-03-agent-loop/design.mdopenspec/changes/archive/2026-03-03-agent-loop/exploration.mdopenspec/changes/archive/2026-03-03-agent-loop/proposal.mdopenspec/changes/archive/2026-03-03-agent-loop/specs/agent-loop/spec.mdopenspec/changes/archive/2026-03-03-agent-loop/tasks.mdopenspec/changes/archive/2026-03-03-agent-loop/verify-report.mdopenspec/changes/archive/2026-03-03-support-mcps-agent-runtime/design.mdopenspec/changes/archive/2026-03-03-support-mcps-agent-runtime/proposal.mdopenspec/changes/archive/2026-03-03-support-mcps-agent-runtime/tasks.mdopenspec/changes/archive/2026-03-03-support-mcps-agent-runtime/verify-report.mdopenspec/changes/archive/2026-03-04-agent-runtime-mission-layer/verify-report.mdopenspec/changes/archive/2026-03-04-web-agent-config/archive-report.mdopenspec/changes/archive/2026-03-04-web-agent-config/design.mdopenspec/changes/archive/2026-03-04-web-agent-config/proposal.mdopenspec/changes/archive/2026-03-04-web-agent-config/specs/agent-config/spec.mdopenspec/changes/archive/2026-03-04-web-agent-config/specs/dashboard-ui/spec.mdopenspec/changes/archive/2026-03-04-web-agent-config/tasks.mdopenspec/changes/archive/2026-03-04-web-agent-config/verify-report.mdopenspec/changes/enhance-auto-update-system/design.mdopenspec/changes/enhance-auto-update-system/exploration.mdopenspec/changes/enhance-auto-update-system/proposal.mdopenspec/changes/enhance-auto-update-system/specs/update-system/spec.mdopenspec/changes/enhance-auto-update-system/tasks.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). (4)
- GitHub Check: sonar
- GitHub Check: pr-checks
- GitHub Check: pr-checks
- GitHub Check: Cloudflare Pages
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{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/changes/archive/2026-03-03-agent-loop/tasks.mdopenspec/changes/archive/2026-03-03-agent-loop/verify-report.mdopenspec/changes/archive/2026-03-04-web-agent-config/verify-report.mdopenspec/changes/archive/2026-03-04-web-agent-config/specs/dashboard-ui/spec.mdopenspec/changes/enhance-auto-update-system/exploration.mdopenspec/changes/archive/2026-03-03-support-mcps-agent-runtime/design.mdopenspec/changes/archive/2026-03-04-web-agent-config/specs/agent-config/spec.mdopenspec/changes/archive/2026-03-03-support-mcps-agent-runtime/verify-report.mdopenspec/changes/archive/2026-03-04-agent-runtime-mission-layer/verify-report.mdopenspec/changes/archive/2026-03-03-agent-loop/exploration.mdopenspec/changes/archive/2026-03-04-web-agent-config/tasks.mdopenspec/changes/enhance-auto-update-system/specs/update-system/spec.mdopenspec/changes/enhance-auto-update-system/tasks.mdopenspec/changes/archive/2026-03-03-support-mcps-agent-runtime/proposal.mdopenspec/changes/archive/2026-03-04-web-agent-config/archive-report.mdopenspec/changes/enhance-auto-update-system/design.mdopenspec/changes/archive/2026-03-03-support-mcps-agent-runtime/tasks.mdopenspec/changes/archive/2026-03-03-agent-loop/proposal.mdopenspec/changes/archive/2026-03-04-web-agent-config/proposal.mdopenspec/changes/archive/2026-03-03-agent-loop/specs/agent-loop/spec.mdopenspec/changes/enhance-auto-update-system/proposal.mdopenspec/changes/archive/2026-03-04-web-agent-config/design.mdopenspec/changes/archive/2026-03-03-agent-loop/archive-report.mdopenspec/changes/archive/2026-03-03-agent-loop/design.md
**/*
⚙️ 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:
openspec/changes/archive/2026-03-03-agent-loop/tasks.mdopenspec/changes/archive/2026-03-03-agent-loop/verify-report.mdopenspec/changes/archive/2026-03-04-web-agent-config/verify-report.mdopenspec/changes/archive/2026-03-04-web-agent-config/specs/dashboard-ui/spec.mdopenspec/changes/enhance-auto-update-system/exploration.mdopenspec/changes/archive/2026-03-03-support-mcps-agent-runtime/design.mdopenspec/changes/archive/2026-03-04-web-agent-config/specs/agent-config/spec.mdopenspec/changes/archive/2026-03-03-support-mcps-agent-runtime/verify-report.mdopenspec/changes/archive/2026-03-04-agent-runtime-mission-layer/verify-report.mdopenspec/changes/archive/2026-03-03-agent-loop/exploration.mdopenspec/changes/archive/2026-03-04-web-agent-config/tasks.mdopenspec/changes/enhance-auto-update-system/specs/update-system/spec.mdopenspec/changes/enhance-auto-update-system/tasks.mdopenspec/changes/archive/2026-03-03-support-mcps-agent-runtime/proposal.mdopenspec/changes/archive/2026-03-04-web-agent-config/archive-report.mdopenspec/changes/enhance-auto-update-system/design.mdopenspec/changes/archive/2026-03-03-support-mcps-agent-runtime/tasks.mdopenspec/changes/archive/2026-03-03-agent-loop/proposal.mdopenspec/changes/archive/2026-03-04-web-agent-config/proposal.mdopenspec/changes/archive/2026-03-03-agent-loop/specs/agent-loop/spec.mdopenspec/changes/enhance-auto-update-system/proposal.mdopenspec/changes/archive/2026-03-04-web-agent-config/design.mdopenspec/changes/archive/2026-03-03-agent-loop/archive-report.mdopenspec/changes/archive/2026-03-03-agent-loop/design.md
🧠 Learnings (15)
📚 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: Keep changes local and avoid cross-module refactors in unrelated tasks to maintain code stability
Applied to files:
openspec/changes/archive/2026-03-03-agent-loop/tasks.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/**/*.rs : Run `cargo fmt --all -- --check`, `cargo clippy --all-targets -- -D warnings`, and `cargo test` for code validation, or document which checks were skipped and why
Applied to files:
openspec/changes/archive/2026-03-03-agent-loop/tasks.mdopenspec/changes/archive/2026-03-03-agent-loop/verify-report.mdopenspec/changes/archive/2026-03-04-web-agent-config/verify-report.mdopenspec/changes/archive/2026-03-03-support-mcps-agent-runtime/verify-report.mdopenspec/changes/archive/2026-03-04-agent-runtime-mission-layer/verify-report.mdopenspec/changes/archive/2026-03-04-web-agent-config/tasks.mdopenspec/changes/archive/2026-03-03-support-mcps-agent-runtime/proposal.mdopenspec/changes/archive/2026-03-03-agent-loop/design.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:
openspec/changes/archive/2026-03-03-agent-loop/tasks.mdopenspec/changes/archive/2026-03-03-agent-loop/verify-report.mdopenspec/changes/archive/2026-03-03-agent-loop/design.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/tools/**/*.rs : Implement `Tool` trait in `src/tools/` with strict parameter schema, validate and sanitize all inputs, and return structured `ToolResult` without panics in runtime path
Applied to files:
openspec/changes/archive/2026-03-03-agent-loop/tasks.mdopenspec/changes/archive/2026-03-03-support-mcps-agent-runtime/verify-report.mdopenspec/changes/archive/2026-03-03-support-mcps-agent-runtime/proposal.mdopenspec/changes/archive/2026-03-03-agent-loop/design.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/main.rs : Preserve CLI contract unless change is intentional and documented; prefer explicit errors over silent fallback for unsupported critical paths
Applied to files:
openspec/changes/archive/2026-03-03-agent-loop/tasks.mdopenspec/changes/archive/2026-03-03-agent-loop/verify-report.mdopenspec/changes/archive/2026-03-03-support-mcps-agent-runtime/design.mdopenspec/changes/archive/2026-03-03-support-mcps-agent-runtime/verify-report.mdopenspec/changes/archive/2026-03-03-agent-loop/exploration.mdopenspec/changes/enhance-auto-update-system/specs/update-system/spec.mdopenspec/changes/archive/2026-03-03-support-mcps-agent-runtime/proposal.mdopenspec/changes/enhance-auto-update-system/design.mdopenspec/changes/archive/2026-03-03-agent-loop/proposal.mdopenspec/changes/archive/2026-03-03-agent-loop/design.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: 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:
openspec/changes/archive/2026-03-03-agent-loop/tasks.mdopenspec/changes/archive/2026-03-03-agent-loop/verify-report.mdopenspec/changes/archive/2026-03-04-web-agent-config/verify-report.mdopenspec/changes/archive/2026-03-03-support-mcps-agent-runtime/design.mdopenspec/changes/archive/2026-03-04-web-agent-config/tasks.mdopenspec/changes/enhance-auto-update-system/specs/update-system/spec.mdopenspec/changes/archive/2026-03-03-support-mcps-agent-runtime/proposal.mdopenspec/changes/archive/2026-03-04-web-agent-config/archive-report.mdopenspec/changes/enhance-auto-update-system/design.mdopenspec/changes/archive/2026-03-03-support-mcps-agent-runtime/tasks.mdopenspec/changes/archive/2026-03-03-agent-loop/proposal.mdopenspec/changes/archive/2026-03-04-web-agent-config/proposal.mdopenspec/changes/archive/2026-03-04-web-agent-config/design.mdopenspec/changes/archive/2026-03-03-agent-loop/design.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/{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:
openspec/changes/archive/2026-03-03-agent-loop/tasks.mdopenspec/changes/archive/2026-03-03-support-mcps-agent-runtime/design.mdopenspec/changes/archive/2026-03-03-agent-loop/exploration.mdopenspec/changes/archive/2026-03-03-support-mcps-agent-runtime/proposal.mdopenspec/changes/enhance-auto-update-system/design.mdopenspec/changes/archive/2026-03-03-agent-loop/proposal.mdopenspec/changes/archive/2026-03-04-web-agent-config/proposal.mdopenspec/changes/archive/2026-03-04-web-agent-config/design.mdopenspec/changes/archive/2026-03-03-agent-loop/design.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/{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:
openspec/changes/archive/2026-03-03-agent-loop/tasks.mdopenspec/changes/archive/2026-03-03-agent-loop/verify-report.mdopenspec/changes/archive/2026-03-03-support-mcps-agent-runtime/design.mdopenspec/changes/archive/2026-03-03-support-mcps-agent-runtime/verify-report.mdopenspec/changes/archive/2026-03-03-agent-loop/exploration.mdopenspec/changes/archive/2026-03-03-support-mcps-agent-runtime/proposal.mdopenspec/changes/archive/2026-03-04-web-agent-config/archive-report.mdopenspec/changes/enhance-auto-update-system/design.mdopenspec/changes/archive/2026-03-03-agent-loop/proposal.mdopenspec/changes/archive/2026-03-04-web-agent-config/proposal.mdopenspec/changes/archive/2026-03-04-web-agent-config/design.mdopenspec/changes/archive/2026-03-03-agent-loop/design.md
📚 Learning: 2026-02-17T07:28:38.934Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-17T07:28:38.934Z
Learning: Applies to .agents/AGENTS.md : Document agent configurations and capabilities in AGENTS.md
Applied to files:
openspec/changes/archive/2026-03-04-web-agent-config/verify-report.mdopenspec/changes/archive/2026-03-03-support-mcps-agent-runtime/design.mdopenspec/changes/archive/2026-03-04-web-agent-config/specs/agent-config/spec.mdopenspec/changes/archive/2026-03-04-web-agent-config/proposal.md
📚 Learning: 2026-02-17T07:28:38.934Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-17T07:28:38.934Z
Learning: Applies to .agents/AGENTS.md : Include version information and compatibility details for agents
Applied to files:
openspec/changes/archive/2026-03-04-web-agent-config/verify-report.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/**/*.rs : Never log secrets, tokens, raw credentials, or sensitive payloads in any logging statements
Applied to files:
openspec/changes/archive/2026-03-03-support-mcps-agent-runtime/proposal.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/**/Cargo.toml : Do not add heavy dependencies for minor convenience; justify new crate additions
Applied to files:
openspec/changes/archive/2026-03-03-support-mcps-agent-runtime/proposal.mdopenspec/changes/archive/2026-03-03-agent-loop/design.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/main.rs : Keep startup path lean and avoid heavy initialization in command parsing flow
Applied to files:
openspec/changes/archive/2026-03-03-support-mcps-agent-runtime/proposal.mdopenspec/changes/archive/2026-03-03-agent-loop/design.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/**/*.rs : Avoid unnecessary allocations, clones, and blocking operations to maintain performance and efficiency
Applied to files:
openspec/changes/archive/2026-03-03-support-mcps-agent-runtime/proposal.mdopenspec/changes/archive/2026-03-03-agent-loop/design.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/providers/**/*.rs : Implement `Provider` trait in `src/providers/` and register in `src/providers/mod.rs` factory when adding a new provider
Applied to files:
openspec/changes/archive/2026-03-03-agent-loop/design.md
🪛 LanguageTool
openspec/changes/archive/2026-03-03-agent-loop/verify-report.md
[style] ~70-~70: ‘prior to’ might be wordy. Consider a shorter alternative.
Context: ...ctive across CLI, channels, and gateway prior to runtime completion paths. - Session-s...
(EN_WORDINESS_PREMIUM_PRIOR_TO)
openspec/changes/enhance-auto-update-system/tasks.md
[grammar] ~75-~75: Use a hyphen to join words.
Context: ... Implement GREEN admin API and dashboard type updates in `clients/agent-runtime...
(QB_NEW_EN_HYPHEN)
openspec/changes/enhance-auto-update-system/design.md
[style] ~414-~414: The double modal “requires managed” is nonstandard (only accepted in certain dialects). Consider “to be managed”.
Context: ...gRestarthandling when policy requires managed restart | |clients/agent-runtime/...
(NEEDS_FIXED)
openspec/changes/enhance-auto-update-system/proposal.md
[uncategorized] ~112-~112: The official name of this software platform is spelled with a capital “H”.
Context: ...se metadata and checksum artifacts from .github/workflows/_publish.yml. - Existing ins...
(GITHUB)
🪛 markdownlint-cli2 (0.21.0)
openspec/changes/archive/2026-03-04-web-agent-config/verify-report.md
[warning] 3-3: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
openspec/changes/archive/2026-03-03-support-mcps-agent-runtime/design.md
[warning] 298-298: Ordered list item prefix
Expected: 1; Actual: 2; Style: 1/1/1
(MD029, ol-prefix)
[warning] 300-300: Ordered list item prefix
Expected: 1; Actual: 3; Style: 1/1/1
(MD029, ol-prefix)
[warning] 302-302: Ordered list item prefix
Expected: 1; Actual: 4; Style: 1/1/1
(MD029, ol-prefix)
openspec/changes/archive/2026-03-03-support-mcps-agent-runtime/proposal.md
[warning] 84-84: Ordered list item prefix
Expected: 1; Actual: 2; Style: 1/1/1
(MD029, ol-prefix)
openspec/changes/archive/2026-03-03-agent-loop/proposal.md
[warning] 84-84: Ordered list item prefix
Expected: 1; Actual: 2; Style: 1/1/1
(MD029, ol-prefix)
[warning] 91-91: Ordered list item prefix
Expected: 1; Actual: 3; Style: 1/1/1
(MD029, ol-prefix)
🔇 Additional comments (16)
openspec/changes/archive/2026-03-03-agent-loop/proposal.md (1)
99-142: Strong security/rollback and verification gates—keep this structure.Clear fail-safe rollback boundaries plus explicit security invariants and measurable conformance/performance gates reduce migration risk and regression ambiguity.
Based on learnings: "Include threat/risk notes and rollback strategy for security, runtime, and gateway changes; add or update tests for boundary checks and failure modes."
openspec/changes/archive/2026-03-03-agent-loop/archive-report.md (5)
13-14: LGTM!The table header spacing adjustment improves readability while maintaining standard markdown format.
28-28: LGTM!The blank line before the list improves document structure and readability.
40-41: LGTM!The sentence line break is a minor formatting adjustment with no impact on content or accuracy.
32-32: No issues found. Verification confirmstasks.mdexists in the archive directory and is correctly listed in the preserved items.
6-7: No parsing tools in the codebase reference the archive-report format, so the multi-line split of the verification prerequisite field poses no actual risk to automated tooling.openspec/changes/archive/2026-03-04-web-agent-config/tasks.md (1)
5-94: No issues in reflowed task content.Formatting-only changes preserve intent and remain aligned with the described implementation/testing flow.
openspec/changes/archive/2026-03-04-web-agent-config/specs/dashboard-ui/spec.md (1)
7-46: Spec reflow is consistent and accurate.No contract or behavior ambiguity introduced in these edits.
openspec/changes/archive/2026-03-04-web-agent-config/archive-report.md (1)
12-59: Archive report edits look good.Reflow preserves factual content and warnings without changing meaning.
openspec/changes/archive/2026-03-03-support-mcps-agent-runtime/proposal.md (1)
107-124:⚠️ Potential issue | 🟡 MinorNormalize ordered-list prefix style to satisfy markdownlint.
This file still triggers
MD029 (ol-prefix). Please use a consistent ordered-list prefix style expected by the repo (typically1.for each item when that rule is configured).⛔ Skipped due to learnings
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 modesopenspec/changes/archive/2026-03-04-web-agent-config/proposal.md (1)
5-72: No actionable issues in changed segments.openspec/changes/enhance-auto-update-system/specs/update-system/spec.md (1)
1-186: Spec content looks consistent and complete for the changed scope.openspec/changes/enhance-auto-update-system/proposal.md (1)
1-130: No blocking issues found in this proposal draft.openspec/changes/enhance-auto-update-system/exploration.md (1)
1-109: Exploration doc is in good shape for this PR scope.openspec/changes/enhance-auto-update-system/tasks.md (1)
1-140: No blocking findings in the task breakdown.openspec/changes/archive/2026-03-03-agent-loop/exploration.md (1)
105-119: Risk and scope framing is clear and actionable.The risks and proposal-ready boundaries are well-scoped and preserve focus on security and behavioral consistency.
Comment on lines
48
to
+60
| 1. Baseline Contract Phase | ||
| - Capture as-is loop behavior as normative fundamentals and acceptance criteria. | ||
| - Identify must-preserve behavior for CLI and channels. | ||
| - Capture as-is loop behavior as normative fundamentals and acceptance criteria. | ||
| - Identify must-preserve behavior for CLI and channels. | ||
| 2. Convergence Phase | ||
| - Move protocol and orchestration responsibilities toward `agent.rs` + `dispatcher.rs` boundaries. | ||
| - Add compatibility adapters so existing entrypoints continue functioning. | ||
| - Move protocol and orchestration responsibilities toward `agent.rs` + `dispatcher.rs` boundaries. | ||
| - Add compatibility adapters so existing entrypoints continue functioning. | ||
| 3. Alignment Phase | ||
| - Align gateway path semantics with canonical fundamentals (or explicitly codify narrow exceptions). | ||
| - Enforce consistent session scoping and approval/risk checks across execution surfaces. | ||
| - Align gateway path semantics with canonical fundamentals (or explicitly codify narrow | ||
| exceptions). | ||
| - Enforce consistent session scoping and approval/risk checks across execution surfaces. | ||
| 4. Hardening Phase | ||
| - Validate reliability/performance budgets and remove duplicated legacy loop paths once parity is proven. | ||
| - Validate reliability/performance budgets and remove duplicated legacy loop paths once parity is | ||
| proven. |
Contributor
There was a problem hiding this comment.
Fix ordered-list prefix style to satisfy markdownlint (MD029).
The phased rollout list should use consistent 1. prefixes per the configured markdownlint style; current numbering triggers warnings (Line 84 and Line 91 in lint output).
Suggested diff
-1. Baseline Contract Phase
+1. Baseline Contract Phase
- Capture as-is loop behavior as normative fundamentals and acceptance criteria.
- Identify must-preserve behavior for CLI and channels.
-2. Convergence Phase
+1. Convergence Phase
- Move protocol and orchestration responsibilities toward `agent.rs` + `dispatcher.rs` boundaries.
- Add compatibility adapters so existing entrypoints continue functioning.
-3. Alignment Phase
+1. Alignment Phase
- Align gateway path semantics with canonical fundamentals (or explicitly codify narrow
exceptions).
- Enforce consistent session scoping and approval/risk checks across execution surfaces.
-4. Hardening Phase
+1. Hardening Phase
- Validate reliability/performance budgets and remove duplicated legacy loop paths once parity is
proven.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| 1. Baseline Contract Phase | |
| - Capture as-is loop behavior as normative fundamentals and acceptance criteria. | |
| - Identify must-preserve behavior for CLI and channels. | |
| - Capture as-is loop behavior as normative fundamentals and acceptance criteria. | |
| - Identify must-preserve behavior for CLI and channels. | |
| 2. Convergence Phase | |
| - Move protocol and orchestration responsibilities toward `agent.rs` + `dispatcher.rs` boundaries. | |
| - Add compatibility adapters so existing entrypoints continue functioning. | |
| - Move protocol and orchestration responsibilities toward `agent.rs` + `dispatcher.rs` boundaries. | |
| - Add compatibility adapters so existing entrypoints continue functioning. | |
| 3. Alignment Phase | |
| - Align gateway path semantics with canonical fundamentals (or explicitly codify narrow exceptions). | |
| - Enforce consistent session scoping and approval/risk checks across execution surfaces. | |
| - Align gateway path semantics with canonical fundamentals (or explicitly codify narrow | |
| exceptions). | |
| - Enforce consistent session scoping and approval/risk checks across execution surfaces. | |
| 4. Hardening Phase | |
| - Validate reliability/performance budgets and remove duplicated legacy loop paths once parity is proven. | |
| - Validate reliability/performance budgets and remove duplicated legacy loop paths once parity is | |
| proven. | |
| 1. Baseline Contract Phase | |
| - Capture as-is loop behavior as normative fundamentals and acceptance criteria. | |
| - Identify must-preserve behavior for CLI and channels. | |
| 1. Convergence Phase | |
| - Move protocol and orchestration responsibilities toward `agent.rs` + `dispatcher.rs` boundaries. | |
| - Add compatibility adapters so existing entrypoints continue functioning. | |
| 1. Alignment Phase | |
| - Align gateway path semantics with canonical fundamentals (or explicitly codify narrow | |
| exceptions). | |
| - Enforce consistent session scoping and approval/risk checks across execution surfaces. | |
| 1. Hardening Phase | |
| - Validate reliability/performance budgets and remove duplicated legacy loop paths once parity is | |
| proven. |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@openspec/changes/archive/2026-03-03-agent-loop/proposal.md` around lines 48 -
60, The ordered list in the phased rollout section uses incremental numbering;
update each list item so every ordered-list prefix is "1." to satisfy
markdownlint MD029 — locate the lines starting with "Baseline Contract Phase",
"Convergence Phase", "Alignment Phase", and "Hardening Phase" in the diff and
change their numeric prefixes to "1." while preserving the existing text and
indentation.
Comment on lines
+59
to
64
| | Timeout Aborts | Runaway Loop Abortion | `clients/agent-runtime/src/agent/unified_loop.rs::test_agent_loop_emits_timeout_error`; `clients/agent-runtime/tests/cli_loop_events_e2e.rs::cli_non_preview_timeout_abort_is_session_scoped`; `clients/agent-runtime/src/gateway/mod.rs::tests::webhook_non_preview_timeout_aborts_with_session_scope` | ✅ COMPLIANT | | ||
| | Error Handling and Fallbacks | Recoverable Tool Failure | `clients/agent-runtime/src/agent/unified_entrypoint.rs::tests::retry_backoff_recovers_timeout_before_fallback`; `...::retry_backoff_uses_fallback_on_persistent_tool_failure` | ✅ COMPLIANT | | ||
| | Error Handling and Fallbacks | Unrecoverable Error | `clients/agent-runtime/src/agent/unified_loop.rs::test_agent_loop_resume_emits_error_when_denied`; non-preview blocking behavior in CLI/Gateway/channel tests | ✅ COMPLIANT | | ||
| | Security Profiling and Invariants | Tool Dispatch with High-Risk Classification | approval-required gating in canonical outcome path for all entry points; tests: CLI non-preview approval override, gateway non-preview approval block/unblock, channel approval block/unblock | ✅ COMPLIANT | | ||
|
|
||
| **Compliance summary**: 7/7 fully compliant |
Contributor
There was a problem hiding this comment.
Timeout compliance is marked complete without evidence for resource-release clause.
Line 59 marks Timeout Aborts as compliant, but listed evidence covers timeout error emission/session scoping only. The current spec also requires safe release of associated session resources, and that isn’t evidenced here. Please downgrade to partial or add concrete test/implementation evidence for cleanup.
As per coding guidelines **/*.{md,mdx}: Verify technical accuracy and that docs stay aligned with code changes.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@openspec/changes/archive/2026-03-03-agent-loop/verify-report.md` around lines
59 - 64, The Timeout Aborts compliance mark is missing evidence for session
resource cleanup; update the verification by either (A) downgrading the "Timeout
Aborts" row to partial compliance or (B) adding concrete test/implementation
evidence that session resources are safely released when timeouts occur: add
tests that assert cleanup of session resources and handles (e.g., file
descriptors, in-memory state, timers) in the same modules
referenced—clients/agent-runtime/src/agent/unified_loop.rs::test_agent_loop_emits_timeout_error,
clients/agent-runtime/tests/cli_loop_events_e2e.rs::cli_non_preview_timeout_abort_is_session_scoped,
and
clients/agent-runtime/src/gateway/mod.rs::tests::webhook_non_preview_timeout_aborts_with_session_scope—or
point to existing cleanup functions invoked on timeout in
unified_loop/unified_entrypoint so the Compliance summary is accurate.
Comment on lines
296
to
+303
| 1. Phase 1: Config + discovery scaffolding | ||
| - Add schema, validation, MCP client, startup discovery behind `mcp.enabled`. | ||
| - Add schema, validation, MCP client, startup discovery behind `mcp.enabled`. | ||
| 2. Phase 2: Identity + dispatch + policy | ||
| - Introduce namespaced registration and centralized risk/approval handling. | ||
| - Introduce namespaced registration and centralized risk/approval handling. | ||
| 3. Phase 3: Hardening | ||
| - Enforce timeouts/output caps, redaction diagnostics, and failure-isolation tests. | ||
| - Enforce timeouts/output caps, redaction diagnostics, and failure-isolation tests. | ||
| 4. Verification gates | ||
| - Targeted unit/integration tests for all scenarios in spec delta. | ||
| - Targeted unit/integration tests for all scenarios in spec delta. |
Contributor
There was a problem hiding this comment.
Fix ordered-list prefix style in Rollout Plan.
Lines 296-303 use incremental numeric markers, but lint expects 1. for each item (MD029 with 1/1/1 style).
Proposed markdown fix
-1. Phase 1: Config + discovery scaffolding
+1. Phase 1: Config + discovery scaffolding
- Add schema, validation, MCP client, startup discovery behind `mcp.enabled`.
-2. Phase 2: Identity + dispatch + policy
+1. Phase 2: Identity + dispatch + policy
- Introduce namespaced registration and centralized risk/approval handling.
-3. Phase 3: Hardening
+1. Phase 3: Hardening
- Enforce timeouts/output caps, redaction diagnostics, and failure-isolation tests.
-4. Verification gates
+1. Verification gates
- Targeted unit/integration tests for all scenarios in spec delta.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| 1. Phase 1: Config + discovery scaffolding | |
| - Add schema, validation, MCP client, startup discovery behind `mcp.enabled`. | |
| - Add schema, validation, MCP client, startup discovery behind `mcp.enabled`. | |
| 2. Phase 2: Identity + dispatch + policy | |
| - Introduce namespaced registration and centralized risk/approval handling. | |
| - Introduce namespaced registration and centralized risk/approval handling. | |
| 3. Phase 3: Hardening | |
| - Enforce timeouts/output caps, redaction diagnostics, and failure-isolation tests. | |
| - Enforce timeouts/output caps, redaction diagnostics, and failure-isolation tests. | |
| 4. Verification gates | |
| - Targeted unit/integration tests for all scenarios in spec delta. | |
| - Targeted unit/integration tests for all scenarios in spec delta. | |
| 1. Phase 1: Config + discovery scaffolding | |
| - Add schema, validation, MCP client, startup discovery behind `mcp.enabled`. | |
| 1. Phase 2: Identity + dispatch + policy | |
| - Introduce namespaced registration and centralized risk/approval handling. | |
| 1. Phase 3: Hardening | |
| - Enforce timeouts/output caps, redaction diagnostics, and failure-isolation tests. | |
| 1. Verification gates | |
| - Targeted unit/integration tests for all scenarios in spec delta. |
🧰 Tools
🪛 markdownlint-cli2 (0.21.0)
[warning] 298-298: Ordered list item prefix
Expected: 1; Actual: 2; Style: 1/1/1
(MD029, ol-prefix)
[warning] 300-300: Ordered list item prefix
Expected: 1; Actual: 3; Style: 1/1/1
(MD029, ol-prefix)
[warning] 302-302: Ordered list item prefix
Expected: 1; Actual: 4; Style: 1/1/1
(MD029, ol-prefix)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@openspec/changes/archive/2026-03-03-support-mcps-agent-runtime/design.md`
around lines 296 - 303, The ordered list under the Rollout Plan uses incremental
numeric prefixes; change each list item so they all start with "1." to satisfy
MD029 (use 1./1./1. style). Specifically update the lines that begin with "1.
Phase 1: Config + discovery scaffolding", "2. Phase 2: Identity + dispatch +
policy", "3. Phase 3: Hardening", and "4. Verification gates" to each start with
"1." while preserving the text for each phase and bullets.
Comment on lines
+32
to
+33
| `clients/agent-runtime/src/tools/mcp/adapter.rs` for stdio initialize/list/call flow; * | ||
| *Acceptance:** module compiles, startup discovery returns adapter instances for valid servers. |
Contributor
There was a problem hiding this comment.
Fix malformed markdown emphasis around “Acceptance”.
The * *Acceptance:** pattern is malformed and renders inconsistently. Replace with **Acceptance:** in these bullets.
Proposed doc fix
- `clients/agent-runtime/src/tools/mcp/adapter.rs` for stdio initialize/list/call flow; *
- *Acceptance:** module compiles, startup discovery returns adapter instances for valid servers.
+ `clients/agent-runtime/src/tools/mcp/adapter.rs` for stdio initialize/list/call flow;
+ **Acceptance:** module compiles, startup discovery returns adapter instances for valid servers.Also applies to: 39-40, 66-68, 80-82, 103-105, 107-109
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@openspec/changes/archive/2026-03-03-support-mcps-agent-runtime/tasks.md`
around lines 32 - 33, Replace the malformed markdown emphasis token "*
*Acceptance:**" with the correct "**Acceptance:**" in the document: search for
the literal substring "* *Acceptance:**" (and similar malformed variants) in
openspec/changes/archive/2026-03-03-support-mcps-agent-runtime/tasks.md and
update each occurrence (notably around the previously flagged ranges) so the
bullet headers render consistently as "**Acceptance:**".
Comment on lines
+49
to
+53
| - | ||
| `cargo test -p corvus --test legacy_loop_guard --test mission_lifecycle_integration --test mission_governance_integration --test mission_security_parity --test mission_config_toggle --test mission_entrypoint_parity` -> | ||
| ✅ passed (25 passed / 0 failed) | ||
| - `cargo test -p corvus concurrent_transition_attempts_are_serialized_with_single_winner` -> ✅ | ||
| passed |
Contributor
There was a problem hiding this comment.
Fix malformed markdown list item for test evidence.
This bullet is split into a standalone - plus wrapped lines, which hurts readability and can render inconsistently.
Proposed formatting fix
--
-`cargo test -p corvus --test legacy_loop_guard --test mission_lifecycle_integration --test mission_governance_integration --test mission_security_parity --test mission_config_toggle --test mission_entrypoint_parity` ->
-✅ passed (25 passed / 0 failed)
+- `cargo test -p corvus --test legacy_loop_guard --test mission_lifecycle_integration --test mission_governance_integration --test mission_security_parity --test mission_config_toggle --test mission_entrypoint_parity` -> ✅ passed (25 passed / 0 failed)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| - | |
| `cargo test -p corvus --test legacy_loop_guard --test mission_lifecycle_integration --test mission_governance_integration --test mission_security_parity --test mission_config_toggle --test mission_entrypoint_parity` -> | |
| ✅ passed (25 passed / 0 failed) | |
| - `cargo test -p corvus concurrent_transition_attempts_are_serialized_with_single_winner` -> ✅ | |
| passed | |
| - `cargo test -p corvus --test legacy_loop_guard --test mission_lifecycle_integration --test mission_governance_integration --test mission_security_parity --test mission_config_toggle --test mission_entrypoint_parity` -> ✅ passed (25 passed / 0 failed) | |
| - `cargo test -p corvus concurrent_transition_attempts_are_serialized_with_single_winner` -> ✅ | |
| passed |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@openspec/changes/archive/2026-03-04-agent-runtime-mission-layer/verify-report.md`
around lines 49 - 53, The markdown has a broken list item split into a stray "-"
line and wrapped lines; fix verify-report.md by merging the fragmented lines
into a single well-formed bullet that contains the command and its result (e.g.,
one bullet like "`cargo test -p corvus --test legacy_loop_guard --test
mission_lifecycle_integration --test mission_governance_integration --test
mission_security_parity --test mission_config_toggle --test
mission_entrypoint_parity` -> ✅ passed (25 passed / 0 failed)" and similarly
ensure the `cargo test -p corvus
concurrent_transition_attempts_are_serialized_with_single_winner` evidence is a
single bullet showing "✅ passed"); ensure there are no standalone "-" lines and
wrapping is preserved within each list item.
Comment on lines
+17
to
+23
| **Choice**: Use Vue 3's native Composition API (reactive/ref) with a centralized composable | ||
| pattern (e.g., `useConfigStore.ts`), rather than introducing Pinia. | ||
| **Alternatives considered**: Pinia (adds unnecessary boilerplate for a relatively flat, form-heavy | ||
| prototype). Prop-drilling (creates brittle and overly coupled components). | ||
| **Rationale**: The configuration state is primarily form data fetched once and synced on save. A | ||
| simple composable provides enough reactivity for nested configuration components to read and update | ||
| their specific slices of the configuration without adding external dependencies. |
Contributor
There was a problem hiding this comment.
Composable filename is inconsistent with task/spec references.
This section references useConfigStore.ts, while related docs/tasks reference useConfig.ts. Use one canonical name to prevent implementation/documentation drift.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@openspec/changes/archive/2026-03-04-web-agent-config/design.md` around lines
17 - 23, The documentation uses two different composable names—useConfigStore.ts
(in design.md) and useConfig.ts (in related docs/tasks); pick one canonical name
(e.g., useConfig.ts) and make the references consistent: rename the file if
necessary and update all mentions (design.md, task descriptions, example
imports/usages) to that chosen symbol so implementation, docs, and task
references match; ensure any import paths, examples, and CI/docs links are
updated to the canonical composable name (useConfig.ts or useConfigStore.ts)
across the repo.
Comment on lines
+7
to
+10
| The backend configuration update payload (`AdminConfigUpdateRequest`) MUST support all fields | ||
| defined in the system's `config.toml`. | ||
| The backend MUST securely accept updates to server settings, agent identity details, and LLM | ||
| provider configurations. |
Contributor
There was a problem hiding this comment.
Requirement overstates update scope (“all fields in config.toml”).
This conflicts with the documented hidden/non-editable sections in the same change set. Please narrow wording to “all admin-editable fields” to keep spec and implementation aligned.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@openspec/changes/archive/2026-03-04-web-agent-config/specs/agent-config/spec.md`
around lines 7 - 10, The spec currently requires AdminConfigUpdateRequest to
support "all fields defined in the system's `config.toml`" which overstates
scope; update the text to require support for "all admin-editable fields"
instead and adjust the following sentence to state that the backend MUST
securely accept updates to admin-editable server settings, agent identity
details, and LLM provider configurations; search for the phrase
"AdminConfigUpdateRequest" and the exact string "all fields defined in the
system's `config.toml`" and replace it with "all admin-editable fields" and
similarly update the subsequent sentence to reference "admin-editable" fields to
keep the spec aligned with hidden/non-editable sections.
Comment on lines
+15
to
19
| - WHEN a POST request is sent to the configuration update endpoint with a complete JSON payload | ||
| representing all `config.toml` fields | ||
| - THEN the server validates the payload | ||
| - AND successfully updates the running configuration and persists it to `config.toml` | ||
| - AND responds with a 200 OK status containing the updated `AdminConfigView` |
Contributor
There was a problem hiding this comment.
Endpoint method is incorrect in the scenario contract.
Line 15 specifies POST, but the documented and verified admin config update flow uses PUT /web/admin/config. This should be corrected to avoid client integration errors.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@openspec/changes/archive/2026-03-04-web-agent-config/specs/agent-config/spec.md`
around lines 15 - 19, Update the scenario contract that currently states "POST"
to use the correct HTTP method "PUT /web/admin/config" so it matches the
verified admin config update flow; change the step that reads WHEN a POST
request is sent to instead specify a PUT to /web/admin/config carrying the full
JSON payload for config.toml, ensuring the rest of the scenario (validation,
persisting to config.toml, and returning 200 with AdminConfigView) remains
unchanged.
Comment on lines
1
to
8
| ## Verification Report | ||
|
|
||
| **Change**: web-agent-config | ||
| **Change**: web-agent-config | ||
| **Version**: N/A (delta specs) | ||
|
|
||
| --- | ||
|
|
||
| ### Completeness |
Contributor
There was a problem hiding this comment.
Fix heading hierarchy to resolve MD001.
The document’s heading levels skip expected progression. Start with a top-level # heading (or adjust subsequent levels) so headings only increment one level at a time.
🧰 Tools
🪛 markdownlint-cli2 (0.21.0)
[warning] 3-3: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@openspec/changes/archive/2026-03-04-web-agent-config/verify-report.md` around
lines 1 - 8, The document's headings skip levels (MD001); update the heading
hierarchy in verify-report.md so it starts with a single top-level heading
(e.g., change "## Verification Report" to "# Verification Report") and ensure
subsequent headings only increase by one level at a time (for example use "##
Completeness" rather than jumping from # to ###). Verify all other headings in
the file follow sequential levels and adjust them accordingly.
Comment on lines
+378
to
+397
| ```rust | ||
| pub struct UpdateStatusView { | ||
| pub current_version: String, | ||
| pub latest_version: Option<String>, | ||
| pub update_available: bool, | ||
| pub last_check_at_unix: Option<u64>, | ||
| pub last_check_outcome: Option<String>, | ||
| pub effective_install_method: String, | ||
| pub detected_install_method: Option<String>, | ||
| pub install_method_source: String, // override|detected|unknown | ||
| pub policy: UpdatePolicyView, | ||
| } | ||
|
|
||
| pub struct UpdatePolicyView { | ||
| pub checks_enabled: bool, | ||
| pub auto_install_enabled: bool, | ||
| pub channel_visibility_enabled: bool, | ||
| pub cli_startup_notice_enabled: bool, | ||
| pub restart_policy: String, | ||
| } |
Contributor
There was a problem hiding this comment.
Tighten Rust↔TS contract details for enum/string fields.
UpdateStatusView/UpdatePolicyView currently use free-form String for fields like install method and restart policy. Please specify canonical serialized values (and casing/serde strategy) so clients/web/apps/dashboard/src/types/admin-config.ts can reliably type and parse them without contract ambiguity.
Proposed contract clarification snippet
pub struct UpdateStatusView {
pub current_version: String,
pub latest_version: Option<String>,
pub update_available: bool,
pub last_check_at_unix: Option<u64>,
pub last_check_outcome: Option<String>,
- pub effective_install_method: String,
- pub detected_install_method: Option<String>,
- pub install_method_source: String, // override|detected|unknown
+ // serialized as snake_case enum strings (e.g., "npm", "homebrew", "unknown")
+ pub effective_install_method: String,
+ pub detected_install_method: Option<String>,
+ // serialized enum: "override" | "detected" | "unknown"
+ pub install_method_source: String,
pub policy: UpdatePolicyView,
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@openspec/changes/enhance-auto-update-system/design.md` around lines 378 -
397, Replace the free-form String fields with explicit enum-backed types and
document their serialized representations and casing/serde strategy: change
install_method and restart_policy related fields in UpdateStatusView
(effective_install_method, detected_install_method, install_method_source) and
UpdatePolicyView (restart_policy) to specific enums (e.g., InstallMethod,
InstallMethodSource, RestartPolicy) and add serde attributes (e.g.,
#[serde(rename_all = "snake_case")]/#[serde(rename = "...")] to guarantee
canonical string values; update the design.md snippet to list every enum variant
and its exact serialized string and casing, and note that
clients/web/apps/dashboard/src/types/admin-config.ts must mirror these exact
strings/types when parsing.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 15
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
clients/web/apps/dashboard/src/components/config/RuntimeSettings.vue (2)
16-21:⚠️ Potential issue | 🔴 CriticalRuntime break: function name mismatch.
The function is defined as
_updateRuntimeKind(line 16) but the template referencesupdateRuntimeKind(line 33). This will throw a runtime error.Either rename the function back or update the template reference.
🐛 Proposed fix
-function _updateRuntimeKind(kind: string): void { +function updateRuntimeKind(kind: string): void { emit("update:modelValue", { ...props.modelValue, runtime_kind: kind, }); }Also applies to: 33-33
🤖 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/RuntimeSettings.vue` around lines 16 - 21, The template calls updateRuntimeKind but the script defines _updateRuntimeKind, causing a runtime error; fix by making the names match—either rename the function _updateRuntimeKind to updateRuntimeKind or update the template to call _updateRuntimeKind; ensure the function that calls emit("update:modelValue", { ...props.modelValue, runtime_kind: kind }) uses the same identifier referenced in the template so the component can update props.modelValue.runtime_kind correctly.
42-42:⚠️ Potential issue | 🔴 CriticalAdd missing component imports.
ButtonandInputcomponents are used throughout the template but not imported. The project has no auto-import plugins configured (Vite config only includes Vue plugin), and components are not globally registered inApp.vue. This will cause runtime errors.Add the following imports at the top of the
<script setup>:import Button from "@/components/ui/button/Button.vue"; import Input from "@/components/ui/input/Input.vue";This issue affects multiple files in the
config/directory (GatewaySettings, GeneralSettings, ObservabilitySettings, RuntimeSettings, SchedulerSettings, SecuritySettings, WebhookSettings) and App.vue itself.🤖 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/RuntimeSettings.vue` at line 42, Add missing component imports for Button and Input at the top of the <script setup> block in RuntimeSettings.vue (and repeat the same fix in GatewaySettings, GeneralSettings, ObservabilitySettings, SchedulerSettings, SecuritySettings, WebhookSettings, and App.vue): import the Button component from components/ui/button/Button.vue and the Input component from components/ui/input/Input.vue so the template’s <Button> and <Input> usages resolve at runtime; in single-file components using <script setup> simply add these imports (no further registration needed).clients/web/apps/dashboard/src/components/config/SecuritySettings.vue (1)
16-24:⚠️ Potential issue | 🔴 CriticalCritical: Function renamed but template references outdated name.
Same issue as ObservabilitySettings.vue. The function is
_updateFieldbut template callsupdateFieldon lines 36, 49, 58, 63, 69, and 76.🐛 Proposed fix
-function _updateField<Key extends keyof AdminConfigForm>( +function updateField<Key extends keyof AdminConfigForm>( key: Key, value: AdminConfigForm[Key] ): void {🤖 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/SecuritySettings.vue` around lines 16 - 24, The template calls updateField but the script defines _updateField, causing runtime errors; either rename the function _updateField to updateField or add a small wrapper named updateField that calls _updateField so the template bindings resolve (update the function name/reference in the SecuritySettings.vue component where _updateField is declared and ensure the template calls on lines mentioned use that exact exported function name).clients/web/apps/dashboard/src/components/config/GatewaySettings.vue (1)
15-23:⚠️ Potential issue | 🔴 CriticalCritical: Function renamed but template references outdated name.
Same issue as other config components. The function is
_updateFieldbut template callsupdateFieldon lines 37, 42, 50, 59, 66, and 74.🐛 Proposed fix
-function _updateField<Key extends keyof AdminConfigForm>( +function updateField<Key extends keyof AdminConfigForm>( key: Key, value: AdminConfigForm[Key] ): void {🤖 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/GatewaySettings.vue` around lines 15 - 23, Template calls use updateField but the script defines _updateField, causing undefined method errors; either rename the function to updateField or add an alias exposed to the template (e.g., const updateField = _updateField) so the template can call it. Locate the function _updateField in GatewaySettings.vue and either change its name to updateField or create a simple wrapper/alias named updateField that forwards to _updateField and ensure it's returned/exposed to the template.clients/web/apps/dashboard/src/components/config/ObservabilitySettings.vue (1)
16-24:⚠️ Potential issue | 🔴 CriticalCritical: Function renamed but template references outdated name.
The function is renamed to
_updateField, but the template (lines 36, 45, 51) still callsupdateField. This will cause a runtime error.Either revert the rename or update all template references.
🐛 Proposed fix
-function _updateField<Key extends keyof AdminConfigForm>( +function updateField<Key extends keyof AdminConfigForm>( key: Key, value: AdminConfigForm[Key] ): void {Or update all template references to
_updateField.🤖 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/ObservabilitySettings.vue` around lines 16 - 24, The template still calls updateField while the function was renamed to _updateField; update all template bindings/calls that reference updateField (e.g., the input/button handlers in the ObservabilitySettings.vue template) to call _updateField instead, or alternatively rename the function back to updateField so the template matches; ensure the symbol names _updateField and updateField are consistent between the script and template to prevent the runtime error.
🤖 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/channels/mod.rs`:
- Around line 466-474: The outer visibility check using
update_visibility_enabled before calling maybe_send_opportunistic_update_notice
duplicates the same guard that maybe_send_opportunistic_update_notice performs
internally; either remove the outer if and call
maybe_send_opportunistic_update_notice directly (letting its internal guard
handle it) or, if you want to keep the outer short-circuit to avoid an async
call, add a concise comment above the if that references
update_visibility_enabled and maybe_send_opportunistic_update_notice and
explains the intentional redundancy to prevent future divergence.
In `@clients/agent-runtime/src/config/schema.rs`:
- Around line 2107-2114: normalize_install_method_override rejects "unknown" but
InstallMethod::from_str in update::mod accepts it, causing a mismatch for
CORVUS_UPDATE_METHOD_OVERRIDE; update normalize_install_method_override to
accept "unknown" (i.e., include "unknown" in the match arm) so its accepted
values align with InstallMethod::from_str and the runtime/config contract is
consistent. Reference normalize_install_method_override and
InstallMethod::from_str when making the change.
In `@clients/agent-runtime/src/daemon/mod.rs`:
- Around line 486-494: The test updater_supervision_follows_update_policy is
flaky because updater_supervision_enabled() depends on
crate::update::is_update_check_disabled() which reads
CORVUS_DISABLE_UPDATE_CHECK from the environment; fix the test by controlling
that env var so the function's result is deterministic: in the test
updater_supervision_follows_update_policy explicitly set or remove
CORVUS_DISABLE_UPDATE_CHECK (e.g., std::env::set_var or std::env::remove_var)
before calling updater_supervision_enabled(), test both config.updates.enabled
true/false with the env var set and unset as needed, and restore the original
env value after the test to avoid side effects. Ensure you reference
updater_supervision_enabled, is_update_check_disabled, and
config.updates.enabled when making changes.
In `@clients/agent-runtime/src/gateway/admin.rs`:
- Around line 554-581: The call to update::get_update_status currently uses
.ok(), silently swallowing errors and turning failures into "unknown"/false in
AdminUpdatesView/AdminUpdateStatusView; instead, preserve and surface failures
by handling the Result explicitly: remove .ok(), capture the Err case from
update::get_update_status(cfg, env!("CARGO_PKG_VERSION")), and propagate an
error (return Result from the handler) or populate AdminUpdateStatusView with an
explicit failure indicator and error message (e.g., set last_check_outcome or a
new status_error field) so operators can see the underlying error; update the
code paths that build AdminUpdatesView/AdminUpdateStatusView to pattern-match on
the Result from update::get_update_status and either use the Ok(view) values or
include the Err details rather than defaulting to "unknown".
In `@clients/agent-runtime/src/update/mod.rs`:
- Around line 606-634: The code is marking
InstallState::InstalledPendingRestart, saving state, appending a success
UpdateAuditEvent, and returning InstallCommandOutcome::Success before actually
running any installer for methods like npm/pnpm/yarn/bun/homebrew/cargo; move
the state change, save_state_snapshot_sync, append_audit_event_sync, and the
success return to occur only after the installer has been executed and confirmed
successful. Detect the installer method via effective.as_str()/effective, invoke
the appropriate install flow (npm/pnpm/yarn/bun/homebrew/cargo handler), check
for errors, and on success set InstallState::InstalledPendingRestart (with
version and installed_at_unix), call save_state_snapshot_sync and
append_audit_event_sync with outcome "success"; on installer failure append an
audit event with outcome "failure" and return the matching InstallCommandOutcome
error case instead of Success. Ensure actor and source values are preserved when
creating UpdateAuditEvent.
- Around line 2108-2114: Do not call unwrap_or_default on
read_update_history_sync because it hides I/O/parse failures and can cause a
lost audit trail; instead handle the Result from read_update_history_sync
explicitly (propagate the Err up or return an error) before mutating events so
that failures abort the update rather than resetting history. Locate the call to
read_update_history_sync and replace the unwrap_or_default usage with proper
error handling (e.g., match/try/? on the Result) so events is only
created/modified when the read succeeds; keep the later logic that trims to
policy.history_max_entries intact.
- Around line 442-444: The current update-state flow mixes a file-based lock
(acquire_file_lock & update_state_lock_path) and an in-process async mutex,
allowing races when both CLI/daemon/channel mutate version_check.json; unify
them by switching the in-process async mutex usage to the same file lock domain:
replace the async mutex-protected critical sections with calls to
acquire_file_lock(&update_state_lock_path(&self.workspace_dir), ...) and hold
that lock across load_state_snapshot_sync, UpdateStateSnapshot::initial
creation, and any write/flush operations so all code paths serialize on the same
file lock; ensure proper error propagation and timely lock release around the
functions load_state_snapshot_sync, any snapshot mutation code, and the writer
that persists version_check.json.
- Around line 724-751: The state mutex (state_lock()) is held across the
long-running await of execute_minimal_update_strategy, serializing other
operations; limit the lock to only the state mutation sequence around
load_state, prune_pending_confirmations, consume_pending_confirmation, and
save_state, then drop the guard before calling
execute_minimal_update_strategy(...). Specifically, keep calls to
version_check_path, load_state, prune_pending_confirmations,
consume_pending_confirmation, and save_state under the guard, then explicitly
drop the guard (or move them into a shorter scope) before awaiting
execute_minimal_update_strategy, and leave append_confirmation_audit_event and
subsequent non-state-mutating work outside the lock; use the existing function
names (state_lock, load_state, prune_pending_confirmations,
consume_pending_confirmation, save_state, execute_minimal_update_strategy,
append_confirmation_audit_event) to locate the code to change.
In `@clients/web/apps/dashboard/src/App.vue`:
- Around line 24-26: The template is still binding to webhookSecretStatusLabel
while the computed was renamed to _webhookSecretStatusLabel, breaking the
binding; rename the computed back to webhookSecretStatusLabel (or update the
template to _webhookSecretStatusLabel) and ensure that the computed is returned
from the setup() return object so the template can access it (refer to the
computed identifier _webhookSecretStatusLabel in App.vue and the template
binding webhookSecretStatusLabel).
In `@clients/web/apps/dashboard/src/components/config/GeneralSettings.vue`:
- Around line 16-19: The template calls updateField(...) but the script only
defines _updateField(...) causing the handler to be undefined; fix by either
renaming the function to export a matching name (change _updateField to
updateField) or add a small wrapper export function updateField(...) that
delegates to _updateField; update all references in the template (the calls in
the form handlers currently calling updateField in lines where field updates
occur) to match the chosen function name so the form updates invoke the defined
handler.
In `@clients/web/apps/dashboard/src/components/config/SchedulerSettings.vue`:
- Around line 15-18: The helper was renamed to _updateField but other callers
still invoke updateField, causing template bindings to fail; update all usages
(the calls at the locations invoking updateField in this component) to call
_updateField instead or restore the original export/alias so the template can
access it (ensure the component's methods/refs expose _updateField under the
name the template expects). Specifically, change every call/site that references
updateField to use _updateField (or create a small wrapper named updateField
that delegates to _updateField) so scheduler form inputs correctly update
AdminConfigForm.
In `@clients/web/apps/dashboard/src/components/config/WebhookSettings.vue`:
- Around line 26-37: The template still calls updateField and handleSave but the
implementation was renamed to _updateField and _handleSave, breaking bindings;
fix by exposing the new functions under the original names so the template can
call them (either rename the functions back or create aliases named updateField
-> _updateField and handleSave -> _handleSave and ensure they are
returned/exposed from the component setup), updating references for functions
_updateField and _handleSave so template calls resolve at runtime.
In `@clients/web/apps/dashboard/src/types/admin-config.ts`:
- Around line 63-79: The TypeScript typing makes the top-level updates and its
nested status optional, but the backend Rust API requires
AdminConfigView.updates and updates.status to be present; change the declaration
so updates is required (remove the ? on updates) and make status required inside
that object (remove the ? on status), leaving individual fields inside status
optional as appropriate to preserve expressiveness while restoring the contract
alignment with AdminConfigView and updates.status.
In `@openspec/changes/enhance-auto-update-system/apply-progress.md`:
- Line 23: The runtime errors come from an inconsistent rename of updateField to
_updateField causing template/script mismatches; locate components where
updateField/_updateField is referenced (template bindings, methods, emits,
watchers, and any imported symbols) and make them consistent: either revert to
updateField everywhere or update all template bindings and script references to
_updateField (including props/emits and any calls from parent components), then
run the Vue build to confirm templates no longer reference the old name and
update related unit/integration tests accordingly.
---
Outside diff comments:
In `@clients/web/apps/dashboard/src/components/config/GatewaySettings.vue`:
- Around line 15-23: Template calls use updateField but the script defines
_updateField, causing undefined method errors; either rename the function to
updateField or add an alias exposed to the template (e.g., const updateField =
_updateField) so the template can call it. Locate the function _updateField in
GatewaySettings.vue and either change its name to updateField or create a simple
wrapper/alias named updateField that forwards to _updateField and ensure it's
returned/exposed to the template.
In `@clients/web/apps/dashboard/src/components/config/ObservabilitySettings.vue`:
- Around line 16-24: The template still calls updateField while the function was
renamed to _updateField; update all template bindings/calls that reference
updateField (e.g., the input/button handlers in the ObservabilitySettings.vue
template) to call _updateField instead, or alternatively rename the function
back to updateField so the template matches; ensure the symbol names
_updateField and updateField are consistent between the script and template to
prevent the runtime error.
In `@clients/web/apps/dashboard/src/components/config/RuntimeSettings.vue`:
- Around line 16-21: The template calls updateRuntimeKind but the script defines
_updateRuntimeKind, causing a runtime error; fix by making the names
match—either rename the function _updateRuntimeKind to updateRuntimeKind or
update the template to call _updateRuntimeKind; ensure the function that calls
emit("update:modelValue", { ...props.modelValue, runtime_kind: kind }) uses the
same identifier referenced in the template so the component can update
props.modelValue.runtime_kind correctly.
- Line 42: Add missing component imports for Button and Input at the top of the
<script setup> block in RuntimeSettings.vue (and repeat the same fix in
GatewaySettings, GeneralSettings, ObservabilitySettings, SchedulerSettings,
SecuritySettings, WebhookSettings, and App.vue): import the Button component
from components/ui/button/Button.vue and the Input component from
components/ui/input/Input.vue so the template’s <Button> and <Input> usages
resolve at runtime; in single-file components using <script setup> simply add
these imports (no further registration needed).
In `@clients/web/apps/dashboard/src/components/config/SecuritySettings.vue`:
- Around line 16-24: The template calls updateField but the script defines
_updateField, causing runtime errors; either rename the function _updateField to
updateField or add a small wrapper named updateField that calls _updateField so
the template bindings resolve (update the function name/reference in the
SecuritySettings.vue component where _updateField is declared and ensure the
template calls on lines mentioned use that exact exported function name).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 71685eac-0a08-4e32-b19d-63e533b1b296
📒 Files selected for processing (29)
README.mdclients/agent-runtime/src/channels/mod.rsclients/agent-runtime/src/config/schema.rsclients/agent-runtime/src/daemon/mod.rsclients/agent-runtime/src/gateway/admin.rsclients/agent-runtime/src/main.rsclients/agent-runtime/src/service/mod.rsclients/agent-runtime/src/update/mod.rsclients/agent-runtime/tests/admin_config_api_integration.rsclients/agent-runtime/tests/update_system_integration.rsclients/web/apps/dashboard/src/App.vueclients/web/apps/dashboard/src/components/config/GatewaySettings.spec.tsclients/web/apps/dashboard/src/components/config/GatewaySettings.vueclients/web/apps/dashboard/src/components/config/GeneralSettings.spec.tsclients/web/apps/dashboard/src/components/config/GeneralSettings.vueclients/web/apps/dashboard/src/components/config/ObservabilitySettings.vueclients/web/apps/dashboard/src/components/config/RuntimeSettings.vueclients/web/apps/dashboard/src/components/config/SchedulerSettings.vueclients/web/apps/dashboard/src/components/config/SecuritySettings.spec.tsclients/web/apps/dashboard/src/components/config/SecuritySettings.vueclients/web/apps/dashboard/src/components/config/WebhookSettings.spec.tsclients/web/apps/dashboard/src/components/config/WebhookSettings.vueclients/web/apps/dashboard/src/composables/configPayload.tsclients/web/apps/dashboard/src/composables/useConfig.spec.tsclients/web/apps/dashboard/src/composables/useConfig.tsclients/web/apps/dashboard/src/types/admin-config.tsopenspec/changes/enhance-auto-update-system/apply-progress.mdopenspec/changes/enhance-auto-update-system/tasks.mdopenspec/changes/enhance-auto-update-system/verify-report.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). (4)
- GitHub Check: pr-checks
- GitHub Check: sonar
- GitHub Check: pr-checks
- GitHub Check: Cloudflare Pages
🧰 Additional context used
📓 Path-based instructions (10)
**/*.{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:
README.mdopenspec/changes/enhance-auto-update-system/verify-report.mdopenspec/changes/enhance-auto-update-system/tasks.mdopenspec/changes/enhance-auto-update-system/apply-progress.md
**/*
⚙️ 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:
README.mdclients/web/apps/dashboard/src/components/config/GatewaySettings.spec.tsopenspec/changes/enhance-auto-update-system/verify-report.mdopenspec/changes/enhance-auto-update-system/tasks.mdclients/agent-runtime/src/service/mod.rsclients/web/apps/dashboard/src/composables/useConfig.spec.tsclients/web/apps/dashboard/src/composables/useConfig.tsclients/agent-runtime/tests/admin_config_api_integration.rsclients/agent-runtime/src/main.rsclients/web/apps/dashboard/src/types/admin-config.tsclients/web/apps/dashboard/src/components/config/GeneralSettings.spec.tsclients/web/apps/dashboard/src/components/config/RuntimeSettings.vueclients/agent-runtime/src/daemon/mod.rsclients/agent-runtime/tests/update_system_integration.rsclients/web/apps/dashboard/src/components/config/SecuritySettings.spec.tsopenspec/changes/enhance-auto-update-system/apply-progress.mdclients/agent-runtime/src/config/schema.rsclients/agent-runtime/src/channels/mod.rsclients/web/apps/dashboard/src/components/config/WebhookSettings.spec.tsclients/web/apps/dashboard/src/components/config/SchedulerSettings.vueclients/web/apps/dashboard/src/components/config/WebhookSettings.vueclients/web/apps/dashboard/src/components/config/ObservabilitySettings.vueclients/web/apps/dashboard/src/components/config/SecuritySettings.vueclients/web/apps/dashboard/src/App.vueclients/agent-runtime/src/update/mod.rsclients/agent-runtime/src/gateway/admin.rsclients/web/apps/dashboard/src/components/config/GeneralSettings.vueclients/web/apps/dashboard/src/composables/configPayload.tsclients/web/apps/dashboard/src/components/config/GatewaySettings.vue
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/service/mod.rsclients/agent-runtime/src/main.rsclients/agent-runtime/src/daemon/mod.rsclients/agent-runtime/src/config/schema.rsclients/agent-runtime/src/channels/mod.rsclients/agent-runtime/src/update/mod.rsclients/agent-runtime/src/gateway/admin.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/service/mod.rsclients/agent-runtime/tests/admin_config_api_integration.rsclients/agent-runtime/src/main.rsclients/agent-runtime/src/daemon/mod.rsclients/agent-runtime/tests/update_system_integration.rsclients/agent-runtime/src/config/schema.rsclients/agent-runtime/src/channels/mod.rsclients/agent-runtime/src/update/mod.rsclients/agent-runtime/src/gateway/admin.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/service/mod.rsclients/agent-runtime/tests/admin_config_api_integration.rsclients/agent-runtime/src/main.rsclients/agent-runtime/src/daemon/mod.rsclients/agent-runtime/tests/update_system_integration.rsclients/agent-runtime/src/config/schema.rsclients/agent-runtime/src/channels/mod.rsclients/agent-runtime/src/update/mod.rsclients/agent-runtime/src/gateway/admin.rs
clients/agent-runtime/src/main.rs
📄 CodeRabbit inference engine (clients/agent-runtime/AGENTS.md)
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
Keep startup path lean and avoid heavy initialization in command parsing flow
Files:
clients/agent-runtime/src/main.rs
**/*.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/RuntimeSettings.vueclients/web/apps/dashboard/src/components/config/SchedulerSettings.vueclients/web/apps/dashboard/src/components/config/WebhookSettings.vueclients/web/apps/dashboard/src/components/config/ObservabilitySettings.vueclients/web/apps/dashboard/src/components/config/SecuritySettings.vueclients/web/apps/dashboard/src/App.vueclients/web/apps/dashboard/src/components/config/GeneralSettings.vueclients/web/apps/dashboard/src/components/config/GatewaySettings.vue
clients/agent-runtime/src/{security,gateway,tools,config}/**/*.rs
📄 CodeRabbit inference engine (clients/agent-runtime/AGENTS.md)
Do not silently weaken security policy or access constraints; keep default behavior secure-by-default with deny-by-default where applicable
Files:
clients/agent-runtime/src/config/schema.rsclients/agent-runtime/src/gateway/admin.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/mod.rs
clients/agent-runtime/src/{security,gateway,tools}/**/*.rs
📄 CodeRabbit inference engine (clients/agent-runtime/AGENTS.md)
Treat
src/security/,src/gateway/,src/tools/as high-risk surfaces and never broaden filesystem/network execution scope without explicit policy checks
Files:
clients/agent-runtime/src/gateway/admin.rs
🧠 Learnings (10)
📚 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/**/*.rs : Run `cargo fmt --all -- --check`, `cargo clippy --all-targets -- -D warnings`, and `cargo test` for code validation, or document which checks were skipped and why
Applied to files:
openspec/changes/enhance-auto-update-system/verify-report.mdclients/agent-runtime/tests/admin_config_api_integration.rsclients/agent-runtime/src/main.rsclients/agent-runtime/tests/update_system_integration.rsclients/agent-runtime/src/config/schema.rsclients/agent-runtime/src/channels/mod.rsclients/agent-runtime/src/update/mod.rsclients/agent-runtime/src/gateway/admin.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/service/mod.rsclients/agent-runtime/tests/admin_config_api_integration.rsclients/agent-runtime/src/main.rsclients/agent-runtime/tests/update_system_integration.rsclients/agent-runtime/src/config/schema.rsclients/agent-runtime/src/channels/mod.rsclients/agent-runtime/src/update/mod.rsclients/agent-runtime/src/gateway/admin.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/{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/agent-runtime/tests/admin_config_api_integration.rsclients/agent-runtime/src/main.rsclients/agent-runtime/tests/update_system_integration.rsclients/agent-runtime/src/config/schema.rsclients/agent-runtime/src/channels/mod.rsclients/agent-runtime/src/update/mod.rsclients/agent-runtime/src/gateway/admin.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 : Keep startup path lean and avoid heavy initialization in command parsing flow
Applied to files:
clients/agent-runtime/src/main.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/{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/agent-runtime/tests/update_system_integration.rsclients/agent-runtime/src/channels/mod.rsclients/agent-runtime/src/update/mod.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/**/Cargo.toml : Do not add heavy dependencies for minor convenience; justify new crate additions
Applied to files:
clients/agent-runtime/tests/update_system_integration.rsclients/agent-runtime/src/update/mod.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/tools/**/*.rs : Implement `Tool` trait in `src/tools/` with strict parameter schema, validate and sanitize all inputs, and return structured `ToolResult` without panics in runtime path
Applied to files:
clients/agent-runtime/tests/update_system_integration.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/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/agent-runtime/tests/update_system_integration.rsclients/agent-runtime/src/channels/mod.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/**/*.rs : Avoid unnecessary allocations, clones, and blocking operations to maintain performance and efficiency
Applied to files:
clients/agent-runtime/tests/update_system_integration.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/**/Cargo.toml : Preserve release-size profile assumptions in `Cargo.toml` and avoid adding heavy dependencies unless clearly justified
Applied to files:
clients/agent-runtime/src/update/mod.rs
🔇 Additional comments (16)
README.md (1)
24-24: Badge update looks good.The Markdown/link change is valid and does not introduce doc accuracy or behavior alignment issues.
clients/web/apps/dashboard/src/components/config/GatewaySettings.spec.ts (1)
3-3: Import reorder is safe and non-functional.No security, behavioral, or contract impact from this change.
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."
clients/web/apps/dashboard/src/components/config/SecuritySettings.spec.ts (1)
1-32: LGTM!Import reordering only; test logic unchanged.
clients/web/apps/dashboard/src/components/config/WebhookSettings.spec.ts (1)
1-32: LGTM!Import reordering only; test logic unchanged.
clients/agent-runtime/src/channels/mod.rs (2)
113-115: LGTM: Clean visibility gate helper.The helper correctly implements the AND condition for both flags.
3185-3198: LGTM: Good test coverage for the visibility gate.Test validates all flag combinations correctly.
openspec/changes/enhance-auto-update-system/verify-report.md (1)
51-57: 🧹 Nitpick | 🔵 TrivialResidual risk appropriately documented.
The acknowledgment that full-repository regression (
make test,make build) was not re-executed is important context. Consider running these before final merge, especially given the Vue component issues identified in the dashboard. Based on learnings: "Runcargo fmt --all -- --check,cargo clippy --all-targets -- -D warnings, andcargo testfor code validation."⛔ Skipped due to learnings
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/**/*.rs : Run `cargo fmt --all -- --check`, `cargo clippy --all-targets -- -D warnings`, and `cargo test` for code validation, or document which checks were skipped and whyLearnt 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 modesclients/agent-runtime/src/service/mod.rs (1)
22-31: Restart decision mapping is clear and exhaustive.This explicit bridge from
RestartActiontoRestartDecisionis straightforward and maintainable.clients/web/apps/dashboard/src/composables/configPayload.ts (1)
71-76: Good addition formemory_backenddiffing.Including this field in
buildGeneralPayloadkeeps payload generation consistent with the form/snapshot contract.clients/agent-runtime/src/main.rs (2)
770-841: Explicit outcome handling for update subcommands looks solid.The command handler cleanly maps each update outcome to deterministic exit behavior, with no silent fallback paths.
1722-1782: Good CLI contract coverage for newupdateverbs.These parser tests protect the intended command surface (
status/check/install/auto-enable/auto-disable/history/confirm) from accidental regressions.clients/agent-runtime/tests/admin_config_api_integration.rs (1)
165-177: Nice coverage expansion for updates in admin config API response.These assertions make the new update status surface contractual in integration tests.
clients/agent-runtime/tests/update_system_integration.rs (1)
170-206: Cross-surface status parity test is valuable.Validating CLI and admin views against the same update facts helps prevent contract drift between interfaces.
clients/web/apps/dashboard/src/components/config/GeneralSettings.spec.ts (1)
3-5: i18n test setup restoration looks correct.The test now mounts with translation context required by localized labels.
clients/agent-runtime/src/gateway/admin.rs (2)
33-54: Good addition of explicit admin update view contracts.The new
AdminUpdatesView/AdminUpdateStatusViewshape is clear and keeps sensitive values out of the payload.
1337-1351: Nice regression coverage for the newupdatespayload.These assertions help lock in the serialized contract presence for nested update status fields.
Comment on lines
+466
to
+474
| if update_visibility_enabled(ctx.config.as_ref()) { | ||
| let _ = crate::update::maybe_send_opportunistic_update_notice( | ||
| ctx.config.as_ref(), | ||
| &msg, | ||
| target_channel.as_ref(), | ||
| env!("CARGO_PKG_VERSION"), | ||
| ) | ||
| .await; | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Minor: Redundant check with function's internal guard.
Per context snippet 1 (update/mod.rs:867-870), maybe_send_opportunistic_update_notice already performs the same enabled && channel_visibility_enabled check internally. The outer gate here is defense-in-depth but creates duplication.
This is acceptable as-is (avoids an async call when disabled), but document the intentional redundancy or extract the check to a shared location to avoid divergence.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@clients/agent-runtime/src/channels/mod.rs` around lines 466 - 474, The outer
visibility check using update_visibility_enabled before calling
maybe_send_opportunistic_update_notice duplicates the same guard that
maybe_send_opportunistic_update_notice performs internally; either remove the
outer if and call maybe_send_opportunistic_update_notice directly (letting its
internal guard handle it) or, if you want to keep the outer short-circuit to
avoid an async call, add a concise comment above the if that references
update_visibility_enabled and maybe_send_opportunistic_update_notice and
explains the intentional redundancy to prevent future divergence.
Comment on lines
+2107
to
+2114
| fn normalize_install_method_override(raw: &str) -> Option<String> { | ||
| let normalized = raw.trim().to_ascii_lowercase(); | ||
| match normalized.as_str() { | ||
| "npm" | "pnpm" | "yarn" | "bun" | "homebrew" | "cargo" | "script_binary" => { | ||
| Some(normalized) | ||
| } | ||
| _ => None, | ||
| } |
Contributor
There was a problem hiding this comment.
Install-method normalization is inconsistent with update core parser.
normalize_install_method_override() rejects "unknown", but clients/agent-runtime/src/update/mod.rs accepts it in InstallMethod::from_str. This creates config/runtime contract drift for CORVUS_UPDATE_METHOD_OVERRIDE.
Proposed fix
fn normalize_install_method_override(raw: &str) -> Option<String> {
let normalized = raw.trim().to_ascii_lowercase();
match normalized.as_str() {
- "npm" | "pnpm" | "yarn" | "bun" | "homebrew" | "cargo" | "script_binary" => {
+ "npm" | "pnpm" | "yarn" | "bun" | "homebrew" | "cargo" | "script_binary" | "unknown" => {
Some(normalized)
}
_ => None,
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@clients/agent-runtime/src/config/schema.rs` around lines 2107 - 2114,
normalize_install_method_override rejects "unknown" but InstallMethod::from_str
in update::mod accepts it, causing a mismatch for CORVUS_UPDATE_METHOD_OVERRIDE;
update normalize_install_method_override to accept "unknown" (i.e., include
"unknown" in the match arm) so its accepted values align with
InstallMethod::from_str and the runtime/config contract is consistent. Reference
normalize_install_method_override and InstallMethod::from_str when making the
change.
Comment on lines
+486
to
+494
| #[test] | ||
| fn updater_supervision_follows_update_policy() { | ||
| let mut config = Config::default(); | ||
| config.updates.enabled = true; | ||
| assert!(updater_supervision_enabled(&config)); | ||
|
|
||
| config.updates.enabled = false; | ||
| assert!(!updater_supervision_enabled(&config)); | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find is_update_check_disabled implementation and usages
rg -n -A5 'fn is_update_check_disabled' clients/agent-runtime/src/Repository: dallay/corvus
Length of output: 544
🏁 Script executed:
#!/bin/bash
# Find updater_supervision_enabled implementation and the test
rg -n -B2 -A8 'fn updater_supervision_enabled|updater_supervision_follows_update_policy' clients/agent-runtime/src/Repository: dallay/corvus
Length of output: 1643
🏁 Script executed:
#!/bin/bash
# Check for environment variable checks related to updates/disable
rg -n 'env::|std::env|CORVUS.*UPDATE|UPDATE.*CHECK|UPDATE_CHECK' clients/agent-runtime/src/update/ clients/agent-runtime/src/daemon/Repository: dallay/corvus
Length of output: 1565
Test updater_supervision_follows_update_policy is flaky due to uncontrolled environment variable.
updater_supervision_enabled() returns config.updates.enabled && !crate::update::is_update_check_disabled(), where is_update_check_disabled() checks the CORVUS_DISABLE_UPDATE_CHECK environment variable. The test only toggles config.updates.enabled without controlling this env var, so assertions can fail nondeterministically if the variable is set during test execution.
Either mock is_update_check_disabled(), unset the env var in the test setup, or add test cases for both scenarios.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@clients/agent-runtime/src/daemon/mod.rs` around lines 486 - 494, The test
updater_supervision_follows_update_policy is flaky because
updater_supervision_enabled() depends on
crate::update::is_update_check_disabled() which reads
CORVUS_DISABLE_UPDATE_CHECK from the environment; fix the test by controlling
that env var so the function's result is deterministic: in the test
updater_supervision_follows_update_policy explicitly set or remove
CORVUS_DISABLE_UPDATE_CHECK (e.g., std::env::set_var or std::env::remove_var)
before calling updater_supervision_enabled(), test both config.updates.enabled
true/false with the env var set and unset as needed, and restore the original
env value after the test to avoid side effects. Ensure you reference
updater_supervision_enabled, is_update_check_disabled, and
config.updates.enabled when making changes.
Comment on lines
+554
to
+581
| updates: { | ||
| let status = update::get_update_status(cfg, env!("CARGO_PKG_VERSION")).ok(); | ||
| AdminUpdatesView { | ||
| enabled: cfg.updates.enabled, | ||
| auto_install_enabled: cfg.updates.auto_install_enabled, | ||
| channel_visibility_enabled: cfg.updates.channel_visibility_enabled, | ||
| cli_startup_notice_enabled: cfg.updates.cli_startup_notice_enabled, | ||
| install_method_override: cfg.updates.install_method_override.clone(), | ||
| restart_policy: cfg.updates.restart_policy.clone(), | ||
| status: AdminUpdateStatusView { | ||
| current_version: status | ||
| .as_ref() | ||
| .map(|view| view.current_version.clone()) | ||
| .unwrap_or_else(|| env!("CARGO_PKG_VERSION").to_string()), | ||
| latest_version: status.as_ref().and_then(|view| view.latest_version.clone()), | ||
| update_available: status.as_ref().is_some_and(|view| view.update_available), | ||
| last_check_at_unix: status.as_ref().and_then(|view| view.last_check_at_unix), | ||
| last_check_outcome: status | ||
| .as_ref() | ||
| .and_then(|view| view.last_check_outcome.clone()), | ||
| effective_install_method: status | ||
| .as_ref() | ||
| .map(|view| view.effective_install_method.clone()) | ||
| .unwrap_or_else(|| "unknown".to_string()), | ||
| install_method_source: status | ||
| .as_ref() | ||
| .map(|view| view.install_method_source.clone()) | ||
| .unwrap_or_else(|| "unknown".to_string()), |
Contributor
There was a problem hiding this comment.
Do not silently swallow update-status resolution failures.
Using .ok() here suppresses errors and collapses failures into "unknown"/false, which makes admin status nondiagnostic and can mislead operators.
Proposed fix (preserve explicit failure signal)
- let status = update::get_update_status(cfg, env!("CARGO_PKG_VERSION")).ok();
+ let status = update::get_update_status(cfg, env!("CARGO_PKG_VERSION"));
AdminUpdatesView {
enabled: cfg.updates.enabled,
auto_install_enabled: cfg.updates.auto_install_enabled,
channel_visibility_enabled: cfg.updates.channel_visibility_enabled,
cli_startup_notice_enabled: cfg.updates.cli_startup_notice_enabled,
install_method_override: cfg.updates.install_method_override.clone(),
restart_policy: cfg.updates.restart_policy.clone(),
status: AdminUpdateStatusView {
- current_version: status
- .as_ref()
- .map(|view| view.current_version.clone())
- .unwrap_or_else(|| env!("CARGO_PKG_VERSION").to_string()),
- latest_version: status.as_ref().and_then(|view| view.latest_version.clone()),
- update_available: status.as_ref().is_some_and(|view| view.update_available),
- last_check_at_unix: status.as_ref().and_then(|view| view.last_check_at_unix),
- last_check_outcome: status
- .as_ref()
- .and_then(|view| view.last_check_outcome.clone()),
- effective_install_method: status
- .as_ref()
- .map(|view| view.effective_install_method.clone())
- .unwrap_or_else(|| "unknown".to_string()),
- install_method_source: status
- .as_ref()
- .map(|view| view.install_method_source.clone())
- .unwrap_or_else(|| "unknown".to_string()),
+ current_version: status
+ .as_ref()
+ .map(|view| view.current_version.clone())
+ .unwrap_or_else(|_| env!("CARGO_PKG_VERSION").to_string()),
+ latest_version: status
+ .as_ref()
+ .ok()
+ .and_then(|view| view.latest_version.clone()),
+ update_available: status.as_ref().ok().is_some_and(|view| view.update_available),
+ last_check_at_unix: status.as_ref().ok().and_then(|view| view.last_check_at_unix),
+ last_check_outcome: match status.as_ref() {
+ Ok(view) => view.last_check_outcome.clone(),
+ Err(_) => Some("status_unavailable".to_string()),
+ },
+ effective_install_method: status
+ .as_ref()
+ .ok()
+ .map(|view| view.effective_install_method.clone())
+ .unwrap_or_else(|| "unknown".to_string()),
+ install_method_source: status
+ .as_ref()
+ .ok()
+ .map(|view| view.install_method_source.clone())
+ .unwrap_or_else(|| "unknown".to_string()),
},
}Based on learnings: "Preserve CLI contract unless change is intentional and documented; prefer explicit errors over silent fallback for unsupported critical paths."
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| updates: { | |
| let status = update::get_update_status(cfg, env!("CARGO_PKG_VERSION")).ok(); | |
| AdminUpdatesView { | |
| enabled: cfg.updates.enabled, | |
| auto_install_enabled: cfg.updates.auto_install_enabled, | |
| channel_visibility_enabled: cfg.updates.channel_visibility_enabled, | |
| cli_startup_notice_enabled: cfg.updates.cli_startup_notice_enabled, | |
| install_method_override: cfg.updates.install_method_override.clone(), | |
| restart_policy: cfg.updates.restart_policy.clone(), | |
| status: AdminUpdateStatusView { | |
| current_version: status | |
| .as_ref() | |
| .map(|view| view.current_version.clone()) | |
| .unwrap_or_else(|| env!("CARGO_PKG_VERSION").to_string()), | |
| latest_version: status.as_ref().and_then(|view| view.latest_version.clone()), | |
| update_available: status.as_ref().is_some_and(|view| view.update_available), | |
| last_check_at_unix: status.as_ref().and_then(|view| view.last_check_at_unix), | |
| last_check_outcome: status | |
| .as_ref() | |
| .and_then(|view| view.last_check_outcome.clone()), | |
| effective_install_method: status | |
| .as_ref() | |
| .map(|view| view.effective_install_method.clone()) | |
| .unwrap_or_else(|| "unknown".to_string()), | |
| install_method_source: status | |
| .as_ref() | |
| .map(|view| view.install_method_source.clone()) | |
| .unwrap_or_else(|| "unknown".to_string()), | |
| updates: { | |
| let status = update::get_update_status(cfg, env!("CARGO_PKG_VERSION")); | |
| AdminUpdatesView { | |
| enabled: cfg.updates.enabled, | |
| auto_install_enabled: cfg.updates.auto_install_enabled, | |
| channel_visibility_enabled: cfg.updates.channel_visibility_enabled, | |
| cli_startup_notice_enabled: cfg.updates.cli_startup_notice_enabled, | |
| install_method_override: cfg.updates.install_method_override.clone(), | |
| restart_policy: cfg.updates.restart_policy.clone(), | |
| status: AdminUpdateStatusView { | |
| current_version: status | |
| .as_ref() | |
| .map(|view| view.current_version.clone()) | |
| .unwrap_or_else(|_| env!("CARGO_PKG_VERSION").to_string()), | |
| latest_version: status | |
| .as_ref() | |
| .ok() | |
| .and_then(|view| view.latest_version.clone()), | |
| update_available: status.as_ref().ok().is_some_and(|view| view.update_available), | |
| last_check_at_unix: status.as_ref().ok().and_then(|view| view.last_check_at_unix), | |
| last_check_outcome: match status.as_ref() { | |
| Ok(view) => view.last_check_outcome.clone(), | |
| Err(_) => Some("status_unavailable".to_string()), | |
| }, | |
| effective_install_method: status | |
| .as_ref() | |
| .ok() | |
| .map(|view| view.effective_install_method.clone()) | |
| .unwrap_or_else(|| "unknown".to_string()), | |
| install_method_source: status | |
| .as_ref() | |
| .ok() | |
| .map(|view| view.install_method_source.clone()) | |
| .unwrap_or_else(|| "unknown".to_string()), | |
| }, | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@clients/agent-runtime/src/gateway/admin.rs` around lines 554 - 581, The call
to update::get_update_status currently uses .ok(), silently swallowing errors
and turning failures into "unknown"/false in
AdminUpdatesView/AdminUpdateStatusView; instead, preserve and surface failures
by handling the Result explicitly: remove .ok(), capture the Err case from
update::get_update_status(cfg, env!("CARGO_PKG_VERSION")), and propagate an
error (return Result from the handler) or populate AdminUpdateStatusView with an
explicit failure indicator and error message (e.g., set last_check_outcome or a
new status_error field) so operators can see the underlying error; update the
code paths that build AdminUpdatesView/AdminUpdateStatusView to pattern-match on
the Result from update::get_update_status and either use the Ok(view) values or
include the Err details rather than defaulting to "unknown".
Comment on lines
+442
to
+444
| let _state_lock = acquire_file_lock(&update_state_lock_path(&self.workspace_dir), 200)?; | ||
| let mut snapshot = load_state_snapshot_sync(&self.workspace_dir)? | ||
| .unwrap_or_else(|| UpdateStateSnapshot::initial(current_version, &self.policy)); |
Contributor
There was a problem hiding this comment.
Unify update-state synchronization; current lock domains can race.
Line 442 uses a file lock, while Line 724 still uses the in-process async mutex for the same state workflow. These are independent, so CLI/daemon/channel paths can concurrently mutate version_check.json and overwrite each other.
As per coding guidelines **/*.rs: Focus on Rust idioms, memory safety, and ownership/borrowing correctness. Prioritize unsafe blocks, FFI boundaries, concurrency races, and secret handling.
Also applies to: 724-725
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@clients/agent-runtime/src/update/mod.rs` around lines 442 - 444, The current
update-state flow mixes a file-based lock (acquire_file_lock &
update_state_lock_path) and an in-process async mutex, allowing races when both
CLI/daemon/channel mutate version_check.json; unify them by switching the
in-process async mutex usage to the same file lock domain: replace the async
mutex-protected critical sections with calls to
acquire_file_lock(&update_state_lock_path(&self.workspace_dir), ...) and hold
that lock across load_state_snapshot_sync, UpdateStateSnapshot::initial
creation, and any write/flush operations so all code paths serialize on the same
file lock; ensure proper error propagation and timely lock release around the
functions load_state_snapshot_sync, any snapshot mutation code, and the writer
that persists version_check.json.
Comment on lines
+15
to
+18
| function _updateField<Key extends keyof AdminConfigForm>( | ||
| key: Key, | ||
| value: AdminConfigForm[Key] | ||
| ): void { |
Contributor
There was a problem hiding this comment.
Fix handler rename regression that breaks scheduler field updates
Line 15 renamed the helper to _updateField, but Line 34, Line 44, and Line 53 still call updateField(...). That leaves the template binding unresolved and prevents model updates from input changes.
Suggested minimal fix
-function _updateField<Key extends keyof AdminConfigForm>(
+function updateField<Key extends keyof AdminConfigForm>(
key: Key,
value: AdminConfigForm[Key]
): void {As per coding guidelines, "Look for behavioral regressions, missing tests, and contract breaks across modules."
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| function _updateField<Key extends keyof AdminConfigForm>( | |
| key: Key, | |
| value: AdminConfigForm[Key] | |
| ): void { | |
| function updateField<Key extends keyof AdminConfigForm>( | |
| key: Key, | |
| value: AdminConfigForm[Key] | |
| ): void { |
🤖 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/SchedulerSettings.vue`
around lines 15 - 18, The helper was renamed to _updateField but other callers
still invoke updateField, causing template bindings to fail; update all usages
(the calls at the locations invoking updateField in this component) to call
_updateField instead or restore the original export/alias so the template can
access it (ensure the component's methods/refs expose _updateField under the
name the template expects). Specifically, change every call/site that references
updateField to use _updateField (or create a small wrapper named updateField
that delegates to _updateField) so scheduler form inputs correctly update
AdminConfigForm.
Comment on lines
+63
to
+79
| updates?: { | ||
| enabled?: boolean; | ||
| auto_install_enabled?: boolean; | ||
| channel_visibility_enabled?: boolean; | ||
| cli_startup_notice_enabled?: boolean; | ||
| install_method_override?: string | null; | ||
| restart_policy?: string; | ||
| status?: { | ||
| current_version?: string; | ||
| latest_version?: string | null; | ||
| update_available?: boolean; | ||
| last_check_at_unix?: number | null; | ||
| last_check_outcome?: string | null; | ||
| effective_install_method?: string; | ||
| install_method_source?: string; | ||
| }; | ||
| }; |
Contributor
There was a problem hiding this comment.
Align updates typing with the backend contract.
AdminConfigView.updates and updates.status are required in the Rust API view, but this TS addition makes them optional, which weakens contract checks and can hide integration regressions.
Proposed type tightening for the new update surface
- updates?: {
- enabled?: boolean;
- auto_install_enabled?: boolean;
- channel_visibility_enabled?: boolean;
- cli_startup_notice_enabled?: boolean;
- install_method_override?: string | null;
- restart_policy?: string;
- status?: {
- current_version?: string;
- latest_version?: string | null;
- update_available?: boolean;
- last_check_at_unix?: number | null;
- last_check_outcome?: string | null;
- effective_install_method?: string;
- install_method_source?: string;
- };
- };
+ updates: {
+ enabled: boolean;
+ auto_install_enabled: boolean;
+ channel_visibility_enabled: boolean;
+ cli_startup_notice_enabled: boolean;
+ install_method_override: string | null;
+ restart_policy: string;
+ status: {
+ current_version: string;
+ latest_version: string | null;
+ update_available: boolean;
+ last_check_at_unix: number | null;
+ last_check_outcome: string | null;
+ effective_install_method: string;
+ install_method_source: string;
+ };
+ };As per coding guidelines: "Look for behavioral regressions, missing tests, and contract breaks across modules."
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| updates?: { | |
| enabled?: boolean; | |
| auto_install_enabled?: boolean; | |
| channel_visibility_enabled?: boolean; | |
| cli_startup_notice_enabled?: boolean; | |
| install_method_override?: string | null; | |
| restart_policy?: string; | |
| status?: { | |
| current_version?: string; | |
| latest_version?: string | null; | |
| update_available?: boolean; | |
| last_check_at_unix?: number | null; | |
| last_check_outcome?: string | null; | |
| effective_install_method?: string; | |
| install_method_source?: string; | |
| }; | |
| }; | |
| updates: { | |
| enabled: boolean; | |
| auto_install_enabled: boolean; | |
| channel_visibility_enabled: boolean; | |
| cli_startup_notice_enabled: boolean; | |
| install_method_override: string | null; | |
| restart_policy: string; | |
| status: { | |
| current_version: string; | |
| latest_version: string | null; | |
| update_available: boolean; | |
| last_check_at_unix: number | null; | |
| last_check_outcome: string | null; | |
| effective_install_method: string; | |
| install_method_source: string; | |
| }; | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@clients/web/apps/dashboard/src/types/admin-config.ts` around lines 63 - 79,
The TypeScript typing makes the top-level updates and its nested status
optional, but the backend Rust API requires AdminConfigView.updates and
updates.status to be present; change the declaration so updates is required
(remove the ? on updates) and make status required inside that object (remove
the ? on status), leaving individual fields inside status optional as
appropriate to preserve expressiveness while restoring the contract alignment
with AdminConfigView and updates.status.
| - Verify-gap follow-up: admin update status now includes `last_check_at_unix` and `last_check_outcome` with CLI parity assertions. | ||
| - Verify-gap follow-up: installation-method detection now uses runtime heuristics (npm/pnpm/yarn/bun, homebrew, cargo, script/binary) instead of test-only override. | ||
| - Verify-gap follow-up: CLI now supports `corvus update confirm <nonce>` and appends `confirm_install` audit events. | ||
| - Dashboard check (`pnpm --filter @corvus/dashboard run check`) still reports pre-existing Biome issues in unrelated config Vue files; update-related type file check passes. |
Contributor
There was a problem hiding this comment.
Note: The "Biome issues" are likely runtime bugs.
The referenced issues in config Vue files are not just linting problems—the updateField → _updateField rename creates actual runtime errors due to template/script mismatches. These should be addressed before merge.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@openspec/changes/enhance-auto-update-system/apply-progress.md` at line 23,
The runtime errors come from an inconsistent rename of updateField to
_updateField causing template/script mismatches; locate components where
updateField/_updateField is referenced (template bindings, methods, emits,
watchers, and any imported symbols) and make them consistent: either revert to
updateField everywhere or update all template bindings and script references to
_updateField (including props/emits and any calls from parent components), then
run the Vue build to confirm templates no longer reference the old name and
update related unit/integration tests accordingly.
|
Merged
This was referenced Apr 9, 2026
Merged
Merged
Merged
9 tasks
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 a comprehensive design and specification for enhancing the auto-update system across the CLI, daemon, in-conversation channels, and client/admin surfaces. The changes provide a clear exploration of the current limitations, a detailed proposal for phased improvements, and a formal specification outlining requirements, scenarios, and acceptance criteria. The goal is to unify update visibility, enforce security and process safety, support multiple installation methods, and provide robust auditability.
Key changes are grouped as follows:
Design and Specification Documentation
enhance-auto-update-system/exploration.md. This includes analysis of update flow, configuration, process safety, and security posture.enhance-auto-update-system/proposal.mdoutlining goals, phased approach, affected areas, risks and mitigations, rollback plan, dependencies, and acceptance criteria for the enhanced update system.enhance-auto-update-system/specs/update-system/spec.mddefining requirements, user scenarios, and command contracts for update visibility, configuration, installation method detection, process safety, CLI commands, and integrity verification.Update System Improvements (as described in the proposal/spec)
UpdateManager, atomic state handling, and inter-process locking, ensuring process safety and reliable update transactions. [1] [2]corvus update status|check|install|auto-enable|auto-disable|history) and ensure deterministic, scriptable behavior with robust policy enforcement and audit logging. [1] [2]Security and Policy
These changes provide the foundation for a secure, observable, and user-friendly update experience across all supported surfaces.
Closes: DALLAY-137