From c286b8187b98efb4471714ef5942bd12b37bd862 Mon Sep 17 00:00:00 2001 From: paulohenriquevn Date: Fri, 4 Sep 2026 20:25:31 -0300 Subject: [PATCH 1/6] fix(dep-check): drop floor legs whose dep this workspace publishes usetheokit/theokit-sdk#569. The floor 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 -- 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 is installed BESIDE the local copy. Measured on theokit-sdk, same checkout and machine: without the override 452 MB peak, 6.6s with it 4,432 MB peak, OOM on the runner 9.8x. tsup's DTS worker walks 14 MB of published .d.ts instead of the workspace source. It blocked the 5.0.1 release twice before anyone looked at why. 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 -- so the leg was validating a package against its own published output. WHY NOT isSibling. My first attempt excluded every ecosystem sibling and broke two existing tests, which was the code telling me I was deleting the feature rather than fixing it: 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 must keep running. The predicate is 'the dep is published by this very workspace', not 'the dep is ours'. Measured across the ecosystem after the change: theokit 1 leg kept, 0 skipped theokit-plugins 1 leg kept (theokit, external), 1 skipped theokit-gateways 0 kept, 1 skipped theokit-tui unchanged theokit-plugins is the proof: the leg the docblock names as the reason this check exists survives. An empty workspace list fails OPEN -- a caller that could not read the manifests must not silently drop every floor and report a green check that ran nothing. Every dropped gap is announced via ::notice::, never silently. The uv and Cargo ecosystems draw this same line: --resolution lowest-direct and -Z direct-minimal-versions lower the direct edges you do not control. --- packages/dep-check/index.mjs | 21 ++++++++- packages/dep-check/src/checks.mjs | 31 +++++++++++++ packages/dep-check/test/checks.test.mjs | 59 +++++++++++++++++++++++++ 3 files changed, 109 insertions(+), 2 deletions(-) diff --git a/packages/dep-check/index.mjs b/packages/dep-check/index.mjs index 40e172d..14c2064 100755 --- a/packages/dep-check/index.mjs +++ b/packages/dep-check/index.mjs @@ -21,7 +21,7 @@ * 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"; @@ -444,7 +444,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 +453,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 }); 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/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); + }); +}); From 594afeeeee5b9bcd7fa32d0db042a0f7595587af Mon Sep 17 00:00:00 2001 From: paulohenriquevn Date: Fri, 4 Sep 2026 20:26:26 -0300 Subject: [PATCH 2/6] chore(release): dep-check 0.9.5 Bumps the manifest and the pin in dep-check.yml together, which the release gate requires -- 0.5.0 and 0.2.0 each published green while the pin stayed behind, so no consumer received the change. Carries the fix for usetheokit/theokit-sdk#569: floor legs whose dep this workspace publishes are dropped, because pinning one installs its own tarball beside the local copy (452 MB -> 4,432 MB peak, measured) and asks a question a consumer never asks. Eleven repositories consume this gate. Moving v1 is the separate step. --- .github/workflows/dep-check.yml | 2 +- packages/dep-check/package.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/dep-check.yml b/.github/workflows/dep-check.yml index aad7caf..7b73ef3 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.9.5' run-floor-check: description: | Also run the suite against the BOTTOM of every declared sibling range, not only diff --git a/packages/dep-check/package.json b/packages/dep-check/package.json index b098b7e..b48d501 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.9.5", "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": { From 8fda0a89b6f9463842a25635b86f0f741be2d980 Mon Sep 17 00:00:00 2001 From: paulohenriquevn Date: Fri, 4 Sep 2026 20:28:37 -0300 Subject: [PATCH 3/6] docs(setup): record that PNPM_CONFIG_TRUST_LOCKFILE saves nothing here vitest-dev/vitest sets it repo-wide and documents ~15-20s per job -- the only hard number anyone published for an install optimisation. Measured on theokit-sdk, pnpm 10.34.1, two runs each against a warm store: default 2.78s, 2.54s TRUST_LOCKFILE 2.52s, 2.54s Nothing. Their saving is real and ours is not, most plausibly because every job here restores a warm store through the cache: pnpm above. Recorded rather than left silent, following the precedent of the turbo-cache note in theokit-sdk's ci.yml: configuration that does nothing is debt wearing an optimisation's clothes, and the next person to read the vitest number should find this instead of re-deriving it. It would have traded a supply-chain check for zero measured seconds. --- actions/setup/action.yml | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) 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} From 27b46c71e0cf3ab0a929d298d3dcb25677c2f29a Mon Sep 17 00:00:00 2001 From: paulohenriquevn Date: Fri, 4 Sep 2026 20:37:21 -0300 Subject: [PATCH 4/6] perf(dep-check): build a floor leg in one invocation, not one per package The 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 (dist and turbo cleared), 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 and the same 20 build successes. Two things this deliberately does NOT do: - batchedWithDeps returns null for npm and yarn rather than guessing. npm has no '...' equivalent at all, and yarn's 'yarn workspace run' takes exactly one name -- batching there needs 'workspaces foreach', different semantics nobody here has measured. Those callers keep the loop, correct if slower. - an empty package list returns null, because 'pnpm run build' with no filter builds the WHOLE workspace, which is the defect the per-package filter exists to prevent. --package is now repeatable, and that needed parseArgs 'multiple: true'. Without it the parser keeps only the LAST occurrence, so the repeated flag would have silently built one package and reported the whole leg green -- caught by running the command with three packages and reading the output, not by the tests. Bumps to 0.10.0 with the workflow pin, together, as the release gate requires. --- .github/workflows/dep-check.yml | 34 +++++++++++------ packages/dep-check/index.mjs | 19 ++++++++-- packages/dep-check/package.json | 2 +- packages/dep-check/src/package-manager.mjs | 31 +++++++++++++++ .../dep-check/test/package-manager.test.mjs | 38 ++++++++++++++++++- 5 files changed, 106 insertions(+), 18 deletions(-) diff --git a/.github/workflows/dep-check.yml b/.github/workflows/dep-check.yml index 7b73ef3..d17ecc2 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.5' + default: '0.10.0' run-floor-check: description: | Also run the suite against the BOTTOM of every declared sibling range, not only @@ -440,19 +440,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/packages/dep-check/index.mjs b/packages/dep-check/index.mjs index 14c2064..d952f89 100755 --- a/packages/dep-check/index.mjs +++ b/packages/dep-check/index.mjs @@ -24,7 +24,7 @@ import { parseArgs } from "node:util"; 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 }, }, }); @@ -552,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; } @@ -597,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 b48d501..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.5", + "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/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/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(); + }); +}); From be5720630860f655065e82981b496574a6d982eb Mon Sep 17 00:00:00 2001 From: paulohenriquevn Date: Fri, 4 Sep 2026 20:41:19 -0300 Subject: [PATCH 5/6] docs(dep-check): record why the floor job has no cache The obvious move is `cache: pnpm` on its Setup Node, and it would be dead configuration. This job only runs when base_ref == 'main' -- a pull request, never a push. A run restores 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 a guaranteed miss, paying the save cost forever. cloudflare/workers-sdk hit this exact shape and wrote it down: actions/cache saves from a post step declaring post-if: success(), so a failing job never saves either -- and this job has been failing. What it was worth, measured on run 101194428887: install 26s and reinstall 10s of a 319s run, about 8%. The two changes that matter took the other 92%. Doing it properly needs cache/restore + explicit cache/save plus a job on push: [main] to populate what the pull requests read. New machinery for 8%, recorded rather than built. --- .github/workflows/dep-check.yml | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/.github/workflows/dep-check.yml b/.github/workflows/dep-check.yml index d17ecc2..41a6fd8 100644 --- a/.github/workflows/dep-check.yml +++ b/.github/workflows/dep-check.yml @@ -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 }} From 23b3cd16ccb8a3d7f47f3082cb25a35a75f69f3f Mon Sep 17 00:00:00 2001 From: paulohenriquevn Date: Fri, 4 Sep 2026 21:20:11 -0300 Subject: [PATCH 6/6] feat(promotion-gate): share the develop gate, nine identical copies become one Measured 2026-09-05 across the ten consumers: nine held byte-identical copies of the 74-line gate and one held a variant that had learned two things the others never received. A fix in one copy reached one repository. The shared version is the union of both: the nine contributed the diagnostics that cite git-safety.md and name the next step, theokit-sdk contributed timeout-minutes and the changesets-bot exemption that cost #535 on 2026-09-03. concurrency was deliberately not carried over, and the file says why, so its absence does not read as an oversight. actionlint and zizmor clean, both verified against a positive control. --- .github/workflows/promotion-gate.yml | 132 +++++++++++++++++++++++++++ CHANGELOG.md | 28 ++++++ 2 files changed, 160 insertions(+) create mode 100644 .github/workflows/promotion-gate.yml 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