fix(discovery): merge all three VS Code scopes, and let $IRIS_NAMESPACE outrank the editor

Cargo.toml

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

README.md

@@ -71,11 +71,17 @@ 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` — 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
+>   `intersystems.servers` — from all **three** VS Code scopes, narrower winning: your user-scope
+>   settings, the nearest `*.code-workspace` above you, and this folder's `.vscode/settings.json`.
+>   That is what lets a folder which merely names a server (`"server": "workshop-iris"`) resolve
+>   against the definition kept in the multi-root `.code-workspace` two levels up — and win over
+>   whatever happens to answer on port 52773.
+>
+>   Two things stay under **your** control, not the editor's. `IRIS_PASSWORD` supplies the
+>   password VS Code does not expose (it keeps secrets in an encrypted store this binary cannot
+>   read), and **`IRIS_NAMESPACE` overrides the `ns` from settings** — so a deliberately
+>   read-only default namespace stays in force and a write that forgets to name its namespace
+>   still fails immediately. 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

@@ -16,12 +16,30 @@
 //! 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`.
+//! Step 4 reads all THREE VS Code scopes and merges them, narrower winning, as VS Code
+//! itself resolves a setting:
+//!   user      `dirs::config_dir()/{Code, Code - Insiders, VSCodium}/User/settings.json`
+//!   workspace the nearest `*.code-workspace` at or above the cwd (its `settings` block)
+//!   folder    `./.vscode/settings.json`
+//!
+//! #187: merging is the whole point, not an optimisation. The real shape is SPLIT — a
+//! multi-root workspace defines `intersystems.servers` once in the `.code-workspace`, and
+//! each folder's `.vscode/settings.json` only references it by name. Every file alone
+//! resolves to NotConfigured, so a reader that checks paths one at a time finds nothing no
+//! matter how many paths it checks.
+//!
+//! OPERATOR SCOPE OVERRIDES EDITOR SCOPE. `$IRIS_PASSWORD` supplies a password VS Code does
+//! not carry; `$IRIS_NAMESPACE` overrides the `ns` it does. The second is a guardrail, not a
+//! convenience: a workshop pins `IRIS_NAMESPACE=DONOTUSE` to a read-only namespace so a write
+//! that forgets to name its namespace fails AT ONCE rather than landing somewhere real.
+//! Taking `ns` from the editor would hand back a namespace that WORKS and delete that
+//! guardrail silently — measured at 95% of namespace-omitted calls failing loudly today,
+//! which is the intended behaviour.
+//!
+//! REMAINING LIMIT, still #187: no OS-keychain reader, and unlikely to gain one — VS Code
+//! keeps secrets in an encrypted store, not as per-server items an outside process can read.
+//! `$IRIS_PASSWORD` is the supported way in; an absent password is reported by name rather
+//! than replaced with `SYS`.
 
 use crate::iris::connection::{DiscoverySource, IrisConnection};
 use crate::iris::vscode_config::VsCodeResolution;
@@ -613,46 +631,124 @@ async fn discover_via_docker() -> Option<IrisConnection> {
     None
 }
 
-/// Resolve a connection from VS Code settings.json, if one is configured here.
+/// VS Code USER-scope settings paths — where Server Manager actually writes
+/// `intersystems.servers`.
 ///
-/// #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")];
+/// `dirs::config_dir()` is the correct root on all three platforms:
+/// macOS `~/Library/Application Support`, Linux `~/.config`, Windows `%APPDATA%`.
+fn vscode_user_settings_paths() -> Vec<std::path::PathBuf> {
+    let Some(base) = dirs::config_dir() else {
+        return Vec::new();
+    };
+    ["Code", "Code - Insiders", "VSCodium"]
+        .iter()
+        .map(|app| base.join(app).join("User").join("settings.json"))
+        .collect()
+}
 
