From 8eee741594f95954e6792ad1e1c9471c9ae4afb9 Mon Sep 17 00:00:00 2001 From: Stener1 Date: Wed, 21 Oct 2026 08:55:41 +0000 Subject: [PATCH 01/26] fix(app): archiving a worktree no longer holds its dialog open (BRA-2199) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Archive… still asks, but pressing Archive closes the dialog at once. The worktree shows Archiving… and goes when its box gets to it, through the same per-box queue Remove worktree uses, now carrying the kind. The toasts are today's: Archived (branch kept), Archiving (its script is running), and a warning for uncommitted work. Bulk archive in the Worktrees table skips a worktree already on its way out, so its box never gets a second request for it. The mock takes ?slowremove=MS to make a box slow to remove a worktree, and the verify skill gains a worktree-archive scenario and feature file. Co-Authored-By: Claude Opus 5.5 --- .../skills/verify-berthd/features/README.md | 1 + .../features/worktree-archive.md | 32 ++++++++ .../scripts/scenarios/worktree-archive.mjs | 80 +++++++++++++++++++ app/e2e/worktree-remove.spec.ts | 30 ++++++- app/src/components/sidebar/actions.tsx | 20 ++--- app/src/lib/mock.ts | 7 +- app/src/lib/remove-worktree.ts | 70 ++++++++-------- app/src/lib/removing.test.ts | 23 +++++- app/src/lib/removing.ts | 28 +++++-- app/src/views/worktrees/use-bulk.ts | 11 ++- docs/changelog.mdx | 6 ++ 11 files changed, 246 insertions(+), 62 deletions(-) create mode 100644 .claude/skills/verify-berthd/features/worktree-archive.md create mode 100644 .claude/skills/verify-berthd/scripts/scenarios/worktree-archive.mjs diff --git a/.claude/skills/verify-berthd/features/README.md b/.claude/skills/verify-berthd/features/README.md index 11a07f637..95aa27181 100644 --- a/.claude/skills/verify-berthd/features/README.md +++ b/.claude/skills/verify-berthd/features/README.md @@ -17,6 +17,7 @@ The maintained list of what a person can do in the app, and how to prove each on | Shortcuts in the keyboard's layout (⌘Å and ⌘¨ for Back and Forward on Norwegian) in tooltips, the Keyboard shortcuts sheet, Settings and ⌘K; the sheet's descriptions fit their lines | [navigation-history.md](navigation-history.md) (`history-layout`) | `keyboard-layout`, `keyboard-layout-us`; `app/e2e/back-forward.spec.ts` asserts it | | Stopping agents from the Agent Dashboard: one, the picked ones, a column, Clean up; archiving their worktrees where it's safe | [dashboard-stop.md](dashboard-stop.md) | `dashboard-stop-archive`; `app/e2e/dashboard-stop-archive.spec.ts` asserts it | | Plugins: built-in and your own, what they add (screens, sidebar items, widgets, panels) | [plugins.md](plugins.md) | `plugin-nav-badges` (sidebar items); `plugin-attach-live` (`api.attach`, a real box, `--agent`) | +| Archiving and removing a worktree: Archive's dialog closes at once, Archiving… until the box is done, one at a time per box, the toasts; Remove worktree | [worktree-archive.md](worktree-archive.md) | `worktree-archive`; `app/e2e/worktree-remove.spec.ts` | | Home's widget grid | [home-widgets.md](home-widgets.md) | none yet; `app/e2e/home-widgets.spec.ts` asserts it | | Questions in chat: an agent's form of questions as a card, answered on a real box; kept with Retry and Copy when it can't be | [chat-questions.md](chat-questions.md) | `chat-questions-live` (a real box, `--agent`); `app/e2e/chat.spec.ts` on the mock | | Chat's foot and reply box: a live turn's spinner, timer, Stop and Queue; a turn that ended while its work runs on shows an hourglass and Send | [chat-turn-state.md](chat-turn-state.md) | `chat-turn-state`; `chat-turn-state-live` (a real box, `--agent`); `app/e2e/chat.spec.ts` asserts it | diff --git a/.claude/skills/verify-berthd/features/worktree-archive.md b/.claude/skills/verify-berthd/features/worktree-archive.md new file mode 100644 index 000000000..9a1e8aae0 --- /dev/null +++ b/.claude/skills/verify-berthd/features/worktree-archive.md @@ -0,0 +1,32 @@ +# Archiving and removing a worktree + +A worktree that is done goes one of two ways. **Archive…** asks first, then stops its sessions and services, runs the repo's archive script if it has one, and removes the folder; the branch stays. **Remove worktree** asks nothing and removes it; the branch stays too. Both go through one queue (`app/src/lib/remove-worktree.ts`): the worktree is marked at once (`lib/removing.ts`), and each box takes one removal at a time (`inTurn`), so archiving several in a row never waits on a dialog. + +## Sub-features + +- `archive-confirm`: the dialog names the worktree, its branch ("Branch … stays") and the archive script, if any. It closes as soon as Archive is pressed, before the box answers. +- `archive-mark`: from that moment the row is struck through and says **Archiving…** (`[data-leaving]`, `LeavingRow` in `components/sidebar/projects.tsx`); zen's switcher says the same, and the worktree's launcher says "Archiving NAME…". It can't be opened meanwhile. +- `archive-done`: without a script the box answers once the folder is gone, then a toast **Archived NAME** / "Branch … is kept." With a script the box answers at once: a toast **Archiving NAME** / "Its archive script is running on the box…", and **Archived NAME** when `worktree.removed` arrives. +- `archive-dirty`: uncommitted work makes git refuse. The worktree comes back as it was, with a warning toast **Couldn't archive NAME** / "NAME has uncommitted changes, so it was left as it is. Commit them first, or use Remove worktree to discard them." +- `archive-queue`: archives (and removals) on one box go one after another; each row keeps its mark until its turn is done. Another box's don't wait. +- `remove`: Remove worktree, the dashboard's Stop and remove worktree, and Delete in the Worktrees table: no dialog, **Removing…**, and a dirty one gives **Kept NAME** with **Remove anyway**. +- Bulk Archive in the Worktrees table (`views/worktrees/archive-dialog.tsx`) is separate: its dialog stays open on purpose, with each row's outcome, and runs one row after another across every box (`use-bulk.ts`). + +## How to get to it (user POV) + +- Right-click a worktree in the sidebar, or its ⋯: **Archive…** or **Remove worktree**. Zen's worktree menu has the same items. +- A worktree with no tabs shows its launcher: **Archive this worktree…**. +- Worktrees (sidebar): select rows, then **Archive N…** or **Delete** in the bar at the bottom. + +## Driving it with drive.mjs + +- `node .claude/skills/verify-berthd/scripts/drive.mjs worktree-archive --port $PORT` opens the mock with `slowremove=4000` (a box takes 4 s to remove a worktree with no archive script, as a big `node_modules` does) and the list sidebar. It archives `gpu/ci-flake` and fails if the dialog stays open over 1 s, checks its row says Archiving…, archives `gpu/judge-v2` straight after (both marked; judge-v2 waits its turn), waits for **Archived ci-flake** with "Branch me/ci-flake is kept." at about 4 s and **Archived judge-v2** at about 8 s, then archives `devl/qa-deck` (script `pnpm db:drop`: **Archiving qa-deck**, then **Archived qa-deck**) and `devl/checkout-fix` (uncommitted: **Couldn't archive checkout-fix**, row still there). Each step logs how long the dialog stayed open. +- Hooks: `[data-testid=worktree-row][data-worktree="BOX/NAME"]` for a row, `[data-leaving]` with the name's text for its leaving state, `getByRole("alertdialog")` for the dialog and its exact **Archive** button, `[data-slot=toast-viewport] > *` with the title's text for a toast. +- `app/e2e/worktree-remove.spec.ts` asserts the same on the mock: Remove (no dialog, Remove anyway), Archive (dialog gone in 1 s with `slowremove`, Archiving…, the toast) and Delete in the table. + +## Gotchas + +- `?slowremove=MS` slows only a worktree with no archive script; one with a script (devl's shop) answers at once and finishes 1.5 s later, as on a box. `checkout-fix` always refuses without force. +- The default sidebar is the rail, which shows one project at a time; the scenario sets `sidebar=list` to see devl's and gpu's worktrees together, as the e2e specs do. +- A worktree already marked is ignored by a second Archive or Remove until its mark goes. +- The real box's slow part (deleting a big folder inside `git worktree remove`) isn't in the mock; a real box is needed to time it (`../scripts/dev-box.sh`). diff --git a/.claude/skills/verify-berthd/scripts/scenarios/worktree-archive.mjs b/.claude/skills/verify-berthd/scripts/scenarios/worktree-archive.mjs new file mode 100644 index 000000000..3200d7612 --- /dev/null +++ b/.claude/skills/verify-berthd/scripts/scenarios/worktree-archive.mjs @@ -0,0 +1,80 @@ +// Archive… from a worktree's menu: the dialog closes as soon as Archive is +// pressed, the row says "Archiving…" until its box is done, and a toast +// says how it went. The mock's box takes 4 s to remove a worktree with no +// archive script (?slowremove), as one with a big node_modules does. +// features/worktree-archive.md. + +export const params = { slowremove: "4000", sidebar: "list" }; + +const SLOW = 4000; + +export async function run({ page, shot, step, expect }) { + const toast = (text) => page.locator("[data-slot=toast-viewport] > *", { hasText: text }); + const leaving = (name) => page.locator("[data-leaving]", { hasText: name }); + const archive = async (w) => { + await page.locator(`[data-testid=worktree-row][data-worktree="${w}"]`).click({ button: "right" }); + await page.getByRole("menuitem", { name: "Archive…" }).click(); + const dialog = page.getByRole("alertdialog"); + await expect(dialog).toBeVisible(); + return dialog; + }; + // pressArchive confirms and says how long the dialog stayed open. The + // shot 300 ms in shows it spinning if it waits on the box. + const pressArchive = async (dialog, label) => { + const t0 = Date.now(); + await dialog.getByRole("button", { name: "Archive", exact: true }).click(); + const closed = dialog.waitFor({ state: "hidden", timeout: SLOW * 3 }).then(() => Date.now() - t0); + await page.waitForTimeout(300); + await shot(label); + const ms = await closed; + step(`the dialog closed ${ms} ms after Archive was pressed`); + if (ms > 1000) throw new Error(`the dialog stayed open ${ms} ms, waiting on the box`); + return t0; + }; + + step("gpu/ci-flake (no archive script): Archive… asks first"); + const dialog = await archive("gpu/ci-flake"); + await expect(dialog).toContainText("Archive ci-flake?"); + await expect(dialog).toContainText("Branch me/ci-flake stays"); + await shot("1-confirm"); + + step("press Archive: the dialog goes at once and the row says Archiving…"); + const t0 = await pressArchive(dialog, "2-just-after-archive"); + await expect(leaving("ci-flake")).toContainText("Archiving…"); + + step("archive gpu/judge-v2 straight after: it waits its turn on gpu, marked too"); + await pressArchive(await archive("gpu/judge-v2"), "3-second-archive"); + await expect(leaving("ci-flake")).toContainText("Archiving…"); + await expect(leaving("judge-v2")).toContainText("Archiving…"); + await shot("4-both-archiving"); + + step("when gpu answers, ci-flake goes and says its branch is kept"); + await expect(toast("Archived ci-flake")).toContainText("Branch me/ci-flake is kept.", { timeout: SLOW * 2 }); + step(`Archived ci-flake toast ${Date.now() - t0} ms after Archive`); + await expect(page.locator('[data-worktree="gpu/ci-flake"]')).toHaveCount(0); + await expect(leaving("ci-flake")).toHaveCount(0); + await expect(leaving("judge-v2")).toContainText("Archiving…"); + await shot("5-first-archived"); + + step("judge-v2 goes after it, one at a time per box"); + await expect(toast("Archived judge-v2")).toContainText("Branch judge-v2 is kept.", { timeout: SLOW * 2 }); + step(`Archived judge-v2 toast ${Date.now() - t0} ms after the first Archive`); + await expect(page.locator('[data-worktree="gpu/judge-v2"]')).toHaveCount(0); + await shot("6-both-archived"); + + step("devl/qa-deck (archive script pnpm db:drop): the box runs it in the background"); + const scripted = await archive("devl/qa-deck"); + await expect(scripted).toContainText("pnpm db:drop"); + await pressArchive(scripted, "7-script-just-after-archive"); + await expect(toast("Archiving qa-deck")).toContainText("Its archive script is running on the box"); + await shot("8-script-running"); + await expect(toast("Archived qa-deck")).toBeVisible({ timeout: 10_000 }); + await expect(page.locator('[data-worktree="devl/qa-deck"]')).toHaveCount(0); + + step("devl/checkout-fix (uncommitted work): kept, and a warning says why"); + await pressArchive(await archive("devl/checkout-fix"), "9-dirty-just-after-archive"); + await expect(toast("Couldn't archive checkout-fix")).toContainText("checkout-fix has uncommitted changes, so it was left as it is. Commit them first, or use Remove worktree to discard them."); + await expect(page.locator('[data-testid=worktree-row][data-worktree="devl/checkout-fix"]')).toBeVisible(); + await expect(leaving("checkout-fix")).toHaveCount(0); + await shot("10-dirty-kept"); +} diff --git a/app/e2e/worktree-remove.spec.ts b/app/e2e/worktree-remove.spec.ts index 3056b7cbc..96feed7c6 100644 --- a/app/e2e/worktree-remove.spec.ts +++ b/app/e2e/worktree-remove.spec.ts @@ -2,7 +2,8 @@ import { expect, mockOnly, test } from "./fixtures"; // Removing a worktree asks nothing: it shows "Removing…" and goes. One with // uncommitted work (checkout-fix, in the fixtures) is kept, and the toast -// that says so offers to remove it anyway. +// that says so offers to remove it anyway. Archive asks, then carries on +// the same way, its dialog gone at once. test("Remove worktree removes it without a dialog, and a dirty one only when told to", async ({ app }) => { mockOnly(); @@ -28,6 +29,33 @@ test("Remove worktree removes it without a dialog, and a dirty one only when tol await expect(dirty).toHaveCount(0, { timeout: 10_000 }); }); +test("Archive closes its dialog at once and the worktree goes when its box is done", async ({ app }) => { + mockOnly(); + // A box takes 4 s to remove a worktree, as one with a big node_modules does. + await app.open({ params: { slowremove: "4000" } }); + const page = app.page; + const toast = (text: string) => page.locator("[data-slot=toast-viewport] > *", { hasText: text }); + const dialog = page.getByRole("alertdialog"); + + const row = app.worktree("gpu/ci-flake"); + await row.click({ button: "right" }); + await page.getByRole("menuitem", { name: "Archive…" }).click(); + await dialog.getByRole("button", { name: "Archive", exact: true }).click(); + await expect(dialog).toBeHidden({ timeout: 1000 }); + await expect(page.locator("[data-leaving]", { hasText: "ci-flake" })).toContainText("Archiving…"); + await expect(toast("Archived ci-flake")).toContainText("Branch me/ci-flake is kept.", { timeout: 10_000 }); + await expect(row).toHaveCount(0); + + // Uncommitted work: kept, and the toast says why. + const dirty = app.worktree("devl/checkout-fix"); + await dirty.click({ button: "right" }); + await page.getByRole("menuitem", { name: "Archive…" }).click(); + await dialog.getByRole("button", { name: "Archive", exact: true }).click(); + await expect(dialog).toBeHidden({ timeout: 1000 }); + await expect(toast("Couldn't archive checkout-fix")).toContainText("Commit them first, or use Remove worktree to discard them."); + await expect(dirty).toBeVisible(); +}); + test("Delete in the Worktrees table removes the selected ones without a dialog", async ({ app }) => { mockOnly(); await app.open(); diff --git a/app/src/components/sidebar/actions.tsx b/app/src/components/sidebar/actions.tsx index 8f9c4af1f..2483a8002 100644 --- a/app/src/components/sidebar/actions.tsx +++ b/app/src/components/sidebar/actions.tsx @@ -26,7 +26,7 @@ import { plainError } from "@/lib/errors"; import { scheduleRefresh, useStore } from "@/lib/store"; import { cn } from "@/lib/utils"; import { isPinned, pinWorktree, unpinWorktree } from "@/lib/pinned-worktrees"; -import { queueRemoval, removeWorktreeOnBox } from "@/lib/remove-worktree"; +import { queueRemoval } from "@/lib/remove-worktree"; import { addGroup, isShown, refOf, selectWorktree, useWorkspaces, wsKey } from "@/lib/workspaces"; import { ToneItems } from "@/components/workspace/tab-group"; import { openWorktreePicker } from "@/components/workspace/worktree-picker"; @@ -422,7 +422,9 @@ export function worktreeActions(box: string, loc: Location, wt: Worktree): Actio // archiveWorktree asks, then archives a worktree: its sessions and services // stop, the repo's archive script runs (on the box, in the background), // then its folder goes. Its branch stays, and git refuses to lose -// uncommitted work, so nothing is lost; Remove is the way to discard. +// uncommitted work, so nothing is lost; Remove is the way to discard. The +// dialog closes as soon as it is confirmed; the row says "Archiving…" until +// the box is done, and a toast says how it went (lib/remove-worktree.ts). export function archiveWorktree(box: string, loc: Location, wt: Worktree) { const script = loc.scripts?.archive; confirm({ @@ -430,19 +432,7 @@ export function archiveWorktree(box: string, loc: Location, wt: Worktree) { description: `${script ? "The repo's archive script runs, then its" : "Its"} folder on ${box} goes and its sessions and services stop. ${wt.branch ? `Branch ${wt.branch} stays, so you can pick it up again.` : ""}`, detail: script ? {script} : undefined, confirm: "Archive", - run: async () => { - const client = useStore.getState().client; - if (!client) throw new Error("not connected"); - let res: { archive?: string }; - try { - res = await removeWorktreeOnBox(client, box, loc.name, wt, "archive"); - } catch (err) { - if (/modified|untracked|uncommitted|contains/i.test(errorMessage(err))) throw new Error(`${wt.name} has uncommitted changes, so it was left as it is. Commit them first, or use Remove worktree to discard them.`); - throw err; - } - if (res.archive) toastManager.add({ title: `Archiving ${wt.name}`, description: "Its archive script is running on the box; the worktree goes when it finishes.", type: "info" }); - else toastManager.add({ title: `Archived ${wt.name}`, description: wt.branch ? `Branch ${wt.branch} is kept.` : box, type: "success" }); - }, + run: () => queueRemoval(box, loc.name, wt, { kind: "archive" }), }); } diff --git a/app/src/lib/mock.ts b/app/src/lib/mock.ts index 729b3412a..21b70ce80 100644 --- a/app/src/lib/mock.ts +++ b/app/src/lib/mock.ts @@ -100,6 +100,10 @@ const mockBuilds: Record = { devl: SHIPPED_BUILD, gpu: "304337b9 // ?mock=1&fresh=1 is a new account: no boxes yet, so onboarding shows. const fresh = new URLSearchParams(location.search).has("fresh"); +// ?mock=1&slowremove=MS: a box takes that long to remove a worktree with no +// archive script, as one with a big node_modules does, and answers once it +// is gone. +const slowRemove = Number(new URLSearchParams(location.search).get("slowremove")) || 0; if (fresh) { status.boxes = []; status.forwards = []; @@ -370,7 +374,7 @@ if (!fresh && !__BERTH_DEMO__ && !new URLSearchParams(location.search).has("stil emit({ type: s.agent_state === "waiting" ? "agent.waiting" : "agent.started", box: "devl", origin: s.agent, data: { path: s.dir } }); }, 25_000); -const delay = (v: T) => new Promise((r) => setTimeout(() => r(structuredClone(v)), 120)); +const delay = (v: T, ms = 120) => new Promise((r) => setTimeout(() => r(structuredClone(v)), ms)); // mockOrchestration answers send, wait and exec: a prompted agent works for // a moment and finishes, and a worktree's check fails once, then passes, so @@ -885,6 +889,7 @@ function boxCall(box: string, method: string, path: string, body?: unknown): Pro }, 1500); return delay({ removing: wt, archive: l.scripts.archive }); } + if (slowRemove) return delay({ removed: wt }, slowRemove).finally(gone); setTimeout(gone, 50); return delay({ removed: wt }); } diff --git a/app/src/lib/remove-worktree.ts b/app/src/lib/remove-worktree.ts index b196e66e2..9cea35c2e 100644 --- a/app/src/lib/remove-worktree.ts +++ b/app/src/lib/remove-worktree.ts @@ -4,7 +4,7 @@ import type { Client } from "@/lib/api"; import { errorMessage } from "@/lib/format"; import { keptReason } from "@/lib/kept-reason"; import { unpinWorktree } from "@/lib/pinned-worktrees"; -import { clearRemoving, markRemoving, markScript, pruneRemovals, type RemovalKind, removalOf, removeResult, useRemovals } from "@/lib/removing"; +import { clearRemoving, inTurn, markRemoving, markScript, pruneRemovals, type RemovalKind, removalOf, removeResult, useRemovals } from "@/lib/removing"; import { scheduleRefresh, useStore } from "@/lib/store"; import { forgetWorktree } from "@/lib/workspaces"; import { worktreesApi } from "@/lib/worktrees"; @@ -42,49 +42,51 @@ export async function removeWorktreeOnBox( return res; } -// Removals asked for and not yet sent, per box. Each box takes one at a -// time, so a slow teardown never has others half done beside it. -const waiting = new Map(); - -// queueRemoval removes a worktree without asking first: it is marked -// "Removing…" at once and goes when its box gets to it. Its branch stays, -// and git still refuses to lose uncommitted work: then a toast offers to -// remove it anyway. -export function queueRemoval(box: string, location: string, wt: { name: string; path: string }, opts: { force?: boolean } = {}) { +// queueRemoval removes or archives a worktree in the background: it is +// marked "Removing…" or "Archiving…" at once, so whatever asked (archive's +// dialog) can close, and goes when its box gets to it, one at a time per +// box (inTurn). Its branch stays, and git still refuses to lose uncommitted +// work: then a toast says so, and a removal's offers to remove it anyway. +export function queueRemoval(box: string, location: string, wt: Leaving, opts: { kind?: RemovalKind; force?: boolean } = {}) { + const kind = opts.kind ?? "remove"; if (removalOf(useRemovals.getState().byKey, box, wt.path)) return; - markRemoving(box, wt.path, "remove", wt.name); - const list = waiting.get(box); - if (list) { - list.push({ location, wt, force: opts.force }); - return; - } - waiting.set(box, [{ location, wt, force: opts.force }]); - void drain(box); + markRemoving(box, wt.path, kind, wt.name); + void inTurn(box, () => send(box, location, wt, kind, opts.force)); } -async function drain(box: string) { - const list = waiting.get(box)!; - while (list.length) { - const { location, wt, force } = list[0]; - try { - const client = useStore.getState().client; - if (!client) throw new Error("not connected"); - await removeWorktreeOnBox(client, box, location, wt, "remove", { force }); - } catch (err) { - clearRemoving(box, wt.path); - const why = force ? null : keptReason(errorMessage(err)); - if (why) { +type Leaving = { name: string; path: string; branch?: string }; + +// keepsWork is git refusing, in its own words, to lose uncommitted work. +export const keepsWork = (err: unknown) => /modified|untracked|uncommitted|contains/i.test(errorMessage(err)); + +async function send(box: string, location: string, wt: Leaving, kind: RemovalKind, force?: boolean) { + try { + const client = useStore.getState().client; + if (!client) throw new Error("not connected"); + const res = await removeWorktreeOnBox(client, box, location, wt, kind, { force }); + if (kind === "remove") return; + if (res.archive) toastManager.add({ title: `Archiving ${wt.name}`, description: "Its archive script is running on the box; the worktree goes when it finishes.", type: "info" }); + else toastManager.add({ title: `Archived ${wt.name}`, description: wt.branch ? `Branch ${wt.branch} is kept.` : box, type: "success" }); + } catch (err) { + clearRemoving(box, wt.path); + const why = force ? null : keptReason(errorMessage(err)); + if (why) { + if (kind === "archive") { + toastManager.add({ + type: "warning", + title: `Couldn't archive ${wt.name}`, + description: `${wt.name} has uncommitted changes, so it was left as it is. Commit them first, or use Remove worktree to discard them.`, + }); + } else { toastManager.add({ type: "warning", title: `Kept ${wt.name}`, description: why, actionProps: { children: "Remove anyway", onClick: () => queueRemoval(box, location, wt, { force: true }) }, }); - } else toastError(err, { title: `Couldn't remove ${wt.name}`, box }); - } - list.shift(); + } + } else toastError(err, { title: `Couldn't ${kind} ${wt.name}`, box }); } - waiting.delete(box); } // worktreeGone is the box saying a worktree went: its workspace (tabs, diff --git a/app/src/lib/removing.test.ts b/app/src/lib/removing.test.ts index 662269e32..309746800 100644 --- a/app/src/lib/removing.test.ts +++ b/app/src/lib/removing.test.ts @@ -2,7 +2,7 @@ import assert from "node:assert/strict"; import { test } from "node:test"; -import { clearRemoving, markRemoving, markScript, pruneRemovals, removalLabel, removalOf, removeResult, STALE_MS, useRemovals } from "./removing.ts"; +import { clearRemoving, inTurn, markRemoving, markScript, pruneRemovals, removalLabel, removalOf, removeResult, STALE_MS, useRemovals } from "./removing.ts"; const reset = () => useRemovals.setState({ byKey: {} }); const of = (box: string, path: string, now?: number) => removalOf(useRemovals.getState().byKey, box, path, now); @@ -50,3 +50,24 @@ test("a DELETE's answer is read from JSON or, from older boxes, text", () => { assert.deepEqual(removeResult("not json"), {}); assert.deepEqual(removeResult(undefined), {}); }); + +test("a box takes its removals one at a time; another box's don't wait", async () => { + const log: string[] = []; + let answer!: () => void; + const slow = new Promise((r) => (answer = r)); + void inTurn("devl", async () => { + log.push("a sent"); + await slow; + log.push("a done"); + }); + const b = inTurn("devl", async () => { + log.push("b sent"); + }); + await inTurn("gpu", async () => { + log.push("c sent"); + }); + assert.deepEqual(log, ["a sent", "c sent"]); + answer(); + await b; + assert.deepEqual(log, ["a sent", "c sent", "a done", "b sent"]); +}); diff --git a/app/src/lib/removing.ts b/app/src/lib/removing.ts index b3191f514..9d4eef695 100644 --- a/app/src/lib/removing.ts +++ b/app/src/lib/removing.ts @@ -1,11 +1,15 @@ import { create } from "zustand"; -// Worktrees on their way out. Removing one is quick, but a repository with -// an archive script has the box run it first, in the background, and the -// worktree only goes when it succeeds: minutes, sometimes. Until the box -// says it is gone (worktree.removed) or that archiving failed, the app shows -// it as "Archiving…" or "Removing…" and keeps it from being opened, so -// nobody starts work in a folder that is about to go. +import { oneAtATime } from "./one-at-a-time.ts"; + +// Worktrees on their way out. The box answers a removal once the folder is +// gone, which for a big checkout (node_modules) takes seconds. A repository +// with an archive script has the box run it first, in the background, and +// the worktree only goes when it succeeds: minutes, sometimes. From the +// moment it is asked until the box says it is gone (worktree.removed) or +// that archiving failed, the app shows it as "Archiving…" or "Removing…" +// and keeps it from being opened, so nobody starts work in a folder that is +// about to go. // // No imports from "@/": pnpm test runs this file in plain node. @@ -58,6 +62,18 @@ export function removalOf(byKey: Record, box: string, path: str export const useRemoval = (box: string, path: string | undefined) => useRemovals((s) => (path ? removalOf(s.byKey, box, path) : undefined)); +// Each box's removals, in line. A box takes one at a time, so a slow +// teardown never has others half done beside it; another box's don't wait. +const lines = new Map>(); + +// inTurn runs job once the box's earlier removals are done, whether they +// worked or failed. +export function inTurn(box: string, job: () => Promise): Promise { + let line = lines.get(box); + if (!line) lines.set(box, (line = oneAtATime())); + return line(job); +} + // removalLabel is what a row says meanwhile. export const removalLabel = (r: Removal) => (r.kind === "archive" ? "Archiving…" : "Removing…"); diff --git a/app/src/views/worktrees/use-bulk.ts b/app/src/views/worktrees/use-bulk.ts index 1cee27d05..c2bcb17f4 100644 --- a/app/src/views/worktrees/use-bulk.ts +++ b/app/src/views/worktrees/use-bulk.ts @@ -1,9 +1,9 @@ import { useCallback, useRef, useState } from "react"; import { boxApi } from "@/lib/api"; -import { errorMessage } from "@/lib/format"; import { plainError } from "@/lib/errors"; -import { removeWorktreeOnBox } from "@/lib/remove-worktree"; +import { keepsWork, removeWorktreeOnBox } from "@/lib/remove-worktree"; +import { removalOf, useRemovals } from "@/lib/removing"; import { scheduleRefresh, useStore } from "@/lib/store"; import { type SyncMode, worktreesApi } from "@/lib/worktrees"; import type { Row } from "@/views/worktrees/use-worktrees"; @@ -58,7 +58,10 @@ export function useBulk(onRowDone: (r: Row, patch: Partial) => void) { set(r.key, { state: "skipped", message: "cancelled" }); continue; } - const why = skipReason(action, r); + // One already on its way out (lib/removing.ts) is left to that, so + // its box never gets a second request for it. + const leaving = action.kind === "archive" && removalOf(useRemovals.getState().byKey, r.box, r.path); + const why = skipReason(action, r) ?? (leaving ? "already on its way out" : undefined); if (why) { set(r.key, { state: "skipped", message: why }); continue; @@ -100,7 +103,7 @@ export function useBulk(onRowDone: (r: Row, patch: Partial) => void) { try { res = await removeWorktreeOnBox(client, r.box, r.location, { name: r.name, path: r.path }, "archive"); } catch (err) { - if (/modified|untracked|uncommitted|contains/i.test(errorMessage(err))) throw new Error("Has uncommitted changes: commit them first, or delete it instead"); + if (keepsWork(err)) throw new Error("Has uncommitted changes: commit them first, or delete it instead"); throw err; } set(r.key, { state: "ok", message: res.archive ? "Archiving: its archive script runs, then it goes" : "Archived; its branch is kept" }); diff --git a/docs/changelog.mdx b/docs/changelog.mdx index bd1f405d4..be49b488f 100644 --- a/docs/changelog.mdx +++ b/docs/changelog.mdx @@ -46,6 +46,12 @@ again. branch, and asks git again only when the repository changes (or after a minute). +- **Archiving a worktree no longer holds its dialog open.** Archive… + still asks first, but the dialog closes as soon as you press Archive: + the worktree shows Archiving… and goes when its box gets to it, one at + a time, so you can archive several in a row. A note says when it's + done, or that one with uncommitted changes was left as it is. + ## v0.3.31 - **Worktrees with submodules can be removed.** git refuses to remove any From 693fc3d5634ba7a5ccb69bc33a80957760bc0785 Mon Sep 17 00:00:00 2001 From: Stener1 Date: Thu, 22 Oct 2026 10:47:24 +0000 Subject: [PATCH 02/26] feat(app): bulk Stop and Archive carry on in a background-jobs tray MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Confirming Stop N agents? (and Clean up…) now closes the dialog at once. The stopping, and the archiving when it was ticked, goes on in a tray in the bottom-right corner, above the status bar and the loops panel: one row per agent, grouped by box, with progress in the title ("Stopping 17 agents · 9/17"). It folds to one line (remembered), Stop the rest sends nothing more, and a clean run sums itself up ("17 stopped, 15 archived"), shows the toast it always did and fades. If something was kept or failed it stays as a report of those rows until you dismiss it. Jobs live in an app-level store, so they carry on when you leave the board. The sidebar's Archive… shows up in the tray too, as a one-row job. Concurrency and the archive safety rules are unchanged; bulk archives now wait in the same per-box line as every other removal. The mock gains ?slowstop=MS to make stops slow. Co-Authored-By: Claude Opus 5.5 --- app/e2e/dashboard-stop-archive.spec.ts | 93 +++++++-- app/e2e/worktree-remove.spec.ts | 8 + app/src/App.tsx | 12 +- app/src/components/jobs-tray.tsx | 191 ++++++++++++++++++ app/src/components/sidebar/actions.tsx | 5 +- app/src/hooks/lift-toasts.ts | 3 +- app/src/lib/jobs.test.ts | 114 +++++++++++ app/src/lib/jobs.ts | 119 ++++++++++++ app/src/lib/mock.ts | 4 +- app/src/lib/remove-worktree.ts | 42 +++- app/src/lib/stop-job.test.ts | 121 ++++++++++++ app/src/lib/stop-job.ts | 134 +++++++++++++ app/src/views/dashboard/stop.tsx | 255 +++++++------------------ 13 files changed, 886 insertions(+), 215 deletions(-) create mode 100644 app/src/components/jobs-tray.tsx create mode 100644 app/src/lib/jobs.test.ts create mode 100644 app/src/lib/jobs.ts create mode 100644 app/src/lib/stop-job.test.ts create mode 100644 app/src/lib/stop-job.ts diff --git a/app/e2e/dashboard-stop-archive.spec.ts b/app/e2e/dashboard-stop-archive.spec.ts index 25358670b..508415198 100644 --- a/app/e2e/dashboard-stop-archive.spec.ts +++ b/app/e2e/dashboard-stop-archive.spec.ts @@ -1,9 +1,12 @@ import { expect, type App, mockOnly, test } from "./fixtures"; -// The board's bulk Stop can archive the stopped agents' worktrees too, where -// it's safe. In the fixtures shop on devl has an archive script, so an -// archive runs it first; checkout-fix has uncommitted work, so git keeps it; -// order-export has three agents, so stopping one leaves the others there. +// The board's bulk Stop asks, then carries on in the background-jobs tray, +// its dialog gone at once. It can archive the stopped agents' worktrees +// too, where it's safe. In the fixtures shop on devl has an archive script, +// so an archive runs it first; ci-flake on gpu has none, so it goes at once +// (slowly, with ?slowremove); checkout-fix has uncommitted work, so git +// keeps it; order-export has three agents, so stopping one leaves the others +// there. const prefs = { labs: false, labsChosen: true, version: 3 }; @@ -20,51 +23,99 @@ async function stopPicked(app: App, titles: string[]) { } const archiveBox = (app: App) => app.page.getByRole("checkbox", { name: /Archive their worktrees where it's safe/ }); -const row = (app: App, place: string) => app.page.getByRole("group", { name: "Agents to stop" }).getByRole("listitem").filter({ hasText: place }); +const tray = (app: App) => app.page.getByRole("region", { name: "Background jobs" }); +const job = (app: App, title: string) => tray(app).getByRole("group", { name: title }); +const jobRow = (app: App, title: string, label: string) => job(app, title).getByRole("listitem").filter({ hasText: label }); +const toast = (app: App, text: string) => app.page.locator("[data-slot=toast-viewport] > *", { hasText: text }); -test("ticked, Stop archives a clean worktree and keeps a dirty one, saying why", async ({ app }) => { +test("ticked, Stop closes at once; the tray shows the run, folds, and reports the kept worktree", async ({ app }) => { mockOnly("stops agents and archives worktrees"); - await app.open({ prefs }); - const dialog = await stopPicked(app, ["Speed up product search", "Fix checkout webhook retries"]); + // gpu takes 3 s to remove a worktree, as one with a big node_modules does. + await app.open({ prefs, params: { slowremove: "3000" } }); + const dialog = await stopPicked(app, ["Speed up product search", "Move acme billing to the new ledger", "Fix checkout webhook retries"]); await expect(archiveBox(app)).not.toBeChecked(); await archiveBox(app).click(); await expect(dialog).toContainText("then its worktree is archived where it's safe"); - await dialog.getByRole("button", { name: "Stop 2 agents" }).click(); + await dialog.getByRole("button", { name: "Stop 3 agents" }).click(); + await expect(dialog).toBeHidden({ timeout: 1000 }); + + // Mid-run: the title counts, and the slow one says it is archiving. + const running = job(app, "Stopping 3 agents"); + await expect(running).toContainText(/Stopping 3 agents · [0-2]\/3/); + await expect(jobRow(app, "Stopping 3 agents", "ci-flake")).toContainText("Archiving…"); + + // Folded, it is one line; the choice is remembered. + await running.getByRole("button", { name: "Fold" }).click(); + const pill = tray(app).getByRole("button", { expanded: false }); + await expect(pill).toContainText(/Stopping 3 agents · [0-2]\/3/); + await expect.poll(() => app.stored("berth.jobs.folded")).toBe(true); + await pill.click(); - // Kept work holds the dialog open, with each row's outcome. - await expect(dialog.getByRole("heading")).toHaveText("Stopped 2 agents · archiving 1 worktree"); - await expect(row(app, "search-perf")).toContainText("Stopped · Archiving: its archive script runs, then it goes"); - await expect(row(app, "checkout-fix")).toContainText("Stopped · Kept: uncommitted changes"); - await dialog.getByRole("button", { name: "Done" }).click(); + // Kept work makes a report that lists only it, until it's dismissed. + await expect(running).toContainText("3 stopped, 1 archived, 1 archiving, 1 kept", { timeout: 10_000 }); + await expect(running.getByRole("listitem")).toHaveCount(1); + await expect(jobRow(app, "Stopping 3 agents", "checkout-fix")).toContainText("Stopped · Kept: uncommitted changes"); + await running.getByRole("button", { name: "Dismiss" }).click(); + await expect(tray(app)).toHaveCount(0); - // The clean one goes once its script has run; the dirty one stays. + // The clean ones go (search-perf once its script has run); the dirty one stays. + await expect(app.worktree("gpu/ci-flake")).toHaveCount(0, { timeout: 10_000 }); await expect(app.worktree("devl/search-perf")).toHaveCount(0, { timeout: 10_000 }); await expect(app.worktree("devl/checkout-fix")).toBeVisible(); }); -test("a worktree where another agent still runs is kept", async ({ app }) => { +test("a worktree where another agent still runs is kept, and the report says so", async ({ app }) => { mockOnly("stops agents and archives worktrees"); await app.open({ prefs }); const dialog = await stopPicked(app, ["Export orders as CSV from the admin"]); await archiveBox(app).click(); await dialog.getByRole("button", { name: "Stop 1 agent" }).click(); - await expect(dialog.getByRole("heading")).toHaveText("Stopped 1 agent"); - await expect(row(app, "order-export")).toContainText("Kept: another agent is still running there"); - await dialog.getByRole("button", { name: "Done" }).click(); + await expect(dialog).toBeHidden({ timeout: 1000 }); + await expect(job(app, "Stopping 1 agent")).toContainText("1 stopped, 1 kept"); + await expect(jobRow(app, "Stopping 1 agent", "order-export")).toContainText("Kept: another agent is still running there"); await expect(app.worktree("devl/order-export")).toBeVisible(); }); -test("unticked, Stop only stops: the worktree stays", async ({ app }) => { +test("unticked, a clean run only stops, says so in a toast, and fades from the tray", async ({ app }) => { mockOnly("stops agents"); await app.open({ prefs }); const dialog = await stopPicked(app, ["Speed up product search"]); await expect(dialog).toContainText("its worktree and files are kept"); await dialog.getByRole("button", { name: "Stop 1 agent" }).click(); await expect(dialog).toHaveCount(0); - await expect(app.page.locator("[data-slot=toast-viewport] > *", { hasText: "Stopped 1 agent" })).toContainText("Their worktrees are kept."); + await expect(toast(app, "Stopped 1 agent")).toContainText("Their worktrees are kept."); + await expect(job(app, "Stopping 1 agent")).toContainText("1 stopped"); + await expect(job(app, "Stopping 1 agent").getByRole("listitem")).toHaveCount(0); + await expect(tray(app)).toHaveCount(0, { timeout: 15_000 }); await expect(app.worktree("devl/search-perf")).toBeVisible(); }); +test("Stop the rest sends no more; the ones already asked stop, the rest say Not stopped", async ({ app }) => { + mockOnly("stops agents"); + // Each stop takes 2 s to answer, so three are in flight and two wait. + await app.open({ prefs, params: { slowstop: "2000" } }); + const dialog = await stopPicked(app, ["Speed up product search", "Move acme billing to the new ledger", "Export orders as CSV from the admin", "Add tests for the export job", "Rerun the evals once the vendor replies"]); + await dialog.getByRole("button", { name: "Stop 5 agents" }).click(); + const running = job(app, "Stopping 5 agents"); + await expect(running.getByRole("listitem").filter({ hasText: "Stopping…" })).toHaveCount(3); + await running.getByRole("button", { name: "Stop the rest" }).click(); + await expect(running).toContainText("3 stopped, 2 not stopped", { timeout: 10_000 }); + await expect(running.getByRole("listitem").filter({ hasText: "Not stopped" })).toHaveCount(2); +}); + +test("the job carries on when you leave the board", async ({ app }) => { + mockOnly("stops agents and archives worktrees"); + await app.open({ prefs, params: { slowremove: "2000" } }); + const dialog = await stopPicked(app, ["Move acme billing to the new ledger"]); + await archiveBox(app).click(); + await dialog.getByRole("button", { name: "Stop 1 agent" }).click(); + await expect(jobRow(app, "Stopping 1 agent", "ci-flake")).toContainText("Archiving…"); + await app.page.getByTestId("nav-worktrees").click(); + await expect(app.page.getByRole("heading", { name: "Agent Dashboard" })).toHaveCount(0); + await expect(job(app, "Stopping 1 agent")).toContainText("1 stopped, 1 archived", { timeout: 10_000 }); + await expect(toast(app, "Stopped 1 agent · archived 1 worktree")).toContainText("Their branches are kept."); +}); + test("the archive choice is remembered, and not offered for the main checkout alone", async ({ app }) => { mockOnly("stops agents"); await app.open({ prefs }); diff --git a/app/e2e/worktree-remove.spec.ts b/app/e2e/worktree-remove.spec.ts index 96feed7c6..27b457a1b 100644 --- a/app/e2e/worktree-remove.spec.ts +++ b/app/e2e/worktree-remove.spec.ts @@ -43,7 +43,11 @@ test("Archive closes its dialog at once and the worktree goes when its box is do await dialog.getByRole("button", { name: "Archive", exact: true }).click(); await expect(dialog).toBeHidden({ timeout: 1000 }); await expect(page.locator("[data-leaving]", { hasText: "ci-flake" })).toContainText("Archiving…"); + // It is a one-row job in the tray, which sums it up when it's done. + const job = page.getByRole("region", { name: "Background jobs" }).getByRole("group", { name: "Archiving ci-flake" }); + await expect(job.getByRole("listitem")).toContainText("Archiving…"); await expect(toast("Archived ci-flake")).toContainText("Branch me/ci-flake is kept.", { timeout: 10_000 }); + await expect(job).toContainText("Archived ci-flake"); await expect(row).toHaveCount(0); // Uncommitted work: kept, and the toast says why. @@ -54,6 +58,10 @@ test("Archive closes its dialog at once and the worktree goes when its box is do await expect(dialog).toBeHidden({ timeout: 1000 }); await expect(toast("Couldn't archive checkout-fix")).toContainText("Commit them first, or use Remove worktree to discard them."); await expect(dirty).toBeVisible(); + // The tray keeps a report of it until it's dismissed. + const kept = page.getByRole("region", { name: "Background jobs" }).getByRole("group", { name: "Archiving checkout-fix" }); + await expect(kept).toContainText("Kept checkout-fix"); + await expect(kept.getByRole("listitem")).toContainText("Kept: uncommitted changes"); }); test("Delete in the Worktrees table removes the selected ones without a dialog", async ({ app }) => { diff --git a/app/src/App.tsx b/app/src/App.tsx index 3e98633ff..03ae9eaa7 100644 --- a/app/src/App.tsx +++ b/app/src/App.tsx @@ -11,6 +11,7 @@ import { TreeDockFrame } from "@/components/files/tree-dock"; import { WorktreePicker } from "@/components/workspace/worktree-picker"; import { ComposerDialog } from "@/components/conversation/composer-dialog"; import { NotificationCenter } from "@/components/notifications/notification-center"; +import { JobsTray } from "@/components/jobs-tray"; import { LoopsPanel } from "@/components/orchestrate/loops-panel"; import { PluginConsentDialog } from "@/components/plugin-consent-dialog"; import { PromptDialogs } from "@/components/prompts"; @@ -171,14 +172,15 @@ export default function App() { return ( {/* Toasts sit bottom-right, as in other desktop tools: above the status - bar (26px) and above the loop panel when there is one, which says - how tall it is in --berth-loops-h. Top-right covered the headers of + bar (26px) and above the loop panel and the background-jobs tray + when they're up, which say how tall they are in --berth-loops-h + and --berth-jobs-h. Top-right covered the headers of pages (Review's, say). A bar floating at a page's bottom (the Dashboard's selection, the Worktrees bulk bar) lifts them above it through --berth-bar-lift (hooks/lift-toasts.ts). With a dialog or sheet open they move to a corner clear of it; see components/ui/toast.tsx. The stack is as wide as the loops panel. */} - +
{fakeTrafficLights() && }
@@ -233,9 +235,11 @@ export default function App() { - {/* Loops float over the workspace, not over Settings' page. */} + {/* Loops and background jobs float over the workspace, not over + Settings' page. */}
+
diff --git a/app/src/components/jobs-tray.tsx b/app/src/components/jobs-tray.tsx new file mode 100644 index 000000000..3b6092e1e --- /dev/null +++ b/app/src/components/jobs-tray.tsx @@ -0,0 +1,191 @@ +import { AlertTriangleIcon, CheckIcon, ChevronDownIcon, ChevronUpIcon, CircleStopIcon, MinusIcon, XIcon } from "lucide-react"; +import { useEffect, useLayoutEffect, useRef, useState } from "react"; + +import { Tip } from "@/components/tip"; +import { Button } from "@/components/ui/button"; +import { Spinner } from "@/components/ui/spinner"; +import { cancelJob, dismissJob, FADE_MS, type Job, type JobItem, leftOver, outcomeOf, progressOf, setFolded, useJobs } from "@/lib/jobs"; +import { cn } from "@/lib/utils"; + +// The background-jobs tray (lib/jobs.ts): bottom right, above the status +// bar (26px) and the loops panel (--berth-loops-h), with toasts stacked +// above it through --berth-jobs-h. Jobs stack newest on top. Folded, it is +// one line with the progress. With no jobs it is gone and reserves no room. +const STATUS_BAR = 26; +const GAP = 12; +const FADE = 700; + +export function JobsTray() { + const jobs = useJobs((s) => s.jobs); + const folded = useJobs((s) => s.folded); + const ref = useRef(null); + + useLayoutEffect(() => { + const root = document.documentElement; + const el = ref.current; + if (!el) { + root.style.setProperty("--berth-jobs-h", "0px"); + return; + } + const sync = () => root.style.setProperty("--berth-jobs-h", `${el.offsetHeight + GAP}px`); + sync(); + const ro = new ResizeObserver(sync); + ro.observe(el); + return () => { + ro.disconnect(); + root.style.setProperty("--berth-jobs-h", "0px"); + }; + }, [jobs.length, folded]); + + if (!jobs.length) return null; + const style = { bottom: `calc(var(--berth-status-h, ${STATUS_BAR}px) + ${GAP}px + var(--berth-loops-h, 0px))`, right: `calc(${GAP}px + var(--berth-dock-w, 0px))` }; + if (folded) { + const running = jobs.filter((j) => outcomeOf(j) === "running"); + const report = jobs.some((j) => outcomeOf(j) === "report"); + const one = jobs.length === 1 ? jobs[0] : undefined; + const p = jobs.reduce((a, j) => ({ settled: a.settled + progressOf(j).settled, total: a.total + progressOf(j).total }), { settled: 0, total: 0 }); + const label = one ? lineOf(one) : `${jobs.length} jobs${running.length ? ` · ${p.settled}/${p.total}` : ""}`; + return ( +
+ +
+ ); + } + return ( +
+ {jobs.map((j) => ( + + ))} +
+ ); +} + +// lineOf is a job in one line: what it is doing and how far it got, or, +// once done, what it did. +function lineOf(j: Job) { + if (j.summary !== undefined) return j.summary; + const p = progressOf(j); + return `${j.title} · ${p.settled}/${p.total}`; +} + +function JobCard({ job: j }: { job: Job }) { + const outcome = outcomeOf(j); + const fading = useFading(j, outcome === "clean"); + // Running, every row; a report, only what didn't go as asked; clean, + // just the summary. + const rows = outcome === "running" ? j.items : outcome === "report" ? leftOver(j) : []; + const groups = [...new Set(rows.map((i) => i.group))].sort(); + const unsent = j.items.some((i) => i.state === "queued"); + return ( +
+
+ {outcome === "running" ? : outcome === "report" ? : } + + {lineOf(j)} + + + + + {outcome !== "running" && ( + + + + )} +
+ {rows.length > 0 && ( +
+ {groups.map((g) => { + const group = rows.filter((i) => i.group === g); + const all = j.items.filter((i) => i.group === g); + return ( +
+ {groups.length > 1 || outcome === "running" ? ( +
+ {g} + {outcome === "running" && ( + + {progressOf({ ...j, items: all }).settled}/{all.length} + + )} +
+ ) : null} +
    + {group.map((i) => ( + + ))} +
+
+ ); + })} +
+ )} + {outcome === "running" && j.cancellable && unsent && ( +
+ +
+ )} +
+ ); +} + +// useFading starts a clean job's fade just before the store lets it go. +function useFading(j: Job, clean: boolean) { + const [fading, setFading] = useState(false); + useEffect(() => { + if (!clean || j.finished === undefined) return; + const t = setTimeout(() => setFading(true), Math.max(0, j.finished + FADE_MS - FADE - Date.now())); + return () => clearTimeout(t); + }, [clean, j.finished]); + return fading; +} + +function JobRow({ item: i }: { item: JobItem }) { + const warn = i.state === "kept" || i.state === "cancelled"; + return ( +
  • + + {i.state === "running" ? ( + + ) : i.state === "done" ? ( + + ) : i.state === "failed" ? ( + + ) : warn ? ( + + ) : ( + + )} + + + + {i.label} + {i.aside && {i.aside}} + + {i.message && ( + {i.message} + )} + +
  • + ); +} diff --git a/app/src/components/sidebar/actions.tsx b/app/src/components/sidebar/actions.tsx index 2483a8002..fce3c8fc1 100644 --- a/app/src/components/sidebar/actions.tsx +++ b/app/src/components/sidebar/actions.tsx @@ -424,7 +424,8 @@ export function worktreeActions(box: string, loc: Location, wt: Worktree): Actio // then its folder goes. Its branch stays, and git refuses to lose // uncommitted work, so nothing is lost; Remove is the way to discard. The // dialog closes as soon as it is confirmed; the row says "Archiving…" until -// the box is done, and a toast says how it went (lib/remove-worktree.ts). +// the box is done, as a one-row job in the background-jobs tray, and a +// toast says how it went (lib/remove-worktree.ts). export function archiveWorktree(box: string, loc: Location, wt: Worktree) { const script = loc.scripts?.archive; confirm({ @@ -432,7 +433,7 @@ export function archiveWorktree(box: string, loc: Location, wt: Worktree) { description: `${script ? "The repo's archive script runs, then its" : "Its"} folder on ${box} goes and its sessions and services stop. ${wt.branch ? `Branch ${wt.branch} stays, so you can pick it up again.` : ""}`, detail: script ? {script} : undefined, confirm: "Archive", - run: () => queueRemoval(box, loc.name, wt, { kind: "archive" }), + run: () => queueRemoval(box, loc.name, wt, { kind: "archive", tray: true }), }); } diff --git a/app/src/hooks/lift-toasts.ts b/app/src/hooks/lift-toasts.ts index deea159b9..20cd750a7 100644 --- a/app/src/hooks/lift-toasts.ts +++ b/app/src/hooks/lift-toasts.ts @@ -3,7 +3,8 @@ // Put it on the bar as its ref: while the bar is up, --berth-bar-lift says // how far above the window's bottom edge the bar's top is (and a gap), and // the toast viewport (App.tsx) starts no lower than that. The loops panel -// does the same with --berth-loops-h. +// and the background-jobs tray do the same with --berth-loops-h and +// --berth-jobs-h. const GAP = 12; export function liftToasts(el: HTMLElement | null): (() => void) | undefined { diff --git a/app/src/lib/jobs.test.ts b/app/src/lib/jobs.test.ts new file mode 100644 index 000000000..6a6aa588f --- /dev/null +++ b/app/src/lib/jobs.test.ts @@ -0,0 +1,114 @@ +// node --experimental-strip-types --test src/lib/jobs.test.ts (pnpm test) +import assert from "node:assert/strict"; +import { mock, test } from "node:test"; + +const stored = new Map(); +globalThis.localStorage = { getItem: (k: string) => stored.get(k) ?? null, setItem: (k: string, v: string) => void stored.set(k, v) } as Storage; +// Folded when the app last quit. +stored.set("berth.jobs.folded", "true"); + +const { cancelJob, dismissJob, FADE_MS, finishJob, isCancelling, jobOf, leftOver, outcomeOf, progressOf, runPool, setFolded, setItem, startJob, useJobs } = await import("./jobs.ts"); + +const reset = () => useJobs.setState({ jobs: [] }); +const items = (...keys: string[]) => keys.map((key) => ({ key, group: "devl", label: key })); + +test("progress counts the rows that have settled, whichever way", () => { + reset(); + const id = startJob({ title: "Stopping 4 agents", items: items("a", "b", "c", "d") }); + assert.deepEqual(progressOf(jobOf(id)!), { settled: 0, total: 4 }); + setItem(id, "a", { state: "running" }); + setItem(id, "b", { state: "done" }); + setItem(id, "c", { state: "failed", message: "Failed: gone" }); + setItem(id, "d", { state: "cancelled" }); + assert.deepEqual(progressOf(jobOf(id)!), { settled: 3, total: 4 }); + assert.equal(outcomeOf(jobOf(id)!), "running"); +}); + +test("several jobs stack, newest on top", () => { + reset(); + const a = startJob({ title: "first", items: items("a") }); + const b = startJob({ title: "second", items: items("b") }); + assert.deepEqual( + useJobs.getState().jobs.map((j) => j.id), + [b, a], + ); +}); + +test("a clean job sums itself up and goes after a while; one with something kept stays as a report", () => { + reset(); + mock.timers.enable({ apis: ["setTimeout"] }); + try { + const clean = startJob({ title: "clean", items: items("a", "b") }); + setItem(clean, "a", { state: "done" }); + setItem(clean, "b", { state: "done" }); + finishJob(clean, "2 stopped"); + assert.equal(outcomeOf(jobOf(clean)!), "clean"); + + const kept = startJob({ title: "kept", items: items("c", "d") }); + setItem(kept, "c", { state: "done" }); + setItem(kept, "d", { state: "kept", message: "Stopped · Kept: uncommitted changes" }); + finishJob(kept, "2 stopped, 1 kept"); + assert.equal(outcomeOf(jobOf(kept)!), "report"); + assert.deepEqual( + leftOver(jobOf(kept)!).map((i) => i.key), + ["d"], + ); + + mock.timers.tick(FADE_MS - 1); + assert.ok(jobOf(clean)); + mock.timers.tick(1); + assert.equal(jobOf(clean), undefined); + assert.ok(jobOf(kept), "a report waits to be dismissed"); + dismissJob(kept); + assert.equal(jobOf(kept), undefined); + } finally { + mock.timers.reset(); + } +}); + +test("a cancelled job takes no more: only the items not yet sent are left out", async () => { + reset(); + const id = startJob({ title: "x", items: items("a", "b", "c", "d", "e"), cancellable: true }); + const sent: string[] = []; + let release!: () => void; + const hold = new Promise((r) => (release = r)); + const run = runPool( + ["a", "b", "c", "d", "e"], + 2, + async (k) => { + sent.push(k); + if (k === "b") { + cancelJob(id); + await hold; + } + }, + () => isCancelling(id), + ); + // a went and finished; b is in flight when the job is cancelled. + await new Promise((r) => setTimeout(r, 0)); + release(); + const unsent = await run; + assert.deepEqual(sent, ["a", "b"]); + assert.deepEqual(unsent.sort(), ["c", "d", "e"]); +}); + +test("the pool runs a few at a time", async () => { + let inFlight = 0; + let most = 0; + await runPool([1, 2, 3, 4, 5, 6, 7], 3, async () => { + most = Math.max(most, ++inFlight); + await new Promise((r) => setTimeout(r, 1)); + inFlight--; + }); + assert.equal(most, 3); +}); + +test("folding the tray is remembered", () => { + assert.equal(useJobs.getState().folded, true, "as it was left"); + setFolded(false); + setFolded(true); + assert.equal(useJobs.getState().folded, true); + assert.equal(stored.get("berth.jobs.folded"), "true"); + setFolded(false); + assert.equal(stored.get("berth.jobs.folded"), "false"); +}); diff --git a/app/src/lib/jobs.ts b/app/src/lib/jobs.ts new file mode 100644 index 000000000..7e75e265b --- /dev/null +++ b/app/src/lib/jobs.ts @@ -0,0 +1,119 @@ +import { create } from "zustand"; + +import { load, save } from "./storage.ts"; + +// Background jobs: work a dialog starts and then lets go of, such as the +// board's bulk Stop or Archive…, so you can get on with something else. The +// tray in the corner (components/jobs-tray.tsx) shows each one's rows as +// they go, folds to a line, and keeps a report when something was kept or +// failed. A clean job sums itself up and fades. The store is the app's, not +// a view's: a job carries on when you leave the page that started it. It +// doesn't outlive the app: rows not yet sent are lost when it quits. +// +// No imports from "@/": pnpm test runs this file in plain node. + +export type ItemState = "queued" | "running" | "done" | "kept" | "failed" | "cancelled"; + +export interface JobItem { + key: string; + // Where it runs: a box. The tray groups rows by it. + group: string; + label: string; + // A second name, when the label alone is ambiguous. + aside?: string; + state: ItemState; + message?: string; +} + +export interface Job { + id: number; + // What it does, while it runs: "Stopping 17 agents". + title: string; + items: JobItem[]; + // Whether "Stop the rest" is offered. + cancellable: boolean; + cancelling?: boolean; + // Set when it ends: what it did, "17 stopped, 15 archived". + summary?: string; + finished?: number; +} + +// A clean job's summary stays this long, then goes. +export const FADE_MS = 10_000; + +const FOLDED = "berth.jobs.folded"; + +export const useJobs = create<{ jobs: Job[]; folded: boolean }>()(() => ({ jobs: [], folded: load(FOLDED, false) })); + +let nextId = 0; + +// startJob puts a job on top of the tray and answers with its id. +export function startJob(job: { title: string; items: Omit[]; cancellable?: boolean }): number { + const id = ++nextId; + const items = job.items.map((i) => ({ ...i, state: "queued" as const })); + useJobs.setState((s) => ({ jobs: [{ id, title: job.title, items, cancellable: !!job.cancellable }, ...s.jobs] })); + return id; +} + +const patchJob = (id: number, f: (j: Job) => Job) => useJobs.setState((s) => ({ jobs: s.jobs.map((j) => (j.id === id ? f(j) : j)) })); + +export function setItem(id: number, key: string, patch: Partial>) { + patchJob(id, (j) => ({ ...j, items: j.items.map((i) => (i.key === key ? { ...i, ...patch } : i)) })); +} + +export const jobOf = (id: number) => useJobs.getState().jobs.find((j) => j.id === id); + +// cancelJob asks a job to send nothing more. What it already sent finishes. +export const cancelJob = (id: number) => patchJob(id, (j) => ({ ...j, cancelling: true })); +export const isCancelling = (id: number) => !!jobOf(id)?.cancelling; + +// finishJob ends a job with its summary. A clean one goes after FADE_MS; one +// with a row kept, failed or not sent stays until it's dismissed. +export function finishJob(id: number, summary: string, now = Date.now()) { + patchJob(id, (j) => ({ ...j, summary, finished: now })); + const j = jobOf(id); + if (!j || outcomeOf(j) !== "clean") return; + // unref: node (pnpm test) needn't wait for it. + const t: unknown = setTimeout(() => dismissJob(id), FADE_MS); + (t as { unref?: () => void }).unref?.(); +} + +export const dismissJob = (id: number) => useJobs.setState((s) => ({ jobs: s.jobs.filter((j) => j.id !== id) })); + +export function setFolded(folded: boolean) { + useJobs.setState({ folded }); + save(FOLDED, folded); +} + +const settledStates: ItemState[] = ["done", "kept", "failed", "cancelled"]; +export const settled = (i: JobItem) => settledStates.includes(i.state); + +export function progressOf(j: Job) { + return { settled: j.items.filter(settled).length, total: j.items.length }; +} + +// The rows a report lists: whatever didn't go as asked. +export const leftOver = (j: Job) => j.items.filter((i) => i.state === "kept" || i.state === "failed" || i.state === "cancelled"); + +export function outcomeOf(j: Job): "running" | "clean" | "report" { + if (j.finished === undefined) return "running"; + return leftOver(j).length ? "report" : "clean"; +} + +// runPool runs fn over items, a few at a time, in order. Once stop() says +// so it takes no more, and answers with the items it never sent. +export async function runPool(items: T[], concurrency: number, fn: (item: T) => Promise, stop: () => boolean = () => false): Promise { + const queue = [...items]; + const unsent: T[] = []; + const worker = async () => { + for (let item = queue.shift(); item !== undefined; item = queue.shift()) { + if (stop()) { + unsent.push(item, ...queue.splice(0)); + return; + } + await fn(item); + } + }; + await Promise.all(Array.from({ length: Math.min(concurrency, items.length) }, worker)); + return unsent; +} diff --git a/app/src/lib/mock.ts b/app/src/lib/mock.ts index 21b70ce80..803a44260 100644 --- a/app/src/lib/mock.ts +++ b/app/src/lib/mock.ts @@ -104,6 +104,8 @@ const fresh = new URLSearchParams(location.search).has("fresh"); // archive script, as one with a big node_modules does, and answers once it // is gone. const slowRemove = Number(new URLSearchParams(location.search).get("slowremove")) || 0; +// ?mock=1&slowstop=MS: a box takes that long to answer a session's stop. +const slowStop = Number(new URLSearchParams(location.search).get("slowstop")) || undefined; if (fresh) { status.boxes = []; status.forwards = []; @@ -944,7 +946,7 @@ function boxCall(box: string, method: string, path: string, body?: unknown): Pro const name = decodeURIComponent(path.slice("sessions/".length)); sessions[box] = sessions[box].filter((s) => s.name !== name); setTimeout(() => emit({ type: "session.stopped", box, data: { name } }), 50); - return delay({ removed: name }); + return delay({ removed: name }, slowStop); } if (key === "POST locations") { const r = body as { name: string; path: string }; diff --git a/app/src/lib/remove-worktree.ts b/app/src/lib/remove-worktree.ts index 9cea35c2e..01d6684b4 100644 --- a/app/src/lib/remove-worktree.ts +++ b/app/src/lib/remove-worktree.ts @@ -1,7 +1,9 @@ import { toastError } from "@/components/error-note"; import { toastManager } from "@/components/ui/toast"; import type { Client } from "@/lib/api"; +import { plainError } from "@/lib/errors"; import { errorMessage } from "@/lib/format"; +import { finishJob, setItem, startJob } from "@/lib/jobs"; import { keptReason } from "@/lib/kept-reason"; import { unpinWorktree } from "@/lib/pinned-worktrees"; import { clearRemoving, inTurn, markRemoving, markScript, pruneRemovals, type RemovalKind, removalOf, removeResult, useRemovals } from "@/lib/removing"; @@ -47,11 +49,37 @@ export async function removeWorktreeOnBox( // dialog) can close, and goes when its box gets to it, one at a time per // box (inTurn). Its branch stays, and git still refuses to lose uncommitted // work: then a toast says so, and a removal's offers to remove it anyway. -export function queueRemoval(box: string, location: string, wt: Leaving, opts: { kind?: RemovalKind; force?: boolean } = {}) { +// With tray, it is a one-row job in the background-jobs tray (lib/jobs.ts) +// too, so a slow one can be watched. +export function queueRemoval(box: string, location: string, wt: Leaving, opts: { kind?: RemovalKind; force?: boolean; tray?: boolean } = {}) { const kind = opts.kind ?? "remove"; if (removalOf(useRemovals.getState().byKey, box, wt.path)) return; markRemoving(box, wt.path, kind, wt.name); - void inTurn(box, () => send(box, location, wt, kind, opts.force)); + const job = opts.tray ? trayJob(box, wt, kind) : undefined; + void inTurn(box, () => send(box, location, wt, kind, opts.force, job)); +} + +const KEY = "worktree"; + +function trayJob(box: string, wt: Leaving, kind: RemovalKind) { + const doing = kind === "archive" ? "Archiving" : "Removing"; + const id = startJob({ title: `${doing} ${wt.name}`, items: [{ key: KEY, group: box, label: wt.name, aside: wt.branch, message: `Waiting for ${box}` }] }); + const word = kind === "archive" ? "archive" : "remove"; + return { + sent: () => setItem(id, KEY, { state: "running", message: `${doing}…` }), + done: (res: { archive?: string }) => { + setItem(id, KEY, { state: "done", message: res.archive ? "Archiving: its archive script runs, then it goes" : kind === "archive" ? "Archived; its branch is kept" : "Removed" }); + finishJob(id, res.archive ? `Archiving ${wt.name}` : `${kind === "archive" ? "Archived" : "Removed"} ${wt.name}`); + }, + kept: () => { + setItem(id, KEY, { state: "kept", message: "Kept: uncommitted changes" }); + finishJob(id, `Kept ${wt.name}`); + }, + failed: (err: unknown) => { + setItem(id, KEY, { state: "failed", message: `Couldn't ${word}: ${plainError(err)}` }); + finishJob(id, `Couldn't ${word} ${wt.name}`); + }, + }; } type Leaving = { name: string; path: string; branch?: string }; @@ -59,11 +87,13 @@ type Leaving = { name: string; path: string; branch?: string }; // keepsWork is git refusing, in its own words, to lose uncommitted work. export const keepsWork = (err: unknown) => /modified|untracked|uncommitted|contains/i.test(errorMessage(err)); -async function send(box: string, location: string, wt: Leaving, kind: RemovalKind, force?: boolean) { +async function send(box: string, location: string, wt: Leaving, kind: RemovalKind, force?: boolean, job?: ReturnType) { try { const client = useStore.getState().client; if (!client) throw new Error("not connected"); + job?.sent(); const res = await removeWorktreeOnBox(client, box, location, wt, kind, { force }); + job?.done(res); if (kind === "remove") return; if (res.archive) toastManager.add({ title: `Archiving ${wt.name}`, description: "Its archive script is running on the box; the worktree goes when it finishes.", type: "info" }); else toastManager.add({ title: `Archived ${wt.name}`, description: wt.branch ? `Branch ${wt.branch} is kept.` : box, type: "success" }); @@ -71,6 +101,7 @@ async function send(box: string, location: string, wt: Leaving, kind: RemovalKin clearRemoving(box, wt.path); const why = force ? null : keptReason(errorMessage(err)); if (why) { + job?.kept(); if (kind === "archive") { toastManager.add({ type: "warning", @@ -85,7 +116,10 @@ async function send(box: string, location: string, wt: Leaving, kind: RemovalKin actionProps: { children: "Remove anyway", onClick: () => queueRemoval(box, location, wt, { force: true }) }, }); } - } else toastError(err, { title: `Couldn't ${kind} ${wt.name}`, box }); + } else { + job?.failed(err); + toastError(err, { title: `Couldn't ${kind} ${wt.name}`, box }); + } } } diff --git a/app/src/lib/stop-job.test.ts b/app/src/lib/stop-job.test.ts new file mode 100644 index 000000000..fb81aacc4 --- /dev/null +++ b/app/src/lib/stop-job.test.ts @@ -0,0 +1,121 @@ +// node --experimental-strip-types --test src/lib/stop-job.test.ts (pnpm test) +import assert from "node:assert/strict"; +import { test } from "node:test"; + +import { KEPT_OTHER_AGENT } from "./archive-after-stop.ts"; +import { cancelJob, jobOf, useJobs } from "./jobs.ts"; +import { runStopJob, type StopDeps, type StopRow, stopSummary } from "./stop-job.ts"; + +const row = (session: string, wt?: string, box = "devl"): StopRow => ({ + key: `${box}/${session}`, + box, + session, + label: wt ?? "shop", + where: { location: "shop", worktree: wt ? { name: wt, path: `/w/${wt}` } : { name: "shop", path: "/w/shop", main: true } }, +}); + +function fakeBoxes(over: Partial = {}) { + const calls: string[] = []; + const deps: StopDeps = { + stop: async (box, s) => void calls.push(`stop ${box}/${s}`), + liveAgents: async () => [], + archive: async (box, _loc, wt) => { + calls.push(`archive ${box}:${wt.name}`); + return {}; + }, + keepsWork: (err) => /uncommitted/.test(String(err)), + errorText: (err) => (err instanceof Error ? err.message : String(err)), + ...over, + }; + return { deps, calls }; +} + +const itemsOf = (id: number) => Object.fromEntries(jobOf(id)!.items.map((i) => [i.key, `${i.state}: ${i.message}`])); + +test("a clean run stops them all, archives their worktrees, and sums it up", async () => { + useJobs.setState({ jobs: [] }); + const { deps, calls } = fakeBoxes(); + const res = await runStopJob([row("a", "fix"), row("b", "perf"), row("c")], true, deps); + assert.equal(res.clean, true); + assert.equal(jobOf(res.job)!.title, "Stopping 3 agents"); + assert.equal(jobOf(res.job)!.summary, "3 stopped, 2 archived"); + assert.deepEqual(itemsOf(res.job), { + "devl/a": "done: Stopped · Archived; its branch is kept", + "devl/b": "done: Stopped · Archived; its branch is kept", + "devl/c": "done: Stopped", + }); + assert.deepEqual(calls.filter((c) => c.startsWith("archive")), ["archive devl:fix", "archive devl:perf"]); +}); + +test("without archiving, only the agents stop", async () => { + const { deps, calls } = fakeBoxes(); + const res = await runStopJob([row("a", "fix")], false, deps); + assert.equal(res.clean, true); + assert.equal(jobOf(res.job)!.summary, "1 stopped"); + assert.deepEqual(calls, ["stop devl/a"]); +}); + +test("kept and failed rows make a report, each saying why", async () => { + const { deps } = fakeBoxes({ + stop: async (_box, s) => { + if (s === "d") throw new Error("box went away"); + }, + liveAgents: async () => [{ dir: "/w/busy", name: "someone-else" }], + archive: async (_box, _loc, wt) => { + if (wt.name === "dirty") throw new Error("contains uncommitted changes"); + if (wt.name === "broken") throw new Error("permission denied"); + return wt.name === "scripted" ? { archive: "pnpm archive" } : {}; + }, + }); + const res = await runStopJob([row("a", "dirty"), row("b", "busy"), row("c", "broken"), row("d", "gone"), row("e", "scripted")], true, deps); + assert.equal(res.clean, false); + assert.deepEqual(itemsOf(res.job), { + "devl/a": "kept: Stopped · Kept: uncommitted changes", + "devl/b": `kept: Stopped · Kept: ${KEPT_OTHER_AGENT}`, + "devl/c": "failed: Stopped · Couldn't archive: permission denied", + "devl/d": "failed: Failed: box went away", + "devl/e": "done: Stopped · Archiving: its archive script runs, then it goes", + }); + assert.equal(jobOf(res.job)!.summary, "4 stopped, 1 archiving, 2 kept, 2 failed"); +}); + +test("Stop the rest sends nothing more; what was sent finishes", async () => { + let release!: () => void; + const hold = new Promise((r) => (release = r)); + const sent: string[] = []; + const { deps } = fakeBoxes({ + stop: async (_box, s) => { + sent.push(s); + await hold; + }, + }); + const rows = ["a", "b", "c", "d", "e", "f"].map((s) => row(s, `wt-${s}`)); + const running = runStopJob(rows, true, deps); + await new Promise((r) => setTimeout(r, 0)); + const id = useJobs.getState().jobs[0].id; + assert.deepEqual(sent, ["a", "b", "c"], "three at a time"); + cancelJob(id); + release(); + const res = await running; + assert.deepEqual(sent, ["a", "b", "c"]); + const items = itemsOf(res.job); + for (const s of ["a", "b", "c"]) assert.equal(items[`devl/${s}`], "cancelled: Stopped · Not archived"); + for (const s of ["d", "e", "f"]) assert.equal(items[`devl/${s}`], "cancelled: Not stopped"); + assert.equal(jobOf(res.job)!.summary, "3 stopped, 3 not stopped, 3 not archived"); + assert.equal(res.clean, false); +}); + +test("the job carries on in the store with nothing holding it", async () => { + useJobs.setState({ jobs: [] }); + const { deps } = fakeBoxes(); + // As the dialog does: start it and let go, with no view to keep it alive. + void runStopJob([row("a", "fix")], true, deps); + const id = useJobs.getState().jobs[0].id; + for (let i = 0; i < 20 && !jobOf(id)!.finished; i++) await new Promise((r) => setTimeout(r, 0)); + assert.equal(jobOf(id)!.summary, "1 stopped, 1 archived"); +}); + +test("the summary names what was kept, failed and never sent", () => { + assert.equal(stopSummary(17, 17, 0, Array(15).fill("archived")), "17 stopped, 15 archived"); + assert.equal(stopSummary(5, 3, 1, ["archived", "kept", "failed"]), "3 stopped, 1 archived, 1 kept, 2 failed, 1 not stopped"); +}); diff --git a/app/src/lib/stop-job.ts b/app/src/lib/stop-job.ts new file mode 100644 index 000000000..3441b28dc --- /dev/null +++ b/app/src/lib/stop-job.ts @@ -0,0 +1,134 @@ +// The board's bulk Stop as a background job (lib/jobs.ts): it stops the +// agents a few at a time, then, when asked, archives their worktrees where +// it's safe (lib/archive-after-stop.ts), one at a time on each box in the +// same line as every other removal there (inTurn), the boxes side by side. +// "Stop the rest" sends nothing more: rows not yet stopped say "Not +// stopped", worktrees not yet archived are left as they are. +// +// No imports from "@/": pnpm test runs this file in plain node. The calls to +// the boxes come in as deps (views/dashboard/stop.tsx). + +import { planArchive, type StopTarget, type WorktreeTarget } from "./archive-after-stop.ts"; +import { finishJob, isCancelling, jobOf, outcomeOf, runPool, setItem, startJob } from "./jobs.ts"; +import { inTurn } from "./removing.ts"; + +export interface StopRow extends StopTarget { + session: string; + // What the tray calls it: its place, and its name when the place is shared. + label: string; + aside?: string; +} + +export interface StopDeps { + stop(box: string, session: string): Promise; + // The agent sessions still live on a box, fresh, or undefined when it + // doesn't answer. + liveAgents(box: string): Promise<{ dir: string; name: string }[] | undefined>; + archive(box: string, location: string, wt: { name: string; path: string }): Promise<{ archive?: string }>; + // Git refusing, in its own words, to lose uncommitted work. + keepsWork(err: unknown): boolean; + errorText(err: unknown): string; +} + +export type WorktreeOutcome = "archived" | "archiving" | "kept" | "failed" | "cancelled"; + +export interface StopResult { + job: number; + stopped: Set; + // By worktree (removalKey). + worktrees: Record; + clean: boolean; +} + +// A few at a time, so a long list is quick without every request landing on +// a box at once. +export const CONCURRENCY = 3; + +const plural = (n: number, one: string, many = `${one}s`) => `${n} ${n === 1 ? one : many}`; + +export async function runStopJob(rows: StopRow[], archive: boolean, deps: StopDeps): Promise { + const job = startJob({ + title: `Stopping ${plural(rows.length, "agent")}`, + items: rows.map((r) => ({ key: r.key, group: r.box, label: r.label, aside: r.aside, message: "Waiting to stop" })), + cancellable: true, + }); + const stopped = new Set(); + const willArchive = (r: StopRow) => archive && !!r.where && !r.where.worktree.main; + const unsent = await runPool( + rows, + CONCURRENCY, + async (r) => { + setItem(job, r.key, { state: "running", message: "Stopping…" }); + try { + await deps.stop(r.box, r.session); + stopped.add(r.key); + setItem(job, r.key, willArchive(r) ? { state: "running", message: "Stopped" } : { state: "done", message: "Stopped" }); + } catch (err) { + setItem(job, r.key, { state: "failed", message: `Failed: ${deps.errorText(err)}` }); + } + }, + () => isCancelling(job), + ); + for (const r of unsent) setItem(job, r.key, { state: "cancelled", message: "Not stopped" }); + + const worktrees: Record = {}; + if (rows.some(willArchive)) { + const report = (w: WorktreeTarget, o: WorktreeOutcome, message: string) => { + worktrees[w.key] = o; + const state = o === "archived" || o === "archiving" ? "done" : o; + for (const k of w.rows) if (stopped.has(k)) setItem(job, k, { state, message: `Stopped · ${message}` }); + }; + const candidates = rows.filter(willArchive); + // The boxes' own listing, fresh, so an agent started a moment ago + // counts. A box that doesn't answer keeps its worktrees. + const boxes = [...new Set(candidates.map((r) => r.box))]; + const listed = await Promise.all(boxes.map(async (box) => ({ box, agents: await deps.liveAgents(box).catch(() => undefined) }))); + const live = listed.flatMap(({ box, agents }) => (agents ?? []).map((a) => ({ box, ...a }))); + const plan = planArchive( + candidates, + stopped, + live, + new Set(listed.filter((l) => !l.agents).map((l) => l.box)), + ); + for (const k of plan.kept) report(k, "kept", `Kept: ${k.why}`); + for (const w of plan.archive) for (const k of w.rows) if (stopped.has(k)) setItem(job, k, { state: "running", message: "Stopped · waiting to archive" }); + await Promise.all( + plan.archive.map((w) => + inTurn(w.box, async () => { + if (isCancelling(job)) return report(w, "cancelled", "Not archived"); + for (const k of w.rows) if (stopped.has(k)) setItem(job, k, { state: "running", message: "Archiving…" }); + try { + const res = await deps.archive(w.box, w.location, w.wt); + if (res.archive) report(w, "archiving", "Archiving: its archive script runs, then it goes"); + else report(w, "archived", "Archived; its branch is kept"); + } catch (err) { + if (deps.keepsWork(err)) report(w, "kept", "Kept: uncommitted changes"); + else report(w, "failed", `Couldn't archive: ${deps.errorText(err)}`); + } + }), + ), + ); + } + + finishJob(job, stopSummary(rows.length, stopped.size, unsent.length, Object.values(worktrees))); + const done = jobOf(job); + return { job, stopped, worktrees, clean: !!done && outcomeOf(done) === "clean" }; +} + +// stopSummary is what a finished job says: "17 stopped, 15 archived", and +// what was kept, failed or never sent. +export function stopSummary(total: number, stopped: number, unsent: number, worktrees: WorktreeOutcome[]) { + const count = (o: WorktreeOutcome) => worktrees.filter((w) => w === o).length; + const failed = total - stopped - unsent + count("failed"); + return [ + `${stopped} stopped`, + count("archived") && `${count("archived")} archived`, + count("archiving") && `${count("archiving")} archiving`, + count("kept") && `${count("kept")} kept`, + failed && `${failed} failed`, + unsent && `${unsent} not stopped`, + count("cancelled") && `${count("cancelled")} not archived`, + ] + .filter(Boolean) + .join(", "); +} diff --git a/app/src/views/dashboard/stop.tsx b/app/src/views/dashboard/stop.tsx index 65508c4ce..665feeb5b 100644 --- a/app/src/views/dashboard/stop.tsx +++ b/app/src/views/dashboard/stop.tsx @@ -1,4 +1,4 @@ -import { AlertTriangleIcon, BrushCleaningIcon, CheckIcon, EllipsisIcon, ListChecksIcon, SquareIcon, Trash2Icon, XIcon } from "lucide-react"; +import { AlertTriangleIcon, BrushCleaningIcon, EllipsisIcon, ListChecksIcon, SquareIcon, Trash2Icon } from "lucide-react"; import { useRef, useState } from "react"; import { create } from "zustand"; @@ -12,18 +12,15 @@ import { AlertDialog, AlertDialogDescription, AlertDialogFooter, AlertDialogHead import { Button } from "@/components/ui/button"; import { Checkbox } from "@/components/ui/checkbox"; import { Menu, MenuItem, MenuPopup, MenuSeparator, MenuTrigger } from "@/components/ui/menu"; -import { Spinner } from "@/components/ui/spinner"; import { toastManager } from "@/components/ui/toast"; import type { SessionEntry } from "@/hooks/use-agent-counts"; import { boxApi } from "@/lib/api"; -import { planArchive, type WorktreeTarget } from "@/lib/archive-after-stop"; import { agentOf, type SessionState } from "@/lib/derive"; import { plainError } from "@/lib/errors"; -import { errorMessage } from "@/lib/format"; -import { removeWorktreeOnBox } from "@/lib/remove-worktree"; -import { removalKey } from "@/lib/removing"; +import { keepsWork, removeWorktreeOnBox } from "@/lib/remove-worktree"; import { load, save } from "@/lib/storage"; import { scheduleRefresh, useStore } from "@/lib/store"; +import { runStopJob } from "@/lib/stop-job"; import { cn } from "@/lib/utils"; import { duration } from "@/views/dashboard/agent-card"; import { type AgentNames, describeAgent, startedAt } from "@/views/dashboard/names"; @@ -41,10 +38,6 @@ const plural = (n: number, one: string, many = `${one}s`) => `${n} ${n === 1 ? o // Busy agents: stopping one loses the turn it is in. const busy = (s: SessionState) => s === "running" || s === "waiting"; -// A few at a time, so a long list is quick without every request landing -// on a box at once. -const CONCURRENCY = 3; - // stopOne ends one session and drops it from the board straight away; the // box's next listing confirms it. async function stopOne(box: string, name: string) { @@ -58,91 +51,45 @@ async function stopOne(box: string, name: string) { scheduleRefresh(box, ["sessions"]); } -interface Progress { - state: "running" | "ok" | "failed"; - message?: string; -} - -// stopAgents stops several, CONCURRENCY at a time, reports each, and -// answers with the keys that stopped. -async function stopAgents(entries: SessionEntry[], onEach: (key: string, p: Progress) => void): Promise> { - const queue = [...entries]; - const stopped = new Set(); - const worker = async () => { - for (let e = queue.shift(); e; e = queue.shift()) { - const key = entryKey(e); - onEach(key, { state: "running" }); - try { - await stopOne(e.box, e.session.name); - stopped.add(key); - onEach(key, { state: "ok" }); - } catch (err) { - onEach(key, { state: "failed", message: plainError(err) }); - } - } - }; - await Promise.all(Array.from({ length: Math.min(CONCURRENCY, entries.length) }, worker)); - return stopped; -} - -// What happened to a stopped agent's worktree when the dialog archived. -interface WorktreeOutcome { - state: "running" | "archived" | "archiving" | "kept" | "failed"; - message: string; -} - -// keepsWork is git refusing, in its own words, to lose uncommitted work. -const keepsWork = (err: unknown) => /modified|untracked|uncommitted|contains/i.test(errorMessage(err)); - -const leftAsItWas = (o?: WorktreeOutcome) => o?.state === "kept" || o?.state === "failed"; - -// archiveStopped archives the stopped rows' worktrees the way the sidebar's -// Archive… does: the archive script runs, the branch is kept, and git -// refuses to lose uncommitted work. One at a time on each box, as removals -// go; the boxes side by side. It answers with each worktree's outcome. -async function archiveStopped(rows: Row[], stopped: Set, onEach: (wtKey: string, o: WorktreeOutcome) => void): Promise> { - const out: Record = {}; - const report = (key: string, o: WorktreeOutcome) => { - out[key] = o; - onEach(key, o); - }; - const { client } = useStore.getState(); - // The boxes' own listing, fresh, so an agent started a moment ago counts. - // A box that doesn't answer keeps its worktrees: nothing says they're free. - const candidates = [...new Set(rows.filter((r) => r.names.where && !r.names.where.worktree.main).map((r) => r.entry.box))]; - const listed = await Promise.all(candidates.map(async (box) => ({ box, sessions: client ? await boxApi.sessions(client, box).catch(() => undefined) : undefined }))); - const live = listed.flatMap(({ box, sessions }) => (sessions ?? []).filter((s) => !s.exited && agentOf(s)).map((s) => ({ box, dir: s.dir, name: s.name }))); - const plan = planArchive( - rows.map((r) => ({ key: r.key, box: r.entry.box, where: r.names.where && { location: r.names.where.location.name, worktree: r.names.where.worktree } })), - stopped, - live, - new Set(listed.filter((l) => !l.sessions).map((l) => l.box)), - ); - for (const k of plan.kept) report(k.key, { state: "kept", message: `Kept: ${k.why}` }); - const archiveOne = async (w: WorktreeTarget) => { - report(w.key, { state: "running", message: "Archiving…" }); - try { - if (!client) throw new Error("Not connected to the laptop agent"); - const res = await removeWorktreeOnBox(client, w.box, w.location, w.wt, "archive"); - report(w.key, res.archive ? { state: "archiving", message: "Archiving: its archive script runs, then it goes" } : { state: "archived", message: "Archived; its branch is kept" }); - } catch (err) { - report(w.key, keepsWork(err) ? { state: "kept", message: "Kept: uncommitted changes" } : { state: "failed", message: `Couldn't archive: ${plainError(err)}` }); - } - }; - const boxesOf = [...new Set(plan.archive.map((w) => w.box))]; - await Promise.all( - boxesOf.map(async (box) => { - for (const w of plan.archive.filter((x) => x.box === box)) await archiveOne(w); - }), +// stopInBackground stops the confirmed rows as a job in the tray +// (lib/stop-job.ts), archiving their worktrees where it's safe if asked. A +// clean run ends with the toast it always had. +async function stopInBackground(rows: Row[], archive: boolean) { + const res = await runStopJob( + rows.map((r) => ({ + key: r.key, + box: r.entry.box, + session: r.entry.session.name, + label: r.names.place, + aside: r.names.crowded ? r.names.name : undefined, + where: r.names.where && { location: r.names.where.location.name, worktree: r.names.where.worktree }, + })), + archive, + { + stop: stopOne, + liveAgents: async (box) => { + const client = useStore.getState().client; + const sessions = client ? await boxApi.sessions(client, box) : undefined; + return sessions?.filter((s) => !s.exited && agentOf(s)).map((s) => ({ dir: s.dir, name: s.name })); + }, + archive: (box, location, wt) => { + const client = useStore.getState().client; + if (!client) throw new Error("Not connected to the laptop agent"); + return removeWorktreeOnBox(client, box, location, wt, "archive"); + }, + keepsWork, + errorText: plainError, + }, ); - return out; -} - -// archiveSummary is " · archived 2 worktrees" (or archiving, while a script -// runs), for the toast and the done title alike. -function archiveSummary(outcomes: WorktreeOutcome[]) { - const gone = outcomes.filter((o) => o.state === "archived" || o.state === "archiving"); - return gone.length ? ` · ${gone.some((o) => o.state === "archiving") ? "archiving" : "archived"} ${plural(gone.length, "worktree")}` : ""; + if (!res.clean) return; + const outcomes = Object.values(res.worktrees); + const gone = outcomes.length; + const boxes = [...new Set(rows.map((r) => r.entry.box))].map((b) => `${rows.filter((r) => r.entry.box === b).length} on ${b}`); + toastManager.add({ + title: `Stopped ${plural(rows.length, "agent")}${gone ? ` · ${outcomes.includes("archiving") ? "archiving" : "archived"} ${plural(gone, "worktree")}` : ""}`, + description: `${boxes.join(", ")}. ${gone ? "Their branches are kept." : "Their worktrees are kept."}`, + type: "success", + }); } function namesOf(e: SessionEntry): AgentNames { @@ -230,14 +177,14 @@ const AFTER: { value: After; label: string; ms: number }[] = [ { value: "1d", label: "1 day", ms: 86_400_000 }, ]; -// StopDialog is the host; the dashboard renders it once. +// StopDialog is the host; the dashboard renders it once. It only asks: +// once confirmed it closes, and the stopping goes on in the tray. export function StopDialog() { const req = useStopDialog((s) => s.req); - const [running, setRunning] = useState(false); - const close = () => !running && useStopDialog.setState({ req: undefined }); + const close = () => useStopDialog.setState({ req: undefined }); return ( !o && close()}> - {req && useStopDialog.setState({ req: undefined })} />} + {req && } ); } @@ -249,7 +196,7 @@ interface Row { idle: number; } -function Body({ req, onRunning, onClose }: { req: StopRequest; onRunning(r: boolean): void; onClose(): void }) { +function Body({ req, onClose }: { req: StopRequest; onClose(): void }) { const cleanup = req.kind === "cleanup"; // Names are taken when the dialog opens: they renumber as agents stop. const [rows] = useState(() => { @@ -261,20 +208,14 @@ function Body({ req, onRunning, onClose }: { req: StopRequest; onRunning(r: bool // time) is idle on purpose: it is listed, never ticked for you. const overFor = (a: After) => new Set(rows.filter((r) => !r.entry.session.parked && r.idle > AFTER.find((x) => x.value === a)!.ms).map((r) => r.key)); const [ticked, setTicked] = useState>(() => (cleanup ? overFor(after) : new Set(rows.map((r) => r.key)))); - const [progress, setProgress] = useState>({}); const [archive, setArchive] = useState(() => load("berth.dashboard.stopArchives", false)); - const [outcomes, setOutcomes] = useState>({}); - const [phase, setPhase] = useState<"confirm" | "running" | "done">("confirm"); const cancel = useRef(null); const chosen = rows.filter((r) => ticked.has(r.key)); const busyCount = chosen.filter((r) => busy(r.entry.state)).length; - const ok = rows.filter((r) => progress[r.key]?.state === "ok").length; - const failed = rows.filter((r) => progress[r.key]?.state === "failed"); // The box shows only when there is a worktree to archive. const canArchive = chosen.some((r) => r.names.where && !r.names.where.worktree.main); const archiving = archive && canArchive; - const outcomeOf = (r: Row) => (r.names.where ? outcomes[removalKey(r.entry.box, r.names.where.worktree.path)] : undefined); const boxes = [...new Set(rows.map((r) => r.entry.box))].sort(); const toggle = (key: string, on: boolean) => setTicked((s) => { @@ -284,44 +225,19 @@ function Body({ req, onRunning, onClose }: { req: StopRequest; onRunning(r: bool return n; }); - const run = async () => { - const targets = chosen.map((r) => r.entry); - setPhase("running"); - onRunning(true); - const stopped = await stopAgents(targets, (key, p) => setProgress((s) => ({ ...s, [key]: p }))); - const found = archiving ? Object.values(await archiveStopped(chosen, stopped, (key, o) => setOutcomes((s) => ({ ...s, [key]: o })))) : []; - onRunning(false); - if (stopped.size < targets.length || found.some(leftAsItWas)) { - setPhase("done"); - return; - } - const perBox = [...new Set(targets.map((t) => t.box))].map((b) => `${targets.filter((t) => t.box === b).length} on ${b}`); - toastManager.add({ - title: `Stopped ${plural(targets.length, "agent")}${archiveSummary(found)}`, - description: `${perBox.join(", ")}. ${found.length ? "Their branches are kept." : "Their worktrees are kept."}`, - type: "success", - }); + const run = () => { onClose(); + void stopInBackground(chosen, archiving); }; - const summary = archiveSummary(Object.values(outcomes)); - - const title = - phase === "done" - ? failed.length - ? `Stopped ${ok} of ${ok + failed.length}${summary}` - : `Stopped ${plural(ok, "agent")}${summary}` - : cleanup - ? "Clean up idle agents" - : req.column - ? `Stop ${chosen.length === rows.length ? "all " : ""}${plural(chosen.length, "agent")} in ${req.column}?` - : `Stop ${plural(chosen.length, "agent")}?`; - const description = - phase === "done" - ? `${failed.length ? "The rest were left running. " : ""}${Object.values(outcomes).some(leftAsItWas) ? "Some worktrees were kept as they are. " : ""}Each one says why below.` - : cleanup - ? `Stops agents in Done and Ready that have sat idle for a while. Working agents and ones waiting on you are left alone. ${archiving ? "Their worktrees are archived where it's safe; branches are kept." : "Worktrees and their files are kept."}` - : `Each one's session ends on its box; ${archiving ? "then its worktree is archived where it's safe, and its branch is kept." : "its worktree and files are kept."}${busyCount ? ` ${busyCount === 1 ? "One is" : `${busyCount} are`} still working or waiting on you, and ${busyCount === 1 ? "its turn ends" : "their turns end"} too.` : ""}`; + const title = cleanup + ? "Clean up idle agents" + : req.column + ? `Stop ${chosen.length === rows.length ? "all " : ""}${plural(chosen.length, "agent")} in ${req.column}?` + : `Stop ${plural(chosen.length, "agent")}?`; + const description = cleanup + ? `Stops agents in Done and Ready that have sat idle for a while. Working agents and ones waiting on you are left alone. ${archiving ? "Their worktrees are archived where it's safe; branches are kept." : "Worktrees and their files are kept."}` + : `Each one's session ends on its box; ${archiving ? "then its worktree is archived where it's safe, and its branch is kept." : "its worktree and files are kept."}${busyCount ? ` ${busyCount === 1 ? "One is" : `${busyCount} are`} still working or waiting on you, and ${busyCount === 1 ? "its turn ends" : "their turns end"} too.` : ""}`; return ( // The keyboard starts on Cancel: Enter right after ⌫ or a menu item must @@ -332,7 +248,7 @@ function Body({ req, onRunning, onClose }: { req: StopRequest; onRunning(r: bool {description} - {cleanup && phase === "confirm" && ( + {cleanup && (
    Idle longer than
      {group.map((r) => ( - toggle(r.key, on)} /> + toggle(r.key, on)} /> ))}
    @@ -370,13 +286,13 @@ function Body({ req, onRunning, onClose }: { req: StopRequest; onRunning(r: bool })}
    - {cleanup && phase === "confirm" && rows.length > 0 && ( + {cleanup && rows.length > 0 && (

    {chosen.length ? `${plural(chosen.length, "agent")} of ${rows.length} picked.` : `None idle that long.`} Tick or untick any of them.

    )} - {phase === "confirm" && canArchive && ( + {canArchive && (