Skip to content

fix(mcp): use explicit mode for setupMCP during init#744

Merged
rexxars merged 2 commits intomainfrom
fix/mcp-setup-mode
Mar 19, 2026
Merged

fix(mcp): use explicit mode for setupMCP during init#744
rexxars merged 2 commits intomainfrom
fix/mcp-setup-mode

Conversation

@binoy14
Copy link
Contributor

@binoy14 binoy14 commented Mar 19, 2026

Summary

  • Adds explicit mode: 'prompt' | 'auto' | 'skip' option to setupMCP, replacing the previous skip boolean and internal isInteractive({skipCi: true}) check
  • The caller (init command) now determines the mode based on flags and environment, instead of setupMCP guessing
  • Removes skipCi parameter from isInteractive() (it was only used by setupMCP and produced incorrect behavior)

Before (broken):

Scenario Behavior
CI (no TTY) Auto-configured all editors
CI (with TTY) Prompted user
--yes (user terminal) Prompted user
Interactive Prompted user

After (correct):

Scenario Behavior
CI (any TTY) Skips MCP entirely
--yes (user terminal) Auto-configures all detected editors
Interactive Prompts user

Files changed

  • setupMCP.ts — replace skip with mode, remove isInteractive dependency
  • init.ts — compute mcpMode from --no-mcp, CI detection, and --yes flag
  • isInteractive.ts — remove skipCi param, simplify to no-arg function
  • configure.test.ts — remove isInteractive mock and non-interactive test
  • New setupMCP.test.ts — unit tests for all three mode behaviors

🤖 Generated with Claude Code

…Interactive

The caller (init command) now determines the MCP setup mode based on
flags and environment, instead of setupMCP guessing from isInteractive.
This fixes incorrect behavior where CI skipped MCP (should skip) but
--yes also prompted (should auto-configure).

- Add mode: 'prompt' | 'auto' | 'skip' to MCPSetupOptions
- Remove skipCi parameter from isInteractive (only used by setupMCP)
- Init command computes mode: CI → skip, --yes → auto, else → prompt
- mcp configure command unchanged (defaults to prompt)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@binoy14 binoy14 requested a review from a team as a code owner March 19, 2026 19:57
@binoy14 binoy14 requested review from ryanbonial and removed request for a team March 19, 2026 19:57
@claude
Copy link

claude bot commented Mar 19, 2026

Claude finished @binoy14's task in 2m 47s —— View job


Review

  • Read changed files
  • Analyze for bugs, logic errors, and issues
  • Post review feedback

configure.ts:43 — Behavior change for CI+TTY

mode: isInteractive() ? 'prompt' : 'auto'

The old setupMCP internals used isInteractive({skipCi: true}), which meant CI+TTY → prompt. The new code uses isInteractive() (no skipCi), which returns false in any CI environment regardless of TTY, so CI+TTY now → auto-configure all editors without prompting.

This is a breaking behavior change for users running sanity mcp:configure interactively in a CI environment (e.g., an interactive terminal in a CI job). Consider whether 'auto' is the right fallback or if the original skipCi-like logic should be preserved for the explicit mcp configure command.

Fix this →


init.ts:468 — Silent MCP skip on --yes without TTY

if (!this.flags.mcp || !this.resolveIsInteractive()) {
  mcpMode = 'skip'
} else if (this.flags.yes) {
  mcpMode = 'auto'
}

If a user runs sanity init --yes without a TTY (e.g., piped input, no CI env var), resolveIsInteractive() returns false, MCP is silently skipped, and the --yes auto-configure path is never reached. No warning or debug message is emitted in this case. If --yes is meant to drive unattended installs without a TTY, MCP would unexpectedly not be configured. Consider adding a mcpDebug log when skipping due to non-interactive (non---no-mcp) conditions.

@binoy14 binoy14 marked this pull request as draft March 19, 2026 19:57
@github-actions
Copy link

github-actions bot commented Mar 19, 2026

Coverage Delta

File Statements
packages/@sanity/cli-core/src/util/isInteractive.ts 100.0% (±0%)
packages/@sanity/cli/src/actions/mcp/setupMCP.ts 100.0% (+ 3.5%)
packages/@sanity/cli/src/commands/init.ts 92.6% (- 0.1%)
packages/@sanity/cli/src/commands/mcp/configure.ts 90.0% (±0%)

Comparing 4 changed files against main @ ff9f15f3f7a599b3bb06dbd25117e2d865623123

Overall Coverage

Metric Coverage
Statements 82.0% (+ 0.0%)
Branches 71.0% (+ 0.1%)
Functions 80.2% (±0%)
Lines 82.4% (+ 0.0%)

- Replace nested ternary in init.ts with readable if/else
- Fix mcp configure command to pass mode: 'auto' in non-interactive
  environments, preventing prompt hang in CI/piped contexts
- Restore non-interactive test in configure.test.ts

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@binoy14 binoy14 marked this pull request as ready for review March 19, 2026 20:29
@binoy14 binoy14 requested a review from rexxars March 19, 2026 20:29
@rexxars rexxars merged commit e11f495 into main Mar 19, 2026
42 checks passed
@rexxars rexxars deleted the fix/mcp-setup-mode branch March 19, 2026 21:20
@squiggler-app squiggler-app bot mentioned this pull request Mar 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.

2 participants