Skip to content

Fix keepAlive supervisor in node SEA binary, fix relative path construction#1701

Closed
pirate wants to merge 42 commits into
mainfrom
resolve-repo-root
Closed

Fix keepAlive supervisor in node SEA binary, fix relative path construction#1701
pirate wants to merge 42 commits into
mainfrom
resolve-repo-root

Conversation

@pirate

@pirate pirate commented Feb 18, 2026

Copy link
Copy Markdown
Contributor

Resolve paths relative to each source module instead of the repo root to stabilize ESM/CJS behavior, CI, and packaging. Adds a supervisor CLI/API and SEA entry that avoid bin resolution, improves Turbo caching, and makes page and session timeouts configurable (first page: 5s default, 30s in CI; Browserbase session: 60s in CI).

  • Refactors

    • Remove repo-root helpers; use fileURLToPath; simplify Playwright config; run core/E2E/server tests from dist; coverage/JUnit accept absolute paths; fix duplicate path handling.
    • CI uses prebuilt artifacts and a two-tier Turbo cache (base + per-commit); turbo.json depends on ci.yml; set Node engines to ^20.19.0 or >=22.12.0; add Vitest CJS config; remove envReporter and unused server unit tests; example closes cleanly with per-action timeouts; increase BROWSERBASE_SESSION_CREATE_MAX_MS to 60s.
  • New Features

    • Add a stagehand CLI (--supervisor), export __internalMaybeRunShutdownSupervisorFromArgv, and a SEA entry that can run supervisor-only mode without relying on a bin; package export exposes ./cli.
    • Evals runner supports source or dist CLI via --cli; first top-level page timeout configurable via STAGEHAND_FIRST_TOP_LEVEL_PAGE_TIMEOUT_MS.

Written for commit 9782860. Summary will update on new commits. Review in cubic

@changeset-bot

changeset-bot Bot commented Feb 18, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 07f45b8

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

@pirate pirate closed this Feb 18, 2026
@pirate pirate changed the title add .js to the end of imports Fix path resolution to construct paths relative to source files instead of repo root Feb 18, 2026
@pirate pirate reopened this Feb 18, 2026
@pirate pirate changed the title Fix path resolution to construct paths relative to source files instead of repo root Fix keepAlive supervisor in node SEA binary, fix relative path construction Feb 19, 2026
@pirate
pirate marked this pull request as ready for review February 19, 2026 00:59
@greptile-apps

greptile-apps Bot commented Feb 19, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR refactors path resolution across the codebase to use fileURLToPath(import.meta.url) instead of repo-root helpers, making module resolution more stable across ESM/CJS/SEA environments. The shutdown supervisor has been significantly improved to work in SEA binaries by moving from IPC-based config to argv-based config and adding a dedicated CLI entrypoint.

Key Changes:

  • New CLI entrypoint (packages/core/lib/v3/cli.js) provides --supervisor mode for standalone supervisor execution
  • SEA binary entry (sea-entry.ts) conditionally runs supervisor or server based on argv
  • Supervisor refactored to parse configuration from command-line arguments instead of IPC messages, fixing keepAlive behavior in SEA binaries
  • Path resolution standardized using fileURLToPath(import.meta.url).split("/packages/*/")[0] pattern across all build and test scripts
  • CI workflow optimized to use prebuilt artifacts and increased Browserbase session timeout from 30s to 60s
  • First page load timeout now configurable via STAGEHAND_FIRST_TOP_LEVEL_PAGE_TIMEOUT_MS (5s default, 30s in CI)
  • Test scripts updated to run from dist/ artifacts instead of source
  • Removed server unit tests workflow, added Node engine constraint (^20.19.0 || >=22.12.0)
  • Example code improved with proper cleanup (finally block) and per-action timeouts

Confidence Score: 5/5

  • This PR is safe to merge with minimal risk
  • The changes are well-structured refactorings focused on improving path resolution stability and fixing the supervisor in SEA binaries. The refactoring follows a consistent pattern across all files (using fileURLToPath), the supervisor changes are logically sound (moving from IPC to argv-based config), and the CI improvements (prebuilt artifacts, increased timeouts) address real timing issues. All changes are internal refactorings with no breaking API changes.
  • No files require special attention

Important Files Changed

Filename Overview
packages/core/lib/v3/cli.js New CLI entrypoint that spawns shutdown supervisor via argv parsing
packages/server/src/sea-entry.ts SEA binary entry that conditionally runs supervisor or server
packages/core/lib/v3/shutdown/supervisor.ts Refactored to parse config from argv instead of IPC messages
packages/core/lib/v3/shutdown/supervisorClient.ts Updated to spawn via CLI path with serialized config args, SEA support
packages/core/scripts/build-cjs.ts Refactored path resolution using fileURLToPath, builds CLI and tests
.github/workflows/ci.yml Increased session timeout to 60s, uses prebuilt artifacts, removed server unit tests

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Stagehand Init] --> B{Environment?}
    B -->|Node.js| C[Launch via cli.js]
    B -->|SEA Binary| D[sea-entry.ts]
    
    C --> E{Args contain --supervisor?}
    D --> E
    
    E -->|Yes| F[Parse config from argv]
    E -->|No| G[Start Stagehand Server]
    
    F --> H[supervisor.ts runs]
    H --> I[Setup stdin lifeline]
    H --> J[Setup IPC channel]
    
    I --> K{Parent alive?}
    J --> L{Receive message?}
    
    K -->|stdin closes| M[Trigger cleanup]
    L -->|exit message| N[Exit without cleanup]
    
    M --> O{keepAlive?}
    O -->|false| P[Cleanup resources]
    O -->|true| Q[Exit gracefully]
    
    P --> R{Config kind?}
    R -->|LOCAL| S[Kill Chrome + Remove temp profile]
    R -->|STAGEHAND_API| T[Request Browserbase session release]
    
    S --> U[Exit]
    T --> U
    Q --> U
    N --> U
Loading

Last reviewed commit: 1f9629d

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

34 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

cubic-dev-ai[bot]

This comment was marked as resolved.

cubic-dev-ai[bot]

This comment was marked as resolved.

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

1 issue found across 2 files (changes from recent commits).

Prompt for AI agents (all issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="turbo.json">

<violation number="1" location="turbo.json:18">
P2: API secret keys in `globalEnv` will unnecessarily bust all build caches. `globalEnv` vars are hashed into every cacheable task's key — but build/lint tasks never use `OPENAI_API_KEY`, `ANTHROPIC_API_KEY`, `GEMINI_API_KEY`, etc. Previously these were only in test task `env` arrays (which all have `cache: false`), so they had zero caching impact. Rotating any API key will now invalidate all build caches.

Consider keeping only build-relevant variables in `globalEnv` (e.g., `CI`, `NODE_OPTIONS`, `NODE_V8_COVERAGE`, `CHROME_PATH`) and either moving the API keys back to task-specific `env` or using Turborepo's `globalPassThroughEnv` so they're available but not hashed.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

Comment thread turbo.json Outdated

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

1 issue found across 1 file (changes from recent commits).

Prompt for AI agents (all issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name=".github/workflows/ci.yml">

<violation number="1" location=".github/workflows/ci.yml:670">
P2: The `merge-coverage` job is unconditionally disabled with `if: false`. This means no merged code-coverage report will ever be generated or uploaded. If this is a temporary measure, consider adding a TODO/tracking issue reference so it doesn't stay disabled indefinitely. If coverage reporting has moved elsewhere, this entire job block should be removed to avoid confusion.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

Comment thread .github/workflows/ci.yml
@pirate pirate closed this Feb 19, 2026
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