fix(daemon): Use syncbranch.IsConfigured() for autoPull (#837)
* test(daemon): Prove autoPull reads sync.branch from SQLite - Verify daemon reads sync.branch from SQLite config source - Show config.yaml sync-branch is ignored (YAML-only key) - Add skipped tracer bullet test for issue #831 Coverage: daemon autoPull config resolution * fix(daemon): Use syncbranch.IsConfigured() for autoPull Problem: - Daemon's periodic sync never activated when sync-branch configured in config.yaml - autoPull check only read sync.branch from SQLite, missing YAML-only configuration Solution: - Replace SQLite-only check with syncbranch.IsConfigured() - Update test to validate correct detection of YAML-configured sync branch Impact: - Periodic sync now activates correctly when sync-branch is configured in config.yaml - Fixes daemon hanging issue for users with YAML-only configuration
This commit is contained in:
committed by
GitHub
parent
a1f706d17a
commit
9801102aa2
@@ -17,6 +17,7 @@ import (
|
||||
"github.com/steveyegge/beads/internal/daemon"
|
||||
"github.com/steveyegge/beads/internal/rpc"
|
||||
"github.com/steveyegge/beads/internal/storage/sqlite"
|
||||
"github.com/steveyegge/beads/internal/syncbranch"
|
||||
)
|
||||
|
||||
var daemonCmd = &cobra.Command{
|
||||
@@ -108,8 +109,10 @@ Run 'bd daemon' with no flags to see available options.`,
|
||||
autoPull = false
|
||||
}
|
||||
} else {
|
||||
// Default: auto_pull is true when sync.branch is configured
|
||||
if syncBranch, err := store.GetConfig(ctx, "sync.branch"); err == nil && syncBranch != "" {
|
||||
// Default: auto_pull is true when sync-branch is configured
|
||||
// Use syncbranch.IsConfigured() which checks env var and config.yaml
|
||||
// (the common case), not just SQLite (legacy)
|
||||
if syncbranch.IsConfigured() {
|
||||
autoPull = true
|
||||
}
|
||||
}
|
||||
|
||||
148
cmd/bd/daemon_autopull_yaml_test.go
Normal file
148
cmd/bd/daemon_autopull_yaml_test.go
Normal file
@@ -0,0 +1,148 @@
|
||||
package main
|
||||
|
||||
import (
|
||||
"context"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"testing"
|
||||
|
||||
"github.com/steveyegge/beads/internal/config"
|
||||
"github.com/steveyegge/beads/internal/storage/sqlite"
|
||||
"github.com/steveyegge/beads/internal/syncbranch"
|
||||
)
|
||||
|
||||
// TestAutoPullDefaultFromYamlConfig verifies that autoPull defaults to true
|
||||
// when sync-branch is configured in config.yaml (not just SQLite).
|
||||
//
|
||||
// This test validates the fix for the bug where daemon.go:111-114 only checked
|
||||
// store.GetConfig("sync.branch") (SQLite), but sync-branch is typically set
|
||||
// in config.yaml (read via viper).
|
||||
//
|
||||
// Fix: daemon.go now uses syncbranch.IsConfigured() which checks env var and
|
||||
// config.yaml (the common case), providing correct autoPull behavior.
|
||||
func TestAutoPullDefaultFromYamlConfig(t *testing.T) {
|
||||
// Create temp directory structure
|
||||
tmpDir := t.TempDir()
|
||||
beadsDir := filepath.Join(tmpDir, ".beads")
|
||||
if err := os.MkdirAll(beadsDir, 0755); err != nil {
|
||||
t.Fatalf("Failed to create .beads directory: %v", err)
|
||||
}
|
||||
|
||||
// Create config.yaml with sync-branch set (the user's configuration)
|
||||
configYAML := `# Beads configuration
|
||||
sync-branch: beads-sync
|
||||
issue-prefix: test
|
||||
`
|
||||
configPath := filepath.Join(beadsDir, "config.yaml")
|
||||
if err := os.WriteFile(configPath, []byte(configYAML), 0644); err != nil {
|
||||
t.Fatalf("Failed to write config.yaml: %v", err)
|
||||
}
|
||||
|
||||
// Create a database WITHOUT sync.branch in the config table
|
||||
dbPath := filepath.Join(beadsDir, "beads.db")
|
||||
ctx := context.Background()
|
||||
testStore, err := sqlite.New(ctx, dbPath)
|
||||
if err != nil {
|
||||
t.Fatalf("Failed to create test database: %v", err)
|
||||
}
|
||||
defer testStore.Close()
|
||||
|
||||
// Verify: sync.branch is NOT set in SQLite
|
||||
dbSyncBranch, _ := testStore.GetConfig(ctx, "sync.branch")
|
||||
if dbSyncBranch != "" {
|
||||
t.Fatalf("Expected no sync.branch in database, got %q", dbSyncBranch)
|
||||
}
|
||||
|
||||
// Reinitialize config package to read from our test directory
|
||||
// This is what happens when bd daemon starts and reads config
|
||||
// Use t.Chdir which automatically restores the original directory on test cleanup
|
||||
t.Chdir(tmpDir)
|
||||
|
||||
// Reinitialize viper config to pick up the new config.yaml
|
||||
if err := config.Initialize(); err != nil {
|
||||
t.Fatalf("Failed to initialize config: %v", err)
|
||||
}
|
||||
|
||||
// Verify: sync.branch is NOT set in SQLite (the scenario we're testing)
|
||||
t.Logf("sync-branch in config.yaml: beads-sync")
|
||||
t.Logf("sync.branch in SQLite: %q (expected empty)", dbSyncBranch)
|
||||
|
||||
// The fix: daemon.go uses syncbranch.IsConfigured() which checks env var and
|
||||
// config.yaml, not just SQLite. This should return true.
|
||||
autoPull := syncbranch.IsConfigured()
|
||||
t.Logf("syncbranch.IsConfigured() = %v", autoPull)
|
||||
|
||||
if !autoPull {
|
||||
t.Errorf("Expected syncbranch.IsConfigured()=true when sync-branch is in config.yaml, got false")
|
||||
}
|
||||
}
|
||||
|
||||
// TestAutoPullDefaultFromEnvVar verifies that env var override works correctly.
|
||||
// This test should PASS because the env var is checked first.
|
||||
func TestAutoPullDefaultFromEnvVar(t *testing.T) {
|
||||
tmpDir := t.TempDir()
|
||||
beadsDir := filepath.Join(tmpDir, ".beads")
|
||||
if err := os.MkdirAll(beadsDir, 0755); err != nil {
|
||||
t.Fatalf("Failed to create .beads directory: %v", err)
|
||||
}
|
||||
|
||||
// Create database without sync.branch
|
||||
dbPath := filepath.Join(beadsDir, "beads.db")
|
||||
ctx := context.Background()
|
||||
testStore, err := sqlite.New(ctx, dbPath)
|
||||
if err != nil {
|
||||
t.Fatalf("Failed to create test database: %v", err)
|
||||
}
|
||||
defer testStore.Close()
|
||||
|
||||
// Set env var (highest priority)
|
||||
t.Setenv("BEADS_SYNC_BRANCH", "env-sync-branch")
|
||||
|
||||
// The daemon DOES check env var (line 97-98), so this path works
|
||||
// But it's in a separate code path from the YAML bug
|
||||
autoPullFromEnv := syncbranch.IsConfigured()
|
||||
|
||||
if !autoPullFromEnv {
|
||||
t.Errorf("Expected autoPull=true when BEADS_SYNC_BRANCH is set, got false")
|
||||
}
|
||||
}
|
||||
|
||||
// TestAutoPullDefaultFromSQLite verifies the legacy SQLite path still works.
|
||||
// This test should PASS because the SQLite path works (it's just not used by config.yaml users).
|
||||
func TestAutoPullDefaultFromSQLite(t *testing.T) {
|
||||
tmpDir := t.TempDir()
|
||||
beadsDir := filepath.Join(tmpDir, ".beads")
|
||||
if err := os.MkdirAll(beadsDir, 0755); err != nil {
|
||||
t.Fatalf("Failed to create .beads directory: %v", err)
|
||||
}
|
||||
|
||||
// Create database WITH sync.branch
|
||||
dbPath := filepath.Join(beadsDir, "beads.db")
|
||||
ctx := context.Background()
|
||||
testStore, err := sqlite.New(ctx, dbPath)
|
||||
if err != nil {
|
||||
t.Fatalf("Failed to create test database: %v", err)
|
||||
}
|
||||
defer testStore.Close()
|
||||
|
||||
// Set sync.branch in SQLite (legacy configuration)
|
||||
if err := testStore.SetConfig(ctx, "sync.branch", "sqlite-sync-branch"); err != nil {
|
||||
t.Fatalf("Failed to set sync.branch in database: %v", err)
|
||||
}
|
||||
|
||||
// The current daemon code should work for this case
|
||||
var autoPullFromDB bool
|
||||
if syncBranch, err := testStore.GetConfig(ctx, "sync.branch"); err == nil && syncBranch != "" {
|
||||
autoPullFromDB = true
|
||||
}
|
||||
|
||||
if !autoPullFromDB {
|
||||
t.Errorf("Expected autoPull=true when sync.branch is in SQLite, got false")
|
||||
}
|
||||
|
||||
// Verify syncbranch.IsConfiguredWithDB also works
|
||||
autoPullFromHelper := syncbranch.IsConfiguredWithDB(dbPath)
|
||||
if !autoPullFromHelper {
|
||||
t.Errorf("Expected IsConfiguredWithDB=true when sync.branch is in SQLite, got false")
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user