feat(candidate): execute exact approved candidates by drewstone · Pull Request #547 · tangle-network/agent-runtime

drewstone · 2026-07-14T05:19:15Z

Problem

Products could analyze and approve an agent change, but Runtime could not safely execute every supported candidate from the exact approved bytes. The package chain also retained schema-version fields and had fallen behind the published Interface, Eval, Knowledge, and Profile Materialize contracts.

Solution

install one published package set: Interface 0.28.0, Eval 0.121.0, Knowledge 3.0.0, and Profile Materialize 0.4.0
remove candidate schema-version fields from this greenfield contract
execute prompt, skill, tool, MCP, model, code, memory, and file-backed knowledge candidates from exact verified inputs
preserve exact profile resources and knowledge files for the executor
support both repository-producing coding tasks and ordinary agent-output tasks
capture the executor stop acknowledgement and task result in one durable receipt
keep paid-model usage bound to the protected router ledger
remove duplicate workspace capture code and all local package links

Verification

pnpm test: 1,600 passed, 2 skipped across 155 files
pnpm typecheck: passed for library and examples
pnpm lint: 454 files passed
pnpm build: all 16 JavaScript and declaration entry points built
pnpm docs:check: generated and curated docs have zero drift
pnpm verify:package: passed
pnpm list --depth 0 --json: all Tangle dependencies resolve from npm
rg -n "(link:|file:|/tmp/)" package.json pnpm-lock.yaml: no local dependency references
git merge-tree --write-tree origin/main HEAD: conflict-free

…ate-outcome

tangletools

✅ Auto-approved drewstone PR — `9a2553dd`

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-14T05:19:31Z}

tangletools

🟡 Value Audit — sound-with-nits


Verdict	sound-with-nits
Concerns	2 (1 low, 1 weak-concern)
Heuristic	0.1s
Duplication	0.0s
Interrogation	239.9s (2 bridge agents)
Total	240.0s

💰 Value — sound

A large but cohesive substrate migration (agent-interface 0.26 / agent-eval 0.118 / agent-knowledge 2.0) that tightens the candidate execution contract (stop/capture split, workspace-vs-output outcomes), consolidates three duplicated helpers into one each, and makes improvement evidence portable — a

What it does: Migrates every candidate document to the current V2 schema from the public registry; splits the executor's single stopAndCapture into separate stop (prove death) + capture (read evidence) so a fresh worker can replay capture after crash; discriminates task outcomes into 'workspace' (git patch) vs 'output' (byte blob) with type-safe verification against the signed task spec; binds profile files to
Goals it achieves: (1) Eliminate schema drift — runtime consumed pre-0.26 candidate types with V1 suffixes and local tarball deps; now on the public registry. (2) Make evidence capture replayable — stop/capture split means crash recovery doesn't depend on in-memory state. (3) Make improvement proposals portable — the proposal now carries a serializable measured comparison instead of embedding agent-eval's SelfImprov
Assessment: Coherent and well-structured. The schema bump is the dominant cost — once agent-interface moves to 0.26, every type name (V1 suffix removal), schemaVersion (1→2/3), and document shape must move in lockstep, which explains the 66-file footprint. The stop/capture split is a strict improvement: it forces the executor to prove process death BEFORE reading evidence, a stronger invariant than the old si
Better / existing approach: none — this is the right approach. Checked for existing equivalents: (1) the new SandboxAgentCandidateExecutor is the only AgentCandidateExecutorPort implementation in src/ (rg confirms no prior sandbox executor existed there; the only other implementor is the Pier trial executor in bench/); (2) the paired-bootstrap recomputation in createAgentImprovementMeasuredComparison uses agent-eval's own pa
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

Migrates candidate execution to schema v2 and adds a sandbox executor for output-only tasks that cleanly complements the existing Pier workspace executor, with profile activation reified as a digest-bound object and cleanup logic shared across three callers.

Integration: Fully wired end-to-end. The new stop/capture split on AgentCandidateExecutorPort (types.ts:500-521) is consumed in sequence by the runtime at execute.ts:617 (stop) and execute.ts:649 (capture), with capture's result sealed against the task outcome spec at execute.ts:662. The new createSandboxApprovedCandidateExecutor is exported from /intelligence (index.ts:136) and verified compilable+importable
Fit with existing patterns: Follows the established candidate-execution grain precisely. The sandbox executor (sandbox-approved-candidate.ts:137) implements the same AgentCandidateExecutorPort as the Pier executor and emits the same canonical evidence bytes; it is the natural output-outcome counterpart to Pier's workspace-outcome path. The two cleanly partition the outcome space — Pier throws unless outcome.kind==='workspace
Real-world viability: Holds up off the happy path. The stop/capture separation makes capture replayable by a fresh worker: the sandbox executor's resolve() re-lists team sandboxes keyed by metadata when in-memory state is gone (sandbox-approved-candidate.ts:294-309), and capture re-derives output/termination from the persisted process when the live handle is missing (lines 234-244). Output collection enforces maxBytes
Model: opencode/zai-coding-plan/glm-5.2
Bridge attempts: 1

🔎 Heuristic Signals

🟡 Cruft: magic number added tests/knowledge-improvement-job.test.ts

   budget: { maxIterations: 2, maxTokens: 1000 },

🎯 Usefulness Audit

🟡 Python contract guard for knowledgeManifestDigest is now vacuous [integration] ``

The TS execution-plan material no longer emits knowledgeManifestDigest (prepare.ts drops the field and the knowledgeManifestDigest spread at lines 280,334 of the old code). bench/pier_agents/candidate_contract.py:657 still guards if plan.get("knowledgeManifestDigest") is not None, which can now never fire because the field is absent. Harmless (the contract still passes) but the guard is dead logic. Drop it or repurpose it to assert the field is absent so the contract documents the new shape.

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 · 20260714T052525Z}

tangletools · 2026-07-14T06:40:35Z

❌ Needs Work — `9a2553dd`

Review health 100/100 · Reviewer score 0/100 · Confidence 95/100 · 26 findings (1 high, 5 medium, 20 low)

glm: Correctness 0 · Security 0 · Testing 0 · Architecture 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 75 changed files. Global verifier still owns final merge decision.

Blocking

🔴 HIGH No tests for 516-line security-critical sandbox executor — src/intelligence/sandbox-approved-candidate.ts

sandbox-approved-candidate.ts implements sandbox lifecycle (create/execute/stop/capture/delete), output capture with byte limits, evidence materialization via canonicalCandidateBytes, path traversal protection (beneath()), egress policy enforcement, process kill/kill-tree semantics, and AbortSignal-driven cancellation. It has zero unit or integration tests — no sandbox*.test.ts file exists. No caller in src/ uses createSandboxApprovedCandidateExecutor or sandboxApprovedCandidateExecutionSupport (confirmed via grep). This is new untested code that will run untrusted candidate processes in fresh sandboxes. Impact: any of the identified edge cases (race conditions, cleanup failures, output hangs) would ship undetected. Fix: add tests covering execute happy path, abort/timeout cancellation, ou

Other

🟠 MEDIUM Format/schema version bumps break all in-flight persisted records with no migration — src/candidate-execution/claim-file-formats.ts

CLAIM_FORMAT_VERSION 7->8, PENDING_FORMAT_VERSION 1->2, TERMINAL_FORMAT_VERSION 3->4 (claim-file-formats.ts:12-14), plus bundle schemaVersion 1->2 (builder.ts:85), workspace-manifest 1->2 (artifacts.ts:113), workspace-snapshot 1->2 (outcome-evidence.ts:227/257/504, finalize.ts:269, workspace-archive.ts:180), execution-plan 1->2 (prepare.ts:247/302), materialization-receipt 1->2 (prepare.ts:315), task-outcome 1->2 (outcome-evidence.ts:265/339/382), model-settlement-evidence 1->2 (outcome-evidence.ts:119), run-receipt 2->3 (finalize.ts:159). readClaim (claim.ts:651) rejects any record whose version !== CLAIM_FORMAT_VERSION, so any claim written by the previous version fails to load with 'unsupported version'. No migration path exists. Acceptable for a pre-1.0 package if intentional, but oper

🟠 MEDIUM executor.capture receives a never-aborted signal, violating its documented contract — src/candidate-execution/execute.ts

Both execute.ts:656 and recover.ts:170 call executor.capture(req, { traceStore, signal: new AbortController().signal }). The fresh controller is never stored, never aborted. The AgentCandidateExecutorPort.capture contract in types.ts:513 documents the signal as 'Aborted at the frozen execution deadline or evaluator cleanup deadline.' The deadline is enforced only by withinCandidateCleanupDeadline's Promise.race timeout (cleanup.ts:38), which does NOT propagate cancellation to the in-flight capture operation; the underlying promise keeps running. Impact: a hanging capture leaks the operation (and any executor-held resources) past the cleanup deadline silently. Fix: create the controller once, pass controller.signal, and abort it either from the withinCandidateCleanupDeadline timer or conv

🟠 MEDIUM Sandbox cloud resource and in-memory state leak on execute failure paths — src/intelligence/sandbox-approved-candidate.ts

The factory's execute wrapper (lines 101-134) calls executor.delete() only on the success path (after line 127). When execute throws — output exceeds maxBytes (line 449), inconsistent terminal status ([line 188](https://github.com/tangle-network/agent-runtime/blob/9a2553ddc9a69799e41ac6d6ab7a37fa85b75951/src/intelligence/sandbox-approved-candidat

🟠 MEDIUM Termination misattribution race between abort and process.wait() resolution — src/intelligence/sandbox-approved-candidate.ts

