Skip to content

[STG-2513] fix: browse skill BROWSE_SESSION precedence + container guidance#2336

Merged
shrey150 merged 1 commit into
mainfrom
shrey/skill-env-precedence
Jul 9, 2026
Merged

[STG-2513] fix: browse skill BROWSE_SESSION precedence + container guidance#2336
shrey150 merged 1 commit into
mainfrom
shrey/skill-env-precedence

Conversation

@shrey150

@shrey150 shrey150 commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Summary

A 3-provider bare-loop smoke test (E2B/Modal/Vercel, real sandboxes, local CLI build) surfaced two systematic gaps in the bundled browse skill (packages/cli/skills/browse/SKILL.md):

  1. 3/3 agents ignored the pre-wired BROWSE_SESSION env var and self-named sessions via --session <name> (one run orphaned a Browserbase session as a result). The skill taught named --session usage but never said "if BROWSE_SESSION is already set, every command already targets that session — don't pass --session or invent a new name." This defeats the sandbox-template env-steering pattern (BROWSERBASE_API_KEY + BROWSE_SESSION=<name> → flagless remote commands).
  2. 2/3 agents tried --local first inside Chrome-less containers, hit "No Chrome or Chromium found", then recovered on retry. The skill lacked explicit guidance that --local requires local Chrome/Chromium and that containers/CI/sandboxes without Chrome should go straight to --remote.

What changed

Two minimal additions to packages/cli/skills/browse/SKILL.md, both in the existing "Browser Target Selection" section (the section that already teaches local/remote choice and named-session usage) — no restructuring, no new sections:

`--local` requires Chrome or Chromium already installed on the machine. In containers, CI, and sandboxes with no browser installed, use `--remote` instead of `--local`. If `--local` fails with "No Chrome or Chromium found" and `BROWSERBASE_API_KEY` is set, switch to `--remote` — do not retry `--local`.
If `BROWSE_SESSION` is already set in the environment, every command already targets that session — do not pass `--session` or invent a new name. An explicit `--session <name>` always overrides `BROWSE_SESSION` for that command, so only pass it to deliberately target a different session.

Verified the precedence claim against the actual implementation before writing the wording (not just trusting the smoke-test writeup):

  • packages/cli/src/lib/driver/flags.ts:73export function sessionName(value?: string): string { return value ?? process.env.BROWSE_SESSION ?? "default"; }. An explicit --session flag always wins over BROWSE_SESSION; when omitted, BROWSE_SESSION becomes the effective session; with neither, it's "default".
  • packages/cli/src/lib/driver/command-cli.ts:75runDriverCommandFromFlags calls sessionName(flags.session) for every driver command, so this precedence applies uniformly across open, snapshot, click, stop, etc.
  • packages/cli/src/lib/driver/remote.ts:130 — exact no-Chrome error string is "No Chrome or Chromium found on this machine. Install one (Linux: apt install chromium · macOS: brew install --cask google-chrome, or Chromium with CHROME_PATH set), attach to a running browser with --cdp <port>, or set BROWSERBASE_API_KEY to use a remote browser."

No changeset

This is a skill-text-only change (SKILL.md), same category as PR #2329 (skill-text-only fix, no changeset). The updated text ships with the next browse release — a release is already queued by #2335's changeset — and is consumed at runtime by the evals harness added in #2334.

E2E Test Matrix

