From 81e8976e6437b061988f40d78ab7e3d7b6c9db79 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Wed, 29 Jul 2026 00:14:14 -0700 Subject: [PATCH 1/5] fix(release): publish @loopover/contract, and catch this class of break 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. --- .github/workflows/publish-contract.yml | 271 +++++++++++++++++++++++ package.json | 3 +- scripts/check-publishable-deps.ts | 125 +++++++++++ test/unit/check-publishable-deps.test.ts | 104 +++++++++ 4 files changed, 502 insertions(+), 1 deletion(-) create mode 100644 .github/workflows/publish-contract.yml create mode 100644 scripts/check-publishable-deps.ts create mode 100644 test/unit/check-publishable-deps.test.ts diff --git a/.github/workflows/publish-contract.yml b/.github/workflows/publish-contract.yml new file mode 100644 index 000000000..1e6342b2d --- /dev/null +++ b/.github/workflows/publish-contract.yml @@ -0,0 +1,271 @@ +name: Publish Contract Package + +# workflow_dispatch-only, mirroring publish-engine.yml's design exactly (see that file for the fuller +# rationale): a GITHUB_TOKEN-created tag doesn't fire push-triggered workflows, so the release +# automation must explicitly dispatch this after tagging. +# +# WHY THIS EXISTS (#9749): @loopover/contract is a RUNTIME dependency of the two published CLIs +# (@loopover/mcp and @loopover/miner both import it from code that ships, built with plain tsc so the +# specifiers survive into the emitted JS) -- but it had no publish workflow, so it was never on npm. The +# currently-published CLI versions predate the dependency and still install; the next release of either +# would have shipped `"@loopover/contract": "^0.1.0"` and failed with E404 for every external user. +# +# RELEASE ORDERING IS A REAL CONSTRAINT: contract must be published BEFORE any mcp/miner release that +# depends on a new version of it. scripts/check-publishable-deps.ts enforces the invariant that a +# published package never depends on an unpublishable workspace package; it cannot enforce ordering +# between two publishes, so that stays a release-runbook step. +# +# BOOTSTRAP: npm's trusted publishing/OIDC cannot CREATE a brand-new package, so the very first publish +# must be done once by a maintainer from an authenticated `npm login` session. After that, configure a +# Trusted Publisher for it in npmjs.com's package settings (GitHub Actions provider, org/repo/workflow +# filename matching this file) -- a one-time dashboard step -- and this workflow takes over. + +on: + workflow_dispatch: + inputs: + released_by_release_please: + description: "Internal: set by the release automation's dispatch so this run skips re-creating the GitHub release it already made." + type: boolean + default: false + +permissions: + contents: read + +concurrency: + group: publish-contract-${{ github.ref_name }} + cancel-in-progress: false + +jobs: + # Unprivileged: resolves the version, runs the package's own test suite, and packs the tarball -- + # all with contents: read only. Same privilege-separation reasoning as publish-mcp.yml's validate + # job (Superagent P2 / mirrors metagraphed's publish-client.yml). + validate: + runs-on: ubuntu-latest + timeout-minutes: 20 + permissions: + contents: read + outputs: + version: ${{ steps.version.outputs.version }} + tag: ${{ steps.version.outputs.tag }} + release_sha: ${{ steps.version.outputs.release_sha }} + steps: + - name: Checkout + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7 + with: + fetch-depth: 0 + persist-credentials: false + + - name: Verify release commit is on main + env: + RELEASE_SHA: ${{ github.sha }} + run: | + set -euo pipefail + git fetch --no-tags origin +refs/heads/main:refs/remotes/origin/main + if ! git merge-base --is-ancestor "$RELEASE_SHA" refs/remotes/origin/main; then + echo "::error::Contract package releases must be cut from a commit reachable from main." + exit 1 + fi + + - name: Setup Node + uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7 + with: + node-version-file: .nvmrc + + - name: Resolve release version + id: version + run: | + set -euo pipefail + VERSION="$(node -p "require('./packages/loopover-contract/package.json').version")" + if ! [[ "$VERSION" =~ ^[0-9]+\.[0-9]+\.[0-9]+$ ]]; then + echo "::error::Invalid package version: $VERSION" + exit 1 + fi + TAG="contract-v${VERSION}" + # The RELEASE COMMIT is the newest main commit that introduced this exact version string + # into package.json (the release PR's merge commit) -- NOT whatever main head happens to + # be when this run was dispatched. A bare/reconcile dispatch used to tag-and-pack HEAD, + # which silently swept any commits that landed after the version bump into the tag and the + # published artifact (#8525: ui-kit-v1.1.2 got tagged two commits late during a runner + # backlog, absorbing the 1.1.3 fix and zombifying its release PR). Resolving the commit + # from the version string keeps the release-please-dispatched path a no-op (it dispatches + # against the tag, so HEAD already IS this commit) while making late dispatches correct. + RELEASE_SHA="$(git log -n 1 --format=%H -S "\"version\": \"${VERSION}\"" refs/remotes/origin/main -- packages/loopover-contract/package.json)" + if [ -z "$RELEASE_SHA" ]; then + echo "::error::Could not resolve the commit that introduced version $VERSION into packages/loopover-contract/package.json -- refusing to guess (would tag main head). (#8525)" + exit 1 + fi + if ! git merge-base --is-ancestor "$RELEASE_SHA" refs/remotes/origin/main; then + echo "::error::Resolved release commit $RELEASE_SHA is not reachable from main." + exit 1 + fi + if git rev-parse -q --verify "refs/tags/$TAG" >/dev/null; then + TAG_SHA="$(git rev-list -n 1 "$TAG")" + if [ "$TAG_SHA" != "$RELEASE_SHA" ]; then + echo "::error::Tag $TAG already exists but points at $TAG_SHA, not the resolved release commit $RELEASE_SHA" + exit 1 + fi + echo "Tag $TAG already exists and matches the resolved release commit." + else + echo "Tag $TAG does not exist yet; the publish job will create it at $RELEASE_SHA." + fi + # Build/typecheck/pack against the release commit's own tree, so the published artifact is + # byte-for-byte the content the version number describes. + git checkout --detach "$RELEASE_SHA" + { + echo "version=$VERSION" + echo "tag=$TAG" + echo "release_sha=$RELEASE_SHA" + } >> "$GITHUB_OUTPUT" + + - name: Install dependencies + run: npm ci + + # The contract package's gate is its build + the repo typecheck that consumes it: every consumer + # resolves its types from dist/, so a broken emit is exactly what must not reach npm. + - name: Build contract package + run: npx turbo run build --filter=@loopover/contract + + # Build + pack happen in THIS unprivileged job (no id-token). The privileged publish job below + # never runs npm install/build, so a compromised build dependency can't reach the OIDC token. + - name: Pack and smoke-test the tarball + run: | + set -euo pipefail + PACK_JSON="$(npm pack --workspace @loopover/contract --pack-destination "$RUNNER_TEMP" --json)" + TARBALL="$(node -e 'const fs=require("fs"); const input=fs.readFileSync(0,"utf8"); process.stdout.write(JSON.parse(input)[0].filename)' <<< "$PACK_JSON")" + TARBALL_PATH="$RUNNER_TEMP/$TARBALL" + UNEXPECTED_FILES="$(tar -tzf "$TARBALL_PATH" | grep -Ev '^(package/dist/.+|package/(package.json|README.md|CHANGELOG.md|LICENSE))$' || true)" + if [ -n "$UNEXPECTED_FILES" ]; then + printf '%s\n' "$UNEXPECTED_FILES" + echo "Unexpected file in package tarball" + exit 1 + fi + if tar -xOf "$TARBALL_PATH" | grep -qE '(BEGIN (RSA |EC |OPENSSH )?PRIVATE KEY|github_pat_|gh[pousr]_|gts_[0-9a-f]{64}|[A-Z0-9_]*(TOKEN|SECRET|PRIVATE_KEY)=)'; then + echo "Secret-like content found in package tarball" + exit 1 + fi + TMP="$(mktemp -d)" + npm --prefix "$TMP" init -y >/dev/null + npm --prefix "$TMP" install "$TARBALL_PATH" >/dev/null + node --input-type=module -e " + import { buildMcpToolCallProperties } from '$TMP/node_modules/@loopover/contract/dist/index.js'; + import { PublicStatsSchema } from '$TMP/node_modules/@loopover/contract/dist/public-api.js'; + if (typeof buildMcpToolCallProperties !== 'function') throw new Error('contract root export smoke test failed'); + if (typeof PublicStatsSchema?.parse !== 'function') throw new Error('public-api subpath smoke test failed'); + " + + - name: Upload package tarball + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + with: + name: loopover-contract-tarball + path: ${{ runner.temp }}/*.tgz + if-no-files-found: error + retention-days: 7 + + # Privileged: tags + publishes the EXACT tarball the unprivileged job already tested. environment: + # release requires reviewer approval per repo Settings > Environments, same gate publish-mcp.yml + # and release-selfhost.yml already use. + publish: + runs-on: ubuntu-latest + needs: validate + environment: release + timeout-minutes: 15 + permissions: + contents: write + id-token: write + steps: + - name: Checkout + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7 + with: + fetch-depth: 0 + persist-credentials: false + + - name: Verify release commit is on main + env: + RELEASE_SHA: ${{ github.sha }} + run: | + set -euo pipefail + git fetch --no-tags origin +refs/heads/main:refs/remotes/origin/main + if ! git merge-base --is-ancestor "$RELEASE_SHA" refs/remotes/origin/main; then + echo "::error::Contract package releases must be cut from a commit reachable from main." + exit 1 + fi + + - name: Setup Node + uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7 + with: + node-version-file: .nvmrc + registry-url: https://registry.npmjs.org + + - name: Create or verify release tag + env: + GH_TOKEN: ${{ github.token }} + TAG: ${{ needs.validate.outputs.tag }} + VERSION: ${{ needs.validate.outputs.version }} + RELEASE_SHA: ${{ needs.validate.outputs.release_sha }} + run: | + set -euo pipefail + if git rev-parse -q --verify "refs/tags/$TAG" >/dev/null; then + echo "Tag $TAG already exists (verified against the resolved release commit by the validate job)." + else + echo "Creating tag $TAG at the resolved release commit ($RELEASE_SHA) -- never at HEAD (#8525)." + git config user.name "github-actions[bot]" + git config user.email "41898282+github-actions[bot]@users.noreply.github.com" + git tag -a "$TAG" -m "@loopover/contract v${VERSION}" "$RELEASE_SHA" + git remote set-url origin "https://github.com/${GITHUB_REPOSITORY}.git" + gh auth setup-git + git push origin "$TAG" + fi + + - name: Download package tarball + uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1 + with: + name: loopover-contract-tarball + path: ${{ runner.temp }}/loopover-contract-package + + - name: Publish to npm (OIDC trusted publishing) + env: + NPM_CONFIG_PROVENANCE: "true" + run: | + set -euo pipefail + count=$(find "$RUNNER_TEMP/loopover-contract-package" -maxdepth 1 -type f -name "*.tgz" | wc -l | tr -d ' ') + if [ "$count" != "1" ]; then + echo "Expected exactly one tarball, found $count" >&2 + find "$RUNNER_TEMP/loopover-contract-package" -maxdepth 1 -type f -name "*.tgz" -print >&2 + exit 1 + fi + tarball=$(find "$RUNNER_TEMP/loopover-contract-package" -maxdepth 1 -type f -name "*.tgz" -print -quit) + npx -y npm@11.15.0 publish "$tarball" --access public --provenance + + github-release: + runs-on: ubuntu-latest + needs: [validate, publish] + # Skip when the release automation dispatched this run: it already created the GitHub release + # with its own generated changelog notes before dispatching, so running this unconditionally + # would overwrite those richer notes with the generic blurb below. + if: ${{ inputs.released_by_release_please != true }} + timeout-minutes: 5 + permissions: + contents: write + steps: + - name: Create GitHub release + env: + GH_TOKEN: ${{ github.token }} + RELEASE_TAG: ${{ needs.validate.outputs.tag }} + RELEASE_VERSION: ${{ needs.validate.outputs.version }} + run: | + set -euo pipefail + NOTES_FILE="$(mktemp)" + cat > "$NOTES_FILE" </dev/null 2>&1; then + gh release edit "$RELEASE_TAG" --repo "$GITHUB_REPOSITORY" --title "@loopover/contract v${RELEASE_VERSION}" --notes-file "$NOTES_FILE" + else + gh release create "$RELEASE_TAG" --repo "$GITHUB_REPOSITORY" --title "@loopover/contract v${RELEASE_VERSION}" --notes-file "$NOTES_FILE" --verify-tag + fi diff --git a/package.json b/package.json index b77c15b80..f626ad4f5 100644 --- a/package.json +++ b/package.json @@ -75,6 +75,7 @@ "validate:no-hand-written-js": "node --experimental-strip-types scripts/validate-no-hand-written-js.ts", "import-specifiers:check": "node --experimental-strip-types scripts/check-import-specifiers.ts", "ui-derived-types:check": "node --experimental-strip-types scripts/check-ui-derived-types.ts", + "publishable-deps:check": "tsx scripts/check-publishable-deps.ts", "dead-source-files:check": "node --experimental-strip-types scripts/check-dead-source-files.ts", "regate-sort-key:check": "node --experimental-strip-types scripts/check-regate-sort-key.ts", "command-redelivery-guards:check": "node --experimental-strip-types scripts/check-command-redelivery-guards.ts", @@ -126,7 +127,7 @@ "test:smoke:browser:install": "playwright install chromium", "test:smoke:browser": "node --experimental-strip-types scripts/smoke-ui-browser.ts", "pretest:ci": "npm run check-node-version", - "test:ci": "git diff --check && npm run actionlint && npm run lint:composite-actions && npm run db:migrations:check && npm run db:schema-drift:check && npm run selfhost:env-reference:check && npm run miner:env-reference:check && npm run selfhost:validate-observability && npm run cf-typegen:check && npm run build --workspace @loopover/engine && npm run build --workspace @loopover/discovery-index && npm run build:mcp && npm run build:miner && npm run build --workspace @loopover/ui-kit && npm run typecheck && npm run test:coverage && npm run test:engine-parity && npm run test:live-gate-parity && npm run test:driver-parity && npm run validate:mcp && npm run test --workspace @loopover/engine && npm run test:workers && npm run test:mcp-pack && npm run test:miner-pack && npm run test:engine-pack && npm run test:ui-kit-pack && npm run test:miner-deployment-docs-audit && npm run rees:test && npm run ui:openapi:check && npm run ui:version-audit && npm run docs:drift-check && npm run coverage-boltons:check && npm run import-specifiers:check && npm run ui-derived-types:check && npm run dead-source-files:check && npm run regate-sort-key:check && npm run command-redelivery-guards:check && npm run dispatch-gate-reasons:check && npm run validate:no-hand-written-js && npm run replay-runner-manifest:check && npm run coco-dev-versions:check && npm run branding-drift:check && npm run manifest:drift-check && npm run engine-parity:drift-check && npm run engines-nvmrc:check && npm run release-manifest:sync:check && npm run command-reference:check && npm run mcp:tool-reference:check && npm run contract:api-schemas:check && npm run ui:lint && npm run ui:typecheck && npm run ui:test && npm run ui:build", + "test:ci": "git diff --check && npm run actionlint && npm run lint:composite-actions && npm run db:migrations:check && npm run db:schema-drift:check && npm run selfhost:env-reference:check && npm run miner:env-reference:check && npm run selfhost:validate-observability && npm run cf-typegen:check && npm run build --workspace @loopover/engine && npm run build --workspace @loopover/discovery-index && npm run build:mcp && npm run build:miner && npm run build --workspace @loopover/ui-kit && npm run typecheck && npm run test:coverage && npm run test:engine-parity && npm run test:live-gate-parity && npm run test:driver-parity && npm run validate:mcp && npm run test --workspace @loopover/engine && npm run test:workers && npm run test:mcp-pack && npm run test:miner-pack && npm run test:engine-pack && npm run test:ui-kit-pack && npm run test:miner-deployment-docs-audit && npm run rees:test && npm run ui:openapi:check && npm run ui:version-audit && npm run docs:drift-check && npm run coverage-boltons:check && npm run import-specifiers:check && npm run ui-derived-types:check && npm run dead-source-files:check && npm run publishable-deps:check && npm run regate-sort-key:check && npm run command-redelivery-guards:check && npm run dispatch-gate-reasons:check && npm run validate:no-hand-written-js && npm run replay-runner-manifest:check && npm run coco-dev-versions:check && npm run branding-drift:check && npm run manifest:drift-check && npm run engine-parity:drift-check && npm run engines-nvmrc:check && npm run release-manifest:sync:check && npm run command-reference:check && npm run mcp:tool-reference:check && npm run contract:api-schemas:check && npm run ui:lint && npm run ui:typecheck && npm run ui:test && npm run ui:build", "test:release": "npm run test:ci && npm run changelog:check", "test:release:mcp": "npm run test:ci", "test:watch": "vitest", diff --git a/scripts/check-publishable-deps.ts b/scripts/check-publishable-deps.ts new file mode 100644 index 000000000..5e8a685be --- /dev/null +++ b/scripts/check-publishable-deps.ts @@ -0,0 +1,125 @@ +#!/usr/bin/env node +// A published package must not depend on an unpublishable one (#9749). +// +// @loopover/contract was split out by #9521 and became a RUNTIME dependency of both published CLIs +// (@loopover/mcp and @loopover/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, so it was never on npm. Nothing caught +// that, because the failure is invisible until the NEXT release: the currently-published CLI versions +// predate the dependency and install fine. The first user to run `npm install -g @loopover/mcp@latest` +// after that release gets E404, and by then the broken version is already public and immutable. +// +// This is the check that would have failed the moment the dependency was added. It is deliberately about +// the CLASS, not about `@loopover/contract`: 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. +// +// WHAT "PUBLISHED" MEANS HERE: a workspace package with a `.github/workflows/publish-.yml`. That is +// the repo's own operational definition -- the four packages that have one are exactly the four on npm -- +// and deriving it from the workflows rather than a hand-maintained list is what keeps this honest when a +// fifth package is added. +// +// WHAT IT DOES NOT CHECK: ordering between two publishes. A contract release must go out BEFORE an mcp +// release that depends on a new version of it, and no static check can see that -- it stays a +// release-runbook step, called out in publish-contract.yml's own header. +import { readdirSync, readFileSync } from "node:fs"; +import { join } from "node:path"; +import { fileURLToPath, URL } from "node:url"; + +export type PackageManifest = { name?: string; private?: boolean; dependencies?: Record }; + +/** One violation, phrased as the failure it will cause rather than as a rule number. */ +export type PublishableDepViolation = { publishedPackage: string; dependency: string; range: string; reason: string }; + +/** + * PURE core: given every workspace manifest and the set of packages that have a publish workflow, name every + * runtime dependency of a published package that cannot itself be installed from npm. + * + * Only `dependencies` are walked. A `devDependency` is not shipped in the tarball and cannot break an + * end user's install, so flagging one would be noise -- and would push contributors toward the wrong fix. + */ +export function findPublishableDepViolations( + manifests: readonly PackageManifest[], + publishedNames: ReadonlySet, +): PublishableDepViolation[] { + const workspaceNames = new Set(manifests.map((manifest) => manifest.name).filter((name): name is string => Boolean(name))); + const privateNames = new Set(manifests.filter((manifest) => manifest.private).map((manifest) => manifest.name)); + const violations: PublishableDepViolation[] = []; + + for (const manifest of manifests) { + if (!manifest.name || !publishedNames.has(manifest.name)) continue; + for (const [dependency, range] of Object.entries(manifest.dependencies ?? {})) { + // Only WORKSPACE packages can be unpublishable in this way; a third-party dependency is on npm by + // definition of being installable at all. + if (!workspaceNames.has(dependency)) continue; + if (privateNames.has(dependency)) { + violations.push({ + publishedPackage: manifest.name, + dependency, + range, + reason: `${dependency} is marked private and can never be published`, + }); + continue; + } + if (!publishedNames.has(dependency)) { + violations.push({ + publishedPackage: manifest.name, + dependency, + range, + reason: `${dependency} has no .github/workflows/publish-*.yml, so it is not published to npm`, + }); + } + } + } + return violations; +} + +/** Map `publish-.yml` to the package name it publishes, by reading the workflow's own filter. Derived + * rather than assumed from the filename: `publish-ui-kit.yml` publishes `@loopover/ui-kit`, and a future + * workflow could name its package anything. */ +export function publishedPackageNames(workflowFiles: ReadonlyArray<{ name: string; text: string }>): Set { + const names = new Set(); + for (const file of workflowFiles) { + if (!/^publish-.+\.ya?ml$/.test(file.name)) continue; + for (const match of file.text.matchAll(/@loopover\/[a-z0-9-]+/g)) names.add(match[0]); + } + return names; +} + +function main(): void { + const root = join(fileURLToPath(new URL(".", import.meta.url)), ".."); + const packagesDir = join(root, "packages"); + const manifests: PackageManifest[] = []; + for (const entry of readdirSync(packagesDir, { withFileTypes: true })) { + if (!entry.isDirectory()) continue; + try { + manifests.push(JSON.parse(readFileSync(join(packagesDir, entry.name, "package.json"), "utf8")) as PackageManifest); + } catch { + // A directory without a manifest is not a workspace package; skip rather than fail the whole check. + } + } + + const workflowsDir = join(root, ".github", "workflows"); + const workflowFiles = readdirSync(workflowsDir) + .filter((name) => /^publish-.+\.ya?ml$/.test(name)) + .map((name) => ({ name, text: readFileSync(join(workflowsDir, name), "utf8") })); + + const published = publishedPackageNames(workflowFiles); + const violations = findPublishableDepViolations(manifests, published); + + if (violations.length > 0) { + console.error("check-publishable-deps: a PUBLISHED package depends on something users cannot install:\n"); + for (const violation of violations) { + console.error(` ${violation.publishedPackage} -> "${violation.dependency}": "${violation.range}"`); + console.error(` ${violation.reason}`); + } + console.error( + "\n The next release of that package would publish an uninstallable tarball (npm E404 for every\n" + + " external user), and a published version cannot be taken back. Fix by adding a\n" + + " .github/workflows/publish-.yml for the dependency, bundling it into the consumer, or\n" + + " demoting it to a devDependency if it is genuinely not needed at runtime.", + ); + process.exit(1); + } + console.log(`check-publishable-deps: OK — ${published.size} published package(s), no runtime dependency on an unpublishable workspace package.`); +} + +if (process.argv[1]?.endsWith("check-publishable-deps.ts")) main(); diff --git a/test/unit/check-publishable-deps.test.ts b/test/unit/check-publishable-deps.test.ts new file mode 100644 index 000000000..0c45c0045 --- /dev/null +++ b/test/unit/check-publishable-deps.test.ts @@ -0,0 +1,104 @@ +import { describe, expect, it } from "vitest"; +import { findPublishableDepViolations, publishedPackageNames } from "../../scripts/check-publishable-deps"; + +// #9749: @loopover/contract became a runtime dependency of two PUBLISHED CLIs but had no publish workflow, +// so it was never on npm. The failure is invisible until the next release — the already-published versions +// predate the dependency and install fine — and by the time a user hits E404 the broken version is public +// and immutable. These pin the check that catches it at the moment the dependency is added. + +const PUBLISHED = new Set(["@loopover/mcp", "@loopover/miner", "@loopover/engine"]); + +describe("findPublishableDepViolations (#9749)", () => { + it("REGRESSION: reproduces the real bug — a published CLI depending on an unpublished workspace package", () => { + const violations = findPublishableDepViolations( + [ + { name: "@loopover/mcp", dependencies: { "@loopover/contract": "^0.1.0", zod: "^4.0.0" } }, + { name: "@loopover/miner", dependencies: { "@loopover/contract": "^0.1.0" } }, + { name: "@loopover/contract", dependencies: {} }, + ], + PUBLISHED, + ); + expect(violations.map((violation) => violation.publishedPackage)).toEqual(["@loopover/mcp", "@loopover/miner"]); + expect(violations[0]).toMatchObject({ dependency: "@loopover/contract", range: "^0.1.0" }); + expect(violations[0]?.reason).toContain("not published to npm"); + }); + + it("passes once the dependency itself becomes published — the fix, not just the detection", () => { + const violations = findPublishableDepViolations( + [ + { name: "@loopover/mcp", dependencies: { "@loopover/contract": "^0.1.0" } }, + { name: "@loopover/contract", dependencies: {} }, + ], + new Set([...PUBLISHED, "@loopover/contract"]), + ); + expect(violations).toEqual([]); + }); + + it("REGRESSION: a PRIVATE dependency is a distinct, harder failure and says so", () => { + // Adding a publish workflow fixes the unpublished case; it cannot fix this one, so the message must + // not send someone down that path. + const violations = findPublishableDepViolations( + [ + { name: "@loopover/mcp", dependencies: { "@loopover/internal": "^1.0.0" } }, + { name: "@loopover/internal", private: true }, + ], + PUBLISHED, + ); + expect(violations).toHaveLength(1); + expect(violations[0]?.reason).toContain("private and can never be published"); + }); + + it("ignores devDependencies — they never ship in the tarball, so they cannot break an install", () => { + const violations = findPublishableDepViolations( + [{ name: "@loopover/mcp", dependencies: { zod: "^4.0.0" } }, { name: "@loopover/contract" }], + PUBLISHED, + ); + expect(violations).toEqual([]); + }); + + it("ignores third-party dependencies and UNPUBLISHED packages' own dependencies", () => { + const violations = findPublishableDepViolations( + [ + // Third-party: on npm by definition of being installable at all. + { name: "@loopover/mcp", dependencies: { zod: "^4.0.0", "posthog-node": "^5.0.0" } }, + // An unpublished package may depend on whatever it likes -- no user ever installs it directly. + { name: "@loopover/ui", dependencies: { "@loopover/contract": "^0.1.0" } }, + { name: "@loopover/contract" }, + ], + PUBLISHED, + ); + expect(violations).toEqual([]); + }); + + it("an empty workspace is not a violation", () => { + expect(findPublishableDepViolations([], PUBLISHED)).toEqual([]); + expect(findPublishableDepViolations([{ dependencies: { "@loopover/contract": "^0.1.0" } }], PUBLISHED)).toEqual([]); + }); +}); + +describe("publishedPackageNames (#9749)", () => { + it("derives the published set from the workflows themselves, not from filenames or a hand-kept list", () => { + // `publish-ui-kit.yml` publishes `@loopover/ui-kit`; a future workflow could name its package anything, + // so the mapping is read from the file rather than inferred from the slug. + const names = publishedPackageNames([ + { name: "publish-ui-kit.yml", text: "run: npm run test --workspace @loopover/ui-kit" }, + { name: "publish-engine.yml", text: "npm pack --workspace @loopover/engine --json" }, + ]); + expect([...names].sort()).toEqual(["@loopover/engine", "@loopover/ui-kit"]); + }); + + it("ignores workflows that are not publish-*, so a CI file mentioning a package cannot fake it published", () => { + const names = publishedPackageNames([ + { name: "ci.yml", text: "npm run build --workspace @loopover/contract" }, + { name: "release-selfhost.yml", text: "@loopover/engine" }, + ]); + expect(names.size).toBe(0); + }); + + it("accepts both .yml and .yaml, and dedupes repeated mentions within one workflow", () => { + const names = publishedPackageNames([ + { name: "publish-mcp.yaml", text: "@loopover/mcp ... @loopover/mcp ... @loopover/mcp" }, + ]); + expect([...names]).toEqual(["@loopover/mcp"]); + }); +}); From 3ba9942aaa9ac0318b597ea43cc882c36145eae0 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Wed, 29 Jul 2026 00:24:14 -0700 Subject: [PATCH 2/5] =?UTF-8?q?feat(release):=20complete=20the=20contract?= =?UTF-8?q?=20publish=20path=20=E2=80=94=20release-please,=20pack=20check,?= =?UTF-8?q?=20watched=20paths=20(#9654)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #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. --- .release-please-manifest.json | 1 + package.json | 3 +- packages/loopover-contract/CHANGELOG.md | 10 +++ packages/loopover-contract/src/telemetry.ts | 6 +- release-please-config.json | 78 +++++++++++++++++---- scripts/check-contract-package.ts | 67 ++++++++++++++++++ scripts/check-publishable-deps.ts | 39 +++++++++-- scripts/contract-package-allowlist.ts | 17 +++++ scripts/lib/validate-mcp/overrides.ts | 5 ++ test/unit/check-contract-package.test.ts | 55 +++++++++++++++ 10 files changed, 259 insertions(+), 22 deletions(-) create mode 100644 packages/loopover-contract/CHANGELOG.md create mode 100644 scripts/check-contract-package.ts create mode 100644 scripts/contract-package-allowlist.ts create mode 100644 test/unit/check-contract-package.test.ts diff --git a/.release-please-manifest.json b/.release-please-manifest.json index dd24bcc4d..915bc59b6 100644 --- a/.release-please-manifest.json +++ b/.release-please-manifest.json @@ -1,4 +1,5 @@ { + "packages/loopover-contract": "0.1.0", "packages/loopover-mcp": "3.15.2", "packages/loopover-engine": "3.16.1", "packages/loopover-miner": "3.15.2", diff --git a/package.json b/package.json index f626ad4f5..d2b20704b 100644 --- a/package.json +++ b/package.json @@ -38,6 +38,7 @@ "drizzle:generate": "drizzle-kit generate", "build:mcp": "npm --workspace @loopover/mcp run build", "build:miner": "turbo run build --filter=@loopover/engine && npm --workspace @loopover/miner run build", + "test:contract-pack": "tsx scripts/check-contract-package.ts", "test:mcp-pack": "tsx scripts/check-mcp-package.ts", "test:miner-pack": "tsx scripts/check-miner-package.ts", "test:engine-pack": "tsx scripts/check-engine-package.ts", @@ -127,7 +128,7 @@ "test:smoke:browser:install": "playwright install chromium", "test:smoke:browser": "node --experimental-strip-types scripts/smoke-ui-browser.ts", "pretest:ci": "npm run check-node-version", - "test:ci": "git diff --check && npm run actionlint && npm run lint:composite-actions && npm run db:migrations:check && npm run db:schema-drift:check && npm run selfhost:env-reference:check && npm run miner:env-reference:check && npm run selfhost:validate-observability && npm run cf-typegen:check && npm run build --workspace @loopover/engine && npm run build --workspace @loopover/discovery-index && npm run build:mcp && npm run build:miner && npm run build --workspace @loopover/ui-kit && npm run typecheck && npm run test:coverage && npm run test:engine-parity && npm run test:live-gate-parity && npm run test:driver-parity && npm run validate:mcp && npm run test --workspace @loopover/engine && npm run test:workers && npm run test:mcp-pack && npm run test:miner-pack && npm run test:engine-pack && npm run test:ui-kit-pack && npm run test:miner-deployment-docs-audit && npm run rees:test && npm run ui:openapi:check && npm run ui:version-audit && npm run docs:drift-check && npm run coverage-boltons:check && npm run import-specifiers:check && npm run ui-derived-types:check && npm run dead-source-files:check && npm run publishable-deps:check && npm run regate-sort-key:check && npm run command-redelivery-guards:check && npm run dispatch-gate-reasons:check && npm run validate:no-hand-written-js && npm run replay-runner-manifest:check && npm run coco-dev-versions:check && npm run branding-drift:check && npm run manifest:drift-check && npm run engine-parity:drift-check && npm run engines-nvmrc:check && npm run release-manifest:sync:check && npm run command-reference:check && npm run mcp:tool-reference:check && npm run contract:api-schemas:check && npm run ui:lint && npm run ui:typecheck && npm run ui:test && npm run ui:build", + "test:ci": "git diff --check && npm run actionlint && npm run lint:composite-actions && npm run db:migrations:check && npm run db:schema-drift:check && npm run selfhost:env-reference:check && npm run miner:env-reference:check && npm run selfhost:validate-observability && npm run cf-typegen:check && npm run build --workspace @loopover/engine && npm run build --workspace @loopover/discovery-index && npm run build:mcp && npm run build:miner && npm run build --workspace @loopover/ui-kit && npm run typecheck && npm run test:coverage && npm run test:engine-parity && npm run test:live-gate-parity && npm run test:driver-parity && npm run validate:mcp && npm run test --workspace @loopover/engine && npm run test:workers && npm run test:mcp-pack && npm run test:contract-pack && npm run test:miner-pack && npm run test:engine-pack && npm run test:ui-kit-pack && npm run test:miner-deployment-docs-audit && npm run rees:test && npm run ui:openapi:check && npm run ui:version-audit && npm run docs:drift-check && npm run coverage-boltons:check && npm run import-specifiers:check && npm run ui-derived-types:check && npm run dead-source-files:check && npm run publishable-deps:check && npm run regate-sort-key:check && npm run command-redelivery-guards:check && npm run dispatch-gate-reasons:check && npm run validate:no-hand-written-js && npm run replay-runner-manifest:check && npm run coco-dev-versions:check && npm run branding-drift:check && npm run manifest:drift-check && npm run engine-parity:drift-check && npm run engines-nvmrc:check && npm run release-manifest:sync:check && npm run command-reference:check && npm run mcp:tool-reference:check && npm run contract:api-schemas:check && npm run ui:lint && npm run ui:typecheck && npm run ui:test && npm run ui:build", "test:release": "npm run test:ci && npm run changelog:check", "test:release:mcp": "npm run test:ci", "test:watch": "vitest", diff --git a/packages/loopover-contract/CHANGELOG.md b/packages/loopover-contract/CHANGELOG.md new file mode 100644 index 000000000..a44659457 --- /dev/null +++ b/packages/loopover-contract/CHANGELOG.md @@ -0,0 +1,10 @@ +# Changelog + +## 0.1.0 + +Initial release. `@loopover/contract` is the single zod source of truth for LoopOver's MCP tool and API +contracts — the schemas, tool metadata, and derived projections every server and client reads. + +It is published because it is a **runtime** dependency of `@loopover/mcp` and `@loopover/miner`, which +import it from code that ships (#9749). It must be published *before* any release of those packages that +depends on a new version of it. diff --git a/packages/loopover-contract/src/telemetry.ts b/packages/loopover-contract/src/telemetry.ts index 597e656a6..adf24e9b5 100644 --- a/packages/loopover-contract/src/telemetry.ts +++ b/packages/loopover-contract/src/telemetry.ts @@ -134,7 +134,11 @@ const SECRET_KEY_PATTERN = /token|secret|password|passwd|dsn|credential|api[_-]? * The token prefixes carry their own `\b`; the PEM header must NOT, because a word boundary before * a leading hyphen never matches and the whole alternative would be dead. That is not hypothetical * -- it was, until the forbidden-content test in test/unit/mcp-dispatch-telemetry.test.ts caught a - * full `-----BEGIN RSA PRIVATE KEY-----` passing through untouched. + * complete RSA PEM private-key header passing through untouched. + * + * That header is described rather than quoted on purpose: this file is PUBLISHED (#9749), and a literal + * key marker in the tarball trips every secret scanner that reads it -- ours in check-contract-package.ts, + * and a consumer's own. The pattern on the next line is what must be exact; the prose need not be. */ const SECRET_VALUE_PATTERN = /\b(?:gh[pousr]_[A-Za-z0-9]{16,}|sk-[A-Za-z0-9]{16,}|phc_[A-Za-z0-9]{16,})|-----BEGIN [A-Z ]*PRIVATE KEY-----/; diff --git a/release-please-config.json b/release-please-config.json index 1c4b4d1b9..4083a147c 100644 --- a/release-please-config.json +++ b/release-please-config.json @@ -1,6 +1,11 @@ { "$schema": "https://raw.githubusercontent.com/googleapis/release-please/main/schemas/config.json", "packages": { + "packages/loopover-contract": { + "release-type": "node", + "component": "contract", + "package-name": "@loopover/contract" + }, "packages/loopover-mcp": { "release-type": "node", "component": "mcp", @@ -34,22 +39,69 @@ { "type": "linked-versions", "groupName": "engine-and-dependents", - "components": ["engine", "mcp", "miner"], + "components": [ + "engine", + "mcp", + "miner" + ], "merge": true } ], "changelog-sections": [ - { "type": "feat", "section": "Features" }, - { "type": "fix", "section": "Fixes" }, - { "type": "perf", "section": "Performance" }, - { "type": "deps", "section": "Dependencies" }, - { "type": "revert", "section": "Reverts" }, - { "type": "docs", "section": "Docs", "hidden": true }, - { "type": "refactor", "section": "Refactors", "hidden": true }, - { "type": "test", "section": "Tests", "hidden": true }, - { "type": "build", "section": "Build", "hidden": true }, - { "type": "ci", "section": "CI", "hidden": true }, - { "type": "style", "section": "Styles", "hidden": true }, - { "type": "chore", "section": "Chores", "hidden": true } + { + "type": "feat", + "section": "Features" + }, + { + "type": "fix", + "section": "Fixes" + }, + { + "type": "perf", + "section": "Performance" + }, + { + "type": "deps", + "section": "Dependencies" + }, + { + "type": "revert", + "section": "Reverts" + }, + { + "type": "docs", + "section": "Docs", + "hidden": true + }, + { + "type": "refactor", + "section": "Refactors", + "hidden": true + }, + { + "type": "test", + "section": "Tests", + "hidden": true + }, + { + "type": "build", + "section": "Build", + "hidden": true + }, + { + "type": "ci", + "section": "CI", + "hidden": true + }, + { + "type": "style", + "section": "Styles", + "hidden": true + }, + { + "type": "chore", + "section": "Chores", + "hidden": true + } ] } diff --git a/scripts/check-contract-package.ts b/scripts/check-contract-package.ts new file mode 100644 index 000000000..c1c5482c6 --- /dev/null +++ b/scripts/check-contract-package.ts @@ -0,0 +1,67 @@ +#!/usr/bin/env node +// Pack check for @loopover/contract (#9654), mirroring check-mcp-package.ts. +// +// The contract package became publishable in #9749 because it is a RUNTIME dependency of both published +// CLIs. A published tarball is immutable, so the file list and its contents are checked BEFORE the +// publish job ever sees them — the same reasoning as the MCP check, against this package's own allowlist. +import { readFileSync } from "node:fs"; +import { join } from "node:path"; +import { spawnSync } from "node:child_process"; +import { fileURLToPath } from "node:url"; +import { FORBIDDEN_CONTENT } from "./forbidden-content"; +import { CONTRACT_PACKAGE_ALLOWED_FILE_PATTERNS } from "./contract-package-allowlist"; + +const FORBIDDEN_PATH = /(^|\/)(\.dev\.vars|\.env|\.npmrc|.*\.pem|.*private.*key.*|.*secret.*)$/i; + +type PackedFile = string | { path: string }; +type ReadContentFn = (file: string) => string; + +/** PURE over an already-resolved pack listing, so the rules are unit-testable without running npm. */ +export function validateContractPackFileList(files: readonly PackedFile[], readContent: ReadContentFn): string[] { + const paths = files.map((file) => (typeof file === "string" ? file : file.path)).sort(); + if (paths.length === 0) throw new Error("Contract package tarball is empty"); + for (const file of paths) { + if (FORBIDDEN_PATH.test(file)) throw new Error(`Forbidden file in contract package: ${file}`); + if (!CONTRACT_PACKAGE_ALLOWED_FILE_PATTERNS.some((pattern) => pattern.test(file))) { + throw new Error(`Unexpected file in contract package: ${file}`); + } + if (FORBIDDEN_CONTENT.test(readContent(file))) throw new Error(`Secret-like content found in contract package file: ${file}`); + } + return paths; +} + +function loadContractPackFromNpm(): { files: PackedFile[] } { + const result = spawnSync("npm", ["pack", "--workspace", "@loopover/contract", "--dry-run", "--json"], { encoding: "utf8" }); + if (result.status !== 0) throw new Error(`npm pack failed: ${result.stderr || result.stdout}`); + const parsed = JSON.parse(result.stdout) as Array<{ files: PackedFile[] }>; + const pack = parsed[0]; + if (!pack) throw new Error("npm pack returned no package entry"); + return pack; +} + +export function runContractPackCheck( + options: { pack?: { files: PackedFile[] }; packageRoot?: string; readContent?: ReadContentFn } = {}, +): string { + const packageRoot = options.packageRoot ?? join(fileURLToPath(import.meta.url), "..", "..", "packages", "loopover-contract"); + const readContent: ReadContentFn = + options.readContent ?? + ((file) => { + try { + return readFileSync(join(packageRoot, file), "utf8"); + } catch { + // A binary or unreadable file has no text to scan; the allowlist above already bounds what may ship. + return ""; + } + }); + const paths = validateContractPackFileList((options.pack ?? loadContractPackFromNpm()).files, readContent); + return `check-contract-package: OK — ${paths.length} packed file(s), all allowlisted and clean.`; +} + +if (process.argv[1]?.endsWith("check-contract-package.ts")) { + try { + console.log(runContractPackCheck()); + } catch (error) { + console.error(String(error instanceof Error ? error.message : error)); + process.exit(1); + } +} diff --git a/scripts/check-publishable-deps.ts b/scripts/check-publishable-deps.ts index 5e8a685be..731195582 100644 --- a/scripts/check-publishable-deps.ts +++ b/scripts/check-publishable-deps.ts @@ -12,10 +12,12 @@ // the CLASS, not about `@loopover/contract`: 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. // -// WHAT "PUBLISHED" MEANS HERE: a workspace package with a `.github/workflows/publish-.yml`. That is -// the repo's own operational definition -- the four packages that have one are exactly the four on npm -- -// and deriving it from the workflows rather than a hand-maintained list is what keeps this honest when a -// fifth package is added. +// WHAT "RELEASABLE" MEANS HERE: listed in `release-please-config.json` AND carrying a +// `.github/workflows/publish-.yml`. Both, because they fail differently and both failures ship the +// same broken tarball: a package absent from release-please never gets a version bump or a changelog entry +// (so consumers pin a range that is never satisfied), and a package with no publish workflow never reaches +// npm at all. Deriving both from the files themselves, rather than a hand-maintained list, is what keeps +// this honest when a fifth package is added. // // WHAT IT DOES NOT CHECK: ordering between two publishes. A contract release must go out BEFORE an mcp // release that depends on a new version of it, and no static check can see that -- it stays a @@ -39,6 +41,7 @@ export type PublishableDepViolation = { publishedPackage: string; dependency: st export function findPublishableDepViolations( manifests: readonly PackageManifest[], publishedNames: ReadonlySet, + releasePleaseNames: ReadonlySet = publishedNames, ): PublishableDepViolation[] { const workspaceNames = new Set(manifests.map((manifest) => manifest.name).filter((name): name is string => Boolean(name))); const privateNames = new Set(manifests.filter((manifest) => manifest.private).map((manifest) => manifest.name)); @@ -66,6 +69,17 @@ export function findPublishableDepViolations( range, reason: `${dependency} has no .github/workflows/publish-*.yml, so it is not published to npm`, }); + continue; + } + // Registered for publishing but not for RELEASING: it would never get a version bump or a changelog + // entry, so a consumer's range could never be satisfied by a new release. + if (!releasePleaseNames.has(dependency)) { + violations.push({ + publishedPackage: manifest.name, + dependency, + range, + reason: `${dependency} is missing from release-please-config.json, so it is never version-bumped or released`, + }); } } } @@ -103,7 +117,15 @@ function main(): void { .map((name) => ({ name, text: readFileSync(join(workflowsDir, name), "utf8") })); const published = publishedPackageNames(workflowFiles); - const violations = findPublishableDepViolations(manifests, published); + const releaseConfig = JSON.parse(readFileSync(join(root, "release-please-config.json"), "utf8")) as { + packages?: Record; + }; + const releasePleaseNames = new Set( + Object.values(releaseConfig.packages ?? {}) + .map((entry) => entry["package-name"]) + .filter((name): name is string => Boolean(name)), + ); + const violations = findPublishableDepViolations(manifests, published, releasePleaseNames); if (violations.length > 0) { console.error("check-publishable-deps: a PUBLISHED package depends on something users cannot install:\n"); @@ -115,11 +137,14 @@ function main(): void { "\n The next release of that package would publish an uninstallable tarball (npm E404 for every\n" + " external user), and a published version cannot be taken back. Fix by adding a\n" + " .github/workflows/publish-.yml for the dependency, bundling it into the consumer, or\n" + - " demoting it to a devDependency if it is genuinely not needed at runtime.", + " demoting it to a devDependency if it is genuinely not needed at runtime. A package must be in BOTH\n" + + " release-please-config.json and a publish workflow to count as releasable.", ); process.exit(1); } - console.log(`check-publishable-deps: OK — ${published.size} published package(s), no runtime dependency on an unpublishable workspace package.`); + console.log( + `check-publishable-deps: OK — ${published.size} published package(s), ${releasePleaseNames.size} release-please-registered, no runtime dependency on an unreleasable workspace package.`, + ); } if (process.argv[1]?.endsWith("check-publishable-deps.ts")) main(); diff --git a/scripts/contract-package-allowlist.ts b/scripts/contract-package-allowlist.ts new file mode 100644 index 000000000..f479d9d46 --- /dev/null +++ b/scripts/contract-package-allowlist.ts @@ -0,0 +1,17 @@ +// Canonical @loopover/contract published-tarball allowlist (#9654). +// +// Declared SEPARATELY from MCP_PACKAGE_ALLOWED_FILE_PATTERNS on purpose: the two packages ship +// completely different trees (the CLI ships named dist/bin and dist/lib entrypoints plus a scripts/ +// helper; the contract ships a whole dist/ of schema modules), so sharing one list would either +// over-permit the CLI or under-permit the contract, and a shared list is exactly how a package +// starts shipping a file nobody reviewed. +export const CONTRACT_PACKAGE_ALLOWED_FILE_PATTERNS: RegExp[] = [ + // Every emitted module, its declarations and its source maps. The exports map has eight subpaths and + // grows with the contract, so enumerating files by name here would be a maintenance trap that fails + // the release rather than catching anything. + /^dist\/.+\.(js|d\.ts|js\.map|d\.ts\.map)$/, + /^package\.json$/, + /^README\.md$/, + /^CHANGELOG\.md$/, + /^LICENSE$/, +]; diff --git a/scripts/lib/validate-mcp/overrides.ts b/scripts/lib/validate-mcp/overrides.ts index b32b7ff5a..f58bff88b 100644 --- a/scripts/lib/validate-mcp/overrides.ts +++ b/scripts/lib/validate-mcp/overrides.ts @@ -47,4 +47,9 @@ export const RELEASE_AUTOMATION_WATCHED_PATHS: readonly string[] = Object.freeze ".github/workflows/publish-mcp.yml", ".github/workflows/publish-engine.yml", ".github/workflows/publish-miner.yml", + // #9749/#9654: the contract package is a RUNTIME dependency of the two published CLIs, so losing any + // of these three silently returns it to the unpublished state that would break their next release. + "packages/loopover-contract/package.json", + "packages/loopover-contract/CHANGELOG.md", + ".github/workflows/publish-contract.yml", ]); diff --git a/test/unit/check-contract-package.test.ts b/test/unit/check-contract-package.test.ts new file mode 100644 index 000000000..5930ec46f --- /dev/null +++ b/test/unit/check-contract-package.test.ts @@ -0,0 +1,55 @@ +import { describe, expect, it } from "vitest"; +import { validateContractPackFileList } from "../../scripts/check-contract-package"; +import { CONTRACT_PACKAGE_ALLOWED_FILE_PATTERNS } from "../../scripts/contract-package-allowlist"; +import { MCP_PACKAGE_ALLOWED_FILE_PATTERNS } from "../../scripts/mcp-package-allowlist"; + +// #9654: @loopover/contract becomes publishable, and a published tarball is immutable — so what may ship +// is checked before the publish job ever sees it, against this package's OWN allowlist. + +const clean = () => ""; + +describe("validateContractPackFileList (#9654)", () => { + it("accepts the real shape: emitted modules, declarations, maps, and the metadata files", () => { + const paths = validateContractPackFileList( + ["dist/index.js", "dist/index.d.ts", "dist/index.js.map", "dist/tools.d.ts.map", "package.json", "README.md", "CHANGELOG.md", "LICENSE"], + clean, + ); + expect(paths).toHaveLength(8); + }); + + it("REGRESSION: rejects a file nobody allowlisted, so a stray emit cannot ship unnoticed", () => { + // A plain unexpected file (the forbidden-PATH rule below catches the secret-shaped names separately). + expect(() => validateContractPackFileList(["dist/index.js", "src/index.ts"], clean)).toThrow(/Unexpected file/); + expect(() => validateContractPackFileList(["dist/index.js", "tsconfig.json"], clean)).toThrow(/Unexpected file/); + expect(() => validateContractPackFileList(["dist/index.js", ".npmrc"], clean)).toThrow(/Forbidden file/); + expect(() => validateContractPackFileList(["dist/index.js", "deploy/id_rsa.pem"], clean)).toThrow(/Forbidden file/); + }); + + it("REGRESSION: rejects secret-like CONTENT even in an allowlisted file", () => { + // This fired for real on first run: dist/telemetry.js carried a literal PEM header inside a doc + // comment. Not a leaked credential, but a string that trips every scanner a consumer might run, so it + // was removed at the source rather than excluded here. + expect(() => + validateContractPackFileList(["dist/telemetry.js"], () => "const example = '-----BEGIN RSA PRIVATE KEY-----';"), + ).toThrow(/Secret-like content/); + }); + + it("an empty tarball is a failure, not a pass — a broken build must not publish as 'clean'", () => { + expect(() => validateContractPackFileList([], clean)).toThrow(/empty/); + }); + + it("accepts npm's object-shaped pack entries as well as bare strings", () => { + expect(validateContractPackFileList([{ path: "dist/index.js" }, "package.json"], clean)).toEqual(["dist/index.js", "package.json"]); + }); + + it("REGRESSION: the contract allowlist is its OWN, not a reuse of the MCP one", () => { + // The two packages ship completely different trees; sharing a list would over-permit one and + // under-permit the other, which is how a package starts shipping a file nobody reviewed. + expect(CONTRACT_PACKAGE_ALLOWED_FILE_PATTERNS).not.toBe(MCP_PACKAGE_ALLOWED_FILE_PATTERNS); + // MCP's named CLI entrypoint is not allowlisted here... + expect(CONTRACT_PACKAGE_ALLOWED_FILE_PATTERNS.some((p) => p.test("scripts/gittensor-score-preview.mjs"))).toBe(false); + // ...and the contract's arbitrary dist modules are not allowlisted by MCP's. + expect(MCP_PACKAGE_ALLOWED_FILE_PATTERNS.some((p) => p.test("dist/agent-specs.d.ts"))).toBe(false); + expect(CONTRACT_PACKAGE_ALLOWED_FILE_PATTERNS.some((p) => p.test("dist/agent-specs.d.ts"))).toBe(true); + }); +}); From ea52a06eaaae161b280e29892da2a72cb0d848e5 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Wed, 29 Jul 2026 00:27:13 -0700 Subject: [PATCH 3/5] fix(ci): derive the published set from each workflow's pack line, not any package mention --- scripts/check-publishable-deps.ts | 21 +++++++-- test/unit/check-publishable-deps.test.ts | 58 ++++++++++++++++++++---- 2 files changed, 66 insertions(+), 13 deletions(-) diff --git a/scripts/check-publishable-deps.ts b/scripts/check-publishable-deps.ts index 731195582..faa03f105 100644 --- a/scripts/check-publishable-deps.ts +++ b/scripts/check-publishable-deps.ts @@ -86,14 +86,27 @@ export function findPublishableDepViolations( return violations; } -/** Map `publish-.yml` to the package name it publishes, by reading the workflow's own filter. Derived - * rather than assumed from the filename: `publish-ui-kit.yml` publishes `@loopover/ui-kit`, and a future - * workflow could name its package anything. */ +/** + * Map `publish-.yml` to the package it actually publishes, read from its own `npm pack --workspace` + * line. + * + * Deliberately NOT "any `@loopover/*` mentioned in the file". A publish workflow legitimately references + * OTHER workspace packages in its build steps (`npx turbo run build --filter=@loopover/contract`, + * `npm run build --workspace @loopover/engine`), and treating those as published would silently mark a + * package releasable because something else happens to build it -- masking exactly the violation this + * check exists to catch. The pack line is the definitive signal: a publish workflow packs precisely the + * one package it publishes, and every workflow in this repo has exactly one. + * + * A publish workflow with no recognizable pack line contributes NOTHING rather than falling back to a + * looser match -- an unreadable workflow should make the check stricter, never quietly more permissive. + */ export function publishedPackageNames(workflowFiles: ReadonlyArray<{ name: string; text: string }>): Set { const names = new Set(); for (const file of workflowFiles) { if (!/^publish-.+\.ya?ml$/.test(file.name)) continue; - for (const match of file.text.matchAll(/@loopover\/[a-z0-9-]+/g)) names.add(match[0]); + for (const match of file.text.matchAll(/npm pack --workspace\s+(@[a-z0-9-]+\/[a-z0-9-]+)/g)) { + if (match[1]) names.add(match[1]); + } } return names; } diff --git a/test/unit/check-publishable-deps.test.ts b/test/unit/check-publishable-deps.test.ts index 0c45c0045..eb370f49d 100644 --- a/test/unit/check-publishable-deps.test.ts +++ b/test/unit/check-publishable-deps.test.ts @@ -77,28 +77,68 @@ describe("findPublishableDepViolations (#9749)", () => { }); describe("publishedPackageNames (#9749)", () => { - it("derives the published set from the workflows themselves, not from filenames or a hand-kept list", () => { - // `publish-ui-kit.yml` publishes `@loopover/ui-kit`; a future workflow could name its package anything, - // so the mapping is read from the file rather than inferred from the slug. + it("reads the package a workflow actually PACKS, not the filename slug", () => { + // `publish-ui-kit.yml` publishes `@loopover/ui-kit`; the mapping is read from the file so a future + // workflow can name its package anything. const names = publishedPackageNames([ - { name: "publish-ui-kit.yml", text: "run: npm run test --workspace @loopover/ui-kit" }, - { name: "publish-engine.yml", text: "npm pack --workspace @loopover/engine --json" }, + { name: "publish-ui-kit.yml", text: 'PACK_JSON="$(npm pack --workspace @loopover/ui-kit --json)"' }, + { name: "publish-engine.yml", text: 'npm pack --workspace @loopover/engine --pack-destination "$RUNNER_TEMP" --json' }, ]); expect([...names].sort()).toEqual(["@loopover/engine", "@loopover/ui-kit"]); }); + it("REGRESSION: a package merely BUILT by another workflow is not treated as published", () => { + // Review caught this: deriving from any `@loopover/*` mention meant a publish workflow's own build step + // could silently mark an unpublished package releasable, masking the exact violation this check exists + // to catch. publish-contract.yml really does contain such a line for itself, and publish workflows + // routinely build sibling packages. + const names = publishedPackageNames([ + { + name: "publish-mcp.yml", + text: [ + "npx turbo run build --filter=@loopover/contract", + "npm run build --workspace @loopover/engine", + "run: npm run test --workspace @loopover/ui-kit", + 'PACK_JSON="$(npm pack --workspace @loopover/mcp --json)"', + ].join("\n"), + }, + ]); + // ONLY the packed package. The three built/tested siblings are not published by this workflow. + expect([...names]).toEqual(["@loopover/mcp"]); + }); + + it("a publish workflow with no recognizable pack line contributes NOTHING, never a looser guess", () => { + // An unreadable workflow must make the check stricter, not quietly more permissive. + const names = publishedPackageNames([{ name: "publish-broken.yml", text: "echo @loopover/engine # no pack line" }]); + expect(names.size).toBe(0); + }); + it("ignores workflows that are not publish-*, so a CI file mentioning a package cannot fake it published", () => { const names = publishedPackageNames([ - { name: "ci.yml", text: "npm run build --workspace @loopover/contract" }, - { name: "release-selfhost.yml", text: "@loopover/engine" }, + { name: "ci.yml", text: "npm pack --workspace @loopover/contract" }, + { name: "release-selfhost.yml", text: "npm pack --workspace @loopover/engine" }, ]); expect(names.size).toBe(0); }); - it("accepts both .yml and .yaml, and dedupes repeated mentions within one workflow", () => { + it("accepts both .yml and .yaml, and dedupes a package packed more than once", () => { const names = publishedPackageNames([ - { name: "publish-mcp.yaml", text: "@loopover/mcp ... @loopover/mcp ... @loopover/mcp" }, + { name: "publish-mcp.yaml", text: "npm pack --workspace @loopover/mcp\nnpm pack --workspace @loopover/mcp" }, ]); expect([...names]).toEqual(["@loopover/mcp"]); }); + + it("INVARIANT: every real publish workflow in this repo yields exactly one package", async () => { + // Guards the assumption the hardening rests on. If a future workflow packs two packages, or none, this + // fails here rather than silently changing what the check considers published. + const { readdirSync, readFileSync } = await import("node:fs"); + const dir = ".github/workflows"; + const files = readdirSync(dir) + .filter((name) => /^publish-.+\.ya?ml$/.test(name)) + .map((name) => ({ name, text: readFileSync(`${dir}/${name}`, "utf8") })); + expect(files.length).toBeGreaterThan(0); + for (const file of files) { + expect({ file: file.name, packed: publishedPackageNames([file]).size }).toEqual({ file: file.name, packed: 1 }); + } + }); }); From c68240e3e9cf9780f417cd53e20acdf90603df3c Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Wed, 29 Jul 2026 00:36:54 -0700 Subject: [PATCH 4/5] test(secrets): force the packaged-tarball scanner to keep up with HARD_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. --- scripts/forbidden-content.ts | 20 +++++++++---- test/unit/forbidden-content.test.ts | 46 +++++++++++++++++++++++------ 2 files changed, 52 insertions(+), 14 deletions(-) diff --git a/scripts/forbidden-content.ts b/scripts/forbidden-content.ts index 32392edc5..249e09c30 100644 --- a/scripts/forbidden-content.ts +++ b/scripts/forbidden-content.ts @@ -5,12 +5,22 @@ // the SAME pattern to assert no MCP tool response ever leaks one — importing it here rather than hand-duplicating // the regex keeps every consumer byte-for-byte in sync instead of relying on manual vigilance. // -// The concrete provider-key shapes below are hand-copied (#7433) from the exact regex bodies of the entries in +// The concrete provider-key shapes below are hand-copied (#7433) from the entries in // src/review/secret-patterns.ts's SECRET_PATTERNS that are in its HARD_SECRET_KINDS set (the "near-zero -// false-positive" subset). They are NOT imported directly: this file is a plain `.mjs` run via `node` -// (test:miner-pack / test:mcp-pack), and secret-patterns.ts is TypeScript with no runtime `.js` sibling on this -// path — a runtime `import` of it from node would fail, so the exact bodies are copied per the issue's stated -// fallback. `seed_or_mnemonic` and `bittensor_key` are deliberately NOT included: they are documented in +// false-positive" subset). +// +// WHY STILL A COPY, now that this is a `.ts` file run via `tsx` and could simply import them: the copy is +// deliberately WIDER than canonical on three shapes, and importing would silently NARROW a scanner that +// guards immutable published tarballs. Canonical `github_token`/`github_pat` carry a 20-char floor and +// `private_key_block` requires the `-----` delimiters — sensible for a review lane weighing false positives +// against a human's attention, wrong for a tarball, where a short `ghp_`-shaped fixture or a bare +// `BEGIN RSA PRIVATE KEY` should still stop the release. Two shapes here have no canonical home at all +// (`gts_*`, and the `*_TOKEN=` assignment the review lane treats as advisory-only). +// +// What keeps a copy from rotting is test/unit/forbidden-content.test.ts: it asserts a probe exists for +// EVERY member of HARD_SECRET_KINDS, and that each probe matched by the canonical pattern is matched here +// too. Adding a kind there without mirroring it here fails CI. `seed_or_mnemonic` and `bittensor_key` are +// deliberately NOT included: they are documented in // secret-patterns.ts as weak, false-positive-prone heuristics intentionally excluded from HARD_SECRET_KINDS // (an ordinary `coldkey:` / `hotkey =` line or the word "mnemonic" in Bittensor docs is not a leaked // credential). `jwt` IS in HARD_SECRET_KINDS today and is included here so packaged tarballs get the same diff --git a/test/unit/forbidden-content.test.ts b/test/unit/forbidden-content.test.ts index e0ccf1074..815b9abe7 100644 --- a/test/unit/forbidden-content.test.ts +++ b/test/unit/forbidden-content.test.ts @@ -3,6 +3,7 @@ import { readFileSync } from "node:fs"; import { join } from "node:path"; import { describe, expect, it } from "vitest"; import { FORBIDDEN_CONTENT } from "../../scripts/forbidden-content"; +import { ADVISORY_ONLY_SECRET_KINDS, HARD_SECRET_KINDS, SECRET_PATTERNS } from "../../src/review/secret-patterns"; // forbidden-content.ts calls itself the single source of truth for the packaged secret-shape detector, but // nothing enforced it: check-mcp-package.ts re-declared the regex as its own local constant and the two could @@ -111,17 +112,44 @@ describe("FORBIDDEN_CONTENT covers the concrete provider-key formats (#7433)", ( expect(FORBIDDEN_CONTENT.test(probe)).toBe(true); }); - it("still matches the pre-existing shapes (private-key block, github_pat, gh*, gts, generic assignment)", () => { - expect(FORBIDDEN_CONTENT.test("BEGIN RSA PRIVATE KEY")).toBe(true); - expect(FORBIDDEN_CONTENT.test("github_pat_" + a(20))).toBe(true); - expect(FORBIDDEN_CONTENT.test("ghp_" + a(30))).toBe(true); - expect(FORBIDDEN_CONTENT.test("gts_" + "0".repeat(64))).toBe(true); - expect(FORBIDDEN_CONTENT.test("MY" + "_TOKEN=" + "x")).toBe(true); + // Every remaining HARD_SECRET_KIND, so the probe set below can be checked for COMPLETENESS rather than + // trusted. These four predate #7433 and were previously asserted in prose-y one-off tests. + const LEGACY_PROBES: Array<[string, string]> = [ + ["private_key_block", "-----BEGIN" + " RSA PRIVATE KEY" + "-----"], + ["github_pat", "github_pat_" + a(22)], + ["github_token", "ghp_" + a(30)], + ["jwt", "eyJ" + A(20) + "." + a(20) + "." + a(20)], + ]; + const ALL_PROBES = [...NEW_FORMAT_PROBES, ...LEGACY_PROBES]; + + it("COMPLETENESS: every HARD_SECRET_KIND has a probe here — adding a 17th kind fails until it is mirrored", () => { + // THE GAP THIS CLOSES: FORBIDDEN_CONTENT hand-copies its provider-key bodies out of SECRET_PATTERNS + // (see the header of scripts/forbidden-content.ts for why the copy still exists). Nothing forced the + // copy to keep up. A 17th entry added to HARD_SECRET_KINDS and forgotten here would leave every + // PUBLISHED TARBALL unscanned for that shape while this whole file still passed green -- the exact + // silent-rot failure mode validate-no-hand-written-js guards against with its stale-entry check. + expect(new Set(ALL_PROBES.map(([kind]) => kind))).toEqual(HARD_SECRET_KINDS); }); - it("matches a jwt-shaped value (#8396)", () => { - // jwt is in HARD_SECRET_KINDS; packaged tarballs must catch the same shape. - expect(FORBIDDEN_CONTENT.test("eyJ" + A(20) + "." + a(20) + "." + a(20))).toBe(true); + it.each(ALL_PROBES)("EQUIVALENCE: the canonical %s pattern and FORBIDDEN_CONTENT agree on the same value", (kind, probe) => { + // The copy is allowed to be WIDER than canonical (it is, deliberately, for gh*/github_pat/private-key + // blocks -- a tarball scanner should not require a 20-char floor or the `-----` delimiters). It must + // never be NARROWER: a value the review lane hard-blocks must never sail into a published tarball. + const canonical = SECRET_PATTERNS.find((pattern) => pattern.name === kind); + expect(canonical, `no SECRET_PATTERNS entry named ${kind}`).toBeDefined(); + expect(canonical?.re.test(probe), `probe for ${kind} does not match its own canonical pattern`).toBe(true); + expect(FORBIDDEN_CONTENT.test(probe), `FORBIDDEN_CONTENT is NARROWER than canonical for ${kind}`).toBe(true); + }); + + it("INVARIANT: the two shapes with no canonical home stay covered", () => { + // `gts_*` (a loopover-issued token) and the `*_TOKEN=` env-assignment shape are intentionally NOT in + // SECRET_PATTERNS -- the review lane treats generic assignments as advisory-only (see + // ADVISORY_ONLY_SECRET_KINDS), but a tarball has no human to advise, so here they are hard blocks. This + // pins that asymmetry as deliberate rather than leftover. + expect(FORBIDDEN_CONTENT.test("gts_" + "0".repeat(64))).toBe(true); + expect(FORBIDDEN_CONTENT.test("MY" + "_TOKEN=" + "x")).toBe(true); + expect(SECRET_PATTERNS.some((pattern) => pattern.name === "gts_token")).toBe(false); + expect(ADVISORY_ONLY_SECRET_KINDS.has("generic_secret_assignment")).toBe(true); }); it("does NOT hard-block the deliberately-excluded weak heuristics (seed / bittensor key shapes)", () => { From 3e9fef4658095771864925cd4a02718c0db97be1 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Wed, 29 Jul 2026 00:44:38 -0700 Subject: [PATCH 5/5] refactor(ci): match the pack line by lookbehind so no unreachable guard is needed --- scripts/check-publishable-deps.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/scripts/check-publishable-deps.ts b/scripts/check-publishable-deps.ts index faa03f105..c47428797 100644 --- a/scripts/check-publishable-deps.ts +++ b/scripts/check-publishable-deps.ts @@ -104,9 +104,10 @@ export function publishedPackageNames(workflowFiles: ReadonlyArray<{ name: strin const names = new Set(); for (const file of workflowFiles) { if (!/^publish-.+\.ya?ml$/.test(file.name)) continue; - for (const match of file.text.matchAll(/npm pack --workspace\s+(@[a-z0-9-]+\/[a-z0-9-]+)/g)) { - if (match[1]) names.add(match[1]); - } + // Lookbehind rather than a capture group, so the package name IS the whole match. A captured group + // would be typed `string | undefined` under noUncheckedIndexedAccess, forcing a guard whose false arm + // can never run -- an unreachable branch is exactly the kind of thing that rots into noise. + for (const match of file.text.matchAll(/(?<=npm pack --workspace\s+)@[a-z0-9-]+\/[a-z0-9-]+/g)) names.add(match[0]); } return names; }