diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index b8149eb..b7f7081 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -23,11 +23,11 @@ jobs: cache: pnpm - uses: astral-sh/setup-uv@d0cc045d04ccac9d8b7881df0226f9e82c39688e # v6 # la-arch-check (run by the architecture tests) and its TypeScript twin, which is not on npm. - - run: uv tool install --no-build living-architecture==0.2.3 + - run: uv tool install --no-build living-architecture==0.3.1 - run: | twin="$RUNNER_TEMP/living-architecture" git init -q "$twin" - git -C "$twin" fetch -q --depth 1 https://github.com/MotleyAI/living-architecture.git 8b5cdc1e31d113435183a6459a252a843f5d7487 # v0.2.3 + git -C "$twin" fetch -q --depth 1 https://github.com/MotleyAI/living-architecture.git 15ed4263ccce08513535aae077c8e7ba7363571d # v0.3.1 git -C "$twin" checkout -q FETCH_HEAD cd "$twin/node" npm ci --ignore-scripts diff --git a/README.md b/README.md index 96f881f..55d6445 100644 --- a/README.md +++ b/README.md @@ -75,7 +75,7 @@ Panes, terminal tabs, worktree rows and repo tabs show the most urgent mark of t ## Development -Requires Node 22 (`.nvmrc`), pnpm, and the `la-*` architecture tools on `PATH` (`uv tool install living-architecture==0.2.3`), which `pnpm test` runs. +Requires Node 22 (`.nvmrc`), pnpm, and the `la-*` architecture tools on `PATH` (`uv tool install living-architecture==0.3.1`), which `pnpm test` runs. ```sh pnpm install diff --git a/architecture/model/typescript.c4 b/architecture/model.c4 similarity index 100% rename from architecture/model/typescript.c4 rename to architecture/model.c4 index d61c61b..cb14da0 100644 --- a/architecture/model/typescript.c4 +++ b/architecture/model.c4 @@ -1,10 +1,10 @@ + specification { element root element node element component tag legacy } - model { typescript = root 'worktree-term' { diff --git a/openspec/changes/worktree-name-search/proposal.md b/openspec/changes/archive/2026-10-12-worktree-name-search/proposal.md similarity index 100% rename from openspec/changes/worktree-name-search/proposal.md rename to openspec/changes/archive/2026-10-12-worktree-name-search/proposal.md diff --git a/openspec/changes/worktree-name-search/specs/web/spec.md b/openspec/changes/archive/2026-10-12-worktree-name-search/specs/web/spec.md similarity index 100% rename from openspec/changes/worktree-name-search/specs/web/spec.md rename to openspec/changes/archive/2026-10-12-worktree-name-search/specs/web/spec.md diff --git a/openspec/changes/worktree-name-search/tasks.md b/openspec/changes/archive/2026-10-12-worktree-name-search/tasks.md similarity index 100% rename from openspec/changes/worktree-name-search/tasks.md rename to openspec/changes/archive/2026-10-12-worktree-name-search/tasks.md diff --git a/openspec/changes/archive/2026-10-15-dev-2080-worktree-term-clicking-anywhere-on-a-sidebar-worktree-row/.openspec.yaml b/openspec/changes/archive/2026-10-15-dev-2080-worktree-term-clicking-anywhere-on-a-sidebar-worktree-row/.openspec.yaml new file mode 100644 index 0000000..b9ebfc7 --- /dev/null +++ b/openspec/changes/archive/2026-10-15-dev-2080-worktree-term-clicking-anywhere-on-a-sidebar-worktree-row/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-10-12 diff --git a/openspec/changes/archive/2026-10-15-dev-2080-worktree-term-clicking-anywhere-on-a-sidebar-worktree-row/design.md b/openspec/changes/archive/2026-10-15-dev-2080-worktree-term-clicking-anywhere-on-a-sidebar-worktree-row/design.md new file mode 100644 index 0000000..0bc2d6f --- /dev/null +++ b/openspec/changes/archive/2026-10-15-dev-2080-worktree-term-clicking-anywhere-on-a-sidebar-worktree-row/design.md @@ -0,0 +1,20 @@ +## Context + +`WorktreeEntry` (`src/web/ui/app.tsx`) puts the select handler on the nested `.label` button, so the hit target is whatever box that child occupies; the `.worktree` row around it (padding, flex gaps, the strips above and below the label and the star, the mark) does nothing on click. Repo tabs already handle the click on the whole tab, with inner controls stopping propagation. + +## Goals / Non-Goals + +**Goals:** the row is the click target, so no future child layout can shrink it again. + +**Non-Goals:** the 5px list padding outside the rows; the context menu, prunable and gone behaviour. + +## Decisions + +- **The row owns the click.** `.worktree` gets `onClick` → `view.selectWorktree(path)`; the label button loses its own `onClick`. Alternative rejected: stretching the label (CSS or a `::after` overlay) — still leaves the star column and gaps as special cases. +- **The label stays the keyboard control.** Enter/Space on the button fire a click that bubbles to the row; the row gets no role or `tabIndex`, avoiding a second tab stop. The row's handler carries `// NOSONAR(S6848) — a pointer shortcut; the label button is the keyboard control`, as at the pane container. +- **The star stops propagation.** Its `onClick` calls `event.stopPropagation()`, so toggling never selects. Clicks above or below the 18px star (29px row) select — a stray select is harmless, a stray star toggle is persisted and shared. +- **Cursor.** `cursor: pointer` moves from `.worktree .label` to `.worktree`. + +## Risks / Trade-offs + +- [A future in-row control forgets to stop propagation and also selects] → the checkbox e2e scenario shows the pattern; the repo-tab close button does the same. diff --git a/openspec/changes/archive/2026-10-15-dev-2080-worktree-term-clicking-anywhere-on-a-sidebar-worktree-row/proposal.md b/openspec/changes/archive/2026-10-15-dev-2080-worktree-term-clicking-anywhere-on-a-sidebar-worktree-row/proposal.md new file mode 100644 index 0000000..2f9138a --- /dev/null +++ b/openspec/changes/archive/2026-10-15-dev-2080-worktree-term-clicking-anywhere-on-a-sidebar-worktree-row/proposal.md @@ -0,0 +1,21 @@ +## Why + +A sidebar worktree switches only when the click lands on its branch-name text: the row is at least 29px tall, but only the text-tall label button inside it handles the click, so clicks above or below the text — between two names — do nothing. + +## What Changes + +- Web: a primary click anywhere in a worktree's row selects it; its checkbox still only toggles the star; a right-click still opens the worktree menu without selecting. The label stays the row's keyboard control (Enter or Space selects). +- The whole row shows the pointer cursor. + +## Capabilities + +### New Capabilities + +### Modified Capabilities +- `web`: the worktree sidebar's click target is the whole row. + +## Impact + +- Code: `src/web/ui/app.tsx` (`WorktreeEntry`), `src/web/main/styles.css`. +- Architecture, wire, upgrade: no change. +- Tests: `test/e2e/sidebar.spec.ts`. diff --git a/openspec/changes/archive/2026-10-15-dev-2080-worktree-term-clicking-anywhere-on-a-sidebar-worktree-row/specs/web/spec.md b/openspec/changes/archive/2026-10-15-dev-2080-worktree-term-clicking-anywhere-on-a-sidebar-worktree-row/specs/web/spec.md new file mode 100644 index 0000000..4c97dc2 --- /dev/null +++ b/openspec/changes/archive/2026-10-15-dev-2080-worktree-term-clicking-anywhere-on-a-sidebar-worktree-row/specs/web/spec.md @@ -0,0 +1,37 @@ +## MODIFIED Requirements + +### Requirement: Worktree sidebar +The sidebar SHALL list the selected repo's worktrees in the daemon's order without bare entries, each labelled with its branch, or ` @ ` when detached, or the directory name when it has no head. On hover a worktree SHALL show its path followed by one line ` : ` per terminal, or "no terminals", where the status is `waiting for input` (`state` `input`), `running` (`state` `working`), `done` (`state` `idle` and `unseen`), `idle` (`state` `idle`), `exited`, `exited with code ` or `killed by `, the exit taking precedence. A prunable worktree SHALL be shown greyed and SHALL offer no new terminal. A worktree no longer listed that still has live terminals SHALL be listed after the others, marked as gone, without a checkbox, until its last terminal closes. Each listed worktree SHALL have a checkbox reflecting the daemon's checked state, and toggling it SHALL send `setChecked`. A primary click anywhere in a worktree's row SHALL select it, except on its checkbox, which only toggles it; a right-click SHALL open the worktree menu without selecting. Enter or Space on a worktree's label SHALL select it. Worktree list changes SHALL show without user action. + +#### Scenario: Labels +- **WHEN** worktrees are on branch `feature/x`, detached at `0123456789…` in directory `wt2`, and bare +- **THEN** the sidebar shows `feature/x` and `wt2 @ 0123456`, and no bare entry + +#### Scenario: Status on hover +- **WHEN** a worktree has a terminal `probe` 3 printing continuously and a terminal `probe` 4 that exited with code 1 +- **THEN** hovering it shows its path, `probe 3: running` and `probe 4: exited with code 1` + +#### Scenario: Vanished worktree keeps its terminals +- **WHEN** a worktree with a running terminal is removed with `git worktree remove --force` +- **THEN** it stays listed as gone, its terminal remains usable, and it disappears after the terminal is closed + +#### Scenario: Checkbox shared +- **WHEN** a worktree is checked in one page +- **THEN** it shows as checked in another page connected to the same hub + +#### Scenario: The whole row selects +- **WHEN** the user clicks an unselected worktree's row at its top edge, at its bottom edge, in the space right of its label, or just above its checkbox +- **THEN** that worktree is selected + +#### Scenario: The checkbox only toggles +- **WHEN** the user clicks an unselected worktree's checkbox +- **THEN** the checkbox toggles and the selected worktree does not change + +#### Scenario: Right-click does not select +- **WHEN** the user right-clicks an unselected worktree's row +- **THEN** the worktree menu opens and the selected worktree does not change + +#### Scenario: Keyboard selects +- **WHEN** the user focuses an unselected worktree's label and presses Enter, or another's and presses Space +- **THEN** that worktree is selected + diff --git a/openspec/changes/archive/2026-10-15-dev-2080-worktree-term-clicking-anywhere-on-a-sidebar-worktree-row/tasks.md b/openspec/changes/archive/2026-10-15-dev-2080-worktree-term-clicking-anywhere-on-a-sidebar-worktree-row/tasks.md new file mode 100644 index 0000000..7ac97da --- /dev/null +++ b/openspec/changes/archive/2026-10-15-dev-2080-worktree-term-clicking-anywhere-on-a-sidebar-worktree-row/tasks.md @@ -0,0 +1,19 @@ +## 1. Corpus + +- [x] 1.1 Archive the merged `worktree-name-search` change in its own commit; `openspec validate --specs --strict` passes + +## 2. Tests (e2e, `test/e2e/sidebar.spec.ts`) + +- [x] 2.1 The whole row selects: `click({ position })` on an unselected worktree's row at y=1, y=height−2, right of the label, and above the star each select it (assert `aria-selected`); fails before the fix +- [x] 2.2 The checkbox only toggles: clicking an unselected worktree's checkbox checks it and leaves the selection unchanged +- [x] 2.3 Right-click does not select: right-clicking an unselected row opens the worktree menu and leaves the selection unchanged +- [x] 2.4 Keyboard selects: extend `a worktree is selected from the keyboard` to press Enter on one label and Space on another's + +## 3. Implementation + +- [x] 3.1 `WorktreeEntry` in `src/web/ui/app.tsx`: row `onClick` selects (with the `NOSONAR(S6848)` comment from design.md), the label button has no `onClick`, the star's `onClick` stops propagation; tests 2.1–2.4 pass +- [x] 3.2 `src/web/main/styles.css`: `cursor: pointer` on `.worktree`, removed from `.worktree .label` + +## 4. Final gates + +- [x] 4.1 `pnpm test` green (incl. Playwright); `pnpm lint` exits 0; `tsc -b` clean; `openspec validate dev-2080-worktree-term-clicking-anywhere-on-a-sidebar-worktree-row --strict` passes diff --git a/openspec/specs/web/spec.md b/openspec/specs/web/spec.md index 77609b5..04fa59e 100644 --- a/openspec/specs/web/spec.md +++ b/openspec/specs/web/spec.md @@ -58,7 +58,7 @@ The page SHALL show one tab per repo of each host, in the order the hub lists th - **THEN** the tab labelled `box:app` shows the status `reconnecting` ### Requirement: Worktree sidebar -The sidebar SHALL list the selected repo's worktrees in the daemon's order without bare entries, each labelled with its branch, or ` @ ` when detached, or the directory name when it has no head. On hover a worktree SHALL show its path followed by one line ` : ` per terminal, or "no terminals", where the status is `waiting for input` (`state` `input`), `running` (`state` `working`), `done` (`state` `idle` and `unseen`), `idle` (`state` `idle`), `exited`, `exited with code ` or `killed by `, the exit taking precedence. A prunable worktree SHALL be shown greyed and SHALL offer no new terminal. A worktree no longer listed that still has live terminals SHALL be listed after the others, marked as gone, without a checkbox, until its last terminal closes. Each listed worktree SHALL have a checkbox reflecting the daemon's checked state, and toggling it SHALL send `setChecked`. Worktree list changes SHALL show without user action. +The sidebar SHALL list the selected repo's worktrees in the daemon's order without bare entries, each labelled with its branch, or ` @ ` when detached, or the directory name when it has no head. On hover a worktree SHALL show its path followed by one line ` : ` per terminal, or "no terminals", where the status is `waiting for input` (`state` `input`), `running` (`state` `working`), `done` (`state` `idle` and `unseen`), `idle` (`state` `idle`), `exited`, `exited with code ` or `killed by `, the exit taking precedence. A prunable worktree SHALL be shown greyed and SHALL offer no new terminal. A worktree no longer listed that still has live terminals SHALL be listed after the others, marked as gone, without a checkbox, until its last terminal closes. Each listed worktree SHALL have a checkbox reflecting the daemon's checked state, and toggling it SHALL send `setChecked`. A primary click anywhere in a worktree's row SHALL select it, except on its checkbox, which only toggles it; a right-click SHALL open the worktree menu without selecting. Enter or Space on a worktree's label SHALL select it. Worktree list changes SHALL show without user action. #### Scenario: Labels - **WHEN** worktrees are on branch `feature/x`, detached at `0123456789…` in directory `wt2`, and bare @@ -76,6 +76,22 @@ The sidebar SHALL list the selected repo's worktrees in the daemon's order witho - **WHEN** a worktree is checked in one page - **THEN** it shows as checked in another page connected to the same hub +#### Scenario: The whole row selects +- **WHEN** the user clicks an unselected worktree's row at its top edge, at its bottom edge, in the space right of its label, or just above its checkbox +- **THEN** that worktree is selected + +#### Scenario: The checkbox only toggles +- **WHEN** the user clicks an unselected worktree's checkbox +- **THEN** the checkbox toggles and the selected worktree does not change + +#### Scenario: Right-click does not select +- **WHEN** the user right-clicks an unselected worktree's row +- **THEN** the worktree menu opens and the selected worktree does not change + +#### Scenario: Keyboard selects +- **WHEN** the user focuses an unselected worktree's label and presses Enter, or another's and presses Space +- **THEN** that worktree is selected + ### Requirement: Checked filter The page SHALL offer, for the selected repo, a toggle between showing all worktrees and only checked ones; each repo SHALL keep its own setting, stored per browser across reloads, and a repo without one SHALL show all. With "checked only", the sidebar SHALL list the checked worktrees and the selected worktree. Selecting another repo SHALL show that repo's setting without animating the toggle; only the user's toggling SHALL animate it. @@ -486,3 +502,30 @@ Only a page that has the focus SHALL send `resize`. A page that gains the focus #### Scenario: Refocus takes it back - **WHEN** the first page gains the focus again - **THEN** the PTY takes the first page's current size + +### Requirement: Worktree name search +The sidebar header SHALL hold, between its title and the checked filter's toggle, a text box for the selected repo: while it holds text, the sidebar SHALL list only the worktrees whose label contains that text, trimmed and ignoring case, that the checked filter also lists, and the selected worktree. While it holds text, a control beside it SHALL empty it. Each repo SHALL keep its own text, stored per browser across reloads; a repo without one SHALL list every worktree the checked filter lists. The text SHALL play no part in which worktree is selected without a stored selection. When the browser refuses to store a setting of the page, the change SHALL still apply and the page SHALL show that saving failed. + +#### Scenario: Search narrows the list +- **WHEN** a repo has worktrees on `feature/login`, `fix/Logout` and `docs`, `feature/login` is selected and the user types `LOG` +- **THEN** the sidebar lists `feature/login` and `fix/Logout` only + +#### Scenario: Selected worktree stays listed +- **WHEN** the selected worktree's label does not contain the typed text +- **THEN** it stays listed and selected + +#### Scenario: Search belongs to its repo and persists +- **WHEN** text is typed in one repo, another repo is selected and the page is reloaded +- **THEN** the other repo shows an empty box and every worktree its checked filter lists, and the first repo shows its text and lists only the matching worktrees and the selected one + +#### Scenario: Clear control empties the search +- **WHEN** the user activates the clear control +- **THEN** the box is empty and focused, the clear control is gone, and every worktree the checked filter lists is listed, with the selected one, also after a reload + +#### Scenario: Search with "checked only" +- **WHEN** the search is `feat` and the checked filter is "checked only" +- **THEN** only checked worktrees whose label contains `feat`, and the selected worktree, are listed + +#### Scenario: Refused storage write +- **WHEN** the browser refuses to store the search and the user types text +- **THEN** the list is narrowed by the text and the page shows that saving its settings failed diff --git a/src/daemon/server/bind.test.ts b/src/daemon/server/bind.test.ts index 711ab36..41972c3 100644 --- a/src/daemon/server/bind.test.ts +++ b/src/daemon/server/bind.test.ts @@ -1,11 +1,22 @@ -import { mkdtempSync, readdirSync, rmSync } from 'node:fs'; +import { existsSync, mkdtempSync, readdirSync, readFileSync, renameSync, rmSync, writeFileSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { afterEach, beforeEach, expect, it, vi } from 'vitest'; import * as files from '../../platform/files/index.js'; import { bindSocket } from './bind.js'; -const hooks = vi.hoisted(() => ({ failRename: false })); +interface Hooks { + failRename: boolean; + replaceAfterRename: boolean; + lockHeld: boolean; + lockPath: string; + /** Each removeFile call, and whether the start lock file existed at that moment. */ + removals: { path: string; locked: boolean }[]; +} + +const hooks = vi.hoisted( + (): Hooks => ({ failRename: false, replaceAfterRename: false, lockHeld: false, lockPath: '', removals: [] }), +); vi.mock('../../platform/files/index.js', async (importOriginal) => { const real = await importOriginal(); @@ -14,6 +25,18 @@ vi.mock('../../platform/files/index.js', async (importOriginal) => { renameFile: async (from: string, to: string): Promise => { if (hooks.failRename) throw Object.assign(new Error('EXDEV: rename failed'), { code: 'EXDEV' }); await real.renameFile(from, to); + if (hooks.replaceAfterRename) { + renameSync(to, `${to}.moved`); + writeFileSync(to, 'someone else'); + } + }, + acquireStartLock: async (path: string): Promise => { + if (hooks.lockHeld) throw new real.StartLockHeldError(`start lock ${path} is held`); + return real.acquireStartLock(path); + }, + removeFile: async (path: string): Promise => { + hooks.removals.push({ path, locked: existsSync(hooks.lockPath) }); + await real.removeFile(path); }, }; }); @@ -23,6 +46,10 @@ let dir: string; beforeEach(() => { dir = mkdtempSync(join(tmpdir(), 'wtd-bind-')); hooks.failRename = false; + hooks.replaceAfterRename = false; + hooks.lockHeld = false; + hooks.lockPath = ''; + hooks.removals = []; }); afterEach(() => { @@ -46,3 +73,29 @@ it('binds the socket when setup succeeds', async () => { expect(readdirSync(paths.runDir)).toEqual(['box.sock']); await listener.close(); }); + +it('keeps a socket replaced the moment it was renamed into place', async () => { + const paths = files.hostPaths({ env: { XDG_STATE_HOME: dir }, home: dir, host: 'box' }); + hooks.replaceAfterRename = true; + const listener = await bindSocket(paths); + await listener.close(); + expect(readFileSync(paths.socket, 'utf8')).toBe('someone else'); +}); + +it('removes its socket on close only while holding the start lock, then releases the lock', async () => { + const paths = files.hostPaths({ env: { XDG_STATE_HOME: dir }, home: dir, host: 'box' }); + hooks.lockPath = paths.lock; + const listener = await bindSocket(paths); + await listener.close(); + expect(hooks.removals.filter((r) => r.path === paths.socket)).toEqual([{ path: paths.socket, locked: true }]); + expect(existsSync(paths.socket)).toBe(false); + expect(existsSync(paths.lock)).toBe(false); +}); + +it('leaves its socket on close while another process holds the start lock', async () => { + const paths = files.hostPaths({ env: { XDG_STATE_HOME: dir }, home: dir, host: 'box' }); + const listener = await bindSocket(paths); + hooks.lockHeld = true; + await listener.close(); + expect(existsSync(paths.socket)).toBe(true); +}); diff --git a/src/daemon/server/bind.ts b/src/daemon/server/bind.ts index 46cac3d..e32f8c2 100644 --- a/src/daemon/server/bind.ts +++ b/src/daemon/server/bind.ts @@ -8,7 +8,9 @@ import { preparePrivateDirs, removeFile, renameFile, + StartLockHeldError, type HostPaths, + type StartLock, } from '../../platform/files/index.js'; /** A daemon already serves this host's socket. */ @@ -20,7 +22,7 @@ export class AlreadyRunningError extends Error { export interface Listener { /** Hands connections, including any that arrived before, to `accept`. */ serve: (accept: (socket: Socket) => void) => void; - /** Stops listening and removes the socket file if it is still the one bound. */ + /** Stops listening and removes the socket file if it is still the one bound and the start lock is free. */ close: () => Promise; } @@ -73,8 +75,9 @@ export const bindSocket = async (paths: HostPaths): Promise => { await listen(server, bound); try { await makeOwnerOnly(bound); + // Taken before the rename, which keeps it, as the socket can be replaced once it is in place. + identity = await fileIdentity(bound); await renameFile(bound, paths.socket); - identity = await fileIdentity(paths.socket); } catch (error) { server.close(); for (const socket of early.splice(0)) socket.destroy(); @@ -91,7 +94,20 @@ export const bindSocket = async (paths: HostPaths): Promise => { }, close: async () => { server.close(); - if (identity !== null && (await fileIdentity(paths.socket)) === identity) await removeFile(paths.socket); + if (identity === null) return; + // Under the start lock, so a starting daemon cannot put its socket in place between check and removal. + let lock: StartLock; + try { + lock = await acquireStartLock(paths.lock); + } catch (error) { + if (error instanceof StartLockHeldError) return; // the next start replaces the socket left behind + throw error; + } + try { + if ((await fileIdentity(paths.socket)) === identity) await removeFile(paths.socket); + } finally { + await lock.release(); + } }, }; }; diff --git a/src/platform/files/files.test.ts b/src/platform/files/files.test.ts index f2f797f..f808586 100644 --- a/src/platform/files/files.test.ts +++ b/src/platform/files/files.test.ts @@ -36,6 +36,7 @@ import { removeFile, renameFile, sshControlPath, + StartLockHeldError, writeFileAtomic, } from './index.js'; @@ -574,7 +575,7 @@ describe('start lock', () => { const path = lockFile(); const started = Date.now(); const acquiring = acquireStartLock(path); - await expect(acquiring).rejects.toThrow(); + await expect(acquiring).rejects.toThrow(StartLockHeldError); const waited = Date.now() - started; expect(waited).toBeGreaterThanOrEqual(4900); expect(waited).toBeLessThan(7000); diff --git a/src/platform/files/index.ts b/src/platform/files/index.ts index 4defb9f..326835a 100644 --- a/src/platform/files/index.ts +++ b/src/platform/files/index.ts @@ -285,6 +285,11 @@ export interface StartLock { release: () => Promise; } +/** A live process held the start lock for the whole wait. */ +export class StartLockHeldError extends Error { + override readonly name = 'StartLockHeldError'; +} + interface Holder { pid: number; nonce: string; @@ -376,7 +381,7 @@ export const acquireStartLock = async (path: string): Promise => { } if (Date.now() >= deadline) { const by = current === null ? '' : ` by process ${String(current.pid)}`; - throw new Error(`start lock ${path} is held${by}`); + throw new StartLockHeldError(`start lock ${path} is held${by}`); } await sleep(LOCK_RETRY_MS); // NOSONAR(S9382) — retry loop } diff --git a/src/web/main/styles.css b/src/web/main/styles.css index 399cfe2..39883e3 100644 --- a/src/web/main/styles.css +++ b/src/web/main/styles.css @@ -277,6 +277,7 @@ input:focus-visible, padding: 1px 7px; border-radius: 5px; white-space: nowrap; + cursor: pointer; } .worktree:hover { @@ -305,7 +306,6 @@ input:focus-visible, text-align: left; overflow: hidden; text-overflow: ellipsis; - cursor: pointer; } .worktree-star { diff --git a/src/web/ui/app.tsx b/src/web/ui/app.tsx index d7d0c4e..d201494 100644 --- a/src/web/ui/app.tsx +++ b/src/web/ui/app.tsx @@ -313,7 +313,7 @@ const WorktreeEntry = ({ terminals: Terminal[]; view: View; }) => ( -
{ + view.selectWorktree(entry.path); + }} onContextMenu={(event) => { event.preventDefault(); view.openWorktreeMenu(entry.path, event.clientX, event.clientY); @@ -333,6 +336,9 @@ const WorktreeEntry = ({ aria-label={`${entry.checked ? 'Unstar' : 'Star'} ${entry.label}`} title={entry.checked ? 'Remove from starred' : 'Add to starred'} checked={entry.checked} + onClick={(event) => { + event.stopPropagation(); + }} onChange={(event) => { view.setChecked(entry.path, event.currentTarget.checked).catch(report('checking a worktree')); }} @@ -342,9 +348,6 @@ const WorktreeEntry = ({ type="button" class="label" data-testid="worktree-label" - onClick={() => { - view.selectWorktree(entry.path); - }} > {entry.label} {entry.gone ? ' (gone)' : ''} diff --git a/test/e2e/contract.ts b/test/e2e/contract.ts index b43b9f9..3e24510 100644 --- a/test/e2e/contract.ts +++ b/test/e2e/contract.ts @@ -56,9 +56,9 @@ export const TID = { addRepoPath: 'add-repo-path', /** In the add-repo dialog after a failed `addRepo`; text holds the error code. */ addRepoError: 'add-repo-error', - /** Sidebar entry; `data-path` = path, `title` = path then ` : ` per terminal (or "no terminals"), `aria-selected`, `data-prunable="true"`, `data-gone="true"`; holds a checkbox input unless gone; right-click opens the worktree menu. */ + /** Sidebar entry; `data-path` = path, `title` = path then ` : ` per terminal (or "no terminals"), `aria-selected`, `data-prunable="true"`, `data-gone="true"`; holds a checkbox input unless gone; a primary click anywhere in it but on the checkbox selects it; right-click opens the worktree menu without selecting. */ worktree: 'worktree', - /** The label inside a sidebar entry; clicking it selects the worktree. */ + /** The label inside a sidebar entry; Enter or Space on it selects the worktree. */ worktreeLabel: 'worktree-label', /** Checkbox input: checked = show checked worktrees only, in the selected repo; each repo keeps its own, across reloads. */ filterChecked: 'filter-checked', diff --git a/test/e2e/sidebar.spec.ts b/test/e2e/sidebar.spec.ts index 723ef1c..708da60 100644 --- a/test/e2e/sidebar.spec.ts +++ b/test/e2e/sidebar.spec.ts @@ -1,5 +1,5 @@ import { existsSync, writeFileSync } from 'node:fs'; -import type { Page } from '@playwright/test'; +import type { Locator, Page } from '@playwright/test'; import { join } from 'node:path'; import type { HubHost } from '../support/hub-host.js'; import { fakeWorktrees, git } from '../support/daemon-host.js'; @@ -26,6 +26,11 @@ import { const label = (path: string): string => `${worktreeEntry(path)} ${byTestId(TID.worktreeLabel)}`; const checkbox = (path: string): string => `${worktreeEntry(path)} input[type="checkbox"]`; +const boxOf = async (locator: Locator): Promise<{ x: number; y: number; width: number; height: number }> => { + const box = await locator.boundingBox(); + if (box === null) throw new Error('the element is not shown'); + return box; +}; test.describe('worktree sidebar', () => { test('labels show the branch, or the directory and short head when detached, with the path on hover', async ({ hub, page }) => { @@ -89,13 +94,74 @@ test.describe('worktree sidebar', () => { }); test('a worktree is selected from the keyboard', async ({ hub, page }) => { - const { repo, worktrees } = makeRepoWith(hub, 'app', ['feat']); - const feat = worktrees[0] ?? ''; + const { repo, worktrees } = makeRepoWith(hub, 'app', ['feat', 'other']); + const [feat = '', other = ''] = worktrees; hub.writeRepos([repo]); await openUi(hub, page); + await selectWorktree(page, repo); await page.locator(label(feat)).focus(); await page.keyboard.press('Enter'); await expect(page.locator(worktreeEntry(feat))).toHaveAttribute('aria-selected', 'true'); + await page.locator(label(other)).focus(); + await page.keyboard.press('Space'); + await expect(page.locator(worktreeEntry(other))).toHaveAttribute('aria-selected', 'true'); + await expect(page.locator(worktreeEntry(feat))).toHaveAttribute('aria-selected', 'false'); + }); + + test('a click anywhere in a worktree row selects it: top and bottom edges, right of the label, above the checkbox', async ({ + hub, + page, + }) => { + const { repo, worktrees } = makeRepoWith(hub, 'app', ['feat']); + const feat = worktrees[0] ?? ''; + hub.writeRepos([repo]); + await openUi(hub, page); + const row = page.locator(worktreeEntry(feat)); + const rowBox = await boxOf(row); + const starBox = await boxOf(page.locator(checkbox(feat))); + const points = [ + { x: rowBox.width / 2, y: 1 }, + { x: rowBox.width / 2, y: rowBox.height - 2 }, + { x: rowBox.width - 3, y: rowBox.height / 2 }, + { x: starBox.x - rowBox.x + starBox.width / 2, y: (starBox.y - rowBox.y) / 2 }, + ]; + for (const position of points) { + await selectWorktree(page, repo); + // The point must miss the label and the checkbox, or the test proves nothing. + expect( + await row.evaluate( + (el, p) => document.elementFromPoint(el.getBoundingClientRect().x + p.x, el.getBoundingClientRect().y + p.y) === el, + position, + ), + ).toBe(true); + await row.click({ position }); + await expect(row, `click at ${JSON.stringify(position)}`).toHaveAttribute('aria-selected', 'true'); + } + await expect(page.locator(checkbox(feat))).not.toBeChecked(); + }); + + test("clicking a worktree's checkbox toggles it without selecting the worktree", async ({ hub, page }) => { + const { repo, worktrees } = makeRepoWith(hub, 'app', ['feat']); + const feat = worktrees[0] ?? ''; + hub.writeRepos([repo]); + await openUi(hub, page); + await selectWorktree(page, repo); + await page.locator(checkbox(feat)).click(); + await expect(page.locator(checkbox(feat))).toBeChecked(); + await expect(page.locator(worktreeEntry(feat))).toHaveAttribute('aria-selected', 'false'); + await expect(page.locator(worktreeEntry(repo))).toHaveAttribute('aria-selected', 'true'); + }); + + test('right-clicking a worktree row opens its menu without selecting it', async ({ hub, page }) => { + const { repo, worktrees } = makeRepoWith(hub, 'app', ['feat']); + const feat = worktrees[0] ?? ''; + hub.writeRepos([repo]); + await openUi(hub, page); + await selectWorktree(page, repo); + await page.locator(worktreeEntry(feat)).click({ button: 'right' }); + await expect(page.locator(byTestId(TID.worktreeMenu))).toBeVisible(); + await expect(page.locator(worktreeEntry(feat))).toHaveAttribute('aria-selected', 'false'); + await expect(page.locator(worktreeEntry(repo))).toHaveAttribute('aria-selected', 'true'); }); test('worktree list changes show without user action', async ({ hub, page }) => {