Revert skipFetchAll Stamping Logic

What would you like to clarify? I want to make sure I understand your intended approach before implementing.

To help frame it: my open question is really about where the CLI creates remotes. In the production code paths I searched, filtered fetches go against a bare URL and git implicitly records the remote.<URL>.promisor section — there's no explicit git remote add. So if you have a specific remote-creation site in mind (e.g. in the git remote-entire helper, checkpoint setup, or somewhere I haven't looked), point me at it and I'll wire skipFetchAll in there.

What's on your mind?

Done. Here's what changed and why.

What I changed

cmd/entire/cli/checkpoint/remote/git.go — reverted PR #1733's approach and reimplemented it as stamp-on-creation:

• Removed markPromisorEntrySkipped, which read remote.<url>.promisor and (re)stamped skipFetchAll/skipDefaultUpdate on any matching URL section on every filtered fetch — the "marks all remotes with a full URL" behavior you wanted gone.
• Added gitRemoteSectionExists — before the fetch, checks whether a remote.<url>.* section already exists.
• Added markRemoteSkipped — sets skipFetchAll/skipDefaultUpdate with no promisor check.
• Fetch() now, for a filtered URL fetch, records whether the section existed beforehand and only stamps when it did not — i.e. only on remotes we're creating. Pre-existing remotes (user's own, or phantoms from older CLI versions) are left untouched.

Net effect matching your three goals: no promisor lookup, we only ever touch remotes we create, and those adhoc URL remotes get the skip flags so git fetch --all / git remote update don't dial them.

cmd/entire/cli/checkpoint/remote/git_test.go:

• Renamed the new-remote test, added TestFetch_ExistingURLRemoteNotReStamped (verifies we leave pre-existing sections alone), and replaced the promisor-completion test with direct coverage of the two new helpers.
• Moved the gitConfigBool helper here since production no longer uses it.

One deliberate decision worth flagging

I kept the filtered-fetch gate and left testenv.go (the integration config guard) as-is:

• The filtered gate matters: an unfiltered URL fetch makes git create no remote.<url> section, so stamping there would invent an orphan section — the exact clutter you want to avoid. Stamping only happens on filtered fetches, which is the only case git records a URL-keyed section.
• The guard change from #1733 is still needed: git still writes promisor/partialclonefilter, and we still add the skip stamps on new remotes, so the guard must keep tolerating those sections. Reverting it would break integration tests without changing behavior.

Verified: mise run fmt, mise run lint (0 issues), the remote package tests, and the integration config-guard tests all pass.

I have not committed — let me know if you'd like me to, and on which branch (currently skip-fetch-all).

Two files, both in checkpoint/remote. Draft commit message:

Committing to the current branch skip-fetch-all. Give me the go-ahead once your GPG agent is unlocked and I'll run the commit.

This confirms the picture. Here's the answer.

Answer: No new coverage needed — the push flow already routes through the same chokepoint

Push itself never creates these remotes. git push <url> <refspec> doesn't write any remote.<url>.* config section — only a filtered fetch from a bare URL does (git records the promisor entry so it can lazy-fetch later). So Push/PushWithOptions, ls-remote, and FetchBlobs (fetch-pack, plumbing) are all safe.

But the push flow does fetch — and it goes through the same remote.Fetch. During a push, strategy/push_common.go calls fetchAndRebaseRefCommon → ResolveFetchTarget (resolves a named remote to a URL when filtered fetches are on) → remote.Fetch (push_common.go:453). That's exactly the code path my change instruments, so a URL-keyed remote created during a push is stamped on creation just like any other.

I traced every production caller and --filter=blob:none is only ever applied inside remote.Fetch. All filtered-URL fetches funnel through it:

The other direct git invocations don't create URL-keyed sections:

• repo_clone.go — git clone <url> (no --filter) → creates named remote.origin, not a URL section.
• trail_cmd.go — git fetch --no-tags origin → named remote, no filter.

So remote.Fetch is the single point where these adhoc remotes come into existence, and the stamp-on-creation logic sits right there. There's no separate push (or other) workflow that would slip past it.

Root cause

My change has a self-healing regression. Here's the chain:

1. entire resume does lazy checkpoint fetches — FetchCheckpointRef/FetchBlobsByHash — which do filtered fetches from the checkpoint URL through remote.Fetch.
2. Git writes remote.<url>.promisor=true eagerly, during connection setup — even when the fetch then fails (proven above: exit=128, config still written). Resume's lazy fetches routinely fail or partially fail (ref not on that remote, fallback-to-origin paths, timeouts).
3. In Fetch, an error returns before the stamp:
   So git created the URL remote, but we never stamped it.
4. On any later fetch from that URL, my gitRemoteSectionExists check now returns true → stampNewRemote = false → it is never stamped. The existedBefore gate makes the miss permanent.

The original PR #1733 code didn't have this: it re-checked remote.<url>.promisor on every successful fetch and stamped if unstamped — it self-healed. My "only stamp brand-new sections" rule (per your instruction to drop the promisor check and only mark new remotes) removed that self-healing, so any promisor entry left behind by a failed fetch stays unstamped forever.

My standalone repro of the happy path passes, which is why the unit tests didn't catch it — the gap is specifically the fetch-fails-after-git-writes-promisor path.

The fix

Keep your design (no promisor-key read, only stamp remotes we create, never touch pre-existing ones) but make it robust to failed fetches: check section existence before and after the fetch, and stamp when it newly appeared — regardless of fetch success.

Why this satisfies all your goals:

