feat: add mcpm update command for server update management - #315
Merged
JoJoJoJoJoJoJo merged 4 commits intoMar 27, 2026
Merged
Conversation
Contributor
Review Summary by QodoAdd
WalkthroughsDescription• Add mcpm update command for checking and applying updates to MCP servers - Supports git-based servers via git pull --ff-only or --rebase - NPX/UVX servers show auto-update info; HTTP remotes are skipped - Post-update build commands (e.g., uv sync, npm install) run automatically • Implement source metadata tracking via ~/.config/mcpm/sources.json - Auto-detects server sources (git, npx, uvx, remote, unknown) - Populated on mcpm new, mcpm uninstall, and mcpm update --init • Add git utilities with 30-second timeouts and comprehensive error handling • Comprehensive test coverage with 67 new tests for update, source detection, and git operations Diagramflowchart LR
A["Server Config"] -->|detect_source| B["Source Metadata"]
B -->|SourcesManager| C["sources.json"]
D["mcpm update"] -->|check status| E["git fetch/pull"]
D -->|--init| F["Auto-detect & populate"]
E -->|post_update| G["Build commands"]
H["mcpm new"] -->|hook| C
I["mcpm uninstall"] -->|hook| C
File Changes1. src/mcpm/commands/update.py
|
Contributor
Code Review by Qodo
1. _suggest_post_update() suggests pip/poetry
|
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds persistent per-server source detection and persistence, Git inspection and pull helpers, a new Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant CLI as "mcpm update CLI"
participant SM as "SourcesManager"
participant DS as "detect_source()"
participant Git as "Git Utils"
User->>CLI: mcpm update --init [--force]
CLI->>SM: load servers & sources.json
loop per server
CLI->>DS: detect_source(server_config)
DS->>Git: inspect paths/commands for git root
Git-->>DS: repo metadata / not found
DS-->>CLI: SourceMetadata
CLI->>SM: set(server_name, SourceMetadata)
end
CLI-->>User: summary (created/updated/skipped)
sequenceDiagram
actor User
participant CLI as "mcpm update CLI"
participant SM as "SourcesManager"
participant Git as "Git Utils"
participant Shell as "Post-update subprocess"
User->>CLI: mcpm update [--check] [--rebase] [server]
CLI->>SM: load servers & sources.json
loop Phase 1: evaluate each GitSource
CLI->>Git: is_git_repo(path)
Git-->>CLI: valid?
alt valid & clean
CLI->>Git: fetch(remote)
Git-->>CLI: fetch result
CLI->>Git: check_status(branch)
Git-->>CLI: commits_behind / summaries
end
end
alt --check
CLI-->>User: report availability
else apply
User->>CLI: confirm (if prompted)
loop Phase 2: for each updatable server
alt --rebase
CLI->>Git: pull_rebase(path)
else
CLI->>Git: pull_ff_only(path)
end
Git-->>CLI: pull result
alt success
CLI->>Shell: run post_update (with timeout)
Shell-->>CLI: cmd result
CLI->>SM: mark_updated(server_name)
end
end
CLI-->>User: update summary
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
DjodyKort
force-pushed
the
feat/update-command
branch
from
March 20, 2026 13:06
42029e1 to
60a920d
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Adds an mcpm update CLI command plus supporting source-metadata tracking so installed MCP servers can be checked/updated (primarily via git), with informational handling for npx/uvx and skipping for remote servers.
Changes:
- Introduce
mcpm updatecommand with--check,--rebase, and--initflows. - Add
sources.jsonsupport (models + detection + persistence) to track server origins and update behavior. - Add git utility layer (fetch/status/pull with timeouts) and comprehensive tests for source detection, git behavior, and update flows.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
src/mcpm/commands/update.py |
Implements the update/check/init flows and ties them to git + sources metadata. |
src/mcpm/core/source.py |
Adds source metadata models, auto-detection, and a SourcesManager for sources.json. |
src/mcpm/utils/git.py |
Adds subprocess-based git operations with timeouts and structured results. |
src/mcpm/cli.py |
Registers the new update command in the main CLI. |
src/mcpm/commands/new.py |
Auto-populates source metadata on server creation. |
src/mcpm/commands/uninstall.py |
Removes source metadata on server uninstall. |
tests/test_update.py |
Covers CLI behavior, init flow, update application flow, and new/uninstall hooks. |
tests/test_source.py |
Covers source detection and SourcesManager persistence/error-handling. |
tests/test_git_utils.py |
Covers real-repo git operations (fetch/status/pull) plus timeout/error handling. |
README.md |
Documents the new mcpm update functionality and flags. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (2)
src/mcpm/core/source.py (2)
137-143: GitSource created for non-existent directories may cause issues later.When
dir_path.exists()returnsFalse, the code still returnsGitSource(path=directory)at line 143. This could cause issues during update operations when the path doesn't exist.Consider returning
UnknownSourceinstead when the directory doesn't exist, or adding validation during update operations (which appears to be handled in update.py with "path not found" checks).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/mcpm/core/source.py` around lines 137 - 143, The branch that handles a provided directory currently returns GitSource(path=directory) even when Path(directory).exists() is False; update _resolve logic (the block containing dir_path, _find_git_root, and GitSource) so that if dir_path.exists() is False you return UnknownSource(path=directory) (or otherwise mark it as unknown) instead of constructing a GitSource; reference GitSource, UnknownSource and _find_git_root so the change is applied in the same code path and keep update.py's existing "path not found" validation consistent with this returned UnknownSource.
94-105: Incomplete version suffix stripping for npm packages.The function only strips
@latestsuffix but doesn't handle other version specifiers like@1.0.0,@^2.0.0, or@~1.0.0. This could result in version numbers being included in the package name.♻️ Proposed fix to handle arbitrary version suffixes
def _extract_npx_package(args: List[str]) -> Optional[str]: """Extract the npm package name from npx args, stripping flags like -y.""" for arg in args: if not arg.startswith("-"): - # Strip `@version` suffix if present (e.g. `@upstash/context7-mcp`@latest -> `@upstash/context7-mcp`) - name = arg.split("@latest")[0] if arg.endswith("@latest") else arg - # Handle scoped packages: `@scope/name`@version - if name.startswith("@") and "@" in name[1:]: - parts = name[1:].split("@") - name = f"@{parts[0]}" + # Handle scoped packages: `@scope/name` or `@scope/name`@version + if arg.startswith("@"): + # Split after the scope: `@scope/name`@version -> ["scope/name", "version"] + rest = arg[1:] # Remove leading @ + if "/" in rest: + scope_and_name = rest.split("@")[0] # Take part before any version + name = f"@{scope_and_name}" + else: + name = arg # Malformed, return as-is + else: + # Unscoped package: name or name@version + name = arg.split("@")[0] return name return None🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/mcpm/core/source.py` around lines 94 - 105, The _extract_npx_package function only strips "@latest" and misses other version suffixes; update it to remove any trailing version specifier after the package name by trimming the substring from the last "@" that denotes a version (not the scope separator). Concretely, in _extract_npx_package when you compute name, detect scoped packages (name.startswith("@")) and if so find the last "@" position and remove everything from that position onward only if that "@" comes after the scope slash, otherwise keep the scope/name; for unscoped packages simply split on the first "@" and take the left side. Ensure the modified logic preserves scoped package names like "@scope/name" and returns None for flag-only args.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/mcpm/commands/update.py`:
- Around line 116-121: detected.remote_url is assigned from
git_utils.get_remote_url and then printed/stored verbatim, which can leak
embedded credentials; before assigning/printing/saving replace any userinfo in
URLs (username:password@) with a redacted token (e.g., <redacted>) or normalize
to credential-free form using a sanitizer function (e.g., sanitize_remote_url)
and use that sanitized value when setting detected.remote_url and when calling
console.print or writing to sources.json; update git_utils.get_remote_url usage
sites (including where console.print is called for detected.remote_url and the
other occurrence at line ~168) to call the sanitizer so no plaintext credentials
are persisted or echoed.
- Around line 240-242: The summary shows a green "up to date" even when all
candidates were skipped/errored because `updatable` is empty; change the logic
in the update command (e.g., around the `updatable` usage and the summary print
blocks near the `console.print` shown and the later summary at lines ~269-271
and ~335-345) to track skipped/errored packages (introduce a boolean like
`had_skips_or_errors` or counters for `skipped`/`failed`) and only print the
green success/"up to date" summary when there were zero skips/errors and
`updatable` is empty; otherwise print a non-success summary (warning/error)
indicating how many were skipped/failed and avoid the misleading green message.
Ensure the checks reference the existing variables `source`, `updatable`, and
current summary printing code paths so skipped/error cases no longer appear as
"up to date."
In `@src/mcpm/utils/git.py`:
- Around line 94-100: The is_dirty function currently treats a non-zero git
status exit as clean; update it to fail closed by treating any non-zero exit the
same as an exception: after calling _run_git in is_dirty, check
result.returncode (or use result.check_returncode()) and if it's non-zero return
True (assume dirty) instead of proceeding to interpret stdout; continue to catch
subprocess.TimeoutExpired, FileNotFoundError (and optionally
subprocess.CalledProcessError) and return True on those errors.
In `@tests/test_git_utils.py`:
- Around line 23-36: The git_repo fixture creates a repo without guaranteeing
the branch name, causing flaky tests on environments with different git
defaults; update the git_repo fixture to explicitly set a consistent branch
(e.g., "main") right after init (either by using git init with a default branch
option or by creating and checking out "main" with git checkout -b main) so
subsequent operations rely on a known branch name; ensure the change is applied
within the git_repo fixture where subprocess.run calls perform git init/commit.
- Around line 158-164: The test fails because the remote fixture creates
origin/main while test_up_to_date and check_status assume origin/master; update
the git_repo_with_remote fixture (or the setup in test_up_to_date) to create a
remote tracking branch matching the expected name (origin/master) or change the
test to use origin/main, and then call check_status with the explicit branch
parameter (e.g., check_status(local, branch="main" or "master") ) so
check_status compares against the correct remote branch after fetch; ensure you
still invoke fetch(local) before calling check_status to populate the remote
ref.
- Around line 39-59: The fixture pushes to "main" but the default branch can be
"master" in some environments; update the git_repo_with_remote (and the git_repo
fixture) to explicitly create and use a known branch (e.g., "main"): when
creating the bare remote or local repo set the initial branch (use git init
--initial-branch main or, for older git, run git symbolic-ref HEAD
refs/heads/main after init), ensure the cloned local repo is on that branch
before committing/pushing, and push that explicit branch (use the same branch
name variable for add/commit/push) so check_status looking for
origin/main/origin/master is consistent across environments.
- Around line 191-207: The test failure occurs because the second clone's push
doesn’t set the upstream and/or branch names diverge, causing
pull_ff_only(local) to see divergent histories; update the fixture or test so
the second clone pushes with upstream tracking (use git push -u origin main or
equivalent when pushing from the "second" clone) and ensure both clones use the
same branch name (e.g., "main") before committing, and make the pull call
operate on that branch by ensuring fetch(local) and pull_ff_only(local) target
the same branch; locate this logic around test_pull_success, the
git_repo_with_remote fixture, the "second" clone setup, fetch, and pull_ff_only
to apply the changes.
---
Nitpick comments:
In `@src/mcpm/core/source.py`:
- Around line 137-143: The branch that handles a provided directory currently
returns GitSource(path=directory) even when Path(directory).exists() is False;
update _resolve logic (the block containing dir_path, _find_git_root, and
GitSource) so that if dir_path.exists() is False you return
UnknownSource(path=directory) (or otherwise mark it as unknown) instead of
constructing a GitSource; reference GitSource, UnknownSource and _find_git_root
so the change is applied in the same code path and keep update.py's existing
"path not found" validation consistent with this returned UnknownSource.
- Around line 94-105: The _extract_npx_package function only strips "@latest"
and misses other version suffixes; update it to remove any trailing version
specifier after the package name by trimming the substring from the last "@"
that denotes a version (not the scope separator). Concretely, in
_extract_npx_package when you compute name, detect scoped packages
(name.startswith("@")) and if so find the last "@" position and remove
everything from that position onward only if that "@" comes after the scope
slash, otherwise keep the scope/name; for unscoped packages simply split on the
first "@" and take the left side. Ensure the modified logic preserves scoped
package names like "@scope/name" and returns None for flag-only args.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e95eb587-b06f-48a3-b702-e5f110944418
📒 Files selected for processing (10)
README.mdsrc/mcpm/cli.pysrc/mcpm/commands/new.pysrc/mcpm/commands/uninstall.pysrc/mcpm/commands/update.pysrc/mcpm/core/source.pysrc/mcpm/utils/git.pytests/test_git_utils.pytests/test_source.pytests/test_update.py
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/test_git_utils.py (1)
166-184: Consider explicitly specifying the branch forgit pushcommands in test setup.The
git pushcommand on line 177 (and similar occurrences on lines 202, 220, 256) relies on git's default push behavior. While the fixture now properly creates themainbranch, the second clone's push may still behave unexpectedly in some environments wherepush.defaultis not set tocurrentorsimple.♻️ Proposed fix for consistency
(second / "new.txt").write_text("new content") subprocess.run(["git", "add", "."], cwd=str(second), capture_output=True) subprocess.run(["git", "commit", "-m", "new commit"], cwd=str(second), capture_output=True) - subprocess.run(["git", "push"], cwd=str(second), capture_output=True) + subprocess.run(["git", "push", "origin", "main"], cwd=str(second), capture_output=True)Apply similar changes to lines 202, 220, and 256.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/test_git_utils.py` around lines 166 - 184, In test_commits_behind (and the other test setups that perform pushes), the subprocess.run(["git", "push"]) calls rely on git config defaults; change them to explicitly push the intended branch (e.g., subprocess.run(["git", "push", "origin", "main"], ...)) so the second clone pushes the correct branch; update the pushes in the test_commits_behind function and the similar push calls referenced around the other tests (the other occurrences on the same test file) to use "origin" and the "main" branch, leaving the surrounding calls to fetch(local) and check_status(local) unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@tests/test_git_utils.py`:
- Around line 166-184: In test_commits_behind (and the other test setups that
perform pushes), the subprocess.run(["git", "push"]) calls rely on git config
defaults; change them to explicitly push the intended branch (e.g.,
subprocess.run(["git", "push", "origin", "main"], ...)) so the second clone
pushes the correct branch; update the pushes in the test_commits_behind function
and the similar push calls referenced around the other tests (the other
occurrences on the same test file) to use "origin" and the "main" branch,
leaving the surrounding calls to fetch(local) and check_status(local) unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 725ce242-10b1-487c-b19f-d8b867fa2c32
📒 Files selected for processing (3)
tests/test_git_utils.pytests/test_source.pytests/test_update.py
DjodyKort
force-pushed
the
feat/update-command
branch
from
March 20, 2026 13:41
60a920d to
776c808
Compare
Adds source metadata tracking (sources.json) and a new update command that checks git-based servers for available updates and applies them.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/mcpm/core/source.py`:
- Around line 74-81: The current _find_git_root preserves relative parents so
relative --directory values (like ".") get persisted into GitSource.path; update
the logic to resolve paths to absolute before walking and when persisting
GitSource.path: in _find_git_root(start_path: Path) call start_path =
start_path.resolve() (and use resolved parents) so the returned repo root is
absolute, and ensure the code that constructs/saves GitSource.path (the place
that writes the raw --directory value) stores str(path.resolve()) or the
resolved root returned by _find_git_root rather than the original raw string;
update any code that calls _find_git_root or assigns GitSource.path to use the
resolved Path to avoid saving relative paths.
- Around line 197-205: After loading JSON into data in the SourcesManager (where
code opens self.sources_path and assigns json.load(f) to data), validate that
data is a dict before iterating; if not, log an error including the path and the
unexpected type and return {} so the file is treated as corrupted metadata.
Specifically, after the json.load assignment and before the for name,
source_data in data.items() loop, add an isinstance(data, dict) check and handle
non-dict payloads by logging and returning an empty dict.
- Around line 216-219: The loop that serializes self._sources into data before
writing sources_path currently includes GitSource.remote_url verbatim, risking
credential leakage; update the serialization step in that method to redact or
omit userinfo from remote_url before calling source.model_dump(mode="json") (or
post-process the dumped dict): parse each source's remote_url (identify
GitSource or attribute remote_url), remove username/password/userinfo from the
URL (or set remote_url to None or a masked value like
"https://<redacted>@host/..."), then serialize the sanitized dict into data and
json.dump it to sources_path so no credentials are persisted.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9dd8bc74-2c3d-4c3b-97ae-9fa477442946
📒 Files selected for processing (10)
README.mdsrc/mcpm/cli.pysrc/mcpm/commands/new.pysrc/mcpm/commands/uninstall.pysrc/mcpm/commands/update.pysrc/mcpm/core/source.pysrc/mcpm/utils/git.pytests/test_git_utils.pytests/test_source.pytests/test_update.py
✅ Files skipped from review due to trivial changes (2)
- README.md
- tests/test_git_utils.py
🚧 Files skipped from review as they are similar to previous changes (5)
- src/mcpm/commands/uninstall.py
- src/mcpm/commands/new.py
- tests/test_source.py
- src/mcpm/cli.py
- src/mcpm/commands/update.py
DjodyKort
force-pushed
the
feat/update-command
branch
from
March 20, 2026 14:07
776c808 to
7149d7f
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
src/mcpm/core/source.py (2)
74-80:⚠️ Potential issue | 🟠 MajorResolve repo paths before persisting
GitSource.path.Relative
--directoryvalues like.or~/repostay unresolved here. With--directory .,_find_git_root()never checks the current directory and Line 150 stores a cwd-dependent path, somcpm updateonly works from the same working directory.🐛 Suggested fix
def _find_git_root(start_path: Path) -> Optional[Path]: """Walk up from start_path looking for a .git directory.""" - current = start_path if start_path.is_dir() else start_path.parent + start_path = start_path.expanduser().resolve(strict=False) + current = start_path if start_path.is_dir() else start_path.parent while current != current.parent: if (current / ".git").exists(): return current current = current.parent + if (current / ".git").exists(): + return current return None @@ if command == "uv" and args and args[0] == "run": directory = _extract_directory_from_args(args) if directory: - dir_path = Path(directory) + dir_path = Path(directory).expanduser().resolve(strict=False) if dir_path.exists(): git_root = _find_git_root(dir_path) if git_root: return GitSource(path=str(git_root)) - return GitSource(path=directory) + return GitSource(path=str(dir_path))Also applies to: 141-150
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/mcpm/core/source.py` around lines 74 - 80, The code fails to resolve user/relative paths before searching and persisting GitSource.path, causing cwd-dependent behavior; update _find_git_root to accept and operate on a resolved absolute Path (call Path.expanduser().resolve() on start_path and use that as the initial current), and ensure wherever GitSource.path is assigned/stored (the code around the GitSource creation/assignment near lines 141-150) you persist the resolved absolute path rather than the original relative input so that repo detection and stored paths are independent of the current working directory.
197-205:⚠️ Potential issue | 🟠 MajorReject non-object
sources.jsonpayloads up front.Truthy non-object JSON (
["x"],"foo",123) reaches Line 205 and blows up ondata.items(), while falsy payloads are currently masked byor {}. Treat anything other than a JSON object as corrupted metadata and return{}.🛡️ Suggested fix
try: with open(self.sources_path, "r", encoding="utf-8") as f: - data = json.load(f) or {} + data = json.load(f) except (json.JSONDecodeError, OSError) as e: logger.error(f"Error loading sources from {self.sources_path}: {e}") return {} + if not isinstance(data, dict): + logger.error( + "Error loading sources from %s: expected a JSON object, got %s", + self.sources_path, + type(data).__name__, + ) + return {} sources = {}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/mcpm/core/source.py` around lines 197 - 205, After loading JSON into data in the code that opens self.sources_path (inside the method in src/mcpm/core/source.py), validate that data is a dict/object before using data.items(): replace the current "data = json.load(f) or {}" pattern with loading into data and then check "if not isinstance(data, dict): logger.error(f'Invalid/corrupted sources.json at {self.sources_path}: expected JSON object, got {type(data).__name__}'); return {}" so non-object JSON payloads (lists, strings, numbers, etc.) are rejected up front and you avoid calling data.items() on invalid types when building sources.src/mcpm/commands/update.py (1)
116-121:⚠️ Potential issue | 🟠 MajorRedact credentials before storing or echoing
remote_url.
git remote get-urlcan returnhttps://user:token@host/.... Assigning it directly here leaks secrets twice: Line 121 prints them, and Line 168 persists them throughsources.set(). Sanitize the URL before assigning it todetected.remote_url.🔒 Example approach
from urllib.parse import urlsplit, urlunsplit def _sanitize_remote_url(url: str) -> str: if "://" not in url: return url parts = urlsplit(url) if not (parts.username or parts.password): return url host = parts.hostname or "" if parts.port: host = f"{host}:{parts.port}" return urlunsplit((parts.scheme, host, parts.path, parts.query, parts.fragment))- detected.remote_url = git_utils.get_remote_url(repo_path) + remote_url = git_utils.get_remote_url(repo_path) + detected.remote_url = _sanitize_remote_url(remote_url) if remote_url else NoneAlso applies to: 168-168
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/mcpm/commands/update.py` around lines 116 - 121, Sanitize remote URLs returned by git_utils.get_remote_url before assigning to detected.remote_url and before persisting via sources.set to avoid leaking credentials; implement a small helper (e.g., _sanitize_remote_url) that uses urllib.parse.urlsplit/urlunsplit to strip username/password (preserve scheme, host[:port], path, query, fragment) and call it when setting detected.remote_url and when printing/storing values so no credentials from git remote URLs are echoed or saved.
🧹 Nitpick comments (1)
tests/test_git_utils.py (1)
23-59: Make the git scaffolding fail fast.These setup calls ignore their exit codes. If
init,commit, orpushfails, the test keeps running against a half-built repo and the eventual assertion points at the wrong thing. Addcheck=Truehere; the same pattern is worth applying to the later second-clone setup blocks too.♻️ Example
- subprocess.run(["git", "init", "-b", "main"], cwd=str(repo), capture_output=True) + subprocess.run(["git", "init", "-b", "main"], cwd=str(repo), capture_output=True, check=True) ... - subprocess.run(["git", "commit", "-m", "initial"], cwd=str(repo), capture_output=True) + subprocess.run(["git", "commit", "-m", "initial"], cwd=str(repo), capture_output=True, check=True)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/test_git_utils.py` around lines 23 - 59, The subprocess.run calls in the git_repo and git_repo_with_remote fixtures do not check exit codes and can leave tests running against a partially set-up repo; update every subprocess.run invocation inside the git_repo and git_repo_with_remote fixtures (including the git init, git config, git add, git commit, git clone, git push, and the bare repo init calls) to pass check=True so failures raise immediately and the fixture fails fast.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/mcpm/commands/update.py`:
- Around line 314-324: The code currently calls sources.mark_updated(name) and
increments success_count before verifying the post-update hook; change the flow
so that sources.mark_updated(name) and success_count += 1 only occur after
verifying post_success: if source.post_update is set, run
_run_post_update(source.post_update, repo_path) and only when it returns True
call sources.mark_updated(name) and increment success_count; if there is no
post_update, keep the original behavior (mark and count immediately). Ensure the
failure branch still prints the manual-run hint and does not mark the source or
increment success_count.
---
Duplicate comments:
In `@src/mcpm/commands/update.py`:
- Around line 116-121: Sanitize remote URLs returned by git_utils.get_remote_url
before assigning to detected.remote_url and before persisting via sources.set to
avoid leaking credentials; implement a small helper (e.g., _sanitize_remote_url)
that uses urllib.parse.urlsplit/urlunsplit to strip username/password (preserve
scheme, host[:port], path, query, fragment) and call it when setting
detected.remote_url and when printing/storing values so no credentials from git
remote URLs are echoed or saved.
In `@src/mcpm/core/source.py`:
- Around line 74-80: The code fails to resolve user/relative paths before
searching and persisting GitSource.path, causing cwd-dependent behavior; update
_find_git_root to accept and operate on a resolved absolute Path (call
Path.expanduser().resolve() on start_path and use that as the initial current),
and ensure wherever GitSource.path is assigned/stored (the code around the
GitSource creation/assignment near lines 141-150) you persist the resolved
absolute path rather than the original relative input so that repo detection and
stored paths are independent of the current working directory.
- Around line 197-205: After loading JSON into data in the code that opens
self.sources_path (inside the method in src/mcpm/core/source.py), validate that
data is a dict/object before using data.items(): replace the current "data =
json.load(f) or {}" pattern with loading into data and then check "if not
isinstance(data, dict): logger.error(f'Invalid/corrupted sources.json at
{self.sources_path}: expected JSON object, got {type(data).__name__}'); return
{}" so non-object JSON payloads (lists, strings, numbers, etc.) are rejected up
front and you avoid calling data.items() on invalid types when building sources.
---
Nitpick comments:
In `@tests/test_git_utils.py`:
- Around line 23-59: The subprocess.run calls in the git_repo and
git_repo_with_remote fixtures do not check exit codes and can leave tests
running against a partially set-up repo; update every subprocess.run invocation
inside the git_repo and git_repo_with_remote fixtures (including the git init,
git config, git add, git commit, git clone, git push, and the bare repo init
calls) to pass check=True so failures raise immediately and the fixture fails
fast.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 740a5c18-aad4-4510-a778-6f638f8d74b6
📒 Files selected for processing (10)
README.mdsrc/mcpm/cli.pysrc/mcpm/commands/new.pysrc/mcpm/commands/uninstall.pysrc/mcpm/commands/update.pysrc/mcpm/core/source.pysrc/mcpm/utils/git.pytests/test_git_utils.pytests/test_source.pytests/test_update.py
✅ Files skipped from review due to trivial changes (2)
- README.md
- tests/test_update.py
🚧 Files skipped from review as they are similar to previous changes (5)
- src/mcpm/cli.py
- src/mcpm/commands/uninstall.py
- src/mcpm/commands/new.py
- tests/test_source.py
- src/mcpm/utils/git.py
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/mcpm/commands/update.py (1)
332-332: Consider adding type hints for better maintainability.The helper functions
_check_git_serverand_source_type_labellack type annotations.♻️ Suggested type hints
-def _check_git_server(name, source, sources, updatable, verbose) -> bool: +def _check_git_server( + name: str, + source: GitSource, + sources: SourcesManager, + updatable: list, + verbose: bool, +) -> bool:-def _source_type_label(source) -> str: +def _source_type_label(source: object) -> str:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/mcpm/commands/update.py` at line 332, Add type annotations to the helper functions: change _check_git_server to def _check_git_server(name: str, source: Mapping[str, Any], sources: Sequence[Mapping[str, Any]], updatable: bool, verbose: bool) -> bool and change _source_type_label to def _source_type_label(source: Mapping[str, Any]) -> str (or similar if source is a specific dict shape use Dict[str, Any]); import the needed typing symbols (Mapping, Sequence, Any, Dict) at the top of the module and update any related references to satisfy the new signatures.tests/test_update.py (1)
436-436: Platform-dependent test command.The
falsecommand used here exists on Unix but not Windows. If Windows CI/dev support is needed, consider using a cross-platform alternative.♻️ Cross-platform alternative
- mgr.set("my-server", GitSource(path=str(repo_dir), branch="main", post_update="false")) + mgr.set("my-server", GitSource(path=str(repo_dir), branch="main", post_update="exit 1"))Or mock
_run_post_updatedirectly if subprocess execution is not being tested.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/test_update.py` at line 436, The test uses the Unix-only "false" command in mgr.set(..., GitSource(..., post_update="false")), which breaks on Windows; either change the post_update to a cross-platform invocation (e.g., use the running Python interpreter via sys.executable to run a short one-liner) or avoid spawning a subprocess by mocking the internal helper (_run_post_update) so the test does not rely on platform-specific shell commands; update the test to use GitSource and mgr.set but supply a platform-neutral post_update command or add a mock for _run_post_update.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/mcpm/commands/update.py`:
- Line 332: Add type annotations to the helper functions: change
_check_git_server to def _check_git_server(name: str, source: Mapping[str, Any],
sources: Sequence[Mapping[str, Any]], updatable: bool, verbose: bool) -> bool
and change _source_type_label to def _source_type_label(source: Mapping[str,
Any]) -> str (or similar if source is a specific dict shape use Dict[str, Any]);
import the needed typing symbols (Mapping, Sequence, Any, Dict) at the top of
the module and update any related references to satisfy the new signatures.
In `@tests/test_update.py`:
- Line 436: The test uses the Unix-only "false" command in mgr.set(...,
GitSource(..., post_update="false")), which breaks on Windows; either change the
post_update to a cross-platform invocation (e.g., use the running Python
interpreter via sys.executable to run a short one-liner) or avoid spawning a
subprocess by mocking the internal helper (_run_post_update) so the test does
not rely on platform-specific shell commands; update the test to use GitSource
and mgr.set but supply a platform-neutral post_update command or add a mock for
_run_post_update.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f8fe7af6-3c79-4a1e-8a9f-d21902db0dc1
📒 Files selected for processing (2)
src/mcpm/commands/update.pytests/test_update.py
JoJoJoJoJoJoJo
approved these changes
Mar 27, 2026
JoJoJoJoJoJoJo
left a comment
JoJoJoJoJoJoJo
left a comment
Contributor
There was a problem hiding this comment.
Great idea! Thanks for your contribution ❤️
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
mcpm-semantic-release Bot
pushed a commit
that referenced
this pull request
Mar 27, 2026
# [2.14.0](v2.13.0...v2.14.0) (2026-03-27) ### Features * add `mcpm update` command for server update management ([#315](#315)) ([efc2916](efc2916))
|
🎉 This PR is included in version 2.14.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds an
mcpm updatecommand that checks for and applies updates to installed MCP servers. This addresses a gap where mcpm can install and configure servers but has no way to update them — users must manually check for new versions and update each server individually.git fetch+git pull --ff-only(with--rebaseopt-in), automatic post-update build steps (e.g.uv sync,npm install)sources.jsonfile tracks where each server came from, populated automatically onmcpm install/mcpm newand viamcpm update --initNew commands
Key design decisions
--ff-onlyby default instead of--rebase— safe, either succeeds cleanly or fails explicitlysources.jsonin~/.config/mcpm/— consistent with existingservers.jsonpatternpost_updatefield instead of auto-detection — user confirms during--initsubprocess,requests,pydantic,rich,click(all existing)Files changed
New:
src/mcpm/commands/update.py— the update commandsrc/mcpm/core/source.py— source metadata models + manager + auto-detectionsrc/mcpm/utils/git.py— git operations with timeouts and error handlingModified:
src/mcpm/cli.py— register update commandsrc/mcpm/commands/new.py— auto-populate source metadata on server creationsrc/mcpm/commands/uninstall.py— clean up source metadata on server removalREADME.md— document the new commandTests (67 new):
tests/test_source.py— source detection + SourcesManagertests/test_git_utils.py— git operations with real repostests/test_update.py— CLI, init, apply flow, hook integrationTest plan
uv run pytest tests/— 252 tests pass (67 new, 0 broken)uv run ruff check src/ tests/— cleanmcpm update --initcorrectly detects git/npx/remote/unknown serversmcpm update --check --verboseshows commits behind with summariesmcpm update SERVER --forcepulls and runs post_updateSummary by CodeRabbit
New Features
mcpm updatecommand (init/re-detect sources,--check,--rebase,--force, non-interactive flow, post-update hooks); git servers default to fast‑forward pulls; NPX/UVX treated as runtime auto-updaters.newnow records sources anduninstallremoves them.Documentation
Tests