fix(mcp): nest ping identity under _meta so strict clients accept the server
The MCP spec says a ping result is empty, but both ping formatters put the OpenPencil discovery identity (server/mode/token) at the result top level. Gemini CLI validates ping with the TS SDK's strict EmptyResultSchema and marked the server disconnected with 'Unrecognized keys: server, mode, token' (issue #199). _meta is the spec's sanctioned extension point and the only key the strict schema permits, so the identity now rides there; the op CLI reads _meta first and falls back to the legacy top-level shape so it still discovers a running pre-0.8.3 editor. (A pre-0.8.3 op CLI cannot discover a 0.8.3 live editor — the CLI ships with the app, so only a stale op on PATH hits this.)
This commit is contained in:
parent
bc1f35ef9b
commit
1b020cebb5
|
|
@ -210,10 +210,21 @@ fn ping_result(port: u16) -> Option<Value> {
|
|||
if !(200..300).contains(&status) {
|
||||
return None;
|
||||
}
|
||||
let value = serde_json::from_str::<Value>(&body).ok()?;
|
||||
ping_reply_identity(&body)
|
||||
}
|
||||
|
||||
/// Extract the OpenPencil identity object from a raw `ping` reply, or
|
||||
/// `None` for a non-OpenPencil responder. Servers ≥ 0.8.3 nest the
|
||||
/// identity under `result._meta` (a spec-compliant ping result is empty
|
||||
/// apart from `_meta` — strict clients reject top-level extras, issue
|
||||
/// #199); older servers reported it at the `result` top level, so fall
|
||||
/// back there to keep discovering a still-running pre-0.8.3 editor.
|
||||
fn ping_reply_identity(body: &str) -> Option<Value> {
|
||||
let value = serde_json::from_str::<Value>(body).ok()?;
|
||||
let result = value.get("result")?;
|
||||
if result.get("server").and_then(Value::as_str) == Some(MCP_SERVER_NAME) {
|
||||
Some(result.clone())
|
||||
let identity = result.get("_meta").unwrap_or(result);
|
||||
if identity.get("server").and_then(Value::as_str) == Some(MCP_SERVER_NAME) {
|
||||
Some(identity.clone())
|
||||
} else {
|
||||
None
|
||||
}
|
||||
|
|
@ -326,4 +337,29 @@ mod tests {
|
|||
));
|
||||
assert!(!is_web_canvas_health("not json"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn ping_identity_reads_meta_and_falls_back_to_legacy_top_level() {
|
||||
// ≥ 0.8.3 servers nest the identity under `result._meta` (spec-empty
|
||||
// ping result, issue #199).
|
||||
let meta = ping_reply_identity(
|
||||
r#"{"jsonrpc":"2.0","id":0,"result":{"_meta":{"server":"openpencil-mcp","mode":"live","token":"t-1"}}}"#,
|
||||
)
|
||||
.expect("meta identity accepted");
|
||||
assert_eq!(meta.get("token").and_then(Value::as_str), Some("t-1"));
|
||||
// A still-running pre-0.8.3 editor reports identity at the result top
|
||||
// level — discovery must keep working against it.
|
||||
let legacy = ping_reply_identity(
|
||||
r#"{"jsonrpc":"2.0","id":0,"result":{"server":"openpencil-mcp","mode":"headless","token":"t-2"}}"#,
|
||||
)
|
||||
.expect("legacy identity accepted");
|
||||
assert_eq!(legacy.get("token").and_then(Value::as_str), Some("t-2"));
|
||||
// A foreign server (spec-compliant empty result, or another
|
||||
// product's marker) is never treated as OpenPencil.
|
||||
assert!(ping_reply_identity(r#"{"jsonrpc":"2.0","id":0,"result":{}}"#).is_none());
|
||||
assert!(ping_reply_identity(
|
||||
r#"{"jsonrpc":"2.0","id":0,"result":{"_meta":{"server":"someone-else"}}}"#
|
||||
)
|
||||
.is_none());
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -157,10 +157,14 @@ pub(super) fn make_live_token() -> String {
|
|||
|
||||
/// Live `ping` reply: OpenPencil identity + `mode:"live"` + the instance
|
||||
/// token, so `op` can distinguish this live canvas server from the
|
||||
/// headless file server and confirm it owns the discovery file.
|
||||
/// headless file server and confirm it owns the discovery file. The
|
||||
/// identity rides in `_meta` — the MCP spec's extension point — because
|
||||
/// strict clients validate a ping result as exactly `{ _meta? }` and
|
||||
/// reject top-level extras (Gemini CLI, issue #199); see
|
||||
/// `mcp_serve::ping_response`.
|
||||
pub(super) fn live_ping_response(id_raw: &str, token: &str) -> String {
|
||||
format!(
|
||||
r#"{{"jsonrpc":"2.0","id":{id_raw},"result":{{"server":"{}","mode":"live","token":"{}"}}}}"#,
|
||||
r#"{{"jsonrpc":"2.0","id":{id_raw},"result":{{"_meta":{{"server":"{}","mode":"live","token":"{}"}}}}}}"#,
|
||||
crate::mcp_serve::MCP_SERVER_NAME,
|
||||
crate::mcp_serve::json_escape(token)
|
||||
)
|
||||
|
|
|
|||
|
|
@ -13,6 +13,34 @@ fn live_ping_response_carries_identity_mode_and_token() {
|
|||
assert!(resp.contains(r#""id":3"#), "{resp}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn live_ping_result_is_spec_empty_apart_from_meta() {
|
||||
// The live server is the one third-party MCP clients attach to, so it
|
||||
// is the path that regressed in issue #199: a ping result must contain
|
||||
// NOTHING but `_meta` or strict validators (Gemini CLI's
|
||||
// `EmptyResultSchema.strict()`) mark the server disconnected. The
|
||||
// identity trio rides inside `_meta`.
|
||||
let resp = live_ping_response("3", "abc-123");
|
||||
let value: serde_json::Value = serde_json::from_str(&resp).expect("valid JSON");
|
||||
let result = value
|
||||
.get("result")
|
||||
.expect("has result")
|
||||
.as_object()
|
||||
.unwrap();
|
||||
assert_eq!(
|
||||
result.keys().collect::<Vec<_>>(),
|
||||
vec!["_meta"],
|
||||
"live ping result must contain only _meta: {resp}"
|
||||
);
|
||||
let meta = &result["_meta"];
|
||||
assert_eq!(
|
||||
meta.get("server").and_then(|v| v.as_str()),
|
||||
Some(crate::mcp_serve::MCP_SERVER_NAME)
|
||||
);
|
||||
assert_eq!(meta.get("mode").and_then(|v| v.as_str()), Some("live"));
|
||||
assert_eq!(meta.get("token").and_then(|v| v.as_str()), Some("abc-123"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn make_live_token_is_nonempty_and_structured() {
|
||||
let token = make_live_token();
|
||||
|
|
|
|||
|
|
@ -46,13 +46,21 @@ pub fn ping_response(id_raw: &str, token: Option<&str>) -> String {
|
|||
// confirm the pid in its manager file owns this port. The token is
|
||||
// passed in (sourced from the env at the call site) so this stays a
|
||||
// pure formatter — testable without mutating process-global env.
|
||||
//
|
||||
// The identity fields MUST live under `_meta`, never at the `result`
|
||||
// top level: the MCP spec says a ping result is empty, and strict
|
||||
// clients validate it as exactly `{ _meta? }` (Gemini CLI's
|
||||
// `EmptyResultSchema.strict()` rejected the old top-level shape with
|
||||
// "Unrecognized keys: server, mode, token" — issue #199). `_meta` is
|
||||
// the spec's sanctioned extension point, so the CLI handshake keeps
|
||||
// working without breaking third-party compliance checks.
|
||||
match token {
|
||||
Some(token) => format!(
|
||||
r#"{{"jsonrpc":"2.0","id":{id_raw},"result":{{"server":"{MCP_SERVER_NAME}","mode":"headless","token":"{token}"}}}}"#
|
||||
r#"{{"jsonrpc":"2.0","id":{id_raw},"result":{{"_meta":{{"server":"{MCP_SERVER_NAME}","mode":"headless","token":"{token}"}}}}}}"#
|
||||
),
|
||||
None => {
|
||||
format!(
|
||||
r#"{{"jsonrpc":"2.0","id":{id_raw},"result":{{"server":"{MCP_SERVER_NAME}"}}}}"#
|
||||
r#"{{"jsonrpc":"2.0","id":{id_raw},"result":{{"_meta":{{"server":"{MCP_SERVER_NAME}"}}}}}}"#
|
||||
)
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -24,6 +24,31 @@ fn ping_response_embeds_headless_token_when_present() {
|
|||
assert!(resp.contains(r#""server":"openpencil-mcp""#), "{resp}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn ping_response_result_is_spec_empty_apart_from_meta() {
|
||||
// The MCP spec says a ping result is empty; strict clients (Gemini
|
||||
// CLI's `EmptyResultSchema.strict()`, issue #199) reject any top-level
|
||||
// key other than `_meta`. The OpenPencil identity must therefore ride
|
||||
// INSIDE `_meta` — a top-level `server`/`mode`/`token` is a regression.
|
||||
for resp in [ping_response("1", None), ping_response("2", Some("tok-9"))] {
|
||||
let value: serde_json::Value = serde_json::from_str(&resp).expect("valid JSON");
|
||||
let result = value
|
||||
.get("result")
|
||||
.expect("has result")
|
||||
.as_object()
|
||||
.unwrap();
|
||||
assert_eq!(
|
||||
result.keys().collect::<Vec<_>>(),
|
||||
vec!["_meta"],
|
||||
"ping result must contain only _meta: {resp}"
|
||||
);
|
||||
assert_eq!(
|
||||
result["_meta"].get("server").and_then(|v| v.as_str()),
|
||||
Some(MCP_SERVER_NAME)
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn sanitize_token_rejects_unsafe_or_empty() {
|
||||
// Safe tokens (the CLI emits pid-nanos hex) pass through.
|
||||
|
|
|
|||
Loading…
Reference in a new issue