fix(mcp): return typed structured results from every tool

[tylerkron]

What was wrong

Every tool returned its result only as a blob of JSON text. MCP clients that consume typed results (scripts, agent frameworks, anything that wants to read sampleRateHz without parsing prose) got no outputSchema in tools/list and no structuredContent in the call result. They had to guess the shape of each response and parse the text themselves.

How it was fixed

Every tool now sets UseStructuredContent = true, next to the safety hints from #744 (descriptions are #761's, unchanged). The server then:

• advertises an outputSchema for each tool in tools/list. The schema is always an object: the spec requires that, so list and string results are wrapped as {"result": ...}.
• returns the typed result in structuredContent alongside the same JSON text block.

Null fields are now written out ("latestVersion": null) rather than omitted. The generated schema lists every record property as required, with nullable ones typed ["T","null"], but the SDK's default serializer drops null properties. Without this change, 13 of the 26 tools could return a result with a required key missing. Examples: get_server_info under --no-version-check, configure_analog_channels whenever the rate was not lowered, and list_analog_outputs before any write. A client that validates results against the schema, such as the official TypeScript SDK client, would reject those calls. Tools are now registered through a shared WithDaqifiTools() that passes the SDK's default serializer options with DefaultIgnoreCondition = Never. This one setting covers both structuredContent and the text block.

Tests

