feat(acp): ✨ Add ACP server support by HayWolf · Pull Request #214 · tiylabs/tiycode

HayWolf · 2026-05-27T23:44:45Z

Summary

Add TiyCode ACP server support with stdio and opt-in loopback HTTP/WebSocket entrypoints.
Bridge ACP sessions, prompts, streaming run events, tool updates, and permission requests into the existing Rust agent runtime.
Document ACP usage and update the queue promotion icon.

Test Plan

cargo fmt --check --manifest-path src-tauri/Cargo.toml
cargo test --locked --manifest-path src-tauri/Cargo.toml
Smoke test with an external ACP client

🤖 Generated with TiyCode

github-actions · 2026-05-27T23:51:41Z

AI Code Review Summary

PR: #214 (feat(acp): ✨ Add ACP server support)
Preferred language: English

Overall Assessment

Detected 11 actionable findings, prioritize CRITICAL/HIGH before merge.

Major Findings by Severity

CRITICAL (4)
- src-tauri/src/commands/system.rs:1149 - CLI uninstall may delete arbitrary files instead of only the symlink
- src-tauri/src/commands/system.rs:1199 - Command injection in macOS privileged symlink creation via osascript
- src-tauri/src/commands/system.rs:1227 - Command injection in Linux privileged symlink creation via pkexec
- src/modules/workbench-shell/ui/runtime-queue-timeline.tsx:149 - Missing import for ListStart icon
HIGH (1)
- src-tauri/src/core/agent_session_model_plan.rs:42 - Missing test for profile with disabled primary model in build_model_plan_from_profile
MEDIUM (6)
- src-tauri/src/commands/system.rs:1290 - Missing test for Windows CLI path manipulation helpers
- src-tauri/src/commands/system.rs:1403 - Windows PATH directory comparison is case-sensitive
- src-tauri/src/core/agent_session_model_plan.rs:22 - Missing test for resolve_active_profile_id fallback chains
- src-tauri/src/core/agent_session_model_plan.rs:79 - Missing test for lightweight role fallback chain
- src-tauri/src/core/app_event_emitter.rs:20 - Missing test for NoopAppEventEmitter silent discard
- src/modules/settings-center/ui/settings-center-overlay.tsx - Missing unit tests for CLI install/uninstall toggle in settings center

Actionable Suggestions

Validate that /usr/local/bin/tiycode is a symlink before deleting it in both uninstall and pre-install cleanup paths.
Use case-insensitive path comparison on Windows for PATH entries (e.g., by canonicalizing paths or converting to lowercase).
Add integration tests for the model plan builder and CLI installer to ensure correctness and prevent regressions.
Rewrite macOS/Linux CLI install/uninstall to avoid passing attacker-controlled paths to shell/script interpreters; use argument-vector APIs for privilege escalation or validate and canonicalize paths before use.
Replace the inline Windows FFI binding for WM_SETTINGCHANGE with the official windows crate to improve safety and auditability.
Strip or redact provider_key and custom_headers from the RuntimeModelPlan JSON before it is passed to session builders or logged.
Canonicalize current_exe() and symlink targets before comparisons in CLI install/uninstall to prevent path traversal.
Add a SpyAppEventEmitter in test code to record events emitted during agent runs and verify correct event types and payloads.
Add integration tests for build_model_plan_from_profile with disabled primary/auxiliary models, missing lightweight model, and empty profile list.
Add unit tests for resolve_active_profile_id covering all three fallback stages.
Update existing integration tests that construct AgentRunManager or AppState to use NoopAppEventEmitter instead of AppHandle.
Add unit tests for Windows PATH helpers (path_contains_dir, remove_dir_from_path) with edge cases like empty strings, trailing semicolons, and duplicate entries.

Potential Risks

