Skip to content

Commit 264a71a

Browse files
fix(storage): persist registry origin/provenance on UpstreamRecord (MCP-866)
CI caught the new ServerConfig fields tripping the storage field-coverage canary (TestSaveServerSyncFieldCoverage) — they were unpersisted. Persist them so a server's registry origin survives a restart; otherwise a reloaded custom-origin server would lose its provenance and the skip_quarantine guard plus the approval/quarantine view would silently stop working. - Add SourceRegistryID + SourceRegistryProvenance to UpstreamRecord. - Carry them through every config<->record conversion (async saveServerSync, Manager.SaveUpstreamServer, GetUpstreamServer, ListUpstreamServers, ListQuarantinedUpstreamServers). - Extend the field-coverage canary's expectedFields; add a save->reload round-trip test (incl. via the quarantine listing). Fixes the Unit Tests / E2E / Build Binaries CI failures on #573 (all ran go test ./... and hit the same storage canary). storage suite green with -race; go build ./... clean; gofmt clean. Related MCP-866 Co-Authored-By: Paperclip <noreply@paperclip.ing>
1 parent eba1787 commit 264a71a

5 files changed

Lines changed: 73 additions & 8 deletions

File tree

internal/storage/async_ops.go

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -169,14 +169,14 @@ func (am *AsyncManager) quarantineServerSync(name string, quarantined bool) erro
169169

170170
func (am *AsyncManager) saveServerSync(serverConfig *config.ServerConfig) error {
171171
record := &UpstreamRecord{
172-
ID: serverConfig.Name,
173-
Name: serverConfig.Name,
174-
URL: serverConfig.URL,
175-
Protocol: serverConfig.Protocol,
176-
Command: serverConfig.Command,
177-
Args: serverConfig.Args,
178-
Env: serverConfig.Env,
179-
WorkingDir: serverConfig.WorkingDir,
172+
ID: serverConfig.Name,
173+
Name: serverConfig.Name,
174+
URL: serverConfig.URL,
175+
Protocol: serverConfig.Protocol,
176+
Command: serverConfig.Command,
177+
Args: serverConfig.Args,
178+
Env: serverConfig.Env,
179+
WorkingDir: serverConfig.WorkingDir,
180180
Enabled: serverConfig.Enabled,
181181
Quarantined: serverConfig.Quarantined,
182182
Headers: serverConfig.Headers,
@@ -189,6 +189,9 @@ func (am *AsyncManager) saveServerSync(serverConfig *config.ServerConfig) error
189189
OAuth: serverConfig.OAuth,
190190
EnabledTools: serverConfig.EnabledTools,
191191
DisabledTools: serverConfig.DisabledTools,
192+
193+
SourceRegistryID: serverConfig.SourceRegistryID,
194+
SourceRegistryProvenance: serverConfig.SourceRegistryProvenance,
192195
}
193196
return am.db.SaveUpstream(record)
194197
}

internal/storage/async_ops_test.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -248,6 +248,10 @@ func TestSaveServerSyncFieldCoverage(t *testing.T) {
248248
"LauncherWaitTimeout": true, // Spec 046: persisted to BBolt so REST-API-added launcher servers survive restarts
249249
"EnabledTools": true, // feat/config-tool-allowlist: persisted to BBolt
250250
"DisabledTools": true, // feat/config-tool-allowlist: persisted to BBolt
251+
// MCP-866: persisted to BBolt so a server's registry origin/provenance
252+
// (and the custom-origin skip_quarantine guard) survive a restart.
253+
"SourceRegistryID": true,
254+
"SourceRegistryProvenance": true,
251255
}
252256

253257
// Get all fields from ServerConfig

internal/storage/manager.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,9 @@ func (m *Manager) SaveUpstreamServer(serverConfig *config.ServerConfig) error {
104104
LauncherWaitTimeout: serverConfig.LauncherWaitTimeout,
105105
EnabledTools: serverConfig.EnabledTools,
106106
DisabledTools: serverConfig.DisabledTools,
107+
108+
SourceRegistryID: serverConfig.SourceRegistryID,
109+
SourceRegistryProvenance: serverConfig.SourceRegistryProvenance,
107110
}
108111

