From bb16f247c6cde11c5ae162b9616a65392639961e Mon Sep 17 00:00:00 2001 From: Jimmy Stridh <61634+jimmystridh@users.noreply.github.com> Date: Sat, 29 Nov 2025 03:51:27 +0100 Subject: [PATCH] fix(init): error on invalid JSON instead of overwriting settings (#404) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously, setupClaudeSettings would silently create an empty settings map when json.Unmarshal failed, then write just a prompt field to the file - destroying all existing user settings (permissions, hooks, etc). Now returns a clear error asking the user to fix the JSON syntax manually, preserving their original file contents. Also properly handles permission errors when reading existing files. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Jimmy Stridh Co-authored-by: Claude --- cmd/bd/init.go | 8 ++- cmd/bd/init_test.go | 166 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 173 insertions(+), 1 deletion(-) diff --git a/cmd/bd/init.go b/cmd/bd/init.go index abd1df14..01cc9c75 100644 --- a/cmd/bd/init.go +++ b/cmd/bd/init.go @@ -1324,9 +1324,15 @@ func setupClaudeSettings(verbose bool) error { // #nosec G304 - user config path if content, err := os.ReadFile(settingsPath); err == nil { if err := json.Unmarshal(content, &existingSettings); err != nil { - existingSettings = make(map[string]interface{}) + // Don't silently overwrite - the user has a file with invalid JSON + // that likely contains important settings they don't want to lose + return fmt.Errorf("existing %s contains invalid JSON: %w\nPlease fix the JSON syntax manually before running bd init", settingsPath, err) } + } else if !os.IsNotExist(err) { + // File exists but couldn't be read (permissions issue, etc.) + return fmt.Errorf("failed to read existing %s: %w", settingsPath, err) } else { + // File doesn't exist - create new empty settings existingSettings = make(map[string]interface{}) } diff --git a/cmd/bd/init_test.go b/cmd/bd/init_test.go index 6aeb426c..8d3c25a1 100644 --- a/cmd/bd/init_test.go +++ b/cmd/bd/init_test.go @@ -996,3 +996,169 @@ func TestReadFirstIssueFromJSONL_EmptyFile(t *testing.T) { t.Errorf("Expected nil issue for empty file, got %+v", issue) } } + +// TestSetupClaudeSettings_InvalidJSON verifies that invalid JSON in existing +// settings.local.json returns an error instead of silently overwriting. +// This is a regression test for bd-5bj where user settings were lost. +func TestSetupClaudeSettings_InvalidJSON(t *testing.T) { + tmpDir := t.TempDir() + originalWd, err := os.Getwd() + if err != nil { + t.Fatalf("Failed to get working directory: %v", err) + } + defer os.Chdir(originalWd) + + if err := os.Chdir(tmpDir); err != nil { + t.Fatalf("Failed to change to temp directory: %v", err) + } + + // Create .claude directory + claudeDir := filepath.Join(tmpDir, ".claude") + if err := os.MkdirAll(claudeDir, 0755); err != nil { + t.Fatalf("Failed to create .claude directory: %v", err) + } + + // Create settings.local.json with invalid JSON (array syntax in object context) + // This is the exact pattern that caused the bug in the user's file + invalidJSON := `{ + "permissions": { + "allow": [ + "Bash(python3:*)" + ], + "deny": [ + "_comment": "Add commands to block here" + ] + } +}` + settingsPath := filepath.Join(claudeDir, "settings.local.json") + if err := os.WriteFile(settingsPath, []byte(invalidJSON), 0644); err != nil { + t.Fatalf("Failed to write invalid settings: %v", err) + } + + // Call setupClaudeSettings - should return an error + err = setupClaudeSettings(false) + if err == nil { + t.Fatal("Expected error for invalid JSON, got nil") + } + + // Verify the error message mentions invalid JSON + if !strings.Contains(err.Error(), "invalid JSON") { + t.Errorf("Expected error to mention 'invalid JSON', got: %v", err) + } + + // Verify the original file was NOT modified + content, err := os.ReadFile(settingsPath) + if err != nil { + t.Fatalf("Failed to read settings file: %v", err) + } + + if !strings.Contains(string(content), "permissions") { + t.Error("Original file content should be preserved") + } + + if strings.Contains(string(content), "bd onboard") { + t.Error("File should NOT contain bd onboard prompt after error") + } +} + +// TestSetupClaudeSettings_ValidJSON verifies that valid JSON is properly updated +func TestSetupClaudeSettings_ValidJSON(t *testing.T) { + tmpDir := t.TempDir() + originalWd, err := os.Getwd() + if err != nil { + t.Fatalf("Failed to get working directory: %v", err) + } + defer os.Chdir(originalWd) + + if err := os.Chdir(tmpDir); err != nil { + t.Fatalf("Failed to change to temp directory: %v", err) + } + + // Create .claude directory + claudeDir := filepath.Join(tmpDir, ".claude") + if err := os.MkdirAll(claudeDir, 0755); err != nil { + t.Fatalf("Failed to create .claude directory: %v", err) + } + + // Create settings.local.json with valid JSON + validJSON := `{ + "permissions": { + "allow": [ + "Bash(python3:*)" + ] + }, + "hooks": { + "PreToolUse": [] + } +}` + settingsPath := filepath.Join(claudeDir, "settings.local.json") + if err := os.WriteFile(settingsPath, []byte(validJSON), 0644); err != nil { + t.Fatalf("Failed to write valid settings: %v", err) + } + + // Call setupClaudeSettings - should succeed + err = setupClaudeSettings(false) + if err != nil { + t.Fatalf("Expected no error for valid JSON, got: %v", err) + } + + // Verify the file was updated with prompt AND preserved existing settings + content, err := os.ReadFile(settingsPath) + if err != nil { + t.Fatalf("Failed to read settings file: %v", err) + } + + contentStr := string(content) + + // Should contain the new prompt + if !strings.Contains(contentStr, "bd onboard") { + t.Error("File should contain bd onboard prompt") + } + + // Should preserve existing permissions + if !strings.Contains(contentStr, "permissions") { + t.Error("File should preserve permissions section") + } + + // Should preserve existing hooks + if !strings.Contains(contentStr, "hooks") { + t.Error("File should preserve hooks section") + } + + if !strings.Contains(contentStr, "PreToolUse") { + t.Error("File should preserve PreToolUse hook") + } +} + +// TestSetupClaudeSettings_NoExistingFile verifies behavior when no file exists +func TestSetupClaudeSettings_NoExistingFile(t *testing.T) { + tmpDir := t.TempDir() + originalWd, err := os.Getwd() + if err != nil { + t.Fatalf("Failed to get working directory: %v", err) + } + defer os.Chdir(originalWd) + + if err := os.Chdir(tmpDir); err != nil { + t.Fatalf("Failed to change to temp directory: %v", err) + } + + // Don't create .claude directory - setupClaudeSettings should create it + + // Call setupClaudeSettings - should succeed + err = setupClaudeSettings(false) + if err != nil { + t.Fatalf("Expected no error when no file exists, got: %v", err) + } + + // Verify the file was created with prompt + settingsPath := filepath.Join(tmpDir, ".claude", "settings.local.json") + content, err := os.ReadFile(settingsPath) + if err != nil { + t.Fatalf("Failed to read settings file: %v", err) + } + + if !strings.Contains(string(content), "bd onboard") { + t.Error("File should contain bd onboard prompt") + } +}