Reviewing Mirror Checkpoint Routing Logic

Short answer: no, that line doesn't cause a regression. It's strictly additive for entire:// URLs.

Here's the reasoning, tracing the actual before/after behavior:

Before commit 855b8cdab, entire was not a case in deriveCheckpointURLFromInfo's switch. Every entire:// origin/push URL fell through to default: → returned an error → and both callers caught that error and fell back to resolveProviderCheckpointURL (provider host over HTTPS/SSH):

• FetchURL — util.go:114-125
• PushURL — the deriveCheckpointURLFromInfo call after line 229, same fallback

After the commit, the new case ProtocolEntire (util.go:312-321) does one of two things:

1. Forge matches the configured provider (ok && EqualFold(info.CanonicalHost(), host)) → derives entire://<cluster>/<forge>/<repo>. This is the new happy path.
2. Provider unknown/empty (!ok) or forge mismatch → returns an error → callers fall back to resolveProviderCheckpointURL — identical to the old behavior.

So the !ok || !EqualFold(...) guard only ever routes into the exact same provider-HTTPS fallback that ran before. Nothing that worked previously stops working.

Why the comparison is sound, not accidentally rejecting valid mirrors:

• For a gh mirror, info.CanonicalHost() maps forge gh → github.com (gitremote.go:77-82), and providerHost("github") → github.com (util.go:444-453). They match → derives. ✓
• There's no false-positive risk either: when the forge is unknown, CanonicalHost() falls back to info.Host, which for entire:// is the cluster host (e.g. aws-us-east-2.entire.io). That can never equal github.com/gitlab.com, so a mismatched mirror can't be mistaken for a match.

The one real behavior change (intended, not a regression): a user with an entire:// origin whose forge matches their github checkpoint provider previously had checkpoints routed direct to github.com; now they route through the mirror. That's precisely the commit's stated purpose ("Route checkpoints through entire:// push-through mirrors"), and ENTIRE_CHECKPOINT_TOKEN still forces the provider-HTTPS path (util.go:222-228) because the token can't ride through the helper.

If you were worried about a specific scenario (e.g. GitLab, or a gh mirror of GitHub Enterprise), tell me which and I'll trace it — but the gate itself preserves the prior fallback in every rejecting case.

No — the forge-match line does not error for forks, and there's a test proving it (util_test.go:337-343).

Two things to separate: the forge/host check (the line you asked about) and the owner check (a separate, pre-existing guard). Forks are about owner, and the forge-match line doesn't key on owner at all.

Trace for a fork entire://app.entire.io/gh/fork/app, provider github, checkpoint repo acme/checkpoints:

• info.Forge = gh → info.CanonicalHost() = github.com
• providerHost("github") = github.com
• !ok || !EqualFold("github.com", "github.com") → false → no error.

A fork can't cross forges (you can't fork github.com → gitlab.com), so the forge/host always matches. The owner living under fork instead of acme is irrelevant to this check — the derived URL takes its owner from config.Repo, not from the remote.

Where forks actually diverge is the owner-mismatch guard in PushURL (util.go:213-220), which runs before deriveCheckpointURLFromInfo:

So for a fork, PushURL short-circuits to the origin fallback with enabled=false (checkpoint push disabled) — it never reaches the forge line. That's exactly what the test at util_test.go:337-343 asserts (wantURL: entire://.../gh/fork/app, wantEnabled: false), and it mirrors the long-standing HTTPS fork case at line 313-319. It's a fallback, not an error, and it predates this commit.

FetchURL has no owner check, so a fork origin does reach the entire:// case — forge matches, and it derives entire://app.entire.io/gh/acme/checkpoints cleanly. No error there either.

One honest nuance worth naming (but not a regression from this change): the owner guard means a fork whose owner differs from the checkpoint-repo owner gets checkpoint push silently disabled. If supporting fork contributors pushing checkpoints to a shared acme/checkpoints is a goal, that guard — not the forge line — is what you'd need to revisit.

Done. Both tests pass, gofmt is clean, and go vet is quiet.

What I added

Two tests in cmd/entire/cli/strategy/checkpoint_remote_test.go, exercising the new entire:// routing through the production push path that PrePush actually uses (resolvePushSettings → remote.PushURL → deriveCheckpointURLFromInfo), against a real git repo with a real entire:// origin:

1. TestResolvePushSettings_WithCheckpointRemote_EntireMirror — the positive assertion you asked for: origin is an entire://app.entire.io/gh/org/main-repo mirror whose forge (gh) matches the github checkpoint provider, so checkpoints route through the same-cluster mirror → entire://app.entire.io/gh/org/checkpoints, not a direct github.com URL.

2. TestResolvePushSettings_EntireMirrorForgeMismatchFallsBackToProvider — the companion that guards the regression we discussed: an entire://.../et/... mirror (forge ≠ provider) must not ride the mirror; it falls back to the provider host (git@github.com:org/checkpoints.git).

Why not a literal e2e/tests/ test

A true E2E test can't exercise this. entire:// needs the git-remote-entire helper plus a live cluster to push/fetch — the E2E and integration harnesses have neither, and remote_operations_test.go:156-161 documents exactly this, deliberately punting entire:///URL-derivation coverage to the resolvePushSettings/unit level. The tests I added are the most end-to-end assertion feasible without live infra: real repo, real remote URL, real production resolver — I just assert the derived target rather than performing a network push (v1 branch is seeded locally to skip the real fetch, matching the existing HTTPS/SSH tests).

If you do want a real-infra E2E variant (spinning up a cluster mirror), that's a larger lift and would belong behind a gated/opt-in harness — say the word and I'll scope it.

Note

The diagnostic flagged at line 985 ([]byte(fmt.Sprintf(...)) → fmt.Appendf) is pre-existing code that shifted down 82 lines from my insertion — not mine, and unrelated to this change. I left it alone. My diff is purely additive.

I haven't run mise run check in full or committed — let me know if you'd like me to.

Author identity matches the DCO sign-off (paulo@entire.io). Here's what I'll commit — waiting for your explicit go-ahead before running git commit (so you can unlock your GPG agent):

File (1):

• cmd/entire/cli/strategy/checkpoint_remote_test.go (+82, test-only)

Draft message:

A couple of notes:

• I did not run the full mise run test:ci (unit + integration + canary) — only fmt, lint (0 issues), and the strategy package tests. Say the word if you want the full suite first.
• Only the test file will be staged; nothing else is pending.

Give me the go-ahead and I'll commit.

Committed as d8cd7647f on swirling-doodling-waffle — 1 file changed, +82 (test-only). GPG signing succeeded and the working tree is clean. Nothing pushed.