Route tool defects opaquely; remove legacy envelope shim - #826
Merged
Conversation
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
executor-marketing | e38616a | Commit Preview URL Branch Preview URL |
May 17 2026, 05:32 AM |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
executor-cloud | e38616a | May 17 2026, 05:33 AM |
@executor-js/cli
@executor-js/config
@executor-js/execution
@executor-js/sdk
@executor-js/codemode-core
@executor-js/runtime-quickjs
@executor-js/plugin-file-secrets
@executor-js/plugin-google-discovery
@executor-js/plugin-graphql
@executor-js/plugin-keychain
@executor-js/plugin-mcp
@executor-js/plugin-onepassword
@executor-js/plugin-openapi
executor
commit: |
Adds `ToolResult<T>` / `ToolError` to @executor-js/sdk and a
`ToolResult.ok` / `ToolResult.fail` constructor pair.
Updates the sandbox tool dispatcher to accept both the new typed union
(passes through unchanged) and the legacy `{ data, error }` envelope.
The legacy branch now walks known upstream shapes (Microsoft Graph,
DealCloud, JSON:API, plain message-bearing bodies) before falling back
to a clamped JSON.stringify of the body, so structured 4xx payloads no
longer collapse to 'Tool execution failed'. The cause/defect branch
keeps its strict .message-only discipline.
…covery
Migrates each dynamic plugin's invokeTool to wrap its result in the
typed ToolResult<T> union. OpenAPI and Google Discovery map non-2xx
responses to ToolResult.fail with an extracted upstream message and
the raw body in error.details; 2xx responses to ToolResult.ok with
{ status, headers, data }. GraphQL maps 200-with-errors bodies to
ToolResult.fail({ code: 'graphql_errors', ... }) and bare data to
ToolResult.ok. MCP maps { isError: true } to ToolResult.fail and
content arrays to ToolResult.ok.
Static plugin tools (executor.openapi.previewSpec / addSource,
executor.graphql.addSource) now also wrap their plain values in
ToolResult.ok for shape consistency.
Updates plugin tests that asserted on the old { status, headers, data,
error } envelope or the bare MCP { content } shape. Tests that consume
the OpenAPI plugin's invocation result use the new unwrapInvocation
helper from @executor-js/plugin-openapi/testing.
Removes the legacy { data, error } envelope shim from the sandbox tool
dispatcher. The invoker now passes ToolResult<T> through unchanged and
wraps any other plain-value plugin return in ToolResult.ok so the
sandbox surface is uniform.
Plugin/infra defects no longer pass their message into the sandbox.
The dispatcher generates a short hex correlation id, logs the full
cause with that id under executor.correlation_id, and rejects with
`Internal tool error [<corrId>]`. The QuickJS bridge and the
dynamic-worker module template defensively re-stamp the same opaque
shape; the MCP host server's top-level execute failure path does the
same.
ExecutionToolError in-band messages from the execution package's
built-in validators (tools.search arg checks, etc.) are still passed
through at the QuickJS bridge so model-facing input errors keep their
useful diagnostic.
Tests:
- ToolResult.ok / fail / isToolResult constructor unit tests.
- repro tests assert structured upstream payloads now reach the
sandbox through ToolResult.error.details (not through .message).
- leak tests pin the new invariant: plugin defects only escape as the
opaque generic + correlation id; no token / connection string / file
path leaks into Error.message.
- QuickJS end-to-end defect test confirms the same shape at the
sandbox boundary.
- Cloud HTTP integration tests, MCP host tests, dynamic-worker
invocation tests, and plugin tests updated for the new ToolResult
wire shape and the opaque-generic defect contract.
RhysSullivan
force-pushed
the
rs/tool-result-defect-routing
branch
from
May 17, 2026 04:17
cd16d15 to
c35794f
Compare
RhysSullivan
marked this pull request as ready for review
May 17, 2026 04:56
RhysSullivan
force-pushed
the
rs/tool-result-defect-routing
branch
3 times, most recently
from
May 17, 2026 05:20
a29cfaa to
96bbeec
Compare
RhysSullivan
force-pushed
the
rs/tool-result-defect-routing
branch
from
May 17, 2026 05:30
96bbeec to
e38616a
Compare
RhysSullivan
added a commit
that referenced
this pull request
May 31, 2026
* feat(tool-result): add ToolResult union and dual-shape invoker
Adds `ToolResult<T>` / `ToolError` to @executor-js/sdk and a
`ToolResult.ok` / `ToolResult.fail` constructor pair.
Updates the sandbox tool dispatcher to accept both the new typed union
(passes through unchanged) and the legacy `{ data, error }` envelope.
The legacy branch now walks known upstream shapes (Microsoft Graph,
DealCloud, JSON:API, plain message-bearing bodies) before falling back
to a clamped JSON.stringify of the body, so structured 4xx payloads no
longer collapse to 'Tool execution failed'. The cause/defect branch
keeps its strict .message-only discipline.
* feat(plugins): emit ToolResult from openapi, graphql, mcp, google-discovery
Migrates each dynamic plugin's invokeTool to wrap its result in the
typed ToolResult<T> union. OpenAPI and Google Discovery map non-2xx
responses to ToolResult.fail with an extracted upstream message and
the raw body in error.details; 2xx responses to ToolResult.ok with
{ status, headers, data }. GraphQL maps 200-with-errors bodies to
ToolResult.fail({ code: 'graphql_errors', ... }) and bare data to
ToolResult.ok. MCP maps { isError: true } to ToolResult.fail and
content arrays to ToolResult.ok.
Static plugin tools (executor.openapi.previewSpec / addSource,
executor.graphql.addSource) now also wrap their plain values in
ToolResult.ok for shape consistency.
Updates plugin tests that asserted on the old { status, headers, data,
error } envelope or the bare MCP { content } shape. Tests that consume
the OpenAPI plugin's invocation result use the new unwrapInvocation
helper from @executor-js/plugin-openapi/testing.
* feat(tool-result): make ToolResult mandatory and route defects opaquely
Removes the legacy { data, error } envelope shim from the sandbox tool
dispatcher. The invoker now passes ToolResult<T> through unchanged and
wraps any other plain-value plugin return in ToolResult.ok so the
sandbox surface is uniform.
Plugin/infra defects no longer pass their message into the sandbox.
The dispatcher generates a short hex correlation id, logs the full
cause with that id under executor.correlation_id, and rejects with
`Internal tool error [<corrId>]`. The QuickJS bridge and the
dynamic-worker module template defensively re-stamp the same opaque
shape; the MCP host server's top-level execute failure path does the
same.
ExecutionToolError in-band messages from the execution package's
built-in validators (tools.search arg checks, etc.) are still passed
through at the QuickJS bridge so model-facing input errors keep their
useful diagnostic.
Tests:
- ToolResult.ok / fail / isToolResult constructor unit tests.
- repro tests assert structured upstream payloads now reach the
sandbox through ToolResult.error.details (not through .message).
- leak tests pin the new invariant: plugin defects only escape as the
opaque generic + correlation id; no token / connection string / file
path leaks into Error.message.
- QuickJS end-to-end defect test confirms the same shape at the
sandbox boundary.
- Cloud HTTP integration tests, MCP host tests, dynamic-worker
invocation tests, and plugin tests updated for the new ToolResult
wire shape and the opaque-generic defect contract.
* Fix tool result contracts
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
1 participant
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
Final phase of the
ToolResultmigration. The dispatcher now passesToolResult<T>straight through to the sandbox and routes escaped Effect failures as opaque generics so internal causes cannot leak.packages/core/execution/src/tool-invoker.ts: removes the old{ data, error }envelope detection helpers. Non-ToolResultplain returns are wrapped inToolResult.okso the sandbox surface is uniform.executor.correlation_id, and surface onlyInternal tool error [<corrId>]to the sandbox. Cause messages, stacks, URLs, tokens, headers, and connection strings stay out of model-visible output.runtime-quickjsandruntime-dynamic-workerdefensively preserve the same opaque defect shape.ExecutionToolErrorfrom in-band validators still passes through so model-facing input errors keep useful diagnostics.ToolResult.fail, the invoker passes that value through unchanged, and sandbox code can inspectresult.ok === falseplusresult.error.details.Tests
tool-result.test.ts: coversToolResult.ok,ToolResult.fail, andisToolResultvia the schema-backed discriminated union.tool-invoker.test.ts: covers the described return type against actual sandbox invocation results, including failedToolResultvalues and structured upstream error payloads.tool-invoker.leak.test.ts: pins the defect-routing invariant: plugin defects escape only as the opaque generic plus correlation id. Tokens, connection strings, file paths, and stacks stay hidden.Test plan
bun run format:checkbun run lintbun run typecheckbun run teste38616a3b.result.error.details.Internal tool error [<corrId>]while server logs carry the full cause.