Skip to content

[BUG] Doctor Dolt Status Check Shows False Uncommitted Changes #1924

@mikij

Description

@mikij

Summary

bd doctor reports "2 uncommitted change(s)" but bd vc status shows clean working set. Doctor is querying a non-existent default database instead of the user's configured database.

Environment

  • Beads version: v0.55.1
  • Backend: Dolt
  • Database name: beads_z (configured in dolt_database field)
  • OS: macOS (Darwin)

Reproduction Steps

  1. Initialize beads with custom Dolt database name:

    // .beads/metadata.json
    {
      "backend": "dolt",
      "dolt_database": "beads_z",
      "jsonl_export": "issues.jsonl"
    }
  2. Ensure no default "beads" database exists:

    ls .beads/dolt/beads/  # Returns "No such file or directory"
    # Only .beads/dolt/beads_z/ exists
  3. Run bd vc status:

    # Output: Clean working set (correct)
    Branch: main
    Commit: 1e6mjfhr
    
  4. Run bd doctor:

    # Output: Shows uncommitted changes (incorrect)
    DATA & CONFIG
      ⚠  Dolt Status: 2 uncommitted change(s)
            Changes: [config: modified  metadata: modified]
    

Expected Behavior

bd doctor should recognize that no database named "beads" exists and either:

  • Return "N/A (not using Dolt backend)" if not detecting Dolt correctly
  • OR correctly read dolt_database config field and query the "beads_z" database
  • Show "Clean working set" if the user's actual database has no uncommitted changes

Actual Behavior

Doctor's CheckDoltStatus reports uncommitted changes ("config: modified, metadata: modified") in a database that doesn't exist, contradicting bd vc status which correctly uses the user's configured database.

Root Cause Analysis

Affected Code Locations

File: cmd/bd/doctor/dolt.go

Function: openDoltDBEmbedded() (lines 92-100)

func openDoltDBEmbedded(beadsDir string) (*sql.DB, error) {
    doltDir := filepath.Join(beadsDir, "dolt")
    connStr := fmt.Sprintf("file://%s?commitname=beads&commitemail=beads@local", doltDir)

    dbName := configfile.DefaultDoltDatabase  // ← BUG: Always uses default "beads"
    if cfg, err := configfile.Load(beadsDir); err == nil && cfg != nil {
        dbName = cfg.GetDoltDatabase()  // ← This correctly returns "beads_z"
    }

    db, err := sql.Open("dolt", connStr)
    // Later: USE `dbName`  // ← But connection opens without database specified!
}

The bug: Connection string file://...?commitname=beads&commitemail=beads@local is opened without specifying a database name via the database parameter. Only when USE beads_z is called does it switch databases.

However, if the initial connection or subsequent operations fail, or if Dolt internally defaults to "beads", the doctor queries the wrong database.

Inconsistent Database Access Patterns

Check Type Function Used Reads Config? Database Used Result
Federation checks dolt.New() directly ❌ NO Default "beads" ❌ Error 1049
Dolt Status openDoltDBWithLock() ✅ YES Config "beads_z" ⚠️ Wrong uncommitted changes
Other checks dolt.NewFromConfigWithOptions() ✅ YES Config "beads_z" ✅ Works
User commands Main storage ✅ YES Config "beads_z" ✅ Works

Key inconsistency: dolt.New() (used by federation) doesn't pass a Database parameter, while dolt.NewFromConfigWithOptions() (used by other checks) does.

Related Bug 1

This is related to the federation checks bug (cmd/bd/doctor/federation.go:136, 346) which also uses dolt.New() without a database parameter.

Both bugs stem from the same root cause: inconsistent Dolt connection initialization patterns in doctor checks.

Impact

Severity: Low/Medium

User-facing impact:

  • Doctor shows an incorrect "uncommitted changes" warning that doesn't reflect reality
  • Federation checks fail with "database not found" errors
  • Users may attempt unnecessary fixes (running bd vc commit, troubleshooting database issues)

