Skip to content

ci: repair the MCP Inspector workflow (pin 2.0.0, drive via --config, make the steps able to fail) - #70

Merged
kalidke merged 3 commits into
mainfrom
fix/mcp-inspector-2
Jul 30, 2026
Merged

ci: repair the MCP Inspector workflow (pin 2.0.0, drive via --config, make the steps able to fail)#70
kalidke merged 3 commits into
mainfrom
fix/mcp-inspector-2

Conversation

@kalidke

@kalidke kalidke commented Jul 30, 2026

Copy link
Copy Markdown
Member

Inspector 2.0.0 shipped and turned main red with no change on our side, because the workflow invoked an unpinned npx @modelcontextprotocol/inspector. Fixing that exposed a second, worse problem: most of these steps could not fail.

Why it broke

2.0's CLI stops collecting the stdio target at the first dashed token (clients/cli/build/index.js, parseArgs), so julia --project=... server.jl lost its arguments. Bare julia then read stdin as Julia source and died on the first JSON-RPC frame:

ERROR: syntax: { } vector syntax is discontinued
{"error":{"code":"error","message":"Connection timed out after 15000 ms"}}

Upstream documents the constraint: "There is currently no way to pass a leading-dash argument through to a stdio server on the --cli command line; put it in the server entry's args in a catalog or config file instead." So the launch command now lives in .github/inspector/mcp-servers.json and is selected with --config/--server.

The steps could not fail

Independent of 2.0, four of seven Inspector steps asserted nothing or were fake-green:

  • resources/list and resources/read only ran cat | jq . — an empty {"resources":[]} passed.
  • prompts/get ended in || echo "Inspector may not support prompts/get".
  • The mcp-remote HTTP step ended in || true, with a warn-only else branch.
  • The direct-curl step had no jq -e at all, and an empty session id silently skipped the tools/list check.

Worse, jq -e on an empty file exits 0 (jq 1.6), and a failed Inspector call writes nothing to stdout — it reports the error on stderr and exits non-zero. So ... || true followed by jq -e passes on a total failure. assert-json.sh therefore checks emptiness before the filter, and filters now name expected values (current_time, the birthday URI and its July 31 contents, greeting/movie_analysis, echo) instead of length > 0, which passes against the wrong server.

Changes

.github/inspector/ (new)

  • package.json + package-lock.json pin Inspector 2.0.0 and mcp-remote 0.1.38 with the full transitive graph (300 entries). An exact npx pkg@2.0.0 would still float 2.0.0's own ^ ranges.
  • Under .github/ deliberately: a General-registry tarball ships the whole tree, and a root package.json makes a Julia package look like an npm project.
  • dependabot.yml gains the npm ecosystem for that directory, so the pin is revisited rather than silently rotting — and a Dependabot PR runs this workflow, so the next breaking major arrives as a red PR instead of a red main.
  • assert-json.sh, wait-http.sh (readiness polling replaces sleep 15; also detects a server that died during startup).

Workflow

  • shell: bash workflow-wide so GitHub uses bash -eo pipefail. The implicit default is bash -e with no pipefail, under which a julia crash in sed | julia | tail -1 was masked by tail.
  • actions/setup-node@v7 added — there was none, so the job used whatever Node the runner image shipped, and 2.0 requires ≥ 22.19.0.
  • --format json for a stable stdout contract. It nests the payload under .result, hence the prefixed assertion paths.
  • Primary HTTP coverage now uses the Inspector's native Streamable HTTP support, exercising our transport with no bridge in between. mcp-remote is kept as explicit Claude-Desktop-parity coverage (the docs tell users to configure it) — pinned, and it must pass now.
  • prompts/get with arguments is covered through the Inspector; the comment claiming that was broken was stale (the flag is --prompt-args, plural). The raw JSON-RPC step stays as the one client-library-free path.
  • Direct HTTP/JSON-RPC tests negotiate 2025-11-25 rather than 2025-06-18, so the smoke test exercises the version the package advertises.
  • Dead http-sse push trigger removed.

Verification

Every run: block was extracted from the final YAML and executed locally against the real example servers, then confirmed in CI via workflow_dispatch (run 30509411640): both jobs green, 16 assertions each printing ok [...], Node v22.23.1, readiness in 3s.

Not included

The dry run surfaced an unrelated package bug — src/transports/http.jl:242 calls close(timer) and :546 calls flush(sse_stream), both resolving to the package's own Transport-only methods instead of Base, and both swallowed by catch. Filed separately; out of scope here.

🤖 Generated with Claude Code

kalidke and others added 2 commits July 29, 2026 20:46
The Inspector was invoked as an unpinned `npx @modelcontextprotocol/inspector`.
Inspector 2.0.0 shipped and turned this repo's CI red with no change on our side:
2.0's CLI stops collecting the stdio target at the first dashed token, so
`julia --project=... server.jl` lost its arguments, bare `julia` read stdin as
Julia source, and it died on the first JSON-RPC frame with
"ERROR: syntax: { } vector syntax is discontinued".

Pin it properly rather than chase it:

- package.json + package-lock.json pin @modelcontextprotocol/inspector 2.0.0 and
  mcp-remote 0.1.38 exactly, with the whole transitive graph locked (300 entries).
  An exact `npx pkg@2.0.0` would still float 2.0.0's own ^-ranges.
- The manifest lives under .github/ rather than the repo root: a General-registry
  tarball ships the full tree, and a root package.json makes a Julia package look
  like an npm project.
- dependabot.yml gains the npm ecosystem for that directory, so the pin gets
  revisited instead of silently rotting. A Dependabot PR runs the Inspector
  workflow, so the next breaking major arrives as a red PR, not a red main.
- mcp-servers.json defines the servers via `--config`, which is how 2.0 documents
  passing a command with dashed arguments. Paths are relative to the repo root
  (the Inspector resolves --config against cwd and spawns the server with that
  same cwd; Actions runs `run:` steps from $GITHUB_WORKSPACE).
- assert-json.sh checks emptiness BEFORE `jq -e`, because `jq -e` on an empty file
  exits 0 (jq 1.6) while a failed Inspector call writes nothing to stdout and
  reports the error on stderr — so an assertion on a total failure would pass.
- wait-http.sh polls for readiness instead of `sleep 15`, and notices a server
  that died during startup. The server answers in ~4s locally.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…config

Two problems, one file. The workflow could not fail, and it invoked the Inspector
in a form 2.0 no longer accepts.

Steps that asserted nothing or could not fail:
- resources/list and resources/read only ran `cat | jq .`; an empty
  {"resources":[]} would have passed.
- prompts/get ended in `|| echo "Inspector may not support prompts/get"`, with a
  follow-up block that only printed.
- The mcp-remote HTTP step ended in `|| true`, with a warn-only else branch.
- The direct-curl step ran no `jq -e` at all, and an empty session id silently
  SKIPPED the tools/list check instead of failing.
Every check now goes through assert-json.sh with a filter naming the expected
value (current_time, the birthday URI and its "July 31" contents, greeting and
movie_analysis, echo) rather than `length > 0`, which passes against the wrong
server.

`shell: bash` is set workflow-wide so GitHub uses `bash -eo pipefail`. The
implicit default is `bash -e` with NO pipefail, under which a julia crash in
`sed | julia | tail -1` was masked by tail's exit status.

Inspector invocations now use `--config`/`--server` with `--format json`. Note
--format json nests the payload under `.result`, so assertion paths are prefixed
accordingly. Also:
- actions/setup-node is added (there was none — the job used whatever Node the
  runner image shipped, and 2.0 requires >= 22.19.0).
- The primary HTTP test now uses the Inspector's native Streamable HTTP support,
  exercising our transport with no bridge process in between. The mcp-remote path
  is kept as explicit Claude-Desktop-parity coverage, since the docs tell users to
  configure it, but it is pinned now and must pass.
- prompts/get WITH arguments is covered through the Inspector; the comment claiming
  that was broken was stale (the flag is --prompt-args, plural, and it works). The
  raw JSON-RPC step stays as the one client-library-free path.
- Direct HTTP/JSON-RPC tests negotiate 2025-11-25 instead of 2025-06-18, so the
  smoke test exercises the version the package actually advertises.
- The dead `http-sse` push trigger is removed.

Verified by executing every `run:` block from this file locally against the real
example servers: 20 assertions, all green.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The first PR run caught a bug I introduced: the HTTP job hung and was killed at
`timeout-minutes: 15`.

Cause: `examples/simple_http_server.jl` hardcodes port 3000, and the three HTTP
steps each started their own server. A step ends as soon as its `kill` is issued —
my teardown was `trap 'kill $SERVER_PID' EXIT`, dropping the `wait` the original
workflow had — so the next step's server hit `EADDRINUSE` and died at startup.
wait-http.sh then polled the *previous* step's server, which had already been sent
SIGTERM: it answered `initialize`, so readiness passed, and the mcp-remote bridge
call then blocked forever on a server that was shutting down under it.

Fixes:

- One server for the whole HTTP job, with all four checks against it. Removes the
  cross-step port contention entirely and drops two Julia startups (the whole step
  now runs in ~9s locally).
- Teardown is `kill` AND `wait`, so the child is reaped and the port is actually
  released before the shell exits.
- `timeout 180` on all ten Inspector invocations. This is the guard that matters:
  it converts any hang into a fast, legible failure instead of a 15-minute job
  cancellation that reads like an infrastructure flake.
- wait-http.sh checks liveness with `ps` state instead of `kill -0`, treating a
  zombie as dead. This is a strict improvement rather than the fix for the observed
  hang — bash reaps background children, so `kill -0` was not actually fooled here.

Verified locally by executing the rebuilt step against the real server: 6
assertions green, 9s, port released.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@kalidke
kalidke merged commit a36d949 into main Jul 30, 2026
8 checks passed
@kalidke
kalidke deleted the fix/mcp-inspector-2 branch July 30, 2026 03:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant