fix(tool): Fix custom tools spreading out string output

[TheodoreSpeaks]

Summary

https://sim-ai.slack.com/archives/C09B82TTM7Y/p1773913304490249

We have a bug introduced with stripping double underscored fields. We incorrectly assumed these outputs would always be jsons, but custom tools can pass a string. This incorrectly spreads out the output into json character by character.

Fix:

1. Do a typecheck on outputs before stripping any tool output.
2. Skip stripping hidden fields on custom tools. This logic should only apply to our own tool calls.

Type of Change

• Bug fix
• New feature
• Breaking change
• Documentation
• Other: ___________

Testing

Duplicated issue by creating custom tool that returns a string. Validated issue, then validated fix. Then validated that double underscore isn't stripped from custom tools.

Checklist

• Code follows project style guidelines
• Self-reviewed my changes
• Tests added/updated and passing
• No new warnings introduced
• I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

Screenshots/Videos

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment

Project	Deployment	Actions	Updated (UTC)
docs	Skipped		Mar 19, 2026 6:38pm

[TheodoreSpeaks]

@cursor review

[cursor]

PR Summary

Medium Risk
Changes tool result sanitization logic, which can alter outputs returned to users; low complexity but touches a central execution path for all tool calls.

Overview
Fixes a regression where output post-processing assumed tool outputs were always JSON objects, causing string/array outputs to be treated like objects and mangled.

stripInternalFields and filterOutputForLog now bail out for non-object/array/null outputs, and executeTool skips __* field stripping entirely for custom tools while still stripping __* fields for built-in tool object outputs. Adds unit tests covering string, array, built-in object stripping, and custom-tool __ field preservation.

^{Written by Cursor Bugbot for commit eb8b17e. This will update automatically on new commits. Configure here.}

[cursor]

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

[TheodoreSpeaks]

@cursor review

[greptile-apps]

Greptile Summary

This PR fixes a bug where custom tool outputs that are strings (instead of objects) were being passed to stripInternalFields, which iterates over Object.entries(). Because strings are iterable with numeric indices, this caused the string to be spread character-by-character into an object like {"0": "h", "1": "e", ...} instead of being returned as-is.

The fix has two complementary parts:

• Custom tools bypass stripping entirely: isCustomTool(normalizedToolId) is now checked at both stripInternalFields call sites in executeTool, and custom tools simply pass their output through without any field stripping. This is semantically correct since the __-prefix convention is internal to the platform's own tools, not custom user tools.
• Defensive type guard: A typeof output !== 'object' check is added to both stripInternalFields and filterOutputForLog to short-circuit on non-object inputs, providing a safety net for any future caller that might pass a primitive.

Key concerns:

• Both new call sites use finalResult.output || {} rather than finalResult.output ?? {}. Since || coerces any falsy value (including empty string "", 0, false) to {}, an empty-string output from a custom tool would be silently lost. Switching to ?? would only fall back on null/undefined and fully address the described bug for all primitive output values.
• The ToolResponse.output type is declared as Record<string, any>, which does not formally include string/primitive values. Updating this type to Record<string, any> | string | unknown (or a union) would make the type contract match the documented runtime behavior of custom tools.

Confidence Score: 3/5

• The core fix is correct and well-tested, but the || {} fallback can still silently discard falsy primitive outputs (e.g. empty strings) from custom tools.
• The primary bug (non-empty string being spread character-by-character) is correctly resolved at both call sites. Four new tests verify the key scenarios. However, the || {} operator at both new call sites introduces a subtle edge case where any falsy primitive output from a custom tool (most notably "") is silently replaced with {}, which is inconsistent with the stated goal of preserving non-object custom tool outputs. This is a minor but real gap in an otherwise straightforward bug fix.
• Pay close attention to the two || {} expressions in apps/sim/tools/index.ts at lines 831–833 and 888–890.

Important Files Changed

Filename	Overview
apps/sim/tools/index.ts	Adds isCustomTool guard to skip stripInternalFields for custom tools; the `
apps/sim/tools/index.test.ts	Four new test cases cover string output preservation, array output preservation, __-field stripping for regular tools, and __-field preservation for custom tools. All cases directly validate the described fix.
apps/sim/executor/utils/output-filter.ts	Defensive early-return guard added before iterating output keys — correctly handles strings, arrays, and null to prevent character-by-character spreading in log filtering.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[executeTool called] --> B{isCustomTool?}
B -- Yes --> C["finalResult.output || {}"]
B -- No --> D["stripInternalFields(finalResult.output || {})"]
D --> E{typeof output === 'object'\nand not null\nand not Array?}
E -- No --> F[Return output as-is\n✅ strings/arrays pass through]
E -- Yes --> G[Iterate Object.entries\nStrip __ prefixed keys]
G --> H[Return filtered object]
C --> I{output is truthy?}
I -- Yes --> J[Return output unchanged\n✅ string/object preserved]
I -- No --> K["Return {} ⚠️ empty string / 0 / false lost"]
J --> L[Set result.output]
H --> L
K --> L
F --> L

Loading

_{Last reviewed commit: "Fix lint"}

[cursor]

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR