Skip to content

test: git-remote coverage (2/4) β€” real-hook P0 tests, cross-machine fetch, explain fix - #1637

Open
Soph wants to merge 12 commits into
mainfrom
soph/git-remote-tests-2-p0
Open

test: git-remote coverage (2/4) β€” real-hook P0 tests, cross-machine fetch, explain fix#1637
Soph wants to merge 12 commits into
mainfrom
soph/git-remote-tests-2-p0

Conversation

@Soph

@Soph Soph commented Jul 4, 2026

Copy link
Copy Markdown
Collaborator

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

Part 2 of the stack on #1636. The P0 test groups from the plan, plus two fixes they surfaced.

Tests

  • A-group (real pre-push hook, both backends): plain-push checkpoint sync with user-push assertions (A1), no-new-checkpoints no-op via remote digest (A2), branch-delete and tag-only pushes through the hook (A3), unreachable checkpoint target degrades gracefully under the real hook (A4), git-refs queue drain via real push (A6 slice).
  • C-group (cross-machine clone flows): production fetchMetadataBranchIfMissing exercised for real over HTTPS (C1 β€” previous clone tests substituted a raw git fetch), git-refs on-demand RefFetcher fetches exactly the needed ref and nothing else (C2), unreachable-remote reads surface the real fetch error (C3), authenticated HTTPS clone auto-fetch under both backends (C4).
  • G1 (e2e): resume_remote_test.go / explain_test.go clone tests now rely on the installed hook (AssertCheckpointsOnRemote) instead of explicit PushCheckpointRefs β€” a broken hook can no longer pass the e2e suite.

Fixes

  • Graceful-degradation test false-positive: the test staged the session's files after the stop hook, so neither backend condensed anything; git-branch passed only because the v1 branch was inherited from the clone. Reordered to stage-before-stop and the assertion now requires a new checkpoint ID.
  • fix: surface git-refs checkpoint fetch failures in explain fallback: matchCheckpointPrefixWithRemoteFallback discarded the FetchCheckpointRef error, reporting an unreachable remote identically to an absent checkpoint β€” re-masking the error class 7bbdad0 fixed at the store layer. The fallback now propagates the error (repos with no resolvable checkpoint source keep the clean not-found; the commit-ref prefetch stays best-effort; the v1-branch path keeps its historical fail-soft behavior). C3 asserts it hard.

Verification

  • mise run fmt && mise run lint clean per commit
  • mise run test:integration green (430 tests, 0 skips at tip)
  • mise run test:e2e:canary green under both E2E_CHECKPOINT_STORE values

πŸ€– Generated with Claude Code

https://claude.ai/code/session_012yi3hHGAGepwfrfjPETjGq


Note

Medium Risk
Changes user-visible error handling on checkpoint explain and attribution when remotes are unreachable; behavior is more correct but could surprise scripts that only matched on β€œnot found”.

Overview
Adds broad integration and e2e coverage for checkpoint sync and cross-machine reads: real git push through the installed pre-push hook (sync, no-op pushes, delete/tag shapes, unreachable checkpoint_remote, git-refs push-queue drain), HTTPS clone flows (fetchMetadataBranchIfMissing, tokenized auto-fetch, per-ref RefFetcher precision), plus e2e tests that assert checkpoints land on the remote via the hook instead of explicit PushCheckpointRefs.

Fixes misleading β€œcheckpoint not found” when the remote fetch actually failed (regression 7bbdad0): matchCheckpointPrefixWithRemoteFallback now returns a third error value; git-refs FetchCheckpointRef failures and post-fetch re-list errors propagate through checkpoint explain, JSON export, and attribution remote enrichment. Repos with no resolvable remote still get a clean local not-found; the v1 metadata-branch path keeps prior fail-soft behavior.

Test harness / correctness tweaks: TestEnv helpers for config, tags, push-queue peek, and hook pushes with arbitrary args; graceful-degradation clone test stages files before stop and requires a new checkpoint ID (fixes a false positive from inherited v1 metadata).

Reviewed by Cursor Bugbot for commit 52f48e2. Configure here.

Soph and others added 9 commits July 4, 2026 19:05
Gap analysis and test plan for remote/upstream git coverage across both
checkpoint storage backends (git-branch and git-refs), based on the
existing e2e/integration inventory, production remote code paths, and
past remote-related regressions.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012yi3hHGAGepwfrfjPETjGq
The integration suite had no git-refs coverage: every remote-touching test
asserted the git-branch v1-branch topology only, while the e2e suite already
ran both backends. This adds the missing axis.

