test: git-remote coverage (1/4) — plan + backend matrix, real-hook push, hermeticity#1636
Open
Soph wants to merge 5 commits into
Open
test: git-remote coverage (1/4) — plan + backend matrix, real-hook push, hermeticity#1636Soph wants to merge 5 commits into
Soph wants to merge 5 commits into
Conversation
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
Contributor
There was a problem hiding this comment.
Pull request overview
Adds the first slice of systematic git-remote test coverage across both checkpoint backends by introducing an integration backend matrix, exercising the real pre-push hook via plain git push, and enforcing hermetic git transport in integration tests to prevent accidental external network access.
Changes:
- Introduces a dual-backend integration test matrix (
git-branchvsgit-refs) with backend-aware assertion helpers. - Adds “real hook” push helpers/tests so a plain
git pushexercises the installed pre-push hook end-to-end. - Adds a hermeticity tripwire for integration tests by routing HTTPS transport for external hosts through a dead loopback proxy, plus a guard test.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
e2e/testutil/repo.go |
Adds an E2E assertion helper to verify checkpoints landed on a bare remote without an explicit checkpoint push. |
docs/testing/git-remote-test-plan.md |
New test plan documenting coverage gaps, priorities, and sequencing for git-remote behaviors across both backends. |
cmd/entire/cli/testutil/testutil.go |
Adds opt-in hermetic git config injection (dead loopback proxy) to prevent accidental external HTTPS dials in tests. |
cmd/entire/cli/integration_test/testenv.go |
Adds backend selection env injection, real-hook push helpers, and realistic stdin for pre-push direct invocation. |
cmd/entire/cli/integration_test/setup_test.go |
Sets GIT_TERMINAL_PROMPT=0 and enables the hermetic git tripwire for the whole integration suite. |
cmd/entire/cli/integration_test/remote_operations_test.go |
Runs remote operation tests under both backends and switches assertions to backend-aware helpers. |
cmd/entire/cli/integration_test/real_hook_push_test.go |
New integration test proving plain git push via the real installed pre-push hook syncs checkpoints to the remote. |
cmd/entire/cli/integration_test/http_remote_test.go |
Runs selected HTTPS remote tests under both backends and adjusts assertions accordingly. |
cmd/entire/cli/integration_test/hermeticity_test.go |
New guard test ensuring external-host git HTTPS attempts fail fast under the hermeticity tripwire. |
cmd/entire/cli/integration_test/explain_test.go |
Runs fetch-on-miss explain coverage under both backends (v1 fetch vs per-checkpoint RefFetcher). |
cmd/entire/cli/integration_test/backend.go |
Adds backend constants, ForEachBackend, and backend-aware local/remote checkpoint assertion helpers. |
Comment on lines
+210
to
+216
| if UsingGitRefs() { | ||
| out := gitOutputSafe(bareDir, "for-each-ref", "--format=%(refname)", checkpointRefPrefix) | ||
| if strings.TrimSpace(out) == "" { | ||
| t.Errorf("expected at least one %s* ref on remote %s, found none", checkpointRefPrefix, bareDir) | ||
| } | ||
| return | ||
| } |
Collaborator
Author
There was a problem hiding this comment.
Fixed in c6636c4 — the git-refs arm now uses GitOutputErr and reports the git error distinctly from "found none".
Comment on lines
+112
to
+118
| cmd := exec.CommandContext(env.T.Context(), "git", "for-each-ref", "--format=%(refname) %(objectname)", prefix) | ||
| cmd.Dir = bareDir | ||
| cmd.Env = testutil.GitIsolatedEnv() | ||
| out, err := cmd.Output() | ||
| if err != nil { | ||
| return "" | ||
| } |
Collaborator
Author
There was a problem hiding this comment.
Fixed in c6636c4 — RemoteCheckpointState now fails the test on git error instead of returning "" (which could have made the idempotence comparison pass vacuously).
Comment on lines
+26
to
+28
| // ls-remote against a public-looking github URL must be refused immediately | ||
| // by the insteadOf redirect to 127.0.0.1:1, not hang on DNS/network or block | ||
| // on a credential prompt. |
Collaborator
Author
There was a problem hiding this comment.
Fixed in c6636c4 — comment now describes the actual mechanism (per-host http..proxy entries from testutil.hermeticGitConfig).
- 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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
https://entire.io/gh/entireio/cli/trails/757
Part 1 of a 4-PR stack adding systematic test coverage for git-remote operations across both checkpoint backends. The plan (committed here as
docs/testing/git-remote-test-plan.md) came out of a survey of existing e2e/integration coverage, the production remote code paths, and past remote-related regressions.Stack: this PR → #2/4 (P0 hook & cross-machine) → #3/4 (divergence matrix) → #4/4 (OPF, degraded remotes, shallow clones, e2e).
What's here
TestEnv.CheckpointStoreinjectsENTIRE_CHECKPOINTS_PRIMARYinto every spawned CLI and git hook process (condensation happens in post-commit),ForEachBackend(t, fn)subtest helper, and backend-aware assertions (CheckpointsPresentLocally/OnRemote,RemoteCheckpointState,LatestCheckpointID) mirroringe2e/testutil/backend.go. The remote-touching suites (remote_operations_test.go,http_remote_test.go, explain fetch-on-miss) now run under both backends.GitPushWithHooks(no--no-verify) +InstallRealPrePushHookrun a plaingit pushthrough the installed pre-push hook exactly as git does (stdin refspecs);RunPrePushnow feeds computed stdin instead of nil. Smoke test proves a plaingit pushalone lands checkpoints on a bare remote under both backends — the first end-to-end assertion of the real hook. Plus e2eAssertCheckpointsOnRemote.insteadOfrewrites broke trail forge detection, so this blocks at transport level only), with a self-test proving external hosts fail in ~20ms. Regression class: tests doing live github.com fetches / triggering macOS keychain prompts (strategy: stop resolvePushSettings tests from fetching github.com #1463).Verification
mise run fmt && mise run lintclean per commitmise run test:integrationgreenmise run test:e2e:canarygreen under bothE2E_CHECKPOINT_STOREvalues🤖 Generated with Claude Code
https://claude.ai/code/session_012yi3hHGAGepwfrfjPETjGq
Note
Low Risk
Test-only and documentation changes; production behavior is unchanged aside from optional hermetic git config when ENTIRE_TEST_GIT_HERMETIC is set in tests.
Overview
Adds
docs/testing/git-remote-test-plan.mdas the roadmap for remote/checkpoint coverage across git-branch and git-refs, and lands the first infrastructure slice from that plan.Dual-backend integration matrix:
ForEachBackendplusTestEnv.CheckpointStoreinjectENTIRE_CHECKPOINTS_PRIMARYinto CLI and hook subprocesses; backend-aware helpers (CheckpointsPresentLocally/OnRemote,CheckpointExistsOnRemote,RemoteCheckpointState,LatestCheckpointID) replace hardcodedentire/checkpoints/v1checks. Remote-focused suites (remote_operations_test,http_remote_test, explain fetch-on-miss) now run under both backends, with comments/skips where behavior still differs (v1-only rebase/treeless tests, a documented git-refs skip for staging-after-stop).Real pre-push path:
GitPushWithHooks/InstallRealPrePushHookexercise a normalgit pushthrough the installed hook;RunPrePushfeeds realistic stdin refspecs. NewTestGitPushWithHooks_SyncsCheckpointsToRemoteasserts checkpoints reach the bare remote without explicitRunPrePush. E2E gainsAssertCheckpointsOnRemote.Hermeticity: integration
TestMainsetsENTIRE_TEST_GIT_HERMETICandGIT_TERMINAL_PROMPT=0;testutilappends per-host HTTPS proxy config to the isolation git config so accidental github.com/gitlab.com dials fail fast, withTestHermeticityGuard_ExternalHostFailsFastas a regression guard.Reviewed by Cursor Bugbot for commit aec192c. Configure here.