feat: Enhanced Non-Interactive Mode for Automation #283
Conversation
WalkthroughAdds explicit non-interactive detection and threads a CLI-level force flag through prompting, confirmation, and global-config flows so prompts short-circuit and config writes respect combined force/non-interactive checks. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI
participant Cmd as Command (install/uninstall)
participant NonInt as NonInteractiveUtil
participant Prompt as prompt_with_default
participant Config as GlobalConfig
User->>CLI: run command (with/without --force)
CLI->>Cmd: invoke (force flag passed)
Cmd->>NonInt: is_explicit_non_interactive()
Cmd->>NonInt: should_force_operation(cli_force_flag=force)
NonInt-->>Cmd: explicit? / forced?
alt Explicit non-interactive OR Forced
Cmd->>Prompt: prompt_with_default(..., force=true) short-circuits
Note right of Prompt: returns default or raises UsageError for required missing value
Cmd->>Config: write global config (should_force_operation=true)
Config-->>Cmd: success
else Interactive
Cmd->>Prompt: prompt_with_default(..., force=false) shows prompt
User-->>Prompt: provides input
Prompt-->>Cmd: input
Cmd->>Config: write global config (should_force_operation=false)
Config-->>Cmd: success
end
Cmd-->>User: operation complete / aborted
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (1)**/*.py📄 CodeRabbit inference engine (CLAUDE.md)
Files:
🧬 Code graph analysis (1)src/mcpm/commands/install.py (4)
🔇 Additional comments (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
PR Compliance Guide 🔍Below is a summary of compliance checks for this PR:
Compliance status legend🟢 - Fully Compliant🟡 - Partial Compliant 🔴 - Not Compliant ⚪ - Requires Further Human Verification 🏷️ - Compliance label |
||||||||||||||||||||||||
PR Code Suggestions ✨Explore these optional code suggestions:
|
|||||||||||||||
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/mcpm/commands/install.py (1)
23-23: Non-interactive short-circuit inprompt_with_defaultis aligned with goalsImporting
is_non_interactive/should_force_operationand using them to short-circuitprompt_with_defaultavoids blocking I/O in headless mode and correctly distinguishes between required-with-default, required-without-default (Abort), and optional fields. Behavior in fully interactive mode is preserved since the early return only triggers when flags/env demand it.Optionally, you could update the docstring of
prompt_with_defaultto mention the non-interactive behavior so callers know it may raiseclick.Abortimmediately when required values lack defaults under MCPM_NON_INTERACTIVE/MCPM_FORCE.Also applies to: 88-96
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/mcpm/commands/install.py(5 hunks)src/mcpm/commands/uninstall.py(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Always format Python code with
ruff.
Files:
src/mcpm/commands/uninstall.pysrc/mcpm/commands/install.py
🧬 Code graph analysis (2)
src/mcpm/commands/uninstall.py (1)
src/mcpm/utils/non_interactive.py (1)
should_force_operation(35-41)
src/mcpm/commands/install.py (2)
src/mcpm/utils/non_interactive.py (2)
should_force_operation(35-41)is_non_interactive(10-32)src/mcpm/schemas/full_server_config.py (1)
to_server_config(72-84)
🔇 Additional comments (5)
src/mcpm/commands/uninstall.py (2)
10-10: LGTM!The import is correctly added to support the new force operation check.
63-70: I attempted to verify the concern by accessing the codebase, but encountered a repository cloning error. Let me analyze the review comment based on the provided context:Analysis of the review concern:
The review comment identifies a logical gap in the code at lines 63-70:
if not force and not should_force_operation(): console.print(f"\n[bold yellow]Are you sure you want to remove:[/] {server_name}") console.print("[italic]To bypass this confirmation, use --force[/]") confirmed = Confirm.ask("Proceed with removal?") if not confirmed: console.print("Removal cancelled.") returnThe concern is valid on its face:
- The condition checks
not forceandnot should_force_operation()(which readsMCPM_FORCE)- However, it does NOT check for non-interactive mode
- If a user sets
MCPM_NON_INTERACTIVE=truewithoutMCPM_FORCE=true, the code would attempt to callConfirm.ask(), which could block on stdin in a non-interactive environmentThis is a logical concern that appears sound, assuming:
is_non_interactive()function exists (mentioned in PR summary)Confirm.ask()from the Rich library blocks on inputinstall.pydemonstrates the correct pattern for handling this caseHowever, I cannot verify these assumptions without codebase access to:
- Confirm
is_non_interactive()exists and is importable- Compare with
install.pyimplementation- Verify Rich's
Confirm.ask()behavior specificationsrc/mcpm/commands/install.py (3)
439-439: Propagating env-based force intoglobal_add_serveris correctPassing
force or should_force_operation()intoglobal_add_servermeans both the CLI--forceflag and the MCPM_FORCE env var now consistently control overwrite behavior. This aligns the global add logic with the non-interactive/force semantics described in the PR without changing the default behavior when neither is set.
172-176: I'll help you verify this review comment. Let me start by examining the code in question and understanding the non-interactive handling patterns in the codebase.
<function_calls>
Examine the specific file and lines mentioned in the review
#!/bin/bashFirst, check the file exists and read the relevant lines
if [ -f "src/mcpm/commands/install.py" ]; then
echo "=== File: src/mcpm/commands/install.py (lines 160-185) ==="
sed -n '160,185p' src/mcpm/commands/install.py | cat -n
else
echo "File not found at src/mcpm/commands/install.py"
find . -name "install.py" -type f 2>/dev/null | head -5
fi
</function_calls><function_calls>
Search for is_non_interactive() definition and usage
#!/bin/bash
echo "=== Searching for is_non_interactive() definition ==="
rg -n "def is_non_interactive" --type pyecho -e "\n=== Searching for is_non_interactive() usage ==="
rg -n "is_non_interactive()" --type py -C 2
</function_calls><function_calls>
Search for should_force_operation() definition and all Confirm.ask calls
#!/bin/bash
echo "=== Searching for should_force_operation() definition ==="
rg -n "def should_force_operation" --type pyecho -e "\n=== Searching for all Confirm.ask() calls ==="
rg -n "Confirm.ask" --type py -C 2
</function_calls>
219-242: I'm unable to verify this review comment due to repository access issues preventing me from cloning the codebase. Without direct access to the repository files, I cannot confirm:
- Whether
is_non_interactive()function exists and how it's defined- How
prompt_with_default()implements non-interactive checks for comparison- All
click.promptcalls in the file to assess scope of the issue- Whether the suggested fix aligns with existing patterns
Since verification requires examining the actual codebase and I cannot access it, I cannot definitively determine if the original review comment's concern is valid or if the proposed fix is appropriate.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/mcpm/commands/install.py (2)
23-23: Non‑interactive short‑circuit inprompt_with_defaultmatches the goals; minor clarity and reuse tweaks possible.The early return/abort when
MCPM_NON_INTERACTIVE=trueorMCPM_FORCE=trueis set nicely prevents blocking I/O in headless runs and fails fast for required/no‑default fields. The behavior for required+default vs optional fields is also consistent with the rest of the function.Two small suggestions:
- The comment about
isatty()is a bit confusing relative tois_non_interactive()’s implementation (it triggers onnot isatty()); consider rephrasing so future readers understand that the real issue is test runs wheresys.stdin.isatty()isFalse, makingis_non_interactive()returnTrueand thereby skipping mocked prompts.- You’re partially re‑implementing the env‑var check that already lives in
is_non_interactive(). To keep behavior centralized while still avoiding theisatty()path in tests, consider adding a helper inmcpm.utils.non_interactivefor “explicit env‑driven non‑interactive only” and calling that here instead of inlining theos.getenv("MCPM_NON_INTERACTIVE")logic.Also applies to: 76-99
175-180: Force flag now correctly skips confirmations & method selection; consider whetherMCPM_NON_INTERACTIVEalone should also bypass these prompts.Using
not force and not should_force_operation()around the install confirmation and method‑selection prompt meansMCPM_FORCE=truenow behaves like a global--force, which fixes the “headless hangs due to missing confirmations” issue.One open design question: right now, a caller that sets only
MCPM_NON_INTERACTIVE=true(but notMCPM_FORCE=trueand without--force) will still hit these interactive prompts. That’s defensible (non‑interactive does not imply “assume yes” for potentially destructive operations), but it does differ fromprompt_with_default, which treatsMCPM_NON_INTERACTIVEas a reason to skip prompting entirely.If you want “non‑interactive but not forced” installs to also auto‑select the default method and auto‑answer “no” or “yes” consistently without blocking, you could fold the explicit non‑interactive flag into these guards as well, e.g. via a small helper from
non_interactive.py:explicit_non_interactive = os.getenv("MCPM_NON_INTERACTIVE", "").lower() == "true" if len(installations) > 1 and not force and not should_force_operation() and not explicit_non_interactive: ...and similarly for the confirmation condition.
Otherwise, it would be good to document that “CI‑safe” usage requires setting
MCPM_FORCE=true(in addition toMCPM_NON_INTERACTIVE=trueif you also want argument prompts to short‑circuit) so consumers know what to expect.Also applies to: 221-244
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/mcpm/commands/install.py(5 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Always format Python code with
ruff.
Files:
src/mcpm/commands/install.py
🧬 Code graph analysis (1)
src/mcpm/commands/install.py (2)
src/mcpm/utils/non_interactive.py (2)
should_force_operation(35-41)is_non_interactive(10-32)src/mcpm/schemas/full_server_config.py (1)
to_server_config(72-84)
🔇 Additional comments (1)
src/mcpm/commands/install.py (1)
441-443: Propagatingforce or should_force_operation()intoglobal_add_serveraligns global config behavior with env‑level force.Passing
force or should_force_operation()here ensuresMCPM_FORCE=truealone is enough to override existing global server entries, even when the CLI wasn’t invoked with--force. That’s consistent with the new confirmation/method‑selection logic and with the stated goal of a single global “force” control for automation.No changes needed from my side.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/mcpm/commands/install.py(5 hunks)src/mcpm/commands/uninstall.py(2 hunks)src/mcpm/utils/non_interactive.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Always format Python code with
ruff.
Files:
src/mcpm/utils/non_interactive.pysrc/mcpm/commands/uninstall.pysrc/mcpm/commands/install.py
🧬 Code graph analysis (1)
src/mcpm/commands/install.py (1)
src/mcpm/utils/non_interactive.py (2)
should_force_operation(35-45)is_non_interactive(10-32)
🔇 Additional comments (7)
src/mcpm/commands/uninstall.py (2)
10-10: LGTM - Import aligns with non-interactive enhancements.The import of
should_force_operationis correctly placed and used at line 63 to gate the confirmation prompt.
63-63: LGTM - Confirmation properly gated by combined force check.The change from
if not forcetoif not should_force_operation(force)correctly implements the PR objective of respecting both the CLI--forceflag and theMCPM_FORCEenvironment variable.src/mcpm/commands/install.py (4)
179-179: LGTM - Install confirmation properly gated.The confirmation prompt correctly uses
should_force_operation(force)to respect both the CLI--forceflag and theMCPM_FORCEenvironment variable, preventing hangs in non-interactive environments.
224-224: LGTM - Method selection properly gated.The installation method selection correctly uses
should_force_operation(force)to automatically use the first/recommended method when forced, preventing hangs when multiple installation methods exist (e.g., stdio vs Docker).
444-444: LGTM - Force flag properly propagated to global_add_server.The call correctly passes
should_force_operation(force)toglobal_add_server, ensuring that server override behavior respects both the CLI flag and environment variable.
23-23: I encountered a repository clone failure and cannot directly verify the codebase. However, based on the review comment provided, I can analyze what's presented:The review comment makes a specific assertion:
is_non_interactiveis imported but not called, with a function call check provided. Since I cannot access the repository to confirm whether:
is_non_interactiveis actually unused in install.pyis_non_interactiveis used elsewhere in the codebase- The line references (89, 92, 179, 224, 444) are accurate
I must flag this for manual verification.
Verify whether
is_non_interactiveimport is unused before removing it.The review claims
is_non_interactiveis imported at line 23 but not called in the file. While the analysis appears plausible (noting that line 92 usesshould_force_operation()instead), this requires confirmation by examining the actual code to ensure removing the import won't break anything.src/mcpm/utils/non_interactive.py (1)
35-45: LGTM - Public API enhanced to support CLI force flag.The addition of the
cli_force_flagparameter toshould_force_operationcorrectly implements the combined forcing behavior. The default value preserves backward compatibility, and the OR logic appropriately respects both the CLI flag and theMCPM_FORCEenvironment variable.
- Remove unused 'is_non_interactive' import in install.py - Use stderr for installation status message in install.py
…non-interactive checks and test fixes - Add is_explicit_non_interactive() helper to avoid TTY detection issues in tests - Update install/uninstall logic to strictly respect force/non-interactive flags - Pass force flag correctly to prompt_with_default - Update prompt_with_default docstring - Update tests to use env vars for input when --force is used
|
I have pushed updates to address the review feedback:
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/mcpm/commands/install.py (1)
68-68: Minor: Inconsistent spacing in Rich markup.The space between
[/ ]creates extra whitespace after "Error:". Consider using[/bold red]or[/]without the trailing space for consistency.- console.print(f"[bold red]Error:[/ ] Server '{server_config.name}' already exists in global configuration.") + console.print(f"[bold red]Error:[/] Server '{server_config.name}' already exists in global configuration.")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/mcpm/commands/install.py(12 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Always format Python code with
ruff.
Files:
src/mcpm/commands/install.py
🧬 Code graph analysis (1)
src/mcpm/commands/install.py (4)
src/mcpm/utils/non_interactive.py (2)
is_explicit_non_interactive(10-20)should_force_operation(48-58)src/mcpm/clients/base.py (3)
add_server(63-72)add_server(263-283)add_server(536-560)src/mcpm/utils/repository.py (1)
get_server_metadata(152-163)src/mcpm/schemas/full_server_config.py (1)
to_server_config(72-84)
🔇 Additional comments (7)
src/mcpm/commands/install.py (7)
23-23: LGTM!The import correctly brings in the non-interactive utilities that are used throughout this file for handling automation scenarios.
76-130: LGTM!The
forceparameter is now properly integrated intoprompt_with_default. The function correctly:
- Returns the default value when in non-interactive or forced mode
- Raises
UsageErrorwhen a required value has no default in non-interactive mode- Propagates the
forceparameter in the recursive call (line 130)
184-187: LGTM!The confirmation prompt is correctly bypassed when either the force flag is set or explicit non-interactive mode is enabled, preventing hangs in automation scenarios.
231-232: LGTM!Multi-method installation selection correctly uses the recommended/first method when in forced or non-interactive mode, aligning with the PR's goal of preventing interactive prompts in automation.
336-360: LGTM!Both call sites to
prompt_with_defaultnow correctly pass theforceparameter (lines 341 and 359), addressing the previously flagged issue about inconsistent force behavior.
454-454: LGTM!Using
should_force_operation(force)ensures both the CLI--forceflag andMCPM_FORCEenvironment variable are respected when adding the server to global configuration.
159-160: LGTM!Using stderr for status messages keeps stdout clean for potential machine-readable output or piping scenarios—good CLI practice.
1 participant
User description
Overview
This Pull Request significantly improves the robustness of the
mcpmCLI when running in non-interactive environments (e.g., CI/CD pipelines, Docker builds, or driven by AI agents).The Problem
Previously, setting
MCPM_NON_INTERACTIVE=trueorMCPM_FORCE=truewas not consistently respected across all prompt types. Specifically:The Solution
We have updated
src/mcpm/commands/install.pyandsrc/mcpm/commands/uninstall.pyto integrate stricter checks foris_non_interactive()andshould_force_operation().Key Changes:
prompt_with_defaulthelper: Now immediately returns the default value (or aborts if required) when in non-interactive mode, preventing IO blocks.global_add_serverreceives the force flag correctly.Usage
Setting standard environment variables now guarantees zero interaction:
PR Type
Enhancement
Description
Adds non-interactive mode support to install/uninstall commands
Integrates
should_force_operation()andis_non_interactive()checks throughoutPrevents hanging prompts in CI/CD and headless environments
Auto-selects first installation method when forced/non-interactive
Diagram Walkthrough
File Walkthrough
install.py
Integrate non-interactive checks into install workflowsrc/mcpm/commands/install.py
should_force_operationandis_non_interactiveutilitiesprompt_with_default()to check non-interactive mode and returndefault/abort
should_force_operation()check to installation confirmationprompt
should_force_operation()check to installation method selectionlogic
global_add_server()calluninstall.py
Add non-interactive support to uninstall confirmationsrc/mcpm/commands/uninstall.py
should_force_operationutilityshould_force_operation()check to uninstall confirmation promptSummary by CodeRabbit
New Features
Behavior Changes
Tests
Other
✏️ Tip: You can customize this high-level summary in your review settings.