If CLI uninstall is triggered with /usr/local/bin/tiycode being a regular file, the file will be deleted without user confirmation, potentially removing a native binary or other important tool.
Windows users with mixed-case PATH entries may experience duplicate additions or failed removals.
The CLI install feature is exposed via Tauri commands with no authorization check; any frontend invocation could trigger privilege escalation dialogs, potentially confusing users or being abused by malicious web content.
The model plan builder includes provider API keys and custom headers without any encryption at rest, making them available to any code path that reads the constructed plan.
All existing integration tests that construct AgentRunManager or AppState with an AppHandle will fail to compile because the constructor now expects AppEventEmitterRef.
The NoopAppEventEmitter silently discards all frontend events; if any runtime logic later depends on observing those events for cleanup or state transitions, ACP headless mode will have a regression.
The Windows registry write for user PATH could fail silently if the user does not have permission to HKCU\Environment, but the error is logged only as a string message—no retry or fallback is implemented.
The osascript privilege escalation may not work on macOS versions below 10.10 where 'with administrator privileges' was introduced; no fallback is documented or tested.
Undefined ListStart causes runtime crash in queue card.
CLI error messages may be shown incorrectly if platform-specific strings change.
CLI button visible but non-functional on web may confuse users.
Error handling in settings overlay relies on exact string matching for user cancellation; backend message changes may inadvertently show error UI.

Test Suggestions

Test resolve_active_profile_id with various settings DB states.
Test build_model_plan_from_profile with normal, missing, and disabled provider/model configurations.
Test the new Tauri app event emitter and no-op emitter.
Test CLI install/uninstall/check on macOS, Linux, and Windows (including permission failures and existing files).
Add regression test for build_model_plan_from_profile when provider records are disabled or missing.
Add fuzz test for CLI install/uninstall path inputs with random strings to ensure robustness.
Add integration test that runs AgentRunManager with the new AppEventEmitter in both Tauri and Noop modes.
Test concurrent agent run finalization with the new Arc-wrapped managers to confirm no deadlocks.
Write a mock AppEventEmitter that records all emitted events and verify that AgentRunManager emits expected events (THREAD_RUN_STARTED, THREAD_RUN_FINISHED, etc.).
Create integration tests for the ACP model plan flow: seed a profile in the database, call build_model_plan_from_profile, and verify the returned JSON matches the expected schema.
Add fuzz tests for parse_capabilities with random JSON inputs to ensure it never panics.
Add a test for install_cli_in_path that verifies the 'already installed' early-return path without actually modifying the filesystem.

File-Level Coverage Notes

