diff --git a/.github/dependabot.yml b/.github/dependabot.yml index 52678b5..1494d03 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml @@ -4,7 +4,7 @@ version: 2 updates: - package-ecosystem: cargo - directory: /runtime + directory: / schedule: interval: weekly open-pull-requests-limit: 5 diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1b98d59..adebc1b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -34,7 +34,7 @@ jobs: components: rustfmt, clippy - uses: Swatinem/rust-cache@6323deb102c322ba6fcbdcafc7e3dddab59af2b6 # v2 with: - workspaces: runtime + workspaces: . # Ordered so a formatting nit cannot mask a test failure. The previous # workflow put fmt first and failed fast, and the result was that clippy diff --git a/runtime/Cargo.lock b/Cargo.lock similarity index 100% rename from runtime/Cargo.lock rename to Cargo.lock diff --git a/Cargo.toml b/Cargo.toml new file mode 100644 index 0000000..4298195 --- /dev/null +++ b/Cargo.toml @@ -0,0 +1,7 @@ +# The repo is one crate today — the runtime in `runtime/` — but the workspace +# lives at the root so `cargo test --workspace`, `cargo fmt --all` and +# `cargo clippy --workspace` work from the top of a checkout instead of only +# inside the member directory. +[workspace] +members = ["runtime"] +resolver = "2" diff --git a/Dockerfile b/Dockerfile index b62afa6..e7b0b3f 100644 --- a/Dockerfile +++ b/Dockerfile @@ -30,20 +30,22 @@ RUN apt-get update \ && rustup target add x86_64-unknown-linux-musl WORKDIR /src -COPY runtime/Cargo.toml runtime/Cargo.lock ./ +COPY Cargo.toml Cargo.lock ./ +COPY runtime/Cargo.toml ./runtime/ # Prime the dependency layer against the manifests alone, so editing src/ does # not re-download and rebuild the tree. -RUN mkdir -p src && echo 'fn main() {}' > src/main.rs && echo '' > src/lib.rs \ +RUN mkdir -p runtime/src && echo 'fn main() {}' > runtime/src/main.rs \ + && echo '' > runtime/src/lib.rs \ && cargo build --release --locked --target x86_64-unknown-linux-musl \ - && rm -rf src + && rm -rf runtime/src -COPY runtime/src ./src +COPY runtime/src ./runtime/src # The touch is load-bearing, not tidiness. Cargo decides freshness by mtime, COPY # preserves the context's timestamps, and if those land older than the stub # artifacts above then cargo declares the stub build fresh and the image ships a # binary whose main() does nothing -- a failure that builds green and only shows # up as a container that exits instantly. -RUN touch src/main.rs src/lib.rs \ +RUN touch runtime/src/main.rs runtime/src/lib.rs \ && cargo build --release --locked --target x86_64-unknown-linux-musl \ && strip target/x86_64-unknown-linux-musl/release/openab-pty diff --git a/Dockerfile.nightly b/Dockerfile.nightly index 5ce2e5c..e923515 100644 --- a/Dockerfile.nightly +++ b/Dockerfile.nightly @@ -49,19 +49,21 @@ RUN apt-get update \ && rustup target add "$(cat /rust-target)" WORKDIR /src -COPY runtime/Cargo.toml runtime/Cargo.lock ./ -RUN mkdir -p src && echo 'fn main() {}' > src/main.rs && echo '' > src/lib.rs \ +COPY Cargo.toml Cargo.lock ./ +COPY runtime/Cargo.toml ./runtime/ +RUN mkdir -p runtime/src && echo 'fn main() {}' > runtime/src/main.rs \ + && echo '' > runtime/src/lib.rs \ && cargo build --release --locked --target "$(cat /rust-target)" \ - && rm -rf src + && rm -rf runtime/src -COPY runtime/src ./src +COPY runtime/src ./runtime/src # The touch is load-bearing: cargo decides freshness by mtime and COPY preserves # the context's timestamps, so without it the stub build above is declared fresh # and the image ships a binary whose main() does nothing. # # The built artifact is copied to an arch-independent path so the final-stage # COPY does not need to know the triple. -RUN touch src/main.rs src/lib.rs \ +RUN touch runtime/src/main.rs runtime/src/lib.rs \ && cargo build --release --locked --target "$(cat /rust-target)" \ && strip "target/$(cat /rust-target)/release/openab-pty" \ && cp "target/$(cat /rust-target)/release/openab-pty" /openab-pty diff --git a/docs/k8s-howto.md b/docs/k8s-howto.md index 50a5a9f..89b720d 100644 --- a/docs/k8s-howto.md +++ b/docs/k8s-howto.md @@ -142,8 +142,10 @@ tailscale status | grep openab-pty # find the address ## 6. Lending a Mac to a session (optional) With `PTY_TOOLS_LISTEN` set (the manifest sets `127.0.0.1:8091`), a Mac running -`oab-instance-mcp` can **dial in** and lend its tools to one session; the coding -CLI inside that session then finds them at the URL in `$OPENAB_TOOLS_MCP_URL`. +`oab-instance-mcp` can **dial in** and lend its tools to one session. The coding +CLI inside that session finds them at the URL in `$OPENAB_TOOLS_MCP_URL` — and +on a known variant (today: `kiro-cli`) the spawn already wrote the `computer` +server and its `allowedTools` trust, so no `mcp add` step exists at all. The pod initiates nothing and stores only a hash. Design: [reverse attach](https://github.com/openabdev/instance-mcp/blob/main/docs/adr/reverse-attach.md); wire contract: §9 of [`../runtime/CLIENT-CONTRACT.md`](../runtime/CLIENT-CONTRACT.md). diff --git a/runtime/CLIENT-CONTRACT.md b/runtime/CLIENT-CONTRACT.md index 5463657..e99c420 100644 --- a/runtime/CLIENT-CONTRACT.md +++ b/runtime/CLIENT-CONTRACT.md @@ -421,10 +421,55 @@ does not edit any CLI's config; whatever installs the CLI does that. Properties: #### Wiring the URL into the coding CLI -The runtime sets the env vars; it does **not** edit the CLI's config. Whatever owns -the session's workspace does that. +The runtime sets the env vars **and** — for the CLI variants it knows — writes +their config at spawn too, so the session's agent starts with the `computer` +server present and its tools pre-trusted. A variant it does not know is left +entirely alone and still works through the env vars and the manual steps below. + +**What the runtime writes (kiro-cli).** At every spawn, when `kiro-cli` is on the +child's PATH (or under the installer's `~/.local` layout) and a JS runtime exists +to run the bridge: + +- `~/.local/share/openab-pty/computer-mcp-bridge.js` — a small stdio MCP + bridge the runtime owns and rewrites each spawn. Every stdin line is one + JSON-RPC message, POSTed to `$OPENAB_TOOLS_MCP_URL`; it handles a JSON or SSE + reply and echoes `Mcp-Session-Id`. +- `~/.kiro/settings/mcp.json` gains (or refreshes) exactly the `computer` + server — every other server and setting in the file is preserved: -**Preferred — one config for every session.** The file never changes, across +```json +{ + "mcpServers": { + "computer": { + "command": "", + "args": ["/.local/share/openab-pty/computer-mcp-bridge.js"], + "env": { "OPENAB_TOOLS_MCP_URL": "${OPENAB_TOOLS_MCP_URL}" } + } + } +} +``` + +- every `~/.kiro/agents/*.json` gains `@computer/*` in `allowedTools` — the + served set under the platform-neutral alias — plus `@computer` in a + restrictive `tools` list, and an agent that opted out of mcp.json + (`includeMcpJson: false`) gets the `computer` entry merged into its own + `mcpServers`. Agent files are never *created*: a session with no agent file + keeps the manual trust path below, which is safer than writing one that + could shadow a built-in agent. + +**Why a bridge and not a URL.** The workspace `mcp.json` is shared by every +session in the pod, so it cannot name one session's `/mcp//` +URL — the key rotates at each spawn, and a hard-coded URL silently dials the +wrong session after a re-lend. The bridge form is session-independent: the +config forwards `$OPENAB_TOOLS_MCP_URL` through `env`, and each CLI process +reads its *own* value. Rotation therefore needs no rewrite — there is nothing +in the file to refresh. + +If a spawn finds no `kiro-cli`, or no `bun`/`node` to run the bridge, nothing +is written and the manual path below applies. A malformed or foreign-shaped +config file is left byte-identical rather than replaced. + +**Manual path — one config for every session.** The file never changes, across sessions, restarts or re-lends, as long as the CLI expands environment variables in HTTP headers. `kiro-cli` does (`${VAR}` in `headers`; it does **not** expand `url`): @@ -442,8 +487,8 @@ HTTP headers. `kiro-cli` does (`${VAR}` in `headers`; it does **not** expand `ur The port is fixed by `PTY_TOOLS_LISTEN`, so the URL is a constant. Each CLI process expands `${OPENAB_TOOLS_MCP_TOKEN}` from its own session's environment. -**Per session — only when the config is private to one session.** For `kiro-cli` -(2.13+), inside the session shell: +**Manual path — per session, only when the config is private to one session.** +For `kiro-cli` (2.13+), inside the session shell: ```sh kiro-cli mcp add --name computer --url "$OPENAB_TOOLS_MCP_URL" --scope global @@ -484,12 +529,12 @@ kiro-cli mcp add --name computer --url "$OPENAB_TOOLS_MCP_URL" --scope global Update `@mac/*` entries in an agent's `allowedTools` to `@computer/*` at the same time; keeping both aliases duplicates every served tool. -**Caveat — the key rotates** (this is what the header form above avoids). The `` in `$OPENAB_TOOLS_MCP_URL` is per session -*generation*: a restart-in-place, or tearing down and re-lending a Mac, mints a new -URL, and any config that hard-codes the old one (both `mcp.json` and the agent's -`allowedTools` server entry) must be updated. Re-run `mcp add`, or read -`$OPENAB_TOOLS_MCP_URL` again, after each re-lend. Injecting and refreshing this -automatically at session spawn is [tracked as #39](https://github.com/openabdev/openab-pty/issues/39); until then it is a documented manual step. +**Caveat — the key rotates** (this is what both session-independent forms +above avoid). The `` in `$OPENAB_TOOLS_MCP_URL` is per session +*generation*: a restart-in-place, or tearing down and re-lending a Mac, mints +a new URL. A spawn writes the runtime-owned form again, so an injected +`computer` entry is never stale; a *hand-configured* one that hard-codes a +URL must be re-run or re-read after each re-lend. ### 9.4 Minimum viable lender @@ -500,4 +545,6 @@ automatically at session spawn is [tracked as #39](https://github.com/openabdev/ other 4xxx stop. 5. Operator: `DELETE …/tools-attach` to withdraw; `POST` again before expiry to renew. -The CLI side is zero steps: the URL is already in its environment. +The CLI side is zero steps: on a known variant the `computer` server is +already configured and trusted; on any other, the URL is already in its +environment. diff --git a/runtime/Cargo.toml b/runtime/Cargo.toml index 9aeb983..1b83744 100644 --- a/runtime/Cargo.toml +++ b/runtime/Cargo.toml @@ -41,5 +41,3 @@ libc = "0.2" [dev-dependencies] tokio-tungstenite = "0.30" - -[workspace] diff --git a/runtime/src/cli_config.rs b/runtime/src/cli_config.rs new file mode 100644 index 0000000..e0765a0 --- /dev/null +++ b/runtime/src/cli_config.rs @@ -0,0 +1,385 @@ +//! Injecting the session's tools MCP into a coding CLI's own config (#39). +//! +//! `OPENAB_TOOLS_MCP_URL` puts the loopback endpoint in the child's +//! environment at spawn (CLIENT-CONTRACT §9.3), but nothing used to wire it +//! into the CLI itself: every session needed a manual `kiro-cli mcp add` plus +//! a hand-edited `allowedTools`, and both hard-coded a URL whose key dies with +//! the session's next spawn. This module writes the *session-independent* +//! form instead: +//! +//! - `~/.kiro/settings/mcp.json` gains a `computer` server that runs a small +//! stdio bridge (`computer-mcp-bridge.js`, shipped inside the binary and +//! re-written under `~/.local/share/openab-pty/` every spawn) on a JS +//! runtime. The bridge reads the URL from *its* environment — +//! `${OPENAB_TOOLS_MCP_URL}` in the server's `env`, the one place Kiro +//! expands variables — so the one shared file serves every session in the +//! pod and survives key rotation with no rewrite. A config that named one +//! session's URL was the live failure behind the field finding on #39: three +//! sessions, one computer. +//! - every `~/.kiro/agents/*.json` gains `@computer/*` in `allowedTools` — the +//! platform-neutral alias per #42, covering whatever the sandbox profile +//! actually serves (never `exec`; the profile does not have it). Agents that +//! also pin a restrictive `tools` list get `@computer` so the server is +//! visible, and an agent that opts out of mcp.json (`includeMcpJson: false`) +//! gets its own copy of the server entry. Agent *files* are never created: +//! an agent with no file yet keeps the documented `--trust-tools` / prompt +//! path, which beats risking a file that shadows a built-in agent. +//! - a CLI this module does not know gets nothing at all: §9.3's env-var +//! manual path is the documented fallback, and "nothing regresses when +//! `tools_listen` is unset" means this is only ever called when the tools +//! plane is enabled. +//! +//! Nothing written here carries a secret — the key exists only in each +//! child's environment — so these files are also safe to leave shared across +//! sessions, which is the whole point. + +use serde_json::{json, Map, Value}; +use std::io; +use std::path::{Path, PathBuf}; + +/// The MCP alias every agent sees — platform-neutral, not `mac`. +pub const SERVER_NAME: &str = "computer"; +/// What `allowedTools` gains: whatever the sandbox profile serves. +pub const TRUST_PATTERN: &str = "@computer/*"; +/// What a restrictive `tools` list gains so the server is visible at all. +const VISIBILITY_PATTERN: &str = "@computer"; +/// `tools` entries that already cover the whole server without an addition. +const COVERING_TOOLS: &[&str] = &["*", "@mcp", "@computer", "@computer/*"]; + +const BRIDGE_JS: &str = include_str!("computer-mcp-bridge.js"); +const BRIDGE_REL: &str = ".local/share/openab-pty/computer-mcp-bridge.js"; + +/// What one spawn-time pass did, for logging and tests. +#[derive(Debug, PartialEq, Eq)] +pub enum Outcome { + /// A known CLI variant was configured; `files` lists everything written. + Injected { + variant: &'static str, + files: Vec, + }, + /// No CLI whose config shape this runtime knows was found — the env-var + /// manual path is what the session gets. + NoKnownVariant, + /// A kiro CLI was found but neither `bun` nor `node` to run the bridge on. + /// A `computer` server nothing can execute is worse than none. + NoJsRuntime { variant: &'static str }, +} + +/// Wire the tools MCP into every known agent-CLI config under `home`. Called +/// once per spawn: the files are session-independent, so the merge is +/// idempotent and a re-lend or restart needs no separate refresh pass — the +/// next spawn's bridge reads the new generation's URL from its own env. +pub fn inject_tools_mcp(home: &Path, env: &[(String, String)]) -> io::Result { + let dirs = path_dirs(home, env); + let Some(kiro) = detect_executable("kiro-cli", home, &dirs) else { + return Ok(Outcome::NoKnownVariant); + }; + let Some(js_runtime) = find_js_runtime(&kiro, home, &dirs) else { + return Ok(Outcome::NoJsRuntime { variant: "kiro" }); + }; + + let mut files = Vec::new(); + // Runtime-owned: rewritten every spawn so an outdated bridge self-heals. + let bridge = home.join(BRIDGE_REL); + write_atomic(&bridge, BRIDGE_JS)?; + files.push(bridge.clone()); + + let entry = computer_server_entry(&js_runtime, &bridge); + let mcp = home.join(".kiro/settings/mcp.json"); + if merge_mcp_json(&mcp, &entry)? { + files.push(mcp); + } + + // Merge into every agent file that already exists; never create one (a + // hand-made `kiro_default.json` can shadow the built-in agent). + let agents = home.join(".kiro/agents"); + if agents.is_dir() { + for entry_result in std::fs::read_dir(&agents)? { + // One unreadable entry must not abort the files after it — the + // merge is per-file, not atomic across the directory. + let Ok(dir_entry) = entry_result else { + tracing::warn!(dir = %agents.display(), "could not read an agents dir entry"); + continue; + }; + let path = dir_entry.path(); + if path.is_file() + && path.extension().and_then(|ext| ext.to_str()) == Some("json") + && merge_agent_json(&path, &entry)? + { + files.push(path); + } + } + } + Ok(Outcome::Injected { + variant: "kiro", + files, + }) +} + +// --------------------------------------------------------------------------- +// Discovery +// --------------------------------------------------------------------------- + +/// Directories a spawned `command` can come from: the child's own PATH, plus +/// `~/.local/bin` for the installs that never made it onto PATH. Absolute +/// system dirs are deliberately *not* appended — detection stays a function +/// of the child's environment, not of wherever the runtime happens to run. +fn path_dirs(home: &Path, env: &[(String, String)]) -> Vec { + let mut dirs: Vec = env + .iter() + .find(|(key, _)| key == "PATH") + .map(|(_, value)| std::env::split_paths(value).collect()) + .unwrap_or_default(); + let local_bin = home.join(".local/bin"); + if !dirs.contains(&local_bin) { + dirs.push(local_bin); + } + dirs +} + +/// An executable file, following symlinks (an installer's `~/.local/bin` +/// entry usually is one). +fn executable(path: &Path) -> bool { + if !path.is_file() { + return false; + } + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + path.metadata() + .map(|meta| meta.permissions().mode() & 0o111 != 0) + .unwrap_or(false) + } + #[cfg(not(unix))] + { + true + } +} + +fn resolved(path: PathBuf) -> PathBuf { + path.canonicalize().unwrap_or(path) +} + +/// `name` on the child's PATH or in the spots its installer uses without one. +fn detect_executable(name: &str, home: &Path, dirs: &[PathBuf]) -> Option { + for dir in dirs { + let candidate = dir.join(name); + if executable(&candidate) { + return Some(resolved(candidate)); + } + } + // The kiro-cli installer drops binaries under the share dir even when + // ~/.local/bin never made it onto PATH. + for candidate in [ + home.join(".local/share/kiro-cli/kiro-cli"), + home.join(".local/share/kiro-cli/bin/kiro-cli"), + ] { + if executable(&candidate) { + return Some(resolved(candidate)); + } + } + None +} + +/// What can execute the bridge: the runtime bundled beside `kiro-cli` first +/// (the shape verified against the real deployment), then a PATH-visible +/// `bun`, then `node`. The bridge is portable JS for exactly this fallback. +fn find_js_runtime(kiro_cli: &Path, home: &Path, dirs: &[PathBuf]) -> Option { + let mut candidates: Vec = Vec::new(); + if let Some(dir) = kiro_cli.parent() { + candidates.push(dir.join("bun")); + } + candidates.push(home.join(".local/share/kiro-cli/bun")); + candidates.push(home.join(".local/share/kiro-cli/bin/bun")); + for name in ["bun", "node"] { + candidates.extend(dirs.iter().map(|dir| dir.join(name))); + } + candidates + .into_iter() + .find(|c| usable_js_runtime(c)) + .map(resolved) +} + +/// `executable`, plus a version probe for `node` — a node older than v18 has +/// no `fetch`, so a `computer` server pointing at it would fail on its first +/// request, which is worse than falling back to the env-var manual path. +/// `bun` has always had `fetch`. +fn usable_js_runtime(path: &Path) -> bool { + if !executable(path) { + return false; + } + if path.file_name().and_then(|n| n.to_str()) != Some("node") { + return true; + } + let Ok(output) = std::process::Command::new(path).arg("--version").output() else { + return false; + }; + if !output.status.success() { + return false; + } + let text = String::from_utf8_lossy(&output.stdout); + text.trim() + .strip_prefix('v') + .and_then(|v| v.split('.').next()) + .and_then(|major| major.parse::().ok()) + .is_some_and(|major| major >= 18) +} + +// --------------------------------------------------------------------------- +// Config merge +// --------------------------------------------------------------------------- + +/// The verified stdio shape: nothing in it names a session, a URL, or a key. +fn computer_server_entry(runtime: &Path, bridge: &Path) -> Value { + json!({ + "command": runtime.to_string_lossy(), + "args": [bridge.to_string_lossy()], + "env": { "OPENAB_TOOLS_MCP_URL": "${OPENAB_TOOLS_MCP_URL}" } + }) +} + +enum MergeResult { + /// The file changed and was (re)written. + Applied, + /// Already had what it needed; not rewritten, so the mtime does not churn + /// on every spawn. + Unchanged, + /// Present but not ours to repair — wrong shape or unparseable content is + /// left byte-identical, never silently replaced. + Skipped(&'static str), +} + +/// Read–mutate–write a JSON config file atomically. A symlinked config is a +/// shared dotfile: the merge writes its target, never replaces the link — and +/// a *dangling* link is left alone entirely rather than overwritten in place, +/// so a dotfile layout whose target appears later is not destroyed. +fn merge_json_file( + path: &Path, + mutate: impl FnOnce(&mut Map) -> MergeResult, +) -> io::Result { + let dangling_link = path + .symlink_metadata() + .map(|meta| meta.file_type().is_symlink()) + .unwrap_or(false) + && path.canonicalize().is_err(); + if dangling_link { + tracing::warn!(file = %path.display(), "skipping config: symlink target does not exist"); + return Ok(false); + } + let real = path.canonicalize().unwrap_or_else(|_| path.to_path_buf()); + let mut root: Map = match std::fs::read_to_string(&real) { + Ok(text) if text.trim().is_empty() => Map::new(), + Ok(text) => match serde_json::from_str::(&text) { + Ok(Value::Object(map)) => map, + _ => { + tracing::warn!(file = %real.display(), "skipping config that is not a JSON object"); + return Ok(false); + } + }, + Err(error) if error.kind() == io::ErrorKind::NotFound => Map::new(), + Err(error) => return Err(error), + }; + match mutate(&mut root) { + MergeResult::Unchanged => Ok(false), + MergeResult::Skipped(reason) => { + tracing::warn!(file = %real.display(), "skipping config: {reason}"); + Ok(false) + } + MergeResult::Applied => { + let text = + serde_json::to_string_pretty(&Value::Object(root)).map_err(io::Error::other)?; + write_atomic(&real, &format!("{text}\n")).map(|_| true) + } + } +} + +/// `mcpServers.computer` = the bridge server. Everything else in the file — +/// other servers, other settings — is preserved. +fn merge_mcp_json(path: &Path, entry: &Value) -> io::Result { + merge_json_file(path, |root| { + let servers = root.entry("mcpServers").or_insert_with(|| json!({})); + let Some(servers) = servers.as_object_mut() else { + return MergeResult::Skipped("`mcpServers` is not an object"); + }; + if servers.get(SERVER_NAME) == Some(entry) { + return MergeResult::Unchanged; + } + servers.insert(SERVER_NAME.into(), entry.clone()); + MergeResult::Applied + }) +} + +/// One agent file: `@computer/*` into `allowedTools`, `@computer` into a +/// restrictive `tools` list, and the server itself into `mcpServers` when the +/// agent opted out of the shared mcp.json. Other fields are untouched. +fn merge_agent_json(path: &Path, entry: &Value) -> io::Result { + merge_json_file(path, |root| { + let mut changed = false; + match root.get_mut("allowedTools") { + None => { + root.insert("allowedTools".into(), json!([TRUST_PATTERN])); + changed = true; + } + Some(Value::Array(list)) => { + let trusted = list + .iter() + .any(|v| matches!(v.as_str(), Some(TRUST_PATTERN) | Some("@computer"))); + if !trusted { + list.push(json!(TRUST_PATTERN)); + changed = true; + } + } + Some(_) => return MergeResult::Skipped("`allowedTools` is not an array"), + } + if let Some(tools) = root.get_mut("tools") { + match tools { + Value::Array(list) => { + let covered = list + .iter() + .any(|v| v.as_str().is_some_and(|s| COVERING_TOOLS.contains(&s))); + if !covered { + list.push(json!(VISIBILITY_PATTERN)); + changed = true; + } + } + _ => return MergeResult::Skipped("`tools` is not an array"), + } + } + if root.get("includeMcpJson") == Some(&json!(false)) { + let servers = root.entry("mcpServers").or_insert_with(|| json!({})); + let Some(servers) = servers.as_object_mut() else { + return MergeResult::Skipped("`mcpServers` is not an object"); + }; + if servers.get(SERVER_NAME) != Some(entry) { + servers.insert(SERVER_NAME.into(), entry.clone()); + changed = true; + } + } + if changed { + MergeResult::Applied + } else { + MergeResult::Unchanged + } + }) +} + +/// Write `text` to `path` via a sibling tempfile + rename, so a reader of the +/// shared config never sees a torn file. An existing file keeps its mode — +/// the tempfile's restrictive default must not tighten a shared dotfile. +fn write_atomic(path: &Path, text: &str) -> io::Result<()> { + use std::io::Write; + let Some(dir) = path.parent() else { + return Err(io::Error::other(format!( + "no parent dir: {}", + path.display() + ))); + }; + std::fs::create_dir_all(dir)?; + let mut tmp = tempfile::NamedTempFile::new_in(dir)?; + tmp.write_all(text.as_bytes())?; + if let Ok(meta) = std::fs::metadata(path) { + std::fs::set_permissions(tmp.path(), meta.permissions())?; + } + tmp.persist(path).map_err(|error| error.error)?; + Ok(()) +} diff --git a/runtime/src/computer-mcp-bridge.js b/runtime/src/computer-mcp-bridge.js new file mode 100644 index 0000000..1d80a92 --- /dev/null +++ b/runtime/src/computer-mcp-bridge.js @@ -0,0 +1,126 @@ +// computer-mcp-bridge.js — written by openab-pty at every session spawn. +// Do not edit: the runtime owns this file and rewrites it on the next spawn. +// +// The `computer` MCP server, stdio to Streamable HTTP. The session's coding CLI +// launches this as a local server; every line on stdin is one JSON-RPC message, +// POSTed to $OPENAB_TOOLS_MCP_URL — the per-session loopback endpoint the +// runtime puts in this session's environment. The workspace mcp.json is shared +// by every session in the pod, which is exactly why the URL lives in the +// environment and not in the file: each session's own env routes it to its own +// session, and a rotated key needs no rewrite. + +"use strict"; + +const endpoint = process.env.OPENAB_TOOLS_MCP_URL; +// A literal "${OPENAB_TOOLS_MCP_URL}" here means the CLI did not expand the +// server env — fail loudly instead of fetch-throwing on every request. +if (!endpoint || endpoint.includes("${")) { + process.stderr.write( + "computer-mcp-bridge: OPENAB_TOOLS_MCP_URL is unset or was not expanded; " + + "the tools MCP only exists inside an openab-pty session with the tools " + + "plane enabled, and kiro must expand env for this wiring to work\n" + ); + process.exit(1); +} + +// Streamable HTTP carries the server session id in a response header and wants +// it echoed on every later request; openab-pty does not issue one today, but a +// deployment that routes this endpoint differently may. +let sessionId = null; + +function errorResult(id, message) { + return { jsonrpc: "2.0", id, error: { code: -32000, message } }; +} + +// `data:` payload of each SSE event block; every one is a complete JSON-RPC +// message from the server. +function* sseData(body) { + for (const block of body.split(/\r?\n\r?\n/)) { + const data = block + .split(/\r?\n/) + .filter((line) => line.startsWith("data:")) + .map((line) => line.slice(5).replace(/^ /, "")) + .join("\n") + .trim(); + if (data && data !== "[DONE]") yield data; + } +} + +function write(message) { + process.stdout.write(message + "\n"); +} + +async function forward(line) { + const headers = { + "content-type": "application/json", + accept: "application/json, text/event-stream", + }; + if (sessionId) headers["mcp-session-id"] = sessionId; + const response = await fetch(endpoint, { method: "POST", headers, body: line }); + const sid = response.headers.get("mcp-session-id"); + if (sid) sessionId = sid; + // A notification's whole acknowledgement is the empty 2xx. + if (response.status === 202 || response.status === 204) return []; + const body = await response.text(); + if (!response.ok) return [{ httpError: response.status, body }]; + const type = response.headers.get("content-type") || ""; + if (type.includes("text/event-stream")) { + return [...sseData(body)].map((data) => ({ data })); + } + return body.trim() ? [{ data: body }] : []; +} + +async function handle(line) { + let id = null; + try { + const parsed = JSON.parse(line); + id = parsed && parsed.id !== undefined && parsed.id !== null ? parsed.id : null; + } catch { + // Not JSON: still forwarded; the endpoint's own parse-error answer applies. + } + try { + for (const out of await forward(line)) { + if (out.httpError !== undefined) { + // A request with an id must get an answer or the client waits forever. + if (id !== null) { + write( + JSON.stringify( + errorResult( + id, + `computer-mcp-bridge: tools endpoint returned HTTP ${out.httpError}: ${String( + out.body + ).slice(0, 200)}` + ) + ) + ); + } + continue; + } + // One JSON-RPC message must occupy exactly one stdout line — an SSE + // block joined multi-line `data:` fields, so re-encode compactly rather + // than relaying the raw text. + try { + write(JSON.stringify(JSON.parse(out.data))); + } catch { + write(out.data.replace(/\r?\n/g, " ")); + } + } + } catch (error) { + if (id !== null) { + write(JSON.stringify(errorResult(id, `computer-mcp-bridge: ${String(error)}`))); + } + } +} + +let buffered = ""; +process.stdin.setEncoding("utf8"); +process.stdin.on("data", (chunk) => { + buffered += chunk; + let at; + while ((at = buffered.indexOf("\n")) >= 0) { + const line = buffered.slice(0, at).trim(); + buffered = buffered.slice(at + 1); + if (line) handle(line); + } +}); +process.stdin.on("end", () => process.exit(0)); diff --git a/runtime/src/lib.rs b/runtime/src/lib.rs index 00d4466..05c194f 100644 --- a/runtime/src/lib.rs +++ b/runtime/src/lib.rs @@ -25,6 +25,7 @@ pub mod admin_auth; pub mod audit; pub mod capproxy; +pub mod cli_config; pub mod config; pub mod containment; pub mod killdomain; diff --git a/runtime/src/session.rs b/runtime/src/session.rs index 85db00f..d7a0cb0 100644 --- a/runtime/src/session.rs +++ b/runtime/src/session.rs @@ -36,6 +36,7 @@ use crate::audit::{AuditEvent, AuditKind, AuditLogger, TerminationClass}; use crate::capproxy::CapabilityProxy; +use crate::cli_config; use crate::close_code; use crate::config::PtyConfig; use crate::killdomain::{KillDomain, KillDomainTier, SessionKillDomain, TeardownOutcome}; @@ -991,6 +992,13 @@ pub struct SessionManager { /// Set once the tools listener is bound. Every child spawned afterwards /// receives `OPENAB_TOOLS_MCP_URL`, its private loopback MCP endpoint. tools: Mutex>, + /// Serializes the shared read–modify–write of CLI config files across + /// concurrent spawns; without it two sessions could interleave a merge. + cli_config_lock: Mutex<()>, + /// The workspace a session lands in, when it is not the process HOME — + /// tests set this so config injection writes into their own tempdir + /// instead of the developer's `~/.kiro`. + workspace_override: Mutex>, } /// Where a session's coding CLI finds its reverse-attached Mac. @@ -1038,6 +1046,8 @@ impl SessionManager { metrics: Metrics::default(), epoch: Instant::now(), tools: Mutex::new(None), + cli_config_lock: Mutex::new(()), + workspace_override: Mutex::new(None), })) } @@ -1199,21 +1209,61 @@ impl SessionManager { ); // The per-session loopback key is issued here, at spawn, so the URL in the // child's environment is the only place it ever exists in plaintext. A - // restart-in-place rotates it with the shell. - if let Some(endpoint) = self.tools.lock().as_ref() { + // restart-in-place rotates it with the shell. Endpoint reads end at the + // semicolon — file I/O for the CLI config happens with the guard dropped, + // so a slow filesystem cannot sit between the tools plane and a request. + let tools_env = self.tools.lock().as_ref().map(|endpoint| { let key = endpoint.hub.issue_loopback_key(&name); + [ + ( + TOOLS_URL_ENV.to_string(), + format!("{}/mcp/{}/{}", endpoint.base_url, name.as_str(), key), + ), + (TOOLS_TOKEN_ENV.to_string(), key), + ( + TOOLS_ENDPOINT_ENV.to_string(), + format!("{}/mcp", endpoint.base_url), + ), + ] + }); + if let Some(tools_env) = tools_env { env.retain(|(k, _)| { k != TOOLS_URL_ENV && k != TOOLS_TOKEN_ENV && k != TOOLS_ENDPOINT_ENV }); - env.push(( - TOOLS_URL_ENV.to_string(), - format!("{}/mcp/{}/{}", endpoint.base_url, name.as_str(), key), - )); - env.push((TOOLS_TOKEN_ENV.to_string(), key)); - env.push(( - TOOLS_ENDPOINT_ENV.to_string(), - format!("{}/mcp", endpoint.base_url), - )); + env.extend(tools_env); + // Wire the tools MCP into the session CLI's config too (#39): the + // files are session-independent — the URL and key live only in + // this child's environment — so a shared workspace config needs + // no refresh on rotation and holds no secret. A failure here must + // never cost a spawn: the §9.3 env-var manual path still stands. + let _serialize = self.cli_config_lock.lock(); + match cli_config::inject_tools_mcp(&self.workspace(), &env) { + Ok(cli_config::Outcome::Injected { variant, files }) => { + tracing::info!( + session = %name, + variant, + files = ?files, + "tools MCP injected into the session CLI config" + ); + } + Ok(cli_config::Outcome::NoKnownVariant) => {} + Ok(cli_config::Outcome::NoJsRuntime { variant }) => { + tracing::warn!( + session = %name, + variant, + "tools MCP not injected: no JS runtime to run the \ + bridge; the env-var manual path remains" + ); + } + Err(error) => { + tracing::warn!( + session = %name, + %error, + "tools MCP CLI config injection failed; the env-var \ + manual path remains" + ); + } + } } let request = SpawnRequest { session: name.clone(), @@ -1341,11 +1391,21 @@ impl SessionManager { } fn workspace(&self) -> PathBuf { + if let Some(path) = self.workspace_override.lock().clone() { + return path; + } std::env::var("HOME") .map(PathBuf::from) .unwrap_or_else(|_| PathBuf::from("/")) } + /// Point `workspace()` at `path` — tests only, so spawn-time config + /// injection never writes into a developer's real home directory. + #[cfg(test)] + pub(crate) fn set_test_workspace(&self, path: PathBuf) { + *self.workspace_override.lock() = Some(path); + } + /// Attach. The token must already have been verified by the attach surface; /// the generation is re-checked here as the authoritative fence. /// @@ -2299,6 +2359,183 @@ mod tests { assert!(env.contains(&("HOME".to_string(), "/workspace".to_string()))); } + // ---- tools MCP config injection (issue #39) -------------------------- + + /// A kiro-cli installer's layout under a fake home: `kiro-cli` and its + /// bundled `bun`, both executable — what `cli_config` detection keys on. + fn kiro_install(home: &std::path::Path) { + let dir = home.join(".local/share/kiro-cli"); + std::fs::create_dir_all(&dir).unwrap(); + for bin in ["kiro-cli", "bun"] { + let path = dir.join(bin); + std::fs::write(&path, b"#!/bin/sh\n").unwrap(); + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + std::fs::set_permissions(&path, std::fs::Permissions::from_mode(0o755)).unwrap(); + } + } + } + + fn tools_endpoint() -> ToolsEndpoint { + ToolsEndpoint { + hub: Arc::new(crate::tools::ToolsHub::new( + Duration::from_secs(60), + AuditLogger, + )), + base_url: "http://127.0.0.1:4711".into(), + } + } + + #[tokio::test] + async fn a_tools_spawn_injects_session_independent_kiro_config() { + let spawner = FakeSpawner::new(); + let manager = manager_with(spawner.clone(), "", SessionPolicy::default()); + let home = tempfile::tempdir().unwrap(); + kiro_install(home.path()); + manager.set_test_workspace(home.path().to_path_buf()); + manager.set_tools_endpoint(tools_endpoint()); + manager + .create(name("alpha"), WindowSize::default()) + .unwrap(); + + // The child env still carries the URL — that is what the injected + // config's bridge actually reads. + let env = spawner.last_env.lock().clone(); + let url = env + .iter() + .find(|(k, _)| k == TOOLS_URL_ENV) + .map(|(_, v)| v.clone()) + .expect("tools spawn must pass the MCP URL to the child"); + assert!(url.starts_with("http://127.0.0.1:4711/mcp/alpha/")); + + let mcp = std::fs::read_to_string(home.path().join(".kiro/settings/mcp.json")).unwrap(); + assert!(mcp.contains("\"computer\""), "server alias: {mcp}"); + assert!( + mcp.contains("${OPENAB_TOOLS_MCP_URL}"), + "env forward: {mcp}" + ); + // The shared workspace file must never pin one session's URL or key: + // it is how a rotated key needed no rewrite. + assert!(!mcp.contains("4711"), "no endpoint literal: {mcp}"); + assert!( + !mcp.contains(&url["http://127.0.0.1:4711".len()..]), + "no per-session URL or key: {mcp}" + ); + assert!(home + .path() + .join(".local/share/openab-pty/computer-mcp-bridge.js") + .is_file()); + } + + #[tokio::test] + async fn a_tools_spawn_on_a_cli_we_do_not_know_still_gets_the_env() { + // No kiro-cli anywhere → the env-var manual path, and zero files. + let spawner = FakeSpawner::new(); + let manager = manager_with(spawner.clone(), "", SessionPolicy::default()); + let home = tempfile::tempdir().unwrap(); + manager.set_test_workspace(home.path().to_path_buf()); + manager.set_tools_endpoint(tools_endpoint()); + manager + .create(name("alpha"), WindowSize::default()) + .unwrap(); + + let env = spawner.last_env.lock().clone(); + assert!(env.iter().any(|(k, _)| k == TOOLS_URL_ENV)); + assert!(!home.path().join(".kiro").exists()); + } + + #[tokio::test] + async fn a_spawn_without_the_tools_plane_writes_no_cli_config() { + let spawner = FakeSpawner::new(); + let manager = manager_with(spawner.clone(), "", SessionPolicy::default()); + let home = tempfile::tempdir().unwrap(); + kiro_install(home.path()); + manager.set_test_workspace(home.path().to_path_buf()); + manager + .create(name("alpha"), WindowSize::default()) + .unwrap(); + + let env = spawner.last_env.lock().clone(); + assert!(env.iter().all(|(k, _)| { + k != TOOLS_URL_ENV && k != TOOLS_TOKEN_ENV && k != TOOLS_ENDPOINT_ENV + })); + assert!(!home.path().join(".kiro").exists()); + assert!(!home.path().join(".local/share/openab-pty").exists()); + } + + #[tokio::test] + async fn a_restart_rotates_the_env_key_and_never_touches_the_config() { + // The literal acceptance criterion of #39: a re-lend or restart-in- + // place mints a new loopback key, and *nothing* has to be rewritten — + // the session-independent file stays byte-identical. + let spawner = FakeSpawner::new(); + let manager = manager_with(spawner.clone(), "", SessionPolicy::default()); + let home = tempfile::tempdir().unwrap(); + kiro_install(home.path()); + manager.set_test_workspace(home.path().to_path_buf()); + manager.set_tools_endpoint(tools_endpoint()); + manager + .create(name("alpha"), WindowSize::default()) + .unwrap(); + let mcp = home.path().join(".kiro/settings/mcp.json"); + let config_before = std::fs::read(&mcp).unwrap(); + let key_before = spawner + .last_env + .lock() + .iter() + .find(|(k, _)| k == TOOLS_TOKEN_ENV) + .map(|(_, v)| v.clone()) + .unwrap(); + + manager.restart_in_place(&name("alpha")).await.unwrap(); + + let env = spawner.last_env.lock().clone(); + let key_after = env + .iter() + .find(|(k, _)| k == TOOLS_TOKEN_ENV) + .map(|(_, v)| v.clone()) + .unwrap(); + assert_ne!(key_before, key_after, "a restart mints a fresh key"); + let url = env + .iter() + .find(|(k, _)| k == TOOLS_URL_ENV) + .map(|(_, v)| v.clone()) + .unwrap(); + assert!(url.contains(&key_after), "the URL carries the new key"); + assert_eq!( + std::fs::read(&mcp).unwrap(), + config_before, + "the shared config needs no refresh on key rotation" + ); + } + + #[tokio::test] + #[cfg(unix)] + async fn a_spawn_survives_an_unwritable_workspace() { + // Injection failing must never cost a spawn: the §9.3 env path still + // stands, and a session that couldn't write config is still a session. + let spawner = FakeSpawner::new(); + let manager = manager_with(spawner.clone(), "", SessionPolicy::default()); + let home = tempfile::tempdir().unwrap(); + { + use std::os::unix::fs::PermissionsExt; + std::fs::set_permissions(home.path(), std::fs::Permissions::from_mode(0o555)).unwrap(); + } + manager.set_test_workspace(home.path().to_path_buf()); + manager.set_tools_endpoint(tools_endpoint()); + manager + .create(name("alpha"), WindowSize::default()) + .expect("a failed injection warns, it does not fail the spawn"); + + let env = spawner.last_env.lock().clone(); + assert!(env.iter().any(|(k, _)| k == TOOLS_URL_ENV)); + { + use std::os::unix::fs::PermissionsExt; + std::fs::set_permissions(home.path(), std::fs::Permissions::from_mode(0o755)).unwrap(); + } + } + // ---- admission ------------------------------------------------------ #[tokio::test] diff --git a/runtime/tests/cli_config.rs b/runtime/tests/cli_config.rs new file mode 100644 index 0000000..393f410 --- /dev/null +++ b/runtime/tests/cli_config.rs @@ -0,0 +1,517 @@ +//! The spawn-time tools-MCP injection into the session CLI's config (#39). +//! +//! A lent Mac's tools must appear in the coding CLI with zero manual +//! `mcp add`, and the config a session writes must be *session-independent*: +//! the workspace `mcp.json` is shared by every session in the pod, so it can +//! never name one session's URL or key — the bridge reads +//! `OPENAB_TOOLS_MCP_URL` from the environment of whatever process launches it. +//! +//! Pure filesystem tests: each one builds its own tempdir HOME with a stub +//! `kiro-cli`/`bun`/`node` layout, so no test touches the real home directory +//! or needs the tools plane running. + +use openab_pty::cli_config::{inject_tools_mcp, Outcome}; +use serde_json::{json, Value}; +use std::fs; +use std::path::{Path, PathBuf}; + +fn executable(path: &Path, contents: &str) { + fs::write(path, contents).unwrap(); + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + fs::set_permissions(path, fs::Permissions::from_mode(0o755)).unwrap(); + } +} + +/// A kiro installer's layout under `home`: `kiro-cli` and its bundled `bun`. +fn kiro_install(home: &Path) -> PathBuf { + let dir = home.join(".local/share/kiro-cli"); + fs::create_dir_all(&dir).unwrap(); + executable(&dir.join("kiro-cli"), "#!/bin/sh\n"); + executable(&dir.join("bun"), "#!/bin/sh\n"); + dir +} + +/// A PATH that resolves nothing but the stubs deliberately placed in `bin`. +fn env_with_path(bin: &Path) -> Vec<(String, String)> { + vec![("PATH".to_string(), bin.to_string_lossy().into_owned())] +} + +fn read_json(path: &Path) -> Value { + serde_json::from_str(&fs::read_to_string(path).unwrap()).unwrap() +} + +fn mcp_path(home: &Path) -> PathBuf { + home.join(".kiro/settings/mcp.json") +} + +fn bridge_path(home: &Path) -> PathBuf { + home.join(".local/share/openab-pty/computer-mcp-bridge.js") +} + +#[test] +fn kiro_gets_a_session_independent_computer_server_and_trust() { + let home = tempfile::tempdir().unwrap(); + let kiro = kiro_install(home.path()); + let bin = tempfile::tempdir().unwrap(); + + let agents = home.path().join(".kiro/agents"); + fs::create_dir_all(&agents).unwrap(); + fs::write( + agents.join("main.json"), + r#"{"name":"main","description":"the agent","tools":["read","write"], + "allowedTools":["read"],"resources":["file://AGENTS.md"]}"#, + ) + .unwrap(); + fs::write(agents.join("notes.md"), "---\nname: notes\n---\n").unwrap(); + + let outcome = inject_tools_mcp(home.path(), &env_with_path(bin.path())).unwrap(); + let Outcome::Injected { variant, files } = outcome else { + panic!("expected Injected, got {outcome:?}"); + }; + assert_eq!(variant, "kiro"); + assert!(files.contains(&mcp_path(home.path()))); + assert!(files.contains(&bridge_path(home.path()))); + assert!(files.contains(&agents.join("main.json"))); + + // The server entry is the verified stdio shape: the bundled runtime on a + // bridge that reads the URL from *its* environment. No URL, no key, so the + // one shared file cannot pin a session — or go stale on rotation. + let mcp = read_json(&mcp_path(home.path())); + let computer = &mcp["mcpServers"]["computer"]; + assert_eq!( + computer["command"], + json!(kiro.join("bun").to_string_lossy()) + ); + assert_eq!( + computer["args"], + json!([bridge_path(home.path()).to_string_lossy()]) + ); + assert_eq!( + computer["env"]["OPENAB_TOOLS_MCP_URL"], + json!("${OPENAB_TOOLS_MCP_URL}") + ); + let raw = fs::read_to_string(mcp_path(home.path())).unwrap(); + assert!(!raw.contains("http"), "no endpoint literal: {raw}"); + assert!(!raw.contains("/mcp/"), "no URL path literal: {raw}"); + + // The bridge itself is the only place the variable is *read*; the config + // only forwards it. And it holds no key either. + let bridge = fs::read_to_string(bridge_path(home.path())).unwrap(); + assert!(bridge.contains("OPENAB_TOOLS_MCP_URL")); + + // Trust: the served set, under the platform-neutral alias. + let agent = read_json(&agents.join("main.json")); + assert_eq!( + agent["allowedTools"], + json!(["read", "@computer/*"]), + "trust is additive, after what the file already had" + ); + assert_eq!( + agent["tools"], + json!(["read", "write", "@computer"]), + "a restrictive tools list must still see the server" + ); + assert_eq!(agent["name"], json!("main")); + assert_eq!(agent["description"], json!("the agent")); + assert_eq!(agent["resources"], json!(["file://AGENTS.md"])); + + // Markdown agents are not JSON-mergeable; they are left alone. + assert_eq!( + fs::read_to_string(agents.join("notes.md")).unwrap(), + "---\nname: notes\n---\n" + ); +} + +#[test] +fn injection_merges_without_touching_unrelated_config() { + let home = tempfile::tempdir().unwrap(); + kiro_install(home.path()); + let bin = tempfile::tempdir().unwrap(); + fs::create_dir_all(mcp_path(home.path()).parent().unwrap()).unwrap(); + fs::write( + mcp_path(home.path()), + r#"{ + "mcpServers": {"github": {"command": "gh-mcp"}}, + "otherSetting": true + }"#, + ) + .unwrap(); + + let outcome = inject_tools_mcp(home.path(), &env_with_path(bin.path())).unwrap(); + assert!(matches!(outcome, Outcome::Injected { .. })); + + let mcp = read_json(&mcp_path(home.path())); + assert_eq!( + mcp["mcpServers"]["github"], + json!({"command": "gh-mcp"}), + "an existing server survives the merge" + ); + assert_eq!(mcp["otherSetting"], json!(true)); + assert!(mcp["mcpServers"]["computer"].is_object()); +} + +#[test] +fn a_cli_the_runtime_does_not_know_gets_the_manual_path() { + let home = tempfile::tempdir().unwrap(); + let bin = tempfile::tempdir().unwrap(); // nothing on PATH either + + let outcome = inject_tools_mcp(home.path(), &env_with_path(bin.path())).unwrap(); + assert_eq!(outcome, Outcome::NoKnownVariant); + assert!( + !home.path().join(".kiro").exists(), + "nothing is written for a CLI whose config shape we do not know" + ); +} + +#[test] +fn no_js_runtime_means_no_broken_server_entry() { + let home = tempfile::tempdir().unwrap(); + let kiro = kiro_install(home.path()); + fs::remove_file(kiro.join("bun")).unwrap(); + let bin = tempfile::tempdir().unwrap(); // no bun, no node + + let outcome = inject_tools_mcp(home.path(), &env_with_path(bin.path())).unwrap(); + let Outcome::NoJsRuntime { variant } = outcome else { + panic!("expected NoJsRuntime, got {outcome:?}"); + }; + assert_eq!(variant, "kiro"); + assert!( + !mcp_path(home.path()).exists(), + "a computer server nothing can execute is worse than none" + ); +} + +#[test] +fn node_is_the_fallback_runtime_when_no_bun_exists() { + let home = tempfile::tempdir().unwrap(); + let kiro = kiro_install(home.path()); + fs::remove_file(kiro.join("bun")).unwrap(); + let bin = tempfile::tempdir().unwrap(); + executable(&bin.path().join("node"), "#!/bin/sh\necho v18.19.0\n"); + + let outcome = inject_tools_mcp(home.path(), &env_with_path(bin.path())).unwrap(); + assert!(matches!(outcome, Outcome::Injected { .. })); + let mcp = read_json(&mcp_path(home.path())); + assert_eq!( + mcp["mcpServers"]["computer"]["command"], + json!(bin.path().join("node").to_string_lossy()) + ); +} + +#[test] +fn a_node_too_old_for_fetch_falls_back_to_the_manual_path() { + // node < 18 has no fetch; pointing `computer` at it injects a server that + // only ever reports errors — better to write nothing at all. + let home = tempfile::tempdir().unwrap(); + let kiro = kiro_install(home.path()); + fs::remove_file(kiro.join("bun")).unwrap(); + let bin = tempfile::tempdir().unwrap(); + executable(&bin.path().join("node"), "#!/bin/sh\necho v16.20.0\n"); + + let outcome = inject_tools_mcp(home.path(), &env_with_path(bin.path())).unwrap(); + assert_eq!(outcome, Outcome::NoJsRuntime { variant: "kiro" }); + assert!(!mcp_path(home.path()).exists()); +} + +#[test] +fn malformed_or_foreign_shaped_config_is_never_clobbered() { + let home = tempfile::tempdir().unwrap(); + kiro_install(home.path()); + let bin = tempfile::tempdir().unwrap(); + + let mangled = "{ not json"; + fs::create_dir_all(mcp_path(home.path()).parent().unwrap()).unwrap(); + fs::write(mcp_path(home.path()), mangled).unwrap(); + + let agents = home.path().join(".kiro/agents"); + fs::create_dir_all(&agents).unwrap(); + fs::write( + agents.join("odd.json"), + r#"{"name":"odd","allowedTools":"read"}"#, + ) + .unwrap(); + + inject_tools_mcp(home.path(), &env_with_path(bin.path())).unwrap(); + + assert_eq!( + fs::read_to_string(mcp_path(home.path())).unwrap(), + mangled, + "a file that is not ours to fix is left byte-identical" + ); + assert_eq!( + fs::read_to_string(agents.join("odd.json")).unwrap(), + r#"{"name":"odd","allowedTools":"read"}"#, + "a non-array allowedTools is skipped, not rewritten" + ); +} + +#[test] +#[cfg(unix)] +fn a_dangling_symlinked_config_is_left_alone() { + // Dotfile managers often link a config whose target exists only later; + // replacing the link with a real file would break that layout. + let home = tempfile::tempdir().unwrap(); + kiro_install(home.path()); + let settings = home.path().join(".kiro/settings"); + std::fs::create_dir_all(&settings).unwrap(); + let mcp = settings.join("mcp.json"); + std::os::unix::fs::symlink(settings.join("dotfiles-mcp.json"), &mcp).unwrap(); + let outcome = inject_tools_mcp(home.path(), &env_with_path(home.path())).unwrap(); + assert!(matches!(outcome, Outcome::Injected { .. })); + assert!(mcp.symlink_metadata().unwrap().file_type().is_symlink()); + assert!(mcp.canonicalize().is_err(), "the link stays dangling"); + assert!(!settings.join("dotfiles-mcp.json").exists()); +} + +#[test] +#[cfg(unix)] +fn a_merged_config_keeps_its_existing_permissions() { + let home = tempfile::tempdir().unwrap(); + kiro_install(home.path()); + let settings = home.path().join(".kiro/settings"); + std::fs::create_dir_all(&settings).unwrap(); + let mcp = settings.join("mcp.json"); + std::fs::write(&mcp, "{}\n").unwrap(); + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + std::fs::set_permissions(&mcp, std::fs::Permissions::from_mode(0o644)).unwrap(); + } + inject_tools_mcp(home.path(), &env_with_path(home.path())).unwrap(); + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + assert_eq!( + std::fs::metadata(&mcp).unwrap().permissions().mode() & 0o777, + 0o644, + "the merge must not tighten a shared dotfile's mode" + ); + } +} + +#[test] +fn a_broken_symlink_target_or_missing_agents_dir_is_not_invented() { + let home = tempfile::tempdir().unwrap(); + kiro_install(home.path()); + let bin = tempfile::tempdir().unwrap(); + + inject_tools_mcp(home.path(), &env_with_path(bin.path())).unwrap(); + assert!( + !home.path().join(".kiro/agents").exists(), + "no agent files exist yet: creating one could shadow a built-in agent, \ + so trust stays the documented manual step" + ); + assert!(mcp_path(home.path()).exists()); +} + +#[test] +fn mcp_json_symlink_is_written_through_not_replaced() { + let home = tempfile::tempdir().unwrap(); + kiro_install(home.path()); + let bin = tempfile::tempdir().unwrap(); + + let real = home.path().join("shared-mcp.json"); + fs::write(&real, r#"{"mcpServers":{}}"#).unwrap(); + fs::create_dir_all(mcp_path(home.path()).parent().unwrap()).unwrap(); + #[cfg(unix)] + std::os::unix::fs::symlink(&real, mcp_path(home.path())).unwrap(); + + inject_tools_mcp(home.path(), &env_with_path(bin.path())).unwrap(); + + assert!( + mcp_path(home.path()) + .symlink_metadata() + .unwrap() + .is_symlink(), + "the link itself must survive; the merge writes its target" + ); + assert!(read_json(&real)["mcpServers"]["computer"].is_object()); +} + +#[test] +fn an_agent_opted_out_of_mcp_json_still_gets_the_server() { + let home = tempfile::tempdir().unwrap(); + kiro_install(home.path()); + let bin = tempfile::tempdir().unwrap(); + let agents = home.path().join(".kiro/agents"); + fs::create_dir_all(&agents).unwrap(); + fs::write( + agents.join("solo.json"), + r#"{"name":"solo","includeMcpJson":false,"mcpServers":{"own":{"command":"x"}}}"#, + ) + .unwrap(); + + inject_tools_mcp(home.path(), &env_with_path(bin.path())).unwrap(); + + let agent = read_json(&agents.join("solo.json")); + assert_eq!(agent["includeMcpJson"], json!(false), "the opt-out stands"); + assert_eq!(agent["mcpServers"]["own"], json!({"command":"x"})); + assert!( + agent["mcpServers"]["computer"].is_object(), + "an agent that excludes mcp.json needs its own copy of the server" + ); + assert_eq!(agent["allowedTools"], json!(["@computer/*"])); +} + +// -------------------------------------------------------------------------- +// The bridge itself. `#[ignore]`d like the other e2e tests: it spawns a real +// JS runtime and binds a socket — it is the only coverage of the wire shape +// the kiro side actually sees. +// -------------------------------------------------------------------------- + +fn js_runtime() -> Option { + for name in ["node", "bun"] { + if let Ok(output) = std::process::Command::new("which").arg(name).output() { + if output.status.success() { + return Some(String::from_utf8(output.stdout).unwrap().trim().into()); + } + } + } + None +} + +/// A stub Streamable-HTTP endpoint: one JSON answer for requests carrying an +/// `id`, an SSE answer with a *multi-line* `data:` field for the SSE probe, +/// and a bare 202 for notifications — the three shapes the bridge must get +/// right. +async fn stub_mcp(body: axum::body::Bytes) -> axum::response::Response { + use axum::response::IntoResponse; + let request: Value = serde_json::from_slice(&body).unwrap(); + if request.get("sse_probe").is_some() { + return ( + [("content-type", "text/event-stream")], + "data: {\"jsonrpc\":\"2.0\",\ndata: \"id\":7,\ndata: \"result\":{\"sse\":true}}\n\n", + ) + .into_response(); + } + match request.get("id") { + Some(id) if !id.is_null() => axum::Json(serde_json::json!({ + "jsonrpc": "2.0", "id": id, "result": {"echo": request.get("method")} + })) + .into_response(), + _ => axum::http::StatusCode::ACCEPTED.into_response(), + } +} + +#[tokio::test] +#[ignore = "spawns a JS runtime and binds a socket"] +async fn the_bridge_relays_one_jsonrpc_message_per_line() { + let Some(runtime) = js_runtime() else { + eprintln!("no node/bun on PATH; skipping"); + return; + }; + let bridge = PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("src/computer-mcp-bridge.js"); + + let app = axum::Router::new().route("/", axum::routing::post(stub_mcp)); + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let port = listener.local_addr().unwrap().port(); + tokio::spawn(async move { axum::serve(listener, app).await.unwrap() }); + + let mut child = tokio::process::Command::new(runtime) + .arg(&bridge) + .env("OPENAB_TOOLS_MCP_URL", format!("http://127.0.0.1:{port}/")) + .stdin(std::process::Stdio::piped()) + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::null()) + .spawn() + .unwrap(); + let mut stdin = child.stdin.take().unwrap(); + let mut stdout = tokio::io::BufReader::new(child.stdout.take().unwrap()).lines(); + + use tokio::io::{AsyncBufReadExt, AsyncWriteExt}; + stdin + .write_all(b"{\"jsonrpc\":\"2.0\",\"id\":1,\"method\":\"initialize\"}\n") + .await + .unwrap(); + let line = tokio::time::timeout(std::time::Duration::from_secs(10), stdout.next_line()) + .await + .unwrap() + .unwrap() + .unwrap(); + let response: Value = serde_json::from_str(&line).unwrap(); + assert_eq!(response["id"], serde_json::json!(1)); + assert_eq!(response["result"]["echo"], serde_json::json!("initialize")); + + // An SSE reply whose `data:` spans multiple lines must still come back as + // exactly one stdout line — the bug the compact re-encode guards. + stdin + .write_all(b"{\"jsonrpc\":\"2.0\",\"id\":7,\"method\":\"x\",\"sse_probe\":true}\n") + .await + .unwrap(); + let line = tokio::time::timeout(std::time::Duration::from_secs(10), stdout.next_line()) + .await + .unwrap() + .unwrap() + .unwrap(); + let response: Value = serde_json::from_str(&line).unwrap(); + assert_eq!(response["result"]["sse"], serde_json::json!(true)); + assert!( + !line.contains('\n'), + "a multi-line data payload would emit more than one line" + ); + + // A notification gets the 202's whole acknowledgement: nothing written. + stdin + .write_all(b"{\"jsonrpc\":\"2.0\",\"method\":\"notifications/initialized\"}\n") + .await + .unwrap(); + let quiet = + tokio::time::timeout(std::time::Duration::from_millis(500), stdout.next_line()).await; + assert!(quiet.is_err(), "a notification must not produce output"); + + child.kill().await.unwrap(); +} + +#[test] +fn repeated_spawns_rewrite_the_server_but_change_nothing_else() { + let home = tempfile::tempdir().unwrap(); + kiro_install(home.path()); + let bin = tempfile::tempdir().unwrap(); + let agents = home.path().join(".kiro/agents"); + fs::create_dir_all(&agents).unwrap(); + fs::write( + agents.join("main.json"), + r#"{"name":"main","allowedTools":["@computer/*"],"tools":["*"]}"#, + ) + .unwrap(); + + let first = inject_tools_mcp(home.path(), &env_with_path(bin.path())).unwrap(); + let Outcome::Injected { + files: first_files, .. + } = first + else { + panic!("expected Injected"); + }; + let second = inject_tools_mcp(home.path(), &env_with_path(bin.path())).unwrap(); + let Outcome::Injected { + files: second_files, + .. + } = second + else { + panic!("expected Injected"); + }; + + let agent = read_json(&agents.join("main.json")); + assert_eq!( + agent["allowedTools"], + json!(["@computer/*"]), + "no duplicate trust entry on the second spawn" + ); + // tools already covers everything via "*"; nothing was appended. + assert_eq!(agent["tools"], json!(["*"])); + // The bridge is always (re)written — it is runtime-owned — but the agent + // file is only rewritten when a merge was actually needed. + assert!(second_files.contains(&bridge_path(home.path()))); + assert!( + !second_files.contains(&agents.join("main.json")), + "an unchanged agent file is not rewritten on every spawn" + ); + assert!( + first_files.contains(&agents.join("main.json")) || { + // unless the first spawn already found it complete + agent["allowedTools"] == json!(["@computer/*"]) + } + ); +}