Functional impact:

  • None - Actual beads operations work correctly
  • The user's bd vc status shows the correct clean state
  • Only diagnostic checks are affected

Configuration Details

User's .beads/metadata.json:

{
  "backend": "dolt",
  "dolt_database": "beads_z",
  "jsonl_export": "issues.jsonl"
}

User's .beads/dolt/ directory:

beads_z/    # Actual database (exists)
beads/       # Default database (does NOT exist)

Suggested Fixes

Fix 1: Make openDoltDBEmbedded() use connection with database parameter

File: cmd/bd/doctor/dolt.go

Change:

func openDoltDBEmbedded(beadsDir string) (*sql.DB, error) {
    doltDir := filepath.Join(beadsDir, "dolt")

    dbName := configfile.DefaultDoltDatabase
    if cfg, err := configfile.Load(beadsDir); err == nil && cfg != nil {
        dbName = cfg.GetDoltDatabase()
    }

    // FIX: Include database in connection string
    connStr := fmt.Sprintf(
        "file://%s?commitname=%s&commitemail=%s&database=%s",
        doltDir,
        "beads",   // Commit name (or use cfg.CommitterName)
        "beads@local",  // Commit email (or use cfg.CommitterEmail)
        dbName,     // ← ADD THIS
    )

    db, err := sql.Open("dolt", connStr)
    // No need for subsequent USE statement
}

Fix 2: Federation checks should pass database name

File: cmd/bd/doctor/federation.go

Locations: Lines 136, 346

Change:

// Before (buggy):
store, err := dolt.New(ctx, &dolt.Config{Path: doltPath, ReadOnly: true})

// After (fixed):
store, err := dolt.New(ctx, &dolt.Config{
    Path: doltPath,
    Database: cfg.GetDoltDatabase(),  // ← ADD THIS
    ReadOnly: true,
})

This requires federation checks to load the config first, similar to how other checks use dolt.NewFromConfigWithOptions().

Fix 3: Unify Dolt connection initialization

Consider refactoring to ensure all doctor checks use consistent database connection patterns:

// Helper function in cmd/bd/doctor/dolt.go
func openDoctorDoltDB(beadsDir string, readOnly bool) (*dolt.DoltConn, error) {
    return dolt.NewFromConfigWithOptions(ctx, beadsDir, &dolt.Config{
        ReadOnly: readOnly,
    })
}

All checks would then use this helper, ensuring consistent config loading and database selection.

Workarounds for Users

Workaround #1: Use default database name

Rename the database to the default "beads":

# If willing to change the database name
mv .beads/dolt/beads_z .beads/dolt/beads

# Update config
sed -i '' 's/"dolt_database": "beads_z"/"dolt_database": "beads"/' .beads/metadata.json

Workaround #2: Ignore doctor warnings

Accept that federation checks show warnings but actual operations work fine. This is currently what the bug reporter is doing (per their preference).

Additional Context

Discovered during investigation of:

  • Stale lock files (unrelated to this bug)
  • Federation checks failing (related Bug bd dep tree mostly broken #1)
  • Config field "database": "dolt" causing confusion (separate issue)

Tested configurations:

  • User has custom dolt_database: "beads_z"
  • No default beads database exists
  • bd vc status correctly uses config
  • Doctor incorrectly uses default database in some checks

Checklist for Fix Implementation

  • Update openDoltDBEmbedded() to include database in connection string
  • Update federation checks to pass Database parameter to dolt.New()
  • Add test case for custom database names
  • Add test case verifying doctor vs bd vc status consistency
  • Consider refactoring to use dolt.NewFromConfigWithOptions() universally

Related Issues

  • Bug 1: Federation checks don't respect dolt_database config (Error 1049)
  • Both issues stem from inconsistent Dolt connection initialization patterns in doctor checks

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions