fix(form): reject empty forms

This commit is contained in:
Aiden Cline 2026-07-09 18:07:04 -05:00
parent 5c9ea59197
commit 68c9d75835
11 changed files with 1749 additions and 821 deletions

File diff suppressed because it is too large Load diff

View file

@ -16,6 +16,9 @@ export type Info = typeof Info.Type
export const Field = Form.Field
export type Field = Form.Field
export const Fields = Form.Fields
export type Fields = Form.Fields
export const When = Form.When
export type When = Form.When
@ -259,6 +262,7 @@ function matches(when: Form.When, value: Form.Value | undefined) {
// are closed. Rejecting these at creation surfaces authoring mistakes to the caller instead of
// silently never matching.
function validateFields(fields: ReadonlyArray<Form.Field>) {
if (fields.length === 0) return "Form must have at least one field"
const earlier = new Map<string, InputField>()
for (const field of fields) {
if (field.type === "link") continue

View file

@ -324,14 +324,16 @@ export const layer = Layer.effect(
)
}
const params = input.params
const [field, ...fields] = Object.entries(params.requestedSchema.properties).map(([key, property]) =>
toElicitationField(key, property, params.requestedSchema.required?.includes(key) === true),
)
if (!field) return { action: "accept", content: {} }
return yield* forms
.ask({
sessionID: GLOBAL_ELICITATION_SESSION_ID,
title: `${input.server} is requesting input`,
metadata: { kind: "mcp-elicitation", server: input.server, message: params.message },
fields: Object.entries(params.requestedSchema.properties).map(([key, property]) =>
toElicitationField(key, property, params.requestedSchema.required?.includes(key) === true),
),
fields: [field, ...fields],
})
.pipe(
Effect.raceFirst(waitForAbort(input.signal)),

View file

@ -22,7 +22,7 @@ Usage notes:
- If you recommend a specific option, make that the first option in the list and add "(Recommended)" at the end of the label`
export const Input = Schema.Struct({
questions: Schema.Array(QuestionV2.Prompt).annotate({ description: "Questions to ask" }),
questions: Schema.NonEmptyArray(QuestionV2.Prompt).annotate({ description: "Questions to ask" }),
})
export const Output = Schema.Struct({
@ -86,20 +86,10 @@ export const Plugin = {
kind: "question",
tool: { messageID: context.assistantMessageID, callID: context.toolCallID },
},
fields: input.questions.map(
(question, index): Form.Field => ({
key: `q${index}`,
title: question.header,
description: question.question,
type: question.multiple === true ? "multiselect" : "string",
options: question.options.map((option) => ({
value: option.label,
label: option.label,
description: option.description,
})),
custom: true,
}),
),
fields: [
toField(input.questions[0], 0),
...input.questions.slice(1).map((question, index) => toField(question, index + 1)),
],
})
.pipe(Effect.orDie),
),
@ -121,3 +111,18 @@ export const Plugin = {
.pipe(Effect.orDie)
}),
}
function toField(question: QuestionV2.Prompt, index: number): Form.Field {
return {
key: `q${index}`,
title: question.header,
description: question.question,
type: question.multiple === true ? "multiselect" : "string",
options: question.options.map((option) => ({
value: option.label,
label: option.label,
description: option.description,
})),
custom: true,
}
}

View file

@ -231,7 +231,7 @@ describe("Form", () => {
it.effect("rejects invalid when definitions at creation", () =>
Effect.gen(function* () {
const service = yield* Form.Service
const flipCreate = (fields: ReadonlyArray<Form.Field>) =>
const flipCreate = (fields: Form.CreateInput["fields"]) =>
service.create({ sessionID: "global", title: "Invalid form", fields }).pipe(Effect.flip)
expect(

View file

@ -47,7 +47,7 @@ type ResourceTemplatePage = {
nextCursor?: string
}
function resourceServer(input: { resources?: boolean; listChanged?: boolean } = {}) {
function resourceServer(input: { resources?: boolean; listChanged?: boolean; emptyElicitation?: boolean } = {}) {
return Effect.acquireRelease(
Effect.promise(async () => {
const state = {
@ -71,7 +71,26 @@ function resourceServer(input: { resources?: boolean; listChanged?: boolean } =
},
},
)
protocol.setRequestHandler(ListToolsRequestSchema, () => Promise.resolve({ tools: [] }))
protocol.setRequestHandler(ListToolsRequestSchema, () =>
Promise.resolve({
tools: input.emptyElicitation
? [{ name: "empty-elicitation", inputSchema: { type: "object" as const, properties: {} } }]
: [],
}),
)
if (input.emptyElicitation) {
protocol.setRequestHandler(CallToolRequestSchema, async () => {
const result = await protocol.elicitInput({
mode: "form",
message: "Confirm",
requestedSchema: { type: "object", properties: {} },
})
return {
content: [{ type: "text", text: JSON.stringify(result) }],
structuredContent: result,
}
})
}
if (input.resources !== false) {
protocol.setRequestHandler(ListResourcesRequestSchema, (request) => {
state.resourceLists += 1
@ -140,7 +159,9 @@ function resourceMcpLayer(url: string) {
data,
} as EventV2.Payload<typeof definition>),
}),
Layer.mock(Form.Service, {}),
Layer.mock(Form.Service, {
ask: () => Effect.die("Empty MCP elicitation must not create a form"),
}),
Layer.mock(Integration.Service, {
connection: {
active: unusedIntegration,
@ -490,6 +511,22 @@ test("skips MCP resource requests when the capability is absent", async () => {
)
})
test("accepts empty MCP elicitations without creating forms", async () => {
await Effect.runPromise(
Effect.scoped(
Effect.gen(function* () {
const server = yield* resourceServer({ resources: false, emptyElicitation: true })
const result = yield* Effect.gen(function* () {
const service = yield* MCP.Service
return yield* service.callTool({ server: "resources", name: "empty-elicitation" })
}).pipe(Effect.provide(resourceMcpLayer(server.url)))
expect(result.structured).toEqual({ action: "accept", content: {} })
}),
),
)
})
test("loads and reads MCP resources", async () => {
await Effect.runPromise(
Effect.scoped(

View file

@ -18,6 +18,15 @@ let captured: Form.CreateInput | undefined
let reject = false
let deny = false
const capturedInput = () => captured
const questionInput = {
questions: [
{
question: "Continue?",
header: "Continue",
options: [{ label: "Yes", description: "Continue" }],
},
],
}
const permission = Layer.succeed(
PermissionV2.Service,
PermissionV2.Service.of({
@ -90,7 +99,7 @@ describe("QuestionTool", () => {
yield* settleTool(registry, {
sessionID,
...toolIdentity,
call: { type: "tool-call", id: "call-question-denied", name: "question", input: { questions: [] } },
call: { type: "tool-call", id: "call-question-denied", name: "question", input: questionInput },
}),
).toEqual({
result: { type: "error", value: "Permission denied: question" },
@ -198,13 +207,22 @@ describe("QuestionTool", () => {
yield* executeTool(registryService, {
sessionID,
...toolIdentity,
call: { type: "tool-call", id: "call-question", name: "question", input: { questions: [] } },
call: { type: "tool-call", id: "call-question", name: "question", input: questionInput },
})
expect(capturedInput()).toEqual({
sessionID,
title: "Questions",
metadata: { kind: "question", tool: { messageID: toolIdentity.assistantMessageID, callID: "call-question" } },
fields: [],
fields: [
{
key: "q0",
title: "Continue",
description: "Continue?",
options: [{ value: "Yes", label: "Yes", description: "Continue" }],
custom: true,
type: "string",
},
],
})
}),
)
@ -218,7 +236,7 @@ describe("QuestionTool", () => {
const fiber = yield* executeTool(registryService, {
sessionID,
...toolIdentity,
call: { type: "tool-call", id: "call-question", name: "question", input: { questions: [] } },
call: { type: "tool-call", id: "call-question", name: "question", input: questionInput },
}).pipe(Effect.forkScoped)
const exit = yield* Fiber.await(fiber)

View file

@ -112,6 +112,9 @@ export const Field = Schema.Union([
]).pipe(Schema.toTaggedUnion("type"), Schema.annotate({ identifier: "Form.Field" }))
export type Field = StringField | NumberField | IntegerField | BooleanField | MultiselectField | LinkField
export const Fields = Schema.NonEmptyArray(Field).annotate({ identifier: "Form.Fields" })
export type Fields = typeof Fields.Type
const InfoBase = {
id: ID,
// This should be typed as SessionID. It is a plain string only because MCP elicitation
@ -125,7 +128,7 @@ const InfoBase = {
export const Info = Schema.Struct({
...InfoBase,
fields: Schema.Array(Field),
fields: Fields,
}).annotate({ identifier: "Form.Info" })
export interface Info extends Schema.Schema.Type<typeof Info> {}

View file

@ -48,6 +48,25 @@ describe("contract hygiene", () => {
).toEqual({ text: "completed" })
})
test("forms require at least one field", () => {
expect(() =>
Schema.decodeUnknownSync(Form.Info)({
id: Form.ID.create(),
sessionID: "global",
title: "Empty form",
fields: [],
}),
).toThrow()
expect(
Schema.decodeUnknownSync(Form.Info)({
id: Form.ID.create(),
sessionID: "global",
title: "Link form",
fields: [{ type: "link", url: "https://example.com" }],
}).fields,
).toHaveLength(1)
})
test("model defaults and provider overlays preserve public invariants", () => {
const id = Model.ID.make("model")
expect(Model.Info.empty(Provider.ID.make("provider"), id)).toMatchObject({ modelID: id, variants: [] })
@ -71,6 +90,7 @@ describe("contract hygiene", () => {
Agent.Color,
FileSystem.Submatch,
Form.Field,
Form.Fields,
Form.Info,
Form.LinkField,
Mcp.Resource,

View file

@ -78,7 +78,7 @@ function form(id: string, sessionID = "session"): Extract<V2Event, { type: "form
id,
sessionID,
title: "Input requested",
fields: [],
fields: [{ type: "link", url: "https://example.com" }],
}
}

View file

@ -12,6 +12,8 @@ import { createSessionRows, type SessionRow } from "../../../src/routes/session/
import { createApi, createClient, createEventStream, createFetch, directory, json } from "../../fixture/tui-sdk"
import { TestTuiContexts } from "../../fixture/tui-environment"
const formFields = [{ type: "link" as const, url: "https://example.com" }]
async function wait(fn: () => boolean, timeout = 2000) {
const start = Date.now()
while (!fn()) {
@ -1510,7 +1512,7 @@ test("adds, dismisses, and refreshes form requests", async () => {
const calls = createFetch((url) => {
if (url.pathname !== "/api/session/ses_1/form") return
return json({
data: [{ id: "frm_remote", sessionID: "ses_1", title: "Input requested", fields: [] }],
data: [{ id: "frm_remote", sessionID: "ses_1", title: "Input requested", fields: formFields }],
})
}, events)
let data!: ReturnType<typeof useData>
@ -1540,13 +1542,13 @@ test("adds, dismisses, and refreshes form requests", async () => {
id: "evt_form_created_1",
created: 0,
type: "form.created",
data: { form: { id: "frm_1", sessionID: "ses_1", title: "Input requested", fields: [] } },
data: { form: { id: "frm_1", sessionID: "ses_1", title: "Input requested", fields: formFields } },
})
emitEvent(events, {
id: "evt_form_created_duplicate",
created: 1,
type: "form.created",
data: { form: { id: "frm_1", sessionID: "ses_1", title: "Input requested", fields: [] } },
data: { form: { id: "frm_1", sessionID: "ses_1", title: "Input requested", fields: formFields } },
})
await wait(() => data.session.form.list("ses_1")?.length === 1)
@ -1562,7 +1564,7 @@ test("adds, dismisses, and refreshes form requests", async () => {
id: "evt_form_created_2",
created: 3,
type: "form.created",
data: { form: { id: "frm_2", sessionID: "ses_1", title: "Input requested", fields: [] } },
data: { form: { id: "frm_2", sessionID: "ses_1", title: "Input requested", fields: formFields } },
})
emitEvent(events, {
id: "evt_form_cancelled_2",
@ -1612,7 +1614,7 @@ test("tracks global forms by location", async () => {
location: other,
type: "form.created",
data: {
form: { id: "frm_other", sessionID: "global", title: "Input requested", fields: [] },
form: { id: "frm_other", sessionID: "global", title: "Input requested", fields: formFields },
},
})
@ -1625,7 +1627,7 @@ test("tracks global forms by location", async () => {
location: { directory },
type: "form.created",
data: {
form: { id: "frm_default", sessionID: "global", title: "Input requested", fields: [] },
form: { id: "frm_default", sessionID: "global", title: "Input requested", fields: formFields },
},
})
await wait(() => data.session.form.list("global", { directory })?.length === 1)
@ -1664,7 +1666,7 @@ test("refreshes global forms for the requested location", async () => {
id: requestedDirectory === other.directory ? "frm_other" : "frm_default",
sessionID: "global",
title: "Input requested",
fields: [],
fields: formFields,
},
],
})
@ -1742,7 +1744,7 @@ test("refreshes global forms once per loaded location after reconnect", async ()
id: `frm_${requestedDirectory === other.directory ? "other" : "default"}_${count}`,
sessionID: "global",
title: "Input requested",
fields: [],
fields: formFields,
},
],
})
@ -1801,7 +1803,7 @@ test("refreshes global forms once per loaded location after reconnect", async ()
test("reconciles all pending form requests when the event stream reconnects", async () => {
const events = createEventStream()
let requests = [
{ id: "frm_old", sessionID: "ses_old", title: "Input requested", fields: [] },
{ id: "frm_old", sessionID: "ses_old", title: "Input requested", fields: formFields },
{
id: "frm_keep",
sessionID: "ses_keep",
@ -1838,7 +1840,7 @@ test("reconciles all pending form requests when the event stream reconnects", as
await wait(() => data.session.form.list("ses_old")?.[0]?.id === "frm_old")
expect(data.session.form.list("ses_keep")?.[0]?.id).toBe("frm_keep")
requests = [{ id: "frm_new", sessionID: "ses_new", title: "Input requested", fields: [] }]
requests = [{ id: "frm_new", sessionID: "ses_new", title: "Input requested", fields: formFields }]
events.disconnect()
await wait(() => calls === 2 && data.session.form.list("ses_new")?.[0]?.id === "frm_new")