8.0 KiB
APP-4218: Git operations dialogs compare against the branch's actual parent — Tech Spec
Product spec: specs/APP-4218/PRODUCT.md
Context
APP-4218 is about avoiding misleading Git Operations previews when a user creates a branch from another feature branch. The original implementation fell back to the detected default branch when a branch had no upstream, so the Push / Publish dialog could show inherited parent-branch commits as if they belonged to the current branch.
This PR implements the first, low-risk part of that behavior: the no-upstream Push / Publish commit list now falls back to a fork-point SHA instead of the default branch. The PR dialog, PR AI inputs, and gh pr create --base path still use the detected default branch in this checkout; they are listed as follow-ups below so the checked-in spec does not overstate what shipped.
Relevant code:
app/src/util/git.rs (199-249)—detect_fork_point, which computes a SHA for the point whereHEADforked from other refs.app/src/util/git.rs (391-428)—get_unpushed_commits, which uses<upstream>..HEADwhen an upstream exists and<fork>..HEADonly in the no-upstream fallback.app/src/code_review/diff_state.rs (1360-1399)—load_metadata_for_repo, which detects the current branch and upstream, then storesunpushed_commitsinDiffMetadata.app/src/code_review/code_review_view.rs (6770-6796)—primary_git_action_mode, which turnsunpushed_commitsinto Publish / Push button state.app/src/util/git.rs (677-904)— PR diff / AI / create helpers, which still compare against and target the detected default branch.
Proposed changes
1. Add detect_fork_point
detect_fork_point(repo_path, current_branch_name) returns the SHA where HEAD forked from other local or remote refs. It accepts the current branch name so the current branch and origin/<current> can be excluded from the comparison set; otherwise the branch would subtract itself and report no unique commits.
The current implementation uses one reachability query plus a rev-parse:
pub async fn detect_fork_point(
repo_path: &Path,
current_branch_name: Option<&str>,
) -> Result<Option<String>>;
Algorithm:
- Normalize
current_branch_name; ignore empty names and detachedHEAD. - Build
git rev-list HEAD --not --exclude=<current> --branches --exclude=origin/<current> --remotes. - Treat the last non-empty line as the oldest commit unique to
HEAD. - Return that commit's parent via
git rev-parse <oldest-unique>^. - If there are no unique commits, resolve
HEADitself; if the git commands fail, returnOk(None). This differs from the earlier plan that introduced a separatefor-each-refstep and a branch-name detector. The branch-name detector is not present in this implementation.
2. Use the fork point only for no-upstream unpushed commits
get_unpushed_commits(repo_path, current_branch_name, upstream_ref) keeps the upstream path unchanged:
- If
upstream_refexists, rungit log <upstream>..HEAD --format=COMMIT:%H\t%s --numstat. - If
upstream_refis missing, calldetect_fork_point(repo_path, current_branch_name)and rungit log <fork>..HEAD .... - If no fork point can be resolved, fall back to
git log HEAD .... This is the behavior that fixes the Publish dialog for stacked no-upstream branches while preserving existing behavior for branches that already have an upstream.
3. Keep dialog and metadata wiring unchanged
DiffStateModel::load_metadata_for_repo already computes the current branch, upstream ref, and unpushed_commits during metadata refresh. The Git Operations button already derives its mode from unpushed_commits, upstream_ref, uncommitted stats, and PR info.
No new UI state is required. The Push / Publish dialog still receives a Vec<Commit> from DiffStateModel when opened, so switching the no-upstream fallback is enough to change the included commit list.
4. Leave PR helpers on the default branch for this PR
The current code still uses detect_main_branch for:
get_branch_diff_entries— Create PR dialog file stats.get_diff_for_pr— AI PR title / body diff input.get_branch_commit_messages— AI PR title / body commit-message input.create_pr—gh pr create --base <default-branch>. This meansPRODUCT.mdbehavior around Create PR targeting the detected parent is not fully implemented by this PR. The tech spec should not claimdetect_pr_base_branchexists or that these helpers use the fork-point SHA.
End-to-end flow
- Code review metadata refresh runs in
DiffStateModel::load_metadata_for_repo. - The model resolves
current_branch_nameand optionalupstream_ref. get_unpushed_commitscomputes either<upstream>..HEADor<fork>..HEAD.CodeReviewView::primary_git_action_modeuses non-emptyunpushed_commitsplus upstream state to choose Publish or Push.- Opening the Push / Publish dialog passes those commits into
GitDialog::new_for_push.
Risks and mitigations
Fork-point SHA is not a PR base branch name
The fork point is enough for a commit range, but gh pr create --base needs a branch name. This PR intentionally does not infer a parent branch name from the fork point. Create PR behavior remains default-branch based until that follow-up lands.
Stale refs can affect fork-point detection
Because rev-list subtracts all branches and remotes except the current branch and origin/<current>, stale local or remote-tracking refs can make the fork point earlier than a user expects. This is conservative for Publish commit lists but can still be surprising. Pruning stale refs remains the mitigation.
Remote name assumption
The current self-exclusion only excludes origin/<current>. Repos whose current branch is pushed to a differently named remote could still include that remote-tracking ref in the subtraction set. That can hide unique commits after a manual push to a non-origin remote. If that matters, resolve the actual remote-tracking branch before building excludes.
Root commit / no other refs
If the oldest unique commit has no parent, rev-parse <sha>^ fails and detect_fork_point returns None; get_unpushed_commits then falls back to logging HEAD. This is acceptable for orphan or brand-new repositories.
Testing and validation
Manual validation for the current implementation:
- Create
feature-afrom main, add commits, then createfeature-bfromfeature-awithout setting an upstream. The Publish dialog onfeature-bshould show only commits unique tofeature-b, not all offeature-a. - Create a fresh no-upstream branch from main and add commits. The Publish dialog should still show commits since the branch forked from main.
- On a branch with an upstream, the Push dialog should still use
<upstream>..HEAD. - Reopen the dialog after rebasing the current branch; metadata refresh should recompute the fallback range from the new graph.
- Create PR dialog validation should expect current behavior for this PR: file stats, AI inputs, and
gh pr create --basestill use the detected default branch. Recommended unit coverage inapp/src/util/git_tests.rs: detect_fork_pointreturns the original fork commit after main advances beyond the branch point.- No-upstream
get_unpushed_commitsexcludes commits inherited from the parent feature branch. - Upstream-backed
get_unpushed_commitsremains unchanged and uses<upstream>..HEAD. - Detached
HEADdoes not try to exclude a branch namedHEAD.
Follow-ups
- Add parent branch-name detection for PR creation. A likely implementation is to find refs that contain or are closest to the fork point, then choose the best branch name with deterministic tiebreakers.
- Switch
get_branch_diff_entries,get_diff_for_pr, andget_branch_commit_messagesfrom default-branch ranges to the detected parent range once branch-name detection exists. - Switch
create_prto pass--base <detected-parent>and striporigin/when the selected parent is a remote-tracking ref. - Consider resolving the actual remote-tracking branch for current-branch self-exclusion instead of assuming
origin/<current>.