diff --git a/docs/WORKTREES.md b/docs/WORKTREES.md index 846dcc4..4f10835 100644 --- a/docs/WORKTREES.md +++ b/docs/WORKTREES.md @@ -79,6 +79,11 @@ would have blocked. A tree whose guard is clear never needed the override, so a forced kill removes it as usual — force changes what happens to blocked work, not ordinary cleanup. +In the web client the kill button sends the unforced `DELETE` straight away — +there is no pre-confirmation, because the daemon is the one that decides. Only +the refusal raises a prompt, and that prompt names what is in the way and offers +"Force kill", which repeats the request with `force=true`. + Ending a session and destroying its work are two separate acts. Maestro only performs the first one. Removing a tree it left behind is the operator's own `git worktree remove`, chosen while looking at exactly what would be lost. diff --git a/web/src/api.ts b/web/src/api.ts index bee6c1b..50f6e20 100644 --- a/web/src/api.ts +++ b/web/src/api.ts @@ -1,6 +1,7 @@ import * as routes from "./routes.js"; import type { ApiError, + InTheWay, Branches, CreateSessionRequest, DaemonInfo, @@ -13,19 +14,38 @@ import type { Uuid, } from "./types.js"; +/** A `/api/*` failure with the daemon's own error envelope kept intact, so a + * caller can branch on `code` — the kill guard's `kill_guarded` (#79) — and + * render `unsavedWork` rather than re-parsing the message. */ +export class ApiRequestError extends Error { + readonly status: number; + readonly code: string; + readonly detail: string; + readonly unsavedWork: InTheWay | null; + + constructor(res: Response, info?: ApiError["error"], text?: string) { + const detail = info ? `${info.code}: ${info.message}` : (text ?? ""); + super(`${res.status} ${res.statusText}${detail ? ` — ${detail}` : ""}`); + this.name = "ApiRequestError"; + this.status = res.status; + this.code = info?.code ?? ""; + this.detail = info?.message ?? text ?? ""; + this.unsavedWork = info?.unsaved_work ?? null; + } +} + async function jsonOrError(res: Response): Promise { if (res.ok) { if (res.status === 204) return undefined as unknown as T; return (await res.json()) as T; } - let detail = ""; try { const body = (await res.json()) as ApiError; - detail = `${body.error.code}: ${body.error.message}`; - } catch { - detail = await res.text(); + throw new ApiRequestError(res, body.error); + } catch (e) { + if (e instanceof ApiRequestError) throw e; + throw new ApiRequestError(res, undefined, await res.text().catch(() => "")); } - throw new Error(`${res.status} ${res.statusText}${detail ? ` — ${detail}` : ""}`); } export async function listSessions(): Promise { @@ -57,9 +77,14 @@ export async function renameSession(id: Uuid, name: string): Promise { ); } -export async function killSession(id: Uuid): Promise { +/** Kill a session. With `force` (#81) the daemon overrides its refusal over a + * worktree holding unsaved work and leaves that worktree on disk. */ +export async function killSession(id: Uuid, force = false): Promise { + const url = force + ? `${routes.session(id)}?${routes.FORCE_QUERY}=true` + : routes.session(id); await jsonOrError( - await fetch(routes.session(id), { + await fetch(url, { method: "DELETE", credentials: "same-origin", }), diff --git a/web/src/catalog.test.ts b/web/src/catalog.test.ts index 3d4c576..fd0e4ab 100644 --- a/web/src/catalog.test.ts +++ b/web/src/catalog.test.ts @@ -1,5 +1,5 @@ // @vitest-environment happy-dom -import { beforeAll, describe, expect, it } from "vitest"; +import { afterEach, beforeAll, describe, expect, it } from "vitest"; import { renderSidebar } from "./catalog.js"; import type { CatalogListener, CatalogStore } from "./events.js"; @@ -16,6 +16,10 @@ beforeAll(() => { window.fetch = () => Promise.reject(new Error("offline")); }); +afterEach(() => { + document.querySelector(".kill-dialog")?.remove(); +}); + function session(overrides: Partial): Session { return { id: "11111111-2222-3333-4444-555555555555", @@ -46,6 +50,11 @@ function rowFor(el: HTMLElement, id: string): HTMLElement | null { return el.querySelector(`[data-session-id="${id}"]`); } +/** Let the kill chain's awaits (fetch, its json body) settle. */ +async function flush(): Promise { + for (let i = 0; i < 8; i++) await Promise.resolve(); +} + describe("blocked-work indicator (#83)", () => { it("marks the row of a blocked session in diff notation", () => { const blocked = session({ @@ -89,37 +98,77 @@ describe("blocked-work indicator (#83)", () => { expect(rowFor(el, unprobed.id)?.querySelector(".leaf-blocked")).toBeNull(); }); - it("restates the warning at the kill confirm", async () => { - const confirmations: string[] = []; - window.confirm = (msg?: string) => { - confirmations.push(msg ?? ""); - return false; - }; + it("kills straight away, without a first confirmation", async () => { + const calls: string[] = []; + window.fetch = ((url: string, init?: RequestInit) => { + calls.push(`${init?.method ?? "GET"} ${url}`); + return Promise.resolve(new Response(null, { status: 204 })); + }) as typeof fetch; + const plain = session({ id: "aaaaaaaa-0000-0000-0000-000000000005" }); + const el = renderSidebar(storeWith([plain])).el; + rowFor(el, plain.id)?.querySelector(".leaf-kill")?.click(); + await flush(); + expect(calls).toContain(`DELETE /api/sessions/${plain.id}`); + expect(document.querySelector(".kill-dialog")).toBeNull(); + }); + + it("offers the override when the daemon refuses a guarded kill", async () => { + const calls: string[] = []; + window.fetch = ((url: string, init?: RequestInit) => { + calls.push(`${init?.method ?? "GET"} ${url}`); + if (url.includes("force=true")) return Promise.resolve(new Response(null, { status: 204 })); + return Promise.resolve( + new Response( + JSON.stringify({ + error: { + code: "kill_guarded", + message: "worktree has unsaved work", + unsaved_work: { untracked_files: 1 }, + }, + }), + { status: 409, headers: { "content-type": "application/json" } }, + ), + ); + }) as typeof fetch; const blocked = session({ id: "aaaaaaaa-0000-0000-0000-000000000004", command: "bash", blocked_work: { untracked_files: 1 }, }); const el = renderSidebar(storeWith([blocked])).el; - const kill = rowFor(el, blocked.id)?.querySelector(".leaf-kill"); - kill?.click(); - await Promise.resolve(); - expect(confirmations).toHaveLength(1); - expect(confirmations[0]).toContain("1 untracked file"); - expect(confirmations[0]).toContain("refused unless forced"); + rowFor(el, blocked.id)?.querySelector(".leaf-kill")?.click(); + await flush(); + const dialog = document.querySelector(".kill-dialog"); + expect(dialog?.textContent).toContain("1 untracked file"); + dialog?.querySelector(".kill-dialog-force")?.click(); + await flush(); + expect(calls).toContain(`DELETE /api/sessions/${blocked.id}?force=true`); + expect(document.querySelector(".kill-dialog")).toBeNull(); }); - it("confirms plainly when nothing blocks", async () => { - const confirmations: string[] = []; - window.confirm = (msg?: string) => { - confirmations.push(msg ?? ""); - return false; - }; - const plain = session({ id: "aaaaaaaa-0000-0000-0000-000000000005" }); - const el = renderSidebar(storeWith([plain])).el; - rowFor(el, plain.id)?.querySelector(".leaf-kill")?.click(); - await Promise.resolve(); - expect(confirmations).toEqual(['Kill session "claude"?']); + it("closes the override prompt without killing on cancel", async () => { + const calls: string[] = []; + window.fetch = ((url: string, init?: RequestInit) => { + calls.push(`${init?.method ?? "GET"} ${url}`); + return Promise.resolve( + new Response( + JSON.stringify({ + error: { code: "kill_guarded", message: "worktree has unsaved work" }, + }), + { status: 409, headers: { "content-type": "application/json" } }, + ), + ); + }) as typeof fetch; + const blocked = session({ + id: "aaaaaaaa-0000-0000-0000-000000000006", + blocked_work: { unpushed_commits: 2 }, + }); + const el = renderSidebar(storeWith([blocked])).el; + rowFor(el, blocked.id)?.querySelector(".leaf-kill")?.click(); + await flush(); + document.querySelector(".kill-dialog-cancel")?.click(); + expect(document.querySelector(".kill-dialog")).toBeNull(); + expect(calls.filter((c) => c.includes("force=true"))).toHaveLength(0); }); }); diff --git a/web/src/catalog.ts b/web/src/catalog.ts index d1fc206..c4e5a00 100644 --- a/web/src/catalog.ts +++ b/web/src/catalog.ts @@ -1,5 +1,6 @@ import { escapeHtml } from "./html.js"; -import { getDaemonInfo, killSession, renameSession } from "./api.js"; +import { ApiRequestError, getDaemonInfo, killSession, renameSession } from "./api.js"; +import { openForceKillDialog } from "./killDialog.js"; import { openContextMenu } from "./contextMenu.js"; import { isTypingTarget, toggleHelp } from "./help.js"; import { CatalogStore } from "./events.js"; @@ -180,7 +181,7 @@ function renderNode(node: TreeNode, depth: number): HTMLElement { killBtn.addEventListener("click", (ev) => { ev.preventDefault(); ev.stopPropagation(); - void confirmAndKill(s, name); + void requestKill(s, name); }); row.addEventListener("contextmenu", (ev) => { ev.preventDefault(); @@ -192,22 +193,31 @@ function renderNode(node: TreeNode, depth: number): HTMLElement { onSelect: () => document.dispatchEvent(new CustomEvent(SPAWN_SIMILAR_EVENT, { detail: s })), }, - { label: "Kill session", danger: true, onSelect: () => void confirmAndKill(s, name) }, + { label: "Kill session", danger: true, onSelect: () => void requestKill(s, name) }, ]); }); return row; } -/** #83: the confirm restates the warning at the moment of decision. */ -async function confirmAndKill(s: Session, name: string): Promise { - const blockers = sessionBlockers(s); - const question = blockers - ? `Kill session "${name}"? Its worktree still has ${formatInTheWay(blockers)} — the kill will be refused unless forced.` - : `Kill session "${name}"?`; - if (!confirm(question)) return; +/** + * Kill on the first click (#81): the daemon is the one that decides whether a + * session may go, and it refuses over a worktree holding unsaved work. Only + * that refusal is worth a prompt, and it needs one `confirm()` cannot give — + * a forced kill. Anything else surfaces as an alert. + */ +async function requestKill(s: Session, name: string): Promise { try { await killSession(s.id); } catch (e) { + if (e instanceof ApiRequestError && e.code === "kill_guarded") { + openForceKillDialog({ + name, + blockers: e.unsavedWork ?? sessionBlockers(s), + reason: e.detail, + onForce: () => killSession(s.id, true), + }); + return; + } alert(`Failed to kill: ${(e as Error).message}`); } } diff --git a/web/src/killDialog.ts b/web/src/killDialog.ts new file mode 100644 index 0000000..3597f82 --- /dev/null +++ b/web/src/killDialog.ts @@ -0,0 +1,104 @@ +/** + * Forced-kill prompt (#81). + * + * The daemon refuses to kill a session whose worktree still holds uncommitted + * or unpushed work. `confirm()` can only ask the same question twice, so the + * refusal gets its own dialog: it names what is in the way and offers the + * override the daemon's `force=true` already implements. A kill the daemon + * accepts never reaches here — there is nothing to warn about. + */ +import { formatInTheWay, type InTheWay } from "./types.js"; + +export interface ForceKillPrompt { + /** Session name, as the sidebar shows it. */ + name: string; + /** What the daemon reported in the way, when it reported anything. */ + blockers: InTheWay | null; + /** Message from the daemon's refusal, shown when there is no breakdown. */ + reason: string; + /** Run the forced kill. A rejection is rendered in the dialog, which stays + * open so the override can be retried. */ + onForce: () => Promise; +} + +let open: { el: HTMLElement; onKeydown: (ev: KeyboardEvent) => void } | null = null; + +export function closeForceKillDialog(): void { + if (!open) return; + window.removeEventListener("keydown", open.onKeydown, true); + open.el.remove(); + open = null; +} + +/** Mount the prompt. Opening while one is up replaces it, so a second refusal + * can never stack two dialogs. */ +export function openForceKillDialog(p: ForceKillPrompt): HTMLElement { + closeForceKillDialog(); + + const el = document.createElement("div"); + el.className = "kill-dialog"; + el.setAttribute("role", "dialog"); + el.setAttribute("aria-modal", "true"); + el.setAttribute("aria-label", `Kill ${p.name}`); + + const panel = document.createElement("div"); + panel.className = "kill-dialog-panel"; + + const title = document.createElement("h2"); + title.className = "kill-dialog-title"; + title.textContent = `Kill "${p.name}"?`; + + const body = document.createElement("p"); + body.className = "kill-dialog-body"; + body.textContent = p.blockers + ? `Its worktree still has ${formatInTheWay(p.blockers)}. Forcing the kill removes the session and leaves the worktree on disk.` + : `${p.reason} Forcing the kill removes the session and leaves the worktree on disk.`; + + const error = document.createElement("p"); + error.className = "kill-dialog-error"; + error.hidden = true; + + const actions = document.createElement("div"); + actions.className = "kill-dialog-actions"; + const cancel = document.createElement("button"); + cancel.type = "button"; + cancel.className = "btn btn-ghost kill-dialog-cancel"; + cancel.textContent = "Cancel"; + cancel.addEventListener("click", closeForceKillDialog); + const force = document.createElement("button"); + force.type = "button"; + force.className = "btn btn-danger kill-dialog-force"; + force.textContent = "Force kill"; + force.addEventListener("click", () => { + force.disabled = true; + void p + .onForce() + .then(closeForceKillDialog) + .catch((e: unknown) => { + force.disabled = false; + error.hidden = false; + error.textContent = `Failed to kill: ${(e as Error).message}`; + }); + }); + actions.append(cancel, force); + + panel.append(title, body, error, actions); + + // A press on the scrim cancels; one inside the panel is left alone. + el.addEventListener("click", (ev) => { + if (!panel.contains(ev.target as Node)) closeForceKillDialog(); + }); + el.appendChild(panel); + document.body.appendChild(el); + + const onKeydown = (ev: KeyboardEvent): void => { + if (ev.key !== "Escape") return; + ev.preventDefault(); + ev.stopPropagation(); + closeForceKillDialog(); + }; + window.addEventListener("keydown", onKeydown, true); + open = { el, onKeydown }; + cancel.focus(); + return el; +} diff --git a/web/src/routes.ts b/web/src/routes.ts index 812160a..69ab42c 100644 --- a/web/src/routes.ts +++ b/web/src/routes.ts @@ -18,6 +18,10 @@ export const EVENTS = "/api/events"; /** Name of the query parameter `DIRS` and `GITROOT` both take. */ export const PATH_QUERY = "path"; +/** Name of the query parameter `DELETE /api/sessions/:id` takes (#81): + * `force=true` overrides the kill refusal over a guarded worktree. */ +export const FORCE_QUERY = "force"; + /** Name of the query parameter `NAMES` takes. */ export const COUNT_QUERY = "count"; diff --git a/web/src/styles.css b/web/src/styles.css index c22ab59..77ce344 100644 --- a/web/src/styles.css +++ b/web/src/styles.css @@ -1058,6 +1058,64 @@ input.leaf-rename:focus { border-color: color-mix(in oklab, var(--danger) 40%, transparent); } +/* ---------- forced-kill prompt (#81) ---------- */ +.kill-dialog { + position: fixed; + inset: 0; + z-index: var(--z-modal); + display: grid; + place-items: center; + padding: var(--sp-4); + background: oklch(0 0 0 / 0.5); + backdrop-filter: blur(4px); +} + +.kill-dialog-panel { + width: min(400px, 100%); + padding: var(--sp-4); + background: var(--surface); + border: 1px solid var(--border-strong); + border-radius: var(--r-3); + box-shadow: var(--shadow-2); +} + +.kill-dialog-title { + margin: 0 0 var(--sp-2); + font-family: var(--font-mono); + font-size: var(--fs-15); + color: var(--ink); +} + +.kill-dialog-body { + margin: 0; + color: var(--muted); + font-size: var(--fs-13); + line-height: 1.5; +} + +.kill-dialog-error { + margin: var(--sp-2) 0 0; + color: var(--danger); + font-size: var(--fs-13); +} + +.kill-dialog-actions { + display: flex; + justify-content: flex-end; + gap: var(--sp-2); + margin-top: var(--sp-4); +} + +.btn-danger { + color: var(--danger); + border-color: color-mix(in oklab, var(--danger) 40%, transparent); +} + +.btn-danger:hover { + background: var(--danger-tint); + border-color: var(--danger); +} + /* ---------- empty / loading ---------- */ .empty { diff --git a/web/src/types.ts b/web/src/types.ts index c86ceb8..12ffe49 100644 --- a/web/src/types.ts +++ b/web/src/types.ts @@ -253,7 +253,12 @@ export type ClientControl = | { type: "Error"; code: string; message: string }; export interface ApiError { - error: { code: string; message: string }; + error: { + code: string; + message: string; + /** Set only by the kill guard (#79): what blocked the unforced kill. */ + unsaved_work?: InTheWay; + }; } export function sessionName(s: Session): string {