Skip to content

fix: consult config.yaml for dolt.host in GetDoltServerHost (mirror GH#2073)#3471

Merged
maphew merged 1 commit into
gastownhall:mainfrom
shaunc:fix/dolt-host-config-yaml
Apr 25, 2026
Merged

fix: consult config.yaml for dolt.host in GetDoltServerHost (mirror GH#2073)#3471
maphew merged 1 commit into
gastownhall:mainfrom
shaunc:fix/dolt-host-config-yaml

Conversation

@shaunc

@shaunc shaunc commented Apr 24, 2026

Copy link
Copy Markdown
Contributor

Summary

GetDoltServerHost() resolves the Dolt server host as
BEADS_DOLT_SERVER_HOST env > metadata.json dolt_server_host > 127.0.0.1
— skipping the ~/.config/bd/config.yaml / global config layer. This
makes the host story asymmetric with port: GH#2073 added config.yaml
consultation for dolt.port in DefaultConfig (commit e6734290),
but no analogous fix for host has landed.

Why this matters

Multi-developer teams sharing a single tailnet-reachable Dolt
sql-server want to pin the host once per user in
~/.config/bd/config.yaml. Without this, teams must either:

  1. Set BEADS_DOLT_SERVER_HOST in every operator's shell profile
    (works but per-shell), or
  2. Sweep per-clone .beads/metadata.json edits (works but per-repo).

Neither matches the documented "user config is consulted" precedence
that bd dolt show advertises.

We hit this on a factfiber.ai migration moving Dolt from per-dev
loopback to a shared tailnet IP. The port fix from GH#2073 helped;
host needed the symmetric treatment.

Patch

internal/configfile/configfile.go:

 // GetDoltServerHost returns the Dolt server host.
-// Checks BEADS_DOLT_SERVER_HOST env var first, then config, then default.
+// Priority: BEADS_DOLT_SERVER_HOST env var > metadata.json dolt_server_host
+// > config.yaml / global config dolt.host > DefaultDoltServerHost.
 func (c *Config) GetDoltServerHost() string {
     if h := os.Getenv("BEADS_DOLT_SERVER_HOST"); h != "" {
         return h
     }
     if c.DoltServerHost != "" {
         return c.DoltServerHost
     }
+    if h := config.GetYamlConfig("dolt.host"); h != "" {
+        return h
+    }
     return DefaultDoltServerHost
 }

Resulting precedence: env > metadata.json > config.yaml > default,
matching the order shown by bd dolt show and consistent with the
GH#2073 port fix.

Test plan

  • New GetDoltServerHost_config_yaml subtest in
    internal/configfile/configfile_test.go covering four
    precedence branches: yaml-wins-when-metadata-empty,
    metadata-over-yaml, env-over-yaml, default-when-no-signal
    (existing default test continues to cover the last case).
  • go test ./internal/configfile/ passes (with
    BEADS_DOLT_SERVER_HOST cleared from env — existing tests in
    TestDoltServerMode/GetDoltServerHost already assume this).
  • gofmt clean, go vet clean.

Notes

  • internal/configfile previously had no dependency on
    internal/config; the new import is one-directional, no cycle.
  • Stale ~/.config/bd/config.yaml from prior bd dolt set host
    invocations could surprise users who expect the previous metadata-
    only behavior. Mitigation: bd doctor --fix or manual cleanup.
    This is the same surprise window the port fix introduced.

…H#2073)

The GetDoltServerHost() resolver checks BEADS_DOLT_SERVER_HOST env
var first, then metadata.json, then falls back to 127.0.0.1 --
skipping the config.yaml / global config layer. This makes the
host story asymmetric with port (GH#2073 fix, commit e673429
added config.yaml consultation for dolt.port in DefaultConfig).

For multi-dev teams sharing a single tailnet-reachable Dolt
sql-server, config.yaml is the natural place to pin the host
once per user. Without this fix, teams must either set
BEADS_DOLT_SERVER_HOST in every operator's shell profile or
sweep per-clone metadata.json edits. Both workarounds exist;
neither is the documented "user config is consulted" story.

Add config.GetYamlConfig("dolt.host") between the metadata.json
and default fallbacks in (*Config).GetDoltServerHost(). Update
the doc comment to reflect the new precedence:

  env var > metadata.json > config.yaml > default

Matches the resolution order described by bd dolt show and the
fix that landed for port via GH#2073. Add test covering the four
precedence branches.
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ All tests successful. No failed tests found.

📢 Thoughts on this report? Let us know!

@maphew maphew left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the host precedence change and ran local targeted validation: go test -tags gms_pure_go -run '^TestDoltServerMode$/^GetDoltServerHost' ./internal/configfile/. The behavior mirrors the existing dolt.port config-yaml layer and keeps env/metadata precedence intact. I’m approving, but leaving merge for when GitHub reports mergeability instead of UNKNOWN.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants