fix(tui): address form review regressions
This commit is contained in:
parent
67b9498c04
commit
749ad0e125
6 changed files with 90 additions and 43 deletions
|
|
@ -667,6 +667,12 @@ export const { use: useData, provider: DataProvider } = createSimpleContext({
|
|||
setStore("session", "info", sessionID, mutable(await sdk.api.session.get({ sessionID })))
|
||||
registerSession(sessionID)
|
||||
},
|
||||
async refreshChildren(sessionID: string) {
|
||||
for (const session of mutable((await sdk.api.session.list({ parentID: sessionID, limit: 200 })).data)) {
|
||||
setStore("session", "info", session.id, session)
|
||||
registerSession(session.id)
|
||||
}
|
||||
},
|
||||
message: {
|
||||
ids(sessionID: string) {
|
||||
return (store.session.message[sessionID] ?? []).map((message) => message.id)
|
||||
|
|
|
|||
|
|
@ -46,13 +46,13 @@ const tui: TuiPlugin = async (api) => {
|
|||
forms.delete(event.data.id)
|
||||
})
|
||||
|
||||
api.event.on("permission.asked", (event) => {
|
||||
api.event.on("permission.v2.asked", (event) => {
|
||||
if (permissions.has(event.data.id)) return
|
||||
permissions.add(event.data.id)
|
||||
notify(api, event.data.sessionID, "Permission needs input", "permission")
|
||||
})
|
||||
|
||||
api.event.on("permission.replied", (event) => {
|
||||
api.event.on("permission.v2.replied", (event) => {
|
||||
permissions.delete(event.data.requestID)
|
||||
})
|
||||
|
||||
|
|
@ -79,9 +79,12 @@ const tui: TuiPlugin = async (api) => {
|
|||
api.event.on("session.next.step.started", (event) => started(event.data.sessionID))
|
||||
api.event.on("session.next.retried", (event) => started(event.data.sessionID))
|
||||
api.event.on("session.next.compaction.started", (event) => started(event.data.sessionID))
|
||||
api.event.on("session.next.shell.ended", (event) => ended(event.data.sessionID))
|
||||
api.event.on("session.next.step.ended", (event) => {
|
||||
if (event.data.finish === "tool-calls") return
|
||||
api.event.on("session.next.execution.settled", (event) => {
|
||||
if (event.data.outcome === "failure" && active.has(event.data.sessionID) && !errored.has(event.data.sessionID)) {
|
||||
errored.add(event.data.sessionID)
|
||||
notify(api, event.data.sessionID, "Session error", "error")
|
||||
}
|
||||
if (event.data.outcome === "interrupted") errored.add(event.data.sessionID)
|
||||
ended(event.data.sessionID)
|
||||
})
|
||||
api.event.on("session.next.step.failed", (event) => {
|
||||
|
|
|
|||
|
|
@ -120,10 +120,19 @@ function stateApi(sync: ReturnType<typeof useSync>, data: ReturnType<typeof useD
|
|||
},
|
||||
session: {
|
||||
count() {
|
||||
return sync.data.session.length
|
||||
return data.session.list().length
|
||||
},
|
||||
get(sessionID) {
|
||||
return sync.session.get(sessionID)
|
||||
const session = data.session.get(sessionID)
|
||||
if (!session) return
|
||||
return {
|
||||
...session,
|
||||
slug: session.id,
|
||||
workspaceID: session.location.workspaceID,
|
||||
directory: session.location.directory,
|
||||
path: session.subpath,
|
||||
version: "2",
|
||||
}
|
||||
},
|
||||
diff(sessionID) {
|
||||
return (sync.data.session_diff[sessionID] ?? []).flatMap((item) =>
|
||||
|
|
@ -140,7 +149,15 @@ function stateApi(sync: ReturnType<typeof useSync>, data: ReturnType<typeof useD
|
|||
return data.session.status(sessionID) === "running" ? { type: "busy" } : { type: "idle" }
|
||||
},
|
||||
permission(sessionID) {
|
||||
return sync.data.permission[sessionID] ?? []
|
||||
return (data.session.permission.list(sessionID) ?? []).map((request) => ({
|
||||
id: request.id,
|
||||
sessionID: request.sessionID,
|
||||
permission: request.action,
|
||||
patterns: request.resources,
|
||||
metadata: request.metadata ?? {},
|
||||
always: request.save ?? [],
|
||||
tool: request.source?.type === "tool" ? { messageID: request.source.messageID, callID: request.source.callID } : undefined,
|
||||
}))
|
||||
},
|
||||
question(sessionID) {
|
||||
return sync.data.question[sessionID] ?? []
|
||||
|
|
|
|||
|
|
@ -52,6 +52,14 @@ function validateText(field: Field, text: string): string | undefined {
|
|||
if (field.format === "date-time" && Number.isNaN(new Date(text).getTime())) return "Expected a date and time"
|
||||
}
|
||||
|
||||
function validateSelection(field: Field, value: FormValue | undefined) {
|
||||
if (field.type !== "multiselect" || value === undefined) return
|
||||
if (!Array.isArray(value)) return "Expected selections"
|
||||
if (field.required && value.length === 0) return "Select at least one option"
|
||||
if (field.minItems !== undefined && value.length < field.minItems) return `Select at least ${field.minItems}`
|
||||
if (field.maxItems !== undefined && value.length > field.maxItems) return `Select at most ${field.maxItems}`
|
||||
}
|
||||
|
||||
function fieldRows(field: Field): { value: FormValue; label: string; description?: string }[] {
|
||||
if (field.type === "boolean")
|
||||
return [
|
||||
|
|
@ -187,6 +195,7 @@ function FieldsPrompt(props: { form: FormInfo & { mode: "form" } }) {
|
|||
})
|
||||
const single = createMemo(() => {
|
||||
const list = fields()
|
||||
if (props.form.fields.length !== 1) return false
|
||||
if (list.length !== 1) return false
|
||||
const field = list[0]!
|
||||
return field.type === "boolean" || (field.type === "string" && field.options !== undefined)
|
||||
|
|
@ -199,7 +208,12 @@ function FieldsPrompt(props: { form: FormInfo & { mode: "form" } }) {
|
|||
)
|
||||
return width <= dimensions().width - 8
|
||||
})
|
||||
const answered = createMemo(() => fields().filter((item) => store.answers[item.key] !== undefined).length)
|
||||
const answered = createMemo(() =>
|
||||
fields().filter((item) => {
|
||||
const value = store.answers[item.key]
|
||||
return Array.isArray(value) ? value.length > 0 : value !== undefined
|
||||
}).length,
|
||||
)
|
||||
const field = createMemo(() => fields()[Math.min(store.tab, fields().length - 1)])
|
||||
const confirm = createMemo(() => !single() && store.tab >= fields().length)
|
||||
const rows = createMemo(() => (field() ? fieldRows(field()!) : []))
|
||||
|
|
@ -285,7 +299,7 @@ function FieldsPrompt(props: { form: FormInfo & { mode: "form" } }) {
|
|||
const index = list.indexOf(value)
|
||||
if (index === -1) list.push(value)
|
||||
if (index !== -1) list.splice(index, 1)
|
||||
answer(current.key, list)
|
||||
answer(current.key, list.length === 0 ? undefined : list)
|
||||
}
|
||||
|
||||
function selectTab(index: number) {
|
||||
|
|
@ -426,7 +440,7 @@ function FieldsPrompt(props: { form: FormInfo & { mode: "form" } }) {
|
|||
if (prev) {
|
||||
const existing = store.answers[current.key]
|
||||
const list = Array.isArray(existing) ? existing.filter((item) => item !== prev) : []
|
||||
answer(current.key, list)
|
||||
answer(current.key, list.length === 0 ? undefined : list)
|
||||
setStore("custom", { ...store.custom, [current.key]: "" })
|
||||
}
|
||||
setStore("editing", false)
|
||||
|
|
@ -521,6 +535,11 @@ function FieldsPrompt(props: { form: FormInfo & { mode: "form" } }) {
|
|||
desc: "Submit form",
|
||||
group: "Form",
|
||||
cmd: () => {
|
||||
const invalid = fields().find((field) => validateSelection(field, store.answers[field.key]))
|
||||
if (invalid) {
|
||||
setStore("error", validateSelection(invalid, store.answers[invalid.key]) ?? "Invalid answer")
|
||||
return
|
||||
}
|
||||
sdk.api.form
|
||||
.reply({
|
||||
sessionID: props.form.sessionID,
|
||||
|
|
@ -824,14 +843,18 @@ function FieldsPrompt(props: { form: FormInfo & { mode: "form" } }) {
|
|||
<For each={fields()}>
|
||||
{(item) => {
|
||||
const value = () => display(item, store.answers[item.key])
|
||||
const answered = () => store.answers[item.key] !== undefined
|
||||
const answered = () => {
|
||||
const value = store.answers[item.key]
|
||||
return Array.isArray(value) ? value.length > 0 : value !== undefined
|
||||
}
|
||||
const missing = () => !answered() && item.required === true
|
||||
const invalid = () => validateSelection(item, store.answers[item.key])
|
||||
return (
|
||||
<box paddingLeft={1}>
|
||||
<text>
|
||||
<span style={{ fg: theme.textMuted }}>{truncate(fieldLabel(item), 40)}:</span>{" "}
|
||||
<span style={{ fg: answered() ? theme.text : missing() ? theme.error : theme.textMuted }}>
|
||||
{answered() ? value() : missing() ? "(required)" : "(not answered)"}
|
||||
<span style={{ fg: invalid() || missing() ? theme.error : answered() ? theme.text : theme.textMuted }}>
|
||||
{invalid() ?? (answered() ? value() : missing() ? "(required)" : "(not answered)")}
|
||||
</span>
|
||||
</text>
|
||||
</box>
|
||||
|
|
|
|||
|
|
@ -182,9 +182,10 @@ export function Session() {
|
|||
)
|
||||
})
|
||||
const forms = createMemo(() => {
|
||||
if (session()?.parentID) return []
|
||||
return [
|
||||
...[route.sessionID, ...descendantSessionIDs()].flatMap((sessionID) => data.session.form.list(sessionID) ?? []),
|
||||
...[route.sessionID, ...(session()?.parentID ? [] : descendantSessionIDs())].flatMap(
|
||||
(sessionID) => data.session.form.list(sessionID) ?? [],
|
||||
),
|
||||
...(data.session.form.list("global") ?? []),
|
||||
]
|
||||
})
|
||||
|
|
@ -251,12 +252,7 @@ export function Session() {
|
|||
createEffect(() => {
|
||||
const sessionID = route.sessionID
|
||||
void (async () => {
|
||||
await Promise.all([
|
||||
data.session.refresh(sessionID),
|
||||
data.session.permission.refresh(sessionID),
|
||||
data.session.form.refresh(sessionID),
|
||||
data.session.form.refresh("global"),
|
||||
])
|
||||
await data.session.refresh(sessionID)
|
||||
const info = data.session.get(sessionID)
|
||||
if (!info) {
|
||||
toast.show({
|
||||
|
|
@ -267,6 +263,12 @@ export function Session() {
|
|||
navigate({ type: "home" })
|
||||
return
|
||||
}
|
||||
if (!info.parentID) await data.session.refreshChildren(sessionID)
|
||||
await Promise.all([
|
||||
data.session.permission.refresh(sessionID),
|
||||
data.session.form.refresh(sessionID),
|
||||
data.session.form.refresh("global"),
|
||||
])
|
||||
|
||||
project.workspace.set(info.location.workspaceID)
|
||||
editor.reconnect(info.location.directory)
|
||||
|
|
@ -944,7 +946,6 @@ export function Session() {
|
|||
onClose={() => setComposer("open", false)}
|
||||
/>
|
||||
<Switch>
|
||||
<Match when={composer.open || !!session()?.parentID}>{null}</Match>
|
||||
<Match when={permissions().length > 0}>
|
||||
<PermissionPrompt request={permissions()[0]} directory={session()?.location.directory} />
|
||||
</Match>
|
||||
|
|
@ -953,6 +954,7 @@ export function Session() {
|
|||
{(form) => <FormPrompt form={form} />}
|
||||
</Show>
|
||||
</Match>
|
||||
<Match when={composer.open || !!session()?.parentID}>{null}</Match>
|
||||
<Match when={!disabled()}>
|
||||
<pluginRuntime.Slot
|
||||
name="session_prompt"
|
||||
|
|
|
|||
|
|
@ -1,6 +1,6 @@
|
|||
import { describe, expect, test } from "bun:test"
|
||||
import Notifications from "../../../../src/feature-plugins/system/notifications"
|
||||
import type { PermissionRequest, Session, V2Event } from "@opencode-ai/sdk/v2"
|
||||
import type { PermissionV2Request, Session, V2Event } from "@opencode-ai/sdk/v2"
|
||||
import type { TuiAttentionNotifyInput } from "@opencode-ai/plugin/tui"
|
||||
import { createTuiPluginApi } from "../../../fixture/tui-plugin"
|
||||
|
||||
|
|
@ -76,14 +76,13 @@ function form(id: string, sessionID = "session"): Extract<V2Event, { type: "form
|
|||
}
|
||||
}
|
||||
|
||||
function permission(id: string, sessionID = "session"): PermissionRequest {
|
||||
function permission(id: string, sessionID = "session"): PermissionV2Request {
|
||||
return {
|
||||
id,
|
||||
sessionID,
|
||||
permission: "edit",
|
||||
patterns: [],
|
||||
action: "edit",
|
||||
resources: [],
|
||||
metadata: {},
|
||||
always: [],
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -101,17 +100,14 @@ function stepStarted(id: string, sessionID = "session"): V2Event {
|
|||
}
|
||||
}
|
||||
|
||||
function stepEnded(id: string, sessionID = "session", finish = "stop"): V2Event {
|
||||
function executionSettled(id: string, sessionID = "session"): V2Event {
|
||||
return {
|
||||
id,
|
||||
type: "session.next.step.ended",
|
||||
type: "session.next.execution.settled",
|
||||
data: {
|
||||
sessionID,
|
||||
assistantMessageID: `msg_${id}`,
|
||||
timestamp: 0,
|
||||
finish,
|
||||
cost: 0,
|
||||
tokens: { input: 0, output: 0, reasoning: 0, cache: { read: 0, write: 0 } },
|
||||
outcome: "success",
|
||||
},
|
||||
}
|
||||
}
|
||||
|
|
@ -148,7 +144,7 @@ describe("internal notifications TUI plugin", () => {
|
|||
const harness = await setup()
|
||||
|
||||
harness.emit({ id: "event-1", type: "form.created", data: { form: form("form-1") } })
|
||||
harness.emit({ id: "event-2", type: "permission.asked", data: permission("permission-1") })
|
||||
harness.emit({ id: "event-2", type: "permission.v2.asked", data: permission("permission-1") })
|
||||
|
||||
expect(harness.notifications).toEqual([formNotification, permissionNotification])
|
||||
})
|
||||
|
|
@ -165,14 +161,14 @@ describe("internal notifications TUI plugin", () => {
|
|||
})
|
||||
harness.emit({ id: "event-4", type: "form.created", data: { form: form("form-1") } })
|
||||
|
||||
harness.emit({ id: "event-5", type: "permission.asked", data: permission("permission-1") })
|
||||
harness.emit({ id: "event-6", type: "permission.asked", data: permission("permission-1") })
|
||||
harness.emit({ id: "event-5", type: "permission.v2.asked", data: permission("permission-1") })
|
||||
harness.emit({ id: "event-6", type: "permission.v2.asked", data: permission("permission-1") })
|
||||
harness.emit({
|
||||
id: "event-7",
|
||||
type: "permission.replied",
|
||||
type: "permission.v2.replied",
|
||||
data: { sessionID: "session", requestID: "permission-1", reply: "once" },
|
||||
})
|
||||
harness.emit({ id: "event-8", type: "permission.asked", data: permission("permission-1") })
|
||||
harness.emit({ id: "event-8", type: "permission.v2.asked", data: permission("permission-1") })
|
||||
|
||||
expect(harness.notifications).toEqual([
|
||||
formNotification,
|
||||
|
|
@ -185,9 +181,9 @@ describe("internal notifications TUI plugin", () => {
|
|||
test("notifies when an active session becomes idle and suppresses no-op idle", async () => {
|
||||
const harness = await setup()
|
||||
|
||||
harness.emit(stepEnded("event-1"))
|
||||
harness.emit(executionSettled("event-1"))
|
||||
harness.emit(stepStarted("event-2"))
|
||||
harness.emit(stepEnded("event-3"))
|
||||
harness.emit(executionSettled("event-3"))
|
||||
|
||||
expect(harness.notifications).toEqual([
|
||||
{
|
||||
|
|
@ -204,7 +200,7 @@ describe("internal notifications TUI plugin", () => {
|
|||
|
||||
harness.emit({ id: "event-1", type: "form.created", data: { form: form("form-1", "subagent") } })
|
||||
harness.emit(stepStarted("event-2", "subagent"))
|
||||
harness.emit(stepEnded("event-3", "subagent"))
|
||||
harness.emit(executionSettled("event-3", "subagent"))
|
||||
|
||||
expect(harness.notifications).toEqual([
|
||||
{
|
||||
|
|
@ -227,7 +223,7 @@ describe("internal notifications TUI plugin", () => {
|
|||
|
||||
harness.emit(stepStarted("event-1"))
|
||||
harness.emit(stepFailed("event-2"))
|
||||
harness.emit(stepEnded("event-3"))
|
||||
harness.emit(executionSettled("event-3"))
|
||||
|
||||
expect(harness.notifications).toEqual([
|
||||
{
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue