fix(mirror): don't cache a suspended empty-upstream placement as mirrored

b7a33f3·

peyton-alt·2d ago·2 files·+78 added/-8 removed

The probe-cache write-through ran right after CreateMirror, keyed off the response's Suspended flag — but for an existing empty-upstream placement suspension only surfaces via the follow-up GetMirror read, so a suspended placement was cached as Mirrored and the onboarding checklist rendered a green 'Repo mirrored' rung for the cache TTL while the mirror never serves.

Move the write-through (healMirrorProbeCache) after every suspension signal has been checked.

Trail-Finding: 019f5de7-4acf-7bda-aae4-1936c5b7cbaa

Co-Authored-By: Claude Fable 5 noreply@anthropic.com

Sessions

01KXM40EH2PS8HRXXEX2NWNCQNView transcript

Changes

2

• cmd/entire/cli

• Mrepo_mirror.go+17/-8

• Mrepo_mirror_test.go+61

414 unmodified lines

if onCreated != nil {
    onCreated(created)
}
// Heal the onboarding probe cache on every successful placement — the
// setup checklist's own hint is `entire repo mirror create <slug>`, and a
// cached "not mirrored" must not survive the prescribed remediation. A
// suspended placement never serves, so it is deliberately not cached.
if !created.Suspended {
    slugOwner, slugRepo := githubSlug(owner, repo)
    defaultMirrorProbeCache().put(slugOwner+"/"+slugRepo, mirrorProbeResult{Mirrored: true}, time.Now())
}
outcome := mirrorCreateOutcome{created: created}
if created.Suspended {
    // The placement already existed and an admin has suspended it, so it
// suspended — suspension follows upstream access loss). Mirrors the old
// finishMirrorCreate behavior; the read is best-effort, so a transient
// GetMirror error just falls through to the benign "nothing to clone".
// The read must precede the probe-cache write-through: CreateMirror's
// Suspended flag does not cover this case, and caching first would
// render a green "Repo mirrored" rung for the cache TTL while the
// placement never serves.
if !created.Created {
    if m, gerr := c.GetMirror(ctx, coreapi.GetMirrorParams{MirrorId: created.MirrorId}); gerr == nil {
        if s, ok := m.Status.Get(); ok && s == coreapi.MirrorStatusSuspended {
        }
    }
}
healMirrorProbeCache(owner, repo)
return outcome, nil
}
healMirrorProbeCache(owner, repo)
if noWait {
    return outcome, nil
}
return outcome, werr
}

// healMirrorProbeCache records a serving placement in the onboarding probe
// cache — the setup checklist's own hint is `entire repo mirror create
// <slug>`, and a cached "not mirrored" must not survive the prescribed
// remediation. A suspended placement never serves, so callers write through
// only after every suspension signal is checked (CreateMirror's Suspended
// flag, plus the GetMirror read for existing empty-upstream placements).
func healMirrorProbeCache(owner, repo string) {
    slugOwner, slugRepo := githubSlug(owner, repo)
    defaultMirrorProbeCache().put(slugOwner+"/"+slugRepo, mirrorProbeResult{Mirrored: true}, time.Now())
}

// reportOneShotMirror renders the human output for `repo mirror create
// <github-url>` from the shared createAndAwaitMirror result. A nil
// outcome.created means CreateMirror itself failed — surface that error (nothing

Mcmd/entire/cli/repo_mirror.go+17/-8

229 unmodified lines

// A suspended *empty-upstream* placement is only detectable via the follow-up
// GetMirror read — CreateMirror's Suspended flag stays false for it. The probe
// cache write-through must wait for that read: caching first would render a
// green "Repo mirrored" rung for the cache TTL while the placement never
// serves.

// Not parallel: redirects the probe cache via XDG_CACHE_HOME.
func TestCreateAndAwaitMirror_EmptyUpstreamCacheWaitsForSuspensionRead(t *testing.T) {
    t.Setenv("XDG_CACHE_HOME", t.TempDir())
    ctx := t.Context()

serve := func(t *testing.T, status coreapi.MirrorStatus) *coreapi.Client {
        t.Helper()
        created := &coreapi.CreatedMirror{Created: false, MirrorId: "m1"}
        created.Empty = true //nolint:staticcheck // deprecated field is exactly the case under test
        srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
            w.Header().Set("Content-Type", "application/json")
            switch {
            case r.Method == http.MethodPost && r.URL.Path == mirrorsAPIPath:
                w.WriteHeader(http.StatusCreated)
                if err := printJSON(w, created); err != nil {
                    t.Errorf("encode created response: %v", err)
                }
            case r.Method == http.MethodGet && strings.HasPrefix(r.URL.Path, "/api/v1/mirrors/"):
                m := &coreapi.Mirror{}
                m.Status = coreapi.NewOptMirrorStatus(status)
                if err := printJSON(w, m); err != nil {
                    t.Errorf("encode mirror response: %v", err)
                }
            default:
                t.Errorf("unexpected request %s %s", r.Method, r.URL.Path)
                w.WriteHeader(http.StatusNotFound)
            }
        }))
        t.Cleanup(srv.Close)
        c, err := coreapi.NewWithBearer(srv.URL, "tok")
        require.NoError(t, err)
        return c
    }

t.Run("suspended is an error and never cached as mirrored", func(t *testing.T) {
        c := serve(t, coreapi.MirrorStatusSuspended)
        outcome, err := createAndAwaitMirror(ctx, c, "sus", "r", "c", false, time.Second, nil, nil)
        require.ErrorIs(t, err, errMirrorSuspended)
        require.Equal(t, coreapi.MirrorStatusSuspended, outcome.status)
        _, _, ok := defaultMirrorProbeCache().get("sus/r", time.Now())
        require.False(t, ok, "suspended empty placement must not be written through to the probe cache")
    })

t.Run("serving empty placement still heals the cache", func(t *testing.T) {
        c := serve(t, coreapi.MirrorStatusReady)
        _, err := createAndAwaitMirror(ctx, c, "ok", "r", "c", false, time.Second, nil, nil)
        require.NoError(t, err)
        probe, unreachable, ok := defaultMirrorProbeCache().get("ok/r", time.Now())
        require.True(t, ok, "serving placement should be cached")
        require.False(t, unreachable)
        require.True(t, probe.Mirrored)
        require.False(t, probe.Suspended)
    })
}

func countEq(xs []string, want string) int {
    n := 0
    for _, x := range xs {