From becc025f130fffa397e1b7fe3209c75e7867e144 Mon Sep 17 00:00:00 2001 From: iris Date: Mon, 7 Sep 2026 18:53:35 +0200 Subject: [PATCH 1/4] swarm-ui: add a destroy trigger to the agents page Closes #4067. Backend half (Wanted::Destroyed + reconcile) shipped in #4065 with no swarm-controller API changes needed -- SetAgentStateRequest already accepted {"state": "destroyed"}, it just had nothing in swarm-ui sending it. Destroy is a separate `quiet`-variant badge next to the existing start/stop toggle (#3988's "wanted" column), not a third state folded into that same click target -- one wrong click on a shared toggle would be irreversible, where a dedicated badge only fires from its own confirm dialog. That confirm is a real `Dialog`, not the native `window.confirm` the reversible stop direction uses -- the wanted column's own comment called this out as the case that would justify one when it was first written. Shared the PUT-declare/pending/error/patch-rows logic between the existing toggleWanted and the new destroyAgent (declareState) rather than duplicating it -- confirmation and target-state selection are the only parts that differ between a toggle and a one-way declaration. Verified: `tsc --noEmit` clean, `nix fmt` reports the expected formatting-only diff, wire shapes (state string "destroyed", AgentDeclaration response) checked against swarm-queue-client's AgentState::as_str and swarm-controller's actual handler rather than assumed from the issue description. --- .../swarm-ui/src/pages/AgentsPage.css | 17 +++ .../swarm-ui/src/pages/AgentsPage.tsx | 122 ++++++++++++++---- 2 files changed, 112 insertions(+), 27 deletions(-) create mode 100644 frontend/packages/swarm-ui/src/pages/AgentsPage.css diff --git a/frontend/packages/swarm-ui/src/pages/AgentsPage.css b/frontend/packages/swarm-ui/src/pages/AgentsPage.css new file mode 100644 index 00000000..ad1b4542 --- /dev/null +++ b/frontend/packages/swarm-ui/src/pages/AgentsPage.css @@ -0,0 +1,17 @@ +/* — the destroy-confirm dialog's own layout. Everything + else on this page (the table, the wanted/destroy badges) draws chrome + from the shared `ui/` kit and needs nothing page-scoped; this file + exists only because the confirm dialog's copy + button row needed + somewhere to live, same reasoning as CreateAgentForm.css owning its + own layout next to the shared form-kit chrome it wraps. */ +.agents-destroy-confirm { + display: flex; + flex-direction: column; + gap: 1em; + max-width: 28em; +} +.agents-destroy-confirm-actions { + display: flex; + justify-content: flex-end; + gap: 0.75em; +} diff --git a/frontend/packages/swarm-ui/src/pages/AgentsPage.tsx b/frontend/packages/swarm-ui/src/pages/AgentsPage.tsx index 14010a1a..b8ee4b5c 100644 --- a/frontend/packages/swarm-ui/src/pages/AgentsPage.tsx +++ b/frontend/packages/swarm-ui/src/pages/AgentsPage.tsx @@ -26,7 +26,8 @@ // its own route. // // The "wanted" column is the start/stop control — see its own `render` -// and `toggleWanted` below for how. +// and `toggleWanted` below for how. Destroy is a separate control next +// to it — see `destroyTarget`/`destroyAgent`. import { useState } from "preact/hooks"; import { ApiErrorPanel } from "@hive/shared/api-error-panel.js"; import { readApiError, type ProblemDetails } from "@hive/shared/api-error.js"; @@ -42,6 +43,7 @@ import { } from "../ui/refresh-interval/RefreshInterval.js"; import { Table, type TableColumn } from "../ui/table/Table.js"; import { CreateAgentForm } from "./CreateAgentForm.js"; +import "./AgentsPage.css"; interface ConfigPrStatus { pr_number: number; @@ -56,8 +58,9 @@ interface AgentStatusSnapshot { running: boolean; } -// `"up"` / `"offline"`, wire-spelled by `swarm_queue_client::wanted::AgentState::as_str` -// — kept as a bare `string | null` rather than a union, same reason +// `"up"` / `"offline"` / `"destroyed"`, wire-spelled by +// `swarm_queue_client::wanted::AgentState::as_str` — kept as a bare +// `string | null` rather than a union, same reason // `config_pr`'s shape isn't re-derived here: this page renders whatever // the wire sends, it doesn't validate the enum client-side. type Wanted = string | null; @@ -114,6 +117,10 @@ export function AgentsPage() { const [actionErrors, setActionErrors] = useState< ReadonlyMap >(new Map()); + // The row pending destroy confirmation, `null` when the dialog is + // closed — not a boolean, so the dialog can name the agent without a + // second piece of state to keep in sync with it. + const [destroyTarget, setDestroyTarget] = useState(null); async function refresh() { const res = await fetch("/api/agents/status"); @@ -132,30 +139,13 @@ export function AgentsPage() { refresh().catch((e: unknown) => setError({ detail: String(e) })); }); - // Declares the opposite of `row`'s current state. A row with no - // declaration yet (`wanted === null`) has nothing to flip, so the - // target is read off the agent's own last-reported `running` instead - // — the button's first click always means "make the declaration match - // reality, then flip it", which is the only reading that makes sense - // without a declaration to toggle. - async function toggleWanted(row: AgentRow) { + // Shared by `toggleWanted` and `destroyAgent` — both are "PUT a new + // `wanted` declaration, track per-agent pending/error state, patch the + // response back into `rows`" with nothing else distinguishing them. + // Confirmation (native vs. `Dialog`) and target-state selection stay + // in each caller, since those are the parts that actually differ. + async function declareState(row: AgentRow, target: string) { if (!row.hive) return; - const impliedCurrent = - row.wanted ?? (row.snapshot?.running ? "up" : "offline"); - const target = impliedCurrent === "up" ? "offline" : "up"; - if ( - target === "offline" && - // Native confirm, not a `Dialog` — no confirm-dialog component - // exists in swarm-ui yet, and stop is the one disruptive direction - // here (start isn't gated). A swarm-level agent destroy, unlike - // this reversible declare, is the irreversible case that will - // justify a real one. - !window.confirm( - `Declare ${row.name} offline? The hive brings its container down on its next reconcile sweep.`, - ) - ) { - return; - } setPendingAgents((prev) => new Set(prev).add(row.name)); setActionErrors((prev) => { const next = new Map(prev); @@ -206,6 +196,41 @@ export function AgentsPage() { } } + // Declares the opposite of `row`'s current state. A row with no + // declaration yet (`wanted === null`) has nothing to flip, so the + // target is read off the agent's own last-reported `running` instead + // — the button's first click always means "make the declaration match + // reality, then flip it", which is the only reading that makes sense + // without a declaration to toggle. + async function toggleWanted(row: AgentRow) { + if (!row.hive) return; + const impliedCurrent = + row.wanted ?? (row.snapshot?.running ? "up" : "offline"); + const target = impliedCurrent === "up" ? "offline" : "up"; + if ( + target === "offline" && + // Native confirm, not a `Dialog` — stop is reversible (a later + // start un-does it), so the lighter-weight native prompt is + // proportionate here. Destroy is the irreversible direction and + // gets the real `Dialog` confirm instead — see `destroyAgent`. + !window.confirm( + `Declare ${row.name} offline? The hive brings its container down on its next reconcile sweep.`, + ) + ) { + return; + } + await declareState(row, target); + } + + // Fires from the confirm `Dialog`, never directly off a row click — + // see `destroyTarget`/the "destroy" badge in the `wanted` column. + // Unlike `toggleWanted`, there's no "current state" to read: destroy + // is a one-way declaration, not a flip. + async function destroyAgent(row: AgentRow) { + setDestroyTarget(null); + await declareState(row, "destroyed"); + } + const columns: TableColumn[] = [ { key: "name", @@ -275,7 +300,11 @@ export function AgentsPage() { // badge toggles it — `Badge`'s own "chip plus, optionally, the // control" shape (see its header comment, which names pause/resume // as the exact motivating case), not a separate status chip next - // to a separate button. + // to a separate button. Destroy is a second, `quiet`-variant badge + // next to it rather than a third toggle state: folding it into the + // same click target would make one wrong click irreversible, and + // `wanted === "destroyed"` already reads fine as this badge's own + // display value without a dedicated affordance to view it. sortBy: (a) => a.wanted ?? "", filterValue: (a) => a.wanted ?? "no declaration", render: (a) => { @@ -285,6 +314,9 @@ export function AgentsPage() { const actionLabel = impliedCurrent === "up" ? "stop" : "start"; const tone: BadgeTone = a.wanted === "up" ? "positive" : "neutral"; const err = actionErrors.get(a.name); + // Already destroyed, or nothing to destroy against — same guard + // shape as the wanted toggle's own `!a.hive` check. + const destroyable = a.hive && a.wanted !== "destroyed"; return ( <> + {destroyable ? ( + setDestroyTarget(a)} + disabled={pending} + title={`destroy ${a.name} — tears the container down, irreversible`} + /> + ) : null} {err ? ( + setDestroyTarget(null)} + label="destroy agent" + > + {destroyTarget ? ( +
+

