fix(mcp): BugBash 2026-05-20 Wave 2 — pin mock to openapi.json + 5 client fixes#10
Closed
mastermanas805 wants to merge 2 commits into
Closed
fix(mcp): BugBash 2026-05-20 Wave 2 — pin mock to openapi.json + 5 client fixes#10mastermanas805 wants to merge 2 commits into
mastermanas805 wants to merge 2 commits into
Conversation
Makes MCP-server testing a hard CI gate. Previously `npm test` ran a shell smoke test (test.sh) against the live api.instanode.dev — not hermetic, not runnable in CI without a cluster, and easy to skip. An MCP-server change could silently break the tools agents depend on. What this adds -------------- - test/integration.test.ts — drives the REAL built server binary over the genuine MCP stdio protocol via the official SDK client. 39 tests covering all 16 registered tools: tool registry + every input schema, success responses, error envelopes (401/402/403/404/400), the multipart create_deploy upload, bearer-token auth (none/valid/bad), the full deploy lifecycle (create→get→redeploy→delete), private deploys + tier gating, and malformed-input rejection. - test/mock-api.ts — hermetic in-process http.Server mock of the agent API (the https://api.instanode.dev/openapi.json contract). The suite runs in CI with zero network, zero cluster, zero secrets. It keeps a ledger of every resource/deployment so the cleanup sweep can assert nothing leaked. - test/live-smoke.test.ts — optional, build-flagged (INSTANODE_LIVE_SMOKE=1) provision-then-teardown smoke test against a real backend; skipped by default. Deletes its resource in a finally block. Resource cleanup (mandatory) ---------------------------- Every test that creates a paid resource or a deployment tears it down. The after() hook runs a final sweep that deletes every still-live deployment + paid resource on the mock, then asserts the deletable ledger is empty. mock.close() runs in a finally so a failed assertion can't leave the server open. CI gate ------- - package.json `test` script now runs the hermetic suite via `node --test` (pretest builds server + tests). Local `npm test` == the CI gate exactly. Legacy shell smoke test moved to `test:smoke`. - .github/workflows/ci.yml runs `npm test` on every push + PR to master/main; a non-zero exit blocks the merge. - zod added as an explicit dependency (index.ts imports it directly; it was only resolving transitively via the MCP SDK). npm test: 39 passed, 0 failed (live-smoke SKIP), exit 0, fully hermetic. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ient fixes
Resolves the six T17 findings (2 P0s, 4 P1s) flagged in BUGHUNT-2026-05-20-T17.md.
Each fix carries a CI-run regression test under
"BugBash 2026-05-20 T17 ..." in test/integration.test.ts so a future revert
fails the gate immediately.
T17 P0-1 — redeploy(): the live api documents POST /deploy/{id}/redeploy as a
bare 202 with no body. The previous client typed it as DeployGetResult and
the index.ts handler dereferenced result.item.app_id, throwing
"TypeError: Cannot read properties of undefined (reading 'app_id')" on every
call against the real api. The hermetic mock fabricated {ok, item} so the
suite green-lit a contract violation. Now:
- client.redeploy() returns {ok, id} and awaits the empty 2xx body
without dereferencing anything;
- index.ts redeploy handler prints "Redeploy accepted for <id>" and
points the agent at get_deployment for the next status;
- mock-api.ts emits a bare 202 (Content-Length: 0, no body) so the
test path actually exercises the empty-body code.
Regression test: "redeploy resolves successfully when the api returns 202
with no body (no TypeError on result.item.app_id)".
T17 P0-2 — get_api_token PAT-creating-PAT: the api enforces "PATs cannot
mint other PATs" with a 403 + code=pat_cannot_mint_pat. Since the dashboard
and get_api_token itself both mint PATs, the typical INSTANODE_TOKEN IS a
PAT, so the tool 403'd 100% of the time in its documented "rotate as needed"
use case. The previous generic 403 error gave the agent no path forward.
Now formatError() maps the code to a clear "use a session JWT — sign in at
https://instanode.dev/dashboard, then create a key from the API token UI"
headline, and the tool description explicitly states PATs cannot mint PATs.
The mock now models a PAT_TOKEN fixture that 403s with the canonical code.
Regression tests:
- "get_api_token with a PAT bearer surfaces the 'use a session JWT'
message (not a generic 403)";
- "get_api_token tool description mentions the session-vs-PAT requirement".
T17 P1-1 — name schema regex: the api enforces
^[A-Za-z0-9][A-Za-z0-9 _-]*$ (start-alnum then letters/digits/spaces/
underscores/hyphens). The previous zod schema was min(1).max(64) only —
names like "-bad" and "@x" passed locally and 400'd on the api. The shared
nameArg and create_deploy.name now both carry the regex, sourced as a
named constant API_NAME_PATTERN (matches the MEMORY rule "use named
constants, not inline strings"). The mock's name validator also enforces
the regex so a single-site fallacy in either layer fails the suite.
Regression tests: 11 cases (6 rejected + 5 accepted) per create_postgres,
1 case on create_deploy, plus a registry-iterating coverage test that fails
if any future create_* tool ships without the pattern.
T17 P1-3 — npm audit clean: 4 vulns (1 high fast-uri, 3 moderate hono +
ip-address + express-rate-limit) → 0 vulns after npm audit fix.
The CI gate "npm install && npm run build && npm test" now reports
"found 0 vulnerabilities".
T17 P1-6 — User-Agent sourced from package.json: client.ts no longer
contains the hardcoded literal "instanode-mcp/0.11.0" in two places.
A single PKG_VERSION + USER_AGENT constant is resolved at module load
from the installed package.json — every release naturally rolls forward.
Regression test: "client UA string equals 'instanode-mcp/<package.json
version>' (no hardcoded 0.11.0 literal)" reads the real package.json,
spawns the mcp against a UA-capturing http.Server, and asserts every
inbound request carries the resolved UA.
T17 P0/P1-5 — mock pinned to openapi.json: pulled the live spec from
https://api.instanode.dev/openapi.json and aligned mock-api.ts's
response shapes to it. Per-endpoint contract-pinning tests under
"BugBash 2026-05-20 T17 P0/P1 — mock-api contract pinning" lock down:
- /deploy/:id/redeploy → bare 202, empty body
- /api/v1/auth/api-keys → 403 pat_cannot_mint_pat for PATs
- /api/v1/auth/api-keys → 400 on missing name, 400 on invalid scope
- /claim → 200 ClaimResponse {ok, team_id, user_id, session_token,
message}, NOT the legacy 201 direct-claim shape
- /db/new (and siblings) → 400 invalid_name on names that fail the
api regex
Other:
- Bumped version 0.11.0 → 0.11.1 (the changes above are non-breaking
for the tool surface but reshape internals).
- Updated server.json version to match.
- Added PAT_TOKEN fixture export to mock-api.ts so future PAT-specific
tests can reuse it.
Gate output (npm install && npm run build && npm test):
- npm install: 0 vulnerabilities
- npm run build: tsc clean, 0 errors
- npm test: 62 pass / 0 fail / 0 skip (live-smoke SKIP by design)
- Previously: 39 pass — the 23 new tests all map to BugBash findings.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This was referenced May 20, 2026
Member
Author
|
Closing as superseded. This PR's fixes (T17 P0-1 redeploy crash, P0-2 PAT clarity, P1 name regex, P1 npm audit, P1 User-Agent) were re-landed on master directly via PR #12 (merged 2026-05-21) since this PR had been sitting open unmerged. The wave3 P2 fixes (tarball cap, allowed_ips invariant) from PR #11 are also on master. Nothing here is missing from master. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resolves the six T17 findings (2 P0s, 4 P1s) flagged in
BUGHUNT-2026-05-20-T17.md. Every fix carries a CI-run regression test underBugBash 2026-05-20 T17 ...intest/integration.test.tsso a future revertfails the gate immediately. Branched off
origin/master.Findings fixed
redeploycrashes on real API (expects{ok,item}, API returns bare 202)client.redeploy()returns{ok, id}and tolerates the empty 2xx body;index.tsredeploy handler no longer dereferencesresult.item.app_id; mock now emits a bare 202 with no bodyredeploy resolves successfully when the api returns 202 with no body (no TypeError on result.item.app_id)get_api_token403s because PATs can't mint PATs (the typicalINSTANODE_TOKENis a PAT)formatErrormapspat_cannot_mint_patto a clear "use a session JWT — sign in at https://instanode.dev/dashboard" headline; tool description rewritten to state the constraint plainly; mock models the 403 against a newPAT_TOKENfixtureget_api_token with a PAT bearer surfaces the 'use a session JWT' message (not a generic 403)+get_api_token tool description mentions the session-vs-PAT requirementmin(1).max(64)doesn't mirror the api's^[A-Za-z0-9][A-Za-z0-9 _-]*$API_NAME_PATTERN) to the sharednameArgANDcreate_deploy.name; mock now enforces the same pattern withinvalid_namecreate_postgres, 1 case oncreate_deploy, plus a registry-iterating coverage test that fails if any futurecreate_*ships without the patternfast-uri, 3 moderate)npm audit fix→ 0 vulnerabilitiesnpm auditruns innpm installstep)User-Agenthardcodesinstanode-mcp/0.11.0in two placespackage.jsonat module load (PKG_VERSION+USER_AGENTconstants); bothheaders()andauthHeaders()read the same constclient UA string equals 'instanode-mcp/<package.json version>' (no hardcoded 0.11.0 literal)openapi.json— masks the bugs abovehttps://api.instanode.dev/openapi.jsonand aligned mock-api.ts shapes:/deploy/{id}/redeploybare 202;/api/v1/auth/api-keys403 for PATs / 400 missing name / 400 invalid scope;/claim200ClaimResponse {ok, team_id, user_id, session_token, message}(not the legacy 201 direct-claim); provisioning routes enforce the live name regexmock-api contract pinningGate output
Previously 39 pass / 0 fail — the 23 new tests all map to BugBash findings
(redeploy bare-202, get_api_token PAT, name regex × 13, registry coverage,
UA from package.json, plus 6 mock contract-pin tests).
Versioning
Bumped
0.11.0 → 0.11.1inpackage.json+server.json. The User-Agenttest reads the resolved version dynamically so it stays green across future
bumps.
Not in scope
delete_resourcedescription wording) — informational, nobehavior change needed.
claim_tokenchain documented as walking throughget_api_token) — needs a product call on whether to remove the chainadvice or wire a session-issuing flow; left for follow-up.
🤖 Generated with Claude Code