Skip to content

version: fix marketplace.json plugins[0].version drift + guard against re-drift - #152

Merged
danielraffel merged 2 commits into
mainfrom
feature/marketplace-plugin-version
Apr 14, 2026
Merged

version: fix marketplace.json plugins[0].version drift + guard against re-drift#152
danielraffel merged 2 commits into
mainfrom
feature/marketplace-plugin-version

Conversation

@danielraffel

Copy link
Copy Markdown
Collaborator

Fix two-line drift on .claude-plugin/marketplace.json: plugins[0].version was at 0.3.0 while plugin.json and the top-level marketplace version were at 0.4.0. The per-plugin entry is what Claude Code's marketplace surfaces per plugin, so it has to match plugin.json.

The drift slipped in because pulp version check only validated the top-level .version field — the nested plugins[0].version was invisible to the gate. Added read_marketplace_plugin_entry_version() + a second equality check so this can't re-drift silently.

Test plan

  • pulp version check now verifies both fields.
  • CI: version-skill-check + build matrix.

… re-drift

plugins[0].version in .claude-plugin/marketplace.json was at 0.3.0
while plugin.json and the top-level marketplace version are at 0.4.0.
The per-plugin entry is what Claude Code's marketplace listing
surfaces, so it has to stay in lockstep with plugin.json's top-level
version.

The drift slipped in because 'pulp version check' only validated the
top-level marketplace.json '.version' field — the nested
plugins[0].version was invisible to the gate. Adding
read_marketplace_plugin_entry_version() + a second equality check so
this can't re-drift silently.

Skill-Update: skip skill=cli-maintenance reason="One-line field add and one helper function — cli-maintenance/SKILL.md's existing 'pulp version check' section covers the multi-JSON-version-field gotcha generically; this commit just applies it to one more field of the same file."

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 96087351d2

ℹ️ 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".

