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.

Author: RhysSullivan

apps/cloud/src/services/sources-api.node.test.ts

@@ -207,9 +207,15 @@ describe("sources api (HTTP)", () => {
       expect(execution.structured).toMatchObject({
         status: "completed",
         result: {
-          message: "hello",
-          suffix: "world",
-          path: "/echo/hello",
+          ok: true,
+          data: {
+            status: 200,
+            data: {
+              message: "hello",
+              suffix: "world",
+              path: "/echo/hello",
+            },
+          },
         },
         logs: [],
       });
@@ -315,7 +321,7 @@ describe("sources api (HTTP)", () => {
       expect(execution.isError).toBe(false);
       expect(execution.structured).toMatchObject({
         status: "completed",
-        result: { hello: "Hello Ada" },
+        result: { ok: true, data: { hello: "Hello Ada" } },
       });
       const requests = yield* server.requests;
       expect(requests.some((request) => request.payload.query?.includes("__schema"))).toBe(true);
@@ -391,7 +397,8 @@ describe("sources api (HTTP)", () => {
       expect(execution.structured).toMatchObject({
         status: "completed",
         result: {
-          content: [{ type: "text", text: "cloud-mcp-ok" }],
+          ok: true,
+          data: [{ type: "text", text: "cloud-mcp-ok" }],
         },
       });
       expect((yield* server.requests).length).toBeGreaterThanOrEqual(2);

packages/core/execution/src/tool-invoker.leak.test.ts

@@ -3,6 +3,7 @@ import { Data, Effect, Schema } from "effect";
 
 import { ElicitationResponse, createExecutor, definePlugin } from "@executor-js/sdk";
 import { makeTestConfig } from "@executor-js/sdk/testing";
+import { ExecutionToolError } from "./errors";
 import { makeExecutorToolInvoker } from "./tool-invoker";
 
 const EmptyInputSchema = Schema.toStandardSchemaV1(
@@ -11,9 +12,9 @@ const EmptyInputSchema = Schema.toStandardSchemaV1(
 
 const acceptAll = () => Effect.succeed(ElicitationResponse.make({ action: "accept" }));
 
-// Simulate a realistic plugin-internal tagged error whose `cause` carries
-// sensitive internal context (DB connection string, full HTTP request with
-// Authorization header echoed back, file paths, stack traces).
+// Plugin-internal tagged error whose `cause` carries sensitive internal
+// context. The dispatcher must route this through the opaque-generic
+// path so none of that context reaches the sandbox via Error.message.
 class FakeOpenApiInvocationError extends Data.TaggedError("OpenApiInvocationError")<{
   readonly message: string;
   readonly cause: unknown;
@@ -33,83 +34,81 @@ const leakyPlugin = definePlugin(() => ({
           description: "",
           inputSchema: EmptyInputSchema,
           handler: () =>
-            Effect.fail(new FakeOpenApiInvocationError({
-              message: "HTTP request failed",
-              cause: {
-                _tag: "HttpClientError",
-                request: {
-                  method: "GET",
-                  url: "https://internal.dealcloud/v1/entities?accessToken=SECRET_TOKEN_xyz",
-                  headers: { Authorization: "Bearer SECRET_TOKEN_xyz" },
+            Effect.fail(
+              new FakeOpenApiInvocationError({
+                message: "HTTP request failed",
+                cause: {
+                  _tag: "HttpClientError",
+                  request: {
+                    method: "GET",
+                    url: "https://internal.dealcloud/v1/entities?accessToken=SECRET_TOKEN_xyz",
+                    headers: { Authorization: "Bearer SECRET_TOKEN_xyz" },
+                  },
+                  stack:
+                    "Error: ECONNREFUSED\n    at /home/svc/executor/packages/plugins/openapi/...:142:11",
+                  dbConnString: "postgres://app:p@ssw0rd@10.0.0.5:5432/executor",
                 },
-                stack:
-                  "Error: ECONNREFUSED\n    at /home/svc/executor/packages/plugins/openapi/...:142:11",
-                dbConnString: "postgres://app:p@ssw0rd@10.0.0.5:5432/executor",
-              },
-            })),
+              }),
+            ),
         },
         {
           name: "throwsRawError",
           description: "",
           inputSchema: EmptyInputSchema,
           handler: () =>
             Effect.fail(
-              Object.assign(new Error("Internal: secret 'sk_live_abcd' rotation failed"), {
-                stack:
-                  "Error: Internal: secret 'sk_live_abcd' rotation failed\n    at /home/svc/.../secret-store.ts:88",
-              }),
+              Object.assign(
+                // oxlint-disable-next-line executor/no-error-constructor -- boundary: leak test deliberately fails with a raw Error + crafted stack to assert the dispatcher's opaque-generic redaction
+                new Error("Internal: secret 'sk_live_abcd' rotation failed"),
+                {
+                  stack:
+                    "Error: Internal: secret 'sk_live_abcd' rotation failed\n    at /home/svc/.../secret-store.ts:88",
+                },
+              ),
             ),
         },
       ],
     },
   ],
 }));
 
-describe("internal-error leak audit", () => {
-  it.effect("plugin tagged error: only .message escapes, cause stays hidden", () =>
+describe("internal-error leak audit (opaque defects)", () => {
+  it.effect("plugin tagged error: defect surfaces only as opaque generic + correlation id", () =>
     Effect.gen(function* () {
-      const executor = yield* createExecutor(
-        makeTestConfig({ plugins: [leakyPlugin()] as const }),
-      );
+      const executor = yield* createExecutor(makeTestConfig({ plugins: [leakyPlugin()] as const }));
       const invoker = makeExecutorToolInvoker(executor, {
         invokeOptions: { onElicitation: acceptAll },
       });
 
-      const err = yield* Effect.flip(
-        invoker.invoke({ path: "leaky.failsWithCause", args: {} }),
-      );
+      const err = yield* Effect.flip(invoker.invoke({ path: "leaky.failsWithCause", args: {} }));
+      expect(err).toBeInstanceOf(ExecutionToolError);
+      // oxlint-disable-next-line executor/no-unknown-error-message -- boundary: leak test inspects the rendered message to assert it is the opaque generic
       const msg = (err as { message: string }).message;
-      // eslint-disable-next-line no-console
-      console.log("[leak failsWithCause]", msg);
-
-      expect(msg).toBe("HTTP request failed");
+      // Must be the canonical opaque shape: "Internal tool error [<hex>]"
+      expect(msg).toMatch(/^Internal tool error \[[0-9a-f]{8}\]$/);
+      // Crucially, no internal context leaks
       expect(msg).not.toContain("SECRET_TOKEN_xyz");
       expect(msg).not.toContain("p@ssw0rd");
       expect(msg).not.toContain("packages/plugins");
       expect(msg).not.toContain("HttpClientError");
+      expect(msg).not.toContain("HTTP request failed");
     }),
   );
 
-  it.effect("plain Error with stack: stack does NOT leak, only message", () =>
+  it.effect("plain Error with stack: stack and message do NOT escape", () =>
     Effect.gen(function* () {
-      const executor = yield* createExecutor(
-        makeTestConfig({ plugins: [leakyPlugin()] as const }),
-      );
+      const executor = yield* createExecutor(makeTestConfig({ plugins: [leakyPlugin()] as const }));
       const invoker = makeExecutorToolInvoker(executor, {
         invokeOptions: { onElicitation: acceptAll },
       });
 
-      const err = yield* Effect.flip(
-        invoker.invoke({ path: "leaky.throwsRawError", args: {} }),
-      );
+      const err = yield* Effect.flip(invoker.invoke({ path: "leaky.throwsRawError", args: {} }));
+      // oxlint-disable-next-line executor/no-unknown-error-message -- boundary: leak test inspects the rendered message to assert it is the opaque generic
       const msg = (err as { message: string }).message;
-      // eslint-disable-next-line no-console
-      console.log("[leak throwsRawError]", msg);
-
-      // message itself contains the secret because the plugin put it there —
-      // that's plugin discipline. But stack and file path should not appear.
+      expect(msg).toMatch(/^Internal tool error \[[0-9a-f]{8}\]$/);
       expect(msg).not.toContain("secret-store.ts");
       expect(msg).not.toContain("at /home/");
+      expect(msg).not.toContain("sk_live_abcd");
     }),
   );
 });