autobrowse: tear down self-owned browser session on exit

[aq17]

Summary

evaluate.mjs left the browser session running when the inner-agent loop ended — relying on the agent to voluntarily browse stop, which LLMs routinely skip after emitting their final answer. On --env remote that leaks a Browserbase session until idle-timeout (cost + concurrency). Found while running a real signup task end-to-end.

It now runs browse stop on both the success and fatal-error paths, only when evaluate.mjs owns the session — i.e. no caller-managed session is attached via BROWSE_SESSION or --session.

Why the guard matters

The browse.sh skill-factory pipeline intentionally keeps the session alive after evaluate.mjs so browser-trace can attach and bisect ("don't release before bisecting"), then releases its own --keep-alive session explicitly. That path sets BROWSE_SESSION, so ownsSession() returns false and we don't interfere. Only standalone/local runs (which create an ad-hoc non-keep-alive session) get auto-torn-down.

Verification

• ownsSession() logic: true when nothing attached; false when BROWSE_SESSION set or --session passed.
• Live: browse open --remote → browse status shows browserConnected: true → browse stop → browserConnected: false. Our browse open runs without --keep-alive, so the remote session ends on disconnect.

Independent of the inbox feature (#119) — branched off main. Surfaced during that testing but reviewable on its own.

🤖 Generated with Claude Code

---

Note

Low Risk
Best-effort cleanup with an explicit guard for caller-managed sessions; failures during stop are ignored and do not affect run outcome.

Overview
evaluate.mjs now runs browse stop on both the normal completion path and the fatal-error handler when the harness owns the browser session (no BROWSE_SESSION env and no --session arg). That closes the gap where the inner agent often never runs cleanup, which especially leaked remote (Browserbase) sessions until idle timeout.

When a caller attaches a managed session (e.g. skill-factory / browser-trace bisect), teardown is skipped so the session can stay alive for follow-up steps.

^{Reviewed by Cursor Bugbot for commit e473b74. Bugbot is set up for automated code reviews on this repo. Configure here.}

[cursor]

Cursor Bugbot has reviewed your changes and found 3 potential issues.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit e473b74. Configure here.}

[cursor]

Session flag skips teardown only

Medium Severity

ownsSession() treats a --session CLI value like caller-managed attachment (same as BROWSE_SESSION), but the harness never copies that flag into process.env.BROWSE_SESSION. The inner agent’s browse subprocesses still use the default session, while exit skips teardownOwnedSession(), so a remote Browserbase session opened during the run can keep running and billing after evaluate.mjs exits.

 

^{Reviewed by Cursor Bugbot for commit e473b74. Configure here.}

[cursor]

Interrupt exits skip session teardown

Medium Severity

This change adds teardownOwnedSession() for normal completion and fatal promise rejections, but nothing registers it for SIGINT or SIGTERM. Interrupting a long run after the agent opened a remote browser exits without calling browse stop, so an owned Browserbase session can keep running when ownsSession() would have been true.

 

^{Reviewed by Cursor Bugbot for commit e473b74. Configure here.}

[cursor]

Equals-form session flag ignored

Medium Severity

ownsSession() treats a passed --session value as caller-managed, but it uses getArg("session"), which only recognizes --session as a separate argv token. Invocations like --session=foo leave ownsSession() true, so teardownOwnedSession() still runs browse stop and can tear down a session the caller meant to keep for follow-up work.

 

^{Reviewed by Cursor Bugbot for commit e473b74. Configure here.}

[shubh24]

Bugbot already flagged the interrupt path (no SIGINT/SIGTERM handler) and a couple of --session parsing edge cases — those are real, agree with them.

Here's one I think Bugbot missed, and it's the bigger one because it breaks the skill's normal usage:

Auto-teardown kills the shared local browser out from under parallel runs.

ownsSession() decides "this session is mine, so I'll browse stop it on exit" purely from the absence of BROWSE_SESSION / --session. Think of it like a shared office printer: the rule is effectively "if nobody told me this printer is someone else's, it must be mine, so I'll unplug it when I leave."

The problem: the skill's primary multi-task mode (SKILL.md Step 3) spawns several sub-agents at once, and in local mode they all share one browse daemon — none of them pass --session, so every one evaluates ownsSession() === true. The first sub-agent to reach the success path runs browse stop, which kills the single shared daemon and drops the CDP connection for every sibling still mid-run.

The PR's verification only exercised --env remote (where each run genuinely gets its own Browserbase session), so this didn't surface — but the default documented path is parallel local sub-agents.

→ Suggested fix: only auto-teardown in remote mode, where ownership is real:

function ownsSession() {
  return browseEnv === "remote" && !process.env.BROWSE_SESSION && !getArg("session");
}

Or capture this run's specific session id at startup and only stop that id, rather than inferring ownership from missing flags. Either also resolves Bugbot's --session edge cases as a side effect, since you'd no longer be guessing ownership.

(One minor extra: teardownOwnedSession() is called on both the success tail and in .catch() with no idempotency guard — harmless today, but a let toreDown = false or a single finally would future-proof it.)