Conversation
Add commit-pinned MCP repository discovery, preview, approval, update, rollback, and removal across the gateway, web UI, main CLI, and moltis-ctl. Repositories are sparsely materialized from explicit manifests, imported disabled, and approval is bound to the exact commit and configuration digest. Private Git credentials use vault-aware storage, strict SSH host pins, DNS/IP controls, fetch quotas, and atomic registry locking.
Adapt managed repository working directories to the newer StdioLaunchOptions API introduced on main.
Greptile SummaryThis PR adds managed Git repository bundles for MCP servers, including repository discovery, approval-gated installation, updates, rollback, removal, credential handling, imports, CLI/RPC integration, and web onboarding.
Confidence Score: 5/5The PR appears safe to merge based on the reviewed follow-up issues. No blocking failure remains; current code preserves active repository revisions during pruning and rejects sealed-vault credential writes without persisting plaintext.
|
| Filename | Overview |
|---|---|
| crates/gateway/src/mcp_service/repositories_storage.rs | Manages owned revision retention and now preserves the registry’s active and previous commits across successful install, update, and rollback operations. |
| crates/auth/src/credential_store/git_https_credentials.rs | Adds transactional HTTPS credential persistence that encrypts tokens when configured and rejects writes when the configured vault cannot encrypt them. |
| crates/gateway/src/mcp_service/repositories.rs | Orchestrates preview, approval-gated installation, update, rollback, removal, and revision cleanup for managed repositories. |
| crates/mcp/src/managed_repositories.rs | Defines managed repository state and lifecycle behavior used by the MCP manager and registry. |
| crates/git-repositories/src/transport.rs | Implements managed HTTPS and pinned SSH transport for repository fetching. |
| crates/web/ui/src/pages/mcp/RepositoryInstaller.tsx | Adds the guided repository preview and installation workflow with shorthand defaults and advanced deployment options. |
Sequence Diagram
sequenceDiagram
participant User
participant UI as CLI / Web UI
participant Gateway
participant Repo as Git Repository
participant Registry as MCP Registry
participant Vault
User->>UI: Preview repository
UI->>Gateway: Repository source and credential selection
Gateway->>Vault: Resolve HTTPS credential or SSH identity
Gateway->>Repo: Fetch pinned revision
Repo-->>Gateway: Repository tree and commit
Gateway-->>UI: Sanitized preview and config digest
User->>UI: Approve selected servers
UI->>Gateway: Install with approved commit and digest
Gateway->>Registry: Commit disabled, unapproved servers
Gateway->>Gateway: Retain active and previous revisions
Gateway-->>UI: Installed repository state
Reviews (7): Last reviewed commit: "test(mcp): remove SSE mock server conten..." | Re-trigger Greptile
There was a problem hiding this comment.
Pull request overview
Adds a first-class “managed MCP repositories” feature that can materialize MCP server definitions from explicit Git manifests (HTTPS/SSH/local), preview/sanitize them, and install/update/rollback/remove them with commit/digest approval gates. This threads through the gateway service layer, CLI/RPC/Web UI workflows, introduces secure Git materialization utilities, and integrates vault lifecycle encryption for new HTTPS Git credentials.
Changes:
- Introduce managed repository discovery/materialization + approval-gated installation/update/rollback/remove flows for MCP servers.
- Add managed Git HTTPS credential storage (with vault migration / disable integration) and surface credential/SSH metadata in the UI.
- Evolve the MCP registry persistence format (servers + repositories), add atomic save semantics, and update importers/tests/docs/container deps accordingly.
Reviewed changes
Copilot reviewed 90 out of 91 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| README.md | Updates feature list to mention managed Git repositories for MCP extensibility. |
| docs/src/mcp.md | Documents managed repository CLI/online flows, approval gates, manifests, and security constraints. |
| Dockerfile | Installs git and openssh-client to support managed repo fetch/SSH flows in the official image. |
| crates/web/ui/src/types/rpc-methods.ts | Adds RPC method keys for managed repos + Git credentials. |
| crates/web/ui/src/pages/McpPage.tsx | Splits “manual” vs managed servers and renders the managed repositories section. |
| crates/web/ui/src/pages/mcp/types.ts | Adds typed models for repository sources, previews, warnings, credentials, and installed state. |
| crates/web/ui/src/pages/mcp/rpc.ts | Adds a small RPC helper for managed-repo related calls with consistent error handling. |
| crates/web/ui/src/pages/mcp/RepositoryPreview.tsx | UI panel to inspect candidates/warnings and choose what to install. |
| crates/web/ui/src/pages/mcp/RepositoryInstaller.tsx | Form to preview + install managed repos (HTTPS/SSH/local) and select candidates. |
| crates/web/ui/src/pages/mcp/ManagedRepositories.tsx | Container component that loads repos/credentials, wires refresh/events, and composes subpanels. |
| crates/web/ui/src/pages/mcp/GitCredentials.tsx | UI to create/update/remove HTTPS Git credentials and list SSH target metadata. |
| crates/vault/src/migration.rs | Adds vault migration routine + tests to encrypt plaintext HTTPS Git credential tokens. |
| crates/service-traits/src/tests.rs | Extends noop service tests to cover managed MCP read/write availability behavior. |
| crates/service-traits/src/interfaces.rs | Extends McpService trait with managed repo and credential methods (+ noop defaults). |
| crates/openclaw-import/src/mcp_servers.rs | Refactors MCP server import to use import-core merge logic and preserve managed registry state. |
| crates/openclaw-import/Cargo.toml | Adds dependency on moltis-import-core. |
| crates/mcp/tests/fixtures/yolo-marketplace/.claude-plugin/marketplace.json | Adds marketplace fixture for repo discovery tests. |
| crates/mcp/tests/fixtures/yolo-marketplace/.mcp.json | Adds root .mcp.json fixture used to assert marketplace precedence behavior. |
| crates/mcp/tests/fixtures/yolo-marketplace/plugins/inline-plugin/.claude-plugin/plugin.json | Adds inline plugin fixture manifest. |
| crates/mcp/tests/fixtures/yolo-marketplace/plugins/plugin-1/.claude-plugin/plugin.json | Adds plugin fixture manifest. |
| crates/mcp/tests/fixtures/yolo-marketplace/plugins/plugin-1/.mcp.json | Adds plugin fixture .mcp.json. |
| crates/mcp/tests/fixtures/yolo-marketplace/plugins/plugin-2/.claude-plugin/plugin.json | Adds plugin fixture manifest. |
| crates/mcp/tests/fixtures/yolo-marketplace/plugins/plugin-2/.mcp.json | Adds plugin fixture .mcp.json. |
| crates/mcp/tests/fixtures/yolo-marketplace/plugins/plugin-3/.claude-plugin/plugin.json | Adds plugin fixture manifest. |
| crates/mcp/tests/fixtures/yolo-marketplace/plugins/plugin-3/.mcp.json | Adds plugin fixture .mcp.json. |
| crates/mcp/tests/fixtures/yolo-marketplace/plugins/plugin-4/.claude-plugin/plugin.json | Adds plugin fixture manifest. |
| crates/mcp/tests/fixtures/yolo-marketplace/plugins/plugin-4/.mcp.json | Adds plugin fixture .mcp.json. |
| crates/mcp/tests/fixtures/yolo-marketplace/plugins/plugin-5/.claude-plugin/plugin.json | Adds plugin fixture manifest. |
| crates/mcp/tests/fixtures/yolo-marketplace/plugins/plugin-5/.mcp.json | Adds plugin fixture .mcp.json. |
| crates/mcp/tests/fixtures/yolo-marketplace/plugins/plugin-6/.claude-plugin/plugin.json | Adds plugin fixture manifest. |
| crates/mcp/tests/fixtures/yolo-marketplace/plugins/plugin-6/.mcp.json | Adds plugin fixture .mcp.json. |
| crates/mcp/tests/fixtures/yolo-marketplace/plugins/plugin-7/.claude-plugin/plugin.json | Adds plugin fixture manifest. |
| crates/mcp/tests/fixtures/yolo-marketplace/plugins/plugin-7/.mcp.json | Adds plugin fixture .mcp.json. |
| crates/mcp/tests/fixtures/yolo-marketplace/plugins/plugin-8/.claude-plugin/plugin.json | Adds plugin fixture manifest. |
| crates/mcp/tests/fixtures/yolo-marketplace/plugins/plugin-8/.mcp.json | Adds plugin fixture .mcp.json. |
| crates/mcp/tests/fixtures/yolo-marketplace/unrelated/nested/.mcp.json | Adds unrelated nested fixture to assert bounded discovery rules. |
| crates/mcp/src/transport.rs | Adds stdio transport test ensuring configured working directory is applied. |
| crates/mcp/src/remote.rs | Extends env placeholder handling and adds placeholder-name extraction utility + tests. |
| crates/mcp/src/registry.rs | Evolves registry format (servers + repositories), adds atomic save, and enforces managed server invariants. |
| crates/mcp/src/manager/lifecycle_tests.rs | Adds lifecycle race tests for start invalidation during remove/update. |
| crates/mcp/src/manager_managed.rs | Adds McpManager helpers for managed repository reconciliation and approval persistence. |
| crates/mcp/src/lib.rs | Exposes managed repository and repository discovery APIs publicly. |
| crates/mcp/src/config_parsing.rs | Adds cwd parsing support and preserves managed provenance on updates. |
| crates/mcp/Cargo.toml | Adds git-repositories + related deps/features for managed repository functionality. |
| crates/import-core/Cargo.toml | Promotes tempfile/fs2 to main deps (used in import merge logic). |
| crates/httpd/src/ssh_routes.rs | Routes SSH key/target deletion through MCP service for managed-repo reference safety. |
| crates/httpd/src/auth_routes/vault.rs | Adds Git credential mutation guard + vault disable report field for git credentials. |
| crates/git-repositories/src/materialize.rs | Introduces secure, bounded materialization of explicit MCP content from immutable Git revisions. |
| crates/git-repositories/src/lib.rs | Exposes git materialization primitives (sources, limits, backends). |
| crates/git-repositories/src/error.rs | Defines error types for repository source/transport/materialization failures. |
| crates/git-repositories/Cargo.toml | Adds new moltis-git-repositories crate and feature flags. |
| crates/gateway/src/vault_lifecycle.rs | Adds vault migration + disable-time decryption for HTTPS Git credentials (+ tests). |
| crates/gateway/src/server/prepare_core.rs | Wires managed-repo lock + passes data dir/lock into LiveMcpService; changes registry load behavior. |
| crates/gateway/src/onboarding.rs | Imports MCP servers into the live MCP service (instead of file merge) for onboarding flows. |
| crates/gateway/src/methods/services/system.rs | Registers repository/credential RPC methods against the MCP service. |
| crates/gateway/src/methods/dispatch.rs | Classifies managed repo + git credential RPC methods into read/write auth scopes (+ tests). |
| crates/gateway/src/mcp_service/repositories_types.rs | Adds request/response types and helpers for managed repo + credential RPC endpoints. |
| crates/gateway/src/mcp_service/repositories_storage.rs | Adds preview materialization/discovery storage utilities and reconciliation diff computation. |
| crates/gateway/src/mcp_service/repositories_sanitize.rs | Adds argument sanitization helpers to reduce accidental secret exposure in previews. |
| crates/gateway/src/mcp_service/repositories_credentials.rs | Implements Git credential CRUD + managed SSH delete guards with repository reference checks. |
| crates/gateway/src/mcp_service/repositories_credentials_tests.rs | Adds focused tests for credential deletion races and SSH reference constraints. |
| crates/gateway/src/mcp_service/repositories.rs | Implements managed repository RPC operations (list/preview/install/update/rollback/remove/approve). |
| crates/gateway/src/mcp_service.rs | Wires repository module, tracks operation concurrency, and adds managed RPC method implementations. |
| crates/gateway/migrations/20260728153305_git_https_credentials.sql | Adds git_https_credentials table migration. |
| crates/gateway/Cargo.toml | Adds moltis-git-repositories + moltis-import-core deps and metrics plumbing. |
| crates/ctl/src/commands/mcp.rs | Adds moltis-ctl mcp repo ... commands for online repository operations via RPC. |
| crates/codex-import/src/mcp_servers.rs | Refactors Codex MCP server import to preserve managed registry state via import-core merge. |
| crates/cli/src/main.rs | Adds top-level moltis mcp ... command wiring. |
| crates/cli/Cargo.toml | Adds deps/features required for offline managed repo + credential flows. |
| crates/claude-import/src/mcp_servers.rs | Refactors Claude MCP server import to collect then merge, preserving managed registry state. |
| crates/auth/src/error.rs | Adds a Git credential validation error variant. |
| crates/auth/src/credential_store/types.rs | Adds Git HTTPS credential types (entry vs resolved secret-bearing struct) with redacted Debug. |
| crates/auth/src/credential_store/tests.rs | Adds CRUD, validation, reset_all, and vault encryption behavior tests for Git HTTPS credentials. |
| crates/auth/src/credential_store/sessions.rs | Creates git credentials table in embedded store setup and clears it on reset_all. |
| crates/auth/src/credential_store/git_https_credentials.rs | Implements credential CRUD with vault-aware token encryption and a mutation guard for lifecycle safety. |
| crates/auth/src/credential_store.rs | Wires git credentials module and exports its types/guards. |
| Cargo.toml | Adds new crate to workspace members/default-members and adds workspace deps/features for gix/fs2. |
| Cargo.lock | Locks new crate additions/dependency graph changes for managed repo support. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Merging this PR will not alter performance
Comparing Footnotes
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 90 out of 91 changed files in this pull request and generated no new comments.
Suppressed comments (4)
crates/gateway/src/server/prepare_core.rs:366
data_diris resolved from the CLI/global override just above, but the MCP registry is still loaded frommoltis_config::data_dir(). This can read the registry from a different directory than the one passed toLiveMcpService::new(...), breaking--data-dirdeployments and potentially causing managed repository state to diverge.
let mcp_registry_path = moltis_config::data_dir().join("mcp-servers.json");
let mcp_reg = moltis_mcp::McpRegistry::load(&mcp_registry_path)?;
crates/gateway/src/mcp_service/repositories_storage.rs:70
- The
materialize()error is discarded (map_err(|_| ...)), so callers lose actionable details like credential host mismatch, missing credentials, or manifest/limit failures. This makes troubleshooting managed repository installs much harder.
})
.await
.map_err(|_| ServiceError::message("repository materialization task failed"))?
.map_err(|_| ServiceError::message("repository materialization failed"))?;
discover_materialized_revision(
crates/mcp/src/registry.rs:165
is_structuredis inferred solely by the presence of top-level keysservers/repositories. A legacy flat registry that happens to contain a server namedserversorrepositories(valid runtime names) will now be mis-detected as the structured format and fail to load, creating a backwards-compatibility break for those users.
crates/gateway/src/mcp_service.rs:172- Using
saturating_add(1)can make this loop non-terminating if the suffix reachesu32::MAXand that candidate name is also taken (the suffix stops changing butadd_server_if_absentkeeps returningOk(false)). Guard against overflow so name allocation always terminates.
.await;
if !matches!(result, Ok(false)) {
break (candidate, result);
}
suffix = suffix.saturating_add(1);
Fail closed when vault encryption cannot protect HTTPS Git credentials, and keep managed repository storage rooted in the resolved data directory. Preserve actionable materialization errors, make revision retention explicit, disambiguate legacy registry entries, terminate name allocation on overflow, and use cryptographic request IDs.
|
@greptile review |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 93 out of 94 changed files in this pull request and generated no new comments.
Suppressed comments (4)
crates/web/ui/src/providers/shared.ts:191
createValidationRequestId()now callsglobalThis.crypto.randomUUID()directly. This can throw ifcrypto/randomUUIDis unavailable (older Chromium/WebView, non-secure contexts), and the codebase already uses a fallback pattern elsewhere (e.g.sw.ts:newNotificationId()). Consider adding a small fallback here to avoid runtime crashes during provider validation.
crates/web/ui/src/provider-validation-progress.ts:78createValidationRequestId()now callsglobalThis.crypto.randomUUID()directly. Ifcrypto.randomUUIDis missing, this will throw at runtime. There is already a fallback implementation incrates/web/ui/src/sw.ts(newNotificationId()), so it would be safer to mirror that pattern here as well.
crates/web/ui/src/pages/SkillsPage.tsx:174doInstall()now relies onglobalThis.crypto.randomUUID()foropId. Ifcrypto.randomUUIDis unavailable, this will throw and prevent skill installation. A small fallback (similar tosw.ts:newNotificationId()) avoids hard failures in older environments.
crates/gateway/src/mcp_service/repositories_types.rs:202BTreeSetCompatis currently an alias forHashSet, which is misleading and makes the code harder to read/maintain. Renaming the alias to reflect the actual type avoids confusion for future concurrency/ordering assumptions.
struct RepositoryOperationGuard {
id: ManagedRepositoryId,
operations: Arc<Mutex<BTreeSetCompat>>,
}
type BTreeSetCompat = std::collections::HashSet<ManagedRepositoryId>;
Use a shared Web Crypto helper that falls back to getRandomValues on browsers without randomUUID, while keeping request identifiers cryptographically strong. Rename the managed repository operation set to match its actual HashSet type.
|
@greptile review |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 94 out of 95 changed files in this pull request and generated no new comments.
Suppressed comments (2)
crates/git-repositories/src/materialize.rs:215
- sync_directory() fsyncs a directory via File::open + sync_all; this is not supported consistently across platforms/filesystems (notably Windows) and can cause repository materialization to fail even though the checkout itself succeeded. Consider making directory fsync a no-op on non-Unix platforms (or otherwise best-effort) to avoid breaking managed repository workflows.
crates/web/ui/src/random-id.ts:7 - randomId() assumes globalThis.crypto and getRandomValues/randomUUID are always available; in non-secure contexts or some runtimes this can be undefined/throw and crash UI flows that generate request/operation ids. Add a safe fallback (e.g., Math.random/Date.now) when WebCrypto is unavailable or errors.
Skip unsupported directory fsync operations on non-Unix targets. Keep request IDs available when Web Crypto is missing or fails by using a page-local timestamp and counter fallback, while retaining cryptographic IDs whenever Web Crypto works.
|
@greptile review |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 94 out of 95 changed files in this pull request and generated no new comments.
Suppressed comments (2)
crates/httpd/src/ssh_routes.rs:408
managed_ssh_target_remove()failures are currently always mapped to400 Bad Request. That can misclassify unexpected storage/DB failures as client errors (and the previous implementation returned 500 for delete failures). Consider returning 400 only for the expected “still assigned” case and 500 for everything else.
crates/httpd/src/ssh_routes.rs:347managed_ssh_key_remove()can fail for reasons other than “still assigned” (e.g., unexpected storage/DB errors). Mapping all failures to400 Bad Requestmakes operational errors look like client input issues and changes prior behavior (these routes previously returned 500 for some delete failures). Consider returning 400 only for the expected “still assigned” case and 500 for everything else.
This issue also appears on line 405 of the same file.
Classify repository-reference conflicts as invalid requests while keeping credential-store failures internal. HTTP SSH deletion routes now return 400 only for expected assignment conflicts and 500 for operational failures.
|
@greptile review |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 95 out of 96 changed files in this pull request and generated no new comments.
Suppressed comments (2)
crates/web/ui/src/pages/mcp/RepositoryPreview.tsx:46
candidate.warningsis mapped withkey={warning}. If the backend returns duplicate warning strings (e.g., multiple warnings with the same kind), this will produce duplicate keys and can cause incorrect badge rendering during updates. Use a stable unique key (e.g., include the index).
crates/service-traits/src/interfaces.rs:531- These default
McpServicemanaged-repository stubs returnServiceError::Messagevia"...".into(), which converts to anINTERNALprotocol error. For an unsupported method/feature, this should be classified asINVALID_REQUEST(viaServiceError::invalid_request) so clients/UI don't treat it as a server fault. Apply the same change to the other managed-repository / git-credential default methods in this block.
Return INVALID_REQUEST for unsupported managed repository and credential methods, including the no-op MCP service, so clients do not report absent features as server faults. Keep duplicate warning badges keyed independently.
|
@greptile review |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 95 out of 96 changed files in this pull request and generated no new comments.
Suppressed comments (1)
crates/gateway/src/mcp_service/repositories_types.rs:229
parse_params/parse_empty_paramscurrently wrap client input/shape errors withServiceError::message(...), which converts tomoltis_protocol::error_codes::INTERNAL. These are client-side request issues and should be classified asINVALID_REQUEST(using the newServiceError::invalid_request) so callers get a 4xx-equivalent error code instead of a 500.
fn parse_repository_id(value: String) -> Result<ManagedRepositoryId, ServiceError> {
ManagedRepositoryId::parse(value).map_err(service_message)
}
fn parse_params<T: for<'de> Deserialize<'de>>(params: Value) -> Result<T, ServiceError> {
serde_json::from_value(params).map_err(|error| ServiceError::message(error.to_string()))
}
fn parse_empty_params(params: Value) -> Result<(), ServiceError> {
if params.is_null() || params.as_object().is_some_and(serde_json::Map::is_empty) {
Ok(())
} else {
Err(ServiceError::message("this method accepts no parameters"))
}
Managed repository setup exposed deployment-oriented fields and timed out long Git operations after five seconds. Default the UI to GitHub owner/repo input, guide users through least-privilege tokens, filter SSH targets to backend-eligible entries, and give repository operations their full timeout budget.
Parallel coverage instrumentation could delay Mockito server threads past the production health timeout, while the cwd test could read its redirected output before pwd wrote it. Use a Tokio one-shot HTTP fixture and atomically publish the cwd result so both tests observe complete, independently owned state.
Workspace coverage instrumentation can starve Mockito's separate server runtimes until request timeouts expire, with different SSE cases failing from run to run. Serve the streamable HTTP fixtures directly on the test Tokio runtime and retain request-level assertions for auth, session replay, and shutdown behavior.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 97 out of 98 changed files in this pull request and generated no new comments.
Suppressed comments (2)
crates/gateway/src/mcp_service/repositories_types.rs:229
parse_params/parse_empty_params(andparse_repository_id) currently returnServiceError::message(...), which maps to the INTERNAL error code. These are client-side input/contract errors and should be classified asInvalidRequestso callers get the correct protocol error code and HTTP 400 mapping.
fn parse_repository_id(value: String) -> Result<ManagedRepositoryId, ServiceError> {
ManagedRepositoryId::parse(value).map_err(service_message)
}
fn parse_params<T: for<'de> Deserialize<'de>>(params: Value) -> Result<T, ServiceError> {
serde_json::from_value(params).map_err(|error| ServiceError::message(error.to_string()))
crates/git-repositories/src/materialize.rs:87
Materializer::materializeis the core security boundary for fetching and publishing immutable revisions (credential checks, fetch limits, publish-path safety). This new code path doesn't have unit tests in this module, so regressions in host-mismatch handling, cache-hit behavior (published.exists()), or publish-path containment could slip through.
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
|
@greptile review |
Summary
owner/reposhorthand, derived defaults, advanced deployment options, and guided fine-grained read-only tokensValidation
Completed
git diff --checknpx biome check --write src/helpers.ts src/pages/mcp/GitCredentials.tsx src/pages/mcp/RepositoryInstaller.tsx src/pages/mcp/rpc.ts e2e/specs/mcp-repositories.spec.jscd crates/web/ui && npm run build:css && npm run build && npx tsc --noEmitcd crates/web/ui && npx playwright test e2e/specs/mcp-repositories.spec.jscargo test -p moltis-mcp sse_transport::tests -- --nocapturecargo +nightly-2026-06-20 llvm-cov --workspace --exclude moltis-tools --exclude moltis-swift-bridge --lcov --output-path lcov.infojust release-preflightLOCAL_VALIDATE_TEST_CMD="cargo +nightly-2026-06-20 nextest run -p moltis-mcp sse_transport::tests" ./scripts/local-validate.sh 1183be41a42d9Remaining
Manual QA
owner/repo, and verify the alias and HTTPS URL are derived in preview.