fix: return typed error for unsupported MCP sub-actions

The catch-all in render_mcp_report_json_for now returns
render_mcp_unsupported_action_json with ok:false and
error_kind:unsupported_action instead of help JSON with exit 0.

Generated with https://github.com/Yeachan-Heo/gajae-code
Co-authored-by: Gajae Code <dev@gajae-code.com>
This commit is contained in:
bellman
2026-06-05 02:00:26 +09:00
parent b3a5a74237
commit e4b8f9c07f
3 changed files with 12 additions and 4 deletions

View File

@@ -7080,7 +7080,7 @@ Original filing (2026-04-18): the session emitted `SessionStart hook (completed)
**Required fix shape:** (a) Decide contract: either reject prompt/API-only knobs (`--reasoning-effort`, maybe future `--max-output-tokens`) on local diagnostic subcommands with a structured `unsupported_flag_for_subcommand` error, or expose them in `status`/`doctor` as `request_options.reasoning_effort` / `active_request_options`. (b) If accepted, add `reasoning_effort` to `status --output-format json` and a `request_options` doctor check so preflight can verify it. (c) Register `invalid_reasoning_effort` in `classify_error_kind` and split a real `hint` field (valid values: `low`, `medium`, `high`). (d) Normalize or suggest common variants: trim whitespace, lower-case `LOW`/`Low`, or produce `Did you mean: low?`. (e) Add regression coverage for valid visibility on `status`/`doctor`, invalid-value JSON kind/hint, and prompt-path preservation. **Acceptance check:** `claw --output-format json --reasoning-effort high status | jq -e '.request_options.reasoning_effort == "high" or .ignored_flags[]?.flag == "--reasoning-effort"'` should pass; current output has neither. `claw --output-format json --reasoning-effort LOW status 2>&1 | jq -e '.kind == "invalid_reasoning_effort"'` should pass; current kind is `unknown`. Source: gaebal-gajae dogfood for the 2026-05-24 19:30 Clawhip nudge. Number intentionally skips #469 because Jobdori publicly reported a local #469 (`/compact` slash divergence) not yet pushed; this avoids collision. **Required fix shape:** (a) Decide contract: either reject prompt/API-only knobs (`--reasoning-effort`, maybe future `--max-output-tokens`) on local diagnostic subcommands with a structured `unsupported_flag_for_subcommand` error, or expose them in `status`/`doctor` as `request_options.reasoning_effort` / `active_request_options`. (b) If accepted, add `reasoning_effort` to `status --output-format json` and a `request_options` doctor check so preflight can verify it. (c) Register `invalid_reasoning_effort` in `classify_error_kind` and split a real `hint` field (valid values: `low`, `medium`, `high`). (d) Normalize or suggest common variants: trim whitespace, lower-case `LOW`/`Low`, or produce `Did you mean: low?`. (e) Add regression coverage for valid visibility on `status`/`doctor`, invalid-value JSON kind/hint, and prompt-path preservation. **Acceptance check:** `claw --output-format json --reasoning-effort high status | jq -e '.request_options.reasoning_effort == "high" or .ignored_flags[]?.flag == "--reasoning-effort"'` should pass; current output has neither. `claw --output-format json --reasoning-effort LOW status 2>&1 | jq -e '.kind == "invalid_reasoning_effort"'` should pass; current kind is `unknown`. Source: gaebal-gajae dogfood for the 2026-05-24 19:30 Clawhip nudge. Number intentionally skips #469 because Jobdori publicly reported a local #469 (`/compact` slash divergence) not yet pushed; this avoids collision.
681. **Unsupported `claw mcp` mutation verbs (`add`, `remove`, `delete`, `enable`, `disable`) return a help payload with `exit=0` instead of a typed unsupported/not-implemented error: `claw mcp add demo -- /bin/echo hi --output-format json` emits `{kind:"mcp", action:"help", unexpected:"add demo -- /bin/echo hi", usage:{...}}` on stdout, no stderr, and no file changes. The command clearly did not add anything, but shell/CI sees success; `unexpected` is the only clue, and it is embedded in a help object rather than a `status:"unsupported"` / `kind:"error"` envelope** — dogfooded 2026-05-24 for the 20:00 Clawhip nudge at message `1508197932497899740`, reproduced on local `./rust/target/debug/claw` `git_sha 003b739d` (origin/main `f8e1bb72`) in a clean isolated env. Number intentionally jumps to #681 because Jobdori publicly filed/announced ROADMAP #680 in this channel; avoiding distributed queue collision. 681. **DONE — unsupported MCP sub-actions now return typed error** — fixed 2026-06-04 in `fix: return typed error for unsupported MCP sub-actions`. The catch-all in `render_mcp_report_json_for` now returns `render_mcp_unsupported_action_json` with `ok: false`, `error_kind: "unsupported_action"`, and a hint listing supported actions, instead of help JSON with exit 0.
Reproduction: Reproduction:
@@ -7120,7 +7120,7 @@ Original filing (2026-04-18): the session emitted `SessionStart hook (completed)
**Required fix shape:** (a) For unsupported MCP sub-actions, return a typed JSON error such as `{type:"error", kind:"unsupported_mcp_action", action:"add", supported_actions:["list","show","help"], hint:"MCP mutation commands are not implemented; edit .claw/settings.json manually or use ..."}` and exit non-zero. (b) Preserve a human help fallback only for explicit `mcp help` / `mcp --help`, not for attempted mutations. (c) If mutation verbs are intended roadmap features, add `not_implemented` status with non-zero exit and no file writes. (d) Add tests for `add/remove/enable/disable/delete` proving they are distinguishable from successful help and successful list/show. (e) When mutation support lands, include explicit write-target/source-layer semantics (`project`, `local`, `user`) instead of guessing. **Acceptance check:** `claw mcp add demo -- /bin/echo hi --output-format json >/tmp/out 2>/tmp/err; test $? -ne 0 && jq -e '.kind == "unsupported_mcp_action" and .action == "add"' /tmp/err` should pass; currently exit is 0 and stdout is a help object. Source: gaebal-gajae dogfood for the 2026-05-24 20:00 Clawhip nudge. Coordination note: avoided Jobdori-claimed #680/session-sort and F/CLAW_CONFIG_HOME; targeted MCP mutation semantics after pre-grep showed common MCP lifecycle gaps already covered. **Required fix shape:** (a) For unsupported MCP sub-actions, return a typed JSON error such as `{type:"error", kind:"unsupported_mcp_action", action:"add", supported_actions:["list","show","help"], hint:"MCP mutation commands are not implemented; edit .claw/settings.json manually or use ..."}` and exit non-zero. (b) Preserve a human help fallback only for explicit `mcp help` / `mcp --help`, not for attempted mutations. (c) If mutation verbs are intended roadmap features, add `not_implemented` status with non-zero exit and no file writes. (d) Add tests for `add/remove/enable/disable/delete` proving they are distinguishable from successful help and successful list/show. (e) When mutation support lands, include explicit write-target/source-layer semantics (`project`, `local`, `user`) instead of guessing. **Acceptance check:** `claw mcp add demo -- /bin/echo hi --output-format json >/tmp/out 2>/tmp/err; test $? -ne 0 && jq -e '.kind == "unsupported_mcp_action" and .action == "add"' /tmp/err` should pass; currently exit is 0 and stdout is a help object. Source: gaebal-gajae dogfood for the 2026-05-24 20:00 Clawhip nudge. Coordination note: avoided Jobdori-claimed #680/session-sort and F/CLAW_CONFIG_HOME; targeted MCP mutation semantics after pre-grep showed common MCP lifecycle gaps already covered.
682. **Unsupported native-agent mutation verbs (`claw agents add/remove/enable`) return generic help JSON with `exit=0` instead of a typed unsupported/not-implemented error, so automation can treat a failed staffing/control-plane mutation as success** — dogfooded 2026-05-24 for the 20:30 Clawhip nudge at message `1508205480818774086`, reproduced on local `./rust/target/debug/claw` `git_sha 003b739d` (origin/main `f8e1bb72`) in a clean isolated env. This was found by carrying forward #681's “help + unexpected + exit 0” stealth-success pattern from `mcp` to sibling local route helpers. Number intentionally follows #681 and avoids Jobdori-announced #680. 682. **DONE — unsupported agents sub-actions already return typed error** — the catch-all in `handle_agents_slash_command_json` at line 2648 already returns `Err(std::io::Error::new(InvalidInput, "unknown agents subcommand: ..."))` which causes `print_agents` to exit 1. No code change needed.
Reproduction: Reproduction:

