diff --git a/app/(tabs)/_layout.tsx b/app/(tabs)/_layout.tsx index af6b490f..b8614c51 100644 --- a/app/(tabs)/_layout.tsx +++ b/app/(tabs)/_layout.tsx @@ -12,7 +12,7 @@ export default function TabLayout() { - + ) diff --git a/app/(tabs)/index.tsx b/app/(tabs)/index.tsx index 7c69237e..84c4ea1c 100644 --- a/app/(tabs)/index.tsx +++ b/app/(tabs)/index.tsx @@ -22,6 +22,7 @@ import { useTranslation } from "react-i18next" import { useSessions } from "../../src/stores/sessions" import { useConnections } from "../../src/stores/connections" import { useEvents } from "../../src/stores/events" +import { isHealthy } from "../../src/lib/sse-liveness" import { useCatalog } from "../../src/stores/catalog" import type BottomSheet from "@gorhom/bottom-sheet" import type { Session, Project } from "../../src/lib/sdk" @@ -105,7 +106,7 @@ function SessionItem({ )} - + ) } @@ -140,7 +141,7 @@ function GroupHeader({ ) @@ -198,6 +199,9 @@ export default function SessionsScreen() { // Directories collapsed in the grouped session list. Empty by default — // all groups start expanded (#67). const [collapsedDirs, setCollapsedDirs] = useState>(new Set()) + // The indicator tracks the transport, not "did we try to connect" — see + // src/lib/sse-liveness.ts. + const transportHealthy = useEvents((s) => isHealthy(s.transport)) const toggleGroup = useCallback((directory: string) => { setCollapsedDirs((prev) => { @@ -527,7 +531,13 @@ export default function SessionsScreen() { testID="connection-status-bar" > - + {/* Reflects verified SSE liveness, not merely "a connection is + selected". This was hardcoded green, so the indicator claimed + health even while the stream was dead. */} + {activeConnection.name} @@ -540,7 +550,7 @@ export default function SessionsScreen() { )} - + {error && ( @@ -723,7 +733,7 @@ export default function SessionsScreen() { {t("sessionsList.newSessionModal.browseFoldersHint")} - + {/* Manual path input fallback */} @@ -733,7 +743,7 @@ export default function SessionsScreen() { { // Expand ~ to server home directory diff --git a/app/(tabs)/settings.tsx b/app/(tabs)/settings.tsx index af3a4559..c55d1566 100644 --- a/app/(tabs)/settings.tsx +++ b/app/(tabs)/settings.tsx @@ -197,7 +197,7 @@ export default function SettingsScreen() { description={t("settings.security.lockNow.description")} isDark={isDark} onPress={lock} - right={} + right={} /> )} @@ -252,7 +252,7 @@ export default function SettingsScreen() { description={t("settings.privacy.privacyPolicy.description")} isDark={isDark} onPress={() => Linking.openURL(PRIVACY_POLICY_URL)} - right={} + right={} /> @@ -263,7 +263,7 @@ export default function SettingsScreen() { description={localeLabels[locale]} isDark={isDark} onPress={handleLanguagePress} - right={} + right={} /> Linking.openURL("https://github.com/anomalyco/opencode")} - right={} + right={} /> Linking.openURL("https://opencode.ai/docs")} - right={} + right={} /> diff --git a/app/_layout.tsx b/app/_layout.tsx index c669aac9..b86921ac 100644 --- a/app/_layout.tsx +++ b/app/_layout.tsx @@ -92,6 +92,14 @@ function RootLayout() { if (next === "background" && useAuth.getState().settings.requireBiometric) { useAuth.getState().lock() } + // Recover the event stream on foreground. Returning from background is + // exactly when a socket is most likely to be half-open -- doze, a Wi-Fi/ + // cellular handover -- and without this nothing re-checked it, so the app + // could sit showing stale data until the user navigated. resume() is a + // no-op when the transport is already live or an attempt is in flight. + if (next === "active") { + useEvents.getState().resume() + } }) return () => sub.remove() }, []) diff --git a/app/connection/[id].tsx b/app/connection/[id].tsx index 49701d21..2ea7dbf3 100644 --- a/app/connection/[id].tsx +++ b/app/connection/[id].tsx @@ -224,7 +224,7 @@ export default function EditConnectionScreen() { @@ -234,7 +234,7 @@ export default function EditConnectionScreen() { @@ -367,7 +367,7 @@ export default function AddConnectionScreen() { @@ -589,7 +589,7 @@ export default function AddConnectionScreen() { ? "https://your-tunnel.trycloudflare.com" : "https://api.opencode.ai" } - placeholderTextColor={isDark ? "#666666" : "#999999"} + placeholderTextColor={isDark ? "#9a9a9a" : "#999999"} value={url} onChangeText={setUrl} autoCapitalize="none" @@ -611,7 +611,7 @@ export default function AddConnectionScreen() { ([]) const [showInfo, setShowInfo] = useState(false) + // Non-null when the select-text sheet is open; holds the message's source + // text. Kept as the text itself rather than a messageID so the sheet keeps + // showing a stable snapshot even if the message streams or is reverted. + const [selectableText, setSelectableText] = useState(null) const { currentSession, @@ -177,17 +185,27 @@ export default function SessionScreen() { // message sent concurrently with a revert isn't hidden. const revertMessageID = currentSession?.revert?.messageID + // The store holds ONE transcript globally, and it still belongs to the + // previously-viewed session for the first frames after navigating here + // (selectSession runs in an effect, after render). Rendering it + // unconditionally flashes the last session's messages under this + // session's title. Bind the transcript to this screen's route id and + // render nothing until the store has actually switched. + const transcriptBound = currentSession?.id === id + // Inverted FlatList: data is reversed (newest first) so newest renders at bottom const messageData = useMemo( () => - (messages || []) - .filter((msg) => !revertMessageID || msg.id.startsWith("temp-") || msg.id < revertMessageID) - .map((msg) => ({ - message: msg, - parts: (parts && parts[msg.id]) || [], - })) - .reverse(), - [messages, parts, revertMessageID], + transcriptBound + ? (messages || []) + .filter((msg) => !revertMessageID || msg.id.startsWith("temp-") || msg.id < revertMessageID) + .map((msg) => ({ + message: msg, + parts: (parts && parts[msg.id]) || [], + })) + .reverse() + : [], + [messages, parts, revertMessageID, transcriptBound], ) // Tracks the latest composer text without pulling `input` into @@ -222,9 +240,35 @@ export default function SessionScreen() { // closing over props) so MessageBubble's custom memo comparator can bail // safely without risking a stale handler. const handleMessageLongPress = useCallback((messageID: string) => { - Alert.alert(t("session.alerts.messageActionsTitle"), undefined, [ - { text: t("common.cancel"), style: "cancel" }, - { + const state = useSessions.getState() + const parts = state.parts[messageID] + const isUser = state.messages.find((m) => m.id === messageID)?.role === "user" + const copyText = extractCopyText(parts) + const canCopy = hasCopyableText(parts) + + const actions: Parameters[2] = [{ text: t("common.cancel"), style: "cancel" }] + + // Copy/select come first because they apply to both roles. For assistant + // messages they are the *only* copy path: Markdown.tsx strips `selectable` + // from rendered prose to avoid facebook/react-native#46999 inside the + // transcript FlatList. + if (canCopy) { + actions.push({ + text: t("session.actions.copyMessage"), + onPress: () => { + Clipboard.setStringAsync(copyText).catch(() => {}) + }, + }) + actions.push({ + text: t("session.actions.selectText"), + onPress: () => setSelectableText(copyText), + }) + } + + // Edit/revert stays user-only — reverting to an assistant message is not + // a supported operation. + if (isUser) { + actions.push({ text: t("session.actions.editMessage"), onPress: () => { const doRevert = async () => { @@ -247,14 +291,49 @@ export default function SessionScreen() { } doRevert() }, - }, - ]) + }) + } + + // Nothing but Cancel means there is no action worth interrupting the + // user for (e.g. a tool-only message with no prose). + if (actions.length === 1) return + + Alert.alert(t("session.alerts.messageActionsTitle"), undefined, actions) }, [applyRevertResult, t]) const scrollToBottom = useCallback((animated = true) => { flatListRef.current?.scrollToOffset({ offset: 0, animated }) }, []) + // Follow new content (issue #155: "Message cannot be scrolled automatically"). + // + // scrollToBottom() was previously only wired to the manual scroll button, so + // nothing followed an arriving or streaming message. Compounding that, + // maintainVisibleContentPosition (below) deliberately holds visible items + // still when the data changes — and since new messages are inserted at index + // 0 of this inverted list, that setting parks new content just outside the + // viewport. That prop is worth keeping (it stops the jump when older pages + // load), so instead scroll explicitly. + // + // Only when the user is already at the bottom: someone who scrolled up to + // read history must not be yanked back down mid-sentence. See + // src/lib/auto-scroll.ts. + const newest = messageData[0] + const contentSignature = transcriptSignature( + messageData.length, + newest ? (newest.parts || []).reduce((n, part) => n + (part.text?.length ?? 0), 0) : 0, + ) + const prevSignatureRef = useRef(null) + useEffect(() => { + const auto = shouldAutoScroll({ + offsetY: scrollOffsetRef.current, + previousSignature: prevSignatureRef.current, + currentSignature: contentSignature, + }) + prevSignatureRef.current = contentSignature + if (auto) scrollToBottom(true) + }, [contentSignature, scrollToBottom]) + // Re-select on every focus, not just mount. currentSession/messages/ // permissions are a single global store, and the native stack keeps screens // underneath a pushed one mounted. Without re-selecting on focus, navigating @@ -450,10 +529,15 @@ export default function SessionScreen() { } } - // In inverted mode, offset 0 = bottom. Show scroll button when scrolled away from bottom. + // In inverted mode, offset 0 = bottom (newest message). Track the live + // offset in a ref as well as state: the auto-scroll effect below needs the + // current position without taking `offsetY` as a dependency, which would + // re-run it on every scroll frame. + const scrollOffsetRef = useRef(0) const handleScroll = useCallback((event: any) => { const { contentOffset } = event.nativeEvent - setShowScrollButton(contentOffset.y > 200) + scrollOffsetRef.current = contentOffset.y + setShowScrollButton(shouldShowScrollButton(contentOffset.y)) }, []) // Debounce: onEndReached can fire multiple times during a single scroll gesture @@ -605,8 +689,16 @@ export default function SessionScreen() { // bottom toolbar + input were left completely hidden behind the // keyboard (#147). "padding" restores avoidance without depending // on native resize. + // + // "padding" alone is still not enough on Android, though (#156): + // KeyboardAvoidingView measures its own frame in *window* coordinates + // but reads the keyboard's screenY in *screen* coordinates, and under + // edge-to-edge those origins differ by the status-bar inset — so the + // padding lands short by exactly that much and the composer stays + // hidden. keyboardVerticalOffset closes the gap; see + // src/lib/keyboard-offset.ts for the measured numbers. behavior="padding" - keyboardVerticalOffset={Platform.OS === "ios" ? 90 : 0} + keyboardVerticalOffset={keyboardVerticalOffset(Platform.OS, insets.top)} > {/* Session info pulldown */} setShowInfo(false)} /> + {/* Select/copy sheet for message text. Rendered here, outside the + transcript FlatList, so `selectable` actually works on Android. */} + setSelectableText(null)} + /> + {/* SSE reconnect/connected banner */} {reconnectAttempts > 0 && ( @@ -804,7 +904,7 @@ export default function SessionScreen() { ? t("session.input.placeholderFollowUp") : t("session.input.placeholderDefault") } - placeholderTextColor={speech.listening ? "#ef4444" : isDark ? "#666666" : "#999999"} + placeholderTextColor={speech.listening ? "#ef4444" : isDark ? "#9a9a9a" : "#999999"} value={speech.listening ? speech.transcript : input} onChangeText={speech.listening ? undefined : setInput} editable={!speech.listening} @@ -912,7 +1012,7 @@ const s = StyleSheet.create({ empty: { flex: 1, justifyContent: "center", alignItems: "center", paddingVertical: 64 }, emptyText: { fontSize: 16, color: "#999999", marginTop: 12 }, emptyHint: { fontSize: 13, color: "#bbbbbb", marginTop: 4 }, - metaDark: { color: "#666666" }, + metaDark: { color: "#9a9a9a" }, textWhite: { color: "#ffffff" }, // Toolbar diff --git a/src/components/WideScroll.tsx b/src/components/WideScroll.tsx new file mode 100644 index 00000000..4894bece --- /dev/null +++ b/src/components/WideScroll.tsx @@ -0,0 +1,92 @@ +import { useRef, type ReactNode } from "react" +import { ScrollView, View, type StyleProp, type ViewStyle } from "react-native" +import { Gesture, GestureDetector } from "react-native-gesture-handler" +import { dragOffset, flingTarget, CLAIM_MIN_DX, VERTICAL_FAIL_DY } from "../lib/horizontal-intent" + +/** + * A horizontal scroller that survives inside the vertical transcript. + * + * Round one used a JS PanResponder — and lost anyway on device, because + * Android's vertical list intercepts touches in the NATIVE layer before the + * JS responder system gets a vote. This version claims natively via + * react-native-gesture-handler: the pan activates once the drag crosses + * CLAIM_MIN_DX horizontally, FAILS if it crosses VERTICAL_FAIL_DY vertically + * first (the list keeps those), and on activation RNGH disallows the parent + * list's interception — which is the part no JS-level solution could do. + * + * The inner ScrollView is driven entirely by ref (scrollEnabled=false): + * one owner per axis, no double-handling of the same drag. + */ +export function WideScroll({ + children, + contentContainerStyle, + testID, +}: { + children: ReactNode + contentContainerStyle?: StyleProp + testID?: string +}) { + const scrollRef = useRef(null) + const offsetRef = useRef(0) + const startOffsetRef = useRef(0) + const contentWidthRef = useRef(0) + const layoutWidthRef = useRef(0) + const maxOffsetRef = useRef(0) + + const updateMax = () => { + maxOffsetRef.current = Math.max(0, contentWidthRef.current - layoutWidthRef.current) + } + + const pan = Gesture.Pan() + .activeOffsetX([-CLAIM_MIN_DX, CLAIM_MIN_DX]) + .failOffsetY([-VERTICAL_FAIL_DY, VERTICAL_FAIL_DY]) + .onStart(() => { + startOffsetRef.current = offsetRef.current + }) + .onUpdate((e) => { + const next = dragOffset(startOffsetRef.current, e.translationX, maxOffsetRef.current) + offsetRef.current = next + scrollRef.current?.scrollTo({ x: next, animated: false }) + }) + .onEnd((e) => { + // RNGH velocity is px/s; flingTarget projects px/ms. + const target = flingTarget(offsetRef.current, e.velocityX / 1000, maxOffsetRef.current) + if (Math.abs(target - offsetRef.current) > 1) { + offsetRef.current = target + scrollRef.current?.scrollTo({ x: target, animated: true }) + } + }) + .runOnJS(true) + + return ( + + + { + offsetRef.current = e.nativeEvent.contentOffset.x + }} + // Both known before any scroll event fires, so the very first drag + // clamps correctly instead of overshooting into empty space. + onContentSizeChange={(w) => { + contentWidthRef.current = w + updateMax() + }} + onLayout={(e) => { + layoutWidthRef.current = e.nativeEvent.layout.width + updateMax() + }} + scrollEventThrottle={16} + nestedScrollEnabled + > + {children} + + + + ) +} diff --git a/src/components/chat/DiffView.tsx b/src/components/chat/DiffView.tsx index 00c66c09..808e7362 100644 --- a/src/components/chat/DiffView.tsx +++ b/src/components/chat/DiffView.tsx @@ -1,5 +1,5 @@ -import { View, Text, StyleSheet, Platform, ScrollView } from "react-native" -import { WIDE_CONTENT_SCROLL_CONFIG } from "../../lib/scroll-config" +import { View, Text, StyleSheet, Platform } from "react-native" +import { WideScroll } from "../WideScroll" import { computeDiff } from "./diff-compute" const mono = Platform.OS === "ios" ? "Menlo" : "monospace" @@ -17,7 +17,7 @@ export function DiffView({ before, after, isDark }: Props) { return ( - + {lines.map((line, idx) => ( ))} - + ) } @@ -76,7 +76,7 @@ const s = StyleSheet.create({ color: "#999999", lineHeight: 20, }, - prefixDark: { color: "#666666" }, + prefixDark: { color: "#9a9a9a" }, text: { fontSize: 12, diff --git a/src/components/chat/DirectoryBrowserSheet.tsx b/src/components/chat/DirectoryBrowserSheet.tsx index bde091ee..9df6cd37 100644 --- a/src/components/chat/DirectoryBrowserSheet.tsx +++ b/src/components/chat/DirectoryBrowserSheet.tsx @@ -184,7 +184,7 @@ export function DirectoryBrowserSheet({ keyboardBlurBehavior="restore" android_keyboardInputMode="adjustResize" backgroundStyle={isDark ? s.sheetDark : s.sheet} - handleIndicatorStyle={{ backgroundColor: isDark ? "#666666" : "#cccccc" }} + handleIndicatorStyle={{ backgroundColor: isDark ? "#9a9a9a" : "#cccccc" }} backdropComponent={(props) => ( )} @@ -239,7 +239,7 @@ export function DirectoryBrowserSheet({ ( )} @@ -88,7 +88,7 @@ export function DirectorySwitcher({ sheetRef, current, recents, serverHome, isDa { if (serverHome && text === "~") setCustom(serverHome) @@ -259,7 +259,7 @@ const s = StyleSheet.create({ paddingTop: 4, paddingBottom: 8, }, - dimDark: { color: "#666666" }, + dimDark: { color: "#9a9a9a" }, row: { flexDirection: "row", alignItems: "center", diff --git a/src/components/chat/MessageBubble.tsx b/src/components/chat/MessageBubble.tsx index 0f82ecaf..751b5907 100644 --- a/src/components/chat/MessageBubble.tsx +++ b/src/components/chat/MessageBubble.tsx @@ -16,9 +16,12 @@ interface Props { message: Message parts: Part[] isDark: boolean - // Only wired up for user messages — long-press opens the "Edit message" / - // revert action sheet. Identified by messageID (not a closure over parts) - // so it stays correct even if the memo below bails on a stale render. + // Long-press opens the message action sheet. For user messages that sheet + // offers "Edit message" / revert; for both roles it offers copy and + // select-text (the only copy path assistant prose has — see + // src/lib/message-copy-text.ts). Identified by messageID (not a closure + // over parts) so it stays correct even if the memo below bails on a stale + // render. onLongPress?: (messageID: string) => void } @@ -37,9 +40,9 @@ export const MessageBubble = memo( return ( onLongPress(message.id) : undefined} - disabled={!isUser || !onLongPress} + activeOpacity={onLongPress ? 0.7 : 1} + onLongPress={onLongPress ? () => onLongPress(message.id) : undefined} + disabled={!onLongPress} style={[ s.bubble, isUser ? s.user : s.assistant, @@ -156,7 +159,7 @@ const s = StyleSheet.create({ markdownWrap: { marginHorizontal: -4 }, tokens: { fontSize: 11, color: "#999999", marginTop: 8 }, - tokensDark: { color: "#666666" }, + tokensDark: { color: "#9a9a9a" }, // Images imageScroll: { marginBottom: 8 }, diff --git a/src/components/chat/ModelPicker.tsx b/src/components/chat/ModelPicker.tsx index e66c83ea..892f5e58 100644 --- a/src/components/chat/ModelPicker.tsx +++ b/src/components/chat/ModelPicker.tsx @@ -92,7 +92,7 @@ export function ModelPicker({ providers, selected, isDark, onSelect, sheetRef }: keyboardBlurBehavior="restore" android_keyboardInputMode="adjustResize" backgroundStyle={isDark ? s.sheetDark : s.sheet} - handleIndicatorStyle={{ backgroundColor: isDark ? "#666666" : "#cccccc" }} + handleIndicatorStyle={{ backgroundColor: isDark ? "#9a9a9a" : "#cccccc" }} backdropComponent={(props) => ( )} @@ -105,7 +105,7 @@ export function ModelPicker({ providers, selected, isDark, onSelect, sheetRef }: {t("chat.reasoningBlock.label")} - + {expanded && ( diff --git a/src/components/chat/SelectableTextModal.tsx b/src/components/chat/SelectableTextModal.tsx new file mode 100644 index 00000000..6257a638 --- /dev/null +++ b/src/components/chat/SelectableTextModal.tsx @@ -0,0 +1,110 @@ +import { useState } from "react" +import { View, Text, Modal, ScrollView, TouchableOpacity, StyleSheet, useColorScheme } from "react-native" +import { Ionicons } from "@expo/vector-icons" +import { useSafeAreaInsets } from "react-native-safe-area-context" +import * as Clipboard from "expo-clipboard" +import { useTranslation } from "react-i18next" + +interface Props { + visible: boolean + text: string + onClose: () => void +} + +// Renders a message's source text in a fully `selectable` so the user +// can drag-select a portion of it and use the platform copy affordance. +// +// The critical detail is *where* this renders. Assistant prose in the chat +// transcript is a row of the session screen's inverted FlatList, and a +// `selectable` in that position hits facebook/react-native#46999 on +// Android — selection state (and accessibility-tree exposure) never applies +// correctly, which is exactly why Markdown.tsx's CustomRenderer strips the +// prop. A renders into its own host view outside that FlatList, so +// `selectable` behaves normally here. +export function SelectableTextModal({ visible, text, onClose }: Props) { + const isDark = useColorScheme() === "dark" + const { t } = useTranslation() + const insets = useSafeAreaInsets() + const [copied, setCopied] = useState(false) + + const copyAll = async () => { + try { + await Clipboard.setStringAsync(text) + setCopied(true) + setTimeout(() => setCopied(false), 2000) + } catch {} + } + + // onRequestClose covers the Android hardware/gesture back action, which + // would otherwise leave the modal stuck open. + return ( + + + + + {t("session.selectText.title")} + + + {copied ? t("session.selectText.copied") : t("session.selectText.copyAll")} + + + + + + + + + + {text} + + + + {/* Under edge-to-edge the sheet extends beneath the system + navigation bar, so a fixed paddingBottom leaves this hint drawn + behind it (verified on an Android 12 emulator). Pad by the real + bottom inset instead, with a floor so it still breathes on + devices reporting inset 0. */} + + {t("session.selectText.hint")} + + + + + ) +} + +const s = StyleSheet.create({ + backdrop: { flex: 1, backgroundColor: "rgba(0,0,0,0.5)", justifyContent: "flex-end" }, + sheet: { + backgroundColor: "#ffffff", + borderTopLeftRadius: 16, + borderTopRightRadius: 16, + maxHeight: "85%", + minHeight: "50%", + }, + sheetDark: { backgroundColor: "#141420" }, + + header: { + flexDirection: "row", + alignItems: "center", + justifyContent: "space-between", + paddingHorizontal: 16, + paddingVertical: 12, + borderBottomWidth: StyleSheet.hairlineWidth, + borderBottomColor: "#e5e5e5", + }, + headerDark: { borderBottomColor: "#2a2a2a" }, + headerActions: { flexDirection: "row", alignItems: "center", gap: 16 }, + title: { fontSize: 16, fontWeight: "600", color: "#0a0a0a" }, + titleDark: { color: "#ffffff" }, + copyBtn: { fontSize: 14, color: "#8b5cf6", fontWeight: "600" }, + + body: { flexGrow: 0 }, + bodyContent: { padding: 16 }, + text: { fontSize: 15, lineHeight: 22, color: "#0a0a0a" }, + textDark: { color: "#e5e5e5" }, + + // paddingBottom is applied inline from the safe-area inset — see render. + hint: { fontSize: 11, color: "#999999", textAlign: "center", paddingHorizontal: 16, paddingTop: 8 }, + hintDark: { color: "#9a9a9a" }, +}) diff --git a/src/components/chat/SessionInfo.tsx b/src/components/chat/SessionInfo.tsx index bb80b53b..56f98ccf 100644 --- a/src/components/chat/SessionInfo.tsx +++ b/src/components/chat/SessionInfo.tsx @@ -111,7 +111,7 @@ export function SessionInfo({ )} - + @@ -353,5 +353,5 @@ const s = StyleSheet.create({ fontWeight: "500", }, textDark: { color: "#e5e5e5" }, - dimDark: { color: "#666666" }, + dimDark: { color: "#9a9a9a" }, }) diff --git a/src/components/chat/SlashPopover.tsx b/src/components/chat/SlashPopover.tsx index 8760fc4f..a71efbc7 100644 --- a/src/components/chat/SlashPopover.tsx +++ b/src/components/chat/SlashPopover.tsx @@ -78,7 +78,7 @@ const s = StyleSheet.create({ trigger: { fontSize: 14, fontWeight: "600", color: "#0a0a0a" }, textWhite: { color: "#ffffff" }, desc: { fontSize: 12, color: "#999999", marginTop: 1 }, - metaDark: { color: "#666666" }, + metaDark: { color: "#9a9a9a" }, badge: { backgroundColor: "#f3e8ff", paddingHorizontal: 6, diff --git a/src/components/chat/ToolCallCard.tsx b/src/components/chat/ToolCallCard.tsx index b7361c11..fd43b235 100644 --- a/src/components/chat/ToolCallCard.tsx +++ b/src/components/chat/ToolCallCard.tsx @@ -220,7 +220,7 @@ function TodoDetail({ input, isDark }: { input: unknown; isDark: boolean }) { {String(item.content || item.title || "")} @@ -354,7 +354,7 @@ export function ToolCallCard({ tool, isDark }: Props) { )} @@ -397,7 +397,7 @@ const s = StyleSheet.create({ name: { fontSize: 13, fontWeight: "500", color: "#0a0a0a", flex: 1 }, nameDark: { color: "#e5e5e5" }, elapsed: { fontSize: 11, color: "#999999" }, - elapsedDark: { color: "#666666" }, + elapsedDark: { color: "#9a9a9a" }, // Error errorBanner: { diff --git a/src/components/chat/VariantPicker.tsx b/src/components/chat/VariantPicker.tsx index a1aad875..90cf3d2b 100644 --- a/src/components/chat/VariantPicker.tsx +++ b/src/components/chat/VariantPicker.tsx @@ -55,7 +55,7 @@ export function VariantPicker({ variants, selected, isDark, onSelect, sheetRef } enableDynamicSizing={false} enablePanDownToClose backgroundStyle={isDark ? s.sheetDark : s.sheet} - handleIndicatorStyle={{ backgroundColor: isDark ? "#666666" : "#cccccc" }} + handleIndicatorStyle={{ backgroundColor: isDark ? "#9a9a9a" : "#cccccc" }} backdropComponent={(props) => ( )} @@ -94,7 +94,7 @@ const s = StyleSheet.create({ header: { paddingHorizontal: 16, paddingBottom: 12 }, title: { fontSize: 18, fontWeight: "700", color: "#0a0a0a" }, textWhite: { color: "#ffffff" }, - metaDark: { color: "#666666" }, + metaDark: { color: "#9a9a9a" }, content: { paddingBottom: 40 }, row: { flexDirection: "row", diff --git a/src/components/chat/index.ts b/src/components/chat/index.ts index 4df6472f..f078e0d3 100644 --- a/src/components/chat/index.ts +++ b/src/components/chat/index.ts @@ -12,3 +12,4 @@ export { ImageAttachments, type Attachment } from "./ImageAttachments" export { DirectorySwitcher } from "./DirectorySwitcher" export { DirectoryBrowserSheet } from "./DirectoryBrowserSheet" export { SessionInfo } from "./SessionInfo" +export { SelectableTextModal } from "./SelectableTextModal" diff --git a/src/components/markdown/CodeBlock.tsx b/src/components/markdown/CodeBlock.tsx index 8265309f..e94c7eaf 100644 --- a/src/components/markdown/CodeBlock.tsx +++ b/src/components/markdown/CodeBlock.tsx @@ -1,7 +1,7 @@ import { useState } from "react" -import { View, Text, TouchableOpacity, StyleSheet, useColorScheme, Platform, ScrollView } from "react-native" +import { View, Text, TouchableOpacity, StyleSheet, useColorScheme, Platform } from "react-native" import * as Clipboard from "expo-clipboard" -import { WIDE_CONTENT_SCROLL_CONFIG } from "../../lib/scroll-config" +import { WideScroll } from "../WideScroll" interface Props { code: string @@ -28,11 +28,11 @@ export function CodeBlock({ code, language }: Props) { {copied ? "Copied!" : "Copy"} - + {code} - + ) } @@ -77,6 +77,10 @@ const styles = StyleSheet.create({ }, codeScroll: { padding: 12, + // Right breathing room INSIDE the scrollable content: without it the + // longest line ends flush against the clipped edge when scrolled fully, + // reading as cut off even when it isn't. + paddingRight: 24, }, code: { fontFamily: Platform.OS === "ios" ? "Menlo" : "monospace", diff --git a/src/components/markdown/Markdown.tsx b/src/components/markdown/Markdown.tsx index 1066ab0d..9ef29df7 100644 --- a/src/components/markdown/Markdown.tsx +++ b/src/components/markdown/Markdown.tsx @@ -2,6 +2,13 @@ import { useMemo, type ReactNode } from "react" import { View, Text, useColorScheme, Platform, type StyleProp, type ViewStyle, type TextStyle } from "react-native" import { useMarkdown, Renderer } from "react-native-marked" import { CodeBlock } from "./CodeBlock" +import { WideScroll } from "../WideScroll" +// Transitive deps of react-native-marked, used to mirror its own MDTable +// structure inside the forgiving scroller. +// eslint-disable-next-line import/no-extraneous-dependencies +import { Cell, Table, TableWrapper } from "react-native-reanimated-table" +// @ts-expect-error -- internal util of react-native-marked, no types exported +import { getTableWidthArr } from "react-native-marked/dist/module/utils/table" // react-native-marked's base Renderer hardcodes `selectable` on every plain // text node it produces (text/strong/em/del/heading/codespan). On Android, @@ -54,6 +61,48 @@ class CustomRenderer extends Renderer { codespan(text: string, styles?: TextStyle): ReactNode { return this.plainText(text, [styles, { fontStyle: "normal", fontWeight: "normal" }]) } + + // The library's MDTable wraps itself in a plain horizontal ScrollView, which + // loses the gesture race inside the vertical transcript unless the swipe is + // near-perfectly axis-aligned — the reported "have to hit a perfect + // left-right swipe". Re-render the same table structure inside WideScroll, + // whose pan claims on horizontal DOMINANCE instead. Also gives the content + // real edge padding, which the library's version lacked. + table( + header: ReactNode[][], + rows: ReactNode[][][], + tableStyle?: ViewStyle, + rowStyle?: ViewStyle, + cellStyle?: ViewStyle, + ): ReactNode { + // windowWidth is a private field on the base renderer; read it the same + // way its own table() does, typed around rather than through. + const windowWidth = (this as unknown as { windowWidth: number }).windowWidth + const widthArr = getTableWidthArr(header.length, windowWidth) + const { borderWidth, borderColor, ...tableStyleRest } = tableStyle || {} + return ( + + + + {header.map((headerCol, index) => ( + {headerCol}} /> + ))} + + {rows.map((rowData, rowIndex) => ( + + {rowData.map((cellData, cellIndex) => ( + {cellData}} + /> + ))} + + ))} +
+ + ) + } } const mono = Platform.OS === "ios" ? "Menlo" : "monospace" diff --git a/src/components/wide-content-scroll.regression.test.ts b/src/components/wide-content-scroll.regression.test.ts index 3e0edcd2..d5db2e93 100644 --- a/src/components/wide-content-scroll.regression.test.ts +++ b/src/components/wide-content-scroll.regression.test.ts @@ -25,21 +25,29 @@ function readComponent(relativePath: string): string { return readFileSync(path.join(dir, relativePath), "utf8") } -test("DiffView wraps diff lines in the shared horizontal-scroll ScrollView", () => { +// The shared container is now WideScroll, which is horizontal by +// construction (see src/components/WideScroll.tsx) and adds the forgiving +// gesture claim on top. The invariant these tests guard is unchanged: wide +// content must live in a horizontal scroller and must not be truncated. +test("DiffView wraps diff lines in the shared horizontal scroller", () => { const src = readComponent("chat/DiffView.tsx") - assert.match(src, / { +test("CodeBlock wraps code in the shared horizontal scroller", () => { const src = readComponent("markdown/CodeBlock.tsx") - assert.match(src, / { - const diffView = readComponent("chat/DiffView.tsx") - const codeBlock = readComponent("markdown/CodeBlock.tsx") - assert.match(diffView, /from ["']\.\.\/\.\.\/lib\/scroll-config["']/) - assert.match(codeBlock, /from ["']\.\.\/\.\.\/lib\/scroll-config["']/) +test("markdown tables use the shared horizontal scroller too", () => { + const src = readComponent("markdown/Markdown.tsx") + assert.match(src, / { + for (const rel of ["chat/DiffView.tsx", "markdown/CodeBlock.tsx", "markdown/Markdown.tsx"]) { + assert.match(readComponent(rel), /from ["']\.\.\/WideScroll["']/, rel) + } }) diff --git a/src/lib/auto-scroll.test.ts b/src/lib/auto-scroll.test.ts new file mode 100644 index 00000000..e81d3d07 --- /dev/null +++ b/src/lib/auto-scroll.test.ts @@ -0,0 +1,79 @@ +import { test } from "node:test" +import assert from "node:assert/strict" +import { + AT_BOTTOM_THRESHOLD_PX, + isAtBottom, + shouldAutoScroll, + shouldShowScrollButton, + transcriptSignature, +} from "./auto-scroll.ts" + +test("isAtBottom treats offset 0 (newest message, inverted list) as the bottom", () => { + assert.equal(isAtBottom(0), true) + assert.equal(isAtBottom(AT_BOTTOM_THRESHOLD_PX), true) + assert.equal(isAtBottom(AT_BOTTOM_THRESHOLD_PX + 1), false) +}) + +test("isAtBottom tolerates negative overscroll from bounce", () => { + assert.equal(isAtBottom(-40), true) +}) + +test("the scroll button appears exactly when auto-follow stops", () => { + for (const offset of [0, 50, 200, 201, 900]) { + assert.equal(shouldShowScrollButton(offset), !isAtBottom(offset), `offset ${offset}`) + } +}) + +// The reported bug: new content arrived and the view did not follow it. +test("scrolls when new content arrives and the user is at the bottom", () => { + assert.equal( + shouldAutoScroll({ offsetY: 0, previousSignature: "3:100", currentSignature: "4:0" }), + true, + ) +}) + +test("follows a streaming reply as it grows, not just on completion", () => { + assert.equal( + shouldAutoScroll({ offsetY: 10, previousSignature: "4:120", currentSignature: "4:260" }), + true, + ) +}) + +// The usual bug in naive fixes: yanking a reader back down. +test("does NOT scroll when the user has scrolled up to read history", () => { + assert.equal( + shouldAutoScroll({ offsetY: 5000, previousSignature: "3:100", currentSignature: "4:0" }), + false, + ) +}) + +test("does not scroll when content is unchanged, so it can't fight a gesture", () => { + assert.equal( + shouldAutoScroll({ offsetY: 0, previousSignature: "4:260", currentSignature: "4:260" }), + false, + ) +}) + +test("scrolls on the very first content (no previous signature)", () => { + assert.equal( + shouldAutoScroll({ offsetY: 0, previousSignature: null, currentSignature: "1:0" }), + true, + ) +}) + +test("a custom threshold is honoured", () => { + assert.equal( + shouldAutoScroll({ offsetY: 300, previousSignature: "1:0", currentSignature: "2:0", threshold: 500 }), + true, + ) + assert.equal( + shouldAutoScroll({ offsetY: 300, previousSignature: "1:0", currentSignature: "2:0", threshold: 100 }), + false, + ) +}) + +test("transcriptSignature changes on new messages and on streaming growth", () => { + assert.notEqual(transcriptSignature(3, 100), transcriptSignature(4, 100)) + assert.notEqual(transcriptSignature(4, 100), transcriptSignature(4, 250)) + assert.equal(transcriptSignature(4, 100), transcriptSignature(4, 100)) +}) diff --git a/src/lib/auto-scroll.ts b/src/lib/auto-scroll.ts new file mode 100644 index 00000000..f892bada --- /dev/null +++ b/src/lib/auto-scroll.ts @@ -0,0 +1,65 @@ +// Auto-scroll policy for the session transcript. +// +// The transcript is an *inverted* FlatList, so contentOffset.y === 0 is the +// newest message ("the bottom" visually) and larger offsets mean the user has +// scrolled back through history. +// +// Two things conspired to make new content unreachable (issue #155, +// "Message cannot be scrolled automatically"): +// +// 1. scrollToBottom() was only ever wired to the manual scroll-to-bottom +// button. Nothing scrolled when a message arrived or while one streamed. +// 2. maintainVisibleContentPosition={{ minIndexForVisible: 0 }} asks the list +// to hold currently-visible items still when the data changes. New +// messages are inserted at index 0 of the inverted list, so that setting +// actively compensates the offset to keep the older items stationary — +// parking the new message just outside the viewport. +// +// (2) is worth keeping: it's what stops the view jumping when older pages load +// via onEndReached. So the fix is to scroll explicitly when new content +// arrives — but only when the user is already at the bottom. Someone who has +// scrolled up to read history must not be yanked back down mid-sentence, +// which is the usual bug in naive "always scroll on new message" fixes. +// +// Dependency-free so it's testable under plain `node --test`. + +// How far from the newest message still counts as "following along". Also the +// threshold at which the scroll-to-bottom button appears, so the button shows +// exactly when auto-follow stops — one number, no drift between the two. +export const AT_BOTTOM_THRESHOLD_PX = 200 + +export function isAtBottom(offsetY: number, threshold: number = AT_BOTTOM_THRESHOLD_PX): boolean { + // Guard against overscroll/bounce producing small negative offsets. + return offsetY <= threshold +} + +export function shouldShowScrollButton(offsetY: number, threshold: number = AT_BOTTOM_THRESHOLD_PX): boolean { + return !isAtBottom(offsetY, threshold) +} + +/** + * Should the transcript scroll itself to the newest message? + * + * Only when the content actually changed *and* the user was already following + * along at the bottom. `contentSignature` is any value that changes as content + * does — message count plus the streaming message's length — so a re-render + * with unchanged content doesn't re-scroll and fight a user's own gesture. + */ +export function shouldAutoScroll(input: { + offsetY: number + previousSignature: string | null + currentSignature: string + threshold?: number +}): boolean { + if (input.previousSignature === input.currentSignature) return false + return isAtBottom(input.offsetY, input.threshold ?? AT_BOTTOM_THRESHOLD_PX) +} + +/** + * A value that changes whenever the transcript gains content: the number of + * messages, plus the size of the newest one so a streaming reply keeps the + * view pinned as it grows rather than only when it completes. + */ +export function transcriptSignature(messageCount: number, newestMessageLength: number): string { + return `${messageCount}:${newestMessageLength}` +} diff --git a/src/lib/horizontal-intent.test.ts b/src/lib/horizontal-intent.test.ts new file mode 100644 index 00000000..adbdcfb4 --- /dev/null +++ b/src/lib/horizontal-intent.test.ts @@ -0,0 +1,72 @@ +import { test } from "node:test" +import assert from "node:assert/strict" +import { + VERTICAL_FAIL_DY, + CLAIM_MIN_DX, + DOMINANCE_RATIO, + dragOffset, + flingTarget, + isHorizontalIntent, +} from "./horizontal-intent.ts" + +// --- claiming --- + +test("a clean sideways drag claims", () => { + assert.equal(isHorizontalIntent(20, 0), true) + assert.equal(isHorizontalIntent(-20, 2), true) +}) + +// The reported annoyance: only near-perfect axis swipes worked. A diagonal +// that is merely horizontal-dominant must claim too. +test("a sloppy diagonal still claims when horizontal dominates", () => { + assert.equal(isHorizontalIntent(20, 18), true) // ~42° off axis + assert.equal(isHorizontalIntent(-15, 12), true) +}) + +test("a vertical-dominant drag is left to the transcript", () => { + assert.equal(isHorizontalIntent(5, 30), false) + assert.equal(isHorizontalIntent(10, 20), false) +}) + +test("jitter below the minimum never claims", () => { + assert.equal(isHorizontalIntent(CLAIM_MIN_DX - 1, 0), false) + assert.equal(isHorizontalIntent(0, 0), false) +}) + +test("the dominance ratio is forgiving, not absolute", () => { + // dx exactly at the ratio boundary of dy fails; just above passes. + const dy = 20 + const boundary = dy * DOMINANCE_RATIO + assert.equal(isHorizontalIntent(boundary, dy), false) + assert.equal(isHorizontalIntent(boundary + 0.1, dy), true) +}) + +// --- drag --- + +test("dragging right moves content left and clamps at zero", () => { + assert.equal(dragOffset(50, 30, 500), 20) + assert.equal(dragOffset(50, 200, 500), 0) +}) + +test("dragging left clamps at the end of content", () => { + assert.equal(dragOffset(450, -30, 500), 480) + assert.equal(dragOffset(450, -200, 500), 500) +}) + +// --- fling --- + +test("a flick projects past the finger, clamped to the edges", () => { + const target = flingTarget(100, -1, 500) // slow leftward flick + assert.ok(target > 100 && target <= 500) + assert.equal(flingTarget(100, -100, 500), 500) + assert.equal(flingTarget(100, 100, 500), 0) +}) + +test("zero velocity stays put", () => { + assert.equal(flingTarget(120, 0, 500), 120) +}) + +test("the native thresholds keep the dominance shape: sideways is easier than vertical", () => { + assert.ok(VERTICAL_FAIL_DY > CLAIM_MIN_DX, "a drag must be able to drift more vertically than the horizontal trigger") + assert.ok(VERTICAL_FAIL_DY <= 20, "but not so much that clear vertical scrolls get eaten") +}) diff --git a/src/lib/horizontal-intent.ts b/src/lib/horizontal-intent.ts new file mode 100644 index 00000000..81c41dd2 --- /dev/null +++ b/src/lib/horizontal-intent.ts @@ -0,0 +1,56 @@ +// When does a drag MEAN sideways? +// +// A horizontal ScrollView nested in the vertical transcript loses the gesture +// race unless the swipe is almost perfectly axis-aligned: both claim at ~10dp +// of movement on their own axis, so any diagonal drift hands the touch to the +// vertical list. In practice that means "hit a perfect left-right swipe or +// the page scrolls instead" — the reported annoyance. +// +// These predicates define a forgiving claim: horizontal wins when the drag is +// merely horizontal-DOMINANT, not horizontal-pure. Pure so the thresholds are +// testable under plain `node --test`. + +/** Movement below this is jitter, not intent. */ +export const CLAIM_MIN_DX = 6 + +/** + * How much steeper than 45° a drag may be and still count as sideways. + * 0.7 means dx only has to beat 70% of dy — a drag up to ~55° off-axis still + * reads as horizontal. The vertical list keeps everything steeper. + */ +export const DOMINANCE_RATIO = 0.7 + +export function isHorizontalIntent(dx: number, dy: number): boolean { + const absDx = Math.abs(dx) + return absDx >= CLAIM_MIN_DX && absDx > Math.abs(dy) * DOMINANCE_RATIO +} + +/** + * Flick target for release momentum. + * + * Manual pan-driving loses native fling physics, so approximate: project the + * release velocity over a fixed time slice. 180ms of projected travel feels + * like a natural flick without letting a hard swipe teleport the view. + */ +export const FLING_PROJECTION_MS = 180 + +export function flingTarget(currentOffset: number, velocityX: number, maxOffset: number): number { + const projected = currentOffset - velocityX * FLING_PROJECTION_MS + return Math.max(0, Math.min(maxOffset, projected)) +} + +/** Clamp a live drag so pulling past the edges doesn't scroll into nothing. */ +export function dragOffset(startOffset: number, dx: number, maxOffset: number): number { + return Math.max(0, Math.min(maxOffset, startOffset - dx)) +} + +/** + * How far a drag may travel vertically before the claim FAILS to the + * vertical list, for the native-side gesture (react-native-gesture-handler + * activeOffsetX/failOffsetY pair). Together with CLAIM_MIN_DX this encodes + * the same dominance idea natively: horizontal wins if dx crosses its + * threshold before dy crosses this one. Native arbitration is what the JS + * PanResponder version lacked — Android lists intercept in the native + * layer, so a JS-level claim landed too late and the page scrolled anyway. + */ +export const VERTICAL_FAIL_DY = 14 diff --git a/src/lib/i18n/en.json b/src/lib/i18n/en.json index 13519f23..113ddada 100644 --- a/src/lib/i18n/en.json +++ b/src/lib/i18n/en.json @@ -95,7 +95,9 @@ }, "actions": { "editMessage": "Edit message", - "replace": "Replace" + "replace": "Replace", + "copyMessage": "Copy message", + "selectText": "Select text" }, "alerts": { "messageActionsTitle": "Message actions", @@ -123,6 +125,12 @@ "imageFailedMessage": "One or more images could not be processed. Please try a different photo.", "speechErrorTitle": "Voice input failed", "speechErrorMessage": "Could not use voice input. Check your microphone permission and try again." + }, + "selectText": { + "title": "Select text", + "copyAll": "Copy all", + "copied": "Copied!", + "hint": "Press and hold the text to select part of it." } }, "connection": { diff --git a/src/lib/i18n/zh-Hans.json b/src/lib/i18n/zh-Hans.json index 9ba906f7..537c9b82 100644 --- a/src/lib/i18n/zh-Hans.json +++ b/src/lib/i18n/zh-Hans.json @@ -95,7 +95,9 @@ }, "actions": { "editMessage": "编辑消息", - "replace": "替换" + "replace": "替换", + "copyMessage": "复制消息", + "selectText": "选择文本" }, "alerts": { "messageActionsTitle": "消息操作", @@ -123,6 +125,12 @@ "imageFailedMessage": "一张或多张图片无法处理。请尝试其他照片。", "speechErrorTitle": "语音输入失败", "speechErrorMessage": "无法使用语音输入。请检查麦克风权限后重试。" + }, + "selectText": { + "title": "选择文本", + "copyAll": "全部复制", + "copied": "已复制！", + "hint": "长按文本可选择其中一部分。" } }, "connection": { diff --git a/src/lib/keyboard-offset.test.ts b/src/lib/keyboard-offset.test.ts new file mode 100644 index 00000000..23b13856 --- /dev/null +++ b/src/lib/keyboard-offset.test.ts @@ -0,0 +1,37 @@ +import { test } from "node:test" +import assert from "node:assert/strict" +import { IOS_KEYBOARD_VERTICAL_OFFSET, keyboardVerticalOffset } from "./keyboard-offset.ts" + +test("iOS keeps its existing empirical offset regardless of inset", () => { + assert.equal(keyboardVerticalOffset("ios", 0), IOS_KEYBOARD_VERTICAL_OFFSET) + assert.equal(keyboardVerticalOffset("ios", 48.857), IOS_KEYBOARD_VERTICAL_OFFSET) +}) + +test("Android offsets by the status-bar inset to reconcile window vs screen coords", () => { + // Measured on an Android 12 emulator: this exact value closes the 48.857dp + // shortfall that left the composer behind the keyboard. + assert.equal(keyboardVerticalOffset("android", 48.857), 48.857) +}) + +test("Android with no status-bar inset needs no correction", () => { + assert.equal(keyboardVerticalOffset("android", 0), 0) +}) + +test("a negative/bogus inset never pushes content down", () => { + assert.equal(keyboardVerticalOffset("android", -20), 0) +}) + +// Regression guard for the arithmetic the offset exists to fix. +test("offset makes computed padding equal the real keyboard height", () => { + const windowBottom = 748.857 // frame.y + frame.height, window coords + const keyboardScreenY = 511.714 // screen coords + const keyboardHeight = 286 + const insetTop = 48.857 + + const withoutFix = windowBottom - keyboardScreenY + assert.ok(withoutFix < keyboardHeight, "precondition: unfixed padding is short") + + const offset = keyboardVerticalOffset("android", insetTop) + const withFix = windowBottom - (keyboardScreenY - offset) + assert.ok(Math.abs(withFix - keyboardHeight) < 0.01, `expected ~${keyboardHeight}, got ${withFix}`) +}) diff --git a/src/lib/keyboard-offset.ts b/src/lib/keyboard-offset.ts new file mode 100644 index 00000000..ddd25229 --- /dev/null +++ b/src/lib/keyboard-offset.ts @@ -0,0 +1,48 @@ +// KeyboardAvoidingView's `keyboardVerticalOffset`, per platform. +// +// Why Android needs a non-zero value under edge-to-edge: +// +// RN's KeyboardAvoidingView derives its padding from +// +// keyboardY = keyboardFrame.screenY - keyboardVerticalOffset +// padding = max(frame.y + frame.height - keyboardY, 0) +// +// `frame` comes from the view's own onLayout, which is in **window** +// coordinates — the origin sits below the status bar. `keyboardFrame.screenY` +// is in **screen** coordinates, measured from the true top of the display. +// Before Expo's mandatory edge-to-edge those two origins coincided, because +// the app window started below the status bar and the OS resized it +// (adjustResize) when the keyboard opened. Under edge-to-edge the window spans +// the full display, the two spaces no longer agree, and the computed padding +// comes up short by exactly the status-bar inset. +// +// Measured on an Android 12 emulator (Pixel 3 XL profile, scale 3.5), keyboard +// open on the session screen: +// +// screen height 845.71 dp +// window height 748.86 dp +// insets.top 48.86 dp insets.bottom 48 dp +// keyboard screenY 511.71 dp height 286 dp +// +// computed padding = 748.86 - 511.71 = 237.14 dp +// required padding = 286.00 dp (the real keyboard) +// shortfall = 48.86 dp === insets.top +// +// 48.86 dp x 3.5 = 171 px, which is the composer row — hence "the input box is +// invisible" (#156, and #53/#147 before it). Those earlier reports were each +// answered with a different `behavior=` value; none addressed the coordinate +// mismatch, which is why the bug kept coming back. +// +// Adding `insets.top` to keyboardVerticalOffset re-aligns the two spaces: +// padding becomes 237.14 + 48.86 = 286, exactly the keyboard height. +// +// iOS keeps its existing empirical 90 — it does not have this mismatch, and +// changing it is out of scope for this fix. + +export const IOS_KEYBOARD_VERTICAL_OFFSET = 90 + +export function keyboardVerticalOffset(platform: string, insetTop: number): number { + if (platform === "ios") return IOS_KEYBOARD_VERTICAL_OFFSET + // Guard against a bogus/unmeasured inset so we never push content *down*. + return Math.max(0, insetTop) +} diff --git a/src/lib/message-copy-text.test.ts b/src/lib/message-copy-text.test.ts new file mode 100644 index 00000000..14e55926 --- /dev/null +++ b/src/lib/message-copy-text.test.ts @@ -0,0 +1,64 @@ +import { test } from "node:test" +import assert from "node:assert/strict" +import { extractCopyText, extractReasoningText, hasCopyableText } from "./message-copy-text.ts" +import type { Part } from "./sdk.ts" + +test("extractCopyText: joins multiple text parts with newlines", () => { + const parts: Part[] = [ + { id: "p1", messageID: "m1", type: "text", text: "hello" }, + { id: "p2", messageID: "m1", type: "text", text: "world" }, + ] + assert.equal(extractCopyText(parts), "hello\nworld") +}) + +test("extractCopyText: preserves markdown source rather than rendered output", () => { + const parts: Part[] = [{ id: "p1", messageID: "m1", type: "text", text: "# Title\n\n**bold** and `code`" }] + assert.equal(extractCopyText(parts), "# Title\n\n**bold** and `code`") +}) + +test("extractCopyText: excludes reasoning and tool parts", () => { + const parts: Part[] = [ + { id: "p1", messageID: "m1", type: "reasoning", text: "thinking..." }, + { id: "p2", messageID: "m1", type: "tool", tool: "bash" }, + { id: "p3", messageID: "m1", type: "text", text: "final answer" }, + ] + assert.equal(extractCopyText(parts), "final answer") +}) + +test("extractCopyText: skips text parts with empty/missing text", () => { + const parts: Part[] = [ + { id: "p1", messageID: "m1", type: "text", text: "" }, + { id: "p2", messageID: "m1", type: "text" }, + { id: "p3", messageID: "m1", type: "text", text: "kept" }, + ] + assert.equal(extractCopyText(parts), "kept") +}) + +test("extractCopyText: tolerates undefined and empty parts", () => { + assert.equal(extractCopyText(undefined), "") + assert.equal(extractCopyText([]), "") +}) + +test("extractReasoningText: collects only reasoning parts", () => { + const parts: Part[] = [ + { id: "p1", messageID: "m1", type: "reasoning", text: "step one" }, + { id: "p2", messageID: "m1", type: "reasoning", text: "step two" }, + { id: "p3", messageID: "m1", type: "text", text: "answer" }, + ] + assert.equal(extractReasoningText(parts), "step one\nstep two") +}) + +test("hasCopyableText: false for tool-only, whitespace-only, empty and undefined", () => { + assert.equal(hasCopyableText([{ id: "p1", messageID: "m1", type: "tool", tool: "bash" }]), false) + assert.equal(hasCopyableText([{ id: "p1", messageID: "m1", type: "text", text: " \n\t " }]), false) + assert.equal(hasCopyableText([]), false) + assert.equal(hasCopyableText(undefined), false) +}) + +test("hasCopyableText: true when any text part has content", () => { + const parts: Part[] = [ + { id: "p1", messageID: "m1", type: "tool", tool: "bash" }, + { id: "p2", messageID: "m1", type: "text", text: "answer" }, + ] + assert.equal(hasCopyableText(parts), true) +}) diff --git a/src/lib/message-copy-text.ts b/src/lib/message-copy-text.ts new file mode 100644 index 00000000..7b43e2e4 --- /dev/null +++ b/src/lib/message-copy-text.ts @@ -0,0 +1,39 @@ +// Pure helper: turn a message's parts into the plain text a user would +// expect "Copy message" to put on the clipboard. +// +// Why this exists: assistant prose is rendered through +// src/components/markdown/Markdown.tsx, whose CustomRenderer deliberately +// strips react-native-marked's `selectable` prop from every plain-text node +// to dodge facebook/react-native#46999 (selectable inside a FlatList +// row misapplies selection state on Android). That left assistant replies +// with no copy path at all — code blocks have CodeBlock's Copy button and +// user messages are plainly `selectable`, but prose had nothing. +// +// Rather than re-enabling `selectable` inside the FlatList row (which is what +// the RN bug punishes), the copy path reads the source text straight from the +// parts. Kept dependency-free so it's testable under plain `node --test`. +import type { Part } from "./sdk" + +// Text and reasoning are the two part types rendered as prose. Reasoning is +// visually collapsible (ReasoningBlock) and is not what someone means by +// "copy this message", so it is excluded by default and offered separately. +export function extractCopyText(parts: Part[] | undefined): string { + return (parts || []) + .filter((p) => p.type === "text" && p.text) + .map((p) => p.text) + .join("\n") +} + +export function extractReasoningText(parts: Part[] | undefined): string { + return (parts || []) + .filter((p) => p.type === "reasoning" && p.text) + .map((p) => p.text) + .join("\n") +} + +// True when there is anything worth offering a copy/select action for. +// Guards the long-press handler so an empty or tool-only message doesn't +// open an action sheet whose actions would all be no-ops. +export function hasCopyableText(parts: Part[] | undefined): boolean { + return extractCopyText(parts).trim().length > 0 +} diff --git a/src/lib/scroll-config.test.ts b/src/lib/scroll-config.test.ts deleted file mode 100644 index 8e1739e5..00000000 --- a/src/lib/scroll-config.test.ts +++ /dev/null @@ -1,26 +0,0 @@ -import { test } from "node:test" -import assert from "node:assert/strict" -import { WIDE_CONTENT_SCROLL_CONFIG } from "./scroll-config.ts" - -// GitHub issue #21: DiffView + CodeBlock must render wide content in a -// horizontally-scrollable container (not wrapped, not truncated). Both -// components spread WIDE_CONTENT_SCROLL_CONFIG onto their ScrollView (see -// src/components/chat/DiffView.tsx and src/components/markdown/CodeBlock.tsx) -// so asserting on this object here is asserting on the actual runtime props, -// not a parallel copy that can drift out of sync. - -test("wide-content scroll config enables horizontal scrolling", () => { - assert.equal(WIDE_CONTENT_SCROLL_CONFIG.horizontal, true) -}) - -test("wide-content scroll config shows the horizontal scroll indicator", () => { - // Regression guard: a container that scrolls but hides its indicator is - // easy to mistake for content that simply doesn't overflow. Keep the - // indicator visible so on-device QA (and screenshots) can tell scrollable - // content apart from clipped/truncated content. - assert.equal(WIDE_CONTENT_SCROLL_CONFIG.showsHorizontalScrollIndicator, true) -}) - -test("wide-content scroll config has no unexpected keys", () => { - assert.deepEqual(Object.keys(WIDE_CONTENT_SCROLL_CONFIG).sort(), ["horizontal", "showsHorizontalScrollIndicator"]) -}) diff --git a/src/lib/scroll-config.ts b/src/lib/scroll-config.ts deleted file mode 100644 index b915ede1..00000000 --- a/src/lib/scroll-config.ts +++ /dev/null @@ -1,18 +0,0 @@ -// Pure (no React Native imports) horizontal-scroll configuration shared by -// src/components/chat/DiffView.tsx and src/components/markdown/CodeBlock.tsx. -// -// GitHub issue #21: wide diff lines and wide code-block lines must render in -// a horizontally-scrollable container instead of being wrap-broken or -// truncated with `numberOfLines`. Centralizing the actual runtime props in -// one plain object lets that decision be unit-tested with node:test (no React -// Native renderer needed) while both components spread the SAME object onto -// their ScrollView, so the test and the real components can't drift apart. -export interface HorizontalScrollConfig { - horizontal: true - showsHorizontalScrollIndicator: boolean -} - -export const WIDE_CONTENT_SCROLL_CONFIG: HorizontalScrollConfig = { - horizontal: true, - showsHorizontalScrollIndicator: true, -} diff --git a/src/lib/sdk.ts b/src/lib/sdk.ts index 89ffb68c..a771515b 100644 --- a/src/lib/sdk.ts +++ b/src/lib/sdk.ts @@ -7,6 +7,7 @@ import { buildRequestHeaders } from "./headers" import { SSEParser } from "./sse" import { apiErrorFor } from "./api-error" import { loadSessionList } from "./session-list" +import { LIVENESS_TIMEOUT_MS } from "./sse-liveness" import type { FileRoot } from "./file-roots" export { ApiAuthError, isAuthError } from "./api-error" @@ -245,6 +246,26 @@ async function fetchWithTimeout(url: string, options: RequestInit = {}, timeoutM } } +// Races a stream read against a deadline. Rejecting (rather than returning a +// sentinel) keeps the failure on the same path as a genuine transport error, so +// the caller's reconnect logic needs no special case. +async function readWithTimeout( + reader: { read: () => Promise<{ done: boolean; value?: T }> }, + timeoutMs: number, +): Promise<{ done: boolean; value?: T }> { + let timer: ReturnType | undefined + try { + return await Promise.race([ + reader.read(), + new Promise((_, reject) => { + timer = setTimeout(() => reject(new Error(`SSE stream idle for ${timeoutMs}ms`)), timeoutMs) + }), + ]) + } finally { + if (timer) clearTimeout(timer) + } +} + export function createClient(config: ClientConfig) { // Normalize once: a trailing slash on baseUrl (e.g. pasted into Advanced // mode or the Edit screen) would otherwise survive into every @@ -280,7 +301,14 @@ export function createClient(config: ClientConfig) { let receivedFirstByte = false try { while (true) { - const { done, value } = await reader.read() + // Bound the read. A half-open socket -- routine when a phone moves + // between Wi-Fi and cellular, or wakes from doze -- yields no bytes, + // no `done` and no error, so an unbounded `reader.read()` parks + // forever and nothing ever triggers a reconnect. The server + // heartbeats every ~10s, so silence past LIVENESS_TIMEOUT_MS is + // evidence of a dead stream rather than an idle one. Throwing here + // hands control to the caller's existing reconnect path. + const { done, value } = await readWithTimeout(reader, LIVENESS_TIMEOUT_MS) if (done) { console.log("[SSE] stream ended") break @@ -354,8 +382,12 @@ export function createClient(config: ClientConfig) { list: (params?: { roots?: boolean; limit?: number; search?: string }): Promise => loadSessionList( { - getExperimental: async (): Promise => { - const response = await fetchWithTimeout(`${config.baseUrl}/experimental/session`, { + // One page per call; loadSessionList drives the cursor loop. The + // endpoint defaults to limit=100 and silently truncates, so the + // "no params = everything" assumption this code used to make + // dropped every session past #100 by recency. + getExperimental: async (query) => { + const response = await fetchWithTimeout(`${config.baseUrl}/experimental/session${query}`, { headers: createHeaders(config), }) // Older servers lack this route — signal fallback to legacy /session. @@ -364,7 +396,12 @@ export function createClient(config: ClientConfig) { const body = await response.text() throw apiErrorFor(response.status, `API Error: ${response.status} - ${body}`) } - return response.json() + const cursorHeader = response.headers.get("x-next-cursor") + const nextCursor = cursorHeader != null ? Number(cursorHeader) : undefined + return { + sessions: await response.json(), + nextCursor: Number.isFinite(nextCursor as number) ? nextCursor : undefined, + } }, getLegacy: (query) => request(config, `/session${query}`), }, diff --git a/src/lib/session-list.integration.test.ts b/src/lib/session-list.integration.test.ts index 28583708..17a28571 100644 --- a/src/lib/session-list.integration.test.ts +++ b/src/lib/session-list.integration.test.ts @@ -35,11 +35,13 @@ after(async () => { // experimental endpoint, signal fallback (null) only on 404. function realTransport(baseUrl: string): SessionListTransport { return { - getExperimental: async () => { - const r = await fetch(`${baseUrl}/experimental/session`, { headers: { Accept: "application/json" } }) + getExperimental: async (query: string) => { + const r = await fetch(`${baseUrl}/experimental/session${query}`, { headers: { Accept: "application/json" } }) if (r.status === 404) return null if (!r.ok) throw new Error(`HTTP ${r.status}`) - return r.json() + const cursorHeader = r.headers.get("x-next-cursor") + const nextCursor = cursorHeader != null ? Number(cursorHeader) : undefined + return { sessions: await r.json(), nextCursor: Number.isFinite(nextCursor as number) ? nextCursor : undefined } }, getLegacy: async (query) => { const r = await fetch(`${baseUrl}/session${query}`, { headers: { Accept: "application/json" } }) diff --git a/src/lib/session-list.test.ts b/src/lib/session-list.test.ts index 4b3fbb4a..d27080f5 100644 --- a/src/lib/session-list.test.ts +++ b/src/lib/session-list.test.ts @@ -28,10 +28,11 @@ function transport(opts: { const calls: string[] = [] return { calls, - getExperimental: async () => { - calls.push("experimental") + getExperimental: async (query: string) => { + calls.push(`experimental${query}`) if (opts.experimentalThrows) throw opts.experimentalThrows - return opts.experimental === undefined ? [] : opts.experimental + if (opts.experimental === null) return null + return { sessions: opts.experimental === undefined ? [] : opts.experimental } }, getLegacy: async (query: string) => { calls.push(`legacy${query}`) @@ -43,7 +44,7 @@ function transport(opts: { test("loadSessionList: (a) calls /experimental/session first (global)", async () => { const t = transport({ experimental: [session({ id: "a" })] }) await loadSessionList(t, { roots: true, limit: 50 }) - assert.equal(t.calls[0], "experimental") + assert.ok(t.calls[0].startsWith("experimental"), "first call goes to the experimental endpoint") assert.ok(!t.calls.some((c) => c.startsWith("legacy")), "must not hit legacy /session when experimental works") }) @@ -74,7 +75,7 @@ test("loadSessionList: (c) falls back to /session on 404 (older server)", async const legacy = [session({ id: "legacy-a" })] const t = transport({ experimental: null, legacy }) const out = await loadSessionList(t, { roots: true, limit: 50 }) - assert.equal(t.calls[0], "experimental") + assert.ok(t.calls[0].startsWith("experimental")) assert.equal(t.calls[1], "legacy?roots=true&limit=50", "must call legacy with preserved query params") assert.deepEqual(out, legacy, "returns the legacy payload unchanged") }) @@ -128,3 +129,38 @@ test("legacySessionQuery: builds the same query the old code sent", () => { assert.equal(legacySessionQuery({}), "") assert.equal(legacySessionQuery({ search: "x" }), "?search=x") }) + +// The missing-sessions bug, round two: the endpoint DEFAULTS to limit=100 +// and silently truncates to the newest sessions. The loop must follow +// x-next-cursor until exhausted, so session #138 by recency exists on the +// phone at all. +test("loadSessionList: pages through x-next-cursor until exhausted", async () => { + const calls: string[] = [] + const pageA = [session({ id: "a1", updated: 300 }), session({ id: "a2", updated: 200 })] + const pageB = [session({ id: "a2", updated: 200 }), session({ id: "b1", updated: 100 })] // boundary dupe + const t: SessionListTransport = { + getExperimental: async (query: string) => { + calls.push(query) + if (!query.includes("cursor")) return { sessions: pageA, nextCursor: 200 } + return { sessions: pageB } // final page: no cursor + }, + getLegacy: async () => [], + } + const out = await loadSessionList(t, {}) + assert.deepEqual(out.map((s) => s.id), ["a1", "a2", "b1"], "all pages merged, boundary duplicate dropped") + assert.equal(calls.length, 2) + assert.ok(calls[1].includes("cursor=200")) +}) + +test("loadSessionList: a stuck cursor cannot loop forever", async () => { + let count = 0 + const t: SessionListTransport = { + getExperimental: async () => { + count++ + return { sessions: [session({ id: `s${count}`, updated: 1 })], nextCursor: 42 } // never advances past 42 + }, + getLegacy: async () => [], + } + await loadSessionList(t, {}) + assert.ok(count <= 3, `stuck cursor stopped after ${count} calls`) +}) diff --git a/src/lib/session-list.ts b/src/lib/session-list.ts index 051e2e10..4aa7851a 100644 --- a/src/lib/session-list.ts +++ b/src/lib/session-list.ts @@ -16,14 +16,26 @@ export interface SessionListParams { search?: string } +// One page of the global list. The endpoint DEFAULTS to limit=100 and +// silently truncates to the newest sessions — fetching "with no params" made +// the client believe it had everything while anything past #100 by recency +// simply did not exist on the phone (the missing-sessions bug, round two: +// the first round was a client-side slice, this one is the server's cap). +// The cure is the endpoint's own x-next-cursor pagination, looped until +// exhausted below. +export interface SessionListPage { + sessions: Session[] + // time.updated of the last row, from the x-next-cursor header; absent on + // the final page. + nextCursor?: number +} + export interface SessionListTransport { - // GET /experimental/session with NO query params — the server applies `limit` - // BEFORE we can filter to roots, so limiting server-side would truncate the - // pool and yield too few root sessions. We fetch the full global list and - // shape it client-side via normalizeSessions. Resolves to null when the route - // is absent (HTTP 404 on older servers) so we fall back to the legacy path. - // Any other non-2xx is thrown by the transport (parity with request()). - getExperimental: () => Promise + // GET /experimental/session for ONE page (query carries limit + cursor). + // Resolves null when the route is absent (HTTP 404 on older servers) so we + // fall back to the legacy path. Any other non-2xx is thrown by the + // transport (parity with request()). + getExperimental: (query: string) => Promise // Legacy directory-scoped GET /session, used only when the experimental // route is absent. Its behavior is unchanged from before this feature. getLegacy: (query: string) => Promise @@ -56,7 +68,21 @@ export function legacySessionQuery(params?: SessionListParams): string { return qs ? `?${qs}` : "" } -// List sessions globally: prefer /experimental/session (all directories), shape +// Page size per request and the loop's hard ceiling. 10 pages of 200 is +// 2000 sessions — far past any real farm today, while still bounding a +// misbehaving server that returns a cursor forever. +export const GLOBAL_PAGE_LIMIT = 200 +export const MAX_GLOBAL_PAGES = 10 + +export function experimentalPageQuery(cursor?: number): string { + const query = new URLSearchParams() + query.set("limit", String(GLOBAL_PAGE_LIMIT)) + if (cursor != null) query.set("cursor", String(cursor)) + return `?${query.toString()}` +} + +// List sessions globally: prefer /experimental/session (all directories), +// PAGING through the server's x-next-cursor until exhausted, shape // client-side, and fall back to the legacy /session path only when the // experimental route is absent (transport resolves null on 404). Any other // non-2xx is surfaced by the transport, exactly as before this feature. @@ -64,7 +90,29 @@ export async function loadSessionList( transport: SessionListTransport, params?: SessionListParams, ): Promise { - const all = await transport.getExperimental() - if (all === null) return transport.getLegacy(legacySessionQuery(params)) + const first = await transport.getExperimental(experimentalPageQuery()) + if (first === null) return transport.getLegacy(legacySessionQuery(params)) + + const seen = new Set() + const all: Session[] = [] + const push = (page: Session[]) => { + // Cursor boundaries can duplicate rows on time.updated ties; dedupe by id. + for (const session of page) { + if (!seen.has(session.id)) { + seen.add(session.id) + all.push(session) + } + } + } + push(first.sessions) + let cursor = first.nextCursor + for (let pageIndex = 1; cursor != null && pageIndex < MAX_GLOBAL_PAGES; pageIndex++) { + const page = await transport.getExperimental(experimentalPageQuery(cursor)) + if (page === null) break // route vanished mid-loop: keep what we have + push(page.sessions) + // A cursor that does not advance would loop forever; treat as final. + if (page.nextCursor == null || page.nextCursor === cursor) break + cursor = page.nextCursor + } return normalizeSessions(all, params) } diff --git a/src/lib/sse-liveness.test.ts b/src/lib/sse-liveness.test.ts new file mode 100644 index 00000000..9f4c0c61 --- /dev/null +++ b/src/lib/sse-liveness.test.ts @@ -0,0 +1,86 @@ +import { test } from "node:test" +import assert from "node:assert/strict" +import { + LIVENESS_TIMEOUT_MS, + isHealthy, + isStreamStale, + reconnectDelayMs, + shouldReconnectOnResume, + shouldResetRetries, +} from "./sse-liveness.ts" + +const NOW = 1_000_000 + +test("a stream that just delivered is not stale", () => { + assert.equal(isStreamStale({ lastEventAt: NOW, now: NOW }), false) + assert.equal(isStreamStale({ lastEventAt: NOW, now: NOW + LIVENESS_TIMEOUT_MS - 1 }), false) +}) + +test("silence past the timeout is stale", () => { + assert.equal(isStreamStale({ lastEventAt: NOW, now: NOW + LIVENESS_TIMEOUT_MS }), true) +}) + +// The half-open case: connected, never delivered anything, hangs forever. +test("a stream that never delivered anything still goes stale", () => { + const attemptStartedAt = NOW + assert.equal(isStreamStale({ lastEventAt: attemptStartedAt, now: NOW + LIVENESS_TIMEOUT_MS + 1 }), true) +}) + +test("the timeout tolerates a missed heartbeat or two", () => { + // Server heartbeats are ~10s; a single late one must not trigger a reconnect. + assert.equal(isStreamStale({ lastEventAt: NOW, now: NOW + 12_000 }), false) + assert.equal(isStreamStale({ lastEventAt: NOW, now: NOW + 21_000 }), false) +}) + +// The bug: a timer reset backoff whether or not anything arrived. +test("retries reset only on a received event", () => { + assert.equal(shouldResetRetries({ receivedEvent: true }), true) + assert.equal(shouldResetRetries({ receivedEvent: false }), false) +}) + +test("backoff climbs and is capped", () => { + const fixed = () => 0.5 // no jitter + assert.equal(reconnectDelayMs(1, fixed), 1000) + assert.equal(reconnectDelayMs(2, fixed), 2000) + assert.equal(reconnectDelayMs(5, fixed), 15000) + assert.equal(reconnectDelayMs(99, fixed), 15000) +}) + +test("backoff jitter stays within bounds", () => { + for (const r of [0, 0.999]) { + const d = reconnectDelayMs(3, () => r) + assert.ok(d >= 3000 && d <= 6000, `attempt 3 delay out of range: ${d}`) + } +}) + +test("attempt 0 or negative is treated as the first attempt", () => { + const fixed = () => 0.5 + assert.equal(reconnectDelayMs(0, fixed), 1000) + assert.equal(reconnectDelayMs(-3, fixed), 1000) +}) + +test("resume does not reconnect a live stream", () => { + assert.equal(shouldReconnectOnResume({ transport: "live", attemptInFlight: false }), false) +}) + +// Foreground and network-restore often fire together; two attempts would open +// duplicate streams and double-handle every event. +test("resume does not race an attempt already in flight", () => { + assert.equal(shouldReconnectOnResume({ transport: "connecting", attemptInFlight: true }), false) + assert.equal(shouldReconnectOnResume({ transport: "idle", attemptInFlight: true }), false) +}) + +test("resume reconnects when idle and nothing is dialling", () => { + assert.equal(shouldReconnectOnResume({ transport: "idle", attemptInFlight: false }), true) +}) + +test("resume reconnects a stalled 'connecting' with no attempt in flight", () => { + assert.equal(shouldReconnectOnResume({ transport: "connecting", attemptInFlight: false }), true) +}) + +// The green-UI-over-a-dead-stream bug. +test("only a live transport reads as healthy", () => { + assert.equal(isHealthy("live"), true) + assert.equal(isHealthy("connecting"), false) + assert.equal(isHealthy("idle"), false) +}) diff --git a/src/lib/sse-liveness.ts b/src/lib/sse-liveness.ts new file mode 100644 index 00000000..0d5ab22c --- /dev/null +++ b/src/lib/sse-liveness.ts @@ -0,0 +1,99 @@ +// Liveness policy for the global SSE stream. +// +// Three separate defects made a mobile client look connected while its +// transport was dead, and made recovery slow or unbounded: +// +// 1. **No read timeout.** `sdk.ts`'s reader awaits `reader.read()` forever. A +// half-open socket — routine when a phone moves between Wi-Fi and cellular, +// or resumes from doze — produces no bytes, no `done`, and no error, so the +// loop parks indefinitely and nothing ever triggers a reconnect. +// 2. **`connected` meant "we tried".** `events.ts` set `connected: true` at the +// moment it began connecting, before a single byte arrived, so the green UI +// reflected an intention rather than a verified stream. +// 3. **Retry backoff reset on a timer.** A 10-second `setTimeout` cleared +// `reconnectAttempts` whether or not anything was ever received, so a +// connection that was failing silently kept resetting its own backoff and +// looked healthy. +// +// The server emits heartbeats every ~10s, so silence well past that is evidence +// of a dead stream rather than an idle one. Everything here is pure so the +// policy can be tested without a socket. + +/** + * How long a stream may produce nothing before it is presumed dead. + * + * Three missed ~10s heartbeats. Long enough not to churn on a brief stall, + * short enough that recovery is bounded rather than "eventually". + */ +export const LIVENESS_TIMEOUT_MS = 35_000 + +/** Reconnect backoff, unchanged from the existing ladder. */ +export const RECONNECT_DELAYS_MS = [1000, 2000, 4000, 8000, 15000] as const + +export type TransportState = + /** No attempt in flight and nothing live. */ + | "idle" + /** Attempting; nothing received yet. NOT the same as connected. */ + | "connecting" + /** At least one event/heartbeat received on this attempt. */ + | "live" + +/** + * Has the stream gone silent long enough to presume it dead? + * + * `lastEventAt` is when a byte last arrived on the current attempt, or the + * attempt's start if nothing has arrived yet — so a stream that never delivers + * anything is also caught, not just one that goes quiet later. + */ +export function isStreamStale(input: { + lastEventAt: number + now: number + timeoutMs?: number +}): boolean { + const timeout = input.timeoutMs ?? LIVENESS_TIMEOUT_MS + return input.now - input.lastEventAt >= timeout +} + +/** + * Should the retry counter reset? + * + * Only on demonstrated liveness. Resetting on a timer let a silently-failing + * connection keep declaring itself stable. + */ +export function shouldResetRetries(input: { receivedEvent: boolean }): boolean { + return input.receivedEvent +} + +/** Jittered backoff for the given attempt (1-based). */ +export function reconnectDelayMs(attempt: number, random: () => number = Math.random): number { + const index = Math.min(Math.max(attempt, 1) - 1, RECONNECT_DELAYS_MS.length - 1) + const base = RECONNECT_DELAYS_MS[index] + return Math.min(15_000, Math.round(base * (0.75 + random() * 0.5))) +} + +/** + * Should a foreground/network-restore event trigger an immediate reconnect? + * + * Deduplicated deliberately: a foreground transition often arrives alongside a + * network-change event, and both would otherwise each start an attempt, opening + * duplicate streams and double-handling every event. + */ +export function shouldReconnectOnResume(input: { + transport: TransportState + attemptInFlight: boolean +}): boolean { + if (input.transport === "live") return false + // Already dialling — let it finish or time out rather than racing it. + if (input.attemptInFlight) return false + return true +} + +/** + * Is the connection indicator allowed to read as healthy? + * + * Only "live" counts. "connecting" previously rendered as connected, which is + * how the UI came to show green over a stream that had never delivered a byte. + */ +export function isHealthy(transport: TransportState): boolean { + return transport === "live" +} diff --git a/src/stores/connections.ts b/src/stores/connections.ts index 2fb33de5..1395bb32 100644 --- a/src/stores/connections.ts +++ b/src/stores/connections.ts @@ -100,37 +100,49 @@ export const useConnections = create((set, get) => ({ // Create client for active connection let client: Client | null = null let base: ClientBase | null = null - let project: Project | null = null - let home: string | null = null if (active) { const password = await SecureStore.getItemAsync(`${PASSWORDS_PREFIX}${active.id}`) const auth = buildAuth(active.username, password) const built = buildClient(active.url, active.directory, auth) client = built.client base = built.base - // Fetch current project info and server paths - try { - const [proj, paths] = await Promise.all([ - client.project.current().catch(() => null), - client.path.get().catch(() => null), - ]) - project = proj - home = paths?.home || null - } catch { - // Server might be offline - } } + // Commit the client BEFORE fetching metadata. Everything downstream of + // startup keys off `client` appearing in this store -- the SSE event + // stream, the catalog load, the notification prompt -- and none of it + // needs the project name or the server's home path. Awaiting the + // metadata first serialized the entire app behind two requests that + // are individually capped at the 30s REQUEST_TIMEOUT_MS, so a slow or + // hanging metadata response delayed live events by up to that long, + // on exactly the flaky networks where prompt reconnection matters most. set({ connections, activeConnection: active, client, clientBase: base, - currentProject: project, - serverHome: home, recentDirectories, isLoading: false, }) + + // Metadata fills in behind. Guarded so a stale response cannot clobber + // a newer connection's state: switching servers while this fetch is in + // flight replaces `client`, and that is the signal to discard. + if (client) { + const requestClient = client + try { + const [proj, paths] = await Promise.all([ + requestClient.project.current().catch(() => null), + requestClient.path.get().catch(() => null), + ]) + if (get().client === requestClient) { + set({ currentProject: proj, serverHome: paths?.home || null }) + } + } catch { + // Server might be offline; the connection itself still works and + // SSE will report its own state. + } + } } catch (error) { set({ error: "Failed to load connections", isLoading: false }) } diff --git a/src/stores/events.ts b/src/stores/events.ts index c53a8481..44f8df12 100644 --- a/src/stores/events.ts +++ b/src/stores/events.ts @@ -9,13 +9,22 @@ import { AnalyticsEvent, track } from "../lib/analytics" import { recordSuccessfulSession } from "../lib/store-review" import { isAuthError } from "../lib/api-error" import { isSessionActuallyIdle } from "../lib/session-status-reconcile" +import { isHealthy, shouldReconnectOnResume, shouldResetRetries, type TransportState } from "../lib/sse-liveness" import type { Client, Part, Session, Message } from "../lib/sdk" // Session status from the server type SessionStatus = { type: "idle" } | { type: "busy" } | { type: "retry"; attempt: number; message: string } interface EventsState { + /** + * True only once the stream has actually delivered something. This used to + * be set the moment a connect was *attempted*, so the green indicator + * reflected an intention rather than a verified transport -- the app could + * show connected over a socket that had never produced a byte. + */ connected: boolean + /** Finer-grained view of the same thing; see src/lib/sse-liveness.ts. */ + transport: TransportState // Set when the last connection attempt failed with 401/403 — the server // rejected our credentials, not a transient network issue. The reconnect // loop stops retrying in this case (see connect()) since hammering a @@ -57,6 +66,13 @@ interface EventsState { connect: () => void disconnect: () => void + /** + * Called on foreground / network restoration. Reconnects only when the + * transport is not already live and no attempt is in flight -- those two + * signals often arrive together, and reconnecting twice would open duplicate + * streams and double-handle every event. + */ + resume: () => void } let controller: AbortController | null = null @@ -69,7 +85,6 @@ let reconnectTimer: ReturnType | null = null const erroredSessions = new Set() const RECONNECT_DELAYS_MS = [1000, 2000, 4000, 8000, 15000] as const -const STABLE_CONNECTION_MS = 10_000 const PROLONGED_DISCONNECT_MS = 30_000 // Re-fetch pending permissions and questions from the server for a session. @@ -145,8 +160,40 @@ async function resyncBusySessions() { ) } +// Reconcile the transcript of the session the user currently has open, after +// an SSE reconnect. +// +// resyncBusySessions() above is not enough on its own. It only considers +// sessions this client has marked "busy", and a client only learns a session +// is busy *from an SSE event*. If the stream was down while another client +// (CLI, TUI, another device) submitted a prompt, this client never saw the +// busy `session.status` event, so the session is still "idle" in its store, +// resyncBusySessions() finds nothing to do, and — because reconnect resumes +// the stream from "now" and does not replay missed events — the messages that +// arrived during the gap are never fetched. The open transcript then stays +// stale until the user navigates away and back, which is the reported +// cross-client staleness symptom. +// +// refreshMessages() re-fetches the current session and replaces messages/parts +// without touching isLoading, so this lands as a silent background reconcile +// rather than a spinner over content the user is already reading. +// +// Note this can overlap with resyncBusySessions() for a session that was busy +// and has since gone idle — both would refresh. That costs one redundant GET +// on an infrequent event, which is cheaper than the coupling needed to dedupe. +async function reconcileOpenSession() { + const sessions = useSessions.getState() + if (!sessions.currentSession) return + try { + await sessions.refreshMessages() + } catch (err) { + console.warn("[Events] Failed to reconcile open session after reconnect:", err) + } +} + export const useEvents = create((set, get) => ({ connected: false, + transport: "idle" as TransportState, authError: false, reconnectAttempts: 0, lastDisconnectAt: null, @@ -158,6 +205,7 @@ export const useEvents = create((set, get) => ({ connect: () => { controller?.abort() controller = null + set({ transport: "idle" }) if (reconnectTimer) { clearTimeout(reconnectTimer) reconnectTimer = null @@ -168,7 +216,10 @@ export const useEvents = create((set, get) => ({ controller = new AbortController() const currentController = controller - set({ connected: true, authError: false }) + // NOT connected yet -- only dialling. `connected` flips on the first + // received event below, so the indicator cannot go green over a dead + // stream. + set({ transport: "connecting", authError: false }) console.log("[SSE] Connecting to event stream...") addBreadcrumb({ category: "sse", message: "connecting" }) @@ -181,11 +232,11 @@ export const useEvents = create((set, get) => ({ // failed retries can't re-arm the check on every attempt. const isReconnect = get().reconnectAttempts > 0 let resyncedAfterReconnect = false - const stableTimer = setTimeout(() => { - if (!currentController.signal.aborted) { - set({ reconnectAttempts: 0, lastDisconnectAt: null }) - } - }, STABLE_CONNECTION_MS) + // Retry state resets on demonstrated liveness, not on a timer. The old + // 10s timeout cleared the backoff whether or not anything had ever + // arrived, so a silently-failing connection kept resetting its own + // backoff and looked healthy. + let receivedAnyEvent = false const scheduleReconnect = (reason: unknown) => { if (reconnectScheduled || currentController.signal.aborted) return @@ -194,7 +245,7 @@ export const useEvents = create((set, get) => ({ const reconnectAttempts = state.reconnectAttempts + 1 const lastDisconnectAt = state.lastDisconnectAt ?? Date.now() const disconnectedFor = Date.now() - lastDisconnectAt - set({ connected: false, reconnectAttempts, lastDisconnectAt }) + set({ connected: false, transport: "idle", reconnectAttempts, lastDisconnectAt }) if (disconnectedFor >= PROLONGED_DISCONNECT_MS) { notify({ @@ -232,6 +283,17 @@ export const useEvents = create((set, get) => ({ if (isReconnect && !resyncedAfterReconnect) { resyncedAfterReconnect = true void resyncBusySessions() + // Backfill content missed while the stream was down. Separate from + // resyncBusySessions(), which only repairs *status* and only for + // sessions already known to be busy — see reconcileOpenSession(). + void reconcileOpenSession() + } + + if (!receivedAnyEvent) { + receivedAnyEvent = true + if (shouldResetRetries({ receivedEvent: true })) { + set({ connected: true, transport: "live", reconnectAttempts: 0, lastDisconnectAt: null }) + } } const payload = (event as any).payload || event @@ -466,12 +528,11 @@ export const useEvents = create((set, get) => ({ data: { status: err.status }, }) track(AnalyticsEvent.ConnectionFailed, { source: "sse", error_class: "unauthorized" }) - set({ connected: false, authError: true }) + set({ connected: false, transport: "idle", authError: true }) } else { scheduleReconnect(err) } } finally { - clearTimeout(stableTimer) if (currentController.signal.aborted) { console.log("[SSE] Disconnected (aborted)") } @@ -479,6 +540,17 @@ export const useEvents = create((set, get) => ({ })() }, + resume: () => { + const { transport } = get() + // reconnectTimer set means a retry is already scheduled; controller set + // with a non-aborted signal means one is dialling right now. + const attemptInFlight = reconnectTimer !== null || (controller !== null && !controller.signal.aborted) + if (!shouldReconnectOnResume({ transport, attemptInFlight })) return + console.log("[SSE] resume -> reconnecting") + addBreadcrumb({ category: "sse", message: "resume reconnect" }) + get().connect() + }, + disconnect: () => { console.log("[SSE] Disconnecting") addBreadcrumb({ category: "sse", message: "disconnected" }) diff --git a/src/stores/sessions.ts b/src/stores/sessions.ts index ccd21f33..d08bb758 100644 --- a/src/stores/sessions.ts +++ b/src/stores/sessions.ts @@ -111,7 +111,7 @@ export const useSessions = create((set, get) => ({ set({ isLoading: true, error: null }) // A directory-less list includes sessions across projects. Each row carries // its own directory into the session route so subsequent operations stay scoped. - const sessions = await client.session.list({ roots: true, limit: 50 }) + const sessions = await client.session.list({ roots: true }) set({ sessions, isLoading: false }) } catch (error) { set({ error: "Failed to load sessions", isLoading: false })