Fix environment variable inheritance for stdio MCP servers by Dumbris · Pull Request #46 · smart-mcp-proxy/mcpproxy-go

Dumbris · 2025-08-29T09:59:21Z

When mcpproxy is launched via Launchd (e.g., from Dock), it inherits a minimal environment that doesn't include user shell customizations from .bashrc, .zshrc, etc. This causes issues when stdio MCP servers try to use commands like 'uvx', 'docker', 'npx' that may not be in the basic PATH.

Changes:

Wrap stdio commands with user's login shell using -l flag
Add proper shell escaping for command arguments
Maintain existing secureenv filtering for security
Add detailed debug logging to show shell wrapping process

The fix works by running '/bin/zsh -l -c "original_command args"' instead of 'original_command args' directly, ensuring full user environment is inherited.

Fixes issues where tools installed via Homebrew, npm global, or other user-specific package managers are not found when mcpproxy is launched from GUI applications.

When mcpproxy is launched via Launchd (e.g., from Dock), it inherits a minimal environment that doesn't include user shell customizations from .bashrc, .zshrc, etc. This causes issues when stdio MCP servers try to use commands like 'uvx', 'docker', 'npx' that may not be in the basic PATH. Changes: - Wrap stdio commands with user's login shell using -l flag - Add proper shell escaping for command arguments - Maintain existing secureenv filtering for security - Add detailed debug logging to show shell wrapping process The fix works by running '/bin/zsh -l -c "original_command args"' instead of 'original_command args' directly, ensuring full user environment is inherited. Fixes issues where tools installed via Homebrew, npm global, or other user-specific package managers are not found when mcpproxy is launched from GUI applications.

…-proxy#46) When mcpproxy is launched via Launchd (e.g., from Dock), it inherits a minimal environment that doesn't include user shell customizations from .bashrc, .zshrc, etc. This causes issues when stdio MCP servers try to use commands like 'uvx', 'docker', 'npx' that may not be in the basic PATH. Changes: - Wrap stdio commands with user's login shell using -l flag - Add proper shell escaping for command arguments - Maintain existing secureenv filtering for security - Add detailed debug logging to show shell wrapping process The fix works by running '/bin/zsh -l -c "original_command args"' instead of 'original_command args' directly, ensuring full user environment is inherited. Fixes issues where tools installed via Homebrew, npm global, or other user-specific package managers are not found when mcpproxy is launched from GUI applications.

Adds a first-run onboarding wizard that adapts to the user's state. Two predicates drive which steps render — "any AI client connected to mcpproxy?" and "any upstream MCP server configured?". With both false the wizard shows two steps; with one false it shows just that one; with both true the dashboard loads directly. Skip on any step lands on the dashboard; once engaged, the wizard never auto-shows again. A "Run setup wizard" link in the dashboard re-runs it on demand. Backend reuses Spec 039's connect endpoint and the existing add-server flow; the wizard never duplicates either. Quarantine remains the trust boundary — the server step explains it plainly but does not change quarantine behaviour. ## Changes Backend (Go): - internal/storage/models.go + bbolt.go + manager.go: new OnboardingBucket and OnboardingState record (engaged, first-shown, per-step status); thread-safe Get/Save accessors. - internal/runtime/runtime.go + internal/server/server.go: pass-through wrappers to storage. - internal/connect/connect.go: GetConnectedCount() and GetConnectedIDs() helpers reusing the existing per-client adapter table. - internal/httpapi/onboarding.go (new): GET /api/v1/onboarding/state returns live predicates plus the persisted record plus a derived should_show_wizard flag; POST /api/v1/onboarding/mark updates per-step status, first-shown timestamp, and engagement. - internal/httpapi/server.go: ServerController interface gains GetOnboardingState / SaveOnboardingState; routes registered alongside /api/v1/connect. - Test mocks (security_test.go, contracts_test.go) updated. Frontend (Vue 3 + Pinia): - frontend/src/types/api.ts: OnboardingState, OnboardingStateResponse, OnboardingMarkRequest. - frontend/src/services/api.ts: getOnboardingState() and markOnboardingState(). - frontend/src/stores/onboarding.ts (new): Pinia store with derived visibleSteps driving adaptive rendering plus per-step mark helpers. - frontend/src/components/OnboardingWizard.vue (new): adaptive 1-2 step modal with progress indicator, per-step skip, and a quarantine explainer in the server step. Reuses the existing AddServerModal for server addition. - frontend/src/views/Dashboard.vue: auto-show on mount when should_show_wizard, plus a "Run setup wizard" link below "Connect Clients" for manual re-run. Spec: - specs/046-local-first-onboarding/spec.md plus checklist. OpenAPI: - oas/docs.go and oas/swagger.yaml regenerated with the new /onboarding endpoints. ## Testing - Storage / connect / httpapi unit tests pass. - Manual verification through Chrome browser automation across all four state combinations (none / clients-only / servers-only / both) with the dev backend on :18080. Auto-show, step advancement, skip, re-run, and engaged-suppression all confirmed. Funnel data (first-shown, engaged-at, per-step status) persisted in BBolt. ## What's deferred - US2 tier-2 client adapters (Antigravity, Zed, OpenCode, etc.) — purely additive to the existing adapter table; one-PR-per-adapter. - US3 heartbeat fields and Cloudflare D1 schema migration — backend signals are already exposed; just need to wire into HeartbeatPayload and migrate the D1 schema. Related #46

