From 9b79ae629b902764f6ca09d4ddcdab17ffa68485 Mon Sep 17 00:00:00 2001 From: ABCxFF <79597906+abcxff@users.noreply.github.com> Date: Fri, 3 Jul 2026 18:15:46 +0000 Subject: [PATCH] [SLOP(claude-opus-4-8-high)] fix(sync): advance to trunk when detached from the frozen chain with nothing owned --- src/jj/bookmarks.rs | 25 +++++++++------- src/workflows/sync.rs | 70 +++++++++++++++++++++++++++++++++---------- tests/sync.rs | 34 +++++++++++++++++++++ 3 files changed, 104 insertions(+), 25 deletions(-) diff --git a/src/jj/bookmarks.rs b/src/jj/bookmarks.rs index 8755456..c5a1809 100644 --- a/src/jj/bookmarks.rs +++ b/src/jj/bookmarks.rs @@ -163,10 +163,16 @@ pub(crate) async fn resolve_stack_resolution( Ok(resolution) } +/// Resolve the stack when nothing is owned but frozen bookmarks are still in +/// scope. Returns `Ok(Some(..))` when `@` sits on the frozen dependency chain +/// (still building on top of not-yet-merged deps), or `Ok(None)` when `@` is +/// detached from every frozen chain — e.g. the frozen deps merged and we are +/// parked on a plain trunk-ish commit. Callers treat `None` as "nothing to +/// sync here; advance to trunk" rather than an error. pub(crate) async fn resolve_purely_frozen_stack( runner: &impl CommandRunner, frozen_bookmarks: Vec, -) -> Result { +) -> Result> { if frozen_bookmarks.is_empty() { bail!( CliError::new("empty owned stack and no frozen bookmarks in scope") @@ -179,10 +185,10 @@ pub(crate) async fn resolve_purely_frozen_stack( if let Some(frozen_dependencies) = frozen_dependency_chain_ending_at(runner, &frozen_changes, &at_commit).await? { - return Ok(StackResolution { + return Ok(Some(StackResolution { owned: Vec::new(), frozen_dependencies, - }); + })); } let current = resolve_stack(runner, "@").await.context("resolve current revision for frozen sync")?; @@ -192,20 +198,19 @@ pub(crate) async fn resolve_purely_frozen_stack( if let Some(frozen_dependencies) = frozen_dependency_chain_ending_at(runner, &frozen_changes, parent).await? { - return Ok(StackResolution { + return Ok(Some(StackResolution { owned: Vec::new(), frozen_dependencies, - }); + })); } } } }; - bail!(CliError::new(format!( - "empty owned stack and current revision {} is not a `forklift/frozen/pr-*` bookmark target or empty child of one", - short_commit_id(&at_commit) - )) - .resolution("run `forklift get ` first, or move to a mutable stack")); + // `@` is off every frozen chain with nothing owned to sync. The frozen + // bookmarks are stale leftovers (their PRs merged, or we moved away); don't + // block getting home — signal the caller to advance to trunk. + Ok(None) } pub(crate) async fn resolve_frozen_changes( diff --git a/src/workflows/sync.rs b/src/workflows/sync.rs index ade4e92..f714910 100644 --- a/src/workflows/sync.rs +++ b/src/workflows/sync.rs @@ -175,30 +175,43 @@ pub(crate) async fn sync_stack( // fetched remote tip and finish, reporting any branches we cleaned up rather // than failing on the empty stack. if stack.is_empty() && frozen_bookmarks.is_empty() { - diagnostics.phase("move-trunk"); - move_trunk_to_remote(runner, config, diagnostics) - .await - .map_err(|error| phase_error("move-trunk", &config.trunk, error))?; - carry_empty_working_copy_to_trunk(runner, config, diagnostics) - .await - .map_err(|error| phase_error("carry-working-copy", "@", error))?; - return Ok(SyncSummary { - rebased_roots: 0, - submit_ran: false, + return finish_sync_at_trunk( + runner, + config, cleaned_branches, pruned_duplicates, - conflicts: 0, - }); + diagnostics, + ) + .await; } let stack_resolution = if stack.is_empty() { - resolve_purely_frozen_stack(runner, frozen_bookmarks).await + match resolve_purely_frozen_stack(runner, frozen_bookmarks) + .await + .map_err(|error| phase_error("resolve-stack", revset, error))? + { + Some(resolution) => resolution, + // `@` is detached from every frozen chain with nothing owned to sync + // (e.g. the frozen deps merged and we're parked on a plain commit). + // Advance to trunk and finish instead of erroring — same as an empty + // stack with no frozen bookmarks. + None => { + return finish_sync_at_trunk( + runner, + config, + cleaned_branches, + pruned_duplicates, + diagnostics, + ) + .await; + } + } } else { match validate_stack_shape(runner, &stack, revset).await { Ok(()) => resolve_stack_resolution(runner, stack, frozen_bookmarks).await, Err(error) => Err(error), } - } - .map_err(|error| phase_error("resolve-stack", revset, error))?; + .map_err(|error| phase_error("resolve-stack", revset, error))? + }; if diagnostics.verbose { print_stack(&stack_resolution.owned); } @@ -322,6 +335,33 @@ pub(crate) async fn sync_stack( }) } +/// Finish a sync that has nothing left to do: move trunk to the fetched remote +/// tip and carry the empty working copy onto it. Used when the owned stack is +/// empty and either no frozen bookmarks remain or `@` is detached from every +/// frozen chain — the "get me back to trunk" recovery path. +async fn finish_sync_at_trunk( + runner: &impl CommandRunner, + config: &AppConfig, + cleaned_branches: usize, + pruned_duplicates: usize, + diagnostics: Diagnostics, +) -> Result { + diagnostics.phase("move-trunk"); + move_trunk_to_remote(runner, config, diagnostics) + .await + .map_err(|error| phase_error("move-trunk", &config.trunk, error))?; + carry_empty_working_copy_to_trunk(runner, config, diagnostics) + .await + .map_err(|error| phase_error("carry-working-copy", "@", error))?; + Ok(SyncSummary { + rebased_roots: 0, + submit_ran: false, + cleaned_branches, + pruned_duplicates, + conflicts: 0, + }) +} + pub(crate) async fn prune_landed_duplicate_changes( runner: &impl CommandRunner, config: &AppConfig, diff --git a/tests/sync.rs b/tests/sync.rs index edbab37..5988ac1 100644 --- a/tests/sync.rs +++ b/tests/sync.rs @@ -234,6 +234,40 @@ fn sync_from_empty_child_above_frozen_pr_succeeds() -> anyhow::Result<()> { Ok(()) } +#[test] +fn sync_from_detached_commit_with_leftover_frozen_advances_to_trunk() -> anyhow::Result<()> { + // After `get` freezes a stack and its PRs later merge, the working copy can + // end up on a plain commit that is no longer on any frozen chain. Sync must + // recover by advancing to trunk instead of erroring that `@` is not a frozen + // bookmark target. + let repo = TestRepo::new("sync-detached-frozen")?; + repo.init_main()?; + let imported = repo.create_change("imported", "imported title", "imported body")?; + let branch = branch_for("imported-title", &imported.change_id); + repo.set_bookmark(&branch, &imported.commit_id)?; + repo.push_bookmark(&branch)?; + repo.seed_pr(11, &branch, "main", "imported title", "imported body")?; + + let get_output = repo.run(&["get", "11"])?; + assert_success("get 11", &get_output); + + // Move off the frozen chain onto a fresh empty child of trunk, leaving the + // `forklift/frozen/pr-11` bookmark behind. + repo.jj(&["new", "main"])?; + + let output = repo.run(&["sync"])?; + assert_success("sync", &output); + let stderr = stderr_of(&output); + // Recovered by advancing to trunk (Moving trunk) rather than erroring on the + // detached `@`, with nothing owned to rebase. + assert!(stderr.contains("Moving trunk"), "stderr:\n{stderr}"); + assert!( + stderr.contains("0 roots rebased"), + "stderr:\n{stderr}" + ); + Ok(()) +} + #[test] fn sync_frozen_suffix_based_on_unfrozen_parent_succeeds() -> anyhow::Result<()> { let repo = TestRepo::new("sync-frozen-suffix")?;