Skip to content

fix(046): persist LauncherWaitTimeout to BBolt + regenerate OAS #454

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Dumbris merged 3 commits into from
May 12, 2026
Merged

fix(046): persist LauncherWaitTimeout to BBolt + regenerate OAS #454

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2e89c7c
fix(046): persist LauncherWaitTimeout to BBolt + regenerate OAS
claude May 12, 2026
77dff7d
fix(046): drain launcher log pumps before cmd.Wait() to stop CI flake
claude May 12, 2026
569e8cd
fix(046): switch TestWaitForURL_InfersDefaultPort host to TEST-NET-1
claude May 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 18 additions & 17 deletions internal/storage/async_ops_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -228,23 +228,24 @@ func TestSaveServerSyncPreservesNilFields(t *testing.T) {
func TestSaveServerSyncFieldCoverage(t *testing.T) {
// List of ServerConfig fields that ARE expected to be copied
expectedFields := map[string]bool{
"Name": true,
"URL": true,
"Protocol": true,
"Command": true,
"Args": true,
"WorkingDir": true,
"Env": true,
"Headers": true,
"OAuth": true,
"Enabled": true,
"Quarantined": true,
"Created": true,
"Updated": true, // Updated is set by saveServerSync, not copied
"Isolation": true,
"Shared": true, // Teams-only: persisted in JSON config, not in BBolt
"SkipQuarantine": true, // Spec 032: runtime-only field, not persisted to BBolt
"ReconnectOnUse": true, // Spec 354: persisted to BBolt for on-demand reconnection
"Name": true,
"URL": true,
"Protocol": true,
"Command": true,
"Args": true,
"WorkingDir": true,
"Env": true,
"Headers": true,
"OAuth": true,
"Enabled": true,
"Quarantined": true,
"Created": true,
"Updated": true, // Updated is set by saveServerSync, not copied
"Isolation": true,
"Shared": true, // Teams-only: persisted in JSON config, not in BBolt
"SkipQuarantine": true, // Spec 032: runtime-only field, not persisted to BBolt
"ReconnectOnUse": true, // Spec 354: persisted to BBolt for on-demand reconnection
"LauncherWaitTimeout": true, // Spec 046: persisted to BBolt so REST-API-added launcher servers survive restarts
}

// Get all fields from ServerConfig
Expand Down
95 changes: 49 additions & 46 deletions internal/storage/manager.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,22 +85,23 @@ func (m *Manager) SaveUpstreamServer(serverConfig *config.ServerConfig) error {
defer m.mu.Unlock()

record := &UpstreamRecord{
ID: serverConfig.Name, // Use name as ID for simplicity
Name: serverConfig.Name,
URL: serverConfig.URL,
Protocol: serverConfig.Protocol,
Command: serverConfig.Command,
Args: serverConfig.Args,
WorkingDir: serverConfig.WorkingDir,
Env: serverConfig.Env,
Headers: serverConfig.Headers,
OAuth: serverConfig.OAuth,
Enabled: serverConfig.Enabled,
Quarantined: serverConfig.Quarantined,
Created: serverConfig.Created,
Updated: time.Now(),
Isolation: serverConfig.Isolation,
ReconnectOnUse: serverConfig.ReconnectOnUse,
ID: serverConfig.Name, // Use name as ID for simplicity
Name: serverConfig.Name,
URL: serverConfig.URL,
Protocol: serverConfig.Protocol,
Command: serverConfig.Command,
Args: serverConfig.Args,
WorkingDir: serverConfig.WorkingDir,
Env: serverConfig.Env,
Headers: serverConfig.Headers,
OAuth: serverConfig.OAuth,
Enabled: serverConfig.Enabled,
Quarantined: serverConfig.Quarantined,
Created: serverConfig.Created,
Updated: time.Now(),
Isolation: serverConfig.Isolation,
ReconnectOnUse: serverConfig.ReconnectOnUse,
LauncherWaitTimeout: serverConfig.LauncherWaitTimeout,
}

return m.db.SaveUpstream(record)
Expand All @@ -117,21 +118,22 @@ func (m *Manager) GetUpstreamServer(name string) (*config.ServerConfig, error) {
}

return &config.ServerConfig{
Name: record.Name,
URL: record.URL,
Protocol: record.Protocol,
Command: record.Command,
Args: record.Args,
WorkingDir: record.WorkingDir,
Env: record.Env,
Headers: record.Headers,
OAuth: record.OAuth,
Enabled: record.Enabled,
Quarantined: record.Quarantined,
Created: record.Created,
Updated: record.Updated,
Isolation: record.Isolation,
ReconnectOnUse: record.ReconnectOnUse,
Name: record.Name,
URL: record.URL,
Protocol: record.Protocol,
Command: record.Command,
Args: record.Args,
WorkingDir: record.WorkingDir,
Env: record.Env,
Headers: record.Headers,
OAuth: record.OAuth,
Enabled: record.Enabled,
Quarantined: record.Quarantined,
Created: record.Created,
Updated: record.Updated,
Isolation: record.Isolation,
ReconnectOnUse: record.ReconnectOnUse,
LauncherWaitTimeout: record.LauncherWaitTimeout,
}, nil
}

Expand All @@ -148,21 +150,22 @@ func (m *Manager) ListUpstreamServers() ([]*config.ServerConfig, error) {
var servers []*config.ServerConfig
for _, record := range records {
servers = append(servers, &config.ServerConfig{
Name: record.Name,
URL: record.URL,
Protocol: record.Protocol,
Command: record.Command,
Args: record.Args,
WorkingDir: record.WorkingDir,
Env: record.Env,
Headers: record.Headers,
OAuth: record.OAuth,
Enabled: record.Enabled,
Quarantined: record.Quarantined,
Created: record.Created,
Updated: record.Updated,
Isolation: record.Isolation,
ReconnectOnUse: record.ReconnectOnUse,
Name: record.Name,
URL: record.URL,
Protocol: record.Protocol,
Command: record.Command,
Args: record.Args,
WorkingDir: record.WorkingDir,
Env: record.Env,
Headers: record.Headers,
OAuth: record.OAuth,
Enabled: record.Enabled,
Quarantined: record.Quarantined,
Created: record.Created,
Updated: record.Updated,
Isolation: record.Isolation,
ReconnectOnUse: record.ReconnectOnUse,
LauncherWaitTimeout: record.LauncherWaitTimeout,
})
}

Expand Down
33 changes: 17 additions & 16 deletions internal/storage/models.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,22 +68,23 @@ const CurrentSchemaVersion = 2

// UpstreamRecord represents an upstream server record in storage
type UpstreamRecord struct {
ID string `json:"id"`
Name string `json:"name"`
URL string `json:"url,omitempty"`
Protocol string `json:"protocol,omitempty"` // stdio, http, sse, streamable-http, auto
Command string `json:"command,omitempty"`
Args []string `json:"args,omitempty"`
WorkingDir string `json:"working_dir,omitempty"` // Working directory for stdio servers
Env map[string]string `json:"env,omitempty"`
Headers map[string]string `json:"headers,omitempty"` // For HTTP authentication
OAuth *config.OAuthConfig `json:"oauth,omitempty"` // OAuth configuration
Enabled bool `json:"enabled"`
Quarantined bool `json:"quarantined"` // Security quarantine status
Created time.Time `json:"created"`
Updated time.Time `json:"updated"`
Isolation *config.IsolationConfig `json:"isolation,omitempty"` // Per-server isolation settings
ReconnectOnUse bool `json:"reconnect_on_use,omitempty"` // Attempt reconnection on tool call
ID string `json:"id"`
Name string `json:"name"`
URL string `json:"url,omitempty"`
Protocol string `json:"protocol,omitempty"` // stdio, http, sse, streamable-http, auto
Command string `json:"command,omitempty"`
Args []string `json:"args,omitempty"`
WorkingDir string `json:"working_dir,omitempty"` // Working directory for stdio servers
Env map[string]string `json:"env,omitempty"`
Headers map[string]string `json:"headers,omitempty"` // For HTTP authentication
OAuth *config.OAuthConfig `json:"oauth,omitempty"` // OAuth configuration
Enabled bool `json:"enabled"`
Quarantined bool `json:"quarantined"` // Security quarantine status
Created time.Time `json:"created"`
Updated time.Time `json:"updated"`
Isolation *config.IsolationConfig `json:"isolation,omitempty"` // Per-server isolation settings
ReconnectOnUse bool `json:"reconnect_on_use,omitempty"` // Attempt reconnection on tool call
LauncherWaitTimeout config.Duration `json:"launcher_wait_timeout,omitempty"` // Spec 046: max wait for locally-launched HTTP/SSE upstream URL to become reachable
}

// ToolStatRecord represents tool usage statistics
Expand Down
10 changes: 8 additions & 2 deletions internal/upstream/launcher/launcher.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -266,9 +266,15 @@ func (h *handle) stopLocked(ctx context.Context) error {
}

func (h *handle) reap() {
err := h.cmd.Wait()
// Drain log pumps so any final output lands in the sink.
// Drain log pumps FIRST. exec.Cmd.Wait() closes the parent-side stdout/stderr
// pipes as part of its cleanup; if we call Wait() before the pumps have read
// to EOF, the pump scanners can race against that close and observe EOF with
// buffered child output still in flight, dropping it on the floor. Per the
// exec.Cmd doc: "it is thus incorrect to call Wait before all reads from the
// pipe have completed." Pumps naturally finish when the child exits and the
// kernel propagates EOF — we don't need Wait() to reach that state.
h.pumpWG.Wait()
err := h.cmd.Wait()

h.waitErrMu.Lock()
h.waitErr = err
Expand Down
7 changes: 6 additions & 1 deletion internal/upstream/launcher/wait_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,8 +127,13 @@ func TestWaitForURL_InfersDefaultPort(t *testing.T) {
// that an http:// URL without a port is parsed (rather than rejected)
// by checking it produces a "not reachable" error rather than a
// parse error within the short timeout window.
//
// Use TEST-NET-1 (RFC 5737, 192.0.2.0/24) as the host — it's reserved
// for documentation and guaranteed non-routable, so the dial fails
// consistently across OSes. 127.0.0.1:80 is bound on Windows GitHub
// runners (IIS/WinNAT), which would make this test pass spuriously.
ctx := context.Background()
err := WaitForURL(ctx, "http://127.0.0.1/", 100*time.Millisecond)
err := WaitForURL(ctx, "http://192.0.2.1/", 100*time.Millisecond)
require.Error(t, err)
assert.Contains(t, err.Error(), "not reachable", "default-port inference should reach the polling loop, not fail at parse")
}
2 changes: 1 addition & 1 deletion oas/docs.go
View file
Open in desktop

Large diffs are not rendered by default.

8 changes: 8 additions & 0 deletions oas/swagger.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -500,6 +500,14 @@ components:
type: object
isolation:
$ref: '#/components/schemas/config.IsolationConfig'
launcher_wait_timeout:
description: |-
LauncherWaitTimeout caps how long mcpproxy will wait for a locally-launched
HTTP/SSE upstream's URL to become reachable after Spawn(). Only consulted
when the server is configured with both Command and an HTTP/SSE URL — i.e.,
mcpproxy starts the process AND connects via network. Stdio servers ignore
this field. Zero or unset → 30s default.
type: string
name:
type: string
oauth:
Expand Down
Loading