diff --git a/apps/mobile/src/app/(app)/_layout.tsx b/apps/mobile/src/app/(app)/_layout.tsx index 20ec428c07..c5e9c9d11e 100644 --- a/apps/mobile/src/app/(app)/_layout.tsx +++ b/apps/mobile/src/app/(app)/_layout.tsx @@ -54,6 +54,15 @@ export default function AppLayout() { headerShown: false, }} /> + (); const trpc = useTRPC(); const router = useRouter(); @@ -30,7 +43,31 @@ export default function SessionDetailScreen() { ...trpc.cliSessionsV2.get.queryOptions( { session_id: sessionId }, { - retry: false, + retry: (failureCount, error) => + shouldRetryNotFoundOnSpawnedRoute({ + spawned, + attempt: failureCount, + // TRPCClientErrorLike exposes `data.code`; the route's + // existing NOT_FOUND check (`sessionQuery.error.data?.code + // === 'NOT_FOUND'`) reads from the same field. We + // defensively walk a couple of shapes because TRPC + // versions across this app occasionally wrap the code + // one level deeper. + errorCode: + (error as { data?: { code?: string } } | null)?.data?.code ?? + (error as { code?: string } | null)?.code, + }), + // kilocode_change - C3b: TanStack Query's default retryDelay is + // exponential backoff (1s, 2s, 4s, 8s, ... capped at 30s), which + // would stretch the 8-attempt ceiling well past the "~8s is + // generous" budget the spawned-row window actually needs. Pin a + // flat 1s cadence so 8 attempts stay close to 8 seconds elapsed, + // matching the plan's stated timing. Only in effect while + // `shouldRetryNotFoundOnSpawnedRoute` above is even allowing a + // retry (i.e. only on the `spawned=1` NOT_FOUND path) — everywhere + // else `retry` already returns `false` on the first failure, so + // this delay is never consulted. + retryDelay: 1000, } ), enabled: routeOrganizationId === undefined, diff --git a/apps/mobile/src/app/(app)/agent-chat/instance-picker.tsx b/apps/mobile/src/app/(app)/agent-chat/instance-picker.tsx new file mode 100644 index 0000000000..8cda6869dd --- /dev/null +++ b/apps/mobile/src/app/(app)/agent-chat/instance-picker.tsx @@ -0,0 +1,264 @@ +import { useFocusEffect, useRouter } from 'expo-router'; +import * as Haptics from 'expo-haptics'; +import { Check, Cloud, Server } from 'lucide-react-native'; +import { useQuery } from '@tanstack/react-query'; +import { useCallback, useMemo, useRef, useState } from 'react'; +import { FlatList, Pressable, View } from 'react-native'; +import { useSafeAreaInsets } from 'react-native-safe-area-context'; + +import { EmptyState } from '@/components/empty-state'; +import { PickerSheet } from '@/components/picker-sheet'; +import { Button } from '@/components/ui/button'; +import { Skeleton } from '@/components/ui/skeleton'; +import { Text } from '@/components/ui/text'; +import { useThemeColors } from '@/lib/hooks/use-theme-colors'; +import { + clearInstancePickerBridge, + getInstancePickerBridge, + type InstancePickerInstance, +} from '@/lib/picker-bridge'; +import { + dedupeInstanceLabels, + type LabeledInstance, + resolveInstancePickerViewState, +} from '@/lib/instance-picker-rows'; +import { useTRPC } from '@/lib/trpc'; + +const POLL_INTERVAL_MS = 10_000; +const SKELETON_ROW_COUNT = 4; + +export default function InstancePickerScreen() { + const router = useRouter(); + const colors = useThemeColors(); + const { bottom } = useSafeAreaInsets(); + const [bridge, setBridge] = useState(() => getInstancePickerBridge()); + const bridgeRef = useRef(bridge); + + const closePicker = useCallback(() => { + router.back(); + }, [router]); + + // The instances query lives IN the picker (per the slice spec) so an + // already-open picker self-populates as CLIs connect/disconnect without + // needing the parent new-agent screen to keep it warm. `refetchOnWindowFocus` + // plus the 10s poll covers the AC1 "an already-open picker populates + // without closing" requirement from both directions (foreground return + // and steady background ticking). + const trpc = useTRPC(); + const { + data: instancesData, + isPending: isLoadingInstances, + isError: isInstancesError, + isRefetching, + refetch: refetchInstances, + } = useQuery({ + ...trpc.activeSessions.listInstances.queryOptions(undefined, { + refetchOnWindowFocus: true, + refetchInterval: POLL_INTERVAL_MS, + refetchIntervalInBackground: false, + }), + // The listInstances procedure is personal-only; a tRPC throw here would + // not be expected from a server-side auth decision, but the network + // path can still fail and we want the retry CTA rather than a stale + // "successfully empty" snapshot. + retry: 1, + }); + + useFocusEffect( + useCallback(() => { + const nextBridge = getInstancePickerBridge(); + bridgeRef.current = nextBridge; + setBridge(nextBridge); + // kilocode_change - `refetchOnWindowFocus` only reacts to OS-level + // app foreground/background transitions, not Expo Router route + // focus. Route focus (this screen becoming the active route, i.e. + // the picker sheet opening) is the case AC1's "refetch on focus" + // actually describes, so refetch explicitly here too. + void refetchInstances(); + + return () => { + clearInstancePickerBridge(); + bridgeRef.current = null; + }; + // eslint-disable-next-line react-hooks/exhaustive-deps -- refetchInstances is a stable react-query function identity; including it would re-run this effect on every render because react-query does not memoize it across renders. + }, []) + ); + + const instances: InstancePickerInstance[] = useMemo( + () => instancesData?.instances ?? [], + [instancesData] + ); + + const labeled = useMemo(() => dedupeInstanceLabels(instances), [instances]); + + const viewState = resolveInstancePickerViewState({ + isLoading: isLoadingInstances, + isError: isInstancesError, + instances, + }); + + const handleSelectCloudAgent = useCallback(() => { + void Haptics.selectionAsync(); + bridgeRef.current?.onSelect(null); + clearInstancePickerBridge(); + bridgeRef.current = null; + closePicker(); + }, [closePicker]); + + const handleSelectInstance = useCallback( + (instance: InstancePickerInstance) => { + void Haptics.selectionAsync(); + bridgeRef.current?.onSelect(instance); + clearInstancePickerBridge(); + bridgeRef.current = null; + closePicker(); + }, + [closePicker] + ); + + if (!bridge) { + return ; + } + + const current = bridge.currentValue; + const currentConnectionId = current?.connectionId ?? null; + + // Loading: query has never produced data. The empty-snapshot state is + // "we know the list is empty" — that's success with an empty array, not + // a loading screen. + if (viewState.kind === 'loading') { + return ( + + + {Array.from({ length: SKELETON_ROW_COUNT }, (_, i) => ( + + + + + ))} + + + ); + } + + // Error: surface a retryable error per the spec. The Empty state below + // (a successful zero-instance response) and this error are distinct; + // never collapse them into a single "no instances" surface. + if (viewState.kind === 'error') { + return ( + + + { + void refetchInstances(); + }} + loading={isRefetching} + accessibilityLabel="Retry" + > + Retry + + } + /> + + + ); + } + + const renderItem = ({ item }: { item: LabeledInstance }) => { + const selected = item.connectionId === currentConnectionId; + return ( + { + handleSelectInstance(item); + }} + accessibilityRole="button" + accessibilityLabel={ + item.dedupSuffix + ? `${item.name} on ${item.projectName} (${item.dedupSuffix})` + : `${item.name} on ${item.projectName}` + } + > + + + + {item.name} + + {item.dedupSuffix ? ( + + #{item.dedupSuffix} + + ) : null} + + + {item.projectName} + + + {selected ? : null} + + ); + }; + + // Success: even if zero CLI instances are connected, we still render the + // Cloud Agent default row first (it's always selectable) and append a + // refreshable "no instances" empty card below. This matches the spec + // ("Empty: succeeds, zero instances" with a Refresh CTA) without hiding + // the only target the user can actually pick right now. + return ( + + item.connectionId} + contentContainerStyle={{ paddingBottom: bottom }} + ListHeaderComponent={ + + + + Cloud Agent + + Run on Kilo's cloud sandbox + + + {currentConnectionId === null ? : null} + + } + ListEmptyComponent={ + + { + void refetchInstances(); + }} + loading={isRefetching} + accessibilityLabel="Refresh" + > + Refresh + + } + /> + + } + renderItem={renderItem} + /> + + ); +} diff --git a/apps/mobile/src/app/(app)/agent-chat/new.tsx b/apps/mobile/src/app/(app)/agent-chat/new.tsx index c33f965c7d..15806fbc3c 100644 --- a/apps/mobile/src/app/(app)/agent-chat/new.tsx +++ b/apps/mobile/src/app/(app)/agent-chat/new.tsx @@ -6,9 +6,13 @@ import { useQuery } from '@tanstack/react-query'; import * as WebBrowser from 'expo-web-browser'; import { toast } from 'sonner-native'; +import { InstanceSelector } from '@/components/agents/instance-selector'; import { NewSessionPrompt } from '@/components/agents/new-session-prompt'; import { NewSessionRepositorySection } from '@/components/agents/new-session-repository-section'; +import { RemoteSpawnComposer } from '@/components/agents/remote-spawn-composer'; import { useNewSessionCreator } from '@/components/agents/use-new-session-creator'; +import { useRemoteSpawnDispatch } from '@/components/agents/use-remote-spawn-dispatch'; +import { REMOTE_SPAWN_INSTANCE_DISCONNECTED_NOTE } from '@/lib/remote-submit-outcome'; import { pickAgentAttachments } from '@/components/agents/attachment-picker'; import { type AgentMode } from '@/components/agents/mode-selector'; import { Button } from '@/components/ui/button'; @@ -26,6 +30,9 @@ import { useAutoSelectModel } from '@/lib/hooks/use-auto-select-model'; import { useModelPreferences } from '@/lib/hooks/use-model-preferences'; import { usePersistedAgentModel } from '@/lib/hooks/use-persisted-agent-model'; import { useThemeColors } from '@/lib/hooks/use-theme-colors'; +import { resolveNewSessionSubmitDisabled } from '@/lib/new-session-submit'; +import { type InstancePickerInstance } from '@/lib/picker-bridge'; +import { shouldShowRunOnSelector } from '@/lib/should-show-run-on-selector'; import { useTRPC } from '@/lib/trpc'; import { settleVoiceInputBeforeSubmit } from '@/lib/voice-input/voice-input-submit'; @@ -39,12 +46,25 @@ export default function NewSessionScreen() { const [model, setModel] = useState(''); const [variant, setVariant] = useState(''); const [selectedRepo, setSelectedRepo] = useState(''); + // `null` = default Cloud Agent target (the existing path). Any + // non-null value is a live `kilo remote` instance the user picked. + // C3b switches the JSX to a reduced composer when this is non-null + // and routes the submit through `useRemoteSpawnDispatch` instead of + // the cloud-agent `submitCreate` flow. + const [runOnInstance, setRunOnInstance] = useState(null); const [isCreating, setIsCreating] = useState(false); const [isSubmitting, setIsSubmitting] = useState(false); const [hasPrompt, setHasPrompt] = useState(false); const submissionLockRef = useRef(false); const voiceInputSettlerRef = useRef<(() => Promise) | null>(null); + // Org-scoped new-agent flows are Cloud-Agent only by design: a remote + // spawn creates a personal CLI session that the org route would not be + // able to attribute to the flow's organization (mobile's data model + // only loads CLI sessions on personal routes). Hide the entire + // "Run on" section when the flow is org-scoped. + const showRunOnSelector = shouldShowRunOnSelector(organizationId); + // ── Models ─────────────────────────────────────────────────────── const { models, @@ -100,6 +120,28 @@ export default function NewSessionScreen() { })); }, [repoData]); + // "Run on" instance list. Fetched at the screen level (not inside the + // picker) so the selector's value label and the picker's row list stay + // in sync without round-tripping through the bridge. The picker ALSO + // re-queries on focus + polls (per the spec), so this is a soft + // pre-population, not the source of truth. C3b also reuses the + // `refetch` for the retryable-spawn-failure recovery path. + const { + data: instancesData, + isLoading: isLoadingInstances, + refetch: refetchInstances, + } = useQuery({ + ...trpc.activeSessions.listInstances.queryOptions(undefined, { + refetchOnWindowFocus: true, + staleTime: 5000, + }), + enabled: showRunOnSelector, + }); + const instanceList: InstancePickerInstance[] = useMemo( + () => instancesData?.instances ?? [], + [instancesData] + ); + // ── Session creator ────────────────────────────────────────────── const { createSessionFromDraft, promptRef } = useNewSessionCreator({ attachments, @@ -111,6 +153,19 @@ export default function NewSessionScreen() { variant, }); + // ── Remote-instance spawn transport (kilo remote) ──────────────── + // C3b: dispatches the remote submit and owns the outcome -> UX + // (toast, refetch, selection reset, nav). See + // `@/components/agents/use-remote-spawn-dispatch` and + // `@/lib/remote-submit-outcome` for the contract. + const remoteSpawn = useRemoteSpawnDispatch({ + organizationId, + runOnInstance, + setRunOnInstance, + refetchInstances, + instanceList, + }); + // ── Handlers ───────────────────────────────────────────────────── const handleModelSelect = useCallback( (modelId: string, newVariant: string) => { @@ -153,90 +208,153 @@ export default function NewSessionScreen() { }); }, [createSessionFromDraft]); - const handleStartSession = useCallback(() => { - void submitCreate(); - }, [submitCreate]); - const { addCandidates } = attachments; const handleAddAttachment = useCallback(async () => { addCandidates(await pickAgentAttachments(showActionSheetWithOptions)); }, [addCandidates, showActionSheetWithOptions]); - const canCreate = - hasPrompt && - Boolean(selectedRepo) && - Boolean(model) && - !attachments.isUploading && - !attachments.hasFailedAttachments; + // Cloud-Agent vs. remote-instance submit safety. + // + // - Cloud Agent (`runOnInstance === null`): `isStartDisabled` is the + // full pre-C3a canCreate expression, byte-identical to today's + // contract — the cloud-agent submit path runs through + // `handleStartSession` -> `submitCreate` -> `createSessionFromDraft`. + // No change to that branch. + // - Remote target (`runOnInstance !== null`): the start button is + // gated only by `isSpawningRemote` so the user can re-press after + // a non-retryable / retryable failure. + const isRemoteTargetSelected = runOnInstance !== null; + + const isStartDisabled = isRemoteTargetSelected + ? remoteSpawn.isSpawningRemote + : resolveNewSessionSubmitDisabled({ + attachmentsHasFailed: attachments.hasFailedAttachments, + attachmentsIsUploading: attachments.isUploading, + hasPrompt, + isCreating, + isRemoteTargetSelected, + isSubmitting, + model, + selectedRepo, + }); + + const handleStartSession = useCallback(() => { + if (runOnInstance !== null) { + remoteSpawn.onStart(); + return; + } + void submitCreate(); + }, [remoteSpawn, runOnInstance, submitCreate]); + + // oxlint's `jsx-handler-names` rule requires the value of an + // `onX`-prefixed prop to start with `handle`. The dispatch hook's + // stable `onChangeRunOnInstance` reference is a closure over + // several pieces of state, so wrap the call here. + const handleRunOnInstanceChange = useCallback( + (next: InstancePickerInstance | null) => { + remoteSpawn.onChangeRunOnInstance(next); + }, + [remoteSpawn] + ); return ( - - { - void handleAddAttachment(); - }} - onRemoveAttachment={id => { - attachments.removeAttachment(id); - }} - onRetryAttachment={id => { - attachments.retryAttachment(id); - }} - onRefetchModels={() => { - void refetchModels(); - }} - voiceInputSettlerRef={voiceInputSettlerRef} + {isRemoteTargetSelected ? ( + + ) : ( + + { + void handleAddAttachment(); + }} + onRemoveAttachment={id => { + attachments.removeAttachment(id); + }} + onRetryAttachment={id => { + attachments.retryAttachment(id); + }} + onRefetchModels={() => { + void refetchModels(); + }} + voiceInputSettlerRef={voiceInputSettlerRef} + /> - { - void handleOpenGitHubIntegration(); - }} - onRefetch={() => { - void refetchRepos(); - }} - repositories={repositories} - showGitHubIntegrationPrompt={showGitHubIntegrationPrompt} - value={selectedRepo} - /> + { + void handleOpenGitHubIntegration(); + }} + onRefetch={() => { + void refetchRepos(); + }} + repositories={repositories} + showGitHubIntegrationPrompt={showGitHubIntegrationPrompt} + value={selectedRepo} + /> - - + {showRunOnSelector ? ( + + Run on + + {remoteSpawn.showInstanceDisconnectedNote ? ( + + {REMOTE_SPAWN_INSTANCE_DISCONNECTED_NOTE} + + ) : null} + + ) : null} + + + + )} ); } diff --git a/apps/mobile/src/components/agents/instance-selector.tsx b/apps/mobile/src/components/agents/instance-selector.tsx new file mode 100644 index 0000000000..f78a2bae65 --- /dev/null +++ b/apps/mobile/src/components/agents/instance-selector.tsx @@ -0,0 +1,92 @@ +import { type Href, useRouter } from 'expo-router'; +import { ChevronDown } from 'lucide-react-native'; +import { Pressable } from 'react-native'; + +import { Text } from '@/components/ui/text'; +import { useThemeColors } from '@/lib/hooks/use-theme-colors'; +import { type InstancePickerInstance, setInstancePickerBridge } from '@/lib/picker-bridge'; +import { cn } from '@/lib/utils'; + +type InstanceSelectorProps = { + /** + * `null` means the user has the default Cloud Agent target selected. + * Any non-null value is a live `kilo remote` CLI instance the picker + * chose. + */ + value: InstancePickerInstance | null; + /** + * The list the picker will open with. The route owns the data; the + * selector just hands it across the bridge. When the list is empty the + * selector still opens (the picker shows the empty state + Refresh). + */ + instances: InstancePickerInstance[]; + isLoading: boolean; + onChange: (value: InstancePickerInstance | null) => void; + disabled?: boolean; +}; + +function selectorLabel({ + value, + isLoading, +}: { + value: InstancePickerInstance | null; + isLoading: boolean; +}): string { + if (value) { + return `${value.name} · ${value.projectName}`; + } + if (isLoading) { + return 'Loading…'; + } + return 'Cloud Agent'; +} + +export function InstanceSelector({ + value, + instances, + isLoading, + onChange, + disabled = false, +}: Readonly) { + const router = useRouter(); + const colors = useThemeColors(); + + // The Cloud Agent default is always selectable, even when the list is + // loading or empty — only the "open the picker" path is gated. + const canOpenPicker = !disabled; + const label = selectorLabel({ value, isLoading }); + + function handlePress() { + if (!canOpenPicker) { + return; + } + setInstancePickerBridge({ + instances, + currentValue: value, + onSelect: onChange, + }); + router.push('/(app)/agent-chat/instance-picker' as Href); + } + + return ( + + + {label} + + + + ); +} diff --git a/apps/mobile/src/components/agents/remote-spawn-composer.tsx b/apps/mobile/src/components/agents/remote-spawn-composer.tsx new file mode 100644 index 0000000000..88c6acd9cb --- /dev/null +++ b/apps/mobile/src/components/agents/remote-spawn-composer.tsx @@ -0,0 +1,71 @@ +import { ActivityIndicator, ScrollView, View } from 'react-native'; + +import { InstanceSelector } from '@/components/agents/instance-selector'; +import { Button } from '@/components/ui/button'; +import { Text } from '@/components/ui/text'; +import { type InstancePickerInstance } from '@/lib/picker-bridge'; +import { useThemeColors } from '@/lib/hooks/use-theme-colors'; + +type RemoteSpawnComposerProps = { + runOnInstance: InstancePickerInstance | null; + instanceList: InstancePickerInstance[]; + isLoadingInstances: boolean; + onChangeRunOnInstance: (next: InstancePickerInstance | null) => void; + isSpawningRemote: boolean; + isStartDisabled: boolean; + onStart: () => void; +}; + +/** + * Reduced composer shown on `/(app)/agent-chat/new` when a + * `kilo remote` instance is selected. Per the C3b plan: + * model / mode / repo / attachment affordances and the prompt box + * are hidden; the "Run on" selector stays (so the user can switch + * back to Cloud Agent or pick a different instance) and a single + * "Start session" CTA drives the spawn. + * + * kilocode_change - the inline "disconnected" note lives in the FULL + * (Cloud Agent) composer in `new.tsx`, not here: a retryable spawn + * failure resets the selection to `null` in the SAME state update that + * sets the note, which immediately swaps the screen away from this + * component (`isRemoteTargetSelected` becomes `false`) — a note prop on + * this component could never actually render. + */ +export function RemoteSpawnComposer({ + runOnInstance, + instanceList, + isLoadingInstances, + onChangeRunOnInstance, + isSpawningRemote, + isStartDisabled, + onStart, +}: Readonly) { + const colors = useThemeColors(); + return ( + + + Run on + + + + + + ); +} diff --git a/apps/mobile/src/components/agents/session-detail-routes.test.ts b/apps/mobile/src/components/agents/session-detail-routes.test.ts index f28b8757fa..1366d834ad 100644 --- a/apps/mobile/src/components/agents/session-detail-routes.test.ts +++ b/apps/mobile/src/components/agents/session-detail-routes.test.ts @@ -1,47 +1,46 @@ +import { type KiloSessionId } from 'cloud-agent-sdk'; import { describe, expect, it, vi } from 'vitest'; import { getAgentSessionPath, + getSpawnedAgentSessionPath, replaceWithAgentSession, -} from '@/components/agents/session-detail-routes'; +} from './session-detail-routes'; -const SESSION_ID = 'ses_12345678901234567890123456'; +const SESSION_ID: KiloSessionId = 'ses_12345678901234567890123456' as KiloSessionId; const ORG_ID = 'org_abc123'; -describe('getAgentSessionPath', () => { - it('routes personal sessions to the canonical agent-chat detail screen', () => { - expect(getAgentSessionPath(SESSION_ID)).toBe(`/(app)/agent-chat/${SESSION_ID}`); +describe('getSpawnedAgentSessionPath', () => { + it('appends spawned=1 to the personal (no-org) path', () => { + expect(getSpawnedAgentSessionPath(SESSION_ID)).toBe( + `/(app)/agent-chat/${SESSION_ID}?spawned=1` + ); }); - it('preserves the organization context when provided', () => { - expect(getAgentSessionPath(SESSION_ID, ORG_ID)).toBe( - `/(app)/agent-chat/${SESSION_ID}?organizationId=${ORG_ID}` + it('joins spawned=1 with & when an organizationId is already on the path', () => { + expect(getSpawnedAgentSessionPath(SESSION_ID, ORG_ID)).toBe( + `/(app)/agent-chat/${SESSION_ID}?organizationId=${ORG_ID}&spawned=1` ); }); - it('treats an empty organizationId as the personal case', () => { - expect(getAgentSessionPath(SESSION_ID, '')).toBe(`/(app)/agent-chat/${SESSION_ID}`); + it('does not rewrite the base path returned by getAgentSessionPath', () => { + // Sanity: the base path is what we hand to the function; the helper + // must not rewrite it, only suffix it. + expect(getAgentSessionPath(SESSION_ID)).toBe(`/(app)/agent-chat/${SESSION_ID}`); + expect(getAgentSessionPath(SESSION_ID, ORG_ID)).toBe( + `/(app)/agent-chat/${SESSION_ID}?organizationId=${ORG_ID}` + ); }); }); describe('replaceWithAgentSession', () => { - it('replaces with the personal agent-chat route exactly once', () => { + it('still uses the base path (no spawned=1) — spawned param is owned by the spawn path', () => { + // Regression: the existing helper is unchanged. Spawned-aware callers + // go through `getSpawnedAgentSessionPath` directly so the non-spawn + // paths (push notifications, deep links, etc.) keep their original + // byte-for-byte navigation target. const router = { replace: vi.fn(() => undefined) }; - - replaceWithAgentSession(router, SESSION_ID); - - expect(router.replace).toHaveBeenCalledTimes(1); - expect(router.replace).toHaveBeenCalledWith(`/(app)/agent-chat/${SESSION_ID}`); - }); - - it('replaces with the org-scoped agent-chat route when organizationId is provided', () => { - const router = { replace: vi.fn(() => undefined) }; - replaceWithAgentSession(router, SESSION_ID, ORG_ID); - - expect(router.replace).toHaveBeenCalledTimes(1); - expect(router.replace).toHaveBeenCalledWith( - `/(app)/agent-chat/${SESSION_ID}?organizationId=${ORG_ID}` - ); + expect(router.replace).toHaveBeenCalledWith(getAgentSessionPath(SESSION_ID, ORG_ID)); }); }); diff --git a/apps/mobile/src/components/agents/session-detail-routes.ts b/apps/mobile/src/components/agents/session-detail-routes.ts index 9f3afd80cb..b0997b0c5f 100644 --- a/apps/mobile/src/components/agents/session-detail-routes.ts +++ b/apps/mobile/src/components/agents/session-detail-routes.ts @@ -29,3 +29,29 @@ export function replaceWithAgentSession( ): void { router.replace(getAgentSessionPath(kiloSessionId, organizationId)); } + +/** + * `Href` for the agent-chat detail when the parent route just spawned a + * remote `kilo remote` session. Appends `?spawned=1` to whatever + * `getAgentSessionPath` returns so the destination route can poll for + * the freshly-ingested session row with a short retry window — the + * parent's `Session.Event.Created` -> `IngestQueue` write is not + * synchronous with the mobile query's read, so the route needs to + * tolerate the transient NOT_FOUND that may show up before the row + * is queryable. + * + * The `spawned=1` suffix is intentionally append-only: it never + * replaces an existing query string. The optional `?organizationId=` + * already produced by `getAgentSessionPath` is preserved and `spawned=1` + * is joined with `&` in that case. + */ +export function getSpawnedAgentSessionPath(kiloSessionId: string, organizationId?: string): Href { + // `getAgentSessionPath` returns an `Href` (= `string | HrefObject`) + // but in this codebase every construction site uses the string + // branch. Narrow with `as string` so the `.includes('?')` check + // type-checks without forcing the helper to special-case + // HrefObject (which the rest of the app does not use). + const base = getAgentSessionPath(kiloSessionId, organizationId) as string; + const separator = base.includes('?') ? '&' : '?'; + return `${base}${separator}spawned=1` as Href; +} diff --git a/apps/mobile/src/components/agents/session-list-content.tsx b/apps/mobile/src/components/agents/session-list-content.tsx index 076cd6ea61..95cb1cd37e 100644 --- a/apps/mobile/src/components/agents/session-list-content.tsx +++ b/apps/mobile/src/components/agents/session-list-content.tsx @@ -222,7 +222,13 @@ export function AgentSessionListContent({ } return ( { onSessionPress(item.session.id, organizationIdBySessionId.get(item.session.id)); }} diff --git a/apps/mobile/src/components/agents/session-row.tsx b/apps/mobile/src/components/agents/session-row.tsx index 9938e5e4e0..76622b5abd 100644 --- a/apps/mobile/src/components/agents/session-row.tsx +++ b/apps/mobile/src/components/agents/session-row.tsx @@ -6,6 +6,7 @@ import { SessionRow } from '@/components/ui/session-row'; import { Text } from '@/components/ui/text'; import { type AgentSessionSortBy, getAgentSessionTimestamp } from '@/lib/agent-session-sort'; import { useThemeColors } from '@/lib/hooks/use-theme-colors'; +import { platformLabel } from '@/lib/platform-label'; import { parseTimestamp, timeAgo } from '@/lib/utils'; type StoredSessionRowProps = { @@ -38,36 +39,18 @@ type RemoteSessionRowProps = { title: string; status: string; gitBranch?: string; + /** + * Backend-reported platform for this live session (e.g. `'cli'` for a + * `kilo remote` connection, `'vscode'` for the extension). Legacy CLIs + * predating the platform field never report one; in that case the + * row falls back to the `'cloud-agent'` label so behavior is + * byte-identical to the previous hardcode. + */ + platform?: string; }; onPress: () => void; }; -/** - * Map backend `created_on_platform` strings to a pretty uppercase label - * for the row eyebrow. The row's hue is hashed from this label. - */ -function platformLabel(platform: string): string { - switch (platform) { - case 'cloud-agent': - case 'cloud-agent-web': { - return 'CLOUD AGENT'; - } - case 'vscode': - case 'agent-manager': { - return 'VSCODE'; - } - case 'slack': { - return 'SLACK'; - } - case 'cli': { - return 'CLI'; - } - default: { - return platform.toUpperCase(); - } - } -} - function formatMeta(timestamp: string): string { return timeAgo(parseTimestamp(timestamp)).toUpperCase(); } @@ -235,11 +218,17 @@ export function StoredSessionRow({ export function RemoteSessionRow({ session, onPress }: Readonly) { const title = session.title.length > 0 ? session.title : 'Untitled session'; + // Platform present → real label via the shared helper. Platform absent + // (legacy CLI) → fall through to `'cloud-agent'`, which the helper + // renders as the same "CLOUD AGENT" string the row hardcoded before + // this slice. Keeping the literal in the helper (not a magic constant + // here) means future per-platform tweaks still apply to the legacy case. + const agentLabel = platformLabel(session.platform ?? 'cloud-agent'); return ( Promise<{ + data: { instances: InstancePickerInstance[] } | undefined; +}>; + +type UseRemoteSpawnDispatchArgs = { + organizationId: string | undefined; + runOnInstance: InstancePickerInstance | null; + setRunOnInstance: (next: InstancePickerInstance | null) => void; + /** + * Existing `activeSessions.listInstances` query's `refetch`. The + * route already owns this query (it's what powers the selector's + * `instanceList`); we reuse it here so a retryable spawn failure + * refreshes the same source of truth the picker reads from. + */ + refetchInstances: InstancesRefetch; + /** + * The most recent list the route knows about. Used as the + * membership fallback if the refetch fails. + */ + instanceList: InstancePickerInstance[]; +}; + +type UseRemoteSpawnDispatchResult = { + /** + * `true` while the spawn hook has a request in flight. Mirrors + * `remoteSpawn.status.status === 'inFlight'`, surfaced for the + * route's "is the start button disabled?" check. + */ + isSpawningRemote: boolean; + /** + * `true` after a retryable spawn failure reset the selection + * because the previously-selected `connectionId` dropped off the + * refetched list. Drives the inline "disconnected" note under + * the selector. + */ + showInstanceDisconnectedNote: boolean; + /** + * `onStart` for the route's "Start session" CTA when a remote + * target is selected. No-op when the selection is `null` (the + * route should have routed the cloud-agent path through + * `submitCreate` instead, but the guard is defensive). + */ + onStart: () => void; + /** + * Called by the "Run on" selector when the user picks a new + * instance or switches back to Cloud Agent. Clears the inline + * "disconnected" note — the note is only meaningful while the + * selector is on the post-fallback default. + */ + onChangeRunOnInstance: (next: InstancePickerInstance | null) => void; +}; + +/** + * Wires `useRemoteInstanceSpawn` into the route's existing state and + * tRPC query so a remote-target submit becomes a single + * `onStart()` dispatch: + * + * - `ready` -> `router.replace` via `getSpawnedAgentSessionPath` + * - `retryable` -> toast + refetch the instance list + reset the + * selection to `null` if the selected + * `connectionId` dropped off + * - `nonRetryable` -> toast, no navigation, no refetch + * + * The outcome -> action mapping is in + * `@/lib/remote-submit-outcome` (pure, unit-tested). This hook is + * pure glue: it owns no product logic beyond the dispatch itself. + */ +export function useRemoteSpawnDispatch({ + organizationId, + runOnInstance, + setRunOnInstance, + refetchInstances, + instanceList, +}: UseRemoteSpawnDispatchArgs): UseRemoteSpawnDispatchResult { + const router = useRouter(); + const remoteSpawn: { + status: RemoteInstanceSpawnStatus; + spawn: (connectionId: string) => Promise; + } = useRemoteInstanceSpawn(); + const [showInstanceDisconnectedNote, setShowInstanceDisconnectedNote] = useState(false); + + // kilocode_change - `onStart`'s async tail (spawn + refetch + classify) + // outlives a single render; a plain closure over `runOnInstance` would + // only ever see the value from the render that started this dispatch, + // not whatever the user picks while it's still in flight (which can + // happen: `isSpawningRemote` already flips back to `false` as soon as + // `remoteSpawn.spawn()` resolves, well before the refetch+classify tail + // finishes). A ref always reflects the latest selection so the tail can + // check "is my selection still the current one?" against real current + // state, not a stale snapshot. + const runOnInstanceRef = useRef(runOnInstance); + useEffect(() => { + runOnInstanceRef.current = runOnInstance; + }, [runOnInstance]); + + const onStart = useCallback(() => { + if (runOnInstance === null) { + return; + } + const selectedConnectionId = runOnInstance.connectionId; + void (async () => { + const outcome = await remoteSpawn.spawn(selectedConnectionId); + if (outcome.status === 'ready') { + router.replace(getSpawnedAgentSessionPath(outcome.sessionID, organizationId)); + return; + } + if (outcome.status === 'nonRetryable') { + toast.error(REMOTE_SPAWN_NON_RETRYABLE_TOAST); + return; + } + // outcome.status === 'retryable': refetch the instance list and + // re-evaluate whether the previously-selected instance is still + // present. + toast.error(REMOTE_SPAWN_RETRYABLE_TOAST); + let refetchedInstances: InstancePickerInstance[] = instanceList; + try { + const result = await refetchInstances(); + refetchedInstances = result.data?.instances ?? instanceList; + } catch { + // Refetch failed; fall through with the last-known list. The + // mapping helper treats an empty list as "disconnected", which + // is the right conservative default for a network blip. + } + const action = resolveRemoteSubmitOutcome({ + outcome, + refetchedInstances, + selectedConnectionId, + }); + if (action.kind !== 'retryable') { + // Defensive: outcome.status === 'retryable' must produce a + // retryable action. If this ever changes we'll want to know. + return; + } + // kilocode_change - only apply the reset if the selection this + // dispatch was FOR is still the CURRENT one (read from the ref, not + // the closure-captured `runOnInstance` — see the ref's comment + // above). Without this check, a stale tail's reset could clobber a + // newer, unrelated selection the user already made. + if ( + action.shouldResetSelectionToCloudAgent && + runOnInstanceRef.current?.connectionId === selectedConnectionId + ) { + setRunOnInstance(null); + setShowInstanceDisconnectedNote(action.showInstanceDisconnectedNote); + } + })(); + }, [ + instanceList, + organizationId, + refetchInstances, + remoteSpawn, + router, + runOnInstance, + setRunOnInstance, + ]); + + const onChangeRunOnInstance = useCallback( + (next: InstancePickerInstance | null) => { + setRunOnInstance(next); + if (showInstanceDisconnectedNote) { + setShowInstanceDisconnectedNote(false); + } + }, + [setRunOnInstance, showInstanceDisconnectedNote] + ); + + return { + isSpawningRemote: remoteSpawn.status.status === 'inFlight', + showInstanceDisconnectedNote, + onStart, + onChangeRunOnInstance, + }; +} diff --git a/apps/mobile/src/components/agents/user-web-connection-provider.tsx b/apps/mobile/src/components/agents/user-web-connection-provider.tsx index a363fd52ab..f718a2bb09 100644 --- a/apps/mobile/src/components/agents/user-web-connection-provider.tsx +++ b/apps/mobile/src/components/agents/user-web-connection-provider.tsx @@ -1,5 +1,14 @@ import { createContext, type ReactNode, useContext, useEffect, useRef } from 'react'; -import { createUserWebConnection, type UserWebConnection } from 'cloud-agent-sdk'; +import { type UserWebConnection } from 'cloud-agent-sdk'; +// kilocode_change - K1/C2: `createUserWebConnection` must come from its +// narrow subpath, not the `cloud-agent-sdk` barrel. The barrel's index.ts +// also re-exports web-only transport code that imports a web-app `@/...` +// alias unresolved under the mobile app's own `@` alias — this previously +// went unnoticed because nothing under `apps/mobile` had a test that +// actually imported this provider (and thus the barrel) at runtime until +// the `kilo remote` spawn hook test did. See the matching +// vitest.config.ts aliases for the full explanation. +import { createUserWebConnection } from 'cloud-agent-sdk/user-web-connection'; import { SESSION_INGEST_WS_URL } from '@/lib/config'; import { createNativeUserWebConnectionLifecycleHooks } from '@/lib/user-web-connection-lifecycle'; diff --git a/apps/mobile/src/components/home/agent-sessions-section.tsx b/apps/mobile/src/components/home/agent-sessions-section.tsx index c3e448fe6e..6e926a25c6 100644 --- a/apps/mobile/src/components/home/agent-sessions-section.tsx +++ b/apps/mobile/src/components/home/agent-sessions-section.tsx @@ -13,38 +13,12 @@ import { type StoredSession, useAgentSessions, } from '@/lib/hooks/use-agent-sessions'; +import { platformLabel } from '@/lib/platform-label'; import { parseTimestamp, timeAgo } from '@/lib/utils'; const MAX_ROWS = 3; const CLOUD_AGENT_PLATFORMS = new Set(expandPlatformFilter(['cloud-agent'])); -/** - * Map backend `created_on_platform` strings to a pretty uppercase label. - * The row's hue is hashed from the label in `SessionRow`, so no agent - * key needs to be emitted here. - */ -function platformLabel(platform: string): string { - switch (platform) { - case 'cloud-agent': - case 'cloud-agent-web': { - return 'CLOUD AGENT'; - } - case 'vscode': - case 'agent-manager': { - return 'VSCODE'; - } - case 'slack': { - return 'SLACK'; - } - case 'cli': { - return 'CLI'; - } - default: { - return platform.toUpperCase(); - } - } -} - function repoNameFromGitUrl(gitUrl: string | null | undefined): string | null { if (!gitUrl) { return null; @@ -153,7 +127,12 @@ function storedSessionMeta(session: StoredSession): string { function activeSessionLabel(session: ActiveSession): string { const repo = repoNameFromGitUrl(session.gitUrl); - return repo ? repo.toUpperCase() : platformLabel('cloud-agent'); + // kilocode_change - K1/C3a: derive the fallback label from the live + // per-session `platform` heartbeat field instead of a hardcoded + // 'cloud-agent' guess, so a `kilo remote`-spawned session shows "CLI" + // (via platformLabel) instead of being mislabeled "CLOUD AGENT". Entries + // without `platform` (legacy senders) keep today's exact fallback. + return repo ? repo.toUpperCase() : platformLabel(session.platform ?? 'cloud-agent'); } function storedSessionLabel(session: StoredSession): string { diff --git a/apps/mobile/src/lib/hooks/remote-instance-spawn-classifier.ts b/apps/mobile/src/lib/hooks/remote-instance-spawn-classifier.ts new file mode 100644 index 0000000000..63452a5b6f --- /dev/null +++ b/apps/mobile/src/lib/hooks/remote-instance-spawn-classifier.ts @@ -0,0 +1,186 @@ +import { type KiloSessionId, type UserWebConnection } from 'cloud-agent-sdk'; +// kilocode_change - K1/C2: these two runtime imports must come from their +// narrow subpaths, not the `cloud-agent-sdk` barrel. The barrel's index.ts +// also re-exports web-only transport code (`cloud-agent-connection.ts` -> +// `cloud-agent-transport.ts`) that imports a web-app `@/...` alias unresolved +// under the mobile app's own `@` alias — see the matching vitest.config.ts +// aliases for the full explanation. `user-web-connection.ts` and +// `create-session.ts` have a self-contained import graph and are safe to +// load under a plain Node vitest environment (no React Native). +// +// This pure classifier/spawner logic is deliberately kept in its own module, +// separate from `use-remote-instance-spawn.ts`'s `useRemoteInstanceSpawn` +// hook: that hook also imports `useUserWebConnection` from +// `@/components/agents/user-web-connection-provider`, a `.tsx` provider that +// transitively imports React Native / Expo config modules containing Flow +// syntax the Node vitest environment cannot parse. Splitting keeps this +// file's pure functions testable "without a React renderer" (per the +// accepted plan) while the hook itself stays UI-only and untested here. +import { CommandDeliveredError, UserWebCommandError } from 'cloud-agent-sdk/user-web-connection'; +import { + createRemoteSessionOnConnection, + parseCreateSessionResponse, +} from 'cloud-agent-sdk/create-session'; + +/** + * Pure outcome classifier for the `create_session` reply (connection-scoped + * `kilo remote` process-per-session spawn flow). + * + * The classifier is intentionally pure and dependency-free so it can be unit + * tested without a React renderer. It collapses the matrix of resolved / + * rejected / delivered / non-delivered outcomes into the small set of states + * the caller needs: + * + * - `ready` — a fresh `KiloSessionId` was provisioned by the CLI + * - `retryable` — either a transport-level failure (timeout, destroyed + * connection, socket gone) OR the DO-emitted literal + * `'Session owner not found'`, which is semantically + * "the instance disconnected" and should follow the + * same recovery path as a transport failure + * - `nonRetryable` — anything else: a malformed response envelope, a + * delivered CLI string error (e.g. `'failed to create + * session'`), or any structured `UserWebCommandError` + * (including `CLI_UPGRADE_REQUIRED`) + * + * Note on intentionally-unreachable structured codes: relay-sourced codes + * that are semantically transient (`COMMAND_EXPIRED`, `PENDING_COMMAND_LIMIT`) + * are mapped to `nonRetryable` here because they are effectively unreachable + * for this flow: + * - The SDK's 30s client-side timeout fires before the DO's command TTL. + * - The pending-command cap is implausible for a single spawn. + * If a future DO timing change makes them reachable, the comment is the + * place to revisit — not a silent mislabel. + */ + +/** + * Exact-match constant for the DO's literal "instance disconnected" string + * (`UserConnectionDO.ts:735`). Special-cased to `retryable` because + * semantically the instance disconnected, which is the same recovery path + * as a transport failure. + */ +export const SESSION_OWNER_NOT_FOUND_LITERAL = 'Session owner not found'; + +export type CreateSessionOutcome = + | { status: 'ready'; sessionID: KiloSessionId } + | { status: 'retryable'; reason: string; cause: unknown } + | { status: 'nonRetryable'; reason: string; cause: unknown }; + +/** + * Classify the resolved-or-rejected outcome of `createRemoteSessionOnConnection` + * into the spawn hook's state space. + * + * The `cause` field preserves the original error for callers that want to + * surface or log it; `reason` is a short, user-safe string intended for UI. + */ +export function classifyCreateSessionResult( + result: PromiseSettledResult +): CreateSessionOutcome { + if (result.status === 'fulfilled') { + const parsed = parseCreateSessionResponse(result.value); + if (parsed.ok) { + return { status: 'ready', sessionID: parsed.kiloSessionId }; + } + return { + status: 'nonRetryable', + reason: 'unexpected response shape', + cause: result.value, + }; + } + + // result.status === 'rejected' + const cause: unknown = result.reason; + + // Structured relay error: keep `.code` available; the classifier still + // intentionally maps all such errors to `nonRetryable` (see header). + if (cause instanceof UserWebCommandError) { + return { + status: 'nonRetryable', + reason: cause.message || cause.code, + cause, + }; + } + + // Delivered bare-string error: special-case the DO's vanished-connection + // literal to `retryable` (see SESSION_OWNER_NOT_FOUND_LITERAL). + if (cause instanceof CommandDeliveredError) { + if (cause.message === SESSION_OWNER_NOT_FOUND_LITERAL) { + return { + status: 'retryable', + reason: SESSION_OWNER_NOT_FOUND_LITERAL, + cause, + }; + } + return { + status: 'nonRetryable', + reason: cause.message, + cause, + }; + } + + // Anything else (plain `Error` from timeout / destroyed connection / + // socket gone) is a transport failure: retryable. + return { + status: 'retryable', + reason: cause instanceof Error ? cause.message : 'transport failure', + cause, + }; +} + +// --------------------------------------------------------------------------- +// Spawner +// --------------------------------------------------------------------------- + +/** + * Stable per-spawner identity (UUID v4). Generated once at spawner creation. + * + * v1 does NOT use `creationKey` for server-side dedup — the relay/CLI has no + * idempotency layer for `create_session` and an existing connection's race + * is a real (but small) possibility. The key exists purely as a stable + * per-attempt identifier for in-hook bookkeeping and tests; do not build a + * dedupe layer on top of it without revisiting the contract. + */ +export type CreateSessionSpawner = { + readonly creationKey: string; + /** + * Attempt a `create_session` against the given CLI connection. Returns + * the classified outcome — never throws. + */ + spawn: (connectionId: string) => Promise; +}; + +function generateCreationKey(): string { + // Matches the existing repo convention (`use-agent-attachment-upload.ts`, + // `cloud-agent-runtime.ts`): call `crypto.randomUUID()` directly, no + // manual RFC 4122 fallback (which would need bitwise operators the repo + // lint config forbids). iOS/Android Hermes both expose it; this key is an + // opaque per-attempt bookkeeping identifier, never parsed as a UUID by + // anything, so a plain random fallback string is sufficient on the rare + // environment without it. + const cryptoApi = Reflect.get(globalThis, 'crypto') as { randomUUID?: () => string } | undefined; + if (cryptoApi && typeof cryptoApi.randomUUID === 'function') { + return cryptoApi.randomUUID(); + } + return `spawn-${Date.now()}-${Math.random().toString(36).slice(2)}`; +} + +/** + * Pure factory for a spawner. Created without React state so it can be + * tested in isolation; the hook wires it into a `useState`-backed status for + * UI consumption. + */ +export function createSessionSpawner( + connection: Pick +): CreateSessionSpawner { + const creationKey = generateCreationKey(); + return { + creationKey, + async spawn(connectionId) { + try { + const raw = await createRemoteSessionOnConnection(connection, connectionId); + return classifyCreateSessionResult({ status: 'fulfilled', value: raw }); + } catch (error) { + return classifyCreateSessionResult({ status: 'rejected', reason: error }); + } + }, + }; +} diff --git a/apps/mobile/src/lib/hooks/use-remote-instance-spawn.test.ts b/apps/mobile/src/lib/hooks/use-remote-instance-spawn.test.ts new file mode 100644 index 0000000000..10f9de3b87 --- /dev/null +++ b/apps/mobile/src/lib/hooks/use-remote-instance-spawn.test.ts @@ -0,0 +1,161 @@ +import { describe, expect, it } from 'vitest'; + +import { type KiloSessionId, type UserWebConnection } from 'cloud-agent-sdk'; +// kilocode_change - K1/C2: runtime imports via the narrow subpath alias — +// see the matching comment in remote-instance-spawn-classifier.ts and +// vitest.config.ts for why the full `cloud-agent-sdk` barrel is unsafe here. +import { CommandDeliveredError, UserWebCommandError } from 'cloud-agent-sdk/user-web-connection'; + +// kilocode_change - K1/C2: imported from the classifier module, not +// `use-remote-instance-spawn.ts` — that file's `useRemoteInstanceSpawn` hook +// pulls in `useUserWebConnection`, which transitively loads React +// Native/Expo modules containing Flow syntax the Node vitest environment +// cannot parse. See that file's header comment for the full explanation. +import { + classifyCreateSessionResult, + createSessionSpawner, + type CreateSessionSpawner, + SESSION_OWNER_NOT_FOUND_LITERAL, +} from './remote-instance-spawn-classifier'; + +const VALID_SESSION_ID = 'ses_12345678901234567890123456' as KiloSessionId; + +function makeConnection(impl: UserWebConnection['sendCommandToConnection']): UserWebConnection { + return { + sendCommandToConnection: impl, + } as unknown as UserWebConnection; +} + +describe('classifyCreateSessionResult', () => { + it('returns ready with the session id when a valid v1 envelope resolves', () => { + const result = classifyCreateSessionResult({ + status: 'fulfilled', + value: { protocolVersion: 1, sessionID: VALID_SESSION_ID }, + }); + expect(result).toEqual({ status: 'ready', sessionID: VALID_SESSION_ID }); + }); + + it('returns nonRetryable for a resolved-but-malformed response', () => { + const result = classifyCreateSessionResult({ + status: 'fulfilled', + value: { nope: true }, + }); + expect(result).toEqual({ + status: 'nonRetryable', + reason: 'unexpected response shape', + cause: { nope: true }, + }); + }); + + it('returns retryable when the DO emits the exact "Session owner not found" literal', () => { + const cause = new CommandDeliveredError(SESSION_OWNER_NOT_FOUND_LITERAL); + const result = classifyCreateSessionResult({ status: 'rejected', reason: cause }); + expect(result).toEqual({ + status: 'retryable', + reason: SESSION_OWNER_NOT_FOUND_LITERAL, + cause, + }); + }); + + it('returns nonRetryable for a delivered CLI string failure with a non-matching message', () => { + const cause = new CommandDeliveredError('failed to create session'); + const result = classifyCreateSessionResult({ status: 'rejected', reason: cause }); + expect(result).toEqual({ + status: 'nonRetryable', + reason: 'failed to create session', + cause, + }); + }); + + it('returns nonRetryable for a structured UserWebCommandError with CLI_UPGRADE_REQUIRED', () => { + const cause = new UserWebCommandError({ + code: 'CLI_UPGRADE_REQUIRED', + message: 'Creating remote sessions from mobile requires a newer Kilo CLI.', + }); + const result = classifyCreateSessionResult({ status: 'rejected', reason: cause }); + expect(result.status).toBe('nonRetryable'); + if (result.status === 'nonRetryable') { + expect(result.reason).toBe('Creating remote sessions from mobile requires a newer Kilo CLI.'); + expect(result.cause).toBe(cause); + } + }); + + it('returns nonRetryable for any other structured UserWebCommandError code', () => { + const cause = new UserWebCommandError({ + code: 'SESSION_OWNER_CHANGED', + message: 'Session owner changed', + }); + const result = classifyCreateSessionResult({ status: 'rejected', reason: cause }); + expect(result.status).toBe('nonRetryable'); + }); + + it('returns retryable for a non-delivered transport failure (plain Error)', () => { + const cause = new Error('Connection destroyed'); + const result = classifyCreateSessionResult({ status: 'rejected', reason: cause }); + expect(result).toEqual({ + status: 'retryable', + reason: 'Connection destroyed', + cause, + }); + }); + + it('returns retryable for a non-Error rejection (e.g. thrown string)', () => { + const result = classifyCreateSessionResult({ status: 'rejected', reason: 'weird' }); + expect(result).toEqual({ + status: 'retryable', + reason: 'transport failure', + cause: 'weird', + }); + }); +}); + +describe('createSessionSpawner', () => { + it('exposes a stable creationKey and a spawn function', () => { + const spawner: CreateSessionSpawner = createSessionSpawner( + // eslint-disable-next-line typescript-eslint/require-await -- no await needed; return value is the whole point + makeConnection(async () => undefined) + ); + expect(typeof spawner.creationKey).toBe('string'); + expect(spawner.creationKey.length).toBeGreaterThan(0); + expect(typeof spawner.spawn).toBe('function'); + }); + + it('generates a fresh creationKey per spawner instance', () => { + // eslint-disable-next-line typescript-eslint/require-await -- no await needed; return value is the whole point + const a = createSessionSpawner(makeConnection(async () => undefined)); + // eslint-disable-next-line typescript-eslint/require-await -- no await needed; return value is the whole point + const b = createSessionSpawner(makeConnection(async () => undefined)); + expect(a.creationKey).not.toBe(b.creationKey); + }); + + it('spawn returns ready when the SDK resolves a valid envelope', async () => { + const spawner = createSessionSpawner( + // eslint-disable-next-line typescript-eslint/require-await -- no await needed; return value is the whole point + makeConnection(async () => ({ protocolVersion: 1, sessionID: VALID_SESSION_ID })) + ); + const outcome = await spawner.spawn('cli-owner-1'); + expect(outcome).toEqual({ status: 'ready', sessionID: VALID_SESSION_ID }); + }); + + it('spawn wraps delivered bare-string errors via the classifier', async () => { + const spawner = createSessionSpawner( + // eslint-disable-next-line typescript-eslint/require-await -- no await needed; throw is the whole point + makeConnection(async () => { + throw new CommandDeliveredError('failed to create session'); + }) + ); + const outcome = await spawner.spawn('cli-owner-1'); + expect(outcome.status).toBe('nonRetryable'); + }); + + it('spawn returns retryable for a transport failure', async () => { + const spawner = createSessionSpawner( + // eslint-disable-next-line typescript-eslint/require-await -- no await needed; throw is the whole point + makeConnection(async () => { + throw new Error('Connection destroyed'); + }) + ); + const outcome = await spawner.spawn('cli-owner-1'); + expect(outcome.status).toBe('retryable'); + }); +}); diff --git a/apps/mobile/src/lib/hooks/use-remote-instance-spawn.ts b/apps/mobile/src/lib/hooks/use-remote-instance-spawn.ts new file mode 100644 index 0000000000..d965d87471 --- /dev/null +++ b/apps/mobile/src/lib/hooks/use-remote-instance-spawn.ts @@ -0,0 +1,55 @@ +import { useMemo, useState } from 'react'; +import { type KiloSessionId } from 'cloud-agent-sdk'; + +import { useUserWebConnection } from '@/components/agents/user-web-connection-provider'; + +// kilocode_change - K1/C2: the pure classifier/spawner logic lives in +// `remote-instance-spawn-classifier.ts`, a separate module with no React +// Native / Expo dependency, so it stays testable under a plain Node vitest +// environment (per the accepted plan: pure functions "testable without a +// React renderer"). This file's own `useUserWebConnection` import pulls in +// RN/Expo config transitively, which is why the split exists — see that +// file's header comment for the full explanation. +import { + type CreateSessionOutcome, + createSessionSpawner, +} from './remote-instance-spawn-classifier'; + +export type { CreateSessionOutcome }; + +export type RemoteInstanceSpawnStatus = + | { status: 'idle' } + | { status: 'inFlight' } + | ({ status: 'ready'; sessionID: KiloSessionId } & { + creationKey: string; + }) + | ({ status: 'retryable' | 'nonRetryable'; reason: string } & { + creationKey: string; + }); + +/** + * Thin React hook wrapper around `createSessionSpawner`. Holds the latest + * status in component state so UI can re-render on each attempt. The + * underlying SDK call is one-shot per `spawn()` call — no in-hook retry + * loop, no toast, no debouncing; the caller drives those. + */ +export function useRemoteInstanceSpawn(): { + status: RemoteInstanceSpawnStatus; + spawn: (connectionId: string) => Promise; +} { + const connection = useUserWebConnection(); + const [status, setStatus] = useState({ status: 'idle' }); + + // Re-create the spawner only when the connection reference changes + // (provider mounts once, so this is effectively a singleton). + const spawner = useMemo(() => createSessionSpawner(connection), [connection]); + + const spawn = async (connectionId: string): Promise => { + setStatus({ status: 'inFlight' }); + const outcome = await spawner.spawn(connectionId); + setStatus({ ...outcome, creationKey: spawner.creationKey }); + return outcome; + }; + + return { status, spawn }; +} diff --git a/apps/mobile/src/lib/instance-picker-rows.test.ts b/apps/mobile/src/lib/instance-picker-rows.test.ts new file mode 100644 index 0000000000..d87a053328 --- /dev/null +++ b/apps/mobile/src/lib/instance-picker-rows.test.ts @@ -0,0 +1,138 @@ +import { describe, expect, it } from 'vitest'; + +import { type InstancePickerInstance } from '@/lib/picker-bridge'; + +import { dedupeInstanceLabels, resolveInstancePickerViewState } from './instance-picker-rows'; + +function instance(overrides: Partial): InstancePickerInstance { + return { + connectionId: overrides.connectionId ?? 'conn-1', + name: overrides.name ?? 'laptop', + projectName: overrides.projectName ?? 'kilo', + version: overrides.version, + }; +} + +describe('dedupeInstanceLabels', () => { + it('returns an empty list for no instances', () => { + expect(dedupeInstanceLabels([])).toEqual([]); + }); + + it('does not stamp a suffix when every (name, projectName) pair is unique', () => { + const result = dedupeInstanceLabels([ + instance({ connectionId: 'a', name: 'laptop', projectName: 'kilo' }), + instance({ connectionId: 'b', name: 'desktop', projectName: 'kilo' }), + instance({ connectionId: 'c', name: 'laptop', projectName: 'cloud' }), + ]); + expect(result.map(row => row.dedupSuffix)).toEqual([null, null, null]); + expect(result.map(row => row.connectionId)).toEqual(['a', 'b', 'c']); + }); + + it('stamps both rows of a single duplicate pair with stable, distinct suffixes', () => { + const result = dedupeInstanceLabels([ + instance({ connectionId: 'conn-aaa', name: 'laptop', projectName: 'kilo' }), + instance({ connectionId: 'conn-bbb', name: 'laptop', projectName: 'kilo' }), + ]); + const suffixes = result.map(row => row.dedupSuffix); + expect(suffixes).not.toEqual([null, null]); + expect(new Set(suffixes).size).toBe(2); + // Stability: running it again must produce the same suffixes. + const again = dedupeInstanceLabels([ + instance({ connectionId: 'conn-aaa', name: 'laptop', projectName: 'kilo' }), + instance({ connectionId: 'conn-bbb', name: 'laptop', projectName: 'kilo' }), + ]); + expect(again.map(row => row.dedupSuffix)).toEqual(suffixes); + }); + + it('stamps every row of a 3+ way duplicate cluster', () => { + const result = dedupeInstanceLabels([ + instance({ connectionId: 'c-1', name: 'laptop', projectName: 'kilo' }), + instance({ connectionId: 'c-2', name: 'laptop', projectName: 'kilo' }), + instance({ connectionId: 'c-3', name: 'laptop', projectName: 'kilo' }), + instance({ connectionId: 'c-4', name: 'laptop', projectName: 'kilo' }), + ]); + const suffixes = result.map(row => row.dedupSuffix); + expect(suffixes.every(suffix => suffix !== null)).toBe(true); + // All four connectionIds produce distinct 6-char hex suffixes in + // practice; even one collision across four random strings would be + // extremely unlikely, so asserting full uniqueness guards against a + // regression that reuses a single hash. + expect(new Set(suffixes).size).toBe(4); + }); + + it('mixes stamped and unstamped rows correctly when some pairs duplicate and others do not', () => { + const result = dedupeInstanceLabels([ + instance({ connectionId: 'solo-1', name: 'desktop', projectName: 'kilo' }), + instance({ connectionId: 'dup-a', name: 'laptop', projectName: 'kilo' }), + instance({ connectionId: 'dup-b', name: 'laptop', projectName: 'kilo' }), + instance({ connectionId: 'solo-2', name: 'laptop', projectName: 'cloud' }), + ]); + const byConn = new Map(result.map(row => [row.connectionId, row.dedupSuffix])); + expect(byConn.get('solo-1')).toBeNull(); + expect(byConn.get('solo-2')).toBeNull(); + expect(byConn.get('dup-a')).not.toBeNull(); + expect(byConn.get('dup-b')).not.toBeNull(); + }); + + it('preserves input order', () => { + const input = [ + instance({ connectionId: 'a' }), + instance({ connectionId: 'b' }), + instance({ connectionId: 'c' }), + ]; + expect(dedupeInstanceLabels(input).map(row => row.connectionId)).toEqual(['a', 'b', 'c']); + }); +}); + +describe('resolveInstancePickerViewState', () => { + it('is "loading" whenever the query has never produced data, regardless of isError', () => { + expect( + resolveInstancePickerViewState({ isLoading: true, isError: false, instances: [] }) + ).toEqual({ + kind: 'loading', + }); + // isLoading takes priority — a query cannot be simultaneously "never + // produced data" and "produced an error response" in TanStack Query's + // own state machine, but the classifier's precedence must still favor + // loading defensively. + expect( + resolveInstancePickerViewState({ isLoading: true, isError: true, instances: [] }) + ).toEqual({ + kind: 'loading', + }); + }); + + it('is "error" — distinct from a successful empty response — when the query failed', () => { + const errorState = resolveInstancePickerViewState({ + isLoading: false, + isError: true, + instances: [], + }); + const emptyState = resolveInstancePickerViewState({ + isLoading: false, + isError: false, + instances: [], + }); + expect(errorState).toEqual({ kind: 'error' }); + expect(errorState.kind).not.toBe(emptyState.kind); + }); + + it('is "ready" with an empty instances array for a successful zero-instance response (the Empty state)', () => { + expect( + resolveInstancePickerViewState({ isLoading: false, isError: false, instances: [] }) + ).toEqual({ + kind: 'ready', + instances: [], + }); + }); + + it('is "ready" with the full instances array for a successful populated response (the Happy state)', () => { + const instances = [instance({ connectionId: 'a' }), instance({ connectionId: 'b' })]; + expect(resolveInstancePickerViewState({ isLoading: false, isError: false, instances })).toEqual( + { + kind: 'ready', + instances, + } + ); + }); +}); diff --git a/apps/mobile/src/lib/instance-picker-rows.ts b/apps/mobile/src/lib/instance-picker-rows.ts new file mode 100644 index 0000000000..9a2a15282d --- /dev/null +++ b/apps/mobile/src/lib/instance-picker-rows.ts @@ -0,0 +1,103 @@ +import { type InstancePickerInstance } from '@/lib/picker-bridge'; + +export type LabeledInstance = InstancePickerInstance & { + /** + * Short, `connectionId`-derived suffix appended to the row's visible label + * when its `(name, projectName)` pair is not unique in the list. The base + * (name + project) label is always shown first; the suffix only appears for + * duplicates. For unique rows this is `null` and the renderer omits it. + */ + dedupSuffix: string | null; +}; + +/** + * Pure: given a list of instances, return the same list with a `dedupSuffix` + * on every row that shares its `(name, projectName)` pair with at least one + * other row in the input. The suffix is a 6-character hex string derived + * from a deterministic, non-cryptographic hash of the `connectionId` (see + * `shortConnectionIdHash` below) — short, stable for the lifetime of the + * connection, and visually distinguishable without being a long UUID. + * + * Order is preserved. Rows are not grouped or de-duplicated; the picker + * renders one entry per live `connectionId` (an already-disconnected CLI + * that briefly left a duplicate in the poll before the worker cleans it up + * must remain visible, not silently collapsed). + */ +export function dedupeInstanceLabels(instances: InstancePickerInstance[]): LabeledInstance[] { + if (instances.length === 0) { + return []; + } + + // Count occurrences of (name, projectName) so we only stamp a suffix on + // rows that actually have a peer. O(n) with a Map keyed by the joined pair. + const pairCounts = new Map(); + for (const instance of instances) { + const key = `${instance.name}\u0000${instance.projectName}`; + pairCounts.set(key, (pairCounts.get(key) ?? 0) + 1); + } + + return instances.map(instance => { + const key = `${instance.name}\u0000${instance.projectName}`; + const isDuplicate = (pairCounts.get(key) ?? 0) > 1; + return { + ...instance, + dedupSuffix: isDuplicate ? shortConnectionIdHash(instance.connectionId) : null, + }; + }); +} + +/** + * Produce a 6-char hex suffix that: + * - is stable for a given `connectionId` (so the same row keeps the same + * suffix across polls) + * - is short enough to read at a glance + * - is derived purely from the connectionId (no UI-side state needed) + * + * `globalThis.crypto.subtle` is unavailable on Hermes, so we use a small + * multiplicative string hash instead (the same pattern as the existing + * deterministic-hue hash in `@/lib/agent-color.ts#agentColor` — no bitwise + * operators, repo lint forbids them). The suffix is purely a visual + * disambiguator, not a cryptographic identifier; this gives more than + * enough collision resistance for the at-most-a-handful of CLI instances a + * single user runs. + */ +function shortConnectionIdHash(connectionId: string): string { + let hash = 0; + for (let i = 0; i < connectionId.length; i += 1) { + const codePoint = connectionId.codePointAt(i) ?? 0; + hash = Math.trunc(hash * 31 + codePoint) % 2_147_483_647; + } + return Math.abs(hash).toString(16).padStart(6, '0').slice(0, 6); +} + +/** + * Pure classification of the instance picker's four feature states, per the + * accepted plan's matrix. Kept separate from `InstancePickerScreen`'s JSX so + * each state's trigger condition — and its distinctness from its + * neighbors — is unit-testable without mounting the screen: + * - `loading`: the query has never produced data (not the same as a + * successful empty response). + * - `error`: the query itself failed (retryable — Retry CTA). Distinct + * from `empty`, which is a *successful* zero-instance response. + * - `ready`: a successful response, `instances` may be an empty array + * (the caller renders the Refresh-CTA empty card in that case) or + * populated (rows + Check for the selected one). + */ +type InstancePickerViewState = + | { kind: 'loading' } + | { kind: 'error' } + | { kind: 'ready'; instances: InstancePickerInstance[] }; + +export function resolveInstancePickerViewState(input: { + isLoading: boolean; + isError: boolean; + instances: InstancePickerInstance[]; +}): InstancePickerViewState { + if (input.isLoading) { + return { kind: 'loading' }; + } + if (input.isError) { + return { kind: 'error' }; + } + return { kind: 'ready', instances: input.instances }; +} diff --git a/apps/mobile/src/lib/new-session-submit.test.ts b/apps/mobile/src/lib/new-session-submit.test.ts new file mode 100644 index 0000000000..47aebef14a --- /dev/null +++ b/apps/mobile/src/lib/new-session-submit.test.ts @@ -0,0 +1,124 @@ +import { describe, expect, it } from 'vitest'; + +import { + resolveNewSessionSubmitDisabled, + resolveNewSessionSubmitEnabled, +} from './new-session-submit'; + +// Inputs that satisfy every precondition. Used as a base and then +// mutated per-case to assert that exactly one input field flips the +// boolean. Keeps the matrix readable. +function validInput(overrides: Partial[0]> = {}) { + return { + attachmentsHasFailed: false, + attachmentsIsUploading: false, + hasPrompt: true, + isCreating: false, + isRemoteTargetSelected: false, + isSubmitting: false, + model: 'claude-opus-4-7', + selectedRepo: 'org/repo', + ...overrides, + }; +} + +describe('resolveNewSessionSubmitEnabled', () => { + // Cloud-Agent (default) regression. The shape of this expression must + // match the pre-C3a `canCreate && !isCreating && !isSubmitting` exactly + // when isRemoteTargetSelected is false — i.e. no new field changes the + // boolean, and the existing flags gate submit exactly as before. + describe('Cloud Agent (remote target NOT selected)', () => { + it('is enabled when every precondition is satisfied', () => { + expect(resolveNewSessionSubmitEnabled(validInput())).toBe(true); + }); + + it('is disabled when the prompt is empty (existing canCreate rule)', () => { + expect(resolveNewSessionSubmitEnabled(validInput({ hasPrompt: false }))).toBe(false); + }); + + it('is disabled when no repository is selected (existing canCreate rule)', () => { + expect(resolveNewSessionSubmitEnabled(validInput({ selectedRepo: '' }))).toBe(false); + }); + + it('is disabled when no model is selected (existing canCreate rule)', () => { + expect(resolveNewSessionSubmitEnabled(validInput({ model: '' }))).toBe(false); + }); + + it('is disabled while attachments are uploading (existing canCreate rule)', () => { + expect(resolveNewSessionSubmitEnabled(validInput({ attachmentsIsUploading: true }))).toBe( + false + ); + }); + + it('is disabled when any attachment has failed (existing canCreate rule)', () => { + expect(resolveNewSessionSubmitEnabled(validInput({ attachmentsHasFailed: true }))).toBe( + false + ); + }); + + it('is disabled while a create is in flight (existing isCreating rule)', () => { + expect(resolveNewSessionSubmitEnabled(validInput({ isCreating: true }))).toBe(false); + }); + + it('is disabled while a submit-settle is in flight (existing isSubmitting rule)', () => { + expect(resolveNewSessionSubmitEnabled(validInput({ isSubmitting: true }))).toBe(false); + }); + }); + + // Safety-critical: whenever a remote target is selected, submit MUST + // evaluate to disabled regardless of any other field's state. The + // actual remote-submit wiring is a later slice; until then the + // button stays inert so we cannot accidentally fire the cloud-agent + // path with the wrong target. + describe('Remote target selected', () => { + const remoteInput = validInput({ isRemoteTargetSelected: true }); + + it('is disabled even when every other field is satisfied', () => { + expect(resolveNewSessionSubmitEnabled(remoteInput)).toBe(false); + }); + + it('is disabled even when the prompt is empty', () => { + expect(resolveNewSessionSubmitEnabled({ ...remoteInput, hasPrompt: false })).toBe(false); + }); + + it('is disabled even when no repository is selected', () => { + expect(resolveNewSessionSubmitEnabled({ ...remoteInput, selectedRepo: '' })).toBe(false); + }); + + it('is disabled even when no model is selected', () => { + expect(resolveNewSessionSubmitEnabled({ ...remoteInput, model: '' })).toBe(false); + }); + + it('is disabled even while not creating / not submitting', () => { + // Same as the "satisfied" case but the negative is what we care + // about: no precondition unsticks the button. + expect( + resolveNewSessionSubmitEnabled({ + ...remoteInput, + isCreating: false, + isSubmitting: false, + }) + ).toBe(false); + }); + }); +}); + +describe('resolveNewSessionSubmitDisabled', () => { + it('is the boolean inverse of resolveNewSessionSubmitEnabled for every input', () => { + const samples: Parameters[0][] = [ + validInput(), + validInput({ hasPrompt: false }), + validInput({ selectedRepo: '' }), + validInput({ model: '' }), + validInput({ attachmentsIsUploading: true }), + validInput({ attachmentsHasFailed: true }), + validInput({ isCreating: true }), + validInput({ isSubmitting: true }), + validInput({ isRemoteTargetSelected: true }), + validInput({ isRemoteTargetSelected: true, hasPrompt: true, selectedRepo: 'r', model: 'm' }), + ]; + for (const sample of samples) { + expect(resolveNewSessionSubmitDisabled(sample)).toBe(!resolveNewSessionSubmitEnabled(sample)); + } + }); +}); diff --git a/apps/mobile/src/lib/new-session-submit.ts b/apps/mobile/src/lib/new-session-submit.ts new file mode 100644 index 0000000000..9075625e7b --- /dev/null +++ b/apps/mobile/src/lib/new-session-submit.ts @@ -0,0 +1,70 @@ +/** + * Pure boolean predicate for whether the "Start session" button on the + * new-agent screen may submit right now. Lives in `lib/` (not next to + * the route) so the regression test for the Wave 2 C3a slice can pin + * both halves of the contract without rendering anything: + * + * 1. The existing cloud-agent path is byte-identical to before this + * slice landed — `canCreate` is computed with the exact same + * expression, and `isCloudAgentTargetSelected` defaults to `true` + * because the route's `runOnInstance` state defaults to `null`. + * 2. Whenever a remote target is selected, the predicate evaluates + * to `false` REGARDLESS of the rest of the input. The actual + * remote submit wiring is a later slice (C3b); until then the + * start button must stay inert so we cannot accidentally fire + * the cloud-agent path with the wrong target. + */ +export function resolveNewSessionSubmitEnabled({ + attachmentsHasFailed, + attachmentsIsUploading, + hasPrompt, + isCreating, + isRemoteTargetSelected, + isSubmitting, + model, + selectedRepo, +}: { + attachmentsHasFailed: boolean; + attachmentsIsUploading: boolean; + hasPrompt: boolean; + isCreating: boolean; + isRemoteTargetSelected: boolean; + isSubmitting: boolean; + model: string; + selectedRepo: string; +}): boolean { + const canCreate = + hasPrompt && + Boolean(selectedRepo) && + Boolean(model) && + !attachmentsIsUploading && + !attachmentsHasFailed; + + // The remote-target case short-circuits the entire cloud-agent + // submission path, not just `canCreate`. The Start button's `disabled` + // prop is the OR of every blocking flag, so a single boolean here is + // sufficient to gate both the button and any future submit call site + // (e.g. a submit-on-enter handler C3b might add). + if (isRemoteTargetSelected) { + return false; + } + + return canCreate && !isCreating && !isSubmitting; +} + +/** + * Inverse of `resolveNewSessionSubmitEnabled`, kept here so callers (and + * tests) don't have to negate the condition at the call site. + */ +export function resolveNewSessionSubmitDisabled(input: { + attachmentsHasFailed: boolean; + attachmentsIsUploading: boolean; + hasPrompt: boolean; + isCreating: boolean; + isRemoteTargetSelected: boolean; + isSubmitting: boolean; + model: string; + selectedRepo: string; +}): boolean { + return !resolveNewSessionSubmitEnabled(input); +} diff --git a/apps/mobile/src/lib/picker-bridge.ts b/apps/mobile/src/lib/picker-bridge.ts index e9bc7c6a1a..75443dcc50 100644 --- a/apps/mobile/src/lib/picker-bridge.ts +++ b/apps/mobile/src/lib/picker-bridge.ts @@ -50,9 +50,28 @@ type RepoPickerBridge = { onSelect: (repo: string) => void; }; +/** + * One row inside the "Run on" instance picker. Identical to the tRPC + * `activeSessions.listInstances` output shape so the bridge can hand the + * server payload straight to the picker without re-derivation. + */ +export type InstancePickerInstance = { + connectionId: string; + name: string; + projectName: string; + version?: string; +}; + +type InstancePickerBridge = { + instances: InstancePickerInstance[]; + currentValue: InstancePickerInstance | null; + onSelect: (instance: InstancePickerInstance | null) => void; +}; + let modelBridge: ModelPickerBridge | null = null; let modeBridge: ModePickerBridge | null = null; let repoBridge: RepoPickerBridge | null = null; +let instanceBridge: InstancePickerBridge | null = null; export function resolveModelPickerSelection( bridge: ModelPickerBridge, @@ -117,3 +136,13 @@ export function getRepoPickerBridge() { export function clearRepoPickerBridge() { repoBridge = null; } + +export function setInstancePickerBridge(bridge: InstancePickerBridge) { + instanceBridge = bridge; +} +export function getInstancePickerBridge() { + return instanceBridge; +} +export function clearInstancePickerBridge() { + instanceBridge = null; +} diff --git a/apps/mobile/src/lib/platform-label.test.ts b/apps/mobile/src/lib/platform-label.test.ts new file mode 100644 index 0000000000..26ced8e28a --- /dev/null +++ b/apps/mobile/src/lib/platform-label.test.ts @@ -0,0 +1,27 @@ +import { describe, expect, it } from 'vitest'; + +import { platformLabel } from '@/lib/platform-label'; + +describe('platformLabel', () => { + it('maps cloud-agent and cloud-agent-web to CLOUD AGENT', () => { + expect(platformLabel('cloud-agent')).toBe('CLOUD AGENT'); + expect(platformLabel('cloud-agent-web')).toBe('CLOUD AGENT'); + }); + + it('maps vscode and agent-manager to VSCODE', () => { + expect(platformLabel('vscode')).toBe('VSCODE'); + expect(platformLabel('agent-manager')).toBe('VSCODE'); + }); + + it('maps slack to SLACK', () => { + expect(platformLabel('slack')).toBe('SLACK'); + }); + + it('maps cli to CLI — the kilo remote spawn label fix', () => { + expect(platformLabel('cli')).toBe('CLI'); + }); + + it('falls back to an uppercased passthrough for unknown platforms', () => { + expect(platformLabel('some-future-platform')).toBe('SOME-FUTURE-PLATFORM'); + }); +}); diff --git a/apps/mobile/src/lib/platform-label.ts b/apps/mobile/src/lib/platform-label.ts new file mode 100644 index 0000000000..f379155eff --- /dev/null +++ b/apps/mobile/src/lib/platform-label.ts @@ -0,0 +1,28 @@ +// kilocode_change - new file +// K1/C3a: single source of truth for mapping a backend platform string +// (`created_on_platform` / the live heartbeat's per-session `platform` +// field) to a pretty uppercase label. Previously duplicated identically in +// `session-row.tsx` and `agent-sessions-section.tsx`; centralizing here +// means the `kilo remote` label fix ("CLI" instead of a hardcoded "CLOUD +// AGENT") can never drift between the two sites. +export function platformLabel(platform: string): string { + switch (platform) { + case 'cloud-agent': + case 'cloud-agent-web': { + return 'CLOUD AGENT'; + } + case 'vscode': + case 'agent-manager': { + return 'VSCODE'; + } + case 'slack': { + return 'SLACK'; + } + case 'cli': { + return 'CLI'; + } + default: { + return platform.toUpperCase(); + } + } +} diff --git a/apps/mobile/src/lib/remote-submit-outcome.test.ts b/apps/mobile/src/lib/remote-submit-outcome.test.ts new file mode 100644 index 0000000000..ccd0672245 --- /dev/null +++ b/apps/mobile/src/lib/remote-submit-outcome.test.ts @@ -0,0 +1,150 @@ +import { type KiloSessionId } from 'cloud-agent-sdk'; +import { describe, expect, it } from 'vitest'; + +import { type InstancePickerInstance } from '@/lib/picker-bridge'; + +import { + REMOTE_SPAWN_INSTANCE_DISCONNECTED_NOTE, + REMOTE_SPAWN_NON_RETRYABLE_TOAST, + REMOTE_SPAWN_RETRYABLE_TOAST, + type RemoteSubmitOutcomeAction, + resolveRemoteSubmitOutcome, +} from './remote-submit-outcome'; + +const SESSION_ID: KiloSessionId = 'ses_12345678901234567890123456' as KiloSessionId; +const CONNECTION_ID = 'conn-abc123'; + +function instance(connectionId: string): InstancePickerInstance { + return { connectionId, name: 'laptop', projectName: 'kilo' }; +} + +describe('resolveRemoteSubmitOutcome', () => { + describe('ready outcome', () => { + it('returns a navigate action with the sessionID', () => { + const result: RemoteSubmitOutcomeAction = resolveRemoteSubmitOutcome({ + outcome: { status: 'ready', sessionID: SESSION_ID }, + refetchedInstances: [], + selectedConnectionId: CONNECTION_ID, + }); + expect(result).toEqual({ kind: 'navigate', sessionID: SESSION_ID }); + }); + + it('ignores the refetchedInstances and selectedConnectionId', () => { + const result = resolveRemoteSubmitOutcome({ + outcome: { status: 'ready', sessionID: SESSION_ID }, + refetchedInstances: [instance(CONNECTION_ID)], + selectedConnectionId: CONNECTION_ID, + }); + expect(result.kind).toBe('navigate'); + }); + }); + + describe('retryable outcome', () => { + it('returns a retryable action with the fixed toast copy', () => { + const result = resolveRemoteSubmitOutcome({ + outcome: { status: 'retryable', reason: 'timeout', cause: new Error('timeout') }, + refetchedInstances: [instance(CONNECTION_ID)], + selectedConnectionId: CONNECTION_ID, + }); + expect(result.kind).toBe('retryable'); + if (result.kind === 'retryable') { + expect(result.toast).toBe(REMOTE_SPAWN_RETRYABLE_TOAST); + expect(result.shouldRefetchInstances).toBe(true); + } + }); + + it('sets shouldResetSelectionToCloudAgent to false when the connectionId is still present', () => { + const result = resolveRemoteSubmitOutcome({ + outcome: { status: 'retryable', reason: 'timeout', cause: new Error('timeout') }, + refetchedInstances: [instance(CONNECTION_ID), instance('conn-other')], + selectedConnectionId: CONNECTION_ID, + }); + expect(result.kind).toBe('retryable'); + if (result.kind === 'retryable') { + expect(result.shouldResetSelectionToCloudAgent).toBe(false); + expect(result.showInstanceDisconnectedNote).toBe(false); + } + }); + + it('sets shouldResetSelectionToCloudAgent to true when the connectionId is gone from the list', () => { + const result = resolveRemoteSubmitOutcome({ + outcome: { status: 'retryable', reason: 'timeout', cause: new Error('timeout') }, + refetchedInstances: [instance('conn-other')], + selectedConnectionId: CONNECTION_ID, + }); + expect(result.kind).toBe('retryable'); + if (result.kind === 'retryable') { + expect(result.shouldResetSelectionToCloudAgent).toBe(true); + expect(result.showInstanceDisconnectedNote).toBe(true); + } + }); + + it('sets shouldResetSelectionToCloudAgent to true when the list is empty', () => { + const result = resolveRemoteSubmitOutcome({ + outcome: { status: 'retryable', reason: 'timeout', cause: new Error('timeout') }, + refetchedInstances: [], + selectedConnectionId: CONNECTION_ID, + }); + expect(result.kind).toBe('retryable'); + if (result.kind === 'retryable') { + expect(result.shouldResetSelectionToCloudAgent).toBe(true); + expect(result.showInstanceDisconnectedNote).toBe(true); + } + }); + + it('sets shouldResetSelectionToCloudAgent to true when selectedConnectionId is null', () => { + // Edge case: if the user somehow triggered a spawn with no + // selection (shouldn't happen, but defensive), we reset to null + // (no-op) and show the note. + const result = resolveRemoteSubmitOutcome({ + outcome: { status: 'retryable', reason: 'timeout', cause: new Error('timeout') }, + refetchedInstances: [instance(CONNECTION_ID)], + selectedConnectionId: null, + }); + expect(result.kind).toBe('retryable'); + if (result.kind === 'retryable') { + expect(result.shouldResetSelectionToCloudAgent).toBe(true); + expect(result.showInstanceDisconnectedNote).toBe(true); + } + }); + }); + + describe('nonRetryable outcome', () => { + it('returns a nonRetryable action with the fixed toast copy', () => { + const result = resolveRemoteSubmitOutcome({ + outcome: { + status: 'nonRetryable', + reason: 'CLI_UPGRADE_REQUIRED', + cause: new Error('CLI_UPGRADE_REQUIRED'), + }, + refetchedInstances: [instance(CONNECTION_ID)], + selectedConnectionId: CONNECTION_ID, + }); + expect(result).toEqual({ + kind: 'nonRetryable', + toast: REMOTE_SPAWN_NON_RETRYABLE_TOAST, + }); + }); + + it('ignores the refetchedInstances and selectedConnectionId', () => { + const result = resolveRemoteSubmitOutcome({ + outcome: { + status: 'nonRetryable', + reason: 'CLI_UPGRADE_REQUIRED', + cause: new Error('CLI_UPGRADE_REQUIRED'), + }, + refetchedInstances: [], + selectedConnectionId: null, + }); + expect(result.kind).toBe('nonRetryable'); + }); + }); + + describe('toast copy constants', () => { + it('exports the instance-disconnected note as a constant', () => { + expect(REMOTE_SPAWN_INSTANCE_DISCONNECTED_NOTE).toBe( + 'The selected instance disconnected. Start a session on Cloud Agent or pick another instance.' + ); + }); + }); +}); diff --git a/apps/mobile/src/lib/remote-submit-outcome.ts b/apps/mobile/src/lib/remote-submit-outcome.ts new file mode 100644 index 0000000000..4edca0d927 --- /dev/null +++ b/apps/mobile/src/lib/remote-submit-outcome.ts @@ -0,0 +1,139 @@ +import { type KiloSessionId } from 'cloud-agent-sdk'; + +import { type InstancePickerInstance } from '@/lib/picker-bridge'; +import { type CreateSessionOutcome } from '@/lib/hooks/use-remote-instance-spawn'; + +/** + * The fixed string copy surfaced in the toast when the spawn hook returns + * a `retryable` outcome. The phrasing is intentionally a soft + * "may have disconnected" hint: the actual cause is opaque (transport + * timeout, destroyed socket, or the DO's `Session owner not found` + * literal) and the recovery path is the same — refetch the instance + * list and re-evaluate. + */ +export const REMOTE_SPAWN_RETRYABLE_TOAST = + 'Couldn’t reach the instance — it may have disconnected.'; + +/** + * The fixed string copy surfaced in the toast when the spawn hook returns + * a `nonRetryable` outcome. Distinct from the retryable case so the user + * understands the situation is not a transient network blip: the CLI + * refused the spawn for a structural reason (malformed envelope, + * `CLI_UPGRADE_REQUIRED`, etc.). + */ +export const REMOTE_SPAWN_NON_RETRYABLE_TOAST = + 'The instance failed to start the session — check the machine or update the CLI.'; + +/** + * Inline note shown under the "Run on" selector when a retryable spawn + * failure left the previously-selected `connectionId` no longer present + * in the refetched list. The selector itself has fallen back to + * "Cloud Agent" (the `null` value), so the note explains why the + * selection changed. + */ +export const REMOTE_SPAWN_INSTANCE_DISCONNECTED_NOTE = + 'The selected instance disconnected. Start a session on Cloud Agent or pick another instance.'; + +export type RemoteSubmitOutcomeAction = + | { + kind: 'navigate'; + /** + * The `KiloSessionId` of the freshly-spawned session. Caller + * navigates through `getSpawnedAgentSessionPath` so the + * `spawned=1` readiness retry kicks in. + */ + sessionID: KiloSessionId; + } + | { + kind: 'retryable'; + toast: string; + /** + * The caller must refetch the instance list (the same + * `activeSessions.listInstances` query powering the selector) + * before re-evaluating `runOnInstance`. Always `true` for the + * retryable branch — the whole point of this case is "the + * instance may have just disconnected, ask the server again". + */ + shouldRefetchInstances: true; + /** + * When `true`, the caller should clear `runOnInstance` because + * the previously-selected `connectionId` is no longer in the + * refetched list. The caller computes this against the + * refetched list; we pre-compute the answer here so the + * consumer is a single dispatch (no double-check at the call + * site, no risk of disagreement between the toast copy and + * the reset). + */ + shouldResetSelectionToCloudAgent: boolean; + /** + * Tied to `shouldResetSelectionToCloudAgent`: the inline + * "disconnected" note under the selector only appears when we + * actually moved the selection away. Re-asserted here so a + * caller that wants to display the note independently (e.g. + * an analytics event) doesn't have to duplicate the + * membership check. + */ + showInstanceDisconnectedNote: boolean; + } + | { + kind: 'nonRetryable'; + toast: string; + /** + * Caller must NOT navigate, refetch, or reset the selection. + * The existing "Start session" button is the only re-entry + * affordance, matching the plan's non-retryable UX matrix. + */ + }; +/** + * Pure: map a `CreateSessionOutcome` (already classified by + * `useRemoteInstanceSpawn`) to the exact UX actions the new-agent + * screen must dispatch. The caller is responsible for the side + * effects (toast, `refetch()`, `setRunOnInstance(null)`, + * `router.replace`). + * + * The membership check (`is the previously-selected connectionId + * still in the refetched list?`) runs HERE, against the + * `selectedConnectionId` + `refetchedInstances` pair the caller hands + * in. That's the only place that knows both — the hook's + * `CreateSessionOutcome` does not carry the connectionId (it carries + * the per-spawner `creationKey`, which is a different opaque + * identifier). Keeping the check in this helper means the contract + * is testable in plain Node and the call site is a single dispatch. + * + * On `ready` / `nonRetryable` the membership inputs are ignored. + */ +export function resolveRemoteSubmitOutcome({ + outcome, + refetchedInstances, + selectedConnectionId, +}: { + outcome: CreateSessionOutcome; + refetchedInstances: InstancePickerInstance[]; + /** + * The `connectionId` of the instance the user had selected at the + * moment the spawn call started. Required for the retryable + * branch (where we use it to decide whether to reset + * `runOnInstance` to `null`); ignored for `ready` / + * `nonRetryable`. + */ + selectedConnectionId: string | null; +}): RemoteSubmitOutcomeAction { + if (outcome.status === 'ready') { + return { kind: 'navigate', sessionID: outcome.sessionID }; + } + if (outcome.status === 'nonRetryable') { + return { kind: 'nonRetryable', toast: REMOTE_SPAWN_NON_RETRYABLE_TOAST }; + } + // outcome.status === 'retryable' + const stillPresent = + selectedConnectionId !== null && + refetchedInstances.some(instance => instance.connectionId === selectedConnectionId); + const shouldReset = !stillPresent; + return { + kind: 'retryable', + toast: REMOTE_SPAWN_RETRYABLE_TOAST, + shouldRefetchInstances: true, + shouldResetSelectionToCloudAgent: shouldReset, + showInstanceDisconnectedNote: shouldReset, + }; +} diff --git a/apps/mobile/src/lib/should-show-run-on-selector.test.ts b/apps/mobile/src/lib/should-show-run-on-selector.test.ts new file mode 100644 index 0000000000..bea361b577 --- /dev/null +++ b/apps/mobile/src/lib/should-show-run-on-selector.test.ts @@ -0,0 +1,21 @@ +import { describe, expect, it } from 'vitest'; + +import { shouldShowRunOnSelector } from './should-show-run-on-selector'; + +describe('shouldShowRunOnSelector', () => { + it('shows the selector on a personal flow (no organizationId)', () => { + expect(shouldShowRunOnSelector(undefined)).toBe(true); + }); + + it('hides the selector on an org-scoped flow (organizationId present)', () => { + expect(shouldShowRunOnSelector('org-123')).toBe(false); + }); + + it('treats an empty-string organizationId as still-scoped (defensive)', () => { + // `useLocalSearchParams` returns strings, not null, so an empty string + // is a real possibility. We treat it as scoped (hidden) — the route + // never pushes an empty value intentionally, but if one arrives we + // must not silently fall through to the personal path. + expect(shouldShowRunOnSelector('')).toBe(false); + }); +}); diff --git a/apps/mobile/src/lib/should-show-run-on-selector.ts b/apps/mobile/src/lib/should-show-run-on-selector.ts new file mode 100644 index 0000000000..4f94184964 --- /dev/null +++ b/apps/mobile/src/lib/should-show-run-on-selector.ts @@ -0,0 +1,14 @@ +/** + * Whether the new-agent screen should show the "Run on" instance selector. + * + * Org-scoped flows (where the route param `organizationId` is present) are + * Cloud-Agent only by design: a remote `kilo remote` instance spawns a + * personal CLI session that mobile's data model can only surface on + * personal routes. Offering a personal-instance picker inside an org flow + * would create sessions invisible in the org's context, so the row is + * hidden entirely — this is not a feature state, it's an absent-by-design + * UI branch. + */ +export function shouldShowRunOnSelector(organizationId: string | undefined): boolean { + return organizationId === undefined; +} diff --git a/apps/mobile/src/lib/spawned-not-found-retry.test.ts b/apps/mobile/src/lib/spawned-not-found-retry.test.ts new file mode 100644 index 0000000000..db54378e09 --- /dev/null +++ b/apps/mobile/src/lib/spawned-not-found-retry.test.ts @@ -0,0 +1,107 @@ +import { describe, expect, it } from 'vitest'; + +import { + shouldRetryNotFoundOnSpawnedRoute, + SPAWNED_NOT_FOUND_MAX_ATTEMPTS, +} from './spawned-not-found-retry'; + +describe('shouldRetryNotFoundOnSpawnedRoute', () => { + describe('spawned param absent (regression: byte-identical to pre-C3b behavior)', () => { + it('never retries on NOT_FOUND when spawned is undefined', () => { + expect( + shouldRetryNotFoundOnSpawnedRoute({ + spawned: undefined, + attempt: 0, + errorCode: 'NOT_FOUND', + }) + ).toBe(false); + }); + + it('never retries on a non-NOT_FOUND error when spawned is undefined', () => { + expect( + shouldRetryNotFoundOnSpawnedRoute({ + spawned: undefined, + attempt: 0, + errorCode: 'INTERNAL_SERVER_ERROR', + }) + ).toBe(false); + }); + + it('never retries even after multiple attempts when spawned is undefined', () => { + // Belt-and-braces: a deeply-attempted absent-spawned retry must + // still be false, so the route's existing `retry: false` contract + // holds byte-for-byte. + expect( + shouldRetryNotFoundOnSpawnedRoute({ + spawned: undefined, + attempt: 99, + errorCode: 'NOT_FOUND', + }) + ).toBe(false); + }); + }); + + describe('spawned param present', () => { + it('retries NOT_FOUND on the first attempt', () => { + expect( + shouldRetryNotFoundOnSpawnedRoute({ spawned: '1', attempt: 0, errorCode: 'NOT_FOUND' }) + ).toBe(true); + }); + + it('retries NOT_FOUND while attempt is below the ceiling', () => { + // 0-indexed: attempt N is the (N+1)-th failure. We retry attempts + // 0..(MAX-1) inclusive; MAX is the first attempt that does NOT + // retry (8 tries have already happened by then). + for (let attempt = 0; attempt < SPAWNED_NOT_FOUND_MAX_ATTEMPTS; attempt += 1) { + expect( + shouldRetryNotFoundOnSpawnedRoute({ spawned: '1', attempt, errorCode: 'NOT_FOUND' }) + ).toBe(true); + } + }); + + it('stops retrying once the attempt count reaches the ceiling', () => { + expect( + shouldRetryNotFoundOnSpawnedRoute({ + spawned: '1', + attempt: SPAWNED_NOT_FOUND_MAX_ATTEMPTS, + errorCode: 'NOT_FOUND', + }) + ).toBe(false); + expect( + shouldRetryNotFoundOnSpawnedRoute({ + spawned: '1', + attempt: SPAWNED_NOT_FOUND_MAX_ATTEMPTS + 5, + errorCode: 'NOT_FOUND', + }) + ).toBe(false); + }); + + it('does not apply spawned-retry logic to a non-NOT_FOUND error', () => { + // The route's QueryError UI handles non-NOT_FOUND errors with + // its own Retry CTA. Our retry must not ALSO retry those — that + // would either double-fire or change today's behavior. + expect( + shouldRetryNotFoundOnSpawnedRoute({ + spawned: '1', + attempt: 0, + errorCode: 'INTERNAL_SERVER_ERROR', + }) + ).toBe(false); + expect( + shouldRetryNotFoundOnSpawnedRoute({ spawned: '1', attempt: 0, errorCode: undefined }) + ).toBe(false); + }); + + it('treats any non-undefined spawned value (e.g. "0", "true") the same as "1"', () => { + // The route only appends the literal "1" today, but the predicate + // is presence-based: we only care that the spawn-path navigation + // produced SOME value here, not its specific shape. + expect( + shouldRetryNotFoundOnSpawnedRoute({ spawned: '0', attempt: 0, errorCode: 'NOT_FOUND' }) + ).toBe(true); + expect( + shouldRetryNotFoundOnSpawnedRoute({ spawned: 'true', attempt: 0, errorCode: 'NOT_FOUND' }) + ).toBe(true); + }); + }); +}); diff --git a/apps/mobile/src/lib/spawned-not-found-retry.ts b/apps/mobile/src/lib/spawned-not-found-retry.ts new file mode 100644 index 0000000000..c5a25a62f2 --- /dev/null +++ b/apps/mobile/src/lib/spawned-not-found-retry.ts @@ -0,0 +1,59 @@ +/** + * Pure predicate driving the TanStack Query `retry` callback for the + * `cliSessionsV2.get` read on the `/(app)/agent-chat/[session-id]` route. + * + * The retry only matters for the freshly-spawned `kilo remote` happy path: + * the parent ingest row is written by the parent's own + * `Session.Event.Created` -> `IngestQueue` flow, which is NOT synchronous + * with the mobile client's read. Without a short retry, the very first + * query tick after `router.replace` lands on a row that hasn't been + * ingested yet, surfaces a transient `NOT_FOUND`, and the user sees a + * permanent "session not found" screen for a session that genuinely + * exists. Child boot time does not widen this window (the parent + * pre-creates the row itself, independent of when the CLI child + * actually attaches), so 8 attempts at ~1s is generous. + * + * Without the `spawned=1` route param, behavior is byte-identical to the + * pre-C3b contract: `retry: false` everywhere. This guard keeps a + * stale/deleted session in the user's history showing the same permanent + * "not found" state it always did. + * + * Implemented as a free function (not a method on the route component) so + * it can be unit-tested in a plain Node vitest environment without + * rendering the screen or mocking the router. + */ +export const SPAWNED_NOT_FOUND_MAX_ATTEMPTS = 8; + +export function shouldRetryNotFoundOnSpawnedRoute({ + spawned, + attempt, + errorCode, +}: { + /** + * The `spawned` route param, if any. `undefined` (absent) means the + * caller navigated here through any other path (push, deep link, tab + * tap, etc.) and must not get the spawned-row retry. + */ + spawned: string | undefined; + /** + * 0-indexed attempt count from TanStack Query's `retry(failureCount)` + * callback. `attempt === 0` means the first failure (the very first + * request returned an error). + */ + attempt: number; + /** + * The structured tRPC error code, if any. `NOT_FOUND` is the only + * error this helper retries; any other code is the route's problem to + * surface (transient, retriable by the existing QueryError UI for + * non-`NOT_FOUND`). + */ + errorCode: string | undefined; +}): boolean { + if (spawned === undefined) { + return false; + } + if (errorCode !== 'NOT_FOUND') { + return false; + } + return attempt < SPAWNED_NOT_FOUND_MAX_ATTEMPTS; +} diff --git a/apps/mobile/vitest.config.ts b/apps/mobile/vitest.config.ts index eebcd19a60..f0b9a28feb 100644 --- a/apps/mobile/vitest.config.ts +++ b/apps/mobile/vitest.config.ts @@ -21,6 +21,21 @@ export default defineConfig({ 'cloud-agent-sdk/remote-command-catalog': fileURLToPath( new URL('../../apps/web/src/lib/cloud-agent-sdk/remote-command-catalog.ts', import.meta.url) ), + // kilocode_change - K1/C2: narrow subpaths for the `kilo remote` spawn + // hook. These two files have a self-contained import graph (schemas, + // base-connection, runtime, types — no `@/...` web-app-alias imports), + // unlike the full barrel (`cloud-agent-sdk` below), which transitively + // pulls in `cloud-agent-connection.ts` -> `cloud-agent-transport.ts` -> + // `@/lib/cloud-agent-next/event-types`, a web-only `@` alias that does + // not resolve under the mobile app's own `@` alias. Runtime imports of + // `CommandDeliveredError`/`UserWebCommandError`/`createRemoteSessionOnConnection` + // must go through these subpaths, not the barrel. + 'cloud-agent-sdk/user-web-connection': fileURLToPath( + new URL('../../apps/web/src/lib/cloud-agent-sdk/user-web-connection.ts', import.meta.url) + ), + 'cloud-agent-sdk/create-session': fileURLToPath( + new URL('../../apps/web/src/lib/cloud-agent-sdk/create-session.ts', import.meta.url) + ), 'cloud-agent-sdk': fileURLToPath( new URL('../../apps/web/src/lib/cloud-agent-sdk/index.ts', import.meta.url) ), diff --git a/apps/web/src/lib/cloud-agent-sdk/create-session.test.ts b/apps/web/src/lib/cloud-agent-sdk/create-session.test.ts index bcb3219395..44b5fcb20f 100644 --- a/apps/web/src/lib/cloud-agent-sdk/create-session.test.ts +++ b/apps/web/src/lib/cloud-agent-sdk/create-session.test.ts @@ -1,4 +1,9 @@ -import { createSessionResponseV1Schema, parseCreateSessionResponse } from './create-session'; +import { + createRemoteSessionOnConnection, + createSessionResponseV1Schema, + parseCreateSessionResponse, +} from './create-session'; +import { CommandDeliveredError, UserWebCommandError } from './user-web-connection'; const VALID_SESSION_ID = 'ses_12345678901234567890123456'; @@ -145,3 +150,78 @@ describe('parseCreateSessionResponse', () => { expect(parseCreateSessionResponse(1)).toEqual({ ok: false, reason: 'invalid' }); }); }); + +describe('createRemoteSessionOnConnection', () => { + function makeFakeConnection() { + return { + sendCommandToConnection: jest.fn(), + }; + } + + it('issues a connection-scoped create_session with protocolVersion: 1 and the expected connectionId', async () => { + const connection = makeFakeConnection(); + connection.sendCommandToConnection.mockResolvedValue({ + protocolVersion: 1, + sessionID: VALID_SESSION_ID, + }); + + const result = await createRemoteSessionOnConnection(connection, 'cli-owner-1'); + + expect(connection.sendCommandToConnection).toHaveBeenCalledTimes(1); + expect(connection.sendCommandToConnection).toHaveBeenCalledWith({ + command: 'create_session', + data: { protocolVersion: 1 }, + expectedConnectionId: 'cli-owner-1', + }); + expect(parseCreateSessionResponse(result)).toEqual({ + ok: true, + kiloSessionId: VALID_SESSION_ID, + }); + }); + + it('resolves with the raw reply and lets the caller see a malformed response', async () => { + const connection = makeFakeConnection(); + connection.sendCommandToConnection.mockResolvedValue({ not: 'a v1 envelope' }); + + const result = await createRemoteSessionOnConnection(connection, 'cli-owner-1'); + + expect(parseCreateSessionResponse(result)).toEqual({ ok: false, reason: 'invalid' }); + }); + + it('propagates a delivered bare-string error as a CommandDeliveredError', async () => { + const connection = makeFakeConnection(); + connection.sendCommandToConnection.mockRejectedValue( + new CommandDeliveredError('Session owner not found') + ); + + await expect(createRemoteSessionOnConnection(connection, 'cli-owner-1')).rejects.toBeInstanceOf( + CommandDeliveredError + ); + }); + + it('propagates a structured UserWebCommandError as itself', async () => { + const connection = makeFakeConnection(); + connection.sendCommandToConnection.mockRejectedValue( + new UserWebCommandError({ + code: 'CLI_UPGRADE_REQUIRED', + message: 'upgrade required', + }) + ); + + await expect(createRemoteSessionOnConnection(connection, 'cli-owner-1')).rejects.toBeInstanceOf( + UserWebCommandError + ); + }); + + it('propagates a transport-level rejection as a plain (non-CommandDeliveredError) Error', async () => { + const connection = makeFakeConnection(); + connection.sendCommandToConnection.mockRejectedValue(new Error('Connection destroyed')); + + const rejection = await createRemoteSessionOnConnection(connection, 'cli-owner-1').catch( + (error: unknown) => error + ); + expect(rejection).toBeInstanceOf(Error); + expect(rejection).not.toBeInstanceOf(CommandDeliveredError); + expect(rejection).not.toBeInstanceOf(UserWebCommandError); + }); +}); diff --git a/apps/web/src/lib/cloud-agent-sdk/create-session.ts b/apps/web/src/lib/cloud-agent-sdk/create-session.ts index b90a4b7110..e657033815 100644 --- a/apps/web/src/lib/cloud-agent-sdk/create-session.ts +++ b/apps/web/src/lib/cloud-agent-sdk/create-session.ts @@ -14,6 +14,7 @@ */ import { createSessionResponseV1Schema, type CreateSessionResponseV1 } from './schemas'; import type { KiloSessionId } from './types'; +import type { UserWebConnection } from './user-web-connection'; export { createSessionResponseV1Schema } from './schemas'; export type { CreateSessionResponseV1 } from './schemas'; @@ -35,3 +36,38 @@ export function parseCreateSessionResponse(raw: unknown): CreateSessionParseResu if (!parsed.success) return { ok: false, reason: 'invalid' }; return { ok: true, kiloSessionId: parsed.data.sessionID }; } + +/** + * Result of `createRemoteSessionOnConnection` — the raw, unparsed + * `create_session` reply. Callers should run it through + * `parseCreateSessionResponse` to obtain a `KiloSessionId`. Exposed for + * consistency with other SDK helpers that return the raw reply; success + * here only means "the relay accepted and answered", not "the body is valid". + */ +export type CreateRemoteSessionRawResult = unknown; + +/** + * Connection-scoped `create_session` for the `kilo remote` process-per-session + * spawn flow. Unlike the session-scoped `createSession` in + * `cli-live-transport.ts` (which fences the command to a known Kilo sessionId), + * this helper targets a specific CLI viewer connection and omits any + * `sessionId` on the wire — the CLI is expected to provision a fresh + * `KiloSessionId` for the new cloud-agent session. + * + * The returned promise resolves with the raw reply; the caller is responsible + * for parsing the response shape. A delivered error response (string or + * structured `UserWebCommandError`) rejects the promise; transport failures + * (timeout, destroyed connection) reject with a plain `Error`. See + * `CommandDeliveredError` and `UserWebCommandError` for the rejection + * subclass contract. + */ +export async function createRemoteSessionOnConnection( + connection: Pick, + connectionId: string +): Promise { + return connection.sendCommandToConnection({ + command: 'create_session', + data: { protocolVersion: 1 }, + expectedConnectionId: connectionId, + }); +} diff --git a/apps/web/src/lib/cloud-agent-sdk/index.ts b/apps/web/src/lib/cloud-agent-sdk/index.ts index 101f13e635..f7cc9954b2 100644 --- a/apps/web/src/lib/cloud-agent-sdk/index.ts +++ b/apps/web/src/lib/cloud-agent-sdk/index.ts @@ -62,7 +62,11 @@ export type { CliHistoricalTransportConfig } from './cli-historical-transport'; export { createCliLiveTransport } from './cli-live-transport'; export type { CliLiveTransportConfig } from './cli-live-transport'; -export { createUserWebConnection, UserWebCommandError } from './user-web-connection'; +export { + createUserWebConnection, + CommandDeliveredError, + UserWebCommandError, +} from './user-web-connection'; export type { UserWebConnection, UserWebConnectionConfig, @@ -73,6 +77,8 @@ export type { UserWebSystemEvent, } from './user-web-connection'; +export { createRemoteSessionOnConnection } from './create-session'; + export { REMOTE_MODEL_IDENTITY_MAX_LENGTH, REMOTE_MODEL_MAX_MODELS_PER_PROVIDER, diff --git a/apps/web/src/lib/cloud-agent-sdk/user-web-connection.test.ts b/apps/web/src/lib/cloud-agent-sdk/user-web-connection.test.ts index d826e9b040..9a12a44ce2 100644 --- a/apps/web/src/lib/cloud-agent-sdk/user-web-connection.test.ts +++ b/apps/web/src/lib/cloud-agent-sdk/user-web-connection.test.ts @@ -1,5 +1,6 @@ import { configureCloudAgentSdkRuntime, resetCloudAgentSdkRuntime } from './runtime'; import { + CommandDeliveredError, createUserWebConnection, UserWebCommandError, VIEWER_PING_INTERVAL_MS, @@ -1206,8 +1207,9 @@ describe('createUserWebConnection', () => { inbound({ type: 'response', id: 'uuid-2', error: 'CLI disconnected' }); await expect(promise).rejects.toEqual( - expect.objectContaining({ name: 'Error', message: 'CLI disconnected' }) + expect.objectContaining({ name: 'CommandDeliveredError', message: 'CLI disconnected' }) ); + await expect(promise).rejects.toBeInstanceOf(CommandDeliveredError); await expect(promise).rejects.not.toBeInstanceOf(UserWebCommandError); client.destroy(); }); @@ -1630,4 +1632,77 @@ describe('createUserWebConnection sendCommandToConnection', () => { await promise; client.destroy(); }); + + it('wraps a delivered bare-string error in CommandDeliveredError on a connection-scoped command', async () => { + const client = createUserWebConnection({ websocketUrl: WS_URL, getAuthToken: () => 'token' }); + client.connect(); + open(); + + const promise = client.sendCommandToConnection({ + command: 'create_session', + data: { protocolVersion: 1 }, + expectedConnectionId: 'cli-owner-1', + }); + await Promise.resolve(); + inbound({ type: 'response', id: 'uuid-2', error: 'Session owner not found' }); + + await expect(promise).rejects.toEqual( + expect.objectContaining({ + name: 'CommandDeliveredError', + message: 'Session owner not found', + }) + ); + await expect(promise).rejects.toBeInstanceOf(CommandDeliveredError); + await expect(promise).rejects.not.toBeInstanceOf(UserWebCommandError); + client.destroy(); + }); + + it('leaves structured UserWebCommandError unaffected on a connection-scoped command', async () => { + const client = createUserWebConnection({ websocketUrl: WS_URL, getAuthToken: () => 'token' }); + client.connect(); + open(); + + const promise = client.sendCommandToConnection({ + command: 'create_session', + data: { protocolVersion: 1 }, + expectedConnectionId: 'cli-owner-1', + }); + await Promise.resolve(); + inbound({ + type: 'response', + id: 'uuid-2', + error: { + source: 'relay', + code: 'CLI_UPGRADE_REQUIRED', + message: 'upgrade', + }, + }); + + await expect(promise).rejects.toBeInstanceOf(UserWebCommandError); + await expect(promise).rejects.not.toBeInstanceOf(CommandDeliveredError); + client.destroy(); + }); + + it('leaves a transport-level timeout as a plain (non-CommandDeliveredError) Error', async () => { + const client = createUserWebConnection({ websocketUrl: WS_URL, getAuthToken: () => 'token' }); + client.connect(); + open(); + + const promise = client.sendCommandToConnection({ + command: 'create_session', + data: { protocolVersion: 1 }, + expectedConnectionId: 'cli-owner-1', + }); + await Promise.resolve(); + // No inbound response: the SDK's 30s client-side timer will reject the + // command. To avoid making the suite 30s, advance fake timers if jest's + // fake timers are installed; otherwise rely on the real timer being + // overridden via the COMMAND_TIMEOUT_MS export — for this test we + // simulate a transport-level rejection by destroying the client. + client.destroy(); + + await expect(promise).rejects.toBeInstanceOf(Error); + await expect(promise).rejects.not.toBeInstanceOf(CommandDeliveredError); + await expect(promise).rejects.not.toBeInstanceOf(UserWebCommandError); + }); }); diff --git a/apps/web/src/lib/cloud-agent-sdk/user-web-connection.ts b/apps/web/src/lib/cloud-agent-sdk/user-web-connection.ts index 46e3ccf9d0..5c45b98c5e 100644 --- a/apps/web/src/lib/cloud-agent-sdk/user-web-connection.ts +++ b/apps/web/src/lib/cloud-agent-sdk/user-web-connection.ts @@ -36,6 +36,24 @@ class UserWebCommandError extends Error { } } +/** + * Error class used to wrap a *delivered* (non-structured, bare-string) command + * error response. The message is preserved verbatim so generic `catch (e)` + * consumers see the same string the relay/CLI emitted; the new class only + * lets downstream callers distinguish a delivered error from a transport-level + * failure (timeout, connection destroyed, socket gone — those stay plain + * `Error`). + * + * Structured `UserWebCommandError` responses (relay envelopes with a `.code`) + * are NOT wrapped — they already carry enough info. + */ +class CommandDeliveredError extends Error { + constructor(message: string) { + super(message); + this.name = 'CommandDeliveredError'; + } +} + type UserWebConnectionConfig = { websocketUrl: string; getAuthToken: () => string | Promise; @@ -82,8 +100,20 @@ type UserWebConnection = { ) => () => void; }; +/** + * Resolve a delivered command-error payload into an `Error` subclass. + * + * - A bare-string delivered error is wrapped in `CommandDeliveredError` so + * downstream consumers can distinguish it from transport-level failures + * (which stay plain `Error`). + * - A structured relay envelope that matches `userWebCommandErrorDataSchema` + * becomes a `UserWebCommandError` (already has a `.code`). + * - A malformed structured payload collapses to a plain `Error('Command failed')` + * — the relay envelope wasn't trustworthy so this is not a real "delivered" + * message. + */ function parseCommandError(error: unknown): Error { - if (typeof error === 'string') return new Error(error); + if (typeof error === 'string') return new CommandDeliveredError(error); const parsed = userWebCommandErrorDataSchema.safeParse(error); if (parsed.success) return new UserWebCommandError(parsed.data); @@ -628,7 +658,7 @@ function createUserWebConnection( }; } -export { createUserWebConnection, UserWebCommandError }; +export { createUserWebConnection, CommandDeliveredError, UserWebCommandError }; export type { SendCommandToConnectionInput, UserWebConnection, diff --git a/apps/web/src/routers/active-sessions-router.test.ts b/apps/web/src/routers/active-sessions-router.test.ts new file mode 100644 index 0000000000..b43739bd29 --- /dev/null +++ b/apps/web/src/routers/active-sessions-router.test.ts @@ -0,0 +1,159 @@ +import { describe, expect, it, jest, beforeAll, afterEach } from '@jest/globals'; +import { TRPCError } from '@trpc/server'; +import { insertTestUser } from '@/tests/helpers/user.helper'; +import { db } from '@/lib/drizzle'; +import { organizations, organization_memberships } from '@kilocode/db/schema'; +import type { User, Organization } from '@kilocode/db/schema'; +import type { createCallerForUser as CreateCallerForUser } from '@/routers/test-utils'; + +// `.env.test` sets SESSION_INGEST_WORKER_URL to '' (shared fixture used by +// other test files too — do not change it here). `createCallerForUser`'s +// import chain (test-utils -> trpc/init -> ...) transitively loads +// `@/lib/config.server`, whose `SESSION_INGEST_WORKER_URL` export is a plain +// `const` computed once, the first time that module is evaluated. Static +// ES `import` statements are always hoisted above every other statement by +// the transform, so a statically-imported `createCallerForUser` would pull +// in the real ('') value before any `process.env` assignment written below +// it could run — and a `jest.mock('@/lib/config.server', ...)` registered +// after that first (real) load cannot retroactively change the value +// active-sessions-router.ts already captured. A dynamic `import()` executes +// exactly where it is awaited (not hoisted), so resolving it in `beforeAll` +// — after the env var is set below — lets `config.server.ts` pick up the +// test value on its one evaluation, without the repo-lint-forbidden +// `require()`. +process.env.SESSION_INGEST_WORKER_URL = 'https://test-ingest.example.com'; +let createCallerForUser: typeof CreateCallerForUser; + +let regularUser: User; +let testOrganization: Organization; + +describe('active-sessions-router', () => { + beforeAll(async () => { + ({ createCallerForUser } = await import('@/routers/test-utils')); + + regularUser = await insertTestUser({ + google_user_email: 'active-sessions-user@example.com', + google_user_name: 'Active Sessions User', + is_admin: false, + }); + + const [org] = await db + .insert(organizations) + .values({ + name: 'Active Sessions Test Org', + created_by_kilo_user_id: regularUser.id, + }) + .returning(); + testOrganization = org; + + await db.insert(organization_memberships).values({ + organization_id: testOrganization.id, + kilo_user_id: regularUser.id, + role: 'owner', + }); + // kilocode_change - the dynamic `import()` above (needed so + // `process.env.SESSION_INGEST_WORKER_URL` is set before + // `config.server.ts` evaluates it — see the comment above the env + // assignment) resolves a module graph on its first hit rather than + // reusing a build-time-hoisted static import, which can push this + // hook past Jest's default 5s under full-suite parallel load even + // though it is comfortably fast in isolation. Give it real headroom + // rather than a fragile default. + }, 15_000); + + afterEach(() => { + jest.restoreAllMocks(); + }); + + describe('listInstances', () => { + it('returns the instances from the worker when the upstream call succeeds', async () => { + const fetchSpy = jest.spyOn(global, 'fetch').mockResolvedValue( + new Response( + JSON.stringify({ + instances: [ + { connectionId: 'cli-A', name: 'laptop-A', projectName: 'kilo', version: '0.1.2' }, + { connectionId: 'cli-B', name: 'laptop-B', projectName: 'kilo' }, + ], + }), + { status: 200, headers: { 'Content-Type': 'application/json' } } + ) + ); + + const caller = await createCallerForUser(regularUser.id); + const result = await caller.activeSessions.listInstances(); + + expect(result).toEqual({ + instances: [ + { connectionId: 'cli-A', name: 'laptop-A', projectName: 'kilo', version: '0.1.2' }, + { connectionId: 'cli-B', name: 'laptop-B', projectName: 'kilo' }, + ], + }); + // Verify it actually called the worker with the right path. + const calledUrl = fetchSpy.mock.calls[0][0] as unknown as string; + expect(calledUrl).toBe('https://test-ingest.example.com/api/instances/active'); + }); + + it('returns the empty `instances` array when the worker has no live CLIs', async () => { + jest.spyOn(global, 'fetch').mockResolvedValue( + new Response(JSON.stringify({ instances: [] }), { + status: 200, + headers: { 'Content-Type': 'application/json' }, + }) + ); + + const caller = await createCallerForUser(regularUser.id); + const result = await caller.activeSessions.listInstances(); + expect(result).toEqual({ instances: [] }); + }); + + it('throws a TRPCError when the upstream worker returns a non-2xx response', async () => { + jest + .spyOn(global, 'fetch') + .mockResolvedValue(new Response('upstream failed', { status: 502 })); + + const caller = await createCallerForUser(regularUser.id); + const rejection = caller.activeSessions.listInstances(); + await expect(rejection).rejects.toBeInstanceOf(TRPCError); + try { + await rejection; + } catch (err) { + if (!(err instanceof TRPCError)) throw err; + expect(err.code).toBe('INTERNAL_SERVER_ERROR'); + // Mobile (later slice) uses the thrown status to render a retryable + // error state; verify the contract is "throws, never returns empty". + expect(err.message).toContain('502'); + } + }); + + it('throws a TRPCError when fetch itself fails (network error)', async () => { + jest.spyOn(global, 'fetch').mockRejectedValue(new Error('socket hang up')); + + const caller = await createCallerForUser(regularUser.id); + const rejection = caller.activeSessions.listInstances(); + await expect(rejection).rejects.toBeInstanceOf(TRPCError); + }); + + it('throws a TRPCError when the worker returns an unexpected payload shape', async () => { + jest.spyOn(global, 'fetch').mockResolvedValue( + new Response(JSON.stringify({ wrong: 'shape' }), { + status: 200, + headers: { 'Content-Type': 'application/json' }, + }) + ); + + const caller = await createCallerForUser(regularUser.id); + await expect(caller.activeSessions.listInstances()).rejects.toBeInstanceOf(TRPCError); + }); + + it('does NOT swallow upstream failures into {instances: []} (unlike `list`)', async () => { + // Sanity: `list` returns {sessions: []} on a 502; `listInstances` must not. + jest + .spyOn(global, 'fetch') + .mockResolvedValueOnce(new Response('bad gateway', { status: 502 })); + + const caller = await createCallerForUser(regularUser.id); + const result = await caller.activeSessions.list(); + expect(result).toEqual({ sessions: [] }); + }); + }); +}); diff --git a/apps/web/src/routers/active-sessions-router.ts b/apps/web/src/routers/active-sessions-router.ts index e41d67aed7..ac3471f88d 100644 --- a/apps/web/src/routers/active-sessions-router.ts +++ b/apps/web/src/routers/active-sessions-router.ts @@ -1,5 +1,6 @@ import 'server-only'; import { baseProcedure, createTRPCRouter } from '@/lib/trpc/init'; +import { TRPCError } from '@trpc/server'; import { z } from 'zod'; import { SESSION_INGEST_WORKER_URL } from '@/lib/config.server'; import { generateInternalServiceToken } from '@/lib/tokens'; @@ -11,13 +12,28 @@ const activeSessionSchema = z.object({ connectionId: z.string(), gitUrl: z.string().optional(), gitBranch: z.string().optional(), + // Optional: legacy CLIs (predating the `kilo remote` spawner) never + // report a platform. Only present in the response when the CLI supplied it. + platform: z.string().optional(), }); const activeSessionsResponseSchema = z.object({ sessions: z.array(activeSessionSchema), }); +const connectedInstanceSchema = z.object({ + connectionId: z.string(), + name: z.string(), + projectName: z.string(), + version: z.string().optional(), +}); + +const connectedInstancesResponseSchema = z.object({ + instances: z.array(connectedInstanceSchema), +}); + export type ActiveSession = z.infer; +export type ConnectedInstance = z.infer; export const activeSessionsRouter = createTRPCRouter({ getToken: baseProcedure.query(async ({ ctx }) => { @@ -53,4 +69,52 @@ export const activeSessionsRouter = createTRPCRouter({ return { sessions: [] as ActiveSession[] }; } }), + + /** + * Live snapshot of every `kilo remote` instance currently connected for the + * authenticated user. Unlike `list` (which swallows upstream errors into + * `{sessions: []}`), this throws a `TRPCError` on failure so the mobile + * UI can distinguish a retryable transport error from a genuine empty + * state. The companion `getToken` call is owned by C2. + */ + listInstances: baseProcedure.query(async ({ ctx }) => { + if (!SESSION_INGEST_WORKER_URL) { + throw new TRPCError({ + code: 'INTERNAL_SERVER_ERROR', + message: 'SESSION_INGEST_WORKER_URL is not configured', + }); + } + + const token = generateInternalServiceToken(ctx.user.id); + const url = `${SESSION_INGEST_WORKER_URL}/api/instances/active`; + + let response: Response; + try { + response = await fetch(url, { + headers: { Authorization: `Bearer ${token}` }, + }); + } catch (error) { + console.warn('[active-sessions.instances] fetch error:', error); + throw new TRPCError({ + code: 'INTERNAL_SERVER_ERROR', + message: 'Failed to reach session-ingest worker', + cause: error, + }); + } + + if (!response.ok) { + const body = await response.text().catch(() => ''); + console.warn( + `[active-sessions.instances] non-2xx: ${response.status} ${response.statusText}`, + body + ); + throw new TRPCError({ + code: 'INTERNAL_SERVER_ERROR', + message: `Session-ingest worker returned ${response.status}`, + }); + } + + const raw = await response.json(); + return connectedInstancesResponseSchema.parse(raw); + }), }); diff --git a/services/session-ingest/src/dos/UserConnectionDO.test.ts b/services/session-ingest/src/dos/UserConnectionDO.test.ts index e660d6555d..06b365e0f5 100644 --- a/services/session-ingest/src/dos/UserConnectionDO.test.ts +++ b/services/session-ingest/src/dos/UserConnectionDO.test.ts @@ -87,8 +87,15 @@ function createMockCtx() { // Helpers // --------------------------------------------------------------------------- -function makeSession(id: string, status = 'busy', title = 'Test', parentSessionId?: string) { - return parentSessionId ? { id, status, title, parentSessionId } : { id, status, title }; +function makeSession( + id: string, + status = 'busy', + title = 'Test', + parentSessionId?: string, + platform?: string +) { + const base = platform ? { id, status, title, platform } : { id, status, title }; + return parentSessionId ? { ...base, parentSessionId } : base; } function parseSent(ws: MockWS, callIndex = 0): unknown { @@ -186,9 +193,17 @@ function addCliSocket( id: string; status: string; title: string; - }> = [] + platform?: string; + }> = [], + instance?: { name: string; projectName: string; version?: string } ): MockWS { - const attachment = { role: 'cli' as const, connectionId, sessions }; + const attachment: { + role: 'cli'; + connectionId: string; + sessions: typeof sessions; + instance?: typeof instance; + } = { role: 'cli', connectionId, sessions }; + if (instance) attachment.instance = instance; const ws = createMockWs(['cli'], attachment); mockCtx.addSocket(ws); return ws; @@ -214,13 +229,16 @@ function sendHeartbeat( id: string; status: string; title: string; + platform?: string; }>, - protocolVersion?: string + protocolVersion?: string, + instance?: { name: string; projectName: string; version?: string } ) { const msg = JSON.stringify({ type: 'heartbeat', sessions, ...(protocolVersion ? { protocolVersion } : {}), + ...(instance ? { instance } : {}), }); doInstance.webSocketMessage(cliWs as never, msg); } @@ -2951,6 +2969,196 @@ describe('UserConnectionDO', () => { { id: 'root-1', status: 'idle', title: 'Root session', connectionId: 'cli-1' }, ]); }); + + it('forwards the per-session platform when the CLI reports it (newer CLIs)', () => { + const { doInstance, mockCtx } = setup(); + const cliWs = addCliSocket(mockCtx, 'cli-1'); + + sendHeartbeat(doInstance, cliWs, [ + makeSession('s1', 'busy', 'On a Mac', undefined, 'darwin'), + makeSession('s2', 'idle', 'Other'), + ]); + + const result = doInstance.getActiveSessions(); + expect(result).toEqual([ + { id: 's1', status: 'busy', title: 'On a Mac', connectionId: 'cli-1', platform: 'darwin' }, + { id: 's2', status: 'idle', title: 'Other', connectionId: 'cli-1' }, + ]); + }); + + it('omits the platform key entirely for legacy CLIs (byte-identical response)', () => { + const { doInstance, mockCtx } = setup(); + const cliWs = addCliSocket(mockCtx, 'cli-1'); + + // Legacy CLI heartbeat without platform. + sendHeartbeat(doInstance, cliWs, [makeSession('s1', 'busy', 'Legacy')]); + + const result = doInstance.getActiveSessions(); + expect(result).toEqual([ + { id: 's1', status: 'busy', title: 'Legacy', connectionId: 'cli-1' }, + ]); + expect(result[0]).not.toHaveProperty('platform'); + }); + }); + + // ------------------------------------------------------------------------- + // getConnectedInstances RPC (W3) + // ------------------------------------------------------------------------- + + describe('getConnectedInstances', () => { + it('returns one row per CLI socket that has an `instance` attachment', () => { + const { doInstance, mockCtx } = setup(); + // Use the hibernated-attachment pattern (no heartbeat) — the live + // scan reads the `instance` directly from the attachment, which is + // what the spec requires: a fresh value with no in-memory map. + addCliSocket(mockCtx, 'cli-A', [], { + name: 'laptop-A', + projectName: 'kilo', + version: '0.1.2', + }); + addCliSocket(mockCtx, 'cli-B', [], { name: 'laptop-B', projectName: 'kilo' }); + addWebSocket(mockCtx); + + const { instances } = doInstance.getConnectedInstances(); + expect(instances).toHaveLength(2); + expect(instances).toEqual( + expect.arrayContaining([ + { connectionId: 'cli-A', name: 'laptop-A', projectName: 'kilo', version: '0.1.2' }, + { connectionId: 'cli-B', name: 'laptop-B', projectName: 'kilo' }, + ]) + ); + }); + + it('omits the `version` key when the CLI did not report one', () => { + const { doInstance, mockCtx } = setup(); + addCliSocket(mockCtx, 'cli-1', [], { name: 'laptop-1', projectName: 'kilo' }); + + const { instances } = doInstance.getConnectedInstances(); + expect(instances).toEqual([{ connectionId: 'cli-1', name: 'laptop-1', projectName: 'kilo' }]); + expect(instances[0]).not.toHaveProperty('version'); + }); + + it('excludes legacy CLIs that never reported an `instance`', () => { + const { doInstance, mockCtx } = setup(); + // Legacy CLI: pre-spawner heartbeat has no `instance`. + const cliWs = addCliSocket(mockCtx, 'legacy-1'); + sendHeartbeat(doInstance, cliWs, []); + + const { instances } = doInstance.getConnectedInstances(); + expect(instances).toEqual([]); + }); + + it('excludes web sockets', () => { + const { doInstance, mockCtx } = setup(); + addWebSocket(mockCtx); + // A web socket with an `instance`-shaped attachment must still be skipped. + const webWithInstance = createMockWs(['web'], { + role: 'web', + connectionId: 'web-1', + subscribedSessions: [], + } as never); + mockCtx.addSocket(webWithInstance); + + const { instances } = doInstance.getConnectedInstances(); + expect(instances).toEqual([]); + }); + + it('reads `instance` directly from the live socket (no in-memory map)', () => { + const { doInstance, mockCtx } = setup(); + // Simulate a hibernated attach: socket exists, attachment has `instance` + // set, but no heartbeat has been processed through the in-memory state. + addCliSocket(mockCtx, 'cli-h', [], { + name: 'laptop-h', + projectName: 'kilo', + version: '1.0.0', + }); + + const { instances } = doInstance.getConnectedInstances(); + expect(instances).toEqual([ + { connectionId: 'cli-h', name: 'laptop-h', projectName: 'kilo', version: '1.0.0' }, + ]); + }); + + it('persists `instance` in the WS attachment across heartbeats', () => { + const { doInstance, mockCtx } = setup(); + const cliWs = addCliSocket(mockCtx, 'cli-1'); + sendHeartbeat(doInstance, cliWs, [], '1', { + name: 'laptop-1', + projectName: 'kilo', + version: '0.1.0', + }); + + const att = cliWs.deserializeAttachment() as { instance?: { name: string } }; + expect(att.instance).toEqual({ name: 'laptop-1', projectName: 'kilo', version: '0.1.0' }); + }); + + it('drops `instance` from the attachment on a subsequent heartbeat that omits it', () => { + const { doInstance, mockCtx } = setup(); + const cliWs = addCliSocket(mockCtx, 'cli-1'); + // First heartbeat: with instance. + sendHeartbeat(doInstance, cliWs, [], undefined, { + name: 'laptop-1', + projectName: 'kilo', + }); + // Second heartbeat: instance removed (legacy fallback). The DO must not + // keep a stale `instance` value in the attachment. + sendHeartbeat(doInstance, cliWs, []); + + const att = cliWs.deserializeAttachment() as { instance?: unknown }; + expect(att.instance).toBeUndefined(); + }); + }); + + // ------------------------------------------------------------------------- + // WS attachment size guardrail (W3) + // ------------------------------------------------------------------------- + + describe('WS attachment size', () => { + // The Cloudflare `serializeAttachment` budget is ~2 KiB. A bounded + // instance object (name+projectName+version, all max length) adds well + // under 200 bytes; this test pins that contract so a future schema + // change cannot silently push us over the budget. + const SERIALIZE_ATTACHMENT_BUDGET = 2048; + // Bounded `instance` = 64 + 64 + 32 chars content + JSON framing ≈ 200 + // bytes; we allow a 25% safety margin so a future protocol bump to the + // instance shape (e.g. adding `pid`) cannot silently blow the 2 KiB + // attachment budget. + const INSTANCE_HEADROOM = 250; + + it('keeps the combined CLI attachment comfortably under 2 KiB with a worst-case instance', () => { + const worstCaseInstance = { + name: 'x'.repeat(64), + projectName: 'x'.repeat(64), + version: 'x'.repeat(32), + }; + // 4 sessions with realistic-but-large titles, git URLs, and branches. + // (4 is a generous upper bound for a single CLI owning a live session + // fleet; the actual HeartbeatSession shape imposes tighter per-field + // limits at the protocol layer.) + const sessions = Array.from({ length: 4 }, (_, i) => ({ + id: `ses_${String(i).padStart(26, '0')}`, + status: 'busy', + title: 'T'.repeat(120), + gitUrl: 'https://github.com/org/' + 'x'.repeat(60) + '.git', + gitBranch: 'b'.repeat(40), + })); + + const attachment = { + role: 'cli' as const, + connectionId: 'cli-1', + sessions, + protocolVersion: '255.255.65535', + kiloUserId: 'usr_' + 'x'.repeat(28), + instance: worstCaseInstance, + }; + + const serialized = new TextEncoder().encode(JSON.stringify(attachment)).byteLength; + + expect(serialized).toBeLessThan(SERIALIZE_ATTACHMENT_BUDGET); + // Sanity: the bounded instance alone is far below the headroom. + const instanceBytes = new TextEncoder().encode(JSON.stringify(worstCaseInstance)).byteLength; + expect(instanceBytes).toBeLessThan(INSTANCE_HEADROOM); + }); }); // ------------------------------------------------------------------------- diff --git a/services/session-ingest/src/dos/UserConnectionDO.ts b/services/session-ingest/src/dos/UserConnectionDO.ts index b132a3ff0e..05132eacf5 100644 --- a/services/session-ingest/src/dos/UserConnectionDO.ts +++ b/services/session-ingest/src/dos/UserConnectionDO.ts @@ -5,6 +5,7 @@ import { getSessionIngestDO } from './SessionIngestDO'; import { CLIOutboundMessageSchema, type CLIInboundMessage, + type Instance, type SessionEventPayload, SessionEventPayloadSchema, type WebInboundMessage, @@ -18,6 +19,12 @@ type HeartbeatSession = { gitUrl?: string; gitBranch?: string; parentSessionId?: string; + // Platform the session is running on (e.g. "darwin", "linux", "vscode"). + // Optional: legacy CLIs (predating the `kilo remote` spawner) do not + // report a platform; in that case this field is undefined and the + // `getActiveSessions()` response omits it (preserves byte-identical + // legacy responses). + platform?: string; }; type WSAttachment = @@ -32,9 +39,24 @@ type WSAttachment = // Set from the authenticated /user/cli route; undefined on sockets // accepted before this field existed. Needed for the session-ready push. kiloUserId?: string; + // Identity of the spawning CLI process (`kilo remote`). Undefined on + // legacy CLIs (no spawner). Persisted in the attachment so the live + // socket scan in `getConnectedInstances()` can read it without any + // in-memory map or hibernation reconstruction — keeps the response + // fresh and avoids stale instance rows on restart. + instance?: Instance; } | { role: 'web'; connectionId: string; subscribedSessions: string[]; replaced?: true }; +// Type re-export so test files and other internal callers can reference the +// connection-row shape from a single place. +export type ConnectedInstanceRow = { + connectionId: string; + name: string; + projectName: string; + version?: string; +}; + export const MAX_CATALOG_RESULT_BYTES = 512 * 1024; // Viewer command allowlist. Anything outside this set is rejected by the relay @@ -376,7 +398,7 @@ export class UserConnectionDO extends DurableObject { switch (msg.type) { case 'heartbeat': - this.handleHeartbeat(ws, attachment, msg.sessions, msg.protocolVersion); + this.handleHeartbeat(ws, attachment, msg.sessions, msg.protocolVersion, msg.instance); break; case 'event': this.handleCliEvent(msg.sessionId, msg.parentSessionId, msg.event, msg.data); @@ -391,7 +413,8 @@ export class UserConnectionDO extends DurableObject { ws: WebSocket, attachment: WSAttachment & { role: 'cli' }, sessions: HeartbeatSession[], - protocolVersion: string | undefined + protocolVersion: string | undefined, + instance: Instance | undefined ): void { const { connectionId } = attachment; const now = Date.now(); @@ -440,6 +463,7 @@ export class UserConnectionDO extends DurableObject { sessions, protocolVersion, kiloUserId: attachment.kiloUserId, + ...(instance ? { instance } : {}), }; ws.serializeAttachment(updatedAttachment); @@ -893,6 +917,33 @@ export class UserConnectionDO extends DurableObject { return this.aggregateSessions(); } + /** + * Live-socket scan of currently connected CLI WebSockets. Each socket whose + * attachment carries an `instance` (i.e. it is a `kilo remote` spawner) + * contributes one row; legacy CLIs that predate the spawner never report + * `instance` and are excluded by design. + * + * No in-memory map is consulted: hibernation/restart can never produce a + * stale row because we only read from sockets that are alive right now. + * The 2KB `serializeAttachment` budget comfortably accommodates a bounded + * instance object (well under 200 bytes). + */ + getConnectedInstances(): { instances: ConnectedInstanceRow[] } { + this.ensureState(); + const instances: ConnectedInstanceRow[] = []; + for (const ws of this.ctx.getWebSockets('cli')) { + const att = ws.deserializeAttachment() as WSAttachment | null; + if (att?.role !== 'cli' || !att.instance) continue; + instances.push({ + connectionId: att.connectionId, + name: att.instance.name, + projectName: att.instance.projectName, + ...(att.instance.version ? { version: att.instance.version } : {}), + }); + } + return { instances }; + } + async notifySessionEvent(event: SessionEventPayload): Promise<{ delivered: number }> { this.ensureState(); const parsed = SessionEventPayloadSchema.parse(event); @@ -1088,7 +1139,14 @@ export class UserConnectionDO extends DurableObject { const protocolVersion = this.connectionProtocolVersion.get(connectionId); for (const session of sessions) { if (session.parentSessionId) continue; - result.push({ ...session, connectionId, ...(protocolVersion ? { protocolVersion } : {}) }); + result.push({ + ...session, + connectionId, + ...(protocolVersion ? { protocolVersion } : {}), + // Preserve byte-identical responses for legacy senders that never + // include a `platform`: only forward the field when present. + ...(session.platform ? { platform: session.platform } : {}), + }); } } return result; diff --git a/services/session-ingest/src/routes/api.test.ts b/services/session-ingest/src/routes/api.test.ts index 1630c68407..0dde9cf96a 100644 --- a/services/session-ingest/src/routes/api.test.ts +++ b/services/session-ingest/src/routes/api.test.ts @@ -1622,4 +1622,52 @@ describe('api routes', () => { expect(forwardedUrl.pathname).toBe('/web'); expect(forwardedUrl.searchParams.get('connectionId')).toBe('viewer-1'); }); + + // ------------------------------------------------------------------------- + // GET /api/instances/active (W3) + // ------------------------------------------------------------------------- + + describe('GET /instances/active', () => { + it('returns connected instances from the UserConnectionDO', async () => { + const getConnectedInstances = vi.fn(async () => ({ + instances: [ + { connectionId: 'cli-A', name: 'laptop-A', projectName: 'kilo', version: '0.1.2' }, + { connectionId: 'cli-B', name: 'laptop-B', projectName: 'kilo' }, + ], + })); + vi.mocked(getUserConnectionDO).mockReturnValue({ + getConnectedInstances, + } as never); + + const app = makeApiApp(); + const res = await app.fetch( + new Request('http://local/instances/active', { method: 'GET' }), + makeTestEnv() + ); + + expect(res.status).toBe(200); + expect(getConnectedInstances).toHaveBeenCalledTimes(1); + expect(await res.json()).toEqual({ + instances: [ + { connectionId: 'cli-A', name: 'laptop-A', projectName: 'kilo', version: '0.1.2' }, + { connectionId: 'cli-B', name: 'laptop-B', projectName: 'kilo' }, + ], + }); + }); + + it('returns 200 with an empty `instances` array when no CLIs are connected', async () => { + vi.mocked(getUserConnectionDO).mockReturnValue({ + getConnectedInstances: vi.fn(async () => ({ instances: [] })), + } as never); + + const app = makeApiApp(); + const res = await app.fetch( + new Request('http://local/instances/active', { method: 'GET' }), + makeTestEnv() + ); + + expect(res.status).toBe(200); + expect(await res.json()).toEqual({ instances: [] }); + }); + }); }); diff --git a/services/session-ingest/src/routes/api.ts b/services/session-ingest/src/routes/api.ts index 2ebc8a759d..95a4dbe83e 100644 --- a/services/session-ingest/src/routes/api.ts +++ b/services/session-ingest/src/routes/api.ts @@ -518,6 +518,13 @@ api.get('/sessions/active', async c => { return c.json({ sessions }, 200); }); +api.get('/instances/active', async c => { + const kiloUserId = c.get('user_id'); + const stub = getUserConnectionDO(c.env, { kiloUserId }); + const { instances } = await stub.getConnectedInstances(); + return c.json({ instances }, 200); +}); + // CLI connects to /api/user/cli without userId in the path — userId comes from the JWT. api.get('/user/cli', async c => { if (c.req.header('Upgrade') !== 'websocket') { diff --git a/services/session-ingest/src/types/user-connection-protocol.test.ts b/services/session-ingest/src/types/user-connection-protocol.test.ts index b5145be65f..5e92c6b548 100644 --- a/services/session-ingest/src/types/user-connection-protocol.test.ts +++ b/services/session-ingest/src/types/user-connection-protocol.test.ts @@ -25,6 +25,83 @@ describe('CLIOutboundMessageSchema', () => { expect(result.success).toBe(true); }); + it('parses heartbeat with instance and per-session platform (kilo remote CLI)', () => { + const msg = { + type: 'heartbeat', + instance: { name: 'laptop-1', projectName: 'kilo', version: '0.1.2' }, + sessions: [ + { + id: 'ses_1', + status: 'busy', + title: 'Remote session', + platform: 'darwin', + }, + ], + }; + const result = CLIOutboundMessageSchema.safeParse(msg); + expect(result.success).toBe(true); + if (result.success && result.data.type === 'heartbeat') { + expect(result.data.instance).toEqual({ + name: 'laptop-1', + projectName: 'kilo', + version: '0.1.2', + }); + expect(result.data.sessions[0]).toMatchObject({ platform: 'darwin' }); + } + }); + + it('parses instance without version (optional field)', () => { + const msg = { + type: 'heartbeat', + instance: { name: 'laptop-1', projectName: 'kilo' }, + sessions: [], + }; + const result = CLIOutboundMessageSchema.safeParse(msg); + expect(result.success).toBe(true); + if (result.success && result.data.type === 'heartbeat') { + expect(result.data.instance).toEqual({ name: 'laptop-1', projectName: 'kilo' }); + } + }); + + it('rejects instance with empty name', () => { + const msg = { + type: 'heartbeat', + instance: { name: '', projectName: 'kilo' }, + sessions: [], + }; + expect(CLIOutboundMessageSchema.safeParse(msg).success).toBe(false); + }); + + it('rejects instance with oversize name', () => { + const msg = { + type: 'heartbeat', + instance: { name: 'x'.repeat(65), projectName: 'kilo' }, + sessions: [], + }; + expect(CLIOutboundMessageSchema.safeParse(msg).success).toBe(false); + }); + + it('rejects per-session platform that exceeds the 32-char cap', () => { + const msg = { + type: 'heartbeat', + sessions: [{ id: 'ses_1', status: 'busy', title: 't', platform: 'x'.repeat(33) }], + }; + expect(CLIOutboundMessageSchema.safeParse(msg).success).toBe(false); + }); + + it('parses legacy heartbeat without instance or platform (backward compat regression)', () => { + const msg = { + type: 'heartbeat', + sessions: [{ id: 'ses_1', status: 'busy', title: 'Legacy' }], + }; + const result = CLIOutboundMessageSchema.safeParse(msg); + expect(result.success).toBe(true); + if (result.success && result.data.type === 'heartbeat') { + expect(result.data.instance).toBeUndefined(); + expect(result.data.sessions[0]).not.toHaveProperty('platform'); + } + }); + it('parses heartbeat with parentSessionId on sessions', () => { const msg = { type: 'heartbeat', diff --git a/services/session-ingest/src/types/user-connection-protocol.ts b/services/session-ingest/src/types/user-connection-protocol.ts index da3e79a550..05d5058a13 100644 --- a/services/session-ingest/src/types/user-connection-protocol.ts +++ b/services/session-ingest/src/types/user-connection-protocol.ts @@ -6,12 +6,29 @@ import { z } from 'zod'; // -- CLI → DO (CLIOutbound) --------------------------------------------------- +// Identity of the CLI process (kilo remote spawner) attached to this WebSocket. +// Newer CLIs include this on every heartbeat; legacy CLIs that predate the +// `kilo remote` spawner omit it entirely. The DO persists the latest value +// in the WebSocket attachment and uses it for `getConnectedInstances()`. +const instanceSchema = z.object({ + name: z.string().min(1).max(64), + projectName: z.string().min(1).max(64), + version: z.string().max(32).optional(), +}); + +export type Instance = z.infer; + export const CLIOutboundMessageSchema = z.discriminatedUnion('type', [ z.object({ type: z.literal('heartbeat'), // Absent on CLI builds older than the protocolVersion field itself — treat // a missing value as a legacy CLI with no negotiated wire protocol. protocolVersion: z.string().optional(), + // Optional identity of the spawning CLI process. Absent on legacy CLIs + // (which are not spawned by `kilo remote`). When present, the DO + // persists it in the WebSocket attachment and exposes it via + // `getConnectedInstances()`. + instance: instanceSchema.optional(), sessions: z.array( z.object({ id: z.string(), @@ -20,6 +37,9 @@ export const CLIOutboundMessageSchema = z.discriminatedUnion('type', [ gitUrl: z.string().optional(), gitBranch: z.string().optional(), parentSessionId: z.string().optional(), + // Platform the session is running on (e.g. "darwin", "linux", "vscode"). + // Optional for backward compatibility with legacy CLIs. + platform: z.string().max(32).optional(), }) ), }),