From 449c88d0c6dc5bd3b412f23de8b6e4bc796fcd8e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20=C5=A0=C4=87eki=C4=87?= Date: Thu, 24 Sep 2026 17:25:57 +0000 Subject: [PATCH] fix(mobile): keep the KiloClaw tab stable and shape the live-session placeholder Surface: mobile-app Explorer finding: 2 visual defects on android: The bottom navigation renders only three; The LIVE NOW section renders a large bla The user-agent explorer found these 2 while using the app like a user. They are one kind of defect on one platform, so they are ONE item: the pull request must fix and prove every one of them. One must-run scenario per finding: the plan needs 2 musts, one for each finding below. The explorer never edits product code. Evidence (from the device runs): --- Finding 1 of 2: agents-empty: The bottom navigation renders only three tabs (HOME, AGENTS, PROFILE) here and on Profile, while the Home and KiloClaw screens of the same app render four (HOME, KILOCLAW, AGENTS, PROFILE), so the tab bar changes shape from screen to screen. Flow: agents-empty Found on revision: d846cc77f Repro: 1. set this state first: uimode day; battery 74 discharging; reviews 5; credits 18; relaunch; deeplink kiloapp://expo-development-client/?url=http%3A%2F%2Flocalhost%3A20081; resume 2. open the app on emulator-5602 3. reach agents-empty 4. the capture shows the defect named below Observed: The bottom navigation renders only three tabs (HOME, AGENTS, PROFILE) here and on Profile, while the Home and KiloClaw screens of the same app render four (HOME, KILOCLAW, AGENTS, PROFILE), so the tab bar changes shape from screen to screen. Expected: the screen renders without this defect - ~/.local/share/kwf/findings/explorer-2-visual-defects-on-android-the-bottom-navi --- .../home/agent-sessions-section.test.ts | 49 ++++++- .../home/agent-sessions-section.tsx | 18 ++- .../home/home-screen.mounted.test.tsx | 22 ++- .../use-kiloclaw-tab-visible.mounted.test.tsx | 126 ++++++++++++++++++ .../src/lib/hooks/use-kiloclaw-tab-visible.ts | 8 +- 5 files changed, 213 insertions(+), 10 deletions(-) create mode 100644 apps/mobile/src/lib/hooks/use-kiloclaw-tab-visible.mounted.test.tsx diff --git a/apps/mobile/src/components/home/agent-sessions-section.test.ts b/apps/mobile/src/components/home/agent-sessions-section.test.ts index 2cda92b3bda..e7490cac3cb 100644 --- a/apps/mobile/src/components/home/agent-sessions-section.test.ts +++ b/apps/mobile/src/components/home/agent-sessions-section.test.ts @@ -3,7 +3,7 @@ import * as ReactQuery from '@tanstack/react-query'; import { act, TestRenderer } from '@/test/renderer'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; -import '@/i18n'; +import { i18n } from '@/i18n'; import { AgentSessionsSection } from '@/components/home/agent-sessions-section'; import { RemoteSessionRow } from '@/components/agents/remote-session-row'; import { type ActiveSession } from '@/lib/hooks/use-agent-sessions'; @@ -142,9 +142,9 @@ function node(type: string, index = 0) { } return result; } -async function render(sessions = settled) { +async function render(sessions = settled, contextOverride = context) { await act(async () => { - const tree = createElement(AgentSessionsSection, { context, sessions }); + const tree = createElement(AgentSessionsSection, { context: contextOverride, sessions }); if (renderer) { renderer.update(tree); } else { @@ -189,6 +189,49 @@ describe('Home live section', () => { expect(sessionDestination.id).toBe('a1'); }); + it('renders the pending state as a row-shaped skeleton inside the reserved frame', async () => { + await render({ ...settled, hasAcceptedSuccess: false }, { ...context, isResolving: true }); + const skeletons = nodes('Skeleton'); + expect(skeletons.length).toBeGreaterThan(0); + expect( + skeletons.some(skeleton => String(skeleton.props.className ?? '').includes('w-full')) + ).toBe(false); + expect( + nodes('View').some(view => { + const className = String(view.props.className ?? ''); + return className.includes('min-h-[72px]') && className.includes('rounded-2xl'); + }) + ).toBe(true); + expect(nodes('Text').some(text => text.children.includes(i18n.t('home.noLiveSessions')))).toBe( + false + ); + }); + + it('places the pending placeholder on the real row geometry so the arriving row cannot reflow it', async () => { + await render({ ...settled, activeSessions: [session('a1')] }); + const rowGeometry = String( + nodes('View').find(view => String(view.props.className ?? '').includes('py-[13px]'))?.props + .className + ); + expect(rowGeometry).toContain('py-[13px]'); + + await render({ ...settled, hasAcceptedSuccess: false }, { ...context, isResolving: true }); + const placeholderRow = nodes('View').find(view => + String(view.props.className ?? '').includes('py-[13px]') + ); + expect(placeholderRow).toBeDefined(); + // The same row box the incoming SessionRow draws: the copy lands on the + // same x-offset, so swapping the placeholder for the row cannot shift it. + expect(String(placeholderRow?.props.className)).toBe(rowGeometry); + // The leading mark is the row's own 3px edge strip, not a 32px circle. + expect( + nodes('Skeleton').some(skeleton => { + const className = String(skeleton.props.className ?? ''); + return className.includes('absolute') && className.includes('w-[3px]'); + }) + ).toBe(true); + }); + it.each([ ['running', 'running', false], ['idle', 'idle', false], diff --git a/apps/mobile/src/components/home/agent-sessions-section.tsx b/apps/mobile/src/components/home/agent-sessions-section.tsx index 75707cdad2b..fb524275d91 100644 --- a/apps/mobile/src/components/home/agent-sessions-section.tsx +++ b/apps/mobile/src/components/home/agent-sessions-section.tsx @@ -303,7 +303,23 @@ export function AgentSessionsSection({ context, sessions }: LiveSessionProps) { sessions={sessions} failureLabel={t('home.couldNotLoadActiveSessions')} /> - {content === 'pending' && } + {content === 'pending' && ( + // The placeholder borrows the real row's geometry: the same card, + // the same row padding, and a 3px leading strip glued to the card + // edge like `SessionRow`'s `stripMode="edge"` in `AgentBadge`. The + // title/eyebrow therefore land on the same x-offset the arriving row + // draws, and the leading mark keeps its size, so replacing the + // placeholder with the row cannot reflow the LIVE NOW card. + + + + + + + + + + )} {content === 'empty' && ( diff --git a/apps/mobile/src/components/home/home-screen.mounted.test.tsx b/apps/mobile/src/components/home/home-screen.mounted.test.tsx index 14eb10eab51..32c387a7e8b 100644 --- a/apps/mobile/src/components/home/home-screen.mounted.test.tsx +++ b/apps/mobile/src/components/home/home-screen.mounted.test.tsx @@ -343,7 +343,7 @@ describe('HomeScreen composition', () => { expect(state.liveQuery).toHaveBeenLastCalledWith({ organizationId: 'org-1', enabled: loaded }); expect(nodes('RemoteSessionRow')).toHaveLength(loaded ? 1 : 0); expect(nodes('NewTaskButton')).toHaveLength(loaded ? 1 : 0); - expect(nodes('Skeleton')).toHaveLength(loaded ? 0 : 1); + expect(nodes('Skeleton')).toHaveLength(loaded ? 0 : 3); }); it.each([false, true])( @@ -411,9 +411,21 @@ describe('Home live presentation', () => { ])('keeps valid actions and truthful content during $name', async test => { Object.assign(state.live, test.patch); await renderHome(); - expect(nodes('Skeleton')).toHaveLength(test.skeleton ? 1 : 0); + expect(nodes('Skeleton')).toHaveLength(test.skeleton ? 3 : 0); if (test.skeleton) { - expect(nodes('Skeleton')[0]?.props.className).toContain('min-h-[72px]'); + // The placeholder keeps the row frame: a reserved min-height card, and + // no full-width skeleton block standing in for the whole surface. + expect( + nodes('View').some(view => { + const className = String(view.props.className ?? ''); + return className.includes('min-h-[72px]') && className.includes('rounded-2xl'); + }) + ).toBe(true); + expect( + nodes('Skeleton').some(skeleton => + String(skeleton.props.className ?? '').includes('w-full') + ) + ).toBe(false); } expect(text().includes('Nothing running right now')).toBe(Boolean(test.empty)); expect(text().includes("Couldn't load active sessions")).toBe(Boolean(test.error)); @@ -461,7 +473,7 @@ describe('Home live presentation', () => { state.connection.isConnected = false; state.live.isPaused = true; await renderHome(); - expect(nodes('Skeleton')).toHaveLength(1); + expect(nodes('Skeleton')).toHaveLength(3); expect(text()).not.toContain('Nothing running right now'); expect(text()).not.toContain('Connecting…'); expect(text()).not.toContain('No internet connection'); @@ -745,7 +757,7 @@ describe('Home admission', () => { expect(text()).not.toContain('Engineering'); } if (mode === 'membership paused') { - expect(nodes('Skeleton')).toHaveLength(1); + expect(nodes('Skeleton')).toHaveLength(3); expect(text()).not.toContain('Organization unavailable'); } if ( diff --git a/apps/mobile/src/lib/hooks/use-kiloclaw-tab-visible.mounted.test.tsx b/apps/mobile/src/lib/hooks/use-kiloclaw-tab-visible.mounted.test.tsx new file mode 100644 index 00000000000..5323849b121 --- /dev/null +++ b/apps/mobile/src/lib/hooks/use-kiloclaw-tab-visible.mounted.test.tsx @@ -0,0 +1,126 @@ +import { createElement } from 'react'; +import { act, TestRenderer } from '@/test/renderer'; +import { beforeEach, describe, expect, it, onTestFinished, vi } from 'vitest'; + +// The instance list is the only input the hook reads from the query layer; the +// mock holds it in one place so a test can move it from pending to resolved. +const instances = vi.hoisted(() => ({ data: undefined as { id: string }[] | undefined })); +const secureStore = vi.hoisted(() => ({ + getItem: vi.fn<(key: string) => string | null>(), + setItem: vi.fn<(key: string, value: string) => void>(), +})); + +vi.mock('@/lib/hooks/use-instance-context', () => ({ + useAllKiloClawInstances: () => ({ data: instances.data }), +})); +vi.mock('expo-secure-store', () => ({ + getItem: secureStore.getItem, + setItem: secureStore.setItem, +})); + +type UseKiloClawTabVisible = () => boolean; + +// The ownership module caches its answer in a module variable, so every test +// reloads the hook after `vi.resetModules()` to start from a cold read. +async function loadHook(): Promise { + const { useKiloClawTabVisible } = await import('./use-kiloclaw-tab-visible'); + return useKiloClawTabVisible; +} + +function Probe({ useVisible }: { useVisible: UseKiloClawTabVisible }) { + return createElement('ProbeText', null, String(useVisible())); +} + +function textOf(renderer: TestRenderer.ReactTestRenderer): string | null { + const json = renderer.toJSON(); + if (!json || Array.isArray(json)) { + return null; + } + const child = json.children[0]; + return typeof child === 'string' ? child : null; +} + +function mount(useVisible: UseKiloClawTabVisible): { + visible: () => string | null; + setInstances: (data: { id: string }[] | undefined) => void; +} { + const ref: { renderer: TestRenderer.ReactTestRenderer | undefined } = { renderer: undefined }; + act(() => { + ref.renderer = TestRenderer.create(createElement(Probe, { useVisible })); + }); + const renderer = ref.renderer; + if (!renderer) { + throw new Error('renderer was not created'); + } + onTestFinished(() => { + act(() => { + renderer.unmount(); + }); + }); + return { + visible: () => textOf(renderer), + setInstances: data => { + instances.data = data; + act(() => { + renderer.update(createElement(Probe, { useVisible })); + }); + }, + }; +} + +beforeEach(() => { + vi.resetModules(); + instances.data = undefined; + secureStore.getItem.mockReset(); + secureStore.getItem.mockReturnValue(null); + secureStore.setItem.mockReset(); +}); + +describe('useKiloClawTabVisible session stability', () => { + it('keeps the tab after a stale persisted "1" is contradicted by an empty list', async () => { + secureStore.getItem.mockReturnValue('1'); + const useVisible = await loadHook(); + const probe = mount(useVisible); + + expect(probe.visible()).toBe('true'); + + probe.setInstances([]); + + expect(probe.visible()).toBe('true'); + }); + + it('keeps the tab hidden for a persisted "0" and an empty list', async () => { + secureStore.getItem.mockReturnValue('0'); + const useVisible = await loadHook(); + const probe = mount(useVisible); + + expect(probe.visible()).toBe('false'); + + probe.setInstances([]); + + expect(probe.visible()).toBe('false'); + }); + + it('shows the tab when the list confirms an instance', async () => { + secureStore.getItem.mockReturnValue('0'); + const useVisible = await loadHook(); + const probe = mount(useVisible); + + expect(probe.visible()).toBe('false'); + + probe.setInstances([{ id: 'instance-1' }]); + + expect(probe.visible()).toBe('true'); + }); + + it('persists the fetched empty-list answer for the next launch', async () => { + secureStore.getItem.mockReturnValue('1'); + const useVisible = await loadHook(); + const probe = mount(useVisible); + + probe.setInstances([]); + + expect(probe.visible()).toBe('true'); + expect(secureStore.setItem).toHaveBeenCalledWith('kiloclaw-owned', '0'); + }); +}); diff --git a/apps/mobile/src/lib/hooks/use-kiloclaw-tab-visible.ts b/apps/mobile/src/lib/hooks/use-kiloclaw-tab-visible.ts index 7cdbce37f3a..e184242c913 100644 --- a/apps/mobile/src/lib/hooks/use-kiloclaw-tab-visible.ts +++ b/apps/mobile/src/lib/hooks/use-kiloclaw-tab-visible.ts @@ -8,6 +8,12 @@ import { persistKiloClawOwned, readKiloClawOwned } from '@/lib/kiloclaw-tab-owne * the tab count is correct on the first frame, then reconciled from the * instance list. A failed or pending fetch keeps the persisted answer. * + * The bar is session-stable: once the entry is shown it stays for the rest of + * the session, so a stale persisted answer contradicted by the fetched list + * never removes a tab the user is looking at. The fetched list may still add + * the entry, and `persistKiloClawOwned` records the fetched truth so the next + * launch hides it again when no instance exists. + * * The poll is off: this hook is mounted for the whole signed-in session, and * ownership changes rarely. The list still refreshes on a cold start, on a Home * pull-to-refresh, and from the KiloClaw tab's own poll and invalidations. @@ -22,7 +28,7 @@ export function useKiloClawTabVisible(): boolean { } const owned = instances.length > 0; persistKiloClawOwned(owned); - setVisible(owned); + setVisible(previous => previous || owned); }, [instances]); return visible;