mirror of
https://github.com/block/buzz.git
synced 2026-08-18 06:50:31 +02:00
## 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>
247 lines
8.6 KiB
JavaScript
247 lines
8.6 KiB
JavaScript
/**
|
|
* Unit tests for the e2eBridge custom harness handlers (C-10).
|
|
*
|
|
* Tests are imported directly from the extracted e2eBridgeCustomHarnesses.ts
|
|
* module — no Tauri mock or browser environment needed — to prove:
|
|
*
|
|
* 1. save returns a catalog entry with source "custom" and correct fields
|
|
* 2. definition_env is preserved through save (non-empty env)
|
|
* 3. empty env produces absent definition_env (mirrors Rust BTreeMap serialization)
|
|
* 4. same-ID edit replaces the existing entry in the store (no duplicates)
|
|
* 5. rename (originalId ≠ id) removes the old key and inserts the new one
|
|
* 6. delete removes the entry from the store
|
|
* 7. delete is idempotent (not-found does not throw)
|
|
* 8. discover integration: saved harness appears in the discover result set
|
|
* alongside the default catalog (verifies the Map is shared by reference)
|
|
*/
|
|
import assert from "node:assert/strict";
|
|
import { beforeEach, describe, it } from "node:test";
|
|
|
|
import {
|
|
mockCustomHarnesses,
|
|
mergeMockCustomHarnesses,
|
|
resetMockCustomHarnesses,
|
|
handleSaveCustomHarness,
|
|
handleDeleteCustomHarness,
|
|
} from "./e2eBridgeCustomHarnesses.ts";
|
|
|
|
function makeArgs(overrides = {}) {
|
|
return {
|
|
definition: {
|
|
id: overrides.id ?? "test-harness",
|
|
label: overrides.label ?? "Test Harness",
|
|
command: overrides.command ?? "test-bin",
|
|
args: overrides.args ?? [],
|
|
env: overrides.env ?? {},
|
|
installInstructionsUrl: overrides.installInstructionsUrl ?? "",
|
|
installHint: overrides.installHint ?? "",
|
|
},
|
|
originalId: overrides.originalId ?? null,
|
|
};
|
|
}
|
|
|
|
// Reset the store before every test so tests are independent.
|
|
beforeEach(() => resetMockCustomHarnesses());
|
|
|
|
// ── save_custom_harness ───────────────────────────────────────────────────────
|
|
|
|
describe("handleSaveCustomHarness", () => {
|
|
it("returns a catalog entry with source 'custom'", () => {
|
|
const entry = handleSaveCustomHarness(
|
|
makeArgs({ id: "my-rt", label: "My RT" }),
|
|
);
|
|
assert.equal(entry.id, "my-rt");
|
|
assert.equal(entry.label, "My RT");
|
|
assert.equal(entry.source, "custom");
|
|
});
|
|
|
|
it("stores the entry in mockCustomHarnesses", () => {
|
|
handleSaveCustomHarness(makeArgs({ id: "stored" }));
|
|
assert.ok(
|
|
mockCustomHarnesses.has("stored"),
|
|
"store must contain the saved id",
|
|
);
|
|
});
|
|
|
|
it("preserves non-empty definition_env", () => {
|
|
const env = { ANTHROPIC_API_KEY: "sk-test", MODEL: "claude-3" };
|
|
const entry = handleSaveCustomHarness(makeArgs({ id: "env-rt", env }));
|
|
assert.deepStrictEqual(entry.definition_env, env);
|
|
assert.deepStrictEqual(
|
|
mockCustomHarnesses.get("env-rt")?.definition_env,
|
|
env,
|
|
);
|
|
});
|
|
|
|
it("produces absent definition_env for empty env (mirrors Rust BTreeMap skip)", () => {
|
|
const entry = handleSaveCustomHarness(makeArgs({ id: "no-env", env: {} }));
|
|
assert.ok(
|
|
entry.definition_env === undefined || entry.definition_env === null,
|
|
"empty env must yield absent definition_env",
|
|
);
|
|
});
|
|
|
|
it("same-ID edit replaces entry — no duplicates in the store", () => {
|
|
handleSaveCustomHarness(makeArgs({ id: "dup", label: "V1" }));
|
|
handleSaveCustomHarness(
|
|
makeArgs({ id: "dup", label: "V2", originalId: "dup" }),
|
|
);
|
|
assert.equal(
|
|
mockCustomHarnesses.size,
|
|
1,
|
|
"same-ID edit must not duplicate store entries",
|
|
);
|
|
assert.equal(mockCustomHarnesses.get("dup")?.label, "V2");
|
|
});
|
|
|
|
it("rename removes old key and inserts new key", () => {
|
|
handleSaveCustomHarness(makeArgs({ id: "old-rt", label: "Old" }));
|
|
handleSaveCustomHarness(
|
|
makeArgs({ id: "new-rt", label: "New", originalId: "old-rt" }),
|
|
);
|
|
assert.ok(
|
|
!mockCustomHarnesses.has("old-rt"),
|
|
"old key must be removed on rename",
|
|
);
|
|
assert.ok(
|
|
mockCustomHarnesses.has("new-rt"),
|
|
"new key must be present after rename",
|
|
);
|
|
assert.equal(mockCustomHarnesses.get("new-rt")?.label, "New");
|
|
});
|
|
});
|
|
|
|
// ── delete_custom_harness ────────────────────────────────────────────────────
|
|
|
|
describe("handleDeleteCustomHarness", () => {
|
|
it("removes an existing entry from the store", () => {
|
|
handleSaveCustomHarness(makeArgs({ id: "to-delete" }));
|
|
assert.ok(mockCustomHarnesses.has("to-delete"));
|
|
|
|
handleDeleteCustomHarness({ id: "to-delete" });
|
|
assert.ok(
|
|
!mockCustomHarnesses.has("to-delete"),
|
|
"entry must be removed after delete",
|
|
);
|
|
});
|
|
|
|
it("is idempotent — deleting non-existent id does not throw", () => {
|
|
assert.doesNotThrow(
|
|
() => handleDeleteCustomHarness({ id: "never-existed" }),
|
|
"delete of non-existent id must not throw",
|
|
);
|
|
});
|
|
|
|
it("throws when deleteCustomHarnessError knob is set", () => {
|
|
handleSaveCustomHarness(makeArgs({ id: "keep-alive" }));
|
|
const config = { mock: { deleteCustomHarnessError: "permission denied" } };
|
|
assert.throws(
|
|
() => handleDeleteCustomHarness({ id: "keep-alive" }, config),
|
|
/permission denied/,
|
|
"must throw the injected error message",
|
|
);
|
|
// Entry must remain in store — the error means delete did not complete.
|
|
assert.ok(
|
|
mockCustomHarnesses.has("keep-alive"),
|
|
"entry must remain when delete throws",
|
|
);
|
|
});
|
|
});
|
|
|
|
// ── discover integration: store is shared by reference ───────────────────────
|
|
|
|
describe("mergeMockCustomHarnesses", () => {
|
|
const seeded = (id, label = id) => ({ id, label, source: "custom" });
|
|
|
|
it("appends a newly saved harness that is not in the seeded catalog", () => {
|
|
handleSaveCustomHarness(makeArgs({ id: "added", label: "Added" }));
|
|
const merged = mergeMockCustomHarnesses([seeded("preset-a")]);
|
|
assert.deepEqual(
|
|
merged.map((e) => e.id),
|
|
["preset-a", "added"],
|
|
);
|
|
});
|
|
|
|
it("replaces a seeded entry in place on same-id save — no duplicate row", () => {
|
|
handleSaveCustomHarness(makeArgs({ id: "seeded-one", label: "V2" }));
|
|
const merged = mergeMockCustomHarnesses([
|
|
seeded("preset-a"),
|
|
seeded("seeded-one", "V1"),
|
|
]);
|
|
assert.deepEqual(
|
|
merged.map((e) => e.id),
|
|
["preset-a", "seeded-one"],
|
|
"must not duplicate the id",
|
|
);
|
|
assert.equal(
|
|
merged.find((e) => e.id === "seeded-one").label,
|
|
"V2",
|
|
"saved entry must win over the seed",
|
|
);
|
|
});
|
|
|
|
it("drops a deleted seeded entry (tombstone, not just store removal)", () => {
|
|
// The regression: a seeded row has no store entry to delete, so without a
|
|
// tombstone the row survived the delete and the spec failed.
|
|
handleDeleteCustomHarness({ id: "seeded-one" });
|
|
const merged = mergeMockCustomHarnesses([
|
|
seeded("preset-a"),
|
|
seeded("seeded-one"),
|
|
]);
|
|
assert.deepEqual(
|
|
merged.map((e) => e.id),
|
|
["preset-a"],
|
|
);
|
|
});
|
|
|
|
it("drops the vacated old id after a rename and surfaces the new one", () => {
|
|
handleSaveCustomHarness(
|
|
makeArgs({ id: "new-id", label: "New", originalId: "old-id" }),
|
|
);
|
|
const merged = mergeMockCustomHarnesses([seeded("old-id", "Old")]);
|
|
assert.deepEqual(
|
|
merged.map((e) => e.id),
|
|
["new-id"],
|
|
);
|
|
});
|
|
|
|
it("re-saving a deleted id resurrects it", () => {
|
|
handleDeleteCustomHarness({ id: "seeded-one" });
|
|
handleSaveCustomHarness(makeArgs({ id: "seeded-one", label: "Back" }));
|
|
const merged = mergeMockCustomHarnesses([seeded("seeded-one", "Original")]);
|
|
assert.deepEqual(
|
|
merged.map((e) => e.id),
|
|
["seeded-one"],
|
|
);
|
|
assert.equal(merged[0].label, "Back");
|
|
});
|
|
|
|
it("leaves a seeded catalog untouched when nothing has been mutated", () => {
|
|
const base = [seeded("preset-a"), seeded("preset-b")];
|
|
assert.deepEqual(
|
|
mergeMockCustomHarnesses(base).map((e) => e.id),
|
|
["preset-a", "preset-b"],
|
|
);
|
|
});
|
|
});
|
|
|
|
describe("mockCustomHarnesses Map reference", () => {
|
|
it("handler writes are immediately visible to callers that read the exported Map", () => {
|
|
// handleDiscoverAcpRuntimes merges this store into the catalog it returns.
|
|
// The exported Map is the same object by reference, so writes via the
|
|
// handler are visible to any reader of the Map.
|
|
assert.equal(
|
|
mockCustomHarnesses.size,
|
|
0,
|
|
"store must be empty after reset",
|
|
);
|
|
|
|
handleSaveCustomHarness(makeArgs({ id: "visible", label: "Visible" }));
|
|
assert.equal(mockCustomHarnesses.size, 1);
|
|
|
|
const [entry] = Array.from(mockCustomHarnesses.values());
|
|
assert.equal(entry.id, "visible");
|
|
assert.equal(entry.source, "custom");
|
|
});
|
|
});
|