feat(agent-runtime): add auth and provider runtime upgrades#29
Conversation
|
Thank you for contributing to this project with this PR, welcome to the community and the amazing world of open source! |
📝 WalkthroughWalkthroughThe PR makes large-scale enhancements across the agent runtime: package metadata update (corvus → zeroclaw), new authentication subsystem (OAuth/profile store/token refresh), query classification for runtime model routing, channel streaming/draft and threading support, multiple provider integrations (GLM, OpenAI Codex, router/resilient changes), Prometheus observability, web-search tool, security policy enforcement across tools, and numerous memory/SQLite, scheduler, gateway, and test changes. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Agent
participant Classifier
participant Provider
participant Tool
participant Memory
User->>Agent: turn(user_message)
Agent->>Classifier: classify(message, rules)
Classifier-->>Agent: hint / None
Agent->>Provider: chat_with_system(..., model=effective_model)
Provider-->>Agent: response (may include tool_calls)
alt tool_calls present
Agent->>Tool: execute(tool_call)
Tool-->>Memory: store/lookup (if needed)
Memory-->>Tool: result
Tool-->>Agent: tool_result
Agent->>Provider: chat_with_history(..., model=effective_model)
Provider-->>Agent: followup response
end
Agent-->>User: final_response
sequenceDiagram
participant Channel
participant ChannelRuntime
participant Agent
participant Memory
participant Provider
Channel->>ChannelRuntime: process_channel_message(msg)
ChannelRuntime->>Memory: build_memory_context(msg, min_relevance_score)
Memory-->>ChannelRuntime: filtered_memories
ChannelRuntime->>ChannelRuntime: load_per_sender_history(sender)
ChannelRuntime->>Agent: turn(message + history + context, max_tool_iterations)
Agent->>Provider: chat_with_history(...)
Provider-->>Agent: response (streamed or final)
alt supports_draft_updates
ChannelRuntime->>Channel: send_draft(initial)
loop stream
ChannelRuntime->>Channel: update_draft(delta)
end
ChannelRuntime->>Channel: finalize_draft(final_text)
else
ChannelRuntime->>Channel: send_message(final_text)
end
ChannelRuntime->>ChannelRuntime: persist_sender_history(updated_history)
sequenceDiagram
participant CLI
participant AuthService
participant Store
participant OAuth
participant SecretStore
CLI->>AuthService: get_valid_openai_access_token(optional_override)
AuthService->>Store: load_profiles()
Store->>SecretStore: decrypt_tokens()
SecretStore-->>Store: tokens
Store-->>AuthService: selected_profile
alt token expired
AuthService->>AuthService: acquire_refresh_lock(profile)
AuthService->>OAuth: refresh_access_token(refresh_token)
OAuth-->>AuthService: new_token_set
AuthService->>SecretStore: encrypt(new_token_set)
SecretStore-->>Store: encrypted_data
Store->>AuthService: persist_profile(updated_profile)
end
AuthService-->>CLI: access_token
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
✅ Contributor ReportUser: @yacosta738
Contributor Report evaluates based on public GitHub activity. Analysis period: 2025-02-18 to 2026-02-18 |
Deploying corvus with
|
| Latest commit: |
d2722ae
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://011f09e6.corvus-42x.pages.dev |
| Branch Preview URL: | https://upgrade.corvus-42x.pages.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
clients/agent-runtime/src/security/landlock.rs (1)
128-133:⚠️ Potential issue | 🟠 Major
wrap_commandpermanently restricts the calling process — not just the spawned child.The comment correctly notes "This affects the current process". Landlock restrictions applied via
restrict_self()are permanent and irreversible for the current process. Every timewrap_commandis called (once per tool invocation),apply_restrictions()creates a new kernel ruleset and callsrestrict_self()again. While repeated calls are safe (stacking only tightens), the primary concern is that the long-running agent process is permanently restricted to{workspace, /tmp, /usr, /bin}after the first tool call — blocking subsequent agent-level I/O like reading configs, making network calls, etc.Consider:
- Applying the sandbox once at agent startup rather than inside
wrap_command, and tracking with aOnceflag.- Or documenting explicitly that
wrap_commandmust only be called once per process lifetime.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/agent-runtime/src/security/landlock.rs` around lines 128 - 133, wrap_command currently calls apply_restrictions (which ends up calling restrict_self) on every invocation, permanently tightening the calling process's Landlock and preventing later agent-level I/O; change the design so the Landlock is applied at agent startup (once) instead of per-tool call: move the call to apply_restrictions out of wrap_command and invoke it during agent initialization guarded by a std::sync::Once (or a boolean flag on the Landlock struct), or alternatively add clear documentation on Landlock::wrap_command that it must only be called once per process; update references to wrap_command, apply_restrictions, and restrict_self accordingly.clients/agent-runtime/src/tools/mod.rs (1)
258-295:⚠️ Potential issue | 🟡 MinorMissing test for
web_search.enabled = true; no empty-key guardTwo gaps:
- No test exercises the
root_config.web_search.enabled = truebranch — theWebSearchToolcode path is completely untested.- There is no guard preventing
WebSearchToolfrom being constructed with an emptybrave_api_key; ifenabled = truebut the key is unset, the tool silently registers and fails at runtime on first invocation.🛡️ Guard + suggested test skeleton
if root_config.web_search.enabled { + if root_config.web_search.brave_api_key.is_empty() { + tracing::warn!("web_search is enabled but brave_api_key is not configured; skipping WebSearchTool"); + } else { tools.push(Box::new(WebSearchTool::new( root_config.web_search.provider.clone(), root_config.web_search.brave_api_key.clone(), root_config.web_search.max_results, root_config.web_search.timeout_secs, ))); + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/agent-runtime/src/tools/mod.rs` around lines 258 - 295, Update all_tools to only construct/register WebSearchTool when root_config.web_search.enabled is true AND root_config.web_search.brave_api_key is non-empty (reject empty-string keys); reference the WebSearchTool construction site in all_tools and add a guard checking cfg.root_config.web_search.brave_api_key.trim().is_empty() to skip registration. Add two tests mirroring the existing all_tools_excludes_browser_when_disabled: one (e.g., all_tools_includes_web_search_when_enabled_with_key) that sets web_search.enabled = true and brave_api_key = "valid" and asserts names contains "web_search" (or the tool name), and another (e.g., all_tools_excludes_web_search_when_enabled_without_key) that sets web_search.enabled = true but brave_api_key = "" and asserts the web search tool is not registered.clients/agent-runtime/src/providers/reliable.rs (2)
398-409:⚠️ Potential issue | 🟡 MinorInconsistent failure message format in
chat_with_toolsvs other methods.
chat_with_system(line 197) andchat_with_history(line 302) use the updated"provider={name} model={model} attempt ..."format, butchat_with_toolsstill uses the old"{name}/{model} attempt ..."format. This will cause the aggregated error message to have mixed formats.🐛 Fix the format string to match other methods
failures.push(format!( - "{provider_name}/{current_model} attempt {}/{}: {failure_reason}", + "provider={provider_name} model={current_model} attempt {}/{}: {failure_reason}", attempt + 1, self.max_retries + 1 ));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/agent-runtime/src/providers/reliable.rs` around lines 398 - 409, The failures entry in chat_with_tools uses the old "{provider}/{model} attempt ..." format; update it to match chat_with_system and chat_with_history by formatting as "provider={provider_name} model={current_model} attempt {}/{}: {failure_reason}" and keep using attempt + 1 and self.max_retries + 1 for the attempt counters; modify the failures.push call (referencing failures, failure_reason, provider_name, current_model, attempt, self.max_retries) to use this new format string.
204-210:⚠️ Potential issue | 🔴 CriticalRemove API key logging in rate-limit rotation messages.
Lines 208, 312, and 416 log the last 4 characters of rotated API keys using
&new_key[new_key.len().saturating_sub(4)..]. This violates the security guideline: "Never log secrets, tokens, raw credentials, or sensitive payloads in any logging statements." API keys are credentials and should never be logged in any form, even partial. Remove the key value from these log messages entirely—log the rotation event without exposing any portion of the key.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/agent-runtime/src/providers/reliable.rs` around lines 204 - 210, The trace logs in the error-handling branch that call self.rotate_key() currently include the rotated API key slice (new_key and &new_key[new_key.len().saturating_sub(4)..]); remove any logging of the key material and instead log only the rotation event and context (e.g., provider_name). Update the tracing::info calls that reference new_key (from the rotate_key() call) to omit the key argument and adjust the message to something like "Rate limited, rotated API key" so no secret bytes are printed; ensure this change is applied to all occurrences that use rotate_key() and new_key in the file (the blocks that currently construct the key-ending slice).clients/agent-runtime/src/agent/agent.rs (1)
164-205:⚠️ Potential issue | 🔴 CriticalFix workspace configuration: missing
crates/robot-kitdirectory blocks all cargo operations.The
Cargo.tomldeclarescrates/robot-kitas a workspace member, but this directory does not exist. This preventscargo fmt,cargo clippy, andcargo testfrom running. Either create thecrates/robot-kitcrate or remove it from the workspace members list. Once this is resolved, standard Rust checks must pass before these changes can be merged.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/agent-runtime/src/agent/agent.rs` around lines 164 - 205, The workspace is failing because the workspace member "crates/robot-kit" declared in Cargo.toml is missing; fix by either creating a new crate at crates/robot-kit (with Cargo.toml and src/lib.rs or src/main.rs) or by removing "crates/robot-kit" from the workspace members list so cargo commands succeed; after making that change, run cargo fmt/clippy/test to ensure the project (including functions like Agent::build) compiles cleanly before merging.clients/agent-runtime/src/channels/telegram.rs (1)
1594-1604:⚠️ Potential issue | 🟡 MinorInline
sendChatActioninlistenusesreply_targetdirectly aschat_id
msg.reply_targetcan now be"chat_id:thread_id"(introduced by lines 666–670), but it is passed verbatim aschat_idtosendChatAction. Telegram rejects the malformed ID with 400; thelet _ = ...suppresses the error, so the one-shot typing indicator silently never fires for any forum-thread message.🐛 Proposed fix
- let typing_body = serde_json::json!({ - "chat_id": &msg.reply_target, - "action": "typing" - }); + let (typing_chat_id, typing_thread_id) = + Self::parse_reply_target(&msg.reply_target); + let mut typing_body = serde_json::json!({ + "chat_id": &typing_chat_id, + "action": "typing" + }); + if let Some(ref tid) = typing_thread_id { + typing_body["message_thread_id"] = + serde_json::Value::String(tid.clone()); + }🤖 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 1594 - 1604, The typing indicator in listen calls sendChatAction with msg.reply_target verbatim, but reply_target may be "chat_id:thread_id" for forum threads; split msg.reply_target on ':' and use only the chat_id portion when constructing typing_body so Telegram gets a valid chat_id (e.g., let chat_id = msg.reply_target.split_once(':').map(|(c,_)| c).unwrap_or(&msg.reply_target)), then build the JSON with that chat_id and call sendChatAction; update the sendChatAction/typing code block in listen to use this parsed chat_id (and maintain the existing ignored-error behavior).
|
|
||
| # CSPRNG for secure token generation | ||
| rand = "0.8" | ||
| rand = "0.9" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check for rand 0.8 patterns that are broken or deprecated in 0.9
rg -n --type=rust \
-e 'rand::distributions' \
-e 'thread_rng\(\)' \
-e 'OsRng\b' \
-e 'Uniform::new\b' \
-A 2Repository: dallay/corvus
Length of output: 1691
🏁 Script executed:
#!/bin/bash
# Check chacha20poly1305 crate version and its RNG trait requirements
rg -n --type=rust 'ChaCha20Poly1305::generate' clients/agent-runtime/src/ -B 2 -A 3Repository: dallay/corvus
Length of output: 1135
🏁 Script executed:
#!/bin/bash
# Check Cargo.toml for chacha20poly1305 version to understand API compatibility
grep -n 'chacha20poly1305' clients/agent-runtime/Cargo.tomlRepository: dallay/corvus
Length of output: 85
🌐 Web query:
chacha20poly1305 0.10 RNG trait TryRngCore compatibility rand 0.9
💡 Result:
chacha20poly1305 = 0.10.x is built on aead = 0.5, which pulls in crypto-common = 0.1.x and thus rand_core = 0.6 (via the rand_core feature). (docs.rs)
rand = 0.9 uses a newer rand_core where the new fallible RNG trait TryRngCore exists, and the trait ecosystem changed accordingly. (rust-random.github.io)
So an RNG type from rand 0.9 (i.e., rand_core 0.9) won’t satisfy bounds expecting rand_core 0.6 (what chacha20poly1305 0.10 expects), causing the compatibility error.
Ways to fix:
- Keep
chacha20poly1305 = 0.10→ userand = 0.8(rand_core 0.6) to match itsrand_coreversion. (docs.rs) - Move to the new RustCrypto prerelease line:
chacha20poly1305 = 0.11.0-rc.*depends onaead = 0.6.0-rc.*and is intended to work with the newerrand_core/os_rngecosystem. (docs.rs) - Bridge the traits with
rand_core_compat, which provides wrappers (includingTry*wrappers) to adapt betweenrand_core 0.6/rand 0.8andrand_core 0.9/rand 0.9. (docs.rs)
Citations:
- 1: https://docs.rs/crate/chacha20poly1305/0.10.1/source/Cargo.toml.orig
- 2: https://rust-random.github.io/book/update-0.9.html?utm_source=openai
- 3: https://docs.rs/crate/chacha20poly1305/0.10.1/source/Cargo.toml.orig
- 4: https://docs.rs/crate/chacha20poly1305/0.11.0-rc.1/source/Cargo.toml?utm_source=openai
- 5: https://docs.rs/rand_core_compat?utm_source=openai
Revert rand to 0.8 or upgrade chacha20poly1305 — incompatible trait bounds with rand 0.9
rand = 0.9 pulls in rand_core = 0.9, but chacha20poly1305 = 0.10 requires rand_core = 0.6. The trait bounds are incompatible and the code will not compile. The codebase uses OsRng with ChaCha20Poly1305::generate_nonce(), generate_key(), and fill_bytes() — all of which expect the older rand_core 0.6 API.
Choose one:
- Downgrade to
rand = 0.8to matchchacha20poly1305 0.10'srand_coredependency. - Upgrade to
chacha20poly1305 = 0.11.0-rc(prerelease) to supportrand_core 0.9. - Use
rand_core_compatas a bridge (adds an extra dependency).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@clients/agent-runtime/Cargo.toml` at line 69, The rand 0.9 entry causes
incompatible rand_core versions with chacha20poly1305 0.10; either revert rand
to 0.8 or upgrade chacha20poly1305 to a rand_core 0.9-compatible release so
OsRng and ChaCha20Poly1305::generate_nonce(), ::generate_key(), and fill_bytes()
keep working. Update Cargo.toml to change the dependency: replace rand = "0.9"
with rand = "0.8" (preferred) OR change chacha20poly1305 to a 0.11 (or rc that
supports rand_core 0.9); after editing run cargo update and cargo build to
ensure trait-bound errors are resolved.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/workflows/core-check.yml (1)
61-78: Remove the now-orphaned Rust toolchain setup and itsPATHprefix.Since
:agent-core-rust:checkwas dropped and only:agent-core-kmp:checkis now executed, the entire "Setup Rust toolchain" step (lines 61–73) and thePATH="$HOME/.cargo/bin:$PATH"prefix on line 78 are dead. If Rust isn't already cached on the runner this step will executecurl | shand install a full toolchain unnecessarily, wasting CI minutes on every KMP-only run.♻️ Proposed cleanup
Remove lines 61–73 entirely, and simplify line 78:
- - name: 🦀 Setup Rust toolchain - shell: bash - run: | - if command -v cargo >/dev/null 2>&1; then - cargo --version - rustc --version - exit 0 - fi - - curl https://sh.rustup.rs -sSf | sh -s -- -y --profile minimal --default-toolchain stable - echo "$HOME/.cargo/bin" >> "$GITHUB_PATH" - "$HOME/.cargo/bin/cargo" --version - "$HOME/.cargo/bin/rustc" --version - - name: ✅ Run core checks shell: bash run: | - PATH="$HOME/.cargo/bin:$PATH" ./gradlew :agent-core-kmp:check + ./gradlew :agent-core-kmp:check🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/core-check.yml around lines 61 - 78, Remove the entire "🦀 Setup Rust toolchain" CI step (the block that installs rustup/cargo and updates GITHUB_PATH) because :agent-core-rust:check was removed, and also remove the PATH="$HOME/.cargo/bin:$PATH" prefix before running ./gradlew :agent-core-kmp:check so the workflow no longer installs or prepends the Rust toolchain for KMP-only runs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.github/workflows/core-check.yml:
- Around line 61-78: Remove the entire "🦀 Setup Rust toolchain" CI step (the
block that installs rustup/cargo and updates GITHUB_PATH) because
:agent-core-rust:check was removed, and also remove the
PATH="$HOME/.cargo/bin:$PATH" prefix before running ./gradlew
:agent-core-kmp:check so the workflow no longer installs or prepends the Rust
toolchain for KMP-only runs.
1 participant
This pull request introduces a new query classification system to the agent runtime, enabling automatic selection of model hints based on user input. It also updates and adds dependencies, improves memory context relevance filtering, and enhances test and build configuration. The most important changes are grouped below.
Query Classification and Model Selection
classifiermodule (classifier.rs) implementing rule-based query classification, allowing the agent to auto-select model hints based on user messages. This includes priority-based rule matching, keyword/pattern support, and length constraints, with comprehensive tests. [1] [2]AgentandAgentBuilderstructs to includeclassification_configandavailable_hints, and integrated classification logic into the agent's message handling flow. Now, the agent can auto-select a model hint if the query matches a classification rule. [1] [2] [3] [4] [5] [6] [7] [8] [9]Memory Context Filtering
DefaultMemoryLoaderto support a configurable minimum relevance score, filtering out low-relevance memory entries from the context. This reduces noise and improves prompt quality. [1] [2]Dependency and Build Updates
tokio-util,urlencoding,ring, andcriterion(for async benchmarks); updated versions fordirectories,rand,cron,console, andrppal. Added a benchmarks section inCargo.toml. [1] [2] [3] [4] [5] [6] [7]Code Quality and Testing
/tmppaths, improving portability and reliability. [1] [2] [3]Summary by CodeRabbit
New Features
Improvements