Skip to content

Commit 5bc40b0

Browse files
Document auto-written TOML config files (#483)
## Summary - switch global config writes to `go-toml/v2` so auto-written TOML can include inline comments - route `roborev config set` global and local writes through typed save helpers so both `config.toml` and `.roborev.toml` get documented output - update `roborev init --agent ...` to create a commented repo config scaffold and add regression coverage for the global, local, and init paths ## Test Plan - go vet ./... - go test ./... Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 891b215 commit 5bc40b0

File tree

9 files changed

+291
-214
lines changed

9 files changed

+291
-214
lines changed

cmd/roborev/config_cmd.go

Lines changed: 23 additions & 123 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,9 @@ import (
55
"fmt"
66
"os"
77
"path/filepath"
8-
"reflect"
98
"strings"
109
"text/tabwriter"
1110

12-
"github.com/BurntSushi/toml"
1311
"github.com/roborev-dev/roborev/internal/config"
1412
"github.com/roborev-dev/roborev/internal/git"
1513
"github.com/spf13/cobra"
@@ -384,27 +382,39 @@ func printKeyValues(kvs []config.KeyValue) {
384382
}
385383
}
386384

387-
// setConfigKey sets a key in a TOML file using raw map manipulation
388-
// to avoid writing default values for every field.
389-
// isGlobal determines which struct (Config vs RepoConfig) validates the key.
385+
// setConfigKey sets a key in a TOML config file.
390386
func setConfigKey(path, key, value string, isGlobal bool) error {
391-
validationCfg, err := validateKeyForScope(key, value, isGlobal)
387+
_, err := validateKeyForScope(key, value, isGlobal)
392388
if err != nil {
393389
return err
394390
}
395391

396-
raw, err := loadRawConfig(path)
392+
if isGlobal {
393+
cfg, err := config.LoadGlobalFrom(path)
394+
if err != nil {
395+
return fmt.Errorf("parse %s: %w", path, err)
396+
}
397+
if err := config.SetConfigValue(cfg, key, value); err != nil {
398+
return err
399+
}
400+
return config.SaveGlobalTo(path, cfg)
401+
}
402+
403+
repoDir := filepath.Dir(path)
404+
repoCfg, err := config.LoadRepoConfig(repoDir)
397405
if err != nil {
406+
return fmt.Errorf("parse %s: %w", path, err)
407+
}
408+
if repoCfg == nil {
409+
repoCfg = &config.RepoConfig{}
410+
}
411+
if err := config.SetConfigValue(repoCfg, key, value); err != nil {
398412
return err
399413
}
400-
401-
setRawMapKey(raw, key, coerceValue(validationCfg, key, value))
402-
403-
return atomicWriteConfig(path, raw, isGlobal)
414+
return config.SaveRepoConfigTo(path, repoCfg)
404415
}
405416

406-
// validateKeyForScope validates a key against the appropriate config struct
407-
// and returns the populated struct for type coercion.
417+
// validateKeyForScope validates a key against the appropriate config scope.
408418
func validateKeyForScope(key, value string, isGlobal bool) (any, error) {
409419
if isGlobal {
410420
cfg := config.DefaultConfig()
@@ -429,72 +439,6 @@ func validateKeyForScope(key, value string, isGlobal bool) (any, error) {
429439
return repoCfg, nil
430440
}
431441

432-
// loadRawConfig loads an existing TOML file as a raw map, or returns
433-
// an empty map if the file doesn't exist.
434-
func loadRawConfig(path string) (map[string]any, error) {
435-
raw := make(map[string]any)
436-
if _, err := os.Stat(path); err == nil {
437-
if _, err := toml.DecodeFile(path, &raw); err != nil {
438-
return nil, fmt.Errorf("parse %s: %w", path, err)
439-
}
440-
}
441-
return raw, nil
442-
}
443-
444-
// atomicWriteConfig writes a config map to a TOML file atomically using
445-
// a temp file and rename. It creates parent directories as needed.
446-
func atomicWriteConfig(path string, raw map[string]any, isGlobal bool) error {
447-
// Ensure directory exists. Use restrictive perms for the global config dir
448-
// since it may contain secrets (API keys, DB credentials).
449-
dirMode := os.FileMode(0755)
450-
if isGlobal {
451-
dirMode = 0700
452-
}
453-
dir := filepath.Dir(path)
454-
if err := os.MkdirAll(dir, dirMode); err != nil {
455-
return err
456-
}
457-
// MkdirAll is a no-op for existing dirs. Tighten global config dir
458-
// in case it was created with a permissive umask.
459-
if isGlobal {
460-
if err := os.Chmod(dir, dirMode); err != nil {
461-
return err
462-
}
463-
}
464-
465-
// For global config, always enforce 0600 since it may contain secrets
466-
// (API keys, DB credentials). Don't inherit existing permissions — the file
467-
// may have been created manually with a permissive umask (0644).
468-
// For repo config, preserve existing permissions (may be tracked in git).
469-
var mode os.FileMode = 0644
470-
if isGlobal {
471-
mode = 0600
472-
} else if info, err := os.Stat(path); err == nil {
473-
mode = info.Mode()
474-
}
475-
476-
f, err := os.CreateTemp(filepath.Dir(path), ".roborev-config-*.toml")
477-
if err != nil {
478-
return err
479-
}
480-
tmpPath := f.Name()
481-
defer os.Remove(tmpPath)
482-
483-
if err := toml.NewEncoder(f).Encode(raw); err != nil {
484-
f.Close()
485-
return err
486-
}
487-
if err := f.Close(); err != nil {
488-
return err
489-
}
490-
491-
if err := os.Chmod(tmpPath, mode); err != nil {
492-
return err
493-
}
494-
495-
return os.Rename(tmpPath, path)
496-
}
497-
498442
// setRawMapKey sets a value in a nested map using dot-separated keys.
499443
func setRawMapKey(m map[string]any, key string, value any) {
500444
parts := strings.Split(key, ".")
@@ -525,47 +469,3 @@ func setRawMapKey(m map[string]any, key string, value any) {
525469

526470
current[parts[len(parts)-1]] = value
527471
}
528-
529-
// coerceValue uses the typed config struct to determine the correct TOML type
530-
// for the given key's value.
531-
func coerceValue(validationCfg any, key, rawVal string) any {
532-
v := reflect.ValueOf(validationCfg)
533-
if v.Kind() == reflect.Ptr {
534-
v = v.Elem()
535-
}
536-
field, err := config.FindFieldByTOMLKey(v, key)
537-
if err != nil {
538-
// Unreachable: key was already validated by SetConfigValue above.
539-
// Fall back to raw string to avoid panicking on impossible paths.
540-
return rawVal
541-
}
542-
543-
switch field.Kind() {
544-
case reflect.String:
545-
return rawVal
546-
case reflect.Bool:
547-
return field.Bool()
548-
case reflect.Int, reflect.Int64:
549-
return field.Int()
550-
case reflect.Slice:
551-
if field.Type().Elem().Kind() == reflect.String {
552-
result := make([]any, field.Len())
553-
for i := 0; i < field.Len(); i++ {
554-
result[i] = field.Index(i).String()
555-
}
556-
return result
557-
}
558-
return rawVal
559-
case reflect.Ptr:
560-
if field.IsNil() {
561-
return rawVal
562-
}
563-
elem := field.Elem()
564-
if elem.Kind() == reflect.Bool {
565-
return elem.Bool()
566-
}
567-
return rawVal
568-
default:
569-
return rawVal
570-
}
571-
}

cmd/roborev/config_cmd_test.go

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -425,6 +425,30 @@ func TestSetConfigKeyRepoConfig(t *testing.T) {
425425
assertConfigValue(t, path, "agent", "claude-code")
426426
}
427427

428+
func TestSetConfigKeyRepoConfigWritesComments(t *testing.T) {
429+
dir := t.TempDir()
430+
path := filepath.Join(dir, ".roborev.toml")
431+
432+
if err := setConfigKey(path, "agent", "claude-code", false); err != nil {
433+
t.Fatalf("setConfigKey repo: %v", err)
434+
}
435+
436+
data, err := os.ReadFile(path)
437+
if err != nil {
438+
t.Fatalf("read config: %v", err)
439+
}
440+
got := string(data)
441+
442+
for _, want := range []string{
443+
"# Default agent for this repo when no workflow-specific agent is set.\n",
444+
"agent = 'claude-code'",
445+
} {
446+
if !strings.Contains(got, want) {
447+
t.Fatalf("repo config missing %q:\n%s", want, got)
448+
}
449+
}
450+
}
451+
428452
func TestSetConfigKeyRepoConfigRejectsGlobalACPSettings(t *testing.T) {
429453
dir := t.TempDir()
430454
path := filepath.Join(dir, ".roborev.toml")
@@ -438,6 +462,30 @@ func TestSetConfigKeyRepoConfigRejectsGlobalACPSettings(t *testing.T) {
438462
}
439463
}
440464

465+
func TestSetConfigKeyGlobalWritesComments(t *testing.T) {
466+
path := setupConfigFile(t)
467+
468+
if err := setConfigKey(path, "default_agent", "codex", true); err != nil {
469+
t.Fatalf("setConfigKey: %v", err)
470+
}
471+
472+
data, err := os.ReadFile(path)
473+
if err != nil {
474+
t.Fatalf("read config: %v", err)
475+
}
476+
got := string(data)
477+
478+
for _, want := range []string{
479+
"# Default agent when no workflow-specific agent is set.\n",
480+
"default_agent = 'codex'",
481+
"# Hide closed reviews by default in the TUI queue.\n",
482+
} {
483+
if !strings.Contains(got, want) {
484+
t.Fatalf("global config missing %q:\n%s", want, got)
485+
}
486+
}
487+
}
488+
441489
func TestSetRawMapKey(t *testing.T) {
442490
tests := []struct {
443491
name string

cmd/roborev/hook_test.go

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -467,3 +467,40 @@ func TestInitNoDaemon(t *testing.T) {
467467
})
468468
}
469469
}
470+
471+
func TestInitNoDaemonWithAgentCreatesCommentedRepoConfig(t *testing.T) {
472+
if runtime.GOOS == "windows" {
473+
t.Skip("skipping on Windows due to shell script stubs")
474+
}
475+
476+
repo := initNoDaemonSetup(t)
477+
setupMockServer(t, func(w http.ResponseWriter, r *http.Request) {
478+
w.WriteHeader(200)
479+
})
480+
481+
output := captureStdout(t, func() {
482+
cmd := initCmd()
483+
cmd.SetArgs([]string{"--no-daemon", "--agent", "codex"})
484+
if err := cmd.Execute(); err != nil {
485+
t.Fatalf("init failed: %v", err)
486+
}
487+
})
488+
489+
if !strings.Contains(output, "Created ") || !strings.Contains(output, ".roborev.toml") {
490+
t.Fatalf("init output missing repo config creation message:\n%s", output)
491+
}
492+
493+
data, err := os.ReadFile(filepath.Join(repo.Root, ".roborev.toml"))
494+
if err != nil {
495+
t.Fatalf("read repo config: %v", err)
496+
}
497+
got := string(data)
498+
for _, want := range []string{
499+
"# Default agent for this repo when no workflow-specific agent is set.\n",
500+
"agent = 'codex'",
501+
} {
502+
if !strings.Contains(got, want) {
503+
t.Fatalf("repo config missing %q:\n%s", want, got)
504+
}
505+
}
506+
}

cmd/roborev/init_cmd.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,8 +56,8 @@ func initCmd() *cobra.Command {
5656
repoConfigPath := filepath.Join(root, ".roborev.toml")
5757
if agent != "" {
5858
if _, err := os.Stat(repoConfigPath); os.IsNotExist(err) {
59-
repoConfig := fmt.Sprintf("# roborev per-repo configuration\nagent = %q\n", agent)
60-
if err := os.WriteFile(repoConfigPath, []byte(repoConfig), 0644); err != nil {
59+
repoConfig := &config.RepoConfig{Agent: agent}
60+
if err := config.SaveRepoConfigTo(repoConfigPath, repoConfig); err != nil {
6161
return fmt.Errorf("create repo config: %w", err)
6262
}
6363
fmt.Printf(" Created %s\n", repoConfigPath)

flake.nix

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@
1919

2020
src = ./.;
2121

22-
vendorHash = "sha256-7meKrlC0b5N/TOrZJov/IQOs8FzhnNJ/3rnbDejtBKs=";
22+
vendorHash = "sha256-HRVNiDpEoEFQZ7oMhGL+oAcqGdng4B14nTMebtBzufk=";
2323

2424
subPackages = [ "cmd/roborev" ];
2525

go.mod

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ require (
1717
github.com/mattn/go-isatty v0.0.20
1818
github.com/mattn/go-runewidth v0.0.20
1919
github.com/muesli/termenv v0.16.0
20+
github.com/pelletier/go-toml/v2 v2.2.4
2021
github.com/sourcegraph/go-diff v0.7.0
2122
github.com/spf13/cobra v1.10.2
2223
github.com/stretchr/testify v1.11.1

go.sum

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,8 @@ github.com/muesli/termenv v0.16.0 h1:S5AlUN9dENB57rsbnkPyfdGuWIlkmzJjbFf0Tf5FWUc
101101
github.com/muesli/termenv v0.16.0/go.mod h1:ZRfOIKPFDYQoDFF4Olj7/QJbW60Ol/kL1pU3VfY/Cnk=
102102
github.com/ncruces/go-strftime v1.0.0 h1:HMFp8mLCTPp341M/ZnA4qaf7ZlsbTc+miZjCLOFAw7w=
103103
github.com/ncruces/go-strftime v1.0.0/go.mod h1:Fwc5htZGVVkseilnfgOVb9mKy6w1naJmn9CehxcKcls=
104+
github.com/pelletier/go-toml/v2 v2.2.4 h1:mye9XuhQ6gvn5h28+VilKrrPoQVanw5PMw/TB0t5Ec4=
105+
github.com/pelletier/go-toml/v2 v2.2.4/go.mod h1:2gIqNv+qfxSVS7cM2xJQKtLSTLUE9V8t9Stt+h56mCY=
104106
github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
105107
github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
106108
github.com/remyoudompheng/bigfft v0.0.0-20230129092748-24d4a6f8daec h1:W09IVJc94icq4NjY3clb7Lk8O1qJ8BdBEF8z0ibU0rE=

0 commit comments

Comments
 (0)