fix(dep-check): substitute a sibling whose contract moved without its version (usetheokit/theokit#659)

CHANGELOG.md

@@ -8,6 +8,30 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+
+### Fixed
+
+- **The install leg substitutes a sibling whose contract moved without its version moving
+  (usetheokit/theokit#659).** Check D skipped any sibling whose version number the registry already
+  had. That answers "does this version exist?", which is a different question from "is the published
+  copy the one this workspace is about to publish?".
+
+  A release that widens a peer range across two workspace packages moves the contract and not the
+  version, because changesets bumps at version time and not at pull-request time. So the leg packed
+  tomorrow's package, resolved yesterday's sibling from npm, and reported two copies of a peer — a
+  pair publication never produces. It is a required check, so the release could not merge, and no
+  ordering of the old design avoided it: bumping first makes the packed tarball ask for a version npm
+  does not have, and the install answers ETARGET instead.
+
+  A sibling now also earns its local tarball when the registry serves its version with different
+  `dependencies`, `peerDependencies` or `optionalDependencies`. The narrowness is deliberate — an
+  identical contract is still installed from the registry, which is what makes the check test what a
+  consumer resolves. Absent evidence answers "unchanged": a registry that returns no manifest must
+  not read as "the contract moved", or every sibling gets a local tarball and the leg tests nothing.
+
+  Each substitution now says WHICH gap it filled. The single note claimed the registry did not have
+  the version, which is false for this case, and a note that names the wrong reason is worse than
+  none because it is read and believed.
 ### Added
 
 - **The back-merge says how long its pull request has been open (usetheokit/theokit-sdk#566).**

packages/dep-check/index.mjs

@@ -23,7 +23,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 { consumersOf, discoverEcosystemPackages, latestVersion, packument, publishedManifest, publishedVersions } from "./src/registry.mjs";
 import { batchedWithDeps, detectBuildScript, detectPackageManager, pinOverrides } from "./src/package-manager.mjs";
 import { installFromTarball } from "./src/tarball.mjs";
 
@@ -222,12 +222,23 @@ async function commandInstall(root) {
     dir: p.dir,
     // Carried so the substitution can follow a substituted tarball's own unpublished asks.
     references: siblingReferences(p.manifest, isSibling),
+    // Carried so the substitution can compare the contract this workspace declares against the one
+    // the registry serves at the same version number (usetheokit/theokit#659).
+    manifest: p.manifest,
   }));
   const substitutions = [];
   // Built once for every workspace package, not per reference: the substitution walks
   // transitively, so it needs an answer for siblings the package under test never names.
   const published = {};
   for (const p of workspace) published[p.name] = await publishedVersions(p.name);
+  // What the registry serves AT the workspace's own version, so a contract that moved without the
+  // version moving is visible. Only that one version is fetched: it is the only one the substitution
+  // decision compares against (usetheokit/theokit#659).
+  const manifests = {};
+  for (const p of workspace) {
+    const served = await publishedManifest(p.name, p.version);
+    if (served) manifests[p.name] = { [p.version]: served };
+  }
   // What the registry SERVES, as opposed to everything it holds. `peerInstallSpecs` asks whether
   // that version satisfies a declared range, which is the only way to tell a real prerelease floor
   // (`>=4.63.4-next.0`, latest does not satisfy) from a prerelease SENTINEL (`>=0.1.0-alpha.0`,
@@ -238,7 +249,7 @@ async function commandInstall(root) {
     const refs = siblingReferences(pkg.manifest, isSibling);
     // What the registry cannot answer yet, taken from the workspace instead. Only the gap —
     // see `unpublishedSiblings`.
-    const localSiblings = unpublishedSiblings({ references: refs, workspace, published });
+    const localSiblings = unpublishedSiblings({ references: refs, workspace, published, manifests });
     // Computed AFTER the substitution and from it: a peer this cut is about to publish must not
     // also be asked for as `@latest`, or the registry copy overrides the packed one on the same
     // command line — see `peerInstallSpecs`.
@@ -265,7 +276,11 @@ async function commandInstall(root) {
   // consumer can install today — and a reader deciding what a green D means has to know which
   // one they got. Same reason `untestedFloors` prints (#6).
   for (const s of substitutions) {
-    console.log(`  note: ${s.pkg} was installed against ${s.name}@${s.version} packed from this workspace — the registry does not have that version yet, so this cut is testing what it is about to publish`);
+    const why =
+      s.reason === 'contract-moved'
+        ? 'the registry serves that version with different dependency ranges, so this cut is testing the pair it is about to publish rather than one publication never produces'
+        : 'the registry does not have that version yet, so this cut is testing what it is about to publish';
+    console.log(`  note: ${s.pkg} was installed against ${s.name}@${s.version} packed from this workspace — ${why}`);
   }
   return findings.length === 0 ? 0 : 1;
 }

packages/dep-check/src/checks.mjs

@@ -314,7 +314,30 @@ export function peerInstallSpecs({ references, localSiblings = [], latest = new
   return [...specs];
 }
 
-export function unpublishedSiblings({ references, workspace, published }) {
+/** The fields a consumer's installer resolves. `devDependencies` never reaches a consumer's tree. */
+const INSTALLED_FIELDS = ["dependencies", "peerDependencies", "optionalDependencies"];
+
+/**
+ * Does the workspace copy declare a different installed contract than the registry serves at the
+ * SAME version number?
+ *
+ * Absent evidence answers NO, deliberately. A registry that did not return a manifest, or a caller
+ * that passed none, must not read as "the contract changed" — that would hand a local tarball to
+ * every sibling and stop the check testing what a consumer actually resolves, which is the reason
+ * the version skip exists (usetheokit/theokit#659).
+ */
+function contractMoved(local, publishedManifest) {
+  if (!publishedManifest || !local.manifest) return false;
+  return INSTALLED_FIELDS.some((field) => !sameRanges(local.manifest[field], publishedManifest[field]));
+}
+
+function sameRanges(a = {}, b = {}) {
+  const keys = new Set([...Object.keys(a), ...Object.keys(b)]);
+  for (const k of keys) if (a[k] !== b[k]) return false;
+  return true;
+}
+
+export function unpublishedSiblings({ references, workspace, published, manifests = {} }) {
   const byName = new Map(workspace.map((p) => [p.name, p]));
   const out = [];
   const seen = new Set();
@@ -333,8 +356,22 @@ export function unpublishedSiblings({ references, workspace, published }) {
     // Missing is not empty: an unreachable registry must not read as "nothing is published",
     // or every sibling gets a local tarball and the check tests nothing real.
     if (!Array.isArray(versions) || !versions.length) continue;
-    if (versions.includes(local.version)) continue;
-    out.push({ name: dep, version: local.version, dir: local.dir });
+    // "The registry has this version number" is a DIFFERENT question from "the registry has the
+    // copy this workspace is about to publish". A release that changes a dependency contract across
+    // two workspace packages moves the contract without moving the version, because changesets bumps
+    // at version time and not at pull-request time. Skipping on the number alone installed tomorrow's
+    // package against yesterday's sibling and reported a duplicate peer — a pair publication never
+    // produces, and one no ordering of the old design could avoid (usetheokit/theokit#659).
+    if (versions.includes(local.version) && !contractMoved(local, manifests[dep]?.[local.version])) continue;
+    out.push({
+      name: dep,
+      version: local.version,
+      dir: local.dir,
+      // Which of the two gaps this substitution fills. The report explains itself with this,
+      // and "the registry does not have that version" is FALSE for a contract that moved
+      // without the version moving (usetheokit/theokit#659).
+      reason: versions.includes(local.version) ? 'contract-moved' : 'unpublished',
+    });
     for (const r of local.references ?? []) queue.push(r.dep);
   }
   return out;

packages/dep-check/src/registry.mjs

@@ -34,6 +34,19 @@ export async function latestVersion(name) {
   return doc?.["dist-tags"]?.latest ?? null;
 }
 
+/**
+ * The manifest the registry SERVES for one exact version, or null when it serves none.
+ *
+ * A version number is not a contract: the registry can hold `0.8.0` while the workspace is about to
+ * republish a different `0.8.0` with a widened peer range, because changesets moves the version at
+ * version time and not at pull-request time. Reading the served manifest is what lets the install
+ * leg tell those two apart (usetheokit/theokit#659).
+ */
+export async function publishedManifest(name, version) {
+  const doc = await packument(name);
+  return doc?.versions?.[version] ?? null;
+}
+
 /** Every published version, in registry order. */
 export async function publishedVersions(name) {
   const doc = await packument(name);

packages/dep-check/test/checks.test.mjs

@@ -247,6 +247,7 @@ describe("unpublishedSiblings — what check D cannot get from the registry yet"
     const out = unpublishedSiblings({ references: [{ dep: "@theokit/agents" }], workspace, published });
     expect(out.map((s) => s.name)).toEqual(["@theokit/agents"]);
     expect(out[0].version).toBe("12.1.0");
+    expect(out[0].reason).toBe("unpublished");
   });
 
   it("test_leaves_a_sibling_the_registry_already_has_to_be_installed_from_the_registry", () => {
@@ -261,6 +262,48 @@ describe("unpublishedSiblings — what check D cannot get from the registry yet"
     expect(unpublishedSiblings({ references: [{ dep: "@theokit/nowhere" }], workspace, published })).toEqual([]);
   });
 
+  it("test_substitutes_a_published_version_whose_local_contract_no_longer_matches", () => {
+    // usetheokit/theokit#659. The skip above asks "is this version number published?", which is a
+    // different question from "is the published copy the one this workspace is about to publish?".
+    // A release that widens a peer range across two workspace packages moves the CONTRACT without
+    // moving the version number — changesets bumps at version time, not at pull-request time. So the
+    // gate installed tomorrow's package against yesterday's sibling and reported two copies of a
+    // peer, a pair publication never produces. Measured on usetheokit/theokit#658, which it blocked.
+    const ws = [
+      {
+        name: "@theokit/presenter",
+        version: "0.8.0",
+        dir: "/w/packages/presenter",
+        manifest: { peerDependencies: { "@theokit/sdk": "^4.49.0 || ^5.0.0" } },
+      },
+    ];
+    const pub = { "@theokit/presenter": ["0.7.0", "0.8.0"] };
+    const manifests = { "@theokit/presenter": { "0.8.0": { peerDependencies: { "@theokit/sdk": "^4.49.0" } } } };
+    const out = unpublishedSiblings({ references: [{ dep: "@theokit/presenter" }], workspace: ws, published: pub, manifests });
+    expect(out.map((s) => s.name)).toEqual(["@theokit/presenter"]);
+    // The report explains every substitution, and the older explanation — "the registry does not
+    // have that version yet" — is FALSE here: it has it, with a different contract. A note that
+    // names the wrong reason is worse than none, because it is read and believed.
+    expect(out[0].reason).toBe("contract-moved");
+  });
+
+  it("test_leaves_a_published_version_alone_when_the_local_contract_is_identical", () => {
+    // The narrowness is the point (#659). Substituting whenever a package is in the workspace would
+    // stop testing what a consumer resolves — the reason the skip exists at all. Only a contract
+    // that DIFFERS earns the local tarball.
+    const ws = [
+      {
+        name: "@theokit/presenter",
+        version: "0.8.0",
+        dir: "/w/packages/presenter",
+        manifest: { peerDependencies: { "@theokit/sdk": "^4.49.0" }, dependencies: {} },
+      },
+    ];
+    const pub = { "@theokit/presenter": ["0.8.0"] };
+    const manifests = { "@theokit/presenter": { "0.8.0": { peerDependencies: { "@theokit/sdk": "^4.49.0" } } } };
+    expect(unpublishedSiblings({ references: [{ dep: "@theokit/presenter" }], workspace: ws, published: pub, manifests })).toEqual([]);
+  });
+
   it("test_follows_a_substituted_tarballs_own_unpublished_asks", () => {
     // `@theokit/tauri` depends on `theokit@0.57.0`, which is substituted — and that tarball then
     // requests `@theokit/agents@^12.1.0`, still absent. Substituting only the direct reference