fix: derive npm publish set instead of hardcoding package list - #308
Merged
Conversation
`.github/workflows/publish-npm.yml` carried its own hardcoded list of 13 package directories while `scripts/release.mjs` DERIVED the same set (every non-private package at the CLI's version). Two answers to one question, and they had drifted: `@metaobjectsdev/docs-site` is a runtime `dependencies` entry of `@metaobjectsdev/cli` and was not in the workflow's list, so a release cut through the workflow would have published a `cli` pinning a `docs-site` version nobody published — `npm i @metaobjectsdev/cli` → ETARGET. Only an external install could have found it; the local release path publishes all 14, which is what kept it latent (docs-site@0.23.2 is on npm today). `scripts/publish-set.mjs` is now the single source of truth for which packages ship and in what order. Both publish paths read it and the workflow's list is gone, so it cannot drift from a derivation it no longer has. The derivation throws rather than returning a wrong set: a member with no declared tier, a set not closed over its own sibling runtime deps, or a tier order that would publish a dependency after its dependent. TIER_ORDER omitted `docs-site` too, and that was not harmless: `indexOf()` returns -1, which does not sort last — it sorts FIRST, so the local path published docs-site ahead of `metadata` and `render`, the two packages it depends on. The tier is declared now, and an undeclared one is an error. - scripts/publish-set.mjs — the shared derivation + its three invariants - scripts/test-publish-set.mjs — each invariant driven against a synthetic workspace, plus a regression pinning docs-site before cli in the real tree - scripts/ci-local.sh — new `publish-set parity` gate in the gates lane - docs/RELEASING.md, AGENTS.md, CHANGELOG.md — record the rule 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
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.
Intent
Fix .github/workflows/publish-npm.yml, which would publish an uninstallable @metaobjectsdev/cli.
THE BUG. The workflow publishes a hardcoded list of 13 package directories. server/typescript/packages/docs-site is not in that list, but @metaobjectsdev/docs-site (a) is not private so it rides the lockstep version like every other published package, and (b) is a runtime 'dependencies' entry of @metaobjectsdev/cli (workspace:*, rewritten to the concrete version at pack time). scripts/release.mjs derives its publish set dynamically (every non-private package whose version equals the CLI's), so the LOCAL release path publishes all 14 and docs-site@0.23.2 is on npm today. A release cut through the WORKFLOW instead would publish a cli whose docs-site dependency does not exist at that version - an uninstallable CLI (npm ETARGET), discoverable only by an external npm install. Two answers to one question that had drifted, and nothing had ever compared them.
WHAT WAS ASKED FOR, and the decisions made:
Make the workflow's publish set correct. The task PREFERRED replacing the hardcoded list with the same derivation scripts/release.mjs already uses so the two paths cannot drift again, over the fallback of adding docs-site to the list plus a divergence check. I took the preferred option: new scripts/publish-set.mjs is the single source of truth, both publish paths read it, and the workflow's list is DELETED rather than corrected - a list that no longer exists cannot drift from the derivation. release.mjs now imports publishSet() instead of enumerating and sorting itself.
Fix the tier ordering. TIER_ORDER omitted docs-site, so indexOf returned -1 and it sorted FIRST. Verified this is worse than the task described: it is not merely 'harmless because nothing but cli depends on it' - sorting first put docs-site ahead of metadata and render, the two packages it DOES depend on, so the local path was already violating the documented tier contract. docs-site now has a declared tier (after sdk, before runtime-web - matching docs/RELEASING.md's tier table and scripts/prerelease.mjs's TIERS, both of which were already correct), and an undeclared tier is now a hard error rather than an accidental position.
Prove the failure and the fix, not just reason about it. Packed the cli tarball and showed it pins @metaobjectsdev/docs-site@0.23.2; extracted the pre-change workflow's list and showed docs-site absent (13 vs the derived 14); showed the derived list after the change is identical to the derivation and closed over every packed-cli sibling dep; and showed the guard now throws on the old TIER_ORDER. Evidence goes in the PR body.
DELIBERATE DESIGN CHOICES a reviewer might otherwise question:
SCOPE CONSTRAINTS the user set, and how they were honored:
ALSO IN THIS COMMIT, and not part of the fix: per the managing harness's project-memory step, CLAUDE.md was promoted to AGENTS.md with CLAUDE.md kept as a tracked symlink (git mode 120000), so every existing reference to CLAUDE.md across README/CONTRIBUTING/port docs still resolves. The durable rule ('never hardcode the npm publish set') was recorded in AGENTS.md beside the existing Publishing pointer. This rename is a repo-convention change riding along in a release-plumbing PR; it was flagged to the user as separable. Also added a CHANGELOG [Unreleased] entry under the existing prerelease/release-fix sections.
Pre-existing and NOT introduced here: two SC2164 shellcheck warnings in scripts/ci-local.sh (confirmed present at HEAD before this change, count unchanged); the new workflow step body is shellcheck-clean.
What Changed
.github/workflows/publish-npm.ymlwith dynamic derivation via newscripts/publish-set.mjs(single source of truth that validates dependency closure and topological tier ordering)docs-sitetoTIER_ORDERand made untiered packages throw instead of silently sorting first (which put them ahead of their own dependencies)scripts/test-publish-set.mjstest suite and CI gate check inscripts/ci-local.sh('publish-set parity')Risk Assessment
✅ Low: Well-tested fix with comprehensive behavioral tests, proper error handling, and good safeguards against future drift; the one finding is a minor test-quality issue that doesn't affect correctness.
Testing
Verified the publish-set fix through unit tests (6 invariant checks including regression test), derivation output (14 packages with docs-site at position #7, cli at #13), CLI tarball inspection (docs-site@0.23.2 present in packed dependencies), and CI gates integration (publish-set parity gate passes). All tests confirm the bug is fixed, the root cause eliminated, and future drift prevented.
Evidence: Test Summary
Evidence: Publish Set Derivation
Evidence: CLI Tarball Dependencies
Evidence: Publish Set Tests
Evidence: Workflow Comparison
Evidence: Workflow Integration
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
scripts/test-publish-set.mjs:115- Redundant source-content check:TIER_ORDER.includes("docs-site")reads the TIER_ORDER constant directly rather than testing behavior. The behavioral invariant (publishSet() throws if a member is untiered) is already tested by the publishSet() call on line 105, which would have thrown if docs-site were not in TIER_ORDER. This check tests source content rather than executable behavior.✅ **Test** - passed
✅ No issues found.
node scripts/test-publish-set.mjs- all 6 unit tests passnode scripts/publish-set.mjs --check- derivation produces 14 packages, tier-ordered, closed over depsbun pm packCLI tarball - verified docs-site@0.23.2 is present in packed dependenciesscripts/ci-local.sh --only gates- all 10 gates pass including new publish-set parity gate✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.