+ Destroy {destroyTarget.name}? The hive tears its + container down on its next reconcile sweep. This is not reversible + from here — bringing it back means redeploying via "+ agent", + which reuses the agent's existing identity, config repo, and forge + collaborator access rather than starting over. +

+
+ + +
+
+ ) : null} + ); } From 82a4324b17043fc0c6618c875452d9edf2f6eae5 Mon Sep 17 00:00:00 2001 From: iris Date: Mon, 7 Sep 2026 19:02:23 +0200 Subject: [PATCH 2/4] swarm-ui: extract ConfirmDialog, use it for the destroy confirm mara, reviewing the swarm-ui destroy-trigger PR: "why the extra styling? shouldnt there be a component that does this already?" There wasnt one -- Dialog is deliberately content-agnostic (see its own file-top comment), so the destroy confirm had grown its own page-scoped .agents-destroy-confirm/-actions CSS for what is really a generic "message + cancel/confirm button row" shape. Extracted ui/confirm-dialog/ConfirmDialog.tsx: wraps Dialog, owns the button row, leaves the message body to the caller via children. AgentsPage now uses it instead of a bare Dialog + bespoke CSS; deleted the now-unused AgentsPage.css. --- .../swarm-ui/src/pages/AgentsPage.css | 17 ------ .../swarm-ui/src/pages/AgentsPage.tsx | 35 +++++------- .../src/ui/confirm-dialog/ConfirmDialog.css | 13 +++++ .../src/ui/confirm-dialog/ConfirmDialog.tsx | 55 +++++++++++++++++++ 4 files changed, 81 insertions(+), 39 deletions(-) delete mode 100644 frontend/packages/swarm-ui/src/pages/AgentsPage.css create mode 100644 frontend/packages/swarm-ui/src/ui/confirm-dialog/ConfirmDialog.css create mode 100644 frontend/packages/swarm-ui/src/ui/confirm-dialog/ConfirmDialog.tsx diff --git a/frontend/packages/swarm-ui/src/pages/AgentsPage.css b/frontend/packages/swarm-ui/src/pages/AgentsPage.css deleted file mode 100644 index ad1b4542..00000000 --- a/frontend/packages/swarm-ui/src/pages/AgentsPage.css +++ /dev/null @@ -1,17 +0,0 @@ -/* — the destroy-confirm dialog's own layout. Everything - else on this page (the table, the wanted/destroy badges) draws chrome - from the shared `ui/` kit and needs nothing page-scoped; this file - exists only because the confirm dialog's copy + button row needed - somewhere to live, same reasoning as CreateAgentForm.css owning its - own layout next to the shared form-kit chrome it wraps. */ -.agents-destroy-confirm { - display: flex; - flex-direction: column; - gap: 1em; - max-width: 28em; -} -.agents-destroy-confirm-actions { - display: flex; - justify-content: flex-end; - gap: 0.75em; -} diff --git a/frontend/packages/swarm-ui/src/pages/AgentsPage.tsx b/frontend/packages/swarm-ui/src/pages/AgentsPage.tsx index b8ee4b5c..680f266b 100644 --- a/frontend/packages/swarm-ui/src/pages/AgentsPage.tsx +++ b/frontend/packages/swarm-ui/src/pages/AgentsPage.tsx @@ -33,6 +33,7 @@ import { ApiErrorPanel } from "@hive/shared/api-error-panel.js"; import { readApiError, type ProblemDetails } from "@hive/shared/api-error.js"; import { Badge, type BadgeTone } from "@hive/shared/badge.js"; import { Button } from "../ui/button/Button.js"; +import { ConfirmDialog } from "../ui/confirm-dialog/ConfirmDialog.js"; import { Dialog } from "../ui/dialog/Dialog.js"; import { Panel } from "../ui/panel/Panel.js"; import { RelativeTime } from "../ui/relative-time/RelativeTime.js"; @@ -43,7 +44,6 @@ import { } from "../ui/refresh-interval/RefreshInterval.js"; import { Table, type TableColumn } from "../ui/table/Table.js"; import { CreateAgentForm } from "./CreateAgentForm.js"; -import "./AgentsPage.css"; interface ConfigPrStatus { pr_number: number; @@ -415,32 +415,23 @@ export function AgentsPage() { > - setDestroyTarget(null)} label="destroy agent" + onCancel={() => setDestroyTarget(null)} + onConfirm={() => destroyTarget && void destroyAgent(destroyTarget)} + confirmLabel="destroy" > {destroyTarget ? ( -
-

- Destroy {destroyTarget.name}? The hive tears its - container down on its next reconcile sweep. This is not reversible - from here — bringing it back means redeploying via "+ agent", - which reuses the agent's existing identity, config repo, and forge - collaborator access rather than starting over. -

-
- - -
-
+

+ Destroy {destroyTarget.name}? The hive tears its + container down on its next reconcile sweep. This is not reversible + from here — bringing it back means redeploying via "+ agent", which + reuses the agent's existing identity, config repo, and forge + collaborator access rather than starting over. +

) : null} - + ); } diff --git a/frontend/packages/swarm-ui/src/ui/confirm-dialog/ConfirmDialog.css b/frontend/packages/swarm-ui/src/ui/confirm-dialog/ConfirmDialog.css new file mode 100644 index 00000000..bec51820 --- /dev/null +++ b/frontend/packages/swarm-ui/src/ui/confirm-dialog/ConfirmDialog.css @@ -0,0 +1,13 @@ +/* — body + button-row layout only; `Dialog.css` owns the + surrounding modal chrome, `Button.css` owns the buttons themselves. */ +.confirm-dialog { + display: flex; + flex-direction: column; + gap: 1em; + max-width: 28em; +} +.confirm-dialog-actions { + display: flex; + justify-content: flex-end; + gap: 0.75em; +} diff --git a/frontend/packages/swarm-ui/src/ui/confirm-dialog/ConfirmDialog.tsx b/frontend/packages/swarm-ui/src/ui/confirm-dialog/ConfirmDialog.tsx new file mode 100644 index 00000000..b73badf7 --- /dev/null +++ b/frontend/packages/swarm-ui/src/ui/confirm-dialog/ConfirmDialog.tsx @@ -0,0 +1,55 @@ +// — a `Dialog` pre-wired for the "message + cancel/confirm +// button row" shape every confirm-before-acting flow needs. Extracted out +// of AgentsPage's destroy confirmation on mara's review question ("why the +// extra styling? shouldnt there be a component that does this already?") +// — there wasn't one yet, so that page had grown its own +// `.agents-destroy-confirm`/`-actions` layout CSS for what turns out to be +// a generic shape. `Dialog` itself stays opinion-free about content (see +// its own file-top comment); this is the one layer up that isn't, so a +// future confirm flow gets the button row for free instead of another +// page-scoped CSS file. +// +// The message itself stays caller-owned via `children` — only the dialog +// wiring, spacing, and button row are shared. +import type { ComponentChildren } from "preact"; +import { Button } from "../button/Button.js"; +import { Dialog } from "../dialog/Dialog.js"; +import "./ConfirmDialog.css"; + +export function ConfirmDialog({ + open, + label, + onCancel, + onConfirm, + confirmLabel = "confirm", + cancelLabel = "cancel", + confirmDisabled = false, + children, +}: { + open: boolean; + label: string; + onCancel: () => void; + onConfirm: () => void; + confirmLabel?: string; + cancelLabel?: string; + confirmDisabled?: boolean; + children: ComponentChildren; +}) { + return ( + +
+
{children}
+
+ + +
+
+ + ); +} From 2874badd4c116d0dcb1cfb69c60766eea1917438 Mon Sep 17 00:00:00 2001 From: iris Date: Mon, 7 Sep 2026 19:07:21 +0200 Subject: [PATCH 3/4] swarm-ui: use ConfirmDialog for the stop confirmation too mara: "use the new component where we already have confirm dialogs." The stop/offline direction of the wanted toggle used a native window.confirm -- the reasoning at the time was that a hand-rolled Dialog felt heavy for a reversible action, with the real Dialog confirm reserved for the irreversible destroy direction. Now that ConfirmDialog is a one-line-per-caller shared component, that argument no longer buys consistency anything, so both directions confirm the same way. --- .../swarm-ui/src/pages/AgentsPage.tsx | 70 +++++++++++++------ 1 file changed, 48 insertions(+), 22 deletions(-) diff --git a/frontend/packages/swarm-ui/src/pages/AgentsPage.tsx b/frontend/packages/swarm-ui/src/pages/AgentsPage.tsx index 680f266b..b24cc6a9 100644 --- a/frontend/packages/swarm-ui/src/pages/AgentsPage.tsx +++ b/frontend/packages/swarm-ui/src/pages/AgentsPage.tsx @@ -25,9 +25,9 @@ // itself (`CreateAgentForm`) mounts inside a `Dialog` here rather than // its own route. // -// The "wanted" column is the start/stop control — see its own `render` -// and `toggleWanted` below for how. Destroy is a separate control next -// to it — see `destroyTarget`/`destroyAgent`. +// The "wanted" column is the start/stop control (`toggleWanted`) plus a +// separate destroy control (`destroyAgent`) — both confirm via the +// shared `ui/confirm-dialog`. import { useState } from "preact/hooks"; import { ApiErrorPanel } from "@hive/shared/api-error-panel.js"; import { readApiError, type ProblemDetails } from "@hive/shared/api-error.js"; @@ -119,8 +119,13 @@ export function AgentsPage() { >(new Map()); // The row pending destroy confirmation, `null` when the dialog is // closed — not a boolean, so the dialog can name the agent without a - // second piece of state to keep in sync with it. + // second piece of state to keep in sync with it. `stopTarget` mirrors + // it for the "declare offline" direction of the same toggle — two + // separate pieces of state (not one "pending confirm" union) since + // both `ConfirmDialog`s can't be open at once anyway and keeping them + // apart means each one's JSX below reads standalone. const [destroyTarget, setDestroyTarget] = useState(null); + const [stopTarget, setStopTarget] = useState(null); async function refresh() { const res = await fetch("/api/agents/status"); @@ -139,11 +144,11 @@ export function AgentsPage() { refresh().catch((e: unknown) => setError({ detail: String(e) })); }); - // Shared by `toggleWanted` and `destroyAgent` — both are "PUT a new + // Shared by `confirmStop` and `destroyAgent` — both are "PUT a new // `wanted` declaration, track per-agent pending/error state, patch the // response back into `rows`" with nothing else distinguishing them. - // Confirmation (native vs. `Dialog`) and target-state selection stay - // in each caller, since those are the parts that actually differ. + // Which `ConfirmDialog` fires it and the target state stay in each + // caller, since those are the parts that actually differ. async function declareState(row: AgentRow, target: string) { if (!row.hive) return; setPendingAgents((prev) => new Set(prev).add(row.name)); @@ -201,31 +206,38 @@ export function AgentsPage() { // target is read off the agent's own last-reported `running` instead // — the button's first click always means "make the declaration match // reality, then flip it", which is the only reading that makes sense - // without a declaration to toggle. + // without a declaration to toggle. The "up" direction needs no + // confirmation and declares straight away; "offline" opens the + // `stopTarget` `ConfirmDialog` instead of declaring directly — see + // `confirmStop`. async function toggleWanted(row: AgentRow) { if (!row.hive) return; const impliedCurrent = row.wanted ?? (row.snapshot?.running ? "up" : "offline"); const target = impliedCurrent === "up" ? "offline" : "up"; - if ( - target === "offline" && - // Native confirm, not a `Dialog` — stop is reversible (a later - // start un-does it), so the lighter-weight native prompt is - // proportionate here. Destroy is the irreversible direction and - // gets the real `Dialog` confirm instead — see `destroyAgent`. - !window.confirm( - `Declare ${row.name} offline? The hive brings its container down on its next reconcile sweep.`, - ) - ) { + if (target === "offline") { + setStopTarget(row); return; } await declareState(row, target); } - // Fires from the confirm `Dialog`, never directly off a row click — - // see `destroyTarget`/the "destroy" badge in the `wanted` column. - // Unlike `toggleWanted`, there's no "current state" to read: destroy - // is a one-way declaration, not a flip. + // Fires from the `stopTarget` confirm `Dialog`. Used to be a native + // `window.confirm` (stop is reversible, a later start un-does it, so a + // lighter-weight prompt than destroy's felt proportionate) — mara, + // reviewing the destroy confirm: use the shared component everywhere + // we already confirm before acting, not just for destroy. Once + // `ConfirmDialog` existed as a one-line-per-caller component, the + // "native is lighter" argument no longer bought consistency anything. + async function confirmStop(row: AgentRow) { + setStopTarget(null); + await declareState(row, "offline"); + } + + // Fires from the `destroyTarget` confirm `Dialog`, never directly off + // a row click — see `destroyTarget`/the "destroy" badge in the + // `wanted` column. Unlike `toggleWanted`, there's no "current state" + // to read: destroy is a one-way declaration, not a flip. async function destroyAgent(row: AgentRow) { setDestroyTarget(null); await declareState(row, "destroyed"); @@ -415,6 +427,20 @@ export function AgentsPage() { > + setStopTarget(null)} + onConfirm={() => stopTarget && void confirmStop(stopTarget)} + confirmLabel="stop" + > + {stopTarget ? ( +

