Skip to content

strategy: stop resolvePushSettings tests from fetching github.com - #1463

Merged
Soph merged 1 commit into
mainfrom
fix/resolve-push-settings-test-no-network
Jun 18, 2026
Merged

strategy: stop resolvePushSettings tests from fetching github.com#1463
Soph merged 1 commit into
mainfrom
fix/resolve-push-settings-test-no-network

Conversation

@Soph

@Soph Soph commented Jun 18, 2026

Copy link
Copy Markdown
Collaborator

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

Why

Three TestResolvePushSettings_* tests in checkpoint_remote_test.go were doing a real network fetch against github.com, which blocks on a macOS keychain prompt and is slow/flaky in CI.

Each test configures a checkpoint_remote of {github, org/checkpoints} and calls resolvePushSettings, but never creates a local entire/checkpoints/v1 branch. resolvePushSettingsfetchMetadataBranchIfMissing then runs a live git fetch against the resolved checkpoint URL (https://github.com/org/checkpoints.git / its SSH form). GitHub returns 401 Basic realm="GitHub", git invokes its osxkeychain credential helper, and the run stalls on a keychain access prompt.

Found while reviewing #1454 — this is a pre-existing bug on main, unrelated to that PR and unrelated to the token-store isolation in #1450 (this is git's own credential helper, a different subsystem).

What changed

Seed the local entire/checkpoints/v1 branch in the three affected tests (via the existing runCheckpointRemoteGit helper) so fetchMetadataBranchIfMissing finds it and short-circuits before fetching. The pushTarget() / hasCheckpointURL() assertions are unaffected — checkpointURL is resolved before the fetch.

The two owner-mismatch tests (ForkDetection, FetchURL_IgnoresOwnerMismatchCheck) already return before the fetch, so they're left untouched.

Verification

  • Re-ran the three tests with credential.helper forced to a logging shim: 0 credential-helper calls (was 2× host=github.com).
  • Each test now runs in ~0.03s (previously multi-second network timeouts).
  • mise run fmt + mise run lint clean (0 issues).

🤖 Generated with Claude Code


Note

Low Risk
Test-only setup changes with no production code paths modified.

Overview
Three TestResolvePushSettings_* cases in checkpoint_remote_test.go now create a local entire/checkpoints/v1 branch (via runCheckpointRemoteGit) before calling resolvePushSettings.

That matches production behavior when metadata is already present and stops fetchMetadataBranchIfMissing from issuing a real git fetch to the derived github.com checkpoint URL—avoiding slow/flaky CI runs and macOS keychain credential prompts on 401. Assertions on pushTarget() and hasCheckpointURL() are unchanged because URL resolution happens before the fetch.

Reviewed by Cursor Bugbot for commit 8767fe1. Configure here.

Three TestResolvePushSettings_* tests configured a github checkpoint_remote
and called resolvePushSettings without a local entire/checkpoints/v1 branch.
resolvePushSettings calls fetchMetadataBranchIfMissing, which then ran a real
`git fetch` against the resolved checkpoint URL (https://github.com/org/
checkpoints.git or its ssh form). GitHub answered 401, git invoked its
osxkeychain credential helper, and the run blocked on a macOS keychain prompt
— besides being a slow, flaky network dependency in a unit test.

Seed the local v1 branch in each affected test so fetchMetadataBranchIfMissing
short-circuits before fetching. The pushTarget()/hasCheckpointURL() assertions
are unaffected (checkpointURL is resolved before the fetch). Each test now runs
in ~0.03s with no network or keychain access.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings June 18, 2026 16:08
@Soph
Soph requested a review from a team as a code owner June 18, 2026 16:08

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 prevents three resolvePushSettings unit tests from performing a real git fetch to github.com by ensuring the local metadata branch (entire/checkpoints/v1) exists during test setup, avoiding slow/flaky network behavior and macOS credential-helper prompts.

Changes:

  • Seed entire/checkpoints/v1 locally in the three affected TestResolvePushSettings_* cases using the existing runCheckpointRemoteGit helper.
  • Add explanatory comments documenting why the metadata branch is created in these tests (to short-circuit fetchMetadataBranchIfMissing).

@Soph
Soph merged commit 9215422 into main Jun 18, 2026
11 checks passed
@Soph
Soph deleted the fix/resolve-push-settings-test-no-network branch June 18, 2026 19:18
Soph added a commit that referenced this pull request Jul 6, 2026
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
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