TestEnv gains a CheckpointStore field injected as ENTIRE_CHECKPOINTS_PRIMARY
into every spawned CLI and git hook (cliEnv + gitHookEnv), and propagated to
clones. A ForEachBackend(t, fn) helper runs "git-branch"/"git-refs" subtests,
plus backend-aware assertion helpers (CheckpointsPresentLocally/OnRemote,
CheckpointExistsOnRemote, RemoteCheckpointState, LatestCheckpointID) mirroring
e2e/testutil/backend.go so one test asserts against either topology.

Wires the pre-push and graceful-degradation tests in remote_operations_test.go,
the HTTPS push/token tests in http_remote_test.go, and the explain
fetch-on-miss test under both backends. Tests whose assertions are inherently
v1-branch-shaped (commit counts/subjects, rebase parent counts, checkpoint_remote
routing, treeless v1 clone, v1-ref-hash divergence) stay git-branch-only with an
explicit rationale pointing at their git-refs follow-up in the test plan.

One genuine git-refs divergence surfaced and is flagged, not papered over:
when a session stages its file changes after the stop hook, git-refs
condensation creates no per-checkpoint ref (git-branch still creates the local
v1 checkpoint). Skipped for git-refs with a KNOWN BUG note; a fresh repo and a
stage-before-stop clone both create the ref correctly.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012yi3hHGAGepwfrfjPETjGq
Entire-Checkpoint: f96568923ebd
The real git-invoked pre-push hook was never asserted end to end. Integration
RunPrePush spawned the hook with Stdin=nil, and GitPush always passed
--no-verify, so a completely broken hook would go unnoticed; e2e remote tests
masked the same gap by always calling PushCheckpointRefs explicitly.

Integration TestEnv gains:
- InstallRealPrePushHook + GitPushWithHooks: a plain `git push` (no --no-verify)
  that runs the installed hook exactly as git runs it β€” realistic stdin refspec
  lines and remote name/URL argv, inheriting the checkpoint-store env.
- RunPrePush now feeds realistic stdin (defaultPrePushStdin) instead of nil,
  with RunPrePushWithStdin as an escape hatch for the empty-stdin no-op case.
  GitPush keeps --no-verify for setup plumbing.

A smoke test (seed of A1) proves GitPushWithHooks lands checkpoints on a bare
remote with NO explicit checkpoint push, under both backends via ForEachBackend.

e2e gains AssertCheckpointsOnRemote(t, s, bareDir), asserting the
backend-appropriate refs are present on the bare remote for later use by the
section-3 e2e tests.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012yi3hHGAGepwfrfjPETjGq
Entire-Checkpoint: 51ee64d92120
Integration/unit tests have historically made live github.com fetches and
triggered macOS keychain prompts when checkpoint-token / checkpoint_remote
resolution was in play (#1463, 53bc37a). This adds a TestMain-level tripwire.

When ENTIRE_TEST_GIT_HERMETIC is set (the integration TestMain sets it, plus
GIT_TERMINAL_PROMPT=0), GitIsolatedEnv's global git config routes HTTPS
transport to github.com/gitlab.com through a dead loopback proxy, so any test
that accidentally dials those hosts fails fast instead of reaching the network
or prompting for credentials.

The config lives in the file GIT_CONFIG_GLOBAL points at because GitIsolatedEnv
strips inherited GIT_CONFIG_* env, so the GIT_CONFIG_COUNT-injected insteadOf
approach would be filtered out for every spawned binary. A dead per-host proxy
is used rather than url.insteadOf: insteadOf rewrites the effective URL git
reports on read, which broke origin-URL forge detection (trail_resume). The
proxy blocks transport only and is scoped per host, so the in-process 127.0.0.1
HTTPS test server and the checkpoint-token GIT_CONFIG_* injection are unaffected.

A self-test proves the tripwire fires: `git ls-remote https://github.com/...`
fails fast (~20ms) without network or prompt.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012yi3hHGAGepwfrfjPETjGq
Entire-Checkpoint: b3d8c9729839
Exercise the git-invoked pre-push hook end to end for both checkpoint
backends, closing the "real hook is never asserted" blind spot from the
git-remote test plan.

- A1: extend the seed test to assert the user branch itself arrives (the
  user push is unaffected by the checkpoint sync).
- A2: a push carrying only a non-checkpointed commit is a fast no-op
  (RemoteCheckpointState digest unchanged).
- A3: `git push --delete` (zero-sha stdin) and a tag-only push run through
  the real hook without crashing and exit 0.
- A4: checkpoints routed to a genuinely unreachable target (a missing repo
  on the in-process HTTPS server) degrade gracefully; the user push still
  lands the feature branch. Local-path origins can't express this (URL
  derivation falls back to origin), hence the HTTPS setup.
- A6 (git-refs slice): a real push drains the per-checkpoint push queue;
  the v1 alternates rebase scenario has no git-refs equivalent.

