Add manifest tool-list parity tests - #102
Open
IvanK-Nitro wants to merge 2 commits into
Open
Conversation
The `tools` array in manifest.json is maintained by hand -- `mcpb pack` copies the manifest through as-is rather than populating it from the running server. Nothing verified the two agreed, and they had already drifted once: a tool was registered but missing from the manifest, so anything reading the manifest rather than calling tools/list advertised an incomplete list. Adds tests that fail when the committed array diverges from what the server registers, covering the three ways they can disagree: a registered tool missing from the manifest, a manifest entry for a tool that no longer exists, and a description edited in only one place. On failure the last test prints the exact array to paste back into manifest.json. Deliberately a test rather than a build-time generator. Generating the array during `task build` would put it on the release path, where a bug in the generator ships silently; a test fails in CI on the PR that causes the drift. Keeping the array committed also means clients that read the manifest get the full list without having to generate one. Extracts the in-memory client setup from annotations.test.ts into tests/helpers/client.ts, now shared by both suites. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
CLAUDE.md already required keeping manifest.json's tools array in sync; point at the test that now checks it, so a failure is recognised as that rule rather than an unrelated breakage. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
1 participant
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.
What
Adds tests that fail when the
toolsarray inmanifest.jsondrifts from the tools the server actually registers.Why
The
toolsarray is maintained by hand.mcpb packcopies the manifest through as-is rather than populating it from the running server, so the committed array is what anything reading the manifest (instead of callingtools/list) advertises.Nothing verified the two agreed, and they had already drifted: a tool was registered at runtime but absent from the manifest. That was corrected by hand in #100, but the array is still hand-maintained, so the next tool added can reintroduce the same gap.
Coverage
Three ways the two can disagree, one test each:
Plus a whole-list equality check whose failure output prints the exact array to paste into
manifest.json.Verified the tests actually fail, not just pass — reproduced both real-world failure modes against a modified manifest:
Design note
This is a test rather than a build-time generator, deliberately. Generating the array during
task buildwould put it on the release path, where a generator bug ships silently — a test fails in CI on the PR that introduces the drift instead. Keeping the array committed also means clients that read the manifest get the full list rather than having to generate one.Also
Extracts the in-memory client setup duplicated from
annotations.test.tsintotests/helpers/client.ts, now shared by both suites. No behaviour change there.Testing
task check(format/types/lint) andtask testpass — 191 passed, 1 pre-existing skip (+4 new tests). No production code touched.🤖 Generated with Claude Code