diff --git a/backend/internal/adapters/workspace/gitworktree/workspace.go b/backend/internal/adapters/workspace/gitworktree/workspace.go index 4fb928c1..3d7f88e5 100644 --- a/backend/internal/adapters/workspace/gitworktree/workspace.go +++ b/backend/internal/adapters/workspace/gitworktree/workspace.go @@ -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. @@ -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 } diff --git a/backend/internal/adapters/workspace/gitworktree/workspace_test.go b/backend/internal/adapters/workspace/gitworktree/workspace_test.go index 9c084b2e..dd75d235 100644 --- a/backend/internal/adapters/workspace/gitworktree/workspace_test.go +++ b/backend/internal/adapters/workspace/gitworktree/workspace_test.go @@ -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 diff --git a/backend/internal/ports/outbound.go b/backend/internal/ports/outbound.go index 4f7b20ac..668da516 100644 --- a/backend/internal/ports/outbound.go +++ b/backend/internal/ports/outbound.go @@ -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. diff --git a/backend/internal/service/session/service.go b/backend/internal/service/session/service.go index 51e43396..2582d5b0 100644 --- a/backend/internal/service/session/service.go +++ b/backend/internal/service/session/service.go @@ -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: diff --git a/backend/internal/service/session/service_test.go b/backend/internal/service/session/service_test.go index 9af3ceea..b3867967 100644 --- a/backend/internal/service/session/service_test.go +++ b/backend/internal/service/session/service_test.go @@ -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 {