Skip to content

fix(release): apply npm's caret rule to a pre-1.0 dependency - #139

Merged
drewstone merged 3 commits into
mainfrom
fix/verify-package-caret-0x
Aug 16, 2026
Merged

fix(release): apply npm's caret rule to a pre-1.0 dependency#139
drewstone merged 3 commits into
mainfrom
fix/verify-package-caret-0x

Conversation

@drewstone

Copy link
Copy Markdown
Contributor

Problem

scripts/verify-package.mjs reads a caret range as "same major, floor at or below the installed version". npm reads a pre-1.0 caret more narrowly: ^0.9.0 is locked to the 0.9 minor and ^0.0.3 to the 0.0.3 patch, because a pre-1.0 package states no additive promise.

Measured on the shipped guard (the function extracted verbatim and executed):

range version npm semver.satisfies shipped guard
^1.0.0 1.0.0 admit admit
^1.0.0 2.0.0 refuse refuse
^0.9.0 0.10.0 refuse admit
^0.0.3 0.0.4 refuse admit

Admitting 0.10.0 under ^0.9.0 is the second-copy shape this guard exists to refuse.

A second, smaller defect: the installed version was read with split('.'), so 1.0.0-rc.1 produced a NaN patch and passed the floor comparison by accident rather than by rule.

caretPeerRange was also unconditional — it returned a caret for any version, including a pre-1.0 version that earns the narrower window.

Change

scripts/lib/peer-range.mjs holds one definition of the rule: expectedPeerRange derives the shape from the dependency's own version (caret from 1.0.0, next-minor window below it), and caretAdmits applies npm's upper bound. verify-package.mjs and verify-official-optimizers.mjs both import it, so the two scripts no longer carry separate copies of the same rule.

This matches the helper agent-runtime adopted in scripts/lib/packed-package-test.mjs.

Proof

The rule, executed after the change:

^1.0.0 | 1.0.0            | admit
^1.0.0 | 1.4.2            | admit
^1.0.0 | 2.0.0            | refuse
^1.2.0 | 1.1.0            | refuse
^0.9.0 | 0.9.3            | admit
^0.9.0 | 0.10.0           | refuse
^0.0.3 | 0.0.3            | admit
^0.0.3 | 0.0.4            | refuse
>=1.0.0 | 1.4.2           | refuse
1.0.0  | 1.0.0            | refuse
^1.0.0 | 1.2.0-develop.1  | admit

Shape derived from a version:

1.0.0     -> ^1.0.0
0.145.21  -> >=0.145.21 <0.146.0
0.27.1    -> >=0.27.1 <0.28.0

The released cohort is unchanged. Derived against this package.json:

@tangle-network/agent-interface devPin 1.0.0    | declared ^1.0.0                | derived ^1.0.0                | MATCH
@tangle-network/agent-eval      devPin 0.145.21 | declared >=0.145.21 <0.146.0   | derived >=0.145.21 <0.146.0   | MATCH

node --check passes on all three files. The full verify:package run packs and installs from the registry, so it runs on CI, not here.

Scope

Scripts only. No published surface changes and no release bump: the current cohort derives the ranges package.json already declares.

The cohort admission check read a caret range as "same major, floor at or
below the installed version". npm reads a pre-1.0 caret more narrowly: it
locks 0.x to its minor and 0.0.z to its patch. The check therefore admitted
0.10.0 under ^0.9.0, which is the second-copy shape this guard exists to
refuse. It also read a prerelease version through split('.'), which made the
patch NaN and passed the floor comparison by accident.

The peer range shape was also unconditional: caretPeerRange returned a caret
for any version, including a pre-1.0 version that states no additive promise.

Both scripts now derive the shape and the admission from one module, so the
rule has one definition in this repository. agent-interface 1.0.0 and
agent-eval 0.145.21 derive the ranges package.json already declares, so the
released cohort is unchanged.
@drewstone

Copy link
Copy Markdown
Contributor Author

@tangletools review now

@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 — c17e60d4

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-16T18:06:35Z

tangletools
tangletools previously approved these changes Aug 16, 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.

✅ Auto-approved drewstone PR — c17e60d4

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-16T18:06:43Z

@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 155.9s (2 bridge agents)
Total 156.0s

💰 Value — sound-with-nits

Fixes a real flaw in the freshly-shipped caret-admission guard (it admitted 0.10.0 under ^0.9.0 and 0.0.4 under ^0.0.3) and deduplicates the range rule shared by two verify scripts into one zero-dependency helper — correct, verified, and in the grain; one dead export is the only nit.

  • What it does: Adds scripts/lib/peer-range.mjs with two exported rules: expectedPeerRange derives the peer-range shape from the dependency's own version (^X.Y.Z from 1.0.0, the >=X.Y.Z <X.Y+1.0 window below it), and caretAdmits applies npm's caret upper bound (major+1 for 1.x, minor+1 for 0.x, patch+1 for 0.0.z) while comparing release parts so a prerelease of an admitted version counts. verify-package.mjs and v
  • Goals it achieves: Makes the release guard refuse what npm refuses: I executed the old inline logic against the new module and confirmed old=admit/new=refuse for ^0.9.0|0.10.0 and ^0.0.3|0.0.4, and that the old split('.') guard passed prerelease versions via the NaN-comparison accident (it only throws when floor > installed, and any NaN comparison is false). Second goal is one definition of the rule: the prior commi
  • Assessment: Good on its merits. It is a direct, proportionate follow-up to the guard that 662c874 introduced hours earlier (git log shows both scripts churned there), it keeps the scripts' deliberate zero-external-dependency grain (only node: builtins plus this local lib), and every behavioral claim in the PR body reproduced when I executed both implementations. The doc comments state the npm pre-1.0 caret ru
  • Better / existing approach: none — this is the right approach. Searched for alternatives: (1) the semver package — package.json has no semver dependency and these scripts use only node builtins; npm's semver.satisfies would also REJECT prereleases of admitted versions ('1.2.0-rc.1' fails ^1.0.0 without includePrerelease), which contradicts the guards' documented one-physical-copy intent, so the hand-rolled subset (two asse
  • 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 demonstrated mis-admission in the release guard (pre-1.0 carets and prerelease parses) and consolidates the duplicated rule into one shared module that both CI and publish gates run, behavior-neutral at today's pins.

  • Integration: Fully reachable now: scripts/lib/peer-range.mjs is imported by both guards (scripts/verify-package.mjs:16, scripts/verify-official-optimizers.mjs:13), which run via verify:package/verify:official-optimizers (package.json:74-75) in .github/workflows/ci.yml:40,63 and .github/workflows/publish.yml:43,46. I executed the module directly: all 11 proof-table rows in the PR body reproduce, and expecte
  • Fit with existing patterns: Consolidates, not competes: verify-package.mjs carried private exactMinorPeerRange/caretPeerRange copies and verify-official-optimizers.mjs hardcoded which package got which shape; both now use one definition. The scripts/lib shared-helper shape matches the sibling agent-runtime helper the PR body cites. Hand-rolling the 30-line rule instead of npm's semver is defensible: semver is not a declare
  • Real-world viability: Holds past the happy path: unreadable versions throw loud in derivation, and that path is pre-gated by exactDevelopmentPin (verify-package.mjs:273-279) and regex pins (verify-official-optimizers.mjs:18,28), consistent with the repo's fail-loud doctrine; caretAdmits returns false on unreadable installed versions while the caller assertCaretAdmits (verify-package.mjs:283-292) throws the descriptive
  • Model: opencode/zai-coding-plan/glm-5.2
  • Bridge attempts: 1

🎯 Usefulness Audit

🟡 caretAdmits intentionally diverges from npm on prereleases [problem-fit] ``

caretAdmits('^1.0.0','1.2.0-develop.1') admits while npm semver.satisfies refuses without includePrerelease (verified by execution; rationale documented at peer-range.mjs:44-51). Correct for the single-copy assertion this guard performs, and installed versions in the verify flow are exact-pinned non-prereleases (verify-package.mjs:115-116), so it cannot misfire today. Reviewer should just know the predicate is deliberately not semver.satisfies; no action needed.

🟡 Pre-1.0 caret branches are ahead of any current caller [integration] ``

caretUpperBound's 0.x branches (peer-range.mjs:39-41) only fire on a declared '^0.x' range, and both cohort checks target agent-interface, now 1.0.0 (package.json:83), while expectedPeerRange never emits a pre-1.0 caret itself. Plausibly reachable if a future cohort declares a caret on a pre-1.0 contract package; it is the complete 4-line statement of npm's rule rather than dead surface, and trimming it would reintroduce silent mis-admission if the data changes. Informational only.

💰 Value Audit

🟡 exactMinorPeerRange is exported with no external consumer [maintenance] ``

scripts/lib/peer-range.mjs:28 exports exactMinorPeerRange, but the only caller is expectedPeerRange at line 34; both verify scripts import only caretAdmits and expectedPeerRange (verify-package.mjs:16, verify-official-optimizers.mjs:13). Drop the export keyword (or the function, inlining the template) so the module surface is exactly what consumers use. Does not gate shipping.


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 · 20260816T180935Z

@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 1 (1 weak-concern)
Heuristic 0.0s
Duplication 0.0s
Interrogation 170.9s (2 bridge agents)
Total 170.9s

💰 Value — sound-with-nits

Fixes a verified correctness bug in the release cohort guard (the old caret check admitted 0.10.0 under ^0.9.0 — the second-copy shape the guard exists to refuse) and consolidates the rule into one zero-dependency module shared by both verify scripts; ship.

  • What it does: Creates scripts/lib/peer-range.mjs with one definition of the peer-range rule and rewires both release guards to import it. Two behavior deltas, both confirmed by executing old and new side by side: (1) assertCaretAdmits in scripts/verify-package.mjs:283 previously read any caret as 'same major, floor <= installed', which admitted 0.10.0 under ^0.9.0 and 0.0.4 under ^0.0.3 (old=true, new=false on
  • Goals it achieves: The verify scripts enforce 'one installed copy of each contract package' (assertSingleInstalledAgentStack, scripts/verify-package.mjs:314); a caret check that admits a version npm refuses lets a cohort declaration satisfy the string check while npm resolves a divergent second copy — the exact failure the guard exists to catch. Second, the same expected-range rule previously lived in three shapes a
  • Assessment: Good on its merits. The bug is real and measured (I re-executed both implementations; the old one admits the exact second-copy shapes npm refuses), the fix is the smallest correct one, and the consolidation is a strict reduction in in-repo duplication. The zero-dependency, node-builtins-only implementation fits the grain of standalone release scripts that bootstrap temp npm installs and must not d
  • Better / existing approach: none — this is the right approach. Alternatives examined: (a) the npm 'semver' package — not a direct dependency (only transitive inside pnpm-lock.yaml:2031; require from the scripts fails, verified), so using it means adding a devDependency to deliberately dependency-free release scripts, and semver.satisfies refuses prereleases the change explicitly wants admitted, so it is not a drop-in; (b) ex
  • Model: opencode/zai-coding-plan/glm-5.2
  • Bridge attempts: 2
  • Bridge warning: opencode/kimi-for-coding/k2p7: opencode: opencode error

🎯 Usefulness — sound

Consolidates two divergent inline copies of the peer-range rule into scripts/lib/peer-range.mjs and fixes the release guard to apply npm's pre-1.0 caret semantics, wired into both CI and the publish gate where the pre-1.0 branch runs live today (agent-eval 0.145.21).

  • Integration: Fully reachable and exercised on every run. verify-package.mjs:16 and verify-official-optimizers.mjs:13 import the new module; those scripts run via pnpm verify:package and pnpm verify:official-optimizers (package.json:74-75), which CI invokes at .github/workflows/ci.yml:40,63 and the publish workflow gates on at .github/workflows/publish.yml:43,46. The pre-1.0 branch of expectedPeerRange is n
  • Fit with existing patterns: Fits the grain and removes duplication rather than adding a competing pattern. Before this PR the same rule existed in two divergent copies (verify-package.mjs's exactMinorPeerRange/caretPeerRange, and the inline computation in verify-official-optimizers.mjs:20-31 that had already drifted to a version-conditional). No existing equivalent competes: the repo has no semver dependency (grep for `fro
  • Real-world viability: Holds up beyond the happy path. Inputs to expectedPeerRange are pre-validated as exact x.y.z pins (exactDevelopmentPin at verify-package.mjs:273-279, regex checks at verify-official-optimizers.mjs:18,28), so readVersion's throw path is unreachable from real callers; caretAdmits's inputs are real published cohort ranges, which are plain ^x.y.z carets, and assertCaretAdmits (verify-package.mjs:283
  • Model: opencode/zai-coding-plan/glm-5.2
  • Bridge attempts: 1

💰 Value Audit

🟡 Two exports have no consumers outside the module [maintenance] ``

exactMinorPeerRange and caretUpperBound (scripts/lib/peer-range.mjs:28,38) are exported but rg across the repo shows no importer — both are only called internally by expectedPeerRange and caretAdmits, and no test file exists for the module. Dropping the export keyword on each would keep the public surface to what the two verify scripts actually consume. Does not gate shipping.


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 · 20260816T180956Z

…umers

exactMinorPeerRange and caretUpperBound are called only from inside the
module. The two verify scripts import expectedPeerRange and caretAdmits.
@drewstone

Copy link
Copy Markdown
Contributor Author

@tangletools review now

@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

Verdict sound
Coverage 2 of 2 lenses (value, usefulness)
Concerns 0 (none)
Heuristic 0.0s
Duplication 0.1s
Interrogation 138.0s (2 bridge agents)
Total 138.1s

💰 Value — sound

Centralizes the peer-range rule into a shared zero-dep helper and fixes the shipped caret guard to match npm's pre-1.0 semantics — a real correctness fix plus needed deduplication; ship.

  • What it does: Extracts the peer-range rule that lived as two divergent inline copies (scripts/verify-package.mjs:280-292 and scripts/verify-official-optimizers.mjs:18-35 pre-change) into scripts/lib/peer-range.mjs: expectedPeerRange derives the range a dependency earns from its own version (caret from 1.0.0, next-minor window below 1.0.0), and caretAdmits applies npm's actual caret admission rule, including
  • Goals it achieves: Three: (1) the package-verification guard refuses the second-copy shapes it exists to refuse — the old assertCaretAdmits admitted 0.10.0 under ^0.9.0 and 0.0.4 under ^0.0.3, versions npm's resolver would not install under those ranges, so the guard understated real incompatibility; (2) prerelease installed versions no longer pass by accident of NaN comparison; (3) one definition of the rule shared
  • Assessment: Good on its merits. The defect is real and measured, the fix is correct by execution, and the shape fits the codebase: the verify scripts are deliberately dependency-free Node run via node scripts/*.mjs (package.json verify scripts), the new scripts/lib/ module keeps that property, and the two consumers now import one definition instead of carrying copies. The PR body notes this mirrors the help
  • Better / existing approach: none — this is the right approach. Searched for alternatives: (a) src/ and scripts/ contain no existing version-range logic to reuse (grep for satisfies/caret/peerRange hits only unrelated TypeScript satisfies keyword uses); (b) npm's semver package could do this, but it is not a declared devDependency here (only transitive via publint/attw per pnpm-lock.yaml), and importing an undeclared pack
  • Model: opencode/zai-coding-plan/glm-5.2
  • Bridge attempts: 2
  • Bridge warning: opencode/kimi-for-coding/k2p7: opencode: opencode error

🎯 Usefulness — sound

A correct fix to a live release gate — it repairs npm's pre-1.0 caret semantics the guard previously got wrong, and consolidates two divergent copies of the rule into one module both verify scripts import; verified executing and wired into CI and the publish workflow.

  • Integration: Fully reachable now. Both exports have live callers: expectedPeerRange at scripts/verify-package.mjs:70,72 and scripts/verify-official-optimizers.mjs:21,31; caretAdmits at scripts/verify-package.mjs:287 (via assertCaretAdmits at lines 172 and 182). Both scripts are npm-script wired (package.json:74-75 verify:package, verify:official-optimizers), run in CI (.github/workflows/ci.yml:40,63) and a
  • Fit with existing patterns: Fits the grain. It removes two divergent per-script copies (the exact bug source: verify-official-optimizers.mjs had the narrow rule, verify-package.mjs had the wrong wide one) into one scripts/lib module, mirroring the helper shape agent-runtime adopted (scripts/lib/packed-package-test.mjs per the PR body). No competing implementation exists in this repo — semver is not a dependency (grep of pa
  • Real-world viability: Holds up beyond the happy path. Every caller pre-validates the exact-pin form (^\d+.\d+.\d+$ at verify-package.mjs:275 and verify-official-optimizers.mjs:18,28) before expectedPeerRange can throw on a malformed version, and caretAdmits fails closed (returns false on non-caret ranges and unparseable versions) with the throw-preservation left to assertCaretAdmits' explicit non-caret error. The one
  • Model: opencode/zai-coding-plan/glm-5.2
  • Bridge attempts: 1

No concerns — sound change, no better or existing approach found. ✅


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 · 20260816T181321Z

@tangletools

Copy link
Copy Markdown
Contributor

✅ No Blockers — c17e60d4

Review health 100/100 · Reviewer score 74/100 · Confidence 75/100 · 15 findings (1 medium, 14 low)

opencode GLM 5.2 opencode DeepSeek v4 Pro opencode DeepSeek v4 Flash aggregate
Readiness 74 82 77 74
Confidence 75 75 75 75
Correctness 74 82 77 74
Security 74 82 77 74
Testing 74 82 77 74
Architecture 74 82 77 74

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

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

🟠 MEDIUM expectedPeerRange omits the 0.0.z patch-lock rule, contradicting caretUpperBound and the docstring — scripts/lib/peer-range.mjs

exactMinorPeerRange('0.0.5') returns '>=0.0.5 <0.1.0', which admits 0.0.6 through 0.0.9. npm's caret rule (and this module's own caretUpperBound, which returns [0,0,6] for [0,0,5]) says ^0.0.5 admits only 0.0.5 (upper bound <0.0.6). The docstring at line 8 states 'a 0.0.z caret to its patch' but the code only implements the 0.x -> next-minor case. Impact: for any pre-1.0 dependency at 0.0.z, expectedPeerRange would generate an over-permissive peer range that admits patch versions npm would never install, silently weakening the exact-copy invariant this module exists to enforce. Not currently triggered (agent-eval is 0.145.21, agent-interface 1.0.0). Fix: make exa

🟡 LOW New module has no unit tests; the fixed bug was invisible to the existing integration guard — scripts/lib/peer-range.mjs

grep of tests/ finds no coverage of peer-range.mjs. The commit message documents that the prior guard admitted 0.10.0 under ^0.9.0 and passed prerelease versions via NaN — a real logic error that CI's verify:package job never caught because it only checks the current cohort, which has always satisfied both the old and new rules. The npm caret expansion table (1.x => <major+1.0, 0.x => <minor+1.0, 0.0.x => <patch+1.0, plus prerelease refusal) is exactly the kind of boundary table that should be pinned in a unit test. Adding a small vitest file for caretAdmits/expectedPeerRange/caretUpperBound would have caught both the old bug and the 0.0.z divergence above.

🟡 LOW No direct unit tests for the pure caret-range logic — scripts/lib/peer-range.mjs

peer-range.mjs is fully pure and encodes an external spec (npm semver caret admission), yet has zero unit tests; grep found no .test. covering it. It is exercised only via verify-package.mjs / verify-official-optimizers.mjs, which require a real npm install of the full agent stack (network-dependent, slow). This is why the 0.0.z inconsistency above went unnoticed. Recommend a small vitest file asserting readVersion, caretUpperBound, caretAdmits, and expectedPeerRange against the known caret cases (^1.2.3, ^0.2.3, ^0.0.3) and the prerelease-admission decision.

🟡 LOW No unit coverage for the pre-1.0 caret boundary logic — scripts/lib/peer-range.mjs

The 0.0.z -> patch-bump and 0.x -> minor-bump rules in caretUpperBound and the >=1 caret/minor flip in expectedPeerRange are pure functions with no test file (no matches for expectedPeerRange/caretAdmits/exactMinorPeerRange under tests/). Their only exercisers are verify-package.mjs and verify-official-optimizers.mjs, which require network/registry access and run on the release path, so a regression in the 0.0.z rule would be caught only during a release. A small table-driven unit test would pin the npm-rule boundaries the commit exists to encode.

🟡 LOW Prerelease admission deviates from npm satisfies (documented, intentional) — scripts/lib/peer-range.mjs

readVersion strips prerelease/build suffixes and compareVersion uses only the release part, so caretAdmits('^1.0.0','1.5.0-beta.2') returns true (verified by execution) while node-semver satisfies without includePrerelease returns false. The doc comment states this is deliberate for the single-physical-copy guard purpose, and the dangerous direction is still closed: a next-major prerelease like 2.0.0-beta is rejected because its release part [2,0,0] is not < upper bound [2,0,0]. Acceptable as designed; noting so future callers don't reuse caretAdmits as a general npm-range predicate.

🟡 LOW compareVersion silently ignores components beyond left's length — scripts/lib/peer-range.mjs

The loop iterates left.length only, so a shorter left against a longer right compares a prefix and can report equality for unequal versions. Every current call site passes fixed 3-tuples from readVersion/caretUpperBound, so there is no live bug; the coupling is implicit. A length assertion or shared tuple type would make the invariant explicit.

🟡 LOW exactMinorPeerRange is wider than npm's caret for a 0.0.z version — scripts/lib/peer-range.mjs

For version 0.0.3 the function returns '>=0.0.3 <0.1.0' (major=0, minor=0 => <0.1.0), but npm's caret for 0.0.z is '>=0.0.3 <0.0.4' — the module's own docstring (line 7) states 'a 0.0.z caret to its patch'. Verified by execution: expectedPeerRange('0.0.3') === '>=0.0.3 <0.1.0'. This is also internally inconsistent: caretAdmits('^0.0.3','0.0.9') correctly returns false, so the generated peer range would admit (as a peer floor) versions caretAdmits refuses. Latent only — current pins are 0.145.21 and 1.0.0 — but a future 0.0.x pin on either dependency would silently generate an over-broad peer range. Fix: special-case minor===0 in exactMinorPeerRange to emit '<0.0.

🟡 LOW Expected agent-eval peer range widens from minor-bounded to caret when the pin reaches 1.0.0 — scripts/verify-official-optimizers.mjs

Old inline formula always produced a minor-bounded range (e.g. pin 1.2.3 -> '>=1.2.3 <1.3.0'); expectedPeerRange(1.2.3) returns '^1.2.3' (major-bounded, verified by execution). For the current pin 0.145.21 the output is identical, so nothing changes today. But the day agent-eval ships 1.0.0, this guard silently starts accepting peer ranges that admit any later 1.x minor instead of only the pinned minor — a loosening that will not be visible in that day's diff. Intentional per the module's documented versioning doctrine (npm caret semantics from 1.0.0), so not a defect; consider having the release checklist note that an agent-eval major-bump review should re-affirm the wider cohort, or pin the eval expectation to exactMinorPeerRange explicitly if the stricter single-copy guarantee is still

🟡 LOW Shared peer-range boundary semantics have no direct tests — scripts/verify-official-optimizers.mjs

The import moves the range-shape rule into scripts/lib/peer-range.mjs, which rg shows is referenced only by this script, verify-package.mjs, and package.json — no unit test covers caretUpperBound's 0.0.z branch, the prerelease-tolerant readVersion, or caretAdmits. This script's strict pre-check (line 18/28) shields it from the unparseable-version throw, but the admission logic used by verify-package.mjs relies on the same untested edges. A small vitest file over the pure module would lock the npm-rule fix this PR ships. Cosmetic for this shot since the file's own behavior is verified end-to-end by the peer check passing.

🟡 LOW expectedPeerRange over-admits for 0.0.z pins relative to npm caret — scripts/verify-official-optimizers.mjs

This file now trusts expectedPeerRange() (scripts/lib/peer-range.mjs:33). For a pre-1.0 pin at patch level, e.g. '0.0.5', it returns '>=0.0.5 <0.1.0', but npm's caret for ^0.0.5 is '>=0.0.5 <0.0.6' (patch-locked). The module docstring even describes npm locking 0.0.z to patch yet returns the minor bound. No current pin hits this path (agent-eval is 0.145.21), so no current impact; a future 0.0.z devDependency pin would derive a peer range broader than npm's own rule. Fix if the module is ever pinned to 0.0.z: special-case minor === 0 to <0.0.(patch+1).0.

🟡 LOW Corrected caret rule has no direct test; only exercised via live-registry verify runs — scripts/verify-package.mjs

The commit's purpose is the pre-1.0 caret fix, but the shared helper (lib/peer-range.mjs) has no unit test and assertCaretAdmits has no in-script self-check, unlike the retained assertEdgeUnsafeStaticImportMatcher (lines 372-387) which locks down its own regex semantics. The npm caret boundary (0.x locks to minor, 0.0.z locks to patch, prerelease admission) is now verified only when the script runs against the real registry, so a future regression of these rules would surface only at release time. Add a small self-check or a vitest unit for caretAdmits/caretUpperBound.

🟡 LOW New peer-range helpers have no direct unit or self-test coverage — scripts/verify-package.mjs

The script self-tests its regex matcher via assertEdgeUnsafeStaticImportMatcher (line 372) but has no equivalent table-driven self-test for caretAdmits/expectedPeerRange, and tests/ contains no peer-range tests (grep confirmed). Coverage is indirect via verify:package in ci.yml/publish.yml, which only exercises the happy path for the currently pinned versions, so a regression in the 0.x-boundary logic would not fail any test until a cohort actually ships a pre-1.0 caret. Cheap fix: mirror the matcher self-test pattern with the boundary cases now encoded in the lib.

🟡 LOW assertCaretAdmits accepts prerelease versions that npm would not install under the caret — scripts/verify-package.mjs

caretAdmits('^1.0.0', '1.0.0-beta.1') returns true (verified by execution) because readVersion strips the prerelease and compares only the release triple; npm semver excludes prereleases from a non-prerelease caret range. The lib documents this as a deliberate single-physical-copy rule, and both call sites (lines 172-176, 182-186) compare ranges and versions that npm itself co-resolved in the temp app, so a false-admit cannot arise from a real install graph. Impact today is nil; risk is only that a future caller trusts the helper as a general npm-semver oracle. If that caller appears, gate prereleases on the range also carrying a prerelease tag.

🟡 LOW exactMinorPeerRange is now an unused export — scripts/verify-package.mjs

After this change both verify scripts call expectedPeerRange, so exactMinorPeerRange (lib/peer-range.mjs:28) has no remaining callers in the repo (grep confirms only its own export). Retaining it invites two sources of truth for the same range shape. Either fold its single line into expectedPeerRange or delete the export.

🟡 LOW expectedPeerRange emits minor-lock for a 0.0.z pin, wider than npm's caret and than its own doc comment — scripts/verify-package.mjs

verify-package.mjs now derives the agent-eval peer range from expectedPeerRange (imported at line 16). For a 0.0.z pin the helper returns exactMinorPeerRange -> '>=0.0.z <0.1.0', but npm's caret rule for ^0.0.z is '>=0.0.z <0.0.(z+1)' and lib/peer-range.mjs:7-8 states 'a 0.0.z caret to its patch'. So the verified peer range would admit cross-patch versions npm would not. Proven by executing expectedPeerRange('0.0.3') -> '>=0.0.3 <0.1.0' vs caretUpperBound([0,0,3]) -> [0,0,4]. No current impact: agent-eval is 0.145.21 (minor-lock is correct there) and agent-interface is 1.0.0, and the output equals the pre-PR script for both. Latent mismatch only if a 0.0.z depen


tangletools · 2026-08-16T18:14:00Z · trace

@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 — 15 non-blocking findings — c17e60d4

Full multi-shot audit completed 3/3 planned shots over 3 changed files. Global verifier still owns final merge decision. | Full multi-shot audit completed 3/3 planned shots over 3 changed files. Global verifier still owns final merge decision. | Full multi-shot audit completed 3/3 planned shots over 3 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-16T18:14:00Z · immutable trace

@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

Verdict sound
Coverage 2 of 2 lenses (value, usefulness)
Concerns 0 (none)
Heuristic 0.0s
Duplication 0.1s
Interrogation 190.3s (2 bridge agents)
Total 190.4s

💰 Value — sound

Extracts one shared, npm-accurate peer-range rule into scripts/lib/peer-range.mjs, fixing a real admission bug (old guard admitted 0.10.0 under ^0.9.0) and de-duplicating the rule across two verify scripts — right-sized and in-grain; ship.

  • What it does: Adds scripts/lib/peer-range.mjs with two exports: expectedPeerRange(version), which derives the peer range shape from the dependency's own version (caret from 1.0.0, exact-minor window below it), and caretAdmits(range, version), which applies npm's pre-1.0 caret rule (0.x locked to minor, 0.0.z locked to patch). verify-package.mjs drops its two hand-rolled range builders and its buggy major-equali
  • Goals it achieves: Makes the release guard actually refuse what npm refuses: the old assertCaretAdmits admitted 0.10.0 under ^0.9.0 and 0.0.4 under ^0.0.3 — exactly the second-copy shape the guard exists to catch. Second, the peer-range shape is now derived from the pinned version instead of hardcoded per package, so when agent-eval (currently 0.145.21, package.json:89) crosses 1.0.0 the expected range flips to care
  • Assessment: Good on its merits. The bug is real and proven in the PR body against npm's semver.satisfies; the fix implements the correct upper-bound rule (caretUpperBound at scripts/lib/peer-range.mjs:38-42). The extraction direction matches the codebase's grain — a zero-dependency scripts/lib module consumed by the repo's verify scripts, mirroring the sibling agent-runtime layout. The head commit (b892f43) a
  • Better / existing approach: none — this is the right approach. Checked: (1) semver is not a dependency anywhere in package.json or the lockfile (rg '"semver"' found nothing), so importing it would add a dependency for two comparisons in a CI guard — more machinery than value; (2) the similar helper in agent-runtime's scripts/lib/packed-package-test.mjs is a different repository, not importable here, and per-repo scripts/li
  • 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 hole in the release guard's core job — the old caret check admitted the pre-1.0 minor crossings npm refuses, which is exactly the second-copy shape the guard exists to block — and consolidates the rule into one dependency-free module both verify scripts already run in CI.

  • Integration: Fully reachable now. Both consumers import the new module: scripts/verify-package.mjs:16 (runs via pnpm verify:package in ci.yml:40 and publish.yml:43, i.e. on every push and every release) and scripts/verify-official-optimizers.mjs:13 (ci.yml:63). Both exports are used (expectedPeerRange at verify-package.mjs:70,72 and verify-official-optimizers.mjs:21,31; caretAdmits at verify-package.mjs:
  • Fit with existing patterns: Fits the grain. The repo's scripts are dependency-free plain-node .mjs and semver appears nowhere in scripts/, src, or package.json (grepped), so a 60-line hand-rolled triplet comparison is the established pattern, not a deviation; adding a semver devDep for two functions would be the heavier design. The extraction removes what were two divergent copies of the same rule (the diff shows verify-pa
  • Real-world viability: Executed the new module against npm's caret rule: ^0.9.0|0.10.0 refuse, ^0.0.3|0.0.4 refuse, ^1.0.0|2.0.0 refuse, floors admitted — the two admission bugs in the PR table reproduce as fixed (the old code at verify-package.mjs compared major-equality plus a numeric floor only, so 0.10.0 and 0.0.4 slipped through, and split('.') on 1.0.0-rc.1 produced NaN). Error paths fail closed: `readVe
  • Model: opencode/zai-coding-plan/glm-5.2
  • Bridge attempts: 1

No concerns — sound change, no better or existing approach found. ✅


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 · 20260816T181423Z

@tangletools

Copy link
Copy Markdown
Contributor

⚠️ Review Incomplete — b892f431

At least one required reviewer lane failed closed. No approval or request-changes review was published. This is a reviewer run failure, not a PR quality score.

Trigger a fresh review on the current PR head.

tangletools · 2026-08-16T18:18:53Z

@drewstone

Copy link
Copy Markdown
Contributor Author

@tangletools review now

@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

Verdict sound
Coverage 2 of 2 lenses (value, usefulness)
Concerns 0 (none)
Heuristic 0.0s
Duplication 0.1s
Interrogation 152.7s (2 bridge agents)
Total 152.8s

💰 Value — sound

Fixes a release guard that mis-applied npm's caret rule to pre-1.0 dependencies and consolidates the duplicated range rule from two verify scripts into one 66-line dependency-free module — coherent, correct, and in the codebase's grain.

  • What it does: Extracts the peer-range rule shared by scripts/verify-package.mjs and scripts/verify-official-optimizers.mjs into scripts/lib/peer-range.mjs with two exports: expectedPeerRange(version) (caret from 1.0.0, exact-minor window below it) and caretAdmits(range, version), which applies npm's true caret upper bound — next major above 1.0, next minor for 0.x, next patch for 0.0.z — instead of the old 'sam
  • Goals it achieves: Makes the single-installed-copy guarantee real: the guard assertCaretAdmits (verify-package.mjs:283) previously admitted a cohort declaring ^0.9.0 while 0.10.0 was installed — the exact second-copy shape assertSingleInstalledAgentStack (verify-package.mjs:314) exists to refuse — because pre-1.0 carets stop at the next minor under npm. Secondarily, it removes the drift risk of the same rule living
  • Assessment: Good on its merits. The rule is correct per npm/semver semantics (the PR body's table matches the code in caretUpperBound), it preserves the deliberate policy that a prerelease of an admitted version counts as admitted (documented at peer-range.mjs:44-51 — a guard on physical copy count, not semver prerelease precedence), and the module's shape is proportionate: zero dependencies, node-builtin-onl
  • Better / existing approach: none — this is the right approach. Searched: (1) existing in-repo semver/range logic via grep for caret|peerRange|major.*minor.*patch|version-regex patterns across src/ and scripts/ — nothing outside the two copies this PR removes; (2) the npm semver package as a possible reuse — it appears only as a transitive dependency (pnpm-lock.yaml:2031,2423,3697), not a direct one, and these release scrip
  • Model: opencode/zai-coding-plan/glm-5.2
  • Bridge attempts: 2
  • Bridge warning: opencode/kimi-for-coding/k2p7: opencode: opencode error

🎯 Usefulness — sound

A verified correctness fix to a release guard that runs on every CI run and publish: it replaces two inline copies of a mis-implemented caret rule with one shared module whose behavior matches npm's semver exactly, and the derived expectations still match the live package.json peers so nothing break

  • Integration: Fully reachable now. scripts/lib/peer-range.mjs is imported by verify-package.mjs:16 and verify-official-optimizers.mjs:13; both scripts run via the verify:package and verify:official-optimizers npm scripts (package.json:74-75), which CI invokes on every push (.github/workflows/ci.yml:40,63) and publish.yml gates the release on (lines 43-46,61). I executed the shipped module directly: all 11 rows
  • Fit with existing patterns: Fits the grain. It consolidates two per-script copies of the same rule (exactMinorPeerRange/caretPeerRange in verify-package.mjs and the inline range template strings in verify-official-optimizers.mjs) into one module under scripts/lib/, a natural shared location. No semver library exists anywhere in this repo (grep for 'semver' imports: zero matches), so a 66-line zero-dependency helper matches t
  • Real-world viability: Holds up beyond the happy path. Single-threaded CI/release context, so no concurrency surface. Malformed input fails loud on the derivation path (readVersion throws 'cannot read version X') and fails closed on the admission path (caretAdmits returns false, producing the assertion error), which matches the repo's stated no-fallbacks doctrine. Prerelease and build suffixes are handled deliberately (
  • Model: opencode/zai-coding-plan/glm-5.2
  • Bridge attempts: 1

No concerns — sound change, no better or existing approach found. ✅


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 · 20260816T182834Z

@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

Verdict sound
Coverage 2 of 2 lenses (value, usefulness)
Concerns 0 (none)
Heuristic 0.0s
Duplication 0.1s
Interrogation 194.4s (2 bridge agents)
Total 194.5s

💰 Value — sound

Fixes a real correctness bug in the release peer-range guard (pre-1.0 carets were admitted too widely, prerelease versions parsed to NaN) and consolidates two drifting copies of the rule into one shared module — verified correct and behavior-preserving against the live pins; ship.

  • What it does: Extracts the peer-range rule into scripts/lib/peer-range.mjs with two exports: expectedPeerRange (caret from 1.0.0, >=X <next-minor below it, matching npm's pre-1.0 caret desugaring) and caretAdmits (admission under npm's caret upper bound, with release-part-only comparison so prereleases of admitted versions count). Both verify-package.mjs (lines 16, 70-72, 283-292) and verify-official-opti
  • Goals it achieves: The guard's stated purpose (verify-package.mjs:281-292, assertSingleInstalledAgentStack at 314) is to refuse a second installed copy of a contract package across the agent stack. Under the old rule, ^0.9.0 admitting 0.10.0 is exactly the two-copies shape the guard exists to refuse, so the guard was structurally unable to catch one of its own failure modes for pre-1.0 packages — and agent-eval
  • Assessment: Good change, verified. I executed the checked-out module: the full admission table from the PR body passes (^0.9.0|0.10.0 refuse, ^0.0.3|0.0.4 refuse, ^1.0.0|1.2.0-develop.1 admit), and expectedPeerRange('0.145.21') -> >=0.145.21 <0.146.0 and ('1.0.0') -> ^1.0.0, byte-identical to the currently shipped peers (package.json:82-83) — so the fix tightens the rule without perturbing the l
  • Better / existing approach: none — this is the right approach. Searches run: (1) grep for semver across scripts/ and src/ and package.json — no hit; the semver package is not a dependency of this repo and scripts/ is node-stdlib-only, so importing it would add a runtime dep for two narrow functions and fight the grain. (2) grep for other version-parsing (split('.')) logic that could be reused — none outside the removed
  • Model: opencode/zai-coding-plan/glm-5.2
  • Bridge attempts: 2
  • Bridge warning: opencode/kimi-for-coding/k2p7: opencode: opencode error

🎯 Usefulness — sound

A correct, verified fix that collapses two divergent copies of npm's caret rule into one shared module, wired into both CI and the publish gate, with behavior cross-checked against npm's own semver.

  • Integration: Fully reachable now. Both exports have live call sites: expectedPeerRange at scripts/verify-package.mjs:70,72 and scripts/verify-official-optimizers.mjs:21,31; caretAdmits at scripts/verify-package.mjs:287. Both consumer scripts are wired into package.json:74-75 (verify:package, verify:official-optimizers) and run in .github/workflows/ci.yml:40,63 plus as the publish gate in .github/workflows/publ
  • Fit with existing patterns: Fits the grain. Before this PR the same rule lived in three inline variants (exactMinorPeerRange and caretPeerRange in verify-package.mjs; two hand-built template strings in verify-official-optimizers.mjs), and they disagreed — the interface path returned an unconditional caret even for pre-1.0 versions. The PR replaces all of them with one definition in a new scripts/lib/, which is the convention
  • Real-world viability: Holds up beyond the happy path, verified by execution. I imported the new module and ran all 11 admission cases from the PR body — 11/11 pass — and cross-checked the contested pre-1.0 semantics against npm's own semver CLI: 0.10.0 fails ^0.9.0, 0.0.4 fails ^0.0.3, 0.146.0 fails ^0.145.21, while 0.9.3 and 1.4.2 admit — exactly matching caretAdmits. The current repo pins (agent-eval 0.145.21, agent-
  • Model: opencode/zai-coding-plan/glm-5.2
  • Bridge attempts: 1

No concerns — sound change, no better or existing approach found. ✅


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 · 20260816T182908Z

@tangletools

Copy link
Copy Markdown
Contributor

✅ No Blockers — c17e60d4

Review health 100/100 · Reviewer score 73/100 · Confidence 75/100 · 5 findings (1 medium, 4 low)

rescue-deepseek-flash: Correctness 73 · Security 73 · Testing 73 · Architecture 73

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

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

🟠 MEDIUM No unit tests for the npm-caret boundary logic — scripts/lib/peer-range.mjs

peer-range.mjs is the release guard's semver core, but no test file exercises it. Coverage is only transitive: verify:package / verify:official-optimizers run in CI against the single live pin (agent-eval 0.145.21), which never hits the 0.0.x patch-lock branch or the exclusive-boundary reject cases (0.3.0 under ^0.2.3, 0.0.4 under ^0.0.3). The module is pure and table-testable; a vitest table covering the three caret regimes (1.x, 0.x, 0.0.x) plus boundary-exact versions would lock the behavior the old same-major check got wrong.

🟡 LOW 0.0.z peer range diverges from npm's caret rule and from caretAdmits — scripts/lib/peer-range.mjs

For a 0.0.z pin, expectedPeerRange -> exactMinorPeerRange emits '>=0.0.z <0.1.0' (next minor), but npm's caret for 0.0.z caps at <0.0.z+1, and this file's own caretUpperBound([0,0,z]) returns [0,0,z+1]. So the peer-range generator and the admission checker disagree for 0.0.z, and the commit title 'apply npm's caret rule' is only true for 0.x (x>0). The top-of-file doc explicitly acknowledges npm's patch lock and states next-minor is the chosen policy, so this is documented intent, not an accident — but the file should either use the same rule in both helpers or note the divergence on expectedPeerRange itself. No current dependency is 0.0.z (agent-eval is 0.145.21), so no live impact. Fix: route exactMinorPeerRange through caretUpperBound or add a one-line note.

🟡 LOW caretAdmits admits prereleases npm would not install — scripts/lib/peer-range.mjs

readVersion strips the prerelease/build suffix, so caretAdmits('^1.2.3', '1.2.4-beta.1') returns true, whereas npm/semver refuses to admit a prerelease unless the range carries a prerelease on the same tuple. The doc comment (lines 44-51) declares this deliberate ('a prerelease build of that copy speaks the same surface'), and it is currently unreachable: both callers validate pins as stable /^\d+.\d+.\d+$/ (verify-package.mjs exactDevelopmentPin and assertCaretAdmits, verify-official-optimizers.mjs:18), so the guard never compares a prerelease. Informational; a one-line note that this intentionally diverges from npm's prerelease rule is already present.

🟡 LOW peer-range helper has no direct unit tests — scripts/verify-official-optimizers.mjs

The import introduces a dependency on scripts/lib/peer-range.mjs, a new semver-rule module with subtle pre-1.0 caret semantics (0.x locked to minor, 0.0.z locked to patch). It is only exercised indirectly through the two release-time verification scripts, both of which are slow end-to-end (the official-optimizers script runs a full pnpm build + npm pack + pip install). The module is not covered by vitest and scripts/ is outside biome's include globs (src/, tests/), so a regression in caretUpperBound/compareVersion would only surface on release. Suggest adding focused unit tests for expectedPeerRange/caretAdmits table-driven cases. No current-cohort impact: verified outputs match the declared peers.

🟡 LOW Extracted semver-caret logic has no unit tests and the fixed upper-bound bug is not regression-tested — scripts/verify-package.mjs

This PR's substantive change is correctness: the old inline comparison (declaredMajor === major AND declaredMinor*1e6+patch <= installed) admitted an installed 0.146.0 under a declared ^0.145.0, because it enforced no upper bound. The new caretAdmits() in scripts/lib/peer-range.mjs fixes this (verified: caretAdmits('^0.145.0','0.146.0') === false), but neither the fix nor the three caret regimes (1.x minor-lock, 0.x minor-lock, 0.0.x patch-lock) nor the documented prerelease-release-part admission are pinned by any test. There is no test harness for scripts/ (package.json test script is vitest on src/tests; lint/typecheck also exclude scripts/), and CI's live verify:package (ci.yml:40) only exercises the current versions (0.145.21 -> exactMinor path, 1.0.0 -> caret path), never the upper-b


tangletools · 2026-08-16T18:31:02Z · trace

@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 — 5 non-blocking findings — c17e60d4

Full multi-shot audit completed 3/3 planned shots over 3 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-16T18:31:02Z · immutable trace

@tangletools

Copy link
Copy Markdown
Contributor

✅ No Blockers — b892f431

Review health 100/100 · Reviewer score 73/100 · Confidence 75/100 · 3 findings (1 medium, 2 low)

rescue-deepseek-flash verifier:deepseek-flash aggregate
Readiness 73 82 73
Confidence 75 88 75
Correctness 73 85 73
Security 73 95 73
Testing 73 50 50
Architecture 73 78 73

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

Full multi-shot audit completed 3/3 planned shots over 3 changed files. Global verifier still owns final merge decision. | The refactor is behavior-preserving for the live pins: I executed expectedPeerRange on package.json's 0.145.21 and 1.0.0 and both match the declared peers exactly, and the new caretAdmits is strictly tighter than the removed floor-only check (it now enforces the upper bound). The one substantive issue, the 0.0.z inconsistency between exactMinorPeerRange and caretUpperBound, is real (execution-confirmed) but unreachable today and fail-closed. No unit tests cover the new mod

🟠 MEDIUM exactMinorPeerRange disagrees with npm and with caretUpperBound for 0.0.z — scripts/lib/peer-range.mjs

For a 0.0.z floor, expectedPeerRange('0.0.3') returns '>=0.0.3 <0.1.0' (next minor). npm's caret rule, stated in the module docstring (lines 7-9) and implemented by caretUpperBound (lines 38-42), locks a 0.0.z caret to the next patch: ^0.0.3 = >=0.0.3 <0.0.4. The generated peer range is therefore wider than npm would admit, and the two functions in the same module encode conflicting rules for the same floor. Unreachable today (consumers call it only with agent-eval 0.145.21 and agent-interface 1.0.0), so no live impact, but it i

🟡 LOW No unit tests for the new caret-rule module — scripts/lib/peer-range.mjs

The PR adds scripts/lib/peer-range.mjs (66 lines) which hand-codes npm's caret semantics (pre-1.0 minor lock, 0.0.z patch lock, prerelease stripping) and wires it into two release guards, but adds no tests for it. The only self-check precedent in this script is assertEdgeUnsafeStaticImportMatcher (verify-package.mjs:372-387), which verifies its matcher inline. A table-driven test pinning npm's rules (1.x, 0.x, 0.0.x, prerelease, malformed input) would have caught the doc-vs-code inconsistency reported above.

🟡 LOW caretAdmits admits prereleases node-semver would exclude — scripts/lib/peer-range.mjs

readVersion strips the -prerelease/+build suffix, so caretAdmits('^1.0.0','1.0.0-beta.2') -> true, whereas node-semver's rule only admits a prerelease when the comparator's [major,minor,patch] tuple also carries one. The deviation is explicitly stated as deliberate in the comment (lines 47-51) and is unreachable in the actual guards, which always pass exact release pins. Recorded so the verifier knows the single-copy guarantee is exact-pin strong, not npm-resolution strong.


tangletools · 2026-08-16T18:45:45Z · trace

tangletools
tangletools previously approved these changes Aug 16, 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 — 3 non-blocking findings — b892f431

Full multi-shot audit completed 3/3 planned shots over 3 changed files. Global verifier still owns final merge decision. | The refactor is behavior-preserving for the live pins: I executed expectedPeerRange on package.json's 0.145.21 and 1.0.0 and both match the declared peers exactly, and the new caretAdmits is strictly tighter than the removed floor-only check (it now enforces the upper bound).

Full immutable report for this review: trace

Summary comment for this run: full summary


tangletools · 2026-08-16T18:45:45Z · immutable trace

@tangletools

Copy link
Copy Markdown
Contributor

⚠️ Review Interrupted — c17e60d4

The review runner stopped before publishing a final verdict: webhook_restarted.

State Detail
Interrupted webhook restarted

No review verdict was produced for this run. Trigger a fresh review on the current PR head if the PR is still open.

tangletools · #139 · model: kimi-for-coding · updated 2026-08-16T19:24:23Z

@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

Verdict sound
Coverage 2 of 2 lenses (value, usefulness)
Concerns 0 (none)
Heuristic 0.0s
Duplication 0.0s
Interrogation 70.9s (2 bridge agents)
Total 70.9s

💰 Value — sound

Fixes a real correctness hole in the release guard (pre-1.0 caret ranges admitted the second-copy shape the guard exists to refuse) and consolidates two per-script copies of the range rule into one zero-dependency helper — ship.

  • What it does: Extracts the peer-range rule from verify-package.mjs and verify-official-optimizers.mjs into scripts/lib/peer-range.mjs with two exports: expectedPeerRange(version) derives the range shape from the dependency's own version (caret from 1.0.0, an explicit >=x.y.z <x.(y+1).0 minor window below it), and caretAdmits(range, version) applies npm's actual caret rule including the pre-1.0 narrowing (^0.9.x
  • Goals it achieves: Two goals, both read from the code. (1) Correctness: verify-package.mjs exists to assert exactly one installed copy of each contract package (assertSingleInstalledAgentStack, verify-package.mjs:314), and assertCaretAdmits is the check that a cohort member's declared caret admits the pinned version — the old same-major floor admitted 0.10.0 under ^0.9.0 and let 1.0.0-rc.1 pass via a NaN from split(
  • Assessment: Good on its merits. The old code and the new helper disagree only in the direction of matching npm's documented semantics, the replacement is behavior-verified against the shipped manifest, and the extraction pattern (scripts/lib/, node-builtin-only imports, strict regexes on already-validated pin formats) matches how the rest of the scripts are written. The 66-line hand-rolled rule is proportiona
  • Better / existing approach: Considered the obvious alternative — npm's semver package (semver.satisfies) — and rejected it on evidence: 'semver' appears nowhere in package.json dependencies or devDependencies (grep of package.json found no entry; the src/ grep hits for 'satisfies' are the TypeScript keyword, e.g. src/memory/run.ts:254), no script in scripts/ imports any external package, and pulling a devDependency into stan
  • Model: opencode/zai-coding-plan/glm-5.2
  • Bridge attempts: 2
  • Bridge warning: opencode/kimi-for-coding/k2p7: opencode: opencode error

🎯 Usefulness — sound

A correctly-scoped fix that replaces two divergent copies of npm's caret rule with one shared, verified helper on the live CI/publish guard path; it matches npm's pre-1.0 semantics and current package.json state.

  • Integration: Fully reachable now, not ahead of a caller: both scripts import the new module (scripts/verify-package.mjs:16, scripts/verify-official-optimizers.mjs:13), both are wired as npm scripts (package.json:74-75) that run in CI (.github/workflows/ci.yml:40,63) and as the publish gate (.github/workflows/publish.yml:43,46). Every function the module exports has a consumer; nothing dead.
  • Fit with existing patterns: Improves fit rather than competing: before the change the rule lived in three places (exactMinorPeerRange + caretPeerRange in verify-package.mjs, inline range-building in verify-official-optimizers.mjs, removed at scripts/verify-official-optimizers.mjs:21-31 in the diff), and the two scripts enforced inconsistent shapes. Extraction to scripts/lib/ follows the repo's existing lib convention. Not a
  • Real-world viability: Executed the module directly: expectedPeerRange('0.145.21') = '>=0.145.21 <0.146.0' and expectedPeerRange('1.0.0') = '^1.0.0', both matching the current peerDependencies pins (package.json:82-83), so the guard passes today with no behavioral break; caretAdmits refuses 0.10.0 under ^0.9.0, refuses 0.0.4 under ^0.0.3, admits 0.9.3, admits a prerelease of an admitted version — matching the PR's claim
  • Model: opencode/zai-coding-plan/glm-5.2
  • Bridge attempts: 1

No concerns — sound change, no better or existing approach found. ✅


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 · 20260816T202156Z

@tangletools

Copy link
Copy Markdown
Contributor

✅ No Blockers — b892f431

Review health 100/100 · Reviewer score 70/100 · Confidence 75/100 · 12 findings (1 medium, 11 low)

opencode GLM 5.2 opencode DeepSeek v4 Flash aggregate
Readiness 77 70 70
Confidence 75 75 75
Correctness 77 70 70
Security 77 70 70
Testing 77 70 70
Architecture 77 70 70

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

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

🟠 MEDIUM No test guards the pre-1.0 admission fix this PR makes — scripts/lib/peer-range.mjs

The behavioral change (commit c17e60d) is that caretAdmits now rejects 0.10.0 under ^0.9.0 via caretUpperBound's minor-lock, where the old verify-package logic (same major + floor comparison) admitted it. But with the current cohort, every caretAdmits call passes agent-interface 1.0.0 against ranges like ^1.x.y (major>=1 branch), where old and new logic are byte-for-byte equivalent — so neither CI (ci.yml verify:package) nor any test can detect a regression of the fixed bug. No unit test exists for expectedPeerRange, caretAdmits, caretUpperBound, or readVersion's throw path. Fix: add a vitest unit test for scripts/lib/peer-range.mjs covering 0.9.0 vs 0.10.0, 0.0.z patch-lock, major-lock, prerelease admission, and malformed inputs.

🟡 LOW Caret-format guard duplicated across module and caller — scripts/lib/peer-range.mjs

verify-package.mjs:284 re-tests /^^(\d+).(\d+).(\d+)$/ before calling caretAdmits, which performs the same regex at peer-range.mjs:53. If the two ever drift, the caller's descriptive error ('must declare a caret range') can disagree with the admission result. Related minor API inconsistency: expectedPeerRange throws on malformed version while caretAdmits silently returns false for the same input; each is defensible at its call site but the asymmetry is undocumented. Consider exporting the format predicate and reusing it.

🟡 LOW Documented deviations from npm's exact caret rule are unguarded — scripts/lib/peer-range.mjs

Two deliberate departures from npm, both docstring'd but neither tested: (1) caretAdmits admits a prerelease of an admitted release (^1.2.3 admits 1.2.3-rc.1), whereas npm's real rule rejects prereleases unless the range itself carries one — intentional for the single-physical-copy guard but the module header says 'npm's rule', so the deviation is easy to misread; (2) expectedPeerRange('0.0.3') generates '>=0.0.3 <0.1.0' (next minor), broader than npm's caret '<0.0.4' (next patch). The 0.0.z case is currently unused (agent-eval is 0.145.x) and would be a silent cohort hole if a 0.0.z dependency ever joined.

🟡 LOW No direct unit tests; the fixed admission bug has no regression case — scripts/lib/peer-range.mjs

The repo runs vitest (package.json:66) with an extensive tests/ tree, but this pure module has no test file. CI covers it only indirectly through verify:package/verify:official-optimizers with the current pins, which exercise caretAdmits solely as ^1.0.0-vs-1.0.0 (verify-package.mjs:172-186) — neither the pre-1.0 admission case the PR fixes (^0.9.0 vs 0.10.0 must be false) nor the 0.0.z branch nor the prerelease tolerance run in CI. A guard module whose entire purpose is refusing the second-copy shape should have a table test pinning the three caret rules and their boundaries. I verified these behaviors manually (15 cases + 1728-pair semver@7 cross-check, 0 mismatches), so correctness is not in doubt — only future regression protection is missing.

🟡 LOW caretAdmits deliberately diverges from npm on prerelease admission — scripts/lib/peer-range.mjs

caretAdmits('^1.2.3','1.2.3-alpha.1') returns true while semver@7 satisfies() returns false (verified by execution). This is documented and intentional (lines 55-59: guards assert one physical copy of a contract package, and a prerelease of an admitted release speaks the same surface), and it is unreachable through current callers: verify-package.mjs passes agentInterfaceVersion, which exactDevelopmentPin (verify-package.mjs:273-279) constrains to /^\d+.\d+.\d+$/. Recorded so the global verifier knows the divergence from npm semantics is scoped and willful, not an oversight. Note the inverse also holds — readVersion silently drops prerelease/build metadata

🟡 LOW expectedPeerRange over-admits for a 0.0.z pin relative to the module's own admission rule — scripts/lib/peer-range.mjs

expectedPeerRange('0.0.3') returns '>=0.0.3 <0.1.0' (verified by execution), while caretUpperBound([0,0,3]) returns [0,0,4] — the same module's own doc (lines 5-6: 'npm locks a 0.x caret to its minor and a 0.0.z caret to its patch') and admission check treat 0.0.z patch bumps as breaking. So for a hypothetical 0.0.z dev pin, the peer range this package declares would admit 0.0.4+ that the admission side (and npm) would refuse, an internal asymmetry. Latent only: current pins are 0.145.21 and 1.0.0 (package.json:89-90) and versions move forward, so the branch is unreachable today. Fix if ever relevant: route major===0 && minor===0 to a patch-stop range (>=0.0.z

🟡 LOW Expected agent-eval peer range widens to caret when agent-eval reaches 1.0.0 — scripts/verify-official-optimizers.mjs

Old inline code always produced a minor-scoped range for agent-eval (>=x.y.z <x.(y+1).0), even at major >=1. The new expectedPeerRange returns caret ^x.y.z for any major >=1, so at agent-eval 1.0.0 the enforced range changes to >=1.0.0 <2.0.0. Verified: oldEval('1.0.0')='>=1.0.0 <1.1.0' vs new '^1.0.0'. This is a deliberate policy documented in scripts/lib/peer-range.mjs (minor-is-additive assumption) and is consistent with verify-package.mjs, so CI self-enforces the package.json update at that boundary. Flagging so the 1.0.0 release updates package.json peer to ^1.0.0 knowingly; no action needed now.

🟡 LOW Gate's accepted peer range loosens when agent-eval pin crosses 1.0 — scripts/verify-official-optimizers.mjs

Base computed >=${V} <${major}.${minor+1}.0 for agent-eval unconditionally; head delegates to expectedPeerRange, which returns ^${V} when major>=1 (scripts/lib/peer-range.mjs:34). At a hypothetical 1.5.0 pin the enforced peer changes from '>=1.5.0 <1.6.0' to '^1.5.0', admitting any later 1.x minor. This matches the PR's stated intent (caret rule from 1.0), so it is a policy change, not a bug — but the next agent-eval major bump will silently loosen the gate's requirement on package.json. Fix: none required; optionally note the 1.0 switch in the release checklist so the peer-range edit accompanying that bump is not a surprise.

🟡 LOW No unit coverage for the shared range helper this gate now depends on — scripts/verify-official-optimizers.mjs

grep for expectedPeerRange/caretAdmits across tests/ returns nothing and the PR adds no test file, so a regression in scripts/lib/peer-range.mjs would surface only when someone runs pnpm verify:official-optimizers or verify:package (full npm-pack + venv + vitest runs). Fix: add a small unit test for expectedPeerRange edge cases (0.x, 0.0.x, 1.x, malformed input) — naturally lands with the peer-range.mjs shot of this PR.

🟡 LOW peer-range.mjs has no direct unit tests — scripts/verify-official-optimizers.mjs

The new helper's branch points (pre-1.0 minor-lock vs post-1.0 caret, 0.0.x and 0.9.x boundaries, caretAdmits upper bounds) are only exercised indirectly through the CI verify scripts (ci.yml:40,63, publish.yml:43,46). I verified behavior by executing the module, but a small vitest for expectedPeerRange/caretAdmits edge cases would lock the npm-caret semantics the docstring describes. Not blocking; the CI integration coverage does run this path on every PR.

🟡 LOW No standalone unit tests for the extracted peer-range admission logic — scripts/verify-package.mjs

verify-package.mjs now delegates its peer-range gate to scripts/lib/peer-range.mjs, whose functions are pure and trivially unit-testable, but no unit test file exists; they are exercised only through verify:package, which requires npm pack plus network installs. A regression in caretAdmits/expectedPeerRange surfaces only as a late, slow CI failure with no case-level diagnosis. Fix: add a small node:test file for expectedPeerRange/caretAdmits covering the 1.x, 0.x, and 0.0.z branches and run it in ci.yml. I verified the current logic passes all 13 such cases empirically, so this is a gap, not a defect.

🟡 LOW Tightened caret-admission logic has no direct unit coverage — scripts/verify-package.mjs

assertCaretAdmits now delegates to caretAdmits, which adds an exclusive upper bound (npm's caret rule) that the previous floor-only check lacked. grep found no test referencing caretAdmits/expectedPeerRange (tests/ has none; vitest suite does not cover scripts/lib/). The new math is exercised only by the live CI/publish verify gate, so a future cohort regression in the range helper would surface only at release time, not in pnpm test. Impact: low; the script self-checks its matcher (assertEdgeUnsafeStaticImportMatcher, line 372) but not this logic. Fix: add a small unit test for caretAdmits/expectedPeerRange (0.x minor-lock, 1.x caret, 0.0.x, prerelease cases),


tangletools · 2026-08-16T20:32:14Z · trace

The admission rule and the range shape had no test: coverage was transitive
through verify:package and verify:official-optimizers, which run against the
one live pin, so a rule that is wrong for another version reaches a release
unseen. That is how the pre-1.0 caret defect survived.

The table is npm's rule, read from semver 7.8.5 with includePrerelease. It
holds the four cases the fix turns over: ^0.9.0 refuses 0.10.0, ^0.0.3
refuses 0.0.4, a prerelease reads by its release part instead of a NaN
patch, and a pre-1.0 version earns the next-minor window.
@drewstone

Copy link
Copy Markdown
Contributor Author

@tangletools review now

@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 — 0778bd0d

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-16T21:14:17Z

@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

Verdict sound
Coverage 1 of 2 lenses (usefulness)
Concerns 0 (none)
Heuristic 0.0s
Duplication 0.1s
Interrogation 268.6s (2 bridge agents)
Total 268.7s

⚠️ Partial audit — the verdict covers only usefulness. value: cli-bridge admission rejected (queue saturated). Treat the missing lens as unexamined, not as clear.

💰 Value — error

value agent never ran: the CLI bridge refused admission (no model was started).

  • Model: opencode/deepseek/deepseek-v4-pro
  • Bridge attempts: 4
  • Bridge error: opencode/kimi-for-coding/k2p7: Bridge returned 503: bridge at capacity (queue_timeout, lane=reserved): active=20/20 queued=6/48 — no model was started

🎯 Usefulness — sound

A correct, dependency-free encoding of npm's caret rule that closes a verified soundness hole in the pre-publish guard and collapses two inline copies of the peer-range rule into one shared module already wired into CI and the publish gate.

  • Integration: Fully reachable now, not ahead of any caller. Both consumers import the new module: scripts/verify-package.mjs:16 (used at lines 70, 72 for peer-range derivation and line 287 for cohort admission) and scripts/verify-official-optimizers.mjs:13 (lines 21, 31). Both scripts run in CI on every push (.github/workflows/ci.yml:40 'pnpm verify:package', ci.yml:63 'pnpm verify:official-optimizers') and gat
  • Fit with existing patterns: Fits the repo's own established release convention rather than inventing one: the minor-window peer for pre-1.0 agent-eval (#137, package.json:82) and the caret peer for 1.0+ agent-interface (#138, package.json:83) were already the shipped policy — this PR encodes exactly that shape rule into one function and deletes the two divergent inline copies (verify-package.mjs's exactMinorPeerRange/caretPe
  • Real-world viability: Holds on the edge paths, not just the happy path: unreadable versions ('', '1.0', 'latest', 'v1.0.0') make caretAdmits return false (refuse, fail-closed) and expectedPeerRange throw with a named message; prereleases (-develop.1, -rc.1) and build metadata (+build) are handled by the strict readVersion regex, which also structurally removes the old NaN-patch hole from split('.') on '1.0.0-rc.1'; the
  • Model: opencode/zai-coding-plan/glm-5.2
  • Bridge attempts: 2

No concerns from the lens that ran (usefulness). The missing lens examined nothing, so this is not a full clean bill of health.


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 · 20260816T212747Z

@drewstone
drewstone merged commit 383ce19 into main Aug 16, 2026
2 checks passed
@drewstone
drewstone deleted the fix/verify-package-caret-0x branch August 16, 2026 21:35
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