Skip to content

fix: bd dolt pull fails when remote added without branch tracking#3443

Merged
maphew merged 2 commits into
gastownhall:mainfrom
rjc123:fix/dolt-pull-branch-tracking
Apr 24, 2026
Merged

fix: bd dolt pull fails when remote added without branch tracking#3443
maphew merged 2 commits into
gastownhall:mainfrom
rjc123:fix/dolt-pull-branch-tracking

Conversation

@rjc123

@rjc123 rjc123 commented Apr 23, 2026

Copy link
Copy Markdown
Contributor

Summary

  • When DOLT_PULL fails with "did not specify a branch" (missing upstream tracking in repo_state.json), falls back to DOLT_FETCH + DOLT_MERGE which does not require tracking config
  • Adds isBranchTrackingError() helper to detect the specific Dolt error
  • Preserves the existing pullWithAutoResolve metadata conflict resolution logic

Root cause

When a remote is added via bd dolt remote add (or CALL DOLT_REMOTE('add', ?, ?)), the remote is registered in repo_state.json under "remotes", but "branches" remains {}. DOLT_PULL validates upstream tracking config before executing, even when the branch is passed explicitly. dolt clone / bd bootstrap set up branch tracking automatically, so this only affects databases where remotes were added after init.

The DOLT_FETCH + DOLT_MERGE approach is already used in versioncontrolops.Pull() (lines 66-80) specifically for this case (GH#3144), but was not applied to the pullWithAutoResolve path used by DoltStore.Pull().

Reproduction

bd init
bd dolt remote add origin <url>
bd dolt push   # works
bd dolt pull   # fails: "did not specify a branch"

Test plan

  • Unit tests: TestIsBranchTrackingError — matches Dolt error, rejects unrelated errors, handles nil
  • Verified repo_state.json has empty "branches": {} on affected database
  • versioncontrolops.Pull already proven in production for the fetch+merge approach

🤖 Generated with Claude Code

@codecov-commenter

codecov-commenter commented Apr 23, 2026 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 18.18182% with 9 lines in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
internal/storage/dolt/store.go 0.00% 9 Missing ⚠️

📢 Thoughts on this report? Let us know!

maphew
maphew requested changes Apr 24, 2026
View reviewed changes

@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.

The fallback approach looks plausible, and the branch-tracking error unit test passes, but this needs coverage for the actual bug path before merge. Please add an integration/regression test that creates or simulates a remote added without branch tracking and verifies DoltStore.Pull / pullWithAutoResolve succeeds through the DOLT_FETCH + DOLT_MERGE fallback.

@rjc123

rjc123 commented Apr 24, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the detailed feedback. Two regression tests added in the latest commit:

Standard test (TestPullWithAutoResolve_BranchTrackingFallback, no build tag — runs in PR CI):

Uses a stored procedure with SIGNAL to inject the exact Dolt GH#3144 error text, then calls pullWithAutoResolve. Since the test store has no configured remote, DOLT_FETCH fails — confirming the fallback was entered. This covers lines A–D of the fallback block (tracking error detected → DOLT_FETCH attempted → rolled back on failure).

Integration test (TestPullWithAutoResolve_BranchTrackingSuccess, //go:build integration — runs in nightly CI):

Reproduces the real failure scenario end-to-end:

  1. Starts a local dolt sql-server with --remotesapi-port as the "remote"
  2. Creates a local DoltStore and adds the remote via DOLT_REMOTE('add', ...) — not clone — so repo_state.json has an empty branches map (the GH#3144 precondition)
  3. Confirms bare DOLT_PULL fails with the tracking error (skips gracefully if Dolt fixes it upstream)
  4. Calls pullWithAutoResolve and asserts it succeeds via the DOLT_FETCH + DOLT_MERGE fallback

This covers lines E–I. Lines E–I require a remotesapi-accessible remote; they can't be exercised in the standard test container (only port 3306 is mapped).

@rjc123

rjc123 commented Apr 24, 2026

Copy link
Copy Markdown
Contributor Author

Note on CI failures: The sanitizeDBName redeclared build error is a pre-existing bug in main, not introduced by this PR. Commit fef73917 added cmd/bd/database_name.go to share the function across build tags but cmd/bd/dolt_database_name.go was not removed — both files now declare the same symbol. Upstream main's own CI is failing for the same reason. This PR only touches internal/storage/dolt/ and cmd/bd is unaffected by these changes.

rjc123 and others added 2 commits April 24, 2026 09:56
@rjc123 @claude @maphew
fix: fall back to DOLT_FETCH+DOLT_MERGE when DOLT_PULL fails on missi…
0c28424 
…ng branch tracking

When a remote is added via bd dolt remote add (rather than bd bootstrap
or dolt clone), repo_state.json has an empty "branches" map. DOLT_PULL
then fails with "did not specify a branch" even when the branch is
passed explicitly, because Dolt checks upstream tracking config first.

This fix detects that specific error in pullWithAutoResolve and falls
back to the DOLT_FETCH + DOLT_MERGE approach already used by
versioncontrolops.Pull (GH#3144). The fallback preserves the existing
metadata conflict auto-resolution logic.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@rjc123 @claude @maphew
test(dolt): add regression tests for GH#3144 branch-tracking fallback
14f29d8 
pullWithAutoResolve_BranchTrackingFallback (no build tag) injects the
exact Dolt tracking-error text via a stored procedure SIGNAL, then
verifies pullWithAutoResolve enters the DOLT_FETCH + DOLT_MERGE fallback
path (confirmed by the 'fetch from' error when no remote is configured).
Covers lines A–D of the fallback block.

pullWithAutoResolve_BranchTrackingSuccess (//go:build integration)
reproduces the real failure scenario end-to-end: a remote is added via
DOLT_REMOTE('add', ...) rather than clone, leaving repo_state.json with
an empty 'branches' map, causing DOLT_PULL to fail with the tracking
error. The test then verifies pullWithAutoResolve succeeds via the
DOLT_FETCH + DOLT_MERGE fallback. Covers lines E–I of the fallback block.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@maphew maphew force-pushed the fix/dolt-pull-branch-tracking branch from a9b9e38 to 14f29d8 Compare April 24, 2026 16:59
maphew
maphew approved these changes Apr 24, 2026
View reviewed changes

@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.

Approved after rebase and added normal-path regression coverage for the DOLT_FETCH + DOLT_MERGE fallback. CI is green.

@maphew maphew merged commit dfde51f into gastownhall:main Apr 24, 2026
39 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@maphew maphew maphew approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@rjc123 @codecov-commenter @maphew