Skip to content

fix: getCliClient() token not passed through#688

Merged
rexxars merged 6 commits intomainfrom
fix/exec-get-user-token
Mar 16, 2026
Merged

fix: getCliClient() token not passed through#688
rexxars merged 6 commits intomainfrom
fix/exec-get-user-token

Conversation

@rexxars
Copy link
Member

@rexxars rexxars commented Mar 16, 2026

Description

sanity exec --with-user-token was not passing the token to the client returned by getCliClient().

The old approach used a configClient.worker.ts that was loaded via --import in the child process. The worker imported getCliClient via a relative path and mutated its __internal__getToken property. But the user's script imports getCliClient from sanity/cli, which re-exports from @sanity/cli through package exports. These two import paths can resolve to different module instances (e.g. src/ vs dist/ during development, or duplicate packages in node_modules), so the mutation never reaches the instance the script uses.

The fix resolves the token in the parent process and passes it to the child via the SANITY_AUTH_TOKEN environment variable. getCliClient() now falls back to that env var when __internal__getToken returns nothing.

What to review

  • execScript.ts - Token resolution moved from the child process worker to the parent. Throws CLIError (no stack trace) if no token is found.
  • cliClient.ts - One-line change: getCliClient() falls back to process.env.SANITY_AUTH_TOKEN.
  • configClient.worker.ts - Deleted, no longer referenced.
  • exec-with-user-token.test.ts - New test file (separate from exec.test.ts to avoid getCliToken cache ordering issues). Tests both the error path and the token-passing path.
  • exec-get-user-token.ts - Test fixture script that verifies the token reaches the client config.

Testing

Two new tests in exec-with-user-token.test.ts:

  • Error case: --with-user-token with no configured token throws CLIError with actionable message
  • Success case: Token from CLI config reaches getCliClient() in the spawned script (uses a fake token, no real API call needed)

@rexxars rexxars requested a review from a team as a code owner March 16, 2026 16:42
@rexxars rexxars requested review from binoy14 and mttdnt and removed request for a team and mttdnt March 16, 2026 16:42
@claude
Copy link

claude bot commented Mar 16, 2026

Claude finished @rexxars's task in 3m 16s —— View job


Review

Two issues to consider:


1. Test order fragility in exec-with-user-token.test.ts

getCliToken() has a module-level cache (let cachedToken). Within the same test file, the error test works correctly only because it runs first (before any token is cached). If the success test runs first, it sets cachedToken = 'test-fake-token-abc123', and the error test would then return that cached token instead of throwing.

The comment at the top of the file explains the cross-file isolation rationale, but the intra-file ordering dependency is still fragile. Consider resetting the module between tests:

beforeEach(() => {
  vi.resetModules() // clears getCliToken's cachedToken
})

Or reorder so the success test (which sets the cache) always runs last — which it currently does, but relies on implicit ordering.

exec-with-user-token.test.ts lines 67–96


2. exec.test.ts line 106: SANITY_AUTH_TOKEN not cleared before --with-user-token test

getCliToken() checks SANITY_AUTH_TOKEN before reading from the config file. The new exec-with-user-token.test.ts correctly stubs it to '' in setupTestAuth. The existing test at line 106 in exec.test.ts does not, meaning if SANITY_AUTH_TOKEN is set in CI (different from SANITY_API_TOKEN), the config-file token written by setupTestAuth(TEST_TOKEN) is silently bypassed and the test uses the env token instead.

The fix from the new test file should be applied here too:

vi.stubEnv('SANITY_AUTH_TOKEN', '') // add to setupTestAuth in exec.test.ts

exec.test.ts line 41–51


The core logic changes in execScript.ts and cliClient.ts are correct and clean.

rexxars and others added 2 commits March 16, 2026 09:58
The assertion on the API response shape without a token is
environment-dependent (empty object locally, error in CI). The test
only needs to verify the script executed successfully.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Clear SANITY_AUTH_TOKEN in beforeEach and restore original in afterEach
- Use vi.stubEnv for env var mutations within individual tests
- Add tests for SANITY_AUTH_TOKEN env var fallback and priority

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@github-actions
Copy link

github-actions bot commented Mar 16, 2026

Coverage Delta

File Statements
packages/@sanity/cli/src/actions/exec/execScript.ts 75.6% (+ 2.6%)
packages/@sanity/cli/src/util/cliClient.ts 100.0% (±0%)

Comparing 2 changed files against main @ aee2e6e8395e0584b7233fe37b41dba954fff7e0

Overall Coverage

Metric Coverage
Statements 81.8% (+ 0.1%)
Branches 70.6% (+ 0.1%)
Functions 80.0% (+ 0.1%)
Lines 82.2% (+ 0.1%)

rexxars and others added 3 commits March 16, 2026 10:25
…t stubEnv

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The test was only checking hasToken === true, which could pass even if
the token came from a leaked SANITY_AUTH_TOKEN env var. Now the fixture
outputs the token value and the test asserts it matches the specific
test token configured via setupTestAuth.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@rexxars rexxars merged commit 15c6b7d into main Mar 16, 2026
42 checks passed
@rexxars rexxars deleted the fix/exec-get-user-token branch March 16, 2026 18:11
@squiggler-app squiggler-app bot mentioned this pull request Mar 16, 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.

2 participants