From 519455ccf27063d39c6aa37c258252afa1e82710 Mon Sep 17 00:00:00 2001 From: Johannes Millan Date: Sat, 10 Oct 2026 10:13:29 +0200 Subject: [PATCH 1/2] feat(tasks): add persistent snooze modes to Later menu --- src/components/TaskTitleBar.client.test.tsx | 187 ++++++++++++++- src/components/TaskTitleBar.tsx | 143 +++++++----- src/components/chat/MoreMenu.css | 77 ++++++ src/components/chat/MoreMenu.tsx | 247 ++++++++++++++++---- src/components/chat/chat.css | 59 ----- src/store/autosave.test.ts | 3 +- src/store/autosave.ts | 1 + src/store/background-tasks.client.test.tsx | 238 ++++++++++++++++++- src/store/background-tasks.ts | 117 +++++++++- src/store/persistence.test.ts | 72 ++++++ src/store/persistence.ts | 12 + src/store/tasks.ts | 13 +- src/store/types.ts | 4 + 13 files changed, 992 insertions(+), 181 deletions(-) create mode 100644 src/components/chat/MoreMenu.css diff --git a/src/components/TaskTitleBar.client.test.tsx b/src/components/TaskTitleBar.client.test.tsx index f19ea7443..6ccb446f2 100644 --- a/src/components/TaskTitleBar.client.test.tsx +++ b/src/components/TaskTitleBar.client.test.tsx @@ -4,6 +4,12 @@ import { reconcile } from 'solid-js/store'; import { render } from 'solid-js/web'; import { TaskTitleBar } from './TaskTitleBar'; import { store, setStore } from '../store/core'; +import { + bringTaskToFront, + getTaskSnoozedUntil, + isTaskBackgrounded, +} from '../store/background-tasks'; +import { invoke } from '../lib/ipc'; import type { Task } from '../store/types'; vi.mock('../lib/ipc', () => ({ invoke: vi.fn(), fireAndForget: vi.fn() })); @@ -29,6 +35,9 @@ const [pushSuccess, setPushSuccess] = createSignal(false); const onFinish = vi.fn(); beforeEach(() => { + vi.mocked(invoke).mockResolvedValue(undefined); + bringTaskToFront('one'); + setStore('collapsedTaskOrder', []); setStore('tasks', reconcile({ one: { ...task } })); setStore('taskOrder', ['one']); setStore('focusMode', false); @@ -90,9 +99,185 @@ it('exposes canvas and focus as named toggles', () => { }); it('keeps plain actions free of toggle state', () => { - for (const name of ['Send task to back until new activity', 'Collapse task', 'Close task']) { + for (const name of ['Later', 'Close task']) { const b = button(name); expect(b).toBeDefined(); expect(b?.hasAttribute('aria-pressed')).toBe(false); } }); + +const menuItem = (label: string) => + [...document.querySelectorAll('[role="menuitem"]')].find( + (item) => item.textContent?.trim() === label, + ); + +it('groups snooze and minimize, and offers restore for a backgrounded task', () => { + expect(button('Collapse task')).toBeUndefined(); + expect(button('Send task to back until new activity')).toBeUndefined(); + button('Later')?.click(); + expect(document.querySelector('[role="menu"]')?.getAttribute('aria-label')).toBe('Later'); + expect(menuItem('Minimize (stop agents)')).toBeDefined(); + menuItem('Snooze until new activity')?.click(); + expect(isTaskBackgrounded('one')).toBe(true); + expect(document.querySelector('[role="menu"]')).toBeNull(); + + button('Later')?.click(); + menuItem('Restore to front')?.click(); + expect(isTaskBackgrounded('one')).toBe(false); +}); + +it('minimizes through the existing collapse action', async () => { + button('Later')?.click(); + menuItem('Minimize (stop agents)')?.click(); + await vi.waitFor(() => expect(store.tasks.one.collapsed).toBe(true)); + expect(store.collapsedTaskOrder).toEqual(['one']); +}); + +it.each([{ coordinatorMode: true }, { delegationParent: true }, { coordinatedBy: 'parent' }])( + 'does not offer minimize for managed tasks: %j', + (fields) => { + setStore('tasks', 'one', fields); + button('Later')?.click(); + expect(menuItem('Minimize (stop agents)')).toBeUndefined(); + expect(menuItem('Snooze until new activity')).toBeDefined(); + }, +); + +it('dismisses Later with Escape or an outside click', () => { + const trigger = button('Later'); + trigger?.focus(); + trigger?.click(); + window.dispatchEvent(new KeyboardEvent('keydown', { key: 'Escape', bubbles: true })); + expect(trigger?.getAttribute('aria-expanded')).toBe('false'); + expect(document.activeElement).toBe(trigger); + + trigger?.click(); + document.querySelector('.chat-menu-backdrop')?.click(); + expect(trigger?.getAttribute('aria-expanded')).toBe('false'); +}); + +it('orders task decisions, view controls, and close', () => { + const labels = [...container.querySelectorAll('.task-title-actions button')].map( + (item) => item.getAttribute('aria-label') ?? item.textContent?.trim(), + ); + expect(labels).toEqual(['Finish', 'Later', 'Open canvas', 'Focus on this task', 'Close task']); +}); + +it('offers running snooze presets directly in a submenu without a dialog', () => { + button('Later')?.click(); + const mode = menuItem('Snooze · keep running'); + expect(mode?.getAttribute('aria-haspopup')).toBe('menu'); + mode?.click(); + expect(document.querySelector('[role="dialog"]')).toBeNull(); + expect(document.querySelectorAll('[role="menu"]')).toHaveLength(2); + expect( + [...document.querySelectorAll('[role="menu"][aria-label="Snooze · keep running"] button')].map( + (item) => item.textContent?.trim(), + ), + ).toEqual(['15 minutes', '30 minutes', '1 hour', '2 hours', '4 hours', '8 hours']); + const now = Date.now(); + menuItem('15 minutes')?.click(); + expect(getTaskSnoozedUntil('one')).toBeGreaterThanOrEqual(now + 0.25 * 3_600_000); + expect(store.tasks.one.collapsed).not.toBe(true); + expect(container.textContent).toContain('Snoozed until'); + expect(document.querySelector('[role="menu"]')).toBeNull(); +}); + +it('offers longer presets for stopping agents while snoozed', async () => { + button('Later')?.click(); + menuItem('Snooze · stop agents')?.click(); + expect( + [...document.querySelectorAll('[role="menu"][aria-label="Snooze · stop agents"] button')].map( + (item) => item.textContent?.trim(), + ), + ).toEqual(['1 hour', '4 hours', '8 hours', '1 day', '3 days', '1 week']); + const now = Date.now(); + menuItem('3 days')?.click(); + await vi.waitFor(() => expect(store.tasks.one.collapsed).toBe(true)); + expect(store.tasks.one.snoozedUntil).toBeGreaterThanOrEqual(now + 72 * 3_600_000); + expect(document.querySelector('[role="dialog"]')).toBeNull(); +}); + +it('disables the stop-agents submenu for coordinated tasks', () => { + setStore('tasks', 'one', 'coordinatorMode', true); + button('Later')?.click(); + const mode = menuItem('Snooze · stop agents'); + expect(mode?.disabled).toBe(true); + expect(mode?.title).toBe('Coordinated tasks need to keep their agents running.'); + mode?.click(); + expect(document.querySelectorAll('[role="menu"]')).toHaveLength(1); +}); + +it('opens presets on hover and switches between the two modes', () => { + button('Later')?.click(); + menuItem('Snooze · keep running')?.dispatchEvent(new MouseEvent('mouseenter')); + expect(menuItem('4 hours')).toBeDefined(); + menuItem('Snooze · stop agents')?.dispatchEvent(new MouseEvent('mouseenter')); + expect(document.querySelector('[role="menu"][aria-label="Snooze · keep running"]')).toBeNull(); + expect(document.querySelector('[role="menu"][aria-label="Snooze · stop agents"]')).not.toBeNull(); + menuItem('Minimize (stop agents)')?.dispatchEvent(new MouseEvent('mouseenter')); + expect(document.querySelectorAll('[role="menu"]')).toHaveLength(1); +}); + +it('navigates the submenu with arrow keys and returns focus with Escape', async () => { + const trigger = button('Later'); + trigger?.focus(); + trigger?.click(); + const mode = menuItem('Snooze · keep running'); + mode?.focus(); + const key = (value: string) => + window.dispatchEvent( + new KeyboardEvent('keydown', { key: value, bubbles: true, cancelable: true }), + ); + key('ArrowRight'); + await Promise.resolve(); + expect(document.activeElement).toBe(menuItem('15 minutes')); + key('ArrowDown'); + expect(document.activeElement).toBe(menuItem('30 minutes')); + key('ArrowLeft'); + expect(document.activeElement).toBe(mode); + expect(document.querySelectorAll('[role="menu"]')).toHaveLength(1); + key('ArrowRight'); + await Promise.resolve(); + key('Escape'); + expect(document.activeElement).toBe(mode); + key('Escape'); + expect(document.activeElement).toBe(trigger); + expect(document.querySelector('[role="menu"]')).toBeNull(); +}); + +it('opens the submenu to the left and keeps it above the bottom edge when space is tight', () => { + button('Later')?.click(); + const mode = menuItem('Snooze · keep running'); + if (!mode) throw new Error('Missing snooze mode'); + const anchor = new DOMRect(window.innerWidth - 230, window.innerHeight - 90, 210, 30); + const rect = vi.spyOn(mode, 'getBoundingClientRect').mockReturnValue(anchor); + try { + mode.click(); + const submenu = document.querySelector( + '[role="menu"][aria-label="Snooze · keep running"]', + ); + expect(submenu).not.toBeNull(); + expect(parseFloat(submenu?.style.left ?? '')).toBeLessThan(anchor.left); + expect(parseFloat(submenu?.style.top ?? '')).toBeLessThan(anchor.top); + expect( + parseFloat(submenu?.style.top ?? '') + parseFloat(submenu?.style.maxHeight ?? ''), + ).toBeLessThanOrEqual(window.innerHeight - 12); + } finally { + rect.mockRestore(); + } +}); + +it.each([ + ['Snooze · keep running', '8 hours', 8], + ['Snooze · stop agents', '1 week', 168], +] as const)('offers the longest preset for %s', async (mode, label, hours) => { + button('Later')?.click(); + menuItem(mode)?.click(); + const now = Date.now(); + menuItem(label)?.click(); + await vi.waitFor(() => + expect(store.tasks.one.snoozedUntil).toBeGreaterThanOrEqual(now + hours * 3_600_000), + ); + expect(store.tasks.one.collapsed === true).toBe(mode === 'Snooze · stop agents'); +}); diff --git a/src/components/TaskTitleBar.tsx b/src/components/TaskTitleBar.tsx index 4a957728f..bceb34670 100644 --- a/src/components/TaskTitleBar.tsx +++ b/src/components/TaskTitleBar.tsx @@ -17,8 +17,9 @@ import { } from '../store/store'; import { EditableText, type EditableTextHandle } from './EditableText'; import { IconButton } from './IconButton'; +import { MoreMenu } from './chat/MoreMenu'; import { StatusDot, getDotTooltip } from './StatusDot'; -import { CloseIcon } from './icons'; +import { ActivityIcon, ClockIcon, CloseIcon, PlayIcon, StopIcon, UndoIcon } from './icons'; import { theme } from '../lib/theme'; import { badgeStyle } from '../lib/badgeStyle'; import { taskCheckSignal } from '../lib/task-check-signal'; @@ -27,7 +28,13 @@ import { getTaskDockerBadgeLabel } from '../lib/docker'; import { displayTaskNameFromPrompt, shouldUsePromptDerivedTaskName } from '../lib/clean-task-name'; import type { Task } from '../store/types'; import { isLandedTaskState } from '../store/landing'; -import { bringTaskToFront, isTaskBackgrounded, sendTaskToBack } from '../store/background-tasks'; +import { + bringTaskToFront, + getTaskSnoozedUntil, + isTaskBackgrounded, + sendTaskToBack, + snoozeTask, +} from '../store/background-tasks'; interface TaskTitleBarProps { task: Task; @@ -41,6 +48,24 @@ interface TaskTitleBarProps { } export function TaskTitleBar(props: TaskTitleBarProps) { + const canStopAgents = () => + !props.task.coordinatorMode && !props.task.delegationParent && !props.task.coordinatedBy; + const snoozeChoices = (keepRunning: boolean) => + (keepRunning ? [0.25, 0.5, 1, 2, 4, 8] : [1, 4, 8, 24, 72, 168]).map((hours) => ({ + label: + hours < 1 + ? `${hours * 60} minutes` + : hours === 1 + ? '1 hour' + : hours === 168 + ? '1 week' + : hours >= 24 + ? `${hours / 24} ${hours === 24 ? 'day' : 'days'}` + : `${hours} hours`, + icon: , + run: () => void snoozeTask(props.task.id, hours, keepRunning), + })); + const snoozedUntil = () => getTaskSnoozedUntil(props.task.id); const dockerBadgeLabel = () => getTaskDockerBadgeLabel(props.task.dockerSource); const isLandedTask = () => isLandedTaskState(props.task.landingState); const landingBadge = () => { @@ -166,7 +191,11 @@ export function TaskTitleBar(props: TaskTitleBarProps) { Imported - Background + + {snoozedUntil() === undefined + ? 'Background' + : `Snoozed until ${new Date(snoozedUntil() ?? 0).toLocaleString([], { weekday: 'short', hour: '2-digit', minute: '2-digit' })}`} +
- -
+
+ -
- + + } + class="task-finish-btn" + items={[ + { + label: isTaskBackgrounded(props.task.id) + ? 'Restore to front' + : 'Snooze until new activity', + icon: isTaskBackgrounded(props.task.id) ? ( + + ) : ( + + ), + run: () => + isTaskBackgrounded(props.task.id) + ? bringTaskToFront(props.task.id) + : sendTaskToBack(props.task.id), + }, + { + label: 'Snooze · keep running', + icon: , + children: snoozeChoices(true), + }, + { + label: 'Snooze · stop agents', + icon: , + children: snoozeChoices(false), + disabled: !canStopAgents(), + title: canStopAgents() + ? undefined + : 'Coordinated tasks need to keep their agents running.', + }, + ...(canStopAgents() + ? [ + { + label: 'Minimize (stop agents)', + icon: ( + + ), + run: () => void collapseTask(props.task.id), + }, + ] + : []), + ]} + /> +
-
- - - - - } - onClick={() => - isTaskBackgrounded(props.task.id) - ? bringTaskToFront(props.task.id) - : sendTaskToBack(props.task.id) - } - title={ - isTaskBackgrounded(props.task.id) - ? 'Bring task to front' - : 'Send task to back until new activity' - } - /> - - - - - } - onClick={() => collapseTask(props.task.id)} - title="Collapse task" - /> - +
} onClick={() => props.onClose()} title="Close task" />
diff --git a/src/components/chat/MoreMenu.css b/src/components/chat/MoreMenu.css new file mode 100644 index 000000000..f4742801b --- /dev/null +++ b/src/components/chat/MoreMenu.css @@ -0,0 +1,77 @@ +/* The menu lives in document.body, outside the chat's variables, so it uses the + app's own theme colors. */ +.chat-menu-backdrop { + position: fixed; + inset: 0; + z-index: 1200; +} +.chat-menu { + position: fixed; + display: flex; + flex-direction: column; + gap: 1px; + padding: 4px; + background: var(--bg-elevated); + border: 1px solid var(--border); + border-radius: var(--radius-md); + box-shadow: 0 6px 20px #0006; + overflow-y: auto; +} +.chat-menu > * { + /* A long history scrolls instead of squeezing its entries. */ + flex-shrink: 0; +} +.chat-menu-group { + padding: 6px 8px 2px; + color: var(--fg-muted); + font-size: 11px; +} +.chat-menu button { + display: flex; + align-items: center; + justify-content: space-between; + gap: 12px; + padding: 6px 8px; + color: var(--fg); + font: inherit; + font-size: 12px; + text-align: left; + background: transparent; + border: 0; + border-radius: var(--radius-sm); + cursor: pointer; +} +.chat-menu button:enabled:hover, +.chat-menu button:focus-visible, +.chat-menu button[aria-expanded='true'] { + background: var(--bg-hover); +} +.chat-menu button:disabled { + color: var(--fg-muted); + cursor: default; +} +.chat-menu-caption { + display: flex; + align-items: center; + gap: 8px; + min-width: 0; +} +.chat-menu-icon { + display: inline-flex; + flex-shrink: 0; + width: 14px; + height: 14px; + color: var(--fg-muted); +} +.chat-menu button > svg { + flex-shrink: 0; +} +.chat-menu-label { + overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; +} +.chat-menu kbd { + color: var(--fg-muted); + font: inherit; +} diff --git a/src/components/chat/MoreMenu.tsx b/src/components/chat/MoreMenu.tsx index 757edadfb..b4e36ef2d 100644 --- a/src/components/chat/MoreMenu.tsx +++ b/src/components/chat/MoreMenu.tsx @@ -1,46 +1,184 @@ -import { For, Show, createSignal, onCleanup, onMount } from 'solid-js'; +import { For, Show, createSignal, onCleanup, onMount, type JSX } from 'solid-js'; import { Portal } from 'solid-js/web'; -import { KebabIcon } from '../icons'; +import { ChevronRightIcon, KebabIcon } from '../icons'; +import './MoreMenu.css'; import { createAnchorEffect, placeBelow, type BelowAnchor } from '../../lib/floating'; -export interface MoreMenuItem { +interface MenuEntry { label: string; + icon?: JSX.Element; + title?: string; /** A keyboard shortcut shown beside the label. */ hint?: string; disabled?: boolean; /** Items sharing a group are listed under it as a heading. */ group?: string; - run: () => void; +} + +type MenuAction = MenuEntry & { run: () => void; children?: never }; +type Submenu = MenuEntry & { children: MenuAction[]; run?: never }; +export type MoreMenuItem = MenuAction | Submenu; + +function MenuButtons(props: { + items: MoreMenuItem[]; + expanded?: MoreMenuItem; + onSelect: (item: MoreMenuItem, button: HTMLButtonElement) => void; + onHover?: (item: MoreMenuItem, button: HTMLButtonElement) => void; +}) { + return ( + + {(item, index) => ( + <> + + + + + + )} + + ); } const MENU_WIDTH = 220; -/** Roughly the height of one entry and of the menu's padding, from chat.css. */ +/** Roughly the height of one entry and of the menu's padding, from MoreMenu.css. */ const ITEM_HEIGHT = 27; const MENU_PADDING = 10; -function MenuList(props: { items: MoreMenuItem[]; position: BelowAnchor; onClose: () => void }) { +function MenuList(props: { + items: MoreMenuItem[]; + position: BelowAnchor; + label: string; + onClose: () => void; +}) { let menu: HTMLDivElement | undefined; - const entries = () => - Array.from(menu?.querySelectorAll('[role="menuitem"]:not(:disabled)') ?? []); + let submenu: HTMLDivElement | undefined; + const [branch, setBranch] = createSignal<{ item: Submenu; button: HTMLButtonElement }>(); + const [branchPosition, setBranchPosition] = createSignal({ top: 0, left: 0, maxHeight: 0 }); + const entries = (panel = menu) => + Array.from( + panel?.querySelectorAll('[role="menuitem"]:not(:disabled)') ?? [], + ); + + createAnchorEffect( + () => Boolean(branch()), + () => { + const current = branch(); + if (!current) return; + const anchor = current.button.getBoundingClientRect(); + const height = current.item.children.length * ITEM_HEIGHT + MENU_PADDING; + const left = + anchor.right + MENU_WIDTH + 12 <= window.innerWidth + ? anchor.right + : anchor.left - MENU_WIDTH; + const top = Math.max(12, Math.min(anchor.top, window.innerHeight - height - 12)); + setBranchPosition({ + left: Math.max(12, Math.min(left, window.innerWidth - MENU_WIDTH - 12)), + top, + maxHeight: Math.max(0, window.innerHeight - top - 12), + }); + }, + ); + + function openBranch(item: MoreMenuItem, button: HTMLButtonElement, focus: boolean) { + if (!item.children || item.disabled) { + setBranch(undefined); + return; + } + setBranch({ item, button }); + if (focus) + queueMicrotask(() => { + if (submenu?.isConnected && button.getAttribute('aria-expanded') === 'true') { + entries(submenu)[0]?.focus(); + } + }); + } + + function select(item: MoreMenuItem, button: HTMLButtonElement) { + if (item.children) { + openBranch(item, button, true); + return; + } + // Restore trigger focus before the action is allowed to move it elsewhere. + props.onClose(); + item.run(); + } + onMount(() => { // The button that opened the menu gets the focus back when it goes. const opener = document.activeElement; - requestAnimationFrame(() => entries()[0]?.focus()); + const frame = requestAnimationFrame(() => entries()[0]?.focus()); const onKey = (event: KeyboardEvent) => { - if (event.key === 'Escape') { - event.stopPropagation(); + if (event.key === 'Tab') { props.onClose(); return; } + if (event.key === 'Escape' || (event.key === 'ArrowLeft' && branch())) { + event.preventDefault(); + event.stopPropagation(); + const current = branch(); + if (current) { + setBranch(undefined); + current.button.focus(); + } else props.onClose(); + return; + } + if (event.key === 'ArrowRight') { + const focused = document.activeElement; + if ( + focused instanceof HTMLButtonElement && + menu?.contains(focused) && + focused.getAttribute('aria-haspopup') === 'menu' + ) { + event.preventDefault(); + event.stopPropagation(); + focused.click(); + } + return; + } if (event.key !== 'ArrowDown' && event.key !== 'ArrowUp') return; event.preventDefault(); - const list = entries(); + const inSubmenu = submenu?.contains(document.activeElement); + const list = entries(inSubmenu ? submenu : menu); + if (!inSubmenu) setBranch(undefined); const at = list.indexOf(document.activeElement as HTMLButtonElement); const step = event.key === 'ArrowDown' ? 1 : -1; list[(at + step + list.length) % list.length]?.focus(); }; window.addEventListener('keydown', onKey, true); onCleanup(() => { + cancelAnimationFrame(frame); window.removeEventListener('keydown', onKey, true); if (opener instanceof HTMLElement && opener.isConnected) opener.focus(); }); @@ -52,7 +190,7 @@ function MenuList(props: { items: MoreMenuItem[]; position: BelowAnchor; onClose ref={menu} class="chat-menu" role="menu" - aria-label="More actions" + aria-label={props.label} onClick={(event) => event.stopPropagation()} style={{ top: `${props.position.top}px`, @@ -61,41 +199,44 @@ function MenuList(props: { items: MoreMenuItem[]; position: BelowAnchor; onClose 'max-height': `${props.position.maxHeight}px`, }} > - - {(item, index) => ( - <> - - - - - - )} - + openBranch(item, button, false)} + />
+ + {(current) => ( + + )} + ); } -/** A "⋯" button holding the conversation actions that are needed only now and then. */ -export function MoreMenu(props: { items: MoreMenuItem[] }) { +/** An action menu with an optional labelled trigger. */ +export function MoreMenu(props: { + items: MoreMenuItem[]; + label?: string; + icon?: JSX.Element; + class?: string; +}) { let button: HTMLButtonElement | undefined; const [open, setOpen] = createSignal(false); const [position, setPosition] = createSignal({ top: 0, right: 0, maxHeight: 0 }); @@ -110,16 +251,28 @@ export function MoreMenu(props: { items: MoreMenuItem[] }) { <> - setOpen(false)} /> + setOpen(false)} + /> ); diff --git a/src/components/chat/chat.css b/src/components/chat/chat.css index a17e4731f..a34cb205e 100644 --- a/src/components/chat/chat.css +++ b/src/components/chat/chat.css @@ -984,65 +984,6 @@ padding: 0; color: var(--fg-subtle); } -/* The menu lives in document.body, outside the chat's variables, so it uses the - app's own theme colors. */ -.chat-menu-backdrop { - position: fixed; - inset: 0; - z-index: 1200; -} -.chat-menu { - position: fixed; - display: flex; - flex-direction: column; - gap: 1px; - padding: 4px; - background: var(--bg-elevated); - border: 1px solid var(--border); - border-radius: var(--radius-md); - box-shadow: 0 6px 20px #0006; - overflow-y: auto; -} -.chat-menu > * { - /* A long history scrolls instead of squeezing its entries. */ - flex-shrink: 0; -} -.chat-menu-group { - padding: 6px 8px 2px; - color: var(--fg-muted); - font-size: 11px; -} -.chat-menu button { - display: flex; - justify-content: space-between; - gap: 12px; - padding: 6px 8px; - color: var(--fg); - font: inherit; - font-size: 12px; - text-align: left; - background: transparent; - border: 0; - border-radius: var(--radius-sm); - cursor: pointer; -} -.chat-menu button:enabled:hover, -.chat-menu button:focus-visible { - background: var(--bg-hover); -} -.chat-menu button:disabled { - color: var(--fg-muted); - cursor: default; -} -.chat-menu-label { - overflow: hidden; - text-overflow: ellipsis; - white-space: nowrap; -} -.chat-menu kbd { - color: var(--fg-muted); - font: inherit; -} .chat-context-chips { display: flex; flex-wrap: wrap; diff --git a/src/store/autosave.test.ts b/src/store/autosave.test.ts index fc95ddd75..b3b094751 100644 --- a/src/store/autosave.test.ts +++ b/src/store/autosave.test.ts @@ -154,6 +154,7 @@ describe('autosave snapshot includes new-task-default fields', () => { 'autoSendChildUpdates', 'propagateSkipPermissions', 'maxConcurrentTasks', + 'snoozedUntil', ] as const)('%s changes the snapshot', (field) => { const taskId = 'autosave-draft-task'; const task: Task = { @@ -174,7 +175,7 @@ describe('autosave snapshot includes new-task-default fields', () => { const before = persistedSnapshot(); if (field === 'promptHistory') { setStore('tasks', taskId, 'promptHistory', [{ text: 'Repeated prompt', sentAt: 1 }]); - } else if (field === 'maxConcurrentTasks') { + } else if (field === 'maxConcurrentTasks' || field === 'snoozedUntil') { setStore('tasks', taskId, field, 5); } else if ( field === 'autoMergeChildren' || diff --git a/src/store/autosave.ts b/src/store/autosave.ts index 55154d043..f29357bbf 100644 --- a/src/store/autosave.ts +++ b/src/store/autosave.ts @@ -83,6 +83,7 @@ function structuralSnapshot(): string { externalWorktree: t.externalWorktree, savedInitialPrompt: t.savedInitialPrompt, collapsed: t.collapsed, + snoozedUntil: t.snoozedUntil, agentSessionIds: t.agentSessionIds, savedAgentSessionIds: t.savedAgentSessionIds, coordinatedBy: t.coordinatedBy, diff --git a/src/store/background-tasks.client.test.tsx b/src/store/background-tasks.client.test.tsx index c440c9ff1..956bc7d86 100644 --- a/src/store/background-tasks.client.test.tsx +++ b/src/store/background-tasks.client.test.tsx @@ -5,13 +5,18 @@ import { TaskTitleBar } from '../components/TaskTitleBar'; import { store, setStore } from './core'; import { AGENT_HOOK_STALE_MS, applyAgentHookEvent, getAgentHookStatus } from './agentHookStatus'; import { clearAgentActivity, getTaskAttentionState, markAgentBusy } from './taskStatus'; +import { invoke } from '../lib/ipc'; +import { IPC } from '../../electron/ipc/channels'; +import { collapseTask, uncollapseTask } from './tasks'; import { setActiveTask } from './navigation'; import { removePrChecks, setPrChecks, type PrChecksState } from './pr-checks-state'; import { computeAttentionEntries } from './sidebar-attention'; import { bringTaskToFront, + getTaskSnoozedUntil, isTaskBackgrounded, sendTaskToBack, + snoozeTask, startBackgroundTaskWatcher, } from './background-tasks'; import type { Task } from './types'; @@ -40,7 +45,9 @@ function hook(state: 'working' | 'waiting' | 'done', event: string, taskId = 'on let stop: () => void; beforeEach(() => { vi.useFakeTimers(); + vi.mocked(invoke).mockClear().mockResolvedValue(undefined); setStore('taskProjectFilter', null); + setStore('focusMode', false); setStore('tasks', reconcile({ one: task('one'), two: task('two'), three: task('three') })); setStore('agents', reconcile({})); setStore('taskGitStatus', reconcile({})); @@ -340,17 +347,20 @@ it('offers the action and a manual return in the task header', () => { container, ); try { - const send = container.querySelector( - 'button[title="Send task to back until new activity"]', + const doLater = container.querySelector('button[aria-label="Later"]'); + doLater?.click(); + const send = [...document.querySelectorAll('[role="menuitem"]')].find( + (item) => item.textContent === 'Snooze until new activity', ); - expect(send).not.toBeNull(); + expect(send).toBeDefined(); send?.click(); expect(isTaskBackgrounded('one')).toBe(true); expect(container.textContent).toContain('Background'); - const restore = container.querySelector( - 'button[title="Bring task to front"]', + doLater?.click(); + const restore = [...document.querySelectorAll('[role="menuitem"]')].find( + (item) => item.textContent === 'Restore to front', ); - expect(restore).not.toBeNull(); + expect(restore).toBeDefined(); restore?.click(); expect(isTaskBackgrounded('one')).toBe(false); expect(container.textContent).not.toContain('Background'); @@ -388,3 +398,219 @@ it('keeps the selected project when sending its active task to the back', () => expect(store.activeTaskId).toBe('three'); expect(store.taskProjectFilter).toBe('project'); }); + +it('keeps timed snooze through new activity and restores at its deadline without taking focus', () => { + const now = Date.now(); + sendTaskToBack('one', 2); + expect(getTaskSnoozedUntil('one')).toBe(now + 2 * 3_600_000); + hook('waiting', 'PermissionRequest'); + setStore('tasks', 'one', 'needsReview', true); + expect(isTaskBackgrounded('one')).toBe(true); + vi.advanceTimersByTime(2 * 3_600_000 - 1); + expect(isTaskBackgrounded('one')).toBe(true); + vi.advanceTimersByTime(1); + expect(isTaskBackgrounded('one')).toBe(false); + expect(getTaskSnoozedUntil('one')).toBeUndefined(); + expect(store.taskOrder[0]).toBe('one'); + expect(store.activeTaskId).toBe('two'); +}); + +it('replaces a snooze deadline and supports early manual restore', () => { + sendTaskToBack('one', 1); + vi.advanceTimersByTime(30 * 60_000); + sendTaskToBack('one', 2); + vi.advanceTimersByTime(30 * 60_000); + expect(isTaskBackgrounded('one')).toBe(true); + bringTaskToFront('one'); + sendTaskToBack('one'); + vi.advanceTimersByTime(2 * 3_600_000); + expect(isTaskBackgrounded('one')).toBe(true); + expect(getTaskSnoozedUntil('one')).toBeUndefined(); +}); + +it('ends timed snooze when the task is selected', () => { + sendTaskToBack('one', 1); + setActiveTask('one'); + expect(isTaskBackgrounded('one')).toBe(false); + expect(getTaskSnoozedUntil('one')).toBeUndefined(); +}); + +it('reschedules the whole snoozed coordinator group from a child', () => { + setStore('tasks', 'two', 'coordinatedBy', 'one'); + sendTaskToBack('one', 1); + sendTaskToBack('two', 2); + expect(getTaskSnoozedUntil('one')).toBe(getTaskSnoozedUntil('two')); + hook('waiting', 'PermissionRequest', 'two'); + vi.advanceTimersByTime(3_600_000); + expect(isTaskBackgrounded('two')).toBe(true); + vi.advanceTimersByTime(3_600_000); + expect(store.taskOrder).toEqual(['one', 'two', 'three']); + expect(isTaskBackgrounded('two')).toBe(false); +}); + +it('expires an overdue snooze after the clock jumps forward', () => { + sendTaskToBack('one', 1); + vi.setSystemTime(Date.now() + 2 * 3_600_000); + vi.advanceTimersByTime(60_000); + expect(isTaskBackgrounded('one')).toBe(false); +}); + +it('cancels scheduled work when the watcher is disposed', () => { + sendTaskToBack('one', 1); + stop(); + vi.advanceTimersByTime(3_600_000); + expect(isTaskBackgrounded('one')).toBe(true); +}); + +it.each([0, -1, 0.001, 169, Infinity, NaN])('rejects invalid snooze hours: %s', (hours) => { + sendTaskToBack('one', hours); + expect(isTaskBackgrounded('one')).toBe(false); + expect(store.taskOrder).toEqual(['one', 'two', 'three']); +}); + +it('rebuilds a saved snooze and retains the original deadline after restart', () => { + stop(); + const deadline = Date.now() + 30 * 60_000; + setStore('tasks', 'one', 'snoozedUntil', deadline); + setStore('taskOrder', ['two', 'three', 'one']); + setActiveTask('two'); + stop = startBackgroundTaskWatcher(); + expect(getTaskSnoozedUntil('one')).toBe(deadline); + hook('waiting', 'PermissionRequest'); + expect(isTaskBackgrounded('one')).toBe(true); + vi.advanceTimersByTime(30 * 60_000); + expect(store.taskOrder[0]).toBe('one'); + expect(store.activeTaskId).toBe('two'); + expect(store.tasks.one.snoozedUntil).toBeUndefined(); +}); + +it('returns a task on startup when its saved deadline passed while the app was closed', () => { + stop(); + setStore('tasks', 'one', 'snoozedUntil', Date.now() - 60_000); + setStore('taskOrder', ['two', 'three', 'one']); + setActiveTask('two'); + stop = startBackgroundTaskWatcher(); + vi.advanceTimersByTime(0); + expect(store.taskOrder[0]).toBe('one'); + expect(isTaskBackgrounded('one')).toBe(false); + expect(store.tasks.one.snoozedUntil).toBeUndefined(); + expect(store.activeTaskId).toBe('two'); +}); + +it('does not cancel a restored snooze through the saved active selection', () => { + stop(); + setStore('taskOrder', ['one']); + setStore('focusMode', true); + setStore('tasks', 'one', 'snoozedUntil', Date.now() + 3_600_000); + stop = startBackgroundTaskWatcher(); + expect(isTaskBackgrounded('one')).toBe(true); + expect(store.activeTaskId).toBeNull(); + expect(store.activeAgentId).toBeNull(); + expect(store.focusMode).toBe(false); +}); + +it('restores coordinator snooze ownership even when a child was the saved selection', () => { + stop(); + setStore('tasks', 'two', 'coordinatedBy', 'one'); + const deadline = Date.now() + 3_600_000; + setStore('tasks', 'one', 'snoozedUntil', deadline); + setActiveTask('two'); + stop = startBackgroundTaskWatcher(); + expect(getTaskSnoozedUntil('two')).toBe(deadline); + expect(store.activeTaskId).toBe('three'); + bringTaskToFront('two'); + expect(store.tasks.one.snoozedUntil).toBeUndefined(); +}); + +it('clears the durable deadline when switching to activity-based backgrounding', () => { + sendTaskToBack('one', 2); + expect(store.tasks.one.snoozedUntil).toBe(getTaskSnoozedUntil('one')); + sendTaskToBack('one'); + expect(store.tasks.one.snoozedUntil).toBeUndefined(); + expect(isTaskBackgrounded('one')).toBe(true); +}); + +it('stops agents for an unchecked snooze and resumes the saved session without taking focus', async () => { + const sessionId = 'fb4f2bc6-62d9-4b29-a795-240caf2fc459'; + setStore('tasks', 'one', 'agentSessionIds', { 'one-agent': sessionId }); + await snoozeTask('one', 1, false); + expect(invoke).toHaveBeenCalledWith(IPC.KillAgent, { agentId: 'one-agent' }); + expect(store.tasks.one.collapsed).toBe(true); + expect(store.tasks.one.agentIds).toEqual([]); + expect(store.tasks.one.savedAgentSessionIds).toEqual([sessionId]); + expect(isTaskBackgrounded('one')).toBe(true); + const selectedTask = store.activeTaskId; + const selectedAgent = store.activeAgentId; + setStore('taskProjectFilter', 'other'); + vi.advanceTimersByTime(3_600_000); + expect(store.tasks.one.collapsed).toBe(false); + const resumedId = store.tasks.one.agentIds[0]; + expect(store.agents[resumedId].resumed).toBe(true); + expect(store.tasks.one.agentSessionIds?.[resumedId]).toBe(sessionId); + expect(store.taskOrder[0]).toBe('one'); + expect(store.activeTaskId).toBe(selectedTask); + expect(store.activeAgentId).toBe(selectedAgent); + expect(store.taskProjectFilter).toBe('other'); +}); + +it('allows early manual restoration of a stopped snooze and cancels its timer', async () => { + await snoozeTask('one', 1, false); + uncollapseTask('one'); + expect(store.tasks.one.snoozedUntil).toBeUndefined(); + expect(isTaskBackgrounded('one')).toBe(false); + const agentIds = [...store.tasks.one.agentIds]; + vi.advanceTimersByTime(3_600_000); + expect(store.tasks.one.agentIds).toEqual(agentIds); +}); + +it('restores a stopped snooze after restarting the app', () => { + stop(); + const def = store.agents['one-agent'].def; + setStore('tasks', 'one', { + collapsed: true, + agentIds: [], + savedAgentDefs: [def], + snoozedUntil: Date.now() + 3_600_000, + }); + setStore('taskOrder', ['two', 'three']); + setStore('collapsedTaskOrder', ['one']); + setActiveTask('two'); + stop = startBackgroundTaskWatcher(); + vi.advanceTimersByTime(3_600_000 - 1); + expect(store.tasks.one.collapsed).toBe(true); + vi.advanceTimersByTime(1); + expect(store.tasks.one.collapsed).toBe(false); + expect(store.tasks.one.agentIds).toHaveLength(1); + expect(store.activeTaskId).toBe('two'); +}); + +it('does not resurrect a stopped snooze that is being closed', async () => { + await snoozeTask('one', 1, false); + setStore('tasks', 'one', 'closingStatus', 'removing'); + vi.advanceTimersByTime(3_600_000); + expect(store.tasks.one.collapsed).toBe(true); + expect(store.taskOrder).not.toContain('one'); + expect(isTaskBackgrounded('one')).toBe(false); +}); + +it.each([{ coordinatorMode: true }, { delegationParent: true }, { coordinatedBy: 'parent' }])( + 'does not stop coordinator-managed tasks: %j', + async (fields) => { + setStore('tasks', 'one', fields); + await snoozeTask('one', 1, false); + expect(store.tasks.one.collapsed).not.toBe(true); + expect(invoke).not.toHaveBeenCalledWith(IPC.KillAgent, expect.anything()); + expect(isTaskBackgrounded('one')).toBe(false); + }, +); + +it('cancels a running snooze when the user explicitly minimizes the task', async () => { + sendTaskToBack('one', 1); + await collapseTask('one'); + expect(store.tasks.one.snoozedUntil).toBeUndefined(); + expect(isTaskBackgrounded('one')).toBe(false); + vi.advanceTimersByTime(3_600_000); + expect(store.tasks.one.collapsed).toBe(true); + expect(store.tasks.one.agentIds).toEqual([]); + expect(store.taskOrder).not.toContain('one'); +}); diff --git a/src/store/background-tasks.ts b/src/store/background-tasks.ts index 166203aa5..4a7d57ccf 100644 --- a/src/store/background-tasks.ts +++ b/src/store/background-tasks.ts @@ -1,18 +1,28 @@ import { matchesTaskProjectFilter } from './task-project-filter'; -import { batch, createEffect, createRoot, createSignal, untrack } from 'solid-js'; +import { batch, createEffect, createRoot, createSignal, onCleanup, untrack } from 'solid-js'; import { store, setStore } from './core'; import { AGENT_HOOK_STALE_MS, getAgentHookStatus, type AgentHookStatus } from './agentHookStatus'; import { scrollTaskIntoView } from './focused-panel'; import { setActiveTask } from './navigation'; +import { collapseTask, uncollapseTask } from './tasks'; import { getPrChecks } from './pr-checks-state'; import { getCoordinatorChildren } from './sidebar-order'; import { getTaskOpenQuestions, isAgentIdle, type TaskOpenQuestion } from './taskStatus'; -// Session-only: restarting the app ends the running agents this baseline describes. -const [backgroundTasks, setBackgroundTasks] = createSignal>( +// Activity baselines are session-only. Timed snoozes are also saved on the task. +const [backgroundTasks, setBackgroundTasks] = createSignal>( new Map(), ); +interface BackgroundTask extends ActivitySnapshot { + snoozedUntil?: number; +} + +export function getTaskSnoozedUntil(taskId: string): number | undefined { + const owner = backgroundOwner(taskId); + return owner ? backgroundTasks().get(owner)?.snoozedUntil : undefined; +} + export function isTaskBackgrounded(taskId: string): boolean { return backgroundOwner(taskId) !== undefined; } @@ -137,19 +147,52 @@ function keepFocusAcrossReorder(): void { }); } -export function sendTaskToBack(taskId: string): void { +function validSnoozeHours(hours: number): boolean { + return Number.isFinite(hours) && hours >= 0.01 && hours <= 168; +} + +export async function snoozeTask(taskId: string, hours: number, keepRunning = true): Promise { + if (!validSnoozeHours(hours)) return; + taskId = backgroundOwner(taskId) ?? taskId; + const task = store.tasks[taskId]; + if (!task || task.collapsed || task.closingStatus || !store.taskOrder.includes(taskId)) return; + if (keepRunning) { + sendTaskToBack(taskId, hours); + return; + } + // The coordinator registry owns its agents; collapse cannot safely replace them. + if (task.coordinatorMode || task.delegationParent || task.coordinatedBy) return; + const snoozedUntil = Date.now() + hours * 3_600_000; + await collapseTask(taskId); + if (!store.tasks[taskId]?.collapsed) return; + batch(() => { + setStore('tasks', taskId, 'snoozedUntil', snoozedUntil); + setBackgroundTasks((previous) => + new Map(previous).set(taskId, { ...activitySnapshot(taskId), snoozedUntil }), + ); + }); +} + +export function sendTaskToBack(taskId: string, snoozeHours?: number): void { + if (snoozeHours !== undefined && !validSnoozeHours(snoozeHours)) return; + taskId = backgroundOwner(taskId) ?? taskId; const task = store.tasks[taskId]; if (!task || task.collapsed || task.closingStatus || !store.taskOrder.includes(taskId)) return; const block = taskBlock(taskId); const remaining = store.taskOrder.filter((id) => !block.includes(id)); - const snapshot = activitySnapshot(taskId); + const snapshot: BackgroundTask = { + ...activitySnapshot(taskId), + snoozedUntil: snoozeHours === undefined ? undefined : Date.now() + snoozeHours * 3_600_000, + }; const neighbor = foregroundNeighbor(taskId, block); batch(() => { + for (const id of block) setStore('tasks', id, 'snoozedUntil', undefined); setBackgroundTasks((previous) => { const next = new Map(previous); for (const id of block) next.delete(id); return next.set(taskId, snapshot); }); + setStore('tasks', taskId, 'snoozedUntil', snapshot.snoozedUntil); setStore('taskOrder', [...remaining, ...block]); if (store.activeTaskId && block.includes(store.activeTaskId)) { if (neighbor) setActiveTask(neighbor); @@ -164,13 +207,19 @@ export function sendTaskToBack(taskId: string): void { export function bringTaskToFront(taskId: string): void { const owner = backgroundOwner(taskId); if (!owner) return; + const restorePaused = + store.tasks[owner]?.snoozedUntil !== undefined && + !store.tasks[owner]?.closingStatus && + store.collapsedTaskOrder.includes(owner); batch(() => { + if (store.tasks[owner]) setStore('tasks', owner, 'snoozedUntil', undefined); setBackgroundTasks((previous) => { const next = new Map(previous); next.delete(owner); return next; }); - if (!store.taskOrder.includes(owner) || store.tasks[owner]?.collapsed) return; + if (restorePaused && store.tasks[owner]?.collapsed) uncollapseTask(owner, { activate: false }); + if (!store.taskOrder.includes(owner)) return; const block = taskBlock(owner); keepFocusAcrossReorder(); setStore('taskOrder', [...block, ...store.taskOrder.filter((id) => !block.includes(id))]); @@ -184,20 +233,66 @@ export function bringTaskToFront(taskId: string): void { * changes order only; it never takes focus from the task the user is working on. */ export function startBackgroundTaskWatcher(): () => void { return createRoot((dispose) => { + // loadState restores task/agent sessions first. Rebuild only timed snoozes; + // untimed activity baselines belong to the previous run. + batch(() => { + const restored = new Map(backgroundTasks()); + for (const taskId of [...store.taskOrder, ...store.collapsedTaskOrder]) { + const task = store.tasks[taskId]; + if (task?.snoozedUntil !== undefined) { + restored.set(taskId, { ...activitySnapshot(taskId), snoozedUntil: task.snoozedUntil }); + } + } + setBackgroundTasks(restored); + // A saved/fallback selection must not immediately cancel a restored snooze. + if (store.activeTaskId && isTaskBackgrounded(store.activeTaskId)) { + const foreground = store.taskOrder.find( + (id) => !isTaskBackgrounded(id) && matchesTaskProjectFilter(id), + ); + if (foreground) setActiveTask(foreground); + else { + setStore('activeTaskId', null); + setStore('activeAgentId', null); + setStore('focusMode', false); + } + } + }); + createEffect(() => { + const deadlines = [...backgroundTasks().values()].flatMap((task) => + task.snoozedUntil === undefined ? [] : [task.snoozedUntil], + ); + if (!deadlines.length) return; + // Recheck wall time periodically, including after sleep or a clock change. + const timer = setTimeout( + () => { + for (const [taskId, task] of backgroundTasks()) { + if (task.snoozedUntil !== undefined && task.snoozedUntil <= Date.now()) { + bringTaskToFront(taskId); + } + } + setBackgroundTasks((tasks) => new Map(tasks)); + }, + Math.max(0, Math.min(60_000, Math.min(...deadlines) - Date.now())), + ); + onCleanup(() => clearTimeout(timer)); + }); createEffect(() => { for (const [taskId, baseline] of backgroundTasks()) { const task = store.tasks[taskId]; - const current = activitySnapshot(taskId); - const change = activityChange(baseline, current); + const current = baseline.snoozedUntil === undefined ? activitySnapshot(taskId) : undefined; + const change = current ? activityChange(baseline, current) : 'same'; if ( !task || - task.collapsed || - !store.taskOrder.includes(taskId) || + task.closingStatus || + (task.collapsed + ? baseline.snoozedUntil === undefined || !store.collapsedTaskOrder.includes(taskId) + : !store.taskOrder.includes(taskId)) || + (baseline.snoozedUntil !== undefined && task.snoozedUntil === undefined) || taskBlock(taskId).includes(store.activeTaskId ?? '') || change === 'new' ) { untrack(() => bringTaskToFront(taskId)); - } else if (change === 'rebaseline') { + } else if (change === 'rebaseline' && current) { untrack(() => setBackgroundTasks((previous) => new Map(previous).set(taskId, current))); } } diff --git a/src/store/persistence.test.ts b/src/store/persistence.test.ts index 5a81ad0c1..67c18f1c1 100644 --- a/src/store/persistence.test.ts +++ b/src/store/persistence.test.ts @@ -2343,3 +2343,75 @@ it.each([false, true])( expect(saved.tasks['task-1'].secondOpinionDismissed).toBe(true); }, ); + +describe('timed snooze persistence', () => { + function savedSnooze(snoozedUntil: unknown): string { + return JSON.stringify({ + projects: [{ id: 'project-1', name: 'Repo', path: '/repo', color: 'hsl(0, 70%, 75%)' }], + taskOrder: ['task-1'], + tasks: { + 'task-1': { + ...persistedTask(agentDef()), + agentIds: ['agent-1'], + agentSessionIds: { 'agent-1': 'fb4f2bc6-62d9-4b29-a795-240caf2fc459' }, + snoozedUntil, + }, + }, + activeTaskId: null, + }); + } + + it.each([-60_000, 3_600_000])( + 'round-trips an overdue or future deadline (%s)', + async (offset) => { + const deadline = Date.now() + offset; + mockInvoke.mockResolvedValueOnce(savedSnooze(deadline)); + await loadState(); + expect(store.tasks['task-1'].snoozedUntil).toBe(deadline); + expect(store.tasks['task-1'].agentSessionIds).toEqual({ + 'agent-1': 'fb4f2bc6-62d9-4b29-a795-240caf2fc459', + }); + expect(store.agents['agent-1'].resumed).toBe(true); + await saveState(); + const savedCall = mockInvoke.mock.calls.findLast(([channel]) => channel === IPC.SaveAppState); + const saved = JSON.parse(savedCall?.[1].json); + expect(saved.tasks['task-1'].snoozedUntil).toBe(deadline); + + mockInvoke.mockResolvedValueOnce(JSON.stringify(saved)); + await loadState(); + expect(store.tasks['task-1'].snoozedUntil).toBe(deadline); + }, + ); + + it('round-trips a stopped snooze without starting agents before its deadline', async () => { + const deadline = Date.now() + 3_600_000; + const state = JSON.parse(savedSnooze(deadline)); + state.taskOrder = []; + state.collapsedTaskOrder = ['task-1']; + state.tasks['task-1'].collapsed = true; + state.tasks['task-1'].savedAgentSessionIds = ['fb4f2bc6-62d9-4b29-a795-240caf2fc459']; + mockInvoke.mockResolvedValueOnce(JSON.stringify(state)); + await loadState(); + expect(store.tasks['task-1'].collapsed).toBe(true); + expect(store.tasks['task-1'].snoozedUntil).toBe(deadline); + expect(store.tasks['task-1'].agentIds).toEqual([]); + expect(store.tasks['task-1'].savedAgentDefs).toHaveLength(1); + await saveState(); + const savedCall = mockInvoke.mock.calls.findLast(([channel]) => channel === IPC.SaveAppState); + const saved = JSON.parse(savedCall?.[1].json); + expect(saved.tasks['task-1'].snoozedUntil).toBe(deadline); + expect(saved.tasks['task-1'].savedAgentSessionIds).toEqual([ + 'fb4f2bc6-62d9-4b29-a795-240caf2fc459', + ]); + mockInvoke.mockResolvedValueOnce(JSON.stringify(saved)); + await loadState(); + expect(store.tasks['task-1'].snoozedUntil).toBe(deadline); + expect(store.tasks['task-1'].agentIds).toEqual([]); + }); + + it.each([null, 'tomorrow', -1, 0, 1e20])('ignores invalid saved deadlines: %s', async (value) => { + mockInvoke.mockResolvedValueOnce(savedSnooze(value)); + await loadState(); + expect(store.tasks['task-1'].snoozedUntil).toBeUndefined(); + }); +}); diff --git a/src/store/persistence.ts b/src/store/persistence.ts index b63af94ac..43f58fe0d 100644 --- a/src/store/persistence.ts +++ b/src/store/persistence.ts @@ -251,6 +251,15 @@ function validBranch(value: unknown, exclude?: string): string | undefined { return typeof value === 'string' && value.length > 0 && value !== exclude ? value : undefined; } +function restoredSnoozedUntil(value: unknown): number | undefined { + return typeof value === 'number' && + Number.isFinite(value) && + value > 0 && + value <= 8_640_000_000_000_000 + ? value + : undefined; +} + /** * Serialize a Task to its persisted shape. The caller supplies agentDefs * because active and collapsed tasks source them differently (live store @@ -309,6 +318,7 @@ function toPersistedTask(task: Task, agentDefs: AgentDef[], collapsed?: boolean) stepsEnabled: task.stepsEnabled, branchAdoptedFrom: task.branchAdoptedFrom, branchOfferDismissed: task.branchOfferDismissed, + snoozedUntil: task.snoozedUntil, ...(collapsed ? { collapsed: true } : {}), delegationParent: task.delegationParent, delegationPaused: task.delegationPaused, @@ -1019,6 +1029,7 @@ export async function loadState(): Promise { claudeChatSessionId: typeof pt.claudeChatSessionId === 'string' ? pt.claudeChatSessionId : undefined, shellAgentIds, + snoozedUntil: restoredSnoozedUntil(pt.snoozedUntil), notes: pt.notes, promptDraft: typeof pt.promptDraft === 'string' ? pt.promptDraft : undefined, browserUrl: typeof pt.browserUrl === 'string' ? pt.browserUrl : undefined, @@ -1199,6 +1210,7 @@ export async function loadState(): Promise { branchAdoptedFrom: validBranch(pt.branchAdoptedFrom, pt.branchName), branchOfferDismissed: validBranch(pt.branchOfferDismissed), collapsed: true, + snoozedUntil: restoredSnoozedUntil(pt.snoozedUntil), savedAgentSessionIds: Array.isArray(pt.savedAgentSessionIds) ? agentDefs.map((_, index) => { const id = pt.savedAgentSessionIds?.[index]; diff --git a/src/store/tasks.ts b/src/store/tasks.ts index 33b041d92..37d9f100a 100644 --- a/src/store/tasks.ts +++ b/src/store/tasks.ts @@ -1060,6 +1060,7 @@ export async function collapseTask(taskId: string): Promise { produce((s) => { if (!s.tasks[taskId]) return; s.tasks[taskId].collapsed = true; + s.tasks[taskId].snoozedUntil = undefined; s.tasks[taskId].savedAgentDef = agentDefs[0]; s.tasks[taskId].savedAgentDefs = agentDefs.length > 0 ? agentDefs : undefined; s.tasks[taskId].savedAgentSessionIds = savedAgentIds.map( @@ -1100,7 +1101,8 @@ export async function collapseTask(taskId: string): Promise { rescheduleTaskStatusPolling(); } -export function uncollapseTask(taskId: string): void { +export function uncollapseTask(taskId: string, options: { activate?: boolean } = {}): void { + const activate = options.activate ?? true; const task = store.tasks[taskId]; if (!task || !task.collapsed) return; @@ -1118,10 +1120,13 @@ export function uncollapseTask(taskId: string): void { produce((s) => { const t = s.tasks[taskId]; t.collapsed = false; + t.snoozedUntil = undefined; s.collapsedTaskOrder = s.collapsedTaskOrder.filter((id) => id !== taskId); s.taskOrder.push(taskId); - if (s.taskProjectFilter && s.taskProjectFilter !== t.projectId) s.taskProjectFilter = null; - s.activeTaskId = taskId; + if (activate) { + if (s.taskProjectFilter && s.taskProjectFilter !== t.projectId) s.taskProjectFilter = null; + s.activeTaskId = taskId; + } for (let i = 0; i < restoredAgents.length; i++) { const { id: agentId, def } = restoredAgents[i]; @@ -1153,7 +1158,7 @@ export function uncollapseTask(taskId: string): void { t.savedAgentSessionIds = undefined; t.savedSelectedAgentIndex = undefined; t.savedPromptedAgentIndexes = undefined; - s.activeAgentId = t.selectedAgentId ?? null; + if (activate) s.activeAgentId = t.selectedAgentId ?? null; }), ); diff --git a/src/store/types.ts b/src/store/types.ts index 9a968a83a..21a5648ad 100644 --- a/src/store/types.ts +++ b/src/store/types.ts @@ -254,6 +254,8 @@ export interface Task { githubUrl?: string; prUrl?: string; superProductivity?: SuperProductivityLink; + /** Absolute return time for a timed snooze, retained across app restarts. */ + snoozedUntil?: number; collapsed?: boolean; savedAgentDef?: AgentDef; savedAgentDefs?: AgentDef[]; @@ -387,6 +389,8 @@ export interface PersistedTask { superProductivity?: unknown; savedInitialPrompt?: string; secondOpinionDismissed?: boolean; + /** Absolute return time for a timed snooze, retained across app restarts. */ + snoozedUntil?: number; collapsed?: boolean; savedAgentSessionIds?: (string | null)[]; savedSelectedAgentIndex?: number; From 6fbe173d57c7a8d53a71440a5f4cefbcd66c2b18 Mon Sep 17 00:00:00 2001 From: Johannes Millan Date: Sat, 10 Oct 2026 11:10:52 +0200 Subject: [PATCH 2/2] feat(finish): merge through the task's open PR When a task has an open PR, Finish merges it on GitHub with a method picker; a local merge stays available. A merged PR turns the footer into Close task. Closed PRs, PRs on another branch, and PRs that fail to load fall back to a local merge. Fix CI and Address review show when relevant. PR logic is shared with PullRequestDialog via pr-actions.ts. --- src/components/FinishDialog.client.test.tsx | 219 +++++++++++++++++- src/components/FinishDialog.tsx | 163 ++++++------- src/components/FinishPrSection.tsx | 244 ++++++++++++++++++++ src/components/PullRequestDialog.tsx | 141 +++-------- src/components/TaskPanel.tsx | 4 + src/components/pr-actions.ts | 232 +++++++++++++++++++ src/lib/theme.ts | 11 + 7 files changed, 824 insertions(+), 190 deletions(-) create mode 100644 src/components/FinishPrSection.tsx create mode 100644 src/components/pr-actions.ts diff --git a/src/components/FinishDialog.client.test.tsx b/src/components/FinishDialog.client.test.tsx index 7683bcd25..64927da1a 100644 --- a/src/components/FinishDialog.client.test.tsx +++ b/src/components/FinishDialog.client.test.tsx @@ -4,8 +4,14 @@ import { afterEach, describe, expect, it, onTestFinished, vi } from 'vitest'; import { FinishDialog } from './FinishDialog'; import { invoke } from '../lib/ipc'; import { getPrChecks, mergeTask, pushTask } from '../store/store'; +import { + getPullRequestDetails, + mergePullRequestForTask, + stageFailedChecksPrompt, +} from '../store/github'; import { IPC } from '../../electron/ipc/channels'; import type { ChangeTourController } from '../lib/create-change-tour'; +import type { PullRequestDetails } from '../ipc/types'; import type { Task } from '../store/types'; vi.mock('../store/store', () => ({ @@ -26,7 +32,7 @@ vi.mock('../lib/ipc', () => ({ invoke: vi.fn(async (channel: string) => { if (channel === IPC.GetBranchLog) return ''; if (channel === IPC.CheckMergeStatus) return { conflicting_files: [], main_ahead_count: 0 }; - return { current_branch: 'task/parent', has_committed_changes: true }; + return { current_branch: 'task/parent', has_committed_changes: true, head_sha: 'local' }; }), Channel: class { onmessage: ((text: string) => void) | null = null; @@ -72,6 +78,13 @@ vi.mock('./EvidencePanel', () => ({ ), })); vi.mock('../store/tasks', () => ({ sendPrompt: vi.fn() })); +vi.mock('../store/notification', () => ({ showNotification: vi.fn() })); +vi.mock('../store/github', () => ({ + getPullRequestDetails: vi.fn(), + mergePullRequestForTask: vi.fn(), + stageFailedChecksPrompt: vi.fn(), + stageReviewFeedbackPrompt: vi.fn(), +})); vi.mock('./DiffViewerDialog', async () => { const { ReviewProvider, useReview } = await import('./ReviewProvider'); function ReviewEditor() { @@ -126,7 +139,12 @@ vi.mock('./DiffViewerDialog', async () => { }; }); vi.mock('./merge-readiness', () => ({ buildMergeReadiness: () => ({ checks: [] }) })); -vi.mock('../lib/theme', () => ({ theme: {}, bannerStyle: () => ({}) })); +vi.mock('../lib/theme', () => ({ + theme: {}, + bannerStyle: () => ({}), + dialogButtonStyle: () => ({}), + linkButtonStyle: {}, +})); let dispose: (() => void) | undefined; afterEach(() => { @@ -167,6 +185,7 @@ function tour(stops = 0): ChangeTourController { function mount(options: { task?: Task; tour?: ChangeTourController } = {}) { const handlers = { onClose: vi.fn(), + onCloseTask: vi.fn(), onOpenPullRequest: vi.fn(), onPushDone: vi.fn(), onRegenerateTour: vi.fn(), @@ -356,7 +375,7 @@ describe('FinishDialog GitHub', () => { it('shows an empty state without linking an issue as a PR', () => { mount({ task: task({ githubUrl: 'https://github.com/o/r/issues/2' }) }); expect(document.body.textContent).toContain('No pull request detected'); - expect(button('PR #2 · Details and actions…')).toBeUndefined(); + expect(button('PR #2 details…')).toBeUndefined(); }); it.each(['prUrl', 'githubUrl'] as const)( @@ -376,11 +395,199 @@ describe('FinishDialog GitHub', () => { const url = 'https://github.com/o/r/pull/42'; const handlers = mount({ task: task({ [field]: url }) }); expect(document.body.textContent).toContain('Unit tests: fail'); - expect(document.body.textContent).toContain('Draft pull request'); - expect(document.body.textContent).toContain('Review: changes requested'); + expect(document.body.textContent).toContain('Draft · Review: changes requested'); expect(document.body.textContent).toContain('PR has conflicts'); - button('PR #42 · Details and actions…')?.click(); + button('PR #42 details…')?.click(); expect(handlers.onOpenPullRequest).toHaveBeenCalledWith(url); }, ); }); + +const PR_URL = 'https://github.com/o/r/pull/42'; + +function prDetails(overrides: Partial = {}): PullRequestDetails { + return { + number: 42, + title: 'Parent', + url: PR_URL, + state: 'OPEN', + isDraft: false, + mergeable: 'MERGEABLE', + mergeStateStatus: 'CLEAN', + baseRefName: 'main', + headRefName: 'task/parent', + headRefOid: 'local', + mergeMethods: ['squash', 'merge'], + ...overrides, + }; +} + +describe('FinishDialog with an open PR', () => { + it('merges the PR on GitHub, then offers to close the task', async () => { + vi.mocked(getPullRequestDetails).mockResolvedValueOnce(prDetails()); + vi.mocked(getPullRequestDetails).mockResolvedValueOnce(prDetails({ state: 'MERGED' })); + vi.mocked(mergePullRequestForTask).mockResolvedValueOnce(true); + const handlers = mount({ task: task({ prUrl: PR_URL }) }); + await flush(); + expect(confirmButton()?.textContent).toBe('Squash and merge PR #42'); + // Local merge options would describe a merge that will not happen. + expect(document.body.textContent).not.toContain('Squash commits'); + confirmButton()?.click(); + await flush(); + await flush(); + expect(mergePullRequestForTask).toHaveBeenCalledWith('parent', { + prUrl: PR_URL, + method: 'squash', + headSha: 'local', + }); + expect(mergeTask).not.toHaveBeenCalled(); + expect(handlers.onClose).not.toHaveBeenCalled(); + expect(confirmButton()?.textContent).toBe('Close task'); + confirmButton()?.click(); + expect(handlers.onClose).toHaveBeenCalled(); + expect(handlers.onCloseTask).toHaveBeenCalled(); + }); + + it('blocks the PR merge while the PR lacks the latest local commit', async () => { + vi.mocked(getPullRequestDetails).mockResolvedValue(prDetails({ headRefOid: 'older' })); + mount({ task: task({ prUrl: PR_URL }) }); + await flush(); + expect(confirmButton()?.disabled).toBe(true); + expect(document.querySelector('[data-testid="footer-note"]')?.textContent).toContain( + 'Push the branch first', + ); + }); + + it('can merge locally instead, and switch back', async () => { + vi.mocked(getPullRequestDetails).mockResolvedValue(prDetails()); + mount({ task: task({ prUrl: PR_URL }) }); + await flush(); + button('Merge locally instead')?.click(); + expect(confirmButton()?.textContent).toBe('Merge into main'); + button('Use PR #42 instead')?.click(); + expect(confirmButton()?.textContent).toBe('Squash and merge PR #42'); + button('Merge locally instead')?.click(); + confirmButton()?.click(); + expect(mergeTask).toHaveBeenCalled(); + expect(mergePullRequestForTask).not.toHaveBeenCalled(); + }); + + it('falls back to a local merge when the PR cannot be loaded', async () => { + vi.mocked(getPullRequestDetails).mockRejectedValue(new Error('gh: not logged in')); + mount({ task: task({ prUrl: PR_URL }) }); + await flush(); + await flush(); + expect(confirmButton()?.textContent).toBe('Merge into main'); + expect(document.body.textContent).toContain('gh: not logged in'); + }); + + it('stages a Fix CI prompt and closes so it can be reviewed', async () => { + vi.mocked(getPullRequestDetails).mockResolvedValue(prDetails()); + vi.mocked(stageFailedChecksPrompt).mockResolvedValueOnce(true); + vi.mocked(getPrChecks).mockReturnValue({ + overall: 'failure', + passing: 0, + pending: 0, + failing: 1, + checks: [{ name: 'Unit tests', bucket: 'fail' }], + checkedAt: '', + }); + const handlers = mount({ task: task({ prUrl: PR_URL }) }); + await flush(); + button('Fix CI')?.click(); + await flush(); + expect(stageFailedChecksPrompt).toHaveBeenCalledWith('parent', { number: 42, url: PR_URL }); + expect(handlers.onClose).toHaveBeenCalled(); + }); + + it('stays on Close task when the refetch after a merge fails', async () => { + vi.mocked(getPullRequestDetails).mockResolvedValueOnce(prDetails()); + vi.mocked(getPullRequestDetails).mockRejectedValueOnce(new Error('rate limited')); + vi.mocked(mergePullRequestForTask).mockResolvedValueOnce(true); + mount({ task: task({ prUrl: PR_URL }) }); + await flush(); + confirmButton()?.click(); + await flush(); + await flush(); + expect(confirmButton()?.textContent).toBe('Close task'); + expect(button('Merge locally instead')).toBeUndefined(); + expect(document.body.textContent).not.toContain('merges locally'); + }); + + it('closes when GitHub only queued the merge', async () => { + vi.mocked(getPullRequestDetails).mockResolvedValue(prDetails()); + vi.mocked(mergePullRequestForTask).mockResolvedValueOnce(false); + const handlers = mount({ task: task({ prUrl: PR_URL }) }); + await flush(); + confirmButton()?.click(); + await flush(); + expect(handlers.onClose).toHaveBeenCalled(); + expect(handlers.onCloseTask).not.toHaveBeenCalled(); + }); + + it('keeps the dialog open with the error when the PR merge fails', async () => { + vi.mocked(getPullRequestDetails).mockResolvedValue(prDetails()); + vi.mocked(mergePullRequestForTask).mockRejectedValueOnce(new Error('head moved')); + const handlers = mount({ task: task({ prUrl: PR_URL }) }); + await flush(); + confirmButton()?.click(); + await flush(); + expect(handlers.onClose).not.toHaveBeenCalled(); + expect(document.querySelector('[role="alert"]')?.textContent).toContain('head moved'); + }); + + it.each([ + [{ state: 'CLOSED' as const }, 'PR #42 is closed'], + [{ headRefName: 'someone/else' }, 'PR #42 is on someone/else'], + [{ headRefName: 'someone/else', state: 'MERGED' as const }, 'PR #42 is on someone/else'], + ])('merges locally when the PR cannot carry the work: %j', async (overrides, note) => { + vi.mocked(getPullRequestDetails).mockResolvedValue(prDetails(overrides)); + mount({ task: task({ githubUrl: PR_URL }) }); + await flush(); + expect(confirmButton()?.textContent).toBe('Merge into main'); + expect(document.body.textContent).toContain(note); + }); + + it('clears the push-first blocker once a push lands on the PR', async () => { + vi.mocked(getPullRequestDetails).mockResolvedValueOnce(prDetails({ headRefOid: 'older' })); + vi.mocked(getPullRequestDetails).mockResolvedValueOnce(prDetails()); + mount({ task: task({ prUrl: PR_URL }) }); + await flush(); + expect(confirmButton()?.disabled).toBe(true); + button('Push branch')?.click(); + await flush(); + await flush(); + expect(confirmButton()?.disabled).toBe(false); + expect(confirmButton()?.textContent).toBe('Squash and merge PR #42'); + }); + + it('retries loading the PR after a failure', async () => { + vi.mocked(getPullRequestDetails).mockRejectedValueOnce(new Error('offline')); + vi.mocked(getPullRequestDetails).mockResolvedValueOnce(prDetails()); + mount({ task: task({ prUrl: PR_URL }) }); + await flush(); + button('Retry')?.click(); + await flush(); + expect(confirmButton()?.textContent).toBe('Squash and merge PR #42'); + }); + + it('offers agent hand-offs only when CI fails or changes are requested', async () => { + vi.mocked(getPullRequestDetails).mockResolvedValue(prDetails()); + mount({ task: task({ prUrl: PR_URL }) }); + await flush(); + expect(button('Fix CI')).toBeUndefined(); + expect(button('Address review')).toBeUndefined(); + }); + + it('offers a local merge for commits made after the PR merged', async () => { + vi.mocked(getPullRequestDetails).mockResolvedValue( + prDetails({ state: 'MERGED', headRefOid: 'older' }), + ); + mount({ task: task({ prUrl: PR_URL }) }); + await flush(); + expect(confirmButton()?.textContent).toBe('Close task'); + expect(document.body.textContent).toContain('commits that are not in PR #42'); + button('Merge locally instead')?.click(); + expect(confirmButton()?.textContent).toBe('Merge into main'); + }); +}); diff --git a/src/components/FinishDialog.tsx b/src/components/FinishDialog.tsx index 151c0bf9b..425b9fc7f 100644 --- a/src/components/FinishDialog.tsx +++ b/src/components/FinishDialog.tsx @@ -1,4 +1,4 @@ -import { For, Show, createEffect, createSignal, on } from 'solid-js'; +import { Show, createEffect, createSignal, on } from 'solid-js'; import type { ChangeTourController } from '../lib/create-change-tour'; import type { Task } from '../store/types'; import type { ChangedFile } from '../ipc/types'; @@ -8,13 +8,15 @@ import { ConfirmDialog } from './ConfirmDialog'; import { DiffViewerDialog } from './DiffViewerDialog'; import { EvidencePanel } from './EvidencePanel'; import { MergeReadinessPanel } from './MergeReadinessPanel'; +import { FinishPrSection, PrMergeRoute } from './FinishPrSection'; import { MergeBlockers, MergeChanges, MergeOptions } from './MergeSection'; import { PushSection } from './PushSection'; import { ReadinessSection } from './ReadinessSection'; import { createMergeState } from './merge-state'; +import { PR_MERGE_ACTIONS, createFinishPr, taskPrUrl } from './pr-actions'; import { createPushRun } from './push-run'; -import { parseGitHubUrl } from '../lib/github-url'; -import { getPrChecks, getProject } from '../store/store'; +import { getProject } from '../store/store'; +import { showNotification } from '../store/notification'; import { theme } from '../lib/theme'; import { SyncIcon, UploadIcon } from './icons'; @@ -30,6 +32,8 @@ interface FinishDialogProps { onTourClick: () => void; onRegenerateTour: () => void; onOpenPullRequest: (url: string) => void; + /** Starts closing the task, once its PR merged on GitHub. */ + onCloseTask: () => void; onPushStart: () => void; onPushDone: (success: boolean) => void; onDiffFileClick: (file: ChangedFile) => void; @@ -70,14 +74,26 @@ export function FinishDialog(props: FinishDialogProps) { return props.task; }, onStart: () => props.onPushStart(), - onDone: (success) => props.onPushDone(success), + onDone: (success) => { + // Both heads may have moved; the push-first blocker compares them. + if (success) { + merge.refetchAll(); + void pr.refetch(); + } + props.onPushDone(success); + }, }); - const prUrl = () => - [props.task.prUrl, props.task.githubUrl].find((url) => { - const parsed = url ? parseGitHubUrl(url) : null; - return parsed?.type === 'pull' && !!parsed.number; - }); - const pr = () => getPrChecks(props.task.id); + const pr = createFinishPr({ + get open() { + return props.open; + }, + get task() { + return props.task; + }, + // Reading an errored resource throws; treat it as an unreadable HEAD. + headSha: () => (merge.worktreeStatus.error ? null : merge.worktreeStatus()?.head_sha), + }); + const prMerging = () => pr.busy() === 'merge'; // Tracks only `open`: reset reads `pushing`, and re-running when a push ends // would wipe the error it just reported. @@ -91,7 +107,11 @@ export function FinishDialog(props: FinishDialogProps) { ); const confirmLabel = () => { - if (merge.merging()) return 'Merging...'; + if (pr.mode() === 'merged') return 'Close task'; + if (merge.merging() || prMerging()) return 'Merging...'; + const method = pr.method(); + if (pr.mode() === 'pr') + return `${method ? PR_MERGE_ACTIONS[method] : 'Merge'} PR #${pr.number()}`; // The target lives on the button, so the dialog needs no sentence restating it. return `${merge.squash() ? 'Squash merge' : 'Merge'} into ${merge.baseBranchName()}`; }; @@ -99,12 +119,40 @@ export function FinishDialog(props: FinishDialogProps) { // verify command in the check list, evidence by the panel these rows sit in. const readinessRows = () => merge.mergeReadiness().checks.filter((check) => !SHOWN_ELSEWHERE.has(check.label)); - const confirmDisabled = () => push.pushing() || !merge.canMerge(); + const prCheckRows = () => + merge.mergeReadiness().checks.filter((check) => check.label === 'PR checks'); + const confirmDisabled = () => { + if (push.pushing()) return true; + if (pr.mode() === 'merged') return false; + if (pr.mode() === 'pr') return !!pr.busy() || !!pr.blocker(); + return !!pr.busy() || !merge.canMerge(); + }; + const footerNote = () => { + if (merge.merging() || prMerging() || pr.mode() === 'merged') return undefined; + return pr.mode() === 'pr' ? pr.blocker() : merge.mergeBlocker(); + }; const mergeAndClose = async () => { if (await merge.merge()) props.onClose(); }; + const mergePr = async () => { + const number = pr.number(); + const outcome = await pr.merge(); + if (outcome === 'failed') return; + if (outcome === 'queued') { + showNotification(`Merge requested for PR #${number}; GitHub has not merged it yet.`); + props.onClose(); + return; + } + showNotification(`Merged PR #${number}`); + // Stay open: the refetched PR flips the footer to Close task. + void pr.refetch(); + }; const confirm = () => { - void mergeAndClose(); + if (pr.mode() === 'merged') { + props.onClose(); + props.onCloseTask(); + } else if (pr.mode() === 'pr') void mergePr(); + else void mergeAndClose(); }; return ( @@ -172,68 +220,23 @@ export function FinishDialog(props: FinishDialogProps) { -
-

- GitHub PR and CI -

- No pull request detected for this task.

}> - {(url) => ( - <> - - -

Merged on GitHub

- - -

Draft pull request

- - - {(decision) =>

Review: {decision().toLowerCase().replace(/_/g, ' ')}

} - - -

PR has conflicts with its base branch.

- - check.label === 'PR checks')} - /> - -
    - - {(check) => ( -
  • - {check.name}: {check.bucket} -
    • - )} -     -
- -

- CI reflects the PR on GitHub, not unpushed local changes. -

- - )} - -
- + props.onOpenPullRequest(url)} + onStaged={() => { + showNotification('Prompt staged in the task input. Review it, then send.'); + props.onClose(); + }} + /> + + + + + +
@@ -245,13 +248,13 @@ export function FinishDialog(props: FinishDialogProps) { } confirmDisabled={confirmDisabled()} - footerNote={merge.merging() ? undefined : merge.mergeBlocker()} - confirmLoading={merge.merging()} + footerNote={footerNote()} + confirmLoading={merge.merging() || prMerging()} confirmLabel={confirmLabel()} cancelLabel={push.pushing() ? 'Close' : 'Cancel'} onConfirm={confirm} diff --git a/src/components/FinishPrSection.tsx b/src/components/FinishPrSection.tsx new file mode 100644 index 000000000..669070896 --- /dev/null +++ b/src/components/FinishPrSection.tsx @@ -0,0 +1,244 @@ +import { For, Show } from 'solid-js'; +import { errMessage } from '../lib/log'; +import { bannerStyle, dialogButtonStyle, linkButtonStyle, theme } from '../lib/theme'; +import { getPrChecks } from '../store/store'; +import type { PrMergeMethod } from '../ipc/types'; +import type { Task } from '../store/types'; +import type { MergeReadinessCheck } from './merge-readiness'; +import { MergeReadinessPanel } from './MergeReadinessPanel'; +import { PR_METHOD_LABELS, taskPrUrl, type FinishPr } from './pr-actions'; + +interface FinishPrSectionProps { + task: Task; + pr: FinishPr; + /** The readiness row summarising PR checks. */ + checkRows: MergeReadinessCheck[]; + /** A push or merge is running; PR actions would race it. */ + disabled: boolean; + onOpenPullRequest: (url: string) => void; + /** A Fix CI or review prompt now waits in the task input. */ + onStaged: () => void; +} + +const mutedText = { color: theme.fgMuted, margin: '6px 0' }; + +/** + * What GitHub says about the task's PR: status, CI, and agent hand-offs that + * only show when there is something to fix. How to merge lives in PrMergeRoute. + */ +export function FinishPrSection(props: FinishPrSectionProps) { + const checks = () => getPrChecks(props.task.id); + const busyOrDisabled = () => props.disabled || !!props.pr.busy(); + const merged = () => props.pr.mode() === 'merged'; + const summary = () => { + const c = checks(); + const parts = [ + c?.isDraft ? 'Draft' : '', + c?.reviewDecision ? `Review: ${c.reviewDecision.toLowerCase().replace(/_/g, ' ')}` : '', + ]; + return parts.filter(Boolean).join(' · '); + }; + const ciFailed = () => (checks()?.failing ?? 0) > 0; + const changesRequested = () => checks()?.reviewDecision === 'CHANGES_REQUESTED'; + const stage = async (kind: 'fix-ci' | 'review') => { + if (await props.pr.stage(kind)) props.onStaged(); + }; + + return ( +
+

+ GitHub PR and CI + + {(url) => ( + <> + {' · '} + + + )} + +

+ No pull request detected for this task.

}> + Merged on GitHub.

}> + +