Comment thread tools/cli/cmd_version.cpp
// indentation (inside `plugins[0]`). If the file grows a second plugin
// entry, this still picks the first one — matching how scroll /
// marketplace listings render.
std::regex re(R"#(^ "version"\s*:\s*"([^"]+)")#", std::regex::multiline);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Parse plugin version without fixed indentation

The new guard relies on ^ "version" (exactly six leading spaces), so any reformatting of .claude-plugin/marketplace.json (e.g., different indent width, tabs, or minified JSON) makes read_marketplace_plugin_entry_version() return empty and silently skip the mismatch check. In that scenario, pulp version check reports success even if plugins[0].version drifts again, which defeats the regression protection this commit is adding.

Useful? React with 👍 / 👎.

Codex P1 follow-ups on PR #149 — the bootstrap scenario where a repo
genuinely has no secrets yet was exactly the case the new nudges were
meant to catch, and the original implementations silently skipped it:

- cli_common.cpp: the RELEASE_BOT_TOKEN doctor row used 'gh api ...
  --jq .secrets[].name' and gated on !secrets_list.empty(). Zero-secret
  repos produce empty stdout, so the row was omitted — identical to
  the 'gh errored' case. Now probes the raw JSON response (no --jq)
  and treats an empty response as 'gh errored' while a non-empty JSON
  body with no RELEASE_BOT_TOKEN match is the missing-secret signal
  we wanted to surface.
- cmd_pr.cpp: same bug, same fix. The '!secrets.empty() &&
  find == npos' guard suppressed the warning in the exact scenario
  users most need it.

Also adds 'gh api --paginate' so repos with more than the default 30
secrets don't false-miss RELEASE_BOT_TOKEN on page 2+.

Skill-Update: skip skill=cli-maintenance reason="Pure internal fixes to the just-shipped doctor + pulp pr flow; no CLI-user-visible interface change, and the existing 'pulp doctor' / 'pulp pr' / 'pulp version check' sections in cli-maintenance/SKILL.md still describe the right UX."
@danielraffel
danielraffel merged commit 7a2a1bd into main Apr 14, 2026
8 of 10 checks passed
@danielraffel
danielraffel deleted the feature/marketplace-plugin-version branch April 14, 2026 01:47
danielraffel added a commit that referenced this pull request Apr 14, 2026
AudioUnitSDK 1.4's own source files (external/AudioUnitSDK/src/*.cpp)
#include <expected>, a C++23 header. Under CMake 3.24's policy, the
root project's CMAKE_CXX_STANDARD=20 is authoritative on every target
regardless of target_compile_features: the existing
'target_compile_features(ausdk PUBLIC cxx_std_23)' declares only a
minimum feature requirement, which is silently satisfied by C++20
without upgrading the standard.

Add 'set_target_properties(ausdk PROPERTIES CXX_STANDARD 23
CXX_STANDARD_REQUIRED ON)' so the ausdk static library's own
translation units compile at C++23 and the std::expected includes
resolve. The PUBLIC cxx_std_23 compile feature stays so downstream
consumers (pulp-format) continue to pick up the minimum requirement
via interface — pulp-format already has the same CXX_STANDARD 23
property pinned at core/format/CMakeLists.txt:83, so this just
plugs the gap between the ausdk target and its own sources.

Affected before this fix: every PR cut after ~2026-04-13 14:00 UTC
failed its macOS (ARM64) github-hosted leg with:
  'error: no template named unexpected in namespace std'
  'error: no template named expected in namespace std'
repeatedly across AUUtility.h and AUBuffer.h until the compiler hit
its error limit. Confirmed affecting PRs #150, #151, the SignalGraph
Phase 0 branch, and #152 (admin-merged) among others.

Closes #155.
danielraffel added a commit that referenced this pull request Apr 14, 2026
…) (#156)

* Fix macOS build break: pin ausdk target to CXX_STANDARD=23 (closes #155)

AudioUnitSDK 1.4's own source files (external/AudioUnitSDK/src/*.cpp)
#include <expected>, a C++23 header. Under CMake 3.24's policy, the
root project's CMAKE_CXX_STANDARD=20 is authoritative on every target
regardless of target_compile_features: the existing
'target_compile_features(ausdk PUBLIC cxx_std_23)' declares only a
minimum feature requirement, which is silently satisfied by C++20
without upgrading the standard.

Add 'set_target_properties(ausdk PROPERTIES CXX_STANDARD 23
CXX_STANDARD_REQUIRED ON)' so the ausdk static library's own
translation units compile at C++23 and the std::expected includes
resolve. The PUBLIC cxx_std_23 compile feature stays so downstream
consumers (pulp-format) continue to pick up the minimum requirement
via interface — pulp-format already has the same CXX_STANDARD 23
property pinned at core/format/CMakeLists.txt:83, so this just
plugs the gap between the ausdk target and its own sources.

Affected before this fix: every PR cut after ~2026-04-13 14:00 UTC
failed its macOS (ARM64) github-hosted leg with:
  'error: no template named unexpected in namespace std'
  'error: no template named expected in namespace std'
repeatedly across AUUtility.h and AUBuffer.h until the compiler hit
its error limit. Confirmed affecting PRs #150, #151, the SignalGraph
Phase 0 branch, and #152 (admin-merged) among others.

Closes #155.

* Pin AudioUnitSDK to 1.3.0 (last C++17-friendly tag)

The CXX_STANDARD=23 pin in the previous commit was necessary but not
sufficient: the ausdk target now compiles at -std=c++23 as intended,
but AppleClang/libc++ on the GitHub-hosted macOS-14 runner doesn't
yet expose std::expected even at C++23. Confirmed on the CI retry —
same errors even with CXX_STANDARD=23 pinned.

AudioUnitSDK-1.4.0 is the release that introduced <expected>
references in its headers (verified via GitHub API: 1.3.0 has 0
std::expected hits in AUUtility.h, 1.4.0 has 4). 1.3.0 compiles
cleanly on C++17+ and is the last tag that works on current macOS
runner libc++.

Changes:
- setup.sh: pin AU_SDK_REF to AudioUnitSDK-1.3.0 (was 1.4.0).
- .github/workflows/sign-and-release.yml: clone --branch
  AudioUnitSDK-1.3.0 explicitly (was unpinned default branch, which
  now resolves to 1.4.x).

The CXX_STANDARD=23 ausdk pin from the prior commit is kept — it's
harmless at 1.3.0 (the code doesn't use C++23) and gives us a
one-flip-back path once AppleClang/libc++ ship std::expected, or
when a new AudioUnitSDK release works around it.

Closes #155 (this time for real).

* Skill bypass for #155 fix

Skill-Update: skip skill=ci reason="Pinning AudioUnitSDK to 1.3.0 via sign-and-release.yml is a dependency-pin change for an upstream incompatibility; no new CI-authoring gotcha beyond what the existing 'Dependency Update Workflow' and #155 capture. The ci skill's 'Versioning & Skill-Sync gates' section is still accurate."
Skill-Update: skip skill=ship reason="Same dep pin; ship's SKILL.md already documents sign-and-release.yml as a tag-triggered workflow and doesn't need to know the upstream AU SDK tag."

* au_v2_instrument: use GetParameter for 1.3.0 compat

AudioUnitSDK 1.4 renamed the RT-safe parameter read to GetParameterRT;
1.3.0 uses GetParameter and is equivalent (inline atomic float load).
Switched the call site to GetParameter so the AU adapter builds
against the 1.3.0 pin from the previous commit. Flip back to
GetParameterRT when we can adopt 1.4+ (requires AppleClang/libc++
with std::expected on the GitHub-hosted macOS runner).

Skill-Update: skip skill=cli-maintenance reason="AU adapter source change, not CLI-surface. cli-maintenance SKILL.md isn't affected."
danielraffel added a commit that referenced this pull request Apr 14, 2026
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."
danielraffel added a commit that referenced this pull request Apr 14, 2026
…#157)

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."
danielraffel added a commit that referenced this pull request Apr 22, 2026
* chore: bump Shipyard pin v0.22.9 → v0.25.0

Pulls in two semantic changes that directly affect pulp's ship flow:

1. v0.24.0 (Shipyard #151) — `shipyard pr` now walks past the
   mechanical "chore: bump versions" commit when composing the
   auto-PR title and body, using the feature commit's subject/body
   instead. Pulp PR #624 was the canonical repro: its title read
   "chore: bump versions" and the body was "Automated by
   `shipyard pr`." — both are gone now. The tool-branding text is
   also scrubbed, so shipped PRs read as first-party.

2. v0.25.0 (Shipyard #152) — `Version-Bump: <surface>=<level>`
   trailers are now authoritative rather than ceiling-raising.
   Previously an author-declared `=patch` could be silently raised
   to `=minor` by the conventional-commit heuristic, defeating the
   point of the trailer. This commit also syncs pulp's bundled
   `tools/scripts/version_bump_check.py` with that behaviour so the
   hook-time, CI-time, and shipyard-time gates all agree.

Third improvement, automatic (no pulp-side code): v0.25.0 also
serves `shipyard ship-state list` from the running daemon via IPC
when one is available (Shipyard #154), bypassing the ~5-6s
PyInstaller cold-start on every call. The macOS GUI polls this
every 7s; `pulp pr` also hits it during preflight.

Verified: new binary installed at ~/.local/bin/shipyard, responds
`shipyard, version 0.25.0`. `version_bump_check.py --mode=report`
and `skill_sync_check.py --mode=report` both clean against
origin/main on this branch.

Version-Bump: sdk=skip reason="shipyard pin + bundled script sync — no SDK API surface moved"
Version-Bump: plugin=skip reason="shipyard pin + bundled script sync — no plugin surface moved"

* docs(ci-skill): note v0.24.0/v0.25.0 behavior changes at new pin

Skill-sync maps tools/shipyard.toml to the `ci` skill. The pin bump
in 3fb011c surfaced three new behaviours agents need to know about:

1. Auto-PR title/body now walks past the bump commit (v0.24.0).
2. Version-Bump trailer is authoritative (v0.25.0).
3. ship-state list goes through the daemon over IPC (v0.25.0).

Updated the ci SKILL.md with a new "Behaviour notes at the current
pin" subsection, and corrected the stale `v0.21.0` pin reference.
danielraffel added a commit that referenced this pull request Apr 24, 2026
Closes one of the `Ported-partial` gaps called out at the Phase 6d
post-mortem. The Rust port now matches `tools/cli/cmd_version.cpp`'s
three subcommands 1:1.

Ported:
- `version bump <major|minor|patch> [--plugin]` — rewrites the first
  `project(... VERSION X.Y.Z ...)` occurrence in `CMakeLists.txt`, or
  (with `--plugin`) the `pulp_add_plugin(... VERSION "X.Y.Z" ...)`
  cell. SDK-side bumps also insert a `## [X.Y.Z]` heading above the
  latest CHANGELOG entry and print the "rebuild + git tag" footer.
- `version check [--with-bump-check]` — ported byte-for-byte against
  the C++ six-step walk: SDK consistency vs CLI version, AU
  Info.plist hardcoded-integer guard, CHANGELOG heading match,
  plugin.json semver shape, plugin.json ↔ marketplace.json drift, and
  marketplace.json `plugins[0].version` (the nested cell that drifted
  silently in #152). `--with-bump-check` shells to
  `tools/scripts/version_bump_check.py --mode=report` through the
  `Spawner` trait so tests stay deterministic.

Tests ship with the fix: 13 new unit tests covering parse variants,
SemVer arithmetic, CMake rewrites (project + plugin forms),
CHANGELOG heading insert ordering, happy-path check, mismatch
detection, and the bump-check delegate exit-code propagation.

`ENV_LOCK`-guarded tests set `PULP_RS_CLI_VERSION` to a pinned value
so parallel test runs don't race on the CLI-version env probe.

Interim semantics note: until Phase 8 swaps the Rust binary in as the
canonical SDK CLI, `check`'s "SDK version" reads the Rust binary
version (`CARGO_PKG_VERSION` or the `PULP_RS_CLI_VERSION` override).
All other drift checks are fully ported and catch the same bugs the
C++ side does.

`UPSTREAM_SYNC.md` classification-matrix update lands in the
Phase-6e summary commit at the end of the series, not per-slice.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant