Skip to content

fix(release): publish first-party dependencies as ranges, not exact pins - #887

Open
drewstone wants to merge 5 commits into
mainfrom
fix/cohort-range-specifiers-20260816
Open

fix(release): publish first-party dependencies as ranges, not exact pins#887
drewstone wants to merge 5 commits into
mainfrom
fix/cohort-range-specifiers-20260816

Conversation

@drewstone

Copy link
Copy Markdown
Contributor

The defect

The published manifest of @tangle-network/agent-bench@0.8.12 names five exact first-party versions:

{
  "@tangle-network/agent-eval": "0.145.21",
  "@tangle-network/agent-interface": "1.0.0",
  "@tangle-network/agent-knowledge": "8.0.5",
  "@tangle-network/sandbox": "0.27.1",
  "@tangle-network/agent-runtime": "0.137.0"
}

An exact pin is not a compatibility statement. It names one version and refuses every other, so a consumer that already holds a later cohort member installs a second physical copy of the pinned package. agent-eval is at 0.146.0 and agent-runtime at 0.138.0 today, so two of those five duplicate right now. agent-interface escapes only by coincidence: 1.0.0 and the fleet's ^1.0.0 happen to resolve to the same version, and interface 1.0.1 reopens it.

Caret semantics do not fix an exact pin. The 1.0 cuts do not fix it either.

The mechanism

Nobody wrote those pins. A catalog: specifier and a workspace:* specifier are both replaced by an exact version when the package is packed, so the source manifest looks clean and only the packed manifest carries the defect.

  • pnpm-workspace.yaml now states a range per first-party catalog entry, in the shape the depended-on package's own versioning earns — the same rule expectedPeerRange applies to a peer range: a caret from 1.0.0, the narrower >=X.Y.Z <X.Y+1.0 window below it.
  • bench declares agent-runtime as workspace:^, not workspace:*.

The floors do not move in this change. Only the shape does.

Proof, on the packed tarball

$ npm_config_ignore_scripts=true pnpm --dir bench pack
$ tar -xOzf tangle-network-agent-bench-0.8.13.tgz package/package.json
PACKED @tangle-network/agent-bench 0.8.13
{
 "@tangle-network/agent-eval": ">=0.145.21 <0.146.0",
 "@tangle-network/agent-interface": "^1.0.0",
 "@tangle-network/agent-knowledge": "^8.0.5",
 "@tangle-network/sandbox": ">=0.27.1 <0.28.0",
 "@tangle-network/agent-runtime": "^0.138.0"
}

The guard

scripts/check-published-ranges.mjs packs every publishable workspace package, reads the archive manifest, and fails when a first-party specifier names one version instead of a range. It runs inside the existing verify:package (root) and verify:package:static (bench) scripts — no new CI.

Negative test, by reverting one catalog entry to 8.0.5:

@tangle-network/agent-runtime publishes exact first-party version pins, which duplicate
the package for every consumer already holding a later one:
  dependencies.@tangle-network/agent-knowledge = 8.0.5
exit=1

scripts/verify-packed-cohort.mjs asserts the same rule on every cohort archive it packs.

Checks that had to move with it

A range lets the installed version float above the catalog floor, so two checks that compared a peer range against the installed version now compare it against the catalog range and assert the installed version is admitted: verify-official-optimizers.mjs, and assertPeerMatchesDevelopmentDependency through the new cohortRange. assertExactDependency admits both range shapes through the new rangeAdmits.

Verification

  • pnpm typecheck exit 0
  • pnpm exec vitest run scripts/lib/packed-package-test.test.mjs — 10 tests passed, 0 failed
  • node scripts/check-published-ranges.mjs exit 0; exit 1 on the reverted entry above
  • node scripts/check-version-bump.mjs exit 0 — bench 0.8.12 -> 0.8.13 pays for 5 consumer-visible changes, root 0.138.0 -> 0.138.1 pays for 3
  • pnpm install --frozen-lockfile exit 0; the lockfile moves 7 specifier lines and no resolved version

What this does NOT fix

Below 1.0 a range still stops at the next minor, so agent-eval at >=0.145.21 <0.146.0 does not admit the published 0.146.0 — that copy survives until the cohort floor moves or agent-eval cuts 1.0.0. Raising that floor is a separate change and belongs to the lane cutting agent-eval 1.0.0.

A `catalog:` entry and a `workspace:*` specifier are both replaced by an
exact version when the package is packed. The published manifest of
agent-bench@0.8.12 therefore named five exact first-party versions, so a
consumer that already held a later cohort member installed a second physical
copy of each.

