Address review comments on promisor skip stamping

6fe8432→main·

toothbrush·4d ago·2 files·+42 added/-7 removed

• Check skipFetchAll/skipDefaultUpdate independently so a partially stamped entry is completed instead of skipped forever.
• Redact the URL from captured git config output before logging, since the key embeds the URL and a URL can carry credentials.
• Stamp the URL git actually fetched from: with ENTIRE_CHECKPOINT_TOKEN set, newCommand rewrites SSH targets to HTTPS and git records the promisor entry under the rewritten URL.

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

Changes

2

• cmd/entire/cli/checkpoint/remote

• Mgit.go+21/-7

• Mgit_test.go+21

94 unmodified lines

95
96
97
98
98
99
100
101
102
103
104
105
106
107
108
11 unmodified lines

120
121
122
116
117
118
123
120
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
126
140
141
128
142
143
144
145

94 unmodified lines

return out, fmt.Errorf("git fetch: %w", err)
}
if filtered && IsURL(opts.Remote) {
    markPromisorEntrySkipped(ctx, opts.Dir, opts.Remote)
    // Stamp the URL git actually fetched from: with a checkpoint token set,
    // newCommand rewrites SSH targets to HTTPS, and git records the
    // promisor entry under the rewritten URL.
    target := opts.Remote
    if token := strings.TrimSpace(os.Getenv(CheckpointTokenEnvVar)); token != "" && isValidToken(token) {
        target, _ = resolveTargetForTokenAuth(ctx, target)
    }
    markPromisorEntrySkipped(ctx, opts.Dir, target)
}
return out, nil
}
11 unmodified lines

// config section that wouldn't otherwise exist.
    return
}
if gitConfigBool(ctx, dir, "remote."+url+".skipFetchAll") {
    return
}
for _, key := range []string{"skipFetchAll", "skipDefaultUpdate"} {
    cmd := exec.CommandContext(ctx, "git", "config", "--local", "remote."+url+"."+key, "true")
    fullKey := "remote." + url + "." + key
    if gitConfigBool(ctx, dir, fullKey) {
        // Checked per key so a partially-stamped entry (e.g. an earlier
        // run failing between the two writes) still gets completed.
        continue
    }
    cmd := exec.CommandContext(ctx, "git", "config", "--local", fullKey, "true")
    if dir != "" {
        cmd.Dir = dir
    }
    if out, cfgErr := cmd.CombinedOutput(); cfgErr != nil {
        redactedURL := RedactURL(url)
        // The output can echo the key, which embeds the URL — and a URL
        // can carry credentials. Redact before logging.
        msg := strings.TrimSpace(strings.ReplaceAll(string(out), url, redactedURL))
        logging.Warn(ctx, "failed to mark promisor config entry as skipped for bulk fetches",
            slog.String("url", RedactURL(url)),
            slog.String("url", redactedURL),
            slog.String("key", key),
            slog.String("output", strings.TrimSpace(string(out))),
            slog.String("output", msg),
            slog.String("error", cfgErr.Error()),
        )
        return

Mcmd/entire/cli/checkpoint/remote/git.go+21

902 unmodified lines

903
904
905
906
907
908
909
910
911
912
913
914
915
916
917
918
919
920
921
922
923
924
925
926

902 unmodified lines

assert.False(t, gitConfigBool(ctx, cloneDir, "remote."+fetchURL+".skipFetchAll"))
    assert.False(t, gitConfigBool(ctx, cloneDir, "remote."+fetchURL+".skipDefaultUpdate"))
}

// TestMarkPromisorEntrySkipped_CompletesPartialStamp verifies the keys are
// checked independently: an entry with skipFetchAll already set (e.g. an
// earlier run failing between the two writes) still gets skipDefaultUpdate.
func TestMarkPromisorEntrySkipped_CompletesPartialStamp(t *testing.T) {
    t.Parallel()
    ctx := context.Background()

repoDir := t.TempDir()
    testutil.InitRepo(t, repoDir)

const url = "https://example.com/org/checkpoints.git"
    runIsolatedGit(ctx, t, repoDir, "config", "--local", "remote."+url+".promisor", "true")
    runIsolatedGit(ctx, t, repoDir, "config", "--local", "remote."+url+".skipFetchAll", "true")

markPromisorEntrySkipped(ctx, repoDir, url)

assert.True(t, gitConfigBool(ctx, repoDir, "remote."+url+".skipFetchAll"))
    assert.True(t, gitConfigBool(ctx, repoDir, "remote."+url+".skipDefaultUpdate"),
        "partially-stamped entry should be completed")
}

Mcmd/entire/cli/checkpoint/remote/git_test.go+21