feat: add Kiro CLI as a third AI agent#342
feat: add Kiro CLI as a third AI agent#342oksusucha wants to merge 12 commits intorefactoringhq:mainfrom
Conversation
Add kiro to AiAgentId type union, AI_AGENT_DEFINITIONS array, AiAgentsStatus interface, and all helper functions (normalize, factory, cycle).
Add kiro field to all AiAgentsStatus test fixtures across onboarding, status bar, badge, preferences, and registry tests.
Accept 'kiro' as a valid default_ai_agent value in normalize_default_ai_agent, with a roundtrip test.
- Add Kiro variant to AiAgentId enum and AiAgentsStatus - Detect kiro-cli binary via PATH, login shell, and known paths - Stream output via 'kiro-cli chat --no-interactive --trust-all-tools' - Strip ANSI escape codes from terminal-colored output - Auto-write .kiro/settings/mcp.json to inject Tolaria MCP server - Refactor codex binary search into shared helpers - Add tests for all new functions
|
Please check. |
Not up to standards ⛔🔴 Issues
|
| Category | Results |
|---|---|
| Complexity | 3 medium |
🟢 Metrics 678 complexity · 91 duplication
Metric Results Complexity 678 Duplication 91
AI Reviewer: first review requested successfully. AI can make mistakes. Always validate suggestions.
TIP This summary will be updated as you push new changes.
There was a problem hiding this comment.
Pull Request Overview
The integration of the Kiro CLI agent is functional but currently presents significant risks regarding user data integrity and execution stability. Specifically, the implementation overwrites the Kiro configuration file entirely, which will delete existing user settings, and uses command-line arguments for prompts, which will fail for large inputs due to OS limits. Furthermore, the PR does not meet general standards due to high duplication between CLI adapters and excessive file length in the backend.
Several critical logic errors regarding UI formatting (stripping necessary whitespace) must be addressed to ensure AI responses are readable. While binary detection and basic streaming are implemented, the logic for updating the MCP configuration must be shifted from overwriting to merging to fulfill the 'update' requirement safely.
About this PR
- There is a systemic pattern of duplication between the Kiro and Codex CLI adapters. Both manage process lifecycles and output streaming in similar but separate blocks. A unified process-runner abstraction is recommended to ensure consistent error handling and ANSI stripping across all CLI-based agents.
- The backend uses
.unwrap()during static JSON serialization. While unlikely to fail with the current hardcoded structures, proper error propagation viamap_errwould prevent potential panics in future iterations where configuration data might be dynamic.
Test suggestions
- Verify kiro-cli binary detection logic across multiple fallback paths.
- Ensure ANSI escape codes are correctly stripped from agent output lines.
- Validate that the .kiro/settings/mcp.json file is correctly generated with the current vault path.
- Verify that 'kiro' is accepted as a valid default agent in settings normalization.
- Confirm that authentication and login errors from the CLI are detected and formatted for the user.
- Verify the prompt builder correctly formats system instructions and user requests for Kiro.
TIP Improve review quality by adding custom instructions
TIP How was this review? Give us feedback
| fn write_kiro_mcp_json(vault_path: &str, mcp_server_path: &str) -> Result<(), String> { | ||
| let config = serde_json::json!({ | ||
| "mcpServers": { | ||
| "tolaria": { | ||
| "command": "node", | ||
| "args": [mcp_server_path], | ||
| "env": { "VAULT_PATH": vault_path }, | ||
| "disabled": false | ||
| } | ||
| } | ||
| }); | ||
|
|
||
| let config_dir = Path::new(vault_path).join(".kiro").join("settings"); | ||
| std::fs::create_dir_all(&config_dir) | ||
| .map_err(|e| format!("Failed to create .kiro/settings: {e}"))?; | ||
|
|
||
| let config_path = config_dir.join("mcp.json"); | ||
| std::fs::write(&config_path, serde_json::to_string_pretty(&config).unwrap()) | ||
| .map_err(|e| format!("Failed to write mcp.json: {e}"))?; | ||
|
|
||
| Ok(()) | ||
| } |
There was a problem hiding this comment.
🔴 HIGH RISK
write_kiro_mcp_json overwrites the entire .kiro/settings/mcp.json file. This will delete any existing MCP servers the user has manually configured in Kiro. The logic should read the existing file, parse it, and merge the tolaria entry into the mcpServers object.
| ] | ||
| } | ||
|
|
||
| fn run_kiro_agent_stream<F>(request: AiAgentStreamRequest, mut emit: F) -> Result<String, String> |
There was a problem hiding this comment.
🟡 MEDIUM RISK
The run_kiro_agent_stream function exceeds complexity limits and duplicates much of the Codex adapter's logic. Extracting a shared helper for command spawning and line-by-line processing is recommended.
| let clean = strip_ansi_codes(&line); | ||
| if !clean.is_empty() { | ||
| emit(AiAgentStreamEvent::TextDelta { | ||
| text: format!("{clean}\n"), | ||
| }); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🟡 MEDIUM RISK
This condition prevents empty lines and whitespace-only lines from being emitted to the frontend. Since AI responses rely on double-newlines for paragraph separation in Markdown, this causes the response to appear as a single block of text. Remove the !line.is_empty() and !clean.is_empty() checks.
| .arg("chat") | ||
| .arg("--no-interactive") | ||
| .arg("--trust-all-tools") | ||
| .arg(prompt) |
There was a problem hiding this comment.
🟡 MEDIUM RISK
Passing the prompt as a command-line argument is vulnerable to OS argument length limits (E2BIG). If a user provides a long message or pastes large context, the process will fail to spawn. Modify the implementation to pass the prompt via stdin.
| @@ -1,3 +1,4 @@ | |||
| use regex::Regex; | |||
There was a problem hiding this comment.
🟡 MEDIUM RISK
Suggestion: This file (776 lines) exceeds the maintainability limit. Consider splitting src-tauri/src/ai_agents.rs by moving provider-specific adapters (Claude, Codex, Kiro) into a new src-tauri/src/agents/ directory.
| fn strip_ansi_codes(input: &str) -> String { | ||
| let re = Regex::new(r"\x1b\[[0-9;]*m").unwrap(); | ||
| re.replace_all(input, "").to_string() | ||
| } |
There was a problem hiding this comment.
⚪ LOW RISK
Suggestion: Regex::new is compiled every time strip_ansi_codes is executed. For better performance, compile the regex only once using OnceLock.
| fn strip_ansi_codes(input: &str) -> String { | |
| let re = Regex::new(r"\x1b\[[0-9;]*m").unwrap(); | |
| re.replace_all(input, "").to_string() | |
| } | |
| fn strip_ansi_codes(input: &str) -> String { | |
| static RE: std::sync::OnceLock<Regex> = std::sync::OnceLock::new(); | |
| let re = RE.get_or_init(|| Regex::new(r"\x1b\[[0-9;]*m").unwrap()); | |
| re.replace_all(input, "").to_string() | |
| } |
Summary
Add Kiro CLI as a third supported AI agent alongside Claude Code and Codex, using the shared agent abstraction from ADR-0062.
4 commits + 1 docs commit:
feat: add kiro as third AI agent in frontend registry— addkirotoAiAgentId,AI_AGENT_DEFINITIONS, status interfaces, and all helper functions.test: update frontend tests for kiro agent support— add kiro field to status fixtures across 7 test files (onboarding, status bar, badge, preferences, registry).feat: add kiro to settings agent normalization— acceptkiroas a validdefault_ai_agentinnormalize_default_ai_agent, with roundtrip test.feat: add kiro CLI backend adapter with MCP injection— binary detection (PATH, login shell, known paths), streaming viakiro-cli chat --no-interactive --trust-all-tools, ANSI escape code stripping, auto-write.kiro/settings/mcp.jsonfor vault MCP tools. 6 new Rust tests.docs: add kiro to README, ARCHITECTURE, and ABSTRACTIONS— update supported agent lists and type definitions in docs.Why
I use kiro-cli daily. Tolaria supports Claude Code and Codex but not Kiro. The agent system (ADR-0062) was built to make this easy — one backend adapter, one frontend definition. This PR does that.
How it works
kiro-cliis installed (same as Claude/Codex).kiro-cli chat --no-interactive --trust-all-toolswith the note context..kiro/settings/mcp.jsonin the vault so kiro-cli can use vault tools (search_notes,get_note, etc.).Test plan
cargo test --lib ai_agents— 18 tests pass (6 new)cargo test --lib settings— 27 tests pass (1 new)pnpm test— 3290 tests pass, 7 test files updatedpnpm tauri dev: Kiro detected as installed, chat works, MCP tools availableNotes
useCliAiAgent/streamAiAgentinfra.--output-format stream-json) or Codex (--json). Output is plain text with ANSI codes, so the adapter streams line-by-line with color stripping..kiro/settings/mcp.json(not global config).