Repository navigation
Resolve a branch = . declaration before it reaches a refspec - #804
Merged
Merged
Conversation
`./config/pull` floats each config-managed submodule with `git fetch origin <branch>`, taking `<branch>` straight from the `.gitmodules` entry. gitmodules(5) gives `.` a special meaning — the branch in the submodule should carry the same name as the current branch in the current repository — so it is a sentinel, not a ref. Passed through as-is, it makes `git fetch origin .` die with `fatal: invalid refspec '.'`, so the float warned about a nonexistent `origin/.` and left the submodule wherever it was. The value is now resolved to the current branch name before anything is printed or touched, so the `Floating ... -> origin/<branch>` message names the real branch. This mirrors, step for step, the fix `init-submodules` received in #802, keeping the two scripts in agreement about how a tracked branch is read. A detached superproject has no branch name to inherit. Unlike `init-submodules`, which floats only a submodule it has just cloned, this loop acts on submodules that are already checked out and in use, so guessing a branch could move one off the commit it is on. Such an entry is therefore skipped outright, which also skips its `git submodule update --init` — the point of skipping, since with the default `checkout` strategy that command resets an already-floated submodule to the superproject's pin. A submodule not yet checked out consequently stays that way here, and `init-submodules`, which initializes before it resolves `.`, remains what materializes it. The warning says that re-running from a branch is what makes the entry float. Everything else about the loop, including its structure and its non-fatal exit behavior, is unchanged; a declaration naming an ordinary branch produces byte-identical output. Nothing in Spine declares `.` today, so this is latent robustness rather than a live outage. Found by a Codex review of #802. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
alexander-yevsyukov
enabled auto-merge
October 16, 2026 21:53
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f1423f5e0d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
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 localized change resolves inherited branches before mutation and safely skips detached states, with no unresolved findings.
No open issues 🎉
What changed in this PR
Updates config/pull to resolve Git’s branch = . sentinel before floating tracked submodules.
Changes:
- Resolves
.to the superproject’s current branch. - Warns and skips the entry when detached, leaving the submodule untouched.
- Documents the implementation and verification.
| File | Description |
|---|---|
pull |
Resolves inherited branches before submodule operations. |
.agents/tasks/pull-resolve-dot-branch.md |
Records rationale, implementation plan, and verification results. |
Review effort: Balanced
alexander-yevsyukov
added a commit
that referenced
this pull request
Oct 19, 2026
`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>
alexander-yevsyukov
added a commit
that referenced
this pull request
Oct 19, 2026
`#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>
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.
./config/pullfloats each config-managed submodule —configitself plusevery submodule declaring a tracked
branchin.gitmodules— withgit fetch origin <branch>,git checkout <branch>,git merge --ff-only origin/<branch>, taking<branch>straight from the.gitmodulesentry.gitmodules(5) gives
.a special meaning: the branch in the submodule shouldcarry the same name as the current branch in the current repository. It is a
sentinel, not a ref. Reproduced against the pre-fix script:
The value is now resolved to the current branch name before anything is
printed or touched, so the message names the real branch. This mirrors, step
for step, the fix
init-submodulesreceived in #802, where a Codex reviewspotted this same gap in
pull. Keeping the two scripts in step about how atracked branch is read is the point — it was left out of #802 only because it
is pre-existing, affects already-checked-out submodules that #802 does not
touch, and would have widened a reviewed three-file diff.
The detached-superproject path
A detached superproject has no branch name to inherit, and
pull— unlikeinit-submodules, which floats only a submodule it has just cloned — acts onsubmodules that are already checked out and in use. Guessing a branch could
move one off the commit it is on, so the entry is skipped outright.
Worth a reviewer's eye: skipping outright also skips that entry's
git submodule update --init. That is deliberate — with the defaultcheckoutstrategy, the command resets an already-floated submodule to thesuperproject's pin, so running it would be the opposite of "left as-is". The
consequence is that a submodule not yet checked out stays that way here;
init-submodules, which initializes before it resolves., remains whatmaterializes it. The comment in the diff says so.
Verification
Throwaway superproject + submodule remotes under a temp dir, running the real
./config/pullthrough a throwawayconfigsubmodule:feature,branch = .Floating 'shared' -> origin/feature; submodule advanced to thefeaturetip and landed on the branch, not detachedbranch = .0)branch = master.resolving to a branch the submodule remote lackspull's existing float-failure warning, naming the resolved branchThe detached case, verbatim:
bash -nandshellcheckare clean —shellcheckreports only thepre-existing SC1091 info on
source migrate, identical before and after. Ashell-only diff has no Gradle build target, and
confighas no rootversion.gradle.kts, so the version gate does not apply.Nothing in Spine declares
.today (all 20 local checkouts with a trackedbranch use the literal
master), so this is latent robustness rather than alive outage.
🤖 Generated with Claude Code