Repository navigation
ci: install the MCP Inspector before its test calls - #143
Conversation
When_InspectorCallsAStatelessHttpProjection gives each Inspector call three minutes, and that budget included npx installing @modelcontextprotocol/inspector@2.6.0 from an empty cache. That install now takes about three to five minutes on its own, so the job timed out with no output on every pull request, whatever the change. A step now installs the same package into the npx cache first; the test's calls then start in about a second.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
@codex review |
|
Codex Review: Didn't find any major issues. 🚀 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
autocarl
left a comment
autocarl
left a comment
There was a problem hiding this comment.
Approved exact head 9f93c856ff5887ce56f96ab83ca269fb9ab93f53.
The change separates cold Inspector installation from the three-minute protocol-call budget. Workflow/test package versions match; the minimal test environment retains HOME/PATH and therefore the default npx cache. Read-only job permissions and non-persisted checkout credentials remain intact. The absence of a committed npm dependency lock is an explicitly deferred pre-existing reproducibility limitation, not a regression introduced here.
Parent Release solution build with -warnaserror: zero warnings/errors; full suite 2,393 passed, 0 failed, 3 expected skips. All applicable live CI checks pass. I read back the MCP HTTP job's steps: installation and the actual Inspector test both succeeded, providing the workflow's end-to-end evidence. Independent source audit found no introduced P0–P2; its isolated pinned-Node cold-cache experiment was blocked by tool consent and is not claimed as runtime validation.
Reviewed introduced patch from merge base 9397b1b, separate from the advanced base. No remaining concrete blocker identified; no merge performed.
The MCP HTTP (Inspector) job now times out on every pull request, whatever the change (#139, and a rerun of #137's previously green job):
MCP Inspector CLI did not finish within 00:03:00, with empty stdout and stderr.Cause
Given_McpInspectorCligives each Inspector call three minutes (RunProcessAsync(process, TimeSpan.FromMinutes(3))), and that budget includednpx -y @modelcontextprotocol/inspector@2.6.0installing the package. CI always starts from an empty npm cache, and that install alone now takes about three to five minutes (189 s and 308 s locally from an empty cache), so the process is killed while still installing.Change
A job step installs the same pinned package into the npx cache before the test. The test keeps
HOME, so itsnpxcalls reuse that cache: locally, the call after the warm-up took one second. The job timeout goes from 15 to 25 minutes to fit the install. A comment ties the version in the workflow toHttpInspectorPackage.Verification
Installing the Inspector from a committed lockfile would also stop npm from re-resolving its open dependency ranges on every run; that is left as a possible follow-up.