From 3520af0dd08cdc27e78dcb2dfd8626b43544ac81 Mon Sep 17 00:00:00 2001 From: Egor Kraev Date: Wed, 7 Oct 2026 21:06:08 +0200 Subject: [PATCH 1/6] Restyle the UI, add a resizable sidebar and per-repo starred filter, run the page under Vite - Darker palette, star toggles for worktrees and a "Starred" switch in the sidebar header - Draggable (and arrow-key) sidebar width, kept in localStorage - The starred filter belongs to each repo; switching repos no longer animates the switch - `pnpm dev` serves the page through Vite, proxying /api and /ws to the running hub; /login signs in - e2e: re-adding a listed repo selects its tab --- package.json | 1 + src/web/main/styles.css | 213 +++++++++++++++++++++++++++++++++---- src/web/ui/app.tsx | 97 +++++++++++++---- src/web/ui/sidebar.test.ts | 58 +++++++++- src/web/ui/sidebar.ts | 22 ++++ src/web/ui/view.ts | 49 +++++++-- test/e2e/add-repo.spec.ts | 16 +++ test/e2e/contract.ts | 4 +- test/e2e/sidebar.spec.ts | 88 ++++++++++++++- vite.config.ts | 73 +++++++++++-- 10 files changed, 554 insertions(+), 67 deletions(-) diff --git a/package.json b/package.json index fe7c4e1..38e46b5 100644 --- a/package.json +++ b/package.json @@ -13,6 +13,7 @@ "wtd": "dist/wtd.mjs" }, "scripts": { + "dev": "vite", "build": "vite build && esbuild src/cli/index.ts --bundle --platform=node --target=node20 --format=esm --outfile=dist/wtd.mjs \"--banner:js=#!/usr/bin/env node\nimport { createRequire } from 'node:module'; const require = createRequire(import.meta.url);\" --external:@homebridge/node-pty-prebuilt-multiarch", "typecheck": "tsc -b", "lint": "tsc -b && eslint . --max-warnings 0", diff --git a/src/web/main/styles.css b/src/web/main/styles.css index f03bd35..08d56b6 100644 --- a/src/web/main/styles.css +++ b/src/web/main/styles.css @@ -1,12 +1,23 @@ :root { color-scheme: dark; - --bg: #1e1e1e; - --panel: #252526; - --border: #3c3c3c; - --text: #d4d4d4; - --muted: #858585; - --accent: #0e639c; - font-family: system-ui, sans-serif; + --bg: #101215; + --panel: #171a1f; + --panel-raised: #1d2128; + --hover: #242932; + --border: #2b3039; + --text: #e7e9ed; + --muted: #9299a6; + --accent: #4c8dff; + --accent-soft: #233b61; + --star: #f5c451; + font-family: + Inter, + ui-sans-serif, + system-ui, + -apple-system, + BlinkMacSystemFont, + 'Segoe UI', + sans-serif; font-size: 13px; } @@ -35,11 +46,11 @@ body { .term-tabs { display: flex; align-items: center; - gap: 2px; + gap: 4px; background: var(--panel); border-bottom: 1px solid var(--border); - min-height: 30px; - padding: 0 4px; + min-height: 38px; + padding: 0 8px; } .repo-tabs .spacer, @@ -52,14 +63,30 @@ button { color: inherit; background: transparent; border: 1px solid transparent; - padding: 3px 8px; + padding: 5px 9px; cursor: pointer; + border-radius: 5px; + transition: + background-color 100ms ease, + border-color 100ms ease, + color 100ms ease; +} + +button:hover:not(:disabled) { + background: var(--hover); +} + +button:focus-visible, +input:focus-visible, +[role='tab']:focus-visible { + outline: 2px solid var(--accent); + outline-offset: -2px; } .repo-tab.active, .term-tab.active { - background: var(--bg); - border-color: var(--border); + background: var(--panel-raised); + border-color: #39414d; } .reconnecting { @@ -85,22 +112,127 @@ button { } .sidebar { - width: 240px; - overflow: auto; + flex: none; + box-sizing: border-box; + display: flex; + flex-direction: column; + overflow: hidden; background: var(--panel); border-right: 1px solid var(--border); } +/* Over the sidebar's right border, so the terminal layer never covers it. */ +.sidebar-resizer { + flex: none; + position: relative; + z-index: 3; + width: 5px; + margin-left: -5px; + cursor: col-resize; + touch-action: none; + transition: background-color 100ms ease; +} + +.sidebar-resizer:hover, +.sidebar-resizer:focus-visible, +.sidebar-resizer:active { + background: var(--accent); + outline: none; +} + +.sidebar-header { + display: flex; + align-items: center; + justify-content: space-between; + min-height: 38px; + padding: 0 10px 0 13px; + border-bottom: 1px solid var(--border); +} + +.sidebar-title { + color: var(--muted); + font-size: 11px; + font-weight: 700; + letter-spacing: 0.08em; + text-transform: uppercase; +} + +.worktree-list { + padding: 5px; + overflow: auto; +} + +.star-filter { + display: flex; + align-items: center; + gap: 7px; + color: var(--muted); + font-size: 11px; + cursor: pointer; + user-select: none; +} + +.star-filter:has(input:checked) { + color: var(--text); +} + +.star-filter input { + appearance: none; + position: relative; + width: 27px; + height: 15px; + margin: 0; + border: 1px solid #454c58; + border-radius: 999px; + background: #252a32; + cursor: pointer; + transition: + background-color 120ms ease, + border-color 120ms ease; +} + +.star-filter input::after { + content: ''; + position: absolute; + top: 2px; + left: 2px; + width: 9px; + height: 9px; + border-radius: 50%; + background: #9da4b0; + transition: + transform 120ms ease, + background-color 120ms ease; +} + +.star-filter input:checked { + border-color: #a87c1f; + background: #6b501b; +} + +.star-filter input:checked::after { + background: var(--star); + transform: translateX(12px); +} + .worktree { display: flex; align-items: center; - gap: 4px; - padding: 3px 6px; + gap: 7px; + min-height: 29px; + padding: 1px 7px; + border-radius: 5px; white-space: nowrap; } +.worktree:hover { + background: var(--hover); +} + .worktree[aria-selected='true'] { - background: var(--accent); + background: var(--accent-soft); + color: #f4f7ff; + box-shadow: inset 2px 0 var(--accent); } .worktree.prunable, @@ -122,6 +254,38 @@ button { cursor: pointer; } +.worktree-star { + appearance: none; + display: grid; + place-items: center; + flex: none; + width: 18px; + height: 18px; + margin: 0; + border: 0; + background: transparent; + color: #626a76; + cursor: pointer; +} + +.worktree-star::before { + content: '☆'; + font-size: 17px; + line-height: 1; +} + +.worktree-star:hover { + color: var(--star); +} + +.worktree-star:checked { + color: var(--star); +} + +.worktree-star:checked::before { + content: '★'; +} + .terminal-pane { flex: 1; display: flex; @@ -138,7 +302,8 @@ button { display: flex; align-items: center; gap: 4px; - padding: 3px 4px 3px 8px; + padding: 4px 4px 4px 9px; + border-radius: 5px; border: 1px solid transparent; cursor: pointer; } @@ -226,10 +391,11 @@ button { .confirm-dialog, .add-repo-dialog { background: var(--panel); - border: 1px solid var(--border); - padding: 8px; + border: 1px solid #3a414c; + border-radius: 8px; + padding: 10px; min-width: 260px; - box-shadow: 0 4px 16px rgb(0 0 0 / 50%); + box-shadow: 0 12px 36px rgb(0 0 0 / 55%); } /* Undo the user agent's centring and colours of an open dialog. */ @@ -342,7 +508,8 @@ dialog.add-repo-dialog { display: flex; align-items: center; gap: 4px; - padding: 3px 8px; + padding: 4px 8px; + border-radius: 5px; border: 1px solid transparent; cursor: pointer; } diff --git a/src/web/ui/app.tsx b/src/web/ui/app.tsx index 86f8035..d2ea077 100644 --- a/src/web/ui/app.tsx +++ b/src/web/ui/app.tsx @@ -7,7 +7,7 @@ import { followArea, type TerminalManager } from '../terminals/index.js'; import { hasBanner, hostAction } from './hosts.js'; import { aggregate, markTitle, type Aggregate } from './marks.js'; import { pickerKey } from './picker.js'; -import type { SidebarEntry } from './sidebar.js'; +import { SIDEBAR_WIDTH, type SidebarEntry } from './sidebar.js'; import type { RepoTab } from './repo-tabs.js'; import { PANE_HEADER, type AddRepoDialog, type CloseRequest, type Confirmation, type ShownPane, type ShownTab, type View } from './view.js'; @@ -293,6 +293,9 @@ const WorktreeEntry = ({ {!entry.gone && ( { view.setChecked(entry.path, event.currentTarget.checked).catch(report('checking a worktree')); @@ -650,6 +653,46 @@ const TerminalArea = ({ client, manager, view }: AppProps) => { ); }; +/** Sidebar width change per arrow key, in pixels. */ +const SIDEBAR_KEY_STEPS: Readonly> = { ArrowLeft: -16, ArrowRight: 16 }; + +/** The draggable edge between the sidebar and the terminals; arrow keys move it too. */ +const SidebarResizer = ({ view }: { view: View }) => { + const width = view.sidebarWidth.value; + return ( + ); }; @@ -642,7 +909,13 @@ const TerminalArea = ({ client, manager, view }: AppProps) => { return (
{view.panes.value.map((pane) => ( - + ))} {view.dividers.value.map((divider) => ( @@ -749,6 +1022,7 @@ const Workspace = ({ client, manager, view }: AppProps) => { {closing !== null && } + {view.worktreeMenu.value !== null && }
); }; diff --git a/src/web/ui/marks.test.ts b/src/web/ui/marks.test.ts index 59285e6..33bfb74 100644 --- a/src/web/ui/marks.test.ts +++ b/src/web/ui/marks.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it } from 'vitest'; import type { Terminal } from '../../protocol/index.js'; -import { aggregate, mark, MARK_ORDER, markTitle, type Mark } from './marks.js'; +import { aggregate, mark, MARK_ORDER, markTitle, terminalStatus, worktreeTitle, type Mark } from './marks.js'; let nextId = 1; @@ -88,3 +88,30 @@ describe('markTitle', () => { expect(markTitle({})).toBe(''); }); }); + +describe('terminalStatus', () => { + it.each([ + ['waiting for input', { state: 'input' }], + ['running', { state: 'working' }], + ['running', { state: 'working', unseen: true }], + ['done', { state: 'idle', unseen: true }], + ['idle', { state: 'idle', unseen: false }], + ['exited', { exit: clean }], + ['exited with code 1', { exit: failedCode }], + ['killed by SIGKILL', { exit: failedSignal }], + ] as const)('says %s', (status, fields) => { + expect(terminalStatus(terminal(fields))).toBe(status); + }); +}); + +describe('worktreeTitle', () => { + it('lists each terminal with its preset, id and status under the path', () => { + const agent = terminal({ termId: 3, preset: 'claude', state: 'input' }); + const shell = terminal({ termId: 4, preset: 'shell', state: 'working' }); + expect(worktreeTitle('/r/wt', [agent, shell])).toBe('/r/wt\nclaude 3: waiting for input\nshell 4: running'); + }); + + it('says so when the worktree has no terminals', () => { + expect(worktreeTitle('/r/wt', [])).toBe('/r/wt\nno terminals'); + }); +}); diff --git a/src/web/ui/marks.ts b/src/web/ui/marks.ts index 9748ad7..72730e0 100644 --- a/src/web/ui/marks.ts +++ b/src/web/ui/marks.ts @@ -35,3 +35,21 @@ export const aggregate = (terminals: readonly Terminal[], level: MarkLevel): Agg /** One line `: ` per mark present, in rank order. */ export const markTitle = (counts: Aggregate['counts']): string => MARK_ORDER.flatMap((m) => (counts[m] === undefined ? [] : [`${m}: ${String(counts[m])}`])).join('\n'); + +/** What a terminal is doing, in words: waiting for input, running, done, idle, or how it exited. */ +export const terminalStatus = (t: Terminal): string => { + if (t.exit !== null) { + if (t.exit.signal !== null) return `killed by ${t.exit.signal}`; + return t.exit.code === 0 ? 'exited' : `exited with code ${String(t.exit.code)}`; + } + if (t.state === 'input') return 'waiting for input'; + if (t.state === 'working') return 'running'; + return t.unseen ? 'done' : 'idle'; +}; + +/** A worktree's hover text: its path, then one line ` : ` per terminal. */ +export const worktreeTitle = (path: string, terminals: readonly Terminal[]): string => + [ + path, + ...(terminals.length === 0 ? ['no terminals'] : terminals.map((t) => `${t.preset} ${String(t.termId)}: ${terminalStatus(t)}`)), + ].join('\n'); diff --git a/src/web/ui/picker.test.ts b/src/web/ui/picker.test.ts index 3bf5310..58f68f9 100644 --- a/src/web/ui/picker.test.ts +++ b/src/web/ui/picker.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it } from 'vitest'; import type { Layout } from '../../protocol/index.js'; -import { pickerKey, pickerValid, type PickerContext, type PickerWorld } from './picker.js'; +import { dropPosition, pickerKey, pickerValid, presetFromForm, type PickerContext, type PickerWorld } from './picker.js'; type Pane = Layout['tabs'][number]['root']; @@ -108,3 +108,46 @@ describe('pickerKey', () => { expect(pickerKey(key, 0, 3)).toBeNull(); }); }); + +describe('presetFromForm', () => { + const presets = [ + { name: 'shell', command: null }, + { name: 'claude', command: 'claude' }, + ]; + + it('trims the name and the command', () => { + expect(presetFromForm(' htop ', ' htop -d 5 ', presets)).toEqual({ preset: { name: 'htop', command: 'htop -d 5' } }); + }); + + it('makes an empty or blank command the login shell', () => { + expect(presetFromForm('zsh', '', presets)).toEqual({ preset: { name: 'zsh', command: null } }); + expect(presetFromForm('zsh', ' ', presets)).toEqual({ preset: { name: 'zsh', command: null } }); + }); + + it.each([ + ['an empty name', '', 'x', 'Give the preset a name'], + ['a blank name', ' \t', 'x', 'Give the preset a name'], + ['a taken name', ' claude ', 'x', 'A preset named claude exists'], + ['a 65-character name', 'n'.repeat(65), 'x', 'Names are at most 64 characters'], + ['a 4097-character command', 'n', 'c'.repeat(4097), 'Commands are at most 4096 characters'], + ])('refuses %s', (_case, name, command, error) => { + expect(presetFromForm(name, command, presets)).toEqual({ error }); + }); + + it('accepts a name differing from a taken one by case', () => { + expect(presetFromForm('Claude', 'claude', presets)).toEqual({ preset: { name: 'Claude', command: 'claude' } }); + }); +}); + +describe('dropPosition', () => { + it.each([ + ['before the first preset', 2, 0, 0], + ['into its own gap above', 2, 2, 2], + ['into its own gap below', 2, 3, 2], + ['after the last of four', 0, 4, 3], + ['between two later presets', 0, 2, 1], + ['between two earlier presets', 3, 1, 1], + ])('places a preset dropped %s', (_case, from, slot, position) => { + expect(dropPosition(from, slot)).toBe(position); + }); +}); diff --git a/src/web/ui/picker.ts b/src/web/ui/picker.ts index 1be97b4..485f0b1 100644 --- a/src/web/ui/picker.ts +++ b/src/web/ui/picker.ts @@ -1,4 +1,4 @@ -import type { HostEntry, Layout } from '../../protocol/index.js'; +import type { HostEntry, Layout, Preset } from '../../protocol/index.js'; import { canAddTab, canSplit, type SplitDir } from '../layout/index.js'; export type PickerOp = { t: 'newTab' } | { t: 'split'; dir: SplitDir; target: number }; @@ -52,3 +52,20 @@ export const pickerKey = (key: string, index: number, count: number): PickerKey return null; } }; + +const MAX_PRESET_NAME = 64; +const MAX_PRESET_COMMAND = 4096; + +/** The preset the add form describes, trimmed, an empty command meaning the login shell; or why it cannot be added. */ +export const presetFromForm = (name: string, command: string, presets: readonly Preset[]): { preset: Preset } | { error: string } => { + const trimmedName = name.trim(); + const trimmedCommand = command.trim(); + if (trimmedName === '') return { error: 'Give the preset a name' }; + if (trimmedName.length > MAX_PRESET_NAME) return { error: `Names are at most ${String(MAX_PRESET_NAME)} characters` }; + if (presets.some((p) => p.name === trimmedName)) return { error: `A preset named ${trimmedName} exists` }; + if (trimmedCommand.length > MAX_PRESET_COMMAND) return { error: `Commands are at most ${String(MAX_PRESET_COMMAND)} characters` }; + return { preset: { name: trimmedName, command: trimmedCommand === '' ? null : trimmedCommand } }; +}; + +/** The position a preset dragged from `from` takes when dropped into gap `slot` (0 = before the first preset). */ +export const dropPosition = (from: number, slot: number): number => (slot > from ? slot - 1 : slot); diff --git a/src/web/ui/sidebar.test.ts b/src/web/ui/sidebar.test.ts index 4d59452..19b0660 100644 --- a/src/web/ui/sidebar.test.ts +++ b/src/web/ui/sidebar.test.ts @@ -4,7 +4,9 @@ import { clampSidebarWidth, defaultWorktree, filterOf, + keptNote, parseSidebarWidth, + removalReasons, SIDEBAR_WIDTH, sidebarEntries, visibleWorktrees, @@ -196,3 +198,37 @@ describe('repo filters', () => { expect(withFilter({ '0:/a': 'checked', '0:/b': 'checked' }, '0:/a', 'all')).toEqual({ '0:/b': 'checked' }); }); }); + +describe('removalReasons', () => { + const safe = { base: 'origin/main', ahead: 0, changes: 0, running: [] }; + + it('gives no reason for a merged, clean worktree without running terminals', () => { + expect(removalReasons(safe)).toEqual([]); + }); + + it('names every reason that applies, in order', () => { + expect(removalReasons({ base: 'origin/main', ahead: 3, changes: 2, running: ['claude 3', 'shell 4'] })).toEqual([ + 'It has 3 commits not in origin/main.', + 'It has 2 uncommitted changes, untracked files included.', + 'Running terminals will be closed: claude 3, shell 4.', + ]); + }); + + it('uses the singular for one commit or change', () => { + expect(removalReasons({ ...safe, ahead: 1, changes: 1 })).toEqual([ + 'It has 1 commit not in origin/main.', + 'It has 1 uncommitted change, untracked files included.', + ]); + }); + + it('says when there is no origin/main to compare with', () => { + expect(removalReasons({ ...safe, base: null, ahead: null })).toEqual(['There is no origin/main to check that its commits are merged.']); + }); +}); + +describe('keptNote', () => { + it('says the branch is kept, or that a detached worktree leaves its commits only in the reflog', () => { + expect(keptNote(false)).toBe('Its branch is kept.'); + expect(keptNote(true)).toContain('reachable only through the reflog'); + }); +}); diff --git a/src/web/ui/sidebar.ts b/src/web/ui/sidebar.ts index 80b7c1c..7fe4d92 100644 --- a/src/web/ui/sidebar.ts +++ b/src/web/ui/sidebar.ts @@ -65,3 +65,31 @@ export const visibleWorktrees = (entries: readonly SidebarEntry[], filter: Workt /** The worktree selected without a stored selection: the first the filter shows, else the first entry. */ export const defaultWorktree = (entries: readonly SidebarEntry[], filter: WorktreeFilter): string | null => (visibleWorktrees(entries, filter, null)[0] ?? entries[0])?.path ?? null; + +/** What the daemon reported deleting a worktree would lose. */ +export interface RemovalRisks { + base: string | null; + ahead: number | null; + changes: number; + /** Labels of the running terminals that would be closed. */ + running: readonly string[]; +} + +const plural = (n: number, one: string, many: string): string => `${String(n)} ${n === 1 ? one : many}`; + +/** Every reason to confirm deleting a worktree, one sentence each. */ +export const removalReasons = (risks: RemovalRisks): string[] => { + const reasons: string[] = []; + if (risks.ahead === null) reasons.push('There is no origin/main to check that its commits are merged.'); + else if (risks.ahead > 0) reasons.push(`It has ${plural(risks.ahead, 'commit', 'commits')} not in ${risks.base ?? 'origin/main'}.`); + if (risks.changes > 0) + reasons.push(`It has ${plural(risks.changes, 'uncommitted change', 'uncommitted changes')}, untracked files included.`); + if (risks.running.length > 0) reasons.push(`Running terminals will be closed: ${risks.running.join(', ')}.`); + return reasons; +}; + +/** What happens to a deleted worktree's commits: kept on its branch, or, detached, left only in the reflog. */ +export const keptNote = (detached: boolean): string => + detached + ? 'It has no branch: once it is deleted, its commits not in origin/main are reachable only through the reflog.' + : 'Its branch is kept.'; diff --git a/src/web/ui/view.ts b/src/web/ui/view.ts index c024655..bdc843c 100644 --- a/src/web/ui/view.ts +++ b/src/web/ui/view.ts @@ -32,7 +32,9 @@ import { clampSidebarWidth, defaultWorktree, filterOf, + keptNote, parseSidebarWidth, + removalReasons, sidebarEntries, visibleWorktrees, withFilter, @@ -87,13 +89,14 @@ const message = (error: unknown): string => (error instanceof Error ? error.mess /** An error as the add-repo dialog shows it: the code of a refused request, then its message. */ const describe = (error: unknown): string => (error instanceof RequestError ? `${error.code}: ${error.message}` : message(error)); -/** The box of a pane's terminal: the pane below its header. */ -export const terminalBoxOf = (rect: Rect): Box => ({ - left: rect.left, - top: rect.top + PANE_HEADER, - width: rect.width, - height: Math.max(0, rect.height - PANE_HEADER), -}); +/** Whether panes show headers: only in a split, where they tell the panes apart. */ +export const hasPaneHeaders = (paneCount: number): boolean => paneCount > 1; + +/** The box of a pane's terminal: the pane below its header, if it has one. */ +export const terminalBoxOf = (rect: Rect, header: boolean): Box => { + const height = header ? PANE_HEADER : 0; + return { left: rect.left, top: rect.top + height, width: rect.width, height: Math.max(0, rect.height - height) }; +}; /** * The focused pane of `tab`: the one last chosen, else the one that took its place, else the first. @@ -155,7 +158,15 @@ interface Picker { /** What the in-page confirmation asks about. */ export type Confirmation = | { t: 'host'; host: HostEntry; action: HostAction; recalled: Recalled | null } - | { t: 'removeRepo'; host: number; repo: string; label: string }; + | { t: 'removeRepo'; host: number; repo: string; label: string } + | { t: 'removeWorktree'; host: number; worktree: string; label: string; reasons: readonly string[]; note: string }; + +/** The worktree context menu, at its viewport position. */ +export interface WorktreeMenu { + worktree: string; + x: number; + y: number; +} /** The add-repo dialog. */ export interface AddRepoDialog { @@ -182,6 +193,7 @@ export class View { /** A problem of the page itself, such as a failed copy. */ readonly notice = signal(null); readonly confirmation = signal(null); + readonly worktreeMenu = signal(null); readonly addRepo = signal(null); private readonly drag = signal(null); /** The focused pane per tab key, with the tab's panes when it was last resolved; replaced when the user chooses a pane. */ @@ -316,7 +328,12 @@ export class View { }); /** Whether a picker or dialog owns the keys. */ readonly modal = computed( - () => this.picker.value !== null || this.closing.value !== null || this.confirmation.value !== null || this.addRepo.value !== null, + () => + this.picker.value !== null || + this.closing.value !== null || + this.confirmation.value !== null || + this.addRepo.value !== null || + this.worktreeMenu.value !== null, ); private shownKey = ''; @@ -451,6 +468,29 @@ export class View { this.picker.value = null; } + /** Adds a preset; rejects with the hub's refusal. */ + addPreset(preset: Preset): Promise { + return this.client.addPreset(preset); + } + + /** Moves the preset named `name` to position `to`, reporting a failure on the page; resolves with whether the hub did. */ + movePreset(name: string, to: number): Promise { + return this.client.movePreset(name, to).then( + () => true, + (error: unknown) => { + this.notice.value = `Moving preset ${name} failed: ${message(error)}`; + return false; + }, + ); + } + + /** Removes the preset named `name`, reporting a refusal on the page. */ + removePreset(name: string): void { + this.client.removePreset(name).catch((error: unknown) => { + this.notice.value = `Removing preset ${name} failed: ${message(error)}`; + }); + } + /** Runs `preset` as a new tab of the selected worktree, which has no terminals. */ choose(preset: Preset): void { const tab = this.tab.value; @@ -499,11 +539,70 @@ export class View { this.confirmation.value = null; } + openWorktreeMenu(worktree: string, x: number, y: number): void { + this.worktreeMenu.value = { worktree, x, y }; + } + + closeWorktreeMenu(): void { + this.worktreeMenu.value = null; + } + + /** Why `worktree` of the selected repo cannot be deleted now, or null when it can. */ + deleteBlocker(worktree: string): string | null { + const entry = this.entries.value.find((e) => e.path === worktree); + const listed = this.repo.value?.worktrees.find((w) => w.path === worktree); + if (entry === undefined || entry.gone || listed === undefined) return 'It is no longer a worktree'; + if (listed.main) return 'The main worktree cannot be deleted'; + if (listed.locked) return 'It is locked'; + if (this.host.value?.status !== 'connected') return 'Its host is not connected'; + return null; + } + + /** Deletes `worktree` of the selected repo; when the daemon reports what that would lose, asks to confirm first. */ + deleteWorktree(worktree: string): void { + this.worktreeMenu.value = null; + const tab = this.tab.value; + const repo = this.repo.value; + if (tab === null || repo === null || this.deleteBlocker(worktree) !== null) return; + const label = this.entries.value.find((e) => e.path === worktree)?.label ?? worktree; + const { host } = tab; + this.client + .request(host, { t: 'removeWorktree', worktree, force: false }) + .then((reply) => { + if (reply.t !== 'worktreeAtRisk') return; + const terminals = this.client.store.repo(host, tab.repo)?.terminals ?? []; + const running = reply.running.map((id) => { + const term = terminals.find((t) => t.termId === id); + return term === undefined ? `terminal ${String(id)}` : `${term.preset} ${String(id)}`; + }); + const detached = repo.worktrees.find((w) => w.path === worktree)?.detached === true; + this.confirmation.value = { + t: 'removeWorktree', + host, + worktree, + label, + reasons: removalReasons({ ...reply, running }), + note: keptNote(detached), + }; + }) + .catch((error: unknown) => { + this.notice.value = `Deleting ${label} failed: ${message(error)}`; + }); + } + /** Runs the confirmed action, showing its failure on the page. */ confirm(): void { const confirmation = this.confirmation.value; this.confirmation.value = null; if (confirmation === null) return; + if (confirmation.t === 'removeWorktree') { + this.client + .request(confirmation.host, { t: 'removeWorktree', worktree: confirmation.worktree, force: true }) + .catch((error: unknown) => { + this.notice.value = `Deleting ${confirmation.label} failed: ${message(error)}`; + }); + return; + } if (confirmation.t === 'removeRepo') { this.client.removeRepo(confirmation.host, confirmation.repo).catch((error: unknown) => { this.notice.value = @@ -765,11 +864,11 @@ export class View { /** The box the terminal `op` creates will have. */ private newPaneBox(op: PickerOp): Box { const area = this.localArea.value; - if (op.t === 'newTab') return terminalBoxOf(area); + if (op.t === 'newTab') return terminalBoxOf(area, false); const layout = split(this.layout.value, op.target, op.dir, 0); const root = layout.tabs[layout.active]?.root; const rect = root === undefined ? undefined : paneRects(root, area).find((p) => p.termId === 0)?.rect; - return terminalBoxOf(rect ?? area); + return terminalBoxOf(rect ?? area, rect !== undefined); } /** Adds the created terminal `termId` to the latest layout, as `op` asked if still possible, and focuses it. */ @@ -832,7 +931,7 @@ export class View { const focused = this.focused.value; const modal = this.modal.value; if ((status !== 'open' && status !== 'reconnecting') || tab === null || worktree === null) { - this.showOnly(null, []); + this.showOnly(null, [], false); return; } if (this.host.value?.status === 'connected' && status === 'open') { @@ -841,20 +940,20 @@ export class View { } } const shown = panes.filter((p) => this.manager.has(tab.host, p.termId)); - this.showOnly(tab.host, shown, this.drag.value !== null); + this.showOnly(tab.host, shown, hasPaneHeaders(panes.length), this.drag.value !== null); const focusKey = `${String(tab.host)}:${shown.map((p) => p.termId).join(',')}:${String(focused)}:${String(modal)}`; if (focusKey === this.focusKey) return; this.focusKey = focusKey; if (!modal && focused !== null && this.manager.has(tab.host, focused)) this.manager.focus(tab.host, focused); } - private showOnly(host: number | null, panes: readonly PaneRect[], dragging = false): void { - const key = `${String(host)}:${JSON.stringify(panes.map((p) => [p.termId, p.rect]))}`; + private showOnly(host: number | null, panes: readonly PaneRect[], headers: boolean, dragging = false): void { + const key = `${String(host)}:${String(headers)}:${JSON.stringify(panes.map((p) => [p.termId, p.rect]))}`; if (key === this.shownKey) return; this.shownKey = key; this.manager.show( host, - panes.map((p) => ({ termId: p.termId, box: terminalBoxOf(p.rect) })), + panes.map((p) => ({ termId: p.termId, box: terminalBoxOf(p.rect, headers) })), dragging, ); } diff --git a/test/e2e/attention.spec.ts b/test/e2e/attention.spec.ts index aae6c41..78ae720 100644 --- a/test/e2e/attention.spec.ts +++ b/test/e2e/attention.spec.ts @@ -86,7 +86,8 @@ test.describe('attention marks', () => { await expectMark(page, worktreeEntry(feat), null); await expectMark(page, repoTab(repo), null); await expectMark(page, termTab(failing), 'failed'); - await expectMark(page, pane(failing), 'failed'); + // A lone pane has no header: its tab carries the mark. + await expectMark(page, pane(failing), null); }); test('a hidden terminal that printed and then stayed quiet for 3 s is marked done', async ({ hub, page }) => { diff --git a/test/e2e/contract.ts b/test/e2e/contract.ts index 28361ba..3ec9644 100644 --- a/test/e2e/contract.ts +++ b/test/e2e/contract.ts @@ -56,12 +56,20 @@ 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; `title` = path, `aria-selected`, `data-prunable="true"`, `data-gone="true"`; holds a checkbox input unless gone. */ + /** 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. */ worktree: 'worktree', /** The label inside a sidebar entry; clicking 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', + /** Context menu of a worktree entry; Escape or a press outside closes it. */ + worktreeMenu: 'worktree-menu', + /** Menu item: deletes the worktree, at once when nothing would be lost, else after the confirmation; `disabled` for the main worktree, a locked or gone one, or an unconnected host. */ + deleteWorktree: 'delete-worktree', + /** One reason in the confirmation of a worktree deletion. */ + confirmReason: 'confirm-reason', + /** In the confirmation of a worktree deletion: whether its branch is kept, or that a detached worktree's commits are left only in the reflog. */ + confirmNote: 'confirm-note', /** Separator on the sidebar's right edge; dragging or ArrowLeft/ArrowRight resizes the sidebar; `aria-valuenow` = its width in px, kept across reloads. */ sidebarResizer: 'sidebar-resizer', /** Terminal tab; `data-term` = its first live terminal's id, `aria-selected`, text contains "exited" once that terminal exited. */ @@ -76,7 +84,7 @@ export const TID = { splitDown: 'split-down', /** Pane frame of the shown tab, placed over its terminal; `data-term`, `data-focused="true"` on the focused pane. */ pane: 'pane', - /** Close button inside a pane; closes its terminal, confirming first when it is running. */ + /** Close button in the header of a pane of a split (a lone pane has no header); closes its terminal, confirming first when it is running. */ paneClose: 'pane-close', /** Divider between the two sides of a split; `data-split` = right|down, `data-path` = path from the tab root, e.g. "", "b", "b.a". */ divider: 'divider', @@ -86,6 +94,22 @@ export const TID = { presetChoices: 'preset-choices', /** One preset in a picker or in the choices, in configured order; text = preset name; `aria-selected="true"` on the highlighted one. */ presetOption: 'preset-option', + /** Button beside each preset while more than one exists; sends `removePreset` for it. */ + presetRemove: 'preset-remove', + /** Row holding a preset option and its remove button; draggable while more than one preset exists, a drop sends `movePreset`. */ + presetRow: 'preset-row', + /** Button under a preset list; opens the add-preset form. */ + presetAdd: 'preset-add', + /** The add-preset form; Escape closes it without sending anything. */ + presetForm: 'preset-form', + /** Name input of the add-preset form. */ + presetName: 'preset-name', + /** Command input of the add-preset form; empty means the login shell. */ + presetCommand: 'preset-command', + /** Submits the add-preset form; sends `addPreset` when the form is valid. */ + presetSubmit: 'preset-submit', + /** In the add-preset form after a refused preset; text says why. */ + presetError: 'preset-error', /** Dialog confirming the close of running terminals. */ closeDialog: 'close-dialog', /** One running terminal listed in the close dialog; text holds its preset and its id. */ @@ -121,7 +145,7 @@ export const repoTab = (path: string): string => `${byTestId(TID.repoTab)}[title export const hostBanner = (host: number): string => `${byTestId(TID.hostBanner)}[data-host="${String(host)}"]`; -export const worktreeEntry = (path: string): string => `${byTestId(TID.worktree)}[title="${path}"]`; +export const worktreeEntry = (path: string): string => `${byTestId(TID.worktree)}[data-path="${path}"]`; export const termTab = (termId: number): string => `${byTestId(TID.termTab)}[data-term="${String(termId)}"]`; diff --git a/test/e2e/fixture.ts b/test/e2e/fixture.ts index 3ff09bf..0390497 100644 --- a/test/e2e/fixture.ts +++ b/test/e2e/fixture.ts @@ -105,7 +105,15 @@ export const confirmAction = async (page: Page): Promise => { /** Hub-level requests (not `host` envelopes or `hello`) the page sent since `from`, without their `req`. */ export const hubRequests = (wire: WireRecorder, from = 0): { t: string; host: number }[] => - wire.sentMessages(from).flatMap((m) => (m.t === 'hello' || m.t === 'host' ? [] : [{ t: m.t, host: m.host }])); + wire.sentMessages(from).flatMap((m) => ('host' in m && m.t !== 'host' ? [{ t: m.t, host: m.host }] : [])); + +/** Preset edits the page sent since `from`, without their `req`. */ +export const presetRequests = (wire: WireRecorder, from = 0): unknown[] => + wire.sentMessages(from).flatMap((m): unknown[] => { + if (m.t === 'addPreset') return [{ t: m.t, preset: m.preset }]; + if (m.t === 'movePreset') return [{ t: m.t, name: m.name, to: m.to }]; + return m.t === 'removePreset' ? [{ t: m.t, name: m.name }] : []; + }); /** Every entry of `host` in the `hosts` messages the page received since `from`. */ export const hostEntries = (wire: WireRecorder, host: number, from = 0): HostEntry[] => @@ -199,7 +207,7 @@ export const selectWorktree = async (page: Page, path: string): Promise => /** Paths of the listed sidebar entries, in order. */ export const listedWorktrees = (page: Page): Promise => - page.locator(byTestId(TID.worktree)).evaluateAll((entries) => entries.map((e) => e.getAttribute('title') ?? '')); + page.locator(byTestId(TID.worktree)).evaluateAll((entries) => entries.map((e) => e.getAttribute('data-path') ?? '')); /** The terminal id of the selected terminal tab. */ export const activeTerm = async (page: Page): Promise => { diff --git a/test/e2e/hosts.spec.ts b/test/e2e/hosts.spec.ts index 0e31428..d67a53b 100644 --- a/test/e2e/hosts.spec.ts +++ b/test/e2e/hosts.spec.ts @@ -1,6 +1,6 @@ import { chmodSync, mkdirSync, rmSync, writeFileSync } from 'node:fs'; import { dirname, join } from 'node:path'; -import { PROTOCOL_VERSION } from '../../src/protocol/index.js'; +import { DAEMON_PROTOCOL_VERSION } from '../../src/protocol/index.js'; import { makeRepo, waitUntil } from '../support/daemon-host.js'; import { FakeDaemon } from '../support/fake-daemon.js'; import { installRemote, type FakeRemote } from '../support/fake-ssh.js'; @@ -149,7 +149,7 @@ test.describe('host actions', () => { process.kill(await oneDaemon(hub), 'SIGKILL'); await waitUntil(() => hub.daemonPids().length === 0, 'the daemon to exit'); rmSync(hub.socket, { force: true }); - const fake = await FakeDaemon.listen(hub.socket, { protocol: PROTOCOL_VERSION - 1, version: '0.0.9', instance }); + const fake = await FakeDaemon.listen(hub.socket, { protocol: DAEMON_PROTOCOL_VERSION - 1, version: '0.0.9', instance }); try { await hub.startHub(); await page.reload(); @@ -173,7 +173,7 @@ test.describe('host actions', () => { const app = makeRepo(join(remote.home, 'app')); await installRemote(hub, ssh, 'box'); const before = remote.current(); - const fake = await FakeDaemon.listen(remote.socket, { protocol: PROTOCOL_VERSION - 1, version: '0.0.9' }); + const fake = await FakeDaemon.listen(remote.socket, { protocol: DAEMON_PROTOCOL_VERSION - 1, version: '0.0.9' }); writeHubConfig(hub, { hosts: [{ name: 'box', ssh: 'box', repos: [app] }] }); try { const browserDialogs = refuseBrowserDialogs(page); diff --git a/test/e2e/picker.spec.ts b/test/e2e/picker.spec.ts index 68b6f80..5628674 100644 --- a/test/e2e/picker.spec.ts +++ b/test/e2e/picker.spec.ts @@ -1,4 +1,5 @@ -import type { Page } from '@playwright/test'; +import { readFileSync } from 'node:fs'; +import type { Locator, Page } from '@playwright/test'; import { decodeMessage } from '../../src/protocol/index.js'; import { byTestId, terminalBox, termTab, TID, worktreeEntry } from './contract.js'; import { @@ -7,7 +8,9 @@ import { LOCAL, makeRepoWith, newTerminal, + openPage, openUi, + presetRequests, PRESETS, ready, selectWorktree, @@ -308,3 +311,159 @@ test.describe('preset choices of a worktree without terminals', () => { await expect.poll(() => createsSent(wire, mark).map((m) => [m.preset, m.command])).toEqual([[SHELL.name, SHELL.command]]); }); }); + +test.describe('editing presets', () => { + const NEW = { name: 'echo', command: 'echo PRESET-NEW; exec cat -v' }; + + /** The configured presets in `config.json`. */ + const configured = (path: string): unknown => { + const config: unknown = JSON.parse(readFileSync(path, 'utf8')); + return typeof config === 'object' && config !== null && 'presets' in config ? config.presets : undefined; + }; + + /** Opens the add form in `list` and submits `name` and `command`. */ + const addPreset = async (list: Locator, name: string, command: string): Promise => { + await list.locator(byTestId(TID.presetAdd)).click(); + const form = list.locator(byTestId(TID.presetForm)); + await expect(form.locator(byTestId(TID.presetName))).toBeFocused(); + await form.locator(byTestId(TID.presetName)).fill(name); + await form.locator(byTestId(TID.presetCommand)).fill(command); + await form.locator(byTestId(TID.presetSubmit)).click(); + }; + + test('a preset added from the choices is saved, shown in every page, and runs its command', async ({ hub, page, wire, context }) => { + const { repo } = makeRepoWith(hub, 'app', []); + hub.writeRepos([repo]); + await openUi(hub, page); + const other = await openPage(hub, context); + const choices = page.locator(byTestId(TID.presetChoices)); + const mark = wire.markSent(); + + await addPreset(choices, ` ${NEW.name} `, NEW.command); + expect(presetRequests(wire, mark)).toEqual([{ t: 'addPreset', preset: NEW }]); + await expect(choices.locator(byTestId(TID.presetForm))).toHaveCount(0); + const names = [...PRESETS.map((p) => p.name), NEW.name]; + await expect(choices.locator(byTestId(TID.presetOption))).toHaveText(names); + await expect(other.page.locator(byTestId(TID.presetChoices)).locator(byTestId(TID.presetOption))).toHaveText(names); + expect(configured(hub.configPath)).toEqual([...PRESETS, NEW]); + + await choices.locator(byTestId(TID.presetOption), { hasText: NEW.name }).click(); + await waitScreen(page, await activeTerm(page), 'PRESET-NEW'); + }); + + test('× removes a preset from every page and config.json, and the last preset offers none', async ({ hub, page, wire, context }) => { + const { repo } = makeRepoWith(hub, 'app', []); + hub.writeRepos([repo]); + await openUi(hub, page); + const other = await openPage(hub, context); + const choices = page.locator(byTestId(TID.presetChoices)); + const mark = wire.markSent(); + + await choices.locator(byTestId(TID.presetOption), { hasText: CAT.name }).locator('..').locator(byTestId(TID.presetRemove)).click(); + expect(presetRequests(wire, mark)).toEqual([{ t: 'removePreset', name: CAT.name }]); + await expect(choices.locator(byTestId(TID.presetOption))).toHaveText([SHELL.name]); + await expect(other.page.locator(byTestId(TID.presetChoices)).locator(byTestId(TID.presetOption))).toHaveText([SHELL.name]); + expect(configured(hub.configPath)).toEqual([SHELL]); + await expect(choices.locator(byTestId(TID.presetRemove))).toHaveCount(0); + }); + + test('the add form refuses a taken name without sending, and Escape closes it', async ({ hub, page, wire }) => { + const { repo } = makeRepoWith(hub, 'app', []); + hub.writeRepos([repo]); + await openUi(hub, page); + const choices = page.locator(byTestId(TID.presetChoices)); + const mark = wire.markSent(); + + await addPreset(choices, CAT.name, 'anything'); + await expect(choices.locator(byTestId(TID.presetError))).toHaveText(`A preset named ${CAT.name} exists`); + await choices.locator(byTestId(TID.presetName)).fill(' '); + await choices.locator(byTestId(TID.presetName)).press('Enter'); + await expect(choices.locator(byTestId(TID.presetError))).toHaveText('Give the preset a name'); + await choices.locator(byTestId(TID.presetName)).press('Escape'); + await expect(choices.locator(byTestId(TID.presetForm))).toHaveCount(0); + await expect(choices.locator(byTestId(TID.presetAdd))).toBeVisible(); + expect(presetRequests(wire, mark)).toEqual([]); + expect(configured(hub.configPath)).toEqual(PRESETS); + }); + + test('a preset added in the new-tab picker keeps the picker open and can be chosen there', async ({ hub, page, wire }) => { + const { repo } = makeRepoWith(hub, 'app', []); + hub.writeRepos([repo]); + await openUi(hub, page); + await newTerminal(page); + await page.locator(byTestId(TID.newTab)).click(); + const picker = page.locator(byTestId(TID.presetPicker)); + await expect(picker).toBeVisible(); + + await addPreset(picker, NEW.name, NEW.command); + await expect(picker.locator(byTestId(TID.presetOption))).toHaveText([...PRESETS.map((p) => p.name), NEW.name]); + await expect(picker).toBeVisible(); + const before = await activeTerm(page); + const mark = wire.markSent(); + await picker.locator(byTestId(TID.presetOption), { hasText: NEW.name }).click(); + await expect.poll(() => createsSent(wire, mark).map((m) => [m.preset, m.command])).toEqual([[NEW.name, NEW.command]]); + await expect.poll(() => activeTerm(page)).not.toBe(before); + await waitScreen(page, await activeTerm(page), 'PRESET-NEW'); + }); + + test('dragging a preset reorders the presets in every page and in config.json', async ({ hub, page, wire, context }) => { + const { repo } = makeRepoWith(hub, 'app', []); + hub.writeRepos([repo]); + await openUi(hub, page); + const other = await openPage(hub, context); + const choices = page.locator(byTestId(TID.presetChoices)); + const row = (name: string): Locator => choices.locator(byTestId(TID.presetRow), { hasText: name }); + const mark = wire.markSent(); + + await row(CAT.name).dragTo(row(SHELL.name), { targetPosition: { x: 20, y: 2 } }); + await expect.poll(() => presetRequests(wire, mark)).toEqual([{ t: 'movePreset', name: CAT.name, to: 0 }]); + await expect(choices.locator(byTestId(TID.presetOption))).toHaveText([CAT.name, SHELL.name]); + await expect(other.page.locator(byTestId(TID.presetChoices)).locator(byTestId(TID.presetOption))).toHaveText([CAT.name, SHELL.name]); + expect(configured(hub.configPath)).toEqual([CAT, SHELL]); + }); + + test('Alt+Up moves the highlighted preset up in the picker, which keeps it highlighted', async ({ hub, page, wire }) => { + const { repo } = makeRepoWith(hub, 'app', []); + hub.writeRepos([repo]); + await openUi(hub, page); + await newTerminal(page); + await page.locator(byTestId(TID.newTab)).click(); + const picker = page.locator(byTestId(TID.presetPicker)); + const options = picker.locator(byTestId(TID.presetOption)); + await page.keyboard.press('ArrowDown'); + await expect(options.nth(1)).toHaveAttribute('aria-selected', 'true'); + + await page.keyboard.press('Alt+ArrowUp'); + await expect(options).toHaveText([CAT.name, SHELL.name]); + await expect(options.nth(0)).toHaveAttribute('aria-selected', 'true'); + await expect(options.nth(0)).toBeFocused(); + expect(configured(hub.configPath)).toEqual([CAT, SHELL]); + + const mark = wire.markSent(); + await page.keyboard.press('Enter'); + await expect.poll(() => createsSent(wire, mark).map((m) => m.preset)).toEqual([CAT.name]); + }); + + test('two quick Alt+Down presses only ever move the highlighted preset', async ({ hub, page }) => { + const third = { name: 'third', command: 'echo THIRD' }; + hub.presets = [...PRESETS, third]; + const { repo } = makeRepoWith(hub, 'app', []); + hub.writeRepos([repo]); + await openUi(hub, page); + await newTerminal(page); + await page.locator(byTestId(TID.newTab)).click(); + const options = page.locator(byTestId(TID.presetPicker)).locator(byTestId(TID.presetOption)); + await expect(options.nth(0)).toBeFocused(); + + await page.keyboard.press('Alt+ArrowDown'); + await page.keyboard.press('Alt+ArrowDown'); + await expect(options.nth(0)).toHaveText(CAT.name); + // The second press either waited for the first move or moved the same preset again. + const order = await options.allTextContents(); + expect([ + [CAT.name, SHELL.name, third.name], + [CAT.name, third.name, SHELL.name], + ]).toContainEqual(order); + expect(configured(hub.configPath)).toEqual(order.map((name) => [...PRESETS, third].find((p) => p.name === name))); + }); +}); diff --git a/test/e2e/sidebar.spec.ts b/test/e2e/sidebar.spec.ts index 10b1bd2..00e465d 100644 --- a/test/e2e/sidebar.spec.ts +++ b/test/e2e/sidebar.spec.ts @@ -1,4 +1,9 @@ +import { existsSync, writeFileSync } from 'node:fs'; +import type { 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'; +import { COMMANDS, runIn } from './attention-helpers.js'; import { byTestId, termTab, TID, worktreeEntry } from './contract.js'; import { confirmClose, @@ -16,6 +21,7 @@ import { test, typeLine, waitScreen, + watcher, } from './fixture.js'; const label = (path: string): string => `${worktreeEntry(path)} ${byTestId(TID.worktreeLabel)}`; @@ -230,3 +236,116 @@ test.describe('sidebar width', () => { expect(await sidebarWidth()).toBe(384); }); }); + +test.describe('worktree status and deletion', () => { + /** Gives `repo` a bare `origin` holding its `main`, with `origin/HEAD` pointing at it. */ + const withOrigin = (hub: HubHost, repo: string): void => { + const origin = join(hub.dir, 'origin.git'); + git(hub.dir, 'init', '-q', '--bare', '-b', 'main', origin); + git(repo, 'remote', 'add', 'origin', origin); + git(repo, 'push', '-q', 'origin', 'main'); + git(repo, 'remote', 'set-head', 'origin', 'main'); + }; + + const titleOf = async (page: Page, path: string): Promise => + (await page.locator(worktreeEntry(path)).getAttribute('title')) ?? ''; + + test('hovering a worktree shows its path and what each of its terminals is doing', async ({ hub, page }) => { + const { repo, worktrees } = makeRepoWith(hub, 'app', ['feat']); + const feat = worktrees[0] ?? ''; + hub.writeRepos([repo]); + await openUi(hub, page); + await expect.poll(() => titleOf(page, feat)).toBe(`${feat}\nno terminals`); + + const client = await watcher(hub, repo); + const ticker = await runIn(client, feat, COMMANDS.ticker); + await expect.poll(() => titleOf(page, feat), { timeout: 10_000 }).toBe(`${feat}\nprobe ${String(ticker)}: running`); + const failing = await runIn(client, feat, COMMANDS.fail); + await expect + .poll(() => titleOf(page, feat), { timeout: 10_000 }) + .toBe(`${feat}\nprobe ${String(ticker)}: running\nprobe ${String(failing)}: exited with code 1`); + }); + + test('deleting a merged, clean worktree without running terminals asks nothing', async ({ hub, page }) => { + const { repo, worktrees } = makeRepoWith(hub, 'app', ['feat']); + const feat = worktrees[0] ?? ''; + withOrigin(hub, repo); + hub.writeRepos([repo]); + await openUi(hub, page); + await page.locator(worktreeEntry(feat)).click({ button: 'right' }); + await page.locator(byTestId(TID.worktreeMenu)).locator(byTestId(TID.deleteWorktree)).click(); + await expect(page.locator(worktreeEntry(feat))).toHaveCount(0); + expect(existsSync(feat)).toBe(false); + await expect(page.locator(byTestId(TID.confirmDialog))).toHaveCount(0); + await expect(page.locator(byTestId(TID.worktreeMenu))).toHaveCount(0); + expect(git(repo, 'branch', '--list', 'feat')).toContain('feat'); + }); + + test('deleting a worktree at risk names every reason, keeps it on cancel and deletes it on confirm', async ({ hub, page }) => { + const { repo, worktrees } = makeRepoWith(hub, 'app', ['feat']); + const feat = worktrees[0] ?? ''; + withOrigin(hub, repo); + writeFileSync(join(feat, 'a'), 'a'); + git(feat, 'add', 'a'); + git(feat, 'commit', '-q', '-m', 'a'); + writeFileSync(join(feat, 'untracked'), 'new'); + hub.writeRepos([repo]); + await openUi(hub, page); + const client = await watcher(hub, repo); + const ticker = await runIn(client, feat, COMMANDS.ticker); + await expect.poll(() => titleOf(page, feat), { timeout: 10_000 }).toContain(`probe ${String(ticker)}`); + const dialog = page.locator(byTestId(TID.confirmDialog)); + const remove = async (): Promise => { + await page.locator(worktreeEntry(feat)).click({ button: 'right' }); + await page.locator(byTestId(TID.deleteWorktree)).click(); + await expect(dialog).toBeVisible(); + }; + + await remove(); + await expect(dialog.locator(byTestId(TID.confirmReason))).toHaveText([ + 'It has 1 commit not in origin/main.', + 'It has 1 uncommitted change, untracked files included.', + `Running terminals will be closed: probe ${String(ticker)}.`, + ]); + await expect(dialog.locator(byTestId(TID.confirmNote))).toHaveText('Its branch is kept.'); + await dialog.locator(byTestId(TID.confirmCancel)).click(); + await expect(dialog).toHaveCount(0); + expect(existsSync(feat)).toBe(true); + + await remove(); + const from = client.mark(); + await dialog.locator(byTestId(TID.confirmOk)).click(); + await expect(page.locator(worktreeEntry(feat))).toHaveCount(0); + expect(existsSync(feat)).toBe(false); + await client.waitFor('termClosed', (m) => m.termId === ticker, { from }); + expect(git(repo, 'rev-parse', 'feat')).toMatch(/^[0-9a-f]{40}$/); + }); + + test('deleting a detached worktree with commits says they are left only in the reflog', async ({ hub, page }) => { + const { repo } = makeRepoWith(hub, 'app', []); + withOrigin(hub, repo); + const detached = detachedWorktree(repo, 'loose'); + writeFileSync(join(detached.path, 'a'), 'a'); + git(detached.path, 'add', 'a'); + git(detached.path, 'commit', '-q', '-m', 'a'); + hub.writeRepos([repo]); + await openUi(hub, page); + await page.locator(worktreeEntry(detached.path)).click({ button: 'right' }); + await page.locator(byTestId(TID.deleteWorktree)).click(); + const dialog = page.locator(byTestId(TID.confirmDialog)); + await expect(dialog.locator(byTestId(TID.confirmReason))).toHaveText(['It has 1 commit not in origin/main.']); + await expect(dialog.locator(byTestId(TID.confirmNote))).toContainText('reachable only through the reflog'); + }); + + test('the main worktree cannot be deleted, and Escape closes the menu', async ({ hub, page }) => { + const { repo } = makeRepoWith(hub, 'app', []); + hub.writeRepos([repo]); + await openUi(hub, page); + await page.locator(worktreeEntry(repo)).click({ button: 'right' }); + const item = page.locator(byTestId(TID.deleteWorktree)); + await expect(item).toBeDisabled(); + await expect(item).toHaveAttribute('title', 'The main worktree cannot be deleted'); + await page.locator(byTestId(TID.worktreeMenu)).press('Escape'); + await expect(page.locator(byTestId(TID.worktreeMenu))).toHaveCount(0); + }); +}); diff --git a/test/e2e/splits.spec.ts b/test/e2e/splits.spec.ts index b1691d5..f576d9f 100644 --- a/test/e2e/splits.spec.ts +++ b/test/e2e/splits.spec.ts @@ -1,4 +1,5 @@ import { chmodSync } from 'node:fs'; +import type { Page } from '@playwright/test'; import type { DaemonClient } from '../support/daemon-client.js'; import { byTestId, pane, terminalBox, termTab, TID } from './contract.js'; import { @@ -529,3 +530,40 @@ test.describe('pane focus', () => { await expect.poll(() => focusedPane(page)).toBe(top); }); }); + +test.describe('pane headers', () => { + /** The top edges of the pane `termId` and of its terminal box. */ + const tops = async (page: Page, termId: number): Promise<[number, number]> => [ + (await page.locator(pane(termId)).boundingBox())?.y ?? Number.NaN, + (await page.locator(terminalBox(LOCAL, termId)).boundingBox())?.y ?? Number.NaN, + ]; + + test('a lone pane has no header and its terminal fills it; split panes each have one', async ({ hub, page }) => { + const { repo } = makeRepoWith(hub, 'app', []); + hub.writeRepos([repo]); + await openUi(hub, page); + const left = await newTerminal(page); + await ready(page, left); + await expect(page.locator(byTestId(TID.paneClose))).toHaveCount(0); + await expect + .poll(async () => { + const [paneTop, boxTop] = await tops(page, left); + return boxTop - paneTop; + }) + .toBe(0); + + const right = await splitPane(page, 'right'); + for (const term of [left, right]) await expect(page.locator(pane(term)).locator(byTestId(TID.paneClose))).toHaveCount(1); + await expect + .poll(async () => { + const [paneTop, boxTop] = await tops(page, right); + return boxTop - paneTop; + }) + .toBe(20); + + await page.locator(pane(right)).locator(byTestId(TID.paneClose)).click(); + await confirmClose(page); + await expect(page.locator(pane(right))).toHaveCount(0); + await expect(page.locator(byTestId(TID.paneClose))).toHaveCount(0); + }); +}); diff --git a/test/e2e/terminals.spec.ts b/test/e2e/terminals.spec.ts index ae50d5e..6bf159c 100644 --- a/test/e2e/terminals.spec.ts +++ b/test/e2e/terminals.spec.ts @@ -151,8 +151,9 @@ test.describe('terminal tabs', () => { await ready(page, term); await typeLine(page, term, "echo BYE''-MARK; exit"); await expect(page.locator(termTab(term))).toContainText('exited'); - await expect.poll(() => markOf(page, pane(term))).toBe('exited'); await expect.poll(() => markOf(page, termTab(term))).toBe('exited'); + // A lone pane has no header: its tab carries the mark. + expect(await markOf(page, pane(term))).toBeNull(); await expect(page.locator(terminalBox(LOCAL, term))).toBeVisible(); expect(await screenOf(page, term)).toContain('BYE-MARK'); }); @@ -527,3 +528,77 @@ test.describe('terminal settings', () => { expect(await opened.evaluate(() => window.opener === null)).toBe(true); }); }); + +declare global { + interface Window { + /** Set by `stubFocus`: makes the page report and announce gaining or losing the focus. */ + __wtdFocus?: (focused: boolean) => void; + } +} + +test.describe('terminal size with several pages', () => { + /** Makes `page` report the focus only when told to, as a browser window does; Playwright reports every page as focused. */ + const stubFocus = async (page: Page, focused: boolean): Promise => { + await page.addInitScript((initial) => { + let current = initial; + document.hasFocus = () => current; + window.__wtdFocus = (next) => { + current = next; + window.dispatchEvent(new Event(next ? 'focus' : 'blur')); + }; + }, focused); + }; + + const setFocus = (page: Page, focused: boolean): Promise => + page.evaluate((next) => { + window.__wtdFocus?.(next); + }, focused); + + let probes = 0; + + /** The PTY's size as `stty size` reports it in the terminal, typed through `page`. */ + const ptySize = async (page: Page, termId: number): Promise => { + const probe = ++probes; + await typeLine(page, termId, `echo SZ${String(probe)}-$(stty size)-END`); + const pattern = new RegExp(`SZ${String(probe)}-(\\d+ \\d+)-END`); + let size = ''; + await expect + .poll(async () => { + size = pattern.exec((await screenOf(page, termId)) ?? '')?.[1] ?? ''; + return size; + }) + .not.toBe(''); + return size; + }; + + test('the PTY takes the size of the page in focus, whatever unfocused pages do', async ({ hub, page, context }) => { + const { repo } = makeRepoWith(hub, 'app', []); + hub.writeRepos([repo]); + await stubFocus(page, true); + await openUi(hub, page); + const term = await newTerminal(page); + await ready(page, term); + const focusedSize = await ptySize(page, term); + + const other = await context.newPage(); + await stubFocus(other, false); + await other.setViewportSize({ width: 700, height: 450 }); + await other.goto(await hub.pageUrl()); + await expect(other.locator(terminalBox(LOCAL, term))).toBeVisible(); + expect(await ptySize(page, term)).toBe(focusedSize); + + await setFocus(page, false); + await setFocus(other, true); + const otherSize = await ptySize(page, term); + expect(otherSize).not.toBe(focusedSize); + + await page.setViewportSize({ width: 1100, height: 650 }); + expect(await ptySize(page, term)).toBe(otherSize); + + await setFocus(other, false); + await setFocus(page, true); + const resized = await ptySize(page, term); + expect(resized).not.toBe(otherSize); + expect(resized).not.toBe(focusedSize); + }); +}); diff --git a/test/integration/ssh-localhost.test.ts b/test/integration/ssh-localhost.test.ts index d6fc60d..76309d6 100644 --- a/test/integration/ssh-localhost.test.ts +++ b/test/integration/ssh-localhost.test.ts @@ -3,7 +3,7 @@ import { chmodSync, mkdtempSync, readdirSync, readFileSync, realpathSync, rmSync import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest'; -import { PROTOCOL_VERSION } from '../../src/protocol/index.js'; +import { DAEMON_PROTOCOL_VERSION } from '../../src/protocol/index.js'; import { alive, makeRepo, waitUntil } from '../support/daemon-host.js'; import { exec, REPO_ROOT } from '../support/exec.js'; import { FakeDaemon } from '../support/fake-daemon.js'; @@ -153,7 +153,11 @@ describe('remote host over real ssh', () => { it('reinstalls and restarts an outdated remote daemon', async () => { await install(); - const fake = await FakeDaemon.listen(remote.socket, { protocol: PROTOCOL_VERSION - 1, version: '0.0.1-old', instance: 'old_remote' }); + const fake = await FakeDaemon.listen(remote.socket, { + protocol: DAEMON_PROTOCOL_VERSION - 1, + version: '0.0.1-old', + instance: 'old_remote', + }); await startHub(); const client = await host.session(); expect(await client.waitHost(REMOTE, (h) => h.status === 'outdated', { timeout: 30_000 })).toMatchObject({ instance: 'old_remote' }); diff --git a/test/process/cli.test.ts b/test/process/cli.test.ts index 04f47d9..6396b08 100644 --- a/test/process/cli.test.ts +++ b/test/process/cli.test.ts @@ -36,7 +36,11 @@ describe('dist/wtd.mjs', () => { }); it('prints its version', () => { - expect(wtd('--version')).toEqual({ status: 0, stdout: `wtd ${packageVersion()} (protocol 5)\n`, stderr: '' }); + expect(wtd('--version')).toEqual({ + status: 0, + stdout: `wtd ${packageVersion()} (daemon protocol 6, browser protocol 8)\n`, + stderr: '', + }); }); it('prints help listing every command and option', () => { diff --git a/test/process/connect.test.ts b/test/process/connect.test.ts index 834f5e6..6ac0792 100644 --- a/test/process/connect.test.ts +++ b/test/process/connect.test.ts @@ -2,7 +2,7 @@ import { spawn } from 'node:child_process'; import { mkdirSync, statSync, writeFileSync } from 'node:fs'; import { join } from 'node:path'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; -import { PROTOCOL_VERSION } from '../../src/protocol/index.js'; +import { DAEMON_PROTOCOL_VERSION } from '../../src/protocol/index.js'; import { DaemonClient } from '../support/daemon-client.js'; import { DaemonHost, makeRepo, sessionOf, waitUntil, type WtdProcess } from '../support/daemon-host.js'; @@ -38,8 +38,8 @@ describe('wtd connect', () => { const { process: connect, client } = bridge(); const hello = await client.waitFor('hello', () => true, { timeout: 10_000 }); expect(client.received[0]).toEqual({ kind: 'message', message: hello }); - expect(hello.protocol).toBe(PROTOCOL_VERSION); - expect(hello.protocol).toBe(5); + expect(hello.protocol).toBe(DAEMON_PROTOCOL_VERSION); + expect(hello.protocol).toBe(6); const [daemon, ...others] = host.daemonPids(); if (daemon === undefined) throw new Error('no daemon is running'); expect(others).toEqual([]); diff --git a/test/process/daemon-server.test.ts b/test/process/daemon-server.test.ts index d7d4b22..0d8995b 100644 --- a/test/process/daemon-server.test.ts +++ b/test/process/daemon-server.test.ts @@ -2,7 +2,7 @@ import { spawn, spawnSync } from 'node:child_process'; import { existsSync, mkdirSync, readFileSync, renameSync, statSync, writeFileSync } from 'node:fs'; import { join } from 'node:path'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; -import { encodeStreamData, PROTOCOL_VERSION } from '../../src/protocol/index.js'; +import { encodeStreamData, DAEMON_PROTOCOL_VERSION } from '../../src/protocol/index.js'; import type { DaemonClient } from '../support/daemon-client.js'; import { REPO_ROOT } from '../support/exec.js'; import { @@ -141,15 +141,15 @@ describe('single instance', () => { }); describe('handshake', () => { - it('greets every connection with hello for protocol 5, the package version and one instance id', async () => { + it('greets every connection with hello for protocol 6, the package version and one instance id', async () => { await host.start(); const a = await host.rawClient(); const b = await host.rawClient(); const helloA = await a.waitFor('hello'); const helloB = await b.waitFor('hello'); expect(a.received[0]).toEqual({ kind: 'message', message: helloA }); - expect(helloA.protocol).toBe(PROTOCOL_VERSION); - expect(helloA.protocol).toBe(5); + expect(helloA.protocol).toBe(DAEMON_PROTOCOL_VERSION); + expect(helloA.protocol).toBe(6); expect(helloA.version).toBe(packageVersion()); expect(helloA.instance).toMatch(/^[A-Za-z0-9_-]{1,64}$/); expect(helloB.instance).toBe(helloA.instance); @@ -192,7 +192,7 @@ describe('handshake', () => { describe('malformed traffic', () => { const output = encodeStreamData({ kind: 'output', termId: 1, offset: 0, data: new Uint8Array([120]) }); const snapshot = encodeStreamData({ kind: 'snapshot', termId: 1, offset: 0, data: new Uint8Array([120]) }); - const hello = { t: 'hello', protocol: PROTOCOL_VERSION, version: '1', instance: 'again' }; + const hello = { t: 'hello', protocol: DAEMON_PROTOCOL_VERSION, version: '1', instance: 'again' }; it.each([ [ diff --git a/test/process/daemon-worktrees.test.ts b/test/process/daemon-worktrees.test.ts index 629f776..4e63cf1 100644 --- a/test/process/daemon-worktrees.test.ts +++ b/test/process/daemon-worktrees.test.ts @@ -386,3 +386,96 @@ describe('discoverRepos', () => { expect(reply).toEqual({ t: 'reposDiscovered', req: ANY_REQ, repos: [app] }); }); }); + +describe('removeWorktree', () => { + /** Gives the repo a bare `origin` holding its `main`, with `origin/HEAD` pointing at it. */ + const withOrigin = (): void => { + const origin = join(host.dir, 'origin.git'); + git(host.dir, 'init', '-q', '--bare', '-b', 'main', origin); + git(repo, 'remote', 'add', 'origin', origin); + git(repo, 'push', '-q', 'origin', 'main'); + git(repo, 'remote', 'set-head', 'origin', 'main'); + }; + + const commit = (cwd: string, file: string): void => { + writeFileSync(join(cwd, file), file); + git(cwd, 'add', file); + git(cwd, 'commit', '-q', '-m', file); + }; + + it('removes a merged, clean worktree at once, closing its exited terminals and keeping its branch', async () => { + withOrigin(); + const wt = addWorktree(repo, join(host.dir, 'wt'), 'feat'); + const client = await host.client(); + await client.watch(repo); + const exited = await client.create(wt, { command: 'true' }); + await client.waitFor('termExited', (m) => m.termId === exited.termId); + const from = client.mark(); + expect(await client.request({ t: 'removeWorktree', worktree: wt, force: false })).toEqual({ t: 'done', req: ANY_REQ }); + expect(existsSync(wt)).toBe(false); + await client.waitFor('termClosed', (m) => m.termId === exited.termId, { from }); + await client.waitFor('worktreesChanged', (m) => find(m.worktrees, wt) === undefined, { from }); + expect(git(repo, 'branch', '--list', 'feat')).toContain('feat'); + }); + + it('keeps a worktree at risk and reports every risk: commits ahead, changes and running terminals', async () => { + withOrigin(); + const wt = addWorktree(repo, join(host.dir, 'wt'), 'feat'); + commit(wt, 'a'); + writeFileSync(join(wt, 'untracked'), 'new'); + const client = await host.client(); + await client.watch(repo); + const running = await client.create(wt, { command: 'exec sleep 60' }); + expect(await client.request({ t: 'removeWorktree', worktree: wt, force: false })).toEqual({ + t: 'worktreeAtRisk', + req: ANY_REQ, + worktree: wt, + base: 'origin/main', + ahead: 1, + changes: 1, + running: [running.termId], + }); + expect(existsSync(wt)).toBe(true); + await client.expectNone('termClosed', () => true, 300); + }); + + it('reports a worktree of a repo without an origin as at risk', async () => { + const wt = addWorktree(repo, join(host.dir, 'wt'), 'feat'); + const client = await host.client(); + await client.watch(repo); + expect(await client.request({ t: 'removeWorktree', worktree: wt, force: false })).toMatchObject({ + t: 'worktreeAtRisk', + base: null, + ahead: null, + changes: 0, + running: [], + }); + expect(existsSync(wt)).toBe(true); + }); + + it('removes a worktree at risk when forced, closing its running terminals', async () => { + withOrigin(); + const wt = addWorktree(repo, join(host.dir, 'wt'), 'feat'); + commit(wt, 'a'); + writeFileSync(join(wt, 'untracked'), 'new'); + const client = await host.client(); + await client.watch(repo); + const running = await client.create(wt, { command: 'exec sleep 60' }); + const from = client.mark(); + expect(await client.request({ t: 'removeWorktree', worktree: wt, force: true })).toEqual({ t: 'done', req: ANY_REQ }); + expect(existsSync(wt)).toBe(false); + await client.waitFor('termClosed', (m) => m.termId === running.termId, { from }); + expect(git(repo, 'rev-parse', 'feat')).toMatch(/^[0-9a-f]{40}$/); + }); + + it('refuses the main worktree, a locked one and a path that is not a worktree', async () => { + const wt = addWorktree(repo, join(host.dir, 'wt'), 'feat'); + git(repo, 'worktree', 'lock', wt); + const client = await host.client(); + await client.watch(repo); + expect(await client.fails({ t: 'removeWorktree', worktree: repo, force: true })).toBe('busy'); + expect(await client.fails({ t: 'removeWorktree', worktree: wt, force: true })).toBe('busy'); + expect(await client.fails({ t: 'removeWorktree', worktree: join(host.dir, 'nope'), force: true })).toBe('unknown-worktree'); + expect(existsSync(repo) && existsSync(wt)).toBe(true); + }); +}); diff --git a/test/process/hub-router.test.ts b/test/process/hub-router.test.ts index b5ee2b0..a60cc94 100644 --- a/test/process/hub-router.test.ts +++ b/test/process/hub-router.test.ts @@ -2,7 +2,7 @@ import { existsSync, mkdirSync, readFileSync, rmSync, symlinkSync, writeFileSync import { hostname } from 'node:os'; import { join } from 'node:path'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; -import { FLOW_HIGH, FrameKind, PROTOCOL_VERSION, type Terminal } from '../../src/protocol/index.js'; +import { FLOW_HIGH, FrameKind, DAEMON_PROTOCOL_VERSION, type Terminal } from '../../src/protocol/index.js'; import { addWorktree, alive, DaemonHost, makeRepo, residentBytes, sleep, waitUntil, type WtdProcess } from '../support/daemon-host.js'; import { FakeDaemon, SocketProxy } from '../support/fake-daemon.js'; import { currentNodeDir, FakeSsh, installRemote, type FakeRemote } from '../support/fake-ssh.js'; @@ -20,6 +20,8 @@ const MiB = 1024 * KiB; const SHELL = { name: 'shell', command: null }; const CLAUDE = { name: 'claude', command: 'claude' }; +/** The presets of a configuration without `presets`. */ +const DEFAULT_PRESETS = [SHELL, CLAUDE, { name: 'codex', command: 'codex' }]; let host: HubHost; let hub: WtdProcess; @@ -113,7 +115,7 @@ describe('configuration snapshots', () => { it('gives a new session an edited configuration, leaving an open session unchanged', async () => { const open = await host.session(); - expect(presetsOf(open)).toEqual([[SHELL]]); + expect(presetsOf(open)).toEqual([DEFAULT_PRESETS]); const other = makeRepo(join(host.dir, 'other')); host.presets = [SHELL, CLAUDE]; host.writeRepos([repo, other]); @@ -122,7 +124,7 @@ describe('configuration snapshots', () => { expect(presetsOf(fresh)).toEqual([[SHELL, CLAUDE]]); await open.expectNone('hosts', (m) => m.hosts.some((h) => h.repos.includes(other)), 1000); expect(open.hosts()[0]?.repos).toEqual([repo]); - expect(presetsOf(open)).toEqual([[SHELL]]); + expect(presetsOf(open)).toEqual([DEFAULT_PRESETS]); }); it.each([ @@ -199,7 +201,7 @@ describe('daemon links and host status', () => { }); it('reports reconnecting, then down after 3 failures, and keeps retrying', async () => { - const fake = await FakeDaemon.listen(host.socket, { protocol: PROTOCOL_VERSION, accept: 'close' }); + const fake = await FakeDaemon.listen(host.socket, { protocol: DAEMON_PROTOCOL_VERSION, accept: 'close' }); const client = await host.session(); await client.waitHost(0, (h) => h.status === 'reconnecting'); await client.waitHost(0, (h) => h.status === 'down', { timeout: 10_000 }); @@ -269,7 +271,7 @@ describe('routing', () => { }); it('answers a request to a reconnecting host with host-unavailable and drops its fire-and-forget messages', async () => { - await FakeDaemon.listen(host.socket, { protocol: PROTOCOL_VERSION, accept: 'close' }); + await FakeDaemon.listen(host.socket, { protocol: DAEMON_PROTOCOL_VERSION, accept: 'close' }); const client = await host.session(); await client.waitHost(0, (h) => h.status === 'reconnecting' || h.status === 'down'); const reply = await client.request(0, { t: 'watchRepo', repo }); @@ -393,12 +395,12 @@ describe('daemon restart', () => { // A daemon the session's reconnect started alongside the restart refuses to start and exits. await waitUntil(() => host.daemonPids().length === 1, 'one daemon'); const direct = await host.client(); - expect((await direct.handshake()).protocol).toBe(PROTOCOL_VERSION); + expect((await direct.handshake()).protocol).toBe(DAEMON_PROTOCOL_VERSION); }); it('restarts a newer daemon whose messages after hello are not valid for the hub’s protocol', async () => { const fake = await FakeDaemon.listen(host.socket, { - protocol: PROTOCOL_VERSION + 1, + protocol: DAEMON_PROTOCOL_VERSION + 1, instance: 'newer_1', afterHello: ['{"t":"frobnicated","widgets":[1,2,3]}', '{"t":"repoState","repo":7}'], }); @@ -454,7 +456,7 @@ describe('daemon restart', () => { }); it('answers host-unavailable for a host that is not connected or outdated', async () => { - const fake = await FakeDaemon.listen(host.socket, { protocol: PROTOCOL_VERSION, accept: 'close' }); + const fake = await FakeDaemon.listen(host.socket, { protocol: DAEMON_PROTOCOL_VERSION, accept: 'close' }); const client = await host.session(); await client.waitHost(0, (h) => h.status === 'reconnecting' || h.status === 'down'); const reply = await client.hubRequest({ t: 'restartDaemon', host: 0 }); @@ -997,6 +999,74 @@ describe('removing repos', () => { }); }); +describe('editing presets', () => { + const HTOP = { name: 'htop', command: 'htop' }; + + it('adds a preset to config.json, answers done, then sends the presets to every open session', async () => { + host.presets = [SHELL]; + host.writeRepos([repo]); + const a = await host.session(); + const b = await host.session(); + const [fromA, fromB] = [a.mark(), b.mark()]; + expect(await a.hubRequest({ t: 'addPreset', preset: HTOP })).toEqual({ from: 'hub', m: { t: 'done', req: 1 } }); + for (const [client, from] of [ + [a, fromA], + [b, fromB], + ] as const) { + expect((await client.waitFor('presets', () => true, { from })).presets).toEqual([SHELL, HTOP]); + } + const doneAt = a.messages.findIndex((m, i) => i >= fromA && m.t === 'done'); + const presetsAt = a.messages.findIndex((m, i) => i >= fromA && m.t === 'presets'); + expect(doneAt).toBeLessThan(presetsAt); + expect(configFile()).toEqual({ port: host.port, repos: [repo], presets: [SHELL, HTOP] }); + }); + + it('removes a preset, writing the defaults without it when config.json had none', async () => { + host.writeConfig({ port: host.port, repos: [repo] }); + const client = await host.session(); + const from = client.mark(); + expect((await client.hubRequest({ t: 'removePreset', name: 'claude' })).m).toEqual({ t: 'done', req: 1 }); + const expected = [SHELL, { name: 'codex', command: 'codex' }]; + expect((await client.waitFor('presets', () => true, { from })).presets).toEqual(expected); + expect(configFile()).toEqual({ port: host.port, repos: [repo], presets: expected }); + }); + + it('moves a preset in config.json, answers done, then sends the new order to every open session', async () => { + host.presets = [SHELL, CLAUDE, HTOP]; + host.writeRepos([repo]); + const a = await host.session(); + const b = await host.session(); + const fromB = b.mark(); + expect((await a.hubRequest({ t: 'movePreset', name: 'htop', to: 0 })).m).toEqual({ t: 'done', req: 1 }); + expect((await b.waitFor('presets', () => true, { from: fromB })).presets).toEqual([HTOP, SHELL, CLAUDE]); + expect(configFile()).toEqual({ port: host.port, repos: [repo], presets: [HTOP, SHELL, CLAUDE] }); + }); + + it('answers done without sending presets for a name it does not list', async () => { + host.presets = [SHELL]; + host.writeRepos([repo]); + const before = readFileSync(host.configPath, 'utf8'); + const client = await host.session(); + expect((await client.hubRequest({ t: 'removePreset', name: 'nope' })).m).toEqual({ t: 'done', req: 1 }); + await client.expectNone('presets', () => true, 300); + expect(readFileSync(host.configPath, 'utf8')).toBe(before); + }); + + it.each([ + ['adding a taken name', { t: 'addPreset', preset: { name: 'shell', command: 'zsh' } }, 'a preset named shell exists'], + ['removing the last preset', { t: 'removePreset', name: 'shell' }, 'the last preset cannot be removed'], + ] as const)('refuses %s with internal, leaving config.json unchanged', async (_name, body, text) => { + host.presets = [SHELL]; + host.writeRepos([repo]); + const before = readFileSync(host.configPath, 'utf8'); + const client = await host.session(); + const reply = await client.hubRequest(body); + expect(reply).toMatchObject({ from: 'hub', m: { t: 'error', req: 1, host: null, code: 'internal' } }); + expect(reply.m.t === 'error' ? reply.m.message : '').toContain(text); + expect(readFileSync(host.configPath, 'utf8')).toBe(before); + }); +}); + describe('configuration edits', () => { it('refuses to edit an invalid config.json, naming the problem and leaving it unchanged', async () => { const added = makeRepo(join(host.dir, 'added')); @@ -1038,6 +1108,6 @@ describe('configuration edits', () => { const from = client.mark(); expect((await client.hubRequest({ t: 'addRepo', host: 0, repo: added })).m).toEqual({ t: 'done', req: 1 }); await client.waitHost(0, (h) => h.repos.includes(added), { from }); - expect(presetsOf(client)).toEqual([[SHELL]]); + expect(presetsOf(client)).toEqual([DEFAULT_PRESETS]); }); }); diff --git a/test/process/hub-server.test.ts b/test/process/hub-server.test.ts index f78501c..e99a132 100644 --- a/test/process/hub-server.test.ts +++ b/test/process/hub-server.test.ts @@ -12,7 +12,7 @@ import { } from 'node:fs'; import { join } from 'node:path'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; -import { encodeWsData, MAX_FRAME, PROTOCOL_VERSION } from '../../src/protocol/index.js'; +import { encodeWsData, MAX_FRAME, BROWSER_PROTOCOL_VERSION } from '../../src/protocol/index.js'; import { sleep, type WtdProcess } from '../support/daemon-host.js'; import { UpgradeRefused, type HubClient, type HubMessageOf } from '../support/hub-client.js'; import { HubHost, type RawResponse } from '../support/hub-host.js'; @@ -563,7 +563,7 @@ describe('session handshake', () => { const client = await host.open(['wtd', `wtd.token.${token}`]); expect(await client.waitFor('hello')).toEqual({ t: 'hello', - protocol: PROTOCOL_VERSION, + protocol: BROWSER_PROTOCOL_VERSION, version: packageVersion(), instance: host.record()?.instance, }); @@ -604,18 +604,30 @@ describe('session handshake', () => { expect(client.messages.slice(from, from + 2).map((m) => m.t)).toEqual(['presets', 'hosts']); }); - it('sends the shell preset alone and no repos without a configuration file', async () => { + it('sends the default presets and no repos without a configuration file', async () => { rmSync(host.configPath); const client = await host.session(); - expect(presetsOf(client)).toEqual([[{ name: 'shell', command: null }]]); + expect(presetsOf(client)).toEqual([ + [ + { name: 'shell', command: null }, + { name: 'claude', command: 'claude' }, + { name: 'codex', command: 'codex' }, + ], + ]); expect(client.hosts()[0]?.repos).toEqual([]); expect(existsSync(host.configPath)).toBe(false); }); - it('sends the shell preset alone when the configuration has no presets', async () => { + it('sends the shell, claude and codex presets when the configuration has no presets', async () => { host.writeConfig({ port: host.port }); const client = await host.session(); - expect(presetsOf(client)).toEqual([[{ name: 'shell', command: null }]]); + expect(presetsOf(client)).toEqual([ + [ + { name: 'shell', command: null }, + { name: 'claude', command: 'claude' }, + { name: 'codex', command: 'codex' }, + ], + ]); }); it('sends exactly the configured presets in their order', async () => { @@ -646,7 +658,7 @@ describe('session handshake', () => { it('answers a hello of another protocol with version-mismatch and closes the session', async () => { const client = await host.open(['wtd', `wtd.token.${token}`]); await client.waitFor('hello'); - client.sendHello(PROTOCOL_VERSION + 1); + client.sendHello(BROWSER_PROTOCOL_VERSION + 1); expect(await client.waitFor('error')).toEqual({ t: 'error', req: null, diff --git a/test/process/install-local.test.ts b/test/process/install-local.test.ts index 1b301d2..8dbe3de 100644 --- a/test/process/install-local.test.ts +++ b/test/process/install-local.test.ts @@ -101,7 +101,11 @@ describe('wtd install-local', () => { } expect(readdirSync(join(dir, PTY_PACKAGE)).sort()).toEqual(['LICENSE', 'lib', 'package.json', 'prebuilds']); expect(readdirSync(join(dir, PTY_PACKAGE, 'prebuilds')).length).toBeGreaterThan(0); - expect(shim(host.home, ['--version'])).toEqual({ status: 0, stdout: `wtd ${version} (protocol 5)\n`, stderr: '' }); + expect(shim(host.home, ['--version'])).toEqual({ + status: 0, + stdout: `wtd ${version} (daemon protocol 6, browser protocol 8)\n`, + stderr: '', + }); }); it('writes the shim as an owner-only POSIX shell script running the installing Node by absolute path', async () => { @@ -146,7 +150,7 @@ describe('wtd install-local', () => { await shimDaemon(host.home); const pid = await waitUntil(() => daemonPidsOf(host.stateHome)[0], 'the shim daemon'); expect(readlinkSync(`/proc/${String(pid)}/exe`)).toBe(node); - expect(shim(host.home, ['--version']).stdout).toBe(`wtd ${packageVersion()} (protocol 5)\n`); + expect(shim(host.home, ['--version']).stdout).toBe(`wtd ${packageVersion()} (daemon protocol 6, browser protocol 8)\n`); }); }); @@ -175,7 +179,7 @@ describe('launcher', () => { expectInstalled(await installLocal([], { HOME: home })); const [program, ...args] = desktopExec(iniGroup(readFileSync(installPaths(home).launcher, 'utf8'), 'Desktop Entry').get('Exec') ?? ''); const result = spawnSync(program ?? '', [...args.slice(0, -1), '--version'], { env: host.env({ HOME: home }), encoding: 'utf8' }); - expect(result.stdout).toBe(`wtd ${packageVersion()} (protocol 5)\n`); + expect(result.stdout).toBe(`wtd ${packageVersion()} (daemon protocol 6, browser protocol 8)\n`); }); it('writes no unit file without --systemd', async () => { @@ -202,7 +206,7 @@ describe('--systemd', () => { ['--user', 'start', UNIT_NAME], ]); await waitUntil(() => canConnect(host.socket), 'the daemon the unit started', 10_000); - expect((await (await client()).waitFor('hello')).protocol).toBe(5); + expect((await (await client()).waitFor('hello')).protocol).toBe(6); }); it('writes an owner-only unit running the shim with daemon, restarting on failure, for default.target', async () => { @@ -310,7 +314,7 @@ describe('reinstalling', () => { const term = await after.create(repo, { command: 'echo after-reinstall; exec sleep 60' }); await after.attach(term.termId); await after.waitOutput(term.termId, 'after-reinstall'); - expect(shim(host.home, ['--version']).stdout).toBe(`wtd ${packageVersion()} (protocol 5)\n`); + expect(shim(host.home, ['--version']).stdout).toBe(`wtd ${packageVersion()} (daemon protocol 6, browser protocol 8)\n`); }); it('runs the newest release through the shim after a reinstall', async () => { diff --git a/test/process/install-remote.test.ts b/test/process/install-remote.test.ts index 91184fd..0864fb0 100644 --- a/test/process/install-remote.test.ts +++ b/test/process/install-remote.test.ts @@ -15,7 +15,7 @@ import { join } from 'node:path'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { hostPaths } from '../../src/platform/files/index.js'; import { installRemote, type InstalledRemote } from '../../src/platform/install/index.js'; -import { PROTOCOL_VERSION } from '../../src/protocol/index.js'; +import { DAEMON_PROTOCOL_VERSION } from '../../src/protocol/index.js'; import { DaemonClient } from '../support/daemon-client.js'; import { alive, BUNDLE, DaemonHost, waitUntil } from '../support/daemon-host.js'; import { REPO_ROOT } from '../support/exec.js'; @@ -73,7 +73,7 @@ const expectRemoteLayout = (remote: FakeRemote): void => { /** Asserts `alias` bridges to a daemon of this version run by the Node at `node`. */ const expectBridged = async (alias: string, remote: FakeRemote, node: string): Promise => { const hello = await remoteConnect(alias).waitFor('hello', () => true, { timeout: 15_000 }); - expect(hello.protocol).toBe(PROTOCOL_VERSION); + expect(hello.protocol).toBe(DAEMON_PROTOCOL_VERSION); expect(hello.version).toBe(packageVersion()); const [pid, ...others] = await waitUntil(() => { const pids = remote.daemonPids(); diff --git a/test/support/daemon-client.ts b/test/support/daemon-client.ts index 67f0669..d637586 100644 --- a/test/support/daemon-client.ts +++ b/test/support/daemon-client.ts @@ -10,7 +10,7 @@ import { encodeMessage, encodeStreamData, FrameKind, - PROTOCOL_VERSION, + DAEMON_PROTOCOL_VERSION, StreamDecoder, type MessageOf, type Terminal, @@ -199,7 +199,7 @@ export class DaemonClient { } /** Waits for the daemon's hello, then sends ours. */ - async handshake(protocol = PROTOCOL_VERSION): Promise> { + async handshake(protocol = DAEMON_PROTOCOL_VERSION): Promise> { const hello = await this.waitFor('hello'); this.send({ t: 'hello', protocol, version: '0.0.0-test', instance: 'test_client' }); return hello; @@ -234,12 +234,15 @@ export class DaemonClient { } /** Sends a request with the next req and resolves with its correlated reply. */ - async request(body: RequestBody, timeout = DEFAULT_TIMEOUT): Promise> { + async request( + body: RequestBody, + timeout = DEFAULT_TIMEOUT, + ): Promise> { const req = this.nextReq++; const from = this.messages.length; const message: unknown = { ...body, req }; this.sendJson(message); - return this.waitFor(['done', 'error', 'termCreated', 'reposDiscovered'], (m) => m.req === req, { from, timeout }); + return this.waitFor(['done', 'error', 'termCreated', 'reposDiscovered', 'worktreeAtRisk'], (m) => m.req === req, { from, timeout }); } /** A request expected to succeed with `done`. */ diff --git a/test/support/hub-client.ts b/test/support/hub-client.ts index c0954aa..258e78e 100644 --- a/test/support/hub-client.ts +++ b/test/support/hub-client.ts @@ -5,7 +5,7 @@ import { decodeWsData, encodeMessage, encodeWsData, - PROTOCOL_VERSION, + BROWSER_PROTOCOL_VERSION, type MessageOf, type Terminal, } from '../../src/protocol/index.js'; @@ -33,7 +33,7 @@ export interface ReceivedData { /** A reply correlated to a request: a daemon reply inside an envelope, or a hub-level reply. */ export type Reply = - | { from: 'daemon'; host: number; m: DaemonEventOf<'done' | 'error' | 'termCreated' | 'reposDiscovered'> } + | { from: 'daemon'; host: number; m: DaemonEventOf<'done' | 'error' | 'termCreated' | 'reposDiscovered' | 'worktreeAtRisk'> } | { from: 'hub'; m: HubMessageOf<'done' | 'error' | 'reposDiscovered'> }; /** Thrown when the hub answers an upgrade with an HTTP response instead of switching protocols. */ @@ -125,7 +125,7 @@ export class HubClient { } /** Waits for the hub's hello, sends ours, and waits for the first `hosts`. */ - async handshake(protocol = PROTOCOL_VERSION): Promise<{ hello: HubMessageOf<'hello'>; hosts: HubMessageOf<'hosts'> }> { + async handshake(protocol = BROWSER_PROTOCOL_VERSION): Promise<{ hello: HubMessageOf<'hello'>; hosts: HubMessageOf<'hosts'> }> { const hello = await this.waitFor('hello'); const from = this.mark(); this.sendHello(protocol); @@ -133,7 +133,7 @@ export class HubClient { return { hello, hosts }; } - sendHello(protocol = PROTOCOL_VERSION): void { + sendHello(protocol = BROWSER_PROTOCOL_VERSION): void { this.send({ t: 'hello', protocol, version: '0.0.0-test', instance: 'test_browser' }); } diff --git a/vite.config.ts b/vite.config.ts index a010b39..8609603 100644 --- a/vite.config.ts +++ b/vite.config.ts @@ -6,6 +6,9 @@ import { currentHostPaths } from './src/platform/files/index.js'; const LOGIN_PATH = '/login'; +/** The dev server's port: `WTD_VITE_PORT`, else 5173. */ +const devPort = (): number => Number(process.env['WTD_VITE_PORT'] ?? '5173'); + /** The running hub's port and token; `pnpm dev` needs a hub started by `wtd ui` or `wtd hub`. */ const runningHub = async (): Promise<{ port: number; token: string }> => { const paths = currentHostPaths(); @@ -14,30 +17,42 @@ const runningHub = async (): Promise<{ port: number; token: string }> => { return { port: record.port, token }; }; -/** Proxies `/api` and `/ws` to the hub, rewriting Host and Origin to the hub's own. */ +/** The dev page's own origin. */ +const devOrigin = (): string => `http://127.0.0.1:${String(devPort())}`; + +/** Responses the dev server sends may not be framed, so no other site can overlay the signed-in page. */ +const NO_FRAMES: Readonly> = { 'X-Frame-Options': 'DENY', 'Content-Security-Policy': "frame-ancestors 'none'" }; + +/** Proxies `/ws` to the hub with its Host; only the dev page's own Origin becomes the hub's, so other sites stay refused. */ const hubProxy = (port: number): Record => { const target = `http://127.0.0.1:${String(port)}`; - const options: ProxyOptions = { - target, - changeOrigin: true, - configure: (proxy) => { - proxy.on('proxyReq', (req) => { - if (req.getHeader('origin') !== undefined) req.setHeader('origin', target); - }); - proxy.on('proxyReqWs', (req) => { - req.setHeader('origin', target); - }); + return { + '/ws': { + target, + ws: true, + changeOrigin: true, + configure: (proxy) => { + proxy.on('proxyReqWs', (proxyReq, req) => { + if (req.headers.origin === devOrigin()) proxyReq.setHeader('origin', target); + }); + }, }, }; - return { '/api': options, '/ws': { ...options, ws: true } }; }; +/** Whether a request may sign in: anything but a request another site made, which browsers mark `cross-site` or `same-site`. */ +const loginAllowed = (site: string | string[] | undefined): boolean => site === undefined || site === 'none' || site === 'same-origin'; + /** `/login` gets a fresh one-time code from the hub and redirects to the page with it. */ const hubLogin = (port: number, token: string): Plugin => ({ name: 'wtd-hub-login', apply: 'serve', configureServer(server) { - server.middlewares.use(LOGIN_PATH, (_req, res: ServerResponse) => { + server.middlewares.use(LOGIN_PATH, (req, res: ServerResponse) => { + if (!loginAllowed(req.headers['sec-fetch-site'])) { + res.writeHead(403, { 'Content-Type': 'text/plain' }).end('sign in from the address bar, not from another site\n'); + return; + } requestCode(port, token).then( (code) => { res.writeHead(302, { Location: `/#code=${code}` }).end(); @@ -64,6 +79,6 @@ export default defineConfig(async ({ command }): Promise => { // esbuild's minifier keeps string literals quoted, so the version stays findable in the bundle. build: { outDir: '../../dist/web', emptyOutDir: true, minify: 'esbuild' }, plugins: hub === null ? [] : [hubLogin(hub.port, hub.token)], - server: hub === null ? {} : { host: '127.0.0.1', port: 5173, strictPort: true, proxy: hubProxy(hub.port) }, + server: hub === null ? {} : { host: '127.0.0.1', port: devPort(), strictPort: true, headers: NO_FRAMES, proxy: hubProxy(hub.port) }, }; }); From 838806a35ecf42d6eae84a5bcb27eab6dbf4286b Mon Sep 17 00:00:00 2001 From: Egor Kraev Date: Thu, 8 Oct 2026 14:07:43 +0200 Subject: [PATCH 3/6] Address review findings on worktree deletion, presets and the sidebar resizer - worktreeRisks counts from the worktree's current head and skips a dangling origin/HEAD - removeWorktree prunes whenever the directory is gone, not only when listed prunable - the detached-worktree note names the base the daemon compared with - README: wtd --version reports the installed CLI's protocol, not the running daemon's - split multi-call throw assertions and a composite assert in tests - Sonar: un-nest a ternary, use dataset, rename a catch parameter, suppress ARIA-splitter false positives --- README.md | 2 +- src/daemon/server/daemon.ts | 2 +- src/daemon/worktrees/git.test.ts | 28 ++++++++++++++++++++++++++- src/daemon/worktrees/git.ts | 16 +++++++++------ src/hub/config/editor.test.ts | 12 ++++++++---- src/web/ui/app.tsx | 10 +++++----- src/web/ui/sidebar.test.ts | 10 ++++++++-- src/web/ui/sidebar.ts | 4 ++-- src/web/ui/view.ts | 2 +- test/e2e/fixture.ts | 2 +- test/process/daemon-worktrees.test.ts | 3 ++- 11 files changed, 66 insertions(+), 25 deletions(-) diff --git a/README.md b/README.md index 4665d05..96f881f 100644 --- a/README.md +++ b/README.md @@ -49,7 +49,7 @@ wtd install-remote devbox --node /opt/node22/bin/node or with "Install" on the page while the host is down. The remote gets the same layout as `wtd install-local`, without launcher or unit. Its shim runs the Node found at install time, so `"$HOME/.local/bin/wtd" connect` works over plain `ssh` with no dotfile changes. -Each browser session holds one `ssh "$HOME/.local/bin/wtd" connect` per remote host, multiplexed over one SSH control master per host (`ControlPersist=10m`); a lost link reconnects to the same daemon and keeps its terminals. A host that keeps failing shows as down with SSH's last error line. sshd's `MaxSessions` (default 10) caps the sessions of one master: each open page uses one per host, plus a brief one for each "Add repo" or "Remove repo". When a host's daemon speaks an older daemon protocol (`wtd --version` prints it), the page offers "Restart daemon" (this machine) or "Reinstall & restart" (a remote host), naming the terminals that will be killed. `WTD_SSH` replaces the `ssh` program. +Each browser session holds one `ssh "$HOME/.local/bin/wtd" connect` per remote host, multiplexed over one SSH control master per host (`ControlPersist=10m`); a lost link reconnects to the same daemon and keeps its terminals. A host that keeps failing shows as down with SSH's last error line. sshd's `MaxSessions` (default 10) caps the sessions of one master: each open page uses one per host, plus a brief one for each "Add repo" or "Remove repo". When a host's running daemon speaks an older daemon protocol than its installed `wtd` (whose protocol `wtd --version` prints; it does not ask the running daemon), the page offers "Restart daemon" (this machine) or "Reinstall & restart" (a remote host), naming the terminals that will be killed. `WTD_SSH` replaces the `ssh` program. ## Keyboard diff --git a/src/daemon/server/daemon.ts b/src/daemon/server/daemon.ts index 1a73543..945f702 100644 --- a/src/daemon/server/daemon.ts +++ b/src/daemon/server/daemon.ts @@ -459,7 +459,7 @@ export class Daemon { this.removing.add(m.worktree); try { // Terminals still starting there count as running, and are closed with the rest. - await Promise.allSettled([...(this.spawnsIn.get(m.worktree) ?? [])]); + await Promise.allSettled([...(this.spawnsIn.get(m.worktree) ?? [])]); // NOSONAR(S7747) — await-thenable only recognises promise arrays return await this.removeWorktreeNow(peer, m); } finally { this.removing.delete(m.worktree); diff --git a/src/daemon/worktrees/git.test.ts b/src/daemon/worktrees/git.test.ts index 68799ca..fa2b188 100644 --- a/src/daemon/worktrees/git.test.ts +++ b/src/daemon/worktrees/git.test.ts @@ -64,6 +64,14 @@ describe('worktreeRisks', () => { expect((await worktreeRisks(repo, await listed(wt))).ahead).toBe(1); }); + it('falls back to origin/main when origin/HEAD names a missing ref', async () => { + withOrigin(); + git(repo, 'symbolic-ref', 'refs/remotes/origin/HEAD', 'refs/remotes/origin/gone'); + const wt = addWorktree(repo, join(root, 'wt'), 'feat'); + commit(wt, 'a'); + expect(await worktreeRisks(repo, await listed(wt))).toEqual({ base: 'origin/main', ahead: 1, changes: 0 }); + }); + it('finds nothing ahead once the branch is merged into origin/main', async () => { withOrigin(); const wt = addWorktree(repo, join(root, 'wt'), 'feat'); @@ -105,6 +113,14 @@ describe('worktreeRisks', () => { expect((await worktreeRisks(repo, await listed(wt))).changes).toBe(2); }); + it('counts commits made after the worktree was listed', async () => { + withOrigin(); + const wt = addWorktree(repo, join(root, 'wt'), 'feat'); + const cached = await listed(wt); + commit(wt, 'a'); + expect((await worktreeRisks(repo, cached)).ahead).toBe(1); + }); + it('reports no changes for a worktree whose directory is gone', async () => { withOrigin(); const wt = addWorktree(repo, join(root, 'wt'), 'feat'); @@ -125,7 +141,8 @@ describe('removeWorktree', () => { it('refuses a worktree with changes unless forced', async () => { const wt = addWorktree(repo, join(root, 'wt'), 'feat'); writeFileSync(join(wt, 'untracked'), 'new'); - await expect(removeWorktree(repo, await listed(wt), false)).rejects.toThrow(); + const changed = await listed(wt); + await expect(removeWorktree(repo, changed, false)).rejects.toThrow(); expect(existsSync(wt)).toBe(true); await removeWorktree(repo, await listed(wt), true); expect(existsSync(wt)).toBe(false); @@ -139,4 +156,13 @@ describe('removeWorktree', () => { await removeWorktree(repo, gone, false); expect((await listWorktrees(repo)).map((w) => w.path)).toEqual([repo]); }); + + it('prunes a worktree whose directory went after it was listed', async () => { + const wt = addWorktree(repo, join(root, 'wt'), 'feat'); + const cached = await listed(wt); + rmSync(wt, { recursive: true }); + expect(cached.prunable).toBe(false); + await removeWorktree(repo, cached, false); + expect((await listWorktrees(repo)).map((w) => w.path)).toEqual([repo]); + }); }); diff --git a/src/daemon/worktrees/git.ts b/src/daemon/worktrees/git.ts index 0cb1b05..fa7db94 100644 --- a/src/daemon/worktrees/git.ts +++ b/src/daemon/worktrees/git.ts @@ -83,11 +83,10 @@ const gitOrNull = async (cwd: string, args: readonly string[]): Promise => { const head = await gitOrNull(repo, ['symbolic-ref', '--quiet', 'refs/remotes/origin/HEAD']); - if (head !== null && head !== '') return head; - for (const candidate of BASE_CANDIDATES) { + for (const candidate of head === null || head === '' ? BASE_CANDIDATES : [head, ...BASE_CANDIDATES]) { if ((await gitOrNull(repo, ['rev-parse', '--verify', '--quiet', candidate])) !== null) return candidate; } return null; @@ -127,18 +126,23 @@ const isDirectory = async (path: string): Promise => { /** What deleting `worktree` of `repo` would lose: commits not in the base ref and uncommitted changes. */ export const worktreeRisks = async (repo: string, worktree: Worktree): Promise => { + // `worktree` may predate its latest commit. + const head = (await listWorktrees(repo)).find((w) => w.path === worktree.path)?.head ?? worktree.head; const ref = await baseRef(repo); if (ref !== null) await fetchBase(repo, ref); - const base = ref === null ? null : ref.slice(ref.startsWith(REMOTES_PREFIX) ? REMOTES_PREFIX.length : 0); + let base: string | null = null; let ahead: number | null = null; - if (ref !== null) ahead = worktree.head === null ? 0 : Number(await git(repo, ['rev-list', '--count', `${ref}..${worktree.head}`])); + if (ref !== null) { + base = ref.startsWith(REMOTES_PREFIX) ? ref.slice(REMOTES_PREFIX.length) : ref; + ahead = head === null ? 0 : Number(await git(repo, ['rev-list', '--count', `${ref}..${head}`])); + } const status = (await isDirectory(worktree.path)) ? await git(worktree.path, ['status', '--porcelain', '--untracked-files=normal']) : ''; return { base, ahead, changes: status.split('\n').filter((line) => line !== '').length }; }; /** Removes `worktree` of `repo`, `force` also discarding its changes; a worktree whose directory is gone is pruned. */ export const removeWorktree = async (repo: string, worktree: Worktree, force: boolean): Promise => { - if (worktree.prunable && !(await isDirectory(worktree.path))) { + if (!(await isDirectory(worktree.path))) { await git(repo, ['worktree', 'prune']); return; } diff --git a/src/hub/config/editor.test.ts b/src/hub/config/editor.test.ts index 0d98e28..a6b2ee0 100644 --- a/src/hub/config/editor.test.ts +++ b/src/hub/config/editor.test.ts @@ -171,13 +171,15 @@ describe('ConfigEditor editing presets', () => { it('refuses a name that is taken, leaving the file unchanged', async () => { write({ presets: [CLAUDE] }); const before = text(); - await expect(editor().addPreset({ name: 'claude', command: 'other' })).rejects.toThrow('a preset named claude exists'); + const edit = editor(); + await expect(edit.addPreset({ name: 'claude', command: 'other' })).rejects.toThrow('a preset named claude exists'); expect(text()).toBe(before); }); it('refuses a 65th preset', async () => { write({ presets: presetList(64) }); - await expect(editor().addPreset({ name: 'one more', command: null })).rejects.toBeInstanceOf(ConfigError); + const edit = editor(); + await expect(edit.addPreset({ name: 'one more', command: null })).rejects.toBeInstanceOf(ConfigError); expect(z.object({ presets: z.array(z.unknown()) }).parse(read()).presets).toHaveLength(64); }); @@ -201,13 +203,15 @@ describe('ConfigEditor editing presets', () => { it('refuses to remove the last preset', async () => { write({ presets: [CLAUDE] }); - await expect(editor().removePreset('claude')).rejects.toThrow('the last preset cannot be removed'); + const edit = editor(); + await expect(edit.removePreset('claude')).rejects.toThrow('the last preset cannot be removed'); expect(read()).toEqual({ presets: [CLAUDE] }); }); it('refuses to edit an invalid file', async () => { write({ presets: [] }); - await expect(editor().addPreset(SHELL)).rejects.toBeInstanceOf(ConfigError); + const edit = editor(); + await expect(edit.addPreset(SHELL)).rejects.toBeInstanceOf(ConfigError); }); it.each([ diff --git a/src/web/ui/app.tsx b/src/web/ui/app.tsx index f600095..ff443f2 100644 --- a/src/web/ui/app.tsx +++ b/src/web/ui/app.tsx @@ -704,7 +704,7 @@ const PresetAdder = ({ view, presets }: { view: View; presets: readonly Preset[] ); } return ( -
{ @@ -717,8 +717,8 @@ const PresetAdder = ({ view, presets }: { view: View; presets: readonly Preset[] setSending(true); view .addPreset(result.preset) - .then(close, (failure: unknown) => { - setError(failure instanceof Error ? failure.message : String(failure)); + .then(close, (error_: unknown) => { + setError(error_ instanceof Error ? error_.message : String(error_)); }) .finally(() => { setSending(false); @@ -933,10 +933,10 @@ const SIDEBAR_KEY_STEPS: Readonly> = { ArrowLeft: -16, Ar const SidebarResizer = ({ view }: { view: View }) => { const width = view.sidebarWidth.value; return ( -