Resolve the tracked branch exactly and fetch it by explicit refspec, in both scripts

[alexander-yevsyukov]

Problem

PR #802 taught init-submodules to float a freshly initialized config-managed
submodule to the tip of its tracked branch. Review of that change — and of this
one — turned up three ways the float can leave the submodule somewhere other
than on that tip. Each needs only valid .gitmodules configuration or
ordinary refs; none is reachable in Spine today, where every tracked branch is
the literal master and no .gitmodules declares shallow. This is latent
robustness, not a live fault.

1. An ambiguous branch name. A branch = . declaration is resolved against
the superproject's current branch. git rev-parse --abbrev-ref HEAD shortens
HEAD to a non-ambiguous name, so a superproject carrying both a branch and
a tag called foo yielded heads/foo. The fetch succeeds, the checkout then
fails, and the submodule keeps the pin — and is never retried, because it is no
longer a - line.

2. A single-branch submodule clone. A submodule declaring shallow = true
is cloned single-branch (a shallow clone implies --single-branch), as
+refs/heads/<default>:refs/remotes/origin/<default>. A bare
git fetch origin <branch> relies on the refspec configured for the remote to
update refs/remotes/origin/<branch> opportunistically as well, which a full
clone's does and that one's does not — so any other tracked branch reached
FETCH_HEAD alone and origin/<branch> never appeared.

