diff --git a/pkg/cli/devcontainer.go b/pkg/cli/devcontainer.go index ae5dfc56e6..8a96116aaa 100644 --- a/pkg/cli/devcontainer.go +++ b/pkg/cli/devcontainer.go @@ -191,17 +191,37 @@ func ensureDevcontainerConfig(verbose bool, additionalRepos []string) error { } } - // Write config file with proper indentation - data, err := json.MarshalIndent(config, "", " ") + // Serialize the new config to JSON + newData, err := json.MarshalIndent(config, "", " ") if err != nil { return fmt.Errorf("failed to marshal devcontainer.json: %w", err) } // Add newline at end of file - data = append(data, '\n') + newData = append(newData, '\n') + + // If file exists, check if content has changed (compare normalized JSON) + if existingConfig != nil { + // Serialize the existing config to compare + existingNormalized, err := json.MarshalIndent(existingConfig, "", " ") + if err != nil { + return fmt.Errorf("failed to marshal existing config for comparison: %w", err) + } + existingNormalized = append(existingNormalized, '\n') + + // Compare normalized JSON - if they're the same, skip writing + if string(newData) == string(existingNormalized) { + devcontainerLog.Printf("No changes detected, skipping write: %s", devcontainerPath) + if verbose { + fmt.Fprintf(os.Stderr, "No changes to devcontainer.json\n") + } + return nil + } + devcontainerLog.Printf("Changes detected, will update file: %s", devcontainerPath) + } // Use owner-only read/write permissions (0600) for security best practices - if err := os.WriteFile(devcontainerPath, data, 0600); err != nil { + if err := os.WriteFile(devcontainerPath, newData, 0600); err != nil { return fmt.Errorf("failed to write devcontainer.json: %w", err) } devcontainerLog.Printf("Wrote file: %s", devcontainerPath) diff --git a/pkg/cli/devcontainer_test.go b/pkg/cli/devcontainer_test.go index 86bd8db4c8..2096d79948 100644 --- a/pkg/cli/devcontainer_test.go +++ b/pkg/cli/devcontainer_test.go @@ -119,10 +119,27 @@ func TestEnsureDevcontainerConfig(t *testing.T) { } // Test that running again doesn't fail (idempotency) + // Get file stat before second run + statBefore, err := os.Stat(devcontainerPath) + if err != nil { + t.Fatalf("Failed to stat devcontainer.json before second run: %v", err) + } + err = ensureDevcontainerConfig(false, []string{}) if err != nil { t.Fatalf("ensureDevcontainerConfig() should be idempotent, but failed: %v", err) } + + // Get file stat after second run + statAfter, err := os.Stat(devcontainerPath) + if err != nil { + t.Fatalf("Failed to stat devcontainer.json after second run: %v", err) + } + + // File modification time should be the same (file should not have been rewritten) + if !statBefore.ModTime().Equal(statAfter.ModTime()) { + t.Error("Expected devcontainer.json to not be rewritten when no changes are needed") + } } func TestEnsureDevcontainerConfigWithAdditionalRepos(t *testing.T) { @@ -768,3 +785,79 @@ func TestGetCurrentRepoName(t *testing.T) { t.Error("Expected getCurrentRepoName() to return a non-empty string") } } + +func TestEnsureDevcontainerConfigNoWriteWhenUnchanged(t *testing.T) { + tmpDir := testutil.TempDir(t, "test-*") + + originalDir, err := os.Getwd() + if err != nil { + t.Fatalf("Failed to get current directory: %v", err) + } + defer func() { + _ = os.Chdir(originalDir) + }() + + if err := os.Chdir(tmpDir); err != nil { + t.Fatalf("Failed to change to temp directory: %v", err) + } + + // Initialize git repo + if err := exec.Command("git", "init").Run(); err != nil { + t.Skip("Git not available") + } + + // Configure git and add remote + exec.Command("git", "config", "user.name", "Test User").Run() + exec.Command("git", "config", "user.email", "test@example.com").Run() + exec.Command("git", "remote", "add", "origin", "https://github.com/testorg/testrepo.git").Run() + + // Create initial devcontainer.json + err = ensureDevcontainerConfig(false, []string{}) + if err != nil { + t.Fatalf("Initial ensureDevcontainerConfig() failed: %v", err) + } + + devcontainerPath := filepath.Join(".devcontainer", "devcontainer.json") + + // Get file info after first write + firstStat, err := os.Stat(devcontainerPath) + if err != nil { + t.Fatalf("Failed to stat file after first write: %v", err) + } + firstModTime := firstStat.ModTime() + + // Read the first content + firstContent, err := os.ReadFile(devcontainerPath) + if err != nil { + t.Fatalf("Failed to read file after first write: %v", err) + } + + // Run again with same parameters - should not write + err = ensureDevcontainerConfig(false, []string{}) + if err != nil { + t.Fatalf("Second ensureDevcontainerConfig() failed: %v", err) + } + + // Get file info after second run + secondStat, err := os.Stat(devcontainerPath) + if err != nil { + t.Fatalf("Failed to stat file after second run: %v", err) + } + secondModTime := secondStat.ModTime() + + // Read the second content + secondContent, err := os.ReadFile(devcontainerPath) + if err != nil { + t.Fatalf("Failed to read file after second run: %v", err) + } + + // Modification times should be equal (file was not rewritten) + if !firstModTime.Equal(secondModTime) { + t.Errorf("File was rewritten when no changes were needed. First modtime: %v, Second modtime: %v", firstModTime, secondModTime) + } + + // Content should be identical + if string(firstContent) != string(secondContent) { + t.Error("File content changed when it should not have") + } +}