diff --git a/CHANGELOG.md b/CHANGELOG.md index 9e3c37dd2..19e7db084 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## Unreleased +### Features + +* **browser** — add `page.evaluate(fn, ...args)` for type-safe browser-context evaluation with JSON-serialized arguments. String evaluation remains supported, but new adapter code should use function form to avoid implicit `wrapForEval` auto-IIFE magic. + ### ⚠ BREAKING CHANGES * **browser** — replace the `--session ` flag with a `` positional argument that immediately follows `browser`. `opencli browser work click 12` instead of `opencli browser --session work click 12`; `opencli browser work bind` instead of `opencli browser bind --session work`. Required-flag semantics are now encoded structurally as a positional, matching the Docker/git convention for required operation-target identifiers. The internal `--session` flag is preserved for the daemon protocol and for direct `program.parseAsync` callers but is no longer part of the user-facing surface. diff --git a/clis/twitter/following.js b/clis/twitter/following.js index dfe0599cb..00cc934c6 100644 --- a/clis/twitter/following.js +++ b/clis/twitter/following.js @@ -164,10 +164,10 @@ cli({ throw new AuthRequiredError('x.com', 'Not logged into x.com (no ct0 cookie)'); if (!targetUser) { - const href = await page.evaluate(`() => { + const href = await page.evaluate(() => { const link = document.querySelector('a[data-testid="AppTabBar_Profile_Link"]'); return link ? link.getAttribute('href') : null; - }`); + }); if (!href) throw new AuthRequiredError('x.com', 'Could not detect logged-in user. Are you logged in?'); targetUser = normalizeScreenName(href.replace('/', '')); @@ -178,21 +178,20 @@ cli({ const followingQueryId = await resolveTwitterQueryId(page, 'Following', FOLLOWING_QUERY_ID); const userByScreenNameQueryId = await resolveTwitterQueryId(page, 'UserByScreenName', USER_BY_SCREEN_NAME_QUERY_ID); - const headers = JSON.stringify({ + const headers = { 'Authorization': `Bearer ${decodeURIComponent(TWITTER_BEARER_TOKEN)}`, 'X-Csrf-Token': ct0, 'X-Twitter-Auth-Type': 'OAuth2Session', 'X-Twitter-Active-User': 'yes', - }); + }; // Get userId from screen_name - const userLookup = await page.evaluate(`async () => { - const url = ${JSON.stringify(buildUserByScreenNameUrl(userByScreenNameQueryId, targetUser))}; - const resp = await fetch(url, { headers: ${headers}, credentials: 'include' }); + const userLookup = await page.evaluate(async (url, headers) => { + const resp = await fetch(url, { headers, credentials: 'include' }); if (!resp.ok) return { error: resp.status }; const d = await resp.json(); return { userId: d.data?.user?.result?.rest_id || null }; - }`); + }, buildUserByScreenNameUrl(userByScreenNameQueryId, targetUser), headers); if (userLookup?.error === 401 || userLookup?.error === 403) { throw new AuthRequiredError('x.com', `Twitter user lookup failed (HTTP ${userLookup.error})`); } @@ -211,10 +210,10 @@ cli({ for (let i = 0; i < maxPages && allUsers.length < limit; i++) { const fetchCount = Math.min(50, limit - allUsers.length + 10); const apiUrl = buildFollowingUrl(followingQueryId, userId, fetchCount, cursor); - const data = await page.evaluate(`async () => { - const r = await fetch("${apiUrl}", { headers: ${headers}, credentials: 'include' }); + const data = await page.evaluate(async (url, headers) => { + const r = await fetch(url, { headers, credentials: 'include' }); return r.ok ? await r.json() : { error: r.status }; - }`); + }, apiUrl, headers); if (data?.error) { if (data.error === 401 || data.error === 403) throw new AuthRequiredError('x.com', `Twitter following request failed (HTTP ${data.error})`); diff --git a/clis/twitter/following.test.js b/clis/twitter/following.test.js index 66933d0c3..41aefc3c1 100644 --- a/clis/twitter/following.test.js +++ b/clis/twitter/following.test.js @@ -206,7 +206,14 @@ function createFollowingPage(followingResponses, { ct0 = 'token', userLookup = { goto: vi.fn().mockResolvedValue(undefined), wait: vi.fn().mockResolvedValue(undefined), getCookies: vi.fn(async () => (ct0 ? [{ name: 'ct0', value: ct0 }] : [])), - evaluate: vi.fn(async (script) => { + evaluate: vi.fn(async (script, ...args) => { + if (typeof script === 'function') { + const haystack = [script.toString(), ...args.map((arg) => String(arg))].join('\n'); + if (haystack.includes('/UserByScreenName')) return userLookup; + if (haystack.includes('/Following')) return followingResponses.shift() || followingPayload([], null); + if (haystack.includes('AppTabBar_Profile_Link')) return '/viewer'; + throw new Error(`Unexpected evaluate function: ${haystack.slice(0, 80)}`); + } if (script.includes('operationName')) return null; if (script.includes('/UserByScreenName')) return userLookup; if (script.includes('/Following')) return followingResponses.shift() || followingPayload([], null); @@ -229,12 +236,13 @@ describe('twitter following command', () => { expect(rows.map((row) => row.screen_name)).toEqual(['alice', 'bob', 'carol']); expect(page.getCookies).toHaveBeenCalledWith({ url: 'https://x.com' }); - const userLookupScript = page.evaluate.mock.calls.find(([script]) => script.includes('/UserByScreenName'))?.[0] || ''; + const callText = (call) => call.map((part) => typeof part === 'function' ? part.toString() : String(part)).join('\n'); + const userLookupScript = callText(page.evaluate.mock.calls.find((call) => callText(call).includes('/UserByScreenName')) || []); expect(decodeURIComponent(userLookupScript)).toContain('"screen_name":"elonmusk"'); expect(decodeURIComponent(userLookupScript)).not.toContain('"screen_name":"@elonmusk"'); - const followingCalls = page.evaluate.mock.calls.filter(([script]) => script.includes('/Following')); + const followingCalls = page.evaluate.mock.calls.filter((call) => callText(call).includes('/Following')); expect(followingCalls).toHaveLength(2); - expect(decodeURIComponent(followingCalls[1][0])).toContain('"cursor":"cursor-1"'); + expect(decodeURIComponent(callText(followingCalls[1]))).toContain('"cursor":"cursor-1"'); }); it('rejects invalid limits before navigating', async () => { diff --git a/docs/developer/ts-adapter.md b/docs/developer/ts-adapter.md index 680a4a7bb..584c34921 100644 --- a/docs/developer/ts-adapter.md +++ b/docs/developer/ts-adapter.md @@ -28,14 +28,12 @@ cli({ // Navigate and extract data await page.goto('https://www.mysite.com'); - const data = await page.evaluate(` - (async () => { - const res = await fetch('/api/search?q=${encodeURIComponent(String(query))}', { - credentials: 'include' - }); - return (await res.json()).results; - })() - `); + const data = await page.evaluate(async (q: string) => { + const res = await fetch(`/api/search?q=${encodeURIComponent(q)}`, { + credentials: 'include', + }); + return (await res.json()).results; + }, String(query)); if (!Array.isArray(data)) throw new CommandExecutionError('MySite returned an unexpected response'); if (!data.length) throw new EmptyResultError('mysite search', 'Try a different keyword'); @@ -112,7 +110,8 @@ persistence with `--site-session persistent`. The `page` parameter provides browser interaction methods: - `page.goto(url)` — Navigate to a URL -- `page.evaluate(script)` — Execute JavaScript in the page context +- `page.evaluate(fn, ...args)` — Execute a serializable function in the page context. Pass Node-side values through JSON-serializable args; the function cannot close over local variables. +- `page.evaluate(script)` — Execute a raw JavaScript string in the page context. Prefer function form for new adapter code. - `page.waitForSelector(selector)` — Wait for an element - `page.click(selector)` — Click an element - `page.type(selector, text)` — Type text into an input diff --git a/skills/opencli-adapter-author/references/adapter-template.md b/skills/opencli-adapter-author/references/adapter-template.md index 599300c07..6296aabed 100644 --- a/skills/opencli-adapter-author/references/adapter-template.md +++ b/skills/opencli-adapter-author/references/adapter-template.md @@ -264,6 +264,19 @@ const data = await page.fetchJson(`${BASE}/api/list`, { 它固定 `credentials: 'include'`，带 timeout，HTTP 非 2xx / 非 JSON 会抛统一 runtime error。adapter 里不用再手写 `page.evaluate(fetch(...))`；如果你需要额外包一层业务语义，按 [`typed-errors.md`](./typed-errors.md) 映射到 `CommandExecutionError` / `AuthRequiredError` / `EmptyResultError`。 +### 页面内 DOM 逻辑用 `page.evaluate(fn, ...args)` + +新 adapter 优先写函数形式，外部变量通过参数传入： + +```javascript +const href = await page.evaluate((selector) => { + const link = document.querySelector(selector); + return link ? link.getAttribute('href') : null; +}, 'a[data-testid="profile"]'); +``` + +`fn` 在浏览器页面上下文执行，不能读取 Node 侧闭包变量；参数必须能被 `JSON.stringify` 序列化。字符串形式 `page.evaluate('document.title')` 仍可用于简单表达式和既有代码，但不要再写依赖隐式 auto-IIFE 的模板字符串函数。 + ### HTML 不走 browser fetch 三个坑，踩一个就重写： diff --git a/skills/opencli-adapter-author/references/api-discovery.md b/skills/opencli-adapter-author/references/api-discovery.md index e364efb02..e919087a2 100644 --- a/skills/opencli-adapter-author/references/api-discovery.md +++ b/skills/opencli-adapter-author/references/api-discovery.md @@ -250,7 +250,9 @@ opencli browser eval "window.__pinia.state.value.someStore.someMethod({...})" ```javascript // func 里 -await page.evaluate(installInterceptorCode, { domain: 'api.xxx.com', path: '/foo' }); +await page.evaluateWithArgs(installInterceptorCode, { + config: { domain: 'api.xxx.com', path: '/foo' }, +}); await page.goto('https://xxx.com/trigger-page'); // 等页面自己发那条请求 const intercepted = await page.evaluate('window.__opencli_intercepted'); diff --git a/src/browser/base-page.test.ts b/src/browser/base-page.test.ts index 3d8378760..10ae9cb96 100644 --- a/src/browser/base-page.test.ts +++ b/src/browser/base-page.test.ts @@ -2,13 +2,15 @@ import { describe, expect, it, vi } from 'vitest'; import { CliError } from '../errors.js'; import { BasePage } from './base-page.js'; import { TargetError } from './target-errors.js'; -import type { ScreenshotOptions } from '../types.js'; +import type { BrowserEvaluateFunction, ScreenshotOptions } from '../types.js'; class TestPage extends BasePage { result: unknown; args: Record | undefined; async goto(): Promise {} + async evaluate(_js: string): Promise; + async evaluate(_fn: BrowserEvaluateFunction, ..._args: Args): Promise>; async evaluate(): Promise { return null; } override async evaluateWithArgs(_js: string, args: Record): Promise { this.args = args; @@ -34,8 +36,10 @@ class ActionPage extends BasePage { cdp?: (method: string, params?: Record) => Promise; async goto(): Promise {} - async evaluate(js: string): Promise { - this.scripts.push(js); + async evaluate(js: string): Promise; + async evaluate(fn: BrowserEvaluateFunction, ...args: Args): Promise>; + async evaluate(input: string | BrowserEvaluateFunction): Promise { + this.scripts.push(typeof input === 'string' ? input : input.toString()); return this.results.shift() ?? null; } override async evaluateWithArgs(js: string, args: Record): Promise { diff --git a/src/browser/base-page.ts b/src/browser/base-page.ts index 9a1a888de..22a326bd2 100644 --- a/src/browser/base-page.ts +++ b/src/browser/base-page.ts @@ -9,7 +9,7 @@ * getCookies, screenshot, tabs, etc. */ -import type { BrowserCookie, FetchJsonOptions, IPage, ScreenshotOptions, SnapshotOptions, WaitOptions } from '../types.js'; +import type { BrowserCookie, BrowserEvaluateFunction, FetchJsonOptions, IPage, ScreenshotOptions, SnapshotOptions, WaitOptions } from '../types.js'; import { generateSnapshotJs, getFormStateJs } from './dom-snapshot.js'; import { buildAxSnapshotFromTrees, findAxRefReplacement, type AxSnapshotTree, type BrowserRef } from './ax-snapshot.js'; import { @@ -146,7 +146,8 @@ export abstract class BasePage implements IPage { // ── Transport-specific methods (must be implemented by subclasses) ── abstract goto(url: string, options?: { waitUntil?: 'load' | 'none'; settleMs?: number; allowBoundNavigation?: boolean }): Promise; - abstract evaluate(js: string): Promise; + abstract evaluate(js: string): Promise; + abstract evaluate(fn: BrowserEvaluateFunction, ...args: Args): Promise>; /** * Safely evaluate JS with pre-serialized arguments. diff --git a/src/browser/cdp.ts b/src/browser/cdp.ts index 78b5c5793..a4be15c50 100644 --- a/src/browser/cdp.ts +++ b/src/browser/cdp.ts @@ -11,9 +11,9 @@ import { WebSocket, type RawData } from 'ws'; import { request as httpRequest } from 'node:http'; import { request as httpsRequest } from 'node:https'; -import type { BrowserCookie, IPage, ScreenshotOptions } from '../types.js'; +import type { BrowserCookie, BrowserEvaluateFunction, IPage, ScreenshotOptions } from '../types.js'; import type { IBrowserFactory } from '../runtime.js'; -import { wrapForEval } from './utils.js'; +import { buildEvaluateExpression } from './utils.js'; import { generateStealthJs } from './stealth.js'; import { waitForDomStableJs } from './dom-helpers.js'; import { isRecord, saveBase64ToFile } from '../utils.js'; @@ -221,8 +221,10 @@ class CDPPage extends BasePage { } } - async evaluate(js: string): Promise { - const expression = wrapForEval(js); + async evaluate(js: string): Promise; + async evaluate(fn: BrowserEvaluateFunction, ...args: Args): Promise>; + async evaluate(input: string | BrowserEvaluateFunction, ...args: unknown[]): Promise { + const expression = buildEvaluateExpression(input, args); const result = await this.bridge.send('Runtime.evaluate', { expression, returnByValue: true, diff --git a/src/browser/page.test.ts b/src/browser/page.test.ts index 9da453b4d..352139b8c 100644 --- a/src/browser/page.test.ts +++ b/src/browser/page.test.ts @@ -92,6 +92,45 @@ describe('Page.evaluate', () => { expect(value).toBe(42); expect(sendCommandMock).toHaveBeenCalledTimes(2); }); + + it('serializes function-form evaluate calls with JSON args', async () => { + sendCommandMock.mockResolvedValueOnce('/opencli'); + + const page = new Page('twitter', undefined, undefined, undefined, 'adapter'); + const href = await page.evaluate((selector: string) => { + const link = document.querySelector(selector); + return link ? link.getAttribute('href') : null; + }, 'a[data-testid="AppTabBar_Profile_Link"]'); + + expect(href).toBe('/opencli'); + expect(sendCommandMock).toHaveBeenCalledWith('exec', expect.objectContaining({ + session: 'twitter', + surface: 'adapter', + code: expect.stringContaining('(...["a[data-testid=\\"AppTabBar_Profile_Link\\"]"])'), + })); + const code = sendCommandMock.mock.calls[0]?.[1]?.code as string; + expect(code).toContain('document.querySelector(selector)'); + }); + + it('rejects non-JSON-serializable evaluate args before sending to the daemon', async () => { + const page = new Page('default'); + const circular: Record = {}; + circular.self = circular; + + await expect(page.evaluate((value: unknown) => value, circular)).rejects.toThrow('JSON-serializable'); + expect(sendCommandMock).not.toHaveBeenCalled(); + }); + + it('keeps string evaluate behavior unchanged', async () => { + sendCommandMock.mockResolvedValueOnce(42); + + const page = new Page('default'); + await expect(page.evaluate('21 + 21')).resolves.toBe(42); + + expect(sendCommandMock).toHaveBeenCalledWith('exec', expect.objectContaining({ + code: '21 + 21', + })); + }); }); describe('Page network capture compatibility', () => { diff --git a/src/browser/page.ts b/src/browser/page.ts index 01b4cb01b..724c594d4 100644 --- a/src/browser/page.ts +++ b/src/browser/page.ts @@ -9,9 +9,9 @@ * page-scoped operations target the correct page without guessing. */ -import type { BrowserCookie, BrowserDownloadWaitResult, ScreenshotOptions } from '../types.js'; +import type { BrowserCookie, BrowserDownloadWaitResult, BrowserEvaluateFunction, ScreenshotOptions } from '../types.js'; import { sendCommand, sendCommandFull } from './daemon-client.js'; -import { wrapForEval } from './utils.js'; +import { buildEvaluateExpression } from './utils.js'; import { saveBase64ToFile } from '../utils.js'; import { generateStealthJs } from './stealth.js'; import { waitForDomStableJs } from './dom-helpers.js'; @@ -141,8 +141,10 @@ export class Page extends BasePage { ); } - async evaluate(js: string): Promise { - const code = wrapForEval(js); + async evaluate(js: string): Promise; + async evaluate(fn: BrowserEvaluateFunction, ...args: Args): Promise>; + async evaluate(input: string | BrowserEvaluateFunction, ...args: unknown[]): Promise { + const code = buildEvaluateExpression(input, args); try { return await sendCommand('exec', { code, ...this._cmdOpts() }); } catch (err) { @@ -302,7 +304,7 @@ export class Page extends BasePage { } async evaluateInFrame(js: string, frameIndex: number): Promise { - const code = wrapForEval(js); + const code = buildEvaluateExpression(js); return sendCommand('exec', { code, frameIndex, ...this._cmdOpts() }); } diff --git a/src/browser/utils.test.ts b/src/browser/utils.test.ts new file mode 100644 index 000000000..4d4ec76cc --- /dev/null +++ b/src/browser/utils.test.ts @@ -0,0 +1,36 @@ +import { describe, expect, it } from 'vitest'; +import { buildEvaluateExpression, wrapForEval } from './utils.js'; + +describe('browser eval utils', () => { + it('keeps existing string eval wrapping behavior', () => { + expect(wrapForEval('21 + 21')).toBe('21 + 21'); + expect(wrapForEval('() => 42')).toBe('(() => 42)()'); + }); + + it('serializes function eval arguments as JSON', () => { + const code = buildEvaluateExpression((selector: string) => { + return document.querySelector(selector)?.textContent ?? null; + }, ['.title']); + + expect(code).toContain('document.querySelector(selector)'); + expect(code).toContain('(...[".title"])'); + }); + + it('accepts compact async arrow functions', () => { + const fn = new Function('return async()=>42')() as () => Promise; + expect(buildEvaluateExpression(fn)).toBe('(async()=>42)(...[])'); + }); + + it('rejects string eval with stray args', () => { + expect(() => buildEvaluateExpression('document.title', ['ignored'])) + .toThrow('use page.evaluate(fn, ...args)'); + }); + + it('rejects non-JSON-serializable function args', () => { + const circular: Record = {}; + circular.self = circular; + + expect(() => buildEvaluateExpression((value: unknown) => value, [circular])) + .toThrow('JSON-serializable'); + }); +}); diff --git a/src/browser/utils.ts b/src/browser/utils.ts index 40017961e..547385901 100644 --- a/src/browser/utils.ts +++ b/src/browser/utils.ts @@ -2,6 +2,39 @@ * Utility functions for browser operations */ +type EvaluateFunction = (...args: any[]) => unknown; + +function describeJsonError(err: unknown): string { + return err instanceof Error ? err.message : String(err); +} + +/** + * Serialize a function-form page.evaluate call for CDP Runtime.evaluate. + * + * Functions execute in the browser page context, so they cannot close over + * Node-side variables. Pass external values as JSON-serializable args instead. + */ +export function serializeFunctionForEval(fn: EvaluateFunction, args: readonly unknown[] = []): string { + const source = fn.toString().trim(); + const isFunctionSource = /^(async\s+)?function[\s(]/.test(source) + || /^(async\s*)?(\([^)]*\)|[A-Za-z_$][\w$]*)\s*=>/.test(source); + if (!isFunctionSource || source.includes('[native code]')) { + throw new Error('page.evaluate(fn) requires a serializable arrow/function expression'); + } + + let serializedArgs: string; + try { + serializedArgs = JSON.stringify(args); + } catch (err) { + throw new Error(`page.evaluate arguments must be JSON-serializable: ${describeJsonError(err)}`); + } + if (serializedArgs === undefined) { + throw new Error('page.evaluate arguments must be JSON-serializable'); + } + + return `(${source})(...${serializedArgs})`; +} + /** * Wrap JS code for CDP Runtime.evaluate: * - Already an IIFE `(...)()` → send as-is @@ -25,3 +58,13 @@ export function wrapForEval(js: string): string { // Everything else: bare expression, `new Promise(...)`, etc. → evaluate directly return code; } + +export function buildEvaluateExpression(input: string | EvaluateFunction, args: readonly unknown[] = []): string { + if (typeof input === 'function') { + return serializeFunctionForEval(input, args); + } + if (args.length > 0) { + throw new Error('page.evaluate string input does not accept args; use page.evaluate(fn, ...args) instead'); + } + return wrapForEval(input); +} diff --git a/src/types.ts b/src/types.ts index 7245debdd..9ed140552 100644 --- a/src/types.ts +++ b/src/types.ts @@ -67,9 +67,12 @@ export interface FetchJsonOptions { timeoutMs?: number; } +export type BrowserEvaluateFunction = (...args: Args) => Result | Promise; + export interface IPage { goto(url: string, options?: { waitUntil?: 'load' | 'none'; settleMs?: number }): Promise; - evaluate(js: string): Promise; + evaluate(js: string): Promise; + evaluate(fn: BrowserEvaluateFunction, ...args: Args): Promise>; /** Safely evaluate JS with pre-serialized arguments — prevents injection. */ evaluateWithArgs?(js: string, args: Record): Promise; /**