feat(release): require a version bump for a manifest or export change - #642
feat(release): require a version bump for a manifest or export change#642drewstone wants to merge 3 commits into
Conversation
tangletools
left a comment
There was a problem hiding this comment.
✅ Auto-approved drewstone PR — 05d1bfa6
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-18T20:06:14Z
|
@tangletools review now |
1 similar comment
|
@tangletools review now |
tangletools
left a comment
There was a problem hiding this comment.
⚠️ Value Audit did not run — no verdict
This is not an approval and not a clean bill of health. Neither interrogation lens returned a judgment, so this PR has no value-audit evidence for or against it.
| Status | audit-incomplete (could not run) |
| Why | value: agent returned no usable verdict; usefulness: agent returned no usable verdict |
| Lenses answered | 0 of 2 |
| What to do | re-run once the CLI bridge has capacity: pr-reviewerctl trigger <repo>#<pr> --force |
💰 Value — error
value agent produced no parseable value-audit JSON.
- Model: opencode/deepseek/deepseek-v4-pro
- Bridge attempts: 5
- Bridge error: opencode/kimi-for-coding/k2p7: opencode: opencode error; opencode/zai-coding-plan/glm-5.2: opencode: opencode error; opencode/deepseek/deepseek-v4-pro: opencode: opencode error
🎯 Usefulness — error
usefulness agent produced no parseable value-audit JSON.
- Model: opencode/deepseek/deepseek-v4-pro
- Bridge attempts: 5
- Bridge error: opencode/zai-coding-plan/glm-5.2: opencode: opencode error; opencode/kimi-for-coding/k2p7: opencode: opencode error; opencode/deepseek/deepseek-v4-pro: opencode: opencode error
No concerns are listed because nothing examined the change — absence of findings here is absence of evidence, not a pass.
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.
tangletools
left a comment
There was a problem hiding this comment.
🟡 Value Audit — sound-with-nits
| Verdict | sound-with-nits |
| Coverage | 2 of 2 lenses (value, usefulness) |
| Concerns | 2 (2 weak-concern) |
| Heuristic | 0.0s |
| Duplication | 0.1s |
| Interrogation | 421.0s (2 bridge agents) |
| Total | 421.1s |
💰 Value — sound-with-nits
Adds a merge-base gate that fails any consumer-visible change (manifest field or exported symbol) shipped without the version bump it implies, closing the 'merged but never published' hole that publish.yml's registry-skip creates — ported from the sibling repos and verified working here; ship.
- What it does: Two checks wired into verify:package (package.json:183-186), which runs in CI after build (.github/workflows/ci.yml:59-62) and again in publish.yml:49. check:version-bump (scripts/check-version-bump.mjs) diffs every publishable manifest's consumer-visible fields — npm-copied fields listed at :95-123, with
catalog:pins resolved through pnpm-workspace.yaml (:250-263) so an indirect version move c - Goals it achieves: Close a real, verified release-integrity hole: publish.yml skips any version already on the registry (.github/workflows/publish.yml:170-193), so a merged change with no bump silently never reaches consumers — the PR body documents this costing two fleet days when exports merged at an already-held 0.140.0. Once merged, neither a manifest change (peer range, dep pin) nor an added export can land wit
- Assessment: Good, and in the grain. I verified the claims: 27/27 tests pass (vitest, fixture repos), check:version-bump runs clean against origin/main on this branch, and the record covers all 27 entry points with ./openapi.json correctly filed as an asset. The design choices are each argued and sound: a committed record instead of a two-build comparison (lib/api-surface.mjs:1-30 — one git-show, no second che
- Better / existing approach: none — this is the right approach for now. Searched: (1) repo-internal duplication — none (verifyVersionLock, publint, attw, check-analyst-benchmark all inspected, no ref-diff gate existed); (2) ecosystem tooling — microsoft/api-extractor's API report does the committed-surface-snapshot job, but it requires the TS compiler API, which TS7/tsgo does not provide, so it cannot serve this repo set; (3)
- Model: opencode/zai-coding-plan/glm-5.2
- Bridge attempts: 5
- Bridge warning: opencode/kimi-for-coding/k2p7: opencode: opencode error
🎯 Usefulness — sound
A byte-verified port of agent-runtime's release gate that closes a real, demonstrated defect class (exports shipping under a version npm already holds), wired into both CI and the publish path and proven green on this very PR.
- Integration: Fully wired and reachable now. package.json:183-186 adds check:version-bump as the first step of verify:package and check:api-surface after it, plus the api:surface regeneration entry. verify:package runs on every PR and push to main (.github/workflows/ci.yml:62) after the build (ci.yml:59), and again on the publish path (.github/workflows/publish.yml:49). ci.yml:14-17 adds fetch-depth: 0 so the m
- Fit with existing patterns: Fits the grain exactly, on two axes. (1) Repo pattern: this repo already runs a verify:package check family (check:skill, check:model-ids, check:analyst-benchmark at package.json:179-182); this slots into the established pipeline rather than inventing a new one. (2) Fleet pattern: I verified the byte-identical claim — sha256s of all three scripts (0f3823…, 246a12…, 6a3807…) match agent-runtime ori
- Real-world viability: Built for the failure paths, not the happy path. The checker fails closed at every ambiguity I probed: CI events with no base branch (check-version-bump.mjs:162-172), unresolvable base ref (:178-183), unresolvable catalog pin (:256-261), a publishable package with no api-surface.json record (:396-409), a stale record vs built dist (check-api-surface.mjs:209-222), and wildcard export patterns match
- Model: opencode/zai-coding-plan/glm-5.2
- Bridge attempts: 2
💰 Value Audit
🟡 The three-repo byte-identical invariant has no automated enforcement [maintenance] ``
check-version-bump.mjs:59-62 states the files are kept byte-identical across agent-eval/agent-knowledge/agent-runtime and that 'an edit to one belongs in all three', but nothing checks it — I found agent-runtime worktrees still carrying the older 415-line manifest-only version (sha 7c6852…), so drift is the resting state until each repo re-ports. A cheap improvement: a periodic job (or release-time step) that hashes the three copies and files an alert on divergence, or fold that into the existin
🟡 Single-package repo carries multi-workspace generality it cannot exercise [proportion] ``
agent-eval is one publishable package, yet check-version-bump.mjs carries workspace enumeration, per-package records keyed by name, and union-of-publishables logic (:223-243, :369-375). That generality is what makes the three copies identical rather than divergent, so it is a deliberate trade, not accidental — but it means ~30% of the file is dead weight in this repo and every future reader must reason past it. Acceptable as the price of the identity invariant; revisit only if the shared-package
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.
tangletools
left a comment
There was a problem hiding this comment.
🟡 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 | 142.9s (2 bridge agents) |
| Total | 142.9s |
💰 Value — sound-with-nits
Adds a CI gate that fails any PR changing a consumer-visible manifest field or an exported symbol without the version bump the change implies, closing a real silent-publish hole (publish.yml skips versions already on npm); a well-built port of proven agent-runtime tooling, wired into the existing ve
- What it does: Adds three byte-identical-to-sibling-repos scripts (verified: local sha256s match the PR body exactly) plus a committed 3,485-line api-surface.json record. check-version-bump.mjs (scripts/check-version-bump.mjs:95-123) diffs every npm-copied manifest field — with catalog: specifiers resolved through pnpm-workspace.yaml (scripts/check-version-bump.mjs:250-263) — and the export-symbol record against
- Goals it achieves: The gap is verified real: publish.yml:191-192 skips publishing any version npm already holds, so a peer-range move or an added export merged without a bump silently never reaches consumers, and nothing goes red — an added export is invisible even to a manifest check because exports[...].types still points at the same dist path. After merge, a consumer-visible change cannot land under an already-pu
- Assessment: Good change, in the grain of the codebase. It extends the existing verify:package gate chain rather than adding a new workflow; it fails closed everywhere (missing surface record = failure at check-version-bump.mjs:396-410, unresolvable catalog = throw, CI event with no base branch = throw at check-version-bump.mjs:162-172); the committed-record design beats a two-build comparison because it runs
- Better / existing approach: No materially better architecture found. Searched: package.json scripts, .github/workflows/{ci,publish}.yml, scripts/ (all 20 files), git log on scripts/. Closest existing capability is verify-package-exports.mjs, which packs the tarball and checks d.ts/runtime agreement (scripts/verify-package-exports.mjs:981,1082) but never compares against the merge base and cannot demand a bump — different job
- Model: opencode/zai-coding-plan/glm-5.2
- Bridge attempts: 2
- Bridge warning: opencode/kimi-for-coding/k2p7: opencode: opencode error
🎯 Usefulness — sound
A fully-wired release gate that makes both manifest and exported-symbol changes require a version bump, ported byte-identically from agent-runtime and enforced in CI and publish; verified passing against origin/main in this checkout with 27/27 fixture tests green.
- Integration: Reachable and enforced on every path that matters. package.json:183 puts check:version-bump first in verify:package and check:api-surface after it; .github/workflows/ci.yml:61-62 runs verify:package on every PR and push, with fetch-depth: 0 (ci.yml:17) added specifically so the merge-base diff works; .github/workflows/publish.yml:49 runs the same gate before publishing. The regeneration path exist
- Fit with existing patterns: Follows the repo's established checker family (scripts/check-skill.mjs, check-model-id-requests.mjs, check-analyst-benchmark-implementation.mjs all run inside verify:package the same way). The byte-identity claim is real: sha256 of all three script files matches /home/drew/code/agent-runtime and /home/drew/code/agent-knowledge (1 unique hash per file), and env vars were neutralized (PACKAGE_VERSIO
- Real-world viability: Fails closed at every joint I could probe: CI event with no base branch errors instead of silently comparing local main (check-version-bump.mjs:162-172); an unresolvable catalog pin throws rather than comparing equal (mjs:256-261); a publishable package with no api-surface.json record is a failure, not a pass (mjs:396-409); a record stale against the built declarations fails with a diff (check-api
- Model: opencode/zai-coding-plan/glm-5.2
- Bridge attempts: 1
💰 Value Audit
🟡 Two d.ts export-name extractors now coexist in scripts/ [maintenance] ``
scripts/verify-package-exports.mjs:1082 (collectDtsExportNames, regex-based, names only) and scripts/check-api-surface.mjs:106 (exportsOfFile, oxc-parser-based, names + value/type kinds) both walk built declaration files following export * chains. The oxc extractor is strictly more capable. They serve different checks (packed-tarball type/runtime agreement vs committed surface record), and the new files must stay byte-identical to agent-runtime/agent-knowledge siblings, so the duplication is exp
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.
tangletools
left a comment
There was a problem hiding this comment.
🟡 Value Audit — sound-with-nits
| Verdict | sound-with-nits |
| Coverage | 2 of 2 lenses (value, usefulness) |
| Concerns | 2 (2 weak-concern) |
| Heuristic | 0.0s |
| Duplication | 0.0s |
| Interrogation | 154.3s (2 bridge agents) |
| Total | 154.3s |
💰 Value — sound-with-nits
Adds a CI gate that fails any PR changing a consumer-visible manifest field or exported symbol without the matching version bump, closing a real hole this repo's idempotent publish workflow creates; executed well and verified claims, with one minor in-repo overlap.
- What it does: Adds three byte-identical-across-repo scripts plus a committed record: (1) scripts/check-version-bump.mjs diffs every publishable manifest's consumer-visible fields (deps/peers/exports/engines/install-scripts, with
catalog:specifiers resolved through pnpm-workspace.yaml) AND the committed api-surface.json against the PR merge base, failing unlessversionrose to the level the change implies ( - Goals it achieves: Prevent consumer-visible changes from merging under a version npm already holds. That failure mode is real here: publish.yml skips any version already on the registry and treats it as success (.github/workflows/publish.yml:191-199), so an unbumped peer-range move or new export silently never ships and nothing goes red — exactly the agent-runtime #893 PursuitProjection incident the header cites. No
- Assessment: Good change, well executed. The committed-record design over a live two-build comparison is reasoned in the file (merge-base symbols are one
git showaway; the record shows in the PR diff; the check runs pre-build — scripts/lib/api-surface.mjs:13-23). Fail-closed defaults are consistent with repo doctrine (missing record = failure at scripts/check-version-bump.mjs:396-409; unresolvable catalog = - Better / existing approach: No materially better approach for the core capability — I looked. Searched repo scripts/, workflows, and history for any prior version-bump or surface-compare logic (none); checked publint/attw usage (different job); the nearest off-the-shelf equivalent of the surface-record half is @microsoft/api-extractor's API report, but it needs the TS compiler API, which TS7 does not ship — the exact reason
- Model: opencode/zai-coding-plan/glm-5.2
- Bridge attempts: 2
- Bridge warning: opencode/kimi-for-coding/k2p7: opencode: opencode error
🎯 Usefulness — sound
Closes a real, twice-burned gap — consumer-visible manifest and export changes shipping under a version npm already holds — with two mutually-reinforcing checks wired into the CI and publish paths on day one, matching the repo's established guard-script pattern and its zero-upward-dependency layerin
- Integration: Fully reachable immediately: check:version-bump and check:api-surface are prepended/appended into verify:package (package.json:183), which CI runs on every PR after build (ci.yml:62) and publish runs before npm publish (publish.yml:49); fetch-depth: 0 (ci.yml:14-17) supplies the merge base. Verified live in this checkout: check-version-bump exits 0 against origin/main, and 27/27 tests pass against
- Fit with existing patterns: Follows the repo's existing pattern exactly: guard scripts (check-skill, check:model-ids, check-analyst-benchmark) aggregated under verify:package. Vendored byte-identical copies across agent-eval/agent-runtime/agent-knowledge are the correct move given the CLAUDE.md layering rule forbids depending on agent-runtime (devDeps included) and the scripts are repo-agnostic by construction (verified: no
- Real-world viability: Fail-closed on every error path probed: missing surface record fails (check-version-bump.mjs:396-410), unresolvable base fails before fallback in CI (check-version-bump.mjs:162-172), unresolvable catalog pin fails (check-version-bump.mjs:255-261), wildcard export matching nothing fails (lib/api-surface.mjs:105-112), external
export * fromfails with a fix instruction (check-api-surface.mjs:137-1 - Model: opencode/zai-coding-plan/glm-5.2
- Bridge attempts: 1
🎯 Usefulness Audit
🟡 Cross-repo byte-identity is convention, not enforcement [problem-fit] ``
The three copies of check-version-bump.mjs / check-api-surface.mjs / lib/api-surface.mjs stay identical only by discipline; nothing automates the comparison (sibling repos are not locally verifiable from this checkout, so the PR's sha256 claims are taken on trust). Drift would be visible in a three-way diff but not flagged anywhere. The PR body already names the consolidation path — agent-eval hosting the implementation for the others once it can gate against a published copy. Acceptable now; re
💰 Value Audit
🟡 Two .d.ts export-name readers now live in the repo [duplication] ``
scripts/verify-package-exports.mjs:1082-1148 already contains collectDtsExportNames, a regex-based reader of exported names from .d.ts (used for d.ts-vs-runtime agreement on the packed tarball), while this PR adds an oxc-parser-based reader (exportsOfFile, scripts/check-api-surface.mjs:106-165) that extracts names AND kinds from dist. The jobs differ (packed-tarball names-only vs dist name-to-kind record) and verify-package-exports.mjs is repo-specific rather than one of the byte-identical files
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.
✅ No Blockers —
|
tangletools
left a comment
There was a problem hiding this comment.
✅ Approved — 19 non-blocking findings — ecd21506
Full multi-shot audit completed 8/8 planned shots over 16 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-18T21:04:43Z · immutable trace
✅ No Blockers —
|
✅ No Blockers —
|
✅ No Blockers —
|
ecd2150 to
184aa52
Compare
tangletools
left a comment
There was a problem hiding this comment.
✅ Auto-approved drewstone PR — 184aa525
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-19T07:20:01Z
|
Closing unmerged — operator decision. The pattern has legitimate prior art (Angular's committed API goldens, api-extractor's .api.md reports), but that context is 1.0 frameworks with mass external consumers. agent-eval is 0.x by settled policy, its consumers are the fleet moving in lockstep, and surface-deletion breakage already has its fix at the consumer side (explicit version-window ranges; runtime refuses 0.x carets). ~1,700 lines of gate code plus a 3,485-line committed artifact is disproportionate machinery for a solved problem — the growth-autopsy instrument-share finding applies. Recorded in the redesign reject list so it is not rebuilt. |
The gap
This repository had no version-bump check at all. Two consumer-visible changes could ship under a version npm already holds, and nothing would go red:
Shape 2 is what cost the fleet two days. agent-runtime #893 added and exported
PursuitProjection,PursuitRunProjectionandPursuitNodeProjectionand merged at 0.140.0, a version npm already held. A downstream session then spent hours proving those types resolved in no published version before concluding, wrongly, that Runtime had to write them. They existed; the release did not. agent-eval publishes exactly the same way.The change
Ported from agent-runtime rather than reinvented —
scripts/check-version-bump.mjs,scripts/check-api-surface.mjsandscripts/lib/api-surface.mjsare byte-identical to the files in agent-runtime and agent-knowledge, verified by sha256:They read everything repo-specific out of the manifest under inspection and name no repository, so a diff between the three repos is proof of drift, not expected variation.
tests/version-bump-check.test.tsis identical too — 27 tests, all against a throwaway fixture repo.A shared dependency was not available. agent-eval may not depend on agent-runtime — zero upward dependencies, devDeps included — and a new published package was out of scope. agent-eval could host the implementation for the others, but only by shipping repo tooling in its own tarball and then gating itself against a published copy one release stale.
Why
oxc-parserand not the TypeScript compiler API: this repository is on TypeScript 7, which ships no JavaScript compiler API —ts.createProgramisundefined. A checker-based extractor would work in agent-runtime (TypeScript 6) and need rewriting here, which is precisely how one rule grows three different bugs.How it works
api-surface.jsonrecords the symbols each entry point exports, with each name's kind, generated from the built declarations.check:version-bumpcompares that record against the merge base and requires the level the change implies. Needs nothing but git.check:api-surfaceregenerates it fromdistand fails when the two disagree, so a stale record cannot hide an export.Both run inside
verify:package, which CI already runs after the build. No workflow was added.The level is read off the consumer's compatibility boundary. At 0.147.0 a
>=0.147.0 <0.148.0window already reaches a patch, so an added export costs a patch and a removal costs a minor. At 1.0+ those become minor and major../openapi.jsonhas no declarations and is recorded as an asset entry, not silently dropped.Negative control
A — export added, version left at 0.147.0
B — same export, 0.147.0 -> 0.147.1
The probe commits were removed; the branch carries none of them.
The one CI line
actions/checkoutdefaulted to depth 1, which does not createorigin/main, so a merge-base check cannot resolve a base and fails closed on every PR. The checkout in thecijob gainsfetch-depth: 0. No job, workflow, or check was added.Publishing is unaffected:
publish.ymlruns onpushandworkflow_dispatch, both baseless events, so the check reports "does not apply" and exits 0. Confirmed against a fixture with noorigin/main.Verification
pnpm typecheckcleanvitest run tests/version-bump-check.test.ts— 27 passedcheck:api-surfacecurrent: 3423 exports across 27 entry pointsCompanion PRs: tangle-network/agent-runtime#900, and the agent-knowledge port.