From 8344c48656131d941a48fc70d421808db8b7cd39 Mon Sep 17 00:00:00 2001 From: Hugo Richard Date: Fri, 28 Aug 2026 17:26:02 +0100 Subject: [PATCH] fix(eve): stamp approval as a direct defineTool property --- .changeset/durable-approval-property.md | 5 + .github/CONTRIBUTING.md | 8 +- .github/workflows/ci.yml | 5 +- AGENTS.md | 16 ++- .../docs/2.frameworks/1.eve-extension.md | 2 +- .../references/eve-extension.md | 2 +- package.json | 1 + packages/github-tools-eve-extension/README.md | 2 +- .../extension/tools/github.ts | 52 +++++--- .../github-tools-eve-extension/package.json | 3 +- .../test/durable-define-tool.test.ts | 121 ++++++++++++++++++ turbo.json | 3 + 12 files changed, 190 insertions(+), 30 deletions(-) create mode 100644 .changeset/durable-approval-property.md create mode 100644 packages/github-tools-eve-extension/test/durable-define-tool.test.ts diff --git a/.changeset/durable-approval-property.md b/.changeset/durable-approval-property.md new file mode 100644 index 0000000..13afa4d --- /dev/null +++ b/.changeset/durable-approval-property.md @@ -0,0 +1,5 @@ +--- +'@github-tools/eve-extension': patch +--- + +Register `approval` as a direct `defineTool` property so eve 0.44+ can stamp a durable `approvalRequest` descriptor. Write tools no longer cause the resolver to discard the whole GitHub toolset. Requires `eve` `>=0.44.0`. diff --git a/.github/CONTRIBUTING.md b/.github/CONTRIBUTING.md index 00691bb..8f4fe72 100644 --- a/.github/CONTRIBUTING.md +++ b/.github/CONTRIBUTING.md @@ -69,9 +69,13 @@ Every tool splits into a **core** function (pure logic) and a **tool factory** ( 7. Run checks: ```sh -pnpm build && pnpm lint && pnpm typecheck +pnpm build && pnpm lint && pnpm typecheck && pnpm test ``` +### eve extension durable callbacks + +`packages/github-tools-eve-extension/extension/tools/github.ts` must set `execute`, `toModelOutput`, and `approval` as **direct** `defineTool` properties with inline functions. Spreading those keys, or passing `resolveEveApproval(...)` / `always()` as the property value, leaves them without a durable descriptor: on eve 0.44+ the resolver then drops every `github__*` tool. `test/durable-define-tool.test.ts` fails CI if that pattern returns. + ## Pull requests - Create a feature branch from `main` @@ -82,7 +86,7 @@ pnpm build && pnpm lint && pnpm typecheck pnpm changeset ``` -- Ensure `pnpm build`, `pnpm lint`, and `pnpm typecheck` all pass before submitting +- Ensure `pnpm build`, `pnpm lint`, `pnpm typecheck`, and `pnpm test` all pass before submitting ## Commit conventions diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ab4f115..e9d4aa7 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -24,7 +24,7 @@ env: jobs: quality: - name: Lint & typecheck + name: Lint, typecheck & test runs-on: ubuntu-latest timeout-minutes: 15 permissions: @@ -54,6 +54,9 @@ jobs: - name: Typecheck run: pnpm exec turbo run typecheck --affected + - name: Test + run: pnpm exec turbo run test --affected + examples: name: Build examples runs-on: ubuntu-latest diff --git a/AGENTS.md b/AGENTS.md index eb9d54a..b25e48c 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -8,6 +8,7 @@ pnpm build # Build all packages (turbo) pnpm lint # Lint all packages pnpm typecheck # Type-check all packages +pnpm test # SDK vitest + eve-extension durable-callback guard pnpm dev # Run the chat app in dev mode pnpm docs:dev # Run the docs site in dev mode @@ -15,6 +16,7 @@ pnpm docs:dev # Run the docs site in dev mode pnpm --filter @github-tools/sdk build pnpm --filter @github-tools/sdk lint pnpm --filter @github-tools/sdk typecheck +pnpm --filter @github-tools/sdk test # Release pnpm changeset # Create a changeset for user-facing changes @@ -22,7 +24,7 @@ pnpm version-packages # Apply changesets pnpm release # Build SDK + publish ``` -There is no test suite. Verify changes with `pnpm build && pnpm lint && pnpm typecheck`. +Verify changes with `pnpm build && pnpm lint && pnpm typecheck && pnpm test`. The SDK has vitest (`packages/github-tools/**/*.test.ts`). The eve extension has an AST guard (`packages/github-tools-eve-extension/test/durable-define-tool.test.ts`) that fails CI if `execute` / `toModelOutput` / `approval` are spread or not inline on `defineTool`. ## Monorepo Structure @@ -32,7 +34,7 @@ pnpm workspaces + Turborepo. Three packages: - **`apps/chat`** — Nuxt 4 demo app with NuxtHub (SQLite + blob), GitHub OAuth, dual-mode agent (standard `ToolLoopAgent` vs durable `WorkflowAgent`). - **`apps/docs`** — Nuxt 4 docs site built on Docus. Also publishes a consumer-facing Agent Skill at `apps/docs/skills/github-tools-agents/` (served via `/.well-known/skills/`, see `apps/docs/content/docs/1.getting-started/4.agent-skills.md`). -Turbo task dependencies: `lint`, `lint:fix`, and `typecheck` all depend on `^build` (upstream packages must build first). +Turbo task dependencies: `lint`, `lint:fix`, `typecheck`, and `test` all depend on `^build` (upstream packages must build first). ## SDK Architecture (`packages/github-tools`) @@ -93,6 +95,10 @@ export const myTool = (token: GithubTokenInput, { needsApproval = true }: ToolOp Ten presets (`code-review`, `issue-triage`, `repo-explorer`, `ci-ops`, `security-audit`, `release-manager`, `discussion-moderator`, `notification-inbox`, `pr-author`, `maintainer`) defined in `src/core/presets.ts` as tool name arrays, with matching system prompts in `src/agents.ts`. Composable via arrays. +## eve extension durable callbacks (`packages/github-tools-eve-extension`) + +On eve 0.44+, a missing durable descriptor on **any** dynamic-tool callback (`execute`, `toModelOutput`, `approval` / `approvalRequest`) discards the **entire** GitHub toolset. In `extension/tools/github.ts`, those three must be **direct** `defineTool` properties with inline functions (or identifiers). Conditional spreads and call expressions (`resolveEveApproval(...)`, `always()`) are invisible to eve's stamp. Callbacks may only close over a serializable tool `name` and re-read config via `buildSessionOptions()`. CI enforces this via `test/durable-define-tool.test.ts`. + ## Chat App Architecture (`apps/chat`) - **Frontend**: Vue 3 + `@nuxt/ui`. Chat pages at `app/pages/chat/[id].vue` with tool invocation rendering and approval UI. @@ -166,7 +172,7 @@ This applies to every `pnpm add`/`pnpm install` in the docs site, including peer A task is complete when **all** of the following pass: -1. `pnpm build`, `pnpm lint`, `pnpm typecheck` exit 0 +1. `pnpm build`, `pnpm lint`, `pnpm typecheck`, and `pnpm test` exit 0 2. New tools follow the full checklist in [`.github/CONTRIBUTING.md`](.github/CONTRIBUTING.md#adding-a-new-tool) 3. A changeset is included for any user-facing change (`pnpm changeset`) 4. New public APIs have JSDoc @@ -175,7 +181,7 @@ A task is complete when **all** of the following pass: ## Boundaries **Always do:** -- Run `pnpm build`, `pnpm lint`, and `pnpm typecheck` before reporting done +- Run `pnpm build`, `pnpm lint`, `pnpm typecheck`, and `pnpm test` before reporting done - Follow existing code patterns — read neighboring files (and the matching `core/`/`tools/` pair) before writing new ones - Add a changeset (`pnpm changeset`) for every user-facing change @@ -186,7 +192,7 @@ A task is complete when **all** of the following pass: **Never:** - Commit secrets, `.env` files, `GITHUB_TOKEN`, or API keys -- Skip lint or typecheck to "fix later" +- Skip lint, typecheck, or test to "fix later" - Widen a type (`as any`) or drop a `.describe()` to silence an error — fix the underlying issue - Return a raw, unshaped API response from a tool - Modify `node_modules/` or generated files (`dist/`, `.nuxt/`, `.output/`) diff --git a/apps/docs/content/docs/2.frameworks/1.eve-extension.md b/apps/docs/content/docs/2.frameworks/1.eve-extension.md index a694c07..1736a00 100644 --- a/apps/docs/content/docs/2.frameworks/1.eve-extension.md +++ b/apps/docs/content/docs/2.frameworks/1.eve-extension.md @@ -208,7 +208,7 @@ export default githubExtension({ ## Durable multi-turn sessions -The extension registers each tool with an **authored inline** `execute` and `toModelOutput` as **direct** `defineTool` properties that only close over a serializable tool `name`, then rebuilds session options from the extension config on every call via `@github-tools/sdk/eve-runtime`. Tools resolve on `step.started` so registration stays fresh across durable steps. That pattern survives multi-turn eve Workflow replay (see [#51](https://github.com/vercel-labs/github-tools/issues/51), [#99](https://github.com/vercel-labs/github-tools/issues/99)). A spread or library-function `toModelOutput` has no durable descriptor: on eve 0.44+ the resolver discards the **entire** GitHub toolset. Prefer this mount over the deprecated [`createGithubTools`](/deprecated/eve) / [`connectGithubTools`](/deprecated/eve) paths for Slack / multi-turn durable agents — those register tools from inside `node_modules` and are skipped on replay. Author `overrides.toModelOutput` inline in the agent; a function imported from a library will not get a durable descriptor. +The extension registers each tool with an **authored inline** `execute`, `toModelOutput`, and `approval` as **direct** `defineTool` properties that only close over a serializable tool `name`, then rebuilds session options from the extension config on every call via `@github-tools/sdk/eve-runtime`. Tools resolve on `step.started` so registration stays fresh across durable steps. That pattern survives multi-turn eve Workflow replay (see [#51](https://github.com/vercel-labs/github-tools/issues/51), [#99](https://github.com/vercel-labs/github-tools/issues/99)). A spread or call-expression callback (`resolveEveApproval(...)`, a ternary-wrapped `toModelOutput`) has no durable descriptor: on eve 0.44+ the resolver discards the **entire** GitHub toolset. Prefer this mount over the deprecated [`createGithubTools`](/deprecated/eve) / [`connectGithubTools`](/deprecated/eve) paths for Slack / multi-turn durable agents — those register tools from inside `node_modules` and are skipped on replay. Author `overrides.toModelOutput` inline in the agent; a function imported from a library will not get a durable descriptor. Object-shaped execute results include `rateLimit` (`remaining`, `limit`, `reset`, `resource`). `toModelOutput` strips it so the model never sees the remaining count; `toolResultFrom` and channels still do. See [Rate-limit metadata](/api/reference#rate-limit-metadata). diff --git a/apps/docs/skills/github-tools-agents/references/eve-extension.md b/apps/docs/skills/github-tools-agents/references/eve-extension.md index 3d596af..aefbb0c 100644 --- a/apps/docs/skills/github-tools-agents/references/eve-extension.md +++ b/apps/docs/skills/github-tools-agents/references/eve-extension.md @@ -55,7 +55,7 @@ export default githubExtension({ }) ``` -Built-in `toModelOutput` formatters are a direct `defineTool` property whose callback only closes over the tool name (a spread ternary is not stamped). That callback also strips `rateLimit` from the model-facing payload. Author `overrides.toModelOutput` inline in the agent — a library function will not get a durable descriptor on eve 0.44+, and the resolver then drops every `github__*` tool. Execute failures return `{ error }` so the model still receives a `tool_result`. Requires `eve` `>=0.44`. +`execute`, `toModelOutput`, and `approval` are direct `defineTool` properties whose callbacks only close over the tool name (a spread or `resolveEveApproval(...)` call is not stamped). `toModelOutput` also strips `rateLimit` from the model-facing payload. Author `overrides.toModelOutput` inline in the agent — a library function will not get a durable descriptor on eve 0.44+, and the resolver then drops every `github__*` tool. Execute failures return `{ error }` so the model still receives a `tool_result`. Requires `eve` `>=0.44`. ## Approval diff --git a/package.json b/package.json index eb4fd99..c56d28c 100644 --- a/package.json +++ b/package.json @@ -15,6 +15,7 @@ "lint": "turbo run lint", "lint:fix": "turbo run lint:fix", "typecheck": "turbo run typecheck", + "test": "turbo run test", "workflow": "pnpm --filter @github-tools/chat exec workflow web", "changeset": "changeset", "version-packages": "changeset version", diff --git a/packages/github-tools-eve-extension/README.md b/packages/github-tools-eve-extension/README.md index 8a1d185..3138b58 100644 --- a/packages/github-tools-eve-extension/README.md +++ b/packages/github-tools-eve-extension/README.md @@ -43,7 +43,7 @@ export default githubExtension({ > `code-review` pairs cleanly with a Connect `connector`. `maintainer` and `repo-explorer` include gist tools, and GitHub only grants gist access to user access tokens, never the installation tokens Connect mints, so gist calls 403 over Connect. Write tools already require approval via `always()` by default, so a plain `{ someTool: true }` is a no-op, use a predicate (as above) when you actually want to narrow or loosen the default. -Tools are registered with **inline** `execute` and `toModelOutput` as direct `defineTool` properties so they survive multi-turn durable eve Workflow replay (see [#51](https://github.com/vercel-labs/github-tools/issues/51), [#99](https://github.com/vercel-labs/github-tools/issues/99)). A spread or imported `toModelOutput` has no durable descriptor: on eve 0.44+ the resolver discards every GitHub tool. `toModelOutput` strips `rateLimit` from the model-facing payload; the execute result still carries it for hooks and channels. Do not use the deprecated `@github-tools/sdk/connect/eve` one-liner for durable Slack/multi-turn agents. +Tools are registered with **inline** `execute`, `toModelOutput`, and `approval` as direct `defineTool` properties so they survive multi-turn durable eve Workflow replay (see [#51](https://github.com/vercel-labs/github-tools/issues/51), [#99](https://github.com/vercel-labs/github-tools/issues/99)). A spread or call-expression callback has no durable descriptor: on eve 0.44+ the resolver discards every GitHub tool. `toModelOutput` strips `rateLimit` from the model-facing payload; the execute result still carries it for hooks and channels. Do not use the deprecated `@github-tools/sdk/connect/eve` one-liner for durable Slack/multi-turn agents. `connector` also accepts a `() => string | Promise` resolver, so the same config can pick a connector dynamically (e.g. by environment): diff --git a/packages/github-tools-eve-extension/extension/tools/github.ts b/packages/github-tools-eve-extension/extension/tools/github.ts index 43db601..aadcaea 100644 --- a/packages/github-tools-eve-extension/extension/tools/github.ts +++ b/packages/github-tools-eve-extension/extension/tools/github.ts @@ -2,6 +2,7 @@ import { connectGithubToken } from '@github-tools/sdk/connect' import { executeGithubEveTool, formatGithubEveToolOutput, + GITHUB_WRITE_TOOLS, isEveApprovalDisabled, listEveToolDescriptors, mapEveApprovalValue, @@ -13,14 +14,16 @@ import { type GithubToolName, type GithubWriteToolName, } from '@github-tools/sdk/eve-runtime' +import type { ApprovalContext } from 'eve/tools/approval' import { defineDynamic, defineTool, type ToolDefinition } from 'eve/tools' import extension from '../extension' /** * Rebuild options from extension config on every call. - * Durable `execute` / `toModelOutput` only close over a serializable tool `name` - * (#51, #99). `toModelOutput` must be a direct `defineTool` property — a spread - * ternary is invisible to eve's stamp, and 0.44+ then drops the whole toolset. + * Durable `execute` / `toModelOutput` / `approval` only close over a serializable + * tool `name` (#51, #99). Those three must be direct `defineTool` properties — + * a spread or call expression is invisible to eve's stamp, and 0.44+ then + * drops the whole toolset. */ function buildSessionOptions(): EveGithubToolsOptions { const { @@ -78,10 +81,36 @@ function approvalDisabled( return false } +function writeToolName(name: GithubToolName): GithubWriteToolName | undefined { + if (!Object.hasOwn(GITHUB_WRITE_TOOLS, name)) return undefined + return GITHUB_WRITE_TOOLS[name as keyof typeof GITHUB_WRITE_TOOLS] +} + async function runGithubEveTool(name: GithubToolName, input: unknown) { return executeGithubEveTool(name, input as Record, buildSessionOptions()) } +function runGithubEveToModelOutput(name: GithubToolName, output: unknown) { + const custom = buildSessionOptions().overrides?.[name]?.toModelOutput + return custom ? custom(output) : formatGithubEveToolOutput(name, output) +} + +function runGithubEveApproval(name: GithubToolName, ctx: ApprovalContext) { + const sessionOptions = buildSessionOptions() + const override = sessionOptions.overrides?.[name]?.approval + const writeTool = writeToolName(name) + + if (!writeTool || approvalDisabled(writeTool, sessionOptions.requireApproval, override)) { + return 'not-applicable' + } + + const policy = override !== undefined + ? mapEveApprovalValue(override) + : resolveEveApproval(writeTool, sessionOptions.requireApproval) + + return policy(ctx) +} + export default defineDynamic({ events: { // Re-resolve each model step (not once per session) so tool registration @@ -95,25 +124,12 @@ export default defineDynamic({ for (const entry of descriptors) { const name = entry.name const override = toolOverrides?.[name] - const skipApproval = approvalDisabled( - entry.writeTool, - sessionOptions.requireApproval, - override?.approval, - ) tools[name] = defineTool({ description: override?.description ?? entry.description, inputSchema: entry.inputSchema, - ...(entry.writeTool && !skipApproval && { - approval: resolveEveApproval(entry.writeTool, sessionOptions.requireApproval), - }), - ...(override?.approval !== undefined && !skipApproval && { - approval: mapEveApprovalValue(override.approval), - }), - toModelOutput: (output: unknown) => { - const custom = buildSessionOptions().overrides?.[name]?.toModelOutput - return custom ? custom(output) : formatGithubEveToolOutput(name, output) - }, + approval: (ctx) => runGithubEveApproval(name, ctx), + toModelOutput: (output: unknown) => runGithubEveToModelOutput(name, output), ...(override?.outputSchema !== undefined && { outputSchema: override.outputSchema, }), diff --git a/packages/github-tools-eve-extension/package.json b/packages/github-tools-eve-extension/package.json index 612a0d0..3de480e 100644 --- a/packages/github-tools-eve-extension/package.json +++ b/packages/github-tools-eve-extension/package.json @@ -39,7 +39,8 @@ "build": "eve extension build", "lint": "eslint .", "lint:fix": "eslint . --fix", - "typecheck": "tsc --noEmit" + "typecheck": "tsc --noEmit", + "test": "node --test --experimental-strip-types ./test/durable-define-tool.test.ts" }, "keywords": [ "ai", diff --git a/packages/github-tools-eve-extension/test/durable-define-tool.test.ts b/packages/github-tools-eve-extension/test/durable-define-tool.test.ts new file mode 100644 index 0000000..fa4ade7 --- /dev/null +++ b/packages/github-tools-eve-extension/test/durable-define-tool.test.ts @@ -0,0 +1,121 @@ +import assert from 'node:assert/strict' +import { readFileSync } from 'node:fs' +import { dirname, join } from 'node:path' +import { describe, it } from 'node:test' +import { fileURLToPath } from 'node:url' +import { + createSourceFile, + forEachChild, + isArrowFunction, + isCallExpression, + isFunctionExpression, + isIdentifier, + isMethodDeclaration, + isObjectLiteralExpression, + isPropertyAssignment, + isSpreadAssignment, + ScriptTarget, + type CallExpression, + type Expression, + type Node, + type ObjectLiteralExpression, +} from 'typescript' + +const CALLBACKS = ['execute', 'toModelOutput', 'approval'] as const +const sourcePath = join(dirname(fileURLToPath(import.meta.url)), '../extension/tools/github.ts') + +function walk(node: Node, visit: (node: Node) => void) { + visit(node) + forEachChild(node, child => walk(child, visit)) +} + +function propertyName(name: Node): string | undefined { + return isIdentifier(name) ? name.text : undefined +} + +function isDirectFunction(init: Expression | Node): boolean { + return isArrowFunction(init) || isFunctionExpression(init) || isIdentifier(init) || isMethodDeclaration(init) +} + +function collectDefineToolCalls(root: Node): CallExpression[] { + const calls: CallExpression[] = [] + walk(root, (node) => { + if (isCallExpression(node) && isIdentifier(node.expression) && node.expression.text === 'defineTool') { + calls.push(node) + } + }) + return calls +} + +function callbackKeysInSpread(spread: Expression, sourceText: (node: Node) => string): string[] { + const keys: string[] = [] + if (isObjectLiteralExpression(spread)) { + for (const inner of spread.properties) { + if (isPropertyAssignment(inner)) { + const key = propertyName(inner.name) + if (key && (CALLBACKS as readonly string[]).includes(key)) keys.push(key) + } + } + return keys + } + const text = sourceText(spread) + for (const key of CALLBACKS) { + if (new RegExp(`\\b${key}\\s*:`).test(text)) keys.push(key) + } + return keys +} + +function inspectDefineToolObject(arg: ObjectLiteralExpression, sourceText: (node: Node) => string) { + const spreadKeys: string[] = [] + const direct = new Map() + + for (const prop of arg.properties) { + if (isSpreadAssignment(prop)) { + spreadKeys.push(...callbackKeysInSpread(prop.expression, sourceText)) + continue + } + if (isPropertyAssignment(prop)) { + const key = propertyName(prop.name) + if (key) direct.set(key, prop.initializer) + continue + } + if (isMethodDeclaration(prop)) { + const key = propertyName(prop.name) + if (key) direct.set(key, prop) + } + } + + return { spreadKeys, direct } +} + +describe('defineTool durable callbacks', () => { + it('registers execute, toModelOutput, and approval as direct inline functions', () => { + const text = readFileSync(sourcePath, 'utf8') + const source = createSourceFile(sourcePath, text, ScriptTarget.Latest, true) + const calls = collectDefineToolCalls(source) + assert.ok(calls.length > 0, 'expected at least one defineTool() call in github.ts') + + const sourceText = (node: Node) => node.getText(source) + + for (const call of calls) { + const arg = call.arguments[0] + assert.ok(arg && isObjectLiteralExpression(arg), 'defineTool() must take an object literal') + + const { spreadKeys, direct } = inspectDefineToolObject(arg, sourceText) + assert.deepEqual( + spreadKeys, + [], + `defineTool() must not spread ${spreadKeys.join(', ')} — eve cannot stamp a durable descriptor`, + ) + + for (const key of CALLBACKS) { + const init = direct.get(key) + assert.ok(init, `defineTool() must set ${key} as a direct property`) + assert.ok( + isDirectFunction(init), + `defineTool().${key} must be an inline function or identifier, not ${init.kind}`, + ) + } + } + }) +}) diff --git a/turbo.json b/turbo.json index 04eb2c5..d67847d 100644 --- a/turbo.json +++ b/turbo.json @@ -34,6 +34,9 @@ }, "typecheck": { "dependsOn": ["^build"] + }, + "test": { + "dependsOn": ["^build"] } } }