diff --git a/frontend/packages/swarm-ui/src/pages/AgentsPage.tsx b/frontend/packages/swarm-ui/src/pages/AgentsPage.tsx index 14010a1a..6da04c0a 100644 --- a/frontend/packages/swarm-ui/src/pages/AgentsPage.tsx +++ b/frontend/packages/swarm-ui/src/pages/AgentsPage.tsx @@ -25,13 +25,15 @@ // 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. +// 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"; 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"; @@ -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,15 @@ 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. `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"); @@ -132,30 +144,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 `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. + // 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; - 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 +201,48 @@ 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. 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") { + setStopTarget(row); + return; + } + await declareState(row, target); + } + + // 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"); + } + const columns: TableColumn[] = [ { key: "name", @@ -275,7 +312,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,19 +326,42 @@ 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"; + 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 ? ( + setDestroyTarget(a)} + disabled={pending} + title={`destroy ${a.name} — tears the container down, irreversible`} + /> + ) : null} {err ? ( + 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} +
+ 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. +

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