fix(profile): preserve harness in profile diffs#27
Conversation
tangletools
left a comment
There was a problem hiding this comment.
✅ Auto-approved drewstone PR — 7d47b36e
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-09T22:04:41Z
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 | 62.3s (2 bridge agents) |
| Total | 62.3s |
💰 Value — sound
Wires the previously-orphaned AgentProfile.harness field through the diff contract (set/remove/changed-axes/prune) and centralizes the property-axis list so the four sites can't drift again — a coherent, in-grain fix with no better approach available.
- What it does: Adds
harnessas a first-class diff axis everywhere the contract enumerates axes: the removal type (profile-diff.ts:51) and its zod schema (profile-schema.ts:142), theapplyRemovaldeletion path (profile-diff.ts:213), and the axis-reporting/pruning loops. It replaces three separately-duplicated axis arrays (previously atprofile-diff.ts~257, ~278, ~318 in the #22 contract) with one sha - Goals it achieves: Closes a real drift gap: PR #22 added the diff contract without
harness, PR #25 later addedAgentProfile.harness, but the diff surfaces were never updated — so a diff could not target, report, or prune the harness axis andchangedAgentProfileAxeswould silently omit it. Centralizing the axis list prevents the next added profile field from drifting the same way. The delete-not-undefined chang - Assessment: Good on its merits and in the codebase's grain. It mirrors the explicit anti-drift philosophy already encoded in
profile-schema.ts:214-228(theMutuallyAssignablecompile-time guard) by applying the same 'single source of truth' idea to the axis enumeration. Scope is proportionate — three files, behavior strictly additive plus the correctness fix for prune — and the new tests pin set-only, rem - Better / existing approach: none — this is the right approach. I searched for an existing axis list to reuse (
grepforas const/axes/keyof AgentProfileacrosspackages/agent-interface/src): the onlyas constarrays are unrelated (sandbox-size.ts:22,interaction.ts:170), and the three axis arrays this PR deletes were the only enumerations. Akeyof AgentProfilederivation would not work: the diff deliberately - Model: opencode/kimi-for-coding/k2p7
- Bridge attempts: 1
🎯 Usefulness — sound
Completes the existing AgentProfileDiff surface by adding the already-typed harness field to set/remove/changed-axis/prune, centralizing the axis list to prevent drift and fixing a base-clobbering bug in prune along the way.
- Integration: Reachable as a public SDK contract:
export * from "./profile-diff.js"at packages/agent-interface/src/index.ts:809 re-exports applyAgentProfileDiff, changedAgentProfileAxes, pruneAgentProfileDiff, defineAgentProfileDiff, and agentProfileDiffSchema. No internal repo caller consumes the diff surface yet (grep across all packages/agent-* turned up only the agent-interface package's own test file), - Fit with existing patterns: Fits the established pattern exactly.
harnessis treated identically to the other scalar-ish axes:set.harnessflows through the existing mergeAgentProfiles overlay spread (agent-profile.ts:442-456, overlay wins);remove.harness = truemirrorsremove.confidential = trueandremove.identity = true(whole-axis clear, profile-diff.ts:213, 235); the schema entry `harness: z.literal(true).opt - Real-world viability: Holds up on the non-happy paths that matter. The
delete set[axis]/delete remove[axis]change in pruneAgentProfileDiff (profile-diff.ts:296-310) is a genuine fix, not cosmetic: the previousset[axis] = undefinedleft the key present with value undefined, which the return guardObject.values(set).some(v => v !== undefined)tolerated but which would then be spread by mergeAgentProfiles as ` - 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.
✅ No Blockers —
|
| opencode-kimi | glm | deepseek | aggregate | |
|---|---|---|---|---|
| Readiness | 89 | 92 | 82 | 82 |
| Confidence | 70 | 70 | 70 | 70 |
| Correctness | 89 | 92 | 82 | 82 |
| Security | 89 | 92 | 82 | 82 |
| Testing | 89 | 92 | 82 | 82 |
| Architecture | 89 | 92 | 82 | 82 |
Reviewer score is advisory once the run is complete and the verdict has no blockers.
Full multi-shot audit completed 2/2 planned shots over 4 changed files. Global verifier still owns final merge decision. | Full multi-shot audit completed 2/2 planned shots over 4 changed files. Global verifier still owns final merge decision. | Full multi-shot audit completed 2/2 planned shots over 4 changed files. Global verifier still owns final merge decision.
🟠 MEDIUM Truthiness check on harness removal inconsistent with !==undefined guards — packages/agent-interface/src/profile-diff.ts
if (remove.harness)uses truthiness while all other new guards (model, permissions, tools, etc.) use!== undefined. Fortrue | undefinedthis is functionally identical, but a maintenance hazard: ifharnessever gained a falsy-but-defined value (e.g.false), this guard would silently skip. Useif (remove.harness !== undefined)for consistency with the pattern established on lines 195, 210, 214, 217, 220, 223, 226, 229, 232, 236, 239.
🟡 LOW No direct-apply test for set+remove of the same harness axis (remove-after-set) — packages/agent-interface/src/profile-diff.test.ts
The only test combining
set.harnesswithremove.harnessprunes before applying, so the documented remove-after-set semantics (applyAgentProfileDiff merges then removes, profile-diff.ts:260-266, interface comment :65-72) for the harness axis is never exercised directly. A diff like{ set:{ harness:'codex' }, remove:{ harness:true } }applied to a profile with harness='claude-code' should deterministically yield harness=undefined (remove wins). Behavior is correct and documented, so non-blocking; add one assertion to lock it and guard the scalar axis the same way the record axes are implicitly covered.
🟡 LOW set+remove on the harness axis in one diff is not directly asserted — packages/agent-interface/src/profile-diff.test.ts
The remove-wins-over-base case is covered (line 170 'explicitly removes only the harness axis'), and the prune test (line 195) exercises a diff carrying both set.harness and remove.harness — but only AFTER pruning harness out. No test applies a diff with BOTH set.harness and remove.harness to confirm the documented 'remove deletes axes AFTER the overlay is applied' ordering for harness specifically. Tracing applyAgentProfileDiff→mergeAgentProfiles (overlay wins → merged.harness='codex') then applyRe
🟡 LOW AgentProfileDiffRemoval schema/interface lack a drift guard — packages/agent-interface/src/profile-schema.ts
profile-schema.ts:224-228 only enforces bidirectional assignability between agentProfileSchema and AgentProfile; there is no equivalent guard for agentProfileDiffRemovalSchema vs AgentProfileDiffRemoval (profile-diff.ts:46-63). This PR keeps them in sync for
harness(literal(true)/true), so not a defect here, but a future axis added to only one side will compile clean and diverge silently. Pre-existing pattern, out of scope to fix in this shot; flagging for a follow-up.
🟡 LOW No MutuallyAssignable guard for AgentProfileDiffRemoval vs zod schema — packages/agent-interface/src/profile-schema.ts
The compile-time drift guard on lines 224-228 covers AgentProfile ↔ agentProfileSchema, but there is no equivalent guard for AgentProfileDiffRemoval ↔ agentProfileDiffRemovalSchema. Adding a new axis to AgentProfileDiffRemoval without the zod counterpart would not produce a compile error. This is pre-existing (all other removal fields have the same gap), and the centralized
agentProfileDiffPropertyAxesarray reduces the risk by making the axis list single-source, but a belt-and-suspenders type-level guard here would prevent future drift.
tangletools · 2026-07-09T22:10:39Z · trace
tangletools
left a comment
There was a problem hiding this comment.
✅ Approved — 5 non-blocking findings — 7d47b36e
Full multi-shot audit completed 2/2 planned shots over 4 changed files. Global verifier still owns final merge decision. | Full multi-shot audit completed 2/2 planned shots over 4 changed files. Global verifier still owns final merge decision. | Full multi-shot audit completed 2/2 planned shots over 4 changed files. Global verifier still owns final merge decision.
Full immutable report for this review: trace
Summary comment for this run: full summary
tangletools · 2026-07-09T22:10:39Z · immutable trace
tangletools
left a comment
There was a problem hiding this comment.
✅ Auto-approved drewstone PR — dba35912
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-09T22:14:15Z
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 | 195.5s (2 bridge agents) |
| Total | 195.5s |
💰 Value — sound-with-nits
Threads the new harness field through the profile-diff contract (set/remove/changed/prune) and centralizes the axis list so the three inline copies can't silently drop a field again — fixes two real latent bugs from PR #25's incomplete rollout.
- What it does: Extends AgentProfileDiff so
harnessis a first-class diff axis: (1) addsharnessto a new centralizedagentProfileDiffPropertyAxesconst that previously existed as three separate inline arrays in profile-diff.ts:268-310, none of which included harness; (2) addsharness?: trueto AgentProfileDiffRemoval (profile-diff.ts:51) and the matchingz.literal(true)to agentProfileDiffRemovalSchema - Goals it achieves: Make harness fully round-trippable through the diff/improvement-loop surface — consistent with PR #25's stated intent that harness is 'a first-class lever an improvement loop can optimize.' Concretely it fixes two latent gaps PR #25 left behind: changedAgentProfileAxes silently missed any harness change (the ablation/causal-analysis view was blind to it), and pruneAgentProfileDiff could not prune
- Assessment: Coherent and in-grain. The scalar-removal handling clones the existing
confidentialpattern exactly (profile-diff.ts:213 vs :235), so no new shape is invented. The guards added around each applyRemoval axis (e.g.if (remove.model !== undefined)) are behavior-preserving — removeKeys/removeValues already short-circuit on undefined (profile-diff.ts:107, :120) — so they add clarity and avoid a red - Better / existing approach: Searched for an existing axis enumerator to reuse (git grep for keyof AgentProfile / profileAxes / ProfileAxis across all .ts) — none exists; only profile-diff.ts references these axes. Considered deriving the list from
keyof AgentProfileto make drift structurally impossible, but it does not fit: AgentProfile intentionally mixes 'identity' axes (name/description/version/tags) with 'property' ax - Model: opencode/zai-coding-plan/glm-5.2
- Bridge attempts: 2
- Bridge warning: opencode/kimi-for-coding/k2p7: Bridge returned 503: {"error":{"message":"cli-bridge admission timed out after 30000ms","type":"admission_rejected","reason":"queue_timeout","admission":{"active":20,"queued":1,"maxActive":20,"maxQueue":48}}}
🎯 Usefulness — sound
Catches the profile-diff contract up to the recently-promoted AgentProfile.harness field (set/remove/changed/prune), centralizes the previously-duplicated 13-axis list to prevent recurrence, and fixes pruning to delete keys instead of leaving undefined-valued ghosts.
- Integration: Fully reachable. The diff helpers are public exports (packages/agent-interface/src/index.ts:809 re-exports profile-diff.js); applyAgentProfileDiff/changedAgentProfileAxes/pruneAgentProfileDiff are the documented contract for 'a portable full-profile patch' (CHANGELOG 0.19.0) and harness is the named 'first-class lever an improvement loop can optimize' (CHANGELOG 0.20.0). This is a published SDK (@
- Fit with existing patterns: Exactly in-grain. The codebase already chose an explicit property-axis list + a compile-time schema/interface drift guard (profile-schema.ts:219) as its pattern, and this PR honors it: the new agentProfileDiffPropertyAxes const (profile-diff.ts:8) replaces four copies of the same array and is the single place a future axis gets added. harness removal is
true-only (scalar, like confidential), dis - Real-world viability: Holds up. Tests cover the load-bearing cases: set-only (164-167), remove-only with schema rejecting
false(183-193), set-then-remove ordering (196-204), and pruning that isolates a co-set axis (206-231). The pruning switch fromset[axis] = undefinedtodelete set[axis]is required for the.not.toHaveProperty('harness')assertion (line 226) and matters downstream for any consumer using `has - Model: opencode/zai-coding-plan/glm-5.2
- Bridge attempts: 1
💰 Value Audit
🟡 Centralized axis list could be compile-time pinned to the interface [maintenance] ``
agentProfileDiffPropertyAxes (profile-diff.ts:8) is the single source of truth now, but nothing enforces it stays a subset of AgentProfile's keys. Adding
as const satisfies readonly (keyof AgentProfile)[]would turn a future typo or a dropped field into a compile error — the same class of drift this PR is fixing. Pure hardening, not required to ship; the bidirectional MutuallyAssignable guard in profile-schema.ts:224 already catches schema/interface drift, just not diff-axis-list drift.
🎯 Usefulness Audit
🟡 Non-harness removal guards are behaviorally redundant [robustness] ``
profile-diff.ts:210-241 wraps every removeKeys/removeResources call in
if (remove.X !== undefined), but removeKeys already early-returns onremoval === undefined(line 107) and removeResources does the same (line 149). Only the harness guard (line 213) is load-bearing, because harness is a scalar handled outside removeKeys. The added guards on model/permissions/tools/mcp/subagents/resources/hooks/modes/metadata/extensions don't change any behavior; they obscure which single guard is actually
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 — 362b0bcb
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-09T22:21:01Z
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 | 103.7s (2 bridge agents) |
| Total | 103.7s |
💰 Value — sound
Closes a real gap: the harness preference (added to AgentProfile in PR #25) was unreachable from the diff contract and pruning silently destroyed base values; this wires harness through set/remove/changed-axes/prune and fixes prune to delete keys instead of writing undefined.
- What it does: Adds harness as a first-class diff axis: harness?: true on AgentProfileDiffRemoval (profile-diff.ts:63) and agentProfileDiffRemovalSchema (profile-schema.ts:142); includes 'harness' in a new single source-of-truth axis list agentProfileDiffPropertyAxes (profile-diff.ts:11-26) consumed by the AgentProfileDiffAxis type (37-39), changedAgentProfileAxes (285/292), and pruneAgentProfileDiff (318). appl
- Goals it achieves: (1) Let an improvement loop target harness routing as a lever — the explicit motivation in the AgentProfile.harness doc (agent-profile.ts:275-286) — which was impossible before because changedAgentProfileAxes never reported 'harness' and remove:{harness:true} failed both zod and the TS interface. (2) Make pruning preserve the base profile: previously pruneAgentProfileDiff(diff,['harness']) left se
- Assessment: Good, in-grain, proportionate. harness is correctly classified as a property axis, not identity (consistent with its doc that it is a PREFERENCE, not IDENTITY). The centralization + exhaustiveness guard directly addresses the root cause (drift between the AgentProfile type and the diff contract) rather than patching the symptom. The delete-vs-undefined change is a genuine correctness fix, not cosm
- Better / existing approach: none — this is the right approach. AgentProfileDiff and all its helpers live only in packages/agent-interface/src/profile-diff.ts (the canonical home; agent-profile.ts:7 notes sandbox merely re-exports), and grep confirms no other package reimplements diff/overlay/prune. The change extends that exact surface and reuses mergeAgentProfiles/applyRemoval rather than reinventing merge semantics. The on
- Model: opencode/kimi-for-coding/k2p7
- Bridge attempts: 1
🎯 Usefulness — sound
Adds the newly-introduced harness axis to the profile-diff system (set/remove/changed/prune) and centralizes the axis list with a compile-time exhaustive guard, fixing the gap left when AgentProfile.harness was added in PR #25.
- Integration: The diff functions (applyAgentProfileDiff, changedAgentProfileAxes, pruneAgentProfileDiff) are public API exported at index.ts:809 and are the established mechanism for portable profile-improvement artifacts. Before this PR, the brand-new harness field (agent-profile.ts:287) was invisible to every one of them: changedAgentProfileAxes would not report it, pruneAgentProfileDiff would not strip it, a
- Fit with existing patterns: Extends the one established pattern rather than competing with it. The centralized agentProfileDiffPropertyAxes array (profile-diff.ts:11) with the exhaustive compile-time guard (profile-diff.ts:32-35) is this codebase's own idiom — the same MutuallyAssignable/never-trick appears in harness.ts:78-82 and profile-schema.ts. Centralization removes the three-way duplication that let harness slip throu
- Real-world viability: Holds up beyond the happy path. The most consequential change is switching prune from set[axis]=undefined to delete set[axis] (profile-diff.ts:320-321): an explicit undefined in the overlay would clobber the base profile's value when the pruned diff is re-applied via mergeAgentProfiles (shallow spread), whereas an absent key preserves it — the test at profile-diff.test.ts:229 asserts base harness
- 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.
✅ No Blockers —
|
| opencode-kimi | glm | deepseek | aggregate | |
|---|---|---|---|---|
| Readiness | 95 | 86 | 92 | 86 |
| Confidence | 70 | 70 | 70 | 70 |
| Correctness | 95 | 86 | 92 | 86 |
| Security | 95 | 86 | 92 | 86 |
| Testing | 95 | 86 | 92 | 86 |
| Architecture | 95 | 86 | 92 | 86 |
Reviewer score is advisory once the run is complete and the verdict has no blockers.
Full multi-shot audit completed 2/2 planned shots over 4 changed files. Global verifier still owns final merge decision. | Full multi-shot audit completed 2/2 planned shots over 4 changed files. Global verifier still owns final merge decision. | Full multi-shot audit completed 2/2 planned shots over 4 changed files. Global verifier still owns final merge decision.
🟡 LOW Patch bump may understate an additive feature — .changeset/tidy-runners-diff.md
The changeset bumps @tangle-network/agent-interface as 'patch', but the description ('Add the profile harness preference to AgentProfileDiff ... operations') describes adding a new preference to a diff set — typically an additive/minor change under semver. If the harness preference is a new public field on AgentProfileDiff, consider 'minor' to signal the additive capability to consumers. No runtime impact; judgment call for the maintainer.
🟡 LOW No test for harness set-overlay when base profile has no harness — packages/agent-interface/src/profile-diff.test.ts
baseProfile always sets
harness: 'claude-code'(line 13). The 'sets only the harness axis' test (line 151) verifies overlay-wins (claude-code → codex), but no test covers the base-has-no-harness → set-harness path. This is the common real-world case (profiles created without a harness preference, then optimized to add one). The logic is correct (spread in mergeAgentProfiles handles absent fields), but the coverage gap means a regression in the merge path for harness wouldn't be caught. Add a test w
🟡 LOW Minor applyRemoval guard style inconsistency with confidential — packages/agent-interface/src/profile-diff.ts
harness uses
if (remove.harness !== undefined)while confidential (line 247) usesif (remove.confidential). Both are functionally equivalent fortrue | undefinedtypes, but inconsistent. Prefer the same guard style across both.
🟡 LOW Redundant undefined-guards in applyRemoval duplicate helper short-circuits — packages/agent-interface/src/profile-diff.ts
The new
if (remove.model !== undefined)/if (remove.tools !== undefined)/ etc. guards wrap calls to removeKeys/removeResources, which ALREADY short-circuit onremoval === undefined(removeKeys line 119:if (!record || removal === undefined) return record). The guards are functionally no-ops for correctness — old codenext.model = removeKeys(next.model, undefined)returned next.model unchanged; new code skips the assignment. The only measurable effect is avoiding oneasMutable(next.tags)shallow copy when remove.tags is undefined ([line 208](https://github.com/tangle-network/agent-sdk/blob/362b0bcb977964f934798feafe1e95f35bc2f8ef/packag
tangletools · 2026-07-09T22:30:45Z · trace
tangletools
left a comment
There was a problem hiding this comment.
✅ Approved — 4 non-blocking findings — 362b0bcb
Full multi-shot audit completed 2/2 planned shots over 4 changed files. Global verifier still owns final merge decision. | Full multi-shot audit completed 2/2 planned shots over 4 changed files. Global verifier still owns final merge decision. | Full multi-shot audit completed 2/2 planned shots over 4 changed files. Global verifier still owns final merge decision.
Full immutable report for this review: trace
Summary comment for this run: full summary
tangletools · 2026-07-09T22:30:45Z · immutable trace
What changed
AgentProfile.harnessfield to profile-diff set, remove, changed-axis, and prune behaviorundefined, preserving the base profileChecked before building:
AgentProfile.harnessand the canonicalAgentProfileDiffimplementation already existed; this extends those surfaces rather than adding another profile type.Verification
git diff --check