Summary
There's significant duplication between MCP server mode and REPL mode. This makes maintenance harder and leads to inconsistencies (e.g., #34 cancellation was MCP-only).
Current Duplication
| Concern |
MCP |
REPL |
| Session management |
McpSession HashMap |
interaction_id variable |
| Output |
JSON response + file log |
termimad to terminal |
| Cancellation |
Task abort (#34) |
CANCELLED AtomicBool |
| Tool execution |
Same run_interaction() |
Same |
Proposed Unification
1. Output Sink Trait
trait OutputSink: Send + Sync {
fn emit_text(&self, text: &str);
fn emit_tool_call(&self, name: &str, args: &Value, duration: Duration);
fn emit_error(&self, error: &str);
}
struct TerminalSink { skin: MadSkin }
struct McpSink { tx: Sender<String> }
struct FileSink { file: File }
2. Unified Session Manager
struct SessionManager {
sessions: HashMap<String, Session>,
}
struct Session {
id: String,
interaction_id: Option<String>,
cancellation: CancellationToken,
}
3. Shared Cancellation
Use tokio_util::sync::CancellationToken instead of AtomicBool. Can be cloned and checked at any await point.
Scope
This is a significant refactor. Consider:
- Incremental migration (one concern at a time)
- Keep
run_interaction() as the shared core
- Add abstractions only where duplication causes bugs
Related
Summary
There's significant duplication between MCP server mode and REPL mode. This makes maintenance harder and leads to inconsistencies (e.g., #34 cancellation was MCP-only).
Current Duplication
McpSessionHashMapinteraction_idvariablerun_interaction()Proposed Unification
1. Output Sink Trait
2. Unified Session Manager
3. Shared Cancellation
Use
tokio_util::sync::CancellationTokeninstead of AtomicBool. Can be cloned and checked at any await point.Scope
This is a significant refactor. Consider:
run_interaction()as the shared coreRelated