Skip to content

fix(pi): close facade browser resources - #2794

Open
shrey150 wants to merge 2 commits into
shrey/eve-facade-session-close-evefrom
shrey/eve-facade-session-close-pi
Open

fix(pi): close facade browser resources#2794
shrey150 wants to merge 2 commits into
shrey/eve-facade-session-close-evefrom
shrey/eve-facade-session-close-pi

Conversation

@shrey150

@shrey150 shrey150 commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

why

This is PR 3 of the facade-close stack. It depends on #2793, which in turn depends on #2788.

Pi is a separate native facade host with its own launch and shutdown lifecycle. Once the shared facade propagates browser.close(), Pi must provide a real host callback; otherwise the compatibility boundary correctly reports that cleanup is unsupported. Keeping this adapter in a separate PR makes Pi's shutdown and retry policy reviewable without mixing it into the Eve implementation.

what changed

  • Pi supplies the shared facade lifecycle callback and detaches the expected tools before cleanup.
  • Cleanup is serialized so shutdown, explicit close, and replacement cannot race ownership.
  • Model-requested close surfaces Stagehand/browser failures; session_shutdown remains best-effort.
  • Browserbase release is captured immediately after launch, before Stagehand.create(), so an initialization failure cannot lose the remote session.
  • A rejected memoized browser close retains a fresh SDK release operation for retry before the next launch and during final shutdown.
  • Explicit cleanup failures are surfaced as a dedicated StagehandFacadeCleanupError with a fixed sanitized message and no raw SDK/CDP causes or aggregate members.
  • Focused tests cover explicit-close failure/retry, best-effort shutdown, and failed initialization followed by retained-release retry.

behavior before and after

Pi lifecycle Before After
await browser.close() No host lifecycle callback. Awaits Pi-owned cleanup.
Explicit cleanup failure Can be swallowed while the tool succeeds. Surfaced to the caller.
Shutdown cleanup Best-effort but can lose failed ownership. Best-effort with retained release targets.
Init + browser-close failure Remote release path can be lost. Release is attempted immediately and retained on failure.
Next launch Can race or bypass failed cleanup. Waits for serialized cleanup/retry first.

test plan

This PR does not claim a separate live Pi-agent E2E; the shared remote lifecycle behavior is covered by the Eve live A/B in #2793, while this PR focuses on Pi-specific ownership, error, retry, and shutdown paths.

No changeset is included because the Pi integration is a private example.


Summary by cubic

Pi now closes facade-launched browser resources and serializes cleanup. Previously model-requested browser.close() had no host callback and failures could be swallowed; now explicit close surfaces a sanitized error while shutdown remains best-effort.

  • Routes browser.close() through a lifecycle callback in StagehandFacadeTools; session_shutdown uses the same path.
  • Serializes teardown via a shared cleanup promise and retries queued Browserbase releases before any re-launch; avoids closing replaced resources.
  • For Browserbase, captures a release operation at launch; on Stagehand init failure, attempts browser close and session release, queues failed releases for retry, and throws a combined init/cleanup error.
  • Explicit-close failures throw StagehandFacadeCleanupError (sanitized); shutdown suppresses errors but retains release targets for retry.
  • Adds focused tests in packages/integrations/pi/tests/lifecycle.test.ts.

Written for commit a34f281. Summary will update on new commits.

Review in cubic

@changeset-bot

changeset-bot Bot commented Aug 20, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: a34f281

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

@shrey150
shrey150 force-pushed the shrey/eve-facade-session-close-pi branch from 0fe2eb3 to f468ef6 Compare August 20, 2026 23:34
@shrey150
shrey150 marked this pull request as ready for review August 20, 2026 23:35

@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.

All reported issues were addressed across 2 files

Architecture diagram
sequenceDiagram
    participant Pi as Pi Extension
    participant ST as StagehandFacadeTools
    participant SM as Stagehand Manager
    participant BB as Browserbase SDK
    participant Q as cleanupTargets/cleanupPromise

    Note over Pi,Q: Pi Browser Cleanup Lifecycle

    Pi->>SM: facadeTools() – first tool call
    SM->>SM: cleanupPromise.then(retryCleanupTargets)
    alt cleanupTargets non-empty
        SM->>Q: retryCleanupTargets()
        loop each releaseSession
            Q->>BB: releaseBrowserbaseSession()
            Q->>Q: delete from cleanupTargets
        end
    end
    SM->>BB: browserbase.launch()
    BB-->>SM: browser + sessionId
    SM->>SM: capture releaseSession() immediately
    SM->>SM: Stagehand.create({browser, ...})
    alt Stagehand.create success
        SM->>ST: new StagehandFacadeTools(stagehand, {close: closeCallback})
        SM-->>Pi: resources
    else Stagehand.create failure
        SM->>BB: browser.close()
        alt browser.close fails AND releaseSession exists
            SM->>BB: releaseSession()
            alt releaseSession also fails
                BB-->>SM: error
                SM->>Q: add releaseSession to cleanupTargets
            end
            SM-->>Pi: AggregateError
        else browser.close fails without releaseSession
            SM-->>Pi: error
        else browser.close succeeds
            SM-->>Pi: original error
        end
    end

    Note over Pi,BB: browser.close() Callback

    Pi->>ST: run() with await browser.close()
    ST->>ST: lifecycle.close()
    ST->>Pi: closeResources(tools, reportErrors=true)
    Pi->>SM: closeResources(expected, reportErrors)
    SM->>SM: wait for pending launch
    alt expected tools mismatch (replaced)
        SM-->>Pi: return (no-op)
    else match
        SM->>SM: resources = undefined
        SM->>Q: cleanupPromise.then(closeResource)
        Q->>SM: closeResource(current)
        SM->>SM: stagehand.close()
        alt stagehand.close fails
            SM->>Q: collect error
        end
        SM->>BB: browser.close()
        alt browser.close fails
            SM->>SM: browserCloseFailed = true
            SM->>BB: releaseSession() (if exists)
            alt releaseSession fails
                SM->>Q: add to cleanupTargets
            else succeeds
                SM->>Q: remove from cleanupTargets
            end
        else browser.close succeeds
            SM->>BB: releaseSession() (if exists)
            SM->>Q: remove from cleanupTargets
        end
        alt any errors
            SM-->>Pi: single error or AggregateError
        else clean
            SM-->>Pi: undefined
        end
    end
    SM-->>ST: result
    ST-->>Pi: propagate error

    Note over Pi,BB: session_shutdown Event

    Pi->>Pi: on("session_shutdown")
    Pi->>SM: closeResources() – no expected, reportErrors=false
    SM->>SM: wait for pending launch
    SM->>Q: cleanupPromise.then(closeResource)
    Q->>SM: closeResource(current) – same as above
    SM->>BB: stagehand.close() – best-effort
    SM->>BB: browser.close() – best-effort
    SM->>BB: releaseSession() if needed – best-effort
    SM->>Q: retryCleanupTargets() – best-effort
    SM-->>Pi: undefined (errors suppressed)
Loading

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread packages/integrations/pi/extensions/stagehand.ts Outdated
@shrey150
shrey150 force-pushed the shrey/eve-facade-session-close-pi branch 2 times, most recently from 235c76d to c97375d Compare August 21, 2026 00:20
Signed-off-by: Shrey Pandya <shrey@browserbase.com>
Signed-off-by: Shrey Pandya <shrey@browserbase.com>
@shrey150
shrey150 force-pushed the shrey/eve-facade-session-close-pi branch from c97375d to a34f281 Compare August 21, 2026 00:26
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.

1 participant