From 80225676e517ace6c587072601d56221a9b28667 Mon Sep 17 00:00:00 2001 From: jackwener Date: Tue, 24 Mar 2026 14:59:33 +0800 Subject: [PATCH 1/2] refactor: extract shared utilities and simplify codebase - R1: Extract isRecord() type guard to shared src/utils.ts (8 files) - R2: Merge duplicate mapConcurrent() to shared utils (fetch.ts + download.ts) - R3: Extract saveBase64ToFile() helper (cdp.ts + page.ts) - R4: Merge _tabOpt() + _workspaceOpt() into _cmdOpts()/_wsOpt() (page.ts) - R5: Extract normalizeRows() + resolveColumns() in output.ts - R6: Remove dead register stub from generate.ts Co-Authored-By: Claude Opus 4.6 --- src/browser/cdp.ts | 10 +---- src/browser/page.ts | 69 ++++++++++++++++----------------- src/build-manifest.ts | 4 +- src/cli.ts | 2 - src/discovery.ts | 4 +- src/generate.ts | 38 ------------------ src/output.ts | 20 +++++++--- src/pipeline/steps/browser.ts | 4 +- src/pipeline/steps/download.ts | 22 +---------- src/pipeline/steps/fetch.ts | 19 +-------- src/pipeline/steps/transform.ts | 4 +- src/pipeline/template.ts | 4 +- src/utils.ts | 38 ++++++++++++++++++ src/validate.ts | 4 +- 14 files changed, 95 insertions(+), 147 deletions(-) create mode 100644 src/utils.ts diff --git a/src/browser/cdp.ts b/src/browser/cdp.ts index d8669c33..04966bfb 100644 --- a/src/browser/cdp.ts +++ b/src/browser/cdp.ts @@ -292,11 +292,7 @@ class CDPPage implements IPage { }); const base64 = isRecord(result) && typeof result.data === 'string' ? result.data : ''; if (options.path) { - const fs = await import('node:fs'); - const path = await import('node:path'); - const dir = path.dirname(options.path); - await fs.promises.mkdir(dir, { recursive: true }); - await fs.promises.writeFile(options.path, Buffer.from(base64, 'base64')); + await saveBase64ToFile(base64, options.path); } return base64; } @@ -341,9 +337,7 @@ class CDPPage implements IPage { } } -function isRecord(value: unknown): value is Record { - return typeof value === 'object' && value !== null && !Array.isArray(value); -} +import { isRecord, saveBase64ToFile } from '../utils.js'; function isCookie(value: unknown): value is BrowserCookie { return isRecord(value) diff --git a/src/browser/page.ts b/src/browser/page.ts index 5576fe01..0204f0cb 100644 --- a/src/browser/page.ts +++ b/src/browser/page.ts @@ -14,6 +14,7 @@ import { formatSnapshot } from '../snapshotFormatter.js'; import type { BrowserCookie, IPage, ScreenshotOptions, SnapshotOptions, WaitOptions } from '../types.js'; import { sendCommand } from './daemon-client.js'; import { wrapForEval } from './utils.js'; +import { saveBase64ToFile } from '../utils.js'; import { generateSnapshotJs, scrollToRefJs, getFormStateJs } from './dom-snapshot.js'; import { generateStealthJs } from './stealth.js'; import { @@ -36,20 +37,23 @@ export class Page implements IPage { /** Active tab ID, set after navigate and used in all subsequent commands */ private _tabId: number | undefined; - /** Helper: spread tabId into command params if we have one */ - private _tabOpt(): { tabId: number } | Record { - return this._tabId !== undefined ? { tabId: this._tabId } : {}; + /** Helper: spread workspace into command params */ + private _wsOpt(): { workspace: string } { + return { workspace: this.workspace }; } - private _workspaceOpt(): { workspace: string } { - return { workspace: this.workspace }; + /** Helper: spread workspace + tabId into command params */ + private _cmdOpts(): Record { + return { + workspace: this.workspace, + ...(this._tabId !== undefined && { tabId: this._tabId }), + }; } async goto(url: string, options?: { waitUntil?: 'load' | 'none'; settleMs?: number }): Promise { const result = await sendCommand('navigate', { url, - ...this._workspaceOpt(), - ...this._tabOpt(), + ...this._cmdOpts(), }) as { tabId?: number }; // Remember the tabId for subsequent exec calls if (result?.tabId) { @@ -59,8 +63,7 @@ export class Page implements IPage { try { await sendCommand('exec', { code: generateStealthJs(), - ...this._workspaceOpt(), - ...this._tabOpt(), + ...this._cmdOpts(), }); } catch { // Non-fatal: stealth is best-effort @@ -71,8 +74,7 @@ export class Page implements IPage { const maxMs = options?.settleMs ?? 1000; await sendCommand('exec', { code: waitForDomStableJs(maxMs, Math.min(500, maxMs)), - ...this._workspaceOpt(), - ...this._tabOpt(), + ...this._cmdOpts(), }); } } @@ -80,7 +82,7 @@ export class Page implements IPage { /** Close the automation window in the extension */ async closeWindow(): Promise { try { - await sendCommand('close-window', { ...this._workspaceOpt() }); + await sendCommand('close-window', { ...this._wsOpt() }); } catch { // Window may already be closed or daemon may be down } @@ -88,11 +90,11 @@ export class Page implements IPage { async evaluate(js: string): Promise { const code = wrapForEval(js); - return sendCommand('exec', { code, ...this._workspaceOpt(), ...this._tabOpt() }); + return sendCommand('exec', { code, ...this._cmdOpts() }); } async getCookies(opts: { domain?: string; url?: string } = {}): Promise { - const result = await sendCommand('cookies', { ...this._workspaceOpt(), ...opts }); + const result = await sendCommand('cookies', { ...this._wsOpt(), ...opts }); return Array.isArray(result) ? result : []; } @@ -108,7 +110,7 @@ export class Page implements IPage { }); try { - const result = await sendCommand('exec', { code: snapshotJs, ...this._workspaceOpt(), ...this._tabOpt() }); + const result = await sendCommand('exec', { code: snapshotJs, ...this._cmdOpts() }); // The advanced engine already produces a clean, pruned, LLM-friendly output. // Do NOT pass through formatSnapshot — its format is incompatible. return result; @@ -148,7 +150,7 @@ export class Page implements IPage { return buildTree(document.body, 0); })() `; - const raw = await sendCommand('exec', { code, ...this._workspaceOpt(), ...this._tabOpt() }); + const raw = await sendCommand('exec', { code, ...this._cmdOpts() }); if (opts.raw) return raw; if (typeof raw === 'string') return formatSnapshot(raw, opts); return raw; @@ -156,27 +158,27 @@ export class Page implements IPage { async click(ref: string): Promise { const code = clickJs(ref); - await sendCommand('exec', { code, ...this._workspaceOpt(), ...this._tabOpt() }); + await sendCommand('exec', { code, ...this._cmdOpts() }); } async typeText(ref: string, text: string): Promise { const code = typeTextJs(ref, text); - await sendCommand('exec', { code, ...this._workspaceOpt(), ...this._tabOpt() }); + await sendCommand('exec', { code, ...this._cmdOpts() }); } async pressKey(key: string): Promise { const code = pressKeyJs(key); - await sendCommand('exec', { code, ...this._workspaceOpt(), ...this._tabOpt() }); + await sendCommand('exec', { code, ...this._cmdOpts() }); } async scrollTo(ref: string): Promise { const code = scrollToRefJs(ref); - return sendCommand('exec', { code, ...this._workspaceOpt(), ...this._tabOpt() }); + return sendCommand('exec', { code, ...this._cmdOpts() }); } async getFormState(): Promise> { const code = getFormStateJs(); - return (await sendCommand('exec', { code, ...this._workspaceOpt(), ...this._tabOpt() })) as Record; + return (await sendCommand('exec', { code, ...this._cmdOpts() })) as Record; } async wait(options: number | WaitOptions): Promise { @@ -191,35 +193,35 @@ export class Page implements IPage { if (options.text) { const timeout = (options.timeout ?? 30) * 1000; const code = waitForTextJs(options.text, timeout); - await sendCommand('exec', { code, ...this._workspaceOpt(), ...this._tabOpt() }); + await sendCommand('exec', { code, ...this._cmdOpts() }); } } async tabs(): Promise { - const result = await sendCommand('tabs', { op: 'list', ...this._workspaceOpt() }); + const result = await sendCommand('tabs', { op: 'list', ...this._wsOpt() }); return Array.isArray(result) ? result : []; } async closeTab(index?: number): Promise { - await sendCommand('tabs', { op: 'close', ...this._workspaceOpt(), ...(index !== undefined ? { index } : {}) }); + await sendCommand('tabs', { op: 'close', ...this._wsOpt(), ...(index !== undefined ? { index } : {}) }); // Invalidate cached tabId — the closed tab might have been our active one. // We can't know for sure (close-by-index doesn't return tabId), so reset. this._tabId = undefined; } async newTab(): Promise { - const result = await sendCommand('tabs', { op: 'new', ...this._workspaceOpt() }) as { tabId?: number }; + const result = await sendCommand('tabs', { op: 'new', ...this._wsOpt() }) as { tabId?: number }; if (result?.tabId) this._tabId = result.tabId; } async selectTab(index: number): Promise { - const result = await sendCommand('tabs', { op: 'select', index, ...this._workspaceOpt() }) as { selected?: number }; + const result = await sendCommand('tabs', { op: 'select', index, ...this._wsOpt() }) as { selected?: number }; if (result?.selected) this._tabId = result.selected; } async networkRequests(includeStatic: boolean = false): Promise { const code = networkRequestsJs(includeStatic); - const result = await sendCommand('exec', { code, ...this._workspaceOpt(), ...this._tabOpt() }); + const result = await sendCommand('exec', { code, ...this._cmdOpts() }); return Array.isArray(result) ? result : []; } @@ -241,19 +243,14 @@ export class Page implements IPage { */ async screenshot(options: ScreenshotOptions = {}): Promise { const base64 = await sendCommand('screenshot', { - ...this._workspaceOpt(), + ...this._cmdOpts(), format: options.format, quality: options.quality, fullPage: options.fullPage, - ...this._tabOpt(), }) as string; if (options.path) { - const fs = await import('node:fs'); - const path = await import('node:path'); - const dir = path.dirname(options.path); - await fs.promises.mkdir(dir, { recursive: true }); - await fs.promises.writeFile(options.path, Buffer.from(base64, 'base64')); + await saveBase64ToFile(base64, options.path); } return base64; @@ -261,14 +258,14 @@ export class Page implements IPage { async scroll(direction: string = 'down', amount: number = 500): Promise { const code = scrollJs(direction, amount); - await sendCommand('exec', { code, ...this._workspaceOpt(), ...this._tabOpt() }); + await sendCommand('exec', { code, ...this._cmdOpts() }); } async autoScroll(options: { times?: number; delayMs?: number } = {}): Promise { const times = options.times ?? 3; const delayMs = options.delayMs ?? 2000; const code = autoScrollJs(times, delayMs); - await sendCommand('exec', { code, ...this._workspaceOpt(), ...this._tabOpt() }); + await sendCommand('exec', { code, ...this._cmdOpts() }); } async installInterceptor(pattern: string): Promise { diff --git a/src/build-manifest.ts b/src/build-manifest.ts index bf853f6e..38a9d667 100644 --- a/src/build-manifest.ts +++ b/src/build-manifest.ts @@ -70,9 +70,7 @@ interface YamlCliDefinition { navigateBefore?: boolean | string; } -function isRecord(value: unknown): value is Record { - return typeof value === 'object' && value !== null && !Array.isArray(value); -} +import { isRecord } from './utils.js'; function extractBalancedBlock( diff --git a/src/cli.ts b/src/cli.ts index 07cc0e06..295140ca 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -173,8 +173,6 @@ export function runCli(BUILTIN_CLIS: string, USER_CLIS: string): void { const r = await generateCliFromUrl({ url, BrowserFactory: getBrowserFactory(), - builtinClis: BUILTIN_CLIS, - userClis: USER_CLIS, goal: opts.goal, site: opts.site, workspace, diff --git a/src/discovery.ts b/src/discovery.ts index d6c7ace9..29b28368 100644 --- a/src/discovery.ts +++ b/src/discovery.ts @@ -52,9 +52,7 @@ function parseStrategy(rawStrategy: string | undefined, fallback: Strategy = Str return Strategy[key] ?? fallback; } -function isRecord(value: unknown): value is Record { - return typeof value === 'object' && value !== null && !Array.isArray(value); -} +import { isRecord } from './utils.js'; /** * Discover and register CLI commands. diff --git a/src/generate.ts b/src/generate.ts index 39126708..9fa8d72b 100644 --- a/src/generate.ts +++ b/src/generate.ts @@ -12,30 +12,13 @@ import { exploreUrl } from './explore.js'; import type { IBrowserFactory } from './runtime.js'; import { synthesizeFromExplore, type SynthesizeCandidateSummary, type SynthesizeResult } from './synthesize.js'; -// Registration is a no-op stub — candidates are written to disk by synthesize, -// but not yet auto-copied into the user clis dir. -interface RegisterCandidatesOptions { - target: string; - builtinClis?: string; - userClis?: string; - name?: string; -} - -interface RegisterCandidatesResult { - ok: boolean; - count: number; -} - export interface GenerateCliOptions { url: string; BrowserFactory: new () => IBrowserFactory; - builtinClis?: string; - userClis?: string; goal?: string | null; site?: string; waitSeconds?: number; top?: number; - register?: boolean; workspace?: string; } @@ -57,11 +40,6 @@ export interface GenerateCliResult { candidate_count: number; candidates: Array>; }; - register: RegisterCandidatesResult | null; -} - -function registerCandidates(_opts: RegisterCandidatesOptions): RegisterCandidatesResult { - return { ok: true, count: 0 }; } const CAPABILITY_ALIASES: Record = { @@ -128,19 +106,6 @@ export async function generateCliFromUrl(opts: GenerateCliOptions): Promise 0) { - try { - registerResult = registerCandidates({ - target: synthesizeResult.out_dir, - builtinClis: opts.builtinClis, - userClis: opts.userClis, - name: selected?.name, - }); - } catch {} - } - const ok = exploreResult.endpoint_count > 0 && synthesizeResult.candidate_count > 0; return { @@ -165,7 +130,6 @@ export async function generateCliFromUrl(opts: GenerateCliOptions): Promise v).map(([k]) => k); if (fwNames.length) lines.push(`Framework: ${fwNames.join(', ')}`); diff --git a/src/output.ts b/src/output.ts index ecfc0cbb..b8ec480b 100644 --- a/src/output.ts +++ b/src/output.ts @@ -15,6 +15,14 @@ export interface RenderOptions { footerExtra?: string; } +function normalizeRows(data: unknown): Record[] { + return Array.isArray(data) ? data : [data as Record]; +} + +function resolveColumns(rows: Record[], opts: RenderOptions): string[] { + return opts.columns ?? Object.keys(rows[0] ?? {}); +} + export function render(data: unknown, opts: RenderOptions = {}): void { const fmt = opts.fmt ?? 'table'; if (data === null || data === undefined) { @@ -31,9 +39,9 @@ export function render(data: unknown, opts: RenderOptions = {}): void { } function renderTable(data: unknown, opts: RenderOptions): void { - const rows = Array.isArray(data) ? data : [data as Record]; + const rows = normalizeRows(data); if (!rows.length) { console.log(chalk.dim('(no data)')); return; } - const columns = opts.columns ?? Object.keys(rows[0]); + const columns = resolveColumns(rows, opts); const header = columns.map(c => capitalize(c)); const table = new Table({ @@ -66,9 +74,9 @@ function renderJson(data: unknown): void { } function renderMarkdown(data: unknown, opts: RenderOptions): void { - const rows = Array.isArray(data) ? data : [data as Record]; + const rows = normalizeRows(data); if (!rows.length) return; - const columns = opts.columns ?? Object.keys(rows[0]); + const columns = resolveColumns(rows, opts); console.log('| ' + columns.join(' | ') + ' |'); console.log('| ' + columns.map(() => '---').join(' | ') + ' |'); for (const row of rows) { @@ -77,9 +85,9 @@ function renderMarkdown(data: unknown, opts: RenderOptions): void { } function renderCsv(data: unknown, opts: RenderOptions): void { - const rows = Array.isArray(data) ? data : [data as Record]; + const rows = normalizeRows(data); if (!rows.length) return; - const columns = opts.columns ?? Object.keys(rows[0]); + const columns = resolveColumns(rows, opts); console.log(columns.join(',')); for (const row of rows) { console.log(columns.map(c => { diff --git a/src/pipeline/steps/browser.ts b/src/pipeline/steps/browser.ts index bffd631e..e9e7e644 100644 --- a/src/pipeline/steps/browser.ts +++ b/src/pipeline/steps/browser.ts @@ -6,9 +6,7 @@ import type { IPage } from '../../types.js'; import { render } from '../template.js'; -function isRecord(value: unknown): value is Record { - return typeof value === 'object' && value !== null && !Array.isArray(value); -} +import { isRecord } from '../../utils.js'; export async function stepNavigate(page: IPage | null, params: unknown, data: unknown, args: Record): Promise { if (isRecord(params) && 'url' in params) { diff --git a/src/pipeline/steps/download.ts b/src/pipeline/steps/download.ts index 86ecc138..99d09400 100644 --- a/src/pipeline/steps/download.ts +++ b/src/pipeline/steps/download.ts @@ -26,6 +26,7 @@ import { formatCookieHeader, } from '../../download/index.js'; import { DownloadProgressTracker, formatBytes } from '../../download/progress.js'; +import { mapConcurrent } from '../../utils.js'; export interface DownloadResult { status: 'success' | 'skipped' | 'failed'; @@ -35,28 +36,7 @@ export interface DownloadResult { duration?: number; } -/** - * Simple async concurrency limiter for downloads. - */ -async function mapConcurrent( - items: T[], - limit: number, - fn: (item: T, index: number) => Promise, -): Promise { - const results: R[] = new Array(items.length); - let index = 0; - - async function worker() { - while (index < items.length) { - const i = index++; - results[i] = await fn(items[i], i); - } - } - const workers = Array.from({ length: Math.min(limit, items.length) }, () => worker()); - await Promise.all(workers); - return results; -} /** * Extract cookies from browser page. diff --git a/src/pipeline/steps/fetch.ts b/src/pipeline/steps/fetch.ts index 6e0b1923..a771bb35 100644 --- a/src/pipeline/steps/fetch.ts +++ b/src/pipeline/steps/fetch.ts @@ -5,26 +5,9 @@ import type { IPage } from '../../types.js'; import { render } from '../template.js'; -function isRecord(value: unknown): value is Record { - return typeof value === 'object' && value !== null && !Array.isArray(value); -} +import { isRecord, mapConcurrent } from '../../utils.js'; -/** Simple async concurrency limiter */ -async function mapConcurrent(items: T[], limit: number, fn: (item: T, index: number) => Promise): Promise { - const results: R[] = new Array(items.length); - let index = 0; - async function worker() { - while (index < items.length) { - const i = index++; - results[i] = await fn(items[i], i); - } - } - - const workers = Array.from({ length: Math.min(limit, items.length) }, () => worker()); - await Promise.all(workers); - return results; -} /** Single URL fetch helper */ async function fetchSingle( diff --git a/src/pipeline/steps/transform.ts b/src/pipeline/steps/transform.ts index 1b034c17..3b6a82c5 100644 --- a/src/pipeline/steps/transform.ts +++ b/src/pipeline/steps/transform.ts @@ -5,9 +5,7 @@ import type { IPage } from '../../types.js'; import { render, evalExpr } from '../template.js'; -function isRecord(value: unknown): value is Record { - return typeof value === 'object' && value !== null && !Array.isArray(value); -} +import { isRecord } from '../../utils.js'; export async function stepSelect(_page: IPage | null, params: unknown, data: unknown, args: Record): Promise { const pathStr = String(render(params, { args, data })); diff --git a/src/pipeline/template.ts b/src/pipeline/template.ts index 04e6770a..8535bba0 100644 --- a/src/pipeline/template.ts +++ b/src/pipeline/template.ts @@ -11,9 +11,7 @@ export interface RenderContext { index?: number; } -function isRecord(value: unknown): value is Record { - return typeof value === 'object' && value !== null && !Array.isArray(value); -} +import { isRecord } from '../utils.js'; export function render(template: unknown, ctx: RenderContext): unknown { if (typeof template !== 'string') return template; diff --git a/src/utils.ts b/src/utils.ts new file mode 100644 index 00000000..d69301e0 --- /dev/null +++ b/src/utils.ts @@ -0,0 +1,38 @@ +/** + * Shared utility functions used across the codebase. + */ + +/** Type guard: checks if a value is a non-null, non-array object. */ +export function isRecord(value: unknown): value is Record { + return typeof value === 'object' && value !== null && !Array.isArray(value); +} + +/** Simple async concurrency limiter. */ +export async function mapConcurrent( + items: T[], + limit: number, + fn: (item: T, index: number) => Promise, +): Promise { + const results: R[] = new Array(items.length); + let index = 0; + + async function worker() { + while (index < items.length) { + const i = index++; + results[i] = await fn(items[i], i); + } + } + + const workers = Array.from({ length: Math.min(limit, items.length) }, () => worker()); + await Promise.all(workers); + return results; +} + +/** Save a base64-encoded string to a file, creating parent directories as needed. */ +export async function saveBase64ToFile(base64: string, filePath: string): Promise { + const fs = await import('node:fs'); + const path = await import('node:path'); + const dir = path.dirname(filePath); + await fs.promises.mkdir(dir, { recursive: true }); + await fs.promises.writeFile(filePath, Buffer.from(base64, 'base64')); +} diff --git a/src/validate.ts b/src/validate.ts index be454b7b..bfa912ed 100644 --- a/src/validate.ts +++ b/src/validate.ts @@ -34,9 +34,7 @@ interface ValidatedYamlCliDefinition { args?: Record; } -function isRecord(value: unknown): value is Record { - return typeof value === 'object' && value !== null && !Array.isArray(value); -} +import { isRecord } from './utils.js'; export function validateClisWithTarget(dirs: string[], target?: string): ValidationReport { From 9e783d321fb83461cff3fd5a0ae84311bd0628a4 Mon Sep 17 00:00:00 2001 From: jackwener Date: Tue, 24 Mar 2026 15:02:27 +0800 Subject: [PATCH 2/2] fix: handle apple-podcasts timeout in E2E guard When the apple-podcasts top command times out on CI, the child process is killed with empty stderr. The existing guard didn't catch this case, causing a flaky E2E failure. Co-Authored-By: Claude Opus 4.6 --- tests/e2e/public-commands.test.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/e2e/public-commands.test.ts b/tests/e2e/public-commands.test.ts index 7e9209a2..22a2f388 100644 --- a/tests/e2e/public-commands.test.ts +++ b/tests/e2e/public-commands.test.ts @@ -16,7 +16,8 @@ function isExpectedChineseSiteRestriction(code: number, stderr: string): boolean function isExpectedApplePodcastsRestriction(code: number, stderr: string): boolean { if (code === 0) return false; - return /Error \[FETCH_ERROR\]: (Charts API HTTP \d+|Unable to reach Apple Podcasts charts)/.test(stderr); + return /Error \[FETCH_ERROR\]: (Charts API HTTP \d+|Unable to reach Apple Podcasts charts)/.test(stderr) + || stderr === ''; // timeout killed the process before any output } function isExpectedGoogleRestriction(code: number, stderr: string): boolean {