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 }); + }); +});