From b77041ef5dfa1651caf0e3be93189ec029a6b8cd Mon Sep 17 00:00:00 2001 From: Stener1 Date: Mon, 2 Nov 2026 15:03:43 +0000 Subject: [PATCH] fix(app): your sidebar layout survives the agent restarting as the app starts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The sidebar's layout read once at connect; a failed read left the default showing for the launch, and the next change saved the default over the stored file. The read is now tried again (lib/poll, and at once on a reconnect) until it arrives, and nothing can change the layout before then: Customize sidebar… waits with a note, and other changes are refused with the same note. A change also keeps the place of pinned or hidden places that aren't available (a plugin off or still loading), and saves run one at a time, so a write that fails can't roll back a later one. The verify skill gains sidebar-layout-reload (mock) and sidebar-layout-restart.sh, which restarts a real laptop agent as the app asks for the layout and checks sidebar.json is unchanged. Co-Authored-By: Claude Opus 5.5 --- .../skills/verify-berthd/features/README.md | 2 +- .../verify-berthd/features/sidebar-nav.md | 9 +- .../skills/verify-berthd/scripts/agent-box.sh | 100 +++++++++++ .../scenarios/sidebar-layout-reload.mjs | 63 +++++++ .../scenarios/sidebar-layout-restart-live.mjs | 62 +++++++ .../scripts/sidebar-layout-restart.sh | 31 ++++ app/e2e/fixtures.ts | 2 +- app/e2e/sidebar-layout-load.spec.ts | 72 ++++++++ app/src/components/sidebar/nav.tsx | 12 +- app/src/hooks/use-berth-connection.ts | 13 +- app/src/lib/mock.ts | 15 ++ app/src/lib/nav-core.test.ts | 167 ++++++++++++++++++ app/src/lib/nav-core.ts | 152 ++++++++++++++++ app/src/lib/nav.ts | 143 ++++++--------- docs/changelog.mdx | 8 + 15 files changed, 750 insertions(+), 101 deletions(-) create mode 100755 .claude/skills/verify-berthd/scripts/agent-box.sh create mode 100644 .claude/skills/verify-berthd/scripts/scenarios/sidebar-layout-reload.mjs create mode 100644 .claude/skills/verify-berthd/scripts/scenarios/sidebar-layout-restart-live.mjs create mode 100755 .claude/skills/verify-berthd/scripts/sidebar-layout-restart.sh create mode 100644 app/e2e/sidebar-layout-load.spec.ts create mode 100644 app/src/lib/nav-core.test.ts create mode 100644 app/src/lib/nav-core.ts diff --git a/.claude/skills/verify-berthd/features/README.md b/.claude/skills/verify-berthd/features/README.md index 11a07f637..2be81d7f0 100644 --- a/.claude/skills/verify-berthd/features/README.md +++ b/.claude/skills/verify-berthd/features/README.md @@ -12,7 +12,7 @@ The maintained list of what a person can do in the app, and how to prove each on | Feature | File | Scenario | | --- | --- | --- | -| 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` | +| Sidebar places: Pinned, More, Hidden, counts on rows (Home's waiting, Review's, a plugin's); the arrangement kept across a reload and an agent restart | [sidebar-nav.md](sidebar-nav.md) | `plugin-nav-badges`; `sidebar-layout-reload` (the layout across a reload, and before it loads); `../scripts/sidebar-layout-restart.sh` (a real agent restarting as the app starts); `app/e2e/sidebar-layout-load.spec.ts` asserts it | | 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 | | 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 | diff --git a/.claude/skills/verify-berthd/features/sidebar-nav.md b/.claude/skills/verify-berthd/features/sidebar-nav.md index 71827ad08..def8aefd0 100644 --- a/.claude/skills/verify-berthd/features/sidebar-nav.md +++ b/.claude/skills/verify-berthd/features/sidebar-nav.md @@ -7,6 +7,7 @@ The top of the sidebar lists the app's places (Home, Review, Worktrees, Automati - `nav-pinned`: pinned places as rows, in the stored order. - `nav-more`: More, one row and a menu of the rest; highlighted, naming the place, while you're on one of its places. - `nav-customize`: the Customize sidebar sheet (drag, or Pin / Move to More / Hide buttons, Reset to default); right-click a row for the same. +- `nav-layout-load`: the arrangement is read from the laptop agent (`/v1/app/sidebar`, `~/.berth/app/sidebar.json`) and outlives a reload. Until it is read the default shows and nothing can change it: More's **Customize sidebar…** is disabled with a note (`nav-not-loaded`), and any other change (a row's right-click, a drag, ⌘K's Customize sidebar…) is refused with the same note in a toast. A failed read is tried again (1 s, doubling to 10 s, and at once when the agent reconnects) until it arrives, then the stored arrangement replaces the default. A missing or empty file is the default, with Customize open. A change keeps the place of pinned or hidden ids that aren't available (a plugin off or still loading). - `nav-counts`: a count on a row. Home shows agents waiting for you (amber), Review shows what's to review (quiet), and a plugin's sidebar item shows its `useCount` (amber, or quiet with `urgent: false`; none for 0, undefined or a hook that throws). An amber count under More also shows on More's row. ## How to get to it (user POV) @@ -18,12 +19,16 @@ The top of the sidebar lists the app's places (Home, Review, Worktrees, Automati ## Driving it with drive.mjs - `node .claude/skills/verify-berthd/scripts/drive.mjs plugin-nav-badges --port $PORT` loads a test plugin with five sidebar items (counts 3, quiet 2, 4 from `useStorage`, 0, and one that throws). It checks More's row shows 3, the More menu shows `Waiting3`, `Quiet2`, `Stored4` and no count on Zero or Boom, pins four of them through Customize, then checks the rows: Waiting `3` amber (`text-warning-foreground`), Quiet `2` not amber, Zero and Boom no `nav-badge`, More's row now `4`, Home's own badge still there, and Waiting still opens its screen (`aria-current="page"`). -- Hooks: `nav-` on each row's button (`nav-home`, `nav-review`, `nav-plugin::`), `nav-more` on More's button, `nav-badge` on a row's count. A row's count is `page.locator("li", { has: page.getByTestId("nav-") }).getByTestId("nav-badge")`. +- `node .claude/skills/verify-berthd/scripts/drive.mjs sidebar-layout-reload --port $PORT` (`nav-layout-load`, mock): pins Kits through Customize, reloads and finds it still pinned; then opens the app with `failapp=sidebar` (the mock fails that document until `window.__mockApp.recover("sidebar")`), checks the default shows, Customize is disabled with the note in More's menu, Hide on a row's right-click is refused with the note, and the stored document didn't change; recovers, and checks Kits comes back and Customize opens. +- `.claude/skills/verify-berthd/scripts/sidebar-layout-restart.sh $PORT` (`nav-layout-load`, a real agent): seeds `sidebar.json` (Kits pinned, Automations hidden) for a laptop agent of this checkout with an empty box (`scripts/agent-box.sh`, ports 9467/9468 and 7546), drives `sidebar-layout-restart-live`, which stops the agent as the app asks for the layout and starts it 2 s later, and compares `sidebar.json`'s sha256 before and after. For a before, serve main's build on another port and run it against that: it stays on the default. +- Hooks: `nav-` on each row's button (`nav-home`, `nav-review`, `nav-plugin::`), `nav-more` on More's button, `nav-badge` on a row's count, `nav-customize` on More's Customize sidebar…, `nav-not-loaded` on its note. A row's count is `page.locator("li", { has: page.getByTestId("nav-") }).getByTestId("nav-badge")`. - Pin through the sheet: `getByRole("dialog", { name: "Customize sidebar" })`, then the `li` holding the label's exact text, its `Pin` button. ## Gotchas - New places, a plugin's included, land under More until pinned, so a plugin's count first shows on More's row, and only the first amber one. - While you're on a place under More, More's row names it instead of showing a count. -- The mock saves the layout in memory only; a reload starts from the default. +- The mock keeps the layout in the tab's session storage, so a reload or a `goto` in the same page keeps it; a new browser context starts from the default. +- Scenarios that look for `nav-` rows need `params: { sidebar: "list" }`: the default layout is the rail. +- A fresh laptop agent with no box shows onboarding instead of the sidebar; `agent-box.sh` pairs an empty box for that reason. - Counts in the More menu are plain text after the label: the menu item's text is `Waiting3`. diff --git a/.claude/skills/verify-berthd/scripts/agent-box.sh b/.claude/skills/verify-berthd/scripts/agent-box.sh new file mode 100755 index 000000000..3e5dcfe9b --- /dev/null +++ b/.claude/skills/verify-berthd/scripts/agent-box.sh @@ -0,0 +1,100 @@ +#!/usr/bin/env bash +# agent-box.sh up|env|stop|start|restart [SECONDS]|down: a laptop agent of +# this checkout's build with one empty box paired ("devr", so the app is +# past onboarding), on their own state and ports (agent UI 127.0.0.1:9468, +# proxy 9467, box 7546; never 1377-1379), for what the app reads from the +# agent itself, such as its /v1/app documents. No agent runs on the box. +# `up` builds both into the state folder; `stop`, `start` and `restart 2` +# (2 seconds between) take the laptop agent down and up again, the box +# untouched; `env` prints what drive.mjs needs; `down` stops both and +# deletes the state. +# +# VERIFY_AGENT_DIR=/tmp/va-NNNN the state (default /tmp/verify-berthd-agent) +# VERIFY_AGENT_BIN= another build of the laptop agent, say main's +# +# The app's documents are in $VERIFY_AGENT_DIR/user/app/.json: seed one +# before `up` (sidebar-layout-restart.sh does). The box's HOME is in the +# state folder too, so it never touches the real one's agent hooks. +set -euo pipefail +root=$(git -C "$(dirname "$0")" rev-parse --show-toplevel) +dir=${VERIFY_AGENT_DIR:-/tmp/verify-berthd-agent} +ui=127.0.0.1:9468 +addr=127.0.0.1:7546 +bin=${VERIFY_AGENT_BIN:-$dir/bin/berth} +lap_env=(BERTH_HOME="$dir/home" BERTH_USER_DIR="$dir/user" BERTH_UI_ADDR="$ui" BERTH_PROXY_ADDR=127.0.0.1:9467) +box_env=(HOME="$dir/box-home" BERTH_HOME="$dir/box-home" BERTH_USER_DIR="$dir/box-user" TMUX_TMPDIR="$dir/tmux") +box() { env "${box_env[@]}" "$dir/bin/berthd" "$@"; } + +# ours: the agent listening on the UI port with this state, no other. +ours() { + for pid in $(ss -ltnpH "sport = :${ui##*:}" 2>/dev/null | grep -o 'pid=[0-9]*' | cut -d= -f2 | sort -u); do + tr '\0' '\n' <"/proc/$pid/environ" 2>/dev/null | grep -qx "BERTH_HOME=$dir/home" && echo "$pid" + done + return 0 +} +start() { + mkdir -p "$dir"/{home,user} + env "${lap_env[@]}" setsid nohup "$bin" agent >>"$dir/agent.log" 2>&1 &2 + exit 1 +} +stop() { + local pids + pids=$(ours) + [[ -z $pids ]] && return + kill $pids + for _ in $(seq 100); do + [[ -z $(ours) ]] && ! ss -ltnH "sport = :${ui##*:}" | grep -q . && return + sleep 0.1 + done + echo "the agent ($pids) is still running after 10 s" >&2 + exit 1 +} + +case ${1:-} in +up) + for p in 7546 9467 9468; do + if ss -ltn | grep -q ":$p "; then echo "port $p is taken: is an agent up already? ($0 down)" >&2; exit 1; fi + done + mkdir -p "$dir"/{home,user,box-home,box-user,tmux,bin} && chmod 700 "$dir/tmux" + (cd "$root" && go build -o "$dir/bin/" ./cmd/berth ./cmd/berthd) + env "${box_env[@]}" setsid nohup "$dir/bin/berthd" serve --listen "$addr" >"$dir/berthd.log" 2>&1 "$dir/berthd.pid" + for _ in $(seq 120); do + grep -q "berthd serving" "$dir/berthd.log" && break + kill -0 $box_pid 2>/dev/null || { echo "berthd exited before it served: $dir/berthd.log" >&2; exit 1; } + sleep 0.5 + done + grep -q "berthd serving" "$dir/berthd.log" || { echo "the box isn't serving: $dir/berthd.log" >&2; exit 1; } + start + env "${lap_env[@]}" "$bin" pair "$(box pair --address "$addr" --json | jq -r .link)" --name devr >/dev/null + echo "up: agent http://$ui with box devr ($addr), state $dir" + ;; +start) start ;; +stop) stop ;; +restart) + stop + sleep "${2:-0}" + start + ;; +env) + # VERIFY_BERTH_TOKEN is a secret: export it, don't print it anywhere else. + echo "export VERIFY_BERTH_TOKEN=$(env "${lap_env[@]}" "$bin" ui-token | jq -r .token) VERIFY_AGENT_DIR=$dir" + ;; +down) + stop + [[ -f $dir/berthd.pid ]] && kill "$(cat "$dir/berthd.pid")" 2>/dev/null || true + env TMUX_TMPDIR="$dir/tmux" tmux -L berth kill-server 2>/dev/null || true + rm -rf "$dir" + echo "down: $dir removed" + ;; +*) + echo "usage: agent-box.sh up|env|stop|start|restart [SECONDS]|down" >&2 + exit 2 + ;; +esac diff --git a/.claude/skills/verify-berthd/scripts/scenarios/sidebar-layout-reload.mjs b/.claude/skills/verify-berthd/scripts/scenarios/sidebar-layout-reload.mjs new file mode 100644 index 000000000..8fafc551b --- /dev/null +++ b/.claude/skills/verify-berthd/scripts/scenarios/sidebar-layout-reload.mjs @@ -0,0 +1,63 @@ +// The sidebar's arrangement outlives a reload, and a layout that doesn't +// load leaves the default showing with Customize blocked until it arrives +// (lib/nav.ts). The mock keeps the document across reloads, and +// ?failapp=sidebar fails it until __mockApp.recover("sidebar"). +// features/sidebar-nav.md. + +// The list sidebar, where the places are rows (nav-). +export const params = { sidebar: "list" }; + +export async function run({ page, shot, step, expect }) { + const sidebar = page.getByTestId("sidebar"); + const stored = () => page.evaluate(() => window.__mockApp.doc("sidebar")); + + step("pin Kits in Customize sidebar"); + await expect(sidebar.getByTestId("nav-kits")).toHaveCount(0); + await page.getByTestId("nav-more").click(); + await page.getByTestId("nav-customize").click(); + const sheet = page.getByRole("dialog", { name: "Customize sidebar" }); + await sheet.locator("li", { has: page.getByText("Kits", { exact: true }) }).getByRole("button", { name: "Pin" }).click(); + await sheet.getByRole("button", { name: "Done" }).click(); + await expect(sheet).toBeHidden(); + await expect(sidebar.getByTestId("nav-kits")).toBeVisible(); + await shot("1-pinned"); + + step("reload: Kits is still pinned"); + await page.reload(); + await expect(sidebar.getByTestId("nav-home")).toBeVisible({ timeout: 20_000 }); + await expect(sidebar.getByTestId("nav-kits")).toBeVisible(); + await shot("2-after-reload"); + const saved = await stored(); + + step("start again while the agent can't answer for the layout: the default shows"); + const url = new URL(page.url()); + url.searchParams.set("failapp", "sidebar"); + await page.goto(url.href); + await expect(sidebar.getByTestId("nav-automations")).toBeVisible({ timeout: 20_000 }); + await expect(sidebar.getByTestId("nav-kits")).toHaveCount(0); + + step("More's Customize sidebar… is blocked, with the note"); + await page.getByTestId("nav-more").click(); + await expect(page.getByTestId("nav-customize")).toHaveAttribute("aria-disabled", "true"); + await expect(page.getByTestId("nav-not-loaded")).toBeVisible(); + await shot("3-customize-blocked"); + await page.keyboard.press("Escape"); + + step("Hide on a row's right-click is refused with the same note, and Review stays"); + await sidebar.getByTestId("nav-review").click({ button: "right" }); + await page.getByRole("menuitem", { name: "Hide" }).click(); + await expect(page.getByText("Your saved layout hasn't loaded yet").last()).toBeVisible(); + await expect(sidebar.getByTestId("nav-review")).toBeVisible(); + await shot("4-change-refused"); + if (JSON.stringify(await stored()) !== JSON.stringify(saved)) throw new Error("the stored layout changed while it hadn't loaded"); + + step("the agent answers again: the stored layout replaces the default, and Customize opens"); + await page.evaluate(() => window.__mockApp.recover("sidebar")); + await expect(sidebar.getByTestId("nav-kits")).toBeVisible({ timeout: 20_000 }); + await page.getByTestId("nav-more").click(); + await expect(page.getByTestId("nav-not-loaded")).toHaveCount(0); + await page.getByTestId("nav-customize").click(); + await expect(sheet).toBeVisible(); + await shot("5-recovered"); + if (JSON.stringify(await stored()) !== JSON.stringify(saved)) throw new Error("the stored layout changed"); +} diff --git a/.claude/skills/verify-berthd/scripts/scenarios/sidebar-layout-restart-live.mjs b/.claude/skills/verify-berthd/scripts/scenarios/sidebar-layout-restart-live.mjs new file mode 100644 index 000000000..a62887be6 --- /dev/null +++ b/.claude/skills/verify-berthd/scripts/scenarios/sidebar-layout-restart-live.mjs @@ -0,0 +1,62 @@ +// A real laptop agent restarts just as the app asks it for the sidebar's +// layout, as the first launch of an updated Mac app restarts its agent. +// The app keeps trying, and the saved layout replaces the default once the +// agent is back; nothing is written meanwhile. Run it through +// ../sidebar-layout-restart.sh, which seeds the layout (Kits pinned, +// Automations hidden), starts the agent (../agent-box.sh) and compares +// sidebar.json before and after. features/sidebar-nav.md. +import { execFileSync, spawn } from "node:child_process"; +import { fileURLToPath } from "node:url"; + +export const live = true; +// The requests that meet the stopped agent. +export const allowErrors = [/ERR_CONNECTION_REFUSED/, /Failed to load resource/, /Failed to fetch/, /NetworkError/]; + +// The list sidebar, where the places are rows (nav-). +export const params = { sidebar: "list" }; + +const agentBox = fileURLToPath(new URL("../agent-box.sh", import.meta.url)); + +export async function run({ page, shot, step, expect }) { + const sidebar = page.getByTestId("sidebar"); + + step("the saved layout shows: Kits pinned, Automations hidden"); + await expect(sidebar.getByTestId("nav-kits")).toBeVisible({ timeout: 20_000 }); + await expect(sidebar.getByTestId("nav-automations")).toHaveCount(0); + await shot("1-saved-layout"); + + step("start the app again; as it asks for the layout, the agent stops, and starts 2 s later"); + let restarted = false; + let back; + await page.route(/\/v1\/app\/sidebar(?:\?|$)/, async (route) => { + if (!restarted && route.request().method() === "GET") { + restarted = true; + execFileSync(agentBox, ["stop"], { stdio: "inherit" }); + back = new Promise((resolve, reject) => { + setTimeout(() => { + const p = spawn(agentBox, ["start"], { stdio: "inherit" }); + p.on("exit", (code) => (code === 0 ? resolve() : reject(new Error(`agent-box.sh start: exit ${code}`)))); + }, 2000); + }); + } + await route.continue().catch(() => {}); + }); + await page.reload(); + await expect(sidebar.getByTestId("nav-home")).toBeVisible({ timeout: 30_000 }); + await expect.poll(() => restarted, { timeout: 30_000 }).toBe(true); + await back; + step("the agent is back: Kits pinned and Automations hidden again"); + await expect(sidebar.getByTestId("nav-kits")).toBeVisible({ timeout: 30_000 }); + await expect(sidebar.getByTestId("nav-automations")).toHaveCount(0); + await shot("2-after-restart"); + + step("Customize sidebar opens, with Kits under Pinned and Automations under Hidden"); + await page.getByTestId("nav-more").click(); + await page.getByTestId("nav-customize").click(); + const sheet = page.getByRole("dialog", { name: "Customize sidebar" }); + await expect(sheet).toBeVisible(); + await expect(sheet.locator("section", { has: page.getByRole("heading", { name: "Pinned" }) })).toContainText("Kits"); + await expect(sheet.locator("section", { has: page.getByRole("heading", { name: "Hidden" }) })).toContainText("Automations"); + await shot("3-customize"); + await sheet.getByRole("button", { name: "Done" }).click(); +} diff --git a/.claude/skills/verify-berthd/scripts/sidebar-layout-restart.sh b/.claude/skills/verify-berthd/scripts/sidebar-layout-restart.sh new file mode 100755 index 000000000..b0af4fd3b --- /dev/null +++ b/.claude/skills/verify-berthd/scripts/sidebar-layout-restart.sh @@ -0,0 +1,31 @@ +#!/usr/bin/env bash +# sidebar-layout-restart.sh PORT: does the sidebar's saved layout survive +# the laptop agent restarting while the app starts? +# +# Seeds a layout (Kits pinned, Automations hidden) in a throwaway agent's +# state, starts a real agent of this checkout (agent-box.sh), drives the +# app served on PORT through scenarios/sidebar-layout-restart-live.mjs +# (which restarts the agent as the app asks for the layout), then checks +# sidebar.json is byte for byte what was seeded. Prints the evidence folder +# and one JSON line; exit 1 if the scenario or the comparison failed. +# +# VERIFY_AGENT_DIR (default /tmp/vb-sidebar-layout) is deleted at the end. +# OUT= for the evidence (drive.mjs --out). +set -euo pipefail +port=${1:?usage: sidebar-layout-restart.sh PORT} +here=$(cd "$(dirname "$0")" && pwd) +export VERIFY_AGENT_DIR=${VERIFY_AGENT_DIR:-/tmp/vb-sidebar-layout} +file=$VERIFY_AGENT_DIR/user/app/sidebar.json +trap '"$here/agent-box.sh" down >/dev/null' EXIT + +rm -rf "$VERIFY_AGENT_DIR" && mkdir -p "$(dirname "$file")" +printf '%s' '{"version":2,"pinned":["home","kits","review","worktrees"],"more":[],"hidden":["automations"]}' >"$file" +before=$(sha256sum "$file" | cut -d' ' -f1) +"$here/agent-box.sh" up +eval "$("$here/agent-box.sh" env)" +ok=true +node "$here/drive.mjs" sidebar-layout-restart-live --port "$port" --agent http://127.0.0.1:9468 ${OUT:+--out "$OUT"} || ok=false +after=$(sha256sum "$file" | cut -d' ' -f1) +[[ $before == "$after" ]] || ok=false +printf '{"ok":%s,"sidebar_json_before":"%s","sidebar_json_after":"%s"}\n' "$ok" "$before" "$after" +$ok diff --git a/app/e2e/fixtures.ts b/app/e2e/fixtures.ts index e7cb5f190..a9ae90b0e 100644 --- a/app/e2e/fixtures.ts +++ b/app/e2e/fixtures.ts @@ -13,7 +13,7 @@ import { type BrowserContext, expect, type Locator, type Page, test as base } fr // // Stable hooks, so tests never click text that moves: data-testid where // nothing else was stable (worktree-row with data-worktree="box/name", -// nav-, nav-more, nav-badge, pane with data-pane-kind, chat, chat-item with data-kind, +// nav-, nav-more, nav-badge, nav-customize, nav-not-loaded, pane with data-pane-kind, chat, chat-item with data-kind, // composer, question-form, queued-reply, artifacts-chip/-popover, // attachment-chip, panel with data-panel, browser-pane with data-mode, // chat-background, header-picture with data-picture, header-picture-settings, settings-nav-, settings-, theme-option with diff --git a/app/e2e/sidebar-layout-load.spec.ts b/app/e2e/sidebar-layout-load.spec.ts new file mode 100644 index 000000000..058a4f7d6 --- /dev/null +++ b/app/e2e/sidebar-layout-load.spec.ts @@ -0,0 +1,72 @@ +import type { Page } from "@playwright/test"; + +import { expect, mockOnly, test } from "./fixtures"; + +// The sidebar's arrangement is read from the laptop agent (lib/nav.ts, +// /v1/app/sidebar). Until it is read the default shows and nothing can be +// changed, so the default is never saved over it; a read that fails is +// tried again until the stored arrangement arrives. The mock keeps the +// document across reloads, and ?failapp=sidebar fails it until +// __mockApp.recover("sidebar"). + +const sidebar = (page: Page) => page.getByTestId("sidebar"); +type MockApp = { doc(key: string): unknown; recover(key: string): void }; +const stored = (page: Page) => page.evaluate(() => (window as unknown as { __mockApp: MockApp }).__mockApp.doc("sidebar")); +const recover = (page: Page) => page.evaluate(() => (window as unknown as { __mockApp: MockApp }).__mockApp.recover("sidebar")); + +async function pinKits(page: Page) { + await page.getByTestId("nav-more").click(); + await page.getByTestId("nav-customize").click(); + const sheet = page.getByRole("dialog", { name: "Customize sidebar" }); + await sheet.locator("li", { has: page.getByText("Kits", { exact: true }) }).getByRole("button", { name: "Pin" }).click(); + await sheet.getByRole("button", { name: "Done" }).click(); + await expect(sheet).toBeHidden(); + await expect(sidebar(page).getByTestId("nav-kits")).toBeVisible(); +} + +test("a pinned place is still pinned after a reload", async ({ app }) => { + mockOnly("pins a place"); + await app.open(); + const page = app.page; + await expect(sidebar(page).getByTestId("nav-kits")).toHaveCount(0); + await pinKits(page); + await page.reload(); + await expect(sidebar(page).getByTestId("nav-home")).toBeVisible(); + await expect(sidebar(page).getByTestId("nav-kits")).toBeVisible(); +}); + +test("a layout that doesn't load: the default shows, Customize waits, then the stored layout replaces it", async ({ app }) => { + mockOnly("fails the sidebar's document"); + await app.open(); + const page = app.page; + await pinKits(page); + const saved = await stored(page); + expect((saved as { pinned: string[] }).pinned).toContain("kits"); + + // The app starts again while the agent can't answer for the layout. + await app.open({ params: { failapp: "sidebar" } }); + await expect(sidebar(page).getByTestId("nav-automations")).toBeVisible(); + await expect(sidebar(page).getByTestId("nav-kits")).toHaveCount(0); + + // Customize is blocked, and says why, in More's menu; a change from a + // row's right-click is refused with the same note. + await page.getByTestId("nav-more").click(); + await expect(page.getByTestId("nav-customize")).toHaveAttribute("aria-disabled", "true"); + await expect(page.getByTestId("nav-not-loaded")).toHaveText("Your saved layout hasn't loaded yet. Trying again…"); + await page.keyboard.press("Escape"); + await sidebar(page).getByTestId("nav-review").click({ button: "right" }); + await page.getByRole("menuitem", { name: "Hide" }).click(); + await expect(page.getByText("Your saved layout hasn't loaded yet. Trying again…").last()).toBeVisible(); + await expect(sidebar(page).getByTestId("nav-review")).toBeVisible(); + // Nothing was written over the stored layout. + expect(await stored(page)).toEqual(saved); + + // The agent answers again: the stored layout replaces the default. + await recover(page); + await expect(sidebar(page).getByTestId("nav-kits")).toBeVisible({ timeout: 20_000 }); + await page.getByTestId("nav-more").click(); + await expect(page.getByTestId("nav-not-loaded")).toHaveCount(0); + await page.getByTestId("nav-customize").click(); + await expect(page.getByRole("dialog", { name: "Customize sidebar" })).toBeVisible(); + expect(await stored(page)).toEqual(saved); +}); diff --git a/app/src/components/sidebar/nav.tsx b/app/src/components/sidebar/nav.tsx index 1732bcea0..24467389d 100644 --- a/app/src/components/sidebar/nav.tsx +++ b/app/src/components/sidebar/nav.tsx @@ -30,7 +30,7 @@ import { Menu, MenuItem, MenuPopup, MenuSeparator, MenuTrigger } from "@/compone import { Sheet, SheetDescription, SheetFooter, SheetHeader, SheetPanel, SheetPopup, SheetTitle } from "@/components/ui/sheet"; import { SidebarMenu, SidebarMenuBadge, SidebarMenuButton, SidebarMenuItem } from "@/components/ui/sidebar"; import { useAgentCounts } from "@/hooks/use-agent-counts"; -import { arrange, type NavList, navActions, useNav } from "@/lib/nav"; +import { arrange, NAV_NOT_LOADED, type NavList, navActions, navReady, useNav } from "@/lib/nav"; import { type NavBadge, readNavBadge } from "@/lib/nav-badge"; import { platformKeys } from "@/lib/platform"; import { usePrefs } from "@/lib/prefs"; @@ -293,6 +293,7 @@ function NavRow({ item, list, index, ids }: { item: NavItem; list: NavList; inde // MoreItems is More's places as menu items, then a way to rearrange them. // The sidebar, the rail and zen's switcher share it. export function MoreItems({ more }: { more: NavItem[] }) { + const loaded = useNav((s) => s.loaded); return ( <> {more.map((n) => ( @@ -303,10 +304,15 @@ export function MoreItems({ more }: { more: NavItem[] }) { ))} {more.length > 0 && } - openCustomize()}> + openCustomize()}> Customize sidebar… + {!loaded && ( +

+ {NAV_NOT_LOADED} +

+ )} ); } @@ -382,7 +388,7 @@ export function Nav({ lead, extra }: { lead?: ReactNode; extra?: ReactNode } = { // The Customize sheet: every place, in its three lists, with drag handles // and buttons, and a way back to the defaults. const useCustomize = create<{ open: boolean }>()(() => ({ open: false })); -export const openCustomize = () => useCustomize.setState({ open: true }); +export const openCustomize = () => navReady() && useCustomize.setState({ open: true }); export function CustomizeSidebarSheet() { const open = useCustomize((s) => s.open); diff --git a/app/src/hooks/use-berth-connection.ts b/app/src/hooks/use-berth-connection.ts index 5d608085c..782c30353 100644 --- a/app/src/hooks/use-berth-connection.ts +++ b/app/src/hooks/use-berth-connection.ts @@ -5,7 +5,7 @@ import { endpoint, httpClient, type Client } from "@/lib/api"; import { handleEvent } from "@/lib/events"; import { errorMessage } from "@/lib/format"; import { useStore } from "@/lib/store"; -import { loadNav } from "@/lib/nav"; +import { loadNav, retryNav } from "@/lib/nav"; import { loadNotifications } from "@/lib/notifications"; import { loadProjects } from "@/lib/project-groups"; import { reloadKits } from "@/views/kits/kits-store"; @@ -59,7 +59,16 @@ export function useBerthConnection() { void loadNotifications(); // Project menus show each project's kit status. void reloadKits(); - client.events(handleEvent, () => void refreshAll(), abort.signal); + // A reconnect also tries the sidebar's layout again, if it didn't load + // (the agent restarting as the app starts). + client.events( + handleEvent, + () => { + void refreshAll(); + void retryNav(); + }, + abort.signal, + ); // The backstop: events say what changed, so this only catches what // one missed. Not while the window is hidden; coming back refreshes. poll = window.setInterval(() => !document.hidden && void refreshAll(), BACKSTOP); diff --git a/app/src/lib/mock.ts b/app/src/lib/mock.ts index 729b3412a..1921187e0 100644 --- a/app/src/lib/mock.ts +++ b/app/src/lib/mock.ts @@ -323,6 +323,19 @@ function saveHooks(machine: string, hooks: Hook[]): Promise { // /v1/app documents, as the laptop agent keeps them. Projects start with a // Work section so sections show. const appDocs: Record = fresh ? {} : { projects: { projects: [{ id: "acme/shop", section: "Work", default_box: "devl" }, { id: "me/notes", section: "Work" }], sections: ["Work", "Personal"] } }; +// The sidebar's layout outlives a reload, as the agent's file does: kept in +// this tab's session storage. ?failapp=sidebar fails reads and writes of +// that document, as an agent that is restarting does, until +// window.__mockApp.recover("sidebar"); __mockApp.doc("sidebar") is what is +// stored. +const KEPT_SIDEBAR = "berth.mock.app.sidebar"; +const keptSidebar = sessionStorage.getItem(KEPT_SIDEBAR); +if (keptSidebar) appDocs.sidebar = JSON.parse(keptSidebar); +const failingDocs = new Set(new URLSearchParams(location.search).getAll("failapp")); +(window as unknown as { __mockApp: unknown }).__mockApp = { + recover: (key: string) => failingDocs.delete(key), + doc: (key: string) => appDocs[key] ?? null, +}; const listeners = new Set<(e: BerthEvent) => void>(); const emit = (e: Omit) => listeners.forEach((l) => l({ ...e, time: new Date().toISOString() })); @@ -1438,9 +1451,11 @@ function mockAgent(): Client { if (method === "PUT" && path === "/v1/hooks") return saveHooks("laptop", (body as { hooks: Hook[] }).hooks) as Promise; // The app's own documents (/v1/app/), such as project groups. const app = /^\/v1\/app\/([a-z0-9-]+)$/.exec(path); + if (app && failingDocs.has(app[1])) return Promise.reject(new ApiError("the Shipyard agent is restarting (mock)", 503)); if (app && method === "GET") return delay(appDocs[app[1]] ?? null) as Promise; if (app && method === "PUT") { appDocs[app[1]] = structuredClone(body); + if (app[1] === "sidebar") sessionStorage.setItem(KEPT_SIDEBAR, JSON.stringify(body)); return delay(body) as Promise; } // A file on "this Mac" goes up as an attachment, as the agent does. diff --git a/app/src/lib/nav-core.test.ts b/app/src/lib/nav-core.test.ts new file mode 100644 index 000000000..27bbd226f --- /dev/null +++ b/app/src/lib/nav-core.test.ts @@ -0,0 +1,167 @@ +// node --experimental-strip-types --test src/lib/nav-core.test.ts (pnpm test) +import assert from "node:assert/strict"; +import { test } from "node:test"; + +import { arrange, DEFAULT, migrate, move, type NavLayout, type NavState, navSync, place } from "./nav-core.ts"; + +const layout = (pinned: string[], more: string[] = [], hidden: string[] = []): NavLayout => ({ version: 2, pinned, more, hidden }); + +test("migrate: nothing stored is the default", () => { + assert.deepEqual(migrate(null), DEFAULT); + assert.deepEqual(migrate(undefined), DEFAULT); + assert.deepEqual(migrate(""), DEFAULT); +}); + +test("migrate: version 1's dashboard becomes home, and the rest stays", () => { + const m = migrate({ pinned: ["dashboard", "review"], more: ["kits"], hidden: ["dashboard"], moreOpen: true }); + assert.deepEqual(m, { version: 2, pinned: ["home", "review"], more: ["kits"], hidden: ["home"], moreOpen: true }); +}); + +test("migrate: version 2 is kept as stored, and lists that aren't lists of names are read safely", () => { + assert.deepEqual(migrate(layout(["dashboard"], ["home"])), layout(["dashboard"], ["home"])); + assert.deepEqual(migrate({ version: 2, pinned: ["home", 3], more: "kits" }), layout(["home"])); +}); + +test("arrange: available ids in their stored lists and order, unknown ones in More, unavailable ones left out", () => { + const l = layout(["review", "plugin:x:a", "home"], ["kits"], ["worktrees"]); + assert.deepEqual(arrange(l, ["home", "review", "worktrees", "kits", "automations"]), { pinned: ["review", "home"], more: ["kits", "automations"], hidden: ["worktrees"] }); +}); + +test("arrange: a place listed twice takes its first list (pinned, then hidden, then more)", () => { + assert.deepEqual(arrange(layout(["home"], ["home", "kits"], ["kits"]), ["home", "kits"]), { pinned: ["home"], more: [], hidden: ["kits"] }); +}); + +test("place: pins an item at the end, or at an index", () => { + const ids = ["home", "review", "kits"]; + assert.deepEqual(place(layout(["home", "review"]), "kits", "pinned", undefined, ids).pinned, ["home", "review", "kits"]); + assert.deepEqual(place(layout(["home", "review"]), "kits", "pinned", 0, ids).pinned, ["kits", "home", "review"]); +}); + +test("place: a pinned plugin that isn't available keeps its place", () => { + // The plugin is off (or still loading) while the person pins Kits. + const l = layout(["home", "plugin:usage:main", "review"], ["plugin:intake:main"], ["plugin:old:x"]); + const next = place(l, "kits", "pinned", undefined, ["home", "review", "kits"]); + assert.deepEqual(next.pinned, ["home", "plugin:usage:main", "review", "kits"]); + assert.deepEqual(next.more, ["plugin:intake:main"]); + assert.deepEqual(next.hidden, ["plugin:old:x"]); + // Back on, it shows where it was pinned. + assert.deepEqual(arrange(next, ["home", "review", "kits", "plugin:usage:main", "plugin:intake:main"]).pinned, ["home", "plugin:usage:main", "review", "kits"]); +}); + +test("move: swaps with the neighbour and keeps unavailable ids in place", () => { + const l = layout(["home", "plugin:usage:main", "review", "worktrees"]); + const ids = ["home", "review", "worktrees"]; + assert.deepEqual(move(l, "worktrees", -1, ids).pinned, ["home", "plugin:usage:main", "worktrees", "review"]); + assert.deepEqual(move(l, "home", -1, ids).pinned, ["home", "plugin:usage:main", "review", "worktrees"]); + assert.equal(move(l, "nowhere", 1, ids), l); +}); + +// A store and an agent to sync with: get answers from gets in turn (an +// Error is thrown), put records what was written. +function harness(gets: unknown[]) { + let state: NavState = { layout: DEFAULT, loaded: false }; + const puts: NavLayout[] = []; + let reads = 0; + const sync = navSync( + { + get: async () => { + const g = gets[Math.min(reads++, gets.length - 1)]; + if (g instanceof Error) throw g; + return g; + }, + put: async (l) => void puts.push(l), + describe: (err) => String((err as Error).message), + }, + { get: () => state, set: (p) => (state = { ...state, ...p }) }, + ); + return { sync, puts, state: () => state, reads: () => reads }; +} + +const saved = layout(["home", "kits"], ["review"]); +const down = new Error("connection refused"); + +test("a missing file is a successful load: the default, and saves are allowed", async () => { + const h = harness([null]); + assert.equal(await h.sync.load(), true); + assert.deepEqual(h.state(), { layout: DEFAULT, loaded: true, error: undefined }); + assert.equal(await h.sync.save((l) => place(l, "kits", "pinned", undefined, ["home", "kits"])), true); + assert.equal(h.puts.length, 1); +}); + +test("a save before a successful load is refused, and writes nothing", async () => { + const h = harness([down]); + assert.equal(await h.sync.load(), false); + assert.equal(h.state().loaded, false); + assert.equal(h.state().error, "connection refused"); + assert.deepEqual(h.state().layout, DEFAULT); + assert.equal(await h.sync.save((l) => place(l, "kits", "pinned", undefined, ["home", "kits"])), false); + assert.equal(await h.sync.save(() => DEFAULT), false); + assert.equal(h.puts.length, 0); + assert.deepEqual(h.state().layout, DEFAULT); +}); + +test("a load that failed, tried again, swaps the default for the stored layout", async () => { + const h = harness([down, down, saved]); + assert.equal(await h.sync.load(), false); + assert.equal(await h.sync.load(), false); + assert.deepEqual(h.state().layout, DEFAULT); + assert.equal(await h.sync.load(), true); + assert.deepEqual(h.state().layout, saved); + assert.equal(h.state().error, undefined); +}); + +test("once loaded, a further load doesn't read again", async () => { + const h = harness([saved]); + await h.sync.load(); + assert.equal(await h.sync.load(), true); + assert.equal(h.reads(), 1); +}); + +test("a failed write puts the old layout back", async () => { + let state: NavState = { layout: saved, loaded: true }; + const failing = navSync( + { get: async () => saved, put: async () => Promise.reject(down), describe: (e) => (e as Error).message }, + { get: () => state, set: (p) => (state = { ...state, ...p }) }, + ); + assert.equal(await failing.save(() => DEFAULT), false); + assert.deepEqual(state.layout, saved); + assert.equal(state.error, "connection refused"); +}); + +test("saves run in turn: an earlier write that fails can't undo a later one", async () => { + // A's write hangs, then fails (the agent restarting); B is made meanwhile. + let state: NavState = { layout: saved, loaded: true }; + const writes: { layout: NavLayout; settle(ok: boolean): void }[] = []; + const sync = navSync( + { + get: async () => saved, + put: (layout) => new Promise((resolve, reject) => writes.push({ layout, settle: (ok) => (ok ? resolve(layout) : reject(down)) })), + describe: (e) => (e as Error).message, + }, + { get: () => state, set: (p) => (state = { ...state, ...p }) }, + ); + const ids = ["home", "kits", "review", "worktrees"]; + const a = sync.save((l) => place(l, "worktrees", "pinned", undefined, ids)); + const b = sync.save((l) => place(l, "review", "hidden", undefined, ids)); + await new Promise((r) => setImmediate(r)); + assert.equal(writes.length, 1, "B waits for A's write"); + writes[0].settle(false); + assert.equal(await a, false); + assert.deepEqual(state.layout, saved); + await new Promise((r) => setImmediate(r)); + assert.equal(writes.length, 2); + // B is made on the layout A's failure left, so it carries nothing of A's. + assert.deepEqual(writes[1].layout.pinned, ["home", "kits"]); + assert.deepEqual(writes[1].layout.hidden, ["review"]); + writes[1].settle(true); + assert.equal(await b, true); + assert.deepEqual(state.layout, writes[1].layout); +}); + +test("reset to default saves the default once loaded", async () => { + const h = harness([saved]); + await h.sync.load(); + assert.equal(await h.sync.save(() => DEFAULT), true); + assert.deepEqual(h.puts, [DEFAULT]); + assert.deepEqual(h.state().layout, DEFAULT); +}); diff --git a/app/src/lib/nav-core.ts b/app/src/lib/nav-core.ts new file mode 100644 index 000000000..33ee5563a --- /dev/null +++ b/app/src/lib/nav-core.ts @@ -0,0 +1,152 @@ +// The sidebar's arrangement as data, and how it is read and saved: no app +// imports, so node's test runner loads it (nav-core.test.ts). lib/nav.ts +// wires it to the laptop agent and the app's store. + +import { oneAtATime } from "./one-at-a-time.ts"; + +export type NavList = "pinned" | "more" | "hidden"; + +export interface NavLayout { + // 2 since Home took the Agent Dashboard's place (see migrate). + version?: number; + pinned: string[]; + more: string[]; + hidden: string[]; + // Version 1 opened More in place in the sidebar. More is a menu now; the + // field is kept as it was for older apps and otherwise ignored. + moreOpen?: boolean; +} + +export const DEFAULT_PINNED = ["home", "review", "worktrees", "automations"]; +export const DEFAULT: NavLayout = { version: 2, pinned: DEFAULT_PINNED, more: [], hidden: [] }; + +// What a list reads as when the stored one isn't a list of names. +const names = (v: unknown, fallback: string[]) => (Array.isArray(v) ? v.filter((x): x is string => typeof x === "string") : fallback); + +// migrate brings a stored arrangement up to date. Version 1 had no Home: its +// first place was the Agent Dashboard, which Home now stands for (and opens, +// without Labs), so "dashboard" becomes "home" wherever the person put it. +// With Labs the dashboard is a place of its own again and, new to the +// arrangement, lands in More. Everything else the person arranged stays. +// Nothing stored (no file, or an empty one) is the default. +export function migrate(doc: unknown): NavLayout { + if (!doc || typeof doc !== "object" || Array.isArray(doc)) return DEFAULT; + const d = doc as Partial; + let lists = { pinned: names(d.pinned, DEFAULT.pinned), more: names(d.more, []), hidden: names(d.hidden, []) }; + if ((d.version ?? 1) < 2) { + const rename = (l: string[]) => l.map((id) => (id === "dashboard" ? "home" : id)); + lists = { pinned: rename(lists.pinned), more: rename(lists.more), hidden: rename(lists.hidden) }; + } + return { ...d, ...lists, version: 2 }; +} + +// The order a place listed twice is taken in: its first list wins. +const ORDER: NavList[] = ["pinned", "hidden", "more"]; + +// arrange places the available ids into the three lists, in the stored +// order; ids the layout does not know yet go to More. +export function arrange(layout: NavLayout, ids: string[]): Record { + const have = new Set(ids); + const seen = new Set(); + const take = (list: string[]) => + list.filter((id) => { + if (!have.has(id) || seen.has(id)) return false; + seen.add(id); + return true; + }); + const pinned = take(layout.pinned); + const hidden = take(layout.hidden); + const more = take(layout.more); + for (const id of ids) if (!seen.has(id)) more.push(id); + return { pinned, more, hidden }; +} + +// keepAway is the arranged lists a change made, with the places that aren't +// available now (a plugin turned off, or not loaded yet) put back where they +// were stored, so a pinned plugin's place is still there when it returns. +function keepAway(l: NavLayout, a: Record, ids: string[]): NavLayout { + const have = new Set(ids); + const seen = new Set(); + const out = { pinned: [...a.pinned], more: [...a.more], hidden: [...a.hidden] }; + for (const k of ORDER) { + l[k].forEach((id, i) => { + if (have.has(id) || seen.has(id)) return; + seen.add(id); + out[k].splice(Math.min(i, out[k].length), 0, id); + }); + } + return { ...l, ...out }; +} + +// place puts id into list at index (the end when omitted). ids is every +// item available now, so items still unplaced keep their place in More. +export function place(l: NavLayout, id: string, list: NavList, index: number | undefined, ids: string[]): NavLayout { + const a = arrange(l, ids); + for (const k of Object.keys(a) as NavList[]) a[k] = a[k].filter((x) => x !== id); + const i = index === undefined ? a[list].length : Math.max(0, Math.min(index, a[list].length)); + a[list].splice(i, 0, id); + return keepAway(l, a, ids); +} + +// move swaps id with its neighbour in its list. +export function move(l: NavLayout, id: string, dir: -1 | 1, ids: string[]): NavLayout { + const a = arrange(l, ids); + const k = ORDER.find((k) => a[k].includes(id)); + if (!k) return l; + const i = a[k].indexOf(id); + const j = i + dir; + if (j >= 0 && j < a[k].length) [a[k][i], a[k][j]] = [a[k][j], a[k][i]]; + return keepAway(l, a, ids); +} + +export interface NavState { + layout: NavLayout; + // The stored arrangement has been read. Until then the default shows and + // nothing is saved: a save would write the default over it. + loaded: boolean; + error?: string; +} + +export interface NavIO { + get(): Promise; + put(layout: NavLayout): Promise; + describe(err: unknown): string; +} + +// navSync reads the stored arrangement, and saves changes only once it has +// it. lib/nav.ts tries load again (lib/poll) until it returns true. +export function navSync(io: NavIO, st: { get(): NavState; set(p: Partial): void }) { + // Saves run in turn, each on the layout the last one left: a failed + // write puts back only what it changed, never over a later one's. + const inTurn = oneAtATime(); + return { + async load(): Promise { + if (st.get().loaded) return true; + try { + st.set({ layout: migrate(await io.get()), loaded: true, error: undefined }); + return true; + } catch (err) { + st.set({ error: io.describe(err) }); + return false; + } + }, + // save applies fn and stores the result, after any save still under + // way. Refused (false) before the stored arrangement is read; a failed + // write puts the old one back. + save: (fn: (l: NavLayout) => NavLayout): Promise => + inTurn(async () => { + const s = st.get(); + if (!s.loaded) return false; + const before = s.layout; + const next = fn(structuredClone(before)); + st.set({ layout: next }); + try { + await io.put(next); + return true; + } catch (err) { + st.set({ layout: before, error: io.describe(err) }); + return false; + } + }), + }; +} diff --git a/app/src/lib/nav.ts b/app/src/lib/nav.ts index 0e97f00e4..fcc7d9c5b 100644 --- a/app/src/lib/nav.ts +++ b/app/src/lib/nav.ts @@ -1,114 +1,73 @@ import { create } from "zustand"; +import { toastManager } from "@/components/ui/toast"; import { plainError } from "@/lib/errors"; +import { DEFAULT, move, type NavLayout, type NavList, type NavState, navSync, place } from "@/lib/nav-core"; +import { type Poller, poll } from "@/lib/poll"; import { useStore } from "@/lib/store"; // The app's places, arranged by the person: a few pinned at the top, the // rest under More, and some hidden (still in ⌘K). The sidebar, its folded // rail and zen's switcher all show this one arrangement. Stored on the // laptop agent (/v1/app/sidebar). Anything new, such as a plugin's screen, -// lands in More, never in Pinned. +// lands in More, never in Pinned. The arranging itself is in nav-core. -export type NavList = "pinned" | "more" | "hidden"; +export { arrange, type NavLayout, type NavList } from "@/lib/nav-core"; -export interface NavLayout { - // 2 since Home took the Agent Dashboard's place (see migrate). - version?: number; - pinned: string[]; - more: string[]; - hidden: string[]; - // Version 1 opened More in place in the sidebar. More is a menu now; the - // field is kept as it was for older apps and otherwise ignored. - moreOpen?: boolean; -} - -export const DEFAULT_PINNED = ["home", "review", "worktrees", "automations"]; -const DEFAULT: NavLayout = { version: 2, pinned: DEFAULT_PINNED, more: [], hidden: [] }; - -// migrate brings a stored arrangement up to date. Version 1 had no Home: its -// first place was the Agent Dashboard, which Home now stands for (and opens, -// without Labs), so "dashboard" becomes "home" wherever the person put it. -// With Labs the dashboard is a place of its own again and, new to the -// arrangement, lands in More. Everything else the person arranged stays. -export function migrate(doc: Partial | null | undefined): NavLayout { - if (!doc) return DEFAULT; - let lists = { pinned: doc.pinned ?? DEFAULT.pinned, more: doc.more ?? [], hidden: doc.hidden ?? [] }; - if ((doc.version ?? 1) < 2) { - const rename = (l: string[]) => l.map((id) => (id === "dashboard" ? "home" : id)); - lists = { pinned: rename(lists.pinned), more: rename(lists.more), hidden: rename(lists.hidden) }; - } - return { ...doc, ...lists, version: 2 }; -} +// What Customize says while the stored arrangement hasn't been read. +export const NAV_NOT_LOADED = "Your saved layout hasn't loaded yet. Trying again…"; -export const useNav = create<{ layout: NavLayout; loaded: boolean; error?: string }>()(() => ({ layout: DEFAULT, loaded: false })); +export const useNav = create()(() => ({ layout: DEFAULT, loaded: false })); -export async function loadNav() { +const sidebarDoc = (method: "GET" | "PUT", body?: unknown) => { const client = useStore.getState().client; - if (!client) return; - try { - const doc = await client.laptop | null>("GET", "/v1/app/sidebar"); - useNav.setState({ layout: migrate(doc), loaded: true }); - } catch (err) { - useNav.setState({ loaded: true, error: plainError(err) }); - } -} + return client ? client.laptop(method, "/v1/app/sidebar", body) : Promise.reject(new Error("not connected to the Shipyard agent")); +}; -async function save(fn: (l: NavLayout) => NavLayout) { - const before = useNav.getState().layout; - const next = fn(structuredClone(before)); - useNav.setState({ layout: next }); - const client = useStore.getState().client; - if (!client) return; - try { - await client.laptop("PUT", "/v1/app/sidebar", next); - } catch (err) { - useNav.setState({ layout: before, error: plainError(err) }); - } -} +const sync = navSync( + { + get: () => sidebarDoc("GET"), + put: (layout) => sidebarDoc("PUT", layout), + describe: (err) => { + const msg = plainError(err); + if (msg !== useNav.getState().error) console.warn(`sidebar layout: ${msg}`); + return msg; + }, + }, + { get: () => useNav.getState(), set: (p) => useNav.setState(p) }, +); -// arrange places the available ids into the three lists, in the stored -// order; ids the layout does not know yet go to More. -export function arrange(layout: NavLayout, ids: string[]): Record { - const have = new Set(ids); - const seen = new Set(); - const take = (list: string[]) => - list.filter((id) => { - if (!have.has(id) || seen.has(id)) return false; - seen.add(id); - return true; - }); - const pinned = take(layout.pinned); - const hidden = take(layout.hidden); - const more = take(layout.more); - for (const id of ids) if (!seen.has(id)) more.push(id); - return { pinned, more, hidden }; +// loadNav reads the stored arrangement, trying again (after 1 s, doubling +// to 10 s, not while the window is hidden) until it has it: the agent may be +// restarting as the app starts, as after an update. +let loading: Poller | undefined; +export function loadNav() { + if (loading || useNav.getState().loaded) return; + const p = poll( + async () => { + if (!(await sync.load())) return false; + p.stop(); + loading = undefined; + }, + { every: 1000, max: 10_000 }, + ); + loading = p; } +// retryNav tries now, as the agent is back (the event stream reconnected). +export const retryNav = () => loading?.kick(); -function where(l: NavLayout, id: string): NavList | undefined { - return (["pinned", "more", "hidden"] as NavList[]).find((k) => l[k].includes(id)); -} +// Until the stored arrangement is read nothing changes it: a change would +// save the default over it. Every way to change it comes through here. +export const navReady = () => { + if (useNav.getState().loaded) return true; + toastManager.add({ type: "info", title: NAV_NOT_LOADED }); + return false; +}; + +const save = (fn: (l: NavLayout) => NavLayout) => (navReady() ? sync.save(fn) : Promise.resolve(false)); export const navActions = { - // place puts id into list at index (the end when omitted). ids is every - // item available now, so items still unplaced keep their place in More. - place: (id: string, list: NavList, index: number | undefined, ids: string[]) => - save((l) => { - const a = arrange(l, ids); - for (const k of Object.keys(a) as NavList[]) a[k] = a[k].filter((x) => x !== id); - const i = index === undefined ? a[list].length : Math.max(0, Math.min(index, a[list].length)); - a[list].splice(i, 0, id); - return { ...l, ...a }; - }), - move: (id: string, dir: -1 | 1, ids: string[]) => - save((l) => { - const a = arrange(l, ids); - const k = where({ ...l, ...a }, id); - if (!k) return l; - const i = a[k].indexOf(id); - const j = i + dir; - if (j < 0 || j >= a[k].length) return { ...l, ...a }; - [a[k][i], a[k][j]] = [a[k][j], a[k][i]]; - return { ...l, ...a }; - }), + place: (id: string, list: NavList, index: number | undefined, ids: string[]) => save((l) => place(l, id, list, index, ids)), + move: (id: string, dir: -1 | 1, ids: string[]) => save((l) => move(l, id, dir, ids)), reset: () => save(() => DEFAULT), }; diff --git a/docs/changelog.mdx b/docs/changelog.mdx index cc4ef4e5a..c09f3c5cd 100644 --- a/docs/changelog.mdx +++ b/docs/changelog.mdx @@ -10,6 +10,14 @@ upgrades with `berth upgrade BOX`, or by running the again. ## Unreleased +- **Your sidebar layout survives a restart.** When the app started while + its agent on this computer was restarting (as after an update), the + sidebar fell back to the default layout for that launch, and any change + then saved the default over yours. The app now keeps asking until your + layout arrives, and **Customize sidebar…** waits until it has, saying so. + A pinned plugin that is off or still loading also keeps its place when + you change something else, and a change that fails to save no longer + undoes one you made after it. - **Quiet projects are grayed out.** On the rail and in the two-column sidebar, a project with no agent or shell open in it is grayed out, so the ones you're using stand out. Switching to one of those opens New