diff --git a/__tests__/unit/components/ErrorRecoverySheetA11y.test.tsx b/__tests__/unit/components/ErrorRecoverySheetA11y.test.tsx new file mode 100644 index 00000000..8e4509b1 --- /dev/null +++ b/__tests__/unit/components/ErrorRecoverySheetA11y.test.tsx @@ -0,0 +1,39 @@ +import { ErrorRecoverySheet } from '@/components/ui/ErrorRecoverySheet' +import { renderWithI18n } from '@/test-utils/render' + +// @gorhom/bottom-sheet defaults `accessible` to true on the container wrapping +// the sheet's children (DEFAULT_ACCESSIBLE). On iOS an accessible View is a +// single accessibility element and its descendants are hidden, so the whole +// sheet collapsed into one opaque node labelled "Bottom Sheet" — VoiceOver +// could not reach the title, any error row, Retry all or Close, and neither +// could XCUITest. Confirmed on a simulator by pairing to a mock server, killing +// it so the sheet auto-opened, and dumping the accessibility hierarchy. +describe('ErrorRecoverySheet accessibility', () => { + it('opts the sheet container out of being one accessibility element', async () => { + const { getByTestId } = await renderWithI18n( + {}} + />, + ) + + expect(getByTestId('bottom-sheet').props.accessible).toBe(false) + }) + + it('leaves the sheet controls individually reachable', async () => { + const { getByTestId } = await renderWithI18n( + {}} + />, + ) + + getByTestId('error-recovery-sheet') + getByTestId('error-sheet-row-messages') + getByTestId('error-sheet-close') + }) +}) diff --git a/__tests__/unit/hooks/useSessionLeaveGuard.test.tsx b/__tests__/unit/hooks/useSessionLeaveGuard.test.tsx index ed8343c0..a93bd34b 100644 --- a/__tests__/unit/hooks/useSessionLeaveGuard.test.tsx +++ b/__tests__/unit/hooks/useSessionLeaveGuard.test.tsx @@ -21,10 +21,12 @@ const live = { function makeNav() { const dispatch = jest.fn() + const navigateHome = jest.fn() return { navigation: { dispatch, }, + navigateHome, fire: async (type = 'GO_BACK') => { const [preventRemove, callback] = (usePreventRemove as jest.Mock).mock.calls.at(-1) ?? [] const action = { type } @@ -33,7 +35,7 @@ function makeNav() { callback({ data: { action } }) }) } - return { preventRemove, action, dispatch } + return { preventRemove, action, dispatch, navigateHome } }, dispatch, } @@ -43,12 +45,14 @@ type StopSessionMutateAsync = Parameters[0]['stopSe function LeaveGuardProbe({ navigation, + navigateHome, session, isPending, skipInitialReplace, stopSessionMutateAsync, }: { navigation: ReturnType['navigation'] + navigateHome: () => void session: typeof live isPending: boolean skipInitialReplace?: boolean @@ -58,6 +62,7 @@ function LeaveGuardProbe({ const { leaveModalVisible, leavePhase, + isLeaving, cancelLeave, confirmLeave, dismissLeaveError, @@ -65,6 +70,7 @@ function LeaveGuardProbe({ } = useSessionLeaveGuard({ // A fresh object every render, exactly like app/session/[id].tsx passes. navigation: { dispatch: (action) => navigation.dispatch(action) }, + navigateHome, serverId: 'srv1', sessionId: 'sess-live', session, @@ -76,6 +82,7 @@ function LeaveGuardProbe({ {leaveModalVisible ? 'yes' : 'no'} {leavePhase} + {isLeaving ? 'yes' : 'no'} confirmLeave('kill', false)} /> confirmLeave('leave', false)} /> @@ -113,6 +120,7 @@ describe('useSessionLeaveGuard', () => { const view = await render( { } it('Always ask: back from live session shows the modal; Cancel stays', async () => { - const { fire, dispatch } = await setup() + const { fire, navigateHome } = await setup() const { preventRemove } = await fire() expect(preventRemove).toBe(true) expect(screen.getByTestId('leave-modal-visible')).toHaveTextContent('yes') - expect(dispatch).not.toHaveBeenCalled() + expect(navigateHome).not.toHaveBeenCalled() expect(stopSessionMutateAsync).not.toHaveBeenCalled() await fireEvent.press(screen.getByTestId('leave-cancel')) expect(screen.getByTestId('leave-modal-visible')).toHaveTextContent('no') - expect(dispatch).not.toHaveBeenCalled() + expect(navigateHome).not.toHaveBeenCalled() expect(stopSessionMutateAsync).not.toHaveBeenCalled() expect(wsManager.holdSessionWaitingInput).not.toHaveBeenCalled() }) it('on iOS, dispatch waits for the real modal dismiss instead of racing it', async () => { - const { fire, dispatch } = await setup() - const { action } = await fire() + const { fire, navigateHome } = await setup() + await fire() await fireEvent.press(screen.getByTestId('leave-confirm-leave')) // The bug this guards: dispatching while the native is still // mid-close can be silently dropped by iOS, which read as "the first // back press does nothing; a second press then navigates with no modal." - expect(dispatch).not.toHaveBeenCalled() + expect(navigateHome).not.toHaveBeenCalled() const [preventRemoveStillArmed] = (usePreventRemove as jest.Mock).mock.calls.at(-1) expect(preventRemoveStillArmed).toBe(true) await fireModalDismiss() - expect(dispatch).toHaveBeenCalledWith(action) + expect(navigateHome).toHaveBeenCalled() }) it('on iOS, dispatch fires from a bounded fallback if onDismiss never comes', async () => { @@ -166,17 +174,17 @@ describe('useSessionLeaveGuard', () => { // first back press. The fallback must not depend on onDismiss at all. jest.useFakeTimers() try { - const { fire, dispatch } = await setup() - const { action } = await fire() + const { fire, navigateHome } = await setup() + await fire() await act(async () => { fireEvent.press(screen.getByTestId('leave-confirm-leave')) }) - expect(dispatch).not.toHaveBeenCalled() + expect(navigateHome).not.toHaveBeenCalled() await act(async () => { jest.advanceTimersByTime(500) }) - expect(dispatch).toHaveBeenCalledWith(action) + expect(navigateHome).toHaveBeenCalled() } finally { jest.useRealTimers() } @@ -184,65 +192,66 @@ describe('useSessionLeaveGuard', () => { it('on Android, dispatch fires immediately — no onDismiss race to defer for', async () => { Platform.OS = 'android' - const { fire, dispatch } = await setup() - const { action } = await fire() + const { fire, navigateHome } = await setup() + await fire() await fireEvent.press(screen.getByTestId('leave-confirm-leave')) - expect(dispatch).toHaveBeenCalledWith(action) + expect(navigateHome).toHaveBeenCalled() }) it('a back press while awaiting the modal dismiss is swallowed, not re-prompted', async () => { - const { fire, dispatch } = await setup() + const { fire, navigateHome } = await setup() await fire() await fireEvent.press(screen.getByTestId('leave-confirm-leave')) - expect(dispatch).not.toHaveBeenCalled() + expect(navigateHome).not.toHaveBeenCalled() await fire() expect(screen.getByTestId('leave-modal-visible')).toHaveTextContent('no') - expect(dispatch).not.toHaveBeenCalled() + expect(navigateHome).not.toHaveBeenCalled() }) it('Confirm+Kill shows a loader, awaits stop, then navigates once dismissed', async () => { - const { fire, dispatch } = await setup() - const { action } = await fire() + const { fire, navigateHome } = await setup() + await fire() await fireEvent.press(screen.getByTestId('leave-confirm-kill')) expect(wsManager.holdSessionWaitingInput).not.toHaveBeenCalled() - await waitFor(() => expect(screen.getByTestId('leave-phase')).toHaveTextContent('idle')) + await waitFor(() => expect(screen.getByTestId('leave-phase')).toHaveTextContent('navigating')) expect(stopSessionMutateAsync).toHaveBeenCalled() - expect(dispatch).not.toHaveBeenCalled() + expect(navigateHome).not.toHaveBeenCalled() await fireModalDismiss() - expect(dispatch).toHaveBeenCalledWith(action) + expect(navigateHome).toHaveBeenCalled() }) it('Confirm+Leave navigates with no stop/hold, once dismissed', async () => { - const { fire, dispatch } = await setup() + const { fire, navigateHome } = await setup() await fire() await fireEvent.press(screen.getByTestId('leave-confirm-leave')) expect(stopSessionMutateAsync).not.toHaveBeenCalled() expect(wsManager.holdSessionWaitingInput).not.toHaveBeenCalled() await fireModalDismiss() - expect(dispatch).toHaveBeenCalled() + expect(navigateHome).toHaveBeenCalled() }) it('allows the automatic replacement that opens a starting session', async () => { // No leave modal is ever shown on this path, so there is nothing to // dismiss — the effect-driven dispatch stays immediate. - const { fire, dispatch } = await setup(live, { skipInitialReplace: true }) + const { fire, dispatch, navigateHome } = await setup(live, { skipInitialReplace: true }) const { action } = await fire('REPLACE') expect(screen.getByTestId('leave-modal-visible')).toHaveTextContent('no') expect(dispatch).toHaveBeenCalledWith(action) + expect(navigateHome).not.toHaveBeenCalled() }) it('dispatches the continued action once, not on every later render', async () => { - const { fire, dispatch } = await setup() + const { fire, navigateHome } = await setup() await fire() await fireEvent.press(screen.getByTestId('leave-confirm-leave')) await fireModalDismiss() await fireEvent.press(screen.getByTestId('force-rerender')) - expect(dispatch).toHaveBeenCalledTimes(1) + expect(navigateHome).toHaveBeenCalledTimes(1) }) it('turns off removal prevention only once the deferred dispatch actually fires', async () => { @@ -259,27 +268,47 @@ describe('useSessionLeaveGuard', () => { }) it('Confirm+Kill on idle sends when: waiting_input, awaits the ack, then navigates once dismissed', async () => { - const { fire, dispatch } = await setup() - const { action } = await fire() + const { fire, navigateHome } = await setup() + await fire() await fireEvent.press(screen.getByTestId('leave-confirm-idle')) expect(wsManager.holdSessionWaitingInput).toHaveBeenCalledWith('srv1', 'sess-live') - await waitFor(() => expect(screen.getByTestId('leave-phase')).toHaveTextContent('idle')) + await waitFor(() => expect(screen.getByTestId('leave-phase')).toHaveTextContent('navigating')) expect(stopSessionMutateAsync).not.toHaveBeenCalled() - expect(dispatch).not.toHaveBeenCalled() + expect(navigateHome).not.toHaveBeenCalled() await fireModalDismiss() - expect(dispatch).toHaveBeenCalledWith(action) + expect(navigateHome).toHaveBeenCalled() + }) + + // app/session/[id].tsx suppresses its own redirect to /conversation/ + // while `isLeaving` is true. A killed session flips to history the moment + // the stop lands, so any gap here hands that redirect the screen and the + // user ends up on the conversation view instead of the homepage. + it('stays "leaving" for the whole deferred window, so the screen keeps suppressing its redirect', async () => { + const { fire, navigateHome } = await setup() + await fire() + await fireEvent.press(screen.getByTestId('leave-confirm-kill')) + + // The stop has resolved and the modal is closing, but the navigation has + // not been dispatched yet — the exact window the redirect used to win. + await waitFor(() => expect(screen.getByTestId('leave-phase')).toHaveTextContent('navigating')) + expect(navigateHome).not.toHaveBeenCalled() + expect(screen.getByTestId('leave-is-leaving')).toHaveTextContent('yes') + + await fireModalDismiss() + expect(navigateHome).toHaveBeenCalled() + expect(screen.getByTestId('leave-is-leaving')).toHaveTextContent('yes') }) it('kill-on-idle with no ack (old streamer / disconnected) still navigates — degrade, not error', async () => { ;(wsManager.holdSessionWaitingInput as jest.Mock).mockResolvedValue(null) - const { fire, dispatch } = await setup() - const { action } = await fire() + const { fire, navigateHome } = await setup() + await fire() await fireEvent.press(screen.getByTestId('leave-confirm-idle')) - await waitFor(() => expect(screen.getByTestId('leave-phase')).toHaveTextContent('idle')) + await waitFor(() => expect(screen.getByTestId('leave-phase')).toHaveTextContent('navigating')) await fireModalDismiss() - expect(dispatch).toHaveBeenCalledWith(action) + expect(navigateHome).toHaveBeenCalled() }) it('kill-on-idle denied by the streamer shows the error state instead of navigating', async () => { @@ -287,29 +316,29 @@ describe('useSessionLeaveGuard', () => { ok: false, reason: 'permission_denied', }) - const { fire, dispatch } = await setup() + const { fire, navigateHome } = await setup() await fire() await fireEvent.press(screen.getByTestId('leave-confirm-idle')) await waitFor(() => expect(screen.getByTestId('leave-phase')).toHaveTextContent('error')) - expect(dispatch).not.toHaveBeenCalled() + expect(navigateHome).not.toHaveBeenCalled() }) it('a failed Kill it shows the error state; dismissing then pressing back navigates home', async () => { stopSessionMutateAsync.mockRejectedValueOnce(new Error('stop failed')) - const { fire, dispatch } = await setup() - const { action } = await fire() + const { fire, navigateHome } = await setup() + await fire() await fireEvent.press(screen.getByTestId('leave-confirm-kill')) await waitFor(() => expect(screen.getByTestId('leave-phase')).toHaveTextContent('error')) - expect(dispatch).not.toHaveBeenCalled() + expect(navigateHome).not.toHaveBeenCalled() // Acknowledging the error itself never dispatches — the error card has // already been visible (and closing) for a while, so no dismiss race. await fireEvent.press(screen.getByTestId('leave-dismiss-error')) expect(screen.getByTestId('leave-phase')).toHaveTextContent('errorAcked') - expect(dispatch).not.toHaveBeenCalled() + expect(navigateHome).not.toHaveBeenCalled() await fire() - expect(dispatch).toHaveBeenCalledWith(action) + expect(navigateHome).toHaveBeenCalled() expect(screen.getByTestId('leave-phase')).toHaveTextContent('idle') }) @@ -320,21 +349,21 @@ describe('useSessionLeaveGuard', () => { resolveStop = resolve }), ) - const { fire, dispatch } = await setup() + const { fire, navigateHome } = await setup() await fire() await fireEvent.press(screen.getByTestId('leave-confirm-kill')) expect(screen.getByTestId('leave-phase')).toHaveTextContent('pending') await fire() expect(screen.getByTestId('leave-modal-visible')).toHaveTextContent('no') - expect(dispatch).not.toHaveBeenCalled() + expect(navigateHome).not.toHaveBeenCalled() await act(async () => { resolveStop() }) - await waitFor(() => expect(screen.getByTestId('leave-phase')).toHaveTextContent('idle')) + await waitFor(() => expect(screen.getByTestId('leave-phase')).toHaveTextContent('navigating')) await fireModalDismiss() - expect(dispatch).toHaveBeenCalled() + expect(navigateHome).toHaveBeenCalled() }) it('Don’t ask again + Kill it persists the setting; next leave stops with no modal', async () => { @@ -342,9 +371,9 @@ describe('useSessionLeaveGuard', () => { await first.fire() await fireEvent.press(screen.getByTestId('leave-confirm-kill-remember')) expect(useSettingsStore.getState().sessionLeaveAction).toBe('kill') - await waitFor(() => expect(screen.getByTestId('leave-phase')).toHaveTextContent('idle')) + await waitFor(() => expect(screen.getByTestId('leave-phase')).toHaveTextContent('navigating')) await fireModalDismiss() - await waitFor(() => expect(first.dispatch).toHaveBeenCalled()) + await waitFor(() => expect(first.navigateHome).toHaveBeenCalled()) await first.unmount() stopSessionMutateAsync.mockClear() @@ -354,7 +383,7 @@ describe('useSessionLeaveGuard', () => { expect(preventRemove).toBe(true) await waitFor(() => expect(stopSessionMutateAsync).toHaveBeenCalled()) await fireModalDismiss() - await waitFor(() => expect(second.dispatch).toHaveBeenCalled()) + await waitFor(() => expect(second.navigateHome).toHaveBeenCalled()) }) it('Settings Always ask restores the modal', async () => { @@ -391,12 +420,12 @@ describe('useSessionLeaveGuard', () => { }) it('Always ask: empty live session also shows the modal (no auto-stop)', async () => { - const { fire, dispatch } = await setup(live) + const { fire, navigateHome } = await setup(live) const { preventRemove } = await fire() expect(screen.getByTestId('leave-modal-visible')).toHaveTextContent('yes') expect(preventRemove).toBe(true) expect(stopSessionMutateAsync).not.toHaveBeenCalled() - expect(dispatch).not.toHaveBeenCalled() + expect(navigateHome).not.toHaveBeenCalled() }) it('idle / on_hold: no modal', async () => { diff --git a/__tests__/unit/scripts/maestro-flow-env.test.js b/__tests__/unit/scripts/maestro-flow-env.test.js index b752a5d1..12809b9e 100644 --- a/__tests__/unit/scripts/maestro-flow-env.test.js +++ b/__tests__/unit/scripts/maestro-flow-env.test.js @@ -3,10 +3,8 @@ * * Two harness guards that fail silently and expensively when they regress. * - * `run-maestro.js` must pass E2E_MOCK_SERVER_URL through with `-e`: Maestro - * resolves `${VAR}` in a flow only from that flag, never from the environment, - * so without it the app dials the literal host `undefined` and every onboarding - * flow fails on a later, unrelated-looking assertion. + * `run-maestro.js` must pass its flow variables through with `-e`: Maestro + * resolves `${VAR}` in a flow only from that flag, never from the environment. * * `wait-for-mock.js` must fail loudly when nothing is listening, rather than * letting the suite run against a mock server that died during startup. @@ -56,6 +54,27 @@ describe('run-maestro.js flow variables', () => { expect(argv).toContain('-e E2E_MOCK_SERVER_URL=http://10.0.2.2:7071'); }); + it('passes E2E_SERVER_TOKEN to maestro with -e', () => { + const { argv } = runWithStub(['test', 'e2e/launch.yaml'], { + E2E_SERVER_TOKEN: 'real-streamer-key', + }); + expect(argv).toContain('-e E2E_SERVER_TOKEN=real-streamer-key'); + }); + + it('defaults E2E_SERVER_TOKEN to the mock server key', () => { + const { argv } = runWithStub(['test', 'e2e/launch.yaml'], {}); + expect(argv).toContain('-e E2E_SERVER_TOKEN=mock-key-123'); + }); + + it("does not override a caller's own token", () => { + const { argv } = runWithStub( + ['test', '-e', 'E2E_SERVER_TOKEN=explicit-key', 'e2e/launch.yaml'], + { E2E_SERVER_TOKEN: 'environment-key' }, + ); + expect(argv).toContain('E2E_SERVER_TOKEN=explicit-key'); + expect(argv).not.toContain('environment-key'); + }); + it('keeps the subcommand first and the flow paths last', () => { const { argv } = runWithStub(['test', 'e2e/a.yaml', 'e2e/b.yaml'], { E2E_MOCK_SERVER_URL: 'http://localhost:7071', diff --git a/__tests__/unit/scripts/run-maestro.test.js b/__tests__/unit/scripts/run-maestro.test.js index 2caa5678..aadbc45a 100644 --- a/__tests__/unit/scripts/run-maestro.test.js +++ b/__tests__/unit/scripts/run-maestro.test.js @@ -184,15 +184,19 @@ test('caller arguments reach Maestro unmangled, without shell re-parsing', () => expect(result.status).toBe(0); - // run-maestro injects `-e E2E_MOCK_SERVER_URL=...` after the subcommand, so the + // run-maestro injects its default flow variables after the subcommand, so the // received list is not identical to the caller's. What this test is about is // that nothing is re-parsed by a shell: every argument the caller passed must - // arrive verbatim and in order around that injection. + // arrive verbatim and in order around those injections. const received = JSON.parse(fs.readFileSync(argsPath, 'utf8')); expect(received[0]).toBe('test'); - expect(received[1]).toBe('-e'); - expect(received[2]).toMatch(/^E2E_MOCK_SERVER_URL=/); - expect(received.slice(3)).toEqual(args.slice(1)); + expect(received.slice(1, 5)).toEqual([ + '-e', + 'E2E_SERVER_TOKEN=mock-key-123', + '-e', + 'E2E_MOCK_SERVER_URL=http://localhost:7071', + ]); + expect(received.slice(5)).toEqual(args.slice(1)); expect(fs.existsSync(path.join(fixture.root, 'nope'))).toBe(false); }); diff --git a/__tests__/unit/services/query-client.test.ts b/__tests__/unit/services/query-client.test.ts index 5d804f0a..21f8b1b7 100644 --- a/__tests__/unit/services/query-client.test.ts +++ b/__tests__/unit/services/query-client.test.ts @@ -182,4 +182,25 @@ describe('error banner opt-out (meta.silentError)', () => { expect(row.status).toBe(404) expect(row.code).toBe('HTTP_404') }) + + // "Kill it" makes the session-detail query 404 by construction, so the sheet + // reported the user's own action back to them as a failure. + it('pushes no row for a 404 when the query opts out of just that', async () => { + await failWith(['session', 'srv1', 'killed'], { persist: false, silentNotFound: true }) + expect(useLoadingStateStore.getState().errors).toHaveLength(0) + }) + + it('still pushes a row for a non-404 when the query only opts out of not-found', async () => { + await queryClient + .fetchQuery({ + queryKey: ['session', 'srv1', 'flaky'], + queryFn: () => Promise.reject(Object.assign(new Error('boom'), { status: 503 })), + retry: false, + meta: { persist: false, silentNotFound: true }, + }) + .catch(() => {}) + jest.advanceTimersByTime(0) + expect(useLoadingStateStore.getState().errors).toHaveLength(1) + expect(useLoadingStateStore.getState().errors[0].status).toBe(503) + }) }) diff --git a/app/session/[id].tsx b/app/session/[id].tsx index bfa5ea83..d65ae1fc 100644 --- a/app/session/[id].tsx +++ b/app/session/[id].tsx @@ -1,4 +1,4 @@ -import React, { useState, useEffect, useMemo, useRef } from 'react' +import React, { useCallback, useState, useEffect, useMemo, useRef } from 'react' import { View, Text, @@ -432,6 +432,11 @@ export default function SessionDetailScreen() { }>() const router = useRouter() const navigation = useNavigation() + // Leaving a session always lands on the homepage, never on whatever screen + // happens to sit under this one — and it must go through the router rather + // than a replayed navigation action; see LeaveContinuation in + // hooks/useSessionLeaveGuard.ts. + const navigateHome = useCallback(() => router.replace('/'), [router]) // Fall back to first server if no server param provided (backwards compat) const fallbackServerId = useServersStore((s) => s.activeServerIds[0] ?? '') @@ -564,6 +569,7 @@ export default function SessionDetailScreen() { navigation.dispatch(action as Parameters[0]) }, }, + navigateHome, serverId, sessionId: id, session, @@ -960,9 +966,7 @@ export default function SessionDetailScreen() { ) - const handleBack = () => { - router.replace('/') - } + const handleBack = navigateHome const presentation = deriveSessionPresentation(session) const capabilityLabel = presentation.capabilities.isObserveOnly diff --git a/components/ui/ErrorRecoverySheet.stories.tsx b/components/ui/ErrorRecoverySheet.stories.tsx new file mode 100644 index 00000000..4ef943f3 --- /dev/null +++ b/components/ui/ErrorRecoverySheet.stories.tsx @@ -0,0 +1,32 @@ +import type { Meta, StoryObj } from '@storybook/react-native-web-vite' +import { fn } from 'storybook/test' +import { ErrorRecoverySheet } from './ErrorRecoverySheet' + +const meta: Meta = { + title: 'ui/ErrorRecoverySheet', + component: ErrorRecoverySheet, + args: { + visible: true, + title: 'Some requests failed', + items: [ + { + id: 'messages', + title: 'Messages failed to load', + message: 'The server did not respond. Try again when it is reachable.', + }, + ], + onClose: fn(), + }, +} + +export default meta +type Story = StoryObj + +export const Default: Story = {} + +export const WithRetry: Story = { + args: { + retryAllLabel: 'Retry all', + onRetryAll: fn(), + }, +} diff --git a/components/ui/ErrorRecoverySheet.tsx b/components/ui/ErrorRecoverySheet.tsx index 6b3ffb58..b4fce275 100644 --- a/components/ui/ErrorRecoverySheet.tsx +++ b/components/ui/ErrorRecoverySheet.tsx @@ -138,11 +138,23 @@ export function ErrorRecoverySheet({ visible, title, items, retryAllLabel, retry backgroundStyle={[s.sheetBg, isGlass && s.sheetBgGlass]} backgroundComponent={glassBackground} handleIndicatorStyle={s.handle} - accessibilityLabel={title} - accessibilityLiveRegion="assertive" + // BottomSheet defaults `accessible` to true (DEFAULT_ACCESSIBLE) and puts + // it on the container that wraps these children. On iOS an accessible + // View is a single accessibility element and its descendants are hidden, + // so the title, every error row, Retry all and Close collapsed into one + // opaque node labelled "Bottom Sheet": VoiceOver could not reach a single + // control in this sheet, and neither could XCUITest. + accessible={false} > + {/* + Nor an accessibilityLabel on — it re-labels that same + container. The sheet already announces itself explicitly through + announceForAccessibility above, and accessibilityLiveRegion is + Android-only, so it belongs on the content view where it costs + nothing on iOS. + */} - + {title} {items.map((item) => )} {retryAllLabel && onRetryAll ? ( diff --git a/docs/research/2026-09-09-real-streamer-ci-e2e.md b/docs/research/2026-09-09-real-streamer-ci-e2e.md new file mode 100644 index 00000000..3a70d138 --- /dev/null +++ b/docs/research/2026-09-09-real-streamer-ci-e2e.md @@ -0,0 +1,183 @@ +# Real Streamer E2E in CI + +**Status:** Proposed + +**Date:** 2026-09-09 + +**Scope:** Run the leave-session navigation matrix against a real `tb-streamer` backend in GitHub Actions without invoking a hosted AI model. + +## Goal + +Exercise the mobile app, HTTP and WebSocket transports, streamer session lifecycle, PTY manager, and navigation behavior in one CI test. + +The provider process should be deterministic and contain no Anthropic credential. + +This test complements the existing mock-server suite. +It does not replace production validation against an installed Claude Code or Codex CLI. + +## Reference: AutoKitteh Web Platform + +The archived AutoKitteh web-platform workflow demonstrates the relevant service-container pattern. +Each browser job runs on an ephemeral `ubuntu-latest` runner and performs these steps: + +1. Restore or pull an AutoKitteh server image. +2. Start the image with `docker run -d` and publish the real backend on runner port 9980. +3. Configure authentication, CORS, and initial database records through environment variables and the backend's startup command. +4. Follow the container logs into a job artifact. +5. Poll the backend until it responds or a five-minute timeout expires. +6. Build and serve the frontend on port 8000. +7. Run Playwright with one worker against the frontend and localhost backend. +8. Let the ephemeral runner dispose of the container and database after the job. + +The workflow and composite action are pinned here: + +- [Test job](https://github.com/autokitteh/web-platform/blob/95d9899f9847abe6fba5a10311541dbd3e0bedc5/.github/workflows/build_test_and_release.yml#L66-L147) +- [Test-environment action](https://github.com/autokitteh/web-platform/blob/95d9899f9847abe6fba5a10311541dbd3e0bedc5/.github/actions/setup-test-env/action.yml) +- [Playwright frontend startup and authentication](https://github.com/autokitteh/web-platform/blob/95d9899f9847abe6fba5a10311541dbd3e0bedc5/playwright.config.ts#L9-L16) + +The backend is real application code, not an HTTP fixture server. +The environment around it is controlled: the database is ephemeral and seeded, the user identity is fixed, and the browser sends a CI JWT with every request. + +The reference workflow has one reproducibility weakness that Threadbase should avoid. +Its repository variable currently names `public.ecr.aws/autokitteh/server:latest`, and its Docker cache key hashes the image name rather than the resolved digest. +A cache hit can therefore select an older image while still reporting the `latest` tag. + +## Existing Threadbase Building Blocks + +`tb-streamer` already contains the backend environment needed for this approach. + +Its `docker/Dockerfile` has a `demo` target that builds the real streamer and installs a deterministic executable as `/usr/local/bin/claude`. +The streamer still creates and controls a real child process through `node-pty`; only the provider executable is substituted. + +The stub: + +- prints the Claude welcome frame and ready prompt marker; +- remains alive while attached to the PTY; +- accepts terminal input and emits scripted output; +- exits on `SIGTERM` or `SIGINT`; and +- performs no network model request. + +The demo entrypoint also provides: + +- an isolated writable home at `/data`; +- a fixed, non-sensitive API key; +- seeded conversation history; +- real project directories beneath `/home/demo/projects`; +- a browse root restricted to those project directories; and +- an HTTP health check. + +The relevant streamer files are: + +- `tb-streamer/docker/Dockerfile` +- `tb-streamer/docker/entrypoint.sh` +- `tb-streamer/docker/claude-code-stub/claude.js` +- `tb-streamer/docker-compose.yml` + +The resulting boundary is suitable for the leave-session matrix: + +| Layer | Test implementation | +|---|---| +| Mobile UI and navigation | Real release app driven by Maestro | +| Pairing and authentication | Real streamer endpoints with a deterministic API key | +| Session list and detail | Real streamer REST and WebSocket state | +| Start, leave, stop, and hold | Real streamer handlers and lifecycle policy | +| PTY management | Real `node-pty` child process | +| Provider | Deterministic Claude stand-in | +| AI model and billing | Not present | + +## Recommended First CI Target + +Run the matrix in the existing Android E2E job on `ubuntu-24.04`. +Linux GitHub-hosted runners support Docker and the Android emulator already used by the repository. + +The first implementation should use a streamer checkout pinned to an explicit commit and build its `demo` Docker target. +Pinning the source commit makes the tested compatibility boundary visible and avoids mutable image tags. + +The job should perform this sequence: + +1. Check out `threadbase-mobile`. +2. Check out `threadbase-streamer` at the selected compatibility commit into a separate path. +3. Build the streamer's `demo` Docker target with BuildKit layer caching. +4. Start a uniquely named container with an ephemeral data volume and publish container port 8080 on runner port 8766. +5. Poll `GET /healthz`, then make an authenticated `GET /api/info` request. +6. Build or restore the mobile Release APK. +7. Boot the Android API 35 emulator and install the APK. +8. Run the six-case leave-session matrix. +9. Upload streamer logs and Maestro artifacts when the test fails. +10. Stop the container and remove its test volume in an `always()` step. + +The runner-side controller should call `http://127.0.0.1:8766`. +The app inside the Android emulator should call `http://10.0.2.2:8766`. + +## Required Harness Changes + +The current `e2e/run-leave-nav.js` assumes that the Node controller and mobile app use the same server URL. +That works on an iOS simulator because both use `localhost`, but it does not work on Android. + +Split the address into two inputs: + +| Input | Android CI value | Purpose | +|---|---|---| +| `REAL_STREAMER_CONTROL_URL` | `http://127.0.0.1:8766` | Node runner health, start, list, and stop requests | +| `REAL_STREAMER_APP_URL` | `http://10.0.2.2:8766` | URL paired inside the Android emulator | + +The current default session path is the host mobile worktree. +That path does not exist inside the container and is outside the demo image's browse root. +CI should instead use a known container path such as `/home/demo/projects/threadbase-mobile`. + +The following changes are required before enabling the job: + +1. Accept separate controller and app URLs. +2. Accept an explicit server-side session path. +3. Replace the iOS-only preflight in `test:e2e:leave-nav` with platform-specific preflights or invoke the platform-neutral runner directly from Android CI. +4. Track every session created by the current test invocation. +5. Stop only those tracked sessions during cleanup. +6. If `POST /api/sessions/start` returns `202`, poll the returned session until it becomes ready or fails instead of treating the pending response as an immediate failure. +7. Write the container ID, selected streamer commit, image ID, and health response to the job summary. + +The cleanup correction is required before CI adoption. +The current runner lists all sessions with `ptyAttached` and stops all of them. +That is acceptable only on a disposable isolated backend, but tracking owned IDs prevents the same helper from terminating unrelated local sessions when developers run it manually. + +## iOS CI + +The same Docker procedure is not directly available in the repository's `macos-26` GitHub-hosted job. +The initial CI gate should therefore use Android. + +An iOS version can follow by checking out and building `tb-streamer` natively on the macOS runner, placing the existing Claude stub first on `PATH`, and starting the streamer with isolated `HOME` and configuration directories. +That path needs its same source pin, readiness probes, owned-session cleanup, log capture, and process teardown as the Docker job. + +Running the test against the public demo server is useful as a separate availability smoke test. +It is a weaker pull-request gate because the deployed backend can change independently, network availability affects the result, and concurrent test runs share state. + +## Scope of Confidence + +This CI test would prove that: + +- the app can pair with and authenticate to a real streamer; +- new and already-running PTY sessions appear through the real API and WebSocket paths; +- each leave-modal action reaches its real streamer handler; +- stop and hold acknowledgements complete without a mock response; +- the session-detail screen yields to the hub without the ended-session redirect winning the race; and +- the real streamer and app contracts remain compatible at the pinned revisions. + +It would not prove that: + +- an installed Claude Code or Codex release accepts the streamer's exact command-line arguments; +- provider authentication and model calls work; +- provider output detection remains correct across provider releases; +- remote tunnels, production TLS, or non-loopback networking work; or +- iOS-specific navigation and accessibility behavior match Android. + +Those boundaries remain covered by local real-provider testing, production/demo smoke tests, and the existing iOS Maestro suite. + +## Rollout + +1. Make the leave-session runner platform-neutral and ownership-safe. +2. Add an opt-in Android workflow dispatch using the pinned streamer checkout. +3. Run the job repeatedly to establish build time and flake rate. +4. Add Docker and Gradle caching without introducing a mutable backend tag. +5. Make the job required after it is stable. +6. Add the native macOS streamer setup if iOS coverage justifies its maintenance cost. + +The Android job is the smallest reliable first step because it combines the repository's existing emulator workflow with the streamer's existing deterministic container target. diff --git a/e2e/leave_session_nav.yaml b/e2e/leave_session_nav.yaml new file mode 100644 index 00000000..7876884b --- /dev/null +++ b/e2e/leave_session_nav.yaml @@ -0,0 +1,153 @@ +# Maestro E2E — the leave-session modal must land on the app homepage +# +# Pressing back on a live session opens the leave-session modal. Whichever of +# the three options is confirmed ("Kill it" / "Leave it" / "Kill on idle"), +# once the action has been applied the app has to be on the homepage (the hub / +# classic / tree list at `/`) — not still on the session screen, and not on the +# conversation screen it redirects to when a session stops being live. +# +# Parameterised with `-e`: +# LEAVE_OPTION kill | leave | kill_on_idle +# SESSION_MODE new — a session this flow starts through Browse +# resumed — a session that already existed on the +# streamer, opened from the hub +# EXISTING_SESSION_ID the pre-created session id (SESSION_MODE=resumed only) +# +# Requires a real threadbase-streamer (`npm run dev:verbose` in tb-streamer) +# reachable at E2E_MOCK_SERVER_URL, whose api_key is E2E_SERVER_TOKEN — the +# mock server has no PTY, so it cannot produce the live session this modal +# guards. +# Usage: node e2e/run-leave-nav.js + +appId: com.ronenmars.threadbase +--- +- runFlow: setup.yaml + +- assertVisible: + id: "hub-screen" + +# --- reach a live session ------------------------------------------------- +- runFlow: + when: + true: ${SESSION_MODE == 'new'} + commands: + - tapOn: + id: "fab-new-session" + retryTapIfNoChange: true + + # One paired server goes straight to Browse; more show the picker. + - runFlow: + when: + visible: + id: "new-session-server-0" + commands: + - tapOn: + id: "new-session-server-0" + + - extendedWaitUntil: + visible: + id: "browse-screen" + timeout: 15000 + + - tapOn: + id: "browse-start-session" + + # Spawning a real agent PTY is far slower than the mock server. + - extendedWaitUntil: + visible: + id: "session-detail-screen" + timeout: 120000 + +- runFlow: + when: + true: ${SESSION_MODE == 'resumed'} + commands: + # The classic list collapses the live-sessions block once a server has + # more than three sessions (app/index.tsx SESSIONS_COLLAPSE_THRESHOLD), + # and a real streamer with history is always over it — so no session row + # renders at all until the eyebrow is expanded. + - extendedWaitUntil: + visible: + id: "live-sessions-header" + timeout: 45000 + + - runFlow: + when: + notVisible: + id: "session-row-.*" + commands: + - tapOn: + id: "live-sessions-header" + + # The classic list merges sessions with a long conversation history, so a + # scroll that starts against a list with no session rows runs straight + # past the row it is looking for and reports it missing. + - extendedWaitUntil: + visible: + id: "session-row-.*" + timeout: 30000 + + - scrollUntilVisible: + element: + id: "session-row-${EXISTING_SESSION_ID}" + direction: DOWN + visibilityPercentage: 30 + timeout: 30000 + + - tapOn: + id: "session-row-${EXISTING_SESSION_ID}" + + - extendedWaitUntil: + visible: + id: "session-detail-screen" + timeout: 60000 + +- takeScreenshot: e2e/_artifacts/screenshots/leave-nav-${SESSION_MODE}-${LEAVE_OPTION}-01-session + +# --- leave the session ---------------------------------------------------- +- tapOn: + id: "screen-header-back-button" + +- extendedWaitUntil: + visible: + id: "leave-session-modal" + timeout: 10000 + +- tapOn: + id: "leave-session-option-${LEAVE_OPTION}" + +- tapOn: + id: "leave-session-confirm" + +# A real streamer produces incidental failures a mock server never does (a slow +# response, a conversation whose history has been pruned), and any error +# auto-opens the recovery sheet over the hub. Close it rather than asserting +# through it — the sheet covers the hub and takes it out of the hierarchy. +- extendedWaitUntil: + visible: + id: "error-sheet-close" + timeout: 5000 + optional: true + +- runFlow: + when: + visible: + id: "error-sheet-close" + commands: + - tapOn: + id: "error-sheet-close" + - waitForAnimationToEnd + +# The whole point of the flow: whatever the action was, we end up home. +- extendedWaitUntil: + visible: + id: "hub-screen" + timeout: 20000 + +- takeScreenshot: e2e/_artifacts/screenshots/leave-nav-${SESSION_MODE}-${LEAVE_OPTION}-02-after + +- assertVisible: + id: "fab-new-session" + +- assertNotVisible: + id: "session-detail-screen" diff --git a/e2e/run-leave-nav.js b/e2e/run-leave-nav.js new file mode 100644 index 00000000..c469f371 --- /dev/null +++ b/e2e/run-leave-nav.js @@ -0,0 +1,152 @@ +#!/usr/bin/env node +'use strict' + +// Runs e2e/leave_session_nav.yaml once per (session mode × leave option) +// combination against a REAL threadbase-streamer — the mock server has no PTY, +// so it cannot produce the live session the leave-session modal guards. +// +// Start the streamer first: cd ../tb-streamer && npm run dev:verbose +// +// Overridable: E2E_MOCK_SERVER_URL (default http://localhost:8766) and +// E2E_SERVER_TOKEN (default: the api_key in ~/.threadbase/server.yaml). +// +// Args narrow the matrix: `node e2e/run-leave-nav.js kill` or `... new/kill`. + +const { spawnSync } = require('child_process') +const fs = require('fs') +const os = require('os') +const path = require('path') + +const OPTIONS = ['kill', 'leave', 'kill_on_idle'] +const MODES = ['new', 'resumed'] +const REPO_ROOT = path.join(__dirname, '..') +// Every session this script spawns is killed again at the end of its combo, so +// the project only has to be a real directory the streamer is allowed to open. +const SESSION_PATH = REPO_ROOT + +function streamerToken() { + if (process.env.E2E_SERVER_TOKEN) return process.env.E2E_SERVER_TOKEN + const yaml = path.join(os.homedir(), '.threadbase/server.yaml') + const match = fs.existsSync(yaml) && /^api_key:\s*(\S+)/m.exec(fs.readFileSync(yaml, 'utf8')) + if (!match) { + console.error('No E2E_SERVER_TOKEN set and no api_key found in ~/.threadbase/server.yaml.') + process.exit(1) + } + return match[1] +} + +function streamer(url, token) { + // The streamer closes some responses (the NDJSON stop stream) as soon as it + // is done writing, which surfaces here as `fetch failed / write EPIPE`. That + // is a transport artefact of a request that did its job, so it must not take + // the run down — every caller below treats `ok: false` as "nothing to do". + const call = async (method, apiPath, body) => { + try { + return await request(method, apiPath, body) + } catch (err) { + return { ok: false, status: 0, text: String(err) } + } + } + const request = async (method, apiPath, body) => { + const res = await fetch(`${url}${apiPath}`, { + method, + headers: { + Authorization: `Bearer ${token}`, + ...(body ? { 'Content-Type': 'application/json' } : {}), + }, + ...(body ? { body: JSON.stringify(body) } : {}), + }) + const text = await res.text() + return { ok: res.ok, status: res.status, text } + } + return { + info: () => call('GET', '/api/info'), + // The 200 shape is `{ session }`; a slow spawn answers 202 `{ id, status: + // 'pending' }`, which is not usable as a row id — treat it as a failure + // rather than tapping a row that does not exist yet. + start: async () => { + const res = await request('POST', '/api/sessions/start', { + path: SESSION_PATH, + projectName: path.basename(SESSION_PATH), + }) + if (!res.ok) throw new Error(`start failed: ${res.status} ${res.text.slice(0, 200)}`) + const parsed = JSON.parse(res.text) + const id = parsed?.session?.id + if (!id) throw new Error(`start did not return a ready session: ${res.text.slice(0, 200)}`) + return id + }, + stop: (id) => call('POST', `/api/sessions/${encodeURIComponent(id)}/stop`), + live: async () => { + const res = await call('GET', '/api/sessions') + if (!res.ok) return [] + return JSON.parse(res.text).filter((s) => s.ptyAttached) + }, + } +} + +function runFlow(env) { + const args = ['test'] + for (const [key, value] of Object.entries(env)) args.push('-e', `${key}=${value}`) + args.push('--debug-output', 'e2e/_artifacts/debug', 'e2e/leave_session_nav.yaml') + return spawnSync(process.execPath, [path.join(__dirname, 'run-maestro.js'), ...args], { + stdio: 'inherit', + cwd: REPO_ROOT, + }).status +} + +async function main() { + const url = process.env.E2E_MOCK_SERVER_URL || 'http://localhost:8766' + const token = streamerToken() + const api = streamer(url, token) + + const probe = await api.info().catch((err) => ({ ok: false, status: err.message })) + if (!probe.ok) { + console.error(`Streamer at ${url} did not answer GET /api/info (${probe.status}).`) + console.error('Start it with `npm run dev:verbose` in tb-streamer.') + process.exit(1) + } + + const only = process.argv.slice(2) + const combos = [] + for (const mode of MODES) { + for (const option of OPTIONS) { + const name = `${mode}/${option}` + if (only.length === 0 || only.includes(name) || only.includes(option) || only.includes(mode)) { + combos.push({ mode, option, name }) + } + } + } + + const results = [] + for (const combo of combos) { + console.log(`\n=== leave_session_nav: ${combo.name} ===`) + let existingId = '' + if (combo.mode === 'resumed') existingId = await api.start() + try { + results.push({ + ...combo, + code: runFlow({ + E2E_MOCK_SERVER_URL: url, + E2E_SERVER_TOKEN: token, + LEAVE_OPTION: combo.option, + SESSION_MODE: combo.mode, + EXISTING_SESSION_ID: existingId, + }), + }) + } finally { + // "Leave it" and "Kill on idle" deliberately keep the PTY alive, and a + // failed flow can strand one at any point — so never let a combo hand + // the next one a machine full of live agents. + for (const session of await api.live()) await api.stop(session.id) + } + } + + console.log('\n=== summary ===') + for (const r of results) console.log(`${r.code === 0 ? 'PASS' : 'FAIL'} ${r.name}`) + process.exit(results.some((r) => r.code !== 0) ? 1 : 0) +} + +main().catch((err) => { + console.error(err) + process.exit(1) +}) diff --git a/e2e/run-maestro.js b/e2e/run-maestro.js index 47d351a9..25235d39 100644 --- a/e2e/run-maestro.js +++ b/e2e/run-maestro.js @@ -257,15 +257,25 @@ async function copyReports(reports, artifactDirectory, warnOnce) { // `test:e2e:mock`, `e2e/run-android-ci.sh` and the iOS scripts at once. A caller // that passes its own `-e E2E_MOCK_SERVER_URL=` wins; the demo and prod scripts // pass unrelated variables and are untouched. -function withMockServerUrl(args) { +function withDefaultEnv(args, name, fallback) { const alreadySet = args.some( - (arg, i) => arg === '-e' && String(args[i + 1] || '').startsWith('E2E_MOCK_SERVER_URL='), + (arg, i) => arg === '-e' && String(args[i + 1] || '').startsWith(`${name}=`), ) if (alreadySet || args.length === 0) return args - const url = process.env.E2E_MOCK_SERVER_URL || 'http://localhost:7071' // After the subcommand (`test`, `record`), before the flow paths. - return [args[0], '-e', `E2E_MOCK_SERVER_URL=${url}`, ...args.slice(1)] + return [args[0], '-e', `${name}=${process.env[name] || fallback}`, ...args.slice(1)] +} + +// `E2E_SERVER_TOKEN` defaults to the mock server's key so every existing flow +// keeps pairing exactly as before; only a caller aiming at a real streamer +// (e2e/run-leave-nav.js) has to supply one. +function withFlowEnv(args) { + return withDefaultEnv( + withDefaultEnv(args, 'E2E_MOCK_SERVER_URL', 'http://localhost:7071'), + 'E2E_SERVER_TOKEN', + 'mock-key-123', + ) } function withRequestedUdid(args) { @@ -278,7 +288,7 @@ function withRequestedUdid(args) { function runMaestro(args) { return new Promise((resolve) => { const command = process.env.MAESTRO_BIN || 'maestro' - const child = spawn(command, withRequestedUdid(withMockServerUrl(args)), { stdio: 'inherit', shell: false }) + const child = spawn(command, withRequestedUdid(withFlowEnv(args)), { stdio: 'inherit', shell: false }) let settled = false let forwardedSignal = null const signalHandlers = new Map() diff --git a/e2e/setup.yaml b/e2e/setup.yaml index 5c6d15cc..89d0b76c 100644 --- a/e2e/setup.yaml +++ b/e2e/setup.yaml @@ -110,7 +110,7 @@ appId: com.ronenmars.threadbase - tapOn: id: "onboarding-connect-token-input" - - inputText: "mock-key-123" + - inputText: "${E2E_SERVER_TOKEN}" - pressKey: Enter diff --git a/hooks/useSession.ts b/hooks/useSession.ts index 754bbbfb..805ba3be 100644 --- a/hooks/useSession.ts +++ b/hooks/useSession.ts @@ -339,7 +339,13 @@ export function useSessionDetail(serverId: string, sessionId: string) { }, // Don't persist session detail across app restarts — each session is // ephemeral and stale persisted state causes false status flickers. - meta: { persist: false }, + // + // silentNotFound: a 404 here is authoritative and usually something the + // user just asked for — "Kill it" makes this query 404 by construction. + // The session screen already renders its own not-found state with a way + // back, so the global recovery sheet only adds a row reporting the user's + // own action as a failure. Other statuses still surface normally. + meta: { persist: false, silentNotFound: true }, // A vanished session is authoritative — retrying a 404 only delays the // not-found recovery UI and keeps stale favorites pinned longer. retry: false, diff --git a/hooks/useSessionLeaveGuard.ts b/hooks/useSessionLeaveGuard.ts index 2cf5fded..91c9ca69 100644 --- a/hooks/useSessionLeaveGuard.ts +++ b/hooks/useSessionLeaveGuard.ts @@ -17,7 +17,27 @@ export interface SessionLeaveNavigation { dispatch: (action: { type: string }) => void } -export type SessionLeavePhase = 'idle' | 'pending' | 'error' | 'errorAcked' +// What to do once the screen is allowed to be removed. 'home' is every path +// the user reached by choosing to leave; 'action' replays a navigation this +// guard intercepted but has no opinion about (the automatic replacement that +// swaps a starting session's placeholder id for its real one). +// +// These cannot be one mechanism. `router.replace('/')` produces a REPLACE +// aimed at the ROOT stack and carrying no `source`, so the router falls back +// to that stack's focused route. Re-dispatching it through this screen's +// navigation stamps `source` with the session route's key — a key the root +// stack has never heard of — and StackRouter answers null: the action is +// dropped in silence, and every leave option left the user on the session +// screen. Going home has to go through the router, not through a replay. +type LeaveContinuation = { kind: 'home' } | { kind: 'action'; action: { type: string } } + +// 'navigating' spans the gap between a successful leave action and the +// navigation it queued actually being dispatched — on iOS that gap is a real +// wait (the native modal's dismiss, or the bounded fallback below). The screen +// reads `isLeaving` to suppress its own history redirect, and treating that gap +// as 'idle' is what let a killed session redirect to /conversation/ before +// the guard's own navigation home landed. +export type SessionLeavePhase = 'idle' | 'pending' | 'navigating' | 'error' | 'errorAcked' function readLeaveSetting(): unknown { const store = useSettingsStore as typeof useSettingsStore & { @@ -47,6 +67,7 @@ async function sendHoldSession(serverId: string, sessionId: string): Promise void serverId: string sessionId: string | undefined session: LeaveSessionSnapshot | null | undefined @@ -64,6 +85,7 @@ export function useSessionLeaveGuard(opts: { } { const { navigation, + navigateHome, serverId, sessionId, session, @@ -73,8 +95,7 @@ export function useSessionLeaveGuard(opts: { } = opts const [leaveModalVisible, setLeaveModalVisible] = useState(false) const [leavePhase, setLeavePhase] = useState('idle') - const [continueAction, setContinueAction] = useState<{ type: string } | null>(null) - const pendingActionRef = useRef<{ type: string } | null>(null) + const [continuation, setContinuation] = useState(null) // One-shot, armed at mount and disarmed by the first REPLACE — not on a timer: // the automatic replacement lands whenever session_ready arrives, which can be // long after the screen mounted. @@ -86,11 +107,12 @@ export function useSessionLeaveGuard(opts: { // finishLeave for why this needs to survive past the state flip that // starts the modal's close animation. const modalIsShowingRef = useRef(false) - const pendingContinueRef = useRef<{ type: string } | null>(null) + const pendingHomeRef = useRef(false) const dismissFallbackRef = useRef | null>(null) const sessionRef = useRef(session) const stopRef = useRef(stopSessionMutateAsync) const navRef = useRef(navigation) + const homeRef = useRef(navigateHome) useEffect(() => { sessionRef.current = session }, [session]) @@ -100,17 +122,17 @@ export function useSessionLeaveGuard(opts: { useEffect(() => { navRef.current = navigation }, [navigation]) + useEffect(() => { + homeRef.current = navigateHome + }, [navigateHome]) useEffect(() => { leavePhaseRef.current = leavePhase }, [leavePhase]) - const proceed = useCallback( - (action: { type: string } | null) => { - if (!action) return - setContinueAction(action) - }, - [], - ) + const proceed = useCallback((next: LeaveContinuation) => { + setLeavePhase('idle') + setContinuation(next) + }, []) // navigation.dispatch() fired while the native is still mid-dismiss // can be silently dropped by iOS UIKit — the app looked like it needed a @@ -133,39 +155,35 @@ export function useSessionLeaveGuard(opts: { } }, []) - const finishLeave = useCallback( - (action: { type: string } | null) => { - if (!action) return - if (Platform.OS === 'ios' && modalIsShowingRef.current) { - pendingContinueRef.current = action - clearDismissFallback() - dismissFallbackRef.current = setTimeout(() => { - dismissFallbackRef.current = null - if (pendingContinueRef.current !== action) return - pendingContinueRef.current = null - modalIsShowingRef.current = false - proceed(action) - }, DISMISS_FALLBACK_MS) - return - } - modalIsShowingRef.current = false - proceed(action) - }, - [proceed, clearDismissFallback], - ) + const finishLeave = useCallback(() => { + if (Platform.OS === 'ios' && modalIsShowingRef.current) { + pendingHomeRef.current = true + clearDismissFallback() + dismissFallbackRef.current = setTimeout(() => { + dismissFallbackRef.current = null + if (!pendingHomeRef.current) return + pendingHomeRef.current = false + modalIsShowingRef.current = false + proceed({ kind: 'home' }) + }, DISMISS_FALLBACK_MS) + return + } + modalIsShowingRef.current = false + proceed({ kind: 'home' }) + }, [proceed, clearDismissFallback]) const onModalDismiss = useCallback(() => { clearDismissFallback() modalIsShowingRef.current = false - const action = pendingContinueRef.current - pendingContinueRef.current = null - if (action) proceed(action) + if (!pendingHomeRef.current) return + pendingHomeRef.current = false + proceed({ kind: 'home' }) }, [proceed, clearDismissFallback]) useEffect(() => clearDismissFallback, [clearDismissFallback]) const runLeaveAction = useCallback( - async (choice: AppliedSessionLeaveAction, action: { type: string } | null) => { + async (choice: AppliedSessionLeaveAction) => { if (!sessionId) return // No special-case shortcut for 'leave': it must go through this same // await (applySessionLeaveAction resolves it instantly with no server @@ -185,8 +203,8 @@ export function useSessionLeaveGuard(opts: { sendHold: () => sendHoldSession(serverId, sessionId), }) if (outcome.ok) { - setLeavePhase('idle') - finishLeave(action) + setLeavePhase('navigating') + finishLeave() return } clientLog.info('session', 'leave action failed', { sessionId, serverId, applied: outcome.applied }) @@ -196,7 +214,7 @@ export function useSessionLeaveGuard(opts: { ) const shouldPreventRemove = - !continueAction && + !continuation && (leavePhase !== 'idle' || (!isPending && Boolean(sessionId) && isLiveAttachedPty(session))) usePreventRemove(shouldPreventRemove, ({ data }) => { if (!sessionId) return @@ -205,27 +223,25 @@ export function useSessionLeaveGuard(opts: { // press (or swipe) is the "now take me home" the spec asks for — skip // the leave-options modal entirely, this choice was already made. if (leavePhaseRef.current === 'errorAcked') { - setLeavePhase('idle') - proceed(pendingActionRef.current ?? data.action) - pendingActionRef.current = null + proceed({ kind: 'home' }) return } // Still sending the action, or the error hasn't been acknowledged yet: // swallow the back press rather than re-opening the modal underneath it. if (leavePhaseRef.current !== 'idle') return - // Confirmed and waiting on the native modal's real dismiss (finishLeave) - // — `leavePhase` is already back to 'idle' for a plain `leave` by this - // point, so without this check a stray back press here would re-run - // decideSessionLeave and could re-open the modal underneath the one - // still animating out. - if (pendingContinueRef.current) return + // Confirmed and waiting on the native modal's real dismiss (finishLeave). + // The 'navigating' phase already swallows this one press above; the ref is + // the belt-and-braces version, because a stray back press here would re-run + // decideSessionLeave and could re-open the modal underneath the one still + // animating out. + if (pendingHomeRef.current) return if (modalVisibleRef.current) return if (skipInitialReplaceRef.current && data.action.type === 'REPLACE') { skipInitialReplaceRef.current = false - proceed(data.action) + proceed({ kind: 'action', action: data.action }) return } @@ -235,17 +251,15 @@ export function useSessionLeaveGuard(opts: { }) if (decision.kind === 'none') { - proceed(data.action) + proceed({ kind: 'action', action: data.action }) return } if (decision.kind === 'apply') { - pendingActionRef.current = data.action - void runLeaveAction(decision.action, data.action) + void runLeaveAction(decision.action) return } - pendingActionRef.current = data.action modalVisibleRef.current = true modalIsShowingRef.current = true setLeaveModalVisible(true) @@ -255,24 +269,26 @@ export function useSessionLeaveGuard(opts: { // render, so depending on it here would re-dispatch the same action on every // subsequent render until the screen unmounts. useEffect(() => { - if (!continueAction) return - navRef.current.dispatch(continueAction) - }, [continueAction]) + if (!continuation) return + if (continuation.kind === 'home') { + homeRef.current() + return + } + navRef.current.dispatch(continuation.action) + }, [continuation]) const cancelLeave = useCallback(() => { modalVisibleRef.current = false modalIsShowingRef.current = false - pendingActionRef.current = null setLeaveModalVisible(false) }, []) const confirmLeave = useCallback( (choice: AppliedSessionLeaveAction, remember: boolean) => { if (remember) persistLeaveSetting(choice) - const action = pendingActionRef.current modalVisibleRef.current = false setLeaveModalVisible(false) - void runLeaveAction(choice, action) + void runLeaveAction(choice) }, [runLeaveAction], ) @@ -288,7 +304,7 @@ export function useSessionLeaveGuard(opts: { return { leaveModalVisible, leavePhase, - isLeaving: leavePhase !== 'idle' || continueAction != null, + isLeaving: leavePhase !== 'idle' || continuation != null, cancelLeave, confirmLeave, dismissLeaveError, diff --git a/package.json b/package.json index 6115e6a7..27f85511 100644 --- a/package.json +++ b/package.json @@ -33,6 +33,7 @@ "test:e2e:mock": "E2E_MOCK_SERVER_URL=${E2E_MOCK_SERVER_URL:-http://localhost:7071}; export E2E_MOCK_SERVER_URL; node e2e/check-sim.js && node e2e/ensure-release-build.js && (MOCK_PORTS=7071,7072 node e2e/mock-server.js & MOCK_PID=$!; node e2e/wait-for-mock.js || { kill $MOCK_PID 2>/dev/null; exit 1; }; node e2e/run-maestro.js test --debug-output e2e/_artifacts/debug --test-output-dir e2e/_artifacts/maestro-output e2e/launch.yaml e2e/browse.yaml e2e/session_lifecycle.yaml e2e/server_drag_reorder.yaml e2e/bug6_bottom_bar_inset.yaml e2e/pty_turn_divider.yaml e2e/feat1_tree_drill_new_session.yaml e2e/feat2_export_in_info_shelf.yaml e2e/codex_parity.yaml e2e/voice_dictation.yaml e2e/settings_qr_scanner.yaml e2e/language_direction.yaml e2e/feedback_flow.yaml e2e/05_chat_flow.yaml e2e/06_search_anchor.yaml e2e/07_conversation_scroll_gaps.yaml; STATUS=$?; kill $MOCK_PID 2>/dev/null || true; exit $STATUS)", "test:e2e:parallel-fetch": "node e2e/check-sim.js && (MOCK_PORT=7073 MOCK_TOTAL_CONVERSATIONS=120 MOCK_TOTAL_SESSIONS=25 MOCK_PAGE_DELAY_MS=3000 node e2e/pagination-mock-server.js & MOCK_PID=$!; sleep 1; node e2e/run-maestro.js test --debug-output e2e/_artifacts/debug e2e/parallel-fetch-progress.yaml; STATUS=$?; kill $MOCK_PID 2>/dev/null || true; exit $STATUS)", "test:e2e:parallel-fetch:non-merged": "node e2e/check-sim.js && (MOCK_PORT=7073 MOCK_TOTAL_CONVERSATIONS=120 MOCK_TOTAL_SESSIONS=25 MOCK_PAGE_DELAY_MS=3000 node e2e/pagination-mock-server.js & MOCK_PID=$!; sleep 1; node e2e/run-maestro.js test --debug-output e2e/_artifacts/debug e2e/non-merged-conv-loading.yaml; STATUS=$?; kill $MOCK_PID 2>/dev/null || true; exit $STATUS)", + "test:e2e:leave-nav": "node e2e/check-sim.js && node e2e/ensure-release-build.js && node e2e/run-leave-nav.js", "test:e2e:ts1": "node e2e/check-sim.js && node e2e/ensure-release-build.js && (MOCK_PORTS=7071,7072 node e2e/mock-server.js & MOCK_PID=$!; sleep 1; node e2e/run-maestro.js test --debug-output e2e/_artifacts/debug e2e/ts1_onboarding_pairing.yaml; STATUS=$?; kill $MOCK_PID 2>/dev/null || true; exit $STATUS)", "test:e2e:ts1:record": "node e2e/check-sim.js && node e2e/ensure-release-build.js && (MOCK_PORTS=7071,7072 node e2e/mock-server.js & MOCK_PID=$!; sleep 1; node scripts/record-simulator-flow.js e2e/ts1_onboarding_pairing.yaml; STATUS=$?; kill $MOCK_PID 2>/dev/null || true; exit $STATUS)", "test:e2e:promo:pairing": "node e2e/check-sim.js && node e2e/ensure-release-build.js && (MOCK_PORTS=7071,7072 node e2e/mock-server.js & MOCK_PID=$!; sleep 1; node scripts/record-simulator-flow.js e2e/promo_02_pairing.yaml; STATUS=$?; kill $MOCK_PID 2>/dev/null || true; exit $STATUS)", diff --git a/services/query-client.ts b/services/query-client.ts index bc95c275..1ed85b1b 100644 --- a/services/query-client.ts +++ b/services/query-client.ts @@ -191,11 +191,15 @@ queryClient.getQueryCache().subscribe((event) => { // using a conversation id, and 404s by construction for any conversation // that was never a session) added a second, phantom "Session details // failed to load" row to every conversation 404. - if ((query.meta as { silentError?: boolean } | undefined)?.silentError) return + const meta = query.meta as { silentError?: boolean; silentNotFound?: boolean } | undefined + if (meta?.silentError) return const err = query.state.error const message = err instanceof Error ? err.message : i18n.t('common:error.unexpected') const status = err && 'status' in (err as object) ? (err as { status?: number }).status : undefined + // Narrower than silentError: the query owns the "this is gone" case on + // its own screen, but still wants a server failure reported globally. + if (status === 404 && meta?.silentNotFound) return const code = err && 'code' in (err as object) ? (err as { code?: string }).code : undefined setTimeout(() => store.pushError({ category, message, status, code }), 0)