refactor(form): model links as fields (#36129)

This commit is contained in:
Aiden Cline 2026-07-10 00:22:01 -05:00 committed by GitHub
commit a6449cb45c
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
24 changed files with 2409 additions and 1430 deletions

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
@ -64,9 +67,7 @@ export class InvalidFormError extends Schema.TaggedErrorClass<InvalidFormError>(
message: Schema.String,
}) {}
export type CreateInput =
| (Omit<Form.FormInfo, "id"> & { readonly id?: ID })
| (Omit<Form.UrlInfo, "id"> & { readonly id?: ID })
export type CreateInput = Omit<Form.Info, "id"> & { readonly id?: ID }
export interface ReplyInput {
readonly id: ID
@ -74,7 +75,7 @@ export interface ReplyInput {
}
export interface ListInput {
readonly sessionID?: Form.FormInfo["sessionID"]
readonly sessionID?: Form.Info["sessionID"]
}
export interface Interface {
@ -125,20 +126,15 @@ export const layer = Layer.effect(
const id = input.id ?? ID.create()
const existing = yield* Cache.getSuccess(forms, id)
if (Option.isSome(existing)) return yield* new AlreadyExistsError({ id })
if (input.mode === "form") {
const invalid = validateFields(input.fields)
if (invalid) return yield* new InvalidFormError({ message: invalid })
}
const base = {
const invalid = validateFields(input.fields)
if (invalid) return yield* new InvalidFormError({ message: invalid })
const form: Info = {
id,
sessionID: input.sessionID,
title: input.title,
...(input.metadata === undefined ? {} : { metadata: input.metadata }),
fields: input.fields,
}
const form: Info =
input.mode === "form"
? { ...base, mode: "form", fields: input.fields }
: { ...base, mode: "url", url: input.url }
const entry: Entry = {
form,
state: { status: "pending" },
@ -228,16 +224,16 @@ export const locationLayer = layer
export const node = makeLocationNode({ service: Service, layer, deps: [EventV2.node] })
function validateAnswer(form: Info, answer: Answer) {
if (form.mode === "url") {
if (Object.keys(answer).length === 0) return
return "URL forms must be answered with an empty answer"
}
const fields = new Map(form.fields.map((field) => [field.key, field]))
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) {
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}`
@ -249,7 +245,9 @@ function validateAnswer(form: Info, answer: Answer) {
}
}
function isActive(field: Form.Field, answer: Answer) {
type InputField = Exclude<Form.Field, Form.ExternalField>
function isActive(field: InputField, answer: Answer) {
if (!field.when) return true
return field.when.every((when) => matches(when, answer[when.key]))
}
@ -267,9 +265,13 @@ 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>) {
const earlier = new Map<string, Form.Field>()
if (fields.length === 0) return "Form must have at least one field"
const earlier = new Map<string, InputField>()
const keys = new Set<string>()
for (const field of fields) {
if (earlier.has(field.key)) return `Duplicate form field key: ${field.key}`
if (keys.has(field.key)) return `Duplicate form field key: ${field.key}`
keys.add(field.key)
if (field.type === "external") continue
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}`
@ -280,7 +282,7 @@ function validateFields(fields: ReadonlyArray<Form.Field>) {
}
}
function validateWhen(when: Form.When, target: Form.Field) {
function validateWhen(when: Form.When, target: InputField) {
if (target.type === "boolean") {
if (typeof when.value !== "boolean") return "Form field condition value must be a boolean"
return
@ -297,7 +299,7 @@ function validateWhen(when: Form.When, target: Form.Field) {
}
}
function validateField(field: Form.Field, value: Form.Value): string | undefined {
function validateField(field: InputField, value: Form.Value): string | undefined {
if (field.type === "string") {
if (typeof value !== "string") return `Expected string for form field: ${field.key}`
if (field.required && value.length === 0) return `Missing required form field: ${field.key}`

View file

@ -123,6 +123,7 @@ type ServerEntry = {
// MCP elicitations are Location-scoped, not Session-scoped: the server cannot attribute them to a
// persisted session row, so their forms are owned by this opaque sentinel session identifier.
const GLOBAL_ELICITATION_SESSION_ID = "global"
const URL_ELICITATION_FIELD_KEY = "elicitation"
export interface Interface {
readonly servers: () => Effect.Effect<ServerInfo[]>
@ -311,8 +312,7 @@ export const layer = Layer.effect(
elicitationID: input.params.elicitationId,
message: input.params.message,
},
mode: "url",
url: input.params.url,
fields: [{ key: URL_ELICITATION_FIELD_KEY, type: "external", url: input.params.url }],
})
.pipe(
Effect.raceFirst(waitForAbort(input.signal)),
@ -325,15 +325,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 },
mode: "form",
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)),
@ -355,7 +356,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: { [URL_ELICITATION_FIELD_KEY]: true } }).pipe(Effect.ignore)
}),
} satisfies MCPClient.ElicitationHandler

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,21 +86,10 @@ export const Plugin = {
kind: "question",
tool: { messageID: context.assistantMessageID, callID: context.toolCallID },
},
mode: "form",
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),
),
@ -122,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

