Skip to content

fix(tui): prevent session picker crash #36774

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
opencode-agent wants to merge 1 commit into
base: v2
Choose a base branch
from
+88 −47
Open

fix(tui): prevent session picker crash #36774

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
96 changes: 50 additions & 46 deletions packages/tui/src/ui/dialog-select.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ export function DialogSelect<T>(props: DialogSelectProps<T>) {
filter: "",
input: "keyboard" as "keyboard" | "mouse",
})
const [focusedAction, setFocusedAction] = createSignal<number>()
const [focusedAction, setFocusedAction] = createSignal<string>()
const actionFocused = createMemo(() => focusedAction() !== undefined)
let selection: { value: T; category?: string } | undefined
let resetSelection = false
Expand All @@ -130,42 +130,6 @@ export function DialogSelect<T>(props: DialogSelectProps<T>) {

let input: InputRenderable

const actions = createMemo(() => props.actions ?? [])
const shownActions = createMemo(() => actions().filter((item) => !item.hidden))
const actionBindings = useKeymapSelector((keymap) =>
keymap.getCommandBindings({
visibility: "registered",
commands: shownActions().map((item) => item.command),
}),
)

const actionLabels = createMemo(() => {
const labels = new Map<string, string>()

for (const action of shownActions()) {
const label = formatKeyBindings(actionBindings().get(action.command), config)
if (label) labels.set(action.command, label)
}

return labels
})
const visibleActions = createMemo(() => [
...shownActions()
.map((item) => ({ ...item, label: actionLabels().get(item.command) ?? "" }))
.filter((item) => item.label),
...(props.footerHints ?? []),
])
const actionItems = createMemo(() =>
visibleActions()
.filter(isActionItem)
.filter((item) => !isActionDisabled(item)),
)

createEffect(() => {
const index = focusedAction()
if (index !== undefined && index >= actionItems().length) setFocusedAction(undefined)
})

const filtered = createMemo(() => {
if (props.skipFilter || props.renderFilter === false) return props.options.filter((x) => x.disabled !== true)
const needle = store.filter.toLowerCase()
Expand Down Expand Up @@ -229,6 +193,44 @@ export function DialogSelect<T>(props: DialogSelectProps<T>) {

const selected = createMemo(() => flat()[store.selected])

// Action availability depends on the selected option, so initialize the
// option graph before registering action bindings that may run immediately.
const actions = createMemo(() => props.actions ?? [])
const shownActions = createMemo(() => actions().filter((item) => !item.hidden))
const actionBindings = useKeymapSelector((keymap) =>
keymap.getCommandBindings({
visibility: "registered",
commands: shownActions().map((item) => item.command),
}),
)

const actionLabels = createMemo(() => {
const labels = new Map<string, string>()

for (const action of shownActions()) {
const label = formatKeyBindings(actionBindings().get(action.command), config)
if (label) labels.set(action.command, label)
}

return labels
})
const visibleActions = createMemo(() => [
...shownActions()
.map((item) => ({ ...item, label: actionLabels().get(item.command) ?? "" }))
.filter((item) => item.label),
...(props.footerHints ?? []),
])
const actionItems = createMemo(() =>
visibleActions()
.filter(isActionItem)
.filter((item) => !isActionDisabled(item)),
)

createEffect(() => {
const command = focusedAction()
if (command && !actionItems().some((item) => item.command === command)) setFocusedAction(undefined)
})

createEffect(
on(
() => props.options,
Expand Down Expand Up @@ -363,9 +365,9 @@ export function DialogSelect<T>(props: DialogSelectProps<T>) {
function submit() {
if (props.locked) return
setStore("input", "keyboard")
const index = focusedAction()
if (index !== undefined) {
trigger(actionItems()[index])
const command = focusedAction()
if (command) {
trigger(actionItems().find((item) => item.command === command))
return
}
const option = selected()
Expand All @@ -376,12 +378,14 @@ export function DialogSelect<T>(props: DialogSelectProps<T>) {

function moveAction(direction: 1 | -1) {
if (props.locked) return
const total = actionItems().length
if (total === 0) return
setFocusedAction((index) => {
if (index === undefined) return direction === 1 ? 0 : total - 1
const items = actionItems()
if (items.length === 0) return
setFocusedAction((command) => {
if (!command) return items[direction === 1 ? 0 : items.length - 1].command
const index = items.findIndex((item) => item.command === command)
if (index === -1) return items[direction === 1 ? 0 : items.length - 1].command
const next = index + direction
return next < 0 || next >= total ? undefined : next
return next < 0 || next >= items.length ? undefined : items[next].command
})
}

Expand Down Expand Up @@ -537,7 +541,7 @@ export function DialogSelect<T>(props: DialogSelectProps<T>) {
function isActionFocused(item: VisibleAction) {
if (props.locked) return false
if (!isActionItem(item)) return false
return actionItems().indexOf(item) === focusedAction()
return item.command === focusedAction()
}

function FooterAction(action: { item: VisibleAction }) {
Expand Down
39 changes: 38 additions & 1 deletion packages/tui/test/cli/tui/dialog-select.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,14 +104,19 @@ async function mountSelect(root: string, initial: DialogSelectOption<string>[])

const selected: string[] = []
const moved: string[] = []
const globals: number[] = []
const rows: string[] = []
let replaceOptions!: (options: DialogSelectOption<string>[]) => void
let disableRow!: () => void

function Harness() {
const renderer = useRenderer()
const keymap = createDefaultOpenTuiKeymap(renderer)
const off = registerOpencodeKeymap(keymap, renderer, config)
const [options, setOptions] = createSignal(initial)
const [rowEnabled, setRowEnabled] = createSignal(true)
replaceOptions = setOptions
disableRow = () => setRowEnabled(false)
onCleanup(off)

function Fixture() {
Expand All @@ -121,8 +126,23 @@ async function mountSelect(root: string, initial: DialogSelectOption<string>[])
<DialogSelect
title="Mutable options"
options={options()}
current={initial[0]?.value}
onMove={(option) => moved.push(option.value)}
onSelect={(option) => selected.push(option.value)}
actions={[
{
command: "dialog.move_session.delete",
title: "delete",
disabled: !rowEnabled(),
onTrigger: (option) => rows.push(option.value),
},
{
command: "dialog.move_session.new",
title: "new",
selection: "none",
onTrigger: () => globals.push(1),
},
]}
/>
)),
)
Expand Down Expand Up @@ -150,7 +170,7 @@ async function mountSelect(root: string, initial: DialogSelectOption<string>[])
app.renderer.start()
await app.waitForFrame((frame) => frame.includes("Mutable options"))
await app.waitFor(() => app.renderer.currentFocusedEditor instanceof InputRenderable)
return { app, moved, replaceOptions, selected }
return { app, disableRow, globals, moved, replaceOptions, rows, selected }
}

test("dialog actions run without options while row actions still require a selection", async () => {
Expand Down Expand Up @@ -201,6 +221,23 @@ test("footer actions run when filtering leaves no selected row", async () => {
}
})

test("does not move focus when the focused action becomes disabled", async () => {
await using tmp = await tmpdir()
const select = await mountSelect(tmp.path, [{ title: "Alpha", value: "alpha" }])

try {
select.app.mockInput.pressTab()
select.disableRow()
select.app.mockInput.pressEnter()

expect(select.globals).toEqual([])
expect(select.rows).toEqual([])
expect(select.selected).toEqual(["alpha"])
} finally {
select.app.renderer.destroy()
}
})

test("row actions receive the selected option", async () => {
await using tmp = await tmpdir()
const rows: string[] = []
Expand Down
Loading