Fix checkpoint_remote silently falling back to origin on owner mismatch#805
Fix checkpoint_remote silently falling back to origin on owner mismatch#805blackgirlbytes wants to merge 1 commit intomainfrom
Conversation
When checkpoint_remote was configured but the origin remote had a different owner, fork detection silently skipped the checkpoint remote and pushed to origin instead. Users had no indication their config was being ignored. Now respects the user's explicit checkpoint_remote configuration regardless of owner mismatch, and prints visible stderr warnings when fallback occurs due to URL derivation or parse failures. Fixes #800 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 328e759b573a
There was a problem hiding this comment.
Pull request overview
This PR fixes a bug in Entire CLI’s checkpoint remote resolution so that an explicitly configured checkpoint_remote is no longer silently ignored when the push remote owner differs, and it adds user-visible warnings when the tool must fall back to pushing checkpoints to the normal push remote.
Changes:
- Removed the owner-mismatch “fork detection” skip so
checkpoint_remoteremains honored even when owners differ. - Added stderr warnings for key fallback paths (remote URL lookup/parse/derivation failures).
- Updated the fork-detection unit test to match the new expected behavior.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| cmd/entire/cli/strategy/checkpoint_remote.go | Stops skipping configured checkpoint remotes on owner mismatch; adds stderr warnings when falling back. |
| cmd/entire/cli/strategy/checkpoint_remote_test.go | Updates test expectations so owner mismatch no longer forces fallback to the push remote. |
| // Previously this skipped checkpoint_remote entirely (fork detection), | ||
| // but the user explicitly configured a checkpoint remote — respect that choice. |
There was a problem hiding this comment.
The behavior here no longer skips checkpoint_remote on owner mismatch, but the function-level comment above resolvePushSettings still states it "Skips if the push remote owner differs". Please update the doc comment to match the new behavior so future readers don't reintroduce the fork-detection skip.
| // Previously this skipped checkpoint_remote entirely (fork detection), | |
| // but the user explicitly configured a checkpoint remote — respect that choice. | |
| // We no longer skip checkpoint_remote in this case (previously used for fork detection); | |
| // the user explicitly configured a checkpoint remote, so we always proceed with it. |
| // Should proceed with checkpoint remote even when owners differ — | ||
| // the user explicitly configured it, respect that choice. | ||
| assert.True(t, ps.hasCheckpointURL()) | ||
| assert.Equal(t, "git@github.com:org/checkpoints.git", ps.pushTarget()) |
There was a problem hiding this comment.
The test name TestResolvePushSettings_ForkDetection is now misleading because the expected behavior is to proceed even when owners differ (i.e., fork-detection no longer disables checkpoint_remote). Consider renaming the test to reflect the new requirement (owner mismatch should not cause fallback).
| fmt.Fprintf(os.Stderr, "[entire] Warning: checkpoint_remote is configured (%s) but could not get push remote URL: %v\n", config.Repo, err) | ||
| fmt.Fprintln(os.Stderr, "[entire] Checkpoints will be pushed to origin instead. Check your git remote configuration.") |
There was a problem hiding this comment.
The fallback message hard-codes "origin", but resolvePushSettings is called with the remote name passed by the git pre-push hook (not always "origin"). This warning can mislead users when pushing to a different remote; consider referencing pushRemoteName (or wording like "your push remote") instead.
| fmt.Fprintf(os.Stderr, "[entire] Warning: checkpoint_remote is configured (%s) but could not parse push remote URL: %v\n", config.Repo, err) | ||
| fmt.Fprintln(os.Stderr, "[entire] Checkpoints will be pushed to origin instead. Check your git remote configuration.") |
There was a problem hiding this comment.
The fallback message hard-codes "origin", but resolvePushSettings may be invoked for other remotes (e.g., when pushing to an alternate remote). Recommend referencing pushRemoteName (or "the push remote") so the stderr guidance is accurate.
| fmt.Fprintf(os.Stderr, "[entire] Warning: checkpoint_remote is configured (%s) but could not derive URL from push remote: %v\n", config.Repo, err) | ||
| fmt.Fprintln(os.Stderr, "[entire] Checkpoints will be pushed to origin instead. Check your checkpoint_remote settings.") |
There was a problem hiding this comment.
This fallback warning says checkpoints will be pushed to "origin", but the code actually falls back to pushing to ps.remote (the remote passed into resolvePushSettings). Update the message to avoid implying a specific remote name.
|
The issue is that the CLI repo has a remote source setup but if you fork it, then you get that setup but have for sure no access to the remote and then the push would also fail. So while this fixes one thing, it makes another thing worse. I wonder if a better fix is:
This could be done by checking whether |
Summary
resolvePushSettings()— when a user explicitly configurescheckpoint_remote, that choice is now respected regardless of owner mismatch between push remote and checkpoint remoteContext
When
checkpoint_remotewas configured (e.g.,{"provider": "github", "repo": "alice/checkpoints"}) but the origin remote had a different owner (e.g.,org/work-repo), fork detection silently skipped the checkpoint remote and pushed to origin instead. All failure messages went to debug/warn logs in.entire/logs/— the user had no idea their config was being ignored.Reproduced with: origin =
hydeparksda/invite-automation, checkpoint_remote =blackgirlbytes/genuary2026.Fixes #800
Test plan
mise run test)mise run test:integration)mise run lint)TestResolvePushSettings_ForkDetectionto expect checkpoint URL is set even with owner mismatch🤖 Generated with Claude Code