fix(release): publish @loopover/contract, and catch this class of break before it ships (#9749) - #9763
Conversation
…ak before it ships (#9749) @loopover/contract is a RUNTIME dependency of both published CLIs -- mcp and miner import it from code that ships, built with plain tsc so the specifiers survive into the emitted JS -- but it had no publish workflow and was never on npm. The currently-published versions predate the dependency and install fine, so nothing has broken yet; the NEXT release of either would publish `"@loopover/contract": "^0.1.0"` and fail with E404 for every external user, and a published version cannot be taken back. Option A (publish it) over the alternatives: the package is already shaped for it (publishConfig.access public, a files allowlist, no private:true) and is the only workspace package with that shape and no publish workflow -- the step was simply never added when #9521 split it out. Bundling it into both consumers would change two build pipelines and their dist shape; vendoring the types would re-create exactly the duplication #9521 removed. publish-contract.yml mirrors publish-engine.yml's privilege separation (unprivileged validate builds/packs/smoke-tests, privileged publish only tags and publishes the tarball that job already tested) with a contract smoke test that imports both the root and a subpath export, since consumers use `@loopover/contract/api-schemas` and friends. The more useful half is scripts/check-publishable-deps.ts: a published package must not depend on an unpublishable workspace one. Proven against the real bug by removing the new workflow and watching it name both consumers and the reason. It is deliberately about the CLASS -- any future workspace package that becomes a runtime dependency of a published one is caught the same way, with no list to remember to update -- and it derives "published" from the publish-*.yml workflows themselves rather than a hand-kept list, so it stays honest when a fifth package appears. devDependencies are ignored: they never ship, so flagging them would be noise that pushes contributors toward the wrong fix. Two things it deliberately does NOT do, both stated where they matter: it cannot enforce ORDERING between two publishes (contract must go out before an mcp release depending on a new version), which stays a runbook step in the workflow's header; and npm's OIDC cannot create a brand-new package, so the first contract publish needs a one-time authenticated bootstrap plus a Trusted Publisher entry -- called out in the workflow header rather than left to be discovered when the first dispatch fails.
|
Warning ⏸️ LoopOver review result - manual review recommendedReview updated: 2026-07-29 09:31:26 UTC
Review summary Nits — 7 non-blocking
Concerns raised — review before merging
📋 Copy for AI agents — paste into your coding agentDecision drivers
Context & advisory signals — never blocks the verdict
Linked issue satisfactionAddressed Review context
Contributor next steps
Signal definitions
🧪 Chat with LoopOverAsk LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.
Full command reference: https://loopover.ai/docs/loopover-commands 🧪 Experimental — new and may change. Decision record
🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed 💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →. Checked by LoopOver, a quiet PR intelligence layer for OSS maintainers.
|
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
loopover-ui | fbf1728 | Commit Preview URL Branch Preview URL |
Jul 29 2026, 09:17 AM |
…ack check, watched paths (#9654) #9654 requires all of it in one PR, and my first pass had only the workflow and the dependency guard. Completing the rest. - release-please registration in both config and manifest, contract FIRST in the packages map so a release PR lists it above its consumers -- it must publish before them. Plus the CHANGELOG.md the registration requires. - RELEASE_AUTOMATION_WATCHED_PATHS gains the contract's package.json, CHANGELOG.md and publish workflow, so checkWatchedPathsExist fails if any is moved or deleted -- losing one silently returns the package to the unpublished state that breaks its consumers' next release. - check-contract-package.ts with the contract's OWN allowlist, never reusing MCP's: the two ship completely different trees (named CLI entrypoints plus a scripts/ helper vs a whole dist/ of schema modules), so one shared list would over-permit one and under-permit the other -- which is how a package starts shipping a file nobody reviewed. Asserted. - The dependency guard now keys on BOTH signals per the issue's spec: a dependency missing from release-please-config.json is never version-bumped or released, which fails differently from having no publish workflow but ships the same broken tarball. The pack check earned its keep on first run: dist/telemetry.js carried a literal `-----BEGIN RSA PRIVATE KEY-----` inside a doc comment explaining the redaction regex. Not a leaked credential, but a marker that trips every secret scanner reading the tarball -- ours and a consumer's own. Fixed at the SOURCE by describing the header instead of quoting it, rather than excluding the file or weakening the scanner. The redaction pattern itself is byte-identical and its 28 tests still pass.
… any package mention
…9749 # Conflicts: # .github/workflows/publish-contract.yml # package.json # release-please-config.json
|
Rebased onto Review finding — Three tests cover it: the regression case (a workflow that builds/tests three siblings and packs one yields only the packed one), the no-pack-line case, and an invariant asserting every real Descoped against One tightening to the workflow That removal rests on the doc comment staying descriptive, so it's pinned by a regression test plus an invariant on Local: |
…D_SECRET_KINDS The provider-key shapes in scripts/forbidden-content.ts are hand-copied from SECRET_PATTERNS, and nothing forced the copy to keep up: a 17th HARD_SECRET_KIND added and forgotten would leave every published tarball unscanned for that shape while the whole suite stayed green. Assert a probe exists for every member of the set, and that the copy is never NARROWER than canonical for any of them. The copy stays a copy on purpose -- it is deliberately wider on three shapes, and importing would narrow a scanner that guards immutable artifacts. The header said the copy existed because this was a .mjs run via node; it is .ts run via tsx, so it now states the real reason.
Bundle ReportBundle size has no change ✅ |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #9763 +/- ##
==========================================
+ Coverage 90.33% 95.57% +5.24%
==========================================
Files 917 814 -103
Lines 113904 62930 -50974
Branches 26966 21930 -5036
==========================================
- Hits 102894 60145 -42749
+ Misses 9681 1474 -8207
+ Partials 1329 1311 -18
Flags with carried forward coverage won't be shown. Click here to find out more.
|
…an npm main added publish-mcp-registry.yml, which publishes to the MCP registry and packs no tarball. publishedPackageNames already handled it correctly -- a workflow with no pack line contributes nothing -- but the invariant asserted every publish-*.yml yields exactly one package. Encode the real rule instead: at most one per workflow, the npm-publishing set is exactly the five packages actually on npm, and the zero-yield workflow is named so a NEW one fails rather than being waved through.
…9749 # Conflicts: # package.json
…9749 # Conflicts: # package.json
1 participant
The break
@loopover/contractis a runtime dependency of both published CLIs —mcpandminerimport it from code that ships, built with plaintscso the specifiers survive into the emitted JS — but it had no publish workflow and was never on npm.Nothing has broken yet: the currently-published versions predate the dependency and install fine. The next release of either would publish
"@loopover/contract": "^0.1.0"and fail with E404 for every external user — and a published version can't be taken back. It also breaks the documented contributor workflow, sinceSKILL.mdinstructsnpm install -g @loopover/mcp@latestas the pre-submit oracle.Option A, and why not B or C
The package is already shaped for publishing —
publishConfig.access: public, afilesallowlist, noprivate: true— and is the only workspace package with that shape and no publish workflow. The step was simply never added when #9521 split it out.Bundling it into both consumers (B) changes two build pipelines and their
distshape. Vendoring the types (C) re-creates exactly the duplication #9521 consolidated into this package.publish-contract.ymlmirrorspublish-engine.yml's privilege separation: the unprivilegedvalidatejob builds, packs and smoke-tests; the privilegedpublishjob only tags and publishes the tarball that job already tested, so a compromised build dependency can't reach the OIDC token. The smoke test imports both the root export and a subpath (/public-api), since consumers use@loopover/contract/api-schemasand friends.The more useful half
scripts/check-publishable-deps.ts— a published package must not depend on an unpublishable workspace one.I proved it catches the real bug rather than assuming: removing the new workflow makes it fail, naming both consumers and the reason.
It's deliberately about the class: any future workspace package that becomes a runtime dependency of a published one is caught the same way, with no list to remember to update. "Published" is derived from the
publish-*.ymlworkflows themselves rather than a hand-kept list, so it stays honest when a fifth package appears — and a non-publish workflow mentioning a package can't fake it published.devDependenciesare ignored: they never ship, so flagging them would be noise that pushes contributors toward the wrong fix. A private dependency gets a distinct message, because adding a publish workflow fixes the unpublished case and cannot fix that one.Wired into
test:cibeside the other drift checks. 9 unit tests on the pure core.Two things this deliberately does not do
Both stated where they matter rather than left to be discovered:
Closes #9749