src-tauri/src/core/mod.rs: Minimal module registration change; no test impact.
src-tauri/src/core/agent_session_model_plan.rs: New module with no dedicated tests. All core functions (resolve_active_profile_id, build_model_plan_from_profile, resolve_role_from_profile_fields, build_runtime_model_role, parse_capabilities, role_to_json) lack coverage. Critical for ACP headless sessions.
src-tauri/src/core/app_event_emitter.rs: New module with no tests. The NoopAppEventEmitter is critical for ACP headless mode, but its behavior is not validated.
src-tauri/src/core/app_state.rs: Refactored to use Arc wrappers and AppEventEmitterRef; existing integration tests should be updated to pass NoopAppEventEmitter instead of AppHandle. (This change may cause compilation failure in existing integration tests if not updated.)
src-tauri/src/core/agent_run_manager.rs: Refactored field from AppHandle to AppEventEmitterRef; no new test logic required beyond updating existing test constructor calls.
src-tauri/src/core/agent_run_event_handler.rs: Updated to use emit_app_event wrapper; should be covered by existing integration tests if they inject a mock AppEventEmitter that records events.
src-tauri/src/core/agent_run_compaction.rs: One-line change to pass app_events.as_ref(); no test impact beyond updating the AppState constructor.
src-tauri/src/core/agent_run_title.rs: Updated to use emit_app_event; compaction and title generation tests may need update to avoid depending on AppHandle.
src-tauri/src/commands/system.rs: New CLI-in-PATH commands added with extensive platform-specific code. No tests added for is_cli_installed, install_cli_in_path, uninstall_cli_from_path, or the Windows PATH helpers.
src/modules/workbench-shell/ui/runtime-queue-timeline.tsx: File restructured layout without changes to logic or async flows. No new testing concerns introduced beyond existing coverage gaps (if any). (Consider verifying the absolute positioning does not cause content overlap under different can* prop combinations, but that's a UI validation concern.)
src/modules/settings-center/ui/settings-center-overlay.tsx: New CLI toggle feature lacks any automated tests for its state management, async operations, and error handling.
src/i18n/locales/en.ts: i18n strings added; no functional code. Testing not applicable. (Ensure all added keys are used in the code to avoid dead keys.)
src/i18n/locales/zh-CN.ts: Same as en.ts; no testing concern.
src-tauri/Cargo.toml: File was in scope but not fully reviewed before budget limits. (planner_signaled_done_before_coverage)
src-tauri/src/acp/agent_handlers.rs: File was in scope but not fully reviewed before budget limits. (planner_signaled_done_before_coverage)
src-tauri/src/acp/event_bridge.rs: File was in scope but not fully reviewed before budget limits. (planner_signaled_done_before_coverage)
src-tauri/src/acp/mod.rs: File was in scope but not fully reviewed before budget limits. (planner_signaled_done_before_coverage)
src-tauri/src/acp/permission_bridge.rs: File was in scope but not fully reviewed before budget limits. (planner_signaled_done_before_coverage)
src-tauri/src/acp/session_map.rs: File was in scope but not fully reviewed before budget limits. (planner_signaled_done_before_coverage)
src-tauri/src/acp/transport.rs: File was in scope but not fully reviewed before budget limits. (planner_signaled_done_before_coverage)
... and 2 more file-level entries.

Inline Downgraded Items (processed but not inline)

src/modules/settings-center/ui/settings-center-overlay.tsx: Missing unit tests for CLI install/uninstall toggle in settings center (line_missing_or_invalid)

Coverage Status

Target files: 22
Covered files: 13
Uncovered files: 9
No-patch/binary covered as file-level: 0
Findings with unknown confidence (N/A): 0

Uncovered list:

src-tauri/Cargo.toml: planner_signaled_done_before_coverage
src-tauri/src/acp/agent_handlers.rs: planner_signaled_done_before_coverage
src-tauri/src/acp/event_bridge.rs: planner_signaled_done_before_coverage
src-tauri/src/acp/mod.rs: planner_signaled_done_before_coverage
src-tauri/src/acp/permission_bridge.rs: planner_signaled_done_before_coverage
src-tauri/src/acp/session_map.rs: planner_signaled_done_before_coverage
src-tauri/src/acp/transport.rs: planner_signaled_done_before_coverage
src-tauri/src/lib.rs: planner_signaled_done_before_coverage
src-tauri/src/main.rs: planner_signaled_done_before_coverage

No-patch covered list:

None

Runtime/Budget

Rounds used: 1/4
Planned batches: 3
Executed batches: 3
Sub-agent runs: 6
Planner calls: 1
Reviewer calls: 8
Model calls: 9/64
Structured-output summary-only degradation: NO

github-actions

Automated PR review completed.

Findings kept: 6
Findings with unknown confidence: 0
Inline comments attempted: 6
Target files: 17
Covered files: 17
Uncovered files: 0
See the summary comment for detailed analysis and coverage details.

Sign in to view

+    }
+}
+
+async fn request_permission(


Sign in to view

                }}
              >
-                {isPromoting ? <Loader2Icon className="size-3.5 animate-spin" /> : <Wand2Icon className="size-3.5" />}
+                {isPromoting ? <Loader2Icon className="size-3.5 animate-spin" /> : <ListStart className="size-3.5" />}


Sign in to view

+    agent_handlers::serve_connection(state, agent_client_protocol_tokio::Stdio::new()).await
+}
+
+pub fn spawn_http_server_if_configured(state: AcpServerState) {


Sign in to view

    Ok(())
 }

+pub fn run_acp_stdio() -> anyhow::Result<()> {


Sign in to view

+                        match task.stage {
+                            TaskStage::Pending => PlanEntryStatus::Pending,
+                            TaskStage::InProgress => PlanEntryStatus::InProgress,
+                            TaskStage::Completed | TaskStage::Failed => PlanEntryStatus::Completed,


Sign in to view

+        .await
+}
+
+fn is_loopback(ip: IpAddr) -> bool {


github-actions

Automated PR review completed.

Findings kept: 1
Findings with unknown confidence: 0
Inline comments attempted: 1
Target files: 17
Covered files: 17
Uncovered files: 0
See the summary comment for detailed analysis and coverage details.

Sign in to view

@@ -9,7 +9,6 @@ import {
  Loader2Icon,
  PencilIcon,
  Trash2Icon,


github-actions

Automated PR review completed.

Findings kept: 11
Findings with unknown confidence: 0
Inline comments attempted: 11
Target files: 17
Covered files: 17
Uncovered files: 0
See the summary comment for detailed analysis and coverage details.

Sign in to view

@@ -1,3 +1,4 @@
+pub mod acp;


Sign in to view

    Ok(())
 }

+pub fn run_acp_stdio() -> anyhow::Result<()> {


Sign in to view

                }}
              >
-                {isPromoting ? <Loader2Icon className="size-3.5 animate-spin" /> : <Wand2Icon className="size-3.5" />}
+                {isPromoting ? <Loader2Icon className="size-3.5 animate-spin" /> : <ListStart className="size-3.5" />}


Sign in to view

    status: RunStatus,
    error_message: Option<&str>,
 ) -> Result<(), AppError> {
    persist_final_run_state(pool, run_id, thread_id, status, error_message).await?;


Sign in to view

                // Broadcast a global event so the sidebar can pick up the new title even
                // when no per-run stream subscription exists (e.g. inactive threads).
-                let _ = app_handle.emit(
+                emit_app_event(


Sign in to view

+            let acp_state = crate::acp::AcpServerState::from_app_state(&state);
            app.manage(state);
            app.manage(desktop_runtime);
+            crate::acp::spawn_http_server(acp_state);


Sign in to view

+                    return Ok(PromptResponse::new(stop_reason));
+                }
+            }
+            Err(broadcast::error::RecvError::Lagged(dropped_events)) => {


Sign in to view

+    pub index_manager: Arc<IndexManager>,
+    pub git_manager: Arc<GitManager>,
+    pub extensions_manager: Arc<ExtensionsManager>,
+    pub app_events: AppEventEmitterRef,


Sign in to view

+                    .message_id(None),
+                ),
+            ))?;
+            return Ok(false);


Sign in to view

@@ -1,9 +1,11 @@
+use std::sync::Arc;


github-actions

Automated PR review completed.

Findings kept: 10
Findings with unknown confidence: 0
Inline comments attempted: 9
Target files: 17
Covered files: 17
Uncovered files: 0
See the summary comment for detailed analysis and coverage details.

Sign in to view

