From 89eb6758442376c59aaa9510128cdc24a7ad889e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20=C5=A0=C4=87eki=C4=87?= Date: Wed, 19 Aug 2026 01:38:02 +0200 Subject: [PATCH 01/34] feat(mobile): add infinite-query retention primitive Add withInfiniteRetention, patchInfiniteEntity, reconcileFirstPage, and scheduleCacheMaintenance so infinite-query owners share one bounded-retention contract. Register the new lib/query test glob in the pure vitest config. --- .../src/lib/query/infinite-retention.test.ts | 150 ++++++++++++++++++ .../src/lib/query/infinite-retention.ts | 112 +++++++++++++ apps/mobile/vitest.pure.config.ts | 1 + 3 files changed, 263 insertions(+) create mode 100644 apps/mobile/src/lib/query/infinite-retention.test.ts create mode 100644 apps/mobile/src/lib/query/infinite-retention.ts diff --git a/apps/mobile/src/lib/query/infinite-retention.test.ts b/apps/mobile/src/lib/query/infinite-retention.test.ts new file mode 100644 index 0000000000..40e59c0978 --- /dev/null +++ b/apps/mobile/src/lib/query/infinite-retention.test.ts @@ -0,0 +1,150 @@ +import { QueryClient } from '@tanstack/react-query'; +import { InteractionManager } from 'react-native'; +import { describe, expect, it, vi } from 'vitest'; + +import { + INFINITE_QUERY_MAX_PAGES, + patchInfiniteEntity, + reconcileFirstPage, + scheduleCacheMaintenance, + withInfiniteRetention, +} from '@/lib/query/infinite-retention'; + +vi.mock('react-native', () => ({ + InteractionManager: { runAfterInteractions: vi.fn() }, +})); + +type Item = { + id: string; + title: string; +}; + +type Page = { + items: Item[]; +}; + +const page = (items: Item[]): Page => ({ items }); + +describe('withInfiniteRetention', () => { + it('merges the default maxPages into the options', () => { + const options = { staleTime: 1000, queryKey: ['sessions'] }; + const result = withInfiniteRetention(options); + + expect(result.maxPages).toBe(INFINITE_QUERY_MAX_PAGES); + expect(result.staleTime).toBe(1000); + expect(result.queryKey).toEqual(['sessions']); + }); + + it('uses the explicit maxPages when provided', () => { + const result = withInfiniteRetention({ staleTime: 1000 }, 10); + + expect(result.maxPages).toBe(10); + }); +}); + +describe('patchInfiniteEntity', () => { + it('replaces only matching entities and keeps non-matching references', () => { + const queryClient = new QueryClient(); + const key = ['sessions', 'list']; + const target = { id: 'a', title: 'old' }; + const other = { id: 'b', title: 'other' }; + const secondPage = page([{ id: 'c', title: 'c' }]); + const firstPage = page([target, other]); + + queryClient.setQueryData(key, { + pages: [firstPage, secondPage], + pageParams: [0, 1], + }); + + patchInfiniteEntity({ + queryClient, + queryKey: key, + selectItems: p => p.items, + replaceItems: (p, items) => ({ ...p, items }), + matches: e => e.id === 'a', + update: e => ({ ...e, title: 'new' }), + }); + + const data = queryClient.getQueryData(key) as { pages: Page[] }; + expect(data.pages).toHaveLength(2); + expect(data.pages[0]?.items[0]?.title).toBe('new'); + // Non-matching entity keeps its exact reference. + expect(data.pages[0]?.items[1]).toBe(other); + // A page with no match keeps its exact reference. + expect(data.pages[1]).toBe(secondPage); + }); + + it('returns the top-level data unchanged when nothing matches', () => { + const queryClient = new QueryClient(); + const key = ['sessions', 'list']; + const data = { + pages: [page([{ id: 'a', title: 'a' }])], + pageParams: [0], + }; + queryClient.setQueryData(key, data); + + patchInfiniteEntity({ + queryClient, + queryKey: key, + selectItems: p => p.items, + replaceItems: (p, items) => ({ ...p, items }), + matches: () => false, + update: e => ({ ...e, title: 'changed' }), + }); + + expect(queryClient.getQueryData(key)).toBe(data); + }); +}); + +describe('reconcileFirstPage', () => { + it('trims to page one through a prefix key when the cached key carries an extra input segment', () => { + const queryClient = new QueryClient(); + const prefix = ['trpc', 'cliSessionsV2', 'list']; + const fullKey = [...prefix, { input: { organizationId: 'org-1' } }]; + queryClient.setQueryData(fullKey, { + pages: [ + page([{ id: 'a', title: 'a' }]), + page([{ id: 'b', title: 'b' }]), + page([{ id: 'c', title: 'c' }]), + ], + pageParams: [0, 1, 2], + }); + + reconcileFirstPage(queryClient, prefix); + + const data = queryClient.getQueryData(fullKey) as { pages: Page[]; pageParams: unknown[] }; + expect(data.pages).toHaveLength(1); + expect(data.pageParams).toHaveLength(1); + }); + + it('leaves a non-infinite entry under the same prefix untouched', () => { + const queryClient = new QueryClient(); + const prefix = ['trpc', 'cliSessionsV2', 'list']; + const nonInfiniteKey = [...prefix, { input: { organizationId: 'org-2' } }]; + const nonInfinite = { repos: [{ name: 'r' }] }; + queryClient.setQueryData(nonInfiniteKey, nonInfinite); + + reconcileFirstPage(queryClient, prefix); + + expect(queryClient.getQueryData(nonInfiniteKey)).toBe(nonInfinite); + }); + + it('returns synchronously', () => { + const queryClient = new QueryClient(); + + // eslint-disable-next-line typescript-eslint/no-confusing-void-expression -- asserting the void return proves the call needs no await. + const returned = reconcileFirstPage(queryClient, ['trpc', 'cliSessionsV2', 'list']); + expect(returned).toBeUndefined(); + }); +}); + +describe('scheduleCacheMaintenance', () => { + it('runs the callback through InteractionManager.runAfterInteractions', () => { + const run = vi.fn<() => void>(); + + scheduleCacheMaintenance(run); + + // eslint-disable-next-line typescript-eslint/unbound-method, typescript-eslint/no-deprecated -- the mock is a plain vi.fn() with no `this`, and runAfterInteractions is the documented deferral API. + expect(InteractionManager.runAfterInteractions).toHaveBeenCalledWith(run); + }); +}); diff --git a/apps/mobile/src/lib/query/infinite-retention.ts b/apps/mobile/src/lib/query/infinite-retention.ts new file mode 100644 index 0000000000..55f3443bcd --- /dev/null +++ b/apps/mobile/src/lib/query/infinite-retention.ts @@ -0,0 +1,112 @@ +import { type QueryClient } from '@tanstack/react-query'; +import { InteractionManager } from 'react-native'; + +/** + * The default number of pages an infinite query keeps in memory. + * + * Bounded retention keeps long-lived session and findings lists from growing + * without limit while a user pages through them. + */ +export const INFINITE_QUERY_MAX_PAGES = 5; + +/** + * Merge the retention bound into an infinite-query options object. + * + * Returns the same options with a numeric `maxPages` added, so every + * in-scope owner states the same retention contract. + */ +export function withInfiniteRetention( + options: T, + maxPages: number = INFINITE_QUERY_MAX_PAGES +): T & { maxPages: number } { + return { ...options, maxPages }; +} + +/** + * Replace only the matching entities across every loaded page of one + * infinite query, without refetching. + * + * Pages with no match and entities that do not match keep their exact object + * reference. The top-level data object is also returned unchanged when + * nothing matches, so unrelated subscribers never see a new reference. + */ +export function patchInfiniteEntity(args: { + queryClient: QueryClient; + queryKey: readonly unknown[]; + selectItems: (page: TPage) => TEntity[]; + replaceItems: (page: TPage, items: TEntity[]) => TPage; + matches: (entity: TEntity) => boolean; + update: (entity: TEntity) => TEntity; +}): void { + const { queryClient, queryKey, selectItems, replaceItems, matches, update } = args; + + queryClient.setQueryData(queryKey, old => { + if (typeof old !== 'object' || old === null || !('pages' in old)) { + return old; + } + const data = old as { pages: TPage[] }; + const nextPages: TPage[] = []; + let changed = false; + for (const page of data.pages) { + const items = selectItems(page); + const nextItems: TEntity[] = []; + let pageChanged = false; + for (const item of items) { + if (matches(item)) { + pageChanged = true; + nextItems.push(update(item)); + } else { + nextItems.push(item); + } + } + if (pageChanged) { + changed = true; + nextPages.push(replaceItems(page, nextItems)); + } else { + nextPages.push(page); + } + } + if (!changed) { + return old; + } + return { ...data, pages: nextPages }; + }); +} + +/** + * Trim every matching infinite query to page one, then invalidate the same + * prefix so the retained page is refetched. + * + * Uses `setQueriesData` (plural) on purpose: callers hold an invalidate + * prefix while the live cache keys carry the query input as well, so + * `setQueryData` on the prefix would match nothing. + * + * A non-infinite entry under the prefix is left untouched by the + * `'pages' in old` guard. + */ +export function reconcileFirstPage( + queryClient: QueryClient, + queryKeyPrefix: readonly unknown[] +): void { + queryClient.setQueriesData({ queryKey: queryKeyPrefix }, old => { + if (typeof old !== 'object' || old === null || !('pages' in old)) { + return old; + } + const data = old as { pages: unknown[]; pageParams: unknown[] }; + return { + ...old, + pages: data.pages.slice(0, 1), + pageParams: data.pageParams.slice(0, 1), + }; + }); + void queryClient.invalidateQueries({ queryKey: queryKeyPrefix }); +} + +/** + * Run cache maintenance after the current interactions settle, so a + * navigation frame never waits on it. + */ +export function scheduleCacheMaintenance(run: () => void): void { + // eslint-disable-next-line typescript-eslint/no-deprecated -- InteractionManager.runAfterInteractions is the documented API for deferring work past the current interaction frame. + InteractionManager.runAfterInteractions(run); +} diff --git a/apps/mobile/vitest.pure.config.ts b/apps/mobile/vitest.pure.config.ts index 3a2a2b9b43..b98c643cdc 100644 --- a/apps/mobile/vitest.pure.config.ts +++ b/apps/mobile/vitest.pure.config.ts @@ -32,6 +32,7 @@ export default defineProject({ 'src/lib/onboarding/**/*.test.ts', 'src/lib/persist/**/*.test.ts', 'src/lib/pr-review/**/*.test.ts', + 'src/lib/query/**/*.test.ts', 'src/lib/voice-input/**/*.test.ts', 'src/components/**/*.test.ts', 'src/components/pr-review/**/*.test.tsx', From fbceec7cfa56d07837a094100a9f56a0ea3d1a24 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20=C5=A0=C4=87eki=C4=87?= Date: Wed, 19 Aug 2026 01:38:28 +0200 Subject: [PATCH 02/34] feat(cloud-agent-sdk): keep failed delivery state and add retry entry point A cloud.message.failed event now keeps a status:failed pending entry instead of deleting it, so the row keeps its recovery affordance. Failed entries survive queue.changed and connected, reset clears them, and a later queued replaces them. Add clearFailedMessage to the service state and SessionManager. --- .../message-delivery-exhausted.ts | 11 +- .../cloud-agent-sdk/src/service-state.test.ts | 153 +++++++++++++++++- packages/cloud-agent-sdk/src/service-state.ts | 30 +++- .../src/session-manager.test.ts | 26 +++ .../cloud-agent-sdk/src/session-manager.ts | 13 ++ 5 files changed, 225 insertions(+), 8 deletions(-) diff --git a/packages/cloud-agent-sdk/src/__fixtures__/message-delivery-exhausted.ts b/packages/cloud-agent-sdk/src/__fixtures__/message-delivery-exhausted.ts index 3024072d4f..6655e27843 100644 --- a/packages/cloud-agent-sdk/src/__fixtures__/message-delivery-exhausted.ts +++ b/packages/cloud-agent-sdk/src/__fixtures__/message-delivery-exhausted.ts @@ -6,7 +6,7 @@ const { createEvent } = createEventHelpers(); const messageDeliveryExhausted: Fixture = { name: 'message-delivery-exhausted', description: - 'cloud.message.queued followed by cloud.message.failed with reason=exhausted clears pending delivery state', + 'cloud.message.queued followed by cloud.message.failed with reason=exhausted keeps a failed pending delivery entry', events: [ createEvent('cloud.message.queued', { messageId: 'msg-queued-1', @@ -25,7 +25,14 @@ const messageDeliveryExhausted: Fixture = { expected: { messageIds: [], parts: {}, - pendingMessages: {}, + pendingMessages: { + 'msg-queued-1': { + status: 'failed', + error: 'Failed to flush queued message after 5 attempts', + reason: 'exhausted', + attempts: 5, + }, + }, }, }; diff --git a/packages/cloud-agent-sdk/src/service-state.test.ts b/packages/cloud-agent-sdk/src/service-state.test.ts index 08c1546085..fd030c553c 100644 --- a/packages/cloud-agent-sdk/src/service-state.test.ts +++ b/packages/cloud-agent-sdk/src/service-state.test.ts @@ -1651,7 +1651,7 @@ describe('createServiceState', () => { expect(cb).toHaveBeenCalledTimes(2); }); - it('cloud.message.failed with reason=exhausted clears pending entry', () => { + it('cloud.message.failed with reason=exhausted keeps a failed pending entry', () => { const state = createServiceState(makeConfig()); state.process({ type: 'cloud.message.queued', messageId: 'm1' }); @@ -1663,7 +1663,12 @@ describe('createServiceState', () => { attempts: 5, }); - expect(state.getPendingMessages().has('m1')).toBe(false); + expect(state.getPendingMessages().get('m1')).toEqual({ + status: 'failed', + error: 'flush failed', + reason: 'exhausted', + attempts: 5, + }); }); it('cloud.message.failed with reason=interrupted settles the session', () => { @@ -1678,12 +1683,16 @@ describe('createServiceState', () => { reason: 'interrupted', }); - expect(state.getPendingMessages().has('m1')).toBe(false); + expect(state.getPendingMessages().get('m1')).toEqual({ + status: 'failed', + error: 'Pending queued message interrupted by user', + reason: 'interrupted', + }); expect(state.getActivity()).toEqual({ type: 'idle' }); expect(state.getStatus()).toEqual({ type: 'interrupted' }); }); - it('cloud.message.failed with reason=execution clears pending entry', () => { + it('cloud.message.failed with reason=execution keeps a failed pending entry', () => { const state = createServiceState(makeConfig()); state.process({ type: 'cloud.message.queued', messageId: 'm1' }); @@ -1694,7 +1703,11 @@ describe('createServiceState', () => { reason: 'execution', }); - expect(state.getPendingMessages().has('m1')).toBe(false); + expect(state.getPendingMessages().get('m1')).toEqual({ + status: 'failed', + error: 'boom', + reason: 'execution', + }); }); it('cloud.message.queued can repopulate an entry after a failed event', () => { @@ -1746,6 +1759,136 @@ describe('createServiceState', () => { expect(state.getPendingMessages().size).toBe(0); }); + describe('failed-entry survival', () => { + const failed = (state: ReturnType, id: string) => + state.getPendingMessages().get(id); + + it('a failed entry survives a non-empty queue.changed snapshot', () => { + const state = createServiceState(makeConfig()); + + state.process({ type: 'cloud.message.queued', messageId: 'm1' }); + state.process({ + type: 'cloud.message.failed', + messageId: 'm1', + error: 'flush failed', + reason: 'exhausted', + attempts: 5, + }); + state.process({ type: 'queue.changed', sessionId: 'root-1', queued: ['m2'] }); + + expect(failed(state, 'm1')).toEqual({ + status: 'failed', + error: 'flush failed', + reason: 'exhausted', + attempts: 5, + }); + expect(state.getPendingMessages().get('m2')).toEqual({ status: 'queued' }); + }); + + it('a failed entry survives an empty queue.changed snapshot', () => { + const state = createServiceState(makeConfig()); + + state.process({ type: 'cloud.message.queued', messageId: 'm1' }); + state.process({ + type: 'cloud.message.failed', + messageId: 'm1', + error: 'flush failed', + reason: 'exhausted', + attempts: 5, + }); + state.process({ type: 'queue.changed', sessionId: 'root-1', queued: [] }); + + expect(failed(state, 'm1')).toEqual({ + status: 'failed', + error: 'flush failed', + reason: 'exhausted', + attempts: 5, + }); + }); + + it('a failed entry survives connected', () => { + const state = createServiceState(makeConfig()); + + state.process({ type: 'cloud.message.queued', messageId: 'm1' }); + state.process({ + type: 'cloud.message.failed', + messageId: 'm1', + error: 'flush failed', + reason: 'exhausted', + attempts: 5, + }); + state.process({ type: 'connected', sessionStatus: { type: 'idle' } }); + + expect(failed(state, 'm1')).toEqual({ + status: 'failed', + error: 'flush failed', + reason: 'exhausted', + attempts: 5, + }); + }); + + it('reset clears a failed entry', () => { + const state = createServiceState(makeConfig()); + + state.process({ type: 'cloud.message.queued', messageId: 'm1' }); + state.process({ + type: 'cloud.message.failed', + messageId: 'm1', + error: 'flush failed', + reason: 'exhausted', + attempts: 5, + }); + state.reset(); + + expect(state.getPendingMessages().size).toBe(0); + }); + + it('a later queued for the same id replaces the failed entry', () => { + const state = createServiceState(makeConfig()); + + state.process({ type: 'cloud.message.queued', messageId: 'm1' }); + state.process({ + type: 'cloud.message.failed', + messageId: 'm1', + error: 'flush failed', + reason: 'exhausted', + attempts: 5, + }); + state.process({ type: 'cloud.message.queued', messageId: 'm1' }); + + expect(state.getPendingMessages().get('m1')).toEqual({ status: 'queued' }); + }); + + it('clearFailedMessage removes exactly one failed entry', () => { + const state = createServiceState(makeConfig()); + + state.process({ type: 'cloud.message.queued', messageId: 'm1' }); + state.process({ + type: 'cloud.message.failed', + messageId: 'm1', + error: 'flush failed', + reason: 'exhausted', + attempts: 5, + }); + state.process({ type: 'cloud.message.queued', messageId: 'm2' }); + state.process({ + type: 'cloud.message.failed', + messageId: 'm2', + error: 'boom', + reason: 'execution', + }); + + state.clearFailedMessage('m1'); + + expect(state.getPendingMessages().has('m1')).toBe(false); + expect(state.getPendingMessages().get('m2')).toEqual({ + status: 'failed', + error: 'boom', + reason: 'execution', + }); + }); + }); + it('fires onMessageQueued callback with messageId', () => { const onMessageQueued = jest.fn(); const state = createServiceState(makeConfig({ onMessageQueued })); diff --git a/packages/cloud-agent-sdk/src/service-state.ts b/packages/cloud-agent-sdk/src/service-state.ts index b25511dc1b..d06d164aed 100644 --- a/packages/cloud-agent-sdk/src/service-state.ts +++ b/packages/cloud-agent-sdk/src/service-state.ts @@ -74,6 +74,8 @@ type ServiceState = { getSuggestion(): SuggestionState | null; getSessionInfo(): SessionInfo | null; getPendingMessages(): ReadonlyMap; + /** Remove one failed delivery entry (called after a successful retry). */ + clearFailedMessage(messageId: string): void; snapshot(): ServiceStateSnapshot; /** Set activity directly (for transport lifecycle events like connecting/disconnected). */ setActivity(activity: SessionActivity): void; @@ -575,7 +577,7 @@ function createServiceState(config: ServiceStateConfig): ServiceState { reason: event.reason, ...(event.attempts !== undefined ? { attempts: event.attempts } : {}), }; - pendingMessages.delete(event.messageId); + pendingMessages.set(event.messageId, deliveryState); if (event.reason === 'interrupted') { activity = { type: 'idle' }; status = { type: 'interrupted' }; @@ -603,7 +605,15 @@ function createServiceState(config: ServiceStateConfig): ServiceState { if (!isRootSession(event.sessionId)) return; if (event.queued.length === 0) { if (pendingMessages.size === 0) return; + // Preserve failed entries — a failed delivery row must survive a + // reconciliation so its recovery affordance stays visible. + const failed = [...pendingMessages.entries()].filter( + ([, state]) => state.status === 'failed' + ); pendingMessages.clear(); + for (const [messageId, state] of failed) { + pendingMessages.set(messageId, state); + } notify(); return; } @@ -611,12 +621,18 @@ function createServiceState(config: ServiceStateConfig): ServiceState { for (const messageId of event.queued) { next.set(messageId, { status: 'queued' }); } + // Preserve failed entries before the wholesale replace, then re-insert + // them after the queued entries are written. + const failed = [...pendingMessages.entries()].filter(([, state]) => state.status === 'failed'); // Reuse the same Map identity where possible to avoid invalidating // existing subscribers that hold onto the previous reference. pendingMessages.clear(); for (const [messageId, state] of next) { pendingMessages.set(messageId, state); } + for (const [messageId, state] of failed) { + pendingMessages.set(messageId, state); + } notify(); } @@ -679,7 +695,14 @@ function createServiceState(config: ServiceStateConfig): ServiceState { // Clear pending-message delivery state — replayed cloud.message.queued // events following the snapshot will repopulate it with the current truth. + // Failed entries survive the reconnect: replayed cloud.message.queued events + // repopulate the queued half, and the failed half must survive a reconnect + // so the row keeps its recovery affordance. + const failed = [...pendingMessages.entries()].filter(([, state]) => state.status === 'failed'); pendingMessages.clear(); + for (const [messageId, state] of failed) { + pendingMessages.set(messageId, state); + } notify(); } @@ -781,6 +804,11 @@ function createServiceState(config: ServiceStateConfig): ServiceState { getSessionInfo: () => sessionInfo, getPendingMessages: () => pendingMessages, + clearFailedMessage(messageId: string): void { + pendingMessages.delete(messageId); + notify(); + }, + snapshot: () => ({ activity, status, diff --git a/packages/cloud-agent-sdk/src/session-manager.test.ts b/packages/cloud-agent-sdk/src/session-manager.test.ts index 3cfab12974..653d014298 100644 --- a/packages/cloud-agent-sdk/src/session-manager.test.ts +++ b/packages/cloud-agent-sdk/src/session-manager.test.ts @@ -99,6 +99,7 @@ const mockSession = { getPermission: jest.fn(() => null), getSuggestion: jest.fn(() => null), getPendingMessages: jest.fn, []>(() => new Map()), + clearFailedMessage: jest.fn(), }, storage: null as JotaiSessionStorage | null, } as unknown as MockSession; @@ -3775,6 +3776,31 @@ describe('createSessionManager', () => { ); expect(pending.size).toBe(0); }); + + it('clearFailedMessage removes one id from the atom and service state', async () => { + const config = createMockConfig(); + const mgr = createSessionManager(config); + + const triggerSubscriber = await switchAndCaptureSubscriber(config, mgr); + + mockSession.state.getPendingMessages.mockReturnValue( + new Map([ + ['m1', { status: 'failed', error: 'x', reason: 'exhausted', attempts: 5 }], + ['m2', { status: 'failed', error: 'y', reason: 'execution' }], + ]) + ); + triggerSubscriber(); + + mgr.clearFailedMessage('m1'); + + const pending = atomValue>( + config.store, + mgr.atoms.pendingMessages + ); + expect(pending.has('m1')).toBe(false); + expect(pending.get('m2')).toEqual({ status: 'failed', error: 'y', reason: 'execution' }); + expect(mockSession.state.clearFailedMessage).toHaveBeenCalledWith('m1'); + }); }); // ------------------------------------------------------------------------- diff --git a/packages/cloud-agent-sdk/src/session-manager.ts b/packages/cloud-agent-sdk/src/session-manager.ts index 134262e892..c793b5f22e 100644 --- a/packages/cloud-agent-sdk/src/session-manager.ts +++ b/packages/cloud-agent-sdk/src/session-manager.ts @@ -410,6 +410,11 @@ type SessionManager = { respondToPermission(requestId: string, response: 'once' | 'always' | 'reject'): Promise; acceptSuggestion(requestId: string, index: number): Promise; dismissSuggestion(requestId: string): Promise; + /** + * Remove one failed delivery entry after a successful retry so its row + * stops showing. + */ + clearFailedMessage(messageId: string): void; createAndStart(input: PrepareInput): Promise; clearError(): void; destroy(): void; @@ -1755,6 +1760,13 @@ function createSessionManager(config: SessionManagerConfig): SessionManager { if (currentSession) await currentSession.dismissSuggestion({ requestId }); } + function clearFailedMessage(messageId: string): void { + currentSession?.state.clearFailedMessage(messageId); + const next = new Map(store.get(pendingMessagesAtom)); + next.delete(messageId); + store.set(pendingMessagesAtom, next); + } + async function createAndStart(input: PrepareInput): Promise { try { const initialMessageId = input.initialMessageId ?? generateMessageId(); @@ -1855,6 +1867,7 @@ function createSessionManager(config: SessionManagerConfig): SessionManager { respondToPermission, acceptSuggestion, dismissSuggestion, + clearFailedMessage, createAndStart, clearError: () => { store.set(errorAtom, null); From 3332442f13f1151881f2fed2ba7eeb72e12090e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20=C5=A0=C4=87eki=C4=87?= Date: Wed, 19 Aug 2026 02:04:36 +0200 Subject: [PATCH 03/34] fix(cloud-agent-sdk): avoid duplicate socket on online-during-connect race When NetInfo reports online while the first socket is still establishing, refreshAndConnect awaited refreshAuth and then closed the now-healthy socket and opened a second one. Re-validate after the refresh gap so an established socket is never replaced. Add a test asserting one socket per open. --- .../src/base-connection.test.ts | 35 ++++++++ .../cloud-agent-sdk/src/base-connection.ts | 8 ++ .../src/cloud-agent-transport.test.ts | 89 +++++++++++++++++++ 3 files changed, 132 insertions(+) diff --git a/packages/cloud-agent-sdk/src/base-connection.test.ts b/packages/cloud-agent-sdk/src/base-connection.test.ts index 2b24908b5e..24aa92c109 100644 --- a/packages/cloud-agent-sdk/src/base-connection.test.ts +++ b/packages/cloud-agent-sdk/src/base-connection.test.ts @@ -406,6 +406,41 @@ describe('createBaseConnection – stale WebSocket recovery', () => { expect(sockets).toHaveLength(2); connection.destroy(); }); + + it('keeps one socket when online fires while the first socket is still connecting', async () => { + const refreshAuth = jest.fn(() => Promise.resolve()); + const onReplacingConnection = jest.fn(); + const { connection, onConnected } = createTestConnection({ + refreshAuth, + onReplacingConnection, + }); + connection.connect(); + + // Socket 1 is still CONNECTING (readyState 0) when the first NetInfo + // `unknown → online` report arrives. handleOnline schedules a reconnect, + // which awaits refreshAuth before opening a replacement socket. + sockets[0].readyState = 0; // WebSocket.CONNECTING + mockWindow.dispatchEvent(new Event('online')); + + // During the async refreshAuth gap, socket 1 finishes its handshake and + // receives its first inbound message (connected becomes true). + sockets[0].readyState = 1; // WebSocket.OPEN + connectSocket(0); + + await Promise.resolve(); + await Promise.resolve(); + + // The refresh completed, but the re-validation in refreshAndConnect saw + // that socket 1 established during the gap and left it alone — one socket + // per open, no duplicate `/stream` socket. + expect(sockets).toHaveLength(1); + expect(onConnected).toHaveBeenCalledTimes(1); + expect(onReplacingConnection).not.toHaveBeenCalled(); + expect(sockets[0].close).not.toHaveBeenCalled(); + expect(refreshAuth).toHaveBeenCalledTimes(1); + + connection.destroy(); + }); }); describe('reconnect attempts reset after exhaustion', () => { diff --git a/packages/cloud-agent-sdk/src/base-connection.ts b/packages/cloud-agent-sdk/src/base-connection.ts index 9254df6f7f..ccd7e77ce8 100644 --- a/packages/cloud-agent-sdk/src/base-connection.ts +++ b/packages/cloud-agent-sdk/src/base-connection.ts @@ -140,6 +140,14 @@ export function createBaseConnection(config: BaseConnectionConfig): Connec // Continue with existing auth — the old ticket might still work } if (destroyed || intentionalDisconnect || expectedGeneration !== generation) return; + // The async refresh gap can let the in-flight socket finish its + // handshake and receive its first message. If it did, there is nothing + // to replace — reconnecting would close a healthy socket and open a + // duplicate (the two-socket `/stream` open). This is the NetInfo + // `unknown → online` race: handleOnline decides to reconnect while the + // socket is still CONNECTING, but the socket establishes before the + // refresh resolves. + if (connected && ws !== null && ws.readyState === WebSocket.OPEN) return; } connectInternal(0, expectedGeneration, true); } finally { diff --git a/packages/cloud-agent-sdk/src/cloud-agent-transport.test.ts b/packages/cloud-agent-sdk/src/cloud-agent-transport.test.ts index 23facf06fb..b18dc6592d 100644 --- a/packages/cloud-agent-sdk/src/cloud-agent-transport.test.ts +++ b/packages/cloud-agent-sdk/src/cloud-agent-transport.test.ts @@ -1646,3 +1646,92 @@ describe('CloudAgentTransport event delivery and replay cursor', () => { } }); }); + +// --------------------------------------------------------------------------- +// Duplicate WebSocket connect investigation (W10.2) +// --------------------------------------------------------------------------- + +describe('CloudAgentTransport single-connect guarantee', () => { + it('opens exactly one connection per connect() call (no duplicate)', async () => { + const getTicket = jest.fn(() => 'test-ticket'); + const fetchSnapshotPage = jest.fn().mockResolvedValue({ + kind: 'success' as const, + info: { id: 'ses-1' }, + messages: [], + nextCursor: null, + omittedItemCount: 0, + }); + + const factory = createCloudAgentTransport({ + sessionId: cloudAgentId('ses-1'), + kiloSessionId: kiloId('ses-1'), + api: createMockApi(), + getTicket, + fetchSnapshot: () => Promise.reject(new Error('legacy fetchSnapshot should not be called')), + fetchSnapshotPage, + websocketBaseUrl: 'ws://localhost:9999', + }); + + const transport = factory({ + onChatEvent: () => {}, + onServiceEvent: () => {}, + }); + + transport.connect(); + await flushPromises(); + + // One connect() must produce exactly one createConnection call. Each + // createConnection maps 1:1 to a WebSocket construction (the transport + // calls connect() once per createConnection, and connectInternal opens + // exactly one socket), so the WebSocket constructor count is the direct + // observable of createConnection calls. A duplicate driven from inside the + // transport would show up here as a second construction. + expect(webSocketConstructor).toHaveBeenCalledTimes(1); + expect(getTicket).toHaveBeenCalledTimes(1); + + transport.destroy(); + }); + + it('opens exactly one connection when the ticket is expiring (refresh does not double-connect)', async () => { + const nowSeconds = Math.floor(Date.now() / 1000); + const getTicket = jest + .fn() + .mockResolvedValueOnce({ ticket: 'expiring-ticket', expiresAt: nowSeconds + 5 }) + .mockResolvedValueOnce({ ticket: 'fresh-ticket', expiresAt: nowSeconds + 60 }); + const fetchSnapshotPage = jest.fn().mockResolvedValue({ + kind: 'success' as const, + info: { id: 'ses-1' }, + messages: [], + nextCursor: null, + omittedItemCount: 0, + }); + + const factory = createCloudAgentTransport({ + sessionId: cloudAgentId('ses-1'), + kiloSessionId: kiloId('ses-1'), + api: createMockApi(), + getTicket, + fetchSnapshot: () => Promise.reject(new Error('legacy fetchSnapshot should not be called')), + fetchSnapshotPage, + websocketBaseUrl: 'ws://localhost:9999', + }); + + const transport = factory({ + onChatEvent: () => {}, + onServiceEvent: () => {}, + }); + + transport.connect(); + await flushPromises(); + await Promise.resolve(); + await Promise.resolve(); + + // The pre-connect ticket refresh is a legitimate trigger: it refreshes the + // ticket (second getTicket) but still opens exactly one WebSocket. + expect(webSocketConstructor).toHaveBeenCalledTimes(1); + expect(getTicket).toHaveBeenCalledTimes(2); + expect(webSocketConstructor.mock.calls[0]?.[0]).toContain('ticket=fresh-ticket'); + + transport.destroy(); + }); +}); From 35ce1c51e655e3b5048ec0427363c3e846b92664 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20=C5=A0=C4=87eki=C4=87?= Date: Wed, 19 Aug 2026 02:23:22 +0200 Subject: [PATCH 04/34] feat(mobile): render failed-delivery and failed-assistant rows with recovery Add a failure footer to the message bubble: Failed to deliver with Retry and Copy to composer for a failed user row, Response failed with Retry only for a failed assistant row. selectMessageFailure maps reasons to fixed copy and never emits raw provider text. Wrap MessageBubble in React.memo. --- .../message-bubble-accessibility.test.ts | 3 +- .../agents/message-bubble-test-utils.ts | 4 +- .../components/agents/message-bubble.test.ts | 173 ++++++++++++++ .../src/components/agents/message-bubble.tsx | 218 ++++++++++++------ .../agents/message-failure-state.test.ts | 122 ++++++++++ .../agents/message-failure-state.ts | 71 ++++++ 6 files changed, 514 insertions(+), 77 deletions(-) create mode 100644 apps/mobile/src/components/agents/message-failure-state.test.ts create mode 100644 apps/mobile/src/components/agents/message-failure-state.ts diff --git a/apps/mobile/src/components/agents/message-bubble-accessibility.test.ts b/apps/mobile/src/components/agents/message-bubble-accessibility.test.ts index 7c106d19a4..98a29ed117 100644 --- a/apps/mobile/src/components/agents/message-bubble-accessibility.test.ts +++ b/apps/mobile/src/components/agents/message-bubble-accessibility.test.ts @@ -153,8 +153,9 @@ describe('MessageBubble long-press details', () => { }); const { MessageBubble } = await import('./message-bubble'); const message = userMessage('m-long'); + // MessageBubble is wrapped in React.memo; invoke its inner component. // eslint-disable-next-line new-cap - const tree = MessageBubble({ message, onLongPressDetails }); + const tree = MessageBubble.type({ message, onLongPressDetails }); const props = pressableProps(tree); expect(props).not.toBeNull(); const handler = props === null ? undefined : props.onLongPress; diff --git a/apps/mobile/src/components/agents/message-bubble-test-utils.ts b/apps/mobile/src/components/agents/message-bubble-test-utils.ts index a00cc1b668..dbae8ccc0f 100644 --- a/apps/mobile/src/components/agents/message-bubble-test-utils.ts +++ b/apps/mobile/src/components/agents/message-bubble-test-utils.ts @@ -49,8 +49,10 @@ export async function renderBubble( holdQueuedSlot?: boolean ): Promise { const { MessageBubble } = await import('./message-bubble'); + // MessageBubble is wrapped in React.memo; invoke its inner component directly + // to inspect the unrendered element tree. // eslint-disable-next-line new-cap - return MessageBubble({ message, deliveryState, holdQueuedSlot }); + return MessageBubble.type({ message, deliveryState, holdQueuedSlot }); } export function findText(node: unknown, predicate: (text: string) => boolean): boolean { diff --git a/apps/mobile/src/components/agents/message-bubble.test.ts b/apps/mobile/src/components/agents/message-bubble.test.ts index b3912dce49..6625b0d6f6 100644 --- a/apps/mobile/src/components/agents/message-bubble.test.ts +++ b/apps/mobile/src/components/agents/message-bubble.test.ts @@ -1,6 +1,8 @@ /* eslint-disable max-lines -- Queued-badge, delivery, and a11y seams share the direct-invocation MessageBubble harness. */ import { describe, expect, it, vi } from 'vitest'; +import { type MessageDeliveryState, type StoredMessage } from '@kilocode/cloud-agent-sdk'; + import { assistantMessage, findElementByType, @@ -33,6 +35,9 @@ vi.mock('@/components/ui/bubble', () => ({ vi.mock('@/components/ui/text', () => ({ Text: ({ children }: { children?: unknown }) => children, })); +vi.mock('@/components/ui/button', () => ({ + Button: 'Button', +})); vi.mock('./chat-markdown-text', () => ({ ChatMarkdownText: () => null, })); @@ -151,6 +156,149 @@ describe('MessageBubble failed delivery state', () => { }); }); +async function renderBubbleWithHandlers( + message: StoredMessage, + props: { + deliveryState?: MessageDeliveryState; + onRetryMessage?: (m: StoredMessage) => void; + onCopyToComposer?: (text: string) => void; + } +): Promise { + const { MessageBubble } = await import('./message-bubble'); + // eslint-disable-next-line new-cap + return MessageBubble.type({ message, ...props }); +} + +function assistantMessageWithError(id: string, errorName: string): StoredMessage { + const message = assistantMessage(id); + (message.info as { error?: { name: string; data: unknown } }).error = { + name: errorName, + data: { message: 'raw' }, + }; + return message; +} + +describe('MessageBubble failure footer', () => { + it('renders the failed-delivery footer with Retry and Copy to composer', async () => { + const tree = await renderBubbleWithHandlers(userMessage('m-fail'), { + deliveryState: { status: 'failed', error: 'nope', reason: 'exhausted' }, + onRetryMessage: vi.fn<(message: StoredMessage) => void>(), + onCopyToComposer: vi.fn<(text: string) => void>(), + }); + expect(findText(tree, t => t === 'Failed to deliver')).toBe(true); + expect( + findText(tree, t => t === 'We could not deliver this message after several attempts.') + ).toBe(true); + const retry = findElementByType(tree, 'Button', p => p.accessibilityLabel === 'Retry'); + expect(retry).not.toBeNull(); + expect(retry?.props.accessibilityRole).toBe('button'); + const copy = findElementByType( + tree, + 'Button', + p => p.accessibilityLabel === 'Copy to composer' + ); + expect(copy).not.toBeNull(); + expect(copy?.props.accessibilityRole).toBe('button'); + }); + + it('renders the assistant failure footer with Retry and no Copy to composer', async () => { + const tree = await renderBubbleWithHandlers(assistantMessageWithError('m-asst', 'APIError'), { + onRetryMessage: vi.fn<(message: StoredMessage) => void>(), + }); + expect(findText(tree, t => t === 'Response failed')).toBe(true); + const retry = findElementByType(tree, 'Button', p => p.accessibilityLabel === 'Retry'); + expect(retry).not.toBeNull(); + expect( + findElementByType(tree, 'Button', p => p.accessibilityLabel === 'Copy to composer') + ).toBeNull(); + }); + + it('omits the Retry button for a non-retryable assistant error', async () => { + const tree = await renderBubbleWithHandlers( + assistantMessageWithError('m-asst-nr', 'ProviderAuthError'), + { onRetryMessage: vi.fn<(message: StoredMessage) => void>() } + ); + expect(findText(tree, t => t === 'Response failed')).toBe(true); + expect(findElementByType(tree, 'Button', p => p.accessibilityLabel === 'Retry')).toBeNull(); + }); + + it('does not render the footer when no handler is supplied', async () => { + const tree = await renderBubbleWithHandlers(userMessage('m-nohandler'), { + deliveryState: { status: 'failed', error: 'nope', reason: 'exhausted' }, + }); + expect(findText(tree, t => t === 'Failed to deliver')).toBe(false); + }); + + it('names the failure row on the title text without grouping the CTA buttons', async () => { + const tree = await renderBubbleWithHandlers(userMessage('m-a11y'), { + deliveryState: { status: 'failed', error: 'nope', reason: 'interrupted' }, + onRetryMessage: vi.fn<(message: StoredMessage) => void>(), + onCopyToComposer: vi.fn<(text: string) => void>(), + }); + + // The footer container is a plain View: no accessible, no role, no label, + // so Retry and Copy stay individually focusable. + const footer = findElementByType(tree, 'View', p => p.className === 'gap-1 px-4 py-1'); + expect(footer).not.toBeNull(); + expect(footer?.props.accessible).toBeUndefined(); + expect(footer?.props.accessibilityRole).toBeUndefined(); + expect(footer?.props.accessibilityLabel).toBeUndefined(); + + // The row name lives on the title Text, which still announces the row. + const title = findElementByLabel(tree, 'Failed to deliver. Retry available.'); + expect(title).not.toBeNull(); + expect(title?.props.children).toBe('Failed to deliver'); + }); + + it('presses Retry to retry the failed message and Copy to composer to restore the user text', async () => { + const { isTextPart } = await import('./part-types'); + vi.mocked(isTextPart).mockReturnValue(true); + + const message = userMessage('m-press'); + const onRetryMessage = vi.fn<(message: StoredMessage) => void>(); + const onCopyToComposer = vi.fn<(text: string) => void>(); + const tree = await renderBubbleWithHandlers(message, { + deliveryState: { status: 'failed', error: 'nope', reason: 'exhausted' }, + onRetryMessage, + onCopyToComposer, + }); + + const retry = findElementByType(tree, 'Button', p => p.accessibilityLabel === 'Retry'); + expect(retry).not.toBeNull(); + if (!retry) { + throw new Error('expected Retry button'); + } + (retry.props.onPress as () => void)(); + expect(onRetryMessage).toHaveBeenCalledWith(message); + + const copy = findElementByType( + tree, + 'Button', + p => p.accessibilityLabel === 'Copy to composer' + ); + expect(copy).not.toBeNull(); + if (!copy) { + throw new Error('expected Copy to composer button'); + } + (copy.props.onPress as () => void)(); + expect(onCopyToComposer).toHaveBeenCalledWith('hi'); + }); + + it('presses Retry on an assistant row to retry the failed message', async () => { + const message = assistantMessageWithError('m-asst-press', 'APIError'); + const onRetryMessage = vi.fn<(message: StoredMessage) => void>(); + const tree = await renderBubbleWithHandlers(message, { onRetryMessage }); + + const retry = findElementByType(tree, 'Button', p => p.accessibilityLabel === 'Retry'); + expect(retry).not.toBeNull(); + if (!retry) { + throw new Error('expected Retry button'); + } + (retry.props.onPress as () => void)(); + expect(onRetryMessage).toHaveBeenCalledWith(message); + }); +}); + describe('MessageBubble regressions', () => { it('holds badge slot when queued and holdQueuedSlot is set after dequeue', async () => { const message = userMessage('m7'); @@ -274,6 +422,31 @@ function findElementByTypeFn( return null; } +function findElementByLabel( + node: unknown, + label: string +): { type: unknown; props: Record } | null { + if (node == null || typeof node !== 'object') { + return null; + } + const element = node as { type?: unknown; props?: Record }; + if (element.props?.accessibilityLabel === label) { + return element as { type: unknown; props: Record }; + } + const children = element.props?.children; + if (Array.isArray(children)) { + for (const child of children) { + const hit = findElementByLabel(child, label); + if (hit) { + return hit; + } + } + } else if (children && typeof children === 'object') { + return findElementByLabel(children, label); + } + return null; +} + function findProvider( node: unknown, providerType: unknown diff --git a/apps/mobile/src/components/agents/message-bubble.tsx b/apps/mobile/src/components/agents/message-bubble.tsx index cdef515d8b..54430a39de 100644 --- a/apps/mobile/src/components/agents/message-bubble.tsx +++ b/apps/mobile/src/components/agents/message-bubble.tsx @@ -1,8 +1,10 @@ +import { memo } from 'react'; import { type MessageDeliveryState, type StoredMessage } from '@kilocode/cloud-agent-sdk'; import { Clock } from '@/components/ui/icons'; import { type AccessibilityActionEvent, Pressable, View } from 'react-native'; import { Bubble } from '@/components/ui/bubble'; +import { Button } from '@/components/ui/button'; import { Text } from '@/components/ui/text'; import { useThemeColors } from '@/lib/hooks/use-theme-colors'; @@ -11,6 +13,7 @@ import { ChatMarkdownText } from './chat-markdown-text'; import { CompactionSeparator } from './compaction-separator'; import { FilePartRenderer } from './file-part-renderer'; import { buildAgentMessageBubbleAccessibilityProps } from './message-bubble-a11y'; +import { selectMessageFailure } from './message-failure-state'; import { PartRenderer } from './part-renderer'; import { isFilePart, isTextPart } from './part-types'; import { useMessageCopy } from './use-message-copy'; @@ -33,9 +36,13 @@ type MessageBubbleProps = { * deliveryState !== 'queued'; the hidden slot retains the same height. */ holdQueuedSlot?: boolean; + /** Retries a failed message. The failure footer renders only when supplied. */ + onRetryMessage?: (message: StoredMessage) => void; + /** Copies a failed user message's text back into the composer. */ + onCopyToComposer?: (text: string) => void; }; -export function MessageBubble({ +function MessageBubbleImpl({ message, isLastAssistantMessage, isSessionStreaming, @@ -45,6 +52,8 @@ export function MessageBubble({ deliveryState, onLongPressDetails, holdQueuedSlot, + onRetryMessage, + onCopyToComposer, }: Readonly) { const isUser = message.info.role === 'user'; const { copyMessage } = useMessageCopy(); @@ -78,53 +87,147 @@ export function MessageBubble({ ); } + // Failed-row footer. Renders only when the relevant handler is wired + // (mobile-w2b wires onRetryMessage/onCopyToComposer); a delivery row needs + // Retry or Copy, an assistant row needs Retry only. + const failure = selectMessageFailure({ deliveryState, info: message.info }); + const relevantHandlerWired = + failure?.kind === 'delivery' + ? onRetryMessage !== undefined || onCopyToComposer !== undefined + : onRetryMessage !== undefined; + const userTextContent = isUser + ? message.parts + .filter(isTextPart) + .map(p => p.text) + .join('\n\n') + : ''; + const failureFooter = + failure !== null && relevantHandlerWired ? ( + + + {failure.title} + + {failure.detail} + + {failure.canRetry && onRetryMessage ? ( + + ) : null} + {failure.canCopy && onCopyToComposer ? ( + + ) : null} + + + ) : null; + if (isUser) { // Composer, queued-message synthesis, and slash commands emit exactly one // human-authored text part, so the separator separates it from synthesized // attachment notices. - const textContent = message.parts - .filter(isTextPart) - .map(p => p.text) - .join('\n\n'); const fileParts = message.parts.filter(isFilePart); const isQueued = deliveryState?.status === 'queued'; const hasBadgeSlot = isQueued || holdQueuedSlot; const a11y = buildAgentMessageBubbleAccessibilityProps({ isUser: true, canCopy: true }); return ( - - - - - {textContent ? ( - - ) : null} - {fileParts.map(part => ( - - ))} - - - {hasBadgeSlot ? ( - - - - Queued + <> + + + + + {userTextContent ? ( + + ) : null} + {fileParts.map(part => ( + + ))} + + + {hasBadgeSlot ? ( + + + + Queued + - + ) : null} + + {a11y.accessibilityActions.length > 0 ? ( + ) : null} - + + {failureFooter} + + ); + } + + // Assistant messages: render parts sequentially without a bubble. + // Row-rhythm contract: py-1 on each of two adjacent wrappers sums to the + // same value as the gap-2 between parts of one message and the user + // wrapper's py-1 — every adjacent transcript row pair sits one gap apart. + const isStreaming = isLastAssistantMessage && isSessionStreaming; + const a11y = buildAgentMessageBubbleAccessibilityProps({ isUser: false, canCopy: true }); + + return ( + <> + + + + {message.parts.map(part => ( + + ))} + + {a11y.accessibilityActions.length > 0 ? ( ) : null} - ); - } - - // Assistant messages: render parts sequentially without a bubble. - // Row-rhythm contract: py-1 on each of two adjacent wrappers sums to the - // same value as the gap-2 between parts of one message and the user - // wrapper's py-1 — every adjacent transcript row pair sits one gap apart. - const isStreaming = isLastAssistantMessage && isSessionStreaming; - const a11y = buildAgentMessageBubbleAccessibilityProps({ isUser: false, canCopy: true }); - - return ( - - - - {message.parts.map(part => ( - - ))} - - - {a11y.accessibilityActions.length > 0 ? ( - - ) : null} - + {failureFooter} + ); } + +export const MessageBubble = memo(MessageBubbleImpl); diff --git a/apps/mobile/src/components/agents/message-failure-state.test.ts b/apps/mobile/src/components/agents/message-failure-state.test.ts new file mode 100644 index 0000000000..0777ca3f2e --- /dev/null +++ b/apps/mobile/src/components/agents/message-failure-state.test.ts @@ -0,0 +1,122 @@ +import { describe, expect, it } from 'vitest'; + +import { type MessageInfo } from '@kilocode/cloud-agent-sdk'; + +import { NON_RETRYABLE_ASSISTANT_ERRORS, selectMessageFailure } from './message-failure-state'; + +function userInfo(): MessageInfo { + return { + id: 'u1', + sessionID: 'ses_1', + role: 'user', + time: { created: 1_761_000_000_000 }, + agent: 'build', + model: { providerID: 'openrouter', modelID: 'anthropic/claude-sonnet-4' }, + }; +} + +type AssistantError = NonNullable['error']>; + +function assistantInfo(errorName: string): MessageInfo { + return { + id: 'a1', + sessionID: 'ses_1', + role: 'assistant', + time: { created: 1_761_000_000_000 }, + // Deliberately carries raw provider text; the helper must never surface it. + error: { name: errorName, data: { message: 'RAW_PROVIDER_TEXT' } } as unknown as AssistantError, + parentID: 'u1', + modelID: 'anthropic/claude-sonnet-4', + providerID: 'kilo', + mode: 'code', + agent: 'build', + path: { cwd: '/', root: '/' }, + cost: 0, + tokens: { total: 0, input: 0, output: 0, reasoning: 0, cache: { read: 0, write: 0 } }, + }; +} + +function assistantInfoWithoutError(): MessageInfo { + const info = assistantInfo('ProviderAuthError') as Extract; + delete info.error; + return info; +} + +describe('selectMessageFailure', () => { + it('returns null when there is no failed delivery and no assistant error', () => { + expect(selectMessageFailure({ info: userInfo() })).toBeNull(); + expect(selectMessageFailure({ info: assistantInfo('UnknownError') })).not.toBeNull(); + }); + + it('returns null for a queued delivery state', () => { + expect( + selectMessageFailure({ info: userInfo(), deliveryState: { status: 'queued' } }) + ).toBeNull(); + }); + + describe('delivery', () => { + it('maps every reason to fixed copy and never emits raw error text', () => { + const cases = [ + { reason: 'interrupted', detail: 'You stopped this message.' }, + { + reason: 'exhausted', + detail: 'We could not deliver this message after several attempts.', + }, + { reason: 'execution', detail: 'The agent could not run this message.' }, + ] as const; + + for (const { reason, detail } of cases) { + const result = selectMessageFailure({ + info: userInfo(), + deliveryState: { status: 'failed', error: 'RAW_TRANSPORT_TEXT', reason }, + }); + expect(result).not.toBeNull(); + expect(result?.kind).toBe('delivery'); + expect(result?.title).toBe('Failed to deliver'); + expect(result?.detail).toBe(detail); + expect(result?.detail).not.toContain('RAW_TRANSPORT_TEXT'); + expect(result?.canRetry).toBe(true); + expect(result?.canCopy).toBe(true); + } + }); + }); + + describe('assistant', () => { + it('returns null for an assistant info with no error', () => { + expect(selectMessageFailure({ info: assistantInfoWithoutError() })).toBeNull(); + }); + + it('derives fixed copy from a known error name and never emits provider text', () => { + const result = selectMessageFailure({ info: assistantInfo('ProviderAuthError') }); + expect(result).not.toBeNull(); + expect(result?.kind).toBe('assistant'); + expect(result?.title).toBe('Response failed'); + expect(result?.detail).toBe('The provider rejected the request.'); + expect(result?.detail).not.toContain('RAW_PROVIDER_TEXT'); + expect(result?.canCopy).toBe(false); + }); + + it('falls back to the generic line for an unknown error name', () => { + const result = selectMessageFailure({ info: assistantInfo('UnknownError') }); + expect(result?.detail).toBe('The response failed.'); + expect(result?.detail).not.toContain('RAW_PROVIDER_TEXT'); + }); + + it('sets canRetry false only for NON_RETRYABLE_ASSISTANT_ERRORS', () => { + for (const name of NON_RETRYABLE_ASSISTANT_ERRORS) { + const result = selectMessageFailure({ info: assistantInfo(name) }); + expect(result?.canRetry).toBe(false); + } + }); + + it('sets canRetry true for an assistant error outside the non-retryable set', () => { + const result = selectMessageFailure({ info: assistantInfo('APIError') }); + expect(result?.canRetry).toBe(true); + }); + + it('never sets canCopy true for an assistant row', () => { + const result = selectMessageFailure({ info: assistantInfo('APIError') }); + expect(result?.canCopy).toBe(false); + }); + }); +}); diff --git a/apps/mobile/src/components/agents/message-failure-state.ts b/apps/mobile/src/components/agents/message-failure-state.ts new file mode 100644 index 0000000000..84186be3a8 --- /dev/null +++ b/apps/mobile/src/components/agents/message-failure-state.ts @@ -0,0 +1,71 @@ +import { type MessageDeliveryState, type MessageInfo } from '@kilocode/cloud-agent-sdk'; + +/** + * Fixed, safe copy for a failed user-message delivery, keyed by the delivery + * `reason`. Never surfaces raw provider or transport text. + */ +type DeliveryReason = Extract['reason']; + +const DELIVERY_DETAIL_BY_REASON: Readonly> = { + interrupted: 'You stopped this message.', + exhausted: 'We could not deliver this message after several attempts.', + execution: 'The agent could not run this message.', +}; + +/** + * Assistant error names that can never be retried. Pinned to the exact names + * in `packages/app-shared/src/opencode.gen.ts`. + */ +export const NON_RETRYABLE_ASSISTANT_ERRORS: readonly string[] = [ + 'ProviderAuthError', + 'MessageAbortedError', + 'ContextOverflowError', +]; + +/** + * Fixed, safe copy for a known assistant error name. Unknown names fall back + * to the generic line. Never surfaces `error.data` or provider message text. + */ +const ASSISTANT_DETAIL_BY_ERROR_NAME: Readonly> = { + ProviderAuthError: 'The provider rejected the request.', + MessageAbortedError: 'The response was stopped.', + ContextOverflowError: 'The conversation is too long for the model.', +}; + +export type MessageFailure = { + kind: 'delivery' | 'assistant'; + title: string; + detail: string; + canRetry: boolean; + canCopy: boolean; +}; + +export function selectMessageFailure(input: { + deliveryState?: MessageDeliveryState; + info: MessageInfo; +}): MessageFailure | null { + const { deliveryState, info } = input; + + if (info.role === 'user' && deliveryState?.status === 'failed') { + return { + kind: 'delivery', + title: 'Failed to deliver', + detail: DELIVERY_DETAIL_BY_REASON[deliveryState.reason], + canRetry: true, + canCopy: true, + }; + } + + if (info.role === 'assistant' && info.error) { + const errorName = info.error.name; + return { + kind: 'assistant', + title: 'Response failed', + detail: ASSISTANT_DETAIL_BY_ERROR_NAME[errorName] ?? 'The response failed.', + canRetry: !NON_RETRYABLE_ASSISTANT_ERRORS.includes(errorName), + canCopy: false, + }; + } + + return null; +} From ebf2d870d923118460ebbc35a15e00ad08ff3c58 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20=C5=A0=C4=87eki=C4=87?= Date: Wed, 19 Aug 2026 03:02:52 +0200 Subject: [PATCH 05/34] feat(mobile): bound agent session queries and defer cache maintenance Wrap the stored-list and search builders with withInfiniteRetention, reconcile the sessions list to page one instead of a blanket invalidation, and defer the onSettled and departure maintenance behind InteractionManager so navigation never waits on cache work. --- .../src/lib/agent-session-cache.test.ts | 17 +++++- apps/mobile/src/lib/agent-session-cache.ts | 17 +++++- .../src/lib/hooks/use-agent-sessions.test.ts | 41 ++++++++++++- .../src/lib/hooks/use-agent-sessions.ts | 57 ++++++++++++------- .../lib/hooks/use-session-mutations.test.ts | 19 ++++++- .../src/lib/hooks/use-session-mutations.ts | 7 ++- 6 files changed, 128 insertions(+), 30 deletions(-) diff --git a/apps/mobile/src/lib/agent-session-cache.test.ts b/apps/mobile/src/lib/agent-session-cache.test.ts index 40764371de..a04a364f70 100644 --- a/apps/mobile/src/lib/agent-session-cache.test.ts +++ b/apps/mobile/src/lib/agent-session-cache.test.ts @@ -2,13 +2,23 @@ import { describe, expect, it, vi } from 'vitest'; import { invalidateAgentSessionQueries } from '@/lib/agent-session-cache'; +const reconcileFirstPageMock = + vi.fn<(queryClient: unknown, queryKeyPrefix: readonly unknown[]) => void>(); + +vi.mock('@/lib/query/infinite-retention', () => ({ + reconcileFirstPage: (queryClient: unknown, queryKeyPrefix: readonly unknown[]) => { + reconcileFirstPageMock(queryClient, queryKeyPrefix); + }, +})); + describe('invalidateAgentSessionQueries', () => { - it('invalidates session list, recent repository, and active session queries', async () => { + it('reconciles the session list to page one and invalidates recent repository and active session queries', async () => { const listFilter = { queryKey: ['cliSessionsV2', 'list'] }; const recentRepositoriesFilter = { queryKey: ['cliSessionsV2', 'recentRepositories'] }; const activeListFilter = { queryKey: ['activeSessions', 'list'] }; const queryClient = { invalidateQueries: vi.fn().mockResolvedValue(undefined), + setQueriesData: vi.fn(), }; const trpc = { cliSessionsV2: { @@ -22,7 +32,10 @@ describe('invalidateAgentSessionQueries', () => { await invalidateAgentSessionQueries(queryClient, trpc); - expect(queryClient.invalidateQueries).toHaveBeenCalledWith(listFilter); + // The stored list is reconciled to page one, not wholesale-invalidated. + expect(reconcileFirstPageMock).toHaveBeenCalledWith(queryClient, listFilter.queryKey); + expect(queryClient.invalidateQueries).not.toHaveBeenCalledWith(listFilter); + // The other two keys keep their plain invalidation. expect(queryClient.invalidateQueries).toHaveBeenCalledWith(recentRepositoriesFilter); expect(queryClient.invalidateQueries).toHaveBeenCalledWith(activeListFilter); }); diff --git a/apps/mobile/src/lib/agent-session-cache.ts b/apps/mobile/src/lib/agent-session-cache.ts index a1e237a3a7..477a25c862 100644 --- a/apps/mobile/src/lib/agent-session-cache.ts +++ b/apps/mobile/src/lib/agent-session-cache.ts @@ -1,12 +1,20 @@ import { type QueryClient } from '@tanstack/react-query'; +import { reconcileFirstPage } from '@/lib/query/infinite-retention'; + type QueryPathFilter = { pathFilter: () => Parameters[0]; }; +type ListQueryPathFilter = { + pathFilter: () => Parameters[0] & { + queryKey: readonly unknown[]; + }; +}; + type AgentSessionTrpcQueries = { cliSessionsV2: { - list: QueryPathFilter; + list: ListQueryPathFilter; recentRepositories: QueryPathFilter; }; activeSessions: { @@ -15,11 +23,14 @@ type AgentSessionTrpcQueries = { }; export async function invalidateAgentSessionQueries( - queryClient: Pick, + queryClient: Pick, trpc: AgentSessionTrpcQueries ): Promise { + // Trim the stored list to page one (dropping later pages) and refetch that + // page. Runs on rename, delete, and create, where the first page is the one + // that changed. + reconcileFirstPage(queryClient as QueryClient, trpc.cliSessionsV2.list.pathFilter().queryKey); await Promise.all([ - queryClient.invalidateQueries(trpc.cliSessionsV2.list.pathFilter()), queryClient.invalidateQueries(trpc.cliSessionsV2.recentRepositories.pathFilter()), queryClient.invalidateQueries(trpc.activeSessions.list.pathFilter()), ]); diff --git a/apps/mobile/src/lib/hooks/use-agent-sessions.test.ts b/apps/mobile/src/lib/hooks/use-agent-sessions.test.ts index 728d1b8cac..e4cab76f5c 100644 --- a/apps/mobile/src/lib/hooks/use-agent-sessions.test.ts +++ b/apps/mobile/src/lib/hooks/use-agent-sessions.test.ts @@ -5,7 +5,10 @@ import { buildAgentSessionListInput, buildAgentSessionSearchInput, } from '@/lib/agent-session-input'; -import { buildStoredSessionsQueryOptions } from '@/lib/hooks/use-agent-sessions'; +import { + buildAgentSessionSearchQueryOptions, + buildStoredSessionsQueryOptions, +} from '@/lib/hooks/use-agent-sessions'; // The hook module transitively imports react-native (via the user-web- // connection lifecycle) and the real tRPC client (via `@/lib/trpc`), which the @@ -23,11 +26,20 @@ vi.mock('@/lib/active-sessions-live-sync', () => ({ refreshActiveSessionsNow: vi.fn().mockResolvedValue(false), })); +vi.mock('react-native', () => ({ + InteractionManager: { runAfterInteractions: vi.fn() }, +})); + function createTrpcStub(infiniteQueryOptions: unknown) { const stub = { cliSessionsV2: { list: { infiniteQueryOptions } } }; return stub as never; } +function createSearchTrpcStub(infiniteQueryOptions: unknown) { + const stub = { cliSessionsV2: { search: { infiniteQueryOptions } } }; + return stub as never; +} + describe('buildAgentSessionListInput', () => { it('defaults to updated_at when sortBy is omitted (matches pre-feature behavior)', () => { expect( @@ -198,4 +210,31 @@ describe('buildStoredSessionsQueryOptions', () => { expect(result.enabled).toBe(false); }); + + it('carries a numeric maxPages retention bound', () => { + const infiniteQueryOptions = vi.fn((_input: unknown, options: object) => options); + const result = buildStoredSessionsQueryOptions(createTrpcStub(infiniteQueryOptions), {}); + + expect(result.maxPages).toBeTypeOf('number'); + }); +}); + +describe('buildAgentSessionSearchQueryOptions', () => { + it('carries a numeric maxPages retention bound', () => { + const infiniteQueryOptions = vi.fn((_input: unknown, options: object) => options); + const result = buildAgentSessionSearchQueryOptions(createSearchTrpcStub(infiniteQueryOptions), { + searchQuery: 'hello', + }); + + expect(result.maxPages).toBeTypeOf('number'); + }); + + it('keeps the search-text gating (enabled passthrough)', () => { + const infiniteQueryOptions = vi.fn((_input: unknown, options: object) => options); + const result = buildAgentSessionSearchQueryOptions(createSearchTrpcStub(infiniteQueryOptions), { + searchQuery: 'hello', + }); + + expect(result.enabled).toBe(true); + }); }); diff --git a/apps/mobile/src/lib/hooks/use-agent-sessions.ts b/apps/mobile/src/lib/hooks/use-agent-sessions.ts index 524387b1d6..45564f9ec8 100644 --- a/apps/mobile/src/lib/hooks/use-agent-sessions.ts +++ b/apps/mobile/src/lib/hooks/use-agent-sessions.ts @@ -21,6 +21,7 @@ import { DEFAULT_AGENT_SESSION_SORT, parseAgentSessionSortBy, } from '@/lib/agent-session-sort'; +import { scheduleCacheMaintenance, withInfiniteRetention } from '@/lib/query/infinite-retention'; import { useTRPC } from '@/lib/trpc'; import { useUserWebConnectionState } from '@/lib/hooks/use-user-web-connection-state'; @@ -100,17 +101,19 @@ export function buildStoredSessionsQueryOptions( trpc: ReturnType, options?: UseAgentSessionsOptions ) { - return trpc.cliSessionsV2.list.infiniteQueryOptions(buildAgentSessionListInput(options ?? {}), { - staleTime: 30_000, - enabled: options?.enabled, - getNextPageParam: lastPage => lastPage.nextCursor, - // Native window-focus refetch stays on by default so Home and the Share - // Gate keep their OS-foreground refresh. The Agents list opts out: its - // screen runs an AppState 'active' callback through the wrapped refetch, - // so the native query lifecycle must not start a stored refetch that - // bypasses the operation coordinator shared with backfill and departure. - refetchOnWindowFocus: options?.refetchOnWindowFocus ?? true, - }); + return withInfiniteRetention( + trpc.cliSessionsV2.list.infiniteQueryOptions(buildAgentSessionListInput(options ?? {}), { + staleTime: 30_000, + enabled: options?.enabled, + getNextPageParam: lastPage => lastPage.nextCursor, + // Native window-focus refetch stays on by default so Home and the Share + // Gate keep their OS-foreground refresh. The Agents list opts out: its + // screen runs an AppState 'active' callback through the wrapped refetch, + // so the native query lifecycle must not start a stored refetch that + // bypasses the operation coordinator shared with backfill and departure. + refetchOnWindowFocus: options?.refetchOnWindowFocus ?? true, + }) + ); } function useStoredSessions(options?: UseAgentSessionsOptions) { @@ -161,6 +164,25 @@ type UseAgentSessionSearchOptions = UseAgentSessionsOptions & { searchQuery: string; }; +/** + * Build the server-side session-search infinite-query options. Extracted as a + * pure builder (mirroring `buildStoredSessionsQueryOptions`) so the options + * are executable-tested without mounting the hook. + */ +export function buildAgentSessionSearchQueryOptions( + trpc: ReturnType, + options: UseAgentSessionSearchOptions +) { + return withInfiniteRetention( + trpc.cliSessionsV2.search.infiniteQueryOptions(buildAgentSessionSearchInput(options), { + staleTime: 30_000, + enabled: (options.enabled ?? true) && options.searchQuery.length > 0, + placeholderData: keepPreviousData, + getNextPageParam: lastPage => lastPage.nextCursor, + }) + ); +} + /** * Server-side session search, now cursor-paginated for consistent * page size and dedupe across pages. Uses `useInfiniteQuery` with @@ -171,14 +193,7 @@ export function useAgentSessionSearch(options: UseAgentSessionSearchOptions) { const trpc = useTRPC(); const sortBy = resolveSortBy(options.sortBy); - const query = useInfiniteQuery( - trpc.cliSessionsV2.search.infiniteQueryOptions(buildAgentSessionSearchInput(options), { - staleTime: 30_000, - enabled: (options.enabled ?? true) && options.searchQuery.length > 0, - placeholderData: keepPreviousData, - getNextPageParam: lastPage => lastPage.nextCursor, - }) - ); + const query = useInfiniteQuery(buildAgentSessionSearchQueryOptions(trpc, options)); const sessions = useMemo(() => collectSearchPages(query.data?.pages), [query.data]); const dateGroups = useMemo(() => groupAgentSessionsByDate(sessions, sortBy), [sessions, sortBy]); @@ -286,7 +301,9 @@ export function useAgentSessions(options?: UseAgentSessionsOptions) { } } if (departedId) { - void storedRefetch(); + scheduleCacheMaintenance(() => { + void storedRefetch(); + }); } }, [activeSessionIds, storedRefetch]); diff --git a/apps/mobile/src/lib/hooks/use-session-mutations.test.ts b/apps/mobile/src/lib/hooks/use-session-mutations.test.ts index 013c0db98b..eb58fc5f4b 100644 --- a/apps/mobile/src/lib/hooks/use-session-mutations.test.ts +++ b/apps/mobile/src/lib/hooks/use-session-mutations.test.ts @@ -32,6 +32,7 @@ const setQueriesDataMock = vi.fn(); const setQueryDataMock = vi.fn(); const invalidateQueriesMock = vi.fn(); const invalidateAgentSessionsMock = vi.fn(); +const scheduleCacheMaintenanceMock = vi.fn<(run: () => void) => void>(); const toastErrorMock = vi.fn(); const toastSuccessMock = vi.fn(); // eslint-disable-next-line typescript-eslint/promise-function-async -- conflicting require-await rule @@ -87,6 +88,12 @@ vi.mock('@/lib/agent-session-cache', () => ({ }, })); +vi.mock('@/lib/query/infinite-retention', () => ({ + scheduleCacheMaintenance: (run: () => void) => { + scheduleCacheMaintenanceMock(run); + }, +})); + vi.mock('sonner-native', () => ({ toast: { error: (msg: string) => toastErrorMock(msg) }, })); @@ -115,6 +122,7 @@ describe('useSessionMutations', () => { setQueryDataMock.mockReset(); invalidateQueriesMock.mockReset(); invalidateAgentSessionsMock.mockReset(); + scheduleCacheMaintenanceMock.mockReset(); toastErrorMock.mockReset(); toastSuccessMock.mockReset(); chainSaveMock.mockClear(); @@ -241,10 +249,17 @@ describe('useSessionMutations', () => { expect(toastErrorMock).toHaveBeenCalledWith('rename failed'); }); - it('onSettled still invalidates agent session queries', async () => { + it('onSettled still invalidates agent session queries', () => { useSessionMutations(); const options = capturedOptions.rename; - await options?.onSettled?.(); + options?.onSettled?.(); + + // The invalidation is deferred behind the interaction scheduler; drive + // the injected callback to run it in this turn. + const scheduled = scheduleCacheMaintenanceMock.mock.calls[0]?.[0]; + expect(scheduled).toBeTypeOf('function'); + scheduled?.(); + expect(invalidateAgentSessionsMock).toHaveBeenCalled(); }); }); diff --git a/apps/mobile/src/lib/hooks/use-session-mutations.ts b/apps/mobile/src/lib/hooks/use-session-mutations.ts index 1cd86dae06..ef09a24e7b 100644 --- a/apps/mobile/src/lib/hooks/use-session-mutations.ts +++ b/apps/mobile/src/lib/hooks/use-session-mutations.ts @@ -4,6 +4,7 @@ import { invalidateAgentSessionQueries } from '@/lib/agent-session-cache'; import { applyActiveSessionTitle, type CachedActiveSessionsData } from '@/lib/active-sessions-live'; import { announcingToast } from '@/lib/a11y/announcing-toast'; import { chainSave } from '@/lib/hooks/save-chain'; +import { scheduleCacheMaintenance } from '@/lib/query/infinite-retention'; import { mapStoredSessions, removeStoredSession, @@ -23,8 +24,10 @@ export function useSessionMutations() { const listKey = trpc.cliSessionsV2.list.infiniteQueryKey(); const activeListFilter = trpc.activeSessions.list.pathFilter(); - const invalidateSessions = async () => { - await invalidateAgentSessionQueries(queryClient, trpc); + const invalidateSessions = () => { + scheduleCacheMaintenance(() => { + void invalidateAgentSessionQueries(queryClient, trpc); + }); }; const snapshotAndUpdate = async ( From 318c87180c7ba9dec54795b7582193d136d09276 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20=C5=A0=C4=87eki=C4=87?= Date: Wed, 19 Aug 2026 03:27:23 +0200 Subject: [PATCH 06/34] perf(cloud-agent-sdk): publish row-local part deltas via partsRevision Replace the parts copy-on-write with a stable map plus a partsRevision atom so a token delta no longer clones the whole parts map. Derived atoms read both parts and partsRevision, and a StoredMessage memo keeps unchanged rows' identity. --- .../src/session-manager.test.ts | 149 +++++++++++++++++- .../cloud-agent-sdk/src/session-manager.ts | 39 ++++- .../src/storage/jotai.bench.test.ts | 52 ++++++ .../cloud-agent-sdk/src/storage/jotai.test.ts | 7 +- packages/cloud-agent-sdk/src/storage/jotai.ts | 59 ++++--- 5 files changed, 275 insertions(+), 31 deletions(-) create mode 100644 packages/cloud-agent-sdk/src/storage/jotai.bench.test.ts diff --git a/packages/cloud-agent-sdk/src/session-manager.test.ts b/packages/cloud-agent-sdk/src/session-manager.test.ts index 653d014298..10e7999cad 100644 --- a/packages/cloud-agent-sdk/src/session-manager.test.ts +++ b/packages/cloud-agent-sdk/src/session-manager.test.ts @@ -23,7 +23,7 @@ import type { CloudAgentSessionDismissSuggestionInput, } from './session'; import type { JotaiSessionStorage } from './storage/jotai'; -import type { AssistantMessage, UserMessage } from '@kilocode/app-shared/opencode'; +import type { AssistantMessage, UserMessage, TextPart } from '@kilocode/app-shared/opencode'; import { kiloId, cloudAgentId, stubUserMessage, stubTextPart, makeSnapshot } from './test-helpers'; import type { CloudStatus, @@ -2218,6 +2218,153 @@ describe('createSessionManager', () => { }); }); + describe('StoredMessage memoization', () => { + it('completed rows keep object identity across a delta on another row', async () => { + const config = createMockConfig(); + const mgr = createSessionManager(config); + + mockSession.connect.mockImplementation(() => { + mockSessionCallbacks.onSessionCreated?.({ id: 'ses-root' }); + }); + + await mgr.switchSession(kiloId('ses-root')); + if (!latestStorage) throw new Error('expected session storage'); + + const completed = createStoredMessage('msg-completed', 'ses-root', 'assistant'); + const streaming = createStoredMessage('msg-streaming', 'ses-root', 'assistant'); + const completedPart = stubTextPart({ + id: 'part-completed', + sessionID: 'ses-root', + messageID: completed.info.id, + text: 'done', + }); + const streamingPart = stubTextPart({ + id: 'part-streaming', + sessionID: 'ses-root', + messageID: streaming.info.id, + text: 'hel', + }); + + latestStorage.upsertMessage(completed.info); + latestStorage.upsertMessage(streaming.info); + latestStorage.upsertPart(completed.info.id, completedPart); + latestStorage.upsertPart(streaming.info.id, streamingPart); + + const before = atomValue(config.store, mgr.atoms.messagesList); + const completedBefore = before.find(m => m.info.id === completed.info.id); + expect(completedBefore).toBeDefined(); + const streamingBefore = before.find(m => m.info.id === streaming.info.id); + expect(streamingBefore).toBeDefined(); + + // A delta on the streaming row must not rebuild the completed row. + latestStorage.applyPartDelta(streaming.info.id, streamingPart.id, 'text', 'lo'); + + const after = atomValue(config.store, mgr.atoms.messagesList); + const completedAfter = after.find(m => m.info.id === completed.info.id); + expect(completedAfter).toBeDefined(); + expect(completedAfter).toBe(completedBefore); + + // The streaming row must rebuild (new object, updated text). This proves + // `partsRevision` drove the recompute: `applyPartDelta` changes neither + // `messageIds`, `messages`, nor the `parts` map reference, so without + // reading `partsRevision` the atom would return the stale array and this + // assertion would fail. + const streamingAfter = after.find(m => m.info.id === streaming.info.id); + expect(streamingAfter).toBeDefined(); + expect(streamingAfter).not.toBe(streamingBefore); + expect((streamingAfter?.parts[0] as TextPart | undefined)?.text).toBe('hello'); + }); + + it('a no-parts row keeps object identity across a delta on another row', async () => { + const config = createMockConfig(); + const mgr = createSessionManager(config); + + mockSession.connect.mockImplementation(() => { + mockSessionCallbacks.onSessionCreated?.({ id: 'ses-root' }); + }); + + await mgr.switchSession(kiloId('ses-root')); + if (!latestStorage) throw new Error('expected session storage'); + + const noParts = createStoredMessage('msg-no-parts', 'ses-root', 'assistant'); + const withParts = createStoredMessage('msg-with-parts', 'ses-root', 'assistant'); + const withPartsPart = stubTextPart({ + id: 'part-with-parts', + sessionID: 'ses-root', + messageID: withParts.info.id, + text: 'hel', + }); + + // The no-parts row gets a message but no parts entry, so the memo must + // fall back to the shared EMPTY_PARTS sentinel. + latestStorage.upsertMessage(noParts.info); + latestStorage.upsertMessage(withParts.info); + latestStorage.upsertPart(withParts.info.id, withPartsPart); + + const before = atomValue(config.store, mgr.atoms.messagesList); + const noPartsBefore = before.find(m => m.info.id === noParts.info.id); + expect(noPartsBefore).toBeDefined(); + + // A delta on the other row bumps `partsRevision` and recomputes the list. + latestStorage.applyPartDelta(withParts.info.id, withPartsPart.id, 'text', 'lo'); + + const after = atomValue(config.store, mgr.atoms.messagesList); + const noPartsAfter = after.find(m => m.info.id === noParts.info.id); + expect(noPartsAfter).toBeDefined(); + + // The no-parts row must keep object identity. If EMPTY_PARTS were a + // fresh `[]` per recompute, `cached.parts === parts` would fail and this + // row would rebuild a new StoredMessage on every partsRevision bump. + expect(noPartsAfter).toBe(noPartsBefore); + }); + + it('childMessagesAtom recomputes after a child-row part delta', async () => { + const config = createMockConfig(); + const mgr = createSessionManager(config); + + mockSession.connect.mockImplementation(() => { + mockSessionCallbacks.onSessionCreated?.({ id: 'ses-root' }); + }); + + await mgr.switchSession(kiloId('ses-root')); + if (!latestStorage) throw new Error('expected session storage'); + + const child = createStoredMessage('msg-child', 'child-1', 'assistant'); + const childPart = stubTextPart({ + id: 'part-child', + sessionID: 'child-1', + messageID: child.info.id, + text: 'hel', + }); + + latestStorage.upsertMessage(child.info); + latestStorage.upsertPart(child.info.id, childPart); + + const childMessagesBefore = atomValue<(childSessionId: string) => StoredMessage[]>( + config.store, + mgr.atoms.childMessages + ); + expect((childMessagesBefore('child-1')[0]?.parts[0] as TextPart | undefined)?.text).toBe( + 'hel' + ); + + latestStorage.applyPartDelta(child.info.id, childPart.id, 'text', 'lo'); + + const childMessagesAfter = atomValue<(childSessionId: string) => StoredMessage[]>( + config.store, + mgr.atoms.childMessages + ); + + // The atom must emit a new function so subscribers re-render (the + // live-sheet freeze path). Without reading `partsRevision`, the cached + // function reference is returned and this assertion fails. + expect(childMessagesAfter).not.toBe(childMessagesBefore); + expect((childMessagesAfter('child-1')[0]?.parts[0] as TextPart | undefined)?.text).toBe( + 'hello' + ); + }); + }); + describe('context usage', () => { it('exposes token footprint and runtime model identity from the root assistant response', async () => { const config = createMockConfig(); diff --git a/packages/cloud-agent-sdk/src/session-manager.ts b/packages/cloud-agent-sdk/src/session-manager.ts index c793b5f22e..a95b96ba73 100644 --- a/packages/cloud-agent-sdk/src/session-manager.ts +++ b/packages/cloud-agent-sdk/src/session-manager.ts @@ -137,6 +137,13 @@ const UUID_RE = /^[0-9a-f]{8}-[0-9a-f]{4}-[1-5][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0- const TRANSCRIPT_CLEARED_INDICATOR = 'View cleared — earlier messages are still on this session'; +/** + * Shared empty-parts sentinel. `memoizedStoredMessage` compares `cached.parts + * === parts`; `partsMap.get(id) ?? []` would allocate a fresh array on every + * `partsRevision` bump, defeating the memo for rows that have no parts entry. + */ +const EMPTY_PARTS: Part[] = []; + /** * Flatten a `ModelSelection` into the Decision 5 create_session model object. * `variant` is nested only when present (no second top-level field). @@ -604,6 +611,28 @@ function createSessionManager(config: SessionManagerConfig): SessionManager { const olderMessagesOmittedItemCountAtom = atom(0); const transcriptClearedAtom = atom(false); + // Memoized per-row StoredMessage objects. Reused while both `info` and the + // parts array keep the same reference, so unchanged rows keep object identity + // across a delta on another row (React.memo relies on this). + const storedMessageMemo = new Map(); + + function memoizedStoredMessage(id: string, info: MessageInfo, parts: Part[]): StoredMessage { + const cached = storedMessageMemo.get(id); + if (cached !== undefined && cached.info === info && cached.parts === parts) { + return cached; + } + const next: StoredMessage = { info, parts }; + storedMessageMemo.set(id, next); + return next; + } + + function pruneStoredMessageMemo(ids: readonly string[]): void { + const idSet = new Set(ids); + for (const id of storedMessageMemo.keys()) { + if (!idSet.has(id)) storedMessageMemo.delete(id); + } + } + // Derived atoms const messagesListAtom = atom(get => { const storage = get(sessionStorageAtom); @@ -611,14 +640,16 @@ function createSessionManager(config: SessionManagerConfig): SessionManager { const ids = get(storage.atoms.messageIds); const msgMap = get(storage.atoms.messages); const partsMap = get(storage.atoms.parts); + get(storage.atoms.partsRevision); const rootSessionId = get(rootSessionIdAtom); const out: StoredMessage[] = []; for (const id of ids) { const info = msgMap.get(id); if (!info) continue; if (rootSessionId !== null && info.sessionID !== rootSessionId) continue; - out.push({ info, parts: partsMap.get(id) ?? [] }); + out.push(memoizedStoredMessage(id, info, partsMap.get(id) ?? EMPTY_PARTS)); } + pruneStoredMessageMemo(ids); return out; }); @@ -641,11 +672,14 @@ function createSessionManager(config: SessionManagerConfig): SessionManager { const ids = get(storage.atoms.messageIds); const msgMap = get(storage.atoms.messages); const partsMap = get(storage.atoms.parts); + get(storage.atoms.partsRevision); + pruneStoredMessageMemo(ids); return (childSessionId: string): StoredMessage[] => { const out: StoredMessage[] = []; for (const id of ids) { const info = msgMap.get(id); - if (info?.sessionID === childSessionId) out.push({ info, parts: partsMap.get(id) ?? [] }); + if (info?.sessionID === childSessionId) + out.push(memoizedStoredMessage(id, info, partsMap.get(id) ?? EMPTY_PARTS)); } return out; }; @@ -733,6 +767,7 @@ function createSessionManager(config: SessionManagerConfig): SessionManager { function clearAllAtoms(): void { store.set(sessionStorageAtom, null); + storedMessageMemo.clear(); store.set(rootSessionIdAtom, null); store.set(isStreamingAtom, false); store.set(isLoadingAtom, false); diff --git a/packages/cloud-agent-sdk/src/storage/jotai.bench.test.ts b/packages/cloud-agent-sdk/src/storage/jotai.bench.test.ts new file mode 100644 index 0000000000..f9aa8ff3a3 --- /dev/null +++ b/packages/cloud-agent-sdk/src/storage/jotai.bench.test.ts @@ -0,0 +1,52 @@ +import { createStore } from 'jotai'; +import { createJotaiStorage } from './jotai'; +import type { Part } from '@kilocode/app-shared/opencode'; +import type { MessageInfo } from '../types'; + +function makeMsg(id: string): MessageInfo { + return { + id, + sessionID: 'ses-bench', + role: 'user', + time: { created: 1 }, + agent: 'build', + model: { providerID: 'a', modelID: 'b' }, + } as MessageInfo; +} + +function makePart(id: string, messageId: string): Part { + return { id, sessionID: 'ses-bench', messageID: messageId, type: 'text', text: 'hello' } as Part; +} + +test('BENCH jotai-parts', () => { + const store = createStore(); + const s = createJotaiStorage(store); + + // 200 messages, each with 3 parts. + for (let m = 0; m < 200; m++) { + const mid = `msg-${m}`; + s.upsertMessage(makeMsg(mid)); + for (let p = 0; p < 3; p++) { + s.upsertPart(mid, makePart(`part-${m}-${p}`, mid)); + } + } + + // Count parts-map writes (Map allocations) by wrapping store.set. + let partsMapWrites = 0; + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const originalSet: any = store.set.bind(store); + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (store as any).set = (atom: unknown, value: unknown) => { + if (atom === s.atoms.parts) partsMapWrites += 1; + return originalSet(atom, value); + }; + + const start = Date.now(); + for (let i = 0; i < 2000; i++) { + s.applyPartDelta('msg-199', 'part-199-0', 'text', 'x'); + } + const elapsedMs = Date.now() - start; + + console.log(`BENCH jotai-parts elapsedMs=${elapsedMs} partsMapWrites=${partsMapWrites}`); + expect(true).toBe(true); +}); diff --git a/packages/cloud-agent-sdk/src/storage/jotai.test.ts b/packages/cloud-agent-sdk/src/storage/jotai.test.ts index c6e288b2e3..b4814e588a 100644 --- a/packages/cloud-agent-sdk/src/storage/jotai.test.ts +++ b/packages/cloud-agent-sdk/src/storage/jotai.test.ts @@ -330,12 +330,15 @@ describe('createJotaiStorage', () => { expect(first).not.toBe(second); }); - test('parts atom gets new Map reference on each mutation', () => { + test('parts atom keeps one Map reference and bumps partsRevision', () => { s.upsertPart('msg-1', makePart('p-1', 'msg-1')); const first = store.get(s.atoms.parts); + const firstRev = store.get(s.atoms.partsRevision); s.upsertPart('msg-1', makePart('p-2', 'msg-1')); const second = store.get(s.atoms.parts); - expect(first).not.toBe(second); + const secondRev = store.get(s.atoms.partsRevision); + expect(first).toBe(second); + expect(secondRev).toBe(firstRev + 1); }); test('clear resets all atoms', () => { diff --git a/packages/cloud-agent-sdk/src/storage/jotai.ts b/packages/cloud-agent-sdk/src/storage/jotai.ts index e58ff38998..e782be3883 100644 --- a/packages/cloud-agent-sdk/src/storage/jotai.ts +++ b/packages/cloud-agent-sdk/src/storage/jotai.ts @@ -24,22 +24,33 @@ type JotaiSessionStorage = SessionStorage & { messageIds: Atom; messages: Atom>; parts: Atom>; + partsRevision: Atom; }; }; function createJotaiStorage(store: JotaiStore): JotaiSessionStorage { const messageIdsAtom = atom([]); const messagesAtom = atom>(new Map()); - const partsAtom = atom>(new Map()); + // Stable parts map. The atom holds this one reference for its lifetime; + // mutations write per-message arrays into it in place and publish via + // `partsRevisionAtom` instead of replacing the map. + const partsMap = new Map(); + const partsAtom = atom>(partsMap); + const partsRevisionAtom = atom(0); const partsSnapshot = new Map(); const subscribers = new Map void>>(); + function bumpPartsRevision(): void { + store.set(partsRevisionAtom, r => r + 1); + } + return { atoms: { messageIds: messageIdsAtom, messages: messagesAtom, parts: partsAtom, + partsRevision: partsRevisionAtom, }, upsertMessage(info) { @@ -65,12 +76,10 @@ function createJotaiStorage(store: JotaiStore): JotaiSessionStorage { }, upsertPart(messageId, part) { - const allParts = store.get(partsAtom); - const arr = allParts.get(messageId) ?? []; + const arr = partsMap.get(messageId) ?? []; const nextArr = upsertPartDroppingStaleSyntheticTextParts(arr, part); - const next = new Map(allParts); - next.set(messageId, nextArr); - store.set(partsAtom, next); + partsMap.set(messageId, nextArr); + bumpPartsRevision(); partsSnapshot.set(messageId, null); notify(subscribers, `parts:${messageId}`); }, @@ -80,17 +89,18 @@ function createJotaiStorage(store: JotaiStore): JotaiSessionStorage { return; } - const allParts = store.get(partsAtom); - const arr = allParts.get(messageId); + const arr = partsMap.get(messageId); - const next = new Map(allParts); if (!arr) { - next.set(messageId, [createSeedTextPart(messageId, partId, delta)]); + partsMap.set(messageId, [createSeedTextPart(messageId, partId, delta)]); } else { const idx = arr.findIndex(p => p.id === partId); const existing = idx >= 0 ? arr[idx] : undefined; if (!existing) { - next.set(messageId, insertPartSorted(arr, createSeedTextPart(messageId, partId, delta))); + partsMap.set( + messageId, + insertPartSorted(arr, createSeedTextPart(messageId, partId, delta)) + ); } else { const updatedPart = applyTextDelta(existing, delta); if (updatedPart === existing) { @@ -98,22 +108,20 @@ function createJotaiStorage(store: JotaiStore): JotaiSessionStorage { } const nextArr = [...arr]; nextArr[idx] = updatedPart; - next.set(messageId, nextArr); + partsMap.set(messageId, nextArr); } } - store.set(partsAtom, next); + bumpPartsRevision(); partsSnapshot.set(messageId, null); notify(subscribers, `parts:${messageId}`); }, deletePart(messageId, partId) { - const allParts = store.get(partsAtom); - const arr = allParts.get(messageId); + const arr = partsMap.get(messageId); if (!arr) return; const filtered = arr.filter(p => p.id !== partId); - const next = new Map(allParts); - next.set(messageId, filtered); - store.set(partsAtom, next); + partsMap.set(messageId, filtered); + bumpPartsRevision(); partsSnapshot.set(messageId, null); notify(subscribers, `parts:${messageId}`); }, @@ -122,7 +130,7 @@ function createJotaiStorage(store: JotaiStore): JotaiSessionStorage { const cached = partsSnapshot.get(messageId); if (cached) return cached; - const arr = store.get(partsAtom).get(messageId); + const arr = partsMap.get(messageId); if (!arr || arr.length === 0) return EMPTY_PARTS; const snapshot = arr.map(part => createReadonlyPartView(clonePart(part))); @@ -145,11 +153,12 @@ function createJotaiStorage(store: JotaiStore): JotaiSessionStorage { clear() { const existingMessageIds = store.get(messageIdsAtom); - const existingPartMessageIds = [...store.get(partsAtom).keys()]; + const existingPartMessageIds = [...partsMap.keys()]; store.set(messagesAtom, new Map()); store.set(messageIdsAtom, []); - store.set(partsAtom, new Map()); + partsMap.clear(); + bumpPartsRevision(); partsSnapshot.clear(); for (const messageId of existingMessageIds) { @@ -173,11 +182,9 @@ function createJotaiStorage(store: JotaiStore): JotaiSessionStorage { const nextMessageIds = messageIds.filter(id => id !== messageId); store.set(messageIdsAtom, nextMessageIds); - const allParts = store.get(partsAtom); - if (allParts.has(messageId)) { - const nextParts = new Map(allParts); - nextParts.delete(messageId); - store.set(partsAtom, nextParts); + if (partsMap.has(messageId)) { + partsMap.delete(messageId); + bumpPartsRevision(); partsSnapshot.delete(messageId); notify(subscribers, `parts:${messageId}`); } From b12e4fd82c3dfe5471051063d6f89d10fe1b8911 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20=C5=A0=C4=87eki=C4=87?= Date: Wed, 19 Aug 2026 03:27:27 +0200 Subject: [PATCH 07/34] feat(mobile): bound security and PR-review infinite queries Wrap the security-findings and PR-review owners with withInfiniteRetention and reconcile findings-list refreshes to page one. Keep offset pagination and the discussion conversation correct under the retention bound. --- .../lib/hooks/use-security-agent-commands.ts | 9 +- .../use-security-agent-mutations.test.ts | 4 + .../lib/hooks/use-security-agent-mutations.ts | 34 +++---- .../lib/hooks/use-security-findings.test.ts | 78 +++++++++++++-- .../src/lib/hooks/use-security-findings.ts | 59 +++++++++--- .../src/lib/hooks/use-security-remediation.ts | 14 ++- .../diff/pr-review-file-list-state.test.ts | 41 ++++++++ .../diff/pr-review-file-list-state.ts | 41 +++++--- .../use-pr-review-discussion-threads.test.ts | 81 ++++++++++++++++ .../use-pr-review-discussion-threads.ts | 95 ++++++++++++++++--- 10 files changed, 388 insertions(+), 68 deletions(-) create mode 100644 apps/mobile/src/lib/pr-review/diff/pr-review-file-list-state.test.ts create mode 100644 apps/mobile/src/lib/pr-review/discussion/use-pr-review-discussion-threads.test.ts diff --git a/apps/mobile/src/lib/hooks/use-security-agent-commands.ts b/apps/mobile/src/lib/hooks/use-security-agent-commands.ts index d8c6af51c4..882530416e 100644 --- a/apps/mobile/src/lib/hooks/use-security-agent-commands.ts +++ b/apps/mobile/src/lib/hooks/use-security-agent-commands.ts @@ -11,6 +11,7 @@ import { useEffect, useRef } from 'react'; import { type QueryClient, useQueries, useQuery, useQueryClient } from '@tanstack/react-query'; import { announcingToast } from '@/lib/a11y/announcing-toast'; +import { reconcileFirstPage, scheduleCacheMaintenance } from '@/lib/query/infinite-retention'; import { type SecurityCommand } from '@/lib/security-agent'; import { useTRPC } from '@/lib/trpc'; @@ -49,7 +50,9 @@ function invalidateSecurityQueryScopes( if (isPersonalSecurityScope(scope)) { const agent = trpc.securityAgent; if (scopeSet.has('findings')) { - void queryClient.invalidateQueries({ queryKey: agent.listFindings.queryKey() }); + scheduleCacheMaintenance(() => { + reconcileFirstPage(queryClient, agent.listFindings.queryKey()); + }); } if (scopeSet.has('findingDetails')) { void queryClient.invalidateQueries({ queryKey: agent.getFinding.queryKey() }); @@ -78,7 +81,9 @@ function invalidateSecurityQueryScopes( const agent = trpc.organizations.securityAgent; const ownerInput = { organizationId: scope }; if (scopeSet.has('findings')) { - void queryClient.invalidateQueries({ queryKey: agent.listFindings.queryKey(ownerInput) }); + scheduleCacheMaintenance(() => { + reconcileFirstPage(queryClient, agent.listFindings.queryKey(ownerInput)); + }); } if (scopeSet.has('findingDetails')) { void queryClient.invalidateQueries({ queryKey: agent.getFinding.queryKey(ownerInput) }); diff --git a/apps/mobile/src/lib/hooks/use-security-agent-mutations.test.ts b/apps/mobile/src/lib/hooks/use-security-agent-mutations.test.ts index 2c3b8ad7df..81d028dd0b 100644 --- a/apps/mobile/src/lib/hooks/use-security-agent-mutations.test.ts +++ b/apps/mobile/src/lib/hooks/use-security-agent-mutations.test.ts @@ -33,6 +33,10 @@ vi.mock('expo-crypto', () => ({ randomUUID: () => 'not-used', })); +vi.mock('react-native', () => ({ + InteractionManager: { runAfterInteractions: vi.fn() }, +})); + vi.mock('@/lib/operation-key', async importOriginal => { const actual = await importOriginal(); return { ...actual, useHoistedOperationKey: () => hoistedKeys }; diff --git a/apps/mobile/src/lib/hooks/use-security-agent-mutations.ts b/apps/mobile/src/lib/hooks/use-security-agent-mutations.ts index 6f593d5e1e..04aee2a0ef 100644 --- a/apps/mobile/src/lib/hooks/use-security-agent-mutations.ts +++ b/apps/mobile/src/lib/hooks/use-security-agent-mutations.ts @@ -10,6 +10,7 @@ import { } from '@/lib/operation-key'; import { useMutationOutbox } from '@/lib/persist/use-mutation-outbox'; import { classifyPrReviewMutationError } from '@/lib/pr-review/classify-pr-review-query-state'; +import { reconcileFirstPage, scheduleCacheMaintenance } from '@/lib/query/infinite-retention'; import { type FlattenedSecurityAgentConfig, type SecurityAgentConfig, @@ -163,26 +164,25 @@ export function useSaveSecurityAgentConfig(scope: string) { onSettled: async () => { await queryClient.invalidateQueries({ queryKey: configQueryKey }); if (isPersonalSecurityScope(scope)) { - await Promise.all([ - queryClient.invalidateQueries({ - queryKey: trpc.securityAgent.getDashboardStats.queryKey(), - }), - queryClient.invalidateQueries({ queryKey: trpc.securityAgent.listFindings.queryKey() }), - ]); + await queryClient.invalidateQueries({ + queryKey: trpc.securityAgent.getDashboardStats.queryKey(), + }); + scheduleCacheMaintenance(() => { + reconcileFirstPage(queryClient, trpc.securityAgent.listFindings.queryKey()); + }); return; } - await Promise.all([ - queryClient.invalidateQueries({ - queryKey: trpc.organizations.securityAgent.getDashboardStats.queryKey({ - organizationId: scope, - }), + await queryClient.invalidateQueries({ + queryKey: trpc.organizations.securityAgent.getDashboardStats.queryKey({ + organizationId: scope, }), - queryClient.invalidateQueries({ - queryKey: trpc.organizations.securityAgent.listFindings.queryKey({ - organizationId: scope, - }), - }), - ]); + }); + scheduleCacheMaintenance(() => { + reconcileFirstPage( + queryClient, + trpc.organizations.securityAgent.listFindings.queryKey({ organizationId: scope }) + ); + }); }, }); } diff --git a/apps/mobile/src/lib/hooks/use-security-findings.test.ts b/apps/mobile/src/lib/hooks/use-security-findings.test.ts index 0ba83ebd15..a9de592e43 100644 --- a/apps/mobile/src/lib/hooks/use-security-findings.test.ts +++ b/apps/mobile/src/lib/hooks/use-security-findings.test.ts @@ -12,8 +12,12 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import type * as OperationKeyModule from '@/lib/operation-key'; +import type * as SecurityAgentModule from '@kilocode/app-shared/security-agent'; +import { INFINITE_QUERY_MAX_PAGES } from '@/lib/query/infinite-retention'; import { + buildSecurityFindingsQueryOptions, dismissFindingIntentFingerprint, + type ListFindingsFilters, useDismissSecurityFinding, useStartSecurityAnalysis, } from './use-security-findings'; @@ -30,17 +34,24 @@ vi.mock('expo-crypto', () => ({ randomUUID: () => 'not-used', })); +vi.mock('react-native', () => ({ + InteractionManager: { runAfterInteractions: vi.fn() }, +})); + vi.mock('@/lib/operation-key', async importOriginal => { const actual = await importOriginal(); return { ...actual, useHoistedOperationKey: () => hoistedKeys }; }); -vi.mock('@kilocode/app-shared/security-agent', () => ({ - isPersonalSecurityScope: (scope: string) => scope === 'personal', - getNextSecurityFindingsOffset: () => undefined, - getRemediationUnavailableCopy: () => undefined, - isActiveRemediationStatus: () => false, -})); +vi.mock('@kilocode/app-shared/security-agent', async importOriginal => { + const actual = await importOriginal(); + return { + ...actual, + isPersonalSecurityScope: (scope: string) => scope === 'personal', + getRemediationUnavailableCopy: () => undefined, + isActiveRemediationStatus: () => false, + }; +}); vi.mock('@/lib/hooks/use-security-agent-commands', () => ({ trackSecurityAgentCommand: trackCommandMock, @@ -292,3 +303,58 @@ describe('dismissFindingIntentFingerprint (P1-A-08e changed-input)', () => { expect(dismissFindingIntentFingerprint(ORG_ID, DISMISS_VARS)).not.toBe(original); }); }); + +function createFindingsTrpcStub() { + const stub = { + securityAgent: { listFindings: { queryKey: () => ['securityAgent', 'listFindings'] } }, + organizations: { + securityAgent: { + listFindings: { queryKey: () => ['organizations', 'securityAgent', 'listFindings'] }, + }, + }, + }; + return stub as never; +} + +describe('buildSecurityFindingsQueryOptions (retention bound)', () => { + it('carries a numeric maxPages for a personal scope', () => { + const filters: ListFindingsFilters = {}; + const options = buildSecurityFindingsQueryOptions( + createFindingsTrpcStub(), + 'personal', + filters + ); + + expect(options.maxPages).toBe(INFINITE_QUERY_MAX_PAGES); + }); + + it('carries a numeric maxPages for an organization scope', () => { + const filters: ListFindingsFilters = {}; + const options = buildSecurityFindingsQueryOptions(createFindingsTrpcStub(), ORG_ID, filters); + + expect(typeof options.maxPages).toBe('number'); + }); +}); + +describe('buildSecurityFindingsQueryOptions (retention-safe pagination)', () => { + it('advances past the trimmed pages on a sixth page (no repeated offset)', () => { + const options = buildSecurityFindingsQueryOptions(createFindingsTrpcStub(), 'personal', {}); + const getNextPageParam = options.getNextPageParam as ( + lastPage: { findings: unknown[]; totalCount: number }, + pages: { findings: unknown[]; totalCount: number }[], + lastPageParam: number + ) => number | undefined; + + // React Query trims to the last 5 pages (maxPages) once a sixth page is + // fetched; the trimmed `pages` array must not drive the next offset. + const trimmedPages = Array.from({ length: 5 }, () => makeFindingsPage(50)); + + // Page 6 was fetched with offset 250; the next offset must be 300, not 250. + expect(getNextPageParam(makeFindingsPage(50), trimmedPages, 250)).toBe(300); + }); +}); + +const makeFindingsPage = (count: number) => ({ + findings: Array.from({ length: count }, (_, i) => ({ id: `f-${i}` })), + totalCount: 400, +}); diff --git a/apps/mobile/src/lib/hooks/use-security-findings.ts b/apps/mobile/src/lib/hooks/use-security-findings.ts index 95824e6d83..151936901f 100644 --- a/apps/mobile/src/lib/hooks/use-security-findings.ts +++ b/apps/mobile/src/lib/hooks/use-security-findings.ts @@ -12,6 +12,11 @@ import { mapSecurityDismissOperationError, } from '@/lib/hooks/use-security-agent-mutations'; import { useHoistedOperationKey } from '@/lib/operation-key'; +import { + reconcileFirstPage, + scheduleCacheMaintenance, + withInfiniteRetention, +} from '@/lib/query/infinite-retention'; import { type SecurityAnalysis } from '@/lib/security-agent'; import { trpcClient, useTRPC } from '@/lib/trpc'; @@ -19,20 +24,29 @@ import { trpcClient, useTRPC } from '@/lib/trpc'; // types even when structurally identical, so we always call both hooks (one // disabled) and return whichever is active. See use-code-reviewer.ts:32. -type ListFindingsFilters = Parameters[0]; +export type ListFindingsFilters = Parameters[0]; -export function useSecurityFindings(scope: string, filters: ListFindingsFilters) { - const trpc = useTRPC(); +type SecurityFindingsPage = Awaited>; + +/** + * Build the findings-list infinite-query options. Kept as a pure builder so + * the retention bound is testable without mounting the hook. + */ +export function buildSecurityFindingsQueryOptions( + trpc: ReturnType, + scope: string, + filters: ListFindingsFilters +) { const isPersonal = isPersonalSecurityScope(scope); const baseQueryKey = isPersonal ? trpc.securityAgent.listFindings.queryKey() : trpc.organizations.securityAgent.listFindings.queryKey({ organizationId: scope }); - return useInfiniteQuery({ + return withInfiniteRetention({ queryKey: [...baseQueryKey, filters], initialPageParam: filters.offset ?? 0, // eslint-disable-next-line typescript-eslint/promise-function-async -- conflicting require-await rule - queryFn: ({ pageParam }) => + queryFn: ({ pageParam }: { pageParam: number }) => isPersonal ? trpcClient.securityAgent.listFindings.query({ ...filters, offset: pageParam }) : trpcClient.organizations.securityAgent.listFindings.query({ @@ -40,13 +54,25 @@ export function useSecurityFindings(scope: string, filters: ListFindingsFilters) ...filters, offset: pageParam, }), - getNextPageParam: (lastPage, pages) => { - const loadedCount = pages.reduce((count, page) => count + page.findings.length, 0); - return getNextSecurityFindingsOffset(filters.offset ?? 0, loadedCount, lastPage.totalCount); - }, + // Derive the next offset from the last page's own offset, not by summing + // the retained `pages` array. `maxPages` trims the oldest page once the + // bound is exceeded, so summing `pages` undercounts and would repeat the + // same offset forever. `lastPageParam` is monotonic and trim-safe. + getNextPageParam: ( + lastPage: SecurityFindingsPage, + _pages: SecurityFindingsPage[], + lastPageParam: number + ) => + getNextSecurityFindingsOffset(lastPageParam, lastPage.findings.length, lastPage.totalCount), }); } +export function useSecurityFindings(scope: string, filters: ListFindingsFilters) { + const trpc = useTRPC(); + + return useInfiniteQuery(buildSecurityFindingsQueryOptions(trpc, scope, filters)); +} + export function useSecurityFinding(scope: string, id: string) { const trpc = useTRPC(); const personal = useQuery({ @@ -176,8 +202,10 @@ export function useStartSecurityAnalysis(scope: string) { queryClient.invalidateQueries({ queryKey: trpc.securityAgent.getFinding.queryKey({ id: vars.findingId }), }), - queryClient.invalidateQueries({ queryKey: trpc.securityAgent.listFindings.queryKey() }), ]); + scheduleCacheMaintenance(() => { + reconcileFirstPage(queryClient, trpc.securityAgent.listFindings.queryKey()); + }); return; } await Promise.all([ @@ -193,12 +221,13 @@ export function useStartSecurityAnalysis(scope: string) { id: vars.findingId, }), }), - queryClient.invalidateQueries({ - queryKey: trpc.organizations.securityAgent.listFindings.queryKey({ - organizationId: scope, - }), - }), ]); + scheduleCacheMaintenance(() => { + reconcileFirstPage( + queryClient, + trpc.organizations.securityAgent.listFindings.queryKey({ organizationId: scope }) + ); + }); }, }); } diff --git a/apps/mobile/src/lib/hooks/use-security-remediation.ts b/apps/mobile/src/lib/hooks/use-security-remediation.ts index d1a78cb47e..f1344bc906 100644 --- a/apps/mobile/src/lib/hooks/use-security-remediation.ts +++ b/apps/mobile/src/lib/hooks/use-security-remediation.ts @@ -8,6 +8,7 @@ import { import { useMutation, useQueryClient } from '@tanstack/react-query'; import { toast } from 'sonner-native'; +import { reconcileFirstPage, scheduleCacheMaintenance } from '@/lib/query/infinite-retention'; import { type SecurityAnalysis } from '@/lib/security-agent'; import { trpcClient, useTRPC } from '@/lib/trpc'; @@ -26,9 +27,11 @@ async function invalidateRemediationQueries( queryKey: trpc.securityAgent.getAnalysis.queryKey({ findingId }), }), queryClient.invalidateQueries({ queryKey: trpc.securityAgent.getFinding.queryKey() }), - queryClient.invalidateQueries({ queryKey: trpc.securityAgent.listFindings.queryKey() }), queryClient.invalidateQueries({ queryKey: trpc.securityAgent.getDashboardStats.queryKey() }), ]); + scheduleCacheMaintenance(() => { + reconcileFirstPage(queryClient, trpc.securityAgent.listFindings.queryKey()); + }); return; } const ownerInput = { organizationId: scope }; @@ -42,13 +45,16 @@ async function invalidateRemediationQueries( queryClient.invalidateQueries({ queryKey: trpc.organizations.securityAgent.getFinding.queryKey(ownerInput), }), - queryClient.invalidateQueries({ - queryKey: trpc.organizations.securityAgent.listFindings.queryKey(ownerInput), - }), queryClient.invalidateQueries({ queryKey: trpc.organizations.securityAgent.getDashboardStats.queryKey(ownerInput), }), ]); + scheduleCacheMaintenance(() => { + reconcileFirstPage( + queryClient, + trpc.organizations.securityAgent.listFindings.queryKey(ownerInput) + ); + }); } export function useStartSecurityRemediation(scope: string) { diff --git a/apps/mobile/src/lib/pr-review/diff/pr-review-file-list-state.test.ts b/apps/mobile/src/lib/pr-review/diff/pr-review-file-list-state.test.ts new file mode 100644 index 0000000000..345770a119 --- /dev/null +++ b/apps/mobile/src/lib/pr-review/diff/pr-review-file-list-state.test.ts @@ -0,0 +1,41 @@ +import { describe, expect, it, vi } from 'vitest'; + +import { buildPrReviewFileListQueryOptions } from './pr-review-file-list-state'; +import { PR_REVIEW_MAX_PAGES } from './pr-review-file-types'; + +// The hook module transitively imports react-native (via +// `@/lib/query/infinite-retention` and the viewed-files store) and the real +// tRPC client (via `@/lib/trpc`), which the node vitest pipeline cannot +// transform. The options builder itself is pure, so only the module-load chain +// needs these mocks; no hook is mounted. +vi.mock('react-native', () => ({ + InteractionManager: { runAfterInteractions: vi.fn() }, +})); + +vi.mock('@/lib/trpc', () => ({ + useTRPC: vi.fn(), +})); + +vi.mock('@/lib/pr-review/viewed-files', () => ({ + getViewedFiles: vi.fn(), + toggleViewedFile: vi.fn(), +})); + +function createTrpcStub(infiniteQueryOptions: unknown) { + const stub = { githubPrReview: { listFiles: { infiniteQueryOptions } } }; + return stub as never; +} + +describe('buildPrReviewFileListQueryOptions', () => { + it('carries a numeric maxPages bound to PR_REVIEW_MAX_PAGES', () => { + const infiniteQueryOptions = vi.fn((_input: unknown, options: object) => options); + const result = buildPrReviewFileListQueryOptions(createTrpcStub(infiniteQueryOptions), { + owner: 'octocat', + repo: 'hello', + number: 1, + enabled: true, + }); + + expect(result.maxPages).toBe(PR_REVIEW_MAX_PAGES); + }); +}); diff --git a/apps/mobile/src/lib/pr-review/diff/pr-review-file-list-state.ts b/apps/mobile/src/lib/pr-review/diff/pr-review-file-list-state.ts index 1bb64f64ee..b591dd8527 100644 --- a/apps/mobile/src/lib/pr-review/diff/pr-review-file-list-state.ts +++ b/apps/mobile/src/lib/pr-review/diff/pr-review-file-list-state.ts @@ -25,8 +25,35 @@ import { classifyPrReviewQueryState } from '@/lib/pr-review/classify-pr-review-q import { flattenFilePages } from '@/lib/pr-review/diff/dedupe-file-pages'; import { PR_REVIEW_MAX_PAGES } from '@/lib/pr-review/diff/pr-review-file-types'; import { getViewedFiles, toggleViewedFile } from '@/lib/pr-review/viewed-files'; +import { withInfiniteRetention } from '@/lib/query/infinite-retention'; import { useTRPC } from '@/lib/trpc'; +/** + * Build the file-list infinite-query options. Kept as a pure builder so the + * retention bound is testable without mounting the hook. + */ +export function buildPrReviewFileListQueryOptions( + trpc: ReturnType, + args: { owner: string; repo: string; number: number; enabled: boolean } +) { + const { owner, repo, number, enabled } = args; + return withInfiniteRetention( + trpc.githubPrReview.listFiles.infiniteQueryOptions( + { owner, repo, number }, + { + staleTime: 30_000, + enabled, + getNextPageParam: lastPage => lastPage.nextCursor ?? undefined, + } + ), + // Cap at the server's page ceiling so we never request page 61. + // 60 pages × 100/page = 6,000 files, which is well above the + // 3,000 truncation banner so fetch-to-completion still has + // headroom to actually finish. + PR_REVIEW_MAX_PAGES + ); +} + export function usePrReviewFileListQuery(args: { owner: string; repo: string; @@ -36,19 +63,7 @@ export function usePrReviewFileListQuery(args: { const { owner, repo, number, enabled } = args; const trpc = useTRPC(); const query = useInfiniteQuery( - trpc.githubPrReview.listFiles.infiniteQueryOptions( - { owner, repo, number }, - { - staleTime: 30_000, - enabled, - getNextPageParam: lastPage => lastPage.nextCursor ?? undefined, - // Cap at the server's page ceiling so we never request page 61. - // 60 pages × 100/page = 6,000 files, which is well above the - // 3,000 truncation banner so fetch-to-completion still has - // headroom to actually finish. - maxPages: PR_REVIEW_MAX_PAGES, - } - ) + buildPrReviewFileListQueryOptions(trpc, { owner, repo, number, enabled }) ); const errorState = query.error ? classifyPrReviewQueryState(query.error) : null; diff --git a/apps/mobile/src/lib/pr-review/discussion/use-pr-review-discussion-threads.test.ts b/apps/mobile/src/lib/pr-review/discussion/use-pr-review-discussion-threads.test.ts new file mode 100644 index 0000000000..f576b992d4 --- /dev/null +++ b/apps/mobile/src/lib/pr-review/discussion/use-pr-review-discussion-threads.test.ts @@ -0,0 +1,81 @@ +import { describe, expect, it, vi } from 'vitest'; + +import { INFINITE_QUERY_MAX_PAGES } from '@/lib/query/infinite-retention'; + +import { + buildPrReviewDiscussionThreadsQueryOptions, + retainConversation, + retainConversationAcrossMounts, +} from './use-pr-review-discussion-threads'; + +// The hook module transitively imports react-native (via +// `@/lib/query/infinite-retention`) and the real tRPC client (via +// `@/lib/trpc`), which the node vitest pipeline cannot transform. The options +// builder itself is pure, so only the module-load chain needs these mocks; no +// hook is mounted. +vi.mock('react-native', () => ({ + InteractionManager: { runAfterInteractions: vi.fn() }, +})); + +vi.mock('@/lib/trpc', () => ({ + useTRPC: vi.fn(), +})); + +function createTrpcStub(infiniteQueryOptions: unknown) { + const stub = { githubPrReview: { listReviewThreads: { infiniteQueryOptions } } }; + return stub as never; +} + +describe('buildPrReviewDiscussionThreadsQueryOptions', () => { + it('carries a numeric maxPages', () => { + const infiniteQueryOptions = vi.fn((_input: unknown, options: object) => options); + const result = buildPrReviewDiscussionThreadsQueryOptions( + createTrpcStub(infiniteQueryOptions), + { + owner: 'octocat', + repo: 'hello', + number: 1, + } + ); + + expect(result.maxPages).toBe(INFINITE_QUERY_MAX_PAGES); + }); +}); + +describe('retainConversation (retention-safe conversation)', () => { + const comment = { id: 'c1' }; + + it('returns the first-page conversation when it is present', () => { + const pages = [{ conversation: [comment] }]; + expect(retainConversation(pages, [])).toEqual([comment]); + }); + + it('keeps the retained conversation after the trim drops the first page', () => { + const retained = [comment]; + // After the retention trim, pages[0] is a later page with conversation: []. + const trimmedPages = [{ conversation: [] }]; + expect(retainConversation(trimmedPages, retained)).toBe(retained); + }); + + it('falls back to an empty list when nothing was ever retained', () => { + expect(retainConversation(undefined, [])).toEqual([]); + }); +}); + +describe('retainConversationAcrossMounts (remount survival)', () => { + const comment = { id: 'c1' }; + + it('keeps the conversation after a remount over the trimmed cache', () => { + const key = 'octocat/hello#1'; + // First mount: the first page holds the conversation. + expect(retainConversationAcrossMounts(key, [{ conversation: [comment] }])).toEqual([comment]); + // Remount over the trimmed cache: pages[0] is a later page with []. + expect(retainConversationAcrossMounts(key, [{ conversation: [] }])).toEqual([comment]); + }); + + it('does not leak the conversation across different PRs', () => { + retainConversationAcrossMounts('octocat/hello#1', [{ conversation: [comment] }]); + // A different PR has never retained anything, so it stays empty. + expect(retainConversationAcrossMounts('octocat/hello#2', [{ conversation: [] }])).toEqual([]); + }); +}); diff --git a/apps/mobile/src/lib/pr-review/discussion/use-pr-review-discussion-threads.ts b/apps/mobile/src/lib/pr-review/discussion/use-pr-review-discussion-threads.ts index e7b8a88b0d..e176d04108 100644 --- a/apps/mobile/src/lib/pr-review/discussion/use-pr-review-discussion-threads.ts +++ b/apps/mobile/src/lib/pr-review/discussion/use-pr-review-discussion-threads.ts @@ -14,23 +14,31 @@ // the tab UI doesn't have to. // // Conversation comments are returned on the first page only (backend -// contract: later pages carry `conversation: []`). We read page 0 -// only — simpler than flatten-then-dedupe and matches the guarantee. +// contract: later pages carry `conversation: []`). We retain the first +// page's conversation in a module-level store keyed by the PR identity so +// it survives both the retention trim (which drops the oldest page once +// `maxPages` is exceeded) and the tab's unmount/remount cycle +// (`PrReviewScreen` unmounts `PrReviewDiscussionTab` on every tab change, +// which a component ref cannot survive). import { useInfiniteQuery } from '@tanstack/react-query'; import { useMemo } from 'react'; import { classifyPrReviewQueryState } from '@/lib/pr-review/classify-pr-review-query-state'; +import { type ConversationComment } from '@/lib/pr-review/discussion/review-discussion-types'; +import { withInfiniteRetention } from '@/lib/query/infinite-retention'; import { useTRPC } from '@/lib/trpc'; -export function usePrReviewDiscussionThreads(args: { - owner: string; - repo: string; - number: number; -}) { +/** + * Build the discussion-threads infinite-query options. Kept as a pure builder + * so the retention bound is testable without mounting the hook. + */ +export function buildPrReviewDiscussionThreadsQueryOptions( + trpc: ReturnType, + args: { owner: string; repo: string; number: number } +) { const { owner, repo, number } = args; - const trpc = useTRPC(); - const query = useInfiniteQuery( + return withInfiniteRetention( trpc.githubPrReview.listReviewThreads.infiniteQueryOptions( { owner, repo, number }, { @@ -39,6 +47,64 @@ export function usePrReviewDiscussionThreads(args: { } ) ); +} + +/** + * Keep the first-page conversation comments across the retention trim. + * + * The backend returns conversation comments on the first page only; later + * pages carry `conversation: []`. Once `maxPages` trims the oldest page, + * `pages[0]` is no longer the first page, so reading `pages[0].conversation` + * would erase the comments. Prefer the current first page's conversation when + * it is non-empty; otherwise fall back to the retained value. + */ +export function retainConversation( + pages: readonly { conversation: readonly C[] }[] | undefined, + retained: readonly C[] +): readonly C[] { + const first = pages?.[0]?.conversation; + return first && first.length > 0 ? first : retained; +} + +// Module-level retention store. Keyed by the PR identity so the retained +// first-page conversation survives the tab's unmount/remount cycle, which +// a component ref cannot. +const conversationRetention = new Map(); + +function conversationRetentionKey(args: { owner: string; repo: string; number: number }): string { + return `${args.owner}/${args.repo}#${args.number}`; +} + +/** + * Read and update the retained first-page conversation for one PR. + * + * Prefer the current first page's conversation when it is non-empty; + * otherwise fall back to the previously retained value. Writes the value + * back when it changed so a later mount (over the trimmed cache) still + * reads it. + */ +export function retainConversationAcrossMounts( + key: string, + pages: readonly { conversation: readonly ConversationComment[] }[] | undefined +): readonly ConversationComment[] { + const retained = conversationRetention.get(key) ?? []; + const conversation = retainConversation(pages, retained); + if (conversation !== retained) { + conversationRetention.set(key, conversation); + } + return conversation; +} + +export function usePrReviewDiscussionThreads(args: { + owner: string; + repo: string; + number: number; +}) { + const { owner, repo, number } = args; + const trpc = useTRPC(); + const query = useInfiniteQuery( + buildPrReviewDiscussionThreadsQueryOptions(trpc, { owner, repo, number }) + ); const hasLoadedPages = (query.data?.pages.length ?? 0) > 0; const firstPagePending = query.isPending; @@ -56,8 +122,15 @@ export function usePrReviewDiscussionThreads(args: { const pages = query.data?.pages; const threads = useMemo(() => (pages ?? []).flatMap(page => page.threads), [pages]); - // First page only — backend guarantees later pages return []. - const conversation = query.data?.pages[0]?.conversation ?? []; + // Conversation comments live only on the first page. Retention trims the + // oldest page once the bound is exceeded, which would erase the comments if + // we read `pages[0]` directly. Keep the last non-empty conversation in a + // module-level store keyed by the PR identity so it survives both the trim + // and the tab's unmount/remount cycle. + const conversation = retainConversationAcrossMounts( + conversationRetentionKey({ owner, repo, number }), + pages + ); return { query, From a226cb1c100e5f84bffb5d4dcd0b3e292b35a49f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20=C5=A0=C4=87eki=C4=87?= Date: Wed, 19 Aug 2026 04:21:01 +0200 Subject: [PATCH 08/34] feat(mobile): wire failed-row retry and copy-to-composer handlers Expose a ChatComposerControl.setText ref so a failed row can copy its prompt back into the composer, and wire Retry to re-send the correct prompt through handleSend, clearing the failed row only on success. Count only non-failed pending messages for the working indicator and keep-awake. --- .../components/agents/chat-composer.test.ts | 2 + .../src/components/agents/chat-composer.tsx | 65 ++++- .../agents/session-detail-content.tsx | 271 +++++++++++------- 3 files changed, 228 insertions(+), 110 deletions(-) diff --git a/apps/mobile/src/components/agents/chat-composer.test.ts b/apps/mobile/src/components/agents/chat-composer.test.ts index 6395deff09..7932bdca8c 100644 --- a/apps/mobile/src/components/agents/chat-composer.test.ts +++ b/apps/mobile/src/components/agents/chat-composer.test.ts @@ -1,3 +1,4 @@ +/* eslint-disable max-lines -- the mocked hook surface and draft-restore contract require a long suite */ /* eslint-disable typescript-eslint/no-deprecated -- react-test-renderer is the DOM-free renderer used to mount React/RN trees under vitest (node env, no jsdom); see src/lib/persist/cache-persistence-mount.test.ts */ /* eslint-disable new-cap -- ChatComposer is called as a plain function, matching repo test convention */ /* eslint-disable require-await, @typescript-eslint/require-await -- the fake hooks and handlers settle without await because they resolve immediately */ @@ -31,6 +32,7 @@ vi.mock('react', async () => { useEffect: vi.fn((fn: React.EffectCallback) => { fn(); }), + useImperativeHandle: vi.fn(() => undefined), useMemo: vi.fn((factory: () => T) => factory()), useRef: vi.fn((initial: T) => { const index = refSlots.cursor; diff --git a/apps/mobile/src/components/agents/chat-composer.tsx b/apps/mobile/src/components/agents/chat-composer.tsx index a9104a6768..6579d59a4d 100644 --- a/apps/mobile/src/components/agents/chat-composer.tsx +++ b/apps/mobile/src/components/agents/chat-composer.tsx @@ -7,7 +7,15 @@ import * as Haptics from 'expo-haptics'; import { useActionSheet } from '@expo/react-native-action-sheet'; import { type SlashCommandInfo } from '@kilocode/cloud-agent-sdk'; import { type RemoteCommandState } from '@kilocode/cloud-agent-sdk/remote-command-catalog'; -import { useCallback, useEffect, useMemo, useRef, useState } from 'react'; +import { + type Ref, + useCallback, + useEffect, + useImperativeHandle, + useMemo, + useRef, + useState, +} from 'react'; import { AppState, type GestureResponderEvent, @@ -104,6 +112,11 @@ type AndroidDismissKeyboardGesture = { failed: boolean; }; +/** Imperative handle the host uses to set composer text (Retry / Copy to composer). */ +export type ChatComposerControl = { + setText: (text: string) => void; +}; + type ChatComposerProps = { onSend: ( text: string, @@ -162,6 +175,8 @@ type ChatComposerProps = { * text the user typed while identity and the draft were still loading. */ initialDraft?: string; + /** Imperative handle the host binds to call `setText`. */ + controlRef?: Ref; }; export function ChatComposer({ @@ -192,6 +207,7 @@ export function ChatComposer({ autoSend, draftKey, initialDraft, + controlRef, }: Readonly) { const colors = useThemeColors(); const { showActionSheetWithOptions } = useActionSheet(); @@ -352,6 +368,41 @@ export function ChatComposer({ const toolbarDisabled = disabled || isSending; const voiceDisabled = toolbarDisabled; + // One place text is written into the live input from an external caller + // (slash-command select, Retry / Copy to composer). Sets text, selection, + // hasText, slash-command state, and the measure node, then persists the + // durable draft exactly like a keystroke. + function applyComposerText(value: string) { + textRef.current = value; + measure.setText(value); + setHasText(value.trim().length > 0); + setSlashCommandInput(null); + inputRef.current?.setNativeProps({ + text: value, + selection: { start: value.length, end: value.length }, + }); + selectionRef.current = { start: value.length, end: value.length }; + inputRef.current?.focus(); + if (draftKey && userId) { + saveDraft(userId, draftKey, value); + } + } + + // Hold the latest applyComposerText so the imperative handle stays stable + // while the composer does not remount when identity resolves. + const applyComposerTextRef = useRef(applyComposerText); + applyComposerTextRef.current = applyComposerText; + + useImperativeHandle( + controlRef, + () => ({ + setText: (text: string) => { + applyComposerTextRef.current(text); + }, + }), + [] + ); + function handleChangeText(value: string) { textRef.current = value; measure.setText(value); @@ -689,17 +740,7 @@ export function ChatComposer({ if (sendLockRef.current.isLocked()) { return; } - const value = `/${command.name} `; - textRef.current = value; - measure.setText(value); - setHasText(true); - setSlashCommandInput(null); - inputRef.current?.setNativeProps({ - text: value, - selection: { start: value.length, end: value.length }, - }); - selectionRef.current = { start: value.length, end: value.length }; - inputRef.current?.focus(); + applyComposerText(`/${command.name} `); } async function submit() { diff --git a/apps/mobile/src/components/agents/session-detail-content.tsx b/apps/mobile/src/components/agents/session-detail-content.tsx index e7b076cafc..1c832745c5 100644 --- a/apps/mobile/src/components/agents/session-detail-content.tsx +++ b/apps/mobile/src/components/agents/session-detail-content.tsx @@ -15,7 +15,7 @@ import { useSafeAreaInsets } from 'react-native-safe-area-context'; import { toast } from 'sonner-native'; import { getBlockingInteraction } from '@/components/agents/agent-interaction-policy'; -import { ChatComposer } from '@/components/agents/chat-composer'; +import { ChatComposer, type ChatComposerControl } from '@/components/agents/chat-composer'; import { type AgentMode, customModeOptionsFromRuntimeAgents, @@ -58,6 +58,10 @@ import { shouldShowFooterWorkingIndicator, shouldShowSessionFooterRow, } from '@/components/agents/session-working-state'; +import { + countInFlightMessages, + retryMessageAndClear, +} from '@/components/agents/session-detail-content-helpers'; import { shouldKeepSessionAwake } from '@/components/agents/session-keep-awake'; import { TranscriptTimeMarker } from '@/components/agents/transcript-time-marker'; import { EmptyState } from '@/components/empty-state'; @@ -86,6 +90,7 @@ import { } from '@/components/agents/child-session-sheet-state'; import { PartDetailSheetHost } from '@/components/agents/part-detail-sheet-host'; import { PartRenderer } from '@/components/agents/part-renderer'; +import { isTextPart } from '@/components/agents/part-types'; import { buildTerminalErrorCopyText, resolveSessionTerminalError, @@ -162,6 +167,7 @@ export function SessionDetailContent({ visible: false, }); const childSheetReleaseTimeoutRef = useRef | null>(null); + const composerControlRef = useRef(null); const clearChildSheetReleaseTimeout = useCallback(() => { if (childSheetReleaseTimeoutRef.current !== null) { @@ -589,6 +595,156 @@ export function SessionDetailContent({ } } + const requiresModel = Boolean(fetchedData?.cloudAgentSessionId); + + const handleSend = useCallback( + async ( + text: string, + attachments?: AgentAttachmentWire, + submission?: AgentAttachmentSubmissionPayload + ) => { + if (requiresModel && !(pinned.model ?? currentModel)) { + toast.error('Select a model before sending'); + return; + } + // Pick the wire shape via the same pure helper the unit test covers: + // - cloud-agent → unchanged `{path, files}` (S3a) + // - remote + supportsAttachments → materialize presigned GETs and + // forward as `attachmentParts` (S3b) + // - everything else → no attachment field on the wire + const kind = resolveSendAttachmentKind( + activeSessionType, + supportsAttachments, + attachments !== undefined + ); + if (shouldRefuseSilentAttachmentDrop(kind, attachments !== undefined)) { + const message = + "This session can't receive files. Remove the attachments to send your message."; + toast.error(message); + throw new Error(message); + } + let attachmentParts: Awaited> | undefined = + undefined; + if (kind === 'remote-capable' && submission) { + const result = await buildRemoteAttachmentPartsWithRetryableFeedback( + submission, + buildRemoteAttachmentParts + ); + if (!result.ok) { + // Retryable presign failure: the manager never reached send(), so + // its onSendFailed toast does not fire. Surface the retryable message + // through the same toast channel and throw so the composer keeps the + // draft/attachments for a retry. + toast.error(result.message); + throw new Error(result.message); + } + attachmentParts = result.parts; + } + const sendModel = + activeSessionType === 'cloud-agent' && pinned.model ? pinned.model : currentModel; + const sendVariant = + activeSessionType === 'cloud-agent' && pinned.model + ? (pinned.variant ?? '') + : currentVariant; + // Sync the override to the exact model/variant being sent so the SDK's + // `cloudAgentModelOverride` preference cannot beat the pin on send, and + // a leftover pin cannot beat a user pick. `sendModel` is always truthy + // here (the guard above returns early when no model resolves), so this + // never clears to null on an unpinned send. + if (activeSessionType === 'cloud-agent') { + manager.setCloudAgentModelOverride( + sendModel ? { model: sendModel, ...(sendVariant ? { variant: sendVariant } : {}) } : null + ); + } + // manager.send() reports failures via its own return value (and toasts + // through the manager's onSendFailed hook) rather than rejecting — it + // is the single toast owner for send failures. Throw here, without a + // second toast, purely so the composer's `await onSend(...)` sees the + // rejection and preserves the draft. + const sent = await manager.send({ + payload: { + type: 'prompt', + prompt: text, + mode: currentMode, + model: sendModel, + variant: sendVariant || undefined, + }, + ...(kind === 'cloud' && attachments ? { attachments } : {}), + ...(kind === 'remote-capable' && attachmentParts ? { attachmentParts } : {}), + }); + if (!sent) { + throw new Error('Failed to send message'); + } + captureEvent(MESSAGE_SENT_EVENT, { surface: analyticsSurface }); + }, + [ + manager, + currentMode, + currentModel, + currentVariant, + pinned.model, + pinned.variant, + requiresModel, + activeSessionType, + supportsAttachments, + analyticsSurface, + ] + ); + + // Retry payload by row kind: a delivery failure re-sends the user row's own + // text; an assistant failure re-sends the newest user row before it. Returns + // null when no user row precedes the assistant row, which suppresses Retry. + const resolveRetryPrompt = useCallback( + (message: StoredMessage): string | null => { + if (message.info.role === 'user') { + return message.parts + .filter(isTextPart) + .map(part => part.text) + .join('\n\n'); + } + const index = messages.findIndex(candidate => candidate.info.id === message.info.id); + for (let i = index - 1; i >= 0; i -= 1) { + const candidate = messages[i]; + if (candidate?.info.role === 'user') { + return candidate.parts + .filter(isTextPart) + .map(part => part.text) + .join('\n\n'); + } + } + return null; + }, + [messages] + ); + + const handleCopyToComposer = useCallback((text: string) => { + composerControlRef.current?.setText(text); + }, []); + + const handleRetryMessage = useCallback( + (message: StoredMessage) => { + const prompt = resolveRetryPrompt(message); + if (prompt === null) { + return; + } + // Same guard handleSend opens with: when no model resolves, run the send + // anyway (the user gets the existing toast) and keep the failed row. + if (requiresModel && !(pinned.model ?? currentModel)) { + void handleSend(prompt); + return; + } + void retryMessageAndClear( + async () => { + await handleSend(prompt); + }, + () => { + manager.clearFailedMessage(message.info.id); + } + ); + }, + [resolveRetryPrompt, requiresModel, pinned.model, currentModel, handleSend, manager] + ); + const renderItem = useCallback( ({ item }: { item: SessionTranscriptItem }) => { if (item.type === 'preparation') { @@ -603,6 +759,8 @@ export function SessionDetailContent({ // so a plain lookup is enough — no render-order guard. const deliveryState = item.message.info.role === 'user' ? pendingMessages.get(item.message.info.id) : undefined; + // Suppress Retry on an assistant failure with no preceding user row. + const retryPrompt = resolveRetryPrompt(item.message); return ( ); }, @@ -625,6 +785,9 @@ export function SessionDetailContent({ handleOpenChildSession, pendingMessages, heldQueuedIds, + resolveRetryPrompt, + handleRetryMessage, + handleCopyToComposer, ] ); @@ -685,9 +848,15 @@ export function SessionDetailContent({ (fetchedData === null && !statusIndicator && !error) || (fetchedData !== null && fetchedData.kiloSessionId !== sessionId); const shouldBlockMessages = shouldShowLoading; + // Failed delivery entries must not count as in-flight: after a terminal + // delivery failure the working spinner and wake lock would otherwise stay on. + const inFlightMessageCount = useMemo( + () => countInFlightMessages(pendingMessages), + [pendingMessages] + ); const shouldShowWorkingIndicator = shouldShowAgentWorkingIndicator({ isStreaming, - pendingMessageCount: pendingMessages.size, + pendingMessageCount: inFlightMessageCount, }); const hasFooterStatusIndicator = statusIndicator !== null || (cloudStatus !== null && cloudStatus.type !== 'ready'); @@ -744,7 +913,6 @@ export function SessionDetailContent({ /> ); - const requiresModel = Boolean(fetchedData?.cloudAgentSessionId); const blockingInteraction = getBlockingInteraction({ activeQuestion, activePermission }); const hasBlockingInteraction = blockingInteraction !== 'none'; // One number for both kinds: the user must see every waiting request, not @@ -794,100 +962,6 @@ export function SessionDetailContent({ (cloudStatus && COMPOSER_PLACEHOLDERS[cloudStatus.type]) ?? 'Message...'; const keyboardContainerKind = getSessionKeyboardContainerKind(Platform.OS); - const handleSend = useCallback( - async ( - text: string, - attachments?: AgentAttachmentWire, - submission?: AgentAttachmentSubmissionPayload - ) => { - if (requiresModel && !(pinned.model ?? currentModel)) { - toast.error('Select a model before sending'); - return; - } - // Pick the wire shape via the same pure helper the unit test covers: - // - cloud-agent → unchanged `{path, files}` (S3a) - // - remote + supportsAttachments → materialize presigned GETs and - // forward as `attachmentParts` (S3b) - // - everything else → no attachment field on the wire - const kind = resolveSendAttachmentKind( - activeSessionType, - supportsAttachments, - attachments !== undefined - ); - if (shouldRefuseSilentAttachmentDrop(kind, attachments !== undefined)) { - const message = - "This session can't receive files. Remove the attachments to send your message."; - toast.error(message); - throw new Error(message); - } - let attachmentParts: Awaited> | undefined = - undefined; - if (kind === 'remote-capable' && submission) { - const result = await buildRemoteAttachmentPartsWithRetryableFeedback( - submission, - buildRemoteAttachmentParts - ); - if (!result.ok) { - // Retryable presign failure: the manager never reached send(), so - // its onSendFailed toast does not fire. Surface the retryable message - // through the same toast channel and throw so the composer keeps the - // draft/attachments for a retry. - toast.error(result.message); - throw new Error(result.message); - } - attachmentParts = result.parts; - } - const sendModel = - activeSessionType === 'cloud-agent' && pinned.model ? pinned.model : currentModel; - const sendVariant = - activeSessionType === 'cloud-agent' && pinned.model - ? (pinned.variant ?? '') - : currentVariant; - // Sync the override to the exact model/variant being sent so the SDK's - // `cloudAgentModelOverride` preference cannot beat the pin on send, and - // a leftover pin cannot beat a user pick. `sendModel` is always truthy - // here (the guard above returns early when no model resolves), so this - // never clears to null on an unpinned send. - if (activeSessionType === 'cloud-agent') { - manager.setCloudAgentModelOverride( - sendModel ? { model: sendModel, ...(sendVariant ? { variant: sendVariant } : {}) } : null - ); - } - // manager.send() reports failures via its own return value (and toasts - // through the manager's onSendFailed hook) rather than rejecting — it - // is the single toast owner for send failures. Throw here, without a - // second toast, purely so the composer's `await onSend(...)` sees the - // rejection and preserves the draft. - const sent = await manager.send({ - payload: { - type: 'prompt', - prompt: text, - mode: currentMode, - model: sendModel, - variant: sendVariant || undefined, - }, - ...(kind === 'cloud' && attachments ? { attachments } : {}), - ...(kind === 'remote-capable' && attachmentParts ? { attachmentParts } : {}), - }); - if (!sent) { - throw new Error('Failed to send message'); - } - captureEvent(MESSAGE_SENT_EVENT, { surface: analyticsSurface }); - }, - [ - manager, - currentMode, - currentModel, - currentVariant, - pinned.model, - pinned.variant, - requiresModel, - activeSessionType, - supportsAttachments, - analyticsSurface, - ] - ); - const handleSendCommand = useCallback( async (command: string, argumentsText: string) => { // Slash commands ride the same manager.send() pipeline. The manager @@ -974,7 +1048,7 @@ export function SessionDetailContent({ isFocused, isDisconnected: agentStatus.type === 'disconnected', isStreaming, - pendingMessageCount: pendingMessages.size, + pendingMessageCount: inFlightMessageCount, }); return ( @@ -1194,6 +1268,7 @@ export function SessionDetailContent({ autoSend={autoSend} draftKey={userId ? sessionComposerDraftKey : undefined} initialDraft={composerDraft.settled ? (composerDraft.text ?? '') : undefined} + controlRef={composerControlRef} /> From 3073eec1d21c2a8daa4d6e5f974b2436324c763d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20=C5=A0=C4=87eki=C4=87?= Date: Wed, 19 Aug 2026 04:21:05 +0200 Subject: [PATCH 09/34] perf(cloud-agent-sdk): coalesce part-delta publication and retain history Coalesce applyPartDelta publication behind an injectable frame scheduler so a token stream publishes once per frame, and keep getParts correct before the flush. Add trimRetainedHistory with a 200-message window that drops the oldest loaded page and restores its cursor. --- .../src/session-manager.test.ts | 182 ++++++++++++++++++ .../cloud-agent-sdk/src/session-manager.ts | 63 +++++- .../cloud-agent-sdk/src/storage/jotai.test.ts | 80 ++++++++ packages/cloud-agent-sdk/src/storage/jotai.ts | 51 ++++- 4 files changed, 372 insertions(+), 4 deletions(-) diff --git a/packages/cloud-agent-sdk/src/session-manager.test.ts b/packages/cloud-agent-sdk/src/session-manager.test.ts index 10e7999cad..14d725d042 100644 --- a/packages/cloud-agent-sdk/src/session-manager.test.ts +++ b/packages/cloud-agent-sdk/src/session-manager.test.ts @@ -4271,6 +4271,41 @@ describe('createSessionManager', () => { expect(fetchSnapshotPage).not.toHaveBeenCalled(); }); + it('resets retained history so trimRetainedHistory cannot restore a pre-clear cursor', async () => { + const fetchSnapshotPage = createPageFetchMock(async (_id, options) => { + if (!options.cursor) { + return makePage({ + kiloSessionId: 'ses-1', + messages: [makePageMessage('init-0', 'ses-1', 'init')], + nextCursor: 'cursor-A', + }); + } + return makePage({ + kiloSessionId: 'ses-1', + messages: [makePageMessage('old-0', 'ses-1', 'old')], + nextCursor: null, + }); + }); + const config = createMockConfig({ fetchSnapshotPage }); + const mgr = createSessionManager(config); + + await mgr.switchSession(kiloId('ses-1')); + await mgr.loadOlderMessages(); // pushes a retained-history stack entry + + mgr.clearTranscript(); + expect(atomValue(config.store, mgr.atoms.hasOlderMessages)).toBe(false); + + // Re-populate storage above the window so a stale stack entry would trim. + for (let i = 0; i < 250; i++) { + latestStorage?.upsertMessage(stubUserMessage({ id: `post-${i}`, sessionID: 'ses-1' })); + } + + mgr.trimRetainedHistory(); + + // The stack was reset by clearTranscript, so no pre-clear cursor is restored. + expect(atomValue(config.store, mgr.atoms.hasOlderMessages)).toBe(false); + }); + it('purges all replayed history on reconnect with no post-clear send', async () => { // E2E case 4: /clear → kill/reconnect, no send → view stays cleared. const fetchSnapshotPage = jest.fn().mockResolvedValue({ @@ -5625,6 +5660,153 @@ describe('createSessionManager — paginated initial snapshot + loadOlderMessage ).toEqual(['msg-current']); }); + // ------------------------------------------------------------------------- + // trimRetainedHistory + // ------------------------------------------------------------------------- + + describe('trimRetainedHistory', () => { + function makeMessages(prefix: string, count: number): SessionSnapshotPage['messages'] { + return Array.from({ length: count }, (_, i) => + makePageMessage(`${prefix}-${i}`, 'ses-1', `${prefix}-${i}`) + ); + } + + it('is a no-op below the window', async () => { + const fetchSnapshotPage = createPageFetchMock(async () => + makePage({ + kiloSessionId: 'ses-1', + messages: makeMessages('init', 5), + nextCursor: 'cursor-A', + }) + ); + const config = createMockConfig({ fetchSnapshotPage }); + const mgr = createSessionManager(config); + + await mgr.switchSession(kiloId('ses-1')); + expect(atomValue(config.store, mgr.atoms.messagesList)).toHaveLength(5); + + mgr.trimRetainedHistory(); + + expect(atomValue(config.store, mgr.atoms.messagesList)).toHaveLength(5); + expect(atomValue(config.store, mgr.atoms.hasOlderMessages)).toBe(true); + }); + + it('is a no-op with an empty stack even above the window (never trims the initial page)', async () => { + const fetchSnapshotPage = createPageFetchMock(async () => + makePage({ + kiloSessionId: 'ses-1', + messages: makeMessages('init', 250), + nextCursor: 'cursor-A', + }) + ); + const config = createMockConfig({ fetchSnapshotPage }); + const mgr = createSessionManager(config); + + await mgr.switchSession(kiloId('ses-1')); + expect(atomValue(config.store, mgr.atoms.messagesList)).toHaveLength(250); + + mgr.trimRetainedHistory(); + + expect(atomValue(config.store, mgr.atoms.messagesList)).toHaveLength(250); + expect(atomValue(config.store, mgr.atoms.hasOlderMessages)).toBe(true); + }); + + it('drops the oldest loaded page above the window, restores its cursor, and loadOlderMessages brings it back', async () => { + const fetchSnapshotPage = createPageFetchMock(async (_id, options) => { + if (!options.cursor) { + return makePage({ + kiloSessionId: 'ses-1', + messages: makeMessages('init', 150), + nextCursor: 'cursor-A', + }); + } + if (options.cursor === 'cursor-A') { + return makePage({ + kiloSessionId: 'ses-1', + messages: makeMessages('old', 100), + nextCursor: 'cursor-B', + }); + } + return makePage({ kiloSessionId: 'ses-1', nextCursor: null }); + }); + const config = createMockConfig({ fetchSnapshotPage }); + const mgr = createSessionManager(config); + + await mgr.switchSession(kiloId('ses-1')); + await mgr.loadOlderMessages(); + + const before = atomValue(config.store, mgr.atoms.messagesList); + expect(before).toHaveLength(250); + expect(atomValue(config.store, mgr.atoms.hasOlderMessages)).toBe(true); + + mgr.trimRetainedHistory(); + + const after = atomValue(config.store, mgr.atoms.messagesList); + expect(after).toHaveLength(150); + expect(after.map(m => m.info.id)).not.toContain('old-0'); + expect(after.map(m => m.info.id)).toContain('init-0'); + // Cursor restored to the pre-page value, so more history is available again. + expect(atomValue(config.store, mgr.atoms.hasOlderMessages)).toBe(true); + + // loadOlderMessages re-fetches from the restored cursor and brings the page back. + await mgr.loadOlderMessages(); + expect(atomValue(config.store, mgr.atoms.messagesList)).toHaveLength(250); + }); + + it('restores the OLDEST popped cursor when two pages drop in one pass', async () => { + const calls: Array<{ cursor?: string }> = []; + const fetchSnapshotPage = createPageFetchMock(async (_id, options) => { + calls.push({ ...options }); + if (!options.cursor) { + return makePage({ + kiloSessionId: 'ses-1', + messages: makeMessages('init', 150), + nextCursor: 'cursor-A', + }); + } + if (options.cursor === 'cursor-A') { + return makePage({ + kiloSessionId: 'ses-1', + messages: makeMessages('oldA', 100), + nextCursor: 'cursor-B', + }); + } + if (options.cursor === 'cursor-B') { + return makePage({ + kiloSessionId: 'ses-1', + messages: makeMessages('oldB', 100), + nextCursor: null, + }); + } + return makePage({ kiloSessionId: 'ses-1', nextCursor: null }); + }); + const config = createMockConfig({ fetchSnapshotPage }); + const mgr = createSessionManager(config); + + await mgr.switchSession(kiloId('ses-1')); + await mgr.loadOlderMessages(); // pageA (cursor-A) + await mgr.loadOlderMessages(); // pageB (cursor-B) + + expect(atomValue(config.store, mgr.atoms.messagesList)).toHaveLength(350); + + mgr.trimRetainedHistory(); + + // Both older pages drop (350 -> 150); only the initial page survives. + expect(atomValue(config.store, mgr.atoms.messagesList)).toHaveLength(150); + + calls.length = 0; + await mgr.loadOlderMessages(); + // Re-fetch starts from the OLDEST dropped page's cursor, so the oldest + // page (oldA) comes back, not the newer dropped page (oldB). + expect(calls[calls.length - 1]).toEqual({ cursor: 'cursor-A' }); + const ids = atomValue(config.store, mgr.atoms.messagesList).map( + m => m.info.id + ); + expect(ids).toContain('oldA-0'); + expect(ids).not.toContain('oldB-0'); + }); + }); + // ------------------------------------------------------------------------- // supportsAttachments gate // ------------------------------------------------------------------------- diff --git a/packages/cloud-agent-sdk/src/session-manager.ts b/packages/cloud-agent-sdk/src/session-manager.ts index a95b96ba73..dedcc53814 100644 --- a/packages/cloud-agent-sdk/src/session-manager.ts +++ b/packages/cloud-agent-sdk/src/session-manager.ts @@ -137,6 +137,12 @@ const UUID_RE = /^[0-9a-f]{8}-[0-9a-f]{4}-[1-5][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0- const TRANSCRIPT_CLEARED_INDICATOR = 'View cleared — earlier messages are still on this session'; +/** + * Maximum number of retained messages. Once the loaded transcript exceeds this, + * `trimRetainedHistory` drops the oldest loaded older-page from local storage. + */ +const RETAINED_MESSAGE_WINDOW = 200; + /** * Shared empty-parts sentinel. `memoizedStoredMessage` compares `cached.parts * === parts`; `partsMap.get(id) ?? []` would allocate a fresh array on every @@ -379,6 +385,14 @@ type SessionManager = { * already surfaced for the active session. */ loadOlderMessages(): Promise; + /** + * Drop the oldest loaded older-page(s) from local storage while the retained + * transcript exceeds `RETAINED_MESSAGE_WINDOW`. Pops the oldest stack entry, + * deletes its messages, restores the pre-page cursor, and re-arms + * `hasOlderMessages`. No-op below the window or with an empty stack. Never + * trims the initial bounded page (it is not on the stack). + */ + trimRetainedHistory(): void; /** * Merge a freshly fetched `associatedPr` (or null after an unlink) into the * current `fetchedSessionData` atom. Mobile calls this after a focus refetch. @@ -746,6 +760,14 @@ function createSessionManager(config: SessionManagerConfig): SessionManager { // Once a non-retryable terminal failure lands, we permanently disable // further older-page loads for the active session. let olderMessagesTerminal: boolean = false; + /** + * Stack of loaded older pages, oldest first. Each entry records the cursor + * that was current before that page was fetched and the message ids the page + * added. `trimRetainedHistory` pops from the front (oldest) to stay under + * `RETAINED_MESSAGE_WINDOW`. The initial bounded page is never pushed here, + * so the live tail always survives. + */ + let retainedHistoryStack: Array<{ cursorBefore: string | null; messageIds: string[] }> = []; /** * Last non-empty `mode` from a remote prompt send. Used as agent inheritance * fallback when `sessionConfigAtom.mode` is absent/`''`. Reset on switch/destroy. @@ -820,6 +842,7 @@ function createSessionManager(config: SessionManagerConfig): SessionManager { olderMessagesInFlight = null; lastPromptMode = null; olderMessagesTerminal = false; + retainedHistoryStack = []; currentCapabilities = undefined; pendingInterruptSession = null; } @@ -1211,7 +1234,12 @@ function createSessionManager(config: SessionManagerConfig): SessionManager { } if (outcome.kind === 'success') { - applyPage(outcome, expectedGeneration); + if (applyPage(outcome, expectedGeneration)) { + retainedHistoryStack.push({ + cursorBefore: cursor, + messageIds: outcome.messages.map(m => m.info.id), + }); + } store.set(isLoadingOlderMessagesAtom, false); return; } @@ -1242,6 +1270,35 @@ function createSessionManager(config: SessionManagerConfig): SessionManager { } } + function trimRetainedHistory(): void { + const storage = store.get(sessionStorageAtom); + if (!storage) return; + // The cursor must be the OLDEST popped entry's `cursorBefore`, not the + // last one popped. When two or more pages drop in one pass, overwriting + // on every pop leaves the cursor pointing at the newest dropped page, so + // `loadOlderMessages` could not re-fetch the oldest dropped page. + let trimmedAny = false; + let oldestCursorBefore: string | null = null; + while ( + storage.getMessageIds().length > RETAINED_MESSAGE_WINDOW && + retainedHistoryStack.length > 0 + ) { + const entry = retainedHistoryStack.shift(); + if (!entry) break; + if (!trimmedAny) { + trimmedAny = true; + oldestCursorBefore = entry.cursorBefore; + } + for (const id of entry.messageIds) { + storage.deleteMessage(id); + } + } + if (trimmedAny) { + olderMessagesCursor = oldestCursorBefore; + store.set(hasOlderMessagesAtom, true); + } + } + async function switchSession(kiloSessionId: KiloSessionId): Promise { childSessionHydrationGeneration += 1; childSessionHydrationRequests.clear(); @@ -1757,6 +1814,9 @@ function createSessionManager(config: SessionManagerConfig): SessionManager { currentSession.storage.clear(); olderMessagesCursor = null; store.set(hasOlderMessagesAtom, false); + // Reset the retained-history stack so a later `trimRetainedHistory` + // cannot restore a pre-clear cursor. + retainedHistoryStack = []; // Same idle reset as clearAllAtoms: an in-flight older-page fetch will // hit the generation guard and return without clearing these atoms. store.set(isLoadingOlderMessagesAtom, false); @@ -1887,6 +1947,7 @@ function createSessionManager(config: SessionManagerConfig): SessionManager { switchSession, hydrateChildSession, loadOlderMessages, + trimRetainedHistory, updateFetchedAssociatedPr, send, setRemoteModelOverride, diff --git a/packages/cloud-agent-sdk/src/storage/jotai.test.ts b/packages/cloud-agent-sdk/src/storage/jotai.test.ts index b4814e588a..399c9720da 100644 --- a/packages/cloud-agent-sdk/src/storage/jotai.test.ts +++ b/packages/cloud-agent-sdk/src/storage/jotai.test.ts @@ -258,6 +258,86 @@ describe('createJotaiStorage', () => { }); }); + describe('delta coalescing', () => { + test('three deltas produce one publication after one flush', () => { + let flush: (() => void) | null = null; + const coalesced = createJotaiStorage(store, { + schedule: cb => { + flush = cb; + }, + }); + + coalesced.upsertPart('msg-1', makePart('p-1', 'msg-1', '')); + + const revCb = jest.fn(); + store.sub(coalesced.atoms.partsRevision, revCb); + const partsCb = jest.fn(); + coalesced.subscribe('parts:msg-1', partsCb); + + coalesced.applyPartDelta('msg-1', 'p-1', 'text', 'a'); + coalesced.applyPartDelta('msg-1', 'p-1', 'text', 'b'); + coalesced.applyPartDelta('msg-1', 'p-1', 'text', 'c'); + + // State write is immediate; publication is deferred. + expect((coalesced.getParts('msg-1')[0] as Part & { text: string }).text).toBe('abc'); + expect(revCb).not.toHaveBeenCalled(); + expect(partsCb).not.toHaveBeenCalled(); + expect(flush).not.toBeNull(); + + flush!(); + + expect(revCb).toHaveBeenCalledTimes(1); + expect(partsCb).toHaveBeenCalledTimes(1); + }); + + test('applyPartDelta then upsertPart publishes the delta before the structural publish', () => { + const coalesced = createJotaiStorage(store, { + schedule: () => { + // Defer the flush so the pending delta is still unflushed when + // `upsertPart` runs and must flush it first. + }, + }); + + coalesced.upsertPart('msg-1', makePart('p-1', 'msg-1', '')); + + const snapshots: string[] = []; + coalesced.subscribe('parts:msg-1', () => { + snapshots.push( + coalesced + .getParts('msg-1') + .map(p => (p as Part & { text?: string }).text ?? '') + .join(',') + ); + }); + + coalesced.applyPartDelta('msg-1', 'p-1', 'text', 'a'); + coalesced.upsertPart('msg-1', makePart('p-2', 'msg-1', 'second')); + + // The pending delta publication runs first (p-1 = 'a'), then the + // structural publish (p-1 = 'a', p-2 = 'second'). + expect(snapshots).toEqual(['a', 'a,second']); + }); + + test('getParts returns new parts after a delta write even with a pending flush', () => { + const coalesced = createJotaiStorage(store, { + schedule: () => { + // Defer the flush so the delta stays unflushed. + }, + }); + + coalesced.upsertPart('msg-1', makePart('p-1', 'msg-1', 'hel')); + // Cache a snapshot for msg-1. + expect((coalesced.getParts('msg-1')[0] as Part & { text: string }).text).toBe('hel'); + + // Delta write with a pending (deferred) flush. + coalesced.applyPartDelta('msg-1', 'p-1', 'text', 'lo'); + + // getParts must rebuild from the freshly written parts, not the stale + // cached snapshot, even though the flush has not run yet. + expect((coalesced.getParts('msg-1')[0] as Part & { text: string }).text).toBe('hello'); + }); + }); + describe('clear', () => { test('resets all state', () => { s.upsertMessage(makeMsg('msg-1')); diff --git a/packages/cloud-agent-sdk/src/storage/jotai.ts b/packages/cloud-agent-sdk/src/storage/jotai.ts index e782be3883..7a8e12e1ef 100644 --- a/packages/cloud-agent-sdk/src/storage/jotai.ts +++ b/packages/cloud-agent-sdk/src/storage/jotai.ts @@ -28,7 +28,19 @@ type JotaiSessionStorage = SessionStorage & { }; }; -function createJotaiStorage(store: JotaiStore): JotaiSessionStorage { +function createJotaiStorage( + store: JotaiStore, + options?: { schedule?: (cb: () => void) => void } +): JotaiSessionStorage { + // Coalescing scheduler. On device this is `requestAnimationFrame`; under + // node (every vitest/jest run) it runs the callback synchronously so landed + // assertions stay deterministic. Never default to `queueMicrotask`. + const schedule = + options?.schedule ?? + (typeof requestAnimationFrame === 'function' + ? (cb: () => void) => requestAnimationFrame(() => cb()) + : (cb: () => void) => cb()); + const messageIdsAtom = atom([]); const messagesAtom = atom>(new Map()); // Stable parts map. The atom holds this one reference for its lifetime; @@ -41,10 +53,35 @@ function createJotaiStorage(store: JotaiStore): JotaiSessionStorage { const partsSnapshot = new Map(); const subscribers = new Map void>>(); + // Coalesced delta publication. `applyPartDelta` marks a message id dirty and + // schedules one flush; the flush bumps `partsRevisionAtom` once and notifies + // once per dirty id. Structural operations flush pending work first. + const dirtyPartIds = new Set(); + let flushScheduled = false; + function bumpPartsRevision(): void { store.set(partsRevisionAtom, r => r + 1); } + function flushPendingDeltas(): void { + if (!flushScheduled) return; + flushScheduled = false; + const dirty = [...dirtyPartIds]; + dirtyPartIds.clear(); + if (dirty.length === 0) return; + bumpPartsRevision(); + for (const messageId of dirty) { + partsSnapshot.set(messageId, null); + notify(subscribers, `parts:${messageId}`); + } + } + + function scheduleFlush(): void { + if (flushScheduled) return; + flushScheduled = true; + schedule(flushPendingDeltas); + } + return { atoms: { messageIds: messageIdsAtom, @@ -54,6 +91,7 @@ function createJotaiStorage(store: JotaiStore): JotaiSessionStorage { }, upsertMessage(info) { + flushPendingDeltas(); const messages = store.get(messagesAtom); const existing = messages.get(info.id); const next = new Map(messages); @@ -76,6 +114,7 @@ function createJotaiStorage(store: JotaiStore): JotaiSessionStorage { }, upsertPart(messageId, part) { + flushPendingDeltas(); const arr = partsMap.get(messageId) ?? []; const nextArr = upsertPartDroppingStaleSyntheticTextParts(arr, part); partsMap.set(messageId, nextArr); @@ -111,12 +150,16 @@ function createJotaiStorage(store: JotaiStore): JotaiSessionStorage { partsMap.set(messageId, nextArr); } } - bumpPartsRevision(); + // State write is immediate; publication is coalesced to one flush. + // Invalidate the cached snapshot so a `getParts` before the flush + // rebuilds from the freshly written parts instead of the stale cache. partsSnapshot.set(messageId, null); - notify(subscribers, `parts:${messageId}`); + dirtyPartIds.add(messageId); + scheduleFlush(); }, deletePart(messageId, partId) { + flushPendingDeltas(); const arr = partsMap.get(messageId); if (!arr) return; const filtered = arr.filter(p => p.id !== partId); @@ -152,6 +195,7 @@ function createJotaiStorage(store: JotaiStore): JotaiSessionStorage { }, clear() { + flushPendingDeltas(); const existingMessageIds = store.get(messageIdsAtom); const existingPartMessageIds = [...partsMap.keys()]; @@ -171,6 +215,7 @@ function createJotaiStorage(store: JotaiStore): JotaiSessionStorage { }, deleteMessage(messageId) { + flushPendingDeltas(); const messages = store.get(messagesAtom); if (!messages.has(messageId)) return; From b056e3ed5d2f93523cbddb7f8b4b5078881f2431 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20=C5=A0=C4=87eki=C4=87?= Date: Wed, 19 Aug 2026 04:23:28 +0200 Subject: [PATCH 10/34] fix(mobile): commit session-detail-content-helpers module The failed-row retry wiring in a226cb1c1 imports countInFlightMessages and retryMessageAndClear from session-detail-content-helpers, but the module and its test were left untracked. Commit them so the branch typechecks and tests on a clean checkout. --- .../session-detail-content-helpers.test.ts | 49 +++++++++++++++++++ .../agents/session-detail-content-helpers.ts | 36 ++++++++++++++ 2 files changed, 85 insertions(+) create mode 100644 apps/mobile/src/components/agents/session-detail-content-helpers.test.ts create mode 100644 apps/mobile/src/components/agents/session-detail-content-helpers.ts diff --git a/apps/mobile/src/components/agents/session-detail-content-helpers.test.ts b/apps/mobile/src/components/agents/session-detail-content-helpers.test.ts new file mode 100644 index 0000000000..60a3219a88 --- /dev/null +++ b/apps/mobile/src/components/agents/session-detail-content-helpers.test.ts @@ -0,0 +1,49 @@ +import { describe, expect, it, vi } from 'vitest'; + +import { type MessageDeliveryState } from '@kilocode/cloud-agent-sdk'; +import { countInFlightMessages, retryMessageAndClear } from './session-detail-content-helpers'; + +describe('countInFlightMessages', () => { + it('excludes a failed pending row from the in-flight count', () => { + const pending = new Map([ + ['m1', { status: 'queued' }], + ['m2', { status: 'failed', error: 'nope', reason: 'exhausted' }], + ]); + expect(countInFlightMessages(pending)).toBe(1); + }); + + it('returns zero when every pending row failed', () => { + const pending = new Map([ + ['m1', { status: 'failed', error: 'nope', reason: 'interrupted' }], + ]); + expect(countInFlightMessages(pending)).toBe(0); + }); + + it('counts every queued row', () => { + const pending = new Map([ + ['m1', { status: 'queued' }], + ['m2', { status: 'queued' }], + ]); + expect(countInFlightMessages(pending)).toBe(2); + }); +}); + +describe('retryMessageAndClear', () => { + it('clears the failed row when the retry send succeeds', async () => { + const send = vi.fn<() => Promise>().mockResolvedValue(undefined); + const clearFailed = vi.fn<() => void>(); + await retryMessageAndClear(send, clearFailed); + expect(send).toHaveBeenCalledTimes(1); + expect(clearFailed).toHaveBeenCalledTimes(1); + }); + + it('does not clear the failed row when the retry send fails', async () => { + const send = vi + .fn<() => Promise>() + .mockRejectedValue(new Error('Failed to send message')); + const clearFailed = vi.fn<() => void>(); + await retryMessageAndClear(send, clearFailed); + expect(send).toHaveBeenCalledTimes(1); + expect(clearFailed).not.toHaveBeenCalled(); + }); +}); diff --git a/apps/mobile/src/components/agents/session-detail-content-helpers.ts b/apps/mobile/src/components/agents/session-detail-content-helpers.ts new file mode 100644 index 0000000000..05e325db32 --- /dev/null +++ b/apps/mobile/src/components/agents/session-detail-content-helpers.ts @@ -0,0 +1,36 @@ +import { type MessageDeliveryState } from '@kilocode/cloud-agent-sdk'; + +/** + * Counts pending messages that are still in flight. A terminal delivery + * failure must not count: after `status === 'failed'` the working spinner and + * wake lock would otherwise stay on forever. + */ +export function countInFlightMessages( + pendingMessages: ReadonlyMap +): number { + let count = 0; + for (const state of pendingMessages.values()) { + if (state.status !== 'failed') { + count += 1; + } + } + return count; +} + +/** + * Re-sends a failed message and clears its failed row only on success. On + * failure the row stays so the user can retry again; the manager has already + * surfaced the failure toast, so the rejection is swallowed here. + */ +export async function retryMessageAndClear( + send: () => Promise, + clearFailed: () => void +): Promise { + try { + await send(); + clearFailed(); + } catch { + // Swallow: the manager already surfaced the failure toast and the failed + // row stays so the user can retry again. + } +} From d5840705d56fa31d903876727fcb015356ddbe78 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20=C5=A0=C4=87eki=C4=87?= Date: Wed, 19 Aug 2026 05:36:51 +0200 Subject: [PATCH 11/34] feat(cloud-agent-sdk): hydrate child sessions by bounded page Prefer fetchSnapshotPage over fetchSnapshot in hydrateChildSession so a child sheet loads a bounded page instead of the unbounded snapshot. Add per-child cursor, hasOlder, isLoadingOlder, olderError, and omittedItemCount to the ready hydration state, and loadOlderChildMessages to page a child by its own cursor without touching root pagination state. --- .../src/session-manager.test.ts | 378 +++++++++++++++++- .../cloud-agent-sdk/src/session-manager.ts | 175 +++++++- 2 files changed, 538 insertions(+), 15 deletions(-) diff --git a/packages/cloud-agent-sdk/src/session-manager.test.ts b/packages/cloud-agent-sdk/src/session-manager.test.ts index 14d725d042..588ea1a075 100644 --- a/packages/cloud-agent-sdk/src/session-manager.test.ts +++ b/packages/cloud-agent-sdk/src/session-manager.test.ts @@ -2513,7 +2513,14 @@ describe('createSessionManager', () => { config.store, mgr.atoms.childSessionHydrationState ); - expect(childHydrationState('child-1')).toEqual({ status: 'ready' }); + expect(childHydrationState('child-1')).toEqual({ + status: 'ready', + cursor: null, + hasOlder: false, + isLoadingOlder: false, + olderError: null, + omittedItemCount: 0, + }); }); it('merges fetched history into live child messages without duplicating them', async () => { @@ -2584,7 +2591,14 @@ describe('createSessionManager', () => { config.store, mgr.atoms.childSessionHydrationState ); - expect(updatedChildHydrationState('child-deduped')).toEqual({ status: 'ready' }); + expect(updatedChildHydrationState('child-deduped')).toEqual({ + status: 'ready', + cursor: null, + hasOlder: false, + isLoadingOlder: false, + olderError: null, + omittedItemCount: 0, + }); }); it('ignores stale child snapshots after the active root session changes', async () => { @@ -2646,7 +2660,365 @@ describe('createSessionManager', () => { const retriedChildHydrationState = atomValue< (childSessionId: string) => { status: string; message?: string } >(config.store, mgr.atoms.childSessionHydrationState); - expect(retriedChildHydrationState('child-retry')).toEqual({ status: 'ready' }); + expect(retriedChildHydrationState('child-retry')).toEqual({ + status: 'ready', + cursor: null, + hasOlder: false, + isLoadingOlder: false, + olderError: null, + omittedItemCount: 0, + }); + }); + + it('prefers fetchSnapshotPage over fetchSnapshot and stores the child cursor', async () => { + const childMessage = createStoredMessage('msg-child-page', 'child-page', 'assistant'); + const childPart = stubTextPart({ + id: 'part-child-page', + sessionID: 'child-page', + messageID: childMessage.info.id, + text: 'Paged child message', + }); + const fetchSnapshotPage = createPageFetchMock(async () => + makePage({ + kiloSessionId: 'child-page', + messages: [{ info: childMessage.info, parts: [childPart] }], + nextCursor: 'cursor-A', + }) + ); + const config = createMockConfig({ fetchSnapshotPage }); + const mgr = createSessionManager(config); + + await mgr.switchSession(kiloId('ses-root')); + if (!latestStorage) throw new Error('expected session storage'); + + await mgr.hydrateChildSession(kiloId('child-page')); + + expect(fetchSnapshotPage).toHaveBeenCalledWith(kiloId('child-page'), {}); + expect(config.fetchSnapshot).not.toHaveBeenCalled(); + const childMessages = atomValue<(childSessionId: string) => StoredMessage[]>( + config.store, + mgr.atoms.childMessages + ); + expect(childMessages('child-page')).toEqual([ + { info: childMessage.info, parts: [childPart] }, + ]); + const state = atomValue< + (childSessionId: string) => { + status: string; + cursor?: string | null; + hasOlder?: boolean; + isLoadingOlder?: boolean; + olderError?: unknown; + } + >(config.store, mgr.atoms.childSessionHydrationState); + expect(state('child-page')).toEqual({ + status: 'ready', + cursor: 'cursor-A', + hasOlder: true, + isLoadingOlder: false, + olderError: null, + omittedItemCount: 0, + }); + }); + + it('sets the error state when the first child page is null', async () => { + const fetchSnapshotPage = createPageFetchMock(async () => null); + const config = createMockConfig({ fetchSnapshotPage }); + const mgr = createSessionManager(config); + + await mgr.switchSession(kiloId('ses-root')); + await mgr.hydrateChildSession(kiloId('child-null')); + + const state = atomValue<(childSessionId: string) => { status: string; message?: string }>( + config.store, + mgr.atoms.childSessionHydrationState + ); + expect(state('child-null')).toEqual({ + status: 'error', + message: 'This session is no longer available.', + }); + }); + + it('sets the error state when the first child page is a typed failure', async () => { + const fetchSnapshotPage = createPageFetchMock(async () => ({ + kind: 'retryable_failure' as const, + })); + const config = createMockConfig({ fetchSnapshotPage }); + const mgr = createSessionManager(config); + + await mgr.switchSession(kiloId('ses-root')); + await mgr.hydrateChildSession(kiloId('child-fail')); + + const state = atomValue<(childSessionId: string) => { status: string; message?: string }>( + config.store, + mgr.atoms.childSessionHydrationState + ); + expect(state('child-fail')).toEqual(expect.objectContaining({ status: 'error' })); + }); + + it('loadOlderChildMessages pages by the child cursor and updates only per-child state', async () => { + const firstMessage = createStoredMessage('msg-child-old-1', 'child-old', 'assistant'); + const secondMessage = createStoredMessage('msg-child-old-2', 'child-old', 'assistant'); + const firstPart = stubTextPart({ + id: 'part-child-old-1', + sessionID: 'child-old', + messageID: firstMessage.info.id, + text: 'First page', + }); + const secondPart = stubTextPart({ + id: 'part-child-old-2', + sessionID: 'child-old', + messageID: secondMessage.info.id, + text: 'Second page', + }); + const fetchSnapshotPage = createPageFetchMock(async (_id, options) => { + if (!options.cursor) { + return makePage({ + kiloSessionId: 'child-old', + messages: [{ info: firstMessage.info, parts: [firstPart] }], + nextCursor: 'cursor-A', + }); + } + return makePage({ + kiloSessionId: 'child-old', + messages: [{ info: secondMessage.info, parts: [secondPart] }], + nextCursor: null, + }); + }); + const config = createMockConfig({ fetchSnapshotPage }); + const mgr = createSessionManager(config); + + await mgr.switchSession(kiloId('ses-root')); + if (!latestStorage) throw new Error('expected session storage'); + await mgr.hydrateChildSession(kiloId('child-old')); + + await mgr.loadOlderChildMessages(kiloId('child-old')); + + expect(fetchSnapshotPage).toHaveBeenCalledWith(kiloId('child-old'), { + cursor: 'cursor-A', + }); + const childMessages = atomValue<(childSessionId: string) => StoredMessage[]>( + config.store, + mgr.atoms.childMessages + ); + expect(childMessages('child-old')).toEqual([ + { info: firstMessage.info, parts: [firstPart] }, + { info: secondMessage.info, parts: [secondPart] }, + ]); + const state = atomValue< + (childSessionId: string) => { + status: string; + cursor?: string | null; + hasOlder?: boolean; + isLoadingOlder?: boolean; + olderError?: unknown; + } + >(config.store, mgr.atoms.childSessionHydrationState); + expect(state('child-old')).toEqual({ + status: 'ready', + cursor: null, + hasOlder: false, + isLoadingOlder: false, + olderError: null, + omittedItemCount: 0, + }); + // Root pagination state is untouched by the child load. + expect(atomValue(config.store, mgr.atoms.isLoadingOlderMessages)).toBe(false); + }); + + it('loadOlderChildMessages is a no-op for a non-ready child', async () => { + const fetchSnapshotPage = createPageFetchMock(async () => + makePage({ kiloSessionId: 'child-x' }) + ); + const config = createMockConfig({ fetchSnapshotPage }); + const mgr = createSessionManager(config); + + await mgr.switchSession(kiloId('ses-root')); + fetchSnapshotPage.mockClear(); + + await mgr.loadOlderChildMessages(kiloId('child-x')); + + expect(fetchSnapshotPage).not.toHaveBeenCalled(); + }); + + it('loadOlderChildMessages keeps status ready and surfaces a retryable older error', async () => { + const fetchSnapshotPage = createPageFetchMock(async (id, options) => { + if (id === 'ses-root') return makePage({ kiloSessionId: id, nextCursor: null }); + if (!options.cursor) return makePage({ kiloSessionId: id, nextCursor: 'cursor-A' }); + return { kind: 'retryable_failure' as const }; + }); + + const config = createMockConfig({ fetchSnapshotPage }); + const mgr = createSessionManager(config); + + await mgr.switchSession(kiloId('ses-root')); + await mgr.hydrateChildSession(kiloId('child-retryable')); + await mgr.loadOlderChildMessages(kiloId('child-retryable')); + + const state = atomValue< + (childSessionId: string) => { + status: string; + cursor?: string | null; + hasOlder?: boolean; + isLoadingOlder?: boolean; + olderError?: unknown; + omittedItemCount?: number; + } + >(config.store, mgr.atoms.childSessionHydrationState); + expect(state('child-retryable')).toEqual({ + status: 'ready', + cursor: 'cursor-A', + hasOlder: true, + isLoadingOlder: false, + olderError: { kind: 'retryable' }, + omittedItemCount: 0, + }); + // Root pagination atoms are untouched by the child's later-page failure. + expect(atomValue(config.store, mgr.atoms.isLoadingOlderMessages)).toBe(false); + expect(atomValue(config.store, mgr.atoms.hasOlderMessages)).toBe(false); + expect( + atomValue<{ kind: string } | null>(config.store, mgr.atoms.olderMessagesError) + ).toBeNull(); + }); + + it('loadOlderChildMessages maps a thrown later-page fetch to a retryable older error', async () => { + const fetchSnapshotPage = createPageFetchMock(async (id, options) => { + if (id === 'ses-root') return makePage({ kiloSessionId: id, nextCursor: null }); + if (!options.cursor) return makePage({ kiloSessionId: id, nextCursor: 'cursor-A' }); + throw new Error('fetch failed'); + }); + + const config = createMockConfig({ fetchSnapshotPage }); + const mgr = createSessionManager(config); + + await mgr.switchSession(kiloId('ses-root')); + await mgr.hydrateChildSession(kiloId('child-throw')); + await mgr.loadOlderChildMessages(kiloId('child-throw')); + + const state = atomValue< + (childSessionId: string) => { + status: string; + cursor?: string | null; + hasOlder?: boolean; + isLoadingOlder?: boolean; + olderError?: unknown; + omittedItemCount?: number; + } + >(config.store, mgr.atoms.childSessionHydrationState); + expect(state('child-throw')).toEqual({ + status: 'ready', + cursor: 'cursor-A', + hasOlder: true, + isLoadingOlder: false, + olderError: { kind: 'retryable' }, + omittedItemCount: 0, + }); + expect(atomValue(config.store, mgr.atoms.isLoadingOlderMessages)).toBe(false); + expect(atomValue(config.store, mgr.atoms.hasOlderMessages)).toBe(false); + expect( + atomValue<{ kind: string } | null>(config.store, mgr.atoms.olderMessagesError) + ).toBeNull(); + }); + + it('loadOlderChildMessages surfaces invalid_data as a non-retryable older error', async () => { + const fetchSnapshotPage = createPageFetchMock(async (id, options) => { + if (id === 'ses-root') return makePage({ kiloSessionId: id, nextCursor: null }); + if (!options.cursor) return makePage({ kiloSessionId: id, nextCursor: 'cursor-A' }); + return { kind: 'invalid_data' as const }; + }); + + const config = createMockConfig({ fetchSnapshotPage }); + const mgr = createSessionManager(config); + + await mgr.switchSession(kiloId('ses-root')); + await mgr.hydrateChildSession(kiloId('child-invalid')); + await mgr.loadOlderChildMessages(kiloId('child-invalid')); + + const state = atomValue< + (childSessionId: string) => { + status: string; + cursor?: string | null; + hasOlder?: boolean; + isLoadingOlder?: boolean; + olderError?: unknown; + omittedItemCount?: number; + } + >(config.store, mgr.atoms.childSessionHydrationState); + expect(state('child-invalid')).toEqual({ + status: 'ready', + cursor: 'cursor-A', + hasOlder: true, + isLoadingOlder: false, + olderError: { kind: 'invalid_data' }, + omittedItemCount: 0, + }); + expect(atomValue(config.store, mgr.atoms.isLoadingOlderMessages)).toBe(false); + expect(atomValue(config.store, mgr.atoms.hasOlderMessages)).toBe(false); + expect( + atomValue<{ kind: string } | null>(config.store, mgr.atoms.olderMessagesError) + ).toBeNull(); + }); + + it('loadOlderChildMessages surfaces too_large as a non-retryable older error', async () => { + const fetchSnapshotPage = createPageFetchMock(async (id, options) => { + if (id === 'ses-root') return makePage({ kiloSessionId: id, nextCursor: null }); + if (!options.cursor) return makePage({ kiloSessionId: id, nextCursor: 'cursor-A' }); + return { kind: 'too_large' as const }; + }); + + const config = createMockConfig({ fetchSnapshotPage }); + const mgr = createSessionManager(config); + + await mgr.switchSession(kiloId('ses-root')); + await mgr.hydrateChildSession(kiloId('child-large')); + await mgr.loadOlderChildMessages(kiloId('child-large')); + + const state = atomValue< + (childSessionId: string) => { + status: string; + cursor?: string | null; + hasOlder?: boolean; + isLoadingOlder?: boolean; + olderError?: unknown; + omittedItemCount?: number; + } + >(config.store, mgr.atoms.childSessionHydrationState); + expect(state('child-large')).toEqual({ + status: 'ready', + cursor: 'cursor-A', + hasOlder: true, + isLoadingOlder: false, + olderError: { kind: 'too_large' }, + omittedItemCount: 0, + }); + expect(atomValue(config.store, mgr.atoms.isLoadingOlderMessages)).toBe(false); + expect(atomValue(config.store, mgr.atoms.hasOlderMessages)).toBe(false); + expect( + atomValue<{ kind: string } | null>(config.store, mgr.atoms.olderMessagesError) + ).toBeNull(); + }); + + it('loadOlderChildMessages accumulates omittedItemCount across pages', async () => { + const fetchSnapshotPage = createPageFetchMock(async (id, options) => { + if (id === 'ses-root') return makePage({ kiloSessionId: id, nextCursor: null }); + if (!options.cursor) + return makePage({ kiloSessionId: id, nextCursor: 'cursor-A', omittedItemCount: 2 }); + return makePage({ kiloSessionId: id, nextCursor: null, omittedItemCount: 3 }); + }); + + const config = createMockConfig({ fetchSnapshotPage }); + const mgr = createSessionManager(config); + + await mgr.switchSession(kiloId('ses-root')); + await mgr.hydrateChildSession(kiloId('child-omit')); + await mgr.loadOlderChildMessages(kiloId('child-omit')); + + const state = atomValue< + (childSessionId: string) => { status: string; omittedItemCount?: number } + >(config.store, mgr.atoms.childSessionHydrationState); + expect(state('child-omit')).toEqual( + expect.objectContaining({ status: 'ready', omittedItemCount: 5 }) + ); }); }); diff --git a/packages/cloud-agent-sdk/src/session-manager.ts b/packages/cloud-agent-sdk/src/session-manager.ts index dedcc53814..d320dfb69b 100644 --- a/packages/cloud-agent-sdk/src/session-manager.ts +++ b/packages/cloud-agent-sdk/src/session-manager.ts @@ -113,7 +113,19 @@ type StandaloneSuggestion = { type ChildSessionHydrationState = | { status: 'idle' } | { status: 'loading' } - | { status: 'ready' } + | { + status: 'ready'; + /** Opaque cursor for the child's next older page, or null when fully read. */ + cursor: string | null; + /** True when the child has more older history to load. */ + hasOlder: boolean; + /** True while `loadOlderChildMessages` is fetching a page for this child. */ + isLoadingOlder: boolean; + /** Typed failure from the child's most recent older-messages load. */ + olderError: OlderMessagesError | null; + /** Total items omitted across every page loaded for this child so far. */ + omittedItemCount: number; + } | { status: 'error'; message: string }; const IDLE_CHILD_SESSION_HYDRATION_STATE = { @@ -377,6 +389,14 @@ type SessionManagerAtoms = { type SessionManager = { switchSession(kiloSessionId: KiloSessionId): Promise; hydrateChildSession(childSessionId: KiloSessionId): Promise; + /** + * Load the next page of older messages for a hydrated child session using + * that child's own cursor. Replays through the child apply path, updates + * only per-child pagination state, and never touches the root session's + * cursor or atoms. No-op when `fetchSnapshotPage` is absent, the child is + * not ready, or the child has no cursor. + */ + loadOlderChildMessages(childSessionId: KiloSessionId): Promise; /** * Load the next page of older messages for the active session using the * stored cursor. Dedupes concurrent calls, never clears existing/live @@ -447,6 +467,8 @@ type SessionManager = { // --------------------------------------------------------------------------- const GENERIC_ERROR = 'Something went wrong. Please retry in a moment.'; +/** Terminal message for a child session whose first page is a worker 404 (not-found). */ +const CHILD_SESSION_NOT_FOUND_MESSAGE = 'This session is no longer available.'; const SELECTED_MODEL_UNAVAILABLE_MESSAGE = 'selected model is not available for this cloud agent session'; const SELECTED_MODEL_UNAVAILABLE_ERROR = @@ -868,6 +890,28 @@ function createSessionManager(config: SessionManagerConfig): SessionManager { ); } + /** + * Replay a child page/snapshot's messages into the active storage through + * the chat processor. Child pages must never go through `applyPage`: that + * path drops any page whose `info.id` is not the root session id, and a + * child page never carries the root id. + */ + function replayChildMessages( + storage: JotaiSessionStorage, + messages: SessionSnapshot['messages'] + ): void { + const chatProcessor = createChatProcessor(storage, { + onToolAttachment: config.onToolAttachment, + onFilePart: config.onFilePart, + }); + for (const message of messages) { + chatProcessor.process({ type: 'message.updated', info: message.info }); + for (const part of message.parts) { + chatProcessor.process({ type: 'message.part.updated', part }); + } + } + } + async function hydrateChildSession(childSessionId: KiloSessionId): Promise { const existingState = store.get(childSessionHydrationStatesAtom).get(childSessionId); if (existingState?.status === 'ready') return; @@ -887,21 +931,52 @@ function createSessionManager(config: SessionManagerConfig): SessionManager { const request = (async () => { try { + if (config.fetchSnapshotPage) { + const page = await config.fetchSnapshotPage(childSessionId, {}); + if (!isCurrentChildSessionHydration(generation, rootSessionId, storage)) return; + + // A null page (worker 404) or any typed failure on the first page is + // a terminal hydration error for this child. + if (page === null) { + setChildSessionHydrationState(childSessionId, { + status: 'error', + message: CHILD_SESSION_NOT_FOUND_MESSAGE, + }); + return; + } + if (page.kind !== 'success') { + setChildSessionHydrationState(childSessionId, { + status: 'error', + message: formatError(page), + }); + return; + } + + replayChildMessages(storage, page.messages); + setChildSessionHydrationState(childSessionId, { + status: 'ready', + cursor: page.nextCursor, + hasOlder: page.nextCursor !== null, + isLoadingOlder: false, + olderError: null, + omittedItemCount: page.omittedItemCount, + }); + return; + } + + // Legacy fallback: full snapshot, no pagination state. const snapshot = await config.fetchSnapshot(childSessionId); if (!isCurrentChildSessionHydration(generation, rootSessionId, storage)) return; - const chatProcessor = createChatProcessor(storage, { - onToolAttachment: config.onToolAttachment, - onFilePart: config.onFilePart, + replayChildMessages(storage, snapshot.messages); + setChildSessionHydrationState(childSessionId, { + status: 'ready', + cursor: null, + hasOlder: false, + isLoadingOlder: false, + olderError: null, + omittedItemCount: 0, }); - for (const message of snapshot.messages) { - chatProcessor.process({ type: 'message.updated', info: message.info }); - for (const part of message.parts) { - chatProcessor.process({ type: 'message.part.updated', part }); - } - } - - setChildSessionHydrationState(childSessionId, { status: 'ready' }); } catch (err) { if (!isCurrentChildSessionHydration(generation, rootSessionId, storage)) return; setChildSessionHydrationState(childSessionId, { @@ -921,6 +996,81 @@ function createSessionManager(config: SessionManagerConfig): SessionManager { } } + async function loadOlderChildMessages(childSessionId: KiloSessionId): Promise { + if (!config.fetchSnapshotPage) return; + const fetchSnapshotPage = config.fetchSnapshotPage; + + const state = store.get(childSessionHydrationStatesAtom).get(childSessionId); + if (!state || state.status !== 'ready') return; + if (state.cursor === null) return; + if (state.isLoadingOlder) return; + + const storage = store.get(sessionStorageAtom); + const rootSessionId = activeSessionId; + if (!storage || !rootSessionId) return; + + const generation = childSessionHydrationGeneration; + const cursor = state.cursor; + + setChildSessionHydrationState(childSessionId, { ...state, isLoadingOlder: true }); + + let outcome: SessionSnapshotPageOutcome | null; + try { + outcome = await fetchSnapshotPage(childSessionId, { cursor }); + } catch (_err) { + // Network/transport-level failure maps to a retryable older error. The + // cursor is preserved so a retry continues from here. + if (!isCurrentChildSessionHydration(generation, rootSessionId, storage)) return; + const current = store.get(childSessionHydrationStatesAtom).get(childSessionId); + if (!current || current.status !== 'ready') return; + setChildSessionHydrationState(childSessionId, { + ...current, + isLoadingOlder: false, + olderError: { kind: 'retryable' }, + }); + return; + } + if (!isCurrentChildSessionHydration(generation, rootSessionId, storage)) return; + + const current = store.get(childSessionHydrationStatesAtom).get(childSessionId); + if (!current || current.status !== 'ready') return; + + if (outcome === null) { + // Access-not-found (worker 404): terminal for this child. + setChildSessionHydrationState(childSessionId, { + ...current, + isLoadingOlder: false, + hasOlder: false, + olderError: { kind: 'invalid_data' }, + }); + return; + } + + if (outcome.kind === 'success') { + replayChildMessages(storage, outcome.messages); + setChildSessionHydrationState(childSessionId, { + ...current, + cursor: outcome.nextCursor, + hasOlder: outcome.nextCursor !== null, + isLoadingOlder: false, + olderError: null, + omittedItemCount: current.omittedItemCount + outcome.omittedItemCount, + }); + return; + } + + // Typed failure. A later-page failure only writes `olderError`; it never + // changes the hydration status (a first-page failure is handled by + // `hydrateChildSession`). `retryable_failure` maps to the retryable kind; + // `invalid_data` and `too_large` map directly. + setChildSessionHydrationState(childSessionId, { + ...current, + isLoadingOlder: false, + olderError: + outcome.kind === 'retryable_failure' ? { kind: 'retryable' } : { kind: outcome.kind }, + }); + } + function updateCapabilityAtoms(session: CloudAgentSession): void { const cloudStatus = store.get(cloudStatusAtom); const cloudReady = cloudStatus === null || cloudStatus.type === 'ready'; @@ -1946,6 +2096,7 @@ function createSessionManager(config: SessionManagerConfig): SessionManager { return { switchSession, hydrateChildSession, + loadOlderChildMessages, loadOlderMessages, trimRetainedHistory, updateFetchedAssociatedPr, From 185045a014a332f0317e65e5f0f640d622a16311 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20=C5=A0=C4=87eki=C4=87?= Date: Wed, 19 Aug 2026 05:36:57 +0200 Subject: [PATCH 12/34] feat(mobile): bound attachment uploads and cancel in-flight work Reject oversized clipboard images before they reach disk, bound the chip thumbnail decode, and expose the upload task's cancelAsync so remove, reset, session switch, and unmount cancel in-flight uploads without a toast. Delete cache-owned partial files but never picker-provided URIs. --- .../agents/attachment-preview-strip.tsx | 3 + .../src/components/agents/chat-composer.tsx | 10 +- .../components/agents/new-session-prompt.tsx | 6 +- .../agent-attachments/clipboard-image.test.ts | 66 +++++++++ .../lib/agent-attachments/clipboard-image.ts | 29 +++- .../src/lib/agent-attachments/upload-task.ts | 14 +- .../use-agent-attachment-upload.test.ts | 130 +++++++++++++++++- .../use-agent-attachment-upload.ts | 82 ++++++++++- .../use-clipboard-paste.test.ts | 48 +++++-- .../agent-attachments/use-clipboard-paste.ts | 23 +++- 10 files changed, 375 insertions(+), 36 deletions(-) diff --git a/apps/mobile/src/components/agents/attachment-preview-strip.tsx b/apps/mobile/src/components/agents/attachment-preview-strip.tsx index 762effa8cf..4c1d0ea2aa 100644 --- a/apps/mobile/src/components/agents/attachment-preview-strip.tsx +++ b/apps/mobile/src/components/agents/attachment-preview-strip.tsx @@ -147,6 +147,9 @@ function AttachmentChip({ className="h-full w-full" contentFit="cover" transition={0} + allowDownscaling + recyclingKey={attachment.id} + cachePolicy="memory" /> ) : ( { toast.error( - reason === 'empty' - ? CLIPBOARD_PASTE_EMPTY_MESSAGE - : describeClassificationFailure('unreadable') + reason === 'empty' ? CLIPBOARD_PASTE_EMPTY_MESSAGE : describeClassificationFailure(reason) ); }, + maxBytes: AGENT_ATTACHMENT_MAX_BYTES, }); const commandList = useMemo( diff --git a/apps/mobile/src/components/agents/new-session-prompt.tsx b/apps/mobile/src/components/agents/new-session-prompt.tsx index e73a2f1976..71585533ba 100644 --- a/apps/mobile/src/components/agents/new-session-prompt.tsx +++ b/apps/mobile/src/components/agents/new-session-prompt.tsx @@ -30,6 +30,7 @@ import { applyVoiceDraftToInput } from '@/lib/voice-input/voice-input-draft'; import { useVoiceInput } from '@/lib/voice-input/use-voice-input'; import { VoiceInputButton, VoiceInputStatus } from '@/components/voice-input-control'; import { describeClassificationFailure } from '@/lib/agent-attachments/validate'; +import { AGENT_ATTACHMENT_MAX_BYTES } from '@/lib/agent-attachments/constants'; import { CLIPBOARD_PASTE_EMPTY_MESSAGE, useClipboardPaste, @@ -181,11 +182,10 @@ export function NewSessionPrompt({ }, onFailure: reason => { toast.error( - reason === 'empty' - ? CLIPBOARD_PASTE_EMPTY_MESSAGE - : describeClassificationFailure('unreadable') + reason === 'empty' ? CLIPBOARD_PASTE_EMPTY_MESSAGE : describeClassificationFailure(reason) ); }, + maxBytes: AGENT_ATTACHMENT_MAX_BYTES, }); function handlePromptInputLayout(event: LayoutChangeEvent) { diff --git a/apps/mobile/src/lib/agent-attachments/clipboard-image.test.ts b/apps/mobile/src/lib/agent-attachments/clipboard-image.test.ts index ebd1a12ed9..95aba97f0d 100644 --- a/apps/mobile/src/lib/agent-attachments/clipboard-image.test.ts +++ b/apps/mobile/src/lib/agent-attachments/clipboard-image.test.ts @@ -1,6 +1,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { + decodedBase64ByteLength, hasClipboardImage, hasClipboardUrl, parseClipboardImageData, @@ -110,6 +111,27 @@ describe('parseClipboardImageData', () => { }); }); +describe('decodedBase64ByteLength', () => { + it('decodes an unpadded payload exactly', () => { + expect(decodedBase64ByteLength('TWFu')).toBe(3); + expect(decodedBase64ByteLength('AAAA')).toBe(3); + }); + + it('decodes a single-padded payload exactly', () => { + expect(decodedBase64ByteLength('TWE=')).toBe(2); + expect(decodedBase64ByteLength('iVBORw0KGgo=')).toBe(8); + }); + + it('decodes a double-padded payload exactly', () => { + expect(decodedBase64ByteLength('TQ==')).toBe(1); + expect(decodedBase64ByteLength('AA==')).toBe(1); + }); + + it('returns 0 for an empty payload', () => { + expect(decodedBase64ByteLength('')).toBe(0); + }); +}); + describe('hasClipboardImage', () => { it('returns true when hasImageAsync resolves true', async () => { clipboardMock.hasImageAsync.mockResolvedValue(true); @@ -214,4 +236,48 @@ describe('readClipboardImageFile', () => { expect(result).toBeNull(); expect(fileInstances[0]?.write).toHaveBeenCalled(); }); + + it("returns 'too-large' when maxBytes is set and the decoded payload exceeds it", async () => { + // 'iVBORw0KGgo=' decodes to 8 bytes. + clipboardMock.getImageAsync.mockResolvedValue({ + data: 'data:image/png;base64,iVBORw0KGgo=', + }); + + const result = await readClipboardImageFile(7); + + expect(result).toBe('too-large'); + // No directory is created and no bytes reach disk. + expect(expoFileSystemMock.Directory).not.toHaveBeenCalled(); + expect(fileInstances).toHaveLength(0); + }); + + it('writes the file when the decoded payload is within maxBytes', async () => { + clipboardMock.getImageAsync.mockResolvedValue({ + data: 'data:image/png;base64,iVBORw0KGgo=', + }); + + const result = await readClipboardImageFile(8); + + expect(result).toEqual({ + uri: 'file:///cache/clipboard-images/pasted-image-uuid-1.png', + name: 'pasted-image.png', + mimeType: 'image/png', + }); + expect(fileInstances).toHaveLength(1); + }); + + it('does not apply the bound when maxBytes is omitted', async () => { + clipboardMock.getImageAsync.mockResolvedValue({ + data: 'data:image/png;base64,iVBORw0KGgo=', + }); + + const result = await readClipboardImageFile(); + + expect(result).toEqual({ + uri: 'file:///cache/clipboard-images/pasted-image-uuid-1.png', + name: 'pasted-image.png', + mimeType: 'image/png', + }); + expect(fileInstances).toHaveLength(1); + }); }); diff --git a/apps/mobile/src/lib/agent-attachments/clipboard-image.ts b/apps/mobile/src/lib/agent-attachments/clipboard-image.ts index ab74c08bbc..a7fd43fc64 100644 --- a/apps/mobile/src/lib/agent-attachments/clipboard-image.ts +++ b/apps/mobile/src/lib/agent-attachments/clipboard-image.ts @@ -15,6 +15,20 @@ export type ParsedClipboardImage = { extension: 'png' | 'jpg'; }; +/** + * Exact decoded byte length of a base64 payload, without decoding it. + * Handles padded (`==`), single-padded (`=`), and unpadded payloads. + */ +export function decodedBase64ByteLength(payload: string): number { + let padding = 0; + if (payload.endsWith('==')) { + padding = 2; + } else if (payload.endsWith('=')) { + padding = 1; + } + return Math.floor((payload.length * 3) / 4) - padding; +} + /** * Parse a `data:;base64,` string returned by * `expo-clipboard`'s `getImageAsync`. Accepts only PNG and JPEG. @@ -94,13 +108,17 @@ export type ClipboardImageFile = { uri: string; name: string; mimeType: string } * * 1. Requests a PNG (`format: 'png'`) from the clipboard. * 2. Parses the returned data URI. - * 3. Writes the decoded base64 payload into `Paths.cache/clipboard-images/` + * 3. Rejects an oversized image before any directory or file is created. + * 4. Writes the decoded base64 payload into `Paths.cache/clipboard-images/` * through the modern `expo-file-system` API. * - * Returns `null` on every failure: clipboard empty, permission denied, - * unsupported type, or a write error. + * Returns `'too-large'` when `maxBytes` is set and the decoded payload + * exceeds it. Returns `null` on every other failure: clipboard empty, + * permission denied, unsupported type, or a write error. */ -export async function readClipboardImageFile(): Promise { +export async function readClipboardImageFile( + maxBytes?: number +): Promise { try { const image = await Clipboard.getImageAsync({ format: 'png' }); if (!image) { @@ -110,6 +128,9 @@ export async function readClipboardImageFile(): Promise maxBytes) { + return 'too-large'; + } const directory = new Directory(Paths.cache, 'clipboard-images'); directory.create({ idempotent: true, intermediates: true }); const filename = `pasted-image-${Crypto.randomUUID()}.${parsed.extension}`; diff --git a/apps/mobile/src/lib/agent-attachments/upload-task.ts b/apps/mobile/src/lib/agent-attachments/upload-task.ts index ef64d87f2a..fae2a0119d 100644 --- a/apps/mobile/src/lib/agent-attachments/upload-task.ts +++ b/apps/mobile/src/lib/agent-attachments/upload-task.ts @@ -43,9 +43,18 @@ export async function uploadOne(args: { contentLength: number; localUri: string; onProgress: (progress: number | null) => void; + onTask?: (task: { cancelAsync: () => Promise }) => void; }): Promise { - const { organizationId, attachmentId, path, contentType, contentLength, localUri, onProgress } = - args; + const { + organizationId, + attachmentId, + path, + contentType, + contentLength, + localUri, + onProgress, + onTask, + } = args; const baseInput = { messageUuid: path, attachmentId, @@ -82,6 +91,7 @@ export async function uploadOne(args: { } } ); + onTask?.(task); const uploadResult = await task.uploadAsync(); if (!uploadResult || uploadResult.status < 200 || uploadResult.status >= 300) { throw new Error(`Upload failed with status ${uploadResult?.status ?? 'no response'}`); diff --git a/apps/mobile/src/lib/agent-attachments/use-agent-attachment-upload.test.ts b/apps/mobile/src/lib/agent-attachments/use-agent-attachment-upload.test.ts index 718b849292..efc9ea12dd 100644 --- a/apps/mobile/src/lib/agent-attachments/use-agent-attachment-upload.test.ts +++ b/apps/mobile/src/lib/agent-attachments/use-agent-attachment-upload.test.ts @@ -32,6 +32,8 @@ const hoisted = vi.hoisted(() => { announceForA11y: vi.fn(), announcingToastError: vi.fn(), measureLocalSize: vi.fn(), + cancelAsync: vi.fn(), + fileDelete: vi.fn(), }; }); @@ -49,6 +51,21 @@ vi.mock('@/lib/agent-attachments/upload-task', () => ({ describeTerminalReason: () => "This file can't be uploaded.", uploadOne: hoisted.uploadOne, })); +vi.mock('expo-file-system', () => { + class FileMock { + uri: string; + constructor(uri: string) { + this.uri = uri; + } + delete() { + hoisted.fileDelete(this.uri); + } + } + return { + File: FileMock, + Paths: { cache: { uri: 'file:///cache' } }, + }; +}); function makeAttachment(overrides: Partial): AgentAttachment { return { @@ -380,6 +397,8 @@ describe('useAgentAttachmentUpload — announcement ownership (Row 3.3)', () => hoisted.announceForA11y.mockReset(); hoisted.announcingToastError.mockReset(); hoisted.measureLocalSize.mockReset(); + hoisted.cancelAsync.mockReset(); + hoisted.fileDelete.mockReset(); hoisted.measureLocalSize.mockResolvedValue(1024); resolveUpload = undefined; rejectUpload = undefined; @@ -389,7 +408,15 @@ describe('useAgentAttachmentUpload — announcement ownership (Row 3.3)', () => resolveUpload = resolve; rejectUpload = reject; }); - hoisted.uploadOne.mockReturnValue(controlled); + // The mock mirrors `uploadOne`'s real contract: it hands the created + // task's `cancelAsync` back through `onTask` before the upload settles. + hoisted.uploadOne.mockImplementation( + async (args: { onTask?: (task: { cancelAsync: () => Promise }) => void }) => { + args.onTask?.({ cancelAsync: hoisted.cancelAsync }); + const result = await controlled; + return result; + } + ); }); it('announces success exactly once when the upload resolves', async () => { @@ -564,4 +591,105 @@ describe('useAgentAttachmentUpload — announcement ownership (Row 3.3)', () => expect(hoisted.announcingToastError).not.toHaveBeenCalled(); renderer.unmount(); }); + + it('cancels the in-flight upload and deletes a cache-owned file on remove', async () => { + const renderer = await mountHook(); + await addDocument(); + const id = hookApi().attachments[0]?.id; + if (!id) { + throw new Error('attachment id missing'); + } + + await act(async () => { + hookApi().removeAttachment(id); + await settle(); + }); + + expect(hoisted.cancelAsync).toHaveBeenCalledTimes(1); + expect(hoisted.fileDelete).toHaveBeenCalledTimes(1); + expect(hoisted.fileDelete).toHaveBeenCalledWith('file:///cache/doc.pdf'); + renderer.unmount(); + }); + + it('does not delete a picker-provided URI on remove', async () => { + const renderer = await mountHook(); + await act(async () => { + await hookApi().addCandidates([{ name: 'doc.pdf', uri: 'file:///documents/picked.pdf' }]); + }); + const id = hookApi().attachments[0]?.id; + if (!id) { + throw new Error('attachment id missing'); + } + + await act(async () => { + hookApi().removeAttachment(id); + await settle(); + }); + + expect(hoisted.cancelAsync).toHaveBeenCalledTimes(1); + expect(hoisted.fileDelete).not.toHaveBeenCalled(); + renderer.unmount(); + }); + + it('cancels every in-flight upload and deletes cache-owned files on reset', async () => { + const renderer = await mountHook(); + await addDocument(); + + await act(async () => { + hookApi().reset(); + await settle(); + }); + + expect(hoisted.cancelAsync).toHaveBeenCalledTimes(1); + expect(hoisted.fileDelete).toHaveBeenCalledTimes(1); + renderer.unmount(); + }); + + it('cancels every in-flight upload on unmount with no toast and no state flip', async () => { + const renderer = await mountHook(); + await addDocument(); + + await act(async () => { + renderer.unmount(); + await settle(); + }); + + expect(hoisted.cancelAsync).toHaveBeenCalledTimes(1); + expect(hoisted.fileDelete).toHaveBeenCalledTimes(1); + + // The cancelled upload later rejects: unmount invalidated the live id, so + // the catch emits no toast and flips no state. + await act(async () => { + rejectUpload?.(new TypeError('Network request failed')); + await settle(); + }); + + expect(hoisted.announcingToastError).not.toHaveBeenCalled(); + expect(hoisted.announceForA11y).not.toHaveBeenCalled(); + }); + + it('a cancelled upload emits no toast and no state flip when it later rejects', async () => { + const renderer = await mountHook(); + await addDocument(); + const id = hookApi().attachments[0]?.id; + if (!id) { + throw new Error('attachment id missing'); + } + + await act(async () => { + hookApi().removeAttachment(id); + await settle(); + }); + expect(hookApi().attachments).toHaveLength(0); + + await act(async () => { + rejectUpload?.(new TypeError('Network request failed')); + await settle(); + }); + + expect(hoisted.announcingToastError).not.toHaveBeenCalled(); + expect(hoisted.announceForA11y).not.toHaveBeenCalled(); + expect(hookApi().attachments).toHaveLength(0); + renderer.unmount(); + }); }); diff --git a/apps/mobile/src/lib/agent-attachments/use-agent-attachment-upload.ts b/apps/mobile/src/lib/agent-attachments/use-agent-attachment-upload.ts index 56038b3bbb..3385e752c7 100644 --- a/apps/mobile/src/lib/agent-attachments/use-agent-attachment-upload.ts +++ b/apps/mobile/src/lib/agent-attachments/use-agent-attachment-upload.ts @@ -1,4 +1,5 @@ import * as Crypto from 'expo-crypto'; +import { File, Paths } from 'expo-file-system'; import { useCallback, useEffect, useMemo, useRef, useState } from 'react'; import { toast } from 'sonner-native'; @@ -38,6 +39,34 @@ export type AgentAttachmentCandidate = { size?: number; }; +/** + * Delete a clipboard-written cache file after its upload is cancelled. A + * picker-provided URI is not owned by the app and is never deleted. Best-effort: + * a failed delete is swallowed so it can never surface as an upload error. + */ +function deleteCacheOwnedFile(localUri: string): void { + if (!localUri.startsWith(Paths.cache.uri)) { + return; + } + try { + new File(localUri).delete(); + } catch { + // Best-effort cleanup. + } +} + +/** + * Run a cancel handle and swallow its rejection. Cancellation is best-effort + * and must never surface as an unhandled rejection. + */ +async function runCancellation(handle: () => Promise): Promise { + try { + await handle(); + } catch { + // Best-effort cancellation. + } +} + type UseAgentAttachmentUploadOptions = { organizationId?: string; }; @@ -71,13 +100,37 @@ export function useAgentAttachmentUpload( // before ids exist: a reset while candidate measurement is in flight. const generationRef = useRef(0); const liveIdsRef = useRef>(new Set()); + // Cancel handles for in-flight uploads, keyed by attachment id. Each handle + // cancels the upload task and deletes a cache-owned partial file. The entry + // is removed when the upload settles (in `startUpload`'s finally) or when a + // cancel runs. + const cancelHandlesRef = useRef(new Map Promise>()); + + const cancelUpload = useCallback((id: string) => { + const handle = cancelHandlesRef.current.get(id); + if (!handle) { + return; + } + cancelHandlesRef.current.delete(id); + void runCancellation(handle); + }, []); useEffect(() => { isMountedRef.current = true; + const handles = cancelHandlesRef.current; + const liveIds = liveIdsRef.current; return () => { isMountedRef.current = false; + // Invalidate the live ids before cancelling so a cancel-triggered + // rejection in `uploadOne` is suppressed by the catch's `liveIdsRef` + // guard: unmount must emit no toast and flip no state. + liveIds.clear(); + // Cancel every in-flight upload on unmount. + for (const id of handles.keys()) { + cancelUpload(id); + } }; - }, []); + }, [cancelUpload]); const updateAttachment = useCallback((id: string, patch: Partial) => { if (!isMountedRef.current) { @@ -114,6 +167,12 @@ export function useAgentAttachmentUpload( onProgress: progress => { updateAttachment(attachment.id, { progress }); }, + onTask: task => { + cancelHandlesRef.current.set(attachment.id, async () => { + await task.cancelAsync(); + deleteCacheOwnedFile(attachment.localUri); + }); + }, }); // Row 3.3 stale-outcome guard: a removed or reset upload must not // flip state or announce for the current composer. Ids are UUIDs, so @@ -151,6 +210,10 @@ export function useAgentAttachmentUpload( announcingToast.error( retryable ? `Failed to upload file: ${reason}` : describeTerminalReason(reason) ); + } finally { + // The upload settled (success or failure): drop the cancel handle so + // a later remove/reset does not try to cancel a finished task. + cancelHandlesRef.current.delete(attachment.id); } }; void run(); @@ -227,10 +290,14 @@ export function useAgentAttachmentUpload( [attachments.length, startUpload] ); - const removeAttachment = useCallback((id: string) => { - liveIdsRef.current.delete(id); - setAttachments(current => current.filter(item => item.id !== id)); - }, []); + const removeAttachment = useCallback( + (id: string) => { + cancelUpload(id); + liveIdsRef.current.delete(id); + setAttachments(current => current.filter(item => item.id !== id)); + }, + [cancelUpload] + ); const retryAttachment = useCallback( (id: string) => { @@ -250,11 +317,14 @@ export function useAgentAttachmentUpload( // candidate-measurement continuation observes the new generation // and drops its candidates instead of adding them post-reset. generationRef.current += 1; + for (const id of liveIdsRef.current) { + cancelUpload(id); + } liveIdsRef.current.clear(); setAttachments([]); pathRef.current = Crypto.randomUUID(); messageUuidRef.current = Crypto.randomUUID(); - }, []); + }, [cancelUpload]); const toWirePayload = useCallback( (): AgentAttachmentWire | undefined => buildWirePayload(attachments, pathRef.current), diff --git a/apps/mobile/src/lib/agent-attachments/use-clipboard-paste.test.ts b/apps/mobile/src/lib/agent-attachments/use-clipboard-paste.test.ts index 9a45f38f7e..63fb014df6 100644 --- a/apps/mobile/src/lib/agent-attachments/use-clipboard-paste.test.ts +++ b/apps/mobile/src/lib/agent-attachments/use-clipboard-paste.test.ts @@ -8,7 +8,7 @@ import { useClipboardPaste } from './use-clipboard-paste'; const hasClipboardImageMock = vi.hoisted(() => vi.fn<() => Promise>()); const hasClipboardUrlMock = vi.hoisted(() => vi.fn<() => Promise>()); const readClipboardImageFileMock = vi.hoisted(() => - vi.fn<() => Promise<{ uri: string; name: string; mimeType: string } | null>>() + vi.fn<() => Promise<{ uri: string; name: string; mimeType: string } | 'too-large' | null>>() ); const readClipboardTextMock = vi.hoisted(() => vi.fn<() => Promise>()); const setHasImageMock = vi.hoisted(() => vi.fn<(value: boolean) => void>()); @@ -45,13 +45,14 @@ function makeOptions(overrides?: { enabled?: boolean; addFile?: () => Promise; addText?: (text: string) => void; - onFailure?: (reason: 'empty' | 'unreadable') => void; + onFailure?: (reason: 'empty' | 'unreadable' | 'too-large') => void; }) { return { enabled: overrides?.enabled ?? true, addFile: overrides?.addFile ?? vi.fn().mockResolvedValue(undefined), addText: overrides?.addText, - onFailure: overrides?.onFailure ?? vi.fn<(reason: 'empty' | 'unreadable') => void>(), + onFailure: + overrides?.onFailure ?? vi.fn<(reason: 'empty' | 'unreadable' | 'too-large') => void>(), }; } @@ -194,7 +195,7 @@ describe('useClipboardPaste', () => { hasClipboardImageMock.mockResolvedValue(true); readClipboardImageFileMock.mockResolvedValue(null); - const onFailure = vi.fn<(reason: 'empty' | 'unreadable') => void>(); + const onFailure = vi.fn<(reason: 'empty' | 'unreadable' | 'too-large') => void>(); const hook = useClipboardPaste(makeOptions({ onFailure })); // Refresh shows the image. @@ -215,6 +216,31 @@ describe('useClipboardPaste', () => { expect(lastSetHasImageArg()).toBe(true); }); + it('reports too-large, does not consume, and does not fall back to text', async () => { + hasClipboardImageMock.mockResolvedValue(true); + readClipboardImageFileMock.mockResolvedValue('too-large'); + readClipboardTextMock.mockResolvedValue('fallback text'); + + const addText = vi.fn<(text: string) => void>(); + const onFailure = vi.fn<(reason: 'empty' | 'unreadable' | 'too-large') => void>(); + const hook = useClipboardPaste(makeOptions({ addText, onFailure })); + + hook.paste(); + await flushUntilCalled(onFailure); + + expect(onFailure).toHaveBeenCalledOnce(); + expect(onFailure).toHaveBeenCalledWith('too-large'); + // An oversized image must not paste its text. + expect(addText).not.toHaveBeenCalled(); + expect(readClipboardTextMock).not.toHaveBeenCalled(); + + // consumedRef is not set: a refresh still shows the hint, so a later + // smaller image can be pasted. + hook.refresh(); + await flushMicrotasks(); + expect(lastSetHasImageArg()).toBe(true); + }); + // ── Text fallback: paste is always available, so text must paste ──────── it('pastes clipboard text without reading an image the clipboard does not hold', async () => { @@ -223,7 +249,7 @@ describe('useClipboardPaste', () => { readClipboardTextMock.mockResolvedValue('https://example.com/spec'); const addText = vi.fn<(text: string) => void>(); - const onFailure = vi.fn<(reason: 'empty' | 'unreadable') => void>(); + const onFailure = vi.fn<(reason: 'empty' | 'unreadable' | 'too-large') => void>(); const hook = useClipboardPaste(makeOptions({ addText, onFailure })); hook.paste(); @@ -245,7 +271,7 @@ describe('useClipboardPaste', () => { readClipboardTextMock.mockResolvedValue('fallback text'); const addText = vi.fn<(text: string) => void>(); - const onFailure = vi.fn<(reason: 'empty' | 'unreadable') => void>(); + const onFailure = vi.fn<(reason: 'empty' | 'unreadable' | 'too-large') => void>(); const hook = useClipboardPaste(makeOptions({ addText, onFailure })); hook.paste(); @@ -265,7 +291,7 @@ describe('useClipboardPaste', () => { readClipboardTextMock.mockResolvedValue(''); const addText = vi.fn<(text: string) => void>(); - const onFailure = vi.fn<(reason: 'empty' | 'unreadable') => void>(); + const onFailure = vi.fn<(reason: 'empty' | 'unreadable' | 'too-large') => void>(); const hook = useClipboardPaste(makeOptions({ addText, onFailure })); hook.paste(); @@ -283,7 +309,7 @@ describe('useClipboardPaste', () => { readClipboardImageFileMock.mockResolvedValue(null); readClipboardTextMock.mockResolvedValue('some text'); - const onFailure = vi.fn<(reason: 'empty' | 'unreadable') => void>(); + const onFailure = vi.fn<(reason: 'empty' | 'unreadable' | 'too-large') => void>(); const hook = useClipboardPaste(makeOptions({ onFailure })); hook.paste(); @@ -299,7 +325,7 @@ describe('useClipboardPaste', () => { hasClipboardImageMock.mockResolvedValue(false); readClipboardImageFileMock.mockResolvedValue(null); - const onFailure = vi.fn<(reason: 'empty' | 'unreadable') => void>(); + const onFailure = vi.fn<(reason: 'empty' | 'unreadable' | 'too-large') => void>(); const hook = useClipboardPaste(makeOptions({ onFailure })); hook.paste(); @@ -318,7 +344,7 @@ describe('useClipboardPaste', () => { readClipboardTextMock.mockResolvedValue(''); const addText = vi.fn<(text: string) => void>(); - const onFailure = vi.fn<(reason: 'empty' | 'unreadable') => void>(); + const onFailure = vi.fn<(reason: 'empty' | 'unreadable' | 'too-large') => void>(); const hook = useClipboardPaste(makeOptions({ addText, onFailure })); hook.paste(); @@ -335,7 +361,7 @@ describe('useClipboardPaste', () => { readClipboardTextMock.mockResolvedValue(''); const addText = vi.fn<(text: string) => void>(); - const onFailure = vi.fn<(reason: 'empty' | 'unreadable') => void>(); + const onFailure = vi.fn<(reason: 'empty' | 'unreadable' | 'too-large') => void>(); const hook = useClipboardPaste(makeOptions({ addText, onFailure })); hook.paste(); diff --git a/apps/mobile/src/lib/agent-attachments/use-clipboard-paste.ts b/apps/mobile/src/lib/agent-attachments/use-clipboard-paste.ts index 23256d86ee..b158ada924 100644 --- a/apps/mobile/src/lib/agent-attachments/use-clipboard-paste.ts +++ b/apps/mobile/src/lib/agent-attachments/use-clipboard-paste.ts @@ -27,10 +27,14 @@ type UseClipboardPasteOptions = { /** Called when neither an image nor text could be used. `'empty'` means no * image, no readable text, and no URL on the clipboard. `'unreadable'` means * an image or URL was present but the read failed or was denied. A denied - * text read is indistinguishable from empty and reports 'empty'. The caller - * supplies its own toast copy to match that composer's existing pick-path - * message. */ - onFailure: (reason: 'empty' | 'unreadable') => void; + * text read is indistinguishable from empty and reports 'empty'. `'too-large'` + * means the clipboard image exceeds `maxBytes`; nothing was written to disk. + * The caller supplies its own toast copy to match that composer's existing + * pick-path message. */ + onFailure: (reason: 'empty' | 'unreadable' | 'too-large') => void; + /** Reject a clipboard image whose decoded byte length exceeds this bound + * before any file is written. Omit to skip the bound (kilo-chat). */ + maxBytes?: number; }; type UseClipboardPasteReturn = { @@ -70,10 +74,12 @@ export function useClipboardPaste(options: UseClipboardPasteOptions): UseClipboa const addFileRef = useRef(options.addFile); const addTextRef = useRef(options.addText); const onFailureRef = useRef(options.onFailure); + const maxBytesRef = useRef(options.maxBytes); useEffect(() => { addFileRef.current = options.addFile; addTextRef.current = options.addText; onFailureRef.current = options.onFailure; + maxBytesRef.current = options.maxBytes; }); const inFlightRef = useRef(false); @@ -141,7 +147,14 @@ export function useClipboardPaste(options: UseClipboardPasteOptions): UseClipboa // path without the image read that would raise a second iOS 16 paste // prompt for content that is not there. const clipboardHasImage = await hasClipboardImage(); - const file = clipboardHasImage ? await readClipboardImageFile() : null; + const file = clipboardHasImage ? await readClipboardImageFile(maxBytesRef.current) : null; + if (file === 'too-large') { + // Oversized clipboard image: nothing reached disk. Do not fall + // through to the text path, and leave `consumedRef` alone so the + // user can copy a smaller image and paste again. + onFailureRef.current('too-large'); + return; + } if (!file) { // No readable image. A caller with an always-present paste control // accepts text, so a text clipboard pastes instead of toasting. From 9d0b9e86e4bdcd38ed5ddeee244a5e1727abc5d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20=C5=A0=C4=87eki=C4=87?= Date: Wed, 19 Aug 2026 05:37:01 +0200 Subject: [PATCH 13/34] feat(mobile): one metadata read per open and paged child sheet Skip the first focus refetch (owned by switchSession) while keeping the 4s pending-decision follow-up, and wire the child sheet's pagination props through the per-child hydration state including the omitted-item count. --- .../agents/child-session-sheet-state.test.ts | 14 +++- .../components/agents/child-session-sheet.tsx | 29 +++++--- .../agents/session-detail-content.tsx | 70 ++++++++++++++++++- .../agents/session-focus-refetch.test.ts | 17 +++++ .../agents/session-focus-refetch.ts | 12 ++++ 5 files changed, 129 insertions(+), 13 deletions(-) create mode 100644 apps/mobile/src/components/agents/session-focus-refetch.test.ts create mode 100644 apps/mobile/src/components/agents/session-focus-refetch.ts diff --git a/apps/mobile/src/components/agents/child-session-sheet-state.test.ts b/apps/mobile/src/components/agents/child-session-sheet-state.test.ts index a9c1f6b336..42af4a528c 100644 --- a/apps/mobile/src/components/agents/child-session-sheet-state.test.ts +++ b/apps/mobile/src/components/agents/child-session-sheet-state.test.ts @@ -19,7 +19,19 @@ describe('getChildSessionSheetState', () => { }); it('shows an empty state after successful hydration with no messages', () => { - expect(getChildSessionSheetState({ status: 'ready' }, 0)).toBe('empty'); + expect( + getChildSessionSheetState( + { + status: 'ready', + cursor: null, + hasOlder: false, + isLoadingOlder: false, + olderError: null, + omittedItemCount: 0, + }, + 0 + ) + ).toBe('empty'); }); it('shows an error after failed hydration with no messages', () => { diff --git a/apps/mobile/src/components/agents/child-session-sheet.tsx b/apps/mobile/src/components/agents/child-session-sheet.tsx index d6a0ece619..d3f15a699c 100644 --- a/apps/mobile/src/components/agents/child-session-sheet.tsx +++ b/apps/mobile/src/components/agents/child-session-sheet.tsx @@ -1,7 +1,11 @@ import { type ReactNode } from 'react'; import { Modal, View } from 'react-native'; import { useSafeAreaInsets } from 'react-native-safe-area-context'; -import { type ChildSessionHydrationState, type StoredMessage } from '@kilocode/cloud-agent-sdk'; +import { + type ChildSessionHydrationState, + type OlderMessagesError, + type StoredMessage, +} from '@kilocode/cloud-agent-sdk'; import { EmptyState } from '@/components/empty-state'; import { QueryError } from '@/components/query-error'; @@ -26,6 +30,11 @@ type ChildSessionSheetProps = { getChildMessages: (sessionId: string) => StoredMessage[]; hydrationState: ChildSessionHydrationState; isStreaming: boolean; + hasOlderMessages: boolean; + isLoadingOlderMessages: boolean; + olderMessagesError: OlderMessagesError | null; + olderMessagesOmittedItemCount: number; + onLoadOlderMessages: () => void; renderPart: RenderPartFn; onOpenChildSession: OpenChildSession; onRetry: () => void; @@ -34,9 +43,6 @@ type ChildSessionSheetProps = { onDismiss?: () => void; }; -// eslint-disable-next-line no-empty-function -- child sessions are hydrated one-shot, no pagination -function noopLoadOlder(): void {} - export function ChildSessionSheet({ visible, sessionId, @@ -44,6 +50,11 @@ export function ChildSessionSheet({ getChildMessages, hydrationState, isStreaming, + hasOlderMessages, + isLoadingOlderMessages, + olderMessagesError, + olderMessagesOmittedItemCount, + onLoadOlderMessages, renderPart, onOpenChildSession, onRetry, @@ -66,11 +77,11 @@ export function ChildSessionSheet({ sessionId={sessionId} items={messages} keyExtractor={message => message.info.id} - hasOlderMessages={false} - isLoadingOlderMessages={false} - olderMessagesError={null} - olderMessagesOmittedItemCount={0} - onLoadOlderMessages={noopLoadOlder} + hasOlderMessages={hasOlderMessages} + isLoadingOlderMessages={isLoadingOlderMessages} + olderMessagesError={olderMessagesError} + olderMessagesOmittedItemCount={olderMessagesOmittedItemCount} + onLoadOlderMessages={onLoadOlderMessages} renderItem={({ item }) => ( diff --git a/apps/mobile/src/components/agents/session-detail-content.tsx b/apps/mobile/src/components/agents/session-detail-content.tsx index 1c832745c5..5b4f3e8849 100644 --- a/apps/mobile/src/components/agents/session-detail-content.tsx +++ b/apps/mobile/src/components/agents/session-detail-content.tsx @@ -5,7 +5,7 @@ import { type StoredMessage, } from '@kilocode/cloud-agent-sdk'; import { type Href, useFocusEffect, useIsFocused, useRouter } from 'expo-router'; -import { useAtomValue, useSetAtom } from 'jotai'; +import { useAtomValue, useSetAtom, useStore } from 'jotai'; import { MessageSquare } from '@/components/ui/icons'; import { useCallback, useEffect, useMemo, useRef, useState } from 'react'; import { useKeepAwake } from 'expo-keep-awake'; @@ -63,6 +63,7 @@ import { retryMessageAndClear, } from '@/components/agents/session-detail-content-helpers'; import { shouldKeepSessionAwake } from '@/components/agents/session-keep-awake'; +import { shouldRefetchOnFocus } from '@/components/agents/session-focus-refetch'; import { TranscriptTimeMarker } from '@/components/agents/transcript-time-marker'; import { EmptyState } from '@/components/empty-state'; import { AppAwareKeyboardPaddingView } from '@/components/kilo-chat/app-aware-keyboard-padding'; @@ -459,10 +460,16 @@ export function SessionDetailContent({ // Refetch the linked PR on every focus so a link, unlink, or mid-session // decision change surfaces without reopening the session. A pending review // decision gets one 4s follow-up refetch — no polling loop. + const store = useStore(); + // The first focus on a session id is owned by `manager.switchSession`, which + // already fetches the session metadata (including `associatedPr`). Every + // later focus refetches; this ref tracks which id has been seeded. + const seededSessionIdRef = useRef(null); useFocusEffect( useCallback(() => { let cancelled = false; let pendingTimeout: ReturnType | null = null; + let unsubscribe: (() => void) | null = null; const refetch = async (scheduleFollowUp: boolean) => { try { @@ -485,15 +492,48 @@ export function SessionDetailContent({ } }; - void refetch(true); + // Check the current `fetchedSessionData` and, when a review decision is + // pending, schedule the one-shot 4s follow-up. Runs once against the + // current value and again on every later write until the session's data + // has landed (or the effect is cancelled). + const checkAndSchedule = () => { + if (cancelled) { + return; + } + const fetched = store.get(manager.atoms.fetchedSessionData); + if (fetched?.kiloSessionId !== sessionId) { + return; + } + unsubscribe?.(); + unsubscribe = null; + if (fetched.associatedPr?.reviewDecisionPending) { + pendingTimeout = setTimeout(() => { + pendingTimeout = null; + void refetch(false); + }, 4000); + } + }; + + if (!shouldRefetchOnFocus(seededSessionIdRef.current, sessionId)) { + // First focus: `switchSession` owns the metadata read, so issue no + // request. Seed the ref and keep the pending-decision follow-up. The + // manager's fetch can land before this effect runs (switchSession + // writes first), so check the current value once before subscribing. + seededSessionIdRef.current = sessionId; + checkAndSchedule(); + unsubscribe = store.sub(manager.atoms.fetchedSessionData, checkAndSchedule); + } else { + void refetch(true); + } return () => { cancelled = true; + unsubscribe?.(); if (pendingTimeout !== null) { clearTimeout(pendingTimeout); } }; - }, [manager, sessionId]) + }, [manager, sessionId, store]) ); useEffect(() => { @@ -1051,6 +1091,21 @@ export function SessionDetailContent({ pendingMessageCount: inFlightMessageCount, }); + // Child-sheet pagination fields live only on the `ready` hydration state. + // The sheet can render `content` before hydration reports ready (live child + // rows already exist), so default every non-ready state. + const openChildSessionId = childSessionSheet.sheet?.sessionId ?? null; + const openChildHydrationState = + openChildSessionId === null ? null : getChildSessionHydrationState(openChildSessionId); + const childHasOlderMessages = + openChildHydrationState?.status === 'ready' ? openChildHydrationState.hasOlder : false; + const childIsLoadingOlderMessages = + openChildHydrationState?.status === 'ready' ? openChildHydrationState.isLoadingOlder : false; + const childOlderMessagesError = + openChildHydrationState?.status === 'ready' ? openChildHydrationState.olderError : null; + const childOlderMessagesOmittedItemCount = + openChildHydrationState?.status === 'ready' ? openChildHydrationState.omittedItemCount : 0; + return ( @@ -1121,6 +1176,15 @@ export function SessionDetailContent({ getChildMessages={getChildMessages} hydrationState={getChildSessionHydrationState(childSessionSheet.sheet.sessionId)} isStreaming={getChildSessionStreaming(messages, childSessionSheet.sheet.sessionId)} + hasOlderMessages={childHasOlderMessages} + isLoadingOlderMessages={childIsLoadingOlderMessages} + olderMessagesError={childOlderMessagesError} + olderMessagesOmittedItemCount={childOlderMessagesOmittedItemCount} + onLoadOlderMessages={() => { + if (openChildSessionId !== null) { + void manager.loadOlderChildMessages(openChildSessionId); + } + }} renderPart={props => } onOpenChildSession={handleOpenChildSession} onRetry={() => { diff --git a/apps/mobile/src/components/agents/session-focus-refetch.test.ts b/apps/mobile/src/components/agents/session-focus-refetch.test.ts new file mode 100644 index 0000000000..fb860f5430 --- /dev/null +++ b/apps/mobile/src/components/agents/session-focus-refetch.test.ts @@ -0,0 +1,17 @@ +import { describe, expect, it } from 'vitest'; + +import { shouldRefetchOnFocus } from '@/components/agents/session-focus-refetch'; + +describe('shouldRefetchOnFocus', () => { + it('returns false on the first focus of a session id', () => { + expect(shouldRefetchOnFocus(null, 'session-a')).toBe(false); + }); + + it('returns true on a second focus of the same session id', () => { + expect(shouldRefetchOnFocus('session-a', 'session-a')).toBe(true); + }); + + it('returns false again on a focus of a different session id', () => { + expect(shouldRefetchOnFocus('session-a', 'session-b')).toBe(false); + }); +}); diff --git a/apps/mobile/src/components/agents/session-focus-refetch.ts b/apps/mobile/src/components/agents/session-focus-refetch.ts new file mode 100644 index 0000000000..e90a757d17 --- /dev/null +++ b/apps/mobile/src/components/agents/session-focus-refetch.ts @@ -0,0 +1,12 @@ +/** + * Decides whether a session focus must refetch its linked-PR metadata. + * + * The first focus on a session id is owned by `manager.switchSession`, which + * already fetches the session (including `associatedPr`) and writes it into + * `fetchedSessionDataAtom`. Every later focus on the same id must refetch so a + * link, unlink, or mid-session decision change surfaces without reopening the + * session. + */ +export function shouldRefetchOnFocus(seededSessionId: string | null, sessionId: string): boolean { + return seededSessionId === sessionId; +} From 9f6cd74d882dc970929159cd725e1a2ae9f4dc2f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20=C5=A0=C4=87eki=C4=87?= Date: Wed, 19 Aug 2026 05:37:05 +0200 Subject: [PATCH 14/34] fix(mobile): type discussion-threads retention test comments retainConversationAcrossMounts takes ConversationComment[], so the test's bare { id: 'c1' } comment no longer typechecks. Cast it to ConversationComment. --- .../discussion/use-pr-review-discussion-threads.test.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/apps/mobile/src/lib/pr-review/discussion/use-pr-review-discussion-threads.test.ts b/apps/mobile/src/lib/pr-review/discussion/use-pr-review-discussion-threads.test.ts index f576b992d4..438d8a7463 100644 --- a/apps/mobile/src/lib/pr-review/discussion/use-pr-review-discussion-threads.test.ts +++ b/apps/mobile/src/lib/pr-review/discussion/use-pr-review-discussion-threads.test.ts @@ -2,6 +2,7 @@ import { describe, expect, it, vi } from 'vitest'; import { INFINITE_QUERY_MAX_PAGES } from '@/lib/query/infinite-retention'; +import { type ConversationComment } from './review-discussion-types'; import { buildPrReviewDiscussionThreadsQueryOptions, retainConversation, @@ -63,7 +64,7 @@ describe('retainConversation (retention-safe conversation)', () => { }); describe('retainConversationAcrossMounts (remount survival)', () => { - const comment = { id: 'c1' }; + const comment = { id: 'c1' } as unknown as ConversationComment; it('keeps the conversation after a remount over the trimmed cache', () => { const key = 'octocat/hello#1'; From ae4dce93556b09b769a8174ec6f0812002f24700 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20=C5=A0=C4=87eki=C4=87?= Date: Wed, 19 Aug 2026 05:51:19 +0200 Subject: [PATCH 15/34] fix(mobile): keep the first-focus pending-decision follow-up one-shot checkAndSchedule now reports whether the session data has landed, and the skip path subscribes only when it has not. A later write no longer starts a second 4-second timer, so the follow-up stays a single refetch. --- .../components/agents/session-detail-content.tsx | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/apps/mobile/src/components/agents/session-detail-content.tsx b/apps/mobile/src/components/agents/session-detail-content.tsx index 5b4f3e8849..488847ae1b 100644 --- a/apps/mobile/src/components/agents/session-detail-content.tsx +++ b/apps/mobile/src/components/agents/session-detail-content.tsx @@ -496,13 +496,13 @@ export function SessionDetailContent({ // pending, schedule the one-shot 4s follow-up. Runs once against the // current value and again on every later write until the session's data // has landed (or the effect is cancelled). - const checkAndSchedule = () => { + const checkAndSchedule = (): boolean => { if (cancelled) { - return; + return true; } const fetched = store.get(manager.atoms.fetchedSessionData); if (fetched?.kiloSessionId !== sessionId) { - return; + return false; } unsubscribe?.(); unsubscribe = null; @@ -512,6 +512,7 @@ export function SessionDetailContent({ void refetch(false); }, 4000); } + return true; }; if (!shouldRefetchOnFocus(seededSessionIdRef.current, sessionId)) { @@ -519,9 +520,14 @@ export function SessionDetailContent({ // request. Seed the ref and keep the pending-decision follow-up. The // manager's fetch can land before this effect runs (switchSession // writes first), so check the current value once before subscribing. + // Subscribe only when the data has not landed yet; a match schedules + // at most one follow-up and stops listening. seededSessionIdRef.current = sessionId; - checkAndSchedule(); - unsubscribe = store.sub(manager.atoms.fetchedSessionData, checkAndSchedule); + if (!checkAndSchedule()) { + unsubscribe = store.sub(manager.atoms.fetchedSessionData, () => { + checkAndSchedule(); + }); + } } else { void refetch(true); } From 6b9ca75e913d5aaede1f5d294916829bc7be319a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20=C5=A0=C4=87eki=C4=87?= Date: Wed, 19 Aug 2026 07:06:17 +0200 Subject: [PATCH 16/34] feat(mobile): trim retained transcript history on reaching bottom Wire the SessionMessageList onReachedBottom transition to SessionManager.trimRetainedHistory so older loaded pages are dropped only when the view returns to the bottom. W4.3 confirmed the markdown renderers already memoize the parse on value; no renderer change was needed. --- .../agents/session-detail-content.tsx | 3 +++ .../agents/session-message-list.tsx | 24 +++++++++++++++++++ 2 files changed, 27 insertions(+) diff --git a/apps/mobile/src/components/agents/session-detail-content.tsx b/apps/mobile/src/components/agents/session-detail-content.tsx index 488847ae1b..ad65d5968d 100644 --- a/apps/mobile/src/components/agents/session-detail-content.tsx +++ b/apps/mobile/src/components/agents/session-detail-content.tsx @@ -1422,6 +1422,9 @@ export function SessionDetailContent({ onLoadOlderMessages={() => { void manager.loadOlderMessages(); }} + onReachedBottom={() => { + manager.trimRetainedHistory(); + }} renderItem={renderItem} /> ); diff --git a/apps/mobile/src/components/agents/session-message-list.tsx b/apps/mobile/src/components/agents/session-message-list.tsx index eb0b49f9a0..4bbf327349 100644 --- a/apps/mobile/src/components/agents/session-message-list.tsx +++ b/apps/mobile/src/components/agents/session-message-list.tsx @@ -43,6 +43,13 @@ type SessionMessageListProps = { * indicators. */ contentBottomInset?: number; + /** + * Optional callback fired when the list returns to the bottom after the + * user scrolled away. Fires only on the false→true transition of + * `isAtBottom`, never on mount. The host uses this to trim retained + * history exactly when the user returns to the live tail. + */ + onReachedBottom?: () => void; }; export function SessionMessageList({ @@ -57,6 +64,7 @@ export function SessionMessageList({ renderItem, ListFooterComponent, contentBottomInset, + onReachedBottom, }: Readonly>) { // FlashList v2 renders the list in chronological order (oldest → newest). // `startRenderingFromBottom` keeps the viewport anchored at the newest @@ -113,6 +121,22 @@ export function SessionMessageList({ inFlightRef.current = false; }, [sessionId]); + // Fire `onReachedBottom` only on the false→true transition of + // `isAtBottom`. The previous-value ref prevents a fire on mount (the list + // starts at the bottom) and on repeat renders while already at the bottom. + // The handler is held in a ref so a new inline callback identity from the + // host never re-runs this effect. + const onReachedBottomRef = useRef(onReachedBottom); + onReachedBottomRef.current = onReachedBottom; + const prevIsAtBottomRef = useRef(isAtBottom); + useEffect(() => { + const prev = prevIsAtBottomRef.current; + prevIsAtBottomRef.current = isAtBottom; + if (isAtBottom && !prev) { + onReachedBottomRef.current?.(); + } + }, [isAtBottom]); + // Non-visual a11y signal for older-page arrival (visual loading skeleton // was removed). Announce only when items were actually prepended. const olderArrivalInitializedRef = useRef(false); From 062d971b8e60f2b28efd3602ca927c72d3ed1c66 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20=C5=A0=C4=87eki=C4=87?= Date: Wed, 19 Aug 2026 07:06:21 +0200 Subject: [PATCH 17/34] perf(mobile): coalesce composer typing and upload progress per frame Add createFrameCoalescer and route composer derived setters and per-attachment upload progress through it, publishing at most once per animation frame. Flush at submit, share-prefill, and terminal-value decision points; cancel fully disables a coalescer so a cancelled upload publishes no late progress. --- .../src/components/agents/chat-composer.tsx | 58 +++++++++- .../use-agent-attachment-upload.ts | 13 ++- apps/mobile/src/lib/coalesce-frame.test.ts | 103 ++++++++++++++++++ apps/mobile/src/lib/coalesce-frame.ts | 75 +++++++++++++ 4 files changed, 244 insertions(+), 5 deletions(-) create mode 100644 apps/mobile/src/lib/coalesce-frame.test.ts create mode 100644 apps/mobile/src/lib/coalesce-frame.ts diff --git a/apps/mobile/src/components/agents/chat-composer.tsx b/apps/mobile/src/components/agents/chat-composer.tsx index 6c5e75189d..caa1595951 100644 --- a/apps/mobile/src/components/agents/chat-composer.tsx +++ b/apps/mobile/src/components/agents/chat-composer.tsx @@ -82,6 +82,7 @@ import { type ModeOption } from '@/components/agents/mode-normalize'; import { useCurrentUserId } from '@/lib/hooks/use-current-user-id'; import { useThemeColors } from '@/lib/hooks/use-theme-colors'; import { resolveMessageInputAppStateTransition } from '@/lib/message-input-app-state'; +import { createFrameCoalescer, type FrameCoalescer } from '@/lib/coalesce-frame'; import { clearDraft as clearStoredDraft, saveDraft } from '@/lib/persist/drafts'; import { useDraftFlushOnBackground } from '@/lib/persist/use-draft-flush'; import { cn } from '@/lib/utils'; @@ -296,6 +297,31 @@ export function ChatComposer({ const measureRef = useRef(measure); measureRef.current = measure; + // Coalesce the three derived setters (measure node, hasText, slash command) + // to at most one publication per animation frame. Typing can fire many + // onChangeText calls in a single frame; publishing derived state once per + // frame keeps the send button and slash suggestions from re-rendering on + // every keystroke. The publish closure reads `measureRef` at call time and + // uses the stable `setHasText`/`setSlashCommandInput` setters, so it stays + // valid for the lifetime of the component. + const composerFrameCoalescerRef = useRef | null>(null); + composerFrameCoalescerRef.current ??= createFrameCoalescer(value => { + measureRef.current.setText(value); + setHasText(value.trim().length > 0); + setSlashCommandInput(getSlashCommandCandidate(value)); + }); + const composerFrameCoalescer = composerFrameCoalescerRef.current; + + // Flush the coalescer on unmount so a pending derived-state publication is + // committed before teardown and the scheduled frame callback becomes a + // no-op instead of firing setState after the component is gone. + useEffect( + () => () => { + composerFrameCoalescer.flush(); + }, + [composerFrameCoalescer] + ); + // Flush the debounced draft write when the app leaves `active` and on // unmount, so a backgrounded-then-killed app (or a navigation away) does // not lose the last keystrokes inside the 500 ms window. @@ -376,6 +402,10 @@ export function ChatComposer({ // hasText, slash-command state, and the measure node, then persists the // durable draft exactly like a keystroke. function applyComposerText(value: string) { + // Drain any pending coalesced typing so a stale value cannot overwrite the + // copied prompt on the next frame. The direct setters below then land the + // copied prompt in one commit. + composerFrameCoalescer.flush(); textRef.current = value; measure.setText(value); setHasText(value.trim().length > 0); @@ -408,9 +438,10 @@ export function ChatComposer({ function handleChangeText(value: string) { textRef.current = value; - measure.setText(value); - setHasText(value.trim().length > 0); - setSlashCommandInput(getSlashCommandCandidate(value)); + // Derived state (measure node, hasText, slash command) is coalesced to one + // publication per frame; the live submit-time ref and the debounced draft + // write stay synchronous so neither can lag a keystroke. + composerFrameCoalescer.push(value); // Delivery applies text BEFORE onDelivered fires, so any // handleChangeText after shareDelivered is a user edit. Disarm // so a later gate resolution (upload completion) cannot @@ -435,6 +466,10 @@ export function ChatComposer({ onChangeText: handleChangeText, addCandidates, onDelivered: () => { + // Commit the coalesced `hasText` before the delivery check so the + // auto-send effect sees the delivered text in the same commit, not on + // the next frame. + composerFrameCoalescer.flush(); setAutoSendArmed( shouldArmAutoSendOnDelivery({ autoSend: autoSendRef.current, @@ -648,6 +683,11 @@ export function ChatComposer({ : undefined; function clearDraft() { + // Drain any pending coalesced value (a final voice transcript can `push` + // after `submit`'s `flush`). Publishing it here clears `hasPending` so the + // already-scheduled frame callback becomes a no-op; the direct setters + // below then override the published value in the same batched commit. + composerFrameCoalescer.flush(); textRef.current = ''; setHasText(false); setSlashCommandInput(null); @@ -662,7 +702,13 @@ export function ChatComposer({ async function handleSend() { const trimmed = textRef.current.trim(); - if (!control.canSend) { + // Decide admission from live values, not render-time `control.canSend`, + // which can lag behind a same-frame edit. An empty prompt with no ready + // attachment is never sent. + const readyAttachmentsCount = upload.attachments.filter( + attachment => attachment.status === 'uploaded' + ).length; + if ((trimmed.length === 0 && readyAttachmentsCount === 0) || disabled || isSending) { return; } if (upload.isUploading) { @@ -746,6 +792,10 @@ export function ChatComposer({ } async function submit() { + // Commit any coalesced derived state (hasText, measure, slash command) + // before the send decision, so a submit in the same frame as the last + // keystroke never reads stale derived state. + composerFrameCoalescer.flush(); // `settleVoiceInputBeforeSubmit` is the sole admission owner for the // entire voice-settle + asynchronous send sequence. It acquires the // SubmitLock, sets pending state, waits for the final transcript, runs diff --git a/apps/mobile/src/lib/agent-attachments/use-agent-attachment-upload.ts b/apps/mobile/src/lib/agent-attachments/use-agent-attachment-upload.ts index 3385e752c7..9d3e8a056b 100644 --- a/apps/mobile/src/lib/agent-attachments/use-agent-attachment-upload.ts +++ b/apps/mobile/src/lib/agent-attachments/use-agent-attachment-upload.ts @@ -5,6 +5,7 @@ import { toast } from 'sonner-native'; import { announceForA11y } from '@/lib/a11y/announce'; import { announcingToast } from '@/lib/a11y/announcing-toast'; +import { createFrameCoalescer } from '@/lib/coalesce-frame'; import { AGENT_ATTACHMENT_MAX_FILES } from '@/lib/agent-attachments/constants'; import { canAddAttachments, @@ -155,6 +156,9 @@ export function useAgentAttachmentUpload( terminal: undefined, progress: 0, }); + const progressCoalescer = createFrameCoalescer(progress => { + updateAttachment(attachment.id, { progress }); + }); try { const { key } = await uploadOne({ organizationId, @@ -165,10 +169,11 @@ export function useAgentAttachmentUpload( contentLength: attachment.size, localUri: attachment.localUri, onProgress: progress => { - updateAttachment(attachment.id, { progress }); + progressCoalescer.push(progress); }, onTask: task => { cancelHandlesRef.current.set(attachment.id, async () => { + progressCoalescer.cancel(); await task.cancelAsync(); deleteCacheOwnedFile(attachment.localUri); }); @@ -181,6 +186,9 @@ export function useAgentAttachmentUpload( if (!liveIdsRef.current.has(attachment.id)) { return; } + // Drain any pending progress before the terminal flip so the chip + // lands on `progress: 1` and never sticks at a stale percentage. + progressCoalescer.flush(); updateAttachment(attachment.id, { status: 'uploaded', remoteFilename: key.split('/').at(-1), @@ -198,6 +206,9 @@ export function useAgentAttachmentUpload( if (!liveIdsRef.current.has(attachment.id)) { return; } + // Drain any pending progress before the terminal flip so the chip + // lands on the error state and never sticks at a stale percentage. + progressCoalescer.flush(); const { retryable, reason } = classifyUploadFailure(error); updateAttachment(attachment.id, { status: 'error', diff --git a/apps/mobile/src/lib/coalesce-frame.test.ts b/apps/mobile/src/lib/coalesce-frame.test.ts new file mode 100644 index 0000000000..5801431014 --- /dev/null +++ b/apps/mobile/src/lib/coalesce-frame.test.ts @@ -0,0 +1,103 @@ +import { describe, expect, it } from 'vitest'; + +import { createFrameCoalescer } from './coalesce-frame'; + +/** A scheduler that queues callbacks and runs them on demand, like a frame. */ +function makeScheduler() { + const queued: (() => void)[] = []; + return { + schedule: (frame: () => void) => { + queued.push(frame); + }, + run: () => { + const pending = queued.splice(0); + for (const frame of pending) { + frame(); + } + }, + pendingCount: () => queued.length, + }; +} + +describe('createFrameCoalescer', () => { + it('publishes the latest value once per frame', () => { + const published: string[] = []; + const scheduler = makeScheduler(); + const coalescer = createFrameCoalescer(value => { + published.push(value); + }, scheduler.schedule); + + coalescer.push('a'); + coalescer.push('b'); + coalescer.push('c'); + + expect(published).toEqual([]); + expect(scheduler.pendingCount()).toBe(1); + + scheduler.run(); + + expect(published).toEqual(['c']); + }); + + it('publishes immediately on flush and does not re-publish on the frame', () => { + const published: string[] = []; + const scheduler = makeScheduler(); + const coalescer = createFrameCoalescer(value => { + published.push(value); + }, scheduler.schedule); + + coalescer.push('a'); + coalescer.flush(); + + expect(published).toEqual(['a']); + + scheduler.run(); + + expect(published).toEqual(['a']); + }); + + it('drops the pending value on cancel so no publication happens', () => { + const published: string[] = []; + const scheduler = makeScheduler(); + const coalescer = createFrameCoalescer(value => { + published.push(value); + }, scheduler.schedule); + + coalescer.push('a'); + coalescer.cancel(); + scheduler.run(); + + expect(published).toEqual([]); + }); + + it('publishes nothing from a later push or flush after cancel', () => { + const published: string[] = []; + const scheduler = makeScheduler(); + const coalescer = createFrameCoalescer(value => { + published.push(value); + }, scheduler.schedule); + + coalescer.cancel(); + coalescer.push('a'); + coalescer.flush(); + scheduler.run(); + + expect(published).toEqual([]); + expect(scheduler.pendingCount()).toBe(0); + }); + + it('schedules again after a flush so later pushes still coalesce', () => { + const published: string[] = []; + const scheduler = makeScheduler(); + const coalescer = createFrameCoalescer(value => { + published.push(value); + }, scheduler.schedule); + + coalescer.push('a'); + coalescer.flush(); + coalescer.push('b'); + scheduler.run(); + + expect(published).toEqual(['a', 'b']); + }); +}); diff --git a/apps/mobile/src/lib/coalesce-frame.ts b/apps/mobile/src/lib/coalesce-frame.ts new file mode 100644 index 0000000000..3d6812731c --- /dev/null +++ b/apps/mobile/src/lib/coalesce-frame.ts @@ -0,0 +1,75 @@ +/** + * Coalesce many value updates into at most one publication per animation + * frame. `push` stores the latest value and schedules exactly one flush per + * frame; `flush` publishes a pending value immediately; `cancel` fully + * disables the coalescer so no later `push` or `flush` publishes anything. + * + * The scheduler is injectable so tests can drive frames synchronously. + */ +export type FrameCoalescer = { + push: (value: T) => void; + flush: () => void; + cancel: () => void; +}; + +/** + * Default scheduler: `requestAnimationFrame` where available (React Native), + * falling back to a zero-delay timeout in environments without it (node + * tests). The fallback preserves "publish eventually" so a coalescer never + * silently drops a pushed value. + */ +const rafSchedule = (frame: () => void): void => { + if (typeof requestAnimationFrame === 'function') { + requestAnimationFrame(frame); + return; + } + setTimeout(frame, 0); +}; + +export function createFrameCoalescer( + publish: (value: T) => void, + schedule: (cb: () => void) => void = rafSchedule +): FrameCoalescer { + let pending: T | undefined = undefined; + let hasPending = false; + // True while a flush is queued on the scheduler. Guards against scheduling + // more than one flush per frame. + let scheduled = false; + // True after `cancel`. Once cancelled, the coalescer is fully disabled: + // `push` stores nothing and `flush` publishes nothing. + let cancelled = false; + + function flush(): void { + if (cancelled || !hasPending) { + return; + } + hasPending = false; + const value = pending as T; + pending = undefined; + publish(value); + } + + function push(value: T): void { + if (cancelled) { + return; + } + pending = value; + hasPending = true; + if (scheduled) { + return; + } + scheduled = true; + schedule(() => { + scheduled = false; + flush(); + }); + } + + function cancel(): void { + cancelled = true; + hasPending = false; + pending = undefined; + } + + return { push, flush, cancel }; +} From b4038a2aabb71b12ff3ba4f45e51399a3828eec5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20=C5=A0=C4=87eki=C4=87?= Date: Wed, 19 Aug 2026 08:40:48 +0200 Subject: [PATCH 18/34] feat(mobile): persist merge, reply, and comment drafts by destination Add prMerge/prReply/prComment draft keys and an isMergeDraft validator, make useFencedDraftLoad generic with a defaulted validator, and wire the merge sheet, PR reply, and PR comment composer to load, save, and clear durable drafts per account and destination. Clear only on completion or confirmed discard. --- apps/mobile/src/app/(app)/agent-chat/new.tsx | 2 +- .../agents/session-detail-content.tsx | 2 +- .../agents/use-new-session-creator.test.ts | 8 +- .../pr-review/discussion/reply-input.test.ts | 174 ++++++++++- .../pr-review/discussion/reply-input.tsx | 34 ++- .../pr-review/merge/pr-merge-sheet.test.tsx | 77 +++++ .../pr-review/merge/pr-merge-sheet.tsx | 119 ++++++-- .../pr-review-comment-composer.test.tsx | 277 ++++++++++++++++++ .../pr-review/pr-review-comment-composer.tsx | 67 ++++- apps/mobile/src/lib/persist/drafts.test.ts | 100 +++++++ apps/mobile/src/lib/persist/drafts.ts | 39 +++ .../src/lib/persist/use-draft-load.test.ts | 240 +++++++++++++++ apps/mobile/src/lib/persist/use-draft-load.ts | 54 ++-- 13 files changed, 1137 insertions(+), 56 deletions(-) create mode 100644 apps/mobile/src/components/pr-review/pr-review-comment-composer.test.tsx create mode 100644 apps/mobile/src/lib/persist/use-draft-load.test.ts diff --git a/apps/mobile/src/app/(app)/agent-chat/new.tsx b/apps/mobile/src/app/(app)/agent-chat/new.tsx index 5d967f5d7a..72b85ea938 100644 --- a/apps/mobile/src/app/(app)/agent-chat/new.tsx +++ b/apps/mobile/src/app/(app)/agent-chat/new.tsx @@ -106,7 +106,7 @@ function NewSessionScreenBody() { // settles. const initialPrompt = resolvePrefillOverDraft( sharePrefillText, - draftState.settled ? draftState.text : null + draftState.settled ? draftState.value : null ); // Save the new-session draft debounced on every text change, and flush the diff --git a/apps/mobile/src/components/agents/session-detail-content.tsx b/apps/mobile/src/components/agents/session-detail-content.tsx index ad65d5968d..bf4b2f49c6 100644 --- a/apps/mobile/src/components/agents/session-detail-content.tsx +++ b/apps/mobile/src/components/agents/session-detail-content.tsx @@ -1337,7 +1337,7 @@ export function SessionDetailContent({ shareId={shareId} autoSend={autoSend} draftKey={userId ? sessionComposerDraftKey : undefined} - initialDraft={composerDraft.settled ? (composerDraft.text ?? '') : undefined} + initialDraft={composerDraft.settled ? (composerDraft.value ?? '') : undefined} controlRef={composerControlRef} /> diff --git a/apps/mobile/src/components/agents/use-new-session-creator.test.ts b/apps/mobile/src/components/agents/use-new-session-creator.test.ts index 084658bbff..821b22f445 100644 --- a/apps/mobile/src/components/agents/use-new-session-creator.test.ts +++ b/apps/mobile/src/components/agents/use-new-session-creator.test.ts @@ -797,7 +797,7 @@ describe('restored new-session submit', () => { }); }); -type DraftLoadState = { settled: boolean; text: string | null }; +type DraftLoadState = { settled: boolean; value: string | null }; function FencedDraftHarness({ userId, @@ -878,14 +878,14 @@ describe('useFencedDraftLoad generation fencing', () => { firstLoad.resolve(staleText); }); await flushMicrotasks(); - expect(renders.at(-1)).toEqual({ settled: false, text: null }); + expect(renders.at(-1)).toEqual({ settled: false, value: null }); // The current generation's load resolves: it publishes. await act(async () => { secondLoad.resolve(freshText); }); await flushMicrotasks(); - expect(renders.at(-1)).toEqual({ settled: true, text: freshText }); + expect(renders.at(-1)).toEqual({ settled: true, value: freshText }); expect(vi.mocked(loadDraft)).toHaveBeenCalledWith( second.userId, second.entityKey, @@ -920,7 +920,7 @@ describe('useFencedDraftLoad generation fencing', () => { gate.resolve('late draft'); }); await flushMicrotasks(); - expect(renders.at(-1)).toEqual({ settled: false, text: null }); + expect(renders.at(-1)).toEqual({ settled: false, value: null }); }); }); diff --git a/apps/mobile/src/components/pr-review/discussion/reply-input.test.ts b/apps/mobile/src/components/pr-review/discussion/reply-input.test.ts index 8fddcb6ec8..782d30c4bc 100644 --- a/apps/mobile/src/components/pr-review/discussion/reply-input.test.ts +++ b/apps/mobile/src/components/pr-review/discussion/reply-input.test.ts @@ -9,9 +9,12 @@ // Cancel buttons the gate renders. The gate is a pure async function, so no // React mounting is required. +import * as React from 'react'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; -import { ensureTermsAcceptedOutcome } from './reply-input'; +import { ensureTermsAcceptedOutcome, ReplyInput } from './reply-input'; +import { clearDraft } from '@/lib/persist/drafts'; +import { type useReplyToCommentMutation } from '@/lib/pr-review/discussion/use-review-discussion-mutations'; type AlertButton = { text?: string; onPress?: () => void }; type AlertCall = { title: string; message: string; buttons: AlertButton[] }; @@ -62,6 +65,51 @@ vi.mock('@/lib/hooks/use-theme-colors', () => ({ useThemeColors: () => ({ mutedForeground: '#000000' }), })); +// `reply-input` imports the durable-draft chain, which pulls in the native +// encrypted-kv → expo-secure-store → expo-modules-core chain that the node +// test environment cannot resolve. Mock the persist chain and the identity +// hook so this suite stays node-only. +vi.mock('@/lib/persist/drafts', () => ({ + saveDraft: vi.fn(), + clearDraft: vi.fn(), + prReplyDraftKey: vi.fn(() => 'pr-reply:key'), + prMergeDraftKey: vi.fn(), + prCommentDraftKey: vi.fn(), +})); + +vi.mock('@/lib/persist/use-draft-load', () => ({ + useFencedDraftLoad: () => ({ settled: true, value: null }), +})); + +vi.mock('@/lib/persist/use-draft-flush', () => ({ + useDraftFlushOnBackground: vi.fn(), +})); + +vi.mock('@/lib/hooks/use-current-user-id', () => ({ + useCurrentUserId: () => ({ userId: 'u1', isLoading: false }), +})); + +// `ReplyInput` is mounted by calling it as a plain function (no renderer), so +// the React hook primitives are stubbed to no-op/simple versions, mirroring +// pr-merge-sheet.test.tsx. The pure `ensureTermsAcceptedOutcome` tests above +// do not touch these. +vi.mock('react', async () => { + const actual = await vi.importActual('react'); + return { + ...actual, + useState: vi.fn((initial: T) => [initial, vi.fn() as () => void] as [T, (value: T) => void]), + useMemo: vi.fn((factory: () => T) => factory()), + useRef: vi.fn((initial: T) => { + const ref: React.RefObject = { current: initial }; + return ref; + }), + useEffect: vi.fn((effect: React.EffectCallback) => { + effect(); + }), + useCallback: vi.fn( unknown>(fn: T) => fn), + }; +}); + /** Drains microtasks so the awaited getTermsStatus/acceptTerms settle. */ async function flush(): Promise { await Promise.resolve(); @@ -187,3 +235,127 @@ describe('ensureTermsAcceptedOutcome', () => { expect(alertCalls).toHaveLength(0); }); }); + +type ReplyMutation = ReturnType; + +function makeReply(mutate: unknown): ReplyMutation { + return { mutate, isPending: false, error: null } as unknown as ReplyMutation; +} + +type FindElementArgs = { + node: unknown; + type: string; + prop: string; + value: unknown; +}; + +function findElement({ node, type, prop, value }: FindElementArgs): React.ReactElement | null { + if (React.isValidElement(node)) { + const element = node; + const props = element.props as Record; + if (element.type === type && props[prop] === value) { + return element; + } + const children = props.children; + if (Array.isArray(children)) { + for (const child of children) { + const found = findElement({ node: child, type, prop, value }); + if (found) { + return found; + } + } + } else if (children !== undefined && children !== null) { + const found = findElement({ node: children, type, prop, value }); + if (found) { + return found; + } + } + } + if (Array.isArray(node)) { + for (const child of node) { + const found = findElement({ node: child, type, prop, value }); + if (found) { + return found; + } + } + } + return null; +} + +/** Mounts ReplyInput, types a body, and presses the submit button. */ +function mountAndSubmit(reply: ReplyMutation): void { + // eslint-disable-next-line new-cap + const element = ReplyInput({ + owner: 'octocat', + repo: 'hello', + number: 1, + commentId: 42, + reply, + }); + const input = findElement({ + node: element, + type: 'TextInput', + prop: 'accessibilityLabel', + value: 'Reply body', + }); + if (!input) { + throw new Error('Reply body TextInput not found'); + } + (input.props as { onChangeText?: (value: string) => void }).onChangeText?.('hello'); + const button = findElement({ + node: element, + type: 'Button', + prop: 'accessibilityLabel', + value: 'Submit reply', + }); + if (!button) { + throw new Error('Submit reply Button not found'); + } + (button.props as { onPress?: () => void }).onPress?.(); +} + +describe('ReplyInput draft clear on submit', () => { + beforeEach(() => { + alertCalls.length = 0; + getTermsStatusMock.mockReset(); + acceptTermsMock.mockReset(); + }); + + afterEach(() => { + vi.clearAllMocks(); + }); + + it('clears the reply draft on a successful reply', async () => { + getTermsStatusMock.mockResolvedValue({ accepted: true, currentVersion: 'v1' }); + const mutate = vi.fn((_input: unknown, options: { onSuccess?: () => void }) => { + options.onSuccess?.(); + }); + mountAndSubmit(makeReply(mutate)); + await flush(); + + expect(clearDraft).toHaveBeenCalledWith('u1', 'pr-reply:key'); + }); + + it('does not clear the reply draft on a failed reply', async () => { + getTermsStatusMock.mockResolvedValue({ accepted: true, currentVersion: 'v1' }); + // A failed mutation never invokes onSuccess, so the draft must survive. + const mutate = vi.fn(); + mountAndSubmit(makeReply(mutate)); + await flush(); + + expect(mutate).toHaveBeenCalledTimes(1); + expect(clearDraft).not.toHaveBeenCalled(); + }); + + it('does not clear the reply draft when the terms gate is dismissed', async () => { + getTermsStatusMock.mockResolvedValue({ accepted: false, currentVersion: 'v1' }); + const mutate = vi.fn(); + mountAndSubmit(makeReply(mutate)); + await flush(); + pressButton('Cancel'); + await flush(); + + expect(mutate).not.toHaveBeenCalled(); + expect(clearDraft).not.toHaveBeenCalled(); + }); +}); diff --git a/apps/mobile/src/components/pr-review/discussion/reply-input.tsx b/apps/mobile/src/components/pr-review/discussion/reply-input.tsx index 2b50dcd602..f67111707b 100644 --- a/apps/mobile/src/components/pr-review/discussion/reply-input.tsx +++ b/apps/mobile/src/components/pr-review/discussion/reply-input.tsx @@ -12,7 +12,11 @@ import { PrReviewReconnectNotice } from '@/components/pr-review/pr-review-reconn import { Button } from '@/components/ui/button'; import { Text } from '@/components/ui/text'; import { WEB_BASE_URL } from '@/lib/config'; +import { useCurrentUserId } from '@/lib/hooks/use-current-user-id'; import { useThemeColors } from '@/lib/hooks/use-theme-colors'; +import { clearDraft, prReplyDraftKey, saveDraft } from '@/lib/persist/drafts'; +import { useDraftFlushOnBackground } from '@/lib/persist/use-draft-flush'; +import { useFencedDraftLoad } from '@/lib/persist/use-draft-load'; import { classifyPrReviewMutationError } from '@/lib/pr-review/classify-pr-review-query-state'; import { type useReplyToCommentMutation } from '@/lib/pr-review/discussion/use-review-discussion-mutations'; import { @@ -157,6 +161,28 @@ export function ReplyInput({ owner, repo, number, commentId, reply }: Readonly(null); const [resetKey, setResetKey] = useState(0); + // Durable reply draft, keyed by account and thread. Nothing is saved or + // restored while the user id is unknown. + const { userId, isLoading: isIdentityLoading } = useCurrentUserId(); + const replyDraftKey = prReplyDraftKey(owner, repo, number, commentId); + const draft = useFencedDraftLoad({ userId, isIdentityLoading, entityKey: replyDraftKey }); + useDraftFlushOnBackground(userId, replyDraftKey, true); + + // Restore the stored draft into the input once the load settles for a new + // identity/thread, and reset the field so a reused instance never keeps the + // previous account's or thread's text (which it could then save under the + // new key). + const replySeedKey = `${userId ?? 'anonymous'}\u0000${replyDraftKey}`; + const seededKeyRef = useRef(null); + useEffect(() => { + if (!draft.settled || seededKeyRef.current === replySeedKey) { + return; + } + seededKeyRef.current = replySeedKey; + bodyRef.current = draft.value ?? ''; + setResetKey(prev => prev + 1); + }, [draft.settled, draft.value, replySeedKey]); + // Mirror mutation error into the inline box. Reply is NOT // optimistic, so the user can hit the inline error and retry // without waiting for a re-fetch. @@ -225,6 +251,9 @@ export function ReplyInput({ owner, repo, number, commentId, reply }: Readonly { bodyRef.current = ''; + if (userId) { + void clearDraft(userId, replyDraftKey); + } setResetKey(prev => prev + 1); }, } @@ -236,13 +265,16 @@ export function ReplyInput({ owner, repo, number, commentId, reply }: Readonly { bodyRef.current = value; + if (userId) { + saveDraft(userId, replyDraftKey, value); + } if (inlineError) { setInlineError(null); setInlineErrorKind(null); diff --git a/apps/mobile/src/components/pr-review/merge/pr-merge-sheet.test.tsx b/apps/mobile/src/components/pr-review/merge/pr-merge-sheet.test.tsx index 355231c585..53a69f50c2 100644 --- a/apps/mobile/src/components/pr-review/merge/pr-merge-sheet.test.tsx +++ b/apps/mobile/src/components/pr-review/merge/pr-merge-sheet.test.tsx @@ -9,6 +9,7 @@ import { } from '@/lib/pr-review/merge/merge-result-banner-store'; import { type PrOverviewRepoSettings } from '@/lib/pr-review/merge/merge-blocked-reasons'; import { MergeNotCompletedError } from '@/lib/pr-review/merge/merge-result-error'; +import { clearDraft } from '@/lib/persist/drafts'; const mergeMutationMocks = vi.hoisted(() => ({ mutateAsync: vi.fn<() => Promise>(), @@ -123,6 +124,33 @@ vi.mock('@/lib/pr-review/merge/merge-commit-defaults', () => ({ defaultCommitMessage: () => '', })); +// The sheet imports the durable-draft chain, which pulls in the native +// encrypted-kv → expo-secure-store → expo-modules-core chain that the node +// test environment cannot resolve. Mock the persist chain and the identity +// hook so this suite stays node-only. +vi.mock('@/lib/persist/drafts', () => ({ + saveDraft: vi.fn(), + clearDraft: vi.fn(), + isMergeDraft: vi.fn(), + prMergeDraftKey: vi.fn( + (owner: string, repo: string, number: number) => `pr-merge:${owner}/${repo}#${number}` + ), + prReplyDraftKey: vi.fn(), + prCommentDraftKey: vi.fn(), +})); + +vi.mock('@/lib/persist/use-draft-load', () => ({ + useFencedDraftLoad: () => ({ settled: true, value: null }), +})); + +vi.mock('@/lib/persist/use-draft-flush', () => ({ + useDraftFlushOnBackground: () => {}, +})); + +vi.mock('@/lib/hooks/use-current-user-id', () => ({ + useCurrentUserId: () => ({ userId: 'u1', isLoading: false }), +})); + const REF = { owner: 'octocat', repo: 'hello', number: 1 }; const repoSettings: PrOverviewRepoSettings = { @@ -256,6 +284,7 @@ describe('PrMergeSheet performSubmit wiring (P0-B-08)', () => { ); expect(onRefetch).toHaveBeenCalledTimes(1); expect(onDismiss).toHaveBeenCalledTimes(1); + expect(clearDraft).toHaveBeenCalledWith('u1', 'pr-merge:octocat/hello#1'); }); it('clean success (merged:true + branchDeleted:true) fires haptic and dismisses without writing a banner', async () => { @@ -278,6 +307,7 @@ describe('PrMergeSheet performSubmit wiring (P0-B-08)', () => { ); expect(onRefetch).toHaveBeenCalledTimes(1); expect(onDismiss).toHaveBeenCalledTimes(1); + expect(clearDraft).toHaveBeenCalledWith('u1', 'pr-merge:octocat/hello#1'); }); it('rejected mutation (merged:false) does not fire haptic, refetch, dismiss, or write a banner', async () => { @@ -294,5 +324,52 @@ describe('PrMergeSheet performSubmit wiring (P0-B-08)', () => { expect(Haptics.notificationAsync).not.toHaveBeenCalled(); expect(onRefetch).not.toHaveBeenCalled(); expect(onDismiss).not.toHaveBeenCalled(); + expect(clearDraft).not.toHaveBeenCalled(); + }); + + it('confirmed cancel clears the draft and dismisses', () => { + const onDismiss = vi.fn(); + const props = { ...baseProps, onDismiss }; + // eslint-disable-next-line new-cap + const element = PrMergeSheet(props); + const formBody = findElement({ + node: element, + type: 'MergeSheetFormBody', + prop: 'submitLabel', + value: 'Merge', + }); + if (!formBody) { + throw new Error('MergeSheetFormBody not found in rendered tree'); + } + const onDismissProp = (formBody.props as { onDismiss?: () => void }).onDismiss; + onDismissProp?.(); + expect(clearDraft).toHaveBeenCalledWith('u1', 'pr-merge:octocat/hello#1'); + expect(onDismiss).toHaveBeenCalledTimes(1); + }); + + it('auto-merge enable success clears the draft and dismisses', async () => { + const onDismiss = vi.fn(); + const onRefetch = vi.fn().mockResolvedValue(undefined); + const props = { ...baseProps, mode: 'enable-auto-merge' as const, onDismiss, onRefetch }; + + autoMergeMutationMocks.mutateAsync.mockResolvedValueOnce({}); + + // eslint-disable-next-line new-cap + const element = PrMergeSheet(props); + const formBody = findElement({ + node: element, + type: 'MergeSheetFormBody', + prop: 'submitLabel', + value: 'Enable auto-merge', + }); + if (!formBody) { + throw new Error('MergeSheetFormBody not found in rendered tree'); + } + const onConfirm = (formBody.props as { onConfirm?: () => void }).onConfirm; + onConfirm?.(); + await flushMicrotasks(); + + expect(clearDraft).toHaveBeenCalledWith('u1', 'pr-merge:octocat/hello#1'); + expect(onDismiss).toHaveBeenCalledTimes(1); }); }); diff --git a/apps/mobile/src/components/pr-review/merge/pr-merge-sheet.tsx b/apps/mobile/src/components/pr-review/merge/pr-merge-sheet.tsx index 9704bacf84..a211ce2f31 100644 --- a/apps/mobile/src/components/pr-review/merge/pr-merge-sheet.tsx +++ b/apps/mobile/src/components/pr-review/merge/pr-merge-sheet.tsx @@ -1,3 +1,4 @@ +/* eslint-disable max-lines -- the merge sheet owns the durable merge draft (load, save, seed, clear) beside the existing merge/auto-merge form; the draft wiring stays with the form it persists */ // S8 merge sheet. The orchestrator mounts this inside the // `[owner]/[repo]/[number]/merge.tsx` route; the orchestrator-wired // `PrReviewMergeScreen` fetches the overview DTO, derives the form's @@ -14,7 +15,7 @@ import * as Haptics from 'expo-haptics'; import { Alert, Keyboard, ScrollView, type TextInput, useWindowDimensions } from 'react-native'; -import { useEffect, useMemo, useRef, useState } from 'react'; +import { useCallback, useEffect, useMemo, useRef, useState } from 'react'; import { PrFormSheetHeader } from '@/components/pr-review/pr-form-sheet-chrome'; import { @@ -41,6 +42,10 @@ import { defaultCommitMessage, defaultCommitTitle, } from '@/lib/pr-review/merge/merge-commit-defaults'; +import { useCurrentUserId } from '@/lib/hooks/use-current-user-id'; +import { clearDraft, isMergeDraft, prMergeDraftKey, saveDraft } from '@/lib/persist/drafts'; +import { useDraftFlushOnBackground } from '@/lib/persist/use-draft-flush'; +import { useFencedDraftLoad } from '@/lib/persist/use-draft-load'; type PrMergeSheetMode = 'merge' | 'enable-auto-merge'; @@ -88,6 +93,25 @@ type AutoMergeInput = { commitMessage?: string; }; +/** + * Wraps an uncontrolled-input ref so every `.current` write (the parts file's + * `onChangeText`) also fires `onWrite`. The merge sheet owns the save but the + * input handlers live in `pr-merge-sheet-parts.tsx`; the proxy hooks the write + * without touching that file. + */ +function savingRef(target: { current: T }, onWrite: () => void): { current: T } { + return new Proxy(target, { + set(obj, prop, value) { + if (prop === 'current') { + obj.current = value as T; + onWrite(); + return true; + } + return Reflect.set(obj, prop, value); + }, + }); +} + export function PrMergeSheet(props: PrMergeSheetProps) { const { owner, @@ -128,6 +152,41 @@ export function PrMergeSheet(props: PrMergeSheetProps) { const titleRef = useRef(defaultCommitTitle(title, number)); const messageRef = useRef(defaultCommitMessage(bodyMarkdown)); const { height: windowHeight } = useWindowDimensions(); + + // Durable merge draft. Identity gates save/restore: nothing is written or + // read while the user id is unknown. The inputs render only once the draft + // settles, seeded from the stored value or today's defaults. + const { userId, isLoading: isIdentityLoading } = useCurrentUserId(); + const mergeDraftKey = prMergeDraftKey(owner, repoName, number); + const draft = useFencedDraftLoad<{ title: string; message: string }>({ + userId, + isIdentityLoading, + entityKey: mergeDraftKey, + validate: isMergeDraft, + }); + // Seed the fields once per identity/destination. The settled gate already + // unmounts the form on an identity/entity change, so re-seeding here (and + // resetting to today's defaults when there is no draft) keeps a reused + // instance from showing or saving the previous account's or PR's text. + const draftSeedKeyRef = useRef(null); + const draftSeedKey = `${userId ?? 'anonymous'}\u0000${mergeDraftKey}`; + if (draft.settled && draftSeedKeyRef.current !== draftSeedKey) { + draftSeedKeyRef.current = draftSeedKey; + titleRef.current = draft.value?.title ?? defaultCommitTitle(title, number); + messageRef.current = draft.value?.message ?? defaultCommitMessage(bodyMarkdown); + } + + const saveMergeDraft = useCallback(() => { + if (userId) { + saveDraft(userId, mergeDraftKey, { title: titleRef.current, message: messageRef.current }); + } + }, [userId, mergeDraftKey]); + // The parts file writes `.current` in its onChangeText handlers; the proxies + // hook those writes into the debounced save. + const titleSaveRef = useMemo(() => savingRef(titleRef, saveMergeDraft), [saveMergeDraft]); + const messageSaveRef = useMemo(() => savingRef(messageRef, saveMergeDraft), [saveMergeDraft]); + useDraftFlushOnBackground(userId, mergeDraftKey, true); + // Half detent (~0.5) vs full: hide delete-branch + tighten message so // Merge/Cancel stay above the closed-sheet limit without scrolling. const [scrollViewportHeight, setScrollViewportHeight] = useState(0); @@ -254,6 +313,11 @@ export function PrMergeSheet(props: PrMergeSheetProps) { if (celebrate) { void Haptics.notificationAsync(Haptics.NotificationFeedbackType.Success); await onRefetch(); + // The merge consumed the draft; clear it before dismissing so it never + // reappears on the next visit. + if (userId) { + void clearDraft(userId, mergeDraftKey); + } // Dismiss exactly this merge route; `onDismiss` (router.back) leaves the // refreshed PR review screen visible. Do NOT also call router.back() // here or it would pop the review screen too. @@ -299,6 +363,15 @@ export function PrMergeSheet(props: PrMergeSheetProps) { // rather than sending a method the repo does not allow. const noMethodsAllowed = methodOptions.length === 0; + // The footer Cancel is an explicit discard: clear the draft and leave. The + // header back (onBack) is a passive dismiss that keeps the draft. + function handleCancel() { + if (userId) { + void clearDraft(userId, mergeDraftKey); + } + onDismiss(); + } + // PickerSheet invariant: [header, ScrollView]; footer is trailing content. return ( <> @@ -314,27 +387,29 @@ export function PrMergeSheet(props: PrMergeSheetProps) { setScrollViewportHeight(event.nativeEvent.layout.height); }} > - + {draft.settled ? ( + + ) : null} ); diff --git a/apps/mobile/src/components/pr-review/pr-review-comment-composer.test.tsx b/apps/mobile/src/components/pr-review/pr-review-comment-composer.test.tsx new file mode 100644 index 0000000000..a48dd872a7 --- /dev/null +++ b/apps/mobile/src/components/pr-review/pr-review-comment-composer.test.tsx @@ -0,0 +1,277 @@ +// Clear-rule coverage for the comment composer's durable draft. The composer +// clears its draft on three committed outcomes — comment post, add-to-review, +// and a confirmed discard — and keeps it on a dismissed-without-confirmation +// discard. `Alert.alert` is captured so the test can press the Discard / +// Keep editing buttons the discard gate renders. +// +// The composer is mounted by calling it as a plain function (no renderer), so +// the React hook primitives are stubbed, mirroring pr-merge-sheet.test.tsx. + +import * as React from 'react'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +import { PrReviewCommentComposer } from './pr-review-comment-composer'; +import { clearDraft } from '@/lib/persist/drafts'; + +type AlertButton = { text?: string; style?: string; onPress?: () => void }; +type AlertCall = { title: string; message: string; buttons: AlertButton[] }; + +const { alertCalls, createCommentMocks } = vi.hoisted(() => ({ + alertCalls: [] as AlertCall[], + createCommentMocks: { + mutateAsync: vi.fn<() => Promise>(), + isPending: false, + error: null as Error | null, + }, +})); + +vi.mock('react', async () => { + const actual = await vi.importActual('react'); + return { + ...actual, + useState: vi.fn( + (initial: T) => [initial, vi.fn() as () => void] as [T, (value: T) => void] + ), + useMemo: vi.fn((factory: () => T) => factory()), + useRef: vi.fn((initial: T) => { + const ref: React.RefObject = { current: initial }; + return ref; + }), + useEffect: vi.fn((effect: React.EffectCallback) => { + effect(); + }), + useCallback: vi.fn( unknown>(fn: T) => fn), + }; +}); + +vi.mock('react-native', () => ({ + Alert: { + alert: (title: string, message: string, buttons: AlertButton[]) => { + alertCalls.push({ title, message, buttons }); + }, + }, + Keyboard: { addListener: vi.fn(() => ({ remove: vi.fn() })) }, + ScrollView: 'ScrollView', + View: 'View', + TextInput: 'TextInput', + Platform: { OS: 'ios' }, +})); + +vi.mock('expo-crypto', () => ({ + randomUUID: () => 'not-used', +})); + +vi.mock('expo-haptics', () => ({ + impactAsync: vi.fn(), + notificationAsync: vi.fn(), + ImpactFeedbackStyle: { Light: 'Light' }, + NotificationFeedbackType: { Success: 'Success' }, +})); + +vi.mock('@/components/ui/button', () => ({ Button: 'Button' })); +vi.mock('@/components/ui/text', () => ({ Text: 'Text' })); + +vi.mock('@/components/pr-review/pr-form-sheet-chrome', () => ({ + PrFormSheetHeader: 'PrFormSheetHeader', + useFormSheetKeyboardVisible: () => false, +})); + +vi.mock('@/components/pr-review/composer-inline-error', () => ({ + ComposerInlineError: 'ComposerInlineError', + useComposerInlineError: () => ({ + inlineError: null, + inlineErrorKind: null, + inlineErrorIsLocal: false, + setInlineError: vi.fn(), + setInlineErrorKind: vi.fn(), + setInlineErrorIsLocal: vi.fn(), + clearBadRequestOnBodyEdit: vi.fn(), + }), +})); + +vi.mock('@/components/pr-review/pr-review-comment-composer-parts', () => ({ + CommentBodyField: 'CommentBodyField', + ComposerFooter: 'ComposerFooter', + composerRangeLabel: (line: number, startLine?: number) => + startLine !== undefined && startLine !== line ? `L${startLine}–L${line}` : `L${line}`, + ContextPreview: 'ContextPreview', +})); + +vi.mock('@/components/pr-review/discussion/reply-input', () => ({ + ensureTermsAcceptedOutcome: vi.fn().mockResolvedValue({ kind: 'accepted' as const }), + TERMS_OUTDATED_COPY: 'outdated', +})); + +vi.mock('@/lib/hooks/use-current-user-id', () => ({ + useCurrentUserId: () => ({ userId: 'u1', isLoading: false }), +})); + +vi.mock('@/lib/persist/drafts', () => ({ + saveDraft: vi.fn(), + clearDraft: vi.fn(), + prCommentDraftKey: vi.fn(() => 'pr-comment:key'), + prReplyDraftKey: vi.fn(), + prMergeDraftKey: vi.fn(), +})); + +vi.mock('@/lib/persist/use-draft-load', () => ({ + useFencedDraftLoad: () => ({ settled: true, value: null }), +})); + +vi.mock('@/lib/persist/use-draft-flush', () => ({ + useDraftFlushOnBackground: vi.fn(), +})); + +vi.mock('@/lib/pr-review/build-suggestion-fence', () => ({ + buildSuggestionFence: () => null, +})); + +vi.mock('@/lib/pr-review/diff-selection-bridge', () => ({ + getDiffSelection: () => null, +})); + +vi.mock('@/lib/pr-review/pending-review-provider', () => ({ + usePendingReview: () => ({ + items: [], + addComment: vi.fn(), + updateComment: vi.fn(), + removeComment: vi.fn(), + clear: vi.fn(), + }), +})); + +vi.mock('@/lib/pr-review/use-pr-review-mutations', () => ({ + useCreateReviewCommentMutation: () => ({ + mutateAsync: createCommentMocks.mutateAsync, + isPending: createCommentMocks.isPending, + error: createCommentMocks.error, + }), +})); + +const baseProps = { + owner: 'octocat', + repo: 'hello', + number: 1, + mode: { kind: 'create', headSha: 'a'.repeat(40) } as const, + path: 'src/a.ts', + side: 'RIGHT' as const, + line: 10, + title: 'Comment', + eyebrow: 'octocat/hello#1', + onDismiss: vi.fn(), +}; + +function findByType(node: unknown, type: string): React.ReactElement | null { + if (React.isValidElement(node)) { + const element = node; + if (element.type === type) { + return element; + } + const children = (element.props as Record).children; + if (Array.isArray(children)) { + for (const child of children) { + const found = findByType(child, type); + if (found) { + return found; + } + } + } else if (children !== undefined && children !== null) { + const found = findByType(children, type); + if (found) { + return found; + } + } + } + if (Array.isArray(node)) { + for (const child of node) { + const found = findByType(child, type); + if (found) { + return found; + } + } + } + return null; +} + +function mountComposer(): React.ReactElement { + // eslint-disable-next-line new-cap + return PrReviewCommentComposer(baseProps); +} + +function typeBody(element: React.ReactElement, text: string): void { + const field = findByType(element, 'CommentBodyField'); + if (!field) { + throw new Error('CommentBodyField not found'); + } + (field.props as { onChangeText?: (value: string) => void }).onChangeText?.(text); +} + +function footerProp(element: React.ReactElement, prop: string): (() => void) | undefined { + const footer = findByType(element, 'ComposerFooter'); + if (!footer) { + throw new Error('ComposerFooter not found'); + } + return (footer.props as Record void) | undefined>)[prop]; +} + +async function flushMicrotasks(): Promise { + await new Promise(resolve => { + setTimeout(resolve, 0); + }); +} + +describe('PrReviewCommentComposer draft clear rules', () => { + beforeEach(() => { + alertCalls.length = 0; + createCommentMocks.mutateAsync.mockReset(); + createCommentMocks.isPending = false; + createCommentMocks.error = null; + vi.clearAllMocks(); + }); + + it('clears the draft on add-to-review', () => { + const element = mountComposer(); + typeBody(element, 'hello'); + footerProp(element, 'onAddToReview')?.(); + + expect(clearDraft).toHaveBeenCalledWith('u1', 'pr-comment:key'); + }); + + it('clears the draft on a successful comment post', async () => { + createCommentMocks.mutateAsync.mockResolvedValueOnce({}); + const element = mountComposer(); + typeBody(element, 'hello'); + footerProp(element, 'onCommentNow')?.(); + await flushMicrotasks(); + + expect(clearDraft).toHaveBeenCalledWith('u1', 'pr-comment:key'); + }); + + it('clears the draft on a confirmed discard', () => { + const element = mountComposer(); + typeBody(element, 'hello'); + footerProp(element, 'onCancel')?.(); + + const call = alertCalls.at(-1); + if (!call) { + throw new Error('No discard Alert was shown'); + } + call.buttons.find(b => b.style === 'destructive')?.onPress?.(); + + expect(clearDraft).toHaveBeenCalledWith('u1', 'pr-comment:key'); + }); + + it('does not clear the draft on a dismissed-without-confirmation discard', () => { + const element = mountComposer(); + typeBody(element, 'hello'); + footerProp(element, 'onCancel')?.(); + + const call = alertCalls.at(-1); + if (!call) { + throw new Error('No discard Alert was shown'); + } + call.buttons.find(b => b.text === 'Keep editing')?.onPress?.(); + + expect(clearDraft).not.toHaveBeenCalled(); + }); +}); diff --git a/apps/mobile/src/components/pr-review/pr-review-comment-composer.tsx b/apps/mobile/src/components/pr-review/pr-review-comment-composer.tsx index 402884d1f8..591e08f0fa 100644 --- a/apps/mobile/src/components/pr-review/pr-review-comment-composer.tsx +++ b/apps/mobile/src/components/pr-review/pr-review-comment-composer.tsx @@ -1,3 +1,4 @@ +/* eslint-disable max-lines -- the comment composer owns the durable comment draft (load, save, seed, clear) beside the existing create/edit form; the draft wiring stays with the form it persists */ // Comment-composer content. Two modes: // - create: Comment now + Add to review + Insert suggestion (needs headSha). // - edit: single Save updating a queued PendingReviewItem (local-only). @@ -29,6 +30,10 @@ import { ensureTermsAcceptedOutcome, TERMS_OUTDATED_COPY, } from '@/components/pr-review/discussion/reply-input'; +import { useCurrentUserId } from '@/lib/hooks/use-current-user-id'; +import { clearDraft, prCommentDraftKey, saveDraft } from '@/lib/persist/drafts'; +import { useDraftFlushOnBackground } from '@/lib/persist/use-draft-flush'; +import { useFencedDraftLoad } from '@/lib/persist/use-draft-load'; import { buildSuggestionFence } from '@/lib/pr-review/build-suggestion-fence'; import { getDiffSelection } from '@/lib/pr-review/diff-selection-bridge'; import { usePendingReview } from '@/lib/pr-review/pending-review-provider'; @@ -73,6 +78,18 @@ export function PrReviewCommentComposer(props: PrReviewCommentComposerProps) { const createComment = useCreateReviewCommentMutation({ owner, repo, number }); const isEdit = mode.kind === 'edit'; + // Durable comment draft (create mode only). Edit mode edits an already-queued + // item, durable through the pending-review provider, so no draft there. + const { userId, isLoading: isIdentityLoading } = useCurrentUserId(); + const commentDraftKey = prCommentDraftKey(owner, repo, number, path, side, line, startLine); + const draftUserId = isEdit ? undefined : userId; + const draft = useFencedDraftLoad({ + userId: draftUserId, + isIdentityLoading, + entityKey: commentDraftKey, + }); + useDraftFlushOnBackground(draftUserId, commentDraftKey, true); + // Edit mode ignores the bridge so editing A never shows B's path/lines. const selection = isEdit ? null : getDiffSelection({ owner, repo, number }); @@ -82,6 +99,17 @@ export function PrReviewCommentComposer(props: PrReviewCommentComposerProps) { const bodyInputRef = useRef(null); const scrollRef = useRef(null); const [hasBody, setHasBody] = useState(() => initialBody.trim().length > 0); + // Seed the refs from the settled draft once per identity/destination, before + // the body field mounts (create mode only). Re-seeding on a key change (and + // resetting to the initial body when there is no draft) keeps a reused + // instance from showing or saving the previous account's or position's text. + const draftSeedKeyRef = useRef(null); + const draftSeedKey = `${draftUserId ?? 'anonymous'}\u0000${commentDraftKey}`; + if (!isEdit && draft.settled && draftSeedKeyRef.current !== draftSeedKey) { + draftSeedKeyRef.current = draftSeedKey; + bodyRef.current = draft.value ?? initialBody; + bodyBaselineRef.current = draft.value ?? initialBody; + } const { inlineError, inlineErrorKind, @@ -113,6 +141,9 @@ export function PrReviewCommentComposer(props: PrReviewCommentComposerProps) { bodyRef.current = value; setHasBody(value.trim().length > 0); clearBadRequestOnBodyEdit(); + if (draftUserId) { + saveDraft(draftUserId, commentDraftKey, value); + } } function handleAddToReview() { @@ -137,6 +168,9 @@ export function PrReviewCommentComposer(props: PrReviewCommentComposerProps) { body, commitSha: mode.headSha, }); + if (draftUserId) { + void clearDraft(draftUserId, commentDraftKey); + } void Haptics.impactAsync(Haptics.ImpactFeedbackStyle.Light); onDismiss(); } @@ -177,6 +211,9 @@ export function PrReviewCommentComposer(props: PrReviewCommentComposerProps) { ...(startLine !== undefined ? { startLine, startSide: side } : {}), commitSha: mode.headSha, }); + if (draftUserId) { + void clearDraft(draftUserId, commentDraftKey); + } void Haptics.notificationAsync(Haptics.NotificationFeedbackType.Success); onDismiss(); } catch { @@ -207,7 +244,16 @@ export function PrReviewCommentComposer(props: PrReviewCommentComposerProps) { if (dirty) { Alert.alert('Discard comment?', 'Your draft will be lost.', [ { text: 'Keep editing', style: 'cancel' }, - { text: 'Discard', style: 'destructive', onPress: onDismiss }, + { + text: 'Discard', + style: 'destructive', + onPress: () => { + if (draftUserId) { + void clearDraft(draftUserId, commentDraftKey); + } + onDismiss(); + }, + }, ]); return; } @@ -225,6 +271,11 @@ export function PrReviewCommentComposer(props: PrReviewCommentComposerProps) { bodyRef.current = block; setHasBody(block.trim().length > 0); clearBadRequestOnBodyEdit(); + // Persist the inserted suggestion like a typed change, so a process kill + // after Insert (with no later keystroke) does not lose the suggestion. + if (draftUserId) { + saveDraft(draftUserId, commentDraftKey, block); + } bodyInputRef.current?.setNativeProps({ text: block, selection: { start: block.length, end: block.length }, @@ -278,12 +329,14 @@ export function PrReviewCommentComposer(props: PrReviewCommentComposerProps) { /> Comment - + {isEdit || draft.settled ? ( + + ) : null} {showInsertSuggestion ? ( ) : null} - {failure.canCopy && onCopyToComposer ? ( + {failure.canCopy && onCopyToComposer && copyText !== '' ? (