From b0503d80c298b1ece3b0a43b41d316829a3379e7 Mon Sep 17 00:00:00 2001 From: Will Pfleger Date: Tue, 28 Jul 2026 16:01:41 -0400 Subject: [PATCH] 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 }); + }); +});