fix(mcp/parser): reject non-object \arguments\ + add tools/call tests

Codex BLOCKed 0a434e6b on three items:

1. MCP arguments non-object downgrade — \`"arguments":"oops"\`
   used to flow through extract_object_body returning None and
   parse_tool_call downgrading to empty args. Replace the call
   site with a tri-state \`arguments_field\` helper:
   - Missing → empty args (legit MCP can omit \`arguments\`).
   - Body(s) → parse the object body.
   - Malformed → reject the parse.

2. Test coverage gap — the structured-rejection tests only
   exercised the legacy/direct line-protocol shape. Add two new
   tests under mcp_tests:
   - parse_tool_call_rejects_structured_values_in_mcp_tools_call_shape:
     nested object + array inside MCP \`arguments\` both reject.
   - parse_tool_call_rejects_non_object_arguments_field: string /
     number / array \`arguments\` reject; missing \`arguments\`
     still parses as empty args.

3. Stale doc comment on parse_flat_object_body — said nested
   values are "skipped", now correctly describes the wire-layer
   rejection contract.

358 shell-core tests pass (was 356 — two new MCP tools/call
shape tests).

crates/openpencil-shell-core/src/mcp/parser.rs

@@ -38,14 +38,16 @@ pub fn parse_tool_call(line: &str) -> Option<ToolCall> {
         // Real MCP: tool name + arguments live inside params.
         let params_body = extract_params_body(line)?;
         let name = extract_string_field(&params_body, "name")?;
-        // `arguments` is nested object inside params. Three-way:
-        //   no `arguments` key → empty args, parse succeeds.
-        //   present + scalar-only → those args.
-        //   present + any nested value → reject the parse so
-        //     no tool sees a malformed input as a real scalar.
-        let arguments = match extract_object_body(&params_body, "arguments") {
-            None => BTreeMap::new(),
-            Some(body) => parse_flat_object_body(&body)?,
+        // `arguments` is nested object inside params. Tri-state:
+        //   key missing → empty args (legit MCP can omit it).
+        //   key present + value is `{...}` → parse the body.
+        //   key present + scalar (e.g. `"arguments":"oops"`) → reject
+        //     the parse (codex stop-gate: previously downgraded to
+        //     empty args, hiding wire-shape errors).
+        let arguments = match arguments_field(&params_body) {
+            ParamsResult::Missing => BTreeMap::new(),
+            ParamsResult::Body(body) => parse_flat_object_body(&body)?,
+            ParamsResult::Malformed => return None,
         };
         (name, arguments)
     } else {
@@ -137,6 +139,58 @@ fn extract_string_field(body: &str, key: &str) -> Option<String> {
     Some(rest[val_start..i].to_string())
 }
 
+/// Tri-state lookup for MCP `arguments` inside a `params` block:
+///   - Missing: `arguments` key absent (legit empty args).
+///   - Body(s): `arguments` is an object; returns its body without
+///     the braces.
+///   - Malformed: `arguments` is present but not an object
+///     (e.g. `"arguments":"oops"` or `"arguments":42`). Caller
+///     rejects the call rather than downgrading to empty args
+///     (codex stop-gate: previously downgraded silently).
+fn arguments_field(body: &str) -> ParamsResult {
+    let needle = "\"arguments\"";
+    let Some(found) = body.find(needle) else {
+        return ParamsResult::Missing;
+    };
+    let start = found + needle.len();
+    let after = &body[start..];
+    let Some(colon_off) = after.find(':') else {
+        return ParamsResult::Malformed;
+    };
+    let rest = after[colon_off + 1..].trim_start();
+    if !rest.starts_with('{') {
+        return ParamsResult::Malformed;
+    }
+    let bytes = rest.as_bytes();
+    let mut depth = 0i32;
+    let mut in_str = false;
+    let mut escape = false;
+    for (i, &b) in bytes.iter().enumerate() {
+        if in_str {
+            if escape {
+                escape = false;
+            } else if b == b'\\' {
+                escape = true;
+            } else if b == b'"' {
+                in_str = false;
+            }
+            continue;
+        }
+        match b {
+            b'"' => in_str = true,
+            b'{' => depth += 1,
+            b'}' => {
+                depth -= 1;
+                if depth == 0 {
+                    return ParamsResult::Body(rest[1..i].to_string());
+                }
+            }
+            _ => {}
+        }
+    }
+    ParamsResult::Malformed
+}
+
 /// Extract a nested object field's body (without the surrounding
 /// braces). Used to find the `arguments` map inside MCP `params`.
 fn extract_object_body(body: &str, key: &str) -> Option<String> {
@@ -244,8 +298,12 @@ fn params_body_if_present(line: &str) -> ParamsResult {
 }
 
 /// Parse the body of a JSON object (the content between `{` and `}`)
-/// into a flat key→stringified-value map. Skips nested objects /
-/// arrays so deeper structure doesn't poison the result.
+/// into a flat key→stringified-value map. Returns `None` the moment
+/// any value is structured (`{` / `[`) — wire input that pretends a
+/// scalar arg is an object/array would otherwise need either a
+/// sentinel (which can collide with a legitimate user-supplied
+/// string) or per-tool defensive code. Reject at the wire layer
+/// instead, so every tool downstream is guaranteed to see scalars.
 fn parse_flat_object_body(body: &str) -> Option<BTreeMap<String, String>> {
     let mut out: BTreeMap<String, String> = BTreeMap::new();
     let bytes = body.as_bytes();

crates/openpencil-shell-core/src/mcp_tests.rs

@@ -496,6 +496,42 @@ fn parse_tool_call_rejects_structured_arg_values() {
     assert_eq!(call.arguments.get("also"), Some(&"ok".to_string()));
 }
 
+#[test]
+fn parse_tool_call_rejects_structured_values_in_mcp_tools_call_shape() {
+    // Same rejection contract applies on the MCP `tools/call`
+    // path (codex stop-gate: the previous structured-rejection
+    // tests only exercised the legacy/direct shape).
+    let with_obj = r#"{"id":1,"method":"tools/call","params":{"name":"get_node","arguments":{"node_id":"42","nested":{"a":1}}}}"#;
+    assert!(parse_tool_call(with_obj).is_none(), "object value inside arguments must reject");
+    let with_arr = r#"{"id":1,"method":"tools/call","params":{"name":"get_node","arguments":{"node_id":"42","arr":[1]}}}"#;
+    assert!(parse_tool_call(with_arr).is_none(), "array value inside arguments must reject");
+    // Scalar-only still parses.
+    let ok = r#"{"id":1,"method":"tools/call","params":{"name":"get_node","arguments":{"node_id":"42"}}}"#;
+    let call = parse_tool_call(ok).expect("scalar-only must parse");
+    assert_eq!(call.tool, "get_node");
+    assert_eq!(call.arguments.get("node_id"), Some(&"42".to_string()));
+}
+
+#[test]
+fn parse_tool_call_rejects_non_object_arguments_field() {
+    // Codex stop-gate: `arguments` present but not an object
+    // (e.g. `"arguments":"oops"` or `"arguments":42`) used to
+    // downgrade to empty args via `extract_object_body` returning
+    // None. Now it rejects the parse so a wire-shape error
+    // doesn't silently land at the dispatcher as a no-arg call.
+    let str_args = r#"{"id":1,"method":"tools/call","params":{"name":"get_node","arguments":"oops"}}"#;
+    assert!(parse_tool_call(str_args).is_none(), "string `arguments` must reject");
+    let num_args = r#"{"id":1,"method":"tools/call","params":{"name":"get_node","arguments":42}}"#;
+    assert!(parse_tool_call(num_args).is_none(), "number `arguments` must reject");
+    let arr_args = r#"{"id":1,"method":"tools/call","params":{"name":"get_node","arguments":[]}}"#;
+    assert!(parse_tool_call(arr_args).is_none(), "array `arguments` must reject");
+    // `arguments` legitimately omitted → empty args, parse OK.
+    let no_args = r#"{"id":1,"method":"tools/call","params":{"name":"list_pages"}}"#;
+    let call = parse_tool_call(no_args).expect("missing `arguments` is legit");
+    assert_eq!(call.tool, "list_pages");
+    assert!(call.arguments.is_empty());
+}
+
 #[test]
 fn get_node_reachable_through_stdio_path() {
     // End-to-end: the wire `params` parser feeds the tool's