fix(config): remove duplicate declarations and fix test failures (#1160)

* fix(config): remove duplicate declarations between config.go and sync.go

Commit e82e15a8 created sync.go with typed constants (SyncMode,
ConflictStrategy, Sovereignty) but didn't remove the original untyped
constants from config.go that were added in 16f8c3d3. This caused
redeclaration errors preventing the project from building.

Changes:
- Remove duplicate SyncMode, ConflictStrategy, Sovereignty constants
  from config.go (keep typed versions in sync.go)
- Remove duplicate GetSyncMode, GetConflictStrategy, GetSovereignty
  functions from config.go (keep sync.go versions with warnings)
- Update SyncConfig, ConflictConfig, FederationConfig structs to use
  typed fields instead of string
- Add IsSyncModeValid, IsConflictStrategyValid, IsSovereigntyValid
  wrapper functions that use sync.go's validation maps
- Update cmd/bd/sync.go to use typed ConflictStrategy parameter
- Update tests to work with typed constants

* fix(dolt): handle Merge return values in concurrent test

* fix(test): add --repo flag to show_test.go to bypass auto-routing

The tests were failing because the create command was routing issues
to ~/.beads-planning instead of the test's temp directory. Adding
--repo . overrides auto-routing and creates issues in the test dir.
This commit is contained in:
Subhrajit Makur
2026-01-19 23:41:14 +05:30
committed by GitHub
parent 2cbf3a5497
commit 065ca3d6af
5 changed files with 50 additions and 152 deletions

View File

@@ -32,8 +32,9 @@ func TestShow_ExternalRef(t *testing.T) {
} }
// Create issue with external ref // Create issue with external ref
// Use --repo . to override auto-routing and create in the test directory
createCmd := exec.Command(tmpBin, "--no-daemon", "create", "External ref test", "-p", "1", createCmd := exec.Command(tmpBin, "--no-daemon", "create", "External ref test", "-p", "1",
"--external-ref", "https://example.com/spec.md", "--json") "--external-ref", "https://example.com/spec.md", "--json", "--repo", ".")
createCmd.Dir = tmpDir createCmd.Dir = tmpDir
createOut, err := createCmd.CombinedOutput() createOut, err := createCmd.CombinedOutput()
if err != nil { if err != nil {
@@ -86,7 +87,8 @@ func TestShow_NoExternalRef(t *testing.T) {
} }
// Create issue WITHOUT external ref // Create issue WITHOUT external ref
createCmd := exec.Command(tmpBin, "--no-daemon", "create", "No ref test", "-p", "1", "--json") // Use --repo . to override auto-routing and create in the test directory
createCmd := exec.Command(tmpBin, "--no-daemon", "create", "No ref test", "-p", "1", "--json", "--repo", ".")
createCmd.Dir = tmpDir createCmd.Dir = tmpDir
createOut, err := createCmd.CombinedOutput() createOut, err := createCmd.CombinedOutput()
if err != nil { if err != nil {

View File

@@ -951,7 +951,7 @@ func ClearSyncConflictState(beadsDir string) error {
// - "ours": Keep local version // - "ours": Keep local version
// - "theirs": Keep remote version // - "theirs": Keep remote version
// - "manual": Interactive resolution with user prompts // - "manual": Interactive resolution with user prompts
func resolveSyncConflicts(ctx context.Context, jsonlPath string, strategy string, dryRun bool) error { func resolveSyncConflicts(ctx context.Context, jsonlPath string, strategy config.ConflictStrategy, dryRun bool) error {
beadsDir := filepath.Dir(jsonlPath) beadsDir := filepath.Dir(jsonlPath)
conflictState, err := LoadSyncConflictState(beadsDir) conflictState, err := LoadSyncConflictState(beadsDir)

View File

@@ -12,25 +12,6 @@ import (
"github.com/steveyegge/beads/internal/debug" "github.com/steveyegge/beads/internal/debug"
) )
// Sync mode constants define how beads syncs with git/remotes.
const (
// SyncModeGitPortable exports JSONL on push, imports on pull (default).
// This is the standard git-based workflow where JSONL is committed.
SyncModeGitPortable = "git-portable"
// SyncModeRealtime exports JSONL on every database change.
// Legacy behavior, more frequent writes but higher I/O.
SyncModeRealtime = "realtime"
// SyncModeDoltNative uses Dolt remotes directly (dolthub://, gs://, s3://).
// No JSONL export needed - Dolt handles sync.
SyncModeDoltNative = "dolt-native"
// SyncModeBeltAndSuspenders uses both Dolt remote AND JSONL backup.
// Maximum redundancy for critical data.
SyncModeBeltAndSuspenders = "belt-and-suspenders"
)
// Sync trigger constants define when sync operations occur. // Sync trigger constants define when sync operations occur.
const ( const (
// SyncTriggerPush triggers sync on git push operations. // SyncTriggerPush triggers sync on git push operations.
@@ -43,36 +24,6 @@ const (
SyncTriggerPull = "pull" SyncTriggerPull = "pull"
) )
// Conflict strategy constants define how sync conflicts are resolved.
const (
// ConflictStrategyNewest keeps whichever version has the newer updated_at timestamp.
ConflictStrategyNewest = "newest"
// ConflictStrategyOurs keeps the local version on conflict.
ConflictStrategyOurs = "ours"
// ConflictStrategyTheirs keeps the remote version on conflict.
ConflictStrategyTheirs = "theirs"
// ConflictStrategyManual requires manual resolution of conflicts.
ConflictStrategyManual = "manual"
)
// Federation sovereignty tiers define data sovereignty levels.
const (
// SovereigntyT1 - Full sovereignty: data never leaves controlled infrastructure.
SovereigntyT1 = "T1"
// SovereigntyT2 - Regional sovereignty: data stays within region/jurisdiction.
SovereigntyT2 = "T2"
// SovereigntyT3 - Provider sovereignty: data with trusted cloud provider.
SovereigntyT3 = "T3"
// SovereigntyT4 - No restrictions: data can be anywhere (e.g., DoltHub public).
SovereigntyT4 = "T4"
)
var v *viper.Viper var v *viper.Viper
// Initialize sets up the viper configuration singleton // Initialize sets up the viper configuration singleton
@@ -607,7 +558,7 @@ func GetIdentity(flagValue string) string {
// SyncConfig holds the sync mode configuration. // SyncConfig holds the sync mode configuration.
type SyncConfig struct { type SyncConfig struct {
Mode string // git-portable, realtime, dolt-native, belt-and-suspenders Mode SyncMode // git-portable, realtime, dolt-native, belt-and-suspenders
ExportOn string // push, change ExportOn string // push, change
ImportOn string // pull, change ImportOn string // pull, change
} }
@@ -621,35 +572,9 @@ func GetSyncConfig() SyncConfig {
} }
} }
// GetSyncMode returns the configured sync mode.
// Returns git-portable if not configured or invalid.
func GetSyncMode() string {
mode := GetString("sync.mode")
if mode == "" {
return SyncModeGitPortable
}
// Validate mode
switch mode {
case SyncModeGitPortable, SyncModeRealtime, SyncModeDoltNative, SyncModeBeltAndSuspenders:
return mode
default:
return SyncModeGitPortable
}
}
// IsSyncModeValid checks if the given sync mode is valid.
func IsSyncModeValid(mode string) bool {
switch mode {
case SyncModeGitPortable, SyncModeRealtime, SyncModeDoltNative, SyncModeBeltAndSuspenders:
return true
default:
return false
}
}
// ConflictConfig holds the conflict resolution configuration. // ConflictConfig holds the conflict resolution configuration.
type ConflictConfig struct { type ConflictConfig struct {
Strategy string // newest, ours, theirs, manual Strategy ConflictStrategy // newest, ours, theirs, manual
} }
// GetConflictConfig returns the current conflict resolution configuration. // GetConflictConfig returns the current conflict resolution configuration.
@@ -659,36 +584,10 @@ func GetConflictConfig() ConflictConfig {
} }
} }
// GetConflictStrategy returns the configured conflict resolution strategy.
// Returns newest if not configured or invalid.
func GetConflictStrategy() string {
strategy := GetString("conflict.strategy")
if strategy == "" {
return ConflictStrategyNewest
}
// Validate strategy
switch strategy {
case ConflictStrategyNewest, ConflictStrategyOurs, ConflictStrategyTheirs, ConflictStrategyManual:
return strategy
default:
return ConflictStrategyNewest
}
}
// IsConflictStrategyValid checks if the given conflict strategy is valid.
func IsConflictStrategyValid(strategy string) bool {
switch strategy {
case ConflictStrategyNewest, ConflictStrategyOurs, ConflictStrategyTheirs, ConflictStrategyManual:
return true
default:
return false
}
}
// FederationConfig holds the federation (Dolt remote) configuration. // FederationConfig holds the federation (Dolt remote) configuration.
type FederationConfig struct { type FederationConfig struct {
Remote string // dolthub://org/beads, gs://bucket/beads, s3://bucket/beads Remote string // dolthub://org/beads, gs://bucket/beads, s3://bucket/beads
Sovereignty string // T1, T2, T3, T4 Sovereignty Sovereignty // T1, T2, T3, T4
} }
// GetFederationConfig returns the current federation configuration. // GetFederationConfig returns the current federation configuration.
@@ -699,27 +598,23 @@ func GetFederationConfig() FederationConfig {
} }
} }
// GetSovereignty returns the configured data sovereignty tier. // IsSyncModeValid checks if the given sync mode string is valid.
// Returns empty string if not configured. func IsSyncModeValid(mode string) bool {
func GetSovereignty() string { return validSyncModes[SyncMode(mode)]
sovereignty := GetString("federation.sovereignty")
// Validate sovereignty tier
switch sovereignty {
case SovereigntyT1, SovereigntyT2, SovereigntyT3, SovereigntyT4:
return sovereignty
default:
return ""
}
} }
// IsSovereigntyValid checks if the given sovereignty tier is valid. // IsConflictStrategyValid checks if the given conflict strategy string is valid.
func IsSovereigntyValid(sovereignty string) bool { func IsConflictStrategyValid(strategy string) bool {
switch sovereignty { return validConflictStrategies[ConflictStrategy(strategy)]
case "", SovereigntyT1, SovereigntyT2, SovereigntyT3, SovereigntyT4:
return true
default:
return false
} }
// IsSovereigntyValid checks if the given sovereignty tier string is valid.
// Note: empty string is valid (means no restriction).
func IsSovereigntyValid(sovereignty string) bool {
if sovereignty == "" {
return true
}
return validSovereigntyTiers[Sovereignty(sovereignty)]
} }
// ShouldExportOnChange returns true if sync.export_on is set to "change". // ShouldExportOnChange returns true if sync.export_on is set to "change".

