Skip to content

feat: add a developer-facing self-improvement meta-harness - #880

Merged
drewstone merged 28 commits into
mainfrom
feat/meta-harness-developer-surface
Aug 17, 2026
Merged

feat: add a developer-facing self-improvement meta-harness#880
drewstone merged 28 commits into
mainfrom
feat/meta-harness-developer-surface

Conversation

@drewstone

@drewstone drewstone commented Aug 16, 2026

Copy link
Copy Markdown
Contributor

Why

agent-runtime already contains the important capabilities—profile improvement, exact execution identities, durable supervision journals, retained runs, and advanced optimizer adapters—but the safest path is fragmented across lower-level calls. The result is a powerful harness that is easier to misuse than it should be.

This PR starts the compressed meta-harness productization plan with additive front doors. It deliberately does not delete, rename, or narrow any existing capability.

What this PR implements

1. Bound profile-improvement harness

Adds createProfileImprovementHarness() under @tangle-network/agent-runtime/improvement.

It binds and freezes, once:

  • the exact baseline AgentProfile;
  • its canonical digest;
  • the complete caller-supplied executionRef;
  • the exact candidate executor;
  • and an optional shared candidate validator.

Every subsequent .run() retains the full existing ImproveMethodOptions surface while removing accidental profile/executor drift between experiments.

2. Canonical prompt.instructions component surface

Adds promptInstructionsProfileComponents for the existing surface: 'agent-profile' path.

The preset:

  • gives every ordered instruction a stable, zero-padded component identity;
  • preserves instruction count and order;
  • changes no unrelated profile field;
  • refuses an empty instruction list rather than inventing a shippable sentinel;
  • uses Runtime's existing exact component-key and baseline-reproduction checks.

This closes the practical gap described in #752 without adding another top-level ImproveSurface or forking the materialization path.

3. Durable supervision identity discovery

Adds discoverDurableSupervisionRun(runDir) under @tangle-network/agent-runtime/durable.

A developer can now inspect a persisted supervision directory and discover:

  • spawn-tree root identities;
  • coordination run identities;
  • owner-scoped coordination identities;
  • legacy unscoped record counts;
  • and the exact journal/log paths.

The helper uses Runtime's canonical committed-JSONL parser: missing files are empty, malformed committed records fail loud, and a torn unacknowledged tail is ignored. This addresses the usability failure in #812 without weakening existing loadTree / loadSpawnForest / coordination-log contracts.

Simplified month-long direction

The original broad roadmap reduces to four invariants:

  1. One candidate path — authored and generated candidates enter the same measured, provenance-bearing proposal path.
  2. One experiment identity — every result resolves to exact profile, executor, model, trace, cost, and artifact evidence.
  3. One durable truth model — admission, dispatch, cancellation, retry, resume, settlement, and cleanup have explicit terminal receipts.
  4. One obvious front door — common workflows are short; advanced APIs remain available and unchanged.

This PR implements the first developer-facing slice of invariants 2–4. Follow-up slices remain reviewable and independently shippable rather than combining a month of state-machine changes into one unsafe rewrite.

Compatibility

  • Additive exports only.
  • Existing improve() overloads remain canonical and unchanged.
  • Existing profile surfaces and optimizer methods remain available.
  • Existing durable journals and side logs are not migrated or rewritten.
  • No capability is removed.

Tests added

  • stable instruction component mapping and unrelated-field preservation;
  • refusal on empty instructions and key-set drift;
  • immutable baseline/executor binding and digest validation;
  • discovery of roots/run IDs/owner IDs;
  • empty-directory behavior;
  • malformed committed-record refusal.

Validation

The branch is merged with current main, generated API documentation is current, and the complete native CI matrix is green:

  • lint and version-bump checks;
  • 2,700+ tests;
  • build and source/example typechecks;
  • packed-package/export verification;
  • PrimeIntellect integration;
  • generated API reference freshness;
  • exact packed dependency cohort;
  • official GEPA and SkillOpt integrations;
  • agent-bench compatibility.

Related: #752, #812, #751, #813, #561

@drewstone
drewstone marked this pull request as ready for review August 16, 2026 20:33

@tangletools tangletools left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Auto-approved drewstone PR — 5ce6b422

This PR was opened by the trusted drewstone account.
The full PR reviewer audit still runs separately and will publish findings if it detects issues.

This approval is provisional. It rests on the audit running. If the audit cannot run — for example the CLI bridge rejects it — this approval is dismissed rather than left standing, so an unrun check never reads as a passing one.

tangletools · auto-approval · reason: drewstone_author · 2026-08-16T20:34:00Z

@tangletools tangletools left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Value Audit — sound-with-nits

Verdict sound-with-nits
Coverage 2 of 2 lenses (value, usefulness)
Concerns 5 (5 weak-concern)
Heuristic 0.0s
Duplication 0.1s
Interrogation 464.5s (2 bridge agents)
Total 464.6s

💰 Value — sound-with-nits

Adds three thin, additive developer front doors — a bind-once profile-improvement harness over improve(), a canonical prompt.instructions component preset for the existing agent-profile surface, and a cold directory reader for durable supervision identities — each fills a verified gap and reuses exi

  • What it does: Three additive exports. (1) createProfileImprovementHarness() (src/improvement/profile-improvement-harness.ts:67) validates, detaches, and freezes one AgentProfile plus its executionRef/agent/validator at construction, and exposes .run() that forwards to improve() with the bound fields omitted, so repeated experiments cannot drift on profile or executor identity. (2) promptInstructionsProfileCompo
  • Goals it achieves: Removes the two easiest drift mistakes in repeated self-improvement runs (mutating the baseline profile between runs, silently changing executionRef); closes the real gap that no existing ImproveSurface targets profile.prompt.instructions as an ordered list ('prompt' is systemPrompt-only at src/improvement/profile-surface.ts:39-43, 'memory' is resources.instructions at :347-356) without forking th
  • Assessment: Coherent and in the codebase's grain. All three delegate to single sources of truth rather than forking: the harness forwards to improve() preserving the full ImproveMethodOptions surface via Omit; the preset rides the existing components contract whose exact-key-set, round-trip, and baseline-reproduction checks already live in src/improvement/profile-surface.ts:172-181, 193-214, 293-305; the disc
  • Better / existing approach: None found that is materially better. Searched for an existing bound-runner/harness helper (none in src beyond this PR), an existing prompt.instructions surface or component preset (none — improve.test.ts:687 shows caller-supplied inline mappings were the only prior pattern, which this preset canonicalizes), and an existing supervision identity listing (none). The plausible alternative — a new Imp
  • Model: opencode/zai-coding-plan/glm-5.2
  • Bridge attempts: 7
  • Bridge warning: opencode/kimi-for-coding/k2p7: opencode: opencode error

🎯 Usefulness — sound-with-nits

Three additive developer front doors that each formalize a pattern the repo already hand-rolls, wired correctly into real surfaces; only nits found.

  • Integration: All three pieces are publicly reachable and compose with existing machinery. createProfileImprovementHarness and promptInstructionsProfileComponents are exported through src/improvement/index.ts and re-exported at the package root via src/index.ts:113 (export * from './improvement'); discoverDurableSupervisionRun is exported from the real ./durable subpath (package.json:32, tsdown entry `durab
  • Fit with existing patterns: Excellent fit — additive thin front doors over established lower-level APIs, no competition. The harness is explicitly documented as not replacing improve() and omits only the three bound fields from ImproveMethodOptions (improve-types.ts:90-125). promptInstructionsProfileComponents is the first shipped preset for an interface that previously required inline hand-rolled read/apply objects (improve
  • Real-world viability: Holds up past the happy path. Frozen-binding claims are real: immutableCandidateValue deep-freezes and detaches (src/candidate-execution/digest.ts:21-25, 72-86), and improve() re-parses so freezing cannot break downstream mutation-based flows. Discovery is a point-in-time read of append-only fsynced files using the same committed-record parser as the runtime; malformed committed records fail loud
  • Model: opencode/zai-coding-plan/glm-5.2
  • Bridge attempts: 4

💰 Value Audit

🟡 Discovery re-encodes the on-disk record format owned by two other modules [maintenance] ``

src/durable/supervision-discovery.ts hardcodes the file names (spawn-journal.jsonl, coordination-log.jsonl — also inline strings at src/runtime/supervise/run-context.ts:111,115) and the record field shapes (kind/root from SpawnJournalRecord in spawn-journal.ts:692, runId/ownerId from coordination-log.ts). No shared constants exist to reuse today, but a layout change now needs edits in two places. If this drifts, consider listRoots()/listRuns() on the owning classes or exporting the filename/reco

🟡 Harness repeats per-run validation improve() already performs [duplication] ``

improve() already parses and freezes the profile (src/improvement/improve.ts:95-101) and method-execution.ts:78,316 already validates the executionRef digest format on every run. The harness's copies (profile-improvement-harness.ts:70-83) are intentional fail-at-construction checks, not a behavior fork, and the conditional validateCandidate spread at :99-103 is unnecessary-but-harmless since method-execution.ts:387 uses optional chaining. Minor; noted only so a reviewer knows the duplication is

🎯 Usefulness Audit

🟡 Advertised @tangle-network/agent-runtime/improvement subpath does not exist in package.json exports [ergonomics] ``

The PR body and the barrel's framing put the harness 'under @tangle-network/agent-runtime/improvement', but package.json's exports map and tsdown.config.ts have no ./improvement entry — the symbols are reachable only via the root entry (src/index.ts:113). A developer following the PR body gets ERR_PACKAGE_PATH_NOT_EXPORTED. Either add the ./improvement export + tsdown entry to complete the front door, or correct the stated path to the root import. Not dead surface either way.

🟡 Discovery roots mixes top-level run roots with nested driver tree keys [robustness] ``

Nested owned trees are begun in the same journal file with parent/child keys (src/runtime/supervise/driver-executor.ts:194, tree-key.ts:13-15), and discoverDurableSupervisionRun collects every begin record's root (supervision-discovery.ts:66-77). loadSpawnForest's own docstring calls nested keys 'a Runtime implementation detail' (spawn-journal.ts:325-330). A developer reading roots to answer 'how many runs live here' will over-count recursive runs. Distinguish top-level roots (e.g. exclude

🟡 Nonexistent runDir silently returns an empty discovery [robustness] ``

readOptionalText maps ENOENT to undefined (supervision-discovery.ts:118-127), so a typo'd path reports 'no identities' rather than an error. This matches the repo convention (FileSpawnJournal.loadTree, spawn-journal.ts:240-244), so it is in-grain, but for a developer-facing discovery entry point a missing directory is almost always a mistake worth failing on. Reviewer's call; note only.


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 · 20260816T213322Z

@tangletools

Copy link
Copy Markdown
Contributor

✅ No Blockers — 5ce6b422

Review health 100/100 · Reviewer score 48/100 · Confidence 75/100 · 24 findings (3 medium, 21 low)

opencode GLM 5.2 opencode DeepSeek v4 Pro opencode DeepSeek v4 Flash aggregate
Readiness 70 74 48 48
Confidence 75 75 75 75
Correctness 70 74 48 48
Security 70 74 48 48
Testing 70 74 48 48
Architecture 70 74 48 48

Reviewer score is advisory once the run is complete and the verdict has no blockers.

Full multi-shot audit completed 3/3 planned shots over 10 changed files. Global verifier still owns final merge decision. | Full multi-shot audit completed 3/3 planned shots over 10 changed files. Global verifier still owns final merge decision. | Full multi-shot audit completed 3/3 planned shots over 10 changed files. Global verifier still owns final merge decision.

🟠 MEDIUM roots mixes top-level and nested owned-tree roots with no way to tell them apart — src/durable/supervision-discovery.ts

The spawn journal records a begin record for EVERY tree: supervisor.ts:563 begins the run's top-level root, and driver-executor.ts:194 begins each nested owned tree (nestedDriverTreeRoot). The docstring advertises loadSpawnForest(journal, root) as the next step, but a caller cannot know which returned root is the top-level run root: begin records carry only {kind,root,at}, and the ownedTreeRoot edge lives only in the parent tree's spawned events, which this helper never scans. Picking a nested root yields a valid-but-wrong forest (nested tree materialized as the root tree, top-level tree omitted). Fix: also scan spawned events' ownedTreeRoot values and either exclude nested roots or label them (e.g. a topLevelRoots field) so loadSpawnForest callers pick the right identity.

🟠 MEDIUM Core run() path has zero test coverage — src/improvement/meta-harness.test.ts

The describe('createProfileImprovementHarness') block only tests construction invariants and failure modes; no test calls harness.run(...) to assert that the bound agent, executionRef, and validator actually reach improve(), or that harness.profileDigest equals result.lineage.baselineProfileDigest. The digest correlation is the load-bearing claim of the harness (execution identity reproducibility), and it is not asserted in CI. I verified manually against the lockfile-pinned agent-interface@1.0.0 that canonicalAgentProfileDigest (RFC-8785 normalization) and the repo contentAddress/stableStringify used for baselineProfileDigest in method-execution.ts produce identical digests for the fixture and edge-value profiles, so the code is currently correct — but any future divergence in either dige

🟠 MEDIUM harness.run() delegation and validator precedence have no test coverage — src/improvement/meta-harness.test.ts

The tests cover construction, immutability, and config errors, but never call harness.run(). run() contains the module's only real branching: the ternary chain at profile-improvement-harness.ts:97-101 (run-level validateCandidate overrides harness default; neither means no validator) and the spread-order guarantee that executionRef/agent cannot be overridden by runtime extra properties. A regression in either would silently change experiment identity semantics — the exact drift this harness exists to prevent. Fix: add a test with a stub improve target (or spy on ./improve) asserting (a) run-level validator wins over the harness default, (b) default applies when run omits it, (c) passing executionRef/agent inside runOptions at runtime does not displace the bound values.

🟡 LOW DurableSupervisionDiscovery.roots is under-documented and includes nested owned-tree roots — docs/api/durable.md

docs/api/durable.md:319-321 renders roots with no description, yet the section intro (line ~309) promises the discovery is the 'first step before calling FileSpawnJournal.loadTree(root), loadSpawnForest(journal, root)'. Implementation (src/durable/supervision-discovery.ts:62-73) collects the root of every kind:'begin' record in the shared spawn-journal.jsonl, and nested driver-owned trees are begun on the same journal file (src/runtime/supervise/driver-executor.ts:194 calls journal.beginTree(nestedRoot, ...)). So a recursive run surfaces the top-level run root PLUS every nested tree root, with no field marking which is the top-level root that loadSpawnForest(journal, root) expects. A developer following the documented flow on a nested root gets a sub-forest. Fix: add a TSDoc line on th

🟡 LOW DurableSupervisionDiscovery.roots renders with no description and conflates top-level and nested tree roots — docs/api/durable.md

The roots property renders bare (source supervision-discovery.ts:22 has no TSDoc on it), and the function doc positions discovery as 'the developer-facing first step before calling FileSpawnJournal.loadTree(root) / loadSpawnForest(journal, root)'. But nested trees are begun on the SAME spawn-journal.jsonl (driver-executor.ts:194 journal.beginTree(nestedRoot,...), alongside the top-level begin at supervisor.ts:563), so roots returns the top-level run root PLUS every nested tree root with nothing to distinguish them (spawn-journal.ts:296 even relies on multiple begin records coexisting). A developer following the documented flow with a nested root silently gets a sub-forest instead of the full run tree. Fix: add a TSDoc line on roots stating it lists every begin-record root including nes

🟡 LOW discoverDurableSupervisionRun doc names follow-up symbols not exported from the durable subpath — docs/api/durable.md

docs/api/durable.md:407-409 instructs developers that discovery is the step 'before calling FileSpawnJournal.loadTree(root), loadSpawnForest(journal, root), or FileCoordinationLog.load(runId, ownerId)', but none of those three are exported from the @tangle-network/agent-runtime/durable subpath that exports discoverDurableSupervisionRun (src/durable/index.ts exports exactly 11 symbols, none of them). They live on the package root entry (src/runtime/index.ts:39,42,587). A developer importing only from the durable subpath cannot resolve the named follow-ups without a second root import. Fix: note the import source in the doc, or re-export the three symbols from the durable subpath so the documented workflow is importable in one place.

🟡 LOW Catalog lists fully-documented types as 'undocumented supporting types' — docs/api/primitive-catalog.md

DurableCoordinationStreamIdentity, CreateProfileImprovementHarnessOptions, and ProfileImprovementHarnessRunOptions are fully documented (with properties and TSDoc) in docs/api/durable.md and docs/api/index.md, yet primitive-catalog.md places them in the 'Undocumented supporting types' list. Root cause: each lacks a TSDoc comment directly on the declaration (only on properties / for the type-alias member), so the catalog generator's 'add a TSDoc line at the declaration to earn a table row' rule is technically satisfied. Not a correctness bug, but the reader sees a documented type labeled undocumented. Optional fix: add a one-line declaration-level TSDoc to each of the three declarations in src/durable/supervision-discovery.ts and src/improvement/profile-improvement-harness.ts so they earn t

🟡 LOW Primitive catalog labels fully-documented types as 'Undocumented supporting types' — docs/api/primitive-catalog.md

docs/api/primitive-catalog.md:282 and :385 list CreateProfileImprovementHarnessOptions, ProfileImprovementHarnessRunOptions, and DurableCoordinationStreamIdentity under 'Undocumented supporting types', but all three have complete documented sections in the API reference (docs/api/index.md and docs/api/durable.md). The root cause is the generator rule in scripts/gen-primitive-catalog.mjs ('add a TSDoc line at the declaration to earn a table row'): these declarations lack a leading TSDoc comment even though their properties are annotated and typedoc documents them. The label 'Undocumented' is therefore misleading to a reader browsing the catalog. Fix: add a one-line TSDoc block to each declaration (src/improvement/profile-improvement-harness.ts:19, :33 and src/durable/supervision-disco

🟡 LOW Test coverage misses half the validation surface — src/durable/supervision-discovery.test.ts

Tests cover the happy path (roots/streams/owners/unscoped dedup + sort), an empty directory, and the spawn-journal missing-root throw. Untested: coordination record with missing/empty runId (line 81-83 throw), coordination record with non-string or empty ownerId (line 96-99 throw), torn unterminated final record being ignored, an empty (0-byte) coordination log, and a spawn journal containing only event records (roots empty). Recommend adding these branches since fail-loud is a core contract.

🟡 LOW Untested validation branches: malformed coordination record, empty runDir, torn tail — src/durable/supervision-discovery.test.ts

The malformed-record test covers only the spawn-journal begin case (/root identity/). The symmetric coordination-log throws (runId missing at supervision-discovery.ts:79, empty ownerId at :87) and the runDir TypeError guard (:47) have no coverage, nor does the documented torn-final-append tolerance. These paths are small and mirror tested ones, but they are the function's entire failure surface. Fix: add one test per throw branch plus a truncated-final-line fixture asserting it is ignored.

🟡 LOW coordination-log failure branches untested — src/durable/supervision-discovery.test.ts

The test suite exercises the spawn-journal 'no root identity' throw (line 78) but never the two coordination-log throws: a record with a non-string/empty runId (supervision-discovery.ts:84) or a present-but-empty ownerId (supervision-discovery.ts:98). These fail-loud paths are load-bearing for the 'malformed committed identity records' contract but unverified. Impact: regression risk on error handling. Fix: add two cases mirroring the existing 'refuses malformed committed identity records' test with coordination-log.jsonl fixtures.

🟡 LOW Fourth copy of isNoEntError in the durable layer — src/durable/supervision-discovery.ts

Identical helpers already exist in jsonl-file.ts:64, spawn-journal.ts:1230, and coordination-log.ts:226. Duplication is the existing convention here, but jsonl-file.ts is the shared module this file already imports from; exporting the helper there would stop the pattern at four copies. Cosmetic; no behavioral risk.

🟡 LOW Inconsistent sort keys undermine the deterministic-display claim — src/durable/supervision-discovery.ts

coordinationStreams is sorted with left.localeCompare(right) while roots and ownerIds use default Array.prototype.sort() (UTF-16 code-unit order). localeCompare ordering can differ across runtime ICU locales, so the 'sorted for deterministic display' contract is not strictly environment-independent for streams, unlike the other two fields. Use the same plain .sort() on all three for a uniform, locale-independent order.

🟡 LOW Inconsistent sort strategies weaken the deterministic-ordering claim — src/durable/supervision-discovery.ts

coordinationStreams are ordered with localeCompare (line 105) while roots (line 119) and ownerIds (line 109) use default code-unit sort. localeCompare is ICU-version sensitive, so the doc comment 'sorted for deterministic display' (line 7) does n

🟡 LOW One malformed committed record poisons discovery for every run sharing the file — src/durable/supervision-discovery.ts

Because several runs share one spawn-journal.jsonl / coordination-log.jsonl (coordination-log.ts:13), the fail-loud throw on any malformed committed record (line 68 root check, line 82 runId check) aborts discovery of ALL runs in the directory, even when the corrupted record belongs to a different run. This matches the runtime loaders' own all-or-nothing parse (parseCommittedJsonLines throws globally), so it is consistent behavior, but worth documenting that a single poisoned run blocks discovery of healthy co-located

🟡 LOW third local copy of isNoEntError — src/durable/supervision-discovery.ts

isNoEntError is duplicated here and already exists privately in jsonl-file.ts:64 and coordination-log.ts:226 (spawn-journal.ts has one too). jsonl-file.ts already owns the shared JSONL utilities (parseCommittedJsonLines, prepareJsonlAppend, writeAllBytes) and does not export its copy. Impact: minor drift risk if ENOENT detection ever changes. Fix: export isNoEntError from jsonl-file.ts and reuse, or leave as-is since it follows the existing per-file pattern.

🟡 LOW unscopedRecords doc comment overstates its meaning — src/durable/supervision-discovery.ts

The comment claims 'Records written before owner-scoped coordination identities were introduced', but the legacy CoordinationLogRecord type (coordination-log.ts:85-90) still carries an optional ownerId, and the current append() path also omits ownerId whenever the caller passes undefined. The field actually counts 'records where ownerId was omitted', which can occur in current records too, not only pre-owner-scoping legacy records. Impact: misleading doc only; no behavior change. Fix: reword to 'Records whose record omitted ownerId'.

🟡 LOW executionRef is caller-asserted, never verified against the bound agent — src/improvement/profile-improvement-harness.ts

The harness's stated purpose is 'binds execution identity once, removing accidental experiment drift', and executionRef is documented as the identity of the bound executor/models/tools/closures. But executionRef is validated only for format, never derived from or cross-checked against the agent closure (impossible for closures). A stale/mismatched digest silently makes two different executors share one execution identity, defeating resume-state isolation. This matches improve()'s existing caller-trust contract, so it is a documented-contract nit rather than a regression; consider documenting that executionRef must be derived by the caller from the exact agent implementation (e.g., canonicalCandidateDigest over the agent's identity inputs).

🟡 LOW executionRef validation duplicates sha256DigestSchema via regex — src/improvement/profile-improvement-harness.ts

The harness re-implements the sha256 digest shape check with /^sha256:[0-9a-f]{64}$/ while method-execution.ts:75-81 already validates the same field via the shared sha256DigestSchema from @tangle-network/agent-interface. Equivalent today, but a second source of truth for the digest format that can drift. Fix: reuse sha256DigestSchema.safeParse for consistency.

🟡 LOW profileDigest and improve() lineage.baselineProfileDigest use different digest canonicalizations — src/improvement/profile-improvement-harness.ts

harness.profileDigest is computed with interface canonicalAgentProfileDigest (RFC 8785 JCS; agent-execution-preparation.ts:419 -> canonicalCandidateDigest over canonicalAgentProfileValue), while improve()'s result lineage.baselineProfileDigest is computed with the runtime-local canonicalCandidateDigest (digest.ts:16 -> contentAddress/stableStringify over sorted keys + JSON.stringify, dropping undefined). The two are different canonicalizations of the same profile, so a consumer correlating harness.profileDigest with (await harness.run(...)).lineage.baselineProfileDigest will see mismatched strings for the identical baseline. Not a functional bug in the harness itself, but a provenance footgun. Fix: either document the two identity spaces or reuse one digest convention for both.

🟡 LOW validateCandidate: null disables the harness validator only by accident — src/improvement/profile-improvement-harness.ts

The merge checks runOptions.validateCandidate !== undefined, so passing null yields { validateCandidate: null }. In runMethodImprovement, validateCandidate?.(...) no-ops on null and buildMethodEvaluationIdentity excludes it from candidateValidation (input.validateCandidate ? ... : null), so null happens to disable the construction-time validator AND removes it from eval identity. But null is not assignable to ImproveCandidateValidator, so there is no typed way for a caller to run without the harness-level validator. Either narrow the check to !== null && !== undefined (rejecting null) or document null as the opt-out.

🟡 LOW Component ordering relies on localeCompare and a 6-digit index cap — src/improvement/prompt-instructions-profile-components.ts

Sorting uses String.prototype.localeCompare, which is locale/collation dependent, and componentName pads to width 6. For any profile with >= 1,000,001 instructions the pad no-ops and lexicographic order diverges from numeric order. Both fail closed today: the strict expected-name check throws a clear ConfigError rather than silently reordering, and equal-width ASCII digit names sort identically under any default collation. Still, a plain codepoint comparison (a < b ? -1 : a > b ? 1 : 0, as used in src/durable/content-address.ts:14) would make ordering deterministic regardless of host locale and make the failure boundary explicit. Impact is theoretical at realistic instruction counts.

🟡 LOW apply() throws raw ZodError instead of ConfigError — src/improvement/prompt-instructions-profile-components.ts

apply() calls agentProfileSchema.parse(...) (throws a raw ZodError) rather than safeParse + ConfigError. The materializer path (profile-surface.ts:200) invokes apply() with no try/catch, so any schema rejection would escape as a non-AgentEvalError, violating the errors.ts contract ('every error this package throws ... extends AgentEvalError'). The parse is also redundant: validateProfileCandidate (profile-surface.ts:275-283) re-parses with safeParse and throws ConfigError. Residual failure modes are narrow (orderedInstructionValues already guarantees a non-empty string array), so impact is minor. Fix: use safeParse and throw ConfigError, or drop the parse here and let the caller validate.

🟡 LOW localeCompare in component key sort is locale-dependent — src/improvement/prompt-instructions-profile-components.ts

Sorting uses left.localeCompare(right). For the ASCII, zero-padded prompt.instruction:000000 key space every locale yields code-unit ordering, so the current behavior is stable and the subsequent exact-key check fails closed on any mis-sort. However, the ordering guarantee rests on a locale assumption; a plain code-unit comparison (a < b) would be locale-proof and match how the materializer and assertCandidateSurfaceKind sort keys elsewhere. Cosmetic hardening, not a defect.


tangletools · 2026-08-16T21:47:09Z · trace

tangletools
tangletools previously approved these changes Aug 16, 2026

@tangletools tangletools left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Approved — 24 non-blocking findings — 5ce6b422

Full multi-shot audit completed 3/3 planned shots over 10 changed files. Global verifier still owns final merge decision. | Full multi-shot audit completed 3/3 planned shots over 10 changed files. Global verifier still owns final merge decision. | Full multi-shot audit completed 3/3 planned shots over 10 changed files. Global verifier still owns final merge decision.

Full immutable report for this review: trace

Summary comment for this run: full summary


tangletools · 2026-08-16T21:47:09Z · immutable trace

@tangletools tangletools left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Auto-approved drewstone PR — c9fbcfda

This PR was opened by the trusted drewstone account.
The full PR reviewer audit still runs separately and will publish findings if it detects issues.

This approval is provisional. It rests on the audit running. If the audit cannot run — for example the CLI bridge rejects it — this approval is dismissed rather than left standing, so an unrun check never reads as a passing one.

tangletools · auto-approval · reason: drewstone_author · 2026-08-16T22:31:03Z

@tangletools tangletools left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Value Audit — sound-with-nits

Verdict sound-with-nits
Coverage 2 of 2 lenses (value, usefulness)
Concerns 4 (4 weak-concern)
Heuristic 0.0s
Duplication 0.1s
Interrogation 194.2s (2 bridge agents)
Total 194.3s

💰 Value — sound-with-nits

Three thin, additive developer front doors over existing lower-level APIs (a freeze-once improvement harness, the first shipped profileComponents preset, and durable supervision identity discovery) — coherent, in-grain, no existing equivalent found; ship.

  • What it does: Three additive exports. (1) createProfileImprovementHarness() (src/improvement/profile-improvement-harness.ts:67) validates, deep-freezes, and digests a baseline AgentProfile plus its executionRef/agent callback once, and exposes .run() that delegates each call to the existing improve() with those bound identities — callers keep every other ImproveMethodOptions knob. (2) `promptInstruc
  • Goals it achieves: Make the safest path the easy path without narrowing anything: prevent profile/executor drift across repeated improvement experiments (bind once, frozen), eliminate the hand-rolled ad-hoc component mappings developers were writing inline (e.g., five separate inline implementations in src/improvement/improve.test.ts:687-796), and remove the need to hand-parse durable files to discover the identitie
  • Assessment: Good on its merits. All three pieces are thin delegation layers over existing seams rather than forks: the harness delegates to improve() (which still re-validates per run, improve.ts:95-101), the preset plugs into the existing generic profileComponents materializer with its exact key-set and baseline-reproduction checks (src/improvement/profile-surface.ts:194-211, 290-303) instead of adding a
  • Better / existing approach: none — this is the right approach. Searched: createHarness|Harness across src/ (only CLI-harness execution concepts, unrelated), ImproveProfileComponents|profileComponents (interface + materializer existed; preset is the first implementation), discover|Discovery (no prior discovery helper), ownedTreeRoot (discovery's nested-root filter mirrors loadSpawnForest's canonical ownership check),
  • Model: opencode/zai-coding-plan/glm-5.2
  • Bridge attempts: 2
  • Bridge warning: opencode/kimi-for-coding/k2p7: opencode: opencode error

🎯 Usefulness — sound-with-nits

Three additive developer front doors — a bound improvement harness, a canonical prompt-instructions component preset, and durable supervision identity discovery — each plugging into an existing extension point in the codebase's grain, filling real ergonomic gaps with no duplication of existing capab

  • Integration: All three APIs are exported and reachable: both improvement additions ship from src/improvement/index.ts (re-exported at the package root via src/index.ts:113 'export * from ./improvement'), and discoverDurableSupervisionRun ships under the existing ./durable package subpath (package.json exports + src/durable/index.ts:27-31). No in-repo caller outside the new tests yet, but this is a published li
  • Fit with existing patterns: Excellent fit on all three. promptInstructionsProfileComponents plugs into the pre-existing caller-owned profileComponents extension point (improve-types.ts:120,174-183) on the existing surface: 'agent-profile' path (profile-surface.ts:64-73); it adds no new ImproveSurface kind and no forked materialization, and Runtime's existing exact-key-preservation and baseline round-trip checks (profile-surf
  • Real-world viability: Holds up beyond the happy path. Torn unacknowledged append tails are ignored while malformed committed records fail loud (jsonl-file.ts:15-17), matching the runtime's own corruption stance; missing files yield an empty frozen discovery rather than an error, which is correct for fresh run dirs; output is deterministically sorted and deeply frozen. The harness test proves the core safety claim at ru
  • Model: opencode/zai-coding-plan/glm-5.2
  • Bridge attempts: 1

💰 Value Audit

🟡 Two independent root-derivation logics for spawn-journal.jsonl [duplication] ``

src/tui/top-model.ts:319-321 infers the journal root from parentless spawned events with label 'root' for the operator view, while src/durable/supervision-discovery.ts:60-86 derives roots from begin records minus ownedTreeRoot values. Different consumers and different failure semantics (TUI swallows corrupt lines to keep the operator view alive; discovery fails loud), so the split is defensible today — but if a third reader appears, extract one shared root-derivation helper rather than a t

🟡 Harness duplicates executionRef validation improve() already performs [maintenance] ``

profile-improvement-harness.ts:76 hand-rolls /^sha256:[0-9a-f]{64}$/ while method-execution.ts:75-78 already validates via sha256DigestSchema; the profile schema parse also runs twice (harness bind + improve per run). Fail-early-at-bind-time is the point of a binder so the cost is acceptable, but exporting/reusing validateExecutionRef would keep one regex-of-record for the invariant.

🎯 Usefulness Audit

🟡 PR body names a package subpath that package.json does not export [ergonomics] ``

The PR body says createProfileImprovementHarness() is added 'under @tangle-network/agent-runtime/improvement', but package.json exports contain no './improvement' entry (verified by listing exports: only '.', ./agent, ./conversation, ./durable, ./tool-loop, ./intelligence, ./kernel, etc.). The API is reachable via the root entry through src/index.ts:113, so nothing is dead, but developers following the PR body's import path will get a resolution error. Either add the './improvement' subpath expo

🟡 Discovery skips ownedTreeRoot canonicity check that loadSpawnForest enforces [robustness] ``

supervision-discovery.ts:79-84 adds any present ownedTreeRoot to nestedRoots without verifying it equals nestedDriverTreeRoot(treeRoot, spawn.id), unlike loadSpawnForest (spawn-journal.ts:414-419) which fails loud on non-canonical values. A corrupted non-canonical ownedTreeRoot would cause discovery to silently omit that tree's root from the top-level list instead of surfacing the corruption. Low impact since loading the forest fails loud anyway; aligning the check would make discovery's failure


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 · 20260817T003810Z

@drewstone
drewstone merged commit bff3c30 into main Aug 17, 2026
4 checks passed
@drewstone
drewstone deleted the feat/meta-harness-developer-surface branch August 17, 2026 00:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants