diff --git a/.claude/skills/verify-berthd/features/README.md b/.claude/skills/verify-berthd/features/README.md index 4db31eba..09a5731e 100644 --- a/.claude/skills/verify-berthd/features/README.md +++ b/.claude/skills/verify-berthd/features/README.md @@ -16,6 +16,7 @@ The maintained list of what a person can do in the app, and how to prove each on | Back and forward: the window strip's arrows, ⌘[ and ⌘], the mouse's buttons; archiving or removing the worktree in front goes back, not Home; the dashboard keeps its scroll | [navigation-history.md](navigation-history.md) | `navigation-history`; `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) | +| 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 and board: no question card or answers but the options the agent's screen shows; the mode menu while turn check waits | [chat-ask-card.md](chat-ask-card.md) | `chat-ask-card` (a real box, `--agent`); `app/e2e/ask-card.spec.ts` on a stand-in box | 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 00000000..9a1e8aae --- /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 00000000..3200d761 --- /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 3056b7cb..96feed7c 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 d2416ece..5ad56054 100644 --- a/app/src/components/sidebar/actions.tsx +++ b/app/src/components/sidebar/actions.tsx @@ -25,7 +25,7 @@ import { ago, errorMessage } from "@/lib/format"; import { plainError } from "@/lib/errors"; import { scheduleRefresh, useStore } from "@/lib/store"; import { cn } from "@/lib/utils"; -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"; @@ -417,7 +417,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({ @@ -425,19 +427,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 a30d5ced..8d49a8ab 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 = []; @@ -344,7 +348,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 @@ -856,6 +860,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 97f6163b..5b20ba9c 100644 --- a/app/src/lib/remove-worktree.ts +++ b/app/src/lib/remove-worktree.ts @@ -2,7 +2,7 @@ import { toastError } from "@/components/error-note"; import { toastManager } from "@/components/ui/toast"; import type { Client } from "@/lib/api"; import { errorMessage } from "@/lib/format"; -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"; @@ -39,49 +39,50 @@ 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); - // git's own words for work it would lose. - if (!force && /modified|untracked|uncommitted|contains/i.test(errorMessage(err))) { +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); + if (!force && keepsWork(err)) { + 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: "It has uncommitted changes, so git left it as it is.", 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 662269e3..30974680 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 b3191f51..9d4eef69 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 1cee27d0..c2bcb17f 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 1402e0c6..649cd69e 100644 --- a/docs/changelog.mdx +++ b/docs/changelog.mdx @@ -54,6 +54,11 @@ again. beside the rail, it now opens New worktree for that project, instead of a menu to add a project or a box. Those stay on the rail's own + and the boxes row at the bottom of the sidebar. +- **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.29