Versioning & skill-sync gates + pulp pr + auto-release on merge - #144
Merged
Conversation
Core of the versioning-v2 plan (Components A + B): - tools/scripts/versioning.json — repo-agnostic config describing surfaces (sdk, plugin), their version files, trigger paths, public- API paths, internal-only paths, and generated-file globs. Uses JSON rather than YAML so there's no PyYAML dependency on PEP-668 Python (same schema, same semantics). - tools/scripts/versioning.schema.json — formal JSON schema for the config. - tools/scripts/skill_path_map.json — one entry per skill under .agents/skills/, mapping glob patterns to skill names. Covers all 13 current skills (aax, android, ci, cli-maintenance, cmajor-external, engine, faust, import-design, jsfx-subset, packages, ship, threejs-bridge, webview-ui). The map routes shipyard-config and CI-workflow changes to the ci skill. - tools/scripts/skill_sync_check.py — hard-fails when a diff touches mapped paths without updating the corresponding SKILL.md, unless a 'Skill-Update: skip skill=<name> reason="..."' trailer is present on the tip commit. Self-check fails if a skill dir lacks a map entry. - tools/scripts/version_bump_check.py — detects which surfaces need a bump, using conservative heuristics (public-header change = minor, internal-only = patch-suggested, 'BREAKING:' subject = major). Three modes: report (CI), apply (rewrites version files in place for pulp pr), hint (agent-hook advisory). Trailer override 'Version-Bump: <surface>=<level>' is authoritative. Both scripts are config-driven via --config so the same code will port to the Shipyard repo once the pulp loop lands.
Checkpoints deliverables 7, 8, and partial 11: - .githooks/pre-push — advisory-by-default runner for both gates. PULP_ENFORCE_PREPUSH=1 upgrades warnings to hard failures (CI sets this); PULP_SKIP_PREPUSH=1 is the single-push escape valve. - tools/scripts/install-githooks.sh — idempotent core.hooksPath installer; setup.sh will call this in a follow-up. - .github/workflows/version-skill-check.yml — authoritative PR gate. Pulls full history so origin/<base_ref> is reachable, runs both scripts in report mode, fails hard on any issue. No bypass other than the commit trailers; audit trail lives in git. - docs/guides/versioning.md — end-to-end guide covering the three layers, the trailer syntax, 'pulp pr' one-command flow, Shipyard-pin-vs-Shipyard-config split, and agent-parity story. Skill-Update: skip skill=ci reason="CI-skill touched (shipyard-config paths added to skill_path_map in prior commit), but the change was the map entry itself; no new gotcha to record yet."
Wires the last four layer-spanning pieces of the plan: - .shipyard/config.toml gains a validation.gates pipeline that runs both scripts with PULP_ENFORCE_PREPUSH=1 via the canonical 'setup' stage. 'shipyard run --pipeline gates' invokes them standalone. - .github/workflows/auto-release.yml (Component J): on push to main, diffs CMakeLists.txt project version + .claude-plugin/plugin.json version against the previous push range. If either moved, creates the matching v<x.y.z> or plugin-v<x.y.z> tag via bot token; the existing tag-triggered release-cli.yml / sign-and-release.yml then build and publish. Safety: concurrency group, idempotent-strict tagging (fails loudly on mismatched existing tags), Release: skip trailer suppression, revert detection. - hooks/scripts/cli-plugin-sync.sh: calls both scripts in --mode=hint after the existing CLI-sync reminders. Advisory only — Layer 1. - CLAUDE.md: new 'Versioning & Skill-Sync Policy' section under Skill Maintenance Rule. Documents the three layers, the 'push a PR' natural-language routing through pulp pr, and the three bypass trailers. Codex inherits via AGENTS.md -> CLAUDE.md pointer. Skill-Update: skip skill=ci reason="auto-release workflow is a new artifact; the ci skill doesn't yet have a gotcha about tag-triggered release flows — a note will land once the loop runs end-to-end."
Ran tools/scripts/version_bump_check.py --mode=apply on versioning-v2: - CMakeLists.txt: Pulp SDK 0.3.0 → 0.4.0 - .claude-plugin/plugin.json: 0.2.0 → 0.3.0 - .claude-plugin/marketplace.json: pulp plugin entry bumped to 0.3.0 - CHANGELOG.md: inherits both bump entries under [Unreleased] Verified afterwards: - version_bump_check.py --mode=report on HEAD is clean - skill_path_map.json and versioning.json are present and load cleanly Skill-Update: skip skill=android reason="version bump only; no code under mapped android paths changed in this commit — script flagged accumulated branch diff, not this commit's contents" Skill-Update: skip skill=ci reason="version bump only; workflows untouched in this commit" Skill-Update: skip skill=cli-maintenance reason="version bump only; no CLI source changed in this commit — the create.md / cmd_create.cpp diff is from earlier commits on this branch" Skill-Update: skip skill=packages reason="CHANGELOG/DEPENDENCIES/NOTICE edits in this commit are driven by the auto-bump, not by a dependency inventory change"
- .agents/skills/ci/SKILL.md: new top section explicitly maps the natural-
language triggers ('ship this', 'we're done', 'push a PR', ...) to
running 'pulp pr' rather than 'gh pr create' + 'shipyard ship'
separately. Preserves the raw shipyard commands below as diagnostic
fallback so existing workflows don't break.
- .claude/commands/pr.md: slash command invokes 'pulp pr' with
user-supplied $ARGUMENTS; documents the 6-step pipeline inline.
Skill-Update: skip skill=android reason="no android paths touched in this commit"
Skill-Update: skip skill=cli-maintenance reason="slash command + skill doc only; no CLI source in this commit"
Skill-Update: skip skill=packages reason="no dependency inventory change in this commit"
Finishes deliverable 12 and half of 5: - cmd_pr.cpp (new): one-shot PR orchestrator wrapping skill_sync_check -> version_bump_check --mode=apply -> commit -> push -> gh pr create -> shipyard ship. Refuses to run on main; refuses if worktree isn't clean after the bump. Flags: --base, --title, --no-ship, --no-push, --dry-run. Wired into pulp_cli.cpp command table, cli_common.hpp, and the CMakeLists source list. - cmd_version.cpp: version_check() now also validates .claude-plugin/plugin.json and .claude-plugin/marketplace.json (versions must be valid semver and match each other). New 'pulp version check --with-bump-check' flag pipes into tools/scripts/version_bump_check.py --mode=report. - .claude/commands/pr.md slash command was already in place from a prior iteration and routes through 'pulp pr'. - .agents/skills/ci/SKILL.md already instructs agents to route natural-language triggers through 'pulp pr'. Verified: pulp pr --dry-run prints the 6-step plan correctly. pulp version check --with-bump-check reports the known drift (SDK=0.4.0 bumped, plugin=0.3.0, marketplace=1.0.0 mismatch) including the version_bump_check verdict. Skill-Update: skip skill=cli-maintenance reason="pulp pr is the orchestrator the existing ci skill already documents; no new CLI-authoring gotcha to record beyond the flags covered in the source comments and the slash command md."
Deliverable 6 (compliance fix): - .claude-plugin/marketplace.json plugins[0].version: 0.2.0 -> 0.3.0 to match plugin.json (which itself was bumped from 0.2.0 -> 0.3.0 earlier on this branch by the version_bump_check auto-apply). - cmd_version.cpp: read_json_version_field() now anchors on the top-level 'version' field (two-space indent or less). The prior regex matched the first occurrence, which picked up marketplace.json's metadata.version (the marketplace FORMAT version) instead of the canonical top-level one. 'pulp version check' now correctly confirms plugin.json and marketplace.json agree on 0.3.0. - .agents/skills/ci/SKILL.md gains a 'Versioning & Skill-Sync gates' section documenting the three-layer enforcement, the path-map gotcha for ci, the auto-release workflow's diff semantics, and the idempotent-strict tag safety property. Dogfoods the skill-update rule this loop is building. Skill-Update: skip skill=android reason="accessibility_android.cpp changed on main before this branch was cut; that drift belongs to the PR that landed it, not versioning-v2." Skill-Update: skip skill=packages reason="DEPENDENCIES.md + NOTICE.md moved on main via the recent MPE and Streams PRs; not introduced by this branch."
… cli-maintenance skill
Two fixes:
- skill_sync_check.py: '.agents/skills' was being chopped to
'agents/skills' because the old code used str.lstrip('./') which
removes *characters* not a prefix, eating the leading dot. Replaced
with explicit './'-prefix stripping. Before the fix, the tool
reported every SKILL.md as NOT updated even when it was in the diff.
- cli-maintenance/SKILL.md: documents 'pulp pr' (the one-shot PR
orchestrator) and 'pulp version check --with-bump-check', including
invariants (refuses on main, refuses dirty worktree post-bump),
anti-patterns (don't call the Python scripts by hand; don't split
gh + shipyard), and the JSON-multiple-version-field gotcha that the
version-check regex anchors around.
With both fixes: skill-sync reports clean (ci and cli-maintenance
now ✓ updated; android and packages bypassed with reasons).
Skill-Update: skip skill=android reason="accessibility_android.cpp drift is from commit on main before this branch was cut — not versioning-v2 scope."
Skill-Update: skip skill=packages reason="DEPENDENCIES.md/NOTICE.md drift is from MPE/Streams PRs on main — not versioning-v2 scope."
Adds tools/scripts/test_gates.py — self-contained unittest harness (no extra deps) that spins up throwaway git repos with a minimal versioning.json + skill_path_map.json and exercises 11 scenarios against version_bump_check.py and skill_sync_check.py: - new public header -> minor-required (bump_check) - comments-only edit -> none (bump_check) - whitespace-only edit -> none (bump_check) - BREAKING: trailer -> major-required (bump_check) - test-only change -> none (bump_check) - generated-file change -> skipped (none) (bump_check) - Version-Bump: skip trailer -> none (wins over feat: ceiling) (bump_check) - revert commit -> none (bump_check) - skill path, no SKILL.md -> fail (skill_check) - skill path + SKILL.md -> pass (skill_check) - skill path + bypass trailer -> pass (skill_check) Writing the fixtures exposed three real bugs; fixed them: 1. heuristic_for_surface: whitespace/comment-only edits on public paths were falling through to the internal-path fallback and returning 'patch' instead of 'none'. Rewrote so paths with no meaningful diff (per git diff --ignore-all-space + a C/C++/Python comment filter) collapse to 'none' for the whole surface. 2. Skip-override interaction: Version-Bump: <surface>=skip was being detected but then raised back up by the conventional-commit ceiling loop (e.g. a feat: subject). Made skip authoritative: it never gets re-raised. 3. Report rendering: patch-suggested verdicts were printing '✗ bump required' even though render_report correctly treated them as warnings (exit 0). Rephrased patch-level unbumped output as '? bump suggested (patch)' so the text matches the exit code. Wired test_gates.py into .github/workflows/version-skill-check.yml as a new step after the gate-script runs so any regression hits CI. setup.sh now calls tools/scripts/install-githooks.sh during bootstrap (before external-SDK setup) so a fresh clone lands with core.hooksPath=.githooks by default. Idempotent — safe to rerun. Skill-Update: skip skill=ci reason="Adds a new CI workflow step that runs the fixture tests. The ci SKILL.md's 'Versioning & Skill-Sync gates' section already documents the workflow surface area; no new gotcha until the fixtures catch a regression." Skill-Update: skip skill=android reason="pre-existing drift, not versioning-v2 scope" Skill-Update: skip skill=packages reason="pre-existing drift, not versioning-v2 scope"
![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: dad4b29914
ℹ️ 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".
| lines.append(f"[{v.surface.name}] {v.surface.label}: no bump needed") | ||
| continue | ||
| # Already bumped since base? | ||
| bumped = any(already_bumped(base, vf, repo) for vf in v.surface.version_files) |
There was a problem hiding this comment.
The gate currently marks a surface as bumped when any configured version file changed, so surfaces with multiple files (like plugin.json + marketplace.json) can pass even if one file is still stale. This also affects --mode=apply, where the same any(...) check skips writing missing companion files once one file was bumped manually, allowing inconsistent version metadata to merge.
Useful? React with 👍 / 👎.
Comment on lines
+475
to
+479
| for subject, body in git_log_subjects_and_bodies(base, head): | ||
| if is_revert_commit(subject, {}): | ||
| continue | ||
| conv_ceiling = max_level(conv_ceiling, classify_conventional(subject)) | ||
| if LEVELS.index(conv_ceiling) > LEVELS.index(final): |
There was a problem hiding this comment.
The conventional-commit ceiling is computed from every commit subject in base..head without checking whether each commit touched the current surface, so a feat: on one surface can incorrectly escalate another touched surface from patch to minor/major. In mixed branches this creates false bump requirements and unnecessary version jumps that contradict the documented per-surface behavior.
Useful? React with 👍 / 👎.
Comment on lines
+228
to
+229
| skill_md_prefix = f"{rel_skills_dir}/{skill}/" | ||
| skill_md_modified = any(p.startswith(skill_md_prefix) for p in changed) |
There was a problem hiding this comment.
The skill-sync gate treats any file change under .agents/skills/<skill>/ as if SKILL.md was updated, so modifying a side file (for example notes/assets/references) satisfies the check even when SKILL.md itself is unchanged. This weakens the enforcement rule described by the script and docs, and allows accidental or intentional bypasses.
Useful? React with 👍 / 👎.
Comment on lines
+133
to
+135
| remote_sha=$(git ls-remote --tags origin "$tag" | awk '{print $1}') | ||
| local_sha=$(git rev-parse HEAD) | ||
| if [ "$remote_sha" = "$local_sha" ]; then |
There was a problem hiding this comment.
The idempotency check compares git ls-remote --tags ... output directly to git rev-parse HEAD, but for annotated tags ls-remote returns the tag object SHA (not the commit SHA). As a result, reruns where the tag already points at the current commit are reported as mismatches and fail instead of no-oping.
Useful? React with 👍 / 👎.
Walks up to the first .git entry instead of hardcoding ../.. from the script's directory. Works unchanged whether the script lives at tools/scripts/ (pulp) or scripts/ (Shipyard). Required for the Shipyard port to reuse the same file without per-repo tweaks.
danielraffel
added a commit
that referenced
this pull request
Apr 13, 2026
P1 — version_bump_check: require ALL version files to move per surface.
Previously 'any(already_bumped ...)' let a plugin surface with both
plugin.json + marketplace.json pass when only one was bumped, creating
split-brain versions. Now tagged '✗ partial bump — not moved: <files>'
and hard-fails. apply_bumps also changed to all()-semantics so a
partial prior state self-heals on re-run.
P2 — version_bump_check: conventional-commit ceiling is now scoped to
commits whose files intersect the surface's trigger_paths. A plugin-
only feat: can no longer raise the SDK ceiling.
P2 — skill_sync_check: skill-md-update detection now requires SKILL.md
specifically (exact match or nested */SKILL.md). Side files like
notes, fixtures, or tool logs under the skill dir no longer satisfy
the gate.
P2 — auto-release.yml: dereference annotated tag object SHAs via
<tag>^{} before comparing to HEAD. The previous direct ls-remote
output returned the tag-object SHA for annotated tags (the kind this
workflow creates via git tag -a), which never matches rev-parse HEAD,
breaking the idempotent-strict check. Falls back to the undereferenced
SHA for lightweight tags.
Three new fixtures cover the P1 partial-bump regression, the P2
skill-side-file case, and the previously-existing scenarios still pass
— 13/13 green.
Skill-Update: skip skill=ci reason="Bug fixes to existing CI-surface infrastructure; the 'Versioning & Skill-Sync gates' section in .agents/skills/ci/SKILL.md already documents the workflow and behavior at the policy level. These are internal heuristic corrections, not new skill-worthy gotchas."
Skill-Update: skip skill=cli-maintenance reason="Pure script-internal fixes — no CLI-user-visible change, no new CLI invariant worth documenting."
2 tasks
danielraffel
added a commit
that referenced
this pull request
Apr 13, 2026
release-cli.yml failed on CLI windows-x64 and CLI windows-arm64 with:
cmd_version.cpp(11,10): error C1083: Cannot open include file:
'sys/wait.h': No such file or directory
cmd_pr.cpp(34,10): error C1083: Cannot open include file:
'sys/wait.h': No such file or directory
The code uses WIFEXITED / WEXITSTATUS macros to unpack std::system's
wait(2)-encoded return value on POSIX. MSVC's std::system returns the
child exit code directly (no encoding), so on Windows we shim these
macros to the identity and skip the POSIX-only include.
Discovered after PR #144 merged because build.yml runs Debug-only on
Windows (via 'cmake --build --parallel' without /m:... tuning that
would hit this) while release-cli.yml runs Release against all 5
platform/arch combos and caught the missing header.
Skill-Update: skip skill=cli-maintenance reason="Portable system()-status extraction is worth recording as a CLI-maintenance gotcha; follow-up commit will add a one-liner to SKILL.md. Skipping on this commit to keep the Windows fix minimal and revertable."
danielraffel
added a commit
that referenced
this pull request
Apr 13, 2026
* Fix Codex P1 + P2 review on versioning-v2 (PR #144) P1 — version_bump_check: require ALL version files to move per surface. Previously 'any(already_bumped ...)' let a plugin surface with both plugin.json + marketplace.json pass when only one was bumped, creating split-brain versions. Now tagged '✗ partial bump — not moved: <files>' and hard-fails. apply_bumps also changed to all()-semantics so a partial prior state self-heals on re-run. P2 — version_bump_check: conventional-commit ceiling is now scoped to commits whose files intersect the surface's trigger_paths. A plugin- only feat: can no longer raise the SDK ceiling. P2 — skill_sync_check: skill-md-update detection now requires SKILL.md specifically (exact match or nested */SKILL.md). Side files like notes, fixtures, or tool logs under the skill dir no longer satisfy the gate. P2 — auto-release.yml: dereference annotated tag object SHAs via <tag>^{} before comparing to HEAD. The previous direct ls-remote output returned the tag-object SHA for annotated tags (the kind this workflow creates via git tag -a), which never matches rev-parse HEAD, breaking the idempotent-strict check. Falls back to the undereferenced SHA for lightweight tags. Three new fixtures cover the P1 partial-bump regression, the P2 skill-side-file case, and the previously-existing scenarios still pass — 13/13 green. Skill-Update: skip skill=ci reason="Bug fixes to existing CI-surface infrastructure; the 'Versioning & Skill-Sync gates' section in .agents/skills/ci/SKILL.md already documents the workflow and behavior at the policy level. These are internal heuristic corrections, not new skill-worthy gotchas." Skill-Update: skip skill=cli-maintenance reason="Pure script-internal fixes — no CLI-user-visible change, no new CLI invariant worth documenting." * scripts: range-scoped trailer detection + add mpe skill to map Two CI-surfaced issues from PR #147's first run: 1. Bypass trailers on the branch tip weren't being honored when the gate ran in CI. CI's actions/checkout produces a synthetic merge commit as HEAD for pull_request events, so 'git log -1 HEAD' saw the merge commit body (auto-generated, no trailers) instead of my tip-commit body. Fix: walk every commit in base..head and merge their trailers. Any commit in the range carrying a bypass now satisfies the gate. Applied to both skill_sync_check.py and version_bump_check.py. 2. 'mpe' skill landed on main via PR #142 after this branch's prior skill_path_map.json update. skill_sync self-check failed with 'skill directory mpe has no entry'. Added core/midi/*mpe*, core/format/*mpe*, examples/mpe-*/ mapping. Skill-Update: skip skill=ci reason="Range-trailer scan is the bug fix the CI skill already implicitly documents via the trailer-on-tip-commit language in docs/guides/versioning.md; no new operator-facing gotcha beyond 'trailers work on any commit in the PR range now'." Skill-Update: skip skill=cli-maintenance reason="Pure script-internal fixes — no CLI invariant changes." Skill-Update: skip skill=mpe reason="Adding the path-map entry only; MPE skill content on main is unchanged." * cli: make cmd_pr + cmd_version compile on Windows (sys/wait.h guard) release-cli.yml failed on CLI windows-x64 and CLI windows-arm64 with: cmd_version.cpp(11,10): error C1083: Cannot open include file: 'sys/wait.h': No such file or directory cmd_pr.cpp(34,10): error C1083: Cannot open include file: 'sys/wait.h': No such file or directory The code uses WIFEXITED / WEXITSTATUS macros to unpack std::system's wait(2)-encoded return value on POSIX. MSVC's std::system returns the child exit code directly (no encoding), so on Windows we shim these macros to the identity and skip the POSIX-only include. Discovered after PR #144 merged because build.yml runs Debug-only on Windows (via 'cmake --build --parallel' without /m:... tuning that would hit this) while release-cli.yml runs Release against all 5 platform/arch combos and caught the missing header. Skill-Update: skip skill=cli-maintenance reason="Portable system()-status extraction is worth recording as a CLI-maintenance gotcha; follow-up commit will add a one-liner to SKILL.md. Skipping on this commit to keep the Windows fix minimal and revertable." * Fix double-bump in apply_bumps: compute from base, not current (PR #147 Codex P1) When --mode=apply re-runs over a partially-applied multi-file bump (e.g. .claude-plugin/plugin.json already at 0.2.0 while marketplace.json is still 0.1.0), v.current_version reflects whichever file was read first. Bumping that value again produces 0.3.0 — a double bump. Fix: refactor the version-extraction logic into a shared _extract_version_from_text / version_at_base pair, then pick the source version for apply_bumps from the **base commit**, not HEAD. A partial-apply retry is now idempotent.
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.
Summary
Adds the three-layer versioning & skill-sync enforcement system. After this lands,
pulp pr(or saying "push a PR" / "ship this" / "we're done") is the single orchestrator for shipping a branch: gates run → version files auto-bump → commit → PR → Shipyard validate + merge → auto-release tag + binaries.What's in this PR
Scripts & config (zero-dep, config-driven for Shipyard port later):
Three enforcement layers (one source of truth):
The `pulp pr` command (Component I):
Auto-release (Component J):
`pulp version check` expanded:
Compliance fix:
Policy & docs:
Test plan
Out of scope (tracked as follow-ups)