fix(docker): resolve docker by absolute path on macOS spawn; honest docker_available (#696)#697
Merged
Merged
Conversation
…ocker_available (#696) On macOS, Docker Desktop installed the default way (without the optional, admin-gated "install CLI tools" step) leaves the docker CLI only inside the app bundle at /Applications/Docker.app/Contents/Resources/bin/docker, which is not on any standard PATH dir. mcpproxy spawned isolated servers with bare `docker` on a sanitized PATH that omits the bundle dir, so docker was unresolvable at spawn (~20x `command not found: docker`), while `upstream_servers list` misreported `docker_available: true`. - Spawn (root cause): setupDockerIsolation resolves docker to an absolute path via shellwrap.ResolveDockerPath (mirroring newDockerCmd) before shell-wrapping; falls back to bare "docker" only when resolution fails. - Enhanced spawn PATH (defense in depth): add the Docker Desktop bundle bin dir on darwin; split the candidate list into a pure, unit-testable function. - Honest availability: MCPProxyServer.resolveDockerStatus and Runtime.IsDockerAvailable report available only when the CLI is resolvable AND `docker info` succeeds — no bare-docker fallback that would misreport. Surface the resolved binary as docker_status.docker_path; diagnostics.checkDockerDaemon reports unavailable with an actionable error when unresolvable. - Diagnostics: formatRecentStderr leads a single actionable "Docker CLI not found" hint on `command not found` and collapses identical consecutive lines into "… (repeated N×)", replacing the 20-line wall. macOS-specific paths guarded behind runtime.GOOS=="darwin"; Linux/Windows unaffected. Docs updated for docker_path, telemetry counter semantics, and a new troubleshooting entry. Related #696
Deploying mcpproxy-docs with
|
| Latest commit: |
6fa8ffa
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://ba4c1d5a.mcpproxy-docs.pages.dev |
| Branch Preview URL: | https://fix-mcp-2715-docker-spawn-pa.mcpproxy-docs.pages.dev |
|
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
…n test
insertCidfileIntoShellDockerCommand checked shellArgs[n-2]=="-c" to
distinguish a shell-wrapped command string from an unrecognised format.
On Windows with cmd.exe (no bash), WrapWithUserShell returns ["/c", cmdStr]
so the flag is "/c", not "-c"; the guard rejected it and silently skipped
cidfile insertion, breaking TestSetupDockerIsolationCidfileInsertionWithAbsolutePath
on windows-latest CI.
Accept both flags ("-c" and "/c") so cidfile insertion works on every
platform. Add TestInsertCidfileWindowsCmdFormat (pure unit, no real Docker)
to pin the Windows cmd format.
📦 Build ArtifactsWorkflow Run: View Run Available Artifacts
How to DownloadOption 1: GitHub Web UI (easiest)
Option 2: GitHub CLI gh run download 27625094138 --repo smart-mcp-proxy/mcpproxy-go
|
![mcpproxy-gatekeeper[bot]](https://avatars.githubusercontent.com/u/216428285?s=60&v=4){kind=small}
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
2 participants
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.
Problem (GitHub #696)
On macOS with Docker Desktop installed the default way (without the optional, admin-gated "install CLI tools" step), the
dockerCLI lives only at/Applications/Docker.app/Contents/Resources/bin/docker— not on any standardPATHdir. mcpproxy spawned Docker-isolated upstream servers with baredockeron a sanitized PATH that omits the bundle dir, sodockerwas unresolvable at spawn:…while
upstream_servers listmisleadingly reporteddocker_status.available: true/ per-serverdocker_available: true.Fixes (TDD — failing test first for each)
internal/upstream/core/connection_docker.go:setupDockerIsolationresolvesdockerviashellwrap.ResolveDockerPath(mirroringnewDockerCmd) and spawns the absolute path before shell-wrapping; falls back to baredockeronly when resolution fails. Indirected via a package var so tests stub resolution with no real Docker install.internal/secureenv/manager.go: add/Applications/Docker.app/Contents/Resources/binon darwin (existence-filtered). Candidate list split into a pure, unit-testableunixCandidatePaths().docker_available—internal/server/mcp.go(resolveDockerStatus) +internal/runtime/runtime.go(IsDockerAvailable) +internal/management/diagnostics.go: report available only when the CLI is resolvable anddocker infosucceeds — no bare-dockerfallback that misreports. Newdocker_status.docker_pathexposes the resolved binary;available:falsewithdocker_path:""means unresolvable.internal/upstream/core/monitoring.go:formatRecentStderrleads a single actionable "Docker CLI not found" hint oncommand not found(zsh + bash forms) and collapses identical consecutive lines into… (repeated N×), replacing the 20-line wall.Cross-platform
macOS-specific paths guarded behind
runtime.GOOS=="darwin"; Linux/Windows unaffected. Shell-script fake-docker tests skip on Windows.Docs (ENG-9)
docs/docker-isolation.md: newdocker_pathfield, corrected telemetry-counter semantics, and acommand not found: dockertroubleshooting entry.Verification
go build ./cmd/mcpproxy(personal) andgo build -tags server ./cmd/mcpproxy— both pass.internal/secureenv(bundle-path inclusion),internal/upstream/core(absolute-path spawn, cidfile insertion, bare fallback, hint extraction, dedup, wall→hint),internal/server(unavailable/working/daemon-down).golangci-lint(CI v2 config) — 0 issues.-raceover touched packages running in CI.Out of scope
MaxConnectionRetriesunchanged.Related #696