+ Declare {stopTarget.name} offline? The hive brings + its container down on its next reconcile sweep. +

+ ) : null} + Date: Mon, 7 Sep 2026 19:11:50 +0200 Subject: [PATCH 4/4] swarm-ui: disable the wanted-toggle badge on a destroyed row argus, review: the primary wanted-toggle badge still had no wanted === "destroyed" guard, so it stayed clickable on a destroyed row and would PUT {state: "up"} on click -- directly contradicting the destroy confirm dialogs own "not reversible from here" copy. Missed this in the previous fix-up (which only addressed the ConfirmDialog-for-stop ask). Added the guard to both the click handler and disabled, plus a title explaining why on a destroyed row. --- .../swarm-ui/src/pages/AgentsPage.tsx | 20 ++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/frontend/packages/swarm-ui/src/pages/AgentsPage.tsx b/frontend/packages/swarm-ui/src/pages/AgentsPage.tsx index b24cc6a9..6da04c0a 100644 --- a/frontend/packages/swarm-ui/src/pages/AgentsPage.tsx +++ b/frontend/packages/swarm-ui/src/pages/AgentsPage.tsx @@ -329,17 +329,27 @@ export function AgentsPage() { // Already destroyed, or nothing to destroy against — same guard // shape as the wanted toggle's own `!a.hive` check. const destroyable = a.hive && a.wanted !== "destroyed"; + const destroyed = a.wanted === "destroyed"; return ( <> void toggleWanted(a) : undefined} - disabled={pending || !a.hive} + // `destroyed` blocks the click too, not just the label — + // argus caught this in review: without it, a click here + // still computed `target = "up"` and PUT a live + // declaration on a destroyed row, contradicting the + // destroy confirm's own "not reversible from here" copy. + onClick={ + a.hive && !destroyed ? () => void toggleWanted(a) : undefined + } + disabled={pending || !a.hive || destroyed} title={ - a.hive - ? `click to ${actionLabel} ${a.name}` - : "no hive on record for this agent — nothing to declare against" + destroyed + ? `${a.name} is destroyed — redeploy via "+ agent" to bring it back` + : a.hive + ? `click to ${actionLabel} ${a.name}` + : "no hive on record for this agent — nothing to declare against" } /> {destroyable ? (