After process.wait() resolves (line 184), the abort listener (cancel()) remains registered until the finally block (line 205). If the signal aborts between lines 184 and 191 (state.termination assignment), cancel() sets cancellationTermination to { kind: 'timeout' } or { kind: 'cancelled' } even though the process already exited normally. [Line 191](h

🟠 MEDIUM stop and capture ignore the runtime context (signal, deadlineAtMs, traceStore) — src/intelligence/sandbox-approved-candidate.ts

AgentCandidateExecutorPort declares stop(request, context: { traceStore, signal, reason, deadlineAtMs }) and capture(request, context: { traceStore, signal }). The sandbox executor's stop (line 210) and capture (line 228) omit the context parameter entirely — they are declared as stop(request) and capture(request). TypeScript allows this (fewer params is assignable), but the contract is violated: (1) stop ignores deadlineAtMs — the contract says 'a later stop acknowledgement cannot produce success

🟡 LOW primitive-catalog lists AgentCandidateProfileActivation interface but no per-symbol anchor exists in intelligence.md — docs/api/primitive-catalog.md

The intelligence table (line ~293) adds AgentCandidateProfileActivation | interface | Exact native profile files and the canonical plan that activated them. and the undocumented-types list shrinks correspondingly, but intelligence.md only generates an anchor for the parse function parseAgentCandidateProfileActivation (line 3304), not for the interface itself. Confirmed by grep: no ^### AgentCandidateProfileActivation heading exists in any docs/api/*.md file. The type is imported from @tangle-network/agent-interface (src/candidate-execution/profile.ts:4), so TypeDoc does not promote it to its own page. Impact: cosmetic — the catalog row points at a sym

🟡 LOW CheckBox.fs.write widened to Promise loses write-result precision — examples/coding-benchmark/eval.ts

Return type went Promise -> Promise to let SandboxInstance.fs (which returns Promise) satisfy CheckBox without a cast. Intentional and safe — seedFile (eval.ts:107) awaits and discards the value, so no caller depends on the result. unknown is the pragmatic widening; if a future caller needed the FileWriteResult it would have to narrow. Not actionable, noted for completeness.

🟡 LOW Loosened workspace file modes (0o0-0o777) rely entirely on schema validation with no upper-bound sanity check — src/candidate-execution/artifacts.ts

scanWorkspace used to reject any mode that wasn't 0o644 or 0o755 (explicit allow-list). It now accepts any integer 0..0o777 via isWorkspaceFileMode (workspace-archive.ts:846) and stores the raw stats.mode & 0o777. This is consistent across artifacts.ts, workspace-archive.ts, and profile.ts and is required to carry setuid/setgid/sticky-style harness modes through the manifest. Confirm downstream executors actually honor arbitrary modes when materializing (writeWorkspaceFiles at workspace-archive.ts:580 just chmods the recorded value); a malicious or buggy materializer could now persist suid binaries into the task workspace. Noting as low because the candidate bundle is already trusted/verified at this point, but the widened surface is worth a deliberate ack.

🟡 LOW buildAgentCandidateBundle still accepts knowledge input that verifyAgentCandidateBundle now rejects — src/candidate-execution/builder.ts

builder.ts:63 still declares knowledge?: AgentCandidateKnowledge on BuildAgentCandidateBundleInput and embeds it into the bundle at line 91, but verify.ts:67 now throws 'candidate knowledge execution is unsupported' whenever parsed.knowledge is present. The result is a bundle that builds successfully and fails late at the verify boundary with a message pointing at the runtime rather than the builder. Either drop the field from BuildAgentCandidateBundleInput (and the bundle shape) or reject it in the builder so callers fail at construction time.

🟡 LOW createAgentCandidateProfileActivation error message misreports mode failures as missing files — src/candidate-execution/profile.ts

The guard if (!source || !Number.isSafeInteger(mode) || mode < 0 || mode > 0o777) throws 'candidate profile activation does not contain every planned native file' for all three failure modes. When the real cause is an out-of-range Unix mode (e.g., 0o1000), the message incorrectly claims the file is missing, sending operators looking in the wrong place. Split the conditions or include the actual relPath/mode in the error text.

🟡 LOW Recovery preparation read runs in parallel with executor.stop and gates capture silently — src/candidate-execution/recover.ts

preparation (readRecoveryPreparation) and processClosure (executor.stop) are dispatched concurrently via Promise.allSettled at line 147. If the artifact read rejects (e.g., transient output-artifacts outage), recoveredPreparation is undefined and the capture/persist block at lines 158-195 is skipped entirely; the function then throws the generic 'expired candidate capture was not persisted' at [line 203](https://github.com/tangle-network/agent-runtime/blob/9a2553ddc9a69799e41ac6d6ab7a37fa85b75951/src/candidate-execu

🟡 LOW Missing-ledger check moved from upfront to per-shot, slightly delaying the failure — src/improvement/agentic-generator.ts

OLD code checked if (opts.codexReproducible && !costLedger) at the top of generate() before any prompt construction. NEW code performs the check inside the shot loop after basePrompt is built, appendProfileResourcePaths runs, and harnessInvocation() is called. For shot 0 this is still before any harness subprocess spawns (runHarness is not called — verified against tests/agentic-generator.test.ts:357 which asserts runHarness not called), so it is NOT a billing/cost issue. But it does mean generate() now does non-trivial string work (defaultBuildPrompt iterates findings, appendProfileResourcePaths maps files) before failing on a config error that was previously a fast-fail. The wasted work is bounded and idempotent, so impact is negligible; calling it out only because the old placement wa

🟡 LOW rethrowAfterCleanup has no direct unit test despite becoming shared infrastructure — src/improvement/cleanup.ts

The helper is now imported by improve.ts, improvement-driver.ts, and intelligence/improvement-cycle.ts (3 call sites). It is only exercised transitively through integration tests that happen to trigger cleanup paths. The three branches (first-attempt success throws cause; retry-succeeded throws AggregateError with '; cleanup retry succeeded'; both-fail throws AggregateError with '; cleanup failed') deserve a focused unit test — especially because the retry-succeeded branch is the one that distinguishes this helper from a plain try/finally and a regression in the loop counter or the continue placement would silently change error semantics. Fix: add tests/improvement/cleanup.test.ts covering all three branches with mock cleanup functions.

🟡 LOW Error message wording lost caller-specific context — src/improvement/improve.ts

OLD: 'improve(): code preparation failed and its baseline worktree could not be cleaned'. NEW: 'improve(): code preparation failed' + rethrowAfterCleanup appends '; cleanup failed' or '; cleanup retry succeeded'. Similarly for the code-improvement-failed path. The new wording is more generic — an operator grepping logs for 'baseline worktree' or 'worktrees could not be cleaned' (the old phrases) will miss these errors. The tradeoff is intentional (shared helper produces uniform suffixes) and the AggregateError.errors array still carries the underlying discard errors for diagnosis. Acceptable; noting only for log-pattern awareness.

🟡 LOW assertMeasuredNumber tolerance tightly couples to agent-eval's exact computation method — src/intelligence/improvement-cycle.ts

assertMeasuredNumber (line 1038) uses an 8-ULP relative tolerance to compare result.lift vs recomputed composite.delta, result.baseline.compositeMean vs composite.baseline, etc. These cross-checks assume both the agent-eval result path and the local recomputation produce bit-identical floating point. Any future change in agent-eval's computation (different aggregation order, different statistic variant, library upgrade) would cause false rejection of ALL proposals — the entire proposal pipeline would throw at createAgentImprovementMeasuredComparison. Impact: latent coupling that could silently block all agent improvement proposals after an agent-eval

🟡 LOW outputPromise await can hang if stdout stays open after process exit — src/intelligence/sandbox-approved-candidate.ts

After process.wait() resolves (process exited), line 190 does state.output = await outputPromise, which awaits collectOutput's for-await loop over process.stdout(). If a grandchild process inherited the stdout pipe and keeps it open, the stdout async iterable never completes and this await hangs with no timeout. The sandbox's spawnExact timeoutMs and killProcess(tree:true) mitigate but do not guarantee stdout closure in all cases (e.g., PID-namespace escapes, zombie fds). Impact: execute hangs indefinitely on a dead process. Fix: race outputPromise against a deadline-bounded timeout, or rely on context.signal (which is checked after but not du

🟡 LOW Redundant verification+isolated-copy call before promoteKnowledgeCandidate — src/knowledge/improvement-job.ts

In the approval branch, await withKnowledgeImprovementCandidate({ root: options.root, candidate }, () => undefined) runs the full withMeasuredCandidateSnapshot verification AND withIsolatedCandidateCopy (mkdtemp + copyKnowledgeWorkspace + hash check) only to discard the result. promoteKnowledgeCandidate at line 203 then re-opens the same run, re-loads state, re-finds the candidate, and re-verifies via withMeasuredCandidateSnapshot. Defensible as defense-in-depth (snapshot integrity is checked twice), but it does an isolated temp-dir copy that is provably unused. Either drop the call (relying on promoteKnowledgeCandidate's own verification) or use a lighte

🟡 LOW Unused 'knowledge-retrieval-config' purpose in captureKnowledgeEvidence type union — src/knowledge/improvement-job.ts

The purpose parameter is typed 'knowledge-retrieval-config' | 'knowledge-evaluation' but only 'knowledge-evaluation' is ever passed (line 283). The retrieval-config branch is dead in this file. Either drop it from the union or wire the retrieval-config evidence capture (AgentCandidateKnowledge.retrievalConfig is declared optional on the interface and is never populated by freezeKnowledgeCandidate, so the frozen candidate currently omits it). Minor; document intent or fill the gap.

🟡 LOW interfaceKnowledgeCandidateRef computed twice with identical inputs — src/knowledge/improvement-job.ts

interfaceKnowledgeCandidateRef(candidate) is invoked once at line 280 (to build the evaluation bytes) and again at line 289 (in the agentCandidateKnowledgeSchema.parse object). Both produce the same frozen object. Hoist to a local const ref = interfaceKnowledgeCandidateRef(candidate) and reuse. Pure cleanup; no behavioral impact.

🟡 LOW Tightened metadata type is a theoretical downstream break — src/knowledge/supervised-update.ts

metadata changed from Record<string, unknown> to NonNullable<RagKnowledgeUpdateResult['metadata']> = Record<string, JsonValue>. This aligns the runtime contract with the underlying agent-knowledge type and is correct, but external consumers that constructed metadata with non-JSON values (functions, undefined, class instances) will now fail TypeScript compile. The change is desirable; flag only because it is a contract narrowing on a public exported type (re-exported via src/knowledge/index.ts).

🟡 LOW Capability surface assertion uses toMatchObject allowing silent expansion — tests/sandbox-approved-candidate.test.ts

expect(sandboxApprovedCandidateExecutionSupport).toMatchObject({outcomes:['output'], code:['disabled'], memory:['disabled']}) uses toMatchObject which for arrays checks element-by-element but does NOT verify array length. If someone adds 'workspace' to outcomes (making it ['output','workspace']), this assertion still passes. For a frozen capability surface where scope containment is a security property, toEqual would be stricter. Same concern for the profile sub-object.

🟡 LOW No assertion on sandbox create metadata used by recovery resolution — tests/sandbox-approved-candidate.test.ts

The create assertion at line 106-119 uses expect.objectContaining checking only image, publicEdge, ephemeral, bare, egressPolicy. The source (sandbox-approved-candidate.ts:365-372) sets metadata with kind, executionId, executionPlanDigest, outputMediaType, outputMaxBytes — these are consumed by matchesExecution() and sandboxOutputSpec() in the recovery/capture path. The test's sandbox mock has metadata:undefined, which works only because the happy-path capture() uses cached state.output. If the cache miss path (capture [lines 238-243](https://github.com/tangle-network/agent-runtime/blob/9a2553ddc9a69799e41ac6d6ab7a37fa85b75951/tests/sandbox-app

🟡 LOW No test for output-overflow kill path in collectOutput — tests/sandbox-approved-candidate.test.ts

The happy-path test uses a 17-byte stdout yield against a 1024-byte maxBytes bound. collectOutput (src/intelligence/sandbox-approved-candidate.ts:441-454) has a critical kill-and-throw path when byteLength > maxBytes that is never exercised. This path calls killProcess and throws — a security-relevant enforcement that output limits are mechanically enforced. Add a test variant where the stdout mock yields bytes exceeding maxBytes and assert the executor rejects the run.

🟡 LOW No tests for signal, timeout, or cancellation termination paths — tests/sandbox-approved-candidate.test.ts

The test only covers exitCode=0 with running=false. Untested: non-zero exit codes (still succeeds but status check at line 187 must pass), signal termination (exitSignal set, processTermination at line 466-475 returns {kind:'signal'}), deadline abort (context.signal.aborted triggers cancel handler at lines 174-182), and the 'fresh candidate sandbox already co

🟡 LOW stop() test only covers the already-deleted case, not process kill — tests/sandbox-approved-candidate.test.ts

The explicit adapter.executor.stop() assertion at line 163-176 runs AFTER adapter.execute() has already called executor.delete() (line 128-131 of sandbox-approved-candidate.ts). At that point the state is purged from this.states and client.list returns [], so resolve() returns undefined and stop() short-circuits to {stopped:true} without testing the actual kill logic ([lines 210-226](https://github.com/tangle-network/agent-runtime/blob/9a2553ddc9a69799e41ac6d6ab7a37fa85b75951/tests/sandbox-approved-

_{tangletools · 2026-07-14T06:40:32Z · trace}

tangletools

❌ 1 Blocking Finding — `9a2553dd`

Full multi-shot audit completed 8/8 planned shots over 75 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-14T06:40:32Z · immutable trace}

…ate-outcome # Conflicts: # docs/api/primitive-catalog.md # docs/canonical-api.md # package.json # src/candidate-execution/finalize.ts # src/candidate-execution/outcome-evidence.ts # tests/candidate-execution-execute.test.ts

tangletools

✅ Auto-approved drewstone PR — `caa1c791`

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-14T15:05:08Z}

tangletools

🟡 Value Audit — sound-with-nits


Verdict	sound-with-nits
Concerns	2 (1 low, 1 weak-concern)
Heuristic	0.1s
Duplication	0.0s
Interrogation	426.1s (2 bridge agents)
Total	426.2s

💰 Value — sound-with-nits

Migrates candidate execution to the agent-interface 0.26 contract (stop/capture split, workspace|output outcome union, exact approval-evidence chain) with genuine de-duplication; one mild smell is the workspace-snapshot builder got inlined into 4 sites after its shared helper was deleted.

What it does: Six concrete deltas, each grounded in code: (1) Splits the executor port's stopAndCapture into stop (prove process death) + capture (replayable post-stop evidence), so a fresh recovery worker can re-capture by reading persisted preparation evidence (src/candidate-execution/types.ts:497, execute.ts:610-665, recover.ts:69-130). (2) Turns the task outcome into a workspace | output discriminat
Goals it achieves: Adopt the current agent-interface/eval/knowledge/sandbox contract versions the runtime had fallen behind on; make a candidate run provably bound to the exact approved bytes (the 'exact improvement contracts' title) end-to-end including profile re-materialization; let a fresh worker recover an expired execution by replaying capture from persisted evidence rather than in-memory state; support non-wo
Assessment: Coherent and in the grain. The repo already lived by sealed brands, canonical (RFC 8785) digests, content-addressed artifact ports, and claim stores; this PR extends that same machinery rather than inventing a parallel one. The stop/capture split is the right shape for replayable recovery, the outcome union is backed by a genuine new consumer (not speculative), and every schemaVersion bump is pair
Better / existing approach: Mostly none — this is the right approach. One small available improvement: deleting src/candidate-execution/workspace-snapshot.ts removed two shared helpers (provisionalCandidateWorkspaceSnapshot, persistCandidateWorkspaceSnapshot) and the agentCandidateWorkspaceSnapshotEvidenceSchema.parse({schemaVersion:2,...}) construction is now inlined at 4 sites (finalize.ts:268, outcome-evidence.ts:22
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

Coherent schema migration plus a new output-kind sandbox executor and restart-safe recovery claim, consolidating duplicated manifest/cleanup logic along the way; nothing materially better or pre-existing-equivalent found.

Integration: Fully wired. executeApprovedAgentCandidate (src/intelligence/improvement-cycle.ts:286) is the spine; createSandboxApprovedCandidateExecutor wraps it (src/intelligence/sandbox-approved-candidate.ts:102) and is exported from /intelligence (src/intelligence/index.ts:136). The Pier executor in bench/ is migrated to split stop/capture with the new captureResult restart-safe reader (bench/src/pier-agent
Fit with existing patterns: Matches the codebase grain precisely. The split stop→acknowledge→capture follows the same branded-provenance pattern used throughout candidate-execution (SealedAgentCandidateExecutorFinalCapture, sealAgentCandidateExecutorStopAcknowledgement in src/candidate-execution/executor-capture.ts:9-65). preparationEvidence on the claim extends the existing claim-carrying-evidence pattern. candidateWorkspac
Real-world viability: Holds up off the happy path. Recovery is now restart-safe: readRecoveryPreparation (src/candidate-execution/recover.ts) re-reads canonical bytes from the persisted artifacts and cross-checks every digest against the claim before any capture attempt. The Pier controller's readResult reads immutable persisted bytes from disk, so a dead container does not lose evidence. Error paths aggregate correctl
Model: opencode/zai-coding-plan/glm-5.2
Bridge attempts: 1

🔎 Heuristic Signals

🟡 Cruft: magic number added tests/knowledge-improvement-job.test.ts

   budget: { maxIterations: 2, maxTokens: 1000 },

💰 Value Audit

🟡 Workspace-snapshot evidence builder inlined into 4 sites after helper deletion [duplication] ``

workspace-snapshot.ts was deleted (good — its persist path was tangled), but the agentCandidateWorkspaceSnapshotEvidenceSchema.parse({schemaVersion:2, kind:'agent-candidate-workspace-snapshot', digest, material, manifest, archive}) shape is now hand-constructed in finalize.ts:268 (memoryReceipt) and outcome-evidence.ts:227, :257, :504 (provisional + final task + memory verify). The manifest builder (candidateWorkspaceManifest, artifacts.ts:116) is correctly shared; only the evidence wrapper

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 · 20260714T151411Z}

tangletools · 2026-07-14T16:02:34Z

✅ No Blockers — `caa1c791`

Review health 100/100 · Reviewer score 17/100 · Confidence 95/100 · 19 findings (3 medium, 16 low)

glm: Correctness 17 · Security 17 · Testing 17 · Architecture 17

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 77 changed files. Global verifier still owns final merge decision.

🟠 MEDIUM No tests for new functions in shot's test file — stub uses type-cast empty objects — src/intelligence/with-intelligence.test.ts

The only test file in this shot updates the candidateExecution stub to the new shape but uses {} as CandidateExecutionEvidence['materializationReceipt'] and {} as CandidateExecutionEvidence['profileActivation'], plus fabricated digest strings. This exercises the telemetry attribute mapping (index.ts lines 600-612) but does NOT validate schema parsing, digest verification, or any of the new functions (createAgentImprovementMeasuredComparison, verifyCandidateExecutionEvidence, deriveChangedSurfaces, assertMeasuredCodeBundle, measuredObjective). Real tests exist in tests/improvement-cycle.test.ts and tests/sandbox-approved-candidate.test.ts

🟠 MEDIUM No test exercises the executor.capture() recovery path — tests/sandbox-approved-candidate.test.ts

The source capture() method (sandbox-approved-candidate.ts:243-290) contains significant logic: re-reading stdout from a stopped process when state.output is missing, computing canonical evidence bytes, and enforcing 'one stopped process before capture'. No test directly exercises capture with a missing-state scenario (where output must be re-collected from the process). The happy-path test only covers capture indirectly through executeApprovedAgentCandidate. A regression in the recovery path (e.g., expired-claim recovery per the source comment at line 89) would go undetected.

🟠 MEDIUM No test for authorizeReview rejection path — tests/sandbox-approved-candidate.test.ts

Both adapter helpers (lines 101, 368) set authorizeReview to always accept: async (candidateReview) => candidateReview.digest === review.digest. There is no test that the executor surfaces 'candidate approval was not authorized by the configured authority' (improvement-cycle.ts:300) when authorizeReview returns false. This is the primary access-control gate for approved candidate execution — an untested rejection path is a security coverage gap. Fix: add a test where authorizeReview returns false and assert the executor rejects with /not authorized/.

🟡 LOW CheckBox.fs.write return widened to Promise loses result visibility — examples/coding-benchmark/eval.ts

The widening from Promise to Promise is correct and necessary to drop the cast (real SandboxInstance.fs.write returns Promise). But Promise is loose: a future caller that wanted to read FileWriteResult (e.g. the written file's hash/size) would get unknown and have to re-narrow. Not a defect today — seedFile is the only caller and discards the result. Optional nit: type as Promise<FileWriteResult | void> or keep a branded alias if the result ever matters. No action required to merge.

🟡 LOW Executor capture discarded for live-failed executions — src/candidate-execution/execute.ts

When execution fails or the finalCapture has no taskOutcome (lines 380-386) or when task outcome verification throws (lines 445-452), the SealedAgentCandidateExecutorFinalCapture available in the ExecutorOutcome is silently discarded. Only persistFailureEvidence (a text-based description) is persisted. In contrast, the recovery path in recover.ts always persists the executor capture via persistAgentCandidateExecutorCapture and binds it as failureEvidence. Impact: diagnostic evidence loss for live failures — pos

🟡 LOW Profile activation file content not cross-checked against signed plan content hashes — src/candidate-execution/profile.ts

The old code (exactProfileExecutorFiles in prepare.ts, now deleted) verified sha256Bytes(bytes) !== expected.contentSha256 for each profile file. The new createAgentCandidateProfileActivation maps plan.files[].content to activation files without verifying the content hashes to the signed profilePlan.material.files[].contentSha256. The activation's own digest (verified in parseAgentCandidateProfileActivation via canonicalCandidateDigest(omitTopLevelDigest(activation))) prevents tampering, and the on-disk verification in assertPreparedCandidateWorkspaces catches mismatches at materialization time. But the in-memory activation document itself could carry content that disagrees with the signed plan hashes if createAgentCandidateProfileActivation receives inconsistent inputs. Impact: defense-

🟡 LOW readRecoveryPreparation ignores cleanup deadline AbortSignal — src/candidate-execution/recover.ts

The preparation read is wrapped in withinCandidateCleanupDeadline which now passes an AbortSignal to the operation callback (cleanup.ts refactored to provide signal cancellation). But readRecoveryPreparation is invoked as () => readRecoveryPreparation(...) — the lambda discards the signal parameter. If the artifact store read hangs, it cannot be cancelled at the cleanup deadline; only the outer timeout fires and abandons the promise. Impact: under storage contention, the artifact read could block longer than necessary without cooperative cancellation, though the deadline is still enforced by the timeout. Fix: pass the signal through to readVerifiedArtifact / port.read calls.

🟡 LOW Workspace file modes widened from {0o644,0o755} to 0-0o777 — src/candidate-execution/workspace-archive.ts

isWorkspaceFileMode now accepts any integer 0-0o777 (previously only 0o644 and 0o755 were admitted). scanWorkspace (artifacts.ts) no longer rejects unsupported modes at scan time. This means world-writable (mode & 0o002) and setuid/setgid (mode & 0o6000) files are now valid in workspace manifests and archives. Files are still created with O_CREAT mode 0o600 then chmod'd to the target mode (writeWorkspaceFiles line 686-694), so no privilege escalation at write time. Impact: reduced security posture in isolated candidate environments — a compromised executor could inject world-writable files that survive into materialized workspaces. Not a di

🟡 LOW assertMeasuredNumber tolerance (EPSILON*8) may be too tight for cross-path float agreement — src/intelligence/improvement-cycle.ts

assertMeasuredNumber compares result.lift (computed by agent-eval's reporting pipeline) against composite.delta (recomputed here via measuredMean(candidate) - measuredMean(baseline)) with tolerance = Number.EPSILON * max(1,|actual|,|expected|) * 8 ~= 1.8e-15 relative. If agent-eval computes means via a different accumulation order or library (e.g. running mean vs reduce sum), accumulated float error can exceed this threshold for n>100 paired cells, causing spurious 'does not agree' failures on valid data. Impact: false rejection of legitimate improvement proposals under certain data sizes. Fix: widen to a ULP-based tolerance scaled by n (e.g. EPSILON * max(1,|actual|,|expected|) * max(8, n)) or use a small absolute floor (e.g. 1e-12).

🟡 LOW tangle.candidate.run_receipt_digest now unconditionally emitted (behavioral change) — src/intelligence/index.ts

Previously: ...(record.candidateExecution.runReceiptDigest ? { 'tangle.candidate.run_receipt_digest': ... } : {}) (conditional). Now: 'tangle.candidate.run_receipt_digest': record.candidateExecution.receipt.digest (unconditional when candidateExecution exists). This is semantically equivalent because the new CandidateExecutionEvidence type only exists for successful executions (succeeded:true), and the old runReceiptDigest was also only present on success. However, any downstream telemetry consumer that explicitly checked for attribute ABSENCE to detect failure-path evidence will no longer see that signal — though the evidence model itself changed (failures no longer produce evidence records). Impact: low, but worth documenting in the PR description as a telemetry contract change.

🟡 LOW Failure-branch finally can mask original error if executor.delete throws — src/intelligence/sandbox-approved-candidate.ts

When finalization.succeeded is false, the code does try { throw new Error('...') } finally { await executor.delete({...}) }. If executor.delete throws (e.g. transient API error during sandbox cleanup), the finally's exception supersedes the original 'execution failed' error, masking the real failure reason from the caller. The success branch (line 125) has the same pattern but is less impactful since the original operation succeeded. Impact: debugging difficulty when both execution and cleanup fail. Fix: catch delete errors in the finally and aggregate: catch (e) { throw new AggregateError([cause, e], '...') }.

🟡 LOW Sandbox executor relies on executePreparedAgentCandidate for exception-path cleanup — src/intelligence/sandbox-approved-candidate.ts

The wrapper only calls executor.delete in two branches (succeeded / failed-finalization). If executeApprovedAgentCandidate itself throws an exception (e.g. verifyAgentImprovementProposal throws after the sandbox was already created by a prior execute call within executePreparedAgentCandidate), neither finally runs and the sandbox is not deleted by this wrapper. Cleanup depends entirely on executePreparedAgentCandidate's internal error handling (outside this shot's files). The resolve() fallback via client.list provides crash-recovery, and idempotencyKey + maxLifetimeSeconds bound the leak, but the wrapper should defensively catch and delete on any exception path for robustness. Impact: transient sandbox leak on unexpected exceptions, bounded by maxLifetimeSeconds (default 900s + timeout).

🟡 LOW Abort signal not forwarded to captureAgentCandidateWorkspace when no artifacts port — src/knowledge/improvement-job.ts

When candidateArtifacts is undefined, the signal is only passed inside the artifactPersistence conditional spread (line 266-274). captureAgentCandidateWorkspace receives an empty options object {} with no signal. The workspace capture (file enumeration + archive encoding) proceeds without abort support. Impact: an aborted job still completes the full workspace capture before returning. Fix: pass signal at the top level of the captureAgentCandidateWorkspace options, not only inside artifactPersistence.

🟡 LOW Post-promotion hash check failure leaves promotion committed with no rollback — src/knowledge/improvement-job.ts

If promoteKnowledgeCandidate returns promoted:true but the subsequent hashKnowledgeBase check mismatches (line 217), the live knowledge base is already mutated — throwing an Error does not roll it back. This is a defense-in-depth assertion that should never fire (promotion is content-addressed and lease-protected), but if it does, the caller sees an error while the KB is in the promoted state. Acceptable for a forward-only knowledge promotion model, but worth documenting that this assertion is post-hoc and non-rollbackable.

🟡 LOW Type assertion as KnowledgeImprovementOptions could be avoided — src/knowledge/improvement-job.ts

The cast as KnowledgeImprovementOptions on the spread {...knowledgeOptions, updateKnowledge: instrumentedUpdateKnowledge} is needed because RunKnowledgeImprovementJobOptions extends Omit<KnowledgeImprovementOptions, 'updateKnowledge'> but TypeScript cannot prove the destructured remainder plus the callback reconstructs the full type. Runtime-safe but loses compile-time exhaustiveness checking on the knowledge options shape.

🟡 LOW Lost concrete adapter examples in feedback-store JSDoc — src/mcp/feedback-store.ts

The old comment named two concrete adapter shapes ('a real KbStore-backed sink, an HTTP relay to gtm-agent's knowledge service'); the new comment only says 'consumers wire their own durable adapter'. No code impact, but a future implementer loses the two most likely wiring patterns. Consider restoring one short example if the project values discoverability of integration patterns.

🟡 LOW No test for blocked-egress (network.mode === 'disabled') sandbox creation — tests/sandbox-approved-candidate.test.ts

The happy-path test only verifies the strict-egress policy (mode: 'strict', allowDomains: ['router.tangle.tools']). The source (sandbox-approved-candidate.ts:369-376) has a distinct branch for network.mode === 'disabled' that produces { mode: 'blocked' }. The fixture's model grant uses maxModelCalls=50 (gateway-only), so the disabled path is never hit. A regression in the blocked-egress branch would not be caught.

🟡 LOW No test for multiple-sandbox-match or missing-sandbox-evidence errors in resolve() — tests/sandbox-approved-candidate.test.ts

The resolve() method (source lines 303-333) throws 'multiple sandboxes match one candidate execution' and 'candidate sandbox evidence is unavailable' for ambiguous/missing lookups. The list mocks always return at most one sandbox. These error paths — relevant for recovery from partial failures — are untested.

🟡 LOW Type-safety holes via 'as never' on required ports bypass compile-time guarantees — tests/sandbox-approved-candidate.test.ts

Lines 215-217 use ports: {} as never, grader: {} as never, outputArtifacts: {} as never and line 227 passes {} as never as the executor context. These are safe today only because assertSupportedRequest (source line 158) throws before any port is accessed. If the source is refactored to touch ports or context before the assertion (e.g., adding tracing or metric

_{tangletools · 2026-07-14T16:02:31Z · trace}

Superseded by re-review — no blocking findings on latest commit.

tangletools

✅ Approved — 19 non-blocking findings — `caa1c791`

Full multi-shot audit completed 8/8 planned shots over 77 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-14T16:02:31Z · immutable trace}

…ate-runtime # Conflicts: # docs/api/index.md # docs/api/primitive-catalog.md # docs/canonical-api.md # package.json # pnpm-lock.yaml # scripts/verify-package-exports.mjs # src/improvement/agentic-generator.ts # src/improvement/improvement-driver.ts

…ate-runtime # Conflicts: # docs/api/index.md # docs/api/intelligence.md # docs/api/primitive-catalog.md # docs/canonical-api.md # package.json # pnpm-lock.yaml # src/candidate-execution/artifacts.ts # src/candidate-execution/builder.ts # src/candidate-execution/executor-capture.ts # src/candidate-execution/finalize.ts # src/candidate-execution/outcome-evidence.ts # src/candidate-execution/prepare.ts # src/candidate-execution/prepared-state.ts # src/candidate-execution/types.ts # src/candidate-execution/verify.ts # src/candidate-execution/workspace-archive.ts # src/candidate-execution/workspace-snapshot.ts # tests/candidate-bundle-builder.test.ts # tests/candidate-execution-core.test.ts # tests/candidate-execution-execute.test.ts # tests/candidate-execution-finalize.test.ts # tests/candidate-execution-prepare.test.ts # tests/candidate-execution-task-outcome.test.ts # tests/candidate-execution-workspace-archive.test.ts # tests/helpers/candidate-execution-fixture.ts

tangletools

✅ Auto-approved drewstone PR — `cff5376c`

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-15T17:58:57Z}

tangletools

🟢 Value Audit — sound


Verdict	sound
Concerns	1 (1 low)
Heuristic	0.1s
Duplication	0.0s
Interrogation	571.2s (2 bridge agents)
Total	571.3s

💰 Value — sound

Extends the candidate-execution spine to run every approved candidate kind (prompt/skill/tool/MCP/model/code/memory/file-backed knowledge) from exact verified bytes, adds an 'output' task outcome plus a sandbox executor adapter, splits the executor stop/capture ports, and aligns the package set with

What it does: Concretely: (1) removes schemaVersion/version fields from the greenfield candidate contract, with the Python bench contract now actively rejecting them (bench/pier_agents/candidate_contract.py:110-120); (2) splits the executor port stopAndCapture into stop (sealAgentCandidateExecutorStopAcknowledgement proves process death, src/candidate-execution/executor-capture.ts:63) and capture (sealAgentCand
Goals it achieves: Closes the gap where products could approve an agent change but Runtime could not execute every supported candidate kind from the exact approved bytes; brings this package's contracts back in lockstep with the published Interface/Eval/Knowledge/Profile-Materialize versions it consumes; unifies stop-acknowledgement and result capture into one durable receipt; supports non-coding (output-only) bench
Assessment: Good on its merits. It follows the repo's declared architecture ('domain behavior lives in adapters'): the spine in src/candidate-execution/{execute,recover,finalize}.ts is shared, and both executor implementations (bench Pier and the new sandbox one) are thin adapters over it — grep confirms AgentCandidateExecutorPort is implemented in exactly those two places plus the shared spine. The stop/capt
Better / existing approach: None found. Checked for duplication of the sandbox executor: src/runtime/* sandbox clients (in-process-sandbox-client.ts, inline-sandbox-client.ts) serve the agent turn/run-loop path, not evidence-grade candidate execution, so the new adapter is not a reinvention. Checked whether stop/capture split duplicated recover.ts logic — it does not; recover.ts consumes the new ports. The deleted workspace-
Model: opencode/kimi-for-coding/k2p7
Bridge attempts: 1

🎯 Usefulness — sound

Coherent, in-grain change that splits the executor stop/capture port, adds the bounded-output task outcome with a fresh-sandbox executor, binds exact profile activation, and pulls the improvement/evidence types up into agent-interface — all wired into the existing candidate-execution spine with no d

Integration: Reachable and wired correctly. executePreparedAgentCandidate (src/candidate-execution/execute.ts:77) is consumed by both in-tree executors: the Pier workspace path (bench/src/pier-agent.ts:646) and the new output-only path via executeApprovedAgentCandidate (src/intelligence/improvement-cycle.ts:308) → createSandboxApprovedCandidateExecutor (src/intelligence/sandbox-approved-candidate.ts:102)
Fit with existing patterns: Follows the established pattern precisely. Both new executors implement the existing AgentCandidateExecutorPort; the sandbox one mirrors the Pier one's shape (identity, trials, recovery). Moving AgentImprovementProposal/Review/CandidateExecutionEvidence/AgentCandidateFixedSpend from local definitions in src/intelligence/improvement-cycle.ts into @tangle-network/agent-interface matches
Real-world viability: Holds up past the happy path. The sandbox executor handles timeout (SIGKILL + tree + delete), oversized output (kill + delete), failed creates, abort-signal cancellation throughout, idempotency-keyed sandbox creation (sandbox-approved-candidate.ts idempotencyKey), retention bound to timeout, and process-status consistency re-checks (capture throws if a process is still running). The stop/capture s
Model: opencode/zai-coding-plan/glm-5.2
Bridge attempts: 1

🔎 Heuristic Signals

🟡 Cruft: magic number added tests/knowledge-improvement-job.test.ts

     budget: { maxIterations: 2, maxTokens: 1000 },

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 · 20260715T181016Z}

tangletools

✅ Auto-approved drewstone PR — `e81b7c1c`

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-15T18:15:25Z}

…ate-runtime # Conflicts: # CLAUDE.md

tangletools

✅ Auto-approved drewstone PR — `52985b5a`

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-15T18:26:45Z}

tangletools

🟢 Value Audit — sound


Verdict	sound
Concerns	1 (1 low)
Heuristic	0.2s
Duplication	0.2s
Interrogation	155.6s (2 bridge agents)
Total	156.0s

💰 Value — sound

Closes the propose→approve→execute loop by running every supported candidate surface from exact approved bytes, via the existing executor port and published package contracts — in the grain, no reinvention found; ship.

What it does: Extends candidate execution so an approved improvement bundle is executed from its exact verified bytes across all supported surfaces: adds file-backed knowledge materialization with deterministic signed paths (src/candidate-execution/knowledge.ts:13-35), generalizes task outcomes from workspace-only to workspace|output (src/candidate-execution/types.ts, AgentCandidateExecutorTaskOutcomeCapture un
Goals it achieves: Before this PR the loop stopped at approval: knowledge candidates were explicitly unimplemented (bench/src/pier-agent.ts previously threw 'does not yet implement immutable knowledge mounts'), only repository-producing workspace outcomes existed, and capture was fused to stop so a fresh recovery worker could not replay result capture. After merging: products can analyze, approve, and actually execu
Assessment: Good on its merits and coherent with the codebase's grain. It extends the two canonical seams rather than building parallel paths: executeApprovedAgentCandidate already existed and keeps its verify→prepare→execute→receipt spine (improvement-cycle.ts:284-327), and the new sandbox executor is just another AgentCandidateExecutorPort implementation alongside the bench pier executor (bench/src/pier-age
Better / existing approach: Looked for existing equivalents the change should have reused instead of built: (1) docs/canonical-api.md:116 already names executeApprovedAgentCandidate as THE approved-candidate seam — the PR extends it rather than bypassing it; (2) considered whether the sandbox executor should reuse openSandboxRun (canonical-api.md:83), but that seam is for persistent multi-turn boxes with deliverables, while
Model: opencode/kimi-for-coding/k2p7
Bridge attempts: 1

🎯 Usefulness — sound

Lands a coherent candidate-execution spine (execute/stop/capture port + proposal→review→execute→verify loop) with two real adapters (sandbox for output-kind, Pier for workspace-kind) sharing one defensive core; schema-version cleanup and package updates are necessary greenfield hygiene.

Integration: Fully reachable. The core executePreparedAgentCandidate (src/candidate-execution/execute.ts:77) is consumed by both executeApprovedAgentCandidate (src/intelligence/improvement-cycle.ts:308, the product path) and executePreparedPierCandidate (bench/src/pier-agent.ts:646, the bench path). createSandboxApprovedCandidateExecutor (src/intelligence/sandbox-approved-candidate.ts:100) is exported from the
Fit with existing patterns: Fits the codebase grain precisely. The stopAndCapture→stop+capture split (types.ts:503-535) matches the runtime's existing two-phase model: prove process death, then capture immutable evidence. It lets the recovery path reuse the exact same AgentCandidateExecutorPort without a parallel interface. The sandbox executor restricts itself to output-kind tasks (assertSupportedRequest at sandbox-approved
Real-world viability: Built defensively for realistic conditions. execute.ts enforces durable claims with lease windows (deadlineAtMs recomputation at execute.ts:156-176), protected model grant activate/settle pairs, memory activate/close pairs, fail-closed cleanup deadlines, idempotent terminal publication (finishClaim at execute.ts:880), and redaction of protected env values. runAndStopExecutor (execute.ts:544) handl
Model: opencode/zai-coding-plan/glm-5.2
Bridge attempts: 1

🔎 Heuristic Signals

🟡 Cruft: magic number added tests/knowledge-improvement-job.test.ts

     budget: { maxIterations: 2, maxTokens: 1000 },

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 · 20260715T184356Z}

tangletools

✅ Auto-approved drewstone PR — `f4aa9795`

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-15T19:31:15Z}

tangletools

✅ Auto-approved drewstone PR — `ef4c0b5e`

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-15T19:48:50Z}

tangletools

🟡 Value Audit — sound-with-nits


Verdict	sound-with-nits
Concerns	2 (1 low, 1 weak-concern)
Heuristic	0.1s
Duplication	0.0s
Interrogation	122.3s (2 bridge agents)
Total	122.4s

💰 Value — sound-with-nits

Aligns runtime with the published substrate set (Interface 0.28, Eval 0.121, Knowledge 3.0, Profile Materialize 0.4), deletes the greenfield schemaVersion contract, and closes the approve→execute loop so every candidate kind runs from exact verified bytes with a durable stop+outcome receipt — cohere

What it does: Four concrete deltas. (1) Contract migration: removes every schemaVersion field from the candidate contract across src/candidate-execution/*, the Python pier contract (bench/pier_agents/candidate_contract.py now rejects obsolete version fields, candidate_contract.py:110-118), and all fixtures; grep confirms zero remaining non-test references in src/candidate-execution or src/intelligence. (2) Exec
Goals it achieves: Close the loop the PR title names: products could analyze and approve a change but Runtime couldn't execute every supported candidate kind from the exact approved bytes. After merge: prompt/skill/tool/MCP/model/code/memory/knowledge candidates all execute from verified inputs; both repository-producing and output-only tasks have an executor; the receipt durably binds stop acknowledgement + task re
Assessment: Good on its merits. The change extends the existing candidate-execution module (prepare/execute/recover/finalize/claim) rather than forking it — new files (executor-capture-evidence.ts, sandbox-approved-candidate.ts) plug into the established ports and sealing patterns (canonicalCandidateBytes, seal* helpers). The schemaVersion removal is total and enforced on the consumer side (Python contract re
Better / existing approach: None found. Checked for executor duplication: the only other AgentCandidateExecutorPort implementation is bench's pier-agent (Docker, repository tasks) — a different substrate for a different outcome kind, so the new sandbox executor is complementary, not duplicative. Checked workspace capture: the PR deletes src/candidate-execution/workspace-snapshot.ts and consolidates on workspace-archive.ts ra
Model: opencode/kimi-for-coding/k2p7
Bridge attempts: 1

🎯 Usefulness — sound

A coherent, well-integrated candidate-execution capability that extends the established verify→prepare→execute→finalize spine, reaches real callers in intelligence and bench, and removes obsolete schema-version fields while binding paid-model usage to the protected ledger.

Integration: Fully reachable: executePreparedAgentCandidate is called by executeApprovedAgentCandidate (src/intelligence/improvement-cycle.ts:309, exported at index.ts:116) and by the Pier benchmark consumer (bench/src/pier-agent.ts:646). The new createSandboxApprovedCandidateExecutor composes executeApprovedAgentCandidate onto @tangle-network/sandbox and is itself a public /intelligence export exercised by sc
Fit with existing patterns: Extends the established candidate-execution pattern (verify→prepare→execute→finalize→receipt) without introducing a competing one. Composes onto @tangle-network/sandbox, the codebase's existing isolation primitive. The duplicate workspace-capture code was removed; the canonical captureAgentCandidateWorkspace lives in workspace-archive.ts:124 and is reused by knowledge/improvement-job.ts:265. Schem
Real-world viability: Robust beyond the happy path: execute.ts enforces fail-closed deadline boundaries (rejects completion observed at/after the deadline), handles claim replay/retry rejection, redacts protected values from every failure reason, persists model settlement even when execution fails, and exposes a crashed-attempt recovery path. Outcome verification independently re-derives the result git tree from the si
Model: opencode/zai-coding-plan/glm-5.2
Bridge attempts: 1

🔎 Heuristic Signals

🟡 Cruft: magic number added tests/knowledge-improvement-job.test.ts

     budget: { maxIterations: 2, maxTokens: 1000 },

💰 Value Audit

🟡 Branch carries two unrelated changes alongside the executor work [proportion] ``

Commits 0179509 (docs/agent-managed-compute/*, ~1,000 lines of architecture/roadmap docs, confirmed not in origin/main) and f4aa979/ef4c0b5 (intelligence measured-comparison metadata) are independent of the candidate-execution goal and inflate an already large diff (+8,161/-4,168 across 111 files, though much is generated docs and lockfile). Not a blocker — but landing the docs and measured-comparison work as their own PRs would keep this one reviewable. Core change itself is proportionate.

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 · 20260715T200147Z}

tangletools · 2026-07-15T20:32:51Z

⚠️ Review Interrupted — `ef4c0b5e`

The review runner stopped before publishing a final verdict: webhook_restarted.

State	Detail
Interrupted	webhook restarted

No review verdict was produced for this run. Trigger a fresh review on the current PR head if the PR is still open.

_{tangletools · #547 · model: kimi-for-coding · updated 2026-07-15T20:32:49Z}

tangletools

✅ Auto-approved drewstone PR — `ef4c0b5e`

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-15T20:47:11Z}

tangletools

🟢 Value Audit — sound


Verdict	sound
Concerns	2 (1 low, 1 weak-concern)
Heuristic	0.1s
Duplication	0.0s
Interrogation	171.9s (2 bridge agents)
Total	172.0s

💰 Value — sound

Extends the existing candidate-execution subsystem so approved improvement candidates execute from exact verified bytes (new sandbox executor, dual workspace/output outcomes, approval-gated knowledge promotion), while replacing local contract copies with the published agent-interface 0.28.0 package

What it does: Adopts the published package set (agent-interface 0.28.0, agent-eval 0.121.0, agent-knowledge 3.0.0, agent-profile-materialize 0.4.0) and deletes the locally-defined AgentImprovementProposal/Review/CandidateExecutionEvidence types in src/intelligence/improvement-cycle.ts:77 in favor of re-exports from agent-interface, dropping every schemaVersion field from the candidate contract (0 remaining unde
Goals it achieves: Close the improvement loop end-to-end: propose → approve → execute the exact approved bytes → independently verifiable evidence. Previously executeApprovedAgentCandidate existed on main but returned a locally-computed evidence summary of digests; now the evidence is a canonical document carrying the materialization receipt and profile activation that verifyCandidateExecutionEvidence (improvement-c
Assessment: Good on its merits. The candidate-execution subsystem (prepare/execute/verify/recover/finalize, port-injected executors) already existed on main; this PR extends it rather than inventing a parallel path, and the new sandbox executor plugs into the exact extension point (AgentCandidateExecutorPort) the subsystem was designed around — the same port the bench pier executor and the expired-claim recov
Better / existing approach: None found — this is the right approach. Searched for an existing production executor before judging the new sandbox one: the only AgentCandidateExecutorPort implementations are bench/src/pier-agent.ts (Docker, bench-scoped) and the new sandbox executor, so nothing is duplicated. Checked whether resolveSandboxClient (src/runtime/resolve-sandbox-client.ts) should be reused: the executor correctly t
Model: opencode/kimi-for-coding/k2p7
Bridge attempts: 1

🎯 Usefulness — sound

Completes the approve→execute-exact-bytes loop with a well-layered, fail-closed executor that has real consumers and fits the established candidate-execution pattern; no competing path or dead surface.

Integration: Fully reachable through a clear layered chain: createSandboxApprovedCandidateExecutor (src/intelligence/sandbox-approved-candidate.ts:100) → executeApprovedAgentCandidate (src/intelligence/improvement-cycle.ts:285) → prepareAgentCandidateExecution + executePreparedAgentCandidate (src/candidate-execution/execute.ts:77). The lower execute/prepare layers have a real consumer today in bench/src/pier-a
Fit with existing patterns: Fits the established candidate-execution grain precisely. The prepare→execute→finalize→receipt spine has 10+ prior commits (#507/#511/#514/#521/#538); this PR iterates it (unversion the contract, upgrade deps, add the sandbox-composed entry) rather than forking. No competing or duplicate execute path exists — git history shows executePreparedAgentCandidate originated once in #507 and this extends
Real-world viability: Built for the hard paths. execute.ts manages frozen deadlines with deadline-recovery (lines 156-159 derive the original task deadline from the frozen cleanup window so shortening cleanup never buys execution time), fail-closed on unproven process death (line 320 requires processStopped AND settlement AND memory-close), idempotent claim replay via the claim store (tryClaim retry-not-eligible handli
Model: opencode/zai-coding-plan/glm-5.2
Bridge attempts: 1

🔎 Heuristic Signals

🟡 Cruft: magic number added tests/knowledge-improvement-job.test.ts

     budget: { maxIterations: 2, maxTokens: 1000 },

💰 Value Audit

🟡 Branch carries the unrelated agent-managed-compute docs set (+1,206 lines) [proportion] ``

docs/agent-managed-compute/* (6 files, commit 0179509, PR #558 content) is in this branch but not in origin/main (verified: git merge-base --is-ancestor 0179509 origin/main → false) and is unrelated to candidate execution. Docs-only, so harmless, but it inflates the diff; the two tiny intelligence measured-comparison commits (f4aa979, ef4c0b5, ~14 lines) at least attach to the improvement-cycle story. FYI only — split the docs commit if a clean candidate-only PR is wanted.

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 · 20260715T205026Z}

tangletools · 2026-07-15T21:46:35Z

❌ Needs Work — `ef4c0b5e`

Review health 100/100 · Reviewer score 0/100 · Confidence 95/100 · 39 findings (1 high, 6 medium, 32 low)

	glm	kimi-code	aggregate
Readiness	11	0	0
Confidence	95	95	95
Correctness	11	0	0
Security	11	0	0
Testing	11	0	0
Architecture	11	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 93 changed files. Global verifier still owns final merge decision. | Full multi-shot audit completed 8/8 planned shots over 93 changed files. Global verifier still owns final merge decision.

Blocking

🔴 HIGH docs/api/intelligence.md is stale — CI docs:check gate will fail — docs/api/intelligence.md

Regenerating docs at head (pnpm run docs:api) produces a diff in this file only: every 'Defined in: intelligence/improvement-cycle.ts:N' reference for declarations after line 124 is off by one (e.g. line 1246 says :126, source has the interface at :127; 28 hunks total). Cause: commit f4aa979 added metadata?: AgentImprovementMeasuredComparison['metadata'] at improvement-cycle.ts:124 without re-running docs:api. .github/workflows/ci.yml runs pnpm run docs:check (typedoc + gen-primitive-catalog + git diff --exit-code -- docs/api), s

Other

🟠 MEDIUM Raw executor evidence bytes persisted without protected-value redaction check — src/candidate-execution/executor-capture-evidence.ts

capture.evidence is persisted verbatim as a durable artifact (purpose 'executor-capture') with no assertNoProtectedBytes scan. Base code ran assertNoProtectedBytes(executorCaptureBytes, protectedValues) over the entire persisted executor-capture document in finalize.ts; the refactor kept redaction checks for the patch/archive/manifest/memory bytes (verifyWorkspaceTaskCapture / verifyMemoryCapture in outcome-evidence.ts) but the new executor-native evidence field is never scanned, even though persistVerifiedAgentCandidateExecutorCapture (outcome-evidence.ts:128) has protectedValues in scope. If an executor adapter embeds protected env/grant values in its native evidence, they land in the durable evidence store. Fix: call assertNoProtectedBytes(capture.evidence, protectedValues) in the s

🟠 MEDIUM capture.evidence bytes persisted without assertNoProtectedBytes screening — src/candidate-execution/executor-capture-evidence.ts

The new optional evidence: Uint8Array field on AgentCandidateExecutorFinalCapture is persisted directly as an 'executor-capture' artifact (lines 47-60) without calling assertNoProtectedBytes. Every other executor-provided byte array is screened: patch/archive/manifest in verifyWorkspaceTaskCapture (outcome-evidence.ts:211-224), output bytes in verifyOutputTaskCapture (outcome-evidence.ts:313), memory archive/manifest/files in verifyMemoryCapture (outcome-evidence.ts:493-511). The evidence field is described as 'executor-native bytes preserved when a fresh worker cannot reconstruct a verified outcome' — these bytes may contain candid

🟠 MEDIUM Measured-comparison cross-checks compare differently-aggregated numbers at ~8-ulp tolerance — src/intelligence/improvement-cycle.ts

assertMeasuredNumber(result.lift/result.baseline.compositeMean/result.winner.compositeMean, composite.*) compares agent-eval aggregates against this module's recomputation, but the two sides aggregate differently: agent-eval computes baseline/winner.compositeMean as mean-over-scenarios of per-scenario means (agent-eval dist contract/index.js:126-137,220-221; computeAggregates includes ALL judgeScores) and per-cell composites INCLUDE failed judge rows (chunk-32BZXMSO.js:3260-3268), while measuredComposite here EXCLUDES failed rows (improvement-cycle.ts measuredComposite) and composite.baseline/delta are means over paired cells. pairMeasuredCells only throws for asymmetric errors, so a run with (a) any failed judge row or (b) a symmetric errored rep (unequal successful reps per scenario) mak

🟠 MEDIUM Post-finalization throw strands a 'succeeded' claim and leaks the sandbox — src/intelligence/improvement-cycle.ts

A candidate whose process exits non-zero but still produces a bounded, gradable output reaches finalization.succeeded===true (nothing in executePreparedAgentCandidate/finalizeAgentCandidateRun inspects the exit code) with receipt.termination={kind:'exit',exitCode:N}. verifyCandidateExecutionEvidence then throws at improvement-cycle.ts:402-404 ('candidate execution evidence is not a successful process execution') AFTER the claim store has recorded a terminal 'succeeded' (execute.ts finishClaim). Consequences: (1) the function's documented union result gains an undocumented third outcome — a throw after durable side effects; (2) the caller cannot retry to obtain the evidence (tryClaim now returns 'already claimed' → 'replayed'); (3) createSandboxApprovedCandidateExecutor.execute (sandbox-app

🟠 MEDIUM Symmetric error cells are silently dropped from heldout comparison — src/intelligence/improvement-cycle.ts

pairMeasuredCells filters cell => !cell.error on BOTH arms before pairArms runs. When baseline and candidate both error on the same scenario+rep (symmetric failure), that pair is silently excluded from every objective — the denominator shrinks and confidence intervals tighten without surfacing the failure. The asymmetric case IS caught (pairArms reports unpairedBaseline/unpairedTreatment), but symmetric errors vanish. Impact: a candidate that crashes 30% of heldout scenarios in lockstep with baseline would show identical CIs to a candidate that never crashes, masking a real reliability regression. The assertMeasuredNumber cross-checks against result.lift/compositeMean only validate arithmetic agreement, not cell-set agreement. Fix: count and surface dropped symmetric-error cells as a pow

🟠 MEDIUM power.sufficient formula inverts the usual meaning of scaleAssumed — src/intelligence/improvement-cycle.ts

sufficient: power.scaleAssumed && !power.underpowered requires scaleAssumed to be TRUE for power to be sufficient. The conventional reading of scaleAssumed === true is 'we lacked data to estimate scale, so we assumed it' — i.e., LESS confident, not more. If that reading is correct, this formula marks the HIGHEST-confidence case (scale estimated from data, not underpowered) as insufficient, and the MEDIUM-confidence case (scale assumed, not underpowered) as sufficient — inverted. Could not verify against @tangle-network/agent-eval source (not in worktree). The field is named sufficient with no doc comment explaining the inversion. If the upstream semantic is the opposite of the conventional one, this is correct as-is and just needs a comment; if not, it is a logic bug that would misla

🟡 LOW ResearchOutputShape comment drops dependency-cycle rationale — docs/api/mcp.md

The TSDoc rewrite at src/mcp/types.ts:229 changed from 'the substrate cannot import the ResearchOutput type from agent-knowledge without inducing a dependency cycle, so the MCP layer treats it structurally' to the shorter 'Provider-neutral research output carried over the MCP boundary. The MCP layer accepts this structural shape instead of coupling its wire contract to one research implementation.' New wording is accurate but loses the architectural reason (dependency-cycle avoidance) that justified the structural typing. Consider keeping one clause noting the cycle-avoidance constraint. Pure doc-quality nit; no functional impact.

🟡 LOW AgentCandidateExecutorProfileFile lost its TSDoc summary — docs/api/primitive-catalog.md

The interface dropped from the documented table to the 'Undocumented supporting types' list in both index.md (root: 342→343 exports) and primitive-catalog.md (candidate-execution: 96→97). Root cause is in src/candidate-execution/types.ts:554 — the interface declaration no longer carries a TSDoc summary line, so TypeDoc demoted it. Doc faithfully reflects source; fix is to restore the JSDoc on the source interface, then re-run pnpm run docs:api. No behavior impact.

🟡 LOW Breaking persistence format change with no migration path — src/candidate-execution/claim-file-formats.ts

All schemaVersion/version fields removed (CLAIM_FORMAT_VERSION=7, TERMINAL_FORMAT_VERSION=3, PENDING_FORMAT_VERSION=1, PHASE_FORMAT_VERSION=1). readClaim/readTerminal/readTransitionIfPresent now use assertExactKeys that rejects any record containing a 'version' key. Simultaneously, preparationEvidence is a new required field on every claim. Old persisted claims (with version, without preparationEvidence) will fail parsing at both the exact-keys check and the requireObject check for preparationEvidence. Any in-flight execution at deploy time becomes orphaned — the claim store cannot read the old record and recovery will throw. This is intentional (branch: test/unversion-candidate-runtime) but there is no migration script, format detection, or graceful rejection. Acceptable for pre-productio

🟡 LOW Removing version from persisted claim records makes pre-upgrade claims unreadable — src/candidate-execution/claim.ts

CLAIM/PENDING/PHASE/TERMINAL_FORMAT_VERSION were deleted and readClaim/readTransitionIfPresent/readTerminal now assertExactKeys without 'version'. Any claim, transition, or terminal file written by the previous build contains a version key and now throws on the exact-key assertion, so executions in flight across a rolling deploy can neither finish nor be recovered (getAttempt throws before the terminal check). Acceptable if the claim directory is ephemeral/pre-production; otherwise this needs a migration note or tolerant reader.

🟡 LOW Two distinct artifacts share the 'executor-capture' purpose — src/candidate-execution/executor-capture-evidence.ts

The raw executor evidence (line 48-53) and the summary document (line 82-89) are both persisted with purpose 'executor-capture' for the same executionId. persistCandidateOutputArtifact verifies the returned locator against the submitted bytes, so a content-addressed port is fine, but a port that keys objects by (executionId, purpose) would silently overwrite the first artifact; the second write's read-back would still pass, and later digest-pinned reads of the first ref would fai

🟡 LOW workspaceTaskRepository is now dead code — src/candidate-execution/outcome-evidence.ts

The import of workspaceTaskRepository from './workspace-task' was removed from outcome-evidence.ts (old line: const repository = workspaceTaskRepository(state.executionPlan.value.material.task)) and prepared-state.ts. The function workspaceTaskRepository in workspace-task.ts is now exported but has zero callers (confirmed via rg across src/, tests/, bench/). The workspace-task.ts file itself was not deleted. Should either delete workspace-task.ts or remove the unused export.

🟡 LOW parseAgentCandidateProfileActivation does not check per-file hashes despite its contract comment — src/candidate-execution/profile.ts

The doc comment claims it will 'Parse and check every native file hash plus both canonical document digests', but the body only verifies (a) the embedded profilePlan's canonical digest/artifact and (b) the activation's self-computed top-level digest. activation.files[i].{path,mode,content} are never compared against profilePlan.material.files[i].{relPath,mode,contentSha256}. Since the activation digest is self-computed (anyone can recompute it after tampering), a consumer that trusts this function alone would accept swapped file contents bound to a genuine plan. Current callers are safe (createAgentCandidateProfileActivation derives files from the plan; intelligence/improvement-cycle.ts:376-384 regenerates and compares digests), but the function is re-exported via src/intelligence/index.ts

🟡 LOW Recover closures don't propagate cleanup abort signal to underlying operations — src/candidate-execution/recover.ts

withinCandidateCleanupDeadline now passes an AbortSignal to the operation callback (cleanup.ts refactor), but readRecoveryPreparation (line 73-77), modelClosure (line 99-117), and memoryClosure (line 119-139) all ignore the signal parameter. If the artifact store or model API is slow, the deadline timer fires and withinCandidateCleanupDeadline rejects, but the underlying rea

🟡 LOW Recovery now hard-requires capture persistence, adding a permanent-failure mode to claim closure — src/candidate-execution/recover.ts

Lines 156-201: unless readRecoveryPreparation, executor.stop, executor.capture AND persistAgentCandidateExecutorCapture all succeed, the function throws (AggregateError or 'expired candidate capture was not persisted') and recoverExpired is never called, so the expired claim can never reach a terminal record. Base code also sealed the capture but did not have to persist it plus a summary document to close the claim. A permanently failing executor.capture (e.g. sandbox already reclaimed) or artifact-store outage leaves the attempt stuck forever, retrying and throwing on every recovery pass. Consider a bounded-attempt fallback that records failureEvide

🟡 LOW AgenticGeneratorShotReceipt loses its schemaVersion marker (unversioned shape change) — src/improvement/agentic-generator.ts

Removing readonly schemaVersion: 1 from the receipt interface and from shotReceipt() (line ~676) means any external consumer that persisted shot receipts can no longer version-discriminate old vs new receipt shapes. Verified safe inside the repo (no reader of receipt.schemaVersion anywhere; tsc clean; agentic-generator tests 25/25 pass) and consistent with the PR-wide schemaVersion removal and agent-interface 0.28.0. Informational only: if receipts are persisted across versions anywhere outside this repo, note the breaking shape change in release notes.

🟡 LOW schemaVersion field removed from AgenticGeneratorShotReceipt without migration note — src/improvement/agentic-generator.ts

The readonly schemaVersion: 1 field was removed from the interface and from shotReceipt() construction (line 675). Any external code persisting or branching on receipt.schemaVersion will silently see undefined. The field was always the constant 1, so impact is cosmetic, but there is no migration guide or changelog entry visible in the diff. Covered by the 0.95.0 bump.

🟡 LOW cleanup.ts both-attempts-fail branch has no direct test — src/improvement/cleanup.ts

The extracted helper has three outcomes; only the first-try-success and retry-succeeds paths are exercised indirectly (tests/improvement-driver.test.ts:328 covers retry-succeeds, 11/11 pass). The path where both cleanup attempts fail (throw new AggregateError([cause, ...cleanupErrors], ${context}; cleanup failed)) has no test asserting the AggregateError contents or message. The logic was traced by hand and matches the removed inline originals, so this is a coverage nit, not a defect. Fix: add a small unit test for cleanup.ts covering all three branches.

🟡 LOW Cleanup error messages lost specificity in the refactor — src/improvement/improve.ts

Base message 'improve(): code preparation failed and its baseline worktree could not be cleaned' became 'improve(): code preparation failed; cleanup failed' (and similarly at line 597). The AggregateError still carries all causes, so diagnosability is mostly preserved, but the message no longer states WHICH resource (baseline worktree vs candidate worktrees) could not be cleaned. Grep confirmed no test asserts the old strings. Cosmetic: consider including the resource in the context string.

🟡 LOW Breaking change: ProfileDiffProposerOptions.proposeDiffs signature changed — src/improvement/profile-diff-proposer.ts

The return type changed from readonly AgentProfileDiffProposal[] (objects with .diff/.label/.rationale) to readonly AgentProfileDiff[]. Any external caller passing {diff, label, rationale} wrapper objects will now be passing objects that fail parseExactAgentProfileDiff at runtime. Mitigated: 0.95.0 minor bump signals breaking; AgentProfileDiffProposal export removed from index.ts so callers get a compile error. No action needed unless external consumers exist outside this monorepo.

🟡 LOW Hard-coded power.confidence !== 0.95 couples proposals to agent-eval's default — src/intelligence/improvement-cycle.ts

power.n !== composite.n || power.confidence !== 0.95 rejects any SelfImproveResult whose powerPreflight confidence differs from 0.95. confidence is a PowerPreflight field (agent-eval contract/index.d.ts), so if agent-eval ever changes its default (or a future caller supplies it), every proposal fails closed with 'agent improvement measurement sources do not agree'. Fix: compare power.confidence against the confidence used for the bootstrap intervals in this module, or drop the literal.

🟡 LOW Redundant double-verification of proposal in execute path — src/intelligence/improvement-cycle.ts

executeApprovedAgentCandidate calls verifyAgentImprovementProposal(options.proposal) at line 288, then verifyCandidateExecutionEvidence at line 321 internally calls verifyAgentImprovementProposal(options.proposal) AGAIN (line 338). Each verification re-parses the bundle schema, re-seals, re-derives changed surfaces, and re-runs all binding checks. Not a bug — defense in depth —

🟡 LOW Type-unsafe as unknown as cast on canonicalized findings — src/intelligence/improvement-cycle.ts

findings: immutableCandidateValue([...findings]) as unknown as AgentImprovementProposal['findings'] acknowledges that the deep-canonicalized JSON round-trip may not preserve the TS type. agentImprovementProposalSchema.parse(...) on line 251 will catch any structural mismatch, so runtime safety holds, but the cast hides any future schema drift between the local canonicalization and the agent-interface schema. Fix: derive the findings type from the schema rather than casting.

🟡 LOW Failure-path cleanup can mask the original execution error — src/intelligence/sandbox-approved-candidate.ts

The failure branch uses try { throw new Error('approved candidate execution failed: ...') } finally { await executor.delete(...) }. If executor.delete throws (e.g., sandbox API transient failure), the original Error('approved candidate execution failed: ...') is suppressed and only the delete error propagates. The caller loses the actual execution-failure reason. Same pattern in the success branch (line 130-137) would replace a valid evidence return with a delete error. Fix: catch delete errors, wrap with AggregateError([originalError, deleteError]), or at minimum log the delete failure and rethrow the original.

🟡 LOW Sandbox delete in finally can mask the primary error or fail a succeeded execution — src/intelligence/sandbox-approved-candidate.ts

Success path: try { return result.evidence } finally { await executor.delete(...) } — delete() rethrows when the sandbox still exists after a failed delete, so a fully executed, verified, receipted execution surfaces as a thrown error and the evidence is dropped. Failure path: try { throw new Error('...failed: reason') } finally { await executor.delete(...) } — a delete failure masks the more actionable finalization reason (and discards finalization.usage). Fix: catch delete errors and attach them (e.g., AggregateError / error cause) without replacing the primary outcome.

🟡 LOW Trace store appendRun inside executor can leave sandbox orphaned — src/intelligence/sandbox-approved-candidate.ts

await context.traceStore.appendRun(...) runs after process exit but before returning termination. If appendRun throws (e.g., persistent trace store full), execute() throws, the state map still holds the sandbox entry, and the sandbox stays alive in the backend until maxLifetimeSeconds expires. The runtime's recovery path (recover.ts) uses executor.stop/capture/delete but may not trigger if execute itself threw. The outer createSandboxApprovedCandidateExecutor wrapper only calls executor.delete on finalization.partial (failure branch) — if execute throws before returning a finalization, no delete happens. Fix: wrap appendRun in try/catch that logs and continues, or move it to a finally that still returns termination.

🟡 LOW capture() replay deadline reuses the unrelated createTimeoutMs — src/intelligence/sandbox-approved-candidate.ts

The fresh-worker output-replay path bounds log collection with Date.now() + (this.options.createTimeoutMs ?? 120_000). createTimeoutMs is documented/used as the sandbox-create budget; reusing it as the evidence-capture budget is surprising (a product tuning create latency silently changes capture tolerance) and is not bounded by the runtime's cleanup deadline passed via context.signal. Functionally safe (signal still bounds it) but worth a dedicated option or deriving from the remaining signal deadline.

🟡 LOW createTimeoutMs option overloaded as capture/output deadline — src/intelligence/sandbox-approved-candidate.ts

Date.now() + (this.options.createTimeoutMs ?? 120_000) is used as the deadline for output collection in capture(), but the option is named createTimeoutMs and documented as sandbox-creation timeout in the CreateSandboxApprovedCandidateExecutorOptions.sandbox.createTimeoutMs field. Operators tuning capture/output latency would not think to set createTimeoutMs. Fix: add a dedicated captureTimeoutMs/outputTimeoutMs option, or rename to a generic 'deadlineMs' covering both phases.

🟡 LOW Digest format bridge adds fragile coupling between two package schemas — src/knowledge/improvement-job.ts

knowledgeDigest/rawKnowledgeDigest translate between agent-knowledge's raw-hex digestSchema (/^[a-f0-9]{64}$/) and agent-interface's Sha256Digest (sha256:-prefixed). The bridge is correct and regex-validated today, but if either package changes its digest format (e.g., agent-knowledge adopts sha256: prefix), the conversion would silently double-prefix or fail. Consider adding a comment documenting the two-schema contract, or push for alignment on a single digest format across packages.

🟡 LOW Digest-conversion helpers duplicate agent-knowledge's exported converters — src/knowledge/improvement-job.ts

These two functions re-implement toAgentCandidateKnowledgeRef / fromAgentCandidateKnowledgeRef, which @tangle-network/agent-knowledge already exports (dist/index.js:3482-3508) for exactly this KnowledgeImprovementCandidateRef <-> AgentCandidateKnowledgeRef conversion. The local copies are also subtly more lenient (knowledgeDigest at line 374 tolerates already-prefixed 'sha256:' input, the substrate versions require it), so the two implementations can drift on malformed input. Impact is limited — inputs here are schema-validated upstream, so behavior matches today — but the duplicated canonical conversion is a maintenance hazard. Fix: import and use the

🟡 LOW Unresolved KnowledgeImprovementOptions cast bypasses structural type check — src/knowledge/improvement-job.ts

The as KnowledgeImprovementOptions cast on the improveKnowledgeBase call (preserved from pre-PR code) suppresses TypeScript's structural validation of the spread object. knowledgeOptions is typed as the Omit remainder plus extra destructured fields, so the cast is likely safe today, but a future field addition to KnowledgeImprovementOptions that isn't in the Omit list would silently pass through unchecked. Replace with a typed reconstruction or remove the cast once the remainder type is proven complete. Not a regression — the old code had the identical cast.

🟡 LOW approvedKnowledgeCandidate does not check signal between verify and promote — src/knowledge/improvement-job.ts

The verify functions (verifyAgentImprovementProposal/verifyAgentImprovementReview) and authorizeReview are awaited before the first signal.throwIfAborted() at line 201. If authorizeReview is a long-running async check (e.g., network call to an approval service), an aborted signal would not interrupt it promptly. The verify calls are synchronous so the window is just authorizeReview. Minor — the throwIfAborted at line 201 catches it immediately after, but consider passing signal into authorizeReview if it can be slow.

🟡 LOW Public metadata type narrowed from Record<string, unknown> to Record<string, JsonValue> — src/knowledge/supervised-update.ts

metadata changed to NonNullable<RagKnowledgeUpdateResult['metadata']> = Record<string, JsonValue>. JsonValue excludes undefined and non-JSON values (Date, class instances), so any external consumer constructing a SupervisedKnowledgeUpdateResult with such metadata now fails to compile — a breaking type change in an exported interface (re-exported from src/knowledge/index.ts). The change is intentional and correct (it makes the result assignable to agent-knowledge's KnowledgeImprovementUpdate contract, and the only in-repo construction site at lines 160-165 is a JSON-safe literal), but it deserves a changelog/release note since this package publi

🟡 LOW Doc-only change; no functional risk — src/mcp/feedback-store.ts

Pure JSDoc rewording. Verified put()/list() bodies, FeedbackEvent/FeedbackStore interfaces, and InMemoryFeedbackStore implementation are byte-identical to base. No behavior, type, or contract change. Noting as informational only — no action required.

🟡 LOW Coverage gap: egress mode 'blocked' path untested — tests/sandbox-approved-candidate.test.ts

Test 1 only exercises egressPolicy with mode:'strict' (model network is gateway-only). The code at sandbox-approved-candidate.ts:373-380 also supports mode:'blocked' when network.mode==='disabled' (maxModelCalls===0 in the fixture's reserveGrant). No test verifies the blocked-egress create options. Low risk since the branch is a simple ternary, but it's an untested security-relevant path.

🟡 LOW No coverage in this file for blocked-egress mode or capture() metadata fallback — tests/sandbox-approved-candidate.test.ts

The four tests exercise strict-egress (network.domains) creation, oversize kill, timeout kill, and stop-after-delete, but not the network.mode==='disabled' → egressPolicy {mode:'blocked'} branch of sandboxCreateOptions, nor the capture() recovery path that re-reads output via sandboxOutputSpec(metadata) when in-memory state is absent (expired-claim recovery, the documented purpose of the executor port). If sibling test files cover these, disregard; otherwise a third sandbox-lifecycle test would close the gap.

🟡 LOW Test 4 uses a tight 25ms execution deadline — tests/sandbox-approved-candidate.test.ts

fixture.task.limits.timeoutMs = 25 produces a real 25ms execution window (deadlineAtMs = claimTime + 25). The prepare/claim/activate chain must complete and spawnExact must be called within that window for kill to fire. Stable across 5 runs (78-141ms total per test), and the mock infrastructure is lightweight, but a heavily loaded CI runner could theoretically cause the deadline to fire before spawn — in which case kill would never be called and the assertion expect(kill).toHaveBeenCalledWith('SIGKILL', { tree: true }) would fail. Consider bumping to 250-500ms for resilience margin on shared CI runners. Not blocking.

🟡 LOW Timeout test relies on a real 25ms timer — tests/sandbox-approved-candidate.test.ts

fixture.task.limits.timeoutMs = 25 makes the test depend on the real AbortSignal deadline firing promptly; under extreme CI event-loop stall the mock process.wait() only resolves via the abort-driven kill, so a sufficiently stalled runner could hit vitest's default 5s test timeout instead of exercising the intended path. Margin is large so flake risk is minimal, but a fake-timer or slightly larger bound (e.g. 50-100ms) would remove the tail risk entirely.

_{tangletools · 2026-07-15T21:46:31Z · trace}

tangletools

❌ 1 Blocking Finding — `ef4c0b5e`

Full multi-shot audit completed 8/8 planned shots over 93 changed files. Global verifier still owns final merge decision. | Full multi-shot audit completed 8/8 planned shots over 93 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-15T21:46:31Z · immutable trace}

drewstone added 3 commits July 13, 2026 10:11

feat(candidate): execute exact approved outcomes

00e85d2

Merge remote-tracking branch 'origin/main' into feat/universal-candid…

ddeab85

…ate-outcome

feat(candidate): adopt exact improvement contracts

9a2553d

tangletools approved these changes Jul 14, 2026

View reviewed changes

tangletools reviewed Jul 14, 2026

View reviewed changes

tangletools previously requested changes Jul 14, 2026

View reviewed changes

tangletools approved these changes Jul 14, 2026

View reviewed changes

tangletools reviewed Jul 14, 2026

View reviewed changes

tangletools previously approved these changes Jul 14, 2026

View reviewed changes

refactor(contract): remove candidate schema versions

2cab99c

drewstone mentioned this pull request Jul 15, 2026

fix(candidate): adopt interface 0.26 contracts #557

Merged

drewstone added 2 commits July 15, 2026 11:45

drewstone dismissed tangletools’s stale review via cff5376 July 15, 2026 17:58

tangletools previously approved these changes Jul 15, 2026

View reviewed changes

drewstone changed the title ~~feat(candidate): adopt exact improvement contracts~~ feat(candidate): execute exact approved candidates Jul 15, 2026

tangletools reviewed Jul 15, 2026

View reviewed changes

fix(candidate): materialize approved knowledge

e81b7c1

drewstone dismissed tangletools’s stale review via e81b7c1 July 15, 2026 18:15

tangletools previously approved these changes Jul 15, 2026

View reviewed changes

Merge remote-tracking branch 'origin/main' into test/unversion-candid…

52985b5

…ate-runtime # Conflicts: # CLAUDE.md

drewstone dismissed tangletools’s stale review via 52985b5 July 15, 2026 18:26

tangletools previously approved these changes Jul 15, 2026

View reviewed changes

tangletools reviewed Jul 15, 2026

View reviewed changes

drewstone added 2 commits July 15, 2026 13:30

fix(package): verify compatible runtime and bench releases

72ebca0

feat(intelligence): bind product metadata to measured comparisons

f4aa979

drewstone dismissed tangletools’s stale review via f4aa979 July 15, 2026 19:31

tangletools previously approved these changes Jul 15, 2026

View reviewed changes

fix(intelligence): stabilize measured comparisons

ef4c0b5

drewstone dismissed tangletools’s stale review via ef4c0b5 July 15, 2026 19:48

tangletools approved these changes Jul 15, 2026

View reviewed changes

tangletools reviewed Jul 15, 2026

View reviewed changes

tangletools approved these changes Jul 15, 2026

View reviewed changes

tangletools reviewed Jul 15, 2026

View reviewed changes

tangletools requested changes Jul 15, 2026

View reviewed changes

drewstone mentioned this pull request Jul 17, 2026

feat(improvement): compose live learning and tested agent optimization #561

Open

72 tasks

Uh oh!

Conversation

drewstone commented Jul 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Solution

Verification

Uh oh!

tangletools left a comment

Choose a reason for hiding this comment

✅ Auto-approved drewstone PR — 9a2553dd

Uh oh!

tangletools left a comment

Choose a reason for hiding this comment

🟡 Value Audit — sound-with-nits

💰 Value — sound

🎯 Usefulness — sound-with-nits

🔎 Heuristic Signals

🎯 Usefulness Audit

Uh oh!

tangletools commented Jul 14, 2026

❌ Needs Work — 9a2553dd

Blocking

Other

Uh oh!

tangletools left a comment

Choose a reason for hiding this comment

❌ 1 Blocking Finding — 9a2553dd

Uh oh!

tangletools left a comment

Choose a reason for hiding this comment

✅ Auto-approved drewstone PR — caa1c791

Uh oh!

tangletools left a comment

Choose a reason for hiding this comment

🟡 Value Audit — sound-with-nits

💰 Value — sound-with-nits

🎯 Usefulness — sound

🔎 Heuristic Signals

💰 Value Audit

Uh oh!

tangletools commented Jul 14, 2026

✅ No Blockers — caa1c791

Uh oh!

tangletools left a comment

Choose a reason for hiding this comment

✅ Approved — 19 non-blocking findings — caa1c791

Uh oh!

tangletools left a comment

Choose a reason for hiding this comment

✅ Auto-approved drewstone PR — cff5376c

Uh oh!

tangletools left a comment

Choose a reason for hiding this comment

🟢 Value Audit — sound

💰 Value — sound

🎯 Usefulness — sound

🔎 Heuristic Signals

Uh oh!

tangletools left a comment

Choose a reason for hiding this comment

✅ Auto-approved drewstone PR — e81b7c1c

Uh oh!

tangletools left a comment

Choose a reason for hiding this comment

✅ Auto-approved drewstone PR — 52985b5a

Uh oh!

tangletools left a comment

Choose a reason for hiding this comment

🟢 Value Audit — sound

💰 Value — sound

🎯 Usefulness — sound

🔎 Heuristic Signals

Uh oh!

tangletools left a comment

Choose a reason for hiding this comment

✅ Auto-approved drewstone PR — f4aa9795

Uh oh!

tangletools left a comment

Choose a reason for hiding this comment

drewstone commented Jul 14, 2026 •

edited

Loading

✅ Auto-approved drewstone PR — `9a2553dd`

❌ Needs Work — `9a2553dd`

❌ 1 Blocking Finding — `9a2553dd`

✅ Auto-approved drewstone PR — `caa1c791`

✅ No Blockers — `caa1c791`

✅ Approved — 19 non-blocking findings — `caa1c791`

✅ Auto-approved drewstone PR — `cff5376c`

✅ Auto-approved drewstone PR — `e81b7c1c`

✅ Auto-approved drewstone PR — `52985b5a`

✅ Auto-approved drewstone PR — `f4aa9795`

✅ Auto-approved drewstone PR — `ef4c0b5e`

⚠️ Review Interrupted — `ef4c0b5e`

✅ Auto-approved drewstone PR — `ef4c0b5e`

❌ Needs Work — `ef4c0b5e`

❌ 1 Blocking Finding — `ef4c0b5e`