Skip to content

fix(git): reject bookmark-track for missing bookmark or remote - #660

Merged
Ziinc merged 4 commits into
mainfrom
ccr-b84e8000-3ab84a-bookmark-track
Oct 3, 2026
Merged

Ziinc merged 4 commits into
mainfrom
ccr-b84e8000-3ab84a-bookmark-track

Conversation

@Ziinc

@Ziinc Ziinc commented Sep 30, 2026 •

Copy link
Copy Markdown
Collaborator
  • git bookmark-track with an unknown bookmark or remote returned success and recorded tracking for an absent ref (e.g. @nonremote).
  • jj_bookmark_track now errors with "Remote '' not found" or "Remote bookmark '@' not found" and changes nothing. jj_push skips the tracking step for branches not yet on origin, so first pushes don't gain a spurious warning.
  • New tests/jj_bookmark_track_test.rs covers missing bookmark, missing remote, and the existing-bookmark success path.
    🤖 Generated with Claude Code
    https://claude.ai/code/session_01MxqTt1WtVRKkjNXQpgkJS9
    Generated by Claude Code

jj_bookmark_track now errors when the remote is not configured or the
remote bookmark does not exist, instead of recording tracking for an
absent ref. Push skips the tracking step for branches not yet on origin.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MxqTt1WtVRKkjNXQpgkJS9
The old unit test tracked main@origin in a repo with no origin remote,
which is the @nonremote tracking this PR removes. jj-lib's push marks
pushed bookmarks tracked, so tracking before the first push is unneeded.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MxqTt1WtVRKkjNXQpgkJS9

@Ziinc Ziinc left a comment

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.

Review at 0fd6dc0d: Reviewed remote/bookmark existence validation, first-push handling, and success/failure regression coverage. No actionable correctness or code-quality issues found in this diff.

Validation: static review of the diff, surrounding implementation and tests; PR-head CI reports success. Rust/integration tests were not rerun locally because this environment has no Cargo toolchain.

rename_workspace refuses names that already exist on origin, and
jj_bookmark_track now rejects a missing remote bookmark, so the
best-effort re-track could only fail and its error was discarded.
The first push tracks the renamed bookmark.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01E1poGWXMSFcPsJ8V5gujWG
@Ziinc
Ziinc merged commit a6d571f into main Oct 3, 2026
22 checks passed
@Ziinc
Ziinc deleted the ccr-b84e8000-3ab84a-bookmark-track branch October 3, 2026 07:06

This branch was successfully deployed

1 active deployment
preview — ddfb31d7 Deployed Oct 3, 2026 by Ziinc via build #1653
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants