Skip to content

fix(harness): derive the harness taxonomy from the canonical enum - #357

Merged
drewstone merged 1 commit into
mainfrom
fix/harness-taxonomy-derive
Jul 31, 2026
Merged

fix(harness): derive the harness taxonomy from the canonical enum#357
drewstone merged 1 commit into
mainfrom
fix/harness-taxonomy-derive

Conversation

@drewstone

Copy link
Copy Markdown
Contributor

Problem

src/harness/index.ts kept a hand-maintained copy of the harness taxonomy, and its header claimed:

agent-app's Harness taxonomy is a superset of agent-interface's HarnessType (it carries forge/cursor, which agent-interface doesn't list).

That was false. KNOWN_HARNESSES omitted gemini, which HarnessType does list — so the two sets overlap, they do not nest. Three as HarnessType casts (:126, :133, :139) rested on that claim, plus an as Harness on the return of snapHarnessToModel.

This was one of four enums describing the same concept across the org (agent-interface HarnessType 13, agent-dev-container BACKEND_TYPE 14, this one 14, MATERIALIZER_HARNESSES 9).

Change

Derive KNOWN_HARNESSES from harnessTypeSchema.options, with two explicitly named lists:

  • NON_BACKEND_HARNESSES = ['gemini']KNOWN_HARNESSES is dispatched as backend.type (src/sandbox/index.ts:2013, :2253, :2641), and src/run/index.ts:9 states it "is exactly the set of sandbox backends". The platform ships no gemini backend (no sdk-provider-gemini in 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. snapHarnessToModel now 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 HarnessType cast leaves every assertion green, because forge/cursor have 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_HARNESSES order changes. It was opencode, 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_HARNESS is 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 = []):

FAIL  src/harness/index.test.ts > derives from the canonical harness enum, dropping only what has no backend
AssertionError: expected [] to deeply equal [ 'gemini' ]
      Tests  1 failed | 15 passed (16)

Restored → 16 passed.

Break 2 — restore the old cast (return aiSnapHarnessToModel(harness as HarnessType, modelId) as Harness):

      Tests  16 passed (16)

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 aiSnapHarnessToModel only ever returns claude-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):

gate baseline after
typecheck exit 0 exit 0
test 18 files / 156 failed, 3703 passed 18 files / 156 failed, 3706 passed

The failing set is byte-identical before and after (diff of the failing-file lists is empty). All 156 share one environmental cause — Error: Could not locate the bindings file (unbuilt native better-sqlite3 binding on this box) — across tests/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 with pnpm docs:gen (tests/codemap-fresh.test.ts caught the staleness).

Related

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 tangletools left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

✅ 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 tangletools left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟢 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_HARNESSES array with one derived at runtime from harnessTypeSchema.options (the canonical peer-dep enum), minus an explicitly named NON_BACKEND_HARNESSES = ['gemini'] (no platform provider adapter) plus an explicitly named EXTRA_HARNESSES = ['forge','cursor'] (real backends the canonical enum lacks). The three as HarnessType casts 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), so harnessSupportsModel(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/coerceHarness are consumed by src/run/index.ts:19,47 (execution-mode dispatch + isKnownSandboxHarness), src/skills-placement/index.ts:21,95 (skill-dir filtering), and assertHarnessModelCompatible by src/sandbox/index.ts:2211,2611 (server-side model guard before dispatch). KNOWN_HARNESSES is dispatched as backend.type (`src/sa
  • Fit with existing patterns: Exactly in-grain. AGENTS.md invariant 4 keeps /harness substrate-free (no sandbox SDK import), so it cannot read BackendType directly; deriving from @tangle-network/agent-interface's harnessTypeSchema (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. The gemini drop is the load-bearing correctness fix: offering it would have been fail-open, since KNOWN_HARNESSES is dispatched as backend.type onto a platform with no gemini adapter. The new snapHarnessToModel throw 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.

value-audit · 20260731T175109Z

@tangletools

Copy link
Copy Markdown

✅ No Blockers — 706f7f6e

Review health 100/100 · Reviewer score 80/100 · Confidence 80/100 · 8 findings (8 low)

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 bare Harness alias, but the Harness entry renders type Harness with no body and KNOWN_HARNESSES renders readonly 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 reference Harness with 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 for Harness, or add the concrete value list to the KNOWN_HARNESSES descripti

🟡 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 derives Harness = 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 that gemini was 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 because preferredHarnessForModel only returns claude-code/codex/kimi-code/null and the input canonical is never gemini (excluded by type), so aiSnapHarnessToModel cannot produce a non-Harness value today. A test that injects a fake aiSnapHarnessToModel returning '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 (_extrasAreNotYetCanonical forces a type error when forge/cursor enter HarnessType), but the drop direction has only the exact-match test expect([...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 a backend.type with 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 tangletools left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

✅ 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

@drewstone
drewstone merged commit d21dc9c into main Jul 31, 2026
1 check failed
@drewstone
drewstone deleted the fix/harness-taxonomy-derive branch July 31, 2026 18:06
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