Repository navigation
Conversation
A missing local file made sync archive the variation in LaunchDarkly. A file is often missing only because the working copy does not have it yet, for example after a Git pull or a branch switch. A plain sync could then archive a variation that other engineers use. Sync now restores a missing file from LaunchDarkly. The sync plan never archives a variation. "ldcli sync prompt detach --archive" is the only way to archive one. It archives each selected variation, and then stops syncing it. It asks for confirmation, and with --no-input it needs --yes. The archive runs first, and an archived variation does not fail, so the same command can run again after a later step fails. The confirmation prompt moves to the interactive package, so that sync and detach use the same prompt.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
|
Review notes. I made no code changes. 1. Medium: you cannot safely rerun
Effect: archive runs before 2. Low: The confirm prompt only asks 3. Inherited from #856: first sync with no Because the baseline falls back to the shared remote manifest when there is no lock, the new "missing file means restore" rule pulls variations from other branches into this branch (checked with a scratch test: |
tonytrinh3
left a comment
tonytrinh3
left a comment
There was a problem hiding this comment.
@ctawiah Overall it looks good to me. Devin pointed out additional tests that needed to be added.
A restored variation file lost its link to the prompt file that it used. The lock now records the link of each variation, and a restore keeps it. Sync stops with a clear error, and writes nothing, when it cannot restore the link safely. This happens when the working copy has no sync.lock, when the linked file is not available, or when the linked file has local edits. Detach now reads each variation before it archives it, because the API rejects the archive of an archived variation. A rerun after a failed step skips the variations that are already archived and finishes. Detach also uses the command context, so a canceled command does not archive. The lock no longer stores the remote version of each entry. Each save still sends the version that sync read, so optimistic locking does not change. A lock entry now changes only when its content or its link changes, which keeps Git diffs and merge conflicts small. New tests cover linked restores, unsafe restores, a partial failure with several variations, a failed delete followed by a rerun, the interactive archive confirmation, and detach --archive through the CLI.
Every other ldcli resource command uses a plural noun, such as flags, projects, and segments. The command also syncs every prompt variation in the workspace, not one. "ldcli sync prompts" now replaces "ldcli sync prompt", with no alias.
The remote manifest is shared by every branch. A working copy without sync.lock used it as its baseline, so variations from other branches looked tracked and missing. Sync then restored or archived them. Now a missing sync.lock gives an empty baseline. The first sync adopts the local files and writes the lock. This removes the HasLockFile flag, the lock write on a sync without changes, and the restore error for a working copy without sync.lock.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 4b72020. Configure here.
| // Version is the version of the remote manifest entry, which LaunchDarkly | ||
| // uses for optimistic locking. Only the remote manifest has it. The lock | ||
| // does not store it, so that a lock entry changes only with its content. | ||
| Version int `yaml:"-"` |
There was a problem hiding this comment.
Existing lock files fail to decode
High Severity
sync.lock still uses formatVersion: 1, but Version is now tagged yaml:"-" while decodeLock enables KnownFields. Existing lock files that include version fail to decode, so sync, detach, and watch cannot load the workspace baseline.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 4b72020. Configure here.
There was a problem hiding this comment.
I confirmed this at 4b72020 with a scratch test, which I deleted afterward. I passed decodeLock a formatVersion: 1 lock that has version: 6 on an entry, and it returns decode: yaml: unmarshal errors (field version not found). The cause is the combination of KnownFields(true) and Version being tagged yaml:"-".
How much this matters depends on release order:
- Only lock files written by feat(sync): keep each working copy's baseline in sync.lock #856's format contain
version. feat(sync): keep each working copy's baseline in sync.lock #856 is not merged or released yet. - If feat(sync): keep each working copy's baseline in sync.lock #856 and feat(sync): restore missing files and archive only with detach --archive #857 ship in the same release, no user ever has such a file, so this mainly affects dev builds and test fixtures.
- If feat(sync): keep each working copy's baseline in sync.lock #856 ships in a release before feat(sync): restore missing files and archive only with detach --archive #857, every user who committed a
sync.lockgets a hard error on everysync,detach, andwatchafter upgrading. Their only way out is to edit or delete the file by hand.
Ways to fix it:
- Drop
versionfrom the lock format in feat(sync): keep each working copy's baseline in sync.lock #856 itself, so no lock withversionever exists. - Or keep accepting
versionon read and ignore it, for example with a lock-only struct that hasVersion int \yaml:"version,omitempty"`` and is never written. - Or bump
formatVersionto 2 and migrate version 1 files.
Whichever you pick, a test that decodes a version 1 lock containing version would catch a regression.
The conflict choice opened in the terminal's alternate screen. That screen hid the diff that sync writes just before the question, so the user saw only the three choices. The conflict choice now shows below the current output, and the diff stays above it. The project, config, and model config pickers keep the full screen, because their lists can be long.
2 participants
Context
A missing local file made sync archive the variation in LaunchDarkly. A file is often missing only because the working copy does not have it yet, for example after a Git pull or a branch switch. A plain sync, or a watch session that applies changes, could then archive a variation that other engineers use.
This PR is stacked on #856. That PR makes
sync.locktrack every project, so sync can find a variation whose file is missing.What changes
archive_serveraction is removed.ldcli sync prompts detach <selector> --archiveis the only way to archive a variation. It archives each selected variation in LaunchDarkly, and then stops syncing it.detach --archiveasks for confirmation in a terminal. With--no-input, it needs--yes, the same rule as other destructive changes.interactivepackage, so sync and detach use one copy.ldcli sync prompts, plural, like the other resource commands. There is no alias forsync prompt.Stop syncing a variation and archive it:
Review focus
detach --archivearchives?detach --archivesafe when a step fails: archive, then baseline, then file deletion?--no-input?Verification
go build ./...go vet ./...go test -race ./internal/sync/... ./cmd/sync/...TestPromptRestoresTrackedMissingFileInsteadOfArchivingdeletes a tracked file, runs sync, and checks that the file comes back and nothing is archived.Related changes
sync.lockdetach --archiveNote
Overview
Renames the CLI entry point to
ldcli sync prompts(nopromptalias) and changes how missing local variations are handled: plain sync restores them from LaunchDarkly instead of archiving, andarchive_serveris removed from the sync plan.detach --archive(with--yes/ terminal confirmation via sharedinteractive.Confirm) is now the only path to archive variations in LaunchDarkly; detach archives first, then updates baseline and deletes local files, with idempotent retries when a variation is already archived.sync.lockno longer stores manifest versions; it can storereffor linked prompt files so a restored variation file keeps its link when safe. A working copy withoutsync.lockno longer treats the shared remote manifest as its baseline, so it won’t restore/archive variations another branch synced. Conflict UI uses inline select so plan text stays visible.Reviewed by Cursor Bugbot for commit fb0cf44. Bugbot is set up for automated code reviews on this repo. Configure here.