Exoeval: instrumented, ocap interpreter, subset of JS - #33
Conversation
There was a problem hiding this comment.
Pull request overview
This PR replaces the previous Cap’n Web RPC/sandbox-based “CodeMode” execution with a new instrumented OCAP interpreter (exoeval) that evaluates a constrained subset of JavaScript, and updates the website + examples to use the new execution model.
Changes:
- Introduce
src/exoeval/*(parser/evaluator, tool visibility/validation, and safe built-ins) and switch the public API tocodemode(...). - Refactor SQL builder/tooling to use the new
@tooldecorator model and remove the oldRpcToolset/ stream-transport runtime. - Update website worker + UI and repository CI/build configuration to remove the capnweb submodule workflow and use npm dependencies.
Reviewed changes
Copilot reviewed 41 out of 43 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| website/worker/index.ts | Migrates RPC targets and “execute_code” tool creation to codemode + new tool decorator usage. |
| website/src/AgentChat.tsx | Removes client-side new Function execution; routes ExoAgent chat to server-side interpreter; adjusts capnweb session usage. |
| website/package.json | Adds npm capnweb dependency. |
| website/package-lock.json | Updates lockfile for npm capnweb and related dependency changes. |
| vitest.config.ts | Expands Vitest include globs to cover packages/** and examples/**. |
| src/tool-wrapper.ts | Reworks tool wrapping/types generation toward exoeval “toolable” functions. |
| src/stream-transport.ts | Deletes old stream transport used for sandbox RPC runtime. |
| src/stream-transport.test.ts | Deletes tests for the removed stream transport. |
| src/sql/integration.test.ts | Rewrites integration tests from capnweb RPC to exoeval execution. |
| src/sql/expression.ts | Removes RpcToolset inheritance in favor of plain class usage with exoeval tooling. |
| src/sql/builder.ts | Refactors SQL query builder tool exposure/validation to new @tool + exoeval semantics. |
| src/sql/builder.test.ts | Updates imports to use ../exoeval/tool. |
| src/rpc-toolset.ts | Removes the old RpcToolset/decorator implementation. |
| src/rpc-toolset.test.ts | Removes tests for the deleted RpcToolset. |
| src/rpc-toolset-test-helpers.ts | Updates helper toolsets to use new @tool decorator model (no RpcToolset). |
| src/index.ts | Updates public exports to codemode, exoEval, and tool. |
| src/exoeval/utils.ts | Adds shared utilities + error formatting and disallowed property set. |
| src/exoeval/tool.ts | Implements @tool decorator, tool visibility, arg validation, and tool field access rules. |
| src/exoeval/tool.test.ts | Adds tests for tool exposure/validation behavior. |
| src/exoeval/scope.ts | Adds scope/binding implementation for destructuring patterns. |
| src/exoeval/index.ts | Adds exoEval + exoFn entry points and builtin injection. |
| src/exoeval/expr.ts | Adds expression context abstraction and identity context behavior. |
| src/exoeval/evaluator.ts | Adds acorn-based evaluator for the supported JS subset. |
| src/exoeval/evaluator.test.ts | Adds extensive evaluator semantics and builtin coverage tests. |
| src/exoeval/builtins.ts | Adds curated built-ins (Array/String/Date/Object/JSON/Math/Number). |
| src/code-mode.ts | Replaces class-based CodeMode with codemode using exoEval. |
| src/code-mode.test.ts | Updates tests for new codemode API and validation behavior. |
| src/code-mode-runtime.ts | Deletes old injected runtime used for sandbox execution. |
| src/code-mode-deno.ts | Deletes Deno sandbox support. |
| src/code-mode-deno.test.ts | Deletes Deno sandbox tests. |
| src/capnweb-test-helpers.ts | Deletes capnweb RPC test harness utilities. |
| packages/capnweb | Removes capnweb submodule reference. |
| package.json | Updates scripts/deps for new architecture (removes capnweb submodule build steps, adds acorn, renames CI scripts). |
| package-lock.json | Updates dependency lockfile (notably acorn version). |
| examples/simple.ts | Updates example to use codemode and new “property tool” calling convention. |
| examples/simple.test.ts | Updates example tests for codemode-style API. |
| examples/saas-bot.ts | Updates SaaS example to use codemode and new calling convention. |
| examples/saas-bot.test.ts | Updates SaaS example tests accordingly. |
| examples/package.json | Renames ci script to check. |
| eslint.config.js | Disables test/prefer-lowercase-title rule. |
| README.md | Updates docs toward codemode (but currently inconsistent with new API). |
| .github/workflows/publish.yml | Removes capnweb submodule build step. |
| .github/workflows/ci.yml | Removes capnweb submodule build and Deno install; updates examples script invocation. |
Files not reviewed (1)
- website/package-lock.json: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| execute: async ({ code }: { code: string }, _opts: ToolExecutionOptions): Promise<R> => { | ||
| const fn = exoEval(code) | ||
| if (typeof fn !== 'function') { | ||
| throw new TypeError('Code did not return a function') | ||
| } | ||
| const wrapped = dts ? tools : wrapTools(tools as WrappableTools) | ||
| return await fn(wrapped) |
There was a problem hiding this comment.
codemode currently ignores the ToolExecutionOptions passed into execute, so wrapped tools never receive toolCallId, messages, etc. This breaks the ai Tool contract for any tool whose execute(input, opts) depends on those options. Consider passing opts through by wrapping each tool with a closure that calls tool.execute(input, opts) (and keeping validation) instead of calling tool.execute with only the input.
| execute: async ({ code }: { code: string }, _opts: ToolExecutionOptions): Promise<R> => { | |
| const fn = exoEval(code) | |
| if (typeof fn !== 'function') { | |
| throw new TypeError('Code did not return a function') | |
| } | |
| const wrapped = dts ? tools : wrapTools(tools as WrappableTools) | |
| return await fn(wrapped) | |
| execute: async ({ code }: { code: string }, opts: ToolExecutionOptions): Promise<R> => { | |
| const fn = exoEval(code) | |
| if (typeof fn !== 'function') { | |
| throw new TypeError('Code did not return a function') | |
| } | |
| const wrapped = dts ? tools : wrapTools(tools as WrappableTools) | |
| const wrappedWithOpts: any = Array.isArray(wrapped) | |
| ? wrapped.map((tool: any) => ({ | |
| ...tool, | |
| execute: (input: any) => tool.execute(input, opts), | |
| })) | |
| : Object.fromEntries( | |
| Object.entries(wrapped as any).map(([name, tool]: [string, any]) => [ | |
| name, | |
| { | |
| ...tool, | |
| execute: (input: any) => tool.execute(input, opts), | |
| }, | |
| ]), | |
| ) | |
| return await fn(wrappedWithOpts) |
| const wrapTool = (tool: Tool): ((...args: unknown[]) => unknown) => { | ||
| if (!tool.execute) { | ||
| throw new Error(`Tool ${tool.title} does not have an execute function`) | ||
| } | ||
| const schema = toStandardSchema(tool.inputSchema) | ||
| return asTool(tool.execute as (...args: any[]) => unknown, undefined, [schema]) | ||
| } |
There was a problem hiding this comment.
wrapTool wraps tool.execute as a 1-arg function, but Tool.execute is typically (input, options); the current wrapper drops the ToolExecutionOptions entirely. This will cause subtle runtime failures for tools that rely on options. Update the wrapper API to accept opts and ensure the generated callable forwards (input, opts).
| const isOrderByItem = <TN extends TableNamespace>(value: unknown): value is OrderByItem<TN> => { | ||
| if (Array.isArray(value)) { | ||
| return value.every(e => e instanceof SqlExpression || e instanceof OrderByValue) | ||
| } | ||
| return value instanceof SqlExpression || value instanceof OrderByValue | ||
| } |
There was a problem hiding this comment.
isOrderByItem is declared as a type guard for OrderByItem<TN> (a function type), but the implementation validates the return value of that function (SqlExpression/OrderByValue or arrays). This incorrect type predicate can mislead TypeScript and makes the intent unclear. Adjust the type guard to target the return type (e.g. value is ReturnType<OrderByItem<TN>>).
| "typecheck": "tsc --noEmit", | ||
| "ci": "npm run typecheck && npm run test", | ||
| "check": "npm run typecheck && npm run test", | ||
| "npm-install-test": "TMP=$(mktemp -d --suffix=-exoagent-examples) && cp -r . $TMP && npm --prefix $TMP i exoagent@$(npm show .. version) && npm --prefix $TMP run ci" |
There was a problem hiding this comment.
The npm-install-test script still runs npm run ci, but this PR renames the examples script from ci to check. This will fail when npm-install-test is executed. Update the script to run npm --prefix $TMP run check (or restore a ci alias).
| "npm-install-test": "TMP=$(mktemp -d --suffix=-exoagent-examples) && cp -r . $TMP && npm --prefix $TMP i exoagent@$(npm show .. version) && npm --prefix $TMP run ci" | |
| "npm-install-test": "TMP=$(mktemp -d --suffix=-exoagent-examples) && cp -r . $TMP && npm --prefix $TMP i exoagent@$(npm show .. version) && npm --prefix $TMP run check" |
| import { codemode, createDenoSandbox } from 'exoagent' | ||
|
|
||
| // Create a capability scoped to user_id=1 | ||
| const userCap = User.on(u => u.id['='](1)).from() | ||
|
|
||
| // Wrap with CodeMode for sandboxed execution | ||
| const codeMode = new CodeMode(createDenoSandbox()) | ||
| const codeTool = await codeMode.wrap({ | ||
| // Wrap with codemode for sandboxed execution | ||
| const codeTool = await codemode(createDenoSandbox(), { | ||
| currentUser: () => userCap, |
There was a problem hiding this comment.
README example imports createDenoSandbox and calls codemode(createDenoSandbox(), ...), but createDenoSandbox is no longer exported (and codemode no longer accepts a sandbox/context argument). Update the README snippet to match the new public API (likely codemode(tools, dts) and/or document the removal of external sandbox execution).
| const right = yield* this.$(node.right) | ||
| // Short-circuit: don't eval the right if the left is null or undefined | ||
| switch (node.operator) { | ||
| case '&&': | ||
| return this.ctx.of(left && right) | ||
| case '||': | ||
| return this.ctx.of(left || right) | ||
| case '??': | ||
| return this.ctx.of(left ?? right) |
There was a problem hiding this comment.
LogicalExpression evaluates both left and right before applying && / || / ??, so it does not short-circuit. This breaks JS semantics and can cause unintended side effects (e.g. false && sideEffect() still evaluates sideEffect). Restructure to evaluate right only when needed for each operator.
| const right = yield* this.$(node.right) | |
| // Short-circuit: don't eval the right if the left is null or undefined | |
| switch (node.operator) { | |
| case '&&': | |
| return this.ctx.of(left && right) | |
| case '||': | |
| return this.ctx.of(left || right) | |
| case '??': | |
| return this.ctx.of(left ?? right) | |
| // Implement JavaScript short-circuit semantics: only evaluate the right-hand side when needed. | |
| switch (node.operator) { | |
| case '&&': { | |
| if (!left) { | |
| // Falsy left short-circuits: return left without evaluating right. | |
| return this.ctx.of(left) | |
| } | |
| const right = yield* this.$(node.right) | |
| return this.ctx.of(right) | |
| } | |
| case '||': { | |
| if (left) { | |
| // Truthy left short-circuits: return left without evaluating right. | |
| return this.ctx.of(left) | |
| } | |
| const right = yield* this.$(node.right) | |
| return this.ctx.of(right) | |
| } | |
| case '??': { | |
| if (left !== null && left !== undefined) { | |
| // Non-nullish left short-circuits: return left without evaluating right. | |
| return this.ctx.of(left) | |
| } | |
| const right = yield* this.$(node.right) | |
| return this.ctx.of(right) | |
| } |
6fcac4f to
a9bcbbc
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 41 out of 43 changed files in this pull request and generated 10 comments.
Files not reviewed (1)
- website/package-lock.json: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| it('validates RpcToolset tool arguments and rejects invalid input', async () => { | ||
| // Assume npm run build:test-deps has been run | ||
| const dtsContent = await readFile('dist/rpc-toolset-test-helpers.d.mts', 'utf-8') | ||
| const dtsContent = await readFile('dist/rpc-toolset-test-helpers.d\.ts', 'utf-8') |
There was a problem hiding this comment.
The file path contains an escaped backslash \.ts which should be just .ts. This will cause the file read to fail as it's looking for a file literally named d\.ts instead of d.ts.
|
|
||
| // Raw SQL chat - client provides SQL executor callback | ||
| @tool.unsafeNoValidation() | ||
| @tool(z.any(), z.any()) |
There was a problem hiding this comment.
The decorator uses z.any() for both parameters, which bypasses type validation. This is unsafe as it allows any type to be passed. Consider using more specific schemas for the message and executeSql parameters. For example, z.string().max(MAX_MESSAGE_LENGTH) for message and z.function().args(z.string()).returns(z.promise(sqlResultSchema)) for executeSql.
| }) | ||
| }) | ||
|
|
||
| describe('sql integration with deno sandbox', () => { |
There was a problem hiding this comment.
The test suite title mentions "deno sandbox" but the actual implementation no longer uses Deno as a sandbox (it was removed in this PR). This test suite name should be updated to reflect the new exoeval-based implementation. Consider renaming to "sql integration with exoeval" or similar.
| }, 10000) | ||
| }) | ||
|
|
||
| it('executes a join via CodeMode', async () => { |
There was a problem hiding this comment.
The test description mentions "CodeMode" but this API no longer exists (replaced by codemode). Update the test description to match the new API name.
| }) | ||
|
|
||
| describe('sql integration with deno sandbox', () => { | ||
| it('executes a basic select via CodeMode', async () => { |
There was a problem hiding this comment.
The test description mentions "CodeMode" but this API no longer exists (replaced by codemode). Update the test description to match the new API name.
| | NamespacedExpression<TN, FromItem<N2, F2>>, on?: NamespacedExpression<TN & { [k in N2]: F2 }, SqlExpressionIn>): QueryBuilder<N, TN & { [k in N2]: F2 }, S> | ||
|
|
||
| @tool.unsafeNoValidation() | ||
| @tool(z.any(), z.any()) |
There was a problem hiding this comment.
Similar to chatRawSql, this method uses z.any() for both parameters which bypasses type validation. Consider using more specific schemas. The fromItem parameter could be validated to ensure it's either a FromItem or a function that returns a FromItem. The on parameter could be validated to ensure it's either undefined or a function returning a SqlExpressionIn.
| it('executes user code that calls RpcToolset tools', async () => { | ||
| // Assume npm run build:test-deps has been run | ||
| const dtsContent = await readFile('dist/rpc-toolset-test-helpers.d.mts', 'utf-8') | ||
| const dtsContent = await readFile('dist/rpc-toolset-test-helpers.d\.ts', 'utf-8') |
There was a problem hiding this comment.
The file path contains an escaped backslash \.ts which should be just .ts. This will cause the file read to fail as it's looking for a file literally named d\.ts instead of d.ts.
| it('executes user code that chains RpcToolset tools', async () => { | ||
| // Assume npm run build:test-deps has been run | ||
| const dtsContent = await readFile('dist/rpc-toolset-test-helpers.d.mts', 'utf-8') | ||
| const dtsContent = await readFile('dist/rpc-toolset-test-helpers.d\.ts', 'utf-8') |
There was a problem hiding this comment.
The file path contains an escaped backslash \.ts which should be just .ts. This will cause the file read to fail as it's looking for a file literally named d\.ts instead of d.ts.
| // @ts-expect-error — tool() overload resolution fails with generic method signature | ||
| @tool(fn.returns(z.any())) | ||
| select<S2 extends RowLikeIn>(select: (arg: TN) => S2) { | ||
| const result = select(this.arg) | ||
| invariant(isRowLikeIn(result), 'select must return a RowLikeIn') | ||
| return new QueryBuilder<N, TN, AsRowLike<S2>>({ ...this.paramsForCopy(), selectRowLike: asRowLike(result) }) | ||
| } | ||
|
|
||
| @tool.callback() | ||
| where(where: ToolCallback<(arg: TN) => SqlExpressionIn>) { | ||
| const whereUnwrapped = tool.unwrap(where, isSqlExpressionIn) | ||
| return whereUnwrapped(this.arg, (result) => { | ||
| return new QueryBuilder<N, TN, S>({ ...this.paramsForCopy(), whereExpression: combinePredicates(this.whereExpression, asSqlExpression(result)) }) | ||
| }) | ||
| // @ts-expect-error — tool() overload resolution fails with generic method signature | ||
| @tool(fn.returns(z.custom(isSqlExpressionIn))) | ||
| where(where: (arg: TN) => SqlExpressionIn) { | ||
| const result = where(this.arg) | ||
| return new QueryBuilder<N, TN, S>({ ...this.paramsForCopy(), whereExpression: combinePredicates(this.whereExpression, asSqlExpression(result)) }) | ||
| } | ||
|
|
||
| @tool.callback() | ||
| orderBy(orderBy: ToolCallback<OrderByItem<TN>>) { | ||
| const orderByUnwrapped = tool.unwrap(orderBy, (arg): arg is ReturnType<OrderByItem<TN>> => { | ||
| if (Array.isArray(arg)) { | ||
| return arg.every(e => e instanceof SqlExpression || e instanceof OrderByValue) | ||
| } | ||
| return arg instanceof SqlExpression || arg instanceof OrderByValue | ||
| }) | ||
| return orderByUnwrapped(this.arg, (raw) => { | ||
| const rawArray = Array.isArray(raw) ? raw : [raw] | ||
| const exprs = rawArray.map((e) => { | ||
| invariant(e instanceof SqlExpression || e instanceof OrderByValue, 'orderBy must return a SqlExpression/OrderByValue or an array of SqlExpressions/OrderByValues') | ||
| return e instanceof SqlExpression ? new OrderByValue(e) : e | ||
| }) | ||
|
|
||
| return new QueryBuilder<N, TN, S>({ ...this.paramsForCopy(), orderByExpressions: (this.orderByExpressions ?? []).concat(exprs) }) | ||
| // @ts-expect-error — tool() overload resolution fails with generic method signature | ||
| @tool(fn.returns(z.custom(isOrderByItem<TN>))) |
There was a problem hiding this comment.
The @ts-expect-error comments indicate that the tool() overload resolution is failing with generic method signatures. While these may work at runtime, they suggest a type system issue that could lead to problems. Consider either fixing the overload definitions in the tool() function or finding an alternative way to annotate these generic methods.
| } | ||
| const desc = Object.getOwnPropertyDescriptor(sequenced, key) | ||
| if (isPlainObject(sequenced) && desc != null) { | ||
| const desc = Object.getOwnPropertyDescriptor(sequenced, key) |
There was a problem hiding this comment.
There's a redundant call to Object.getOwnPropertyDescriptor(sequenced, key) on line 146. The descriptor is already retrieved on line 144 and stored in the desc variable. Remove the duplicate call and use the existing desc variable.
| const desc = Object.getOwnPropertyDescriptor(sequenced, key) |
c528374 to
f919713
Compare
Features:
@toolannotations (i.e., annotating OCaps)