From 00cad4fa91b2dd7d84a961bdc5bc65c809b967ca Mon Sep 17 00:00:00 2001 From: Stener1 Date: Wed, 21 Oct 2026 14:28:42 +0000 Subject: [PATCH 1/2] fix(box): a dirty worktree is refused before its archive script runs Archiving or removing a worktree without force used to stop its services and run the archive script first, and only then let git refuse a worktree with uncommitted or untracked files: the teardown ran on work that was then kept, and the box had already answered "archiving". Now it checks git status up front and refuses in git's own words, so every Archive path keeps dirty work before anything is torn down. Co-Authored-By: Claude Opus 5.5 --- internal/box/api.go | 15 ++++++++++++--- internal/box/locations.go | 12 ++++++++++++ internal/box/scripts_test.go | 27 +++++++++++++++++++++++++++ 3 files changed, 51 insertions(+), 3 deletions(-) diff --git a/internal/box/api.go b/internal/box/api.go index c7c92f243..208ab006c 100644 --- a/internal/box/api.go +++ b/internal/box/api.go @@ -578,9 +578,10 @@ func (b *Box) removeWorktree(w http.ResponseWriter, r *http.Request) error { // dropWorktree removes a worktree: its services stop, and with an archive // script the script runs in the background and the removal follows if it -// succeeds (async); without one it goes at once. done, when set, hears how -// it ended. A review worktree's removal is journaled as review.removed, -// with why. +// succeeds (async); without one it goes at once. Unless forced, a worktree +// with uncommitted or untracked files is refused before any of that, in +// git's words. done, when set, hears how it ended. A review worktree's +// removal is journaled as review.removed, with why. func (b *Box) dropWorktree(from string, loc Location, name, dir, branch string, force bool, reason string, done func(error)) (async bool, err error) { _, mark := b.Locations.reviewAt(dir) if mark != nil && branch == "" { @@ -594,6 +595,14 @@ func (b *Box) dropWorktree(from string, loc Location, name, dir, branch string, done(err) } } + // git refuses a worktree with uncommitted or untracked files unless + // forced. Ask before anything is torn down, so an archive script never + // runs on work that git will then keep. + if !force { + if dirtyWorktree(dir) { + return false, fmt.Errorf("git worktree remove: '%s' contains modified or untracked files, use --force to delete it", dir) + } + } b.stopServices(loc.Name, name) removed := func(ctx context.Context) { if b.Sessions != nil { diff --git a/internal/box/locations.go b/internal/box/locations.go index 164729e6b..c9367126a 100644 --- a/internal/box/locations.go +++ b/internal/box/locations.go @@ -537,6 +537,18 @@ func git(ctx context.Context, args ...string) ([]byte, error) { return cmd.CombinedOutput() } +// dirtyWorktree says git would refuse to remove dir unless forced: it has +// uncommitted or untracked files. When git can't tell, it says no, and git's +// own check at removal decides. +func dirtyWorktree(dir string) bool { + ctx, cancel := context.WithTimeout(context.Background(), 60*time.Second) + defer cancel() + cmd := exec.CommandContext(ctx, "git", "-C", dir, "status", "--porcelain") + cmd.Env = append(os.Environ(), "GIT_TERMINAL_PROMPT=0") + out, err := cmd.Output() + return err == nil && strings.TrimSpace(string(out)) != "" +} + func expandHome(path string) string { if path == "~" || strings.HasPrefix(path, "~/") { if home, err := os.UserHomeDir(); err == nil { diff --git a/internal/box/scripts_test.go b/internal/box/scripts_test.go index d2f519b24..220b3a35d 100644 --- a/internal/box/scripts_test.go +++ b/internal/box/scripts_test.go @@ -131,3 +131,30 @@ func TestArchiveRunsBeforeRemovalAndAFailureKeepsTheWorktree(t *testing.T) { t.Fatal("a successful archive did not remove the worktree") } } + +func TestADirtyWorktreeIsRefusedBeforeItsArchiveScriptRuns(t *testing.T) { + repo := gitRepo(t) + t.Setenv("SHELL", "/bin/sh") + ctx := context.Background() + b := &Box{Name: "devbox", Locations: NewLocations(filepath.Join(t.TempDir(), "locations.json")), Events: &events.Bus{}, LogDir: t.TempDir()} + loc, _ := b.Locations.Add(ctx, "cal", repo) + wt, _ := b.Locations.CreateWorktree(ctx, "cal", "billing", "", "") + if err := os.WriteFile(filepath.Join(wt.Path, "notes.txt"), []byte("not committed\n"), 0o644); err != nil { + t.Fatal(err) + } + ran := filepath.Join(t.TempDir(), "ran") + loc.Scripts.Archive = "touch " + ran + + async, err := b.dropWorktree("app", loc, wt.Name, wt.Path, "", false, "removed", nil) + // git's own words, as its refusal would have been: the app keys on them. + if err == nil || async || !strings.Contains(err.Error(), "contains modified or untracked files") { + t.Fatalf("a dirty worktree was not refused up front: async=%v err=%v", async, err) + } + time.Sleep(200 * time.Millisecond) + if _, err := os.Stat(ran); !os.IsNotExist(err) { + t.Fatal("the archive script ran on a dirty worktree") + } + if _, err := os.Stat(filepath.Join(wt.Path, "notes.txt")); err != nil { + t.Fatal("the uncommitted file is gone") + } +} From 64bc8b4d8df6fe9517b7ab80463a3962f81d5728 Mon Sep 17 00:00:00 2001 From: Stener1 Date: Wed, 21 Oct 2026 14:28:47 +0000 Subject: [PATCH 2/2] feat(app): stopping several agents can archive their worktrees too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Agent Dashboard's Stop and Clean up dialogs get "Archive their worktrees where it's safe" (remembered, off at first). Ticked, the agents stop, then each of their worktrees is archived once, through the same path as the sidebar's Archive…: the archive script runs and the branch is kept. One with uncommitted changes, with another agent still running in it or a folder under it, where an agent didn't stop, or on a box that didn't list its sessions is kept as it is, and its row says why. Co-Authored-By: Claude Opus 5.5 --- .../skills/verify-berthd/features/README.md | 1 + .../verify-berthd/features/dashboard-stop.md | 27 ++++ .../scripts/archive-dirty-live.sh | 32 ++++ .../scenarios/dashboard-stop-archive.mjs | 65 +++++++++ app/e2e/dashboard-stop-archive.spec.ts | 88 +++++++++++ app/src/lib/archive-after-stop.test.ts | 59 ++++++++ app/src/lib/archive-after-stop.ts | 64 ++++++++ app/src/views/dashboard/stop.tsx | 137 ++++++++++++++++-- docs/changelog.mdx | 15 ++ 9 files changed, 472 insertions(+), 16 deletions(-) create mode 100644 .claude/skills/verify-berthd/features/dashboard-stop.md create mode 100755 .claude/skills/verify-berthd/scripts/archive-dirty-live.sh create mode 100644 .claude/skills/verify-berthd/scripts/scenarios/dashboard-stop-archive.mjs create mode 100644 app/e2e/dashboard-stop-archive.spec.ts create mode 100644 app/src/lib/archive-after-stop.test.ts create mode 100644 app/src/lib/archive-after-stop.ts diff --git a/.claude/skills/verify-berthd/features/README.md b/.claude/skills/verify-berthd/features/README.md index 34d009765..11bac2e6a 100644 --- a/.claude/skills/verify-berthd/features/README.md +++ b/.claude/skills/verify-berthd/features/README.md @@ -14,6 +14,7 @@ The maintained list of what a person can do in the app, and how to prove each on | --- | --- | --- | | Sidebar places: Pinned, More, Hidden, counts on rows (Home's waiting, Review's, a plugin's) | [sidebar-nav.md](sidebar-nav.md) | `plugin-nav-badges` | | 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) | | 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 | diff --git a/.claude/skills/verify-berthd/features/dashboard-stop.md b/.claude/skills/verify-berthd/features/dashboard-stop.md new file mode 100644 index 000000000..aded969f5 --- /dev/null +++ b/.claude/skills/verify-berthd/features/dashboard-stop.md @@ -0,0 +1,27 @@ +# Stopping agents from the Agent Dashboard + +Stopping ends an agent's session on its box (`app/src/views/dashboard/stop.tsx`). Its worktree and files stay unless the bulk dialog is told to archive them. The archive goes through `removeWorktreeOnBox(…, "archive")` (`lib/remove-worktree.ts`), the same path as the sidebar's Archive… and the Worktrees table's bulk archive. `lib/archive-after-stop.ts` decides which worktrees to archive and which to keep (unit tests next to it). The box refuses a worktree with uncommitted or untracked files before it stops anything or runs the archive script (`dropWorktree` in `internal/box/api.go`), in git's words, so every Archive path keeps dirty work up front. + +## Sub-features + +- `stop-one`: a card's ⋯ → **Stop agent…** (asks first) and **Stop and remove worktree** (removes it without asking; git keeps a dirty one, and a toast offers Remove anyway). +- `stop-picked`: **Select**, click cards, **Stop N…** (or ⌫). Also a column's ⋯ → **Stop all N in …**. The dialog lists them by box, each with a checkbox. A busy one says "working" or "needs you". +- `cleanup`: **Clean up…** in the header, or a Done/Ready column's ⋯ → **Clean up idle agents…**. It ticks the ones idle longer than 1 hour / 4 hours / 1 day (remembered in `berth.dashboard.cleanupAfter`). A parked agent is listed but never ticked for you. +- `stop-archive`: in both bulk dialogs, **Archive their worktrees where it's safe**. It shows only when a ticked row has a worktree that isn't the main checkout, starts unticked, and is remembered (`berth.dashboard.stopArchives`). Ticked, the agents stop first. Then each of their worktrees is archived once: one at a time on each box, the boxes side by side. A row reads `Stopped · Archived; its branch is kept`, `Stopped · Archiving: its archive script runs, then it goes` (the location has an archive script), `Stopped · Kept: uncommitted changes` (git refused), `Stopped · Kept: another agent is still running there` (an agent session outside the batch is live in it or in a folder under it, by the box's own fresh listing; plain shells and services don't count), `Stopped · Kept: an agent there didn't stop`, `Stopped · Kept: couldn't check for other agents on its box` (its box didn't answer the listing), or `Stopped · Couldn't archive: …`. A kept or failed one holds the dialog open on "Stopped N agents · archived/archiving M worktrees". Otherwise it closes with a toast saying the same. + +## How to get to it (user POV) + +- ⌘J opens the Agent Dashboard. **Select**, click cards, then **Stop N…** in the bar at the bottom. + +## Driving it with drive.mjs + +- `node .claude/skills/verify-berthd/scripts/drive.mjs dashboard-stop-archive --port $PORT` picks four agents. It covers each outcome: search-perf archives through shop's archive script, ci-flake on gpu archives at once, checkout-fix is kept for uncommitted changes, and order-export is kept because two more agents run there. It ticks the box, stops them, checks each row, then checks the Worktrees table (two gone, two still there), and that **Clean up…** opens with the box ticked. +- On a real box: `scripts/dev-box.sh up`, then `scripts/archive-dirty-live.sh` archives a dirty and a clean worktree of acme-shop under an archive script, through the agent's API. The dirty one is refused at once (400, "contains modified or untracked files"), and its file stays while the script doesn't run. The clean one answers 202, the script runs in it, and it goes. `dev-box.sh down` afterwards. +- `app/e2e/dashboard-stop-archive.spec.ts` asserts the same, plus unticked (nothing archived, the old copy and toast) and a main checkout alone (no box). + +## Gotchas + +- With the default prefs the sidebar is the rail layout, which shows one project at a time. Read the outcome in the Worktrees table, or open with `labs: false` prefs, as the e2e does, to use the sidebar's `worktree-row`s. +- In the mock, checkout-fix's agent is in Needs you, not Done: it is the only dirty worktree in the fixtures. +- The mock's archive script takes 1.5s, so search-perf leaves the table a moment after the dialog says Archiving. +- axe over the open dialog flags the board's cards behind it (`.opacity-70` meta text), not the dialog. diff --git a/.claude/skills/verify-berthd/scripts/archive-dirty-live.sh b/.claude/skills/verify-berthd/scripts/archive-dirty-live.sh new file mode 100755 index 000000000..0a34657c3 --- /dev/null +++ b/.claude/skills/verify-berthd/scripts/archive-dirty-live.sh @@ -0,0 +1,32 @@ +#!/usr/bin/env bash +# archive-dirty-live.sh: on the box dev-box.sh starts, archive a dirty and a +# clean worktree of acme-shop under an archive script, through the laptop +# agent's API as the app does. The dirty one must be refused up front in +# git's words (400), with its file kept and the script never run; the clean +# one archives (202), the script runs in it, and it goes. +# Run dev-box.sh up first; dev-box.sh down cleans it all up. +set -euo pipefail +here=$(cd "$(dirname "$0")" && pwd) +eval "$("$here/dev-box.sh" env 2>/dev/null)" +api=http://127.0.0.1:9378/v1/boxes/devq/api +log=$(mktemp -d)/ran.log +call() { curl -sS -H "Authorization: Bearer $VERIFY_BERTH_TOKEN" -H "Content-Type: application/json" "$@"; } +fail() { echo "FAIL: $*" >&2; exit 1; } + +call -X PUT "$api/locations/acme-shop/scripts" -d "{\"Archive\":\"echo \\\"ran in \$PWD\\\" >> $log\"}" >/dev/null +dirty=$(call -X POST "$api/locations/acme-shop/worktrees" -d '{"name":"verify-dirty"}' | python3 -c 'import json,sys; print(json.load(sys.stdin)["path"])') +call -X POST "$api/locations/acme-shop/worktrees" -d '{"name":"verify-clean"}' >/dev/null +echo "not committed" > "$dirty/notes.txt" + +out=$(call -w '\n%{http_code}' -X DELETE "$api/locations/acme-shop/worktrees/verify-dirty") +echo "dirty: $out" +[[ $(tail -1 <<<"$out") == 400 && $out == *"contains modified or untracked files"* ]] || fail "the dirty worktree was not refused up front" +out=$(call -w '\n%{http_code}' -X DELETE "$api/locations/acme-shop/worktrees/verify-clean") +echo "clean: $out" +[[ $(tail -1 <<<"$out") == 202 ]] || fail "the clean worktree did not start archiving" + +for _ in $(seq 40); do grep -q verify-clean "$log" 2>/dev/null && break; sleep 0.25; done +grep -q verify-clean "$log" || fail "the archive script did not run on the clean worktree" +! grep -q verify-dirty "$log" || fail "the archive script ran on the dirty worktree" +[[ -f $dirty/notes.txt ]] || fail "the dirty worktree lost its file" +echo "OK: dirty refused before its script, clean archived" diff --git a/.claude/skills/verify-berthd/scripts/scenarios/dashboard-stop-archive.mjs b/.claude/skills/verify-berthd/scripts/scenarios/dashboard-stop-archive.mjs new file mode 100644 index 000000000..856aefe9e --- /dev/null +++ b/.claude/skills/verify-berthd/scripts/scenarios/dashboard-stop-archive.mjs @@ -0,0 +1,65 @@ +// The Agent Dashboard's bulk Stop, with "Archive their worktrees where it's +// safe" ticked (views/dashboard/stop.tsx): a clean worktree with an archive +// script archives in the background, one without archives at once, a dirty +// one is kept, and one with another agent still in it is kept. +// features/dashboard-stop.md. + +const picks = [ + // devl / search-perf: shop has an archive script, so it runs first. + "Speed up product search", + // gpu / ci-flake: no archive script, so it goes at once. + "Move acme billing to the new ledger", + // devl / checkout-fix: uncommitted work, so git keeps it. + "Fix checkout webhook retries", + // devl / order-export: two more agents still run there. + "Export orders as CSV from the admin", +]; + +export async function run({ page, shot, step, expect }) { + const dialog = page.getByRole("alertdialog"); + const archiveBox = dialog.getByRole("checkbox", { name: /Archive their worktrees where it's safe/ }); + const row = (place) => dialog.getByRole("group", { name: "Agents to stop" }).getByRole("listitem").filter({ hasText: place }); + // The Worktrees table lists every box's; the sidebar shows one project. + const listed = (name) => page.getByRole("checkbox", { name: `Select ${name}`, exact: true }); + + step("open the Agent Dashboard, Select, and pick four agents"); + await page.keyboard.press("Meta+j"); + await expect(page.getByRole("heading", { name: "Agent Dashboard" })).toBeVisible(); + await page.getByRole("button", { name: "Select", exact: true }).click(); + for (const t of picks) await page.locator("article[role=button]", { hasText: t }).getByText(t).click(); + await page.getByRole("button", { name: `Stop ${picks.length}…` }).click(); + await expect(archiveBox).not.toBeChecked(); + await expect(dialog).toContainText("its worktree and files are kept"); + await shot("1-stop-dialog-unticked"); + + step("tick 'Archive their worktrees where it's safe': the copy says so"); + await archiveBox.click(); + await expect(archiveBox).toBeChecked(); + await expect(dialog).toContainText("then its worktree is archived where it's safe"); + await shot("2-stop-dialog-ticked"); + + step("Stop 4 agents: kept work holds the dialog open, each row with its outcome"); + await dialog.getByRole("button", { name: `Stop ${picks.length} agents` }).click(); + await expect(dialog.getByRole("heading")).toHaveText("Stopped 4 agents · archiving 2 worktrees"); + await expect(row("search-perf")).toContainText("Stopped · Archiving: its archive script runs, then it goes"); + await expect(row("ci-flake")).toContainText("Stopped · Archived; its branch is kept"); + await expect(row("checkout-fix")).toContainText("Stopped · Kept: uncommitted changes"); + await expect(row("order-export")).toContainText("Stopped · Kept: another agent is still running there"); + await shot("3-stop-dialog-done"); + + step("Done, then the Worktrees table: the archived ones are gone, the kept ones stay"); + await dialog.getByRole("button", { name: "Done" }).click(); + await page.getByTestId("nav-worktrees").click(); + await expect(listed("checkout-fix")).toBeVisible(); + await expect(listed("order-export")).toBeVisible(); + await expect(listed("ci-flake")).toHaveCount(0, { timeout: 10_000 }); + await expect(listed("search-perf")).toHaveCount(0, { timeout: 10_000 }); + await shot("4-worktrees-after"); + + step("the choice is remembered: Clean up… opens with the box ticked"); + await page.keyboard.press("Meta+j"); + await page.getByRole("button", { name: "Clean up…" }).click(); + await expect(archiveBox).toBeChecked(); + await expect(dialog).toContainText("Their worktrees are archived where it's safe"); + await shot("5-cleanup-remembers"); +} diff --git a/app/e2e/dashboard-stop-archive.spec.ts b/app/e2e/dashboard-stop-archive.spec.ts new file mode 100644 index 000000000..25358670b --- /dev/null +++ b/app/e2e/dashboard-stop-archive.spec.ts @@ -0,0 +1,88 @@ +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. + +const prefs = { labs: false, labsChosen: true, version: 3 }; + +// A card on the board, picked while selecting (or opened otherwise). +const pick = (app: App, title: string) => app.page.locator("article[role=button]", { hasText: title }).getByText(title).click(); + +async function stopPicked(app: App, titles: string[]) { + const { page } = app; + await page.getByTestId("nav-home").click(); + await page.getByRole("button", { name: "Select", exact: true }).click(); + for (const t of titles) await pick(app, t); + await page.getByRole("button", { name: `Stop ${titles.length}…` }).click(); + return page.getByRole("alertdialog"); +} + +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 }); + +test("ticked, Stop archives a clean worktree and keeps a dirty one, saying why", 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"]); + 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(); + + // 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(); + + // The clean one goes once its script has run; the dirty one stays. + 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 }) => { + 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(app.worktree("devl/order-export")).toBeVisible(); +}); + +test("unticked, Stop only stops: the worktree stays", 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(app.worktree("devl/search-perf")).toBeVisible(); +}); + +test("the archive choice is remembered, and not offered for the main checkout alone", async ({ app }) => { + mockOnly("stops agents"); + await app.open({ prefs }); + let dialog = await stopPicked(app, ["Speed up product search"]); + await archiveBox(app).click(); + await dialog.getByRole("button", { name: "Cancel" }).click(); + await expect(dialog).toHaveCount(0); + await expect.poll(() => app.stored("berth.dashboard.stopArchives")).toBe(true); + + await app.page.getByRole("button", { name: "Stop 1…" }).click(); + dialog = app.page.getByRole("alertdialog"); + await expect(archiveBox(app)).toBeChecked(); + await dialog.getByRole("button", { name: "Cancel" }).click(); + + // Only gpu's main checkout: there is nothing to archive. + await pick(app, "Speed up product search"); + await pick(app, "Plan the checkout release"); + await app.page.getByRole("button", { name: "Stop 1…" }).click(); + await expect(app.page.getByRole("alertdialog")).toContainText("its worktree and files are kept"); + await expect(archiveBox(app)).toHaveCount(0); +}); diff --git a/app/src/lib/archive-after-stop.test.ts b/app/src/lib/archive-after-stop.test.ts new file mode 100644 index 000000000..48c1a6982 --- /dev/null +++ b/app/src/lib/archive-after-stop.test.ts @@ -0,0 +1,59 @@ +// node --experimental-strip-types --test src/lib/archive-after-stop.test.ts (pnpm test) +import assert from "node:assert/strict"; +import { test } from "node:test"; + +import { KEPT_OTHER_AGENT, KEPT_STOP_FAILED, KEPT_UNCHECKED, planArchive, type StopTarget } from "./archive-after-stop.ts"; + +const at = (session: string, wt: string, main = false): StopTarget => ({ key: `devl/${session}`, box: "devl", where: { location: "shop", worktree: { name: wt, path: `/w/${wt}`, main } } }); +const names = (ws: { wt: { name: string } }[]) => ws.map((w) => w.wt.name); + +test("each worktree is archived once, however many stopped agents worked there", () => { + const targets = [at("a", "fix"), at("b", "fix"), at("c", "perf")]; + const plan = planArchive(targets, new Set(targets.map((t) => t.key)), []); + assert.deepEqual(names(plan.archive), ["fix", "perf"]); + assert.deepEqual(plan.archive[0].rows, ["devl/a", "devl/b"]); + assert.deepEqual(plan.kept, []); +}); + +test("the main checkout and an agent with no worktree are never candidates", () => { + const loose: StopTarget = { key: "devl/x", box: "devl" }; + const targets = [at("a", "shop", true), loose]; + const plan = planArchive(targets, new Set(targets.map((t) => t.key)), []); + assert.deepEqual(plan, { archive: [], kept: [] }); +}); + +test("another live agent outside the batch keeps the worktree; the batch's own don't", () => { + const targets = [at("a", "fix"), at("b", "perf")]; + const live = [ + { box: "devl", dir: "/w/fix/app", name: "other" }, + { box: "devl", dir: "/w/perf-old", name: "beside" }, + { box: "devl", dir: "/w/perf", name: "b" }, + { box: "gpu", dir: "/w/perf", name: "elsewhere" }, + ]; + const plan = planArchive(targets, new Set(targets.map((t) => t.key)), live); + assert.deepEqual(names(plan.archive), ["perf"]); + assert.deepEqual( + plan.kept.map((k) => [k.wt.name, k.why]), + [["fix", KEPT_OTHER_AGENT]], + ); +}); + +test("a worktree whose stops all failed is left out; one where some failed is kept", () => { + const targets = [at("a", "fix"), at("b", "perf"), at("c", "perf")]; + const plan = planArchive(targets, new Set(["devl/b"]), []); + assert.deepEqual(plan.archive, []); + assert.deepEqual( + plan.kept.map((k) => [k.wt.name, k.why]), + [["perf", KEPT_STOP_FAILED]], + ); +}); + +test("a box that didn't list its sessions keeps its worktrees", () => { + const targets = [at("a", "fix"), { ...at("b", "perf"), key: "gpu/b", box: "gpu" }]; + const plan = planArchive(targets, new Set(targets.map((t) => t.key)), [], new Set(["devl"])); + assert.deepEqual(names(plan.archive), ["perf"]); + assert.deepEqual( + plan.kept.map((k) => [k.wt.name, k.why]), + [["fix", KEPT_UNCHECKED]], + ); +}); diff --git a/app/src/lib/archive-after-stop.ts b/app/src/lib/archive-after-stop.ts new file mode 100644 index 000000000..022d75e22 --- /dev/null +++ b/app/src/lib/archive-after-stop.ts @@ -0,0 +1,64 @@ +// After the board's bulk Stop: which of the stopped agents' worktrees to +// archive, and which to keep as they are. Each worktree is archived once, +// however many of the stopped agents worked in it. The main checkout never +// is, nor one where an agent of the batch didn't stop, nor one where an agent +// outside the batch still runs, there or in a folder under it (the box ends +// those too). Plain shells don't count: they close with the archive, as when +// archiving by hand. + +import { removalKey } from "./removing.ts"; + +export interface StopTarget { + // box/session, as the board keys its agents. + key: string; + box: string; + where?: { location: string; worktree: { name: string; path: string; main?: boolean } }; +} + +export interface WorktreeTarget { + // removalKey(box, path). + key: string; + box: string; + location: string; + wt: { name: string; path: string }; + // The batch's rows that worked in it. + rows: string[]; +} + +export interface Plan { + archive: WorktreeTarget[]; + kept: (WorktreeTarget & { why: string })[]; +} + +const within = (dir: string, root: string) => dir === root || dir.startsWith(root.endsWith("/") ? root : `${root}/`); + +export const KEPT_STOP_FAILED = "an agent there didn't stop"; +export const KEPT_OTHER_AGENT = "another agent is still running there"; +export const KEPT_UNCHECKED = "couldn't check for other agents on its box"; + +// planArchive groups the stopped rows by worktree. stopped holds the keys +// whose stop went through; liveAgents are the agent sessions still live on +// the boxes (stopped ones may not have left the listing yet); unchecked are +// boxes that didn't list them, whose worktrees are kept. +export function planArchive(targets: StopTarget[], stopped: Set, liveAgents: { box: string; dir: string; name: string }[], unchecked: ReadonlySet = new Set()): Plan { + const byWt = new Map(); + for (const t of targets) { + if (!t.where || t.where.worktree.main) continue; + const key = removalKey(t.box, t.where.worktree.path); + const w = byWt.get(key) ?? { key, box: t.box, location: t.where.location, wt: { name: t.where.worktree.name, path: t.where.worktree.path }, rows: [] }; + w.rows.push(t.key); + byWt.set(key, w); + } + const batch = new Set(targets.map((t) => t.key)); + const plan: Plan = { archive: [], kept: [] }; + for (const w of byWt.values()) { + const ok = w.rows.filter((r) => stopped.has(r)).length; + // Every stop failed: those rows say so, and nothing here changed. + if (ok === 0) continue; + if (ok < w.rows.length) plan.kept.push({ ...w, why: KEPT_STOP_FAILED }); + else if (unchecked.has(w.box)) plan.kept.push({ ...w, why: KEPT_UNCHECKED }); + else if (liveAgents.some((s) => s.box === w.box && within(s.dir, w.wt.path) && !batch.has(`${s.box}/${s.name}`))) plan.kept.push({ ...w, why: KEPT_OTHER_AGENT }); + else plan.archive.push(w); + } + return plan; +} diff --git a/app/src/views/dashboard/stop.tsx b/app/src/views/dashboard/stop.tsx index b80c09d98..65508c4ce 100644 --- a/app/src/views/dashboard/stop.tsx +++ b/app/src/views/dashboard/stop.tsx @@ -16,8 +16,12 @@ 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 { load, save } from "@/lib/storage"; import { scheduleRefresh, useStore } from "@/lib/store"; import { cn } from "@/lib/utils"; @@ -28,6 +32,7 @@ import { type AgentNames, describeAgent, startedAt } from "@/views/dashboard/nam // column, or the ones that have sat idle. Stopping ends the agent's session // on its box; its worktree and files stay, so another agent can pick up // there. Nothing working or waiting on you is stopped unless you picked it. +// The bulk dialog can also archive their worktrees after, where it's safe. export const entryKey = (e: SessionEntry) => `${e.box}/${e.session.name}`; const sinceOf = (e: SessionEntry) => new Date(e.session.state_since ?? e.session.created).getTime(); @@ -58,25 +63,86 @@ interface Progress { message?: string; } -// stopAgents stops several, CONCURRENCY at a time, and reports each. -async function stopAgents(entries: SessionEntry[], onEach: (key: string, p: Progress) => void): Promise { +// 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]; - let failed = 0; + 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) { - failed++; onEach(key, { state: "failed", message: plainError(err) }); } } }; await Promise.all(Array.from({ length: Math.min(CONCURRENCY, entries.length) }, worker)); - return failed; + 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); + }), + ); + 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")}` : ""; } function namesOf(e: SessionEntry): AgentNames { @@ -196,6 +262,8 @@ function Body({ req, onRunning, onClose }: { req: StopRequest; onRunning(r: bool 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); @@ -203,6 +271,10 @@ function Body({ req, onRunning, onClose }: { req: StopRequest; onRunning(r: bool 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) => { @@ -216,20 +288,29 @@ function Body({ req, onRunning, onClose }: { req: StopRequest; onRunning(r: bool const targets = chosen.map((r) => r.entry); setPhase("running"); onRunning(true); - const failures = await stopAgents(targets, (key, p) => setProgress((s) => ({ ...s, [key]: p }))); + 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 (failures) { + 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")}`, description: `${perBox.join(", ")}. Their worktrees are kept.`, type: "success" }); + 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", + }); onClose(); }; + const summary = archiveSummary(Object.values(outcomes)); + const title = phase === "done" - ? `Stopped ${ok} of ${ok + failed.length}` + ? failed.length + ? `Stopped ${ok} of ${ok + failed.length}${summary}` + : `Stopped ${plural(ok, "agent")}${summary}` : cleanup ? "Clean up idle agents" : req.column @@ -237,10 +318,10 @@ function Body({ req, onRunning, onClose }: { req: StopRequest; onRunning(r: bool : `Stop ${plural(chosen.length, "agent")}?`; const description = phase === "done" - ? "The rest were left running. Each one says why below." + ? `${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. Worktrees and their files are kept." - : `Each one's session ends on its box; 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.` : ""}`; + ? `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 @@ -281,7 +362,7 @@ function Body({ req, onRunning, onClose }: { req: StopRequest; onRunning(r: bool
    {group.map((r) => ( - toggle(r.key, on)} /> + toggle(r.key, on)} /> ))}
@@ -295,6 +376,23 @@ function Body({ req, onRunning, onClose }: { req: StopRequest; onRunning(r: bool

)} + {phase === "confirm" && canArchive && ( + + )} + {phase === "done" ? ( @@ -314,16 +412,19 @@ function Body({ req, onRunning, onClose }: { req: StopRequest; onRunning(r: bool ); } -function StopRow({ row, ticked, progress, locked, cleanup, onToggle }: { row: Row; ticked: boolean; progress?: Progress; locked: boolean; cleanup: boolean; onToggle(on: boolean): void }) { +function StopRow({ row, ticked, progress, outcome, locked, cleanup, onToggle }: { row: Row; ticked: boolean; progress?: Progress; outcome?: WorktreeOutcome; locked: boolean; cleanup: boolean; onToggle(on: boolean): void }) { const { entry, names } = row; const state = entry.state; + const stopped = progress?.state === "ok"; return (