109112
return m.db.SaveUpstream(record)
@@ -138,6 +141,9 @@ func (m *Manager) GetUpstreamServer(name string) (*config.ServerConfig, error) {
138141
LauncherWaitTimeout: record.LauncherWaitTimeout,
139142
EnabledTools: record.EnabledTools,
140143
DisabledTools: record.DisabledTools,
144+
145+
SourceRegistryID: record.SourceRegistryID,
146+
SourceRegistryProvenance: record.SourceRegistryProvenance,
141147
}, nil
142148
}
143149

@@ -172,6 +178,9 @@ func (m *Manager) ListUpstreamServers() ([]*config.ServerConfig, error) {
172178
LauncherWaitTimeout: record.LauncherWaitTimeout,
173179
EnabledTools: record.EnabledTools,
174180
DisabledTools: record.DisabledTools,
181+
182+
SourceRegistryID: record.SourceRegistryID,
183+
SourceRegistryProvenance: record.SourceRegistryProvenance,
175184
})
176185
}
177186

@@ -220,6 +229,9 @@ func (m *Manager) ListQuarantinedUpstreamServers() ([]*config.ServerConfig, erro
220229
Isolation: record.Isolation,
221230
EnabledTools: record.EnabledTools,
222231
DisabledTools: record.DisabledTools,
232+
233+
SourceRegistryID: record.SourceRegistryID,
234+
SourceRegistryProvenance: record.SourceRegistryProvenance,
223235
})
224236

225237
m.logger.Debugw("Added server to quarantined list",

internal/storage/models.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,11 @@ type UpstreamRecord struct {
101101
LauncherWaitTimeout config.Duration `json:"launcher_wait_timeout,omitempty"` // Spec 046: max wait for locally-launched HTTP/SSE upstream URL to become reachable
102102
EnabledTools []string `json:"enabled_tools,omitempty"` // Allowlist: only these tools are exposed
103103
DisabledTools []string `json:"disabled_tools,omitempty"` // Denylist: these tools are hidden
104+
// MCP-866: persist a server's registry origin + provenance so the
105+
// approval/quarantine view and the custom-origin skip_quarantine guard
106+
// survive a restart.
107+
SourceRegistryID string `json:"source_registry_id,omitempty"`
108+
SourceRegistryProvenance string `json:"source_registry_provenance,omitempty"`
104109
}
105110

106111
// ToolStatRecord represents tool usage statistics
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
package storage
2+
3+
import (
4+
"testing"
5+
6+
"github.com/smart-mcp-proxy/mcpproxy-go/internal/config"
7+
"github.com/stretchr/testify/assert"
8+
"github.com/stretchr/testify/require"
9+
"go.uber.org/zap"
10+
)
11+
12+
// MCP-866: a server's registry origin + provenance must survive a save→reload
13+
// so the approval/quarantine view and the custom-origin skip_quarantine guard
14+
// keep working after a restart.
15+
func TestUpstreamServer_ProvenanceRoundTrips(t *testing.T) {
16+
m, err := NewManager(t.TempDir(), zap.NewNop().Sugar())
17+
require.NoError(t, err)
18+
defer m.Close()
19+
20+
require.NoError(t, m.SaveUpstreamServer(&config.ServerConfig{
21+
Name: "acme-widget",
22+
Protocol: "stdio",
23+
Command: "npx",
24+
Enabled: true,
25+
Quarantined: true,
26+
SourceRegistryID: "acme",
27+
SourceRegistryProvenance: config.RegistryProvenanceCustom,
28+
}))
29+
30+
got, err := m.GetUpstreamServer("acme-widget")
31+
require.NoError(t, err)
32+
assert.Equal(t, "acme", got.SourceRegistryID)
33+
assert.Equal(t, config.RegistryProvenanceCustom, got.SourceRegistryProvenance)
34+
35+
// And it surfaces through the quarantine listing (the approval view).
36+
q, err := m.ListQuarantinedUpstreamServers()
37+
require.NoError(t, err)
38+
require.Len(t, q, 1)
39+
assert.Equal(t, "acme", q[0].SourceRegistryID)
40+
assert.Equal(t, config.RegistryProvenanceCustom, q[0].SourceRegistryProvenance)
41+
}

0 commit comments

Comments
 (0)