Skip to content

Commit e585a61

Browse files
osamu2001osamu2001
authored
fix: avoid false shared-server bootstrap no-op for synthesized worktrees
Co-authored-by: Okano, Osamu <19066+osamu2001@users.noreply.github.com>
1 parent 04ec3f5 commit e585a61

4 files changed

Lines changed: 335 additions & 51 deletions

File tree

‎cmd/bd/bootstrap.go‎

Lines changed: 142 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"fmt"
88
"os"
99
"path/filepath"
10+
"strconv"
1011
"strings"
1112
"time"
1213

@@ -228,52 +229,23 @@ func detectBootstrapAction(beadsDir string, cfg *configfile.Config) BootstrapPla
228229
Database: cfg.GetDoltDatabase(),
229230
}
230231

231-
// Check for existing database (path differs between server and embedded mode).
232-
// Use cfg.IsDoltServerMode() (which checks metadata.json + env vars) plus
233-
// doltserver.IsSharedServerMode() (which also checks config.yaml) so that
234-
// shared-server mode configured via dolt.shared-server: true in config.yaml
235-
// correctly resolves the database path. (GH#30)
236-
isServer := cfg.IsDoltServerMode() || doltserver.IsSharedServerMode()
237-
var dbPath string
238-
if isServer {
239-
dbPath = doltserver.ResolveDoltDir(beadsDir)
240-
} else {
241-
dbPath = filepath.Join(beadsDir, "embeddeddolt")
242-
}
243-
if info, err := os.Stat(dbPath); err == nil && info.IsDir() {
244-
entries, _ := os.ReadDir(dbPath)
245-
if len(entries) > 0 {
246-
if isServer {
247-
resolved := doltserver.DefaultConfig(beadsDir)
248-
probeCfg := bootstrapServerProbeConfig{
249-
host: cfg.GetDoltServerHost(),
250-
port: resolved.Port,
251-
user: cfg.GetDoltServerUser(),
252-
pass: cfg.GetDoltServerPassword(),
253-
database: cfg.GetDoltDatabase(),
254-
tls: cfg.GetDoltServerTLS(),
255-
}
256-
result := checkBootstrapServerDB(probeCfg)
257-
if result.Err != nil {
258-
plan.Action = "none"
259-
plan.Reason = fmt.Sprintf("Could not verify existing server database %s: %v", cfg.GetDoltDatabase(), result.Err)
260-
return plan
261-
}
262-
if result.Exists {
263-
plan.HasExisting = true
264-
plan.Action = "none"
265-
plan.Reason = fmt.Sprintf("Database %s already exists on server at %s:%d", probeCfg.database, probeCfg.host, probeCfg.port)
266-
return plan
267-
}
268-
} else {
269-
plan.HasExisting = true
270-
plan.Action = "none"
271-
plan.Reason = "Database already exists at " + dbPath
272-
return plan
273-
}
274-
}
232+
// When bootstrap synthesized a fallback beadsDir for a fresh clone or
233+
// worktree recovery, the path may not exist yet. In that case we must let
234+
// sync.remote / refs/dolt/data detection run before treating an existing
235+
// shared-server database as "nothing to do", otherwise an unrelated default
236+
// "beads" database can mask the real recovery path.
237+
beadsDirExists := false
238+
if info, err := os.Stat(beadsDir); err == nil && info.IsDir() {
239+
beadsDirExists = true
275240
}
276241

242+
// Check for existing database (path differs between server and embedded mode).
243+
// Determine server/shared-server mode from the target workspace itself
244+
// (metadata.json, env vars, and the target config.yaml when present) rather
245+
// than unrelated global config loaded from the caller's current repo.
246+
isSharedServer := bootstrapSharedServerMode(beadsDir)
247+
isServer := cfg.IsDoltServerMode() || isSharedServer
248+
277249
// Check sync.remote (primary) or sync.git-remote (deprecated fallback)
278250
syncRemote := resolveSyncRemote()
279251
if syncRemote != "" {
@@ -297,6 +269,20 @@ func detectBootstrapAction(beadsDir string, cfg *configfile.Config) BootstrapPla
297269
}
298270
}
299271

272+
if dbAction, ok := existingBootstrapDBPlan(beadsDir, cfg, isServer, isSharedServer); ok {
273+
// If the local beadsDir does not exist yet, prefer recovering via sync
274+
// first. This avoids false "nothing to do" results when the default
275+
// shared-server database name happens to exist for another project.
276+
if beadsDirExists || dbAction.Action != "none" {
277+
return dbAction
278+
}
279+
// For synthesized paths with no local workspace directory yet, defer the
280+
// existing-db no-op until we've ruled out all other recovery paths.
281+
// This preserves the sync-precedence fix without downgrading the
282+
// legitimate "database already exists" case into a fresh init.
283+
plan = dbAction
284+
}
285+
300286
// Check for backup JSONL files (must be non-empty to be useful)
301287
backupDir := filepath.Join(beadsDir, "backup")
302288
issuesFile := filepath.Join(backupDir, "issues.jsonl")
@@ -316,12 +302,124 @@ func detectBootstrapAction(beadsDir string, cfg *configfile.Config) BootstrapPla
316302
return plan
317303
}
318304