+        .on_receive_request(
+            {
+                async move |_request: AuthenticateRequest, responder, _cx| {
+                    responder.respond(AuthenticateResponse::new())


Sign in to view

+        ))
+}
+
+async fn handle_new_session(


Sign in to view

+const REJECT_ONCE_OPTION_ID: &str = "reject_once";
+const PERMISSION_TIMEOUT: Duration = Duration::from_secs(60);
+
+pub async fn request_permission_and_resolve(


Sign in to view

    Ok(())
 }

+pub fn run_acp_stdio() -> anyhow::Result<()> {


Sign in to view

+    request: ListSessionsRequest,
+) -> Result<ListSessionsResponse, agent_client_protocol::Error> {
+    let mut infos = Vec::new();
+    let workspaces = state.workspace_manager.list().await.map_err(to_acp_error)?;


Sign in to view

+        Some(thread.title),
+        Some(thread.last_active_at),
+    );
+    state.sessions.insert(record.clone()).await;


Sign in to view

+            message_id,
+            reasoning,
+            ..
+        } if !reasoning.trim().is_empty() => {


Sign in to view

            crate::core::settings_manager::apply_bundled_catalog_if_newer(app.handle());
            tracing::info!(elapsed_ms = t0.elapsed().as_millis(), "⏱ [startup] apply_bundled_catalog_if_newer");

+            let acp_state = crate::acp::AcpServerState::from_app_state(&state);


Sign in to view

 reqwest = { version = "0.12", default-features = false, features = ["json", "rustls-tls"] }

+# ACP server
+agent-client-protocol = { version = "0.11.1", features = ["unstable"] }


github-actions

Automated PR review completed.

Findings kept: 3
Findings with unknown confidence: 0
Inline comments attempted: 3
Target files: 17
Covered files: 17
Uncovered files: 0
See the summary comment for detailed analysis and coverage details.

Sign in to view

+        None => None,
+    };
+
+    for workspace in workspaces {


Sign in to view

+            agent_run_manager: Arc::clone(&state.agent_run_manager),
+            tool_gateway: Arc::clone(&state.tool_gateway),
+            terminal_manager: Arc::clone(&state.terminal_manager),
+            index_manager: Arc::new(IndexManager::new()),


Sign in to view

+        assert!(!is_loopback("0.0.0.0".parse().unwrap()));
+        assert!(!is_loopback("192.168.1.10".parse().unwrap()));
+        // Be conservative: IPv4-mapped IPv6 is not accepted as an ACP bind target.
+        assert!(!is_loopback("::ffff:127.0.0.1".parse().unwrap()));


- Add is_cli_installed, install_cli_in_path, uninstall_cli_from_path Tauri commands that create/remove a symlink at /usr/local/bin/tiycode pointing to the current app binary (macOS: osascript privilege escalation, Linux: pkexec) - Add CLI registration row in General Settings Preferences section with i18n support (en/zh-CN), describing ACP server usage - Refactor ACP transport: support --http <addr> alongside --stdio, expose run_http() as public API, simplify address parsing

…tion Reorganize the settings UI by moving the CLI installation row out of the Preferences section into a new "ACP Server" section with updated labels and descriptions that better describe both stdio and HTTP transport options. - Add acpServerTitle, acpServerCliLabel, acpServerCliDesc translation keys for English and Chinese locales - Move CLI install/uninstall row from Preferences to new ACP Server section for clearer logical grouping

Resolve active agent profile and construct RuntimeModelPlan on the backend so ACP (headless) sessions inherit the user's model configuration without relying on the frontend to supply it. - Resolve active profile ID when creating new ACP sessions - Build model plan from profile at prompt time as fallback - Add agent_session_model_plan module to core

Position action buttons absolutely in the top-right corner instead of as a flex sibling, so the message content area uses the full card width without reserving space for non-overlapping buttons.

github-actions

Automated PR review completed.

Findings kept: 4
Findings with unknown confidence: 0
Inline comments attempted: 4
Target files: 22
Covered files: 22
Uncovered files: 0
See the summary comment for detailed analysis and coverage details.

Sign in to view

+#[cfg(target_os = "macos")]
+fn create_symlink_privileged(target: &Path, link: &Path) -> Result<(), String> {
+    let script = format!(
+        "do shell script \"ln -sf '{}' '{}'\" with administrator privileges",


Sign in to view

                }}
              >
-                {isPromoting ? <Loader2Icon className="size-3.5 animate-spin" /> : <Wand2Icon className="size-3.5" />}
+                {isPromoting ? <Loader2Icon className="size-3.5 animate-spin" /> : <ListStart className="size-3.5" />}


Sign in to view

+/// Install the CLI by creating a symlink at `/usr/local/bin/tiycode`.
+/// On macOS, uses `osascript` to prompt for admin privileges when needed.
+#[tauri::command]
+pub fn install_cli_in_path() -> Result<String, String> {


Sign in to view

  // ── Settings: General Panel ────────────────────────────────
  "settings.general.newProfile": "New Profile",
  "settings.general.preferences": "Preferences",
+  "settings.general.cliLabel": "CLI in PATH",


github-actions

Automated PR review completed.

Findings kept: 6
Findings with unknown confidence: 0
Inline comments attempted: 3
Target files: 22
Covered files: 22
Uncovered files: 0
See the summary comment for detailed analysis and coverage details.

github-actions · 2026-05-28T04:17:23Z

                }}
              >
-                {isPromoting ? <Loader2Icon className="size-3.5 animate-spin" /> : <Wand2Icon className="size-3.5" />}
+                {isPromoting ? <Loader2Icon className="size-3.5 animate-spin" /> : <ListStart className="size-3.5" />}


[CRITICAL] Missing import for ListStart icon

The ListStart icon component from lucide-react is used but not imported, which will cause a TypeScript compilation error and possibly a runtime error.

Suggestion: Add 'ListStart' to the lucide-react import statement (e.g., import { ..., ListStart } from 'lucide-react';).

