Skip to content

feat(sync): keep each working copy's baseline in sync.lock - #856

Open
ctawiah wants to merge 1 commit into
mainfrom
ctawiah/sync-lock-baseline
Open

ctawiah wants to merge 1 commit into
mainfrom
ctawiah/sync-lock-baseline

Conversation

@ctawiah

@ctawiah ctawiah commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

Context

All clones of a repository shared one remote manifest as their sync baseline. The source key of that manifest comes only from the Git origin, so every clone and branch read and wrote the same baseline. Each clone has different local files, so the three-way comparison broke:

  • A clone that was behind saw its older file as a local change. Sync then wrote the older content to LaunchDarkly.
  • A clone that did not have a new variation file archived that variation.
  • Two clones that edited one variation got no conflict. The last sync won.

What changes

  • Each working copy keeps its baseline in .launchdarkly/sync.lock. Engineers commit the file, so the baseline moves with the files on each pull and branch switch.
  • The plan compares local files and LaunchDarkly with the lock. A clone that is behind pulls the newer state, and two clones that edit one variation get a conflict.
  • The remote manifest still supplies the version of each entry. Each save sends the version that sync read from LaunchDarkly, so LaunchDarkly rejects two saves of one entry at the same time.
  • A working copy without sync.lock uses the remote manifest once. Its next sync writes the file. --dry-run never writes it.
  • The review shows a note and a suggestion when another working copy synced a different state of a variation. The JSON output has syncedElsewhere and suggestion fields.
  • The lock lists every tracked project, so sync no longer asks Git for deleted files.

The lock file is one sorted entry per resource:

# Written by ldcli sync. Commit this file with the .launchdarkly files.
formatVersion: 1
resources:
  - kind: variation
    project: production
    key: support/default
    fingerprint: sha256:…
    version: 6

Design notes

  • sync.lock is the existing manifest.Manifest saved as YAML, so it reuses the existing validation, sorting, and entry methods.
  • manifest.BaselineStore wraps the existing Store.Update, so batching, the 409 handling, and the read-back after an uncertain write do not change.
  • One manifest.Baselines interface replaces the three identical ManifestStore interfaces in bootstrap, detach, and prompt.
  • Staleness compares fingerprints, not versions. A newer version with the same fingerprint has nothing to pull.
  • This PR keeps today's archive behavior for a missing file. The next PR in the stack changes it.

Review focus

  • Does the plan use the lock as the baseline in every path: sync, bootstrap, and detach?
  • Does each manifest save send the version that sync read from LaunchDarkly?
  • Is the first sync of a workspace without sync.lock safe?
  • Is the staleness note correct and useful?

Verification

  • go build ./...
  • go vet ./...
  • go test -race ./internal/sync/... ./cmd/sync/...
  • TestPromptStaleWorkingCopyPullsInsteadOfReverting fails with the old shared baseline and passes with the lock.

Related changes

  1. This PR: keep each working copy's baseline in sync.lock
  2. feat(sync): restore missing files and archive only with detach --archive #857: restore missing files and archive only with detach --archive

Note

Overview
Moves the sync three-way baseline from a single shared remote manifest to a committed .launchdarkly/sync.lock per working copy, while the remote manifest still supplies optimistic-lock versions.

manifest.BaselineStore loads sync.lock (or falls back to the remote manifest once), compares lock fingerprints to the remote to detect staleness (SyncedElsewhere), patches LaunchDarkly with versions read from the remote, then writes the updated lock. Bootstrap, detach, and prompt sync now take Baselines instead of local ManifestStore interfaces and plan/apply against baseline.Lock.

Adds lock read/write on the local store (atomic, symlink-safe), YAML lock encoding on manifest.Resource, and review/JSON fields for notes and git-pull suggestions when another clone synced ahead. Project discovery uses lock entries instead of Git deleted-path scanning; the file watcher ignores sync.lock updates.

Reviewed by Cursor Bugbot for commit adca116. Bugbot is set up for automated code reviews on this repo. Configure here.

All clones of a repository shared one remote manifest as their sync
baseline. A clone that was behind then looked like it changed its files
back, so sync wrote the older content to LaunchDarkly or archived new
variations.

Each working copy now keeps its baseline in .launchdarkly/sync.lock.
Engineers commit the file, so the baseline moves with the files on each
pull and branch switch. The plan compares local files and LaunchDarkly
with the lock. A clone that is behind pulls the newer state, and two
clones that edit one variation get a conflict.

The remote manifest still supplies the version of each entry. Each save
sends the version that sync read, so LaunchDarkly rejects two saves of
one entry at the same time. A working copy without sync.lock uses the
remote manifest once, and its next sync writes the file.

The review shows a note and a suggestion when another working copy
synced a different state of a variation. The lock lists every tracked
project, so sync no longer asks Git for deleted files.
@devin-ai-integration

Copy link
Copy Markdown
Contributor

Review notes. I made no code changes.

1. High: a working copy with no sync.lock still archives prompts from other branches (the bug this PR fixes)

When sync.lock does not exist, BaselineStore.Load (internal/sync/manifest/baseline.go) uses a copy of the shared remote manifest as the lock (lock = remote.Clone()). That remote manifest holds entries from every branch and clone. So on the first sync after upgrade, or on any branch that has not committed a lock yet, a variation that another branch tracks but this branch has no file for looks like "deleted locally", and sync archives it.

I checked this with a scratch acceptance test (not committed). The remote manifest tracks first and second. The working copy has only first and no lock. sync prompt --yes gives:

- production/support/first   action=in_sync         status=succeeded
- production/support/second  action=archive_server  status=succeeded
PATCH .../ai-configs/support/variations/second

In #857 the same case pulls second down into this branch instead (update_local). That is safer, but it still adds files from other branches. detach with no lock has the same cause: it writes a lock that is the remote manifest minus the selection.

2. Low: if Save fails to write the lock, the copy's own change is labeled "synced elsewhere" from then on

BaselineStore.Save updates the remote manifest first and then writes sync.lock. If writeLock fails, the remote has the new fingerprint but the lock keeps the old one. On later runs Stale returns true for that resource, so it shows "synced elsewhere". Because a sync with no changes only writes the lock when the file is missing, this label stays until the resource changes again. (Found by reading the code. I did not run it.)

3. Low: per-resource version in sync.lock causes Git merge conflicts

Save calls WithVersionsFrom(remote), which rewrites the version of every lock entry with the remote value, not only the entries this run changed. Nothing reads the lock's version (optimistic locking uses current.remote, and Stale compares fingerprints). So two branches that sync different prompts can still conflict in sync.lock on lines neither of them changed.

Written by Devin

Comment thread internal/sync/manifest/baseline.go
Comment thread internal/sync/manifest/model.go
Comment thread internal/sync/prompt/sync.go
Comment thread internal/sync/prompt/state.go
@ctawiah
ctawiah marked this pull request as ready for review October 9, 2026 20:04
@ctawiah
ctawiah requested a review from a team as a code owner October 9, 2026 20:04
@devin-ai-integration

Copy link
Copy Markdown
Contributor

Update on the High item in my earlier review (a working copy with no sync.lock archives prompts from other branches): 4b72020 on #857 fixes it. The fix is not in this PR yet.

On #857, BaselineStore.Load now starts from an empty lock instead of remote.Clone(). On a first sync, chooseAction handles each variation like this:

  • If the local file and LaunchDarkly match, sync records the variation in the lock.
  • If they differ, sync reports a conflict.
  • If only LaunchDarkly has the variation, sync leaves it alone.

So sync no longer archives or restores another branch's variation. TestPromptWithoutSyncLockIgnoresVariationsOfOtherBranches covers this, and go test ./internal/sync/... ./cmd/sync/... passes at 4b72020.

This PR (adca116) still has lock = remote.Clone(), so it still has the bug if it merges or ships without #857. To close the gap, you can either merge the two PRs together or apply the same change to baseline.go here.

Written by Devin

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.

2 participants