Skip to content

fix(sync): set remote location when syncing to explicit revision - #137

Open
diredex wants to merge 1 commit into
EpicGames:mainfrom
diredex:fix-sync-explicit-remote-branch-pointer
Open

fix(sync): set remote location when syncing to explicit revision#137
diredex wants to merge 1 commit into
EpicGames:mainfrom
diredex:fix-sync-explicit-remote-branch-pointer

Conversation

@diredex

@diredex diredex commented Jul 20, 2026

Copy link
Copy Markdown

Fix: Update branch Latest pointer on explicit --remote sync

Summary

Fixes a bug where syncing explicitly to a specific remote revision (lore sync <revision> --remote) would correctly update the local working copy but fail to advance the local branch's Latest pointer metadata.

As a result of the stale pointer, lore status would falsely report Local branch is behind remote, and subsequent lore sync operations would fail to realize they were already fully synced.

Root Cause

When lore sync resolves a target implicitly (without a user-provided hash), it compares local vs. remote latest pointers and appropriately sets the internal location flag to either Local or Remote. The local Latest metadata pointer is only updated at the end of the sync if location == Remote.

However, when a user explicitly provides a revision hash, this divergence resolution logic was completely bypassed. The target revision was resolved blindly, and location was never updated from its default value (Local). Consequently, the branch pointer update block was skipped entirely at the end of the sync process.

Changes

  • lore-revision/src/revision/sync.rs: Added logic in the explicit revision path to search the remote timeline (history::find_branch_point) when --remote is active. If the explicit revision exists on the remote, the location is properly set to Remote, allowing the pointer update block to execute normally.
  • scripts/test/test_sync.py: Added an automated end-to-end Python integration test (test_sync_remote_explicit_revision) that reproduces the bug structure and asserts that status and sync behave correctly post-sync.

Verification

  • Verified against the Python test_sync.py suite.
  • Rust lore-revision unit tests and standard clippy/fmt tasks pass cleanly.

@diredex
diredex force-pushed the fix-sync-explicit-remote-branch-pointer branch from 9659ae3 to d430dd9 Compare July 20, 2026 22:29
@ajcarberry ajcarberry added the area:core Core library and its interfaces (lib, C API); revision, storage, transport, protocol internals label Aug 19, 2026
@mjansson

Copy link
Copy Markdown
Collaborator

@diredex Could you update to latest main and resolve the conflicts? Thanks!

@diredex
diredex force-pushed the fix-sync-explicit-remote-branch-pointer branch from d430dd9 to 15df387 Compare August 31, 2026 12:42
@diredex

diredex commented Aug 31, 2026

Copy link
Copy Markdown
Author

@diredex Could you update to latest main and resolve the conflicts? Thanks!

Thanks! I've rebased onto the latest main, resolved the merge conflicts, and pushed the updated branch. Let me know if there's anything else that needs to be addressed.

@diredex
diredex force-pushed the fix-sync-explicit-remote-branch-pointer branch from 15df387 to 15267d0 Compare August 31, 2026 12:53
@mjansson

Copy link
Copy Markdown
Collaborator

Seems to make some smoke tests fail - please evaluate if you think the tests are wrong and should be updated, or if something in this change needs to amended.

@diredex
diredex force-pushed the fix-sync-explicit-remote-branch-pointer branch from 15267d0 to 5d3a875 Compare September 2, 2026 09:39
Signed-off-by: Dex <diredex.dev@gmail.com>
@diredex
diredex force-pushed the fix-sync-explicit-remote-branch-pointer branch from 5d3a875 to f4ae1b1 Compare September 2, 2026 10:04
@diredex

diredex commented Sep 2, 2026

Copy link
Copy Markdown
Author

Seems to make some smoke tests fail - please evaluate if you think the tests are wrong and should be updated, or if something in this change needs to amended.

The smoke failures were regressions from my change the condition for setting location = Remote was too broad (it fired for the default search mode, not just --remote), which broke the Latest pointer for regular explicit-revision syncs and caused the merge check to trigger incorrectly.

I've amended the PR with two fixes:

Narrowed the remote location check to only apply when --remote is explicitly passed (ResolveSearchLocation::Remote)
Added a guard to skip the merge-divergence path for explicit revision syncs
All sync/history/layer/search_nearest tests pass now. The only remaining failure is test_service_resolves_relative_paths_against_caller which is pre-existing (this PR only touches sync.rs and test_sync.py).

@mjansson mjansson added the ready-to-import Approved by Epic staff for import into Lore label Sep 3, 2026
@epic-lore-bot epic-lore-bot Bot added imported Imported into Lore for internal review and removed ready-to-import Approved by Epic staff for import into Lore labels Sep 3, 2026
@epic-lore-bot

epic-lore-bot Bot commented Sep 3, 2026

Copy link
Copy Markdown

Imported as Lore CR-531.

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

Labels

area:core Core library and its interfaces (lib, C API); revision, storage, transport, protocol internals imported Imported into Lore for internal review

Development

Successfully merging this pull request may close these issues.

3 participants