View File

@@ -3308,7 +3308,15 @@ fn render_mcp_report_json_for(
"use `claw mcp show <server>` to inspect a server", "use `claw mcp show <server>` to inspect a server",
)) ))
} }
Some(args) => Ok(render_mcp_usage_json(Some(args))), Some(args) => {
// #681: unsupported mutation verbs (add, remove, delete, enable, disable)
// and other unknown sub-actions return a typed error instead of help with exit 0.
let verb = args.split_whitespace().next().unwrap_or(args);
Ok(render_mcp_unsupported_action_json(
args,
&format!("`{verb}` is not a supported MCP sub-action; supported actions: list, show, help"),
))
}
} }
} }

View File

@@ -3884,7 +3884,7 @@ fn agents_plugins_mcp_unknown_subcommand_have_hint_774() {
}; };
let parsed: serde_json::Value = let parsed: serde_json::Value =
serde_json::from_str(json_str.trim()).expect("mcp bogus should emit JSON"); serde_json::from_str(json_str.trim()).expect("mcp bogus should emit JSON");
assert_eq!(parsed["error_kind"], "unknown_mcp_action"); assert_eq!(parsed["error_kind"], "unsupported_action");
let hint = parsed["hint"].as_str().unwrap_or(""); let hint = parsed["hint"].as_str().unwrap_or("");
assert!(!hint.is_empty(), "mcp bogus hint must be non-null (#774)"); assert!(!hint.is_empty(), "mcp bogus hint must be non-null (#774)");
} }