Skip to content

chore(release): prepare agent-knowledge 6.1.10 - #96

Merged
drewstone merged 4 commits into
mainfrom
chore/release-6.1.9
Jul 28, 2026
Merged

chore(release): prepare agent-knowledge 6.1.10#96
drewstone merged 4 commits into
mainfrom
chore/release-6.1.9

Conversation

@drewstone

@drewstone drewstone commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Summary

  • prepare @tangle-network/agent-knowledge@6.1.10
  • pin @tangle-network/agent-eval@0.135.1
  • reject bare side-effect imports of Worker-unsafe packages in packed output
  • refresh the lockfile resolution, install command, and changelog

Checks

Full release branch checks on Node 22.22.3 and pnpm 10.34.5:

  • pnpm install --frozen-lockfile
  • pnpm lint: 188 files checked
  • pnpm typecheck: source and public contracts passed
  • AGENT_KNOWLEDGE_RUN_NETWORK_TESTS=1 pnpm test: 473 passed, 7 skipped, 480 total
  • pnpm build: 6 public entry points built
  • pnpm audit --prod --audit-level low: 0 known vulnerabilities

Focused checks after merging current origin/main:

  • lock tests: 39 passed across 3 files
  • static-import matcher: 5 cases passed, including bare import rejection and dynamic import allowance
  • pnpm verify:package at 6.1.10: 5 runtime imports, skill, CLI version, exact dependencies, clean install, and re-pack passed
  • official GEPA and SkillOpt integration: 2 passed across 1 file with agent-eval-rpc==0.135.1
  • git merge-tree --write-tree origin/main HEAD

@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 — 2f90770d

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.

tangletools · auto-approval · reason: drewstone_author · 2026-07-28T19:05:11Z

@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
Concerns 0 (none)
Heuristic 0.0s
Duplication 0.0s
Interrogation 39.9s (2 bridge agents)
Total 39.9s

💰 Value — sound

