Files
buzz/desktop/src/features/agents/lib/instanceInputForDefinition.test.mjs
Will Pfleger 95fdf97880 feat(acp): bring your own harness (BYOH) — generic ACP runtime seam + settings gallery (#2773)
## 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, omp, grok, opencode, kimi, amp,
hermes, openclaw, 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-agent acp` |
| `omp` | Oh My Pi | `omp acp` |
| `grok` | Grok Build | `grok agent --always-approve stdio` |
| `opencode` | OpenCode | `opencode acp` |
| `kimi` | Kimi Code | `kimi acp` |
| `amp` | Amp | `amp-acp` |
| `hermes` | Hermes Agent | `hermes-acp` |
| `openclaw` | OpenClaw | `openclaw acp` |

## Review-fix pass (2026-07-26, Eva)

Fixes from the three-way review (Wren / Dawn / Eva) in the
buzz-generic-acp-harnesses thread, pushed as new commits (no rewrite):

1. **installHint edit round-trip** — form seeding extracted to
`formValuesFromCatalogEntry` (single source of truth), input rendered,
full-definition lossless round-trip regression.
2. **Dangling-delete coherence** — delete allowed; confirm counts
referencing agents (direct pin + persona-inherited); summary rows render
`harness (deleted): <id>`; spawn errors become actionable sentences
(`user_facing_harness_error`); composed delete→summary→start test.
3. **Comma-in-args** — rejected at `validate_harness_definition` (shared
by save AND disk load), mirrored inline in the form.
4. **Registry publish race** — collision/dup filtering moved into
`load_custom_harnesses` (both loaders inherit shadowing rules);
discovery publishes by re-reading the dir under `persist_mutex` (lock
scoped to publish only); deterministic interleaving regressions for
save-during-discovery and delete-during-discovery.
5. **Mechanical** — discarded `belongs_to_us` sweep arg deleted,
`load_global_agent_config` hoisted out of the per-record summary loop,
duplicated doc paragraph + stray SAFETY comment removed.
6. **PGID test de-flaked** — leader kept alive through the assertion.

Known follow-up (filed in review, not blocking): file-size split-outs
queued in `check-file-sizes.mjs` entries.

## Gate table — head `bf53f1d60`

| Gate | Result |
|------|--------|
| `cargo test --lib` (desktop/src-tauri) | **1701 passed**, 0 failed, 14
ignored |
| desktop JS suite (`pnpm test`) | **3605 passed**, 0 failed |
| `tsc --noEmit` | clean |
| `biome check` + file-size/px/pubkey checks | clean |
| `cargo clippy --lib -- -D warnings` | clean |
| `cargo fmt --check` | clean |

PR head: `bf53f1d60e3cbd07392e1287b83bb37ba90d0d33` — includes merge of
origin/main (`c2a4ee711`, conflicts in agent_models composed with
#2890's live Databricks discovery)

---------

Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: tlongwell-block <109685178+tlongwell-block@users.noreply.github.com>
Signed-off-by: Tyler Longwell <tlongwell@block.xyz>
Co-authored-by: npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7 <dcfd242e557282d7a1e2cf2e6877522682f1e5c6156dc92ca7d90eaedd3b0f95@buzz.block.builderlab.xyz>
Co-authored-by: tlongwell-block <109685178+tlongwell-block@users.noreply.github.com>
Co-authored-by: Dawn (sprout agent) <c6237ef84fa537c78dcee78efd2d4e59f728859c7f194da42ac51ededfa0be05@sprout-oss.stage.blox.sqprod.co>
Co-authored-by: Tyler Longwell <tlongwell@block.xyz>
Co-authored-by: npub1qyvc0c5kl4gqv2fd97fsk46tu378sqgy35vc83rvgfwne90sel7s0ed67d <011987e296fd5006292d2f930b574be47c7801048d1983c46c425d3c95f0cffd@buzz.block.builderlab.xyz>
2026-07-26 20:06:20 -04:00

337 lines
11 KiB
JavaScript

import assert from "node:assert/strict";
import test from "node:test";
import {
availableRuntimesForStart,
buildInstanceInputForDefinition,
resolveStartRuntimeForDefinition,
} from "./instanceInputForDefinition.ts";
// ── Phase 1B.3.5: the single definition→instance mapping ────────────────────
//
// Every surface that starts an agent from a definition maps through
// buildInstanceInputForDefinition + resolveStartRuntimeForDefinition +
// availableRuntimesForStart. These tests pin the decided rows:
// row 1: refuse (actionable error) when the configured runtime is missing
// row 2: harnessOverride = !persona.runtime || persona.runtime === runtime.id
// row 3: avatar through resolveManagedAgentAvatarUrl (injectable upload)
// row 4: create input NEVER contains definition env vars
// row 6: runtime list acquisition is refetch-aware
const gooseRuntime = {
id: "goose",
label: "Goose",
avatarUrl: "https://runtime/goose.png",
availability: "available",
command: "goose-cmd",
binaryPath: "/bin/goose",
defaultArgs: ["--acp"],
mcpCommand: "goose-mcp",
installHint: "",
installInstructionsUrl: "",
canAutoInstall: false,
underlyingCliPath: null,
};
const claudeRuntime = {
...gooseRuntime,
id: "claude",
label: "Claude",
command: "claude-cmd",
mcpCommand: null,
};
const buzzAgentRuntime = {
...gooseRuntime,
id: "buzz-agent",
label: "Buzz Agent",
command: "buzz-agent-cmd",
mcpCommand: null,
};
function persona(overrides = {}) {
return {
id: "p-1",
displayName: "Test Agent",
systemPrompt: "prompt",
model: null,
runtime: "goose",
avatarUrl: "https://example.com/a.png",
envVars: { ANTHROPIC_API_KEY: "persona-secret" },
isBuiltIn: false,
...overrides,
};
}
test("row 4: create input never contains definition env vars", async () => {
const input = await buildInstanceInputForDefinition(persona(), gooseRuntime);
assert.equal(
"envVars" in input,
false,
"definition env must never be seeded into the create input — " +
"record.env_vars is overrides-only and spawn merges the live definition env",
);
});
test("row 2: harnessOverride follows the backend-aligned formula", async () => {
const match = await buildInstanceInputForDefinition(
persona({ runtime: "goose" }),
gooseRuntime,
);
assert.equal(match.harnessOverride, true, "picked == configured → true");
const noPreference = await buildInstanceInputForDefinition(
persona({ runtime: undefined }),
gooseRuntime,
);
assert.equal(noPreference.harnessOverride, true, "no preference → true");
const differs = await buildInstanceInputForDefinition(
persona({ runtime: "claude" }),
gooseRuntime,
);
assert.equal(
differs.harnessOverride,
false,
"picked != configured → false (definition stays authoritative)",
);
});
test("row 3: plain avatar URLs pass through; base64 data URIs upload via the injectable", async () => {
const plain = await buildInstanceInputForDefinition(persona(), gooseRuntime);
assert.equal(plain.avatarUrl, "https://example.com/a.png");
const uploads = [];
const uploaded = await buildInstanceInputForDefinition(
persona({ avatarUrl: "data:image/png;base64,aGk=" }),
gooseRuntime,
async (bytes) => {
uploads.push(bytes);
return {
url: "https://cdn/blob.png",
sha256: "x",
size: 2,
type: "image/png",
uploaded: 0,
};
},
);
assert.equal(uploaded.avatarUrl, "https://cdn/blob.png");
assert.equal(uploads.length, 1, "upload must go through the injected fn");
});
test("row 3: failed persona avatar upload never substitutes the runtime avatar", async () => {
const input = await buildInstanceInputForDefinition(
persona({
id: "builtin:fizz",
displayName: "Fizz",
avatarUrl: "data:image/png;base64,aGk=",
}),
claudeRuntime,
async () => {
throw new Error("upload failed");
},
);
assert.equal(input.avatarUrl, undefined);
assert.notEqual(input.avatarUrl, claudeRuntime.avatarUrl);
});
test("mapping carries the runtime and definition fields", async () => {
const input = await buildInstanceInputForDefinition(persona(), gooseRuntime);
assert.equal(input.name, "Test Agent");
assert.equal(input.acpCommand, "buzz-acp");
assert.equal(input.agentCommand, "goose-cmd");
// B-5: agentArgs is intentionally empty at create time — spawn reads args
// live from the definition on every start so definition edits take effect
// without recreating the agent. Seeding from runtime.defaultArgs here would
// freeze args at create-time and silently ignore later definition edits.
assert.deepEqual(input.agentArgs, []);
assert.equal(input.mcpCommand, "goose-mcp");
assert.equal(input.personaId, "p-1");
assert.equal(input.systemPrompt, "prompt");
assert.equal(input.model, undefined);
assert.equal(input.provider, undefined);
assert.equal(input.spawnAfterCreate, true);
assert.equal(input.startOnAppLaunch, true);
assert.deepEqual(input.backend, { type: "local" });
});
test("no backend intent is byte-identical to the pre-intent mapping", async () => {
// The 3 pre-B5 call sites (useManagedAgentActions, usePersonaActions,
// UserProfilePanel) pass no intent; their output must not move.
// B-5: agentArgs is [] — args are NOT seeded from the definition at create
// time. Spawn reads live args from the definition on every start.
const input = await buildInstanceInputForDefinition(persona(), gooseRuntime);
assert.deepEqual(input, {
name: "Test Agent",
personaId: "p-1",
systemPrompt: "prompt",
avatarUrl: "https://example.com/a.png",
acpCommand: "buzz-acp",
agentCommand: "goose-cmd",
agentArgs: [],
mcpCommand: "goose-mcp",
harnessOverride: true,
model: undefined,
provider: undefined,
spawnAfterCreate: true,
startOnAppLaunch: true,
backend: { type: "local" },
});
});
test("Buzz shared compute definition carries native provider and auto model", async () => {
const input = await buildInstanceInputForDefinition(
persona({
runtime: "buzz-agent",
provider: "relay-mesh",
model: "auto",
}),
{ ...gooseRuntime, id: "buzz-agent", command: "buzz-agent" },
);
assert.equal(input.agentCommand, "buzz-agent");
assert.equal(input.provider, "relay-mesh");
assert.equal(input.model, "auto");
assert.equal(input.spawnAfterCreate, true);
assert.equal(input.startOnAppLaunch, true);
});
test("provider intent forces startOnAppLaunch off and omits local commands", async () => {
const input = await buildInstanceInputForDefinition(
persona(),
gooseRuntime,
undefined,
{ type: "provider", id: "blox", config: { region: "us" } },
);
assert.deepEqual(input.backend, {
type: "provider",
id: "blox",
config: { region: "us" },
});
assert.equal(input.startOnAppLaunch, false, "remote agents never auto-start");
assert.equal(input.spawnAfterCreate, true);
assert.equal(input.harnessOverride, false);
// Provider agents spawn no local ACP — the legacy provider branch omitted
// all local commands and model/provider, and so does the intent path.
for (const key of [
"acpCommand",
"agentCommand",
"agentArgs",
"mcpCommand",
"model",
"provider",
"envVars",
"relayMesh",
]) {
assert.equal(key in input, false, `provider intent must omit ${key}`);
}
assert.equal(input.personaId, "p-1", "definition link is kept");
assert.equal(input.systemPrompt, "prompt");
});
test("row 1: refuses when the configured runtime is not available", () => {
assert.throws(
() =>
resolveStartRuntimeForDefinition(persona({ runtime: "missing" }), [
gooseRuntime,
claudeRuntime,
]),
/not available|No available runtime/i,
"configured-but-missing runtime must refuse, never silently fall back",
);
});
test("row 1: resolves the configured runtime when available", () => {
const { runtime, warnings } = resolveStartRuntimeForDefinition(
persona({ runtime: "claude" }),
[gooseRuntime, claudeRuntime],
);
assert.equal(runtime.id, "claude");
assert.deepEqual(warnings, []);
});
test("row 1: no preference resolves the default with no warnings", () => {
const { runtime, warnings } = resolveStartRuntimeForDefinition(
persona({ runtime: undefined }),
[gooseRuntime, claudeRuntime],
);
assert.equal(runtime.id, "goose");
assert.deepEqual(warnings, []);
});
test("row 1: refuses when no runtimes exist at all", () => {
assert.throws(
() => resolveStartRuntimeForDefinition(persona({ runtime: undefined }), []),
/No available runtime/,
);
});
test("row 6: fetched query uses cached data without refetching", async () => {
let refetched = false;
const runtimes = await availableRuntimesForStart({
isFetched: true,
data: [gooseRuntime, { ...claudeRuntime, availability: "missing" }],
refetch: async () => {
refetched = true;
return { data: [] };
},
});
assert.equal(refetched, false);
assert.deepEqual(
runtimes.map((r) => r.id),
["goose"],
"unavailable runtimes are filtered out",
);
});
test("row 6: unfetched query refetches instead of resolving empty", async () => {
const runtimes = await availableRuntimesForStart({
isFetched: false,
data: undefined,
refetch: async () => ({ data: [claudeRuntime] }),
});
assert.deepEqual(
runtimes.map((r) => r.id),
["claude"],
"an unfetched query must fetch, not spuriously report no runtimes",
);
});
// ── item-13 regression: buzz-agent-first default runtime ─────────────────────
//
// Before this fix, resolveStartRuntimeForDefinition used runtimes[0] (catalog
// order: goose, claude, codex, buzz-agent), so an installed goose would beat
// the bundled buzz-agent sidecar as the default for runtime-less personas.
// The fix applies the preference order: buzz-agent → goose → first available.
test("item-13: goose+buzz-agent both available — persona with no runtime resolves buzz-agent", () => {
const { runtime, warnings } = resolveStartRuntimeForDefinition(
persona({ runtime: undefined }),
[gooseRuntime, claudeRuntime, buzzAgentRuntime],
);
assert.equal(
runtime.id,
"buzz-agent",
"buzz-agent must win over catalog-first goose for runtime-less personas",
);
assert.deepEqual(warnings, []);
});
test("item-13: goose-only available — persona with no runtime resolves goose", () => {
const { runtime, warnings } = resolveStartRuntimeForDefinition(
persona({ runtime: undefined }),
[gooseRuntime, claudeRuntime],
);
assert.equal(runtime.id, "goose");
assert.deepEqual(warnings, []);
});
test("item-13: no runtimes available — refuses with actionable error", () => {
assert.throws(
() => resolveStartRuntimeForDefinition(persona({ runtime: undefined }), []),
/No available runtime/,
"empty runtime list must throw, not silently return null",
);
});