… guards Adds five anonymous, fixed-enum heartbeat fields per Spec 046 US3 so we can measure whether the new onboarding wizard moves day-2 retention from the current 11.8% baseline. Bumps the heartbeat schema_version to 4. Existing v3 consumers ignore the new fields; v1/v2/v3 clients continue to send the exact same payload they did before. The provider closes over connect.Service (for connected-client count + fixed- enum IDs) and the BBolt-backed OnboardingState (for wizard engagement + per-step status). Wired in internal/server/server.go alongside the existing connect-service wire-up so a single consultation returns fresh data each heartbeat. ## Changes - internal/telemetry/telemetry.go: - SchemaVersion bumped 3 -> 4 with a doc comment summarising the v4 delta - HeartbeatPayload gains connected_client_count, connected_client_ids, wizard_engaged, wizard_connect_step, wizard_server_step (all omitempty) - New OnboardingSnapshot struct + SetOnboardingProvider setter, populated in buildHeartbeat with the standard nil-safe pattern - internal/server/server.go: wires the onboarding provider on the telemetry service inside the existing "if cfg := s.runtime.Config()" block so the closure captures the just-built connect.Service - internal/telemetry/payload_privacy_test.go: extends the canary test to v4 (schema_version 4) and adds two new tests — TestPayloadV4_OnboardingFieldsArePresent and TestPayloadV4_OnboardingDoesNotLeakUserStrings — that assert the wire shape carries no paths/URLs/free-text, only fixed-enum adapter IDs - internal/telemetry/payload_v2_test.go + telemetry_test.go: tripwire schema_version assertions updated 3 -> 4 ## Privacy posture Inherits Spec 042 / Spec 044: opt-out, anonymous, off-by-default in CI/test, no transmission when telemetry is disabled. The new fields cannot carry user-entered values: - connected_client_count is a non-negative integer - connected_client_ids comes strictly from connect.Service.GetConnectedIDs() which iterates the fixed per-client adapter table — paths, names, and arbitrary strings cannot reach this field - wizard_engaged is a boolean - wizard_connect_step and wizard_server_step are tri-state strings (""/completed/skipped) sourced from the OnboardingState bucket The Cloudflare Worker (companion change in mcpproxy-telemetry) re-validates all five fields shape-side before INSERT, so a regression on either side gets caught. ## Tests - internal/telemetry: full suite passes (privacy, schema, payload v2, v4) - internal/connect, internal/storage, internal/httpapi: unchanged + still pass - Build: ok ./... - Manual end-to-end against /api/v1/telemetry/payload — schema_version=4 with all five fields populated correctly from BBolt + connect.Service Companion changes: - mcpproxy-telemetry: D1 migration 0005_add_onboarding_funnel.sql, validateV4Payload, INSERT/UPDATE bindings, 29 new validation tests (74 -> 103 passing) - mcpproxy.app-website: /telemetry page documents the five new fields with the same level of detail as existing fields Related #46

