Skip to content

feat(config): isolation.mode enum + mode-aware selector (MCP-34.2)#759

Merged
Dumbris merged 3 commits into
mainfrom
feat/mcp-3233-isolation-mode
Jun 25, 2026
Merged

feat(config): isolation.mode enum + mode-aware selector (MCP-34.2)#759
Dumbris merged 3 commits into
mainfrom
feat/mcp-3233-isolation-mode

Conversation

@Dumbris

@Dumbris Dumbris commented Jun 24, 2026

Copy link
Copy Markdown
Member

Summary

Adds a three-way isolation mode (docker | sandbox | none) to global and per-server config, the config foundation for native (non-Docker) sandbox isolation (MCP-34). Implements MCP-34.2.

Changes

  • config.IsolationMode type + constants (docker/sandbox/none) and IsValid().
  • Global DockerIsolationConfig.Mode + ResolvedMode() back-compat mapping: explicit Mode wins; else legacy enabled:true ⇒ docker, enabled:false ⇒ none.
  • Per-server IsolationConfig.Mode (*IsolationMode, nil = inherit global).
  • IsolationManager.ResolveMode() mode resolver. ShouldIsolate() reimplemented as ResolveMode() == docker, so existing Docker behavior is byte-for-byte unchanged. Precedence: per-server explicit Mode > global gate > per-server bool opt-out; structural gates (HTTP / docker-command servers) apply to all modes.
  • Validation rejects unknown global/per-server mode strings.
  • Per-server Mode round-trips through BBolt (UpstreamRecord); regenerated oas/swagger.yaml + docs.go.

Tests (red→green, TDD)

  • internal/config/isolation_mode_test.goResolvedMode() back-compat, IsValid(), validation.
  • internal/upstream/core/isolation_mode_test.goResolveMode() matrix (global/per-server/gates) + ShouldIsolate consistency.
  • internal/storage/async_ops_test.go — per-server Mode BBolt round-trip assertion.

Verification: go test -race on config/core/appctx green; v2 golangci-lint clean; make swagger committed.

Scope / handoff

Sandbox-mode launch wiring (consuming ResolveMode()==sandbox in the connection launchers) is deferred to MCP-34.3; no consumer acts on the sandbox value yet, so this PR cannot change runtime behavior for existing docker/none setups.

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Jun 24, 2026

Copy link
Copy Markdown

Deploying mcpproxy-docs with  Cloudflare Pages  Cloudflare Pages

Latest commit: b2cb1ac
Status: ✅  Deploy successful!
Preview URL: https://5766dd2f.mcpproxy-docs.pages.dev
Branch Preview URL: https://feat-mcp-3233-isolation-mode.mcpproxy-docs.pages.dev

View logs

@codecov-commenter

codecov-commenter commented Jun 24, 2026

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@github-actions

github-actions Bot commented Jun 24, 2026

Copy link
Copy Markdown

📦 Build Artifacts

Workflow Run: View Run
Branch: feat/mcp-3233-isolation-mode

Available Artifacts

  • archive-darwin-amd64 (28 MB)
  • archive-darwin-arm64 (25 MB)
  • archive-linux-amd64 (16 MB)
  • archive-linux-arm64 (14 MB)
  • archive-windows-amd64 (28 MB)
  • archive-windows-arm64 (25 MB)
  • frontend-dist-pr (0 MB)
  • installer-dmg-darwin-amd64 (21 MB)
  • installer-dmg-darwin-arm64 (19 MB)

How to Download

Option 1: GitHub Web UI (easiest)

  1. Go to the workflow run page linked above
  2. Scroll to the bottom "Artifacts" section
  3. Click on the artifact you want to download

Option 2: GitHub CLI

gh run download 28144173698 --repo smart-mcp-proxy/mcpproxy-go

Note: Artifacts expire in 14 days.

Introduce a three-way isolation mode (docker | sandbox | none) on both the
global DockerIsolationConfig and per-server IsolationConfig, laying the config
foundation for native (non-Docker) sandbox isolation (MCP-34).

- New `config.IsolationMode` type + constants and `IsValid()` helper.
- Global `DockerIsolationConfig.Mode` with `ResolvedMode()` back-compat mapping:
  explicit Mode wins; else legacy enabled:true ⇒ docker, enabled:false ⇒ none.
- Per-server `IsolationConfig.Mode` (*IsolationMode, nil = inherit global).
- `IsolationManager.ResolveMode()` mode resolver; `ShouldIsolate()` reimplemented
  as `ResolveMode() == docker` so existing Docker behavior is unchanged.
  Precedence: per-server explicit Mode > global gate > per-server bool opt-out;
  structural gates (HTTP / docker-command servers) apply to all modes.
