feat(exec): move cliClient

[cngonzalez]

Description

Moves cliClient.ts to the new CLI. This is only used in exec, so its impact is pretty limited. It's a bit larger than I'd like because it would be a breaking change to make getting the client asynchronous, so some functions had to be duplicated in a synchronous way.

What to review

Are we using all utilities in the correct way? Anything look out of place?

Testing

Has some basic tests, and is tested via integration test in #186 .

[cngonzalez]

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

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

[claude]

Claude finished @cngonzalez's task —— View job

---

Code Review Complete ✅

Issues That Need Fixing:

1. Potential Double Error Message in findProjectRootSync.ts:70-73
The catch block wraps all errors with additional context, which creates confusing messages when the inner error is already descriptive (e.g., "No project root found" → "Error occurred trying to resolve project root: No project root found").

Fix: Only wrap non-descriptive errors or extract specific error types:

} catch (err: unknown) {
  if (err instanceof Error && err.message.includes('No project root found')) {
    throw err // Re-throw descriptive errors as-is
  }
  const message = err instanceof Error ? err.message : `${err}`
  throw new Error(`Error occurred trying to resolve project root:\n${message}`)
}

2. Missing Error Handling in getCliConfigSync.ts:44
The require(configPath) call can throw various errors (syntax errors, module not found, etc.) but is only wrapped with a finally block. The coverage report shows lines 34-57 are uncovered, indicating missing error handling tests.

Fix: Add try-catch around the require call:

try {
  const require = createRequire(import.meta.url)
  const loaded = require(configPath)
  cliConfig = tryGetDefaultExport(loaded) as CliConfig | undefined
} catch (requireErr: unknown) {
  const message = requireErr instanceof Error ? requireErr.message : `${requireErr}`
  throw new Error(`Failed to load CLI config from ${configPath}: ${message}`)
} finally {
  unregister()
}

3. Insufficient Test Coverage for Error Paths
The getCliConfigSync.test.ts only tests basic file existence scenarios but misses:

• Invalid TypeScript/JavaScript syntax in config files
• Schema validation failures
• createRequire/tsx registration failures

Fix: Add tests for these error scenarios in getCliConfigSync.test.ts to improve coverage from 35.29% to match the async version.

4. Potential Memory Leak in configClient.ts:5-12
Top-level await with no error handling could leave the module in a partially loaded state if getCliToken() fails.

Fix: Wrap in try-catch or use conditional initialization:

let token: string | undefined
try {
  token = await getCliToken()
} catch {
  // Token will remain undefined, handled by downstream code
}

if (token) {
  getCliClient.__internal__getToken = () => token
}

---

[github-actions]

Coverage Report

Status	Category	Percentage	Covered / Total
🔵	Lines	81.2%	4598 / 5662
🔵	Statements	80.8%	4695 / 5810
🔵	Functions	70.56%	645 / 914
🔵	Branches	66.99%	1957 / 2921

File Coverage

File	Stmts	Branches	Functions	Lines	Uncovered Lines
Changed Files
packages/@sanity/cli-core/src/config/findProjectRootSync.ts	100%	86.66%	100%	100%
packages/@sanity/cli-core/src/config/cli/getCliConfigSync.ts	42.1%	50%	100%	35.29%	34-57
packages/@sanity/cli-core/src/config/util/configPathsSync.ts	78.12%	80%	70%	82.14%	24, 73-74, 93-94
packages/@sanity/cli/src/threads/configClient.ts	0%	0%	0%	0%	5-12

Generated in workflow #1014 for commit d47bb70 by the Vitest Coverage Report Action

[cngonzalez]

To address the Claude feedback:

1. Insufficient Test Coverage for Sync Utilities
   For the new sync utilities, I added tests to match what existed for the async utilities. I agree this is not sufficient testing, but this should be handled by integration / more robust testing probably.

2. Error Type Inconsistency
   We have a linting rule that enforces TypeError and not Error.

3. Missing Error Handling in configClient.ts
   This is how it's done in the original utility (although we add a top-level await which should be fine since loaded via tsloader).

4. Incomplete Test Coverage for getCliConfigSync
   It's a bit complex (and requires a lot of mocking) to mock things like createRequire

[ryanbonial]

Approved with one non-blocking error message wording fix suggestion.