diff --git a/apps/staged/src-tauri/src/git/state.rs b/apps/staged/src-tauri/src/git/state.rs index 8895c4eb..277398d2 100644 --- a/apps/staged/src-tauri/src/git/state.rs +++ b/apps/staged/src-tauri/src/git/state.rs @@ -74,6 +74,13 @@ pub struct UpstreamGitState { pub ahead: u32, pub behind: u32, pub merge_base_sha: Option, + /// Number of commits `origin/{base_branch}` is ahead of `origin/{branch_name}`. + /// + /// Non-zero means the remote branch tip is stale relative to base. The + /// timeline UI uses this to disable "Rebase onto Origin" when rebasing + /// onto a behind-base remote tip would skip the latest base commits and + /// almost always be the wrong action. + pub behind_base: u32, } #[derive(Debug, Clone, Copy, Serialize, PartialEq, Eq)] @@ -157,6 +164,7 @@ impl FastGitState { ahead: 0, behind: 0, merge_base_sha: None, + behind_base: 0, }, base: BaseGitState { r#ref: base_ref, @@ -521,6 +529,7 @@ where fn compute_upstream_state( run_git: &F, upstream_ref: String, + base_ref: &str, head_sha: Option<&str>, upstream_known_missing: bool, ) -> UpstreamGitState @@ -543,6 +552,7 @@ where ahead: 0, behind: 0, merge_base_sha: None, + behind_base: 0, }; } @@ -562,6 +572,15 @@ where }; let merge_base_sha = merge_base(run_git, "HEAD", &upstream_ref); + // How many commits `origin/{base}` is ahead of `origin/{branch}`. When the + // upstream ref *is* the base ref this is always 0, which is the correct + // signal for "branch tracking its own base" workflows. + let behind_base = if upstream_ref == base_ref { + 0 + } else { + rev_count(run_git, &format!("{upstream_ref}..{base_ref}")) + }; + UpstreamGitState { r#ref: upstream_ref, exists, @@ -570,6 +589,7 @@ where ahead, behind, merge_base_sha, + behind_base, } } @@ -652,6 +672,7 @@ where .unwrap_or(false); let upstream_ref = origin_ref_for_branch(branch_name); let base_ref = origin_ref_for_branch(base_branch); + let base_ref_for_upstream = base_ref.clone(); // Phase 2: Upstream and base state computations are independent of each // other but both need fetch to have completed (for up-to-date remote refs) @@ -661,6 +682,7 @@ where compute_upstream_state( &run_git, upstream_ref, + &base_ref_for_upstream, head_sha.as_deref(), refresh.upstream_known_missing, ) @@ -803,12 +825,14 @@ pub fn complete_local_git_state( refresh_refs_if_needed(&cache_key, &run_git, branch_name, base_branch, fetch_mode); let upstream_ref = origin_ref_for_branch(branch_name); let base_ref = origin_ref_for_branch(base_branch); + let base_ref_for_upstream = base_ref.clone(); let (upstream, base) = std::thread::scope(|s| { let u = s.spawn(|| { compute_upstream_state( &run_git, upstream_ref, + &base_ref_for_upstream, fast.head_sha.as_deref(), refresh.upstream_known_missing, ) @@ -904,6 +928,11 @@ const BATCH_GIT_STATE_SCRIPT: &str = concat!( "if [ -n \"$up_sha\" ] && [ -n \"$head_sha\" ]; then\n", " printf 'UP_COUNTS=%s\\n' \"$(git rev-list --left-right --count \"$head_sha\"...\"$4\" 2>/dev/null || echo '0 0')\"\n", " printf 'UP_MB=%s\\n' \"$(git merge-base \"$head_sha\" \"$4\" 2>/dev/null || true)\"\n", + " if [ \"$4\" != \"$5\" ]; then\n", + " printf 'UP_BEHIND_BASE=%s\\n' \"$(git rev-list --count \"$4\"..\"$5\" 2>/dev/null || echo 0)\"\n", + " else\n", + " printf 'UP_BEHIND_BASE=0\\n'\n", + " fi\n", "fi\n", // --- Base state --- "base_sha=$(git rev-parse --verify \"$5\" 2>/dev/null || true)\n", @@ -929,6 +958,7 @@ struct BatchGitStateOutput { up_ahead: u32, up_behind: u32, up_merge_base: Option, + up_behind_base: u32, base_sha: Option, base_behind: u32, } @@ -944,6 +974,7 @@ fn parse_batch_git_state_output(raw: &str) -> BatchGitStateOutput { let mut up_ahead = 0u32; let mut up_behind = 0u32; let mut up_merge_base = None; + let mut up_behind_base = 0u32; let mut base_sha = None; let mut base_behind = 0u32; @@ -994,6 +1025,8 @@ fn parse_batch_git_state_output(raw: &str) -> BatchGitStateOutput { if !v.is_empty() { up_merge_base = Some(v.to_string()); } + } else if let Some(val) = line.strip_prefix("UP_BEHIND_BASE=") { + up_behind_base = parse_u32(Some(val.trim())); } else if let Some(val) = line.strip_prefix("BASE_SHA=") { let v = val.trim(); if !v.is_empty() { @@ -1016,6 +1049,7 @@ fn parse_batch_git_state_output(raw: &str) -> BatchGitStateOutput { up_ahead, up_behind, up_merge_base, + up_behind_base, base_sha, base_behind, } @@ -1364,6 +1398,7 @@ where ahead: parsed.up_ahead, behind: parsed.up_behind, merge_base_sha: parsed.up_merge_base, + behind_base: parsed.up_behind_base, }; let base = BaseGitState { @@ -1395,6 +1430,7 @@ where ahead: 0, behind: 0, merge_base_sha: None, + behind_base: 0, }, base: BaseGitState { r#ref: base_ref, diff --git a/apps/staged/src-tauri/src/git/state_tests.rs b/apps/staged/src-tauri/src/git/state_tests.rs index 96101bd3..6f116762 100644 --- a/apps/staged/src-tauri/src/git/state_tests.rs +++ b/apps/staged/src-tauri/src/git/state_tests.rs @@ -259,3 +259,29 @@ fn detects_base_branch_moved() { assert_eq!(state.base.commits_since_fork, 1); } + +#[test] +fn upstream_behind_base_zero_when_origin_branch_is_caught_up() { + let (_origin, clone) = remote_backed_feature(); + let state = state(&clone.path, FetchMode::Force); + + // Branch and its origin tip are both forked from main, so origin/main is + // not ahead of origin/feature. + assert_eq!(state.upstream.behind_base, 0); +} + +#[test] +fn upstream_behind_base_counts_origin_base_commits_missing_from_origin_branch() { + let (origin, clone) = remote_backed_feature(); + // Land two commits on origin/main *after* origin/feature was pushed, so + // origin/feature is now two commits behind origin/main. + origin.run_git(&["checkout", "main"]); + origin.write_file("base1.txt", "one\n"); + origin.commit("base 1"); + origin.write_file("base2.txt", "two\n"); + origin.commit("base 2"); + + let state = state(&clone.path, FetchMode::Force); + + assert_eq!(state.upstream.behind_base, 2); +} diff --git a/apps/staged/src-tauri/src/prs.rs b/apps/staged/src-tauri/src/prs.rs index 2ed0fd81..29df3413 100644 --- a/apps/staged/src-tauri/src/prs.rs +++ b/apps/staged/src-tauri/src/prs.rs @@ -225,28 +225,61 @@ fn git_push_with_fallback(args: &str) -> String { format!("if ! git push {args}; then git -c '{HTTPS_FALLBACK_CONFIG}' push {args}; fi") } -fn build_commit_pipeline_steps(kind: &PipelineKind, base_branch: &str) -> Vec { +/// Build the steps for a rebase or squash pipeline. +/// +/// `base_branch` is the branch's configured upstream base (e.g. `main`). For +/// squash this is also the bound the reset must not cross. For rebase it is +/// purely informational in the AI handoff prompts so the agent can sanity-check +/// the requested target. +/// +/// `rebase_target` is the remote ref the rebase will actually run against. It +/// equals `base_branch` for the default "rebase onto base" action and equals +/// the branch's own name for "rebase onto origin" (used when local has +/// diverged from `origin/{branch}`). Only the rebase variant consults this +/// value; squash always operates against the base branch. +fn build_commit_pipeline_steps( + kind: &PipelineKind, + base_branch: &str, + rebase_target: &str, +) -> Vec { match kind { - PipelineKind::Rebase => vec![ - PipelineStep::Command { - label: "Fetch latest base".to_string(), - command: git_fetch_with_fallback(base_branch), - on_failure: FailureStrategy::HandoffToAi { - prompt_template: format!( - "The fetch failed. Diagnose and fix the issue, then rebase this branch onto `origin/{base_branch}` with DCO signoffs. Resolve conflicts if present and continue the rebase. Do not push the branch.\n\n{{step_outputs}}" - ), + PipelineKind::Rebase => { + let target_note = if base_branch == rebase_target { + String::new() + } else { + format!( + " The branch's configured base is `origin/{base_branch}`; the requested target `origin/{rebase_target}` is different. If `origin/{rebase_target}` is itself behind `origin/{base_branch}` by a non-trivial amount, surface that to the user before continuing — the rebase target may be wrong." + ) + }; + let (fetch_label, rebase_label) = if base_branch == rebase_target { + ("Fetch latest base".to_string(), "Rebase onto base".to_string()) + } else { + ( + format!("Fetch origin/{rebase_target}"), + format!("Rebase onto origin/{rebase_target}"), + ) + }; + vec![ + PipelineStep::Command { + label: fetch_label, + command: git_fetch_with_fallback(rebase_target), + on_failure: FailureStrategy::HandoffToAi { + prompt_template: format!( + "The fetch failed. Diagnose and fix the issue, then rebase this branch onto `origin/{rebase_target}` with DCO signoffs. Resolve conflicts if present and continue the rebase. Do not push the branch.{target_note}\n\n{{step_outputs}}" + ), + }, }, - }, - PipelineStep::Command { - label: "Rebase onto base".to_string(), - command: format!("git rebase --signoff origin/{base_branch}"), - on_failure: FailureStrategy::HandoffToAi { - prompt_template: format!( - "The rebase failed. Inspect the output, recover from the actual failure, resolve conflicts if present, then continue the rebase onto `origin/{base_branch}` with DCO signoffs. Do not push the branch.\n\n{{step_outputs}}" - ), + PipelineStep::Command { + label: rebase_label, + command: format!("git rebase --signoff origin/{rebase_target}"), + on_failure: FailureStrategy::HandoffToAi { + prompt_template: format!( + "The rebase failed. Inspect the output, recover from the actual failure, resolve conflicts if present, then continue the rebase onto `origin/{rebase_target}` with DCO signoffs. Do not push the branch.{target_note}\n\n{{step_outputs}}" + ), + }, }, - }, - ], + ] + } PipelineKind::Squash => vec![ // Fetch first so origin/{base} is up-to-date before computing the // merge-base for the destructive soft reset. Without this, a stale @@ -299,17 +332,22 @@ Here is the context from the prior steps: } } +#[allow(clippy::too_many_arguments)] async fn start_running_commit_pipeline_for_branch( ctx: BranchPipelineContext, kind: PipelineKind, steps: Vec, + rebase_target: Option, provider: Option, store: Arc, app_handle: &tauri::AppHandle, registry: &Arc, ) -> Result { let prompt = commit_pipeline_prompt(&kind); - let pipeline = PipelineExecution::from_steps(&steps).with_kind(kind); + let mut pipeline = PipelineExecution::from_steps(&steps).with_kind(kind); + if let Some(target) = rebase_target { + pipeline = pipeline.with_rebase_target(target); + } let mut session = store::Session::new_running(prompt, &ctx.working_dir); if let Some(ref p) = provider { @@ -379,9 +417,13 @@ pub(crate) async fn start_or_queue_commit_pipeline_for_branch( .get_branch(&branch_id) .map_err(|e| e.to_string())? .ok_or_else(|| format!("Branch not found: {branch_id}"))?; + let base_branch = base_branch_name(&branch); let rebase_ref = rebase_ref_for_target(&branch, target.as_deref()); - let steps = build_commit_pipeline_steps(&kind, &rebase_ref); - let pipeline = PipelineExecution::from_steps(&steps).with_kind(kind); + let steps = build_commit_pipeline_steps(&kind, base_branch, &rebase_ref); + let mut pipeline = PipelineExecution::from_steps(&steps).with_kind(kind); + if matches!(target.as_deref(), Some("origin")) { + pipeline = pipeline.with_rebase_target(rebase_ref.clone()); + } let mut session = store::Session::new_queued(prompt); if let Some(ref p) = provider { session = session.with_provider(p); @@ -396,13 +438,17 @@ pub(crate) async fn start_or_queue_commit_pipeline_for_branch( } let ctx = resolve_branch_pipeline_context(&store, &branch_id)?; + let base_branch = base_branch_name(&ctx.branch).to_string(); let rebase_ref = rebase_ref_for_target(&ctx.branch, target.as_deref()); - let steps = build_commit_pipeline_steps(&kind, &rebase_ref); + let steps = build_commit_pipeline_steps(&kind, &base_branch, &rebase_ref); + let persisted_rebase_target = + matches!(target.as_deref(), Some("origin")).then(|| rebase_ref.clone()); start_running_commit_pipeline_for_branch( ctx, kind, steps, + persisted_rebase_target, provider, store, &app_handle, @@ -424,12 +470,22 @@ pub(crate) async fn start_queued_commit_pipeline_for_branch( .as_ref() .and_then(|pipeline| pipeline.kind.clone()) .ok_or_else(|| format!("Queued session {} has no pipeline kind", session.id))?; + let queued_rebase_target = session + .pipeline + .as_ref() + .and_then(|pipeline| pipeline.rebase_target.clone()); let ctx = resolve_branch_pipeline_context(&store, &branch_id)?; - let base_branch = base_branch_name(&ctx.branch); - let steps = build_commit_pipeline_steps(&kind, base_branch); + let base_branch = base_branch_name(&ctx.branch).to_string(); + let rebase_ref = queued_rebase_target + .clone() + .unwrap_or_else(|| base_branch.clone()); + let steps = build_commit_pipeline_steps(&kind, &base_branch, &rebase_ref); let prompt = commit_pipeline_prompt(&kind); - let pipeline = PipelineExecution::from_steps(&steps).with_kind(kind); + let mut pipeline = PipelineExecution::from_steps(&steps).with_kind(kind); + if let Some(target) = queued_rebase_target { + pipeline = pipeline.with_rebase_target(target); + } let effective_provider = session.provider.clone().or(provider); let transitioned = store @@ -1239,21 +1295,82 @@ mod tests { #[test] fn rebase_pipeline_uses_signoff() { - let steps = build_commit_pipeline_steps(&PipelineKind::Rebase, "main"); + let steps = build_commit_pipeline_steps(&PipelineKind::Rebase, "main", "main"); - let (_, command, _) = command_at(&steps, 0); + let (label, command, _) = command_at(&steps, 0); + assert_eq!(label, "Fetch latest base"); assert_eq!( command, "if ! git fetch origin main; then git -c 'url.https://github.com/.insteadOf=git@github.com:' fetch origin main; fi" ); - let (_, command, _) = command_at(&steps, 1); + let (label, command, _) = command_at(&steps, 1); + assert_eq!(label, "Rebase onto base"); assert_eq!(command, "git rebase --signoff origin/main"); } + #[test] + fn rebase_pipeline_targets_origin_branch_when_target_differs() { + let steps = build_commit_pipeline_steps(&PipelineKind::Rebase, "main", "feature-branch"); + + let (label, command, _) = command_at(&steps, 0); + assert_eq!(label, "Fetch origin/feature-branch"); + assert_eq!( + command, + "if ! git fetch origin feature-branch; then git -c 'url.https://github.com/.insteadOf=git@github.com:' fetch origin feature-branch; fi" + ); + + let (label, command, _) = command_at(&steps, 1); + assert_eq!(label, "Rebase onto origin/feature-branch"); + assert_eq!(command, "git rebase --signoff origin/feature-branch"); + } + + #[test] + fn rebase_pipeline_prompt_mentions_base_when_target_differs() { + let steps = build_commit_pipeline_steps(&PipelineKind::Rebase, "main", "feature-branch"); + + let fetch_failure = match &steps[0] { + PipelineStep::Command { + on_failure: FailureStrategy::HandoffToAi { prompt_template }, + .. + } => prompt_template.clone(), + _ => panic!("expected fetch step to have HandoffToAi failure"), + }; + assert!(fetch_failure.contains("origin/feature-branch")); + assert!(fetch_failure.contains("origin/main")); + assert!(fetch_failure.contains("the rebase target may be wrong")); + + let rebase_failure = match &steps[1] { + PipelineStep::Command { + on_failure: FailureStrategy::HandoffToAi { prompt_template }, + .. + } => prompt_template.clone(), + _ => panic!("expected rebase step to have HandoffToAi failure"), + }; + assert!(rebase_failure.contains("origin/feature-branch")); + assert!(rebase_failure.contains("origin/main")); + assert!(rebase_failure.contains("the rebase target may be wrong")); + } + + #[test] + fn rebase_pipeline_prompt_omits_target_note_when_target_matches_base() { + let steps = build_commit_pipeline_steps(&PipelineKind::Rebase, "main", "main"); + + for step in &steps { + if let PipelineStep::Command { + on_failure: FailureStrategy::HandoffToAi { prompt_template }, + .. + } = step + { + assert!(!prompt_template.contains("the rebase target may be wrong")); + assert!(!prompt_template.contains("is different")); + } + } + } + #[test] fn squash_pipeline_prompt_requires_signoff() { - let steps = build_commit_pipeline_steps(&PipelineKind::Squash, "main"); + let steps = build_commit_pipeline_steps(&PipelineKind::Squash, "main", "main"); let (_, prompt) = ai_prompt_at(&steps, 3); assert!(prompt.contains("Use the user's global git identity")); diff --git a/apps/staged/src-tauri/src/store/models.rs b/apps/staged/src-tauri/src/store/models.rs index 8858faa0..b6c6d6a0 100644 --- a/apps/staged/src-tauri/src/store/models.rs +++ b/apps/staged/src-tauri/src/store/models.rs @@ -1267,6 +1267,14 @@ pub struct PipelineStepStatus { pub struct PipelineExecution { #[serde(default, skip_serializing_if = "Option::is_none")] pub kind: Option, + /// Remote ref the rebase variant should target (without the `origin/` prefix). + /// + /// `None` means the pipeline targets the branch's configured base (today's + /// default). `Some("feature-x")` records that a "Rebase onto Origin" was + /// requested so the queued path can re-derive the same steps on dequeue + /// rather than silently downgrading to a base rebase. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub rebase_target: Option, pub steps: Vec, pub current_step: usize, /// Set when pipeline completes without needing AI. @@ -1302,6 +1310,7 @@ impl PipelineExecution { Self { kind: None, + rebase_target: None, steps: step_statuses, current_step: 0, completed_without_ai: false, @@ -1312,6 +1321,11 @@ impl PipelineExecution { self.kind = Some(kind); self } + + pub fn with_rebase_target(mut self, target: String) -> Self { + self.rebase_target = Some(target); + self + } } /// Durable identity for commit-producing command pipelines. @@ -1348,6 +1362,28 @@ mod pipeline_tests { assert_eq!(execution.kind, None); } + + #[test] + fn pipeline_rebase_target_is_optional_for_legacy_pipeline_json() { + let execution: PipelineExecution = serde_json::from_str( + r#"{"kind":"rebase","steps":[],"currentStep":0,"completedWithoutAi":false}"#, + ) + .unwrap(); + + assert_eq!(execution.rebase_target, None); + } + + #[test] + fn pipeline_rebase_target_round_trips() { + let execution: PipelineExecution = serde_json::from_str( + r#"{"kind":"rebase","rebaseTarget":"feature-x","steps":[],"currentStep":0,"completedWithoutAi":false}"#, + ) + .unwrap(); + + assert_eq!(execution.rebase_target.as_deref(), Some("feature-x")); + let json = serde_json::to_string(&execution).unwrap(); + assert!(json.contains("\"rebaseTarget\":\"feature-x\"")); + } } // ============================================================================= diff --git a/apps/staged/src/lib/features/branches/prButtonGitState.test.ts b/apps/staged/src/lib/features/branches/prButtonGitState.test.ts index aeb5a3d8..adb14d54 100644 --- a/apps/staged/src/lib/features/branches/prButtonGitState.test.ts +++ b/apps/staged/src/lib/features/branches/prButtonGitState.test.ts @@ -19,6 +19,7 @@ function makeGitState( ahead: relation === 'localAhead' || relation === 'diverged' ? 1 : 0, behind: relation === 'originAhead' || relation === 'diverged' ? 1 : 0, mergeBaseSha: null, + behindBase: 0, }, base: { ref: 'main', sha: null, commitsSinceFork: 0 }, worktree: { diff --git a/apps/staged/src/lib/features/timeline/BranchTimeline.svelte b/apps/staged/src/lib/features/timeline/BranchTimeline.svelte index a030daa4..2d01697d 100644 --- a/apps/staged/src/lib/features/timeline/BranchTimeline.svelte +++ b/apps/staged/src/lib/features/timeline/BranchTimeline.svelte @@ -431,20 +431,39 @@ 2 ); const behindCount = state.upstream.behind; + // When `origin/{branch}` is itself behind `origin/{base}` rebasing onto + // origin would replay branch commits over a stale tip — almost never + // what the user wants. Surface that fact and disable the action; the + // companion "rebase onto base" action remains available via onRebase + // wired on other rows. + const baseRefShort = state.base.ref.replace(/^origin\//, '') || state.base.ref; + const upstreamBehindBase = state.upstream.behindBase; + const originBehindBaseReason = + upstreamBehindBase > 0 + ? `origin/${state.currentBranch ?? 'branch'} is ${plural(upstreamBehindBase, 'commit')} behind ${baseRefShort}; rebase onto ${baseRefShort} instead` + : undefined; + const baseSummary = + upstreamBehindBase > 0 + ? ` and is ${plural(upstreamBehindBase, 'commit')} behind ${baseRefShort}` + : ''; + const divergedTitle = `origin diverges here and has ${plural(behindCount, 'more commit')}${baseSummary}`; + const divergedTitleHtml = `origin diverges here and has ${escapeHtml(plural(behindCount, 'more commit'))}${escapeHtml(baseSummary)}`; + const rebaseReason = forcePushingOrigin + ? 'Force push in progress' + : originBehindBaseReason + ? originBehindBaseReason + : onRebaseBranchOntoOrigin + ? (rebaseBranchDisabledReason ?? undefined) + : undefined; rows.push({ key: 'git-diverged', type: 'git-merge-warning', - title: `origin diverges here and has ${plural(behindCount, 'more commit')}`, - titleHtml: `origin diverges here and has ${escapeHtml(plural(behindCount, 'more commit'))}`, + title: divergedTitle, + titleHtml: divergedTitleHtml, timestamp: placement.timestamp, order: placement.order, - onRebase: - rebaseBranchDisabledReason || forcePushingOrigin ? undefined : onRebaseBranchOntoOrigin, - rebaseDisabledReason: forcePushingOrigin - ? 'Force push in progress' - : onRebaseBranchOntoOrigin - ? (rebaseBranchDisabledReason ?? undefined) - : undefined, + onRebase: rebaseReason ? undefined : onRebaseBranchOntoOrigin, + rebaseDisabledReason: rebaseReason, onForcePush: forcePushingOrigin ? onOpenForcePushSession : rebaseBranchDisabledReason diff --git a/apps/staged/src/lib/types.ts b/apps/staged/src/lib/types.ts index 3c9cbe4c..956f9d81 100644 --- a/apps/staged/src/lib/types.ts +++ b/apps/staged/src/lib/types.ts @@ -203,6 +203,8 @@ export interface BranchGitState { ahead: number; behind: number; mergeBaseSha: string | null; + /** Number of commits `origin/{base}` is ahead of `origin/{branch}`. */ + behindBase: number; }; base: { ref: string;