feat(maestro): force-kill prompt in the web client
The kill button now sends the unforced DELETE immediately instead of asking first. Only the daemon's guard refusal raises a prompt, and it is a real dialog rather than confirm(), so it can offer the override the daemon already implements. The API error envelope is decoded into ApiRequestError, keeping the code and the unsaved-work breakdown so the dialog names what is in the way.
This commit is contained in:
@@ -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.
|
||||
|
||||
+32
-7
@@ -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<T>(res: Response): Promise<T> {
|
||||
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<Session[]> {
|
||||
@@ -57,9 +77,14 @@ export async function renameSession(id: Uuid, name: string): Promise<Session> {
|
||||
);
|
||||
}
|
||||
|
||||
export async function killSession(id: Uuid): Promise<void> {
|
||||
/** 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<void> {
|
||||
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",
|
||||
}),
|
||||
|
||||
+73
-24
@@ -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>): Session {
|
||||
return {
|
||||
id: "11111111-2222-3333-4444-555555555555",
|
||||
@@ -46,6 +50,11 @@ function rowFor(el: HTMLElement, id: string): HTMLElement | null {
|
||||
return el.querySelector<HTMLElement>(`[data-session-id="${id}"]`);
|
||||
}
|
||||
|
||||
/** Let the kill chain's awaits (fetch, its json body) settle. */
|
||||
async function flush(): Promise<void> {
|
||||
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<HTMLButtonElement>(".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<HTMLButtonElement>(".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<HTMLButtonElement>(".leaf-kill")?.click();
|
||||
await flush();
|
||||
const dialog = document.querySelector(".kill-dialog");
|
||||
expect(dialog?.textContent).toContain("1 untracked file");
|
||||
dialog?.querySelector<HTMLButtonElement>(".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<HTMLButtonElement>(".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<HTMLButtonElement>(".leaf-kill")?.click();
|
||||
await flush();
|
||||
document.querySelector<HTMLButtonElement>(".kill-dialog-cancel")?.click();
|
||||
expect(document.querySelector(".kill-dialog")).toBeNull();
|
||||
expect(calls.filter((c) => c.includes("force=true"))).toHaveLength(0);
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
+20
-10
@@ -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<void> {
|
||||
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<void> {
|
||||
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}`);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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<void>;
|
||||
}
|
||||
|
||||
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;
|
||||
}
|
||||
@@ -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";
|
||||
|
||||
|
||||
@@ -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 {
|
||||
|
||||
+6
-1
@@ -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 {
|
||||
|
||||
Reference in New Issue
Block a user