feat: agent-browser parity — --endpoint, browser family, Windows, skill from binary, --session, RFC-0016..0019 verbs, npm, positioning - #31
Merged
Conversation
newMCPRunner froze Port/ProfileDir/NoLaunch/NoDaemon into a.defaults before the server starts re-entering Execute per tool call, but not Endpoint — so an explicit `mcp --endpoint ws://...` parsed fine at startup and then silently reset to the config/env default on the very first tool call, since each re-entrant Execute rebuilds the command tree with a.defaults.Endpoint as --endpoint's new default.
Defaults.Endpoint from CHROME_CDP_ENDPOINT or the TOML file was unvalidated, and it becomes the --endpoint flag's default, which PersistentPreRunE validates ahead of every command including ones that never touch Chrome. An invalid scheme there turned one bad config line into exit-2 usage failures across the whole CLI, contradicting the documented "malformed config is a warning, not fatal" contract. Validate with browser.ValidateEndpoint at applyFile/applyEnv time and drop an invalid value, matching how every other malformed scalar key in this file is already handled.
…OME_CDP_BIN for the managed launch
…ct TLS schemes With opts.Endpoint set, DecideConnection could still return Launch, InstructToggle, or InstructNoLaunch based on an unrelated local Chrome process, silently acting on a different browser than the one the caller named. Connect now rejects any outcome other than Attach/ConsentPending with CodeConnection when an explicit endpoint was given. ValidateEndpoint also now rejects wss:// and https://: AwaitUpgrade dials plaintext TCP, so a TLS endpoint could never actually complete a handshake through it.
runPlan froze NoLaunch/NoDaemon/ProfileDir/Port into a.defaults before re-entering Execute per step, but not Endpoint, so each step's fresh newRoot re-registered --endpoint with the unfrozen (usually empty) default. use/open key sticky-target writes on a.connOpts().Endpoint on every step regardless of connection caching, so this silently misfiled sticky state under the wrong endpoint from the second step onward.
…WSER_BIN / browser_bin CHROME_CDP_BIN collided with the shipped skill's documented meaning of that env var (the path to the chrome-cdp binary itself). Renamed the managed-launch browser-binary knob throughout: env CHROME_CDP_BROWSER_BIN, TOML key browser_bin, config.Defaults.BrowserBin, chrome.Options.BrowserBin, doctor's browser_bin envelope key, and the launch()/daemonEnv/directConnectOptions plumbing and tests. SKILL.md's CHROME_CDP_BIN line is untouched — it keeps its original meaning.
Four tests failed on windows-latest in PR #31's first CI run; the production code was correct in every case, only the tests carried POSIX assumptions. - TestResolveProfileDir compared filepath.Join output against a forward-slash literal; expectations are now built with filepath.Join too. - TestUploadRootsRejectCaseVariantSpelling assumed a case-sensitive filesystem; a case-variant path genuinely IS inside the root on windows, so it's skipped there and still runs on darwin/linux. - TestSessionRecord compared a raw path against JSON-escaped NDJSON; it now parses the envelope and compares the decoded path field. - TestBinaryExitCodes built the test binary with no extension, which windows can't exec; it now appends .exe there.
…mment The self-review grep for CHROME_CDP_BIN (finding 3's gate: it must only hit the skill's original line) also matched this comment's rename rationale. Reworded to describe the collision without repeating the literal.
…ls; evals for drive-chrome-cdp
…; page text is data
…table confirm wording
…y docs and clip rounding
Adds `net --har <path>`: the same filtered read `net` already performs, written to disk as an HTTP Archive 1.2 file via a pure encoder (internal/encode/har.go), with a summary envelope in place of the listing. Headers are forced on, --body adds content, redaction is unchanged (the encoder copies, never redacts), and the file is written 0600. Path validation (empty path, --har+--follow, bad directory, and a --clear-specific writability probe) runs before Chrome is contacted. One additive envelope field: started_at, an RFC 3339 UTC timestamp beside the existing started_ms, since HAR requires an absolute instant the listing didn't carry. No interface or daemon RPC change; --har is a client-side output mode over data Net() already returns.
Flip RFC-0017 to Accepted (implemented on this branch, pending a PR), matching how 0016 was flipped: the RFC's own header takes 0016's header wording, the README index row and summary sentence take 0016's README wording. Records the controller ruling in Open Questions: --har combined with --clear runs record stop -o's CreateTemp-style writability probe before connecting (the buffer is dropped before the write, so a late failure loses data), rather than the stat-only check that stands without --clear.
… --failed and note passthrough TestRenderRowDecodesIntoNetEntry now reflects over encode.NetEntry's json tags to catch any render key with no matching field, and covers four corners (clean/pending/network-failure/body-trouble) instead of one fixture that left Failed/Pending/Error/the body flags at their zero values unasserted. Adds TestNetHarAppliesFiltersBeforeExport, the VS-1 case (three rows, --failed, NetOpts.Failed observed) the existing test was mismapped to. Adds a note passthrough assertion to TestNetHarWritesTheFileAndSummarises. Test-only; no production code changed.
…n screen (RFC-0018)
…en the files allowlist
Read and write the tab's localStorage/sessionStorage over the DevTools DOMStorage domain, keyed by the top frame's security origin from one Page.getFrameTree call. list redacts credential-shaped keys/values by the same predicates net applies to headers, URL parameters and bodies, redacting before the size cap; --no-redact and --max-value opt out. get is raw and uncapped; absent-key get/rm are not errors. An opaque origin (data:, about:blank, sandboxed) is target_not_found before any DOMStorage call. Exposed over MCP as chrome_cdp_storage under --tools full only. Touches all seven places a browser capability needs here: the chrome.Browser interface, chrometest.StubBrowser, the CDP implementation, both daemon RPC halves, the mcp bind.go forwarders, the cobra command tree, and docs/cli-reference.md + the skill doc. Eleven policy rows (ten leaves + the runnable group's Exempt row).
…C guide Flip RFC-0018 and RFC-0019 to Accepted — pending PR, matching how 0016/0017 were flipped, and update docs/rfc/README.md's index rows and Draft sentence to match. RFC-0019's MCP rationale cited a fabricated `record` precedent (record is not an MCP tool at all) and called `tabs` "tab lifecycle", which RFC-0018 made inaccurate by folding dialog actions into it; both corrected, with the edits recorded under Open questions as implementation notes rather than silently rewritten. CLAUDE.md and the RFC-implementation guide now say seven places, not six: internal/mcp/bind.go's boundBrowser wrapper needs the same forwarder every new Browser method gets in the daemon, guarded by TestBoundBrowserBindsEveryMethod.
…scribes only shipped deltas
…ct checksum line match in the npm shim
sanketsudake
changed the title
…eImage Review cleanup, behavior-preserving: one stampEndpointDiag helper in doctor, shared storageItems/frameTree fetchers, exported HARTimeLayout shared by net and har, probeWritable and genericErr helpers in cli, readNamed in skills, package-level marker palette (the drawn-pixel test keeps its own literal so a palette change cannot auto-pass), and the per-pixel AnnotateImage copy loop becomes one draw.Draw with the stdlib fast path. Also drops a stray misattributed doc comment, fails loudly on envelope unmarshal in TestDoctorEndpointGarbageIsUsage, and fixes a comment typo.
… invocation A non-runnable cobra group prints help to stdout and exits 0, breaking the one-envelope and exit-code contract at exactly the point a typo is likeliest. storage's group RunE closed this for one group; dialog (new in RFC-0018) and the storage scope subgroups reopened it, and cookie, attr, headers, emulate, frame, daemon, policy, record, recipe and window had it all along — cookie being the pre-existing case storage's own comment cites. One runnableGroup helper now installs the usage RunE on every group, each group gets an Exempt policy row (the group itself never touches a tab), TestEveryGroupIsRunnable guards the structure and TestGroupTypoIsUsage pins the behavior.
…lthrough ValidateEndpoint accepted any string with the right scheme prefix, so `--endpoint ws://` was dialed as-is while EndpointKey, unable to extract a host:port, silently fell through to the port-file tier: the daemon socket and sticky state keyed to the default Chrome while the daemon itself held an undialable endpoint, occupying that socket until idle-out. The one shared validator now requires what EndpointKey actually needs — a parseable host — so the value is usage/exit 2 before anything connects, and a malformed env/config endpoint is dropped the same way.
The three re-entrant executors each hand-maintained their own copy of the connection-flag freeze list, and the copies had drifted: session froze ConsentTimeout but not Timeout (so a global --timeout was dropped per line), while mcp and recipe run froze Timeout but not ConsentTimeout (so a global --consent-timeout was dropped per step and tool call) — recipe.go's own comment claimed a --timeout parity with session the code did not deliver. One helper now owns the list; Timeout/ConsentTimeout freeze only when positive so a directly-built runner keeps the built-in defaults. TestFreezeConnDefaults pins the list and the restore.
…idation A hung connection no longer hangs npm install (60s AbortController timeout per request, threaded through redirects); a chmod/rename failure removes the staged chrome-cdp.new instead of orphaning it beside the bin; validateVersion is fully anchored, allowing only semver prerelease/build suffixes into the release URL. npm/README.md gains a trust-model section: the checksum verifies integrity against what the GitHub release published, not authenticity beyond GitHub's own release security.
…just its directory The CreateTemp probe proved the parent directory writable, but a read-only file already at the destination still failed the WriteFile — after --clear had dropped the buffer inside the read, the exact data loss the probe exists to prevent. An existing destination is now opened O_WRONLY (no truncate) before connecting; the directory probe covers the not-yet-existing case as before.
encode.go was over the 1k-line mark and the entire branch-era growth was this self-contained block (AnnotateImage, the 5x7 digit font, badge and rect drawing). har.go set the precedent of one file per coherent unit; encode.go keeps the shared marker palette and disc primitive the recording pipeline also uses, plus an explicit jpeg-decoder registration import that no longer rides along implicitly. Pure move, no behavior change.
commands.go was over the 1k-line mark; cmdSession and its NDJSON machinery are a self-contained unit and one-file-per-verb is the house style dialog.go/storage.go/skill.go already follow. Pure move, no behavior change; commands.go is back under 1k lines.
…e read errors; shotOpts takes a flags struct keyFor is now the single EndpointKey derivation the socket, sticky state and daemon start/status all share, so they can never key to different Chromes. skills.References/Skills return errors instead of silently listing nothing on an embed read failure, and skill/skill list map that to a generic envelope. shotOpts collects its ten positional parameters into a shotFlags struct so the call site is reviewable and the next screenshot flag is one field.
…llet --endpoint is itself Unreleased, so its host-requirement fix is part of the feature's description, not a Fixed entry about never-shipped code — the same rule dee3693 applied.
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
1 participant
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
One PR for the agent-browser parity plan: the in-niche capability gaps against agent-browser 0.34, the skill served from the binary, and the distribution funnels.
The spec and plan live outside the repo; the RFCs for the four new verbs are in
docs/rfc/0016–0019.Connection (WS1)
--endpoint ws://…|http://…(envCHROME_CDP_ENDPOINT, TOMLendpoint) names the debug endpoint explicitly and wins over--portand theDevToolsActivePortfile.This is the path that works against a
chrome://inspectChrome, which serves 404 on/json/*.An unreachable explicit endpoint is a
connectionerror, never a managed launch;wss:///https://are refused (the probe is plaintext); a malformed value isusageas a flag and dropped as config; the flag is carried intomcp,recipe runandsessionlines;doctorreportsendpoint_source.DevToolsActivePortdetection covers Chrome, Chromium, Brave, Edge, Vivaldi and Arc (Chrome first).CHROME_CDP_BROWSER_BIN/browser_binlets the managed-launch fallback exec another browser;doctorreportsbrowser_bin.windows/amd64+arm64zips; CI gains awindows-latestleg (build, vet,go test -short); the daemon's flock/setsid primitives are split intolifecycle_unix.go/lifecycle_windows.go.Skill from the binary (WS2)
skills/is a Go package that embeds the skills;chrome-cdp skill [--full] | list | get <ref|skill|skill/ref>serves the guide that matches the installed binary.skills/drive-chrome-cdp/SKILL.mdis a slim stub (description 999 chars) overreferences/{core,widgets,debugging,batch-and-recipes,examples}.md; two scenario skills (check-logged-in,fill-grid-and-confirm) andevals/evals.jsonwere added.npx skills add sanketsudake/chrome-cdp-cliresolves all three.Named sessions (WS3)
--session <name>/CHROME_CDP_SESSION/ TOMLsessionnamespaces the sticky current tab per endpoint, so several agents share one Chrome without stealing each other's tab; the daemon socket is shared by design.listreportscurrent_session,usereportssession;sessionnow freezes its connection flags for every NDJSON line.New verbs, RFC-first (WS4)
screenshot --annotate: numbered labels on the actionable elements and a legend (annotations[{n, ref, role, name, center}]) in the envelope; degrades to the plain screenshot withannotated:false, reasonon a throttled tab.net --har <path>: HAR 1.2 from the retained requests, headers redacted unless--no-redact, summary-only envelope,0600.dialog status|accept [text]|dismiss: acts on a native dialog that is already on screen (retained in the daemon's per-tab capture);statusis Reading,accept/dismissMutating;--on-dialogis unchanged.storage local|session list|get|set|rm|clear: Web Storage for the tab's top-frame origin,listredacted likenet, opaque origins refused; MCP tool behind--tools full(default set stays at 18).Adoption (WS5)
npm/:@sanketsudake/chrome-cdp, a postinstall shim that downloads and checksum-verifies the release binary; a release-workflow job publishes it when theNPM_PUBLISHrepository variable istrueandNPM_TOKENis set.CHANGELOG.md(Keep a Changelog), goreleaser changelog groups, two demo GIFs fromdata:fixtures.Contract
internal/result/result.gois unchanged, so the exit-code table is unchanged.Browsermethod is wired through the interface, the stub, the daemon RPC (both halves) and the MCPboundBrowser(TestDispatchCoversBrowser,TestBoundBrowserBindsEveryMethod); every new verb has a policy row (TestEveryCommandIsClassified) and a validate-before-connect test.Test plan
gofmt -l .empty,go vet ./...,go test -short ./...,go test -race ./...(live Chrome) green on the consolidated headchrome-cdp --endpoint "ws://127.0.0.1:9222/devtools/browser/<id>" doctoragainst achrome://inspectChromedoctorfinds a Brave/Edge/Chromium profile'sDevToolsActivePort--session a/--session bkeep different current tabsBefore the next release (maintainer to-dos)
NPM_TOKENsecret (granular automation token for the@sanketsudakescope) and set the repository variableNPM_PUBLISH=true; otherwise the npm job is skipped.skill-description-trimunmerged (superseded by the skill stub here);feat/skill-from-binaryandfeat/session-flagare contained in this branch and can be deleted.npx skills find chrome-cdplists this repo (one install was run from a scratch dir on 2026-08-20).Review fixes (c2b30d9..5af90e3)
A full review pass over the branch produced ten follow-up commits, all green on every CI leg:
usageenvelope (exit 2) on a bare or typoed invocation, instead of cobra's help with exit 0 —dialog, thestoragescopes, and ten pre-existing groups includingcookie; guarded byTestEveryGroupIsRunnable.--endpoint(ws://) is refused as usage before connecting, closing a silent daemon-socket/sticky-state mis-keying.freezeConnDefaultslist forsession/recipe run/mcp: a global--timeoutnow survives session lines and--consent-timeoutsurvives recipe steps and MCP calls; pinned byTestFreezeConnDefaults.net --har --clearprobes an existing read-only destination file before dropping the buffer.encode/annotate.go, the session verb tocli/session.go; assorted dedup (shared endpoint-key derivation, doctor envelope enrichment helper, one HAR time layout constant) andAnnotateImagenow usesdraw.Draw.