Skip to content

fix: support running jj-spr from secondary workspaces#115

Open
edmondop wants to merge 11 commits intoLucioFranco:mainfrom
edmondop:fix/workspace-support
Open

fix: support running jj-spr from secondary workspaces#115
edmondop wants to merge 11 commits intoLucioFranco:mainfrom
edmondop:fix/workspace-support

Conversation

@edmondop
Copy link
Copy Markdown
Contributor

Summary

  • jj-spr only works from primary workspaces. Secondary workspaces (created via jj workspace add) have no .git directory, so git2::Repository::discover fails.
  • Added discover_git_repo() that falls back to jj's internal pointers (.jj/repo → primary's .jj/repo/store/git_target) to locate the git repository.
  • Pass the current workspace path to Jujutsu so jj commands run from the correct workspace context.

Test plan

  • New unit test: creates an isolated sibling workspace and verifies discover_git_repo + Git::new work
  • New integration test: runs the jj-spr binary from a secondary workspace in a separate directory tree
  • All existing tests pass

🤖 Generated with Claude Code

edmondop and others added 11 commits March 4, 2026 09:16
* fix: include error source chain in error messages

octocrab's Error::GitHub variant uses snafu without a custom display
attribute, so its Display impl outputs just "GitHub" — the variant name.
The actual API error message lives in the source field but was never
surfaced because From<E> only used format!("{}", error).

Walk std::error::Error::source() chain so wrapped errors include the
underlying message. Also add context to the merge call in land.rs with
PR number and head SHA for better diagnostics.

Before: 🛑 GitHub
After:  🛑 GitHub: <actual reason from GitHub API>

* trigger CI

* style: cargo fmt
* Adding dry run mode

* Improve dry run output formatting

Show richer details (base/head branches, draft status, reviewers)
and suppress commit title output during dry run mode.

* trigger CI

* style: cargo fmt
* Add jj spr cleanup command

List and delete orphan SPR branches on the remote that are no longer
associated with any open pull request. Dry-run by default; pass
--confirm to actually delete.

Extract branch filtering logic into testable functions with unit tests.

* trigger CI

* style: cargo fmt
* Retarget PR to master when commit is now directly on master

After the bottom of a stack is landed and the remaining stack is rebased
onto master, jj spr diff now detects that the next PR's commit is directly
on master and retargets the PR from its synthetic base to master. The old
synthetic base branch is deleted as part of cleanup.

Previously, diff would always keep an existing synthetic base even when
the commit was rebased directly onto master, leaving the PR targeting
a stale synthetic branch.

Extract determine_base_branch() for testability.

* trigger CI
Deleting the synthetic base branch before retargeting the PR to master
causes GitHub to close the PR, since its base branch no longer exists.
Remove the deletion entirely — orphan branches can be cleaned up via
`jj spr cleanup` instead.

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
* fix: stop deleting old synthetic base branch on retarget

Deleting the synthetic base branch before retargeting the PR to master
causes GitHub to close the PR, since its base branch no longer exists.
Remove the deletion entirely — orphan branches can be cleaned up via
`jj spr cleanup` instead.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* feat: confirm before creating new pull requests

When jj-spr is about to create a new PR (i.e., no `Pull Request:` URL
found in the change description), it now prompts for confirmation first.

This guards against accidental duplicate PR creation, which is the most
common failure mode: an AI assistant or user rewrites the jj change
description via `jj describe -m "..."` and drops the `Pull Request:`
trailer. jj-spr then thinks the change has no PR and creates a new one
with a suffixed branch name (-1, -2, -3...).

The confirmation prompt shows the PR title and defaults to yes, so the
normal workflow is unaffected (just press Enter). But it gives the user
a chance to notice "wait, this change already has a PR" before a
duplicate is created.

Skipped automatically in --dry-run mode (that code path is separate).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
jj-spr only worked from primary workspaces because
git2::Repository::discover fails in secondary workspaces (no .git
present). Fix by falling back to jj's internal pointers (.jj/repo →
primary's .jj/repo/store/git_target) to locate the git repository.

Also pass the current workspace path to Jujutsu so jj commands run
from the correct workspace context, not the primary.

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
new_with_workspace checked for .jj only in the exact CWD, so running
from a subdirectory of a workspace failed. Walk up to find .jj and
use its parent as repo_path.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
All git push/fetch commands inherited the process CWD, which fails in
secondary workspaces where there's no .git directory. Set current_dir
to the git repo workdir on every git command invocation.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
git rev-parse for resolving PR branch OIDs ran without current_dir,
failing in secondary workspaces and producing null OIDs that later
caused "odb: cannot read object: null OID" errors. Added git_workdir
to Config and set current_dir on all git subprocess invocations.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add .current_dir(&config.git_workdir) to the remaining git fetch call
in get_pull_request that was missing it, preventing "not a git
repository" errors from secondary workspaces.

Add integration tests that verify git rev-parse and git fetch work
from isolated secondary workspaces when using the discovered
git_workdir, and that they fail without it (proving the fix is
necessary).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@edmondop edmondop force-pushed the fix/workspace-support branch from ba45651 to f59def2 Compare March 20, 2026 17:43
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.

1 participant