{summary()}

+ + +

+ PR has conflicts with its base branch. +

+ + + +
    + + {(check) => ( +
  • + {check.name}: {check.bucket} +
    • + )} +     +
+ + +

+ CI reflects the PR on GitHub, not unpushed local commits. +

+ + +
+ + + + + + +
+ + +

+ {props.pr.info()} +

+ + + +
+ ); +} + +const selectStyle = { + padding: '6px 8px', + background: theme.bgInput, + border: `1px solid ${theme.border}`, + 'border-radius': 'var(--radius-md)', + color: theme.fg, + 'font-size': '13px', +}; + +/** + * How a task with a PR merges: the GitHub merge method, or why and how it + * merges locally instead. Renders nothing without a PR. + */ +export function PrMergeRoute(props: { pr: FinishPr; disabled: boolean }) { + const disabled = () => props.disabled || !!props.pr.busy(); + const backToPr = () => ( + + ); + const mergeLocally = () => ( + + ); + + return ( +
+ + + {(details) => ( + <> + + {/* UNSTABLE: non-required checks fail; GitHub allows the merge. */} + +

Some checks are failing; GitHub still allows the merge.

+ + + )} + +

{mergeLocally()}

+ + +

+ This worktree has commits that are not in PR #{props.pr.number()}. {mergeLocally()} +

+ + + {(reason) =>

{reason()}

} + +

{backToPr()}

+ + + {(err) => ( +

+ Could not load the pull request ({errMessage(err())}), so this merges locally.{' '} + +

+ )} + + + +
+ {props.pr.error()} +
+ +
+ ); +} diff --git a/src/components/PullRequestDialog.tsx b/src/components/PullRequestDialog.tsx index d1d1ff10e..e189bdf61 100644 --- a/src/components/PullRequestDialog.tsx +++ b/src/components/PullRequestDialog.tsx @@ -1,18 +1,13 @@ -import { For, Show, createEffect, createResource, createSignal, on } from 'solid-js'; +import { For, Show, createEffect, createSignal, on } from 'solid-js'; import { Dialog } from './Dialog'; import { errMessage } from '../lib/log'; import { bannerStyle, dialogButtonStyle, theme } from '../lib/theme'; -import { - getPullRequestDetails, - mergePullRequestForTask, - stageFailedChecksPrompt, - stageReviewFeedbackPrompt, -} from '../store/github'; import { getPrChecks } from '../store/store'; import { showNotification } from '../store/notification'; -import type { PrCheckBucket, PrMergeMethod, PullRequestDetails } from '../ipc/types'; +import type { PrCheckBucket, PrMergeMethod } from '../ipc/types'; import type { Task } from '../store/types'; import { CommentIcon, GitHubIcon, GitMergeIcon, ToolsIcon } from './icons'; +import { PR_METHOD_LABELS, createPrActions, prMergeBlocker } from './pr-actions'; interface PullRequestDialogProps { open: boolean; @@ -21,14 +16,6 @@ interface PullRequestDialogProps { onClose: () => void; } -type Busy = 'fix-ci' | 'review' | 'merge' | null; - -const METHOD_LABELS: Record = { - squash: 'Squash and merge', - merge: 'Create a merge commit', - rebase: 'Rebase and merge', -}; - const BUCKET_COLORS: Record = { pass: theme.success, skipping: theme.fgSubtle, @@ -37,103 +24,47 @@ const BUCKET_COLORS: Record = { cancel: theme.error, }; -/** Why merging is not offered, or null when GitHub may accept it. */ -function mergeBlocker(pr: PullRequestDetails): string | null { - if (pr.state === 'MERGED') return 'Already merged.'; - if (pr.state === 'CLOSED') return 'The pull request is closed.'; - if (pr.isDraft) return 'Draft pull requests cannot be merged. Mark it ready on GitHub first.'; - if (pr.mergeable === 'CONFLICTING') return 'The branch has conflicts with its base branch.'; - if (pr.mergeStateStatus === 'BEHIND') - return 'The branch must be updated with its base branch first.'; - if (pr.mergeStateStatus === 'BLOCKED') { - return 'GitHub blocks merging until required reviews or checks pass.'; - } - if (pr.mergeMethods.length === 0) return 'The repository allows no merge method you can use.'; - if (!pr.headRefOid) return 'GitHub did not report the head commit; reopen to retry.'; - return null; -} - /** PR status for a task, plus actions that feed GitHub feedback to the agent or merge. */ export function PullRequestDialog(props: PullRequestDialogProps) { - const [busy, setBusy] = createSignal(null); - const [error, setError] = createSignal(''); - const [info, setInfo] = createSignal(''); - const [method, setMethod] = createSignal(null); + const actions = createPrActions(props); + const { details, busy, error, info } = actions; const [confirmingMerge, setConfirmingMerge] = createSignal(false); - const [details] = createResource( - () => (props.open ? props.prUrl : null), - (url) => getPullRequestDetails(url), - ); const checks = () => getPrChecks(props.task.id); - const prRef = () => ({ number: details()?.number ?? 0, url: props.prUrl }); + const chosenMethod = actions.method; + const setMethod = actions.setMethod; const blocker = () => { - const d = details(); - return d ? mergeBlocker(d) : null; + const d = actions.pr(); + return d ? prMergeBlocker(d) : null; }; createEffect( on( () => props.open, (open) => { - if (!open) return; - setError(''); - setInfo(''); - setMethod(null); - setConfirmingMerge(false); + if (open) setConfirmingMerge(false); }, ), ); - function begin(kind: Exclude) { - setBusy(kind); - setError(''); - setInfo(''); - } - async function stage(kind: 'fix-ci' | 'review') { - begin(kind); - try { - const stageFn = kind === 'fix-ci' ? stageFailedChecksPrompt : stageReviewFeedbackPrompt; - if (!(await stageFn(props.task.id, prRef()))) { - setInfo(kind === 'fix-ci' ? 'No failed checks found.' : 'No open review feedback found.'); - return; - } - showNotification('Prompt staged in the task input. Review it, then send.'); - props.onClose(); - } catch (err) { - setError(errMessage(err)); - } finally { - setBusy(null); - } + if (!(await actions.stage(kind))) return; + showNotification('Prompt staged in the task input. Review it, then send.'); + props.onClose(); } - const chosenMethod = () => method() ?? details()?.mergeMethods[0]; - async function merge() { - const chosen = chosenMethod(); - const pr = details(); - if (!chosen || !pr) return; + const number = actions.pr()?.number; setConfirmingMerge(false); - begin('merge'); - try { - const merged = await mergePullRequestForTask(props.task.id, { - prUrl: props.prUrl, - method: chosen, - headSha: pr.headRefOid, - }); - showNotification( - merged - ? `Merged PR #${pr.number}` - : `Merge requested for PR #${pr.number}; GitHub has not merged it yet (it may be in a merge queue).`, - ); - // Closing avoids showing the pre-merge details (and merge button) until - // a refetch lands; the PR chip updates from the checks watcher. - props.onClose(); - } catch (err) { - setError(errMessage(err)); - } finally { - setBusy(null); - } + const outcome = await actions.merge(); + if (outcome === 'failed') return; + showNotification( + outcome === 'merged' + ? `Merged PR #${number}` + : `Merge requested for PR #${number}; GitHub has not merged it yet (it may be in a merge queue).`, + ); + // Closing avoids showing the pre-merge details (and merge button) until + // a refetch lands; the PR chip updates from the checks watcher. + props.onClose(); } return ( @@ -141,7 +72,7 @@ export function PullRequestDialog(props: PullRequestDialogProps) {

Pull Request - + {(d) => ( <> {' '} @@ -163,7 +94,7 @@ export function PullRequestDialog(props: PullRequestDialogProps) { )} - + {(d) => (
)} - +
This task works on {props.task.branchName}, not the PR branch{' '} - {details()?.headRefName}. Pushing from this task does not update the + {actions.pr()?.headRefName}. Pushing from this task does not update the PR.
@@ -251,10 +184,10 @@ export function PullRequestDialog(props: PullRequestDialogProps) {

- + {(d) => ( - {(m) => } + {(m) => }