feat(release): adopt Agent Interface 2 cohort - #178
Conversation
tangletools
left a comment
There was a problem hiding this comment.
✅ Auto-approved drewstone PR — c63e8465
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-31T16:30:11Z
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.0s |
| Interrogation | 274.6s (2 bridge agents) |
| Total | 274.6s |
💰 Value — sound-with-nits
A disciplined major release that moves the dependency cohort to Interface 2 / Eval 0.171.0 with one shared Zod copy, plus a type-first schema annotation and a real declaration-portability guard — everything it claims verified in this checkout, only minor nits.
- What it does: Releases @tangle-network/agent-knowledge 11.0.0 on the Agent Interface 2.0.0 / Agent Eval 0.171.0 cohort: peer ranges move to >=0.171.0 <0.172.0 and ^2.0.0 (package.json:87-88), dev pins follow (94-95), and Zod is exact-pinned 4.5.4 (84) with a minimumReleaseAgeExclude entry (122). It rewrites the persisted activation-record contract as an explicit interface with the schema annotated z.ZodType<Kno
- Goals it achieves: Keep exactly one copy of every cohort package (eval, core, interface, zod) in a consumer install, so schema types are structurally identical at compile time and runtime — the pin is provably correct, since installed agent-interface 2.0.0 and agent-eval 0.171.0 both depend on zod exactly 4.5.4 (node_modules/.../agent-interface/package.json:88, agent-eval:207). Make published declarations reference
- Assessment: Good on its merits and squarely in the codebase's grain. The verify-package additions copy the script's established idiom exactly (exactVersion mirrors exactDevelopmentPin at 285-298; declarationFiles mirrors the javascriptFiles walker; the zod single-copy check extends the existing cohort assertion at 199-203). The type-first annotation matches how the cohort sibling itself ships — agent-interfac
- Better / existing approach: none — this is the right approach. I searched for alternatives: publint and attw (already in the verify:package chain) check export resolution and manifest lint, not .pnpm/absolute-path content inside declaration text; scripts/check-api-surface.mjs records shape digests, not path portability; no shared d.ts-portability or schema-annotation helper exists in src or scripts. The one empirical wrinkle
- Model: opencode/zai-coding-plan/glm-5.2
- Bridge attempts: 2
- Bridge warning: opencode/kimi-for-coding/k2p7: opencode: opencode error event without a message: {"type":"error","timestamp":1788193939485,"sessionID":"ses_fa755e252ffede3eTowciizJmq","error":{"name":"UnknownError","data":{"message":"Unexpected server error. Check server logs for details.","ref":"err_6fca5b2e"}}}
🎯 Usefulness — sound
A clean cohort-release PR: the schema rewrite is on the live activation-persistence path, the zod pin and portability checks extend the existing release gate rather than inventing a new one, and I reproduced the build, tests, and api-surface checks locally.
- Integration: Everything changed is reachable and used. knowledgeImprovementActivationRecordSchema is not new surface but a rewrite of live runtime code: persistKnowledgeActivationResult parses it before durable write (src/kb-improvement/activation.ts:152) and loadKnowledgeActivationRecord parses it on every read-back (src/kb-improvement/activation.ts:207), both driven by the exported promote/restore transition
- Fit with existing patterns: Follows established patterns rather than competing. Peer ranges keep the house style (exact minor window for agent-eval
>=0.171.0 <0.172.0, caret for agent-interface^2.0.0, exact dev pins) and the verify script extends the existing single-installed-copy proof to zod instead of adding a second mechanism. The explicit-interface-plus-z.ZodType-annotation pattern is new to this repo (only occu - Real-world viability: Holds up beyond the happy path. I ran the new contract tests: 4/4 passed (round-trip alignment plus three traversal/control-char candidateId rejections matching the repo's path-containment doctrine). The schema's only consumers use .parse (no .shape/narrowing that the ZodType widening would break, verified by grep across src). The verify additions fail loud with typed errors (exactVersion rejec
- Model: opencode/zai-coding-plan/glm-5.2
- Bridge attempts: 1
🎯 Usefulness Audit
🟡 Interface↔schema drift guard has an optional-field blind spot [robustness] ``
The z.ZodType annotation (src/kb-improvement/contracts.ts:185) makes TypeScript catch drift on required fields, and the new round-trip test (tests/kb-improvement/contracts.test.ts:35) catches unknown-key and constraint drift. But an optional field added to the interface without a matching schema entry would pass both (output lacking an optional is still assignable; the test fixture wouldn't set it). The standard mitigation would be a
satisfies-style two-wa
🟡 Exact-string coupling to agent-interface's zod dependency format [problem-fit] ``
verify-package.mjs now asserts installedAgentInterface.dependencies.zod === '4.5.4' exactly. If a future agent-interface 2.0.x declares '^4.5.4' or moves to 4.5.6 within this package's caret peer range, verify:package fails even though the peer range admits it. This is a conscious cohort lock, not a bug, and it fails loud at release time — but a human should know the gate now pins the dependency format of a peer package, slightly tighter than the existing version-admission checks (assertCaretA
💰 Value Audit
🟡 Type-first annotation widens the static type beyond the runtime contract [maintenance] ``
The explicit interface declares candidateId: string while the schema enforces safePathSegmentSchema (1-128 chars, [A-Za-z0-9._-]) at contracts.ts:161-165; a caller can now statically construct a record parse would reject. This is the inherent tradeoff of the z.ZodType pattern (Input defaults to unknown) and the new test covers the runtime side, so it does not gate — but if more schemas adopt the pattern, a SafePathSegment branded type would restore the lost precision.
🟡 declarationFiles duplicates the javascriptFiles walker [duplication] ``
scripts/verify-package.mjs:430-448 now has two structurally identical recursive walkers differing only in the extension test. A shared walk(directory, filter) would remove the copy, but each checker owning its own walker matches the script's existing per-assertion style; trivial.
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 —
|
| opencode GLM 5.2 | opencode DeepSeek v4 Pro | opencode DeepSeek v4 Flash | aggregate | |
|---|---|---|---|---|
| Readiness | 52 | 77 | 46 | 46 |
| Confidence | 95 | 95 | 95 | 95 |
| Correctness | 52 | 77 | 46 | 46 |
| Security | 52 | 77 | 46 | 46 |
| Testing | 52 | 77 | 46 | 46 |
| Architecture | 52 | 77 | 46 | 46 |
Reviewer score is advisory once the run is complete and the verdict has no blockers.
Full multi-shot audit completed 7/7 planned shots over 7 changed files. Global verifier still owns final merge decision. | Full multi-shot audit completed 7/7 planned shots over 7 changed files. Global verifier still owns final merge decision. | Full multi-shot audit completed 7/7 planned shots over 7 changed files. Global verifier still owns final merge decision.
🟠 MEDIUM Single-zod-copy guarantee claimed but not proven; agent-eval zod drift undetected unless npm hoists it — scripts/verify-package.mjs
Lines 158-160 read only top-level node_modules/zod/package.json; line 174 checks only agent-interface's DECLARED zod. installedAgentEval.dependencies?.zod is never read, and no npm ls zod --all --parseable single-copy proof exists (contrast assertSingleInstalledAgentStack at line 355). Experiment: with fake-knowledge/interface pinning zod 4.5.4 and fake-eval pinning 4.5.3, npm install of the three t
🟠 MEDIUM zod single-copy invariant is asserted for agent-interface only; a nested duplicate zod under agent-eval passes verification — scripts/verify-package.mjs
Lines 171-178 check installedZod.version and installedAgentInterface.dependencies.zod, but agent-eval's (and agent-core's) zod dependency is never asserted, and zod is not added to assertSingleInstalledAgentStack (line 199), which is the only check that proves 'one installed copy' via npm ls. If agent-eval later pins a different exact zod, npm installs a nested node_modules/@tangle-network/agent-eval/node_modules/zod, every existing check still passes, and [line 263](https://github.com/tangle-network/agent-knowledge/blob/c63e84
🟡 LOW 11.0.0 entry omits the consumer-migration note that every prior peer-range bump carried — CHANGELOG.md
Interface jumps from ^1.6.0 to ^2.0.0 (a peer major), and Eval moves to 0.171.x. Prior entries that moved the peer cohort (10.7.1, 10.7.0, 7.2.0) explicitly warned consumers: 'A consumer must move Eval and Interface with this package.' — installing 11.0.0 beside Interface 1.x or Eval 0.170.x is an unmet peer that blocks the install. The new entry says only 'Consumers now resolve one canonical profile, candidate, and tool-definition type' and lists no 'Breaking Changes' heading or 'What to do' action, so an upgrading consumer has no stated migration step. Evidence: diff at CHANGELOG.md:7-10 vs the 10.7.1 entry at CHANGELOG.md:39-42 and the 10.0.0 major at CHANGELOG.md:123-127. Fix: add a bolded consumer-action line naming the two peer moves, and consider a '### Breaking Changes' section
🟡 LOW Changelog omits activation-record schema annotation and portable-declaration verification work — CHANGELOG.md
Commit 411beae ('fix(package): annotate activation record schema') re-typed knowledgeImprovementActivationRecordSchema as an explicit interface with a z.ZodType<> annotation, added assertPortableDeclarations plus zod-cohort checks to scripts/verify-package.mjs, and added 49 lines of tests (tests/kb-improvement/contracts.test.ts). None of this appears under 11.0.0. The exported type shape is unchanged, so consumer impact is nil and the omission does not mislead; still, the repo's changelog convention (see 10.7.1's exhaustive entries) would record it. Fix: add a short bullet under Changed or Fixed noting the activation-record schema annotation and the portable-declaration gate.
🟡 LOW Install command pins exact versions; agent-knowledge@11.0.0 not yet on registry — README.md
The command installs exact pins: pnpm add @tangle-network/agent-knowledge@11.0.0 @tangle-network/agent-eval@0.171.0 @tangle-network/agent-interface@2.0.0. The two peers exist on npm (verified 0.171.0 and 2.0.0), but agent-knowledge@11.0.0 currently 404s (latest published is 10.8.0). This is normal for a release PR whose publish is tag-triggered post-merge, so not a defect in this change; if publish fails or is delayed, the documented command breaks. Exact-pin convention also means the README goes stale each release cycle (it previously advertised 8.0.8 while 10.8.0 was latest). No action required beyond confirming the 11.0.0 publish lands.
🟡 LOW Exact zod pin guarantees one schema copy only within the cohort, not across consumers — package.json
zod moves from ^4.4.3 to the exact pin 4.5.4 to 'keep one schema implementation' (CHANGELOG 11.0.0). zod is a direct dependency, not a peer, so a consumer whose own tree resolves zod 4.4.x exactly (a range 4.5.4 does not satisfy) installs a second zod copy, and the two zod schemas remain distinguishable by instanceof. Acceptable for the stated cohort goal, and partially self-healing because agent-interface@2.0.0 also pins 4.5.4 (verify-package.mjs asserts it), so dedupe works whenever the consumer range admits 4.5.4. Nit only; no fix required unless cross-copy schema identity must hold for all consumers.
🟡 LOW Runtime dependency added to pnpm minimumReleaseAgeExclude supply-chain exemption — package.json
zod@4.5.4 is added to pnpm.minimumReleaseAgeExclude while pnpm.minimumReleaseAge stays 4320. Every prior entry is an internal @tangle-network package or a dev-only tool (esbuild, vite); zod is the first runtime dependency shipped to consumers to be exempted from the 3-day release-age guard, so a freshly published zod@4.5.4 installs without the age check. Impact is narrow because the dep is exact-pinned and the repo installs from a frozen lockfile, but the guard is deliberately relaxed for a runtime dep. Fix: leave the entry only if zod 4.5.4's age requires it; otherwise drop it, or document why a runtime dep needs the exemption.
🟡 LOW minimumReleaseAgeExclude admits zod published inside the repo's own 72h supply-chain soak window — package.json
minimumReleaseAge is 4320 minutes (3 days), and zod@4.5.4 was published 2026-08-29 (verified via npm view) while this PR is dated 2026-08-31, so install only resolves because 'zod@4.5.4' was added to minimumReleaseAgeExclude. This is deliberate (CHANGELOG: 'Pin Zod 4.5.4 to match Interface 2') and scoped to the exact version so a future publish cannot ride the exclusion, but it means the package ships depending on a release younger than the window the repo set to defend against freshly-compromised publishes. Acceptable given the upstream cohort forces 4.5.4; consider removing the exclude once the version ages past 3 days so the gate's default posture is restored for any later bump. No action required to merge.
🟡 LOW Dev toolchain engine gap for node 21.x — pnpm-lock.yaml
New transitive dev-toolchain deps declare engines '^20.19.0 || >=22.12.0' (oxc-parser@0.147.0, rolldown@1.2.6, vite@8.2.2), which excludes node 21.x. Repo package.json engines is '>=20.19.0', nominally permitting node 21.x. On node 21.0-21.6 a fresh install would emit engine warnings or fail under engine-strict. Dev-only (build/lint/test tooling), node 21 is EOL, and the main runtime deps (proper-lockfile, zod 4.5.4) carry no such constraint, so impact is nil in practice. Informational only; consistent with the previous 0.144.0/1.1.5 entries which already used the same range.
🟡 LOW Major transitive bump: @noble/hashes 1.8.0 -> 2.4.0 via agent-interface 2.0.0 — pnpm-lock.yaml
agent-interface jumped 1.6.0 -> 2.0.0 and its resolved snapshot now depends on @noble/hashes@2.4.0 (packages entry line 367) instead of 1.8.0, a major-version API change. This is the deliberate release cohort from commit 'feat(release): adopt Agent Interface 2 cohort', and the lockfile correctly reflects it (validated: all refs resolve, engines >=20.19.0 match repo engines). Impact is on the code-review shots: agent-knowledge runtime code that re-exports or uses agent-interface types/hashing must be confirmed compatible with the 2.x API. No lockfile action needed; verified the old 1.8.0 entry is fully removed (grep count 0).
🟡 LOW Transitive major bump of @noble/hashes 1.8.0 -> 2.4.0 via agent-interface 2.0.0 — pnpm-lock.yaml
agent-interface@2.0.0 now depends on @noble/hashes@2.4.0 (was 1.8.0); pnpm removed the 1.8.0 entry entirely, so no other package in the tree requires 1.x. This is a major-version jump of a cryptographic hashing library consumed transitively through the public agent-interface dependency. No known CVE and no resolution conflict, but the verifier should confirm the interface-2.0 API-surface/contract checks (other shots) account for any @noble/hashes 2.x behavioral change (e.g. return-type/byte-encoding differences).
🟡 LOW zod pinned to 4.5.4, a release exempted from the minimumReleaseAge supply-chain guard — pnpm-lock.yaml
The lockfile pins zod to exactly 4.5.4 (specifier '4.5.4', resolution at line 2542 with integrity sha512-X5XaVWZ...). package.json (outside this shot) adds 'zod@4.5.4' to minimumReleaseAgeExclude, bypassing the 4320-minute (3-day) release-age guard for this fresh version. zod is a runtime dependency of the published package, so a compromised-or-broken fresh release would flow straight to consumers; the exemption also converts the prior caret range (^4.4.3) to an exact pin, freezing every consumer install at the unaged version. Impact is mitigated by the integrity hash and the scoped (single-version) exclusion. Fix: once 4.5.4 is >3 days old, drop the minimumReleaseAgeExcl
🟡 LOW .pnpm substring match can false-positive on comment text — scripts/verify-package.mjs
/.pnpm[/\]/ scans raw file text, so a JSDoc comment or string literal mentioning '.pnpm/' (e.g. documentation about pnpm behavior carried into dist) fails verification even when all specifiers are portable. No src file currently contains '.pnpm' (grep'd src/), so this cannot fire today; a comment-position guard or anchoring to import syntax would remove the spurious-failure mode.
🟡 LOW Final message claims 'one installed copy ... and zod' but single-copy is never verified for zod — scripts/verify-package.mjs
The summary prints 'one installed copy each of ... and zod', but zod is not passed to assertSingleInstalledAgentStack (line 199 only covers agent-eval, agent-core, agent-interface). The zod check only reads the top-level node_modules/zod/package.json version. pnpm-lock.yaml contains a separate zod@3.25.76 entry (a transitive dependency still pulls zod v3), so npm's install used by this script can legitimately install a nested zod@3 copy alongside zod@4.5.4 without this script noticing. The claim overstates the guarantee. If single-zod-copy is the real intent, add zod to the assertSingleInstalledAgentStack call (or an npm ls zod assertion).
🟡 LOW Portable-declarations regexes false-positive on comments/JSDoc and miss file:// and triple-slash absolute refs — scripts/verify-package.mjs
The check at lines 304-307 applies both regexes to raw file text, so any comment/JSDoc containing 'from "/abs"', 'import "/abs"', or 'from "C:..."' flags the file and fails the release gate. Verified by direct execution: '// from "/usr/lib/foo"' and '/** import { a } from "C:\foo" */' both match the absolute-path regex. Generated .d.ts routinely preserves JSDoc. Conversely the regex misses 'from "file:///..."' and '/// ' (no from/import token), so coverage is incomplete. Impact: spurious release blocker on an unrelated comment; absolute file:// refs can slip through. Fix: strip comments before matching (or restrict to lin
🟡 LOW Published agent-knowledge manifest's own zod dependency is never asserted — scripts/verify-package.mjs
assertPublishedRequiredPeer (lines 179-180) validates only the two agent peers in the packed manifest. Nothing checks installedPackage.dependencies?.zod === zodVersion, so a packaging regression that drops or retypes zod in the shipped manifest goes undetected: zod still installs transitively via agent-eval/agent-interface, the top-level read at line 158 passes, and the gate is green while the published package silently loses its declared zod dependency. Symmetric to the peer assertions. Fix: add an assertPublishedRequiredDepen
🟡 LOW absolute-path regex misses import("/abs") type-form, UNC paths, and triple-slash references — scripts/verify-package.mjs
/(?:from|import)\s*"'/ requires the quote directly after from/import, so declare const a: import("/abs/path").X (a very common tsc type-position emission), \server\share UNC paths, and /// are all unflagged (verified synthetically). The .pnpm textual scan independently covers the main pnpm-store leak, so this is a robustness gap, not a hole for the known failure mode. Tightening to also match import(\s*["'] with a leading / or drive letter would close most of it.
🟡 LOW assertPortableDeclarations regexes lack the self-test the other matcher has — scripts/verify-package.mjs
hasStaticImport has a dedicated self-test (assertEdgeUnsafeStaticImportMatcher, lines 413-428) that validates positive/negative cases. The two new regexes in assertPortableDeclarations (
\.pnpm[/\\]|node_modules[/\\]\.pnpmand(?:from|import)\s*["'](?:\/|[A-Za-z]:[/\\])) have no such self-test, so a regex regression (e.g. an unanchored 'from'/'import' matching inside a JSDoc comment) would only surface as a spurious CI failure against real dist output. Minor: add a small matcher self-test mirroring the existing pattern.
🟡 LOW exactVersion duplicates exactDevelopmentPin logic — scripts/verify-package.mjs
exactVersion (line 293) is the same /^\d+.\d+.\d+$/ + throw shape as exactDevelopmentPin (line 285); only the error message differs via the description param. exactDevelopmentPin could call exactVersion (or a shared helper) with the package name as the description, removing the duplicated regex and control flow. Nit only; no behavior change.
🟡 LOW new portability matcher has no self-test despite the established pattern in this script — scripts/verify-package.mjs
The sibling regex matcher hasStaticImport is guarded by assertEdgeUnsafeStaticImportMatcher (lines 413-428) with labeled positive/negative cases run on every invocation; assertPortableDeclarations and exactVersion get no equivalent, so a regex regression (e.g., the import()-form gap in the previous finding) ships silently. declarationFiles (line 440) also duplicates the javascriptFiles walker shape for a one-suffix difference. Adding a labeled case table like the sibling's would match the file's own convention.
🟡 LOW zod manifest check demands exact pin, contradicting the file's own caret-range rationale — scripts/verify-package.mjs
if (installedAgentInterface.dependencies?.zod !== zodVersion)requires agent-interface to declare an EXACT zod version. This is the opposite of the principle documented at lines 181-183, where assertCaretAdmits is used because 'the cohort is proven by the single installed copy, not by the specifier's shape' and exact pins duplicate packages. If agent-interface later declareszod: ^4.5.0(a caret range that npm would still dedupe to a single 4.5.4 copy),installedZod.version === zodVersionpasses but this check throws 'agent-interface must use zod 4.5.4, received ^4.5.0', failing CI with no actual dual-zod problem. It also couples agent-knowledge CI t
🟡 LOW Explicit record interface can drift from the schema; the ZodType annotation guards only one direction — src/kb-improvement/contracts.ts
KnowledgeImprovementActivationRecord is now a hand-maintained interface mirrored against knowledgeImprovementActivationRecordSchema, annotated as z.ZodType. TS only enforces that the schema's inferred output is assignable to the interface. It does not enforce the reverse: if a future edit adds a required field to the schema but not the interface, the schema output remains assignable to the narrower interface and tsc passes while .parse() silently returns objects carrying extra data. It also cannot catch a schema that weakens a constraint (interface declares candidateId: string; the schema enforces safePathSegmentSchema — a value constructed without .parse() would only fail at runtime). Current state is fully in sync (verified by round-trip parse, tsc,
🟡 LOW Interface loosens candidateId from safePathSegmentSchema to plain string — src/kb-improvement/contracts.ts
The hand-written interface types candidateId as
stringwhile the schema constrains it via safePathSegmentSchema (min 1, max 128, [A-Za-z0-9][A-Za-z0-9._-]*). A caller could construct a type-valid record with candidateId '../escape' that parse then rejects. Every persistence path (persistKnowledgeActivationResult at activation.ts:152 and loadKnowledgeActivationRecord at activation.ts:207) funnels through parse, so no unvalidated value reaches disk; the widening matches the file's existing convention (KnowledgeImprovementCandidateRecord.candidateId is also plain string). Informational only.
🟡 LOW ZodType annotation widens schema type and drops compile-time excess-field drift check — src/kb-improvement/contracts.ts
Annotating as
z.ZodType<KnowledgeImprovementActivationRecord>instead of the inferredZodObject<...>means (a) future consumers cannot .extend/.pick/.shape without a cast, and (b) assignability is one-directional: a schema output field added later but absent from the interface would compile silently until .strict() rejects a valid-looking record at parse time. Current mitigation is adequate (tests/kb-improvement/contracts.test.ts:33-36 round-trips an interface-typed fixture through parse with toEqual, and strict() throws on any field the interface declares but the schema drops), so this is a nit, not a defect. Optionally add a type-level exactness assertion (e.g. a satisfies/Equal check in the test) if the schema grows.
🟡 LOW Alignment test covers only one outcome variant — tests/kb-improvement/contracts.test.ts
The
z.ZodType<KnowledgeImprovementActivationRecord>annotation declares thatparse()returns the hand-written interfaceAgentImprovementActivationResult, while runtime validation usesagentImprovementActivationResultSchema. These are two independent type sources that can drift. The new guard test (tests/kb-improvement/contracts.test.ts:35) round-trips a single representative record whoseoutcomeis only{ status: 'expired' }, but the schema'soutcomeis a 6-variant discriminated union (applied,already-applied,conflict,expired,unsupported,failed,indeterminate). Drift in any of the other five variants would not be caught. Impact is type-only (no runtime change); fix by round-tripping one record per outcome variant or adding a compile-timesatisfiescheck
🟡 LOW Fixture mutation asserts changed=false with differing hashes — tests/kb-improvement/contracts.test.ts
The fixture sets changed: false while beforeHash (64xa) differs from afterHash (64xb). The receipt schema has no cross-field refinement so this is schema-valid (the passing test proves it), but it documents a semantically inconsistent receipt as the canonical example. Fix: either set changed: true, or make beforeHash === afterHash with changed: false.
🟡 LOW Mutation receipt transactionId only tested as null — tests/kb-improvement/contracts.test.ts
knowledgeImprovementMutationReceiptSchema constrains transactionId to z.string().uuid().nullable(), but the fixture only ever sets null, so the .uuid() branch is never exercised. A uuid rejection bug would pass this suite. Minor gap; the positive uuid path is left to activation.test.ts.
🟡 LOW No accept-boundary tests for safePathSegmentSchema — tests/kb-improvement/contracts.test.ts
Only the fixture 'candidate-1' exercises the accept path of safePathSegmentSchema (/^[A-Za-z0-9][A-Za-z0-9.-]*$/). Portable edge ids that the production writer (src/kb-improvement/workspace.ts:287, state.ts:249) will happily store — 'candidate.sub', 'candidate-child_1', and the 128-char max-length boundary — are unpinned. A future tightening of the regex (e.g. banning '') would silently reject ids the filesystem layer accepts and this test would not catch it. Low because the negative cases are the security-relevant ones and they are covered.
🟡 LOW No negative tests for record-level strictness or mutation receipt fields — tests/kb-improvement/contracts.test.ts
The it.each block only rejects bad candidateId values. The schema guards the durable read path (loadKnowledgeActivationRecord parses untrusted disk JSON at src/kb-improvement/activation.ts:207), and nothing verifies rejection of an unknown extra key, a wrong kind literal, a non-uuid transactionId, a non-hex beforeHash, or an out-of-enum target. Impact: a future weakening of .strict() or the receipt schema would not fail this test. Fix: add an it.each of invalid records covering kind, mutation.target, mutation.transactionId, and mutation.beforeHash.
🟡 LOW Only the 'expired' outcome variant is exercised — tests/kb-improvement/contracts.test.ts
The fixture always uses outcome: { status: 'expired' }, the only outcome variant with no targets array. The production shapes persisted by persistKnowledgeActivationResult are 'applied' (transactionId + targets tuple) and 'already-applied'/'conflict' (targets state tuple). Impact: the round-trip type-alignment claim is proven only for the degenerate variant. Fix: parameterize the fixture over at least the 'applied' outcome with a single target transition. Mitigated by the result schema being owned and tested by @tangle-network/agent-interface.
🟡 LOW Path-rejection coverage is narrow — tests/kb-improvement/contracts.test.ts
The it.each covers three non-portable paths (../, backslash, NUL) but not the other safePathSegmentSchema constraints: empty string (min(1)), >128 chars (max(128)), a leading non-alphanumeric such as '-foo' or '.foo', or a leading digit. These are the boundary conditions most likely to regress if the schema is loosened later, and the test's stated purpose is to pin the runtime contract. Add cases for '', 'x'.repeat(129), and '-leading' to lock the full regex. Non-blocking; the tested cases are all correct.
🟡 LOW it.each asserts bare toThrow without message or field — tests/kb-improvement/contracts.test.ts
Rejection assertions use expect(...).toThrow() with no message match, so they cannot distinguish a candidateId rejection from an unrelated schema failure. Sound today because the fixture is proven valid by the round-trip test and only candidateId differs, but asserting the zod path (e.g. /candidateId/) would make the intent explicit and the failure diagnosis faster.
tangletools · 2026-08-31T16:53:36Z · trace
tangletools
left a comment
There was a problem hiding this comment.
✅ Approved — 32 non-blocking findings — c63e8465
Full multi-shot audit completed 7/7 planned shots over 7 changed files. Global verifier still owns final merge decision. | Full multi-shot audit completed 7/7 planned shots over 7 changed files. Global verifier still owns final merge decision. | Full multi-shot audit completed 7/7 planned shots over 7 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-31T16:53:36Z · immutable trace
tangletools
left a comment
There was a problem hiding this comment.
✅ Auto-approved drewstone PR — 31d74c18
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-31T16:54:34Z
Summary
@tangle-network/agent-knowledgeas11.0.0for the Agent Interface2.0.0and Agent Eval0.171.0cohort.4.5.4to match both cohort packages and keep one schema implementation.0.171.0became public.Verification
pnpm exec biome check src tests scripts/verify-package.mjspnpm typecheckpnpm exec vitest run tests/kb-improvement/contracts.test.ts tests/kb-improvement/activation.test.ts(13 passed)pnpm build(35 files)pnpm run verify:package(public Eval0.171.0, Interface2.0.0, Core0.9.6, and Zod4.5.4; portable declarations and five package imports passed)pnpm outdated --recursive(no output; exit 0)Release prerequisite
Eval
0.171.0is public on npm and PyPI.Its npm tarball shasum is
c82e612719fe793b6b37f73c720789b3205c0eae.Its npm integrity is
sha512-SBbzZIDdoqeA8QmbpnGAKpOO5hDBgCIMNnmt2/vDpvTqCKpOA+tqQw8vROx183DrMPC0rmmnVR8qWBq508dF4w==.