Repository navigation
Initialize config-managed submodules at their branch tip - #802
Conversation
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. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e3f3ed3a4f
ℹ️ 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".
`init-submodules` initialized a missing config-managed submodule and left it at the commit the superproject pins. That pin is not a version anyone maintains: `.gitmodules` declares `ignore = all` for `.agents/shared`, hiding the submodule from `git status` and `git diff`, and `./config/pull` floats the on-disk copy without ever committing the moved gitlink. An audit on 2026-10-16 found the pin months stale in all 19 Spine consumer repositories, so every fresh `git worktree add`, CI checkout, and cloud session materialized hook scripts, skills, and guidelines nobody had run for months. A freshly initialized submodule is now advanced to the tip of its tracked branch with `./config/pull`'s own three steps, leaving it on the branch rather than in a detached `HEAD`. The "config-managed" rule is unchanged — `config` itself plus every submodule declaring a tracked `branch` — so consumer-owned submodules are still skipped, and the script still acts only on the `-` lines of `git submodule status`, so a submodule already present is neither rewound nor advanced. A float that fails warns and leaves the submodule wherever the steps that did succeed put it, at worst the pin, which is still a usable checkout; because the submodule is checked out by then, no later run retries the float, so the warning points at `./config/pull`. The script keeps its `exit 0` semantics throughout, so the `SessionStart` hook can never fail a session. A `branch = .` declaration, which gitmodules(5) defines as the branch the superproject is on, is resolved before it reaches a refspec: `git fetch origin .` is a `fatal: invalid refspec`, which would leave the pin with no retry. With the superproject itself detached there is no name to inherit, so the float is skipped and the pin kept. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
e3f3ed3 to
11ffb77
Compare
There was a problem hiding this comment.
🟢 Approved
No blocking issues were identified in the scoped initialization, branch selection, or failure handling.
No open issues 🎉
What changed in this PR
Updates bootstrap initialization so fresh working trees receive current config-managed submodules rather than stale pinned commits.
Changes:
- Initializes missing managed submodules and advances them to their tracked branch tips.
- Preserves existing checkouts, consumer-owned exclusions, and non-fatal failure handling.
- Updates bootstrap documentation and records verification.
| File | Description |
|---|---|
init-submodules |
Adds tracked-branch resolution and floating after initialization. |
AGENTS.md |
Documents branch-tip initialization and failure behavior. |
.agents/tasks/init-submodules-float-to-branch-tip.md |
Records rationale, implementation decisions, and verification. |
Review effort: Balanced
`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>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 896480d222
ℹ️ 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".
Problem
init-submodulesis the bootstrap that gives a fresh working tree its agentassets: a
git worktree add, a shallow CI checkout, or a cloud session startswith no submodules checked out, the
.agents/skills→.agents/shared/skillssymlinks dangle, and no skills, scripts, or guidelines resolve. A
SessionStarthook runs the script automatically to fix that.
Until now it initialized a missing config-managed submodule and stopped at the
commit the superproject pins. That pin is not a version anyone maintains:
.gitmodulesdeclaresignore = allfor.agents/shared, which suppresses thesubmodule from
git statusandgit diff;./config/pullfloats the on-disk copy to the tracked branch tip withoutcommitting the moved gitlink.
So the drift is invisible by design, and the pin keeps the value it had on the day
the submodule was adopted —
git log --all -- .agents/sharedreturns exactly onecommit, the adoption commit, in 17 of 19 consumer repositories.
An audit on 2026-10-16 found all 19 Spine consumer repos pinned at a
.agents/sharedcommit predatingd0abf64("Make the shared hook scriptsworktree-aware", 2026-10-05) — pins dating from June to 1 October. Every fresh
worktree, CI checkout, and cloud session therefore materialized hook scripts,
skills, and guidelines nobody had run for months. In
compiler, pinned at7bd2d9ee,.agents/scripts/session-work-tree.shdid not exist at all.Bumping the committed pins is a treadmill, and not even an available one:
Version Guardandensure-reports-updated.ymlmake a submodule-pointer-only PRstructurally unmergeable in 15 of the 19 repos. The durable fix is to stop fresh
checkouts from materializing the stale pin in the first place.
Change
A config-managed submodule that is not yet checked out is now initialized and
then advanced to the tip of its tracked branch, with
./config/pull's own threesteps (
fetch,checkout,merge --ff-only), leaving it on the branch ratherthan in a detached
HEAD.Everything else is deliberately unchanged:
configitself plus everysubmodule declaring a tracked
branchin.gitmodules— exactly what./config/pullfloats, so the two scripts cannot disagree. Consumer-ownedsubmodules (a Hugo theme, a vendored library, doc examples) declare no tracked
branch and are still skipped, so the automatic per-session run never tries to
clone — or fail on credentials for — a submodule the project does not manage.
pin nor advanced. The script still acts only on the
-lines ofgit submodule status, and floating makes a submodule+(ahead of the pin), soa second run does nothing and touches the network not at all. A tree someone
floated with
./config/pull, or deliberately parked on a commit, is neverdisturbed.
exit 0semantics are preserved. Withno network the clone is what fails, and the submodule is left uninitialized just
as before. A float that fails after a successful clone warns and leaves the
submodule wherever the steps that did succeed put it — at worst the pin, a
usable, if older, tree. Because the submodule is checked out by then, it is no
longer a
-line and no later run retries the float, so that warning points at./config/pull.The
path<TAB>branchtable replaces the old path-only set, so one scan of.gitmodulesanswers both "is this config-managed?" and "which branch?". Declaredentries come first, so a
.gitmodulesthat does giveconfigan explicit trackedbranch wins over the
masterdefaultpullapplies in step 1.The header comment block and the
AGENTS.mdparagraph thatmigratedistributesto consumers both documented the pin behaviour; both are updated.
Verification
In a throwaway
git worktreeofcompiler(pinned at7bd2d9ee):.agents/sharedlanded on theagentsorigin/mastertipafabda57, onheads/master, andconfigon its ownorigin/mastertip;.agents/scripts/session-work-tree.shis present afterwards — it does not existat
7bd2d9eeat all (git cat-fileon the pin fails);0.In a synthetic consumer repo with one branch-tracking and one consumer-owned
submodule:
file that exists only at the tip appeared;
a note on stderr;
exited
0.Reviewed by the
review-docsandspine-code-reviewagents; the latterreproduced all five invariants in its own throwaway repos. Both verdicts were
"approve with changes", and the changes they asked for are applied: the offline
wording now distinguishes a failed clone from a failed float, and the two
gitcalls inside the loop read from
/dev/nullso they cannot consume the loop'spath list.
A second review round, run against this exact diff before the PR was opened,
caught the documentation still overclaiming where the first round had only fixed
the inline comment: the header, the
AGENTS.mdparagraph, and the task file allsaid a failed float leaves the submodule "as the clone left it", which is untrue
for one of the failure cases the header itself lists — when
checkoutsucceedsand
merge --ff-onlydoes not,HEADhas already moved off the pin. All threenow match the inline comment, and both reviewers' companion point is documented
too: a float that fails after a successful clone is never retried, because the
submodule is no longer a
-line, so the warning now names./config/pullinstead of claiming the submodule was "left as-is". No executable line changed
beyond that one warning string;
bash -nandshellcheckare clean.Notes
init-submoduleslives in this repository, not inagents:migratecopies itto the consumer root (
cp init-submodules ..), deliberately outside anysubmodule so a fresh worktree always checks it out. Consumers pick the change up
on their next
./config/pull.no-
cwdfallback hardening independently.agentsrepo: theStalePinTestdocstring inscripts/tests/test_hooks.pysays a worktree's.agents/sharedsits "at thebranch's old pin, as
init-submodulesleaves it". The test itself builds thatfixture with
git submodule update --initand does not callinit-submodules,so nothing breaks — only the aside is now inaccurate.
🤖 Generated with Claude Code