From c6d21804178c286bfa3c658c4ef90c4a32fed7ee Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Fri, 31 Jul 2026 15:35:56 -0700 Subject: [PATCH 1/3] fix(mobile): keep source-control layout steady while Create PR eligibility loads (#11467) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(mobile): keep source-control layout steady while Create PR eligibility loads The Create PR entry unmounted until the first hostedReview.getCreationEligibility answer arrived, so on a cold open the changed-files list painted first and then shifted down 54pt (createPrBlock marginTop 12 + createPrButton minHeight 42) when the button appeared — while the user was already tapping (#8411). - buildMobileCreatePrAction: cold loading now reserves the row with a disabled placeholder instead of unmounting it. - useMobileHostedReviewEligibility: a fetch-imminent idle frame renders as an in-flight load, so the reservation is present on the first painted frame. - New per-worktree+branch memory of the last resolved eligibility seeds cold loads, so branches whose answer is hidden (existing review, unsupported provider) do not get a placeholder that collapses on every reopen. Fixes #8411 * fix(mobile): harden source-control layout reservation * fix(mobile): keep review status row footprint fixed * fix(mobile): derive eligibility state from keyed snapshots --- .../MobileSourceControlCreatePrEntry.tsx | 17 +- .../mobile-create-pr-action.test.ts | 109 +++++- .../source-control/mobile-create-pr-action.ts | 46 ++- .../mobile-source-control-styles.ts | 13 +- ...e-mobile-hosted-review-eligibility.test.ts | 324 +++++++++++++++++- .../use-mobile-hosted-review-eligibility.ts | 114 +++--- ...-mobile-source-control-create-pr-action.ts | 3 + .../use-mobile-source-control-state.ts | 1 + 8 files changed, 525 insertions(+), 102 deletions(-) diff --git a/mobile/src/source-control/MobileSourceControlCreatePrEntry.tsx b/mobile/src/source-control/MobileSourceControlCreatePrEntry.tsx index c58838e68aa..b60c632175d 100644 --- a/mobile/src/source-control/MobileSourceControlCreatePrEntry.tsx +++ b/mobile/src/source-control/MobileSourceControlCreatePrEntry.tsx @@ -13,6 +13,7 @@ export function MobileSourceControlCreatePrEntry({ action }: Props) { return null } const enabled = !action.disabled + const copy = action.hint ?? action.label return ( )} - - {action.label} + + {copy} - {action.hint ? ( - - {action.hint} - - ) : null} ) } diff --git a/mobile/src/source-control/mobile-create-pr-action.test.ts b/mobile/src/source-control/mobile-create-pr-action.test.ts index 99b86cc85e7..c665b034f7f 100644 --- a/mobile/src/source-control/mobile-create-pr-action.test.ts +++ b/mobile/src/source-control/mobile-create-pr-action.test.ts @@ -4,7 +4,10 @@ import type { HostedReviewCreationEligibility, HostedReviewProvider } from '../../../src/shared/hosted-review' -import { buildMobileCreatePrAction } from './mobile-create-pr-action' +import { + buildMobileCreatePrAction, + type MobileCreatePrEligibilityState +} from './mobile-create-pr-action' function eligibility( overrides: Partial = {} @@ -108,26 +111,21 @@ describe('buildMobileCreatePrAction', () => { 'unsupported_provider', 'detached_head', null - ])('hides %s', (blockedReason) => { + ])('keeps a disabled status row for %s', (blockedReason) => { const { descriptor } = action({ eligibility: eligibility({ canCreate: false, blockedReason }) }) - expect(descriptor.visible).toBe(false) + expect(descriptor).toMatchObject({ visible: true, disabled: true }) + expect(descriptor.hint).toBeTruthy() }) - it('hides cold loading, errors, and missing branches', () => { - const onCreatePr = vi.fn() - - expect( - buildMobileCreatePrAction({ - branch: 'feature', - eligibilityState: { kind: 'loading', eligibility: null }, - busyAction: null, - onCreatePr - }).visible - ).toBe(false) - expect(action({ eligibility: null }).descriptor.visible).toBe(false) + it('keeps an unavailable row after errors but hides missing branches', () => { + expect(action({ eligibility: null }).descriptor).toMatchObject({ + visible: true, + disabled: true, + label: 'Review status unavailable' + }) expect(action({ branch: null }).descriptor.visible).toBe(false) }) @@ -146,12 +144,16 @@ describe('buildMobileCreatePrAction', () => { expect(onCreatePr).not.toHaveBeenCalled() }) - it('hides any provider that does not support hosted-review creation', () => { + it('keeps a disabled status row for providers without review creation', () => { const { descriptor } = action({ eligibility: eligibility({ provider: 'bitbucket' as HostedReviewProvider }) }) - expect(descriptor.visible).toBe(false) + expect(descriptor).toMatchObject({ + visible: true, + disabled: true, + label: 'Review creation unavailable for this provider' + }) }) it('keeps a creatable button visible but disabled while a newer eligibility loads', () => { @@ -167,3 +169,76 @@ describe('buildMobileCreatePrAction', () => { expect(descriptor).toMatchObject({ visible: true, disabled: true, loading: false }) }) }) + +// Issue #8411: the Create PR entry renders directly above the Stage All row and +// the changed-files list, so appearing late shifts the list down by +// createPrBlock marginTop (12) + createPrButton height (42) = 54pt while the +// user is already reading it. The row must keep its footprint across the cold +// eligibility fetch. +describe('cold-mount layout stability (issue #8411)', () => { + // useMobileHostedReviewEligibility's real cold-mount sequence for a creatable + // branch: no prior snapshot exists, so `loading` carries eligibility: null. + const coldMount: MobileCreatePrEligibilityState[] = [ + { kind: 'loading', eligibility: null }, + { kind: 'ready', eligibility: eligibility({ canCreate: true }) } + ] + + it('reserves the button row while the first eligibility request is in flight', () => { + const descriptors = coldMount.map((eligibilityState) => + buildMobileCreatePrAction({ + branch: 'feature', + eligibilityState, + busyAction: null, + onCreatePr: vi.fn() + }) + ) + + const footprint = descriptors.map(({ visible }) => visible) + + // On the buggy parent this was [false, true] -- the false -> true step is the jump. + expect(footprint).toEqual([true, true]) + }) + + it.each([ + [ + 'existing review', + { + kind: 'ready', + eligibility: eligibility({ canCreate: false, blockedReason: 'existing_review' }) + } satisfies MobileCreatePrEligibilityState + ], + [ + 'unsupported provider', + { + kind: 'ready', + eligibility: eligibility({ provider: 'bitbucket' as HostedReviewProvider }) + } satisfies MobileCreatePrEligibilityState + ], + ['eligibility error', { kind: 'error' } satisfies MobileCreatePrEligibilityState] + ])('keeps the cold-load footprint when %s resolves', (_label, resolvedState) => { + const footprint = [coldMount[0], resolvedState].map((eligibilityState) => { + const descriptor = buildMobileCreatePrAction({ + branch: 'feature', + eligibilityState, + busyAction: null, + onCreatePr: vi.fn() + }) + return descriptor.visible + }) + + expect(footprint).toEqual([true, true]) + }) + + it('does not offer a tappable action against unresolved eligibility', () => { + const descriptor = buildMobileCreatePrAction({ + branch: 'feature', + eligibilityState: { kind: 'loading', eligibility: null }, + busyAction: null, + onCreatePr: vi.fn() + }) + + // Reserving space must not make the placeholder actionable: there is no + // canCreate/pushFirst answer yet, so a tap has nothing correct to do. + expect(descriptor.disabled).toBe(true) + }) +}) diff --git a/mobile/src/source-control/mobile-create-pr-action.ts b/mobile/src/source-control/mobile-create-pr-action.ts index 0636fc01588..fe75cc60a58 100644 --- a/mobile/src/source-control/mobile-create-pr-action.ts +++ b/mobile/src/source-control/mobile-create-pr-action.ts @@ -1,7 +1,4 @@ -import type { - HostedReviewCreationBlockedReason, - HostedReviewCreationEligibility -} from '../../../src/shared/hosted-review' +import type { HostedReviewCreationEligibility } from '../../../src/shared/hosted-review' import { supportsHostedReviewCreation } from '../../../src/shared/hosted-review-creation-providers' import { hostedReviewCopy } from './hosted-review-copy' import { getMobilePrCreateBlockMessage } from './mobile-pr-create' @@ -29,13 +26,6 @@ export type BuildMobileCreatePrActionArgs = { onCreatePr: (pushFirst: boolean) => void } -const HIDDEN_BLOCKED_REASONS = new Set([ - 'detached_head', - 'existing_review', - 'unsupported_provider', - null -]) - const BUSY_ACTIONS = new Set(['create-pr', 'push-create-pr']) function hiddenAction(onPress: () => void): MobileCreatePrAction { @@ -56,18 +46,42 @@ export function buildMobileCreatePrAction({ onCreatePr }: BuildMobileCreatePrActionArgs): MobileCreatePrAction { const noop = () => {} - if (!branch || eligibilityState.kind === 'idle' || eligibilityState.kind === 'error') { + if (!branch || eligibilityState.kind === 'idle') { return hiddenAction(noop) } + if (eligibilityState.kind === 'error') { + return { + visible: true, + label: 'Review status unavailable', + disabled: true, + loading: false, + pushFirst: false, + onPress: noop + } + } const eligibility = eligibilityState.eligibility if (!eligibility) { - return hiddenAction(noop) + return { + visible: true, + label: 'Checking review status…', + disabled: true, + loading: true, + pushFirst: false, + onPress: noop + } } // Why: mirror desktop's structural provider gate (supportsHostedReviewCreation) // instead of relying on the host always emitting a hidden blockedReason for // non-creatable providers like bitbucket. if (!supportsHostedReviewCreation(eligibility.provider)) { - return hiddenAction(noop) + return { + visible: true, + label: 'Review creation unavailable for this provider', + disabled: true, + loading: false, + pushFirst: false, + onPress: noop + } } const copy = hostedReviewCopy(eligibility.provider) const label = `Create ${copy.titleLabel}` @@ -95,10 +109,6 @@ export function buildMobileCreatePrAction({ } } - if (HIDDEN_BLOCKED_REASONS.has(eligibility.blockedReason)) { - return hiddenAction(noop) - } - const hint = getMobilePrCreateBlockMessage({ provider: eligibility.provider, diff --git a/mobile/src/source-control/mobile-source-control-styles.ts b/mobile/src/source-control/mobile-source-control-styles.ts index d8dc20a1469..fe8d8da0a90 100644 --- a/mobile/src/source-control/mobile-source-control-styles.ts +++ b/mobile/src/source-control/mobile-source-control-styles.ts @@ -207,11 +207,10 @@ const baseStyles = StyleSheet.create({ fontWeight: '600' }, createPrBlock: { - marginTop: spacing.md, - gap: spacing.xs + marginTop: spacing.md }, createPrButton: { - minHeight: 42, + height: 42, borderRadius: radii.button, backgroundColor: colors.textPrimary, alignItems: 'center', @@ -236,10 +235,12 @@ const baseStyles = StyleSheet.create({ createPrButtonTextDisabled: { color: colors.textSecondary }, - createPrHint: { - color: colors.textMuted, + createPrButtonHint: { fontSize: typography.metaSize, - lineHeight: 16 + fontWeight: '600', + lineHeight: 16, + textAlign: 'center', + flexShrink: 1 } }) 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 cc1f4965dc8..13109a610db 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,10 +1,48 @@ -import { describe, expect, it } from 'vitest' +import { createElement } from 'react' +import { act, create, type ReactTestRenderer } from 'react-test-renderer' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type { HostedReviewCreationEligibility } from '../../../src/shared/hosted-review' import { - acceptsMobileHostedReviewEligibilityLoad, buildMobileHostedReviewEligibilityLoadKey, eligibilityStateAfterMobileHostedReviewError, - shouldFetchMobileHostedReviewEligibility + renderedMobileHostedReviewEligibilityState, + shouldFetchMobileHostedReviewEligibility, + useMobileHostedReviewEligibility, + type MobileHostedReviewEligibilityLoadKey, + type MobileHostedReviewEligibilityLoadSnapshot } from './use-mobile-hosted-review-eligibility' +import type { MobileCreatePrEligibilityState } from './mobile-create-pr-action' + +function eligibility( + overrides: Partial = {} +): HostedReviewCreationEligibility { + return { + provider: 'github', + review: null, + canCreate: true, + blockedReason: null, + nextAction: null, + defaultBaseRef: 'main', + title: 'feature', + body: '', + ...overrides + } +} + +function deferred(): { promise: Promise; resolve: (value: T) => void } { + let resolve!: (value: T) => void + const promise = new Promise((resolvePromise) => { + resolve = resolvePromise + }) + return { promise, resolve } +} + +function loadSnapshot( + key: MobileHostedReviewEligibilityLoadKey, + state: MobileCreatePrEligibilityState +): MobileHostedReviewEligibilityLoadSnapshot { + return { key, state } +} describe('mobile hosted review eligibility loader core', () => { it('does not fetch while disconnected or detached', () => { @@ -24,8 +62,9 @@ describe('mobile hosted review eligibility loader core', () => { ).toBe(false) }) - it('accepts only the latest generation for the current worktree branch identity', () => { + it('builds distinct keys for different branches', () => { const first = buildMobileHostedReviewEligibilityLoadKey({ + hostId: 'host-1', worktreeId: 'wt-1', branch: 'feature-a', hasUpstream: true, @@ -34,6 +73,7 @@ describe('mobile hosted review eligibility loader core', () => { hasUncommittedChanges: false }) const second = buildMobileHostedReviewEligibilityLoadKey({ + hostId: 'host-1', worktreeId: 'wt-1', branch: 'feature-b', hasUpstream: true, @@ -42,17 +82,281 @@ describe('mobile hosted review eligibility loader core', () => { hasUncommittedChanges: false }) + expect(first.identity).not.toBe(second.identity) + expect(first.fetch).not.toBe(second.fetch) + }) + + it('scopes load identity to the paired host', () => { + const input = { + worktreeId: 'repo-1::/workspace', + branch: 'feature', + hasUpstream: true, + ahead: 0, + behind: 0, + hasUncommittedChanges: false + } + const local = buildMobileHostedReviewEligibilityLoadKey({ ...input, hostId: 'local' }) + const ssh = buildMobileHostedReviewEligibilityLoadKey({ ...input, hostId: 'ssh-builder' }) + + expect(local.identity).not.toBe(ssh.identity) + expect(local.fetch).not.toBe(ssh.fetch) + }) + + it('renders a superseded same-identity snapshot as loading', () => { + const input = { + hostId: 'host-1', + worktreeId: 'wt-1', + branch: 'feature', + hasUpstream: true, + ahead: 0, + behind: 0 + } + const older = buildMobileHostedReviewEligibilityLoadKey({ + ...input, + hasUncommittedChanges: false + }) + const newest = buildMobileHostedReviewEligibilityLoadKey({ + ...input, + hasUncommittedChanges: true + }) + + expect(older.identity).toBe(newest.identity) expect( - acceptsMobileHostedReviewEligibilityLoad({ - generation: 1, - currentGeneration: 2, - identity: first.identity, - currentIdentity: second.identity + renderedMobileHostedReviewEligibilityState({ + snapshot: loadSnapshot(older, { kind: 'ready', eligibility: eligibility() }), + key: newest, + shouldFetch: true }) - ).toBe(false) + ).toMatchObject({ kind: 'loading', eligibility: eligibility() }) }) it('fails closed after errors', () => { expect(eligibilityStateAfterMobileHostedReviewError()).toEqual({ kind: 'error' }) }) }) + +// #8411: what the hook returns is what paints. A fetch-imminent frame must not +// render as `idle` (hidden row) or the Create PR row pops in a frame later. +describe('rendered eligibility state', () => { + it('renders fetch-imminent idle as an in-flight load', () => { + const key = buildMobileHostedReviewEligibilityLoadKey({ + hostId: 'host-1', + worktreeId: 'wt-1', + branch: 'feature', + hasUpstream: true, + ahead: 0, + behind: 0, + hasUncommittedChanges: false + }) + expect( + renderedMobileHostedReviewEligibilityState({ + snapshot: loadSnapshot(key, { kind: 'idle' }), + key, + shouldFetch: true + }) + ).toEqual({ kind: 'loading', eligibility: null }) + }) + + it('passes resolved and refetch states through untouched', () => { + const key = buildMobileHostedReviewEligibilityLoadKey({ + hostId: 'host-1', + worktreeId: 'wt-1', + branch: 'feature', + hasUpstream: true, + ahead: 0, + behind: 0, + hasUncommittedChanges: false + }) + const ready = { kind: 'ready', eligibility: eligibility() } as const + const refetch = { kind: 'loading', eligibility: eligibility() } as const + const error = { kind: 'error' } as const + + for (const state of [ready, refetch, error]) { + expect( + renderedMobileHostedReviewEligibilityState({ + snapshot: loadSnapshot(key, state), + key, + shouldFetch: true + }) + ).toBe(state) + } + }) + + it('renders idle when a fetch is not possible, hiding stale snapshots in the same render', () => { + const key = buildMobileHostedReviewEligibilityLoadKey({ + hostId: 'host-1', + worktreeId: 'wt-1', + branch: 'feature', + hasUpstream: true, + ahead: 0, + behind: 0, + hasUncommittedChanges: false + }) + expect( + renderedMobileHostedReviewEligibilityState({ + snapshot: loadSnapshot(key, { kind: 'ready', eligibility: eligibility() }), + key, + shouldFetch: false + }) + ).toEqual({ kind: 'idle' }) + }) +}) + +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 + }) + + it('does not let an older response overwrite the newest state', async () => { + type Response = { ok: true; result: HostedReviewCreationEligibility } + const requests: ReturnType>[] = [] + const client = { + sendRequest: vi.fn(() => { + const request = deferred() + requests.push(request) + return request.promise + }) + } + const newest = eligibility({ canCreate: false, blockedReason: 'existing_review' }) + const older = eligibility() + + function Harness({ dirty }: { dirty: boolean }): null { + renderedState = useMobileHostedReviewEligibility({ + client: client as never, + connState: 'connected', + hostId: 'host-1', + worktreeId: 'wt-1', + branch: 'feature', + hasUpstream: true, + ahead: 0, + behind: 0, + hasUncommittedChanges: dirty + }) + return null + } + + await act(async () => { + renderer = create(createElement(Harness, { dirty: false })) + await Promise.resolve() + }) + await act(async () => { + renderer?.update(createElement(Harness, { dirty: true })) + await Promise.resolve() + }) + expect(requests).toHaveLength(2) + + await act(async () => { + requests[1]!.resolve({ ok: true, result: newest }) + await Promise.resolve() + }) + await act(async () => { + requests[0]!.resolve({ ok: true, result: older }) + await Promise.resolve() + }) + + expect(renderedState).toMatchObject({ kind: 'ready', eligibility: newest }) + }) + + it('disables a ready snapshot in the render that changes its fetch key', async () => { + type Response = { ok: true; result: HostedReviewCreationEligibility } + const requests: ReturnType>[] = [] + const client = { + sendRequest: vi.fn(() => { + const request = deferred() + requests.push(request) + return request.promise + }) + } + const ready = eligibility() + + function Harness({ dirty }: { dirty: boolean }): null { + renderedState = useMobileHostedReviewEligibility({ + client: client as never, + connState: 'connected', + hostId: 'host-1', + worktreeId: 'wt-1', + branch: 'feature', + hasUpstream: true, + ahead: 0, + behind: 0, + hasUncommittedChanges: dirty + }) + return null + } + + await act(async () => { + renderer = create(createElement(Harness, { dirty: false })) + await Promise.resolve() + }) + await act(async () => { + requests[0]!.resolve({ ok: true, result: ready }) + await Promise.resolve() + }) + expect(renderedState).toMatchObject({ kind: 'ready', eligibility: ready }) + + await act(async () => { + renderer?.update(createElement(Harness, { dirty: true })) + await Promise.resolve() + }) + expect(renderedState).toMatchObject({ kind: 'loading', eligibility: ready }) + }) + + it('invalidates a request when its hook instance unmounts', async () => { + type Response = { ok: true; result: HostedReviewCreationEligibility } + const requests: ReturnType>[] = [] + const client = { + sendRequest: vi.fn(() => { + const request = deferred() + requests.push(request) + return request.promise + }) + } + const newest = eligibility({ canCreate: false, blockedReason: 'existing_review' }) + + function Harness(): null { + renderedState = useMobileHostedReviewEligibility({ + client: client as never, + connState: 'connected', + hostId: 'host-1', + worktreeId: 'wt-1', + branch: 'feature', + hasUpstream: true, + ahead: 0, + behind: 0, + hasUncommittedChanges: false + }) + return null + } + + await act(async () => { + renderer = create(createElement(Harness)) + await Promise.resolve() + }) + act(() => renderer?.unmount()) + renderer = null + await act(async () => { + renderer = create(createElement(Harness)) + await Promise.resolve() + }) + expect(requests).toHaveLength(2) + + await act(async () => { + requests[1]!.resolve({ ok: true, result: newest }) + await Promise.resolve() + }) + await act(async () => { + requests[0]!.resolve({ ok: true, result: eligibility() }) + await Promise.resolve() + }) + + expect(renderedState).toMatchObject({ kind: 'ready', eligibility: newest }) + }) +}) diff --git a/mobile/src/source-control/use-mobile-hosted-review-eligibility.ts b/mobile/src/source-control/use-mobile-hosted-review-eligibility.ts index ff5a98c433b..e229d495311 100644 --- a/mobile/src/source-control/use-mobile-hosted-review-eligibility.ts +++ b/mobile/src/source-control/use-mobile-hosted-review-eligibility.ts @@ -1,4 +1,4 @@ -import { useEffect, useRef, useState } from 'react' +import { useEffect, useState } from 'react' import type { HostedReviewCreationEligibility } from '../../../src/shared/hosted-review' import type { RpcClient } from '../transport/rpc-client' import type { ConnectionState } from '../transport/types' @@ -11,6 +11,7 @@ import type { MobileCreatePrEligibilityState } from './mobile-create-pr-action' export type MobileHostedReviewEligibilityLoaderInput = { client: RpcClient | null connState: ConnectionState + hostId: string worktreeId: string branch: string | null | undefined hasUpstream: boolean | undefined @@ -24,13 +25,19 @@ export type MobileHostedReviewEligibilityLoadKey = { fetch: string } +export type MobileHostedReviewEligibilityLoadSnapshot = { + key: MobileHostedReviewEligibilityLoadKey + state: MobileCreatePrEligibilityState +} + export function buildMobileHostedReviewEligibilityLoadKey( input: Omit ): MobileHostedReviewEligibilityLoadKey { const branch = input.branch ?? '' return { - identity: `${input.worktreeId}\0${branch}`, + identity: `${input.hostId}\0${input.worktreeId}\0${branch}`, fetch: [ + input.hostId, input.worktreeId, branch, String(input.hasUpstream ?? ''), @@ -47,25 +54,42 @@ export function shouldFetchMobileHostedReviewEligibility( return input.connState === 'connected' && input.client !== null && !!input.branch } -export function acceptsMobileHostedReviewEligibilityLoad(args: { - generation: number - currentGeneration: number - identity: string - currentIdentity: string -}): boolean { - return args.generation === args.currentGeneration && args.identity === args.currentIdentity -} - export function eligibilityStateAfterMobileHostedReviewError(): MobileCreatePrEligibilityState { return { kind: 'error' } } +export function renderedMobileHostedReviewEligibilityState(args: { + snapshot: MobileHostedReviewEligibilityLoadSnapshot + key: MobileHostedReviewEligibilityLoadKey + shouldFetch: boolean +}): MobileCreatePrEligibilityState { + if (!args.shouldFetch) { + return { kind: 'idle' } + } + const { snapshot, key } = args + if (snapshot.key.identity !== key.identity) { + return { kind: 'loading', eligibility: null } + } + const { state } = snapshot + if (snapshot.key.fetch !== key.fetch) { + return { + kind: 'loading', + eligibility: state.kind === 'ready' || state.kind === 'loading' ? state.eligibility : null + } + } + if (state.kind === 'idle') { + return { kind: 'loading', eligibility: null } + } + return state +} + export function useMobileHostedReviewEligibility( input: MobileHostedReviewEligibilityLoaderInput ): MobileCreatePrEligibilityState { const { client, connState, + hostId, worktreeId, branch, hasUpstream, @@ -74,11 +98,8 @@ export function useMobileHostedReviewEligibility( hasUncommittedChanges } = input const shouldFetch = shouldFetchMobileHostedReviewEligibility({ client, connState, branch }) - const [state, setState] = useState({ kind: 'idle' }) - const generationRef = useRef(0) - const currentIdentityRef = useRef('') - const lastResetIdentityRef = useRef('') const key = buildMobileHostedReviewEligibilityLoadKey({ + hostId, worktreeId, branch, hasUpstream, @@ -86,38 +107,35 @@ export function useMobileHostedReviewEligibility( behind, hasUncommittedChanges }) - - if (lastResetIdentityRef.current !== key.identity) { - lastResetIdentityRef.current = key.identity - setState({ kind: 'idle' }) - } - currentIdentityRef.current = key.identity + const [snapshot, setSnapshot] = useState({ + key: { identity: '', fetch: '' }, + state: { kind: 'idle' } + }) useEffect(() => { - const generation = generationRef.current + 1 - generationRef.current = generation - const isCurrent = () => - acceptsMobileHostedReviewEligibilityLoad({ - generation, - currentGeneration: generationRef.current, - identity: key.identity, - currentIdentity: currentIdentityRef.current - }) + let active = true if (!shouldFetch) { - if (isCurrent()) { - setState({ kind: 'idle' }) + setSnapshot({ key, state: { kind: 'idle' } }) + return () => { + active = false } - return } if (!client || !branch) { - return + return () => { + active = false + } } - setState((prev) => ({ - kind: 'loading', - eligibility: prev.kind === 'ready' ? prev.eligibility : null - })) + setSnapshot((previous) => { + const previousState = previous.state + const eligibility = + previous.key.identity === key.identity && + (previousState.kind === 'ready' || previousState.kind === 'loading') + ? previousState.eligibility + : null + return { key, state: { kind: 'loading', eligibility } } + }) const requestInput: MobileHostedReviewEligibilityInput = { branch, hasUncommittedChanges, @@ -127,20 +145,23 @@ export function useMobileHostedReviewEligibility( } void fetchMobileHostedReviewEligibility(client, worktreeId, requestInput) .then((eligibility: HostedReviewCreationEligibility | null) => { - if (!isCurrent()) { + if (!active) { return } if (!eligibility) { - setState({ kind: 'error' }) + setSnapshot({ key, state: eligibilityStateAfterMobileHostedReviewError() }) return } - setState({ kind: 'ready', eligibility }) + setSnapshot({ key, state: { kind: 'ready', eligibility } }) }) .catch(() => { - if (isCurrent()) { - setState(eligibilityStateAfterMobileHostedReviewError()) + if (active) { + setSnapshot({ key, state: eligibilityStateAfterMobileHostedReviewError() }) } }) + return () => { + active = false + } }, [ ahead, behind, @@ -149,6 +170,7 @@ export function useMobileHostedReviewEligibility( connState, hasUncommittedChanges, hasUpstream, + hostId, key.fetch, key.identity, shouldFetch, @@ -159,5 +181,9 @@ export function useMobileHostedReviewEligibility( // snapshot in the same render, before the effect posts `idle` — otherwise the // Create PR button could stay enabled for one paint after the worktree is // no longer fetchable. - return shouldFetch ? state : { kind: 'idle' } + return renderedMobileHostedReviewEligibilityState({ + snapshot, + key, + shouldFetch + }) } diff --git a/mobile/src/source-control/use-mobile-source-control-create-pr-action.ts b/mobile/src/source-control/use-mobile-source-control-create-pr-action.ts index 44655d14706..49ec9bda83a 100644 --- a/mobile/src/source-control/use-mobile-source-control-create-pr-action.ts +++ b/mobile/src/source-control/use-mobile-source-control-create-pr-action.ts @@ -6,6 +6,7 @@ import type { MobileGitStatusResult } from './mobile-git-status' type Params = { client: Parameters[0]['client'] connState: Parameters[0]['connState'] + hostId: string worktreeId: string status: MobileGitStatusResult | null hasUncommittedChanges: boolean @@ -16,6 +17,7 @@ type Params = { export function useMobileSourceControlCreatePrAction({ client, connState, + hostId, worktreeId, status, hasUncommittedChanges, @@ -26,6 +28,7 @@ export function useMobileSourceControlCreatePrAction({ const eligibilityState = useMobileHostedReviewEligibility({ client, connState, + hostId, worktreeId, branch: status?.branch, hasUpstream: upstream?.hasUpstream, diff --git a/mobile/src/source-control/use-mobile-source-control-state.ts b/mobile/src/source-control/use-mobile-source-control-state.ts index e50525f3c62..1f541739d9a 100644 --- a/mobile/src/source-control/use-mobile-source-control-state.ts +++ b/mobile/src/source-control/use-mobile-source-control-state.ts @@ -209,6 +209,7 @@ export function useMobileSourceControlState(params: MobileSourceControlStatePara const createPrAction = useMobileSourceControlCreatePrAction({ client, connState, + hostId, worktreeId, status, hasUncommittedChanges: entries.length > 0, From fcb23e19cde9f67e415f6394ab99cdf05380cb84 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Fri, 31 Jul 2026 15:38:08 -0700 Subject: [PATCH 2/3] fix(sidebar): stabilize cross-host project grouping (#11805) * fix(sidebar): stabilize cross-host project grouping * fix(runtime): suppress outage toast on disconnect * fix(runtime): dismiss intentional outage toast * fix(runtime): keep offline retry toast visible --- .../settings/RuntimeEnvironmentsPane.tsx | 12 +- .../sidebar/worktree-list-groups.test.ts | 137 ++++++++++++++- .../sidebar/worktree-list-groups.ts | 62 ++++--- .../status-bar/SshStatusSegment.tsx | 6 +- src/renderer/src/i18n/locales/en.json | 5 + .../src/store/slices/runtime-status.test.ts | 164 +++++++++++++++++- .../src/store/slices/runtime-status.ts | 117 ++++++++++++- 7 files changed, 464 insertions(+), 39 deletions(-) diff --git a/src/renderer/src/components/settings/RuntimeEnvironmentsPane.tsx b/src/renderer/src/components/settings/RuntimeEnvironmentsPane.tsx index 19d8621cbc1..bd7d31f9652 100644 --- a/src/renderer/src/components/settings/RuntimeEnvironmentsPane.tsx +++ b/src/renderer/src/components/settings/RuntimeEnvironmentsPane.tsx @@ -592,10 +592,14 @@ export function RuntimeEnvironmentsPane({ await window.api.runtimeEnvironments.disconnect({ selector: environment.id }) // Why: disconnect is non-destructive; keep the saved server but show the // user that this live client is no longer attached to it. - useAppStore.getState().setRuntimeEnvironmentStatus(environment.id, { - status: null, - checkedAt: Date.now() - }) + useAppStore.getState().setRuntimeEnvironmentStatus( + environment.id, + { + status: null, + checkedAt: Date.now() + }, + { suppressDisconnectToast: true } + ) if (mountedRef.current) { setDetailsByEnvironmentId((current) => ({ ...current, diff --git a/src/renderer/src/components/sidebar/worktree-list-groups.test.ts b/src/renderer/src/components/sidebar/worktree-list-groups.test.ts index 7d6dae4770c..c5a7f9606b2 100644 --- a/src/renderer/src/components/sidebar/worktree-list-groups.test.ts +++ b/src/renderer/src/components/sidebar/worktree-list-groups.test.ts @@ -765,7 +765,7 @@ describe('buildRows with pinned worktrees', () => { ) }) - it('splits all project setups when one host has duplicate checkouts', () => { + it('splits only the surface with duplicate checkouts', () => { const localRepoB: Repo = { ...repo, id: 'repo-local-b', @@ -822,10 +822,143 @@ describe('buildRows with pinned worktrees', () => { expect(headers.map((row) => row.key)).toEqual([ 'project:github:stablyai/orca::setup:repo-1', 'project:github:stablyai/orca::setup:repo-local-b', - 'project:github:stablyai/orca::setup:repo-remote' + 'project:github:stablyai/orca' ]) }) + it('counts projection twins for one directory as one checkout', () => { + const runtimeHostId = 'runtime:m2-air' + const runtimeRepo: Repo = { + ...remoteRepo, + id: 'repo-runtime', + path: '/home/alice/orca-runtime', + connectionId: undefined, + executionHostId: runtimeHostId + } + const runtimeWorktree: Worktree = { + ...remoteWorktree, + id: 'wt-runtime', + repoId: runtimeRepo.id, + path: '/home/alice/orca-runtime-feature' + } + const runtimeSetup: ProjectHostSetup = { + ...projectHostSetups[1]!, + id: runtimeRepo.id, + repoId: runtimeRepo.id, + path: runtimeRepo.path, + hostId: runtimeHostId, + executionHostId: runtimeHostId + } + const derivedSetup: ProjectHostSetup = { + ...projectHostSetups[1]!, + hostId: runtimeHostId, + connectionId: 'intel mac', + executionHostId: runtimeHostId + } + const authoritativeSetup: ProjectHostSetup = { + ...derivedSetup, + id: 'setup-authoritative', + path: `${derivedSetup.path}/`, + connectionId: null, + executionHostId: 'ssh:intel%20mac' + } + const grouping = { + projects: [{ ...project, sourceRepoIds: [remoteRepo.id, runtimeRepo.id] }], + projectHostSetups: [runtimeSetup, derivedSetup, authoritativeSetup] + } + + expect([ + getGroupKeyForWorktree( + 'repo', + remoteWorktree, + new Map([[remoteRepo.id, remoteRepo]]), + null, + undefined, + undefined, + grouping + ), + getGroupKeyForWorktree( + 'repo', + runtimeWorktree, + new Map([[runtimeRepo.id, runtimeRepo]]), + null, + undefined, + undefined, + grouping + ) + ]).toEqual(['project:github:stablyai/orca', 'project:github:stablyai/orca']) + }) + + it('keeps Git hosts grouped when folder setups share the project identity', () => { + const windowsHostId = 'runtime:windows-server' + const windowsRepo: Repo = { + ...repo, + id: 'repo-windows', + path: 'C:\\Users\\neil\\orca\\orca', + executionHostId: windowsHostId + } + const folderRepoA: Repo = { + ...repo, + id: 'folder-qa-a', + path: '/tmp/pr11751-folder-qa', + displayName: 'pr11751-folder-qa', + kind: 'folder' + } + const folderRepoB: Repo = { + ...folderRepoA, + id: 'folder-qa-b', + path: '/tmp/pr11767-runtime-folder', + displayName: 'pr11767-runtime-folder' + } + const setups: ProjectHostSetup[] = [ + projectHostSetups[0]!, + { ...projectHostSetups[0]!, id: 'projection-twin' }, + projectHostSetups[1]!, + { + ...projectHostSetups[0]!, + id: windowsRepo.id, + repoId: windowsRepo.id, + path: windowsRepo.path, + hostId: windowsHostId, + executionHostId: windowsHostId + }, + { + ...projectHostSetups[0]!, + id: folderRepoA.id, + repoId: folderRepoA.id, + path: folderRepoA.path, + displayName: folderRepoA.displayName, + kind: 'folder' + }, + { + ...projectHostSetups[0]!, + id: folderRepoB.id, + repoId: folderRepoB.id, + path: folderRepoB.path, + displayName: folderRepoB.displayName, + kind: 'folder' + } + ] + const repos = [repo, remoteRepo, windowsRepo, folderRepoA, folderRepoB] + const grouping = { + projects: [{ ...project, sourceRepoIds: repos.map((entry) => entry.id) }], + projectHostSetups: setups + } + + const groupKeys = repos.map((entry) => + getGroupKeyForWorktree( + 'repo', + { ...worktree, id: `wt-${entry.id}`, repoId: entry.id, path: entry.path }, + new Map([[entry.id, entry]]), + null, + undefined, + undefined, + grouping + ) + ) + expect(new Set(groupKeys)).toEqual(new Set(['project:github:stablyai/orca'])) + }) + it('keeps a provisioned runtime copy under the project header alongside a same-host checkout', () => { const runtimeRepoB: Repo = { ...repo, diff --git a/src/renderer/src/components/sidebar/worktree-list-groups.ts b/src/renderer/src/components/sidebar/worktree-list-groups.ts index cfc0955a398..777e9fcfbe6 100644 --- a/src/renderer/src/components/sidebar/worktree-list-groups.ts +++ b/src/renderer/src/components/sidebar/worktree-list-groups.ts @@ -39,10 +39,14 @@ import { LOCAL_EXECUTION_HOST_ID, getRepoExecutionHostId, getWorktreeExecutionHostId, + toSshExecutionHostId, type ExecutionHostId } from '../../../../shared/execution-host' import { parseWslUncPath } from '../../../../shared/wsl-paths' -import { isWindowsAbsolutePathLike } from '../../../../shared/cross-platform-path' +import { + isWindowsAbsolutePathLike, + normalizeRuntimePathForComparison +} from '../../../../shared/cross-platform-path' import { getCyclicProjectedWorktreeLineageIds, getLineageRenderInfo @@ -177,28 +181,43 @@ type WorktreeGroupEntry = { type ProjectGroupingIndex = { projectById: Map setupByRepoId: Map - projectIdsRequiringSetupGroups: Set + surfaceKeysRequiringSetupGroups: Set } const projectGroupingIndexCache = new WeakMap() -// Why: `provisioned` setups are ephemeral recipe-created runtime copies that -// nest under the project header; every other method is a real user checkout. See #5374. +// Why: provisioned and folder setups are not independent Git checkouts. function isDistinctUserCheckout(setup: ProjectHostSetup): boolean { - return setup.setupMethod !== 'provisioned' + return setup.setupMethod !== 'provisioned' && setup.kind !== 'folder' } +// Why: execution target and filesystem namespace independently identify a surface. function getProjectSetupSurfaceKey(setup: ProjectHostSetup): string { + return `${setup.projectId}::${setup.hostId}::${getExecutionSurface(setup)}::${getPathSurface(setup)}` +} + +function getExecutionSurface(setup: ProjectHostSetup): string { + const connectionId = setup.connectionId?.trim() + if (connectionId) { + return toSshExecutionHostId(connectionId) + } + return setup.executionHostId?.trim() || setup.hostId +} + +// Why: projection twins differ by row identity, not checkout directory. +function getCheckoutIdentity(setup: ProjectHostSetup): string { + return normalizeRuntimePathForComparison(setup.path.trim()) || setup.repoId || setup.id +} + +function getPathSurface(setup: ProjectHostSetup): string { const wslPath = parseWslUncPath(setup.path) if (wslPath) { - // Why: Windows host and WSL on one machine are separate execution surfaces; - // only duplicate checkouts within one surface make project grouping ambiguous. - return `${setup.projectId}::${setup.hostId}::wsl:${wslPath.distro.toLowerCase()}` + return `wsl:${wslPath.distro.toLowerCase()}` } if (isWindowsAbsolutePathLike(setup.path)) { - return `${setup.projectId}::${setup.hostId}::windows-host` + return 'windows-host' } - return `${setup.projectId}::${setup.hostId}::default` + return 'default' } function buildProjectGroupingIndex(model?: ProjectGroupingModel): ProjectGroupingIndex | null { @@ -215,9 +234,7 @@ function buildProjectGroupingIndex(model?: ProjectGroupingModel): ProjectGroupin projectGroupingIndexCache.set(model, null) return null } - // Count real user checkouts per host surface (provisioned copies excluded so - // they keep nesting); more than one on a surface makes that project ambiguous. - const checkoutsByProjectSurface = new Map() + const checkoutsByProjectSurface = new Map>() for (const setup of projectHostSetups) { if (!isDistinctUserCheckout(setup)) { continue @@ -225,21 +242,21 @@ function buildProjectGroupingIndex(model?: ProjectGroupingModel): ProjectGroupin const key = getProjectSetupSurfaceKey(setup) const existing = checkoutsByProjectSurface.get(key) if (existing) { - existing.count += 1 + existing.add(getCheckoutIdentity(setup)) } else { - checkoutsByProjectSurface.set(key, { projectId: setup.projectId, count: 1 }) + checkoutsByProjectSurface.set(key, new Set([getCheckoutIdentity(setup)])) } } - const projectIdsRequiringSetupGroups = new Set() - for (const { projectId, count } of checkoutsByProjectSurface.values()) { - if (count > 1) { - projectIdsRequiringSetupGroups.add(projectId) + const surfaceKeysRequiringSetupGroups = new Set() + for (const [surfaceKey, checkouts] of checkoutsByProjectSurface) { + if (checkouts.size > 1) { + surfaceKeysRequiringSetupGroups.add(surfaceKey) } } const index = { projectById: new Map(projects.map((project) => [project.id, project])), setupByRepoId: new Map(projectHostSetups.map((setup) => [setup.repoId, setup])), - projectIdsRequiringSetupGroups + surfaceKeysRequiringSetupGroups } projectGroupingIndexCache.set(model, index) return index @@ -268,11 +285,10 @@ function getProjectGroupingForRepo( } } if ( - projectIndex?.projectIdsRequiringSetupGroups.has(setup.projectId) && + projectIndex?.surfaceKeysRequiringSetupGroups.has(getProjectSetupSurfaceKey(setup)) && isDistinctUserCheckout(setup) ) { - // Why: independent user checkouts of one project on the same host surface - // can't be safely merged, so each keeps its own sidebar entry. See #5374. + // Why: only the ambiguous surface needs checkout-specific headers. return { key: `project:${project.id}::setup:${repoId}`, label: repo?.displayName ?? setup.displayName, diff --git a/src/renderer/src/components/status-bar/SshStatusSegment.tsx b/src/renderer/src/components/status-bar/SshStatusSegment.tsx index d67fb82cf80..42049cb5a25 100644 --- a/src/renderer/src/components/status-bar/SshStatusSegment.tsx +++ b/src/renderer/src/components/status-bar/SshStatusSegment.tsx @@ -254,7 +254,11 @@ export function SshStatusSegment({ async (environmentId: string): Promise => { try { await window.api.runtimeEnvironments.disconnect({ selector: environmentId }) - setRuntimeEnvironmentStatus(environmentId, { status: null, checkedAt: Date.now() }) + setRuntimeEnvironmentStatus( + environmentId, + { status: null, checkedAt: Date.now() }, + { suppressDisconnectToast: true } + ) recordFeatureInteraction('ssh') } catch (err) { toast.error( diff --git a/src/renderer/src/i18n/locales/en.json b/src/renderer/src/i18n/locales/en.json index ff895e0563e..6dc956368ec 100644 --- a/src/renderer/src/i18n/locales/en.json +++ b/src/renderer/src/i18n/locales/en.json @@ -322,6 +322,11 @@ "f518e89aa5": "Project already exists in that profile", "f03ae7f27b": "Failed to transfer project" } + }, + "runtime": { + "status": { + "runtimeHostDisconnectedDescription": "Workspaces and terminals on this server are unavailable." + } } } }, diff --git a/src/renderer/src/store/slices/runtime-status.test.ts b/src/renderer/src/store/slices/runtime-status.test.ts index a82350f06e0..caa40785b23 100644 --- a/src/renderer/src/store/slices/runtime-status.test.ts +++ b/src/renderer/src/store/slices/runtime-status.test.ts @@ -1,6 +1,8 @@ -import { afterEach, describe, expect, it, vi } from 'vitest' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { create } from 'zustand' +import { toast } from 'sonner' import type { RuntimeStatus } from '../../../../shared/runtime-types' +import type { PublicKnownRuntimeEnvironment } from '../../../../shared/runtime-environments' import { createCompatibleRuntimeStatusResponse } from '../../runtime/runtime-compatibility-test-fixture' import { callRuntimeRpc, @@ -12,6 +14,10 @@ import { getRuntimeEnvironmentConnectionGeneration } from './runtime-status' +vi.mock('sonner', () => ({ + toast: { warning: vi.fn(), dismiss: vi.fn() } +})) + function createSliceStore() { return create()((...a) => ({ ...createRuntimeStatusSlice(...(a as unknown as Parameters)) @@ -32,6 +38,22 @@ function makeStatus(overrides: Partial = {}): RuntimeStatus { } as RuntimeStatus } +function makeEnvironment( + overrides: Partial = {} +): PublicKnownRuntimeEnvironment { + return { + id: 'env-a', + name: 'Dev Box', + createdAt: 1, + updatedAt: 1, + lastUsedAt: null, + runtimeId: null, + endpoints: [{ id: 'ws-a', kind: 'websocket', label: 'WebSocket', endpoint: 'ws://x' }], + preferredEndpointId: 'ws-a', + ...overrides + } +} + function stubRuntimeEnvironmentApi({ getStatus = vi.fn(), list = vi.fn() @@ -50,7 +72,13 @@ function stubRuntimeEnvironmentApi({ return { getStatus, list } } +beforeEach(() => { + vi.mocked(toast.warning).mockReset() + vi.mocked(toast.dismiss).mockReset() +}) + afterEach(() => { + vi.useRealTimers() vi.unstubAllGlobals() }) @@ -142,6 +170,140 @@ describe('runtime-status slice', () => { expect(map.get('env-a')).toEqual({ status: null, checkedAt: 5, connectionGeneration: 1 }) }) + it('does not toast when the first probe finds a saved server offline', () => { + const store = createSliceStore() + store.setState({ runtimeEnvironments: [makeEnvironment()] }) + + store.getState().setRuntimeEnvironmentStatus('env-a', { status: null, checkedAt: 1 }) + + expect(toast.warning).not.toHaveBeenCalled() + }) + + it('toasts once when a connected server becomes unavailable', () => { + const store = createSliceStore() + store.setState({ runtimeEnvironments: [makeEnvironment()] }) + store.getState().setRuntimeEnvironmentStatus('env-a', { status: makeStatus(), checkedAt: 1 }) + + store.getState().setRuntimeEnvironmentStatus('env-a', { status: null, checkedAt: 2 }) + store.getState().setRuntimeEnvironmentStatus('env-a', { status: null, checkedAt: 3 }) + + expect(toast.warning).toHaveBeenCalledTimes(1) + expect(toast.warning).toHaveBeenCalledWith( + 'Dev Box disconnected', + expect.objectContaining({ + id: 'runtime-environment-disconnected:env-a', + description: 'Workspaces and terminals on this server are unavailable.', + action: expect.objectContaining({ label: 'Retry' }) + }) + ) + }) + + it('dismisses the disconnect toast when the server recovers', () => { + const store = createSliceStore() + store.getState().setRuntimeEnvironmentStatus('env-a', { status: makeStatus(), checkedAt: 1 }) + store.getState().setRuntimeEnvironmentStatus('env-a', { status: null, checkedAt: 2 }) + + store.getState().setRuntimeEnvironmentStatus('env-a', { status: makeStatus(), checkedAt: 3 }) + + expect(toast.dismiss).toHaveBeenCalledWith('runtime-environment-disconnected:env-a') + }) + + it('keeps the keyed action toast visible when an offline retry settles', async () => { + vi.useFakeTimers() + const toastId = 'runtime-environment-disconnected:env-a' + const visibleToastIds = new Set() + vi.mocked(toast.warning).mockImplementation((_title, options) => { + if (options?.id !== undefined) { + visibleToastIds.add(options.id) + } + return options?.id ?? '' + }) + const getStatus = vi.fn().mockRejectedValue(new Error('closed')) + stubRuntimeEnvironmentApi({ getStatus }) + const store = createSliceStore() + store.setState({ runtimeEnvironments: [makeEnvironment()] }) + store.getState().setRuntimeEnvironmentStatus('env-a', { status: makeStatus(), checkedAt: 1 }) + store.getState().setRuntimeEnvironmentStatus('env-a', { status: null, checkedAt: 2 }) + const options = vi.mocked(toast.warning).mock.calls[0]?.[1] as unknown as { + action: { onClick: (event: { preventDefault: () => void }) => void } + } + const clickAction = (): { defaultPrevented: boolean } => { + const event = { + defaultPrevented: false, + preventDefault() { + this.defaultPrevented = true + } + } + options.action.onClick(event) + if (!event.defaultPrevented) { + setTimeout(() => visibleToastIds.delete(toastId), 200) + } + return event + } + + const firstClick = clickAction() + const secondClick = clickAction() + await vi.advanceTimersByTimeAsync(0) + + expect(firstClick.defaultPrevented).toBe(true) + expect(secondClick.defaultPrevented).toBe(true) + expect(getStatus).toHaveBeenCalledWith({ selector: 'env-a', timeoutMs: 10_000 }) + await vi.advanceTimersByTimeAsync(200) + expect(visibleToastIds.has(toastId)).toBe(true) + expect(getStatus).toHaveBeenCalledTimes(1) + expect(toast.warning).toHaveBeenCalledTimes(3) + expect(vi.mocked(toast.warning).mock.calls.map((call) => call[1]?.duration)).toEqual([ + 4_000, + Number.POSITIVE_INFINITY, + 4_000 + ]) + expect(vi.mocked(toast.warning).mock.calls.every((call) => call[1]?.id === toastId)).toBe(true) + }) + + it('does not report removal or explicit status clearing as a disconnect', () => { + const store = createSliceStore() + store.setState({ runtimeEnvironments: [makeEnvironment()] }) + store.getState().setRuntimeEnvironmentStatus('env-a', { status: makeStatus(), checkedAt: 1 }) + + store.getState().clearRuntimeEnvironmentStatus('env-a') + store.getState().setRuntimeEnvironments([]) + + expect(toast.warning).not.toHaveBeenCalled() + }) + + it('does not report an intentional disconnect as an outage', () => { + const store = createSliceStore() + store.getState().setRuntimeEnvironmentStatus('env-a', { status: makeStatus(), checkedAt: 1 }) + + store + .getState() + .setRuntimeEnvironmentStatus( + 'env-a', + { status: null, checkedAt: 2 }, + { suppressDisconnectToast: true } + ) + + expect(toast.warning).not.toHaveBeenCalled() + expect(store.getState().runtimeStatusByEnvironmentId.get('env-a')?.status).toBeNull() + }) + + it('dismisses an outage toast opened while an intentional disconnect was pending', () => { + const store = createSliceStore() + store.getState().setRuntimeEnvironmentStatus('env-a', { status: makeStatus(), checkedAt: 1 }) + store.getState().setRuntimeEnvironmentStatus('env-a', { status: null, checkedAt: 2 }) + + store + .getState() + .setRuntimeEnvironmentStatus( + 'env-a', + { status: null, checkedAt: 3 }, + { suppressDisconnectToast: true } + ) + + expect(toast.warning).toHaveBeenCalledTimes(1) + expect(toast.dismiss).toHaveBeenCalledWith('runtime-environment-disconnected:env-a') + }) + it('clears a single environment entry', () => { const store = createSliceStore() store.getState().setRuntimeEnvironmentStatus('env-a', { status: makeStatus(), checkedAt: 1 }) diff --git a/src/renderer/src/store/slices/runtime-status.ts b/src/renderer/src/store/slices/runtime-status.ts index 03fdac1224f..1eda7652518 100644 --- a/src/renderer/src/store/slices/runtime-status.ts +++ b/src/renderer/src/store/slices/runtime-status.ts @@ -1,4 +1,5 @@ import type { StateCreator } from 'zustand' +import { toast } from 'sonner' import type { AppState } from '../types' import type { PublicKnownRuntimeEnvironment } from '../../../../shared/runtime-environments' import type { RuntimeStatus } from '../../../../shared/runtime-types' @@ -8,6 +9,7 @@ import { unwrapRuntimeRpcResult } from '@/runtime/runtime-rpc-client' import { replaceRuntimeEnvironmentRevisions } from '@/runtime/runtime-environment-revision' +import { translate } from '@/i18n/i18n' /** Live status for one saved runtime environment, as last observed by the * renderer. `status === null` records a probe that failed or timed out so the @@ -44,7 +46,11 @@ export type RuntimeStatusSlice = { * retires state owned by any environment that just left the saved list. */ setRuntimeEnvironments: (environments: PublicKnownRuntimeEnvironment[]) => void /** Merges one environment's status. Replaces the prior entry for that id. */ - setRuntimeEnvironmentStatus: (environmentId: string, status: RuntimeEnvironmentStatus) => void + setRuntimeEnvironmentStatus: ( + environmentId: string, + status: RuntimeEnvironmentStatus, + options?: { suppressDisconnectToast?: boolean } + ) => void /** Drops a removed environment so stale hosts don't linger in the registry. */ clearRuntimeEnvironmentStatus: (environmentId: string) => void /** Drops every entry whose id is not in the saved-environments set. */ @@ -57,6 +63,85 @@ export type RuntimeStatusSlice = { } const connectionGenerationByEnvironment = new Map() +const activeRuntimeDisconnectedToasts = new Map() +const RUNTIME_DISCONNECTED_TOAST_DURATION_MS = 4_000 + +function getRuntimeDisconnectedToastId(environmentId: string): string { + return `runtime-environment-disconnected:${environmentId}` +} + +function showRuntimeDisconnectedToast(environmentId: string, getState: () => AppState): void { + const environment = getState().runtimeEnvironments.find((entry) => entry.id === environmentId) + const toastId = getRuntimeDisconnectedToastId(environmentId) + const activation = Symbol(toastId) + const title = environment?.name + ? translate( + 'auto.components.sidebar.WorktreeCard.runtimeHostDisconnectedNamed', + '{{hostName}} disconnected', + { hostName: environment.name } + ) + : translate( + 'auto.components.sidebar.WorktreeCard.runtimeHostDisconnected', + 'Server disconnected' + ) + activeRuntimeDisconnectedToasts.set(toastId, activation) + const clearActiveToast = (): void => { + if (activeRuntimeDisconnectedToasts.get(toastId) === activation) { + activeRuntimeDisconnectedToasts.delete(toastId) + } + } + let retrying = false + const showToast = (duration = RUNTIME_DISCONNECTED_TOAST_DURATION_MS): void => { + toast.warning(title, { + id: toastId, + description: translate( + 'auto.store.slices.runtime.status.runtimeHostDisconnectedDescription', + 'Workspaces and terminals on this server are unavailable.' + ), + duration, + action: { + label: translate('browser.loadFailure.retry', 'Retry'), + onClick: (event) => { + // Why: Sonner otherwise deletes the keyed toast after the action callback. + event.preventDefault() + if (retrying) { + return + } + retrying = true + showToast(Number.POSITIVE_INFINITY) + void getState() + .refreshRuntimeEnvironmentStatus(environmentId) + .then((reachable) => { + const stillSaved = getState().runtimeEnvironments.some( + (entry) => entry.id === environmentId + ) + if ( + !reachable && + stillSaved && + activeRuntimeDisconnectedToasts.get(toastId) === activation + ) { + showToast() + } + }) + .finally(() => { + retrying = false + }) + } + }, + onDismiss: clearActiveToast, + onAutoClose: clearActiveToast + }) + } + showToast() +} + +function dismissRuntimeDisconnectedToast(environmentId: string): void { + const toastId = getRuntimeDisconnectedToastId(environmentId) + if (!activeRuntimeDisconnectedToasts.delete(toastId)) { + return + } + toast.dismiss?.(toastId) +} export function getRuntimeEnvironmentConnectionGeneration(environmentId: string): number { return connectionGenerationByEnvironment.get(environmentId) ?? 0 @@ -158,10 +243,12 @@ export const createRuntimeStatusSlice: StateCreator 0) { get().purgeStaleRuntimeHostState?.(retiredEnvironmentIds) + retiredEnvironmentIds.forEach(dismissRuntimeDisconnectedToast) } }, - setRuntimeEnvironmentStatus: (environmentId, status) => { + setRuntimeEnvironmentStatus: (environmentId, status, options) => { + const previous = get().runtimeStatusByEnvironmentId.get(environmentId) // Why: a non-null status proves the runtime just answered, so drop any stale // "offline" compat failure before this online transition fires the // reuse-flagged background refetches — a recovered host must re-probe. @@ -170,7 +257,6 @@ export const createRuntimeStatusSlice: StateCreator { const next = new Map(s.runtimeStatusByEnvironmentId) - const previous = next.get(environmentId) const connectionChanged = status.status !== null && (previous?.status == null || previous.status.runtimeId !== status.status.runtimeId) @@ -185,9 +271,17 @@ export const createRuntimeStatusSlice: StateCreator + clearRuntimeEnvironmentStatus: (environmentId) => { + dismissRuntimeDisconnectedToast(environmentId) set((s) => { advanceRuntimeEnvironmentConnectionGeneration(environmentId) if (!s.runtimeStatusByEnvironmentId.has(environmentId)) { @@ -196,11 +290,17 @@ export const createRuntimeStatusSlice: StateCreator + retainRuntimeEnvironmentStatuses: (environmentIds) => { + const keep = new Set(environmentIds) + for (const id of get().runtimeStatusByEnvironmentId.keys()) { + if (!keep.has(id)) { + dismissRuntimeDisconnectedToast(id) + } + } set((s) => { - const keep = new Set(environmentIds) let changed = false const next = new Map(s.runtimeStatusByEnvironmentId) for (const id of next.keys()) { @@ -210,7 +310,8 @@ export const createRuntimeStatusSlice: StateCreator { try { From 475f63ea1b4f5950561b3efc047d5b5018d12d92 Mon Sep 17 00:00:00 2001 From: Yunqian Fan Date: Sat, 1 Aug 2026 07:01:44 +0800 Subject: [PATCH 3/3] fix(remote): scope renderer throttling to paired terminal publication (#11581) * fix(remote): unthrottle host renderer while serving a paired client A paired desktop host left in the background could not open or close agent sessions for its remote/relay client: the action stalled and eventually failed with the host-side "Timed out waiting for terminal surface after creation" (10s) error, while an already-live terminal's keystrokes stayed fast. Root cause: creating/closing a session routes through the host renderer's setTimeout-coalesced graph sync to publish the terminal surface, but the host window runs with Electron background throttling (the hidden-window default, reaffirmed on macOS). When the window is backgrounded/occluded, those renderer timers are throttled to a crawl and the surface publication misses the 10s deadline. Live keystrokes are unaffected because PTY I/O flows through the main process, never the renderer. Keep the authoritative renderer unthrottled while at least one remote client is connected and restore the throttled power-saving default once the last one disconnects. Connect/disconnect are driven from the shared MobileSocketWiring onReady/onClose, so both direct-WS and cloud-relay clients are covered; headless serve has no window and is a safe no-op. Co-Authored-By: Claude Opus 5 (1M context) * refactor(remote): tidy renderer-throttle comment and test per review Address automated review nits on #11581: - Trim the module-level rationale comment to the non-obvious contract, matching the repo's concise-comment guideline. - Drop the dead `detachedThrottle` variable from the reapply test; the detached-target scenario is already covered by the lazy-resolution test, so the case now asserts only what it exercises. No behavior change. Co-Authored-By: Claude Opus 5 (1M context) * fix(remote): scope paired terminal publication throttling Keep headed paired terminal creation and close renderer-owned so host inventory, input routing, ACK recovery, and cleanup retain the established lifecycle. Hold a reference-counted background-throttle lease only while the renderer publishes a paired operation, and epoch-fence async resolution so renderer reloads reject before any request or PTY spawn. Preserve headless main ownership and prevent paired clients from falling back to a local terminal. * test(e2e): verify minimized host terminal repaint * fix(remote): preserve paired terminal inventory through graph gaps --------- Co-authored-by: fanyunqian.1 Co-authored-by: Claude Opus 5 (1M context) Co-authored-by: OrcaWin <293788423+OrcaWin@users.noreply.github.com> --- src/main/runtime/orca-runtime.test.ts | 437 +++++++++++++++++- src/main/runtime/orca-runtime.ts | 410 ++++++++++------ .../rpc/methods/session-tab-close-methods.ts | 8 +- .../runtime/rpc/methods/session-tabs.test.ts | 6 +- src/main/runtime/runtime-rpc.test.ts | 41 ++ .../renderer-publication-throttle.test.ts | 58 +++ .../window/renderer-publication-throttle.ts | 32 ++ .../store/slices/cmd-j-create-actions.test.ts | 61 +++ src/renderer/src/store/slices/terminals.ts | 4 + ...red-remote-terminal-stall-recovery.spec.ts | 397 +++++++++++++++- 10 files changed, 1305 insertions(+), 149 deletions(-) create mode 100644 src/main/window/renderer-publication-throttle.test.ts create mode 100644 src/main/window/renderer-publication-throttle.ts diff --git a/src/main/runtime/orca-runtime.test.ts b/src/main/runtime/orca-runtime.test.ts index 153f7840760..e9c32dd16d9 100644 --- a/src/main/runtime/orca-runtime.test.ts +++ b/src/main/runtime/orca-runtime.test.ts @@ -1409,14 +1409,16 @@ function makeRuntimeStoreWithWorkspaceSession(initialSession: WorkspaceSessionSt persistPtyBinding: ReturnType } getSession: () => WorkspaceSessionState + setSession: (next: WorkspaceSessionState) => void } { let session = initialSession + const setSession = (next: WorkspaceSessionState): void => { + session = next + } const runtimeStore = { ...store, getWorkspaceSession: () => session, - setWorkspaceSession: vi.fn((next: WorkspaceSessionState) => { - session = next - }), + setWorkspaceSession: vi.fn(setSession), persistPtyBinding: vi.fn( (args: { worktreeId: string; tabId: string; leafId: string; ptyId: string }) => { const tabs = session.tabsByWorktree[args.worktreeId] ?? [] @@ -1446,7 +1448,7 @@ function makeRuntimeStoreWithWorkspaceSession(initialSession: WorkspaceSessionSt } ) } - return { runtimeStore, getSession: () => session } + return { runtimeStore, getSession: () => session, setSession } } function makeWorkspaceSessionWithHeadlessTerminal( @@ -24830,6 +24832,267 @@ describe('OrcaRuntimeService', () => { ]) }) + it('leases renderer publication for a paired create and preserves host-owned inventory', async () => { + const leafId = '91919191-9191-4919-8919-919191919191' + const spawn = vi.fn() + const setBackgroundThrottling = vi.fn() + const runtime = new OrcaRuntimeService(store) + runtime.setPtyController({ + spawn, + write: () => true, + kill: () => true, + getForegroundProcess: async () => null + }) + runtime.attachWindow(1) + runtime.syncWindowGraph(1, { tabs: [], leaves: [] }) + const webContents: { + isDestroyed: () => boolean + setBackgroundThrottling: typeof setBackgroundThrottling + send: ReturnType + } = { + isDestroyed: () => false, + setBackgroundThrottling, + send: vi.fn() + } + webContents.send.mockImplementation((_channel: string, payload: { requestId: string }) => { + expect(setBackgroundThrottling).toHaveBeenCalledWith(false) + runtime.registerPty('pty-paired-headed', TEST_WORKTREE_ID, null, { + tabId: 'tab-paired-headed', + leafId + }) + ipcMain.emit( + 'terminal:tabCreateReply', + { sender: webContents }, + { requestId: payload.requestId, tabId: 'tab-paired-headed', title: 'Terminal' } + ) + runtime.syncWindowGraph(1, { + tabs: [ + { + tabId: 'tab-paired-headed', + worktreeId: TEST_WORKTREE_ID, + title: 'Terminal', + activeLeafId: leafId, + layout: null + } + ], + leaves: [ + { + tabId: 'tab-paired-headed', + worktreeId: TEST_WORKTREE_ID, + leafId, + paneRuntimeId: 1, + ptyId: 'pty-paired-headed' + } + ], + mobileSessionTabs: [ + { + worktree: TEST_WORKTREE_ID, + publicationEpoch: 'renderer:paired-headed', + snapshotVersion: 1, + activeGroupId: 'group-1', + activeTabId: null, + activeTabType: null, + tabs: [ + { + type: 'terminal', + id: `tab-paired-headed::${leafId}`, + parentTabId: 'tab-paired-headed', + leafId, + ptyId: 'pty-paired-headed', + title: 'Terminal', + viewMode: 'chat', + isActive: false + } + ] + } + ] + }) + }) + electronMocks.BrowserWindow.fromId.mockReturnValue({ + isDestroyed: () => false, + webContents + }) + + const result = await runtime.createMobileSessionTerminal(`id:${TEST_WORKTREE_ID}`, { + clientNavigationId: 'device-a', + navigation: 'caller', + activate: false, + select: false, + viewMode: 'chat' + }) + + expect(webContents.send).toHaveBeenCalledWith( + 'terminal:requestTabCreate', + expect.objectContaining({ source: 'runtime-session', viewMode: 'chat' }) + ) + expect(spawn).not.toHaveBeenCalled() + expect(setBackgroundThrottling.mock.calls).toEqual([[false], [true]]) + expect(result.tab).toMatchObject({ + type: 'terminal', + status: 'ready', + ptyId: 'pty-paired-headed', + terminal: expect.stringMatching(/^term_/), + viewMode: 'chat' + }) + const host = await runtime.listMobileSessionTabs(`id:${TEST_WORKTREE_ID}`) + const clientA = await runtime.listMobileSessionTabs(`id:${TEST_WORKTREE_ID}`, 'device-a') + const clientB = await runtime.listMobileSessionTabs(`id:${TEST_WORKTREE_ID}`, 'device-b') + const inventory = (snapshot: RuntimeMobileSessionTabsResult) => + snapshot.tabs.map((tab) => + tab.type === 'terminal' + ? { + id: tab.id, + type: tab.type, + leafId: tab.leafId, + parentTabId: tab.parentTabId, + ptyId: tab.ptyId + } + : { id: tab.id, type: tab.type } + ) + expect(inventory(clientA)).toEqual(inventory(host)) + expect(inventory(clientB)).toEqual(inventory(host)) + + runtime['mobileSessionTabsByWorktree'].delete(TEST_WORKTREE_ID) + expect(runtime['syncMobileSessionTabs']([])).toEqual(new Set([TEST_WORKTREE_ID])) + expect(runtime['mobileSessionTabsByWorktree'].get(TEST_WORKTREE_ID)?.tabs).toEqual([ + expect.objectContaining({ + parentTabId: 'tab-paired-headed', + leafId, + ptyId: 'pty-paired-headed' + }) + ]) + const restoredHost = await runtime.listMobileSessionTabs(`id:${TEST_WORKTREE_ID}`) + const restoredClientA = await runtime.listMobileSessionTabs( + `id:${TEST_WORKTREE_ID}`, + 'device-a' + ) + const restoredClientB = await runtime.listMobileSessionTabs( + `id:${TEST_WORKTREE_ID}`, + 'device-b' + ) + expect(inventory(restoredHost)).toEqual(inventory(host)) + expect(inventory(restoredClientA)).toEqual(inventory(host)) + expect(inventory(restoredClientB)).toEqual(inventory(host)) + expect(spawn).not.toHaveBeenCalled() + }) + + it.each([ + ['paired', 'reloading'], + ['paired', 'unavailable'], + ['unpaired', 'reloading'], + ['unpaired', 'unavailable'] + ] as const)( + 'does not spawn a %s terminal while the headed renderer graph is %s', + async (caller, graphStatus) => { + const spawn = vi.fn() + const send = vi.fn() + const runtime = new OrcaRuntimeService(store) + runtime.setPtyController({ + spawn, + write: () => true, + kill: () => true, + getForegroundProcess: async () => null + }) + runtime.attachWindow(1) + runtime.syncWindowGraph(1, { tabs: [], leaves: [] }) + electronMocks.BrowserWindow.fromId.mockReturnValue({ + isDestroyed: () => false, + webContents: { isDestroyed: () => false, send, setBackgroundThrottling: vi.fn() } + }) + if (graphStatus === 'reloading') { + runtime.markRendererReloading(1) + } else { + runtime.markGraphUnavailable(1) + } + + await expect( + runtime.createMobileSessionTerminal( + `id:${TEST_WORKTREE_ID}`, + caller === 'paired' + ? { clientNavigationId: 'device-a', navigation: 'caller' as const } + : {} + ) + ).rejects.toThrow('runtime_unavailable') + expect(spawn).not.toHaveBeenCalled() + expect(send).not.toHaveBeenCalled() + } + ) + + it('rejects a paired create if renderer authority changes across async resolution', async () => { + const spawn = vi.fn() + const send = vi.fn() + const setBackgroundThrottling = vi.fn() + const runtime = new OrcaRuntimeService(store) + runtime.setPtyController({ + spawn, + write: () => true, + kill: () => true, + getForegroundProcess: async () => null + }) + runtime.attachWindow(1) + electronMocks.BrowserWindow.fromId.mockReturnValue({ + isDestroyed: () => false, + webContents: { isDestroyed: () => false, send, setBackgroundThrottling } + }) + runtime.syncWindowGraph(1, { tabs: [], leaves: [] }) + let resolutionStarted = (): void => {} + const started = new Promise((resolve) => { + resolutionStarted = resolve + }) + let releaseResolution = (): void => {} + vi.mocked(listWorktrees).mockImplementationOnce( + () => + new Promise((resolve) => { + releaseResolution = () => resolve(MOCK_GIT_WORKTREES) + resolutionStarted() + }) + ) + + const create = runtime.createMobileSessionTerminal(`id:${TEST_WORKTREE_ID}`, { + clientNavigationId: 'device-a', + navigation: 'caller' + }) + await started + expect(spawn).not.toHaveBeenCalled() + runtime.markRendererReloading(1) + releaseResolution() + await expect(create).rejects.toThrow('runtime_unavailable') + expect(send).not.toHaveBeenCalled() + expect(spawn).not.toHaveBeenCalled() + expect(setBackgroundThrottling).not.toHaveBeenCalled() + }) + + it('cancels a paired renderer-owned create before publication when its client disconnects', async () => { + const spawn = vi.fn() + const send = vi.fn() + const setBackgroundThrottling = vi.fn() + const abort = new AbortController() + const runtime = new OrcaRuntimeService(store) + runtime.setPtyController({ + spawn, + write: () => true, + kill: () => true, + getForegroundProcess: async () => null + }) + runtime.attachWindow(1) + runtime.syncWindowGraph(1, { tabs: [], leaves: [] }) + electronMocks.BrowserWindow.fromId.mockReturnValue({ + isDestroyed: () => false, + webContents: { isDestroyed: () => false, send, setBackgroundThrottling } + }) + abort.abort() + + await expect( + runtime.createMobileSessionTerminal(`id:${TEST_WORKTREE_ID}`, { + clientNavigationId: 'device-a', + signal: abort.signal + }) + ).rejects.toThrow('client_disconnected') + expect(spawn).not.toHaveBeenCalled() + expect(send).not.toHaveBeenCalled() + expect(setBackgroundThrottling).not.toHaveBeenCalled() + }) + it('selects a created terminal only for the paired caller', async () => { let spawnIndex = 0 const runtime = new OrcaRuntimeService(store) @@ -25790,6 +26053,172 @@ describe('OrcaRuntimeService', () => { }) }) + it('leases renderer publication until a paired whole-tab close is acknowledged', async () => { + const { runtimeStore } = makeRuntimeStoreWithWorkspaceSession( + makeWorkspaceSessionWithHeadlessTerminal() + ) + const acknowledged = makeDeferred() + const closeTerminalTab = vi.fn(() => acknowledged.promise) + const setBackgroundThrottling = vi.fn() + const runtime = new OrcaRuntimeService(runtimeStore as never) + runtime.setNotifier({ closeTerminal: vi.fn(), closeTerminalTab } as never) + runtime.setPtyController({ + write: () => true, + kill: () => true, + getForegroundProcess: async () => null, + listProcesses: async () => [] + }) + runtime.syncWindowGraph(1, { + tabs: [ + { + tabId: 'host-tab', + worktreeId: TEST_WORKTREE_ID, + title: 'Durable', + activeLeafId: HEADLESS_LEAF_ID, + layout: null + } + ], + leaves: [ + { + tabId: 'host-tab', + worktreeId: TEST_WORKTREE_ID, + leafId: HEADLESS_LEAF_ID, + paneRuntimeId: 1, + ptyId: 'persisted-pty' + } + ], + mobileSessionTabs: [ + { + worktree: TEST_WORKTREE_ID, + publicationEpoch: 'renderer:paired-close', + snapshotVersion: 1, + activeGroupId: 'group-1', + activeTabId: `host-tab::${HEADLESS_LEAF_ID}`, + activeTabType: 'terminal', + tabs: [ + { + type: 'terminal', + id: `host-tab::${HEADLESS_LEAF_ID}`, + parentTabId: 'host-tab', + leafId: HEADLESS_LEAF_ID, + ptyId: 'persisted-pty', + title: 'Durable', + isActive: true + } + ] + } + ] + }) + runtime.registerPty('persisted-pty', TEST_WORKTREE_ID, null, { + tabId: 'host-tab', + leafId: HEADLESS_LEAF_ID + }) + electronMocks.BrowserWindow.fromId.mockReturnValue({ + isDestroyed: () => false, + webContents: { + isDestroyed: () => false, + setBackgroundThrottling + } + }) + + const pending = runtime.closeMobileSessionTab(`id:${TEST_WORKTREE_ID}`, 'host-tab', { + reason: 'user', + clientNavigationId: 'device-a' + }) + await vi.waitFor(() => expect(closeTerminalTab).toHaveBeenCalledWith('host-tab')) + expect(setBackgroundThrottling.mock.calls).toEqual([[false]]) + + acknowledged.resolve() + await expect(pending).resolves.toEqual({ closed: true }) + expect(setBackgroundThrottling.mock.calls).toEqual([[false], [true]]) + }) + + it('accepts a paired close after the renderer already persisted its removal', async () => { + const { runtimeStore, getSession, setSession } = makeRuntimeStoreWithWorkspaceSession( + makeWorkspaceSessionWithHeadlessTerminal() + ) + const acknowledged = makeDeferred() + const closeTerminalTab = vi.fn(() => acknowledged.promise) + const kill = vi.fn(() => true) + const runtime = new OrcaRuntimeService(runtimeStore as never) + runtime.setNotifier({ closeTerminal: vi.fn(), closeTerminalTab } as never) + runtime.setPtyController({ + write: () => true, + kill, + getForegroundProcess: async () => null, + listProcesses: async () => [] + }) + runtime.syncWindowGraph(1, { + tabs: [ + { + tabId: 'host-tab', + worktreeId: TEST_WORKTREE_ID, + title: 'Durable', + activeLeafId: HEADLESS_LEAF_ID, + layout: null + } + ], + leaves: [ + { + tabId: 'host-tab', + worktreeId: TEST_WORKTREE_ID, + leafId: HEADLESS_LEAF_ID, + paneRuntimeId: 1, + ptyId: 'persisted-pty' + } + ], + mobileSessionTabs: [ + { + worktree: TEST_WORKTREE_ID, + publicationEpoch: 'renderer:paired-close', + snapshotVersion: 1, + activeGroupId: 'group-1', + activeTabId: `host-tab::${HEADLESS_LEAF_ID}`, + activeTabType: 'terminal', + tabs: [ + { + type: 'terminal', + id: `host-tab::${HEADLESS_LEAF_ID}`, + parentTabId: 'host-tab', + leafId: HEADLESS_LEAF_ID, + ptyId: 'persisted-pty', + title: 'Durable', + isActive: true + } + ] + } + ] + }) + runtime.registerPty('persisted-pty', TEST_WORKTREE_ID, null, { + tabId: 'host-tab', + leafId: HEADLESS_LEAF_ID + }) + electronMocks.BrowserWindow.fromId.mockReturnValue({ + isDestroyed: () => false, + webContents: { + isDestroyed: () => false, + setBackgroundThrottling: vi.fn() + } + }) + + const pending = runtime.closeMobileSessionTab(`id:${TEST_WORKTREE_ID}`, 'host-tab', { + reason: 'user', + clientNavigationId: 'device-a' + }) + await vi.waitFor(() => expect(closeTerminalTab).toHaveBeenCalledWith('host-tab')) + const session = getSession() + setSession({ + ...session, + tabsByWorktree: { ...session.tabsByWorktree, [TEST_WORKTREE_ID]: [] }, + terminalLayoutsByTabId: {} + }) + runtime.syncWindowGraph(1, { tabs: [], leaves: [] }) + + acknowledged.resolve() + await expect(pending).resolves.toEqual({ closed: true }) + expect(kill).toHaveBeenCalledWith('persisted-pty') + }) + it('reuses pane close for live PTYs that do not own a renderer tab', async () => { const kill = vi.fn(() => true) const closeTerminalTab = vi.fn(async () => {}) diff --git a/src/main/runtime/orca-runtime.ts b/src/main/runtime/orca-runtime.ts index abc82437657..7f4c42c9a90 100644 --- a/src/main/runtime/orca-runtime.ts +++ b/src/main/runtime/orca-runtime.ts @@ -541,6 +541,7 @@ import { import { joinWorktreeRelativePath } from './runtime-relative-paths' import { collectMemorySnapshot } from '../memory/collector' import { app, BrowserWindow, ipcMain, Notification } from 'electron' +import { RendererPublicationThrottle } from '../window/renderer-publication-throttle' import type { AgentBrowserBridge } from '../browser/agent-browser-bridge' import type { BrowserBackend } from '../browser/browser-backend' import { BrowserError } from '../browser/cdp-bridge' @@ -2640,6 +2641,8 @@ export class OrcaRuntimeService { private rendererGraphEpoch = 0 private graphStatus: RuntimeGraphStatus = 'unavailable' private authoritativeWindowId: number | null = null + // Why: paired graph transactions need foreground timer cadence only until their publication settles. + private readonly rendererPublicationThrottle = new RendererPublicationThrottle() private tabs = new Map() private mobileSessionTabsByWorktree = new Map() // Why: renderer publication ordering must be judged against the renderer's @@ -2689,6 +2692,7 @@ export class OrcaRuntimeService { string, { activate: boolean + paired: boolean selectIfNoActiveTab: boolean viewMode?: 'terminal' | 'chat' } @@ -2804,6 +2808,7 @@ export class OrcaRuntimeService { // iterates them all. Listeners are cleaned up via subscriptionCleanups. private notificationListeners = new Set<(event: MobileNotificationEvent) => void>() private ptysById = new Map() + private readonly pairedRendererSessionOwnedPtyIds = new Set() private wslDistroByPtyId = new Map() private titleObservationSequence = 0 private headlessTerminals = new Map() @@ -5431,6 +5436,7 @@ export class OrcaRuntimeService { }) this.hydrateHeadlessMobileSessionTabsFromWorkspaceSession(explicitWorktreeId) await this.refreshMobileSessionPtyRecords(explicitWorktreeId) + this.restoreLivePairedRendererSessionOwnedMobileTerminals(explicitWorktreeId) return this.getMobileSessionTabsForWorktree(explicitWorktreeId, clientNavigationId) } const worktree = await this.resolveWorktreeSelector(worktreeSelector) @@ -5440,6 +5446,7 @@ export class OrcaRuntimeService { }) this.hydrateHeadlessMobileSessionTabsFromWorkspaceSession(worktree.id) await this.refreshMobileSessionPtyRecords() + this.restoreLivePairedRendererSessionOwnedMobileTerminals(worktree.id) return this.getMobileSessionTabsForWorktree(worktree.id, clientNavigationId) } @@ -6008,6 +6015,7 @@ export class OrcaRuntimeService { const pty = this.ptysById.get(ptyId) if (pty?.worktreeId === worktreeId && pty.tabId === parentTabId) { pty.runtimeSessionOwned = false + this.setPairedRendererSessionOwnership(pty.ptyId, false) } } } @@ -6154,6 +6162,7 @@ export class OrcaRuntimeService { startupCwd?: string viewMode?: 'terminal' | 'chat' split?: { splitFromLeafId: string; direction: 'horizontal' | 'vertical' } + notify?: boolean } ): void { if ( @@ -6262,7 +6271,9 @@ export class OrcaRuntimeService { tabs } this.mobileSessionTabsByWorktree.set(worktreeId, next) - this.notifyMobileSessionTabsChanged(worktreeId) + if (args.notify !== false) { + this.notifyMobileSessionTabsChanged(worktreeId) + } } private touchMobileSessionSnapshotsForPty( @@ -6879,7 +6890,11 @@ export class OrcaRuntimeService { } } - private removePersistedHeadlessTerminalTab(worktreeId: string, parentTabId: string): string[] { + private removePersistedHeadlessTerminalTab( + worktreeId: string, + parentTabId: string, + options: { allowMissing?: boolean } = {} + ): string[] { const session = this.getWorkspaceSessionForWorktree(worktreeId) if (!session || !this.store?.setWorkspaceSession) { throw new Error('workspace_session_unavailable') @@ -6889,6 +6904,9 @@ export class OrcaRuntimeService { throw new Error('terminal_tab_pinned') } if (!result.closed) { + if (options.allowMissing) { + return [] + } throw new Error('tab_not_found') } this.setWorkspaceSessionForWorktree( @@ -7049,7 +7067,7 @@ export class OrcaRuntimeService { } } try { - await this.createHeadlessMobileSessionTerminal(worktreeId, targetsHost, undefined, { + await this.createRuntimeOwnedMobileSessionTerminal(worktreeId, targetsHost, undefined, { identity: { tabId: tab.parentTabId, leafId: tab.leafId, @@ -7317,13 +7335,19 @@ export class OrcaRuntimeService { reason?: RuntimeSessionTabCloseReason expectedPublicationEpoch?: string expectedTerminalHandle?: string + clientNavigationId?: string } = {} ): Promise { + const graphEpoch = options.clientNavigationId ? this.captureReadyGraphEpoch() : null const explicitWorktreeId = this.getValidatedExplicitWorktreeIdSelector(worktreeSelector) const worktreeId = explicitWorktreeId ?? (await this.resolveWorktreeSelector(worktreeSelector)).id this.hydrateHeadlessMobileSessionTabsFromWorkspaceSession(worktreeId) const observedPtyIds = await this.refreshMobileSessionPtyRecords() + if (graphEpoch !== null) { + this.assertStableReadyGraph(graphEpoch) + } + this.restoreLivePairedRendererSessionOwnedMobileTerminals(worktreeId) const snapshot = this.mobileSessionTabsByWorktree.get(worktreeId) if (options.reason !== undefined && options.reason !== 'user' && observedPtyIds === null) { // Why: keep-on-unknown must also restore the mirror the caller already pruned. @@ -7455,7 +7479,19 @@ export class OrcaRuntimeService { if (closingWholeParent && this.notifier?.closeTerminalTab) { // Why: whole-tab close is a lifecycle transaction. The renderer reply // arrives only after canonical retirement and a forced session flush. - await this.notifier.closeTerminalTab(tab.parentTabId) + const win = this.getAvailableAuthoritativeWindow() + if (win?.webContents.isDestroyed?.()) { + throw new Error('runtime_unavailable') + } + const releasePublicationThrottle = + options.clientNavigationId && win + ? this.rendererPublicationThrottle.acquire(win.webContents) + : () => {} + try { + await this.notifier.closeTerminalTab(tab.parentTabId) + } finally { + releasePublicationThrottle() + } const remainingSnapshot = this.mobileSessionTabsByWorktree.get(worktreeId) const remainingTab = remainingSnapshot?.tabs.find( (candidate): candidate is RuntimeMobileSessionTerminalTab => @@ -7467,7 +7503,10 @@ export class OrcaRuntimeService { this.isRuntimeOwnedHeadlessMobileTab(worktreeId, remainingTab) ) { // Why: after relay recovery the renderer can acknowledge a tab it no longer mirrors; the HUB must still retire its SSH-owned surface. - this.closeHeadlessMobileTerminalTab(worktreeId, remainingSnapshot, remainingTab) + this.closeHeadlessMobileTerminalTab(worktreeId, remainingSnapshot, remainingTab, { + // Why: the renderer may already have durably removed the tab before acknowledging. + allowMissingPersistedTab: true + }) this.notifyRendererOfHeadlessTerminalClose(tab.parentTabId) this.store?.flushOrThrow?.() } @@ -7641,11 +7680,13 @@ export class OrcaRuntimeService { worktreeId: string, snapshot: RuntimeMobileSessionTabsSnapshot, tab: RuntimeMobileSessionTerminalTab, - options: { killPtys?: boolean } = {} + options: { allowMissingPersistedTab?: boolean; killPtys?: boolean } = {} ): void { const closedParentTabId = tab.parentTabId this.clearRuntimeSessionOwnershipForMobileTab(worktreeId, snapshot, closedParentTabId) - const projectedPtyIds = this.removePersistedHeadlessTerminalTab(worktreeId, closedParentTabId) + const projectedPtyIds = this.removePersistedHeadlessTerminalTab(worktreeId, closedParentTabId, { + allowMissing: options.allowMissingPersistedTab + }) // Why: local provider ids can be reused after restart, so a dormant // persisted id is not kill authority. SSH relay ids remain durable exact // identities even before pane metadata reconnects. @@ -12847,6 +12888,7 @@ export class OrcaRuntimeService { if (pty) { pty.connected = false pty.runtimeSessionOwned = false + this.setPairedRendererSessionOwnership(pty.ptyId, false) pty.disconnectedAt = Date.now() pty.lastExitCode = exitCode this.resolvePtyExitWaiters(pty, ptyId) @@ -24651,11 +24693,13 @@ export class OrcaRuntimeService { launchAgent?: TuiAgent viewMode?: 'terminal' | 'chat' activate?: boolean + clientNavigationId?: string clientMutationId?: string signal?: AbortSignal } = {} ): Promise { - this.assertGraphReady() + const pairedCreate = Boolean(opts.clientNavigationId) + const graphEpoch = this.captureReadyGraphEpoch() const workspace = await this.resolveTerminalWorkspaceLaunchScope(worktreeSelector) const worktreeId = workspace.id const cwd = this.resolveWorkspaceTerminalStartupCwd(workspace, opts.cwd) @@ -24670,10 +24714,13 @@ export class OrcaRuntimeService { afterDesktopTabId = anchor.type === 'terminal' ? anchor.parentTabId : anchor.id } const startupCommand = await this.resolveMobileSessionTerminalCommand(workspace, opts) - + this.assertStableReadyGraph(graphEpoch) + if (opts.signal?.aborted) { + throw new Error('client_disconnected') + } const win = this.getAvailableAuthoritativeWindow() if (!win) { - return await this.createHeadlessMobileSessionTerminal( + return await this.createRuntimeOwnedMobileSessionTerminal( worktreeId, opts.activate !== false, opts.afterTabId, @@ -24686,142 +24733,155 @@ export class OrcaRuntimeService { launchAgent: startupCommand.launchAgent, viewMode: opts.viewMode, targetGroupId: opts.targetGroupId, - launchConfig: startupCommand.launchConfig + launchConfig: startupCommand.launchConfig, + signal: opts.signal } ) } - const requestId = randomUUID() - const reply = await new Promise<{ tabId: string; title: string }>((resolve, reject) => { - const timer = setTimeout(() => { - ipcMain.removeListener('terminal:tabCreateReply', handler) - opts.signal?.removeEventListener('abort', onAbort) - reject(new Error('Terminal creation timed out')) - }, 10_000) - // Why: a dead client connection cancels the wait; the renderer tab (and - // its shell) stays alive for the host and mirrors on reconnect (#7718). - const onAbort = (): void => { - clearTimeout(timer) - ipcMain.removeListener('terminal:tabCreateReply', handler) - reject(new Error('client_disconnected')) - } - - const handler = ( - event: Electron.IpcMainEvent, - r: { requestId: string; tabId?: string; title?: string; error?: string } - ): void => { - if (event.sender !== win.webContents || r.requestId !== requestId) { - return - } - clearTimeout(timer) - ipcMain.removeListener('terminal:tabCreateReply', handler) - opts.signal?.removeEventListener('abort', onAbort) - if (r.error) { - reject(new Error(r.error)) - } else { - resolve({ tabId: r.tabId!, title: r.title ?? '' }) - } - } - opts.signal?.addEventListener('abort', onAbort, { once: true }) - ipcMain.on('terminal:tabCreateReply', handler) - win.webContents.send('terminal:requestTabCreate', { - requestId, - worktreeId, - afterTabId: afterDesktopTabId, - targetGroupId: opts.targetGroupId, - command: startupCommand.command, - cwd, - ...(startupCommand.env ? { env: startupCommand.env } : {}), - ...(startupCommand.envToDelete ? { envToDelete: startupCommand.envToDelete } : {}), - ...(startupCommand.launchConfig ? { launchConfig: startupCommand.launchConfig } : {}), - ...(startupCommand.launchAgent ? { launchAgent: startupCommand.launchAgent } : {}), - ...(opts.viewMode ? { viewMode: opts.viewMode } : {}), - startupCommandDelivery: startupCommand.startupCommandDelivery, - source: 'runtime-session', - activate: opts.activate - }) - }) - - if (opts.activate !== false) { - this.notifier?.focusTerminal(reply.tabId, worktreeId, null) + if (win.webContents.isDestroyed?.()) { + throw new Error('runtime_unavailable') } - // Why: register the wait before the renderer's PTY spawn arrives so that - // spawn (registerPty) can publish the pty-backed surface main-side even if - // graph-sync is stalled (#7587). Removed in the finally below. - const pendingCreateKey = `${worktreeId}::${reply.tabId}` - // Why: a rescue publishes into the active group (opts.targetGroupId is not - // threaded); the renderer's reconciling publication then moves the tab to the - // requested group, so any wrong-group placement is cosmetic and stall-window-only. - this.pendingMobileTerminalCreatesByKey.set(pendingCreateKey, { - activate: opts.activate !== false, - selectIfNoActiveTab: true, - ...(opts.viewMode ? { viewMode: opts.viewMode } : {}) - }) + const releasePublicationThrottle = pairedCreate + ? this.rendererPublicationThrottle.acquire(win.webContents) + : () => {} try { - // Why: the PTY spawn and the tabCreate reply race on independent IPC - // channels; if the spawn already registered, publish immediately so the - // wait resolves without depending on a graph sync. - this.ensurePtyBackedMobileSurfaceForRendererTab(worktreeId, reply.tabId) - const surface = await this.waitForMobileTerminalSurface(worktreeId, reply.tabId, { - timeoutMs: MOBILE_TERMINAL_SURFACE_TIMEOUT_MS, - signal: opts.signal - }) - if (this.isReadyMobileTerminalSurface(surface)) { - return surface - } - const readySurface = await this.waitForMobileTerminalSurface(worktreeId, reply.tabId, { - timeoutMs: MOBILE_TERMINAL_READY_FALLBACK_MS, - requireReady: true, - signal: opts.signal - }).catch(() => null) - if (readySurface) { - return readySurface - } - if (opts.signal?.aborted) { - // Why: nobody awaits this create anymore; don't materialize or roll back — the renderer's own publication settles the tab. - throw new Error('client_disconnected') - } - const pendingSurface = this.findMobileTerminalSurface(worktreeId, reply.tabId) - if (!pendingSurface) { - throw new Error('Timed out waiting for terminal surface after creation') - } - // Why: a hidden renderer can publish the tab shell before the PTY spawns; reuse the same identity so later focus adopts instead of creating another tab. - return await this.createHeadlessMobileSessionTerminal( - worktreeId, - opts.activate !== false, - opts.afterTabId, - { + const requestId = randomUUID() + const reply = await new Promise<{ tabId: string; title: string }>((resolve, reject) => { + const timer = setTimeout(() => { + ipcMain.removeListener('terminal:tabCreateReply', handler) + opts.signal?.removeEventListener('abort', onAbort) + reject(new Error('Terminal creation timed out')) + }, 10_000) + // Why: a dead client connection cancels the wait; the renderer tab (and + // its shell) stays alive for the host and mirrors on reconnect (#7718). + const onAbort = (): void => { + clearTimeout(timer) + ipcMain.removeListener('terminal:tabCreateReply', handler) + reject(new Error('client_disconnected')) + } + + const handler = ( + event: Electron.IpcMainEvent, + r: { requestId: string; tabId?: string; title?: string; error?: string } + ): void => { + if (event.sender !== win.webContents || r.requestId !== requestId) { + return + } + clearTimeout(timer) + ipcMain.removeListener('terminal:tabCreateReply', handler) + opts.signal?.removeEventListener('abort', onAbort) + if (r.error) { + reject(new Error(r.error)) + } else { + resolve({ tabId: r.tabId!, title: r.title ?? '' }) + } + } + opts.signal?.addEventListener('abort', onAbort, { once: true }) + ipcMain.on('terminal:tabCreateReply', handler) + win.webContents.send('terminal:requestTabCreate', { + requestId, + worktreeId, + afterTabId: afterDesktopTabId, + targetGroupId: opts.targetGroupId, command: startupCommand.command, cwd, - env: startupCommand.env, - envToDelete: startupCommand.envToDelete, + ...(startupCommand.env ? { env: startupCommand.env } : {}), + ...(startupCommand.envToDelete ? { envToDelete: startupCommand.envToDelete } : {}), + ...(startupCommand.launchConfig ? { launchConfig: startupCommand.launchConfig } : {}), + ...(startupCommand.launchAgent ? { launchAgent: startupCommand.launchAgent } : {}), + ...(opts.viewMode ? { viewMode: opts.viewMode } : {}), startupCommandDelivery: startupCommand.startupCommandDelivery, - identity: { tabId: pendingSurface.tab.parentTabId, leafId: pendingSurface.tab.leafId }, - launchAgent: startupCommand.launchAgent, - viewMode: opts.viewMode, - targetGroupId: opts.targetGroupId, - launchConfig: startupCommand.launchConfig - } - ) - } catch (error) { - // Why: publication latency (hidden renderer) can trip the surface timeout; rescue only when a live PTY backs the tab, else a ghost tab skips rollback (#7587). - if (this.findLiveRegisteredPtyForRendererTab(worktreeId, reply.tabId)) { - const rescued = this.ensurePtyBackedMobileSurfaceForRendererTab(worktreeId, reply.tabId) - if (rescued) { - return rescued - } + source: 'runtime-session', + activate: opts.activate + }) + }) + + if (opts.activate !== false) { + this.notifier?.focusTerminal(reply.tabId, worktreeId, null) } - // Why: don't roll back on a client disconnect or a live shell already backing the tab — that would kill a visible terminal ("tab dies after ~10s", #7718). - if ( - isClientDisconnectedError(error) || - this.hasLiveShellForRendererTab(worktreeId, reply.tabId) - ) { + // Why: register the wait before the renderer's PTY spawn arrives so that + // spawn (registerPty) can publish the pty-backed surface main-side even if + // graph-sync is stalled (#7587). Removed in the finally below. + const pendingCreateKey = `${worktreeId}::${reply.tabId}` + // Why: a rescue publishes into the active group (opts.targetGroupId is not + // threaded); the renderer's reconciling publication then moves the tab to the + // requested group, so any wrong-group placement is cosmetic and stall-window-only. + this.pendingMobileTerminalCreatesByKey.set(pendingCreateKey, { + activate: opts.activate !== false, + paired: pairedCreate, + selectIfNoActiveTab: true, + ...(opts.viewMode ? { viewMode: opts.viewMode } : {}) + }) + try { + // Why: the PTY spawn and the tabCreate reply race on independent IPC + // channels; if the spawn already registered, publish immediately so the + // wait resolves without depending on a graph sync. + this.ensurePtyBackedMobileSurfaceForRendererTab(worktreeId, reply.tabId) + const surface = await this.waitForMobileTerminalSurface(worktreeId, reply.tabId, { + timeoutMs: MOBILE_TERMINAL_SURFACE_TIMEOUT_MS, + signal: opts.signal + }) + if (this.isReadyMobileTerminalSurface(surface)) { + return surface + } + const readySurface = await this.waitForMobileTerminalSurface(worktreeId, reply.tabId, { + timeoutMs: MOBILE_TERMINAL_READY_FALLBACK_MS, + requireReady: true, + signal: opts.signal + }).catch(() => null) + if (readySurface) { + return readySurface + } + if (opts.signal?.aborted) { + // Why: nobody awaits this create anymore; don't materialize or roll back — the renderer's own publication settles the tab. + throw new Error('client_disconnected') + } + const pendingSurface = this.findMobileTerminalSurface(worktreeId, reply.tabId) + if (!pendingSurface) { + throw new Error('Timed out waiting for terminal surface after creation') + } + // Why: a hidden renderer can publish the tab shell before the PTY spawns; reuse the same identity so later focus adopts instead of creating another tab. + return await this.createRuntimeOwnedMobileSessionTerminal( + worktreeId, + opts.activate !== false, + opts.afterTabId, + { + command: startupCommand.command, + cwd, + env: startupCommand.env, + envToDelete: startupCommand.envToDelete, + startupCommandDelivery: startupCommand.startupCommandDelivery, + identity: { tabId: pendingSurface.tab.parentTabId, leafId: pendingSurface.tab.leafId }, + launchAgent: startupCommand.launchAgent, + viewMode: opts.viewMode, + targetGroupId: opts.targetGroupId, + launchConfig: startupCommand.launchConfig, + signal: opts.signal + } + ) + } catch (error) { + // Why: publication latency (hidden renderer) can trip the surface timeout; rescue only when a live PTY backs the tab, else a ghost tab skips rollback (#7587). + if (this.findLiveRegisteredPtyForRendererTab(worktreeId, reply.tabId)) { + const rescued = this.ensurePtyBackedMobileSurfaceForRendererTab(worktreeId, reply.tabId) + if (rescued) { + return rescued + } + } + // Why: don't roll back on a client disconnect or a live shell already backing the tab — that would kill a visible terminal ("tab dies after ~10s", #7718). + if ( + isClientDisconnectedError(error) || + this.hasLiveShellForRendererTab(worktreeId, reply.tabId) + ) { + throw error + } + // Why: renderer made the tab but no live PTY backs it (real spawn/handle failure); roll it back so it can't linger as a ghost in mobile snapshots. + this.notifier?.closeTerminal(reply.tabId) throw error + } finally { + this.pendingMobileTerminalCreatesByKey.delete(pendingCreateKey) } - // Why: renderer made the tab but no live PTY backs it (real spawn/handle failure); roll it back so it can't linger as a ghost in mobile snapshots. - this.notifier?.closeTerminal(reply.tabId) - throw error } finally { - this.pendingMobileTerminalCreatesByKey.delete(pendingCreateKey) + releasePublicationThrottle() } } @@ -24909,7 +24969,7 @@ export class OrcaRuntimeService { } } - private async createHeadlessMobileSessionTerminal( + private async createRuntimeOwnedMobileSessionTerminal( worktreeId: string, activate: boolean, afterTabId?: string, @@ -24924,6 +24984,7 @@ export class OrcaRuntimeService { viewMode?: 'terminal' | 'chat' targetGroupId?: string launchConfig?: SleepingAgentLaunchConfig + signal?: AbortSignal } = {} ): Promise { const workspace = await this.resolveTerminalWorkspaceLaunchScope(`id:${worktreeId}`) @@ -24952,7 +25013,8 @@ export class OrcaRuntimeService { : {}), persistHostSessionBinding: true, // Why: this method publishes the authoritative snapshot below; skip the intermediate publish to avoid a wrong-group flash. - deferMobileSessionPublish: true + deferMobileSessionPublish: true, + signal: opts.signal }) const livePty = this.getLivePtyForHandle(terminal.handle) if (!livePty) { @@ -25148,6 +25210,9 @@ export class OrcaRuntimeService { const pty = this.findLiveRegisteredPtyForRendererTab(worktreeId, tabId) if (pty) { pty.runtimeSessionOwned = true + if (pending.paired) { + this.setPairedRendererSessionOwnership(pty.ptyId, true) + } } if ( existing && @@ -25176,6 +25241,69 @@ export class OrcaRuntimeService { return this.findMobileTerminalSurface(worktreeId, tabId) } + private restoreLivePairedRendererSessionOwnedMobileTerminals( + worktreeId: string | null, + options: { missingSnapshotOnly?: boolean; notify?: boolean } = {} + ): void { + for (const ptyId of this.pairedRendererSessionOwnedPtyIds) { + const pty = this.ptysById.get(ptyId) + if ( + !pty?.connected || + !pty.tabId || + (worktreeId !== null && !runtimeWorktreeIdsEqual(pty.worktreeId, worktreeId)) + ) { + continue + } + const targetWorktreeId = worktreeId ?? pty.worktreeId + const pane = parsePaneKey(pty.paneKey ?? '') + if (!pane || pane.tabId !== pty.tabId) { + continue + } + const existing = this.mobileSessionTabsByWorktree.get(targetWorktreeId) + if (existing && options.missingSnapshotOnly) { + continue + } + if ( + existing?.tabs.some( + (tab) => + tab.type === 'terminal' && + (tab.ptyId === pty.ptyId || + (tab.parentTabId === pty.tabId && tab.leafId === pane.leafId)) + ) + ) { + continue + } + if (!existing) { + this.mobileSessionTabsByWorktree.set(targetWorktreeId, { + worktree: targetWorktreeId, + publicationEpoch: `renderer-rescue:${Date.now().toString(36)}`, + snapshotVersion: 0, + activeGroupId: null, + activeTabId: null, + activeTabType: null, + tabGroups: [], + tabs: [] + }) + } + this.publishPtyBackedMobileSessionTerminal(targetWorktreeId, pty, { + tabId: pty.tabId, + leafId: pane.leafId, + title: null, + activate: false, + selectIfNoActiveTab: false, + notify: options.notify + }) + } + } + + private setPairedRendererSessionOwnership(ptyId: string, owned: boolean): void { + if (owned) { + this.pairedRendererSessionOwnedPtyIds.add(ptyId) + } else { + this.pairedRendererSessionOwnedPtyIds.delete(ptyId) + } + } + private findLiveRegisteredPtyForRendererTab( worktreeId: string, tabId: string @@ -25602,6 +25730,10 @@ export class OrcaRuntimeService { createdPty.tabId = parentTabId createdPty.paneKey = paneKey createdPty.runtimeSessionOwned = pty.runtimeSessionOwned + this.setPairedRendererSessionOwnership( + createdPty.ptyId, + this.pairedRendererSessionOwnedPtyIds.has(pty.ptyId) + ) } try { @@ -25617,6 +25749,7 @@ export class OrcaRuntimeService { splitTelemetrySource: opts.telemetrySource }) } catch (error) { + this.setPairedRendererSessionOwnership(result.id, false) this.ptyController.kill?.(result.id) throw error } @@ -27978,6 +28111,7 @@ export class OrcaRuntimeService { private dropDisconnectedPtyRecord(ptyId: string): void { // Why: pruning can remove a PTY without the normal exit callback. this.advancePtyLifecycleGeneration(ptyId) + this.pairedRendererSessionOwnedPtyIds.delete(ptyId) this.ptysById.delete(ptyId) this.recentPtyOutputById.delete(ptyId) this.setupCompletionTokenByPtyId.delete(ptyId) @@ -28110,6 +28244,10 @@ export class OrcaRuntimeService { // renderer resends before they replace an entry — so reference identity // before/after detects exactly the entries that actually changed. const before = new Map(this.mobileSessionTabsByWorktree) + this.restoreLivePairedRendererSessionOwnedMobileTerminals(null, { + missingSnapshotOnly: true, + notify: false + }) const worktreeIdsToHydrate = this.getKnownWorkspaceSessionWorktreeIds() for (const snapshot of snapshots) { worktreeIdsToHydrate.add(snapshot.worktree) diff --git a/src/main/runtime/rpc/methods/session-tab-close-methods.ts b/src/main/runtime/rpc/methods/session-tab-close-methods.ts index e11e2b78aa5..eabffa24a73 100644 --- a/src/main/runtime/rpc/methods/session-tab-close-methods.ts +++ b/src/main/runtime/rpc/methods/session-tab-close-methods.ts @@ -28,7 +28,10 @@ export const SESSION_TAB_CLOSE_METHODS: RpcAnyMethod[] = [ const result = await context.runtime.closeMobileSessionTab( params.worktree, params.tabId, - { reason: 'user' } + { + reason: 'user', + ...(context.pairedDeviceId ? { clientNavigationId: context.pairedDeviceId } : {}) + } ) span.setAttribute( 'decision', @@ -68,7 +71,8 @@ export const SESSION_TAB_CLOSE_METHODS: RpcAnyMethod[] = [ { reason: params.reason, expectedPublicationEpoch: params.publicationEpoch, - expectedTerminalHandle: params.terminal + expectedTerminalHandle: params.terminal, + ...(context.pairedDeviceId ? { clientNavigationId: context.pairedDeviceId } : {}) } ) span.setAttribute( diff --git a/src/main/runtime/rpc/methods/session-tabs.test.ts b/src/main/runtime/rpc/methods/session-tabs.test.ts index b9496b3da54..428fde012a3 100644 --- a/src/main/runtime/rpc/methods/session-tabs.test.ts +++ b/src/main/runtime/rpc/methods/session-tabs.test.ts @@ -136,7 +136,8 @@ describe('session tab RPC methods', () => { expect(replies).toHaveLength(1) expect(runtime.closeMobileSessionTab).toHaveBeenCalledWith('id:wt-1', 'tab-1', { - reason: 'user' + reason: 'user', + clientNavigationId: 'current-runtime' }) expect(runtime.refuseUnattributedMobileSessionTabClose).not.toHaveBeenCalled() }) @@ -180,7 +181,8 @@ describe('session tab RPC methods', () => { expect(replies).toHaveLength(1) expect(runtime.closeMobileSessionTab).toHaveBeenCalledWith('id:wt-1', 'tab-1', { - reason: 'user' + reason: 'user', + clientNavigationId: 'legacy-runtime' }) expect(runtime.refuseUnattributedMobileSessionTabClose).not.toHaveBeenCalled() }) diff --git a/src/main/runtime/runtime-rpc.test.ts b/src/main/runtime/runtime-rpc.test.ts index 4339679355c..fe44dc64b22 100644 --- a/src/main/runtime/runtime-rpc.test.ts +++ b/src/main/runtime/runtime-rpc.test.ts @@ -3419,6 +3419,47 @@ describe('OrcaRuntimeRpcServer', () => { ) }) + it('rejects unpaired terminal creates before runtime dispatch', async () => { + const userDataPath = mkdtempSync(join(tmpdir(), 'orca-runtime-rpc-')) + const createMobileSessionTerminal = vi.fn() + const runtime = { + getRuntimeId: () => 'test-runtime', + createMobileSessionTerminal + } as unknown as OrcaRuntimeService + const server = new OrcaRuntimeRpcServer({ runtime, userDataPath, enableWebSocket: false }) + server['deviceRegistry'] = new DeviceRegistry(userDataPath) + const replies: Record[] = [] + const send = async (id: string, deviceToken?: string): Promise => { + await server['handleWebSocketMessage']( + JSON.stringify({ + id, + method: 'session.tabs.createTerminal', + ...(deviceToken ? { deviceToken } : {}), + params: { worktree: 'id:wt-1' } + }), + (response) => replies.push(JSON.parse(response) as Record), + () => {} + ) + } + + await send('req_missing') + await send('req_invalid', 'invalid-token') + + expect(replies).toEqual([ + expect.objectContaining({ + id: 'req_missing', + error: expect.objectContaining({ code: 'unauthorized' }), + ok: false + }), + expect.objectContaining({ + id: 'req_invalid', + error: expect.objectContaining({ code: 'unauthorized' }), + ok: false + }) + ]) + expect(createMobileSessionTerminal).not.toHaveBeenCalled() + }) + it('allows runtime-scoped WebSocket tokens to use the full RPC surface', async () => { const userDataPath = mkdtempSync(join(tmpdir(), 'orca-runtime-rpc-')) const pushRuntimeGit = vi.fn().mockResolvedValue({ ok: true }) diff --git a/src/main/window/renderer-publication-throttle.test.ts b/src/main/window/renderer-publication-throttle.test.ts new file mode 100644 index 00000000000..5715ef25ae1 --- /dev/null +++ b/src/main/window/renderer-publication-throttle.test.ts @@ -0,0 +1,58 @@ +import { describe, expect, it } from 'vitest' +import { + RendererPublicationThrottle, + type RendererPublicationThrottleTarget +} from './renderer-publication-throttle' + +function createTarget(): RendererPublicationThrottleTarget & { + calls: boolean[] + destroyed: boolean +} { + const calls: boolean[] = [] + return { + calls, + destroyed: false, + isDestroyed() { + return this.destroyed + }, + setBackgroundThrottling(allowed) { + calls.push(allowed) + } + } +} + +describe('RendererPublicationThrottle', () => { + it('unthrottles only while a publication lease is active', () => { + const target = createTarget() + const throttle = new RendererPublicationThrottle() + + const release = throttle.acquire(target) + release() + + expect(target.calls).toEqual([false, true]) + }) + + it('keeps overlapping publications unthrottled until the last release', () => { + const target = createTarget() + const throttle = new RendererPublicationThrottle() + + const releaseFirst = throttle.acquire(target) + const releaseSecond = throttle.acquire(target) + releaseFirst() + releaseSecond() + releaseSecond() + + expect(target.calls).toEqual([false, true]) + }) + + it('does not restore throttling on a destroyed renderer', () => { + const target = createTarget() + const throttle = new RendererPublicationThrottle() + + const release = throttle.acquire(target) + target.destroyed = true + release() + + expect(target.calls).toEqual([false]) + }) +}) diff --git a/src/main/window/renderer-publication-throttle.ts b/src/main/window/renderer-publication-throttle.ts new file mode 100644 index 00000000000..c7a63d74597 --- /dev/null +++ b/src/main/window/renderer-publication-throttle.ts @@ -0,0 +1,32 @@ +export type RendererPublicationThrottleTarget = { + isDestroyed?: () => boolean + setBackgroundThrottling: (allowed: boolean) => void +} + +export class RendererPublicationThrottle { + private readonly leasesByTarget = new Map() + + acquire(target: RendererPublicationThrottleTarget): () => void { + const leaseCount = this.leasesByTarget.get(target) ?? 0 + if (leaseCount === 0) { + target.setBackgroundThrottling(false) + } + this.leasesByTarget.set(target, leaseCount + 1) + let released = false + return () => { + if (released) { + return + } + released = true + const remaining = (this.leasesByTarget.get(target) ?? 1) - 1 + if (remaining > 0) { + this.leasesByTarget.set(target, remaining) + return + } + this.leasesByTarget.delete(target) + if (target.isDestroyed?.() !== true) { + target.setBackgroundThrottling(true) + } + } + } +} diff --git a/src/renderer/src/store/slices/cmd-j-create-actions.test.ts b/src/renderer/src/store/slices/cmd-j-create-actions.test.ts index 17932185516..abf1904cc19 100644 --- a/src/renderer/src/store/slices/cmd-j-create-actions.test.ts +++ b/src/renderer/src/store/slices/cmd-j-create-actions.test.ts @@ -1,5 +1,6 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { FLOATING_TERMINAL_WORKTREE_ID } from '../../../../shared/constants' +import { folderWorkspaceKey } from '../../../../shared/workspace-scope' import type { AppState } from '../types' import { createTestStore, makeWorktree, seedStore, TEST_REPO } from './store-test-helpers' @@ -15,6 +16,11 @@ vi.mock('@/lib/focus-terminal-tab-surface', () => ({ focusTerminalTabSurface: vi.fn() })) +vi.mock('@/lib/web-client-location', () => ({ + isWebClientLocation: () => + Boolean((globalThis as { __ORCA_WEB_CLIENT__?: boolean }).__ORCA_WEB_CLIENT__) +})) + const pairedWebFlag = globalThis as { __ORCA_WEB_CLIENT__?: boolean } function seedActiveWorkspace(store: ReturnType): void { @@ -165,6 +171,61 @@ describe('Cmd+J lifted creation actions', () => { expect(store.getState().tabsByWorktree['wt-1'] ?? []).toEqual([]) }) + it('does not create a local folder terminal while paired ownership is unresolved', async () => { + const folderId = 'folder-1' + const workspaceKey = folderWorkspaceKey(folderId) + const store = createTestStore() + seedStore(store, { + activeWorktreeId: workspaceKey, + settings: { activeRuntimeEnvironmentId: null } as AppState['settings'], + folderWorkspaces: [ + { + id: folderId, + projectGroupId: 'group-1', + name: 'Folder workspace', + folderPath: '/tmp/folder', + connectionId: null, + linkedTask: null, + comment: '', + isArchived: false, + isUnread: false, + isPinned: false, + sortOrder: 1, + lastActivityAt: 0, + createdAt: 1, + updatedAt: 1 + } + ], + projectGroups: [ + { + id: 'group-1', + name: 'Folder group', + parentPath: '/tmp', + connectionId: null, + executionHostId: null, + parentGroupId: null, + createdFrom: 'manual', + tabOrder: 0, + isCollapsed: false, + color: null, + createdAt: 1, + updatedAt: 1 + } + ], + groupsByWorktree: { + [workspaceKey]: [ + { id: 'tab-group-1', worktreeId: workspaceKey, activeTabId: null, tabOrder: [] } + ] + }, + activeGroupIdByWorktree: { [workspaceKey]: 'tab-group-1' } + }) + + await store.getState().openNewTerminalTabInActiveWorkspace('tab-group-1') + + expect(createWebRuntimeSessionTerminalMock).not.toHaveBeenCalled() + expect(store.getState().tabsByWorktree[workspaceKey] ?? []).toEqual([]) + }) + it('creates desktop remote-server terminal tabs through the owning runtime', async () => { delete pairedWebFlag.__ORCA_WEB_CLIENT__ createWebRuntimeSessionTerminalMock.mockResolvedValue(false) diff --git a/src/renderer/src/store/slices/terminals.ts b/src/renderer/src/store/slices/terminals.ts index 210c2f35537..4d1492db8fa 100644 --- a/src/renderer/src/store/slices/terminals.ts +++ b/src/renderer/src/store/slices/terminals.ts @@ -124,6 +124,7 @@ import { focusTerminalTabSurface } from '@/lib/focus-terminal-tab-surface' import { getRuntimeEnvironmentIdForWorktree } from '@/lib/worktree-runtime-owner' import { resolveTerminalWorktreeRoute } from '@/lib/terminal-worktree-route' import { resolveWorktreeOperationRouteResult } from '@/lib/worktree-operation-route' +import { isWebClientLocation } from '@/lib/web-client-location' import { getLocalProjectExecutionRuntimeContext } from '@/lib/local-preflight-context' import type { NativeChatLaunchDraft, NativeChatLaunchPrompt } from '@/lib/native-chat-launch-prompt' import { resolveAgentPaneAuthorityKey } from './agent-pane-authority' @@ -1527,6 +1528,9 @@ export const createTerminalSlice: StateCreator }) return } + if (isWebClientLocation() && worktreeId !== FLOATING_TERMINAL_WORKTREE_ID) { + return + } const terminal = get().createTab(worktreeId, groupId) get().setActiveTab(terminal.id) get().setActiveTabType('terminal') diff --git a/tests/e2e/paired-remote-terminal-stall-recovery.spec.ts b/tests/e2e/paired-remote-terminal-stall-recovery.spec.ts index 2f6026f0b2e..4406ebd2f18 100644 --- a/tests/e2e/paired-remote-terminal-stall-recovery.spec.ts +++ b/tests/e2e/paired-remote-terminal-stall-recovery.spec.ts @@ -1,7 +1,8 @@ import { mkdtempSync, rmSync, writeFileSync } from 'node:fs' import os from 'node:os' import path from 'node:path' -import type { Page } from '@stablyai/playwright-test' +import type { ElectronApplication, Page } from '@stablyai/playwright-test' +import { PNG } from 'pngjs' import type { RuntimeTerminalRead } from '../../src/shared/runtime-types' import { toWebTerminalSurfaceTabId } from '../../src/shared/terminal-surface-id' import { expect, test } from './helpers/orca-app' @@ -12,6 +13,7 @@ import { import { getTerminalContent, waitForActivePanePtyId } from './helpers/terminal' const MIN_EXHAUSTED_ACK_BYTES = 400 * 1024 +const PUBLICATION_DEADLINE_MS = 10_000 const scratch = mkdtempSync(path.join(os.tmpdir(), 'orca-paired-stalled-stream-')) const fixturePath = path.join(scratch, 'stalled-stream-terminal.mjs') writeFileSync( @@ -30,7 +32,7 @@ writeFileSync( " process.stdout.write('HOST_FLOOD_COMPLETE\\r\\n')", ' continue', ' }', - ' process.stdout.write(`LIVE:${input}\\r\\n`)', + " process.stdout.write(`\\x1b[48;2;24;96;144mLIVE:${input}${' '.repeat(48)}\\x1b[0m\\r\\n`)", ' }', '})', 'process.stdin.resume()' @@ -65,11 +67,169 @@ async function callRuntime(page: Page, method: string, params: unknown) ) as Promise } +type AppResourceProxies = { + gpuCpuPercent: number + gpuIdleWakeupsPerSecond: number + processCount: number + totalCpuPercent: number + totalIdleWakeupsPerSecond: number +} + +async function getAppResourceProxies( + electronApp: ElectronApplication +): Promise { + return electronApp.evaluate(({ app }) => { + const metrics = app.getAppMetrics() + const gpu = metrics.find((metric) => metric.type === 'GPU') + return { + gpuCpuPercent: gpu?.cpu.percentCPUUsage ?? 0, + gpuIdleWakeupsPerSecond: gpu?.cpu.idleWakeupsPerSecond ?? 0, + processCount: metrics.length, + totalCpuPercent: metrics.reduce((total, metric) => total + metric.cpu.percentCPUUsage, 0), + totalIdleWakeupsPerSecond: metrics.reduce( + (total, metric) => total + metric.cpu.idleWakeupsPerSecond, + 0 + ) + } + }) +} + +async function minimizeHeadedHost(electronApp: ElectronApplication, page: Page): Promise { + const host = await electronApp.browserWindow(page) + await host.evaluate((window) => window.minimize()) + await expect + .poll(() => + host.evaluate((window) => ({ + backgroundThrottling: window.webContents.getBackgroundThrottling(), + minimized: window.isMinimized(), + visible: window.isVisible() + })) + ) + .toEqual({ backgroundThrottling: true, minimized: true, visible: false }) +} + +async function restoreHeadedHost(electronApp: ElectronApplication, page: Page): Promise { + const host = await electronApp.browserWindow(page) + await host.evaluate((window) => { + window.restore() + window.show() + window.focus() + }) + await expect + .poll(() => + host.evaluate((window) => ({ + backgroundThrottling: window.webContents.getBackgroundThrottling(), + minimized: window.isMinimized(), + visible: window.isVisible() + })) + ) + .toEqual({ backgroundThrottling: true, minimized: false, visible: true }) + await expect.poll(() => page.evaluate(() => document.visibilityState)).toBe('visible') + await page.evaluate( + () => new Promise((resolve) => requestAnimationFrame(() => requestAnimationFrame(resolve))) + ) +} + +async function showHeadedClient(electronApp: ElectronApplication, page: Page): Promise { + const clientWindow = await electronApp.browserWindow(page) + await clientWindow.evaluate((window) => { + window.show() + window.focus() + }) + await expect.poll(() => clientWindow.evaluate((window) => window.isVisible())).toBe(true) +} + +function countForegroundPixels(buffer: Buffer): number { + const image = PNG.sync.read(buffer) + const buckets = new Map() + for (let offset = 0; offset < image.data.length; offset += 4) { + if ((image.data[offset + 3] ?? 0) < 128) { + continue + } + const red = image.data[offset] ?? 0 + const green = image.data[offset + 1] ?? 0 + const blue = image.data[offset + 2] ?? 0 + const key = `${red >> 3},${green >> 3},${blue >> 3}` + const bucket = buckets.get(key) ?? { count: 0, red, green, blue } + bucket.count += 1 + buckets.set(key, bucket) + } + const background = [...buckets.values()].sort((a, b) => b.count - a.count)[0] + if (!background) { + return 0 + } + let foregroundPixels = 0 + for (let offset = 0; offset < image.data.length; offset += 4) { + if ((image.data[offset + 3] ?? 0) < 128) { + continue + } + const distance = + Math.abs((image.data[offset] ?? 0) - background.red) + + Math.abs((image.data[offset + 1] ?? 0) - background.green) + + Math.abs((image.data[offset + 2] ?? 0) - background.blue) + if (distance > 48) { + foregroundPixels += 1 + } + } + return foregroundPixels +} + +function countVisualMarkerPixels(buffer: Buffer): number { + const image = PNG.sync.read(buffer) + let count = 0 + for (let offset = 0; offset < image.data.length; offset += 4) { + const red = image.data[offset] ?? 0 + const green = image.data[offset + 1] ?? 0 + const blue = image.data[offset + 2] ?? 0 + if ( + (image.data[offset + 3] ?? 0) >= 245 && + blue >= 100 && + blue - red >= 50 && + blue - green >= 20 && + green - red >= 10 + ) { + count += 1 + } + } + return count +} + +async function findHostPaneWithMarker( + page: Page, + marker: string +): Promise<{ paneId: number; ptyId: string; tabId: string }> { + let target: { paneId: number; ptyId: string; tabId: string } | null = null + await expect + .poll( + async () => { + target = await page.evaluate((expectedMarker) => { + for (const [tabId, manager] of window.__paneManagers?.entries() ?? []) { + for (const pane of manager.getPanes?.() ?? []) { + const content = pane.serializeAddon?.serialize?.() ?? '' + const ptyId = pane.container?.dataset?.ptyId + if (content.includes(expectedMarker) && ptyId) { + return { paneId: pane.id, ptyId, tabId } + } + } + } + return null + }, marker) + return target !== null + }, + { timeout: 30_000, message: 'host renderer never mirrored the paired terminal marker' } + ) + .toBe(true) + if (!target) { + throw new Error('Host terminal marker target disappeared') + } + return target +} + test('restarts one ACK-starved paired terminal stream without replacing its PTY @headful', async ({ electronApp, orcaPage -}) => { - test.setTimeout(120_000) +}, testInfo) => { + test.setTimeout(150_000) const liveMarker = `PAIRED_STALL_RECOVERED_${Date.now()}` const worktree = await orcaPage.evaluate(() => { const state = window.__store?.getState() @@ -80,11 +240,13 @@ test('restarts one ACK-starved paired terminal stream without replacing its PTY } return { id: active.id } }) + const noClientResources = await getAppResourceProxies(electronApp) const offer = await createRuntimeDesktopPairingOffer(orcaPage) const client = await launchPairedWebClient(electronApp, offer, { disableRemoteTerminalStallRecovery: process.env.ORCA_E2E_DISABLE_REMOTE_TERMINAL_STALL_RECOVERY === '1' }) + let observer: Awaited> | null = null let terminal: string | null = null try { await expect @@ -101,8 +263,29 @@ test('restarts one ACK-starved paired terminal stream without replacing its PTY { timeout: 30_000 } ) .toBe(true) + const observerOffer = await createRuntimeDesktopPairingOffer(orcaPage) + observer = await launchPairedWebClient(electronApp, observerOffer) + await showHeadedClient(electronApp, client.page) + await showHeadedClient(electronApp, observer.page) + await expect + .poll( + () => + observer?.page.evaluate( + (worktreeId) => + window.__store + ?.getState() + .allWorktrees() + .some((candidate) => candidate.id === worktreeId), + worktree.id + ), + { timeout: 30_000 } + ) + .toBe(true) + const connectedIdleResources = await getAppResourceProxies(electronApp) + await minimizeHeadedHost(electronApp, orcaPage) + const createStartedAt = performance.now() const created = await callRuntime<{ - tab: { parentTabId: string; terminal: string | null } + tab: { id: string; parentTabId: string; terminal: string | null } }>(client.page, 'session.tabs.createTerminal', { worktree: `id:${worktree.id}`, command: fixtureCommand(), @@ -110,6 +293,8 @@ test('restarts one ACK-starved paired terminal stream without replacing its PTY select: false, navigation: 'caller' }) + const createLatencyMs = performance.now() - createStartedAt + expect(createLatencyMs).toBeLessThan(PUBLICATION_DEADLINE_MS) terminal = created.tab.terminal if (!terminal) { throw new Error('Paired runtime did not publish the stalled-stream fixture') @@ -128,6 +313,42 @@ test('restarts one ACK-starved paired terminal stream without replacing its PTY { timeout: 30_000 } ) .toBe(true) + await expect + .poll( + () => + observer?.page.evaluate( + ({ tabId, worktreeId }) => + (window.__store?.getState().tabsByWorktree[worktreeId] ?? []).some( + (tab) => tab.id === tabId + ), + { tabId: webTabId, worktreeId: worktree.id } + ), + { timeout: 30_000 } + ) + .toBe(true) + await expect + .poll( + () => + orcaPage.evaluate( + ({ tabId, worktreeId }) => + (window.__store?.getState().tabsByWorktree[worktreeId] ?? []).some( + (tab) => tab.id === tabId + ), + { tabId: created.tab.parentTabId, worktreeId: worktree.id } + ), + { timeout: 30_000 } + ) + .toBe(true) + expect( + ( + await callRuntime<{ tabs: { terminal?: string | null }[] }>( + client.page, + 'session.tabs.list', + { worktree: `id:${worktree.id}` } + ) + ).tabs.some((candidate) => candidate.terminal === terminal), + 'authoritative inventory dropped the terminal before ACK starvation' + ).toBe(true) await client.page.evaluate( (worktreeId) => window.__store?.getState().setActiveWorktree(worktreeId), worktree.id @@ -139,6 +360,20 @@ test('restarts one ACK-starved paired terminal stream without replacing its PTY await expect .poll(() => getTerminalContent(client.page), { timeout: 30_000 }) .toContain('PAIRED_STALL_READY') + await observer.page.evaluate( + (worktreeId) => window.__store?.getState().setActiveWorktree(worktreeId), + worktree.id + ) + const observerTab = observer.page.locator( + `[data-testid="sortable-tab"][data-tab-id="${webTabId}"]` + ) + await expect(observerTab).toBeVisible({ timeout: 30_000 }) + await observerTab.click() + const observerOriginalPtyId = await waitForActivePanePtyId(observer.page, 30_000) + expect(observerOriginalPtyId.split('@@').at(-1)).toBe(originalPtyId.split('@@').at(-1)) + await expect + .poll(() => getTerminalContent(observer.page), { timeout: 30_000 }) + .toContain('PAIRED_STALL_READY') await client.page.evaluate((target) => { const gate = ( @@ -233,6 +468,157 @@ test('restarts one ACK-starved paired terminal stream without replacing its PTY .toContain(`LIVE:${liveMarker}`) expect(await waitForActivePanePtyId(client.page, 30_000)).toBe(originalPtyId) await expect(tab).toHaveAttribute('data-active', 'true') + await expect + .poll(() => getTerminalContent(observer.page), { timeout: 30_000 }) + .toContain(`LIVE:${liveMarker}`) + expect(await waitForActivePanePtyId(observer.page, 30_000)).toBe(observerOriginalPtyId) + expect( + ( + await callRuntime<{ tabs: { terminal?: string | null }[] }>( + client.page, + 'session.tabs.list', + { worktree: `id:${worktree.id}` } + ) + ).tabs.some((candidate) => candidate.terminal === terminal), + 'authoritative inventory dropped the terminal during ACK recovery' + ).toBe(true) + + await restoreHeadedHost(electronApp, orcaPage) + await orcaPage.evaluate( + (worktreeId) => window.__store?.getState().setActiveWorktree(worktreeId), + worktree.id + ) + const hostTab = orcaPage.locator( + `[data-testid="sortable-tab"][data-tab-id="${created.tab.parentTabId}"]` + ) + await expect(hostTab).toBeVisible({ timeout: 30_000 }) + await hostTab.click() + const hostPane = await findHostPaneWithMarker(orcaPage, `LIVE:${liveMarker}`) + expect(hostPane.tabId).toBe(created.tab.parentTabId) + await orcaPage.evaluate(({ paneId, tabId }) => { + const manager = window.__paneManagers?.get(tabId) + manager?.setActivePane?.(paneId, { focus: true }) + }, hostPane) + await expect + .poll(() => getTerminalContent(orcaPage), { timeout: 30_000 }) + .toContain(`LIVE:${liveMarker}`) + const restoredTerminalScreenshot = await orcaPage + .locator( + `[data-terminal-tab-id="${hostPane.tabId}"] .pane[data-pane-id="${hostPane.paneId}"] .xterm-screen` + ) + .screenshot({ animations: 'disabled' }) + await testInfo.attach('host-terminal-after-create-restore', { + body: restoredTerminalScreenshot, + contentType: 'image/png' + }) + expect( + countVisualMarkerPixels(restoredTerminalScreenshot), + 'host terminal marker did not repaint after the background publication toggle' + ).toBeGreaterThan(500) + expect( + ( + await callRuntime<{ tabs: { terminal?: string | null }[] }>( + client.page, + 'session.tabs.list', + { worktree: `id:${worktree.id}` } + ) + ).tabs.some((candidate) => candidate.terminal === terminal), + 'authoritative inventory dropped the terminal while restoring the host' + ).toBe(true) + + await minimizeHeadedHost(electronApp, orcaPage) + await showHeadedClient(electronApp, observer.page) + + const authoritativeInventory = await callRuntime<{ + tabs: { id: string; parentTabId?: string; terminal?: string | null }[] + }>(client.page, 'session.tabs.list', { worktree: `id:${worktree.id}` }) + const authoritativeTab = authoritativeInventory.tabs.find( + (candidate) => candidate.terminal === terminal + ) + if (!authoritativeTab) { + throw new Error( + `Paired terminal was absent from authoritative inventory before close: ${JSON.stringify( + authoritativeInventory.tabs.map((candidate) => ({ + id: candidate.id, + parentTabId: candidate.parentTabId, + terminal: candidate.terminal + })) + )}` + ) + } + const closeStartedAt = performance.now() + await callRuntime(client.page, 'session.tabs.close', { + worktree: `id:${worktree.id}`, + tabId: authoritativeTab.id, + reason: 'user', + navigation: 'caller' + }) + const closeLatencyMs = performance.now() - closeStartedAt + expect(closeLatencyMs).toBeLessThan(PUBLICATION_DEADLINE_MS) + await expect + .poll( + () => + Promise.all([ + orcaPage.evaluate( + ({ tabId, worktreeId }) => + (window.__store?.getState().tabsByWorktree[worktreeId] ?? []).some( + (candidate) => candidate.id === tabId + ), + { tabId: created.tab.parentTabId, worktreeId: worktree.id } + ), + client.page.evaluate( + ({ tabId, worktreeId }) => + (window.__store?.getState().tabsByWorktree[worktreeId] ?? []).some( + (candidate) => candidate.id === tabId + ), + { tabId: webTabId, worktreeId: worktree.id } + ), + observer.page.evaluate( + ({ tabId, worktreeId }) => + (window.__store?.getState().tabsByWorktree[worktreeId] ?? []).some( + (candidate) => candidate.id === tabId + ), + { tabId: webTabId, worktreeId: worktree.id } + ) + ]), + { timeout: 30_000 } + ) + .toEqual([false, false, false]) + terminal = null + + await restoreHeadedHost(electronApp, orcaPage) + const restoredHostScreenshot = await orcaPage.screenshot({ fullPage: true }) + expect( + countForegroundPixels(restoredHostScreenshot), + 'host compositor remained blank after the background close toggle' + ).toBeGreaterThan(1_000) + await testInfo.attach('host-window-after-close-restore', { + body: restoredHostScreenshot, + contentType: 'image/png' + }) + + const afterPublicationResources = await getAppResourceProxies(electronApp) + const evidencePath = test.info().outputPath('publication-evidence.json') + writeFileSync( + evidencePath, + `${JSON.stringify( + { + afterPublicationResources, + closeLatencyMs, + connectedIdleResources, + createLatencyMs, + hostMinimizedDuringCreateAndClose: true, + hostRestoredAfterEachPublication: true, + noClientResources + }, + null, + 2 + )}\n` + ) + await test.info().attach('publication-evidence', { + contentType: 'application/json', + path: evidencePath + }) } finally { await client.page .evaluate(() => { @@ -243,6 +629,7 @@ test('restarts one ACK-starved paired terminal stream without replacing its PTY ).__remoteTerminalMultiplexAckGate?.release() }) .catch(() => undefined) + await observer?.dispose() if (terminal) { await callRuntime(client.page, 'terminal.closeTab', { terminal }).catch(() => undefined) }