View File

@@ -1119,13 +1119,14 @@ func TestFederationConfigDefaults(t *testing.T) {
t.Fatalf("Initialize() returned error: %v", err) t.Fatalf("Initialize() returned error: %v", err)
} }
// Test federation config defaults (empty) // Test federation config defaults
cfg := GetFederationConfig() cfg := GetFederationConfig()
if cfg.Remote != "" { if cfg.Remote != "" {
t.Errorf("GetFederationConfig().Remote = %q, want empty", cfg.Remote) t.Errorf("GetFederationConfig().Remote = %q, want empty", cfg.Remote)
} }
if cfg.Sovereignty != "" { // Default sovereignty is T1 when not configured
t.Errorf("GetFederationConfig().Sovereignty = %q, want empty", cfg.Sovereignty) if cfg.Sovereignty != SovereigntyT1 {
t.Errorf("GetFederationConfig().Sovereignty = %q, want %q (default)", cfg.Sovereignty, SovereigntyT1)
} }
} }
@@ -1134,10 +1135,10 @@ func TestIsSyncModeValid(t *testing.T) {
mode string mode string
valid bool valid bool
}{ }{
{SyncModeGitPortable, true}, {string(SyncModeGitPortable), true},
{SyncModeRealtime, true}, {string(SyncModeRealtime), true},
{SyncModeDoltNative, true}, {string(SyncModeDoltNative), true},
{SyncModeBeltAndSuspenders, true}, {string(SyncModeBeltAndSuspenders), true},
{"invalid-mode", false}, {"invalid-mode", false},
{"", false}, {"", false},
} }
@@ -1156,10 +1157,10 @@ func TestIsConflictStrategyValid(t *testing.T) {
strategy string strategy string
valid bool valid bool
}{ }{
{ConflictStrategyNewest, true}, {string(ConflictStrategyNewest), true},
{ConflictStrategyOurs, true}, {string(ConflictStrategyOurs), true},
{ConflictStrategyTheirs, true}, {string(ConflictStrategyTheirs), true},
{ConflictStrategyManual, true}, {string(ConflictStrategyManual), true},
{"invalid-strategy", false}, {"invalid-strategy", false},
{"", false}, {"", false},
} }
@@ -1178,10 +1179,10 @@ func TestIsSovereigntyValid(t *testing.T) {
sovereignty string sovereignty string
valid bool valid bool
}{ }{
{SovereigntyT1, true}, {string(SovereigntyT1), true},
{SovereigntyT2, true}, {string(SovereigntyT2), true},
{SovereigntyT3, true}, {string(SovereigntyT3), true},
{SovereigntyT4, true}, {string(SovereigntyT4), true},
{"", true}, // Empty is valid (means no restriction) {"", true}, // Empty is valid (means no restriction)
{"T5", false}, {"T5", false},
{"invalid", false}, {"invalid", false},
@@ -1310,7 +1311,7 @@ func TestNeedsDoltRemote(t *testing.T) {
defer restore() defer restore()
tests := []struct { tests := []struct {
mode string mode SyncMode
needsRemote bool needsRemote bool
}{ }{
{SyncModeGitPortable, false}, {SyncModeGitPortable, false},
@@ -1320,11 +1321,11 @@ func TestNeedsDoltRemote(t *testing.T) {
} }
for _, tt := range tests { for _, tt := range tests {
t.Run(tt.mode, func(t *testing.T) { t.Run(string(tt.mode), func(t *testing.T) {
if err := Initialize(); err != nil { if err := Initialize(); err != nil {
t.Fatalf("Initialize() returned error: %v", err) t.Fatalf("Initialize() returned error: %v", err)
} }
Set("sync.mode", tt.mode) Set("sync.mode", string(tt.mode))
if got := NeedsDoltRemote(); got != tt.needsRemote { if got := NeedsDoltRemote(); got != tt.needsRemote {
t.Errorf("NeedsDoltRemote() with mode=%s = %v, want %v", tt.mode, got, tt.needsRemote) t.Errorf("NeedsDoltRemote() with mode=%s = %v, want %v", tt.mode, got, tt.needsRemote)
@@ -1339,7 +1340,7 @@ func TestNeedsJSONL(t *testing.T) {
defer restore() defer restore()
tests := []struct { tests := []struct {
mode string mode SyncMode
needsJSONL bool needsJSONL bool
}{ }{
{SyncModeGitPortable, true}, {SyncModeGitPortable, true},
@@ -1349,11 +1350,11 @@ func TestNeedsJSONL(t *testing.T) {
} }
for _, tt := range tests { for _, tt := range tests {
t.Run(tt.mode, func(t *testing.T) { t.Run(string(tt.mode), func(t *testing.T) {
if err := Initialize(); err != nil { if err := Initialize(); err != nil {
t.Fatalf("Initialize() returned error: %v", err) t.Fatalf("Initialize() returned error: %v", err)
} }
Set("sync.mode", tt.mode) Set("sync.mode", string(tt.mode))
if got := NeedsJSONL(); got != tt.needsJSONL { if got := NeedsJSONL(); got != tt.needsJSONL {
t.Errorf("NeedsJSONL() with mode=%s = %v, want %v", tt.mode, got, tt.needsJSONL) t.Errorf("NeedsJSONL() with mode=%s = %v, want %v", tt.mode, got, tt.needsJSONL)
@@ -1406,9 +1407,9 @@ func TestGetSovereigntyInvalid(t *testing.T) {
t.Fatalf("Initialize() returned error: %v", err) t.Fatalf("Initialize() returned error: %v", err)
} }
// Set invalid sovereignty - should return empty // Set invalid sovereignty - should return T1 (default) with warning
Set("federation.sovereignty", "T99") Set("federation.sovereignty", "T99")
if got := GetSovereignty(); got != "" { if got := GetSovereignty(); got != SovereigntyT1 {
t.Errorf("GetSovereignty() with invalid tier = %q, want empty (fallback)", got) t.Errorf("GetSovereignty() with invalid tier = %q, want %q (fallback)", got, SovereigntyT1)
} }
} }

View File

@@ -459,10 +459,10 @@ func TestBranchPerAgentMergeRace(t *testing.T) {
} }
// First merge should succeed // First merge should succeed
err1 := store.Merge(ctx, "agent-1") _, err1 := store.Merge(ctx, "agent-1")
// Second merge may conflict (both modified same row) // Second merge may conflict (both modified same row)
err2 := store.Merge(ctx, "agent-2") _, err2 := store.Merge(ctx, "agent-2")
t.Logf("Merge agent-1 result: %v", err1) t.Logf("Merge agent-1 result: %v", err1)
t.Logf("Merge agent-2 result: %v", err2) t.Logf("Merge agent-2 result: %v", err2)