Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions .claude/skills/prod-telemetry/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,15 @@ join the same traces via traceparent).

**Span names worth querying** (and their custom attrs):

- `mcp.execute` / `mcp.execute.resume` — `mcp.execute.mode`
(`pausable`/`inline`), `mcp.execute.code_length`, and
`mcp.execute.outcome` (`ok`/`fail`/`paused`) with, on failures,
`mcp.execute.error_kind` (`type_error` | `reference_error` |
`syntax_error` | `range_error` | `tool_error` | `timeout` |
`resource_limit` | `serialization_error` | `thrown` | `unknown`).
Sandbox script failures ride the MCP success channel, so `status.code`
stays OK — filter on these attributes, not span status. Spans from
before the attributes shipped carry neither; absence is not success.
- `executor.tool.execute` — `mcp.tool.name` (full address), and since
PR #992: `executor.tool.outcome` (`ok`/`fail`),
`executor.tool.error_code`, `executor.tool.error_status`,
Expand Down
42 changes: 37 additions & 5 deletions packages/core/execution/src/engine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,25 @@ export type ResumeResponse = {
// caller is itself the human approver (the operator-facing Run/Test panel).
const acceptAllHandler: ElicitationHandler = () => Effect.succeed({ action: "accept" });

/**
* Stamp the current `mcp.execute` / `mcp.execute.resume` span with how the
* execution ended. Sandbox failures ride the success channel as
* `ExecuteResult.error`, so without this the span reads OK and the failure
* class is unqueryable. Attributes stay enumerable identifiers — never the
* error message itself.
*/
const annotateExecuteOutcome = (result: ExecuteResult) =>
Effect.annotateCurrentSpan(
result.error
? { "mcp.execute.outcome": "fail", "mcp.execute.error_kind": result.errorKind ?? "unknown" }
: { "mcp.execute.outcome": "ok" },
);

const annotateExecutionOutcome = (execution: ExecutionResult) =>
execution.status === "paused"
? Effect.annotateCurrentSpan({ "mcp.execute.outcome": "paused" })
: annotateExecuteOutcome(execution.result);

// ---------------------------------------------------------------------------
// Result formatting
// ---------------------------------------------------------------------------
Expand Down Expand Up @@ -529,6 +548,7 @@ export const createExecutionEngine = <E extends Cause.YieldableError = CodeExecu
if (options?.autoApprove) {
yield* Effect.annotateCurrentSpan({ "mcp.execute.auto_approve": true });
const result = yield* runInlineExecution(code, { onElicitation: acceptAllHandler });
yield* annotateExecuteOutcome(result);
return { status: "completed", result } satisfies ExecutionResult;
}

Expand Down Expand Up @@ -596,7 +616,9 @@ export const createExecutionEngine = <E extends Cause.YieldableError = CodeExecu
),
);

return (yield* awaitCompletionOrPause(fiber, pauseQueue)) as ExecutionResult;
const outcome = (yield* awaitCompletionOrPause(fiber, pauseQueue)) as ExecutionResult;
yield* annotateExecutionOutcome(outcome);
return outcome;
});