Risk: TypeScript compile error, application will likely fail to build or run.

Confidence: 0.95

[From SubAgent: general]

Sign in to view

+            other => panic!("unexpected updates: {other:?}"),
+        }
+    }
+}


Sign in to view

+            Ok(Message::Close(_)) => None,
+            // Axum handles websocket ping/pong control frames; ACP JSON-RPC
+            // payloads are carried only by text/binary data frames.
+            Ok(Message::Ping(_)) | Ok(Message::Pong(_)) => None,


…and visual rendering

github-actions

Automated PR review completed.

Findings kept: 10
Findings with unknown confidence: 0
Inline comments attempted: 8
Target files: 22
Covered files: 22
Uncovered files: 0
See the summary comment for detailed analysis and coverage details.

Sign in to view

+
+        // Fall back to privileged removal.
+        remove_cli_link_privileged(&link_path)?;
+        Ok("CLI uninstalled successfully.".into())


Sign in to view

+    }
+  }, []);
+
+  const handleToggleCli = async () => {


Sign in to view

+    setCliLoading(true);
+    try {
+      if (cliInstalled) {
+        await invoke<string>("uninstall_cli_from_path");


Sign in to view

+        let link_path = PathBuf::from(CLI_SYMLINK_PATH);
+
+        // If already correctly installed, return early.
+        if link_path.exists() || link_path.symlink_metadata().is_ok() {


Sign in to view

+    request: CloseSessionRequest,
+) -> Result<CloseSessionResponse, agent_client_protocol::Error> {
+    let record = ensure_session_loaded(&state, request.session_id.clone(), None).await?;
+    let _ = state.agent_run_manager.cancel_run(&record.thread_id).await;


Sign in to view

  // ── Settings: General Panel ────────────────────────────────
  "settings.general.newProfile": "New Profile",
  "settings.general.preferences": "Preferences",
+  "settings.general.cliLabel": "CLI in PATH",


Sign in to view

+        await invoke<string>("install_cli_in_path");
+        setCliInstalled(true);
+      }
+    } catch {


- Add TODO(security) comment for no-op ACP authentication handler (#2) - Escape single quotes in macOS osascript paths to prevent injection (#3) - Surface non-cancel CLI toggle errors in settings UI instead of silently swallowing them (#4)

github-actions

Automated PR review completed.

Findings kept: 11
Findings with unknown confidence: 0
Inline comments attempted: 10
Target files: 22
Covered files: 13
Uncovered files: 9
See the summary comment for detailed analysis and coverage details.

github-actions · 2026-05-28T06:28:55Z

+        }
+
+        // Try direct removal first.
+        if std::fs::remove_file(&link_path).is_ok() {


[CRITICAL] CLI uninstall may delete arbitrary files instead of only the symlink

The CLI uninstall functions do not verify that the target path is a symlink before deleting it, which could lead to accidental removal of unrelated files.

Suggestion: Use std::fs::symlink_metadata to confirm that the target is a symlink before attempting deletion. If it is not a symlink, refuse to uninstall and return an appropriate error message.

Risk: Blind file deletion may destroy important files, especially if the user or another tool has placed a binary at /usr/local/bin/tiycode. This is a local denial-of-service/data loss risk.

Confidence: 0.95

[From SubAgent: general]

github-actions · 2026-05-28T06:28:56Z

+}
+
+#[cfg(target_os = "macos")]
+fn create_symlink_privileged(target: &Path, link: &Path) -> Result<(), String> {


[CRITICAL] Command injection in macOS privileged symlink creation via osascript

Attackers who can influence the binary path (e.g., via symlink or executable location) can inject shell commands when the macOS CLI installer invokes osascript with administrator privileges.

Suggestion: Instead of embedding attacker-controlled paths directly in a shell string, construct the do shell script using a fixed, trusted format and pass paths through quoted form of AppleScript directive or use macOS security-scoped APIs. Avoid invoking a shell at all; use a direct privilege-escalation mechanism that accepts argument vectors (e.g., Security.framework).

Risk: Arbitrary command execution as root; complete system compromise during CLI installation.

Confidence: 0.95

[From SubAgent: security]

github-actions · 2026-05-28T06:28:56Z

+
+#[cfg(all(target_os = "linux", not(target_os = "macos")))]
+fn create_symlink_privileged(target: &Path, link: &Path) -> Result<(), String> {
+    let output = Command::new("pkexec")


[CRITICAL] Command injection in Linux privileged symlink creation via pkexec

Paths derived from environment or binary location are passed to pkexec ln and pkexec rm as arguments, which can still lead to unintended behavior if special characters or relative path tricks are used.

Suggestion: Validate that both paths are absolute, contain only safe characters, and match expected patterns before passing them to pkexec. Consider using Path::canonicalize() before constructing the command to resolve symlinks and relative segments.

Risk: Privilege escalation and arbitrary file operations if binary path is attacker-controlled.

Confidence: 0.95

[From SubAgent: security]

github-actions · 2026-05-28T06:28:56Z

+/// Returns `Err` if:
+/// - The profile does not exist
+/// - The profile's primary provider/model is missing or disabled
+pub async fn build_model_plan_from_profile(


[HIGH] Missing test for profile with disabled primary model in build_model_plan_from_profile

The critical error path for disabled primary models in ACP model plan construction has no test coverage. If the model plan silently fails, ACP sessions will not start.

Suggestion: Add integration tests that create a profile with a disabled primary provider/model and assert that build_model_plan_from_profile returns the expected AppError with the correct error code.

Risk: ACP users could receive cryptic 'Model plan missing' errors or silent crashes if the primary model is disabled, because the error is not validated in tests.

Confidence: 0.95

[From SubAgent: testing]

github-actions · 2026-05-28T06:28:56Z

+    }
+}
+
+// ---------------------------------------------------------------------------


[MEDIUM] Missing test for Windows CLI path manipulation helpers

The new Windows PATH manipulation functions (path_contains_dir and remove_dir_from_path) have no unit tests, making it hard to verify correctness across edge cases like trailing semicolons, duplicate entries, and case-insensitive directory comparison.

Suggestion: Add unit tests for path_contains_dir and remove_dir_from_path with various semicolon-separated PATH strings, including empty strings, trailing semicolons, and directory entries with different casing (since Windows paths are case-insensitive).

Risk: A bug in remove_dir_from_path could corrupt the user's PATH environment variable, breaking other applications that rely on it.

Confidence: 0.90

[From SubAgent: testing]

github-actions · 2026-05-28T06:28:56Z

+    path_value
+        .split(';')
+        .filter(|s| !s.is_empty())
+        .any(|entry| Path::new(entry) == dir)


[MEDIUM] Windows PATH directory comparison is case-sensitive

The PATH directory equality check is case-sensitive on Windows, causing potential misdetection of already-installed binaries.

Suggestion: Convert both paths to a canonical form (e.g., lowercase) or use fs::canonicalize and compare canonical paths, or use a case-insensitive comparison such as eq_ignore_ascii_case on the path components.

Risk: Duplicate PATH entries may be added or removal may miss existing entries, leading to a broken CLI experience on Windows.

Confidence: 0.90

[From SubAgent: general]

github-actions · 2026-05-28T06:28:56Z

+/// 1. `active_profile_id` from settings KV table
+/// 2. The first profile with `is_default = true` (list_all sorts by is_default DESC)
+/// 3. The first profile in the list (alphabetical fallback)
+pub async fn resolve_active_profile_id(pool: &SqlitePool) -> Result<Option<String>, AppError> {


[MEDIUM] Missing test for resolve_active_profile_id fallback chains

The new resolve_active_profile_id function has multiple fallback paths that are not validated in tests. The empty-settings and empty-profile-list paths could silently return None without any observable error.

Suggestion: Add integration tests in src-tauri/tests/ that seed the settings_repo with and without active_profile_id, and with empty or non-empty profile lists, and assert the correct profile ID is resolved or None is returned.

Risk: No debug assertion or logging when all fallbacks fail; could lead to silent failures in ACP mode where no profile is found and the model plan cannot be built.

Confidence: 0.90

[From SubAgent: testing]

github-actions · 2026-05-28T06:28:56Z

+    )
+    .await?;
+
+    // Resolve lightweight with fallback chain: explicit → auxiliary → primary


[MEDIUM] Missing test for lightweight role fallback chain

The lightweight role fallback logic lacks test coverage for the fallback chain, including the case where all three roles are undefined.

Suggestion: Add integration tests for profiles with only primary set, only auxiliary set, and no lightweight role, verifying the correct fallback role is used.

Risk: If the fallback chain has a logic bug, the lightweight model could be silently set to an incorrect role, affecting compaction and title generation behavior in ACP mode.

Confidence: 0.85

[From SubAgent: testing]

github-actions · 2026-05-28T06:28:56Z

+#[derive(Debug, Default)]
+pub struct NoopAppEventEmitter;
+
+impl AppEventEmitter for NoopAppEventEmitter {


[MEDIUM] Missing test for NoopAppEventEmitter silent discard

The NoopAppEventEmitter is crucial for ACP headless mode, but there is no test confirming that the system works without Tauri event delivery. This is a regression risk.

Suggestion: Add a test that initializes the agent run manager with a NoopAppEventEmitter and runs a basic session to completion, verifying that no panics or hangs occur due to missing event delivery.

Risk: If a future refactor assumes events are always delivered, ACP mode could crash or hang because the NoopAppEventEmitter silently discards everything.

Confidence: 0.85

[From SubAgent: testing]

jorben added 2 commits May 28, 2026 07:42

feat(acp): ✨ add ACP server support

0e01cf8

fix(ui): 🐛 use queue promotion icon

f481cbf

github-actions Bot reviewed May 27, 2026

View reviewed changes

fix(acp): 🐛 address ACP review feedback

15ab325

github-actions Bot reviewed May 28, 2026

View reviewed changes

Comment thread src/modules/workbench-shell/ui/runtime-queue-timeline.tsx

@@ -9,7 +9,6 @@ import {

Loader2Icon,

PencilIcon,

Trash2Icon,

This comment was marked as outdated.

Sign in to view

fix(acp): 🐛 resolve ACP review concerns

00a0751

github-actions Bot reviewed May 28, 2026

View reviewed changes

fix(acp): 🐛 cancel lagged ACP streams

b19c73a

github-actions Bot reviewed May 28, 2026

View reviewed changes

test(acp): ✅ add test for empty reasoning guard in event bridge

b14d571

github-actions Bot reviewed May 28, 2026

View reviewed changes

jorben added 4 commits May 28, 2026 11:08

fix(queue): 🐛 remove unnecessary right padding in queue message card

db3e8d5

Position action buttons absolutely in the top-right corner instead of as a flex sibling, so the message content area uses the full card width without reserving space for non-overlapping buttons.

github-actions Bot reviewed May 28, 2026

View reviewed changes

docs: 📝 add ACP Server feature to README and AGENTS docs

3d8c30b

github-actions Bot reviewed May 28, 2026

View reviewed changes

jorben added 3 commits May 28, 2026 12:33

fix(system): 🐛 enable Command import on Linux

c94b834

docs(skills): 📝 update TiyCode guide with ACP, subagents, web search …

702b137

…and visual rendering

feat(cli): ✨ implement Windows CLI-in-PATH install via registry

107d411

github-actions Bot reviewed May 28, 2026

View reviewed changes

fix(acp,cli): address PR #214 review feedback

9465016

- Add TODO(security) comment for no-op ACP authentication handler (#2) - Escape single quotes in macOS osascript paths to prevent injection (#3) - Surface non-cancel CLI toggle errors in settings UI instead of silently swallowing them (#4)

github-actions Bot reviewed May 28, 2026

View reviewed changes

jorben merged commit 2666174 into master May 28, 2026
3 of 4 checks passed

jorben deleted the feat/tiycode-acp-support branch May 28, 2026 06:49

Conversation

HayWolf commented May 27, 2026

Summary

Test Plan

Uh oh!

github-actions Bot commented May 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

AI Code Review Summary

Overall Assessment

Major Findings by Severity

Actionable Suggestions

Potential Risks

Test Suggestions

File-Level Coverage Notes

Inline Downgraded Items (processed but not inline)

Coverage Status

Runtime/Budget

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Uh oh!

This comment was marked as outdated.

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

github-actions Bot commented May 27, 2026 •

edited

Loading