Adds GitPushArgsWithHooks, GitTag, PushQueueRefs, and SetGitConfig helpers.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012yi3hHGAGepwfrfjPETjGq
Entire-Checkpoint: 0d5ed27f5bb0
Cover the clone β†’ fetch β†’ read paths for both checkpoint backends.

- C1 (git-branch, HTTPS): exercise the production
  fetchMetadataBranchIfMissing path β€” a clone that lacks the local v1
  branch fetches it during pre-push settings resolution, no manual git
  fetch. Runs over HTTPS because local-path origins can't derive a
  checkpoint URL.
- C2 (git-refs): a clone with no checkpoint refs reads a checkpoint; the
  on-demand RefFetcher fetches EXACTLY that ref, leaving an unrelated
  checkpoint's ref absent until it too is read.
- C3 (git-refs): unreachable remote + locally-missing ref. Documents a
  KNOWN BUG (regression class 7bbdad0): explain's git-refs prefix-match
  remote fallback (explain_export.go:216-227) discards the
  FetchCheckpointRef error and reports "checkpoint not found", masking an
  unreachable remote as an absent checkpoint. The test self-heals: it skips
  while the masking is present and asserts once fixed. No production change.
- C4 (both, HTTPS): reading a checkpoint from a fresh authenticated clone
  auto-fetches its data with the token (v1 branch on miss for git-branch,
  the exact per-checkpoint ref for git-refs).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012yi3hHGAGepwfrfjPETjGq
Entire-Checkpoint: 5d532318e7a7
The remote clone tests pushed checkpoints explicitly via PushCheckpointRefs
right after the user push, which would mask a broken pre-push hook. Replace
those calls with AssertCheckpointsOnRemote so a plain `git push` must sync
checkpoints through the installed hook end to end (A5/G1 in spirit).

No prompt text changed, so the vogon canary parser is unaffected. Verified
green under both E2E_CHECKPOINT_STORE values.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012yi3hHGAGepwfrfjPETjGq
Entire-Checkpoint: cfbc41e5de4b
Root-caused the git-refs KNOWN BUG skip: it was a harness artifact, not a
backend divergence. The test staged the session's file changes after the
stop hook, so the "no files modified during session" gate skipped
condensation on BOTH backends. git-branch still passed only because the
assertion (v1 branch exists) was satisfied by the checkpoint inherited
from origin via the plain clone; git-refs correctly reported no local
checkpoint (per-checkpoint refs are not fetched by a default clone).

Stage before stop like a real agent session (and like the
createCheckpointedCommit helper), drop the skip, and assert a NEW
checkpoint ID was condensed so inherited clone data can never satisfy
the check again.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012yi3hHGAGepwfrfjPETjGq
Entire-Checkpoint: 8429f8830bfd
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 7bbdad0 fixed at the store layer.

The fallback now returns the fetch error and callers surface it as
"checkpoint <id> 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
Entire-Checkpoint: ca56eea65fe0
@Soph
Soph requested a review from a team as a code owner July 4, 2026 21:26
Copilot AI review requested due to automatic review settings July 4, 2026 21:26

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 (part 2/4 of the #1636 stack) expands integration + e2e coverage around git-remote checkpoint behaviorβ€”especially β€œreal git push through installed pre-push hook” flows and cross-machine clone/read fetch pathsβ€”and fixes a production bug where checkpoint explain could incorrectly mask an unreachable-remote fetch failure as β€œcheckpoint not found”.

Changes:

  • Adds integration tests for real-hook pushes (sync, no-op, delete/tag shapes, unreachable checkpoint_remote degradation, git-refs push-queue drain) and HTTPS clone/read auto-fetch flows.
  • Adds git-refs clone read coverage to ensure the on-demand RefFetcher fetches exactly the needed checkpoint ref and surfaces real remote fetch errors.
  • Updates explain/prefix-match remote fallback to propagate git-refs fetch errors (instead of reporting β€œnot found”), and updates e2e clone tests to assert hook-driven remote checkpoint sync.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
e2e/tests/resume_remote_test.go Switches clone flow to assert hook-driven remote checkpoint sync instead of explicitly pushing checkpoint refs.
e2e/tests/explain_test.go Same as above for explain-from-clone e2e coverage.
cmd/entire/cli/integration_test/testenv.go Adds integration test helpers (git config/tag, push queue peek, push-with-hooks args).
cmd/entire/cli/integration_test/remote_operations_test.go Fixes graceful-degradation test ordering and strengthens assertion to require a new checkpoint.
cmd/entire/cli/integration_test/refs_fetch_test.go New C2/C3 git-refs clone/read tests for exact-ref fetch and unreachable-remote error surfacing.
cmd/entire/cli/integration_test/real_hook_push_test.go Adds A2/A3/A4/A6 integration coverage around real pre-push hook behavior.
cmd/entire/cli/integration_test/http_remote_test.go Adds HTTPS cross-machine clone β†’ fetch/read tests (C1/C4).
cmd/entire/cli/git_operations.go Updates FetchCheckpointRef contract comment to require surfacing fetch failures.
cmd/entire/cli/explain.go Propagates remote fetch failures from the prefix-match fallback instead of returning not-found.
cmd/entire/cli/explain_export.go Threads fallback errors through export + commit-ref prefetch behavior; updates fallback helper signature.
cmd/entire/cli/attribution.go Ensures attribution refresh path sees remote fallback errors (instead of treating as not-found).

Comment on lines +2034 to +2035
queuePath := filepath.Join(env.RepoDir, ".git", "entire-checkpoint-push-queue.jsonl")
data, err := os.ReadFile(queuePath) //nolint:gosec // G304: path built from test env, not user input

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Fixed in a9b0334 β€” PushQueueRefs now resolves the common dir via git rev-parse --path-format=absolute --git-common-dir, matching production (checkpoint/pushqueue.go), so it's correct from linked worktrees.

Comment on lines +2062 to +2066
// GitPushArgsWithHooks installs the pre-push hook and runs `git push <args>`
// (WITHOUT --no-verify) so the real hook fires exactly as git runs it, then
// returns the combined output and any error instead of failing the test. Use it
// for push shapes GitPushWithHooks can't express β€” a `--delete` (zero-sha stdin),
// a tag-only push, or an expected graceful-degradation exit code.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Reworded in a9b0334. One nuance: the wrapper forwarding only the remote name is not a fidelity gap β€” it matches the production hook template (strategy/hooks.go runs entire hooks git pre-push "$1"), and stdin is inherited via exec. The docs now state that explicitly instead of the blanket "exactly as git runs it".

Soph and others added 3 commits July 5, 2026 10:10
- AssertCheckpointsOnRemote (git-refs arm): surface git errors instead of
  reporting a misleading "found none" via gitOutputSafe
- RemoteCheckpointState: fail the test on git error instead of returning
  "" (two broken invocations comparing equal would pass vacuously)
- hermeticity self-test comment: the guard is per-host http proxy entries,
  not an insteadOf redirect

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012yi3hHGAGepwfrfjPETjGq
Entire-Checkpoint: d4f06e259d79
- PushQueueRefs: resolve the git common dir via rev-parse like production
  (checkpoint/pushqueue.go) instead of hard-coding RepoDir/.git, so the
  helper is correct from linked worktrees
- clarify InstallRealPrePushHook/GitPushArgsWithHooks docs: the wrapper
  forwards only the remote name, matching the production hook template

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012yi3hHGAGepwfrfjPETjGq
Entire-Checkpoint: 283495b6edfd
@Soph
Soph force-pushed the soph/git-remote-tests-1-infra branch from 7a45cf4 to a4a29a0 Compare July 6, 2026 18:38
Base automatically changed from soph/git-remote-tests-1-infra to main July 6, 2026 18:55
t.Fatal("checkpoint should be on the HTTPS remote after push")
}

clone := cloneFromBareWithHTTPS(t, env, mainBare, httpsURL)

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.

I'm not 100% sure how applicable it is but it came up in an automated review using Fable:

I think the git-branch half of this test passes without actually exercising the auto-fetch. cloneFromBareWithHTTPS does a plain clone of the bare dir, and since env.RunPrePush("origin") already pushed checkpoints by that point, the clone comes with refs/remotes/origin/entire/checkpoints/v1 and all its objects included. The store is happy to serve reads from that remote-tracking ref (getSessionsBranchTree in checkpoint/persistent.go), so the checkpoint explain at the end never talks to the HTTPS server and never uses the token β€” everything it needs is already local.

The CheckpointsPresentLocally() guard doesn't catch this because it only looks at the local branch, not the remote-tracking ref. It's the same trap as the inherited-v1 false positive this PR fixes in the graceful-degradation test. (Cloning over HTTPS instead of the file path wouldn't change anything β€” the remote-tracking ref comes along either way.)

Suggested fix: for the git-branch case, delete the remote-tracking v1 ref in the clone before the read β€” TestExplain_CheckpointFetchesFromRemoteWhenMissingLocally already does exactly this. Then assert clone.BranchExists(paths.MetadataBranchName) after the explain, which proves the fallback fetch really ran.

One more small thing: the docstring says "Both require ENTIRE_CHECKPOINT_TOKEN to reach the server", but the test server only checks Authorization on push β€” reads are anonymous. I'd either adjust the docstring ("token-authenticated push, anonymous HTTPS read"), or add an opt-in requireAuthForReads to startGitHTTPSServer and assert the read fails without a token. The latter fits nicely into the plan's F-group, so a follow-up is fine.```

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.

3 participants