/**
Expand All @@ -619,13 +641,17 @@ export const createExecutionEngine = <E extends Cause.YieldableError = CodeExecu
const settled = settledOutcomes.get(executionId);
if (settled) {
yield* Effect.annotateCurrentSpan({ "mcp.execute.resume.replayed": true });
return (yield* settled) as ExecutionResult;
const replayed = (yield* settled) as ExecutionResult;
yield* annotateExecutionOutcome(replayed);
return replayed;
}

const pending = pendingResumes.get(executionId);
if (pending) {
yield* Effect.annotateCurrentSpan({ "mcp.execute.resume.joined_inflight": true });
return (yield* Deferred.await(pending)) as ExecutionResult;
const joined = (yield* Deferred.await(pending)) as ExecutionResult;
yield* annotateExecutionOutcome(joined);
return joined;
}

const paused = pausedExecutions.get(executionId);
Expand All @@ -640,7 +666,7 @@ export const createExecutionEngine = <E extends Cause.YieldableError = CodeExecu
content: response.content,
});

return (yield* awaitCompletionOrPause(paused.fiber, paused.pauseQueue).pipe(
const outcome = (yield* awaitCompletionOrPause(paused.fiber, paused.pauseQueue).pipe(
Effect.onExit((exit) =>
Effect.gen(function* () {
recordSettledOutcome(executionId, exit);
Expand All @@ -649,6 +675,8 @@ export const createExecutionEngine = <E extends Cause.YieldableError = CodeExecu
}),
),
)) as ExecutionResult;
yield* annotateExecutionOutcome(outcome);
return outcome;
});

/**
Expand All @@ -670,7 +698,11 @@ export const createExecutionEngine = <E extends Cause.YieldableError = CodeExecu
},
toolDiscoveryProvider,
);
return yield* codeExecutor.execute(code, invoker).pipe(Effect.withSpan("executor.code.exec"));
const result = yield* codeExecutor
.execute(code, invoker)
.pipe(Effect.withSpan("executor.code.exec"));
yield* annotateExecuteOutcome(result);
return result;
});

return {
Expand Down
25 changes: 25 additions & 0 deletions packages/kernel/core/src/error-kind.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import { describe, expect, it } from "@effect/vitest";

import { classifyThrownExecuteError } from "./error-kind";

describe("classifyThrownExecuteError", () => {
it("maps built-in JS error names to their kinds", () => {
expect(classifyThrownExecuteError("SyntaxError")).toBe("syntax_error");
expect(classifyThrownExecuteError("TypeError")).toBe("type_error");
expect(classifyThrownExecuteError("ReferenceError")).toBe("reference_error");
expect(classifyThrownExecuteError("RangeError")).toBe("range_error");
expect(classifyThrownExecuteError("DataCloneError")).toBe("serialization_error");
});

it("maps runtime-tagged names to their kinds", () => {
expect(classifyThrownExecuteError("ExecutionToolError")).toBe("tool_error");
expect(classifyThrownExecuteError("ExecutionTimeoutError")).toBe("timeout");
});

it("treats anything else as the script's own throw", () => {
expect(classifyThrownExecuteError("Error")).toBe("thrown");
expect(classifyThrownExecuteError("CustomDomainError")).toBe("thrown");
expect(classifyThrownExecuteError(null)).toBe("thrown");
expect(classifyThrownExecuteError(undefined)).toBe("thrown");
});
});
37 changes: 37 additions & 0 deletions packages/kernel/core/src/error-kind.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
/**
* Enumerable classification of a failed sandbox execution, derived from the
* structured error the runtime already holds (the thrown error's `name`, or
* the runtime's own typed failure). Carried on `ExecuteResult.errorKind`
* beside the descriptive `error` string so telemetry can count failure
* classes as identifiers without ever recording message content.
*/
export type ExecuteErrorKind =
| "syntax_error"
| "type_error"
| "reference_error"
| "range_error"
| "tool_error"
| "timeout"
| "resource_limit"
| "serialization_error"
| "thrown";

const KIND_BY_ERROR_NAME: Readonly<Record<string, ExecuteErrorKind>> = {
SyntaxError: "syntax_error",
TypeError: "type_error",
ReferenceError: "reference_error",
RangeError: "range_error",
ExecutionToolError: "tool_error",
ExecutionTimeoutError: "timeout",
DataCloneError: "serialization_error",
};

/**
* Classify an error thrown inside the sandbox by its `name`. Runtimes tag
* their own conditions with dedicated names (`ExecutionToolError` for a
* failed tool-dispatch rethrow, `ExecutionTimeoutError` for the in-sandbox
* deadline); anything unrecognized is the script's own `throw`.
*/
export const classifyThrownExecuteError = (
errorName: string | null | undefined,
): ExecuteErrorKind => (errorName != null ? KIND_BY_ERROR_NAME[errorName] : undefined) ?? "thrown";
1 change: 1 addition & 0 deletions packages/kernel/core/src/index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
export * from "./types";
export * from "./error-kind";
export * from "./validation";
export * from "./json-schema";
export * from "./effect-errors";
Expand Down
3 changes: 3 additions & 0 deletions packages/kernel/core/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import type * as Cause from "effect/Cause";
import type * as Effect from "effect/Effect";

