diff --git a/packages/client/src/promise/generated/types.ts b/packages/client/src/promise/generated/types.ts index b74f223006e..17d21221a70 100644 --- a/packages/client/src/promise/generated/types.ts +++ b/packages/client/src/promise/generated/types.ts @@ -2510,7 +2510,7 @@ export type FormRequestListOutput = { custom?: boolean default?: Array } - | { type: "external"; url: string; title?: string; description?: string } + | { key: string; type: "external"; url: string; title?: string; description?: string } ), ...Array< | { @@ -2593,7 +2593,7 @@ export type FormRequestListOutput = { custom?: boolean default?: Array } - | { type: "external"; url: string; title?: string; description?: string } + | { key: string; type: "external"; url: string; title?: string; description?: string } >, ] }> @@ -2689,7 +2689,7 @@ export type FormListOutput = { custom?: boolean default?: Array } - | { type: "external"; url: string; title?: string; description?: string } + | { key: string; type: "external"; url: string; title?: string; description?: string } ), ...Array< | { @@ -2772,7 +2772,7 @@ export type FormListOutput = { custom?: boolean default?: Array } - | { type: "external"; url: string; title?: string; description?: string } + | { key: string; type: "external"; url: string; title?: string; description?: string } >, ] }> @@ -2874,7 +2874,13 @@ export type FormCreateInput = { readonly custom?: boolean readonly default?: ReadonlyArray } - | { readonly type: "external"; readonly url: string; readonly title?: string; readonly description?: string } + | { + readonly key: string + readonly type: "external" + readonly url: string + readonly title?: string + readonly description?: string + } ), ...Array< | { @@ -2965,7 +2971,13 @@ export type FormCreateInput = { readonly custom?: boolean readonly default?: ReadonlyArray } - | { readonly type: "external"; readonly url: string; readonly title?: string; readonly description?: string } + | { + readonly key: string + readonly type: "external" + readonly url: string + readonly title?: string + readonly description?: string + } >, ] }["id"] @@ -3063,7 +3075,13 @@ export type FormCreateInput = { readonly custom?: boolean readonly default?: ReadonlyArray } - | { readonly type: "external"; readonly url: string; readonly title?: string; readonly description?: string } + | { + readonly key: string + readonly type: "external" + readonly url: string + readonly title?: string + readonly description?: string + } ), ...Array< | { @@ -3154,7 +3172,13 @@ export type FormCreateInput = { readonly custom?: boolean readonly default?: ReadonlyArray } - | { readonly type: "external"; readonly url: string; readonly title?: string; readonly description?: string } + | { + readonly key: string + readonly type: "external" + readonly url: string + readonly title?: string + readonly description?: string + } >, ] }["title"] @@ -3252,7 +3276,13 @@ export type FormCreateInput = { readonly custom?: boolean readonly default?: ReadonlyArray } - | { readonly type: "external"; readonly url: string; readonly title?: string; readonly description?: string } + | { + readonly key: string + readonly type: "external" + readonly url: string + readonly title?: string + readonly description?: string + } ), ...Array< | { @@ -3343,7 +3373,13 @@ export type FormCreateInput = { readonly custom?: boolean readonly default?: ReadonlyArray } - | { readonly type: "external"; readonly url: string; readonly title?: string; readonly description?: string } + | { + readonly key: string + readonly type: "external" + readonly url: string + readonly title?: string + readonly description?: string + } >, ] }["metadata"] @@ -3441,7 +3477,13 @@ export type FormCreateInput = { readonly custom?: boolean readonly default?: ReadonlyArray } - | { readonly type: "external"; readonly url: string; readonly title?: string; readonly description?: string } + | { + readonly key: string + readonly type: "external" + readonly url: string + readonly title?: string + readonly description?: string + } ), ...Array< | { @@ -3532,7 +3574,13 @@ export type FormCreateInput = { readonly custom?: boolean readonly default?: ReadonlyArray } - | { readonly type: "external"; readonly url: string; readonly title?: string; readonly description?: string } + | { + readonly key: string + readonly type: "external" + readonly url: string + readonly title?: string + readonly description?: string + } >, ] }["fields"] @@ -3626,7 +3674,7 @@ export type FormCreateOutput = { custom?: boolean default?: Array } - | { type: "external"; url: string; title?: string; description?: string } + | { key: string; type: "external"; url: string; title?: string; description?: string } ), ...Array< | { @@ -3709,7 +3757,7 @@ export type FormCreateOutput = { custom?: boolean default?: Array } - | { type: "external"; url: string; title?: string; description?: string } + | { key: string; type: "external"; url: string; title?: string; description?: string } >, ] } @@ -3808,7 +3856,7 @@ export type FormGetOutput = { custom?: boolean default?: Array } - | { type: "external"; url: string; title?: string; description?: string } + | { key: string; type: "external"; url: string; title?: string; description?: string } ), ...Array< | { @@ -3891,7 +3939,7 @@ export type FormGetOutput = { custom?: boolean default?: Array } - | { type: "external"; url: string; title?: string; description?: string } + | { key: string; type: "external"; url: string; title?: string; description?: string } >, ] } @@ -5429,7 +5477,7 @@ export type EventSubscribeOutput = custom?: boolean default?: Array } - | { type: "external"; url: string; title?: string; description?: string } + | { key: string; type: "external"; url: string; title?: string; description?: string } ), ...Array< | { @@ -5492,7 +5540,7 @@ export type EventSubscribeOutput = custom?: boolean default?: Array } - | { type: "external"; url: string; title?: string; description?: string } + | { key: string; type: "external"; url: string; title?: string; description?: string } >, ] } diff --git a/packages/core/src/form.ts b/packages/core/src/form.ts index 8c810e6c8ae..30ae70e16c2 100644 --- a/packages/core/src/form.ts +++ b/packages/core/src/form.ts @@ -224,15 +224,16 @@ export const locationLayer = layer export const node = makeLocationNode({ service: Service, layer, deps: [EventV2.node] }) function validateAnswer(form: Info, answer: Answer) { - const fields = new Map( - form.fields.flatMap((field) => (field.type === "external" ? [] : [[field.key, field] as const])), - ) + const fields = new Map(form.fields.map((field) => [field.key, field] as const)) for (const key of Object.keys(answer)) { if (!fields.has(key)) return `Unknown form field: ${key}` } for (const field of form.fields) { - if (field.type === "external") continue const value = answer[field.key] + if (field.type === "external") { + if (value !== true) return `External form field must be acknowledged: ${field.key}` + continue + } const active = isActive(field, answer) if (value === undefined) { if (field.required && active) return `Missing required form field: ${field.key}` @@ -266,9 +267,11 @@ function matches(when: Form.When, value: Form.Value | undefined) { function validateFields(fields: ReadonlyArray) { if (fields.length === 0) return "Form must have at least one field" const earlier = new Map() + const keys = new Set() for (const field of fields) { + if (keys.has(field.key)) return `Duplicate form field key: ${field.key}` + keys.add(field.key) if (field.type === "external") continue - if (earlier.has(field.key)) return `Duplicate form field key: ${field.key}` for (const when of field.when ?? []) { const target = earlier.get(when.key) if (!target) return `Form field condition must reference an earlier field: ${field.key} -> ${when.key}` diff --git a/packages/core/src/mcp/index.ts b/packages/core/src/mcp/index.ts index 0da4ed01eeb..731e4a08adb 100644 --- a/packages/core/src/mcp/index.ts +++ b/packages/core/src/mcp/index.ts @@ -311,7 +311,7 @@ export const layer = Layer.effect( elicitationID: input.params.elicitationId, message: input.params.message, }, - fields: [{ type: "external", url: input.params.url }], + fields: [{ key: "elicitation", type: "external", url: input.params.url }], }) .pipe( Effect.raceFirst(waitForAbort(input.signal)), @@ -355,7 +355,7 @@ export const layer = Layer.effect( Effect.gen(function* () { const formID = urlElicitations.get(input.server + "\u0000" + input.elicitationID) if (!formID) return - yield* forms.reply({ id: formID, answer: {} }).pipe(Effect.ignore) + yield* forms.reply({ id: formID, answer: { elicitation: true } }).pipe(Effect.ignore) }), } satisfies MCPClient.ElicitationHandler diff --git a/packages/core/test/form.test.ts b/packages/core/test/form.test.ts index 44893d3b826..33283722032 100644 --- a/packages/core/test/form.test.ts +++ b/packages/core/test/form.test.ts @@ -61,10 +61,10 @@ describe("Form", () => { const externalOnly = yield* service.create({ sessionID: "global", title: "External setup", - fields: [{ type: "external", url: "https://example.com/setup" }], + fields: [{ key: "setup", type: "external", url: "https://example.com/setup" }], }) - yield* service.reply({ id: externalOnly.id, answer: {} }) - expect(yield* service.state(externalOnly.id)).toEqual({ status: "answered", answer: {} }) + yield* service.reply({ id: externalOnly.id, answer: { setup: true } }) + expect(yield* service.state(externalOnly.id)).toEqual({ status: "answered", answer: { setup: true } }) }), ) @@ -247,6 +247,13 @@ describe("Form", () => { ]), ).toEqual(new Form.InvalidFormError({ message: "Duplicate form field key: a" })) + expect( + yield* flipCreate([ + { key: "a", type: "external", url: "https://example.com" }, + { key: "a", type: "string" }, + ]), + ).toEqual(new Form.InvalidFormError({ message: "Duplicate form field key: a" })) + expect( yield* flipCreate([ { key: "a", type: "boolean" }, @@ -267,25 +274,37 @@ describe("Form", () => { }), ) - it.effect("treats external fields as non-answerable", () => + it.effect("requires external field acknowledgements", () => Effect.gen(function* () { const service = yield* Form.Service const created = yield* service.create({ sessionID: "global", title: "External setup", fields: [ - { type: "external", url: "https://example.com/setup", title: "Open setup" }, + { key: "authorization", type: "external", url: "https://example.com/setup", title: "Open setup" }, { key: "name", type: "string", required: true }, ], }) - const invalid = yield* service - .reply({ id: created.id, answer: { link: "opened", name: "Ava" } }) - .pipe(Effect.flip) - expect(invalid).toEqual(new Form.InvalidAnswerError({ id: created.id, message: "Unknown form field: link" })) + const invalidAnswers: ReadonlyArray = [ + { name: "Ava" }, + { authorization: false, name: "Ava" }, + { authorization: "yes", name: "Ava" }, + ] + for (const answer of invalidAnswers) { + expect(yield* service.reply({ id: created.id, answer }).pipe(Effect.flip)).toEqual( + new Form.InvalidAnswerError({ + id: created.id, + message: "External form field must be acknowledged: authorization", + }), + ) + } - yield* service.reply({ id: created.id, answer: { name: "Ava" } }) - expect(yield* service.state(created.id)).toEqual({ status: "answered", answer: { name: "Ava" } }) + yield* service.reply({ id: created.id, answer: { authorization: true, name: "Ava" } }) + expect(yield* service.state(created.id)).toEqual({ + status: "answered", + answer: { authorization: true, name: "Ava" }, + }) }), ) diff --git a/packages/core/test/mcp.test.ts b/packages/core/test/mcp.test.ts index 727247ba5c1..68ee1eb0ac0 100644 --- a/packages/core/test/mcp.test.ts +++ b/packages/core/test/mcp.test.ts @@ -47,7 +47,9 @@ type ResourceTemplatePage = { nextCursor?: string } -function resourceServer(input: { resources?: boolean; listChanged?: boolean; emptyElicitation?: boolean } = {}) { +function resourceServer( + input: { resources?: boolean; listChanged?: boolean; emptyElicitation?: boolean; urlElicitation?: boolean } = {}, +) { return Effect.acquireRelease( Effect.promise(async () => { const state = { @@ -75,7 +77,9 @@ function resourceServer(input: { resources?: boolean; listChanged?: boolean; emp Promise.resolve({ tools: input.emptyElicitation ? [{ name: "empty-elicitation", inputSchema: { type: "object" as const, properties: {} } }] - : [], + : input.urlElicitation + ? [{ name: "url-elicitation", inputSchema: { type: "object" as const, properties: {} } }] + : [], }), ) if (input.emptyElicitation) { @@ -91,6 +95,20 @@ function resourceServer(input: { resources?: boolean; listChanged?: boolean; emp } }) } + if (input.urlElicitation) { + protocol.setRequestHandler(CallToolRequestSchema, async () => { + const result = await protocol.elicitInput({ + mode: "url", + message: "Authorize access", + url: "https://example.com/authorize", + elicitationId: "elicitation-test", + }) + return { + content: [{ type: "text", text: JSON.stringify(result) }], + structuredContent: result, + } + }) + } if (input.resources !== false) { protocol.setRequestHandler(ListResourcesRequestSchema, (request) => { state.resourceLists += 1 @@ -117,6 +135,7 @@ function resourceServer(input: { resources?: boolean; listChanged?: boolean; emp state, url: http.url.toString(), sendResourceListChanged: () => protocol.sendResourceListChanged(), + completeElicitation: () => protocol.createElicitationCompletionNotifier("elicitation-test")(), close: async () => { await protocol.close().catch(() => {}) await http.stop(true) @@ -127,7 +146,10 @@ function resourceServer(input: { resources?: boolean; listChanged?: boolean; emp ) } -function resourceMcpLayer(url: string) { +function resourceMcpLayer( + url: string, + form: Partial = { ask: () => Effect.die("Empty MCP elicitation must not create a form") }, +) { const directory = AbsolutePath.make(import.meta.dir) const unusedIntegration = () => Effect.die("unused integration service") return MCP.layer.pipe( @@ -159,9 +181,7 @@ function resourceMcpLayer(url: string) { data, } as EventV2.Payload), }), - Layer.mock(Form.Service, { - ask: () => Effect.die("Empty MCP elicitation must not create a form"), - }), + Layer.mock(Form.Service, form), Layer.mock(Integration.Service, { connection: { active: unusedIntegration, @@ -527,6 +547,42 @@ test("accepts empty MCP elicitations without creating forms", async () => { ) }) +test("acknowledges completed MCP URL elicitations without returning internal content", async () => { + await Effect.runPromise( + Effect.scoped( + Effect.gen(function* () { + const server = yield* resourceServer({ resources: false, urlElicitation: true }) + const asked = yield* Deferred.make() + const terminal = yield* Deferred.make() + const replied = yield* Deferred.make() + const call = yield* Effect.gen(function* () { + const service = yield* MCP.Service + return yield* service.callTool({ server: "resources", name: "url-elicitation" }) + }).pipe( + Effect.provide( + resourceMcpLayer(server.url, { + ask: (input) => Deferred.succeed(asked, input).pipe(Effect.andThen(Deferred.await(terminal))), + reply: (input) => + Effect.gen(function* () { + yield* Deferred.succeed(replied, input) + yield* Deferred.succeed(terminal, { status: "answered", answer: input.answer }) + }), + }), + ), + Effect.forkScoped, + ) + + const form = yield* Deferred.await(asked) + expect(form.fields).toEqual([{ key: "elicitation", type: "external", url: "https://example.com/authorize" }]) + + yield* Effect.promise(server.completeElicitation) + expect((yield* Deferred.await(replied)).answer).toEqual({ elicitation: true }) + expect((yield* Fiber.join(call)).structured).toEqual({ action: "accept" }) + }), + ), + ) +}) + test("loads and reads MCP resources", async () => { await Effect.runPromise( Effect.scoped( diff --git a/packages/opencode/test/cli/run/noninteractive.test.ts b/packages/opencode/test/cli/run/noninteractive.test.ts index 043dcde413c..2b13155403d 100644 --- a/packages/opencode/test/cli/run/noninteractive.test.ts +++ b/packages/opencode/test/cli/run/noninteractive.test.ts @@ -14,7 +14,7 @@ function form(id: string, sessionID: string): FormInfo { id, sessionID, title: "Input requested", - fields: [{ type: "external", url: "https://example.com/form" }], + fields: [{ key: "authorization", type: "external", url: "https://example.com/form" }], } } diff --git a/packages/opencode/test/server/httpapi-exercise/index.ts b/packages/opencode/test/server/httpapi-exercise/index.ts index 71a0bea3bd6..88617f46ab5 100644 --- a/packages/opencode/test/server/httpapi-exercise/index.ts +++ b/packages/opencode/test/server/httpapi-exercise/index.ts @@ -840,7 +840,7 @@ const scenarios: Scenario[] = [ headers: ctx.headers(), body: { title: "External form", - fields: [{ type: "external", url: "https://example.com/form" }], + fields: [{ key: "authorization", type: "external", url: "https://example.com/form" }], }, })) .json(200, (body) => { diff --git a/packages/schema/src/form.ts b/packages/schema/src/form.ts index c12620b0274..6c2fdc29e80 100644 --- a/packages/schema/src/form.ts +++ b/packages/schema/src/form.ts @@ -95,6 +95,7 @@ export const MultiselectField = Schema.Struct({ export interface MultiselectField extends Schema.Schema.Type {} export const ExternalField = Schema.Struct({ + key: Schema.String, type: Schema.Literal("external"), url: Schema.String, title: Schema.String.pipe(optional), diff --git a/packages/schema/test/contract-hygiene.test.ts b/packages/schema/test/contract-hygiene.test.ts index 8a0484f692c..c441ad88c38 100644 --- a/packages/schema/test/contract-hygiene.test.ts +++ b/packages/schema/test/contract-hygiene.test.ts @@ -62,9 +62,17 @@ describe("contract hygiene", () => { id: Form.ID.create(), sessionID: "global", title: "External form", - fields: [{ type: "external", url: "https://example.com" }], + fields: [{ key: "authorization", type: "external", url: "https://example.com" }], }).fields, ).toHaveLength(1) + expect(() => + Schema.decodeUnknownSync(Form.Info)({ + id: Form.ID.create(), + sessionID: "global", + title: "External form", + fields: [{ type: "external", url: "https://example.com" }], + }), + ).toThrow() }) test("model defaults and provider overlays preserve public invariants", () => { diff --git a/packages/sdk/js/src/v2/gen/types.gen.ts b/packages/sdk/js/src/v2/gen/types.gen.ts index 64c1297d873..56a9bff6b66 100644 --- a/packages/sdk/js/src/v2/gen/types.gen.ts +++ b/packages/sdk/js/src/v2/gen/types.gen.ts @@ -3546,6 +3546,7 @@ export type FormMultiselectField = { } export type FormExternalField = { + key: string type: "external" url: string title?: string diff --git a/packages/tui/src/routes/session/form.tsx b/packages/tui/src/routes/session/form.tsx index 4620a4ee695..e3ec51c48d4 100644 --- a/packages/tui/src/routes/session/form.tsx +++ b/packages/tui/src/routes/session/form.tsx @@ -178,6 +178,7 @@ function FieldsPrompt(props: { form: FormInfo }) { return value === undefined ? [] : [[field.key, value]] }), ) as Record, + externalReady: {} as Record, selected: selectedRow(configuredFields()[0], configuredFields()[0]?.default), editing: false, error: "", @@ -219,7 +220,7 @@ function FieldsPrompt(props: { form: FormInfo }) { }) const answered = createMemo( () => - answerable().filter((item) => { + fields().filter((item) => { const value = store.answers[item.key] return value !== undefined }).length, @@ -262,8 +263,12 @@ function FieldsPrompt(props: { form: FormInfo }) { }) const multi = createMemo(() => answerField()?.type === "multiselect") const actionLabel = createMemo(() => { - if (confirm()) return answerable().length === 0 ? "I finished" : "submit" - if (externalField()) return "open link" + if (confirm()) return "submit" + const external = externalField() + if (external) { + if (store.answers[external.key] === true) return "continue" + return store.externalReady[external.key] ? "I finished" : "open link" + } if (multi()) return "toggle" if (single()) return "submit" return "confirm" @@ -490,10 +495,9 @@ function FieldsPrompt(props: { form: FormInfo }) { function openExternal() { const current = externalField() if (!current) return - const index = store.tab setStore("error", "") void open(current.url) - .then(() => selectTab(index + 1)) + .then(() => setStore("externalReady", { ...store.externalReady, [current.key]: true })) .catch(() => setStore("error", "Could not open the browser. Copy the URL and continue manually.")) } @@ -502,11 +506,34 @@ function FieldsPrompt(props: { form: FormInfo }) { if (!current || !clipboard.write) return void clipboard .write(current.url) - .then(() => toast.show({ message: "Copied URL to clipboard", variant: "info" })) + .then(() => { + setStore("externalReady", { ...store.externalReady, [current.key]: true }) + toast.show({ message: "Copied URL to clipboard", variant: "info" }) + }) .catch(toast.error) } + function acknowledgeExternal() { + const current = externalField() + if (!current) return + if (store.answers[current.key] === true) { + selectTab(store.tab + 1) + return + } + if (!store.externalReady[current.key]) { + openExternal() + return + } + answer(current.key, true) + selectTab(store.tab + 1) + } + function submit() { + const unacknowledged = fields().find((field) => field.type === "external" && store.answers[field.key] !== true) + if (unacknowledged) { + setStore("error", `External action must be acknowledged: ${fieldLabel(unacknowledged)}`) + return + } const invalid = answerable().find((field) => validateValue(field, store.answers[field.key])) if (invalid) { setStore("error", validateValue(invalid, store.answers[invalid.key]) ?? "Invalid answer") @@ -518,7 +545,7 @@ function FieldsPrompt(props: { form: FormInfo }) { sessionID: props.form.sessionID, formID: props.form.id, answer: Object.fromEntries( - answerable().flatMap((field) => { + fields().flatMap((field) => { const value = store.answers[field.key] return value === undefined ? [] : [[field.key, value] as const] }), @@ -618,6 +645,7 @@ function FieldsPrompt(props: { form: FormInfo }) { useBindings(() => { const total = rows().length + (custom() ? 1 : 0) const max = Math.min(total, 9) + const external = externalField() return { mode: FORM_MODE, @@ -657,9 +685,19 @@ function FieldsPrompt(props: { form: FormInfo }) { group: "Form", cmd: () => selectTab((store.tab - 1 + tabs()) % tabs()), }, - ...(externalField() + ...(external ? [ - { key: "return", desc: "Open link", group: "Form", cmd: openExternal }, + { + key: "return", + desc: + store.answers[external.key] === true + ? "Continue" + : store.externalReady[external.key] + ? "Confirm completion" + : "Open link", + group: "Form", + cmd: acknowledgeExternal, + }, { key: "c", desc: "Copy link", group: "Form", cmd: copyExternal }, { key: "escape", desc: "Dismiss form", group: "Form", cmd: cancel }, ...tuiConfig.keybinds.get("app.exit"), @@ -668,7 +706,7 @@ function FieldsPrompt(props: { form: FormInfo }) { ? [ { key: "return", - desc: answerable().length === 0 ? "Finish" : "Submit form", + desc: "Submit form", group: "Form", cmd: submit, }, @@ -752,9 +790,9 @@ function FieldsPrompt(props: { form: FormInfo }) { {confirm() ? "Review" : `Field ${Math.min(store.tab, fields().length - 1) + 1} of ${fields().length}`} - 0}> + 0}> - · {answered()}/{answerable().length} answered + · {answered()}/{fields().length} completed @@ -764,7 +802,7 @@ function FieldsPrompt(props: { form: FormInfo }) { {(item, index) => { const isTab = () => index() === store.tab - const isAnswered = () => item.type !== "external" && store.answers[item.key] !== undefined + const isAnswered = () => store.answers[item.key] !== undefined return ( {external().url} + + {store.answers[external().key] === true + ? "✓ Acknowledged" + : store.externalReady[external().key] + ? "Complete the external action, then press enter to confirm." + : "Open or copy the URL, complete the external action, then confirm."} + )} @@ -963,51 +1008,52 @@ function FieldsPrompt(props: { form: FormInfo }) { - 0}> + Review - 0} - fallback={ - - Complete the browser flow before continuing. - - } + (review = r)} > - (review = r)} - > - - {(item) => { - const value = () => display(item, store.answers[item.key]) - const answered = () => { - const value = store.answers[item.key] - return value !== undefined - } - const missing = () => !answered() && item.required === true - const invalid = () => validateValue(item, store.answers[item.key]) + + {(item) => { + if (item.type === "external") { + const acknowledged = () => store.answers[item.key] === true return ( {truncate(fieldLabel(item), 40)}:{" "} - - {invalid() ?? (answered() ? value() : missing() ? "(required)" : "(not answered)")} + + {acknowledged() ? "Acknowledged" : "(acknowledgement required)"} ) - }} - - - + } + const value = () => display(item, store.answers[item.key]) + const answered = () => store.answers[item.key] !== undefined + const missing = () => !answered() && item.required === true + const invalid = () => validateValue(item, store.answers[item.key]) + return ( + + + {truncate(fieldLabel(item), 40)}:{" "} + + {invalid() ?? (answered() ? value() : missing() ? "(required)" : "(not answered)")} + + + + ) + }} + + select - 0}> + 0}> {"↑↓"} scroll @@ -1040,7 +1086,7 @@ function FieldsPrompt(props: { form: FormInfo }) { onMouseUp={() => { if (renderer.getSelection()?.getSelectedText()) return if (confirm()) submit() - if (externalField()) openExternal() + if (externalField()) acknowledgeExternal() }} > enter {actionLabel()} diff --git a/packages/tui/test/cli/cmd/tui/notifications.test.ts b/packages/tui/test/cli/cmd/tui/notifications.test.ts index efd46c38e8f..a5931c4481a 100644 --- a/packages/tui/test/cli/cmd/tui/notifications.test.ts +++ b/packages/tui/test/cli/cmd/tui/notifications.test.ts @@ -82,7 +82,7 @@ function form(id: string, sessionID = "session"): Extract ({ + default: (url: string) => { + opened.push(url) + return failOpen ? Promise.reject(new Error("open failed")) : Promise.resolve() + }, +})) + +beforeEach(() => { + opened.length = 0 + failOpen = false +}) + +async function wait(fn: () => boolean, timeout = 2000) { + const start = Date.now() + while (!fn()) { + if (Date.now() - start > timeout) throw new Error("timed out waiting for condition") + await Bun.sleep(10) + } +} + +async function mountForm(width = 80) { + const tmp = await tmpdir() + const state = path.join(tmp.path, "state") + await mkdir(state, { recursive: true }) + await Bun.write(path.join(state, "kv.json"), "{}") + + const events = createEventStream() + const replies: unknown[] = [] + const copied: string[] = [] + const transport = createFetch(undefined, events) + const fetch = Object.assign( + async (input: RequestInfo | URL, init?: RequestInit) => { + const request = input instanceof Request ? input : new Request(input, init) + if (new URL(request.url).pathname === "/api/session/ses_test/form/frm_test/reply") { + replies.push(await request.clone().json()) + return new Response(null, { status: 204 }) + } + return transport.fetch(request) + }, + { preconnect: globalThis.fetch.preconnect }, + ) + const config = createTuiResolvedConfig() + const form = { + id: "frm_test", + sessionID: "ses_test", + title: "Authorization required", + fields: [ + { + key: "authorization", + type: "external", + url: "https://example.com/authorize", + title: "Authorize access", + }, + ], + } satisfies FormInfo + const { FormPrompt } = await import("../../../src/routes/session/form") + + function Harness() { + const renderer = useRenderer() + const keymap = createDefaultOpenTuiKeymap(renderer) + const off = registerOpencodeKeymap(keymap, renderer, config) + onCleanup(off) + + return ( + + + + + + + Promise.resolve({}) }}> + + + + + + + + + + + ) + } + + const app = await testRender(() => , { width, height: 20, kittyKeyboard: true }) + await wait(() => app.captureCharFrame().includes("Authorization required")) + return { + app, + copied, + replies, + async cleanup() { + app.renderer.destroy() + await tmp[Symbol.asyncDispose]() + }, + } +} + +test("requires explicit acknowledgement after a successful browser launch", async () => { + const prompt = await mountForm() + try { + prompt.app.mockInput.pressKey("right") + await wait(() => prompt.app.captureCharFrame().includes("(acknowledgement required)")) + prompt.app.mockInput.pressEnter() + await wait(() => prompt.app.captureCharFrame().includes("External action must be acknowledged")) + expect(prompt.replies).toEqual([]) + expect(prompt.app.captureCharFrame()).toContain("External action must be acknowledged") + + prompt.app.mockInput.pressKey("left") + failOpen = true + prompt.app.mockInput.pressEnter() + await wait(() => prompt.app.captureCharFrame().includes("Could not open the browser")) + expect(prompt.app.captureCharFrame()).not.toContain("press enter to confirm") + + failOpen = false + prompt.app.mockInput.pressEnter() + await wait(() => prompt.app.captureCharFrame().includes("press enter to confirm")) + expect(opened).toEqual(["https://example.com/authorize", "https://example.com/authorize"]) + expect(prompt.replies).toEqual([]) + + prompt.app.mockInput.pressEnter() + await wait(() => prompt.app.captureCharFrame().includes("Acknowledged")) + expect(prompt.replies).toEqual([]) + + prompt.app.mockInput.pressEnter() + await wait(() => prompt.replies.length === 1) + expect(prompt.replies).toEqual([{ answer: { authorization: true } }]) + } finally { + await prompt.cleanup() + } +}) + +test("includes external acknowledgements in progress", async () => { + const prompt = await mountForm(32) + try { + expect(prompt.app.captureCharFrame()).toContain("0/1") + expect(prompt.replies).toEqual([]) + } finally { + await prompt.cleanup() + } +}) + +test("requires explicit acknowledgement after copying an external URL", async () => { + const prompt = await mountForm() + try { + prompt.app.mockInput.pressKey("c") + await wait(() => prompt.copied.length === 1 && prompt.app.captureCharFrame().includes("press enter to confirm")) + expect(prompt.copied).toEqual(["https://example.com/authorize"]) + expect(opened).toEqual([]) + expect(prompt.app.captureCharFrame()).toContain("press enter to confirm") + expect(prompt.replies).toEqual([]) + + prompt.app.mockInput.pressEnter() + await wait(() => prompt.app.captureCharFrame().includes("Acknowledged")) + expect(prompt.replies).toEqual([]) + + prompt.app.mockInput.pressEnter() + await wait(() => prompt.replies.length === 1) + expect(prompt.replies).toEqual([{ answer: { authorization: true } }]) + } finally { + await prompt.cleanup() + } +})