-    for path in &candidates {
+/// Nearest `*.code-workspace` at or above `start`, which is where a multi-root workspace
+/// defines its servers. #187: the workshop keeps `intersystems.servers` here and references
+/// it by name from each folder's `.vscode/settings.json`, so a reader that never walks up
+/// finds a name it cannot resolve. `workspace_config` already walks up for
+/// `.iris-agentic-dev.toml`; this is the same move.
+fn find_code_workspace(start: &std::path::Path) -> Option<std::path::PathBuf> {
+    let mut dir = Some(start);
+    for _ in 0..12 {
+        let d = dir?;
+        let mut hits: Vec<_> = std::fs::read_dir(d)
+            .ok()?
+            .flatten()
+            .map(|e| e.path())
+            .filter(|p| p.extension().is_some_and(|e| e == "code-workspace"))
+            .collect();
+        if !hits.is_empty() {
+            hits.sort(); // deterministic when a folder holds more than one
+            return hits.into_iter().next();
+        }
+        dir = d.parent();
+    }
+    None
+}
+
+/// Resolve a connection from VS Code settings, merging user scope with the workspace.
+///
+/// #187: the real-world shape is SPLIT across two files. Server Manager writes the server
+/// definition into user-scope settings; the workspace `.vscode/settings.json` only names it
+/// (`"server": "workshop-iris"`). Each file alone resolves to `NotConfigured`, so this reads
+/// both and overlays the workspace on user scope before resolving once. Adding the user path
+/// as another candidate in a loop would have found nothing — that is the trap this shape sets.
+///
+/// A configured-but-passwordless entry returns `None` like an unconfigured one, but emits a
+/// warning naming the file and the server first.
+async fn discover_via_vscode_settings() -> Option<IrisConnection> {
+    fn load(path: &std::path::Path) -> Option<crate::iris::vscode_config::VsCodeSettings> {
         if !path.exists() {
-            continue;
+            return None;
         }
-        let settings = match crate::iris::vscode_config::parse_vscode_settings(path) {
-            Ok(s) => s,
+        match crate::iris::vscode_config::parse_vscode_settings(path) {
+            Ok(s) => Some(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;
+                None
             }
         }
     }
-    None
+
+    // User scope is the BASE: the first VS Code flavour that has a settings file wins.
+    let mut user = crate::iris::vscode_config::VsCodeSettings::default();
+    let mut user_path = None;
+    for path in vscode_user_settings_paths() {
+        if let Some(s) = load(&path) {
+            user = s;
+            user_path = Some(path);
+            break;
+        }
+    }
+
+    // Middle scope: the nearest .code-workspace at or above the cwd, where a multi-root
+    // workspace defines its servers.
+    let cwd = std::env::current_dir().ok();
+    let ws_file = cwd.as_deref().and_then(find_code_workspace);
+    let multi_root = ws_file
+        .as_deref()
+        .and_then(
+            |p| match crate::iris::vscode_config::parse_code_workspace(p) {
+                Ok(s) => Some(s),
+                Err(e) => {
+                    tracing::warn!("Could not parse {}: {}", p.display(), e);
+                    None
+                }
+            },
+        )
+        .unwrap_or_default();
+
+    // Narrowest scope: this folder's .vscode/settings.json.
+    let folder_path = cwd.map(|d| d.join(".vscode/settings.json"));
+    let folder = folder_path.as_deref().and_then(load).unwrap_or_default();
+
+    let named_in = folder_path.filter(|p| p.exists()).or(ws_file).or(user_path);
+
+    match folder.overlay_on(multi_root.overlay_on(user)).resolve() {
+        VsCodeResolution::Resolved(conn) => Some(conn),
+        VsCodeResolution::NotConfigured => None,
+        VsCodeResolution::MissingPassword { server } => {
+            let what = match server.as_deref() {
+                Some(name) => format!("server '{name}'"),
+                None => "a connection".to_string(),
+            };
+            tracing::warn!(
+                "VS Code settings configure {} but no password is available for it{}. Server \
+                 Manager keeps the password in the OS keychain, which this binary cannot read — \
+                 set IRIS_PASSWORD to supply it. Continuing discovery without this entry. \
+                 (issue #187)",
+                what,
+                named_in
+                    .map(|p| format!(" ({})", p.display()))
+                    .unwrap_or_default()
+            );
+            None
+        }
+    }
 }
 
 #[cfg(test)]

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

@@ -132,6 +132,25 @@ fn strip_jsonc(src: &str) -> String {
     out
 }
 
+/// A `.code-workspace` file: the same settings, nested under a `settings` key.
+#[derive(Debug, Deserialize, Default)]
+struct CodeWorkspaceFile {
+    settings: Option<VsCodeSettings>,
+}
+
+/// Parse the `settings` block of a `.code-workspace` file.
+///
+/// #187: this is where the workshop actually defines its server — not user-scope settings,
+/// and not `.vscode/settings.json`. A multi-root workspace names the server once here and
+/// each folder's `.vscode/settings.json` references it by name.
+pub fn parse_code_workspace(path: impl AsRef<Path>) -> anyhow::Result<VsCodeSettings> {
+    let content = std::fs::read_to_string(path.as_ref())?;
+    let parsed: CodeWorkspaceFile = serde_json::from_str(&content)
+        .or_else(|_| serde_json::from_str(&strip_jsonc(&content)))
+        .unwrap_or_default();
+    Ok(parsed.settings.unwrap_or_default())
+}
+
 /// Parse a VS Code settings.json file.
 /// Bug 10: the old parser only stripped full-line // comments.
 /// This version handles inline //, /* */ block comments, and trailing commas.
@@ -170,13 +189,58 @@ pub enum VsCodeResolution {
 }
 
 impl VsCodeSettings {
+    /// Overlay `self` (workspace scope) on top of `base` (user scope), the way VS Code
+    /// resolves a setting: the narrower scope wins, key by key.
+    ///
+    /// #187: this exists because the workshop shape is SPLIT across the two files and
+    /// neither half resolves alone. Server Manager writes `intersystems.servers` into
+    /// user-scope settings; the workspace `.vscode/settings.json` only references a
+    /// server by name. Reading either file on its own yields `NotConfigured`, so adding
+    /// the user path as one more candidate in the discovery loop would have changed
+    /// nothing — the scopes have to be merged BEFORE resolution.
+    ///
+    /// `objectscript.conn` is taken whole from the narrower scope that defines one, which
+    /// is what makes `"active": false` in a workspace a real opt-out rather than something
+    /// user scope can silently undo. `intersystems.servers` merges per key so a workspace
+    /// can override one server without having to restate the rest.
+    pub fn overlay_on(self, base: VsCodeSettings) -> VsCodeSettings {
+        let intersystems_servers = match (self.intersystems_servers, base.intersystems_servers) {
+            (Some(mine), Some(mut theirs)) => {
+                theirs.extend(mine); // workspace entries win per key
+                Some(theirs)
+            }
+            (mine, theirs) => mine.or(theirs),
+        };
+        VsCodeSettings {
+            objectscript_conn: self.objectscript_conn.or(base.objectscript_conn),
+            intersystems_servers,
+        }
+    }
+
     /// 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 {
+        self.resolve_with_env(env_password, None)
+    }
+
+    /// Resolve, letting the OPERATOR environment override what the EDITOR configured.
+    ///
+    /// `$IRIS_PASSWORD` fills a password VS Code does not carry; `$IRIS_NAMESPACE` overrides
+    /// the `ns` it does. #187: the second exists because a workshop pins
+    /// `IRIS_NAMESPACE=DONOTUSE` deliberately — a read-only namespace so that a write which
+    /// forgets to name its namespace fails AT ONCE instead of quietly landing in the wrong
+    /// one. Taking `ns` from `.vscode/settings.json` would hand back a namespace that WORKS
+    /// and silently delete that guardrail. Operator scope is the wider promise, so it wins.
+    pub fn resolve_with_env(
+        &self,
+        env_password: Option<&str>,
+        env_namespace: 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 env_namespace = env_namespace.filter(|n| !n.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() {
@@ -186,7 +250,7 @@ impl VsCodeSettings {
         if conn.active == Some(false) {
             return VsCodeResolution::NotConfigured;
         }
-        let ns = conn.ns.as_deref().unwrap_or("USER");
+        let ns = env_namespace.or(conn.ns.as_deref()).unwrap_or("USER");
 
         // Named server path — resolve through `intersystems.servers`.
         if let Some(server_name) = &conn.server {
@@ -255,6 +319,9 @@ impl 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())
+        self.resolve_with_env(
+            std::env::var("IRIS_PASSWORD").ok().as_deref(),
+            std::env::var("IRIS_NAMESPACE").ok().as_deref(),
+        )
     }
 }