Skip to content

Fix membrane pointerization for structured tool output#81

Merged
christopherkarani merged 3 commits into
christopherkarani:mainfrom
objectiveous:fix/membrane-structured-tool-output-json-upstream
May 5, 2026
Merged

Fix membrane pointerization for structured tool output#81
christopherkarani merged 3 commits into
christopherkarani:mainfrom
objectiveous:fix/membrane-structured-tool-output-json-upstream

Conversation

@objectiveous
Copy link
Copy Markdown
Contributor

@objectiveous objectiveous commented May 4, 2026

Summary

Structured tool results were converted with SendableValue.description before being passed into Membrane pointerization. That representation is not a JSON contract: it does not escape ", \, or \n, so any structured tool output containing those characters produces a non-parsable pointer payload.

This changes the Agent tool-output boundary to serialize non-string SendableValue results as canonical JSON (sorted keys, proper escaping, fragments allowed) before Membrane transforms or pointerizes them. Plain string tool results still pass through unchanged.

What changed

  • Agent.toolOutputText(for:) — new helper. String passes through; everything else goes through JSONSerialization with [.sortedKeys, .fragmentsAllowed] so the conversation/transcript/pointer payload is parsable JSON.
  • NaN/Infinity hardening. JSONSerialization raises an Objective-C NSException on non-finite doubles, which Swift do/catch cannot trap. The helper pre-screens the value tree and falls back to description for non-finite doubles instead of crashing the agent.
  • Diagnostic logging. A genuine JSONSerialization failure now logs at Log.agents.warning instead of being silently swallowed.
  • MCP SDK test compatibility. SwarmMCPServerServiceTests is updated to the newer CallTool.Result.Content.text(text:annotations:_meta:) case shape so the test target compiles against the currently-pinned MCP SDK. Without this the test target won't build, so we can't deliver the fix without it.

Tests

Local upstream verification (CI runs are currently blocked by an Actions billing issue on the upstream account):

SWARM_HIVE_RUNTIME=1 SWARM_INCLUDE_HIVE=1 swift test --no-parallel
# Test run with 1708 tests in 234 suites passed after 17.297 seconds.

Targeted suites:

  • swift test --filter AgentToolOutputTextTests — 8 tests, all pass (new file)
  • swift test --filter MembraneIntegrationTests — 7 tests, all pass (includes the new pointerizedStructuredToolOutputResolvesAsJSON integration test)
  • swift test --filter SwarmMCPServerServiceTests — 13 tests, all pass

The new unit tests in AgentToolOutputTextTests cover:

  • Plain string passthrough (with embedded quotes/newlines preserved verbatim)
  • Dictionary serializes as JSON with sorted keys and proper escape sequences
  • Nested dictionary/array round-trip via JSONSerialization.jsonObject
  • Scalar fragments (Int, Double, Bool, null)
  • Top-level array
  • Non-finite double fallback at the top level and nested inside a dictionary (regression guard against NSException crash)

Notes for the reviewer

  • The PR is rebased on current main (02537a6, including the Wax 0.1.20 fix from Bump Wax pin from 0.1.19 to 0.1.20 (Swift 6.2 strict-concurrency fix) #80).
  • The MCP SDK case-pattern update is technically unrelated to the membrane fix but is required for the test target to compile against the current MCP SDK pin — happy to split into a separate PR if preferred, but the fix can't be tested without it.
  • Behavior change visible to the model: dictionary/array tool outputs now reach the model as canonical JSON instead of description's JSON-ish format. This is strictly an improvement in correctness; full local suite passes confirm no test snapshots assumed the old representation.

@objectiveous objectiveous force-pushed the fix/membrane-structured-tool-output-json-upstream branch from 4411d22 to 724463b Compare May 4, 2026 04:06
JSONSerialization raises an Objective-C NSException (not a Swift
error) when a Double is NaN or Infinity, which Swift do/catch cannot
trap. Pre-screen the SendableValue tree so we fall back to
description before reaching JSONSerialization in those cases instead
of crashing the agent.

Also replace the silent catch with a Log.agents.warning so any
future serialization failure is diagnosable, and add focused unit
tests covering string passthrough, sorted-key JSON, escaped strings,
nested round-trip, scalar fragments, top-level arrays, and the
non-finite fallback (top-level and nested).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@christopherkarani christopherkarani merged commit e79d5b2 into christopherkarani:main May 5, 2026
0 of 4 checks passed
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