Skip to content

Commit 86295b9

Browse files
committed
fix(logs): route daemon + CLI log reads through sanitized filename helper
Addresses Codex review on PR #604: the writer sanitized per-server log filenames but two READ sites still derived the raw server-%s.log path, so GET /api/v1/servers/{id}/logs and no-daemon CLI reads still 404'd for namespace/name registry servers. Export ServerLogFilename and route both read sites through it; tie the read-path filename to the writer's flat file in the regression test. Related #598
1 parent 55eb07a commit 86295b9

4 files changed

Lines changed: 19 additions & 2 deletions

File tree

cmd/mcpproxy/upstream_cmd.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -772,7 +772,7 @@ func runUpstreamLogsFromFile(globalConfig *config.Config, serverName string) err
772772
}
773773
}
774774

775-
logFile := filepath.Join(logDir, fmt.Sprintf("server-%s.log", serverName))
775+
logFile := filepath.Join(logDir, logs.ServerLogFilename(serverName))
776776

777777
// Check if file exists
778778
if _, err := os.Stat(logFile); os.IsNotExist(err) {

internal/logs/logger.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -280,6 +280,16 @@ func serverLogFilename(serverName string) string {
280280
return fmt.Sprintf("server-%s.log", sanitizeServerLogName(serverName))
281281
}
282282

283+
// ServerLogFilename is the exported accessor for the per-server log filename.
284+
// Read sites outside this package (the REST/daemon log reader in internal/server
285+
// and the no-daemon CLI file reader in cmd/mcpproxy) MUST derive the path through
286+
// this helper so reads resolve to the same sanitized flat file the writers create
287+
// for namespace/name registry servers (MCP-1111). Do not re-derive "server-%s.log"
288+
// by hand at a call site.
289+
func ServerLogFilename(serverName string) string {
290+
return serverLogFilename(serverName)
291+
}
292+
283293
// sanitizeServerLogName replaces every character that is not safe in a single
284294
// filename element with "_", keeping ASCII letters, digits, ".", "-" and "_".
285295
// The result is always a single path element (no "/" or "\\"), so it can never

internal/logs/logger_test.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,4 +77,11 @@ func TestCreateUpstreamServerLogger_NamespacedNameFlatFile(t *testing.T) {
7777
require.NoError(t, err)
7878
require.NotEmpty(t, lines, "tail reader must round-trip the namespaced server name to its flat log file")
7979
assert.Contains(t, lines[len(lines)-1], "hello from polymarket-guard")
80+
81+
// The exported helper (used by the out-of-package read sites in internal/server
82+
// and cmd/mcpproxy) MUST resolve a namespaced name to the same flat file the
83+
// writer created — otherwise daemon/CLI log reads 404 for slash-bearing names
84+
// (the read-site divergence Codex flagged on PR #604).
85+
assert.Equal(t, flatPath, filepath.Join(logDir, ServerLogFilename(serverName)),
86+
"ServerLogFilename must resolve to the writer's flat file for a namespaced name")
8087
}

internal/server/server.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2282,7 +2282,7 @@ func (s *Server) GetServerLogs(serverName string, tail int) ([]contracts.LogEntr
22822282
}
22832283
}
22842284

2285-
logFile := filepath.Join(logDir, fmt.Sprintf("server-%s.log", serverName))
2285+
logFile := filepath.Join(logDir, logs.ServerLogFilename(serverName))
22862286

22872287
// Check if file exists
22882288
if _, err := os.Stat(logFile); os.IsNotExist(err) {

0 commit comments

Comments
 (0)