fix(strategy): expire the empty-remote bootstrap marker instead of trusting it forever

30a6c53→main·

karthik-rameshkumar·3d ago·2 files·+40 added/-11 removed

Addresses trail review on #1744: a permanently-trusted marker means a remote that is later force-emptied (all branches deleted) or recreated empty under the same URL would sail past the guard, letting entire/checkpoints/v1 become the first/default branch again — the exact regression the fix prevents.

Bound the marker's trust with a TTL (pushBootstrapTTL, 1h) keyed on the file's mtime: a fresh, fingerprint-matching marker still short-circuits, but once it expires the remote is re-probed and the marker refreshed on success. This caps the network cost at one ls-remote per interval (vs. per push) while shrinking the re-emptied-remote risk window from "forever" to the TTL.

Adds a test asserting an expired marker is re-validated rather than trusted.

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

Sessions

01KXFZ70CRE94KPZGESHBMCHAEView transcript

Changes

2

• cmd/entire/cli/strategy

  • Mmanual_commit_push.go+25/-9

  • Mmanual_commit_push_test.go+15/-2

11 unmodified lines

// The empty-remote hazard exists only during the first-push window. Once
// every push target has been observed with at least one branch we record a
// fingerprint of that target set locally, so subsequent pushes short-circuit
// here instead of paying an ls-remote network round trip on every push
// forever. The fingerprint self-invalidates if the push URLs change.
// here instead of paying an ls-remote network round trip on every push. The
// marker self-invalidates if the push URLs change, and — because a remote can
// later be force-emptied or recreated empty under the same URL — it is only
// trusted for pushBootstrapTTL before the remote is re-probed. That bounds
// both the network cost (at most one probe per TTL) and the window in which a
// re-emptied remote could wrongly skip the guard.
fingerprint := pushTargetsFingerprint(targets)
if readPushBootstrapMarker(ctx) == fingerprint {
if stored, fresh := readPushBootstrapMarker(ctx); fresh && stored == fingerprint {
return false
}

// pushBootstrapTTL bounds how long a recorded bootstrap observation is trusted
// before the remote is re-probed. It caps the network cost at one ls-remote per
// interval while keeping the window small in which a remote that was emptied or
// recreated under the same URL could wrongly skip the guard.
const pushBootstrapTTL = time.Hour

// pushBootstrapMarkerPath is the repo-level file recording that every resolved
// push target has been observed to carry at least one branch. It lives under
// the git common dir (shared across worktrees) rather than in .git/config so it
return filepath.Join(commonDir, "entire", "checkpoint-push-bootstrap"), nil
}

// readPushBootstrapMarker returns the stored fingerprint, or "" if the marker is
// absent or unreadable.
func readPushBootstrapMarker(ctx context.Context) string {
// readPushBootstrapMarker returns the stored fingerprint and whether it is still
// fresh (written within pushBootstrapTTL). A stale, absent, or unreadable marker
// reports fresh=false so the caller re-probes the remote.
func readPushBootstrapMarker(ctx context.Context) (fingerprint string, fresh bool) {
path, err := pushBootstrapMarkerPath(ctx)
if err != nil {
return ""
return "", false
}
data, err := os.ReadFile(path) //nolint:gosec // path is git common dir + constant, not user input
if err != nil {
return ""
return "", false
}
return strings.TrimSpace(string(data))
info, err := os.Stat(path)
if err != nil {
return "", false
}
return strings.TrimSpace(string(data)), time.Since(info.ModTime()) < pushBootstrapTTL
}

// writePushBootstrapMarker records fingerprint. Best-effort: a failure only

Mcmd/entire/cli/strategy/manual_commit_push.go+25/-9

1 unmodified line

// A stale marker (targets changed) must not short-circuit: it falls back to
// probing and defers on the unreachable remote.
// A marker whose fingerprint doesn't match the current targets must not
// short-circuit: it falls back to probing and defers on the unreachable remote.
writePushBootstrapMarker(ctx, "stale-fingerprint")
require.True(t, deferCheckpointPushOnEmptyRemote(ctx, ps),
"a marker that does not match the current targets must not short-circuit")

// An expired marker must not be trusted even when the fingerprint matches:
// the remote could have been emptied/recreated since, so it is re-probed
// (and defers on the unreachable remote). Backdate the file past the TTL.
writePushBootstrapMarker(ctx, pushTargetsFingerprint(targets))
markerPath, err := pushBootstrapMarkerPath(ctx)
require.NoError(t, err)
stale := time.Now().Add(-pushBootstrapTTL - time.Minute)
require.NoError(t, os.Chtimes(markerPath, stale, stale))
require.True(t, deferCheckpointPushOnEmptyRemote(ctx, ps),
"an expired marker must be re-validated, not trusted indefinitely")
}
``

Mcmd/entire/cli/strategy/manual_commit_push_test.go+15/-2