feat(browser): remove silent html truncation, add --as json (#1102)

* feat(browser): remove silent html truncation, add --as json tree output `browser get html` had two agent-hostile defaults: 1. A silent 50000-char cap on the returned HTML — agents that got a truncated page had no signal they were looking at half the DOM. 2. Only raw HTML string output, forcing agents to re-parse for structured extraction. Changes: - Default output is now the full outerHTML, no truncation - `--max <n>` opts in to a character cap; when the cap actually trips, the HTML is prepended with `` so agents always see the signal - `--as json` returns `{selector, matched, tree}` where `tree` is `{tag, attrs, text, children}` recursively. `matched` is the full count of selector matches so agents know when more elements exist beyond the first. `text` is the node's own direct text children, whitespace-collapsed; child elements live in `children`. - `--selector` not matching any element now emits structured `{error:{code:"selector_not_found", ...}}` with a non-zero exit code, in both raw and json modes (was `(empty)` stdout previously, indistinguishable from empty element) - Invalid `--as` / negative `--max` emit structured `invalid_format` / `invalid_max` error codes Extracted the tree serializer as `src/browser/html-tree.ts` so the JS expression can be unit-tested against a DOM stub. * fix(browser get html): structured errors for invalid selector & strict --max Both edges previously bypassed the structured-error contract introduced in #1102, which agents rely on for branching: - Invalid CSS selector: querySelector(All) would throw SyntaxError through page.evaluate into the generic exception path. Wrap the lookup in try/catch inside page context for both raw and --as json paths; surface as {error:{code:"invalid_selector", message}} + non-zero exit. - --max validation: parseInt silently accepted "1.5" -> 1 and "10abc" -> 10. Switch to a strict /^\\d+$/ check so fractional, negative, and non-numeric values all return {error:{code:"invalid_max"}}; validation runs up front so bad values never reach the page. Covered by new unit tests in cli.test.ts (fractional, non-numeric, invalid selector on raw + json) and html-tree.test.ts (SyntaxError -> invalidSelector envelope). Co-authored-by: freemandealer <freeman.zhang1992@gmail.com> --------- Co-authored-by: freemandealer <freeman.zhang1992@gmail.com>
2026-04-21 02:16:44 +08:00
parent 7fd8bd6fdc
commit 6cf5cb2f25
4 changed files with 441 additions and 3 deletions
@@ -0,0 +1,108 @@
+import { describe, expect, it } from 'vitest';
+import { buildHtmlTreeJs, type HtmlTreeResult } from './html-tree.js';
+
+/**
+ * The serializer runs in a page context via `page.evaluate`. In unit tests we
+ * substitute `document` with a minimal stub that mirrors the DOM surface used
+ * by the expression, then Function-eval the returned JS.
+ */
+function runTreeJs(root: unknown, selectorMatches: unknown[], selector: string | null): HtmlTreeResult {
+    const js = buildHtmlTreeJs({ selector });
+    const fakeDocument = {
+        querySelectorAll: () => selectorMatches,
+        documentElement: root,
+    };
+    const fn = new Function('document', `return ${js};`);
+    return fn(fakeDocument) as HtmlTreeResult;
+}
+
+function runTreeJsInvalid(selector: string, errorMessage: string): unknown {
+    const js = buildHtmlTreeJs({ selector });
+    const fakeDocument = {
+        querySelectorAll: () => { const e = new Error(errorMessage); e.name = 'SyntaxError'; throw e; },
+        documentElement: null,
+    };
+    const fn = new Function('document', `return ${js};`);
+    return fn(fakeDocument);
+}
+
+function el(tag: string, attrs: Record<string, string>, children: Array<ChildOf>): FakeEl {
+    return {
+        nodeType: 1,
+        tagName: tag.toUpperCase(),
+        attributes: Object.entries(attrs).map(([name, value]) => ({ name, value })),
+        childNodes: children,
+    };
+}
+
+function txt(value: string): FakeText { return { nodeType: 3, nodeValue: value }; }
+
+type FakeEl = { nodeType: 1; tagName: string; attributes: Array<{ name: string; value: string }>; childNodes: Array<ChildOf> };
+type FakeText = { nodeType: 3; nodeValue: string };
+type ChildOf = FakeEl | FakeText;
+
+describe('buildHtmlTreeJs', () => {
+    it('serializes a simple element into {tag, attrs, text, children}', () => {
+        const root = el('div', { class: 'hero', id: 'x' }, [txt('Hello')]);
+        const result = runTreeJs(root, [root], null);
+        expect(result.selector).toBeNull();
+        expect(result.matched).toBe(1);
+        expect(result.tree).toEqual({
+            tag: 'div',
+            attrs: { class: 'hero', id: 'x' },
+            text: 'Hello',
+            children: [],
+        });
+    });
+
+    it('collapses whitespace in direct text content only', () => {
+        const root = el('p', {}, [
+            txt('  line  \n  one  '),
+            el('span', {}, [txt('inner text')]),
+            txt('\tline two\t'),
+        ]);
+        const result = runTreeJs(root, [root], null);
+        expect(result.tree?.text).toBe('line one line two');
+        expect(result.tree?.children[0].text).toBe('inner text');
+    });
+
+    it('recurses into element children and preserves their attrs', () => {
+        const root = el('ul', { role: 'list' }, [
+            el('li', { 'data-id': '1' }, [txt('first')]),
+            el('li', { 'data-id': '2' }, [txt('second')]),
+        ]);
+        const result = runTreeJs(root, [root], null);
+        expect(result.tree?.children).toHaveLength(2);
+        expect(result.tree?.children[0]).toEqual({
+            tag: 'li',
+            attrs: { 'data-id': '1' },
+            text: 'first',
+            children: [],
+        });
+    });
+
+    it('returns matched=N and serializes only the first match', () => {
+        const first = el('article', { id: 'a' }, [txt('first')]);
+        const second = el('article', { id: 'b' }, [txt('second')]);
+        const result = runTreeJs(null, [first, second], 'article');
+        expect(result.matched).toBe(2);
+        expect(result.tree?.attrs.id).toBe('a');
+    });
+
+    it('returns tree=null and matched=0 when selector matches nothing', () => {
+        const result = runTreeJs(null, [], '.nothing');
+        expect(result.matched).toBe(0);
+        expect(result.tree).toBeNull();
+    });
+
+    it('catches SyntaxError from querySelectorAll and returns {invalidSelector:true, reason}', () => {
+        const result = runTreeJsInvalid('##$@@', "'##$@@' is not a valid selector") as {
+            selector: string;
+            invalidSelector: boolean;
+            reason: string;
+        };
+        expect(result.invalidSelector).toBe(true);
+        expect(result.selector).toBe('##$@@');
+        expect(result.reason).toContain('not a valid selector');
+    });
+});
@@ -0,0 +1,78 @@
+/**
+ * Client-side HTML → structured tree serializer.
+ *
+ * Returned as a JS string that gets passed to `page.evaluate`. The expression
+ * walks the DOM subtree rooted at the first selector match (or documentElement
+ * when no selector is given) and emits a compact `{tag, attrs, text, children}`
+ * tree for agents to consume instead of re-parsing raw HTML.
+ *
+ * Text handling: `text` is the concatenated text of direct text children only,
+ * whitespace-collapsed. Nested element text is left inside `children[].text`.
+ * Ordering between text and elements is not preserved — agents that need it
+ * should fall back to raw HTML mode.
+ */
+
+export interface BuildHtmlTreeJsOptions {
+    /** CSS selector to scope the tree; unscoped = documentElement */
+    selector?: string | null;
+}
+
+/**
+ * Returns a JS expression string. When evaluated in a page context the
+ * expression resolves to either
+ *   `{selector, matched: number, tree: HtmlNode | null}` on success, or
+ *   `{selector, invalidSelector: true, reason}` when `querySelectorAll`
+ *   throws a `SyntaxError` for an unparseable selector.
+ *
+ * Callers must branch on `invalidSelector` to convert it into the CLI's
+ * `invalid_selector` structured error; otherwise the browser-level exception
+ * would bubble out of `page.evaluate` and bypass the structured-error
+ * contract that agents rely on.
+ */
+export function buildHtmlTreeJs(opts: BuildHtmlTreeJsOptions = {}): string {
+    const selectorLiteral = opts.selector ? JSON.stringify(opts.selector) : 'null';
+    return `(() => {
+  const selector = ${selectorLiteral};
+  let matches;
+  if (selector) {
+    try { matches = document.querySelectorAll(selector); }
+    catch (e) {
+      return { selector: selector, invalidSelector: true, reason: (e && e.message) || String(e) };
+    }
+  } else {
+    matches = [document.documentElement];
+  }
+  const matched = matches.length;
+  const root = matches[0] || null;
+  function serialize(el) {
+    if (!el || el.nodeType !== 1) return null;
+    const attrs = {};
+    for (const a of el.attributes) attrs[a.name] = a.value;
+    let text = '';
+    const children = [];
+    for (const n of el.childNodes) {
+      if (n.nodeType === 3) {
+        text += n.nodeValue;
+      } else if (n.nodeType === 1) {
+        const child = serialize(n);
+        if (child) children.push(child);
+      }
+    }
+    return { tag: el.tagName.toLowerCase(), attrs, text: text.replace(/\\s+/g, ' ').trim(), children };
+  }
+  return { selector: selector, matched: matched, tree: root ? serialize(root) : null };
+})()`;
+}
+
+export interface HtmlNode {
+    tag: string;
+    attrs: Record<string, string>;
+    text: string;
+    children: HtmlNode[];
+}
+
+export interface HtmlTreeResult {
+    selector: string | null;
+    matched: number;
+    tree: HtmlNode | null;
+}
@@ -442,6 +442,166 @@ describe('browser network command', () => {
  });
 });

+describe('browser get html command', () => {
+  const consoleLogSpy = vi.spyOn(console, 'log').mockImplementation(() => {});
+
+  function lastLogArg(): unknown {
+    const calls = consoleLogSpy.mock.calls;
+    if (calls.length === 0) throw new Error('expected console.log call');
+    return calls[calls.length - 1][0];
+  }
+  function lastJsonLog(): any {
+    const arg = lastLogArg();
+    if (typeof arg !== 'string') throw new Error(`expected string arg, got ${typeof arg}`);
+    return JSON.parse(arg);
+  }
+
+  beforeEach(() => {
+    process.exitCode = undefined;
+    process.env.OPENCLI_CACHE_DIR = fs.mkdtempSync(path.join(os.tmpdir(), 'opencli-html-'));
+    consoleLogSpy.mockClear();
+    mockBrowserConnect.mockClear();
+    mockBrowserClose.mockReset().mockResolvedValue(undefined);
+
+    browserState.page = {
+      setActivePage: vi.fn(),
+      getActivePage: vi.fn().mockReturnValue('tab-1'),
+      tabs: vi.fn().mockResolvedValue([{ page: 'tab-1', active: true }]),
+      evaluate: vi.fn(),
+    } as unknown as IPage;
+  });
+
+  it('returns full outerHTML by default with no truncation', async () => {
+    const big = '<div>' + 'x'.repeat(100_000) + '</div>';
+    (browserState.page!.evaluate as any).mockResolvedValueOnce({ kind: 'ok', html: big });
+    const program = createProgram('', '');
+
+    await program.parseAsync(['node', 'opencli', 'browser', 'get', 'html']);
+
+    expect(lastLogArg()).toBe(big);
+  });
+
+  it('caps output with --max and prepends a visible truncation marker', async () => {
+    const big = '<div>' + 'x'.repeat(500) + '</div>';
+    (browserState.page!.evaluate as any).mockResolvedValueOnce({ kind: 'ok', html: big });
+    const program = createProgram('', '');
+
+    await program.parseAsync(['node', 'opencli', 'browser', 'get', 'html', '--max', '100']);
+
+    const out = String(lastLogArg());
+    expect(out.startsWith('<!-- opencli: truncated 100 of')).toBe(true);
+    expect(out.length).toBeGreaterThan(100);
+    expect(out.length).toBeLessThan(big.length);
+  });
+
+  it('rejects negative --max with invalid_max error', async () => {
+    const program = createProgram('', '');
+
+    await program.parseAsync(['node', 'opencli', 'browser', 'get', 'html', '--max', '-1']);
+
+    expect(lastJsonLog().error.code).toBe('invalid_max');
+    expect(process.exitCode).toBeDefined();
+    expect(browserState.page!.evaluate).not.toHaveBeenCalled();
+  });
+
+  it('rejects fractional --max with invalid_max error', async () => {
+    const program = createProgram('', '');
+
+    await program.parseAsync(['node', 'opencli', 'browser', 'get', 'html', '--max', '1.5']);
+
+    expect(lastJsonLog().error.code).toBe('invalid_max');
+    expect(process.exitCode).toBeDefined();
+    expect(browserState.page!.evaluate).not.toHaveBeenCalled();
+  });
+
+  it('rejects non-numeric --max (e.g. "10abc") with invalid_max error', async () => {
+    const program = createProgram('', '');
+
+    await program.parseAsync(['node', 'opencli', 'browser', 'get', 'html', '--max', '10abc']);
+
+    expect(lastJsonLog().error.code).toBe('invalid_max');
+    expect(process.exitCode).toBeDefined();
+    expect(browserState.page!.evaluate).not.toHaveBeenCalled();
+  });
+
+  it('--as json returns structured tree envelope', async () => {
+    (browserState.page!.evaluate as any).mockResolvedValueOnce({
+      selector: '.hero',
+      matched: 1,
+      tree: { tag: 'div', attrs: { class: 'hero' }, text: 'Hi', children: [] },
+    });
+    const program = createProgram('', '');
+
+    await program.parseAsync(['node', 'opencli', 'browser', 'get', 'html', '--selector', '.hero', '--as', 'json']);
+
+    const out = lastJsonLog();
+    expect(out.matched).toBe(1);
+    expect(out.tree.tag).toBe('div');
+    expect(out.tree.attrs.class).toBe('hero');
+  });
+
+  it('--as json emits selector_not_found when matched is 0', async () => {
+    (browserState.page!.evaluate as any).mockResolvedValueOnce({ selector: '.missing', matched: 0, tree: null });
+    const program = createProgram('', '');
+
+    await program.parseAsync(['node', 'opencli', 'browser', 'get', 'html', '--selector', '.missing', '--as', 'json']);
+
+    expect(lastJsonLog().error.code).toBe('selector_not_found');
+    expect(process.exitCode).toBeDefined();
+  });
+
+  it('raw mode emits selector_not_found when the selector matches nothing', async () => {
+    (browserState.page!.evaluate as any).mockResolvedValueOnce({ kind: 'ok', html: null });
+    const program = createProgram('', '');
+
+    await program.parseAsync(['node', 'opencli', 'browser', 'get', 'html', '--selector', '.missing']);
+
+    expect(lastJsonLog().error.code).toBe('selector_not_found');
+    expect(process.exitCode).toBeDefined();
+  });
+
+  it('raw mode emits invalid_selector when the page rejects the selector syntax', async () => {
+    (browserState.page!.evaluate as any).mockResolvedValueOnce({
+      kind: 'invalid_selector',
+      reason: "'##$@@' is not a valid selector",
+    });
+    const program = createProgram('', '');
+
+    await program.parseAsync(['node', 'opencli', 'browser', 'get', 'html', '--selector', '##$@@']);
+
+    const err = lastJsonLog().error;
+    expect(err.code).toBe('invalid_selector');
+    expect(err.message).toContain('##$@@');
+    expect(err.message).toContain('not a valid selector');
+    expect(process.exitCode).toBeDefined();
+  });
+
+  it('--as json emits invalid_selector when the page rejects the selector syntax', async () => {
+    (browserState.page!.evaluate as any).mockResolvedValueOnce({
+      selector: '##$@@',
+      invalidSelector: true,
+      reason: "'##$@@' is not a valid selector",
+    });
+    const program = createProgram('', '');
+
+    await program.parseAsync(['node', 'opencli', 'browser', 'get', 'html', '--selector', '##$@@', '--as', 'json']);
+
+    const err = lastJsonLog().error;
+    expect(err.code).toBe('invalid_selector');
+    expect(err.message).toContain('##$@@');
+    expect(process.exitCode).toBeDefined();
+  });
+
+  it('rejects unknown --as format with invalid_format error', async () => {
+    const program = createProgram('', '');
+
+    await program.parseAsync(['node', 'opencli', 'browser', 'get', 'html', '--as', 'yaml']);
+
+    expect(lastJsonLog().error.code).toBe('invalid_format');
+    expect(process.exitCode).toBeDefined();
+  });
+});
+
 describe('findPackageRoot', () => {
  it('walks up from dist/src to the package root', () => {
    const packageRoot = path.join('repo-root');
@@ -25,6 +25,7 @@ import { resolveTargetJs, getTextResolvedJs, getValueResolvedJs, getAttributesRe
 import { inferShape } from './browser/shape.js';
 import { assignKeys } from './browser/network-key.js';
 import { DEFAULT_TTL_MS, findEntry, loadNetworkCache, saveNetworkCache, type CachedNetworkEntry } from './browser/network-cache.js';
+import { buildHtmlTreeJs, type HtmlTreeResult } from './browser/html-tree.js';
 import { daemonStatus, daemonStop } from './commands/daemon.js';
 import { log } from './logger.js';

@@ -546,11 +547,102 @@ export function createProgram(BUILTIN_CLIS: string, USER_CLIS: string): Command
      console.log(val ?? '(empty)');
    }));

-  addBrowserTabOption(get.command('html').option('--selector <css>', 'CSS selector scope').description('Page HTML (or scoped)'))
+  addBrowserTabOption(
+    get.command('html')
+      .option('--selector <css>', 'CSS selector scope (first match)')
+      .option('--as <format>', 'Output format: "html" (default) or "json" for structured tree', 'html')
+      .option('--max <n>', 'Max characters of raw HTML to return (0 = unlimited)', '0')
+      .description('Page HTML (or scoped); use --as json for a {tag, attrs, text, children} tree'),
+  )
    .action(browserAction(async (page, opts) => {
+      const format = String(opts.as || 'html').toLowerCase();
+      if (format !== 'html' && format !== 'json') {
+        console.log(JSON.stringify({ error: { code: 'invalid_format', message: `--as must be "html" or "json", got "${opts.as}"` } }, null, 2));
+        process.exitCode = EXIT_CODES.USAGE_ERROR;
+        return;
+      }
+
+      // `--max` is validated up-front (before touching the page) so a bad value
+      // gets the same structured error regardless of selector/format path.
+      const rawMax = String(opts.max ?? '0');
+      if (!/^\d+$/.test(rawMax)) {
+        console.log(JSON.stringify({ error: { code: 'invalid_max', message: `--max must be a non-negative integer, got "${opts.max}"` } }, null, 2));
+        process.exitCode = EXIT_CODES.USAGE_ERROR;
+        return;
+      }
+      const max = Number.parseInt(rawMax, 10);
+
+      if (format === 'json') {
+        const js = buildHtmlTreeJs({ selector: opts.selector ?? null });
+        const result = await page.evaluate(js) as HtmlTreeResult | { selector: string; invalidSelector: true; reason: string } | null;
+        if (result && typeof result === 'object' && 'invalidSelector' in result && result.invalidSelector) {
+          console.log(JSON.stringify({
+            error: { code: 'invalid_selector', message: `Selector "${opts.selector}" is not a valid CSS selector: ${result.reason}` },
+          }, null, 2));
+          process.exitCode = EXIT_CODES.USAGE_ERROR;
+          return;
+        }
+        const ok = result as HtmlTreeResult | null;
+        if (!ok || ok.matched === 0) {
+          console.log(JSON.stringify({
+            error: {
+              code: 'selector_not_found',
+              message: opts.selector
+                ? `Selector "${opts.selector}" matched 0 elements.`
+                : 'Page has no documentElement.',
+            },
+          }, null, 2));
+          process.exitCode = EXIT_CODES.USAGE_ERROR;
+          return;
+        }
+        console.log(JSON.stringify(ok, null, 2));
+        return;
+      }
+
+      // Raw HTML path — unbounded by default; --max optionally caps with a visible marker.
+      // Selector lookup is wrapped in try/catch inside page context so an invalid
+      // selector returns a structured signal instead of throwing through page.evaluate.
      const sel = opts.selector ? JSON.stringify(opts.selector) : 'null';
-      const html = await page.evaluate(`(${sel} ? document.querySelector(${sel})?.outerHTML : document.documentElement.outerHTML)?.slice(0, 50000)`);
-      console.log(html ?? '(empty)');
+      const rawResult = await page.evaluate(
+        `(() => {
+          const s = ${sel};
+          if (s) {
+            try {
+              const el = document.querySelector(s);
+              return { kind: 'ok', html: el ? el.outerHTML : null };
+            } catch (e) {
+              return { kind: 'invalid_selector', reason: (e && e.message) || String(e) };
+            }
+          }
+          return { kind: 'ok', html: document.documentElement ? document.documentElement.outerHTML : null };
+        })()`,
+      ) as { kind: 'ok'; html: string | null } | { kind: 'invalid_selector'; reason: string };
+
+      if (rawResult.kind === 'invalid_selector') {
+        console.log(JSON.stringify({
+          error: { code: 'invalid_selector', message: `Selector "${opts.selector}" is not a valid CSS selector: ${rawResult.reason}` },
+        }, null, 2));
+        process.exitCode = EXIT_CODES.USAGE_ERROR;
+        return;
+      }
+      const html = rawResult.html;
+
+      if (html === null) {
+        if (opts.selector) {
+          console.log(JSON.stringify({
+            error: { code: 'selector_not_found', message: `Selector "${opts.selector}" matched 0 elements.` },
+          }, null, 2));
+          process.exitCode = EXIT_CODES.USAGE_ERROR;
+          return;
+        }
+        console.log('(empty)');
+        return;
+      }
+      if (max > 0 && html.length > max) {
+        console.log(`<!-- opencli: truncated ${max} of ${html.length} chars; re-run without --max (or --max 0) for full -->\n${html.slice(0, max)}`);
+        return;
+      }
+      console.log(html);
    }));

  addBrowserTabOption(get.command('attributes').argument('<index>', 'Element index').description('Element attributes'))