fix(discovery): run VS Code settings before the blind scans, and never invent a password

Cargo.toml

@@ -13,7 +13,7 @@ members = [
 resolver = "2"
 
 [workspace.package]
-version = "0.16.0"
+version = "0.17.0"
 edition = "2021"
 authors = ["Thomas Dyar <thomas.dyar@intersystems.com>"]
 license = "MIT"

README.md

@@ -71,8 +71,11 @@ Restart Claude and **verify with the `check_config` tool** that it connects and
 >   Claude Code and reads the same user-scope `~/.claude.json` registration shown above.
 > - **Connection settings from VS Code are read natively by this binary.** `iris-interop-dev` parses
 >   `.vscode/settings.json` — `objectscript.conn`, including named servers resolved through
->   `intersystems.servers` — as the last step of its discovery cascade. The order, and the known
->   limits of that step, are documented at the top of
+>   `intersystems.servers` — ahead of the blind localhost and Docker scans, so a workspace that
+>   names its server wins over whatever happens to answer on port 52773. The password is the one
+>   field it will not invent: Server Manager keeps it in the OS keychain, which this binary cannot
+>   read, so supply it with `IRIS_PASSWORD`. The full order, and the remaining limits, are at the
+>   top of
 >   [`discovery.rs`](crates/iris-agentic-dev-core/src/iris/discovery.rs) (issue #187).
 > - **What this fork does not carry is the Marketplace extension** that auto-registers the server,
 >   and `skill install --agent copilot`. That path belongs to the upstream community tool — see

crates/iris-agentic-dev-core/src/iris/discovery.rs

@@ -4,27 +4,27 @@
 //! 1. Explicit IrisConnection passed directly
 //! 2. Env vars (IRIS_HOST + IRIS_WEB_PORT)
 //! 3. IRIS_CONTAINER — named Docker container, web port resolved via bollard
-//! 4. Localhost port scan (100ms timeout, parallel), env-var credentials
-//! 5. Docker container scan via bollard
-//! 6. VS Code settings.json — `objectscript.conn`, resolving named servers through
+//! 4. VS Code settings.json — `objectscript.conn`, resolving named servers through
 //!    `intersystems.servers` (see `vscode_config.rs`)
+//! 5. Localhost port scan (100ms timeout, parallel), env-var credentials
+//! 6. Docker container scan via bollard
 //!
 //! Each step fails silently and falls through to the next.
 //!
-//! KNOWN LIMITS of step 6, tracked by issue #187. Recorded here because a reader
-//! concluded from a README sentence that no VS Code config path existed at all —
-//! the code is the only place that can answer that:
-//! - It runs LAST. Wherever IRIS answers on localhost or in Docker, steps 4-5 win
-//!   and settings.json is never opened. Step 3 already carries the equivalent
-//!   precedence fix for the named-container case; step 6 has no counterpart.
-//! - `discover_via_vscode_settings` searches exactly one path,
-//!   `current_dir()/.vscode/settings.json` — not user-scope settings, which is where
-//!   Server Manager normally keeps servers.
-//! - There is no OS-keychain reader. A server entry whose password lives in the
-//!   keychain arrives here with no password and defaults to _SYSTEM/SYS instead of
-//!   failing, producing a 401 that names no cause.
+//! Steps 3 and 4 are both EXPLICIT configuration and both sit ahead of the blind scans
+//! deliberately: they say which instance the user means, and a scan cannot tell one IRIS
+//! on 52773 from another. #187 moved step 4 up from last, where it could not run on any
+//! machine that had IRIS answering locally.
+//!
+//! REMAINING LIMIT of step 4, still tracked by #187: `discover_via_vscode_settings`
+//! searches exactly one path, `current_dir()/.vscode/settings.json` — not user-scope VS
+//! Code settings, which is where Server Manager normally keeps servers. A workspace that
+//! defines its own server is found; a server added through the Server Manager UI is not.
+//! There is still no OS-keychain reader either, but an absent password is now reported
+//! rather than replaced with `SYS`.
 
 use crate::iris::connection::{DiscoverySource, IrisConnection};
+use crate::iris::vscode_config::VsCodeResolution;
 use std::time::Duration;
 
 /// The ports we scan on localhost for IRIS web servers.
@@ -229,7 +229,17 @@ pub async fn discover_iris(explicit: Option<IrisConnection>) -> IrisDiscovery {
         }
     }
 
-    // 4. Localhost scan (parallel, 100ms each). Uses env var credentials.
+    // 4. VS Code settings.json — EXPLICIT configuration, so it runs before the blind
+    // scans for exactly the reason step 3 does: a settings.json naming a server states
+    // WHICH instance the user means, and a port scan cannot tell one IRIS on 52773 from
+    // another. #187: this used to be step 6, last, which made it unreachable on any
+    // machine where IRIS answered on localhost or in Docker — the code was present,
+    // tested, and never executed.
+    if let Some(conn) = discover_via_vscode_settings().await {
+        return IrisDiscovery::Found(conn);
+    }
+
+    // 5. Localhost scan (parallel, 100ms each). Uses env var credentials.
     let username = std::env::var("IRIS_USERNAME").unwrap_or_else(|_| "_SYSTEM".to_string());
     let password = std::env::var("IRIS_PASSWORD").unwrap_or_else(|_| "SYS".to_string());
     let namespace = std::env::var("IRIS_NAMESPACE").unwrap_or_else(|_| "USER".to_string());
@@ -262,16 +272,11 @@ pub async fn discover_iris(explicit: Option<IrisConnection>) -> IrisDiscovery {
         }
     }
 
-    // 5. Docker scan via bollard
+    // 6. Docker scan via bollard
     if let Some(conn) = discover_via_docker().await {
         return IrisDiscovery::Found(conn);
     }
 
-    // 6. VS Code settings.json
-    if let Some(conn) = discover_via_vscode_settings().await {
-        return IrisDiscovery::Found(conn);
-    }
-
     IrisDiscovery::NotFound
 }
 
@@ -608,17 +613,42 @@ async fn discover_via_docker() -> Option<IrisConnection> {
     None
 }
 
-/// Attempt to find IRIS connection from VS Code settings.json in common locations.
+/// Resolve a connection from VS Code settings.json, if one is configured here.
+///
+/// #187: a configured-but-passwordless entry returns `None` like an unconfigured one,
+/// but emits a warning naming the file and the server first. The two used to be
+/// indistinguishable because the missing password was filled in with `SYS`, so the
+/// cascade stopped on a connection that could only ever 401.
 async fn discover_via_vscode_settings() -> Option<IrisConnection> {
     let candidates = [std::env::current_dir().ok()?.join(".vscode/settings.json")];
 
     for path in &candidates {
         if !path.exists() {
             continue;
         }
-        if let Ok(settings) = crate::iris::vscode_config::parse_vscode_settings(path) {
-            if let Some(conn) = settings.to_iris_connection().await {
-                return Some(conn);
+        let settings = match crate::iris::vscode_config::parse_vscode_settings(path) {
+            Ok(s) => s,
+            Err(e) => {
+                tracing::warn!("Could not parse {}: {}", path.display(), e);
+                continue;
+            }
+        };
+        match settings.resolve() {
+            VsCodeResolution::Resolved(conn) => return Some(conn),
+            VsCodeResolution::NotConfigured => continue,
+            VsCodeResolution::MissingPassword { server } => {
+                let what = match server.as_deref() {
+                    Some(name) => format!("server '{name}'"),
+                    None => "a connection".to_string(),
+                };
+                tracing::warn!(
+                    "{} configures {} but carries no password. VS Code Server Manager keeps it in \
+                     the OS keychain, which this binary cannot read — set IRIS_PASSWORD to supply \
+                     it. Continuing discovery without this entry. (issue #187)",
+                    path.display(),
+                    what
+                );
+                continue;
             }
         }
     }

crates/iris-agentic-dev-core/src/iris/vscode_config.rs

@@ -147,18 +147,58 @@ pub fn parse_vscode_settings(path: impl AsRef<Path>) -> anyhow::Result<VsCodeSet
     Ok(settings)
 }
 
+/// Outcome of resolving parsed settings into a connection.
+///
+/// #187: this used to be a bare `Option<IrisConnection>`, which collapsed "nothing is
+/// configured here" and "a connection IS configured but its password is missing" into
+/// the same `None` — and then avoided the second case entirely by defaulting the
+/// password to `SYS`. Those are different answers and the caller needs them apart.
+#[derive(Debug)]
+pub enum VsCodeResolution {
+    /// A complete connection was resolved.
+    Resolved(IrisConnection),
+    /// Nothing is configured here: no `objectscript.conn`, `active: false`, or a
+    /// `server:` name with no matching `intersystems.servers` entry.
+    NotConfigured,
+    /// A connection is configured but no password is available for it.
+    ///
+    /// This is the Server Manager case. The VS Code extension keeps the password in
+    /// the OS keychain, so it is absent from settings.json, and this binary has no
+    /// keychain reader. Fabricating `SYS` here produced a connection that looked
+    /// configured and was wrong — a 401 naming no cause.
+    MissingPassword { server: Option<String> },
+}
+
 impl VsCodeSettings {
-    /// Convert parsed settings to an IrisConnection, resolving named servers.
-    pub async fn to_iris_connection(&self) -> Option<IrisConnection> {
-        let conn = self.objectscript_conn.as_ref()?;
+    /// Resolve to a connection, taking the fallback password from the caller.
+    ///
+    /// Pure — `resolve()` supplies `$IRIS_PASSWORD`. Split so tests can cover the
+    /// credential rules without mutating process environment.
+    pub fn resolve_with(&self, env_password: Option<&str>) -> VsCodeResolution {
+        // An empty string is an absent password, not a password of length zero.
+        let env_password = env_password.filter(|p| !p.is_empty());
+        let present = |v: Option<&str>| v.filter(|p| !p.is_empty()).map(str::to_owned);
+
+        let conn = match self.objectscript_conn.as_ref() {
+            Some(c) => c,
+            None => return VsCodeResolution::NotConfigured,
+        };
         if conn.active == Some(false) {
-            return None;
+            return VsCodeResolution::NotConfigured;
         }
+        let ns = conn.ns.as_deref().unwrap_or("USER");
 
-        // Named server path
+        // Named server path — resolve through `intersystems.servers`.
         if let Some(server_name) = &conn.server {
-            let servers = self.intersystems_servers.as_ref()?;
-            let server = servers.get(server_name)?;
+            let server = match self
+                .intersystems_servers
+                .as_ref()
+                .and_then(|servers| servers.get(server_name))
+            {
+                Some(s) => s,
+                // A name with no entry is a typo, not a licence to guess localhost.
+                None => return VsCodeResolution::NotConfigured,
+            };
             let host = server.web_server.host.as_deref().unwrap_or("localhost");
             let web_port = server.web_server.port.unwrap_or(52773);
             let scheme = server.web_server.scheme.as_deref().unwrap_or("http");
@@ -173,35 +213,48 @@ impl VsCodeSettings {
             } else {
                 format!("{}://{}:{}/{}", scheme, host, web_port, path_prefix)
             };
+            // Username is not a secret and `_SYSTEM` is this codebase's documented
+            // default everywhere else; the password is the field Server Manager
+            // deliberately does not write here, so it is the one we refuse to invent.
             let username = server.username.as_deref().unwrap_or("_SYSTEM");
-            let password = server.password.as_deref().unwrap_or("SYS");
-            let ns = conn.ns.as_deref().unwrap_or("USER");
-
-            let iris_conn = IrisConnection::new(
+            let password =
+                match present(server.password.as_deref()).or_else(|| present(env_password)) {
+                    Some(p) => p,
+                    None => {
+                        return VsCodeResolution::MissingPassword {
+                            server: Some(server_name.clone()),
+                        }
+                    }
+                };
+            return VsCodeResolution::Resolved(IrisConnection::new(
                 base_url,
                 ns,
                 username,
                 password,
                 DiscoverySource::VsCodeSettings,
-            );
-            // Note: super_server_port is available if needed for native connections
-            return Some(iris_conn);
+            ));
         }
 
-        // Direct host/port path
+        // Direct host/port path.
         let host = conn.host.as_deref().unwrap_or("localhost");
         let port = conn.port.unwrap_or(52773);
         let username = conn.username.as_deref().unwrap_or("_SYSTEM");
-        let password = conn.password.as_deref().unwrap_or("SYS");
-        let ns = conn.ns.as_deref().unwrap_or("USER");
-        let base_url = format!("http://{}:{}", host, port);
-
-        Some(IrisConnection::new(
-            base_url,
+        let password = match present(conn.password.as_deref()).or_else(|| present(env_password)) {
+            Some(p) => p,
+            None => return VsCodeResolution::MissingPassword { server: None },
+        };
+        VsCodeResolution::Resolved(IrisConnection::new(
+            format!("http://{}:{}", host, port),
             ns,
             username,
             password,
             DiscoverySource::VsCodeSettings,
         ))
     }
+
+    /// Resolve to a connection, falling back to `$IRIS_PASSWORD` for the secret that
+    /// Server Manager keeps in the OS keychain.
+    pub fn resolve(&self) -> VsCodeResolution {
+        self.resolve_with(std::env::var("IRIS_PASSWORD").ok().as_deref())
+    }
 }