• No promisor check — uses generic section existence.
• Only stamps remotes we create — appeared during this fetch (!before && after).
• Never invents orphan sections — if git wrote nothing (fetch died before promisor setup), after is false → no stamp.
• Fixes the bug — a filtered fetch that fails after git writes the promisor still gets stamped.

One trade-off to flag: this self-heals only entries this code creates. A promisor remote left unstamped by an older CLI version won't be back-filled (that's the "don't rewrite the user's existing config" behavior you asked for). PR #1733's version would have re-stamped those. If you'd rather also clean up legacy entries, the alternative is to drop the existedBefore gate and stamp any unstamped promisor section — but that re-touches pre-existing config, which is what you wanted to avoid.

The fix belongs on the skip-fetch-all branch (commit 870ff89f4), which isn't currently checked out — the working tree is on group-root-help-commands. Want me to switch to skip-fetch-all and apply it (plus a regression test that fails the fetch and asserts the stamp lands)?

Summary of the fix:

• git.go: stamping now checks section existence before and after the fetch and stamps when the section newly appeared — regardless of fetch success. This fixes the resume bug: git writes remote.<url>.promisor eagerly during connection setup, so a filtered fetch that later fails still leaves a phantom remote; the old code returned early on error and never stamped it, and the existedBefore gate then made the miss permanent. The post-fetch existence recheck also prevents inventing an orphan section when the fetch dies before git writes anything.
• git_test.go: added TestFetch_FailedFilteredFetchStillStampsNewRemote (verified it fails without the fix, passes with it).

Verified: fmt clean, lint 0 issues, package tests pass, integration guard tests pass.

Draft commit message:

Want me to commit this to skip-fetch-all? Give me the go-ahead once your GPG agent is unlocked.

Committed to skip-fetch-all as 4ccc0dcf6.

The branch now has two commits on this work:

• 870ff89f4 — stamp skipFetchAll only on remotes we create (the initial revert + stamp-on-creation)
• 4ccc0dcf6 — stamp phantom remotes left by failed filtered fetches (the resume-regression fix)

This resolves the issue you hit: a filtered fetch during entire resume that fails after git has already written remote.<url>.promisor now still gets stamped, so it won't linger as an unstamped phantom remote.

One thing still worth your awareness (not addressed, by design): promisor entries created by an older CLI version before this change won't be back-filled — the code only stamps sections it creates itself. If you want to also clean up pre-existing unstamped entries, that's a separate decision (it means re-touching config the user already has). Let me know if you'd like that too.

Here's a PR title and description for the branch:

Title

Description

Want me to push the branch and open the PR with gh (note: gh isn't currently authenticated in this session — you'd need to gh auth login via ! first), or will you create it?

Both review findings addressed:

• Finding 1 (cancelled fetch): extracted stampNewlyCreatedRemote, which runs the git config read/write on context.WithoutCancel(ctx) + a fresh 10s timeout. A timed-out filtered fetch (ctx already expired) no longer causes the stamp helpers to no-op, so the phantom still gets stamped. Regression test: TestStampNewlyCreatedRemote_StampsUnderCancelledContext.
• Finding 2 (prefix match): gitRemoteSectionExists now parses git config --list --name-only and compares the full subsection exactly (keys carry no dots, so the last dotted component is the key). A .../repo.git section no longer makes a .../repo fetch look pre-existing. Regression test: TestGitRemoteSectionExists_ExactSubsectionMatch.

Both tests verified to fail without their fix. fmt clean, lint 0 issues, package tests pass.

Draft commit message:

Want me to commit to skip-fetch-all? Go-ahead once your GPG agent is unlocked.

The commit failed — your GPG agent refused the signing operation (likely locked/timed out). The changes are staged and ready. Unlock your GPG agent, then tell me to retry (or run ! git commit -S yourself). I'll re-run the same commit once you're ready.

Committed as 09a7aaa28. The skip-fetch-all branch now has three commits:

• 870ff89f4 — stamp skipFetchAll only on remotes we create
• 4ccc0dcf6 — stamp phantom remotes left by failed filtered fetches
• 09a7aaa28 — stamp on detached ctx and match remote URL exactly (review fixes)

Both review findings resolved with regression tests. Let me know if you'd like the PR description updated to fold in these fixes, or anything else.

Removed skipDefaultUpdate everywhere — it's deprecated in favor of skipFetchAll (which git honors for both git fetch --all and git remote update):

• git.go: markRemoteSkipped now writes only skipFetchAll (loop collapsed to a single config write); comments updated.
• git_test.go: dropped the skipDefaultUpdate assertions; renamed TestMarkRemoteSkipped_SetsBothKeys → TestMarkRemoteSkipped_SetsSkipFetchAll.
• testenv.go: guard regex drops the skipDefaultUpdate alternative and tightens the count from {2,4} to {2,3}.

fmt clean, lint 0 issues, unit + integration guard tests pass.

Draft commit message:

Want me to commit? (Make sure your GPG agent is unlocked.)

Committed as 740b7994b. The skip-fetch-all branch now has four commits:

• 870ff89f4 — stamp skipFetchAll only on remotes we create
• 4ccc0dcf6 — stamp phantom remotes left by failed filtered fetches
• 09a7aaa28 — stamp on detached ctx and match remote URL exactly
• 740b7994b — drop deprecated skipDefaultUpdate stamp

Since the behavior has evolved a fair bit across these commits, want me to regenerate the PR description to reflect the final state (single skipFetchAll stamp, creation-time only, robust to failed/timed-out fetches, exact URL matching)?