fix: consult config.yaml for dolt.host in GetDoltServerHost (mirror GH#2073)#3471
Merged
Merged
Conversation
…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 Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
3 tasks
maphew
approved these changes
Apr 25, 2026
maphew
left a comment
Collaborator
There was a problem hiding this comment.
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
GetDoltServerHost()resolves the Dolt server host asBEADS_DOLT_SERVER_HOST env > metadata.json dolt_server_host > 127.0.0.1— skipping the
~/.config/bd/config.yaml/ global config layer. Thismakes the host story asymmetric with port: GH#2073 added config.yaml
consultation for
dolt.portinDefaultConfig(commite6734290),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:BEADS_DOLT_SERVER_HOSTin every operator's shell profile(works but per-shell), or
.beads/metadata.jsonedits (works but per-repo).Neither matches the documented "user config is consulted" precedence
that
bd dolt showadvertises.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:Resulting precedence:
env > metadata.json > config.yaml > default,matching the order shown by
bd dolt showand consistent with theGH#2073 port fix.
Test plan
GetDoltServerHost_config_yamlsubtest ininternal/configfile/configfile_test.gocovering fourprecedence 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 (withBEADS_DOLT_SERVER_HOSTcleared from env — existing tests inTestDoltServerMode/GetDoltServerHostalready assume this).gofmtclean,go vetclean.Notes
internal/configfilepreviously had no dependency oninternal/config; the new import is one-directional, no cycle.~/.config/bd/config.yamlfrom priorbd dolt set hostinvocations could surprise users who expect the previous metadata-
only behavior. Mitigation:
bd doctor --fixor manual cleanup.This is the same surprise window the port fix introduced.