- Config validation rejects unknown global/per-server mode strings.
- Per-server Mode round-trips through BBolt (UpstreamRecord) — test asserts it.
- Regenerated oas/swagger.yaml + docs.go for the new fields.

Sandbox-mode launch wiring (consuming ResolveMode==sandbox) is deferred to
MCP-34.3; no consumer acts on the sandbox value yet.

Co-Authored-By: Paperclip <[email protected]>
@Dumbris Dumbris force-pushed the feat/mcp-3233-isolation-mode branch from 56c2d93 to 3b0e63e Compare June 24, 2026 07:00
@Dumbris

Dumbris commented Jun 24, 2026

Copy link
Copy Markdown
Member Author

Rebased onto origin/main and dropped the out-of-scope MCP-3246 tray-marketplace commit (56f63cf3) that had leaked in via a wrong fork base. New head 3b0e63ec contains only the MCP-34.2 isolation-config change (7 files: config, upstream/core isolation + tests, storage test, swagger). That tray work continues on its own branch feat/mcp-3246-go-tray-install-server.

Local verification on the clean branch:

  • go build ./... — OK
  • go test -race ./internal/config/... ./internal/upstream/core/... ./internal/storage/... — all PASS
  • golangci-lint run --config .github/.golangci.yml (v2) on changed pkgs — 0 issues
  • make swagger — no drift (in sync)

Related #759

The shared config.IsolationMode enum schema (and the inlined global
DockerIsolationConfig.mode field) inherited the per-server field's
"Per-server isolation mode override / nil = inherit global" wording
via swaggo, which is inaccurate for the global field: the global Mode
is a non-pointer whose empty value falls back to the legacy Enabled
bool, not "inherit global".

Rewrite the per-server Mode field comment (the swaggo source for the
shared schema) to a neutral description accurate for both contexts:
unset per-server inherits the global mode; unset globally falls back
to the legacy enabled flag. Regenerate OpenAPI.

Related #MCP-3233
@Dumbris

Dumbris commented Jun 24, 2026

Copy link
Copy Markdown
Member Author

Addressed Codex's doc-accuracy blocker (MCP-3285). The shared config.IsolationMode swaggo schema (and the inlined global mode field) inherited per-server-only wording ("Per-server isolation mode override / nil = inherit global"), inaccurate for the global field whose empty value falls back to the legacy enabled bool. Commit 3bd787a5 rewrites the controlling field comment to a neutral description accurate for both contexts and regenerates OpenAPI; both swagger locations now read correctly. No behavior change. Local: go build, go test -race ./internal/config/..., v2 golangci-lint (0 issues), make swagger (no further drift) all green.

@Dumbris

Dumbris commented Jun 24, 2026

Copy link
Copy Markdown
Member Author

ACCEPT (CEO / fallback reviewer — MCP-3300)

Code-review verdict: ACCEPT. GeminiCritic adapter was unavailable; CEO performed model-diverse fallback review (spec 064 S-1 provenance).

What I verified

  • IsolationMode enum + IsValid() — correct
  • DockerIsolationConfig.ResolvedMode() back-compat mapping (explicit Mode > legacy Enabled bool) — correct
  • Per-server *IsolationMode nil-inherit + resolveConfiguredMode precedence chain — correct
  • ShouldIsolate reimplemented as ResolveMode()==docker (byte-for-byte behavior preserved) — correct
  • Validation rejects unknown mode strings at global + per-server level — correct
  • BBolt round-trip for per-server Mode — confirmed by test
  • go test -race ./internal/config/... ./internal/upstream/core/... ./internal/storage/... — all green

Scope check: sandbox launch wiring correctly deferred to MCP-34.3; no consumer acts on IsolationModeSandbox yet — existing docker/none behavior unchanged.

No blocking findings. PR is clear to merge (pending QA gate).

@mcpproxy-gatekeeper mcpproxy-gatekeeper Bot 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.

Gatekeeper approval — review verdict: ACCEPT (by ClaudeReviewer+KimiReviewer, model-diverse fallback).

This approval is posted automatically by the MCPProxy Gatekeeper App on behalf of ClaudeReviewer+KimiReviewer — the model-diverse reviewer-fallback reviewer of record (verdict lives in the Paperclip review thread). Author≠approver satisfied; QA + CI gates enforced separately.

Auto-approved per Model B (MCP-1249) + reviewer-fallback (MCP-3066).

@Dumbris Dumbris merged commit 5ed2c30 into main Jun 25, 2026
34 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants