feat: move mock browser utils

[cngonzalez]

Description

This PR exposes some of the logic that was previously invoked in getting browser globals and environment variables set up in studioLoader.worker.ts in a new function called mockBrowserEnvironment, meant to simulate the mockBrowserEnvironment function in original-cli. While studioWorkerTask is probably a good fit for most commands that invoked mockBrowserEnvironment, moving user scripts to be executed by a worker probably constitutes a breaking change, so there has to be some way to set up the environment in a child process for sanity exec.

Some of the functionality in the original mockBrowserEnvironment -- notably registerESBuild no longer seem necessary, but I may be wrong about this.

What to review

Does the code re-organization make sense? Any missing logic?

Testing

I don't see tests for utils, so they're omitted here in the interest of time. exec will hopefully test all of this.

[claude]

Claude finished @cngonzalez's task —— View job

---

Code Review

Reviewing PR changes for issues that need fixing:

• Analyze refactored code structure
• Check for bugs and logic errors
• Review error handling
• Examine type safety issues
• Check for security vulnerabilities
• Verify dependency usage

Issues Found

1. Race condition risk in registerBrowserEnv.ts:9-13

• Top-level await without proper error handling could cause process exit
• Multiple signal handlers calling same cleanup function may conflict

// Fix: Add error handling and guard against multiple cleanups
try {
  const cleanup = await mockBrowserEnvironment(rootPath)
  let isCleanedUp = false
  const safeCleanup = () => {
    if (!isCleanedUp) {
      isCleanedUp = true
      cleanup()
    }
  }
  const signals = ['exit', 'beforeExit', 'SIGINT', 'SIGTERM'] as const
  for (const signal of signals) {
    process.on(signal, safeCleanup)
  }
} catch (error) {
  console.error('Failed to initialize browser environment:', error)
  process.exit(1)
}

2. Memory leak potential in setupBrowserStubs.ts:22-30

• stubbedKeys array grows but cleanup function only deletes current iteration keys
• If called multiple times, previous keys won't be cleaned up properly

3. Type safety issues throughout

• Excessive use of unknown casting instead of proper types
• setupBrowserStubs.ts:21 and mockBrowserEnvironment.ts lose type information
• Should define proper interfaces for mock objects instead of Record<string, unknown>

4. Missing dependency validation in getStudioEnvironmentVariables.ts:26-34

• No validation that imported function returns expected Record<string, string> type
• Should validate return type before using it in environment variables

const result = getEnvVars(rootPath)
if (!result || typeof result !== 'object') {
  throw new TypeError('getStudioEnvironmentVariables must return an object')
}

[github-actions]

Coverage Report

Status	Category	Percentage	Covered / Total
🔵	Lines	84.55%	4310 / 5097
🔵	Statements	84.26%	4396 / 5217
🔵	Functions	76.87%	595 / 774
🔵	Branches	71.09%	1800 / 2532

File Coverage

File	Stmts	Branches	Functions	Lines	Uncovered Lines
Changed Files
packages/@sanity/cli-core/src/loaders/studio/studioWorkerLoader.worker.ts	0%	0%	0%	0%	15-114
packages/@sanity/cli-core/src/util/environment/getStudioEnvironmentVariables.ts	0%	0%	0%	0%	21-39
packages/@sanity/cli-core/src/util/environment/mockBrowserEnvironment.ts	0%	0%	0%	0%	22-48
packages/@sanity/cli-core/src/util/environment/setupBrowserStubs.ts	0%	0%	0%	0%	14-45
packages/@sanity/cli/src/threads/registerBrowserEnv.ts	0%	0%	100%	0%	7-12

Generated in workflow #915 for commit 2950118 by the Vitest Coverage Report Action

[cngonzalez]

• feat(exec): move exec command #186
• feat(exec): move cliClient #180
• feat: move mock browser utils #175 👈 (View in Graphite)
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[cngonzalez]

I humbly acknowledge the Claude PR feedback but all of these are issues that existed in the existing utility, so I'm not inclined to fix all of them right now (I fixed a few)

[laurenashpole]

LGTM! I left some suggestions to get rid of anys in the mockBrowserEnvironment.ts file but otherwise I was able to plug this into my branch and everything worked fine.

[laurenashpole]

LGTM! Dropped it into my branch and called mockBrowserEnvironment and everything was working as expected.