feat(git,branches,ui): push or publish any branch to origin without switching
Brought to you by:
thebguy
Menu
▾
▴
#76 feat(git,branches,ui): push or publish any branch to origin without switching
Originally created by: theBGuy
Originally owned by: theBGuy
This change lets you push or publish a local branch to origin directly from the branch switcher's right-click menu, without checking out the branch. It streamlines managing multiple branches — including ones checked out in other worktrees — by decoupling push/publish from the working tree (a push touches refs, never a working tree).
Scope (origin-centric v1): the menu surfaces Push to origin/… for a branch that's ahead of its origin remote, and Publish branch for an unpushed (or upstream-deleted) branch. A branch tracked on a non-origin remote is intentionally left out of v1 (multi-remote push is a follow-up).
Branch switcher UI
- Adds Push to origin/... and Publish branch actions to the right-click menu for local branches in
src/features/repository/BranchSwitcher.tsx - Sync-out options only appear if
originexists (useRemotes) - Push is gated/enabled based on branch status (ahead / diverged → disabled with reason, untracked → Publish, in-sync / non-origin → hidden)
- Handles push logic and toast notifications for "Push" vs. "Publish"
Git backend and API changes
- Updates
src-tauri/src/git/remote.rsto accept an explicit branch name for push operations (optionally, instead of always pushing the current branch);branch: Nonereproduces the pre-change HEAD push byte-for-byte - Implements push argument logic in the pure, unit-tested
build_push_args, supporting tracked/untracked/gone upstreams. Refspecs are fully qualified (refs/heads/<src>:refs/heads/<dst>) so a branch named+x/-xcan never be read as a force/delete indicator (round-1 security hardening) - Changes
src-tauri/src/mcp_server/write_git.rsto accept a target branch for push, withensure_not_flag+ensure_not_session_branchsafeguards;force_pushstays HEAD-only - Modifies
src/lib/git/api.tsandsrc/lib/git/queries.tsto pass an optional branch name to the push call
Documentation and help content
- Adds
changelog.d/added-push-branch-without-switching.md - Updates
src/features/help/content.ts,README.md, and the marketing-sitesite/src/data/capabilities.ts(non-AI capability, shown in both views)
Related
Originally posted by: cloudflare-workers-and-pages[bot]
Deploying gitdesktop with
Cloudflare Pages
cd1d037View logs
Originally posted by: theBGuy
Context for reviewers — deliberate decisions on the record (posted before the first review round so dispositions ground against it):
Contract & backward-compat
git_push(branch: None)produces byte-identical git args to the pre-change code for all four(set_upstream, force)combos — the existing SyncControls "Push" path is behaviorally unchanged. Traced againstHEAD:remote.rsduring internal review; theNonearm is a verbatim transplant.build_push_argswith 8 unit tests (untracked publish, gone publish, tracked same-name, different-name → explicit<b>:<up>refspec, non-origin plain push,set_upstreamretrack,--force-with-leaseordering ×2).Deliberate semantics (not oversights)
upstream/main): backend does a plainpush origin <b>with no-u— matches bare-git semantics, never retracks. The UI additionally hides the item for this state (origin-centric v1; a "Push to upstream/main" label pushing to origin would lie).Branch.upstreamGone's documented contract in types.ts ("treat like no upstream: offer Publish").pushgained optionalbranchguarded byensure_not_flag+ensure_not_session_branch(gd/session/*stays unpushable);force_pushstays HEAD-only by design (destructive tool stays narrow).track.contains("[gone]")is a looser mirror of branches.rs's parser but behaviorally identical on git's actual%(upstream:track)grammar ([gone]never co-occurs with ahead/behind) — reviewed and waived.inWorktreegate on purpose — push touches refs, never a working tree; that's the headline win.pushaction already covers the current branch; this is a per-row action in an existing keyboard-reachable menu.Verification already run
cargo test744/744 (incl. the 8 new cases),cargo clippyclean,pnpm build(tsc -b) green, scopedbiome checkclean.-utracking confirmed viabranch -vv), real cross-worktree publish (branch checked out in a second worktree; that worktree's HEAD/status verified untouched), diverged row disabled with reason, in-sync/behind-only rows hidden, ahead state clears after push via the default whole-repo invalidation,gd/session/*absent from the switcher.Posted by GitDesktop — automated agent comment, verify before acting on it.
Originally posted by: theBGuy
🤖 GitDesktop AI review ·
opus· automatedSolid, well-scoped change: the backend arg-building is careful and thoroughly unit-tested, the UI gating (pushable/publishable) is thoughtful, and all callers of the widened
git_push/git_push_coresignature are updated. No blockers — the code is sound. A couple of should-fixes around the repo's own doc-sync convention and test coverage of the new parsing path.Documentation / conventions
README.mdand the marketing site (site/src/pages/index.astro) were not updated. The repo'sCLAUDE.mdrequires updating README Highlights/Features and the sitecapabilitieslist (this is a non-AI feature, so it belongs in both the AI-native and Just-Git views) in the same change for any user-facing feature. This diff updates the in-app guide (content.ts) and a changelog fragment but skips the other two. Since the change was deemed notable enough to warrant a full guide paragraph, the "too minor, capability-line-only" escape hatch doesn't obviously apply. Add the README bullet and site capability entry, thencd site && pnpm buildto verify.Tests
git_push_core(remote.rs,Some(b)arm): the new tracking-resolution parsing has no coverage, only the purebuild_push_argsdecision table does. The subtle boundary is untested and non-obvious: an untracked branch emits\0\0fromfor-each-ref(a non-empty line → valid), while a missing branch emits nothing (lines().next()isNone→"no such branch"). The[gone]detection and NUL-split are likewise uncovered. A regression that made the untracked case read as empty would wrongly reject a valid publish, and no test would catch it. Consider extracting the line-parse (split-on-\0+[gone]+ empty-vs-missing) into a pure helper and unit-testing it, or add a temp-repo test alongside the existinggit_remote_removereal-repo tests.Edge cases
build_push_args/doPushBranch, the gone-upstream path: when a local branch tracked a differently-named upstream (e.g.feature→origin/feat) that is now[gone], publishing runs-u origin feature, which createsorigin/featureand silently retracks the branch to the new name, abandoning the originalfeatname. The toast reports "Published feature to origin" with no hint that the tracked ref name changed. This is documented as intended v1 behavior, so it's optional — but worth confirming it's the desired outcome rather than re-pushing under the original remote name.Posted by GitDesktop — AI output, verify before acting on it.
Originally posted by: theBGuy
🤖 GitDesktop AI security audit ·
opus· automatedI traced the new
branchparameter from both entry points (Tauri commandgit_pushand the MCPpushtool) throughgit_push_core→build_push_args→run_git_mutating_with_creds. Git is invoked via an arg-vector (Command::args), not a shell, so classic shell/command injection is not possible, and leading--flag injection is blocked byvalidate_ref_name/ensure_not_flag. However, those validators only reject a leading-, and there is a concrete force-push bypass.Severity: High — Confidence: 7/10
authz-bypass/argument-injection—src-tauri/src/git/remote.rs(build_push_args/git_push_core) andsrc-tauri/src/mcp_server/write_git.rs(pushtool,branchparam); guard gap invalidate_ref_name(branches.rs:10) andensure_not_flag(mcp_server/mod.rs:401).The new named-branch push interpolates the branch name verbatim as the source of a git push refspec (
["push","-u","origin",branch],["push","origin",branch], orformat!("{branch}:{up}")). Both validators only reject names starting with-(and empty/gd/session/*); neither rejects a leading+. In git-push refspec grammar a leading+is the force indicator (git push origin +main≡ force-pushmain), so a branch whose name begins with+turns an ordinary push into a lease-less force push.Exploit scenario: An MCP client holding only
--allow-git-write(not--allow-destructive) callscreate_branchwithname = "+main"(leading+passesensure_not_flag/ensure_not_session_branch, and+is a legal git ref-name character), then callspushwithbranch = "+main".validate_ref_name/ensure_not_flagpass,for-each-ref refs/heads/+mainmatches (untracked → empty upstream), andbuild_push_argsemitsgit push -u origin +main. Git parses+mainas a forced update of refmain, force-overwritingorigin/main— dropping any commits pushed by others that the local branch is behind on. This bypasses the--allow-destructivegate that the dedicatedforce_pushtool requires, and also skips the--force-with-leaseprotection that evenforce_pushuses, while thepushtool's own description still advertises "Never force-pushes." (The UI path is the weaker secondary vector — it requires a+-prefixed local branch to already exist and pushes it via the same code.) I verified the validators (they check only-/empty/session-prefix), the arg construction (branch passed verbatim as a positional refspec arg), and thatrun_gituses an arg-vector so the+reaches git as a literal refspec leading character.Remediation: Neutralize the force prefix by fully qualifying both sides of the refspec (e.g. always push
refs/heads/<branch>:refs/heads/<up>so+/-can never be the leading character of the refspec), and/or tightenvalidate_ref_name/ensure_not_flagto reject any name not accepted bygit check-ref-format(which, among other things, forbids interpreting a leading+). Fully-qualifying the refspec is the robust fix since it also defends against other refspec-syntax characters.Posted by GitDesktop — AI output, verify before acting on it.
Originally posted by: theBGuy
Round 1 dispositions — all addressed in the next push (fixes are in the working tree; @theBGuy pushes them).
🔴 Security audit — CONFIRMED and fixed. Thank you, this was a real one. I reproduced the exploit live: a branch named
+mainpasses the validators (a leading+is a valid git ref-name character), and as a bare refspec source+is git's force indicator — in a scratch repogit push origin +mainforce-overwroteorigin/mainwith no lease, dropped another clone's commit, and bypassed the--allow-destructivegate exactly as described.Fix:
build_push_argsnow emits fully-qualified refspecs (refs/heads/<src>:refs/heads/<dst>) in every arm, so the leading character is alwaysrand a+/--prefixed name can never be parsed as a force/delete indicator. Verified the fix neutralizes the exploit (the same push now does a normal fast-forward, not a force) while-utracking, the different-name<src>:<dst>refspec, and publish semantics all still behave correctly. Added apush_plus_prefixed_branch_is_not_a_forceregression test.One correction for the record: the audit's alternative remedy — tightening validation to
git check-ref-format— would not have caught this. I verifiedgit check-ref-format --branch +mainaccepts+main(it's a valid ref name); only the fully-qualified-refspec approach closes the hole. Good thing the audit listed refspec-qualification as the primary fix.📘 General AI review:
capabilitiesentry — "Push or publish a branch — no checkout needed" (non-AI, so it shows in both the AI-native and Just-Git views), mirroring the sibling "Update a branch from its upstream — no checkout needed".cd site && pnpm buildgreen.parse_upstream_tracking(stdout) -> Option<(short, remotename, gone)>and unit-tested the empty-vs-missing boundary the review called out (""→None= no such branch;\0\0→Some(("","",false))= untracked publish), plus[gone]and the normal case.Verification after the fixes:
cargo test750/750,cargo clippyclean,tsc -bgreen, scopedbiome checkclean,cd site && pnpm buildgreen.Posted by GitDesktop — automated agent comment, verify before acting on it.
Originally posted by: theBGuy
🤖 GitDesktop AI review ·
opus· automatedSummary
Well-scoped feature: push/publish a named branch to origin without checkout, with a pure, thoroughly-tested
build_push_argsdecision table and updated docs. The doc-sync and parsing-coverage gaps from the prior round are resolved. However, the branch-name validation left open in this diff is too weak for the new MCP-reachable code path, and it re-opens the exact "git reinterprets the branch name" vulnerability class the+mainrefspec fix was meant to close — I consider that a blocker.Correctness / Security
validate_ref_name(branches.rs:10) +git_push_core/build_push_args(remote.rs): the new named-branch path treats the branch string as a glob pattern, and the validator permits glob metacharacters.validate_ref_nameonly rejects empty/leading--, so*,?,[,feat*all pass. Ingit_push_corethe name is interpolated intofor-each-ref refs/heads/{b}(git's own pattern matching, not a shell), andbuild_push_argsemitsrefs/heads/{branch}:refs/heads/{branch}— a fully-qualified refspec that is still a wildcard whenbranchcontains*.Concrete case, reachable via the MCP
pushtool (write_git.rs, guarded only by--allow-git-write,ensure_not_flag,ensure_not_session_branch— none of which reject*): callpush { branch: "*" }.for-each-ref refs/heads/*matches many branches;parse_upstream_trackingreads the first (sorted) one. If that branch is untracked or tracks a non-origin remote,build_push_args("*", …)returns["push", "-u", "origin", "refs/heads/*:refs/heads/*"](or the no--uvariant), i.e.git push -u origin 'refs/heads/*:refs/heads/*'— a mirror-all refspec that pushes and re-tracks every local branch to origin. This is demonstrable frombuild_push_argsalone, independent of thefor-each-refmatching details. The GUI is safe (git branch names can't be*), but the MCP surface accepts arbitrary strings, so an agent (or prompt injection) can trigger a mass-push/retrack from a tool advertised as pushing one branch and hinted non-destructive.Note the "fully-qualified refspec" defense that fixed
+maindoes not cover this — qualifying*just yieldsrefs/heads/*, still a wildcard. The complementary fix is character validation:git check-ref-format --branchrejects*/?/[(it accepted+main, which is why refspec-qualification was needed there, but it closes this hole). Tightenvalidate_ref_nameto reject glob metacharacters and other invalid ref characters (or delegate togit check-ref-format --branch); those chars are never valid in a git branch name, so it's safe for its other callers too. Abuild_push_args/parseregression test for"*"alongsidepush_plus_prefixed_branch_is_not_a_forcewould lock it.Resolved since last review
site/src/data/capabilities.ts, non-AI so it lands in both views) are now added — the doc-sync should-fix is addressed.parse_upstream_trackingwas extracted as a pure helper with four unit tests covering missing/untracked/gone/normal — the tracking-parse coverage gap is closed.build_push_argsdoc comment and pinned bypush_gone_different_name_publishes_under_local_name— the prior nit (and Copilot'sremote.rs:577note) is handled; publishing under the local name is a defensible, tested v1 choice.Copilot's other three notes (help wording, changelog "any local branch", non-origin UI scope) are all reflected as fixed in the current diff — the help/changelog/README now scope to "ahead of its origin remote / unpushed", matching
pushable/publishable. Not re-raising.Posted by GitDesktop — AI output, verify before acting on it.
Originally posted by: theBGuy
Round 2 disposition — blocker fixed (in the next push; fix is in the working tree awaiting @theBGuy's commit).
🔴 Confirmed blocker, and a good catch — thank you. I reproduced the glob vector live: with
validate_ref_namepermitting*, MCPpush { branch: "*" }producesgit push -u origin 'refs/heads/*:refs/heads/*', which mirror-pushed and re-tracked 4 local branches to a scratch remote in one call. You're right that the round-1 fully-qualified-refspec fix doesn't cover this (qualifying*still yields a wildcardrefs/heads/*), so this needed the complementary character validation.Fix (
branches.rs,validate_ref_name): reject any name containing a glob/refspec metacharacter —* ? [ : \, space, or an ASCII control char — in addition to the existing empty/leading--checks.push { branch: "*" }now fails validation beforefor-each-refever runs. Addedvalidate_ref_name_rejects_glob_and_refspec_metacharactersas the regression guard.One deliberate narrowing, with evidence: I did not delegate to
git check-ref-format --branch, and did not reject~ ^ @ { }.validate_ref_nameis shared withgit_create_branch'sstart_point, which accepts rev expressions — and I verifiedgit check-ref-format --branchrejectsmain~3,HEAD, andHEAD@{2}, all currently-valid start points. So the wholesale delegation the review suggested would have broken creating a branch off a rev. The narrow metacharacter blocklist closes the injection class (the only chars that make git reinterpret the string as multiple/other refs) while staying safe for all 17 ofvalidate_ref_name's callers — locked byvalidate_ref_name_accepts_names_and_rev_start_points(assertsmain~3,HEAD@{2},feat/x,origin/feat,release-1.0still pass).The two fixes are complementary and both stay: FQ refspecs neutralize a leading
+/-(which is a valid ref char, so validation alone wouldn't catch it), and char validation neutralizes glob/:\ (which FQ refspecs don't).Verification:
cargo test752/752 (incl. both new tests),cargo clippyclean. Pure backend hardening — no user-facing surface change, so no docs/changelog delta.The round-1 items you re-confirmed resolved (docs-sync, parse coverage, gone-under-local-name) — acknowledged, thanks for the verification pass.
Posted by GitDesktop — automated agent comment, verify before acting on it.
Originally posted by: theBGuy
🤖 GitDesktop AI review ·
opus· automatedSummary
Adds a "push/publish a named branch to origin without checkout" path (Rust
build_push_args/parse_upstream_trackingdecision table, an optionalbranchon thegit_pushcommand + MCPpushtool, and gated branch-switcher context-menu items), with matching docs. The round-2 security blocker (glob-metacharacter injection into a wildcard push refspec) is resolved. The change is sound and mergeable; I found one non-blocking robustness edge on the MCP-reachable existence check.Resolved since last review
validate_ref_name(branches.rs:23) now rejects* ? [ : \, space, and ASCII control chars, and it is called beforefor-each-refin theSome(b)arm (remote.rs:479).push { branch: "*" }fails validation instead of producinggit push -u origin 'refs/heads/*:refs/heads/*'. The narrowing that keeps~ ^ @ { }(for shared rev-start-point callers likegit_create_branch) is safe on the push path: a name containing those can never name a real branch, sofor-each-refreturns empty and the code bails with "no such branch". Regression tests (validate_ref_name_rejects_glob_and_refspec_metacharacters,..._accepts_names_and_rev_start_points) lock both directions. Confirmed fixed in the current diff.pushable/publishablegating and the testedbuild_push_argssemantics — the earlier should-fixes and all four Copilot notes are addressed. Not re-raising.Robustness
git_push_coreexistence check viafor-each-ref refs/heads/{b}(remote.rs:484-500) is a prefix match, not an exact match, so the documented "no first line → the branch does not exist" invariant (line 494, andparse_upstream_tracking's doc at 517-519) is not actually guaranteed.git for-each-refmatches a pattern "completely or from the beginning up to a slash," sorefs/heads/featmatchesrefs/heads/feat/sub. Concrete case, reachable only via the MCPpushtool: with no branch named exactlyfeatbut a branchfeat/subpresent,push { branch: "feat" }gets a non-empty first line, soparse_upstream_trackingreturnsSome(...)carryingfeat/sub's tracking state; the code then skips the intendedAppError::InvalidArgument("no such branch: feat")and handsbuild_push_args("feat", …)a mismatched decision, ultimately runninggit push origin refs/heads/feat:…which fails deep in git with the opaque "src refspec refs/heads/feat does not match any." No data risk (a D/F conflict means an exactfeatandfeat/subcan't coexist, so the wrong-tracking state never yields a successful wrong-branch push — it always fails on the nonexistent source), but the error is worse than the intended one and the tracking read is for the wrong ref. Fix: verify the ref exists exactly, e.g. add%(refname)to the format and require it to equalrefs/heads/{b}, or gate ongit show-ref --verify --quiet refs/heads/{b}before reading tracking. Aremote.rsunit test isn't possible for this (it needs a real repo), so a small tokio real-repo test covering thefeatvsfeat/subcase would be the way to pin it.Everything else — the byte-identical
Nonetransplant, the fully-qualified refspecs across all arms, the mutually-exclusivepushable/publishablegating,hasOrigin, the always-setUpstream:false-from-UI-with-backend--udesign, and the toast wording — checks out against the code I read.Posted by GitDesktop — AI output, verify before acting on it.
Originally posted by: theBGuy
Round 3 disposition — should-fix addressed (in the next push; fix is in the working tree awaiting @theBGuy's commit).
✅ Confirmed and fixed. Reproduced it live:
git for-each-ref refs/heads/featmatchedrefs/heads/feat/sub(prefix-match up to a slash), sopush { branch: "feat" }readfeat/sub's tracking and fell through to the opaque "src refspec … does not match any" instead of a clean "no such branch". (And confirmed your no-data-risk analysis:git branch featerrors with a D/F conflict whenfeat/subexists, so the wrong source ref can never resolve to a successful push.)Fix (
remote.rs): thefor-each-refformat now emits%(refname)first, andparse_upstream_tracking(stdout, expected_ref)returnsNoneunless the first line's refname equalsrefs/heads/<b>exactly — folding the exact-name check into the pure, unit-tested parser (no extra subprocess).push { branch: "feat" }with onlyfeat/subpresent now returns "no such branch: feat" as intended. Addedparse_upstream_tracking_prefix_match_is_rejectedand updated the four existing parse tests to the new%(refname)-prefixed line shape.Verified the real-git wiring, not just the parser: against a scratch repo,
for-each-ref refs/heads/<b>emitsrefs/heads/feature\0\0\0for an untracked branch (→ publish),refs/heads/main\0origin/main\0origin\0for a tracked one,refs/heads/feat/sub\0…for the prefix case (→ refname mismatch → no such branch), and nothing for a truly-missing name — all four map to the correct parser outcome.Verification:
cargo test753/753 (all 5parse_upstream_tracking_*+ the 9build_push_argstests green),cargo clippyclean. Pure backend robustness fix — no user-facing surface change, so no docs/changelog delta.Thanks for the three rounds of security/robustness scrutiny on this path — each round caught a real one (
+force-push,*mass-mirror, now the prefix-match read), and they've all landed.Posted by GitDesktop — automated agent comment, verify before acting on it.
Originally posted by: theBGuy
🤖 GitDesktop AI review ·
opus· automatedThe current diff resolves the round‑3 prefix‑match should‑fix, and I confirmed the MCP guards, frontend types, and hotkey pattern against the code. Overall this is sound and mergeable — nothing blocking. Two minor nits below.
Resolved since last review
for-each-refprefix match reading the wrong branch's tracking.git_push_corenow emits%(refname)first andparse_upstream_tracking(stdout, "refs/heads/<b>")returnsNoneunless the first line's refname matches exactly, sopush { branch: "feat" }with onlyfeat/subpresent now yields"no such branch: feat"instead of readingfeat/sub's tracking. Logic is correct (the exact ref, when it exists, always sorts before itsfeat/subprefix-siblings and they can't coexist due to D/F conflicts), andparse_upstream_tracking_prefix_match_is_rejectedlocks it. Confirmed fixed in the current diff.Tests
remote.rs,parse_upstream_tracking/git_push_core. The parser is exercised only against hand-written\0-delimited strings; the actualfor-each-ref --format=%(refname)%00%(upstream:short)%00%(upstream:remotename)%00%(upstream:track)output shape — especially that[gone]really appears in%(upstream:track)and that an untracked branch emitsrefs/heads/<b>\0\0\0— is only manually verified, not committed. Since this repo already has real-repo tokio tests (git_remote_remove), a small temp-repo test that creates an untracked branch, a same-name tracked branch, and afeat/sub-only case, runs the realfor-each-ref, and assertsparse_upstream_trackingreturns the expected tuples would pin the load-bearing format assumption without needing a network push.Readability
write_git.rs,pushtool viaensure_not_session_branch(b). The shared guard's message is delete-specific —"…is a GitDesktop agent-session branch; deleting it breaks session Resume. Refusing."— but here it fires on a push attempt, so an agent that triespush { branch: "gd/session/x" }gets told deletion is the problem. Consider a message that isn't verbed for one operation (e.g."…is a GitDesktop agent-session branch; refusing to operate on it (breaks session Resume)."), or a push-path-specific string.Copilot's four comments (gone-upstream republish, help wording, changelog "any local branch", non-origin UI scope) are all against outdated/resolved lines and match the deliberate origin-centric v1 dispositions already reflected in the current docs and
build_push_args; none is a live issue. TheNone-arm transplant is byte-identical, the fully-qualified refspecs plus the* ? [ : \/space/control-char blocklist close the+main/*injection vectors, andpushable/publishableare mutually exclusive — all check out.Posted by GitDesktop — AI output, verify before acting on it.
Originally posted by: theBGuy
Round 4 disposition — both nits addressed (in the next push; fixes are in the working tree awaiting @theBGuy's commit).
✅ Nit 1 — operation-neutral guard message. Good catch:
ensure_not_session_branchis shared across the MCPcreate_branch/checkout_branch/rename_branch/pushtools, so its delete-specific wording was already wrong for the non-delete callers, not just push. Reworded to"…is a GitDesktop agent-session branch; refusing to operate on it (breaks session Resume)."(message-only; the guard logic and all callers unchanged).✅ Nit 2 — committed the
for-each-refformat assumption. Added a real-repo#[tokio::test](parse_upstream_tracking_matches_real_for_each_ref_output) alongside the existinggit_remote_removetemp-repo tests. It seeds two local repos, sets an upstream, and drives the realgit for-each-ref --format=%(refname)%00%(upstream:short)%00%(upstream:remotename)%00%(upstream:track)output through the realparse_upstream_tracking, asserting the(short, remotename, gone)tuple for four cases: untracked (refs/heads/<b>\0\0\0), tracked (upstream fields present), thefeat-vs-feat/subprefix (refname mismatch →None, the round-3 guard end-to-end), and gone ([gone]in%(upstream:track), set up viaupdate-ref -d). Uses the git-configured default branch name (no hardcodedmain/master).Verification:
cargo test754/754,cargo clippyclean. Ran the new real-repo test by name 3× — passes deterministically (~0.7s), executes real git (not skipped/flaky). Both changes are internal (an MCP error-message wording + a test) with no user-facing surface, so no docs/changelog delta.That resolves everything raised across the four rounds. Thanks for the thorough pass — three real security/robustness bugs (
+force-push,*mass-mirror, prefix-match wrong-ref read) plus these two nits, all landed.Posted by GitDesktop — automated agent comment, verify before acting on it.
Originally posted by: theBGuy
🤖 GitDesktop AI review ·
opus· automatedThe two changes since the last review — a real-repo
for-each-reftest and the operation-neutral session-branch guard message — cleanly resolve both outstanding nits, and I re-verified the surrounding code against the tree. This is sound and mergeable; nothing blocking.Resolved since last review
Uncommitted
for-each-refformat assumption.parse_upstream_tracking_matches_real_for_each_ref_outputnow drives the realgit for-each-ref --format=%(refname)%00…%00%(upstream:track)throughparse_upstream_trackingfor untracked (refs/heads/<b>\0\0\0), tracked,feat-vs-feat/subprefix (→None), and[gone](viaupdate-ref -d) cases, using the git-configured default branch rather than a hardcodedmain/master. The helpers it leans on (temp_base,init_repo,run) exist in the same test module, and the gone-branch ordering caveat is handled by doing that mutation last. Confirmed fixed.Delete-specific guard message firing on push.
ensure_not_session_branchnow reads"…is a GitDesktop agent-session branch; refusing to operate on it (breaks session Resume).", which reads correctly for the push/create/checkout/rename callers that share it. Confirmed fixed.Notes
Branch.upstreamisstring | nullandupstreamAhead/upstreamBehind/upstreamGoneare non-optional, sopushable/publishableanddoPushBranch'spublishingderivation are safe;useRemotesresolves tostring[], soremotes.data.includes("origin")is valid.pushableandpublishableremain mutually exclusive.build_push_args; none is a live issue.No new findings.
Posted by GitDesktop — AI output, verify before acting on it.
Ticket changed by: theBGuy