• ToolStructuredContentContractTests builds the tools the way the server does and checks that each one advertises an object output schema. It fails if a tool is added without a row in its table.
• NullField_IsStillWritten_SoTheResultMatchesItsOutputSchema runs get_server_info (version check off) through the SDK's own invoke path. It asserts that every key the schema requires is in structuredContent and that latestVersion is an explicit null. Setting the ignore condition back to the SDK default makes this test fail.
• Both contract-test harnesses (this one and fix(mcp): set accurate tool ReadOnly/Destructive/OpenWorld hints #744's ToolAnnotationContractTests) now register through WithDaqifiTools(), the same call Program.cs makes.

Verification

• Ran the built server over stdio. All 26 tools advertise an object outputSchema. get_server_info returns structuredContent that matches its schema, including latestVersion: null. list_connected_devices returns {"result": []}. disconnect_device returns {"result": "..."} plus the plain-text block. Errors still come back as isError text with no structuredContent.
• tools/list is byte-identical with and without the serializer change. Input and output schemas are unaffected; only result serialization changes.
• Full suite green locally (net9 + net10). MCP-only change; no hardware needed.

Notes

• Visible change for text-only clients: null fields now appear as "field": null in the text block instead of being left out. The tool descriptions already say these fields "come back null", so the text now matches them.
• tools/list grows by ~10 KB (the schemas). Clients that forward output schemas to the model pay that in context once per session.
• Once this lands, the output schemas are part of the tool contract: adding, removing or renaming a result field changes what tools/list advertises. (fix(mcp): drop dead digital sampleRateAdjustedFromHz #789 already removed the always-null ConfigureDigitalResult.SampleRateAdjustedFromHz, so that field is never advertised.)
• A tool added later needs UseStructuredContent = true and a row in both contract-test tables; the completeness tests fail otherwise.

🤖 Generated with Claude Code

[qodo-code-review]

PR Summary by Qodo

Enable structured content for all MCP tools

🐞 Bug fix 🧪 Tests 🕐 20-40 Minutes

AI Description

• Enable output schemas and typed responses for all 26 MCP tools.
• Prevent MCP SDK 2.2 from degrading tool results into text-only content blocks.
• Add contract coverage enforcing schemas and complete registration for every advertised tool.

Diagram

sequenceDiagram
    actor Client as MCP Client
    participant Server as MCP Server
    participant SDK as MCP SDK 2.2
    participant Tools as DAQiFi Tools
    Client->>Server: tools/list
    Server->>SDK: Discover tools
    SDK->>Tools: Read attributes
    Tools-->>SDK: Structured flag
    SDK-->>Server: Output schemas
    Server-->>Client: Advertised schemas
    Client->>Server: tools/call
    Server->>Tools: Invoke tool
    Tools-->>SDK: Typed result
    SDK-->>Server: structuredContent
    Server-->>Client: Structured response

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Discover contract cases dynamically

• ➕ Automatically tests every attributed tool without maintaining a separate name table.
• ➕ Reduces duplication between production attributes and test data.
• ➖ Would not detect accidental tool removal or renaming.
• ➖ Provides a less explicit inventory of the expected public tool surface.

Recommendation: Keep the PR's explicit attribute configuration and expected-tool table. The SDK requires the flag at tool registration, while the table verifies both structured schemas and the intended public tool inventory; dynamic discovery alone would miss accidental removals.

Files changed (2) +133 / -26

Bug fix (1) +26 / -26

DaqifiTools.csEnable structured responses across all MCP tools +26/-26

Enable structured responses across all MCP tools

• Sets 'UseStructuredContent = true' on all 26 'McpServerTool' attributes. MCP SDK 2.2 can now expose each return schema through 'tools/list' and serialize typed results into 'structuredContent'.

src/Daqifi.Mcp/Tools/DaqifiTools.cs

Tests (1) +107 / -0

ToolStructuredContentContractTests.csAdd structured-content contracts for every MCP tool +107/-0

Add structured-content contracts for every MCP tool

• Adds protocol-level tests that construct each tool through the SDK and verify it advertises an object output schema. A reflection-based completeness assertion keeps the 26-tool expectation table synchronized with the advertised tool surface.

src/Daqifi.Mcp.Tests/ToolStructuredContentContractTests.cs

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. New tool classes escape the contract ✓ Resolved 🐞 Bug ⚙ Maintainability

Description

ToolMethods restricts its reflection scan to DaqifiTools, while the production server discovers
tools across the assembly. Adding another attributed tool class therefore allows an unstructured
production tool to bypass both the expected-name table and output-schema test.

Code

src/Daqifi.Mcp.Tests/ToolStructuredContentContractTests.cs[R99-102]

+    private static IEnumerable<MethodInfo> ToolMethods() =>
+        typeof(DaqifiTools)
+            .GetMethods(BindingFlags.Public | BindingFlags.Static)
+            .Where(m => m.GetCustomAttribute<McpServerToolAttribute>() is not null);

Relevance

●●● Strong

Accepted precedents favor independent reflection discovery to prevent newly added types or entry
points bypassing completeness tests.

PR-#715
PR-#680

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

Production calls assembly-wide WithToolsFromAssembly, but the new helper reflects only over
typeof(DaqifiTools), proving that tools declared in another class are outside the contract.

src/Daqifi.Mcp.Tests/ToolStructuredContentContractTests.cs[99-102]
src/Daqifi.Mcp/Program.cs[37-40]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The structured-content contract scans only `DaqifiTools`, but production uses assembly-wide MCP tool discovery. Tools added in another attributed class would be registered without entering this contract.

## Fix Focus Areas
- src/Daqifi.Mcp.Tests/ToolStructuredContentContractTests.cs[99-102]
- src/Daqifi.Mcp/Program.cs[37-40]

## Recommended Fix
Enumerate all types in the production MCP assembly and collect every public method carrying `McpServerToolAttribute`, matching the assembly-wide discovery used by the server. Use that complete set for both the expected-name comparison and output-schema checks.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Context sources

✅ Compliance rules (platform): 13 rules

✅ Cross-repo context — repo relationships

Review mode: 🧠 Deep: This push contains substantial, independent behavioral changes across concurrency, caching, CI/release publishing, and MCP tool contracts, creating a dense set of easy-to-miss defects.

Tip of the day

💡 Did you know, you can turn on the rule miner and Qodo learns your standards from review history

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

[tylerkron]

/agentic_review

[qodo-code-review]

Code review by qodo was updated up to the latest commit 532d08b

[tylerkron]

/agentic_review

[qodo-code-review]

Code review by qodo was updated up to the latest commit c1ca79d

[tylerkron]

/agentic_review

[qodo-code-review]

Code review by qodo was updated up to the latest commit 4ca5fde

[tylerkron]

/agentic_review

[qodo-code-review]

Code review by qodo was updated up to the latest commit 8aed4b1

[tylerkron]

Reviewed (Claude): verified end-to-end over stdio that all 26 tools advertise an object outputSchema and return structuredContent alongside the unchanged text; contract test now reads the real WithToolsFromAssembly registration. Qodo-clean on 8aed4b1, CI green — ready for review. Note: conflicts textually with #744 (every attribute line) and #738; trivial rebase for whichever merges later.

[tylerkron]

/agentic_review

[qodo-code-review]

Code review by qodo was updated up to the latest commit 8a36cc6

[tylerkron]

Rebased onto main after #738 merged; Qodo-clean on 8a36cc6, CI green.

[tylerkron]

/agentic_review

[qodo-code-review]

Code review by qodo was updated up to the latest commit ef38167

[tylerkron]

Qodo-clean, CI green — ready for review

[tylerkron]

/agentic_review

[qodo-code-review]

No code changes since the last review — review skipped

[tylerkron]

Qodo-clean, CI green — ready for review