fix(harness): derive the harness taxonomy from the canonical enum - #357
Conversation
KNOWN_HARNESSES was a hand-maintained copy of the harness taxonomy, and the module documented itself as "a superset of agent-interface's HarnessType". That was false — it omitted gemini — and three `as HarnessType` casts rested on it. Derive the list from harnessTypeSchema.options instead, with two named lists: NON_BACKEND_HARNESSES (gemini — KNOWN_HARNESSES is dispatched as backend.type and the platform ships no gemini backend) and EXTRA_HARNESSES (forge, cursor — backends the canonical enum has yet to adopt, guarded so their adoption upstream becomes a compile error). Replace the casts with an explicit canonical narrowing: a harness outside the canonical enum is answered here as router-backed rather than cast into a type it does not belong to. Runtime behavior is unchanged; snapHarnessToModel now fails loud instead of casting a result this platform has no backend for.
tangletools
left a comment
There was a problem hiding this comment.
✅ Auto-approved drewstone PR — 706f7f6e
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-31T17:48: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 | 157.0s (2 bridge agents) |
| Total | 157.0s |
💰 Value — sound
Derives the hand-maintained harness list from agent-interface's canonical zod enum and replaces three unsafe as HarnessType casts with a runtime narrowing — same output set, no drift, no latent cast bugs.
- What it does: Replaces the hand-written
KNOWN_HARNESSESarray with one derived at runtime fromharnessTypeSchema.options(the canonical peer-dep enum), minus an explicitly namedNON_BACKEND_HARNESSES = ['gemini'](no platform provider adapter) plus an explicitly namedEXTRA_HARNESSES = ['forge','cursor'](real backends the canonical enum lacks). The threeas HarnessTypecasts in the model-compatibilit - Goals it achieves: (1) Eliminate a hand-maintained copy of an enum that lives in the peer dep, so a harness added upstream reaches this shell with no edit here and no silent drift. (2) Remove genuinely unsafe casts: forge/cursor are NOT in HarnessType (verified:
harnessTypeSchema.options= 13 ids, forge/cursor absent), soharnessSupportsModel(harness as HarnessType, ...)was feeding the capability table an id it - Assessment: Sound and in the grain of the codebase. Verified
harnessTypeSchema.options= [claude-code, nanoclaw, codex, opencode, kimi-code, pi, gemini, hermes, openclaw, amp, factory-droids, acp, cli-base]; new set = (canonical − gemini) + [forge, cursor] = the SAME 14 ids as the old hand list, so zero behavioral regression on the taxonomy (confirmed by set-equality check and all 15 tests passing). The cas - Better / existing approach: none — this is the right approach. Checked: (a) the sandbox SDK's BackendType cannot be imported — invariant 4 keeps this module substrate-free, so agent-interface's zod schema IS the correct single source of truth; (b) no other module in the repo derives this set — grep shows KNOWN_HARNESSES is only consumed (by src/run/index.ts:47, src/skills-placement/index.ts:95), never re-derived; (c) pushing
- Model: opencode/zai-coding-plan/glm-5.2
- Bridge attempts: 2
- Bridge warning: opencode/kimi-for-coding/k2p7: bridge stream ended without value-audit content
🎯 Usefulness — sound
Replaces a hand-maintained harness copy (that had already drifted — it was silently missing canonical gemini) with a derivation from the canonical peer enum, plus principled fail-closed exclusions and a compile-time drift guard — no consumer's runtime behavior changes.
- Integration: Fully reachable.
KNOWN_HARNESSES/Harness/isHarness/coerceHarnessare consumed bysrc/run/index.ts:19,47(execution-mode dispatch +isKnownSandboxHarness),src/skills-placement/index.ts:21,95(skill-dir filtering), andassertHarnessModelCompatiblebysrc/sandbox/index.ts:2211,2611(server-side model guard before dispatch).KNOWN_HARNESSESis dispatched asbackend.type(`src/sa - Fit with existing patterns: Exactly in-grain. AGENTS.md invariant 4 keeps
/harnesssubstrate-free (no sandbox SDK import), so it cannot readBackendTypedirectly; deriving from@tangle-network/agent-interface'sharnessTypeSchema(already a peer import at index.ts:24) is the correct single-source-of-truth move and matches the AGENTS.md 'extend, never duplicate / reuse the canonical enum' directive. The `canonicalHarne - Real-world viability: Holds up. The new set is identical to the old set (verified: old 14 = new canonical-13 −
gemini+forge/cursor), so no consumer regresses. Thegeminidrop is the load-bearing correctness fix: offering it would have been fail-open, sinceKNOWN_HARNESSESis dispatched asbackend.typeonto a platform with nogeminiadapter. The newsnapHarnessToModelthrow cannot fire on current upstr - 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 —
|
| glm | deepseek | deepseek-flash | aggregate | |
|---|---|---|---|---|
| Readiness | 89 | 92 | 80 | 80 |
| Confidence | 80 | 80 | 80 | 80 |
| Correctness | 89 | 92 | 80 | 80 |
| Security | 89 | 92 | 80 | 80 |
| Testing | 89 | 92 | 80 | 80 |
| Architecture | 89 | 92 | 80 | 80 |
Reviewer score is advisory once the run is complete and the verdict has no blockers.
Full multi-shot audit completed 4/4 planned shots over 8 changed files. Global verifier still owns final merge decision. | Full multi-shot audit completed 4/4 planned shots over 8 changed files. Global verifier still owns final merge decision. | Full multi-shot audit completed 4/4 planned shots over 8 changed files. Global verifier still owns final merge decision.
🟡 LOW API reference no longer enumerates valid Harness values — docs/api/harness.md
The regenerated docs replaced every expanded union (
"opencode" | "claude-code" | ... | "cli-base") with a bareHarnessalias, but theHarnessentry renderstype Harnesswith no body andKNOWN_HARNESSESrendersreadonly Harness[]. No page in this shot (harness.md, run.md, sandbox.md, skills-placement.md) lists the concrete valid values, and run.md:30/sandbox.md:38/skills-placement.md:38,54 referenceHarnesswith no definition or pointer to./harness. Base version exposed the full union in ~13 signatures, so a doc consumer loses the discoverable list of harness strings (opencode, claude-code, forge, cursor, cli-base, ...) with no replacement. Fix: have the agent-docs generator render alias bodies forHarness, or add the concrete value list to theKNOWN_HARNESSESdescripti
🟡 LOW Codemap doc strings are truncated by the generator — docs/codemap.json
resolveSkillDir and unsupportedSkillHarnesses docs end in '...' mid-sentence (e.g. '...or that a caller should warn about before offering…'), dropping the source JSDoc tail ('install UX' / the null-when-unbridged clause). This is pre-existing codemap-generator truncation behavior, identical to how the replaced union signatures were truncated, so it is cosmetic and consistent — but the file is the public API map, and truncating at 140 chars can elide load-bearing clauses. Optional: raise the generator's truncation width or ellipsize on a sentence boundary. No action required for this PR.
🟡 LOW llms-full.txt loses the only in-file enumeration of valid harness values — docs/llms-full.txt
Before this change, every harness-typed signature carried the inline union (
"opencode" | "claude-code" | ... | "forge"...) and the Harness type entry pointed at KNOWN_HARNESSES. Now all 13 rendered signatures and the Harness type show only the bare alias, and no line in the file enumerates valid values (grep for any literal harness string across docs/llms-full.txt returns zero). This is a faithful rendering of the source refactor (src/harness/index.ts now derivesHarness = Exclude<HarnessType, 'gemini'> | 'forge' | 'cursor'from the agent-interface schema), so it is not an inaccuracy — but for a doc whose purpose is complete self-contained LLM consumption, an LLM can no longer learn the valid harness set or thatgeminiwas excluded without opening the type's definition upstream. Cons
🟡 LOW Asymmetric drift guard — no compile-time signal when a dropped harness gains a backend — src/harness/index.ts
There is a compile-time guard preventing EXTRA_HARNESSES from going stale (
_extrasAreNotYetCanonical), but the symmetric risk for NON_BACKEND_HARNESSES is only caught at test time, not compile time. If the platform ships a gemini backend tomorrow,NON_BACKEND_HARNESSES = ['gemini']silently keeps dropping it and the only signal is the runtime test[...canonical].filter((h) => !known.has(h))).toEqual(['gemini'])— which still passes (gemini stays dropped). The shell would refuse a backend the platform now supports. Not a bug today (no gemini backend exists), but the asymmetry is worth a one-line comment, or a peer-review note. The test at index.test.ts:36 is the sole guard and is correct for the current state.
🟡 LOW KNOWN_HARNESSES order changes: opencode-first becomes claude-code-first — src/harness/index.ts
The old literal array led with 'opencode'; the derived array preserves the canonical enum order (claude-code, nanoclaw, codex, opencode, …) with forge/cursor appended. No in-repo consumer depends on order (all usages are Set membership / .filter / .map in src/run and src/skills-placement — verified by grep), but the array is exported and products that render a harness picker from it will now sort 'claude-code' before 'opencode'. DEFAULT_HARNESS is still explicitly 'opencode', so no internal default shifts. Fix: document the order contract in the JSDoc or, if a canonical picker order matters, sort a copy for display in the product. Not blocking.
🟡 LOW New defensive throw in snapHarnessToModel is untested — src/harness/index.ts
The
if (!isHarness(snapped)) throw new Error(...)branch added at lines 172-176 has no test. The repo's own bar (AGENTS.md 'Prove a new test can fail') requires a test that exercises the guard. It is currently unreachable with agent-interface@0.40.0 becausepreferredHarnessForModelonly returns claude-code/codex/kimi-code/null and the inputcanonicalis never gemini (excluded by type), soaiSnapHarnessToModelcannot produce a non-Harness value today. A test that injects a fakeaiSnapHarnessToModelreturning 'gemini' would prove the guard fires when a future agent-interface maps google→gemini. Without it, the guard could silently rot or be deleted as 'unreach
🟡 LOW No tripwire for upstream enum gaining a NEW non-backend harness — src/harness/index.ts
The extras direction has a compile-time guard (
_extrasAreNotYetCanonicalforces a type error when forge/cursor enter HarnessType), but the drop direction has only the exact-match testexpect([...canonical].filter(...)).toEqual(['gemini']). If the canonical enum adds a second member the platform ships no backend for, that member flows into KNOWN_HARNESSES and the dispatchable set silently (the drop-set test still passes since the drop set is unchanged), so a session could resolve onto abackend.typewith no runner — the exact failure NON_BACKEND_HARNESSES exists to prevent. Current behavior is correct (only gemini is non-backend today, verified against 0.40.0); this is an asymmetry in the guard design worth closing when the enum next moves. Fix options: assert at build/test time that
🟡 LOW Unreachable throw path in snapHarnessToModel with current exclusions — src/harness/index.ts
The throw at line 173 guards against aiSnapHarnessToModel returning a value that isn't a known Harness. With the present NON_BACKEND_HARNESSES=['gemini'], aiSnapHarnessToModel can only return claude-code/codex/kimi-code/opencode or the input harness itself — all of which are in KNOWN_HARNESSES. The path is unreachable today. It's defensive code that pays for itself if NON_BACKEND_HARNESSES expands (e.g. kimi-code dropped), so it's correct to keep. The test suite has no coverage for this line, which is acceptable for a safety-net path, but worth noting.
tangletools · 2026-07-31T18:03:49Z · trace
tangletools
left a comment
There was a problem hiding this comment.
✅ Approved — 8 non-blocking findings — 706f7f6e
Full multi-shot audit completed 4/4 planned shots over 8 changed files. Global verifier still owns final merge decision. | Full multi-shot audit completed 4/4 planned shots over 8 changed files. Global verifier still owns final merge decision. | Full multi-shot audit completed 4/4 planned shots over 8 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-31T18:03:49Z · immutable trace
Problem
src/harness/index.tskept a hand-maintained copy of the harness taxonomy, and its header claimed:That was false.
KNOWN_HARNESSESomittedgemini, whichHarnessTypedoes list — so the two sets overlap, they do not nest. Threeas HarnessTypecasts (:126,:133,:139) rested on that claim, plus anas Harnesson the return ofsnapHarnessToModel.This was one of four enums describing the same concept across the org (agent-interface
HarnessType13, agent-dev-containerBACKEND_TYPE14, this one 14,MATERIALIZER_HARNESSES9).Change
Derive
KNOWN_HARNESSESfromharnessTypeSchema.options, with two explicitly named lists:NON_BACKEND_HARNESSES = ['gemini']—KNOWN_HARNESSESis dispatched asbackend.type(src/sandbox/index.ts:2013,:2253,:2641), andsrc/run/index.ts:9states it "is exactly the set of sandbox backends". The platform ships no gemini backend (nosdk-provider-geminiin agent-dev-container), so offering it would resolve a session onto a runner that cannot start. Deriving without this exclusion would have been a fail-open regression.EXTRA_HARNESSES = ['forge', 'cursor']— real backends the canonical enum has yet to adopt, with a compile-time guard (ExtrasAlreadyCanonical) that turns their adoption upstream into a build error so the list gets emptied rather than duplicating a canonical id.The casts are replaced by an explicit
canonicalHarness()narrowing. A harness outside the canonical enum is answered here — those runners are multi-provider CLIs with no vendor lock, so they run any model and never snap — instead of being cast into a type it does not belong to.snapHarnessToModelnow throws if it ever snapped to a harness this platform has no backend for, rather than casting.The false superset paragraph is replaced with an accurate description of the overlap.
Behavior
Runtime behavior for existing ids is unchanged. Proven by a deliberate break (below): reverting to the old
as HarnessTypecast leaves every assertion green, becauseforge/cursorhave no provider lock and so already resolved as router-backed. This change buys type soundness and single-sourcing, not different answers.One disclosed side effect:
KNOWN_HARNESSESorder changes. It wasopencode, claude-code, nanoclaw, kimi-code, …; it is now canonical order then extras (claude-code, nanoclaw, codex, opencode, …, forge, cursor). Nothing in this repo depends on the order (DEFAULT_HARNESSis set explicitly, and the two consumers use.includes/.filter), but a consumer rendering the array directly in a picker would see a reordered list.Verification — including the "prove a new test can fail" bar
Per CLAUDE.md, new tests were broken on purpose to confirm they can go red.
Break 1 — stop excluding gemini (
NON_BACKEND_HARNESSES = []):Restored → 16 passed.
Break 2 — restore the old cast (
return aiSnapHarnessToModel(harness as HarnessType, modelId) as Harness):This one stayed green, which is the useful result: it proved a test I had written ("never snaps to a harness this platform has no backend for") could not fail, because
aiSnapHarnessToModelonly ever returnsclaude-code/codex/kimi-code/the input/opencode. Per the repo rule that a test which cannot fail is worse than no test, I deleted it rather than shipping a tautology. The remaining tests all discriminate.Gate (clean worktree off
origin/main, baseline vs after):typechecktestThe failing set is byte-identical before and after (
diffof the failing-file lists is empty). All 156 share one environmental cause —Error: Could not locate the bindings file(unbuilt nativebetter-sqlite3binding on this box) — acrosstests/app-auth,tests/chat-store/*,tests/teams/*,tests/intakes/*,tests/vertical/*and friends. Pre-existing, unrelated to this change, and disclosed rather than claimed green.docs/regenerated withpnpm docs:gen(tests/codemap-fresh.test.tscaught the staleness).Related
forge/cursorto the canonical enum. When that publishes, theExtrasAlreadyCanonicalguard here fails the build, andEXTRA_HARNESSESgets emptied.fix/single-harness-enum) — the same derivation forBACKEND_TYPE.