Skip to content

remote: classify a remoteless repo as checkpoint-ref absence, not outage - #1886

Open
peyton-alt wants to merge 4 commits into
mainfrom
fix/local-repo-checkpoint-probe
Open

remote: classify a remoteless repo as checkpoint-ref absence, not outage#1886
peyton-alt wants to merge 4 commits into
mainfrom
fix/local-repo-checkpoint-probe

Conversation

@peyton-alt

@peyton-alt peyton-alt commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

https://entire.io/gh/entireio/cli/trails/963

Main is currently red: TestRunExplainAuto_GeneratePersistsHexOnBranchUnderRefsPrimary fails on the tip (see #1885's test-core run — that PR only touches trail show, and the failure reproduces on pristine main with its diff stashed).

Root cause

A semantic conflict between the last two merges — each green alone:

In a fully local repository — no origin remote, no checkpoint_remote — the probe's origin-name fallback runs git ls-remote origin, gets exit 128 ('origin' does not appear to be a git repository), and the contract turns "this repo has no remotes" into a hard error. Backfill routing then fails instead of falling through to the branch store.

Fix

Before probing: if the fetch target is the non-authoritative origin fallback, no checkpoint_remote is configured, and origin has no URL, return the ref's absence (plumbing.ErrReferenceNotFound) — a repo with no remotes has no remote that could host checkpoint refs. Deliberately narrow:

  • checkpoint_remote configured but unresolvable → unchanged (fallback emptiness still refuses to classify as absence)
  • genuine transport failures → unchanged (still never absence)

New regression test TestFetchCheckpointRef_NoRemoteAtAllIsAbsence alongside the existing probe-contract tests, all green; the failing explain test passes with this fix.

🤖 Generated with Claude Code


Note

Low Risk
Small, scoped change to checkpoint ref fetch error classification with a regression test; does not relax transport-vs-absence rules for configured or reachable remotes.

Overview
Fixes backfill / explain --generate routing in fully local repos where on-demand checkpoint ref probing would hit the non-authoritative origin fallback and surface git ls-remote exit 128 as a transport error instead of “ref not found.”

FetchCheckpointRef adds a narrow pre-probe guard: when the target is the origin fallback, checkpoint_remote is not configured, and origin has no URL, it returns plumbing.ErrReferenceNotFound (wrapped) without calling ls-remote. Behavior is unchanged when checkpoint_remote is set (including unresolvable) or when a real remote exists but is unreachable.

Adds regression test TestFetchCheckpointRef_NoRemoteAtAllIsAbsence for a repo with no remotes.

Reviewed by Cursor Bugbot for commit 66c06c5. Configure here.

Semantic conflict between #1824 and #1811: each was green alone, but
combined they break fully local repositories. #1811 routes backfill
writes through the git-refs store, whose local miss triggers #1824's
on-demand fetch; in a repo with no origin remote and no
checkpoint_remote, the origin-name fallback probe runs `git ls-remote
origin`, gets exit 128, and the "transport failure is never absence"
contract turns a repo that simply has no remotes into a hard error —
breaking `explain --generate` (and main CI via
TestRunExplainAuto_GeneratePersistsHexOnBranchUnderRefsPrimary).

A repository with no remote at all has no remote that could host
checkpoint refs, so the ref's local absence is the final verdict. The
guard is deliberately narrow: when a checkpoint_remote IS configured
but unresolvable, the existing fallback-emptiness refusal still
applies, and genuine transport failures still never classify as
absence.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 31, 2026 19:09
@peyton-alt
peyton-alt requested a review from a team as a code owner July 31, 2026 19:09

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 66c06c5. Configure here.

Comment thread cmd/entire/cli/checkpoint/remote/checkpoint_ref.go

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a regression in checkpoint ref fetching for fully local Git repositories (no origin remote and no checkpoint_remote configured) by classifying the situation as “ref absent” instead of a transport outage, unblocking backfill flows like entire checkpoint explain --generate under git-refs primary.

Changes:

  • Add a narrow pre-probe guard in FetchCheckpointRef to treat “no remotes at all” as plumbing.ErrReferenceNotFound rather than running git ls-remote origin and surfacing exit 128.
  • Add a regression test covering the fully-local (no remotes) repo case.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
cmd/entire/cli/checkpoint/remote/checkpoint_ref.go Adds a guard to classify remoteless repos as checkpoint-ref absence before probing/fetching.
cmd/entire/cli/checkpoint/remote/checkpoint_ref_test.go Adds a regression test ensuring remoteless repos are treated as absence, not transport failure.

Comment thread cmd/entire/cli/checkpoint/remote/checkpoint_ref.go Outdated
peyton-alt and others added 3 commits July 31, 2026 15:36
Review finding (Cursor + Copilot): Configured() returns false when
settings fail to load, so an origin-less repo with a corrupt
.entire/settings.json would have classified as absence — the exact
undeterminable case the fallback-emptiness refusal exists for. The
shortcut now requires a successful settings load showing no
checkpoint_remote; a load error keeps the hard-failure behavior.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Trail review finding: GetCheckpointRemote returns nil both when
checkpoint_remote is absent and when it is present but malformed (wrong
type, missing provider/repo, repo without a slash). A user who
configured — and botched — a checkpoint remote was therefore classified
as "no remote exists", the false-absence this file's contract forbids.
The shortcut now requires the strategy options to carry no
checkpoint_remote entry at all; any present entry, valid or not, keeps
the hard-failure behavior.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Multi-agent review of this PR found two false-absence holes sharing one
root: the guard inferred "no origin remote" from an undifferentiated
GetRemoteURL failure, which a dead caller context (Ctrl-C mid-loop,
expired hook budget) or a repo whose only remote is non-origin (git
clone -o upstream) also produces. Both then classified as absence in
repositories whose checkpoint refs may genuinely live on a remote —
the exact misrouting this function's contract forbids. Checkpoint refs
are pushed to whatever remote the pre-push hook fires for, so "no
origin" never proved "remoteless".

The guard now requires positive evidence: a live context, a readable
config with no checkpoint_remote key, and a successful, EMPTY
`git remote` listing. Any enumeration failure falls through to the
probe. Also per review: the key-presence predicate moves into the
settings package as HasCheckpointRemoteKey (CLAUDE.md forbids settings
helpers outside it), the swallowed settings-load error now logs a
warning (its old failure mode was an undebuggable exit-128 message),
guard firings log at debug, the guard comment's two inaccurate claims
are corrected, the FetchCheckpointRef contract documents the new
absence source, and routing_store.go's stale "backfill absence probes
are local-only" note — the reconciliation #1824 asked of whichever PR
landed second — is rewritten to match the fetching reality.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants