feat(campaign)!: make code surfaces content-addressed#336
Conversation
tangletools
left a comment
There was a problem hiding this comment.
✅ Auto-approved drewstone PR — c284c942
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-10T07:14:11Z
tangletools
left a comment
There was a problem hiding this comment.
🟡 Value Audit — sound-with-nits
| Verdict | sound-with-nits |
| Concerns | 1 (1 weak-concern) |
| Heuristic | 0.0s |
| Duplication | 0.0s |
| Interrogation | 130.2s (2 bridge agents) |
| Total | 130.2s |
💰 Value — sound-with-nits
Makes code-tier candidates content-addressed (commits/trees/binary-patch) and verified before evaluation, extending the repo's existing promptHash content-identity philosophy to code — a coherent, in-grain change with no existing equivalent found.
- What it does: CodeSurface (
src/campaign/types.ts:132) now requiresbaseCommit/baseTree/candidateCommit/candidateTreeplus a{format:'git-diff-binary', sha256, byteLength}patch descriptor, all readonly. A newsrc/campaign/surface-identity.tsownsassertCodeSurfaceIdentity,codeSurfaceIdentityMaterial,surfaceContentHash(full sha256) andsurfaceHash(16-char loop key) derived from ONE identity. - Goals it achieves: Recorded optimization candidates can no longer point at mutable paths or moved refs: the identity used by selection/promotion (
surfaceHash/surfaceContentHash/RunRecord.promptHash) is now bound to the exact evaluated bytes. Unifies the previously-divergent 16-char loop key and full provenance hash so they cannot disagree, and gives code cells the same real content hash prompt cells already ha - Assessment: Good change, in the grain. It mirrors the established promptHash content-identity pattern (
run-record.ts:145,agent-profile-cell.ts) rather than inventing a new one, follows the repo's fail-loud/no-fallback doctrine (asserts + typedWorktreeAdapterError, legacy path-only surfaces rejected), and genuinely de-duplicates the two inline hashers inprovenance.tsandrun-optimization.tsinto o - Better / existing approach: None materially better found. Searched for prior art —
grep candidateHash|contentAddress|patchHash|candidateTree|surfaceContentHash|promptHashandCandidateBundle|base64|deployable: prompt-tier content identity already exists (promptHash) but no code-tier content identity or candidate bundle existed; code surfaces were path/ref-only, so this fills a real gap rather than reinventing. Considered - Model: opencode/kimi-for-coding/k2p7
- Bridge attempts: 1
🎯 Usefulness — sound
Content-addresses code-candidate identity through the measurement/selection/promotion chain, consolidating two prior path-hashing helpers into one immutable-identity module with hardened verification primitives.
- Integration: The identity functions are LIVE in the loop today: runOptimization calls surfaceHash on every candidate (run-optimization.ts:143,215) for the loop dedup key, and provenance uses surfaceContentHash for the byte-verifiable record (provenance.ts:40). assertCodeSurfaceIdentity is wired into policyEditProposer.coerceCandidateSurface (policy-edit.ts:112) to reject legacy path-only surfaces at the propos
- Fit with existing patterns: Fits the established grain and IMPROVES on it. The codebase previously had TWO separate identity functions hashing the SAME mutable material (worktreeRef+baseRef) — surfaceHash in run-optimization.ts and surfaceContentHash in provenance.ts. This PR consolidates both into surface-identity.ts (run-optimization.ts:358 and provenance.ts:43 now re-export from it) and switches the hashed material from m
- Real-world viability: Hardened well beyond the happy path. Verification rejects hidden index entries (worktree/index.ts:174-181), dirty/ignored files (183-202), moved refs (204-216), wrong trees (218-241), and patch-byte mismatches (281-287); rebuilds the index via read-tree --reset to defeat stat-cache spoofing (246); strips GIT_* repository-redirection env vars (85-90); and uses binary-safe (Uint8Array) diff handling
- Model: opencode/zai-coding-plan/glm-5.2
- Bridge attempts: 1
💰 Value Audit
🟡 worktreeRef is required-but-not-identity [maintenance] ``
CodeSurface.worktreeRefis typed required andassertCodeSurfaceIdentityrejects empty values (surface-identity.ts:14), yet its own doc says it is a 'mutable locator deliberately excluded from content hashes' (types.ts). It is needed byresolveWorktreePath/verifyCodeSurfaceto find the checkout, so carrying it is justified — but a type that is mandatory yet non-identity-bearing is a small consistency wart a reviewer may want named (e.g. a separatelocatorvs identity split). Does no
What this audit checks
It judges the change on its merits — not whether it was tasked out in an issue. Unticketed, fast-moving work is fine; the question is whether the change is good and whether a better or existing approach should be used instead.
| Pass | What it asks |
|---|---|
| Heuristic | Vague title? Whitespace-only or cruft-bearing diff? (content signals only) |
| Duplication | Do added function/class names already exist elsewhere in the repo? |
| Value Audit | What does it do? What goal does it achieve? Is it good? Better architecture or already-exists? |
| Usefulness Audit | Does it integrate and fit? Will it hold up in real use and actually get used? |
Findings are concerns, not blocks — the human reviewer decides what to do with them.
tangletools
left a comment
There was a problem hiding this comment.
✅ Auto-approved drewstone PR — d70a91f2
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-10T07:25:04Z
tangletools
left a comment
There was a problem hiding this comment.
🟡 Value Audit — sound-with-nits
| Verdict | sound-with-nits |
| Concerns | 2 (2 weak-concern) |
| Heuristic | 0.0s |
| Duplication | 0.0s |
| Interrogation | 223.6s (2 bridge agents) |
| Total | 223.6s |
💰 Value — sound-with-nits
Replaces a mutable path+ref CodeSurface locator with a content-addressed identity (exact commits, tree, and a deterministic binary-patch digest) plus a verifier that re-binds that identity to the live checkout — a coherent, in-grain trust fix with no existing equivalent to reuse, carrying only minor
- What it does: Pre-PR,
gitWorktreeAdapter.finalize()returned{kind:'code', worktreeRef, baseRef, summary}and bothsurfaceHash(presets/run-optimization.ts) andsurfaceContentHash(provenance.ts) hashed the locator stringsworktreeRef+baseRef. The change: (1) makesCodeSurfacecarrybaseCommit/baseTree/candidateCommit/candidateTreeplus apatch{format:'git-diff-binary', sha256, byteLength}(src/ - Goals it achieves: Make the bytes actually evaluated identical to the candidate identity used for selection/promotion/provenance. Closes the gap where a recorded candidate could silently point at moved refs, a filter-distorted-but-
git cleancheckout, replacement objects, or symlink/submodule content not bound to the signed tree. Also removes a real prior inconsistency: two separate hash functions over the same (wr - Assessment: Good on its merits. The threat model is concrete and each defense maps to a named bypass with a real-Git regression test (tests/campaign/worktree.test.ts covers clean/smudge filters, hidden mode changes, symlink paths/targets, submodules, moved refs, env redirection). The design is in the codebase's grain: fail-loud typed errors (
WorktreeAdapterError), no silent fallback, identity-by-content, VC - Better / existing approach: none — this is the right approach, and no existing equivalent was found. Searched:
git grepforexecFileSync('git'/ls-tree/rev-parse/diff-indexacross src — the only other git use is a tiny read-only metadata helper in src/product-benchmark/export.ts:127 (export provenance, not verification, not reusable).verdict-cache.ts:90 contentHashis a generic canonical-JSON hasher; reusing it w - Model: opencode/kimi-for-coding/k2p7
- Bridge attempts: 1
🎯 Usefulness — sound
Content-addressed code-surface identity replaces mutable path-based hashing across the full optimization loop, with comprehensive real-Git verification at finalization — well-wired, no dead surface, no competing pattern.
- Integration: Fully wired into the hot path.
surfaceHash/surfaceContentHash(now centralized insurface-identity.ts:57-64) are the identity functions consumed byrun-optimization.ts:215(candidate dedup keys),provenance.ts:175(byte-identical provenance record),self-improve.ts:730(RunRecord.promptHash),run-improvement-loop.ts:140(winner detection),run-lineage-loop.ts:340, and `profile/inde - Fit with existing patterns: Consolidates two scattered, weaker implementations (the 16-char loop key in
run-optimization.tsand the full content hash inprovenance.ts) into one canonical module. The old code hashed{kind, worktreeRef, baseRef}— mutable path labels — so two identical candidates in different worktrees got different hashes and a renamed worktree looked like a new candidate. The new identity (`surface-ide - Real-world viability: Verification is thorough and adversarial: scrubs inherited Git env vars that redirect the repo (
worktree/index.ts:36-46,96-102), detects clean/smudge filter entries vials-files -v(line 447), catches untracked and ignored files (lines 456-466), verifies raw blob bytes against tree objects with TOCTOU-safe open→fstat→read→recheck (lines 273-313), checks executable modes (line 400), validates s - Model: opencode/zai-coding-plan/glm-5.2
- Bridge attempts: 1
💰 Value Audit
🟡 ~650 lines of hand-rolled git plumbing is a real maintenance surface [maintenance] ``
worktree/index.ts grew to 654 lines including a binary
ls-tree -zparser (parseGitTreeEntries, line 174), streaming blob hashing with TOCTOU fd guards (hashGitBlobFile, line 273), and symlink/submodule binding. Justified by the goal (raw-byte verification necessarily bypasses git's own clean/smudge path, so it can't justgit diff --exit-code) and covered by real-Git tests, but it is bespoke code that future git behavior could make brittle. No in-repo library exists to delegate to, so this is
🟡 Verification is opt-in; nothing in this package invokes it on the run path [against-grain] ``
verifyCodeSurface/resolveWorktreePathare exported (index.ts:342-343) and unit-tested, butgit grepshows no in-src caller — the only references are docs (SKILL.md:979) telling consumers to call it before execution. TheCodeSurface.patchdocstring (types.ts:155) also references a 'deployable candidate bundle' with base64 content that has no in-repo implementation (git grep candidate-bundle|deployablein src → none). This is consistent with substrate layering (consumers wire it), but a
What this audit checks
It judges the change on its merits — not whether it was tasked out in an issue. Unticketed, fast-moving work is fine; the question is whether the change is good and whether a better or existing approach should be used instead.
| Pass | What it asks |
|---|---|
| Heuristic | Vague title? Whitespace-only or cruft-bearing diff? (content signals only) |
| Duplication | Do added function/class names already exist elsewhere in the repo? |
| Value Audit | What does it do? What goal does it achieve? Is it good? Better architecture or already-exists? |
| Usefulness Audit | Does it integrate and fit? Will it hold up in real use and actually get used? |
Findings are concerns, not blocks — the human reviewer decides what to do with them.
tangletools
left a comment
There was a problem hiding this comment.
✅ Auto-approved drewstone PR — 95702ea0
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-10T07:35:46Z
tangletools
left a comment
There was a problem hiding this comment.
✅ Auto-approved drewstone PR — ecde3882
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-10T07:40:23Z
tangletools
left a comment
There was a problem hiding this comment.
🟡 Value Audit — sound-with-nits
| Verdict | sound-with-nits |
| Concerns | 2 (2 weak-concern) |
| Heuristic | 0.0s |
| Duplication | 0.0s |
| Interrogation | 216.6s (2 bridge agents) |
| Total | 216.6s |
💰 Value — sound-with-nits
Replaces path/ref-based code-candidate identity with a fail-closed, byte-for-byte content address (git object ids + SHA-256 binary patch) verified against the real checkout before evaluation — coherent, in-grain, and it closes a real integrity hole; ship with one optional DRY note.
- What it does: Before,
CodeSurfacewas{kind, worktreeRef, baseRef?, summary?}and bothsurfaceHash(run-optimization.ts) andsurfaceContentHash(provenance.ts) hashedJSON.stringify({kind, worktreeRef, baseRef})— i.e. the mutable locator, never the bytes — with no checkout verification. After (src/campaign/types.ts:132-164, surface-identity.ts, worktree/index.ts), a finalized surface carries exact `b - Goals it achieves: Makes tier-4 (repository-level) optimization trustworthy: a recorded candidate is byte-for-byte the checkout that gets measured and promoted, immune to moved refs, mutable paths, post-finalization edits, clean/smudge-filter deception, hidden filemode flips, inherited
GIT_*env redirection, replacement objects, symlink escapes/trampolines, and submodule ambiguity. It also consolidates two near-du - Assessment: Good on its merits and squarely in the codebase's grain. The repo is already pervasively content-addressed and fail-loud —
canonicalJson/contentHash(src/verdict-cache.ts:84-92), attestation (src/attestation.ts:4-20,64), pre-registration (src/pre-registration.ts:46-49,133), dataset, agent-profile-cell, analyst/policy-edit all usesha256:content identity with strict no-coercion — and CLAUDE. - Better / existing approach: Checked for an existing equivalent to fold into:
src/integrity/has no git/worktree/tree/checkout logic (grepgit|worktree|tree|commit|verifyCode|checkout→ no matches), and there was no priorverifyCodeSurface— so nothing is duplicated. The only reuse candidate is the genericcanonicalJson/contentHashin src/verdict-cache.ts:84-92 (already imported by src/campaign/run-campaign.ts:15): - Model: opencode/kimi-for-coding/k2p7
- Bridge attempts: 1
🎯 Usefulness — sound-with-nits
Content-addressed CodeSurface identity replaces the old path-only identity, deeply wired into the loop machinery and provenance, with a fail-closed byte-level verifier — coherent, correct grain, no competing pattern.
- Integration: The hash functions (surfaceHash/surfaceContentHash) are immediately and deeply consumed: run-optimization.ts:215 (loop dedup + Pareto), run-improvement-loop.ts:140 (winner==baseline detection), run-lineage-loop.ts:221 (dispatchRef + runDir), provenance.ts:193 (baseline/winner content hashes), self-improve.ts:730 (RunRecord.promptHash), and profile/index.ts:219. assertCodeSurfaceIdentity is called
- Fit with existing patterns: Fits the established substrate pattern exactly. The WorktreeAdapter interface was already VCS-pluggable by design (worktree/index.ts:8-10); this strengthens the existing finalize→CodeSurface→measure pipeline rather than adding a parallel one. MutableSurface = string | CodeSurface (types.ts:173) is the existing union; the change deepens CodeSurface's fields rather than replacing the type. No compet
- Real-world viability: Extremely robust verification at worktree/index.ts:426-555. Strips all GIT_* repository-redirection env vars (lines 36-46, 96-101) and sets GIT_NO_REPLACE_OBJECTS=1 to prevent object replacement bypass. Verifies raw file bytes against Git tree object IDs via streaming hash (lines 273-313), not git status which clean/smudge filters can deceive. Checks executable modes (line 406), rejects submodules
- Model: opencode/zai-coding-plan/glm-5.2
- Bridge attempts: 1
🎯 Usefulness Audit
🟡 Snapshot surfaceHash in self-improve.ts still uses location-dependent JSON.stringify, diverging from the loop's content-addressed identity [integration] ``
self-improve.ts:647-650 computes the snapshot's surfaceHash via hashString(JSON.stringify(surface)), which for a CodeSurface includes the mutable worktreeRef field. The loop's surfaceHash (surface-identity.ts:67-68) deliberately excludes worktreeRef via codeSurfaceIdentityMaterial. So two identical candidates from different worktree paths produce different snapshot hashes but identical loop hashes — a semantic mismatch between what the contract output reports and how the loop deduplicates. The s
💰 Value Audit
🟡 surfaceContentHash hand-rolls sha256 instead of delegating to existing contentHash() [duplication] ``
src/campaign/surface-identity.ts:60-69 recomputes
createHash('sha256').update(...)over a JSON string while src/verdict-cache.ts:88-92 already exports the repo's canonical strictcontentHash(value)(used by src/campaign/run-campaign.ts:15 and src/attestation.ts:20). Delegating would remove one more ad-hoc hasher and keep all content addressing on one primitive. Mitigating: the identity object is fixed-key-order and built only fromassertCodeSurfaceIdentity-validated primitives (surface-ide
What this audit checks
It judges the change on its merits — not whether it was tasked out in an issue. Unticketed, fast-moving work is fine; the question is whether the change is good and whether a better or existing approach should be used instead.
| Pass | What it asks |
|---|---|
| Heuristic | Vague title? Whitespace-only or cruft-bearing diff? (content signals only) |
| Duplication | Do added function/class names already exist elsewhere in the repo? |
| Value Audit | What does it do? What goal does it achieve? Is it good? Better architecture or already-exists? |
| Usefulness Audit | Does it integrate and fit? Will it hold up in real use and actually get used? |
Findings are concerns, not blocks — the human reviewer decides what to do with them.
tangletools
left a comment
There was a problem hiding this comment.
✅ Auto-approved drewstone PR — 1c0312dd
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-10T07:57:51Z
tangletools
left a comment
There was a problem hiding this comment.
🟢 Value Audit — sound
| Verdict | sound |
| Concerns | 0 (none) |
| Heuristic | 0.0s |
| Duplication | 0.0s |
| Interrogation | 375.5s (2 bridge agents) |
| Total | 375.5s |
💰 Value — sound
Makes tier-4 code candidates byte-identifiable and verifies the checkout's raw bytes against the signed tree before eval — a coherent, in-grain trust-boundary hardening that also removes prior duplication.
- What it does: Before:
gitWorktreeAdapter.finalize()returned aCodeSurfacecarrying only a mutable locator (worktreeRef+ optionalbaseRef) — see oldsrc/campaign/worktree/index.ts:98-106. Selection, provenance, dedup and no-op detection all keyed onJSON.stringify({kind, worktreeRef, baseRef}), so two byte-identical candidates at different paths looked different and a recorded candidate could silen - Goals it achieves: Evaluation consumers get a byte-for-byte guarantee that the checkout they run is the candidate selected for measurement/promotion — closing the gap where Git reports clean while raw bytes differ (filters, replacement objects, mode-only changes). It also fixes a real correctness bug: content-identical candidates at different worktree paths now collapse to one (regression at `proposers/composite.tes
- Assessment: Good on its merits and squarely in the codebase's grain. It matches the documented substrate doctrine —
src/attestation.ts:10-13states content-addressing is the substrate's job (signing is the consumer's), and this change is exactly substrate-side content-addressing + verification. It honors the repo's fail-loud/no-fallback rule (every mismatch throwsWorktreeAdapterError; legacy path-only su - Better / existing approach: None — this is the right approach and it removes duplication rather than creating it. I checked
src/attestation.tsandsrc/verdict-cache.ts(contentHashover canonical JSON): those address serializable report objects, not Git checkout bytes — none of them re-hash raw tracked blobs against a signed tree, so there is no existing equivalent to reuse. The change actually consolidates two pre-e - Model: opencode/kimi-for-coding/k2p7
- Bridge attempts: 1
🎯 Usefulness — sound
Content-addressed code surfaces replace the old mutable path/ref identity with byte-verifiable Git tree + binary-patch identity, fully wired into the optimization loop's hashing, dedup, provenance, and RunRecord paths.
- Integration: Fully reachable and correctly wired. The sole production constructor —
gitWorktreeAdapter().finalize()atworktree/index.ts:610-624— populates every new field (baseCommit, baseTree, candidateCommit, candidateTree, patch) and self-verifies viaverifyCodeSurfaceWithGitbefore returning (line 625).surfaceContentHashis consumed in 6 production modules: provenance records (`provenance.ts:175 - Fit with existing patterns: Replaces and consolidates cleanly. Previously
surfaceHashlived inrun-optimization.ts:355andsurfaceContentHashinprovenance.ts:46— both hashed by mutableworktreeRef + baseRefJSON. This PR consolidates both intosurface-identity.tswith content-based identity that excludes locator paths and commit metadata. Backward-compat re-exports fromprovenance.ts:43and `run-optimization. - Real-world viability: Strong adversarial coverage: 23 real-git test cases in
worktree.test.tsexercise clean/smudge filters, hidden executable-mode changes, symlink traversal/trampoline escapes, submodules, ignored dirty files, moved refs, inherited GIT_DIR/GIT_WORK_TREE env bypass, and forged patch digests — all against actual temp git repos viaexecFileSync('git'), not mocks. Content-stability (identical bytes at - Model: opencode/zai-coding-plan/glm-5.2
- Bridge attempts: 1
No concerns — sound change, no better or existing approach found. ✅
What this audit checks
It judges the change on its merits — not whether it was tasked out in an issue. Unticketed, fast-moving work is fine; the question is whether the change is good and whether a better or existing approach should be used instead.
| Pass | What it asks |
|---|---|
| Heuristic | Vague title? Whitespace-only or cruft-bearing diff? (content signals only) |
| Duplication | Do added function/class names already exist elsewhere in the repo? |
| Value Audit | What does it do? What goal does it achieve? Is it good? Better architecture or already-exists? |
| Usefulness Audit | Does it integrate and fit? Will it hold up in real use and actually get used? |
Findings are concerns, not blocks — the human reviewer decides what to do with them.
❌ Needs Work —
|
| deepseek | kimi-code | aggregate | |
|---|---|---|---|
| Readiness | 51 | 73 | 51 |
| Confidence | 95 | 95 | 95 |
| Correctness | 51 | 73 | 51 |
| Security | 51 | 73 | 51 |
| Testing | 51 | 73 | 51 |
| Architecture | 51 | 73 | 51 |
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 26 changed files. Global verifier still owns final merge decision. | Full multi-shot audit completed 8/8 planned shots over 26 changed files. Global verifier still owns final merge decision.
Blocking
🔴 HIGH resolveWorktreePath now throws on verification failure — breaking semantic change — src/campaign/worktree/index.ts
Previously resolveWorktreePath returned a path via isAbsolute/existsSync/join — a best-effort locator resolution. Now it calls verifyCodeSurface() which throws WorktreeAdapterError on ANY verification failure (dirty tree, hidden index entries, symlink traversal, wrong commit, etc.). Callers that previously used resolveWorktreePath for non-finalized code surfaces or surfaces whose worktree became dirty over time will now receive unexpected exceptions. This is a deliberate design change (verify-before-use) but is undocumented as a breaking API change. Recommend: add a non-verifying variant or document the break in changelog.
Other
🟠 MEDIUM CodeSurface type made fully required — all existing consumers must add 7 new fields — src/campaign/types.ts
baseRef went from optional to required. Six new required fields (baseCommit, baseTree, candidateCommit, candidateTree, patch.format, patch.sha256, patch.byteLength) added. All fields made readonly. The canonical producer gitWorktreeAdapter.finalize() was updated, but any test utility, integration, or external consumer manually constructing a CodeSurface (memory.test.ts:91, skill-opt.test.ts:220, analyst-surface.test.ts:65) must update. The as never casts in those test paths indicate awareness, but downstream packages (agent-runtime, agent-knowledge) importing CodeSurface will get compilation errors until updated.
🟠 MEDIUM Binary-patch identity is not reproducible across git environments, yet verification recomputes it — src/campaign/worktree/index.ts
The whole point of this PR is an immutable, location-independent candidate identity, but the patch digest is not actually reproducible for binary files. PATCH_ARGS (lines 131-143) pins color/ext-diff/textconv/renames/algorithm/prefixes but never pins compression, and defaultGit (lines 94-110) only scrubs GIT_CONFIG/GIT_CONFIG_* env overrides — git still reads the repo config and ~/.gitconfig. I reproduced it directly: for the same binary change,
git diff --binary ...produced sha256 cdcc9ec65d6dc9440c040cb5118a2e4170950e77f
🟡 LOW One identity-less code candidate truncates a composite member's entire remaining slate — src/campaign/proposers/composite.ts
The dedup key changed from
JSON.stringify(surface)(never throws) tosurfaceContentHash(surface)(line 103), which calls assertCodeSurfaceIdentity and throws a TypeError for any code surface missing baseCommit/baseTree/candidateCommit/candidateTree/patch. That throw sits inside the per-membertry(lines 93-110) that wraps the ENTIRE inner proposal loop, so a single malformed code candidate from a member aborts processing of that member's remaining proposals — valid later candidates are silently dropped and the memb
🟡 LOW gitBytes wraps defaultGit Buffer return in unnecessary Buffer.from() copy — src/campaign/worktree/index.ts
defaultGit returns a Buffer from execFileSync. gitBytes calls typeof output === 'string' ? Buffer.from(output, 'utf8') : Buffer.from(output). The Buffer.from(buffer) branch creates a copy of potentially up to 256MB (MAX_GIT_OUTPUT_BYTES) for binary diffs. The copy is wasted: patchBytes, parseGitTreeEntries, and parseGitPathList all subsequently call Buffer.from() again on the result. A zero-copy path using output directly or output.subarray() would be more efficient. Not a correctness issue.
🟡 LOW Hosted test only covers CodeSurface path, not string-surface hash path — src/contract/self-improve.test.ts
The new test verifies that two CodeSurfaces with different worktreeRef produce identical surfaceHash values. It does not exercise the string path of surfaceHash (where surface is a plain string) through the hosted ingest pipeline. The string path is covered implicitly by the existing power-analysis test (which uses
'be careful'as baselineSurface) but the hosted event's surfaceHash field isn't asserted there. Low risk since surfaceHash's string branch is trivial (sha256(string)).
🟡 LOW New identity test asserts only the baseline snapshot, not the proposed-candidate generation snapshot — src/contract/self-improve.test.ts
The test finds the ingested event whose baseline.surfaceHash === surfaceHash(baseline) and asserts event.generations[0].surfaceHash === expected plus a standalone surfaceHash(sameBytesElsewhere) === expected. generations[0] is the baseline (shipEvalRunToHosted pushes index 0 = baseline first), so the snapshot-level assertions only prove the BASELINE uses surfaceHash(); the proposed candidate (sameBytesElsewhere) would land in generations[1], which is never asserted. Because baseline and the candidate share content here the candidate would coincidentally also equal expected, but a regression that kept surfaceHash for the baseline while reverting a generation snapshot to a path-derived hash would not be caught. Fix: also assert the candidate generation snapshot (e.g. generations[1]?.surfaceH
🟡 LOW String-surface hosted snapshot hash silently changes format (no string-branch test or continuity note) — src/contract/self-improve.ts
The swap to surfaceHash(surface) applies to ALL MutableSurface, including the common string-prompt baseline. For strings the value changes from the local 8-hex FNV-1a hashString(surface) to a 16-hex sha256 prefix. The hosted wire doc (src/hosted/types.ts:79) only promises a 'stable hash' string, so this is not a contract violation, but it is an observable cross-version discontinuity of the orchestrator/dashboard pivot + dedup key for every existing prompt-surface run: historical and new runs for the identical prompt will no longer share a surfaceHash. There is no string-surface test of snapshotFromCampaign (the new test only constructs CodeSurface) and no changelog/note flagging the pivot-key change. Fix: either add a one-line note/changelog about the string-surface hash change and a minim
🟡 LOW hashString (FNV-1a) remains unused for surface identity but still exported in file — src/contract/self-improve.ts
The non-cryptographic hashString is still defined and used only as a seed mix-in for cellsToRunRecords (line 767). It is no longer the surface identity hash — that role moved to surfaceHash from surface-identity. Consider marking it local-only or inlining it into cellsToRunRecords since its original purpose (surface identity) is gone. Not a correctness issue — SHA-256 is now used for identity.
🟡 LOW Filter-based worktree tests invoke sed without a win32 guard — tests/campaign/worktree.test.ts
Lines 179-180 and 199-200 register git clean/smudge filters that shell out to
sed 's/^evil$/safe/'/sed 's/^safe$/evil/', yet unlike the sibling executable-mode test (line 214:if (process.platform === 'win32') return) these tests are not platform-guarded. On a Windows runnersedis unavailable, so the filter program errors and the asserted bypass (git statusclean while raw bytes differ) may not be reached for the intended reason. Impact is environmental only — project CI is Linux andsedis standard on Linux/macO
tangletools · 2026-07-10T08:47:46Z · trace
tangletools
left a comment
There was a problem hiding this comment.
❌ 1 Blocking Finding — 1c0312dd
Full multi-shot audit completed 8/8 planned shots over 26 changed files. Global verifier still owns final merge decision. | Full multi-shot audit completed 8/8 planned shots over 26 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-10T08:47:46Z · immutable trace
tangletools
left a comment
There was a problem hiding this comment.
✅ Auto-approved drewstone PR — 14ac8d88
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-10T21:17:04Z
tangletools
left a comment
There was a problem hiding this comment.
🟢 Value Audit — sound
| Verdict | sound |
| Concerns | 0 (none) |
| Heuristic | 0.0s |
| Duplication | 0.0s |
| Interrogation | 142.6s (2 bridge agents) |
| Total | 142.6s |
💰 Value — sound
Replaces path-only code-candidate locators with a fail-closed, content-addressed identity (exact base/candidate commits + trees + a compression-pinned SHA-256 binary patch) and verifies the live checkout byte-for-byte before evaluation — a coherent, in-grain integrity fix with no existing equivalent
- What it does: Makes tier-4 code candidates content-addressed and verified.
CodeSurface(src/campaign/types.ts:135) now requires exactbaseCommit/baseTree/candidateCommit/candidateTreeplus a{format:'git-diff-binary', sha256, byteLength}patch descriptor;worktreeRefis explicitly a mutable locator excluded from identity. A new single source of truth, src/campaign/surface-identity.ts:1, defines `a - Goals it achieves: Closes a real integrity hole: previously a recorded candidate was just
{worktreeRef, baseRef}(old types.ts / old worktree/index.ts finalize returned only those), and bothsurfaceHash/surfaceContentHashhashedJSON.stringify({kind, worktreeRef, baseRef})— so a candidate could silently point at moved refs, a dirty-but-"clean"-reporting checkout, config-dependent patch bytes, or swapped raw - Assessment: Good on its merits and squarely in the grain. The repo's stated doctrine (CLAUDE.md) is "No fallbacks. Fail loud" and typed external-boundary outcomes — this PR is that doctrine applied to the Git boundary, with intentionally no lenient fallback. The breaking type change is justified (path-only surfaces were the bug). Git history shows the feature (9460139) followed by four adversarial hardening f
- Better / existing approach: none — this is the right approach. I checked for an existing equivalent to reuse: grepped
surfaceContentHash|surfaceHash|content-address|immutableandverifyCodeSurface|resolveWorktreePath|assertCodeSurfaceIdentityacross src, and inspectedgit log -- src/campaign/worktree/index.ts src/campaign/provenance.ts src/campaign/types.ts. Before this PR the worktree adapter did zero verification (ol - Model: opencode/kimi-for-coding/k2p7
- Bridge attempts: 1
🎯 Usefulness — sound
Consolidates two previously-divergent code-surface hashers into one content-addressed identity and adds byte-for-byte Git verification at finalize + resolve time — coherent, fully wired, no dead surface.
- Integration: Fully reachable. The producer path (gitWorktreeAdapter.finalize → verifyCodeSurfaceWithGit at worktree/index.ts:630) is wired end-to-end. All four identity primitives (surfaceHash, surfaceContentHash, assertCodeSurfaceIdentity, codeSurfaceIdentityMaterial) have real internal callers: composite proposer dedup (composite.ts:103), policy-edit equality (policy-edit.ts:121), provenance content hashes (
- Fit with existing patterns: Matches the codebase grain exactly. Consolidates a real duplication: surfaceHash lived in run-optimization.ts:358 and surfaceContentHash lived in provenance.ts, with different material for code surfaces (one hashed worktreeRef, the other hashed kind+worktreeRef+baseRef) — now both derive from one location-independent identity in surface-identity.ts. No competing git-content-addressing exists; the
- Real-world viability: Genuinely adversarial, tested against real Git (not mocks). The 422-line test suite at tests/campaign/worktree.test.ts covers: repo+global compression drift (line 141), clean-filter bypass (226), smudge-filter rewrite (246), assume-unchanged hidden mutations (216), executable-mode drift under core.filemode=false (261), symlink locators + symlink parents + external targets + leave-and-reenter tramp
- Model: opencode/zai-coding-plan/glm-5.2
- Bridge attempts: 1
No concerns — sound change, no better or existing approach found. ✅
What this audit checks
It judges the change on its merits — not whether it was tasked out in an issue. Unticketed, fast-moving work is fine; the question is whether the change is good and whether a better or existing approach should be used instead.
| Pass | What it asks |
|---|---|
| Heuristic | Vague title? Whitespace-only or cruft-bearing diff? (content signals only) |
| Duplication | Do added function/class names already exist elsewhere in the repo? |
| Value Audit | What does it do? What goal does it achieve? Is it good? Better architecture or already-exists? |
| Usefulness Audit | Does it integrate and fit? Will it hold up in real use and actually get used? |
Findings are concerns, not blocks — the human reviewer decides what to do with them.
What changed
Why
Repository-level optimization is not trustworthy if a recorded candidate can point at mutable paths, moved Git refs, config-dependent patch bytes, or a checkout whose raw bytes differ from the signed tree while Git reports it clean.
This gives evaluation consumers a byte-for-byte proof that the checkout they are about to run is the candidate selected for measurement and promotion.
Breaking change
resolveWorktreePath()no longer returns a best-effort unchecked locator; it verifies the finalized candidate and throws on any identity mismatch.Path-only
CodeSurfaceobjects are invalid, and downstream callers must migrate to adapter-finalized candidates when adopting 0.112.0.There is intentionally no lenient fallback on the evaluation path.
Verification
pnpm test: 270 files, 2,774 passed, 2 skippedpnpm typecheckpnpm buildpnpm verify:packagepnpm lint: 0 errors, 4 existing warningsgit merge-tree --write-tree origin/main HEAD: clean