fix: surface git-refs checkpoint fetch failures in explain fallback

52f48e2·

Soph·1w ago·5 files·+60 added/-33 removed

matchCheckpointPrefixWithRemoteFallback discarded the FetchCheckpointRef error in its git-refs branch, so an unreachable remote was reported identically to a genuinely absent checkpoint ("checkpoint not found") — re-masking exactly the error class 7bbdad09c fixed at the store layer.

The fallback now returns the fetch error and callers surface it as "checkpoint not found locally; fetching from remote failed: ...". A repo with no resolvable checkpoint source skips the fetch entirely (local absence is authoritative there), the commit-ref prefetch path deliberately keeps ignoring it (the downstream store read surfaces real failures with correct absent-vs-error semantics), and the v1-branch fallback keeps its historical fail-soft behavior for now.

Converts the self-healing KNOWN BUG skip in TestGitRefsClone_UnreachableRemoteMissingRefSurfacesRealError into hard regression assertions.

Co-Authored-By: Claude Fable 5 noreply@anthropic.com Claude-Session: https://claude.ai/code/session_012yi3hHGAGepwfrfjPETjGq

Sessions

ca56eea65fe0View transcript

Changes

5

• cmd/entire/cli

• Mattribution.go+4/-1

• Mexplain.go+7/-1

• Mexplain_export.go+38/-18

• Mgit_operations.go+2/-1

• integration_test

• Mrefs_fetch_test.go+9/-12

570 unmodified lines

570 unmodified lines

}
    defer lookup.Close()

matches, fresh := matchCheckpointPrefixWithRemoteFallback(r.ctx, io.Discard, lookup, cpID.String())
matches, fresh, fallbackErr := matchCheckpointPrefixWithRemoteFallback(r.ctx, io.Discard, lookup, cpID.String())
if fresh != lookup {
    defer fresh.Close()
}
if len(matches) != 1 {
    if fallbackErr != nil {
        return attributionCheckpointContext{}, fallbackErr
    }
    return attributionCheckpointContext{}, checkpoint.ErrCheckpointNotFound
}

Mcmd/entire/cli/attribution.go+4/-1

598 unmodified lines

598 unmodified lines

// Match the prefix locally; on miss, fetch from remote and retry once.
    matches, lookup := matchCheckpointPrefixWithRemoteFallback(ctx, errW, lookup, checkpointIDPrefix)
matches, lookup, fallbackErr := matchCheckpointPrefixWithRemoteFallback(ctx, errW, lookup, checkpointIDPrefix)

var fullCheckpointID id.CheckpointID
    switch len(matches) {

Mcmd/entire/cli/explain.go+7/-1

10 unmodified lines

10 unmodified lines

// FetchCheckpointRef fetches a single per-checkpoint ref (refs/entire/checkpoints/
// <shard>/<id>) from the checkpoint remote into the local ref of the same name,
// so the git-refs store can resolve a checkpoint written on another machine.
// Best-effort: the caller treats a fetch failure as "checkpoint not found".
// Callers must surface a fetch failure as a real error, not "checkpoint not
// found" — the checkpoint may exist on a remote we simply could not reach.
func FetchCheckpointRef(ctx context.Context, ref plumbing.ReferenceName) error {
    ctx, cancel := context.WithTimeout(ctx, 2*time.Minute)
    defer cancel()

Mcmd/entire/cli/git_operations.go+2/-1

93 unmodified lines

93 unmodified lines

t.Fatalf("explain should fail when the ref is missing and the remote is unreachable, got success:\n%s", out)
    }

// KNOWN BUG (regression class 7bbdad09c): the store layer preserves the real
    // fetch error, but explain's git-refs prefix-match remote fallback discards
    // the FetchCheckpointRef error (explain_export.go:216-227) and returns
    // ErrCheckpointNotFound, so an unreachable remote is reported identically to a
    // genuinely absent checkpoint. A parallel investigation owns the production
    // fix — this test does not touch production code. It self-heals: once the
    // fallback surfaces the fetch error, the skip stops firing and the assertions
    // below guard against regressing back to the masked message.
    if strings.Contains(out, "checkpoint not found") {
        t.Skipf("KNOWN BUG (7bbdad09c): unreachable-remote fetch failure masked as 'checkpoint not found':\n%s", out)
    // The store layer preserves the real fetch error (7bbdad09c), and explain's
    // git-refs prefix-match remote fallback must not re-mask it as a plain
    // "checkpoint not found" — that would report an unreachable remote
    // identically to a genuinely absent checkpoint.
    if strings.Contains(out, "checkpoint not found:") {
        t.Errorf("unreachable-remote fetch failure masked as 'checkpoint not found':\n%s", out)
    }
    // Reaching here means the fallback surfaced a real error (the bug is fixed):
    // err != nil is already asserted above, and the message is not the masked one.
    if !strings.Contains(out, "fetching from remote failed") {
        t.Errorf("explain should surface the real fetch failure, got:\n%s", out)
    }
}

Mcmd/entire/cli/integration_test/refs_fetch_test.go+9/-12