Skip to content

feat(mcp): add managed repository bundles - #1183

Open
penso wants to merge 12 commits into
mainfrom
tin-sousaphone
Open

penso wants to merge 12 commits into
mainfrom
tin-sousaphone

Conversation

@penso

@penso penso commented Aug 2, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • add managed Git repository bundles for discovering, previewing, installing, updating, rolling back, and removing MCP servers
  • support HTTPS credentials, pinned managed SSH transport, vault lifecycle integration, and imported repository-backed MCP configurations
  • simplify web onboarding with GitHub owner/repo shorthand, derived defaults, advanced deployment options, and guided fine-grained read-only tokens
  • preserve mandatory sanitized previews and commit/config-digest approval gates across CLI, RPC, and web UI workflows

Validation

Completed

  • git diff --check
  • npx 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.js
  • cd crates/web/ui && npm run build:css && npm run build && npx tsc --noEmit
  • cd crates/web/ui && npx playwright test e2e/specs/mcp-repositories.spec.js
  • cargo test -p moltis-mcp sse_transport::tests -- --nocapture
  • cargo +nightly-2026-06-20 llvm-cov --workspace --exclude moltis-tools --exclude moltis-swift-bridge --lcov --output-path lcov.info
  • just release-preflight
  • LOCAL_VALIDATE_TEST_CMD="cargo +nightly-2026-06-20 nextest run -p moltis-mcp sse_transport::tests" ./scripts/local-validate.sh 1183
  • GitHub status checks for be41a42d9

Remaining

  • None.

Manual QA

  1. Open Settings → MCP Servers, enter a public GitHub repository as owner/repo, and verify the alias and HTTPS URL are derived in preview.
  2. Expand Advanced options and verify custom refs, IDs, aliases, HTTPS URLs, SSH remotes, and local paths remain available.
  3. Use Connect GitHub to open the fine-grained-token form, select only the intended repositories with Contents read-only, save the token, and preview a private repository.
  4. Configure pinned and unpinned SSH targets, then verify only managed-key, pinned targets matching the remote host and port are selectable.
  5. Install selected servers and confirm they remain disabled and unapproved until explicitly approved; then exercise update, rollback, and removal.

Fabien Penso added 3 commits August 2, 2026 16:24
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.
Copilot AI review requested due to automatic review settings August 2, 2026 23:25
Comment thread crates/web/ui/e2e/specs/mcp-repositories.spec.js Fixed
@greptile-apps

greptile-apps Bot commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This 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.

  • Introduces repository materialization and managed MCP lifecycle state.
  • Adds HTTPS credentials, pinned SSH transport, and vault lifecycle integration.
  • Adds CLI, RPC, documentation, tests, and web UI workflows for repository management.

Confidence Score: 5/5

The 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.

Important Files Changed

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
Loading

Reviews (7): Last reviewed commit: "test(mcp): remove SSE mock server conten..." | Re-trigger Greptile

Comment thread crates/gateway/src/mcp_service/repositories_storage.rs Outdated
Comment thread crates/auth/src/credential_store/git_https_credentials.rs Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread crates/gateway/src/server/prepare_core.rs Outdated
Comment thread crates/mcp/src/registry.rs
Comment thread crates/gateway/src/mcp_service/repositories_storage.rs
@codspeed

codspeed Bot commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

Merging this PR will not alter performance

✅ 39 untouched benchmarks
⏩ 9 skipped benchmarks1


Comparing tin-sousaphone (be41a42) with main (678d407)

Open in CodSpeed

Footnotes

  1. 9 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

Copilot AI review requested due to automatic review settings August 2, 2026 23:33

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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_dir is resolved from the CLI/global override just above, but the MCP registry is still loaded from moltis_config::data_dir(). This can read the registry from a different directory than the one passed to LiveMcpService::new(...), breaking --data-dir deployments 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_structured is inferred solely by the presence of top-level keys servers / repositories. A legacy flat registry that happens to contain a server named servers or repositories (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 reaches u32::MAX and that candidate name is also taken (the suffix stops changing but add_server_if_absent keeps returning Ok(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.
Copilot AI review requested due to automatic review settings August 3, 2026 00:04
@penso

penso commented Aug 3, 2026

Copy link
Copy Markdown
Collaborator Author

@greptile review

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 calls globalThis.crypto.randomUUID() directly. This can throw if crypto/randomUUID is 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:78
  • createValidationRequestId() now calls globalThis.crypto.randomUUID() directly. If crypto.randomUUID is missing, this will throw at runtime. There is already a fallback implementation in crates/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:174
  • doInstall() now relies on globalThis.crypto.randomUUID() for opId. If crypto.randomUUID is unavailable, this will throw and prevent skill installation. A small fallback (similar to sw.ts:newNotificationId()) avoids hard failures in older environments.
    crates/gateway/src/mcp_service/repositories_types.rs:202
  • BTreeSetCompat is currently an alias for HashSet, 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.
Copilot AI review requested due to automatic review settings August 3, 2026 00:19
@penso

penso commented Aug 3, 2026

Copy link
Copy Markdown
Collaborator Author

@greptile review

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.
Copilot AI review requested due to automatic review settings August 3, 2026 01:37
@penso

penso commented Aug 3, 2026

Copy link
Copy Markdown
Collaborator Author

@greptile review

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 to 400 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:347
  • managed_ssh_key_remove() can fail for reasons other than “still assigned” (e.g., unexpected storage/DB errors). Mapping all failures to 400 Bad Request makes 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.
Copilot AI review requested due to automatic review settings August 3, 2026 01:49
@penso

penso commented Aug 3, 2026

Copy link
Copy Markdown
Collaborator Author

@greptile review

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.warnings is mapped with key={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 McpService managed-repository stubs return ServiceError::Message via "...".into(), which converts to an INTERNAL protocol error. For an unsupported method/feature, this should be classified as INVALID_REQUEST (via ServiceError::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.
Copilot AI review requested due to automatic review settings August 3, 2026 02:00
@penso

penso commented Aug 3, 2026

Copy link
Copy Markdown
Collaborator Author

@greptile review

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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_params currently wrap client input/shape errors with ServiceError::message(...), which converts to moltis_protocol::error_codes::INTERNAL. These are client-side request issues and should be classified as INVALID_REQUEST (using the new ServiceError::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.
Copilot AI review requested due to automatic review settings August 3, 2026 03:29

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 96 out of 97 changed files in this pull request and generated 1 comment.

Comment thread crates/web/ui/src/pages/mcp/rpc.ts
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.
Copilot AI review requested due to automatic review settings August 3, 2026 05:50

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 97 out of 98 changed files in this pull request and generated no new comments.

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.
Copilot AI review requested due to automatic review settings August 3, 2026 06:24

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 (and parse_repository_id) currently return ServiceError::message(...), which maps to the INTERNAL error code. These are client-side input/contract errors and should be classified as InvalidRequest so 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::materialize is 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.

@penso

penso commented Aug 3, 2026

Copy link
Copy Markdown
Collaborator Author

@greptile review

This branch has not been deployed

No deployments
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.

3 participants