Repository navigation
fix(plugin): expose session form replies to server plugins - #59655
Hamza-Abili wants to merge 1 commit into
Conversation
![opencode-agent[bot]](https://avatars.githubusercontent.com/in/1549082?s=60&v=4){kind=small}
There was a problem hiding this comment.
Looks sound. I copied the new test file onto the base, where the plugin has no ctx.session.form and all three tests fail with TypeError; on this branch they pass. The rest of packages/core/test/plugin and packages/plugin also pass, and typecheck:tsc is clean in both packages. The bridge uses the location's Form.Service, the same instance Form.ask uses, and get/reply/cancel check ownership the same way the HTTP handlers do.
Public API (plugin API): adds ctx.session.form.list, .get, .reply and .cancel to the Promise and Effect SessionDomain, using the existing session.form.* endpoint schemas. Purely additive and backwards compatible; the HTTP API and SDK are unchanged.
One small inline point: form.list doesn't report an unknown session the way the endpoint does.
There was a problem hiding this comment.
The new commit addresses the earlier point: form.list, get, reply and cancel now check that the session exists first. An unknown session fails with Session.NotFoundError, like the HTTP endpoint, and "global" is let through the same way the server's form middleware does. The updated test file passes, and typecheck:tsc is clean in packages/core and packages/plugin. The public API is the same as in my last review: it adds ctx.session.form.{list,get,reply,cancel} to the plugin session domain, which is backwards compatible. No new concerns.
|
@thdxr could a maintainer approve the pending workflow runs for this PR? The branch is now a single squashed commit (5ca2d71) on the latest |
ddcb7af to
f9215eb
Compare
|
Hi @rekram1-node, could you take a look when you have a moment? The branch is rebased onto the latest |
5ca2d71 to
8eba087
Compare
8eba087 to
bd6ec8e
Compare
|
Rebased onto current
Verified on the rebased head:
On demand: @kitlangton you added the analogous |
SessionDomain omitted form, so server plugins could receive form events but had no supported way to answer or cancel a question. This adds a four-method form surface (list, get, reply, cancel) to the Promise and Effect session domains, bridges it through Form.Service in the plugin host with the server's session-ownership check, wires the Promise adapter, and adds a regression test that settles a Form.ask request through the plugin surface. Create is intentionally left out of this change: anomalyco#51108 is about answering forms, and plugin-created forms need the client metadata.kind contract. The form methods resolve the session through Session.Service before touching Form.Service, matching the HTTP form route's session lookup (including the "global" MCP elicitation escape hatch), so callers can distinguish an unknown session from an empty form list. Docs explain that form.list returns every pending form kind, that question answers use the form's field keys (q0, q1, ...), and that multiselect answers are string arrays. Port of anomalyco#51142 to current dev; original implementation by @holny. Fixes anomalyco#51108 Co-authored-by: holny <[email protected]>
bd6ec8e to
5d24a10
Compare
Issue for this PR
Closes #51108
Type of change
What does this PR do?
Server plugins can receive
form.created/form.replied/form.cancelledevents but cannot answer a form:SessionDomainomittedform, although the HTTP API and client already exposesession.form.{list,get,reply,cancel}.This adds a four-method
formsurface to the Promise and Effect plugin session domains: list pending forms, inspect their state, reply, and cancel with an optional message. The methods are bridged throughForm.Servicein the plugin host with the same session-ownership check as the server handlers forget/reply/cancel, and the Promise adapter is wired to the existing endpoints.createis intentionally not exposed here. #51108 is about answering forms, and plugin-created forms need the clientmetadata.kindcontract before they can be rendered; that is a separate change.This is a port of #51142 to current
dev(the original branch predates the docs split and theinstructionsdomain). Thanks to @holny for the original implementation. The PR is opened here so the fix can land while the original author is at the repository's open-PR cap.Docs:
server.mdxandeffect.mdxexplain thatform.listreturns every pending form kind, so callers should filtermetadata.kind === "question"; question answers use the form's field keys (q0,q1, …), and multiselect answers are string arrays.How did you verify your code works?
cd packages/core && bun test test/plugin/session-form.test.ts— 3 pass (30/30 with--rerun-each 10)cd packages/core && bun typecheck— cleancd packages/plugin && bun typecheck— cleancd packages/plugin && bun run typecheck:tsc— cleanThe tests cover Effect and Promise plugins: list/get/reply/cancel, ownership mismatches on
get/reply/cancel, missing-session errors for all four methods, the cancel message, and aForm.askrequest settled through the plugin surface to prove both use the sameForm.Serviceinstance.Visual
Not applicable: no UI change.
Screenshots / recordings
Not applicable.
Checklist
Added by the opencode bot (AI-generated).
Public API: adds ctx.session.form.{list,get,reply,cancel} to the server plugin session domain (backwards compatible)