Skip to content

feat(acp): bring your own harness (BYOH) — generic ACP runtime seam + settings gallery#2773

Draft
wpfleger96 wants to merge 3 commits into
mainfrom
duncan/byoh-generic-acp-harness
Draft

feat(acp): bring your own harness (BYOH) — generic ACP runtime seam + settings gallery#2773
wpfleger96 wants to merge 3 commits into
mainfrom
duncan/byoh-generic-acp-harness

Conversation

@wpfleger96

@wpfleger96 wpfleger96 commented Jul 24, 2026

Copy link
Copy Markdown
Member

What

Implements a "bring your own harness" (BYOH) generic ACP mechanism — replacing per-harness backend code with a data-driven 3-tier system:

  • Tier 1 (compiled-in builtins): goose, claude, codex, buzz-agent — unchanged behavior
  • Tier 2 (bundled presets): cursor, amp, hermes, openclaw, pi, and any future additions — defined in PRESET_HARNESSES, no code duplication, icons stay TerminalSquare/bundled-asset-only
  • Tier 3 (user-defined custom): JSON definitions saved to custom_harnesses/ under app data; managed via Settings → Agents UI

Changes

Core data model

  • HarnessDefinition — id, label, command, args, env, install URL/hint
  • PRESET_HARNESSES static table — single source of truth for all presets; preset_harness_ids() derives reserved IDs (D-11: no hand-maintained copy)
  • source: "builtin" | "preset" | "custom" tagging on every catalog entry

Persistence (B-4, B-6)

  • save_custom_harness_to_dir(dir, definition, rename_old_id) — backup-swap atomic write (backs up target → .bak, commits temp → target, restores .bak on failure, removes .bak on success); safe on Windows where fs::rename over an existing file is "access denied"
  • save_and_warm / delete_and_warm — hold PERSIST_MUTEX for the write + registry-warm pair, eliminating the lost-update race (B-6) where two concurrent saves could interleave their warm calls and leave a stale registry snapshot
  • Validate-before-mutate: both IDs and env validated before any filesystem mutation

Env validation boundary (B-3)

  • validate_harness_definition_pub calls validate_user_env_keys on definition env at save AND load
  • Rejects malformed keys (BUZZ_AUTH_TAG=x forgery shape), reserved keys (BUZZ_MANAGED_AGENT etc.), NUL bytes, oversized values

TypeScript boundary (B-2 / Thufir CRITICAL)

  • RawAcpRuntimeCatalogEntry now declares definition_env?: Record<string,string> and source: "builtin" | "preset" | "custom"
  • fromRawAcpRuntimeCatalogEntry maps definition_env → definitionEnv (camelCase); absent field defaults to {}
  • Edit form reads entry.definitionEnv — env no longer erased on save-then-edit cycle

Unified descriptor (Phase A / Thufir F4)

  • EffectiveHarnessDescriptor { command, args, env } in readiness.rs
  • resolve_effective_harness_descriptor() — single resolver used by spawn, spawn_hash, summary, get_agent_models (both saved and unsaved), and readiness
  • No competing arg-resolution forms

Other fixes

  • B-5: stop freezing runtime.defaultArgs into record.agent_args on normal create paths
  • B-7: readiness exec-check — MissingBinary variant for custom commands not found on PATH
  • B-8: onboarding transition — setTimeout(0) removed, parent-owned route intent via navigateAfterComplete prop
  • C-9: collector-discriminating sweep tests with injectable filters
  • C-10: HarnessManagementCard uses harnessGalleryLogic helpers (killed duplicate filter/sort)
  • D-11: BUILTIN_IDS derived from PRESET_HARNESSES (no hand-maintained copy)
  • D-12: mobile/pubspec.lock churn reverted
  • D-13: false ownership fast-path comment fixed
  • D-14: URL scheme validation for installInstructionsUrl
  • D-15: OpenClaw Gateway env-locus README line

Tests added

B-4 persistence (6 tests): save_to_dir_create_writes_file_and_loads_back, save_to_dir_same_id_edit_replaces_content, save_to_dir_backup_is_cleaned_up_after_same_id_edit, save_to_dir_rename_removes_old_file_and_creates_new, save_to_dir_rename_nonexistent_old_id_is_non_fatal, save_to_dir_roundtrip_with_env_preserves_values

B-3 env validation (6 tests): validate_rejects_malformed_key_with_equals_sign, validate_rejects_reserved_key_buzz_managed_agent, validate_rejects_reserved_key_case_insensitive, validate_rejects_nul_byte_in_value, validate_rejects_value_over_per_value_size_limit, validate_accepts_well_formed_env

B-2 API boundary (4 TS tests in tauri.test.mjs): fromRawAcpRuntimeCatalogEntry maps definition_env to definitionEnv, defaults definitionEnv to {} when absent, preserves source preset, env round-trips through edit payload shape

Preset catalog

ID Label Command
cursor Cursor cursor
amp Amp amp
hermes Hermes Agent hermes-agent
openclaw OpenClaw openclaw
pi Pi pi-agent
codestory Code Story Aide aide

Gate table — head f7c55505e

Gate Result
cargo test --lib 1691 passed, 0 failed, 14 ignored
just desktop-test 3531 passed, 0 failed
just desktop-typecheck clean
just desktop-check clean
just desktop-tauri-clippy clean
just desktop-build clean

PR head: f7c55505e66f8f8634d997f54c022f3fbe68e417 — MERGEABLE

@wpfleger96
wpfleger96 requested a review from a team as a code owner July 24, 2026 22:01
@wpfleger96
wpfleger96 marked this pull request as draft July 24, 2026 23:16
@wpfleger96
wpfleger96 force-pushed the duncan/byoh-generic-acp-harness branch 2 times, most recently from bda1871 to fa4034b Compare July 25, 2026 02:55
@wpfleger96
wpfleger96 force-pushed the duncan/byoh-generic-acp-harness branch from fa4034b to 5ebabda Compare July 25, 2026 05:16
Implement a generic BYOH mechanism that lets any ACP-speaking harness
(Cursor, Pi, Amp, OpenClaw, Hermes, etc.) work with Buzz without
maintaining separate backend code per harness type.

Backend (Rust):
- Add HarnessDefinition, HarnessSource, PRESET_HARNESSES (8 presets:
  cursor/omp/grok/opencode/kimi/amp/hermes/openclaw) + custom harness
  loading from ~/.config/buzz/custom-harnesses/
- warm_harness_registry_from_dir called at startup (before restore) and
  transactionally on save/delete; try_record_agent_command returns typed
  DANGLING_HARNESS_ID:<id> error — never silently falls to default
- Full descriptor resolved at spawn/readiness/spawn_hash paths; definition
  env merges below Buzz-reserved vars; edit round-trip carries definition_env
  in catalog entry
- All sweep paths (reap_dead_instance_agents, kill_stale_tracked_processes,
  receipt cleanup) use marker/receipt ownership — no name-gating
- save_custom_harness validates before mutation; atomic write + old-file
  delete on rename; preset ids reserved in check_id_collision
- buzz_sweep_owns_process cross-platform (no #[cfg(unix)] guard)

Frontend (TypeScript):
- Settings > Agents BYOH gallery: preset cards (Detected/Not-installed +
  docs link, no Add button), custom cards (full edit/delete)
- ArgsEditor (repeatable rows) + EnvEditor (KEY=VALUE pairs)
- HarnessManagementCard + harnessFormLogic.ts (21 behavior tests)
- harnessGalleryLogic.ts (11 tests: detected-first sort, preset filtering)
- Onboarding: actionable 'More harnesses...' button → Settings > Agents
- PRESET_LOGOS bundled map; avatarUrl stripped from all surfaces

Tests: +27 frontend / +20 Rust lib (registry lifecycle, env round-trip,
sweep ownership, legacy-JSON serde, preset collision)

Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
@wpfleger96
wpfleger96 force-pushed the duncan/byoh-generic-acp-harness branch from 5ebabda to 277d8b0 Compare July 25, 2026 05:20
npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7 and others added 2 commits July 25, 2026 01:36
…safe registry

B-4 (persistence contract): Replace the two simulated round-trip tests that
called raw fs::write/remove_file with six tests that exercise the real
save_custom_harness_to_dir helper directly:
- save_to_dir_create_writes_file_and_loads_back
- save_to_dir_same_id_edit_replaces_content
- save_to_dir_backup_is_cleaned_up_after_same_id_edit
- save_to_dir_rename_removes_old_file_and_creates_new
- save_to_dir_rename_nonexistent_old_id_is_non_fatal
- save_to_dir_roundtrip_with_env_preserves_values

B-3 (env validation boundary): Six tests exercising validate_harness_definition_pub
integration with validate_user_env_keys for the documented attack surfaces:
- validate_rejects_malformed_key_with_equals_sign (BUZZ_AUTH_TAG=x forgery shape)
- validate_rejects_reserved_key_buzz_managed_agent (ownership marker)
- validate_rejects_reserved_key_case_insensitive (lowercase bypass)
- validate_rejects_nul_byte_in_value (Command::env panic protection)
- validate_rejects_value_over_per_value_size_limit
- validate_accepts_well_formed_env

B-2 (API boundary): Export fromRawAcpRuntimeCatalogEntry from tauri.ts and
add four tests to tauri.test.mjs proving the Rust definition_env snake_case
field is mapped to definitionEnv camelCase, that absent definition_env defaults
to {} (not undefined), and that env round-trips end-to-end through the mapper
so a save-then-edit cycle cannot erase definition env.

B-6 (concurrent-safe registry): Add save_and_warm + delete_and_warm functions
in custom_harnesses.rs that hold a PERSIST_MUTEX for the filesystem mutation
and the subsequent warm_harness_registry_from_dir call as an atomic unit.
Update save_custom_harness and delete_custom_harness Tauri commands to use
these helpers. Eliminates the lost-update window where two concurrent saves
could interleave their warm calls and produce a stale registry snapshot.

File-size override added for custom_harnesses.rs (1042 lines after the new
tests; queued to split once the feature stabilizes).

Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
D-12: Revert mobile/pubspec.lock to origin/main.  The meta 1.17→1.18 /
test 1.30→1.31 churn was a no-source-change lockfile bump introduced by
accident; reverting eliminates the unrelated diff.

D-15: Add OpenClaw execution-locus note to the preset's install_hint.
Eva's finding: openclaw acp executes tools inside the Gateway daemon, not
in the Desktop process.  Desktop-injected BUZZ_* env vars reach the
openclaw harness process itself but do NOT automatically propagate to
the Gateway's execution environment.  The install_hint now surfaces this
caveat so users who need BUZZ_* credentials at execution time know they
must set them on the Gateway's own environment.

F8 (onboarding navigate test): New pure-logic test file
postOnboardingNav.test.mjs proves the App.tsx postOnboardingNav
useEffect predicate — navigateAfterComplete does not fire before
machine.stage reaches 'ready', fires exactly once on the ready
transition, is cleared after firing (no double-fire), fires immediately
if nav arrives while already ready, and carries the exact
{to: '/settings', search: {section: 'agents'}} shape from
MachineOnboardingFlow's navigateToAgentSettings action.

C-10 (e2eBridge + handler tests): Extract save_custom_harness /
delete_custom_harness handler logic into e2eBridgeCustomHarnesses.ts
(exported module-level functions + mockCustomHarnesses Map +
resetMockCustomHarnesses).  Wire the handlers into e2eBridge.ts:
  - mockCustomHarnesses imported and appended to discover_acp_providers
    results so saved custom harnesses appear in future discovery calls
  - case 'save_custom_harness' → handleSaveCustomHarness
  - case 'delete_custom_harness' → handleDeleteCustomHarness
New test file e2eBridgeCustomHarnesses.test.mjs (14 tests across 3
describe groups) proves: save → store, non-empty env preserved,
empty env absent, same-ID edit replaces, rename removes old + inserts
new, delete removes, delete idempotent, Map reference is shared.

Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
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.

1 participant