From 2d86175d505261152211ce5cac089e0cfa75b0dc Mon Sep 17 00:00:00 2001 From: m-szymanska Date: Sun, 5 Jul 2026 22:37:51 -0700 Subject: [PATCH] fix(onboarding): surface MCP setup prompt when config is absent (PR #52 smoke) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit probe_mcp_status returned a single neutral "no mcp.json" row even when no config existed, so the onboarding readiness check `!mcp.rows.isEmpty` was always true and the new mcpSetupPrompt (PR #52) was dead code. Add a `configured` flag on McpStatusReport: false when there is no config yet (missing mcp.json OR present-but-no-servers), true otherwise (including a present-but-broken config — the file exists, show the concrete error, not the onboarding prompt). Carry it through CsMcpStatusReport to Swift and gate the readiness step on `mcp.configured` instead of row count. Co-Authored-By: Claude Fable 5 --- app/agent/tools/mcp.rs | 37 ++++++++++++++++++- bridge/src/agent_status.rs | 6 +++ .../Screens/Onboarding/OnboardingSteps.swift | 2 +- .../Screens/Settings/AgentStatusEngine.swift | 1 + 4 files changed, 44 insertions(+), 2 deletions(-) diff --git a/app/agent/tools/mcp.rs b/app/agent/tools/mcp.rs index 91f469f0..f65bb7de 100644 --- a/app/agent/tools/mcp.rs +++ b/app/agent/tools/mcp.rs @@ -63,6 +63,13 @@ pub struct McpStatusRow { /// Honest snapshot of MCP config + runtime state for the Settings UI. pub struct McpStatusReport { pub config_path_display: String, + /// Whether the user has an `mcp.json` with at least one server defined. + /// `false` means "no config yet" (missing file OR present-but-empty) — the + /// onboarding readiness step surfaces the setup prompt instead of a status + /// card. A present config that fails to load still counts as `true`: the + /// file exists, the user configured something, so we show the concrete error + /// rather than pretend nothing is there. + configured: bool, rows: Vec, } @@ -71,9 +78,23 @@ impl McpStatusReport { &self.rows } - fn single(config_path_display: String, label: &str, value: String, tone: McpRowTone) -> Self { + /// `true` when an `mcp.json` exists with at least one server defined (or when + /// a present config failed to load — the file is still there). `false` only + /// when there is no config yet: missing file or present-but-no-servers. + pub fn configured(&self) -> bool { + self.configured + } + + fn single( + config_path_display: String, + configured: bool, + label: &str, + value: String, + tone: McpRowTone, + ) -> Self { Self { config_path_display, + configured, rows: vec![McpStatusRow { label: label.to_string(), value, @@ -95,6 +116,7 @@ pub fn probe_mcp_status() -> McpStatusReport { Err(error) => { return McpStatusReport::single( "unavailable".to_string(), + true, "Status:", format!("config path unavailable: {error}"), McpRowTone::Bad, @@ -110,6 +132,7 @@ fn probe_mcp_status_at(path: &Path) -> McpStatusReport { if !path.exists() { return McpStatusReport::single( config_path_display, + false, "Status:", "no mcp.json (optional — MCP off)".to_string(), McpRowTone::Neutral, @@ -121,6 +144,7 @@ fn probe_mcp_status_at(path: &Path) -> McpStatusReport { Err(error) => { return McpStatusReport::single( config_path_display, + true, "Config error:", anyhow_root_cause(&error), McpRowTone::Bad, @@ -131,6 +155,7 @@ fn probe_mcp_status_at(path: &Path) -> McpStatusReport { if config.servers.is_empty() { return McpStatusReport::single( config_path_display, + false, "Status:", "config present, no servers defined".to_string(), McpRowTone::Warn, @@ -170,6 +195,7 @@ fn probe_mcp_status_at(path: &Path) -> McpStatusReport { McpStatusReport { config_path_display, + configured: true, rows, } } @@ -708,6 +734,8 @@ mod tests { "got: {}", rows[0].value ); + // No config yet → the onboarding step must offer the setup prompt. + assert!(!report.configured()); } #[test] @@ -722,6 +750,9 @@ mod tests { assert_eq!(rows[0].tone, McpRowTone::Bad); assert_eq!(rows[0].label, "Config error:"); assert!(!rows[0].value.is_empty()); + // File exists (user configured something) → show the error, not the + // onboarding prompt. + assert!(report.configured()); } #[test] @@ -733,6 +764,8 @@ mod tests { let rows = report.summary_rows(); assert_eq!(rows[0].tone, McpRowTone::Warn); assert!(rows[0].value.contains("no servers")); + // Present but empty config counts as "not configured yet" → prompt. + assert!(!report.configured()); } #[test] @@ -762,6 +795,8 @@ mod tests { "got: {}", row.value ); + // A config with at least one server → configured; no setup prompt. + assert!(report.configured()); } #[tokio::test] diff --git a/bridge/src/agent_status.rs b/bridge/src/agent_status.rs index ed19c7fa..d525c52c 100644 --- a/bridge/src/agent_status.rs +++ b/bridge/src/agent_status.rs @@ -59,11 +59,16 @@ impl From<&McpStatusRow> for CsMcpStatusRow { #[derive(uniffi::Record)] pub struct CsMcpStatusReport { pub config_path_display: String, + /// `false` when there is no MCP config yet (missing `mcp.json` or a present + /// config with no servers). The onboarding readiness step uses this to choose + /// between the status card and the "set up MCP servers" prompt. + pub configured: bool, pub rows: Vec, } impl From for CsMcpStatusReport { fn from(report: McpStatusReport) -> Self { + let configured = report.configured(); let rows = report .summary_rows() .iter() @@ -71,6 +76,7 @@ impl From for CsMcpStatusReport { .collect(); Self { config_path_display: report.config_path_display, + configured, rows, } } diff --git a/macos/Codescribe/Screens/Onboarding/OnboardingSteps.swift b/macos/Codescribe/Screens/Onboarding/OnboardingSteps.swift index e5775403..0040b935 100644 --- a/macos/Codescribe/Screens/Onboarding/OnboardingSteps.swift +++ b/macos/Codescribe/Screens/Onboarding/OnboardingSteps.swift @@ -258,7 +258,7 @@ struct AgenticReadinessStepView: View { statusCard(rows: readiness.rows) } - if let mcp = model.mcpStatus, !mcp.rows.isEmpty { + if let mcp = model.mcpStatus, mcp.configured { Text("MCP servers") .font(CSFont.mono(10, .semibold)) .tracking(0.4) diff --git a/macos/Codescribe/Screens/Settings/AgentStatusEngine.swift b/macos/Codescribe/Screens/Settings/AgentStatusEngine.swift index 088c5289..8a0e8fbf 100644 --- a/macos/Codescribe/Screens/Settings/AgentStatusEngine.swift +++ b/macos/Codescribe/Screens/Settings/AgentStatusEngine.swift @@ -47,6 +47,7 @@ extension CsMcpStatusReport { /// Sample MCP status with a mix of live / pending servers (preview seed). static let sample = CsMcpStatusReport( configPathDisplay: "~/.codescribe/mcp.json", + configured: true, rows: [ CsMcpStatusRow(label: "loctree-mcp:", value: "9 tool(s)", tone: .good), CsMcpStatusRow(label: "aicx-mcp:", value: "configured (agent not started)", tone: .warn),