Repository navigation
Read the current branch name exactly - #807
Merged
Merged
Conversation
`git rev-parse --abbrev-ref HEAD` shortens a ref to a *non-ambiguous* name, so a superproject that carries both a branch and a tag called `foo` resolves `.` to `heads/foo`. That is not a no-op: the fetch of `heads/foo` succeeds, the following `git checkout heads/foo` then fails in the submodule, and the float leaves the submodule stale and detached at the superproject's pin while warning about `origin/heads/foo`. `git branch --show-current` prints the branch name exactly and prints nothing when `HEAD` is detached, so it both fixes the ambiguity and keeps signalling detachment. `git symbolic-ref --short` is no alternative — it applies the same shortening and returns `heads/foo` too, which the throwaway repo confirmed. The `[ "$branch" = "HEAD" ]` half of the detachment guard is dropped along with the old command, since `--show-current` never prints that. `init-submodules` resolves `.` the same way on the unmerged branch of #802 and needs the identical change; the two scripts are kept in step deliberately. Reported by the Codex review of #804. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`#804` merged without the review fix it had just taken: config's auto-merge fired about two minutes before `6a39f556` was pushed, so the merge commit's branch-side parent sits one commit below it. The PR reads "merged" with green checks while `master` still ships the bug the review reported. The task's `status` is `in-review`, not `done`, so the plan file is kept rather than deleted: `.agents/tasks/README.md` ties deletion to the work having landed on `master`, and half of it did not. The remaining step is the re-land, and it is checked with `git merge-base --is-ancestor` rather than the PR's merged state, which was true for `#804` while the fix was absent. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`6a39f556` was pushed after the review that prompted it and was never itself reviewed, so this is its first pass. `review-docs` found the new comment wrapped `git rev-parse --abbrev-ref HEAD` and `git symbolic-ref --short` across two lines each, which breaks copy-paste and hides the command from a `grep` of the script that warns about it. Each now sits on one line, and the sentence about the failed checkout drops a needless conditional. Also reworded the plan item to say the fix commit was *pushed* two minutes after the merge, matching the Log entry, and put the link definitions in numeric order. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Contributor
There was a problem hiding this comment.
🟢 Approved
The focused fix addresses ambiguous branch names without changing literal-branch or detached-HEAD behavior, and no blocking issues remain.
No open issues 🎉
What changed in this PR
Fixes branch-name resolution in pull when a superproject branch and tag share the same name.
Changes:
- Uses
git branch --show-currentto preserve the exact branch name. - Removes the obsolete
HEADcheck while preserving detached-HEAD handling. - Updates the task record with verification and recovery history.
| File | Description |
|---|---|
pull |
Resolves exact branch names and retains the detached-state guard. |
.agents/tasks/pull-resolve-dot-branch.md |
Records verification and tracks landing the previously omitted fix. |
Review effort: Balanced
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Recovers a commit that #804 merged without.
#804 took a Codex review fix as
6a39f556, but config's auto-merge firedabout two minutes before that commit was pushed: the merge commit
6356d50ehas branch-side parent
65cd8609, whose own parentf1423f5esits one commitbelow the fix. The PR reads "merged" with green checks, and
masterstill shipsthe exact bug the review reported. This is the same failure mode that cost
#708 a fix once before.
6a39f556is cherry-picked here rather than retyped, so its message andauthorship survive.
What it fixes
pullresolves abranch = .submodule declaration — gitmodules(5)'s sentinelfor "the submodule's branch carries the same name as the superproject's current
branch" — to the superproject's branch name. It did so with
git rev-parse --abbrev-ref HEAD, which shortens a ref only as far as anon-ambiguous name. A superproject on branch
foothat also carries a tagfootherefore resolved toheads/foo: the fetch ofheads/foosucceeds, thefollowing
git checkoutfails, and the submodule is left stale at the pin whilethe warning names
origin/heads/foo.git branch --show-currentprints the name exactly, and prints nothing whenHEADis detached — so it fixes the ambiguity and keeps signalling detachment.git symbolic-ref -q --short HEADis not an alternative: it applies thesame shortening and returns
heads/footoo.The
[ "$branch" = "HEAD" ]half of the detachment guard goes with the oldcommand, since
--show-currentnever prints that. Review confirmed this isunreachable-code removal, not a narrowing: a branch literally named
HEADcannot be created (
fatal: 'HEAD' is not a valid branch name), so[ -z "$branch" ]is a complete guard.--show-currentneeds git 2.22 (2019).Nothing in Spine declares
branch = .today — all 20 local checkouts with atracked branch use the literal
master— so this is latent robustness, not alive outage.
master'sinit-submodulesdoes not resolve.at all (it onlyinitializes), so there is no sibling copy of this bug to keep in step with here.
Verification
Throwaway superprojects under a temp dir, each with its own
configsubmodulecarrying either the pre-fix or the fixed
pull, so the real./config/pullruns end to end. Submodule pin
A; remote tipsM2/F2/T2per branch.foo+ tagfooorigin/heads/foo→error: pathspec 'heads/foo' did not match any file(s), float warningAfoo+ tagfooorigin/fooF2, on branchfootopicorigin/topicT2, on branchtopiccontinueAbranch = master, run twiceorigin/masterM2, onmaster, idempotentbash -npasses.shellcheck pullreports only the pre-existing SC1091 info onsource migrate, identical to theorigin/masterbaseline.Notes for review
6a39f556was pushed after the review that prompted it and so was neveritself reviewed. This PR is its first pass;
review-docsflagged that the newcomment wrapped
git rev-parse --abbrev-ref HEADandgit symbolic-ref --shortacross two lines each, breaking copy-paste and hiding the commandfrom a
grepof the script that warns about it. Fixed in a separate commit sothe cherry-pick stays faithful to the original.
.agents/tasks/pull-resolve-dot-branch.mdis kept, not deleted. Itsstatusisin-review, notdone, and.agents/tasks/README.mdtiesdeletion to the work having landed on
master— half of it did not. Deletingit now would erase the only tracking record of work still in flight. It should
be deleted when this PR merges.
pull's pre-existing comment (added by#804) says
init-submodules"initializes before it resolves." and"floats only a submodule it has just cloned". Master's
init-submodulesdoesneither — it does not float and never resolves
.. The comment anticipates#802's unmerged behaviour.
🤖 Generated with Claude Code