Skip to content

Commit ef2d8fd

Browse files
jucorclaude
andcommitted
fix: helpful error when jj commits are immutable
When GetLocalCommitStack encounters a commit without a commit-id trailer and jj describe fails (e.g. because the commit is immutable due to untracked remote bookmarks), spr now panics with an actionable message: which commit is affected, why it is likely immutable, and how to fix it (jj bookmark track). Also adds error response support to the jj mock and injects the commit-id generator for test determinism. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 13fb821 commit ef2d8fd

3 files changed

Lines changed: 81 additions & 12 deletions

File tree

vcs/jj_ops.go

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,14 +14,20 @@ import (
1414
// JjOps implements VCSOperations using jj (Jujutsu) commands.
1515
// Git commands are still used for push operations (via gitcmd).
1616
type JjOps struct {
17-
cfg *config.Config
18-
jjcmd JjInterface
19-
gitcmd git.GitInterface
17+
cfg *config.Config
18+
jjcmd JjInterface
19+
gitcmd git.GitInterface
20+
genCommitID func() string
2021
}
2122

2223
// NewJjOps creates a jj-based VCSOperations implementation.
2324
func NewJjOps(cfg *config.Config, jjcmd JjInterface, gitcmd git.GitInterface) *JjOps {
24-
return &JjOps{cfg: cfg, jjcmd: jjcmd, gitcmd: gitcmd}
25+
return &JjOps{
26+
cfg: cfg,
27+
jjcmd: jjcmd,
28+
gitcmd: gitcmd,
29+
genCommitID: func() string { return uuid.New().String()[:8] },
30+
}
2531
}
2632

2733
// FetchAndRebase fetches from remote and rebases using jj commands.
@@ -61,12 +67,21 @@ func (j *JjOps) GetLocalCommitStack(cfg *config.Config, gitcmd git.GitInterface)
6167
// Add commit-id trailers to commits that lack them
6268
for i, p := range parsed {
6369
if p.sprCommitID == "" && !p.empty {
64-
newID := uuid.New().String()[:8]
70+
newID := j.genCommitID()
6571
newDesc := strings.TrimRight(p.description, "\n")
6672
newDesc += "\n\ncommit-id:" + newID
6773
err := j.jjcmd.JjArgs([]string{"describe", "-r", p.changeID, "-m", newDesc}, nil)
6874
if err != nil {
69-
panic(fmt.Sprintf("failed to add commit-id to %s: %v", p.changeID, err))
75+
panic(fmt.Sprintf(
76+
"error: cannot add commit-id trailer to %s (%s) — commit is likely immutable\n\n"+
77+
"This happens when an untracked remote bookmark points at this commit\n"+
78+
"(or one of its descendants), making the entire chain immutable.\n\n"+
79+
"To fix, track the remote bookmarks that cover this commit:\n"+
80+
" jj bookmark track <bookmark> --remote <remote>\n\n"+
81+
"To find which bookmarks are involved:\n"+
82+
" jj log -r '%s::' -T 'change_id.short() ++ \" \" ++ remote_bookmarks ++ \"\\n\"'\n\n"+
83+
"Underlying error: %v",
84+
p.changeID, p.subject, p.changeID, err))
7085
}
7186
parsed[i].sprCommitID = newID
7287
}

vcs/jj_ops_test.go

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package vcs
22

33
import (
44
"context"
5+
"fmt"
56
"os"
67
"path/filepath"
78
"testing"
@@ -102,6 +103,36 @@ func TestJjOpsGetLocalCommitStack_WIPCommit(t *testing.T) {
102103
jjmock.ExpectationsMet()
103104
}
104105

106+
func TestJjOpsGetLocalCommitStack_ImmutableCommitError(t *testing.T) {
107+
cfg := makeJjTestConfig()
108+
jjmock := mockjj.NewMockJj(t)
109+
ops := NewJjOps(cfg, jjmock, nil)
110+
ops.genCommitID = func() string { return "deadbeef" }
111+
112+
// Commit without commit-id trailer — triggers jj describe attempt
113+
c1 := &git.Commit{
114+
CommitHash: "c100000000000000000000000000000000000000",
115+
ChangeID: "jjchangeimmutable",
116+
Subject: "feat: some old commit",
117+
// No CommitID
118+
}
119+
120+
jjmock.ExpectLogAndRespond([]*git.Commit{c1})
121+
jjmock.ExpectDescribeAndFail("jjchangeimmutable",
122+
"feat: some old commit\n\ncommit-id:deadbeef",
123+
fmt.Errorf("Error: Commit abc123 is immutable"))
124+
125+
defer func() {
126+
r := recover()
127+
require.NotNil(t, r, "expected panic for immutable commit")
128+
msg := fmt.Sprintf("%v", r)
129+
assert.Contains(t, msg, "jjchangeimmutable")
130+
assert.Contains(t, msg, "immutable")
131+
assert.Contains(t, msg, "jj bookmark track")
132+
}()
133+
ops.GetLocalCommitStack(cfg, nil)
134+
}
135+
105136
// --- AmendInto ---
106137

107138
func TestJjOpsAmendInto(t *testing.T) {

vcs/mockjj/mockjj.go

Lines changed: 29 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -34,16 +34,20 @@ func (m *Mock) Jj(args string, output *string) error {
3434
actual := "jj " + args
3535
m.assert.Equal(expected, actual)
3636

37-
if m.response[0].Valid() {
37+
resp := m.response[0]
38+
m.expectedCmd = m.expectedCmd[1:]
39+
m.response = m.response[1:]
40+
41+
if resp.Err() != nil {
42+
return resp.Err()
43+
}
44+
if resp.Valid() {
3845
m.assert.NotNil(output)
39-
*output = m.response[0].Output()
46+
*output = resp.Output()
4047
} else if output != nil {
4148
*output = ""
4249
}
4350

44-
m.expectedCmd = m.expectedCmd[1:]
45-
m.response = m.response[1:]
46-
4751
return nil
4852
}
4953

@@ -137,18 +141,37 @@ func (m *Mock) respond(response string) {
137141
}
138142
}
139143

144+
func (m *Mock) respondWithError(err error) {
145+
m.response[len(m.response)-1] = &errResponse{err: err}
146+
}
147+
148+
// ExpectDescribeAndFail expects a jj describe command and returns an error.
149+
func (m *Mock) ExpectDescribeAndFail(changeID, message string, err error) {
150+
m.expect(fmt.Sprintf("jj describe -r %s -m %s", changeID, message)).respondWithError(err)
151+
}
152+
140153
type responder interface {
141154
Valid() bool
142155
Output() string
156+
Err() error
143157
}
144158

145159
type stringResponse struct {
146160
valid bool
147161
output string
148162
}
149163

150-
func (r *stringResponse) Valid() bool { return r.valid }
164+
func (r *stringResponse) Valid() bool { return r.valid }
151165
func (r *stringResponse) Output() string { return r.output }
166+
func (r *stringResponse) Err() error { return nil }
167+
168+
type errResponse struct {
169+
err error
170+
}
171+
172+
func (r *errResponse) Valid() bool { return false }
173+
func (r *errResponse) Output() string { return "" }
174+
func (r *errResponse) Err() error { return r.err }
152175

153176
// formatJjLogResponse formats commits as jj log output with field/record separators.
154177
func formatJjLogResponse(commits []*git.Commit) string {

0 commit comments

Comments
 (0)