3. A tag shadowing the tracked branch (predates #802). Plain
git checkout <branch> guesses a branch from a remote-tracking ref only when
the name resolves to nothing. If the submodule carries a tag of that name, the
name resolves, so Git quietly checks the tag out and detaches HEAD. The
following merge --ff-only then fast-forwards that detached HEAD to the
branch tip and reports success. The submodule ends on the right commit, on no
branch at all, with no warning — contradicting the script's own promise to leave
it "ON the branch rather than in a detached HEAD".

4. A tag named origin/<branch> capturing the merge. Found while reviewing
this PR. Even with the fetch and the checkout spelled out as full refs, the
final merge --ff-only was handed the origin/<branch> shorthand.
gitrevisions(7) resolves a bare name through refs/tags/ before
refs/remotes/, so a tag of that exact name wins it, and an ordinary clone
fetches tags, so it is there to win. The checkout placed the branch exactly and
the merge then fast-forwarded it onto whatever the tag pointed at, reporting
success: where the tag sits off the branch — after a force-reset past it — the
submodule ends on the right branch at the wrong commit, silently. For the
narrow-clone path that turned a warned failure into a silent wrong float, which
is precisely the trade this PR exists to avoid.

Change

init-submodules

• Resolve branch = . with git branch --show-current, which prints the name
  exactly and prints nothing when HEAD is detached — the case the existing
  guard already handles. ./config/pull resolves it this way and documents the
  same reasoning.
• Name the fetch destination:
  +refs/heads/<branch>:refs/remotes/origin/<branch>. The leading + matches
  what a default clone configures.
• Ask git show-ref --verify whether the branch exists locally instead of
  letting Git work the checkout out. Both answers are otherwise wrong in a
  reachable case: a fresh clone has a local branch only for the default branch,
  so -b alone would fail with already exists on the common path, while plain
  git checkout carries problems 2 and 3 above.
• Merge refs/remotes/origin/<branch> rather than the shorthand, so problem 4
  cannot undo the exact checkout. Every ref in the chain is now fully
  qualified, in both scripts.

merge --ff-only is kept after either checkout path, so a local branch that has
diverged from origin/<branch> is reported rather than overwritten. Nothing
force-moves a ref except the remote-tracking ref the fetch owns, which a default
clone force-updates anyway. exit 0 semantics are untouched, so the
SessionStart hook still cannot fail a session.

./config/pull

pull floats the same submodules with the same three steps and carried both
problems 2 and 3, so it gets the same fetch destination and the same
show-ref-guarded checkout rather than being left to diverge. Problem 1 was
already fixed there in #804.

The silent detach of problem 3 matters more in pull than in
init-submodules, which only ever touches a submodule it has just cloned:
pull floats submodules that are already checked out and in use and may
carry local work. So --ff-only still guards both checkout paths there too, and
the one case that protects is verified below. pull keeps its pre-existing
source migrate SC1091; the change adds no new shellcheck findings.

Verification

Against throwaway repositories, each case run with the merged script and with
this one:

Scenario	merged 896480d2	this PR
submodule tag shadows the tracked branch	detached, no warning	on the branch
shallow = true, non-default tracked branch	detached at pin, warned	on the branch, at the tip
branch = ., superproject branch name also a tag	detached at pin, warned	on the branch, at the tip
tag named origin/<branch> off the branch	on the branch at the stale tag commit, silent	on the branch, at the tip
ordinary tracked branch, full clone	on the branch	unchanged
branch = ., unambiguous name	on the branch	unchanged
detached superproject, branch = .	warns, keeps pin	unchanged
second run (idempotence), consumer-owned skip	no-op / skipped	unchanged

exit 0 on every path above. bash -n and shellcheck are clean.

For pull, end to end against throwaway consumer repositories built with a
config submodule and a shared one, so the real ./config/pull runs all three
of its own steps:

Scenario	before	after
shallow = true, non-default tracked branch	detached at pin, warned	on the branch
submodule tag shadows the tracked branch	detached, no warning	on the branch
local branch merely ahead of origin/<branch>	commit kept, silent	unchanged
local branch genuinely diverged	commit kept, warned	unchanged
ordinary tracked branch	on the branch	unchanged
second run (idempotence)	—	no-op

The local-work cases are what matter for pull, and they are unchanged: the
commits survive either way, and the two differ only in whether anything is
said. A branch merely ahead of the remote fast-forwards silently; one with
commits on both sides fails --ff-only and hits the existing warning. Neither
depends on the update strategy.

The verification paragraph of the first pull commit called the diverged case
silent. That was wrong, and the third commit corrects it: the fixture behind it
had the submodule's pin already at the branch tip, so update = merge
reconciled the branches before the float ran. Caught in review, re-tested with
the remote advanced as well and under both strategies.

Reviewed by the spine-code-review and review-docs agents. Both returned
"approve with changes" with no must-fix items; problem 3 above is the code
reviewer's finding, reproduced before being fixed, and its correction of the
--ff-only guarantee (it requires HEAD to be an ancestor of the branch tip,
not a clean working tree) is applied to the comments.

Notes

• An earlier revision of this PR, and the body of its first commit, said pull
  still carried the bare fetch and bare checkout and left that for a follow-up.
  That follow-up is now in this PR instead, so the two scripts stay in step;
  the parity clause in the init-submodules comment is reworded to match. The
  loop-stdin guard init-submodules carries is deliberately not added to
  pull: with three config-managed submodules declared, all three floated, so
  it is not reachable there today, and adding it would widen this diff for no
  demonstrable fault. Review added a reason to keep it out rather than add it
  half-way — the stdin-reading candidate in pull's loop is its unguarded
  git submodule update --init, which sits outside the subshell, so guarding
  only the subshell would not close the gap. If parity is wanted, both lines
  should get it, as its own hardening change.
• Two benign side effects of the explicit refspec, noted in review: the fetch
  now auto-follows tags that point into the fetched branch (additive only;
  existing tags are never rewritten), and in a single-branch clone the local
  branch it creates has no upstream tracking, since the configured refspec does
  not cover it. Nothing here relies on either.
• PR Initialize config-managed submodules at their branch tip #802 merged while this review feedback was being addressed, so these fixes
  come as their own PR rather than as commits on that branch.

🤖 Generated with Claude Code

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-19T16:07:33.680383Z	a08157d	New commits

ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[Copilot]

🟢 Approved

No unresolved findings remain, and the focused changes preserve failure handling with reported syntax and scenario checks passing.

No open issues 🎉

What changed in this PR

Hardens fresh submodule initialization so tracked branches resolve correctly even with ambiguous names or single-branch clones.

Changes:

• Resolves inherited branch names exactly and fetches into an explicit remote-tracking ref.
• Selects or creates the local branch explicitly while preserving fast-forward-only updates.
• Documents the edge cases and related follow-up work.

File	Description
init-submodules	Makes branch resolution, fetching, and checkout explicit.
.agents/​tasks/​init-submodules-float-to-branch-tip.md	Records the fixes and their rationale.

_{Review effort: Balanced}

[alexander-yevsyukov]

Re-requesting review: this PR has grown since the last pass. It now also gives
./config/pull the same explicit fetch destination and show-ref-guarded
checkout, because it carried both of the same faults — and there the tag-shadow
case detached HEAD with no warning at all. New commits since the last review:

• ba4762c7 — the pull change, plus a reworded parity comment in init-submodules
• a1b46b5f — corrects ba4762c7's claim that a diverged local branch is silent
  (ahead-only is silent; genuinely diverged fails --ff-only and warns)

The earlier approval predates both, and pull floats submodules that are
already checked out and in use, so the local-work paths are the part most worth
a second look.

@codex review

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a1b46b5fe6

ℹ️ 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".

[Copilot]

🟢 Approved

No unresolved findings remain, and reported syntax and control-flow checks support the focused changes.

No open issues 🎉

_{Review effort: Balanced}