feat(release): require a version bump for a manifest or export change - #155
Conversation
tangletools
left a comment
tangletools
left a comment
There was a problem hiding this comment.
✅ Auto-approved drewstone PR — 7f46af46
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:40Z
|
@tangletools review now |
tangletools
left a comment
tangletools
left a comment
There was a problem hiding this comment.
✅ Auto-approved drewstone PR — 7f46af46
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:07:21Z
|
@tangletools review now |
1 similar comment
|
@tangletools review now |
tangletools
left a comment
tangletools
left a comment
There was a problem hiding this comment.
🟢 Value Audit — sound
| Verdict | sound |
| Coverage | 1 of 2 lenses (usefulness) |
| Concerns | 2 (2 weak-concern) |
| Heuristic | 0.0s |
| Duplication | 0.2s |
| Interrogation | 619.8s (2 bridge agents) |
| Total | 620.0s |
⚠️ 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: 6
- Bridge error: opencode/kimi-for-coding/k2p7: opencode: opencode error; opencode/zai-coding-plan/glm-5.2: Bridge returned 503: bridge at capacity (queue_timeout, lane=reserved): active=20/20 queued=9/48 — no model was started
🎯 Usefulness — sound
A fully-wired, proven-firing release gate that closes a real, historically-costly gap (exports and manifest changes shipping under a version npm already holds), ported from an already-merged sibling rather than reinvented.
- Integration: Reachable on every path that matters and proven live.
check:version-bumpruns FIRST inverify:package(package.json:74), which CI runs afterpnpm build(.github/workflows/ci.yml:40-44) withfetch-depth: 0so the merge base exists (ci.yml:15-17); publish.yml:43 runs the same chain on tags. Verified end-to-end in this checkout: (a)node scripts/check-api-surface.mjson a real build reports - Fit with existing patterns: Extends the established release-gate pattern in the repo's grain:
verify:packagealready chains check:skills → publint → attw → verify-package.mjs, and this adds two links to the same chain with the same fail-loud doctrine the repo mandates (missing record fails closed at check-version-bump.mjs:396-409; unresolvable catalog pin fails closed at resolveSpecifier; baseless CI event fails closed at - Real-world viability: Held up under every realistic path exercised. Edge behavior verified or code-traced: bootstrap (base without a record → no export delta demanded); tag-publish runs (shallow checkout, 'push' is a baseless event → 'No base ref' and exit 0, so publish.yml never false-fails); downgrade detection ('lower' rank pays nothing); prerelease ordering; catalog: resolution through pnpm-workspace.yaml with fail
- Model: opencode/zai-coding-plan/glm-5.2
- Bridge attempts: 4
🎯 Usefulness Audit
🟡 PR body's sha256 for check-api-surface.mjs is stale against HEAD [problem-fit] ``
The body claims e59e627e... for scripts/check-api-surface.mjs, but follow-up commit 9ec6dbc (the memoization fix, made to re-sync with siblings) changed it to 246a12a2.... The other two hashes still match. If the byte-identical-across-repos property is ever checked mechanically, update the body hash; otherwise no action.
🟡 Enforcement begins only for PRs whose merge base postdates this merge [integration] ``
Any PR whose merge base predates api-surface.json takes the bootstrap path (no export delta enforceable) — inherent to introducing a record, and the manifest half still applies there. After this merges, every subsequent PR is fully covered. No action possible or needed; noted so nobody expects enforcement on back-based branches. Relatedly, the publish.yml run of check:version-bump is structurally a no-op (shallow, baseless) — the PR gate is the enforcement point, which is the design.
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
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 | 4 (4 weak-concern) |
| Heuristic | 0.0s |
| Duplication | 0.1s |
| Interrogation | 747.3s (2 bridge agents) |
| Total | 747.4s |
💰 Value — sound-with-nits
Adds a release gate that fails any consumer-visible change (manifest field or exported symbol) that ships without a higher version, closing a real and demonstrated gap (publish.yml:95-96 silently skips versions npm already holds) — coherent, fail-closed, and in the repo's grain; only minor maintenan
- What it does: Two scripts plus a committed record, wired into the existing verify:package chain (package.json:74-78) that ci.yml:44 and publish.yml:43 already run. (1) scripts/check-version-bump.mjs diffs every publishable manifest's consumer-visible fields against the merge base — resolving
catalog:specifiers through pnpm-workspace.yaml, skippingworkspace:and non-consumer fields — and requires a strictl - Goals it achieves: No consumer-visible change can merge under a version the registry already holds — the exact failure mode where publish.yml:95-96 prints 'already on registry; skipping publish' and the change never reaches a consumer with nothing red. Manifest comparison alone cannot see an added export (every manifest field stays byte-identical), hence the committed export record; a second goal is replacing hand-m
- Assessment: Good on its merits. The design decisions are justified in-repo and hold up: a committed record rather than a two-build comparison (merge-base symbols are one
git showaway; the record shows in the PR diff); oxc-parser rather than the TS compiler API (this repo pins typescript ^7.0.2 at package.json:100, and TS7 ships no JS compiler API — check-api-surface.mjs:10-17); names-and-kinds granularity - Better / existing approach: None — this is the right approach. Searched for existing equivalents: git log on scripts/check-version-bump.mjs shows only this commit (no prior version-bump logic in-repo); publint and attw are already in the chain but check manifest/exports hygiene, not version-vs-change; npm 404s for @tangle-network/repo-tools and @tangle-network/api-surface, confirming the PR body's claim that no shared packag
- Model: opencode/zai-coding-plan/glm-5.2
- Bridge attempts: 5
- Bridge warning: opencode/kimi-for-coding/k2p7: opencode: opencode error
🎯 Usefulness — sound-with-nits
A working, CI-enforced gate that closes a twice-demonstrated release defect (manifest and export changes shipping under registry-held versions), verified end-to-end on both the pass and fail paths, with only stale-doc and manual-sync nits.
- Integration: Fully wired now, not merely imminent: package.json:74 puts check:version-bump first in verify:package and adds check:api-surface; ci.yml:44 runs verify:package on every PR to main with fetch-depth: 0 (ci.yml:15-17) so the merge-base diff works; publish.yml:43 re-runs it after build (publish.yml:39-43), so a stale record blocks publish too. I ran the scripts directly: check:version-bump passes on t
- Fit with existing patterns: Fits the established grain rather than competing: this repo already composes plain scripts/*.mjs checks into verify:package (check-skills, verify-package). The existing hand-maintained requiredRootExports/forbiddenRootExports (scripts/verify-package.mjs:26,34) remain useful — the new record enforces 'a surface change must carry a bump' while forbiddenRootExports still blocks accidental additions o
- Real-world viability: Built for the failure modes that actually occur: catalog: pins compared by resolution through pnpm-workspace.yaml with fail-closed on unresolvable entries (check-version-bump.mjs:250-263); CI base-ref fail-closed before any local-main fallback (check-version-bump.mjs:158-172); workspace packages keyed by name so directory moves still compare; prerelease ordering, unordered fields, install-lifecycl
- Model: opencode/zai-coding-plan/glm-5.2
- Bridge attempts: 2
💰 Value Audit
🟡 Cross-repo byte-identity of the three shared scripts is convention, not enforced [maintenance] ``
check-version-bump.mjs:59-62 states the files are kept byte-identical across agent-eval, agent-knowledge and agent-runtime, but nothing in this repo's CI compares them; drift is caught only when a human diffs. The follow-up commit 9ec6dbc ('keep check-api-surface.mjs identical to its siblings') shows re-sync is already a manual act. Optional hardening: a scheduled or PR job that fetches the two sibling copies and fails on hash mismatch. Does not gate shipping — the header makes the invariant exp
🟡 verify-package.mjs hand-maintained requiredRootExports is now subsumed by api-surface.json [duplication] ``
verify-package.mjs:26-33 asserts six remembered root exports exist in the packed tarball; the committed api-surface.json now records all 906 exports and check-api-surface fails on any removal, making the hand list strictly weaker for the 'did a promised export disappear' purpose (it retains marginal value as a tarball-level rather than declaration-level check). Keeping both is defensible defense-in-depth; the hand list is a candidate for retirement in a later change, not something this PR must d
🎯 Usefulness Audit
🟡 PR body's byte-identity evidence is stale for check-api-surface.mjs [problem-fit] ``
The body claims sha256 e59e627e… for scripts/check-api-surface.mjs, but the file at HEAD (after follow-up commit 9ec6dbc, whose purpose was restoring sibling identity) hashes to 246a12a2… — and 246a12a2… is what all three repos' pending branches carry. The identity claim is true; the cited digest is not. Refresh or drop the per-file digests so the evidence in the PR matches the merged tree.
🟡 Tri-repo byte-identity is maintained by hand with no mechanical drift check [robustness] ``
Nothing verifies the three copies stay identical after these branches merge; an edit in one repo silently diverges the rule fleet-wide, and the header's 'an edit to one belongs in all three' (check-version-bump.mjs:59-62) is convention only. A shared home is blocked today by the no-upward-dependency layering, so this is an accepted tradeoff — but a cheap periodic hash comparison across the three repos (e.g. in a scheduled workflow) would make drift loud. Note for a human, does not gate this merg
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
tangletools
left a comment
There was a problem hiding this comment.
🟢 Value Audit — sound
| Verdict | sound |
| Coverage | 1 of 2 lenses (usefulness) |
| Concerns | 1 (1 weak-concern) |
| Heuristic | 0.0s |
| Duplication | 0.0s |
| Interrogation | 609.2s (2 bridge agents) |
| Total | 609.2s |
⚠️ Partial audit — the verdict covers only usefulness. value: agent returned no usable verdict. Treat the missing lens as unexamined, not as clear.
💰 Value — error
value agent produced no parseable value-audit JSON.
- Model: opencode/deepseek/deepseek-v4-pro
- Bridge attempts: 6
- 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 — sound
A correctly-wired, verified-working release gate that fails CI when a consumer-visible manifest or export change ships without the version bump it implies — ported from sibling repos into the existing verify:package chain, with the committed 906-export record proven to match a fresh build.
- Integration: Fully wired into present-tense callers, not waiting on a future one: package.json:74 puts check:version-bump FIRST in verify:package; .github/workflows/ci.yml:44 runs verify:package on every PR with fetch-depth: 0 (ci.yml:15-17, commented as required by the check) after a build step (ci.yml:41) that check-api-surface needs; .github/workflows/publish.yml:43 gates the actual publish path on the same
- Fit with existing patterns: Fits the established grain rather than competing. It extends the repo's existing release-gate chain (verify:package already ran publint, attw, verify-package.mjs), and supersedes the blind spot of the hand-maintained requiredRootExports/forbiddenRootExports lists (scripts/verify-package.mjs:26-34) — those police a handful of remembered names while api-surface.json records all 906; the old lists re
- Real-world viability: Proven on the failure paths, not just the happy path. I built a throwaway fixture repo and ran five scenarios through PACKAGE_VERSION_BUMP_ROOT: added export without bump → exit 1 naming the symbol and required level; removed export → exit 1 demanding major; peer-floor move with no bump → exit 1 (the exact historical 0.119.0 defect the header documents); correct minor bump → pass with accounting;
- Model: opencode/zai-coding-plan/glm-5.2
- Bridge attempts: 3
🎯 Usefulness Audit
🟡 Declaration resolver can return a directory and crash with EISDIR instead of a clear error [robustness] ``
scripts/check-api-surface.mjs:81-93 — for an extensionless re-export (e.g.
export * from './kb-improvement'), candidate 1 (base.replace(/\.m?js$/, '.d.ts'), a no-op without a .js suffix) is the bare path, and existsSync returns true for directories. On this checkout's stale dist (which carried dist/kb-improvement/ and dist/sources/ chunk directories) the script crashed with EISDIR at line 111 instead of naming the unresolved specifier. The pinned tsdown 0.22.14 with clean:true emits flat has
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.
2 participants
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-knowledge publishes exactly the same way.scripts/verify-package.mjsalready carries hand-maintainedrequiredRootExportsandforbiddenRootExportslists. Those police a handful of names somebody remembered to add. This records all 906.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-eval, 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.
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 8.0.8 a
^8.0.8range admits every 8.x, so an added export costs a minor and a removal or narrowing costs a major. Below 1.0 the boundary sits one position further right and those become patch and minor.Negative control
A — export added, version left at 8.0.8
B1 — a patch offered where a minor is required
B2 — 8.0.8 -> 8.1.0 pays for it
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 typecheckclean (tsc --noEmitandtsconfig.contracts.json)pnpm test— 707 passed, 12 skipped, 0 failedcheck:api-surfacecurrent: 906 exports across 6 entry pointsCompanion PRs: tangle-network/agent-runtime#900, tangle-network/agent-eval#642.