Skip to content

fix(mcp): recognize implicit form elicitation capability - #757

Closed
rudycelekli wants to merge 1 commit into
morluto:mainfrom
rudycelekli:fix/mcp-implicit-elicitation-20261006
Closed

rudycelekli wants to merge 1 commit into
morluto:mainfrom
rudycelekli:fix/mcp-implicit-elicitation-20261006

Conversation

@rudycelekli

Copy link
Copy Markdown
Contributor

Problem

binary_session reports elicitation_form: false when a request declares elicitation: {}. The MCP protocol defines that declaration as implicit form-mode support. This makes REA's reported client capabilities disagree with the negotiated request and the SDK's compatibility behavior.

Change

Recognize implicit form support only when an elicitation capability object is present. A missing capability still reports no support; explicit URL-only declarations remain URL-only. The projection continues to use request-scoped metadata.

Verification

  • Actual pinned SDK 2.3.1 Client and InMemoryTransport calling REA's registered binary_session tool: before the fix, the implicit-form case failed while explicit form, URL-only, and absent-capability controls passed.
  • After the fix: five capability cases plus the four existing server-identity tests passed (9 total).
  • npm run check passed all five tasks, with 124 existing lint warnings and no errors.
  • No tool schema, provider behavior, packaging, or Hopper/Ghidra support claim changes.

Protocol reference: MCP elicitation capabilities.

AI assistance: Codex investigated, implemented, and tested this change; an independent agent reviewed the source and protocol interpretation.

Signed-off-by: RudyCelekli <47457359+rudycelekli@users.noreply.github.com>
@rudycelekli

Copy link
Copy Markdown
Contributor Author

CI follow-up (Codex assisting on behalf of @rudycelekli): all executed source checks and test shards pass on signed head 257d1645e386428c18f50beef2e4a9937aad749b. The Windows native-control job was cancelled after approximately ten minutes, leaving the dependent package/Windows checks skipped. I could not retry that job, and retrying the run requires repository admin rights. Could a maintainer rerun the cancelled Windows job and its dependent checks? This run is incomplete; I am not treating it as fully passing.

@morluto

morluto commented Oct 6, 2026

Copy link
Copy Markdown
Owner

Reviewed: implicit-form elicitation truth table is exactly right ({} -> form-only, {form:{}} form, {url:{}} url-only, both -> both, absent/non-object -> none) per MCP form-compatibility. Real Client + InMemoryTransport boundary test (5 cases, pinned SDK 2.3.1) covers it, same client_features shape so no catalog change. APPROVE. Note: packaging lanes show CANCELLED/SKIPPED — worth confirming they pass on merge (change is server-only, no packaging impact expected).

@morluto

morluto commented Oct 6, 2026

Copy link
Copy Markdown
Owner

Review — APPROVE

Summary

binary_session's client_features now reports elicitation_form: true for a declared-but-modeless elicitation: {} capability (per the MCP elicitation spec's form-compatibility rule), while a missing capability still reports no support and { url: {} } stays URL-only.

Verification

Truth table in the new code is exactly right:

capability form url
{} (implicit) ✅ ❌
{ form: {} } ✅ ❌
{ url: {} } ❌ ✅
{ form: {}, url: {} } ✅ ✅
absent / non-object ❌ ❌
  • elicitation: null or a non-object fails isRecord → no support (correct — no declared capability).
  • Test is a real boundary test (pinned SDK 2.3.1 Client + InMemoryTransport against createServer, 5 capability cases), not a mock of the projection logic.
  • No schema change (same client_features shape, corrected values), so no catalog update needed.
  • Functional checks SUCCESS (static checks, build, all 4 test shards, coverage).

Note (non-blocking)

  • Build bundled Windows x64 native controls shows CANCELLED and the two Package/install + Windows capability and package lane jobs show SKIPPED. Worth re-running or confirming on merge, though the change is server-only with no packaging impact expected.

@rudycelekli

Copy link
Copy Markdown
Contributor Author

Thanks for the review. Hosted packaging is still incomplete: the Windows native-controls job was cancelled and the dependent lanes skipped. My rerun attempt was denied because GitHub requires repository admin rights. The SDK round-trip tests and required local checks passed, but I am not treating the cancelled/skipped lanes as passing; an upstream rerun is still needed.

@morluto

morluto commented Oct 7, 2026

Copy link
Copy Markdown
Owner

Superseded by #815, which integrated this change and has now merged (3af3811). Thanks @rudycelekli for the fix and the thorough verification — much appreciated!

@morluto morluto closed this Oct 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants