[STG-2274] docs(cli): add 401 playbook + anti-retry guidance to bundled browse skill#2245
Merged
Merged
Conversation
…uidance to bundled browse skill Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
shrey150
commented
Jun 12, 2026
…w default) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
ajmcquilkin
approved these changes
Jun 16, 2026
shrey150
added a commit
that referenced
this pull request
Jun 25, 2026
… legacy stdout) (#2246) ## Summary Bare `browse screenshot` now writes a file by default — `screenshot-<yyyymmdd-hhmmss>.<type>` in the current directory, never overwriting (atomic collision counter) — and prints the small `{ "saved": "<path>" }` JSON. A new `--base64` flag preserves the legacy stdout contract (`{ "base64": "..." }`); it is mutually exclusive with `--path`. Explicit `--path` behavior is unchanged. Linear: [STG-2275](https://linear.app/browserbase/issue/STG-2275/default-browse-screenshot-to-file-output-with-base64-legacy-flag) ## Impact if merged screenshot is one of the two commands in the runaway agent retry loops (the loop population generates 92.3% of all CLI telemetry) and is used by 1,337 users/30d (89.9% success last-7d). Every bare invocation today prints ~22KB of base64 JSON directly into the calling agent's context window — a per-call token tax on exactly the ICP population (claude-code/codex agents driving the CLI; agent-tagged usage is 90%-successful and growing). Defaulting to a file write makes the common case agent-safe at zero cost to `--path` users; `--base64` preserves the old contract for scripts. Stdout contract change for bare invocations is called out in the changeset with a `--base64` migration note (minor bump — the bare-invocation stdout contract changes; `browse` is `<1.0.0` so this is advisory). ## Implementation notes - **Breaking change (stdout contract):** bare `browse screenshot` now prints `{ "saved": "<path>" }` instead of `{ "base64": "..." }`. **Migration:** pass `--base64` to restore the old output. - Command-layer only: the driver handler (`runtime.ts`) already supported both branches (`{ saved }` when a path is given, `{ base64 }` otherwise), so the change is confined to `src/commands/screenshot.ts`. - Default filename respects `--type` (`.jpeg` vs `.png`) and resolves against the invoking shell's cwd (absolute path passed to the driver so a daemon with a different cwd can't misplace the file). - The default filename is **atomically reserved** via exclusive create (`openSync(path, "wx")`), advancing a `-2`, `-3`, ... counter on `EEXIST` — concurrent same-second invocations can never claim the same file (addresses cubic's race-condition review). If the command fails afterward, the empty placeholder is removed best-effort. - `--base64` is mutually exclusive with `--path` via oclif's `exclusive` option. - Changeset: `browse` **minor** (per review) — the bare-invocation stdout contract change is called out in the changeset body with the `--base64` migration note. - Bundled skill doc: `skills/browse/SKILL.md` screenshot snippet updated in this PR (moved from #2245 per review) to document the new default: bare saves a file, `--path` chooses it, `--base64` is the legacy stdout form. ## E2E Test Matrix All rows ran against the local build (`pnpm build` in `packages/cli`, invoked as `node bin/run.js`) from a `<scratch dir>`. | Command / flow | Observed output | Confidence / sufficiency | | --- | --- | --- | | `node bin/run.js open https://example.com` | `{ "title": "Example Domain", "url": "https://example.com/", ... }`, exit 0 | Session setup for the runs below; proves the local build is functional end-to-end. | | `node bin/run.js screenshot` (bare) | `{ "saved": "<scratch dir>/screenshot-20260612-123732.png" }`, exit 0; `ls -la` shows the file at 15,988 bytes; stdout is the small JSON only | Proves the new default: file written to cwd with timestamped name, no base64 on stdout. | | `node bin/run.js screenshot --base64 \| head -c 200` | `{ "base64": "iVBORw0KGgoAAAANSUhEUgAABQgAAALH..."` (PNG magic in base64), exit 0 | Proves the legacy stdout contract is fully preserved behind `--base64`. | | `node bin/run.js screenshot --path /tmp/custom.png` | `{ "saved": "/tmp/custom.png" }`, exit 0; file exists at 15,238 bytes | Proves explicit `--path` behavior is unchanged. | | Two **concurrent** bare runs (same second, shell `&` + `wait`) | `{ "saved": ".../screenshot-20260612-123741.png" }` and `{ "saved": ".../screenshot-20260612-123741-2.png" }`; both files present and non-empty (4,202 bytes each) | Proves the atomic no-overwrite reservation under true same-second concurrency — the exact race cubic flagged. | | `node bin/run.js screenshot --cdp http://127.0.0.1:1` (forced failure) | `TypeError: fetch failed`, nonzero exit; directory left empty — no placeholder file | Proves failed runs do not leave empty `screenshot-*.png` placeholders behind. | | `node bin/run.js screenshot --base64 --path /tmp/x.png` | `Error: --path=/tmp/x.png cannot also be provided when using --base64`, exit 2 | Proves the oclif mutual exclusion works. | | `node bin/run.js stop` | `{ "stopped": true, "session": "default" }`, exit 0 | Clean session teardown. | | `pnpm test` (packages/cli) | `Test Files 15 passed (15), Tests 213 passed (213)` — re-run after the race fix | Supporting: no regressions in the existing CLI suite (includes the screenshot `--help` surface test). | | `pnpm lint` (packages/cli) | tsc/eslint/prettier clean | Supporting only. | 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
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
2 participants
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
Linear: https://linear.app/browserbase/issue/STG-2274/add-death-loop-prevention-guidance-to-bundled-browse-skill
Adds death-loop prevention guidance to the bundled browse skill (
packages/cli/skills/browse/SKILL.md) — the skill agents install viabrowse skills install.Impact if merged
Most agent traffic reaches the CLI through this bundled skill — it is the agent interface. The current text has no 401/invalid-key playbook and no anti-retry rule, the two behaviors behind the death-loop population: 71 installs generating 92.3% of all CLI telemetry (~5.5M events/30d) retrying
get/screenshot, including ~375k events from tagged claude-code/codex agents — the core ICP. Docs-only, zero runtime risk; pairs with the in-flight driver-error remediation PR (the code-side fix for the same loops).Changes
BROWSERBASE_API_KEYmakesbrowsedefault to remote mode (src/lib/driver/remote.tsdefaultRemoteTarget), so an invalid key turns every driver command into a 401. New entry tells agents to fix the key at https://browserbase.com/settings, unset it, or pass--local.browse doctor --json, then change approach. Init failures are cached in the daemon for several seconds (INIT_FAILURE_RETRY_MSinsession-manager.ts), so instant retries return identical errors.3. Screenshot guidance— moved to [STG-2275] feat(cli): default screenshot to file output (--base64 for legacy stdout) #2246 per review, where it documents the new file-by-default behavior that PR ships (reverted here in cb6e0c6).No changeset: docs/prompt-text only — ships with the next regular
browserelease, no version bump needed.E2E Test Matrix
<local build>:pnpm turbo run build --filter=browse...thennode bin/run.js skills install<worktree>/packages/cli/skills/browseto~/.agents/skills/browseacross 71 agents (universal + symlinked)skills installshipsgrep -n "401 Unauthorized|Never retry a failing command|stop retrying" ~/.agents/skills/browse/SKILL.mdnpx vitest run tests/skills.test.ts tests/skills-install.test.tsnpx vitest run(full packages/cli suite)npx prettier --check packages/cli/skills/browse/SKILL.md🤖 Generated with Claude Code
Summary by cubic
Add a 401 troubleshooting playbook and an anti-retry rule to the bundled
browseskill docs to prevent agent death-loops and reduce noisy telemetry. Addresses Linear STG-2274; docs-only change inpackages/cli/skills/browse/SKILL.md.BROWSERBASE_API_KEY(fix the key, unset it, or use--local).browse doctor --json, then change approach (fix key, switch--local/--remote, orbrowse stop --force). Init failures can be cached for a few seconds, so instant retries re-fail.Written for commit cb6e0c6. Summary will update on new commits.