Fix membrane pointerization for structured tool output#81
Merged
christopherkarani merged 3 commits intoMay 5, 2026
Conversation
4411d22 to
724463b
Compare
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>
e79d5b2
into
christopherkarani:main
0 of 4 checks passed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Structured tool results were converted with
SendableValue.descriptionbefore 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
SendableValueresults 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 throughJSONSerializationwith[.sortedKeys, .fragmentsAllowed]so the conversation/transcript/pointer payload is parsable JSON.JSONSerializationraises an Objective-CNSExceptionon non-finite doubles, which Swiftdo/catchcannot trap. The helper pre-screens the value tree and falls back todescriptionfor non-finite doubles instead of crashing the agent.JSONSerializationfailure now logs atLog.agents.warninginstead of being silently swallowed.SwarmMCPServerServiceTestsis updated to the newerCallTool.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):
Targeted suites:
swift test --filter AgentToolOutputTextTests— 8 tests, all pass (new file)swift test --filter MembraneIntegrationTests— 7 tests, all pass (includes the newpointerizedStructuredToolOutputResolvesAsJSONintegration test)swift test --filter SwarmMCPServerServiceTests— 13 tests, all passThe new unit tests in
AgentToolOutputTextTestscover:JSONSerialization.jsonObjectInt,Double,Bool,null)Notes for the reviewer
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).description's JSON-ish format. This is strictly an improvement in correctness; full local suite passes confirm no test snapshots assumed the old representation.