feat(intelligence): ship reviewable improvement lifecycle#511
Conversation
tangletools
left a comment
There was a problem hiding this comment.
✅ Auto-approved drewstone PR — 41d68861
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-11T07:31:15Z
tangletools
left a comment
There was a problem hiding this comment.
🟡 Value Audit — sound-with-nits
| Verdict | sound-with-nits |
| Concerns | 2 (1 medium-concern, 1 weak-concern) |
| Heuristic | 0.0s |
| Duplication | 0.0s |
| Interrogation | 258.0s (2 bridge agents) |
| Total | 258.0s |
💰 Value — sound-with-nits
Adds a content-addressed propose→review→execute lifecycle that composes existing analyst/improve/candidate-execution primitives into the missing human-approval front door — coherent and in-grain, with one hand-rolled profile projection that belongs in the substrate.
- What it does: Exposes three public functions (+2 verifiers) in
src/intelligence/improvement-cycle.ts:proposeAgentImprovement(improvement-cycle.ts:156) runsrunAnalystLoop+ held-outimprove(), optionally seals an executable bundle via a callerbuildCandidateadapter, and emits a canonical-digestedAgentImprovementProposal;reviewAgentImprovementProposal(198) emits a digested approve/reject/requ - Goals it achieves: Close the gap where Intelligence could analyze and generate measured candidates but had no durable, human-reviewable, fail-closed path to execution. After merge: a promotion executes only the exact bundle a human approved, every stage is digest-bound and tamper-evident, the linkage lands on the run trace for audit, and live trace export stays best-effort while candidate execution stays fail-closed
- Assessment: Sound. It is glue, not a reimplementation: it composes
runAnalystLoop,improve(), and the existing candidate-execution verify/prepare/execute primitives (candidate-execution/verify.ts:37, prepare.ts:97, execute.ts:75) over the established content-addressed grain (canonicalCandidateDocument,Sha256Digest,immutableCandidateValue). The fail-closed ordering (verify proposal → verify review - Better / existing approach: Searched for an existing lifecycle: analyst-loop (proposeFromFindings/withheld_for_review), mcp/delegates
CoderReview, and deliveryProposedProfileDiff/applyAgentProfileDiff— none provide a persisted, digest-bound propose→review→execute over sealed candidate bundles, so this is the right layer, not a duplicate. The one real design smell iscandidateProfileAsAgentProfile+publicResource/` - Model: opencode/kimi-for-coding/k2p7
- Bridge attempts: 1
🎯 Usefulness — sound
The reviewable improvement lifecycle is a clean, digest-bound propose→review→execute composition that sits naturally on top of the existing analyst, improve(), and candidate-execution primitives — no competing pattern, no dead surface.
- Integration: The three new public verbs are exported from the public ./intelligence subpath (src/intelligence/index.ts:108-114) and are reachable as the package's published API. They compose exclusively existing primitives: runAnalystLoop (src/analyst-loop), improve() (src/improvement/improve.ts:465), sealAgentCandidateBundle (a 17-line wrapper over canonicalCandidateDigest + agentCandidateBundleSchema in src/
- Fit with existing patterns: Fits the codebase's grain precisely. The prior PRs (#507 candidate execution, #509 protected model grants) built the immutable verify→prepare→execute→receipt pipeline specifically as the foundation for this lifecycle. The surfaces.ts expansion (skills, mcp, hooks, subagents, workflows, code, rolloutPolicy, agentProfile) mirrors the ImproveSurface union in src/improvement/improve.ts:75-86 one-to-on
- Real-world viability: Holds up under stress. Every boundary is digest-verified: proposeAgentImprovement binds the candidate profile hash + optional sealed bundle (improvement-cycle.ts:185-190), reviewAgentImprovementProposal refuses approval without both a passing measured gate AND a sealed bundle (improvement-cycle.ts:205-211), and executeApprovedAgentCandidate checks review.decision, proposalDigest match, candidateBu
- Model: opencode/zai-coding-plan/glm-5.2
- Bridge attempts: 1
💰 Value Audit
🟠 Hand-rolled sealed↔public AgentProfile projection will drift with the substrate [maintenance] ``
candidateProfileAsAgentProfilepluspublicResource/publicValue/mapPublicValues/shellQuote(improvement-cycle.ts:324–439) manually re-project the sealedAgentCandidateProfileinto a publicAgentProfile(re-wrapping mcp args/env, hooks command strings, resource refs) just soassertCandidateProfileBindingcan compare digests across agent-interface's two schemas. This enumerates profile fields by name, so any new field agent-interface adds is silently dropped from the binding check o
🟡 canonicalJsonValue pre-pass is largely redundant with the canonical digest [duplication] ``
canonicalJsonValue(improvement-cycle.ts:487–506) is applied toevaluationandfindings(177–178) before embedding, but the proposal digest is then computed over the wholewithoutDigestobject viacanonicalCandidateDocument→canonicalCandidateBytes→canonicalJson(digest.ts:12–18), which already canonicalizes those nested values. The only added behavior is strippingundefinedand rejecting non-finite numbers/undefinedarray entries. Confirm whethercanonicalJsonalready covers
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.
✅ Auto-approved drewstone PR — 40d53334
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-11T07:39:49Z
tangletools
left a comment
There was a problem hiding this comment.
🟢 Value Audit — sound
| Verdict | sound |
| Concerns | 1 (1 weak-concern) |
| Heuristic | 0.0s |
| Duplication | 0.0s |
| Interrogation | 231.0s (2 bridge agents) |
| Total | 231.0s |
💰 Value — sound
Composes the existing measured-improvement + sealed-candidate + fail-closed-execution primitives into a public, digest-bound propose→review→execute-approved lifecycle that the runtime genuinely lacked; in-grain, proportionate, no existing equivalent found.
- What it does: Adds two new source files (
src/candidate-execution/bundle.ts,src/intelligence/improvement-cycle.ts) and wires them into the public API.proposeAgentImprovementrunsrunAnalystLoop+improve(), optionally seals anAgentCandidateBundlevia the newsealAgentCandidateBundle, and emits a content-addressedAgentImprovementProposal.reviewAgentImprovementProposalproduces a digest-boun - Goals it achieves: Close the gap where Intelligence could analyze and generate candidates but the public runtime exposed no durable, human-reviewable, fail-closed path from measured recommendation to approved execution. After merge: a product gets one canonical, tamper-evident lifecycle (proposal→review→approved-execution→receipt) instead of each hand-wiring its own; live trace export stays best-effort while candida
- Assessment: Good, coherent, in the codebase's grain. The lifecycle reuses already-shipped, already-tested substrate —
runAnalystLoop,improve(),canonicalCandidateDigest/canonicalCandidateDocument/omitTopLevelDigest/immutableCandidateValue,verifyAgentCandidateBundle,prepareAgentCandidateExecution,executePreparedAgentCandidate(all from #507/#509, confirmed present pre-PR). The only net-ne - Better / existing approach: none — this is the right approach. I checked for an existing equivalent:
loop-runner.ts'review'mode is a code-worktree review loop, not an improvement approval gate;types.ts:387proposal_createdis an unrelated runtime stream event;analyst-loopreview-then-apply (autoApplyoff,withheld_for_review) emits unstructured edits with no sealed bundle, digest-bound review, or execute-ap - Model: opencode/kimi-for-coding/k2p7
- Bridge attempts: 1
🎯 Usefulness — sound
A coherent proposal→review→execute lifecycle that binds existing analyst/improvement/candidate-execution primitives with digest-linked accountability, fail-closed approval gates, and OTLP evidence export — fills a real gap with no dead surface or competing pattern.
- Integration: All three verbs are exported from the ./intelligence subpath (intelligence/index.ts:108-114) and compose existing primitives: runAnalystLoop, improve, sealAgentCandidateBundle/verifyAgentCandidateBundle/prepareAgentCandidateExecution/executePreparedAgentCandidate. CandidateExecutionEvidence is threaded into RunRecord/RunReport (intelligence/index.ts:183-184,583-601) and forwarded by withIntelligen
- Fit with existing patterns: Thin composition over established primitives — does not reimplement the analyst loop, improvement loop, or candidate-execution pipeline. Matches the codebase grain of composing lower-level primitives into a public surface. Complementary to the remote withIntelligence→pullCertified receive path (which delivers remotely-certified improvements); this lifecycle produces locally-measured improvements.
- Real-world viability: Deliberately fail-closed: no skill write-back pre-approval (improvement-cycle.ts:159-161), approve refused without ship verdict + sealed bundle (:205-212), execute verifies proposal/review/bundle digests + bundle↔review binding + product-owned authorizeReview callback (:231-244), judge-derived findings rejected (:166-169), canonical digest re-verified on every verify (:280-294,456-457), candidate-
- Model: opencode/zai-coding-plan/glm-5.2
- Bridge attempts: 1
💰 Value Audit
🟡 candidateProfileAsAgentProfile hand-enumerates AgentProfile fields [maintenance] ``
improvement-cycle.ts:324-406 rebuilds an AgentProfile from a bundle's candidate profile by listing field names ('name','prompt',...,'resources'). If AgentProfile gains a sealable field this allowlist must be updated or new fields silently drop. It is bounded by
assertCandidateProfileBinding(rejects connections/metadata/extensions and re-digests), so it cannot ship a skewed profile — but it is a maintenance coupling to watch. Not a ship-blocker; the boundary is deliberate.
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.
❌ Needs Work —
|
| opencode-kimi | glm | deepseek | aggregate | |
|---|---|---|---|---|
| Readiness | 0 | 20 | 0 | 0 |
| Confidence | 95 | 95 | 95 | 95 |
| Correctness | 0 | 20 | 0 | 0 |
| Security | 0 | 20 | 0 | 0 |
| Testing | 0 | 20 | 0 | 0 |
| Architecture | 0 | 20 | 0 | 0 |
Reviewer score is advisory once the run is complete and the verdict has no blockers.
Full multi-shot audit completed 8/8 planned shots over 39 changed files. Global verifier still owns final merge decision. | Full multi-shot audit completed 8/8 planned shots over 39 changed files. Global verifier still owns final merge decision. | Full multi-shot audit completed 8/8 planned shots over 39 changed files. Global verifier still owns final merge decision.
Blocking
🔴 HIGH Generated mcp.md union line permuted off source order — breaks enforced docs:check freshness gate — docs/api/mcp.md
Line 836 now renders
recommendation:"ship" \| "reject" \| "approve-with-nits" \| "changes-requested"withDefined in: mcp/delegates.ts:102. The cited sourcesrc/mcp/delegates.ts:102declaresrecommendation: 'ship' | 'approve-with-nits' | 'changes-requested' | 'reject'and is byte-identical base→head (PR does not touch this file). typedoc.jsonsort: ["source-order"]and the generator versions (typedoc 0.28.19, typedoc-plugin-markdown 4.12.0, typescript ^5.7.0) are all unchanged base→head, and the base doc already emitted source order — so a freshpnpm docs:apimust reproduceship | approve-with-nits | changes-requested | reject, never the committed order. Ev
🔴 HIGH New surface kinds lack path traversal protection — src/agent/surfaces.ts
The
skill(L154-159),mcp(L173-180),hook(L181-186),subagent(L187-193), andworkflow(L194-200) cases construct paths viajoin(surfaces.*, subject.name, ...)withoutsafeJoinwrapping. Thesubject.name,subject.server, andsubject.toolfields originate fromparseFindingSubjectwhich parses LLM-generated finding strings — effectively untrusted input. Thecodecase (L206-209) in the same PR does usesafeJoin, and tests exist for traversal rejection oncode/knowledge.raw/rag(tests/agent.test.ts L222-234). No traversal-rejection test exists forskill/mcp/hook/subagent/workflow. An LLM-generated subject likekind:mcp server:../secretswould producemcp/../secrets/README.mdand escape themcpsurface root.
🔴 HIGH New security-critical improvement-cycle module ships with no tests — src/intelligence/improvement-cycle.ts
improvement-cycle.ts is an Added file (506 lines) implementing the propose/review/execute/verify approval chain — exactly the code that decides whether a measured candidate may run. It contains the canonical-digest binding (verifyAgentImprovementProposal lines 270-296), the review↔proposal↔bundle linkage checks (executeApprovedAgentCandidate lines 228-267), the measured-vs-bundled profile normalization contract (assertCandidateProfileBinding / candidateProfileAsAgentProfile [lines 310-406](https://github.
Other
🟠 MEDIUM Path-traversal sanitization applied asymmetrically — new surface kinds join raw subject components — src/agent/surfaces.ts
The PR adds safeJoin() and routes
code,knowledge.raw, andragdocId through it, and its new tests (tests/agent.test.ts:222-235) establish resolveSubjectPath as the sanitization boundary by constructing subjects directly and expecting null. But the new kinds added in the same function join untrusted components raw:join(surfaces.skills, subject.name, 'SKILL.md')(line 157), mcp server/tool (lines 176-180), hook/subagent/workflow names ([lines 184](https://github.com/tangle-network/agent-runtime/blob/40d53334b8fdf7ca5d91657075a128c5d8
🟠 MEDIUM rag case: subject.corpus bypasses safeJoin via join() normalization — src/agent/surfaces.ts
The
ragcase callssafeJoin(join(surfaces.rag, subject.corpus), ...).safeJoinonly validates the second argument (child). The first argument (root) isjoin(surfaces.rag, subject.corpus)— ifsubject.corpus = '../../etc', Node'spath.joinnormalizes this to'etc'beforesafeJoinever sees it.safeJoin('etc', 'doc.md')returns'etc/doc.md', escaping the rag directory. The existing test (tests/agent.test.ts L232-234) only testsdocIdtraversal, notcorpustraversal. The corpus field should be validated separately, or the entire rag path construction should use a two-stage safeJoin:safeJoin(safeJoin(surfaces.rag, subject.corpus), docId).
🟠 MEDIUM Plan-declared grader identity is never enforced against the grader actually executed — src/candidate-execution/prepare.ts
This PR signs
task.graderinto the canonical execution plan (prepare.ts:292) and validates it (prepare.ts:571-577), and types.ts:154 adds the identity type — but nothing downstream compares it to the grader that actually runs.persistCandidateBenchmarkResult(outcome-evidence.ts:275-279, unchanged) writesgraderinto the benchmark-result material fromgraded.grader, i.e. the grader PORT's self-reported name/version/artifact snapshot, andrunBoundCandidateBenchmarkGrader(benchmark-grader.ts:60-72) only binds implementation bytes ↔ task outcome ↔ output. Grep of the repo confirms no code readsexecutionPlan.material.graderafter prepare. Impact: an evaluator can declare grader A in the signed plan yet run grader B at score time; the two content-addressed artifacts silently disag
🟠 MEDIUM The code-surface ship + winner-retention cleanup path is completely untested — src/improvement/improve.test.ts
The only code-surface test uses a flat judge (composite 0.5 for everything) and asserts
result.shipped === false(line 449), sopreparedCode.cleanup(retainedWinner)is only ever exercised withretainedWinner === undefined. The retention logic is the most data-loss-sensitive new code in this shot:isCodeSurface()discriminant check (surface.kind === 'code', improve.ts:311-313), the worktreeRef match against the driver'sfinalizedmap (improvement-driver.ts:127, 136-143), and skipping discard for the retained winner. If a code run ever ships and retention fails (e.g. the substrate's CodeSurface discriminant differs), the shipped winner worktree wou
🟠 MEDIUM Post-ship cleanup failure masks a successful run and hides the shipped winner — src/improvement/improve.ts
In the success path,
await preparedCode?.cleanup(shipped ? raw.winner.surface : undefined)runs AFTERshippedis computed but BEFORE the result is built/returned (lines 551-565). The cleanup impl (lines 346-361) collects errors from BOTHmanaged.cleanup()(losing-candidate discard) andworktree.discard(baselineWorktree)and throws an AggregateError on any failure. Consequence: a transient git/FS failure discarding the ephemeral baseline worktree — or a locked losing candidate — turns a completed, paid-forshipverdic
🟠 MEDIUM Resource leak: baseline worktree not discarded when finalize fails after create succeeds — src/improvement/improve.ts
At line 325,
worktree.create()creates a baseline worktree on disk. At line 326,worktree.finalize()commits it into aCodeSurface. Iffinalizethrows (e.g., git commit failure on a broken repo), thebaselineWorktreeis leaked — it exists on disk butpreparedCodewas never assigned, so the caller's catch block at line 538 has nopreparedCodeto callcleanup()on. The orphaned worktree directory
🟠 MEDIUM Zero test coverage for 506-line improvement-cycle.ts with security-critical digest/approval logic — src/intelligence/improvement-cycle.ts
No test file exists (globbed src/**/improvement-cycle* — only the implementation file). The five exported functions enforce a content-addressed approval chain: proposal digest over canonical JSON, candidate bundle re-sealing, profile-bundle binding via candidateProfileAsAgentProfile normalization (including shellQuote for hooks), approval gate (gateDecision==='ship' + sealed bundle required), review↔proposal digest binding, and authorizeReview callback. None of these paths are exercised by any test in this PR. The with-intelligence.test.ts only passes opaque sha256 strings through candidateExecution. At minimum, a propose→verify round-trip, an approve-without-bundle rejection, a tampered-digest rejection, and a profile-bundle mismatch rejection are needed before shipping.
🟠 MEDIUM canonicalJsonValue silently drops undefined object properties, throws on undefined array entries — src/intelligence/improvement-cycle.ts
Line 493-496:
[undefined]throws'undefined array entry at $[0]'. Line 500-501:{a: undefined}silently becomes{}. This asymmetry means undefined properties in objects are silently dropped from the canonical JSON, while undefined array entries halt the computation. If a finding or evaluation field has an undefined property (e.g., an optional field), it's silently removed from the digest computation, potentially hiding missing data. Fix: either throw for undefined object values too, or handle both
🟠 MEDIUM Profile hash and byte count mismatch in exportRunRecord — src/intelligence/index.ts
addPayloadAttributes (line 604) sets tangle.agent.profile_hash from the REDACTED profile and tangle.agent.profile_bytes from the redacted profile's serialized length. Line 610 then overwrites tangle.agent.profile_hash with contentHash(record.profile) — the RAW profile hash. But tangle.agent.profile_bytes is NOT overwritten, so it remains the byte count of the redacted profile. Result: the hash and byte count refer to different underlying data. A consumer that validates the hash against the byte count would fail. Fix: either move [line 610
🟠 MEDIUM Profile hash overwritten with unredacted content, breaking hash-content consistency — src/intelligence/index.ts
Line 604-609 calls addPayloadAttributes(labels, 'tangle.agent.profile', redactor(record.profile), includeFullPayload), which sets tangle.agent.profile_hash = contentHash(serializeJson(redacted)) and tangle.agent.profile_bytes = byteLength(redacted). Line 610 then OVERWRITES: labels['tangle.agent.profile_hash'] = contentHash(record.profile) — the UNREDACTED profile hash. Result: when payloadAttributes='full', the exported tangle.agent.profile is the redacted profile but _hash is of unredacted — a consumer cannot verify the hash agains
🟠 MEDIUM serializeJson unbounded for full-payload exports — src/intelligence/index.ts
The old previewJson had a 4096-char truncation limit. The new serializeJson removes the limit entirely. When payloadAttributes='full', a multi-megabyte input/output could produce span attributes exceeding OTLP protocol limits (~128KB per span), causing export failures or excessive memory. The config comment on line 257-262 documents this as opt-in, but there's no safety valve in code (no size warning, no truncation fallback). Fix: add a configurable max-size guard or a warning log when serialized content exceeds a safe threshold.
🟠 MEDIUM tangle.agent.profile hash/bytes/content describe two different values — src/intelligence/index.ts
In the
if (record.profile)block (lines 603-611), addPayloadAttributes(labels,'tangle.agent.profile', redactor(record.profile), includeFullPayload) sets tangle.agent.profile_hash = contentHash(serializeJson(redactor(...))), tangle.agent.profile_bytes = byteLength of the redacted serialization, and (when full) tangle.agent.profile = redacted content. The very next statement (line 610) then OVERWRITES labels['tangle.agent.profile_hash'] = contentHash(record.profile) — a hash of the RAW, un-redacted, un-serialized object. End state: _
🟠 MEDIUM No attribute value size cap — large tool payloads risk silent collector drop — src/otel-export.ts
serialized() produces unbounded strings. For tool_call/tool_result events with includeControlPayloads enabled, the payload is serialized TWICE: once inside tangle.runtime.event (full record JSON) and again as tool.input/tool.output (line 289). A 5MB tool result becomes ~10MB of span attributes. OTLP/JSON collectors enforce per-attribute limits (Jaeger default 1024 chars, Tempo configurable) and will silently truncate or drop these. The test at tests/otel-export.test.ts:35 explicitly asserts no truncation occurs, confirming this is by design — but there is no caller-side truncation either. For production tool outputs (file reads, search results), this will cause data loss
🟠 MEDIUM Non-finite latencyMs crashes the span builder (RangeError from BigInt) — src/otel-export.ts
endMs = startMs + event.latencyMs(lines 313-316) feedsflatOtelSpan→msToNs→BigInt(Math.floor(ms))(line 623). For a type-legalllm_callevent withlatencyMs: NaNorInfinity, this throwsRangeError: The number NaN/Infinity cannot be converted to a BigInt— reproduced in a throwaway vitest probe.RuntimeStreamEventis a public @stable type any custom backend can emit, and the sibling builder in this same file (buildLoopSpanNodes) guards every numeric with aNumber.isFinitenum()helper, so the hardening pattern already
🟡 LOW Generated docs reorder union members differently than source — docs/api/mcp.md
The generated docs show
'ship' | 'reject' | 'approve-with-nits' | 'changes-requested'but src/mcp/delegates.ts:102 (unchanged in both base and head) has'ship' | 'approve-with-nits' | 'changes-requested' | 'reject'. Union order is semantically irrelevant but doc-source divergence erodes trust in generated docs. Root cause is likely non-deterministic ordering in typedoc or the docs generator. Not a code bug, but nits-worthy.
🟡 LOW mcp.md recommendation union reordered without source change (noise diff) — docs/api/mcp.md
The doc line changed from
"ship" | "approve-with-nits" | "changes-requested" | "reject"to"ship" | "reject" | "approve-with-nits" | "changes-requested", butgit diff base head -- src/mcp/delegates.tsis EMPTY — the source at delegates.ts:102 still has the original order. The value SET is identical (TS unions are unordered), so this is semantically a no-op, but it produces a misleading diff implying a source change. Likely a TypeDoc ordering side-effect from a transitive type rebuild. Not blocking; flag only because noise-diffs in generated docs reduce the signal value of future regens.
🟡 LOW No test for optional surface type mismatch — src/agent/surfaces.ts
The
validateSurfacesfunction (L332-337) checks that optional surfaces are the right type (directory vs file), and a test covers this for required surfaces (tests/agent.test.ts L619-636). But no test covers the optional-surface path — e.g., declaringskills: 'not-a-dir'wherenot-a-diris an existing file. A missing continue/fallthrough bug here would be undetected.
🟡 LOW Stale operator-facing docs omit the 8 new optional surfaces — src/agent/surfaces.ts
The error message rendered when validation fails (lines 355-359) and the AgentSurfaces doc comment (line 33) still say optional surfaces are only 'scaffolding / memory / rag / outputSchema'. Operators hitting a validation failure get guidance that doesn't mention skills/mcp/hooks/subagents/workflows/rolloutPolicy/agentProfile/code, the surfaces this PR adds. Cosmetic but user-facing in the exact error path this PR extends.
🟡 LOW Unchanged error message doesn't mention new surfaces — src/agent/surfaces.ts
The error message at L356-358 reads: 'Optional surfaces (scaffolding / memory / rag / outputSchema) may be omitted; the loop will reject...' — this lists only the original 4 optional surfaces, not the 8 new ones added in this PR (skills, mcp, hooks, subagents, workflows, rolloutPolicy, agentProfile, code). Operators who misconfigure new surfaces will see incomplete guidance in the error.
🟡 LOW memory documented as a file store but new isDirectory() enforcement rejects file-backed memory — src/agent/surfaces.ts
The interface doc comment (line 50) declares memory as 'memory store path (JSONL / SQLite / DB)' — a file. The PR's new type enforcement puts
memoryin optionalDirSurfaces and flags any existing file as 'not-directory', failing defineAgent at startup. Verified: validateSurfaces({...surfaces, memory: 'memory.jsonl'}, root) → [{ surface: 'memory', path: 'memory.jsonl', reason: 'not-directory' }]. Pre-PR this manifest validated cleanly, so this is a silent breaking change for consumers following the documented contract (the memory routing at [line 215](agent-runtime/src/agent/surfaces.ts
Line 21 in 40d5333
🟡 LOW existsSync + statSync TOCTOU can crash validation instead of reporting an issue — src/agent/surfaces.ts
The new checks call existsSync(abs) and then statSync(abs) on the next line (307, 320, 333/335). If the path is removed or permissions change between the two calls, statSync throws ENOENT/EACCES and validateSurfaces crashes at defineAgent time instead of returning a SurfaceValidationIssue — the opposite of the function's fail-loud-with-issues contract. Fix: replace existsSync+statSync with a single try/catch around statSync, mapping ENOENT/ENOTDIR to reason 'missing'.
🟡 LOW renderSurfaceIssues footer omits the newly-added optional surfaces — src/agent/surfaces.ts
The help footer still reads 'Optional surfaces (scaffolding / memory / rag / outputSchema)' but the validated set now also includes skills / mcp / hooks / subagents / workflows / code / rolloutPolicy / agentProfile. An operator who hits a 'not-directory' issue on e.g.
skillswon't see it acknowledged in the rendered guidance. The rendered (i.reason) value is correct; only the prose is stale. Fix: either drop the parenthetical list ('Optional surfaces may be omitted; ...') or regenerate it from the optionalDirSurfaces/optionalFileSurfaces arrays so it can't drift again.
🟡 LOW statSync after existsSync is TOCTOU-fragile (theoretical) — src/agent/surfaces.ts
Pattern
if (!existsSync(abs)) {...} else if (!statSync(abs).isDirectory()) {...}appears 4x (lines 305-309, 318-322, 333-337). If the path is deleted between existsSync and statSync, statSync throws ENOENT uncaught, crashing defineAgent with a raw Node error instead of a clean SurfaceValidationIssue. Window is microseconds and this runs once at startup, so practical impact is near-zero; a try/catch around statSync (or replacing the existsSync+statSync pair with a single statSync in a try/catch) would make it robust. Not a merge blocker.
🟡 LOW No dedicated unit test for sealAgentCandidateBundle digest mismatch path — src/candidate-execution/bundle.ts
The function's core invariant (parsedDigest !== digest -> throw) is never directly tested. It's exercised indirectly through verifyAgentImprovementProposal in improvement-cycle.test.ts, but only on valid input. A direct test passing schema-valid-but-mutated input (or a mock schema that transforms) would prove the guard fires. Impact: a future schema change adding a transform could silently break content-addressing without test coverage catching it.
🟡 LOW AgentCandidateBundleInput not exported from candidate-execution/index.ts despite appearing in exported intelligence API — src/candidate-execution/index.ts
AgentCandidateBundleInput is the return type of ProposeAgentImprovementOptions.buildCandidate (improvement-cycle.ts:118), and ProposeAgentImprovementOptions is exported via the intelligence barrel. However, AgentCandidateBundleInput itself is only importable from './bundle', not from the candidate-execution barrel. Currently not a blocker because src/index.ts does not re-export the intelligence module, making this internal-only. But if intelligence is ever surfaced as a public entry point, consumers cannot import the named type. Fix: add
type AgentCandidateBundleInputandsealAgentCandidateBundleto the index.ts export block alongside the existing types.
🟡 LOW sealAgentCandidateBundle / AgentCandidateBundleInput are not exported from the candidate-execution index — src/candidate-execution/index.ts
bundle.ts is absent from index.ts's export list, so the new
./candidate-executionpackage subpath (package.json, added by this PR) cannot reachsealAgentCandidateBundleor theAgentCandidateBundleInputtype. Yet the PR's regenerated public docs (docs/api/intelligence.md:1335) nameAgentCandidateBundleInputas theProposeAgentImprovementOptions.buildCandidatereturn type, and src/index.ts re-exports only./candidate-execution(index.ts). External consumers cannot import the type from any public entry point (deep imports are blocked by the exports map). Fix: addexport { type AgentCandidateBundleInput, sealAgentCandidateBundle } from './bundle'to index.ts (or export the type from the intelligence entry).
🟡 LOW sealAgentCandidateBundle not exported from candidate-execution public subpath — src/candidate-execution/index.ts
package.json adds a new ./candidate-execution export subpath pointing to dist/candidate-execution/index.js, but sealAgentCandidateBundle and AgentCandidateBundleInput (from bundle.ts) are not re-exported in src/candidate-execution/index.ts. They are only accessible via direct file import from src/intelligence/improvement-cycle.ts. If external consumers are expected to use the candidate-execution subpath, these should be added to the barrel export.
🟡 LOW New grader validation and plan-stamping have no direct test coverage — src/candidate-execution/prepare.ts
The test diff only adds a valid
graderto two fixtures (tests/candidate-execution-prepare.test.ts:269, tests/helpers/candidate-execution-fixture.ts:362). No test exercises the new rejection branches (empty grader name/version,artifact.byteLength <= 0, malformedartifact.sha256at prepare.ts:571-577), and the main binding test asserts many plan fields but neverplan.grader(grep confirms nograderassertion outside the fixture). A regression that dropped or corrupted the grader identity from the plan would pass the suite. Fix: add assertions thatexecutionPlan.value.material.graderequals the declared identity and one rejection case per new validation branch.
🟡 LOW AgentCandidateBenchmarkGraderIdentity missing TSDoc comment — src/candidate-execution/types.ts
The new interface lacks a TSDoc comment, causing it to appear as 'Undocumented supporting type' in the API docs primitive catalog (confirmed at docs/api/primitive-catalog.md:208). All other public types in this file have documentation.
🟡 LOW Error-path worktree cleanup branch is untested — src/improvement/improve.test.ts
The catch block at improve.ts:538-549 (selfImprove throws → cleanup all worktrees → rethrow) is never exercised by any test. The happy-path cleanup (line 552) IS verified via the worktree list assertion in the code-surface test (line 450:
toHaveLength(1)), but a regression that breaks error-path cleanup (e.g., a future change that swallows the cleanup error or skips the discard) would not be caught. Impact: resource leak regressions on error paths go undetected. Fix: add a test that injects a selfImprove failure (e.g., a stu
🟡 LOW Baseline worktree leaks if finalize() throws inside prepareCodeRun — src/improvement/improve.ts
prepareCodeRun is called at line 506 OUTSIDE the try/catch block (which starts at line 520). Inside prepareCodeRun (lines 325-326), worktree.create() succeeds, then worktree.finalize() is called. If finalize() throws (e.g., git commit failure, disk full), the baseline worktree created at [line 325](https://github.com/tangle-network/agent-runtime/blob/40d53334b8fdf7ca5d91657075a128c5d8f8bc5a/src/improvemen
🟡 LOW ImproveOptions type refactor silently inherits all SelfImproveOptions passthrough properties — src/improvement/improve.ts
The type change from
interfacetoOmit<SelfImproveOptions, 'analyzeGeneration' | 'baselineSurface' | 'findings' | 'gate' | 'proposer'> & { ... }means callers can now pass anySelfImproveOptionsproperty (onProgress, onProvenance, collectWorkerRecords, expectUsage, captureSource, autoOnPromote, etc.) that were not previously on the ImproveOptions interface. This is intentional and tested (see 'forwards evaluation controls' test), but it means the public API surface expanded without any deprecation or migration note. If a caller passes a SelfImproveOptions property that selfImprove rejects at runtime, the error surfaces from deep inside selfImprove rather than as a TypeScript type error. Callers who type-check against the published ImproveOptions type should be unaffected, but runtime
🟡 LOW New mandatory input-profile schema validation is a silent breaking change for existing callers — src/improvement/improve.ts
Lines 483-488 add
agentProfileSchema.safeParse(profile)at entry, rejecting any profile the zod schema considers invalid — previously improve() accepted any profile and only touched the lever field. Tests prove minimal{name}fixtures pass and a type-invalid subagent (maxSteps: 'many') is rejected, so the schema is permissive-but-typed. Still, any existing caller passing a persisted profile with a shape the strict schema rejects (unknown to this shot since agent-interface's schema isn't in these files) now gets a ConfigError where it previously ran — including forsurface: 'code'runs that never consume profile fields for the surface. This may be int
🟡 LOW isCodeSurface type-narrowing is structural, not branded — fragile to upstream CodeSurface shape changes — src/improvement/improve.ts
The
isCodeSurfaceguard checkstypeof surface === 'object' && surface !== null && surface.kind === 'code'. This correctly narrowsMutableSurfacetoCodeSurfacegiven the current upstream type definition, but if@tangle-network/agent-eval/campaignadds a new surface variant withkind: 'code'(or changes the discriminator field name), this guard silently starts matching the wrong type, potentially extracting a non-existentworktreeRef. A branded check (e.g., importing and using aCodeSurfacetype guard from the substrate) would be more robust. Not blocking — the current upstream shape is stable.
🟡 LOW skills option doc still promises a refs-array default that now throws — src/improvement/improve.ts
The doc comment (lines 130-134) and baselineSurfaceFor (lines 228-230) still describe 'Without this, surface: skills optimizes the profile's skills REFS array' as the default back-compat behavior. But the new guard at lines 489-493 throws
ConfigErrorwhenever surface==='skills' and neithergeneratornorskillsis provided — so the refs-array baseline is now reachable ONLY when a caller sup
🟡 LOW candidateProfileAsAgentProfile is a tight, fragile equality contract that can falsely reject valid candidates — src/intelligence/improvement-cycle.ts
Approval requires canonicalCandidateDigest(measured) === canonicalCandidateDigest(candidateProfileAsAgentProfile(bundled)) (lines 318-321), and proposeAgentImprovement runs the same check at creation (line 175), so any drift fails closed. The normalization re-derives the public profile: hooks are re-rendered as a shell command via shellQuote joining executable+args ([lines 355-367](https://github.com/tangle-network/agent-runtime/blob/40d53334b8fdf7ca5d91657075a128c5d8f8bc5a/src/intelligence/improvement-cycle.t
🟡 LOW canonicalJsonValue doesn't handle Date/RegExp/Buffer/non-POJO objects — src/intelligence/improvement-cycle.ts
Non-plain objects like Date, RegExp, and class instances fall through to the typeof==='object' branch (line 499). Object.entries(new Date()) returns [], turning such objects into {}. Object.entries(/test/) also returns []. While none of the current improvementEvaluation or finding fields are these types, a future SelfImproveResult or AnalystFinding change adding such types would silently corrupt digest computations. Fix: add an explicit check for plain-object vs exotic-object (e.g., checking Object.prototype.toString).
🟡 LOW payloadAttributes:'full' ships unbounded payload size in a single span attribute — src/intelligence/index.ts
The old previewJson bounded payloads to 4096 chars with a
[truncated]marker. serializeJson (lines 407-419) now returns the full JSON.stringify output and addPayloadAttributes (lines 421-431) writes it verbatim into labels[key] whenever includeFullPayload is true, with no size cap. The with-intelligence test exercises a 5000-char input to prove no truncation. Impact: when an operator opts into payloadAttributes:'full', a multi-MB input/output/profile is emitted as one OTLP attribute; many OTLP backends drop or reject oversized
🟡 LOW Test attrsOf ignores OTLP array/kvlist/bytes attribute types — src/intelligence/with-intelligence.test.ts
The test helper only extracts stringValue, intValue, doubleValue, and boolValue from span attributes. It silently drops arrayValue, kvlistValue, and bytesValue. If any future span attribute uses these OTLP types, tests would report them as undefined, potentially masking bugs. Low severity because no current attributes use these types, but the helper should either handle them or throw on unhandled types to prevent silent test gaps.
🟡 LOW Runtime-event token summarization drops cachedInput and reasoning tokens — src/intelligence/with-intelligence.ts
summarizeRuntimeEvents accumulates only tokensIn/tokensOut (lines 124-126) into summary.inputTokens/outputTokens, and exportCompleted builds the fallback tokens object as { input, output } only (lines 226-230). RunRecord.tokens (index.ts:176-181) supports cachedInput and reasoning, and index.ts:556-567 exports gen_ai.usage.cache_read_input_tokens / reasoning_tokens when present — but those fields are only ever populated from an explicit report.tokens, never from runtimeEvents. Impact: a caller that stream
🟡 LOW Unrecorded successful runs now export tangle.outcome.success = true by default — src/intelligence/with-intelligence.ts
Success is resolved as
report.success ?? (caught !== undefined ? false : (eventSummary.success ?? error === undefined)). For a run that returns normally, records nothing, and emits no runtime events, all inputs are undefined and the expression yieldserror === undefined→ true, so the run span carries tangle.outcome.success = true. The previous code only emitted the success attribute when the agent explicitly recorded it (...(report.success !== undefined ? { success: report.success } : {})), so an un-instrumented run exported no success value. Impact: dashboards/billing that count success by tangle.outcome.success === true will now treat every fire-and-forget run as a success even when the agent's output was never evaluated; this is an unannounced semantic shift. The complement (caug
🟡 LOW exportCompleted not wrapped in try/catch — telemetry-side throw can fail a successful agent turn or mask the original error — src/intelligence/with-intelligence.ts
exportCompleted (lines 222-273) runs summarizeRuntimeEvents and constructs the RunRecord BEFORE calling client.exportRunRecord (which has its own try/catch). The call sites are: try { output = await agent(...); exportCompleted(output); return output } catch (cause) { exportCompleted(undefined, cause); throw cause }. If exportCompleted throws in the success path (e.g., a malformed runtime event causes summarizeRuntimeEvents to throw), the catch block treats the telemetry error as 'cause', calls exportCompleted again (may throw again), and throws the telemetry error — the successful agent output is lost. If exportCompleted throws in the catch path,
🟡 LOW success always derived and defaults to true on no-signal runs — behavior change from optional to always-present — src/intelligence/with-intelligence.ts
The old code (diff -) used: ...(report.success !== undefined ? { success: report.success } : {}) — success was omitted from the RunRecord when the agent didn't report it. The new code (line 240-242) always sets success to a boolean via: report.success ?? (caught !== undefined ? false : (eventSummary.success ?? error === undefined)). When the agent completes without error, without explicit success, and without a final event, success defaults to true (because error === undefined). Downstream consumers that previously distinguished 'success unknown' (attribute absent) from 'success: true' now see success=true unconditionally. The exported span always
🟡 LOW Error span status not tested — backend_error and non-completed final paths unverified — src/otel-export.ts
The span.status override (code 2 for backend_error and non-completed final events) is the most security-relevant path — it's what makes failures visible in trace dashboards. No test covers it. A regression that silently drops the status override would hide all backend errors from operators. Add a test asserting span.status.code === 2 and span.status.message for both a backend_error event and a final event with status 'failed'.
🟡 LOW Hardcoded GenAI semconv strings instead of GEN_AI constant; missing gen_ai.provider.name — src/otel-export.ts
The llm_call branch hardcodes 'gen_ai.usage.input_tokens', 'gen_ai.usage.output_tokens', etc. as string literals instead of reusing the file-level GEN_AI constant (lines 73-79). The file's own comment (line 71) says 'We use provider.name + input/output_tokens' but neither the GEN_AI constant nor this function sets gen_ai.provider.name. Pre-existing gap inherited by the new code. Minor consistency nit; functionally the spans will render in any OTel-compatible viewer but may fail strict GenAI semconv validators.
🟡 LOW Missing test coverage for backend_error, final, and redact callback paths — src/otel-export.ts
Tests cover tool_call (with MCP identity) and llm_call, plus default-redact for tool payloads. The backend_error (lines 300-302), final (lines 303-309), error-status override (lines 318-323), and customer redact callback (line 269) paths have zero test coverage. The logic is straightforward and follo
🟡 LOW Non-finite numeric attrs serialize as invalid OTLP/JSON (doubleValue: null) — src/otel-export.ts
costUsd,tokensIn,tokensOut,latencyMsare copied into attrs without finite checks (lines 294-297);toAttributes(608-620) maps a non-integer number to{ doubleValue: value }, andJSON.stringifyturns NaN/Infinity intonull— confirmed by probe: wire attr becomes{"key":"tangle.cost.usd","value":{"doubleValue":null}}, which violates the OTLP/JSON schema and can get the whole/v1/tracesbatch rejected by a strict collector (telemetry data loss). Same root cause as the RangeError finding; aNumber.isFiniteguard in toAttributes fixes it globally for every builder in this file.
🟡 LOW Recoverable backend_error events always produce ERROR span status — src/otel-export.ts
The condition
event.type === 'backend_error'unconditionally sets status code 2, ignoring the event'srecoverable: booleanfield. OTel convention distinguishes transient/retried errors from terminal ones. Since each event maps to its own span and recovery is a separate final event, this is a defensible per-event semantic — but if the backend retries and succeeds, the trace will still show a red ERROR span for the transient failure. Consider gating on!event.recoverableor documenting the intent.
🟡 LOW Untimestamped events get export-time (Date.now) span timestamps, distorting trace timelines — src/otel-export.ts
Events with missing/unparseable
timestamp(optional on tool_call/tool_result/llm_call/text_delta/reasoning_delta variants) fall back toDate.now()evaluated when spans are BUILT, not when events occurred. The intelligence flow (src/intelligence/index.ts:631) buffers runtimeEvents until run end and builds spans at export time, so every untimestamped event clusters at export wall-clock with zero duration (or start=now/end=now+latency for llm_call), interleaved falsely with correctly-timestamped events. Deterministic from the code; data-fidelity impact only. Consider falling back to monotonic interpolation between neighboring timestamped events or the run span's start, or document the behavior on RuntimeEventOtelOptions.
🟡 LOW serialized has no size bound on span attributes — src/otel-export.ts
tangle.runtime.event (line 276) and tool.input/tool.output (line 289) serialize the full sanitized record via serialized() with no size limit. When includeControlPayloads is true, a large tool result (e.g. 10MB file read) produces an oversized span attribute that some OTEL collectors reject (e.g. Datadog 25KB per-attribute limit). This is by-design 'lossless' per the function JSDoc, but callers should be aware that opting into payloads can silently drop spans if the collector enforces attribute size limits. Consider documenting this in the JSDoc or add
🟡 LOW Fixture grader artifact locators disagree on key — tests/helpers/candidate-execution-fixture.ts
task.grader.artifact.locator uses key 'grader/fixture', while createCandidateOutputFixture's grader port artifact (put at line 233) uses key
grader/<sha256hex>for the same bytes. sha256 and byteLength match, and no src code or test currently compares the two refs (prepare.ts:292 signs task.grader into the plan; outcome-evidence.ts:278 records the port's artifact in the result), so there is no functional break today. But the fixture models an evaluator that pins one immutable grader artifact; the divergent keys weaken that invariant and would produce a confusing failure if a future test deep-equals plan vs result grader artifacts. Fix: derive
🟡 LOW Grader artifact locator key inconsistency between task.grader and output fixture — tests/helpers/candidate-execution-fixture.ts
task.grader.artifact.locator.key is hardcoded as 'grader/fixture' (line 366), while createCandidateOutputFixture's put() generates the key as
grader/<sha256-hex>(line 225). Both refs share the same sha256 and byteLength, and reads are sha256-keyed so this has zero functional impact. However, task.grader is embedded into the signed execution plan material (prepare.ts:292), so the locator key 'grader/fixture' becomes part of canonicalized evidence. For consistency and to avoid confusing future reader
🟡 LOW Locator key inconsistent between task identity and output fixture — tests/helpers/candidate-execution-fixture.ts
The grader artifact locator key is hardcoded as 'grader/fixture' (line 366), while createCandidateOutputFixture's put function (line 225) computes keys as 'grader/<sha256_hex>'. Both reference the same sha256 (same fixtureGraderBytes), so correctness is unaffected — the read path resolves by sha256, not locator. This is a test-only fixture inconsistency. If a future test were to compare artifacts by locator instead of sha256, it would spuriously fail. Align the key format or document the divergence.
🟡 LOW Evidence assertion omits two of the four receipt digests — tests/improvement-cycle.test.ts
executed.evidenceis asserted with toMatchObject over proposalDigest/reviewDigest/bundleDigest/executionId/succeeded/runReceiptDigest, but CandidateExecutionEvidence also carriesexecutionPlanDigestandmaterializationReceiptDigest(improvement-cycle.ts:99-100) — the two digests that bind the receipt to the exact materialized plan. A regression that dropped or corrupted those fields would pass this test (toMatchObject ignores unlisted keys, and it is the file's own stated goal to link 'one exact receipt'). Fix: extend the matchObject withexecutionPlanDigest: expect.stringMatching(/^sha256:/)andmaterializationReceiptDigest: expect.stringMatching(/^sha256:/).
🟡 LOW Happy-path test asserts request presence but not its content — tests/improvement-cycle.test.ts
After execution the test only asserts
expect(request).toBeDefined(). The sibling suite (tests/candidate-execution-execute.test.ts:108-121) asserts the same request carries trace tags, launch env isolation (no PATH leak), and hard/observed limits. Adding e.g.expect(request?.trace.tags).toMatchObject(...)or an env-isolation assertion would make this integration test catch a regression where the approved-candidate path drops trace linkage or leaks evaluator env, rather than only catching 'executor was called'. Fix: assert one or two request fields (trace tags + launch.env keys) already validated by the lower-level suite.
🟡 LOW No coverage for the 'request-changes' review decision branch — tests/improvement-cycle.test.ts
AgentImprovementReviewDecision = 'approve' | 'reject' | 'request-changes' (improvement-cycle.ts:79). Tests cover approve (line 162), reject (line 234), and unauthorized-approve (line 280), but never exercise 'request-changes'. Per improvement-cycle.ts:233, executeApprovedAgentCandidate treats any non-approve decision identically (throws 'not an approval'), so this is not a correctness hole — but
🟡 LOW No test coverage for 'request-changes' review decision — tests/improvement-cycle.test.ts
The
AgentImprovementReviewDecisiontype includes'request-changes'but no test exercises this path throughreviewAgentImprovementProposal. Both'approve'and'reject'are tested, andexecuteApprovedAgentCandidatecorrectly rejects'reject', but'request-changes'is never validated to produce a structurally valid review without the approval guardrails (gateDecision + candidateBundle checks at improvement-cycle.ts:206-211).
🟡 LOW No test for improvement.shipped=false path (proposer returns no winner) — tests/improvement-cycle.test.ts
All test proposers return a winner with a score that passes the gate (
gateDecision === 'ship'). Theimprovement.shipped = falsepath at improvement-cycle.ts:172-173 results incandidateBundle = undefined, which then causesreviewAgentImprovementProposalto reject approval with'candidate cannot be approved until an executable bundle is sealed'. This path is untested — a regression in theimprovement.shippedlogic or thegateDecisionfiltering could go undetected.
🟡 LOW Optional chaining on candidateBundle?.digest weakens evidence assertion — tests/improvement-cycle.test.ts
Line 214:
bundleDigest: proposed.proposal.candidateBundle?.digest— ifcandidateBundlewere somehow undefined (impossible in this test flow sinceexecuteApprovedAgentCandidatewould throw first),toMatchObjectwould silently matchbundleDigest: undefinedagainst the evidence. The optional chaining provides no benefit here and could mask a regression in a code path that changes howcandidateBundleis populated. Replace withproposed.proposal.candidateBundle.digestsince line 152 already asserts it exists.
🟡 LOW afterEach calls vi.restoreAllMocks() but no test installs a vi.spyOn — tests/improvement-cycle.test.ts
Lines 129-132:
afterEach(() => { cleanupCandidateFixtures(); vi.restoreAllMocks() }). None of the four tests call vi.spyOn, vi.fn with mocked modules, or vi.doMock — the executors and registries are hand-rolled objects.vi.restoreAllMocks()is therefore dead. Harmless, but it implies a mock discipline that isn't actually in play and may mislead future contributors into thinking mocks are being cleaned up. Either remove the call or add a comment noting it's a guard for future spies.
🟡 LOW alignedBundle helper only propagates systemPrompt, silently dropping other profile changes — tests/improvement-cycle.test.ts
The helper at line 121 accepts
profile: { prompt?: { systemPrompt?: string } }and only mapssystemPromptto the bundle. If the improvement modified model, tools, MCP, hooks, or other profile fields (which is possible when surface !== 'prompt'), those changes would be silently dropped in the sealed bundle. TheassertCandidateProfileBindingcall inproposeAgentImprovement(improvement-cycle.ts:175) would then fail for those surfaces, but the test would misleadingly pass forsurface: 'prompt'alone. This helper should at minimum carry all profile fields or be explicitly scoped to the prompt surface.
tangletools · 2026-07-11T09:19:18Z · trace
tangletools
left a comment
There was a problem hiding this comment.
❌ 3 Blocking Findings — 40d53334
Full multi-shot audit completed 8/8 planned shots over 39 changed files. Global verifier still owns final merge decision. | Full multi-shot audit completed 8/8 planned shots over 39 changed files. Global verifier still owns final merge decision. | Full multi-shot audit completed 8/8 planned shots over 39 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-11T09:19:18Z · immutable trace
tangletools
left a comment
There was a problem hiding this comment.
✅ Auto-approved drewstone PR — 2e8c889d
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-11T15:33:33Z
Problem
Intelligence could analyze and generate candidates, but the public runtime did not expose one durable proposal -> review -> approved execution lifecycle.
Solution
proposeAgentImprovement,reviewAgentImprovementProposal, andexecuteApprovedAgentCandidate.agent-eval0.113,agent-interface0.25, andagent-profile-materialize0.3.1.Proof
pnpm typecheckpnpm lintpnpm test: 148 files, 1478 passed, 2 skippedpnpm buildpnpm verify:packagepnpm docs:freshnessRelease package version:
0.92.1.