Domain-type the ElicitationRequester seam

[tgrunnagle]

Domain-type the ElicitationRequester seam

Closes #5436

Summary

The composer's elicitation seam (SDKElicitationRequester) leaked mcp-go's mcp.ElicitationRequest/mcp.ElicitationResult types across what is about to become the VMCP domain boundary. This is bounded rewrite R4: before the core constructor (New(cfg) -> VMCP, #5437) is introduced, that SDK coupling must be confined to the adapter so it does not cross the new boundary (vMCP anti-pattern #5). This PR replaces the SDK-typed interface with a domain-typed vmcp.ElicitationRequester plus ElicitationRequest/ElicitationResult value types that carry no mcp-go types, and moves all mcp-go translation into the sole adapter in pkg/vmcp/server.

This is a one-to-one field mirror of the mcp-go shapes the composer already uses — not a redesign. Request/response semantics, security validation, defaults, and error transforms are unchanged.

Changes Made

pkg/vmcp (new domain seam)

• Added pkg/vmcp/elicitation.go defining the domain ElicitationRequester interface and the ElicitationRequest/ElicitationResult value types, with no mcp-go imports. URL-mode fields (ElicitationID/URL/Mode) are deliberately omitted because the composer never constructs them.
• Added the go:generate directive and regenerated mock at pkg/vmcp/mocks/mock_elicitation_requester.go.

pkg/vmcp/composer

• Retyped DefaultElicitationHandler and NewDefaultElicitationHandler from SDKElicitationRequester to the domain vmcp.ElicitationRequester; deleted the old SDK-typed interface.
• RequestElicitation now builds a domain ElicitationRequest and reads a domain ElicitationResult. All security validation (timeout cap, schema size/depth, content size) and the Content any -> map[string]any assertion are unchanged.
• Removed the github.com/mark3labs/mcp-go/mcp import from the elicitation path; package composer no longer imports mcp-go for elicitation (verified — no remaining mcp-go references in the package).
• Removed the obsolete composer mock (mocks/mock_sdk_elicitation_requester.go).

pkg/vmcp/server

• sdkElicitationAdapter now satisfies vmcp.ElicitationRequester and is the sole point translating domain types to/from mcp-go: domain request -> mcp.ElicitationParams, and mcp.ElicitationResult -> domain result (string(Action), Content pass-through).
• NewSDKElicitationAdapter return type changed from composer.SDKElicitationRequester to vmcp.ElicitationRequester.

Implementation Details

• The adapter now depends on a minimal internal mcpElicitationRequester seam (just RequestElicitation) instead of *server.MCPServer directly, so the domain ⇄ mcp-go translation can be unit-tested against a fake SDK without a live session. *server.MCPServer satisfies it in production.
• Meta is only attached when the caller actually sets it. Because mcp.NewMetaFromMap mutates its argument (it deletes progressToken), the adapter clones the map first (maps.Clone) to avoid mutating caller-provided data, per the copy-before-mutate convention.
• ElicitationResult.Content stays typed as any so the composer's existing map[string]any assertion and validation behave byte-for-byte as before.

Testing

• Adapter translation tests: domain request maps to the equivalent mcp.ElicitationParams; mcp.ElicitationResult maps back with Action string-converted and Content preserved. Tests exercise the new fake-SDK seam.
• Composer handler and integration tests updated to drive elicitation through the domain requester (via the regenerated mock) and assert the unchanged ElicitationResponse for accept/decline/cancel/timeout.
• Security validation parity confirmed through the domain seam: schema size, schema depth, response content size limits, and the timeout cap.
• task test passes; task lint-fix clean; mocks regenerated via task gen.

Additional Notes

• No public API change: server.New's signature and the SDK-backed elicitation flow embedders use via (*Server).MCPServer() are unchanged.
• Unblocks P1.4 New(cfg) -> VMCP core constructor #5437 (New(cfg) -> VMCP core constructor), which depends on the domain ElicitationRequester.
• Out of scope (unchanged): URL-mode elicitation, security validation logic/defaults/error types, and the composer.ElicitationProtocolHandler/ElicitationResponse/ElicitationConfig public shapes.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 68.86%. Comparing base (255686a) to head (d988921).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #5456   +/-   ##
=======================================
  Coverage   68.85%   68.86%           
=======================================
  Files         634      634           
  Lines       64439    64471   +32     
=======================================
+ Hits        44370    44398   +28     
- Misses      16789    16794    +5     
+ Partials     3280     3279    -1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[tgrunnagle]

Automated multi-agent review

Recommendation: COMMENT — clean, well-scoped R4 bounded rewrite. The production change is faithful and low-risk; the only actionable items are two issue-mandated test-strength gaps (non-blocking).

Summary

Replaces the mcp-go–typed composer.SDKElicitationRequester with a domain-typed vmcp.ElicitationRequester + value types carrying no SDK types, and confines all mcp-go translation to the sole adapter in pkg/vmcp/server. Verified against the field-mapping table in #5436 (mcp-go v0.54.0): mapping is one-to-one, URL-mode fields correctly omitted, error returned unwrapped so errors.Is(context.DeadlineExceeded) still fires, and maps.Clone(req.Meta) correctly guards the caller's map against NewMetaFromMap's in-place progressToken delete.

What's strong

• Dependency direction improves: adapter now depends on pkg/vmcp (domain) instead of pkg/vmcp/composer.
• Security validation (timeout cap/default, schema size/depth, content size/depth) and the any → map[string]any assertion are genuinely untouched and remain on-path in unchanged order.
• The internal mcpElicitationRequester test seam is private, single-method, and adapter-local — does not trip anti-pattern #8.
• Corrects a stale doc comment (max 1hour → max 10min).

Findings (2 MEDIUM, both test-strength; non-blocking)

See inline comments. Both are edge cases the #5436 Testing Strategy explicitly lists as required.

Consensus summary

#	Category	Severity	Finding	Score
1	Test Coverage	MEDIUM	Issue-required "non-map Content → nil" edge case is untested	8/10
2	Test Coverage	MEDIUM	timeout_capped_to_max asserts only no-error; would pass even if cap logic were deleted	8/10

Dropped (score < 7, FYI): mockSDK test vars now hold a domain mock (rename → mockRequester); MetaIsCopied doesn't assert the non-progressToken key survives; no handler-level RequestedSchema mapping assertion; Meta doc comment doesn't note progressToken is reserved.

_{5 specialist agents (architecture, MCP protocol, security, Go quality, tests). Codex cross-review skipped (CLI not installed).}