From 4886ecd5201f3f7f49615e269b487843482dd938 Mon Sep 17 00:00:00 2001 From: cliffhall Date: Sun, 6 Sep 2026 14:03:26 -0400 Subject: [PATCH] docs: address Copilot review round 3 on the CLI smoke-testing guide MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two findings, both real; the first was introduced by the round-2 fix. 1. The envelope table said `result` is present "Always". It is not: `emitResult` returns early for `--app-info` and writes `{appInfo}` alone, with no `result` key — in the `hasApp:false` case too, which is signalled through exit code 2 rather than through a key. So a consumer that requires `.result` breaks on every probe. Documented as a distinct shape rather than a footnote on the table, since that is what it is. Verified against the mcp-app-http fixture: keys are ["appInfo"] with --app-info for a tool with and without an app, and ["appInfo","result"] for the same tool called without it. Worth noting this was self-inflicted — the table arrived in the round-2 fix for `schemaFindings`, and stating a shape more precisely is exactly where an over-broad "always" slips in. 2. The Actions job ran `./smoke.sh` while the guide never tells anyone to chmod +x, and a file copied out of a code block does not carry the mode bit. Now `bash smoke.sh`, with a comment saying why so it is not tidied back. Verified: ./nonexec.sh fails with "permission denied", bash nonexec.sh runs. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01KsKC3HniVF9dCZtDVbYp5B Signed-off-by: cliffhall --- docs/cli-smoke-testing.md | 26 +++++++++++++++++++++----- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/docs/cli-smoke-testing.md b/docs/cli-smoke-testing.md index 336845e6..06410591 100644 --- a/docs/cli-smoke-testing.md +++ b/docs/cli-smoke-testing.md @@ -94,17 +94,30 @@ npx @modelcontextprotocol/inspector --cli node build/index.js \ # → {"result":{"tools":[{"name":"echo","description":"…","inputSchema":{…}}, …]}} ``` -The envelope always carries `"result"`, and up to two optional siblings: +For every method **except** an `--app-info` probe, the envelope carries +`"result"` plus up to two optional siblings: | Key | Present when | | --- | --- | -| `result` | Always. | +| `result` | Always — except under `--app-info`, see below. | | `appInfo` | The result belongs to an [MCP App](./mcp-app-review.md) tool. | | `schemaFindings` | `--strict` is passed to `tools/list` **and** there is at least one portability finding. | +⚠️ **`--app-info` is a different shape, not a variation on this one.** It probes +without invoking the tool, so there is no result to report and the envelope is +`{"appInfo": …}` **alone** — including in the `hasApp:false` case, which is +reported through exit code `2` rather than through a key. A consumer that +requires `.result` will break on every `--app-info` run: + +```bash +mcp-inspector --cli --method tools/call --tool-name --app-info --format json +# → {"appInfo":{"hasApp":true,…}} — no "result" key, in either case +``` + Parse the envelope by key rather than assuming a fixed shape — a consumer that -rejects unknown keys, or that reads `.result` and stops, will silently drop the -`schemaFindings` diagnostics described in [§7](#7-negative-assertions). +reads `.result` and stops will drop the `schemaFindings` diagnostics described in +[§7](#7-negative-assertions), and one that *requires* it will reject the +`--app-info` shape outright. Three more things worth knowing before you build a pipeline on it: @@ -460,7 +473,10 @@ smoke: - uses: actions/setup-node@v7 with: node-version: "22.x" - - run: ./smoke.sh + # `bash smoke.sh`, not `./smoke.sh` — a file copied out of this guide (or + # checked out on a runner that did not preserve the mode bit) is not + # executable, and `./smoke.sh` fails with "Permission denied". + - run: bash smoke.sh env: SERVER_URL: ${{ vars.MCP_SERVER_URL }} ```