Mechanical 6.1.8→6.1.9 release-prep that bumps the heavily-used agent-eval dep to 0.135.1; follows the repo's established release pattern exactly with no better approach needed.

  • What it does: Bumps @tangle-network/agent-knowledge from 6.1.8 to 6.1.9 and its @tangle-network/agent-eval dependency from 0.134.2 to 0.135.1 (exact pin), refreshes pnpm-lock.yaml, updates the README install example to pin knowledge@6.1.9 + eval@0.135.1, and adds a CHANGELOG entry. The actual commit touches 4 files (package.json, pnpm-lock.yaml, README.md, CHANGELOG.md) consistently — the PR diff in the prompt
  • Goals it achieves: Consume upstream agent-eval 0.135.1 fixes (strict rollout-record validation + stable estimated-cost receipt validation, per CHANGELOG.md:7) so knowledge consumers resolve the corrected Eval implementation without installing an older copy. agent-eval is imported across src/optimization.ts, src/research-loop.ts, src/retrieval-eval.ts, src/proposals.ts and ~20 test files, so the package directly inhe
  • Assessment: Textbook release-prep, fully in the grain of the codebase. The prior eight releases (6.1.1 through 6.1.8 in git log) use the identical chore(release): prepare agent-knowledge X.Y.Z commit shape and the same four-file change set. Exact-pinning agent-eval is enforced by scripts/verify-package.mjs:53 ('must be pinned to one exact version') and scripts/verify-official-optimizers.mjs:18 — this change
  • Better / existing approach: none — this is the right approach. Searched for an existing release automation script (glob scripts/*, grep 'prepare agent-knowledge') and found only verify-package.mjs / verify-official-optimizers.mjs which validate the pinned versions but do not generate the bump; the manual four-file release-prep is the established repo workflow and there is no helper being bypassed.
  • Model: opencode/zai-coding-plan/glm-5.2
  • Bridge attempts: 2
  • Bridge warning: opencode/kimi-for-coding/k2p7: bridge stream ended without value-audit content

🎯 Usefulness — sound

Clean release-prep PR: version bump 6.1.8→6.1.9 plus agent-eval 0.134.2→0.135.1, following the repo's established release cadence exactly.

  • Integration: The agent-eval 0.135.1 bump lands on 40+ existing import sites in src/ (e.g. validateRunRecord in src/kb-improvement/contracts.ts:1, CostReceipt in src/memory/experiment/types.ts:1); the package itself is consumed downstream by agent-runtime per AGENTS.md layering. No new surface to wire — the dependency change flows through already-reachable code, and the PR's own typecheck + 473 passing tests co
  • Fit with existing patterns: Matches the established release grain precisely. git log shows 6.1.3 through 6.1.8 all shipped under identical 'chore(release): prepare agent-knowledge X.Y.Z' messages with the same package.json + lockfile + CHANGELOG + README install-pin shape. No competing pattern.
  • Real-world viability: Release-prep only: a pinned dependency bump and version string. Robustness is owned by the upstream agent-eval 0.135.1 release and the package's existing test suite (473 passing). The disclosed verify:official-optimizers block (PyPI agent-eval-rpc lagging at 0.135.0) is a verification-infra gap, not a defect in this release's usefulness.
  • 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 · 20260728T190552Z

@tangletools

Copy link
Copy Markdown
Contributor

✅ No Blockers — 2f90770d

Review health 100/100 · Reviewer score 95/100 · Confidence 80/100 · 0 findings (none)

glm: Correctness 95 · Security 95 · Testing 95 · Architecture 95

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

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

No findings.


tangletools · 2026-07-28T19:07:49Z · trace

tangletools
tangletools previously approved these changes Jul 28, 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.

✅ Clean — 2f90770d

Full multi-shot audit completed 4/4 planned shots over 4 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-07-28T19:07:49Z · 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-with-nits

Verdict sound-with-nits
Concerns 1 (1 weak-concern)
Heuristic 0.0s
Duplication 0.0s
Interrogation 116.2s (2 bridge agents)
Total 116.2s

💰 Value — sound

Lazy-loads proper-lockfile so the package is importable in Cloudflare Workers and adds a pack-time guard against the whole class; clean, minimal, in-grain — ship.

  • What it does: Converts the static import { check, lock } from 'proper-lockfile' (src/mutation-lock.ts:4) into a memoized dynamic import() (loadLockfile(), mutation-lock.ts:30-35) invoked at the two async call sites (mutation-lock.ts:307 and :448). LockOptions stays a type-only import. Adds assertNoEdgeUnsafeStaticImports (scripts/verify-package.mjs:226-248) that walks the packed dist/, regex-scans e
  • Goals it achieves: Make @tangle-network/agent-knowledge importable from a Cloudflare Worker again. proper-lockfile pulls graceful-fs, which reassigns fs.close/fs.closeSync at module scope; workerd exposes those as getter-only accessors, so the assignment throws during Cloudflare's upload-time startup validation (Cannot set property close ... [code 10021]), rejecting the entire Worker — including downstre
  • Assessment: Good change on its merits. The fix is minimal and behavior-preserving: both lock call sites are already async, so the dynamic import adds no new control flow — lockfileModule ??= import(...) mirrors the existing memoization style (state.closePromise ??= adapter.close() at src/memory/branch.ts:588, respond ??= at benchmarks/memory-runner.ts:263). The verify-package guard is exactly where rele
  • Better / existing approach: none — this is the right approach. Searched src/ for existing lazy-import helpers, edge-safety checks, or Worker-safe lock alternatives: none exist. Replacing proper-lockfile would be far more invasive for zero benefit (file locks only ever run in Node; the lazy import means it never executes in a Worker). The post-pack regex scan in verify-package.mjs is the correct home — it is the existing rele
  • Model: opencode/zai-coding-plan/glm-5.2
  • Bridge attempts: 2
  • Bridge warning: opencode/kimi-for-coding/k2p7: bridge stream ended without value-audit content

🎯 Usefulness — sound-with-nits

Defers the proper-lockfile import so the package stops crashing Cloudflare Workers on upload, plus a CI/publish-wired regression guard that catches the static-import class.

  • Integration: Fully reachable, not dead surface. The lazy loadLockfile() (src/mutation-lock.ts:32) fires from acquireDurableFileLock (src/mutation-lock.ts:307) and hasActiveMutationLock (src/mutation-lock.ts:448). acquireDurableFileLock backs every mutation: withKnowledgeMutation (src/mutation-lock.ts:118), healAbandonedMutationEpoch (src/mutation-lock.ts:415), and the KB-improvement run state machi
  • Fit with existing patterns: Idiomatic and minimal. Keeps import type { LockOptions } (erased at compile time, so no runtime static import leaks) and converts only the value import to a memoized dynamic import() (src/mutation-lock.ts:30-35). Both call sites were already async, so no signature or caller changes were needed — zero blast radius. This is the standard Node pattern for a dep whose module scope has side effects,
  • Real-world viability: Holds up on the paths that matter. Concurrency: the lockfileModule ??= import('proper-lockfile') assignment (src/mutation-lock.ts:33) is settled before any await and Node dedupes in-flight dynamic imports, so concurrent first-callers all observe the same Promise — no race. Error path: if the dynamic import rejects, the rejected promise is cached, so subsequent callers fail consistently rather th
  • Model: opencode/zai-coding-plan/glm-5.2
  • Bridge attempts: 1

🎯 Usefulness Audit

🟡 Regression guard's regex misses bare side-effect imports [robustness] ``

The guard at scripts/verify-package.mjs:234 matches from 'x' and require('x') but NOT a bare side-effect import 'proper-lockfile' / import 'graceful-fs' (no from, no require). The PR's own framing (CHANGELOG.md:10, scripts/verify-package.mjs:53-55) claims it fails on 'any static import' of these packages; that claim is slightly stronger than the implementation. Realistically nobody writes a bare side-effect import of a lockfile lib, so impact is negligible, but the guard's stated gua


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 · 20260728T191851Z

@tangletools

Copy link
Copy Markdown
Contributor

✅ No Blockers — 978d3e2f

Review health 100/100 · Reviewer score 95/100 · Confidence 80/100 · 0 findings (none)

glm verifier:glm51 aggregate
Readiness 95 95 95
Confidence 80 95 80
Correctness 95 95 95
Security 95 95 95
Testing 95 80 80
Architecture 95 90 90

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

Full multi-shot audit completed 4/4 planned shots over 4 changed files. Global verifier still owns final merge decision. | Reviewer glm returned no findings; independent end-to-end read of all four changed files (CHANGELOG.md, README.md, package.json, pnpm-lock.yaml) confirms the agent-eval 0.134.2→0.135.1 bump is internally consistent across package.json:78, README.md:12, and pnpm-lock.yaml (specifier, version, and integrity hash all align at 0.135.1), with the snapshot's transitive deps unchanged. No source/API/security changes; the previous stale README pin (0.134.1 vs package.json's 0.134.

No findings.


tangletools · 2026-07-28T19:19:46Z · trace

tangletools
tangletools previously approved these changes Jul 28, 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.

✅ Clean — 978d3e2f

Full multi-shot audit completed 4/4 planned shots over 4 changed files. Global verifier still owns final merge decision. | Reviewer glm returned no findings; independent end-to-end read of all four changed files (CHANGELOG.md, README.md, package.json, pnpm-lock.yaml) confirms the agent-eval 0.134.2→0.135.1 bump is internally consistent across package.json:78, README.md:12, and pnpm-lock.yaml (spec

Full immutable report for this review: trace

Summary comment for this run: full summary


tangletools · 2026-07-28T19:19:46Z · 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
Concerns 1 (1 weak-concern)
Heuristic 0.0s
Duplication 0.0s
Interrogation 58.4s (2 bridge agents)
Total 58.4s

💰 Value — sound

Lazily loads proper-lockfile so the package stops poisoning Cloudflare Worker bundles at upload, and adds a regression check on the published dist that catches what wrangler --dry-run structurally cannot.

  • What it does: Converts the static import { check, lock } from 'proper-lockfile' in src/mutation-lock.ts:4 into a memoized dynamic import() (loadLockfile, src/mutation-lock.ts:30-35) consumed at the two async call sites (src/mutation-lock.ts:307 and :448); bumps the package to 6.1.9 and agent-eval to 0.135.1; and adds a verify-package step (scripts/verify-package.mjs:227-284) that scans the packed dist for a
  • Goals it achieves: Make this package (and the agent-runtime root barrel that re-exports it) importable in Cloudflare Workers. graceful-fs assigns fs.close/fs.closeSync at module scope; workerd exposes those as getter-only accessors, so a static import made workerd reject the whole Worker at upload validation (code 10021) before any request frame — uncatchable, even for consumers that never take a lock. Second
  • Assessment: Correct and minimal. Both call sites were already async, so deferring module scope to first lock use is behavior-preserving; the loader is cached (??=) so module scope runs at most once per process. The fix is the only place proper-lockfile is imported in src (grep across the tree confirms no other static import remains), so the source change is complete. The verifier is the strong part: it ru
  • Better / existing approach: none — this is the right approach. Checked for an existing lazy-import utility to reuse: rg '\?\?=.*import\(' in src returns only this new loadLockfile, so there is no prior pattern to extend. Dropping proper-lockfile for a Worker-native lock would be a rewrite, disproportionate to a patch release and out of scope. An AST-based static-import detector would be more robust than the regex at script
  • Model: opencode/zai-coding-plan/glm-5.2
  • Bridge attempts: 2
  • Bridge warning: opencode/kimi-for-coding/k2p7: bridge stream ended without value-audit content

🎯 Usefulness — sound

Release prep that ships a minimal, correct Worker-safety fix (lazy proper-lockfile) plus a regression gate covering a structural blind spot in wrangler dry-runs; the fix lands on the package's most-used primitive.

  • Integration: Fully reachable. mutation-lock is imported by 13 modules across the KB pipeline (sources.ts:8, indexer.ts:3, kb-store.ts:5, freshness.ts:5, store.ts:12, proposals.ts:4, investment-thesis-task.ts:27, index.ts:29, kb-improvement/{state,transition,workspace,run}.ts) and exports withKnowledgeMutation/withKnowledgeRead/acquireDurableFileLock publicly. Both runtime call sites of proper-lockfile (lock at
  • Fit with existing patterns: Matches the codebase grain exactly. It does not introduce a competing lock mechanism — it defers the module-scope side effect of the existing proper-lockfile dependency via idiomatic dynamic import(), the standard way to keep a module importable when a transitive patches a builtin. The verifier extends the established verify:package pattern (which already checks declarations, pinned versions, clea
  • Real-world viability: Holds up under realistic use. loadLockfile() assigns the import promise synchronously via ??=, so concurrent first-callers share one promise (no import storm, no race); subsequent calls return the cached module. Both consumers are already async, so behavior is unchanged. The static-import detector carries a self-test (assertEdgeUnsafeStaticImportMatcher) covering bound, bare, re-export, require, a
  • Model: opencode/zai-coding-plan/glm-5.2
  • Bridge attempts: 1

💰 Value Audit

🟡 Static-import detector is regex-based, not AST-based [maintenance] ``

scripts/verify-package.mjs:249-257 detects edge-unsafe imports via a regex over raw dist text. It correctly handles the documented cases (self-tested at :259-274, including the dynamic-import exclusion), but can in principle be evaded by computed specifiers or aliasing. For a two-package blocklist this is proportionate; if the blocklist grows, swap the regex for an acorn/ts-morph AST walk over the dist so the guarantee is structural rather than lexical.


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 · 20260728T192403Z

@drewstone

Copy link
Copy Markdown
Contributor Author

FYI — I tagged v6.1.9 from main (f455a68) to get the Worker-safe lazy proper-lockfile fix (#97) out immediately: every Cloudflare product on agent-runtime@0.108+ is currently undeployable, and the fix is blocked behind an agent-knowledge publish because agent-runtime pins this package at an exact version.

This branch will need to re-cut as 6.1.10. Two reasons to keep them separate rather than folding the fix into your release:

  • This branch moves @tangle-network/agent-eval to 0.135.1, which is outside agent-runtime's peer range >=0.134.2 <0.135.0. Shipping the deploy fix inside that bump would force an agent-runtime peer widening into the critical path.
  • main at 6.1.9 keeps agent-eval@0.134.2, so agent-runtime can pin 6.1.9 and stay a coherent cohort today.

Your fix(package): detect bare edge-unsafe imports (6847839) is a real improvement over what I merged — my matcher missed bare side-effect imports (import 'proper-lockfile'). Please keep it in 6.1.10; I'm porting the same hasStaticImport matcher to agent-runtime's equivalent check.

tangletools
tangletools previously approved these changes Jul 28, 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 — 68478397

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.

tangletools · auto-approval · reason: drewstone_author · 2026-07-28T19:24:31Z

@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 — 2c49c800

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.

tangletools · auto-approval · reason: drewstone_author · 2026-07-28T19:28:01Z

@drewstone drewstone changed the title chore(release): prepare agent-knowledge 6.1.9 chore(release): prepare agent-knowledge 6.1.10 Jul 28, 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.

🟢 Value Audit — sound

Verdict sound
Concerns 0 (none)
Heuristic 0.0s
Duplication 0.0s
Interrogation 69.1s (2 bridge agents)
Total 69.1s

💰 Value — sound

Makes agent-knowledge importable in Cloudflare Workers by lazy-loading proper-lockfile (side-stepping a graceful-fs module-scope patch that workerd rejects on upload), with a self-tested regression guard in verify-package.

  • What it does: Two real changes folded into release 6.1.10. (1) src/mutation-lock.ts:4 replaces the static import { check, type LockOptions, lock } from 'proper-lockfile' with a type-only import plus a cached dynamic loader (loadLockfile() at src/mutation-lock.ts:32, called at src/mutation-lock.ts:307 and src/mutation-lock.ts:448). Both call sites are already inside async functions, so behavior is unchanged
  • Goals it achieves: Read from the change: keep @tangle-network/agent-knowledge importable from a Cloudflare Worker bundle. The detailed comment at src/mutation-lock.ts:22-29 explains the failure mode precisely — workerd exposes fs.close/fs.closeSync as getter-only accessors, so graceful-fs's module-scope assignment throws during upload validation, rejecting the entire Worker before any request frame, uncatcha
  • Assessment: Good change on its merits. The fix is the smallest possible: two await loadLockfile() calls replace destructured bindings, the API surface is untouched, and the cached-Promise pattern (lockfileModule ??= import(...) at src/mutation-lock.ts:33) is idiomatic and correctly memoizes (a rejected promise stays rejected, which is the right semantics for a missing dependency). The regression guard is
  • Better / existing approach: none — this is the right approach. I searched for an existing lazy-load pattern to extend (rg for let \w+Module, await import(, loadLockfile across src/) and found none — proper-lockfile is the only consumer that needs this treatment and loadLockfile is the only lazy loader in the codebase, so there is nothing to fold into or reuse. A larger redesign (replace proper-lockfile with a Worke
  • Model: opencode/zai-coding-plan/glm-5.2
  • Bridge attempts: 2
  • Bridge warning: opencode/kimi-for-coding/k2p7: bridge stream ended without value-audit content

🎯 Usefulness — sound

Converts proper-lockfile to a memoized dynamic import so the package stops poisoning Cloudflare Worker bundles, and adds a CI/publish gate that scans the real published dist for static imports of module-scope-patching packages.

  • Integration: Both proper-lockfile consumers are converted (lock at mutation-lock.ts:307, check at mutation-lock.ts:448) via a memoized lazy loader (lines 30-35); the module is reachable from 8+ call sites (sources.ts:8, indexer.ts:3, kb-store.ts:5, kb-improvement/state.ts:12, freshness.ts:5, proposals.ts:4, investment-thesis-task.ts:27, store.ts:12), and the new verify check is wired into both ci.yml:40 and pu
  • Fit with existing patterns: proper-lockfile is imported in exactly one file and the type-only import at mutation-lock.ts:4 is elided at build time, so no static import survives in the bundle; the memoized lazy loader is idiomatic and no competing locking mechanism exists in the package.
  • Real-world viability: The guard closes a real gap — wrangler deploy --dry-run bundles without executing, so it cannot see module-scope patches like graceful-fs reassigning fs.close; the self-test at verify-package.mjs:72 proves the matcher distinguishes static from dynamic import, and all lock call sites were already async so the await adds no friction.
  • 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 · 20260728T192939Z

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

💰 Value — sound

Makes the package importable from Cloudflare Workers by deferring proper-lockfile (which pulls graceful-fs, a Node-builtin monkey-patcher) behind a memoized dynamic import, plus a release-time guard that prevents regression — minimal, correct, in-grain.

  • What it does: Two real changes plus a version bump. (1) src/mutation-lock.ts:4,30-35 replaces the static import { check, lock } from 'proper-lockfile' with a type-only import (erased at compile) plus a memoized loadLockfile() that calls import('proper-lockfile') on first use. Both call sites (acquireDurableFileLock at line 307 and hasActiveMutationLock at line 448) are already async, so the change is a pure
  • Goals it achieves: Make @tangle-network/agent-knowledge importable from Cloudflare Workers (and any other runtime where graceful-fs's module-scope patching of fs.close/fs.closeSync is rejected). workerd exposes those fs accessors as getter-only, so the assignment throws during Cloudflare's upload-time startup validation — Cannot set property close of #<Object> which has only a getter [code: 10021] — reje
  • Assessment: A textbook minimal root-cause fix shipped with a regression guard. The static import is the actual cause (graceful-fs's module-scope side effect); the type-only import + lazy runtime load is the smallest change that fixes it without altering any call path or surface API. It fits the grain of verify-package.mjs, which already enforces the same shape of release-time invariant (forbiddenDeclarationNa
  • Better / existing approach: none — this is the right approach. Considered alternatives and rejected each: (a) A separate workerd/browser export condition in package.json — heavier, splits the public surface, only helps opt-in consumers, and would still require either a second build or a stub; the lazy import achieves the same property more cheaply. (b) Drop proper-lockfile entirely and reimplement on O_EXCL/mkdir atomici
  • Model: opencode/zai-coding-plan/glm-5.2
  • Bridge attempts: 2
  • Bridge warning: opencode/kimi-for-coding/k2p7: bridge stream ended without value-audit content

🎯 Usefulness — sound

Dynamic-import of proper-lockfile unblocks Cloudflare-Worker consumers poisoned by graceful-fs's module-scope fs patch, and the new dist-scanning verifier guards against regression — both wired into live code paths.

  • Integration: Reachable and exercised. loadLockfile() (src/mutation-lock.ts:32) is called from the two real lock sites — acquireDurableFileLock (line 307) and hasActiveMutationLock (line 448) — both already async; every external caller (withKnowledgeMutation, healAbandonedMutationEpoch, and acquireRunLease at src/kb-improvement/state.ts:187) is async too, so the added await is transparent. The ver
  • Fit with existing patterns: Matches the codebase grain precisely. Memoized module-level Promise<typeof import(...)> via ??= is the standard Node idiom for deferring a side-effectful import; it reuses the existing proper-lockfile dependency rather than replacing the lock primitive. The verifier extends the already-run release checker instead of introducing a parallel gate, and scans built JS rather than source — the rig
  • Real-world viability: Solid on the happy path and the targeted failure. Concurrency is safe: lockfileModule ??= import('proper-lockfile') resolves to the same shared Promise for all concurrent callers, so the module loads once. The class of failure it fixes is real and correctly diagnosed — graceful-fs mutates fs.close/fs.closeSync at module scope, workerd exposes those as getter-only accessors, and `wrangler dep
  • Model: opencode/zai-coding-plan/glm-5.2
  • Bridge attempts: 1

🎯 Usefulness Audit

🟡 Rejected dynamic import is cached permanently [robustness] ``

lockfileModule ??= import('proper-lockfile') (src/mutation-lock.ts:33) stores the Promise before it settles; if the import rejects, lockfileModule holds the rejected Promise and every subsequent loadLockfile() re-await it forever. For a pinned, installed-at-install-time dependency this is effectively permanent and harmless, and the behavior matches the standard memoization idiom — but if you ever want self-healing after a transient load error, reset lockfileModule = undefined in a catch.

💰 Value Audit

🟡 Release guard's correctness silently depends on tsdown neverBundle [maintenance] ``

scripts/verify-package.mjs:249-257 keys the static-import check off the literal bare specifier 'proper-lockfile'. This is reliable today only because tsdown.config.ts:19-21 sets deps.neverBundle: true, so bare specifiers survive verbatim into dist/. The guard's comment block (lines 48-58) thoroughly explains the workerd failure mode but does not state the dependency on neverBundle. If a future PR flips to bundling deps, a bundler could rewrite import('proper-lockfile') into a chunk path (`


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 · 20260728T193042Z

@tangletools

Copy link
Copy Markdown
Contributor

✅ No Blockers — 2c49c800

Review health 100/100 · Reviewer score 86/100 · Confidence 85/100 · 3 findings (3 low)

glm: Correctness 86 · Security 86 · Testing 86 · Architecture 86

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

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

🟡 LOW agent-eval minor bump (0.134.x -> 0.135.x) not validated within package.json scope — package.json

The dependency moves a minor version (0.134.2 -> 0.135.1). Tangle packages have not always strictly followed semver, so a minor bump can in principle carry breaking API changes that affect this package's source. package.json itself cannot validate compatibility; confirmation must come from typecheck/build/tests run against 0.135.1 (covered by other shots reviewing src changes and by CI). No action required within this file.

🟡 LOW Matcher false-positives on import/require text inside comments or string literals — scripts/verify-package.mjs

The regex scans raw text, so // import 'proper-lockfile' or a string containing that text would trip the gate. Pre-existing limitation (the old from/require matcher had the identical issue) and fail-SAFE — a false positive blocks a publish rather than allowing an offender. Bundled/minified dist output rarely carries such comments. No fix required for this PR; noting for completeness. If it ever bites, a cheap mitigation is to strip line/block comments before scanning.

🟡 LOW Self-test covers only one of the two watched specifiers — scripts/verify-package.mjs

assertEdgeUnsafeStaticImportMatcher hardcodes proper-lockfile; graceful-fs is not exercised. The escape is specifier-agnostic (verified: both classify bare=true, dynamic=false identically), so this is coverage breadth, not a correctness gap. Could parameterize over edgeUnsafeStaticImports for belt-and-suspenders.


tangletools · 2026-07-28T19:31:50Z · 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 — 3 non-blocking findings — 2c49c800

Full multi-shot audit completed 5/5 planned shots over 5 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-07-28T19:31:50Z · immutable trace

@drewstone
drewstone merged commit 502ef49 into main Jul 28, 2026
2 checks passed
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