import type { CodeExecutionError } from "./effect-errors";
import type { ExecuteErrorKind } from "./error-kind";

/** Branded tool path */
export type ToolPath = string & { readonly __toolPath: unique symbol };
Expand Down Expand Up @@ -42,6 +43,8 @@ export type ExecuteResult = {
result: unknown;
output?: ExecuteOutputItem[];
error?: string;
/** Enumerable failure class for telemetry; never carries message content. */
errorKind?: ExecuteErrorKind;
logs?: string[];
};

Expand Down
40 changes: 37 additions & 3 deletions packages/kernel/runtime-dynamic-worker/src/executor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,14 @@ import * as Data from "effect/Data";
import * as Effect from "effect/Effect";

import {
classifyThrownExecuteError,
CodeCompilationError,
recoverExecutionBody,
SandboxHostTimeoutError,
SandboxRuntimeError,
stripTypeScript,
type CodeExecutor,
type ExecuteErrorKind,
type ExecuteOutputItem,
type ExecuteResult,
type SandboxToolInvoker,
Expand Down Expand Up @@ -231,6 +233,23 @@ const RUNTIME_SIGNATURES = [

export type SandboxFailureKind = "compilation" | "runtime" | "internal";

/**
* Signatures of the serialization subset of `RUNTIME_SIGNATURES`, used to
* split `SandboxRuntimeError` into its two telemetry classes: the user
* returned something that can't cross the sandbox boundary vs the isolate
* hit a CPU/memory/capacity limit.
*/
const SERIALIZATION_SIGNATURES = [
"could not be cloned",
"does not support serialization",
"Could not serialize",
] as const;

const sandboxRuntimeErrorKind = (message: string): ExecuteErrorKind =>
SERIALIZATION_SIGNATURES.some((signature) => message.includes(signature))
? "serialization_error"
: "resource_limit";

/**
* Classify a sandbox rejection so the runtime knows whether to surface
* its message descriptively (the user's mistake or a transient,
Expand Down Expand Up @@ -785,6 +804,9 @@ const evaluate = (
return {
result: error ? null : response.result,
error,
...(response.error
? { errorKind: classifyThrownExecuteError(serializedErrorName(response.error.primary)) }
: {}),
output:
Array.isArray(response.output) && response.output.length > 0 ? response.output : undefined,
logs: response.logs,
Expand Down Expand Up @@ -812,16 +834,28 @@ const runInDynamicWorker = (
// `DynamicWorkerExecutionError` and remain opaque.
Effect.catchTags({
CodeCompilationError: (error) =>
Effect.succeed({ result: null, error: error.message } satisfies ExecuteResult),
Effect.succeed({
result: null,
error: error.message,
errorKind: "syntax_error",
} satisfies ExecuteResult),
SandboxRuntimeError: (error) =>
Effect.succeed({ result: null, error: error.message } satisfies ExecuteResult),
Effect.succeed({
result: null,
error: error.message,
errorKind: sandboxRuntimeErrorKind(error.message),
} satisfies ExecuteResult),
// A wedged isolate that the in-sandbox timer failed to catch is
// unrecoverable, but reporting it as a delivered, descriptive error beats
// the original symptom (open-ended silence). Fold it into the success
// channel like the other safe-to-report conditions so it reaches the
// model instead of collapsing to an opaque internal error.
SandboxHostTimeoutError: (error) =>
Effect.succeed({ result: null, error: error.message } satisfies ExecuteResult),
Effect.succeed({
result: null,
error: error.message,
errorKind: "timeout",
} satisfies ExecuteResult),
}),
Effect.withSpan("executor.code.exec.dynamic_worker", {
attributes: { "executor.runtime": "dynamic-worker" },
Expand Down
26 changes: 26 additions & 0 deletions packages/kernel/runtime-dynamic-worker/src/invocation.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -283,6 +283,7 @@ describe("makeDynamicWorkerExecutor", () => {
);

expect(result.error).toBe('{"code":"bad_request","detail":"team missing"}');
expect(result.errorKind).toBe("thrown");
expect(result.result).toBeNull();
});

Expand All @@ -304,6 +305,7 @@ describe("makeDynamicWorkerExecutor", () => {
);

expect(result.error).toBeUndefined();
expect(result.errorKind).toBeUndefined();
expect(result.result).toBe(7);
});

Expand All @@ -316,6 +318,25 @@ describe("makeDynamicWorkerExecutor", () => {
);

expect(result.error).toBe("Internal tool error");
expect(result.errorKind).toBe("tool_error");
});

it("classifies a wrong-shape property access as a type error", async () => {
const executor = makeDynamicWorkerExecutor({ loader });
const invoker = makeInvoker(() => ({ ok: true, data: {} }));

// The wrong-shape-guess signature: the script assumes a response field
// that isn't there and dereferences undefined.
const result = await Effect.runPromise(
executor.execute(
"async () => { const r = await tools.issues.list({}); return r.issues.length; }",
invoker,
),
);

expect(result.result).toBeNull();
expect(result.error).toContain("undefined");
expect(result.errorKind).toBe("type_error");
});

it("surfaces a syntax error with the parser's descriptive message, not an opaque generic", async () => {
Expand All @@ -335,6 +356,7 @@ describe("makeDynamicWorkerExecutor", () => {
expect(result.error).not.toBe("Internal tool error");
expect(result.error).not.toContain("Internal tool error");
expect(result.error?.toLowerCase()).toContain("unexpected");
expect(result.errorKind).toBe("syntax_error");
});

it("surfaces smart-quote paste errors descriptively", async () => {
Expand Down Expand Up @@ -369,6 +391,7 @@ describe("makeDynamicWorkerExecutor", () => {
expect(result.error).not.toBe("Internal tool error");
expect(result.error).not.toContain("Internal tool error");
expect(result.error?.toLowerCase()).toContain("could not be cloned");
expect(result.errorKind).toBe("serialization_error");
});

it("preserves public ExecutionToolError messages across the worker bridge", async () => {
Expand All @@ -390,6 +413,7 @@ describe("makeDynamicWorkerExecutor", () => {
expect(result.error).toBe(
"tools.search expects an object: { query?: string; namespace?: string; limit?: number; offset?: number }",
);
expect(result.errorKind).toBe("tool_error");
});

it("does not expose host error stack details to sandbox error handlers", async () => {
Expand Down Expand Up @@ -518,6 +542,7 @@ describe("makeDynamicWorkerExecutor", () => {

expect(result.result).toBeNull();
expect(result.error).toBe(`Execution timed out after ${timeoutMs}ms`);
expect(result.errorKind).toBe("timeout");
});

it("resets the execution deadline after a tool dispatch returns", async () => {
Expand All @@ -540,6 +565,7 @@ describe("makeDynamicWorkerExecutor", () => {

expect(result.result).toBeNull();
expect(result.error).toBe(`Execution timed out after ${timeoutMs}ms`);
expect(result.errorKind).toBe("timeout");
});

it("returns an execution error for circular tool args", async () => {
Expand Down
4 changes: 2 additions & 2 deletions packages/kernel/runtime-dynamic-worker/src/module-template.ts
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ export const buildExecutorModule = (body: string, timeoutMs: number): string =>
" __inFlight -= 1;",
" __lastReturnedAt = Date.now();",
" }",
" if (!data.ok) throw new Error(__publicToolErrorMessage(data.error) || 'Internal tool error');",
" if (!data.ok) throw Object.assign(new Error(__publicToolErrorMessage(data.error) || 'Internal tool error'), { name: 'ExecutionToolError' });",
" return __decodeBinary(data.result);",
" })();",
" },",
Expand All @@ -219,7 +219,7 @@ export const buildExecutorModule = (body: string, timeoutMs: number): string =>
" __watchdogInterval = setInterval(() => {",
" const now = Date.now();",
` if (__inFlight === 0 && now - Math.max(__start, __lastReturnedAt) >= ${timeoutMs}) {`,
` reject(new Error("Execution timed out after ${timeoutMs}ms"));`,
` reject(Object.assign(new Error("Execution timed out after ${timeoutMs}ms"), { name: "ExecutionTimeoutError" }));`,
" }",
" }, 1000);",
" });",
Expand Down
Loading