[STG-2503] fix(evals): generate core browse CLI contract from oclif manifest, fix stale commands#2331
Open
shrey150 wants to merge 3 commits into
Open
[STG-2503] fix(evals): generate core browse CLI contract from oclif manifest, fix stale commands#2331shrey150 wants to merge 3 commits into
shrey150 wants to merge 3 commits into
Conversation
…x stale commands core/tools/browse_cli.ts hand-duplicated its own copy of "which browse CLI commands/flags exist" separately from framework/claudeCodeToolAdapter.ts, with no link back to the CLI's own command definitions. That copy had drifted badly: newpage/click_xy/hover/scroll/tab_switch/tab_close/pages/env no longer exist (renamed to tab new/mouse click/mouse hover/mouse scroll/ tab switch/tab close/tab list, or removed outright), open/wait used removed short flags (-t/-s instead of --timeout/--state), and fill's --no-press-enter and screenshot's -f/-t/-q are all "Nonexistent flag" errors against the current CLI (verified live against a local build). Fix: a generator script reads packages/cli/oclif.manifest.json (which oclif already emits at CLI build time from each command's own `static flags`) into a committed, typed contract module. browse_cli.ts now calls the corrected commands and derives its modeless-command list (commands that reject --local/--remote) from the generated contract instead of a hand-typed Set -- which also fixes a latent bug where `cdp`/`daemon` were never in that hand-typed list to begin with. A staleness-guard test fails if the committed contract ever diverges from a fresh manifest build. framework/claudeCodeToolAdapter.ts is intentionally untouched here: its equivalent hand-typed stop/status check only exists on the still-unmerged #2299, so generate-ifying it here would either redo #2299's fix or edit code that doesn't exist yet on main. Swapping it to the now-shared BROWSE_CLI_MODELESS_COMMANDS is a trivial follow-up once #2299 merges.
|
Contributor
There was a problem hiding this comment.
3 issues found across 5 files
Confidence score: 3/5
- In
packages/evals/scripts/generateBrowseCliContract.ts, theimport.meta.urlentrypoint check is not Windows-safe, somain()may never run there and contract generation can silently fail for Windows contributors/CI — switch to the existingruntimePaths.tshelper for a cross-platform invocation check before merging. - In
packages/evals/core/tools/browse_cli.ts, mode-flag injection now affects allbrowse_cliinvocations, but there’s no focused argv test for modeful vs. modeless command behavior; this leaves a user-facing regression path unguarded — add a targeted unit test around argv construction to de-risk merge. - In
packages/evals/scripts/generateBrowseCliContract.ts, Windows backslashes in generated repo-relative paths can create inconsistent logs/errors and copy-paste friction against the repo’s slash convention — normalize output with template paths orpath.posix.joinbefore merging.
Architecture diagram
sequenceDiagram
participant Dev as Developer
participant CLI as Browse CLI (oclif build)
participant Manifest as oclif.manifest.json
participant Gen as generateBrowseCliContract.ts
participant Contract as browseCliContract.generated.ts
participant BrowseCli as browse_cli.ts (BrowseCliRuntime)
participant EvalTest as Core Evaluation Test
participant GuardTest as Staleness Guard Test
Note over Dev,GuardTest: PR: Generate CLI Contract from oclif Manifest
Dev->>CLI: pnpm --dir packages/cli build
CLI->>CLI: Build commands from src/commands/**
CLI->>Manifest: Write oclif.manifest.json (JSON manifest of commands/flags)
Dev->>Gen: pnpm --dir packages/evals generate:browse-cli-contract
Gen->>Manifest: Read manifest JSON
Manifest-->>Gen: Raw command/flag data
Gen->>Gen: Parse & transform to BrowseCliContract (commandId → {flags: [...]})
Gen->>Contract: Write browseCliContract.generated.ts (typed module)
Note over Contract: Exports: BROWSE_CLI_CONTRACT, BrowseCliCommandId,<br/>commandAcceptsFlag(), BROWSE_CLI_MODELESS_COMMANDS
EvalTest->>BrowseCli: Start session (ToolStartResult)
BrowseCli->>BrowseCli: Construct BrowseCliRuntime(session, modeFlag)
alt Modeless command (e.g., stop, status, cdp, daemon)
BrowseCli->>Contract: BROWSE_CLI_MODELESS_COMMANDS.includes(commandId)
Contract-->>BrowseCli: true
BrowseCli->>CLI: execFile(entrypoint, [argv, --json, --session, ...])
else Commands accepting --local/--remote
BrowseCli->>Contract: BROWSE_CLI_MODELESS_COMMANDS.includes(commandId)
Contract-->>BrowseCli: false
BrowseCli->>CLI: execFile(entrypoint, [argv, --json, --local, --session, ...])
end
CLI-->>BrowseCli: stdout: JSON (tabs: [...], base64 string, etc.)
BrowseCli->>CLI: Command: tab:new [url]
CLI-->>BrowseCli: {targetId, url, index} (no 'created' field)
BrowseCli->>CLI: Command: tab:list
CLI-->>BrowseCli: {tabs: [{index, url, targetId}]}
BrowseCli->>CLI: Command: mouse:click x y
BrowseCli->>CLI: Command: mouse:hover x y
BrowseCli->>CLI: Command: mouse:scroll x y dx dy
BrowseCli->>CLI: Command: open url --timeout <ms>
BrowseCli->>CLI: Command: wait selector <s> --timeout <ms> --state <state>
BrowseCli->>CLI: Command: wait load <state> --timeout <ms>
BrowseCli->>CLI: Command: fill <s> <text> (no --no-press-enter flag)
BrowseCli->>CLI: Command: screenshot --base64 --full-page --type <type> --quality <quality>
CLI-->>BrowseCli: {base64: "..."}
BrowseCli->>BrowseCli: Buffer.from(base64, "base64")
Note over GuardTest: Staleness Guard (CI)
GuardTest->>Manifest: Read fresh manifest
Manifest-->>GuardTest: Raw JSON
GuardTest->>GuardTest: buildBrowseCliContractFromManifest()
GuardTest->>Contract: deep-equal fresh contract vs committed BROWSE_CLI_CONTRACT
alt Contracts match
GuardTest-->>GuardTest: Test passes
else Contracts differ
GuardTest-->>GuardTest: Test fails → CI fails → regenerate required
end
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
- Use runtimePaths.ts's isMainModule() instead of a manual import.meta.url comparison for the generator's CLI-entry check -- the manual comparison silently never runs main() on Windows. - Normalize generator log/error message paths to forward slashes for display consistency (fs calls themselves are unaffected). - Add a focused unit test for BrowseCliRuntime's mode-flag injection: asserts --local is appended for a modeful command (tab list), --remote for a BROWSERBASE session, and omitted entirely for a modeless command (stop) -- the exact behavior this PR's contract generation drives.
shrey150
commented
Jul 8, 2026
Per review: the removed-env-subcommand explanation is changelog content, not a non-obvious current invariant.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Follow-up to Miguel's review comment on #2299: #2299 (comment)
#2299 fixed the browse CLI v0.9.1 contract for the
benchpath (framework/claudeCodeToolAdapter.ts, shared by theclaude_code/codexexternal harnesses). This PR does the deferredcorehalf:core/tools/browse_cli.ts, thebrowse_clitool-surface implementation used only by the deterministiccoretool-vs-tool comparison suite (core/contracts/tool.ts'sToolSurfaceunion:browse_cli,playwright_code,understudy_code,cdp_code,playwright_mcp,chrome_devtools_mcp).The actual drift (verified live against a local
packages/clibuild)core/tools/browse_cli.tshand-encoded the CLI's command/flag surface independently offramework/claudeCodeToolAdapter.ts, with no link back to the CLI's real command definitions. Both files drift independently; this file's copy was far more stale than Miguel's 4 illustrative examples suggested — full list found by auditing every command this file invokes againstpackages/cli/src/commands/**:start()env local/env remotenewPage()newpage [url]tab new [url]fetchPages()pagestab list(returns{tabs: [...]}, not{pages: [...]})selectPage()tab_switch <i>tab switch <i>closePage()tab_close <i>tab close <i>click(x,y)click_xy <x> <y>mouse click <x> <y>hover(x,y)hover <x> <y>mouse hover <x> <y>scroll()scroll <x> <y> <dx> <dy>mouse scroll <x> <y> <dx> <dy>goto()open <url> -t <ms>open <url> --timeout <ms>wait()(selector)wait selector <s> -t <ms> -s <state>wait selector <s> --timeout <ms> --state <state>wait()(load)wait load <state> -t <ms>wait load <state> --timeout <ms>type()(selector)fill <s> <text> --no-press-enterfill <s> <text>(Enter-unpressed is the default)screenshot()screenshot -f -t <type> -q <quality>screenshot --full-page --type <type> --quality <quality> --base64--base64the CLI now writes a file to disk and returns its path instead of image bytes — a silent behavior change, not just a flag renameAlso fixed:
tab.new's old return type declared acreatedfield that was never real (Miguel's note) — dropped it.The fix: generate the contract instead of hand-typing it twice
packages/cliis built with oclif, which already emitspackages/cli/oclif.manifest.jsonat build time — a JSON manifest of every command and its flags, generated straight from each command class's ownstatic flags.packages/evals/scripts/generateBrowseCliContract.tsreads that manifest into a committed, typed module (core/tools/browseCliContract.generated.ts) exportingBROWSE_CLI_CONTRACT,commandAcceptsFlag(), andBROWSE_CLI_MODELESS_COMMANDS.core/tools/browse_cli.tsnow derives its "which commands reject--local/--remote" list from this generated contract instead of a hand-typedSet(["stop","status"])— which incidentally surfaced a second latent bug:cdpanddaemonalso lack--local/--remotein the real CLI and were never in the old hand-typed list either (harmless today since this file never calls them, but a generated list is now correct by construction instead of by luck).A staleness-guard test (
tests/core/browseCliContract.test.ts) rebuilds the contract from a fresh manifest and deep-equals it against the committed file, so apackages/clichange that isn't followed by regenerating fails CI immediately instead of surfacing as a runtime "Nonexistent flag" days later.Not in scope:
framework/claudeCodeToolAdapter.ts's equivalent hand-typedstop/statuscheck only exists on the still-unmerged #2299 (this branch is offorigin/main, which doesn't have #2299's fix yet). Generate-ifying it here would either redo #2299's fix or edit code that doesn't exist onmainyet. Swapping it toBROWSE_CLI_MODELESS_COMMANDSis a one-line follow-up once #2299 merges.No changeset:
@browserbasehq/stagehand-evalsisprivate: true, never published.E2E Test Matrix
pnpm --dir packages/cli buildthenpnpm --dir packages/evals generate:browse-cli-contractpackages/cli/oclif.manifest.json; generator wrotebrowseCliContract.generated.tspnpm --dir packages/evals lint(prettier + eslint + typecheck)pnpm --dir packages/evals test:unit(vitest run)<local build>ofpackages/cli(click_xy,newpage,hover,scroll,tab_switch,tab_close,env,pages,open ... -t,wait ... -t -s,screenshot -f,fill ... --no-press-enter)"<cmd>" is not a browse commandorError: Nonexistent flagmain.BrowseCliTooldriven directly (not mocked) against<local build>ofpackages/cliinLOCALmode:newPage,wait(all 3 kinds),title/url(get),listPages/selectPage/closePage(tab list/switch/close),click/hover/scrollat coords (mouse click/hover/scroll),click/pressby selector,typeviafill(no flag),screenshotwith--base64,represent(snapshot)https://example.com: real tab targetIds,title: "Example Domain", valid PNG bytes (15988bytes, correct magic number) fromscreenshot, snapshot content returned, session cleaned up viastop --forcewith no orphaned daemon process afterwardorigin/mainand touches onlycore/tools/browse_cli.ts+ new files;framework/claudeCodeToolAdapter.tsuntouchedgit diff --statconfirms 0 lines changed in that fileLinear
STG-2503 — https://linear.app/browserbase/issue/STG-2503/fixevals-generate-core-browse-cli-contract-from-oclif-manifest-fix
Summary by cubic
Generates the core
browse_clicontract from theoclifmanifest and updatescore/tools/browse_cli.tsto match the current CLI, fixing stale commands/flags used by core evals. Addresses STG-2503 with a CI guard and a unit test for mode flag injection.Bug Fixes
--local/--remote(removedenv).tab new/list/switch/close,mouse click/hover/scroll(replacednewpage,pages,tab_switch,tab_close,click_xy,hover,scroll).--timeout/--state; removed--no-press-enter.screenshotreturns bytes via--base64(supports--full-page/--type/--quality).tab list→{tabs: [...]}; dropped nonexistentcreated.Refactors
packages/evals/scripts/generateBrowseCliContract.tsto readpackages/cli/oclif.manifest.jsonand emitbrowseCliContract.generated.ts; addedpnpm generate:browse-cli-contract.browse_cli.tsderives modeless commands from the contract and appends mode flags only when accepted.isMainModule()and normalize displayed paths.browse_cliadapter.Written for commit 6068ab3. Summary will update on new commits.