diff --git a/internal/envutil/expand_env_args.go b/internal/envutil/expand_env_args.go index 0502922e..bf0c18ff 100644 --- a/internal/envutil/expand_env_args.go +++ b/internal/envutil/expand_env_args.go @@ -30,6 +30,8 @@ func WalkDockerEnvArgs(args []string, fn func(index int, varName, value string, // ExpandEnvArgs expands Docker -e flags that reference environment variables. // Converts "-e VAR_NAME" to "-e VAR_NAME=value" by reading from the process environment. // If the variable is not set, the flag is passed through unchanged. +// This behavior is intentionally different from config expansion in internal/config/expand.go, +// which fails hard when a referenced variable is undefined. func ExpandEnvArgs(args []string) []string { logExpand.Printf("Expanding env args: input_count=%d", len(args)) expandedValues := make(map[int]string) diff --git a/internal/logger/fileutil.go b/internal/logger/fileutil.go index 436a60af..72622f5a 100644 --- a/internal/logger/fileutil.go +++ b/internal/logger/fileutil.go @@ -137,10 +137,10 @@ func writeJSONToFile(logDir, fileName string, data any, perm os.FileMode) error // data MyData // } // -// The embedded jsonFileSink.writeJSON method can then be called from -// writeToFile to write data to the configured JSON file: +// The embedded jsonFileSink.writeJSON method can then be called directly +// at the persistence site to write data to the configured JSON file: // -// func (l *MyLogger) writeToFile() error { +// func (l *MyLogger) LogData() error { // return l.writeJSON(l.data, 0644) // } type jsonFileSink struct { diff --git a/internal/logger/observed_url_domains_logger.go b/internal/logger/observed_url_domains_logger.go index 9f440fd5..d5e6eae5 100644 --- a/internal/logger/observed_url_domains_logger.go +++ b/internal/logger/observed_url_domains_logger.go @@ -47,7 +47,7 @@ var observedURLDomainsLoggerFactory = newLoggerFactory( jsonFileSink: jsonFileSink{logDir: logDir, fileName: fileName}, data: make(map[string]map[string]struct{}), } - if err := l.writeToFile(); err != nil { + if err := l.writeJSON(make(map[string][]string), 0600); err != nil { return nil, err } log.Printf("Observed URL domains logging to file: %s", filepath.Join(logDir, fileName)) @@ -98,18 +98,14 @@ func (l *ObservedURLDomainsLogger) LogDomains(serverID string, domains []string) if !changed { return nil } - return l.writeToFile() + serialized := make(map[string][]string, len(l.data)) + for serverID, serverDomains := range l.data { + serialized[serverID] = util.SortedSetKeys(serverDomains) + } + return l.writeJSON(serialized, 0600) }) } -func (l *ObservedURLDomainsLogger) writeToFile() error { - serialized := make(map[string][]string, len(l.data)) - for serverID, domains := range l.data { - serialized[serverID] = util.SortedSetKeys(domains) - } - return l.writeJSON(serialized, 0600) -} - func (l *ObservedURLDomainsLogger) Close() error { return nil } // LogObservedURLDomains appends newly observed domains for a server. diff --git a/internal/logger/tools_logger.go b/internal/logger/tools_logger.go index 6f999671..3fbe7bd3 100644 --- a/internal/logger/tools_logger.go +++ b/internal/logger/tools_logger.go @@ -80,16 +80,10 @@ func (tl *ToolsLogger) LogTools(serverID string, tools []ToolInfo) error { tl.data.Servers[serverID] = tools // Write the updated data to file - return tl.writeToFile() + return tl.writeJSON(tl.data, 0644) }) } -// writeToFile writes the current tools data to the JSON file. -// Caller must hold tl.mu lock. -func (tl *ToolsLogger) writeToFile() error { - return tl.writeJSON(tl.data, 0644) -} - // Close is a no-op for ToolsLogger (implements closableLogger interface) func (tl *ToolsLogger) Close() error { // No file handle to close since we write directly each time diff --git a/internal/logger/tools_logger_test.go b/internal/logger/tools_logger_test.go index 0a417293..b799cd75 100644 --- a/internal/logger/tools_logger_test.go +++ b/internal/logger/tools_logger_test.go @@ -214,8 +214,8 @@ func TestToolsLoggerFallback(t *testing.T) { assert.NoError(err, "CloseAllLoggers failed") } -// TestWriteToFile_Success verifies writeToFile writes valid JSON atomically. -func TestWriteToFile_Success(t *testing.T) { +// TestWriteJSON_Success verifies writeJSON writes valid JSON atomically. +func TestWriteJSON_Success(t *testing.T) { assert := assert.New(t) require := require.New(t) @@ -231,8 +231,8 @@ func TestWriteToFile_Success(t *testing.T) { }, } - err := tl.writeToFile() - require.NoError(err, "writeToFile should succeed") + err := tl.writeJSON(tl.data, 0644) + require.NoError(err, "writeJSON should succeed") // Verify file was written data, err := os.ReadFile(filepath.Join(tmpDir, "tools.json")) @@ -246,8 +246,8 @@ func TestWriteToFile_Success(t *testing.T) { assert.True(os.IsNotExist(err), "temp file should be removed after rename") } -// TestWriteToFile_WriteFileFails verifies the error path when os.WriteFile fails. -func TestWriteToFile_WriteFileFails(t *testing.T) { +// TestWriteJSON_WriteFileFails verifies the error path when os.WriteFile fails. +func TestWriteJSON_WriteFileFails(t *testing.T) { assert := assert.New(t) tl := &ToolsLogger{ @@ -255,14 +255,14 @@ func TestWriteToFile_WriteFileFails(t *testing.T) { data: &ToolsData{Servers: make(map[string][]ToolInfo)}, } - err := tl.writeToFile() - assert.Error(err, "writeToFile should fail when logDir does not exist") + err := tl.writeJSON(tl.data, 0644) + assert.Error(err, "writeJSON should fail when logDir does not exist") assert.ErrorContains(err, "failed to write temp file") } -// TestWriteToFile_RenameFails verifies the error and cleanup path when os.Rename fails. +// TestWriteJSON_RenameFails verifies the error and cleanup path when os.Rename fails. // On Linux, renaming a regular file to a path occupied by a directory returns EISDIR. -func TestWriteToFile_RenameFails(t *testing.T) { +func TestWriteJSON_RenameFails(t *testing.T) { assert := assert.New(t) require := require.New(t) @@ -277,8 +277,8 @@ func TestWriteToFile_RenameFails(t *testing.T) { data: &ToolsData{Servers: make(map[string][]ToolInfo)}, } - err := tl.writeToFile() - assert.Error(err, "writeToFile should fail when rename target is a directory") + err := tl.writeJSON(tl.data, 0644) + assert.Error(err, "writeJSON should fail when rename target is a directory") assert.ErrorContains(err, "failed to rename temp file") // Verify that the cleanup removed the temp file.