feat: remaining gaps — shared context, cost per tool, validate-config#28
Merged
kienbui1995 merged 1 commit intomainfrom Apr 11, 2026
Merged
feat: remaining gaps — shared context, cost per tool, validate-config#28kienbui1995 merged 1 commit intomainfrom
kienbui1995 merged 1 commit intomainfrom
Conversation
- Subagent shared context: agents can see each other's results via SharedContext board injected into system prompt - Cost per tool: /cost now shows tool call counts breakdown - --validate-config: validate config and exit with summary - tool_call_counts tracked per tool name in TUI
📝 WalkthroughWalkthroughThis pull request introduces a shared context system for inter-task communication in subagents, adds per-tool call tracking to the TUI application, and implements a CLI flag for early configuration validation without starting the execution flow. Changes
Sequence Diagram(s)sequenceDiagram
participant TUI
participant SubagentSpawner
participant SharedContext
participant Agent
TUI->>SubagentSpawner: run_task(task_prompt, system_prompt)
activate SubagentSpawner
SubagentSpawner->>SharedContext: summary()
activate SharedContext
SharedContext-->>SubagentSpawner: context_summary
deactivate SharedContext
SubagentSpawner->>SubagentSpawner: build enriched_prompt<br/>(system_prompt + context_summary)
SubagentSpawner->>Agent: run_simple_agent(enriched_prompt)
activate Agent
Agent-->>SubagentSpawner: output/error
deactivate Agent
SubagentSpawner->>SharedContext: set(task_key, result)
activate SharedContext
SharedContext-->>SubagentSpawner: stored
deactivate SharedContext
SubagentSpawner-->>TUI: task_result
deactivate SubagentSpawner
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
mc/crates/mc-cli/src/main.rs (1)
382-382:⚠️ Potential issue | 🟡 MinorMultiple CI formatting failures throughout the file.
The CI reports formatting issues at lines 382, 790, 1109, 1419-1420, 1506, 1524, and 1531. Run
cargo fmtto fix all formatting issues in this file.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mc/crates/mc-cli/src/main.rs` at line 382, This file has multiple Rust formatting failures; run `cargo fmt` in the workspace to auto-fix formatting across mc-cli/src/main.rs (including the stated locations around the main function and other blocks where stray spacing/indentation exists, e.g., near the closing parenthesis/semicolon tokens shown in the diff) and commit the resulting changes; ensure rustfmt is configured consistently (rustfmt.toml if used) so subsequent CI runs pass.
🧹 Nitpick comments (1)
mc/crates/mc-core/src/subagent.rs (1)
24-28: Silent failure on lock poisoning inset().If the mutex is poisoned (a panic occurred while holding the lock),
set()silently discards the write. Consider logging when this happens.🔧 Proposed fix to add logging
pub fn set(&self, key: &str, value: &str) { - if let Ok(mut map) = self.entries.lock() { - map.insert(key.to_string(), value.to_string()); + match self.entries.lock() { + Ok(mut map) => { + map.insert(key.to_string(), value.to_string()); + } + Err(e) => { + tracing::warn!("SharedContext lock poisoned, recovering: {e}"); + e.into_inner().insert(key.to_string(), value.to_string()); + } } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mc/crates/mc-core/src/subagent.rs` around lines 24 - 28, The set() method currently ignores a poisoned mutex since it only handles Ok from self.entries.lock(); change it to detect Err from entries.lock() and log an error when the mutex is poisoned (including the key and a contextual message and the lock error), then still attempt to recover if appropriate (e.g., lock.into_inner() or map access) or return early; update references to entries.lock() and the map.insert(...) call in set() to include this logging so silent failures are visible.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@mc/crates/mc-core/src/subagent.rs`:
- Around line 88-94: Replace the redundant format!("{system_prompt}") call when
building enriched_prompt: use system_prompt.to_string() for the empty-shared
branch and build the combined string without the unnecessary named-format
pattern (e.g., system_prompt.to_string() + &shared or format!("{}{}",
system_prompt, shared)) so the code uses system_prompt.to_string() in the if
branch and a simple concatenation/format for the else branch; key symbols:
shared_context.summary(), enriched_prompt, system_prompt.
- Around line 108-113: The current use of
task_prompt.chars().take(50).collect::<String>() as task_key risks collisions
for prompts that share prefixes; change task_key generation in subagent (where
task_key is created) to derive a unique identifier instead—e.g., compute a
stable hash (sha256/xxhash) of the full task_prompt or combine a truncated
prefix with a short hash of task_prompt, then use that as the key passed to
self.shared_context.set; ensure both the Ok(output) and Err(e) branches use the
new key so results don't overwrite each other for different contexts.
---
Outside diff comments:
In `@mc/crates/mc-cli/src/main.rs`:
- Line 382: This file has multiple Rust formatting failures; run `cargo fmt` in
the workspace to auto-fix formatting across mc-cli/src/main.rs (including the
stated locations around the main function and other blocks where stray
spacing/indentation exists, e.g., near the closing parenthesis/semicolon tokens
shown in the diff) and commit the resulting changes; ensure rustfmt is
configured consistently (rustfmt.toml if used) so subsequent CI runs pass.
---
Nitpick comments:
In `@mc/crates/mc-core/src/subagent.rs`:
- Around line 24-28: The set() method currently ignores a poisoned mutex since
it only handles Ok from self.entries.lock(); change it to detect Err from
entries.lock() and log an error when the mutex is poisoned (including the key
and a contextual message and the lock error), then still attempt to recover if
appropriate (e.g., lock.into_inner() or map access) or return early; update
references to entries.lock() and the map.insert(...) call in set() to include
this logging so silent failures are visible.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8e144df2-6ce0-43c7-ad66-aa12cb008476
📒 Files selected for processing (5)
mc/crates/mc-cli/src/main.rsmc/crates/mc-core/src/lib.rsmc/crates/mc-core/src/subagent.rsmc/crates/mc-tui/src/app.rsmc/crates/mc-tui/src/commands.rs
Comment on lines
+88
to
+94
| // Inject shared context from other agents | ||
| let shared = self.shared_context.summary(); | ||
| let enriched_prompt = if shared.is_empty() { | ||
| format!("{system_prompt}") | ||
| } else { | ||
| format!("{system_prompt}{shared}") | ||
| }; |
There was a problem hiding this comment.
Redundant format! macro and formatting issue flagged by CI.
Line 91 uses format!("{system_prompt}") which is unnecessary—just use system_prompt.to_string(). This also addresses the CI formatting failure on line 42 (which appears to be a line number discrepancy in the CI output pointing to this format! call pattern).
🧹 Proposed fix
// Inject shared context from other agents
let shared = self.shared_context.summary();
let enriched_prompt = if shared.is_empty() {
- format!("{system_prompt}")
+ system_prompt.to_string()
} else {
format!("{system_prompt}{shared}")
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // Inject shared context from other agents | |
| let shared = self.shared_context.summary(); | |
| let enriched_prompt = if shared.is_empty() { | |
| format!("{system_prompt}") | |
| } else { | |
| format!("{system_prompt}{shared}") | |
| }; | |
| // Inject shared context from other agents | |
| let shared = self.shared_context.summary(); | |
| let enriched_prompt = if shared.is_empty() { | |
| system_prompt.to_string() | |
| } else { | |
| format!("{system_prompt}{shared}") | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@mc/crates/mc-core/src/subagent.rs` around lines 88 - 94, Replace the
redundant format!("{system_prompt}") call when building enriched_prompt: use
system_prompt.to_string() for the empty-shared branch and build the combined
string without the unnecessary named-format pattern (e.g.,
system_prompt.to_string() + &shared or format!("{}{}", system_prompt, shared))
so the code uses system_prompt.to_string() in the if branch and a simple
concatenation/format for the else branch; key symbols: shared_context.summary(),
enriched_prompt, system_prompt.
Comment on lines
+108
to
+113
| // Store result in shared context for other agents | ||
| let task_key = task_prompt.chars().take(50).collect::<String>(); | ||
| match &result { | ||
| Ok(output) => self.shared_context.set(&task_key, output), | ||
| Err(e) => self.shared_context.set(&task_key, &format!("error: {e}")), | ||
| } |
There was a problem hiding this comment.
Task key collision risk with truncated keys.
Using the first 50 characters of task_prompt as the key can cause collisions when multiple tasks share common prefixes. Per the context snippet from runtime.rs:920-948, task prompts are constructed as "{task}\n\nContext:\n{context}", so tasks with the same initial text but different contexts will overwrite each other's results.
Consider using a hash or including a unique identifier.
🔧 Proposed fix using hash
+use std::collections::hash_map::DefaultHasher;
+use std::hash::{Hash, Hasher};
// Store result in shared context for other agents
- let task_key = task_prompt.chars().take(50).collect::<String>();
+ let task_key = {
+ let mut hasher = DefaultHasher::new();
+ task_prompt.hash(&mut hasher);
+ format!("task_{:x}", hasher.finish())
+ };
match &result {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // Store result in shared context for other agents | |
| let task_key = task_prompt.chars().take(50).collect::<String>(); | |
| match &result { | |
| Ok(output) => self.shared_context.set(&task_key, output), | |
| Err(e) => self.shared_context.set(&task_key, &format!("error: {e}")), | |
| } | |
| use std::collections::hash_map::DefaultHasher; | |
| use std::hash::{Hash, Hasher}; | |
| // Store result in shared context for other agents | |
| let task_key = { | |
| let mut hasher = DefaultHasher::new(); | |
| task_prompt.hash(&mut hasher); | |
| format!("task_{:x}", hasher.finish()) | |
| }; | |
| match &result { | |
| Ok(output) => self.shared_context.set(&task_key, output), | |
| Err(e) => self.shared_context.set(&task_key, &format!("error: {e}")), | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@mc/crates/mc-core/src/subagent.rs` around lines 108 - 113, The current use of
task_prompt.chars().take(50).collect::<String>() as task_key risks collisions
for prompts that share prefixes; change task_key generation in subagent (where
task_key is created) to derive a unique identifier instead—e.g., compute a
stable hash (sha256/xxhash) of the full task_prompt or combine a truncated
prefix with a short hash of task_prompt, then use that as the key passed to
self.shared_context.set; ensure both the Ok(output) and Err(e) branches use the
new key so results don't overwrite each other for different contexts.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Subagent shared context (Tier 1)
Agents share results via
SharedContextboard. Each subagent's output is stored and injected into subsequent agents' system prompts.Cost per tool (Tier 2)
/costnow shows tool call counts:--validate-config (Tier 3)
magic-code --validate-config ✅ Config valid: anthropic provider, model claude-sonnet-4-20250514 MCP servers: 1 Hooks: 0 Tool permissions: {}180 tests pass.
Summary by CodeRabbit
--validate-configCLI flag to validate configuration and exit without launching the interface./costcommand to display per-tool call counts for improved cost tracking.