Fix Tracking Ref Updates for Remote Mirrors

Claude Code·Opus 5.5·Soph·7h ago·1hr 38min·1 Checkpoint·9 file changes·+535/-85·48.1K tokens

I reviewed trail 1502 (head c5b65e83, the same commit this worktree is on). It's a small, careful change and I'd approve it after you settle the first point below. The new gate tests pass locally (go test ./cmd/entire/cli/strategy -run TestCheckpointSyncAllowedForRemote). The trail itself shows CI and the agent findings review still pending, with no approvals yet.

What it does: a push to a remote other than the elected checkpoint sync remote can now carry checkpoints, if every push URL of both remotes names the same github.com owner/repo. For example, git@github.com:acme/widgets and entire://<cluster>/gh/acme/widgets. The election itself, capture, and the read chain don't change.

Findings

1. Correctness rests on an unverified claim about mirrors. Worth confirming before merge. The comment at checkpoint_sync_remote.go:159 assumes mirrors forward every pushed ref to GitHub, including entire/checkpoints/v1 and the git-refs backend's refs/entire/*. I can confirm that branch pushes go through to GitHub. Nothing in this repo shows that non-branch refs do, because that part is server-side. If the mirror keeps them, or GitHub rejects them (a ruleset, or the mirror's app lacking permission), you get this:

  • On the git-refs backend, prePushCheckpointRefs empties the queue once the mirror accepts the push. Those checkpoints then exist only in the mirror.
  • Reads go to the elected remote first, then fall back to origin. If the mirror is named origin, as in the test setup, reads still find them. If it has any other name (say entire), nothing ever reads it, so the checkpoints are lost to every read path and the CLI never warns.

Before this change, the worst case was "checkpoints waiting", which showed up as a hint. Now it can be silent loss. Either confirm with the server side that push-through covers refs/entire/* and non-default branches, or limit the match to setups where the mirror is also in the read chain.

2. entire status will keep reporting checkpoints as unpushed after a mirror push (git-branch backend). countUnpushedV1Commits (unpushed_checkpoints.go:51) compares against refs/remotes/<elected>/entire/checkpoints/v1. A push through the mirror updates refs/remotes/origin/..., not refs/remotes/github/.... So this exact scenario now prints "N checkpoints … they sync on next push" for data that is already on GitHub. formatUnpushedCheckpointsLine already notes that stale tracking refs over-report, but before this change the number was true in this case. One fix is to also count against the tracking refs of remotes that pass pushesToSameForgeRepository. At minimum, the doc line should mention it.

3. Minor: the entire:// cluster host is trusted without checking. ParseURL takes the forge from the path on any entire:// host, so entire://anything.example/gh/acme/widgets matches the elected GitHub remote. Remotes are local, user-written config, and the remote helper already sends the user's Entire credentials to that host, so I wouldn't block on this. Still, the gate exists to keep transcripts away from remotes the user didn't choose, and this widens it to any host that claims the right path. I'd note it in the function's comment or tighten it if it's cheap.

4. Nits

  • repo_remote.go:339 already builds a lowercased {forge, owner, repo} tuple (mirrorRepoRef) for the same "GitHub folds case" reason. It lives in package cli, so strategy can't import it. That's fine, but a shared helper in gitremote would stop the two from drifting apart.
  • SSH host aliases (git@github-work:…) never match. That's conservative and documented, but multi-account GitHub setups are where people tend to have both a GitHub remote and a mirror. Worth a line in the README paragraph so nobody files it as a bug.
  • The tests are thorough: they put the mismatched multi-URL first, and cover a raw-URL push, a pushurl override, and the elected remote's own push URL. There's no test for the mirror being the elected remote, but the code is symmetric, so that's optional.

I haven't posted anything to the trail. Should I add these as trail findings or comments?

  1. is confirmed, where is the best place to track this so it does not come up again in the next review
  2. can we make it check tracking refs against those too, alternatively for branches we could just do a second tracking ref? it does not really matter since it should always be a fast forward if the push go to one or the other?
  3. ignore
  4. yes do a shared helper in git_remote, why could we not match ssh hosts too? add the test
7h ago·9m

All four points are done. The changes are uncommitted in the worktree; I haven't committed, pushed, or run the full mise run check. Lint is clean and the tests for every touched package pass (gitremote, checkpoint/remote, strategy, docs/development).

1. Where the push-through fact lives. I put it in two places, because those are what the next reviewer reads:

  • The doc comment on pushesToSameForgeRepository. A reviewer reading the diff hits it first. It records that the mirror forwards entire/checkpoints/v1 and refs/entire/* as a verified contract, confirmed 2026-10-06. It also notes that CI doesn't cover it, and what breaks if it stops holding.
  • The checkpoint-sync bullet in docs/development/checkpoint-implementation.md. The repo instructions send anyone touching checkpoint sync there. It uses the same wording the doc already uses for the GitLab token contract ("verified … not enforced by CI").

2. Unpushed count. I went with your second option. After a push through an equivalent remote delivers a branch ref, the new advanceSyncRemoteTrackingRef in checkpoint_sync_remote.go moves the elected remote's tracking ref up to match the pushed remote's. I picked it over counting against several tracking refs for two reasons:

  • entire status stays at one comparison.
  • The next git push github no longer re-sends a ref GitHub already has.

You're right that it's always a fast-forward, but I still check with merge-base --is-ancestor and write with a compare-and-swap update-ref. If something unexpected happens, it leaves the ref for the next fetch to fix instead of moving it backwards. It runs after capture, so it compares against the final election, and it only applies to branch refs. The git-refs backend counts its push queue, not tracking refs, so it didn't need this. I checked that the fast-forward test fails when the ancestry check is removed.

4. Shared helper and SSH aliases. The new file gitremote/repository.go has:

  • gitremote.Repository with Info.Repository(), which returns the lowercased forge/owner/repo identity for a known forge.
  • gitremote.ResolveRepository(ctx, dir, env, url), which also resolves SSH host aliases. It runs ssh -G <alias>, which reads your ssh config the same way a push would but never connects, and takes its hostname line.

Aliases weren't matched before because the URL alone doesn't say where github-work goes; only your ssh config does. I added one limit: it only resolves when git would run plain ssh. With a custom GIT_SSH_COMMAND, core.sshCommand or GIT_SSH, the command might read a different config or not be OpenSSH at all, so those cases still don't match. To get that check, I moved effectiveSSHCommand from checkpoint/remote/git.go into gitremote as EffectiveSSHCommand, so there's now one copy instead of two.

Tests. There's a new testutil.InstallFakeSSH, which puts a fake ssh -G on PATH. OpenSSH reads ~/.ssh/config from the user's account entry, not $HOME, so a test can't redirect it. Without the fake, your own github-work alias could decide the result. The fake is a shell script, so these tests are skipped on Windows. The tests cover:

  • An alias that resolves to GitHub, in both the scp form and ssh://.
  • An alias as the elected remote, with a push through the mirror.
  • Aliases that should not match: one pointing to another host, and one pointing to GitHub but a different repository.
  • A custom ssh command, and ssh missing from PATH.

I removed the old "SSH host alias is rejected" case, since it would have run your real ssh and its result now depends on your ssh config.

I tried using the new type in repo_remote.go's mirrorRepoRef too, but backed it out. It added a branch that can never run and would silently drop that remote's entry from the error message if it ever did. That function is unchanged.

The README paragraph now explains the SSH-alias behaviour, including that a custom ssh command turns it off.

can you commit and push

6h ago·7m

Pushed. The repo rules say to check the trail's findings after every push: