From 768e1d9df1dce70a2bb89f83dfd7b73bc81a4729 Mon Sep 17 00:00:00 2001 From: Adrian Cosentino Date: Wed, 15 Jul 2026 17:37:16 -0400 Subject: [PATCH 01/10] feat: apply configured pull request base --- CHANGELOG.md | 3 + crates/bitbygit-core/src/policy.rs | 21 ++++++ crates/bitbygit-tui/src/lib.rs | 116 +++++++++++++++++++++++++++-- docs/configuration.md | 33 ++++++++ 4 files changed, 166 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4039d84..dfa99d1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -33,3 +33,6 @@ semantic versioning once releases begin. in [#13](https://github.com/cosentinode/bitbygit/issues/13). - Added strict typed TOML configuration loading with complete safe-default fallback and redacted path-aware diagnostics. +- Applied configured pull request base defaults after explicit prompt targets + and before provider defaults, with provider validation and visible safe config + diagnostics in the TUI. diff --git a/crates/bitbygit-core/src/policy.rs b/crates/bitbygit-core/src/policy.rs index 0937232..fc5e4af 100644 --- a/crates/bitbygit-core/src/policy.rs +++ b/crates/bitbygit-core/src/policy.rs @@ -10,6 +10,7 @@ pub struct EffectivePolicy { additional_protected_branches: Vec, confirmation: ConfirmationConfig, disabled_operations: Vec, + default_pull_request_base: Option, prompt_enabled: bool, } @@ -25,6 +26,7 @@ impl EffectivePolicy { additional_protected_branches: config.policy.additional_protected_branches.clone(), confirmation: config.policy.confirmation.clone(), disabled_operations: config.policy.disabled_operations.clone(), + default_pull_request_base: config.pull_requests.default_base_branch.clone(), prompt_enabled: config.prompt.enabled, } } @@ -111,6 +113,10 @@ impl EffectivePolicy { pub const fn prompt_enabled(&self) -> bool { self.prompt_enabled } + + pub fn default_pull_request_base(&self) -> Option<&str> { + self.default_pull_request_base.as_deref() + } } impl Default for EffectivePolicy { @@ -390,6 +396,21 @@ mod tests { assert!(!EffectivePolicy::new(&config).prompt_enabled()); } + #[test] + fn pull_request_default_is_available_to_runtime_planning() { + let mut config = AppConfig::default(); + assert_eq!( + EffectivePolicy::new(&config).default_pull_request_base(), + None + ); + + config.pull_requests.default_base_branch = Some("develop".to_owned()); + assert_eq!( + EffectivePolicy::new(&config).default_pull_request_base(), + Some("develop") + ); + } + #[test] fn safe_fallback_blocks_every_operation_and_prompt() { let policy = EffectivePolicy::safe_fallback(); diff --git a/crates/bitbygit-tui/src/lib.rs b/crates/bitbygit-tui/src/lib.rs index def2644..0db2a6c 100644 --- a/crates/bitbygit-tui/src/lib.rs +++ b/crates/bitbygit-tui/src/lib.rs @@ -2514,7 +2514,15 @@ impl OperationPlanner { &canonical_github_repository, &upstream_branch, )?; - let base = requested_base.unwrap_or(repository.default_branch); + let configured_base = + requested_base.is_none() && self.policy.default_pull_request_base().is_some(); + let base = requested_base + .or_else(|| { + self.policy + .default_pull_request_base() + .map(ToOwned::to_owned) + }) + .unwrap_or(repository.default_branch); if canonical_head_github_repository.eq_ignore_ascii_case(&canonical_github_repository) && base == upstream_branch { @@ -2527,10 +2535,16 @@ impl OperationPlanner { .branch_exists(&base) .map_err(open_pull_request_gh_error)? { - return Err(format!( + let mut error = format!( "Open pull request blocked: base branch {base} was not found in {}.", repository.name_with_owner - )); + ); + if configured_base { + error.push_str( + " Update pull-requests.default-base-branch or choose an existing branch with `open pr to `.", + ); + } + return Err(error); } let existing = github .existing_pull_requests(&head) @@ -5589,6 +5603,57 @@ mod tests { Ok(()) } + #[test] + fn pull_request_base_precedence_is_explicit_then_configured_then_provider() + -> Result<(), Box> { + let repo = pushed_branch_repo("open-pr-base-precedence")?; + let fake_gh = fake_gh("open-pr-base-precedence", false)?; + let mut config = AppConfig::default(); + config.pull_requests.default_base_branch = Some("release".to_owned()); + let planner = OperationPlanner { + repo_root: repo.clone(), + github_executable: Some(fake_gh.clone()), + policy: EffectivePolicy::new(&config), + ssh_executable: Some(test_ssh_command()?), + }; + + let configured = planner + .plan_request(OperationRequest::OpenPullRequest { base: None }) + .map_err(std::io::Error::other)?; + assert_eq!( + configured.plan.request, + OperationRequest::OpenPullRequest { + base: Some("release".to_owned()) + } + ); + assert!(configured.plan.preview_text().contains("base: release")); + let execution = PlanExecutor::with_audit_paths_and_ssh( + &repo, + isolated_store_paths("open-pr-base-precedence-audit")?, + test_ssh_command()?, + ) + .execute(&configured.plan, configured.context); + assert!(execution.succeeded(), "{}", execution.message()); + assert!( + std::fs::read_to_string(fake_gh.with_file_name("invocations"))? + .contains("--base release") + ); + + let explicit = planner + .plan_request(OperationRequest::OpenPullRequest { + base: Some("main".to_owned()), + }) + .map_err(std::io::Error::other)?; + assert_eq!( + explicit.plan.request, + OperationRequest::OpenPullRequest { + base: Some("main".to_owned()) + } + ); + assert!(explicit.plan.preview_text().contains("base: main")); + Ok(()) + } + #[cfg(unix)] #[test] fn private_https_pull_request_uses_authenticated_api_not_repository_credential_helpers() @@ -5642,10 +5707,12 @@ mod tests { ], )?; let fake_gh = fake_gh("open-pr-enterprise-https", false)?; + let mut config = AppConfig::default(); + config.pull_requests.default_base_branch = Some("release".to_owned()); let planner = OperationPlanner { repo_root: repo.clone(), github_executable: Some(fake_gh.clone()), - policy: EffectivePolicy::default(), + policy: EffectivePolicy::new(&config), ssh_executable: Some(test_ssh_command()?), }; @@ -5657,7 +5724,7 @@ mod tests { operation .plan .preview_text() - .contains("https://git.example.com/octo/repo/compare/main...feature/open-pr") + .contains("https://git.example.com/octo/repo/compare/release...feature/open-pr") ); let execution = PlanExecutor::with_audit_paths_and_ssh( &repo, @@ -5676,6 +5743,7 @@ mod tests { assert!(invocations.contains("--hostname git.example.com")); assert!(invocations.contains("pr create")); assert!(invocations.contains("--repo git.example.com/octo/repo")); + assert!(invocations.contains("--base release")); Ok(()) } @@ -5964,6 +6032,35 @@ mod tests { Ok(()) } + #[test] + fn missing_configured_pull_request_base_fails_closed_with_guidance() + -> Result<(), Box> { + let repo = pushed_branch_repo("open-pr-missing-configured-base")?; + let fake_gh = fake_gh("open-pr-missing-configured-base", false)?; + let mut config = AppConfig::default(); + config.pull_requests.default_base_branch = Some("missing".to_owned()); + let planner = OperationPlanner { + repo_root: repo, + github_executable: Some(fake_gh), + policy: EffectivePolicy::new(&config), + ssh_executable: Some(test_ssh_command()?), + }; + + let error = match planner.plan_request(OperationRequest::OpenPullRequest { base: None }) { + Ok(_) => { + return Err( + std::io::Error::other("missing configured base must fail closed").into(), + ); + } + Err(error) => error, + }; + + assert!(error.contains("base branch missing was not found")); + assert!(error.contains("pull-requests.default-base-branch")); + assert!(error.contains("open pr to ")); + Ok(()) + } + #[test] fn open_pull_request_execution_rejects_deleted_base_branch() -> Result<(), Box> { let repo = pushed_branch_repo("open-pr-deleted-base")?; @@ -7550,7 +7647,10 @@ mod tests { fn startup_policy_fails_closed_and_surfaces_malformed_config() -> Result<(), Box> { let paths = isolated_store_paths("invalid-startup-policy")?; let store = LocalStore::open(paths.clone())?; - std::fs::write(&store.paths().config_file, "not valid toml = [")?; + std::fs::write( + &store.paths().config_file, + "credential = \"super-secret-value\"\nnot valid toml = [", + )?; let startup = startup_policy_from_paths(&paths); @@ -7562,7 +7662,9 @@ mod tests { .is_some_and(|diagnostic| diagnostic.contains("is invalid")) ); let app = App::from_startup(startup); - assert!(details_text(&app).contains("safe fallback configuration")); + let details = details_text(&app); + assert!(details.contains("safe fallback configuration")); + assert!(!details.contains("super-secret-value")); Ok(()) } diff --git a/docs/configuration.md b/docs/configuration.md index 5313b9a..2e1f502 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -39,6 +39,39 @@ high = "explicit-confirmation" enabled = true ``` +## Examples + +Use a repository branch other than the provider default for pull requests opened +without an explicit target: + +```toml +[pull-requests] +default-base-branch = "develop" +``` + +For `open pr`, the target is selected in this order: an explicit prompt target +such as `open pr to release`, `default-base-branch`, then the GitHub repository's +default branch. The selected branch is checked against the upstream GitHub +repository during planning on both GitHub.com and GitHub Enterprise. If a +configured branch does not exist, the operation is blocked with guidance to fix +the setting or provide an explicit target. + +Add team-specific protected branches, require stronger confirmation, disable +selected operation families, and turn off prompt input: + +```toml +[policy] +additional-protected-branches = ["production", "stable/*"] +disabled-operations = ["rebase"] + +[policy.confirmation] +medium = "explicit-confirmation" +high = "blocked" + +[prompt] +enabled = false +``` + Setting `prompt.enabled` to `false` rejects prompt submissions. It does not alter operation policy, and the prompt configuration has no command, shell, or hook extension points; enabled prompts still produce only built-in typed plans. From df0a64fa06bb6bca0f7bffd503a2d1235684c18d Mon Sep 17 00:00:00 2001 From: Adrian Cosentino Date: Wed, 15 Jul 2026 17:58:39 -0400 Subject: [PATCH 02/10] fix: bind deferred pull request targets --- crates/bitbygit-tui/src/lib.rs | 526 ++++++++++++++++++++++++++++----- 1 file changed, 446 insertions(+), 80 deletions(-) diff --git a/crates/bitbygit-tui/src/lib.rs b/crates/bitbygit-tui/src/lib.rs index 0db2a6c..d8d471f 100644 --- a/crates/bitbygit-tui/src/lib.rs +++ b/crates/bitbygit-tui/src/lib.rs @@ -677,10 +677,17 @@ impl PreparedPromptSequence { struct QueuedPromptSequence { first: PreparedOperation, remaining_requests: Vec, + deferred_pull_request_targets: Vec>, policy_evaluations: Vec, sequence_policy_evaluations: Vec, } +#[derive(Debug, Clone, PartialEq, Eq)] +struct DeferredPullRequestTarget { + repository: GitHubRepository, + base: String, +} + #[derive(Debug, Clone, PartialEq, Eq)] struct GitHubRepository(String); @@ -708,12 +715,14 @@ impl QueuedPromptSequence { fn new( first: PreparedOperation, remaining_requests: Vec, + deferred_pull_request_targets: Vec>, policy_evaluations: Vec, sequence_policy_evaluations: Vec, ) -> Self { Self { first, remaining_requests, + deferred_pull_request_targets, policy_evaluations, sequence_policy_evaluations, } @@ -1658,13 +1667,23 @@ fn checkout_resulting_branch(target: &BranchTarget) -> Result { fn prompt_sequence_plan( requests: &[OperationRequest], first: &PreparedOperation, + deferred_pull_request_targets: &[Option], ) -> Result { let Some(first_step) = first.plan.first_step() else { return Err("Prompt sequence blocked: first step produced no visible plan.".to_owned()); }; let mut steps = vec![prompt_sequence_first_step(1, first_step)]; - for (index, request) in requests.iter().enumerate().skip(1) { - steps.push(prompt_sequence_deferred_step(index + 1, request)?); + for ((index, request), pull_request_target) in requests + .iter() + .enumerate() + .skip(1) + .zip(deferred_pull_request_targets) + { + steps.push(prompt_sequence_deferred_step( + index + 1, + request, + pull_request_target.as_ref(), + )?); } let confirmation_prompt = prompt_sequence_confirmation_prompt(&steps, requests.len()); Ok(OperationPlan::new( @@ -1692,8 +1711,18 @@ fn prompt_sequence_first_step(index: usize, step: &OperationStep) -> OperationSt fn prompt_sequence_deferred_step( index: usize, request: &OperationRequest, + pull_request_target: Option<&DeferredPullRequestTarget>, ) -> Result { - let preview = prompt_sequence_request_preview(request)?; + let mut preview = prompt_sequence_request_preview(request)?; + if let Some(target) = pull_request_target { + preview + .details + .retain(|detail| !detail.starts_with("base: ")); + preview + .details + .push(format!("repository: {}", target.repository.0)); + preview.details.push(format!("base: {}", target.base)); + } let mut step = OperationStep::new( preview.kind, sequence_step_risk(preview.risk_level), @@ -2085,6 +2114,8 @@ impl OperationPlanner { .ok() .and_then(|target| head_target_branch(&target).map(ToOwned::to_owned)) }); + let deferred_pull_request_targets = + self.deferred_pull_request_targets(&requests, branch.clone())?; let (policy_evaluations, sequence_policy_evaluations) = prompt_sequence_policy_evaluations(&self.policy, &git, &requests, branch)?; let first_request = requests @@ -2100,7 +2131,7 @@ impl OperationPlanner { } else { self.plan_request(first_request)? }; - let mut plan = prompt_sequence_plan(&requests, &first)?; + let mut plan = prompt_sequence_plan(&requests, &first, &deferred_pull_request_targets)?; apply_policy_evaluation( &mut plan, combined_policy_evaluation( @@ -2116,12 +2147,118 @@ impl OperationPlanner { QueuedPromptSequence::new( first, remaining_requests, + deferred_pull_request_targets, policy_evaluations, sequence_policy_evaluations, ), )) } + fn deferred_pull_request_targets( + &self, + requests: &[OperationRequest], + initial_branch: Option, + ) -> Result>, String> { + let git = self.git(); + let mut branch = initial_branch; + let mut targets = Vec::with_capacity(requests.len().saturating_sub(1)); + + for (index, request) in requests.iter().enumerate() { + if index > 0 { + let target = match request { + OperationRequest::OpenPullRequest { base } => { + let branch = branch.as_deref().ok_or_else(|| { + "Open pull request blocked: unable to resolve the deferred branch." + .to_owned() + })?; + Some(self.deferred_pull_request_target(&git, branch, base.as_deref())?) + } + _ => None, + }; + targets.push(target); + } + + match request { + OperationRequest::Checkout { branch: target } + if branch.as_deref() != Some(target) => + { + branch = Some(sequence_checkout_resulting_branch(&git, target)?); + } + OperationRequest::CreateBranch { branch: target, .. } => { + branch = Some(target.clone()); + } + _ => {} + } + } + + Ok(targets) + } + + fn deferred_pull_request_target( + &self, + git: &Git, + branch: &str, + requested_base: Option<&str>, + ) -> Result { + let remote = git + .push_target(branch) + .map_err(|error| format!("Unable to prepare pull request target: {error}"))? + .map(|(remote, _branch)| remote) + .or_else(|| self.default_remote_name()) + .ok_or_else(|| "Open pull request blocked: no remotes are configured.".to_owned())?; + let remote_urls = git + .remote_push_urls(&remote) + .map_err(|error| format!("Unable to prepare pull request remote: {error}"))?; + let push_url = single_pull_request_push_url(&remote, &remote_urls)?; + let head_repository = github_repository_from_push_url(push_url) + .map_err(|reason| format!("Open pull request blocked: remote {remote} {reason}"))?; + let github_repository = github_base_repository(git, &remote, &head_repository)?; + if !head_repository + .hostname() + .eq_ignore_ascii_case(github_repository.hostname()) + { + return Err( + "Open pull request blocked: source and upstream remotes use different GitHub hosts." + .to_owned(), + ); + } + let github = self.github(&github_repository); + let repository = github.repository().map_err(open_pull_request_gh_error)?; + let configured_base = + requested_base.is_none() && self.policy.default_pull_request_base().is_some(); + let base = requested_base + .map(ToOwned::to_owned) + .or_else(|| { + self.policy + .default_pull_request_base() + .map(ToOwned::to_owned) + }) + .unwrap_or_else(|| repository.default_branch.clone()); + if !github + .branch_exists(&base) + .map_err(open_pull_request_gh_error)? + { + let mut error = format!( + "Open pull request blocked: base branch {base} was not found in {}.", + repository.name_with_owner + ); + if configured_base { + error.push_str( + " Update pull-requests.default-base-branch or choose an existing branch with `open pr to `.", + ); + } + return Err(error); + } + + Ok(DeferredPullRequestTarget { + repository: GitHubRepository::new( + github_repository.hostname(), + &repository.name_with_owner, + ), + base, + }) + } + fn plan_stage_pathspecs(&self, paths: Vec) -> Result { self.ensure_operation_enabled(OperationKind::StagePaths)?; Ok(self.apply_policy(PreparedOperation::new( @@ -3320,6 +3457,24 @@ fn policy_branch(context: &ExecutionContext) -> Option<&str> { } } +fn prepared_pull_request_target( + operation: &PreparedOperation, +) -> Option { + let PendingPayload::OpenPullRequest { + base, + repository, + github_repository, + .. + } = operation.context.payload.as_ref()? + else { + return None; + }; + Some(DeferredPullRequestTarget { + repository: GitHubRepository::new(github_repository.hostname(), repository), + base: base.clone(), + }) +} + fn head_target_branch(target: &HeadTarget) -> Option<&str> { target.reference.as_deref()?.strip_prefix("refs/heads/") } @@ -4056,6 +4211,7 @@ impl PromptSequenceExecutor { let QueuedPromptSequence { first, remaining_requests, + deferred_pull_request_targets, policy_evaluations, sequence_policy_evaluations, } = sequence; @@ -4073,6 +4229,23 @@ impl PromptSequenceExecutor { .ok() .and_then(|status| branch_name(&status.branch).ok()) }); + let current_pull_request_targets = OperationPlanner { + repo_root: self.repo_root.clone(), + github_executable: self.github_executable.clone(), + policy: current_policy.clone(), + #[cfg(test)] + ssh_executable: self.ssh_executable.clone(), + } + .deferred_pull_request_targets(&requests, branch.clone()); + if current_pull_request_targets.as_ref() != Ok(&deferred_pull_request_targets) { + step_results.push(PromptSequenceStepResult::planning_failed( + 1, + first.plan.title.clone(), + "pull request target changed since the sequence preview; create a new sequence preview and confirmation" + .to_owned(), + )); + return PromptSequenceExecutionResult::new(total_steps, step_results, current_policy); + } let (current_policies, current_sequence_policies) = match prompt_sequence_policy_evaluations(¤t_policy, &git, &requests, branch) { Ok(evaluations) => evaluations, @@ -4108,6 +4281,7 @@ impl PromptSequenceExecutor { let mut previewed_policies = policy_evaluations.into_iter(); let mut previewed_sequence_policies = sequence_policy_evaluations.into_iter(); + let mut previewed_pull_request_targets = deferred_pull_request_targets.into_iter(); let (Some(previewed_policy), Some(previewed_sequence_policy)) = ( previewed_policies.next(), @@ -4158,6 +4332,18 @@ impl PromptSequenceExecutor { for (index, request) in remaining_requests.into_iter().enumerate() { let step_number = index + 2; + let Some(previewed_pull_request_target) = previewed_pull_request_targets.next() else { + step_results.push(PromptSequenceStepResult::planning_failed( + step_number, + prompt_sequence_request_title(&request), + "sequence preview is missing pull request target context".to_owned(), + )); + return PromptSequenceExecutionResult::new( + total_steps, + step_results, + current_policy, + ); + }; current_policy = self.load_policy(¤t_policy); let planner = OperationPlanner { repo_root: self.repo_root.clone(), @@ -4169,6 +4355,13 @@ impl PromptSequenceExecutor { let operation = match planner.plan_request(request.clone()) { Ok(operation) => operation, Err(error) => { + let error = if previewed_pull_request_target.is_some() { + format!( + "pull request target must be re-previewed and confirmed before execution: {error}" + ) + } else { + error + }; step_results.push(PromptSequenceStepResult::planning_failed( step_number, prompt_sequence_request_title(&request), @@ -4181,6 +4374,21 @@ impl PromptSequenceExecutor { ); } }; + if let Some(previewed_target) = previewed_pull_request_target { + if prepared_pull_request_target(&operation).as_ref() != Some(&previewed_target) { + step_results.push(PromptSequenceStepResult::planning_failed( + step_number, + operation.plan.title.clone(), + "pull request target changed since the sequence preview; create a new sequence preview and confirmation" + .to_owned(), + )); + return PromptSequenceExecutionResult::new( + total_steps, + step_results, + current_policy, + ); + } + } let (Some(previewed_policy), Some(previewed_sequence_policy)) = ( previewed_policies.next(), previewed_sequence_policies.next(), @@ -5217,23 +5425,27 @@ mod tests { #[test] fn prompt_sequence_stops_before_push_and_pr_when_commit_execution_fails() -> Result<(), Box> { - let repo = isolated_git_repo("prompt-sequence-commit-fails")?; - configure_git_identity(&repo)?; + let repo = pushed_branch_repo("prompt-sequence-commit-fails")?; + let fake_gh = fake_gh("prompt-sequence-commit-fails", false)?; std::fs::write(repo.join("file.txt"), "hello\n")?; git_stdout(&repo, &["add", "file.txt"])?; - let sequence = OperationPlanner::new(&repo) - .plan_prompt_sequence(vec![ - OperationRequest::Commit { - message: "ship staged".to_owned(), - }, - OperationRequest::Push, - OperationRequest::OpenPullRequest { base: None }, - ]) - .map_err(std::io::Error::other)?; + let sequence = OperationPlanner { + repo_root: repo.clone(), + github_executable: Some(fake_gh.clone()), + policy: EffectivePolicy::default(), + ssh_executable: Some(test_ssh_command()?), + } + .plan_prompt_sequence(vec![ + OperationRequest::Commit { + message: "ship staged".to_owned(), + }, + OperationRequest::Push, + OperationRequest::OpenPullRequest { base: None }, + ]) + .map_err(std::io::Error::other)?; std::fs::write(repo.join("file.txt"), "changed after preview\n")?; git_stdout(&repo, &["add", "file.txt"])?; let paths = isolated_store_paths("prompt-sequence-commit-fails-audit")?; - let fake_gh = fake_gh("prompt-sequence-commit-fails", false)?; let result = PromptSequenceExecutor::with_audit_paths_and_tools( &repo, @@ -5248,7 +5460,9 @@ mod tests { .message() .contains("Prompt sequence stopped after step 1 of 3.") ); - assert!(!fake_gh.with_file_name("invocations").exists()); + assert!( + !std::fs::read_to_string(fake_gh.with_file_name("invocations"))?.contains("pr:create") + ); let entries = LocalStore::open(paths)?.list_audit_entries()?; let operations = entries .iter() @@ -5289,7 +5503,9 @@ mod tests { .message() .contains("Prompt sequence stopped after step 1 of 2.") ); - assert!(!fake_gh.with_file_name("invocations").exists()); + assert!( + !std::fs::read_to_string(fake_gh.with_file_name("invocations"))?.contains("pr:create") + ); let entries = LocalStore::open(paths)?.list_audit_entries()?; let operations = entries .iter() @@ -5375,7 +5591,8 @@ mod tests { assert!(preview.contains("1. commit 1 staged file(s)")); assert!(preview.contains("2. push current branch")); assert!(preview.contains("3. open or surface pull request")); - assert!(preview.contains("base: repository default (resolved after push)")); + assert!(preview.contains("repository: github.com/octo/repo")); + assert!(preview.contains("base: main")); assert!(preview.contains("planned after step 2 succeeds")); let paths = isolated_store_paths("prompt-sequence-open-pr-audit")?; let result = @@ -5414,47 +5631,215 @@ mod tests { } #[test] - fn missing_gh_after_push_stops_pr_with_setup_guidance() -> Result<(), Box> { - let repo = pushed_branch_repo("prompt-sequence-missing-gh")?; + fn deferred_configured_pull_request_base_is_previewed_and_executed() + -> Result<(), Box> { + let repo = pushed_branch_repo("prompt-sequence-configured-pr-base")?; + let fake_gh = fake_gh("prompt-sequence-configured-pr-base", false)?; let ssh = test_ssh_command()?; - std::fs::write(repo.join("file.txt"), "push before setup check\n")?; - git_stdout(&repo, &["add", "file.txt"])?; - git_stdout(&repo, &["commit", "-m", "ahead"])?; - let missing_gh = repo.join("missing-gh"); + let mut config = AppConfig::default(); + config.pull_requests.default_base_branch = Some("release".to_owned()); + let policy = EffectivePolicy::new(&config); let planner = OperationPlanner { repo_root: repo.clone(), - github_executable: Some(missing_gh.clone()), - policy: EffectivePolicy::default(), + github_executable: Some(fake_gh.clone()), + policy: policy.clone(), ssh_executable: Some(ssh.clone()), }; + let sequence = planner .plan_prompt_sequence(vec![ - OperationRequest::Push, + OperationRequest::Fetch, OperationRequest::OpenPullRequest { base: None }, ]) .map_err(std::io::Error::other)?; - let paths = isolated_store_paths("prompt-sequence-missing-gh-audit")?; - let result = PromptSequenceExecutor::with_audit_paths_and_tools( - &repo, - paths.clone(), - missing_gh, - ssh.clone(), - ) + let preview = sequence.plan.preview_text(); + assert!(preview.contains("repository: github.com/octo/repo")); + assert!(preview.contains("base: release")); + assert!(!preview.contains("repository default")); + assert_eq!( + sequence.sequence.remaining_requests, + vec![OperationRequest::OpenPullRequest { base: None }] + ); + let paths = isolated_store_paths("prompt-sequence-configured-pr-base-audit")?; + let store = LocalStore::open(paths.clone())?; + std::fs::write( + &store.paths().config_file, + "schema-version = 1\n[pull-requests]\ndefault-base-branch = \"release\"\n", + )?; + let result = PromptSequenceExecutor { + repo_root: repo, + audit: AuditDestination::Paths(paths), + github_executable: Some(fake_gh.clone()), + policy, + ssh_executable: Some(ssh), + } + .execute(sequence.sequence); + + assert!( + result + .message() + .contains("Prompt sequence completed 2 step(s)."), + "{}", + result.message() + ); + assert!( + std::fs::read_to_string(fake_gh.with_file_name("invocations"))? + .contains("--base release") + ); + Ok(()) + } + + #[test] + fn deferred_pull_request_stops_when_config_changes_effective_base() -> Result<(), Box> + { + let repo = pushed_branch_repo("prompt-sequence-changed-pr-base")?; + let fake_gh = fake_gh("prompt-sequence-changed-pr-base", false)?; + let paths = isolated_store_paths("prompt-sequence-changed-pr-base-audit")?; + let store = LocalStore::open(paths.clone())?; + std::fs::write( + &store.paths().config_file, + "schema-version = 1\n[pull-requests]\ndefault-base-branch = \"release\"\n", + )?; + let policy = EffectivePolicy::new(&store.load_config().settings); + let ssh_root = isolated_temp_root("prompt-sequence-changed-pr-base-ssh")?; + std::fs::create_dir_all(&ssh_root)?; + let ssh = ssh_root.join("ssh"); + std::fs::write( + &ssh, + format!( + "#!/bin/sh\nprintf '%s\\n' 'schema-version = 1' '[pull-requests]' 'default-base-branch = \"main\"' > '{}'\ntarget=$(git config --get-regexp '^remote\\..*\\.testbare$' | cut -d' ' -f2-)\nexec git-upload-pack \"$target\"\n", + store.paths().config_file.display() + ), + )?; + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + let mut permissions = std::fs::metadata(&ssh)?.permissions(); + permissions.set_mode(0o755); + std::fs::set_permissions(&ssh, permissions)?; + } + let planner = OperationPlanner { + repo_root: repo.clone(), + github_executable: Some(fake_gh.clone()), + policy: policy.clone(), + ssh_executable: Some(ssh.clone()), + }; + let sequence = planner + .plan_prompt_sequence(vec![ + OperationRequest::Fetch, + OperationRequest::OpenPullRequest { base: None }, + ]) + .map_err(std::io::Error::other)?; + assert!(sequence.plan.preview_text().contains("base: release")); + + let result = PromptSequenceExecutor { + repo_root: repo, + audit: AuditDestination::Paths(paths), + github_executable: Some(fake_gh.clone()), + policy, + ssh_executable: Some(ssh), + } .execute(sequence.sequence); let message = result.message(); - assert!(message.contains("Prompt sequence stopped before step 2 of 2.")); - assert!(message.contains(bitbygit_gh::INSTALL_GH_GUIDANCE)); - assert_remote_matches_head(&repo, &ssh)?; - let entries = LocalStore::open(paths)?.list_audit_entries()?; - assert_eq!(entries.len(), 2); - assert!(entries.iter().all(|entry| entry.operation == "push")); + assert!( + message.contains("Prompt sequence stopped before step 2 of 2."), + "{message}" + ); + assert!( + message.contains("pull request target changed since the sequence preview"), + "{message}" + ); + assert!(message.contains("new sequence preview and confirmation")); + assert!( + !std::fs::read_to_string(fake_gh.with_file_name("invocations"))?.contains("pr:create") + ); Ok(()) } #[test] - fn gh_auth_failure_after_push_stops_pr_with_setup_guidance() -> Result<(), Box> { + fn deferred_explicit_pull_request_base_keeps_enterprise_parity() -> Result<(), Box> { + let repo = pushed_branch_repo("prompt-sequence-enterprise-explicit-base")?; + git_stdout( + &repo, + &[ + "remote", + "set-url", + "origin", + "https://git.example.com/octo/repo.git", + ], + )?; + let fake_gh = fake_gh("prompt-sequence-enterprise-explicit-base", false)?; + let mut config = AppConfig::default(); + config.pull_requests.default_base_branch = Some("release".to_owned()); + let sequence = OperationPlanner { + repo_root: repo, + github_executable: Some(fake_gh), + policy: EffectivePolicy::new(&config), + ssh_executable: Some(test_ssh_command()?), + } + .plan_prompt_sequence(vec![ + OperationRequest::Branches, + OperationRequest::OpenPullRequest { + base: Some("main".to_owned()), + }, + ]) + .map_err(std::io::Error::other)?; + + let preview = sequence.plan.preview_text(); + assert!(preview.contains("repository: git.example.com/octo/repo")); + assert!(preview.contains("base: main")); + assert!(!preview.contains("base: release")); + assert_eq!( + sequence.sequence.remaining_requests, + vec![OperationRequest::OpenPullRequest { + base: Some("main".to_owned()) + }] + ); + Ok(()) + } + + #[test] + fn missing_gh_blocks_sequence_before_push_with_setup_guidance() -> Result<(), Box> { + let repo = pushed_branch_repo("prompt-sequence-missing-gh")?; + let ssh = test_ssh_command()?; + std::fs::write(repo.join("file.txt"), "push before setup check\n")?; + git_stdout(&repo, &["add", "file.txt"])?; + git_stdout(&repo, &["commit", "-m", "ahead"])?; + let missing_gh = repo.join("missing-gh"); + let planner = OperationPlanner { + repo_root: repo.clone(), + github_executable: Some(missing_gh.clone()), + policy: EffectivePolicy::default(), + ssh_executable: Some(ssh.clone()), + }; + let error = match planner.plan_prompt_sequence(vec![ + OperationRequest::Push, + OperationRequest::OpenPullRequest { base: None }, + ]) { + Ok(_) => return Err(std::io::Error::other("missing gh must block preview").into()), + Err(error) => error, + }; + + assert!(error.contains(bitbygit_gh::INSTALL_GH_GUIDANCE)); + let branch = git_stdout(&repo, &["branch", "--show-current"])?; + let remote = git_stdout_with_ssh( + &repo, + &[ + "ls-remote", + "origin", + &format!("refs/heads/{}", branch.trim()), + ], + &ssh, + )?; + assert!(!remote.starts_with(git_stdout(&repo, &["rev-parse", "HEAD"])?.trim())); + Ok(()) + } + + #[test] + fn gh_auth_failure_blocks_sequence_before_push_with_setup_guidance() + -> Result<(), Box> { let repo = pushed_branch_repo("prompt-sequence-gh-auth")?; let fake_gh = fake_gh("prompt-sequence-gh-auth", false)?; std::fs::write(fake_gh.with_file_name("auth-fails"), "")?; @@ -5468,33 +5853,30 @@ mod tests { policy: EffectivePolicy::default(), ssh_executable: Some(ssh.clone()), }; - let sequence = planner - .plan_prompt_sequence(vec![ - OperationRequest::Push, - OperationRequest::OpenPullRequest { base: None }, - ]) - .map_err(std::io::Error::other)?; - let paths = isolated_store_paths("prompt-sequence-gh-auth-audit")?; + let error = match planner.plan_prompt_sequence(vec![ + OperationRequest::Push, + OperationRequest::OpenPullRequest { base: None }, + ]) { + Ok(_) => return Err(std::io::Error::other("gh auth must block preview").into()), + Err(error) => error, + }; - let result = PromptSequenceExecutor::with_audit_paths_and_tools( + assert!(error.contains("GitHub CLI is not authenticated for github.com")); + assert!(error.contains("gh auth login --hostname github.com")); + let branch = git_stdout(&repo, &["branch", "--show-current"])?; + let remote = git_stdout_with_ssh( &repo, - paths.clone(), - &fake_gh, - ssh.clone(), - ) - .execute(sequence.sequence); - - let message = result.message(); - assert!(message.contains("Prompt sequence stopped before step 2 of 2.")); - assert!(message.contains("GitHub CLI is not authenticated for github.com")); - assert!(message.contains("gh auth login --hostname github.com")); - assert_remote_matches_head(&repo, &ssh)?; + &[ + "ls-remote", + "origin", + &format!("refs/heads/{}", branch.trim()), + ], + &ssh, + )?; + assert!(!remote.starts_with(git_stdout(&repo, &["rev-parse", "HEAD"])?.trim())); let invocations = std::fs::read_to_string(fake_gh.with_file_name("invocations"))?; assert!(invocations.contains("auth:status")); assert!(!invocations.contains("pr:create")); - let entries = LocalStore::open(paths)?.list_audit_entries()?; - assert_eq!(entries.len(), 2); - assert!(entries.iter().all(|entry| entry.operation == "push")); Ok(()) } @@ -9007,22 +9389,6 @@ mod tests { Ok(String::from_utf8(output.stdout)?) } - fn assert_remote_matches_head( - repo: &std::path::Path, - ssh_executable: &std::path::Path, - ) -> Result<(), Box> { - let branch = git_stdout(repo, &["branch", "--show-current"])?; - let remote_ref = format!("refs/heads/{}", branch.trim()); - let remote = git_stdout_with_ssh( - repo, - &["ls-remote", "origin", remote_ref.as_str()], - ssh_executable, - )?; - let head = git_stdout(repo, &["rev-parse", "HEAD"])?; - assert!(remote.starts_with(head.trim())); - Ok(()) - } - fn test_ssh_command() -> Result> { use std::os::unix::fs::PermissionsExt; use std::sync::OnceLock; From 9a64da93d1404a2444b9abeb691a5ea33e6f2865 Mon Sep 17 00:00:00 2001 From: Adrian Cosentino Date: Wed, 15 Jul 2026 18:12:20 -0400 Subject: [PATCH 03/10] fix: preflight remote checkout PR targets --- crates/bitbygit-tui/src/lib.rs | 215 ++++++++++++++++++++++++++++++--- 1 file changed, 196 insertions(+), 19 deletions(-) diff --git a/crates/bitbygit-tui/src/lib.rs b/crates/bitbygit-tui/src/lib.rs index d8d471f..fc1af53 100644 --- a/crates/bitbygit-tui/src/lib.rs +++ b/crates/bitbygit-tui/src/lib.rs @@ -685,9 +685,17 @@ struct QueuedPromptSequence { #[derive(Debug, Clone, PartialEq, Eq)] struct DeferredPullRequestTarget { repository: GitHubRepository, + head_repository: GitHubRepository, + head: String, base: String, } +#[derive(Debug, Clone, PartialEq, Eq)] +struct SequenceBranch { + name: String, + push_target: Option<(String, String)>, +} + #[derive(Debug, Clone, PartialEq, Eq)] struct GitHubRepository(String); @@ -1715,13 +1723,23 @@ fn prompt_sequence_deferred_step( ) -> Result { let mut preview = prompt_sequence_request_preview(request)?; if let Some(target) = pull_request_target { - preview - .details - .retain(|detail| !detail.starts_with("base: ")); + preview.details.retain(|detail| { + !detail.starts_with("base: ") && !detail.starts_with("re-plan branch, remote, base") + }); preview .details .push(format!("repository: {}", target.repository.0)); + preview.details.push(format!("head: {}", target.head)); preview.details.push(format!("base: {}", target.base)); + preview.details.push(format!( + "target: https://{}/compare/{}...{}?expand=1", + target.repository.0, + compare_url_ref(&target.base), + compare_url_ref(&target.head) + )); + preview.details.push( + "revalidate the bound provider, repository, head, and base before execution".to_owned(), + ); } let mut step = OperationStep::new( preview.kind, @@ -2160,14 +2178,17 @@ impl OperationPlanner { initial_branch: Option, ) -> Result>, String> { let git = self.git(); - let mut branch = initial_branch; + let mut branch = initial_branch.map(|name| SequenceBranch { + name, + push_target: None, + }); let mut targets = Vec::with_capacity(requests.len().saturating_sub(1)); for (index, request) in requests.iter().enumerate() { if index > 0 { let target = match request { OperationRequest::OpenPullRequest { base } => { - let branch = branch.as_deref().ok_or_else(|| { + let branch = branch.as_ref().ok_or_else(|| { "Open pull request blocked: unable to resolve the deferred branch." .to_owned() })?; @@ -2180,12 +2201,46 @@ impl OperationPlanner { match request { OperationRequest::Checkout { branch: target } - if branch.as_deref() != Some(target) => + if branch.as_ref().map(|branch| &branch.name) != Some(target) => { - branch = Some(sequence_checkout_resulting_branch(&git, target)?); + let target = git + .branch_target(target) + .map_err(|error| format!("Unable to resolve sequence checkout: {error}"))? + .ok_or_else(|| { + format!( + "Prompt sequence blocked: checkout target {target} cannot be validated during preflight; create a new sequence preview." + ) + })?; + git.ensure_remote_checkout_target_available(&target) + .map_err(|error| { + format!( + "Prompt sequence blocked: checkout target cannot be validated during preflight: {error}; create a new sequence preview." + ) + })?; + let name = checkout_resulting_branch(&target)?; + let push_target = if target.kind == BranchKind::Remote { + Some( + target + .name + .split_once('/') + .map(|(remote, branch)| (remote.to_owned(), branch.to_owned())) + .ok_or_else(|| { + format!( + "Prompt sequence blocked: remote checkout target {} cannot be bound during preflight; create a new sequence preview.", + target.name + ) + })?, + ) + } else { + None + }; + branch = Some(SequenceBranch { name, push_target }); } OperationRequest::CreateBranch { branch: target, .. } => { - branch = Some(target.clone()); + branch = Some(SequenceBranch { + name: target.clone(), + push_target: None, + }); } _ => {} } @@ -2197,15 +2252,22 @@ impl OperationPlanner { fn deferred_pull_request_target( &self, git: &Git, - branch: &str, + branch: &SequenceBranch, requested_base: Option<&str>, ) -> Result { - let remote = git - .push_target(branch) - .map_err(|error| format!("Unable to prepare pull request target: {error}"))? - .map(|(remote, _branch)| remote) - .or_else(|| self.default_remote_name()) - .ok_or_else(|| "Open pull request blocked: no remotes are configured.".to_owned())?; + let (remote, upstream_branch) = match &branch.push_target { + Some(target) => target.clone(), + None => git + .push_target(&branch.name) + .map_err(|error| format!("Unable to prepare pull request target: {error}"))? + .or_else(|| { + self.default_remote_name() + .map(|remote| (remote, branch.name.clone())) + }) + .ok_or_else(|| { + "Open pull request blocked: no remotes are configured.".to_owned() + })?, + }; let remote_urls = git .remote_push_urls(&remote) .map_err(|error| format!("Unable to prepare pull request remote: {error}"))?; @@ -2224,6 +2286,23 @@ impl OperationPlanner { } let github = self.github(&github_repository); let repository = github.repository().map_err(open_pull_request_gh_error)?; + let canonical_github_repository = + GitHubRepository::new(github_repository.hostname(), &repository.name_with_owner); + let canonical_head_repository = if head_repository.eq_ignore_ascii_case(&github_repository) + { + canonical_github_repository.clone() + } else { + let head = self + .github(&head_repository) + .repository() + .map_err(open_pull_request_gh_error)?; + GitHubRepository::new(head_repository.hostname(), &head.name_with_owner) + }; + let head = pull_request_head( + &canonical_head_repository, + &canonical_github_repository, + &upstream_branch, + )?; let configured_base = requested_base.is_none() && self.policy.default_pull_request_base().is_some(); let base = requested_base @@ -2251,10 +2330,9 @@ impl OperationPlanner { } Ok(DeferredPullRequestTarget { - repository: GitHubRepository::new( - github_repository.hostname(), - &repository.name_with_owner, - ), + repository: canonical_github_repository, + head_repository: canonical_head_repository, + head, base, }) } @@ -3464,6 +3542,9 @@ fn prepared_pull_request_target( base, repository, github_repository, + head_github_repository, + head_repository, + head, .. } = operation.context.payload.as_ref()? else { @@ -3471,6 +3552,8 @@ fn prepared_pull_request_target( }; Some(DeferredPullRequestTarget { repository: GitHubRepository::new(github_repository.hostname(), repository), + head_repository: GitHubRepository::new(head_github_repository.hostname(), head_repository), + head: head.clone(), base: base.clone(), }) } @@ -5630,6 +5713,100 @@ mod tests { Ok(()) } + #[test] + fn remote_checkout_push_pr_revalidates_target_before_any_step() -> Result<(), Box> { + let repo = isolated_git_repo("prompt-sequence-remote-checkout-pr")?; + let fork = isolated_bare_git_repo("prompt-sequence-remote-checkout-pr-fork")?; + configure_git_identity(&repo)?; + std::fs::write(repo.join("file.txt"), "base\n")?; + git_stdout(&repo, &["add", "file.txt"])?; + git_stdout(&repo, &["commit", "-m", "initial"])?; + let original_branch = git_stdout(&repo, &["branch", "--show-current"])?; + git_stdout( + &repo, + &[ + "remote", + "add", + "origin", + "ssh://git@github.com/upstream/repo.git", + ], + )?; + git_stdout( + &repo, + &[ + "remote", + "add", + "upstream", + "ssh://git@github.com/upstream/repo.git", + ], + )?; + add_github_remote(&repo, "fork", &fork)?; + git_stdout(&repo, &["update-ref", "refs/remotes/fork/topic", "HEAD"])?; + let fake_gh = fake_gh("prompt-sequence-remote-checkout-pr", false)?; + let ssh = test_ssh_command()?; + let planner = OperationPlanner { + repo_root: repo.clone(), + github_executable: Some(fake_gh.clone()), + policy: EffectivePolicy::default(), + ssh_executable: Some(ssh.clone()), + }; + let sequence = planner + .plan_prompt_sequence(vec![ + OperationRequest::Checkout { + branch: "fork/topic".to_owned(), + }, + OperationRequest::Push, + OperationRequest::OpenPullRequest { base: None }, + ]) + .map_err(std::io::Error::other)?; + + let preview = sequence.plan.preview_text(); + assert!(preview.contains("repository: github.com/upstream/repo")); + assert!(preview.contains("head: octo:topic")); + assert!(preview.contains( + "target: https://github.com/upstream/repo/compare/main...octo%3Atopic?expand=1" + )); + git_stdout( + &repo, + &[ + "remote", + "set-url", + "fork", + "ssh://git@github.com/bob/repo.git", + ], + )?; + let paths = isolated_store_paths("prompt-sequence-remote-checkout-pr-audit")?; + + let result = + PromptSequenceExecutor::with_audit_paths_and_tools(&repo, paths.clone(), &fake_gh, ssh) + .execute(sequence.sequence); + + let message = result.message(); + assert!( + message.contains("Prompt sequence stopped before step 1 of 3."), + "{message}" + ); + assert!( + message.contains("pull request target changed since the sequence preview"), + "{message}" + ); + assert_eq!( + git_stdout(&repo, &["branch", "--show-current"])?, + original_branch + ); + assert!(LocalStore::open(paths)?.list_audit_entries()?.is_empty()); + assert!( + !std::process::Command::new("git") + .arg("--git-dir") + .arg(&fork) + .args(["show-ref", "--verify", "--quiet", "refs/heads/topic"]) + .status()? + .success(), + "push must not run before the changed pull request target is rejected" + ); + Ok(()) + } + #[test] fn deferred_configured_pull_request_base_is_previewed_and_executed() -> Result<(), Box> { From d9e8fd35b54af3ad5c443aadad94653e7bac31b9 Mon Sep 17 00:00:00 2001 From: Adrian Cosentino Date: Wed, 15 Jul 2026 18:17:12 -0400 Subject: [PATCH 04/10] fix: bind prospective PR push targets --- crates/bitbygit-git/src/lib.rs | 72 +++++++++++++++++++++ crates/bitbygit-tui/src/lib.rs | 113 +++++++++++++++++++++++---------- 2 files changed, 150 insertions(+), 35 deletions(-) diff --git a/crates/bitbygit-git/src/lib.rs b/crates/bitbygit-git/src/lib.rs index 8a5ab6f..6e94055 100644 --- a/crates/bitbygit-git/src/lib.rs +++ b/crates/bitbygit-git/src/lib.rs @@ -458,6 +458,36 @@ impl Git { Ok(Some((remote.to_owned(), push_branch.to_owned()))) } + pub fn prospective_push_target( + &self, + branch: &str, + upstream_remote: &str, + upstream_branch: &str, + ) -> Result, GitError> { + let remote = self + .config_value(["config", "--get", &format!("branch.{branch}.pushRemote")])? + .or(self.config_value(["config", "--get", "remote.pushDefault"])?) + .unwrap_or_else(|| upstream_remote.to_owned()); + if remote.is_empty() { + return Ok(None); + } + let push_default = self + .config_value(["config", "--get", "push.default"])? + .unwrap_or_else(|| "simple".to_owned()); + let target = match push_default.as_str() { + "simple" if remote != upstream_remote => (remote, branch.to_owned()), + "simple" | "upstream" => (upstream_remote.to_owned(), upstream_branch.to_owned()), + "current" | "matching" => (remote, branch.to_owned()), + "nothing" => return Ok(None), + _ => { + return Err(GitError::Blocked { + message: format!("push.default has unsupported value {push_default}"), + }); + } + }; + Ok(Some(target)) + } + pub fn remote_tracking_oid( &self, remote: &str, @@ -3219,6 +3249,48 @@ mod tests { Ok(()) } + #[test] + fn prospective_push_target_matches_configured_git_push_target() -> Result<(), Box> { + let repo = TempRepo::new()?; + repo.run(["init", "-b", "topic"])?; + repo.run(["config", "user.email", "bitbygit@example.invalid"])?; + repo.run(["config", "user.name", "bitbygit test"])?; + repo.run(["commit", "--allow-empty", "-m", "initial"])?; + repo.run(["remote", "add", "fork", "https://github.com/octo/repo.git"])?; + repo.run([ + "remote", + "add", + "origin", + "https://github.com/upstream/repo.git", + ])?; + repo.run(["config", "branch.topic.remote", "fork"])?; + repo.run(["config", "branch.topic.merge", "refs/heads/topic"])?; + let git = Git::new(repo.path()); + + assert_eq!( + git.prospective_push_target("topic", "fork", "topic")?, + git.push_target("topic")? + ); + + repo.run(["config", "remote.pushDefault", "origin"])?; + for push_default in ["simple", "current", "upstream", "matching", "nothing"] { + repo.run(["config", "push.default", push_default])?; + assert_eq!( + git.prospective_push_target("topic", "fork", "topic")?, + git.push_target("topic")?, + "push.default={push_default}" + ); + } + + repo.run(["config", "branch.topic.pushRemote", "fork"])?; + repo.run(["config", "push.default", "current"])?; + assert_eq!( + git.prospective_push_target("topic", "fork", "topic")?, + git.push_target("topic")? + ); + Ok(()) + } + #[cfg(unix)] #[test] fn remote_url_head_oid_ignores_configured_ssh_command() -> Result<(), Box> { diff --git a/crates/bitbygit-tui/src/lib.rs b/crates/bitbygit-tui/src/lib.rs index fc1af53..96aadef 100644 --- a/crates/bitbygit-tui/src/lib.rs +++ b/crates/bitbygit-tui/src/lib.rs @@ -693,7 +693,14 @@ struct DeferredPullRequestTarget { #[derive(Debug, Clone, PartialEq, Eq)] struct SequenceBranch { name: String, - push_target: Option<(String, String)>, + upstream: SequenceUpstream, +} + +#[derive(Debug, Clone, PartialEq, Eq)] +enum SequenceUpstream { + Existing, + Missing, + Bound { remote: String, branch: String }, } #[derive(Debug, Clone, PartialEq, Eq)] @@ -2180,7 +2187,7 @@ impl OperationPlanner { let git = self.git(); let mut branch = initial_branch.map(|name| SequenceBranch { name, - push_target: None, + upstream: SequenceUpstream::Existing, }); let mut targets = Vec::with_capacity(requests.len().saturating_sub(1)); @@ -2199,6 +2206,12 @@ impl OperationPlanner { targets.push(target); } + if !requests[index + 1..] + .iter() + .any(|request| matches!(request, OperationRequest::OpenPullRequest { .. })) + { + continue; + } match request { OperationRequest::Checkout { branch: target } if branch.as_ref().map(|branch| &branch.name) != Some(target) => @@ -2218,30 +2231,54 @@ impl OperationPlanner { ) })?; let name = checkout_resulting_branch(&target)?; - let push_target = if target.kind == BranchKind::Remote { - Some( - target - .name - .split_once('/') - .map(|(remote, branch)| (remote.to_owned(), branch.to_owned())) - .ok_or_else(|| { - format!( - "Prompt sequence blocked: remote checkout target {} cannot be bound during preflight; create a new sequence preview.", - target.name - ) - })?, - ) + let upstream = if target.kind == BranchKind::Remote { + let (remote, branch) = target.name.split_once('/').ok_or_else(|| { + format!( + "Prompt sequence blocked: remote checkout target {} cannot be bound during preflight; create a new sequence preview.", + target.name + ) + })?; + SequenceUpstream::Bound { + remote: remote.to_owned(), + branch: branch.to_owned(), + } } else { - None + SequenceUpstream::Existing }; - branch = Some(SequenceBranch { name, push_target }); + branch = Some(SequenceBranch { name, upstream }); } OperationRequest::CreateBranch { branch: target, .. } => { branch = Some(SequenceBranch { name: target.clone(), - push_target: None, + upstream: SequenceUpstream::Missing, }); } + OperationRequest::Push => { + let branch = branch.as_mut().ok_or_else(|| { + "Prompt sequence blocked: push branch cannot be resolved during preflight; create a new sequence preview." + .to_owned() + })?; + let missing_upstream = match branch.upstream { + SequenceUpstream::Missing => true, + SequenceUpstream::Existing => git + .upstream_push_target(&branch.name) + .map_err(|error| { + format!("Unable to resolve sequence push target: {error}") + })? + .is_none(), + SequenceUpstream::Bound { .. } => false, + }; + if missing_upstream { + let remote = self.default_remote_name().ok_or_else(|| { + "Prompt sequence blocked: push target cannot be resolved during preflight; create a new sequence preview." + .to_owned() + })?; + branch.upstream = SequenceUpstream::Bound { + remote, + branch: branch.name.clone(), + }; + } + } _ => {} } } @@ -2255,17 +2292,31 @@ impl OperationPlanner { branch: &SequenceBranch, requested_base: Option<&str>, ) -> Result { - let (remote, upstream_branch) = match &branch.push_target { - Some(target) => target.clone(), - None => git + let (remote, upstream_branch) = match &branch.upstream { + SequenceUpstream::Existing => git .push_target(&branch.name) .map_err(|error| format!("Unable to prepare pull request target: {error}"))? - .or_else(|| { - self.default_remote_name() - .map(|remote| (remote, branch.name.clone())) - }) .ok_or_else(|| { - "Open pull request blocked: no remotes are configured.".to_owned() + "Open pull request blocked: the deferred branch has no resolvable upstream; add `push` before `open pr` and create a new sequence preview." + .to_owned() + })?, + SequenceUpstream::Missing => { + return Err( + "Open pull request blocked: the deferred branch will not have an upstream; add `push` before `open pr` and create a new sequence preview." + .to_owned(), + ); + } + SequenceUpstream::Bound { + remote, + branch: upstream_branch, + } => git + .prospective_push_target(&branch.name, remote, upstream_branch) + .map_err(|error| { + format!("Unable to prepare deferred pull request target: {error}") + })? + .ok_or_else(|| { + "Open pull request blocked: the deferred push target cannot be bound during preflight; update push.default and create a new sequence preview." + .to_owned() })?, }; let remote_urls = git @@ -5766,15 +5817,7 @@ mod tests { assert!(preview.contains( "target: https://github.com/upstream/repo/compare/main...octo%3Atopic?expand=1" )); - git_stdout( - &repo, - &[ - "remote", - "set-url", - "fork", - "ssh://git@github.com/bob/repo.git", - ], - )?; + git_stdout(&repo, &["config", "remote.pushDefault", "origin"])?; let paths = isolated_store_paths("prompt-sequence-remote-checkout-pr-audit")?; let result = From b6cc7e275a0252e0d94dd7841cb817584bd459a5 Mon Sep 17 00:00:00 2001 From: Adrian Cosentino Date: Wed, 15 Jul 2026 18:51:12 -0400 Subject: [PATCH 05/10] fix: share typed push target resolution --- crates/bitbygit-git/src/lib.rs | 28 +- crates/bitbygit-tui/src/lib.rs | 505 ++++++++++++++++++++++++++------- 2 files changed, 417 insertions(+), 116 deletions(-) diff --git a/crates/bitbygit-git/src/lib.rs b/crates/bitbygit-git/src/lib.rs index 6e94055..81de4de 100644 --- a/crates/bitbygit-git/src/lib.rs +++ b/crates/bitbygit-git/src/lib.rs @@ -458,19 +458,26 @@ impl Git { Ok(Some((remote.to_owned(), push_branch.to_owned()))) } - pub fn prospective_push_target( + pub fn typed_push_target( &self, branch: &str, - upstream_remote: &str, - upstream_branch: &str, + upstream: Option<(&str, &str)>, + default_remote: Option<&str>, ) -> Result, GitError> { let remote = self .config_value(["config", "--get", &format!("branch.{branch}.pushRemote")])? .or(self.config_value(["config", "--get", "remote.pushDefault"])?) - .unwrap_or_else(|| upstream_remote.to_owned()); + .or_else(|| upstream.map(|(remote, _)| remote.to_owned())) + .or_else(|| default_remote.map(ToOwned::to_owned)); + let Some(remote) = remote else { + return Ok(None); + }; if remote.is_empty() { return Ok(None); } + let Some((upstream_remote, upstream_branch)) = upstream else { + return Ok(Some((remote, branch.to_owned()))); + }; let push_default = self .config_value(["config", "--get", "push.default"])? .unwrap_or_else(|| "simple".to_owned()); @@ -3250,7 +3257,7 @@ mod tests { } #[test] - fn prospective_push_target_matches_configured_git_push_target() -> Result<(), Box> { + fn typed_push_target_resolves_tracked_and_new_branches() -> Result<(), Box> { let repo = TempRepo::new()?; repo.run(["init", "-b", "topic"])?; repo.run(["config", "user.email", "bitbygit@example.invalid"])?; @@ -3268,7 +3275,7 @@ mod tests { let git = Git::new(repo.path()); assert_eq!( - git.prospective_push_target("topic", "fork", "topic")?, + git.typed_push_target("topic", Some(("fork", "topic")), Some("origin"))?, git.push_target("topic")? ); @@ -3276,7 +3283,7 @@ mod tests { for push_default in ["simple", "current", "upstream", "matching", "nothing"] { repo.run(["config", "push.default", push_default])?; assert_eq!( - git.prospective_push_target("topic", "fork", "topic")?, + git.typed_push_target("topic", Some(("fork", "topic")), Some("fork"))?, git.push_target("topic")?, "push.default={push_default}" ); @@ -3285,9 +3292,14 @@ mod tests { repo.run(["config", "branch.topic.pushRemote", "fork"])?; repo.run(["config", "push.default", "current"])?; assert_eq!( - git.prospective_push_target("topic", "fork", "topic")?, + git.typed_push_target("topic", Some(("fork", "topic")), Some("origin"))?, git.push_target("topic")? ); + repo.run(["config", "branch.new.pushRemote", "fork"])?; + assert_eq!( + git.typed_push_target("new", None, Some("origin"))?, + Some(("fork".to_owned(), "new".to_owned())) + ); Ok(()) } diff --git a/crates/bitbygit-tui/src/lib.rs b/crates/bitbygit-tui/src/lib.rs index 96aadef..ff25c69 100644 --- a/crates/bitbygit-tui/src/lib.rs +++ b/crates/bitbygit-tui/src/lib.rs @@ -703,6 +703,13 @@ enum SequenceUpstream { Bound { remote: String, branch: String }, } +#[derive(Debug, Clone, PartialEq, Eq)] +struct TypedPushTarget { + remote: String, + branch: String, + sets_upstream: bool, +} + #[derive(Debug, Clone, PartialEq, Eq)] struct GitHubRepository(String); @@ -2258,24 +2265,11 @@ impl OperationPlanner { "Prompt sequence blocked: push branch cannot be resolved during preflight; create a new sequence preview." .to_owned() })?; - let missing_upstream = match branch.upstream { - SequenceUpstream::Missing => true, - SequenceUpstream::Existing => git - .upstream_push_target(&branch.name) - .map_err(|error| { - format!("Unable to resolve sequence push target: {error}") - })? - .is_none(), - SequenceUpstream::Bound { .. } => false, - }; - if missing_upstream { - let remote = self.default_remote_name().ok_or_else(|| { - "Prompt sequence blocked: push target cannot be resolved during preflight; create a new sequence preview." - .to_owned() - })?; + let target = self.typed_push_target(&git, &branch.name, &branch.upstream)?; + if target.sets_upstream { branch.upstream = SequenceUpstream::Bound { - remote, - branch: branch.name.clone(), + remote: target.remote, + branch: target.branch, }; } } @@ -2292,33 +2286,18 @@ impl OperationPlanner { branch: &SequenceBranch, requested_base: Option<&str>, ) -> Result { - let (remote, upstream_branch) = match &branch.upstream { - SequenceUpstream::Existing => git - .push_target(&branch.name) - .map_err(|error| format!("Unable to prepare pull request target: {error}"))? - .ok_or_else(|| { - "Open pull request blocked: the deferred branch has no resolvable upstream; add `push` before `open pr` and create a new sequence preview." - .to_owned() - })?, - SequenceUpstream::Missing => { - return Err( - "Open pull request blocked: the deferred branch will not have an upstream; add `push` before `open pr` and create a new sequence preview." - .to_owned(), - ); - } - SequenceUpstream::Bound { - remote, - branch: upstream_branch, - } => git - .prospective_push_target(&branch.name, remote, upstream_branch) - .map_err(|error| { - format!("Unable to prepare deferred pull request target: {error}") - })? - .ok_or_else(|| { - "Open pull request blocked: the deferred push target cannot be bound during preflight; update push.default and create a new sequence preview." - .to_owned() - })?, - }; + let push_target = self.typed_push_target(git, &branch.name, &branch.upstream)?; + if push_target.sets_upstream { + return Err( + "Open pull request blocked: the deferred branch will not have an upstream; add `push` before `open pr` and create a new sequence preview." + .to_owned(), + ); + } + let TypedPushTarget { + remote, + branch: upstream_branch, + .. + } = push_target; let remote_urls = git .remote_push_urls(&remote) .map_err(|error| format!("Unable to prepare pull request remote: {error}"))?; @@ -2364,6 +2343,12 @@ impl OperationPlanner { .map(ToOwned::to_owned) }) .unwrap_or_else(|| repository.default_branch.clone()); + validate_pull_request_base_head( + &canonical_head_repository, + &canonical_github_repository, + &base, + &upstream_branch, + )?; if !github .branch_exists(&base) .map_err(open_pull_request_gh_error)? @@ -2459,62 +2444,61 @@ impl OperationPlanner { .map_err(|error| format!("Unable to snapshot push target: {error}"))?; let ahead = status.branch.ahead; - match status.branch.upstream { + let upstream = status.branch.upstream; + let sequence_upstream = match upstream.as_deref() { Some(upstream) => { - let (remote, upstream_branch) = git + let (remote, branch) = git .upstream_push_target(&branch) .map_err(|error| format!("Unable to prepare push target: {error}"))? .ok_or_else(|| { format!("Push blocked: unable to resolve upstream {upstream}.") })?; - let expected_upstream = format!("{remote}/{upstream_branch}"); + let expected_upstream = format!("{remote}/{branch}"); if expected_upstream != upstream { return Err(format!( "Push blocked: upstream config does not match {upstream}." )); } - let remote_urls = git - .remote_push_urls(&remote) - .map_err(|error| format!("Unable to snapshot push remote URLs: {error}"))?; - let push_url = single_push_url(&remote, &remote_urls)?; - let expected_remote_oid = git - .remote_url_head_oid(push_url, &upstream_branch) - .map_err(|error| format!("Unable to snapshot push remote: {error}"))?; - Ok(PreparedOperation::new( - push_plan(&branch, &upstream, ahead), - ExecutionContext::from_payload(PendingPayload::Push { - local_branch: branch, - target: head_target, - remote, - upstream_branch, - upstream, - expected_remote_oid, - remote_urls, - }), - )) - } - None => { - let Some(remote) = self.default_remote_name() else { - return Err("Push blocked: no remotes are configured.".to_owned()); - }; - let remote_urls = git - .remote_push_urls(&remote) - .map_err(|error| format!("Unable to snapshot push remote URLs: {error}"))?; - let push_url = single_push_url(&remote, &remote_urls)?; - let expected_remote_oid = git - .remote_url_head_oid(push_url, &branch) - .map_err(|error| format!("Unable to snapshot push remote: {error}"))?; - Ok(PreparedOperation::new( - push_set_upstream_plan(&branch, &remote), - ExecutionContext::from_payload(PendingPayload::PushSetUpstream { - remote, - branch, - target: head_target, - expected_remote_oid, - remote_urls, - }), - )) + SequenceUpstream::Bound { remote, branch } } + None => SequenceUpstream::Missing, + }; + let push_target = self.typed_push_target(&git, &branch, &sequence_upstream)?; + let remote_urls = git + .remote_push_urls(&push_target.remote) + .map_err(|error| format!("Unable to snapshot push remote URLs: {error}"))?; + let push_url = single_push_url(&push_target.remote, &remote_urls)?; + let expected_remote_oid = git + .remote_url_head_oid(push_url, &push_target.branch) + .map_err(|error| format!("Unable to snapshot push remote: {error}"))?; + if push_target.sets_upstream { + Ok(PreparedOperation::new( + push_set_upstream_plan(&branch, &push_target.remote), + ExecutionContext::from_payload(PendingPayload::PushSetUpstream { + remote: push_target.remote, + branch, + target: head_target, + expected_remote_oid, + remote_urls, + }), + )) + } else { + let upstream = upstream.ok_or_else(|| { + "Push blocked: tracked push destination has no upstream.".to_owned() + })?; + let destination = format!("{}/{}", push_target.remote, push_target.branch); + Ok(PreparedOperation::new( + push_plan(&branch, &destination, ahead), + ExecutionContext::from_payload(PendingPayload::Push { + local_branch: branch, + target: head_target, + remote: push_target.remote, + upstream_branch: push_target.branch, + upstream, + expected_remote_oid, + remote_urls, + }), + )) } } @@ -2723,13 +2707,28 @@ impl OperationPlanner { let Some(upstream) = status.branch.upstream.clone() else { return Err("Open pull request blocked: current branch has no upstream. Push it with `push` first.".to_owned()); }; - let (remote, upstream_branch) = git - .push_target(&branch) + let (tracking_remote, tracking_branch) = git + .upstream_push_target(&branch) .map_err(|error| format!("Unable to prepare pull request target: {error}"))? .ok_or_else(|| { - "Open pull request blocked: unable to resolve the current branch push target." + "Open pull request blocked: unable to resolve the current branch upstream." .to_owned() })?; + if format!("{tracking_remote}/{tracking_branch}") != upstream { + return Err(format!( + "Open pull request blocked: upstream config does not match {upstream}." + )); + } + let push_target = self.typed_push_target( + &git, + &branch, + &SequenceUpstream::Bound { + remote: tracking_remote, + branch: tracking_branch, + }, + )?; + let remote = push_target.remote; + let upstream_branch = push_target.branch; let remote_urls = git .remote_push_urls(&remote) .map_err(|error| format!("Unable to prepare pull request remote: {error}"))?; @@ -2789,14 +2788,12 @@ impl OperationPlanner { .map(ToOwned::to_owned) }) .unwrap_or(repository.default_branch); - if canonical_head_github_repository.eq_ignore_ascii_case(&canonical_github_repository) - && base == upstream_branch - { - return Err( - "Open pull request blocked: base branch must differ from the pull request head." - .to_owned(), - ); - } + validate_pull_request_base_head( + &canonical_head_github_repository, + &canonical_github_repository, + &base, + &upstream_branch, + )?; if !github .branch_exists(&base) .map_err(open_pull_request_gh_error)? @@ -2865,13 +2862,37 @@ impl OperationPlanner { branch_name(&status.branch).map_err(|error| error.replace("Operation", action)) } - fn default_remote_name(&self) -> Option { - let remotes = self.git().remotes().ok()?; - remotes - .iter() - .find(|remote| remote.name == "origin") - .or_else(|| remotes.first()) - .map(|remote| remote.name.clone()) + fn typed_push_target( + &self, + git: &Git, + branch: &str, + upstream: &SequenceUpstream, + ) -> Result { + let upstream = match upstream { + SequenceUpstream::Existing => git + .upstream_push_target(branch) + .map_err(|error| format!("Unable to prepare push target: {error}"))?, + SequenceUpstream::Missing => None, + SequenceUpstream::Bound { remote, branch } => Some((remote.clone(), branch.clone())), + }; + let default_remote = default_remote_name(git); + let (remote, target_branch) = git + .typed_push_target( + branch, + upstream + .as_ref() + .map(|(remote, branch)| (remote.as_str(), branch.as_str())), + default_remote.as_deref(), + ) + .map_err(|error| format!("Unable to prepare push target: {error}"))? + .ok_or_else(|| { + "Push blocked: no configured push destination could be resolved.".to_owned() + })?; + Ok(TypedPushTarget { + remote, + branch: target_branch, + sets_upstream: upstream.is_none(), + }) } fn git(&self) -> Git { @@ -2900,6 +2921,15 @@ impl OperationPlanner { } } +fn default_remote_name(git: &Git) -> Option { + let remotes = git.remotes().ok()?; + remotes + .iter() + .find(|remote| remote.name == "origin") + .or_else(|| remotes.first()) + .map(|remote| remote.name.clone()) +} + fn prompt_sequence_preflight_operation( request: OperationRequest, ) -> Result { @@ -3091,6 +3121,21 @@ fn pull_request_head( Ok(format!("{owner}:{branch}")) } +fn validate_pull_request_base_head( + head_repository: &GitHubRepository, + base_repository: &GitHubRepository, + base: &str, + head_branch: &str, +) -> Result<(), String> { + if head_repository.eq_ignore_ascii_case(base_repository) && base == head_branch { + return Err( + "Open pull request blocked: base branch must differ from the pull request head." + .to_owned(), + ); + } + Ok(()) +} + fn matching_pull_request<'a>( pull_requests: &'a [bitbygit_gh::PullRequest], base: &str, @@ -3117,6 +3162,7 @@ fn validate_push_plan( expected_upstream: Option<&str>, target: &HeadTarget, remote: &str, + remote_branch: &str, remote_urls: &[String], ) -> Result<(), String> { if git @@ -3136,6 +3182,40 @@ fn validate_push_plan( if status.branch.upstream.as_deref() != expected_upstream { return Err("Push blocked: upstream changed since the plan was shown.".to_owned()); } + let upstream = match expected_upstream { + Some(expected_upstream) => { + let upstream = git + .upstream_push_target(branch) + .map_err(|error| format!("Unable to revalidate push target: {error}"))? + .ok_or_else(|| { + "Push blocked: upstream config is no longer available.".to_owned() + })?; + if format!("{}/{}", upstream.0, upstream.1) != expected_upstream { + return Err( + "Push blocked: upstream config changed since the plan was shown.".to_owned(), + ); + } + Some(upstream) + } + None => None, + }; + let default_remote = default_remote_name(git); + let current_target = git + .typed_push_target( + branch, + upstream + .as_ref() + .map(|(remote, branch)| (remote.as_str(), branch.as_str())), + default_remote.as_deref(), + ) + .map_err(|error| format!("Unable to revalidate push target: {error}"))?; + if current_target + .as_ref() + .map(|(remote, branch)| (remote.as_str(), branch.as_str())) + != Some((remote, remote_branch)) + { + return Err("Push blocked: push destination changed since the plan was shown.".to_owned()); + } let current_remote_urls = git .remote_push_urls(remote) .map_err(|error| format!("Unable to revalidate push remote URLs: {error}"))?; @@ -3208,7 +3288,27 @@ fn validate_open_pull_request_plan( "Open pull request blocked: upstream changed since the plan was shown.".to_owned(), )); } - let current_target = git.push_target(branch).map_err(StepExecutionError::Git)?; + let tracking_target = git + .upstream_push_target(branch) + .map_err(StepExecutionError::Git)? + .ok_or_else(|| { + StepExecutionError::Blocked( + "Open pull request blocked: upstream config is no longer available.".to_owned(), + ) + })?; + if format!("{}/{}", tracking_target.0, tracking_target.1) != expected_upstream { + return Err(StepExecutionError::Blocked( + "Open pull request blocked: upstream config changed since the plan was shown." + .to_owned(), + )); + } + let current_target = git + .typed_push_target( + branch, + Some((tracking_target.0.as_str(), tracking_target.1.as_str())), + None, + ) + .map_err(StepExecutionError::Git)?; if current_target .as_ref() .map(|(name, branch)| (name.as_str(), branch.as_str())) @@ -3872,6 +3972,7 @@ impl PlanExecutor { Some(upstream), target, remote, + upstream_branch, remote_urls, ) .map_err(StepExecutionError::Blocked)?; @@ -3899,7 +4000,7 @@ impl PlanExecutor { else { return Err(mismatched_context(OperationKind::PushSetUpstream)); }; - validate_push_plan(git, branch, None, target, remote, remote_urls) + validate_push_plan(git, branch, None, target, remote, branch, remote_urls) .map_err(StepExecutionError::Blocked)?; let source_oid = target.oid.as_deref().ok_or_else(|| { StepExecutionError::Git(GitError::Blocked { @@ -5850,6 +5951,194 @@ mod tests { Ok(()) } + #[test] + fn remote_checkout_push_pr_uses_configured_push_destination() -> Result<(), Box> { + let repo = isolated_git_repo("prompt-sequence-remote-checkout-push-target")?; + let origin = isolated_bare_git_repo("prompt-sequence-remote-checkout-origin")?; + configure_git_identity(&repo)?; + std::fs::write(repo.join("file.txt"), "base\n")?; + git_stdout(&repo, &["add", "file.txt"])?; + git_stdout(&repo, &["commit", "-m", "initial"])?; + git_stdout( + &repo, + &["remote", "add", "fork", "ssh://git@github.com/bob/repo.git"], + )?; + git_stdout(&repo, &["update-ref", "refs/remotes/fork/topic", "HEAD"])?; + add_github_remote(&repo, "origin", &origin)?; + git_stdout(&repo, &["config", "remote.pushDefault", "origin"])?; + let fake_gh = fake_gh("prompt-sequence-remote-checkout-push-target", false)?; + let ssh = test_ssh_command()?; + let planner = OperationPlanner { + repo_root: repo.clone(), + github_executable: Some(fake_gh.clone()), + policy: EffectivePolicy::default(), + ssh_executable: Some(ssh.clone()), + }; + + let sequence = planner + .plan_prompt_sequence(vec![ + OperationRequest::Checkout { + branch: "fork/topic".to_owned(), + }, + OperationRequest::Push, + OperationRequest::OpenPullRequest { base: None }, + ]) + .map_err(std::io::Error::other)?; + + let preview = sequence.plan.preview_text(); + assert!( + preview.contains("repository: github.com/octo/repo"), + "{preview}" + ); + assert!(preview.contains("head: topic"), "{preview}"); + let paths = isolated_store_paths("prompt-sequence-remote-checkout-push-target-audit")?; + let result = + PromptSequenceExecutor::with_audit_paths_and_tools(&repo, paths, &fake_gh, ssh) + .execute(sequence.sequence); + + assert!( + result.message().contains("completed 3 step(s)"), + "{}", + result.message() + ); + let local_head = git_stdout(&repo, &["rev-parse", "HEAD"])?; + let pushed = git_stdout(&origin, &["rev-parse", "refs/heads/topic"])?; + assert_eq!(pushed.trim(), local_head.trim()); + let invocations = std::fs::read_to_string(fake_gh.with_file_name("invocations"))?; + assert!(invocations.contains("--head topic"), "{invocations}"); + Ok(()) + } + + #[test] + fn create_branch_push_pr_uses_branch_push_remote() -> Result<(), Box> { + let repo = isolated_git_repo("prompt-sequence-create-branch-push-target")?; + let fork = isolated_bare_git_repo("prompt-sequence-create-branch-fork")?; + configure_git_identity(&repo)?; + std::fs::write(repo.join("file.txt"), "base\n")?; + git_stdout(&repo, &["add", "file.txt"])?; + git_stdout(&repo, &["commit", "-m", "initial"])?; + git_stdout( + &repo, + &[ + "remote", + "add", + "origin", + "ssh://git@github.com/upstream/repo.git", + ], + )?; + git_stdout( + &repo, + &[ + "remote", + "add", + "upstream", + "ssh://git@github.com/upstream/repo.git", + ], + )?; + add_github_remote(&repo, "fork", &fork)?; + git_stdout(&repo, &["config", "branch.feature/new.pushRemote", "fork"])?; + let fake_gh = fake_gh("prompt-sequence-create-branch-push-target", false)?; + let ssh = test_ssh_command()?; + let planner = OperationPlanner { + repo_root: repo.clone(), + github_executable: Some(fake_gh.clone()), + policy: EffectivePolicy::default(), + ssh_executable: Some(ssh.clone()), + }; + + let sequence = planner + .plan_prompt_sequence(vec![ + OperationRequest::CreateBranch { + branch: "feature/new".to_owned(), + base: None, + }, + OperationRequest::Push, + OperationRequest::OpenPullRequest { base: None }, + ]) + .map_err(std::io::Error::other)?; + + let preview = sequence.plan.preview_text(); + assert!( + preview.contains("repository: github.com/upstream/repo"), + "{preview}" + ); + assert!(preview.contains("head: octo:feature/new"), "{preview}"); + let paths = isolated_store_paths("prompt-sequence-create-branch-push-target-audit")?; + let result = + PromptSequenceExecutor::with_audit_paths_and_tools(&repo, paths, &fake_gh, ssh) + .execute(sequence.sequence); + + assert!( + result.message().contains("completed 3 step(s)"), + "{}", + result.message() + ); + let local_head = git_stdout(&repo, &["rev-parse", "HEAD"])?; + let pushed = git_stdout(&fork, &["rev-parse", "refs/heads/feature/new"])?; + assert_eq!(pushed.trim(), local_head.trim()); + let invocations = std::fs::read_to_string(fake_gh.with_file_name("invocations"))?; + assert!( + invocations.contains("--head octo:feature/new"), + "{invocations}" + ); + Ok(()) + } + + #[test] + fn deferred_matching_base_and_head_fails_before_any_step() -> Result<(), Box> { + let repo = isolated_git_repo("prompt-sequence-matching-base-head")?; + let origin = isolated_bare_git_repo("prompt-sequence-matching-base-head-origin")?; + configure_git_identity(&repo)?; + std::fs::write(repo.join("file.txt"), "base\n")?; + git_stdout(&repo, &["add", "file.txt"])?; + git_stdout(&repo, &["commit", "-m", "initial"])?; + let original_branch = git_stdout(&repo, &["branch", "--show-current"])?; + git_stdout(&repo, &["update-ref", "refs/remotes/origin/topic", "HEAD"])?; + add_github_remote(&repo, "origin", &origin)?; + let fake_gh = fake_gh("prompt-sequence-matching-base-head", false)?; + let planner = OperationPlanner { + repo_root: repo.clone(), + github_executable: Some(fake_gh), + policy: EffectivePolicy::default(), + ssh_executable: Some(test_ssh_command()?), + }; + + let error = match planner.plan_prompt_sequence(vec![ + OperationRequest::Checkout { + branch: "origin/topic".to_owned(), + }, + OperationRequest::Push, + OperationRequest::OpenPullRequest { + base: Some("topic".to_owned()), + }, + ]) { + Ok(_) => { + return Err(std::io::Error::other( + "matching base and head must fail during sequence preflight", + ) + .into()); + } + Err(error) => error, + }; + + assert!(error.contains("base branch must differ"), "{error}"); + assert_eq!( + git_stdout(&repo, &["branch", "--show-current"])?, + original_branch + ); + assert!( + !std::process::Command::new("git") + .arg("--git-dir") + .arg(&origin) + .args(["show-ref", "--verify", "--quiet", "refs/heads/topic"]) + .status()? + .success() + ); + let paths = isolated_store_paths("prompt-sequence-matching-base-head-audit")?; + assert!(LocalStore::open(paths)?.list_audit_entries()?.is_empty()); + Ok(()) + } + #[test] fn deferred_configured_pull_request_base_is_previewed_and_executed() -> Result<(), Box> { From 2353645ada2f6c4222680d3c30c58619e88ea59b Mon Sep 17 00:00:00 2001 From: Adrian Cosentino Date: Fri, 17 Jul 2026 16:58:43 -0400 Subject: [PATCH 06/10] fix: revalidate downstream PR targets --- crates/bitbygit-tui/src/lib.rs | 151 ++++++++++++++++++++++++++++++++- 1 file changed, 149 insertions(+), 2 deletions(-) diff --git a/crates/bitbygit-tui/src/lib.rs b/crates/bitbygit-tui/src/lib.rs index ff25c69..96a1488 100644 --- a/crates/bitbygit-tui/src/lib.rs +++ b/crates/bitbygit-tui/src/lib.rs @@ -1900,6 +1900,21 @@ fn prompt_sequence_request_preview( } } +fn prompt_sequence_request_is_side_effecting(request: &OperationRequest) -> bool { + matches!( + request, + OperationRequest::Fetch + | OperationRequest::Commit { .. } + | OperationRequest::Push + | OperationRequest::Pull { .. } + | OperationRequest::Checkout { .. } + | OperationRequest::CreateBranch { .. } + | OperationRequest::Merge { .. } + | OperationRequest::Rebase { .. } + | OperationRequest::OpenPullRequest { .. } + ) +} + fn repo_list(app: &App) -> List<'_> { let items = app .repos @@ -4516,7 +4531,6 @@ impl PromptSequenceExecutor { let mut previewed_policies = policy_evaluations.into_iter(); let mut previewed_sequence_policies = sequence_policy_evaluations.into_iter(); - let mut previewed_pull_request_targets = deferred_pull_request_targets.into_iter(); let (Some(previewed_policy), Some(previewed_sequence_policy)) = ( previewed_policies.next(), @@ -4567,7 +4581,9 @@ impl PromptSequenceExecutor { for (index, request) in remaining_requests.into_iter().enumerate() { let step_number = index + 2; - let Some(previewed_pull_request_target) = previewed_pull_request_targets.next() else { + let Some(previewed_pull_request_target) = + deferred_pull_request_targets.get(index).cloned() + else { step_results.push(PromptSequenceStepResult::planning_failed( step_number, prompt_sequence_request_title(&request), @@ -4587,6 +4603,38 @@ impl PromptSequenceExecutor { #[cfg(test)] ssh_executable: self.ssh_executable.clone(), }; + let downstream_pull_request_targets = &deferred_pull_request_targets[index + 1..]; + if prompt_sequence_request_is_side_effecting(&request) + && downstream_pull_request_targets.iter().any(Option::is_some) + { + let branch = git + .status() + .ok() + .and_then(|status| branch_name(&status.branch).ok()) + .or_else(|| { + git.head_target() + .ok() + .and_then(|target| head_target_branch(&target).map(ToOwned::to_owned)) + }); + let current_targets = + planner.deferred_pull_request_targets(&requests[index + 1..], branch); + if !current_targets + .as_ref() + .is_ok_and(|targets| targets == downstream_pull_request_targets) + { + step_results.push(PromptSequenceStepResult::planning_failed( + step_number, + prompt_sequence_request_title(&request), + "pull request target changed since the sequence preview; create a new sequence preview and confirmation" + .to_owned(), + )); + return PromptSequenceExecutionResult::new( + total_steps, + step_results, + current_policy, + ); + } + } let operation = match planner.plan_request(request.clone()) { Ok(operation) => operation, Err(error) => { @@ -5951,6 +5999,105 @@ mod tests { Ok(()) } + #[test] + fn push_revalidates_downstream_pr_target_after_fetch() -> Result<(), Box> { + use std::os::unix::fs::PermissionsExt; + + let repo = isolated_git_repo("prompt-sequence-fetch-changes-push-target")?; + let origin = isolated_bare_git_repo("prompt-sequence-fetch-origin")?; + let fork = isolated_bare_git_repo("prompt-sequence-fetch-fork")?; + configure_git_identity(&repo)?; + std::fs::write(repo.join("file.txt"), "base\n")?; + git_stdout(&repo, &["add", "file.txt"])?; + git_stdout(&repo, &["commit", "-m", "initial"])?; + let branch = git_stdout(&repo, &["branch", "--show-current"])? + .trim() + .to_owned(); + add_github_remote(&repo, "origin", &origin)?; + git_stdout( + &repo, + &[ + "remote", + "add", + "fork", + "ssh://git@github.com/fork/repo.git", + ], + )?; + git_stdout( + &repo, + &[ + "config", + "remote.fork.testbare", + &fork.display().to_string(), + ], + )?; + let fake_gh = fake_gh("prompt-sequence-fetch-changes-push-target", false)?; + let ssh_root = isolated_temp_root("prompt-sequence-fetch-changes-push-target-ssh")?; + std::fs::create_dir_all(&ssh_root)?; + let ssh = ssh_root.join("ssh"); + std::fs::write( + &ssh, + format!( + "#!/bin/sh\ngit config remote.pushDefault fork\ncase \"$*\" in\n*fork/repo.git*) target='{}' ;;\n*) target='{}' ;;\nesac\ncase \"$*\" in\n*git-receive-pack*) exec git-receive-pack \"$target\" ;;\n*) exec git-upload-pack \"$target\" ;;\nesac\n", + fork.display(), + origin.display() + ), + )?; + let mut permissions = std::fs::metadata(&ssh)?.permissions(); + permissions.set_mode(0o755); + std::fs::set_permissions(&ssh, permissions)?; + let planner = OperationPlanner { + repo_root: repo.clone(), + github_executable: Some(fake_gh.clone()), + policy: EffectivePolicy::default(), + ssh_executable: Some(ssh.clone()), + }; + let sequence = planner + .plan_prompt_sequence(vec![ + OperationRequest::Fetch, + OperationRequest::Push, + OperationRequest::OpenPullRequest { base: None }, + ]) + .map_err(std::io::Error::other)?; + let preview = sequence.plan.preview_text(); + assert!( + preview.contains("repository: github.com/octo/repo"), + "{preview}" + ); + let paths = isolated_store_paths("prompt-sequence-fetch-changes-push-target-audit")?; + + let result = + PromptSequenceExecutor::with_audit_paths_and_tools(&repo, paths.clone(), &fake_gh, ssh) + .execute(sequence.sequence); + + let message = result.message(); + assert!( + message.contains("Prompt sequence stopped before step 2 of 3."), + "{message}" + ); + assert!( + message.contains("pull request target changed since the sequence preview"), + "{message}" + ); + let entries = LocalStore::open(paths)?.list_audit_entries()?; + assert!(entries.iter().all(|entry| entry.operation != "push")); + assert!( + !std::process::Command::new("git") + .arg("--git-dir") + .arg(&fork) + .args([ + "show-ref", + "--verify", + "--quiet", + &format!("refs/heads/{branch}"), + ]) + .status()? + .success(), + "push must not create a ref at the changed destination" + ); + Ok(()) + } + #[test] fn remote_checkout_push_pr_uses_configured_push_destination() -> Result<(), Box> { let repo = isolated_git_repo("prompt-sequence-remote-checkout-push-target")?; From c92e89d3d00b1bd50750fbda233d4a9b55641e16 Mon Sep 17 00:00:00 2001 From: Adrian Cosentino Date: Fri, 17 Jul 2026 17:26:11 -0400 Subject: [PATCH 07/10] fix: preserve blocked sequence preflight --- crates/bitbygit-git/src/lib.rs | 8 ++++-- crates/bitbygit-tui/src/lib.rs | 46 ++++++++++++++++++++++++++++------ 2 files changed, 45 insertions(+), 9 deletions(-) diff --git a/crates/bitbygit-git/src/lib.rs b/crates/bitbygit-git/src/lib.rs index 81de4de..6065013 100644 --- a/crates/bitbygit-git/src/lib.rs +++ b/crates/bitbygit-git/src/lib.rs @@ -483,7 +483,9 @@ impl Git { .unwrap_or_else(|| "simple".to_owned()); let target = match push_default.as_str() { "simple" if remote != upstream_remote => (remote, branch.to_owned()), - "simple" | "upstream" => (upstream_remote.to_owned(), upstream_branch.to_owned()), + "simple" | "upstream" | "tracking" => { + (upstream_remote.to_owned(), upstream_branch.to_owned()) + } "current" | "matching" => (remote, branch.to_owned()), "nothing" => return Ok(None), _ => { @@ -3280,7 +3282,9 @@ mod tests { ); repo.run(["config", "remote.pushDefault", "origin"])?; - for push_default in ["simple", "current", "upstream", "matching", "nothing"] { + for push_default in [ + "simple", "current", "upstream", "tracking", "matching", "nothing", + ] { repo.run(["config", "push.default", push_default])?; assert_eq!( git.typed_push_target("topic", Some(("fork", "topic")), Some("fork"))?, diff --git a/crates/bitbygit-tui/src/lib.rs b/crates/bitbygit-tui/src/lib.rs index 96a1488..c07327d 100644 --- a/crates/bitbygit-tui/src/lib.rs +++ b/crates/bitbygit-tui/src/lib.rs @@ -2161,18 +2161,21 @@ impl OperationPlanner { .ok() .and_then(|target| head_target_branch(&target).map(ToOwned::to_owned)) }); - let deferred_pull_request_targets = - self.deferred_pull_request_targets(&requests, branch.clone())?; let (policy_evaluations, sequence_policy_evaluations) = - prompt_sequence_policy_evaluations(&self.policy, &git, &requests, branch)?; - let first_request = requests - .first() - .cloned() - .ok_or_else(|| "Prompt sequence requires at least two steps.".to_owned())?; + prompt_sequence_policy_evaluations(&self.policy, &git, &requests, branch.clone())?; let blocked = policy_evaluations .iter() .chain(&sequence_policy_evaluations) .any(|evaluation| evaluation.requirement == ConfirmationRequirement::Blocked); + let deferred_pull_request_targets = if blocked { + vec![None; requests.len().saturating_sub(1)] + } else { + self.deferred_pull_request_targets(&requests, branch.clone())? + }; + let first_request = requests + .first() + .cloned() + .ok_or_else(|| "Prompt sequence requires at least two steps.".to_owned())?; let first = if blocked { prompt_sequence_preflight_operation(first_request)? } else { @@ -8420,6 +8423,35 @@ mod tests { Ok(()) } + #[test] + fn blocked_prompt_sequence_does_not_bind_pull_request_target() -> Result<(), Box> { + let repo = pushed_branch_repo("blocked-sequence-pull-request-target")?; + let fake_gh = fake_gh("blocked-sequence-pull-request-target", false)?; + let mut config = AppConfig::default(); + config.policy.confirmation.medium = ConfirmationSetting::Blocked; + let planner = OperationPlanner { + repo_root: repo, + github_executable: Some(fake_gh.clone()), + policy: EffectivePolicy::new(&config), + ssh_executable: Some(test_ssh_command()?), + }; + + let sequence = planner + .plan_prompt_sequence(vec![ + OperationRequest::Branches, + OperationRequest::OpenPullRequest { base: None }, + ]) + .map_err(std::io::Error::other)?; + + assert_eq!( + sequence.plan.confirmation.requirement, + ConfirmationRequirement::Blocked + ); + assert_eq!(sequence.sequence.deferred_pull_request_targets, vec![None]); + assert!(!fake_gh.with_file_name("invocations").exists()); + Ok(()) + } + #[test] fn low_explicit_policy_applies_to_low_risk_prompt_sequences() -> Result<(), Box> { let repo = isolated_git_repo("sequence-low-explicit-policy")?; From 918fe010fbe9c2a79c6fe7bf8ea4c2eae892ecdf Mon Sep 17 00:00:00 2001 From: Adrian Cosentino Date: Fri, 17 Jul 2026 17:51:57 -0400 Subject: [PATCH 08/10] fix: revalidate deferred targets after planning --- crates/bitbygit-tui/src/lib.rs | 145 +++++++++++++++++++++++++++++++++ 1 file changed, 145 insertions(+) diff --git a/crates/bitbygit-tui/src/lib.rs b/crates/bitbygit-tui/src/lib.rs index c07327d..ebc7044 100644 --- a/crates/bitbygit-tui/src/lib.rs +++ b/crates/bitbygit-tui/src/lib.rs @@ -4660,6 +4660,37 @@ impl PromptSequenceExecutor { ); } }; + if prompt_sequence_request_is_side_effecting(&request) + && downstream_pull_request_targets.iter().any(Option::is_some) + { + let branch = git + .status() + .ok() + .and_then(|status| branch_name(&status.branch).ok()) + .or_else(|| { + git.head_target() + .ok() + .and_then(|target| head_target_branch(&target).map(ToOwned::to_owned)) + }); + let current_targets = + planner.deferred_pull_request_targets(&requests[index + 1..], branch); + if !current_targets + .as_ref() + .is_ok_and(|targets| targets == downstream_pull_request_targets) + { + step_results.push(PromptSequenceStepResult::planning_failed( + step_number, + operation.plan.title.clone(), + "pull request target changed since the sequence preview; create a new sequence preview and confirmation" + .to_owned(), + )); + return PromptSequenceExecutionResult::new( + total_steps, + step_results, + current_policy, + ); + } + } if let Some(previewed_target) = previewed_pull_request_target { if prepared_pull_request_target(&operation).as_ref() != Some(&previewed_target) { step_results.push(PromptSequenceStepResult::planning_failed( @@ -6101,6 +6132,120 @@ mod tests { Ok(()) } + #[test] + fn push_revalidates_downstream_pr_target_after_replanning() -> Result<(), Box> { + use std::os::unix::fs::PermissionsExt; + + let repo = isolated_git_repo("prompt-sequence-replan-changes-push-target")?; + let origin = isolated_bare_git_repo("prompt-sequence-replan-origin")?; + let fork = isolated_bare_git_repo("prompt-sequence-replan-fork")?; + configure_git_identity(&repo)?; + std::fs::write(repo.join("file.txt"), "base\n")?; + git_stdout(&repo, &["add", "file.txt"])?; + git_stdout(&repo, &["commit", "-m", "initial"])?; + let branch = git_stdout(&repo, &["branch", "--show-current"])? + .trim() + .to_owned(); + add_github_remote(&repo, "origin", &origin)?; + git_stdout( + &repo, + &[ + "remote", + "add", + "fork", + "ssh://git@github.com/fork/repo.git", + ], + )?; + git_stdout( + &repo, + &[ + "config", + "remote.fork.testbare", + &fork.display().to_string(), + ], + )?; + + let delegated_gh = fake_gh("prompt-sequence-replan-changes-push-target", false)?; + let gh_root = isolated_temp_root("prompt-sequence-replan-changes-push-target-gh")?; + std::fs::create_dir_all(&gh_root)?; + let fake_gh = gh_root.join("gh"); + let repo_view_count = gh_root.join("repo-view-count"); + std::fs::write( + &fake_gh, + format!( + "#!/bin/sh\nif [ \"$1:$2\" = repo:view ]; then\n count=0\n [ ! -f '{}' ] || count=$(cat '{}')\n count=$((count + 1))\n printf '%s\\n' \"$count\" > '{}'\n if [ \"$count\" -eq 5 ]; then git config remote.pushDefault fork; fi\nfi\nexec '{}' \"$@\"\n", + repo_view_count.display(), + repo_view_count.display(), + repo_view_count.display(), + delegated_gh.display(), + ), + )?; + let mut permissions = std::fs::metadata(&fake_gh)?.permissions(); + permissions.set_mode(0o755); + std::fs::set_permissions(&fake_gh, permissions)?; + + let ssh_root = isolated_temp_root("prompt-sequence-replan-changes-push-target-ssh")?; + std::fs::create_dir_all(&ssh_root)?; + let ssh = ssh_root.join("ssh"); + std::fs::write( + &ssh, + format!( + "#!/bin/sh\ncase \"$*\" in\n*fork/repo.git*) target='{}' ;;\n*) target='{}' ;;\nesac\ncase \"$*\" in\n*git-receive-pack*) exec git-receive-pack \"$target\" ;;\n*) exec git-upload-pack \"$target\" ;;\nesac\n", + fork.display(), + origin.display() + ), + )?; + let mut permissions = std::fs::metadata(&ssh)?.permissions(); + permissions.set_mode(0o755); + std::fs::set_permissions(&ssh, permissions)?; + + let planner = OperationPlanner { + repo_root: repo.clone(), + github_executable: Some(fake_gh.clone()), + policy: EffectivePolicy::default(), + ssh_executable: Some(ssh.clone()), + }; + let sequence = planner + .plan_prompt_sequence(vec![ + OperationRequest::Fetch, + OperationRequest::Push, + OperationRequest::OpenPullRequest { base: None }, + ]) + .map_err(std::io::Error::other)?; + let paths = isolated_store_paths("prompt-sequence-replan-changes-push-target-audit")?; + + let result = + PromptSequenceExecutor::with_audit_paths_and_tools(&repo, paths.clone(), &fake_gh, ssh) + .execute(sequence.sequence); + + let message = result.message(); + assert!( + message.contains("Prompt sequence stopped before step 2 of 3."), + "{message}" + ); + assert!( + message.contains("pull request target changed since the sequence preview"), + "{message}" + ); + let entries = LocalStore::open(paths)?.list_audit_entries()?; + assert!(entries.iter().all(|entry| entry.operation != "push")); + assert!( + !std::process::Command::new("git") + .arg("--git-dir") + .arg(&fork) + .args([ + "show-ref", + "--verify", + "--quiet", + &format!("refs/heads/{branch}"), + ]) + .status()? + .success(), + "push must not create a ref at the replanned destination" + ); + Ok(()) + } + #[test] fn remote_checkout_push_pr_uses_configured_push_destination() -> Result<(), Box> { let repo = isolated_git_repo("prompt-sequence-remote-checkout-push-target")?; From 617574fb54901df4686058e304ba46a3503ae951 Mon Sep 17 00:00:00 2001 From: Adrian Cosentino Date: Fri, 17 Jul 2026 18:04:26 -0400 Subject: [PATCH 09/10] fix: rebind deferred targets after config reload --- crates/bitbygit-tui/src/lib.rs | 152 +++++++++++++++++++++++++++++---- 1 file changed, 134 insertions(+), 18 deletions(-) diff --git a/crates/bitbygit-tui/src/lib.rs b/crates/bitbygit-tui/src/lib.rs index ebc7044..1f8f048 100644 --- a/crates/bitbygit-tui/src/lib.rs +++ b/crates/bitbygit-tui/src/lib.rs @@ -4482,14 +4482,16 @@ impl PromptSequenceExecutor { .ok() .and_then(|status| branch_name(&status.branch).ok()) }); - let current_pull_request_targets = OperationPlanner { - repo_root: self.repo_root.clone(), - github_executable: self.github_executable.clone(), - policy: current_policy.clone(), - #[cfg(test)] - ssh_executable: self.ssh_executable.clone(), - } - .deferred_pull_request_targets(&requests, branch.clone()); + let planner = self.planner(current_policy.clone()); + let current_pull_request_targets = + planner.deferred_pull_request_targets(&requests, branch.clone()); + current_policy = self.load_policy(¤t_policy); + let current_pull_request_targets = if planner.policy == current_policy { + current_pull_request_targets + } else { + self.planner(current_policy.clone()) + .deferred_pull_request_targets(&requests, branch.clone()) + }; if current_pull_request_targets.as_ref() != Ok(&deferred_pull_request_targets) { step_results.push(PromptSequenceStepResult::planning_failed( 1, @@ -4599,13 +4601,7 @@ impl PromptSequenceExecutor { ); }; current_policy = self.load_policy(¤t_policy); - let planner = OperationPlanner { - repo_root: self.repo_root.clone(), - github_executable: self.github_executable.clone(), - policy: current_policy.clone(), - #[cfg(test)] - ssh_executable: self.ssh_executable.clone(), - }; + let planner = self.planner(current_policy.clone()); let downstream_pull_request_targets = &deferred_pull_request_targets[index + 1..]; if prompt_sequence_request_is_side_effecting(&request) && downstream_pull_request_targets.iter().any(Option::is_some) @@ -4620,7 +4616,14 @@ impl PromptSequenceExecutor { .and_then(|target| head_target_branch(&target).map(ToOwned::to_owned)) }); let current_targets = - planner.deferred_pull_request_targets(&requests[index + 1..], branch); + planner.deferred_pull_request_targets(&requests[index + 1..], branch.clone()); + current_policy = self.load_policy(¤t_policy); + let current_targets = if planner.policy == current_policy { + current_targets + } else { + self.planner(current_policy.clone()) + .deferred_pull_request_targets(&requests[index + 1..], branch) + }; if !current_targets .as_ref() .is_ok_and(|targets| targets == downstream_pull_request_targets) @@ -4638,6 +4641,7 @@ impl PromptSequenceExecutor { ); } } + let planner = self.planner(current_policy.clone()); let operation = match planner.plan_request(request.clone()) { Ok(operation) => operation, Err(error) => { @@ -4673,7 +4677,14 @@ impl PromptSequenceExecutor { .and_then(|target| head_target_branch(&target).map(ToOwned::to_owned)) }); let current_targets = - planner.deferred_pull_request_targets(&requests[index + 1..], branch); + planner.deferred_pull_request_targets(&requests[index + 1..], branch.clone()); + current_policy = self.load_policy(¤t_policy); + let current_targets = if planner.policy == current_policy { + current_targets + } else { + self.planner(current_policy.clone()) + .deferred_pull_request_targets(&requests[index + 1..], branch) + }; if !current_targets .as_ref() .is_ok_and(|targets| targets == downstream_pull_request_targets) @@ -4690,6 +4701,8 @@ impl PromptSequenceExecutor { current_policy, ); } + } else { + current_policy = self.load_policy(¤t_policy); } if let Some(previewed_target) = previewed_pull_request_target { if prepared_pull_request_target(&operation).as_ref() != Some(&previewed_target) { @@ -4721,7 +4734,6 @@ impl PromptSequenceExecutor { current_policy, ); }; - current_policy = self.load_policy(¤t_policy); let branch = policy_branch(&operation.context); let current_evaluation = evaluate_plan_policy(¤t_policy, &operation.plan, branch); let current_sequence_evaluation = @@ -4774,6 +4786,16 @@ impl PromptSequenceExecutor { self.audit.load_policy(fallback) } + fn planner(&self, policy: EffectivePolicy) -> OperationPlanner { + OperationPlanner { + repo_root: self.repo_root.clone(), + github_executable: self.github_executable.clone(), + policy, + #[cfg(test)] + ssh_executable: self.ssh_executable.clone(), + } + } + fn execute_prepared_step( executor: &PlanExecutor, step_number: usize, @@ -6246,6 +6268,100 @@ mod tests { Ok(()) } + #[test] + fn push_revalidates_downstream_pr_base_after_config_reload() -> Result<(), Box> { + use std::os::unix::fs::PermissionsExt; + + let repo = isolated_git_repo("prompt-sequence-reload-changes-pr-base")?; + let origin = isolated_bare_git_repo("prompt-sequence-reload-pr-base-origin")?; + configure_git_identity(&repo)?; + std::fs::write(repo.join("file.txt"), "base\n")?; + git_stdout(&repo, &["add", "file.txt"])?; + git_stdout(&repo, &["commit", "-m", "initial"])?; + let branch = git_stdout(&repo, &["branch", "--show-current"])? + .trim() + .to_owned(); + add_github_remote(&repo, "origin", &origin)?; + + let paths = isolated_store_paths("prompt-sequence-reload-changes-pr-base-audit")?; + let store = LocalStore::open(paths.clone())?; + std::fs::write( + &store.paths().config_file, + "schema-version = 1\n[pull-requests]\ndefault-base-branch = \"release\"\n", + )?; + let policy = EffectivePolicy::new(&store.load_config().settings); + let delegated_gh = fake_gh("prompt-sequence-reload-changes-pr-base", false)?; + let gh_root = isolated_temp_root("prompt-sequence-reload-changes-pr-base-gh")?; + std::fs::create_dir_all(&gh_root)?; + let fake_gh = gh_root.join("gh"); + let repo_view_count = gh_root.join("repo-view-count"); + std::fs::write( + &fake_gh, + format!( + "#!/bin/sh\nif [ \"$1:$2\" = repo:view ]; then\n count=0\n [ ! -f '{}' ] || count=$(cat '{}')\n count=$((count + 1))\n printf '%s\\n' \"$count\" > '{}'\n if [ \"$count\" -eq 5 ]; then printf '%s\\n' 'schema-version = 1' '[pull-requests]' 'default-base-branch = \"main\"' > '{}'; fi\nfi\nexec '{}' \"$@\"\n", + repo_view_count.display(), + repo_view_count.display(), + repo_view_count.display(), + store.paths().config_file.display(), + delegated_gh.display(), + ), + )?; + let mut permissions = std::fs::metadata(&fake_gh)?.permissions(); + permissions.set_mode(0o755); + std::fs::set_permissions(&fake_gh, permissions)?; + let ssh = test_ssh_command()?; + let planner = OperationPlanner { + repo_root: repo.clone(), + github_executable: Some(fake_gh.clone()), + policy: policy.clone(), + ssh_executable: Some(ssh.clone()), + }; + let sequence = planner + .plan_prompt_sequence(vec![ + OperationRequest::Fetch, + OperationRequest::Push, + OperationRequest::OpenPullRequest { base: None }, + ]) + .map_err(std::io::Error::other)?; + assert!(sequence.plan.preview_text().contains("base: release")); + + let result = PromptSequenceExecutor { + repo_root: repo, + audit: AuditDestination::Paths(paths.clone()), + github_executable: Some(fake_gh), + policy, + ssh_executable: Some(ssh), + } + .execute(sequence.sequence); + + let message = result.message(); + assert!( + message.contains("Prompt sequence stopped before step 2 of 3."), + "{message}" + ); + assert!( + message.contains("pull request target changed since the sequence preview"), + "{message}" + ); + let entries = LocalStore::open(paths)?.list_audit_entries()?; + assert!(entries.iter().all(|entry| entry.operation != "push")); + assert!( + !std::process::Command::new("git") + .arg("--git-dir") + .arg(&origin) + .args([ + "show-ref", + "--verify", + "--quiet", + &format!("refs/heads/{branch}"), + ]) + .status()? + .success(), + "push must not create a ref after the configured base changes" + ); + Ok(()) + } + #[test] fn remote_checkout_push_pr_uses_configured_push_destination() -> Result<(), Box> { let repo = isolated_git_repo("prompt-sequence-remote-checkout-push-target")?; From 3a7c4a28a7aae1a325bb997db14f9267df34621c Mon Sep 17 00:00:00 2001 From: Adrian Cosentino Date: Fri, 17 Jul 2026 18:08:11 -0400 Subject: [PATCH 10/10] fix: fail closed on deferred base reload --- crates/bitbygit-tui/src/lib.rs | 51 +++++++++++++++++++--------------- 1 file changed, 28 insertions(+), 23 deletions(-) diff --git a/crates/bitbygit-tui/src/lib.rs b/crates/bitbygit-tui/src/lib.rs index 1f8f048..f684a60 100644 --- a/crates/bitbygit-tui/src/lib.rs +++ b/crates/bitbygit-tui/src/lib.rs @@ -1915,6 +1915,17 @@ fn prompt_sequence_request_is_side_effecting(request: &OperationRequest) -> bool ) } +fn deferred_pull_request_base_changed( + previous: &EffectivePolicy, + current: &EffectivePolicy, + requests: &[OperationRequest], +) -> bool { + previous.default_pull_request_base() != current.default_pull_request_base() + && requests + .iter() + .any(|request| matches!(request, OperationRequest::OpenPullRequest { base: None })) +} + fn repo_list(app: &App) -> List<'_> { let items = app .repos @@ -4486,13 +4497,9 @@ impl PromptSequenceExecutor { let current_pull_request_targets = planner.deferred_pull_request_targets(&requests, branch.clone()); current_policy = self.load_policy(¤t_policy); - let current_pull_request_targets = if planner.policy == current_policy { - current_pull_request_targets - } else { - self.planner(current_policy.clone()) - .deferred_pull_request_targets(&requests, branch.clone()) - }; - if current_pull_request_targets.as_ref() != Ok(&deferred_pull_request_targets) { + if deferred_pull_request_base_changed(&planner.policy, ¤t_policy, &requests) + || current_pull_request_targets.as_ref() != Ok(&deferred_pull_request_targets) + { step_results.push(PromptSequenceStepResult::planning_failed( 1, first.plan.title.clone(), @@ -4615,16 +4622,15 @@ impl PromptSequenceExecutor { .ok() .and_then(|target| head_target_branch(&target).map(ToOwned::to_owned)) }); + let remaining_requests = &requests[index + 1..]; let current_targets = - planner.deferred_pull_request_targets(&requests[index + 1..], branch.clone()); + planner.deferred_pull_request_targets(remaining_requests, branch); current_policy = self.load_policy(¤t_policy); - let current_targets = if planner.policy == current_policy { - current_targets - } else { - self.planner(current_policy.clone()) - .deferred_pull_request_targets(&requests[index + 1..], branch) - }; - if !current_targets + if deferred_pull_request_base_changed( + &planner.policy, + ¤t_policy, + remaining_requests, + ) || !current_targets .as_ref() .is_ok_and(|targets| targets == downstream_pull_request_targets) { @@ -4676,16 +4682,15 @@ impl PromptSequenceExecutor { .ok() .and_then(|target| head_target_branch(&target).map(ToOwned::to_owned)) }); + let remaining_requests = &requests[index + 1..]; let current_targets = - planner.deferred_pull_request_targets(&requests[index + 1..], branch.clone()); + planner.deferred_pull_request_targets(remaining_requests, branch); current_policy = self.load_policy(¤t_policy); - let current_targets = if planner.policy == current_policy { - current_targets - } else { - self.planner(current_policy.clone()) - .deferred_pull_request_targets(&requests[index + 1..], branch) - }; - if !current_targets + if deferred_pull_request_base_changed( + &planner.policy, + ¤t_policy, + remaining_requests, + ) || !current_targets .as_ref() .is_ok_and(|targets| targets == downstream_pull_request_targets) {