mirror of
https://github.com/stablyai/orca.git
synced 2026-10-21 08:03:33 +00:00
fix(mobile): close four session-option picker defects found in review
D1 — a picker apply could interleave with a composer send. The composer already blocks a text send while an apply is dispatching, but not the reverse: the host spaces a send's body and its Enter ~500ms apart, so an apply tapped inside that window was submitted as part of the user's prompt, and the pill then claimed a model change that never ran as a command. The pickers render inside the composer, so they now take its in-flight state directly — the same guard, mirrored. D2 — an option was filed under the wrong model. `setTrackedSessionOption` resolves the owning model when it commits, not when the command was built, and the report effect mutates the same record off-queue. A report landing mid-dispatch therefore recorded `/effort low` against the model it switched TO. Ports desktop's supersession guard, which skips the commit when the baseline moved. D3 — a command template's prefix also matches prose that starts with it, so "/model is a weird word" tracked that prose as the current model, rendered it as the pill label, and matched no catalog model, dropping every per-model option. Parsed values are now canonicalized against the catalog; a typed value containing whitespace is treated as a prompt rather than a command. Perf — `/` on a Codex tab returned all 45 commands into a non-virtualized ScrollView showing ~5, re-reconciled on every streaming tick above the transcript. Capped at 12. Also splits the row primitives out of MobileNativeChatSessionOptionPickers.tsx, which the D1 guard pushed to 402 effective lines against a 400 cap.
This commit is contained in:
Brennan Benson
@@ -116,6 +116,74 @@ describe('MobileNativeChatComposer', () => {
|
||||
expect(onSend).toHaveBeenCalledWith(' /clear is prose')
|
||||
})
|
||||
|
||||
it('locks the option pickers while a composer send is in flight', async () => {
|
||||
// The reverse of the test below. The host spaces a send's body and its Enter
|
||||
// ~500ms apart, so an apply tapped inside that window would be submitted as
|
||||
// part of the user's prompt instead of running as its own command.
|
||||
let releaseSend: ((accepted: boolean) => void) | undefined
|
||||
const onSend = vi.fn(
|
||||
() =>
|
||||
new Promise<boolean>((resolve) => {
|
||||
releaseSend = resolve
|
||||
})
|
||||
)
|
||||
const controller = {
|
||||
snapshot: [
|
||||
{
|
||||
id: 'model',
|
||||
label: 'Model',
|
||||
category: 'model' as const,
|
||||
kind: {
|
||||
type: 'select' as const,
|
||||
choices: [
|
||||
{ value: 'sonnet', label: 'Sonnet 5' },
|
||||
{ value: 'opus', label: 'Opus 4.8' }
|
||||
]
|
||||
},
|
||||
valueSource: 'unknown' as const,
|
||||
settable: true
|
||||
}
|
||||
],
|
||||
pendingId: null,
|
||||
setOption: vi.fn(),
|
||||
invokeAction: vi.fn(),
|
||||
recordCommand: vi.fn()
|
||||
}
|
||||
const restore = suppressRendererWarning()
|
||||
try {
|
||||
await act(async () => {
|
||||
renderer = create(
|
||||
createElement(MobileNativeChatComposer, {
|
||||
value: 'run the tests',
|
||||
onChangeText: vi.fn(),
|
||||
onSend,
|
||||
sessionOptions: { isWorking: false, controller }
|
||||
})
|
||||
)
|
||||
})
|
||||
const modelPill = (): { props: { accessibilityState: { disabled: boolean } } } =>
|
||||
renderer!.root.find(
|
||||
(node) => node.type === 'Pressable' && node.props.accessibilityLabel === 'Model, Model'
|
||||
) as { props: { accessibilityState: { disabled: boolean } } }
|
||||
expect(modelPill().props.accessibilityState).toMatchObject({ disabled: false })
|
||||
// Start the send but don't await it — it stays in flight on purpose.
|
||||
let pressed!: Promise<void>
|
||||
await act(async () => {
|
||||
pressed = sendButton().props.onPress()
|
||||
await Promise.resolve()
|
||||
})
|
||||
expect(onSend).toHaveBeenCalled()
|
||||
expect(modelPill().props.accessibilityState).toMatchObject({ disabled: true })
|
||||
await act(async () => {
|
||||
releaseSend?.(true)
|
||||
await pressed
|
||||
})
|
||||
expect(modelPill().props.accessibilityState).toMatchObject({ disabled: false })
|
||||
} finally {
|
||||
restore()
|
||||
}
|
||||
})
|
||||
|
||||
it('blocks composer submission while a session-option command is pending', async () => {
|
||||
const onSend = vi.fn().mockResolvedValue(true)
|
||||
const restore = suppressRendererWarning()
|
||||
|
||||
@@ -106,7 +106,10 @@ export function MobileNativeChatComposer({
|
||||
}
|
||||
if (trigger.kind === 'slash') {
|
||||
const commands = agent ? getVerifiedNativeChatCommands(agent) : []
|
||||
return rankSlashCommandSuggestions(commands, trigger.query).map((command) => ({
|
||||
// Why: Codex's catalog is 45 commands and this list is a plain ScrollView
|
||||
// (~5 rows visible), so an uncapped `/` would mount every row and
|
||||
// re-reconcile them on each streaming tick right above the transcript.
|
||||
return rankSlashCommandSuggestions(commands, trigger.query, 12).map((command) => ({
|
||||
kind: 'command' as const,
|
||||
command
|
||||
}))
|
||||
@@ -192,7 +195,12 @@ export function MobileNativeChatComposer({
|
||||
))}
|
||||
</ScrollView>
|
||||
) : null}
|
||||
{sessionOptions ? <MobileNativeChatSessionOptionPickers {...sessionOptions} /> : null}
|
||||
{sessionOptions ? (
|
||||
<MobileNativeChatSessionOptionPickers
|
||||
{...sessionOptions}
|
||||
sendInFlight={sending || isAttaching}
|
||||
/>
|
||||
) : null}
|
||||
<View style={styles.bar}>
|
||||
{onAttachImage ? (
|
||||
<Pressable
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
import { useState } from 'react'
|
||||
import { ActivityIndicator, Pressable, ScrollView, StyleSheet, Text, View } from 'react-native'
|
||||
import { Check, ChevronDown, X } from 'lucide-react-native'
|
||||
import { colors, radii, spacing, typography } from '../theme/mobile-theme'
|
||||
import { X } from 'lucide-react-native'
|
||||
import { colors, spacing, typography } from '../theme/mobile-theme'
|
||||
import type {
|
||||
SessionOptionDescriptor,
|
||||
SessionOptionValue
|
||||
@@ -13,6 +13,7 @@ import {
|
||||
mobileSessionOptionDisabledReason,
|
||||
sortedMobileSessionOptions
|
||||
} from './mobile-native-chat-session-option-labels'
|
||||
import { DescriptorRows, Pill, SessionOptionCaption } from './MobileNativeChatSessionOptionRows'
|
||||
import type { MobileNativeChatSessionOptionsController } from './use-mobile-native-chat-session-options'
|
||||
|
||||
export type MobileNativeChatSessionOptionPickersProps = {
|
||||
@@ -20,172 +21,19 @@ export type MobileNativeChatSessionOptionPickersProps = {
|
||||
/** Pickers lock while the agent works — a mid-turn `/model` interleaves with
|
||||
* the agent's own output (desktop parity). */
|
||||
isWorking: boolean
|
||||
}
|
||||
|
||||
function Pill({
|
||||
label,
|
||||
accessibleName,
|
||||
disabled,
|
||||
onPress
|
||||
}: {
|
||||
label: string
|
||||
accessibleName: string
|
||||
disabled: boolean
|
||||
onPress: () => void
|
||||
}): React.JSX.Element {
|
||||
return (
|
||||
<Pressable
|
||||
accessibilityLabel={accessibleName}
|
||||
accessibilityRole="button"
|
||||
accessibilityState={{ disabled }}
|
||||
style={({ pressed }) => [styles.pill, pressed && !disabled && styles.pressed]}
|
||||
onPress={onPress}
|
||||
disabled={disabled}
|
||||
hitSlop={6}
|
||||
>
|
||||
<Text
|
||||
style={[styles.pillText, disabled && styles.pillTextDisabled]}
|
||||
numberOfLines={1}
|
||||
ellipsizeMode="tail"
|
||||
>
|
||||
{label}
|
||||
</Text>
|
||||
<ChevronDown size={12} color={disabled ? colors.textMuted : colors.textSecondary} />
|
||||
</Pressable>
|
||||
)
|
||||
}
|
||||
|
||||
function ChoiceRow({
|
||||
label,
|
||||
description,
|
||||
selected,
|
||||
disabled,
|
||||
onPress
|
||||
}: {
|
||||
label: string
|
||||
description?: string
|
||||
selected: boolean
|
||||
disabled: boolean
|
||||
onPress: () => void
|
||||
}): React.JSX.Element {
|
||||
return (
|
||||
<Pressable
|
||||
accessibilityRole="radio"
|
||||
accessibilityState={{ checked: selected, disabled }}
|
||||
style={[styles.row, selected && styles.rowSelected, disabled && styles.rowDisabled]}
|
||||
onPress={onPress}
|
||||
disabled={disabled}
|
||||
>
|
||||
<View style={[styles.radio, selected && styles.radioOn]}>
|
||||
{selected ? <Check size={12} color={colors.bgBase} strokeWidth={3} /> : null}
|
||||
</View>
|
||||
<View style={styles.rowBody}>
|
||||
<Text style={styles.rowLabel}>{label}</Text>
|
||||
{description ? (
|
||||
<Text style={styles.rowDescription} numberOfLines={2}>
|
||||
{description}
|
||||
</Text>
|
||||
) : null}
|
||||
</View>
|
||||
</Pressable>
|
||||
)
|
||||
}
|
||||
|
||||
function ActionRow({
|
||||
label,
|
||||
disabled,
|
||||
onPress
|
||||
}: {
|
||||
label: string
|
||||
disabled: boolean
|
||||
onPress: () => void
|
||||
}): React.JSX.Element {
|
||||
return (
|
||||
<Pressable
|
||||
accessibilityRole="button"
|
||||
accessibilityState={{ disabled }}
|
||||
style={[styles.row, disabled && styles.rowDisabled]}
|
||||
onPress={onPress}
|
||||
disabled={disabled}
|
||||
>
|
||||
<View style={styles.rowBody}>
|
||||
<Text style={styles.rowLabel}>{label}</Text>
|
||||
</View>
|
||||
</Pressable>
|
||||
)
|
||||
}
|
||||
|
||||
function DescriptorRows({
|
||||
descriptor,
|
||||
disabled,
|
||||
onSetOption,
|
||||
onInvokeAction
|
||||
}: {
|
||||
descriptor: SessionOptionDescriptor
|
||||
disabled: boolean
|
||||
onSetOption: (value: SessionOptionValue) => void
|
||||
onInvokeAction: () => void
|
||||
}): React.JSX.Element {
|
||||
const locked = disabled || !descriptor.settable
|
||||
// Why: flip-only without a baseline is an action — never claim On/Off.
|
||||
if (descriptor.action?.type === 'toggle-command') {
|
||||
return (
|
||||
<ActionRow
|
||||
label={`Toggle ${descriptor.label.toLowerCase()}`}
|
||||
disabled={locked}
|
||||
onPress={onInvokeAction}
|
||||
/>
|
||||
)
|
||||
}
|
||||
// Why: agent-picker opens the TUI; it is not a set of radio choices.
|
||||
if (descriptor.action?.type === 'agent-picker') {
|
||||
return <ActionRow label="Choose in agent picker…" disabled={locked} onPress={onInvokeAction} />
|
||||
}
|
||||
// Unknown booleans leave both radios unselected instead of inventing truth.
|
||||
if (descriptor.kind.type === 'boolean') {
|
||||
const current = descriptor.kind.currentValue
|
||||
return (
|
||||
<>
|
||||
{current === undefined ? (
|
||||
<Text style={styles.caption}>Current value unknown — pick On or Off</Text>
|
||||
) : null}
|
||||
<ChoiceRow
|
||||
label="On"
|
||||
selected={current === true}
|
||||
disabled={locked}
|
||||
onPress={() => onSetOption(true)}
|
||||
/>
|
||||
<ChoiceRow
|
||||
label="Off"
|
||||
selected={current === false}
|
||||
disabled={locked}
|
||||
onPress={() => onSetOption(false)}
|
||||
/>
|
||||
</>
|
||||
)
|
||||
}
|
||||
const { currentValue, choices } = descriptor.kind
|
||||
return (
|
||||
<>
|
||||
{choices.map((choice) => (
|
||||
<ChoiceRow
|
||||
key={choice.value}
|
||||
label={choice.label}
|
||||
description={choice.description}
|
||||
selected={choice.value === currentValue}
|
||||
disabled={locked}
|
||||
onPress={() => onSetOption(choice.value)}
|
||||
/>
|
||||
))}
|
||||
</>
|
||||
)
|
||||
/** A composer send owns the TUI input line until it settles. The host spaces a
|
||||
* send's body and its Enter ~500ms apart, so an apply dispatched inside that
|
||||
* window would be submitted as part of the user's prompt. The composer blocks
|
||||
* the reverse direction on `pendingId`; this is the same guard mirrored. */
|
||||
sendInFlight?: boolean
|
||||
}
|
||||
|
||||
/** Model + session-option pills for the chat composer, opening an inline card
|
||||
* of choices (the Ask-card pattern) instead of desktop's dropdown menus. */
|
||||
export function MobileNativeChatSessionOptionPickers({
|
||||
controller,
|
||||
isWorking
|
||||
isWorking,
|
||||
sendInFlight = false
|
||||
}: MobileNativeChatSessionOptionPickersProps): React.JSX.Element | null {
|
||||
const [open, setOpen] = useState<'model' | 'options' | null>(null)
|
||||
const { snapshot, pendingId } = controller
|
||||
@@ -194,7 +42,7 @@ export function MobileNativeChatSessionOptionPickers({
|
||||
if (!model) {
|
||||
return null
|
||||
}
|
||||
const disabled = isWorking || pendingId !== null
|
||||
const disabled = isWorking || pendingId !== null || sendInFlight
|
||||
const sheetDescriptors = open === 'model' ? [model] : options
|
||||
const sheetTitle = open === 'model' ? 'Model' : mobileOptionsPillTitle(options)
|
||||
const dispatched = sheetDescriptors.some((descriptor) => descriptor.valueSource === 'dispatched')
|
||||
@@ -249,9 +97,9 @@ export function MobileNativeChatSessionOptionPickers({
|
||||
</Pressable>
|
||||
</View>
|
||||
{dispatched ? (
|
||||
<Text style={styles.caption}>Sent to the agent — not confirmed</Text>
|
||||
<SessionOptionCaption>Sent to the agent — not confirmed</SessionOptionCaption>
|
||||
) : null}
|
||||
{reason ? <Text style={styles.caption}>{reason}</Text> : null}
|
||||
{reason ? <SessionOptionCaption>{reason}</SessionOptionCaption> : null}
|
||||
<ScrollView style={styles.sheetScroll} keyboardShouldPersistTaps="always">
|
||||
{sheetDescriptors.map((descriptor, index) => (
|
||||
<View key={descriptor.id}>
|
||||
@@ -300,31 +148,6 @@ const styles = StyleSheet.create({
|
||||
paddingTop: spacing.xs,
|
||||
backgroundColor: colors.bgPanel
|
||||
},
|
||||
pill: {
|
||||
flexDirection: 'row',
|
||||
alignItems: 'center',
|
||||
gap: 4,
|
||||
maxWidth: 180,
|
||||
minHeight: 28,
|
||||
paddingHorizontal: spacing.sm,
|
||||
paddingVertical: 4,
|
||||
borderRadius: radii.button,
|
||||
borderWidth: StyleSheet.hairlineWidth,
|
||||
borderColor: colors.borderSubtle,
|
||||
backgroundColor: colors.bgRaised
|
||||
},
|
||||
pillText: {
|
||||
color: colors.textSecondary,
|
||||
fontSize: typography.metaSize,
|
||||
fontWeight: '600',
|
||||
flexShrink: 1
|
||||
},
|
||||
pillTextDisabled: {
|
||||
color: colors.textMuted
|
||||
},
|
||||
pressed: {
|
||||
opacity: 0.7
|
||||
},
|
||||
sheet: {
|
||||
maxHeight: 320,
|
||||
backgroundColor: colors.bgPanel,
|
||||
@@ -363,55 +186,5 @@ const styles = StyleSheet.create({
|
||||
},
|
||||
sectionLabelSpaced: {
|
||||
marginTop: spacing.sm
|
||||
},
|
||||
caption: {
|
||||
color: colors.textMuted,
|
||||
fontSize: typography.metaSize,
|
||||
paddingHorizontal: spacing.md,
|
||||
paddingBottom: spacing.xs
|
||||
},
|
||||
row: {
|
||||
flexDirection: 'row',
|
||||
gap: spacing.sm,
|
||||
padding: spacing.sm,
|
||||
minHeight: 44,
|
||||
alignItems: 'center',
|
||||
borderRadius: radii.card,
|
||||
backgroundColor: colors.bgRaised,
|
||||
borderWidth: 1,
|
||||
borderColor: colors.borderSubtle,
|
||||
marginBottom: spacing.xs
|
||||
},
|
||||
rowSelected: {
|
||||
borderColor: colors.statusGreen
|
||||
},
|
||||
rowDisabled: {
|
||||
opacity: 0.5
|
||||
},
|
||||
radio: {
|
||||
width: 18,
|
||||
height: 18,
|
||||
borderRadius: 9,
|
||||
borderWidth: 1.5,
|
||||
borderColor: colors.textMuted,
|
||||
alignItems: 'center',
|
||||
justifyContent: 'center'
|
||||
},
|
||||
radioOn: {
|
||||
backgroundColor: colors.statusGreen,
|
||||
borderColor: colors.statusGreen
|
||||
},
|
||||
rowBody: {
|
||||
flex: 1,
|
||||
gap: 2
|
||||
},
|
||||
rowLabel: {
|
||||
color: colors.textPrimary,
|
||||
fontSize: typography.bodySize,
|
||||
fontWeight: '600'
|
||||
},
|
||||
rowDescription: {
|
||||
color: colors.textSecondary,
|
||||
fontSize: typography.metaSize
|
||||
}
|
||||
})
|
||||
|
||||
@@ -0,0 +1,252 @@
|
||||
// The pill and choice-row primitives the session-option card is built from, kept
|
||||
// beside it so the card file stays about layout and apply wiring.
|
||||
|
||||
import { Pressable, StyleSheet, Text, View } from 'react-native'
|
||||
import { Check, ChevronDown } from 'lucide-react-native'
|
||||
import { colors, radii, spacing, typography } from '../theme/mobile-theme'
|
||||
import type {
|
||||
SessionOptionDescriptor,
|
||||
SessionOptionValue
|
||||
} from '../../../src/shared/native-chat-session-options'
|
||||
|
||||
/** Muted one-liner above a group — dispatch state, or why a row is locked. */
|
||||
export function SessionOptionCaption({ children }: { children: string }): React.JSX.Element {
|
||||
return <Text style={styles.caption}>{children}</Text>
|
||||
}
|
||||
|
||||
export function Pill({
|
||||
label,
|
||||
accessibleName,
|
||||
disabled,
|
||||
onPress
|
||||
}: {
|
||||
label: string
|
||||
accessibleName: string
|
||||
disabled: boolean
|
||||
onPress: () => void
|
||||
}): React.JSX.Element {
|
||||
return (
|
||||
<Pressable
|
||||
accessibilityLabel={accessibleName}
|
||||
accessibilityRole="button"
|
||||
accessibilityState={{ disabled }}
|
||||
style={({ pressed }) => [styles.pill, pressed && !disabled && styles.pressed]}
|
||||
onPress={onPress}
|
||||
disabled={disabled}
|
||||
hitSlop={6}
|
||||
>
|
||||
<Text
|
||||
style={[styles.pillText, disabled && styles.pillTextDisabled]}
|
||||
numberOfLines={1}
|
||||
ellipsizeMode="tail"
|
||||
>
|
||||
{label}
|
||||
</Text>
|
||||
<ChevronDown size={12} color={disabled ? colors.textMuted : colors.textSecondary} />
|
||||
</Pressable>
|
||||
)
|
||||
}
|
||||
|
||||
function ChoiceRow({
|
||||
label,
|
||||
description,
|
||||
selected,
|
||||
disabled,
|
||||
onPress
|
||||
}: {
|
||||
label: string
|
||||
description?: string
|
||||
selected: boolean
|
||||
disabled: boolean
|
||||
onPress: () => void
|
||||
}): React.JSX.Element {
|
||||
return (
|
||||
<Pressable
|
||||
accessibilityRole="radio"
|
||||
accessibilityState={{ checked: selected, disabled }}
|
||||
style={[styles.row, selected && styles.rowSelected, disabled && styles.rowDisabled]}
|
||||
onPress={onPress}
|
||||
disabled={disabled}
|
||||
>
|
||||
<View style={[styles.radio, selected && styles.radioOn]}>
|
||||
{selected ? <Check size={12} color={colors.bgBase} strokeWidth={3} /> : null}
|
||||
</View>
|
||||
<View style={styles.rowBody}>
|
||||
<Text style={styles.rowLabel}>{label}</Text>
|
||||
{description ? (
|
||||
<Text style={styles.rowDescription} numberOfLines={2}>
|
||||
{description}
|
||||
</Text>
|
||||
) : null}
|
||||
</View>
|
||||
</Pressable>
|
||||
)
|
||||
}
|
||||
|
||||
function ActionRow({
|
||||
label,
|
||||
disabled,
|
||||
onPress
|
||||
}: {
|
||||
label: string
|
||||
disabled: boolean
|
||||
onPress: () => void
|
||||
}): React.JSX.Element {
|
||||
return (
|
||||
<Pressable
|
||||
accessibilityRole="button"
|
||||
accessibilityState={{ disabled }}
|
||||
style={[styles.row, disabled && styles.rowDisabled]}
|
||||
onPress={onPress}
|
||||
disabled={disabled}
|
||||
>
|
||||
<View style={styles.rowBody}>
|
||||
<Text style={styles.rowLabel}>{label}</Text>
|
||||
</View>
|
||||
</Pressable>
|
||||
)
|
||||
}
|
||||
|
||||
export function DescriptorRows({
|
||||
descriptor,
|
||||
disabled,
|
||||
onSetOption,
|
||||
onInvokeAction
|
||||
}: {
|
||||
descriptor: SessionOptionDescriptor
|
||||
disabled: boolean
|
||||
onSetOption: (value: SessionOptionValue) => void
|
||||
onInvokeAction: () => void
|
||||
}): React.JSX.Element {
|
||||
const locked = disabled || !descriptor.settable
|
||||
// Why: flip-only without a baseline is an action — never claim On/Off.
|
||||
if (descriptor.action?.type === 'toggle-command') {
|
||||
return (
|
||||
<ActionRow
|
||||
label={`Toggle ${descriptor.label.toLowerCase()}`}
|
||||
disabled={locked}
|
||||
onPress={onInvokeAction}
|
||||
/>
|
||||
)
|
||||
}
|
||||
// Why: agent-picker opens the TUI; it is not a set of radio choices.
|
||||
if (descriptor.action?.type === 'agent-picker') {
|
||||
return <ActionRow label="Choose in agent picker…" disabled={locked} onPress={onInvokeAction} />
|
||||
}
|
||||
// Unknown booleans leave both radios unselected instead of inventing truth.
|
||||
if (descriptor.kind.type === 'boolean') {
|
||||
const current = descriptor.kind.currentValue
|
||||
return (
|
||||
<>
|
||||
{current === undefined ? (
|
||||
<SessionOptionCaption>Current value unknown — pick On or Off</SessionOptionCaption>
|
||||
) : null}
|
||||
<ChoiceRow
|
||||
label="On"
|
||||
selected={current === true}
|
||||
disabled={locked}
|
||||
onPress={() => onSetOption(true)}
|
||||
/>
|
||||
<ChoiceRow
|
||||
label="Off"
|
||||
selected={current === false}
|
||||
disabled={locked}
|
||||
onPress={() => onSetOption(false)}
|
||||
/>
|
||||
</>
|
||||
)
|
||||
}
|
||||
const { currentValue, choices } = descriptor.kind
|
||||
return (
|
||||
<>
|
||||
{choices.map((choice) => (
|
||||
<ChoiceRow
|
||||
key={choice.value}
|
||||
label={choice.label}
|
||||
description={choice.description}
|
||||
selected={choice.value === currentValue}
|
||||
disabled={locked}
|
||||
onPress={() => onSetOption(choice.value)}
|
||||
/>
|
||||
))}
|
||||
</>
|
||||
)
|
||||
}
|
||||
|
||||
const styles = StyleSheet.create({
|
||||
pill: {
|
||||
flexDirection: 'row',
|
||||
alignItems: 'center',
|
||||
gap: 4,
|
||||
maxWidth: 180,
|
||||
minHeight: 28,
|
||||
paddingHorizontal: spacing.sm,
|
||||
paddingVertical: 4,
|
||||
borderRadius: radii.button,
|
||||
borderWidth: StyleSheet.hairlineWidth,
|
||||
borderColor: colors.borderSubtle,
|
||||
backgroundColor: colors.bgRaised
|
||||
},
|
||||
pillText: {
|
||||
color: colors.textSecondary,
|
||||
fontSize: typography.metaSize,
|
||||
fontWeight: '600',
|
||||
flexShrink: 1
|
||||
},
|
||||
pillTextDisabled: {
|
||||
color: colors.textMuted
|
||||
},
|
||||
pressed: {
|
||||
opacity: 0.7
|
||||
},
|
||||
caption: {
|
||||
color: colors.textMuted,
|
||||
fontSize: typography.metaSize,
|
||||
paddingHorizontal: spacing.md,
|
||||
paddingBottom: spacing.xs
|
||||
},
|
||||
row: {
|
||||
flexDirection: 'row',
|
||||
gap: spacing.sm,
|
||||
padding: spacing.sm,
|
||||
minHeight: 44,
|
||||
alignItems: 'center',
|
||||
borderRadius: radii.card,
|
||||
backgroundColor: colors.bgRaised,
|
||||
borderWidth: 1,
|
||||
borderColor: colors.borderSubtle,
|
||||
marginBottom: spacing.xs
|
||||
},
|
||||
rowSelected: {
|
||||
borderColor: colors.statusGreen
|
||||
},
|
||||
rowDisabled: {
|
||||
opacity: 0.5
|
||||
},
|
||||
radio: {
|
||||
width: 18,
|
||||
height: 18,
|
||||
borderRadius: 9,
|
||||
borderWidth: 1.5,
|
||||
borderColor: colors.textMuted,
|
||||
alignItems: 'center',
|
||||
justifyContent: 'center'
|
||||
},
|
||||
radioOn: {
|
||||
backgroundColor: colors.statusGreen,
|
||||
borderColor: colors.statusGreen
|
||||
},
|
||||
rowBody: {
|
||||
flex: 1,
|
||||
gap: 2
|
||||
},
|
||||
rowLabel: {
|
||||
color: colors.textPrimary,
|
||||
fontSize: typography.bodySize,
|
||||
fontWeight: '600'
|
||||
},
|
||||
rowDescription: {
|
||||
color: colors.textSecondary,
|
||||
fontSize: typography.metaSize
|
||||
}
|
||||
})
|
||||
@@ -135,6 +135,28 @@ describe('useMobileNativeChatSessionOptions', () => {
|
||||
expect(effort!.kind).toMatchObject({ currentValue: 'low' })
|
||||
})
|
||||
|
||||
it('does not file an option under a model that changed mid-dispatch', async () => {
|
||||
const resolvers: ((outcome: MobileNativeChatSendOutcome) => void)[] = []
|
||||
dispatchCommand.mockImplementation(
|
||||
() => new Promise<MobileNativeChatSendOutcome>((resolve) => resolvers.push(resolve))
|
||||
)
|
||||
mount({ reportedModel: 'claude-sonnet-5' })
|
||||
let applied!: Promise<boolean>
|
||||
await act(async () => {
|
||||
applied = api!.setOption('effort', 'low')
|
||||
await Promise.resolve()
|
||||
})
|
||||
// A report lands while `/effort low` is still in flight and moves the model.
|
||||
update({ reportedModel: 'claude-opus-5' })
|
||||
await act(async () => {
|
||||
resolvers[0]!('accepted')
|
||||
await applied
|
||||
})
|
||||
// The effort must not be recorded against Opus — it was sent for Sonnet.
|
||||
const effort = api!.snapshot.find((descriptor) => descriptor.id === 'effort')
|
||||
expect(effort?.kind).not.toMatchObject({ currentValue: 'low' })
|
||||
})
|
||||
|
||||
it('does not revive a stale session-start report over a newer local pick', async () => {
|
||||
mount({ reportedModel: 'claude-sonnet-5' })
|
||||
await act(async () => {
|
||||
|
||||
@@ -222,10 +222,28 @@ export function useMobileNativeChatSessionOptions(args: {
|
||||
if (!command) {
|
||||
return false
|
||||
}
|
||||
// Baseline for detecting a hook report or typed command that lands while
|
||||
// the dispatch is in flight — the record is shared mutable state and the
|
||||
// report effect is not on this queue.
|
||||
const trackedBeforeDispatch =
|
||||
id === 'model' ? undefined : getTrackedSessionOption(record, previousModelId, id)
|
||||
const outcome = await dispatchCommand(command)
|
||||
if (outcome === 'rejected') {
|
||||
return false
|
||||
}
|
||||
if (id !== 'model') {
|
||||
// Why (desktop parity): `setTrackedSessionOption` resolves the owning
|
||||
// model when it commits, not when the command was built. If the model
|
||||
// moved during the dispatch, committing now would file this value under
|
||||
// the NEW model — claiming an effort the agent was never asked for.
|
||||
const modelStill = typeof record.model?.value === 'string' ? record.model.value : null
|
||||
if (
|
||||
modelStill !== previousModelId ||
|
||||
getTrackedSessionOption(record, previousModelId, id) !== trackedBeforeDispatch
|
||||
) {
|
||||
return true
|
||||
}
|
||||
}
|
||||
if (id === 'model') {
|
||||
if (typeof value === 'string' && previousModelId !== value) {
|
||||
// Why: switching models can reset effort/toggles for the destination model.
|
||||
|
||||
@@ -148,6 +148,61 @@ describe('recordNativeChatSessionOptionCommand', () => {
|
||||
expect(record.valuesByModel.opus?.fastMode).toBeUndefined()
|
||||
})
|
||||
|
||||
it('rejects prose that merely starts with a command template', () => {
|
||||
// The `/model ` prefix matches this, so an unvalidated parse tracked
|
||||
// "is a weird word" as the current model — which then rendered as the pill
|
||||
// label and, matching no catalog model, dropped every per-model option.
|
||||
const record = claudeRecord('sonnet')
|
||||
expect(
|
||||
recordNativeChatSessionOptionCommand({
|
||||
catalog: CLAUDE_SESSION_OPTION_CATALOG,
|
||||
models: CLAUDE_SESSION_OPTION_CATALOG.models,
|
||||
record,
|
||||
command: '/model is a weird word'
|
||||
})
|
||||
).toEqual({ changed: false, opensAgentPicker: false })
|
||||
expect(record.model).toEqual({ value: 'sonnet', source: 'dispatched' })
|
||||
})
|
||||
|
||||
it('rejects an option value outside the catalog choices', () => {
|
||||
const record = claudeRecord('sonnet')
|
||||
recordNativeChatSessionOptionCommand({
|
||||
catalog: CLAUDE_SESSION_OPTION_CATALOG,
|
||||
models: CLAUDE_SESSION_OPTION_CATALOG.models,
|
||||
record,
|
||||
command: '/effort of will is required'
|
||||
})
|
||||
expect(record.valuesByModel.sonnet?.effort).toBeUndefined()
|
||||
})
|
||||
|
||||
it('tracks nothing for a multi-line paste whose first line looks like a command', () => {
|
||||
const record = claudeRecord('opus')
|
||||
recordNativeChatSessionOptionCommand({
|
||||
catalog: CLAUDE_SESSION_OPTION_CATALOG,
|
||||
models: CLAUDE_SESSION_OPTION_CATALOG.models,
|
||||
record,
|
||||
command: '/model sonnet\nplease review this'
|
||||
})
|
||||
expect(record.model).toEqual({ value: 'opus', source: 'dispatched' })
|
||||
})
|
||||
|
||||
it('still accepts an alias, a full provider id, and extra spacing', () => {
|
||||
for (const [command, expected] of [
|
||||
['/model opus', 'opus'],
|
||||
['/model claude-sonnet-5', 'sonnet'],
|
||||
['/model sonnet', 'sonnet']
|
||||
] as const) {
|
||||
const record = claudeRecord()
|
||||
recordNativeChatSessionOptionCommand({
|
||||
catalog: CLAUDE_SESSION_OPTION_CATALOG,
|
||||
models: CLAUDE_SESSION_OPTION_CATALOG.models,
|
||||
record,
|
||||
command
|
||||
})
|
||||
expect(record.model).toEqual({ value: expected, source: 'dispatched' })
|
||||
}
|
||||
})
|
||||
|
||||
it('ignores unrelated commands', () => {
|
||||
const record = claudeRecord('sonnet')
|
||||
expect(
|
||||
|
||||
@@ -10,6 +10,7 @@ import {
|
||||
clearTrackedSessionOption,
|
||||
flattenNativeChatSessionOptionRecord,
|
||||
isFlipOnlyMidSession,
|
||||
matchNativeChatCatalogModelId,
|
||||
type NativeChatSessionOptionRecord
|
||||
} from './native-chat-session-option-state'
|
||||
|
||||
@@ -84,9 +85,14 @@ function recordCommandApply(args: {
|
||||
optionId: string
|
||||
midSession: CatalogMidSessionApply | undefined
|
||||
command: string
|
||||
/** Canonicalize a parsed value, or reject it. A template's prefix also matches
|
||||
* prose that merely starts with it (`/model` parses `is a weird word` out of
|
||||
* "/model is a weird word"), and tracking that renders raw prose as the
|
||||
* current value — and, for the model, drops every option the model owns. */
|
||||
canonicalize: (value: string) => string | null
|
||||
persist?: PersistSessionOption
|
||||
}): boolean {
|
||||
const { record, optionId, midSession, command, persist } = args
|
||||
const { record, optionId, midSession, command, canonicalize, persist } = args
|
||||
if (!midSession || midSession.kind === 'unsupported') {
|
||||
return false
|
||||
}
|
||||
@@ -102,7 +108,11 @@ function recordCommandApply(args: {
|
||||
if (midSession.kind !== 'command') {
|
||||
return false
|
||||
}
|
||||
const value = parseBuiltSessionOptionCommand(midSession.build, command)
|
||||
const parsed = parseBuiltSessionOptionCommand(midSession.build, command)
|
||||
if (!parsed) {
|
||||
return false
|
||||
}
|
||||
const value = canonicalize(parsed)
|
||||
if (!value) {
|
||||
return false
|
||||
}
|
||||
@@ -143,6 +153,15 @@ export function recordNativeChatSessionOptionCommand(args: {
|
||||
optionId: 'model',
|
||||
midSession: catalog.modelApply.midSession,
|
||||
command,
|
||||
// A typed model can be an alias or a full provider id, so reuse the same
|
||||
// matcher the hook reports go through rather than demanding a catalog id.
|
||||
// Whitespace means this was never a bare command — a multi-line paste whose
|
||||
// first line happens to read `/model sonnet` is a prompt as far as we can
|
||||
// tell, so track nothing rather than guess. Model ids never contain spaces.
|
||||
canonicalize: (value) =>
|
||||
/\s/.test(value)
|
||||
? null
|
||||
: matchNativeChatCatalogModelId({ ...catalog, models: [...models] }, value),
|
||||
persist
|
||||
})
|
||||
const modelId = typeof record.model?.value === 'string' ? record.model.value : null
|
||||
@@ -156,6 +175,11 @@ export function recordNativeChatSessionOptionCommand(args: {
|
||||
optionId: option.id,
|
||||
midSession: option.apply.midSession,
|
||||
command,
|
||||
canonicalize: (value) =>
|
||||
option.kind.type === 'select' &&
|
||||
!option.kind.choices.some((choice) => choice.value === value)
|
||||
? null
|
||||
: value,
|
||||
persist
|
||||
}) || changed
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user