fix(strategy): scope empty-remote guard to configured remotes; document staleness limit

b5ddba3→main·

karthik-rameshkumar·3d ago·2 files·+35 added/-0 removed

Addresses two trail-review findings on the local-ref guard:

• URL-push defers forever (fix): git hands the pre-push hook the bare URL as the remote arg when a push targets a URL directly (verified: $1 is the URL, not a name). git never records refs/remotes//*, so remoteHasTrackingRefs was always false → checkpoints deferred permanently for URL-push workflows. Guard now runs only when the arg resolves to a configured remote (isConfiguredRemote via git remote get-url, local/no-network); a bare-URL target publishes instead of stranding metadata. The first-branch hazard the guard exists for always uses a named remote, so no protection is lost. This also matches the reviewer's "…or default to origin" note.

• Recreated-empty-remote staleness (documented): a tracking ref left from before a remote was deleted and recreated empty under the same URL reads as "established", so v1 could publish to the now-empty remote. Detecting that needs a network round trip, which this design deliberately avoids; the earlier TTL only bounded a network probe's cache and no longer applies. Rare and recoverable — noted inline as an accepted limitation rather than reintroducing the network hit.

Test adds a bare-URL case asserting the guard publishes (does not defer).

Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com

Changes

2

• cmd/entire/cli/strategy

• Mmanual_commit_push.go+29

  • Mmanual_commit_push_test.go+6

186 unmodified lines  
187  
188  
189  
190  
191  
192  
193  
194  
195  
196  
197  
198  
199  
200  
201  
202  
203  
204  
205  
206  
207  
208  
209  
210  
211  
212  
213  
214  
215  
216  
217  
218  
219  
220  
221  
222  
223  
224

186 unmodified lines

if ps.hasCheckpointURL() {
        return false
    }

// The hazard only arises for a configured remote (the `git remote add
    // origin …` then first-push flow). Pushing straight to a bare URL hands that
    // URL to the hook as the remote arg, and git never records a
    // refs/remotes/<url>/* tracking ref for it — so a tracking-ref check would
    // defer the metadata forever. Publish for a non-configured (URL) target
    // rather than strand it; the first-branch scenario always uses a named
    // remote.
    if !isConfiguredRemote(ctx, ps.remote) {
        return false
    }

// Known limitation, accepted for the no-network design: a tracking ref left
    // over from before a remote was deleted and recreated empty under the same
    // URL reads as "established", so v1 would publish to the now-empty remote.
    // Detecting that requires asking the remote — the network round trip we
    // deliberately avoid here. The scenario is rare and its default branch is
    // recoverable by resetting it on the forge.
    return !remoteHasTrackingRefs(ctx, ps.remote)
}

// isConfiguredRemote reports whether name is a configured git remote, as
// opposed to a bare URL that git passes through verbatim when a push targets a
// URL directly. Local and best-effort (reads config, no network); any error is
// treated as "not a configured remote".
func isConfiguredRemote(ctx context.Context, name string) bool {
    if name == "" {
        return false
    }
    return exec.CommandContext(ctx, "git", "remote", "get-url", name).Run() == nil
}

// remoteHasTrackingRefs reports whether any refs/remotes/<remote>/* ref exists
// locally. Its presence means the remote has been fetched from or pushed to
// before and so already has at least one branch. Local-only and best-effort:

Mcmd/entire/cli/strategy/manual_commit_push.go+29

36 unmodified lines  
37  
38  
39  
40  
41  
42  
43  
44  
45  
46  
47  
48

36 unmodified lines

require.True(t, deferCheckpointPushOnEmptyRemote(ctx, ps),
        "a remote with no tracking refs must defer")

// A push straight to a bare URL is not a configured remote; git never records
    // a tracking ref for it, so the guard must publish rather than defer forever.
    require.False(t,
        deferCheckpointPushOnEmptyRemote(ctx, pushSettings{remote: "https://example.invalid/repo.git"}),
        "a bare-URL push target must not defer")

// git records a remote-tracking ref after the first successful push; simulate
    // that locally (no network). The remote is now established → publish.
    run("update-ref", "refs/remotes/origin/main", "HEAD")

Mcmd/entire/cli/strategy/manual_commit_push_test.go+6