fix(docker): observable spawn decision + accurate transport error label (#696)#708
Merged
Merged
Conversation
…el (#696) Related #696 The v0.40.0 docker-isolation spawn already direct-execs the resolved bundle path on macOS, but two diagnosability defects made the field failure look like a bare-docker spawn: - transport error reported c.config.Command (always "docker") instead of the resolved finalCommand actually exec'd, so a successful direct-exec of an absolute bundle path looked identical to a bare-docker spawn. - no INFO-level spawn-decision log to tell direct-exec from shell-wrap. ## Changes - connection_stdio.go: transport error reports finalCommand (real argv[0]) - connection_docker.go: INFO logs for direct-exec vs login-shell-wrap fallback - shellwrap/testhooks.go: cross-package test seam (well-known paths + cache reset) - connection_docker_integration_test.go: real-resolver #696 repro (bundle/path/ unresolvable legs) + status/spawn no-divergence guard - connection_docker_exec_e2e_test.go: recording-shim proves OS execs abs path ## Testing - go test -race ./internal/upstream/core/ ./internal/shellwrap/ green - golangci-lint v2 clean
Deploying mcpproxy-docs with
|
| Latest commit: |
4295047
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://4fd8f6ee.mcpproxy-docs.pages.dev |
| Branch Preview URL: | https://fix-696-docker-spawn-diagnos.mcpproxy-docs.pages.dev |
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
📦 Build ArtifactsWorkflow Run: View Run Available Artifacts
How to DownloadOption 1: GitHub Web UI (easiest)
Option 2: GitHub CLI gh run download 27733130012 --repo smart-mcp-proxy/mcpproxy-go
|
![mcpproxy-gatekeeper[bot]](https://avatars.githubusercontent.com/u/216428285?s=60&v=4){kind=small}
There was a problem hiding this comment.
Approved: CodexReviewer accept verdict (Paperclip review of record); CI green. #696 docker spawn diagnosability.
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.
Summary
Closes the diagnosability gap behind #696. Investigation proved the v0.40.0 docker-isolation spawn already direct-execs the resolved Docker Desktop bundle path on macOS — #696 is not a reproducible source bug. The field report looked like a bare-
dockerspawn because of two diagnosability defects, now fixed:connection_stdio.go— the transport error reportedc.config.Command(always"docker"for an isolated server) instead of the resolvedfinalCommandactually exec'd. A successful direct-exec of an absolute bundle path was therefore indistinguishable in the error from a bare-dockerspawn — actively misdirecting diagnosis. Now reportsfinalCommand(the realargv[0]: absolute path on direct-exec, login shell on the shell-wrap fallback).connection_docker.go— no INFO-level spawn-decision log. Added INFO lines for both the direct-exec path and the login-shell-wrap fallback so a report like Docker Desktop not detected on macOS: servers fail withcommand not found: dockerdespitedocker_available: true#696 ("docker installed but not on PATH") can be triaged from onemain.logline: which docker binary was chosen and whether it was exec'd directly vs shell-wrapped.Test evidence
internal/shellwrap/testhooks.go— cross-package test seam (well-known paths + cache reset).connection_docker_integration_test.go— real-resolver Docker Desktop not detected on macOS: servers fail withcommand not found: dockerdespitedocker_available: true#696 repro (bundle / path / unresolvable legs) + a status/spawn no-divergence guard.connection_docker_exec_e2e_test.go— recording-shim proving the OS execs the absolute path.Local verification
go build ./cmd/mcpproxy— OKgo test -race ./internal/upstream/core/ ./internal/shellwrap/— greengolangci-lint v2.5.0 --config .github/.golangci.ymlon changed packages — 0 issuesNo user-facing surface changes (internal log lines + an error-label correction), so no docs diff required.
Related #696