@ -15,7 +15,6 @@ const input = {
id: formID,
sessionID: SessionSchema.ID.make("ses_test"),
title: "Test form",
mode: "form",
fields: [{ key: "name", type: "string", required: true }],
} satisfies Form.CreateInput
@ -47,7 +46,6 @@ describe("Form", () => {
const created = yield* service.create({
sessionID: "global",
title: "MCP input",
mode: "form",
fields: [{ key: "name", type: "string", required: true }],
})
expect(created.sessionID).toBe("global")
@ -59,6 +57,14 @@ describe("Form", () => {
yield* service.reply({ id: created.id, answer: { name: "Ava" } })
expect(yield* service.state(created.id)).toEqual({ status: "answered", answer: { name: "Ava" } })
const externalOnly = yield* service.create({
sessionID: "global",
title: "External setup",
fields: [{ key: "setup", type: "external", url: "https://example.com/setup" }],
})
yield* service.reply({ id: externalOnly.id, answer: { setup: true } })
expect(yield* service.state(externalOnly.id)).toEqual({ status: "answered", answer: { setup: true } })
}),
)
@ -68,15 +74,18 @@ describe("Form", () => {
const created = yield* service.create({
sessionID: "global",
title: "Conditional form",
mode: "form",
fields: [
{ key: "confirm", type: "boolean", required: true },
{ key: "reason", type: "string", required: true, when: [{ key: "confirm", op: "eq", value: false }] },
],
})
const inactive = yield* service.reply({ id: created.id, answer: { confirm: true, reason: "x" } }).pipe(Effect.flip)
expect(inactive).toEqual(new Form.InvalidAnswerError({ id: created.id, message: "Form field is not active: reason" }))
const inactive = yield* service
.reply({ id: created.id, answer: { confirm: true, reason: "x" } })
.pipe(Effect.flip)
expect(inactive).toEqual(
new Form.InvalidAnswerError({ id: created.id, message: "Form field is not active: reason" }),
)
const missing = yield* service.reply({ id: created.id, answer: { confirm: false } }).pipe(Effect.flip)
expect(missing).toEqual(
@ -101,7 +110,6 @@ describe("Form", () => {
const created = yield* service.create({
sessionID: "global",
title: "Multiselect form",
mode: "form",
fields: [
{ key: "langs", type: "multiselect", options },
{ key: "goVersion", type: "string", required: true, when: [{ key: "langs", op: "eq", value: "go" }] },
@ -124,7 +132,6 @@ describe("Form", () => {
const created = yield* service.create({
sessionID: "global",
title: "Dependent form",
mode: "form",
fields: [
{ key: "a", type: "boolean" },
{ key: "b", type: "boolean" },
@ -142,7 +149,9 @@ describe("Form", () => {
})
const missingX = yield* service.reply({ id: created.id, answer: { a: true, b: true, z: "ok" } }).pipe(Effect.flip)
expect(missingX).toEqual(new Form.InvalidAnswerError({ id: created.id, message: "Missing required form field: x" }))
expect(missingX).toEqual(
new Form.InvalidAnswerError({ id: created.id, message: "Missing required form field: x" }),
)
const inactiveX = yield* service
.reply({ id: created.id, answer: { a: true, b: false, x: "nope", z: "ok" } })
@ -150,7 +159,9 @@ describe("Form", () => {
expect(inactiveX).toEqual(new Form.InvalidAnswerError({ id: created.id, message: "Form field is not active: x" }))
const missingZ = yield* service.reply({ id: created.id, answer: { a: true, b: false } }).pipe(Effect.flip)
expect(missingZ).toEqual(new Form.InvalidAnswerError({ id: created.id, message: "Missing required form field: z" }))
expect(missingZ).toEqual(
new Form.InvalidAnswerError({ id: created.id, message: "Missing required form field: z" }),
)
yield* service.reply({ id: created.id, answer: { a: true, b: false, z: "ok" } })
expect(yield* service.state(created.id)).toEqual({ status: "answered", answer: { a: true, b: false, z: "ok" } })
@ -167,7 +178,6 @@ describe("Form", () => {
const created = yield* service.create({
sessionID: "global",
title: "Selection form",
mode: "form",
fields: [
{ key: "langs", type: "multiselect", options },
{ key: "note", type: "string", required: true, when: [{ key: "langs", op: "neq", value: "go" }] },
@ -186,7 +196,9 @@ describe("Form", () => {
)
const inactive = yield* service.reply({ id: created.id, answer: { langs: ["go"], note: "x" } }).pipe(Effect.flip)
expect(inactive).toEqual(new Form.InvalidAnswerError({ id: created.id, message: "Form field is not active: note" }))
expect(inactive).toEqual(
new Form.InvalidAnswerError({ id: created.id, message: "Form field is not active: note" }),
)
yield* service.reply({ id: created.id, answer: { langs: ["go"] } })
expect(yield* service.state(created.id)).toEqual({ status: "answered", answer: { langs: ["go"] } })
@ -199,7 +211,6 @@ describe("Form", () => {
const created = yield* service.create({
sessionID: "global",
title: "Cascading form",
mode: "form",
fields: [
{ key: "a", type: "boolean" },
{ key: "b", type: "string", when: [{ key: "a", op: "eq", value: true }] },
@ -220,14 +231,14 @@ describe("Form", () => {
it.effect("rejects invalid when definitions at creation", () =>
Effect.gen(function* () {
const service = yield* Form.Service
const flipCreate = (fields: ReadonlyArray<Form.Field>) =>
service.create({ sessionID: "global", title: "Invalid form", mode: "form", fields }).pipe(Effect.flip)
const flipCreate = (fields: Form.CreateInput["fields"]) =>
service.create({ sessionID: "global", title: "Invalid form", fields }).pipe(Effect.flip)
expect(
yield* flipCreate([
{ key: "b", type: "string", when: [{ key: "missing", op: "eq", value: "x" }] },
]),
).toEqual(new Form.InvalidFormError({ message: "Form field condition must reference an earlier field: b -> missing" }))
yield* flipCreate([{ key: "b", type: "string", when: [{ key: "missing", op: "eq", value: "x" }] }]),
).toEqual(
new Form.InvalidFormError({ message: "Form field condition must reference an earlier field: b -> missing" }),
)
expect(
yield* flipCreate([
@ -236,14 +247,19 @@ 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" },
{ key: "b", type: "string", when: [{ key: "a", op: "eq", value: "yes" }] },
]),
).toEqual(
new Form.InvalidFormError({ message: "Form field condition value must be a boolean: b -> a" }),
)
).toEqual(new Form.InvalidFormError({ message: "Form field condition value must be a boolean: b -> a" }))
expect(
yield* flipCreate([
@ -258,6 +274,40 @@ describe("Form", () => {
}),
)
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: [
{ key: "authorization", type: "external", url: "https://example.com/setup", title: "Open setup" },
{ key: "name", type: "string", required: true },
],
})
const invalidAnswers: ReadonlyArray<Form.Answer> = [
{ 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: { authorization: true, name: "Ava" } })
expect(yield* service.state(created.id)).toEqual({
status: "answered",
answer: { authorization: true, name: "Ava" },
})
}),
)
it.effect("cleans up created forms when event publication fails", () =>
Effect.gen(function* () {
const service = yield* Form.Service

View file

@ -28,7 +28,7 @@ import { SessionV2 } from "@opencode-ai/core/session"
import { McpTool } from "@opencode-ai/core/tool/mcp"
import { ToolRegistry } from "@opencode-ai/core/tool/registry"
import { ToolOutputStore } from "@opencode-ai/core/tool-output-store"
import { Deferred, Effect, Exit, Fiber, Layer, Stream } from "effect"
import { Deferred, Effect, Exit, Fiber, Layer, Schema, Stream } from "effect"
import { testEffect } from "./lib/effect"
import { location } from "./fixture/location"
import { settleTool, toolDefinitions, toolIdentity, waitForTool } from "./lib/tool"
@ -47,7 +47,9 @@ type ResourceTemplatePage = {
nextCursor?: string
}
function resourceServer(input: { resources?: boolean; listChanged?: boolean } = {}) {
function resourceServer(
input: { resources?: boolean; listChanged?: boolean; emptyElicitation?: boolean; urlElicitation?: boolean } = {},
) {
return Effect.acquireRelease(
Effect.promise(async () => {
const state = {
@ -71,7 +73,42 @@ 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: {} } }]
: input.urlElicitation
? [{ name: "url-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.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
@ -98,6 +135,7 @@ function resourceServer(input: { resources?: boolean; listChanged?: boolean } =
state,
url: http.url.toString(),
sendResourceListChanged: () => protocol.sendResourceListChanged(),
completeElicitation: () => protocol.createElicitationCompletionNotifier("elicitation-test")(),
close: async () => {
await protocol.close().catch(() => {})
await http.stop(true)
@ -108,10 +146,11 @@ function resourceServer(input: { resources?: boolean; listChanged?: boolean } =
)
}
function resourceMcpLayer(url: string) {
function resourceMcpLayer(url: string, onFormCreated?: (form: Form.Info) => Effect.Effect<void>) {
const directory = AbsolutePath.make(import.meta.dir)
const unusedIntegration = () => Effect.die("unused integration service")
return MCP.layer.pipe(
Layer.provideMerge(Form.layer),
Layer.provide(
Layer.mergeAll(
Layer.succeed(
@ -133,14 +172,16 @@ function resourceMcpLayer(url: string) {
Layer.succeed(Location.Service, Location.Service.of(location({ directory }))),
Layer.mock(EventV2.Service, {
subscribe: () => Stream.never,
publish: (definition, data) =>
Effect.succeed({
publish: (definition, data) => {
const event = {
id: EventV2.ID.create(),
type: definition.type,
data,
} as EventV2.Payload<typeof definition>),
} as EventV2.Payload<typeof definition>
if (event.type !== Form.Event.Created.type || !onFormCreated) return Effect.succeed(event)
return onFormCreated(Schema.decodeUnknownSync(Form.Event.Created.data)(data).form).pipe(Effect.as(event))
},
}),
Layer.mock(Form.Service, {}),
Layer.mock(Integration.Service, {
connection: {
active: unusedIntegration,
@ -490,6 +531,53 @@ 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
const forms = yield* Form.Service
const result = yield* service.callTool({ server: "resources", name: "empty-elicitation" })
expect(yield* forms.list()).toEqual([])
return result
}).pipe(Effect.provide(resourceMcpLayer(server.url)))
expect(result.structured).toEqual({ action: "accept", content: {} })
}),
),
)
})
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 created = yield* Deferred.make<Form.Info>()
const result = yield* Effect.gen(function* () {
const service = yield* MCP.Service
const forms = yield* Form.Service
const call = yield* service.callTool({ server: "resources", name: "url-elicitation" }).pipe(Effect.forkScoped)
const form = yield* Deferred.await(created)
expect(form.fields).toEqual([{ key: "elicitation", type: "external", url: "https://example.com/authorize" }])
yield* Effect.promise(server.completeElicitation)
const result = yield* Fiber.join(call)
expect(yield* forms.state(form.id)).toEqual({ status: "answered", answer: { elicitation: true } })
return result
}).pipe(
Effect.provide(resourceMcpLayer(server.url, (form) => Deferred.succeed(created, form).pipe(Effect.asVoid))),
)
expect(result.structured).toEqual({ action: "accept" })
}),
),
)
})
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" },
@ -158,7 +167,6 @@ describe("QuestionTool", () => {
sessionID,
title: "Questions",
metadata: { kind: "question", tool: { messageID: toolIdentity.assistantMessageID, callID: "call-question" } },
mode: "form",
fields: [
{
key: "q0",
@ -199,14 +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" } },
mode: "form",
fields: [],
fields: [
{
key: "q0",
title: "Continue",
description: "Continue?",
options: [{ value: "Yes", label: "Yes", description: "Continue" }],
custom: true,
type: "string",
},
],
})
}),
)
@ -220,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)