…ame, activity panel Related #46 Reframes the first-run wizard as a persistent, idempotent setup surface driven by the activation funnel telemetry (SynapBus #27345, 78.2% → 11.7% cliff at IDE connect). Replaces the linear v1 flow with three idempotent tabs (Clients · Servers · Verify), a top-pinned sidebar entry with an animated badge, and a passive Verify tab that flips green automatically on the first successful MCP `initialize` round-trip — closing the loop on the cliff in-product. ## Changes Spec - specs/046-local-first-onboarding/spec.md: appended a "v2 — Wizard Surface Redesign" section. v1 text preserved verbatim. Backend (Go) - internal/httpapi/onboarding.go: extended /api/v1/onboarding/state with first_mcp_client_ever, mcp_clients_seen_ever, incomplete_tab_count. ShouldShowWizard semantics widened to cover Verify. - internal/httpapi/server.go + internal/server/server.go + internal/runtime/runtime.go: new GetActivationFirstMCPClient() reads Spec 044's existing FirstMCPClientEver / MCPClientsSeenEver from the ActivationStore. No new BBolt buckets, no new MCP hook. - internal/httpapi/import.go: ImportFromPathRequest gains skip_quarantine query param ("Import as active") and a rename map applied after parsing — so the wizard can disambiguate cross-source name collisions like "mcpproxy" appearing in both Claude Code and Claude Desktop. Frontend (Vue 3 + Pinia) - frontend/src/components/OnboardingWizard.vue: full rewrite as a 3-tab modal at min(960px, 90vw) × min(640px, 85vh). Tabs are bidirectionally clickable; per-tab idempotent state. - Clients tab: detected sort -> always-pinned trio (Claude Code / Codex / Gemini) -> "Show N more" collapse. Inline security expander. - Servers tab: sectioned checkbox list with indented server rows under each section header; per-section "select all" with proper indeterminate state; cross-source name collisions render an inline rename pill ("→ <renamed>"); sticky non-scrollable footer with selection count, conflict count, and two equal-width action buttons (Import & quarantine, Import as active). Compact toggles for Docker isolation and quarantine_enabled with inline Docker- availability warning. Educational note about post-hoc scanning. - Verify tab: passive, flips green on FirstMCPClientEver. Three sample prompts annotated with the built-in tool each one exercises. Recent Activity panel below shows the last 5 records with status badges and "View all in Activity Log" link. - frontend/src/components/SidebarNav.vue: top-pinned "Setup" entry above Dashboard for personal edition. - frontend/src/views/Dashboard.vue: removed the v1 "Run setup wizard" button. - frontend/src/stores/onboarding.ts + types/api.ts + services/api.ts: store gains firstMCPClientEver, mcpClientsSeenEver, incompleteTabCount computeds. importServersFromPath supports skip_quarantine + rename. OpenAPI - oas/swagger.yaml + oas/docs.go: ImportFromPathRequest gains rename. Verification - specs/046-local-first-onboarding/verification/: 7 Playwright-captured PNGs and a self-contained report.html (1.4 MB, base64-embedded screenshots). ## Testing - internal/httpapi unit + contract tests pass; mocks updated. - frontend vue-tsc clean. - Playwright sweep on a fresh data dir: all scenarios pass; out-of-band curl confirms rename produces mcpproxy_claude_code / mcpproxy_claude_desktop with quarantined=true.

Adds local browser-driven Playwright coverage for: - engagement persistence (no auto-popup, sidebar reopens) - select-all toggle off (footer state) - per-section indeterminate state on partial selection - "View all in Activity Log →" link navigation - Docker isolation + quarantine toggle visual feedback - skip-import flow (no servers added on Close) Renders updated report.html with 14 scenarios + a "Local browser test run" section noting the Docker/quarantine toggle persistence finding: the wizard wraps the apply payload in `{config: …}` but the backend expects the bare Config — UI flips locally but server-side persistence falls back to best-effort. Dedicated PATCH endpoint works. Related #46

Related #46 The wizard's `patchConfig` was wrapping the body in `{config: merged}`, but `/api/v1/config/apply` decodes the body directly into `config.Config`. Result: every toggle (require_mcp_auth, docker_isolation, quarantine_enabled) flipped the UI state but the server-side apply rejected the wrapped payload with a stale field-validation error ("tools_limit: must be between 1 and 1000"). Surfaced by the v2 verification subagent (scenarios 12, 13). Settings.vue calls `api.applyConfig(config)` with a bare config object — the wizard now matches that contract. Also surfaces the apply response's `success` flag so a server-side rejection produces a visible "Save failed" toast instead of silently swallowing the error. Verified end-to-end: setting quarantine_enabled=false + require_mcp_auth=true via the wizard now persists to disk and a fresh `GET /api/v1/config` reflects the new values.

