diff --git a/cmd/roborev/tui/actions.go b/cmd/roborev/tui/actions.go index 3ae7dba6..e016208d 100644 --- a/cmd/roborev/tui/actions.go +++ b/cmd/roborev/tui/actions.go @@ -3,8 +3,6 @@ package tui import ( "errors" "fmt" - "io" - "net/http" "os" "os/exec" "strings" @@ -244,9 +242,9 @@ func (m model) triggerFix(parentJobID int64, prompt, gitRef string) tea.Cmd { // needWorktree if the branch is not checked out anywhere. func (m model) applyFixPatch(jobID int64) tea.Cmd { return func() tea.Msg { - patch, jobDetail, msg := m.fetchPatchAndJob(jobID) - if msg != nil { - return *msg + patch, jobDetail, err := m.loadPatchAndJob(jobID) + if err != nil { + return applyPatchResultMsg{jobID: jobID, err: err} } // Resolve the target directory: if the branch has its own worktree, @@ -271,9 +269,9 @@ func (m model) applyFixPatch(jobID int64) tea.Cmd { // patch there, commits, and removes the worktree. The commit persists on the branch. func (m model) applyFixPatchInWorktree(jobID int64) tea.Cmd { return func() tea.Msg { - patch, jobDetail, msg := m.fetchPatchAndJob(jobID) - if msg != nil { - return *msg + patch, jobDetail, err := m.loadPatchAndJob(jobID) + if err != nil { + return applyPatchResultMsg{jobID: jobID, err: err} } // Create a temporary worktree on the branch. @@ -309,32 +307,16 @@ func (m model) applyFixPatchInWorktree(jobID int64) tea.Cmd { } } -// fetchPatchAndJob fetches the patch content and job details for a fix job. -// Returns nil msg on success; a non-nil msg should be returned to the TUI immediately. -func (m model) fetchPatchAndJob(jobID int64) (string, *storage.ReviewJob, *applyPatchResultMsg) { - url := m.endpoint.BaseURL() + fmt.Sprintf("/api/job/patch?job_id=%d", jobID) - resp, err := m.client.Get(url) +// loadPatchAndJob fetches the patch content and job details for a fix job. +func (m model) loadPatchAndJob(jobID int64) (string, *storage.ReviewJob, error) { + patch, err := m.loadPatch(jobID) if err != nil { - return "", nil, &applyPatchResultMsg{jobID: jobID, err: err} - } - defer resp.Body.Close() - - if resp.StatusCode != http.StatusOK { - return "", nil, &applyPatchResultMsg{jobID: jobID, err: fmt.Errorf("no patch available")} + return "", nil, err } - patchData, err := io.ReadAll(resp.Body) + jobDetail, err := m.loadJob(jobID) if err != nil { - return "", nil, &applyPatchResultMsg{jobID: jobID, err: err} - } - patch := string(patchData) - if patch == "" { - return "", nil, &applyPatchResultMsg{jobID: jobID, err: fmt.Errorf("empty patch")} - } - - jobDetail, jErr := m.fetchJobByID(jobID) - if jErr != nil { - return "", nil, &applyPatchResultMsg{jobID: jobID, err: jErr} + return "", nil, err } return patch, jobDetail, nil @@ -476,7 +458,7 @@ func patchFiles(patch string) ([]string, error) { func (m model) triggerRebase(staleJobID int64) tea.Cmd { return func() tea.Msg { // Find the parent job ID (the original review this fix was for) - staleJob, fetchErr := m.fetchJobByID(staleJobID) + staleJob, fetchErr := m.loadJob(staleJobID) if fetchErr != nil { return fixTriggerResultMsg{err: fmt.Errorf("stale job %d not found: %w", staleJobID, fetchErr)} } diff --git a/cmd/roborev/tui/api.go b/cmd/roborev/tui/api.go index 7cf7f491..412d5e65 100644 --- a/cmd/roborev/tui/api.go +++ b/cmd/roborev/tui/api.go @@ -56,6 +56,30 @@ func (m model) getJSON(path string, out any) error { return nil } +// getText performs a GET request and returns the raw response body as text. +// Returns errNotFound for 404 responses. Other errors include the server's message. +func (m model) getText(path string) (string, error) { + url := m.endpoint.BaseURL() + path + resp, err := m.client.Get(url) + if err != nil { + return "", err + } + defer resp.Body.Close() + + if resp.StatusCode == http.StatusNotFound { + return "", fmt.Errorf("%w: %s", errNotFound, readErrorBody(resp.Body, resp.Status)) + } + if resp.StatusCode != http.StatusOK { + return "", fmt.Errorf("%s", readErrorBody(resp.Body, resp.Status)) + } + + data, err := io.ReadAll(resp.Body) + if err != nil { + return "", fmt.Errorf("read response: %w", err) + } + return string(data), nil +} + // postJSON performs a POST request with a JSON body and decodes the response into out. // If out is nil, the response body is discarded. // Returns errNotFound (wrapped with server message) for 404 responses. diff --git a/cmd/roborev/tui/fetch.go b/cmd/roborev/tui/fetch.go index 82e6d37d..7af458b8 100644 --- a/cmd/roborev/tui/fetch.go +++ b/cmd/roborev/tui/fetch.go @@ -5,7 +5,6 @@ import ( "encoding/json" "errors" "fmt" - "io" "net/http" neturl "net/url" "sort" @@ -48,6 +47,79 @@ func (m model) tickInterval() time.Duration { return tickIntervalIdle } +type jobsPageResult struct { + Jobs []storage.ReviewJob `json:"jobs"` + HasMore bool `json:"has_more"` + Stats storage.JobStats `json:"stats"` +} + +type repoListResult struct { + Repos []struct { + Name string `json:"name"` + RootPath string `json:"root_path"` + Count int `json:"count"` + } `json:"repos"` + TotalCount int `json:"total_count"` +} + +type branchListResult struct { + Branches []struct { + Name string `json:"name"` + Count int `json:"count"` + } `json:"branches"` + TotalCount int `json:"total_count"` +} + +func (m model) loadJobsPage(params neturl.Values) (*jobsPageResult, error) { + path := "/api/jobs" + if encoded := params.Encode(); encoded != "" { + path += "?" + encoded + } + + var result jobsPageResult + if err := m.getJSON(path, &result); err != nil { + return nil, err + } + return &result, nil +} + +func (m model) loadRepoList(branchFilter string) (*repoListResult, bool, error) { + path := "/api/repos" + branchFiltered := branchFilter != "" && branchFilter != branchNone + if branchFiltered { + params := neturl.Values{} + params.Set("branch", branchFilter) + path += "?" + params.Encode() + } + + var result repoListResult + if err := m.getJSON(path, &result); err != nil { + return nil, false, err + } + return &result, branchFiltered, nil +} + +func (m model) loadBranchList(rootPaths []string) (*branchListResult, error) { + path := "/api/branches" + if len(rootPaths) > 0 { + params := neturl.Values{} + for _, repoPath := range rootPaths { + if repoPath != "" { + params.Add("repo", repoPath) + } + } + if encoded := params.Encode(); encoded != "" { + path += "?" + encoded + } + } + + var result branchListResult + if err := m.getJSON(path, &result); err != nil { + return nil, err + } + return &result, nil +} + func (m model) fetchJobs() tea.Cmd { // Fetch enough to fill the visible area plus a buffer for smooth scrolling. // Use minimum of 100 only before first WindowSizeMsg (when height is default 24) @@ -100,24 +172,12 @@ func (m model) fetchJobs() tea.Cmd { params.Set("limit", fmt.Sprintf("%d", limit)) } - url := fmt.Sprintf("%s/api/jobs?%s", m.endpoint.BaseURL(), params.Encode()) - resp, err := m.client.Get(url) + result, err := m.loadJobsPage(params) if err != nil { - return jobsErrMsg{err: err, seq: seq} - } - defer resp.Body.Close() - - if resp.StatusCode != http.StatusOK { - return jobsErrMsg{err: fmt.Errorf("fetch jobs: %s", resp.Status), seq: seq} - } - - var result struct { - Jobs []storage.ReviewJob `json:"jobs"` - HasMore bool `json:"has_more"` - Stats storage.JobStats `json:"stats"` - } - if err := json.NewDecoder(resp.Body).Decode(&result); err != nil { - return jobsErrMsg{err: err, seq: seq} + return jobsErrMsg{ + err: fmt.Errorf("fetch jobs: %w", err), + seq: seq, + } } return jobsMsg{jobs: result.Jobs, hasMore: result.HasMore, append: false, seq: seq, stats: result.Stats} } @@ -144,23 +204,12 @@ func (m model) fetchMoreJobs() tea.Cmd { params.Set("closed", "false") } params.Set("exclude_job_type", "fix") - url := fmt.Sprintf("%s/api/jobs?%s", m.endpoint.BaseURL(), params.Encode()) - resp, err := m.client.Get(url) + result, err := m.loadJobsPage(params) if err != nil { - return paginationErrMsg{err: err, seq: seq} - } - defer resp.Body.Close() - - if resp.StatusCode != http.StatusOK { - return paginationErrMsg{err: fmt.Errorf("fetch more jobs: %s", resp.Status), seq: seq} - } - - var result struct { - Jobs []storage.ReviewJob `json:"jobs"` - HasMore bool `json:"has_more"` - } - if err := json.NewDecoder(resp.Body).Decode(&result); err != nil { - return paginationErrMsg{err: err, seq: seq} + return paginationErrMsg{ + err: fmt.Errorf("fetch more jobs: %w", err), + seq: seq, + } } return jobsMsg{jobs: result.Jobs, hasMore: result.HasMore, append: true, seq: seq} } @@ -201,28 +250,11 @@ func (m model) tryReconnect() tea.Cmd { // fetchRepoNames fetches the unfiltered repo list and builds a // display-name-to-root-paths mapping for control socket resolution. func (m model) fetchRepoNames() tea.Cmd { - client := m.client - baseURL := m.endpoint.BaseURL() - return func() tea.Msg { - resp, err := client.Get(baseURL + "/api/repos") + result, _, err := m.loadRepoList("") if err != nil { return repoNamesMsg{} // non-fatal; map stays nil } - defer resp.Body.Close() - if resp.StatusCode != http.StatusOK { - return repoNamesMsg{} - } - - var result struct { - Repos []struct { - Name string `json:"name"` - RootPath string `json:"root_path"` - } `json:"repos"` - } - if err := json.NewDecoder(resp.Body).Decode(&result); err != nil { - return repoNamesMsg{} - } names := make(map[string][]string) for _, r := range result.Repos { @@ -237,41 +269,12 @@ func (m model) fetchRepoNames() tea.Cmd { } func (m model) fetchRepos() tea.Cmd { - // Capture values for use in goroutine - client := m.client - baseURL := m.endpoint.BaseURL() activeBranchFilter := m.activeBranchFilter // Constrain repos by active branch filter return func() tea.Msg { - // Build URL with optional branch filter (URL-encoded) - // Skip sending branch for branchNone sentinel - it's a client-side filter - reposURL := baseURL + "/api/repos" - if activeBranchFilter != "" && activeBranchFilter != branchNone { - params := neturl.Values{} - params.Set("branch", activeBranchFilter) - reposURL += "?" + params.Encode() - } - - resp, err := client.Get(reposURL) + reposResult, filtered, err := m.loadRepoList(activeBranchFilter) if err != nil { - return errMsg(err) - } - defer resp.Body.Close() - - if resp.StatusCode != http.StatusOK { - return errMsg(fmt.Errorf("fetch repos: %s", resp.Status)) - } - - var reposResult struct { - Repos []struct { - Name string `json:"name"` - RootPath string `json:"root_path"` - Count int `json:"count"` - } `json:"repos"` - TotalCount int `json:"total_count"` - } - if err := json.NewDecoder(resp.Body).Decode(&reposResult); err != nil { - return errMsg(err) + return errMsg(fmt.Errorf("fetch repos: %w", err)) } // Aggregate repos by display name @@ -298,8 +301,6 @@ func (m model) fetchRepos() tea.Cmd { for i, name := range displayNameOrder { repos[i] = *displayNameMap[name] } - filtered := activeBranchFilter != "" && - activeBranchFilter != branchNone return reposMsg{repos: repos, branchFiltered: filtered} } } @@ -312,9 +313,6 @@ func (m model) fetchRepos() tea.Cmd { func (m model) fetchBranchesForRepo( rootPaths []string, repoIdx int, expand bool, searchSeq int, ) tea.Cmd { - client := m.client - baseURL := m.endpoint.BaseURL() - errMsg := func(err error) repoBranchesMsg { return repoBranchesMsg{ repoIdx: repoIdx, @@ -326,40 +324,9 @@ func (m model) fetchBranchesForRepo( } return func() tea.Msg { - branchURL := baseURL + "/api/branches" - if len(rootPaths) > 0 { - params := neturl.Values{} - for _, repoPath := range rootPaths { - if repoPath != "" { - params.Add("repo", repoPath) - } - } - if len(params) > 0 { - branchURL += "?" + params.Encode() - } - } - - resp, err := client.Get(branchURL) + branchResult, err := m.loadBranchList(rootPaths) if err != nil { - return errMsg(err) - } - defer resp.Body.Close() - - if resp.StatusCode != http.StatusOK { - return errMsg( - fmt.Errorf("fetch branches for repo: %s", resp.Status), - ) - } - - var branchResult struct { - Branches []struct { - Name string `json:"name"` - Count int `json:"count"` - } `json:"branches"` - TotalCount int `json:"total_count"` - } - if err := json.NewDecoder(resp.Body).Decode(&branchResult); err != nil { - return errMsg(err) + return errMsg(fmt.Errorf("fetch branches for repo: %w", err)) } branches := make([]branchFilterItem, len(branchResult.Branches)) @@ -537,6 +504,36 @@ func (m model) loadResponses(jobID int64, review *storage.Review) []storage.Resp return responses } +func (m model) loadPatch(jobID int64) (string, error) { + patch, err := m.getText(fmt.Sprintf("/api/job/patch?job_id=%d", jobID)) + if err != nil { + if errors.Is(err, errNotFound) { + return "", fmt.Errorf("no patch available") + } + return "", fmt.Errorf("fetch patch: %w", err) + } + if patch == "" { + return "", fmt.Errorf("empty patch") + } + return patch, nil +} + +func (m model) loadJob(jobID int64) (*storage.ReviewJob, error) { + params := neturl.Values{} + params.Set("id", fmt.Sprintf("%d", jobID)) + + result, err := m.loadJobsPage(params) + if err != nil { + return nil, fmt.Errorf("fetch job: %w", err) + } + for i := range result.Jobs { + if result.Jobs[i].ID == jobID { + return &result.Jobs[i], nil + } + } + return nil, fmt.Errorf("job %d not found", jobID) +} + func (m model) fetchReview(jobID int64) tea.Cmd { return func() tea.Msg { review, err := m.loadReview(jobID) @@ -809,44 +806,22 @@ func (m model) fetchCommitMsg(job *storage.ReviewJob) tea.Cmd { } func (m model) fetchPatch(jobID int64) tea.Cmd { return func() tea.Msg { - url := m.endpoint.BaseURL() + fmt.Sprintf("/api/job/patch?job_id=%d", jobID) - resp, err := m.client.Get(url) - if err != nil { - return patchMsg{jobID: jobID, err: err} - } - defer resp.Body.Close() - if resp.StatusCode != http.StatusOK { - return patchMsg{jobID: jobID, err: fmt.Errorf("no patch available (HTTP %d)", resp.StatusCode)} - } - data, err := io.ReadAll(resp.Body) + patch, err := m.loadPatch(jobID) if err != nil { return patchMsg{jobID: jobID, err: err} } - return patchMsg{jobID: jobID, patch: string(data)} + return patchMsg{jobID: jobID, patch: patch} } } -func (m model) fetchJobByID(jobID int64) (*storage.ReviewJob, error) { - var result struct { - Jobs []storage.ReviewJob `json:"jobs"` - } - if err := m.getJSON(fmt.Sprintf("/api/jobs?id=%d", jobID), &result); err != nil { - return nil, err - } - for i := range result.Jobs { - if result.Jobs[i].ID == jobID { - return &result.Jobs[i], nil - } - } - return nil, fmt.Errorf("job %d not found", jobID) -} // fetchFixJobs fetches fix jobs from the daemon. func (m model) fetchFixJobs() tea.Cmd { return func() tea.Msg { - var result struct { - Jobs []storage.ReviewJob `json:"jobs"` - } - err := m.getJSON("/api/jobs?job_type=fix&limit=200", &result) + params := neturl.Values{} + params.Set("job_type", "fix") + params.Set("limit", "200") + + result, err := m.loadJobsPage(params) if err != nil { return fixJobsMsg{err: err} }