305+
if plan.Action == "none" {
306+
return plan
307+
}
308+
319309
// Fresh setup
320310
plan.Action = "init"
321311
plan.Reason = "No existing database, remote, or backup — will create fresh database"
322312
return plan
323313
}
324314

315+
func existingBootstrapDBPlan(beadsDir string, cfg *configfile.Config, isServer, isSharedServer bool) (BootstrapPlan, bool) {
316+
plan := BootstrapPlan{
317+
BeadsDir: beadsDir,
318+
Database: cfg.GetDoltDatabase(),
319+
}
320+
321+
var dbPath string
322+
if isServer {
323+
dbPath = bootstrapServerDoltDir(beadsDir, cfg, isSharedServer)
324+
} else {
325+
dbPath = filepath.Join(beadsDir, "embeddeddolt")
326+
}
327+
if info, err := os.Stat(dbPath); err != nil || !info.IsDir() {
328+
return BootstrapPlan{}, false
329+
}
330+
331+
entries, _ := os.ReadDir(dbPath)
332+
if len(entries) == 0 {
333+
return BootstrapPlan{}, false
334+
}
335+
336+
if isServer {
337+
probeCfg := bootstrapServerProbeConfig{
338+
host: cfg.GetDoltServerHost(),
339+
port: bootstrapServerPort(beadsDir, cfg, isSharedServer),
340+
user: cfg.GetDoltServerUser(),
341+
pass: cfg.GetDoltServerPassword(),
342+
database: cfg.GetDoltDatabase(),
343+
tls: cfg.GetDoltServerTLS(),
344+
}
345+
result := checkBootstrapServerDB(probeCfg)
346+
if result.Err != nil {
347+
plan.Action = "none"
348+
plan.Reason = fmt.Sprintf("Could not verify existing server database %s: %v", cfg.GetDoltDatabase(), result.Err)
349+
return plan, true
350+
}
351+
if result.Exists {
352+
plan.HasExisting = true
353+
plan.Action = "none"
354+
plan.Reason = fmt.Sprintf("Database %s already exists on server at %s:%d", probeCfg.database, probeCfg.host, probeCfg.port)
355+
return plan, true
356+
}
357+
return BootstrapPlan{}, false
358+
}
359+
360+
plan.HasExisting = true
361+
plan.Action = "none"
362+
plan.Reason = "Database already exists at " + dbPath
363+
return plan, true
364+
}
365+
366+
func bootstrapSharedServerMode(beadsDir string) bool {
367+
if v := os.Getenv("BEADS_DOLT_SHARED_SERVER"); v == "1" || strings.EqualFold(v, "true") {
368+
return true
369+
}
370+
return strings.EqualFold(config.GetStringFromDir(beadsDir, "dolt.shared-server"), "true")
371+
}
372+
373+
func bootstrapServerDoltDir(beadsDir string, cfg *configfile.Config, isSharedServer bool) string {
374+
if isSharedServer {
375+
if dir, err := doltserver.SharedDoltDir(); err == nil {
376+
return dir
377+
}
378+
}
379+
380+
if d := cfg.GetDoltDataDir(); d != "" {
381+
if filepath.IsAbs(d) {
382+
return d
383+
}
384+
return filepath.Join(beadsDir, d)
385+
}
386+
387+
return filepath.Join(beadsDir, "dolt")
388+
}
389+
390+
func bootstrapServerPort(beadsDir string, cfg *configfile.Config, isSharedServer bool) int {
391+
if p := os.Getenv("BEADS_DOLT_SERVER_PORT"); p != "" {
392+
if port, err := strconv.Atoi(p); err == nil && port > 0 {
393+
return port
394+
}
395+
}
396+
397+
if isSharedServer {
398+
if sharedDir, err := doltserver.SharedServerDir(); err == nil {
399+
if port := doltserver.ReadPortFile(sharedDir); port > 0 {
400+
return port
401+
}
402+
}
403+
return doltserver.DefaultSharedServerPort
404+
}
405+
406+
if port := doltserver.ReadPortFile(beadsDir); port > 0 {
407+
return port
408+
}
409+
410+
if p := config.GetStringFromDir(beadsDir, "dolt.port"); p != "" {
411+
if port, err := strconv.Atoi(p); err == nil && port > 0 {
412+
return port
413+
}
414+
}
415+
416+
if cfg.DoltServerPort > 0 {
417+
return cfg.DoltServerPort
418+
}
419+
420+
return configfile.DefaultDoltServerPort
421+
}
422+
325423
func printBootstrapPlan(plan BootstrapPlan) {
326424
switch plan.Action {
327425
case "none":

‎cmd/bd/bootstrap_test.go‎

Lines changed: 139 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ func snapshotBootstrapEnv(t *testing.T) func() {
4040

4141
func TestDetectBootstrapAction_NoneWhenDatabaseExists(t *testing.T) {
4242
t.Setenv("BEADS_DOLT_DATA_DIR", "")
43+
t.Setenv("BEADS_DOLT_SHARED_SERVER", "")
4344
t.Setenv("BEADS_DOLT_SERVER_DATABASE", "")
4445
t.Setenv("BEADS_DOLT_SERVER_HOST", "")
4546
t.Setenv("BEADS_DOLT_SERVER_PORT", "")
@@ -214,6 +215,7 @@ func TestDetectBootstrapAction_ServerModeMissingConfiguredDBDoesNotReturnNone(t
214215

215216
func TestDetectBootstrapAction_ServerModeProbeErrorStopsWithReason(t *testing.T) {
216217
t.Setenv("BEADS_DOLT_DATA_DIR", "")
218+
t.Setenv("BEADS_DOLT_SHARED_SERVER", "")
217219
t.Setenv("BEADS_DOLT_SERVER_DATABASE", "")
218220
t.Setenv("BEADS_DOLT_SERVER_HOST", "")
219221
t.Setenv("BEADS_DOLT_SERVER_PORT", "")
@@ -971,6 +973,143 @@ func TestDetectBootstrapAction_SharedServerEnvUsesSharedPath(t *testing.T) {
971973
}
972974
}
973975

976+
// TestDetectBootstrapAction_WorktreeSynthesizedDirPrefersSyncOverDefaultSharedDB
977+
// verifies that when bootstrap is running from a worktree whose fallback
978+
// .beads path lives under a bare/common git directory, remote recovery via
979+
// refs/dolt/data wins over an unrelated default "beads" database already
980+
// present on the shared server.
981+
func TestDetectBootstrapAction_WorktreeSynthesizedDirPrefersSyncOverDefaultSharedDB(t *testing.T) {
982+
t.Setenv("BEADS_DOLT_DATA_DIR", "")
983+
t.Setenv("BEADS_DOLT_SERVER_DATABASE", "")
984+
t.Setenv("BEADS_DOLT_SERVER_HOST", "")
985+
t.Setenv("BEADS_DOLT_SERVER_PORT", "")
986+
t.Setenv("BEADS_DOLT_SERVER_MODE", "")
987+
t.Setenv("BEADS_DOLT_SHARED_SERVER", "1")
988+
989+
homeDir := t.TempDir()
990+
t.Setenv("HOME", homeDir)
991+
992+
originBare := filepath.Join(t.TempDir(), "origin.git")
993+
runGitForBootstrapTest(t, "", "init", "--bare", "--initial-branch=main", originBare)
994+
995+
sourceDir := t.TempDir()
996+
runGitForBootstrapTest(t, sourceDir, "init", "-b", "main")
997+
runGitForBootstrapTest(t, sourceDir, "config", "user.email", "test@test.com")
998+
runGitForBootstrapTest(t, sourceDir, "config", "user.name", "Test User")
999+
runGitForBootstrapTest(t, sourceDir, "commit", "--allow-empty", "-m", "init")
1000+
runGitForBootstrapTest(t, sourceDir, "remote", "add", "origin", originBare)
1001+
runGitForBootstrapTest(t, sourceDir, "push", "origin", "main")
1002+
runGitForBootstrapTest(t, sourceDir, "push", "origin", "HEAD:refs/dolt/data")
1003+
1004+
localBare := filepath.Join(t.TempDir(), "local-bare.git")
1005+
runGitForBootstrapTest(t, "", "clone", "--bare", originBare, localBare)
1006+
1007+
worktreeDir := filepath.Join(t.TempDir(), "worktree")
1008+
runGitForBootstrapTest(t, "", "--git-dir="+localBare, "worktree", "add", worktreeDir, "main")
1009+
1010+
sharedDoltDir := filepath.Join(homeDir, ".beads", "shared-server", "dolt")
1011+
if err := os.MkdirAll(filepath.Join(sharedDoltDir, "beads"), 0o750); err != nil {
1012+
t.Fatal(err)
1013+
}
1014+
1015+
oldWd, err := os.Getwd()
1016+
if err != nil {
1017+
t.Fatal(err)
1018+
}
1019+
defer func() { _ = os.Chdir(oldWd) }()
1020+
if err := os.Chdir(worktreeDir); err != nil {
1021+
t.Fatal(err)
1022+
}
1023+
1024+
synthesizedDir := filepath.Join(localBare, ".beads")
1025+
cfg, cfgErr := configfile.Load(synthesizedDir)
1026+
if cfgErr != nil || cfg == nil {
1027+
cfg = findParentConfig(synthesizedDir)
1028+
}
1029+
if cfg == nil {
1030+
cfg = configfile.DefaultConfig()
1031+
}
1032+
1033+
if got := cfg.GetDoltDatabase(); got != "beads" {
1034+
t.Fatalf("GetDoltDatabase() = %q, want %q (default expected without local metadata)", got, "beads")
1035+
}
1036+
1037+
origCheck := checkBootstrapServerDB
1038+
checkBootstrapServerDB = func(probeCfg bootstrapServerProbeConfig) bootstrapServerDBCheck {
1039+
if probeCfg.database != "beads" {
1040+
t.Fatalf("probeCfg.database = %q, want %q", probeCfg.database, "beads")
1041+
}
1042+
return bootstrapServerDBCheck{Exists: true, Reachable: true}
1043+
}
1044+
defer func() { checkBootstrapServerDB = origCheck }()
1045+
1046+
plan := detectBootstrapAction(synthesizedDir, cfg)
1047+
1048+
if plan.Action != "sync" {
1049+
t.Fatalf("expected action=%q, got %q: %s", "sync", plan.Action, plan.Reason)
1050+
}
1051+
if plan.SyncRemote == "" {
1052+
t.Fatal("expected SyncRemote to be populated from origin refs/dolt/data detection")
1053+
}
1054+
if plan.Database != "beads" {
1055+
t.Errorf("plan.Database = %q, want %q (default metadata-free value should still recover via sync)", plan.Database, "beads")
1056+
}
1057+
}
1058+
1059+
func TestDetectBootstrapAction_SynthesizedDirWithoutRecoveryStillUsesExistingSharedDB(t *testing.T) {
1060+
t.Setenv("BEADS_DOLT_DATA_DIR", "")
1061+
t.Setenv("BEADS_DOLT_SERVER_DATABASE", "")
1062+
t.Setenv("BEADS_DOLT_SERVER_HOST", "")
1063+
t.Setenv("BEADS_DOLT_SERVER_PORT", "")
1064+
t.Setenv("BEADS_DOLT_SERVER_MODE", "")
1065+
t.Setenv("BEADS_DOLT_SHARED_SERVER", "1")
1066+
1067+
homeDir := t.TempDir()
1068+
t.Setenv("HOME", homeDir)
1069+
1070+
worktreeDir := filepath.Join(t.TempDir(), "worktree")
1071+
if err := os.MkdirAll(worktreeDir, 0o750); err != nil {
1072+
t.Fatal(err)
1073+
}
1074+
1075+
oldWd, err := os.Getwd()
1076+
if err != nil {
1077+
t.Fatal(err)
1078+
}
1079+
defer func() { _ = os.Chdir(oldWd) }()
1080+
if err := os.Chdir(worktreeDir); err != nil {
1081+
t.Fatal(err)
1082+
}
1083+
1084+
sharedDoltDir := filepath.Join(homeDir, ".beads", "shared-server", "dolt")
1085+
if err := os.MkdirAll(filepath.Join(sharedDoltDir, "project_existing"), 0o750); err != nil {
1086+
t.Fatal(err)
1087+
}
1088+
1089+
synthesizedDir := filepath.Join(worktreeDir, ".beads")
1090+
cfg := configfile.DefaultConfig()
1091+
cfg.DoltMode = configfile.DoltModeServer
1092+
cfg.DoltDatabase = "project_existing"
1093+
1094+
origCheck := checkBootstrapServerDB
1095+
checkBootstrapServerDB = func(probeCfg bootstrapServerProbeConfig) bootstrapServerDBCheck {
1096+
if probeCfg.database != "project_existing" {
1097+
t.Fatalf("probeCfg.database = %q, want %q", probeCfg.database, "project_existing")
1098+
}
1099+
return bootstrapServerDBCheck{Exists: true, Reachable: true}
1100+
}
1101+
defer func() { checkBootstrapServerDB = origCheck }()
1102+
1103+
plan := detectBootstrapAction(synthesizedDir, cfg)
1104+
1105+
if plan.Action != "none" {
1106+
t.Fatalf("expected action=%q, got %q: %s", "none", plan.Action, plan.Reason)
1107+
}
1108+
if !plan.HasExisting {
1109+
t.Fatal("expected HasExisting to be true when configured shared-server DB already exists")
1110+
}
1111+
}
1112+
9741113
// TestFinalizeSyncedBootstrapWritesConfigFiles verifies that after a sync
9751114
// clone, finalizeSyncedBootstrap writes the metadata.json and config.yaml
9761115
// files bd needs to reopen the cloned database. This is the regression

0 commit comments

Comments
 (0)