Repository navigation
Add named Playwright executor support to browsers playwright commands - #281
Conversation
…flags Update kernel-go-sdk to 0f34ffb9d53a87043f20e0e8605c057183849299 and cover the named Playwright executors surface (ported from #281, using the published SDK instead of a staging replace directive): - `kernel browsers playwright execute --executor <name>` (BrowserPlaywrightExecuteParams.Executor); table output shows the bound tab - `kernel browsers playwright executors list <id>` (client.Browsers.Playwright.Executors.List) - `kernel browsers playwright executors delete <id> <executor> [--close-tab]` (client.Browsers.Playwright.Executors.Delete, CloseTab param) Tested: browsers playwright execute --executor (new + reused tab, -o json), browsers playwright executors list (table + json), browsers playwright executors delete (named, --close-tab=false, default restart) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 99617b9. Configure here.
masnwilliams
left a comment
There was a problem hiding this comment.
lgtm, solid change and the httptest coverage is thorough. one thing worth fixing before merge (inline on the 409 path), the rest are optional:
- file layout:
newBrowsersPlaywrightCommand()now definesexecutein this file, butPlaywrightExecute/BrowsersPlaywrightExecuteInput/runBrowsersPlaywrightExecute/BrowserPlaywrightServicestill live inbrowsers.go. might be cleaner asbrowsers_playwright.goowning the whole group likebrowsers_webmcp.godoes. alsoBrowsersCmd.executors->playwrightExecutorsreads less ambiguously next to the other service fields. --executorusage string: it restatesplaywrightExecutorsLong, which is already inexecute's Long. a one-liner likeNamed executor to run the call in; each owns its own tab (default: the active tab)would do, and keeps the server's limit of 8 out of one more place.- follow-up:
executestill doesbrowsers.Getthen calls withSessionID, whileExecutetakes id-or-name directly like the new list/delete do. pre-existing, but dropping it saves a round trip per call. - tests:
TestPlaywrightExecuteOutputbranches onstrings.Contains(tc.response, "tab"). an explicitwantTabfield would be clearer.strings.Count(rows[1], " - ") == 2is a bit tied to table padding. - the ci-fix comment /
ci-fix/hypeship/playwright-executorsbranch are stale now that v0.120.0 is out, probably worth deleting so nobody opens a PR from it.
| } | ||
| } | ||
| sb.WriteString("\nDelete one with 'kernel browsers playwright executors delete <id-or-name> <executor>'") | ||
| return errors.New(sb.String()) |
There was a problem hiding this comment.
+1 to bugbot here. i ran this through renderCommandError with fang's ErrorText transform (strings.Fields + Join) and it comes out as one line:
ERROR Browser already has 8 named Playwright executors Current executors: default checkout (busy) https://example.com/cart Delete one with 'kernel browsers playwright executors delete <id-or-name> <executor>'
simplest fix is probably a single-line hint with the real identifier and a pointer to executors list, since that already renders the executors:
var apiErr *kernel.Error
if errors.As(err, &apiErr) && apiErr.StatusCode == http.StatusConflict && in.Executor != "" {
// Not %w: the root handler re-renders any wrapped *kernel.Error as "code: message", dropping the hint.
return fmt.Errorf("%s. See them with 'kernel browsers playwright executors list %s' and free one with 'kernel browsers playwright executors delete %s <executor>'", util.CleanedUpSdkError{Err: err}, in.Identifier, in.Identifier)
}that also avoids treating every 409 as the executor limit. if you'd rather keep the multi-line list, the fix belongs in renderCommandError (generalize the PromptError carve-out into a "preformatted" error interface).
There was a problem hiding this comment.
Took this approach in 47ab106, with one tweak: the limit body has no code field, so CleanedUpSdkError would render : message. The hint reads message from the body directly instead and falls back to the generic rendering when it is missing. Guarded on in.Executor != "" as you suggested. Test updated to pin the exact single-line string.
…p to its own file
|
Addressed in 47ab106:
|
Main (#281) added the browsers playwright executors list/delete commands and --executor flag in cmd/browsers_playwright.go. The branch's earlier versions in cmd/browsers_playwright_executors.go redeclared the same symbols and broke the build, so drop them in favor of main's implementation. SDK stays at kernel-go-sdk 454206ab83bc3c94fe8f8b1746a1c422b4681983; the SDK diff is empty and a full api.md enumeration found no coverage gaps. Tested: go build ./..., go vet ./cmd/..., go test ./... all pass. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ew commands/flags (#278) This PR updates the Go SDK to 454206ab83bc3c94fe8f8b1746a1c422b4681983 and adds CLI commands/flags for new SDK methods. ## SDK Update - Updated kernel-go-sdk to 454206ab83bc3c94fe8f8b1746a1c422b4681983 - Merged main again, which now includes #281 (Playwright executors). This branch had the same commands in `cmd/browsers_playwright_executors.go`, and the copies redeclared main's symbols in `cmd/browsers_playwright.go` and broke the build. That file and its test are removed, so main's implementation is the only one. The executor commands and flags are no longer part of this PR's diff. - The latest bump, from 207ed0d79aba to 454206ab83bc ("Infer a challenge result for unobserved captcha providers in the relay"), changes only `browsertelemetry.go`. Captcha telemetry events gain `inferred`, `captcha_provider`, and `task_kind` as output-only fields, and `challenge_id` is now optional. The CLI prints telemetry events as JSON, so the new fields show up without code changes. `api.md` is unchanged, and a full enumeration found no new methods or param fields. - This branch merges main, including #286 (SDK v0.121.0). The CLI now follows main's handling of datacenter proxies and 1Password access requests (see below). The merge had also dropped KR from the `proxies create` help, broken `TestOnePasswordOperationValidation`, and removed two `1pw_fill` validation cases. All three are fixed in this PR. - The latest bump, from 168ca3675c63 to 207ed0d79aba ("Describe vault fill as safe to retry"), only changes doc comments in `vaultitem.go`: a plain `fill` never submits the page, so it is safe to retry after a failure, an `unknown` outcome, or a transport error. The CLI already says this (`vaultFillUncertain`, and the credentials help says "fill never submits and is safe to retry"). The 1Password flow still says not to retry `1pw_fill` automatically, because that operation does submit. `api.md` is unchanged, and a full enumeration found no new coverage gaps. - The bump before that, from 08ae962810430 to 168ca3675c63 ("Add feature-gated Korean ISP proxies"), only changes doc comments: KR is now a supported ISP country. `kernel proxies create` help (`--type` description and `--country` flag) now lists KR. `api.md` is unchanged, and a full enumeration found no new coverage gaps. - An earlier bump, from v0.121.0 to 08ae962810430 ("Support proxy routes in browser pools"), lets browser pools take `network.proxy_routes`. It also adds `captcha_provider` and `task_kind` to the captcha telemetry events (output only; the CLI prints telemetry events as JSON, so these show up without changes). `api.md` is unchanged. - The bump before that, from aa4c1318242d to 05978d27d142 (release v0.121.0), only changes `internal/version.go`. A full enumeration found no new coverage gaps. - An earlier bump, from b71ecfcbaeec to aa4c1318242d ("Create and poll 1Password access requests over the 1Password API"), only changes `vaultitem.go`. `api.md` is unchanged. - The earlier bump, from v0.120.0 to b71ecfcbaeec ("Deprecate datacenter proxies in the API"), has two effects. You can now update `network.allowed_hosts` on a running browser. The SDK also drops the `datacenter` proxy type. ## Coverage Analysis This PR was generated by performing a full enumeration of SDK methods and CLI commands. Every method in api.md has a CLI command, except the `x-cli-skip` endpoints (config-registry, `/mpp/browsers`, `/auth/connections/{id}/exchange`). ## New Flags - `--proxy-route` on `kernel browser-pools create` and `kernel browser-pools update` for `BrowserPoolNewParams.Network.ProxyRoutes` / `BrowserPoolUpdateParams.Network.ProxyRoutes`. It uses the same `HOST[,HOST...]=PROXY` syntax as `kernel browsers create --proxy-route`. - `--clear-proxy-routes` on `kernel browser-pools update`. It sends `network: {proxy_routes: []}`, plus any `--private-host` entries given in the same command. - `kernel browser-pools get` now has a **Proxy Routes** row. - Pool updates replace the whole `network` object. So `--proxy-route` on its own drops an existing private-host override, and `--clear-private-hosts` also drops the routes. The help text and README say so: pass both flags to keep both settings. - The `--proxy-route` help on `browsers create` and the README no longer say that `start_url` uses the top-level proxy. Routes now apply from the start of the session. - `--allowed-host` on `kernel browsers update` for `BrowserUpdateParams.Network.AllowedHosts` (`BrowserNetworkUpdateParam`). It replaces the allowlist of a running session. - `--clear-allowed-hosts` on `kernel browsers update`. It removes the allowlist by sending `allowed_hosts: null`, which returns the session to unfiltered egress. The API rejects an empty list, so the CLI rejects `--allowed-host` with no entries and points to this flag. ## 1Password access requests no longer take a browser The SDK removed `BrowserID` from `OnePasswordRequestAccessVaultItemOperationRequestParam` and `VaultItemPerformOperationParamsBody1pwAccessRequestStatus`. Without a change the CLI would not compile. - `kernel vaults items invoke <vault> <key> 1pw_create_access_request` accepts only `goal`, `reason`, and `keywords`. `1pw_access_request_status` accepts only `timeout_seconds`. Passing `browser_id` to either one now fails validation locally. - `--params` is now optional for these two operations, because all of their fields are optional. - `1pw_fill` still requires `browser_id` and `page_url`. - Updated the help text, the examples, and the 1Password credentials flow ("Invoke 1pw_create_access_request once (no browser needed)"). ## Datacenter proxy removal This follows main (#286). The SDK no longer has the `datacenter` proxy type. - `kernel proxies create --type datacenter` now fails as an invalid type. It no longer prints a deprecation warning and passes the request through. - `datacenter` is removed from the `--type` help, from the examples, and from the README. The README examples now use `isp`. - `proxies get`, `list`, and `check` handle only the remaining proxy types. - The `--allowed-host` help on `browsers create` no longer says "Create-only". ## Testing - Main merge (#281 executors): `go build ./...`, `go vet ./cmd/...`, and `go test ./...` pass after removing the duplicate files. No new commands or flags, so there was nothing to smoke-test. - Latest bump (captcha telemetry fields): `go build ./...`, `go vet ./cmd/`, and `go test ./...` pass. Smoke-tested against the production API: - `proxies create --type isp --country KR` succeeded. - `proxies get` showed `Country: kr`, and the proxy was then deleted. - `browsers telemetry events <id> --categories captcha` ran without errors on a fresh browser, which was deleted afterward. - Vault fill doc bump: `go build ./...`, `go vet ./cmd/...`, and `go test ./...` pass. No new commands or flags, so there was nothing to smoke-test. - Korean ISP bump: `go build ./...` and `go test ./cmd/proxies/...` pass. `proxies create --help` lists KR. `proxies create --type isp --country SG` followed by delete works against production. `--country KR` currently returns `Internal_error: failed to apply proxy config` for the test org, which is expected while the feature is gated server-side. No proxy was left behind. - Latest bump (pool proxy routes): `go test ./...` passes. New unit tests cover pool create/update with routes, clearing routes (with and without `--private-host`), the conflicting-flag check, and the Get row. Smoke-tested against the production API on a temporary pool, which was deleted afterward: - `browser-pools create --proxy-route 'api.ipify.org,*.ipify.org=<id>'` worked, and `get` showed the routes. - `update --proxy-route example.com=name:us-residential-test --private-host 10.0.0.0/8` set both. The API resolved the name to an ID. - `update --clear-proxy-routes --private-host 10.0.0.0/8` removed the routes and kept the private hosts. - `update --proxy-route ...` on its own replaced the network config, dropping the private hosts. `--clear-private-hosts` then removed everything. - `--proxy-route` together with `--clear-proxy-routes` is rejected. - v0.121.0 bump: `go build ./...` and `go test ./...` pass. No new commands or flags, so there was nothing to smoke-test. - Latest bump: `go test ./...` passes, with unit tests updated for request bodies without `browser_id` and for invoking with no `--params`. Smoke-tested against the production API: `1pw_create_access_request` with no params and `1pw_access_request_status --params '{"timeout_seconds":5}'` both reach the API (404 for a nonexistent vault). Passing `browser_id` is rejected locally, and `1pw_fill` still requires `--params`. A full approval round trip was not tested, because it needs a connected 1Password account. - `go build ./...`, `go vet ./...` and `go test ./...` pass. New unit tests cover forwarding allowed hosts, sending null on clear, leaving `network` out of unrelated updates, and validation (set+clear, empty entries, more than 100 entries). - Smoke-tested against the production API: - `browsers create --allowed-host example.com`, then `browsers update --allowed-host example.com,*.wikipedia.org`. `browsers get` shows the new list. - `--allowed-host en.wikipedia.org --start-url https://en.wikipedia.org` worked in a single update. - `--clear-allowed-hosts` removed the allowlist (`network` no longer has `allowed_hosts`). `-o json` output is correct. - API validation errors are shown, e.g. `hostnames must not include a URL scheme`. - `proxies create/get/list/delete --type isp` work. - `proxies create --type datacenter` shows the warning and passes API validation. Provisioning then fails upstream with "Account is suspended" from the datacenter provider, which matches the deprecation. Triggered by: kernel/kernel-go-sdk@454206a Reviewer: @kernel-internal[bot] (previous bumps: @rgarcia) 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Changes touch egress network policy (allowlists, pool proxy routes) and vault credential/payment flows; mistakes could block browser traffic or misconfigure pools, though behavior is mostly API-aligned with local validation. > > **Overview** > Bumps **kernel-go-sdk** and wires several new API surfaces into the CLI, with README updates to match. > > **Browsers:** Adds **`--allowed-host`** at create and **`--allowed-host` / `--clear-allowed-hosts`** on update for Kernel-managed egress allowlists (proxy v3, not pools). Create/update/get output shows **Allowed Hosts**; pool acquire rejects allowlists. **`--proxy-route`** help now says routes apply from session start (including setup traffic). > > **Browser pools:** Adds **`--proxy-route`** on create/update and **`--clear-proxy-routes`** on update, with display on get. Network updates replace the whole **`network`** object—docs and validation call out pairing **`--private-host`** with route changes. > > **Vaults:** **`vaults list --query`**; **`managed_auth`** credential specs via **`connection_id`**; **`kernel`** provider for wallets/cards (no card update). Output/help covers managed auth and Kernel card fields. > > **Other:** **`us-west`** in region docs; ISP proxies add **KR**; credentials show TOTP metadata; **`logs`** forwards **`--since`** only when set; 1Password **`1pw_create_access_request`** / **`1pw_access_request_status`** allow empty params (no browser). > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 3f864d3. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Co-authored-by: kernel-internal[bot] <260533166+kernel-internal[bot]@users.noreply.github.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: meliaj <17581886+meliaj@users.noreply.github.com> Co-authored-by: Rafael <raf@kernel.sh>

Summary
Adds named Playwright executor support to
kernel browsers playwright.kernel browsers playwright execute --executor <name>: runs the call in a named executor (sent only when set). Help text explains concurrency across executors, serialization within one, the always-presentdefaultexecutor bound to the active tab, named executors owning a background tab thatpageis bound to, and the limit of 8 named executors.tabthe call was bound to (Tab Target ID,Tab Created) in the table output; JSON output passes it through from the API response.kernel browsers playwright executors list <id-or-name>: table of name, busy, created at, last used at, target ID, URL (default first); supports-o json.kernel browsers playwright executors delete <id-or-name> <executor> [--close-tab]:--close-tabdefaults to true and is only sent when set explicitly. Deletingdefaultreports a restart rather than a deletion, matching API semantics.newBrowsersPlaywrightCommand()so it can be exercised in tests the same way the webmcp group is.Dependencies
Bumps
github.com/kernel/kernel-go-sdkto v0.120.0 (released), which adds the executors surface:BrowserPlaywrightExecuteParams.Executor,Tabon the execute response,BrowserPlaywrightExecutorService.List/Delete,Executor, andExecutorList.Tests
cmd/browsers_playwright_executors_test.gocovers command wiring and flag defaults, theexecutorrequest param (absent vs set), tab rendering in table and JSON output, the 409 limit error, list table/JSON rendering, delete path andclose_tabquery handling, API error cleanup, and argument validation, using anhttptestserver like the webmcp tests.go build ./...,go vet ./...,go test ./..., andgolangci-lint runall pass locally.