Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 40 additions & 21 deletions crates/sp-harness/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,31 +46,42 @@ fn json_keys(text: &str, key: &str) -> Vec<String> {
let Ok(value) = serde_json::from_str::<serde_json::Value>(text) else {
return Vec::new();
};
value
.get(key)
.and_then(serde_json::Value::as_object)
.map(json_servers)
.unwrap_or_default()
mcp_entries(&value, key).keys().cloned().collect()
}

/// The servers in a JSON map. Some configs nest them one level deeper,
/// under `servers`.
fn json_servers(map: &serde_json::Map<String, serde_json::Value>) -> Vec<String> {
let mut names = Vec::new();
for (name, server) in map {
let Some(fields) = server.as_object() else {
continue;
};
let defines = ["type", "command", "url", "httpUrl", "serverUrl"]
.iter()
.any(|field| fields.contains_key(*field));
if name == "servers" && !defines {
names.extend(json_servers(fields));
} else {
names.push(name.clone());
type Map = serde_json::Map<String, serde_json::Value>;

/// The `servers` object under an assistant's MCP key, if the servers are
/// nested one level deeper (OpenCode 2's `mcp.servers`). A server that
/// happens to be called `servers` doesn't count.
pub fn nested_servers(map: &Map) -> Option<&Map> {
let fields = map.get("servers")?.as_object()?;
let defines = ["type", "command", "url", "httpUrl", "serverUrl"]
.iter()
.any(|field| fields.contains_key(*field));
(!defines).then_some(fields)
}

/// The server entries under `key` in a parsed config, by name: the ones
/// nested under `servers` and any written beside it.
pub fn mcp_entries(value: &serde_json::Value, key: &str) -> Map {
let Some(map) = value.get(key).and_then(serde_json::Value::as_object) else {
return Map::new();
};
let nested = nested_servers(map);
let mut entries: Map = map
.iter()
.filter(|(name, server)| {
server.is_object() && !(nested.is_some() && name.as_str() == "servers")
})
.map(|(name, server)| (name.clone(), server.clone()))
.collect();
for (name, server) in nested.into_iter().flatten() {
if server.is_object() {
entries.insert(name.clone(), server.clone());
}
}
names
entries
}

fn toml_keys(text: &str, key: &str) -> Vec<String> {
Expand Down Expand Up @@ -178,6 +189,14 @@ mod tests {
);
assert_eq!(flat, ["fs"]);
assert_eq!(nested, ["a", "b"]);
// Written both ways in one file, both count; values that aren't
// servers don't.
let mut both = json_keys(
r#"{"mcp":{"timeout":5,"old":{"command":["y"]},"servers":{"a":{}}}}"#,
"mcp",
);
both.sort();
assert_eq!(both, ["a", "old"]);
// A server that happens to be called `servers` is still one.
assert_eq!(
json_keys(r#"{"mcp":{"servers":{"type":"local"}}}"#, "mcp"),
Expand Down
8 changes: 5 additions & 3 deletions crates/sp-harness/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
mod config;
mod files;

pub use config::strip_jsonc;
pub use config::{mcp_entries, nested_servers, strip_jsonc};
pub use files::{IGNORED_DIRS, ignored_file};

use serde::{Deserialize, Serialize};
Expand Down Expand Up @@ -509,7 +509,8 @@ impl Places {
pub enum McpFormat {
/// `mcpServers`, `type` stdio/http/sse (Claude Code, Cursor, Factory).
Claude,
/// `mcp`, `local` with a command array, or `remote` (OpenCode).
/// `mcp.servers` (`mcp` in OpenCode 1), `local` with a command array, or
/// `remote` (OpenCode).
OpenCode,
/// `[mcp_servers]` in TOML (Codex).
Codex,
Expand Down Expand Up @@ -618,7 +619,8 @@ pub enum PluginFormat {
/// `enabledPlugins` and `extraKnownMarketplaces` in `settings.json`
/// (Claude Code, Factory).
Claude,
/// npm packages in the `plugin` list of `opencode.json` (OpenCode).
/// npm packages in the `plugins` list of `opencode.json` (OpenCode;
/// `plugin` in OpenCode 1).
OpenCode,
}

Expand Down
122 changes: 72 additions & 50 deletions crates/sp-sync/src/jsonc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -176,12 +176,12 @@ fn value_of(text: &str, member: &Member) -> Option<Value> {
.ok()
}

/// Sets `key` (an object at the top level) to hold `entries` for the names
/// SkillPouch owns: `managed` names not in `entries` are removed, others
/// are left as the user has them. Returns the new text.
/// Sets the object at `path` (keys from the top) to hold `entries` for the
/// names SkillPouch owns: `managed` names not in `entries` are removed,
/// others are left as the user has them. Returns the new text.
pub fn set_entries(
text: &str,
key: &str,
path: &[&str],
entries: &BTreeMap<String, Value>,
managed: &std::collections::BTreeSet<String>,
) -> Parsed<String> {
Expand All @@ -195,21 +195,8 @@ pub fn set_entries(
if text.as_bytes().get(root_open) != Some(&b'{') {
return Err("it isn't a JSON object".into());
}
let root = object(text.as_bytes(), root_open)?;
let servers = match root.members.iter().find(|m| m.name == key) {
Some(member) if text.as_bytes()[member.value_start] == b'{' => member.value_start,
Some(_) => return Err(format!("`{key}` isn't an object")),
None if entries.is_empty() => return Ok(text),
None => {
let member = format!("{}: {{}}", serde_json::to_string(key).unwrap_or_default());
text = insert_member(&text, &root, &member, "", &unit);
let root = object(text.as_bytes(), root_open)?;
root.members
.iter()
.find(|m| m.name == key)
.map(|m| m.value_start)
.ok_or("couldn't add the key")?
}
let Some(servers) = open_path(&mut text, root_open, path, !entries.is_empty(), &unit)? else {
return Ok(text);
};

// Changes inside the object, applied one at a time on fresh offsets.
Expand Down Expand Up @@ -261,27 +248,9 @@ pub fn set_value(text: &str, path: &[&str], value: Option<&Value>) -> Parsed<Str
if text.as_bytes().get(root) != Some(&b'{') {
return Err("it isn't a JSON object".into());
}
let mut at = root;
for key in parents {
let current = object(text.as_bytes(), at)?;
match current.members.iter().find(|m| m.name == *key) {
Some(member) if text.as_bytes()[member.value_start] == b'{' => at = member.value_start,
Some(_) => return Err(format!("`{key}` isn't an object")),
None if value.is_none() => return Ok(text),
None => {
let base = member_indent(&text, &current, &unit);
let member = format!("{}: {{}}", serde_json::to_string(key).unwrap_or_default());
text = insert_member(&text, &current, &member, &base, &unit);
let current = object(text.as_bytes(), at)?;
at = current
.members
.iter()
.find(|m| m.name == *key)
.map(|m| m.value_start)
.ok_or("couldn't add the key")?;
}
}
}
let Some(at) = open_path(&mut text, root, parents, value.is_some(), &unit)? else {
return Ok(text);
};
let current = object(text.as_bytes(), at)?;
let base = member_indent(&text, &current, &unit);
let index = current.members.iter().position(|m| m.name == *last);
Expand All @@ -307,6 +276,38 @@ pub fn set_value(text: &str, path: &[&str], value: Option<&Value>) -> Parsed<Str
Ok(text)
}

/// Where the object at `path` (keys from the object at `at`) opens, adding
/// the missing objects on the way when `create`; `None` if one is missing.
fn open_path(
text: &mut String,
mut at: usize,
path: &[&str],
create: bool,
unit: &str,
) -> Parsed<Option<usize>> {
for key in path {
let current = object(text.as_bytes(), at)?;
match current.members.iter().find(|m| m.name == *key) {
Some(member) if text.as_bytes()[member.value_start] == b'{' => at = member.value_start,
Some(_) => return Err(format!("`{key}` isn't an object")),
None if !create => return Ok(None),
None => {
let base = member_indent(text, &current, unit);
let member = format!("{}: {{}}", serde_json::to_string(key).unwrap_or_default());
*text = insert_member(text, &current, &member, &base, unit);
let current = object(text.as_bytes(), at)?;
at = current
.members
.iter()
.find(|m| m.name == *key)
.map(|m| m.value_start)
.ok_or("couldn't add the key")?;
}
}
}
Ok(Some(at))
}

/// How the members of `object` are indented (one level in from its line).
fn member_indent(text: &str, object: &Object, unit: &str) -> String {
match object.members.first() {
Expand Down Expand Up @@ -407,7 +408,7 @@ mod tests {
let managed = BTreeSet::from(["old".to_string(), "gh".to_string()]);
let out = set_entries(
text,
"mcp",
&["mcp"],
&entries(&[("gh", json!({"type": "remote", "url": "u"}))]),
&managed,
)?;
Expand All @@ -425,7 +426,7 @@ mod tests {
assert_eq!(
set_entries(
&out,
"mcp",
&["mcp"],
&entries(&[("gh", json!({"type": "remote", "url": "u"}))]),
&managed
)?,
Expand All @@ -434,7 +435,7 @@ mod tests {
// Changed: only that value is rewritten.
let changed = set_entries(
&out,
"mcp",
&["mcp"],
&entries(&[("gh", json!({"url": "v"}))]),
&managed,
)?;
Expand All @@ -447,14 +448,14 @@ mod tests {
fn creates_the_key_and_the_file() -> Parsed<()> {
let out = set_entries(
"",
"mcpServers",
&["mcpServers"],
&entries(&[("a", json!({"command": "x"}))]),
&BTreeSet::new(),
)?;
assert_eq!(parsed(&out)["mcpServers"]["a"]["command"], "x");
let out = set_entries(
"{\n\t\"theme\": 1\n}",
"mcpServers",
&["mcpServers"],
&entries(&[("a", json!({"command": "x"}))]),
&BTreeSet::new(),
)?;
Expand All @@ -463,20 +464,41 @@ mod tests {
Ok(())
}

#[test]
fn sets_entries_in_a_nested_object() -> Parsed<()> {
let path = ["mcp", "servers"];
let a = entries(&[("a", json!({"command": ["x"]}))]);
let out = set_entries(
"{\n // mine\n \"model\": \"x\"\n}\n",
&path,
&a,
&BTreeSet::new(),
)?;
assert!(out.contains("// mine"), "{out}");
assert_eq!(parsed(&out)["mcp"]["servers"]["a"]["command"], json!(["x"]));
assert_eq!(set_entries(&out, &path, &a, &BTreeSet::new())?, out);
// Nothing to set, nothing created.
assert_eq!(
set_entries("{}", &path, &BTreeMap::new(), &BTreeSet::new())?,
"{}"
);
Ok(())
}

#[test]
fn removing_the_last_entry_leaves_valid_json() -> Parsed<()> {
let text = "{\"mcpServers\": {\n \"a\": {},\n \"b\": {}\n}}";
let out = set_entries(
text,
"mcpServers",
&["mcpServers"],
&BTreeMap::new(),
&BTreeSet::from(["b".to_string()]),
)?;
let value: Value = serde_json::from_str(&out).map_err(|e| format!("{e}: {out}"))?;
assert_eq!(value["mcpServers"], json!({"a": {}}));
let out = set_entries(
&out,
"mcpServers",
&["mcpServers"],
&BTreeMap::new(),
&BTreeSet::from(["a".to_string()]),
)?;
Expand Down Expand Up @@ -507,16 +529,16 @@ mod tests {

#[test]
fn refuses_what_it_cannot_parse() {
assert!(set_entries("[1]", "mcp", &BTreeMap::new(), &BTreeSet::new()).is_err());
assert!(set_entries("[1]", &["mcp"], &BTreeMap::new(), &BTreeSet::new()).is_err());
assert!(
set_entries(
"{\"mcp\": 3}",
"mcp",
&["mcp"],
&entries(&[("a", json!({}))]),
&BTreeSet::new()
)
.is_err()
);
assert!(set_entries("{\"a\" 1}", "mcp", &BTreeMap::new(), &BTreeSet::new()).is_err());
assert!(set_entries("{\"a\" 1}", &["mcp"], &BTreeMap::new(), &BTreeSet::new()).is_err());
}
}
Loading
Loading