diff --git a/src/tools/agent.ts b/src/tools/agent.ts index a39b500..c5f69c2 100644 --- a/src/tools/agent.ts +++ b/src/tools/agent.ts @@ -679,12 +679,12 @@ export function registerAgentTools( // The advertised tool schema flattens `commands` so OpenAI's hosted-MCP // import accepts it; re-validate a provided batch against the full // per-command contract here (the method/key guards above own their - // specific messages). Single-command calls stay loose, as before — the - // browser backend validates their params. - if (params.commands && params.commands.length > 0) { + // specific messages). Legacy single-command calls stay loose; outcome + // reports need local validation because delivery is best-effort. + if (params.commands?.length || params.method === 'reportOutcome') { const commandContract = z .array(compliant ? CompliantAgentCommandSchema : AgentCommandSchema) - .safeParse(params.commands); + .safeParse(params.commands?.length ? params.commands : commands); if (!commandContract.success) { throw new UserError( commandContract.error.issues @@ -884,7 +884,11 @@ export function registerAgentTools( } // Execute all commands sequentially - const results: Array<{ method: string; result?: unknown }> = []; + const results: Array<{ + method: string; + params: Record; + result?: unknown; + }> = []; let closedDuringBatch = false; // Cross-origin baseline: prefer the URL from the previous snapshot, // else the first URL seen this batch — so [goto A, goto B, snapshot] @@ -904,11 +908,14 @@ export function registerAgentTools( integrationId, allowedDomains, ); - results.push({ method: 'close', result: { closed: true } }); + results.push({ ...cmd, result: { closed: true } }); closedDuringBatch = true; break; } - if (cmd.method === 'reportSkillOutcome') { + if ( + cmd.method === 'reportSkillOutcome' || + cmd.method === 'reportOutcome' + ) { try { await send(agentSession, cmd.method, cmd.params); } catch { @@ -1079,14 +1086,14 @@ export function registerAgentTools( ); } - results.push({ method: cmd.method, result: resp.result }); + results.push({ ...cmd, result: resp.result }); } // If the batch ended with close, format the result around the // command before close (close itself has no useful payload). const reportable = closedDuringBatch ? results.slice(0, -1) : results; - // Nothing user-facing ran (batch was only close and/or an internal - // reportSkillOutcome) — the deref below would throw, so short-circuit. + // Nothing user-facing ran (only close and/or outcome reports), so + // there is no page result to format. if (reportable.length === 0) { return [ { @@ -1097,7 +1104,7 @@ export function registerAgentTools( } const last = reportable[reportable.length - 1]; const lastResult = last.result as Record; - const lastCmd = commands[reportable.length - 1]; + const lastCmd = last; const closedSuffix = closedDuringBatch ? '\n\nBrowser session closed.' diff --git a/src/tools/schemas.ts b/src/tools/schemas.ts index c66907a..abf666e 100644 --- a/src/tools/schemas.ts +++ b/src/tools/schemas.ts @@ -655,6 +655,24 @@ const GetDownloadsCommandSchema = z.object({ params: z.object({}).optional().default({}), }); +const ReportOutcomeCommandSchema = z.object({ + method: z.literal('reportOutcome'), + params: z.object({ + success: z.boolean().describe('Whether the agent completed the task.'), + reason: z + .enum([ + 'completed', + 'blocked_by_site', + 'captcha', + 'login_required', + 'timeout', + 'other', + ]) + .optional() + .describe('Optional category for how the task ended.'), + }), +}); + const CloseCommandSchema = z.object({ method: z.literal('close'), params: z.object({}).optional().default({}), @@ -711,6 +729,7 @@ const specificCommandSchemas = [ GetDownloadsCommandSchema, StartRecordingCommandSchema, StopRecordingCommandSchema, + ReportOutcomeCommandSchema, CloseCommandSchema, ] as const; @@ -993,6 +1012,7 @@ const compliantCommandSchemas = [ // so its capture gate never arms and there is nothing to clear. // No uploadFile/getDownloads: upload impersonates a human write (vendor-TOS), // download is the paired file-I/O — a compliant web agent reads, doesn't move files. + ReportOutcomeCommandSchema, CloseCommandSchema, ] as const; diff --git a/test/tools/agent.spec.ts b/test/tools/agent.spec.ts index c35b352..dc5c682 100644 --- a/test/tools/agent.spec.ts +++ b/test/tools/agent.spec.ts @@ -1,6 +1,6 @@ import { expect } from 'chai'; import sinon from 'sinon'; -import { FastMCP } from 'fastmcp'; +import { FastMCP, UserError } from 'fastmcp'; import type { Content } from 'fastmcp'; import { buildCrossOriginNotice, @@ -1087,35 +1087,40 @@ describe('browserless_agent retry-guard (runCommands)', () => { } }); - it('returns the saved-download handle when a screenshot { toDisk } batch ends with close', async () => { - // 1x1 PNG so getScreenshotPayload sees a real base64 payload. - const png = - 'iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNk+M9QDwADhgGAWjR9awAAAABJRU5ErkJggg=='; - const srv = await makeRespondingServer((method) => - method === 'screenshot' ? { base64: png } : { closed: true }, - ); - try { - const execute = getAgentExecute(srv.url); - const result = (await execute( - { - commands: [ - { method: 'screenshot', params: { toDisk: true } }, - { method: 'close' }, - ], - }, - ctx('todisk-then-close'), - )) as { content: Content[] }; - const text = (result.content[0] as Extract) - .text; - // toDisk branch fired: a reusable path, not the inline image/JSON the - // close-as-lastCmd bug produced. - expect(text).to.include('Screenshot saved to disk'); - expect(text).to.include('reuse as uploadFile'); - expect(result.content.some((c) => c.type === 'image')).to.equal(false); - } finally { - await srv.close(); - } - }); + for (const reportFirst of [false, true]) { + it(`returns the saved-download handle when a screenshot { toDisk } batch ends with close (reportFirst=${reportFirst})`, async () => { + // 1x1 PNG so getScreenshotPayload sees a real base64 payload. + const png = + 'iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNk+M9QDwADhgGAWjR9awAAAABJRU5ErkJggg=='; + const srv = await makeRespondingServer((method) => + method === 'screenshot' ? { base64: png } : { closed: true }, + ); + try { + const execute = getAgentExecute(srv.url); + const result = (await execute( + { + commands: [ + ...(reportFirst + ? [{ method: 'reportOutcome', params: { success: true } }] + : []), + { method: 'screenshot', params: { toDisk: true } }, + { method: 'close' }, + ], + }, + ctx(`todisk-then-close-${reportFirst}`), + )) as { content: Content[] }; + const text = (result.content[0] as Extract) + .text; + // toDisk branch fired: a reusable path, not the inline image/JSON the + // close-as-lastCmd bug produced. + expect(text).to.include('Screenshot saved to disk'); + expect(text).to.include('reuse as uploadFile'); + expect(result.content.some((c) => c.type === 'image')).to.equal(false); + } finally { + await srv.close(); + } + }); + } it('DOES retry once on a retryable upgrade failure (503)', async () => { const srv = await makeRejectingServer(503, 'Service Unavailable'); @@ -1232,6 +1237,110 @@ describe('browserless_agent _prompt capture', () => { }); }); +describe('browserless_agent reportOutcome', () => { + afterEach(() => sinon.restore()); + + it('rejects malformed top-level verdicts before forwarding', async () => { + const calls: string[] = []; + const srv = await makeRespondingServer((method) => { + calls.push(method); + return { recorded: true }; + }); + try { + const execute = getAgentExecute(srv.url); + for (const params of [ + {}, + { success: 'yes' }, + { success: false, reason: 'unlisted' }, + ]) { + try { + await execute({ method: 'reportOutcome', params }, mockContext); + expect.fail('expected invalid verdict to be rejected'); + } catch (err) { + expect(err).to.be.instanceOf(UserError); + } + } + expect(calls).to.deep.equal([]); + expect(srv.hits()).to.equal(0); + } finally { + await srv.close(); + } + }); + + it('forwards a valid false top-level verdict', async () => { + const calls: Array<{ method: string; params: unknown }> = []; + const srv = await makeRespondingServer((method, params) => { + calls.push({ method, params }); + return { recorded: true }; + }); + try { + await getAgentExecute(srv.url)( + { + method: 'reportOutcome', + params: { success: false, reason: 'captcha' }, + }, + mockContext, + ); + expect(calls).to.deep.equal([ + { + method: 'reportOutcome', + params: { success: false, reason: 'captcha' }, + }, + ]); + } finally { + await srv.close(); + } + }); + + for (const complianceMode of [false, true]) { + it(`forwards the verdict without replacing the page result (compliant=${complianceMode})`, async () => { + const calls: Array<{ method: string; params: unknown }> = []; + const srv = await makeRespondingServer((method, params) => { + calls.push({ method, params }); + return method === 'reportOutcome' + ? { recorded: true } + : { status: 200, marker: 'page-result' }; + }); + try { + const server = new FastMCP({ name: 'test', version: '0.1.0' }); + const spy = sinon.spy(server, 'addTool'); + registerAgentTools(server, { + ...mockConfig, + browserlessApiUrl: srv.url, + complianceMode, + }); + const tool = spy + .getCalls() + .find((c) => c.args[0].name === 'browserless_agent')!.args[0]; + const execute = tool.execute as ( + args: unknown, + ctx: unknown, + ) => Promise<{ content: Content[] }>; + const verdict = { success: false, reason: 'captcha' }; + const result = await execute( + { + commands: [ + { method: 'goto', params: { url: 'https://example.com' } }, + { method: 'reportOutcome', params: verdict }, + { method: 'close' }, + ], + }, + { ...mockContext, sessionId: `outcome-${complianceMode}` }, + ); + expect(calls).to.deep.equal([ + { method: 'goto', params: { url: 'https://example.com' } }, + { method: 'reportOutcome', params: verdict }, + ]); + expect(JSON.stringify(result.content)) + .to.include('page-result') + .and.not.include('recorded'); + } finally { + await srv.close(); + } + }); + } +}); + describe('browserless_agent session handle on errors', () => { afterEach(() => sinon.restore()); diff --git a/test/tools/compliance-mode.spec.ts b/test/tools/compliance-mode.spec.ts index f344b9c..18ea886 100644 --- a/test/tools/compliance-mode.spec.ts +++ b/test/tools/compliance-mode.spec.ts @@ -255,6 +255,7 @@ describe('compliance mode — compliant tool surface', () => { 'waitForResponse', 'liveURL', 'screenshot', + 'reportOutcome', 'close', ]; expect([...COMPLIANT_AGENT_METHODS]).to.have.members(EXPECTED_METHODS); diff --git a/test/tools/schemas.spec.ts b/test/tools/schemas.spec.ts index 952a26b..3549795 100644 --- a/test/tools/schemas.spec.ts +++ b/test/tools/schemas.spec.ts @@ -15,6 +15,41 @@ for (const [name, schema] of [ ['AgentCommandSchema', AgentCommandSchema], ['CompliantAgentCommandSchema', CompliantAgentCommandSchema], ] as const) { + describe(`${name} reportOutcome`, () => { + it('accepts boolean verdicts and each fixed reason', () => { + for (const success of [true, false]) { + for (const reason of [ + undefined, + 'completed', + 'blocked_by_site', + 'captcha', + 'login_required', + 'timeout', + 'other', + ]) { + const command = { + method: 'reportOutcome', + params: { success, ...(reason ? { reason } : {}) }, + }; + expect(schema.parse(command)).to.deep.equal(command); + } + } + }); + + it('rejects malformed verdicts rather than using generic passthrough', () => { + for (const params of [ + {}, + { success: 'yes' }, + { success: 1 }, + { success: true, reason: 'https://example.com?token=secret' }, + ]) { + expect( + schema.safeParse({ method: 'reportOutcome', params }).success, + ).to.equal(false); + } + }); + }); + describe(`${name} navigation URLs`, () => { it('preserves HTTP and HTTPS URLs for navigation and new tabs', () => { for (const method of ['goto', 'createTab'] as const) {