Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion backend/internal/adapters/workspace/gitworktree/workspace.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ var (
var (
ErrBranchCheckedOutElsewhere = ports.ErrWorkspaceBranchCheckedOutElsewhere
ErrBranchNotFetched = ports.ErrWorkspaceBranchNotFetched
ErrBranchInvalid = ports.ErrWorkspaceBranchInvalid
)

// RepoResolver maps a project to the absolute path of its source git repo.
Expand Down Expand Up @@ -262,7 +263,7 @@ func (w *Workspace) addWorktree(ctx context.Context, repo, path, branch, baseBra

func (w *Workspace) validateBranch(ctx context.Context, repo, branch string) error {
if _, err := w.run(ctx, w.binary, checkRefFormatBranchArgs(repo, branch)...); err != nil {
return fmt.Errorf("gitworktree: invalid branch %q: %w", branch, err)
return fmt.Errorf("%w: %q (%w)", ErrBranchInvalid, branch, err)
}
return nil
}
Expand Down
28 changes: 28 additions & 0 deletions backend/internal/adapters/workspace/gitworktree/workspace_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -369,6 +369,34 @@ func TestAddWorktreeRefusesBranchCheckedOutElsewhere(t *testing.T) {
}
}

// TestCreateRejectsInvalidBranchName covers the residual of #152 Bug 3: a branch
// name rejected by `git check-ref-format` must surface
// ports.ErrWorkspaceBranchInvalid so the HTTP layer renders a typed 400 instead
// of leaking raw git stderr through a 500.
func TestCreateRejectsInvalidBranchName(t *testing.T) {
root := t.TempDir()
repo := t.TempDir()
ws, err := New(Options{ManagedRoot: root, RepoResolver: StaticRepoResolver{"proj": repo}})
if err != nil {
t.Fatalf("new: %v", err)
}
ws.run = func(_ context.Context, _ string, args ...string) ([]byte, error) {
joined := strings.Join(args, " ")
if strings.Contains(joined, "check-ref-format") {
return nil, errors.New("fatal: 'bad branch!!' is not a valid branch name")
}
t.Fatalf("no git beyond check-ref-format should run for an invalid branch: %v", args)
return nil, nil
}
_, err = ws.Create(context.Background(), ports.WorkspaceConfig{ProjectID: "proj", SessionID: "sess", Branch: "bad branch!!"})
if !errors.Is(err, ports.ErrWorkspaceBranchInvalid) {
t.Fatalf("err = %v, want ports.ErrWorkspaceBranchInvalid", err)
}
if !strings.Contains(err.Error(), "bad branch!!") {
t.Fatalf("err = %v, want message to include the rejected branch name", err)
}
}

// TestAddWorktreeReportsBranchNotFetched covers Bug 3 (b): if no local head,
// no origin remote-tracking branch, no default branch ref, and no tag of the
// same name is reachable, Create must surface ports.ErrWorkspaceBranchNotFetched
Expand Down
3 changes: 3 additions & 0 deletions backend/internal/ports/outbound.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,9 @@ var (
// ErrWorkspaceBranchNotFetched reports the requested branch exists nowhere
// reachable (no local head, no remote-tracking branch, no tag).
ErrWorkspaceBranchNotFetched = errors.New("workspace: branch is not fetched")
// ErrWorkspaceBranchInvalid reports the requested branch name is not a valid
// git ref (rejected by `git check-ref-format`).
ErrWorkspaceBranchInvalid = errors.New("workspace: invalid branch name")
// ErrWorkspaceDirty reports Destroy refused to remove a workspace because
// it holds uncommitted changes or untracked files. Teardown is never
// forced; callers treat the workspace as intentionally preserved.
Expand Down
2 changes: 2 additions & 0 deletions backend/internal/service/session/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -322,6 +322,8 @@ func toAPIError(err error) error {
return apierr.Conflict("BRANCH_CHECKED_OUT_ELSEWHERE", err.Error(), nil)
case errors.Is(err, ports.ErrWorkspaceBranchNotFetched):
return apierr.Invalid("BRANCH_NOT_FETCHED", err.Error(), nil)
case errors.Is(err, ports.ErrWorkspaceBranchInvalid):
return apierr.Invalid("INVALID_BRANCH", err.Error(), nil)
case errors.Is(err, ports.ErrAgentBinaryNotFound):
return apierr.Invalid("AGENT_BINARY_NOT_FOUND", err.Error(), nil)
default:
Expand Down
1 change: 1 addition & 0 deletions backend/internal/service/session/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,7 @@ func TestToAPIErrorMapsWorkspaceBranchSentinels(t *testing.T) {
}{
{"checked out elsewhere", fmt.Errorf("spawn mer-1: workspace: %w: \"x\" is checked out at \"/tmp\"", ports.ErrWorkspaceBranchCheckedOutElsewhere), apierr.KindConflict, "BRANCH_CHECKED_OUT_ELSEWHERE"},
{"not fetched", fmt.Errorf("spawn mer-1: workspace: %w: \"x\" has no local head", ports.ErrWorkspaceBranchNotFetched), apierr.KindInvalid, "BRANCH_NOT_FETCHED"},
{"invalid branch", fmt.Errorf("spawn mer-1: workspace: %w: \"bad!!\" (exit 1)", ports.ErrWorkspaceBranchInvalid), apierr.KindInvalid, "INVALID_BRANCH"},
{"agent binary not found", fmt.Errorf("spawn mer-1: %w", ports.ErrAgentBinaryNotFound), apierr.KindInvalid, "AGENT_BINARY_NOT_FOUND"},
}
for _, tc := range cases {
Expand Down
Loading