…tion docs Related #46 Reworks the Servers tab security toggles per the latest UX feedback: - Renamed "Quarantine new tools" → "Quarantine new servers". - Pulled both toggles out of the scrollable list into a sticky non- scrollable footer panel framed as global settings, not per-import options. - Wrapped them in a collapsed-by-default <details> labeled "Runtime isolation and MCP server quarantine" with a "global settings — apply to every imported server" summary hint. - Each toggle is now a card with long-form security explanation, links to docs.mcpproxy.app/security/{docker-isolation,quarantine}, and (for Docker) a green "Docker detected" or warning "Docker not detected" badge with an inline Docker Desktop install hint. - Quarantine description leads with bold "Recommended." and "Important:" clauses calling out that the AI agent itself can register upstream servers via mcpproxy's built-in MCP tools. - Docker toggle disables when Docker isn't detected. - "Import & quarantine" disables when Quarantine new servers is off (with a hover hint). "Import as active" stays enabled — opting out is an explicit user choice. Companion docs: - specs/046-local-first-onboarding/verification/srv-*.png + appended "Servers tab — toggles overhaul" section in report.html (6 scenarios). - CLAUDE.md gains a "Verifying Web UI changes" section describing the Playwright + base64-embedded HTML report workflow plus gotchas.

Related #46 Per UX feedback: the "global settings — apply to every imported server" hint on the Servers-tab security panel summary was muted opacity-50 text and the wording suggested the toggles only affected the current import. Both toggles are actually backed by the persistent mcpproxy config — flipping them writes through to mcp_config.json and stays in effect forever. - Promote "Global settings" to a bold primary-color badge (badge-primary badge-sm font-semibold) so it reads at a glance. - Replace the right-side caption with "saved to your mcpproxy config" so the persistence story is explicit, not implied. - Caption hides on narrow viewports (badge alone is enough).

Related #46 The "Docker detected" / "Docker not detected" pills next to the Docker isolation title were redundant — the install hint already appears below the description when the daemon is missing, and the toggle's disabled state is the real signal. Same for the "Recommended" badge next to "Quarantine new servers" — the description already opens with bold "Recommended." which carries the weight. Removing both leaves the panel uncluttered: just the title row, description with bold-emphasised security context, and the docs link. The prominent "Global settings — saved to your mcpproxy config" header still sits top-right of the panel summary so users know they're editing persistent state. Verification spec updated to no longer assert the badge count.