Command / flow Observed output Confidence / sufficiency
pnpm install && pnpm turbo run build --filter=browse (worktree, local build) Tasks: 4 successful, 4 total Proves the local CLI build succeeds with the change in place; no build regressions from a docs-only edit.
Resolve bundledCliSkillPath() from the compiled dist/lib/skills/install.js via require.resolve + manual path join (no skills show command exists on main yet — it ships in PR #2335, unmerged at time of this PR) resolved bundledCliSkillPath: <worktree>/packages/cli/skills/browse Proves the exact file browse skills install resolves to at runtime is the file this PR edits, not a stale copy.
grep -n "already set in the environment|do not retry" packages/cli/skills/browse/SKILL.md 76:If \BROWSE_SESSION` is already set in the environment, every command already targets that session... 68:...If `--local` fails with "No Chrome or Chromium found"...do not retry `--local`.` Confirms the exact new lines are present at the bundled path resolved above — the new guidance flows through to the real command path.
node bin/run.js skills install --help Prints usage for browse skills install (no side effects triggered — did not run the real install, which mutates the global ~/.agents/skills pool via npx skills add --global) Confirms the command is wired and reachable; deliberately did not execute the real install to avoid mutating local global skill state outside this change's scope.
pnpm test:cli (vitest, worktree build) Test Files 23 passed (23) / Tests 344 passed (344) Full existing CLI test suite still green; this change touches no runtime code.
pnpm eslint . No output / exit 0 Lint clean.
pnpm format:check (prettier) All matched files use Prettier code style! Formatting clean, including the edited Markdown.
tsc --noEmit -p tsconfig.json No output / exit 0 Typecheck clean (unaffected by a Markdown-only change, included for completeness).

Related

🤖 Generated with Claude Code


Summary by cubic

Clarifies the browse skill docs to prevent session misrouting and container failures by documenting BROWSE_SESSION precedence and when to use --remote instead of --local. Addresses Linear STG-2513.

  • Bug Fixes
    • Documented that if BROWSE_SESSION is set, commands target that session; only pass --session to override (precedence: --session > BROWSE_SESSION > default).
    • Stated that --local requires Chrome/Chromium; in containers/CI/sandboxes use --remote. If you see "No Chrome or Chromium found," switch to --remote instead of retrying --local.

Written for commit 829ff7b. Summary will update on new commits.

Review in cubic

…ce in browse skill

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@changeset-bot

changeset-bot Bot commented Jul 9, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 829ff7b

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 1 file

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

Re-trigger cubic

@shrey150

shrey150 commented Jul 9, 2026

Copy link
Copy Markdown
Contributor Author

Behavioral A/B evidence (post-review, informational): ran the exact same 4-task bare-loop sandbox smoke twice — stock skill vs this PR's text (only delta between tarballs = these two paragraphs; n=1/task, qualitative).

Measure Stock This PR
Agent honored pre-set BROWSE_SESSION (browser-using runs) 0/3 2/4
Leaked RUNNING Browserbase sessions after run 2/3 0/4
Tried --local first in Chrome-less sandbox 4/4 4/4 (unchanged)

Reading: the BROWSE_SESSION-precedence paragraph works — once agents keep the env session name, cleanup paths line up and session leaks disappear. The container/--remote paragraph doesn't prevent the initial --local attempt (it prescribes post-failure behavior; a model can't know a priori it's in a container) — prevention is being tracked separately as STG-2514 (failed --local launch leaves a zombie local-mode daemon under the session name; CLI-side root fix) plus environment-specific prompt lines in the sandbox templates, which do know their environment. No new misbehavior attributable to the added text.

Full trajectories: <smoke artifacts> (sweep2/sweep2b logs, available on request).

@shrey150 shrey150 merged commit 82c489b into main Jul 9, 2026
37 checks passed
shrey150 added a commit that referenced this pull request Jul 9, 2026
… 22 (#2344)

## Why

Every stagehand release since 2026-07-09 18:52 UTC is broken. The
Release workflow's "Update npm for Trusted Publishing" step runs `npm
install -g npm@latest`; npm@12.0.0 was published 2026-07-08 21:06 UTC
and became `latest`, and it requires node `^22.22.2 || ^24.15.0 || >=26`
— the release job runs node 20, so the step dies with `EBADENGINE`
before versioning even starts.

- Last green Release: 2026-07-08 17:50 UTC
- First failure: 2026-07-09 18:52 UTC
([run](https://github.com/browserbase/stagehand/actions/runs/29043380120)
— `npm error notsup Required: {"node":"^22.22.2 || ^24.15.0 ||
>=26.0.0"} / Actual: {"npm":"10.8.2","node":"v20.20.2"}`)
- This currently blocks the browse release carrying `browse skills show`
(#2335) and the browse-skill guidance fixes (#2336), whose changeset is
pending on main.

## What

Pin the step to `npm@11` (currently 11.18.0): supports node >= 20.17
**and** Trusted Publishing/OIDC (needs npm >= 11.5.1) — the reason this
step exists, since node 20 bundles npm 10.8.2 which predates it. One
line + a comment stating the constraint.

The alternative (bump the job to node 22/24 and keep `@latest`) changes
the entire release build environment (SEA binaries, prebuilds) and isn't
hotfix material; it can be considered separately.

## Validation

| Command / flow | Observed output | Confidence / sufficiency |
| --- | --- | --- |
| Failure-log inspection of both failed Release runs | `EBADENGINE ...
npm@12.0.0 ... Required node ^22.22.2 \|\| ^24.15.0 \|\| >=26 / Actual
node v20.20.2`, exit 1 at the pinned step | Proves the failure mechanism
exactly at this line |
| `curl registry.npmjs.org/npm` → dist-tags + publish times | `latest =
12.0.0`, published 2026-07-08T21:06Z — inside the 07-08 17:50 → 07-09
18:52 breakage window | Proves `@latest` drift is the trigger; nothing
repo-side changed |
| npm 11.x engine range vs job node | npm 11.18.0 supports node >=
20.17; job runs 20.20.2 | Proves the pin resolves the engine conflict
while keeping Trusted Publishing (>= 11.5.1) |
| Real release exercise | Not possible pre-merge — the Release workflow
only runs from main. Merging this PR pushes to main, which itself
triggers Release with the pending browse changeset — that run is the
live verification | The one path that cannot be tested from a branch |

Linear: STG-2520

🤖 Generated with [Claude Code](https://claude.com/claude-code)

<!-- This is an auto-generated description by cubic. -->
---
## Summary by cubic
Pin `npm` in the Release workflow to `11.18.0` to avoid `npm@12`
requiring Node >=22. Restores releases on Node 20 and keeps Trusted
Publishing (Linear: STG-2520).

<sup>Written for commit 90a6d71.
Summary will update on new commits.</sup>

<a
href="https://cubic.dev/pr/browserbase/stagehand/pull/2344?utm_source=github"
target="_blank" rel="noopener noreferrer"
data-no-image-dialog="true"><picture><source
media="(prefers-color-scheme: dark)"
srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source
media="(prefers-color-scheme: light)"
srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img
alt="Review in cubic"
src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a>

<!-- End of auto-generated description by cubic. -->

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants