docs: address Copilot review round 3 on the CLI smoke-testing guide
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) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KsKC3HniVF9dCZtDVbYp5B
Signed-off-by: cliffhall <cliff@futurescale.com>
This commit is contained in:
parent
d4f2f2b127
commit
4886ecd520
1 changed files with 21 additions and 5 deletions
|
|
@ -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 <server> --method tools/call --tool-name <t> --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 }}
|
||||
```
|
||||
|
|
|
|||
Loading…
Reference in a new issue