…d import, passive Verify (#433) Reframes the first-run wizard as a persistent, idempotent setup surface driven by the activation funnel telemetry (78.2% → 11.7% cliff at IDE connect). Replaces the linear v1 flow with three idempotent tabs (Clients · Servers · Verify), a top-pinned sidebar entry with an animated badge, and a passive Verify tab that flips green automatically on the first successful MCP initialize round-trip. Highlights: - Backend: /api/v1/onboarding/state extended with first_mcp_client_ever, mcp_clients_seen_ever, incomplete_tab_count. Reuses Spec 044's ActivationStore — no new BBolt buckets. - Backend: ImportFromPathRequest gains skip_quarantine + rename map for the wizard's bulk-import path with cross-source name disambiguation. - Frontend: 3-tab modal (Clients · Servers · Verify); Servers tab is a sectioned checkbox list with indented rows, sticky non-scrollable footer panel exposing global Docker isolation + Quarantine toggles with deep links to docs.mcpproxy.app/security/. Conflict pills rename "mcpproxy" from multiple sources to mcpproxy_<format>. - Frontend: top-pinned sidebar Setup entry with badge that mirrors incomplete_tab_count; Dashboard's old Run-setup-wizard button removed. - Verify tab surfaces recent activity records inline + sample prompts exercising built-in mcpproxy tools (retrieve_tools, upstream_servers, quarantine_security). Verification: specs/046-local-first-onboarding/verification/report.html documents 20 Playwright scenarios across the v2 redesign. Companion docs: - docs.mcpproxy.app/security/{docker-isolation,quarantine} — separate PR on the website repo, already merged. - CLAUDE.md gains "Verifying Web UI changes" methodology section. - SynapBus wiki: webui-verification-playwright-rich-report. Related #46

… guards Adds five anonymous, fixed-enum heartbeat fields per Spec 046 US3 so we can measure whether the new onboarding wizard moves day-2 retention from the current 11.8% baseline. Bumps the heartbeat schema_version to 4. Existing v3 consumers ignore the new fields; v1/v2/v3 clients continue to send the exact same payload they did before. The provider closes over connect.Service (for connected-client count + fixed- enum IDs) and the BBolt-backed OnboardingState (for wizard engagement + per-step status). Wired in internal/server/server.go alongside the existing connect-service wire-up so a single consultation returns fresh data each heartbeat. ## Changes - internal/telemetry/telemetry.go: - SchemaVersion bumped 3 -> 4 with a doc comment summarising the v4 delta - HeartbeatPayload gains connected_client_count, connected_client_ids, wizard_engaged, wizard_connect_step, wizard_server_step (all omitempty) - New OnboardingSnapshot struct + SetOnboardingProvider setter, populated in buildHeartbeat with the standard nil-safe pattern - internal/server/server.go: wires the onboarding provider on the telemetry service inside the existing "if cfg := s.runtime.Config()" block so the closure captures the just-built connect.Service - internal/telemetry/payload_privacy_test.go: extends the canary test to v4 (schema_version 4) and adds two new tests — TestPayloadV4_OnboardingFieldsArePresent and TestPayloadV4_OnboardingDoesNotLeakUserStrings — that assert the wire shape carries no paths/URLs/free-text, only fixed-enum adapter IDs - internal/telemetry/payload_v2_test.go + telemetry_test.go: tripwire schema_version assertions updated 3 -> 4 ## Privacy posture Inherits Spec 042 / Spec 044: opt-out, anonymous, off-by-default in CI/test, no transmission when telemetry is disabled. The new fields cannot carry user-entered values: - connected_client_count is a non-negative integer - connected_client_ids comes strictly from connect.Service.GetConnectedIDs() which iterates the fixed per-client adapter table — paths, names, and arbitrary strings cannot reach this field - wizard_engaged is a boolean - wizard_connect_step and wizard_server_step are tri-state strings (""/completed/skipped) sourced from the OnboardingState bucket The Cloudflare Worker (companion change in mcpproxy-telemetry) re-validates all five fields shape-side before INSERT, so a regression on either side gets caught. ## Tests - internal/telemetry: full suite passes (privacy, schema, payload v2, v4) - internal/connect, internal/storage, internal/httpapi: unchanged + still pass - Build: ok ./... - Manual end-to-end against /api/v1/telemetry/payload — schema_version=4 with all five fields populated correctly from BBolt + connect.Service Companion changes: - mcpproxy-telemetry: D1 migration 0005_add_onboarding_funnel.sql, validateV4Payload, INSERT/UPDATE bindings, 29 new validation tests (74 -> 103 passing) - mcpproxy.app-website: /telemetry page documents the five new fields with the same level of detail as existing fields Related #46

… guards (#434) Adds five anonymous, fixed-enum heartbeat fields per Spec 046 US3 so we can measure whether the wizard moves day-2 retention from the 11.8% pre-feature baseline. Bumps schema_version 3 → 4. Existing v3 consumers ignore the new fields; v1/v2/v3 clients keep sending the same payload they did before. Fields added to HeartbeatPayload: - connected_client_count (int) - connected_client_ids ([]string, fixed enum from adapter table) - wizard_engaged (bool) - wizard_connect_step ("" / "completed" / "skipped") - wizard_server_step ("" / "completed" / "skipped") Privacy posture inherits Spec 042 / Spec 044: opt-out, anonymous, off-by- default in CI/test, no transmission when telemetry is disabled. Two new privacy regression tests in payload_privacy_test.go assert the wire shape and that the new fields cannot carry user-entered values. Wired in internal/server/server.go alongside the existing connect-service wire-up so a single consultation returns fresh data each heartbeat. Companion changes (separate repos): - mcpproxy-telemetry: D1 migration + worker validation - mcpproxy.app-website: /telemetry page documentation Related #46

Dumbris merged commit 6a21277 into main Aug 29, 2025
33 checks passed

Dumbris deleted the feature/shell-wrapper branch August 29, 2025 10:18

This was referenced Apr 28, 2026

feat(046): adaptive onboarding wizard with state-driven steps #433

Merged

feat(046-us3): onboarding-funnel telemetry — payload fields + privacy guards #434

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix environment variable inheritance for stdio MCP servers#46

Fix environment variable inheritance for stdio MCP servers#46
Dumbris merged 1 commit into
mainfrom
feature/shell-wrapper

Dumbris commented Aug 29, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

Dumbris commented Aug 29, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant