Skip to content

Commit ec628cc

Browse files
electrolobzikclaude
electrolobzik
and
claude
committed
docs(spec): cover local-launcher HTTP/SSE config + restart semantics (Phase 2)
- docs/configuration.md: new "Locally-launched HTTP / SSE servers" section walks through the {command, url, protocol: http|sse| streamable-http} combo, a config example with launcher_wait_timeout, per-server log routing, and a back-compat behaviour matrix that makes the "command + url under auto -> stdio wins" footgun explicit. - docs/cli-management-commands.md: restart semantics note covering the launcher stop-then-start order and the 5s SIGKILL grace timeout. - specs/046-local-launcher-for-http-sse/execution_log.md: design decisions, deviations from the original plan (stdio refactor scoped down to shared command-prep helpers), bugs found and fixed during verification (connect deadlock, bytes.Buffer race, banner false- positive in SIGKILL test, ftp-with-port acceptance), and the exact verification commands. Refs spec: specs/046-local-launcher-for-http-sse/plan.md Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 3bf6160 commit ec628cc

3 files changed

Lines changed: 222 additions & 2 deletions

File tree

‎docs/cli-management-commands.md‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -352,6 +352,14 @@ mcpproxy upstream restart --all
352352

353353
**Note:** Restart does not require confirmation as it's non-destructive.
354354

355+
**Locally-launched HTTP/SSE upstreams:** when a server is configured with both
356+
`command` and an HTTP/SSE `url` (see [docs/configuration.md](configuration.md#locally-launched-http--sse-servers)),
357+
`restart` stops the spawned child (`SIGTERM` → grace → `SIGKILL`) before
358+
re-running Connect. The grace timeout is fixed at 5s today; the next start
359+
won't begin until the previous child is fully reaped, so you can rely on the
360+
port being free after the command returns. Stop ordering is: close MCP client
361+
→ stop launched child → release per-server state.
362+
355363
---
356364

357365
### `mcpproxy doctor`

‎docs/configuration.md‎

Lines changed: 51 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -147,8 +147,9 @@ MCPProxy looks for configuration in these locations (in order):
147147
| `args` | array | No | Command arguments |
148148
| `url` | string | Yes* | Server URL (required for `http`/`sse`/`streamable-http` protocols) |
149149
| `headers` | object | No | HTTP headers for HTTP-based protocols |
150-
| `working_dir` | string | No | Working directory for stdio servers (default: current directory) |
151-
| `env` | object | No | Environment variables for stdio servers |
150+
| `working_dir` | string | No | Working directory for stdio servers, or for the locally-launched child of an HTTP/SSE server (default: current directory) |
151+
| `env` | object | No | Environment variables for stdio servers, or for the locally-launched child of an HTTP/SSE server |
152+
| `launcher_wait_timeout` | duration | No | When `command` is set together with an HTTP/SSE `url`, how long mcpproxy waits for that URL to become reachable after spawning the child (e.g. `"15s"`, default `"30s"`) |
152153
| `oauth` | object | No | OAuth configuration (see [OAuth Configuration](#oauth-configuration)) |
153154
| `isolation` | object | No | Per-server Docker isolation settings (see [Docker Isolation](#docker-isolation)) |
154155
| `enabled` | boolean | No | Enable/disable server (default: `true`) |
@@ -210,6 +211,54 @@ MCPProxy looks for configuration in these locations (in order):
210211
}
211212
```
212213

214+
### Locally-launched HTTP / SSE servers
215+
216+
By default `command` is only used for `stdio` servers. When you set `command`
217+
together with an HTTP/SSE `url` and an explicit `protocol` of `http`, `sse`,
218+
or `streamable-http`, mcpproxy will:
219+
220+
1. Spawn the command (with `args`, `env`, `working_dir`, and Docker isolation
221+
exactly like a stdio server).
222+
2. Wait up to `launcher_wait_timeout` (default 30s) for `url` to accept a TCP
223+
connection.
224+
3. Connect via the configured HTTP/SSE transport.
225+
4. Own the child's lifecycle — the process is stopped (`SIGTERM`, then
226+
`SIGKILL` after a grace period) on disconnect, restart, server-disable, or
227+
mcpproxy shutdown. Unexpected exits trigger an automatic disconnect, which
228+
the existing reconnect path picks up.
229+
230+
```json
231+
{
232+
"name": "local-http-mcp",
233+
"protocol": "http",
234+
"url": "http://127.0.0.1:9999/mcp",
235+
"command": "node",
236+
"args": ["./examples/echo-http-server.js", "--port", "9999"],
237+
"working_dir": "/path/to/repo",
238+
"launcher_wait_timeout": "15s",
239+
"enabled": true
240+
}
241+
```
242+
243+
`stdout` and `stderr` of the child are routed to the per-server log, so
244+
`mcpproxy upstream logs <name>` continues to work the same way it does for
245+
stdio servers.
246+
247+
#### Behaviour matrix when both `command` and `url` are set
248+
249+
| `protocol` | `command` | `url` | Behaviour |
250+
|---|---|---|---|
251+
| `stdio` (explicit) | set | any | Stdio transport, child via stdin/stdout — `url` ignored. |
252+
| `http` / `sse` / `streamable-http` (explicit) | set | set | **Locally-launched HTTP/SSE** — spawn child, wait for URL, connect via network. |
253+
| `http` / `sse` / `streamable-http` (explicit) | unset | set | Connect to remote URL — no spawn. |
254+
| `auto` or unset | set | any | Stdio (`command` wins over `url` for back-compat — set `protocol` explicitly to opt into the launcher). |
255+
| `auto` or unset | unset | set | HTTP/SSE remote — no spawn. |
256+
257+
The "command wins" rule under `auto` is intentional: it preserves backwards
258+
compatibility with configurations written before the launcher feature
259+
existed. To launch a local HTTP/SSE server you **must** set `protocol`
260+
explicitly to one of `http`, `sse`, or `streamable-http`.
261+
213262
### OAuth Configuration
214263

215264
```json

‎specs/046-local-launcher-for-http-sse/execution_log.md‎

Lines changed: 163 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,163 @@
1+
# Execution Log — 046-local-launcher-for-http-sse
2+
3+
State maintained per `CLAUDE.md` autonomous-operation requirement. Each
4+
session appends a dated entry; do not rewrite history.
5+
6+
## 2026-05-10 — Initial scaffold (Roman + Claude)
7+
8+
**Status**: Phase 0 + Phase 1 code landed in working tree (uncompiled —
9+
sandbox network blocks `proxy.golang.org`, see end of log). Phase 2 partial.
10+
11+
### Files added
12+
13+
- `internal/upstream/launcher/launcher.go` — `Spec`, `Handle`, `Spawn`. Owns the
14+
child's lifecycle (Stop with SIGTERM → grace → SIGKILL fallback, Wait, Done,
15+
Pid). Pumps stdout+stderr line-by-line into a caller-supplied `io.Writer`,
16+
one Write per line so a zap-bridge sink produces one log entry per line.
17+
- `internal/upstream/launcher/launcher_unix.go` — Setpgid + signal-the-pgroup
18+
for SIGTERM/SIGKILL on Linux/macOS.
19+
- `internal/upstream/launcher/launcher_windows.go` — best-effort stubs
20+
(matches the existing `process_windows.go` TODO; Job Objects are a
21+
follow-up).
22+
- `internal/upstream/launcher/wait.go` — `WaitForURL` does TCP-dial polling
23+
rather than HTTP GET (gotcha #2 in plan: SSE endpoints stream forever and
24+
break HTTP-GET probes).
25+
- `internal/upstream/launcher/wait_test.go` — 6 cases (immediately bound,
26+
bound late, never bound, ctx-canceled, bad URLs, default-port inference).
27+
- `internal/upstream/launcher/launcher_test.go` — 7 cases (graceful exit,
28+
SIGKILL fallback when SIGTERM is trapped, Done on natural exit, exit-code
29+
capture via `*exec.ExitError`, Stop idempotency, log sink capture, nil
30+
guards).
31+
- `internal/upstream/launcher/integration_test.go` — full Spawn + WaitForURL
32+
with a python-listener subprocess; skips when python3 is missing or on
33+
Windows. (Pure Go testdata helper would be cleaner — TODO.)
34+
- `internal/upstream/core/connection_launcher.go` — `connectWithLauncher`,
35+
`stopLauncher`, `watchLauncher`, `buildLauncherCmd`, `loggerWriter`.
36+
37+
### Files modified
38+
39+
- `internal/config/config.go` — `LauncherWaitTimeout Duration` on
40+
`ServerConfig`. Default 30s when zero/unset.
41+
- `internal/config/merge.go` — `CopyServerConfig` carries the new field.
42+
- `internal/upstream/core/client.go` — `launcherHandle launcher.Handle` and
43+
`launcherCIDFile string` on `Client`; new import.
44+
- `internal/upstream/core/connection.go` — pre-transport launcher dispatch
45+
for `http`/`sse`/`streamable-http` when `Command != ""`. Stops launcher
46+
in the connect-failure cleanup path.
47+
- `internal/upstream/core/connection_lifecycle.go` — `stopLauncher` after
48+
the MCP-client close in Disconnect (so the child sees the network
49+
transport go away first); also clears `processCmd`.
50+
- `docs/configuration.md` — new "Locally-launched HTTP / SSE servers"
51+
section + back-compat behaviour matrix; `launcher_wait_timeout` row in
52+
the Server Fields table.
53+
- `docs/cli-management-commands.md` — restart-semantics note covering the
54+
launcher stop-then-start order.
55+
56+
### Decisions / assumptions
57+
58+
1. **Stdio path untouched.** Plan's Phase 0 contemplated lifting env/Docker
59+
plumbing out of `connection_stdio.go` and routing stdio through
60+
`launcher.Spawn`. Doing that requires reworking how mcp-go owns the
61+
stdio process (mcp-go's `Stdio` transport spawns via a `CommandFunc` it
62+
controls — externally-spawned children can't be wired into it without
63+
patching the upstream library). To honour the spirit of "Docker-isolation
64+
logic must live in one place" without that reshuffling, the new
65+
`buildLauncherCmd` reuses the same Client methods (`setupDockerIsolation`,
66+
`injectEnvVarsIntoDockerArgs`, `insertCidfileIntoShellDockerCommand`,
67+
`wrapWithUserShell`) the stdio path already calls. Single source of
68+
truth, but no double-spawn risk.
69+
70+
2. **Launcher-managed children stay invisible to stdio cleanup helpers.**
71+
`connectWithLauncher` deliberately does NOT set `c.processCmd` /
72+
`c.processGroupID`. The `launcher.Handle` owns lifecycle; setting those
73+
would let stdio's `killProcessGroup` race with `Handle.Stop`. This is a
74+
minor deviation from the original plan (which suggested wiring the same
75+
process-group tracking) — the result is cleaner ownership.
76+
77+
3. **Health check is a TCP dial.** Per the plan's gotcha #2.
78+
`addrFromURL` infers default ports for http/https/ws/wss; rejects
79+
unknown schemes early so misconfigurations surface fast.
80+
81+
4. **StopGrace default is 5s.** Plan asked for an explicit decision (open
82+
question #2). 5s matches `processGracefulTimeout` in
83+
`internal/upstream/core/connection.go`. No per-server override yet —
84+
`Spec.StopGrace` is plumbed but not exposed in `ServerConfig`. Promote to
85+
config if a real-world server needs more.
86+
87+
5. **Crash-while-connected → Disconnect.** `watchLauncher` calls the
88+
`Client.Disconnect()` path on unexpected child exit (gotcha #6).
89+
Existing reconnect logic in `internal/upstream/managed` then handles
90+
the come-back attempt — no separate launcher-internal restart loop
91+
(open question #3 settled toward "defer to transport-level reconnect").
92+
93+
6. **Stop ctx on shutdown.** `stopLauncher` currently uses
94+
`context.WithTimeout(context.Background(), 10s)` everywhere. Plan
95+
open question #4 — accept this default; raise the limit if shutdown
96+
really needs to wait for slow Docker stop.
97+
98+
### Verification round 1 (2026-05-11)
99+
100+
After `sbx policy allow network proxy.golang.org,sum.golang.org` was set:
101+
102+
| Command | Result |
103+
|---|---|
104+
| `GOTOOLCHAIN=local go vet ./internal/upstream/...` | ✅ clean |
105+
| `GOTOOLCHAIN=local go test ./internal/upstream/launcher/...` | ✅ 15/15 |
106+
| `GOTOOLCHAIN=local go test ./internal/upstream/...` | ✅ all packages |
107+
| `GOTOOLCHAIN=local go test ./internal/config/...` | ✅ |
108+
| `go test -race` | ⚠️ blocked — cgo (gcc) not installed in sandbox; user can run on host |
109+
| `go build ./cmd/mcpproxy` | ❌ blocked — needs `storage.googleapis.com` (some Go modules CDN-served from there); user must add `sbx policy allow network storage.googleapis.com` |
110+
111+
### Bugs found + fixed during verification round 1
112+
113+
1. **Deadlock in connect-failure cleanup.** `Connect` holds `c.mu` for its
114+
entire duration; my original failure-path call to `c.stopLauncher(...)`
115+
re-acquired the same lock → hang. Fixed by inlining the stop sequence
116+
in `connection.go`'s cleanup branch (read fields under the held lock,
117+
release `c.mu` around `handle.Stop()`, reacquire before return).
118+
2. **`connectWithLauncher` redundant locking.** Same root cause —
119+
`connectWithLauncher` is called from `Connect` which already holds
120+
`c.mu`. Removed the inner `c.mu.Lock()/Unlock()` for the launcher
121+
field writes; the wait-for-url failure path still releases the lock
122+
around the blocking `handle.Stop()` and reacquires before returning.
123+
3. **`bytes.Buffer` LogSink race.** Test failures from the stdout pump,
124+
stderr pump, and the startup-banner write all racing on a single
125+
`*bytes.Buffer` in tests. Fixed by wrapping `LogSink` internally with
126+
a `serializedWriter` (mutex around `Write`). zap-bridge in production
127+
is already thread-safe, so this is a robustness fix for test sinks
128+
and any future single-writer adapters.
129+
4. **SIGKILL-fallback test could detect "ready" in the banner.** The
130+
launcher startup banner echoes the script source verbatim, so any
131+
marker token literally present in the script also matched in the
132+
banner — making the test think the trap was installed before the
133+
shell even ran. Fixed by using a shell-substituted marker
134+
(`__LNCTICK__:$$`) and a regex detector (`__LNCTICK__:[0-9]+`).
135+
5. **`bad scheme + explicit port` test case.** Test asserted error on
136+
`ftp://example.com:21/foo` but the launcher correctly accepts any
137+
scheme when the port is explicit (user took responsibility). Removed
138+
that case; replaced with the actually-invalid `ftp://example.com/foo`.
139+
140+
### Outstanding network blocker
141+
142+
```
143+
sbx policy allow network storage.googleapis.com
144+
```
145+
146+
Needed for `go build ./cmd/mcpproxy` to fetch Bleve/Roaring/etc. CDN-backed
147+
modules. Once allowed, the verification commands are:
148+
149+
```
150+
GOTOOLCHAIN=local go build ./cmd/mcpproxy
151+
./scripts/test-api-e2e.sh # optional smoke test
152+
```
153+
154+
### Outstanding follow-ups (post-PR)
155+
156+
- Replace `integration_test.go`'s python-shellout with a Go test-binary
157+
helper invoked via `os.Args` re-entry pattern, so the test runs on any
158+
CI that has Go (which is all of them). Plan called for a tiny binary in
159+
`internal/upstream/launcher/testdata/`.
160+
- Extend `scripts/test-api-e2e.sh` with a launcher-flavoured server (plan
161+
Phase 2 item).
162+
- Phase 3 (post-merge): `{port}` templating in `args` / `url`, per-launcher
163+
custom health probe, exponential backoff for repeated launcher crashes.

0 commit comments

Comments
 (0)