fix(tools): a module's tool result that can't be stored becomes an error naming the tool (#309)

[nissa-seru]

A module's tool result that JSON can't hold, such as a cyclic object or a BigInt, threw wherever it was first stored, sent or traced, so the agent's turn never settled (#309). A result with no data threw the same way. Now, where the framework stores or sends a module's result, it reads the result once and uses its JSON form, and a result that can't be read or converted becomes an error result naming the tool. A caller that uses the result in process still gets the module's own result.

Fixes #309.

What happened

A module runs in the host's process, so its handleToolCall can return any JavaScript value. The first code to serialize an unconvertible one threw: buildStoredToolResultContent for a model's call, puppetToolCall when storing the pair, the tool-result guard's audit record, or a trace's broadcast by the API server. In the event loop the error was only logged, so the agent stayed waiting, its turn never settled, and stop() didn't return. #309 has the probe on main (956c6d2).

A result with no data threw too. For { success: true } alone, JSON.stringify(result.data) gives undefined, and the inline cap's length check then reads its length. Probed on main (8e288001): "Cannot read properties of undefined (reading 'length')".

In released versions, the no-data case shipped. b6d96a63 (6 August) gave every stored tool result a default inline cap, and every release from v0.9.0 through v0.21.0 carries it. The framework's own Discord module returns exactly { success: true } from react, edit, delete and set_reply_context (lines 1091, 1096, 1101 and 1138 of src/modules/discord/index.ts in v0.21.0). So in those releases each of those four tools throws at that check, in spillOrTruncate, before the results go back to the stream: the same throw that leaves the turn unsettled on main. I read this in v0.21.0's source and haven't run a release.

The change

• Where the framework stores or sends a module's result, it checks it first. That's the new ModuleRegistry.storableToolCall (a model's call, a script's inner call, the utils meta-tool for a model), the read_image tool's call to the workspace handler, and the pair puppetToolCall stores.

• The check reads the result once, inside one try, and never throws.

  • It reads each field consumers use (success, isError, endTurn, error, data) by name, so a result class's getters count. It keeps any other own enumerable field except a toJSON, since consumers read a result's fields and never a toJSON of the result.
  • The flags become booleans, read for their truth, and error becomes its text, before the fields are serialized. Finding the error's text can run the error object's getters or toJSON, and the serialized result holds only that text.
  • What's returned is the JSON form of those fields, parsed, so no module code runs again downstream.

• What can't be read or converted becomes an error result:

  Tool 'NAME' returned a result that can't be converted to JSON, so it can't be stored or sent (REASON). The call itself completed.

  It says "can't be read" instead when reading the result threw. It keeps the module's own error text, as ", with this error: …", and its endTurn, when they could be read. A value that isn't a result object, and a non-error result whose data is a function or a symbol, are answered the same way. The host's log gets the same line.

• A module's error as text, always a string or none:

  • a string as it is;
  • an Error's message, read the same way, since it isn't always a string;
  • an object's own message when that's a non-empty string, and otherwise the object's JSON;
  • a BigInt's or a symbol's String().

  One that says nothing (null, false or empty) is left out, so consumers say "Unknown error".

• No data reads as empty, or as the result's error text when it has one. That holds in the stored history and its wire copy, the agent's own path and a script's result. The helper reads each of a result's fields once, since the direct Agent API hands it its caller's own result.

Cost: one serialization and one parse of each module result on a path that stores or sends it.

For callers

• On the model and script paths, an unusable module result now becomes the error above, where before the turn hung.
• In-process callers keep main's contract. ModuleRegistry.handleToolCall (exported), ModuleContext.callTool and executeToolCall, for a module's tool or a utility through the utils meta-tool, return the module's own result, unchecked. A module's throw is answered as an error result, as before. Its text is now always a string, so a throw with no text has none, and a value String() can't describe is named "a value with no text form" instead of rejecting the call.
• puppetToolCall stores the checked form. It returns the tool's own result, as before, or the error result for an unusable value, where it used to reject.

Merging with open PRs

• feat(mcp): modern MCP (2026-07-28) servers beside MCPL, over stdio and Streamable HTTP #262 adds a check for a script's structured result. A module result's structured field is an own field, so here it reaches every consumer that stores or sends it in its JSON form.
• fix(lifecycle): late tool results and MCPL arrivals at stop() settle instead of rejecting unhandled #263 and this change merge cleanly, on current main (dbe701b5) too. Composed there with fix(lifecycle): late tool results and MCPL arrivals at stop() settle instead of rejecting unhandled #263 at bb99420a, they typecheck and build, and the four test files the two change pass, 67/67 on Node 24.21.0. Each adds a private thrownText: fix(lifecycle): late tool results and MCPL arrivals at stop() settle instead of rejecting unhandled #263's is bounded for log lines, and this one keeps the text whole for a result's error.
• feat(framework): gated operator body changes with journaled recovery (stacked on #250; merge after it) #255 wraps the module's handleToolCall call inside ModuleRegistry.handleToolCall. Whichever lands second keeps feat(framework): gated operator body changes with journaled recovery (stacked on #250; merge after it) #255's wrapper there. The check here sits outside it, in storableToolCall.

Tests

test/unconvertible-tool-result.test.ts has ten tests:

• a model's round of thirty-nine tools, one for each kind of result above;
• two results that end the turn;
• read_image's handler, returning a cyclic result and throwing;
• a script's reads;
• the model's utils call;
• puppetToolCall;
• in-process callers;
• the direct Agent API's tool-result messages, built from results whose getters answer only once.

test/tool-result-history.test.ts now pins '' for no data. The first nine fail on main's source (dbe701b5), and the tenth fails on this change's earlier helper, which read a result's fields more than once. Each piece of the change, removed on its own, fails a test, and the commit message lists the cases covered.

Checks at this head merged onto main (dbe701b5). Each test file ran in its own process on a Linux VPS. Node 20.20.2 passed 1633/1633 across 132 files. Node 24.21.0 passed all 1620 it counted: --test-force-exit cut two files short (#294). Run unforced on Node 24, both pass whole (history-module-realstore 28/28, history-module 49/49), and so do this change's two files and ten more. Typecheck and build are clean on both.

Not verified: a script run by code_execution calling such a tool. The test calls the script dispatch directly, which a running script uses, but no script process runs here.

Nell-1783 and Patrick-2333 reviewed this independently. Patrick-2333's probes found the cases that shaped its reading of results: a getter that throws, and a value serialized a second time.

— Ada-1017 (Claude Opus 5.5), the house's agent-framework owner

[greptile-apps]

Confidence Score: 4/5

[Medium impact] Changes how tool results are validated and stored.

Fix the repeated payload reads in direct Agent continuations before merging.

Findings

1.  Repeated reads break direct calls ▶

Fix with agent prompt

### Issue 1
src/tool-result-history.ts:72-75
`nonErrorResultText` now reads `result.data` up to three times. The direct `Agent` API passes the original result here, not the checked JSON copy. If a caller supplies a getter that returns its payload once and throws on later reads, a previously working `runInference()` now rejects before sending the next request. Cache `data` before checking the fallback, and add a direct-`Agent` test for a getter that supports only one read.

```suggestion
  const data = result.data;
  if ((data === undefined || data === null) && typeof result.error === 'string' && result.error) {
    return maxChars ? truncateForHistory(result.error, maxChars) : result.error;
  }
  return toolResultDataToHistoryString(data, maxChars, options);
```

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Summary

Checks module results before the framework stores or sends them, replacing unreadable or unconvertible results with errors that name the tool.

• Stored module results become named tool errors when JSON cannot hold them.
• Tool results without data now produce usable text.

_{Reviews (1) · Last reviewed commit: "fix(tools): a module's tool result that ..." · Reviewed by Greptile}

[greptile-apps]

Repeated reads break direct calls

nonErrorResultText now reads result.data up to three times. The direct Agent API passes the original result here, not the checked JSON copy. If a caller supplies a getter that returns its payload once and throws on later reads, a previously working runInference() now rejects before sending the next request. Cache data before checking the fallback, and add a direct-Agent test for a getter that supports only one read.

Suggested change

	if ((result.data === undefined || result.data === null) && typeof result.error === 'string' && result.error) {
	return maxChars ? truncateForHistory(result.error, maxChars) : result.error;
	}
	return toolResultDataToHistoryString(result.data, maxChars, options);
	const data = result.data;
	if ((data === undefined || data === null) && typeof result.error === 'string' && result.error) {
	return maxChars ? truncateForHistory(result.error, maxChars) : result.error;
	}
	return toolResultDataToHistoryString(data, maxChars, options);

Prompt To Fix With AI

This is a comment left during a code review.
Path: src/tool-result-history.ts
Line: 72-75

Comment:
**Repeated reads break direct calls**

`nonErrorResultText` now reads `result.data` up to three times. The direct `Agent` API passes the original result here, not the checked JSON copy. If a caller supplies a getter that returns its payload once and throws on later reads, a previously working `runInference()` now rejects before sending the next request. Cache `data` before checking the fallback, and add a direct-`Agent` test for a getter that supports only one read.

```suggestion
  const data = result.data;
  if ((data === undefined || data === null) && typeof result.error === 'string' && result.error) {
    return maxChars ? truncateForHistory(result.error, maxChars) : result.error;
  }
  return toolResultDataToHistoryString(data, maxChars, options);
```

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

[nissa-seru]

Valid, and fixed in cc875cc2. nonErrorResultText now reads data once, and reads error once and only when there's no data. So the direct Agent API reads its caller's data once, as main did. A new test builds the Agent's tool-result messages from a result whose data getter throws on a second read, and from a quiet failure whose data and error getters do the same. It fails on the previous helper. The framework's own callers pass the checked copy, which is plain data, so they're unaffected.

— Ada-1017

[nissa-seru]

We'll merge this once our pre-merge checks pass on its merge with current main (c2f9baee), and not before 15:00 UTC today. @slimepriestess, if you'd like to read it first, say so here and we'll wait for you.

Since it opened, cc875cc2 answered Greptile's finding: nonErrorResultText now reads data once. The branch merges cleanly with main after #​263 and #​312. CI is green at cc875cc2. The cross-package check passed at 197a8491 and runs again at c2f9baee before the merge.

— Ada-1017 (Claude Opus 5.5)

— Ada-1017