From 6885facdd177821a320024cc3490d8e200c42dee Mon Sep 17 00:00:00 2001 From: Les Orchard Date: Wed, 10 Jun 2026 14:20:12 -0700 Subject: [PATCH] feat: surface uploadable files to the agent When upload_allowed_paths names specific files, advertise those exact paths in the agent prompt so it can complete upload-gated forms instead of giving up. Also stop advertising upload_file when uploads are disabled. - resolveAdvertisedUploadFiles(): Node-only helper resolving allowlist entries that are existing regular files (directories/missing skipped); exported from index.ts only, never the browser-safe core.ts. - buildToolExamples gates the upload_file example on hasFileUpload and lists advertised file paths; threaded through buildActionLoopSystemPrompt and buildStepErrorFeedbackPrompt. - WebAgent stays browser-safe (no fs): CLI and server compute the file list and pass advertisedUploadFiles via WebAgentOptions. - Complete the pilo-core mocks in the server test suites (routes/pilo, routes/piloWs, sensitive-data-leak, taskRunner) with the new export so runTask's unconditional call resolves under the hand-written mocks. Builds on #494 (upload_file action); addresses the upload gap from #462. Co-Authored-By: Claude Opus 4.8 (1M context) --- packages/cli/src/commands/run.ts | 4 ++ packages/core/src/index.ts | 3 ++ packages/core/src/prompts.ts | 34 ++++++++++++-- packages/core/src/utils/uploadFiles.ts | 32 +++++++++++++ packages/core/src/webAgent.ts | 17 +++++++ packages/core/test/prompts.test.ts | 32 +++++++++++++ packages/core/test/utils/uploadFiles.test.ts | 45 +++++++++++++++++++ packages/server/src/routes/pilo.test.ts | 1 + packages/server/src/taskRunner.test.ts | 1 + packages/server/src/taskRunner.ts | 3 ++ .../server/test/sensitive-data-leak.test.ts | 1 + 11 files changed, 169 insertions(+), 4 deletions(-) create mode 100644 packages/core/src/utils/uploadFiles.ts create mode 100644 packages/core/test/utils/uploadFiles.test.ts diff --git a/packages/cli/src/commands/run.ts b/packages/cli/src/commands/run.ts index e430678a..1cbee13b 100644 --- a/packages/cli/src/commands/run.ts +++ b/packages/cli/src/commands/run.ts @@ -14,6 +14,7 @@ import { MetricsCollector, SecretsRedactor, PLAYWRIGHT_BROWSERS, + resolveAdvertisedUploadFiles, } from "pilo-core"; import type { FileUploadConfig, @@ -201,6 +202,8 @@ async function executeRunCommand(task: string, options: any): Promise { ? { allowedPaths: uploadAllowedPaths } : false; + const advertisedUploadFiles = await resolveAdvertisedUploadFiles(allowFileUpload); + // Create logger const loggerType = options.logger ?? cfg.logger; const metricsIncremental = options.metricsIncremental ?? cfg.metrics_incremental; @@ -345,6 +348,7 @@ async function executeRunCommand(task: string, options: any): Promise { trustedHostnames: options.trustedHostnames ?? cfg.trusted_hostnames, unsafeMode: options.unsafe ?? cfg.unsafe_mode, allowFileUpload, + advertisedUploadFiles, providerConfig, logger, eventEmitter, diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 4a08bdf5..317711b5 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -74,3 +74,6 @@ export type { ProviderConfig } from "./provider.js"; // Config merge utilities export { mergeWithDefaults, createNavigationRetryConfig } from "./utils/configMerge.js"; + +// Upload file utilities (Node-only) +export { resolveAdvertisedUploadFiles } from "./utils/uploadFiles.js"; diff --git a/packages/core/src/prompts.ts b/packages/core/src/prompts.ts index 0ef1f6fd..8fa0651f 100644 --- a/packages/core/src/prompts.ts +++ b/packages/core/src/prompts.ts @@ -174,11 +174,12 @@ function buildToolExamples( hasWebSearch: boolean, hasTabstack: boolean = false, hasInteractive: boolean = false, + hasFileUpload: boolean = false, + advertisedUploadFiles: string[] = [], ): string { const lines = [ `- click({"ref": "${TOOL_STRINGS.webActions.common.elementRefExample}"}) - ${TOOL_STRINGS.webActions.click.description}`, `- fill({"ref": "${TOOL_STRINGS.webActions.common.elementRefExample}", "value": "text"}) - ${TOOL_STRINGS.webActions.fill.description}`, - `- upload_file({"ref": "${TOOL_STRINGS.webActions.common.elementRefExample}", "path": "/path/to/file"}) - ${TOOL_STRINGS.webActions.uploadFile.description}`, `- select({"ref": "${TOOL_STRINGS.webActions.common.elementRefExample}", "value": "option"}) - ${TOOL_STRINGS.webActions.select.description}`, `- hover({"ref": "${TOOL_STRINGS.webActions.common.elementRefExample}"}) - ${TOOL_STRINGS.webActions.hover.description}`, `- check({"ref": "${TOOL_STRINGS.webActions.common.elementRefExample}"}) - ${TOOL_STRINGS.webActions.check.description}`, @@ -193,6 +194,15 @@ function buildToolExamples( `- extract({"description": "data to extract"}) - ${TOOL_STRINGS.webActions.extract.description}`, ]; + if (hasFileUpload) { + let uploadLine = `- upload_file({"ref": "${TOOL_STRINGS.webActions.common.elementRefExample}", "path": "/path/to/file"}) - ${TOOL_STRINGS.webActions.uploadFile.description}`; + if (advertisedUploadFiles.length > 0) { + uploadLine += `\n Files available to upload (use one of these exact paths as "path"): ${advertisedUploadFiles.join(", ")}`; + } + // Insert just after the fill() example (index 1) to preserve ordering. + lines.splice(2, 0, uploadLine); + } + if (hasWebSearch) { lines.push( `- webSearch({"query": "search terms"}) - ${TOOL_STRINGS.webActions.webSearch.description}`, @@ -431,13 +441,15 @@ ${toolCallInstruction} `.trim(), ); -/** Build action system prompt with optional guardrails, web search, Tabstack, and interactive tools. */ +/** Build action system prompt with optional guardrails, web search, Tabstack, interactive tools, and file upload. */ const buildActionLoopSystemPrompt = ( hasGuardrails: boolean, hasWebSearch: boolean = false, hasTabstack: boolean = false, hasStartingUrl: boolean = false, hasInteractive: boolean = false, + hasFileUpload: boolean = false, + advertisedUploadFiles: string[] = [], ) => actionLoopSystemPromptTemplate({ hasGuardrails, @@ -445,7 +457,13 @@ const buildActionLoopSystemPrompt = ( hasTabstack, hasStartingUrl, hasInteractive, - toolExamples: buildToolExamples(hasWebSearch, hasTabstack, hasInteractive), + toolExamples: buildToolExamples( + hasWebSearch, + hasTabstack, + hasInteractive, + hasFileUpload, + advertisedUploadFiles, + ), currentDate: getCurrentFormattedDate(), }); @@ -572,11 +590,19 @@ export const buildStepErrorFeedbackPrompt = ( hasGuardrails: boolean = false, hasWebSearch: boolean = false, hasTabstack: boolean = false, + hasFileUpload: boolean = false, + advertisedUploadFiles: string[] = [], ) => stepErrorFeedbackTemplate({ error, hasGuardrails, - toolExamples: buildToolExamples(hasWebSearch, hasTabstack), + toolExamples: buildToolExamples( + hasWebSearch, + hasTabstack, + false, + hasFileUpload, + advertisedUploadFiles, + ), }); /** diff --git a/packages/core/src/utils/uploadFiles.ts b/packages/core/src/utils/uploadFiles.ts new file mode 100644 index 00000000..a06804fc --- /dev/null +++ b/packages/core/src/utils/uploadFiles.ts @@ -0,0 +1,32 @@ +import * as fs from "node:fs/promises"; +import * as path from "node:path"; +import type { FileUploadConfig } from "../browser/ariaBrowser.js"; + +/** + * Resolve the subset of upload-allowlist entries that are existing regular files, + * as absolute paths. Directory entries and missing/unreadable paths are skipped. + * + * Used to advertise concrete uploadable file paths to the agent. Node-only + * (filesystem access): exported from `index.ts`, never from the browser-safe + * `core.ts` barrel. + */ +export async function resolveAdvertisedUploadFiles( + allowFileUpload: false | FileUploadConfig, +): Promise { + if (!allowFileUpload || allowFileUpload.allowedPaths.length === 0) { + return []; + } + const files: string[] = []; + for (const entry of allowFileUpload.allowedPaths) { + try { + const resolved = path.resolve(entry); + const stat = await fs.stat(resolved); + if (stat.isFile()) { + files.push(resolved); + } + } catch { + // Missing or unreadable entry — cannot be advertised. + } + } + return files; +} diff --git a/packages/core/src/webAgent.ts b/packages/core/src/webAgent.ts index 5185e639..2e281a62 100644 --- a/packages/core/src/webAgent.ts +++ b/packages/core/src/webAgent.ts @@ -139,6 +139,13 @@ export interface WebAgentOptions { * roots. */ allowFileUpload?: false | FileUploadConfig; + /** + * Concrete local file paths the agent may upload, surfaced in its prompt so it + * knows what is available. Derived from `allowFileUpload` by the caller + * (CLI/server) via `resolveAdvertisedUploadFiles`. Empty when uploads are + * disabled or the allowlist names only directories. + */ + advertisedUploadFiles?: string[]; /** Timeout for LLM provider calls in milliseconds (default: from config) */ llmProviderTimeoutMs?: number; /** @@ -296,6 +303,7 @@ export class WebAgent { private readonly taskId: string | undefined; private readonly firewall: FirewallConfig; private readonly allowFileUpload: false | FileUploadConfig; + private readonly advertisedUploadFiles: string[]; private readonly llmProviderTimeoutMs: number; private readonly iterationTimeoutMs: number; // Host of the caller-provided start URL (options.startingUrl), captured at @@ -341,6 +349,7 @@ export class WebAgent { unsafeMode: Boolean(options.unsafeMode), }); this.allowFileUpload = options.allowFileUpload ?? false; + this.advertisedUploadFiles = options.advertisedUploadFiles ?? []; this.llmProviderTimeoutMs = options.llmProviderTimeoutMs ?? defaults.llm_provider_timeout_ms; this.iterationTimeoutMs = options.iterationTimeoutMs ?? DEFAULT_ITERATION_TIMEOUT_MS; @@ -874,10 +883,16 @@ export class WebAgent { Boolean(this.guardrails), this.searchProvider !== "none", Boolean(this.tabstackApiKey), + this.hasFileUpload, + this.advertisedUploadFiles, ); this.messages.push({ role: "user", content: errorFeedback }); } + private get hasFileUpload(): boolean { + return this.allowFileUpload !== false && this.allowFileUpload.allowedPaths.length > 0; + } + /** Remove image parts from the most recent user message (fallback for AI SDK image crashes). */ private stripImagesFromLastMessage(): void { for (let i = this.messages.length - 1; i >= 0; i--) { @@ -1923,6 +1938,8 @@ export class WebAgent { hasTabstack, hasStartingUrl, hasInteractive, + this.hasFileUpload, + this.advertisedUploadFiles, ); this.messages = [ diff --git a/packages/core/test/prompts.test.ts b/packages/core/test/prompts.test.ts index 458a37c5..6b7dad4f 100644 --- a/packages/core/test/prompts.test.ts +++ b/packages/core/test/prompts.test.ts @@ -753,6 +753,38 @@ describe("prompts", () => { }); }); + describe("upload_file tool surfacing", () => { + it("omits upload_file when uploads are disabled", () => { + const prompt = buildActionLoopSystemPrompt(false, false, false, false, false, false, []); + expect(prompt).not.toContain("upload_file("); + }); + + it("shows upload_file when enabled, without a files note if none advertised", () => { + const prompt = buildActionLoopSystemPrompt(false, false, false, false, false, true, []); + expect(prompt).toContain("upload_file("); + expect(prompt).not.toContain("Files available to upload"); + }); + + it("lists advertised files when present", () => { + const prompt = buildActionLoopSystemPrompt(false, false, false, false, false, true, [ + "/fixtures/sample.txt", + ]); + expect(prompt).toContain("upload_file("); + expect(prompt).toContain("Files available to upload"); + expect(prompt).toContain("/fixtures/sample.txt"); + }); + + it("error-feedback prompt also gates upload_file on enablement", () => { + const disabled = buildStepErrorFeedbackPrompt("boom", false, false, false, false, []); + expect(disabled).not.toContain("upload_file("); + const enabled = buildStepErrorFeedbackPrompt("boom", false, false, false, true, [ + "/fixtures/sample.txt", + ]); + expect(enabled).toContain("upload_file("); + expect(enabled).toContain("/fixtures/sample.txt"); + }); + }); + describe("Template consistency", () => { it("should use consistent prompt style across functions", () => { const prompts = [ diff --git a/packages/core/test/utils/uploadFiles.test.ts b/packages/core/test/utils/uploadFiles.test.ts new file mode 100644 index 00000000..be198ea7 --- /dev/null +++ b/packages/core/test/utils/uploadFiles.test.ts @@ -0,0 +1,45 @@ +import { describe, it, expect, beforeAll, afterAll } from "vitest"; +import * as fs from "node:fs/promises"; +import * as os from "node:os"; +import * as path from "node:path"; +import { resolveAdvertisedUploadFiles } from "../../src/utils/uploadFiles.js"; + +describe("resolveAdvertisedUploadFiles", () => { + let dir: string; + let filePath: string; + + beforeAll(async () => { + dir = await fs.mkdtemp(path.join(os.tmpdir(), "pilo-upload-")); + filePath = path.join(dir, "sample.txt"); + await fs.writeFile(filePath, "x"); + }); + + afterAll(async () => { + await fs.rm(dir, { recursive: true, force: true }); + }); + + it("returns [] when uploads are disabled", async () => { + expect(await resolveAdvertisedUploadFiles(false)).toEqual([]); + }); + + it("returns [] when the allowlist is empty", async () => { + expect(await resolveAdvertisedUploadFiles({ allowedPaths: [] })).toEqual([]); + }); + + it("includes file entries, resolved to absolute paths", async () => { + const result = await resolveAdvertisedUploadFiles({ allowedPaths: [filePath] }); + expect(result).toEqual([path.resolve(filePath)]); + }); + + it("excludes directory entries", async () => { + const result = await resolveAdvertisedUploadFiles({ allowedPaths: [dir] }); + expect(result).toEqual([]); + }); + + it("skips missing or unreadable entries", async () => { + const result = await resolveAdvertisedUploadFiles({ + allowedPaths: [path.join(dir, "does-not-exist.txt"), filePath], + }); + expect(result).toEqual([path.resolve(filePath)]); + }); +}); diff --git a/packages/server/src/routes/pilo.test.ts b/packages/server/src/routes/pilo.test.ts index e635e601..24c9b32b 100644 --- a/packages/server/src/routes/pilo.test.ts +++ b/packages/server/src/routes/pilo.test.ts @@ -47,6 +47,7 @@ vi.mock("pilo-core", () => ({ DEFAULT_VISION: false, DEFAULT_MAX_ITERATIONS: 50, DEFAULT_MAX_VALIDATION_ATTEMPTS: 3, + resolveAdvertisedUploadFiles: vi.fn().mockResolvedValue([]), })); // Mock the AI SDK diff --git a/packages/server/src/taskRunner.test.ts b/packages/server/src/taskRunner.test.ts index f2bdb524..44e811c9 100644 --- a/packages/server/src/taskRunner.test.ts +++ b/packages/server/src/taskRunner.test.ts @@ -51,6 +51,7 @@ vi.mock("pilo-core", () => { })), SEARCH_PROVIDERS: ["none", "duckduckgo", "google", "bing", "parallel-api"], PLAYWRIGHT_BROWSERS: ["firefox", "chrome", "chromium", "safari", "webkit", "edge"], + resolveAdvertisedUploadFiles: vi.fn().mockResolvedValue([]), }; }); diff --git a/packages/server/src/taskRunner.ts b/packages/server/src/taskRunner.ts index ab2faf94..7c845b3a 100644 --- a/packages/server/src/taskRunner.ts +++ b/packages/server/src/taskRunner.ts @@ -11,6 +11,7 @@ import { RecoverableError, SEARCH_PROVIDERS, PLAYWRIGHT_BROWSERS, + resolveAdvertisedUploadFiles, } from "pilo-core"; import type { FileUploadConfig, TaskExecutionResult, UserDataCallback } from "pilo-core"; import * as path from "node:path"; @@ -323,6 +324,7 @@ export async function runTask(options: TaskRunnerOptions): Promise 0 ? { allowedPaths: uploadAllowedPaths } : false; + const advertisedUploadFiles = await resolveAdvertisedUploadFiles(allowFileUpload); const browserConfig = { browser: browserName as (typeof PLAYWRIGHT_BROWSERS)[number], @@ -368,6 +370,7 @@ export async function runTask(options: TaskRunnerOptions): Promise { SEARCH_PROVIDERS: ["none", "duckduckgo", "google", "bing", "parallel-api"], PLAYWRIGHT_BROWSERS: ["firefox", "chrome", "chromium", "safari", "webkit", "edge"], withRemoteContext: vi.fn((_headers: unknown, fn: () => unknown) => fn()), + resolveAdvertisedUploadFiles: vi.fn().mockResolvedValue([]), }; });