The catalog now states a range per first-party entry, in the shape the
depended-on package's own versioning earns: a caret from 1.0.0, and the
narrower `>=X.Y.Z <X.Y+1.0` window below it. bench declares agent-runtime as
`workspace:^`.

`check:published-ranges` packs every publishable workspace package and fails
when a packed first-party specifier names one version instead of a range.

@tangletools tangletools left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Auto-approved drewstone PR — 8990c125

This PR was opened by the trusted drewstone account.
The full PR reviewer audit still runs separately and will publish findings if it detects issues.

This approval is provisional. It rests on the audit running. If the audit cannot run — for example the CLI bridge rejects it — this approval is dismissed rather than left standing, so an unrun check never reads as a passing one.

tangletools · auto-approval · reason: drewstone_author · 2026-08-16T22:29:00Z

@tangletools tangletools left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Value Audit — sound-with-nits

Verdict sound-with-nits
Coverage 2 of 2 lenses (value, usefulness)
Concerns 3 (3 weak-concern)
Heuristic 0.0s
Duplication 0.1s
Interrogation 198.5s (2 bridge agents)
Total 198.6s

💰 Value — sound-with-nits

Converts every first-party catalog/workspace dependency from an exact pin to a cohort range and adds a pack-time gate that refuses exact first-party pins in packed manifests — a correct root-cause fix for a real duplication defect, built squarely in the grain of the existing packed-package test libr

  • What it does: Three things. (1) pnpm-workspace.yaml:27-41 changes every @tangle-network/* catalog entry from an exact version (e.g. 0.145.21) to a range in the shape the depended-on package's own versioning earns — ^1.0.0 from 1.0.0, >=X.Y.Z <X.Y+1.0 below it — and bench/package.json:48 changes agent-runtime from workspace:* to workspace:^, so the packed manifest carries ranges instead of exact ve
  • Goals it achieves: Prevent published manifests from exact-pinning first-party packages. An exact pin forces any consumer that already holds a later cohort member (agent-eval 0.146.0, agent-runtime 0.138.0 exist today) to install a second physical copy — two class identities, two module registries, instanceof false across the seam. The goal is two-sided: fix the current manifests (version bumps 0.138.0→0.138.1 and
  • Assessment: Good, and coherent. The diagnosis is correct and evidenced: pnpm substitutes catalog and workspace specifiers with exact versions at pack time, so only the packed manifest carries the defect — hence the check packs and reads the archive rather than the source (check-published-ranges.mjs:11-14). Crucially, the existing heavyweight gates genuinely cannot catch this: verify-packed-cohort.mjs and benc
  • Better / existing approach: none — this is the right approach. Searched scripts/ for existing equivalents: check-publish-workflow.mjs validates CI workflow hygiene only; check-version-bump.mjs validates bump hygiene; verify-packed-cohort.mjs and verify-packed-consumer.mjs are fresh-install gates blind to this failure mode (exact pins resolve single-copy in a fresh tree); publint/attw validate exports and types, not dependenc
  • Model: opencode/zai-coding-plan/glm-5.2
  • Bridge attempts: 2
  • Bridge warning: opencode/kimi-for-coding/k2p7: opencode: opencode error

🎯 Usefulness — sound

Fixes a real published-manifest defect (exact first-party pins causing duplicate installs) by stating catalog ranges plus workspace:^, and wires a fail-closed packed-tarball guard into both CI and the publish gate — verified green on the current tree and red on a sabotaged exact pin.

  • Integration: Fully reachable now, not merely imminently. The new guard (scripts/check-published-ranges.mjs) is invoked from root verify:package (package.json:140) which runs in CI (ci.yml:65) and gates npm publishing (publish.yml:132; publish-npm needs verify), and from bench's verify:package:static (bench/package.json:40) via verify:bench/verify:bench:published (publish.yml:174,303; ci.yml:137). The new lib s
  • Fit with existing patterns: In the grain of the codebase. The range shape rule already existed as expectedPeerRange for peers (scripts/lib/packed-package-test.mjs:51-55); cohortRange composes it rather than duplicating it, and the catalog entries in pnpm-workspace.yaml:30-37 apply the same rule. The split is correct for this workspace: workspace member (agent-runtime in bench) uses workspace:^, external first-party packages
  • Real-world viability: Holds up off the happy path. The check reads the packed tarball — the artifact consumers actually resolve against — not the source manifest, which is the correct ground truth since the defect only exists after pack substitution. It is fail-closed: missing install errors through execFileSync, multiple archives error (check-published-ranges.mjs:56-59), missing package.json errors (line 78). The wind
  • Model: opencode/zai-coding-plan/glm-5.2
  • Bridge attempts: 1

💰 Value Audit

🟡 Two spellings for a pre-1.0 window; caretAdmits refuses the one bench now publishes [maintenance] ``

The change standardizes pre-1.0 ranges as >=X.Y.Z <X.Y+1.0 (pnpm-workspace.yaml:32,35,37) but bench's workspace:^ on the pre-1.0 agent-runtime packs as ^0.138.0 — semantically the same minor window under npm rules, yet a second spelling in the fleet. Meanwhile caretAdmits (packed-package-test.mjs:64) returns false whenever floorMajor < 1, so rangeAdmits('^0.138.0', '0.138.1') is false even though the window admits it. Today no live path passes a 0.x caret to rangeAdmits (`assertFirst

🟡 Third copy of the pack-and-read-tarball idiom [duplication] ``

check-published-ranges.mjs:46-70 (pnpm pack to scratch dir, find the single .tgz, tar -xOzf ... package/package.json) now repeats the same plumbing that verify-official-optimizers.mjs:72-82 and verify-packed-cohort.mjs's buildAndPack each carry in their own harness style (execFileSync vs spawnSync run()). ~15 lines each and the surrounding harnesses genuinely differ, so consolidation is a judgment call, not a must — but a fourth consumer would make a shared packAndReadManifest(dir) helper

🎯 Usefulness Audit

🟡 Guard hard-fails if pnpm-workspace.yaml ever gains a glob entry [robustness] ``

workspacePackageDirectories throws on any workspace entry containing '*' (scripts/check-published-ranges.mjs:38-39). Today the workspace is only bench, so this is fine, and the failure is fail-closed (blocks verify:package rather than silently skipping a package). But the first person to add e.g. packages/* to pnpm-workspace.yaml will hit a hard error in the publish gate and must teach this script globs. Deliberate per the error message; noting so a human knows the tradeoff was chosen, not o


What this audit checks

It judges the change on its merits — not whether it was tasked out in an issue. Unticketed, fast-moving work is fine; the question is whether the change is good and whether a better or existing approach should be used instead.

Pass What it asks
Heuristic Vague title? Whitespace-only or cruft-bearing diff? (content signals only)
Duplication Do added function/class names already exist elsewhere in the repo?
Value Audit What does it do? What goal does it achieve? Is it good? Better architecture or already-exists?
Usefulness Audit Does it integrate and fit? Will it hold up in real use and actually get used?

Findings are concerns, not blocks — the human reviewer decides what to do with them.

value-audit · 20260817T002330Z

@tangletools

Copy link
Copy Markdown
Contributor

✅ No Blockers — 8990c125

Review health 100/100 · Reviewer score 26/100 · Confidence 85/100 · 35 findings (4 medium, 31 low)

opencode GLM 5.2 opencode DeepSeek v4 Pro opencode DeepSeek v4 Flash aggregate
Readiness 52 74 26 26
Confidence 85 85 85 85
Correctness 52 74 26 26
Security 52 74 26 26
Testing 52 74 26 26
Architecture 52 74 26 26

Reviewer score is advisory once the run is complete and the verdict has no blockers.

Full multi-shot audit completed 5/5 planned shots over 10 changed files. Global verifier still owns final merge decision. | Full multi-shot audit completed 5/5 planned shots over 10 changed files. Global verifier still owns final merge decision. | Full multi-shot audit completed 5/5 planned shots over 10 changed files. Global verifier still owns final merge decision.

🟠 MEDIUM Runner has no automated test coverage — scripts/check-published-ranges.mjs

The pure helpers (isExactVersionSpec, cohortRange, rangeAdmits, assertFirstPartyRangeSpecs) are unit-tested in scripts/lib/packed-package-test.test.mjs, but the runner itself — the pack invocation, workspace-directory enumeration, tarball extraction, and the exit-code contract (lines 32-106) — has no test. The exact behaviors this PR's premise rests on (catalog:/workspace: resolution inside the packed manifest, the 'exactly one archive' assumption, tar layout 'package/package.json') are exercised only by CI's verify:package execution, which made me hand-verify them at review time. An integration test packing a fixture package and asserting exit code w

🟠 MEDIUM pnpm pack failure crashes the loop and swallows pnpm's stderr — scripts/check-published-ranges.mjs

packedManifest(directory) is called outside the try/catch at lines 81-88, so a failing 'pnpm pack' throws uncaught: the script dies with a stack trace, remaining workspace packages are never checked, and the piped stderr (stdio 'pipe' at line 52, available as error.stderr) is never printed, so CI logs cannot show WHY pack failed. The gate still exits non-zero, so it fails closed, but a pack failure in package 1 of N masks all later results and is hard to diagnose. Fix: move packedManifest inside the try, and on catch a

🟠 MEDIUM isExactVersionSpec misses npm-valid exact pins (v-prefixed, =, npm: aliases) — scripts/lib/packed-package-test.mjs

const exactVersion = /^\d+.\d+.\d+(?:[-+].*)?$/ only matches bare versions. Confirmed by execution: isExactVersionSpec('v1.0.0') === false, isExactVersionSpec('=1.0.0') === false, and assertFirstPartyRangeSpecs accepts a first-party dependency of 'v1.0.0'. npm treats v1.0.0, =1.0.0, and npm:@tangle-network/x@1.0.0 as exact pins, so these publish as exact versions and produce the duplicate-physical-copy defect the PR exists to prevent, while the guard reports clean. pnpm catalog/workspace substitution produces bare versions so current tree is unaffected, but the guard silently passes a class of pins it claims to forbid. Fix: strip optional ^v/=v prefixes and npm: aliases before the exact-version test, or reject them explicitly.

🟠 MEDIUM JS/Python bridge version pairing can diverge in consumer verification — scripts/verify-official-optimizers.mjs

assertInstalledAdmitted(appDir, '@tangle-network/agent-eval', packedAgentEvalVersion) (lines 161-172) replaces the old exact assertInstalledVersion. The consumer app now declares the range (>=0.145.21 <0.146.0) and npm installs the NEWEST in-range version, while the Python bridge stays pinned to the workspace version: installWheelPythonPackages uses agent-eval-rpc==${agentEvalVersion} (line 204) and AGENT_EVAL_EXPECTED_BRIDGE_VERSION=agentEvalVersion=workspaceAgentEvalVersion ([lines 40-42](https://github.co

🟡 LOW Doc overstates catalog: pack substitution as 'exact version' — docs/STABILITY.md

Line 55: 'A catalog: specifier and a workspace:* specifier are both replaced by an exact version when the package is packed.' Verified empirically with pnpm 11.22: a catalog: specifier is substituted with the catalog entry value verbatim, so a ranged entry (this repo's >=0.9.4 <0.10.0, ^1.0.0) packs as that same range, not an exact pin. Only workspace:* and catalog entries whose value is already exact (e.g. a future @tangle-network/*: 0.9.4 in pnpm-workspace.yaml) produce an exact pin. Impact: the doc's stated mechanism is wrong for the common ranged case, which could mislead a reader into thinking ranged catalog entries are flattened to pins on pack (they ar

🟡 LOW Pack-replacement sentence overclaims: catalog: is replaced by the catalog entry, not always an exact version — docs/STABILITY.md

The sentence 'A catalog: specifier and a workspace:* specifier are both replaced by an exact version when the package is packed' is false for catalog: entries that hold a range. Empirical proof: I ran pnpm run check:published-ranges at head; it passes and prints packed specs like @tangle-network/agent-interface@^1.0.0 and @tangle-network/agent-core@>=0.9.4 <0.10.0 — ranges, not exact versions. The repo's own pnpm-workspace.yaml comment states the correct semantics ('A catalog: specifier is replaced by the entry below when the package is packed'), and the same wrong wording is duplicated in the header of scripts/check-published-ranges.mjs:11-13 (outside this shot's scope). The doc's conclusion is unaffected — the defect path (exact catalog entry, or workspace:* which does become

🟡 LOW Below-1.0 windows still duplicate across the next minor line — pnpm-workspace.yaml

>=0.145.21 <0.146.0 (agent-eval), >=0.9.4 <0.10.0 (agent-core), >=0.16.0 <0.17.0 (agent-profile-materialize), >=0.27.1 <0.28.0 (sandbox) guarantee one physical copy only within a single minor line. A consumer already holding agent-eval 0.146.0 (pulled by a newer agent-runtime) who installs runtime@0.138.1 will still get a second 0.145.x copy. This is inherent to the documented below-1.0 policy and is arguably correct ('a minor may remove'), but the comment's framing (lines 18-21: 'so every first-party entry states a RANGE') can read as fully solving the duplicate-copy defect when it in fact only bounds it to the minor window. Doc nit only; the behavior is int

🟡 LOW Caret ranges above 1.0 trust the depended-on package's minor-is-additive promise — pnpm-workspace.yaml

@tangle-network/agent-interface: ^1.0.0, @tangle-network/agent-knowledge: ^8.0.5, and @tangle-network/agent-trace-contract: ^1.0.2 admit every later minor up to the next major. The comment (lines 23-26) asserts 'a minor is additive' from 1.0.0, but that is a semver policy statement, not a verified fact about these packages' changelogs. If any 1.x/8.x minor removes or narrows an API agent-runtime@0.138.1 relies on, a consumer resolving the range will silently install the breaking minor — the same class of duplicate/conflict the PR exists to prevent. Impact is limited because the below-1.0 entries (agent-core, agent-eval, agent-profile-materialize, sandbox) keep

🟡 LOW Range catalog + minimumReleaseAgeExclude lets fresh resolutions pick a same-day first-party release — pnpm-workspace.yaml

minimumReleaseAge: 4320 (72h) gates new resolutions, but minimumReleaseAgeExclude lists '@tangle-network/*'. Before this PR the exact pin meant a release entered this repo only when a human moved the pin; now any fresh resolution without a lockfile (new CI checkout with frozen-lockfile disabled, lockfile regen, new contributor) silently picks the newest cohort member, e.g. a 0.9.5 of agent-core published minutes earlier. The committed lockfile pins 0.9.4 so CI and existing checkouts are unaffected, and first-party trust is the stated reason for the exclude, so this is a documented tradeoff — but the coupling between the new floating specifiers and the age-gate exclusion is not mentioned in the new comment block. Fix: add one sentence to the comment naming it, or narrow the exclude if suppl

🟡 LOW bench package pack path not re-verifiable in a fresh worktree — pnpm-workspace.yaml

pnpm pack of @tangle-network/agent-bench fails in this worktree with ERR_PNPM_CANNOT_RESOLVE_WORKSPACE_PROTOCOL on @tangle-network/agent-runtime: workspace:^ (bench/package.json:48) because no pnpm install has populated the store — workspace-protocol resolution needs installed links. scripts/check-published-ranges.mjs runs this pack, so the new check:published-ranges gate depends on a prior install; CI's verify:package:static for bench runs pnpm run build first (which itself requires node_modules), so the gate is satisfiable, and the root package packed cleanly in the same run. Verification limitation in this environment, not a code defect; flagging so the reviewer of the check script confirms CI ordering.

🟡 LOW windowAdmits parser couples to the hand-written single-space YAML form — pnpm-workspace.yaml

scripts/lib/packed-package-test.mjs windowAdmits() accepts only '>=X.Y.Z <X.Y.Z' with exactly one space; a future edit like '>=0.9.4 <0.10.0' or a trailing space silently fails both caretAdmits and windowAdmits, making rangeAdmits return false and surfacing as a confusing 'installed X is outside its declared range' failure in verify-official-optimizers.mjs rather than a parse error. Current committed values match the strict form (verified against all five window entries), so this is brittleness, not a live defect. Fix: trim/collapse whitespace in windowAdmits' regex, or add a test asserting every catalog entry parses as exactly one admitted shape.

🟡 LOW Default invocation hard-crashes on glob entries in pnpm-workspace.yaml — scripts/check-published-ranges.mjs

Line 38 throws on any entry containing '*'. The ubiquitous pnpm pattern packages/* (or bench/*) would crash the no-arg invocation of this check rather than skip or glob-expand. Currently harmless because this repo's pnpm-workspace.yaml lists only 'bench' (verified), but the failure mode is a full abort with no guidance. Suggest expanding simple globs or degrading gracefully with a clear error naming the unsupported entry.

🟡 LOW Glob workspace entries hard-fail; duplicate '.' entry packs twice — scripts/check-published-ranges.mjs

Any pnpm-workspace.yaml entry containing '' throws, so the natural growth path (adding 'packages/') breaks verify:package until this script is taught directory expansion; and a '.' entry would push resolve(repoRoot, '.') again, packing the root twice and printing duplicate output. Both are fail-closed and not reachable with the current workspace (only 'bench'), so this is maintenance friction to note, not a live bug. Fix: expand simple one-level globs with readdirSync, and dedupe the directories array with a Set.

🟡 LOW No timeout on execFileSync child processes — scripts/check-published-ranges.mjs

execFileSync('pnpm', [...]) and the tar extraction have no timeout option. A hung pack (corepack prompt, filesystem stall) hangs verify:package indefinitely instead of failing the gate. Fix: pass timeout (e.g. 120_000) and let the thrown error accumulate as a failure.

🟡 LOW Report omits optionalDependencies, assertions cover them — scripts/check-published-ranges.mjs

The first-party stdout report spreads only manifest.dependencies and manifest.peerDependencies (lines 89-92), but assertFirstPartyRangeSpecs (callee in scripts/lib/packed-package-test.mjs:116) also enforces optionalDependencies. An optional first-party dep is validated yet never printed in the summary line — the report can contradict what was actually checked. Merge the spread with ...(manifest.optionalDependencies ?? {}) for consistency.

🟡 LOW Requested package args resolve against repoRoot, not the caller's cwd — scripts/check-published-ranges.mjs

process.argv.slice(2).map(entry => resolve(repoRoot, entry)) silently ignores the invoking cwd. The documented in-repo call (node ../scripts/check-published-ranges.mjs bench from bench/, and node scripts/check-published-ranges.mjs from root) resolves correctly, but an ad-hoc relative path passed from a subdirectory points at the wrong directory and either throws 'no package.json at ...' or checks an unintended package. Resolve against process.cwd() or document that args are repo-root-relative.

🟡 LOW pack/source-parse failure aborts with raw stack trace, not an aggregated summary — scripts/check-published-ranges.mjs

packedManifest() (pnpm pack, tar, JSON.parse) and the source JSON.parse at line 79 throw out of the loop instead of joining the failures array, so one bad package ends the script with an uncaught exception rather than a summary of all failing packages. Exit code is still non-zero (CI fails correctly), but error UX is worse and remaining packages are never checked. Wrapping the per-directory pack in the existing try/catch would make the report consistent.

🟡 LOW pnpm pack failure aborts the whole run instead of being collected — scripts/check-published-ranges.mjs

packedManifest() is called outside the per-package try/catch (which only wraps the two assert* calls at lines 82-88). Any pack-time failure — verified live here: ERR_PNPM_CANNOT_RESOLVE_WORKSPACE_PROTOCOL for bench ('Try running pnpm install') — propagates as an unhandled Node stack trace and kills the script before other packages are reported. Still fail-closed (exit 1), so CI stays gated, but one bad package masks every other and the diagnostic is a raw crash. Fix: wrap the packedManifest call in the same try/catch and push its error.message into failures, or preflight-check workspace resolution before looping.

🟡 LOW report omits optionalDependencies while the assertion covers them — scripts/check-published-ranges.mjs

The firstParty summary merges only manifest.dependencies and manifest.peerDependencies, but assertFirstPartyRangeSpecs (packed-package-test.mjs:116) also scans optionalDependencies. A first-party optional dependency that is a range passes the assertion but is silently absent from the human-readable output; an exact pin there would be reported by the assert anyway, so this is cosmetic, not a correctness gap.

🟡 LOW workspacePackageDirectories throws on glob entries — scripts/check-published-ranges.mjs

Any pnpm-workspace.yaml entry containing '*' (the common packages/* convention) throws and aborts the default run. The current workspace lists only bench (verified), so it passes today, but the first glob adoption breaks pnpm run check:published-ranges with no per-package fallback. Consider resolving globs with a matcher or documenting the plain-directory-only constraint next to the pnpm-workspace.yaml packages list.

🟡 LOW Exact-pin guard misses npm's v-prefix and = exact forms — scripts/lib/packed-package-test.mjs

exactVersion = /^\d+.\d+.\d+(?:[-+].)?$/ does not match 'v1.2.3' or '=1.2.3', both of which npm resolves as exact pins. Verified by execution: assertFirstPartyRangeSpecs({dependencies:{'@tangle-network/sandbox':'v0.27.1'}}) passes, so a catalog entry holding such a shape would publish an exact pin the guard exists to block. Not reachable today (catalog entries are hand-curated to the two documented range shapes) and the strict packed install backstops it, hence low. Fix: /^(?:v|=)?\d+.\d+.\d+(?:[-+].)?$/ after trim, or reject unknown shapes outright.

🟡 LOW Prerelease versions compared without prerelease ordering — scripts/lib/packed-package-test.mjs

windowAdmits strips the prerelease suffix via /^(\d+).(\d+).(\d+)/ on the version, so a prerelease is ordered as its numeric base. Verified: rangeAdmits('>=0.27.1 <0.28.0', '0.27.1-rc.0') returns true even though 0.27.1-rc.0 sorts below 0.27.1 in semver, and caretAdmits similarly ignores prerelease floors. Impact: the only consumers (assertExactDependency, assertCatalogAdmits, assertInstalledAdmitted) compare against installed/packed stable versions, so prereleases never reach these paths. Fix (optional): reject or explicit-handle prerelease versions in found/floor matches, or document that these helpers assume stable versions only.

🟡 LOW caretAdmits rejects all pre-1.0 carets and prerelease carets, unlike npm — scripts/lib/packed-package-test.mjs

return floorMajor < 1 || major !== floorMajor means rangeAdmits('^0.2.3', '0.2.3') === false (confirmed by execution), even though npm's ^0.2.3 is >=0.2.3 <0.3.0 — semantically identical to the window shape this code prefers for pre-1.0. caretAdmits also only matches /^^(\d+).(\d+).(\d+)$/, so '^1.2.3-rc.1' never admits anything. Impact is a false positive in assertExactDependency (verify-packed-cohort.mjs): a first-party dependency declared '^0.2.3' in dependencies fails with the misleading message 'requires X@^0.2.3, packed 0.2.3' even though the packed version is admitted by npm. Deliberate convention (pre-1.0 must use the window shape) but the divergence from npm semantics and the misleading error are undocumented.

🟡 LOW isExactVersionSpec and expectedPeerRange disagree on build metadata — scripts/lib/packed-package-test.mjs

isExactVersionSpec uses /^\d+.\d+.\d+(?:[-+].)?$/ (line 68), which accepts '1.0.0+build'. cohortRange then calls expectedPeerRange on that spec, but expectedPeerRange (line 52) and currentMinorPeerRange (line 38) use /^(.)...(?:-.+)?$/ which only admits a '-' prerelease, not '+'. Verified: cohortRange('1.0.0+build') throws 'cannot derive peer range from version 1.0.0+build'. Imp

🟡 LOW windowAdmits diverges from npm for prereleases of the window ceiling — scripts/lib/packed-package-test.mjs

The window extracts major.minor.patch from the target version (found = /^(\d+).(\d+).(\d+)/) and compares numerically, so it rejects 0.146.0-rc.1 under '>=0.145.21 <0.146.0' (target 0.146.0 fails target < ceiling). npm semver admits 0.146.0-rc.1 here because a prerelease sorts below its release. Same class of divergence for a prerelease floor (0.145.21-rc.1, correctly rejected by both). Only reachable when a first-party package ships prerelease versions, which the cohort currently does not; would surface as a false positive in assertExactDependency/assertInstalledAdmitted. Document the divergence or handle prerelease components explicitly.

🟡 LOW windowAdmits ignores semver prerelease semantics in both directions — scripts/lib/packed-package-test.mjs

The version regex /^(\d+).(\d+).(\d+)/ drops the prerelease suffix, so windowAdmits('>=0.145.21 <0.146.0', '0.145.21-rc.1') returns true (verified by execution) even though npm semver refuses a prerelease in a release-only comparator window — a fail-open deviation used by assertInstalledAdmitted/assertCatalogAdmits (verify-official-optimizers.mjs:306,330) and assertExactDependency (verify-packed-cohort.mjs:609). Conversely the bound regex rejects prerelease floors: currentMinorPeerRange('0.27.1-rc.1') emits '>=0.27.1-rc.1 <0.28.0' (verified), which windowAdmits can never admit, so a future prerelease catalog entry would spuriously red the cohort verification. Fix: capture and compare prerelease segments, or explicitly document prereleases as out of scope and assert their absence. Note ca

🟡 LOW windowAdmits integer ordering loses precision above 8-digit majors — scripts/lib/packed-package-test.mjs

order() packs (major,minor,patch) into one float with a 1e12 major scale; beyond Number.MAX_SAFE_INTEGER comparisons become garbage: windowAdmits('>=99999999.0.0 <99999999.0.1', '99999999.0.0') returns false (verified) when the version sits exactly at the floor and must admit. No real package uses such majors, and neighboring caretAdmits (lines 62-65) avoids the problem by comparing major by equality first. Fix: compare component tuples lexicographically like caretAdmits does.

🟡 LOW Modified assertPeerMatchesDevelopmentDependency behavior has no direct unit test — scripts/lib/packed-package-test.test.mjs

The only change to existing production behavior — line 133, expected = cohortRange(version) instead of expectedPeerRange(version), which newly accepts a range dev dependency and demands verbatim peer equality — is covered only by the heavyweight CI scripts (verify-packed-cohort, verify-official-optimizers, verify-package-exports), not by the new unit test file. cohortRange's throw paths (non-string, empty string, build-metadata pins like '1.2.3+build.1' which throw 'cannot derive peer range') are also untested. Add a describe block: exact dev spec '0.27.1' expects peer '>=0.27.1 <0.28.0', range dev spec '^1.0.0' expects peer '^1.0.0', mismatch throw

🟡 LOW No regression test for the modified assertPeerMatchesDevelopmentDependency — scripts/lib/packed-package-test.test.mjs

The one production behavior change to a pre-existing function — expected = cohortRange(version) at packed-package-test.mjs:133, replacing expectedPeerRange(version) — has no direct test. The new suite covers only the four newly added helpers. The change is behavior-visible: a devDependency declared as a range (e.g. '>=0.145.21 <0.146.0') now yields that range verbatim as the expected peer, where the old code threw on any non-exact spec. Add cases: exact version -> canonical cohort shape (unchanged path), range devDep -> verbatim peer match, and a range devDep whose peer differs -> throws.

🟡 LOW Exact-pin catalog entry produces a misleading failure message — scripts/verify-official-optimizers.mjs

assertVersion(packedAgentEvalVersion, catalogRange('@tangle-network/agent-eval'), ...) compares the packed devDep against cohortRange(catalogSpec). If a maintainer sets the catalog to an exact pin (e.g. 0.145.21), cohortRange expands it to >=0.145.21 <0.146.0 while pnpm packs the literal 0.145.21, so the error reads 'must be >=0.145.21 <0.146.0, found 0.145.21' — blaming the packed dependency instead of the catalog entry that caused it. Works as a gate but the message should point at pnpm-workspace.yaml.

🟡 LOW New inline helpers lack direct unit coverage — scripts/verify-official-optimizers.mjs

assertInstalledAdmitted, catalogRange, assertCohortRange, and assertCatalogAdmits (lines 304-335) are only exercised by the full verify:official-optimizers run, which requires Python 3.12, pip network installs, and a 10-minute npm install — not part of fast CI. The shared cohortRange/rangeAdmits helpers they compose are unit-tested, but the wiring (e.g. that catalogRange rejects a missing catalog entry, that assertCohortRange compares against peerDependencies) has no fast path test. Consider extracting these into packed-package-test.mjs alongside their peers so catalog-driven assertions get the same fast coverage.

🟡 LOW Prerelease versions and prerelease-bearing ranges are mishandled by prefix-only version parsing — scripts/verify-official-optimizers.mjs

caretAdmits/windowAdmits match versions with /^(\d+).(\d+).(\d+)/ and ignore prerelease suffixes, while their range regexes accept no prerelease on the floor/ceiling. Consequences: an installed 0.145.22-rc1 would be admitted by >=0.145.21 <0.146.0 where npm semver would reject it (over-admission in a verification gate), and a catalog range containing a prerelease floor fails both parsers and reports the version as outside its own range. Cannot trigger with the current catalog (all stable specs) and npm-resolved stable installs, so latent; worth normalizing to a real semver satisfiedBy if these checks ever guard prerelease cohorts.

🟡 LOW Tautological admission checks on direct consumer dependencies — scripts/verify-official-optimizers.mjs

Lines 161-167 call assertInstalledAdmitted with the same range string the consumer's own package.json (lines 130-132) declares as the direct dependency. npm already guarantees a resolved install sits within its declared range, so rangeAdmits can never fail here. This intentionally replaces the former assertInstalledVersion exact-equality check (installed === packed devDependency version), which used to prove the consumer got the exact version the workspace builds/tested against. That relaxation is the P

🟡 LOW Temp dir leaks when the new pre-try catalog asserts fail — scripts/verify-official-optimizers.mjs

tempRoot=mkdtempSync(...) at line 47 runs before the try/finally at line 56, and the new assertCohortRange/assertCatalogAdmits calls (lines 51-54) sit between them. Any failure there skips the finally rmSync(tempRoot) at line 197. The

🟡 LOW rangeAdmits cannot evaluate a ^0.x catalog spec, yielding a confusing false failure — scripts/verify-official-optimizers.mjs

assertInstalledAdmitted and assertCatalogAdmits (line 330) rely on rangeAdmits, which is caretAdmits || windowAdmits. caretAdmits (lib/packed-package-test.mjs:64) returns false whenever floorMajor < 1, and windowAdmits only matches the >=X <Y shape. So if a maintainer writes a pre-1.0 catalog entry as ^0.27.1 instead of the documented window form (the yaml comment is the only guard), every check reports installed 0.27.1 is outside its declared range ^0.27.1 even though the version is semver-compatible — a fail-closed but misleading CI/publish blocker. Fix: either reject ^0.x specs at catalogRange() with a message pointing to the window form, o


tangletools · 2026-08-17T00:44:15Z · trace

tangletools
tangletools previously approved these changes Aug 17, 2026

@tangletools tangletools left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Approved — 35 non-blocking findings — 8990c125

Full multi-shot audit completed 5/5 planned shots over 10 changed files. Global verifier still owns final merge decision. | Full multi-shot audit completed 5/5 planned shots over 10 changed files. Global verifier still owns final merge decision. | Full multi-shot audit completed 5/5 planned shots over 10 changed files. Global verifier still owns final merge decision.

Full immutable report for this review: trace

Summary comment for this run: full summary


tangletools · 2026-08-17T00:44:15Z · immutable trace

# Conflicts:
#	pnpm-lock.yaml
#	pnpm-workspace.yaml
The packed manifest now carries a range for its first-party dependencies,
which is a consumer-visible change and cannot ship under a published version.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants