From b0503d80c298b1ece3b0a43b41d316829a3379e7 Mon Sep 17 00:00:00 2001 From: Will Pfleger Date: Tue, 28 Jul 2026 16:01:41 -0400 Subject: [PATCH 01/11] feat(desktop): add custom harness inline from agent dialogs (#3252) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Registering a custom ACP harness works today, but only from Settings → Agents. Anyone whose first touchpoint is "New agent" has no way to discover the custom path — the dropdown just lists the baked-in presets plus whatever was registered earlier. This adds an inline "Add custom harness…" entry to the harness dropdown in all three agent surfaces: create, edit-definition (`AgentDefinitionDialog`), and instance edit (`AgentInstanceEditDialog`). The entry is a sentinel option (`ADD_CUSTOM_HARNESS_VALUE`, NUL-prefixed so it can never collide with a real harness id — backend ids match `[a-z0-9_][a-z0-9_-]*`), mirroring the `CUSTOM_ENTRY_ID` trick already used in `HarnessCatalogDialog`. Picking it never writes into form state; it opens `AddCustomHarnessDialog`, a thin modal wrapper hosting the existing `CustomHarnessForm` in `chromeless` mode. `CustomHarnessForm`'s `onSaved` now carries the saved `definition.id` (the form may rewrite it); the two existing call sites ignore the argument, so their behavior is unchanged. Selection after save is deferred rather than immediate. `usePendingHarnessSelection` holds the saved id until the runtime catalog actually publishes it via discovery, then selects it exactly once — so the dialog never selects an id it cannot render, and back-to-back registrations resolve correctly. The wait is scoped to the owning dialog's `open` state: both host dialogs stay mounted when closed, so an unpublished id is dropped on close rather than selecting into reset form state when discovery later catches up. Selection is routed through each dialog's normal dropdown change handler, so provider/model reset (and command pinning in the instance dialog) behave identically to a hand-picked harness. Dismissing the modal leaves the previous selection untouched. `AgentInstanceEditDialog`'s existing "Custom command" option is a different feature (ad-hoc command override vs. a registered reusable harness) and is untouched. Coverage is 16 unit tests in `addCustomHarness.test.mjs` (real React mount, following the existing `.test.mjs` pattern) plus 4 Playwright specs in `inline-custom-harness.spec.ts` covering all three surfaces end-to-end. Both suites were mutation-verified: treating the sentinel as a real selection, selecting before the catalog publishes, never clearing the pending id, ignoring the dialog's open state, and reversing latest-save-wins each turn the unit tests red; reverting the two dialog diffs turns all four e2e specs red. The `check-file-sizes.mjs` overrides for the two dialogs are ratcheted to their exact new counts (1048 and 1229) — verified tight in both directions, N passes and N−1 fails, so no headroom is introduced. --------- Signed-off-by: Will Pfleger --- desktop/playwright.config.ts | 1 + desktop/scripts/check-file-sizes.mjs | 10 +- .../agents/ui/AddCustomHarnessDialog.tsx | 47 +++ .../agents/ui/AgentDefinitionDialog.tsx | 32 +- .../agents/ui/AgentInstanceEditDialog.tsx | 32 +- .../agents/ui/addCustomHarness.test.mjs | 344 ++++++++++++++++++ .../features/agents/ui/addCustomHarness.ts | 104 ++++++ .../settings/ui/CustomHarnessForm.tsx | 11 +- .../tests/e2e/inline-custom-harness.spec.ts | 194 ++++++++++ 9 files changed, 763 insertions(+), 12 deletions(-) create mode 100644 desktop/src/features/agents/ui/AddCustomHarnessDialog.tsx create mode 100644 desktop/src/features/agents/ui/addCustomHarness.test.mjs create mode 100644 desktop/src/features/agents/ui/addCustomHarness.ts create mode 100644 desktop/tests/e2e/inline-custom-harness.spec.ts diff --git a/desktop/playwright.config.ts b/desktop/playwright.config.ts index 0d89b8e2d2..459fa75743 100644 --- a/desktop/playwright.config.ts +++ b/desktop/playwright.config.ts @@ -126,6 +126,7 @@ export default defineConfig({ "**/observer-archive-policy.spec.ts", "**/harness-management.spec.ts", "**/harness-catalog-screenshots.spec.ts", + "**/inline-custom-harness.spec.ts", ], use: { ...devices["Desktop Chrome"], diff --git a/desktop/scripts/check-file-sizes.mjs b/desktop/scripts/check-file-sizes.mjs index f8ee32dfab..3983fa591d 100644 --- a/desktop/scripts/check-file-sizes.mjs +++ b/desktop/scripts/check-file-sizes.mjs @@ -697,12 +697,18 @@ const overrides = new Map([ // hidden-key projection keeps the top-level secret out of Advanced rows. // +6 (1195 -> 1201): rebase onto main — this PR's model-source label wiring // lands on top of main's dialog growth. Queued to split. - ["src/features/agents/ui/AgentInstanceEditDialog.tsx", 1201], + // +28 (1201 -> 1229): inline "Add custom harness…" entry — sentinel option, + // modal state, and the AddCustomHarnessDialog mount. The shared routing and + // deferred-selection logic lives in addCustomHarness.ts to keep this minimal. + ["src/features/agents/ui/AgentInstanceEditDialog.tsx", 1229], // AgentDefinitionDialog grew past 1000 with the following load-bearing fixes: // isRuntimeAutoSeededRef tracking for edit-mode seeding (Fizz shows models); // runtimeSupportsLlmProviderSelection guard on discovery provider (codex fix); // hideProviderIds computation for Databricks v1 gate. Queued to split. - ["src/features/agents/ui/AgentDefinitionDialog.tsx", 1035], + // +28 (1020 -> 1048): inline "Add custom harness…" entry — sentinel option, + // modal state, and the AddCustomHarnessDialog mount. The shared routing and + // deferred-selection logic lives in addCustomHarness.ts to keep this minimal. + ["src/features/agents/ui/AgentDefinitionDialog.tsx", 1048], // #2630 emoji picker search: the shadow-root search-input autofocus effect // (rAF retry loop) took this file 999 -> 1026 and landed without this entry, // so main's Desktop Core went red. Queued to split with the rest of this list. diff --git a/desktop/src/features/agents/ui/AddCustomHarnessDialog.tsx b/desktop/src/features/agents/ui/AddCustomHarnessDialog.tsx new file mode 100644 index 0000000000..0c84e99275 --- /dev/null +++ b/desktop/src/features/agents/ui/AddCustomHarnessDialog.tsx @@ -0,0 +1,47 @@ +import { CustomHarnessForm } from "@/features/settings/ui/CustomHarnessForm"; +import { ChooserDialogContent } from "@/shared/ui/chooser-dialog-content"; +import { Dialog } from "@/shared/ui/dialog"; + +/** + * Registers a custom ACP harness from inside an agent dialog, so "New agent" + * is a complete entry point and not a dead end that sends the user to + * Settings. Hosts the same `CustomHarnessForm` the harness catalog uses. + */ +export function AddCustomHarnessDialog({ + onOpenChange, + onSaved, + open, +}: { + onOpenChange: (open: boolean) => void; + /** Called with the id of the harness that was just registered. */ + onSaved: (id: string) => void; + open: boolean; +}) { + return ( + + + + Register any ACP-speaking agent tool as a selectable harness. +

+ } + onCancel={() => onOpenChange(false)} + onSaved={(id) => { + // Dismiss on save as well as cancel — both exits belong to this + // dialog, so callers only handle the resulting selection. + onOpenChange(false); + onSaved(id); + }} + /> + + + ); +} diff --git a/desktop/src/features/agents/ui/AgentDefinitionDialog.tsx b/desktop/src/features/agents/ui/AgentDefinitionDialog.tsx index 45031e0371..5425131448 100644 --- a/desktop/src/features/agents/ui/AgentDefinitionDialog.tsx +++ b/desktop/src/features/agents/ui/AgentDefinitionDialog.tsx @@ -83,6 +83,12 @@ import { import { useProviderApiKeyFieldState } from "./providerApiKeyFieldState"; import { buildRuntimeModelProviderPayload } from "./agentDefinitionSubmitPayload"; import { AgentDefinitionDialogFooter } from "./AgentDefinitionDialogFooter"; +import { AddCustomHarnessDialog } from "./AddCustomHarnessDialog"; +import { + ADD_CUSTOM_HARNESS_OPTION, + runtimeDropdownAction, + usePendingHarnessSelection, +} from "./addCustomHarness"; type AgentDefinitionDialogProps = { open: boolean; @@ -167,6 +173,7 @@ export function AgentDefinitionDialog({ const [isAvatarUploadPending, setIsAvatarUploadPending] = React.useState(false); const [hasUserChanges, setHasUserChanges] = React.useState(false); + const [isAddHarnessOpen, setIsAddHarnessOpen] = React.useState(false); const { globalConfig, inheritedDefaults: { @@ -308,6 +315,7 @@ export function AgentDefinitionDialog({ setShowAdvancedFields(false); setIsAvatarUploadPending(false); setHasUserChanges(false); + setIsAddHarnessOpen(false); // isRuntimeAutoSeededRef and hasSeededForOpenRef are NOT reset here — the // [initialValues, open] effect resets both when the dialog re-opens. } @@ -578,6 +586,7 @@ export function AgentDefinitionDialog({ runtimes, runtimesLoading, }); + runtimeDropdownOptions.push(ADD_CUSTOM_HARNESS_OPTION); const runtimeSummaryLabel = selectedRuntime ? formatRuntimeOptionLabel(selectedRuntime) : runtime.trim() || "Not configured"; @@ -662,9 +671,13 @@ export function AgentDefinitionDialog({ } function handleRuntimeDropdownChange(nextValue: string) { + const action = runtimeDropdownAction(nextValue); + if (action.kind === "add-custom-harness") { + setIsAddHarnessOpen(true); + return; + } setHasUserChanges(true); - const nextRuntime = - nextValue === NO_RUNTIME_DROPDOWN_VALUE ? "" : nextValue; + const nextRuntime = action.runtimeId; // The user made an explicit choice — no longer auto-seeded. isRuntimeAutoSeededRef.current = false; setRuntime(nextRuntime); @@ -680,6 +693,15 @@ export function AgentDefinitionDialog({ ); } + // Routed through the normal change handler so a harness registered inline + // resets model/provider exactly as a hand-picked one would. Scoped to `open` + // so a pending id can't outlive the dialog that started the registration. + const selectSavedHarness = usePendingHarnessSelection( + runtimes, + handleRuntimeDropdownChange, + open, + ); + function handleProviderDropdownChange(nextValue: string) { setHasUserChanges(true); const nextProvider = @@ -944,6 +966,12 @@ export function AgentDefinitionDialog({ returnFocusRef={aiDefaultsTriggerRef} /> + + {isCreateMode ? createRunSection : null}
diff --git a/desktop/src/features/agents/ui/AgentInstanceEditDialog.tsx b/desktop/src/features/agents/ui/AgentInstanceEditDialog.tsx index 601d57f95d..f3c410e2ff 100644 --- a/desktop/src/features/agents/ui/AgentInstanceEditDialog.tsx +++ b/desktop/src/features/agents/ui/AgentInstanceEditDialog.tsx @@ -83,6 +83,12 @@ import { useProviderApiKeyFieldState } from "./providerApiKeyFieldState"; import { resolveModelFieldStatusMessage } from "./agentConfigControls"; import { AdvancedRequiredBadge } from "./AdvancedRequiredBadge"; import { showAgentProfileSyncWarning } from "./agentProfileSyncWarning"; +import { AddCustomHarnessDialog } from "./AddCustomHarnessDialog"; +import { + ADD_CUSTOM_HARNESS_OPTION, + runtimeDropdownAction, + usePendingHarnessSelection, +} from "./addCustomHarness"; const ADVANCED_FIELDS_MOTION_TRANSITION = { duration: 0.18, @@ -157,6 +163,7 @@ export function AgentInstanceEditDialog({ const [avatarUrl, setAvatarUrl] = React.useState(agent.avatarUrl ?? ""); const [isAvatarUploadPending, setIsAvatarUploadPending] = React.useState(false); + const [isAddHarnessOpen, setIsAddHarnessOpen] = React.useState(false); const shouldReduceMotion = useReducedMotion(); // Runtime selector: defaults to "custom" until the dialog opens and the @@ -191,6 +198,7 @@ export function AgentInstanceEditDialog({ setAvatarUrl(agent.avatarUrl ?? ""); setShowAdvancedFields(false); setIsAvatarUploadPending(false); + setIsAddHarnessOpen(false); runtimeTouched.current = false; const matched = runtimes.find((r) => r.command?.trim() === agent.agentCommand.trim()) ?? @@ -244,6 +252,7 @@ export function AgentInstanceEditDialog({ value: selectedRuntimeId, }); } + options.push(ADD_CUSTOM_HARNESS_OPTION); return options; }, [sortedRuntimes, selectedRuntimeId]); @@ -484,8 +493,12 @@ export function AgentInstanceEditDialog({ } function handleRuntimeDropdownChange(nextValue: string) { - const nextRuntimeId = - nextValue === NO_RUNTIME_DROPDOWN_VALUE ? "" : nextValue; + const action = runtimeDropdownAction(nextValue); + if (action.kind === "add-custom-harness") { + setIsAddHarnessOpen(true); + return; + } + const nextRuntimeId = action.runtimeId; const previousRuntimeId = selectedRuntimeId; const nextRuntime = runtimes.find((r) => r.id === nextRuntimeId); @@ -532,6 +545,16 @@ export function AgentInstanceEditDialog({ ); } + // Routed through the normal change handler so a harness registered inline + // pins its command and resets model/provider like a hand-picked one. Scoped + // to `open` so a pending id can't outlive the dialog that started the + // registration. + const selectSavedHarness = usePendingHarnessSelection( + runtimes, + handleRuntimeDropdownChange, + open, + ); + function handleProviderDropdownChange(nextValue: string) { const nextProvider = nextValue === AUTO_PROVIDER_DROPDOWN_VALUE ? "" : nextValue; @@ -949,6 +972,11 @@ export function AgentInstanceEditDialog({

) : null} +
{selectedRuntimeId === "custom" && !inheritHarness ? (
diff --git a/desktop/src/features/agents/ui/addCustomHarness.test.mjs b/desktop/src/features/agents/ui/addCustomHarness.test.mjs new file mode 100644 index 0000000000..6c0aa32daf --- /dev/null +++ b/desktop/src/features/agents/ui/addCustomHarness.test.mjs @@ -0,0 +1,344 @@ +/** + * Behavior tests for the inline "Add custom harness…" dropdown entry shared by + * AgentDefinitionDialog and AgentInstanceEditDialog. + * + * Two seams carry the feature, and both are pinned here: + * + * 1. ROUTING (`runtimeDropdownAction`) — the sentinel must resolve to "open + * the form", never to a selection. If it ever resolved to a selection the + * dialogs would write "\u0000add-custom-harness" into `runtime` and try to + * spawn an agent on a harness that does not exist. + * 2. DEFERRED SELECTION (`usePendingHarnessSelection`) — saving only writes + * the definition file; the harness becomes a catalog entry when the + * invalidated discovery query refetches. Selecting on save would pick an + * id no entry backs. The hook must wait for the catalog, fire exactly + * once, stay silent when the user cancels, and drop the pending id when + * its dialog closes — the host dialogs stay mounted, so a stale id would + * otherwise select into reset form state on a later publish. + * + * The hook is mounted for real (react-dom/client + act) rather than simulated, + * so its effect wiring — including the guard that survives the dialogs' + * non-memoized change handlers — is what gets tested. + */ + +import assert from "node:assert/strict"; +import test from "node:test"; + +// ── Minimal DOM shim ───────────────────────────────────────────────────────── +// react-dom/client needs a container element and a document; node has neither. +// The harness renders null, so no real node operations are exercised. + +class ElementShim { + constructor() { + this.children = []; + this.childNodes = []; + this.nodeType = 1; + this.nodeName = "DIV"; + this.tagName = "DIV"; + this.namespaceURI = "http://www.w3.org/1999/xhtml"; + } + get ownerDocument() { + return globalThis.document; + } + addEventListener() {} + removeEventListener() {} + appendChild(child) { + this.children.push(child); + this.childNodes.push(child); + return child; + } + removeChild(child) { + this.children = this.children.filter((current) => current !== child); + this.childNodes = this.childNodes.filter((current) => current !== child); + return child; + } + insertBefore(child) { + return this.appendChild(child); + } + contains(target) { + return this === target; + } +} + +globalThis.document = { + activeElement: null, + addEventListener() {}, + createElement: () => new ElementShim(), + get defaultView() { + return globalThis.window; + }, + nodeType: 9, + removeEventListener() {}, +}; +// react-dom derives update priority from window.event and walks iframe +// boundaries via window.HTMLIFrameElement during commit. +Object.defineProperty(globalThis, "window", { + configurable: true, + value: { + addEventListener() {}, + document: globalThis.document, + event: undefined, + HTMLIFrameElement: ElementShim, + removeEventListener() {}, + }, +}); +globalThis.HTMLElement = ElementShim; +globalThis.Node = ElementShim; +globalThis.IS_REACT_ACT_ENVIRONMENT = true; + +import React from "react"; +import { act } from "react"; +import { createRoot } from "react-dom/client"; + +import { NO_RUNTIME_DROPDOWN_VALUE } from "./agentConfigOptions.tsx"; +import { + ADD_CUSTOM_HARNESS_OPTION, + ADD_CUSTOM_HARNESS_VALUE, + readyHarnessId, + runtimeDropdownAction, + usePendingHarnessSelection, +} from "./addCustomHarness.ts"; + +// ── Routing: the sentinel opens the form, it is never a selection ──────────── + +test("selecting the add-custom entry requests the form and yields no runtime id", () => { + const action = runtimeDropdownAction(ADD_CUSTOM_HARNESS_VALUE); + assert.equal(action.kind, "add-custom-harness"); + // The dialogs read `action.runtimeId` on the select branch; the sentinel + // must not carry one, or it could leak into form state. + assert.equal("runtimeId" in action, false); +}); + +test("selecting a harness yields that harness id", () => { + assert.deepEqual(runtimeDropdownAction("my-harness"), { + kind: "select", + runtimeId: "my-harness", + }); +}); + +test("selecting the no-runtime entry yields the empty id", () => { + assert.deepEqual(runtimeDropdownAction(NO_RUNTIME_DROPDOWN_VALUE), { + kind: "select", + runtimeId: "", + }); +}); + +test("the add-custom sentinel cannot collide with a backend-valid harness id", () => { + // Backend ids match [a-z0-9_][a-z0-9_-]* (custom_harnesses.rs), so a + // NUL-prefixed value is unreachable as a real id. + assert.equal(ADD_CUSTOM_HARNESS_VALUE.startsWith("\u0000"), true); + assert.equal(ADD_CUSTOM_HARNESS_OPTION.value, ADD_CUSTOM_HARNESS_VALUE); + assert.equal(ADD_CUSTOM_HARNESS_OPTION.label, "Add custom harness…"); +}); + +// ── Readiness: an id is selectable only once the catalog publishes it ──────── + +test("a pending id absent from the catalog is not ready", () => { + assert.equal(readyHarnessId([{ id: "claude" }], "my-harness"), null); +}); + +test("a pending id present in the catalog is ready", () => { + assert.equal( + readyHarnessId([{ id: "claude" }, { id: "my-harness" }], "my-harness"), + "my-harness", + ); +}); + +test("no pending id is never ready even against a populated catalog", () => { + assert.equal(readyHarnessId([{ id: "claude" }], null), null); +}); + +// ── Deferred selection: mounted hook ───────────────────────────────────────── + +/** + * Mount the real hook over a mutable catalog. Returns the setter the dialogs + * call on save, a `setRuntimes` to simulate the discovery refetch, a `setOpen` + * to simulate the owning dialog closing and reopening, and the log of ids the + * hook handed back for selection. + */ +async function mountPendingSelection(initialRuntimes = []) { + const selected = []; + const control = {}; + + function Harness() { + const [runtimes, setRuntimes] = React.useState(initialRuntimes); + const [open, setOpen] = React.useState(true); + // Deliberately NOT memoized: both dialogs pass a plain function + // declaration, so `onReady` has a fresh identity on every render. + const onReady = (id) => selected.push(id); + control.save = usePendingHarnessSelection(runtimes, onReady, open); + control.setRuntimes = setRuntimes; + control.setOpen = setOpen; + return null; + } + + const root = createRoot(new ElementShim()); + await act(async () => { + root.render(React.createElement(Harness)); + }); + return { control, root, selected }; +} + +test("saving a harness selects it only once the catalog publishes it", async () => { + const { control, root, selected } = await mountPendingSelection([ + { id: "claude" }, + ]); + + // Save returns before discovery refetches — nothing to select yet. + await act(async () => control.save("my-harness")); + assert.deepEqual(selected, []); + + // The invalidated discovery query resolves with the new entry. + await act(async () => + control.setRuntimes([{ id: "claude" }, { id: "my-harness" }]), + ); + assert.deepEqual(selected, ["my-harness"]); + + await act(async () => root.unmount()); +}); + +test("a published harness is selected exactly once across later catalog updates", async () => { + const { control, root, selected } = await mountPendingSelection([ + { id: "claude" }, + ]); + + await act(async () => control.save("my-harness")); + await act(async () => + control.setRuntimes([{ id: "claude" }, { id: "my-harness" }]), + ); + assert.deepEqual(selected, ["my-harness"]); + + // Any later refetch re-renders with a new array identity and a new onReady + // identity. Re-firing here would clobber a selection the user made in + // between, so the pending id must have been cleared. + await act(async () => + control.setRuntimes([ + { id: "claude" }, + { id: "my-harness" }, + { id: "codex" }, + ]), + ); + assert.deepEqual(selected, ["my-harness"]); + + await act(async () => root.unmount()); +}); + +test("cancelling the form leaves the current selection untouched", async () => { + const { control, root, selected } = await mountPendingSelection([ + { id: "claude" }, + ]); + + // Cancel never reports a saved id, so no selection is ever requested — even + // as the catalog keeps refreshing underneath. + await act(async () => + control.setRuntimes([{ id: "claude" }, { id: "codex" }]), + ); + assert.deepEqual(selected, []); + + await act(async () => root.unmount()); +}); + +test("a saved harness discovery never publishes is never selected", async () => { + const { control, root, selected } = await mountPendingSelection([ + { id: "claude" }, + ]); + + // e.g. the definition file was written but the entry failed to load. The + // hook must stall rather than select an id no catalog entry backs. + await act(async () => control.save("ghost-harness")); + await act(async () => + control.setRuntimes([{ id: "claude" }, { id: "codex" }]), + ); + assert.deepEqual(selected, []); + + await act(async () => root.unmount()); +}); + +test("two harnesses registered in a row are each selected when published", async () => { + const { control, root, selected } = await mountPendingSelection([ + { id: "claude" }, + ]); + + await act(async () => control.save("first")); + await act(async () => + control.setRuntimes([{ id: "claude" }, { id: "first" }]), + ); + await act(async () => control.save("second")); + await act(async () => + control.setRuntimes([{ id: "claude" }, { id: "first" }, { id: "second" }]), + ); + assert.deepEqual(selected, ["first", "second"]); + + await act(async () => root.unmount()); +}); + +test("a second save before the first publishes selects only the later harness", async () => { + const { control, root, selected } = await mountPendingSelection([ + { id: "claude" }, + ]); + + // The dropdown holds one harness, so the latest registration wins: the + // first id is dropped rather than queued behind the second. + await act(async () => control.save("first")); + await act(async () => control.save("second")); + await act(async () => + control.setRuntimes([{ id: "claude" }, { id: "first" }, { id: "second" }]), + ); + assert.deepEqual(selected, ["second"]); + + await act(async () => root.unmount()); +}); + +// ── Lifecycle: a pending id never outlives the dialog that created it ──────── + +test("a harness published after its dialog closed is never selected", async () => { + const { control, root, selected } = await mountPendingSelection([ + { id: "claude" }, + ]); + + // Both host dialogs stay mounted when closed, so the hook keeps running. + await act(async () => control.save("my-harness")); + await act(async () => control.setOpen(false)); + await act(async () => + control.setRuntimes([{ id: "claude" }, { id: "my-harness" }]), + ); + + // Selecting here would write into form state the close already reset. + assert.deepEqual(selected, []); + + await act(async () => root.unmount()); +}); + +test("reopening after closing mid-registration does not select the abandoned harness", async () => { + const { control, root, selected } = await mountPendingSelection([ + { id: "claude" }, + ]); + + await act(async () => control.save("my-harness")); + await act(async () => control.setOpen(false)); + await act(async () => + control.setRuntimes([{ id: "claude" }, { id: "my-harness" }]), + ); + // The reopened dialog seeds from its own initial values; a stale pending id + // must not overwrite them. + await act(async () => control.setOpen(true)); + assert.deepEqual(selected, []); + + await act(async () => root.unmount()); +}); + +test("a harness saved after reopening is still selected when published", async () => { + const { control, root, selected } = await mountPendingSelection([ + { id: "claude" }, + ]); + + await act(async () => control.setOpen(false)); + await act(async () => control.setOpen(true)); + await act(async () => control.save("my-harness")); + await act(async () => + control.setRuntimes([{ id: "claude" }, { id: "my-harness" }]), + ); + assert.deepEqual(selected, ["my-harness"]); + + await act(async () => root.unmount()); +}); diff --git a/desktop/src/features/agents/ui/addCustomHarness.ts b/desktop/src/features/agents/ui/addCustomHarness.ts new file mode 100644 index 0000000000..f9c2143530 --- /dev/null +++ b/desktop/src/features/agents/ui/addCustomHarness.ts @@ -0,0 +1,104 @@ +/** + * Shared pieces of the inline "Add custom harness…" entry the agent dialogs + * append to their harness dropdown. + * + * Registering a custom harness used to be reachable only from Settings, so + * anyone whose first stop was "New agent" never learned the path existed. + * These helpers keep the entry identical across the dropdowns, keep its + * sentinel value out of form state, and defer selecting a freshly registered + * harness until discovery has actually published it. + */ + +import * as React from "react"; + +import { + NO_RUNTIME_DROPDOWN_VALUE, + type PersonaDropdownOption, +} from "./agentConfigOptions"; + +/** + * Dropdown value for the add-custom-harness entry. NUL-prefixed so it can + * never collide with a harness id (`[a-z0-9_][a-z0-9_-]*`) — same trick as the + * harness catalog's `CUSTOM_ENTRY_ID`. + */ +export const ADD_CUSTOM_HARNESS_VALUE = "\u0000add-custom-harness"; + +export const ADD_CUSTOM_HARNESS_OPTION: PersonaDropdownOption = { + label: "Add custom harness…", + value: ADD_CUSTOM_HARNESS_VALUE, +}; + +export type RuntimeDropdownAction = + | { kind: "add-custom-harness" } + | { kind: "select"; runtimeId: string }; + +/** + * Route a harness-dropdown change. The add-custom entry only opens the + * registration form — it is never a selection, so its sentinel can't reach + * form state. Every other value selects, with the no-runtime sentinel + * normalized to the empty id. + */ +export function runtimeDropdownAction(value: string): RuntimeDropdownAction { + if (value === ADD_CUSTOM_HARNESS_VALUE) { + return { kind: "add-custom-harness" }; + } + return { + kind: "select", + runtimeId: value === NO_RUNTIME_DROPDOWN_VALUE ? "" : value, + }; +} + +/** + * The pending harness id once discovery has published it, else `null`. + * + * Saving only writes the definition file — the harness becomes a catalog entry + * when the invalidated discovery query refetches. Selecting before then would + * pick an id no entry backs: the create dialog would block Save on an unknown + * availability, and the instance dialog could not read the command to pin. + */ +export function readyHarnessId( + runtimes: ReadonlyArray<{ id: string }>, + pendingId: string | null, +): string | null { + return runtimes.some((runtime) => runtime.id === pendingId) + ? pendingId + : null; +} + +/** + * Selects a newly registered custom harness once discovery publishes it. + * + * Returns the setter to hand the saved id; `onReady` then fires with it, so + * callers reuse their normal dropdown-change path instead of growing a second + * selection code path. + * + * `active` is the owning dialog's open state. The wait is only meaningful + * while that dialog is open: both host dialogs stay mounted across closes, so + * a pending id would otherwise survive the close and select into reset — or + * hidden — form state whenever discovery caught up. Going inactive both blocks + * `onReady` and drops the pending id, so a later publish is a no-op and + * reopening starts clean. A second save before the first publishes replaces + * it: the field holds one harness, so the latest save wins. + */ +export function usePendingHarnessSelection( + runtimes: ReadonlyArray<{ id: string }>, + onReady: (id: string) => void, + active: boolean, +): (id: string) => void { + const [pendingId, setPendingId] = React.useState(null); + // Gated at render, not just in the effect, so a catalog update landing in + // the same commit as the close cannot slip a selection through. + const readyId = active ? readyHarnessId(runtimes, pendingId) : null; + + React.useEffect(() => { + if (!active) { + setPendingId(null); + return; + } + if (readyId === null) return; + setPendingId(null); + onReady(readyId); + }, [active, onReady, readyId]); + + return setPendingId; +} diff --git a/desktop/src/features/settings/ui/CustomHarnessForm.tsx b/desktop/src/features/settings/ui/CustomHarnessForm.tsx index 60f3a66af1..52e7906265 100644 --- a/desktop/src/features/settings/ui/CustomHarnessForm.tsx +++ b/desktop/src/features/settings/ui/CustomHarnessForm.tsx @@ -216,7 +216,8 @@ export function CustomHarnessForm({ * delete the old file when the id changes. */ originalId?: string; onCancel: () => void; - onSaved: () => void; + /** Receives the id the harness was saved under (the form may rewrite it). */ + onSaved: (id: string) => void; /** Render without the bordered card chrome (for embedding in the catalog * dialog detail pane). */ chromeless?: boolean; @@ -273,11 +274,9 @@ export function CustomHarnessForm({ return; } try { - await save.mutateAsync({ - definition: definitionFromFormValues(form), - originalId, - }); - onSaved(); + const definition = definitionFromFormValues(form); + await save.mutateAsync({ definition, originalId }); + onSaved(definition.id); } catch (err) { setError(err instanceof Error ? err.message : String(err)); } diff --git a/desktop/tests/e2e/inline-custom-harness.spec.ts b/desktop/tests/e2e/inline-custom-harness.spec.ts new file mode 100644 index 0000000000..6b3845d65a --- /dev/null +++ b/desktop/tests/e2e/inline-custom-harness.spec.ts @@ -0,0 +1,194 @@ +/** + * E2E spec for the inline "Add custom harness…" entry in the agent dialogs. + * + * Registering a custom harness used to be reachable only from Settings → + * Agents, so anyone whose first touchpoint was "New agent" never learned the + * path existed. The harness dropdowns now carry the entry directly. + * + * Covers, on all three surfaces (create, edit definition, edit instance): + * - choosing the entry opens the registration form + * - saving registers the harness and selects it in the dropdown + * + * Create and instance edit additionally assert the sentinel never becomes the + * selection; create alone covers dismissing the form leaving the previous + * selection untouched. The three surfaces share the same routing, so those + * checks are not repeated on every one. + */ +import { expect, test } from "@playwright/test"; + +import { installMockBridge } from "../helpers/bridge"; + +const ADD_ENTRY = "Add custom harness…"; +const HARNESS_LABEL = "My Weird Agent"; +const HARNESS_COMMAND = "my-weird-acp"; + +type Page = import("@playwright/test").Page; +type Locator = import("@playwright/test").Locator; + +/** Open a PersonaDropdownField (button trigger + menuitemradio options). */ +async function openDropdown(trigger: Locator) { + await expect(trigger).toBeVisible({ timeout: 10_000 }); + await trigger.click(); +} + +/** Register a harness through the inline form and wait for it to close. */ +async function registerHarness(page: Page) { + const form = page.getByTestId("custom-harness-form"); + await expect(form).toBeVisible({ timeout: 8_000 }); + await page.fill("#ch-label", HARNESS_LABEL); + await page.fill("#ch-command", HARNESS_COMMAND); + // The id auto-derives from the label; it is what the dropdown selects on. + await expect(page.locator("#ch-id")).toHaveValue("my-weird-agent"); + await form.getByRole("button", { name: "Save", exact: true }).click(); + await expect(page.getByTestId("add-custom-harness-dialog")).not.toBeVisible({ + timeout: 8_000, + }); +} + +/** Open the create-agent dialog (AgentDefinitionDialog, create mode). */ +async function openCreateDialog(page: Page) { + await page.goto("/", { waitUntil: "domcontentloaded" }); + await page.getByTestId("open-agents-view").click(); + await page.getByTestId("new-agent-card").click(); + await page.getByRole("menuitem", { name: "Create agent" }).click(); + const dialog = page.getByTestId("persona-dialog"); + await expect(dialog).toBeVisible({ timeout: 10_000 }); + await dialog.getByRole("tab", { name: "Customize for this agent" }).click(); + return dialog; +} + +/** Open the edit dialog for a saved definition (same dialog, edit mode). */ +async function openDefinitionEditDialog(page: Page, name: string) { + await page.goto("/", { waitUntil: "domcontentloaded" }); + await page.getByTestId("open-agents-view").click(); + await expect(page.getByTestId("agents-library-personas")).toBeVisible({ + timeout: 10_000, + }); + await page.getByRole("button", { name: `Open actions for ${name}` }).click(); + await page.getByRole("menuitem", { name: "Edit" }).click(); + const dialog = page.getByTestId("persona-dialog"); + await expect(dialog).toBeVisible({ timeout: 10_000 }); + await dialog.getByRole("tab", { name: "Customize for this agent" }).click(); + return dialog; +} + +test.describe("inline add custom harness", () => { + test("create dialog registers a harness inline and selects it", async ({ + page, + }) => { + await installMockBridge(page); + const dialog = await openCreateDialog(page); + + const harness = dialog.locator("#persona-runtime"); + await openDropdown(harness); + await page.getByRole("menuitemradio", { name: ADD_ENTRY }).click(); + + // The sentinel opens the form; it must never become the selection. + await expect(page.getByTestId("add-custom-harness-dialog")).toBeVisible({ + timeout: 8_000, + }); + await expect(harness).not.toContainText(ADD_ENTRY); + + await registerHarness(page); + + // The saved harness is now the selected harness. + await expect(harness).toContainText(HARNESS_LABEL, { timeout: 8_000 }); + }); + + test("dismissing the form leaves the create dialog's harness unchanged", async ({ + page, + }) => { + await installMockBridge(page); + const dialog = await openCreateDialog(page); + + const harness = dialog.locator("#persona-runtime"); + await expect(harness).toBeVisible({ timeout: 10_000 }); + const before = await harness.textContent(); + + await openDropdown(harness); + await page.getByRole("menuitemradio", { name: ADD_ENTRY }).click(); + await expect(page.getByTestId("add-custom-harness-dialog")).toBeVisible({ + timeout: 8_000, + }); + + await page.keyboard.press("Escape"); + await expect( + page.getByTestId("add-custom-harness-dialog"), + ).not.toBeVisible(); + + // No harness was registered, so the prior selection must survive. + await expect(harness).toHaveText(before ?? ""); + }); + + test("definition edit dialog registers a harness inline and selects it", async ({ + page, + }) => { + await installMockBridge(page, { + personas: [ + { + displayName: "Editable Agent", + systemPrompt: "An agent whose harness gets replaced.", + }, + ], + }); + const dialog = await openDefinitionEditDialog(page, "Editable Agent"); + + const harness = dialog.locator("#persona-runtime"); + await openDropdown(harness); + await page.getByRole("menuitemradio", { name: ADD_ENTRY }).click(); + await expect(page.getByTestId("add-custom-harness-dialog")).toBeVisible({ + timeout: 8_000, + }); + + await registerHarness(page); + + await expect(harness).toContainText(HARNESS_LABEL, { timeout: 8_000 }); + }); + + test("instance edit dialog registers a harness inline and keeps Custom command", async ({ + page, + }) => { + await installMockBridge(page, { + managedAgents: [ + { + pubkey: + "npub1e2e00000000000000000000000000000000000000000000000000000000", + name: "Instance Agent", + status: "stopped", + channelNames: ["agents"], + }, + ], + }); + + await page.goto("/", { waitUntil: "domcontentloaded" }); + await page.getByTestId("open-agents-view").click(); + await page + .getByRole("button", { name: "Instance Agent agent profile" }) + .click(); + await expect(page.getByTestId("user-profile-panel")).toBeVisible({ + timeout: 10_000, + }); + await page.getByTestId("user-profile-edit-agent").click(); + await expect(page.getByTestId("edit-agent-dialog")).toBeVisible({ + timeout: 10_000, + }); + + const provider = page.locator("#edit-agent-runtime"); + await openDropdown(provider); + + // "Custom command" is a different feature (ad-hoc command override) and + // must survive alongside the new entry. + await expect( + page.getByRole("menuitemradio", { name: "Custom command" }), + ).toBeVisible(); + await page.getByRole("menuitemradio", { name: ADD_ENTRY }).click(); + await expect(page.getByTestId("add-custom-harness-dialog")).toBeVisible({ + timeout: 8_000, + }); + await expect(provider).not.toContainText(ADD_ENTRY); + + await registerHarness(page); + + await expect(provider).toContainText(HARNESS_LABEL, { timeout: 8_000 }); + }); +}); From 1d4f97b959a0d91f7bac0e1f97189e5c10347712 Mon Sep 17 00:00:00 2001 From: Will Pfleger Date: Tue, 28 Jul 2026 16:03:13 -0400 Subject: [PATCH 02/11] fix(acp): disable goose cron scheduler in managed agent children (#3144) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A Buzz install with a scheduled goose recipe fires each cron entry once per `goose acp` child instead of once, because every child unconditionally starts its own cron scheduler over the shared `~/.local/share/goose/schedule.json`. With a pool of N children per harness and multiple harnesses, one scheduled recipe fans out to N × harness_count executions — each running under the managed agent's identity rather than the operator's, and racing the operator's own standalone goose over the same schedule file. This injects `GOOSE_ACP_SCHEDULER_DISABLED=true` into every child spawned by `AcpClient::spawn`, so a managed agent never owns the operator's cron schedule. ## Placement The `cmd.env` call is set last — after the `extra_env` operator-wins loop and after the `CODEX_CONFIG` merge — deliberately with no escape hatch. Managed children not running the operator's schedule is a correctness invariant rather than an operator-tunable default, so the injection must beat both a conflicting persona `extra_env` entry and any value inherited from the parent process. It is injected for all agents, not just goose. Agent builds that don't recognize the variable ignore it. ## Sequencing The goose-side flag that reads this variable and skips scheduler startup lands separately (repo TBD). Until it does, this change is a forward-compatible no-op: it sets an environment variable nothing currently reads. Merging it first means no coordinated release is needed — the fix takes effect as soon as the goose side ships. Related: https://github.com/aaif-goose/goose/pull/10738 Signed-off-by: Will Pfleger --- crates/buzz-acp/src/acp.rs | 85 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 85 insertions(+) diff --git a/crates/buzz-acp/src/acp.rs b/crates/buzz-acp/src/acp.rs index c0147baf1b..23f0345e96 100644 --- a/crates/buzz-acp/src/acp.rs +++ b/crates/buzz-acp/src/acp.rs @@ -20,6 +20,10 @@ use crate::usage::{TurnUsage, UsageTracker}; /// Lines exceeding this limit are rejected to prevent OOM from rogue agents. const MAX_LINE_SIZE: usize = 10_000_000; // 10 MB +/// Env var that tells a goose ACP child not to start its cron scheduler. +/// Injected unconditionally by [`AcpClient::spawn`]; see the call site for why. +pub(crate) const GOOSE_SCHEDULER_DISABLED_ENV: &str = "GOOSE_ACP_SCHEDULER_DISABLED"; + /// An MCP server configuration passed to `session/new`. /// /// Corresponds to the `McpServerStdio` variant in the ACP schema. @@ -460,6 +464,16 @@ impl AcpClient { cmd.env("CODEX_CONFIG", merged); } + // Buzz-managed agents must never execute the operator's personal cron + // schedule. A goose ACP child starts a scheduler over the shared + // `schedule.json`, so a pool of N children fires every scheduled job N + // times — under the wrong identity and racing standalone goose. + // + // Set last, and with no operator-wins escape hatch, so it beats both a + // conflicting persona `extra_env` entry and any inherited parent value. + // Agent builds that don't recognize the variable ignore it. + cmd.env(GOOSE_SCHEDULER_DISABLED_ENV, "true"); + // Spawn the agent in its own process group so SIGKILL doesn't propagate // to the harness's own process group on Unix. // tokio::process::Command::process_group is a stable tokio API (no extra imports needed). @@ -2653,6 +2667,77 @@ mod tests { .expect("failed to spawn test script") } + /// Spawn a script that echoes the named env vars as the child observes + /// them, one per line. `` means the child did not receive the var. + async fn spawn_and_read_child_env( + vars: &[&str], + extra_env: &[(String, String)], + ) -> Vec { + let script = vars + .iter() + .map(|var| format!("printf '%s\\n' \"${{{var}:-}}\"")) + .collect::>() + .join("\n"); + let mut client = AcpClient::spawn("bash", &["-c".into(), script], extra_env, false) + .await + .expect("failed to spawn env probe script"); + let mut observed = Vec::with_capacity(vars.len()); + for var in vars { + observed.push( + client + .reader + .next() + .await + .unwrap_or_else(|| panic!("child produced no output for {var}")) + .expect("child stdout was not readable"), + ); + } + observed + } + + /// Every spawned agent must be told not to run the operator's cron + /// schedule, without the caller having to opt in. + #[tokio::test] + async fn spawn_injects_scheduler_disabled_env_by_default() { + let observed = spawn_and_read_child_env(&[GOOSE_SCHEDULER_DISABLED_ENV], &[]).await; + assert_eq!( + observed, + vec!["true"], + "{GOOSE_SCHEDULER_DISABLED_ENV} must be injected into every spawn" + ); + } + + /// Persona config must not be able to re-enable the scheduler: this is a + /// correctness invariant, not an operator-tunable default, so the + /// injection is set after (and therefore wins over) the `extra_env` loop. + /// + /// The control var pins that `extra_env` really did reach the child, so a + /// pass here means the conflicting entry lost the fight rather than + /// `extra_env` being dropped wholesale. + #[tokio::test] + async fn spawn_scheduler_disabled_env_overrides_conflicting_extra_env() { + let extra_env = vec![ + ( + GOOSE_SCHEDULER_DISABLED_ENV.to_string(), + "false".to_string(), + ), + ( + "BUZZ_ENV_PROBE_CONTROL".to_string(), + "delivered".to_string(), + ), + ]; + let observed = spawn_and_read_child_env( + &[GOOSE_SCHEDULER_DISABLED_ENV, "BUZZ_ENV_PROBE_CONTROL"], + &extra_env, + ) + .await; + assert_eq!( + observed, + vec!["true", "delivered"], + "a persona extra_env entry must not override {GOOSE_SCHEDULER_DISABLED_ENV}" + ); + } + #[tokio::test] async fn idle_timeout_fires_on_silent_process() { let mut client = spawn_script("sleep 10").await; From 1d3b810ad70d6325718ed91e723f32c4a376d5e1 Mon Sep 17 00:00:00 2001 From: Wes Date: Tue, 28 Jul 2026 14:17:01 -0600 Subject: [PATCH 03/11] fix(desktop): paint community rail full height (#3382) ## Summary - paint the community rail across the full app height instead of exposing the parent background through external margins - preserve the existing community-button alignment and balanced horizontal gutters by moving vertical spacing inside the rail - update the rail geometry coverage to require full-height paint ownership ## Root cause PR #2972 aligned the rail box with the inset content by adding top and bottom margins to the `bg-sidebar` element. Margins are outside the painted box, so flat light and dark themes exposed a differently colored app background above and below the rail. ## Validation - pre-push `desktop-check` - pre-push desktop unit suite: 3,751 passed - `git diff --check` Local Playwright/E2E was not run; CI owns the full browser matrix. Signed-off-by: Wes Co-authored-by: Carl --- .../src/features/sidebar/ui/CommunityRail.tsx | 2 +- desktop/tests/e2e/community-rail.spec.ts | 16 +++++++++++++++- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/desktop/src/features/sidebar/ui/CommunityRail.tsx b/desktop/src/features/sidebar/ui/CommunityRail.tsx index b15e0bab71..386ee20691 100644 --- a/desktop/src/features/sidebar/ui/CommunityRail.tsx +++ b/desktop/src/features/sidebar/ui/CommunityRail.tsx @@ -370,7 +370,7 @@ export function CommunityRail({ return (