From 8d16fabaed3dc8c65d00715536f450267c5a6e83 Mon Sep 17 00:00:00 2001 From: Jerry Yan <792602257@qq.com> Date: Wed, 10 Jun 2026 18:20:54 +0800 Subject: [PATCH] =?UTF-8?q?fix:=20address=20code=20review=20=E2=80=94=20er?= =?UTF-8?q?ror-path=20tests,=20edge=20cases,=20UX=20improvements?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- internal/infra/git/branch.go | 2 +- internal/infra/git/branch_test.go | 2 +- internal/workspace/workspace_service.go | 6 ++ internal/workspace/workspace_service_test.go | 68 +++++++++++++++++-- web/src/api/workspaces.ts | 7 +- .../workspaces/components/WorktreesTab.vue | 2 + 6 files changed, 77 insertions(+), 10 deletions(-) diff --git a/internal/infra/git/branch.go b/internal/infra/git/branch.go index 1e132a7..4c66ebd 100644 --- a/internal/infra/git/branch.go +++ b/internal/infra/git/branch.go @@ -52,7 +52,7 @@ func parseRemoteBranches(buf []byte) []string { var branches []string sc := bufio.NewScanner(bytes.NewReader(buf)) for sc.Scan() { - line := sc.Text() + line := strings.TrimSpace(sc.Text()) if idx := strings.Index(line, "\trefs/heads/"); idx >= 0 { name := line[idx+len("\trefs/heads/"):] if name != "" { diff --git a/internal/infra/git/branch_test.go b/internal/infra/git/branch_test.go index 5c08695..812ab2f 100644 --- a/internal/infra/git/branch_test.go +++ b/internal/infra/git/branch_test.go @@ -8,7 +8,7 @@ import ( func TestParseRemoteBranches(t *testing.T) { t.Parallel() - input := []byte("aabbccdd\trefs/heads/main\n" + + input := []byte("aabbccdd\trefs/heads/main\r\n" + "eeff0011\trefs/heads/develop\n" + "22334455\trefs/heads/feature/foo\n" + "deadbeef\trefs/tags/v1.0\n") diff --git a/internal/workspace/workspace_service.go b/internal/workspace/workspace_service.go index 43bb120..5314aa0 100644 --- a/internal/workspace/workspace_service.go +++ b/internal/workspace/workspace_service.go @@ -338,6 +338,12 @@ func (s *workspaceService) ListBranches(ctx context.Context, c Caller, wsID uuid } else if !canRead { return nil, errs.New(errs.CodeForbidden, "no read access") } + if ws.MainPath == "" { + return nil, errs.New(errs.CodeInvalidInput, "workspace not yet cloned") + } + if _, statErr := os.Stat(ws.MainPath); errors.Is(statErr, os.ErrNotExist) { + return nil, errs.New(errs.CodeInvalidInput, "workspace not yet cloned") + } cred, err := s.loadDecryptedCredential(ctx, wsID) if err != nil { return nil, err diff --git a/internal/workspace/workspace_service_test.go b/internal/workspace/workspace_service_test.go index 1d2cd3b..fa9554a 100644 --- a/internal/workspace/workspace_service_test.go +++ b/internal/workspace/workspace_service_test.go @@ -250,6 +250,22 @@ func (t *trackGit) Checkout(_ context.Context, _, branch string) error { return nil } +type errFetchGit struct { + git.Runner + err error +} + +func (e *errFetchGit) FetchRemoteBranch(_ context.Context, _, _ string, _ *git.Credential) error { + return e.err +} + +type errCheckoutGit struct { + git.Runner + err error +} + +func (e *errCheckoutGit) Checkout(_ context.Context, _, _ string) error { return e.err } + type noopAudit struct{} func (noopAudit) Record(_ context.Context, _ audit.Entry) error { return nil } @@ -404,11 +420,9 @@ func TestCloneRunner_Failure(t *testing.T) { } func TestWorkspaceService_Update_DefaultBranch_Switch(t *testing.T) { - // 切换 default_branch 时，service 必须先 fetch+checkout 远端分支，再落库。 - // 注意：fakeGit 默认 stub 不会记录调用，本测试仅断言最终 DB 状态。 t.Parallel() pa := &fakePA{pid: uuid.New(), canRead: true, canWrite: true} - fg := &fakeGit{} + fg := &trackGit{Runner: &fakeGit{}} svc, repo := newSvc(t, pa, nil, fg) caller := Caller{UserID: uuid.New()} @@ -426,6 +440,52 @@ func TestWorkspaceService_Update_DefaultBranch_Switch(t *testing.T) { }) require.NoError(t, err) require.Equal(t, "develop", repo.wss[wsID].DefaultBranch) + require.Equal(t, []string{"develop"}, fg.fetchRemoteBranchCalls) + require.Equal(t, []string{"develop"}, fg.checkoutCalls) +} + +func TestWorkspaceService_Update_DefaultBranch_FetchFails(t *testing.T) { + t.Parallel() + pa := &fakePA{pid: uuid.New(), canRead: true, canWrite: true} + fg := &errFetchGit{Runner: &fakeGit{}, err: errors.New("remote branch not found")} + svc, repo := newSvc(t, pa, nil, fg) + caller := Caller{UserID: uuid.New()} + + wsID := uuid.New() + repo.wss[wsID] = &Workspace{ + ID: wsID, ProjectID: pa.pid, Slug: "ws1", Name: "W1", + DefaultBranch: "main", MainPath: "/data/ws1/main", + } + + newBr := "nonexistent" + _, err := svc.Update(context.Background(), caller, wsID, UpdateWorkspaceInput{ + DefaultBranch: &newBr, + }) + require.Error(t, err) + // DB should NOT be updated + require.Equal(t, "main", repo.wss[wsID].DefaultBranch) +} + +func TestWorkspaceService_Update_DefaultBranch_CheckoutFails(t *testing.T) { + t.Parallel() + pa := &fakePA{pid: uuid.New(), canRead: true, canWrite: true} + fg := &errCheckoutGit{Runner: &fakeGit{}, err: errors.New("dirty working tree")} + svc, repo := newSvc(t, pa, nil, fg) + caller := Caller{UserID: uuid.New()} + + wsID := uuid.New() + repo.wss[wsID] = &Workspace{ + ID: wsID, ProjectID: pa.pid, Slug: "ws1", Name: "W1", + DefaultBranch: "main", MainPath: "/data/ws1/main", + } + + newBr := "develop" + _, err := svc.Update(context.Background(), caller, wsID, UpdateWorkspaceInput{ + DefaultBranch: &newBr, + }) + require.Error(t, err) + // DB should NOT be updated + require.Equal(t, "main", repo.wss[wsID].DefaultBranch) } func TestWorkspaceService_Update_DefaultBranch_SameValue_NoGitOps(t *testing.T) { @@ -460,7 +520,7 @@ func TestWorkspaceService_ListBranches(t *testing.T) { wsID := uuid.New() repo.wss[wsID] = &Workspace{ ID: wsID, ProjectID: pa.pid, Slug: "ws1", - DefaultBranch: "main", MainPath: "/data/ws1/main", + DefaultBranch: "main", MainPath: t.TempDir(), } branches, err := svc.ListBranches(context.Background(), caller, wsID) diff --git a/web/src/api/workspaces.ts b/web/src/api/workspaces.ts index b2a8869..f7a6142 100644 --- a/web/src/api/workspaces.ts +++ b/web/src/api/workspaces.ts @@ -52,10 +52,9 @@ export const workspacesApi = { release: (wtID: string) => request(`/api/v1/worktrees/${enc(wtID)}/release`, { method: 'POST' }), - listBranches(wsID: string): Promise { - return request<{ branches: string[] }>(`/api/v1/workspaces/${enc(wsID)}/branches`) - .then(r => r.branches) - }, + listBranches: (wsID: string): Promise => + request<{ branches: string[] }>(`/api/v1/workspaces/${enc(wsID)}/branches`) + .then(r => r.branches), // ===== Git ops ===== statusOnMain: (wsID: string) => diff --git a/web/src/views/workspaces/components/WorktreesTab.vue b/web/src/views/workspaces/components/WorktreesTab.vue index e0093d9..dbfd196 100644 --- a/web/src/views/workspaces/components/WorktreesTab.vue +++ b/web/src/views/workspaces/components/WorktreesTab.vue @@ -36,6 +36,8 @@ :options="baseBranchOptions" :loading="branchesLoading" clearable + filterable + tag placeholder="默认使用 default_branch" />