diff --git a/.github/workflows/dep-check.yml b/.github/workflows/dep-check.yml index aad7caf..41a6fd8 100644 --- a/.github/workflows/dep-check.yml +++ b/.github/workflows/dep-check.yml @@ -32,7 +32,7 @@ on: TOOL is a semver artifact, so a behaviour change is a version bump somebody reviewed. The pin lives here rather than in eleven callers. type: string - default: '0.9.4' + default: '0.10.0' run-floor-check: description: | Also run the suite against the BOTTOM of every declared sibling range, not only @@ -106,6 +106,23 @@ jobs: corepack prepare "pnpm@${PNPM_VERSION}" --activate - name: Setup Node + # NO `cache:` here, and that is a measurement rather than an omission. + # + # The obvious move is `cache: pnpm`, and it would be dead configuration. This job only runs + # when `base_ref == 'main'` — a pull request, never a push. GitHub lets a run restore a + # cache from its own branch or from the DEFAULT branch, so a cache only ever written on + # release pull requests is one no other run can reach: every leg would be a guaranteed + # miss, paying the save cost forever and restoring nothing. + # + # cloudflare/workers-sdk hit this exact shape and wrote it down: `actions/cache` saves from + # a post step declaring `post-if: success()`, so a job that fails for any reason never + # saves either — and this job has been failing. + # + # What it would be worth: the install steps took 26s and 10s of a 319s run, so ~8%. The two + # changes that matter took the other 92% (first-party floors dropped entirely; the build + # batched, -34%). Doing it properly means `cache/restore` + explicit `cache/save` plus a + # job on `push: [main]` to populate what the pull requests read — new machinery for 8%. + # Recorded so the next person finds the measurement instead of the intuition. uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7.0.0 with: node-version: ${{ inputs.node-version }} @@ -440,19 +457,29 @@ jobs: # workspace dependencies, and nothing else. env: PACKAGES: ${{ join(matrix.run.packages, ' ') }} + # ONE invocation, not one per package. Ten invocations re-plan the task graph ten times and + # rebuild the shared dependencies each round. Measured on theokit-plugins, whose leg claims + # 10 packages, cold cache, two rounds each: + # + # loop (10 invocations) 35.2s, 39.6s + # batched (1 invocation) 23.4s, 24.0s + # + # -34% and -39%, producing the same 10 `dist/` directories. `build-command` falls back to + # the single-package form for any package manager whose multi-package shape is unverified, + # so this is a speed-up where it applies and unchanged everywhere else. run: | set -euo pipefail - for pkg in $PACKAGES; do - cmd=$(npx --yes "@theokit/dep-check@${DEP_CHECK_VERSION}" build-command --root repo --package "$pkg") - if [ -z "$cmd" ]; then - echo "no build script in this repository — nothing to build" - exit 0 - fi - echo "::group::build $pkg" - echo "detected: $cmd" - (cd repo && $cmd) - echo "::endgroup::" - done + args=() + for pkg in $PACKAGES; do args+=(--package "$pkg"); done + cmd=$(npx --yes "@theokit/dep-check@${DEP_CHECK_VERSION}" build-command --root repo "${args[@]}") + if [ -z "$cmd" ]; then + echo "no build script in this repository — nothing to build" + exit 0 + fi + echo "::group::build $PACKAGES" + echo "detected: $cmd" + (cd repo && $cmd) + echo "::endgroup::" - name: Suite, for the packages that claim this floor env: diff --git a/.github/workflows/promotion-gate.yml b/.github/workflows/promotion-gate.yml new file mode 100644 index 0000000..7b421aa --- /dev/null +++ b/.github/workflows/promotion-gate.yml @@ -0,0 +1,132 @@ +# `develop` accepts one thing: the promotion from `workspace`. +# +# Called as: +# +# jobs: +# promotion-gate: +# uses: usetheokit/shared-workflows/.github/workflows/promotion-gate.yml@v1 +# +# WHY A WORKFLOW AND NOT THE HOOK. `rules/git-safety.md` § 1 says `develop` "advances only by +# promoting `workspace`" and to "never merge anything other than `workspace` into it", and +# `hooks/validate-command.sh` blocks it — for a `git merge` typed in a local checkout. Nothing merges +# that way. Promotions land through `gh pr merge`, server-side, where no local hook exists. The rule +# was enforced on the path nobody uses and unenforced on the path everybody uses. +# +# That is not hypothetical, and both numbers below were measured rather than estimated: +# +# - Across the eleven repositories in this organisation that have a `develop` (2026-08-31): +# exactly ONE had a check that looks at where a pull request comes from. +# - `theokit-sdk` had accumulated 48 merged `main → develop` pull requests through the gap — +# #193 on 2026-08-11 through #493 on 2026-08-31 — against a rule that forbids them and a hook +# that blocks them, because the hook watched the path nobody uses. +# +# The 48 was first reported as thirty, which is `gh pr list`'s default page size and not a count. +# A total landing exactly on the default limit is a total nobody counted. The difference is not +# pedantic: thirty over twenty days reads as history, 48 reads as current practice. +# +# It does NOT replace the hook. The hook guards the local path and this guards the server path; a +# repository with only one of them is guarded on one side. `git-safety.md` § 1 already distinguishes +# the hook (origin) from branch protection (review) — this is the third column that was missing. +# +# REQUIRED, or it is decoration. An advisory check is a green tick nobody reads; what makes it a +# gate is branch protection listing it. Adding it as required BEFORE it has ever reported would +# block every pull request on a context that never arrives — so it is armed after its first green +# run in a repository, never in the same change. +# +# NO `paths:` FILTER in the caller, deliberately. This is marked required in branch protection, and +# a required check that does not run on some pull requests deadlocks them permanently. +# +# WHAT IT DOES NOT COVER. A push directly to `develop`, which branch protection already refuses, and +# a merge performed by someone with permission to bypass protection. It answers one question — did +# this work originate on `workspace` — which is the question the rule asks. +# +# WHY IT LIVES HERE. Measured 2026-09-05 across the ten consumers of this repository: nine held +# byte-identical copies of this gate and one held an improved variant, so a fix reached one +# repository at a time and the improvements never travelled. The `@v1` ref is what makes a change +# to the policy reach every caller; that is the same argument written at the top of `dep-check.yml`, +# with the same limit — the POLICY moves by ref, and anything versioned moves by a bump somebody +# reviewed. +# +# WHAT WAS DELIBERATELY NOT CARRIED OVER. The `theokit-sdk` variant declared a `concurrency` group +# with `cancel-in-progress`. This job is two string comparisons with no checkout and finishes in +# seconds, so cancelling a superseded run reclaims a few seconds of a runner, and GitHub already +# reports the newest run's conclusion as the check status — the stale-verdict problem the setting +# usually solves does not arise here. It is left out rather than copied, so that nobody reads its +# absence as an oversight. `timeout-minutes` below WAS carried over: it guards a different failure +# (a hang holding a runner for the six-hour default), which no other mechanism covers. +name: Promotion gate + +on: + workflow_call: + +permissions: + contents: read + +jobs: + head-must-be-workspace: + name: develop accepts only workspace + runs-on: ubuntu-latest + # Two string comparisons and no checkout; it has never taken more than a few seconds. A tight + # ceiling turns a hang into a fast, obvious failure instead of a runner held for GitHub's + # six-hour default. + timeout-minutes: 5 + steps: + - name: Refuse a head that is not workspace + env: + # Through the environment, never `${{ }}` inside `run:`. On a pull request from a fork the + # head branch name is text the author chose, so interpolating it into the script would put + # an attacker's string where the shell parses commands. + HEAD_REF: ${{ github.event.pull_request.head.ref }} + # The branch NAME is not enough. The head ref on a fork pull request is the branch name + # inside THE FORK, so anyone may fork a public repository, name a branch `workspace`, and + # satisfy a name-only check. The promotion comes from this repository's own `workspace` or + # it is not the promotion. + HEAD_REPO: ${{ github.event.pull_request.head.repo.full_name }} + BASE_REPO: ${{ github.repository }} + BASE_REF: ${{ github.event.pull_request.base.ref }} + run: | + set -euo pipefail + + echo "head: ${HEAD_REPO}:${HEAD_REF} -> base: ${BASE_REPO}:${BASE_REF}" + + if [ "${HEAD_REPO}" != "${BASE_REPO}" ]; then + echo "::error::a fork cannot promote into develop. The head is '${HEAD_REPO}', not '${BASE_REPO}'." + echo + echo "Work originates on this repository's own workspace branch. A branch named" + echo "'workspace' in a fork is a different branch that happens to share a name." + exit 1 + fi + + if [ "${HEAD_REF}" = "workspace" ]; then + echo "head is this repository's workspace — the promotion the rule allows" + exit 0 + fi + + # The changesets bot opens its OWN pull request, named by `changesets/action` and never by + # a human. Where a repository's release runs from `develop`, that pull request targets + # `develop` and lands here. It is not the promotion this gate exists to guard + # (`git-safety.md` § 1 is about where WORK originates); it is generated tooling output + # under a different contract. Exempted by EXACT name, not a prefix match, and only + # reachable after the fork check above already confirmed HEAD_REPO == BASE_REPO — a fork + # cannot forge this repository's own bot branch. + # + # Found 2026-09-03 (usetheokit/theokit-sdk#535): every Version Packages pull request + # against develop failed this check unconditionally, because no exemption existed. + # GitHub's branch protection refused even an admin override — the only real fix was here. + # + # Harmless where it is unused: measured 2026-09-05, every consumer releases from `main`, + # so the bot's branch is `changeset-release/main` and never reaches this gate. The + # exemption costs those repositories nothing and spares the next one the same deadlock. + if [ "${HEAD_REF}" = "changeset-release/develop" ]; then + echo "head is the changesets bot's own branch — generated tooling output, not a promotion" + exit 0 + fi + + echo "::error::'${HEAD_REF}' cannot be merged into develop." + echo + echo "rules/git-safety.md § 1: develop advances ONLY by promoting workspace, and nothing" + echo "other than workspace may be merged into it. Work originates on workspace; develop" + echo "integrates it." + echo + echo "Land this on workspace first, then open workspace -> develop." + exit 1 diff --git a/CHANGELOG.md b/CHANGELOG.md index 76b7c53..35e91c3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,34 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- **`promotion-gate.yml` is now a reusable workflow, so the gate that protects `develop` has one + home instead of ten (#51).** Measured 2026-09-05 across the ten consumers of this repository: + nine held **byte-identical** copies of a 74-line gate (identical after stripping comments and + blank lines) and one, `theokit-sdk`, held a variant that had learned two things the other nine + never received. + + That is the shape this repository exists to remove. A fix written into one copy reaches one + repository, and the improvements the tenth made stayed there for as long as nobody compared. + + The shared version is the **union**, not either copy: + + | carried from | what | + |---|---| + | the nine | the multi-line diagnostics that cite `rules/git-safety.md` § 1 and tell the author what to do next | + | `theokit-sdk` | `timeout-minutes`, and the exemption for the changesets bot's own `changeset-release/develop` branch | + + The changesets exemption is what `usetheokit/theokit-sdk#535` cost on 2026-09-03: every Version + Packages pull request against `develop` failed the gate unconditionally, and branch protection + refused even an admin override. It is harmless where unused — measured, every consumer releases + from `main`, so the bot's branch is `changeset-release/main` and never reaches this gate — and it + spares the next repository that moves its release the same deadlock. + + `concurrency` was deliberately **not** carried over, and the workflow says so rather than leaving + its absence to read as an oversight: the job is two string comparisons with no checkout, and + GitHub already reports the newest run's conclusion as the check status. + - **A preview that falls back to publishing everything now says it is not installable (#48).** The scoping added in `#46`/`#47` leaves one case open: a commit that touches no package directory — a CI change, a docs change, a root config change. There the enumeration publishes the whole set in diff --git a/actions/setup/action.yml b/actions/setup/action.yml index 8fa4115..777451e 100644 --- a/actions/setup/action.yml +++ b/actions/setup/action.yml @@ -94,4 +94,21 @@ runs: EXTRA_ARGS: ${{ inputs.install-args }} # Frozen: CI installs what the lockfile says or fails. A non-frozen install in CI resolves # something the committed lockfile does not describe, and then the suite passes about that. + # + # NO `PNPM_CONFIG_TRUST_LOCKFILE`, and that is a measurement rather than an omission. + # + # vitest-dev/vitest sets it repo-wide (`ci.yml:23-26`) and documents ~15-20s saved per job by + # skipping pnpm's per-install supply-chain verification. Measured here on theokit-sdk, + # pnpm 10.34.1, two runs each against a warm store: + # + # default (verification on) 2.78s, 2.54s + # TRUST_LOCKFILE=true 2.52s, 2.54s + # + # Nothing. Their saving is real and ours is not — most plausibly because their number comes + # from a colder store with more packages to verify, and every job here restores a warm one + # through the `cache: pnpm` above. + # + # Recorded rather than left silent so nobody re-derives it from the same blog-shaped + # intuition: this trades a supply-chain check for zero measured seconds. Worth re-testing if + # the cache stops hitting, and only against a measurement. run: pnpm install --frozen-lockfile ${EXTRA_ARGS} diff --git a/packages/dep-check/index.mjs b/packages/dep-check/index.mjs index 40e172d..d952f89 100755 --- a/packages/dep-check/index.mjs +++ b/packages/dep-check/index.mjs @@ -21,10 +21,10 @@ * none of the other three mean anything either. */ import { parseArgs } from "node:util"; -import { ceilingDrift, consumersLeftBehind, groupUntestedFloors, installedDrift, isSibling, peerInstallSpecs, pinnableSiblings, rangeFloor, sharedFloor, unpublishedSiblings, unpublishedWorkspaceVersions, untestedFloors } from "./src/checks.mjs"; +import { ceilingDrift, consumersLeftBehind, floorsInOwnWorkspace, groupUntestedFloors, installedDrift, isSibling, peerInstallSpecs, pinnableSiblings, rangeFloor, sharedFloor, unpublishedSiblings, unpublishedWorkspaceVersions, untestedFloors } from "./src/checks.mjs"; import { findPublishablePackages, resolveInstalledVersion, siblingReferences } from "./src/ecosystem.mjs"; import { consumersOf, discoverEcosystemPackages, latestVersion, packument, publishedVersions } from "./src/registry.mjs"; -import { detectBuildScript, detectPackageManager, pinOverrides } from "./src/package-manager.mjs"; +import { batchedWithDeps, detectBuildScript, detectPackageManager, pinOverrides } from "./src/package-manager.mjs"; import { installFromTarball } from "./src/tarball.mjs"; const { values: flags, positionals } = parseArgs({ @@ -34,7 +34,11 @@ const { values: flags, positionals } = parseArgs({ json: { type: "boolean", default: false }, markdown: { type: "boolean", default: false }, unlocked: { type: "boolean", default: false }, - package: { type: "string" }, + // Repeatable: the floor leg builds every package that claims the floor, and one invocation + // with N filters is measurably faster than N invocations (theokit-plugins, 10 packages, cold: + // 35.2s -> 23.4s). Without `multiple`, parseArgs keeps only the LAST occurrence, so a repeated + // flag would silently build one package and report the whole leg green. + package: { type: "string", multiple: true }, help: { type: "boolean", short: "h", default: false }, }, }); @@ -444,7 +448,6 @@ const MAX_FLOOR_RUNS = 20; */ async function commandFloorMatrix(root) { await lowestFloors(root); - const runs = groupUntestedFloors(lowestFloors.lastUntested ?? []); // #27 — the leg reinstalls from the registry, so a workspace version the registry does not have // yet turns every run into ERR_PNPM_NO_MATCHING_VERSION. An empty matrix is the honest answer; @@ -454,6 +457,24 @@ async function commandFloorMatrix(root) { name: p.manifest.name, version: p.manifest.version, })); + + // usetheokit/theokit-sdk#569 — drop the floors whose dep this workspace publishes. Overriding + // one of our own packages installs its published tarball beside the workspace copy (452 MB -> + // 4,432 MB peak, measured) and asks a question the leg cannot answer anyway. + const exercisable = floorsInOwnWorkspace({ + untested: lowestFloors.lastUntested ?? [], + workspace: workspace.map((p) => p.name), + }); + const dropped = (lowestFloors.lastUntested ?? []).length - exercisable.length; + if (dropped > 0) { + // Never silent: a floor nobody exercises must not read as a floor that passed. + console.error( + `::notice::${dropped} floor gap(s) skipped — the dep is published by this workspace, so ` + + "pinning it installs its own tarball beside the local copy. A consumer never resolves it " + + "that way, so the leg could not answer the question it was asked.", + ); + } + const runs = groupUntestedFloors(exercisable); const published = {}; for (const pkg of workspace) published[pkg.name] = await publishedVersions(pkg.name); const blocked = unpublishedWorkspaceVersions({ workspace, published }); @@ -535,7 +556,7 @@ function commandRunCommand(root, script) { // `--package` narrows it to one workspace member, which the per-package floor leg needs: it // installs a sibling at ONE package's declared floor, and running the whole workspace there // would fail packages whose own ranges exclude that version — the defect #4 was. - const base = flags.package ? detected.filtered(flags.package) : detected.run; + const base = flags.package?.length ? detected.filtered(flags.package[0]) : detected.run; console.log([...base, script || "test"].join(" ")); return 0; } @@ -580,7 +601,14 @@ function commandBuildCommand(root) { // measured on theokit-sdk, `pnpm build` at `@theokit/sdk@4.4.1` failed on `sdk-cache`, which // declares `>=4.54.0` and has no business being compiled there. That is the defect #4 was, // reintroduced one level down. - const base = flags.package ? detected.filteredWithDeps(flags.package) : detected.run; + // `--package` may be repeated. One invocation for the whole leg plans the task graph once and + // builds each shared dependency once; the per-package loop it replaces did both N times. + // Measured on theokit-plugins (10 packages, cold): 35.2s/39.6s sequential vs 23.4s/24.0s + // batched. `batchedWithDeps` returns null for managers whose multi-package form is unverified, + // and the single-package path is what it always was. + const packages = flags.package === undefined ? [] : [flags.package].flat(); + const batched = packages.length > 1 ? batchedWithDeps(detected, packages) : null; + const base = batched ?? (packages.length ? detected.filteredWithDeps(packages[0]) : detected.run); if (script) console.log([...base, script].join(" ")); return 0; } diff --git a/packages/dep-check/package.json b/packages/dep-check/package.json index b098b7e..025a7f8 100644 --- a/packages/dep-check/package.json +++ b/packages/dep-check/package.json @@ -1,6 +1,6 @@ { "name": "@theokit/dep-check", - "version": "0.9.4", + "version": "0.10.0", "description": "The ecosystem dependency gate: does a package's declared range still describe the sibling it ships against? Four checks, kept apart by what they need to answer and therefore by whether they may fail a build.", "type": "module", "engines": { diff --git a/packages/dep-check/src/checks.mjs b/packages/dep-check/src/checks.mjs index d840b3e..15a5d66 100644 --- a/packages/dep-check/src/checks.mjs +++ b/packages/dep-check/src/checks.mjs @@ -184,6 +184,37 @@ export function unpublishedWorkspaceVersions({ workspace, published }) { * Sorted, because an unstable matrix makes one failing job impossible to compare against the same * job yesterday. */ +/** + * The floors from `untestedFloors` that this repository's floor leg can actually exercise. + * + * The leg pins the dep under the package manager's override field and reinstalls. When the dep is + * a package of THIS workspace, that override rewrites its spec into a plain semver range — which + * destroys the `workspace:` protocol guarantee, since pnpm "will refuse to resolve to anything + * other than a local workspace package" only while that protocol is in the spec. With + * `linkWorkspacePackages` defaulting to false, the range then resolves from the registry and the + * published tarball is installed BESIDE the workspace copy. + * + * Measured on usetheokit/theokit-sdk#569, same checkout and machine: peak RSS 452 MB without the + * override, 4,432 MB with it. `tsup`'s DTS worker walks 14 MB of published `.d.ts` instead of the + * workspace source, the runner OOMs, and the release pull request is blocked. + * + * It is also the wrong question. A floor is a claim about what a CONSUMER resolves, and a consumer + * has never had this workspace's copy — so the leg was validating a package against its own + * published output, which is a different thing from what it claims to check. + * + * WHY THIS AND NOT `isSibling`. Excluding every ecosystem sibling would gut the check: its primary + * case is `theokit-plugins`, where fourteen packages declare `theokit >=0.50.1` and `theokit` is a + * DIFFERENT repository. Overriding that duplicates nothing and the leg works. The predicate is + * "the dep is published by this very workspace", not "the dep is ours". + * + * An empty `workspace` fails OPEN — a caller that could not read the manifests must not silently + * drop every floor and report a green check that exercised nothing. + */ +export function floorsInOwnWorkspace({ untested, workspace }) { + const own = new Set(workspace ?? []); + return untested.filter((gap) => !own.has(gap.dep)); +} + export function groupUntestedFloors(untested) { const byFloor = new Map(); for (const gap of untested) { diff --git a/packages/dep-check/src/package-manager.mjs b/packages/dep-check/src/package-manager.mjs index 1ff616f..4caddf4 100644 --- a/packages/dep-check/src/package-manager.mjs +++ b/packages/dep-check/src/package-manager.mjs @@ -23,6 +23,37 @@ const LOCKFILES = [ { file: "yarn.lock", manager: "yarn", install: ["yarn", "install", "--immutable"], unlocked: ["yarn", "install"], run: ["yarn", "run"], filtered: (pkg) => ["yarn", "workspace", pkg, "run"], filteredWithDeps: (pkg) => ["yarn", "workspace", pkg, "run"], overridesPath: ["resolutions"] }, ]; +/** + * One invocation that builds several packages and their workspace dependencies, or `null` when + * this package manager has no verified multi-package form. + * + * The floor leg builds every package that claims the floor. Doing it one `--filter` at a time + * re-plans the task graph once per package and rebuilds the shared dependencies each round. + * + * Measured on `theokit-plugins`, whose leg claims 10 packages, cold cache, two rounds each: + * + * sequential (10 invocations) 35.2s, 39.6s + * batched (1 invocation) 23.4s, 24.0s + * + * -34% and -39%, and the batched run produces the same 10 `dist/` directories. + * + * NULL RATHER THAN A GUESS for npm and yarn. npm has no `...` equivalent — its `filteredWithDeps` + * is already the same as `filtered` — and yarn's `yarn workspace run` takes exactly one + * name, so batching there means `workspaces foreach`, a different command with different + * semantics that nobody here has measured. The caller keeps its loop, which is correct if slower. + * A batch that silently built the wrong set would be worse than the time it saved. + * + * An empty list is `null` too: `pnpm run build` with no filter builds the WHOLE workspace, which + * is the defect the per-package filter exists to prevent — packages whose own ranges exclude this + * floor fail on it. + */ +export function batchedWithDeps(manager, packages) { + if (!manager || !packages?.length) return null; + if (manager.file !== "pnpm-lock.yaml") return null; + const filters = packages.flatMap((pkg) => ["--filter", `${pkg}...`]); + return ["pnpm", ...filters, "run"]; +} + /** Null when no lockfile is present — the caller decides whether that is an error. */ export function detectPackageManager(repoRoot) { return LOCKFILES.find((candidate) => existsSync(join(repoRoot, candidate.file))) ?? null; diff --git a/packages/dep-check/test/checks.test.mjs b/packages/dep-check/test/checks.test.mjs index d6457c7..3ecb2fd 100644 --- a/packages/dep-check/test/checks.test.mjs +++ b/packages/dep-check/test/checks.test.mjs @@ -7,6 +7,7 @@ import { isSibling, rangeFloor, groupUntestedFloors, + floorsInOwnWorkspace, unpublishedWorkspaceVersions, pinnableSiblings, sharedFloor, @@ -609,3 +610,61 @@ describe("unpublishedWorkspaceVersions — the reason a floor leg cannot ask the expect(blocked.map((b) => b.name)).toEqual(["@theokit/http", "theokit"]); }); }); + +describe("floorsInOwnWorkspace — a floor the leg cannot exercise because it would duplicate the package", () => { + it("test_drops_a_floor_on_a_package_this_very_workspace_publishes", () => { + // usetheokit/theokit-sdk#569. The leg pins the dep under `pnpm.overrides` and reinstalls. + // When the dep is ALSO a package in this workspace, the override rewrites its spec into a + // plain semver range, destroying the `workspace:` protocol guarantee — pnpm "will refuse to + // resolve to anything other than a local workspace package" only while that protocol is in + // the spec. With `linkWorkspacePackages` defaulting to false, the range resolves from the + // registry and the published tarball lands beside the workspace copy. + // + // Measured on theokit-sdk, same checkout and machine: peak RSS 452 MB without the override, + // 4,432 MB with it. tsup's DTS worker walks 14 MB of published `.d.ts` instead of the + // workspace source, and the runner OOMs. + // + // It is also the wrong question: a floor is a claim about what a CONSUMER resolves, and a + // consumer never has this workspace's copy. + const untested = [ + { pkg: "@theokit/sdk-tools", dep: "@theokit/sdk", range: ">=5.0.0-next.1", claims: "5.0.0", tested: "5.0.1" }, + { pkg: "@theokit/sdk-tools", dep: "vite", range: ">=5.0.0", claims: "5.0.0", tested: "5.4.0" }, + ]; + + const out = floorsInOwnWorkspace({ untested, workspace: ["@theokit/sdk", "@theokit/sdk-tools"] }); + + expect(out.map((r) => r.dep)).toEqual(["vite"]); + }); + + it("test_keeps_a_sibling_floor_that_lives_in_a_DIFFERENT_repository", () => { + // The primary case this whole check exists for: `theokit-plugins` has fourteen packages on + // `theokit >=0.50.1`, and `theokit` is a different repository. Overriding it duplicates + // nothing, so the leg works and must keep running. Excluding every sibling would have gutted + // the feature to fix the workspace-self case. + const untested = [ + { pkg: "@theokit/plugin-a", dep: "theokit", range: ">=0.50.1", claims: "0.50.1", tested: "0.57.0" }, + ]; + + const out = floorsInOwnWorkspace({ untested, workspace: ["@theokit/plugin-a", "@theokit/plugin-b"] }); + + expect(out.map((r) => r.dep)).toEqual(["theokit"]); + }); + + it("test_an_all_self_referential_input_yields_nothing_rather_than_a_leg_that_OOMs", () => { + const untested = [ + { pkg: "@theokit/cli", dep: "@theokit/sdk", range: ">=5.0.0", claims: "5.0.0", tested: "5.0.1" }, + ]; + + expect(floorsInOwnWorkspace({ untested, workspace: ["@theokit/sdk", "@theokit/cli"] })).toEqual([]); + }); + + it("test_an_empty_workspace_list_changes_nothing", () => { + // Fail open, not closed: a caller that could not read the workspace must not silently drop + // every floor and report a green check that exercised nothing. + const untested = [ + { pkg: "@theokit/cli", dep: "@theokit/sdk", range: ">=5.0.0", claims: "5.0.0", tested: "5.0.1" }, + ]; + + expect(floorsInOwnWorkspace({ untested, workspace: [] })).toHaveLength(1); + }); +}); diff --git a/packages/dep-check/test/package-manager.test.mjs b/packages/dep-check/test/package-manager.test.mjs index d8f06d9..056d074 100644 --- a/packages/dep-check/test/package-manager.test.mjs +++ b/packages/dep-check/test/package-manager.test.mjs @@ -2,7 +2,7 @@ import { mkdtempSync, readFileSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { describe, expect, it } from "vitest"; -import { detectPackageManager, pinOverrides } from "../src/package-manager.mjs"; +import { detectPackageManager, pinOverrides, batchedWithDeps } from "../src/package-manager.mjs"; function scratch({ lockfile, manifest = { name: "x" } }) { const root = mkdtempSync(join(tmpdir(), "dep-check-pm-")); @@ -71,3 +71,39 @@ describe("pinOverrides", () => { expect(() => pinOverrides(scratch({ lockfile: null }), { a: "1" })).toThrow(/no lockfile/); }); }); + +describe("batchedWithDeps — one invocation for a leg that claims several packages", () => { + it("test_pnpm_repeats_the_filter_flag_so_the_task_graph_is_planned_once", () => { + // Measured on theokit-plugins, whose floor leg claims 10 packages, cold cache, two rounds: + // + // sequential (10 invocations) 35.2s, 39.6s + // batched (1 invocation) 23.4s, 24.0s + // + // -34% and -39%. Ten invocations re-plan the graph ten times and rebuild the shared + // dependencies each round; one invocation plans once and builds each dependency once. + const pnpm = { file: "pnpm-lock.yaml" }; + expect(batchedWithDeps(pnpm, ["@theokit/a", "@theokit/b"])).toEqual([ + "pnpm", "--filter", "@theokit/a...", "--filter", "@theokit/b...", "run", + ]); + }); + + it("test_a_single_package_batches_to_the_same_thing_the_loop_would_run", () => { + const pnpm = { file: "pnpm-lock.yaml" }; + expect(batchedWithDeps(pnpm, ["@theokit/a"])).toEqual(["pnpm", "--filter", "@theokit/a...", "run"]); + }); + + it("test_returns_null_for_a_manager_whose_multi_package_form_was_not_verified", () => { + // npm has no `...` equivalent at all, and yarn's `yarn workspace run` takes exactly one + // name — batching there needs `workspaces foreach`, a different command with different + // semantics. Returning null rather than guessing keeps the caller on the loop it already has, + // which is correct if slower. A wrong batch would silently build the wrong set. + expect(batchedWithDeps({ file: "package-lock.json" }, ["a", "b"])).toBeNull(); + expect(batchedWithDeps({ file: "yarn.lock" }, ["a", "b"])).toBeNull(); + }); + + it("test_an_empty_package_list_is_null_not_a_command_that_builds_everything", () => { + // `pnpm run build` with no filter builds the WHOLE workspace, which is the defect the + // per-package filter exists to prevent: packages whose own ranges exclude this floor fail. + expect(batchedWithDeps({ file: "pnpm-lock.yaml" }, [])).toBeNull(); + }); +});