fix(validate-plugin): report malformed plugin.json/marketplace.json as validation errors - #206
Merged
Merged
Conversation
…s validation errors A JSON syntax error in .claude-plugin/plugin.json or .claude-plugin/marketplace.json threw an unhandled SyntaxError and killed the gate with a stack trace instead of flowing through errors[] like the sibling .mcp.json read already does. Wrap both reads in the same try/catch pattern so every manifest parse failure lands in the VALIDATION FAILED report, aggregated with any other findings. Adds a VALIDATE_PLUGIN_ROOT test hook so the regression tests can point the validator at fixture roots with deliberately malformed manifests. Closes #182
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes scripts/validate-plugin.mjs so malformed JSON in .claude-plugin/plugin.json or .claude-plugin/marketplace.json is reported as a structured validation error (aggregated in errors[]) instead of crashing with an unhandled SyntaxError, and adds regression tests to lock that behavior in.
Changes:
- Wrap
readJson()calls forplugin.jsonandmarketplace.jsonintry/catchand append parse failures toerrors[]. - Add a
VALIDATE_PLUGIN_ROOTenvironment-variable hook to allow running the validator against a fixture root (for tests). - Add subprocess-based regression tests covering single and aggregated manifest parse failures.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| scripts/validate-plugin.mjs | Adds guarded JSON parsing for manifests and introduces the VALIDATE_PLUGIN_ROOT hook used by tests. |
| tests/validate-plugin-manifest-parse.test.mjs | Adds regression coverage ensuring malformed manifests are reported via VALIDATION FAILED: without stack traces and with aggregation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The test hook read the override with `??`, which only falls back on null/undefined. An exported-but-empty VALIDATE_PLUGIN_ROOT therefore set ROOT to "", making every manifest path cwd-relative: the gate validated whatever happened to be in the working directory instead of this plugin. The failure is cwd-dependent and so intermittent — run from the repo root it passes by accident, run from anywhere else it reports a spurious ".claude-plugin/plugin.json: missing". Read the override with `||` instead, matching how every other env override in this codebase is read (strOrNull in lib/provenance-ledger.mjs, the CLAUDE_CONFIG_DIR guard in lib/provenance-config.mjs, and process.env.PATH in lib/doctor.mjs). Adds a regression test that spawns the validator from a temp cwd with the override set to "" and asserts it still resolves this repository. The temp cwd is load-bearing: with cwd at the repo root the test passes either way.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
Suppressed comments (1)
tests/validate-plugin-manifest-parse.test.mjs:56
- This assertion couples the test to the repository's current plugin name ("mif-docs"), which makes the test brittle if the manifest name ever changes. The test goal (empty VALIDATE_PLUGIN_ROOT falls back to repo root, not temp cwd) is already demonstrated by a successful exit status; if you want an additional stdout assertion, prefer something invariant like the reported error count.
assert.match(r.stdout, /plugin: mif-docs/);
Merged
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.
Summary
A JSON syntax error in
.claude-plugin/plugin.jsonor.claude-plugin/marketplace.jsonthrew an unhandledSyntaxErrorand terminatedscripts/validate-plugin.mjswith a Node stack trace, instead of pushing a structured entry ontoerrors[]and reporting it through the gate's normalVALIDATION FAILED:path. The.mcp.jsonread in section 3 already guards itsreadJson()call; this extends the identicaltry/catchpattern to the other two manifest reads, so every manifest parse failure is reported alongside any other findings in the same run.Changes
scripts/validate-plugin.mjs: wrap theplugin.json(section 1) andmarketplace.json(section 2) reads intry/catch, appending the parse message toerrors[]— matching section 3's existing pattern.scripts/validate-plugin.mjs: add aVALIDATE_PLUGIN_ROOTenv test hook so the test suite can point the validator at a fixture plugin root instead of this repository itself.tests/validate-plugin-manifest-parse.test.mjs: regression tests — a valid fixture root passes; a malformedplugin.jsonand a malformedmarketplace.jsoneach produce a structuredVALIDATION FAILED:entry (exit 1, no stack trace); and both parse failures aggregate in one run instead of the first aborting the script. All four fail against the pre-fix script and pass with it.Verification
npm run test:hook— 273/273 pass (afternpm run hydrate-schema, as CI does)npm run validate-plugin— OK, 0 errorsnpm run lint:md— 0 issuesCloses #182