From b2e29a22474b725e5e0f5679dcb5a6296bb8dc2d Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Sun, 9 Aug 2026 18:14:39 -0700 Subject: [PATCH 01/12] Fix Ctrl+Z in Windows and Linux terminals (#13398) * fix(terminal): preserve Windows undo chords * fix(terminal): keep undo menu hints unregistered --- src/main/menu/register-app-menu.test.ts | 20 ++++++++++++++++++++ src/main/menu/register-app-menu.ts | 8 ++++++-- 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/src/main/menu/register-app-menu.test.ts b/src/main/menu/register-app-menu.test.ts index 82d1bc184de..08b5a489a9c 100644 --- a/src/main/menu/register-app-menu.test.ts +++ b/src/main/menu/register-app-menu.test.ts @@ -259,6 +259,26 @@ describe('registerAppMenu', () => { } ) + it.each(['darwin', 'linux', 'win32'] as const)( + 'preserves terminal undo and redo chords on %s', + (platform) => { + vi.spyOn(process, 'platform', 'get').mockReturnValue(platform) + registerAppMenu(buildMenuOptions()) + + const editSubmenu = getSubmenu(getTemplate(), 'Edit') + const expectedRegistration = platform === 'darwin' ? undefined : false + const undoItem = editSubmenu.find((item) => item.role === 'undo') + const redoItem = editSubmenu.find((item) => item.role === 'redo') + + expect(undoItem?.accelerator).toBeUndefined() + expect(redoItem?.accelerator).toBeUndefined() + expect(undoItem && 'registerAccelerator' in undoItem).toBe(platform !== 'darwin') + expect(redoItem && 'registerAccelerator' in redoItem).toBe(platform !== 'darwin') + expect(undoItem?.registerAccelerator).toBe(expectedRegistration) + expect(redoItem?.registerAccelerator).toBe(expectedRegistration) + } + ) + it('keeps selection actions native in a focused guest webview', () => { const send = vi.fn() const guestContents = { copy: vi.fn(), selectAll: vi.fn() } diff --git a/src/main/menu/register-app-menu.ts b/src/main/menu/register-app-menu.ts index 2feeef9fe47..cc99c69cbc7 100644 --- a/src/main/menu/register-app-menu.ts +++ b/src/main/menu/register-app-menu.ts @@ -167,11 +167,15 @@ function buildAndApplyMenu(options: RegisterAppMenuOptions): void { ] } + // Why: keep native menu hints while letting non-macOS Ctrl+Z/Ctrl+Y reach the focused terminal or DOM control. + const undoRedoOptions: Electron.MenuItemConstructorOptions = isMac + ? {} + : { registerAccelerator: false } const editMenu: Electron.MenuItemConstructorOptions = { label: translateMain('menu.edit', 'Edit'), submenu: [ - { role: 'undo' }, - { role: 'redo' }, + { role: 'undo', ...undoRedoOptions }, + { role: 'redo', ...undoRedoOptions }, { type: 'separator' }, { role: 'cut' }, createAppMenuSelectionItem({ From ced2719b26a101440c5bca3e96aa6c5a6aae7383 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sun, 9 Aug 2026 18:29:32 -0700 Subject: [PATCH 02/12] refactor: remove unreachable code (#13400) --- .../worker-terminal-ownership.ts | 30 ------------- .../runtime/relay/relay-control-protocol.ts | 1 - .../rpc/methods/ui-state-schema-parity.ts | 10 ----- src/main/ssh/relay-protocol.ts | 16 ------- .../ssh/ssh-relay-session-test-fixtures.ts | 7 ---- .../remote-browser-stream-errors.ts | 4 -- .../cmd-j/palette-filter-options.ts | 8 ---- .../agent-map-label-declutter.ts | 8 ---- .../components/sidebar/FilterToggleRow.tsx | 2 - .../__fixtures__/xterm-bypass-event.ts | 17 -------- .../src/components/ui/button-group.tsx | 42 +------------------ src/renderer/src/components/ui/card.tsx | 22 +--------- .../src/components/ui/context-menu.tsx | 39 +---------------- .../src/components/ui/dropdown-menu.tsx | 5 --- src/renderer/src/components/ui/select.tsx | 16 ------- src/renderer/src/components/ui/sheet.tsx | 18 +------- .../src/lib/agent-feature-install-commands.ts | 1 - .../src/lib/ime-composition-keyboard-event.ts | 10 ----- src/shared/agent-feature-install-commands.ts | 4 -- src/shared/daemon-lifecycle-telemetry.ts | 5 --- src/shared/hermes-run-ref-retention.ts | 1 - src/shared/left-sidebar-appearance.ts | 2 - src/shared/plugins/plugin-capabilities.ts | 4 -- src/shared/plugins/plugin-panel-bridge.ts | 20 +-------- src/shared/terminal-title-agent-type.ts | 4 -- 25 files changed, 5 insertions(+), 291 deletions(-) delete mode 100644 src/renderer/src/components/terminal-pane/__fixtures__/xterm-bypass-event.ts diff --git a/src/main/runtime/orchestration/worker-terminal-ownership.ts b/src/main/runtime/orchestration/worker-terminal-ownership.ts index d6f160cda4a..022146f0e75 100644 --- a/src/main/runtime/orchestration/worker-terminal-ownership.ts +++ b/src/main/runtime/orchestration/worker-terminal-ownership.ts @@ -75,36 +75,6 @@ export const WORKER_SETTLED_STATES: readonly WorkerDispatchState[] = [ export const WORKER_RELEASABLE_STATES: readonly WorkerDispatchState[] = ['succeeded', 'failed'] -// Same-operation creation evidence: worker-start labels its own agent terminal 'created' -// (existing worktree) or 'reused_agent_terminal' (agent-first worktree creation). An explicit -// --terminal reuse is labeled 'reused' and never claims ownership. -export function residualResourcesClaimAgentTerminal( - residualResourcesJson: string, - terminalHandle: string -): boolean { - let parsed: unknown - try { - parsed = JSON.parse(residualResourcesJson) - } catch { - return false - } - if (!Array.isArray(parsed)) { - return false - } - return parsed.some((effect) => { - if (!effect || typeof effect !== 'object') { - return false - } - const candidate = effect as { kind?: unknown; role?: unknown; action?: unknown; id?: unknown } - return ( - candidate.kind === 'terminal' && - candidate.role === 'agent' && - candidate.id === terminalHandle && - (candidate.action === 'created' || candidate.action === 'reused_agent_terminal') - ) - }) -} - // Process accounting for worker-list; deliberately independent of Task/Dispatch outcome. export function deriveWorkerTerminalListState(params: { workerState: WorkerDispatchState diff --git a/src/main/runtime/relay/relay-control-protocol.ts b/src/main/runtime/relay/relay-control-protocol.ts index 06cf61a95fd..c94797f4815 100644 --- a/src/main/runtime/relay/relay-control-protocol.ts +++ b/src/main/runtime/relay/relay-control-protocol.ts @@ -119,7 +119,6 @@ export const RelayControlErrorMessageSchema = z }) .strict() -export type RelayHostChallengeMessage = z.infer export type RelayHostHelloAckMessage = z.infer export type RelayConnectionOpenMessage = z.infer export type RelayDrainMessage = z.infer diff --git a/src/main/runtime/rpc/methods/ui-state-schema-parity.ts b/src/main/runtime/rpc/methods/ui-state-schema-parity.ts index 29d205e6092..555e8f44cfb 100644 --- a/src/main/runtime/rpc/methods/ui-state-schema-parity.ts +++ b/src/main/runtime/rpc/methods/ui-state-schema-parity.ts @@ -13,16 +13,6 @@ export type AssertNoMissingKeys> ? true : { missingFromSchema: Exclude } -/** - * The mirror of `AssertNoMissingKeys`: a key the schema accepts but the shared - * type does not model. `.strict()` cannot catch this one — it lets the payload - * through, and the renderer then receives a field it has no type for. - */ -export type AssertNoExtraKeys> = - Exclude extends never - ? true - : { absentFromType: Exclude } - /** * Key parity alone is blind to VALUE drift: a schema can list `rightSidebarTab` * yet omit half its union members, which the strict schema then rejects. This diff --git a/src/main/ssh/relay-protocol.ts b/src/main/ssh/relay-protocol.ts index da61921ca6a..8c9f35b96d1 100644 --- a/src/main/ssh/relay-protocol.ts +++ b/src/main/ssh/relay-protocol.ts @@ -63,24 +63,8 @@ export const JsonRpcErrorCode = { * 256KB raw → ~340KB base64, well under MAX_MESSAGE_SIZE. */ export const STREAM_CHUNK_SIZE = 256 * 1024 -/** Cap on concurrent in-flight streams per relay; mirrors fs.watch's - * 20-watcher cap idiom. Prevents file-descriptor exhaustion from a buggy - * client. */ -export const MAX_CONCURRENT_STREAMS = 16 - // ── Git response streaming (see docs/relay-git-response-stream-design.md) ── -/** Serialized-JSON size above which the relay chunks a streamable git response - * (diff family + exec) onto the bulk lane instead of one JSON-RPC frame. Mirror - * of the relay-side constant; the client only opts in — the relay owns the - * decision — so this is documentation of the shared contract. */ -export const GIT_RESPONSE_STREAM_THRESHOLD = 256 * 1024 - -/** Per-chunk size (serialized-result UTF-8 bytes) for git response streaming. - * The client reassembles by concatenation and does not depend on this value, - * so it stays cross-version safe. */ -export const GIT_RESPONSE_CHUNK_SIZE = 128 * 1024 - /** Sentinel the relay returns as the RPC result when the real payload streams * as git.responseChunk frames. Absent from old relays, so a new client falls * back to the plain result they return. */ diff --git a/src/main/ssh/ssh-relay-session-test-fixtures.ts b/src/main/ssh/ssh-relay-session-test-fixtures.ts index 48461af68ca..c5f084a8785 100644 --- a/src/main/ssh/ssh-relay-session-test-fixtures.ts +++ b/src/main/ssh/ssh-relay-session-test-fixtures.ts @@ -1,6 +1,5 @@ import { vi, type Mock } from 'vitest' import type { BrowserWindow } from 'electron' -import { PTY_CONSUMER_STALE_OWNER_RECOVERY_ERROR } from '../../shared/pty-consumer-session' import type { SshConnection } from './ssh-connection' import type { Store } from '../persistence' import type { SshPortForwardManager } from './ssh-port-forward' @@ -54,9 +53,3 @@ export function mockDeploySuccess(): void { platform: 'linux-x64' }) } - -export function createMismatchedOwnerRecoveryError(): unknown { - return Object.assign(new Error('Owner recovery lease is stale'), { - code: PTY_CONSUMER_STALE_OWNER_RECOVERY_ERROR - }) -} diff --git a/src/renderer/src/components/browser-pane/remote-browser-stream-errors.ts b/src/renderer/src/components/browser-pane/remote-browser-stream-errors.ts index 7db8458d266..6e37bb3021a 100644 --- a/src/renderer/src/components/browser-pane/remote-browser-stream-errors.ts +++ b/src/renderer/src/components/browser-pane/remote-browser-stream-errors.ts @@ -48,10 +48,6 @@ function readErrorCode(error: unknown): string | null { return typeof code === 'string' ? code : null } -export function isRemoteBrowserStreamUnsupportedError(error: unknown): boolean { - return readErrorCode(error) === REMOTE_BROWSER_STREAM_UNSUPPORTED -} - export function isRemoteBrowserPageMissingCode(code: unknown): boolean { return typeof code === 'string' && REMOTE_BROWSER_PAGE_MISSING_CODES.has(code) } diff --git a/src/renderer/src/components/cmd-j/palette-filter-options.ts b/src/renderer/src/components/cmd-j/palette-filter-options.ts index 58dfabb8eb2..9eb5fdce4ac 100644 --- a/src/renderer/src/components/cmd-j/palette-filter-options.ts +++ b/src/renderer/src/components/cmd-j/palette-filter-options.ts @@ -51,14 +51,6 @@ export type PaletteFilterModel = { defaultHostId: ExecutionHostId } -export const EMPTY_PALETTE_FILTER_MODEL: PaletteFilterModel = { - hosts: [], - projects: [], - repoIdsByProjectKey: new Map(), - hostIdByRepoId: new Map(), - defaultHostId: LOCAL_EXECUTION_HOST_ID -} - /** * Precomputes only the repos that actually carry a host stamp so the lookup miss * below stays equivalent to getWorktreeExecutionHostId's `defaultHostId` branch. diff --git a/src/renderer/src/components/dashboard-popout/agent-map-label-declutter.ts b/src/renderer/src/components/dashboard-popout/agent-map-label-declutter.ts index 020d8453dd2..58a1f53c4d6 100644 --- a/src/renderer/src/components/dashboard-popout/agent-map-label-declutter.ts +++ b/src/renderer/src/components/dashboard-popout/agent-map-label-declutter.ts @@ -49,14 +49,6 @@ function textWidth(text: string, fontPx: number, ratio = GLYPH_WIDTH_RATIO): num return text.length * fontPx * ratio } -export function agentMapWorktreeLabelWidth(text: string): number { - return textWidth(text, WORKTREE_LABEL_FONT_PX) -} - -export function agentMapCountLabelWidth(text: string): number { - return textWidth(text, COUNT_FONT_PX) -} - function boxesOverlap(a: LabelBox, b: LabelBox): boolean { return a.left < b.right && b.left < a.right && a.top < b.bottom && b.top < a.bottom } diff --git a/src/renderer/src/components/sidebar/FilterToggleRow.tsx b/src/renderer/src/components/sidebar/FilterToggleRow.tsx index 7fbc0b77874..bcb47550649 100644 --- a/src/renderer/src/components/sidebar/FilterToggleRow.tsx +++ b/src/renderer/src/components/sidebar/FilterToggleRow.tsx @@ -69,5 +69,3 @@ export function FilterToggleRow({ ) } - -export default FilterToggleRow diff --git a/src/renderer/src/components/terminal-pane/__fixtures__/xterm-bypass-event.ts b/src/renderer/src/components/terminal-pane/__fixtures__/xterm-bypass-event.ts deleted file mode 100644 index f4b78599107..00000000000 --- a/src/renderer/src/components/terminal-pane/__fixtures__/xterm-bypass-event.ts +++ /dev/null @@ -1,17 +0,0 @@ -import type { XtermBypassEvent } from '../xterm-bypass-policy' - -// Shared test fixture: builds a fully-defaulted XtermBypassEvent so the bypass -// and IME candidate-guard suites all stay in sync when the event shape changes. -export function event(overrides: Partial): XtermBypassEvent { - return { - type: 'keydown', - key: '', - code: '', - defaultPrevented: false, - metaKey: false, - ctrlKey: false, - altKey: false, - shiftKey: false, - ...overrides - } -} diff --git a/src/renderer/src/components/ui/button-group.tsx b/src/renderer/src/components/ui/button-group.tsx index 4cd3151d485..6607102236e 100644 --- a/src/renderer/src/components/ui/button-group.tsx +++ b/src/renderer/src/components/ui/button-group.tsx @@ -1,8 +1,6 @@ import * as React from 'react' import { cva, type VariantProps } from 'class-variance-authority' -import { Slot } from 'radix-ui' -import { Separator } from '@/components/ui/separator' import { cn } from '@/lib/utils' const buttonGroupVariants = cva( @@ -38,42 +36,4 @@ function ButtonGroup({ ) } -function ButtonGroupText({ - className, - asChild = false, - ...props -}: React.ComponentProps<'div'> & { - asChild?: boolean -}) { - const Comp = asChild ? Slot.Root : 'div' - - return ( - - ) -} - -function ButtonGroupSeparator({ - className, - orientation = 'vertical', - ...props -}: React.ComponentProps) { - return ( - - ) -} - -export { ButtonGroup, ButtonGroupSeparator, ButtonGroupText, buttonGroupVariants } +export { ButtonGroup, buttonGroupVariants } diff --git a/src/renderer/src/components/ui/card.tsx b/src/renderer/src/components/ui/card.tsx index 15f22c2bf1d..38fadc90124 100644 --- a/src/renderer/src/components/ui/card.tsx +++ b/src/renderer/src/components/ui/card.tsx @@ -48,28 +48,8 @@ function CardDescription({ className, ...props }: React.ComponentProps<'div'>) { ) } -function CardAction({ className, ...props }: React.ComponentProps<'div'>) { - return ( -
- ) -} - function CardContent({ className, ...props }: React.ComponentProps<'div'>) { return
} -function CardFooter({ className, ...props }: React.ComponentProps<'div'>) { - return ( -
- ) -} - -export { Card, CardHeader, CardFooter, CardTitle, CardAction, CardDescription, CardContent } +export { Card, CardHeader, CardTitle, CardDescription, CardContent } diff --git a/src/renderer/src/components/ui/context-menu.tsx b/src/renderer/src/components/ui/context-menu.tsx index 2b0423e0cd7..60ab6e38055 100644 --- a/src/renderer/src/components/ui/context-menu.tsx +++ b/src/renderer/src/components/ui/context-menu.tsx @@ -1,5 +1,5 @@ import * as React from 'react' -import { CheckIcon, ChevronRightIcon, CircleIcon } from 'lucide-react' +import { ChevronRightIcon, CircleIcon } from 'lucide-react' import { ContextMenu as ContextMenuPrimitive } from 'radix-ui' import { cn } from '@/lib/utils' @@ -14,14 +14,6 @@ function ContextMenuTrigger({ return } -function ContextMenuGroup({ ...props }: React.ComponentProps) { - return -} - -function ContextMenuPortal({ ...props }: React.ComponentProps) { - return -} - function ContextMenuSub({ ...props }: React.ComponentProps) { return } @@ -123,32 +115,6 @@ function ContextMenuItem({ ) } -function ContextMenuCheckboxItem({ - className, - children, - checked, - ...props -}: React.ComponentProps) { - return ( - - - - - - - {children} - - ) -} - function ContextMenuRadioItem({ className, children, @@ -224,13 +190,10 @@ export { ContextMenuTrigger, ContextMenuContent, ContextMenuItem, - ContextMenuCheckboxItem, ContextMenuRadioItem, ContextMenuLabel, ContextMenuSeparator, ContextMenuShortcut, - ContextMenuGroup, - ContextMenuPortal, ContextMenuSub, ContextMenuSubContent, ContextMenuSubTrigger, diff --git a/src/renderer/src/components/ui/dropdown-menu.tsx b/src/renderer/src/components/ui/dropdown-menu.tsx index d7fa2787671..eeebe9a4d2d 100644 --- a/src/renderer/src/components/ui/dropdown-menu.tsx +++ b/src/renderer/src/components/ui/dropdown-menu.tsx @@ -45,10 +45,6 @@ function DropdownMenuContent({ ) } -function DropdownMenuGroup({ ...props }: React.ComponentProps) { - return -} - function DropdownMenuItem({ className, inset, @@ -229,7 +225,6 @@ export { DropdownMenuPortal, DropdownMenuTrigger, DropdownMenuContent, - DropdownMenuGroup, DropdownMenuLabel, DropdownMenuItem, DropdownMenuCheckboxItem, diff --git a/src/renderer/src/components/ui/select.tsx b/src/renderer/src/components/ui/select.tsx index d25c887b328..c94cc4659fc 100644 --- a/src/renderer/src/components/ui/select.tsx +++ b/src/renderer/src/components/ui/select.tsx @@ -10,10 +10,6 @@ function Select({ ...props }: React.ComponentProps) return } -function SelectGroup({ ...props }: React.ComponentProps) { - return -} - function SelectValue({ ...props }: React.ComponentProps) { return } @@ -88,16 +84,6 @@ function SelectContent({ ) } -function SelectLabel({ className, ...props }: React.ComponentProps) { - return ( - - ) -} - function SelectItem({ className, children, @@ -171,9 +157,7 @@ function SelectScrollDownButton({ export { Select, SelectContent, - SelectGroup, SelectItem, - SelectLabel, SelectScrollDownButton, SelectScrollUpButton, SelectSeparator, diff --git a/src/renderer/src/components/ui/sheet.tsx b/src/renderer/src/components/ui/sheet.tsx index 0e722f1a4f5..24f5dfcebe5 100644 --- a/src/renderer/src/components/ui/sheet.tsx +++ b/src/renderer/src/components/ui/sheet.tsx @@ -12,10 +12,6 @@ function Sheet({ ...props }: React.ComponentProps) { return } -function SheetTrigger({ ...props }: React.ComponentProps) { - return -} - function SheetClose({ ...props }: React.ComponentProps) { return } @@ -124,16 +120,6 @@ function SheetHeader({ className, ...props }: React.ComponentProps<'div'>) { ) } -function SheetFooter({ className, ...props }: React.ComponentProps<'div'>) { - return ( -
- ) -} - function SheetTitle({ className, ...props }: React.ComponentProps) { return ( -export function isPluginCapabilityKind(value: string): value is PluginCapabilityKind { - return (PLUGIN_CAPABILITY_KINDS as readonly string[]).includes(value) -} - /** Plain-language consent copy per capability. Shown verbatim in the install * preview / consent dialog; keep each line honest about what is enforced. */ export const PLUGIN_CAPABILITY_DESCRIPTIONS: Record = { diff --git a/src/shared/plugins/plugin-panel-bridge.ts b/src/shared/plugins/plugin-panel-bridge.ts index 95c8a62d5ff..7d659fcc97f 100644 --- a/src/shared/plugins/plugin-panel-bridge.ts +++ b/src/shared/plugins/plugin-panel-bridge.ts @@ -1,5 +1,5 @@ import { z } from 'zod' -import { getPluginHostMethodSpec, isPluginPanelAction } from './plugin-host-api' +import { isPluginPanelAction } from './plugin-host-api' /** * postMessage protocol between a sandboxed plugin panel iframe and the host @@ -150,21 +150,3 @@ export function readPanelPongId(data: unknown): number | null { // wider reader would admit ids the watchdog can never have issued. return Number.isSafeInteger(frame.pingId) && frame.pingId >= 0 ? frame.pingId : null } - -/** Validates action params against the host API spec (shared with workers). */ -export function parsePanelActionParams( - action: string, - params: unknown -): { ok: true; params: unknown } | { ok: false; error: string } { - const spec = getPluginHostMethodSpec(action) - if (!spec) { - return { ok: false, error: `unknown action: ${action}` } - } - const parsed = spec.params.safeParse(params) - if (!parsed.success) { - const issue = parsed.error.issues[0] - const path = issue?.path.join('.') || '(root)' - return { ok: false, error: `${path}: ${issue?.message ?? 'invalid action params'}` } - } - return { ok: true, params: parsed.data } -} diff --git a/src/shared/terminal-title-agent-type.ts b/src/shared/terminal-title-agent-type.ts index 6287648739b..7e38a227f34 100644 --- a/src/shared/terminal-title-agent-type.ts +++ b/src/shared/terminal-title-agent-type.ts @@ -51,10 +51,6 @@ export function isGeminiTerminalTitle(title: string): boolean { return titleHasAgentName(title, 'gemini') } -export function isPiTerminalTitle(title: string): boolean { - return isLegacyPiCompatibleTitle(title) && !containsBrailleSpinner(title) -} - // Why: Grok Build's working OSC titles use a fixed frame shape — // "spinner - - grok" — so every frame is a distinct title // that flips tab and sidebar labels. Require BOTH the post-spinner " - " From 954de385077e379e969c89d0a2497d514b3c0590 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sun, 9 Aug 2026 18:30:44 -0700 Subject: [PATCH 03/12] refactor(mobile): remove unused PR and review code (#13404) --- .../src/components/MobilePrComposeSheet.tsx | 52 ------------------- mobile/src/components/mobile-pr-url.ts | 6 +++ .../pr-sidebar/MobilePrComposeForm.tsx | 6 +-- .../components/pr-sidebar/PRSidebarHeader.tsx | 2 +- .../pr-sidebar/PrSidebarCreateEmptyState.tsx | 2 +- .../src/session/mobile-diff-review-state.ts | 25 --------- .../use-native-chat-action-outcomes.ts | 13 ----- .../MobileSourceControlModals.tsx | 2 +- .../MobileSourceControlPanel.tsx | 2 +- .../source-control/mobile-base-ref-search.ts | 21 -------- .../source-control/mobile-open-pr-prefill.ts | 26 +--------- 11 files changed, 12 insertions(+), 145 deletions(-) delete mode 100644 mobile/src/components/MobilePrComposeSheet.tsx create mode 100644 mobile/src/components/mobile-pr-url.ts diff --git a/mobile/src/components/MobilePrComposeSheet.tsx b/mobile/src/components/MobilePrComposeSheet.tsx deleted file mode 100644 index 124f9192256..00000000000 --- a/mobile/src/components/MobilePrComposeSheet.tsx +++ /dev/null @@ -1,52 +0,0 @@ -import { Linking } from 'react-native' -import { BottomDrawer } from './BottomDrawer' -import { MobilePrComposeForm, type PrComposePrefill } from './pr-sidebar/MobilePrComposeForm' -import type { RpcClient } from '../transport/rpc-client' - -type Props = { - visible: boolean - client: RpcClient | null - worktreeId: string - prefill: PrComposePrefill - // Head branch — enables the base≠head guard and the "from " hint. - head?: string | null - onClose: () => void - onCreated: (url: string, warning?: string) => void -} - -// BottomDrawer wrapper around the inline compose form, for full-screen roots -// (source-control modals). The PR sidebar empty-state renders MobilePrComposeForm -// inline instead, since a BottomDrawer overlay nested in a ScrollView clips it. -export function MobilePrComposeSheet({ - visible, - client, - worktreeId, - prefill, - head, - onClose, - onCreated -}: Props) { - return ( - - {/* Why: key on visible + prefill fields so reopening (or a new prefill) - remounts the form with fresh initial values; the previous sheet reset - its fields via an effect on the same signals. */} - - - ) -} - -export function openMobilePrUrl(url: string): void { - // Why: Linking.openURL rejects when iOS/Android can't open the URL (no app, - // bad scheme, etc.). Without a catch that surfaces as LogBox "Uncaught - // (in promise) Error: Unable to open URL…" over the PR screen. - void Linking.openURL(url).catch(() => {}) -} diff --git a/mobile/src/components/mobile-pr-url.ts b/mobile/src/components/mobile-pr-url.ts new file mode 100644 index 00000000000..3f642d5ccf6 --- /dev/null +++ b/mobile/src/components/mobile-pr-url.ts @@ -0,0 +1,6 @@ +import { Linking } from 'react-native' + +export function openMobilePrUrl(url: string): void { + // Prevent unhandled rejections when no app accepts the URL. + void Linking.openURL(url).catch(() => {}) +} diff --git a/mobile/src/components/pr-sidebar/MobilePrComposeForm.tsx b/mobile/src/components/pr-sidebar/MobilePrComposeForm.tsx index 618e399d1a0..d832d38c88d 100644 --- a/mobile/src/components/pr-sidebar/MobilePrComposeForm.tsx +++ b/mobile/src/components/pr-sidebar/MobilePrComposeForm.tsx @@ -38,11 +38,7 @@ type Props = { onCreated: (url: string, warning?: string) => void } -// PR compose form body: title/body/base/draft with AI prefill (git.generate -// PullRequestFields), submitting via createMobilePr. Renders a plain View so it -// can sit inline inside the PR sidebar's existing ScrollView (a BottomDrawer -// overlay trapped in a ScrollView clips the form). The BottomDrawer wrapper -// MobilePrComposeSheet reuses this body at full-screen roots. +// A plain View keeps the compose form usable inside the PR sidebar ScrollView. export function MobilePrComposeForm({ client, worktreeId, diff --git a/mobile/src/components/pr-sidebar/PRSidebarHeader.tsx b/mobile/src/components/pr-sidebar/PRSidebarHeader.tsx index 8a399babe78..335108e07ae 100644 --- a/mobile/src/components/pr-sidebar/PRSidebarHeader.tsx +++ b/mobile/src/components/pr-sidebar/PRSidebarHeader.tsx @@ -7,7 +7,7 @@ import type { MobilePrTitleAction } from '../../session/use-mobile-pr-title-acti import { prStateBadge } from './pr-checks-presentation' import { statusColor } from './pr-sidebar-status-color' import { canEditPRTitle } from '../../session/pr-title-edit' -import { openMobilePrUrl } from '../MobilePrComposeSheet' +import { openMobilePrUrl } from '../mobile-pr-url' import { mobilePrSidebarStyles as styles } from './mobile-pr-sidebar-styles' import { prCommentComposerStyles as composerStyles } from './pr-comment-composer-styles' diff --git a/mobile/src/components/pr-sidebar/PrSidebarCreateEmptyState.tsx b/mobile/src/components/pr-sidebar/PrSidebarCreateEmptyState.tsx index ebc724d947d..cffde86ea5b 100644 --- a/mobile/src/components/pr-sidebar/PrSidebarCreateEmptyState.tsx +++ b/mobile/src/components/pr-sidebar/PrSidebarCreateEmptyState.tsx @@ -18,7 +18,7 @@ import { runMobileHostedReviewCreateIntent } from '../../source-control/mobile-hosted-review-create-intent-runner' import { fetchWorktreeLinkedPR } from '../../source-control/mobile-pr-link' -import { openMobilePrUrl } from '../MobilePrComposeSheet' +import { openMobilePrUrl } from '../mobile-pr-url' import { MobileLinkPrForm } from './MobileLinkPrForm' import { prCreateEmptyStateStyles as styles } from './pr-create-empty-state-styles' diff --git a/mobile/src/session/mobile-diff-review-state.ts b/mobile/src/session/mobile-diff-review-state.ts index a9bab23471b..a9680a66df5 100644 --- a/mobile/src/session/mobile-diff-review-state.ts +++ b/mobile/src/session/mobile-diff-review-state.ts @@ -104,31 +104,6 @@ export function mergeMobileDiffReviewState( } } -export function markMobileDiffReviewFileOpened( - state: MobileDiffReviewState, - descriptor: MobileDiffReviewFileDescriptor, - now: number -): MobileDiffReviewState { - const previous = state.files[descriptor.key] - return { - ...state, - updatedAt: now, - files: { - ...state.files, - [descriptor.key]: { - key: descriptor.key, - filePath: descriptor.filePath, - oldPath: descriptor.oldPath, - scope: descriptor.scope, - reviewedAt: previous?.reviewedAt, - reviewDiffIdentity: previous?.reviewDiffIdentity, - lastOpenedAt: now, - lastSeenDiffIdentity: descriptor.diffIdentity - } - } - } -} - export function markMobileDiffReviewFileReviewed( state: MobileDiffReviewState, descriptor: MobileDiffReviewFileDescriptor, diff --git a/mobile/src/session/use-native-chat-action-outcomes.ts b/mobile/src/session/use-native-chat-action-outcomes.ts index 51a6f831b5a..a81d08dfdff 100644 --- a/mobile/src/session/use-native-chat-action-outcomes.ts +++ b/mobile/src/session/use-native-chat-action-outcomes.ts @@ -1,5 +1,4 @@ import { useCallback } from 'react' -import type { MobileNativeChatSendOutcome } from './mobile-native-chat-send' /** Wraps a chat card action so an accepted write also retires the route's held * failure banner. @@ -23,15 +22,3 @@ export function useNativeChatAcceptedAction( [action, onAccepted] ) } - -/** Boolean surface for callers with no pre-pasted input: 'unknown' stays true - * (the send usually landed; the optimistic echo is already held unconfirmed). */ -export function useNativeChatSentFlag( - send: (text: string, images?: string[]) => Promise -): (text: string, images?: string[]) => Promise { - return useCallback( - async (text: string, images?: string[]): Promise => - (await send(text, images)) !== 'rejected', - [send] - ) -} diff --git a/mobile/src/source-control/MobileSourceControlModals.tsx b/mobile/src/source-control/MobileSourceControlModals.tsx index b58d2520723..f9659240e4a 100644 --- a/mobile/src/source-control/MobileSourceControlModals.tsx +++ b/mobile/src/source-control/MobileSourceControlModals.tsx @@ -1,7 +1,7 @@ import { ActionSheetModal, type ActionSheetAction } from '../components/ActionSheetModal' import { ConfirmModal } from '../components/ConfirmModal' import { PickerModal } from '../components/PickerModal' -import { openMobilePrUrl } from '../components/MobilePrComposeSheet' +import { openMobilePrUrl } from '../components/mobile-pr-url' import { MobileBranchDiffPreviewDrawer } from './MobileBranchDiffPreviewDrawer' import type { MobileSourceControlState } from './use-mobile-source-control-state' diff --git a/mobile/src/source-control/MobileSourceControlPanel.tsx b/mobile/src/source-control/MobileSourceControlPanel.tsx index 9c3ebb27062..3244d440b01 100644 --- a/mobile/src/source-control/MobileSourceControlPanel.tsx +++ b/mobile/src/source-control/MobileSourceControlPanel.tsx @@ -18,7 +18,7 @@ import { isMobileConflictAborting } from './mobile-source-control-conflict-abort import { useMobilePrSidebarController } from '../session/use-mobile-pr-sidebar-controller' import { prSidebarDetailsNeedFetch } from '../session/mobile-pr-sidebar-state' import { MobilePrViewPanelBody } from '../components/pr-sidebar/MobilePrViewPanel' -import { openMobilePrUrl } from '../components/MobilePrComposeSheet' +import { openMobilePrUrl } from '../components/mobile-pr-url' export type MobileSourceControlPanelProps = { hostId: string diff --git a/mobile/src/source-control/mobile-base-ref-search.ts b/mobile/src/source-control/mobile-base-ref-search.ts index 8e51fe0d582..84a962a41e5 100644 --- a/mobile/src/source-control/mobile-base-ref-search.ts +++ b/mobile/src/source-control/mobile-base-ref-search.ts @@ -2,13 +2,6 @@ import type { RpcClient } from '../transport/rpc-client' import type { RpcSuccess } from '../transport/types' import { mobileRepoSelectorFromWorktreeId } from './mobile-pr-create' -// Base-branch selection for the create-PR composer, mirroring the desktop -// useCreatePullRequestDialogFields flow: a default ref from repo.baseRefDefault and -// a debounced search via repo.searchRefs (both allowlisted for mobile). Result -// mapping is pure + unit-tested; the wrappers are thin sendRequest calls. - -// Defensively normalize the repo.searchRefs payload (`{ refs: string[] }`) to a -// clean string[] — drops non-string / malformed entries instead of throwing. export function mapBaseRefResults(raw: unknown): string[] { if (raw === null || typeof raw !== 'object') { return [] @@ -20,20 +13,6 @@ export function mapBaseRefResults(raw: unknown): string[] { return refs.filter((r): r is string => typeof r === 'string' && r.length > 0) } -export async function fetchDefaultBaseRef( - client: Pick, - worktreeId: string -): Promise { - const response = await client.sendRequest('repo.baseRefDefault', { - repo: mobileRepoSelectorFromWorktreeId(worktreeId) - }) - if (!response.ok) { - return null - } - const result = (response as RpcSuccess).result as { defaultBaseRef?: string | null } - return typeof result?.defaultBaseRef === 'string' ? result.defaultBaseRef : null -} - export async function searchBaseRefs( client: Pick, worktreeId: string, diff --git a/mobile/src/source-control/mobile-open-pr-prefill.ts b/mobile/src/source-control/mobile-open-pr-prefill.ts index 705b4dc68e5..2369095f328 100644 --- a/mobile/src/source-control/mobile-open-pr-prefill.ts +++ b/mobile/src/source-control/mobile-open-pr-prefill.ts @@ -1,14 +1,7 @@ -import type { RpcClient } from '../transport/rpc-client' import { readMobileGitStatusResult } from '../session/mobile-diff-review-rpc' import type { MobileGitStatusResult } from './mobile-git-status' -import { resolveMobilePrPrefill, type MobilePrPrefill } from './mobile-pr-create' -// Resolves the create-PR prefill from a git status snapshot. Split from the -// runners hook to keep that file under the line limit. - -// Reads a fresh git.status after a push so the prefill reflects the just-pushed -// branch's upstream/ahead data instead of the pre-push captured status. Best-effort: -// returns the captured status on any read failure. +// Refresh after a push when possible so readiness reflects the new upstream state. export async function readFreshGitStatus( worktreeId: string, fallback: MobileGitStatusResult | null, @@ -22,23 +15,6 @@ export async function readFreshGitStatus( } } -export async function buildOpenPrPrefill( - client: Pick | null, - worktreeId: string, - status: MobileGitStatusResult | null, - branchLabel: string -): Promise { - if (!client) { - return { provider: 'github', base: 'main', title: branchLabel, body: '' } - } - const gitReadiness = getMobilePrEligibilityReadiness(status) - return resolveMobilePrPrefill(client, worktreeId, { - branch: status?.branch, - title: branchLabel, - ...gitReadiness - }) -} - export function getMobilePrEligibilityReadiness(status: MobileGitStatusResult | null): { hasUncommittedChanges?: boolean hasUpstream?: boolean From c56ff58dd736bb163fa682ca995ef9ae996a2f5a Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sun, 9 Aug 2026 18:31:00 -0700 Subject: [PATCH 04/12] test(mobile): centralize renderer setup (#13408) --- .../src/accounts-route-reset-credit.test.ts | 25 +- .../components/CodexResetCreditAction.test.ts | 44 +-- .../src/components/HostProtocolGate.test.ts | 1 - .../components/MobileHomeQuickActions.test.ts | 6 +- mobile/src/components/MobileHostCard.test.ts | 6 +- mobile/src/components/MobileHostCard.test.tsx | 6 +- .../MobileMarkdown.file-links.test.ts | 1 - .../src/components/NewWorktreeModal.test.tsx | 1 - .../PickerModal.accessibility.test.ts | 1 - .../src/components/WorktreeAgentList.test.tsx | 6 +- mobile/src/components/WorktreeListRow.test.ts | 1 - .../bottom-drawer-close-lifecycle.test.ts | 4 +- .../mobile-agent-icon-gradient.test.ts | 6 +- .../pr-sidebar/PRCommentsSection.test.ts | 24 +- .../src/files/MobileFileExplorerPanel.test.ts | 37 +- .../files/MobileFileMarkdownPreview.test.ts | 23 +- mobile/src/hooks/use-now.test.ts | 12 +- .../src/host-edit-route-accessibility.test.ts | 26 +- mobile/src/host-edit-save-flow.test.ts | 27 +- .../onboarding/MobileOnboardingPage.test.ts | 6 +- .../mobile-onboarding-screen.test.ts | 1 - .../session/MobileNativeChatComposer.test.ts | 373 ++++++++---------- .../session/MobileNativeChatMessage.test.ts | 20 +- .../session/MobileNativeChatOverlay.test.ts | 28 +- .../MobileNativeChatPermission.test.ts | 31 +- ...bileNativeChatSessionOptionPickers.test.ts | 1 - .../src/session/MobileNativeChatView.test.ts | 28 +- mobile/src/session/QuickCommandsList.test.ts | 4 +- mobile/src/session/QuickCommandsSheet.test.ts | 13 +- ...mobile-native-chat-permission-send.test.ts | 1 - ...nitial-session-terminal-autocreate.test.ts | 1 - .../session/use-live-worktree-name.test.ts | 54 +-- .../use-missing-worktree-bounce.test.ts | 1 - ...mobile-attachment-input-lease-gate.test.ts | 10 - ...le-default-session-view-preference.test.ts | 1 - .../use-mobile-diff-review-controller.test.ts | 1 - ...se-mobile-diff-review-diff-loading.test.ts | 1 - ...se-mobile-diff-review-send-actions.test.ts | 16 +- .../use-mobile-file-tap-handlers.test.ts | 1 - ...use-mobile-native-chat-answer-send.test.ts | 16 +- ...use-mobile-native-chat-ask-dismiss.test.ts | 18 +- .../use-mobile-native-chat-controller.test.ts | 64 +-- ...le-native-chat-drafts-launch-draft.test.ts | 21 +- .../use-mobile-native-chat-drafts.test.ts | 21 +- ...use-mobile-native-chat-file-search.test.ts | 23 +- ...bile-native-chat-image-attachments.test.ts | 16 +- ...use-mobile-native-chat-input-lease.test.ts | 86 ++-- ...se-mobile-native-chat-message-send.test.ts | 1 - ...use-mobile-native-chat-readability.test.ts | 55 +-- .../use-mobile-native-chat-send-error.test.ts | 68 +--- ...mobile-native-chat-session-options.test.ts | 1 - .../use-mobile-native-chat-session.test.ts | 32 +- .../use-mobile-native-chat-stop.test.ts | 1 - ...mobile-native-chat-terminal-stream.test.ts | 297 ++++++-------- ...mobile-session-tabs-reconciliation.test.ts | 11 - .../use-mobile-session-view-mode.test.ts | 1 - mobile/src/session/use-quick-commands.test.ts | 16 +- ...erminal-live-input-mode-preference.test.ts | 23 +- .../use-throttled-latest-value.test.ts | 12 +- .../MobileGitHistoryList.test.tsx | 6 +- ...e-mobile-hosted-review-eligibility.test.ts | 6 +- .../terminal-webview-engine-error.test.ts | 23 +- ...rminal-live-accessory-input-commit.test.ts | 23 +- .../use-terminal-live-input-commit.test.ts | 23 +- .../use-terminal-live-input-focus.test.ts | 28 +- mobile/src/transport/client-context.test.ts | 64 +-- .../src/transport/host-status-gates.test.ts | 19 - .../settings-host-client-lifecycle.test.ts | 130 ++---- mobile/vitest.config.ts | 2 + mobile/vitest.setup.ts | 1 + 70 files changed, 508 insertions(+), 1449 deletions(-) create mode 100644 mobile/vitest.setup.ts diff --git a/mobile/src/accounts-route-reset-credit.test.ts b/mobile/src/accounts-route-reset-credit.test.ts index 76be8071930..2de1e96881d 100644 --- a/mobile/src/accounts-route-reset-credit.test.ts +++ b/mobile/src/accounts-route-reset-credit.test.ts @@ -157,28 +157,12 @@ const RESET_SNAPSHOT = { } } as const -function suppressReactTestRendererDeprecationWarning(): () => void { - const originalConsoleError = console.error - const spy = vi.spyOn(console, 'error').mockImplementation((...args) => { - if (typeof args[0] === 'string' && args[0].includes('react-test-renderer is deprecated')) { - return - } - originalConsoleError(...args) - }) - return () => spy.mockRestore() -} - async function renderAccountsRoute(): Promise { let renderer: ReactTestRenderer | null = null - const restoreConsoleError = suppressReactTestRendererDeprecationWarning() - try { - await act(async () => { - renderer = create(createElement(AccountsScreen)) - await Promise.resolve() - }) - } finally { - restoreConsoleError() - } + await act(async () => { + renderer = create(createElement(AccountsScreen)) + await Promise.resolve() + }) if (!renderer) { throw new Error('Accounts route did not render') } @@ -229,7 +213,6 @@ describe('accounts route Codex reset credit', () => { let storedValues: Map beforeEach(() => { - globalThis.IS_REACT_ACT_ENVIRONMENT = true resetCodexResetAttemptJournalForTests() storedValues = new Map() dependencies.alert.mockReset() diff --git a/mobile/src/components/CodexResetCreditAction.test.ts b/mobile/src/components/CodexResetCreditAction.test.ts index 25e39591701..0c1a6ea57b4 100644 --- a/mobile/src/components/CodexResetCreditAction.test.ts +++ b/mobile/src/components/CodexResetCreditAction.test.ts @@ -1,6 +1,6 @@ import { createElement } from 'react' import { act, create, type ReactTestRenderer } from 'react-test-renderer' -import { afterEach, describe, expect, it, vi } from 'vitest' +import { describe, expect, it, vi } from 'vitest' import { CodexResetCreditAction } from './CodexResetCreditAction' vi.mock('react-native', () => ({ @@ -19,35 +19,19 @@ const summary = { expiryLabel: 'Expires in 5d' } -function suppressRendererWarning(): () => void { - const original = console.error - const spy = vi.spyOn(console, 'error').mockImplementation((...args) => { - if (typeof args[0] === 'string' && args[0].includes('react-test-renderer is deprecated')) { - return - } - original(...args) - }) - return () => spy.mockRestore() -} - function renderAction(busy: boolean, disabled: boolean): ReactTestRenderer { let renderer: ReactTestRenderer | null = null - const restore = suppressRendererWarning() - try { - act(() => { - renderer = create( - createElement(CodexResetCreditAction, { - summary, - scopeLabel: 'dev@example.com on the host', - busy, - disabled, - onPress: vi.fn() - }) - ) - }) - } finally { - restore() - } + act(() => { + renderer = create( + createElement(CodexResetCreditAction, { + summary, + scopeLabel: 'dev@example.com on the host', + busy, + disabled, + onPress: vi.fn() + }) + ) + }) if (!renderer) { throw new Error('Reset action did not render') } @@ -55,10 +39,6 @@ function renderAction(busy: boolean, disabled: boolean): ReactTestRenderer { } describe('CodexResetCreditAction', () => { - afterEach(() => { - vi.restoreAllMocks() - }) - it('exposes a 44pt touch target and enabled accessibility state', () => { const renderer = renderAction(false, false) const button = renderer.root.findByType('Pressable') diff --git a/mobile/src/components/HostProtocolGate.test.ts b/mobile/src/components/HostProtocolGate.test.ts index 94cae6d9170..44a2265ccb3 100644 --- a/mobile/src/components/HostProtocolGate.test.ts +++ b/mobile/src/components/HostProtocolGate.test.ts @@ -78,7 +78,6 @@ describe('HostProtocolGate', () => { let renderer: ReactTestRenderer | null = null beforeEach(() => { - globalThis.IS_REACT_ACT_ENVIRONMENT = true nativeTestState.openUrl.mockClear() nativeTestState.platform.OS = 'ios' probeMounts.count = 0 diff --git a/mobile/src/components/MobileHomeQuickActions.test.ts b/mobile/src/components/MobileHomeQuickActions.test.ts index ba2e0ba750f..e71e1d8ff83 100644 --- a/mobile/src/components/MobileHomeQuickActions.test.ts +++ b/mobile/src/components/MobileHomeQuickActions.test.ts @@ -1,6 +1,6 @@ import { createElement } from 'react' import { act, create, type ReactTestRenderer } from 'react-test-renderer' -import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { afterEach, describe, expect, it, vi } from 'vitest' import type { HostProfile } from '../transport/types' import { MobileHomeQuickActions } from './MobileHomeQuickActions' @@ -37,10 +37,6 @@ function host(id: string, name: string, endpoint: string): HostProfile { describe('MobileHomeQuickActions', () => { let renderer: ReactTestRenderer | null = null - beforeEach(() => { - globalThis.IS_REACT_ACT_ENVIRONMENT = true - }) - afterEach(() => { act(() => renderer?.unmount()) renderer = null diff --git a/mobile/src/components/MobileHostCard.test.ts b/mobile/src/components/MobileHostCard.test.ts index f7c494cfdbf..f4033e67112 100644 --- a/mobile/src/components/MobileHostCard.test.ts +++ b/mobile/src/components/MobileHostCard.test.ts @@ -1,6 +1,6 @@ import { createElement } from 'react' import { act, create, type ReactTestRenderer } from 'react-test-renderer' -import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { afterEach, describe, expect, it, vi } from 'vitest' import { MobileHostCard } from './MobileHostCard' vi.mock('react-native', () => ({ @@ -30,10 +30,6 @@ function suppressRendererDeprecation() { describe('MobileHostCard', () => { let renderer: ReactTestRenderer | null = null - beforeEach(() => { - globalThis.IS_REACT_ACT_ENVIRONMENT = true - }) - afterEach(() => { act(() => renderer?.unmount()) renderer = null diff --git a/mobile/src/components/MobileHostCard.test.tsx b/mobile/src/components/MobileHostCard.test.tsx index 66e978a14ce..a921571555c 100644 --- a/mobile/src/components/MobileHostCard.test.tsx +++ b/mobile/src/components/MobileHostCard.test.tsx @@ -1,6 +1,6 @@ import { createElement } from 'react' import { act, create, type ReactTestRenderer } from 'react-test-renderer' -import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { afterEach, describe, expect, it, vi } from 'vitest' import type { ConnectionVerdict } from '../transport/connection-health' import type { MobileConnectionPath } from '../transport/stable-logical-rpc-client' import type { ConnectionState, HostCredentialStatus, HostProfile } from '../transport/types' @@ -39,10 +39,6 @@ const loaded: HostWorktreeInfo = { describe('MobileHostCard', () => { let renderer: ReactTestRenderer | null = null - beforeEach(() => { - globalThis.IS_REACT_ACT_ENVIRONMENT = true - }) - afterEach(() => { act(() => renderer?.unmount()) renderer = null diff --git a/mobile/src/components/MobileMarkdown.file-links.test.ts b/mobile/src/components/MobileMarkdown.file-links.test.ts index a8e381865d8..962538a8842 100644 --- a/mobile/src/components/MobileMarkdown.file-links.test.ts +++ b/mobile/src/components/MobileMarkdown.file-links.test.ts @@ -38,7 +38,6 @@ describe('MobileMarkdown file links', () => { const onOpenFile = vi.fn() beforeEach(() => { - globalThis.IS_REACT_ACT_ENVIRONMENT = true onOpenFile.mockClear() openURL.mockClear() }) diff --git a/mobile/src/components/NewWorktreeModal.test.tsx b/mobile/src/components/NewWorktreeModal.test.tsx index 3fb76dd3122..4d8e95bec89 100644 --- a/mobile/src/components/NewWorktreeModal.test.tsx +++ b/mobile/src/components/NewWorktreeModal.test.tsx @@ -58,7 +58,6 @@ describe('NewWorktreeModal repo list', () => { let renderer: ReactTestRenderer | null = null beforeEach(() => { - globalThis.IS_REACT_ACT_ENVIRONMENT = true setCachedRepos('host-1', repos) }) diff --git a/mobile/src/components/PickerModal.accessibility.test.ts b/mobile/src/components/PickerModal.accessibility.test.ts index 4508f227554..46610a6e631 100644 --- a/mobile/src/components/PickerModal.accessibility.test.ts +++ b/mobile/src/components/PickerModal.accessibility.test.ts @@ -24,7 +24,6 @@ describe('PickerModal accessibility', () => { let renderer: ReactTestRenderer | null = null beforeEach(() => { - globalThis.IS_REACT_ACT_ENVIRONMENT = true 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])) diff --git a/mobile/src/components/WorktreeAgentList.test.tsx b/mobile/src/components/WorktreeAgentList.test.tsx index 036f5d35996..aef5161cb4c 100644 --- a/mobile/src/components/WorktreeAgentList.test.tsx +++ b/mobile/src/components/WorktreeAgentList.test.tsx @@ -1,6 +1,6 @@ import { createElement } from 'react' import { act, create, type ReactTestRenderer } from 'react-test-renderer' -import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { afterEach, describe, expect, it, vi } from 'vitest' import type { RuntimeWorktreeAgentRow } from '../../../src/shared/runtime-types' import { WorktreeAgentList } from './WorktreeAgentList' @@ -39,10 +39,6 @@ function agent(paneKey: string, parentPaneKey: string | null = null): RuntimeWor describe('WorktreeAgentList', () => { let renderer: ReactTestRenderer | null = null - beforeEach(() => { - globalThis.IS_REACT_ACT_ENVIRONMENT = true - }) - afterEach(() => { act(() => renderer?.unmount()) renderer = null diff --git a/mobile/src/components/WorktreeListRow.test.ts b/mobile/src/components/WorktreeListRow.test.ts index 9fe0aa11b02..d0825412636 100644 --- a/mobile/src/components/WorktreeListRow.test.ts +++ b/mobile/src/components/WorktreeListRow.test.ts @@ -127,7 +127,6 @@ describe('memoized worktree rows', () => { let renderer: ReactTestRenderer | null = null beforeEach(() => { - ;(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true agentSpinnerRender.mockClear() agentStateDotRender.mockClear() }) diff --git a/mobile/src/components/bottom-drawer-close-lifecycle.test.ts b/mobile/src/components/bottom-drawer-close-lifecycle.test.ts index b4362412db7..94eb8a8495c 100644 --- a/mobile/src/components/bottom-drawer-close-lifecycle.test.ts +++ b/mobile/src/components/bottom-drawer-close-lifecycle.test.ts @@ -51,14 +51,12 @@ function mountedDrawer(renderer: ReactTestRenderer) { describe('BottomDrawer close lifecycle', () => { beforeEach(() => { - globalThis.IS_REACT_ACT_ENVIRONMENT = true const originalConsoleError = console.error vi.spyOn(console, 'error').mockImplementation((...args) => { const message = args[0] if ( typeof message === 'string' && - (message.includes('react-test-renderer is deprecated') || - message.includes('The current testing environment is not configured to support act')) + message.includes('The current testing environment is not configured to support act') ) { return } diff --git a/mobile/src/components/mobile-agent-icon-gradient.test.ts b/mobile/src/components/mobile-agent-icon-gradient.test.ts index 8e2781edfb2..c2221b62771 100644 --- a/mobile/src/components/mobile-agent-icon-gradient.test.ts +++ b/mobile/src/components/mobile-agent-icon-gradient.test.ts @@ -1,6 +1,6 @@ import { createElement } from 'react' import { act, create, type ReactTestRenderer } from 'react-test-renderer' -import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { afterEach, describe, expect, it, vi } from 'vitest' import { MobileAgentIcon } from './MobileAgentIcon' vi.mock('react-native', () => ({ @@ -35,10 +35,6 @@ vi.mock('./AgentIcons', () => ({ describe('MobileAgentIcon OMP gradient', () => { let renderer: ReactTestRenderer | null = null - beforeEach(() => { - globalThis.IS_REACT_ACT_ENVIRONMENT = true - }) - afterEach(() => { act(() => renderer?.unmount()) renderer = null diff --git a/mobile/src/components/pr-sidebar/PRCommentsSection.test.ts b/mobile/src/components/pr-sidebar/PRCommentsSection.test.ts index d174f455c67..08fc02c357b 100644 --- a/mobile/src/components/pr-sidebar/PRCommentsSection.test.ts +++ b/mobile/src/components/pr-sidebar/PRCommentsSection.test.ts @@ -32,18 +32,6 @@ vi.mock('./pr-comments-styles', () => ({ prCommentsStyles: {} })) vi.mock('./mobile-pr-sidebar-styles', () => ({ mobilePrSidebarStyles: {} })) vi.mock('../../theme/mobile-theme', () => ({ colors: { textSecondary: '#999' } })) -function suppressReactTestRendererDeprecationWarning(): () => void { - const originalConsoleError = console.error - const consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation((...args) => { - const firstArg = args[0] - if (typeof firstArg === 'string' && firstArg.includes('react-test-renderer is deprecated')) { - return - } - originalConsoleError(...args) - }) - return () => consoleErrorSpy.mockRestore() -} - function comment(id: number): PRComment { return { id, @@ -65,14 +53,9 @@ function detailsWithComments(count: number): GitHubWorkItemDetails { async function renderComments(details: GitHubWorkItemDetails): Promise { let renderer: ReactTestRenderer | null = null - const restoreConsoleError = suppressReactTestRendererDeprecationWarning() - try { - await act(async () => { - renderer = create(createElement(PRCommentsSection, { details, prState: 'open' })) - }) - } finally { - restoreConsoleError() - } + await act(async () => { + renderer = create(createElement(PRCommentsSection, { details, prState: 'open' })) + }) if (!renderer) { throw new Error('PRCommentsSection did not render') } @@ -107,7 +90,6 @@ describe('PRCommentsSection', () => { afterEach(() => { renderer?.unmount() renderer = null - vi.restoreAllMocks() }) it('resets pagination only when the user chooses a different audience filter', async () => { diff --git a/mobile/src/files/MobileFileExplorerPanel.test.ts b/mobile/src/files/MobileFileExplorerPanel.test.ts index 0d003e9f456..ff407ff21f6 100644 --- a/mobile/src/files/MobileFileExplorerPanel.test.ts +++ b/mobile/src/files/MobileFileExplorerPanel.test.ts @@ -83,18 +83,6 @@ vi.mock('../transport/client-context', () => ({ }) })) -function suppressReactTestRendererDeprecationWarning(): () => void { - const originalConsoleError = console.error - const consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation((...args) => { - const firstArg = args[0] - if (typeof firstArg === 'string' && firstArg.includes('react-test-renderer is deprecated')) { - return - } - originalConsoleError(...args) - }) - return () => consoleErrorSpy.mockRestore() -} - function entry(name: string, isDirectory = false): MobileDirEntry { return { name, isDirectory } } @@ -113,21 +101,16 @@ function createMockClient(entriesByPath: Record): Mock async function renderExplorer(): Promise { let renderer: ReactTestRenderer | null = null - const restoreConsoleError = suppressReactTestRendererDeprecationWarning() - try { - await act(async () => { - renderer = create( - createElement(MobileFileExplorerPanel, { - hostId: 'host-a', - worktreeId: 'worktree-a', - name: 'Example Worktree', - embedded: true - }) - ) - }) - } finally { - restoreConsoleError() - } + await act(async () => { + renderer = create( + createElement(MobileFileExplorerPanel, { + hostId: 'host-a', + worktreeId: 'worktree-a', + name: 'Example Worktree', + embedded: true + }) + ) + }) if (!renderer) { throw new Error('MobileFileExplorerPanel did not render') } diff --git a/mobile/src/files/MobileFileMarkdownPreview.test.ts b/mobile/src/files/MobileFileMarkdownPreview.test.ts index 2afdb0b2ce9..8ac3770eb35 100644 --- a/mobile/src/files/MobileFileMarkdownPreview.test.ts +++ b/mobile/src/files/MobileFileMarkdownPreview.test.ts @@ -33,28 +33,11 @@ vi.mock('./mobile-file-preview-styles', () => ({ type PreviewProps = Parameters[0] -function suppressReactTestRendererDeprecationWarning(): () => void { - const originalConsoleError = console.error - const consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation((...args) => { - const firstArg = args[0] - if (typeof firstArg === 'string' && firstArg.includes('react-test-renderer is deprecated')) { - return - } - originalConsoleError(...args) - }) - return () => consoleErrorSpy.mockRestore() -} - async function renderPreview(props: PreviewProps): Promise { let renderer: ReactTestRenderer | null = null - const restoreConsoleError = suppressReactTestRendererDeprecationWarning() - try { - await act(async () => { - renderer = create(createElement(MobileFileMarkdownPreview, props)) - }) - } finally { - restoreConsoleError() - } + await act(async () => { + renderer = create(createElement(MobileFileMarkdownPreview, props)) + }) if (!renderer) { throw new Error('MobileFileMarkdownPreview did not render') } diff --git a/mobile/src/hooks/use-now.test.ts b/mobile/src/hooks/use-now.test.ts index c3fe56a980b..14264b513ee 100644 --- a/mobile/src/hooks/use-now.test.ts +++ b/mobile/src/hooks/use-now.test.ts @@ -1,6 +1,6 @@ import { createElement } from 'react' import { act, create, type ReactTestRenderer } from 'react-test-renderer' -import { afterEach, beforeEach, describe, expect, it, vi, type MockInstance } from 'vitest' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' const appState = vi.hoisted(() => ({ current: 'active', @@ -25,7 +25,6 @@ import { useNow } from './use-now' describe('useNow', () => { let renderer: ReactTestRenderer | null = null let latest = 0 - let consoleSpy: MockInstance function Harness({ enabled = true }: { enabled?: boolean }): null { latest = useNow(1_000, enabled) @@ -42,18 +41,10 @@ describe('useNow', () => { beforeEach(() => { vi.useFakeTimers() vi.setSystemTime(1_000) - globalThis.IS_REACT_ACT_ENVIRONMENT = true appState.current = 'active' appState.listener = null appState.remove.mockClear() latest = 0 - const original = console.error - consoleSpy = vi.spyOn(console, 'error').mockImplementation((...args) => { - if (typeof args[0] === 'string' && args[0].includes('react-test-renderer is deprecated')) { - return - } - original(...args) - }) act(() => { renderer = create(createElement(Harness)) }) @@ -63,7 +54,6 @@ describe('useNow', () => { act(() => renderer?.unmount()) renderer = null vi.useRealTimers() - consoleSpy.mockRestore() }) it('ticks while active, pauses in the background, and refreshes immediately on resume', () => { diff --git a/mobile/src/host-edit-route-accessibility.test.ts b/mobile/src/host-edit-route-accessibility.test.ts index bb5ddf4faa6..d5fc20521a9 100644 --- a/mobile/src/host-edit-route-accessibility.test.ts +++ b/mobile/src/host-edit-route-accessibility.test.ts @@ -46,29 +46,12 @@ vi.mock('./transport/client-context', () => ({ usePrimeHosts: () => dependencies.primeHosts })) -function suppressReactTestRendererDeprecationWarning(): () => void { - const originalConsoleError = console.error - const consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation((...args) => { - const firstArg = args[0] - if (typeof firstArg === 'string' && firstArg.includes('react-test-renderer is deprecated')) { - return - } - originalConsoleError(...args) - }) - return () => consoleErrorSpy.mockRestore() -} - async function renderEditHostRoute(): Promise { let renderer: ReactTestRenderer | null = null - const restoreConsoleError = suppressReactTestRendererDeprecationWarning() - try { - await act(async () => { - renderer = create(createElement(EditHostScreen)) - await Promise.resolve() - }) - } finally { - restoreConsoleError() - } + await act(async () => { + renderer = create(createElement(EditHostScreen)) + await Promise.resolve() + }) if (!renderer) { throw new Error('Edit host route did not render') } @@ -77,7 +60,6 @@ async function renderEditHostRoute(): Promise { describe('edit host route accessibility', () => { beforeEach(() => { - globalThis.IS_REACT_ACT_ENVIRONMENT = true dependencies.loadHosts.mockReset().mockResolvedValue([ { id: 'host-1', diff --git a/mobile/src/host-edit-save-flow.test.ts b/mobile/src/host-edit-save-flow.test.ts index aa0069a7c69..a0a4561b2b2 100644 --- a/mobile/src/host-edit-save-flow.test.ts +++ b/mobile/src/host-edit-save-flow.test.ts @@ -56,29 +56,12 @@ const HOST_FIXTURE = { lastConnected: 1 } -function suppressReactTestRendererDeprecationWarning(): () => void { - const originalConsoleError = console.error - const consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation((...args) => { - const firstArg = args[0] - if (typeof firstArg === 'string' && firstArg.includes('react-test-renderer is deprecated')) { - return - } - originalConsoleError(...args) - }) - return () => consoleErrorSpy.mockRestore() -} - async function renderEditHostRoute(): Promise { let renderer: ReactTestRenderer | null = null - const restoreConsoleError = suppressReactTestRendererDeprecationWarning() - try { - await act(async () => { - renderer = create(createElement(EditHostScreen)) - await Promise.resolve() - }) - } finally { - restoreConsoleError() - } + await act(async () => { + renderer = create(createElement(EditHostScreen)) + await Promise.resolve() + }) if (!renderer) { throw new Error('Edit host route did not render') } @@ -135,7 +118,6 @@ function findText(renderer: ReactTestRenderer, match: string): boolean { describe('edit host handleSave', () => { beforeEach(() => { - globalThis.IS_REACT_ACT_ENVIRONMENT = true dependencies.hostId = 'host-1' dependencies.back.mockReset() dependencies.forceReconnectHost.mockReset().mockResolvedValue(undefined) @@ -285,7 +267,6 @@ describe('edit host handleSave', () => { describe('edit host load() error states', () => { beforeEach(() => { - globalThis.IS_REACT_ACT_ENVIRONMENT = true dependencies.hostId = 'host-1' dependencies.back.mockReset() dependencies.forceReconnectHost.mockReset().mockResolvedValue(undefined) diff --git a/mobile/src/onboarding/MobileOnboardingPage.test.ts b/mobile/src/onboarding/MobileOnboardingPage.test.ts index a99052c3ec9..d62e496f0bd 100644 --- a/mobile/src/onboarding/MobileOnboardingPage.test.ts +++ b/mobile/src/onboarding/MobileOnboardingPage.test.ts @@ -1,6 +1,6 @@ import { createElement } from 'react' import { act, create, type ReactTestRenderer } from 'react-test-renderer' -import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { afterEach, describe, expect, it, vi } from 'vitest' import { MobileOnboardingPage } from './MobileOnboardingPage' vi.mock('react-native', async () => { @@ -24,10 +24,6 @@ vi.mock('lucide-react-native', () => ({ describe('MobileOnboardingPage', () => { let renderer: ReactTestRenderer | null = null - beforeEach(() => { - globalThis.IS_REACT_ACT_ENVIRONMENT = true - }) - afterEach(() => { act(() => renderer?.unmount()) renderer = null diff --git a/mobile/src/onboarding/mobile-onboarding-screen.test.ts b/mobile/src/onboarding/mobile-onboarding-screen.test.ts index d571ce28a01..05dcc6b9ad8 100644 --- a/mobile/src/onboarding/mobile-onboarding-screen.test.ts +++ b/mobile/src/onboarding/mobile-onboarding-screen.test.ts @@ -56,7 +56,6 @@ describe('MobileOnboardingScreen', () => { let renderer: ReactTestRenderer | null = null beforeEach(() => { - globalThis.IS_REACT_ACT_ENVIRONMENT = true mocks.params = { hostId: 'paired-host', steps: 'session-view,notifications' } mocks.replace.mockReset() mocks.reducedMotionEnabled = false diff --git a/mobile/src/session/MobileNativeChatComposer.test.ts b/mobile/src/session/MobileNativeChatComposer.test.ts index 4ee001ca4af..eaf50ba08ab 100644 --- a/mobile/src/session/MobileNativeChatComposer.test.ts +++ b/mobile/src/session/MobileNativeChatComposer.test.ts @@ -1,6 +1,6 @@ import { createElement } from 'react' import { act, create, type ReactTestRenderer } from 'react-test-renderer' -import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { afterEach, describe, expect, it, vi } from 'vitest' import { radii, spacing } from '../theme/mobile-theme' import { MobileNativeChatComposer } from './MobileNativeChatComposer' @@ -43,24 +43,9 @@ vi.mock('../components/BottomDrawer', async () => { } }) -function suppressRendererWarning(): () => void { - const original = console.error - const spy = vi.spyOn(console, 'error').mockImplementation((...args) => { - if (typeof args[0] === 'string' && args[0].includes('react-test-renderer is deprecated')) { - return - } - original(...args) - }) - return () => spy.mockRestore() -} - describe('MobileNativeChatComposer', () => { let renderer: ReactTestRenderer | null = null - beforeEach(() => { - globalThis.IS_REACT_ACT_ENVIRONMENT = true - }) - afterEach(() => { act(() => renderer?.unmount()) renderer = null @@ -71,21 +56,16 @@ describe('MobileNativeChatComposer', () => { onChangeText: () => void, isAttaching = false ) { - const restore = suppressRendererWarning() - try { - await act(async () => { - renderer = create( - createElement(MobileNativeChatComposer, { - value: ' hello ', - onChangeText, - onSend, - isAttaching - }) - ) - }) - } finally { - restore() - } + await act(async () => { + renderer = create( + createElement(MobileNativeChatComposer, { + value: ' hello ', + onChangeText, + onSend, + isAttaching + }) + ) + }) } function sendButton(): { props: { onPress: () => Promise } } { @@ -130,20 +110,15 @@ describe('MobileNativeChatComposer', () => { it('preserves leading whitespace so prose is not turned into a slash command', async () => { const onSend = vi.fn().mockResolvedValue(true) - const restore = suppressRendererWarning() - try { - await act(async () => { - renderer = create( - createElement(MobileNativeChatComposer, { - value: ' /clear is prose ', - onChangeText: vi.fn(), - onSend - }) - ) - }) - } finally { - restore() - } + await act(async () => { + renderer = create( + createElement(MobileNativeChatComposer, { + value: ' /clear is prose ', + onChangeText: vi.fn(), + onSend + }) + ) + }) await act(async () => sendButton().props.onPress()) expect(onSend).toHaveBeenCalledWith(' /clear is prose') }) @@ -181,67 +156,57 @@ describe('MobileNativeChatComposer', () => { invokeAction: vi.fn(), recordCommand: vi.fn() } - const restore = suppressRendererWarning() - try { - await act(async () => { - renderer = create( - createElement(MobileNativeChatComposer, { - value: 'run the tests', - onChangeText: vi.fn(), - onSend, - sessionOptions: { isWorking: false, controller } - }) - ) - }) - const modelPill = (): { props: { accessibilityState: { disabled: boolean } } } => - renderer!.root.find( - (node) => node.type === 'Pressable' && node.props.accessibilityLabel === 'Model, Model' - ) as { props: { accessibilityState: { disabled: boolean } } } - expect(modelPill().props.accessibilityState).toMatchObject({ disabled: false }) - // Start the send but don't await it — it stays in flight on purpose. - let pressed!: Promise - await act(async () => { - pressed = sendButton().props.onPress() - await Promise.resolve() - }) - expect(onSend).toHaveBeenCalled() - expect(modelPill().props.accessibilityState).toMatchObject({ disabled: true }) - await act(async () => { - releaseSend?.(true) - await pressed - }) - expect(modelPill().props.accessibilityState).toMatchObject({ disabled: false }) - } finally { - restore() - } + await act(async () => { + renderer = create( + createElement(MobileNativeChatComposer, { + value: 'run the tests', + onChangeText: vi.fn(), + onSend, + sessionOptions: { isWorking: false, controller } + }) + ) + }) + const modelPill = (): { props: { accessibilityState: { disabled: boolean } } } => + renderer!.root.find( + (node) => node.type === 'Pressable' && node.props.accessibilityLabel === 'Model, Model' + ) as { props: { accessibilityState: { disabled: boolean } } } + expect(modelPill().props.accessibilityState).toMatchObject({ disabled: false }) + // Start the send but don't await it — it stays in flight on purpose. + let pressed!: Promise + await act(async () => { + pressed = sendButton().props.onPress() + await Promise.resolve() + }) + expect(onSend).toHaveBeenCalled() + expect(modelPill().props.accessibilityState).toMatchObject({ disabled: true }) + await act(async () => { + releaseSend?.(true) + await pressed + }) + expect(modelPill().props.accessibilityState).toMatchObject({ disabled: false }) }) it('blocks composer submission while a session-option command is pending', async () => { const onSend = vi.fn().mockResolvedValue(true) - const restore = suppressRendererWarning() - try { - await act(async () => { - renderer = create( - createElement(MobileNativeChatComposer, { - value: 'hello', - onChangeText: vi.fn(), - onSend, - sessionOptions: { - isWorking: false, - controller: { - snapshot: [], - pendingId: 'model', - setOption: vi.fn(), - invokeAction: vi.fn(), - recordCommand: vi.fn() - } + await act(async () => { + renderer = create( + createElement(MobileNativeChatComposer, { + value: 'hello', + onChangeText: vi.fn(), + onSend, + sessionOptions: { + isWorking: false, + controller: { + snapshot: [], + pendingId: 'model', + setOption: vi.fn(), + invokeAction: vi.fn(), + recordCommand: vi.fn() } - }) - ) - }) - } finally { - restore() - } + } + }) + ) + }) expect(sendButton().props).toMatchObject({ disabled: true }) await act(async () => sendButton().props.onPress()) expect(onSend).not.toHaveBeenCalled() @@ -268,21 +233,16 @@ describe('MobileNativeChatComposer', () => { }) it('keeps the text input editable while the send is locked', async () => { - const restore = suppressRendererWarning() - try { - await act(async () => { - renderer = create( - createElement(MobileNativeChatComposer, { - value: 'half-typed', - onChangeText: vi.fn(), - onSend: vi.fn().mockResolvedValue(true), - disabled: true - }) - ) - }) - } finally { - restore() - } + await act(async () => { + renderer = create( + createElement(MobileNativeChatComposer, { + value: 'half-typed', + onChangeText: vi.fn(), + onSend: vi.fn().mockResolvedValue(true), + disabled: true + }) + ) + }) // Revoking `editable` on a focused field resigns first responder on iOS and // yanks the keyboard mid-typing (#10681) — the lock may only gate sending. const input = renderer!.root.find((node) => node.type === 'TextInput') as { @@ -294,25 +254,20 @@ describe('MobileNativeChatComposer', () => { it('renders a removable thumbnail for each pending image attachment', async () => { const onRemoveAttachment = vi.fn() - const restore = suppressRendererWarning() - try { - await act(async () => { - renderer = create( - createElement(MobileNativeChatComposer, { - value: '', - onChangeText: vi.fn(), - onSend: vi.fn().mockResolvedValue(true), - attachments: [ - { id: 'img-1', path: '/tmp/a.png', previewUri: 'file:///a.png' }, - { id: 'img-2', path: '/tmp/b.png', previewUri: 'file:///b.png' } - ], - onRemoveAttachment - }) - ) - }) - } finally { - restore() - } + await act(async () => { + renderer = create( + createElement(MobileNativeChatComposer, { + value: '', + onChangeText: vi.fn(), + onSend: vi.fn().mockResolvedValue(true), + attachments: [ + { id: 'img-1', path: '/tmp/a.png', previewUri: 'file:///a.png' }, + { id: 'img-2', path: '/tmp/b.png', previewUri: 'file:///b.png' } + ], + onRemoveAttachment + }) + ) + }) const thumbs = renderer!.root.findAll((node) => node.type === 'Image') as Array<{ props: { source: { uri: string } } }> @@ -327,42 +282,32 @@ describe('MobileNativeChatComposer', () => { it('enables send with an attached image even when the text is empty', async () => { const onSend = vi.fn().mockResolvedValue(true) - const restore = suppressRendererWarning() - try { - await act(async () => { - renderer = create( - createElement(MobileNativeChatComposer, { - value: '', - onChangeText: vi.fn(), - onSend, - attachments: [{ id: 'img-1', path: '/tmp/a.png', previewUri: 'file:///a.png' }] - }) - ) - }) - } finally { - restore() - } + await act(async () => { + renderer = create( + createElement(MobileNativeChatComposer, { + value: '', + onChangeText: vi.fn(), + onSend, + attachments: [{ id: 'img-1', path: '/tmp/a.png', previewUri: 'file:///a.png' }] + }) + ) + }) expect(sendButton().props).toMatchObject({ disabled: false }) await act(async () => sendButton().props.onPress()) expect(onSend).toHaveBeenCalledWith('') }) it('moves the caret to the insert point after an autocomplete pick, then releases control', async () => { - const restore = suppressRendererWarning() - try { - await act(async () => { - renderer = create( - createElement(MobileNativeChatComposer, { - value: '/c', - onChangeText: vi.fn(), - onSend: vi.fn().mockResolvedValue(true), - agent: 'claude' - }) - ) - }) - } finally { - restore() - } + await act(async () => { + renderer = create( + createElement(MobileNativeChatComposer, { + value: '/c', + onChangeText: vi.fn(), + onSend: vi.fn().mockResolvedValue(true), + agent: 'claude' + }) + ) + }) const input = () => renderer!.root.find((node) => node.type === 'TextInput') as { props: { @@ -390,21 +335,16 @@ describe('MobileNativeChatComposer', () => { }) it('serves the active agent’s shared command catalog with descriptions', async () => { - const restore = suppressRendererWarning() - try { - await act(async () => { - renderer = create( - createElement(MobileNativeChatComposer, { - value: '/', - onChangeText: vi.fn(), - onSend: vi.fn().mockResolvedValue(true), - agent: 'codex' - }) - ) - }) - } finally { - restore() - } + await act(async () => { + renderer = create( + createElement(MobileNativeChatComposer, { + value: '/', + onChangeText: vi.fn(), + onSend: vi.fn().mockResolvedValue(true), + agent: 'codex' + }) + ) + }) const input = renderer!.root.find((node) => node.type === 'TextInput') as { props: { onSelectionChange: (e: { nativeEvent: { selection: { end: number } } }) => void } } @@ -428,45 +368,40 @@ describe('MobileNativeChatComposer', () => { (node) => node.type === 'Pressable' && node.props.accessibilityLabel === 'Dictate' ) as { props: { onPress?: unknown; onPressIn?: unknown; onPressOut?: unknown } } - const restore = suppressRendererWarning() - try { - await act(async () => { - renderer = create( - createElement(MobileNativeChatComposer, { - value: '', - onChangeText: vi.fn(), - onSend: vi.fn().mockResolvedValue(true), - onMicPress, - dictationMode: 'hold', - onMicPressIn, - onMicPressOut - }) - ) - }) - // Hold mode is walkie-talkie: press-in/out drive dictation, tap is inert. - expect(mic().props.onPress).toBeUndefined() - expect(mic().props.onPressIn).toBe(onMicPressIn) - expect(mic().props.onPressOut).toBe(onMicPressOut) + await act(async () => { + renderer = create( + createElement(MobileNativeChatComposer, { + value: '', + onChangeText: vi.fn(), + onSend: vi.fn().mockResolvedValue(true), + onMicPress, + dictationMode: 'hold', + onMicPressIn, + onMicPressOut + }) + ) + }) + // Hold mode is walkie-talkie: press-in/out drive dictation, tap is inert. + expect(mic().props.onPress).toBeUndefined() + expect(mic().props.onPressIn).toBe(onMicPressIn) + expect(mic().props.onPressOut).toBe(onMicPressOut) - await act(async () => { - renderer!.update( - createElement(MobileNativeChatComposer, { - value: '', - onChangeText: vi.fn(), - onSend: vi.fn().mockResolvedValue(true), - onMicPress, - dictationMode: 'toggle', - onMicPressIn, - onMicPressOut - }) - ) - }) - // Toggle mode: tap drives dictation, press-in/out inert. - expect(mic().props.onPress).toBe(onMicPress) - expect(mic().props.onPressIn).toBeUndefined() - expect(mic().props.onPressOut).toBeUndefined() - } finally { - restore() - } + await act(async () => { + renderer!.update( + createElement(MobileNativeChatComposer, { + value: '', + onChangeText: vi.fn(), + onSend: vi.fn().mockResolvedValue(true), + onMicPress, + dictationMode: 'toggle', + onMicPressIn, + onMicPressOut + }) + ) + }) + // Toggle mode: tap drives dictation, press-in/out inert. + expect(mic().props.onPress).toBe(onMicPress) + expect(mic().props.onPressIn).toBeUndefined() + expect(mic().props.onPressOut).toBeUndefined() }) }) diff --git a/mobile/src/session/MobileNativeChatMessage.test.ts b/mobile/src/session/MobileNativeChatMessage.test.ts index 9d33db069ff..bb2335c7054 100644 --- a/mobile/src/session/MobileNativeChatMessage.test.ts +++ b/mobile/src/session/MobileNativeChatMessage.test.ts @@ -1,6 +1,6 @@ import { createElement } from 'react' import { act, create, type ReactTestInstance, type ReactTestRenderer } from 'react-test-renderer' -import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { afterEach, describe, expect, it, vi } from 'vitest' import type { NativeChatMessage } from '../../../src/shared/native-chat-types' import { MAX_TOOL_DETAIL_LENGTH } from './mobile-native-chat-tool-summary' @@ -38,9 +38,6 @@ function toolMessage(blocks: NativeChatMessage['blocks']): NativeChatMessage { describe('MobileNativeChatMessage', () => { let renderer: ReactTestRenderer | null = null - beforeEach(() => { - globalThis.IS_REACT_ACT_ENVIRONMENT = true - }) afterEach(() => { act(() => renderer?.unmount()) renderer = null @@ -50,20 +47,9 @@ describe('MobileNativeChatMessage', () => { message: NativeChatMessage, props: { toolsExpanded?: boolean } = {} ): ReactTestRenderer { - const original = console.error - const spy = vi.spyOn(console, 'error').mockImplementation((...a) => { - if (typeof a[0] === 'string' && a[0].includes('react-test-renderer is deprecated')) { - return - } - original(...a) + act(() => { + renderer = create(createElement(MobileNativeChatMessage, { message, ...props })) }) - try { - act(() => { - renderer = create(createElement(MobileNativeChatMessage, { message, ...props })) - }) - } finally { - spy.mockRestore() - } return renderer! } diff --git a/mobile/src/session/MobileNativeChatOverlay.test.ts b/mobile/src/session/MobileNativeChatOverlay.test.ts index c8e62c146cb..b43a7c79f3b 100644 --- a/mobile/src/session/MobileNativeChatOverlay.test.ts +++ b/mobile/src/session/MobileNativeChatOverlay.test.ts @@ -1,6 +1,6 @@ import { createElement } from 'react' import { act, create, type ReactTestRenderer } from 'react-test-renderer' -import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { afterEach, describe, expect, it, vi } from 'vitest' import type { NativeChatMessage } from '../../../src/shared/native-chat-types' import { MobileNativeChatOverlay } from './MobileNativeChatOverlay' import type { MobileNativeChatController } from './use-mobile-native-chat-controller' @@ -16,17 +16,6 @@ function assistantTurn(id: string, text: string): NativeChatMessage { return { id, role: 'assistant', blocks: [{ type: 'text', text }], timestamp: 0, source: 'hook' } } -function suppressRendererWarning(): () => void { - const original = console.error - const spy = vi.spyOn(console, 'error').mockImplementation((...args) => { - if (typeof args[0] === 'string' && args[0].includes('react-test-renderer is deprecated')) { - return - } - original(...args) - }) - return () => spy.mockRestore() -} - /** One render of the route: chat visible or not, the transcript it currently * holds, and the agent-status stream behind it. */ type Tick = { @@ -69,24 +58,15 @@ function overlayElement(tick: Tick): ReturnType { describe('MobileNativeChatOverlay streaming gate', () => { let renderer: ReactTestRenderer | null = null - beforeEach(() => { - globalThis.IS_REACT_ACT_ENVIRONMENT = true - }) - afterEach(() => { act(() => renderer?.unmount()) renderer = null }) async function render(tick: Tick): Promise { - const restore = suppressRendererWarning() - try { - await act(async () => { - renderer = create(overlayElement(tick)) - }) - } finally { - restore() - } + await act(async () => { + renderer = create(overlayElement(tick)) + }) } async function update(tick: Tick): Promise { diff --git a/mobile/src/session/MobileNativeChatPermission.test.ts b/mobile/src/session/MobileNativeChatPermission.test.ts index 8e0223f19ec..b39188b381f 100644 --- a/mobile/src/session/MobileNativeChatPermission.test.ts +++ b/mobile/src/session/MobileNativeChatPermission.test.ts @@ -1,6 +1,6 @@ import { createElement } from 'react' import { act, create, type ReactTestRenderer } from 'react-test-renderer' -import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { afterEach, describe, expect, it, vi } from 'vitest' import { MobileNativeChatPermission } from './MobileNativeChatPermission' vi.mock('react-native', () => ({ @@ -15,10 +15,6 @@ vi.mock('lucide-react-native', () => ({ ShieldQuestion: 'ShieldQuestion' })) describe('MobileNativeChatPermission', () => { let renderer: ReactTestRenderer | null = null - beforeEach(() => { - globalThis.IS_REACT_ACT_ENVIRONMENT = true - }) - afterEach(() => { act(() => renderer?.unmount()) renderer = null @@ -28,25 +24,14 @@ describe('MobileNativeChatPermission', () => { let resolveResponse: (accepted: boolean) => void = () => {} const response = new Promise((resolve) => (resolveResponse = resolve)) const onRespond = vi.fn(() => response) - const original = console.error - const consoleSpy = vi.spyOn(console, 'error').mockImplementation((...args) => { - if (typeof args[0] === 'string' && args[0].includes('react-test-renderer is deprecated')) { - return - } - original(...args) + await act(async () => { + renderer = create( + createElement(MobileNativeChatPermission, { + permission: { title: 'Approve?', options: [{ label: 'Allow', send: '1' }] }, + onRespond + }) + ) }) - try { - await act(async () => { - renderer = create( - createElement(MobileNativeChatPermission, { - permission: { title: 'Approve?', options: [{ label: 'Allow', send: '1' }] }, - onRespond - }) - ) - }) - } finally { - consoleSpy.mockRestore() - } const button = renderer.root.findByType('Pressable') act(() => { diff --git a/mobile/src/session/MobileNativeChatSessionOptionPickers.test.ts b/mobile/src/session/MobileNativeChatSessionOptionPickers.test.ts index f5af434fb92..4430c60115a 100644 --- a/mobile/src/session/MobileNativeChatSessionOptionPickers.test.ts +++ b/mobile/src/session/MobileNativeChatSessionOptionPickers.test.ts @@ -149,7 +149,6 @@ describe('MobileNativeChatSessionOptionPickers', () => { } beforeEach(() => { - globalThis.IS_REACT_ACT_ENVIRONMENT = true setOption.mockReset() setOption.mockResolvedValue(true) invokeAction.mockReset() diff --git a/mobile/src/session/MobileNativeChatView.test.ts b/mobile/src/session/MobileNativeChatView.test.ts index 21a5a7c4aa1..fb4072c4f2d 100644 --- a/mobile/src/session/MobileNativeChatView.test.ts +++ b/mobile/src/session/MobileNativeChatView.test.ts @@ -1,6 +1,6 @@ import { createElement } from 'react' import { act, create, type ReactTestInstance, type ReactTestRenderer } from 'react-test-renderer' -import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { afterEach, describe, expect, it, vi } from 'vitest' import type { NativeChatMessage } from '../../../src/shared/native-chat-types' import { MobileNativeChatView } from './MobileNativeChatView' @@ -73,17 +73,6 @@ type Overrides = { onSend?: (text: string) => Promise } -function suppressRendererWarning(): () => void { - const original = console.error - const spy = vi.spyOn(console, 'error').mockImplementation((...args) => { - if (typeof args[0] === 'string' && args[0].includes('react-test-renderer is deprecated')) { - return - } - original(...args) - }) - return () => spy.mockRestore() -} - function assistantTurn(id: string, text: string): NativeChatMessage { return { id, role: 'assistant', blocks: [{ type: 'text', text }], timestamp: 0, source: 'hook' } } @@ -105,24 +94,15 @@ function chatViewElement(overrides: Overrides): ReturnType describe('MobileNativeChatView', () => { let renderer: ReactTestRenderer | null = null - beforeEach(() => { - globalThis.IS_REACT_ACT_ENVIRONMENT = true - }) - afterEach(() => { act(() => renderer?.unmount()) renderer = null }) async function render(overrides: Overrides = {}): Promise { - const restore = suppressRendererWarning() - try { - await act(async () => { - renderer = create(chatViewElement(overrides)) - }) - } finally { - restore() - } + await act(async () => { + renderer = create(chatViewElement(overrides)) + }) } async function update(overrides: Overrides = {}): Promise { diff --git a/mobile/src/session/QuickCommandsList.test.ts b/mobile/src/session/QuickCommandsList.test.ts index eb2e2c787c0..a396932acb6 100644 --- a/mobile/src/session/QuickCommandsList.test.ts +++ b/mobile/src/session/QuickCommandsList.test.ts @@ -29,9 +29,7 @@ vi.mock('../components/MobileAgentIcon', () => ({ MobileAgentIcon: 'MobileAgentI describe('QuickCommandsList search', () => { let renderer: ReactTestRenderer | null = null - beforeEach(() => { - globalThis.IS_REACT_ACT_ENVIRONMENT = true - }) + beforeEach(() => {}) afterEach(() => { act(() => renderer?.unmount()) diff --git a/mobile/src/session/QuickCommandsSheet.test.ts b/mobile/src/session/QuickCommandsSheet.test.ts index 654882cd2ad..9951742b64f 100644 --- a/mobile/src/session/QuickCommandsSheet.test.ts +++ b/mobile/src/session/QuickCommandsSheet.test.ts @@ -1,6 +1,6 @@ import { createElement, type ReactNode } from 'react' import { act, create, type ReactTestRenderer } from 'react-test-renderer' -import { afterEach, beforeEach, describe, expect, it, vi, type MockInstance } from 'vitest' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import type { TerminalQuickCommand } from '../../../src/shared/types' import type { RpcClient } from '../transport/rpc-client' import { MAX_QUICK_COMMANDS } from '../terminal/quick-commands' @@ -65,26 +65,15 @@ function deferred() { describe('QuickCommandsSheet', () => { let renderer: ReactTestRenderer | null = null - let consoleSpy: MockInstance - beforeEach(() => { - globalThis.IS_REACT_ACT_ENVIRONMENT = true mocks.alert.mockReset() mocks.commands = [] mocks.persist.mockReset() - const originalConsoleError = console.error - consoleSpy = vi.spyOn(console, 'error').mockImplementation((...args) => { - if (typeof args[0] === 'string' && args[0].includes('react-test-renderer is deprecated')) { - return - } - originalConsoleError(...args) - }) }) afterEach(() => { act(() => renderer?.unmount()) renderer = null - consoleSpy.mockRestore() }) it('keeps the sheet open when a launch is rejected', async () => { diff --git a/mobile/src/session/mobile-native-chat-permission-send.test.ts b/mobile/src/session/mobile-native-chat-permission-send.test.ts index 253f8c8b85a..f74289d97ab 100644 --- a/mobile/src/session/mobile-native-chat-permission-send.test.ts +++ b/mobile/src/session/mobile-native-chat-permission-send.test.ts @@ -67,7 +67,6 @@ describe('useMobileNativeChatPermissionSend', () => { let respond: ((text: string) => Promise) | null = null beforeEach(() => { - globalThis.IS_REACT_ACT_ENVIRONMENT = true resetMobileNativeChatStaleInputForTests() resetMobileNativeChatTerminalWritesForTests() }) diff --git a/mobile/src/session/use-initial-session-terminal-autocreate.test.ts b/mobile/src/session/use-initial-session-terminal-autocreate.test.ts index 46a7bb859fa..5e4c8d425a4 100644 --- a/mobile/src/session/use-initial-session-terminal-autocreate.test.ts +++ b/mobile/src/session/use-initial-session-terminal-autocreate.test.ts @@ -52,7 +52,6 @@ describe('useInitialSessionTerminalAutoCreate', () => { } beforeEach(() => { - globalThis.IS_REACT_ACT_ENVIRONMENT = true stateRef = { current: { autoCreatedForWorktree: null, sawSessionTabs: false } } diff --git a/mobile/src/session/use-live-worktree-name.test.ts b/mobile/src/session/use-live-worktree-name.test.ts index 45e64205d1d..ad15bda3cb7 100644 --- a/mobile/src/session/use-live-worktree-name.test.ts +++ b/mobile/src/session/use-live-worktree-name.test.ts @@ -13,18 +13,6 @@ vi.mock('expo-router', async () => { } }) -function suppressReactTestRendererDeprecationWarning(): () => void { - const originalConsoleError = console.error - const consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation((...args) => { - const firstArg = args[0] - if (typeof firstArg === 'string' && firstArg.includes('react-test-renderer is deprecated')) { - return - } - originalConsoleError(...args) - }) - return () => consoleErrorSpy.mockRestore() -} - describe('useLiveWorktreeName request volume', () => { let renderer: ReactTestRenderer | null = null let eventListener: ((payload: unknown) => void) | null = null @@ -54,15 +42,10 @@ describe('useLiveWorktreeName request volume', () => { return null } - const restoreConsoleError = suppressReactTestRendererDeprecationWarning() - try { - await act(async () => { - renderer = create(createElement(Harness)) - await Promise.resolve() - }) - } finally { - restoreConsoleError() - } + await act(async () => { + renderer = create(createElement(Harness)) + await Promise.resolve() + }) } async function emitEvent(payload: unknown): Promise { @@ -75,7 +58,6 @@ describe('useLiveWorktreeName request volume', () => { beforeEach(() => { vi.useFakeTimers() - globalThis.IS_REACT_ACT_ENVIRONMENT = true eventListener = null sendRequest.mockClear().mockResolvedValue({ id: 'worktree-show', @@ -179,15 +161,10 @@ describe('useLiveWorktreeName request volume', () => { return null } - const restoreConsoleError = suppressReactTestRendererDeprecationWarning() - try { - await act(async () => { - renderer = create(createElement(FloatingHarness)) - await Promise.resolve() - }) - } finally { - restoreConsoleError() - } + await act(async () => { + renderer = create(createElement(FloatingHarness)) + await Promise.resolve() + }) await act(async () => { await vi.advanceTimersByTimeAsync(30_000) }) @@ -214,7 +191,6 @@ describe('useLiveWorktreeName request volume', () => { return null } - const restoreConsoleError = suppressReactTestRendererDeprecationWarning() try { await act(async () => { renderer = create( @@ -241,7 +217,6 @@ describe('useLiveWorktreeName request volume', () => { ) }) } finally { - restoreConsoleError() act(() => renderer?.unmount()) } @@ -263,15 +238,10 @@ describe('useLiveWorktreeName request volume', () => { return null } const mount = async (): Promise => { - const restoreConsoleError = suppressReactTestRendererDeprecationWarning() - try { - await act(async () => { - renderer = create(createElement(VerdictHarness)) - await Promise.resolve() - }) - } finally { - restoreConsoleError() - } + await act(async () => { + renderer = create(createElement(VerdictHarness)) + await Promise.resolve() + }) } await mount() diff --git a/mobile/src/session/use-missing-worktree-bounce.test.ts b/mobile/src/session/use-missing-worktree-bounce.test.ts index 209486bc8d1..df1bccc5678 100644 --- a/mobile/src/session/use-missing-worktree-bounce.test.ts +++ b/mobile/src/session/use-missing-worktree-bounce.test.ts @@ -27,7 +27,6 @@ describe('useMissingWorktreeBounce', () => { const bounce = vi.fn() beforeEach(() => { - globalThis.IS_REACT_ACT_ENVIRONMENT = true bounce.mockReset() renderer = null }) diff --git a/mobile/src/session/use-mobile-attachment-input-lease-gate.test.ts b/mobile/src/session/use-mobile-attachment-input-lease-gate.test.ts index a69d66accdd..9b11ad70c6d 100644 --- a/mobile/src/session/use-mobile-attachment-input-lease-gate.test.ts +++ b/mobile/src/session/use-mobile-attachment-input-lease-gate.test.ts @@ -7,16 +7,7 @@ type Gate = (targetHandle: string) => Promise describe('useMobileAttachmentInputLeaseGate', () => { let renderer: ReactTestRenderer | null = null - let errorSpy: ReturnType | null = null - beforeEach(() => { - const original = console.error - errorSpy = vi.spyOn(console, 'error').mockImplementation((...args: unknown[]) => { - if (typeof args[0] === 'string' && args[0].includes('react-test-renderer is deprecated')) { - return - } - original(...(args as Parameters)) - }) vi.useFakeTimers() }) @@ -24,7 +15,6 @@ describe('useMobileAttachmentInputLeaseGate', () => { act(() => renderer?.unmount()) renderer = null vi.useRealTimers() - errorSpy?.mockRestore() }) function renderGate(args: { diff --git a/mobile/src/session/use-mobile-default-session-view-preference.test.ts b/mobile/src/session/use-mobile-default-session-view-preference.test.ts index eb2624efd36..084164b6a40 100644 --- a/mobile/src/session/use-mobile-default-session-view-preference.test.ts +++ b/mobile/src/session/use-mobile-default-session-view-preference.test.ts @@ -30,7 +30,6 @@ describe('useMobileDefaultSessionViewPreference', () => { let preference: MobileDefaultSessionViewPreference | null = null beforeEach(() => { - globalThis.IS_REACT_ACT_ENVIRONMENT = true vi.mocked(loadDefaultSessionView).mockReset().mockResolvedValue('terminal') vi.mocked(saveDefaultSessionView).mockReset().mockResolvedValue(undefined) }) diff --git a/mobile/src/session/use-mobile-diff-review-controller.test.ts b/mobile/src/session/use-mobile-diff-review-controller.test.ts index 7c0a80455bc..5056b3cb546 100644 --- a/mobile/src/session/use-mobile-diff-review-controller.test.ts +++ b/mobile/src/session/use-mobile-diff-review-controller.test.ts @@ -63,7 +63,6 @@ describe('useMobileDiffReviewController', () => { } beforeEach(() => { - globalThis.IS_REACT_ACT_ENVIRONMENT = true loadSnapshot.mockReset() }) diff --git a/mobile/src/session/use-mobile-diff-review-diff-loading.test.ts b/mobile/src/session/use-mobile-diff-review-diff-loading.test.ts index 3245c228254..4abb7443188 100644 --- a/mobile/src/session/use-mobile-diff-review-diff-loading.test.ts +++ b/mobile/src/session/use-mobile-diff-review-diff-loading.test.ts @@ -57,7 +57,6 @@ describe('useMobileDiffReviewDiffLoading', () => { } beforeEach(() => { - globalThis.IS_REACT_ACT_ENVIRONMENT = true loadDiff.mockReset() }) diff --git a/mobile/src/session/use-mobile-diff-review-send-actions.test.ts b/mobile/src/session/use-mobile-diff-review-send-actions.test.ts index f7594fd6e57..1d06190948c 100644 --- a/mobile/src/session/use-mobile-diff-review-send-actions.test.ts +++ b/mobile/src/session/use-mobile-diff-review-send-actions.test.ts @@ -52,7 +52,6 @@ describe('useMobileDiffReviewSendActions', () => { let saveCommentsAndReviewState: ReturnType beforeEach(() => { - globalThis.IS_REACT_ACT_ENVIRONMENT = true resetMobileNativeChatStaleInputForTests() setActionError = vi.fn() setSendSheet = vi.fn() @@ -81,20 +80,9 @@ describe('useMobileDiffReviewSendActions', () => { async function mount(client: RpcClient): Promise { mountedClient = client - const original = console.error - const consoleSpy = vi.spyOn(console, 'error').mockImplementation((...args) => { - if (typeof args[0] === 'string' && args[0].includes('react-test-renderer is deprecated')) { - return - } - original(...args) + await act(async () => { + renderer = create(createElement(Harness)) }) - try { - await act(async () => { - renderer = create(createElement(Harness)) - }) - } finally { - consoleSpy.mockRestore() - } } it('heals a marked terminal BEFORE submitting the notes', async () => { diff --git a/mobile/src/session/use-mobile-file-tap-handlers.test.ts b/mobile/src/session/use-mobile-file-tap-handlers.test.ts index 6f48b2a771d..0cfa617f269 100644 --- a/mobile/src/session/use-mobile-file-tap-handlers.test.ts +++ b/mobile/src/session/use-mobile-file-tap-handlers.test.ts @@ -19,7 +19,6 @@ describe('useMobileFileTapHandlers', () => { let handlers: Handlers | null = null beforeEach(() => { - globalThis.IS_REACT_ACT_ENVIRONMENT = true push.mockClear() }) diff --git a/mobile/src/session/use-mobile-native-chat-answer-send.test.ts b/mobile/src/session/use-mobile-native-chat-answer-send.test.ts index b296ea4851b..64fa30aecc1 100644 --- a/mobile/src/session/use-mobile-native-chat-answer-send.test.ts +++ b/mobile/src/session/use-mobile-native-chat-answer-send.test.ts @@ -54,7 +54,6 @@ describe('useMobileNativeChatAnswerSend', () => { beforeEach(() => { onAccepted = vi.fn() vi.useFakeTimers() - globalThis.IS_REACT_ACT_ENVIRONMENT = true resetMobileNativeChatStaleInputForTests() resetMobileNativeChatTerminalWritesForTests() }) @@ -93,20 +92,9 @@ describe('useMobileNativeChatAnswerSend', () => { mountedClient = client mountedOnSendError = onSendError mountedAgent = agent - const original = console.error - const consoleSpy = vi.spyOn(console, 'error').mockImplementation((...args) => { - if (typeof args[0] === 'string' && args[0].includes('react-test-renderer is deprecated')) { - return - } - original(...args) + await act(async () => { + renderer = create(createElement(Harness, { enabled: true })) }) - try { - await act(async () => { - renderer = create(createElement(Harness, { enabled: true })) - }) - } finally { - consoleSpy.mockRestore() - } } async function setEnabled(enabled: boolean): Promise { diff --git a/mobile/src/session/use-mobile-native-chat-ask-dismiss.test.ts b/mobile/src/session/use-mobile-native-chat-ask-dismiss.test.ts index 460ffca54cd..de9d53fb094 100644 --- a/mobile/src/session/use-mobile-native-chat-ask-dismiss.test.ts +++ b/mobile/src/session/use-mobile-native-chat-ask-dismiss.test.ts @@ -1,6 +1,6 @@ import { createElement } from 'react' import { act, create, type ReactTestRenderer } from 'react-test-renderer' -import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { afterEach, beforeEach, describe, expect, it } from 'vitest' import type { AskPrompt } from './mobile-native-chat-ask' import { useMobileNativeChatAskDismiss } from './use-mobile-native-chat-ask-dismiss' @@ -10,7 +10,6 @@ describe('useMobileNativeChatAskDismiss', () => { let renders = 0 beforeEach(() => { - globalThis.IS_REACT_ACT_ENVIRONMENT = true renders = 0 }) @@ -45,20 +44,9 @@ describe('useMobileNativeChatAskDismiss', () => { } async function mount(props: Parameters[0]): Promise { - const original = console.error - const consoleSpy = vi.spyOn(console, 'error').mockImplementation((...args) => { - if (typeof args[0] === 'string' && args[0].includes('react-test-renderer is deprecated')) { - return - } - original(...args) + await act(async () => { + renderer = create(createElement(Harness, props)) }) - try { - await act(async () => { - renderer = create(createElement(Harness, props)) - }) - } finally { - consoleSpy.mockRestore() - } } async function update(props: Parameters[0]): Promise { diff --git a/mobile/src/session/use-mobile-native-chat-controller.test.ts b/mobile/src/session/use-mobile-native-chat-controller.test.ts index a9cd1201639..c04b560798d 100644 --- a/mobile/src/session/use-mobile-native-chat-controller.test.ts +++ b/mobile/src/session/use-mobile-native-chat-controller.test.ts @@ -127,24 +127,12 @@ describe('useMobileNativeChatController handleNativeChatSend', () => { } beforeEach(() => { - globalThis.IS_REACT_ACT_ENVIRONMENT = true vi.clearAllMocks() resetMobileNativeChatStaleInputForTests() captureSendOrigin.mockReturnValue(ORIGIN) - const original = console.error - const spy = vi.spyOn(console, 'error').mockImplementation((...a) => { - if (typeof a[0] === 'string' && a[0].includes('react-test-renderer is deprecated')) { - return - } - original(...a) + act(() => { + renderer = create(createElement(Harness)) }) - try { - act(() => { - renderer = create(createElement(Harness)) - }) - } finally { - spy.mockRestore() - } }) afterEach(() => { act(() => renderer?.unmount()) @@ -364,24 +352,12 @@ describe('useMobileNativeChatController launch-draft wiring', () => { } function render(tab: unknown): void { - const original = console.error - const spy = vi.spyOn(console, 'error').mockImplementation((...a) => { - if (typeof a[0] === 'string' && a[0].includes('react-test-renderer is deprecated')) { - return - } - original(...a) + act(() => { + renderer = create(createElement(Harness, { tab })) }) - try { - act(() => { - renderer = create(createElement(Harness, { tab })) - }) - } finally { - spy.mockRestore() - } } beforeEach(() => { - globalThis.IS_REACT_ACT_ENVIRONMENT = true draftsArgs.length = 0 viewMode.isTabChatView = () => true sessionState.messages = [] @@ -487,25 +463,13 @@ describe('useMobileNativeChatController ask dismissal across a transcript reload } beforeEach(() => { - globalThis.IS_REACT_ACT_ENVIRONMENT = true viewMode.isTabChatView = () => true setTranscript('ready') promptsState.ask = PROMPT promptsState.detectedAsk = PROMPT - const original = console.error - const spy = vi.spyOn(console, 'error').mockImplementation((...a) => { - if (typeof a[0] === 'string' && a[0].includes('react-test-renderer is deprecated')) { - return - } - original(...a) + act(() => { + renderer = create(createElement(Harness)) }) - try { - act(() => { - renderer = create(createElement(Harness)) - }) - } finally { - spy.mockRestore() - } }) afterEach(() => { @@ -757,22 +721,10 @@ describe('useMobileNativeChatController streaming scope', () => { } beforeEach(() => { - globalThis.IS_REACT_ACT_ENVIRONMENT = true viewMode.isTabChatView = () => true - const original = console.error - const spy = vi.spyOn(console, 'error').mockImplementation((...a) => { - if (typeof a[0] === 'string' && a[0].includes('react-test-renderer is deprecated')) { - return - } - original(...a) + act(() => { + renderer = create(createElement(Harness)) }) - try { - act(() => { - renderer = create(createElement(Harness)) - }) - } finally { - spy.mockRestore() - } }) afterEach(() => { diff --git a/mobile/src/session/use-mobile-native-chat-drafts-launch-draft.test.ts b/mobile/src/session/use-mobile-native-chat-drafts-launch-draft.test.ts index 91c84814c41..665d3ac1f2f 100644 --- a/mobile/src/session/use-mobile-native-chat-drafts-launch-draft.test.ts +++ b/mobile/src/session/use-mobile-native-chat-drafts-launch-draft.test.ts @@ -1,6 +1,6 @@ import { createElement } from 'react' import { act, create, type ReactTestRenderer } from 'react-test-renderer' -import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { afterEach, describe, expect, it } from 'vitest' import type { NativeChatMessage } from '../../../src/shared/native-chat-types' import { useMobileNativeChatDrafts } from './use-mobile-native-chat-drafts' @@ -23,10 +23,6 @@ describe('useMobileNativeChatDrafts launch draft', () => { let renderer: ReactTestRenderer | null = null let state: DraftState | null = null - beforeEach(() => { - globalThis.IS_REACT_ACT_ENVIRONMENT = true - }) - afterEach(() => { act(() => renderer?.unmount()) renderer = null @@ -65,20 +61,9 @@ describe('useMobileNativeChatDrafts launch draft', () => { } async function mount(tabId: string): Promise { - const original = console.error - const consoleSpy = vi.spyOn(console, 'error').mockImplementation((...args) => { - if (typeof args[0] === 'string' && args[0].includes('react-test-renderer is deprecated')) { - return - } - original(...args) + await act(async () => { + renderer = create(createElement(Harness, { tabId })) }) - try { - await act(async () => { - renderer = create(createElement(Harness, { tabId })) - }) - } finally { - consoleSpy.mockRestore() - } } it('prefills the composer from a host launch draft exactly once', async () => { diff --git a/mobile/src/session/use-mobile-native-chat-drafts.test.ts b/mobile/src/session/use-mobile-native-chat-drafts.test.ts index 154ef3fb870..db8e76696d0 100644 --- a/mobile/src/session/use-mobile-native-chat-drafts.test.ts +++ b/mobile/src/session/use-mobile-native-chat-drafts.test.ts @@ -1,6 +1,6 @@ import { createElement } from 'react' import { act, create, type ReactTestRenderer } from 'react-test-renderer' -import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { afterEach, describe, expect, it, vi } from 'vitest' import type { NativeChatMessage } from '../../../src/shared/native-chat-types' import { useMobileNativeChatDrafts } from './use-mobile-native-chat-drafts' @@ -30,10 +30,6 @@ describe('useMobileNativeChatDrafts', () => { let renderer: ReactTestRenderer | null = null let state: DraftState | null = null - beforeEach(() => { - globalThis.IS_REACT_ACT_ENVIRONMENT = true - }) - afterEach(() => { act(() => renderer?.unmount()) renderer = null @@ -69,20 +65,9 @@ describe('useMobileNativeChatDrafts', () => { } async function mount(tabId: string): Promise { - const original = console.error - const consoleSpy = vi.spyOn(console, 'error').mockImplementation((...args) => { - if (typeof args[0] === 'string' && args[0].includes('react-test-renderer is deprecated')) { - return - } - original(...args) + await act(async () => { + renderer = create(createElement(Harness, { tabId })) }) - try { - await act(async () => { - renderer = create(createElement(Harness, { tabId })) - }) - } finally { - consoleSpy.mockRestore() - } } async function switchTo(tabId: string): Promise { diff --git a/mobile/src/session/use-mobile-native-chat-file-search.test.ts b/mobile/src/session/use-mobile-native-chat-file-search.test.ts index e0500019844..93db87c8b32 100644 --- a/mobile/src/session/use-mobile-native-chat-file-search.test.ts +++ b/mobile/src/session/use-mobile-native-chat-file-search.test.ts @@ -15,17 +15,6 @@ function rpcSuccess(files: string[]): Awaited void { - const original = console.error - const spy = vi.spyOn(console, 'error').mockImplementation((...args) => { - if (typeof args[0] === 'string' && args[0].includes('react-test-renderer is deprecated')) { - return - } - original(...args) - }) - return () => spy.mockRestore() -} - describe('useMobileNativeChatFileSearch', () => { let renderer: ReactTestRenderer | null = null let state: SearchState | null = null @@ -35,19 +24,13 @@ describe('useMobileNativeChatFileSearch', () => { state = useMobileNativeChatFileSearch({ client, worktreeId: 'wt-1' }) return null } - const restore = suppressRendererWarning() - try { - await act(async () => { - renderer = create(createElement(Harness)) - }) - } finally { - restore() - } + await act(async () => { + renderer = create(createElement(Harness)) + }) } beforeEach(() => { vi.useFakeTimers() - globalThis.IS_REACT_ACT_ENVIRONMENT = true state = null }) diff --git a/mobile/src/session/use-mobile-native-chat-image-attachments.test.ts b/mobile/src/session/use-mobile-native-chat-image-attachments.test.ts index a57e1c7520c..cb8522bae7a 100644 --- a/mobile/src/session/use-mobile-native-chat-image-attachments.test.ts +++ b/mobile/src/session/use-mobile-native-chat-image-attachments.test.ts @@ -87,7 +87,6 @@ describe('useMobileNativeChatImageAttachments', () => { } beforeEach(() => { - globalThis.IS_REACT_ACT_ENVIRONMENT = true pick.mockReset() // Stale markers and write locks live at module scope (they outlive the // screen), so they also outlive a test. @@ -101,20 +100,9 @@ describe('useMobileNativeChatImageAttachments', () => { }) function mount(args: HookArgs): void { - const original = console.error - const spy = vi.spyOn(console, 'error').mockImplementation((...a) => { - if (typeof a[0] === 'string' && a[0].includes('react-test-renderer is deprecated')) { - return - } - original(...a) + act(() => { + renderer = create(createElement(Harness, { args })) }) - try { - act(() => { - renderer = create(createElement(Harness, { args })) - }) - } finally { - spy.mockRestore() - } } function update(args: HookArgs): void { diff --git a/mobile/src/session/use-mobile-native-chat-input-lease.test.ts b/mobile/src/session/use-mobile-native-chat-input-lease.test.ts index c7e60170f0a..7491f74500f 100644 --- a/mobile/src/session/use-mobile-native-chat-input-lease.test.ts +++ b/mobile/src/session/use-mobile-native-chat-input-lease.test.ts @@ -1,6 +1,6 @@ import { createElement } from 'react' import { act, create, type ReactTestRenderer } from 'react-test-renderer' -import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { afterEach, describe, expect, it } from 'vitest' import { useMobileNativeChatInputLease } from './use-mobile-native-chat-input-lease' type Lease = ReturnType @@ -9,10 +9,6 @@ describe('useMobileNativeChatInputLease', () => { let renderer: ReactTestRenderer | null = null let lease: Lease | null = null - beforeEach(() => { - globalThis.IS_REACT_ACT_ENVIRONMENT = true - }) - afterEach(() => { act(() => renderer?.unmount()) renderer = null @@ -25,65 +21,43 @@ describe('useMobileNativeChatInputLease', () => { } it('unlocks only after acknowledgement and clears on disconnect', async () => { - const original = console.error - const consoleSpy = vi.spyOn(console, 'error').mockImplementation((...args) => { - if (typeof args[0] === 'string' && args[0].includes('react-test-renderer is deprecated')) { - return - } - original(...args) + await act(async () => { + renderer = create(createElement(Harness, { connected: true })) }) - try { - await act(async () => { - renderer = create(createElement(Harness, { connected: true })) - }) - expect(lease?.ready).toBe(false) - expect(lease?.lockReason).toBe('waiting') - act(() => lease?.markReady('terminal')) - expect(lease?.ready).toBe(true) - expect(lease?.lockReason).toBeNull() + expect(lease?.ready).toBe(false) + expect(lease?.lockReason).toBe('waiting') + act(() => lease?.markReady('terminal')) + expect(lease?.ready).toBe(true) + expect(lease?.lockReason).toBeNull() - act(() => lease?.clear()) - expect(lease?.ready).toBe(false) - act(() => lease?.markReady('terminal')) - expect(lease?.ready).toBe(true) + act(() => lease?.clear()) + expect(lease?.ready).toBe(false) + act(() => lease?.markReady('terminal')) + expect(lease?.ready).toBe(true) - await act(async () => renderer?.update(createElement(Harness, { connected: false }))) - expect(lease?.ready).toBe(false) - expect(lease?.lockReason).toBe('disconnected') - } finally { - consoleSpy.mockRestore() - } + await act(async () => renderer?.update(createElement(Harness, { connected: false }))) + expect(lease?.ready).toBe(false) + expect(lease?.lockReason).toBe('disconnected') }) it('reports whether a clear actually dropped a lease', async () => { - const original = console.error - const consoleSpy = vi.spyOn(console, 'error').mockImplementation((...args) => { - if (typeof args[0] === 'string' && args[0].includes('react-test-renderer is deprecated')) { - return - } - original(...args) + await act(async () => { + renderer = create(createElement(Harness, { connected: true })) }) - try { - await act(async () => { - renderer = create(createElement(Harness, { connected: true })) - }) - // The route reads this to tell a real teardown from one React never sees. - expect(lease?.clear('terminal')).toBe(false) - expect(lease?.clear()).toBe(false) + // The route reads this to tell a real teardown from one React never sees. + expect(lease?.clear('terminal')).toBe(false) + expect(lease?.clear()).toBe(false) - act(() => lease?.markReady('terminal')) - let dropped: boolean | undefined - act(() => { - dropped = lease?.clear('terminal') - }) - expect(dropped).toBe(true) - expect(lease?.ready).toBe(false) - expect(lease?.clear('terminal')).toBe(false) + act(() => lease?.markReady('terminal')) + let dropped: boolean | undefined + act(() => { + dropped = lease?.clear('terminal') + }) + expect(dropped).toBe(true) + expect(lease?.ready).toBe(false) + expect(lease?.clear('terminal')).toBe(false) - act(() => lease?.markReady('other')) - expect(lease?.clear()).toBe(true) - } finally { - consoleSpy.mockRestore() - } + act(() => lease?.markReady('other')) + expect(lease?.clear()).toBe(true) }) }) diff --git a/mobile/src/session/use-mobile-native-chat-message-send.test.ts b/mobile/src/session/use-mobile-native-chat-message-send.test.ts index 0ceda5714da..06b30f5cdb4 100644 --- a/mobile/src/session/use-mobile-native-chat-message-send.test.ts +++ b/mobile/src/session/use-mobile-native-chat-message-send.test.ts @@ -80,7 +80,6 @@ describe('useMobileNativeChatMessageSend', () => { (clearInputWrite.mock.calls[0]?.[0] ?? {}) as { clearInput?: string } beforeEach(() => { - globalThis.IS_REACT_ACT_ENVIRONMENT = true sendWithOutcome.mockReset() sendWithOutcome.mockResolvedValue('accepted') clearInputWrite.mockReset() diff --git a/mobile/src/session/use-mobile-native-chat-readability.test.ts b/mobile/src/session/use-mobile-native-chat-readability.test.ts index 41d08c3c32c..b7e2c5daab3 100644 --- a/mobile/src/session/use-mobile-native-chat-readability.test.ts +++ b/mobile/src/session/use-mobile-native-chat-readability.test.ts @@ -10,7 +10,6 @@ describe('useMobileNativeChatReadability', () => { let readable = false beforeEach(() => { - globalThis.IS_REACT_ACT_ENVIRONMENT = true readable = false }) @@ -34,21 +33,10 @@ describe('useMobileNativeChatReadability', () => { readable = useMobileNativeChatReadability(client, worktreeId) return null } - const original = console.error - const consoleSpy = vi.spyOn(console, 'error').mockImplementation((...args) => { - if (typeof args[0] === 'string' && args[0].includes('react-test-renderer is deprecated')) { - return - } - original(...args) + await act(async () => { + renderer = create(createElement(Harness)) + await Promise.resolve() }) - try { - await act(async () => { - renderer = create(createElement(Harness)) - await Promise.resolve() - }) - } finally { - consoleSpy.mockRestore() - } return sendRequest } @@ -89,32 +77,21 @@ describe('useMobileNativeChatReadability', () => { readable = useMobileNativeChatReadability(client, worktreeId) return null } - const original = console.error - const consoleSpy = vi.spyOn(console, 'error').mockImplementation((...args) => { - if (typeof args[0] === 'string' && args[0].includes('react-test-renderer is deprecated')) { - return - } - original(...args) + await act(async () => { + renderer = create(createElement(Harness, { worktreeId: 'local-repo::/one' })) + await Promise.resolve() }) - try { - await act(async () => { - renderer = create(createElement(Harness, { worktreeId: 'local-repo::/one' })) - await Promise.resolve() - }) - expect(readable).toBe(true) + expect(readable).toBe(true) - act(() => renderer?.update(createElement(Harness, { worktreeId: 'ssh-repo::/two' }))) - expect(readable).toBe(false) - await act(async () => { - resolveNext({ - ok: true, - result: { repos: [{ id: 'ssh-repo', connectionId: 'model-a-ssh' }] } - }) - await Promise.resolve() + act(() => renderer?.update(createElement(Harness, { worktreeId: 'ssh-repo::/two' }))) + expect(readable).toBe(false) + await act(async () => { + resolveNext({ + ok: true, + result: { repos: [{ id: 'ssh-repo', connectionId: 'model-a-ssh' }] } }) - expect(readable).toBe(false) - } finally { - consoleSpy.mockRestore() - } + await Promise.resolve() + }) + expect(readable).toBe(false) }) }) diff --git a/mobile/src/session/use-mobile-native-chat-send-error.test.ts b/mobile/src/session/use-mobile-native-chat-send-error.test.ts index a22eee3a0a4..49ca07198d0 100644 --- a/mobile/src/session/use-mobile-native-chat-send-error.test.ts +++ b/mobile/src/session/use-mobile-native-chat-send-error.test.ts @@ -31,31 +31,13 @@ describe('useMobileNativeChatSendError', () => { return apiRef.current } - /** react-test-renderer logs a deprecation notice on every render; keep real errors. */ - function suppressRendererWarning(): () => void { - const original = console.error - const spy = vi.spyOn(console, 'error').mockImplementation((...args) => { - if (typeof args[0] === 'string' && args[0].includes('react-test-renderer is deprecated')) { - return - } - original(...args) - }) - return () => spy.mockRestore() - } - async function render(scopeKey: string | null = 'terminal-1'): Promise { - const restore = suppressRendererWarning() - try { - await act(async () => { - renderer = create(createElement(Harness, { scopeKey })) - }) - } finally { - restore() - } + await act(async () => { + renderer = create(createElement(Harness, { scopeKey })) + }) } beforeEach(() => { - globalThis.IS_REACT_ACT_ENVIRONMENT = true apiRef.current = null showToast.mockClear() vi.useFakeTimers() @@ -117,26 +99,16 @@ describe('useMobileNativeChatSendError', () => { await act(async () => api().show('a')) expect(api().message).toBe('a') - const restore = suppressRendererWarning() - try { - await act(async () => { - renderer?.update(createElement(Harness, { scopeKey: 'terminal-2' })) - }) - } finally { - restore() - } + await act(async () => { + renderer?.update(createElement(Harness, { scopeKey: 'terminal-2' })) + }) expect(api().message).toBeNull() }) it('falls back to the toast when the banner is not mounted', async () => { - const restore = suppressRendererWarning() - try { - await act(async () => { - renderer = create(createElement(Harness, { scopeKey: 'terminal-1', bannerMounted: false })) - }) - } finally { - restore() - } + await act(async () => { + renderer = create(createElement(Harness, { scopeKey: 'terminal-1', bannerMounted: false })) + }) // A deferred failure landing after the user left chat must still be seen. await act(async () => api().show('Delivery unconfirmed')) @@ -149,14 +121,9 @@ describe('useMobileNativeChatSendError', () => { // Captured while tab A was live; a 20s unconfirmed send resolves much later. const showFromTabA = api().show - const restore = suppressRendererWarning() - try { - await act(async () => { - renderer?.update(createElement(Harness, { scopeKey: 'terminal-2' })) - }) - } finally { - restore() - } + await act(async () => { + renderer?.update(createElement(Harness, { scopeKey: 'terminal-2' })) + }) await act(async () => showFromTabA('Message not sent')) // The banner belongs to terminal-2 now, so A's failure must not paint there. @@ -168,14 +135,9 @@ describe('useMobileNativeChatSendError', () => { await render('terminal-1') const clearFromTabA = api().clear - const restore = suppressRendererWarning() - try { - await act(async () => { - renderer?.update(createElement(Harness, { scopeKey: 'terminal-2' })) - }) - } finally { - restore() - } + await act(async () => { + renderer?.update(createElement(Harness, { scopeKey: 'terminal-2' })) + }) await act(async () => api().show('b')) // An accepted card action from tab A resolving late must not retire B's warning. await act(async () => clearFromTabA()) diff --git a/mobile/src/session/use-mobile-native-chat-session-options.test.ts b/mobile/src/session/use-mobile-native-chat-session-options.test.ts index 246c3021948..d7cab9a4c37 100644 --- a/mobile/src/session/use-mobile-native-chat-session-options.test.ts +++ b/mobile/src/session/use-mobile-native-chat-session-options.test.ts @@ -44,7 +44,6 @@ describe('useMobileNativeChatSessionOptions', () => { } beforeEach(() => { - globalThis.IS_REACT_ACT_ENVIRONMENT = true clearMobileSessionOptionRecordsForTests() dispatchCommand.mockReset() dispatchCommand.mockResolvedValue('accepted') diff --git a/mobile/src/session/use-mobile-native-chat-session.test.ts b/mobile/src/session/use-mobile-native-chat-session.test.ts index 8c685d00202..dfe5f285937 100644 --- a/mobile/src/session/use-mobile-native-chat-session.test.ts +++ b/mobile/src/session/use-mobile-native-chat-session.test.ts @@ -24,7 +24,6 @@ describe('useMobileNativeChatSession', () => { let emit: (frame: unknown) => void = () => {} beforeEach(() => { - globalThis.IS_REACT_ACT_ENVIRONMENT = true state = null }) @@ -45,20 +44,9 @@ describe('useMobileNativeChatSession', () => { } async function mount(client: RpcClient): Promise { - const original = console.error - const consoleSpy = vi.spyOn(console, 'error').mockImplementation((...args) => { - if (typeof args[0] === 'string' && args[0].includes('react-test-renderer is deprecated')) { - return - } - original(...args) + await act(async () => { + renderer = create(createElement(Harness, { client })) }) - try { - await act(async () => { - renderer = create(createElement(Harness, { client })) - }) - } finally { - consoleSpy.mockRestore() - } } it('drops an older-page response captured before transcript replacement', async () => { @@ -445,7 +433,6 @@ describe('useMobileNativeChatSession transcriptLoading', () => { }[] = [] beforeEach(() => { - globalThis.IS_REACT_ACT_ENVIRONMENT = true renders.length = 0 }) @@ -482,20 +469,9 @@ describe('useMobileNativeChatSession transcriptLoading', () => { } async function mountAt(client: RpcClient | null, sessionId: string | null): Promise { - const original = console.error - const consoleSpy = vi.spyOn(console, 'error').mockImplementation((...args) => { - if (typeof args[0] === 'string' && args[0].includes('react-test-renderer is deprecated')) { - return - } - original(...args) + await act(async () => { + renderer = create(createElement(Harness, { client, sessionId })) }) - try { - await act(async () => { - renderer = create(createElement(Harness, { client, sessionId })) - }) - } finally { - consoleSpy.mockRestore() - } } it('reports loading on the very first render, before the subscription effect runs', async () => { diff --git a/mobile/src/session/use-mobile-native-chat-stop.test.ts b/mobile/src/session/use-mobile-native-chat-stop.test.ts index 2a5460b67fa..bdc405cb57d 100644 --- a/mobile/src/session/use-mobile-native-chat-stop.test.ts +++ b/mobile/src/session/use-mobile-native-chat-stop.test.ts @@ -14,7 +14,6 @@ describe('useMobileNativeChatStop', () => { beforeEach(() => { vi.useFakeTimers() - globalThis.IS_REACT_ACT_ENVIRONMENT = true sendRequest.mockReset().mockResolvedValue({ ok: true, result: { send: { accepted: true } } diff --git a/mobile/src/session/use-mobile-native-chat-terminal-stream.test.ts b/mobile/src/session/use-mobile-native-chat-terminal-stream.test.ts index fdfb5041402..ce282fcd5b1 100644 --- a/mobile/src/session/use-mobile-native-chat-terminal-stream.test.ts +++ b/mobile/src/session/use-mobile-native-chat-terminal-stream.test.ts @@ -17,7 +17,6 @@ describe('useMobileNativeChatTerminalStream', () => { const hasTabsRecoveryNeedRef = { current: (): boolean => false } beforeEach(() => { - globalThis.IS_REACT_ACT_ENVIRONMENT = true subscriptionsRef.current = new Map([['terminal-1', () => {}]]) subscribingRef.current = new Set() webReadyRef.current = new Set(['terminal-1']) @@ -85,113 +84,73 @@ describe('useMobileNativeChatTerminalStream', () => { } it('replaces output with a lease-only stream while covered, then restores output', async () => { - const original = console.error - const consoleSpy = vi.spyOn(console, 'error').mockImplementation((...args) => { - if (typeof args[0] === 'string' && args[0].includes('react-test-renderer is deprecated')) { - return - } - original(...args) + await act(async () => { + renderer = create(createElement(Harness, { showNativeChat: false })) + }) + await act(async () => { + notifyWebReadyRef.current('terminal-1', false) + }) + expect(harnessRenderCount).toBe(1) + await act(async () => { + renderer?.update(createElement(Harness, { showNativeChat: true })) }) - try { - await act(async () => { - renderer = create(createElement(Harness, { showNativeChat: false })) - }) - await act(async () => { - notifyWebReadyRef.current('terminal-1', false) - }) - expect(harnessRenderCount).toBe(1) - await act(async () => { - renderer?.update(createElement(Harness, { showNativeChat: true })) - }) - expect(unsubscribe).toHaveBeenNthCalledWith(1, 'terminal-1') - expect(subscribe).toHaveBeenNthCalledWith(1, 'terminal-1') - expect(initializedRef.current.has('terminal-1')).toBe(false) + expect(unsubscribe).toHaveBeenNthCalledWith(1, 'terminal-1') + expect(subscribe).toHaveBeenNthCalledWith(1, 'terminal-1') + expect(initializedRef.current.has('terminal-1')).toBe(false) - await act(async () => { - renderer?.update(createElement(Harness, { showNativeChat: false })) - }) + await act(async () => { + renderer?.update(createElement(Harness, { showNativeChat: false })) + }) - expect(unsubscribe).toHaveBeenNthCalledWith(2, 'terminal-1') - expect(subscribe).toHaveBeenNthCalledWith(2, 'terminal-1') - } finally { - consoleSpy.mockRestore() - } + expect(unsubscribe).toHaveBeenNthCalledWith(2, 'terminal-1') + expect(subscribe).toHaveBeenNthCalledWith(2, 'terminal-1') }) it('re-subscribes a covered stream torn down under chat (#10681)', async () => { - const original = console.error - const consoleSpy = vi.spyOn(console, 'error').mockImplementation((...args) => { - if (typeof args[0] === 'string' && args[0].includes('react-test-renderer is deprecated')) { - return - } - original(...args) + await act(async () => { + renderer = create(createElement(Harness, { showNativeChat: true })) + }) + subscribe.mockClear() + unsubscribe.mockClear() + // What a terminal.list prune / `end` frame does to the lease-only stream: + // the subscription is gone and the lease drops with it. + subscriptionsRef.current.delete('terminal-1') + await act(async () => { + renderer?.update(createElement(Harness, { showNativeChat: true, leaseReady: false })) }) - try { - await act(async () => { - renderer = create(createElement(Harness, { showNativeChat: true })) - }) - subscribe.mockClear() - unsubscribe.mockClear() - // What a terminal.list prune / `end` frame does to the lease-only stream: - // the subscription is gone and the lease drops with it. - subscriptionsRef.current.delete('terminal-1') - await act(async () => { - renderer?.update(createElement(Harness, { showNativeChat: true, leaseReady: false })) - }) - expect(subscribe).toHaveBeenCalledOnce() - expect(subscribe).toHaveBeenCalledWith('terminal-1') - // Pins the rearm branch specifically: without it the action falls through to - // the resume tail, which also subscribes — but drops coverage on the way. - expect(unsubscribe).not.toHaveBeenCalled() - } finally { - consoleSpy.mockRestore() - } + expect(subscribe).toHaveBeenCalledOnce() + expect(subscribe).toHaveBeenCalledWith('terminal-1') + // Pins the rearm branch specifically: without it the action falls through to + // the resume tail, which also subscribes — but drops coverage on the way. + expect(unsubscribe).not.toHaveBeenCalled() }) it('stops rearming a handle whose stream never comes back (#10681)', async () => { - const original = console.error - const consoleSpy = vi.spyOn(console, 'error').mockImplementation((...args) => { - if (typeof args[0] === 'string' && args[0].includes('react-test-renderer is deprecated')) { - return - } - original(...args) + await act(async () => { + renderer = create(createElement(Harness, { showNativeChat: true })) }) - try { + subscribe.mockClear() + // A dead PTY answers every subscribe with `subscribed`+`end`, so the stream is + // gone again on each pass. Unbounded, that is a ~10s resubscribe loop. + for (let revision = 1; revision <= 6; revision += 1) { + subscriptionsRef.current.delete('terminal-1') await act(async () => { - renderer = create(createElement(Harness, { showNativeChat: true })) + renderer?.update( + createElement(Harness, { + showNativeChat: true, + leaseReady: false, + streamRevision: revision + }) + ) }) - subscribe.mockClear() - // A dead PTY answers every subscribe with `subscribed`+`end`, so the stream is - // gone again on each pass. Unbounded, that is a ~10s resubscribe loop. - for (let revision = 1; revision <= 6; revision += 1) { - subscriptionsRef.current.delete('terminal-1') - await act(async () => { - renderer?.update( - createElement(Harness, { - showNativeChat: true, - leaseReady: false, - streamRevision: revision - }) - ) - }) - } - - expect(subscribe.mock.calls.length).toBeLessThanOrEqual(3) - } finally { - consoleSpy.mockRestore() } + + expect(subscribe.mock.calls.length).toBeLessThanOrEqual(3) }) it('does not charge the rearm budget for a subscribe its own gates turned away', async () => { - const original = console.error - const consoleSpy = vi.spyOn(console, 'error').mockImplementation((...args) => { - if (typeof args[0] === 'string' && args[0].includes('react-test-renderer is deprecated')) { - return - } - original(...args) - }) try { await act(async () => { renderer = create(createElement(Harness, { showNativeChat: true })) @@ -218,89 +177,66 @@ describe('useMobileNativeChatTerminalStream', () => { subscribe.mockImplementation((handle: string) => subscriptionsRef.current.set(handle, () => {}) ) - consoleSpy.mockRestore() } }) it('refills the rearm budget when terminal.list reports the handle again (#10681)', async () => { - const original = console.error - const consoleSpy = vi.spyOn(console, 'error').mockImplementation((...args) => { - if (typeof args[0] === 'string' && args[0].includes('react-test-renderer is deprecated')) { - return - } - original(...args) + await act(async () => { + renderer = create(createElement(Harness, { showNativeChat: true })) }) - try { - await act(async () => { - renderer = create(createElement(Harness, { showNativeChat: true })) - }) - for (let revision = 1; revision <= 5; revision += 1) { - subscriptionsRef.current.delete('terminal-1') - await act(async () => { - renderer?.update( - createElement(Harness, { - showNativeChat: true, - leaseReady: false, - streamRevision: revision - }) - ) - }) - } - subscribe.mockClear() - // Budget is spent, so a further teardown signal alone changes nothing. - await act(async () => { - renderer?.update( - createElement(Harness, { showNativeChat: true, leaseReady: false, streamRevision: 6 }) - ) - }) - expect(subscribe).not.toHaveBeenCalled() - - // A dead-but-listed handle must not buy a new budget on every list refresh, - // or the resubscribe loop this bound exists to stop comes straight back. - await act(async () => { - notifyListedHandlesRef.current(new Set(['terminal-1'])) - notifyListedHandlesRef.current(new Set(['terminal-1'])) - }) - expect(subscribe).not.toHaveBeenCalled() - - // The handle actually went away and came back: it may have a live PTY once more. - await act(async () => { - notifyListedHandlesRef.current(new Set()) - notifyListedHandlesRef.current(new Set(['terminal-1'])) - }) - - expect(subscribe).toHaveBeenCalledWith('terminal-1') - } finally { - consoleSpy.mockRestore() - } - }) - - it('rearms on a teardown the lease cannot report (#10681)', async () => { - const original = console.error - const consoleSpy = vi.spyOn(console, 'error').mockImplementation((...args) => { - if (typeof args[0] === 'string' && args[0].includes('react-test-renderer is deprecated')) { - return - } - original(...args) - }) - try { - await act(async () => { - renderer = create(createElement(Harness, { showNativeChat: true })) - }) - subscribe.mockClear() - // `end` with no preceding `subscribed` leaves the lease untouched, so - // `leaseReady` holds its value and only the revision bump can re-run us. + for (let revision = 1; revision <= 5; revision += 1) { subscriptionsRef.current.delete('terminal-1') await act(async () => { renderer?.update( - createElement(Harness, { showNativeChat: true, leaseReady: true, streamRevision: 1 }) + createElement(Harness, { + showNativeChat: true, + leaseReady: false, + streamRevision: revision + }) ) }) - - expect(subscribe).toHaveBeenCalledWith('terminal-1') - } finally { - consoleSpy.mockRestore() } + subscribe.mockClear() + // Budget is spent, so a further teardown signal alone changes nothing. + await act(async () => { + renderer?.update( + createElement(Harness, { showNativeChat: true, leaseReady: false, streamRevision: 6 }) + ) + }) + expect(subscribe).not.toHaveBeenCalled() + + // A dead-but-listed handle must not buy a new budget on every list refresh, + // or the resubscribe loop this bound exists to stop comes straight back. + await act(async () => { + notifyListedHandlesRef.current(new Set(['terminal-1'])) + notifyListedHandlesRef.current(new Set(['terminal-1'])) + }) + expect(subscribe).not.toHaveBeenCalled() + + // The handle actually went away and came back: it may have a live PTY once more. + await act(async () => { + notifyListedHandlesRef.current(new Set()) + notifyListedHandlesRef.current(new Set(['terminal-1'])) + }) + + expect(subscribe).toHaveBeenCalledWith('terminal-1') + }) + + it('rearms on a teardown the lease cannot report (#10681)', async () => { + await act(async () => { + renderer = create(createElement(Harness, { showNativeChat: true })) + }) + subscribe.mockClear() + // `end` with no preceding `subscribed` leaves the lease untouched, so + // `leaseReady` holds its value and only the revision bump can re-run us. + subscriptionsRef.current.delete('terminal-1') + await act(async () => { + renderer?.update( + createElement(Harness, { showNativeChat: true, leaseReady: true, streamRevision: 1 }) + ) + }) + + expect(subscribe).toHaveBeenCalledWith('terminal-1') }) it('does not let a dead PTY buy a new budget with its own `subscribed` ack', async () => { @@ -411,34 +347,23 @@ describe('useMobileNativeChatTerminalStream', () => { }) it('resumes a cold-start lease-only stream when WebView readiness arrives late', async () => { - const original = console.error - const consoleSpy = vi.spyOn(console, 'error').mockImplementation((...args) => { - if (typeof args[0] === 'string' && args[0].includes('react-test-renderer is deprecated')) { - return - } - original(...args) + webReadyRef.current.clear() + await act(async () => { + renderer = create(createElement(Harness, { showNativeChat: true })) + }) + await act(async () => { + renderer?.update(createElement(Harness, { showNativeChat: false })) }) - try { - webReadyRef.current.clear() - await act(async () => { - renderer = create(createElement(Harness, { showNativeChat: true })) - }) - await act(async () => { - renderer?.update(createElement(Harness, { showNativeChat: false })) - }) - expect(unsubscribe).toHaveBeenCalledOnce() - expect(subscribe).toHaveBeenCalledOnce() + expect(unsubscribe).toHaveBeenCalledOnce() + expect(subscribe).toHaveBeenCalledOnce() - webReadyRef.current.add('terminal-1') - await act(async () => { - notifyWebReadyRef.current('terminal-1', false) - }) + webReadyRef.current.add('terminal-1') + await act(async () => { + notifyWebReadyRef.current('terminal-1', false) + }) - expect(unsubscribe).toHaveBeenNthCalledWith(2, 'terminal-1') - expect(subscribe).toHaveBeenNthCalledWith(2, 'terminal-1') - } finally { - consoleSpy.mockRestore() - } + expect(unsubscribe).toHaveBeenNthCalledWith(2, 'terminal-1') + expect(subscribe).toHaveBeenNthCalledWith(2, 'terminal-1') }) }) diff --git a/mobile/src/session/use-mobile-session-tabs-reconciliation.test.ts b/mobile/src/session/use-mobile-session-tabs-reconciliation.test.ts index 2ec37155bda..33c507f2712 100644 --- a/mobile/src/session/use-mobile-session-tabs-reconciliation.test.ts +++ b/mobile/src/session/use-mobile-session-tabs-reconciliation.test.ts @@ -112,8 +112,6 @@ async function setAppState(state: string): Promise { describe('useMobileSessionTabsReconciliation', () => { let renderer: ReactTestRenderer | null = null - let consoleErrorSpy: ReturnType - async function mount(): Promise { await act(async () => { renderer = create(createElement(Harness)) @@ -124,14 +122,6 @@ describe('useMobileSessionTabsReconciliation', () => { beforeEach(() => { vi.useFakeTimers() vi.setSystemTime(0) - globalThis.IS_REACT_ACT_ENVIRONMENT = true - const originalConsoleError = console.error - consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation((...args) => { - if (typeof args[0] === 'string' && args[0].includes('react-test-renderer is deprecated')) { - return - } - originalConsoleError(...args) - }) lifecycle.appState = 'active' lifecycle.focused = true lifecycle.listeners.clear() @@ -157,7 +147,6 @@ describe('useMobileSessionTabsReconciliation', () => { act(() => renderer?.unmount()) renderer = null streamListener = null - consoleErrorSpy.mockRestore() vi.useRealTimers() }) diff --git a/mobile/src/session/use-mobile-session-view-mode.test.ts b/mobile/src/session/use-mobile-session-view-mode.test.ts index 4315bda35bf..0eda383487c 100644 --- a/mobile/src/session/use-mobile-session-view-mode.test.ts +++ b/mobile/src/session/use-mobile-session-view-mode.test.ts @@ -52,7 +52,6 @@ describe('useMobileSessionViewMode', () => { let controller: MobileSessionViewModeController | null = null beforeEach(() => { - globalThis.IS_REACT_ACT_ENVIRONMENT = true vi.mocked(loadDefaultSessionView).mockReset().mockResolvedValue('terminal') vi.mocked(readSessionViewOverridesPreference) .mockReset() diff --git a/mobile/src/session/use-quick-commands.test.ts b/mobile/src/session/use-quick-commands.test.ts index 66fec5bf4e5..a4f93b326fc 100644 --- a/mobile/src/session/use-quick-commands.test.ts +++ b/mobile/src/session/use-quick-commands.test.ts @@ -1,6 +1,6 @@ import { createElement } from 'react' import { act, create, type ReactTestRenderer } from 'react-test-renderer' -import { afterEach, beforeEach, describe, expect, it, vi, type MockInstance } from 'vitest' +import { afterEach, describe, expect, it, vi } from 'vitest' import type { TerminalQuickCommand } from '../../../src/shared/types' import type { RpcClient } from '../transport/rpc-client' import { LogicalClientCutoverError } from '../transport/stable-logical-rpc-client' @@ -49,24 +49,10 @@ function deferred() { describe('useQuickCommands', () => { let renderer: ReactTestRenderer | null = null let state: ReturnType | null = null - let consoleSpy: MockInstance - - beforeEach(() => { - globalThis.IS_REACT_ACT_ENVIRONMENT = true - const original = console.error - consoleSpy = vi.spyOn(console, 'error').mockImplementation((...args) => { - if (typeof args[0] === 'string' && args[0].includes('react-test-renderer is deprecated')) { - return - } - original(...args) - }) - }) - afterEach(() => { act(() => renderer?.unmount()) renderer = null state = null - consoleSpy.mockRestore() }) async function mount(client: RpcClient, enabled = true): Promise { diff --git a/mobile/src/session/use-terminal-live-input-mode-preference.test.ts b/mobile/src/session/use-terminal-live-input-mode-preference.test.ts index d95fab23698..0a98d737792 100644 --- a/mobile/src/session/use-terminal-live-input-mode-preference.test.ts +++ b/mobile/src/session/use-terminal-live-input-mode-preference.test.ts @@ -34,18 +34,6 @@ function createDeferred(): Deferred { return { promise, resolve } } -function suppressReactTestRendererDeprecationWarning(): () => void { - const originalConsoleError = console.error - const consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation((...args) => { - const firstArg = args[0] - if (typeof firstArg === 'string' && firstArg.includes('react-test-renderer is deprecated')) { - return - } - originalConsoleError(...args) - }) - return () => consoleErrorSpy.mockRestore() -} - function createTerminalLiveInputModePreferenceHarness(): TerminalLiveInputModePreferenceHarness { let current: ReturnType | null = null let renderer: ReactTestRenderer | null = null @@ -58,14 +46,9 @@ function createTerminalLiveInputModePreferenceHarness(): TerminalLiveInputModePr return null } - const restoreConsoleError = suppressReactTestRendererDeprecationWarning() - try { - act(() => { - renderer = create(createElement(Harness)) - }) - } finally { - restoreConsoleError() - } + act(() => { + renderer = create(createElement(Harness)) + }) if (!current || !renderer) { throw new Error('terminal live input mode preference hook did not render') } diff --git a/mobile/src/session/use-throttled-latest-value.test.ts b/mobile/src/session/use-throttled-latest-value.test.ts index bdea29c7459..29e58c8fa35 100644 --- a/mobile/src/session/use-throttled-latest-value.test.ts +++ b/mobile/src/session/use-throttled-latest-value.test.ts @@ -1,12 +1,11 @@ import { createElement } from 'react' import { act, create, type ReactTestRenderer } from 'react-test-renderer' -import { afterEach, beforeEach, describe, expect, it, vi, type MockInstance } from 'vitest' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { useThrottledLatestValue } from './use-throttled-latest-value' describe('useThrottledLatestValue', () => { let renderer: ReactTestRenderer | null = null let latest: string | undefined - let consoleSpy: MockInstance function Harness({ value }: { value: string | undefined }): null { latest = useThrottledLatestValue(value, 50) @@ -25,22 +24,13 @@ describe('useThrottledLatestValue', () => { beforeEach(() => { vi.useFakeTimers() - globalThis.IS_REACT_ACT_ENVIRONMENT = true latest = undefined - const original = console.error - consoleSpy = vi.spyOn(console, 'error').mockImplementation((...args) => { - if (typeof args[0] === 'string' && args[0].includes('react-test-renderer is deprecated')) { - return - } - original(...args) - }) }) afterEach(() => { act(() => renderer?.unmount()) renderer = null vi.useRealTimers() - consoleSpy.mockRestore() }) it('emits the first frame immediately', () => { diff --git a/mobile/src/source-control/MobileGitHistoryList.test.tsx b/mobile/src/source-control/MobileGitHistoryList.test.tsx index 50b0fa7fe73..b19a069c80f 100644 --- a/mobile/src/source-control/MobileGitHistoryList.test.tsx +++ b/mobile/src/source-control/MobileGitHistoryList.test.tsx @@ -1,6 +1,6 @@ import { createElement, type ReactElement } from 'react' import { act, create, type ReactTestRenderer } from 'react-test-renderer' -import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { afterEach, describe, expect, it, vi } from 'vitest' import type { RpcClient } from '../transport/rpc-client' import type { ConnectionState } from '../transport/types' import { MobileGitHistoryList } from './MobileGitHistoryList' @@ -44,10 +44,6 @@ const compareResponse = { describe('MobileGitHistoryList', () => { let renderer: ReactTestRenderer | null = null - beforeEach(() => { - globalThis.IS_REACT_ACT_ENVIRONMENT = true - }) - afterEach(() => { act(() => renderer?.unmount()) renderer = null diff --git a/mobile/src/source-control/use-mobile-hosted-review-eligibility.test.ts b/mobile/src/source-control/use-mobile-hosted-review-eligibility.test.ts index 13109a610db..f6e3eab8873 100644 --- a/mobile/src/source-control/use-mobile-hosted-review-eligibility.test.ts +++ b/mobile/src/source-control/use-mobile-hosted-review-eligibility.test.ts @@ -1,6 +1,6 @@ import { createElement } from 'react' import { act, create, type ReactTestRenderer } from 'react-test-renderer' -import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { afterEach, describe, expect, it, vi } from 'vitest' import type { HostedReviewCreationEligibility } from '../../../src/shared/hosted-review' import { buildMobileHostedReviewEligibilityLoadKey, @@ -206,10 +206,6 @@ describe('eligibility request ordering', () => { let renderer: ReactTestRenderer | null = null let renderedState: MobileCreatePrEligibilityState = { kind: 'idle' } - beforeEach(() => { - globalThis.IS_REACT_ACT_ENVIRONMENT = true - }) - afterEach(() => { act(() => renderer?.unmount()) renderer = null diff --git a/mobile/src/terminal/terminal-webview-engine-error.test.ts b/mobile/src/terminal/terminal-webview-engine-error.test.ts index 4e29c287321..2a1ba5df93d 100644 --- a/mobile/src/terminal/terminal-webview-engine-error.test.ts +++ b/mobile/src/terminal/terminal-webview-engine-error.test.ts @@ -41,18 +41,6 @@ vi.mock('lucide-react-native', () => ({ RefreshCw: 'RefreshCw' })) -function suppressReactTestRendererDeprecationWarning(): () => void { - const originalConsoleError = console.error - const consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation((...args) => { - const firstArg = args[0] - if (typeof firstArg === 'string' && firstArg.includes('react-test-renderer is deprecated')) { - return - } - originalConsoleError(...args) - }) - return () => consoleErrorSpy.mockRestore() -} - // Why: mounting TerminalWebView arms the web-ready watchdog; tests must // unmount so the timer can't survive into later tests and fire in teardown. let activeRenderer: ReactTestRenderer | null = null @@ -62,14 +50,9 @@ function createTerminalWebViewRenderer( props: Record = {} ) { let renderer: ReactTestRenderer | null = null - const restoreConsoleError = suppressReactTestRendererDeprecationWarning() - try { - act(() => { - renderer = create(createElement(TerminalWebView, { onEngineError, ...props })) - }) - } finally { - restoreConsoleError() - } + act(() => { + renderer = create(createElement(TerminalWebView, { onEngineError, ...props })) + }) if (!renderer) { throw new Error('TerminalWebView did not render') } diff --git a/mobile/src/terminal/use-terminal-live-accessory-input-commit.test.ts b/mobile/src/terminal/use-terminal-live-accessory-input-commit.test.ts index 1ca5216c7a8..2542b315a8f 100644 --- a/mobile/src/terminal/use-terminal-live-accessory-input-commit.test.ts +++ b/mobile/src/terminal/use-terminal-live-accessory-input-commit.test.ts @@ -25,18 +25,6 @@ function createDeferredBoolean(): DeferredBoolean { return { promise, resolve: resolvePromise } } -function suppressReactTestRendererDeprecationWarning(): () => void { - const originalConsoleError = console.error - const consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation((...args) => { - const firstArg = args[0] - if (typeof firstArg === 'string' && firstArg.includes('react-test-renderer is deprecated')) { - return - } - originalConsoleError(...args) - }) - return () => consoleErrorSpy.mockRestore() -} - type AccessoryInputCommitHarnessOptions = { readonly heldText?: string readonly sentText?: string @@ -105,14 +93,9 @@ function createAccessoryInputCommitHarness({ return null } - const restoreConsoleError = suppressReactTestRendererDeprecationWarning() - try { - act(() => { - renderer = create(createElement(Harness)) - }) - } finally { - restoreConsoleError() - } + act(() => { + renderer = create(createElement(Harness)) + }) if (!commit || !renderer) { throw new Error('terminal live accessory input hook did not render') } diff --git a/mobile/src/terminal/use-terminal-live-input-commit.test.ts b/mobile/src/terminal/use-terminal-live-input-commit.test.ts index c211ff447ec..544d57a1bf1 100644 --- a/mobile/src/terminal/use-terminal-live-input-commit.test.ts +++ b/mobile/src/terminal/use-terminal-live-input-commit.test.ts @@ -20,18 +20,6 @@ type TerminalLiveInputCommitHarnessOptions = { readonly sendResult?: boolean } -function suppressReactTestRendererDeprecationWarning(): () => void { - const originalConsoleError = console.error - const consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation((...args) => { - const firstArg = args[0] - if (typeof firstArg === 'string' && firstArg.includes('react-test-renderer is deprecated')) { - return - } - originalConsoleError(...args) - }) - return () => consoleErrorSpy.mockRestore() -} - function createTerminalLiveInputCommitHarness({ sendResult = true }: TerminalLiveInputCommitHarnessOptions = {}): TerminalLiveInputCommitHarness { @@ -77,14 +65,9 @@ function createTerminalLiveInputCommitHarness({ return null } - const restoreConsoleError = suppressReactTestRendererDeprecationWarning() - try { - act(() => { - renderer = create(createElement(Harness)) - }) - } finally { - restoreConsoleError() - } + act(() => { + renderer = create(createElement(Harness)) + }) if (!handlers || !renderer) { throw new Error('terminal live input hook did not render') } diff --git a/mobile/src/terminal/use-terminal-live-input-focus.test.ts b/mobile/src/terminal/use-terminal-live-input-focus.test.ts index f1b6c5eadb9..92ce4e2c455 100644 --- a/mobile/src/terminal/use-terminal-live-input-focus.test.ts +++ b/mobile/src/terminal/use-terminal-live-input-focus.test.ts @@ -1,6 +1,6 @@ import { createElement, type RefObject } from 'react' import { act, create, type ReactTestRenderer } from 'react-test-renderer' -import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { afterEach, describe, expect, it, vi } from 'vitest' import type { TerminalLiveInputFocusTarget, TerminalLiveInputFocusTimerRef @@ -20,17 +20,6 @@ type HarnessProps = { type FocusHandlers = ReturnType -function suppressReactTestRendererWarning(): () => void { - const originalConsoleError = console.error - const spy = vi.spyOn(console, 'error').mockImplementation((...args) => { - if (typeof args[0] === 'string' && args[0].includes('react-test-renderer is deprecated')) { - return - } - originalConsoleError(...args) - }) - return () => spy.mockRestore() -} - function createFocusTarget(initiallyFocused = false): TerminalLiveInputFocusTarget & { readonly blur: ReturnType readonly focus: ReturnType @@ -67,14 +56,9 @@ function createHarness(initialProps: HarnessProps): { return null } - const restoreWarning = suppressReactTestRendererWarning() - try { - act(() => { - renderer = create(createElement(Harness, initialProps)) - }) - } finally { - restoreWarning() - } + act(() => { + renderer = create(createElement(Harness, initialProps)) + }) if (!handlers || !renderer) { throw new Error('terminal live input focus harness did not render') } @@ -113,10 +97,6 @@ function connectedProps( } describe('terminal live input focus hook', () => { - beforeEach(() => { - globalThis.IS_REACT_ACT_ENVIRONMENT = true - }) - afterEach(() => { vi.useRealTimers() }) diff --git a/mobile/src/transport/client-context.test.ts b/mobile/src/transport/client-context.test.ts index b08899f5333..59b0a7ac5dc 100644 --- a/mobile/src/transport/client-context.test.ts +++ b/mobile/src/transport/client-context.test.ts @@ -71,18 +71,6 @@ type Harness = { readonly unmount: () => void } -function suppressReactTestRendererDeprecationWarning(): () => void { - const originalConsoleError = console.error - const spy = vi.spyOn(console, 'error').mockImplementation((...args) => { - const firstArg = args[0] - if (typeof firstArg === 'string' && firstArg.includes('react-test-renderer is deprecated')) { - return - } - originalConsoleError(...args) - }) - return () => spy.mockRestore() -} - async function renderHarness(hostId: string): Promise { let hook: ReturnType | null = null let closeHost: ((hostId: string) => void) | null = null @@ -94,14 +82,9 @@ async function renderHarness(hostId: string): Promise { return null } - const restore = suppressReactTestRendererDeprecationWarning() - try { - await act(async () => { - renderer = create(createElement(RpcClientProvider, null, createElement(Probe))) - }) - } finally { - restore() - } + await act(async () => { + renderer = create(createElement(RpcClientProvider, null, createElement(Probe))) + }) if (!hook || !closeHost || !renderer) { throw new Error('harness did not render') } @@ -124,7 +107,6 @@ async function renderHarness(hostId: string): Promise { } beforeEach(() => { - globalThis.IS_REACT_ACT_ENVIRONMENT = true connectMock.mockReset() loadHostsMock.mockReset() }) @@ -149,7 +131,6 @@ describe('useHostClient', () => { return null } - const restore = suppressReactTestRendererDeprecationWarning() try { await act(async () => { renderer = create(createElement(RpcClientProvider, null, createElement(Probe))) @@ -169,7 +150,6 @@ describe('useHostClient', () => { expect(selectedState).toBe('disconnected') expect(connectMock).toHaveBeenCalledTimes(2) } finally { - restore() act(() => renderer?.unmount()) } }) @@ -188,7 +168,6 @@ describe('useHostClient', () => { return null } - const restore = suppressReactTestRendererDeprecationWarning() try { await act(async () => { renderer = create(createElement(RpcClientProvider, null, createElement(Probe))) @@ -211,7 +190,6 @@ describe('useHostClient', () => { }) expect(stateByRenderTick.get(2)).toBe('connecting') } finally { - restore() act(() => renderer?.unmount()) } }) @@ -263,7 +241,6 @@ describe('useHostClient', () => { states.push(useHostClient(HOST.id).state) return null } - const restore = suppressReactTestRendererDeprecationWarning() try { act(() => { renderer = create(createElement(RpcClientProvider, null, createElement(Probe))) @@ -277,7 +254,6 @@ describe('useHostClient', () => { expect(states.at(-1)).toBe('connecting') expect(states).not.toContain('disconnected') } finally { - restore() act(() => renderer?.unmount()) } }) @@ -296,7 +272,6 @@ describe('useHostClient', () => { states.push(useHostClient(HOST.id).state) return null } - const restore = suppressReactTestRendererDeprecationWarning() try { await act(async () => { renderer = create(createElement(RpcClientProvider, null, createElement(Probe))) @@ -311,7 +286,6 @@ describe('useHostClient', () => { expect(states.at(-1)).toBe('connecting') expect(states).not.toContain('disconnected') } finally { - restore() act(() => renderer?.unmount()) } }) @@ -333,14 +307,9 @@ describe('useHostClient', () => { return null } - const restore = suppressReactTestRendererDeprecationWarning() - try { - act(() => { - renderer = create(createElement(RpcClientProvider, null, createElement(Probe))) - }) - } finally { - restore() - } + act(() => { + renderer = create(createElement(RpcClientProvider, null, createElement(Probe))) + }) expect(loadHostsMock).toHaveBeenCalledOnce() if (!closeHost || !resolveHosts || !renderer) { throw new Error('pending-open harness did not initialize') @@ -370,14 +339,9 @@ describe('useHostClient', () => { useHostClient(HOST.id) return null } - const restore = suppressReactTestRendererDeprecationWarning() - try { - act(() => { - renderer = create(createElement(RpcClientProvider, null, createElement(Probe))) - }) - } finally { - restore() - } + act(() => { + renderer = create(createElement(RpcClientProvider, null, createElement(Probe))) + }) expect(loadHostsMock).toHaveBeenCalledOnce() act(() => renderer?.unmount()) await act(async () => { @@ -401,7 +365,6 @@ describe('useAllHostClients', () => { return null } - const restore = suppressReactTestRendererDeprecationWarning() try { await act(async () => { renderer = create(createElement(RpcClientProvider, null, createElement(Probe))) @@ -410,7 +373,6 @@ describe('useAllHostClients', () => { expect(connectMock).toHaveBeenCalledOnce() expect(connectMock).toHaveBeenCalledWith(host2, expect.any(Function)) } finally { - restore() act(() => renderer?.unmount()) } }) @@ -433,7 +395,6 @@ describe('useAllHostClients', () => { return null } - const restore = suppressReactTestRendererDeprecationWarning() try { await act(async () => { renderer = create(createElement(RpcClientProvider, null, createElement(Probe))) @@ -446,7 +407,6 @@ describe('useAllHostClients', () => { 'host-997' ]) } finally { - restore() act(() => renderer?.unmount()) } }) @@ -477,7 +437,6 @@ describe('useAllHostClients', () => { return null } - const restore = suppressReactTestRendererDeprecationWarning() try { await act(async () => { renderer = create( @@ -504,7 +463,6 @@ describe('useAllHostClients', () => { expect(clients.get('host-c')?.closeMock).toHaveBeenCalledOnce() expect(clients.get('host-d')?.closeMock).not.toHaveBeenCalled() } finally { - restore() act(() => renderer?.unmount()) } }) @@ -520,7 +478,6 @@ describe('useAllHostClients', () => { return null } - const restore = suppressReactTestRendererDeprecationWarning() try { await act(async () => { renderer = create(createElement(RpcClientProvider, null, createElement(Probe))) @@ -528,7 +485,6 @@ describe('useAllHostClients', () => { }) expect(connectMock).toHaveBeenCalledTimes(2) } finally { - restore() act(() => renderer?.unmount()) } }) @@ -545,7 +501,6 @@ describe('useAllHostClients', () => { return null } - const restore = suppressReactTestRendererDeprecationWarning() try { await act(async () => { renderer = create(createElement(RpcClientProvider, null, createElement(Probe))) @@ -559,7 +514,6 @@ describe('useAllHostClients', () => { }) expect(connectMock).toHaveBeenCalledOnce() } finally { - restore() act(() => renderer?.unmount()) } }) diff --git a/mobile/src/transport/host-status-gates.test.ts b/mobile/src/transport/host-status-gates.test.ts index 483cfcca50f..f0454243703 100644 --- a/mobile/src/transport/host-status-gates.test.ts +++ b/mobile/src/transport/host-status-gates.test.ts @@ -4,17 +4,6 @@ import { describe, expect, it, vi } from 'vitest' import type { RpcClient } from './rpc-client' import { useHostStatusGates, type HostStatusGates } from './host-status-gates' -function suppressReactTestRendererDeprecationWarning(): () => void { - const originalConsoleError = console.error - const spy = vi.spyOn(console, 'error').mockImplementation((...args) => { - if (typeof args[0] === 'string' && args[0].includes('react-test-renderer is deprecated')) { - return - } - originalConsoleError(...args) - }) - return () => spy.mockRestore() -} - describe('useHostStatusGates', () => { it('clears every prior-host gate and ignores its late response while the client is replaced', async () => { let resolveOldStatus: ((response: unknown) => void) | null = null @@ -43,7 +32,6 @@ describe('useHostStatusGates', () => { return null } - const restore = suppressReactTestRendererDeprecationWarning() try { await act(async () => { renderer = create(createElement(Probe, { hostId: 'host-1', client: oldClient })) @@ -80,7 +68,6 @@ describe('useHostStatusGates', () => { expect(oldSendRequest).toHaveBeenCalledOnce() expect(newSendRequest).toHaveBeenCalledOnce() } finally { - restore() renderer?.unmount() } }) @@ -102,7 +89,6 @@ describe('useHostStatusGates', () => { return null } - const restore = suppressReactTestRendererDeprecationWarning() try { await act(async () => { renderer = create(createElement(Probe, { hostId: 'host-1' })) @@ -115,7 +101,6 @@ describe('useHostStatusGates', () => { expect(sendRequest).toHaveBeenCalledOnce() } finally { - restore() renderer?.unmount() } }) @@ -141,7 +126,6 @@ describe('useHostStatusGates', () => { return null } - const restore = suppressReactTestRendererDeprecationWarning() try { await act(async () => { renderer = create(createElement(Probe, { connState: 'connected' })) @@ -181,7 +165,6 @@ describe('useHostStatusGates', () => { statusPending: false }) } finally { - restore() renderer?.unmount() } }) @@ -204,7 +187,6 @@ describe('useHostStatusGates', () => { return null } - const restore = suppressReactTestRendererDeprecationWarning() try { await act(async () => { renderer = create(createElement(Probe, { client: firstClient })) @@ -217,7 +199,6 @@ describe('useHostStatusGates', () => { }) expect(gates).toMatchObject({ hostCapabilities: [], statusPending: true }) } finally { - restore() renderer?.unmount() } }) diff --git a/mobile/src/transport/settings-host-client-lifecycle.test.ts b/mobile/src/transport/settings-host-client-lifecycle.test.ts index 7a23c86ac71..2762e7e85a5 100644 --- a/mobile/src/transport/settings-host-client-lifecycle.test.ts +++ b/mobile/src/transport/settings-host-client-lifecycle.test.ts @@ -151,29 +151,13 @@ function TestApp({ ) } -function suppressRendererWarning(): () => void { - const originalConsoleError = console.error - const spy = vi.spyOn(console, 'error').mockImplementation((...args) => { - if (typeof args[0] === 'string' && args[0].includes('react-test-renderer is deprecated')) { - return - } - originalConsoleError(...args) - }) - return () => spy.mockRestore() -} - async function renderScreen(screen: Screen, detailHostId?: string): Promise { let renderer: ReactTestRenderer | null = null - const restore = suppressRendererWarning() - try { - await act(async () => { - renderer = create(createElement(TestApp, { screen, detailHostId })) - await Promise.resolve() - await Promise.resolve() - }) - } finally { - restore() - } + await act(async () => { + renderer = create(createElement(TestApp, { screen, detailHostId })) + await Promise.resolve() + await Promise.resolve() + }) if (!renderer) { throw new Error('settings lifecycle harness did not render') } @@ -203,7 +187,6 @@ function activeHostIds(): string[] { } beforeEach(() => { - globalThis.IS_REACT_ACT_ENVIRONMENT = true context = null routeFocus.effect = null connectMock.mockReset() @@ -235,16 +218,11 @@ describe('settings host client lifecycle', () => { } let renderer: ReactTestRenderer | null = null - const restore = suppressRendererWarning() - try { - await act(async () => { - renderer = create(createElement(NavigationStack, { settingsVisible: false })) - await Promise.resolve() - await Promise.resolve() - }) - } finally { - restore() - } + await act(async () => { + renderer = create(createElement(NavigationStack, { settingsVisible: false })) + await Promise.resolve() + await Promise.resolve() + }) if (!renderer) { throw new Error('navigation stack harness did not render') } @@ -354,20 +332,15 @@ describe('settings host client lifecycle', () => { } let renderer: ReactTestRenderer | null = null - const restore = suppressRendererWarning() - try { - await act(async () => { - renderer = create( - createElement(HostListApp, { - settingsHostIds: [retainedHostId, sharedHostId, removedHostId] - }) - ) - await Promise.resolve() - await Promise.resolve() - }) - } finally { - restore() - } + await act(async () => { + renderer = create( + createElement(HostListApp, { + settingsHostIds: [retainedHostId, sharedHostId, removedHostId] + }) + ) + await Promise.resolve() + await Promise.resolve() + }) if (!renderer) { throw new Error('host-list lifecycle harness did not render') } @@ -451,14 +424,9 @@ describe('settings host client lifecycle', () => { } let renderer: ReactTestRenderer | null = null - const restore = suppressRendererWarning() - try { - act(() => { - renderer = create(createElement(RetryApp, { settingsVisible: true })) - }) - } finally { - restore() - } + act(() => { + renderer = create(createElement(RetryApp, { settingsVisible: true })) + }) if (!renderer || !resolveInitial || !resolveRetry) { throw new Error('retry lifecycle harness did not initialize') } @@ -517,14 +485,9 @@ describe('settings host client lifecycle', () => { } let renderer: ReactTestRenderer | null = null - const restore = suppressRendererWarning() - try { - act(() => { - renderer = create(createElement(PendingApp, { visible: true })) - }) - } finally { - restore() - } + act(() => { + renderer = create(createElement(PendingApp, { visible: true })) + }) if (!renderer || !resolveFirst || !resolveSecond) { throw new Error('pending settings harness did not initialize') } @@ -580,17 +543,12 @@ describe('settings host client lifecycle', () => { } let renderer: ReactTestRenderer | null = null - const restore = suppressRendererWarning() - try { - await act(async () => { - renderer = create( - createElement(ReplacementApp, { detailVisible: false, settingsVisible: false }) - ) - await Promise.resolve() - }) - } finally { - restore() - } + await act(async () => { + renderer = create( + createElement(ReplacementApp, { detailVisible: false, settingsVisible: false }) + ) + await Promise.resolve() + }) if (!renderer || !context) { throw new Error('replacement lifecycle harness did not initialize') } @@ -665,14 +623,9 @@ describe('settings host client lifecycle', () => { } let renderer: ReactTestRenderer | null = null - const restore = suppressRendererWarning() - try { - act(() => { - renderer = create(createElement(ManualApp, { selected: false })) - }) - } finally { - restore() - } + act(() => { + renderer = create(createElement(ManualApp, { selected: false })) + }) if (!renderer || !context) { throw new Error('manual connection harness did not initialize') } @@ -735,16 +688,11 @@ describe('settings host client lifecycle', () => { } let renderer: ReactTestRenderer | null = null - const restore = suppressRendererWarning() - try { - await act(async () => { - renderer = create(createElement(FocusStack)) - await Promise.resolve() - await Promise.resolve() - }) - } finally { - restore() - } + await act(async () => { + renderer = create(createElement(FocusStack)) + await Promise.resolve() + await Promise.resolve() + }) if (!renderer || !routeFocus.effect) { throw new Error('settings focus harness did not initialize') } diff --git a/mobile/vitest.config.ts b/mobile/vitest.config.ts index e39f90eedf1..981a1b55e74 100644 --- a/mobile/vitest.config.ts +++ b/mobile/vitest.config.ts @@ -9,6 +9,8 @@ export default defineConfig({ oxc: vitestOxcConfig, test: { environment: 'node', + setupFiles: ['./vitest.setup.ts'], + onConsoleLog: (log) => !log.includes('react-test-renderer is deprecated'), // .tsx too: component tests exist (react-test-renderer + mocked react-native) and were // silently never collected, so render-level regressions shipped untested. include: ['src/**/*.test.ts', 'src/**/*.test.tsx'] diff --git a/mobile/vitest.setup.ts b/mobile/vitest.setup.ts new file mode 100644 index 00000000000..740c1eb1bab --- /dev/null +++ b/mobile/vitest.setup.ts @@ -0,0 +1 @@ +Object.assign(globalThis, { IS_REACT_ACT_ENVIRONMENT: true }) From 65a5e8470ad89efafe17795137c0f75d02a9d159 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sun, 9 Aug 2026 18:31:16 -0700 Subject: [PATCH 05/12] refactor: share GitHub link normalization (#13409) --- src/renderer/src/lib/github-links.ts | 57 +--------------------------- 1 file changed, 1 insertion(+), 56 deletions(-) diff --git a/src/renderer/src/lib/github-links.ts b/src/renderer/src/lib/github-links.ts index 34a53c0494e..766d8a0eb28 100644 --- a/src/renderer/src/lib/github-links.ts +++ b/src/renderer/src/lib/github-links.ts @@ -1,56 +1 @@ -// Why: the parsing core moved to shared so main's terminal side-effect -// tracker can emit pr-link facts (terminal-side-effect-authority.md, slice 3). -// Re-exported here so renderer consumers keep their '@/lib' import path. -// normalizeGitHubLinkQuery stays renderer-side: its too-large guard is link- -// picker input policy, not parsing. -import { - type GitHubIssueOrPRLink, - parseGitHubIssueOrPRLink, - parseGitHubIssueOrPRNumber -} from '../../../shared/github-links' - -import { isWorkItemLinkQueryTooLarge } from './work-item-link-query-bounds' - -export * from '../../../shared/github-links' - -const HTTP_URL_PREFIX_RE = /^https?:\/\//i - -export type GitHubLinkQuery = { - query: string - directNumber: number | null - directLink?: GitHubIssueOrPRLink - tooLarge?: boolean -} - -/** - * Normalizes link-picker input so both raw issue/PR numbers and full GitHub - * URLs resolve to a usable query + direct-number lookup. - */ -export function normalizeGitHubLinkQuery(raw: string): GitHubLinkQuery { - if (isWorkItemLinkQueryTooLarge(raw)) { - return { query: '', directNumber: null, tooLarge: true } - } - const trimmed = raw.trim() - if (!trimmed) { - return { query: '', directNumber: null } - } - - const direct = parseGitHubIssueOrPRNumber(trimmed) - if (direct !== null && !HTTP_URL_PREFIX_RE.test(trimmed)) { - return { query: trimmed, directNumber: direct } - } - - const link = parseGitHubIssueOrPRLink(trimmed) - if (!link) { - return { query: trimmed, directNumber: null } - } - - // Why: any GitHub-shaped issue/pull URL is accepted by number regardless of - // slug, since fork checkouts can legitimately target upstream issues whose - // slug differs from the origin remote. - return { - query: trimmed, - directNumber: link.number, - directLink: link - } -} +export * from '../../../shared/new-workspace/github-links' From 1e72534709c5a1f56d8ddca4da1b48cde9e8e535 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sun, 9 Aug 2026 18:31:36 -0700 Subject: [PATCH 06/12] refactor: deduplicate repository search entries (#13415) --- .../settings/RepositoryPane.test.ts | 7 ++++ .../components/settings/repository-search.ts | 35 ------------------- 2 files changed, 7 insertions(+), 35 deletions(-) diff --git a/src/renderer/src/components/settings/RepositoryPane.test.ts b/src/renderer/src/components/settings/RepositoryPane.test.ts index f7b5d09da09..47c00a0a601 100644 --- a/src/renderer/src/components/settings/RepositoryPane.test.ts +++ b/src/renderer/src/components/settings/RepositoryPane.test.ts @@ -104,6 +104,13 @@ describe('RepositoryPane search entries', () => { expect(matchesSettingsSearch('worktree path', entries)).toBe(true) }) + it('includes each project search section once', () => { + const entries = getRepositoryPaneSearchEntries(repo) + + expect(entries.filter(({ title }) => title === 'Project Icon')).toHaveLength(1) + expect(entries.filter(({ title }) => title === 'Default Worktree Base')).toHaveLength(1) + }) + it('omits project runtime search for remote or unsupported repos', () => { expect(matchesSettingsSearch('project runtime', getRepositoryPaneSearchEntries(repo))).toBe( false diff --git a/src/renderer/src/components/settings/repository-search.ts b/src/renderer/src/components/settings/repository-search.ts index 1b7a39e7f75..ea878ee2b8e 100644 --- a/src/renderer/src/components/settings/repository-search.ts +++ b/src/renderer/src/components/settings/repository-search.ts @@ -185,41 +185,6 @@ export function getRepositoryPaneSearchEntries( ) ] }, - { - title: translate('auto.components.settings.repository.search.b24f00294a', 'Project Icon'), - description: translate( - 'auto.components.settings.repository.search.a1f3a2bd47', - 'Project icon and color used in the sidebar and tabs.' - ), - keywords: [ - repo.displayName, - ...translateSearchKeyword( - 'auto.components.settings.repository.search.6438a94c63', - 'project icon' - ), - ...translateSearchKeyword( - 'auto.components.settings.repository.search.b2546efab5', - 'repository icon' - ), - ...translateSearchKeyword('auto.components.settings.repository.search.8d045419b1', 'color'), - ...translateSearchKeyword('auto.components.settings.repository.search.6d8de2f090', 'hex'), - ...translateSearchKeyword('auto.components.settings.repository.search.c1075178cf', 'badge'), - ...translateSearchKeyword( - 'auto.components.settings.repository.search.cb4b4de666', - 'avatar' - ), - ...translateSearchKeyword( - 'auto.components.settings.repository.search.9dc60d7f6d', - 'github' - ), - ...translateSearchKeyword('auto.components.settings.repository.search.1e73e840ff', 'emoji'), - ...translateSearchKeyword( - 'auto.components.settings.repository.search.27733eb6c1', - 'favicon' - ) - ] - }, - ...(isFolder ? [] : getRepositoryGitWorktreeSearchEntries(repo)), ...(isFolder ? [] : [...getRepositoryGitAuthorSearchEntries(repo), ...getRepositoryGitHooksSearchEntries(repo)]) From cb6069ceea4692ee31c6065e630fd24f9895b8b6 Mon Sep 17 00:00:00 2001 From: Seiyoung Park Date: Mon, 10 Aug 2026 10:34:05 +0900 Subject: [PATCH 07/12] fix(ui): align toggle switch handle symmetrically in on state (#9714) * fix(ui): align toggle switch handle symmetrically in on state The switch handle (translate-x-4 / 16px) left a 6px gap on the right in the on state while the off state had only a 2px gap on the left. Accounting for border-box sizing (1px border each side reduces content width to 34px), the correct on-position is translate-x-4.5 (18px) so both sides have a 2px inset. * docs: add JSDoc to exported functions in changed files for docstring coverage * fix(ui): align toggle switch handle symmetrically in on state The `h-5 w-9` switch track is `border-box` with a 1px border, leaving 34px of content for the 14px `size-3.5` handle. `translate-x-0.5` insets the off state by 2px, so the on state needs 34 - 14 - 2 = 18px. It used `translate-x-4` (16px), leaving 4px on the right against 2px on the left. Adds a boundary test so the offset cannot drift again across the 26 hand-rolled switch call sites, and drops the docstrings the original patch added to satisfy a coverage bot (the surrounding files carry none). Original patch by @sei0. Co-authored-by: Orca * test(ui): widen the switch-handle alignment guard to every h-5 w-9 track Scan the renderer with the readdirSync walk and TypeScript AST already used by no-top-level-translate.test.ts instead of shelling out to `git grep`, and find each knob inside its own track rather than by scanning source text forward. Why: gating on `role="switch"` silently skipped ClaudeUsageLoadingState.tsx (a loading skeleton, so a div with no role) and HiddenExperimentalGroup.tsx, so regressing the loading-state handle back to translate-x-4 left the guard green. The guard now also fails when the premise behind 34 - 14 - 2 = 18px stops holding: a track that loses its 1px border or gains padding, a conditional whose two branches no longer pair off/on, and a track whose knob moved into a child component where the scan would otherwise just stop seeing it. --------- Co-authored-by: Neil <4138956+nwparker@users.noreply.github.com> Co-authored-by: Orca Co-authored-by: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> --- .../components/NewWorkspaceComposerCard.tsx | 2 +- .../ContextualTourControl.tsx | 2 +- .../feature-wall/AiCommitPrSettingsSwitch.tsx | 2 +- .../components/feature-wall/KeepAwakeCard.tsx | 2 +- .../src/components/settings/AccountsPane.tsx | 2 +- .../components/settings/AgentAwakeSetting.tsx | 2 +- .../AutoRenameBranchFromWorkSetting.tsx | 2 +- .../settings/BrowserLinkRoutingSetting.tsx | 2 +- .../settings/BrowserUseEnableSwitch.tsx | 2 +- .../src/components/settings/CliSection.tsx | 2 +- .../settings/CommitMessageAiPane.tsx | 2 +- .../components/settings/ExperimentalPane.tsx | 6 +- .../src/components/settings/GitPane.tsx | 4 +- .../src/components/settings/InputPane.tsx | 2 +- .../settings/NotificationSettingToggle.tsx | 2 +- .../src/components/settings/PrivacyPane.tsx | 2 +- .../settings/SettingsFormControls.tsx | 2 +- .../settings/TerminalWindowSection.tsx | 4 +- .../VoiceDictationSettingsSection.tsx | 2 +- .../settings/WslCliRegistration.tsx | 2 +- .../components/sidebar/HostRemoveDialog.tsx | 2 +- .../stats/ClaudeUsageLoadingState.tsx | 2 +- .../src/components/stats/ClaudeUsagePane.tsx | 2 +- .../src/components/stats/CodexUsagePane.tsx | 2 +- .../components/stats/OpenCodeUsagePane.tsx | 2 +- .../TerminalQuickCommandAppendEnterSwitch.tsx | 2 +- .../toggle-switch-handle-alignment.test.ts | 231 ++++++++++++++++++ 27 files changed, 261 insertions(+), 30 deletions(-) create mode 100644 src/renderer/src/components/toggle-switch-handle-alignment.test.ts diff --git a/src/renderer/src/components/NewWorkspaceComposerCard.tsx b/src/renderer/src/components/NewWorkspaceComposerCard.tsx index 478494916dd..2447e45f21b 100644 --- a/src/renderer/src/components/NewWorkspaceComposerCard.tsx +++ b/src/renderer/src/components/NewWorkspaceComposerCard.tsx @@ -1219,7 +1219,7 @@ export default function NewWorkspaceComposerCard({ diff --git a/src/renderer/src/components/contextual-tours/ContextualTourControl.tsx b/src/renderer/src/components/contextual-tours/ContextualTourControl.tsx index 7898b579521..0a2186c5077 100644 --- a/src/renderer/src/components/contextual-tours/ContextualTourControl.tsx +++ b/src/renderer/src/components/contextual-tours/ContextualTourControl.tsx @@ -66,7 +66,7 @@ function AutoRenameBranchFromWorkControl(): JSX.Element { diff --git a/src/renderer/src/components/feature-wall/AiCommitPrSettingsSwitch.tsx b/src/renderer/src/components/feature-wall/AiCommitPrSettingsSwitch.tsx index 7655cd88ec5..751722d95ff 100644 --- a/src/renderer/src/components/feature-wall/AiCommitPrSettingsSwitch.tsx +++ b/src/renderer/src/components/feature-wall/AiCommitPrSettingsSwitch.tsx @@ -27,7 +27,7 @@ export function AiCommitPrSettingsSwitch({ diff --git a/src/renderer/src/components/feature-wall/KeepAwakeCard.tsx b/src/renderer/src/components/feature-wall/KeepAwakeCard.tsx index 6340d2f1b71..2a7b57adc67 100644 --- a/src/renderer/src/components/feature-wall/KeepAwakeCard.tsx +++ b/src/renderer/src/components/feature-wall/KeepAwakeCard.tsx @@ -38,7 +38,7 @@ export function KeepAwakeCard(props: { diff --git a/src/renderer/src/components/settings/AccountsPane.tsx b/src/renderer/src/components/settings/AccountsPane.tsx index d9db52dd9c6..0a3dd5ae786 100644 --- a/src/renderer/src/components/settings/AccountsPane.tsx +++ b/src/renderer/src/components/settings/AccountsPane.tsx @@ -1570,7 +1570,7 @@ export function AccountsPane({ > diff --git a/src/renderer/src/components/settings/AgentAwakeSetting.tsx b/src/renderer/src/components/settings/AgentAwakeSetting.tsx index 66e56c32aa2..28cd7cc0114 100644 --- a/src/renderer/src/components/settings/AgentAwakeSetting.tsx +++ b/src/renderer/src/components/settings/AgentAwakeSetting.tsx @@ -50,7 +50,7 @@ export function AgentAwakeSetting({ > diff --git a/src/renderer/src/components/settings/AutoRenameBranchFromWorkSetting.tsx b/src/renderer/src/components/settings/AutoRenameBranchFromWorkSetting.tsx index c4d520cbdb7..2cbdf227e0d 100644 --- a/src/renderer/src/components/settings/AutoRenameBranchFromWorkSetting.tsx +++ b/src/renderer/src/components/settings/AutoRenameBranchFromWorkSetting.tsx @@ -186,7 +186,7 @@ export function AutoRenameBranchFromWorkSetting({ > diff --git a/src/renderer/src/components/settings/BrowserLinkRoutingSetting.tsx b/src/renderer/src/components/settings/BrowserLinkRoutingSetting.tsx index 6b0885a86ef..b26c1e2fd78 100644 --- a/src/renderer/src/components/settings/BrowserLinkRoutingSetting.tsx +++ b/src/renderer/src/components/settings/BrowserLinkRoutingSetting.tsx @@ -54,7 +54,7 @@ export function BrowserLinkRoutingSetting({ > diff --git a/src/renderer/src/components/settings/BrowserUseEnableSwitch.tsx b/src/renderer/src/components/settings/BrowserUseEnableSwitch.tsx index cbb4c3361b4..28a071b1583 100644 --- a/src/renderer/src/components/settings/BrowserUseEnableSwitch.tsx +++ b/src/renderer/src/components/settings/BrowserUseEnableSwitch.tsx @@ -21,7 +21,7 @@ export function BrowserUseEnableSwitch({ > diff --git a/src/renderer/src/components/settings/CliSection.tsx b/src/renderer/src/components/settings/CliSection.tsx index 41984fe2038..86a5c89f346 100644 --- a/src/renderer/src/components/settings/CliSection.tsx +++ b/src/renderer/src/components/settings/CliSection.tsx @@ -299,7 +299,7 @@ export function CliSection({ > diff --git a/src/renderer/src/components/settings/CommitMessageAiPane.tsx b/src/renderer/src/components/settings/CommitMessageAiPane.tsx index e24fc97ee40..13f1b10cf52 100644 --- a/src/renderer/src/components/settings/CommitMessageAiPane.tsx +++ b/src/renderer/src/components/settings/CommitMessageAiPane.tsx @@ -209,7 +209,7 @@ export function CommitMessageAiPane({ > diff --git a/src/renderer/src/components/settings/ExperimentalPane.tsx b/src/renderer/src/components/settings/ExperimentalPane.tsx index 62d8420be13..9760591d477 100644 --- a/src/renderer/src/components/settings/ExperimentalPane.tsx +++ b/src/renderer/src/components/settings/ExperimentalPane.tsx @@ -99,7 +99,7 @@ export function ExperimentalPane({ > @@ -144,7 +144,7 @@ export function ExperimentalPane({ > @@ -203,7 +203,7 @@ export function ExperimentalPane({ > diff --git a/src/renderer/src/components/settings/GitPane.tsx b/src/renderer/src/components/settings/GitPane.tsx index da1a34c3fe8..490cef11e36 100644 --- a/src/renderer/src/components/settings/GitPane.tsx +++ b/src/renderer/src/components/settings/GitPane.tsx @@ -291,7 +291,7 @@ export function GitPane({ > @@ -385,7 +385,7 @@ export function GitPane({ > diff --git a/src/renderer/src/components/settings/InputPane.tsx b/src/renderer/src/components/settings/InputPane.tsx index 55094ec4a7a..125e90cc003 100644 --- a/src/renderer/src/components/settings/InputPane.tsx +++ b/src/renderer/src/components/settings/InputPane.tsx @@ -70,7 +70,7 @@ export function InputPane({ settings, updateSettings }: InputPaneProps): React.J > diff --git a/src/renderer/src/components/settings/NotificationSettingToggle.tsx b/src/renderer/src/components/settings/NotificationSettingToggle.tsx index 4de95f53b3e..93502392487 100644 --- a/src/renderer/src/components/settings/NotificationSettingToggle.tsx +++ b/src/renderer/src/components/settings/NotificationSettingToggle.tsx @@ -39,7 +39,7 @@ export function NotificationSettingToggle({ > diff --git a/src/renderer/src/components/settings/PrivacyPane.tsx b/src/renderer/src/components/settings/PrivacyPane.tsx index f2334e8676e..8a096f8c2c1 100644 --- a/src/renderer/src/components/settings/PrivacyPane.tsx +++ b/src/renderer/src/components/settings/PrivacyPane.tsx @@ -127,7 +127,7 @@ export function PrivacyPane({ settings }: PrivacyPaneProps): React.JSX.Element { > diff --git a/src/renderer/src/components/settings/SettingsFormControls.tsx b/src/renderer/src/components/settings/SettingsFormControls.tsx index a2d57a6ac24..27bcf7acc12 100644 --- a/src/renderer/src/components/settings/SettingsFormControls.tsx +++ b/src/renderer/src/components/settings/SettingsFormControls.tsx @@ -52,7 +52,7 @@ export function SettingsSwitch({ diff --git a/src/renderer/src/components/settings/TerminalWindowSection.tsx b/src/renderer/src/components/settings/TerminalWindowSection.tsx index 0cd00060223..3ca33d25c39 100644 --- a/src/renderer/src/components/settings/TerminalWindowSection.tsx +++ b/src/renderer/src/components/settings/TerminalWindowSection.tsx @@ -133,7 +133,7 @@ export function TerminalWindowSection({ > @@ -270,7 +270,7 @@ export function TerminalWindowSection({ diff --git a/src/renderer/src/components/settings/VoiceDictationSettingsSection.tsx b/src/renderer/src/components/settings/VoiceDictationSettingsSection.tsx index f148e631dc4..3747781b993 100644 --- a/src/renderer/src/components/settings/VoiceDictationSettingsSection.tsx +++ b/src/renderer/src/components/settings/VoiceDictationSettingsSection.tsx @@ -51,7 +51,7 @@ export function VoiceDictationSettingsSection({ > diff --git a/src/renderer/src/components/settings/WslCliRegistration.tsx b/src/renderer/src/components/settings/WslCliRegistration.tsx index 7d01d96d58b..586d306846e 100644 --- a/src/renderer/src/components/settings/WslCliRegistration.tsx +++ b/src/renderer/src/components/settings/WslCliRegistration.tsx @@ -198,7 +198,7 @@ export function WslCliRegistration({ > diff --git a/src/renderer/src/components/sidebar/HostRemoveDialog.tsx b/src/renderer/src/components/sidebar/HostRemoveDialog.tsx index 97135f97eeb..d99f5e860e9 100644 --- a/src/renderer/src/components/sidebar/HostRemoveDialog.tsx +++ b/src/renderer/src/components/sidebar/HostRemoveDialog.tsx @@ -251,7 +251,7 @@ export function HostRemoveDialog({ diff --git a/src/renderer/src/components/stats/ClaudeUsageLoadingState.tsx b/src/renderer/src/components/stats/ClaudeUsageLoadingState.tsx index e459ab9c6ba..59c378cc8c3 100644 --- a/src/renderer/src/components/stats/ClaudeUsageLoadingState.tsx +++ b/src/renderer/src/components/stats/ClaudeUsageLoadingState.tsx @@ -21,7 +21,7 @@ export function ClaudeUsageLoadingState({
- +
diff --git a/src/renderer/src/components/stats/ClaudeUsagePane.tsx b/src/renderer/src/components/stats/ClaudeUsagePane.tsx index 32536628f8d..dfd83b6663c 100644 --- a/src/renderer/src/components/stats/ClaudeUsagePane.tsx +++ b/src/renderer/src/components/stats/ClaudeUsagePane.tsx @@ -233,7 +233,7 @@ export function ClaudeUsagePane(): React.JSX.Element { onClick={() => handleSetEnabled(false)} className="relative inline-flex h-5 w-9 shrink-0 cursor-pointer items-center rounded-full border border-transparent bg-foreground transition-colors" > - +
diff --git a/src/renderer/src/components/stats/CodexUsagePane.tsx b/src/renderer/src/components/stats/CodexUsagePane.tsx index 2ebd6913d44..4ae5074de51 100644 --- a/src/renderer/src/components/stats/CodexUsagePane.tsx +++ b/src/renderer/src/components/stats/CodexUsagePane.tsx @@ -235,7 +235,7 @@ export function CodexUsagePane(): React.JSX.Element { onClick={() => handleSetEnabled(false)} className="relative inline-flex h-5 w-9 shrink-0 cursor-pointer items-center rounded-full border border-transparent bg-foreground transition-colors" > - +
diff --git a/src/renderer/src/components/stats/OpenCodeUsagePane.tsx b/src/renderer/src/components/stats/OpenCodeUsagePane.tsx index c09f2a5a179..b70b065934e 100644 --- a/src/renderer/src/components/stats/OpenCodeUsagePane.tsx +++ b/src/renderer/src/components/stats/OpenCodeUsagePane.tsx @@ -246,7 +246,7 @@ export function OpenCodeUsagePane(): React.JSX.Element { onClick={() => handleSetEnabled(false)} className="relative inline-flex h-5 w-9 shrink-0 cursor-pointer items-center rounded-full border border-transparent bg-foreground transition-colors" > - + diff --git a/src/renderer/src/components/terminal-quick-commands/TerminalQuickCommandAppendEnterSwitch.tsx b/src/renderer/src/components/terminal-quick-commands/TerminalQuickCommandAppendEnterSwitch.tsx index ad45db4b08d..0ddd6222465 100644 --- a/src/renderer/src/components/terminal-quick-commands/TerminalQuickCommandAppendEnterSwitch.tsx +++ b/src/renderer/src/components/terminal-quick-commands/TerminalQuickCommandAppendEnterSwitch.tsx @@ -39,7 +39,7 @@ export function TerminalQuickCommandAppendEnterSwitch({ > diff --git a/src/renderer/src/components/toggle-switch-handle-alignment.test.ts b/src/renderer/src/components/toggle-switch-handle-alignment.test.ts new file mode 100644 index 00000000000..b0a5574dd27 --- /dev/null +++ b/src/renderer/src/components/toggle-switch-handle-alignment.test.ts @@ -0,0 +1,231 @@ +import { readdirSync, readFileSync, statSync } from 'node:fs' +import { relative, resolve } from 'node:path' +// TypeScript 7 is a native CLI; AST tests still need the legacy JavaScript API. +import ts from 'typescript-api' +import { describe, expect, it } from 'vitest' + +// Why: the h-5 w-9 switch is hand-rolled at every call site instead of living in a +// primitive, so the handle offset drifts. The 1px-bordered track leaves a 34px content +// box for the 14px (`size-3.5`) handle, and `translate-x-0.5` insets the off state by +// 2px, so a symmetric on state is 34 - 14 - 2 = 18px (`translate-x-4.5`). + +const RENDERER_ROOT = resolve('src/renderer/src') + +const OFF_STATE = 'translate-x-0.5' +const ON_STATE = 'translate-x-4.5' + +/** The track geometry the arithmetic above is derived from; other switch sizes are not ours to check. */ +const TRACK = ['h-5', 'w-9'] + +/** The handle's 14px size, in both the shorthand and long-hand spellings in use. */ +const HANDLE_SIZE = [['size-3.5'], ['h-3.5', 'w-3.5']] + +/** Every current handle is the round knob; this is what separates it from a size-3.5 icon in the same track. */ +const HANDLE_SHAPE = 'rounded-full' + +/** Any horizontal translate — negative, arbitrary, variant-prefixed or named — so no respelling reads as "no offset". */ +const OFFSET = /(?:^|:)-?translate-x-\S+$/ + +/** Anything that moves the track off the 34px content box the 18px on-state is derived from. */ +const BORDER_WIDTH = /^border(?:-[xytrbles])?-(?:\d+|\[)/ +const TRACK_PADDING = /^p[xytrbles]?-(?:\d|\[)/ + +function collectSourceFiles(dir: string, files: string[] = []): string[] { + for (const name of readdirSync(dir)) { + const filePath = resolve(dir, name) + if (statSync(filePath).isDirectory()) { + collectSourceFiles(filePath, files) + } else if (name.endsWith('.tsx') && !name.endsWith('.test.tsx')) { + files.push(filePath) + } + } + return files +} + +/** Class tokens anywhere under a node, flattening the ternaries, template holes and `cn()` args a className is built from. */ +function tokensIn(node: ts.Node): string[] { + const tokens: string[] = [] + const read = (child: ts.Node): void => { + if (ts.isStringLiteral(child) || ts.isTemplateLiteralToken(child)) { + tokens.push(...child.text.split(/\s+/).filter(Boolean)) + } + ts.forEachChild(child, read) + } + read(node) + return tokens +} + +/** Reading the className attribute's own subtree keeps a child element's classes out of its parent's token set. */ +function classNameOf(element: ts.JsxOpeningLikeElement): ts.Node | undefined { + const className = element.attributes.properties.find( + (property): property is ts.JsxAttribute => + ts.isJsxAttribute(property) && property.name.getText() === 'className' + ) + return className?.initializer +} + +function hasAll(tokens: string[], required: string[]): boolean { + return required.every((token) => tokens.includes(token)) +} + +/** + * The offsets each side of a conditional className contributes. A handle that toggles is + * two-state by construction, so this tells a deliberately single-state knob apart from one + * that lost a branch — which a flat token list cannot. + */ +function offsetBranches(node: ts.Node): [string[], string[]][] { + const branches: [string[], string[]][] = [] + const read = (child: ts.Node): void => { + if (ts.isConditionalExpression(child)) { + const whenTrue = tokensIn(child.whenTrue).filter((token) => OFFSET.test(token)) + const whenFalse = tokensIn(child.whenFalse).filter((token) => OFFSET.test(token)) + if (whenTrue.length > 0 || whenFalse.length > 0) { + branches.push([whenTrue, whenFalse]) + return + } + } + ts.forEachChild(child, read) + } + read(node) + return branches +} + +function trackDrift(tokens: string[]): string | undefined { + if (!tokens.includes('border')) { + return 'no 1px border' + } + if (tokens.includes('box-content')) { + return 'box-content' + } + return tokens.find((token) => BORDER_WIDTH.test(token) || TRACK_PADDING.test(token)) +} + +type Handle = { at: string; offsets: string[] } +type Toggle = { at: string; branches: [string[], string[]] } +type Scan = { handles: Handle[]; toggles: Toggle[]; drifted: string[]; knobless: string[] } + +/** + * Handles are found *inside* a track element rather than by scanning source text forward, so + * neither a sibling element nor a same-file icon can be mistaken for the knob. + */ +function collectFile(filePath: string, source: string, scan: Scan): void { + const sourceFile = ts.createSourceFile( + filePath, + source, + ts.ScriptTarget.Latest, + true, + ts.ScriptKind.TSX + ) + + const at = (node: ts.Node): string => { + const { line } = sourceFile.getLineAndCharacterOfPosition(node.getStart(sourceFile)) + return `${relative(RENDERER_ROOT, filePath)}:${line + 1}` + } + + const readHandles = (node: ts.Node): void => { + if (ts.isJsxOpeningLikeElement(node)) { + const className = classNameOf(node) + const tokens = className ? tokensIn(className) : [] + if ( + className && + HANDLE_SIZE.some((size) => hasAll(tokens, size)) && + tokens.includes(HANDLE_SHAPE) + ) { + scan.handles.push({ at: at(node), offsets: tokens.filter((token) => OFFSET.test(token)) }) + for (const branches of offsetBranches(className)) { + scan.toggles.push({ at: at(node), branches }) + } + } + } + ts.forEachChild(node, readHandles) + } + + const visit = (node: ts.Node): void => { + if (ts.isJsxOpeningLikeElement(node)) { + const className = classNameOf(node) + const tokens = className ? tokensIn(className) : [] + if (hasAll(tokens, TRACK)) { + const drift = trackDrift(tokens) + if (drift) { + scan.drifted.push(`${at(node)}: ${drift}`) + } + // The knob lives in the track's subtree; a self-closing track has no subtree to walk. + const found = scan.handles.length + if (ts.isJsxOpeningElement(node)) { + readHandles(node.parent) + } + // A track whose knob moved into a child component would otherwise leave the scan silently. + if (scan.handles.length === found) { + scan.knobless.push(at(node)) + } + return + } + } + ts.forEachChild(node, visit) + } + + visit(sourceFile) +} + +let cached: Scan | undefined + +/** Parsing every gated file is the expensive part, so all three assertions share one scan. */ +function scanRenderer(): Scan { + if (!cached) { + const scan: Scan = { handles: [], toggles: [], drifted: [], knobless: [] } + + for (const filePath of collectSourceFiles(RENDERER_ROOT)) { + const source = readFileSync(filePath, 'utf8') + if (TRACK.every((token) => source.includes(token))) { + collectFile(filePath, source, scan) + } + } + + // A track nested inside another h-5 w-9 element would otherwise be collected twice. + const seen = new Set() + scan.handles = scan.handles.filter((handle) => !seen.has(handle.at) && seen.add(handle.at)) + cached = scan + } + + // A scan that stops finding switches has lost its bounds, not proven the tree clean. The + // floor only has to catch a collapse — `knobless` and `drifted` catch the structural cases — + // so it sits well under the ~34 handles in the tree rather than pinning the current count. + expect(cached.handles.length, 'the switch scan lost its bounds').toBeGreaterThan(25) + + return cached +} + +describe('toggle switch handle alignment', () => { + it('insets the handle by 2px at both ends of the track', () => { + const offenders = scanRenderer().handles.flatMap((handle) => + handle.offsets.length === 0 + ? [`${handle.at}: no translate-x offset`] + : handle.offsets + .filter((offset) => offset !== OFF_STATE && offset !== ON_STATE) + .map((offset) => `${handle.at}: ${offset}`) + ) + + expect(offenders).toEqual([]) + }) + + it('gives every toggling handle both ends of the pair', () => { + const pairs = (on: string[], off: string[]): boolean => + on.length === 1 && on[0] === ON_STATE && off.length === 1 && off[0] === OFF_STATE + + const offenders = scanRenderer() + .toggles.filter(({ branches: [a, b] }) => !pairs(a, b) && !pairs(b, a)) + .map( + ({ at, branches }) => `${at}: ${branches.map((side) => side.join(' ') || '—').join(' / ')}` + ) + + expect(offenders).toEqual([]) + }) + + it('keeps every track on the geometry the 18px on-state is derived from', () => { + expect(scanRenderer().drifted).toEqual([]) + }) + + it('keeps every track a knob the scan can see', () => { + expect(scanRenderer().knobless).toEqual([]) + }) +}) From b075a95b06307abac4bf42d175cf2c0b9439737f Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Sun, 9 Aug 2026 18:40:01 -0700 Subject: [PATCH 08/12] Strip liveness gate from AI Vault session delete (#13279) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Strip liveness gate from AI Vault session delete Delete now requires only path validation + user confirmation — no process roster, no liveness check, no quiescence, no ownership ledger. Co-authored-by: Orca * Remove obsolete AI Vault liveness delete reliability gate Session delete no longer checks process liveness, so drop the manifest entry that still referenced the deleted test files. * minor fix --------- Co-authored-by: Orca --- config/reliability-gates.jsonc | 99 +------ .../session-delete-liveness.repro.test.ts | 179 ------------ src/main/ai-vault/session-delete.test.ts | 10 +- src/main/ai-vault/session-delete.ts | 31 +-- src/main/ai-vault/session-liveness.test.ts | 263 ------------------ src/main/ai-vault/session-liveness.ts | 187 ------------- src/main/ipc/ai-vault-delete.ts | 25 +- src/main/ipc/ai-vault.test.ts | 6 +- src/main/ipc/ai-vault.ts | 6 +- src/main/ipc/register-core-handlers.ts | 3 +- src/main/runtime/orca-runtime.ts | 75 +---- .../right-sidebar/AiVaultSessionRow.tsx | 2 +- .../ai-vault-session-deletability.test.ts | 23 -- .../ai-vault-session-deletability.ts | 18 +- 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 +- src/shared/ai-vault-session-deletion.ts | 7 +- .../terminal-macos-system-key-remap.spec.ts | 7 +- 21 files changed, 32 insertions(+), 924 deletions(-) delete mode 100644 src/main/ai-vault/session-delete-liveness.repro.test.ts delete mode 100644 src/main/ai-vault/session-liveness.test.ts delete mode 100644 src/main/ai-vault/session-liveness.ts diff --git a/config/reliability-gates.jsonc b/config/reliability-gates.jsonc index 3e4a32a0cfb..3993a178f38 100644 --- a/config/reliability-gates.jsonc +++ b/config/reliability-gates.jsonc @@ -1,6 +1,6 @@ { "schemaVersion": 1, - "updatedAt": "2026-08-07", + "updatedAt": "2026-08-09", "policy": { "maturityLevels": ["experimental", "soak", "blocking", "accepted-gap", "deprecated"], "blockingPromotion": { @@ -10,103 +10,6 @@ } }, "gates": [ - { - "id": "ai-vault.live-session-delete-authority", - "title": "AI Vault deletion fails closed against owning-runtime session liveness", - "maturity": "experimental", - "protection": "partial", - "owner": "ai-vault-runtime", - "layer": "main-process-filesystem-mutation", - "surfaces": [ - "AI Vault transcript deletion", - "local and WSL agent sessions", - "paired-client-owned host sessions" - ], - "platforms": ["macos", "linux", "windows"], - "providers": ["local", "wsl", "ssh", "paired-runtime"], - "coveredPlatforms": ["macos", "linux", "windows"], - "coveredProviders": ["local", "wsl", "ssh", "paired-runtime"], - "coverageNotes": "Platform-independent contracts cover local and WSL authority, paired-owned and unmanaged live sessions absent from renderer or managed-PTY state, identity retention across UI dismissal, inventory and foreground-inspection ordering, SSH exclusion, unavailable and ambiguous inventory, bounded inspection concurrency, oversized inventories, returned rejection, and real temporary-file survival. Live headed/headless paired-runtime, physical Windows/WSL, and Docker SSH journeys remain uncollected.", - "motivatingLinks": ["https://github.com/stablyai/orca/releases/tag/v1.4.177-rc.0"], - "invariant": "Main may delete a local AI transcript only after the owning runtime freshly proves that no live local or WSL provider-session identity exists, including exact hook identities absent from managed PTY inventory; live and unknown liveness preserve the transcript regardless of renderer freshness.", - "oracle": "Open confirmation against a stale empty renderer snapshot, transition the authoritative session to live before main authorization and during a controlled foreground-inspection barrier, and separately model paired and unmanaged local owners absent from renderer or managed-PTY state. Dismiss a live row and require its identity-only authority to remain. Require session-live or session-liveness-unknown rejection and the real temporary transcript to survive. Make process inventory unavailable or ambiguous and require unknown with the file preserved; prove known other sessions permit deletion and inspection concurrency stays at or below eight.", - "commands": [ - "pnpm exec vitest run --config config/vitest.config.ts src/main/ai-vault/session-delete-liveness.repro.test.ts src/main/ai-vault/session-liveness.test.ts src/main/agent-hooks/server-ai-vault-liveness.test.ts --reporter=dot" - ], - "testFiles": [ - "src/main/ai-vault/session-delete-liveness.repro.test.ts", - "src/main/ai-vault/session-liveness.test.ts", - "src/main/agent-hooks/server-ai-vault-liveness.test.ts" - ], - "assertionRefs": [ - { - "file": "src/main/ai-vault/session-delete-liveness.repro.test.ts", - "assertions": [ - "confirmation TOCTOU returns session-live and preserves the real transcript", - "paired ownership missing from the renderer snapshot returns session-live and preserves the real transcript", - "exact live local hook identity missing from managed PTY inventory returns unknown and preserves the real transcript", - "dismissed live status retained as identity-only authority returns unknown and preserves the real transcript", - "unknown authoritative liveness fails closed and preserves the real transcript" - ] - }, - { - "file": "src/main/ai-vault/session-liveness.test.ts", - "assertions": [ - "local, WSL, paired-runtime, and SSH host partitions resolve without local fallback", - "renderer session identity must match the identity parsed from the validated transcript", - "hook identity is captured after the controlled process-inventory barrier", - "hook identity is captured after the controlled foreground-inspection barrier", - "missing identity, unavailable inventory, unattributed ownership, and oversized inventory remain unknown", - "foreground inspection concurrency never exceeds eight and stops scheduling at the shared deadline" - ] - }, - { - "file": "src/main/agent-hooks/server-ai-vault-liveness.test.ts", - "assertions": [ - "user dismissal retains provider identity without retaining a visible turn-status row", - "restart hydration preserves dismissed identity as unconfirmed liveness-only authority" - ] - } - ], - "evidenceRuns": [ - { - "date": "2026-08-07", - "runner": "local", - "platform": "macos", - "command": "pnpm exec vitest run --config config/vitest.config.ts src/main/ai-vault/session-delete-liveness.repro.test.ts src/main/ai-vault/session-liveness.test.ts src/main/agent-hooks/server-ai-vault-liveness.test.ts --reporter=dot", - "result": "passed", - "durationSeconds": 1, - "summary": "Twenty deterministic deletion, identity-binding, inventory/inspection-ordering, dismissal/restart-retention, unmanaged-ownership, unknown-state, filesystem-survival, host-partition, deadline, and bounded-concurrency assertions passed." - } - ], - "runtimeBudget": { - "p95Seconds": 2, - "scope": "focused main-process liveness and temporary-filesystem contracts" - }, - "flakeHistory": { - "status": "unknown", - "evidence": "One local deterministic run is recorded; CI and soak history have not started." - }, - "redGreenEvidence": { - "status": "complete", - "evidence": "The byte-identical two-scenario oracle failed on v1.4.177-rc.0 (9e948fbdf4) and latest main (7a867f12aa): both returned deleted and both real transcripts were removed. On PR commit 8e2fcfd681, an exact live local hook identity absent from empty managed-PTY inventory returned deleted and removed the real transcript. Independent review then proved a session-other to session-live transition during foreground inspection returned not-live, and live-row dismissal erased the only external identity. The corrected candidate passed all 20 assertions, including restart hydration of dismissed identity; temporarily disabling only the original liveness decision made all three original deletion-safety tests red again." - }, - "performanceBudget": { - "required": true, - "evidence": "Deletion parses only the validated target through the incremental parse cache, performs one local-provider process listing, makes no SSH or paired-runtime request, caps inventory at 512 PTYs, bounds foreground inspections to eight concurrent calls under one shared three-second fail-closed deadline, and adds no polling or background work." - }, - "promotionCriteria": [ - "Collect CI soak history for the deterministic gate.", - "Collect headed and headless paired-runtime deletion-refusal evidence.", - "Collect physical Windows/WSL and Docker SSH host-partition evidence." - ], - "knownGaps": [ - "No live headed/headless paired-runtime journey was run.", - "No physical Windows/WSL or Docker SSH journey was run.", - "The authorization check cannot make an external provider process start atomic with filesystem trashing; it closes the reported renderer-confirmation window at the main mutation boundary." - ], - "demotionRule": "Keep experimental or demote if live or unknown ownership can reach trashing, remote ownership falls back locally, process inspection becomes unbounded, or the deterministic filesystem oracle flakes without a product or harness defect." - }, { "id": "editor.restored-sibling-owner-reparent", "title": "Restored sibling tabs migrate filesystem authority before becoming editable", diff --git a/src/main/ai-vault/session-delete-liveness.repro.test.ts b/src/main/ai-vault/session-delete-liveness.repro.test.ts deleted file mode 100644 index 6e4928a84ea..00000000000 --- a/src/main/ai-vault/session-delete-liveness.repro.test.ts +++ /dev/null @@ -1,179 +0,0 @@ -import { existsSync } from 'node:fs' -import { mkdtemp, rm, writeFile } from 'node:fs/promises' -import { tmpdir } from 'node:os' -import { join } from 'node:path' -import { afterEach, describe, expect, it, vi } from 'vitest' -import type { AgentStatusIpcPayload } from '../../shared/agent-status-types' -import type { AiVaultDeleteSessionResult } from '../../shared/ai-vault-session-deletion' -import type { ValidateAiVaultSessionDeleteTargetArgs } from './session-delete-target' - -const { trashItemMock } = vi.hoisted(() => ({ trashItemMock: vi.fn() })) - -vi.mock('electron', () => ({ shell: { trashItem: trashItemMock } })) -vi.mock('../wsl-unc-delete', () => ({ tryDeleteWslUncPath: vi.fn().mockResolvedValue(false) })) - -import { deleteAiVaultSessionFile } from './session-delete' -import { resolveAiVaultSessionLiveness } from './session-liveness' - -type AiVaultSessionLiveness = 'live' | 'not-live' | 'unknown' - -type DeleteWithLiveness = ( - args: ValidateAiVaultSessionDeleteTargetArgs & { sessionId: string }, - deps: { getSessionLiveness: () => Promise } -) => Promise - -const deleteWithLiveness = deleteAiVaultSessionFile as unknown as DeleteWithLiveness -const fixtureRoots: string[] = [] - -async function createTranscript(): Promise<{ filePath: string; root: string }> { - const root = await mkdtemp(join(tmpdir(), 'orca-ai-vault-live-delete-repro-')) - fixtureRoots.push(root) - const filePath = join(root, 'session-live.json') - await writeFile(filePath, '{"sessionId":"session-live"}\n') - return { filePath, root } -} - -async function attemptDelete( - getSessionLiveness: () => Promise -): Promise<{ filePath: string; result: AiVaultDeleteSessionResult }> { - const { filePath, root } = await createTranscript() - const result = await deleteWithLiveness( - { - agent: 'gemini', - sessionId: 'session-live', - filePath, - executionHostId: 'local', - rootOptions: { geminiSessionsDir: root } - }, - { getSessionLiveness } - ) - return { filePath, result } -} - -describe('live AI Vault session delete safety invariant', () => { - afterEach(async () => { - trashItemMock.mockReset() - await Promise.all( - fixtureRoots.splice(0).map((root) => rm(root, { recursive: true, force: true })) - ) - }) - - it('survives when the session becomes live after confirmation but before main authorization', async () => { - trashItemMock.mockImplementation((path: string) => rm(path)) - const rendererPaneSnapshot = new Set() - expect(rendererPaneSnapshot.has('session-live')).toBe(false) - - let authoritativeLiveness: AiVaultSessionLiveness = 'not-live' - const confirmationOpened = true - authoritativeLiveness = 'live' - expect(confirmationOpened).toBe(true) - - const { filePath, result } = await attemptDelete(async () => authoritativeLiveness) - - expect.soft(result).toEqual({ outcome: 'rejected', agent: 'gemini', reason: 'session-live' }) - expect.soft(existsSync(filePath), 'live transcript must survive on disk').toBe(true) - }) - - it('survives when a paired owner is absent from the renderer pane snapshot', async () => { - trashItemMock.mockImplementation((path: string) => rm(path)) - const rendererPaneSnapshot = new Set() - const authoritativeOwner = { - runtimeId: 'host-runtime', - connectionId: 'paired-client', - generation: 'generation-1', - sessionId: 'session-live', - liveness: 'live' as const - } - expect(rendererPaneSnapshot.has(authoritativeOwner.sessionId)).toBe(false) - - const { filePath, result } = await attemptDelete(async () => authoritativeOwner.liveness) - - expect.soft(result).toEqual({ outcome: 'rejected', agent: 'gemini', reason: 'session-live' }) - expect.soft(existsSync(filePath), 'externally owned transcript must survive on disk').toBe(true) - }) - - it('survives an exact live local hook identity absent from managed PTY inventory', async () => { - trashItemMock.mockImplementation((path: string) => rm(path)) - const inspectForegroundProcess = vi.fn() - const externalLiveStatus: AgentStatusIpcPayload = { - paneKey: 'tab:external-live', - terminalHandle: 'term_external-live', - connectionId: null, - receivedAt: 1, - stateStartedAt: 1, - state: 'working', - prompt: 'test', - agentType: 'gemini', - providerSession: { key: 'session_id', id: 'session-live' } - } - - const { filePath, result } = await attemptDelete(() => - resolveAiVaultSessionLiveness( - { agent: 'gemini', sessionId: 'session-live' }, - { - listProcesses: async () => [], - getStatusSnapshot: () => [externalLiveStatus], - inspectForegroundProcess, - getStatusPtyId: () => 'external-live', - getAgentHint: () => null - } - ) - ) - - expect.soft(result).toEqual({ - outcome: 'rejected', - agent: 'gemini', - reason: 'session-liveness-unknown' - }) - expect.soft(existsSync(filePath), 'external live transcript must survive on disk').toBe(true) - expect(inspectForegroundProcess).not.toHaveBeenCalled() - }) - - it('survives a dismissed live row retained as liveness-only identity', async () => { - trashItemMock.mockImplementation((path: string) => rm(path)) - const retainedIdentity: AgentStatusIpcPayload = { - paneKey: 'tab:external-live', - connectionId: null, - receivedAt: 1, - stateStartedAt: 1, - state: 'working', - prompt: 'test', - agentType: 'gemini', - providerSession: { key: 'session_id', id: 'session-live' }, - providerSessionOnly: true - } - - const { filePath, result } = await attemptDelete(() => - resolveAiVaultSessionLiveness( - { agent: 'gemini', sessionId: 'session-live' }, - { - listProcesses: async () => [], - getStatusSnapshot: () => [retainedIdentity], - inspectForegroundProcess: vi.fn(), - getStatusPtyId: () => null, - getAgentHint: () => null - } - ) - ) - - expect.soft(result).toEqual({ - outcome: 'rejected', - agent: 'gemini', - reason: 'session-liveness-unknown' - }) - expect.soft(existsSync(filePath), 'dismissed live transcript must survive on disk').toBe(true) - }) - - it('fails closed when authoritative liveness is unavailable', async () => { - trashItemMock.mockImplementation((path: string) => rm(path)) - - const { filePath, result } = await attemptDelete(async () => 'unknown') - - expect(result).toEqual({ - outcome: 'rejected', - agent: 'gemini', - reason: 'session-liveness-unknown' - }) - expect(existsSync(filePath), 'unknown-liveness transcript must survive on disk').toBe(true) - }) -}) diff --git a/src/main/ai-vault/session-delete.test.ts b/src/main/ai-vault/session-delete.test.ts index 3c6f86f43a7..cacb0dbbda5 100644 --- a/src/main/ai-vault/session-delete.test.ts +++ b/src/main/ai-vault/session-delete.test.ts @@ -33,21 +33,13 @@ vi.mock('../wsl-unc-delete', () => ({ WslDeleteValidationError: WslDeleteValidationErrorMock })) -import { deleteAiVaultSessionFile as deleteAiVaultSessionFileWithLiveness } from './session-delete' +import { deleteAiVaultSessionFile } from './session-delete' const HOME = join('/tmp', 'orca-ai-vault-delete-exec-fixture-home') const GEMINI_ROOT = join(HOME, '.gemini', 'tmp') const CLAUDE_ROOT = join(HOME, '.claude', 'projects') const ROVO_ROOT = join(HOME, '.rovodev', 'sessions') -function deleteAiVaultSessionFile( - args: Parameters[0] -) { - return deleteAiVaultSessionFileWithLiveness(args, { - getSessionLiveness: async () => 'not-live' - }) -} - function enoent(): NodeJS.ErrnoException { const error = new Error('not found') as NodeJS.ErrnoException error.code = 'ENOENT' diff --git a/src/main/ai-vault/session-delete.ts b/src/main/ai-vault/session-delete.ts index fb6876904ae..4676749068b 100644 --- a/src/main/ai-vault/session-delete.ts +++ b/src/main/ai-vault/session-delete.ts @@ -2,10 +2,8 @@ import { lstat, realpath } from 'node:fs/promises' import { shell } from 'electron' import type { AiVaultDeleteSessionResult, - AiVaultSessionDeleteRemoval, - AiVaultSessionLiveness + AiVaultSessionDeleteRemoval } from '../../shared/ai-vault-session-deletion' -import type { AiVaultAgent } from '../../shared/ai-vault-types' import { isPathInsideOrEqual } from '../../shared/cross-platform-path' import { validateAiVaultSessionDeleteTarget, @@ -19,14 +17,7 @@ import { isENOENT } from '../ipc/filesystem-auth' // Never throws: IPC payloads are untyped at runtime, so a bad input and an fs // error both resolve to a discriminated result the handler can render. export async function deleteAiVaultSessionFile( - args: ValidateAiVaultSessionDeleteTargetArgs & { sessionId?: string }, - deps: { - getSessionLiveness: (target: { - agent: AiVaultAgent - sessionId: string | undefined - filePath: string - }) => Promise - } + args: ValidateAiVaultSessionDeleteTargetArgs & { sessionId?: string } ): Promise { const validation = validateAiVaultSessionDeleteTarget(args) if (!validation.allowed) { @@ -34,24 +25,6 @@ export async function deleteAiVaultSessionFile( } const { agent, removals } = validation - let liveness: AiVaultSessionLiveness = 'unknown' - try { - liveness = await deps.getSessionLiveness({ - agent, - sessionId: args.sessionId, - filePath: validation.resolvedPath - }) - } catch { - // Inspection failure is unknown, never permission to delete. - } - if (liveness !== 'not-live') { - return { - outcome: 'rejected', - agent, - reason: liveness === 'live' ? 'session-live' : 'session-liveness-unknown' - } - } - try { for (const removal of removals) { const rejection = await removeOne(removal) diff --git a/src/main/ai-vault/session-liveness.test.ts b/src/main/ai-vault/session-liveness.test.ts deleted file mode 100644 index c4eb2a70328..00000000000 --- a/src/main/ai-vault/session-liveness.test.ts +++ /dev/null @@ -1,263 +0,0 @@ -import { mkdtemp, rm, writeFile } from 'node:fs/promises' -import { tmpdir } from 'node:os' -import { join } from 'node:path' -import { describe, expect, it, vi } from 'vitest' -import type { AgentStatusIpcPayload } from '../../shared/agent-status-types' -import type { PtyProcessInfo } from '../providers/pty-process-info' -import { - readAiVaultSessionIdentity, - resolveAiVaultSessionLiveness, - type AiVaultSessionLivenessDependencies -} from './session-liveness' - -function processInfo(id: string, terminalHandle = `term_${id}`): PtyProcessInfo { - return { id, terminalHandle, cwd: '/workspace', title: 'terminal' } -} - -function status(args: { - sessionId: string - ptyId: string - connectionId?: string | null -}): AgentStatusIpcPayload { - return { - paneKey: `tab:${args.ptyId}`, - terminalHandle: `term_${args.ptyId}`, - connectionId: args.connectionId ?? null, - receivedAt: 1, - stateStartedAt: 1, - state: 'working', - prompt: 'test', - agentType: 'gemini', - providerSession: { key: 'session_id', id: args.sessionId } - } -} - -function dependencies( - overrides: Partial = {} -): AiVaultSessionLivenessDependencies { - return { - listProcesses: async () => [], - getStatusSnapshot: () => [], - inspectForegroundProcess: async () => ({ available: true, process: 'zsh' }), - getStatusPtyId: (row) => row.terminalHandle?.slice('term_'.length) ?? null, - getAgentHint: () => null, - ...overrides - } -} - -describe('resolveAiVaultSessionLiveness', () => { - it('binds renderer identity to the session parsed from the validated transcript', async () => { - const root = await mkdtemp(join(tmpdir(), 'orca-ai-vault-delete-identity-')) - const filePath = join(root, 'session.json') - await writeFile( - filePath, - JSON.stringify({ - sessionId: 'authoritative-session', - startTime: '2026-08-07T00:00:00.000Z', - lastUpdated: '2026-08-07T00:01:00.000Z', - messages: [{ type: 'user', content: 'test' }] - }) - ) - try { - await expect( - readAiVaultSessionIdentity({ - agent: 'gemini', - sessionId: 'spoofed-session', - filePath - }) - ).resolves.toEqual({ outcome: 'unknown' }) - await expect( - readAiVaultSessionIdentity({ - agent: 'gemini', - sessionId: 'authoritative-session', - filePath - }) - ).resolves.toEqual({ outcome: 'found', sessionId: 'authoritative-session' }) - } finally { - await rm(root, { recursive: true, force: true }) - } - }) - - it('finds a paired-owned live session without consulting a renderer pane snapshot', async () => { - const deps = dependencies({ - listProcesses: async () => [processInfo('paired-pty')], - getStatusSnapshot: () => [status({ sessionId: 'session-live', ptyId: 'paired-pty' })], - inspectForegroundProcess: async () => ({ available: true, process: 'gemini' }) - }) - - await expect( - resolveAiVaultSessionLiveness({ agent: 'gemini', sessionId: 'session-live' }, deps) - ).resolves.toBe('live') - }) - - it('reads hook identity after a controlled process-inventory barrier', async () => { - let releaseInventory: (processes: PtyProcessInfo[]) => void = () => {} - const inventory = new Promise((resolve) => { - releaseInventory = resolve - }) - let statuses = [status({ sessionId: 'session-other', ptyId: 'paired-pty' })] - const deps = dependencies({ - listProcesses: () => inventory, - getStatusSnapshot: () => statuses, - inspectForegroundProcess: async () => ({ available: true, process: 'gemini' }) - }) - - const liveness = resolveAiVaultSessionLiveness( - { agent: 'gemini', sessionId: 'session-live' }, - deps - ) - statuses = [status({ sessionId: 'session-live', ptyId: 'paired-pty' })] - releaseInventory([processInfo('paired-pty')]) - - await expect(liveness).resolves.toBe('live') - }) - - it('reads hook identity after a controlled foreground-inspection barrier', async () => { - let markInspectionStarted: () => void = () => {} - const inspectionStarted = new Promise((resolve) => { - markInspectionStarted = resolve - }) - let releaseInspection: (inspection: { - available: boolean - process: string | null - }) => void = () => {} - const inspection = new Promise<{ available: boolean; process: string | null }>((resolve) => { - releaseInspection = resolve - }) - let statuses = [status({ sessionId: 'session-other', ptyId: 'managed-pty' })] - const deps = dependencies({ - listProcesses: async () => [processInfo('managed-pty')], - getStatusSnapshot: () => statuses, - inspectForegroundProcess: () => { - markInspectionStarted() - return inspection - } - }) - - const liveness = resolveAiVaultSessionLiveness( - { agent: 'gemini', sessionId: 'session-live' }, - deps - ) - await inspectionStarted - statuses = [status({ sessionId: 'session-live', ptyId: 'managed-pty' })] - releaseInspection({ available: true, process: 'gemini' }) - - await expect(liveness).resolves.toBe('live') - }) - - it('treats WSL hook ownership as local authority', async () => { - const deps = dependencies({ - listProcesses: async () => [processInfo('wsl-pty')], - getStatusSnapshot: () => [ - status({ sessionId: 'session-live', ptyId: 'wsl-pty', connectionId: 'wsl:Ubuntu' }) - ], - inspectForegroundProcess: async () => ({ available: true, process: 'gemini' }) - }) - - await expect( - resolveAiVaultSessionLiveness({ agent: 'gemini', sessionId: 'session-live' }, deps) - ).resolves.toBe('live') - }) - - it('does not use SSH identity as local liveness evidence', async () => { - const deps = dependencies({ - listProcesses: async () => [processInfo('local-pty')], - getStatusSnapshot: () => [ - status({ sessionId: 'session-live', ptyId: 'local-pty', connectionId: 'ssh:dev-box' }) - ], - inspectForegroundProcess: async () => ({ available: true, process: 'gemini' }) - }) - - await expect( - resolveAiVaultSessionLiveness({ agent: 'gemini', sessionId: 'session-live' }, deps) - ).resolves.toBe('unknown') - }) - - it('returns not-live when every local Gemini process owns another known session', async () => { - const deps = dependencies({ - listProcesses: async () => [processInfo('other-pty')], - getStatusSnapshot: () => [status({ sessionId: 'session-other', ptyId: 'other-pty' })], - inspectForegroundProcess: async () => ({ available: true, process: 'gemini' }) - }) - - await expect( - resolveAiVaultSessionLiveness({ agent: 'gemini', sessionId: 'session-target' }, deps) - ).resolves.toBe('not-live') - }) - - it.each([ - ['missing session identity', dependencies(), undefined], - [ - 'unavailable process inventory', - dependencies({ - listProcesses: async () => { - throw new Error('daemon offline') - } - }), - 'session-live' - ], - [ - 'unattributed live agent process', - dependencies({ - listProcesses: async () => [processInfo('unknown-pty')], - inspectForegroundProcess: async () => ({ available: true, process: 'gemini' }) - }), - 'session-live' - ] - ])('preserves unknown for %s', async (_name, deps, sessionId) => { - await expect(resolveAiVaultSessionLiveness({ agent: 'gemini', sessionId }, deps)).resolves.toBe( - 'unknown' - ) - }) - - it('bounds foreground inspection concurrency', async () => { - let active = 0 - let maxActive = 0 - const inspectForegroundProcess = vi.fn(async () => { - active += 1 - maxActive = Math.max(maxActive, active) - await new Promise((resolve) => queueMicrotask(resolve)) - active -= 1 - return { available: true, process: 'zsh' } - }) - const deps = dependencies({ - listProcesses: async () => - Array.from({ length: 20 }, (_, index) => processInfo(`shell-${index}`)), - inspectForegroundProcess - }) - - await expect( - resolveAiVaultSessionLiveness({ agent: 'gemini', sessionId: 'session-target' }, deps) - ).resolves.toBe('not-live') - expect(inspectForegroundProcess).toHaveBeenCalledTimes(20) - expect(maxActive).toBeLessThanOrEqual(8) - }) - - it('fails closed before inspecting an oversized inventory', async () => { - const inspectForegroundProcess = vi.fn() - const deps = dependencies({ - listProcesses: async () => - Array.from({ length: 513 }, (_, index) => processInfo(`shell-${index}`)), - inspectForegroundProcess - }) - - await expect( - resolveAiVaultSessionLiveness({ agent: 'gemini', sessionId: 'session-target' }, deps) - ).resolves.toBe('unknown') - expect(inspectForegroundProcess).not.toHaveBeenCalled() - }) - - it('does not start foreground inspections after the shared deadline', async () => { - const inspectForegroundProcess = vi.fn() - const deps = dependencies({ - deadlineMs: 0, - listProcesses: async () => [processInfo('late-pty')], - inspectForegroundProcess - }) - - await expect( - resolveAiVaultSessionLiveness({ agent: 'gemini', sessionId: 'session-target' }, deps) - ).resolves.toBe('unknown') - expect(inspectForegroundProcess).not.toHaveBeenCalled() - }) -}) diff --git a/src/main/ai-vault/session-liveness.ts b/src/main/ai-vault/session-liveness.ts deleted file mode 100644 index 6f7170687ad..00000000000 --- a/src/main/ai-vault/session-liveness.ts +++ /dev/null @@ -1,187 +0,0 @@ -import { isShellProcess } from '../../shared/agent-detection' -import { recognizeAgentProcess } from '../../shared/agent-process-recognition' -import type { AgentStatusIpcPayload } from '../../shared/agent-status-types' -import type { AiVaultSessionLiveness } from '../../shared/ai-vault-session-deletion' -import type { AiVaultAgent } from '../../shared/ai-vault-types' -import type { PtyProcessInfo } from '../providers/pty-process-info' -import { isWslHookRelayConnectionId } from '../../shared/wsl-hook-relay-contract' -import { lstat } from 'node:fs/promises' -import { parseAgentSessionFileCached } from './session-scanner-parse-cache' - -const MAX_LIVENESS_PROCESSES = 512 -const PROCESS_INSPECTION_CONCURRENCY = 8 - -type ProcessInspection = { - available: boolean - process: string | null -} - -type AgentProcessInspection = 'target-agent' | 'unknown' | 'other' - -export type AiVaultSessionLivenessDependencies = { - deadlineMs?: number - listProcesses: () => Promise - getStatusSnapshot: () => AgentStatusIpcPayload[] - inspectForegroundProcess: (ptyId: string) => Promise - getStatusPtyId: (status: AgentStatusIpcPayload) => string | null - getAgentHint: (process: PtyProcessInfo) => string | null -} - -export type AiVaultSessionIdentityRead = - | { outcome: 'found'; sessionId: string } - | { outcome: 'missing' } - | { outcome: 'unknown' } - -/** Binds the requested identity to the validated transcript on disk. */ -export async function readAiVaultSessionIdentity(target: { - agent: AiVaultAgent - sessionId: string | undefined - filePath: string -}): Promise { - try { - const fileStats = await lstat(target.filePath) - if (!fileStats.isFile()) { - return { outcome: 'unknown' } - } - const session = await parseAgentSessionFileCached( - { - agent: target.agent, - file: { - path: target.filePath, - mtimeMs: fileStats.mtimeMs, - modifiedAt: fileStats.mtime.toISOString(), - sizeBytes: fileStats.size - }, - codexHome: null - }, - process.platform - ) - return session?.sessionId && session.sessionId === target.sessionId - ? { outcome: 'found', sessionId: session.sessionId } - : { outcome: 'unknown' } - } catch (error) { - return (error as NodeJS.ErrnoException).code === 'ENOENT' - ? { outcome: 'missing' } - : { outcome: 'unknown' } - } -} - -function isLocalStatus(status: AgentStatusIpcPayload): boolean { - return status.connectionId === null || isWslHookRelayConnectionId(status.connectionId) -} - -function isValidSessionId(sessionId: string | undefined): sessionId is string { - return ( - typeof sessionId === 'string' && - sessionId.length > 0 && - sessionId.length <= 512 && - sessionId.trim() === sessionId - ) -} - -async function inspectInBatches( - values: readonly T[], - inspect: (value: T) => Promise, - deadlineMs?: number -): Promise { - const results: AgentProcessInspection[] = [] - let nextIndex = 0 - const worker = async (): Promise => { - while (nextIndex < values.length) { - const index = nextIndex - nextIndex += 1 - if (deadlineMs !== undefined && Date.now() >= deadlineMs) { - results[index] = 'unknown' - continue - } - results[index] = await inspect(values[index]!) - } - } - await Promise.all( - Array.from({ length: Math.min(PROCESS_INSPECTION_CONCURRENCY, values.length) }, worker) - ) - return results -} - -/** Resolves owning-runtime liveness without treating unavailable evidence as absence. */ -export async function resolveAiVaultSessionLiveness( - target: { agent: AiVaultAgent; sessionId: string | undefined }, - deps: AiVaultSessionLivenessDependencies -): Promise { - if (!isValidSessionId(target.sessionId)) { - return 'unknown' - } - - let processes: PtyProcessInfo[] - try { - processes = await deps.listProcesses() - } catch { - return 'unknown' - } - if (processes.length > MAX_LIVENESS_PROCESSES) { - return 'unknown' - } - - const processInspections = await inspectInBatches( - processes, - async (process) => { - let inspection: ProcessInspection - try { - inspection = await deps.inspectForegroundProcess(process.id) - } catch { - return 'unknown' - } - if (!inspection.available) { - return 'unknown' - } - - const recognizedAgent = recognizeAgentProcess(inspection.process)?.agent ?? null - const hintedAgent = deps.getAgentHint(process) - const ownsTargetAgent = - recognizedAgent === target.agent || - (recognizedAgent === null && - (inspection.process === null || !isShellProcess(inspection.process)) && - hintedAgent === target.agent) - if (!ownsTargetAgent) { - return 'other' - } - return 'target-agent' - }, - deps.deadlineMs - ) - - let localStatuses: AgentStatusIpcPayload[] - try { - localStatuses = deps.getStatusSnapshot().filter(isLocalStatus) - } catch { - return 'unknown' - } - const processIds = new Set(processes.map((process) => process.id)) - const hasUnmatchedTargetStatus = localStatuses.some( - (status) => - status.agentType === target.agent && - status.providerSession?.id === target.sessionId && - !processIds.has(deps.getStatusPtyId(status) ?? '') - ) - if (hasUnmatchedTargetStatus) { - return 'unknown' - } - - const results = processInspections.map((inspection, index) => { - if (inspection !== 'target-agent') { - return inspection - } - const identities = localStatuses.filter( - (status) => - status.agentType === target.agent && deps.getStatusPtyId(status) === processes[index]!.id - ) - if (identities.some((status) => status.providerSession?.id === target.sessionId)) { - return 'live' - } - return identities.some((status) => status.providerSession) ? 'other' : 'unknown' - }) - if (results.includes('live')) { - return 'live' - } - return results.includes('unknown') ? 'unknown' : 'not-live' -} diff --git a/src/main/ipc/ai-vault-delete.ts b/src/main/ipc/ai-vault-delete.ts index 3f3a1c8abd4..a221cc28ceb 100644 --- a/src/main/ipc/ai-vault-delete.ts +++ b/src/main/ipc/ai-vault-delete.ts @@ -8,19 +8,13 @@ import { invalidateSessionParseCacheEntry } from '../ai-vault/session-scanner-pa import type { AiVaultAgent } from '../../shared/ai-vault-types' import type { AiVaultDeleteSessionArgs, - AiVaultDeleteSessionResult, - AiVaultSessionLiveness + AiVaultDeleteSessionResult } from '../../shared/ai-vault-session-deletion' // Which cache backs the multi-host list is ai-vault.ts's concern, so its // invalidation is injected rather than reached into from here. type AiVaultDeleteDeps = { invalidateMultiHostListCache: () => void - getSessionLiveness: (target: { - agent: AiVaultAgent - sessionId: string | undefined - filePath: string - }) => Promise } // Binds the delete orchestration to the caller's cache-invalidation seam. @@ -40,16 +34,13 @@ export async function deleteAiVaultSession( // The validator tolerates a malformed agent/filePath but destructures `args`, // so an absent payload is defaulted here to keep the never-throws boundary. const wslHomeDirs = await getAiVaultWslHomeDirs() - const result = await deleteAiVaultSessionFile( - { - agent: args?.agent as AiVaultAgent, - sessionId: args?.sessionId, - filePath: args?.filePath ?? '', - executionHostId: args?.executionHostId, - wslHomeDirs - }, - deps - ) + const result = await deleteAiVaultSessionFile({ + agent: args?.agent as AiVaultAgent, + sessionId: args?.sessionId, + filePath: args?.filePath ?? '', + executionHostId: args?.executionHostId, + wslHomeDirs + }) if (result.outcome === 'deleted') { // Three caches could otherwise resurrect it: the desktop per-host leg diff --git a/src/main/ipc/ai-vault.test.ts b/src/main/ipc/ai-vault.test.ts index e36708e18b0..9af91f92a33 100644 --- a/src/main/ipc/ai-vault.test.ts +++ b/src/main/ipc/ai-vault.test.ts @@ -800,8 +800,7 @@ describe('deleteAiVaultSession', () => { sessionId: args.sessionId, filePath: args.filePath, executionHostId: 'local' - }), - expect.objectContaining({ getSessionLiveness: expect.any(Function) }) + }) ) expect(mocks.invalidateAiVaultSessionListCache).toHaveBeenCalledTimes(1) expect(mocks.invalidateSessionParseCacheEntry).toHaveBeenCalledWith(args.filePath) @@ -850,8 +849,7 @@ describe('deleteAiVaultSession', () => { reason: 'invalid-path' }) expect(mocks.deleteAiVaultSessionFile).toHaveBeenCalledWith( - expect.objectContaining({ filePath: '' }), - expect.objectContaining({ getSessionLiveness: expect.any(Function) }) + expect.objectContaining({ filePath: '' }) ) expect(mocks.invalidateAiVaultSessionListCache).not.toHaveBeenCalled() }) diff --git a/src/main/ipc/ai-vault.ts b/src/main/ipc/ai-vault.ts index 3f4e2d49af6..c2f5be216c7 100644 --- a/src/main/ipc/ai-vault.ts +++ b/src/main/ipc/ai-vault.ts @@ -71,7 +71,6 @@ type AiVaultHandlerOptions = AiVaultSessionSources & getActiveRuntimeAiVaultHostInfos?: () => readonly RuntimeAiVaultHostInfo[] scanRuntimeAiVaultSessions?: RuntimeAiVaultScanner resolveRuntimeAiVaultSessionTitles?: RuntimeAiVaultSessionTitleResolver - getSessionLiveness?: Parameters[1]['getSessionLiveness'] } let scanCoordinator = new AiVaultScanCoordinator() @@ -80,10 +79,7 @@ const listCancellations = createSenderScopedRequestCancellations() // Shared by the IPC registration and the test internals: a delete must drop // the multi-host leg cache, which this module owns the only caller of. const aiVaultDeleteDeps = { - invalidateMultiHostListCache: invalidateAiVaultHostLegCache, - getSessionLiveness: ( - target: Parameters>[0] - ) => handlerOptions.getSessionLiveness?.(target) ?? Promise.resolve('unknown' as const) + invalidateMultiHostListCache: invalidateAiVaultHostLegCache } const resolveAiVaultSessionTitles = ( diff --git a/src/main/ipc/register-core-handlers.ts b/src/main/ipc/register-core-handlers.ts index b685f470eb0..607aa08ce3e 100644 --- a/src/main/ipc/register-core-handlers.ts +++ b/src/main/ipc/register-core-handlers.ts @@ -227,8 +227,7 @@ export function registerCoreHandlers( resolveRuntimeAiVaultSessionTitles: async (environmentId, args) => resolveRuntimeAiVaultSessionTitles(app.getPath('userData'), environmentId, args), prepareRuntimeSessionResume: async (environmentId, args) => - prepareRuntimeAiVaultSessionResume(app.getPath('userData'), environmentId, args), - getSessionLiveness: (target) => runtime.getAiVaultSessionLiveness(target) + prepareRuntimeAiVaultSessionResume(app.getPath('userData'), environmentId, args) }) registerNativeChatHandlers() registerClipboardHandlers(store) diff --git a/src/main/runtime/orca-runtime.ts b/src/main/runtime/orca-runtime.ts index 85888b94615..f0127702406 100644 --- a/src/main/runtime/orca-runtime.ts +++ b/src/main/runtime/orca-runtime.ts @@ -490,16 +490,11 @@ import { listAiVaultSessions } from '../ai-vault/cached-session-list' import { resolveLocalAiVaultSessionTitles } from '../ai-vault/session-title-resolver' -import { - readAiVaultSessionIdentity, - resolveAiVaultSessionLiveness -} from '../ai-vault/session-liveness' -import type { AiVaultAgent, AiVaultListArgs, AiVaultListResult } from '../../shared/ai-vault-types' +import type { AiVaultListArgs, AiVaultListResult } from '../../shared/ai-vault-types' import type { AiVaultSessionTitleRequest, AiVaultSessionTitlesResult } from '../../shared/ai-vault-session-title' -import type { AiVaultSessionLiveness } from '../../shared/ai-vault-session-deletion' import type { AiVaultPrepareSessionResumeArgs, AiVaultPrepareSessionResumeResult @@ -4990,74 +4985,6 @@ export class OrcaRuntimeService { return resolveLocalAiVaultSessionTitles(requests, signal) } - async getAiVaultSessionLiveness(target: { - agent: AiVaultAgent - sessionId: string | undefined - filePath: string - }): Promise { - const provider = this.getLocalProviderFn?.() - if (!provider || !this.getAgentProviderSessionSnapshotFn) { - return 'unknown' - } - const identity = await readAiVaultSessionIdentity(target) - if (identity.outcome !== 'found') { - return 'unknown' - } - const deadlineMs = Date.now() + 3_000 - return await resolveAiVaultSessionLiveness( - { - agent: target.agent, - sessionId: identity.sessionId - }, - { - deadlineMs, - listProcesses: async () => { - const result = await withTimeoutResult( - provider.listProcesses({ deadlineMs }), - Math.max(1, deadlineMs - Date.now()) - ) - if (!result.ok) { - throw new Error('agent_session_ownership_unknown') - } - return result.value - }, - getStatusSnapshot: this.getAgentProviderSessionSnapshotFn, - inspectForegroundProcess: async (ptyId) => { - const result = await withTimeoutResult( - provider.getForegroundProcess(ptyId), - Math.max(1, deadlineMs - Date.now()) - ) - return result.ok - ? { available: true, process: result.value } - : { available: false, process: null } - }, - getStatusPtyId: (status) => { - if (status.terminalHandle) { - const live = this.getLivePtyForHandle(status.terminalHandle) - if (live) { - return live.pty.ptyId - } - for (const [ptyId, handle] of this.handleByPtyId) { - if (handle === status.terminalHandle) { - return ptyId - } - } - } - return this.getPtyRecordForPaneKey(status.paneKey)?.ptyId ?? null - }, - getAgentHint: (process) => { - const pty = this.ptysById.get(process.id) - const runtimeHint = pty?.foregroundAgent ?? pty?.launchAgent - if (runtimeHint) { - return runtimeHint - } - const ownerAgents = new Set(process.agentSessionOwners?.map((owner) => owner.claim.agent)) - return ownerAgents.size === 1 ? ([...ownerAgents][0] ?? null) : null - } - } - ) - } - prepareAiVaultSessionResume( args: AiVaultPrepareSessionResumeArgs ): Promise { diff --git a/src/renderer/src/components/right-sidebar/AiVaultSessionRow.tsx b/src/renderer/src/components/right-sidebar/AiVaultSessionRow.tsx index 9e2f1702f7c..e4af4f3e972 100644 --- a/src/renderer/src/components/right-sidebar/AiVaultSessionRow.tsx +++ b/src/renderer/src/components/right-sidebar/AiVaultSessionRow.tsx @@ -85,7 +85,7 @@ export function VaultSessionRow({ const detailsId = getSessionDetailsId(session.id) const latestTurn = latestSessionConversationTurn(session) // Computed once so the dropdown menu and the context menu never disagree. - const deleteBlockedReason = aiVaultSessionDeleteBlockedReason(session, liveState) + const deleteBlockedReason = aiVaultSessionDeleteBlockedReason(session) const requestDelete = (): void => onRequestDelete(session) const detailsTooltip = detailsExpanded ? translate('auto.components.right.sidebar.AiVaultSessionRow.hideDetails', 'Hide Details') diff --git a/src/renderer/src/components/right-sidebar/ai-vault-session-deletability.test.ts b/src/renderer/src/components/right-sidebar/ai-vault-session-deletability.test.ts index 49e8d39e123..3e2a370e8f5 100644 --- a/src/renderer/src/components/right-sidebar/ai-vault-session-deletability.test.ts +++ b/src/renderer/src/components/right-sidebar/ai-vault-session-deletability.test.ts @@ -5,7 +5,6 @@ import { aiVaultSessionDeleteBlockedReason } from './ai-vault-session-deletabili // assertions pin the English copy as well as the gate order. const NON_LOCAL = 'Only sessions on this device can be deleted.' const SYNTHETIC = "This session can't be deleted from Orca." -const LIVE = 'This session is still running — wait for it to finish before deleting.' const localGeminiSession = { agent: 'gemini' as const, @@ -28,28 +27,6 @@ describe('aiVaultSessionDeleteBlockedReason', () => { ).toBeNull() }) - it('tells the user to wait while the agent is still running', () => { - for (const live of ['working', 'blocked', 'waiting'] as const) { - expect(aiVaultSessionDeleteBlockedReason(localGeminiSession, live)).toBe(LIVE) - } - }) - - it('offers Delete for a finished session (done) and one with no live state', () => { - expect(aiVaultSessionDeleteBlockedReason(localGeminiSession, 'done')).toBeNull() - expect(aiVaultSessionDeleteBlockedReason(localGeminiSession, null)).toBeNull() - }) - - it('keeps the permanent reason over "running" for an unsupported live session', () => { - // A live but unsupported agent stays "unsupported" — it would never become - // deletable, so "wait for it to finish" would mislead. - expect( - aiVaultSessionDeleteBlockedReason( - { agent: 'codex', executionHostId: 'local', filePath: '/home/user/.codex/x.jsonl' }, - 'working' - ) - ).toBe("Codex sessions can't be deleted from Orca.") - }) - it('blocks ssh- and runtime-hosted sessions regardless of agent', () => { for (const executionHostId of ['ssh:dev-box', 'runtime:gpu-box'] as const) { expect(aiVaultSessionDeleteBlockedReason({ ...localGeminiSession, executionHostId })).toBe( diff --git a/src/renderer/src/components/right-sidebar/ai-vault-session-deletability.ts b/src/renderer/src/components/right-sidebar/ai-vault-session-deletability.ts index 033ebd87c81..87007c376af 100644 --- a/src/renderer/src/components/right-sidebar/ai-vault-session-deletability.ts +++ b/src/renderer/src/components/right-sidebar/ai-vault-session-deletability.ts @@ -1,6 +1,5 @@ import { isAiVaultDeletableAgent } from '../../../../shared/ai-vault-session-deletion' import type { AiVaultSession } from '../../../../shared/ai-vault-types' -import type { AgentStatusState } from '../../../../shared/agent-status-types' import { translate } from '@/i18n/i18n' import { agentLabel } from './ai-vault-session-filters' import { @@ -8,11 +7,6 @@ import { isSyntheticAiVaultSessionPath } from './ai-vault-session-path-actions' -// Matches the active-dot rule in ai-vault-session-row-display. -function isSessionLive(liveState: AgentStatusState | null | undefined): boolean { - return liveState != null && liveState !== 'done' -} - /** * Why Delete is unavailable for this session, as the tooltip text to show — or * null when it is offered. Each message says which sessions are affected, never @@ -25,8 +19,7 @@ function isSessionLive(liveState: AgentStatusState | null | undefined): boolean * it does: both consult the same shared agent set and host/synthetic predicates. */ export function aiVaultSessionDeleteBlockedReason( - session: Pick, - liveState?: AgentStatusState | null + session: Pick ): string | null { if (!canUseLocalAiVaultSessionPathActions(session.executionHostId)) { return translate( @@ -47,14 +40,5 @@ export function aiVaultSessionDeleteBlockedReason( { value0: agentLabel(session.agent) } ) } - // Last, so an otherwise-deletable session reads as "wait for it to finish" - // rather than a permanent reason. Trashing a live transcript would drop the - // writes the agent is still appending. - if (isSessionLive(liveState)) { - return translate( - 'auto.components.right.sidebar.AiVaultSessionRow.deleteReasonSessionLive', - 'This session is still running — wait for it to finish before deleting.' - ) - } return null } diff --git a/src/renderer/src/i18n/locales/en.json b/src/renderer/src/i18n/locales/en.json index a1eedc20694..08fd9b389c8 100644 --- a/src/renderer/src/i18n/locales/en.json +++ b/src/renderer/src/i18n/locales/en.json @@ -11738,8 +11738,7 @@ "delete": "Delete", "deleteReasonNonLocalHost": "Only sessions on this device can be deleted.", "deleteReasonSyntheticPath": "This session can't be deleted from Orca.", - "deleteReasonUnsupportedAgent": "{{value0}} sessions can't be deleted from Orca.", - "deleteReasonSessionLive": "This session is still running — wait for it to finish before deleting." + "deleteReasonUnsupportedAgent": "{{value0}} sessions can't be deleted from Orca." }, "AiVaultSessionDeleteDialog": { "title": "Delete this session?", diff --git a/src/renderer/src/i18n/locales/es.json b/src/renderer/src/i18n/locales/es.json index dc1024c8b27..d84aa89b132 100644 --- a/src/renderer/src/i18n/locales/es.json +++ b/src/renderer/src/i18n/locales/es.json @@ -11318,8 +11318,7 @@ "delete": "Eliminar", "deleteReasonNonLocalHost": "Solo se pueden eliminar las sesiones de este dispositivo.", "deleteReasonSyntheticPath": "Esta sesión no se puede eliminar desde Orca.", - "deleteReasonUnsupportedAgent": "Las sesiones de {{value0}} no se pueden eliminar desde Orca.", - "deleteReasonSessionLive": "Esta sesión aún se está ejecutando: espera a que termine antes de eliminarla." + "deleteReasonUnsupportedAgent": "Las sesiones de {{value0}} no se pueden eliminar desde Orca." }, "AiVaultSessionDeleteDialog": { "title": "¿Eliminar esta sesión?", diff --git a/src/renderer/src/i18n/locales/ja.json b/src/renderer/src/i18n/locales/ja.json index 3d7991b1475..76f154d39e8 100644 --- a/src/renderer/src/i18n/locales/ja.json +++ b/src/renderer/src/i18n/locales/ja.json @@ -11318,8 +11318,7 @@ "delete": "削除", "deleteReasonNonLocalHost": "このデバイス上のセッションのみ削除できます。", "deleteReasonSyntheticPath": "このセッションは Orca では削除できません。", - "deleteReasonUnsupportedAgent": "{{value0}} のセッションは Orca では削除できません。", - "deleteReasonSessionLive": "このセッションはまだ実行中です — 削除する前に完了を待ってください。" + "deleteReasonUnsupportedAgent": "{{value0}} のセッションは Orca では削除できません。" }, "AiVaultSessionDeleteDialog": { "title": "このセッションを削除しますか?", diff --git a/src/renderer/src/i18n/locales/ko.json b/src/renderer/src/i18n/locales/ko.json index c04d93c1629..c86caa3e852 100644 --- a/src/renderer/src/i18n/locales/ko.json +++ b/src/renderer/src/i18n/locales/ko.json @@ -11318,8 +11318,7 @@ "delete": "삭제", "deleteReasonNonLocalHost": "이 기기에 있는 세션만 삭제할 수 있습니다.", "deleteReasonSyntheticPath": "이 세션은 Orca에서 삭제할 수 없습니다.", - "deleteReasonUnsupportedAgent": "{{value0}} 세션은 Orca에서 삭제할 수 없습니다.", - "deleteReasonSessionLive": "이 세션은 아직 실행 중입니다 — 삭제하려면 완료될 때까지 기다리세요." + "deleteReasonUnsupportedAgent": "{{value0}} 세션은 Orca에서 삭제할 수 없습니다." }, "AiVaultSessionDeleteDialog": { "title": "이 세션을 삭제할까요?", diff --git a/src/renderer/src/i18n/locales/zh.json b/src/renderer/src/i18n/locales/zh.json index 85b4cd051ba..62090dd10ea 100644 --- a/src/renderer/src/i18n/locales/zh.json +++ b/src/renderer/src/i18n/locales/zh.json @@ -11338,8 +11338,7 @@ "delete": "删除", "deleteReasonNonLocalHost": "只能删除此设备上的会话。", "deleteReasonSyntheticPath": "此会话无法在 Orca 中删除。", - "deleteReasonUnsupportedAgent": "{{value0}} 会话无法在 Orca 中删除。", - "deleteReasonSessionLive": "此会话仍在运行 — 请等待其完成后再删除。" + "deleteReasonUnsupportedAgent": "{{value0}} 会话无法在 Orca 中删除。" }, "AiVaultSessionDeleteDialog": { "title": "要删除此会话吗？", diff --git a/src/shared/ai-vault-session-deletion.ts b/src/shared/ai-vault-session-deletion.ts index 7fdde68b0e4..9e25f632abe 100644 --- a/src/shared/ai-vault-session-deletion.ts +++ b/src/shared/ai-vault-session-deletion.ts @@ -4,7 +4,8 @@ import type { ExecutionHostId } from './execution-host' // IPC payload for aiVault:deleteSession. export type AiVaultDeleteSessionArgs = { agent: AiVaultAgent - // Optional for mixed renderer/main versions; main rejects missing identity. + // Optional for mixed renderer/main versions. Main ignores this field on delete + // (path + host + agent validation only; no identity/liveness check). sessionId?: string filePath: string // The session's host; only a local session may be deleted. @@ -71,10 +72,6 @@ export type AiVaultSessionDeleteRejectionCode = // fs-side guard: lstat disagrees with the removal's declared kind (a symlink, // or a file where the plan expects a directory). | 'unexpected-target-kind' - | 'session-live' - | 'session-liveness-unknown' - -export type AiVaultSessionLiveness = 'live' | 'not-live' | 'unknown' // One path the executor removes. A `kind` mismatch on disk is a rejection, // never a coerced delete. `roots` are what the path's realpath must still diff --git a/tests/e2e/terminal-macos-system-key-remap.spec.ts b/tests/e2e/terminal-macos-system-key-remap.spec.ts index 711b7a6b602..f858f224386 100644 --- a/tests/e2e/terminal-macos-system-key-remap.spec.ts +++ b/tests/e2e/terminal-macos-system-key-remap.spec.ts @@ -149,7 +149,12 @@ test.describe('Terminal macOS system key remap', () => { expect(received).toEqual([Buffer.from(`${committed}\n`).toString('hex')]) completed = true } finally { - await closeTerminalImePaneArena(arena, testInfo, `${arm.slug}-${layout.label}`, !completed) + await closeTerminalImePaneArena( + arena, + testInfo, + `${arm.slug}-${layout.label}`, + !completed + ) removeTerminalImeByteReader(reader) } }) From 75650936f4283e2902ae7cfe522feef9912d64d7 Mon Sep 17 00:00:00 2001 From: OrcaWin Date: Sun, 9 Aug 2026 18:42:15 -0700 Subject: [PATCH 09/12] fix(source-control): hydrate Windows shell profile PATH (#13418) --- src/main/index.ts | 36 ++- src/main/ipc/settings.ts | 2 +- src/main/runtime/orca-runtime.test.ts | 2 +- src/main/runtime/orca-runtime.ts | 2 +- src/main/startup/hydrate-shell-path.test.ts | 20 ++ src/main/startup/hydrate-shell-path.ts | 177 +++++++++-- .../hydrate-shell-path.windows.test.ts | 297 ++++++++++++++++++ .../windows-shell-path-hydration.test.ts | 214 +++++++++++++ .../startup/windows-shell-path-hydration.ts | 107 +++++++ 9 files changed, 822 insertions(+), 35 deletions(-) create mode 100644 src/main/startup/hydrate-shell-path.windows.test.ts create mode 100644 src/main/startup/windows-shell-path-hydration.test.ts create mode 100644 src/main/startup/windows-shell-path-hydration.ts diff --git a/src/main/index.ts b/src/main/index.ts index a8153952c02..ddf79fd9455 100644 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -159,6 +159,7 @@ import { recoverLegacyWorkerTerminalsForRendererStartup } from './startup/legacy import { createWslCliReconciliationStartupBarrier } from './startup/wsl-cli-reconciliation-startup-barrier' import { getDevInstanceIdentity } from './startup/dev-instance-identity' import { hydrateShellPath, mergePathSegments } from './startup/hydrate-shell-path' +import { createWindowsShellPathHydration } from './startup/windows-shell-path-hydration' import { acquireSingleInstanceLock, logSingleInstanceLockBypass, @@ -2124,6 +2125,21 @@ void app.whenReady().then(async () => { const activeOrcaProfile = ensureActiveOrcaProfile() store = new Store({ dataFile: activeOrcaProfile.dataFile }) + const windowsShellPathHydration = createWindowsShellPathHydration() + if (process.platform === 'win32') { + const settings = store.getSettings() + if (app.isPackaged) { + void windowsShellPathHydration.hydrate( + settings.terminalWindowsShell, + settings.terminalWindowsPowerShellImplementation + ) + } else { + windowsShellPathHydration.configure( + settings.terminalWindowsShell, + settings.terminalWindowsPowerShellImplementation + ) + } + } wslHookRelayManager.setManagedHookSettingsResolver(() => store?.getSettings() ?? null) logStartupMilestone('store-loaded') // Why: apply initial fallback WSL distro from store settings for global git/CLI calls. @@ -2133,6 +2149,22 @@ void app.whenReady().then(async () => { // Why: synchronize fallback WSL distro updates to runner. setDefaultWslDistroOverride(settings.terminalWindowsWslDistro ?? null) } + if ( + ('terminalWindowsShell' in updates || 'terminalWindowsPowerShellImplementation' in updates) && + process.platform === 'win32' + ) { + if (app.isPackaged) { + void windowsShellPathHydration.hydrate( + settings.terminalWindowsShell, + settings.terminalWindowsPowerShellImplementation + ) + } else { + windowsShellPathHydration.configure( + settings.terminalWindowsShell, + settings.terminalWindowsPowerShellImplementation + ) + } + } if ('showMenuBarIcon' in updates) { // Why: Store is the mutation authority for all settings writes, so every macOS toggle updates the native item live. syncMacMenuBarIcon(settings.showMenuBarIcon !== false) @@ -2742,7 +2774,7 @@ void app.whenReady().then(async () => { if (isAgentStatusHooksEnabled(store.getSettings())) { const managedHookStore = store void applyAgentStatusHooksEnabled(true, managedHookStore.getSettings(), { - shouldHydrateShellPath: app.isPackaged && process.platform !== 'win32', + shouldHydrateShellPath: app.isPackaged, onInstallError: recordManagedHookInstallFailure, shouldContinue: (agent) => { const settings = managedHookStore.getSettings() @@ -2916,6 +2948,8 @@ void app.whenReady().then(async () => { } }) + // Why: Git hooks inherit process.env, so Source Control must not open before profile PATH settles. + await windowsShellPathHydration.whenReady() startTerminalRuntimeStartupServices() app.on('activate', handleMacAppActivation) diff --git a/src/main/ipc/settings.ts b/src/main/ipc/settings.ts index 275ba54c82a..df3817c949c 100644 --- a/src/main/ipc/settings.ts +++ b/src/main/ipc/settings.ts @@ -172,7 +172,7 @@ export function registerSettingsHandlers( if (hookSettingChanged) { try { await applyAgentStatusHooksEnabled(result.agentStatusHooksEnabled, result, { - shouldHydrateShellPath: app.isPackaged && process.platform !== 'win32', + shouldHydrateShellPath: app.isPackaged, onInstallError: recordManagedHookInstallFailure, shouldContinue: (agent) => { const settings = store.getSettings() diff --git a/src/main/runtime/orca-runtime.test.ts b/src/main/runtime/orca-runtime.test.ts index ec656c70144..7967d422f46 100644 --- a/src/main/runtime/orca-runtime.test.ts +++ b/src/main/runtime/orca-runtime.test.ts @@ -1815,7 +1815,7 @@ describe('OrcaRuntimeService', () => { expect.objectContaining({ disabledTuiAgents: ['claude'] }), expect.objectContaining({ shouldContinue: expect.any(Function), - shouldHydrateShellPath: process.platform !== 'win32' + shouldHydrateShellPath: true }) ) }) diff --git a/src/main/runtime/orca-runtime.ts b/src/main/runtime/orca-runtime.ts index f0127702406..526acea823b 100644 --- a/src/main/runtime/orca-runtime.ts +++ b/src/main/runtime/orca-runtime.ts @@ -3607,7 +3607,7 @@ export class OrcaRuntimeService { return } await applyAgentStatusHooksEnabled(settings.agentStatusHooksEnabled !== false, settings, { - shouldHydrateShellPath: app.isPackaged && process.platform !== 'win32', + shouldHydrateShellPath: app.isPackaged, onInstallError: recordManagedHookInstallFailure, shouldContinue: (agent) => { const current = this.store?.getSettings() diff --git a/src/main/startup/hydrate-shell-path.test.ts b/src/main/startup/hydrate-shell-path.test.ts index 7ff6ee2ae81..316baad7280 100644 --- a/src/main/startup/hydrate-shell-path.test.ts +++ b/src/main/startup/hydrate-shell-path.test.ts @@ -93,6 +93,26 @@ describe('hydrateShellPath', () => { expect(spawnCount).toBe(2) }) + it('continues the probe queue after a rejected hydration', async () => { + const rejectedSpawner = vi.fn(async () => { + throw new Error('profile failed') + }) + const successfulSpawner = vi.fn(async () => ({ + segments: ['/current'], + ok: true, + failureReason: 'none' + })) + + await expect( + hydrateShellPath({ shellOverride: '/bin/zsh', spawner: rejectedSpawner, force: true }) + ).rejects.toThrow('profile failed') + await expect( + hydrateShellPath({ shellOverride: '/bin/bash', spawner: successfulSpawner, force: true }) + ).resolves.toEqual({ segments: ['/current'], ok: true, failureReason: 'none' }) + + expect(successfulSpawner).toHaveBeenCalledWith('/bin/bash') + }) + it('returns failureReason:no_shell when no shell is available (Windows path)', async () => { const result = await hydrateShellPath({ shellOverride: null, diff --git a/src/main/startup/hydrate-shell-path.ts b/src/main/startup/hydrate-shell-path.ts index 9411b0437d3..87747399615 100644 --- a/src/main/startup/hydrate-shell-path.ts +++ b/src/main/startup/hydrate-shell-path.ts @@ -1,18 +1,14 @@ import { spawn } from 'node:child_process' -import { delimiter } from 'node:path' +import { delimiter, win32 as pathWin32 } from 'node:path' import type { ShellHydrationFailureReason } from '../../shared/types' +import { resolveWindowsShellStartupFamily } from '../../shared/windows-terminal-shell' -// Why: GUI-launched Electron on macOS/Linux inherits a minimal PATH from launchd -// that does not include dirs appended by the user's shell rc files (~/.zshrc, -// ~/.bashrc). Tools installed into ~/.opencode/bin, ~/.cargo/bin, pyenv/volta +// Why: GUI-launched Electron can miss PATH entries added by shell profiles. +// Tools installed into ~/.opencode/bin, ~/.cargo/bin, pyenv/volta/fnm // shims, and countless other user-local locations end up invisible to our // `which` probe even though they work fine from Terminal (see stablyai/orca#829). // -// Rather than play whack-a-mole adding every agent's install dir to a hardcoded -// list, we spawn the user's login shell once per app session and read the PATH -// it would export. This matches the behavior of every popular Electron app that -// handles this problem (Hyper, VS Code, Cursor, etc. via shell-env/fix-path) — -// we implement it inline to avoid adding a dependency. +// Probe the profile-loading shell once instead of hard-coding every tool's install path. const DELIMITER = '__ORCA_SHELL_PATH__' const SPAWN_TIMEOUT_MS = 5000 @@ -34,15 +30,35 @@ export type HydrationResult = } let cached: Promise | null = null +let probeQueue = Promise.resolve() +let configuredWindowsShell = 'powershell.exe' +let configuredWindowsGitBashPath: string | null = null +let configuredWindowsFallbackShell: string | null = null +let windowsShellConfigurationVersion = 0 +const windowsIntroducedPathKeys = new Set() /** @internal - tests need a clean hydration cache between cases. */ export function _resetHydrateShellPathCache(): void { cached = null + probeQueue = Promise.resolve() + configuredWindowsShell = 'powershell.exe' + configuredWindowsGitBashPath = null + configuredWindowsFallbackShell = null + windowsShellConfigurationVersion = 0 + windowsIntroducedPathKeys.clear() } function pickShell(): string | null { if (process.platform === 'win32') { - return null + const family = resolveWindowsShellStartupFamily(configuredWindowsShell) + if (family === 'cmd') { + return null + } + if (family === 'posix') { + return configuredWindowsGitBashPath + } + const basename = pathWin32.basename(configuredWindowsShell).toLowerCase() + return basename === 'powershell.exe' || basename === 'pwsh.exe' ? configuredWindowsShell : null } const shell = process.env.SHELL if (shell && shell.length > 0) { @@ -51,7 +67,7 @@ function pickShell(): string | null { return process.platform === 'darwin' ? '/bin/zsh' : '/bin/bash' } -function parseCapturedPath(stdout: string): string[] { +function parseCapturedPath(stdout: string, pathDelimiter: string = delimiter): string[] { const cleaned = stdout.replace(ANSI_RE, '') const first = cleaned.indexOf(DELIMITER) if (first < 0) { @@ -70,32 +86,48 @@ function parseCapturedPath(stdout: string): string[] { return [ ...new Set( value - .split(delimiter) + .split(pathDelimiter) .map((s) => s.trim()) .filter(Boolean) ) ] } +function shellPathProbe(shell: string): { args: string[]; pathDelimiter: string } { + if (process.platform !== 'win32') { + const command = `printf '%s' '${DELIMITER}'; printf '%s' "$PATH"; printf '%s' '${DELIMITER}'` + return { args: ['-ilc', command], pathDelimiter: delimiter } + } + if (resolveWindowsShellStartupFamily(shell) === 'posix') { + // Why: native child processes cannot resolve Git Bash's /c/... PATH entries. + const command = `printf '%s' '${DELIMITER}'; cygpath -wp "$PATH"; printf '%s' '${DELIMITER}'` + return { args: ['-ilc', command], pathDelimiter: ';' } + } + const command = + `[Console]::OutputEncoding = [System.Text.UTF8Encoding]::new($false); ` + + `[Console]::Write('${DELIMITER}'); [Console]::Write($env:Path); ` + + `[Console]::Write('${DELIMITER}')` + // Why: omitting -NoProfile is the behavior this probe exists to capture. + return { args: ['-NoLogo', '-Command', command], pathDelimiter: ';' } +} + function spawnShellAndReadPath(shell: string): Promise { return new Promise((resolve) => { - // Why: printing $PATH between delimiters is resilient to rc-file banners, - // MOTDs, and `echo` invocations that shells like fish print unprompted. - // `-ilc` runs the shell as a login+interactive so both .profile/.zprofile - // and .bashrc/.zshrc are sourced — matches what `which` in Terminal sees. - const command = `printf '%s' '${DELIMITER}'; printf '%s' "$PATH"; printf '%s' '${DELIMITER}'` + // Why: delimiters isolate PATH from profile banners and MOTDs. + const probe = shellPathProbe(shell) let finished = false let stdout = '' let timer: ReturnType | null = null - const child = spawn(shell, ['-ilc', command], { + const child = spawn(shell, probe.args, { // Why: inherit current env so the shell sees the same baseline, then let // it layer its own rc files on top. Do NOT forward stdio — some shells // (oh-my-zsh setups, powerlevel10k) print a lot to stderr on startup, // and we don't want that in Orca's console. env: process.env, stdio: ['ignore', 'pipe', 'ignore'], - detached: false + detached: false, + windowsHide: true }) const cleanup = (): void => { @@ -137,7 +169,7 @@ function spawnShellAndReadPath(shell: string): Promise { } const onClose = (): void => { - const segments = parseCapturedPath(stdout) + const segments = parseCapturedPath(stdout, probe.pathDelimiter) if (segments.length === 0) { finish({ segments: [], ok: false, failureReason: 'empty_path' }) return @@ -160,7 +192,7 @@ type HydrateOptions = { } /** - * Spawn the user's login shell once and return the PATH it would export. + * Spawn the user's profile-loading shell once and return the PATH it would export. * Caches the promise for the lifetime of the process — call * `_resetHydrateShellPathCache()` in tests or `hydrateShellPath({ force: true })` * when the user asks to re-probe (e.g. after installing a new CLI). @@ -169,17 +201,92 @@ export function hydrateShellPath(options: HydrateOptions = {}): Promise { + const result = await spawner(shell) + if (!result.ok && result.failureReason === 'spawn_error' && fallbackShell) { + return spawner(fallbackShell) + } + return result + }) + // Why: one rejected profile must not block later refreshes or shell changes. + probeQueue = probe.then( + () => undefined, + () => undefined + ) + cached = probe.then((result) => { + if ( + platform === 'win32' && + options.shellOverride === undefined && + configurationVersion !== windowsShellConfigurationVersion + ) { + return hydrateShellPath() + } + return result + }) return cached } +export function configureWindowsShellPathHydration( + shell: string | null | undefined, + gitBashPath: string | null = null, + fallbackShell: string | null = null +): void { + const next = shell?.trim() || 'powershell.exe' + if ( + next === configuredWindowsShell && + gitBashPath === configuredWindowsGitBashPath && + fallbackShell === configuredWindowsFallbackShell + ) { + return + } + clearWindowsIntroducedPathSegments() + configuredWindowsShell = next + configuredWindowsGitBashPath = gitBashPath + configuredWindowsFallbackShell = fallbackShell + windowsShellConfigurationVersion += 1 + cached = null +} + +function uniquePathSegments(segments: string[], pathKey: (segment: string) => string): string[] { + const seen = new Set() + return segments.filter((segment) => { + const key = pathKey(segment) + if (seen.has(key)) { + return false + } + seen.add(key) + return true + }) +} + +function windowsPathKey(segment: string): string { + const normalized = pathWin32.normalize(segment) + const root = pathWin32.parse(normalized).root + const withoutTrailingSlash = + normalized.length > root.length ? normalized.replace(/[\\/]+$/, '') : normalized + return withoutTrailingSlash.toLowerCase() +} + +function clearWindowsIntroducedPathSegments(): void { + if (process.platform !== 'win32' || windowsIntroducedPathKeys.size === 0) { + return + } + const currentSegments = (process.env.PATH ?? '').split(pathWin32.delimiter).filter(Boolean) + process.env.PATH = currentSegments + .filter((segment) => !windowsIntroducedPathKeys.has(windowsPathKey(segment))) + .join(pathWin32.delimiter) + windowsIntroducedPathKeys.clear() +} + /** * Promote shell-discovered PATH segments to the front of process.env.PATH, * preserving shell ordering and avoiding duplicates. Returns the segments that @@ -190,16 +297,19 @@ export function mergePathSegments(segments: string[]): string[] { return [] } const current = process.env.PATH ?? '' - const currentSegments = current.split(delimiter).filter(Boolean) - const shellSegments = [...new Set(segments)] - const shellSegmentSet = new Set(shellSegments) - const existing = new Set(currentSegments) - const added = shellSegments.filter((segment) => !existing.has(segment)) + const pathDelimiter = process.platform === 'win32' ? pathWin32.delimiter : delimiter + const currentSegments = current.split(pathDelimiter).filter(Boolean) + const pathKey = + process.platform === 'win32' ? windowsPathKey : (segment: string): string => segment + const shellSegments = uniquePathSegments(segments, pathKey) + const shellSegmentSet = new Set(shellSegments.map(pathKey)) + const existing = new Set(currentSegments.map(pathKey)) + const added = shellSegments.filter((segment) => !existing.has(pathKey(segment))) const merged = [ ...shellSegments, - ...currentSegments.filter((segment) => !shellSegmentSet.has(segment)) + ...currentSegments.filter((segment) => !shellSegmentSet.has(pathKey(segment))) ] - const next = merged.join(delimiter) + const next = merged.join(pathDelimiter) if (next === current) { return [] } @@ -207,5 +317,10 @@ export function mergePathSegments(segments: string[]): string[] { // A seeded fallback can point at a stale CLI while the user's shell resolves // a healthy one from the same directory list in a different order. process.env.PATH = next + if (process.platform === 'win32') { + for (const segment of added) { + windowsIntroducedPathKeys.add(windowsPathKey(segment)) + } + } return added } diff --git a/src/main/startup/hydrate-shell-path.windows.test.ts b/src/main/startup/hydrate-shell-path.windows.test.ts new file mode 100644 index 00000000000..ee1957c8a42 --- /dev/null +++ b/src/main/startup/hydrate-shell-path.windows.test.ts @@ -0,0 +1,297 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { EventEmitter } from 'node:events' +import type { ChildProcessWithoutNullStreams } from 'node:child_process' +import { + _resetHydrateShellPathCache, + configureWindowsShellPathHydration, + hydrateShellPath, + mergePathSegments, + type HydrationResult +} from './hydrate-shell-path' + +const { spawnMock } = vi.hoisted(() => ({ + spawnMock: vi.fn() +})) + +vi.mock('child_process', () => ({ + spawn: spawnMock +})) + +type HydrationSpawner = (shell: string) => Promise + +type Deferred = { + promise: Promise + resolve: (value: T) => void +} + +function deferred(): Deferred { + let resolve!: (value: T) => void + const promise = new Promise((next) => { + resolve = next + }) + return { promise, resolve } +} + +function createMockShellProcess(): ChildProcessWithoutNullStreams { + const proc = new EventEmitter() as ChildProcessWithoutNullStreams + Object.assign(proc, { + stdout: new EventEmitter(), + stderr: new EventEmitter(), + stdin: new EventEmitter(), + kill: vi.fn() + }) + return proc +} + +describe('Windows shell PATH hydration', () => { + const originalPath = process.env.PATH + + beforeEach(() => { + vi.spyOn(process, 'platform', 'get').mockReturnValue('win32') + _resetHydrateShellPathCache() + spawnMock.mockReset() + }) + + afterEach(() => { + vi.restoreAllMocks() + if (originalPath === undefined) { + delete process.env.PATH + } else { + process.env.PATH = originalPath + } + }) + + it('hydrates the default PowerShell profile PATH on Windows (#13328)', async () => { + const spawner = vi.fn(async () => ({ + segments: ['C:\\Users\\tester\\AppData\\Local\\fnm'], + ok: true, + failureReason: 'none' + })) + + await expect(hydrateShellPath({ spawner })).resolves.toEqual({ + segments: ['C:\\Users\\tester\\AppData\\Local\\fnm'], + ok: true, + failureReason: 'none' + }) + expect(spawner).toHaveBeenCalledWith('powershell.exe') + }) + + it('uses the configured PowerShell executable', async () => { + const spawner = vi.fn(async () => ({ + segments: ['C:\\tools'], + ok: true, + failureReason: 'none' + })) + + configureWindowsShellPathHydration('C:\\Program Files\\PowerShell\\7\\pwsh.exe') + await hydrateShellPath({ spawner }) + + expect(spawner).toHaveBeenCalledWith('C:\\Program Files\\PowerShell\\7\\pwsh.exe') + }) + + it('falls back to Windows PowerShell when preferred pwsh cannot spawn', async () => { + const spawner = vi.fn() + spawner + .mockResolvedValueOnce({ segments: [], ok: false, failureReason: 'spawn_error' }) + .mockResolvedValueOnce({ + segments: ['C:\\WindowsPowerShell-profile'], + ok: true, + failureReason: 'none' + }) + configureWindowsShellPathHydration('pwsh.exe', null, 'powershell.exe') + + await expect(hydrateShellPath({ spawner })).resolves.toEqual({ + segments: ['C:\\WindowsPowerShell-profile'], + ok: true, + failureReason: 'none' + }) + expect(spawner.mock.calls).toEqual([['pwsh.exe'], ['powershell.exe']]) + }) + + it('uses the resolved Git Bash executable for the configured sentinel', async () => { + const spawner = vi.fn(async () => ({ + segments: ['C:\\tools'], + ok: true, + failureReason: 'none' + })) + + configureWindowsShellPathHydration('git-bash', 'C:\\Program Files\\Git\\bin\\bash.exe') + await hydrateShellPath({ spawner }) + + expect(spawner).toHaveBeenCalledWith('C:\\Program Files\\Git\\bin\\bash.exe') + }) + + it('loads PowerShell profiles and parses their Windows PATH', async () => { + const proc = createMockShellProcess() + spawnMock.mockReturnValue(proc) + const resultPromise = hydrateShellPath({ force: true }) + await vi.waitFor(() => expect(spawnMock).toHaveBeenCalledOnce()) + + proc.stdout.emit( + 'data', + Buffer.from( + 'profile banner\r\n__ORCA_SHELL_PATH__C:\\profile-node;C:\\Windows__ORCA_SHELL_PATH__' + ) + ) + proc.emit('close', 0, null) + + await expect(resultPromise).resolves.toEqual({ + segments: ['C:\\profile-node', 'C:\\Windows'], + ok: true, + failureReason: 'none' + }) + expect(spawnMock).toHaveBeenCalledWith( + 'powershell.exe', + ['-NoLogo', '-Command', expect.stringContaining('$env:Path')], + expect.objectContaining({ windowsHide: true }) + ) + expect(spawnMock.mock.calls[0]?.[1]).not.toContain('-NoProfile') + }) + + it('converts a Git Bash PATH to Windows segments before parsing it', async () => { + const proc = createMockShellProcess() + spawnMock.mockReturnValue(proc) + const resultPromise = hydrateShellPath({ + shellOverride: 'C:\\Program Files\\Git\\bin\\bash.exe', + force: true + }) + await vi.waitFor(() => expect(spawnMock).toHaveBeenCalledOnce()) + + proc.stdout.emit( + 'data', + Buffer.from('__ORCA_SHELL_PATH__C:\\git-node;C:\\Windows__ORCA_SHELL_PATH__') + ) + proc.emit('close', 0, null) + + await expect(resultPromise).resolves.toEqual({ + segments: ['C:\\git-node', 'C:\\Windows'], + ok: true, + failureReason: 'none' + }) + expect(spawnMock).toHaveBeenCalledWith( + 'C:\\Program Files\\Git\\bin\\bash.exe', + ['-ilc', expect.stringContaining('cygpath -wp "$PATH"')], + expect.objectContaining({ windowsHide: true }) + ) + }) + + it.each(['cmd.exe', 'wsl.exe', 'git-bash', 'C:\\Tools\\zsh.exe'])( + 'does not merge %s guest or profile-less paths', + async (shell) => { + const spawner = vi.fn() + + configureWindowsShellPathHydration(shell) + + await expect(hydrateShellPath({ spawner })).resolves.toEqual({ + segments: [], + ok: false, + failureReason: 'no_shell' + }) + expect(spawner).not.toHaveBeenCalled() + } + ) + + it('serializes shell changes, discards stale results, and caches the newest request', async () => { + const powerShellResult = deferred() + const gitBashResult = deferred() + let activeProbes = 0 + let maxActiveProbes = 0 + const powerShellSpawner = vi.fn(async () => { + activeProbes += 1 + maxActiveProbes = Math.max(maxActiveProbes, activeProbes) + const result = await powerShellResult.promise + activeProbes -= 1 + return result + }) + const gitBashSpawner = vi.fn(async () => { + activeProbes += 1 + maxActiveProbes = Math.max(maxActiveProbes, activeProbes) + const result = await gitBashResult.promise + activeProbes -= 1 + return result + }) + process.env.PATH = 'C:\\Windows' + const powerShellReady = hydrateShellPath({ spawner: powerShellSpawner, force: true }) + const powerShellMerged = powerShellReady.then((result) => mergePathSegments(result.segments)) + await vi.waitFor(() => expect(powerShellSpawner).toHaveBeenCalledWith('powershell.exe')) + + configureWindowsShellPathHydration('git-bash', 'C:\\Git\\bin\\bash.exe') + const gitBashReady = hydrateShellPath({ spawner: gitBashSpawner, force: true }) + const gitBashMerged = gitBashReady.then((result) => mergePathSegments(result.segments)) + const cachedGitBashReady = hydrateShellPath({ spawner: gitBashSpawner }) + expect(cachedGitBashReady).toBe(gitBashReady) + expect(gitBashSpawner).not.toHaveBeenCalled() + + powerShellResult.resolve({ + segments: ['C:\\stale-powershell'], + ok: true, + failureReason: 'none' + }) + await vi.waitFor(() => expect(gitBashSpawner).toHaveBeenCalledWith('C:\\Git\\bin\\bash.exe')) + gitBashResult.resolve({ + segments: ['C:\\current-git-bash'], + ok: true, + failureReason: 'none' + }) + await Promise.all([powerShellMerged, gitBashMerged, cachedGitBashReady]) + + expect(maxActiveProbes).toBe(1) + expect(powerShellSpawner).toHaveBeenCalledOnce() + expect(gitBashSpawner).toHaveBeenCalledOnce() + expect(process.env.PATH).toBe('C:\\current-git-bash;C:\\Windows') + }) + + it('discards an active probe when the configured shell has no profile', async () => { + const powerShellResult = deferred() + const spawner = vi.fn(() => powerShellResult.promise) + const staleReady = hydrateShellPath({ spawner, force: true }) + await vi.waitFor(() => expect(spawner).toHaveBeenCalledOnce()) + + configureWindowsShellPathHydration('cmd.exe') + const currentReady = hydrateShellPath({ spawner }) + powerShellResult.resolve({ + segments: ['C:\\stale-powershell'], + ok: true, + failureReason: 'none' + }) + + await expect(Promise.all([staleReady, currentReady])).resolves.toEqual([ + { segments: [], ok: false, failureReason: 'no_shell' }, + { segments: [], ok: false, failureReason: 'no_shell' } + ]) + expect(spawner).toHaveBeenCalledOnce() + }) + + it('deduplicates PATH entries case-insensitively', () => { + process.env.PATH = 'C:\\Tools;C:\\Windows' + + expect(mergePathSegments(['c:\\tools', 'C:\\profile-node'])).toEqual(['C:\\profile-node']) + expect(process.env.PATH).toBe('c:\\tools;C:\\profile-node;C:\\Windows') + }) + + it('deduplicates PATH entries with trailing slashes', () => { + process.env.PATH = 'C:\\Windows\\;C:\\Tools' + + expect(mergePathSegments(['C:\\Windows', 'C:\\profile-node\\'])).toEqual(['C:\\profile-node\\']) + expect(process.env.PATH).toBe('C:\\Windows;C:\\profile-node\\;C:\\Tools') + }) + + it('removes profile entries when switching to a shell without a profile', () => { + process.env.PATH = 'C:\\Inherited;C:\\Windows' + mergePathSegments(['C:\\PowerShell-profile', 'C:\\Inherited']) + + configureWindowsShellPathHydration('cmd.exe') + + expect(process.env.PATH).toBe('C:\\Inherited;C:\\Windows') + }) + + it('replaces profile entries introduced by the previously configured shell', () => { + process.env.PATH = 'C:\\Inherited;C:\\Windows' + mergePathSegments(['C:\\PowerShell-profile', 'C:\\Inherited']) + + configureWindowsShellPathHydration('git-bash', 'C:\\Git\\bin\\bash.exe') + mergePathSegments(['C:\\GitBash-profile', 'C:\\Inherited']) + + expect(process.env.PATH).toBe('C:\\GitBash-profile;C:\\Inherited;C:\\Windows') + }) +}) diff --git a/src/main/startup/windows-shell-path-hydration.test.ts b/src/main/startup/windows-shell-path-hydration.test.ts new file mode 100644 index 00000000000..87175008bfd --- /dev/null +++ b/src/main/startup/windows-shell-path-hydration.test.ts @@ -0,0 +1,214 @@ +import { describe, expect, it, vi } from 'vitest' +import { createWindowsShellPathHydration } from './windows-shell-path-hydration' +import type { HydrationResult } from './hydrate-shell-path' + +type Deferred = { + promise: Promise + resolve: (value: T) => void +} + +function deferred(): Deferred { + let resolve!: (value: T) => void + const promise = new Promise((next) => { + resolve = next + }) + return { promise, resolve } +} + +function success(segment: string): HydrationResult { + return { segments: [segment], ok: true, failureReason: 'none' } +} + +describe('Windows shell PATH hydration coordination', () => { + it('serializes probes and never merges a superseded shell result', async () => { + const first = deferred() + const second = deferred() + const hydrate = vi.fn<() => Promise>() + hydrate.mockReturnValueOnce(first.promise).mockReturnValueOnce(second.promise) + const merge = vi.fn<(segments: string[]) => string[]>(() => []) + const configure = vi.fn() + const coordinator = createWindowsShellPathHydration({ + configure, + hydrate, + merge, + resolveGitBashPath: (shell) => (shell === 'git-bash' ? 'C:\\Git\\bin\\bash.exe' : null) + }) + + const powerShellReady = coordinator.hydrate('powershell.exe') + await vi.waitFor(() => expect(hydrate).toHaveBeenCalledTimes(1)) + const gitBashReady = coordinator.hydrate('git-bash') + + first.resolve(success('C:\\stale-powershell')) + await vi.waitFor(() => expect(hydrate).toHaveBeenCalledTimes(2)) + expect(merge).not.toHaveBeenCalled() + + second.resolve(success('C:\\current-git-bash')) + await Promise.all([powerShellReady, gitBashReady]) + + expect(merge).toHaveBeenCalledOnce() + expect(merge).toHaveBeenCalledWith(['C:\\current-git-bash']) + expect(configure).toHaveBeenLastCalledWith('git-bash', 'C:\\Git\\bin\\bash.exe', null) + }) + + it('coalesces queued shell changes to the latest configured shell', async () => { + const active = deferred() + const latest = deferred() + const hydrate = vi.fn<() => Promise>() + hydrate.mockReturnValueOnce(active.promise).mockReturnValueOnce(latest.promise) + const merge = vi.fn<(segments: string[]) => string[]>(() => []) + const coordinator = createWindowsShellPathHydration({ hydrate, merge }) + + const activeReady = coordinator.hydrate('powershell.exe') + await vi.waitFor(() => expect(hydrate).toHaveBeenCalledTimes(1)) + const skippedReady = coordinator.hydrate('cmd.exe') + const latestReady = coordinator.hydrate('powershell.exe') + active.resolve(success('C:\\stale')) + await vi.waitFor(() => expect(hydrate).toHaveBeenCalledTimes(2)) + latest.resolve(success('C:\\latest')) + + await Promise.all([activeReady, skippedReady, latestReady]) + expect(hydrate).toHaveBeenCalledTimes(2) + expect(merge).toHaveBeenCalledOnce() + expect(merge).toHaveBeenCalledWith(['C:\\latest']) + }) + + it('keeps the startup barrier behind shell changes queued while it waits', async () => { + const first = deferred() + const second = deferred() + const hydrate = vi.fn<() => Promise>() + hydrate.mockReturnValueOnce(first.promise).mockReturnValueOnce(second.promise) + const coordinator = createWindowsShellPathHydration({ hydrate }) + void coordinator.hydrate('powershell.exe', 'powershell.exe') + await vi.waitFor(() => expect(hydrate).toHaveBeenCalledOnce()) + let startupReady = false + const startupBarrier = coordinator.whenReady().then(() => { + startupReady = true + }) + + void coordinator.hydrate('git-bash') + first.resolve(success('C:\\stale')) + await vi.waitFor(() => expect(hydrate).toHaveBeenCalledTimes(2)) + expect(startupReady).toBe(false) + + second.resolve(success('C:\\latest')) + await startupBarrier + expect(startupReady).toBe(true) + }) + + it('uses the terminal safety chain for a custom PowerShell path', () => { + const configure = vi.fn() + const coordinator = createWindowsShellPathHydration({ + configure, + resolvePowerShellChain: () => [ + 'C:\\Resolved\\pwsh.exe', + 'C:\\Resolved\\powershell.exe', + 'C:\\Resolved\\cmd.exe' + ] + }) + + coordinator.configure('C:\\Program Files\\PowerShell\\7\\pwsh.exe', 'pwsh.exe') + + expect(configure).toHaveBeenCalledWith( + 'C:\\Resolved\\pwsh.exe', + null, + 'C:\\Resolved\\powershell.exe' + ) + }) + + it('configures refresh callers without spawning in development', () => { + const configure = vi.fn() + const hydrate = vi.fn<() => Promise>() + const coordinator = createWindowsShellPathHydration({ + configure, + hydrate, + resolveGitBashPath: () => 'C:\\Git\\bin\\bash.exe' + }) + + coordinator.configure('git-bash') + + expect(configure).toHaveBeenCalledWith('git-bash', 'C:\\Git\\bin\\bash.exe', null) + expect(hydrate).not.toHaveBeenCalled() + }) + + it.each([ + { + implementation: 'auto' as const, + expectedShell: 'pwsh.exe', + expectedFallback: 'powershell.exe' + }, + { + implementation: 'pwsh.exe' as const, + expectedShell: 'pwsh.exe', + expectedFallback: 'powershell.exe' + }, + { + implementation: 'powershell.exe' as const, + expectedShell: 'powershell.exe', + expectedFallback: null + } + ])( + 'configures $implementation to hydrate the effective PowerShell profile', + ({ implementation, expectedShell, expectedFallback }) => { + const configure = vi.fn() + const coordinator = createWindowsShellPathHydration({ + configure, + resolvePowerShellChain: (family) => + family === 'pwsh.exe' + ? ['pwsh.exe', 'powershell.exe', 'cmd.exe'] + : ['powershell.exe', 'cmd.exe'] + }) + + coordinator.configure('powershell.exe', implementation) + + expect(configure).toHaveBeenCalledWith(expectedShell, null, expectedFallback) + } + ) + + it('reconfigures when the PowerShell implementation changes', () => { + const configure = vi.fn() + const coordinator = createWindowsShellPathHydration({ + configure, + resolvePowerShellChain: (family) => + family === 'pwsh.exe' + ? ['pwsh.exe', 'powershell.exe', 'cmd.exe'] + : ['powershell.exe', 'cmd.exe'] + }) + + coordinator.configure('powershell.exe', 'powershell.exe') + coordinator.configure('powershell.exe', 'pwsh.exe') + + expect(configure).toHaveBeenNthCalledWith(1, 'powershell.exe', null, null) + expect(configure).toHaveBeenNthCalledWith(2, 'pwsh.exe', null, 'powershell.exe') + }) + + it('uses inbox PowerShell when pwsh has no safe executable behind its Store alias', () => { + const configure = vi.fn() + const coordinator = createWindowsShellPathHydration({ + configure, + resolvePowerShellChain: () => [ + 'C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe', + 'C:\\Windows\\System32\\cmd.exe' + ] + }) + + coordinator.configure('powershell.exe', 'auto') + + expect(configure).toHaveBeenCalledWith( + 'C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe', + null, + null + ) + }) + + it('skips hydration when no PowerShell executable resolves safely', () => { + const configure = vi.fn() + const coordinator = createWindowsShellPathHydration({ + configure, + resolvePowerShellChain: () => ['C:\\Windows\\System32\\cmd.exe'] + }) + + coordinator.configure('powershell.exe', 'auto') + + expect(configure).toHaveBeenCalledWith('C:\\Windows\\System32\\cmd.exe', null, null) + }) +}) diff --git a/src/main/startup/windows-shell-path-hydration.ts b/src/main/startup/windows-shell-path-hydration.ts new file mode 100644 index 00000000000..083ea8dbe57 --- /dev/null +++ b/src/main/startup/windows-shell-path-hydration.ts @@ -0,0 +1,107 @@ +import { win32 as pathWin32 } from 'node:path' +import { resolveWindowsGitBashShellPath } from '../git-bash' +import { + resolveEffectiveWindowsPowerShell, + type WindowsPowerShellImplementation, + type WindowsPowerShellShellFamily +} from '../providers/windows-powershell' +import { resolveWindowsPowerShellSpawnChain } from '../providers/windows-powershell-executable' +import { + configureWindowsShellPathHydration, + hydrateShellPath, + mergePathSegments, + type HydrationResult +} from './hydrate-shell-path' + +type WindowsShellPathHydrationOptions = { + configure?: ( + shell: string | null | undefined, + gitBashPath: string | null, + fallbackShell: string | null + ) => void + hydrate?: () => Promise + merge?: (segments: string[]) => string[] + resolveGitBashPath?: (shell: string) => string | null + resolvePowerShellChain?: (family: 'powershell.exe' | 'pwsh.exe') => string[] + warn?: (error: unknown) => void +} + +export function createWindowsShellPathHydration(options: WindowsShellPathHydrationOptions = {}) { + const configure = options.configure ?? configureWindowsShellPathHydration + const hydrate = options.hydrate ?? hydrateShellPath + const merge = options.merge ?? mergePathSegments + const resolveGitBashPath = options.resolveGitBashPath ?? resolveWindowsGitBashShellPath + const resolvePowerShellChain = + options.resolvePowerShellChain ?? resolveWindowsPowerShellSpawnChain + const warn = + options.warn ?? + ((error: unknown) => { + console.warn('[shell-path] Windows profile hydration failed; using inherited PATH:', error) + }) + let generation = 0 + let ready = Promise.resolve() + + const setConfiguredShell = ( + shell: string | null | undefined, + implementation?: WindowsPowerShellImplementation + ): void => { + const requestedShell = shell?.trim() || 'powershell.exe' + const basename = pathWin32.basename(requestedShell).toLowerCase() + const shellFamily: WindowsPowerShellShellFamily = + basename === 'powershell.exe' || basename === 'pwsh.exe' ? basename : undefined + const effectivePowerShell = resolveEffectiveWindowsPowerShell({ + shellFamily, + implementation, + pwshAvailable: true + }) + if (!effectivePowerShell) { + configure(requestedShell, resolveGitBashPath(requestedShell), null) + return + } + const spawnChain = resolvePowerShellChain(effectivePowerShell) + const profileShell = spawnChain[0] ?? 'cmd.exe' + const fallbackShell = spawnChain.slice(1).find((candidate) => { + const candidateBasename = pathWin32.basename(candidate).toLowerCase() + return candidateBasename === 'powershell.exe' || candidateBasename === 'pwsh.exe' + }) + configure(profileShell, null, fallbackShell ?? null) + } + + return { + configure: (shell, implementation?: WindowsPowerShellImplementation) => { + generation += 1 + setConfiguredShell(shell, implementation) + }, + hydrate: (shell, implementation?: WindowsPowerShellImplementation) => { + generation += 1 + const requestGeneration = generation + setConfiguredShell(shell, implementation) + ready = ready.then(async () => { + if (requestGeneration !== generation) { + return + } + try { + const result = await hydrate() + if (requestGeneration === generation && result.ok) { + merge(result.segments) + } + } catch (error) { + if (requestGeneration === generation) { + warn(error) + } + } + }) + return ready + }, + whenReady: async () => { + let pending = ready + while (true) { + await pending + if (pending === ready) { + return + } + pending = ready + } + } + } +} From 8a38e2f3f8d91a3d884d1ae54ab17aae5d466bdf Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sun, 9 Aug 2026 18:50:56 -0700 Subject: [PATCH 10/12] refactor(renderer): reuse existing control paths (#13425) --- .../components/browser-pane/useGrabMode.ts | 39 ++++++------------- src/renderer/src/lib/setup-script-prompt.ts | 20 +--------- 2 files changed, 12 insertions(+), 47 deletions(-) diff --git a/src/renderer/src/components/browser-pane/useGrabMode.ts b/src/renderer/src/components/browser-pane/useGrabMode.ts index b3ee2580572..018fb0368fe 100644 --- a/src/renderer/src/components/browser-pane/useGrabMode.ts +++ b/src/renderer/src/components/browser-pane/useGrabMode.ts @@ -183,34 +183,6 @@ export function useGrabMode(browserPageId: string): GrabModeHook { } }, [mountedRef]) - const toggle = useCallback(() => { - if ((state === 'idle' || state === 'error') && grabTabIdRef.current === null) { - setError(null) - setPayload(null) - setContextMenu(false) - void armAndAwait() - } else { - // Disable grab mode - const targetTabId = grabTabIdRef.current ?? browserTabIdRef.current - armGenerationRef.current += 1 - void window.api.browser.setGrabMode({ - browserPageId: targetTabId, - enabled: false - }) - if (activeOpIdRef.current) { - void window.api.browser.cancelGrab({ - browserPageId: targetTabId - }) - activeOpIdRef.current = null - } - grabTabIdRef.current = null - setState('idle') - setPayload(null) - setError(null) - setContextMenu(false) - } - }, [state, armAndAwait]) - const cancel = useCallback(() => { const targetTabId = grabTabIdRef.current ?? browserTabIdRef.current armGenerationRef.current += 1 @@ -231,6 +203,17 @@ export function useGrabMode(browserPageId: string): GrabModeHook { setContextMenu(false) }, []) + const toggle = useCallback(() => { + if ((state === 'idle' || state === 'error') && grabTabIdRef.current === null) { + setError(null) + setPayload(null) + setContextMenu(false) + void armAndAwait() + } else { + cancel() + } + }, [state, armAndAwait, cancel]) + // Why: Copy re-arms so the user can quickly pick another element without // re-clicking the toolbar button. Attach to AI exits because the user's // intent is to continue in the chat, not keep selecting. diff --git a/src/renderer/src/lib/setup-script-prompt.ts b/src/renderer/src/lib/setup-script-prompt.ts index d85a3a9ec30..f25b806d1ef 100644 --- a/src/renderer/src/lib/setup-script-prompt.ts +++ b/src/renderer/src/lib/setup-script-prompt.ts @@ -4,6 +4,7 @@ import type { SetupScriptImportCandidate } from '../../../shared/setup-script-im import type { Repo, RepoHookSettings } from '../../../shared/types' import type { HookCheckResult } from '@/runtime/runtime-hooks-client' import { isRuntimeScopeForbiddenError } from '@/runtime/runtime-rpc-client' +import { hasEffectiveSetupCommand } from './setup-script-status' const SETUP_SCRIPT_PROMPT_DISMISSAL_PREFIX = 'generation-v1:' @@ -69,25 +70,6 @@ export async function inspectSetupScriptPromptState({ } } -export function hasEffectiveSetupCommand(repo: Repo, hooksResult: HookCheckResult): boolean { - const localSetup = repo.hookSettings?.scripts?.setup?.trim() - const sharedSetup = hooksResult.hooks?.scripts?.setup?.trim() - const rawPolicy = repo.hookSettings?.commandSourcePolicy - const sourcePolicy = resolveHookCommandSourcePolicy(rawPolicy, { - hasLocalScript: Boolean(localSetup) - }) - - if (sourcePolicy === 'local-only') { - return Boolean(localSetup) - } - - if (sourcePolicy === 'run-both') { - return Boolean(sharedSetup || localSetup) - } - - return Boolean(sharedSetup) -} - export function ignoresSharedSetupScripts(repo: Pick): boolean { const localSetup = repo.hookSettings?.scripts?.setup?.trim() return ( From 934faaec7634b34bf4923de7e8e1574216c594ec Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sun, 9 Aug 2026 18:54:22 -0700 Subject: [PATCH 11/12] refactor: deduplicate Linear team readers (#13417) --- src/main/linear/teams.test.ts | 30 ++++++++- src/main/linear/teams.ts | 118 +++++++--------------------------- 2 files changed, 51 insertions(+), 97 deletions(-) diff --git a/src/main/linear/teams.test.ts b/src/main/linear/teams.test.ts index d04845e6f28..00d68988bab 100644 --- a/src/main/linear/teams.test.ts +++ b/src/main/linear/teams.test.ts @@ -5,10 +5,12 @@ import { credentialDecryptionMessage } from '../../shared/integration-credential const getClients = vi.fn() const clearToken = vi.fn() const isAuthError = vi.fn() +const acquire = vi.fn().mockResolvedValue(undefined) +const release = vi.fn() vi.mock('./client', () => ({ - acquire: vi.fn().mockResolvedValue(undefined), - release: vi.fn(), + acquire, + release, getClients: (...args: unknown[]) => getClients(...args), isAuthError: (...args: unknown[]) => isAuthError(...args), clearToken: (...args: unknown[]) => clearToken(...args) @@ -283,6 +285,30 @@ describe('Linear teams', () => { expect(members).toHaveBeenCalledWith({ first: 100 }) }) + it.each([ + ['getTeamStates', 'getTeamStatesOrThrow'], + ['getTeamLabels', 'getTeamLabelsOrThrow'], + ['getTeamMembers', 'getTeamMembersOrThrow'] + ] as const)('preserves the %s error policy', async (fallbackReader, throwingReader) => { + const error = new Error('request failed') + const entry = makeTeamLookupEntry('workspace-1', 'Workspace', null) + vi.mocked(entry.client.team).mockRejectedValue(error) + getClients.mockReturnValue([entry]) + const readers = await import('./teams') + const warn = vi.spyOn(console, 'warn').mockImplementation(() => undefined) + + await expect(readers[fallbackReader]('team-1', 'workspace-1')).resolves.toEqual([]) + await expect(readers[throwingReader]('team-1', 'workspace-1')).rejects.toBe(error) + expect(warn).toHaveBeenCalledWith(`[linear] ${fallbackReader} failed:`, error) + expect(release).toHaveBeenCalledTimes(2) + + isAuthError.mockReturnValue(true) + await expect(readers[fallbackReader]('team-1', 'workspace-1')).rejects.toBe(error) + expect(clearToken).toHaveBeenCalledWith('workspace-1') + expect(release).toHaveBeenCalledTimes(3) + warn.mockRestore() + }) + it('surfaces Linear credential decrypt errors on active team reads', async () => { const error = new Error(credentialDecryptionMessage('Linear')) getClients.mockImplementation(() => { diff --git a/src/main/linear/teams.ts b/src/main/linear/teams.ts index e06697e8d46..004c8fbd92a 100644 --- a/src/main/linear/teams.ts +++ b/src/main/linear/teams.ts @@ -1,3 +1,4 @@ +import type { Team } from '@linear/sdk' import type { LinearTeam, LinearWorkflowState, @@ -108,10 +109,12 @@ export async function listTeamsForAgent( } } -export async function getTeamStates( +async function readTeamResource( teamId: string, - workspaceId?: string | null -): Promise { + workspaceId: string | null | undefined, + fetchAll: (team: Team) => Promise, + fallbackWarning?: string +): Promise { const entry = getClients(workspaceId)[0] if (!entry) { return [] @@ -119,137 +122,62 @@ export async function getTeamStates( await acquire() try { - const team = await entry.client.team(teamId) - return await fetchAllTeamStates(team) + return await fetchAll(await entry.client.team(teamId)) } catch (error) { if (isAuthError(error)) { clearToken(entry.workspace.id) throw error } - console.warn('[linear] getTeamStates failed:', error) + if (!fallbackWarning) { + throw error + } + console.warn(`[linear] ${fallbackWarning} failed:`, error) return [] } finally { release() } } +export async function getTeamStates( + teamId: string, + workspaceId?: string | null +): Promise { + return readTeamResource(teamId, workspaceId, fetchAllTeamStates, 'getTeamStates') +} + export async function getTeamStatesOrThrow( teamId: string, workspaceId?: string | null ): Promise { - const entry = getClients(workspaceId)[0] - if (!entry) { - return [] - } - - await acquire() - try { - const team = await entry.client.team(teamId) - return await fetchAllTeamStates(team) - } catch (error) { - if (isAuthError(error)) { - clearToken(entry.workspace.id) - } - throw error - } finally { - release() - } + return readTeamResource(teamId, workspaceId, fetchAllTeamStates) } export async function getTeamLabels( teamId: string, workspaceId?: string | null ): Promise { - const entry = getClients(workspaceId)[0] - if (!entry) { - return [] - } - - await acquire() - try { - const team = await entry.client.team(teamId) - return await fetchAllTeamLabels(team) - } catch (error) { - if (isAuthError(error)) { - clearToken(entry.workspace.id) - throw error - } - console.warn('[linear] getTeamLabels failed:', error) - return [] - } finally { - release() - } + return readTeamResource(teamId, workspaceId, fetchAllTeamLabels, 'getTeamLabels') } export async function getTeamLabelsOrThrow( teamId: string, workspaceId?: string | null ): Promise { - const entry = getClients(workspaceId)[0] - if (!entry) { - return [] - } - - await acquire() - try { - const team = await entry.client.team(teamId) - return await fetchAllTeamLabels(team) - } catch (error) { - if (isAuthError(error)) { - clearToken(entry.workspace.id) - } - throw error - } finally { - release() - } + return readTeamResource(teamId, workspaceId, fetchAllTeamLabels) } export async function getTeamMembers( teamId: string, workspaceId?: string | null ): Promise { - const entry = getClients(workspaceId)[0] - if (!entry) { - return [] - } - - await acquire() - try { - const team = await entry.client.team(teamId) - return await fetchAllTeamMembers(team) - } catch (error) { - if (isAuthError(error)) { - clearToken(entry.workspace.id) - throw error - } - console.warn('[linear] getTeamMembers failed:', error) - return [] - } finally { - release() - } + return readTeamResource(teamId, workspaceId, fetchAllTeamMembers, 'getTeamMembers') } export async function getTeamMembersOrThrow( teamId: string, workspaceId?: string | null ): Promise { - const entry = getClients(workspaceId)[0] - if (!entry) { - return [] - } - - await acquire() - try { - const team = await entry.client.team(teamId) - return await fetchAllTeamMembers(team) - } catch (error) { - if (isAuthError(error)) { - clearToken(entry.workspace.id) - } - throw error - } finally { - release() - } + return readTeamResource(teamId, workspaceId, fetchAllTeamMembers) } export async function getViewerForWorkspaceOrThrow( From 403c60bbadb734d870a22c372aca8a988e348d21 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sun, 9 Aug 2026 18:55:36 -0700 Subject: [PATCH 12/12] refactor(mobile): remove unused review code (#13421) --- mobile/src/source-control/mobile-pr-create.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/mobile/src/source-control/mobile-pr-create.ts b/mobile/src/source-control/mobile-pr-create.ts index 20f908d3df2..65a70a63ed9 100644 --- a/mobile/src/source-control/mobile-pr-create.ts +++ b/mobile/src/source-control/mobile-pr-create.ts @@ -2,7 +2,6 @@ import { hostedReviewCopy } from './hosted-review-copy' import { buildMobileHostedReviewCreateParams, createMobileHostedReview, - fetchMobileHostedReviewEligibility, mobileRepoSelectorFromWorktreeId, resolveMobileHostedReviewPrefill, shouldPushBeforeMobileHostedReviewCreate, @@ -20,7 +19,6 @@ export type MobilePrCreateOutcome = MobileHostedReviewCreateOutcome export { buildMobileHostedReviewCreateParams as buildMobilePrCreateParams, createMobileHostedReview as createMobilePr, - fetchMobileHostedReviewEligibility as fetchMobilePrEligibility, mobileRepoSelectorFromWorktreeId, resolveMobileHostedReviewPrefill as resolveMobilePrPrefill, shouldPushBeforeMobileHostedReviewCreate as shouldPushBeforeMobilePrCreate