From 98ae8e4c8c5f0c5f63976a54d7c084109411789f Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Sun, 2 Aug 2026 12:05:27 -0700 Subject: [PATCH 001/106] Allow clearing all agents from AI Vault session history filter (#12128) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Allow clearing all agents from AI Vault session history filter Add "Select all" / "Clear" buttons so users can quickly isolate one agent without unchecking each box individually. Previously, at least one agent had to remain enabled; now users can filter to zero agents and re-enable selectively. * Address PR #12128 review feedback - Make Select all / Clear real DropdownMenuItems so Radix roving focus reaches them by keyboard. - Rename the zero-agent empty state to a neutral "No agents selected" now that zero agents is a valid filter. - Use 모두 해제 for the Korean Clear label instead of 지우기 (erase). Co-authored-by: Orca --------- Co-authored-by: Orca --- config/scripts/locale-ko-key-overrides.json | 9 ++++ .../components/right-sidebar/AiVaultPanel.tsx | 3 ++ .../right-sidebar/AiVaultPanelControls.tsx | 45 +++++++++++++++++-- .../right-sidebar/AiVaultPanelHeader.tsx | 3 ++ .../AiVaultSessionVirtualList.tsx | 17 +++++-- .../ai-vault-view-options-persistence.test.ts | 6 +-- .../ai-vault-view-options-persistence.ts | 6 +-- ...e-persisted-ai-vault-view-options.test.tsx | 17 ++++++- .../use-persisted-ai-vault-view-options.ts | 29 ++++++++++-- src/renderer/src/i18n/locales/en.json | 3 ++ src/renderer/src/i18n/locales/es.json | 3 ++ src/renderer/src/i18n/locales/ja.json | 3 ++ src/renderer/src/i18n/locales/ko.json | 3 ++ src/renderer/src/i18n/locales/zh.json | 3 ++ 14 files changed, 129 insertions(+), 21 deletions(-) diff --git a/config/scripts/locale-ko-key-overrides.json b/config/scripts/locale-ko-key-overrides.json index 4618f052f1e..9b40ad61f50 100644 --- a/config/scripts/locale-ko-key-overrides.json +++ b/config/scripts/locale-ko-key-overrides.json @@ -5038,5 +5038,14 @@ }, "settings.appearance.statusBar.sshToggleDescription": { "ko": "사용 가능한 SSH 및 원격 Orca 호스트가 있으면 표시합니다." + }, + "auto.components.right.sidebar.AiVaultPanelControls.selectAllAgents": { + "ko": "모두 선택" + }, + "auto.components.right.sidebar.AiVaultPanelControls.clearAgents": { + "ko": "모두 해제" + }, + "auto.components.right.sidebar.AiVaultPanel.noAgentsSelected": { + "ko": "선택된 에이전트가 없습니다" } } diff --git a/src/renderer/src/components/right-sidebar/AiVaultPanel.tsx b/src/renderer/src/components/right-sidebar/AiVaultPanel.tsx index 048ba89252e..0113f0d22e9 100644 --- a/src/renderer/src/components/right-sidebar/AiVaultPanel.tsx +++ b/src/renderer/src/components/right-sidebar/AiVaultPanel.tsx @@ -81,6 +81,7 @@ export default function AiVaultPanel(): React.JSX.Element { setGroup, setHideEmptySessions, setAgentEnabled, + setAllAgentsEnabled, resetViewOptions } = usePersistedAiVaultViewOptions() const [collapsedGroups, setCollapsedGroups] = useState>(() => new Set()) @@ -320,6 +321,7 @@ export default function AiVaultPanel(): React.JSX.Element { onScopeChange={handleScopeChange} onExecutionHostScopeChange={onExecutionHostScopeChange} onAgentEnabledChange={setAgentEnabled} + onAllAgentsEnabledChange={setAllAgentsEnabled} onSortChange={setSort} onGroupChange={setGroup} onHideEmptySessionsChange={setHideEmptySessions} @@ -349,6 +351,7 @@ export default function AiVaultPanel(): React.JSX.Element { loading={loading} sessionsCount={sessions.length} filteredSessionsCount={filteredSessions.length} + noAgentsSelected={agents.length === 0} error={error} vaultScope={scope} buildResumeStartup={launchActions.buildResumeStartup} diff --git a/src/renderer/src/components/right-sidebar/AiVaultPanelControls.tsx b/src/renderer/src/components/right-sidebar/AiVaultPanelControls.tsx index b64308b943e..0a82ca00e2b 100644 --- a/src/renderer/src/components/right-sidebar/AiVaultPanelControls.tsx +++ b/src/renderer/src/components/right-sidebar/AiVaultPanelControls.tsx @@ -39,6 +39,9 @@ import type { AiVaultHostScopeOption } from './ai-vault-host-scope' const VAULT_HEADER_CONTROL_CLASS = 'size-6 shrink-0' +const AGENT_BULK_ACTION_CLASS = + 'rounded-full px-2 py-0.5 text-[11px] font-normal text-muted-foreground focus:text-foreground' + // Why: match ToggleGroup's spacing+outline qualifiers so selected edges out-specify its border-l-0 collapse. const VAULT_SCOPE_SELECTED_EDGE_CLASS = 'data-[spacing=0]:data-[variant=outline]:aria-[checked=true]:border-l data-[spacing=0]:data-[variant=outline]:data-[state=on]:border-l' @@ -234,6 +237,7 @@ export function VaultViewMenu({ hideEmptySessions, adjustmentCount, onAgentEnabledChange, + onAllAgentsEnabledChange, onSortChange, onGroupChange, onHideEmptySessionsChange, @@ -245,11 +249,15 @@ export function VaultViewMenu({ hideEmptySessions: boolean adjustmentCount: number onAgentEnabledChange: (agent: AiVaultAgent, enabled: boolean) => void + onAllAgentsEnabledChange: (enabled: boolean) => void onSortChange: (sort: AiVaultSort) => void onGroupChange: (group: AiVaultGroup) => void onHideEmptySessionsChange: (hideEmptySessions: boolean) => void onReset: () => void }): React.JSX.Element { + const allAgentsSelected = agents.length === AI_VAULT_AGENTS.length + const noAgentsSelected = agents.length === 0 + return ( @@ -284,14 +292,43 @@ export function VaultViewMenu({ - - {translate('auto.components.right.sidebar.AiVaultPanelControls.agents', 'Agents')} - + {/* Why: Select all / Clear lets users isolate one agent without unchecking 15 boxes. */} +
+ + {translate('auto.components.right.sidebar.AiVaultPanelControls.agents', 'Agents')} + + {/* Why: real menu items so arrow keys reach them; plain buttons are skipped by Radix roving focus. */} +
+ { + event.preventDefault() + onAllAgentsEnabledChange(true) + }} + className={AGENT_BULK_ACTION_CLASS} + > + {translate( + 'auto.components.right.sidebar.AiVaultPanelControls.selectAllAgents', + 'Select all' + )} + + { + event.preventDefault() + onAllAgentsEnabledChange(false) + }} + className={AGENT_BULK_ACTION_CLASS} + > + {translate('auto.components.right.sidebar.AiVaultPanelControls.clearAgents', 'Clear')} + +
+
{AI_VAULT_AGENTS.map((agent) => ( onAgentEnabledChange(agent, checked === true)} onSelect={(event) => event.preventDefault()} > diff --git a/src/renderer/src/components/right-sidebar/AiVaultPanelHeader.tsx b/src/renderer/src/components/right-sidebar/AiVaultPanelHeader.tsx index b3ec2ba36e3..86c728aa8a4 100644 --- a/src/renderer/src/components/right-sidebar/AiVaultPanelHeader.tsx +++ b/src/renderer/src/components/right-sidebar/AiVaultPanelHeader.tsx @@ -31,6 +31,7 @@ type AiVaultPanelHeaderProps = { onScopeChange: (scope: AiVaultScope) => void onExecutionHostScopeChange: (scope: ExecutionHostScope) => void onAgentEnabledChange: (agent: AiVaultAgent, enabled: boolean) => void + onAllAgentsEnabledChange: (enabled: boolean) => void onSortChange: (sort: AiVaultSort) => void onGroupChange: (group: AiVaultGroup) => void onHideEmptySessionsChange: (hideEmptySessions: boolean) => void @@ -58,6 +59,7 @@ export function AiVaultPanelHeader({ onScopeChange, onExecutionHostScopeChange, onAgentEnabledChange, + onAllAgentsEnabledChange, onSortChange, onGroupChange, onHideEmptySessionsChange, @@ -119,6 +121,7 @@ export function AiVaultPanelHeader({ hideEmptySessions={hideEmptySessions} adjustmentCount={adjustmentCount} onAgentEnabledChange={onAgentEnabledChange} + onAllAgentsEnabledChange={onAllAgentsEnabledChange} onSortChange={onSortChange} onGroupChange={onGroupChange} onHideEmptySessionsChange={onHideEmptySessionsChange} diff --git a/src/renderer/src/components/right-sidebar/AiVaultSessionVirtualList.tsx b/src/renderer/src/components/right-sidebar/AiVaultSessionVirtualList.tsx index 79d17f2305b..026e9ad1d75 100644 --- a/src/renderer/src/components/right-sidebar/AiVaultSessionVirtualList.tsx +++ b/src/renderer/src/components/right-sidebar/AiVaultSessionVirtualList.tsx @@ -46,6 +46,7 @@ export function AiVaultSessionVirtualList({ loading, sessionsCount, filteredSessionsCount, + noAgentsSelected, error, vaultScope, buildResumeStartup, @@ -71,6 +72,7 @@ export function AiVaultSessionVirtualList({ loading: boolean sessionsCount: number filteredSessionsCount: number + noAgentsSelected: boolean error: string | null vaultScope: AiVaultScope buildResumeStartup: (session: AiVaultSession, worktreeId?: string | null) => AiVaultResumeStartup @@ -181,10 +183,17 @@ export function AiVaultSessionVirtualList({ {sessionsCount > 0 && filteredSessionsCount === 0 ? ( ) : null} diff --git a/src/renderer/src/components/right-sidebar/ai-vault-view-options-persistence.test.ts b/src/renderer/src/components/right-sidebar/ai-vault-view-options-persistence.test.ts index 2ad8e260bdd..76624f047cf 100644 --- a/src/renderer/src/components/right-sidebar/ai-vault-view-options-persistence.test.ts +++ b/src/renderer/src/components/right-sidebar/ai-vault-view-options-persistence.test.ts @@ -66,7 +66,7 @@ describe('AI Vault view option persistence', () => { expect(normalizeAiVaultViewOptions({ group: 'agent' }).group).toBe('agent') }) - it('falls back to all enabled when stored state disables the whole catalog', () => { + it('preserves a fully cleared agent selection', () => { const normalized = normalizeAiVaultViewOptions({ disabledAgents: [...AI_VAULT_AGENTS], sort: 'created', @@ -74,8 +74,8 @@ describe('AI Vault view option persistence', () => { hideEmptySessions: true }) - expect(normalized.disabledAgents).toEqual([]) - expect(enabledAiVaultAgents(normalized.disabledAgents)).toEqual([...AI_VAULT_AGENTS]) + expect(normalized.disabledAgents).toEqual([...AI_VAULT_AGENTS]) + expect(enabledAiVaultAgents(normalized.disabledAgents)).toEqual([]) }) it('falls back safely when JSON or storage access is unavailable', () => { diff --git a/src/renderer/src/components/right-sidebar/ai-vault-view-options-persistence.ts b/src/renderer/src/components/right-sidebar/ai-vault-view-options-persistence.ts index 5b7a4b799cf..cace6a9c307 100644 --- a/src/renderer/src/components/right-sidebar/ai-vault-view-options-persistence.ts +++ b/src/renderer/src/components/right-sidebar/ai-vault-view-options-persistence.ts @@ -49,14 +49,12 @@ function isAiVaultGroup(value: unknown): value is AiVaultGroup { export function normalizeAiVaultViewOptions(value: unknown): AiVaultViewOptions { const record = value && typeof value === 'object' ? (value as Record) : {} const catalog = new Set(AI_VAULT_AGENTS) - const normalizedDisabledAgents = Array.isArray(record.disabledAgents) + // Why: empty selection is intentional (Clear all agents) so users can enable only one agent. + const disabledAgents = Array.isArray(record.disabledAgents) ? [...new Set(record.disabledAgents)].filter( (agent): agent is AiVaultAgent => typeof agent === 'string' && catalog.has(agent) ) : [] - // Why: a stale catalog or hand-edited value must not leave the panel with no selectable agents. - const disabledAgents = - normalizedDisabledAgents.length < AI_VAULT_AGENTS.length ? normalizedDisabledAgents : [] return { disabledAgents, diff --git a/src/renderer/src/components/right-sidebar/use-persisted-ai-vault-view-options.test.tsx b/src/renderer/src/components/right-sidebar/use-persisted-ai-vault-view-options.test.tsx index ba824696c9a..3acc9ae44be 100644 --- a/src/renderer/src/components/right-sidebar/use-persisted-ai-vault-view-options.test.tsx +++ b/src/renderer/src/components/right-sidebar/use-persisted-ai-vault-view-options.test.tsx @@ -34,7 +34,20 @@ describe('usePersistedAiVaultViewOptions', () => { expect(restored.result.current.hideEmptySessions).toBe(true) }) - it('keeps at least one agent enabled', () => { + it('allows clearing every agent so a single agent can be re-enabled', () => { + const hook = renderHook(() => usePersistedAiVaultViewOptions()) + + act(() => hook.result.current.setAllAgentsEnabled(false)) + expect(hook.result.current.agents).toEqual([]) + + act(() => hook.result.current.setAgentEnabled('claude', true)) + expect(hook.result.current.agents).toEqual(['claude']) + + act(() => hook.result.current.setAllAgentsEnabled(true)) + expect(hook.result.current.agents).toEqual([...AI_VAULT_AGENTS]) + }) + + it('allows disabling the last remaining agent', () => { const hook = renderHook(() => usePersistedAiVaultViewOptions()) const lastEnabled = AI_VAULT_AGENTS[0] @@ -46,7 +59,7 @@ describe('usePersistedAiVaultViewOptions', () => { expect(hook.result.current.agents).toEqual([lastEnabled]) act(() => hook.result.current.setAgentEnabled(lastEnabled, false)) - expect(hook.result.current.agents).toEqual([lastEnabled]) + expect(hook.result.current.agents).toEqual([]) }) it('keeps in-memory options usable when persistence fails', () => { diff --git a/src/renderer/src/components/right-sidebar/use-persisted-ai-vault-view-options.ts b/src/renderer/src/components/right-sidebar/use-persisted-ai-vault-view-options.ts index ffd6816d4a6..0315c242af7 100644 --- a/src/renderer/src/components/right-sidebar/use-persisted-ai-vault-view-options.ts +++ b/src/renderer/src/components/right-sidebar/use-persisted-ai-vault-view-options.ts @@ -1,5 +1,10 @@ import { useCallback, useMemo, useRef, useState } from 'react' -import type { AiVaultAgent, AiVaultGroup, AiVaultSort } from '../../../../shared/ai-vault-types' +import { + AI_VAULT_AGENTS, + type AiVaultAgent, + type AiVaultGroup, + type AiVaultSort +} from '../../../../shared/ai-vault-types' import { createDefaultAiVaultViewOptions, enabledAiVaultAgents, @@ -19,6 +24,7 @@ export function usePersistedAiVaultViewOptions(): { setGroup: (group: AiVaultGroup) => void setHideEmptySessions: (hide: boolean) => void setAgentEnabled: (agent: AiVaultAgent, enabled: boolean) => void + setAllAgentsEnabled: (enabled: boolean) => void resetViewOptions: () => void } { const [options, setOptions] = useState(() => readAiVaultViewOptions()) @@ -66,12 +72,26 @@ export function usePersistedAiVaultViewOptions(): { if (enabled === !isDisabled) { return current } + // Why: allow zero enabled agents so Clear + re-check one agent is a two-step filter. const disabledAgents = enabled ? current.disabledAgents.filter((entry) => entry !== agent) : [...current.disabledAgents, agent] - return enabledAiVaultAgents(disabledAgents).length > 0 - ? { ...current, disabledAgents } - : current + return { ...current, disabledAgents } + }) + }, + [updateOptions] + ) + const setAllAgentsEnabled = useCallback( + (enabled: boolean) => { + updateOptions((current) => { + const disabledAgents = enabled ? [] : [...AI_VAULT_AGENTS] + if ( + disabledAgents.length === current.disabledAgents.length && + disabledAgents.every((agent) => current.disabledAgents.includes(agent)) + ) { + return current + } + return { ...current, disabledAgents } }) }, [updateOptions] @@ -94,6 +114,7 @@ export function usePersistedAiVaultViewOptions(): { setGroup, setHideEmptySessions, setAgentEnabled, + setAllAgentsEnabled, resetViewOptions } } diff --git a/src/renderer/src/i18n/locales/en.json b/src/renderer/src/i18n/locales/en.json index 0354113eb90..641b9b018a7 100644 --- a/src/renderer/src/i18n/locales/en.json +++ b/src/renderer/src/i18n/locales/en.json @@ -11202,6 +11202,7 @@ "transcriptsSkipped": "{{count}} transcript skipped", "noAgentSessionsFound": "No agent sessions found", "noSessionsMatchFilters": "No sessions match the current filters", + "noAgentsSelected": "No agents selected", "sessionId": "Session ID", "logPath": "Log path", "originalPaneUnavailable": "Original pane is no longer available.", @@ -11225,6 +11226,8 @@ "viewOptionsAriaLabel": "Session History view options", "viewOptions": "View options", "agents": "Agents", + "selectAllAgents": "Select all", + "clearAgents": "Clear", "sort": "Sort", "lastUpdated": "Last updated", "created": "Created", diff --git a/src/renderer/src/i18n/locales/es.json b/src/renderer/src/i18n/locales/es.json index 9fbec004772..f251c3e0fa9 100644 --- a/src/renderer/src/i18n/locales/es.json +++ b/src/renderer/src/i18n/locales/es.json @@ -11091,6 +11091,7 @@ "transcriptsSkipped": "{{count}} transcripciones omitidas", "noAgentSessionsFound": "No se encontraron sesiones de agentes", "noSessionsMatchFilters": "Ninguna sesión coincide con los filtros actuales", + "noAgentsSelected": "Ningún agente seleccionado", "sessionId": "ID de sesión", "logPath": "Ruta del log", "agents": "Agentes", @@ -11116,6 +11117,8 @@ "viewOptionsAriaLabel": "Opciones de vista del historial de sesiones", "viewOptions": "Opciones de vista", "agents": "Agentes", + "selectAllAgents": "Seleccionar todo", + "clearAgents": "Limpiar", "sort": "Ordenar", "lastUpdated": "Última actualización", "created": "Creado", diff --git a/src/renderer/src/i18n/locales/ja.json b/src/renderer/src/i18n/locales/ja.json index 69b65346353..0e53b3b1e29 100644 --- a/src/renderer/src/i18n/locales/ja.json +++ b/src/renderer/src/i18n/locales/ja.json @@ -11091,6 +11091,7 @@ "transcriptsSkipped": "{{count}} transcript skipped", "noAgentSessionsFound": "No agent sessions found", "noSessionsMatchFilters": "No sessions match the current filters", + "noAgentsSelected": "エージェントが選択されていません", "sessionId": "セッション ID", "logPath": "ログパス", "agents": "エージェント", @@ -11116,6 +11117,8 @@ "viewOptionsAriaLabel": "Session History view options", "viewOptions": "表示オプション", "agents": "エージェント", + "selectAllAgents": "すべて選択", + "clearAgents": "クリア", "sort": "並び替え", "lastUpdated": "最終更新", "created": "Created", diff --git a/src/renderer/src/i18n/locales/ko.json b/src/renderer/src/i18n/locales/ko.json index ae90177d18d..d44e2d7501f 100644 --- a/src/renderer/src/i18n/locales/ko.json +++ b/src/renderer/src/i18n/locales/ko.json @@ -11091,6 +11091,7 @@ "transcriptsSkipped": "대화 기록 {{count}}개 건너뜀", "noAgentSessionsFound": "agent 세션을 찾을 수 없습니다", "noSessionsMatchFilters": "현재 필터와 일치하는 세션이 없습니다", + "noAgentsSelected": "선택된 에이전트가 없습니다", "sessionId": "세션 ID", "logPath": "로그 경로", "agents": "에이전트", @@ -11116,6 +11117,8 @@ "viewOptionsAriaLabel": "세션 기록 보기 옵션", "viewOptions": "보기 옵션", "agents": "에이전트", + "selectAllAgents": "모두 선택", + "clearAgents": "모두 해제", "sort": "정렬", "lastUpdated": "마지막 업데이트", "created": "생성됨", diff --git a/src/renderer/src/i18n/locales/zh.json b/src/renderer/src/i18n/locales/zh.json index 0c3fb081227..626cbcc869b 100644 --- a/src/renderer/src/i18n/locales/zh.json +++ b/src/renderer/src/i18n/locales/zh.json @@ -11091,6 +11091,7 @@ "transcriptsSkipped": "跳过了 {{count}} 个记录", "noAgentSessionsFound": "未找到智能体会话", "noSessionsMatchFilters": "没有会话符合当前筛选条件", + "noAgentsSelected": "未选择任何智能体", "sessionId": "会话 ID", "logPath": "日志路径", "agents": "智能体", @@ -11116,6 +11117,8 @@ "viewOptionsAriaLabel": "会话历史查看选项", "viewOptions": "查看选项", "agents": "智能体", + "selectAllAgents": "选择全部", + "clearAgents": "清除", "sort": "排序", "lastUpdated": "最后更新", "created": "创建时间", From 56ab5fd1dc6f94aad8ccaaa4abe4f716c9a7e9cc Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Sun, 2 Aug 2026 12:05:50 -0700 Subject: [PATCH 002/106] =?UTF-8?q?fix(tasks):=20make=20GitHub=20paginatio?= =?UTF-8?q?n=20honest=20=E2=80=94=20cap=20unreachable=20pages,=20survive?= =?UTF-8?q?=20background=20refreshes,=20explain=20empty=20pages=20(#11584)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(tasks): cap advertised GitHub pages at the search result window GitHub's Search API rejects requests past its first-1000-results window with HTTP 422, but totalPages was derived from the raw total_count, so the pagination bar advertised pages that could never load and clicks on them silently did nothing (#11485). Cap per-repo advertised pages at floor(1000 / perRepoLimit), and when a page load comes back empty, say so with a toast instead of ignoring the click — clamping the advertised count only when no fetch threw, so transient failures don't shrink the bar. * fix(tasks): key pagination resets on repo selection, not array identity The repos store installs a fresh array on every repos:changed event, so the pagination-reset effect fired on background refreshes and bumped the request generation, silently discarding any in-flight page navigation — clicking an unloaded page did nothing whenever a repo refresh landed during the fetch. Key the effect on the stable selection string instead. * fix(tasks): distinguish end-of-data, window 422s, and failures on empty pages Adversarial-review round 1 rework: - fetchWorkItemsNextPage now returns issue-side envelope error types — the channel the search-window 422 actually travels on (failedCount only counts thrown repo calls). - resolveEmptyPageOutcome (unit-tested) maps an empty page to window-unreachable (clamp + toast), load-failed (toast only; may be transient), or end-of-data (silently withdraw the speculative page the count-fallback advertises). - The work-items fetch effect is keyed on selectedReposKey too — its unconditional page reset re-fired on every repos:changed array identity, bouncing the user to page 1 mid-click. The key now includes the resolved GitHub source context so identity changes still re-dispatch. - Toasts carry stable ids so repeats replace instead of stack. - Cap comment documents the conservative PR-scope tail loss; cap tests pinned at shipped (36 → 27) and dividing (25 → 40) limits. * fix(tasks): withdraw the speculative page when the failed count is zero countedTotalPages of 0 comes from a swallowed count failure and routes totalPages through the fallback, so the clamp must replace it like null. * fix(tasks): tighten empty-page outcomes after round-2 review - en.json's loadPageUnreachable carried the pre-reword text, and the catalog beats the inline default — the two toasts were identical. - end-of-data clamps only while the count is unknown/failed: the PR list path swallows its own failures into clean-empty results, and clamping a real count silently hid healthy pages (worse than the pre-fix no-op). - A window 422 no longer clamps when a sibling repo's fetch threw. - The generation effect mirrors every fetch-effect dep that resets page state, so manual refresh/source switches invalidate in-flight clicks. - selectedReposKey extracted as buildSelectedReposKey with stability tests; envelope error types wire-tested through the store. * fix(tasks): clamp against the committed count, not the click-time closure Round-3 review: the count promise routinely resolves between click and response, so deciding the end-of-data clamp from the closure value let a stale null overwrite a real count. applyEmptyPageClamp now runs inside the functional updater against the committed value, never raises an earlier clamp, and a window 422 coinciding with a thrown sibling repo resolves as load-failed so the toast and the clamp always agree. * fix(tasks): only an all-window-422 empty page may clamp; harden count merges Round-4 review: a sibling repo's envelope 403/404 arrives with failedCount still 0, so the window branch now requires every error to be the window 422 (non-window validation errors are demoted at the store); the count resolution mins against an applied clamp instead of re-advertising withdrawn pages; the generation effect mirrors taskResumeApplied so its doc claim holds. * fix(tasks): split the proven window limit from the count slot Round-5 review: min-ing the count against an applied clamp pinned a SPECULATIVE end-of-data withdrawal that raced ahead of the count, permanently collapsing the bar for the generation. Proven window-422 limits now live in provenPageLimit (set once, only lowered, reset per generation); the count overwrites its own slot unconditionally; and deriveAdvertisedTotalPages (unit-tested for both arrival orders) caps the count-or-fallback estimate with the proven limit, floored at the loaded pages. * fix(tasks): surface PR-side list failures so they can't read as end-of-data Round-6 review: PartialWorkItemsResult had no PR error slot, so a swallowed gh pr list failure reached the renderer as a clean empty page — and with the count blocked (0) the speculative withdrawal deleted the pagination bar with no toast and no recovery (a regression vs main's silent no-op). PR-side errors now ride the envelope (errors.prs), demoted so they can never join the issue-only window-422 signal; errorTypes replaces issueErrorTypes; an empty page that a real count said should exist now toasts instead of looking dead. * test(tasks): cover the PR-error envelope end-to-end; neutral no-more-results toast Round-7 review: the two literal gh-utils mocks lacked classifyListPrsError (a PR-side rejection in those suites would TypeError instead of assert), and the producer half of the errors.prs contract had no main-side test — added both, plus a classifier contract test pinning the search-window phrase the renderer keys on. The refused-clamp toast now reads the committed count via a synchronous ref mirror instead of the click-time closure, and says 'No more results' — nothing failed on that branch. Both toast keys plus the new one are translated in es/ja/ko/zh. * fix(tasks): preserve final reachable GitHub search page * Extract GitHub search result window error pattern to shared constant Extract the 1000-result window detection pattern to a single source of truth so the classifier and consumer stay synchronized. The pattern is the only signal separating a permanently unreachable page from a transient validation failure, so drift or trimming silently demotes window 422s to generic failures and stops capping the advertised page count (#11485). --------- Co-authored-by: Jinjing <6427696+AmethystLiang@users.noreply.github.com> --- .../client-work-items-query-paging.test.ts | 19 +- src/main/github/client-work-items.test.ts | 3 +- src/main/github/client.ts | 17 +- src/main/github/gh-error-classification.ts | 8 + src/main/github/gh-utils.test.ts | 14 ++ src/main/github/gh-utils.ts | 6 +- src/renderer/src/components/TaskPage.tsx | 123 +++++++++-- .../task-page-work-item-pagination.test.ts | 193 ++++++++++++++++++ .../task-page-work-item-pagination.ts | 121 ++++++++++- src/renderer/src/i18n/locales/en.json | 5 +- src/renderer/src/i18n/locales/es.json | 5 +- src/renderer/src/i18n/locales/ja.json | 5 +- src/renderer/src/i18n/locales/ko.json | 5 +- src/renderer/src/i18n/locales/zh.json | 5 +- .../slices/github-work-items-query-bounds.ts | 1 + src/renderer/src/store/slices/github.test.ts | 139 ++++++++++++- src/renderer/src/store/slices/github.ts | 43 +++- src/shared/github-work-items-query-bounds.ts | 11 + src/shared/types.ts | 1 + 19 files changed, 688 insertions(+), 36 deletions(-) diff --git a/src/main/github/client-work-items-query-paging.test.ts b/src/main/github/client-work-items-query-paging.test.ts index f9cf934e24e..7af6b124c34 100644 --- a/src/main/github/client-work-items-query-paging.test.ts +++ b/src/main/github/client-work-items-query-paging.test.ts @@ -63,7 +63,8 @@ vi.mock('./gh-utils', () => ({ release: releaseMock, _resetOwnerRepoCache: vi.fn(), classifyGhError: (stderr: string) => ({ type: 'unknown', message: stderr }), - classifyListIssuesError: (stderr: string) => ({ type: 'unknown', message: stderr }) + classifyListIssuesError: (stderr: string) => ({ type: 'unknown', message: stderr }), + classifyListPrsError: (stderr: string) => ({ type: 'unknown', message: stderr }) })) vi.mock('../git/runner', () => ({ @@ -266,6 +267,22 @@ describe('listWorkItems query paging', () => { expect(items.map((item) => item.number)).toEqual([4, 3]) }) + it('lifts a swallowed PR-side failure onto errors.prs instead of reading as end-of-data', async () => { + getIssueOwnerRepoMock.mockResolvedValueOnce({ owner: 'acme', repo: 'widgets' }) + getOwnerRepoMock.mockResolvedValueOnce({ owner: 'acme', repo: 'widgets' }) + // Non-availability failure (plain 403): swallowed into [], must still surface. + ghExecFileAsyncMock.mockRejectedValueOnce(new Error('HTTP 403: Forbidden')) + + const envelope = await listWorkItems('/repo-root', 10, 'is:pr is:open', 2) + + expect(envelope.items).toEqual([]) + expect(envelope.errors?.issues).toBeUndefined() + expect(envelope.errors?.prs).toEqual({ + type: 'unknown', + message: expect.stringContaining('HTTP 403') + }) + }) + it('filters pull request rows out of issue Search API results', async () => { getIssueOwnerRepoMock.mockResolvedValueOnce({ owner: 'acme', repo: 'widgets' }) getOwnerRepoMock.mockResolvedValueOnce({ owner: 'acme', repo: 'widgets' }) diff --git a/src/main/github/client-work-items.test.ts b/src/main/github/client-work-items.test.ts index 0226b2af42f..84119b3b2a2 100644 --- a/src/main/github/client-work-items.test.ts +++ b/src/main/github/client-work-items.test.ts @@ -55,7 +55,8 @@ vi.mock('./gh-utils', () => ({ release: releaseMock, _resetOwnerRepoCache: vi.fn(), classifyGhError: (stderr: string) => ({ type: 'unknown', message: stderr }), - classifyListIssuesError: (stderr: string) => ({ type: 'unknown', message: stderr }) + classifyListIssuesError: (stderr: string) => ({ type: 'unknown', message: stderr }), + classifyListPrsError: (stderr: string) => ({ type: 'unknown', message: stderr }) })) vi.mock('../git/runner', () => ({ diff --git a/src/main/github/client.ts b/src/main/github/client.ts index 3684d7729b1..24772ebee4b 100644 --- a/src/main/github/client.ts +++ b/src/main/github/client.ts @@ -55,6 +55,7 @@ import { release, classifyGhError, classifyListIssuesError, + classifyListPrsError, ghRepoExecOptions, githubRepoContext, getRemoteUrlForRepo, @@ -1122,10 +1123,11 @@ function buildWorkItemListRequest(args: { return { args: out, offset: (page - 1) * limit } } -// Why: shared shape so listWorkItems can lift the issue-side error (#1076 silent wrongness) into the IPC envelope; PR errors out of scope (§6). +// Why: shared shape so listWorkItems can lift per-side errors (#1076 silent wrongness) into the IPC envelope — a swallowed side reads as end-of-data to pagination (#11485). type PartialWorkItemsResult = { items: MainWorkItem[] issuesError?: ClassifiedError + prsError?: ClassifiedError } function assertSshRepoHasResolvedGitHubSource(args: { @@ -1279,6 +1281,7 @@ async function listQueriedWorkItems( let successfulRequestCount = 0 let nonAvailabilityFailureCount = 0 let availabilityError: unknown + let prsError: ClassifiedError | undefined // Why: surface the issue-side error separately for the IPC envelope; PR-side keeps prior swallow-and-log (parent doc §6). const issueFetch = (async (): Promise => { @@ -1350,6 +1353,7 @@ async function listQueriedWorkItems( } catch (err) { console.warn('listQueriedWorkItems PRs partial failure:', err) const stderr = err instanceof Error ? err.message : String(err) + prsError = classifyListPrsError(stderr) if (classifyGitHubUnavailable(stderr)) { availabilityError ??= err } else { @@ -1366,7 +1370,8 @@ async function listQueriedWorkItems( } return { items: sortWorkItemsByNumber([...issueResult.items, ...prItems]).slice(0, limit), - issuesError: issueResult.issuesError + issuesError: issueResult.issuesError, + prsError } } @@ -1424,7 +1429,13 @@ export async function listWorkItems( localGitOptions ) - const errors = partial.issuesError ? { issues: partial.issuesError } : undefined + const errors = + partial.issuesError || partial.prsError + ? { + ...(partial.issuesError ? { issues: partial.issuesError } : {}), + ...(partial.prsError ? { prs: partial.prsError } : {}) + } + : undefined return { items: partial.items, sources: { diff --git a/src/main/github/gh-error-classification.ts b/src/main/github/gh-error-classification.ts index 2de37919d25..4d2453899ac 100644 --- a/src/main/github/gh-error-classification.ts +++ b/src/main/github/gh-error-classification.ts @@ -55,3 +55,11 @@ export function classifyListIssuesError(stderr: string): ClassifiedError { } return { type: c.type, message: readMessages[c.type] } } + +// Why: PR-side list failures need the same read-op classification — pagination +// decisions key on the type, and swallowing them made failures look like +// end-of-data (#11485). +export function classifyListPrsError(stderr: string): ClassifiedError { + const c = classifyGhError(stderr) + return { type: c.type, message: `Failed to load pull requests: ${stderr.trim()}` } +} diff --git a/src/main/github/gh-utils.test.ts b/src/main/github/gh-utils.test.ts index 0d7329e21e3..ca9098e53bf 100644 --- a/src/main/github/gh-utils.test.ts +++ b/src/main/github/gh-utils.test.ts @@ -38,6 +38,7 @@ import { __resetLocalGitConfigSignatureCacheForTests, readLocalGitConfigSignature } from './local-git-config-signature' +import { GITHUB_SEARCH_RESULT_WINDOW_ERROR_PATTERN } from '../../shared/github-work-items-query-bounds' describe('github owner/repo resolution', () => { beforeEach(() => { @@ -807,4 +808,17 @@ describe('gh error classification', () => { message: 'Issues are disabled on this repository.' }) }) + + // Why: the renderer detects the Search API 1000-result window by matching + // GITHUB_SEARCH_RESULT_WINDOW_ERROR_PATTERN against this message (#11485) — + // trimming the raw stderr out of the validation_error copy, or drifting the + // pattern off GitHub's real wording, would silently downgrade every window + // 422 to a generic failure. The stderr stays verbatim so this pins both ends. + it('keeps the search-window phrase in validation_error list messages', () => { + const stderr = + 'Command failed: gh api --hostname github.com search/issues\nValidation Failed: Only the first 1000 search results are available (HTTP 422)' + const classified = classifyListIssuesError(stderr) + expect(classified.type).toBe('validation_error') + expect(classified.message).toMatch(GITHUB_SEARCH_RESULT_WINDOW_ERROR_PATTERN) + }) }) diff --git a/src/main/github/gh-utils.ts b/src/main/github/gh-utils.ts index d2148edc3c1..d6770189924 100644 --- a/src/main/github/gh-utils.ts +++ b/src/main/github/gh-utils.ts @@ -9,7 +9,11 @@ import { extractExecError, parseRetryAfterMs } from '../git/exec-error' // WSL-aware routing. Repo-scoped callers should use the runner exports below. export const execFileAsync = promisify(execFile) export { ghExecFileAsync, gitExecFileAsync, extractExecError, parseRetryAfterMs } -export { classifyGhError, classifyListIssuesError } from './gh-error-classification' +export { + classifyGhError, + classifyListIssuesError, + classifyListPrsError +} from './gh-error-classification' export { _getOwnerRepoCacheSize, _resetOwnerRepoCache, diff --git a/src/renderer/src/components/TaskPage.tsx b/src/renderer/src/components/TaskPage.tsx index 3ff8a909b7e..99d0785c795 100644 --- a/src/renderer/src/components/TaskPage.tsx +++ b/src/renderer/src/components/TaskPage.tsx @@ -205,7 +205,12 @@ import { } from '@/components/task-page-cache-selectors' import { shouldHideTaskPageListChrome } from '@/components/task-page-list-chrome-visibility' import { + applyEmptyPageClamp, + applyWindowPageLimit, + buildSelectedReposKey, + deriveAdvertisedTotalPages, getTaskPagePerRepoLimit, + resolveEmptyPageOutcome, taskPageToGitHubApiPage } from '@/components/task-page-work-item-pagination' import { sortWorkItemsByNumber } from '../../../shared/work-items' @@ -3175,6 +3180,18 @@ export default function TaskPage(): React.JSX.Element { [eligibleRepos, repoSelection] ) + // Why: see buildSelectedReposKey — array-identity deps re-fire on every + // repos:changed even when the selection is unchanged. The context part is + // resolved as GitHub, but every provider-independent field (projectId, + // hostId, projectHostSetupId, repoId) is identical across providers, so the + // GitLab effect can key off this too — it passes no gitlabProjectRef, so its + // context carries no providerIdentity of its own. Thread a projectRef into + // that call and this key needs a GitLab-scoped part. + const selectedReposKey = useMemo( + () => buildSelectedReposKey(selectedRepos, (r) => getTaskPageRepoSourceContext(r, 'github')), + [selectedRepos] + ) + // Why: many affordances need *a* repo; use the first selected as default, while cross-repo dialogs still let the user override per-action. const primaryRepo = selectedRepos[0] ?? null const linearWorkspaces = linearStatus.workspaces ?? [] @@ -3777,14 +3794,34 @@ export default function TaskPage(): React.JSX.Element { const [paginationLoading, setPaginationLoading] = useState(false) const [loadingTargetPage, setLoadingTargetPage] = useState(null) const [countedTotalPages, setCountedTotalPages] = useState(null) + // Proven window-422 page limit — separate from the count so a late count + // can't resurrect proven-unreachable pages, nor be pinned by a speculative + // withdrawal (see deriveAdvertisedTotalPages). + const [provenPageLimit, setProvenPageLimit] = useState(null) + // Why: synchronous mirror of countedTotalPages — the empty-page branch needs + // the committed value, not a click-time closure, and refs update immediately. + const countedTotalPagesRef = useRef(null) const fetchWorkItemsNextPage = useAppStore((s) => s.fetchWorkItemsNextPage) const countWorkItemsAcrossRepos = useAppStore((s) => s.countWorkItemsAcrossRepos) + // Why: keyed on selectedReposKey, not the selectedRepos array — a background + // repos:changed refresh mid-flight would otherwise bump the generation and + // silently discard the user's page navigation (#11485). Mirrors every dep of + // the fetch effect that resets page state, so a reset always invalidates + // in-flight page requests. useEffect(() => { paginationGenerationRef.current += 1 setPaginationLoading(false) setLoadingTargetPage(null) - }, [selectedRepos, appliedTaskSearch, workItemsInvalidationNonce]) + }, [ + selectedReposKey, + appliedTaskSearch, + workItemsInvalidationNonce, + taskRefreshNonce, + taskSource, + githubMode, + taskResumeApplied + ]) // Why: the dialog's "Use" button routes through the same direct-launch flow as the row-level "Use" CTA so behavior is consistent regardless of entry point. const githubTaskDrawerWorkItem = useAppStore((s) => s.githubTaskDrawerWorkItem) @@ -4799,15 +4836,6 @@ export default function TaskPage(): React.JSX.Element { jiraTaskSourceContext ]) - // Why: stable string key for selectedRepos so the GitLab effect doesn't re-run on every parent render from a new array ref. - const selectedReposKey = useMemo( - () => - selectedRepos - .map((r) => `${r.id}|${r.path}|${r.connectionId ?? ''}|${r.executionHostId ?? ''}`) - .join(','), - [selectedRepos] - ) - // Why: fetch GitLab Issues and MRs separately so errors stay isolated per tab (mirrors GitHub's split endpoints). useEffect(() => { if (taskSource !== 'gitlab') { @@ -4914,7 +4942,7 @@ export default function TaskPage(): React.JSX.Element { return () => { stale = true } - // eslint-disable-next-line react-hooks/exhaustive-deps -- selectedReposKey encodes the only selectedRepos fields read above; keying off the array ref would re-run on every parent render. + // eslint-disable-next-line react-hooks/exhaustive-deps -- selectedReposKey covers every selectedRepos field read above (see its GitHub-scoped-context note); keying off the array ref would re-run on every parent render. }, [taskSource, gitlabView, activeGitlabFilter, gitlabRefreshNonce, selectedReposKey]) // Why: Todos fetch has its own effect — different trigger (no chip filter) and data path (gl.todos is user-scoped, not repo-scoped). @@ -6111,10 +6139,12 @@ export default function TaskPage(): React.JSX.Element { const fallbackTotalPages = lastLoadedPageFull ? Math.max(pages.length, lastLoadedPageIndex + 2) : Math.max(1, pages.length) - const totalPages = - countedTotalPages && countedTotalPages > 0 - ? Math.max(pages.length, countedTotalPages) - : fallbackTotalPages + const totalPages = deriveAdvertisedTotalPages({ + loadedPages: pages.length, + countedTotalPages, + fallbackTotalPages, + provenPageLimit + }) // Why: load only the clicked page so a high-page jump doesn't exhaust GitHub's Search API rate bucket. const handleLoadNextPage = useCallback( @@ -6135,7 +6165,7 @@ export default function TaskPage(): React.JSX.Element { setPaginationLoading(true) setLoadingTargetPage(target) try { - const { items } = await fetchWorkItemsNextPage( + const { items, failedCount, errorTypes } = await fetchWorkItemsNextPage( repoArgs, githubPerRepoPageLimit, githubPageSize, @@ -6146,6 +6176,55 @@ export default function TaskPage(): React.JSX.Element { return } if (items.length === 0) { + // Why: see resolveEmptyPageOutcome — a dead click needs feedback only + // when something actually failed; a clean empty probe is end-of-data. + // The reason never depends on the count, so it's safe to derive here; + // the clamp is not (see applyEmptyPageClamp) and runs in the updater. + const { reason } = resolveEmptyPageOutcome({ + target, + failedCount, + errorTypes, + countedTotalPages: null + }) + if (reason === 'window-unreachable') { + toast.error( + translate( + 'auto.components.TaskPage.loadPageUnreachable', + 'Page {{value0}} is beyond what GitHub search can return.', + { value0: String(target + 1) } + ), + { id: 'work-items-page-unreachable' } + ) + setProvenPageLimit((previous) => applyWindowPageLimit(previous, target)) + } else if (reason === 'load-failed') { + toast.error( + translate( + 'auto.components.TaskPage.loadPageFailed', + 'Page {{value0}} could not be loaded from GitHub.', + { value0: String(target + 1) } + ), + { id: 'work-items-page-load-failed' } + ) + } else { + // Why: with a real count the clamp is refused, so without feedback + // the click would look dead — the count over-advertised; nothing + // failed, so the copy stays neutral. The ref carries the committed + // count, immune to the click-time closure race. + const committedCount = countedTotalPagesRef.current + if (committedCount !== null && committedCount > 0) { + toast( + translate( + 'auto.components.TaskPage.loadPageNoMoreResults', + 'No more results on page {{value0}}.', + { value0: String(target + 1) } + ), + { id: 'work-items-page-no-more-results' } + ) + } + const next = applyEmptyPageClamp(committedCount, { target, failedCount, errorTypes }) + countedTotalPagesRef.current = next + setCountedTotalPages(next) + } return } setPages((previous) => { @@ -6253,6 +6332,8 @@ export default function TaskPage(): React.JSX.Element { setPages([page0]) setCurrentPage(0) setCountedTotalPages(null) + countedTotalPagesRef.current = null + setProvenPageLimit(null) setTasksError(null) setFailedCount(0) // reset so a prior failure banner doesn't linger setGithubUnavailable(false) @@ -6357,6 +6438,10 @@ export default function TaskPage(): React.JSX.Element { githubPerRepoPageLimit ).then(({ totalPages: countedPages }) => { if (!cancelled) { + // Why: the count overwrites unconditionally — proven window limits live + // in provenPageLimit, so a late count can't be pinned by a speculative + // end-of-data withdrawal, and can't resurrect proven-dead pages either. + countedTotalPagesRef.current = countedPages setCountedTotalPages(countedPages) } }) @@ -6365,9 +6450,13 @@ export default function TaskPage(): React.JSX.Element { cancelled = true } // Why: store selectors are stable (omit from deps); workItemsInvalidationNonce included so a preference flip re-dispatches. + // selectedReposKey stands in for selectedRepos — the array gets a fresh + // identity on every repos:changed event, and re-running this effect then + // resets pagination mid-click (#11485). The key covers every repo field the + // requests read. // eslint-disable-next-line react-hooks/exhaustive-deps }, [ - selectedRepos, + selectedReposKey, appliedTaskSearch, taskRefreshNonce, taskSource, diff --git a/src/renderer/src/components/task-page-work-item-pagination.test.ts b/src/renderer/src/components/task-page-work-item-pagination.test.ts index 22ad6bf6cde..670c862be98 100644 --- a/src/renderer/src/components/task-page-work-item-pagination.test.ts +++ b/src/renderer/src/components/task-page-work-item-pagination.test.ts @@ -2,7 +2,12 @@ import { describe, expect, it, vi } from 'vitest' import type { GitHubWorkItem } from '../../../shared/types' import { accumulateWorkItemPages, + applyEmptyPageClamp, + applyWindowPageLimit, + buildSelectedReposKey, + deriveAdvertisedTotalPages, getTaskPagePerRepoLimit, + resolveEmptyPageOutcome, taskPageToGitHubApiPage, workItemIdentity } from './task-page-work-item-pagination' @@ -59,6 +64,194 @@ describe('numbered GitHub pagination', () => { }) }) +describe('resolveEmptyPageOutcome', () => { + const base = { failedCount: 0, errorTypes: [] as const, countedTotalPages: null } + + it('clamps and reports unreachable when the search window 422 is present', () => { + expect( + resolveEmptyPageOutcome({ ...base, target: 33, errorTypes: ['validation_error'] }) + ).toEqual({ reason: 'window-unreachable', clampTotalPagesTo: 33 }) + // A real count is still clamped — the count over-advertising is the bug. + expect( + resolveEmptyPageOutcome({ + ...base, + target: 33, + errorTypes: ['validation_error'], + countedTotalPages: 39 + }) + ).toEqual({ reason: 'window-unreachable', clampTotalPagesTo: 33 }) + }) + + it('resolves as load-failed when a window 422 coincides with a sibling failure', () => { + // Repos advance independently; another repo's failure — thrown or on the + // envelope channel — says nothing about the healthy repos' remaining + // pages, and the transient failure is the actionable signal. + expect( + resolveEmptyPageOutcome({ + ...base, + target: 5, + failedCount: 1, + errorTypes: ['validation_error'] + }) + ).toEqual({ reason: 'load-failed', clampTotalPagesTo: null }) + expect( + resolveEmptyPageOutcome({ + ...base, + target: 5, + errorTypes: ['validation_error', 'permission_denied'] + }) + ).toEqual({ reason: 'load-failed', clampTotalPagesTo: null }) + }) + + it('reports a failure without clamping for transient or unclassified errors', () => { + expect(resolveEmptyPageOutcome({ ...base, target: 5, failedCount: 2 })).toEqual({ + reason: 'load-failed', + clampTotalPagesTo: null + }) + for (const type of ['permission_denied', 'not_found', 'rate_limited', 'unknown'] as const) { + expect(resolveEmptyPageOutcome({ ...base, target: 5, errorTypes: [type] })).toEqual({ + reason: 'load-failed', + clampTotalPagesTo: null + }) + } + }) + + it('withdraws the speculative page on clean empty only while the count is unknown', () => { + expect(resolveEmptyPageOutcome({ ...base, target: 2 })).toEqual({ + reason: 'end-of-data', + clampTotalPagesTo: 2 + }) + // 0 means the count itself failed — treat like unknown. + expect(resolveEmptyPageOutcome({ ...base, target: 2, countedTotalPages: 0 })).toEqual({ + reason: 'end-of-data', + clampTotalPagesTo: 2 + }) + // A real count must not shrink: the PR list path swallows its own failures + // into clean-empty results, and clamping would hide healthy pages silently. + expect(resolveEmptyPageOutcome({ ...base, target: 2, countedTotalPages: 34 })).toEqual({ + reason: 'end-of-data', + clampTotalPagesTo: null + }) + // Never clamps below one advertised page. + expect(resolveEmptyPageOutcome({ ...base, target: 0 })).toEqual({ + reason: 'end-of-data', + clampTotalPagesTo: 1 + }) + }) +}) + +describe('applyEmptyPageClamp', () => { + const clean = { failedCount: 0, errorTypes: [] as const } + + it('keeps a real count that resolved between click and response', () => { + // The count promise routinely lands mid-flight; the committed value, not + // the click-time closure, must decide whether end-of-data may clamp. + expect(applyEmptyPageClamp(33, { ...clean, target: 2 })).toBe(33) + }) + + it('withdraws the speculative page while the count is unknown or failed', () => { + expect(applyEmptyPageClamp(null, { ...clean, target: 2 })).toBe(2) + expect(applyEmptyPageClamp(0, { ...clean, target: 2 })).toBe(2) + }) + + it('never touches the count for window or failure outcomes', () => { + // Window 422 limits live in provenPageLimit (applyWindowPageLimit), not + // the count slot. + expect( + applyEmptyPageClamp(39, { target: 33, failedCount: 0, errorTypes: ['validation_error'] }) + ).toBe(39) + expect(applyEmptyPageClamp(33, { target: 5, failedCount: 2, errorTypes: [] })).toBe(33) + expect(applyEmptyPageClamp(null, { target: 5, failedCount: 2, errorTypes: [] })).toBe(null) + }) +}) + +describe('applyWindowPageLimit', () => { + it('sets, only lowers, and never goes below one page', () => { + expect(applyWindowPageLimit(null, 33)).toBe(33) + expect(applyWindowPageLimit(33, 20)).toBe(20) + expect(applyWindowPageLimit(20, 33)).toBe(20) + expect(applyWindowPageLimit(null, 0)).toBe(1) + }) +}) + +describe('deriveAdvertisedTotalPages', () => { + it('is order-independent: a count landing after a speculative withdrawal wins', () => { + // Probe first: end-of-data wrote countedTotalPages=1 (speculative). Count + // then overwrites with 39 — the bar must show 39, not stay collapsed. + expect( + deriveAdvertisedTotalPages({ + loadedPages: 1, + countedTotalPages: 39, + fallbackTotalPages: 2, + provenPageLimit: null + }) + ).toBe(39) + }) + + it('caps a real count with the proven window limit', () => { + expect( + deriveAdvertisedTotalPages({ + loadedPages: 3, + countedTotalPages: 39, + fallbackTotalPages: 4, + provenPageLimit: 33 + }) + ).toBe(33) + }) + + it('uses the fallback while the count is unknown/failed, still window-capped', () => { + expect( + deriveAdvertisedTotalPages({ + loadedPages: 1, + countedTotalPages: null, + fallbackTotalPages: 2, + provenPageLimit: null + }) + ).toBe(2) + expect( + deriveAdvertisedTotalPages({ + loadedPages: 1, + countedTotalPages: 0, + fallbackTotalPages: 5, + provenPageLimit: 3 + }) + ).toBe(3) + }) + + it('never advertises fewer pages than are loaded', () => { + expect( + deriveAdvertisedTotalPages({ + loadedPages: 5, + countedTotalPages: 2, + fallbackTotalPages: 2, + provenPageLimit: 2 + }) + ).toBe(5) + }) +}) + +describe('buildSelectedReposKey', () => { + const contextFor = (r: { id: string }) => ({ provider: 'github', repoId: r.id }) + + it('is stable across fresh arrays with identical contents', () => { + const a = [{ id: 'r1', path: '/a', connectionId: null, executionHostId: null }] + const b = [{ ...a[0] }] + expect(buildSelectedReposKey(a, contextFor)).toBe(buildSelectedReposKey(b, contextFor)) + }) + + it('changes when any request-relevant field changes', () => { + const repo = { id: 'r1', path: '/a', connectionId: null, executionHostId: null } + const key = buildSelectedReposKey([repo], contextFor) + expect(buildSelectedReposKey([{ ...repo, executionHostId: 'ssh:host' }], contextFor)).not.toBe( + key + ) + expect(buildSelectedReposKey([{ ...repo, path: '/b' }], contextFor)).not.toBe(key) + expect( + buildSelectedReposKey([repo], (r) => ({ provider: 'github', repoId: r.id, owner: 'new' })) + ).not.toBe(key) + }) +}) + describe('accumulateWorkItemPages', () => { it('drops the re-fetched boundary row that shares the previous page cursor', async () => { const boundary = item('r', 'issue:2', '2026-07-02') diff --git a/src/renderer/src/components/task-page-work-item-pagination.ts b/src/renderer/src/components/task-page-work-item-pagination.ts index e8f0a8749e3..8209b61bdf0 100644 --- a/src/renderer/src/components/task-page-work-item-pagination.ts +++ b/src/renderer/src/components/task-page-work-item-pagination.ts @@ -1,4 +1,4 @@ -import type { GitHubWorkItem } from '../../../shared/types' +import type { ClassifiedError, GitHubWorkItem } from '../../../shared/types' /** * Cross-repo Tasks pagination is cursor-based on `updatedAt`: each page's oldest @@ -35,6 +35,125 @@ export function taskPageToGitHubApiPage(taskPage: number): number { return Math.max(0, Math.floor(taskPage)) + 1 } +export type EmptyPageOutcome = { + reason: 'window-unreachable' | 'load-failed' | 'end-of-data' + /** New advertised page count, or null to leave the current count alone. + * Window clamps are applied via applyWindowPageLimit — this field carries + * the same value there for symmetry, but the count slot must not use it. */ + clampTotalPagesTo: number | null +} + +/** + * An empty page load has three distinct meanings, and only the caller-side + * error channels can tell them apart (#11485): + * - a `validation_error` is GitHub's 422 for pages past its 1000-result search + * window — the page can never load, so stop advertising it. When a sibling + * repo's fetch also threw, the transient failure wins (load-failed, no + * clamp) so the toast and the clamp never disagree. The window signal is + * issue-side only: PR-side errors arrive demoted (never validation_error). + * - any other per-repo error (thrown, or on either envelope channel) may be + * transient (rate limit, permissions), so surface it but keep the count; + * - no error at all is end-of-data. That only warrants a clamp while the count + * is unknown (null) or failed (0), to withdraw the speculative page + * `fallbackTotalPages` advertises — a real count must not shrink. + */ +export function resolveEmptyPageOutcome(args: { + target: number + failedCount: number + errorTypes: readonly ClassifiedError['type'][] + countedTotalPages: number | null +}): EmptyPageOutcome { + const clamp = Math.max(1, Math.floor(args.target)) + // Why: every error must be the window 422 — a sibling repo's envelope + // 403/404 alongside it means a repo that may still have pages, so the + // transient-failure branch must win the toast and block the clamp. + const onlyWindowErrors = + args.errorTypes.length > 0 && args.errorTypes.every((type) => type === 'validation_error') + if (onlyWindowErrors && args.failedCount === 0) { + return { reason: 'window-unreachable', clampTotalPagesTo: clamp } + } + if (args.failedCount > 0 || args.errorTypes.length > 0) { + return { reason: 'load-failed', clampTotalPagesTo: null } + } + const countUnknown = args.countedTotalPages === null || args.countedTotalPages === 0 + return { reason: 'end-of-data', clampTotalPagesTo: countUnknown ? clamp : null } +} + +/** + * Functional-updater body for the SPECULATIVE end-of-data withdrawal only. + * Must be evaluated against the COMMITTED count (React updater `previous`), + * not a click-time closure — the count promise routinely resolves between + * click and response. Window 422 clamps are PROVEN and live in their own + * state (applyWindowPageLimit); keeping them out of the count slot lets a + * later real count overwrite the speculative value instead of being pinned + * under it. + */ +export function applyEmptyPageClamp( + previous: number | null, + args: { + target: number + failedCount: number + errorTypes: readonly ClassifiedError['type'][] + } +): number | null { + const outcome = resolveEmptyPageOutcome({ ...args, countedTotalPages: previous }) + if (outcome.reason !== 'end-of-data' || outcome.clampTotalPagesTo === null) { + return previous + } + return outcome.clampTotalPagesTo +} + +/** Proven window 422 limit: set once, only ever lowered, reset per generation. */ +export function applyWindowPageLimit(previous: number | null, target: number): number { + const clamp = Math.max(1, Math.floor(target)) + return previous === null ? clamp : Math.min(previous, clamp) +} + +/** + * Advertised page count = the count-or-fallback estimate, capped by the proven + * window limit, floored at the loaded pages. Splitting the proven cap from the + * count slot makes the result order-independent: a count arriving after a + * speculative withdrawal overwrites it, while a proven limit survives. + */ +export function deriveAdvertisedTotalPages(args: { + loadedPages: number + countedTotalPages: number | null + fallbackTotalPages: number + provenPageLimit: number | null +}): number { + const uncapped = + args.countedTotalPages && args.countedTotalPages > 0 + ? Math.max(args.loadedPages, args.countedTotalPages) + : args.fallbackTotalPages + const capped = args.provenPageLimit === null ? uncapped : Math.min(uncapped, args.provenPageLimit) + return Math.max(args.loadedPages, capped) +} + +/** + * Stable identity string for a repo selection. The repos store installs a fresh + * array on every repos:changed event, so effects keyed on array identity re-fire + * with the selection unchanged — resetting pagination mid-click (#11485). The + * key must cover every repo field the work-item requests read; the caller + * supplies the resolved source context (which must stay free of timestamps). + */ +export function buildSelectedReposKey< + T extends { + id: string + path: string + connectionId?: string | null + executionHostId?: string | null + } +>(repos: readonly T[], sourceContextFor: (repo: T) => unknown): string { + return repos + .map( + (r) => + `${r.id}|${r.path}|${r.connectionId ?? ''}|${r.executionHostId ?? ''}|${JSON.stringify( + sourceContextFor(r) + )}` + ) + .join(',') +} + // Why: provider pages cannot spill truncated rows into the next page. Divide // the display budget up front so every fetched row remains reachable. export function getTaskPagePerRepoLimit( diff --git a/src/renderer/src/i18n/locales/en.json b/src/renderer/src/i18n/locales/en.json index 641b9b018a7..7e276012c36 100644 --- a/src/renderer/src/i18n/locales/en.json +++ b/src/renderer/src/i18n/locales/en.json @@ -1897,7 +1897,10 @@ "75a38d7df8": "GitHub data is temporarily unavailable. Its API may be down, rate-limited, or unreachable. Please try again shortly.", "noGithubSourceDetected": "No GitHub source detected for", "noGithubSourceDetectedHint": "it may have no GitHub remote, or the source could not be resolved.", - "jiraLinkSourceUnavailable": "Couldn’t link this Jira issue. Reconnect Jira or pick the matching site, then try again." + "jiraLinkSourceUnavailable": "Couldn’t link this Jira issue. Reconnect Jira or pick the matching site, then try again.", + "loadPageUnreachable": "Page {{value0}} is beyond what GitHub search can return.", + "loadPageFailed": "Page {{value0}} could not be loaded from GitHub.", + "loadPageNoMoreResults": "No more results on page {{value0}}." }, "Terminal": { "73768427cf": "Close", diff --git a/src/renderer/src/i18n/locales/es.json b/src/renderer/src/i18n/locales/es.json index f251c3e0fa9..02bc8bf3b73 100644 --- a/src/renderer/src/i18n/locales/es.json +++ b/src/renderer/src/i18n/locales/es.json @@ -1856,7 +1856,10 @@ "75a38d7df8": "GitHub data is temporarily unavailable. Its API may be down, rate-limited, or unreachable. Please try again shortly.", "noGithubSourceDetected": "No GitHub source detected for", "noGithubSourceDetectedHint": "it may have no GitHub remote, or the source could not be resolved.", - "jiraLinkSourceUnavailable": "Couldn’t link this Jira issue. Reconnect Jira or pick the matching site, then try again." + "jiraLinkSourceUnavailable": "Couldn’t link this Jira issue. Reconnect Jira or pick the matching site, then try again.", + "loadPageUnreachable": "La página {{value0}} está más allá de lo que la búsqueda de GitHub puede devolver.", + "loadPageFailed": "No se pudo cargar la página {{value0}} desde GitHub.", + "loadPageNoMoreResults": "No hay más resultados en la página {{value0}}." }, "Terminal": { "73768427cf": "Cerrar", diff --git a/src/renderer/src/i18n/locales/ja.json b/src/renderer/src/i18n/locales/ja.json index 0e53b3b1e29..b7ce3fcef07 100644 --- a/src/renderer/src/i18n/locales/ja.json +++ b/src/renderer/src/i18n/locales/ja.json @@ -1856,7 +1856,10 @@ "75a38d7df8": "GitHub data is temporarily unavailable. Its API may be down, rate-limited, or unreachable. Please try again shortly.", "noGithubSourceDetected": "No GitHub source detected for", "noGithubSourceDetectedHint": "it may have no GitHub remote, or the source could not be resolved.", - "jiraLinkSourceUnavailable": "Couldn’t link this Jira issue. Reconnect Jira or pick the matching site, then try again." + "jiraLinkSourceUnavailable": "Couldn’t link this Jira issue. Reconnect Jira or pick the matching site, then try again.", + "loadPageUnreachable": "ページ {{value0}} は GitHub 検索が返せる範囲を超えています。", + "loadPageFailed": "GitHub からページ {{value0}} を読み込めませんでした。", + "loadPageNoMoreResults": "ページ {{value0}} にはこれ以上結果がありません。" }, "Terminal": { "73768427cf": "閉じる", diff --git a/src/renderer/src/i18n/locales/ko.json b/src/renderer/src/i18n/locales/ko.json index d44e2d7501f..aa9b6bf609d 100644 --- a/src/renderer/src/i18n/locales/ko.json +++ b/src/renderer/src/i18n/locales/ko.json @@ -1856,7 +1856,10 @@ "75a38d7df8": "GitHub data is temporarily unavailable. Its API may be down, rate-limited, or unreachable. Please try again shortly.", "noGithubSourceDetected": "No GitHub source detected for", "noGithubSourceDetectedHint": "it may have no GitHub remote, or the source could not be resolved.", - "jiraLinkSourceUnavailable": "Couldn’t link this Jira issue. Reconnect Jira or pick the matching site, then try again." + "jiraLinkSourceUnavailable": "Couldn’t link this Jira issue. Reconnect Jira or pick the matching site, then try again.", + "loadPageUnreachable": "{{value0}} 페이지는 GitHub 검색이 반환할 수 있는 범위를 벗어났습니다.", + "loadPageFailed": "GitHub에서 {{value0}} 페이지를 불러오지 못했습니다.", + "loadPageNoMoreResults": "{{value0}} 페이지에 더 이상 결과가 없습니다." }, "Terminal": { "73768427cf": "닫기", diff --git a/src/renderer/src/i18n/locales/zh.json b/src/renderer/src/i18n/locales/zh.json index 626cbcc869b..cfc09f5aee2 100644 --- a/src/renderer/src/i18n/locales/zh.json +++ b/src/renderer/src/i18n/locales/zh.json @@ -1856,7 +1856,10 @@ "75a38d7df8": "GitHub data is temporarily unavailable. Its API may be down, rate-limited, or unreachable. Please try again shortly.", "noGithubSourceDetected": "No GitHub source detected for", "noGithubSourceDetectedHint": "it may have no GitHub remote, or the source could not be resolved.", - "jiraLinkSourceUnavailable": "Couldn’t link this Jira issue. Reconnect Jira or pick the matching site, then try again." + "jiraLinkSourceUnavailable": "Couldn’t link this Jira issue. Reconnect Jira or pick the matching site, then try again.", + "loadPageUnreachable": "第 {{value0}} 页超出了 GitHub 搜索可返回的范围。", + "loadPageFailed": "无法从 GitHub 加载第 {{value0}} 页。", + "loadPageNoMoreResults": "第 {{value0}} 页没有更多结果。" }, "Terminal": { "73768427cf": "关闭", diff --git a/src/renderer/src/store/slices/github-work-items-query-bounds.ts b/src/renderer/src/store/slices/github-work-items-query-bounds.ts index c60529c3e57..0ac2107a8a2 100644 --- a/src/renderer/src/store/slices/github-work-items-query-bounds.ts +++ b/src/renderer/src/store/slices/github-work-items-query-bounds.ts @@ -1,4 +1,5 @@ export { + GITHUB_SEARCH_RESULT_WINDOW_ERROR_PATTERN, GITHUB_WORK_ITEMS_QUERY_MAX_BYTES, isGitHubWorkItemsQueryTooLarge } from '../../../../shared/github-work-items-query-bounds' diff --git a/src/renderer/src/store/slices/github.test.ts b/src/renderer/src/store/slices/github.test.ts index 91269b0b310..13f72009428 100644 --- a/src/renderer/src/store/slices/github.test.ts +++ b/src/renderer/src/store/slices/github.test.ts @@ -6656,10 +6656,108 @@ describe('createGitHubSlice.fetchWorkItems source/error envelope', () => { }) expect(result).toEqual({ items: [{ ...item, repoId: 'caller-repo-id' }], - failedCount: 0 + failedCount: 0, + errorTypes: [] }) }) + it('surfaces issue-side envelope errors as errorTypes on next-page fetches', async () => { + runtimeEnvironmentCall.mockResolvedValueOnce({ + id: 'rpc-work-items-page-422', + ok: true, + result: { + items: [], + sources: { + issues: { owner: 'up', repo: 'r' }, + prs: null, + originCandidate: { owner: 'up', repo: 'r' }, + upstreamCandidate: null + }, + errors: { + issues: { type: 'validation_error', message: 'only the first 1000 search results' } + } + }, + _meta: { runtimeId: 'remote-runtime' } + }) + const store = createTestStore() + store.setState({ + settings: { activeRuntimeEnvironmentId: 'env-1' }, + repos: [{ id: 'runtime-repo-id', path: '/server/repo', name: 'repo', kind: 'git' }] + } as unknown as Partial) + + const result = await store + .getState() + .fetchWorkItemsNextPage([{ repoId: 'caller-repo-id', path: '/server/repo' }], 24, 100, '', 34) + + // The window 422 travels on the envelope error channel, not failedCount — + // resolveEmptyPageOutcome keys on this exact string (#11485). + expect(result).toEqual({ items: [], failedCount: 0, errorTypes: ['validation_error'] }) + }) + + it('demotes non-window validation errors so they cannot drive the unreachable clamp', async () => { + runtimeEnvironmentCall.mockResolvedValueOnce({ + id: 'rpc-work-items-page-422-other', + ok: true, + result: { + items: [], + sources: { + issues: { owner: 'up', repo: 'r' }, + prs: null, + originCandidate: { owner: 'up', repo: 'r' }, + upstreamCandidate: null + }, + errors: { + issues: { type: 'validation_error', message: 'Validation Failed: query is malformed' } + } + }, + _meta: { runtimeId: 'remote-runtime' } + }) + const store = createTestStore() + store.setState({ + settings: { activeRuntimeEnvironmentId: 'env-1' }, + repos: [{ id: 'runtime-repo-id', path: '/server/repo', name: 'repo', kind: 'git' }] + } as unknown as Partial) + + const result = await store + .getState() + .fetchWorkItemsNextPage([{ repoId: 'caller-repo-id', path: '/server/repo' }], 24, 100, '', 2) + + expect(result).toEqual({ items: [], failedCount: 0, errorTypes: ['unknown'] }) + }) + + it('surfaces PR-side envelope errors demoted so they read as failures, never window 422s', async () => { + runtimeEnvironmentCall.mockResolvedValueOnce({ + id: 'rpc-work-items-page-prs-error', + ok: true, + result: { + items: [], + sources: { + issues: null, + prs: { owner: 'up', repo: 'r' }, + originCandidate: { owner: 'up', repo: 'r' }, + upstreamCandidate: null + }, + errors: { + prs: { type: 'validation_error', message: 'Failed to load pull requests: bad flag' } + } + }, + _meta: { runtimeId: 'remote-runtime' } + }) + const store = createTestStore() + store.setState({ + settings: { activeRuntimeEnvironmentId: 'env-1' }, + repos: [{ id: 'runtime-repo-id', path: '/server/repo', name: 'repo', kind: 'git' }] + } as unknown as Partial) + + const result = await store + .getState() + .fetchWorkItemsNextPage([{ repoId: 'caller-repo-id', path: '/server/repo' }], 24, 100, '', 2) + + // A swallowed PR-side failure must not read as end-of-data (#11485), and a + // PR-side validation error must never join the issue-only window signal. + expect(result).toEqual({ items: [], failedCount: 0, errorTypes: ['unknown'] }) + }) + it('routes work-item counts through the active runtime environment', async () => { runtimeEnvironmentCall.mockResolvedValueOnce({ id: 'rpc-work-items-count', @@ -6727,6 +6825,43 @@ describe('createGitHubSlice.fetchWorkItems source/error envelope', () => { expect(result).toEqual({ totalCount: 101, totalPages: 3 }) }) + it('caps advertised pages at the GitHub search result window', async () => { + const store = createTestStore() + mockApi.gh.countWorkItems.mockResolvedValueOnce(1170).mockResolvedValueOnce(1) + + const result = await store.getState().countWorkItemsAcrossRepos( + [ + { repoId: 'large-repo', path: '/local/large' }, + { repoId: 'small-repo', path: '/local/small' } + ], + 'is:issue', + 30 + ) + + // 1170 results → 39 naive pages, but the Search API 422s once a page + // starts past its 1000-result window; ceil(1000 / 30) = 34 stay reachable. + expect(result).toEqual({ totalCount: 1171, totalPages: 34 }) + }) + + it('pins the search-window cap at dividing and non-dividing per-repo limits', async () => { + const store = createTestStore() + // 36 is the shipped single-repo limit: page 28 starts at result 973 and is + // served; page 29 starts past 1000 and 422s. + mockApi.gh.countWorkItems.mockResolvedValueOnce(2000) + await expect( + store + .getState() + .countWorkItemsAcrossRepos([{ repoId: 'repo-id', path: '/local/repo' }], 'is:issue', 36) + ).resolves.toEqual({ totalCount: 2000, totalPages: 28 }) + // 25 divides 1000 evenly: the full window stays reachable (40 pages). + mockApi.gh.countWorkItems.mockResolvedValueOnce(2000) + await expect( + store + .getState() + .countWorkItemsAcrossRepos([{ repoId: 'repo-id', path: '/local/repo' }], 'is:issue', 25) + ).resolves.toEqual({ totalCount: 2000, totalPages: 40 }) + }) + it('rejects oversized work-item queries before cache keys or provider calls', async () => { const store = createTestStore() const secret = 'github-work-items-secret' @@ -6755,7 +6890,7 @@ describe('createGitHubSlice.fetchWorkItems source/error envelope', () => { oversizedQuery, 1 ) - ).resolves.toEqual({ items: [], failedCount: 0 }) + ).resolves.toEqual({ items: [], failedCount: 0, errorTypes: [] }) await expect( store .getState() diff --git a/src/renderer/src/store/slices/github.ts b/src/renderer/src/store/slices/github.ts index 4f197565874..5ebe76f2c87 100644 --- a/src/renderer/src/store/slices/github.ts +++ b/src/renderer/src/store/slices/github.ts @@ -53,7 +53,10 @@ import { rightSidebarShowsPullRequestData } from '@/lib/right-sidebar-visibility import { hostedReviewInfoFromGitHubPRInfo } from '../../../../shared/hosted-review-github' import { getHostedReviewCacheKey, linkedReviewHintKey } from './hosted-review-cache-identity' import { getGitHubPRCacheKey, getGitHubRepoCacheKey } from './github-cache-key' -import { isGitHubWorkItemsQueryTooLarge } from './github-work-items-query-bounds' +import { + GITHUB_SEARCH_RESULT_WINDOW_ERROR_PATTERN, + isGitHubWorkItemsQueryTooLarge +} from './github-work-items-query-bounds' import { classifyGitHubUnavailable } from '../../../../shared/github-api-availability' import { isMacAppDataPath } from '@/lib/passive-macos-app-data-access' import { translate } from '@/i18n/i18n' @@ -655,6 +658,9 @@ const CHECKS_CACHE_TTL = 60_000 // 1 minute — checks change more frequently const EMPTY_CHECKS_CACHE_TTL = 10_000 // Why: the work-item list is a browse surface, not a source of truth, so 60s staleness is fine (SWR keeps it current). const WORK_ITEMS_CACHE_TTL = 60_000 +// GitHub's Search API serves the page that starts within its first 1000 results; +// the next page 422s even when the final reachable page crosses the boundary. +const GITHUB_SEARCH_RESULT_WINDOW = 1000 // Why: long-lived (matches repos.ts) so the user has time to read + act on persist failures before the toast vanishes. const ERROR_TOAST_DURATION = 60_000 @@ -1964,7 +1970,11 @@ export type GitHubSlice = { displayLimit: number, query: string, page: number - ) => Promise<{ items: GitHubWorkItem[]; failedCount: number }> + ) => Promise<{ + items: GitHubWorkItem[] + failedCount: number + errorTypes: ClassifiedError['type'][] + }> /** Count items and derive pages from the largest per-repo result set. */ countWorkItemsAcrossRepos: ( repos: { @@ -2784,9 +2794,10 @@ export const createGitHubSlice: StateCreator = (s fetchWorkItemsNextPage: async (repos, perRepoLimit, displayLimit, query, page) => { if (isGitHubWorkItemsQueryTooLarge(query)) { - return { items: [], failedCount: 0 } + return { items: [], failedCount: 0, errorTypes: [] } } let failedCount = 0 + const errorTypes: ClassifiedError['type'][] = [] const perProjectResults = await Promise.all( repos.map(async (r) => { const requestState = get() @@ -2812,11 +2823,30 @@ export const createGitHubSlice: StateCreator = (s }) // Why: page-N failures aren't in the per-repo banner (keyed on the initial fetch); log them so pagination failures are observable instead of silently truncating (richer surface deferred, design doc §6). if (envelope.errors?.issues) { + const { type, message } = envelope.errors.issues + // Why: only the 1000-result-window 422 may drive the unreachable + // clamp; demote other validation errors so they read as failures. + errorTypes.push( + type === 'validation_error' && + !GITHUB_SEARCH_RESULT_WINDOW_ERROR_PATTERN.test(message) + ? 'unknown' + : type + ) console.warn( `[workItems] next page ${r.repoId} issues-side partial failure:`, envelope.errors.issues ) } + if (envelope.errors?.prs) { + // Why: the window 422 is issue-side only — a PR-side validation + // error must never join the unreachable signal. + const { type } = envelope.errors.prs + errorTypes.push(type === 'validation_error' ? 'unknown' : type) + console.warn( + `[workItems] next page ${r.repoId} prs-side partial failure:`, + envelope.errors.prs + ) + } return envelope.items.map((item): GitHubWorkItem => ({ ...item, repoId: r.repoId })) } catch (err) { if (isGitHubWorkItemsSshRemoteRequiredError(err)) { @@ -2831,7 +2861,7 @@ export const createGitHubSlice: StateCreator = (s }) ) const merged = sortWorkItemsByNumber(perProjectResults.flat()).slice(0, displayLimit) - return { items: merged, failedCount } + return { items: merged, failedCount, errorTypes } }, countWorkItemsAcrossRepos: async (repos, query, perRepoLimit) => { @@ -2839,6 +2869,8 @@ export const createGitHubSlice: StateCreator = (s return { totalCount: 0, totalPages: 0 } } const normalizedLimit = Math.max(1, Math.floor(perRepoLimit)) + // Why: GitHub 422s pages that start past its 1000-result search window. + const maxReachablePages = Math.max(1, Math.ceil(GITHUB_SEARCH_RESULT_WINDOW / normalizedLimit)) const counts = await Promise.all( repos.map(async (r) => { // Why: same stampede cap as item-fetch — without a slot a 90-repo selection fires 90 concurrent count IPCs before the main-side rate-limit guard sees the first 403. @@ -2870,7 +2902,8 @@ export const createGitHubSlice: StateCreator = (s totalCount: counts.reduce((sum, count) => sum + count, 0), // Why: repos advance independently by page, so take the max across repos — a sum/page-width undercounts when one repo owns most results. totalPages: counts.reduce( - (maxPages, count) => Math.max(maxPages, Math.ceil(count / normalizedLimit)), + (maxPages, count) => + Math.max(maxPages, Math.min(Math.ceil(count / normalizedLimit), maxReachablePages)), 0 ) } diff --git a/src/shared/github-work-items-query-bounds.ts b/src/shared/github-work-items-query-bounds.ts index 4d3ae6e6d49..48f90bceb38 100644 --- a/src/shared/github-work-items-query-bounds.ts +++ b/src/shared/github-work-items-query-bounds.ts @@ -2,6 +2,17 @@ import { isClipboardTextByteLengthOverLimit } from './clipboard-text' export const GITHUB_WORK_ITEMS_QUERY_MAX_BYTES = 8 * 1024 +/** + * GitHub's Search API only pages through the first 1000 matches; beyond that it + * 422s with "Only the first 1000 search results are available". Matching that + * free-text wording is the ONLY signal separating a permanently unreachable page + * from a transient failure (#11485), so the phrase is pinned here rather than + * inlined: if GitHub rewords it, window 422s silently demote to generic + * failures and the advertised page count stops being capped. + * `gh-utils.test.ts` asserts the classified message still carries this phrase. + */ +export const GITHUB_SEARCH_RESULT_WINDOW_ERROR_PATTERN = /first 1000 search results/i + export function isGitHubWorkItemsQueryTooLarge( query: string, maxBytes = GITHUB_WORK_ITEMS_QUERY_MAX_BYTES diff --git a/src/shared/types.ts b/src/shared/types.ts index faf50e6eb2e..b52aa1365c6 100644 --- a/src/shared/types.ts +++ b/src/shared/types.ts @@ -2065,6 +2065,7 @@ export type ListWorkItemsResult = { } errors?: { issues?: ClassifiedError + prs?: ClassifiedError } /** True when the user's per-repo preference was `'upstream'` but no upstream * remote is configured, so the resolver fell back to origin. Renderer uses From 99b94a38eb53b9c90efe21a0d2609ee483b72ff3 Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Sun, 2 Aug 2026 12:14:12 -0700 Subject: [PATCH 003/106] docs(tasks): trim the search-window error pattern comment (#12130) Keeps only the non-obvious rationale for pinning GitHub's free-text 422 wording. Co-authored-by: Orca --- src/shared/github-work-items-query-bounds.ts | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/src/shared/github-work-items-query-bounds.ts b/src/shared/github-work-items-query-bounds.ts index 48f90bceb38..85616688a47 100644 --- a/src/shared/github-work-items-query-bounds.ts +++ b/src/shared/github-work-items-query-bounds.ts @@ -3,13 +3,8 @@ import { isClipboardTextByteLengthOverLimit } from './clipboard-text' export const GITHUB_WORK_ITEMS_QUERY_MAX_BYTES = 8 * 1024 /** - * GitHub's Search API only pages through the first 1000 matches; beyond that it - * 422s with "Only the first 1000 search results are available". Matching that - * free-text wording is the ONLY signal separating a permanently unreachable page - * from a transient failure (#11485), so the phrase is pinned here rather than - * inlined: if GitHub rewords it, window 422s silently demote to generic - * failures and the advertised page count stops being capped. - * `gh-utils.test.ts` asserts the classified message still carries this phrase. + * The Search API's free-text 422 wording is the only signal separating a + * permanently unreachable page (past the first 1000 matches) from a transient failure. */ export const GITHUB_SEARCH_RESULT_WINDOW_ERROR_PATTERN = /first 1000 search results/i From 0c2893695c5d558b84dfb07d89ddbfdbf1081511 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Sun, 2 Aug 2026 15:23:33 -0700 Subject: [PATCH 004/106] fix(browser): keep browser tabs rendering across worktree switches (STA-3228) (#12137) * fix(browser): keep browser tabs rendering across worktree switches (STA-3228) Switching away from a worktree that had a targeted background mount unmounted BrowserPaneOverlayLayer, pulling the persistent slots out of the DOM and killing their guests; the stale viewport cache then kept rendering into the removed subtree, so the tab stayed blank forever and reload threw. Keep the overlay mounted for hidden worktrees (slots park their panes, so this stays cheap) and rebuild cached viewports whose slot root remounted. * fix(browser): retain live overlay slots without background churn * fix(browser): latch overlay retention after commit --- src/renderer/src/components/Terminal.tsx | 18 ++++-- .../BrowserPane.webview-preferences.test.ts | 32 ++++++++++ .../BrowserPaneOverlayLayer.test.tsx | 62 ++++++++++++++++++- .../browser-pane/BrowserPaneOverlayLayer.tsx | 24 ++++++- .../browser-page-viewport.test.ts | 35 +++++++++++ .../browser-pane/browser-page-viewport.ts | 9 ++- .../browser-pane/browser-page-webview.ts | 16 ++--- .../browser-pane/webview-registry.ts | 13 +++- 8 files changed, 189 insertions(+), 20 deletions(-) diff --git a/src/renderer/src/components/Terminal.tsx b/src/renderer/src/components/Terminal.tsx index 4189b0e44a1..505c88e648a 100644 --- a/src/renderer/src/components/Terminal.tsx +++ b/src/renderer/src/components/Terminal.tsx @@ -45,7 +45,7 @@ import type { } from '../../../shared/types' import { hasFeatureInteraction } from '../../../shared/feature-interactions' import BrowserPane from './browser-pane/BrowserPane' -import BrowserPaneOverlayLayer from './browser-pane/BrowserPaneOverlayLayer' +import { RetainedBrowserPaneOverlayLayer } from './browser-pane/BrowserPaneOverlayLayer' import EmulatorPaneOverlayLayer from './emulator-pane/EmulatorPaneOverlayLayer' import { useBrowserAutomationVisibilityForAny } from './browser-pane/browser-automation-visibility' import { useBrowserMobileDriverForAny } from '@/lib/pane-manager/browser-mobile-driver-state' @@ -2674,11 +2674,19 @@ const WorktreeSplitSurface = React.memo(function WorktreeSplitSurface({ backgroundMountTabIds={backgroundMountTabIds} activationDeferredMountTabIds={activationDeferredMountTabIds} /> + {/* Why: once eligible, retain slot DOM so hidden worktrees keep their Electron guests alive (STA-3228). */} + {isVisible || backgroundMountTabIds === null ? ( - <> - - - + ) : null} diff --git a/src/renderer/src/components/browser-pane/BrowserPane.webview-preferences.test.ts b/src/renderer/src/components/browser-pane/BrowserPane.webview-preferences.test.ts index 203fce6fdbf..8d9e541d8fe 100644 --- a/src/renderer/src/components/browser-pane/BrowserPane.webview-preferences.test.ts +++ b/src/renderer/src/components/browser-pane/BrowserPane.webview-preferences.test.ts @@ -103,4 +103,36 @@ describe('BrowserPane webview preferences', () => { expect(ensuredWebview?.webview.style.pointerEvents).toBe('none') expect(refreshedContainer.lastElementChild).toBe(ensuredWebview?.webview as unknown as Element) }) + + it('keeps a replacement viewport while rebuilding a parent-drifted webview (STA-3228)', () => { + const staleContainer = createContainer('stale') + const staleWebview = document.createElement('webview') as Electron.WebviewTag + staleWebview.setAttribute('partition', 'persist:orca-browser') + staleContainer.appendChild(staleWebview) + registryMocks.webviewRegistry.set('browser-page-1', staleWebview) + const replacementContainer = createContainer('replacement') + const resolveContainer = vi.fn(() => replacementContainer) + registryMocks.destroyPersistentWebview.mockImplementation(() => { + staleWebview.remove() + staleContainer.remove() + registryMocks.webviewRegistry.delete('browser-page-1') + }) + + const ensuredWebview = ensureBrowserPageWebview({ + browserTabId: 'browser-page-1', + container: replacementContainer, + inputLocked: false, + webviewPartition: 'persist:orca-browser', + resolveContainer + }) + + expect(registryMocks.destroyPersistentWebview).toHaveBeenCalledWith('browser-page-1', { + preserveViewport: true + }) + expect(ensuredWebview?.container).toBe(replacementContainer) + expect(replacementContainer.isConnected).toBe(true) + expect(replacementContainer.lastElementChild).toBe( + ensuredWebview?.webview as unknown as Element + ) + }) }) diff --git a/src/renderer/src/components/browser-pane/BrowserPaneOverlayLayer.test.tsx b/src/renderer/src/components/browser-pane/BrowserPaneOverlayLayer.test.tsx index 4e49c241722..e23df8ae29e 100644 --- a/src/renderer/src/components/browser-pane/BrowserPaneOverlayLayer.test.tsx +++ b/src/renderer/src/components/browser-pane/BrowserPaneOverlayLayer.test.tsx @@ -1,5 +1,8 @@ +// @vitest-environment happy-dom +import { cleanup, render } from '@testing-library/react' +import { Suspense } from 'react' import { renderToStaticMarkup } from 'react-dom/server' -import { beforeEach, describe, expect, it, vi } from 'vitest' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import type { BrowserTab as BrowserTabState, Tab, TabGroup } from '../../../../shared/types' type MockAppState = { @@ -54,7 +57,7 @@ vi.mock('./BrowserPane', () => ({ ) })) -import BrowserPaneOverlayLayer from './BrowserPaneOverlayLayer' +import BrowserPaneOverlayLayer, { RetainedBrowserPaneOverlayLayer } from './BrowserPaneOverlayLayer' describe('BrowserPaneOverlayLayer', () => { beforeEach(() => { @@ -64,6 +67,61 @@ describe('BrowserPaneOverlayLayer', () => { mocks.state = createState() }) + afterEach(() => cleanup()) + + it('defers browser slots for a restricted hidden mount, then retains them after activation', () => { + const view = render( + + ) + + expect(view.container.querySelectorAll('[data-browser-overlay-tab-id]')).toHaveLength(0) + + view.rerender( + + ) + expect(view.container.querySelectorAll('[data-browser-overlay-tab-id]')).toHaveLength(2) + + view.rerender( + + ) + expect(view.container.querySelectorAll('[data-browser-overlay-tab-id]')).toHaveLength(2) + }) + + it('does not retain browser slots from an eligible render that never commits', () => { + const pending = new Promise(() => {}) + const BlockCommit = ({ blocked }: { blocked: boolean }): null => { + if (blocked) { + throw pending + } + return null + } + const renderBoundary = (mountEligible: boolean, blocked: boolean) => ( + }> + + + + ) + const view = render(renderBoundary(false, false)) + + view.rerender(renderBoundary(true, true)) + expect(view.container.querySelector('[data-suspended]')).not.toBeNull() + + view.rerender(renderBoundary(false, false)) + expect(view.container.querySelectorAll('[data-browser-overlay-tab-id]')).toHaveLength(0) + }) + it('keeps inactive browser panes mounted for a visible worktree', () => { const markup = renderOverlay({ isWorktreeActive: true }) diff --git a/src/renderer/src/components/browser-pane/BrowserPaneOverlayLayer.tsx b/src/renderer/src/components/browser-pane/BrowserPaneOverlayLayer.tsx index b019f1025e7..67d6aa4da9e 100644 --- a/src/renderer/src/components/browser-pane/BrowserPaneOverlayLayer.tsx +++ b/src/renderer/src/components/browser-pane/BrowserPaneOverlayLayer.tsx @@ -1,4 +1,4 @@ -import { memo, useCallback, useMemo } from 'react' +import { memo, useCallback, useLayoutEffect, useMemo, useState } from 'react' import { registerBrowserOverlaySlotViewport } from './browser-page-viewport' import { useShallow } from 'zustand/react/shallow' import { useAppStore } from '../../store' @@ -194,4 +194,26 @@ const BrowserPaneOverlayLayer = memo(function BrowserPaneOverlayLayer({ ) }) +export const RetainedBrowserPaneOverlayLayer = memo(function RetainedBrowserPaneOverlayLayer({ + worktreeId, + isWorktreeActive, + mountEligible +}: { + worktreeId: string + isWorktreeActive: boolean + mountEligible: boolean +}): React.JSX.Element | null { + const [hasCommittedMount, setHasCommittedMount] = useState(false) + // Why: commit the latch with the persistent slot DOM so discarded renders cannot retain a guest host. + useLayoutEffect(() => { + if (mountEligible && !hasCommittedMount) { + setHasCommittedMount(true) + } + }, [hasCommittedMount, mountEligible]) + if (!mountEligible && !hasCommittedMount) { + return null + } + return +}) + export default BrowserPaneOverlayLayer diff --git a/src/renderer/src/components/browser-pane/browser-page-viewport.test.ts b/src/renderer/src/components/browser-pane/browser-page-viewport.test.ts index 9172ddae7f2..c4df770d2eb 100644 --- a/src/renderer/src/components/browser-pane/browser-page-viewport.test.ts +++ b/src/renderer/src/components/browser-pane/browser-page-viewport.test.ts @@ -46,6 +46,41 @@ describe('ensureBrowserPageViewport', () => { it('returns null until the slot viewport root is registered', () => { expect(ensureBrowserPageViewport('page-1', 'workspace-missing')).toBeNull() }) + + it('reuses the cached viewport while the slot root is unchanged', () => { + mountSlotViewport('workspace-1') + const first = ensureBrowserPageViewport('page-1', 'workspace-1') + const second = ensureBrowserPageViewport('page-1', 'workspace-1') + + expect(second).toBe(first) + }) + + it('keeps the parked viewport while the slot root is unregistered', () => { + const root = mountSlotViewport('workspace-1') + const parked = ensureBrowserPageViewport('page-1', 'workspace-1') + root.remove() + registerBrowserOverlaySlotViewport('workspace-1', null) + + expect(ensureBrowserPageViewport('page-1', 'workspace-1')).toBe(parked) + }) + + it('rebuilds under a replacement slot root instead of returning the stranded shell (STA-3228)', () => { + const oldRoot = mountSlotViewport('workspace-1') + const stale = ensureBrowserPageViewport('page-1', 'workspace-1')! + // Worktree overlay unmounts while hidden: slot root leaves the DOM with the shell inside. + oldRoot.remove() + registerBrowserOverlaySlotViewport('workspace-1', null) + const newRoot = mountSlotViewport('workspace-1') + + const rebuilt = ensureBrowserPageViewport('page-1', 'workspace-1') + + expect(rebuilt).not.toBeNull() + expect(rebuilt).not.toBe(stale) + expect(rebuilt!.shell.parentElement).toBe(newRoot) + expect(rebuilt!.shell.isConnected).toBe(true) + expect(stale.shell.isConnected).toBe(false) + expect(getBrowserPageViewportContainer('page-1')).toBe(rebuilt!.container) + }) }) describe('syncBrowserPageChromeInset', () => { diff --git a/src/renderer/src/components/browser-pane/browser-page-viewport.ts b/src/renderer/src/components/browser-pane/browser-page-viewport.ts index f6cae76105a..d34fbf744f4 100644 --- a/src/renderer/src/components/browser-pane/browser-page-viewport.ts +++ b/src/renderer/src/components/browser-pane/browser-page-viewport.ts @@ -65,11 +65,16 @@ export function ensureBrowserPageViewport( browserPageId: string, workspaceTabId: string ): BrowserPageViewport | null { + const root = slotViewportRoots.get(workspaceTabId) const existing = browserPageViewports.get(browserPageId) if (existing) { - return existing + if (!root || existing.shell.parentElement === root) { + return existing + } + // Why: a remounted slot strands this shell after Electron destroys its detached guest (STA-3228). + existing.shell.remove() + browserPageViewports.delete(browserPageId) } - const root = slotViewportRoots.get(workspaceTabId) if (!root) { return null } diff --git a/src/renderer/src/components/browser-pane/browser-page-webview.ts b/src/renderer/src/components/browser-pane/browser-page-webview.ts index 1179d0da63b..798b1845048 100644 --- a/src/renderer/src/components/browser-pane/browser-page-webview.ts +++ b/src/renderer/src/components/browser-pane/browser-page-webview.ts @@ -21,17 +21,19 @@ export function ensureBrowserPageWebview({ let webview = webviewRegistry.get(browserTabId) let created = false let activeContainer = container + const parentDrifted = webview?.parentElement !== container // Why: a persisted guest must be torn down and rebuilt when its DOM parent // drifted (moving a across parents can recreate the guest document) // or when its partition no longer matches — Electron partitions are immutable - // after creation, so reuse would keep the stale session. Re-resolve the - // viewport container the teardown may have detached; bail if it is gone. - if ( - webview && - (webview.parentElement !== container || webview.getAttribute('partition') !== webviewPartition) - ) { - destroyPersistentWebview(browserTabId) + // after creation, so reuse would keep the stale session. Parent-drift repair + // preserves the newly replaced viewport; always verify the resolved container. + if (webview && (parentDrifted || webview.getAttribute('partition') !== webviewPartition)) { + if (parentDrifted) { + destroyPersistentWebview(browserTabId, { preserveViewport: true }) + } else { + destroyPersistentWebview(browserTabId) + } webview = undefined const refreshedContainer = resolveContainer() if (!refreshedContainer) { diff --git a/src/renderer/src/components/browser-pane/webview-registry.ts b/src/renderer/src/components/browser-pane/webview-registry.ts index 51e3721cceb..f603839abd1 100644 --- a/src/renderer/src/components/browser-pane/webview-registry.ts +++ b/src/renderer/src/components/browser-pane/webview-registry.ts @@ -189,7 +189,10 @@ export function moveFocusToRendererBeforeWebviewDetach(webview: Electron.Webview moveFocusToRendererIfWebviewOwnsFocus(webview) } -export function destroyPersistentWebview(browserTabId: string): void { +export function destroyPersistentWebview( + browserTabId: string, + { preserveViewport = false }: { preserveViewport?: boolean } = {} +): void { const webview = webviewRegistry.get(browserTabId) // The guest is gone, so its user-applied zoom must not be inherited by a // later tab that reuses the id. @@ -197,7 +200,9 @@ export function destroyPersistentWebview(browserTabId: string): void { if (!webview) { // Why: the viewport can outlive a missing webview entry; tear it down on // explicit close paths so overlay slots do not leak parked shells. - removeBrowserPageViewport(browserTabId) + if (!preserveViewport) { + removeBrowserPageViewport(browserTabId) + } registeredWebContentsIds.delete(browserTabId) clearLiveBrowserUrl(browserTabId) return @@ -206,7 +211,9 @@ export function destroyPersistentWebview(browserTabId: string): void { moveFocusToRendererBeforeWebviewDetach(webview) webview.remove() unregisterPersistentWebview(browserTabId) - removeBrowserPageViewport(browserTabId) + if (!preserveViewport) { + removeBrowserPageViewport(browserTabId) + } registeredWebContentsIds.delete(browserTabId) clearLiveBrowserUrl(browserTabId) } From c443bc0d81db40774a3e012f7e5497343192206f Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Sun, 2 Aug 2026 15:48:18 -0700 Subject: [PATCH 005/106] fix(mobile): correct OMP icon gradient stop (#11650) --- mobile/src/components/MobileAgentIcon.tsx | 2 +- .../mobile-agent-icon-gradient.test.ts | 65 +++++++++++++++++++ 2 files changed, 66 insertions(+), 1 deletion(-) create mode 100644 mobile/src/components/mobile-agent-icon-gradient.test.ts diff --git a/mobile/src/components/MobileAgentIcon.tsx b/mobile/src/components/MobileAgentIcon.tsx index 6c47fce7a5b..c5557ab3934 100644 --- a/mobile/src/components/MobileAgentIcon.tsx +++ b/mobile/src/components/MobileAgentIcon.tsx @@ -32,7 +32,7 @@ function OmpIcon({ size = 16 }: { size?: number }) { - + diff --git a/mobile/src/components/mobile-agent-icon-gradient.test.ts b/mobile/src/components/mobile-agent-icon-gradient.test.ts new file mode 100644 index 00000000000..8e2781edfb2 --- /dev/null +++ b/mobile/src/components/mobile-agent-icon-gradient.test.ts @@ -0,0 +1,65 @@ +import { createElement } from 'react' +import { act, create, type ReactTestRenderer } from 'react-test-renderer' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { MobileAgentIcon } from './MobileAgentIcon' + +vi.mock('react-native', () => ({ + Image: 'Image', + StyleSheet: { create: (styles: unknown) => styles }, + Text: 'Text', + View: 'View' +})) + +vi.mock('lucide-react-native', () => ({ + Terminal: 'Terminal' +})) + +vi.mock('react-native-svg', () => ({ + default: 'Svg', + Defs: 'Defs', + G: 'G', + LinearGradient: 'LinearGradient', + Path: 'Path', + Stop: 'Stop' +})) + +vi.mock('./mobile-agent-icon-assets', () => ({ + MOBILE_AGENT_ICON_ASSETS: {} +})) + +vi.mock('./AgentIcons', () => ({ + ClaudeIcon: 'ClaudeIcon', + OpenAIIcon: 'OpenAIIcon' +})) + +describe('MobileAgentIcon OMP gradient', () => { + let renderer: ReactTestRenderer | null = null + + beforeEach(() => { + globalThis.IS_REACT_ACT_ENVIRONMENT = true + }) + + afterEach(() => { + act(() => renderer?.unmount()) + renderer = null + vi.restoreAllMocks() + }) + + it('uses valid SVG gradient stop offsets', async () => { + const consoleError = vi.spyOn(console, 'error').mockImplementation((...args) => { + if (typeof args[0] !== 'string' || !args[0].includes('react-test-renderer is deprecated')) { + throw new Error(String(args[0])) + } + }) + await act(async () => { + renderer = create(createElement(MobileAgentIcon, { agentId: 'omp' })) + }) + consoleError.mockRestore() + + expect(renderer.root.findAllByType('Stop').map((stop) => stop.props.offset)).toEqual([ + '0', + '0.5', + '1' + ]) + }) +}) From b07dce7421b00d761b460d478722a46c65b87408 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Sun, 2 Aug 2026 15:50:40 -0700 Subject: [PATCH 006/106] fix(mobile): improve secondary text contrast (#11651) --- .../src/theme/mobile-theme-contrast.test.ts | 38 +++++++++++++++++++ mobile/src/theme/mobile-theme.ts | 4 +- 2 files changed, 40 insertions(+), 2 deletions(-) create mode 100644 mobile/src/theme/mobile-theme-contrast.test.ts diff --git a/mobile/src/theme/mobile-theme-contrast.test.ts b/mobile/src/theme/mobile-theme-contrast.test.ts new file mode 100644 index 00000000000..6546e6aaa43 --- /dev/null +++ b/mobile/src/theme/mobile-theme-contrast.test.ts @@ -0,0 +1,38 @@ +import { describe, expect, it } from 'vitest' +import { colors } from './mobile-theme' + +function channelLuminance(channel: number): number { + const value = channel / 255 + return value <= 0.04045 ? value / 12.92 : ((value + 0.055) / 1.055) ** 2.4 +} + +function relativeLuminance(hex: string): number { + const red = Number.parseInt(hex.slice(1, 3), 16) + const green = Number.parseInt(hex.slice(3, 5), 16) + const blue = Number.parseInt(hex.slice(5, 7), 16) + return ( + 0.2126 * channelLuminance(red) + + 0.7152 * channelLuminance(green) + + 0.0722 * channelLuminance(blue) + ) +} + +function contrastRatio(foreground: string, background: string): number { + const lighter = Math.max(relativeLuminance(foreground), relativeLuminance(background)) + const darker = Math.min(relativeLuminance(foreground), relativeLuminance(background)) + return (lighter + 0.05) / (darker + 0.05) +} + +describe('mobile text contrast', () => { + it('keeps muted text readable on every standard dark surface', () => { + for (const surface of [colors.bgBase, colors.bgPanel, colors.bgRaised]) { + expect(contrastRatio(colors.textMuted, surface)).toBeGreaterThanOrEqual(4.5) + } + }) + + it('keeps secondary text more prominent than muted text', () => { + expect(contrastRatio(colors.textSecondary, colors.bgPanel)).toBeGreaterThan( + contrastRatio(colors.textMuted, colors.bgPanel) + ) + }) +}) diff --git a/mobile/src/theme/mobile-theme.ts b/mobile/src/theme/mobile-theme.ts index dbf09b745b9..0ee1771ecb7 100644 --- a/mobile/src/theme/mobile-theme.ts +++ b/mobile/src/theme/mobile-theme.ts @@ -9,8 +9,8 @@ export const colors = { editorSurface: '#1e1e1e', textPrimary: '#e0e0e0', - textSecondary: '#888888', - textMuted: '#555555', + textSecondary: '#a1a1a1', + textMuted: '#8c8c8c', // Crisp near-white surface for the single primary action on a screen (the // worktree FAB). Brighter than textPrimary so it reads as a solid button, not From 2efc6e5476f2b02bfe2005a1d8d6f9ce50fcb891 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Sun, 2 Aug 2026 15:53:06 -0700 Subject: [PATCH 007/106] fix(mobile): stop support modules from registering as routes (#11652) * fix(mobile): keep support modules out of Expo routes * test(mobile): parse Expo route exports * test(mobile): reject platform-specific API routes * test(mobile): reject platform API routes unconditionally --- mobile/app/h/[hostId]/accounts.tsx | 2 +- .../app/h/[hostId]/session/[worktreeId].tsx | 6 +- .../mobile-accounts-screen-styles.ts} | 2 +- mobile/src/expo-route-module-boundary.test.ts | 121 ++++++++++++++++++ .../session/MobileBrowserTabActionSheet.tsx | 2 +- .../session/MobileSessionHeaderIconButton.tsx | 2 +- .../session/QuickCommandsTabButton.tsx | 2 +- .../mobile-bulk-close-sheet-actions.ts | 5 +- .../mobile-session-command-input-styles.ts | 2 +- .../session/mobile-session-frame-styles.ts | 2 +- .../session/mobile-session-reader-styles.ts | 2 +- .../mobile-session-review-comment-styles.ts | 2 +- .../session/mobile-session-route-types.ts | 17 +-- .../session/mobile-session-styles.ts | 0 .../session/mobile-terminal-tab-agent.test.ts | 2 +- .../src/session/mobile-terminal-tab-agent.ts | 2 +- .../terminal-live-input-affordance.test.ts | 2 +- 17 files changed, 144 insertions(+), 29 deletions(-) rename mobile/{app/h/[hostId]/accounts-screen-styles.ts => src/accounts/mobile-accounts-screen-styles.ts} (97%) create mode 100644 mobile/src/expo-route-module-boundary.test.ts rename mobile/{app/h/[hostId] => src}/session/QuickCommandsTabButton.tsx (92%) rename mobile/{app/h/[hostId] => src}/session/mobile-session-command-input-styles.ts (98%) rename mobile/{app/h/[hostId] => src}/session/mobile-session-frame-styles.ts (97%) rename mobile/{app/h/[hostId] => src}/session/mobile-session-reader-styles.ts (97%) rename mobile/{app/h/[hostId] => src}/session/mobile-session-review-comment-styles.ts (98%) rename mobile/{app/h/[hostId] => src}/session/mobile-session-route-types.ts (85%) rename mobile/{app/h/[hostId] => src}/session/mobile-session-styles.ts (100%) diff --git a/mobile/app/h/[hostId]/accounts.tsx b/mobile/app/h/[hostId]/accounts.tsx index 1a07a0d6ecc..b107f34da97 100644 --- a/mobile/app/h/[hostId]/accounts.tsx +++ b/mobile/app/h/[hostId]/accounts.tsx @@ -14,7 +14,7 @@ import { ChevronLeft, Check, RefreshCw, User } from 'lucide-react-native' import { loadHosts } from '../../../src/transport/host-store' import { useHostClient } from '../../../src/transport/client-context' import { colors, spacing } from '../../../src/theme/mobile-theme' -import { styles } from './accounts-screen-styles' +import { styles } from '../../../src/accounts/mobile-accounts-screen-styles' import { useNow } from '../../../src/hooks/use-now' import { ClaudeIcon, OpenAIIcon } from '../../../src/components/AgentIcons' import { diff --git a/mobile/app/h/[hostId]/session/[worktreeId].tsx b/mobile/app/h/[hostId]/session/[worktreeId].tsx index 5d9ba390035..8f6d7ca0c97 100644 --- a/mobile/app/h/[hostId]/session/[worktreeId].tsx +++ b/mobile/app/h/[hostId]/session/[worktreeId].tsx @@ -267,8 +267,8 @@ import { reconcileMobileSessionCreateWarningState } from '../../../../src/session/mobile-session-create-warning-state' import { colors, spacing } from '../../../../src/theme/mobile-theme' -import { styles } from './mobile-session-styles' -import { QuickCommandsTabButton } from './QuickCommandsTabButton' +import { QuickCommandsTabButton } from '../../../../src/session/QuickCommandsTabButton' +import { styles } from '../../../../src/session/mobile-session-styles' import type { DiffComment, TerminalQuickCommand } from '../../../../../src/shared/types' import type { DiffCommentActions, @@ -289,7 +289,7 @@ import type { TerminalCreateResult, TerminalGestureInputBucket, TerminalGestureInputQueue -} from './mobile-session-route-types' +} from '../../../../src/session/mobile-session-route-types' const TERMINAL_KEYBOARD_DISMISS_ACTION_SHEET_FALLBACK_MS = 450 diff --git a/mobile/app/h/[hostId]/accounts-screen-styles.ts b/mobile/src/accounts/mobile-accounts-screen-styles.ts similarity index 97% rename from mobile/app/h/[hostId]/accounts-screen-styles.ts rename to mobile/src/accounts/mobile-accounts-screen-styles.ts index 4a987b7a442..65074f340ae 100644 --- a/mobile/app/h/[hostId]/accounts-screen-styles.ts +++ b/mobile/src/accounts/mobile-accounts-screen-styles.ts @@ -1,5 +1,5 @@ import { StyleSheet } from 'react-native' -import { colors, spacing, typography, radii } from '../../../src/theme/mobile-theme' +import { colors, spacing, typography, radii } from '../theme/mobile-theme' export const styles = StyleSheet.create({ container: { diff --git a/mobile/src/expo-route-module-boundary.test.ts b/mobile/src/expo-route-module-boundary.test.ts new file mode 100644 index 00000000000..3870f32c216 --- /dev/null +++ b/mobile/src/expo-route-module-boundary.test.ts @@ -0,0 +1,121 @@ +import { readFileSync, readdirSync } from 'node:fs' +import { fileURLToPath } from 'node:url' +import { basename, dirname, extname, join, relative } from 'node:path' +import ts from 'typescript' +import { describe, expect, it } from 'vitest' + +const appDirectory = fileURLToPath(new URL('../app', import.meta.url)) +const routeSourceExtensions = new Set(['.js', '.jsx', '.ts', '.tsx']) + +function sourceFiles(directory: string): string[] { + return readdirSync(directory, { withFileTypes: true }).flatMap((entry) => { + const path = join(directory, entry.name) + return entry.isDirectory() ? sourceFiles(path) : [path] + }) +} + +function isNonScreenExpoModule(path: string): boolean { + const fileName = basename(path) + if (/\+api\.[jt]sx?$/.test(fileName)) { + return true + } + return ( + dirname(relative(appDirectory, path)) === '.' && + /^\+(?:html|middleware|native-intent)\.[jt]sx?$/.test(fileName) + ) +} + +function isPlatformSpecificApiRoute(path: string): boolean { + return /\+api\.(?:android|ios|native|web)\.[jt]sx?$/.test(basename(path)) +} + +function hasDefaultExport(path: string, source: string): boolean { + const extension = extname(path) + const sourceFile = ts.createSourceFile( + path, + source, + ts.ScriptTarget.Latest, + true, + extension === '.jsx' + ? ts.ScriptKind.JSX + : extension === '.js' + ? ts.ScriptKind.JS + : extension === '.tsx' + ? ts.ScriptKind.TSX + : ts.ScriptKind.TS + ) + + return sourceFile.statements.some((statement) => { + if (ts.isExportAssignment(statement)) { + return !statement.isExportEquals + } + if (ts.isExportDeclaration(statement) && !statement.isTypeOnly && statement.exportClause) { + if (ts.isNamespaceExport(statement.exportClause)) { + return statement.exportClause.name.text === 'default' + } + return statement.exportClause.elements.some( + (element) => !element.isTypeOnly && element.name.text === 'default' + ) + } + const modifiers = ts.canHaveModifiers(statement) ? ts.getModifiers(statement) : undefined + return ( + !ts.isInterfaceDeclaration(statement) && + !ts.isTypeAliasDeclaration(statement) && + modifiers?.some((modifier) => modifier.kind === ts.SyntaxKind.DeclareKeyword) !== true && + modifiers?.some((modifier) => modifier.kind === ts.SyntaxKind.ExportKeyword) === true && + modifiers.some((modifier) => modifier.kind === ts.SyntaxKind.DefaultKeyword) + ) + }) +} + +function isInvalidRouteModule(path: string, source: string): boolean { + return ( + isPlatformSpecificApiRoute(path) || + (!isNonScreenExpoModule(path) && !hasDefaultExport(path, source)) + ) +} + +describe('Expo route module boundary', () => { + it('allows Expo modules that are not screen routes', () => { + expect(isNonScreenExpoModule(join(appDirectory, 'health+api.ts'))).toBe(true) + expect(isNonScreenExpoModule(join(appDirectory, 'health+api.ios.ts'))).toBe(false) + expect(isNonScreenExpoModule(join(appDirectory, '+html.tsx'))).toBe(true) + expect(isNonScreenExpoModule(join(appDirectory, '+middleware.ts'))).toBe(true) + expect(isNonScreenExpoModule(join(appDirectory, '+native-intent.ts'))).toBe(true) + expect(isNonScreenExpoModule(join(appDirectory, 'nested', '+middleware.ts'))).toBe(false) + }) + + it('recognizes syntax-level default exports', () => { + expect(hasDefaultExport('route.tsx', 'export default function Route() {}')).toBe(true) + expect( + hasDefaultExport('route.jsx', 'export default function Route() { return }') + ).toBe(true) + expect(hasDefaultExport('route.ts', "export { default } from './route-screen'")).toBe(true) + expect(hasDefaultExport('route.ts', "export { Route as default } from './route-screen'")).toBe( + true + ) + expect(hasDefaultExport('support.ts', '// export default')).toBe(false) + expect(hasDefaultExport('support.ts', "const marker = 'export default'")).toBe(false) + expect(hasDefaultExport('support.ts', 'export default interface Support {}')).toBe(false) + expect( + hasDefaultExport('support.ts', "export type { Support as default } from './types'") + ).toBe(false) + }) + + it('rejects platform-specific API routes even with a default export', () => { + expect(isPlatformSpecificApiRoute(join(appDirectory, 'health+api.ts'))).toBe(false) + for (const platform of ['android', 'ios', 'native', 'web']) { + const path = join(appDirectory, `health+api.${platform}.ts`) + expect(isInvalidRouteModule(path, 'export default function Route() {}')).toBe(true) + } + }) + + it('keeps support modules outside the app route directory', () => { + const invalidRoutes = sourceFiles(appDirectory) + .filter((path) => routeSourceExtensions.has(extname(path))) + .filter((path) => isInvalidRouteModule(path, readFileSync(path, 'utf8'))) + .map((path) => relative(appDirectory, path)) + + expect(invalidRoutes).toEqual([]) + }) +}) diff --git a/mobile/src/session/MobileBrowserTabActionSheet.tsx b/mobile/src/session/MobileBrowserTabActionSheet.tsx index 655aa717ab9..5f605d1395c 100644 --- a/mobile/src/session/MobileBrowserTabActionSheet.tsx +++ b/mobile/src/session/MobileBrowserTabActionSheet.tsx @@ -1,5 +1,5 @@ import { ChevronLeft, ChevronRight, RefreshCw } from 'lucide-react-native' -import type { MobileSessionTab } from '../../app/h/[hostId]/session/mobile-session-route-types' +import type { MobileSessionTab } from './mobile-session-route-types' import { ActionSheetModal, type ActionSheetAction } from '../components/ActionSheetModal' import { getMobileSessionTabTitle } from './mobile-terminal-tab-agent' diff --git a/mobile/src/session/MobileSessionHeaderIconButton.tsx b/mobile/src/session/MobileSessionHeaderIconButton.tsx index 68e1a1173a4..b1037bb958f 100644 --- a/mobile/src/session/MobileSessionHeaderIconButton.tsx +++ b/mobile/src/session/MobileSessionHeaderIconButton.tsx @@ -1,7 +1,7 @@ import type { ComponentType } from 'react' import { Pressable } from 'react-native' import { colors } from '../theme/mobile-theme' -import { styles } from '../../app/h/[hostId]/session/mobile-session-styles' +import { styles } from './mobile-session-styles' type HeaderIconProps = { size?: number diff --git a/mobile/app/h/[hostId]/session/QuickCommandsTabButton.tsx b/mobile/src/session/QuickCommandsTabButton.tsx similarity index 92% rename from mobile/app/h/[hostId]/session/QuickCommandsTabButton.tsx rename to mobile/src/session/QuickCommandsTabButton.tsx index 5faebb7691d..364a35daf75 100644 --- a/mobile/app/h/[hostId]/session/QuickCommandsTabButton.tsx +++ b/mobile/src/session/QuickCommandsTabButton.tsx @@ -1,7 +1,7 @@ import { Pressable, View } from 'react-native' import { SquareChevronRight } from 'lucide-react-native' -import { colors } from '../../../../src/theme/mobile-theme' +import { colors } from '../theme/mobile-theme' import { styles } from './mobile-session-styles' type Props = { diff --git a/mobile/src/session/mobile-bulk-close-sheet-actions.ts b/mobile/src/session/mobile-bulk-close-sheet-actions.ts index 8565575fe08..68dee5c2af3 100644 --- a/mobile/src/session/mobile-bulk-close-sheet-actions.ts +++ b/mobile/src/session/mobile-bulk-close-sheet-actions.ts @@ -1,7 +1,4 @@ -import type { - MarkdownDocState, - MobileSessionTab -} from '../../app/h/[hostId]/session/mobile-session-route-types' +import type { MarkdownDocState, MobileSessionTab } from './mobile-session-route-types' import type { ActionSheetAction } from '../components/ActionSheetModal' import { BULK_TAB_CLOSE_ACTIONS, diff --git a/mobile/app/h/[hostId]/session/mobile-session-command-input-styles.ts b/mobile/src/session/mobile-session-command-input-styles.ts similarity index 98% rename from mobile/app/h/[hostId]/session/mobile-session-command-input-styles.ts rename to mobile/src/session/mobile-session-command-input-styles.ts index 1bb852c4218..b19c13a502e 100644 --- a/mobile/app/h/[hostId]/session/mobile-session-command-input-styles.ts +++ b/mobile/src/session/mobile-session-command-input-styles.ts @@ -1,6 +1,6 @@ import { StyleSheet } from 'react-native' -import { colors, spacing, radii, typography } from '../../../../src/theme/mobile-theme' +import { colors, spacing, radii, typography } from '../theme/mobile-theme' export const mobileSessionCommandInputStyles = StyleSheet.create({ createWarningBanner: { diff --git a/mobile/app/h/[hostId]/session/mobile-session-frame-styles.ts b/mobile/src/session/mobile-session-frame-styles.ts similarity index 97% rename from mobile/app/h/[hostId]/session/mobile-session-frame-styles.ts rename to mobile/src/session/mobile-session-frame-styles.ts index 738632ba495..a02c14be014 100644 --- a/mobile/app/h/[hostId]/session/mobile-session-frame-styles.ts +++ b/mobile/src/session/mobile-session-frame-styles.ts @@ -1,6 +1,6 @@ import { StyleSheet } from 'react-native' -import { colors, spacing, radii, typography } from '../../../../src/theme/mobile-theme' +import { colors, spacing, radii, typography } from '../theme/mobile-theme' export const mobileSessionFrameStyles = StyleSheet.create({ container: { diff --git a/mobile/app/h/[hostId]/session/mobile-session-reader-styles.ts b/mobile/src/session/mobile-session-reader-styles.ts similarity index 97% rename from mobile/app/h/[hostId]/session/mobile-session-reader-styles.ts rename to mobile/src/session/mobile-session-reader-styles.ts index aba5340ec7a..eedd47d328b 100644 --- a/mobile/app/h/[hostId]/session/mobile-session-reader-styles.ts +++ b/mobile/src/session/mobile-session-reader-styles.ts @@ -1,6 +1,6 @@ import { Platform, StyleSheet } from 'react-native' -import { colors, spacing, radii, typography } from '../../../../src/theme/mobile-theme' +import { colors, spacing, radii, typography } from '../theme/mobile-theme' export const mobileSessionReaderStyles = StyleSheet.create({ markdownTextInput: { diff --git a/mobile/app/h/[hostId]/session/mobile-session-review-comment-styles.ts b/mobile/src/session/mobile-session-review-comment-styles.ts similarity index 98% rename from mobile/app/h/[hostId]/session/mobile-session-review-comment-styles.ts rename to mobile/src/session/mobile-session-review-comment-styles.ts index b9d19578b83..8d8b2d11ccd 100644 --- a/mobile/app/h/[hostId]/session/mobile-session-review-comment-styles.ts +++ b/mobile/src/session/mobile-session-review-comment-styles.ts @@ -1,6 +1,6 @@ import { StyleSheet } from 'react-native' -import { colors, spacing, radii, typography } from '../../../../src/theme/mobile-theme' +import { colors, spacing, radii, typography } from '../theme/mobile-theme' export const mobileSessionReviewCommentStyles = StyleSheet.create({ diffCommentAddButton: { diff --git a/mobile/app/h/[hostId]/session/mobile-session-route-types.ts b/mobile/src/session/mobile-session-route-types.ts similarity index 85% rename from mobile/app/h/[hostId]/session/mobile-session-route-types.ts rename to mobile/src/session/mobile-session-route-types.ts index c5a94e9d38f..7e74a5d04c0 100644 --- a/mobile/app/h/[hostId]/session/mobile-session-route-types.ts +++ b/mobile/src/session/mobile-session-route-types.ts @@ -1,13 +1,10 @@ -import type { MobileBrowserTab } from '../../../../src/browser/MobileBrowserPane' -import type { MobileTerminalTheme } from '../../../../src/terminal/terminal-webview-contract' -import type { MobileDiffLine } from '../../../../src/session/mobile-diff-lines' -import type { - MobileHighlightedDiffLine, - MobileSyntaxSegment -} from '../../../../src/session/mobile-file-syntax' -import type { TerminalRecord } from '../../../../src/session/mobile-terminal-records' -import type { DiffComment, TuiAgent } from '../../../../../src/shared/types' -import type { AgentStatusEntry } from '../../../../../src/shared/agent-status-types' +import type { DiffComment, TuiAgent } from '../../../src/shared/types' +import type { AgentStatusEntry } from '../../../src/shared/agent-status-types' +import type { MobileBrowserTab } from '../browser/MobileBrowserPane' +import type { MobileTerminalTheme } from '../terminal/terminal-webview-contract' +import type { MobileDiffLine } from './mobile-diff-lines' +import type { MobileHighlightedDiffLine, MobileSyntaxSegment } from './mobile-file-syntax' +import type { TerminalRecord } from './mobile-terminal-records' export type Terminal = TerminalRecord diff --git a/mobile/app/h/[hostId]/session/mobile-session-styles.ts b/mobile/src/session/mobile-session-styles.ts similarity index 100% rename from mobile/app/h/[hostId]/session/mobile-session-styles.ts rename to mobile/src/session/mobile-session-styles.ts diff --git a/mobile/src/session/mobile-terminal-tab-agent.test.ts b/mobile/src/session/mobile-terminal-tab-agent.test.ts index fcc7452e989..8449f988e24 100644 --- a/mobile/src/session/mobile-terminal-tab-agent.test.ts +++ b/mobile/src/session/mobile-terminal-tab-agent.test.ts @@ -1,7 +1,7 @@ import { describe, expect, it } from 'vitest' import type { AgentStatusEntry } from '../../../src/shared/agent-status-types' import type { TuiAgent } from '../../../src/shared/types' -import type { MobileSessionTab } from '../../app/h/[hostId]/session/mobile-session-route-types' +import type { MobileSessionTab } from './mobile-session-route-types' import { getMobileSessionTabTitle, resolveMobileTerminalTabAgentId diff --git a/mobile/src/session/mobile-terminal-tab-agent.ts b/mobile/src/session/mobile-terminal-tab-agent.ts index 1ca19521b32..731d5bf4a24 100644 --- a/mobile/src/session/mobile-terminal-tab-agent.ts +++ b/mobile/src/session/mobile-terminal-tab-agent.ts @@ -3,7 +3,7 @@ import { resolveExplicitTerminalTitleAgentType } from '../../../src/shared/termi import type { AgentStatusEntry } from '../../../src/shared/agent-status-types' import type { TuiAgent } from '../../../src/shared/types' import { isBlankBrowserUrl } from '../browser/browser-url' -import type { MobileSessionTab } from '../../app/h/[hostId]/session/mobile-session-route-types' +import type { MobileSessionTab } from './mobile-session-route-types' // Why: tab identity + title cleaning uses the same shared glyph/label maps as // desktop, so the two platforms do not drift on which titles identify agents. diff --git a/mobile/src/terminal/terminal-live-input-affordance.test.ts b/mobile/src/terminal/terminal-live-input-affordance.test.ts index cc681e2056e..0125cb053e6 100644 --- a/mobile/src/terminal/terminal-live-input-affordance.test.ts +++ b/mobile/src/terminal/terminal-live-input-affordance.test.ts @@ -10,7 +10,7 @@ const liveInputStatusSource = readFileSync( 'utf8' ) const commandInputStylesSource = readFileSync( - new URL('../../app/h/[hostId]/session/mobile-session-command-input-styles.ts', import.meta.url), + new URL('../session/mobile-session-command-input-styles.ts', import.meta.url), 'utf8' ) const liveInputFocusSource = readFileSync( From 3c329c43b12f6b577e05dc495daecf5fc4803d16 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Sun, 2 Aug 2026 23:03:25 +0000 Subject: [PATCH 008/106] release: v1.4.165-rc.0 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index b68ff2de99a..d2ccfa30283 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "orca", - "version": "1.4.164-rc.2", + "version": "1.4.165-rc.0", "description": "Next-gen IDE for parallel agentic development", "homepage": "https://github.com/stablyai/orca", "author": "stablyai", From 7c7167028cb4c1fc6ae3601a3a65e5f5a6a07d6b Mon Sep 17 00:00:00 2001 From: Kyle Date: Mon, 3 Aug 2026 07:25:56 +0800 Subject: [PATCH 009/106] feat(voice): allow selecting a microphone for dictation (#12119) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(voice): allow selecting a microphone for dictation Persist a preferred audioinput device in Voice settings and pass it into getUserMedia, falling back to the system default when the device is gone. * fix(voice): resolve mic preference by label and detect mid-capture loss Drop Chromium's 'default'/'communications' aliases from the picker — pinning one behaved exactly like system default and silently defeated the setting. Resolve a stored preference against the live device list before capturing: a unique label match heals an id that Chromium re-salted, a known-missing device skips the doomed getUserMedia attempt that clipped the first words, and an unreadable list no longer reads as "unplugged". Surface the input ending mid-dictation instead of feeding silent zeros, add a permission affordance so the picker is not empty before mic access, and toast the fallback once per preference rather than once per utterance. Co-authored-by: Orca * add e2e tests * add e2e tests --------- Co-authored-by: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Co-authored-by: Orca --- .../dictation/DictationController.tsx | 46 ++- .../dictation/microphone-devices.test.ts | 372 ++++++++++++++++++ .../dictation/microphone-devices.ts | 260 ++++++++++++ .../VoiceDictationSettingsSection.tsx | 8 + .../settings/VoiceMicrophoneSetting.tsx | 205 ++++++++++ .../components/settings/voice-pane-search.ts | 25 ++ .../use-audio-capture.capture-loss.test.ts | 83 ++++ src/renderer/src/hooks/use-audio-capture.ts | 51 ++- src/renderer/src/i18n/locales/en.json | 20 +- src/shared/constants.ts | 4 +- src/shared/speech-types.ts | 4 + tests/e2e/voice-microphone-selection.spec.ts | 177 +++++++++ 12 files changed, 1243 insertions(+), 12 deletions(-) create mode 100644 src/renderer/src/components/dictation/microphone-devices.test.ts create mode 100644 src/renderer/src/components/dictation/microphone-devices.ts create mode 100644 src/renderer/src/components/settings/VoiceMicrophoneSetting.tsx create mode 100644 src/renderer/src/hooks/use-audio-capture.capture-loss.test.ts create mode 100644 tests/e2e/voice-microphone-selection.spec.ts diff --git a/src/renderer/src/components/dictation/DictationController.tsx b/src/renderer/src/components/dictation/DictationController.tsx index 1d8486becf5..7985c13befa 100644 --- a/src/renderer/src/components/dictation/DictationController.tsx +++ b/src/renderer/src/components/dictation/DictationController.tsx @@ -43,6 +43,10 @@ export function DictationController() { const erroredSessionIdsRef = useRef(new Set()) const intentionalTargetCancellationRef = useRef(false) const insertedFinalTranscriptRef = useRef('') + // Why: push-to-talk restarts capture per utterance; toast once per preference, + // not once per press, while the selected mic stays gone. + const micFallbackNotifiedForRef = useRef(null) + const stopDictationRef = useRef<(() => void) | null>(null) const drainStoppedSession = useCallback((sessionId: string) => { void waitForStoppedSession(sessionId, stoppedSessionIdsRef, stoppedResolversRef) @@ -131,8 +135,44 @@ export function DictationController() { try { // Why: worker startup can take seconds after idle teardown. Capture first // and buffer locally so speech during "Starting..." is not discarded. - await startCapture({ bufferAudio: true, sessionId }) + const preferredMicrophoneDeviceId = settings?.voice?.microphoneDeviceId ?? null + const captureResult = await startCapture({ + bufferAudio: true, + sessionId, + microphoneDeviceId: preferredMicrophoneDeviceId, + microphoneDeviceLabel: settings?.voice?.microphoneDeviceLabel ?? null, + onCaptureLost: () => { + if (dictationRunRef.current !== runId) { + return + } + toast.message( + translate( + 'auto.components.dictation.DictationController.micDisconnected', + 'Microphone disconnected. Dictation stopped.' + ) + ) + stopDictationRef.current?.() + } + }) captureStarted = true + if (captureResult?.fellBackToDefaultMicrophone) { + // Why: a stop requested during startup tears this capture down below, so the + // notice would describe a fallback that never records anything. + if ( + !stopRequestedDuringStartRef.current && + micFallbackNotifiedForRef.current !== preferredMicrophoneDeviceId + ) { + micFallbackNotifiedForRef.current = preferredMicrophoneDeviceId + toast.message( + translate( + 'auto.components.dictation.DictationController.micFallback', + 'Selected microphone unavailable. Using system default.' + ) + ) + } + } else { + micFallbackNotifiedForRef.current = null + } if (stopRequestedDuringStartRef.current) { stopCapture({ preserveBufferedAudio: true }) } @@ -233,6 +273,10 @@ export function DictationController() { await finishDictationSession(sessionId) }, [finishDictationSession, setDictationState, stopCapture]) + // Why: capture-loss fires from a stream opened before stopDictation exists; + // route through a ref so the two callbacks do not depend on each other. + stopDictationRef.current = () => void stopDictation() + // Toggle mode: use IPC from main process (before-input-event intercepts // the keyDown so Cmd+E doesn't reach xterm or trigger system shortcuts). useEffect(() => { diff --git a/src/renderer/src/components/dictation/microphone-devices.test.ts b/src/renderer/src/components/dictation/microphone-devices.test.ts new file mode 100644 index 00000000000..d449e758d3d --- /dev/null +++ b/src/renderer/src/components/dictation/microphone-devices.test.ts @@ -0,0 +1,372 @@ +import { describe, expect, it, vi } from 'vitest' +import { getDefaultVoiceSettings } from '../../../../shared/constants' +import { + buildAudioCaptureConstraints, + buildVoiceMicrophoneSelectOptions, + isMicrophoneDeviceConstraintError, + listVoiceMicrophoneDevices, + microphoneDeviceIdFromSelectValue, + microphoneSelectValueFromDeviceId, + normalizeMicrophoneDeviceId, + openMicrophoneCaptureStream, + resolveMicrophoneDevice, + SYSTEM_DEFAULT_MICROPHONE_SELECT_VALUE +} from './microphone-devices' + +describe('normalizeMicrophoneDeviceId', () => { + it('treats empty and whitespace as system default', () => { + expect(normalizeMicrophoneDeviceId(null)).toBeNull() + expect(normalizeMicrophoneDeviceId(undefined)).toBeNull() + expect(normalizeMicrophoneDeviceId('')).toBeNull() + expect(normalizeMicrophoneDeviceId(' ')).toBeNull() + }) + + it('treats Chromium aggregate ids as system default', () => { + expect(normalizeMicrophoneDeviceId('default')).toBeNull() + expect(normalizeMicrophoneDeviceId('communications')).toBeNull() + }) + + it('preserves non-empty device ids', () => { + expect(normalizeMicrophoneDeviceId('abc-123')).toBe('abc-123') + }) +}) + +describe('getDefaultVoiceSettings microphone fields', () => { + it('defaults microphone selection to system default', () => { + const voice = getDefaultVoiceSettings() + expect(voice.microphoneDeviceId).toBeNull() + expect(voice.microphoneDeviceLabel).toBeNull() + }) +}) + +describe('buildAudioCaptureConstraints', () => { + it('omits deviceId for system default', () => { + expect(buildAudioCaptureConstraints(null)).toEqual({ + audio: { + channelCount: 1, + echoCancellation: true, + noiseSuppression: true, + autoGainControl: true + } + }) + }) + + it('pins exact deviceId when a mic is selected', () => { + expect(buildAudioCaptureConstraints('usb-mic-1')).toEqual({ + audio: { + channelCount: 1, + echoCancellation: true, + noiseSuppression: true, + autoGainControl: true, + deviceId: { exact: 'usb-mic-1' } + } + }) + }) + + it('does not pin the aggregate default id', () => { + expect(buildAudioCaptureConstraints('default')).toEqual(buildAudioCaptureConstraints(null)) + }) +}) + +describe('isMicrophoneDeviceConstraintError', () => { + it('matches OverconstrainedError and NotFoundError only', () => { + expect(isMicrophoneDeviceConstraintError(new DOMException('x', 'OverconstrainedError'))).toBe( + true + ) + expect(isMicrophoneDeviceConstraintError(new DOMException('x', 'NotFoundError'))).toBe(true) + expect(isMicrophoneDeviceConstraintError(new DOMException('x', 'NotAllowedError'))).toBe(false) + expect(isMicrophoneDeviceConstraintError(new Error('boom'))).toBe(false) + expect(isMicrophoneDeviceConstraintError(null)).toBe(false) + }) +}) + +describe('resolveMicrophoneDevice', () => { + const devices = [ + { deviceId: 'mic-1', label: 'Built-in' }, + { deviceId: 'mic-2', label: 'AirPods' } + ] + + it('reports system default when nothing is preferred', () => { + expect( + resolveMicrophoneDevice({ devices, preferredDeviceId: null, preferredDeviceLabel: null }) + ).toEqual({ deviceId: null, kind: 'system-default' }) + }) + + it('matches by id first', () => { + expect( + resolveMicrophoneDevice({ + devices, + preferredDeviceId: 'mic-2', + preferredDeviceLabel: 'Built-in' + }) + ).toEqual({ deviceId: 'mic-2', kind: 'exact' }) + }) + + it('heals a re-salted id via a unique label match', () => { + expect( + resolveMicrophoneDevice({ + devices, + preferredDeviceId: 'stale-id', + preferredDeviceLabel: ' airpods ' + }) + ).toEqual({ deviceId: 'mic-2', kind: 'relabeled' }) + }) + + it('refuses to guess between duplicate labels', () => { + expect( + resolveMicrophoneDevice({ + devices: [ + { deviceId: 'a', label: 'Yeti' }, + { deviceId: 'b', label: 'Yeti' } + ], + preferredDeviceId: 'stale-id', + preferredDeviceLabel: 'Yeti' + }) + ).toEqual({ deviceId: null, kind: 'missing' }) + }) + + it('does not treat an unreadable list as a missing device', () => { + expect( + resolveMicrophoneDevice({ + devices: null, + preferredDeviceId: 'mic-9', + preferredDeviceLabel: null + }) + ).toEqual({ deviceId: 'mic-9', kind: 'unknown' }) + expect( + resolveMicrophoneDevice({ + devices: [], + preferredDeviceId: 'mic-9', + preferredDeviceLabel: null + }) + ).toEqual({ deviceId: 'mic-9', kind: 'unknown' }) + }) +}) + +describe('openMicrophoneCaptureStream', () => { + it('opens with preferred device when available', async () => { + const stream = { id: 'preferred' } as unknown as MediaStream + const getUserMedia = vi.fn().mockResolvedValue(stream) + + await expect( + openMicrophoneCaptureStream({ + preferredDeviceId: 'mic-a', + getUserMedia + }) + ).resolves.toEqual({ + stream, + fellBackToDefaultMicrophone: false, + usedDeviceId: 'mic-a' + }) + + expect(getUserMedia).toHaveBeenCalledTimes(1) + expect(getUserMedia).toHaveBeenCalledWith(buildAudioCaptureConstraints('mic-a')) + }) + + it('skips the doomed attempt when enumeration proves the device is gone', async () => { + const fallbackStream = { id: 'default' } as unknown as MediaStream + const getUserMedia = vi.fn().mockResolvedValue(fallbackStream) + const enumerateDevices = vi + .fn() + .mockResolvedValue([{ deviceId: 'mic-1', kind: 'audioinput', label: 'Built-in' }]) + + await expect( + openMicrophoneCaptureStream({ + preferredDeviceId: 'unplugged', + preferredDeviceLabel: 'Unplugged Mic', + getUserMedia, + enumerateDevices + }) + ).resolves.toEqual({ + stream: fallbackStream, + fellBackToDefaultMicrophone: true, + usedDeviceId: null + }) + + expect(getUserMedia).toHaveBeenCalledTimes(1) + expect(getUserMedia).toHaveBeenCalledWith(buildAudioCaptureConstraints(null)) + }) + + it('captures on the relabeled device after an id rotation', async () => { + const stream = { id: 'healed' } as unknown as MediaStream + const getUserMedia = vi.fn().mockResolvedValue(stream) + const enumerateDevices = vi + .fn() + .mockResolvedValue([{ deviceId: 'fresh-id', kind: 'audioinput', label: 'Yeti' }]) + + await expect( + openMicrophoneCaptureStream({ + preferredDeviceId: 'stale-id', + preferredDeviceLabel: 'Yeti', + getUserMedia, + enumerateDevices + }) + ).resolves.toEqual({ + stream, + fellBackToDefaultMicrophone: false, + usedDeviceId: 'fresh-id' + }) + + expect(getUserMedia).toHaveBeenCalledWith(buildAudioCaptureConstraints('fresh-id')) + }) + + it('still falls back when the device vanishes after enumeration', async () => { + const fallbackStream = { id: 'default' } as unknown as MediaStream + const getUserMedia = vi + .fn() + .mockRejectedValueOnce(new DOMException('gone', 'OverconstrainedError')) + .mockResolvedValueOnce(fallbackStream) + + await expect( + openMicrophoneCaptureStream({ + preferredDeviceId: 'missing-mic', + getUserMedia + }) + ).resolves.toEqual({ + stream: fallbackStream, + fellBackToDefaultMicrophone: true, + usedDeviceId: null + }) + + expect(getUserMedia).toHaveBeenNthCalledWith(1, buildAudioCaptureConstraints('missing-mic')) + expect(getUserMedia).toHaveBeenNthCalledWith(2, buildAudioCaptureConstraints(null)) + }) + + it('keeps the preference when the device list cannot be read', async () => { + const stream = { id: 'preferred' } as unknown as MediaStream + const getUserMedia = vi.fn().mockResolvedValue(stream) + const enumerateDevices = vi.fn().mockRejectedValue(new Error('nope')) + + await expect( + openMicrophoneCaptureStream({ + preferredDeviceId: 'mic-a', + getUserMedia, + enumerateDevices + }) + ).resolves.toMatchObject({ usedDeviceId: 'mic-a', fellBackToDefaultMicrophone: false }) + }) + + it('does not fall back on permission errors', async () => { + const getUserMedia = vi.fn().mockRejectedValue(new DOMException('denied', 'NotAllowedError')) + + await expect( + openMicrophoneCaptureStream({ + preferredDeviceId: 'mic-a', + getUserMedia + }) + ).rejects.toMatchObject({ name: 'NotAllowedError' }) + + expect(getUserMedia).toHaveBeenCalledTimes(1) + }) +}) + +describe('listVoiceMicrophoneDevices', () => { + it('keeps audio inputs and fills empty labels', () => { + expect( + listVoiceMicrophoneDevices([ + { deviceId: 'out-1', kind: 'audiooutput', label: 'Speakers' }, + { deviceId: 'mic-1', kind: 'audioinput', label: 'Built-in' }, + { deviceId: 'mic-2', kind: 'audioinput', label: '' }, + { deviceId: '', kind: 'audioinput', label: 'Empty id' } + ]) + ).toEqual([ + { deviceId: 'mic-1', label: 'Built-in' }, + { deviceId: 'mic-2', label: 'Microphone 2' } + ]) + }) + + it('drops the Chromium default and communications aliases', () => { + expect( + listVoiceMicrophoneDevices([ + { deviceId: 'default', kind: 'audioinput', label: 'Default - Built-in Microphone' }, + { deviceId: 'communications', kind: 'audioinput', label: 'Communications - Headset' }, + { deviceId: 'mic-1', kind: 'audioinput', label: 'Built-in Microphone' } + ]) + ).toEqual([{ deviceId: 'mic-1', label: 'Built-in Microphone' }]) + }) +}) + +describe('microphone select values', () => { + it('round-trips system default and concrete device ids', () => { + expect(microphoneSelectValueFromDeviceId(null)).toBe(SYSTEM_DEFAULT_MICROPHONE_SELECT_VALUE) + expect(microphoneDeviceIdFromSelectValue(SYSTEM_DEFAULT_MICROPHONE_SELECT_VALUE)).toBeNull() + expect(microphoneDeviceIdFromSelectValue('usb-1')).toBe('usb-1') + }) +}) + +describe('buildVoiceMicrophoneSelectOptions', () => { + const labels = { systemDefaultLabel: 'System default', unavailableSuffix: 'unavailable' } + + it('includes system default and available devices', () => { + expect( + buildVoiceMicrophoneSelectOptions({ + devices: [{ deviceId: 'mic-1', label: 'Built-in' }], + devicesKnown: true, + preferredDeviceId: null, + preferredDeviceLabel: null, + ...labels + }) + ).toEqual({ + options: [ + { value: SYSTEM_DEFAULT_MICROPHONE_SELECT_VALUE, label: 'System default' }, + { value: 'mic-1', label: 'Built-in' } + ], + selectedValue: SYSTEM_DEFAULT_MICROPHONE_SELECT_VALUE + }) + }) + + it('keeps a missing preferred device as an unavailable option', () => { + expect( + buildVoiceMicrophoneSelectOptions({ + devices: [{ deviceId: 'mic-1', label: 'Built-in' }], + devicesKnown: true, + preferredDeviceId: 'airpods', + preferredDeviceLabel: 'AirPods', + ...labels + }) + ).toEqual({ + options: [ + { value: SYSTEM_DEFAULT_MICROPHONE_SELECT_VALUE, label: 'System default' }, + { value: 'mic-1', label: 'Built-in' }, + { value: 'airpods', label: 'AirPods (unavailable)', unavailable: true } + ], + selectedValue: 'airpods' + }) + }) + + it('selects the relabeled device instead of showing a ghost row', () => { + expect( + buildVoiceMicrophoneSelectOptions({ + devices: [{ deviceId: 'fresh-id', label: 'AirPods' }], + devicesKnown: true, + preferredDeviceId: 'stale-id', + preferredDeviceLabel: 'AirPods', + ...labels + }) + ).toEqual({ + options: [ + { value: SYSTEM_DEFAULT_MICROPHONE_SELECT_VALUE, label: 'System default' }, + { value: 'fresh-id', label: 'AirPods' } + ], + selectedValue: 'fresh-id' + }) + }) + + it('does not label a device unavailable before the list is known', () => { + expect( + buildVoiceMicrophoneSelectOptions({ + devices: [], + devicesKnown: false, + preferredDeviceId: 'airpods', + preferredDeviceLabel: 'AirPods', + ...labels + }) + ).toEqual({ + options: [ + { value: SYSTEM_DEFAULT_MICROPHONE_SELECT_VALUE, label: 'System default' }, + { value: 'airpods', label: 'AirPods' } + ], + selectedValue: 'airpods' + }) + }) +}) diff --git a/src/renderer/src/components/dictation/microphone-devices.ts b/src/renderer/src/components/dictation/microphone-devices.ts new file mode 100644 index 00000000000..9f97a6f286c --- /dev/null +++ b/src/renderer/src/components/dictation/microphone-devices.ts @@ -0,0 +1,260 @@ +export type VoiceMicrophoneDevice = { + deviceId: string + label: string +} + +export type VoiceMicrophoneSelectOption = { + value: string + label: string + unavailable?: boolean +} + +export type MicrophoneResolutionKind = + | 'system-default' + | 'exact' + | 'relabeled' + | 'missing' + | 'unknown' + +export type MicrophoneResolution = { + deviceId: string | null + kind: MicrophoneResolutionKind +} + +export type OpenMicrophoneCaptureStreamResult = { + stream: MediaStream + fellBackToDefaultMicrophone: boolean + usedDeviceId: string | null +} + +export type EnumeratedDevice = Pick + +export const SYSTEM_DEFAULT_MICROPHONE_SELECT_VALUE = 'system-default' + +// Why: Chromium synthesizes these ids as aliases for whatever the OS default is, +// so pinning one behaves exactly like "system default" and defeats the setting. +const AGGREGATE_DEVICE_IDS = new Set(['default', 'communications']) + +const BASE_AUDIO_CONSTRAINTS: MediaTrackConstraints = { + channelCount: 1, + echoCancellation: true, + noiseSuppression: true, + autoGainControl: true +} + +export function normalizeMicrophoneDeviceId(deviceId: string | null | undefined): string | null { + if (typeof deviceId !== 'string') { + return null + } + const trimmed = deviceId.trim() + if (trimmed.length === 0) { + return null + } + return AGGREGATE_DEVICE_IDS.has(trimmed) ? null : trimmed +} + +export function buildAudioCaptureConstraints( + deviceId: string | null | undefined +): MediaStreamConstraints { + const preferredDeviceId = normalizeMicrophoneDeviceId(deviceId) + if (!preferredDeviceId) { + return { audio: { ...BASE_AUDIO_CONSTRAINTS } } + } + return { + audio: { + ...BASE_AUDIO_CONSTRAINTS, + deviceId: { exact: preferredDeviceId } + } + } +} + +export function isMicrophoneDeviceConstraintError(error: unknown): boolean { + if (!error || typeof error !== 'object') { + return false + } + const name = 'name' in error ? String(error.name) : '' + // Why: exact deviceId fails as OverconstrainedError; unplugged devices often surface as NotFoundError. + return name === 'OverconstrainedError' || name === 'NotFoundError' +} + +export function listVoiceMicrophoneDevices( + devices: readonly EnumeratedDevice[] +): VoiceMicrophoneDevice[] { + return devices + .filter( + (device) => + device.kind === 'audioinput' && normalizeMicrophoneDeviceId(device.deviceId) !== null + ) + .map((device, index) => ({ + deviceId: device.deviceId.trim(), + label: device.label.trim() || `Microphone ${index + 1}` + })) +} + +function findSoleDeviceByLabel( + devices: readonly VoiceMicrophoneDevice[], + label: string | null | undefined +): VoiceMicrophoneDevice | null { + const wanted = label?.trim().toLowerCase() + if (!wanted) { + return null + } + const matches = devices.filter((device) => device.label.trim().toLowerCase() === wanted) + // Why: identical labels (two of the same headset) give no basis to pick one. + return matches.length === 1 ? (matches[0] ?? null) : null +} + +/** + * Resolves a stored preference against the live device list. Pass `devices: null` + * when the list could not be read — that is not evidence the device is gone. + */ +export function resolveMicrophoneDevice(args: { + devices: readonly VoiceMicrophoneDevice[] | null + preferredDeviceId: string | null | undefined + preferredDeviceLabel: string | null | undefined +}): MicrophoneResolution { + const preferredDeviceId = normalizeMicrophoneDeviceId(args.preferredDeviceId) + if (!preferredDeviceId) { + return { deviceId: null, kind: 'system-default' } + } + // Why: before mic permission, enumeration returns only blank placeholders, so an + // empty list means "cannot tell yet" rather than "unplugged". + if (!args.devices || args.devices.length === 0) { + return { deviceId: preferredDeviceId, kind: 'unknown' } + } + if (args.devices.some((device) => device.deviceId === preferredDeviceId)) { + return { deviceId: preferredDeviceId, kind: 'exact' } + } + // Why: Chromium re-salts device ids per profile+origin, so a surviving label match + // heals a preference whose id rotated instead of stranding it forever. + const relabeled = findSoleDeviceByLabel(args.devices, args.preferredDeviceLabel) + if (relabeled) { + return { deviceId: relabeled.deviceId, kind: 'relabeled' } + } + return { deviceId: null, kind: 'missing' } +} + +async function enumerateMicrophonesOrNull( + enumerateDevices: (() => Promise) | undefined +): Promise { + if (!enumerateDevices) { + return null + } + try { + return listVoiceMicrophoneDevices(await enumerateDevices()) + } catch { + return null + } +} + +export async function openMicrophoneCaptureStream(args: { + preferredDeviceId: string | null | undefined + preferredDeviceLabel?: string | null + getUserMedia: (constraints: MediaStreamConstraints) => Promise + enumerateDevices?: () => Promise +}): Promise { + const preferredDeviceId = normalizeMicrophoneDeviceId(args.preferredDeviceId) + if (!preferredDeviceId) { + return { + stream: await args.getUserMedia(buildAudioCaptureConstraints(null)), + fellBackToDefaultMicrophone: false, + usedDeviceId: null + } + } + + const resolution = resolveMicrophoneDevice({ + devices: await enumerateMicrophonesOrNull(args.enumerateDevices), + preferredDeviceId, + preferredDeviceLabel: args.preferredDeviceLabel + }) + + // Why: enumeration is a cheap cached lookup, so a known-missing device skips the + // failed getUserMedia round trip that would otherwise clip the first words. + if (resolution.kind === 'missing') { + return { + stream: await args.getUserMedia(buildAudioCaptureConstraints(null)), + fellBackToDefaultMicrophone: true, + usedDeviceId: null + } + } + + const targetDeviceId = resolution.deviceId ?? preferredDeviceId + try { + const stream = await args.getUserMedia(buildAudioCaptureConstraints(targetDeviceId)) + return { stream, fellBackToDefaultMicrophone: false, usedDeviceId: targetDeviceId } + } catch (error) { + // Why: still needed — the device can vanish between enumeration and capture. + if (!isMicrophoneDeviceConstraintError(error)) { + throw error + } + return { + stream: await args.getUserMedia(buildAudioCaptureConstraints(null)), + fellBackToDefaultMicrophone: true, + usedDeviceId: null + } + } +} + +export function microphoneSelectValueFromDeviceId(deviceId: string | null | undefined): string { + return normalizeMicrophoneDeviceId(deviceId) ?? SYSTEM_DEFAULT_MICROPHONE_SELECT_VALUE +} + +export function microphoneDeviceIdFromSelectValue(value: string): string | null { + if (value === SYSTEM_DEFAULT_MICROPHONE_SELECT_VALUE) { + return null + } + return normalizeMicrophoneDeviceId(value) +} + +export function buildVoiceMicrophoneSelectOptions(args: { + devices: readonly VoiceMicrophoneDevice[] + /** false until enumeration has produced a usable list */ + devicesKnown: boolean + preferredDeviceId: string | null | undefined + preferredDeviceLabel: string | null | undefined + systemDefaultLabel: string + unavailableSuffix: string +}): { options: VoiceMicrophoneSelectOption[]; selectedValue: string } { + const options: VoiceMicrophoneSelectOption[] = [ + { + value: SYSTEM_DEFAULT_MICROPHONE_SELECT_VALUE, + label: args.systemDefaultLabel + }, + ...args.devices.map((device) => ({ + value: device.deviceId, + label: device.label + })) + ] + + const resolution = resolveMicrophoneDevice({ + devices: args.devicesKnown ? args.devices : null, + preferredDeviceId: args.preferredDeviceId, + preferredDeviceLabel: args.preferredDeviceLabel + }) + + if (resolution.kind === 'system-default') { + return { options, selectedValue: SYSTEM_DEFAULT_MICROPHONE_SELECT_VALUE } + } + if (resolution.kind === 'exact' || resolution.kind === 'relabeled') { + return { options, selectedValue: resolution.deviceId ?? SYSTEM_DEFAULT_MICROPHONE_SELECT_VALUE } + } + + const preferredDeviceId = normalizeMicrophoneDeviceId(args.preferredDeviceId) + if (!preferredDeviceId) { + return { options, selectedValue: SYSTEM_DEFAULT_MICROPHONE_SELECT_VALUE } + } + const cachedLabel = args.preferredDeviceLabel?.trim() + const baseLabel = cachedLabel && cachedLabel.length > 0 ? cachedLabel : preferredDeviceId + // Why: only call it unavailable once we have actually seen the device list — + // an un-enumerated list would otherwise flag every mic as missing. + options.push( + resolution.kind === 'missing' + ? { + value: preferredDeviceId, + label: `${baseLabel} (${args.unavailableSuffix})`, + unavailable: true + } + : { value: preferredDeviceId, label: baseLabel } + ) + return { options, selectedValue: preferredDeviceId } +} diff --git a/src/renderer/src/components/settings/VoiceDictationSettingsSection.tsx b/src/renderer/src/components/settings/VoiceDictationSettingsSection.tsx index f7deec5cd3c..f148e631dc4 100644 --- a/src/renderer/src/components/settings/VoiceDictationSettingsSection.tsx +++ b/src/renderer/src/components/settings/VoiceDictationSettingsSection.tsx @@ -3,6 +3,7 @@ import { Label } from '../ui/label' import { Separator } from '../ui/separator' import { useShortcutLabel } from '@/hooks/useShortcutLabel' import { translate } from '@/i18n/i18n' +import { VoiceMicrophoneSetting } from './VoiceMicrophoneSetting' type VoiceDictationSettingsSectionProps = { voiceSettings: VoiceSettings @@ -94,6 +95,13 @@ export function VoiceDictationSettingsSection({ + + + + ) } diff --git a/src/renderer/src/components/settings/VoiceMicrophoneSetting.tsx b/src/renderer/src/components/settings/VoiceMicrophoneSetting.tsx new file mode 100644 index 00000000000..b5dd245db11 --- /dev/null +++ b/src/renderer/src/components/settings/VoiceMicrophoneSetting.tsx @@ -0,0 +1,205 @@ +import { useCallback, useEffect, useMemo, useRef, useState } from 'react' +import type { VoiceSettings } from '../../../../shared/speech-types' +import { Button } from '../ui/button' +import { Label } from '../ui/label' +import { Select, SelectContent, SelectItem, SelectTrigger, SelectValue } from '../ui/select' +import { + buildVoiceMicrophoneSelectOptions, + listVoiceMicrophoneDevices, + microphoneDeviceIdFromSelectValue, + type VoiceMicrophoneDevice +} from '@/components/dictation/microphone-devices' +import { translate } from '@/i18n/i18n' + +type VoiceMicrophoneSettingProps = { + voiceSettings: VoiceSettings + onUpdateVoiceSettings: (updates: Partial) => void +} + +function sameDeviceList( + a: readonly VoiceMicrophoneDevice[], + b: readonly VoiceMicrophoneDevice[] +): boolean { + return ( + a.length === b.length && + a.every((device, index) => { + const other = b[index] + return device.deviceId === other?.deviceId && device.label === other.label + }) + ) +} + +export function VoiceMicrophoneSetting({ + voiceSettings, + onUpdateVoiceSettings +}: VoiceMicrophoneSettingProps): React.JSX.Element { + const [devices, setDevices] = useState([]) + const [devicesKnown, setDevicesKnown] = useState(false) + const [accessPending, setAccessPending] = useState(false) + const mountedRef = useRef(true) + // Why: devicechange fires several times per Bluetooth connect; drop enumerations + // that resolve out of order so a stale list cannot land last. + const refreshGenerationRef = useRef(0) + + useEffect(() => { + mountedRef.current = true + return () => { + mountedRef.current = false + } + }, []) + + const refreshDevices = useCallback(async (): Promise => { + const generation = refreshGenerationRef.current + 1 + refreshGenerationRef.current = generation + if (typeof navigator === 'undefined' || !navigator.mediaDevices?.enumerateDevices) { + return + } + let next: VoiceMicrophoneDevice[] = [] + try { + next = listVoiceMicrophoneDevices(await navigator.mediaDevices.enumerateDevices()) + } catch { + next = [] + } + if (!mountedRef.current || refreshGenerationRef.current !== generation) { + return + } + setDevicesKnown(next.length > 0) + setDevices((current) => (sameDeviceList(current, next) ? current : next)) + }, []) + + // Why: voiceSettings.enabled is a dependency so enabling dictation re-scans — + // that toggle is often when mic permission lands and real labels appear. + useEffect(() => { + void refreshDevices() + if (typeof navigator === 'undefined' || !navigator.mediaDevices?.addEventListener) { + return + } + const handleDeviceChange = (): void => { + void refreshDevices() + } + navigator.mediaDevices.addEventListener('devicechange', handleDeviceChange) + return () => { + navigator.mediaDevices.removeEventListener('devicechange', handleDeviceChange) + } + }, [refreshDevices, voiceSettings.enabled]) + + // Why: enumerateDevices hides ids and labels until mic permission is granted, so + // the list stays empty until something opens a stream at least once. + const requestMicrophoneAccess = useCallback(async (): Promise => { + if (typeof navigator === 'undefined' || !navigator.mediaDevices?.getUserMedia) { + return + } + setAccessPending(true) + try { + const stream = await navigator.mediaDevices.getUserMedia({ audio: true }) + stream.getTracks().forEach((track) => track.stop()) + await refreshDevices() + } catch { + // Denied or unavailable — the hint stays visible so the user can retry. + } finally { + if (mountedRef.current) { + setAccessPending(false) + } + } + }, [refreshDevices]) + + const { options, selectedValue } = useMemo( + () => + buildVoiceMicrophoneSelectOptions({ + devices, + devicesKnown, + preferredDeviceId: voiceSettings.microphoneDeviceId, + preferredDeviceLabel: voiceSettings.microphoneDeviceLabel, + systemDefaultLabel: translate( + 'auto.components.settings.VoiceMicrophoneSetting.systemDefault', + 'System default' + ), + unavailableSuffix: translate( + 'auto.components.settings.VoiceMicrophoneSetting.unavailable', + 'unavailable' + ) + }), + [devices, devicesKnown, voiceSettings.microphoneDeviceId, voiceSettings.microphoneDeviceLabel] + ) + + const showAccessHint = voiceSettings.enabled && devices.length === 0 + + return ( +
+
+ +

+ {translate( + 'auto.components.settings.VoiceMicrophoneSetting.description', + 'Input device used for voice dictation. System default follows the OS microphone setting.' + )} +

+ {showAccessHint && ( +
+

+ {translate( + 'auto.components.settings.VoiceMicrophoneSetting.accessHint', + 'Allow microphone access to list input devices.' + )} +

+ +
+ )} +
+ +
+ ) +} diff --git a/src/renderer/src/components/settings/voice-pane-search.ts b/src/renderer/src/components/settings/voice-pane-search.ts index 4568f65d7bd..7dbf56c8b57 100644 --- a/src/renderer/src/components/settings/voice-pane-search.ts +++ b/src/renderer/src/components/settings/voice-pane-search.ts @@ -73,6 +73,31 @@ export const getVoicePaneSearchEntries = createLocalizedCatalog(() => [ ) ] }, + { + title: translate('auto.components.settings.voice.pane.search.microphoneTitle', 'Microphone'), + description: translate( + 'auto.components.settings.voice.pane.search.microphoneDescription', + 'Choose which input device voice dictation uses.' + ), + keywords: [ + ...translateSearchKeyword('auto.components.settings.voice.pane.search.7640ed9848', 'voice'), + ...translateSearchKeyword( + 'auto.components.settings.voice.pane.search.089d31a45b', + 'dictation' + ), + ...translateSearchKeyword( + 'auto.components.settings.voice.pane.search.e360027a65', + 'microphone' + ), + ...translateSearchKeyword('auto.components.settings.voice.pane.search.micInput', 'input'), + ...translateSearchKeyword('auto.components.settings.voice.pane.search.micDevice', 'device'), + ...translateSearchKeyword('auto.components.settings.voice.pane.search.micAirpods', 'airpods'), + ...translateSearchKeyword( + 'auto.components.settings.voice.pane.search.micDefault', + 'system default' + ) + ] + }, getOpenaiTranscriptionSearchEntry(), { title: translate('auto.components.settings.voice.pane.search.7e62cd7c41', 'Speech Model'), diff --git a/src/renderer/src/hooks/use-audio-capture.capture-loss.test.ts b/src/renderer/src/hooks/use-audio-capture.capture-loss.test.ts new file mode 100644 index 00000000000..2f9dd125e91 --- /dev/null +++ b/src/renderer/src/hooks/use-audio-capture.capture-loss.test.ts @@ -0,0 +1,83 @@ +// @vitest-environment happy-dom + +import { renderHook } from '@testing-library/react' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { useAudioCapture } from './use-audio-capture' + +class FakeTrack extends EventTarget { + stop = vi.fn() +} + +function makeStream(track: FakeTrack): MediaStream { + return { + getTracks: () => [track], + getAudioTracks: () => [track] + } as unknown as MediaStream +} + +function installAudioContext(): void { + const processor = { + connect: vi.fn(), + disconnect: vi.fn(), + onaudioprocess: null + } + const source = { connect: vi.fn(), disconnect: vi.fn() } + vi.stubGlobal( + 'AudioContext', + class { + state = 'running' + sampleRate = 48_000 + destination = {} + createMediaStreamSource = vi.fn(() => source) + createScriptProcessor = vi.fn(() => processor) + resume = vi.fn(async () => undefined) + close = vi.fn(async () => undefined) + } + ) +} + +describe('useAudioCapture device loss', () => { + let track: FakeTrack + + beforeEach(() => { + track = new FakeTrack() + installAudioContext() + vi.stubGlobal('navigator', { + mediaDevices: { + getUserMedia: vi.fn(async () => makeStream(track)), + enumerateDevices: vi.fn(async () => [ + { deviceId: 'mic-1', kind: 'audioinput', label: 'Built-in' } + ]) + } + }) + vi.stubGlobal('window', { + ...globalThis.window, + api: { speech: { feedAudio: vi.fn(async () => undefined) } } + }) + }) + + afterEach(() => { + vi.unstubAllGlobals() + }) + + it('reports the device disappearing mid-capture', async () => { + const onCaptureLost = vi.fn() + const { result } = renderHook(() => useAudioCapture()) + + await result.current.start({ microphoneDeviceId: 'mic-1', onCaptureLost }) + track.dispatchEvent(new Event('ended')) + + expect(onCaptureLost).toHaveBeenCalledTimes(1) + }) + + it('stays silent when we stopped capture ourselves', async () => { + const onCaptureLost = vi.fn() + const { result } = renderHook(() => useAudioCapture()) + + await result.current.start({ microphoneDeviceId: 'mic-1', onCaptureLost }) + result.current.stop() + track.dispatchEvent(new Event('ended')) + + expect(onCaptureLost).not.toHaveBeenCalled() + }) +}) diff --git a/src/renderer/src/hooks/use-audio-capture.ts b/src/renderer/src/hooks/use-audio-capture.ts index 7348bce2419..025e29092e2 100644 --- a/src/renderer/src/hooks/use-audio-capture.ts +++ b/src/renderer/src/hooks/use-audio-capture.ts @@ -1,4 +1,5 @@ import { useRef, useCallback } from 'react' +import { openMicrophoneCaptureStream } from '@/components/dictation/microphone-devices' type BufferedAudioChunk = { samples: Float32Array @@ -9,6 +10,16 @@ type BufferedAudioChunk = { type StartAudioCaptureOptions = { bufferAudio?: boolean sessionId?: string + /** null/undefined = system default input device */ + microphoneDeviceId?: string | null + /** Cached label, used to re-resolve a device whose id was re-salted */ + microphoneDeviceLabel?: string | null + /** Fires when the live input device disappears mid-capture */ + onCaptureLost?: () => void +} + +export type StartAudioCaptureResult = { + fellBackToDefaultMicrophone: boolean } type StopAudioCaptureOptions = { @@ -32,8 +43,12 @@ export function useAudioCapture() { const bufferedAudioSecondsRef = useRef(0) const capturedChunkCountRef = useRef(0) const sessionIdRef = useRef('desktop') + const trackLostCleanupRef = useRef<(() => void) | null>(null) const cleanupCaptureResources = useCallback(() => { + trackLostCleanupRef.current?.() + trackLostCleanupRef.current = null + processorRef.current?.disconnect() sourceRef.current?.disconnect() processorRef.current = null @@ -84,7 +99,9 @@ export function useAudioCapture() { ) const start = useCallback( - async (options: StartAudioCaptureOptions = {}) => { + async ( + options: StartAudioCaptureOptions = {} + ): Promise => { if (isCapturingRef.current) { return } @@ -96,13 +113,13 @@ export function useAudioCapture() { resetBufferedAudio() capturedChunkCountRef.current = 0 - const stream = await navigator.mediaDevices.getUserMedia({ - audio: { - channelCount: 1, - echoCancellation: true, - noiseSuppression: true, - autoGainControl: true - } + const { stream, fellBackToDefaultMicrophone } = await openMicrophoneCaptureStream({ + preferredDeviceId: options.microphoneDeviceId, + preferredDeviceLabel: options.microphoneDeviceLabel, + getUserMedia: (constraints) => navigator.mediaDevices.getUserMedia(constraints), + enumerateDevices: navigator.mediaDevices?.enumerateDevices + ? () => navigator.mediaDevices.enumerateDevices() + : undefined }) if (startRequestRef.current !== startRequest) { stream.getTracks().forEach((track) => track.stop()) @@ -178,6 +195,24 @@ export function useAudioCapture() { processorRef.current = processor sourceRef.current = source isCapturingRef.current = true + + // Why: unplugging the input ends the track without ending the graph — the + // processor keeps feeding zeros, so dictation looks live while capturing nothing. + const onCaptureLost = options.onCaptureLost + const audioTrack = stream.getAudioTracks()[0] + if (onCaptureLost && audioTrack) { + const handleTrackEnded = (): void => { + if (startRequestRef.current !== startRequest || !isCapturingRef.current) { + return + } + onCaptureLost() + } + audioTrack.addEventListener('ended', handleTrackEnded) + trackLostCleanupRef.current = () => { + audioTrack.removeEventListener('ended', handleTrackEnded) + } + } + return { fellBackToDefaultMicrophone } } catch (err) { processor?.disconnect() source?.disconnect() diff --git a/src/renderer/src/i18n/locales/en.json b/src/renderer/src/i18n/locales/en.json index 7e276012c36..3f4429d0c70 100644 --- a/src/renderer/src/i18n/locales/en.json +++ b/src/renderer/src/i18n/locales/en.json @@ -9145,7 +9145,13 @@ "20574cbc72": "Enable Voice Dictation", "322d457a0d": "transcription", "dcc7846641": "Configure the OpenAI API key used for cloud speech-to-text models.", - "ebfd0b32e5": "OpenAI Transcription" + "ebfd0b32e5": "OpenAI Transcription", + "microphoneTitle": "Microphone", + "microphoneDescription": "Choose which input device voice dictation uses.", + "micInput": "input", + "micDevice": "device", + "micAirpods": "airpods", + "micDefault": "system default" } } }, @@ -10136,6 +10142,14 @@ "webUnavailable": "Switching builds is only available in the desktop app.", "devChannelMacOnlyAria": "{{value0}} (macOS only)", "devChannelMacOnly": "{{value0}} builds are produced only for macOS. Linux and Windows stay on Stable or RC." + }, + "VoiceMicrophoneSetting": { + "systemDefault": "System default", + "unavailable": "unavailable", + "label": "Microphone", + "description": "Input device used for voice dictation. System default follows the OS microphone setting.", + "accessHint": "Allow microphone access to list input devices.", + "allowAccess": "Allow access" } }, "right": { @@ -13328,7 +13342,9 @@ "55127a3706": "Dictation failed: {{value0}}", "bb7f599ee7": "Open Settings", "2d5b9fabf9": "Microphone access denied. Grant access in system settings, then restart Orca.", - "5d2c3e7ae3": "No speech detected." + "5d2c3e7ae3": "No speech detected.", + "micFallback": "Selected microphone unavailable. Using system default.", + "micDisconnected": "Microphone disconnected. Dictation stopped." }, "DictationIndicator": { "335e1bc6cb": "Stop dictation" diff --git a/src/shared/constants.ts b/src/shared/constants.ts index 279296834e4..45afce66118 100644 --- a/src/shared/constants.ts +++ b/src/shared/constants.ts @@ -402,7 +402,9 @@ export function getDefaultVoiceSettings(): VoiceSettings { dictationMode: 'toggle' as const, terminalConfirmBeforeInsert: false, userModels: [], - openAiApiKeyConfigured: false + openAiApiKeyConfigured: false, + microphoneDeviceId: null, + microphoneDeviceLabel: null } } diff --git a/src/shared/speech-types.ts b/src/shared/speech-types.ts index 93e1989dbfc..1cf42f1d6e1 100644 --- a/src/shared/speech-types.ts +++ b/src/shared/speech-types.ts @@ -75,4 +75,8 @@ export type VoiceSettings = { terminalConfirmBeforeInsert: boolean userModels: UserModelConfig[] openAiApiKeyConfigured: boolean + /** null = system default input device */ + microphoneDeviceId: string | null + /** Cached label for display when the preferred device is unplugged */ + microphoneDeviceLabel: string | null } diff --git a/tests/e2e/voice-microphone-selection.spec.ts b/tests/e2e/voice-microphone-selection.spec.ts new file mode 100644 index 00000000000..54f3edeb37a --- /dev/null +++ b/tests/e2e/voice-microphone-selection.spec.ts @@ -0,0 +1,177 @@ +import { test, expect } from './helpers/orca-app' +import { waitForSessionReady } from './helpers/store' + +type FakeMicrophoneDevice = { + deviceId: string + label: string +} + +type FakeMicrophoneState = { + devices: FakeMicrophoneDevice[] + dispatchDeviceChange: () => void +} + +async function installFakeMicrophoneDevices( + page: Parameters[0], + devices: FakeMicrophoneDevice[] +): Promise { + await page.addInitScript((initialDevices) => { + const listeners = new Set() + const state: FakeMicrophoneState = { + devices: initialDevices, + dispatchDeviceChange: () => { + for (const listener of listeners) { + listener(new Event('devicechange')) + } + } + } + const mediaDevices = { + enumerateDevices: async () => + state.devices.map((device) => ({ + ...device, + kind: 'audioinput' as const, + groupId: '' + })), + getUserMedia: async () => { + throw new DOMException('E2E microphone access is not used by this spec', 'NotAllowedError') + }, + addEventListener: (type: string, listener: EventListener) => { + if (type === 'devicechange') { + listeners.add(listener) + } + }, + removeEventListener: (type: string, listener: EventListener) => { + if (type === 'devicechange') { + listeners.delete(listener) + } + } + } + + Object.defineProperty(navigator, 'mediaDevices', { + configurable: true, + value: mediaDevices + }) + ;( + window as Window & { __orcaE2EFakeMicrophone?: FakeMicrophoneState } + ).__orcaE2EFakeMicrophone = state + }, devices) +} + +async function prepareVoiceSettings( + page: Parameters[0], + microphoneDeviceId: string | null, + microphoneDeviceLabel: string | null +): Promise { + await page.evaluate( + async ({ microphoneDeviceId, microphoneDeviceLabel }) => { + const store = window.__store + const settings = await window.api.settings.get() + if (!store || !settings.voice) { + throw new Error('Voice settings are not available') + } + await store.getState().updateSettings({ + uiLanguage: 'en', + voice: { + ...settings.voice, + enabled: true, + microphoneDeviceId, + microphoneDeviceLabel + } + }) + store.getState().openSettingsTarget({ pane: 'voice', repoId: null }) + store.getState().openSettingsPage() + }, + { microphoneDeviceId, microphoneDeviceLabel } + ) + await expect(page.getByPlaceholder('Search settings')).toBeVisible() + const featureTipDialog = page.getByRole('dialog', { name: 'Voice Dictation is here' }) + if (await featureTipDialog.isVisible().catch(() => false)) { + await page.getByRole('button', { name: 'Maybe Later' }).click() + } + await expect(page.getByRole('heading', { name: 'Voice', exact: true })).toBeVisible() +} + +async function readMicrophoneSettings( + page: Parameters[0] +): Promise<{ deviceId: string | null; label: string | null }> { + return page.evaluate(async () => { + const voice = (await window.api.settings.get()).voice + return { + deviceId: voice?.microphoneDeviceId ?? null, + label: voice?.microphoneDeviceLabel ?? null + } + }) +} + +test.describe('Voice microphone selection', () => { + test('lists devices, persists a selected microphone, and restores it', async ({ orcaPage }) => { + await waitForSessionReady(orcaPage) + await installFakeMicrophoneDevices(orcaPage, [ + { deviceId: 'built-in', label: 'Built-in Microphone' }, + { deviceId: 'usb-mic', label: 'USB Microphone' } + ]) + await orcaPage.reload({ waitUntil: 'domcontentloaded' }) + await waitForSessionReady(orcaPage) + await prepareVoiceSettings(orcaPage, null, null) + + const microphone = orcaPage.getByRole('combobox', { name: 'Microphone' }) + await expect(microphone).toHaveText('System default') + await microphone.click() + await expect(orcaPage.getByRole('option', { name: 'USB Microphone' })).toBeVisible() + await orcaPage.getByRole('option', { name: 'USB Microphone' }).click() + + await expect + .poll(() => readMicrophoneSettings(orcaPage), { + message: 'selected microphone did not persist' + }) + .toEqual({ deviceId: 'usb-mic', label: 'USB Microphone' }) + await expect(microphone).toHaveText('USB Microphone') + + await orcaPage.reload({ waitUntil: 'domcontentloaded' }) + await waitForSessionReady(orcaPage) + await prepareVoiceSettings(orcaPage, 'usb-mic', 'USB Microphone') + await expect(orcaPage.getByRole('combobox', { name: 'Microphone' })).toHaveText( + 'USB Microphone' + ) + }) + + test('marks an unplugged device unavailable and follows a relabeled device', async ({ + orcaPage + }) => { + await waitForSessionReady(orcaPage) + await installFakeMicrophoneDevices(orcaPage, [ + { deviceId: 'built-in', label: 'Built-in Microphone' } + ]) + await orcaPage.reload({ waitUntil: 'domcontentloaded' }) + await waitForSessionReady(orcaPage) + await prepareVoiceSettings(orcaPage, 'stale-airpods-id', 'AirPods') + + const microphone = orcaPage.getByRole('combobox', { name: 'Microphone' }) + await microphone.click() + await expect(orcaPage.getByRole('option', { name: 'AirPods (unavailable)' })).toBeVisible() + await orcaPage.keyboard.press('Escape') + + await orcaPage.evaluate(() => { + const state = (window as Window & { __orcaE2EFakeMicrophone?: FakeMicrophoneState }) + .__orcaE2EFakeMicrophone + if (!state) { + throw new Error('Fake microphone state is not available') + } + state.devices = [ + { deviceId: 'built-in', label: 'Built-in Microphone' }, + { deviceId: 'fresh-airpods-id', label: 'AirPods' } + ] + state.dispatchDeviceChange() + }) + + await expect(microphone).toHaveText('AirPods') + await microphone.click() + await expect(orcaPage.getByRole('option', { name: 'AirPods' })).toBeVisible() + await expect(orcaPage.getByRole('option', { name: 'AirPods (unavailable)' })).toHaveCount(0) + await orcaPage.keyboard.press('Escape') + await expect(readMicrophoneSettings(orcaPage)).resolves.toEqual({ + deviceId: 'stale-airpods-id', + label: 'AirPods' + }) + }) +}) From 040734b18e8ef05e088d6003fd2d39db75f141dd Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sun, 2 Aug 2026 16:56:08 -0700 Subject: [PATCH 010/106] fix(sidebar): show collapse chevron on pinned section headers (#12147) --- .../WorktreeList.lineage-child-card.test.ts | 24 +++++++++++++++++++ .../src/components/sidebar/WorktreeList.tsx | 7 ++++-- 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/src/renderer/src/components/sidebar/WorktreeList.lineage-child-card.test.ts b/src/renderer/src/components/sidebar/WorktreeList.lineage-child-card.test.ts index 7de4eb12f95..53d16e0d7b4 100644 --- a/src/renderer/src/components/sidebar/WorktreeList.lineage-child-card.test.ts +++ b/src/renderer/src/components/sidebar/WorktreeList.lineage-child-card.test.ts @@ -917,6 +917,30 @@ describe('WorktreeList lineage child card renderer', () => { expect(markup).toContain('aria-expanded="true"') }) + it('renders a collapse chevron on the pinned section header with worktrees', async () => { + setPinnedFixtureState() + const markup = await renderWorktreeListMarkup() + + expect(markup).toContain('Pinned') + expect(markup).toContain('data-workspace-pin-drop-target=""') + expect(markup).toContain('data-repo-header-collapse-affordance=""') + expect(markup).toContain('aria-expanded="true"') + }) + + it('renders collapsed pinned section header affordance state', async () => { + setPinnedFixtureState() + mockStore.state = { + ...mockStore.state, + collapsedGroups: new Set(['pinned']) + } + const markup = await renderWorktreeListMarkup() + + expect(markup).toContain('data-workspace-pin-drop-target=""') + expect(markup).toContain('data-repo-header-collapse-affordance=""') + expect(markup).toContain('aria-expanded="false"') + expect(markup).toContain('-rotate-90') + }) + it('renders a collapse chevron on grouped repo headers with worktrees', async () => { setLineageFixtureState('repo') const markup = await renderWorktreeListMarkup() diff --git a/src/renderer/src/components/sidebar/WorktreeList.tsx b/src/renderer/src/components/sidebar/WorktreeList.tsx index 8cc18bb43f3..e21ccdd01e9 100644 --- a/src/renderer/src/components/sidebar/WorktreeList.tsx +++ b/src/renderer/src/components/sidebar/WorktreeList.tsx @@ -4196,10 +4196,13 @@ const VirtualizedWorktreeViewport = React.memo(function VirtualizedWorktreeViewp projectGroupPathStatus.reason === 'ambiguous-connection') const projectGroupDepth = row.projectGroupDepth ?? 0 const isHeaderCollapsed = collapsedGroups.has(row.key) - // Why: repo/project and status headers share compact section chrome; flat "All" stays a simple label. + // Why: repo/project/status/pinned share compact section chrome; flat "All" stays a simple label. const showHeaderCollapseAffordance = row.count > 0 && - (isRepoHeader || isProjectGroupHeader || headerWorkspaceStatus !== null) + (isRepoHeader || + isProjectGroupHeader || + headerWorkspaceStatus !== null || + isPinnedHeader) // Why: non-project headers like "All" are flat-list labels; don't reserve project hierarchy indent. const headerPaddingLeft = isRepoHeader || isProjectGroupHeader From ccb5850390536d608a10db58351cdb3b9152c552 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Sun, 2 Aug 2026 17:01:49 -0700 Subject: [PATCH 011/106] fix(cmd-j): center palette row icons on the first text line (#12144) The leading-icon gutter used self-start with a hand-tuned pt-0.5 nudge, leaving status dots ~2px and lucide icons ~1px above the 20px title line box. Give the gutter h-5 to match the line box so icons center on the first line for single- and two-line rows alike. --- src/renderer/src/components/WorktreeJumpPalette.tsx | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/renderer/src/components/WorktreeJumpPalette.tsx b/src/renderer/src/components/WorktreeJumpPalette.tsx index 9eee43a6a4b..c85a2d700e7 100644 --- a/src/renderer/src/components/WorktreeJumpPalette.tsx +++ b/src/renderer/src/components/WorktreeJumpPalette.tsx @@ -1838,7 +1838,7 @@ export default function WorktreeJumpPalette(): React.JSX.Element | null { 'data-[selected=true]:border-border data-[selected=true]:bg-accent data-[selected=true]:text-foreground' )} > -
+
@@ -1966,7 +1966,7 @@ export default function WorktreeJumpPalette(): React.JSX.Element | null { 'data-[selected=true]:border-border data-[selected=true]:bg-accent data-[selected=true]:text-foreground' )} > -
+
@@ -2013,7 +2013,7 @@ export default function WorktreeJumpPalette(): React.JSX.Element | null { 'data-[selected=true]:border-border data-[selected=true]:bg-accent data-[selected=true]:text-foreground' )} > -
+
@@ -2054,7 +2054,7 @@ export default function WorktreeJumpPalette(): React.JSX.Element | null { 'data-[selected=true]:border-border data-[selected=true]:bg-accent data-[selected=true]:text-foreground' )} > -
+
@@ -2135,7 +2135,7 @@ export default function WorktreeJumpPalette(): React.JSX.Element | null { 'data-[selected=true]:border-border data-[selected=true]:bg-accent data-[selected=true]:text-foreground' )} > -
+
@@ -2213,7 +2213,7 @@ export default function WorktreeJumpPalette(): React.JSX.Element | null { 'data-[selected=true]:border-border data-[selected=true]:bg-accent data-[selected=true]:text-foreground' )} > -
+
From 1685b96a9752108b0899678f8381852dc7529953 Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Sun, 2 Aug 2026 17:06:11 -0700 Subject: [PATCH 012/106] fix(workspace-cleanup): read reflog timestamps to avoid git maintenance (#12131) * fix(workspace-cleanup): read reflog timestamps to avoid git maintenance Workspace activity detection now reads the reflog to find the newest HEAD movement, avoiding false activity signals from `git gc` and `git status` restamping logs/HEAD. Extraction of git mtime probes to exclude files that maintenance rewrites (gitdir, index, logs/HEAD), and instead read commit markers (COMMIT_EDITMSG, ORIG_HEAD) and reflog entry timestamps. Expands the scan with a renderer-side activity estimate to reconcile against the Resource Manager button's fast count. Adds deletion phase tracking (queued vs deleting) and a mismatch notice when the two counts diverge. * fix(workspace-cleanup): parse reflog timestamps with fewer digits and im - Regex now accepts 1-11 digit timestamps (was 9-11); trailing timezone anchor makes digit-count floor unnecessary - Add `removalInFlight` state to prevent duplicate removal batches; UI checks this flag alongside `removalProgress` - Filter scan errors by selected repos; only show estimate-mismatch notice when scan is complete and error-free - Mark candidate rows as non-selectable while deleting, even if `removing` flag is omitted --- .../ipc/workspace-cleanup-activity.test.ts | 100 +++++++++++++--- src/main/ipc/workspace-cleanup-activity.ts | 53 ++++++--- src/main/ipc/workspace-cleanup-scan.ts | 2 +- .../status-bar/ResourceUsageStatusSegment.tsx | 21 +--- .../WorkspaceCleanupDialog.tsx | 110 ++++++++++++++---- .../inactive-workspace-estimate.test.ts | 69 +++++++++++ .../inactive-workspace-estimate.ts | 37 ++++++ .../workspace-cleanup-candidate-row.test.tsx | 36 ++++++ .../workspace-cleanup-candidate-row.tsx | 29 ++++- src/renderer/src/i18n/locales/en.json | 14 ++- src/renderer/src/i18n/locales/zh.json | 14 ++- .../claude-background-task-status.test.ts | 22 ++++ src/shared/workspace-cleanup.ts | 10 ++ 13 files changed, 445 insertions(+), 72 deletions(-) create mode 100644 src/renderer/src/components/workspace-cleanup/inactive-workspace-estimate.test.ts create mode 100644 src/renderer/src/components/workspace-cleanup/inactive-workspace-estimate.ts diff --git a/src/main/ipc/workspace-cleanup-activity.test.ts b/src/main/ipc/workspace-cleanup-activity.test.ts index 72f20291f4a..bfb1c080db3 100644 --- a/src/main/ipc/workspace-cleanup-activity.test.ts +++ b/src/main/ipc/workspace-cleanup-activity.test.ts @@ -55,17 +55,18 @@ describe('resolveWorkspaceCleanupActivityWorktree', () => { expect(worktree.lastActivityAt).toBe(20_000) }) - it('uses linked worktree gitdir metadata when the .git pointer is stale', async () => { + it('uses linked worktree commit metadata when the .git pointer is stale', async () => { const gitDirPath = path.join('/repo', '.git', 'worktrees', 'repo-feature') const gitDirHeadPath = path.join(gitDirPath, 'HEAD') - const gitDirLogsHeadPath = path.join(gitDirPath, 'logs', 'HEAD') + const commitEditMsgPath = path.join(gitDirPath, 'COMMIT_EDITMSG') + const origHeadPath = path.join(gitDirPath, 'ORIG_HEAD') const statPath = vi.fn(async (targetPath: string) => { const mtimes: Record = { '/repo-feature': 10_000, [path.join('/repo-feature', '.git')]: 20_000, - [gitDirPath]: 50_000, [gitDirHeadPath]: 60_000, - [gitDirLogsHeadPath]: 70_000 + [origHeadPath]: 65_000, + [commitEditMsgPath]: 70_000 } return { mtimeMs: mtimes[targetPath] ?? 0 } }) @@ -79,17 +80,86 @@ describe('resolveWorkspaceCleanupActivityWorktree', () => { ) expect(readTextFile).toHaveBeenCalledWith(path.join('/repo-feature', '.git')) - expect(statPath).toHaveBeenCalledWith(gitDirPath) expect(statPath).toHaveBeenCalledWith(gitDirHeadPath) - expect(statPath).toHaveBeenCalledWith(gitDirLogsHeadPath) + expect(statPath).toHaveBeenCalledWith(commitEditMsgPath) + expect(statPath).toHaveBeenCalledWith(origHeadPath) expect(worktree.lastActivityAt).toBe(70_000) }) + it('ignores gitdir metadata that git maintenance and git status restamp', async () => { + const gitDirPath = path.join('/repo', '.git', 'worktrees', 'repo-feature') + const statPath = vi.fn(async (targetPath: string) => { + const mtimes: Record = { + '/repo-feature': 10_000, + [path.join('/repo-feature', '.git')]: 10_000, + [gitDirPath]: 90_000, + [path.join(gitDirPath, 'index')]: 90_000, + [path.join(gitDirPath, 'logs', 'HEAD')]: 90_000 + } + return { mtimeMs: mtimes[targetPath] ?? 0 } + }) + const readTextFile = vi.fn(async () => `gitdir: ${gitDirPath}\n`) + + const worktree = await resolveWorkspaceCleanupActivityWorktree( + REPO, + makeWorktree(), + statPath, + readTextFile + ) + + expect(statPath).not.toHaveBeenCalledWith(gitDirPath) + expect(statPath).not.toHaveBeenCalledWith(path.join(gitDirPath, 'index')) + expect(statPath).not.toHaveBeenCalledWith(path.join(gitDirPath, 'logs', 'HEAD')) + expect(worktree.lastActivityAt).toBe(10_000) + }) + + it('reads the newest reflog entry timestamp instead of the reflog file mtime', async () => { + const gitDirPath = path.join('/repo', '.git', 'worktrees', 'repo-feature') + const reflogPath = path.join(gitDirPath, 'logs', 'HEAD') + const statPath = vi.fn(async () => ({ mtimeMs: 10_000 })) + const readTextFile = vi.fn(async (targetPath: string) => + targetPath === reflogPath + ? [ + '0000 1111 Dev 1700000000 -0700\tbranch: Created from HEAD', + '1111 2222 Dev 1700000900 -0700\tcommit: work', + '' + ].join('\n') + : `gitdir: ${gitDirPath}\n` + ) + + const worktree = await resolveWorkspaceCleanupActivityWorktree( + REPO, + makeWorktree(), + statPath, + readTextFile + ) + + expect(readTextFile).toHaveBeenCalledWith(reflogPath) + expect(worktree.lastActivityAt).toBe(1_700_000_900_000) + }) + + it('degrades to other probes when the reflog was expired to an empty file', async () => { + const gitDirPath = path.join('/repo', '.git', 'worktrees', 'repo-feature') + const statPath = vi.fn(async () => ({ mtimeMs: 10_000 })) + const readTextFile = vi.fn(async (targetPath: string) => + targetPath === path.join(gitDirPath, 'logs', 'HEAD') ? '' : `gitdir: ${gitDirPath}\n` + ) + + const worktree = await resolveWorkspaceCleanupActivityWorktree( + REPO, + makeWorktree(), + statPath, + readTextFile + ) + + expect(worktree.lastActivityAt).toBe(10_000) + }) + it('resolves relative linked worktree gitdir pointers from the worktree path', async () => { const gitDirPath = path.resolve('/repo-feature', '.repo/gitdir') - const gitDirLogsHeadPath = path.join(gitDirPath, 'logs', 'HEAD') + const commitEditMsgPath = path.join(gitDirPath, 'COMMIT_EDITMSG') const statPath = vi.fn(async (targetPath: string) => ({ - mtimeMs: targetPath === gitDirLogsHeadPath ? 40_000 : 10_000 + mtimeMs: targetPath === commitEditMsgPath ? 40_000 : 10_000 })) const readTextFile = vi.fn(async () => 'gitdir: .repo/gitdir\n') @@ -100,7 +170,7 @@ describe('resolveWorkspaceCleanupActivityWorktree', () => { readTextFile ) - expect(statPath).toHaveBeenCalledWith(gitDirLogsHeadPath) + expect(statPath).toHaveBeenCalledWith(commitEditMsgPath) expect(worktree.lastActivityAt).toBe(40_000) }) @@ -108,14 +178,13 @@ describe('resolveWorkspaceCleanupActivityWorktree', () => { const worktreePath = String.raw`\\wsl.localhost\Ubuntu\home\me\repo-feature` const gitDirPath = String.raw`\\wsl.localhost\Ubuntu\home\me\repo\.git\worktrees\repo-feature` const gitDirHeadPath = path.join(gitDirPath, 'HEAD') - const gitDirLogsHeadPath = path.join(gitDirPath, 'logs', 'HEAD') + const commitEditMsgPath = path.join(gitDirPath, 'COMMIT_EDITMSG') const statPath = vi.fn(async (targetPath: string) => { const mtimes: Record = { [worktreePath]: 10_000, [path.join(worktreePath, '.git')]: 20_000, - [gitDirPath]: 50_000, [gitDirHeadPath]: 60_000, - [gitDirLogsHeadPath]: 70_000 + [commitEditMsgPath]: 70_000 } return { mtimeMs: mtimes[targetPath] ?? 0 } }) @@ -129,10 +198,11 @@ describe('resolveWorkspaceCleanupActivityWorktree', () => { ) expect(readTextFile).toHaveBeenCalledWith(path.join(worktreePath, '.git')) - expect(statPath).toHaveBeenCalledWith(gitDirPath) expect(statPath).toHaveBeenCalledWith(gitDirHeadPath) - expect(statPath).toHaveBeenCalledWith(gitDirLogsHeadPath) - expect(statPath).not.toHaveBeenCalledWith('/home/me/repo/.git/worktrees/repo-feature') + expect(statPath).toHaveBeenCalledWith(commitEditMsgPath) + expect(statPath).not.toHaveBeenCalledWith( + path.join('/home/me/repo/.git/worktrees/repo-feature', 'HEAD') + ) expect(worktree.lastActivityAt).toBe(70_000) }) diff --git a/src/main/ipc/workspace-cleanup-activity.ts b/src/main/ipc/workspace-cleanup-activity.ts index 9026b4464d9..56ac36f6b29 100644 --- a/src/main/ipc/workspace-cleanup-activity.ts +++ b/src/main/ipc/workspace-cleanup-activity.ts @@ -1,20 +1,13 @@ import { lstat, readFile } from 'node:fs/promises' import path from 'node:path' import type { Repo, Worktree } from '../../shared/types' +import { getPersistedWorkspaceCleanupActivityAt } from '../../shared/workspace-cleanup' import { parseWslUncPath } from '../../shared/wsl-paths' import { toWindowsWslPath } from '../wsl' type StatPath = (targetPath: string) => Promise<{ mtimeMs: number }> type ReadTextFile = (targetPath: string) => Promise -export function getPersistedWorkspaceCleanupActivityAt( - worktree: Pick -): number { - const persistedActivityAt = Number.isFinite(worktree.lastActivityAt) ? worktree.lastActivityAt : 0 - const createdAt = Number.isFinite(worktree.createdAt) ? (worktree.createdAt ?? 0) : 0 - return Math.max(persistedActivityAt, createdAt) -} - export function resolvePersistedWorkspaceCleanupActivityWorktree(worktree: Worktree): Worktree { const persistedActivityAt = getPersistedWorkspaceCleanupActivityAt(worktree) if (persistedActivityAt <= worktree.lastActivityAt) { @@ -56,7 +49,7 @@ async function resolveWorkspaceCleanupActivityAt( return persistedActivityAt } - const filesystemActivityAt = await getNewestLocalWorktreeStatMtime( + const filesystemActivityAt = await getNewestLocalWorktreeActivityAt( worktree.path, statPath, readTextFile @@ -67,26 +60,54 @@ async function resolveWorkspaceCleanupActivityAt( // Why: best-effort only. Win32 stat over \\wsl.localhost (9P) can falsely // report ENOENT (see wslUncDirectoryExists), so a failed stat degrades to the // persisted activity timestamp instead of blocking or mislabeling the row. -async function getNewestLocalWorktreeStatMtime( +async function getNewestLocalWorktreeActivityAt( worktreePath: string, statPath: StatPath, readTextFile: ReadTextFile ): Promise { const gitPath = path.join(worktreePath, '.git') const gitDirPath = await readLocalWorktreeGitDir(worktreePath, gitPath, readTextFile) - const gitDirMtimePromises = gitDirPath + // Why: every entry here must move only on a user action. Excluded on purpose: + // the gitdir directory and logs/HEAD (restamped for every linked worktree at + // once by `git gc` / `git reflog expire`, which made one maintenance run look + // like activity everywhere), and index (rewritten by `git status` — this scan + // runs one per candidate, so it would erase the evidence it just read). + const gitDirProbes = gitDirPath ? [ - readMtime(gitDirPath, statPath), readMtime(path.join(gitDirPath, 'HEAD'), statPath), - readMtime(path.join(gitDirPath, 'logs', 'HEAD'), statPath) + readMtime(path.join(gitDirPath, 'COMMIT_EDITMSG'), statPath), + readMtime(path.join(gitDirPath, 'ORIG_HEAD'), statPath), + readNewestReflogEntryAt(path.join(gitDirPath, 'logs', 'HEAD'), readTextFile) ] : [] - const mtimes = await Promise.all([ + const timestamps = await Promise.all([ readMtime(worktreePath, statPath), readMtime(gitPath, statPath), - ...gitDirMtimePromises + ...gitDirProbes ]) - return Math.max(0, ...mtimes) + return Math.max(0, ...timestamps) +} + +// Why: the reflog records when HEAD actually moved, so it survives the mtime +// churn that makes logs/HEAD itself unreadable as a signal. Expired reflogs are +// truncated to an empty file, which degrades to the other probes. +async function readNewestReflogEntryAt( + reflogPath: string, + readTextFile: ReadTextFile +): Promise { + try { + const lines = (await readTextFile(reflogPath)).split('\n') + for (let index = lines.length - 1; index >= 0; index -= 1) { + // The trailing timezone + tab anchors the capture, so no digit-count floor is needed. + const seconds = /\s(\d{1,11})\s[-+]\d{4}\t/.exec(lines[index] ?? '')?.[1] + if (seconds) { + return Number(seconds) * 1000 + } + } + return 0 + } catch { + return 0 + } } async function readLocalWorktreeGitDir( diff --git a/src/main/ipc/workspace-cleanup-scan.ts b/src/main/ipc/workspace-cleanup-scan.ts index f5deb16a2bd..32794f61977 100644 --- a/src/main/ipc/workspace-cleanup-scan.ts +++ b/src/main/ipc/workspace-cleanup-scan.ts @@ -13,8 +13,8 @@ import type { WorkspaceCleanupScanProgress, WorkspaceCleanupScanResult } from '../../shared/workspace-cleanup' +import { getPersistedWorkspaceCleanupActivityAt } from '../../shared/workspace-cleanup' import { - getPersistedWorkspaceCleanupActivityAt, resolvePersistedWorkspaceCleanupActivityWorktree, resolveWorkspaceCleanupActivityWorktree } from './workspace-cleanup-activity' diff --git a/src/renderer/src/components/status-bar/ResourceUsageStatusSegment.tsx b/src/renderer/src/components/status-bar/ResourceUsageStatusSegment.tsx index de06fcac580..a8ece07593d 100644 --- a/src/renderer/src/components/status-bar/ResourceUsageStatusSegment.tsx +++ b/src/renderer/src/components/status-bar/ResourceUsageStatusSegment.tsx @@ -34,8 +34,7 @@ import { useDaemonActions, DaemonActionDialog } from '../shared/useDaemonActions import type { AppMemory, BrowserWorkspace, UsageValues, Worktree } from '../../../../shared/types' import { ORPHAN_WORKTREE_ID } from '../../../../shared/constants' import { getRepoExecutionHostId, parseExecutionHostId } from '../../../../shared/execution-host' -import { isFolderRepo } from '../../../../shared/repo-kind' -import { isWorkspaceOldForCleanup } from '../../../../shared/workspace-cleanup' +import { countEstimatedInactiveWorkspaces } from '../workspace-cleanup/inactive-workspace-estimate' import { mergeSnapshotAndSessions, UNATTRIBUTED_REPO_ID } from './mergeSnapshotAndSessions' import type { Metric, @@ -917,20 +916,10 @@ export function ResourceUsageStatusSegment({ [allWorktrees] ) - const oldWorkspaceCount = useMemo(() => { - const now = Date.now() - let count = 0 - for (const worktree of allWorktrees) { - const repo = repoById.get(worktree.repoId) - if (!repo || isFolderRepo(repo) || worktree.isMainWorktree) { - continue - } - if (isWorkspaceOldForCleanup(worktree, now)) { - count += 1 - } - } - return count - }, [allWorktrees, repoById]) + const oldWorkspaceCount = useMemo( + () => countEstimatedInactiveWorkspaces(allWorktrees, repoById, Date.now()), + [allWorktrees, repoById] + ) // Why: skip the merge when closed; the always-mounted segment recomputing on every keystroke-driven store mutation made the app laggy. const unifiedRepos = useMemo( diff --git a/src/renderer/src/components/workspace-cleanup/WorkspaceCleanupDialog.tsx b/src/renderer/src/components/workspace-cleanup/WorkspaceCleanupDialog.tsx index 1bfe02aebbf..0c870ee4485 100644 --- a/src/renderer/src/components/workspace-cleanup/WorkspaceCleanupDialog.tsx +++ b/src/renderer/src/components/workspace-cleanup/WorkspaceCleanupDialog.tsx @@ -4,6 +4,7 @@ import React, { useCallback, useEffect, useMemo, useRef, useState } from 'react' import { AlertTriangle, + Info, Loader2, RefreshCcw, Search, @@ -68,7 +69,8 @@ import { startWorkspaceCleanupBackgroundRemoval, type WorkspaceCleanupRemovalProgress } from './workspace-cleanup-background-removal' -import { CandidateRow } from './workspace-cleanup-candidate-row' +import { countEstimatedInactiveWorkspaces } from './inactive-workspace-estimate' +import { CandidateRow, type WorkspaceCleanupDeletionPhase } from './workspace-cleanup-candidate-row' import { WorkspaceCleanupCandidateList } from './workspace-cleanup-candidate-list' import { getCandidateStatus, @@ -198,15 +200,20 @@ export default function WorkspaceCleanupDialog(): React.JSX.Element { const removeCandidates = useAppStore((s) => s.removeWorkspaceCleanupCandidates) const markWorktreesQueuedForDeletion = useAppStore((s) => s.markWorktreesQueuedForDeletion) const clearWorktreeDeleteState = useAppStore((s) => s.clearWorktreeDeleteState) - const deletingWorktreeIds = useAppStore( - useShallow( - (s) => - new Set( - Object.entries(s.deleteStateByWorktreeId) - .filter(([, state]) => state.isDeleting) - .map(([worktreeId]) => worktreeId) - ) - ) + const deletionPhaseByWorktreeId = useAppStore( + useShallow((s) => { + const phases: Record = {} + for (const [worktreeId, state] of Object.entries(s.deleteStateByWorktreeId)) { + if (state.isDeleting) { + phases[worktreeId] = state.phase ?? 'deleting' + } + } + return phases + }) + ) + const deletingWorktreeIds = useMemo( + () => new Set(Object.keys(deletionPhaseByWorktreeId)), + [deletionPhaseByWorktreeId] ) const open = activeModal === 'workspace-cleanup' @@ -220,6 +227,9 @@ export default function WorkspaceCleanupDialog(): React.JSX.Element { const [removalProgress, setRemovalProgress] = useState( null ) + // Why: `removalProgress` only arrives once the batch reports, so rendering + // needs its own in-flight flag; removalInFlightRef stays the synchronous guard. + const [removalInFlight, setRemovalInFlight] = useState(false) const [rowFailures, setRowFailures] = useState>({}) const [repoSelection, setRepoSelection] = useState>(() => new Set()) const [filters, setFilters] = useState(DEFAULT_FILTERS) @@ -342,6 +352,10 @@ export default function WorkspaceCleanupDialog(): React.JSX.Element { } return new Set(eligibleRepoIds) }, [eligibleRepoIds, repoSelection]) + const selectedScanErrors = useMemo( + () => (scan?.errors ?? []).filter((error) => effectiveRepoSelection.has(error.repoId)), + [effectiveRepoSelection, scan?.errors] + ) const filteredCandidates = useMemo(() => { if ( effectiveRepoSelection.size === 0 || @@ -351,6 +365,35 @@ export default function WorkspaceCleanupDialog(): React.JSX.Element { } return candidates.filter((candidate) => effectiveRepoSelection.has(candidate.repoId)) }, [candidates, effectiveRepoSelection, eligibleRepoIds.length]) + // Why: the Resource Manager button counts from the renderer's activity record only, + // so it routinely disagrees with the scanned list. Reconcile it rather than leaving + // the user to wonder which number is real. + const estimatedInactiveCount = useMemo(() => { + if (!open) { + return null + } + return countEstimatedInactiveWorkspaces( + Object.values(reviewStateInputs.worktreesByRepo).flat(), + new Map(reviewStateInputs.repos.map((repo) => [repo.id, repo])), + Date.now() + ) + }, [open, reviewStateInputs]) + // Why: the reconciliation only holds for a complete scan; a failed or partial + // one would blame the difference on git-history checks instead of the error. + const estimateMismatchNotice = + !loading && + !error && + selectedScanErrors.length === 0 && + scan && + estimatedInactiveCount !== null && + estimatedInactiveCount !== candidates.length && + filteredCandidates.length === candidates.length + ? translate( + 'auto.components.workspace.cleanup.WorkspaceCleanupDialog.f637f63882', + "Resource Manager counts {{value0}}; this list found {{value1}}. That counter reads Orca's activity record alone, while this scan also checks each workspace's git history and skips disconnected remotes.", + { value0: estimatedInactiveCount, value1: candidates.length } + ) + : null useEffect(() => { if (loading || !scan || selectedDefaultsScanAtRef.current === scan.scannedAt) { @@ -407,10 +450,6 @@ export default function WorkspaceCleanupDialog(): React.JSX.Element { () => new Map(repos.map((repo) => [repo.id, repo.displayName || repo.path])), [repos] ) - const selectedScanErrors = useMemo( - () => (scan?.errors ?? []).filter((error) => effectiveRepoSelection.has(error.repoId)), - [effectiveRepoSelection, scan?.errors] - ) const scanNoticeMessage = useMemo( () => formatScanNoticeMessage(selectedScanErrors, repoNameById), [repoNameById, selectedScanErrors] @@ -530,7 +569,7 @@ export default function WorkspaceCleanupDialog(): React.JSX.Element { // instances from re-rendering on scan stream-in and selection changes. const handleRemoveRow = useCallback( (candidate: WorkspaceCleanupCandidate) => { - if (loading) { + if (loading || removalInFlightRef.current) { return } setSelectedIds(new Set([candidate.worktreeId])) @@ -557,6 +596,15 @@ export default function WorkspaceCleanupDialog(): React.JSX.Element { setConfirmCandidates([]) }, [closeModal, removalProgress]) + // Why: the header X reads as "leave this screen", not "abandon the dialog". The batch + // keeps running in the background either way, and the list shows each row's progress. + // Diverges from cancelConfirmRemove, which closes the dialog mid-batch: here + // removalProgress stays set until the batch settles, so re-entry is still blocked. + const backToWorkspaceCleanupList = useCallback(() => { + setConfirming(false) + setConfirmCandidates([]) + }, []) + const clearQueuedDeleteState = useCallback( (worktreeId: string) => { const deleteState = useAppStore.getState().deleteStateByWorktreeId[worktreeId] @@ -597,6 +645,7 @@ export default function WorkspaceCleanupDialog(): React.JSX.Element { return } removalInFlightRef.current = true + setRemovalInFlight(true) removalBatchIdRef.current += 1 const removalBatchId = removalBatchIdRef.current // Why: a hung late settlement retains these callbacks for the renderer's @@ -625,6 +674,7 @@ export default function WorkspaceCleanupDialog(): React.JSX.Element { setRowFailures(nextFailures) deselectRemovedIds(result.removedIds) setRemovalProgress(null) + setRemovalInFlight(false) setConfirming(false) setConfirmCandidates([]) } @@ -657,6 +707,7 @@ export default function WorkspaceCleanupDialog(): React.JSX.Element { } if (mountedRef.current) { setRemovalProgress(null) + setRemovalInFlight(false) setConfirming(false) setConfirmCandidates([]) } @@ -779,7 +830,11 @@ export default function WorkspaceCleanupDialog(): React.JSX.Element { variant="destructive" size="sm" onClick={() => openConfirmRemove(selectedCandidates)} - disabled={selectedCount === 0 || loading} + // Why: leaving the progress view no longer closes the dialog, so the + // list is reachable mid-batch; a second batch would silently no-op. + disabled={ + selectedCount === 0 || loading || removalProgress !== null || removalInFlight + } > {translate( @@ -819,6 +874,13 @@ export default function WorkspaceCleanupDialog(): React.JSX.Element {
) : null} + {estimateMismatchNotice ? ( +
+ + {estimateMismatchNotice} +
+ ) : null} +
1 && index === activeRows.length - 1} expanded={expandedRowIds.has(candidate.worktreeId)} lastActivityLabel={formatRelativeTime(candidate.lastActivityAt)} - removing={loading || deletingWorktreeIds.has(candidate.worktreeId)} + deletionPhase={deletionPhaseByWorktreeId[candidate.worktreeId]} + removing={ + loading || + removalInFlight || + deletingWorktreeIds.has(candidate.worktreeId) + } selected={ selectedIds.has(candidate.worktreeId) && !loading && @@ -952,6 +1019,7 @@ export default function WorkspaceCleanupDialog(): React.JSX.Element { candidates={confirmCandidates} reviewInfoByWorktreeId={reviewInfoByWorktreeId} progress={removalProgress} + onBack={backToWorkspaceCleanupList} onCancel={cancelConfirmRemove} onConfirm={confirmRemove} /> @@ -1275,12 +1343,14 @@ function ConfirmRemove({ candidates, reviewInfoByWorktreeId, progress, + onBack, onCancel, onConfirm }: { candidates: WorkspaceCleanupCandidate[] reviewInfoByWorktreeId: ReadonlyMap progress: WorkspaceCleanupRemovalProgress | null + onBack: () => void onCancel: () => void onConfirm: () => void }): React.JSX.Element { @@ -1332,10 +1402,10 @@ function ConfirmRemove({ variant="ghost" size="icon-sm" aria-label={translate( - 'auto.components.workspace.cleanup.WorkspaceCleanupDialog.191f0bc98e', - 'Close' + 'auto.components.workspace.cleanup.WorkspaceCleanupDialog.74f6c16279', + 'Back' )} - onClick={onCancel} + onClick={onBack} > diff --git a/src/renderer/src/components/workspace-cleanup/inactive-workspace-estimate.test.ts b/src/renderer/src/components/workspace-cleanup/inactive-workspace-estimate.test.ts new file mode 100644 index 00000000000..e2546210158 --- /dev/null +++ b/src/renderer/src/components/workspace-cleanup/inactive-workspace-estimate.test.ts @@ -0,0 +1,69 @@ +import { describe, expect, it } from 'vitest' +import type { Repo, Worktree } from '../../../../shared/types' +import { countEstimatedInactiveWorkspaces } from './inactive-workspace-estimate' + +const NOW = 1_800_000_000_000 +const DAY_MS = 24 * 60 * 60 * 1000 + +const GIT_REPO = { id: 'repo-1', path: '/repo', displayName: 'Repo' } as Repo +const FOLDER_REPO = { + id: 'folder-1', + path: '/folder', + displayName: 'Folder', + kind: 'folder' +} as Repo + +const REPOS = new Map([ + [GIT_REPO.id, GIT_REPO], + [FOLDER_REPO.id, FOLDER_REPO] +]) + +function makeWorktree(overrides: Partial = {}): Worktree { + return { + id: 'repo-1::/repo-feature', + repoId: 'repo-1', + path: '/repo-feature', + isMainWorktree: false, + isArchived: false, + lastActivityAt: NOW - 90 * DAY_MS, + ...overrides + } as Worktree +} + +describe('countEstimatedInactiveWorkspaces', () => { + it('counts workspaces idle past the cleanup threshold', () => { + expect(countEstimatedInactiveWorkspaces([makeWorktree()], REPOS, NOW)).toBe(1) + }) + + it('does not count a missing activity stamp as ancient', () => { + const worktrees = [ + makeWorktree({ lastActivityAt: 0 }), + makeWorktree({ lastActivityAt: Number.NaN }) + ] + + expect(countEstimatedInactiveWorkspaces(worktrees, REPOS, NOW)).toBe(0) + }) + + it('falls back to the creation stamp when activity was never recorded', () => { + const recent = makeWorktree({ lastActivityAt: 0, createdAt: NOW - DAY_MS }) + const old = makeWorktree({ lastActivityAt: 0, createdAt: NOW - 90 * DAY_MS }) + + expect(countEstimatedInactiveWorkspaces([recent, old], REPOS, NOW)).toBe(1) + }) + + it('skips main worktrees, folder workspaces, and unknown repos', () => { + const worktrees = [ + makeWorktree({ isMainWorktree: true }), + makeWorktree({ repoId: FOLDER_REPO.id }), + makeWorktree({ repoId: 'gone' }) + ] + + expect(countEstimatedInactiveWorkspaces(worktrees, REPOS, NOW)).toBe(0) + }) + + it('uses the shorter archived threshold', () => { + const worktree = makeWorktree({ isArchived: true, lastActivityAt: NOW - 10 * DAY_MS }) + + expect(countEstimatedInactiveWorkspaces([worktree], REPOS, NOW)).toBe(1) + }) +}) diff --git a/src/renderer/src/components/workspace-cleanup/inactive-workspace-estimate.ts b/src/renderer/src/components/workspace-cleanup/inactive-workspace-estimate.ts new file mode 100644 index 00000000000..a0103360f5f --- /dev/null +++ b/src/renderer/src/components/workspace-cleanup/inactive-workspace-estimate.ts @@ -0,0 +1,37 @@ +import type { Repo, Worktree } from '../../../../shared/types' +import { isFolderRepo } from '../../../../shared/repo-kind' +import { + getPersistedWorkspaceCleanupActivityAt, + isWorkspaceOldForCleanup +} from '../../../../shared/workspace-cleanup' + +/** + * Renderer-side estimate of how many workspaces the cleanup scan would list. It + * sees only Orca's persisted activity record, so it disagrees with the scan by + * design — the scan additionally reads each worktree's git history and cannot + * inspect disconnected remotes. + */ +export function countEstimatedInactiveWorkspaces( + worktrees: readonly Worktree[], + repoById: ReadonlyMap, + now: number +): number { + let count = 0 + for (const worktree of worktrees) { + const repo = repoById.get(worktree.repoId) + if (!repo || isFolderRepo(repo) || worktree.isMainWorktree) { + continue + } + // Why: an unstamped workspace (externally created, or never opened here) reads as + // epoch 0, which counted every one of them as idle and inflated the estimate far + // past what the scan lists. Unknown is not old. + const lastActivityAt = getPersistedWorkspaceCleanupActivityAt(worktree) + if ( + lastActivityAt > 0 && + isWorkspaceOldForCleanup({ isArchived: worktree.isArchived, lastActivityAt }, now) + ) { + count += 1 + } + } + return count +} diff --git a/src/renderer/src/components/workspace-cleanup/workspace-cleanup-candidate-row.test.tsx b/src/renderer/src/components/workspace-cleanup/workspace-cleanup-candidate-row.test.tsx index b65e495a7ea..7de51d662cb 100644 --- a/src/renderer/src/components/workspace-cleanup/workspace-cleanup-candidate-row.test.tsx +++ b/src/renderer/src/components/workspace-cleanup/workspace-cleanup-candidate-row.test.tsx @@ -62,4 +62,40 @@ describe('CandidateRow', () => { expect(container?.querySelector(`[aria-label="Select ${candidate.displayName}"]`)).toBeNull() expect(container?.querySelector(`[aria-label="Remove ${candidate.displayName}"]`)).toBeNull() }) + + it.each([ + ['deleting' as const, 'Deleting…'], + ['queued' as const, 'Queued for deletion'] + ])('replaces the status pill with the %s state', (deletionPhase, label) => { + const candidate = makeCandidate() + + act(() => { + root?.render( + + ) + }) + + expect(container?.textContent).toContain(label) + expect(container?.textContent).not.toContain('Ready') + }) }) diff --git a/src/renderer/src/components/workspace-cleanup/workspace-cleanup-candidate-row.tsx b/src/renderer/src/components/workspace-cleanup/workspace-cleanup-candidate-row.tsx index faf540f3a29..2953f46fa5a 100644 --- a/src/renderer/src/components/workspace-cleanup/workspace-cleanup-candidate-row.tsx +++ b/src/renderer/src/components/workspace-cleanup/workspace-cleanup-candidate-row.tsx @@ -8,6 +8,7 @@ import { FileWarning, GitBranch, GitPullRequest, + Loader2, Search, SquareTerminal, Trash2, @@ -40,8 +41,11 @@ import { } from './workspace-cleanup-candidate-row-data' import { StatusPill } from './workspace-cleanup-status-pill' +export type WorkspaceCleanupDeletionPhase = 'deleting' | 'queued' + type CandidateRowProps = { candidate: WorkspaceCleanupCandidate + deletionPhase?: WorkspaceCleanupDeletionPhase expanded: boolean failure?: string last: boolean @@ -99,6 +103,7 @@ function MetadataIconChip({ // prop identity changes); virtualization, not memo, bounds that cost. export const CandidateRow = React.memo(function CandidateRow({ candidate, + deletionPhase, expanded, failure, last, @@ -112,7 +117,10 @@ export const CandidateRow = React.memo(function CandidateRow({ onToggleSelected, onView }: CandidateRowProps): React.JSX.Element { - const selectable = canQueueWorkspaceCleanupCandidate(candidate) && !removing + const deleting = deletionPhase !== undefined + // Why: derive from `deleting` too, so the row never offers a checkbox or + // Remove button while it is queuing/deleting, even if `removing` was omitted. + const selectable = canQueueWorkspaceCleanupCandidate(candidate) && !removing && !deleting const ignored = candidate.blockers.includes('dismissed') const blockers = getWorkspaceCleanupBlockerLabels(candidate) const contextDetails = formatContextDetails(candidate) @@ -133,6 +141,7 @@ export const CandidateRow = React.memo(function CandidateRow({ className={cn( 'group w-full border-b border-border/60 px-3 py-2.5 text-left text-foreground transition-colors hover:bg-accent/40', selected && 'bg-accent/30', + deleting && 'opacity-70', last && 'border-b-0' )} > @@ -152,6 +161,8 @@ export const CandidateRow = React.memo(function CandidateRow({ > {selected ? : null} + ) : deleting ? ( + ) : (