feat: expand mcp platform support beyond tools only integration#336
Conversation
Add first-class MCP resource and prompt capabilities alongside existing tool support, implementing a three-tier capability model with per-server gating, unified naming, and fail-closed security posture. - Add per-server 'capabilities' config with ['tools'] default for backward compatibility - Implement McpResourceAdapter (read-only, on-demand, output-limited) - Implement McpPromptAdapter (argument validation, provenance tagging, content scanner hook for injection mitigation) - Extend ToolSourceKind with McpResource/McpPrompt variants, both mapping to ApprovalRequired - Refactor discover_tools() to discover_capabilities() with capability-gated registration and cross-type collision detection - Add normalize_resource_name/normalize_prompt_name with reserved word protection - Update parent MCP spec to reference delta spec for resources/prompts - Include SDD artifacts: exploration, proposal, spec (18 requirements, 51 scenarios), design (8 architecture decisions), tasks, verify report
Sync delta spec to main specs and move change to archive after successful verification (PASS WITH WARNINGS, all resolved).
…on enforcement The lockfile was out of sync with the forced netty 4.1.132.Final and protobuf 3.25.9 versions in locking.gradle.kts, causing configuration resolution failures.
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds first-class MCP "resources" and "prompts" capabilities: config fields and validation, capability-gated discovery, canonical naming/normalization, new McpClient surfaces, resource/prompt adapters exposing Tool semantics, policy/source-kind changes requiring operator approval, tests, and spec/design/docs for the feature. Changes
Sequence Diagram(s)sequenceDiagram
participant Agent as Agent Runtime
participant Discovery as MCP Capability Discovery
participant MCP as MCP Server
participant Registry as Tool Registry
participant Policy as Policy Evaluator
participant Adapter as Resource/Prompt Adapter
Agent->>Discovery: discover_capabilities(config)
Discovery->>Discovery: for each server: check server.capabilities
alt "tools" enabled
Discovery->>MCP: list_tools()
MCP-->>Discovery: tool manifests
Discovery->>Registry: register mcp.<srv>.<tool>
end
alt "resources" enabled
Discovery->>MCP: list_resources()
MCP-->>Discovery: resource manifests
Discovery->>Policy: classify mcp.<srv>.resource.<name>
Policy-->>Discovery: ApprovalRequired/AllowWithLimits
Discovery->>Registry: register mcp.<srv>.resource.<name>
end
alt "prompts" enabled
Discovery->>MCP: list_prompts()
MCP-->>Discovery: prompt manifests
Discovery->>Policy: classify mcp.<srv>.prompt.<name>
Policy-->>Discovery: ApprovalRequired
Discovery->>Registry: register mcp.<srv>.prompt.<name>
end
Agent->>Registry: execute mcp.<srv>.resource.<name> / prompt
Registry->>Adapter: execute(args)
Adapter->>MCP: read_resource()/get_prompt()
MCP-->>Adapter: content/messages
Adapter->>Adapter: enforce limits, add provenance, redact on error
Adapter-->>Registry: ToolResult
Registry-->>Agent: result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
✅ Contributor ReportUser: @yacosta738
Contributor Report evaluates based on public GitHub activity. Analysis period: 2025-03-27 to 2026-03-27 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
clients/agent-runtime/src/tools/mcp/normalize.rs (1)
61-64:⚠️ Potential issue | 🟠 MajorDon't reserve
resource/promptas server names.Because
validate_identifier()is used for the server segment too, a pre-existing tool-only config likename = "prompt"now failsnormalize_tool_name()and drops every MCP tool from that server. The server segment is positional already, so this adds a backward-compat break without preventing a real canonical-ID collision.As per coding guidelines:
**/*: Look for behavioral regressions, missing tests, and contract breaks across modules.Also applies to: 106-115
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/agent-runtime/src/tools/mcp/normalize.rs` around lines 61 - 64, The server segment must not be run through validate_identifier (which forbids reserved names like "resource"/"prompt"); update normalize_tool_name to stop calling validate_identifier for the server: only validate the tool segment via validate_identifier("tool", tool_name) and either remove server validation or replace it with a minimal check (non-empty, no dots/slashes) that does not treat reserved words as invalid; apply the same change to the other MCP normalization function(s) that currently validate the server segment the same way (the block around the later similar function).
🤖 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/tools/mcp/client.rs`:
- Around line 66-74: The current list_resources method silently returns
Ok(Vec::new()) for non-mock servers; change it to return an explicit error
instead so callers know the capability is unsupported for live MCP servers.
Update list_resources (and the other similar methods referenced around lines
76-88, 90-98, 100-119) to match the mock-vs-live pattern: when
self.server.command != "__mcp_mock__", emit a tracing::debug/info note and
return Err(anyhow::anyhow!("MCP resource discovery not supported for live
server: {}", self.server.name)) (or a similarly phrased anyhow error) rather
than Ok(empty), so the CLI contract surfaces an explicit unsupported-capability
error. Ensure you reference the existing list_resources_from_mock_payload and
analogous mock helper functions for the mock branch.
In `@clients/agent-runtime/src/tools/mcp/mod.rs`:
- Around line 190-207: discover_capabilities() currently boxes the
McpPromptAdapter returned by McpPromptAdapter::from_manifest(server, manifest,
client.clone()) without attaching a ContentScanner, so every adapter is created
with content_scanner: None; update the adapter construction path in
discover_capabilities (the match branch that handles Ok(adapter)) to obtain or
create the ContentScanner instance (or accept a scanner param) and set it on the
adapter before pushing it into tools (i.e., ensure the adapter’s content_scanner
field is populated from the runtime scanner provider rather than left None), so
discovered prompts run with a real ContentScanner instead of test-only behavior.
In `@clients/agent-runtime/src/tools/mcp/prompt_adapter.rs`:
- Around line 89-150: validate_arguments currently only checks presence and
unknown keys but must also enforce the parameter schema (string-only) so nulls,
objects, arrays, numbers, and booleans are rejected; update validate_arguments
to iterate self.arguments and for each expected argument (by name) ensure the
provided serde_json::Value.is_string() (or treat null as missing only when not
required), collect any mismatched-type names into an error similar to the
existing missing/unknown checks, and when returning Ok produce an Object
containing only the validated string values (convert to Value::String by cloning
the original string). Reference validate_arguments, parameters_schema,
get_prompt and the Tool trait in your changes so the runtime never forwards
arbitrary JSON into get_prompt and returns a structured ToolResult on type
validation failures.
In `@clients/agent-runtime/src/tools/mcp/resource_adapter.rs`:
- Around line 29-37: The MCP resource metadata is using the tool's URI instead
of the resource name for source.original_name: update code in
resource_adapter.rs so source_metadata_resource() and spec() populate
source.original_name with the manifest/canonical name (e.g., use manifest.name
or the local canonical_name field) rather than self.uri; make the same change in
the other occurrence around the spec() call (the block referenced at ~138-147)
so MCP metadata aligns with the helper contract and the tool/prompt adapters.
In `@clients/composeApp/buildscript-gradle.lockfile`:
- Around line 119-142: The lockfile shows mixed Kotlin versions: ensure all
stdlib and reflect artifacts match the declared catalog version (kotlin =
"2.3.20") by updating the entries for org.jetbrains.kotlin:kotlin-stdlib-jdk7,
org.jetbrains.kotlin:kotlin-stdlib-jdk8, org.jetbrains.kotlin:kotlin-stdlib and
org.jetbrains.kotlin:kotlin-reflect to 2.3.20 (or align the catalog if a
different version is intended), then regenerate the Gradle lockfile (e.g.,
refresh dependencies or run the build that writes the lockfile) so
org.jetbrains.kotlin:* artifacts are consistent on the classpath and avoid
bytecode incompatibilities.
In `@gradle/build-logic/gradle.lockfile`:
- Around line 6-13: The gradle.lockfile contains mixed Spotless versions (e.g.,
com.diffplug.spotless:com.diffplug.spotless.gradle.plugin locked at 8.2.1 for
testCompileClasspath and 8.4.0 for compileClasspath, likewise
spotless-lib/spotless-lib-extra/spotless-plugin-gradle), so regenerate the
lockfile to sync versions: run ./gradlew writeLocksAll to rewrite
gradle.lockfile, verify Spotless artifacts
(com.diffplug.spotless:com.diffplug.spotless.gradle.plugin,
com.diffplug.spotless:spotless-lib, com.diffplug.spotless:spotless-lib-extra,
com.diffplug.spotless:spotless-plugin-gradle) now have a single consistent
version, then commit the updated gradle.lockfile.
- Around line 123-130: The gradle lockfile contains mixed Kover versions
(entries for
org.jetbrains.kotlinx.kover:org.jetbrains.kotlinx.kover.gradle.plugin and
org.jetbrains.kotlinx:kover-gradle-plugin showing 0.9.7 and 0.9.8); fix by
reconciling to a single version (prefer 0.9.8) across all scopes: either
regenerate the Gradle dependency lockfiles with the project's lockfile
generation task or manually update all Kover entries in the lockfile so every
occurrence of
org.jetbrains.kotlinx.kover:org.jetbrains.kotlinx.kover.gradle.plugin and
org.jetbrains.kotlinx:kover-gradle-plugin uses the same version (0.9.8), then
commit the updated lockfile.
In
`@openspec/changes/archive/2026-03-27-mcp-platform-capabilities/specs/mcp-platform-capabilities/spec.md`:
- Around line 402-407: Update the scenario text "Scenario: Prompt with required
parameters is registered with schema" to accurately reflect the implementation:
remove claims of typing support and type preservation and state that the runtime
models prompt arguments only with name/description/required and that the
generated schema hard-codes each argument as type: "string"; ensure references
to argument fields use the actual modeled symbols ('name', 'description',
'required') and change phrases like "typed required arguments" / "types
preserved" to "required arguments (represented as strings) and their
names/descriptions/required flags are preserved".
- Around line 313-319: The archived spec text is out of sync with runtime
behavior: replace references to resource_limits.output_limit_bytes with the
implemented resource_output_limit_bytes field and change the documented MCP
resource default from AllowWithLimits to ApprovalRequired; update the scenario
example (the resource-specific field used in the GIVEN/AND/THEN lines) to use
resource_output_limit_bytes = 131072 and ensure the server-level default
description and any mentions of output_limit_bytes reflect the new field and
default behavior; make the same edits in the corresponding archived section
noted (the block around the later occurrence) so the archive report/verdict
matches the code.
In `@openspec/changes/archive/2026-03-27-mcp-platform-capabilities/tasks.md`:
- Around line 219-227: Add an explicit rollback note to Phase 3 (Integration)
describing a quick rollback strategy and tests for dispatcher/gateway
regressions: update the Phase 3 checklist in
openspec/changes/archive/2026-03-27-mcp-platform-capabilities/tasks.md to
include a new bullet (e.g., "Rollback & Risk") that specifies steps to revert
gateway/dispatcher changes (rollback deployment, revert configuration, disable
new webhook paths), call out verification targets (src/agent/dispatcher.rs and
gateway `/webhook` integration points), and add acceptance tests for boundary
checks and failure modes to validate the rollback path.
In
`@openspec/changes/archive/2026-03-27-mcp-platform-capabilities/verify-report.md`:
- Line 39: The report is out of sync with the current discovery code: update any
statements that say resource adapters are not pushed into the unified registry
to reflect that discover_capabilities() now pushes resource adapters into the
tools vector (the unified registry), and accordingly remove or adjust the
PARTIAL result, the warning list entries, and the `47/51` compliance summary;
search for references to discover_capabilities(), tools (or tools vec), and the
comment at mod.rs:142 to locate affected sections (including lines noted: 74-75,
141-144, 229-230) and edit the narrative and summaries so they accurately state
that resources are validated, collision-checked, and registered as callable
tools.
In `@openspec/specs/mcp-platform-capabilities/spec.md`:
- Around line 313-319: Update the spec's resource section to match the runtime
changes: replace the nested resource_limits.output_limit_bytes references with
the flat resource_output_limit_bytes field, change any default or enum mentions
of AllowWithLimits to reflect the runtime behavior, and update the resource
approval classification from AllowWithLimits to ApprovalRequired for
McpResource; ensure the "Resource-specific output limit overrides server
default" scenario (and the similar block at 348-360) uses
resource_output_limit_bytes and states that McpResource is ApprovalRequired so
the documented config shape and approval requirements match the implementation.
- Around line 402-407: The spec promises typed prompt arguments and type
preservation, but the implementation (PromptArgument struct and
McpPromptAdapter::parameters_schema()) only emits name, description, required
and currently sets type: "string" for every argument; update the spec language
to reflect current behavior or implement full typing—either (A) change the spec
text in this scenario to state that only name, description, and required flags
are preserved and that all arguments are emitted as strings by
McpPromptAdapter::parameters_schema(), or (B) implement typed argument support
by extending PromptArgument to include a type field and updating
McpPromptAdapter::parameters_schema() to emit the actual types instead of always
"string" so the spec’s claims are accurate.
---
Outside diff comments:
In `@clients/agent-runtime/src/tools/mcp/normalize.rs`:
- Around line 61-64: The server segment must not be run through
validate_identifier (which forbids reserved names like "resource"/"prompt");
update normalize_tool_name to stop calling validate_identifier for the server:
only validate the tool segment via validate_identifier("tool", tool_name) and
either remove server validation or replace it with a minimal check (non-empty,
no dots/slashes) that does not treat reserved words as invalid; apply the same
change to the other MCP normalization function(s) that currently validate the
server segment the same way (the block around the later similar function).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: f3f6cb93-6b82-40fc-b722-381c2649f475
⛔ Files ignored due to path filters (2)
clients/agent-runtime/Cargo.lockis excluded by!**/*.lockclients/web/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (38)
clients/agent-runtime/Cargo.tomlclients/agent-runtime/src/agent/dispatcher.rsclients/agent-runtime/src/config/mod.rsclients/agent-runtime/src/config/schema.rsclients/agent-runtime/src/security/policy.rsclients/agent-runtime/src/test_support.rsclients/agent-runtime/src/tools/mcp/cerebro.rsclients/agent-runtime/src/tools/mcp/client.rsclients/agent-runtime/src/tools/mcp/mod.rsclients/agent-runtime/src/tools/mcp/normalize.rsclients/agent-runtime/src/tools/mcp/prompt_adapter.rsclients/agent-runtime/src/tools/mcp/resource_adapter.rsclients/agent-runtime/tests/mcp_config_validation.rsclients/agent-runtime/tests/mcp_cross_capability_collision.rsclients/agent-runtime/tests/mcp_execution_limits.rsclients/agent-runtime/tests/mcp_native_regression.rsclients/agent-runtime/tests/mcp_policy_approval_parity.rsclients/agent-runtime/tests/mcp_registry_integration.rsclients/agent-runtime/tests/mcp_runtime_e2e.rsclients/androidApp/buildscript-gradle.lockfileclients/composeApp/buildscript-gradle.lockfileclients/composeApp/gradle.lockfileclients/web/pnpm-workspace.yamlgradle/aggregation/gradle.lockfilegradle/build-logic/gradle.lockfilegradle/build-logic/src/main/kotlin/com.profiletailors.check.locking.gradle.ktsgradle/libs.versions.tomlgradle/versions/gradle.lockfilemodules/agent-core-kmp/gradle.lockfileopenspec/changes/archive/2026-03-27-mcp-platform-capabilities/archive-report.mdopenspec/changes/archive/2026-03-27-mcp-platform-capabilities/design.mdopenspec/changes/archive/2026-03-27-mcp-platform-capabilities/exploration.mdopenspec/changes/archive/2026-03-27-mcp-platform-capabilities/proposal.mdopenspec/changes/archive/2026-03-27-mcp-platform-capabilities/specs/mcp-platform-capabilities/spec.mdopenspec/changes/archive/2026-03-27-mcp-platform-capabilities/tasks.mdopenspec/changes/archive/2026-03-27-mcp-platform-capabilities/verify-report.mdopenspec/specs/mcp-platform-capabilities/spec.mdopenspec/specs/mcp-runtime/spec.md
👮 Files not reviewed due to content moderation or server errors (4)
- clients/agent-runtime/src/config/schema.rs
- openspec/changes/archive/2026-03-27-mcp-platform-capabilities/proposal.md
- clients/agent-runtime/tests/mcp_cross_capability_collision.rs
- openspec/changes/archive/2026-03-27-mcp-platform-capabilities/design.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: core-checks
- GitHub Check: sonar
- GitHub Check: Cloudflare Pages
🧰 Additional context used
📓 Path-based instructions (10)
clients/agent-runtime/src/tools/**/*.rs
📄 CodeRabbit inference engine (clients/agent-runtime/AGENTS.md)
Implement
Tooltrait insrc/tools/with strict parameter schema, validate and sanitize all inputs, and return structuredToolResultwithout panics in runtime path
Files:
clients/agent-runtime/src/tools/mcp/cerebro.rsclients/agent-runtime/src/tools/mcp/mod.rsclients/agent-runtime/src/tools/mcp/prompt_adapter.rsclients/agent-runtime/src/tools/mcp/client.rsclients/agent-runtime/src/tools/mcp/normalize.rsclients/agent-runtime/src/tools/mcp/resource_adapter.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/tools/mcp/cerebro.rsclients/agent-runtime/src/security/policy.rsclients/agent-runtime/src/tools/mcp/mod.rsclients/agent-runtime/src/tools/mcp/prompt_adapter.rsclients/agent-runtime/src/tools/mcp/client.rsclients/agent-runtime/src/tools/mcp/normalize.rsclients/agent-runtime/src/tools/mcp/resource_adapter.rs
clients/agent-runtime/src/**/*.rs
📄 CodeRabbit inference engine (clients/agent-runtime/AGENTS.md)
clients/agent-runtime/src/**/*.rs: Never log secrets, tokens, raw credentials, or sensitive payloads in any logging statements
Avoid unnecessary allocations, clones, and blocking operations to maintain performance and efficiency
Files:
clients/agent-runtime/src/tools/mcp/cerebro.rsclients/agent-runtime/src/test_support.rsclients/agent-runtime/src/agent/dispatcher.rsclients/agent-runtime/src/config/mod.rsclients/agent-runtime/src/security/policy.rsclients/agent-runtime/src/config/schema.rsclients/agent-runtime/src/tools/mcp/mod.rsclients/agent-runtime/src/tools/mcp/prompt_adapter.rsclients/agent-runtime/src/tools/mcp/client.rsclients/agent-runtime/src/tools/mcp/normalize.rsclients/agent-runtime/src/tools/mcp/resource_adapter.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/tools/mcp/cerebro.rsclients/agent-runtime/src/test_support.rsclients/agent-runtime/tests/mcp_config_validation.rsclients/agent-runtime/src/agent/dispatcher.rsclients/agent-runtime/tests/mcp_execution_limits.rsclients/agent-runtime/tests/mcp_native_regression.rsclients/agent-runtime/src/config/mod.rsclients/agent-runtime/tests/mcp_runtime_e2e.rsclients/agent-runtime/tests/mcp_policy_approval_parity.rsclients/agent-runtime/tests/mcp_registry_integration.rsclients/agent-runtime/src/security/policy.rsclients/agent-runtime/tests/mcp_cross_capability_collision.rsclients/agent-runtime/src/config/schema.rsclients/agent-runtime/src/tools/mcp/mod.rsclients/agent-runtime/src/tools/mcp/prompt_adapter.rsclients/agent-runtime/src/tools/mcp/client.rsclients/agent-runtime/src/tools/mcp/normalize.rsclients/agent-runtime/src/tools/mcp/resource_adapter.rs
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/tools/mcp/cerebro.rsclients/agent-runtime/src/config/mod.rsclients/agent-runtime/src/security/policy.rsclients/agent-runtime/src/config/schema.rsclients/agent-runtime/src/tools/mcp/mod.rsclients/agent-runtime/src/tools/mcp/prompt_adapter.rsclients/agent-runtime/src/tools/mcp/client.rsclients/agent-runtime/src/tools/mcp/normalize.rsclients/agent-runtime/src/tools/mcp/resource_adapter.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/tools/mcp/cerebro.rsclients/agent-runtime/src/test_support.rsclients/agent-runtime/tests/mcp_config_validation.rsclients/agent-runtime/src/agent/dispatcher.rsclients/agent-runtime/tests/mcp_execution_limits.rsclients/agent-runtime/tests/mcp_native_regression.rsclients/agent-runtime/src/config/mod.rsclients/agent-runtime/tests/mcp_runtime_e2e.rsclients/agent-runtime/tests/mcp_policy_approval_parity.rsclients/agent-runtime/tests/mcp_registry_integration.rsclients/agent-runtime/src/security/policy.rsclients/agent-runtime/tests/mcp_cross_capability_collision.rsclients/agent-runtime/src/config/schema.rsclients/agent-runtime/src/tools/mcp/mod.rsclients/agent-runtime/src/tools/mcp/prompt_adapter.rsclients/agent-runtime/src/tools/mcp/client.rsclients/agent-runtime/src/tools/mcp/normalize.rsclients/agent-runtime/src/tools/mcp/resource_adapter.rs
**/*
⚙️ CodeRabbit configuration file
**/*: Security first, performance second.
Validate input boundaries, auth/authz implications, and secret management.
Look for behavioral regressions, missing tests, and contract breaks across modules.
Files:
clients/agent-runtime/src/tools/mcp/cerebro.rsmodules/agent-core-kmp/gradle.lockfileclients/agent-runtime/src/test_support.rsclients/agent-runtime/tests/mcp_config_validation.rsclients/agent-runtime/src/agent/dispatcher.rsclients/agent-runtime/Cargo.tomlclients/agent-runtime/tests/mcp_execution_limits.rsclients/agent-runtime/tests/mcp_native_regression.rsclients/agent-runtime/src/config/mod.rsclients/agent-runtime/tests/mcp_runtime_e2e.rsgradle/aggregation/gradle.lockfileopenspec/changes/archive/2026-03-27-mcp-platform-capabilities/archive-report.mdgradle/versions/gradle.lockfileclients/androidApp/buildscript-gradle.lockfilegradle/build-logic/src/main/kotlin/com.profiletailors.check.locking.gradle.ktsopenspec/specs/mcp-runtime/spec.mdopenspec/changes/archive/2026-03-27-mcp-platform-capabilities/exploration.mdclients/composeApp/buildscript-gradle.lockfilegradle/build-logic/gradle.lockfileclients/composeApp/gradle.lockfileopenspec/changes/archive/2026-03-27-mcp-platform-capabilities/proposal.mdclients/agent-runtime/tests/mcp_policy_approval_parity.rsclients/agent-runtime/tests/mcp_registry_integration.rsopenspec/changes/archive/2026-03-27-mcp-platform-capabilities/verify-report.mdopenspec/changes/archive/2026-03-27-mcp-platform-capabilities/design.mdgradle/libs.versions.tomlclients/agent-runtime/src/security/policy.rsclients/agent-runtime/tests/mcp_cross_capability_collision.rsopenspec/specs/mcp-platform-capabilities/spec.mdclients/agent-runtime/src/config/schema.rsclients/web/pnpm-workspace.yamlclients/agent-runtime/src/tools/mcp/mod.rsopenspec/changes/archive/2026-03-27-mcp-platform-capabilities/tasks.mdclients/agent-runtime/src/tools/mcp/prompt_adapter.rsclients/agent-runtime/src/tools/mcp/client.rsclients/agent-runtime/src/tools/mcp/normalize.rsclients/agent-runtime/src/tools/mcp/resource_adapter.rsopenspec/changes/archive/2026-03-27-mcp-platform-capabilities/specs/mcp-platform-capabilities/spec.md
clients/agent-runtime/**/Cargo.toml
📄 CodeRabbit inference engine (clients/agent-runtime/AGENTS.md)
clients/agent-runtime/**/Cargo.toml: Preserve release-size profile assumptions inCargo.tomland avoid adding heavy dependencies unless clearly justified
Do not add heavy dependencies for minor convenience; justify new crate additions
Files:
clients/agent-runtime/Cargo.toml
**/*.{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-27-mcp-platform-capabilities/archive-report.mdopenspec/specs/mcp-runtime/spec.mdopenspec/changes/archive/2026-03-27-mcp-platform-capabilities/exploration.mdopenspec/changes/archive/2026-03-27-mcp-platform-capabilities/proposal.mdopenspec/changes/archive/2026-03-27-mcp-platform-capabilities/verify-report.mdopenspec/changes/archive/2026-03-27-mcp-platform-capabilities/design.mdopenspec/specs/mcp-platform-capabilities/spec.mdopenspec/changes/archive/2026-03-27-mcp-platform-capabilities/tasks.mdopenspec/changes/archive/2026-03-27-mcp-platform-capabilities/specs/mcp-platform-capabilities/spec.md
**/*.gradle.kts
⚙️ CodeRabbit configuration file
**/*.gradle.kts: Prefer tasks.register/configureEach, avoid afterEvaluate, and preserve configuration cache.
Ensure dependencies come from version catalogs and avoid eager task realization.
Review plugin/config changes for supply-chain and reproducibility risks.
Files:
gradle/build-logic/src/main/kotlin/com.profiletailors.check.locking.gradle.kts
🧠 Learnings (9)
📚 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/test_support.rsclients/agent-runtime/src/agent/dispatcher.rsclients/agent-runtime/tests/mcp_execution_limits.rsclients/agent-runtime/src/config/mod.rsclients/agent-runtime/tests/mcp_runtime_e2e.rsclients/agent-runtime/tests/mcp_policy_approval_parity.rsclients/agent-runtime/tests/mcp_registry_integration.rsclients/agent-runtime/src/security/policy.rsclients/agent-runtime/tests/mcp_cross_capability_collision.rsclients/agent-runtime/src/config/schema.rsclients/agent-runtime/src/tools/mcp/mod.rsclients/agent-runtime/src/tools/mcp/prompt_adapter.rsclients/agent-runtime/src/tools/mcp/client.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/src/agent/dispatcher.rsopenspec/specs/mcp-runtime/spec.mdclients/agent-runtime/src/security/policy.rsclients/agent-runtime/tests/mcp_cross_capability_collision.rsclients/agent-runtime/src/tools/mcp/mod.rsclients/agent-runtime/src/tools/mcp/prompt_adapter.rsclients/agent-runtime/src/tools/mcp/client.rsclients/agent-runtime/src/tools/mcp/normalize.rsclients/agent-runtime/src/tools/mcp/resource_adapter.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/src/agent/dispatcher.rsclients/agent-runtime/tests/mcp_policy_approval_parity.rsclients/agent-runtime/src/security/policy.rsclients/agent-runtime/tests/mcp_cross_capability_collision.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/src/agent/dispatcher.rsclients/agent-runtime/src/config/mod.rsclients/agent-runtime/tests/mcp_runtime_e2e.rsclients/agent-runtime/tests/mcp_policy_approval_parity.rsclients/agent-runtime/tests/mcp_registry_integration.rsclients/agent-runtime/src/security/policy.rsclients/agent-runtime/tests/mcp_cross_capability_collision.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/Cargo.tomlclients/agent-runtime/tests/mcp_cross_capability_collision.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/Cargo.toml
📚 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:
clients/agent-runtime/Cargo.tomlclients/agent-runtime/tests/mcp_runtime_e2e.rsclients/agent-runtime/tests/mcp_policy_approval_parity.rsclients/agent-runtime/tests/mcp_cross_capability_collision.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/mcp_cross_capability_collision.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: 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-27-mcp-platform-capabilities/tasks.md
🪛 GitHub Actions: PR Checks Build Logic
gradle/build-logic/gradle.lockfile
[error] 1-1: Task ':checkLocks' failed: gradle/build-logic/gradle.lockfile changed; run './gradlew writeLocksAll' and commit the updated lockfile.
🪛 LanguageTool
openspec/changes/archive/2026-03-27-mcp-platform-capabilities/archive-report.md
[style] ~32-~32: ‘prior to’ might be wordy. Consider a shorter alternative.
Context: ...WITH WARNINGS** — all warnings resolved prior to archive: - 47/51 spec scenarios COMPLIA...
(EN_WORDINESS_PREMIUM_PRIOR_TO)
openspec/changes/archive/2026-03-27-mcp-platform-capabilities/exploration.md
[style] ~125-~125: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ...nspec/specs/mcp-runtime/spec.md` — Spec needs to be extended or a new spec created --- ...
(REP_NEED_TO_VB)
openspec/changes/archive/2026-03-27-mcp-platform-capabilities/proposal.md
[grammar] ~68-~68: Ensure spelling is correct
Context: ...thers don't. ### Q5: Transport scope → Stdio-only initially Resources and prompts f...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
openspec/changes/archive/2026-03-27-mcp-platform-capabilities/verify-report.md
[style] ~116-~116: The double modal “required rejected” is nonstandard (only accepted in certain dialects). Consider “to be rejected”.
Context: ...rameter Validation** | Missing required rejected | `prompt_adapter.rs::execute_rejects_m...
(NEEDS_FIXED)
🪛 markdownlint-cli2 (0.22.0)
openspec/changes/archive/2026-03-27-mcp-platform-capabilities/proposal.md
[warning] 86-86: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
openspec/changes/archive/2026-03-27-mcp-platform-capabilities/verify-report.md
[warning] 198-198: Ordered list item prefix
Expected: 1; Actual: 4; Style: 1/2/3
(MD029, ol-prefix)
[warning] 199-199: Ordered list item prefix
Expected: 2; Actual: 5; Style: 1/2/3
(MD029, ol-prefix)
[warning] 203-203: Ordered list item prefix
Expected: 1; Actual: 6; Style: 1/2/3
(MD029, ol-prefix)
[warning] 204-204: Ordered list item prefix
Expected: 2; Actual: 7; Style: 1/2/3
(MD029, ol-prefix)
[warning] 205-205: Ordered list item prefix
Expected: 3; Actual: 8; Style: 1/2/3
(MD029, ol-prefix)
[warning] 209-209: Ordered list item prefix
Expected: 1; Actual: 9; Style: 1/1/1
(MD029, ol-prefix)
openspec/changes/archive/2026-03-27-mcp-platform-capabilities/design.md
[warning] 34-34: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 140-140: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 268-268: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 313-313: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 369-369: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (36)
clients/web/pnpm-workspace.yaml (1)
8-8: These package versions are valid and safe with no security vulnerabilities or breaking changes.All seven packages exist on npm and have only patch-level updates with bug fixes and minor improvements—no security advisories or behavioral regressions.
While these web client dependency updates are tangential to the PR's MCP platform expansion, the updates themselves pose no risk and can proceed.
clients/agent-runtime/src/agent/dispatcher.rs (1)
68-70: Approval gating correctly includes MCP resources and prompts.Good update: this keeps new MCP capability types fail-closed in dispatcher risk evaluation.
clients/agent-runtime/src/test_support.rs (1)
28-30: Test fixture defaulting is aligned and backward-compatible.Using the shared default capabilities helper here reduces drift across MCP tests.
clients/agent-runtime/tests/mcp_config_validation.rs (1)
14-16: Validation baseline update looks correct.This keeps
valid_server()explicit about MCP capability defaults and avoids accidental field omission regressions.clients/agent-runtime/src/tools/mcp/cerebro.rs (1)
49-51: Cerebro server config keeps secure MCP defaults.Good to explicitly carry the new fields while retaining tools-only default capability behavior.
clients/agent-runtime/tests/mcp_native_regression.rs (1)
47-49: Both MCP test fixtures were updated consistently.These additions keep regression tests aligned with the expanded
McpServerConfigwithout changing their behavioral assertions.Also applies to: 107-109
clients/agent-runtime/src/config/mod.rs (1)
5-10: Config re-export update is a good API consistency improvement.Exposing
default_mcp_capabilitiesthroughconfigkeeps downstream construction paths uniform.clients/agent-runtime/Cargo.toml (1)
139-139: No action needed. Verification confirms the patch bump resolves cleanly without introducing duplicate crates or binary bloat. Version mismatches at the patch level (0.31.0 vs 0.31.1) across the OpenTelemetry family are normal and expected in Rust dependency resolution.clients/agent-runtime/tests/mcp_runtime_e2e.rs (1)
18-20: LGTM!New
McpServerConfigfields are correctly initialized with backward-compatible defaults (["tools"]capability,Nonefor per-capability limits). Aligns with schema changes inconfig/schema.rs.clients/agent-runtime/tests/mcp_execution_limits.rs (1)
17-19: LGTM!Consistent with other test helper updates — new
McpServerConfigfields initialized with correct defaults.openspec/specs/mcp-runtime/spec.md (2)
9-12: LGTM!Scope statement correctly updated to direct resource/prompt handling to the platform capabilities delta spec while maintaining the existing hot reload exclusion.
224-234: LGTM!New scenario correctly specifies capability-gating behavior: runtime ignores advertised capabilities not listed in server's
capabilitiesconfig. Cross-reference to the delta spec provides clear traceability for the three-tier model.openspec/changes/archive/2026-03-27-mcp-platform-capabilities/archive-report.md (1)
1-42: LGTM!Archive report accurately documents the SDD cycle completion with honest verdict ("PASS WITH WARNINGS" for 4 partial timeout tests) and transparent task tracking. Security audit checklist items align with PR objectives.
clients/agent-runtime/src/security/policy.rs (4)
38-39: LGTM!New
ToolSourceKindvariants for MCP resources and prompts enable fine-grained capability classification while maintaining the existingMcpvariant for tools.
57-64: Detection logic is secure and deterministic.The
.contains(".resource.")/.contains(".prompt.")checks correctly identify capability types from canonical names likemcp.server.resource.api-spec. The ordering (resource before prompt) provides deterministic classification for edge cases.
241-244: LGTM — deny-by-default preserved.All MCP capability types (
Mcp,McpResource,McpPrompt) require explicit approval, maintaining the secure-by-default policy mandated by the spec. As per coding guidelines: "Do not silently weaken security policy or access constraints."
1340-1382: LGTM!Comprehensive test coverage for the new detection logic and policy evaluation. Tests verify:
McpResourcedetection via.resource.segmentMcpPromptdetection via.prompt.segment- Regular MCP tools unchanged
- Both new types require approval
clients/agent-runtime/tests/mcp_policy_approval_parity.rs (2)
1-5: LGTM!Imports correctly updated to include
evaluate_tool_risk_for_originandExecutionOriginfor parity testing.
43-88: Excellent parity coverage.These tests verify that MCP resources and prompts require approval in the dispatcher and confirm entry-point parity across
StandardandMissionorigins. This directly validates the spec requirement: "no entry point MAY bypass MCP approval gates."clients/agent-runtime/tests/mcp_registry_integration.rs (5)
1-1: LGTM!Import of
default_mcp_capabilitiesand explicit field initialization inmock_serveralign with the new schema structure.Also applies to: 16-18
124-125: Assertion updated to match new error format.The collision error message now uses "canonical id is unique" (without "tool") to accommodate the unified format
mcp.<server>.<type>.<name>for all capability types. This matches the implementation intools/mcp/mod.rs(context snippet 3).
127-161: Strong backward-compatibility regression test.Verifies that
default_mcp_capabilities()produces tools-only discovery even when servers advertise resources and prompts. This ensures existing integrations work unchanged.
163-204: LGTM — critical regression coverage.Confirms that adding
"resources"to capabilities doesn't alter the existing tool's identity, description, or source metadata. The v1 vs v2 comparison approach is thorough.
206-242: LGTM!End-to-end verification that tool discovery characteristics (naming pattern, spec metadata, policy classification) remain unchanged with default capabilities. The
ApprovalRequiredassertion confirms security posture is preserved.openspec/changes/archive/2026-03-27-mcp-platform-capabilities/exploration.md (1)
1-203: LGTM — thorough exploration document.The analysis correctly identifies:
- Prompts are the highest-risk capability because they directly inject content into the LLM conversation (line 86)
- Three-tier capability model with extended naming (
mcp.<server>.resource.<name>)- Resources-first implementation ordering given the lower risk profile
Technical accuracy is solid — the current state description and architecture mapping align with the codebase.
gradle/libs.versions.toml (3)
10-10: Version catalog updates are consistent with lockfile changes.Kotlin, Spotless, and Kover version bumps align with corresponding lockfile updates across the repository.
Also applies to: 71-71, 87-87
24-25: Netty and Protobuf versions align with enforced safe versions.These match the
safeNettyVersionandsafeProtobufVersionin the locking script, ensuring consistency.
22-22: JUnit 5.14.3 is the latest stable version in the JUnit 5 series; no action needed.modules/agent-core-kmp/gradle.lockfile (1)
30-30: Kover agent version update is consistent.Aligns with the
org.jetbrains.kotlinx.koverversion bump to0.9.8in the version catalog.clients/composeApp/gradle.lockfile (1)
292-292: LGTM.Kover agent bump to 0.9.8 is consistent with version catalog.
clients/androidApp/buildscript-gradle.lockfile (1)
62-63: Security dependency updates correctly applied.Protobuf
3.25.9and Netty4.1.132.Finalmatch the enforced safe versions, addressing the native SslHandler crash and parser recursion DoS vulnerabilities.Also applies to: 83-93
gradle/aggregation/gradle.lockfile (1)
4-5: BOM updates aligned with enforced safe versions.gradle/versions/gradle.lockfile (1)
4-5: Versions lockfile consistent with other updates.gradle/build-logic/src/main/kotlin/com.profiletailors.check.locking.gradle.kts (1)
8-9: Security version enforcements are appropriate.Netty 4.1.132.Final is confirmed to address multiple SslHandler security vulnerabilities (CERT-Bund advisory WID-SEC-2026-0846, CVSS 7.5). Protobuf 3.25.9 includes the parser recursion DoS fix from CVE-2024-7254. Both versions correctly enforce the stated security mitigations.
clients/composeApp/buildscript-gradle.lockfile (2)
62-63: Good security-forward lock bump for Protobuf.Patch upgrade looks appropriate for reducing dependency risk on the build classpath.
As per coding guidelines, "Security first, performance second."
83-93: Netty module set is upgraded consistently.All changed Netty modules move together to the same patch level, which is the right pattern for classpath stability.
As per coding guidelines, "Security first, performance second."
| pub fn list_resources(&self) -> anyhow::Result<Vec<McpResourceManifest>> { | ||
| match self.server.command.as_str() { | ||
| "__mcp_mock__" => self.list_resources_from_mock_payload(), | ||
| _ => { | ||
| tracing::debug!(server = %self.server.name, "list_resources not yet implemented for live servers"); | ||
| Ok(Vec::new()) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Live MCP resources/prompts are still unsupported.
For non-mock servers, discovery returns Ok(Vec::new()) while execution hard-fails. That lets operators enable resources/prompts in config and get a silent no-op at startup instead of an explicit unsupported-capability error.
Based on learnings: Preserve CLI contract unless change is intentional and documented; prefer explicit errors over silent fallback for unsupported critical paths.
Also applies to: 76-88, 90-98, 100-119
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@clients/agent-runtime/src/tools/mcp/client.rs` around lines 66 - 74, The
current list_resources method silently returns Ok(Vec::new()) for non-mock
servers; change it to return an explicit error instead so callers know the
capability is unsupported for live MCP servers. Update list_resources (and the
other similar methods referenced around lines 76-88, 90-98, 100-119) to match
the mock-vs-live pattern: when self.server.command != "__mcp_mock__", emit a
tracing::debug/info note and return Err(anyhow::anyhow!("MCP resource discovery
not supported for live server: {}", self.server.name)) (or a similarly phrased
anyhow error) rather than Ok(empty), so the CLI contract surfaces an explicit
unsupported-capability error. Ensure you reference the existing
list_resources_from_mock_payload and analogous mock helper functions for the
mock branch.
| let adapter = match prompt_adapter::McpPromptAdapter::from_manifest( | ||
| server, | ||
| manifest, | ||
| client.clone(), | ||
| ) { | ||
| Ok(adapter) => adapter, | ||
| Err(error) => { | ||
| let redacted = redact_error_message(&error.to_string()); | ||
| tracing::warn!( | ||
| server = %server.name, | ||
| error = %redacted, | ||
| "MCP prompt adapter creation failed; skipping prompt" | ||
| ); | ||
| continue; | ||
| } | ||
| }; | ||
|
|
||
| tools.push(Box::new(adapter)); | ||
| tools.push(Box::new(adapter)); |
There was a problem hiding this comment.
The content scanner hook is unreachable in the runtime path.
discover_capabilities() boxes the prompt adapter immediately after from_manifest(), and nothing here threads a ContentScanner into it. As written, every discovered prompt runs with content_scanner: None, so the prompt-scanning mitigation is effectively test-only.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@clients/agent-runtime/src/tools/mcp/mod.rs` around lines 190 - 207,
discover_capabilities() currently boxes the McpPromptAdapter returned by
McpPromptAdapter::from_manifest(server, manifest, client.clone()) without
attaching a ContentScanner, so every adapter is created with content_scanner:
None; update the adapter construction path in discover_capabilities (the match
branch that handles Ok(adapter)) to obtain or create the ContentScanner instance
(or accept a scanner param) and set it on the adapter before pushing it into
tools (i.e., ensure the adapter’s content_scanner field is populated from the
runtime scanner provider rather than left None), so discovered prompts run with
a real ContentScanner instead of test-only behavior.
| fn validate_arguments( | ||
| &self, | ||
| args: &serde_json::Value, | ||
| ) -> Result<serde_json::Value, ToolResult> { | ||
| let obj = match args.as_object() { | ||
| Some(obj) => obj, | ||
| None if args.is_null() => { | ||
| // Treat null as empty object | ||
| &serde_json::Map::new() | ||
| } | ||
| None => { | ||
| return Err(ToolResult { | ||
| success: false, | ||
| output: String::new(), | ||
| error: Some("prompt arguments must be a JSON object".to_string()), | ||
| structured: None, | ||
| }); | ||
| } | ||
| }; | ||
|
|
||
| // Check for missing required arguments | ||
| let mut missing: Vec<&str> = Vec::new(); | ||
| for arg in &self.arguments { | ||
| if arg.required && !obj.contains_key(&arg.name) { | ||
| missing.push(&arg.name); | ||
| } | ||
| } | ||
| if !missing.is_empty() { | ||
| return Err(ToolResult { | ||
| success: false, | ||
| output: String::new(), | ||
| error: Some(format!( | ||
| "missing required prompt argument(s): {}", | ||
| missing.join(", ") | ||
| )), | ||
| structured: None, | ||
| }); | ||
| } | ||
|
|
||
| // Check for unknown arguments | ||
| let known_names: std::collections::HashSet<&str> = | ||
| self.arguments.iter().map(|a| a.name.as_str()).collect(); | ||
| let mut unknown: Vec<&str> = Vec::new(); | ||
| for key in obj.keys() { | ||
| if !known_names.contains(key.as_str()) { | ||
| unknown.push(key.as_str()); | ||
| } | ||
| } | ||
| if !unknown.is_empty() { | ||
| return Err(ToolResult { | ||
| success: false, | ||
| output: String::new(), | ||
| error: Some(format!( | ||
| "unknown prompt argument(s): {}", | ||
| unknown.join(", ") | ||
| )), | ||
| structured: None, | ||
| }); | ||
| } | ||
|
|
||
| // Return validated args as Value | ||
| Ok(serde_json::Value::Object(obj.clone())) |
There was a problem hiding this comment.
Validate prompt argument value types, not just keys.
A payload like {"language": null} or {"language": {"nested": true}} currently passes because validate_arguments() only checks presence and unknown names. That violates the string-only schema emitted by parameters_schema() and forwards arbitrary JSON into get_prompt().
Suggested fix
let mut missing: Vec<&str> = Vec::new();
for arg in &self.arguments {
- if arg.required && !obj.contains_key(&arg.name) {
- missing.push(&arg.name);
+ match obj.get(&arg.name) {
+ Some(value) if value.is_string() => {}
+ Some(_) => {
+ return Err(ToolResult {
+ success: false,
+ output: String::new(),
+ error: Some(format!("prompt argument '{}' must be a string", arg.name)),
+ structured: None,
+ });
+ }
+ None if arg.required => missing.push(&arg.name),
+ None => {}
}
}Based on learnings: Implement Tool trait in src/tools/ with strict parameter schema, validate and sanitize all inputs, and return structured ToolResult without panics in runtime path.
Also applies to: 177-205
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@clients/agent-runtime/src/tools/mcp/prompt_adapter.rs` around lines 89 - 150,
validate_arguments currently only checks presence and unknown keys but must also
enforce the parameter schema (string-only) so nulls, objects, arrays, numbers,
and booleans are rejected; update validate_arguments to iterate self.arguments
and for each expected argument (by name) ensure the provided
serde_json::Value.is_string() (or treat null as missing only when not required),
collect any mismatched-type names into an error similar to the existing
missing/unknown checks, and when returning Ok produce an Object containing only
the validated string values (convert to Value::String by cloning the original
string). Reference validate_arguments, parameters_schema, get_prompt and the
Tool trait in your changes so the runtime never forwards arbitrary JSON into
get_prompt and returns a structured ToolResult on type validation failures.
| Ok(Self { | ||
| name: canonical_name, | ||
| description: manifest.description, | ||
| uri: manifest.uri, | ||
| mime_type: manifest.mime_type, | ||
| server_name: server.name.clone(), | ||
| call_timeout_ms: server.call_timeout_ms, | ||
| output_limit_bytes: output_limit, | ||
| client, |
There was a problem hiding this comment.
Use the resource name, not the URI, in source.original_name.
source_metadata_resource() is defined around the manifest name, but spec() passes self.uri. That makes MCP resource metadata inconsistent with the helper contract and with the tool/prompt adapters.
Suggested fix
pub struct McpResourceAdapter {
name: String,
description: String,
+ original_name: String,
uri: String,
mime_type: Option<String>,
server_name: String,
@@
Ok(Self {
name: canonical_name,
description: manifest.description,
+ original_name: manifest.name,
uri: manifest.uri,
mime_type: manifest.mime_type,
server_name: server.name.clone(),
@@
source: Some(normalize::source_metadata_resource(
&self.server_name,
- &self.uri,
+ &self.original_name,
)),
}
}Also applies to: 138-147
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@clients/agent-runtime/src/tools/mcp/resource_adapter.rs` around lines 29 -
37, The MCP resource metadata is using the tool's URI instead of the resource
name for source.original_name: update code in resource_adapter.rs so
source_metadata_resource() and spec() populate source.original_name with the
manifest/canonical name (e.g., use manifest.name or the local canonical_name
field) rather than self.uri; make the same change in the other occurrence around
the spec() call (the block referenced at ~138-147) so MCP metadata aligns with
the helper contract and the tool/prompt adapters.
| org.jetbrains.kotlin.plugin.compose:org.jetbrains.kotlin.plugin.compose.gradle.plugin:2.3.20=classpath | ||
| org.jetbrains.kotlin:abi-tools-api:2.3.20=classpath | ||
| org.jetbrains.kotlin:compose-compiler-gradle-plugin:2.3.20=classpath | ||
| org.jetbrains.kotlin:fus-statistics-gradle-plugin:2.3.20=classpath | ||
| org.jetbrains.kotlin:kotlin-build-statistics:2.3.20=classpath | ||
| org.jetbrains.kotlin:kotlin-build-tools-api:2.3.20=classpath | ||
| org.jetbrains.kotlin:kotlin-compiler-runner:2.3.20=classpath | ||
| org.jetbrains.kotlin:kotlin-daemon-client:2.3.20=classpath | ||
| org.jetbrains.kotlin:kotlin-gradle-plugin-annotations:2.3.20=classpath | ||
| org.jetbrains.kotlin:kotlin-gradle-plugin-api:2.3.20=classpath | ||
| org.jetbrains.kotlin:kotlin-gradle-plugin-idea-proto:2.3.20=classpath | ||
| org.jetbrains.kotlin:kotlin-gradle-plugin-idea:2.3.20=classpath | ||
| org.jetbrains.kotlin:kotlin-gradle-plugin:2.3.20=classpath | ||
| org.jetbrains.kotlin:kotlin-gradle-plugins-bom:2.3.20=classpath | ||
| org.jetbrains.kotlin:kotlin-klib-commonizer-api:2.3.20=classpath | ||
| org.jetbrains.kotlin:kotlin-native-utils:2.3.20=classpath | ||
| org.jetbrains.kotlin:kotlin-reflect:2.3.0=classpath | ||
| org.jetbrains.kotlin:kotlin-stdlib-jdk7:2.1.20=classpath | ||
| org.jetbrains.kotlin:kotlin-stdlib-jdk8:2.1.20=classpath | ||
| org.jetbrains.kotlin:kotlin-stdlib:2.3.0=classpath | ||
| org.jetbrains.kotlin:kotlin-tooling-core:2.3.10=classpath | ||
| org.jetbrains.kotlin:kotlin-util-io:2.3.10=classpath | ||
| org.jetbrains.kotlin:kotlin-util-klib-metadata:2.3.10=classpath | ||
| org.jetbrains.kotlin:kotlin-util-klib:2.3.10=classpath | ||
| org.jetbrains.kotlin:kotlin-tooling-core:2.3.20=classpath | ||
| org.jetbrains.kotlin:kotlin-util-io:2.3.20=classpath | ||
| org.jetbrains.kotlin:kotlin-util-klib-metadata:2.3.20=classpath | ||
| org.jetbrains.kotlin:kotlin-util-klib:2.3.20=classpath |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Kotlin entries in clients/composeApp/buildscript-gradle.lockfile =="
rg -n '^org\.jetbrains\.kotlin:' clients/composeApp/buildscript-gradle.lockfile | sed 's/=classpath$//'
echo
echo "== Kotlin version frequency in that lockfile =="
rg '^org\.jetbrains\.kotlin:' clients/composeApp/buildscript-gradle.lockfile \
| sed -E 's/.*:([0-9][^=]*)=classpath/\1/' \
| sort | uniq -c | sort -nr
echo
echo "== Kotlin version declarations in version catalogs (if present) =="
fd 'libs\.versions\.toml$' -HI | xargs -r rg -n 'kotlin|compose'Repository: dallay/corvus
Length of output: 4043
Confirm intentional version skew for Kotlin stdlib variants.
Version catalog declares kotlin = "2.3.20", but the lockfile contains mixed versions: 19 artifacts at 2.3.20, but kotlin-stdlib-jdk7 and kotlin-stdlib-jdk8 at 2.1.20, and kotlin-stdlib/kotlin-reflect at 2.3.0. Verify this is intentional—mixed Kotlin stdlib versions on the classpath can trigger bytecode incompatibilities and hard-to-debug build failures.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@clients/composeApp/buildscript-gradle.lockfile` around lines 119 - 142, The
lockfile shows mixed Kotlin versions: ensure all stdlib and reflect artifacts
match the declared catalog version (kotlin = "2.3.20") by updating the entries
for org.jetbrains.kotlin:kotlin-stdlib-jdk7,
org.jetbrains.kotlin:kotlin-stdlib-jdk8, org.jetbrains.kotlin:kotlin-stdlib and
org.jetbrains.kotlin:kotlin-reflect to 2.3.20 (or align the catalog if a
different version is intended), then regenerate the Gradle lockfile (e.g.,
refresh dependencies or run the build that writes the lockfile) so
org.jetbrains.kotlin:* artifacts are consistent on the classpath and avoid
bytecode incompatibilities.
| #### Scenario: Prompt with required parameters is registered with schema | ||
|
|
||
| - GIVEN an MCP server advertising a prompt with typed required arguments | ||
| - WHEN prompt discovery runs | ||
| - THEN the runtime MUST register the prompt with its full parameter schema | ||
| - AND the parameter names, types, and required flags MUST be preserved. |
There was a problem hiding this comment.
The archived prompt schema text also overstates typing support.
The runtime only models prompt args as name/description/required, and the generated schema hard-codes every arg to type: "string". “Typed required arguments” and “types preserved” are not true for the implementation in this PR.
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-27-mcp-platform-capabilities/specs/mcp-platform-capabilities/spec.md`
around lines 402 - 407, Update the scenario text "Scenario: Prompt with required
parameters is registered with schema" to accurately reflect the implementation:
remove claims of typing support and type preservation and state that the runtime
models prompt arguments only with name/description/required and that the
generated schema hard-codes each argument as type: "string"; ensure references
to argument fields use the actual modeled symbols ('name', 'description',
'required') and change phrases like "typed required arguments" / "types
preserved" to "required arguments (represented as strings) and their
names/descriptions/required flags are preserved".
| ## Phase 3: Integration (cross-cutting concerns and parity) | ||
|
|
||
| - [x] **3.1** Verify entry-point parity for resources and prompts | ||
| - Confirm registered resources and prompts are accessible via CLI, channels, and dispatcher-backed gateway `/webhook` | ||
| - Confirm policy enforcement is equivalent across all entry points | ||
| - Confirm fallback gateway path does NOT claim resource/prompt parity | ||
| - **Files**: `src/agent/dispatcher.rs`, gateway integration points | ||
| - **Spec**: Entry-Point Parity for All Capability Types (all 3 scenarios) | ||
| - **Complexity**: M |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Add an explicit rollback note for cross-entry-point rollout.
Phase 3 covers parity and verification, but a short rollback step/reference for gateway/dispatcher regressions would make this operationally complete.
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."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@openspec/changes/archive/2026-03-27-mcp-platform-capabilities/tasks.md`
around lines 219 - 227, Add an explicit rollback note to Phase 3 (Integration)
describing a quick rollback strategy and tests for dispatcher/gateway
regressions: update the Phase 3 checklist in
openspec/changes/archive/2026-03-27-mcp-platform-capabilities/tasks.md to
include a new bullet (e.g., "Rollback & Risk") that specifies steps to revert
gateway/dispatcher changes (rollback deployment, revert configuration, disable
new webhook paths), call out verification targets (src/agent/dispatcher.rs and
gateway `/webhook` integration points), and add acceptance tests for boundary
checks and failure modes to validate the rollback path.
|
|
||
| **Verdict**: The tasks are **implemented but not marked complete** in `tasks.md`. This is a **WARNING** — the task list is stale but the code is present. | ||
|
|
||
| > Note: Task 0.6 mentions "Resource adapter registration will be added in Phase 1 (task 1.2)" as a comment in `mod.rs:142`, and resource adapters are **not pushed into the `tools` vec** during discovery — only collision detection runs. This means resources are validated and collision-checked at startup, but **not actually registered as callable tools** in the unified registry. This is a genuine gap (see Issues below). |
There was a problem hiding this comment.
Update the report to match the current discovery code.
These sections still describe resource adapters as not being registered, but discover_capabilities() now pushes them into the unified registry. That leaves the PARTIAL result, warning list, and 47/51 compliance summary stale.
As per coding guidelines: **/*.{md,mdx}: Verify technical accuracy and that docs stay aligned with code changes.
Also applies to: 74-75, 141-144, 229-230
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@openspec/changes/archive/2026-03-27-mcp-platform-capabilities/verify-report.md`
at line 39, The report is out of sync with the current discovery code: update
any statements that say resource adapters are not pushed into the unified
registry to reflect that discover_capabilities() now pushes resource adapters
into the tools vector (the unified registry), and accordingly remove or adjust
the PARTIAL result, the warning list entries, and the `47/51` compliance
summary; search for references to discover_capabilities(), tools (or tools vec),
and the comment at mod.rs:142 to locate affected sections (including lines
noted: 74-75, 141-144, 229-230) and edit the narrative and summaries so they
accurately state that resources are validated, collision-checked, and registered
as callable tools.
| #### Scenario: Resource-specific output limit overrides server default | ||
|
|
||
| - GIVEN an MCP server with `output_limit_bytes = 65536` | ||
| - AND resource-specific config with `resource_limits.output_limit_bytes = 131072` | ||
| - WHEN a resource read is executed | ||
| - THEN the resource-specific limit of 131072 bytes MUST apply | ||
| - AND the server-level default MUST NOT constrain the resource read. |
There was a problem hiding this comment.
Sync the resource section with the shipped runtime.
This still documents resource_limits.output_limit_bytes and a default of AllowWithLimits, but the implementation in this PR uses the flat resource_output_limit_bytes field and classifies McpResource as ApprovalRequired. Leaving the spec looser than the runtime will mislead operators about both config shape and approval requirements.
As per coding guidelines: **/*.{md,mdx}: Verify technical accuracy and that docs stay aligned with code changes.
Also applies to: 348-360
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@openspec/specs/mcp-platform-capabilities/spec.md` around lines 313 - 319,
Update the spec's resource section to match the runtime changes: replace the
nested resource_limits.output_limit_bytes references with the flat
resource_output_limit_bytes field, change any default or enum mentions of
AllowWithLimits to reflect the runtime behavior, and update the resource
approval classification from AllowWithLimits to ApprovalRequired for
McpResource; ensure the "Resource-specific output limit overrides server
default" scenario (and the similar block at 348-360) uses
resource_output_limit_bytes and states that McpResource is ApprovalRequired so
the documented config shape and approval requirements match the implementation.
| #### Scenario: Prompt with required parameters is registered with schema | ||
|
|
||
| - GIVEN an MCP server advertising a prompt with typed required arguments | ||
| - WHEN prompt discovery runs | ||
| - THEN the runtime MUST register the prompt with its full parameter schema | ||
| - AND the parameter names, types, and required flags MUST be preserved. |
There was a problem hiding this comment.
Don't promise typed prompt arguments yet.
PromptArgument only carries name, description, and required, and McpPromptAdapter::parameters_schema() emits type: "string" for every argument. Saying typed arguments and type preservation are supported overstates the implementation.
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/specs/mcp-platform-capabilities/spec.md` around lines 402 - 407, The
spec promises typed prompt arguments and type preservation, but the
implementation (PromptArgument struct and McpPromptAdapter::parameters_schema())
only emits name, description, required and currently sets type: "string" for
every argument; update the spec language to reflect current behavior or
implement full typing—either (A) change the spec text in this scenario to state
that only name, description, and required flags are preserved and that all
arguments are emitted as strings by McpPromptAdapter::parameters_schema(), or
(B) implement typed argument support by extending PromptArgument to include a
type field and updating McpPromptAdapter::parameters_schema() to emit the actual
types instead of always "string" so the spec’s claims are accurate.
|
- Replace window with globalThis across useChat.ts, useGateway.ts, useConfig.ts - Move inner functions to module scope (createSessionId, trimSlashes, etc.) - Replace charCodeAt with codePointAt in useGateway.ts - Refactor nested ternaries in onboarding.ts into helper function - Reduce cognitive complexity in validate-docs-metadata.mjs via extracted helpers - Use String.raw and replaceAll in validate-docs-metadata.mjs - Add explicit return 0 in check-tools.sh - ~41 SonarQube issues resolved
- sandbox.rs: 86→~5 — extract 11 path validation helpers - channels/mod.rs: 67→~7 — extract 8 message processing stage helpers - skills/mod.rs: 31→~5, 36→~8 — extract skill loading and install helpers - scanner.rs: 35→~3 — extract pattern scanning helpers - lockfile.rs: 24→~3 — extract disk reconciliation helpers - telegram.rs: 26→~10 — extract content part building helpers - All 6,132 tests passing, clippy clean, fmt clean
Add the missing @vue/compiler-dom dependency required by Vue component coverage tests and remove a non-null assertion in useConfig so the web workspace passes check and coverage during make all.
…tools-only-integration' into fix/sonarqube-code-smells # Conflicts: # clients/web/pnpm-lock.yaml
…, and related dependencies
- Refactor lockfile hash computation to avoid redundant file reads and clarify logic - Update extract_user_text to prefer canonical content and handle empty parts correctly - Add path fingerprinting for improved sandbox path error logging - Refactor Telegram message parsing to consistently preserve caption text for non-image media and improve command extraction logic - Harden useConfig conflict error handling for null/undefined payloads - Generalize external link detection and restrict markdown link resolution to docs root in docs metadata validation
… upgrade JUnit to 5.14.3
fix: restore clean web validation for make all
Deploying corvus with
|
| Latest commit: |
ec60af0
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://045cac4f.corvus-42x.pages.dev |
| Branch Preview URL: | https://feat-258-expand-mcp-platform.corvus-42x.pages.dev |
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
clients/web/apps/marketing/public/install (1)
799-818:⚠️ Potential issue | 🟠 MajorLifecycle script execution controls differ across package managers; proposed fix is technically incorrect.
The security concern is valid, but the suggested
--ignore-scriptsflag doesn't work uniformly:
yarn dlx: Does not support--ignore-scripts. UseYARN_ENABLE_SCRIPTS=falseenvironment variable or setenableScripts: falsein.yarnrc.yml.yarn global add: The--ignore-scriptsflag is documented but unreliable in practice (GitHub issue#8291). For Yarn Berry v2+, useenableScripts: falsein config.bun add --global: Does not support--ignore-scripts(only available forbun install). Bun skips most lifecycle scripts by default unless intrustedDependencies.Recommend environment-based or config-file approaches instead of CLI flags for consistent behavior across all three package managers.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/web/apps/marketing/public/install` around lines 799 - 818, Do not attempt to use a uniform CLI flag like --ignore-scripts; instead update the INSTALL_METHOD branches to disable lifecycle scripts via environment/config approaches: for the yarn dlx and yarn global add branches wrap the invocation with YARN_ENABLE_SCRIPTS=false (e.g. YARN_ENABLE_SCRIPTS=false yarn dlx "$CLI_PACKAGE" and YARN_ENABLE_SCRIPTS=false yarn global add "$CLI_PACKAGE") and add a comment pointing maintainers to set enableScripts: false in .yarnrc.yml for Yarn Berry v2+; for the bun add --global branch leave the command as-is (bun skips most lifecycle scripts by default) and add a short comment about trustedDependencies behavior. Ensure you change the exact calls yarn dlx "$CLI_PACKAGE", yarn global add "$CLI_PACKAGE", and bun add --global "$CLI_PACKAGE" accordingly and remove any use of --ignore-scripts.clients/web/apps/dashboard/src/composables/useConfig.ts (1)
19-32:⚠️ Potential issue | 🟡 MinorInconsistent base URL:
.hrefvs.origin.Line 22 uses
globalThis.location.hrefas the base for relative URL parsing, whileuseGateway.ts(line 15) andgatewayUrlbelow (line 357) useglobalThis.location.origin.Using
.hrefincludes the current path, so a relative URL like/apiwould resolve differently if the page is at/dashboard/settingsvs/. This could causeisUrlSafeForSecretsto behave inconsistently between files.Proposed fix for consistency
- parsed = rawUrl.startsWith("/") ? new URL(rawUrl, globalThis.location.href) : new URL(rawUrl); + parsed = rawUrl.startsWith("/") ? new URL(rawUrl, globalThis.location.origin) : new URL(rawUrl);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/web/apps/dashboard/src/composables/useConfig.ts` around lines 19 - 32, The isUrlSafeForSecrets function uses globalThis.location.href as the base when constructing a URL for relative inputs; change that to globalThis.location.origin so relative paths (e.g., "/api") resolve consistently with useGateway.ts and gatewayUrl elsewhere. Update the URL construction in isUrlSafeForSecrets to use new URL(rawUrl, globalThis.location.origin) for inputs starting with "/" and keep the existing try/catch and subsequent checks (parsed.username, parsed.password, parsed.search, parsed.hash, parsed.protocol, isTrustedLocalHost(parsed.hostname)) otherwise unchanged.
🤖 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/telegram.rs`:
- Around line 24-31: The caption string is being added twice: once as a
standalone ContentPart::Text (variable caption) and again as Image.caption_text,
causing duplicate text in ChannelMessage::text_projection; update the logic in
telegram.rs (the blocks that handle caption at the photo/document-image branches
and the initial caption extraction) so that you only push ContentPart::Text {
text: cap.clone() } for non-image messages, or alternatively set
Image.caption_text to None/empty when you also push the standalone Text part;
ensure the fix touches the same pattern in the other two spots mentioned (the
blocks around the other image/document handlers) and keep
ChannelMessage::text_projection unchanged.
In `@clients/agent-runtime/src/skills/lockfile.rs`:
- Around line 175-188: collect_disk_skills() currently uses
entry.file_type()?.is_dir() which treats symlinks as non-directories and causes
repair_lockfile() to drop symlinked skills; change the check to follow symlinks
by using entry.path().is_dir() (i.e., replace the file_type()?.is_dir()
conditional with entry.path().is_dir()) so on_disk correctly includes symlinked
skill dirs and reconcile_skill_entry / remove_orphaned_entries() will not delete
their lockfile entries; this aligns with install_local_skill() /
link_or_copy_local_skill() behavior.
In `@clients/agent-runtime/src/skills/mod.rs`:
- Around line 899-909: The helper currently ignores errors from
write_install_lock_entry causing a security gap; change the helper to return
Result<()> and check the Result from write_install_lock_entry (call site around
write_install_lock_entry(workspace_dir, &skill_dir, &skill_source, fm,
content_hash)); if it Errs, perform rollback (remove the unpacked skill_dir /
any partial files) and return the error instead of continuing to print success;
ensure the success println! lines are only executed after
write_install_lock_entry returns Ok; apply the same pattern to the other
identical block (the later block around lines 1033-1068) so both code paths
return Result and abort on lockfile write failures.
- Around line 950-965: scan_and_gate_install currently rereads SKILL.md and
silently skips scanning on read failure and always uses DEFAULT_SCAN_THRESHOLD;
change its signature to accept the already-validated SKILL.md content (e.g.,
content: &str) instead of reading the file, remove the file read/TOCTOU logic,
and make scanning honor config.scan_threshold as an Option: if
config.scan_threshold is None then skip scanning (return Ok(())) but if
Some(threshold) run scanner::scan_skill_content on the provided content,
propagate any scanner errors (return Err on failure), and compare the scan
result to the provided threshold to fail the install when the score exceeds the
threshold (keep fail-closed behavior).
In `@clients/web/apps/dashboard/src/composables/useConfig.ts`:
- Around line 696-698: The code in the fetch response handling throws a generic
Error("save") when response.ok is false, which relies on the outer try/catch in
saveSection to map failures to errorMessage.value = t("form.saveError"); replace
the generic throw with a clearer, named error or include a distinctive message
so the coupling is explicit — e.g., create and throw a SaveError (or throw new
Error("SaveFailed")) in the response.ok branch, and ensure saveSection's catch
still matches that error (or documents/handles it) so the mapping to
errorMessage.value remains correct; reference the response.ok check and the
saveSection catch that sets errorMessage.value for where to make this change.
In `@clients/web/packages/shared/onboarding.ts`:
- Line 76: The final-step call to resolveStepStatus currently hardcodes
isBlocked to false, preventing blocked recovery flows from being represented;
update the call to pass the actual blocked condition instead of false — e.g.,
derive it from the progress object (using progress[finalProgressKey],
progress.transportConnected, or an explicit progress.isBlocked flag) and pass
that value into resolveStepStatus so the final step can become "blocked" for
flows like "paired_but_not_connected".
In `@clients/web/pnpm-workspace.yaml`:
- Line 15: The workspace pins '@vue/compiler-dom' to 3.5.30 which mismatches the
pinned 'vue' at 3.5.31; update the '@vue/compiler-dom' entry to 3.5.31 in the
pnpm-workspace.yaml so both packages use the same compiler/runtime version
(ensure the line containing '@vue/compiler-dom' is changed from 3.5.30 to 3.5.31
and run your lockfile update/install to materialize the single version).
In `@scripts/validate-docs-metadata.mjs`:
- Around line 225-232: The function collectFrontmatterLinkSlugs currently runs
its regex over the entire contents string, which can match `link:` occurrences
outside frontmatter; either rename it to collectLinkSlugs to reflect that
behavior, or change it to only scan the YAML frontmatter: locate the frontmatter
block delimited by leading and trailing `---` (or extract the frontmatter before
callers pass it in), then run the existing matchAll(/\blink:\s*([^\s]+)/g) logic
on that extracted frontmatter and add normalized targets to the referenced Set
(preserving the existing trim/replace logic and isExternalOrEmpty check).
---
Outside diff comments:
In `@clients/web/apps/dashboard/src/composables/useConfig.ts`:
- Around line 19-32: The isUrlSafeForSecrets function uses
globalThis.location.href as the base when constructing a URL for relative
inputs; change that to globalThis.location.origin so relative paths (e.g.,
"/api") resolve consistently with useGateway.ts and gatewayUrl elsewhere. Update
the URL construction in isUrlSafeForSecrets to use new URL(rawUrl,
globalThis.location.origin) for inputs starting with "/" and keep the existing
try/catch and subsequent checks (parsed.username, parsed.password,
parsed.search, parsed.hash, parsed.protocol,
isTrustedLocalHost(parsed.hostname)) otherwise unchanged.
In `@clients/web/apps/marketing/public/install`:
- Around line 799-818: Do not attempt to use a uniform CLI flag like
--ignore-scripts; instead update the INSTALL_METHOD branches to disable
lifecycle scripts via environment/config approaches: for the yarn dlx and yarn
global add branches wrap the invocation with YARN_ENABLE_SCRIPTS=false (e.g.
YARN_ENABLE_SCRIPTS=false yarn dlx "$CLI_PACKAGE" and YARN_ENABLE_SCRIPTS=false
yarn global add "$CLI_PACKAGE") and add a comment pointing maintainers to set
enableScripts: false in .yarnrc.yml for Yarn Berry v2+; for the bun add --global
branch leave the command as-is (bun skips most lifecycle scripts by default) and
add a short comment about trustedDependencies behavior. Ensure you change the
exact calls yarn dlx "$CLI_PACKAGE", yarn global add "$CLI_PACKAGE", and bun add
--global "$CLI_PACKAGE" accordingly and remove any use of --ignore-scripts.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 0daf9c10-8759-4551-a765-55065b4f0bd9
⛔ Files ignored due to path filters (1)
clients/web/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (19)
clients/agent-runtime/src/channels/mod.rsclients/agent-runtime/src/channels/telegram.rsclients/agent-runtime/src/skills/lockfile.rsclients/agent-runtime/src/skills/mod.rsclients/agent-runtime/src/skills/sandbox.rsclients/agent-runtime/src/skills/scanner.rsclients/androidApp/gradle.lockfileclients/web/apps/chat/package.jsonclients/web/apps/chat/src/composables/useChat.tsclients/web/apps/chat/src/composables/useGateway.tsclients/web/apps/dashboard/package.jsonclients/web/apps/dashboard/src/composables/useConfig.tsclients/web/apps/marketing/public/installclients/web/package.jsonclients/web/packages/shared/onboarding.tsclients/web/pnpm-workspace.yamlgradle/build-logic/gradle.lockfilescripts/check-tools.shscripts/validate-docs-metadata.mjs
📜 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). (5)
- GitHub Check: pr-checks
- GitHub Check: pr-checks-build-logic
- GitHub Check: sonar
- GitHub Check: submit-gradle
- GitHub Check: Cloudflare Pages
🧰 Additional context used
📓 Path-based instructions (5)
**/*
⚙️ CodeRabbit configuration file
**/*: Security first, performance second.
Validate input boundaries, auth/authz implications, and secret management.
Look for behavioral regressions, missing tests, and contract breaks across modules.
Files:
clients/web/apps/chat/package.jsonclients/web/package.jsonclients/web/apps/dashboard/package.jsonscripts/check-tools.shclients/web/apps/marketing/public/installclients/web/apps/chat/src/composables/useChat.tsclients/web/apps/chat/src/composables/useGateway.tsclients/web/packages/shared/onboarding.tsclients/agent-runtime/src/skills/scanner.rsclients/agent-runtime/src/skills/lockfile.rsclients/agent-runtime/src/skills/mod.rsclients/androidApp/gradle.lockfileclients/web/apps/dashboard/src/composables/useConfig.tsscripts/validate-docs-metadata.mjsclients/web/pnpm-workspace.yamlclients/agent-runtime/src/channels/telegram.rsclients/agent-runtime/src/skills/sandbox.rsclients/agent-runtime/src/channels/mod.rsgradle/build-logic/gradle.lockfile
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/skills/scanner.rsclients/agent-runtime/src/skills/lockfile.rsclients/agent-runtime/src/skills/mod.rsclients/agent-runtime/src/channels/telegram.rsclients/agent-runtime/src/skills/sandbox.rsclients/agent-runtime/src/channels/mod.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/skills/scanner.rsclients/agent-runtime/src/skills/lockfile.rsclients/agent-runtime/src/skills/mod.rsclients/agent-runtime/src/channels/telegram.rsclients/agent-runtime/src/skills/sandbox.rsclients/agent-runtime/src/channels/mod.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/skills/scanner.rsclients/agent-runtime/src/skills/lockfile.rsclients/agent-runtime/src/skills/mod.rsclients/agent-runtime/src/channels/telegram.rsclients/agent-runtime/src/skills/sandbox.rsclients/agent-runtime/src/channels/mod.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/telegram.rsclients/agent-runtime/src/channels/mod.rs
🧠 Learnings (8)
📚 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/src/skills/scanner.rsclients/agent-runtime/src/skills/mod.rsclients/agent-runtime/src/skills/sandbox.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/**/*.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:
clients/agent-runtime/src/skills/scanner.rsclients/agent-runtime/src/skills/mod.rsclients/agent-runtime/src/skills/sandbox.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/src/skills/scanner.rsclients/agent-runtime/src/skills/mod.rsclients/agent-runtime/src/skills/sandbox.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/skills/scanner.rsclients/agent-runtime/src/skills/lockfile.rsclients/agent-runtime/src/skills/mod.rsclients/agent-runtime/src/channels/telegram.rsclients/agent-runtime/src/skills/sandbox.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/src/skills/scanner.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/src/skills/mod.rsclients/agent-runtime/src/skills/sandbox.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/skills/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/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/src/channels/telegram.rsclients/agent-runtime/src/channels/mod.rs
🔇 Additional comments (28)
scripts/check-tools.sh (1)
22-22: Good hardening for strict mode behavior.Explicit
return 0innumeric_prefix()is a safe improvement underset -euo pipefailand avoids accidental non-zero function exits from future edits.gradle/build-logic/gradle.lockfile (1)
6-9: Lockfile updates look consistent and regression-safe.The updated Spotless, Kover, and JUnit entries are internally consistent across scopes and align with the current version-catalog expectations; this resolves the prior mixed-version risk in this file.
Also applies to: 119-119, 124-124, 128-131
clients/web/apps/marketing/public/install (1)
793-797: Good security hardening for pnpm/npm installs.Adding
--ignore-scriptsprevents execution of potentially malicious lifecycle scripts during global package installation. This aligns with the PR's security focus.clients/web/apps/chat/src/composables/useGateway.ts (2)
11-26: LGTM on URL safety validation.The explicit rejection of protocol-relative URLs (
//) at line 12 before URL parsing is a solid defense against scheme-relative attacks. TheglobalThis.location.originusage aligns with the project-wide pattern for safer SSR compatibility.
141-171: LGTM on module-scope helper extraction.Moving pure functions (
trimTrailingSlashes,trimLeadingSlashes,createIdempotencyKey,readJsonPayload) to module scope is appropriate—they don't depend on reactive state. Thecrypto.randomUUIDfallback handles environments without native UUID support.clients/web/apps/dashboard/src/composables/useConfig.ts (3)
272-282: LGTM on auth/JSON helpers.
isAuthRejectedandreadJsonPayloadare clean extractions that improve readability. ThereadJsonPayloadbehavior matchesuseGateway.ts.
609-658: LGTM on quick-pair SSR guards.The
globalThis.windowguard prevents runtime errors in SSR. Hash parameters are safely parsed viaURLSearchParams, and the pairing code goes throughisUrlSafeForSecretsvalidation before use.
680-712: LGTM on save response processing.The flow correctly:
- Handles auth rejection early
- Parses 409 conflict fields for user feedback
- Resets webhook form fields only on webhook section saves
- Preserves
pendingWebhookSecretacross reconnect to avoid losing user inputclients/web/apps/chat/src/composables/useChat.ts (2)
46-52: LGTM on session ID generation.The
crypto.randomUUIDfallback pattern matchesuseGateway.ts. The"session-"prefix (vs"chat-"inuseGateway) appropriately distinguishes the ID's purpose.
107-141: LGTM on storage access patterns.The
globalThis.windowguards combined with try/catch aroundsessionStorageaccess handle both SSR environments and browser storage exceptions (quota exceeded, private browsing restrictions).clients/androidApp/gradle.lockfile (1)
160-162: Dependency versions are consistent and secure.Protobuf 3.25.9 and Netty 4.1.132.Final in the lockfile correctly reflect
gradle/libs.versions.toml. Both versions are free of known CVEs—Protobuf 3.25.9 is safe from CVE-2024-7254 (which affects versions below 3.25.5), and Netty 4.1.132.Final includes fixes for recent HTTP smuggling and DoS vulnerabilities.scripts/validate-docs-metadata.mjs (5)
152-154: LGTM!Using
String.rawavoids escape-sequence issues, and the quote-stripping regex correctly handles both single and double quotes at string boundaries.
186-194: Good defensive path validation.The
isWithinDocsRootcheck correctly handles path traversal attempts and the Windows cross-drive edge case wherepath.relativecan return an absolute path.
196-223: Solid ReDoS mitigation and path traversal protection.The 500-char regex limits prevent catastrophic backtracking, and
isWithinDocsRootfiltering on line 211 ensures untrusted link targets from markdown content cannot escape the docs directory.
343-378: Clean validation extraction.The UTC midnight suffix ensures consistent date comparisons across timezones, and the validation hierarchy (presence → format → semantics) is well-structured. Skipping max-age for deprecated docs is a sensible policy.
414-431: LGTM!Integration of the refactored validation helpers is clean and consistent with the existing error-accumulation pattern.
clients/web/package.json (1)
32-32: No actionable issue in this hunk.
Dependency is added via workspace catalog as expected; any risk is in catalog version alignment (reviewed inclients/web/pnpm-workspace.yaml).clients/web/apps/chat/package.json (1)
39-39: No direct issue here.
This app-level catalog reference is fine; correctness depends on the catalog version defined inclients/web/pnpm-workspace.yaml.clients/web/apps/dashboard/package.json (1)
40-40: No direct issue in this line.
Catalog-based dependency usage is correct; version alignment concerns are at workspace catalog level.clients/agent-runtime/src/skills/scanner.rs (1)
52-202: Behavior-preserving refactor.The helper split keeps the scoring table centralized, and the final
emit_multiline_b64(...)call cleanly covers the trailing multiline block case.clients/agent-runtime/src/channels/mod.rs (8)
786-798: LGTM — clean text extraction with proper fallback.The logic correctly handles the two message patterns: channels like Slack that pre-populate
content, and multimodal channels like Telegram that may needtext_projection()recomputation from parts. Based on learnings, this aligns with the existingderive_text_projectionpattern in telegram.rs.
800-830: LGTM — memory operations correctly gated on non-empty text.Image-only messages won't trigger spurious memory recall/store operations. The guard
user_text.trim().is_empty()at lines 806 and 812 ensures this.
860-912: LGTM — clean gating with fail-fast rejection.The early
Ok(None)return for text-only messages (line 870) and consolidated rejection viareject_image_turnkeeps the main processing function focused. TheErr(())return type clearly signals that the rejection response was already sent.
927-990: LGTM — fail-closed staging with proper RAII cleanup.The fail-closed check at lines 972-986 ensures channels that haven't implemented image staging don't silently drop images. The
StagedImageGuardensures temp files are cleaned on all exit paths.
1008-1041: LGTM — observability emission with proper guards.The early returns at lines 1017 and 1025 prevent spurious events for text-only messages. The distinction between success (metadata from staged) and error (count from original parts) cases is correct.
625-680: LGTM — refactored control flow is cleaner.The extraction of gating logic into helpers improves readability. Memory enrichment at lines 625-626 happens before image gating, which is correct since text content should be processed regardless of image rejection outcome.
740-780: LGTM — consistent observability across all provider outcomes.The unified
emit_image_provider_outcomecall replaces previously scattered emission logic. For text-only messages, the helper's early return (route=None) prevents spurious events.
914-925: LGTM — user-friendly rejection messages with session traceability.The session ID prefix enables log correlation. Messages are informative without exposing internal implementation details.
Also applies to: 992-1006
| let caption = message | ||
| .get("caption") | ||
| .and_then(serde_json::Value::as_str) | ||
| .map(String::from); | ||
|
|
||
| if let Some(ref cap) = caption { | ||
| parts.push(ContentPart::Text { text: cap.clone() }); | ||
| } |
There was a problem hiding this comment.
Image captions are represented twice in parts.
These lines push the caption as a standalone Text part and also copy it into Image.caption_text. ChannelMessage::text_projection() in clients/agent-runtime/src/channels/traits.rs Lines 31-55 already emits caption_text, so photo and document-image messages will project as caption\n\ncaption for downstream callers. Keep the standalone Text part only for non-image messages, or clear caption_text on the image part.
Also applies to: 48-60, 93-100
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@clients/agent-runtime/src/channels/telegram.rs` around lines 24 - 31, The
caption string is being added twice: once as a standalone ContentPart::Text
(variable caption) and again as Image.caption_text, causing duplicate text in
ChannelMessage::text_projection; update the logic in telegram.rs (the blocks
that handle caption at the photo/document-image branches and the initial caption
extraction) so that you only push ContentPart::Text { text: cap.clone() } for
non-image messages, or alternatively set Image.caption_text to None/empty when
you also push the standalone Text part; ensure the fix touches the same pattern
in the other two spots mentioned (the blocks around the other image/document
handlers) and keep ChannelMessage::text_projection unchanged.
| for entry in std::fs::read_dir(skills_path)? { | ||
| let entry = entry?; | ||
| if !entry.file_type()?.is_dir() { | ||
| continue; | ||
| } | ||
| let name = entry.file_name().to_string_lossy().to_string(); | ||
| let skill_md = entry.path().join("SKILL.md"); | ||
|
|
||
| if !skill_md.exists() { | ||
| continue; | ||
| } | ||
|
|
||
| on_disk.insert(name.clone()); | ||
| reconcile_skill_entry(&name, &skill_md, current, summary); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Rust std::fs::DirEntry, does file_type()?.is_dir() report false for a symlink that points to a directory?
💡 Result:
Yes, std::fs::DirEntry::file_type?.is_dir reports false for a symlink that points to a directory. It returns the file type of the symlink itself (equivalent to lstat or symlink_metadata), which identifies it as a symlink, not a directory.
Citations:
- 1: https://doc.rust-lang.org/std/fs/struct.DirEntry.html
- 2: https://doc.rust-lang.org/std/fs/struct.Metadata.html
- 3: https://web.mit.edu/rust-lang_v1.25/arch/amd64_ubuntu1404/share/doc/rust/html/std/fs/struct.DirEntry.html
🏁 Script executed:
# Check install_local_skill function around lines 1142-1145
fd --type f -path "*/clients/agent-runtime/src/skills/mod.rs" | head -1 | xargs -I {} head -1200 {} | tail -100Repository: dallay/corvus
Length of output: 225
🏁 Script executed:
# Also check if there are platform-specific symlink operations
rg -A 10 "install_local_skill" clients/agent-runtime/src/skills/mod.rs | head -40Repository: dallay/corvus
Length of output: 1708
🏁 Script executed:
# Check the remove_orphaned_entries function
rg -B 5 -A 15 "remove_orphaned_entries" clients/agent-runtime/src/skills/lockfile.rsRepository: dallay/corvus
Length of output: 1329
🏁 Script executed:
# Check what happens with on_disk - see full context of the repair_lockfile function
rg -B 20 "collect_disk_skills" clients/agent-runtime/src/skills/lockfile.rs | head -50Repository: dallay/corvus
Length of output: 1147
🏁 Script executed:
# Verify behavior of entry.path().is_dir() vs file_type().is_dir()
# by checking if there are any existing uses of these patterns
rg "entry\.path\(\)\.is_dir\(\)" clients/agent-runtime/Repository: dallay/corvus
Length of output: 39
🏁 Script executed:
# Get full install_local_skill implementation
rg -A 30 "^fn install_local_skill" clients/agent-runtime/src/skills/mod.rsRepository: dallay/corvus
Length of output: 1042
🏁 Script executed:
# Look for symlink/unix-specific code in the skills module
rg "symlink|unix|cfg" clients/agent-runtime/src/skills/mod.rs | head -20Repository: dallay/corvus
Length of output: 326
🏁 Script executed:
# Check the full collect_disk_skills to see exact filtering logic
rg -A 25 "fn collect_disk_skills" clients/agent-runtime/src/skills/lockfile.rsRepository: dallay/corvus
Length of output: 797
🏁 Script executed:
# Look for any documentation or comments about symlink handling
rg -B 5 -A 5 "symlink|DirEntry" clients/agent-runtime/src/skills/lockfile.rsRepository: dallay/corvus
Length of output: 39
repair_lockfile() will delete lockfile entries for symlinked skill directories.
On Unix and Windows, install_local_skill() creates symlinks to local skills (via link_or_copy_local_skill()), but collect_disk_skills() filters with entry.file_type()?.is_dir(). Since file_type() on a DirEntry returns metadata of the symlink itself (not what it points to), symlinks are skipped from the on_disk set. This causes remove_orphaned_entries() to delete their lockfile entries during repair.
Use entry.path().is_dir() to follow symlinks and properly recognize symlinked skill directories.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@clients/agent-runtime/src/skills/lockfile.rs` around lines 175 - 188,
collect_disk_skills() currently uses entry.file_type()?.is_dir() which treats
symlinks as non-directories and causes repair_lockfile() to drop symlinked
skills; change the check to follow symlinks by using entry.path().is_dir()
(i.e., replace the file_type()?.is_dir() conditional with entry.path().is_dir())
so on_disk correctly includes symlinked skill dirs and reconcile_skill_entry /
remove_orphaned_entries() will not delete their lockfile entries; this aligns
with install_local_skill() / link_or_copy_local_skill() behavior.
| // 5. Build and write lock entry | ||
| write_install_lock_entry(workspace_dir, &skill_dir, &skill_source, fm, content_hash); | ||
|
|
||
| // 6. Print success with trust info | ||
| println!( | ||
| " {} Skill installed successfully! (trust: {})", | ||
| console::style("✓").green().bold(), | ||
| skill_trust.as_str(), | ||
| ); | ||
| println!(" Restart `corvus channel start` to activate."); | ||
| Ok(()) |
There was a problem hiding this comment.
Do not continue after the lockfile write fails.
If write_lock_entry() errors here, the skill stays on disk and the caller still prints success. On the next load, Lines 87-96 leave a missing lockfile entry at default Local trust, and Lines 267-279 expose all Local tools, so a third-party install can come back fully trusted after restart. Return Result<()> from this helper and fail or roll back before printing success.
Based on learnings: Do not silently weaken security policy or access constraints; keep default behavior secure-by-default with deny-by-default where applicable.
Also applies to: 1033-1068
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@clients/agent-runtime/src/skills/mod.rs` around lines 899 - 909, The helper
currently ignores errors from write_install_lock_entry causing a security gap;
change the helper to return Result<()> and check the Result from
write_install_lock_entry (call site around
write_install_lock_entry(workspace_dir, &skill_dir, &skill_source, fm,
content_hash)); if it Errs, perform rollback (remove the unpacked skill_dir /
any partial files) and return the error instead of continuing to print success;
ensure the success println! lines are only executed after
write_install_lock_entry returns Ok; apply the same pattern to the other
identical block (the later block around lines 1033-1068) so both code paths
return Result and abort on lockfile write failures.
| fn scan_and_gate_install( | ||
| skill_dir: &Path, | ||
| skill_trust: trust::SkillTrust, | ||
| config: &crate::config::SkillsConfig, | ||
| trust_flag: bool, | ||
| ) -> Result<()> { | ||
| if skill_trust != trust::SkillTrust::ThirdParty { | ||
| return Ok(()); | ||
| } | ||
| let Ok(content) = std::fs::read_to_string(skill_dir.join("SKILL.md")) else { | ||
| return Ok(()); | ||
| }; | ||
| let scan = scanner::scan_skill_content(&content); | ||
| let threshold = config | ||
| .scan_threshold | ||
| .unwrap_or(scanner::DEFAULT_SCAN_THRESHOLD); |
There was a problem hiding this comment.
Keep install-time scanning fail-closed and aligned with scan_threshold.
This helper rereads SKILL.md after validate_and_parse_skill_md() already parsed it. The second read returns Ok(()) on failure, so a read/TOCTOU error skips scanning, and unwrap_or(DEFAULT_SCAN_THRESHOLD) ignores the explicit None disable that Lines 158-160 honor during load. Pass the validated content into this helper and error if the scan cannot run.
Based on learnings: Do not silently weaken security policy or access constraints; keep default behavior secure-by-default with deny-by-default where applicable.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@clients/agent-runtime/src/skills/mod.rs` around lines 950 - 965,
scan_and_gate_install currently rereads SKILL.md and silently skips scanning on
read failure and always uses DEFAULT_SCAN_THRESHOLD; change its signature to
accept the already-validated SKILL.md content (e.g., content: &str) instead of
reading the file, remove the file read/TOCTOU logic, and make scanning honor
config.scan_threshold as an Option: if config.scan_threshold is None then skip
scanning (return Ok(())) but if Some(threshold) run scanner::scan_skill_content
on the provided content, propagate any scanner errors (return Err on failure),
and compare the scan result to the provided threshold to fail the install when
the score exceeds the threshold (keep fail-closed behavior).
| if (!response.ok) { | ||
| throw new Error("save"); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Generic throw at line 697 relies on outer catch.
throw new Error("save") is caught by the outer try/catch in saveSection (line 756-757), which sets errorMessage.value = t("form.saveError"). This is correct but consider documenting this coupling or using a named error class for clarity.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@clients/web/apps/dashboard/src/composables/useConfig.ts` around lines 696 -
698, The code in the fetch response handling throws a generic Error("save") when
response.ok is false, which relies on the outer try/catch in saveSection to map
failures to errorMessage.value = t("form.saveError"); replace the generic throw
with a clearer, named error or include a distinctive message so the coupling is
explicit — e.g., create and throw a SaveError (or throw new Error("SaveFailed"))
in the response.ok branch, and ensure saveSection's catch still matches that
error (or documents/handles it) so the mapping to errorMessage.value remains
correct; reference the response.ok check and the saveSection catch that sets
errorMessage.value for where to make this change.
| : !progress.transportConnected | ||
| ? "pending" | ||
| : "current", | ||
| status: resolveStepStatus(progress[finalProgressKey], progress.transportConnected, false), |
There was a problem hiding this comment.
Final-step blocked state regression in onboarding status computation
Line 76 hardcodes isBlocked to false, so the final step can no longer become "blocked" under blocked recovery flows (e.g., "paired_but_not_connected"). This breaks existing behavior expected by consumers/tests and can surface incorrect UI state.
Suggested fix
- status: resolveStepStatus(progress[finalProgressKey], progress.transportConnected, false),
+ status: resolveStepStatus(
+ progress[finalProgressKey],
+ progress.transportConnected,
+ blockedRecovery === "paired_but_not_connected"
+ ),As per coding guidelines, "Look for behavioral regressions, missing tests, and contract breaks across modules."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@clients/web/packages/shared/onboarding.ts` at line 76, The final-step call to
resolveStepStatus currently hardcodes isBlocked to false, preventing blocked
recovery flows from being represented; update the call to pass the actual
blocked condition instead of false — e.g., derive it from the progress object
(using progress[finalProgressKey], progress.transportConnected, or an explicit
progress.isBlocked flag) and pass that value into resolveStepStatus so the final
step can become "blocked" for flows like "paired_but_not_connected".
| '@vitejs/plugin-vue': 6.0.4 | ||
| '@vitejs/plugin-vue': 6.0.5 | ||
| '@vitest/coverage-v8': 4.0.18 | ||
| '@vue/compiler-dom': 3.5.30 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Catalog versions =="
rg -n "(`@vue/compiler-dom`|^ vue:)" clients/web/pnpm-workspace.yaml
echo
echo "== App manifests referencing catalog entries =="
rg -n "\"(`@vue/compiler-dom`|vue)\": \"catalog:\"" \
clients/web/package.json \
clients/web/apps/chat/package.json \
clients/web/apps/dashboard/package.json
echo
echo "== Lockfile resolution check (if present) =="
if fd -a '^pnpm-lock.yaml$' . >/dev/null; then
rg -n "@vue/compiler-dom@|/vue@" pnpm-lock.yaml | head -n 80
else
echo "pnpm-lock.yaml not found in repository root."
fiRepository: dallay/corvus
Length of output: 571
🏁 Script executed:
#!/bin/bash
# Check for lockfile in various locations
echo "=== Searching for lockfile ==="
fd -a 'pnpm-lock\.' . 2>/dev/null | head -5
echo
echo "=== Checking clients/web directory structure ==="
ls -la clients/web/ | grep -E "lock|pnpm" || echo "No lock files in clients/web/"
echo
echo "=== Vue 3.5.x version compatibility context ==="
# Check if there are any explicit version constraints or comments in the workspace file
sed -n '10,35p' clients/web/pnpm-workspace.yamlRepository: dallay/corvus
Length of output: 1082
🏁 Script executed:
#!/bin/bash
echo "=== Resolved versions in pnpm-lock.yaml (clients/web) ==="
rg "@vue/compiler-dom@3\.|/vue@3\." clients/web/pnpm-lock.yaml | head -20
echo
echo "=== Check if both versions coexist in lockfile ==="
echo "Compiler-dom@3.5.30 instances:"
rg "@vue/compiler-dom@3\.5\.30" clients/web/pnpm-lock.yaml | wc -l
echo "Compiler-dom@3.5.31 instances:"
rg "@vue/compiler-dom@3\.5\.31" clients/web/pnpm-lock.yaml | wc -l
echo "Vue@3.5.30 instances:"
rg "/vue@3\.5\.30" clients/web/pnpm-lock.yaml | wc -l
echo "Vue@3.5.31 instances:"
rg "/vue@3\.5\.31" clients/web/pnpm-lock.yaml | wc -l
echo
echo "=== Check root lockfile (pnpm-lock.yaml) ==="
rg "@vue/compiler-dom@3\.|/vue@3\." pnpm-lock.yaml | head -20Repository: dallay/corvus
Length of output: 446
🏁 Script executed:
#!/bin/bash
echo "=== Lockfile entries for `@vue/compiler-dom`@3.5.30 ==="
rg -A 5 "@vue/compiler-dom@3\.5\.30':" clients/web/pnpm-lock.yaml | head -30
echo
echo "=== Lockfile entries for `@vue/compiler-dom`@3.5.31 ==="
rg -A 5 "@vue/compiler-dom@3\.5\.31':" clients/web/pnpm-lock.yaml | head -30
echo
echo "=== Check which packages depend on `@vue/compiler-dom` (from lockfile) ==="
rg "^\s+@vue/compiler-dom:" clients/web/pnpm-lock.yaml -B 2 | head -50Repository: dallay/corvus
Length of output: 1153
🏁 Script executed:
#!/bin/bash
echo "=== Search for dependency entries pulling `@vue/compiler-dom` versions ==="
rg -B 3 "@vue/compiler-dom@3\.5\.(30|31)" clients/web/pnpm-lock.yaml | grep -E "^\s+[a-zA-Z0-9@/-]+:" | sort | uniq
echo
echo "=== Check if vue@3.5.31 peerDependencies specify a compiler-dom version ==="
rg -A 10 "^ vue@3\.5\.31:" clients/web/pnpm-lock.yaml | head -20
echo
echo "=== Check app manifests for direct `@vue/compiler-dom` overrides ==="
rg "@vue/compiler-dom" clients/web/package.json clients/web/apps/*/package.jsonRepository: dallay/corvus
Length of output: 1190
🏁 Script executed:
#!/bin/bash
echo "=== Find all packages that explicitly depend on `@vue/compiler-dom` ==="
rg -B 2 "dependencies:" clients/web/pnpm-lock.yaml | rg -B 2 "@vue/compiler-dom" | head -50
echo
echo "=== Check if any package requires specific `@vue/compiler-dom` version ==="
rg "^ `@vue/compiler-dom`: " clients/web/pnpm-lock.yaml
echo
echo "=== Search across all dependency sections ==="
rg "^\s+@vue/compiler-dom@" clients/web/pnpm-lock.yaml | sort | uniq -cRepository: dallay/corvus
Length of output: 318
🏁 Script executed:
#!/bin/bash
echo "=== List all direct dependencies in app manifests ==="
echo "clients/web/package.json devDependencies:"
rg '"([a-z@/-]+)":\s*"' clients/web/package.json | head -20
echo
echo "=== Check if astro or other packages pull vue/compiler-dom ==="
rg -A 2 "^ (astro|vite|@vitejs)@" clients/web/pnpm-lock.yaml | grep -E "^\s+(dependencies|@vue)" | head -20Repository: dallay/corvus
Length of output: 951
Align @vue/compiler-dom to 3.5.31 to match vue.
Catalog declares @vue/compiler-dom at 3.5.30 while vue is pinned to 3.5.31. Lockfile shows both versions are materialized, resulting in different template compiler versions across the workspace. This introduces behavioral regression risk across chat/dashboard builds.
Proposed fix
- '@vue/compiler-dom': 3.5.30
+ '@vue/compiler-dom': 3.5.31📝 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.
| '@vue/compiler-dom': 3.5.30 | |
| '@vue/compiler-dom': 3.5.31 |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@clients/web/pnpm-workspace.yaml` at line 15, The workspace pins
'@vue/compiler-dom' to 3.5.30 which mismatches the pinned 'vue' at 3.5.31;
update the '@vue/compiler-dom' entry to 3.5.31 in the pnpm-workspace.yaml so
both packages use the same compiler/runtime version (ensure the line containing
'@vue/compiler-dom' is changed from 3.5.30 to 3.5.31 and run your lockfile
update/install to materialize the single version).
| function collectFrontmatterLinkSlugs(contents, referenced) { | ||
| for (const match of contents.matchAll(/\blink:\s*([^\s]+)/g)) { | ||
| const target = match[1].trim().replaceAll(/^['"]|['"]$/g, "").replace(/\/$/, ""); | ||
| if (!isExternalOrEmpty(target)) { | ||
| referenced.add(target.replace(/^\//, "")); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Function scans entire file, not just frontmatter.
collectFrontmatterLinkSlugs applies its regex to the full contents string rather than the extracted frontmatter block. This could match link: patterns in code blocks or prose.
Impact is low—false positives just prevent orphan warnings—but consider either:
- Renaming to
collectLinkSlugsto reflect actual behavior, or - Passing only the frontmatter section as input
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/validate-docs-metadata.mjs` around lines 225 - 232, The function
collectFrontmatterLinkSlugs currently runs its regex over the entire contents
string, which can match `link:` occurrences outside frontmatter; either rename
it to collectLinkSlugs to reflect that behavior, or change it to only scan the
YAML frontmatter: locate the frontmatter block delimited by leading and trailing
`---` (or extract the frontmatter before callers pass it in), then run the
existing matchAll(/\blink:\s*([^\s]+)/g) logic on that extracted frontmatter and
add normalized targets to the referenced Set (preserving the existing
trim/replace logic and isExternalOrEmpty check).
yacosta738
deleted the
feat/258-expand-mcp-platform-support-beyond-tools-only-integration
branch
1 participant
This pull request significantly extends the MCP (Modular Capability Provider) system to support new capability types—resources and prompts—alongside tools. It introduces new configuration options, validation logic, and mock/test support for these capabilities. Additionally, security policy logic is updated to recognize and require approval for these new MCP capability types.
The most important changes are:
MCP Capabilities Framework Expansion:
McpServerConfigstruct now supports a newcapabilitiesfield (defaulting to["tools"]for backward compatibility), as well as optional per-capability output limits for resources and prompts. Validation logic ensures only valid, non-duplicate capability types are allowed, and that output limits are within bounds. [1] [2] [3] [4]Resource and Prompt Capability Support:
McpResourceManifest,McpPromptManifest,PromptArgument,PromptMessage) and corresponding methods toMcpClientfor listing and retrieving resources and prompts, with mock implementations for testing. [1] [2] [3] [4]Security Policy Updates:
ToolSourceKindenum and related detection logic now distinguish betweenMcp,McpResource, andMcpPrompttool types based on tool name patterns. Policy evaluation requires explicit approval for all MCP-based tool types. [1] [2] [3] [4]Codebase Maintenance:
opentelemetry-otlpbumped to version0.31.1.default_mcp_capabilitiesin the config module for broader use.Closes: #258