Replace indent-fragile marketplace version regex with real JSON parser - #157
Conversation
Codex P2 on #152 flagged that my previous marketplace.json check anchored on exactly '^ "version"' (six spaces), which would silently stop working if the file's indentation ever changed (tab reformat, 2/4-space flip, minification, etc.). Switched both read_json_version_field() and read_marketplace_plugin_entry_version() to use the in-repo tools/cli/json_parser.hpp that the package subsystem already ships. Now the check walks the actual JSON object tree: - top-level .version for plugin.json and marketplace.json - plugins[0].version for marketplace.json per-plugin entry Indentation/formatting can't re-introduce drift. While verifying locally, the new parser caught real drift already on main: marketplace.json plugins[0].version was at 0.4.0 while plugin.json is at 0.5.0. Bumped plugins[0].version to 0.5.0 in the same commit since the check now (correctly) refuses to pass without it. That's a fresh drift introduced by one of the intermediate PRs that bumped plugin.json without updating the nested plugin entry — exactly the class of drift this PR is hardening against. Skill-Update: skip skill=cli-maintenance reason="Internal swap from regex to JSON parser; the cli-maintenance SKILL.md's 'pulp version check' section already documents the multi-version-field gotcha at the policy level — what check lives there. Implementation detail update only."
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c57e7f572b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
| pulp::cli::pkg::JsonParser parser{content}; | ||
| parser.skip_ws(); | ||
| auto root = parser.parse_value(); |
There was a problem hiding this comment.
Switching to JsonParser here introduces a hang path that the old regex did not have: if plugin.json or marketplace.json is temporarily malformed (for example, a trailing comma while editing), parse_value() can enter a non-advancing loop in parse_object/parse_array and never return, so pulp version check stalls instead of reporting a clear error. Because this check runs in version-gate workflows, a simple syntax mistake can block CI/hooks rather than failing fast; wrap parsing with an error/timeout-safe path (or fall back to a non-blocking parse failure).
Useful? React with 👍 / 👎.
1 participant
Why
Codex P2 on #152 flagged that `read_marketplace_plugin_entry_version` anchored on exactly `^ "version"` (six leading spaces). Any reformatting of `.claude-plugin/marketplace.json` — tab reformat, 2/4-space flip, minification — would silently skip the check and let drift back in.
What
Switched both `read_json_version_field()` and `read_marketplace_plugin_entry_version()` in `tools/cli/cmd_version.cpp` to use the in-repo `tools/cli/json_parser.hpp` (same parser the package subsystem uses). The check now walks the actual JSON object tree:
Indentation/formatting can't re-introduce drift.
Bonus: fresh drift found
While verifying locally, the new parser caught real drift already on main: `marketplace.json` `plugins[0].version` was at 0.4.0 while `plugin.json` is at 0.5.0. One of the intermediate PRs bumped `plugin.json` without updating the nested plugin entry — exactly the class of drift this PR hardens against. Bumped to 0.5.0 in the same commit since the check now (correctly) refuses to pass without it.
Test plan