From f2ed0b65c4a033c6d2eff48dd3029de655ec4425 Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Sat, 26 Sep 2026 21:16:33 -0500 Subject: [PATCH] fix(runner): enable API tools by default (#14186) ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - The native runner exposes tools for company tasks. > - API search and call tools cover operations without a dedicated tool. > - The current default hides these tools unless an operator sets an environment variable. > - This pull request enables the tools when that variable is absent. > - Operators can still disable the tools or restrict them to selected companies. ## Linked Issues or Issue Description Refs #13003, which added the guarded API tools. **What happened?** The native runner does not advertise `search_api` or `call_api` with the default server configuration. **Expected behavior** The tools are available without a special environment variable. Existing authorization checks still apply. **Steps to reproduce** Remove `PAPERCLIP_RUNNER_API_TOOLS_ENABLED` and `PAPERCLIP_RUNNER_API_TOOLS_COMPANY_IDS`. Create a normal runner authority. Inspect its tool definitions. **Paperclip version or commit** a6c4e7a. ## What Changed - Enable API tools when the server flag is absent. - Keep explicit disable, invalid-value rejection, company restrictions, and binding restrictions. - Test default tool definitions and run HTTP integration tests without the enabling flag. - Update operator and hiring documentation. The existing shared gate also controls `hire_agent`. - Use the same policy in the E2E evidence summary so an unset flag is not reported as disabled. - Isolate default-availability tests from operator environment variables. ## Verification - Red test: two new rollout policy assertions failed before the fix. - Focused policy, authority, and HTTP tests: 3 files passed; 41 tests passed, 2 skipped. The two runnerd transport cases require a Rust-built binary absent from this workspace. - Command: `pnpm exec vitest run server/src/services/native-runtime/runner-api-rollout.test.ts server/src/services/native-runtime/paperclip-runner-tool-authority.test.ts server/src/services/native-runtime/runner-api.integration.test.ts`. - Attempted `pnpm -r typecheck`: blocked by missing `cargo` in this workspace. - Attempted `pnpm build`: terminated at the 4 GiB memory limit. - Attempted `pnpm test:run`: stopped after memory pressure to run focused tests alone. - Repeated the 41 passing focused tests with an inherited disabled flag and a foreign-company restriction; test isolation passed. - `pnpm test:e2e:runner:unit`: 44 files and 543 tests passed. - `pnpm test:e2e:runner:typecheck` exceeded the workspace memory limit, including a retry with bounded Go memory settings. - CI results will be recorded before handoff. ## Risks - More native runs can discover API tools and the existing `hire_agent` tool by default. - The change does not remove company, run, mode, credential, lifecycle, or approval checks. - Explicit operator restrictions still take precedence. No database migration is required. ## Model Used OpenAI Codex agent. The runtime does not expose the exact model ID or context-window size. Used reasoning, repository tools, shell execution, and tests. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip --- doc/runner-api-tools.md | 33 ++++++++++--------- .../paperclip-runner-tool-authority.test.ts | 30 ++++++++++++----- .../native-runtime/runner-api-rollout.test.ts | 15 ++++++--- .../native-runtime/runner-api-rollout.ts | 4 +-- .../runner-api.integration.test.ts | 17 +++++----- tests/hiring-ai-connections/README.md | 8 ++--- tests/runner-e2e/README.md | 3 +- tests/runner-e2e/everyday-flow.ts | 4 +-- tests/runner-e2e/harness-env.ts | 4 +-- 9 files changed, 69 insertions(+), 49 deletions(-) diff --git a/doc/runner-api-tools.md b/doc/runner-api-tools.md index 55b1933927..f095e14783 100644 --- a/doc/runner-api-tools.md +++ b/doc/runner-api-tools.md @@ -6,25 +6,26 @@ agents do not have to search before using them. Only two tool definitions are advertised. The API catalog is returned on demand, never injected into the initial prompt. -## Controlled rollout +## Default availability and operator controls -The escape hatch is disabled by default. Set -`PAPERCLIP_RUNNER_API_TOOLS_ENABLED=true` on the server to enable it. For an -initial company rollout, also set `PAPERCLIP_RUNNER_API_TOOLS_COMPANY_IDS` to a -comma-separated list of company UUIDs. An unset list allows every company; -an explicitly empty list allows none. IDs must match exactly. +The escape hatch is enabled by default. No environment variable is required. +The native `hire_agent` tool shares this availability policy. +Set `PAPERCLIP_RUNNER_API_TOOLS_ENABLED=false` on the server to disable these +tools. An explicit `true` also enables them; other explicit values fail closed. -The server always requires the explicit `true` flag, including for server-owned -bindings. A binding can disable these tools for a baseline eval but cannot enable -them without operator opt-in. Setting the flag to `false` disables them. The server checks this switch when advertising tools, -when accepting a call, and immediately before HTTP dispatch after preparing any -files. Existing dedicated tools remain available. Operators must update the -environment of each server process and restart it for deployment-level changes; -this environment switch is not a live settings API. +Operators can restrict availability with `PAPERCLIP_RUNNER_API_TOOLS_COMPANY_IDS`, +a comma-separated list of company UUIDs. An unset list allows every company; +an explicitly empty list allows none. IDs must match exactly. This restriction +also applies when the enabled flag is unset. -Evaluate selected companies first. Compare success, unnecessary fallback calls, -cost, and latency against the dedicated-tool baseline before widening. Keep the -switch disabled if authorization, replay, or cost accounting fails. +A server-owned binding can disable these tools for a baseline eval but cannot +override an operator restriction. The server checks the policy when advertising +tools, when accepting a call, and immediately before HTTP dispatch after +preparing any files. Existing dedicated tools remain available. Operators must +update the environment of each server process and restart it for deployment-level +changes; this environment switch is not a live settings API. + +All company, run, work-mode, credential, and lifecycle checks below still apply. ## Discovery and requests diff --git a/server/src/services/native-runtime/paperclip-runner-tool-authority.test.ts b/server/src/services/native-runtime/paperclip-runner-tool-authority.test.ts index 1dc5d9de19..307a6f6bbc 100644 --- a/server/src/services/native-runtime/paperclip-runner-tool-authority.test.ts +++ b/server/src/services/native-runtime/paperclip-runner-tool-authority.test.ts @@ -1,5 +1,5 @@ import * as cloudIdentity from "../cloud-runtime-identity.js"; -import { afterAll, beforeAll, describe, expect, it, vi } from "vitest"; +import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; import { randomUUID } from "node:crypto"; import { and, eq, inArray } from "drizzle-orm"; import { @@ -33,6 +33,12 @@ describe("PaperclipRunnerToolAuthority", () => { const issueId = "00000000-0000-4000-8000-000000000103"; const runId = "00000000-0000-4000-8000-000000000104"; + beforeEach(() => { + vi.stubEnv("PAPERCLIP_RUNNER_API_TOOLS_ENABLED", undefined); + vi.stubEnv("PAPERCLIP_RUNNER_API_TOOLS_COMPANY_IDS", undefined); + }); + afterEach(() => vi.unstubAllEnvs()); + beforeAll(async () => { temporary = await startEmbeddedPostgresTestDatabase( "paperclip-runner-tools-", @@ -91,7 +97,7 @@ describe("PaperclipRunnerToolAuthority", () => { issueId, runId, }); - expect(authority.definitions()).toHaveLength(27); + expect(authority.definitions()).toHaveLength(30); const questions = authority.definitions().find(tool => tool.name === "request_human_input")!; expect(questions.description).toContain("ask only the next unanswered question"); expect(questions.description).toContain("Never infer answers"); @@ -103,6 +109,7 @@ describe("PaperclipRunnerToolAuthority", () => { expect.arrayContaining([ "connections_search", "connection_request", "create_project", "list_project_repositories", "list_projects", + "search_api", "call_api", "hire_agent", "get_task_context", "get_task_history", "search_tasks", @@ -216,7 +223,7 @@ describe("PaperclipRunnerToolAuthority", () => { } }); - it("preserves direct-chat file tools across the guarded API rollout", () => { + it("advertises API tools by default and preserves direct-chat file tools when disabled", () => { const previousEnabled = process.env.PAPERCLIP_RUNNER_API_TOOLS_ENABLED; const previousCompanies = process.env.PAPERCLIP_RUNNER_API_TOOLS_COMPANY_IDS; @@ -240,15 +247,15 @@ describe("PaperclipRunnerToolAuthority", () => { try { delete process.env.PAPERCLIP_RUNNER_API_TOOLS_ENABLED; delete process.env.PAPERCLIP_RUNNER_API_TOOLS_COMPANY_IDS; - const disabledNames = createAuthority() + const defaultNames = createAuthority() .definitions() .map((tool) => tool.name); - expect(disabledNames).toEqual( + expect(defaultNames).toEqual( expect.arrayContaining(requiredChatFileTools), ); - expect(disabledNames).not.toContain("search_api"); - expect(disabledNames).not.toContain("call_api"); - expect(disabledNames).not.toContain("hire_agent"); + expect(defaultNames).toContain("search_api"); + expect(defaultNames).toContain("call_api"); + expect(defaultNames).toContain("hire_agent"); process.env.PAPERCLIP_RUNNER_API_TOOLS_ENABLED = "true"; process.env.PAPERCLIP_RUNNER_API_TOOLS_COMPANY_IDS = companyId; @@ -268,6 +275,13 @@ describe("PaperclipRunnerToolAuthority", () => { expect(hireSchema.properties).toEqual(expect.objectContaining({ name: expect.any(Object), role: expect.any(Object) })); expect(hireSchema.properties).not.toHaveProperty("adapterConfig"); expect(hireSchema.properties).not.toHaveProperty("env"); + + process.env.PAPERCLIP_RUNNER_API_TOOLS_ENABLED = "false"; + const disabledNames = createAuthority().definitions().map((tool) => tool.name); + expect(disabledNames).toEqual(expect.arrayContaining(requiredChatFileTools)); + for (const name of ["search_api", "call_api", "hire_agent"]) { + expect(disabledNames).not.toContain(name); + } } finally { if (previousEnabled === undefined) { delete process.env.PAPERCLIP_RUNNER_API_TOOLS_ENABLED; diff --git a/server/src/services/native-runtime/runner-api-rollout.test.ts b/server/src/services/native-runtime/runner-api-rollout.test.ts index 8372cd9fa8..8a9a119b27 100644 --- a/server/src/services/native-runtime/runner-api-rollout.test.ts +++ b/server/src/services/native-runtime/runner-api-rollout.test.ts @@ -2,14 +2,19 @@ import { describe, expect, it } from "vitest"; import { runnerApiToolsEnabled } from "./runner-api-rollout.js"; describe("runner API rollout", () => { - it("requires operator opt-in and fails closed for invalid settings", () => { - for (const value of [undefined, "", "TRUE", "1", "invalid"]) { + it("enables API tools without operator configuration", () => { + expect(runnerApiToolsEnabled("company", undefined, {})).toBe(true); + expect(runnerApiToolsEnabled("company", true, {})).toBe(true); + expect(runnerApiToolsEnabled("company", false, {})).toBe(false); + }); + it("fails closed for explicit disabled or invalid settings", () => { + for (const value of ["false", "", "TRUE", "1", "invalid"]) { for (const binding of [undefined, false, true]) expect(runnerApiToolsEnabled("company", binding, { PAPERCLIP_RUNNER_API_TOOLS_ENABLED: value })).toBe(false); } expect(runnerApiToolsEnabled("company", undefined, { PAPERCLIP_RUNNER_API_TOOLS_ENABLED: "true" })).toBe(true); }); - it("restricts an enabled rollout to exact company IDs", () => { - const env = { PAPERCLIP_RUNNER_API_TOOLS_ENABLED: "true", PAPERCLIP_RUNNER_API_TOOLS_COMPANY_IDS: " alpha, beta " }; + it.each([undefined, "true"])("restricts enabled tools to exact company IDs (flag: %s)", (enabled) => { + const env = { PAPERCLIP_RUNNER_API_TOOLS_ENABLED: enabled, PAPERCLIP_RUNNER_API_TOOLS_COMPANY_IDS: " alpha, beta " }; expect(runnerApiToolsEnabled("alpha", undefined, env)).toBe(true); expect(runnerApiToolsEnabled("alph", undefined, env)).toBe(false); expect(runnerApiToolsEnabled("foreign", true, env)).toBe(false); @@ -17,7 +22,7 @@ describe("runner API rollout", () => { }); it("keeps baseline tools disabled and honors an operator stop over explicit bindings", () => { expect(runnerApiToolsEnabled("company", false, { PAPERCLIP_RUNNER_API_TOOLS_ENABLED: "true" })).toBe(false); - expect(runnerApiToolsEnabled("company", true, {})).toBe(false); + expect(runnerApiToolsEnabled("company", false, {})).toBe(false); expect(runnerApiToolsEnabled("company", true, { PAPERCLIP_RUNNER_API_TOOLS_ENABLED: "false" })).toBe(false); }); }); diff --git a/server/src/services/native-runtime/runner-api-rollout.ts b/server/src/services/native-runtime/runner-api-rollout.ts index 8d3fd94d76..66b5fdb145 100644 --- a/server/src/services/native-runtime/runner-api-rollout.ts +++ b/server/src/services/native-runtime/runner-api-rollout.ts @@ -5,8 +5,8 @@ export function runnerApiToolsEnabled( environment: NodeJS.ProcessEnv = process.env, ): boolean { const enabled = environment.PAPERCLIP_RUNNER_API_TOOLS_ENABLED; - // A binding can only narrow operator permission, never opt into this surface. - if (enabled !== "true" || bindingOverride === false) return false; + // Enabled by default. Explicit settings fail closed; bindings can only narrow access. + if ((enabled !== undefined && enabled !== "true") || bindingOverride === false) return false; const companies = environment.PAPERCLIP_RUNNER_API_TOOLS_COMPANY_IDS; if (companies === undefined) return true; return companies.split(",").map(value => value.trim()).filter(Boolean).includes(companyId); diff --git a/server/src/services/native-runtime/runner-api.integration.test.ts b/server/src/services/native-runtime/runner-api.integration.test.ts index 3f19b9397f..805a618c6b 100644 --- a/server/src/services/native-runtime/runner-api.integration.test.ts +++ b/server/src/services/native-runtime/runner-api.integration.test.ts @@ -4,7 +4,7 @@ import { chmod, writeFile, symlink } from "node:fs/promises"; import { join } from "node:path"; import { eq } from "drizzle-orm"; import { documents, heartbeatRuns, issues, routineDocuments, routines } from "@paperclipai/db"; -import { beforeAll, afterAll, describe, expect, it, vi } from "vitest"; +import { beforeAll, afterAll, beforeEach, afterEach, describe, expect, it, vi } from "vitest"; import { startRunnerApiTestServer } from "../../__tests__/helpers/runner-api-server.js"; import { createRunnerdCodexTransport, defaultCapabilityRunnerdBinary } from "../../vendor/paperclip-runner/index.js"; import { runnerApiCatalog } from "./runner-api-catalog.js"; @@ -13,18 +13,19 @@ import { registerRunnerPrpAuthority } from "../../realtime/runner-prp-ws.js"; describe("runner API against real HTTP routes", () => { let server: Awaited>; const oldSecret = process.env.PAPERCLIP_AGENT_JWT_SECRET; - const oldEnabled = process.env.PAPERCLIP_RUNNER_API_TOOLS_ENABLED; + beforeEach(() => { + vi.stubEnv("PAPERCLIP_RUNNER_API_TOOLS_ENABLED", undefined); + vi.stubEnv("PAPERCLIP_RUNNER_API_TOOLS_COMPANY_IDS", undefined); + }); + afterEach(() => vi.unstubAllEnvs()); beforeAll(async () => { process.env.PAPERCLIP_AGENT_JWT_SECRET = randomUUID(); - process.env.PAPERCLIP_RUNNER_API_TOOLS_ENABLED = "true"; server = await startRunnerApiTestServer(); }, 60_000); afterAll(async () => { await server?.close(); if (oldSecret === undefined) delete process.env.PAPERCLIP_AGENT_JWT_SECRET; else process.env.PAPERCLIP_AGENT_JWT_SECRET = oldSecret; - if (oldEnabled === undefined) delete process.env.PAPERCLIP_RUNNER_API_TOOLS_ENABLED; - else process.env.PAPERCLIP_RUNNER_API_TOOLS_ENABLED = oldEnabled; }); it.skipIf(!process.env.PAPERCLIP_REQUIRE_RUNNER_API_INTEGRATION && !existsSync(defaultCapabilityRunnerdBinary())).each(["current", "legacy_http"])("runs runnerd → PRP → authority → actual authenticated HTTP (%s receipt)", async (receiptFormat) => { @@ -92,16 +93,16 @@ else if(m.id!==undefined) send({id:m.id,result:{}}); } finally { await bundle.transport.close(); } }, 30_000); - it("cannot opt into API tools through a binding when the operator flag is absent", async () => { + it("cannot opt into API tools through a binding when the operator flag is false", async () => { const fixture = await server.fixture({ apiToolsEnabled: true }); - delete process.env.PAPERCLIP_RUNNER_API_TOOLS_ENABLED; + process.env.PAPERCLIP_RUNNER_API_TOOLS_ENABLED = "false"; try { const names = (await fixture.authority.definitions()).map(tool => tool.name); expect(names).toContain("get_task_context"); expect(names).not.toContain("search_api"); expect(names).not.toContain("call_api"); await expect(fixture.authority.execute({ tool: "call_api", callId: "disabled", arguments: { operationId: "GET /api/companies/{companyId}/projects" } })).rejects.toThrow("not_advertised"); - } finally { process.env.PAPERCLIP_RUNNER_API_TOOLS_ENABLED = "true"; } + } finally { delete process.env.PAPERCLIP_RUNNER_API_TOOLS_ENABLED; } }); it("rejects credential calls before any durable receipt or secret result exists", async () => { diff --git a/tests/hiring-ai-connections/README.md b/tests/hiring-ai-connections/README.md index 748f672a3e..094b64146b 100644 --- a/tests/hiring-ai-connections/README.md +++ b/tests/hiring-ai-connections/README.md @@ -34,11 +34,9 @@ Provider calls use real credentials and incur usage. The fixture uses Claude Son not automated by this live suite. Integration tests cover subscription inheritance, credential serialization, and automatic retry after the parent's lease is released. -For representative coverage of the newer native runner, put -`PAPERCLIP_RUNNER_API_TOOLS_ENABLED=true` in the disposable data directory's -`instances/default/.env`, then restart test-drive. Test-drive clears inherited -`PAPERCLIP_*` variables before loading that file. The existing opt-in managed API -tools are how native agents hire. Then use: +The newer native runner enables its managed API and hiring tools by default. +No enabling environment variable is required. Operator restrictions still apply; +see [Runner API tools](../../doc/runner-api-tools.md). For native coverage, use: ```sh HIRING_AI_LIVE=1 HIRING_AI_RUNNER=native HIRING_AI_TEST_URL=http://127.0.0.1:3100 \ diff --git a/tests/runner-e2e/README.md b/tests/runner-e2e/README.md index 79c37c27b5..893e4b3a8a 100644 --- a/tests/runner-e2e/README.md +++ b/tests/runner-e2e/README.md @@ -201,7 +201,8 @@ write endpoint rejects a send without creating work, and re-enables the same conversation with its remembered context. The company, credential, and native agent are fixture-provisioned. This qualifies the experimental-settings path, not native first-run onboarding: the current production wizard offers legacy -adapters, and native API tools remain an independent opt-in. +adapters. Native API tools are enabled by default, subject to the +[operator controls](../../doc/runner-api-tools.md#default-availability-and-operator-controls). `followup-while-running` and `revise-while-running` send a second browser message while the provider runs a bounded command waiting for a fixture brief file. diff --git a/tests/runner-e2e/everyday-flow.ts b/tests/runner-e2e/everyday-flow.ts index 635bcd96a4..1fc1a11851 100644 --- a/tests/runner-e2e/everyday-flow.ts +++ b/tests/runner-e2e/everyday-flow.ts @@ -1,4 +1,5 @@ import { expect, type Page } from "@playwright/test"; +import { runnerApiToolsEnabled } from "../../server/src/services/native-runtime/runner-api-rollout.js"; import { spawn } from "node:child_process"; import { createHash } from "node:crypto"; import { mkdir, readFile, readdir } from "node:fs/promises"; @@ -131,8 +132,7 @@ export async function runEverydayFlow(input: Input) { caseId: execution.task.id, prompt: execution.task.buildPrompt(nonce), fixtureConfiguration: { - apiToolsEnabled: - process.env.PAPERCLIP_RUNNER_API_TOOLS_ENABLED === "true", + apiToolsEnabled: runnerApiToolsEnabled(fixtures.company.id), aiConnection: fixtures.aiConnection, }, checks: [], diff --git a/tests/runner-e2e/harness-env.ts b/tests/runner-e2e/harness-env.ts index 6539f905b8..aea1808633 100644 --- a/tests/runner-e2e/harness-env.ts +++ b/tests/runner-e2e/harness-env.ts @@ -108,8 +108,8 @@ export function buildRunnerE2EProcessEnvironment( // Announcements are unrelated to the scenarios and obscure screenshot evidence. result.PAPERCLIP_ANNOUNCEMENTS_ENABLED = "false"; delete result.OPENCODE_ALLOW_ALL_MODELS; - // Hiring needs the opt-in native API surface. Scope this to the explicit - // manual hiring story; production and other suites retain their defaults. + // These stories explicitly require the native API surface. Other suites + // retain the server default or any supplied operator restriction. if (executions.some((e) => isManagedHiringCase(e.suite.id, e.task.id) || chatNeedsApiTools(e.suite.id, e.task.id))) { result.PAPERCLIP_RUNNER_API_TOOLS_ENABLED = "true"; }