fix: support issue links in remote task dialog (#1462)

This commit is contained in:
Carlos Florêncio
2026-06-22 15:16:43 +01:00
committed by GitHub
parent 31b32836f6
commit d58d9d03bb
27 changed files with 974 additions and 72 deletions
+3
View File
@@ -16,6 +16,9 @@ type Client interface {
// GetPR retrieves a single pull request by number.
GetPR(ctx context.Context, owner, repo string, number int) (*PR, error)
// GetIssue retrieves a single GitHub issue by number.
GetIssue(ctx context.Context, owner, repo string, number int) (*Issue, error)
// FindPRByBranch finds an open PR for the given head branch.
FindPRByBranch(ctx context.Context, owner, repo, branch string) (*PR, error)
+55 -17
View File
@@ -50,6 +50,7 @@ func (c *Controller) RegisterHTTPRoutes(router *gin.Engine) {
api.POST("/prs/statuses", c.httpGetPRStatusesBatch)
api.POST("/prs/:owner/:repo/:number/reviews", c.httpSubmitReview)
api.PUT("/prs/:owner/:repo/:number/merge", c.httpMergePR)
api.GET("/issues/:owner/:repo/:number/info", c.httpGetIssueInfo)
api.GET("/watches/pr", c.httpListPRWatches)
api.DELETE("/watches/pr/:id", c.httpDeletePRWatch)
@@ -354,32 +355,69 @@ func (c *Controller) httpGetPRStatusesBatch(ctx *gin.Context) {
func (c *Controller) httpGetPRInfo(ctx *gin.Context) {
owner := ctx.Param("owner")
repo := ctx.Param("repo")
numberStr := ctx.Param("number")
number, err := strconv.Atoi(numberStr)
if err != nil {
ctx.JSON(http.StatusBadRequest, gin.H{"error": "invalid PR number"})
number, ok := parseRouteNumber(ctx, "invalid PR number")
if !ok {
return
}
pr, err := c.service.GetPR(ctx.Request.Context(), owner, repo, number)
if err != nil {
status := http.StatusInternalServerError
var apiErr *GitHubAPIError
if errors.As(err, &apiErr) {
switch apiErr.StatusCode {
case http.StatusNotFound:
status = http.StatusNotFound
case http.StatusUnauthorized:
status = http.StatusUnauthorized
case http.StatusForbidden:
status = http.StatusForbidden
}
}
ctx.JSON(status, gin.H{"error": err.Error()})
handleGitHubInfoError(ctx, err)
return
}
ctx.JSON(http.StatusOK, pr)
}
func (c *Controller) httpGetIssueInfo(ctx *gin.Context) {
owner := ctx.Param("owner")
repo := ctx.Param("repo")
number, ok := parseRouteNumber(ctx, "invalid issue number")
if !ok {
return
}
issue, err := c.service.GetIssue(ctx.Request.Context(), owner, repo, number)
if err != nil {
handleGitHubInfoError(ctx, err)
return
}
ctx.JSON(http.StatusOK, issue)
}
func parseRouteNumber(ctx *gin.Context, invalidMessage string) (int, bool) {
number, err := strconv.Atoi(ctx.Param("number"))
if err != nil {
ctx.JSON(http.StatusBadRequest, gin.H{"error": invalidMessage})
return 0, false
}
return number, true
}
func handleGitHubInfoError(ctx *gin.Context, err error) {
if errors.Is(err, ErrNoClient) {
ctx.JSON(http.StatusServiceUnavailable, gin.H{
"error": "GitHub is not configured. Connect GitHub in Settings > Integrations.",
"code": "github_not_configured",
})
return
}
status := http.StatusInternalServerError
message := "failed to fetch GitHub info"
var apiErr *GitHubAPIError
if errors.As(err, &apiErr) {
switch apiErr.StatusCode {
case http.StatusNotFound:
status = http.StatusNotFound
case http.StatusUnauthorized:
status = http.StatusUnauthorized
case http.StatusForbidden:
status = http.StatusForbidden
}
}
if status != http.StatusInternalServerError {
message = err.Error()
}
ctx.JSON(status, gin.H{"error": message})
}
func (c *Controller) httpSubmitReview(ctx *gin.Context) {
owner := ctx.Param("owner")
repo := ctx.Param("repo")
@@ -23,6 +23,7 @@ import (
// stubClient implements Client with no-op defaults; override fields as needed.
type stubClient struct {
getPRFunc func(ctx context.Context, owner, repo string, number int) (*PR, error)
getIssueFunc func(ctx context.Context, owner, repo string, number int) (*Issue, error)
mergePRFn func(ctx context.Context, owner, repo string, number int, mergeMethod string) error
getRepoMergeMethodsFn func() (RepoMergeMethods, error)
}
@@ -37,6 +38,12 @@ func (s *stubClient) GetPR(ctx context.Context, owner, repo string, number int)
}
return nil, fmt.Errorf("not implemented")
}
func (s *stubClient) GetIssue(ctx context.Context, owner, repo string, number int) (*Issue, error) {
if s.getIssueFunc != nil {
return s.getIssueFunc(ctx, owner, repo, number)
}
return nil, fmt.Errorf("not implemented")
}
func (s *stubClient) FindPRByBranch(context.Context, string, string, string) (*PR, error) {
return nil, nil
}
@@ -337,6 +344,143 @@ func TestHttpGetPRInfo_ServiceError(t *testing.T) {
}
}
func TestHttpGetPRInfo_NoClient(t *testing.T) {
router, _ := setupControllerTest(nil)
req := httptest.NewRequest(http.MethodGet, "/api/v1/github/prs/acme/widget/99/info", nil)
w := httptest.NewRecorder()
router.ServeHTTP(w, req)
if w.Code != http.StatusServiceUnavailable {
t.Fatalf("expected 503, got %d: %s", w.Code, w.Body.String())
}
var got struct {
Code string `json:"code"`
}
if err := json.NewDecoder(w.Body).Decode(&got); err != nil {
t.Fatalf("decode response: %v", err)
}
if got.Code != "github_not_configured" {
t.Fatalf("expected github_not_configured code, got %q", got.Code)
}
}
func TestHttpGetPRInfo_GitHubAPIErrorStatus(t *testing.T) {
sc := &stubClient{
getPRFunc: func(context.Context, string, string, int) (*PR, error) {
return nil, &GitHubAPIError{
StatusCode: http.StatusNotFound,
Endpoint: "/repos/acme/widget/pulls/99",
Body: "upstream error",
}
},
}
router, _ := setupControllerTest(sc)
req := httptest.NewRequest(http.MethodGet, "/api/v1/github/prs/acme/widget/99/info", nil)
w := httptest.NewRecorder()
router.ServeHTTP(w, req)
if w.Code != http.StatusNotFound {
t.Fatalf("expected 404, got %d", w.Code)
}
}
func TestHttpGetIssueInfo_Success(t *testing.T) {
sc := &stubClient{
getIssueFunc: func(_ context.Context, owner, repo string, number int) (*Issue, error) {
if owner != "acme" || repo != "widget" || number != 1456 {
t.Errorf("unexpected params: %s/%s#%d", owner, repo, number)
}
return &Issue{Number: 1456, Title: "fix remote picker"}, nil
},
}
router, _ := setupControllerTest(sc)
req := httptest.NewRequest(http.MethodGet, "/api/v1/github/issues/acme/widget/1456/info", nil)
w := httptest.NewRecorder()
router.ServeHTTP(w, req)
if w.Code != http.StatusOK {
t.Fatalf("expected 200, got %d: %s", w.Code, w.Body.String())
}
var issue Issue
if err := json.NewDecoder(w.Body).Decode(&issue); err != nil {
t.Fatalf("failed to decode response: %v", err)
}
if issue.Number != 1456 {
t.Errorf("expected issue number 1456, got %d", issue.Number)
}
if issue.Title != "fix remote picker" {
t.Errorf("expected issue title, got %q", issue.Title)
}
}
func TestHttpGetIssueInfo_InvalidNumber(t *testing.T) {
router, _ := setupControllerTest(&stubClient{})
req := httptest.NewRequest(http.MethodGet, "/api/v1/github/issues/acme/widget/abc/info", nil)
w := httptest.NewRecorder()
router.ServeHTTP(w, req)
if w.Code != http.StatusBadRequest {
t.Fatalf("expected 400, got %d", w.Code)
}
}
func TestHttpGetIssueInfo_ServiceError(t *testing.T) {
sc := &stubClient{
getIssueFunc: func(context.Context, string, string, int) (*Issue, error) {
return nil, fmt.Errorf("not found")
},
}
router, _ := setupControllerTest(sc)
req := httptest.NewRequest(http.MethodGet, "/api/v1/github/issues/acme/widget/99/info", nil)
w := httptest.NewRecorder()
router.ServeHTTP(w, req)
if w.Code != http.StatusInternalServerError {
t.Fatalf("expected 500, got %d", w.Code)
}
}
func TestHttpGetIssueInfo_GitHubAPIErrorStatus(t *testing.T) {
tests := []struct {
name string
apiStatus int
wantStatus int
}{
{name: "not found", apiStatus: http.StatusNotFound, wantStatus: http.StatusNotFound},
{name: "unauthorized", apiStatus: http.StatusUnauthorized, wantStatus: http.StatusUnauthorized},
{name: "forbidden", apiStatus: http.StatusForbidden, wantStatus: http.StatusForbidden},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
sc := &stubClient{
getIssueFunc: func(context.Context, string, string, int) (*Issue, error) {
return nil, &GitHubAPIError{
StatusCode: tt.apiStatus,
Endpoint: "/repos/acme/widget/issues/99",
Body: "upstream error",
}
},
}
router, _ := setupControllerTest(sc)
req := httptest.NewRequest(http.MethodGet, "/api/v1/github/issues/acme/widget/99/info", nil)
w := httptest.NewRecorder()
router.ServeHTTP(w, req)
if w.Code != tt.wantStatus {
t.Fatalf("expected %d, got %d", tt.wantStatus, w.Code)
}
})
}
}
func TestHttpMergePR_Success(t *testing.T) {
var called struct {
owner string
+76
View File
@@ -204,11 +204,39 @@ type ghPR struct {
} `json:"author"`
}
type ghIssue struct {
Number int `json:"number"`
Title string `json:"title"`
URL string `json:"url"`
State string `json:"state"`
Body string `json:"body"`
CreatedAt time.Time `json:"createdAt"`
UpdatedAt time.Time `json:"updatedAt"`
// The gh CLI currently emits an empty string for open issues.
ClosedAt string `json:"closedAt"`
Author struct {
Login string `json:"login"`
} `json:"author"`
Labels []struct {
Name string `json:"name"`
} `json:"labels"`
Assignees []struct {
Login string `json:"login"`
} `json:"assignees"`
}
func (c *GHClient) GetPR(ctx context.Context, owner, repo string, number int) (*PR, error) {
out, err := c.run(ctx, "pr", "view", fmt.Sprintf("%d", number),
"--repo", fmt.Sprintf("%s/%s", owner, repo),
"--json", "number,title,url,state,body,headRefName,headRefOid,baseRefName,author,isDraft,mergeable,mergeStateStatus,additions,deletions,createdAt,updatedAt,mergedAt,closedAt,reviewRequests")
if err != nil {
if isNotFoundErr(err) {
return nil, &GitHubAPIError{
StatusCode: http.StatusNotFound,
Endpoint: fmt.Sprintf("/repos/%s/%s/pulls/%d", owner, repo, number),
Body: err.Error(),
}
}
return nil, fmt.Errorf("get PR #%d: %w", number, err)
}
var raw ghPR
@@ -218,6 +246,27 @@ func (c *GHClient) GetPR(ctx context.Context, owner, repo string, number int) (*
return convertGHPR(&raw, owner, repo), nil
}
func (c *GHClient) GetIssue(ctx context.Context, owner, repo string, number int) (*Issue, error) {
out, err := c.run(ctx, "issue", "view", fmt.Sprintf("%d", number),
"--repo", fmt.Sprintf("%s/%s", owner, repo),
"--json", "number,title,url,state,body,author,labels,assignees,createdAt,updatedAt,closedAt")
if err != nil {
if isNotFoundErr(err) {
return nil, &GitHubAPIError{
StatusCode: http.StatusNotFound,
Endpoint: fmt.Sprintf("/repos/%s/%s/issues/%d", owner, repo, number),
Body: err.Error(),
}
}
return nil, fmt.Errorf("get issue #%d: %w", number, err)
}
var raw ghIssue
if err := json.Unmarshal([]byte(out), &raw); err != nil {
return nil, fmt.Errorf("parse issue response: %w", err)
}
return convertGHIssue(&raw, owner, repo), nil
}
func (c *GHClient) FindPRByBranch(ctx context.Context, owner, repo, branch string) (*PR, error) {
out, err := c.run(ctx, "pr", "list",
"--repo", fmt.Sprintf("%s/%s", owner, repo),
@@ -1053,6 +1102,33 @@ func convertGHPR(raw *ghPR, owner, repo string) *PR {
return pr
}
func convertGHIssue(raw *ghIssue, owner, repo string) *Issue {
labels := make([]string, len(raw.Labels))
for i, label := range raw.Labels {
labels[i] = label.Name
}
assignees := make([]string, len(raw.Assignees))
for i, assignee := range raw.Assignees {
assignees[i] = assignee.Login
}
return &Issue{
Number: raw.Number,
Title: raw.Title,
URL: raw.URL,
HTMLURL: raw.URL,
State: strings.ToLower(raw.State),
Body: raw.Body,
AuthorLogin: raw.Author.Login,
RepoOwner: owner,
RepoName: repo,
Labels: labels,
Assignees: assignees,
CreatedAt: raw.CreatedAt,
UpdatedAt: raw.UpdatedAt,
ClosedAt: parseTimePtr(raw.ClosedAt),
}
}
func convertGHRequestedReviewers(raw []ghRequestedReviewer) []RequestedReviewer {
reviewers := make([]RequestedReviewer, 0, len(raw))
for _, req := range raw {
@@ -18,6 +18,12 @@ type prKey struct {
Number int
}
type issueKey struct {
Owner string
Repo string
Number int
}
// branchKey is a composite key for PR lookups by owner/repo/branch.
type branchKey struct {
Owner string
@@ -70,6 +76,7 @@ type MockClient struct {
// the whole mock client out of the wiring.
reposUnavailable bool
prs map[prKey]*PR
issues map[issueKey]*Issue
prsByBranch map[branchKey]*PR
orgs []GitHubOrg
repos map[string][]GitHubRepo
@@ -115,6 +122,7 @@ func NewMockClient() *MockClient {
user: mockDefaultUser,
authenticated: true,
prs: make(map[prKey]*PR),
issues: make(map[issueKey]*Issue),
prsByBranch: make(map[branchKey]*PR),
repos: make(map[string][]GitHubRepo),
branches: make(map[repoKey][]RepoBranch),
@@ -152,6 +160,16 @@ func (m *MockClient) GetPR(_ context.Context, owner, repo string, number int) (*
return pr, nil
}
func (m *MockClient) GetIssue(_ context.Context, owner, repo string, number int) (*Issue, error) {
m.mu.RLock()
defer m.mu.RUnlock()
issue, ok := m.issues[issueKey{owner, repo, number}]
if !ok {
return nil, fmt.Errorf("mock: issue %s/%s#%d not found", owner, repo, number)
}
return issue, nil
}
func (m *MockClient) FindPRByBranch(_ context.Context, owner, repo, branch string) (*PR, error) {
m.findPRByBranchCalls.Add(1)
m.mu.RLock()
@@ -523,6 +541,12 @@ func (m *MockClient) AddPR(pr *PR) {
}
}
func (m *MockClient) AddIssue(issue *Issue) {
m.mu.Lock()
defer m.mu.Unlock()
m.issues[issueKey{issue.RepoOwner, issue.RepoName, issue.Number}] = issue
}
// AddOrgs appends organizations to the mock data store.
func (m *MockClient) AddOrgs(orgs []GitHubOrg) {
m.mu.Lock()
@@ -625,6 +649,7 @@ func (m *MockClient) Reset() {
m.authError = ""
m.reposUnavailable = false
m.prs = make(map[prKey]*PR)
m.issues = make(map[issueKey]*Issue)
m.prsByBranch = make(map[branchKey]*PR)
m.orgs = nil
m.repos = make(map[string][]GitHubRepo)
@@ -69,6 +69,37 @@ func TestMockClient_AddPR_GetPR(t *testing.T) {
}
}
func TestMockClient_AddIssueGetIssueAndReset(t *testing.T) {
m := NewMockClient()
ctx := context.Background()
_, err := m.GetIssue(ctx, "owner", "repo", 1456)
if err == nil || err.Error() != "mock: issue owner/repo#1456 not found" {
t.Fatalf("expected missing issue error, got %v", err)
}
m.AddIssue(&Issue{
Number: 1456,
Title: "Test issue",
RepoOwner: "owner",
RepoName: "repo",
})
issue, err := m.GetIssue(ctx, "owner", "repo", 1456)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if issue.Title != "Test issue" {
t.Fatalf("expected title 'Test issue', got %q", issue.Title)
}
m.Reset()
_, err = m.GetIssue(ctx, "owner", "repo", 1456)
if err == nil || err.Error() != "mock: issue owner/repo#1456 not found" {
t.Fatalf("expected reset issue error, got %v", err)
}
}
func TestMockClient_FindPRByBranch(t *testing.T) {
m := NewMockClient()
ctx := context.Background()
@@ -35,6 +35,7 @@ func (c *MockController) RegisterRoutes(router *gin.Engine) {
api := router.Group("/api/v1/github/mock")
api.PUT("/user", c.setUser)
api.POST("/prs", c.addPRs)
api.POST("/issues", c.addIssues)
api.POST("/orgs", c.addOrgs)
api.POST("/repos", c.addRepos)
api.POST("/reviews", c.addReviews)
@@ -111,6 +112,20 @@ func (c *MockController) addPRs(ctx *gin.Context) {
ctx.JSON(http.StatusOK, gin.H{"added": len(req.PRs)})
}
func (c *MockController) addIssues(ctx *gin.Context) {
var req struct {
Issues []Issue `json:"issues"`
}
if err := ctx.ShouldBindJSON(&req); err != nil {
ctx.JSON(http.StatusBadRequest, gin.H{"error": "invalid payload"})
return
}
for i := range req.Issues {
c.mock.AddIssue(&req.Issues[i])
}
ctx.JSON(http.StatusOK, gin.H{"added": len(req.Issues)})
}
func (c *MockController) addOrgs(ctx *gin.Context) {
var req struct {
Orgs []GitHubOrg `json:"orgs"`
@@ -1,11 +1,69 @@
package github
import (
"bytes"
"context"
"encoding/json"
"net/http"
"net/http/httptest"
"testing"
"time"
"github.com/gin-gonic/gin"
)
func setupMockControllerTestForAddIssues() (*gin.Engine, *MockClient) {
gin.SetMode(gin.TestMode)
mock := NewMockClient()
ctrl := NewMockController(mock, nil, nil, nil, newControllerTestLogger())
router := gin.New()
ctrl.RegisterRoutes(router)
return router, mock
}
func TestMockControllerAddIssues(t *testing.T) {
router, mock := setupMockControllerTestForAddIssues()
body := bytes.NewBufferString(`{"issues":[{"number":1456,"title":"Fix picker","repo_owner":"owner","repo_name":"repo"}]}`)
req := httptest.NewRequest(http.MethodPost, "/api/v1/github/mock/issues", body)
req.Header.Set("Content-Type", "application/json")
w := httptest.NewRecorder()
router.ServeHTTP(w, req)
if w.Code != http.StatusOK {
t.Fatalf("expected 200, got %d: %s", w.Code, w.Body.String())
}
var got struct {
Added int `json:"added"`
}
if err := json.NewDecoder(w.Body).Decode(&got); err != nil {
t.Fatalf("decode response: %v", err)
}
if got.Added != 1 {
t.Fatalf("expected added=1, got %d", got.Added)
}
issue, err := mock.GetIssue(context.Background(), "owner", "repo", 1456)
if err != nil {
t.Fatalf("get seeded issue: %v", err)
}
if issue.Title != "Fix picker" {
t.Fatalf("expected seeded issue title, got %q", issue.Title)
}
}
func TestMockControllerAddIssuesInvalidPayload(t *testing.T) {
router, _ := setupMockControllerTestForAddIssues()
req := httptest.NewRequest(http.MethodPost, "/api/v1/github/mock/issues", bytes.NewBufferString(`{`))
req.Header.Set("Content-Type", "application/json")
w := httptest.NewRecorder()
router.ServeHTTP(w, req)
if w.Code != http.StatusBadRequest {
t.Fatalf("expected 400, got %d: %s", w.Code, w.Body.String())
}
}
func TestEnsureMockPRForRequestCopiesMergeableState(t *testing.T) {
mock := NewMockClient()
controller := &MockController{mock: mock}
@@ -25,6 +25,10 @@ func (c *NoopClient) GetPR(context.Context, string, string, int) (*PR, error) {
return nil, ErrNoClient
}
func (c *NoopClient) GetIssue(context.Context, string, string, int) (*Issue, error) {
return nil, ErrNoClient
}
func (c *NoopClient) FindPRByBranch(context.Context, string, string, string) (*PR, error) {
return nil, ErrNoClient
}
@@ -165,6 +165,15 @@ func (c *PATClient) GetPR(ctx context.Context, owner, repo string, number int) (
return convertPatPR(&raw, owner, repo), nil
}
func (c *PATClient) GetIssue(ctx context.Context, owner, repo string, number int) (*Issue, error) {
var raw patIssue
endpoint := fmt.Sprintf("/repos/%s/%s/issues/%d", owner, repo, number)
if err := c.get(ctx, endpoint, &raw); err != nil {
return nil, fmt.Errorf("get issue #%d: %w", number, err)
}
return convertPatIssue(&raw, owner, repo), nil
}
func (c *PATClient) FindPRByBranch(ctx context.Context, owner, repo, branch string) (*PR, error) {
statuses, err := runBatchedBranchQuery(ctx, c, []graphQLBranchRef{{
Owner: owner,
@@ -905,6 +914,26 @@ type patPR struct {
} `json:"base"`
}
type patIssue struct {
Number int `json:"number"`
Title string `json:"title"`
HTMLURL string `json:"html_url"`
Body string `json:"body"`
State string `json:"state"`
CreatedAt time.Time `json:"created_at"`
UpdatedAt time.Time `json:"updated_at"`
ClosedAt *string `json:"closed_at"`
User struct {
Login string `json:"login"`
} `json:"user"`
Labels []struct {
Name string `json:"name"`
} `json:"labels"`
Assignees []struct {
Login string `json:"login"`
} `json:"assignees"`
}
type patSearchItem struct {
Number int `json:"number"`
Title string `json:"title"`
@@ -958,6 +987,36 @@ func convertPatPR(raw *patPR, owner, repo string) *PR {
return pr
}
func convertPatIssue(raw *patIssue, owner, repo string) *Issue {
labels := make([]string, len(raw.Labels))
for i, label := range raw.Labels {
labels[i] = label.Name
}
assignees := make([]string, len(raw.Assignees))
for i, assignee := range raw.Assignees {
assignees[i] = assignee.Login
}
issue := &Issue{
Number: raw.Number,
Title: raw.Title,
URL: raw.HTMLURL,
HTMLURL: raw.HTMLURL,
State: strings.ToLower(raw.State),
Body: raw.Body,
AuthorLogin: raw.User.Login,
RepoOwner: owner,
RepoName: repo,
Labels: labels,
Assignees: assignees,
CreatedAt: raw.CreatedAt,
UpdatedAt: raw.UpdatedAt,
}
if raw.ClosedAt != nil {
issue.ClosedAt = parseTimePtr(*raw.ClosedAt)
}
return issue
}
func convertPatRequestedReviewers(raw *patPR) []RequestedReviewer {
reviewers := make([]RequestedReviewer, 0, len(raw.RequestedReviewers)+len(raw.RequestedTeams))
for _, reviewer := range raw.RequestedReviewers {
+10 -1
View File
@@ -102,11 +102,20 @@ func pickDefaultMergeMethod(m RepoMergeMethods) string {
// GetPR fetches basic PR details from GitHub.
func (s *Service) GetPR(ctx context.Context, owner, repo string, number int) (*PR, error) {
if s.client == nil {
return nil, fmt.Errorf("github client not available")
return nil, ErrNoClient
}
return s.client.GetPR(ctx, owner, repo, number)
}
// GetIssue fetches basic issue details from GitHub. The create-task dialog is
// currently the only caller and dedupes requests per URL on the frontend.
func (s *Service) GetIssue(ctx context.Context, owner, repo string, number int) (*Issue, error) {
if s.client == nil {
return nil, ErrNoClient
}
return s.client.GetIssue(ctx, owner, repo, number)
}
// GetPRFeedback fetches live PR feedback from GitHub. Cached briefly with
// singleflight coalescing: the task page fires this for the same PR many times
// in quick succession (chat-bar CI chip, detail panel, hover popover — each on
+2 -2
View File
@@ -179,8 +179,8 @@ func TestGetPR_NilClient(t *testing.T) {
if err == nil {
t.Fatal("expected error when client is nil")
}
if !strings.Contains(err.Error(), "github client not available") {
t.Errorf("unexpected error: %v", err)
if !errors.Is(err, ErrNoClient) {
t.Errorf("err = %v, want ErrNoClient", err)
}
}
@@ -338,6 +338,25 @@ describe("RemoteRepoChip — picker loading state", () => {
});
describe("RemoteRepoChip — popover content", () => {
it("constrains the inline popover to fit inside the create-task dialog body", () => {
renderInProvider(
<RemoteRepoChip
row={row()}
branches={[]}
branchesLoading={false}
accessibleRepos={makeAccessible()}
onURLChange={vi.fn()}
onBranchChange={noopBranch}
onRemove={noopRemove}
/>,
);
fireEvent.click(screen.getByTestId(TRIGGER_TID));
const content = screen.getByTestId("remote-repo-popover-content");
expect(content.className).toContain("max-w-[calc(100vw-2rem)]");
expect(content.className).toContain("max-h-[min(420px,calc(100vh-12rem))]");
expect(content.className).toContain("overflow-y-auto");
});
it("renders the 'Connect GitHub' banner when accessibleRepos.unavailable=true", () => {
renderInProvider(
<RemoteRepoChip
@@ -216,7 +216,12 @@ function RemoteRepoPill({
<span className="truncate max-w-[240px]">{triggerLabel}</span>
</button>
</PopoverTrigger>
<PopoverContent className="w-[380px] p-0" align="start" portal={false}>
<PopoverContent
data-testid="remote-repo-popover-content"
className="w-[380px] max-w-[calc(100vw-2rem)] max-h-[min(420px,calc(100vh-12rem))] overflow-y-auto p-0"
align="start"
portal={false}
>
<RemoteRepoPopoverContent
accessible={accessibleRepos}
onPick={(repo) => {
@@ -410,7 +415,7 @@ function PasteSection({ onPaste }: { onPaste: (value: string) => void }) {
return (
<div className="flex flex-col gap-1 p-2">
<label className="text-[11px] text-muted-foreground" htmlFor="remote-paste-url-input">
…or paste a URL/PR
…or paste a URL/PR/issue
</label>
<input
id="remote-paste-url-input"
@@ -438,7 +443,7 @@ function PasteSection({ onPaste }: { onPaste: (value: string) => void }) {
commit();
}
}}
placeholder="github.com/owner/repo or .../pull/123"
placeholder="github.com/owner/repo, .../pull/123, or .../issues/123"
data-testid="remote-paste-url-input"
className={cn(
"h-8 rounded-md px-2 text-xs bg-muted/30 border border-border/60",
@@ -18,11 +18,17 @@ vi.mock("@/hooks/domains/github/use-branches-by-url", () => ({
// `usePRInfoByURL` also touches the network on ensure(); stub it to a
// per-test-controlled cache so the title-autofill effect can be exercised
// without an actual fetch. Each test that needs a specific PR-info value
// writes into `prInfoMap` before calling `setUseRemote(true)`.
// without an actual fetch. Each test that needs a specific GitHub URL info
// value writes into `prInfoMap` before calling `setUseRemote(true)`.
const prInfoMap = new Map<
string,
{ prHeadBranch: string; prBaseBranch: string; prNumber: number; suggestedTitle: string }
{
prHeadBranch?: string;
prBaseBranch?: string;
prNumber?: number;
issueNumber?: number;
suggestedTitle: string;
}
>();
vi.mock("@/hooks/domains/github/use-pr-info-by-url", async (importOriginal) => {
const original =
@@ -246,6 +252,9 @@ describe("buildRepositoriesPayload — remoteRepos rows", () => {
const PR_URL_42 = "https://github.com/acme/site/pull/42";
const PR_TITLE_42 = "PR #42: Test PR";
const USER_TYPED_TITLE = "my own title";
const ISSUE_URL_1456 = "https://github.com/acme/site/issues/1456";
const ISSUE_TITLE_1456 = "Issue #1456: Fix remote picker";
function seedPRInfo(url: string, prNumber: number, suggestedTitle: string) {
prInfoMap.set(url, {
@@ -256,7 +265,14 @@ function seedPRInfo(url: string, prNumber: number, suggestedTitle: string) {
});
}
describe("useDialogFormState — title autofill from first row PR info", () => {
function seedIssueInfo(url: string, issueNumber: number, suggestedTitle: string) {
prInfoMap.set(url, {
issueNumber,
suggestedTitle,
});
}
describe("useDialogFormState — title autofill from first row GitHub URL info", () => {
beforeEach(() => {
prInfoMap.clear();
});
@@ -279,14 +295,14 @@ describe("useDialogFormState — title autofill from first row PR info", () => {
seedPRInfo(PR_URL_42, 42, PR_TITLE_42);
const { result } = renderHook(() => useDialogFormState(true, "ws-1", null));
act(() => {
result.current.setTaskName("my own title");
result.current.setTaskName(USER_TYPED_TITLE);
result.current.setUseRemote(true);
});
const key = result.current.remoteRepos[0]?.key;
act(() => {
result.current.updateRemoteRepo(key!, { url: PR_URL_42 });
});
expect(result.current.taskName).toBe("my own title");
expect(result.current.taskName).toBe(USER_TYPED_TITLE);
});
it("does NOT re-apply autofill after the user clears the title (user took ownership)", () => {
@@ -362,3 +378,78 @@ describe("useDialogFormState — title autofill from first row PR info", () => {
expect(result.current.taskName).toBe("");
});
});
describe("useDialogFormState — title autofill from first row GitHub issue info", () => {
beforeEach(() => {
prInfoMap.clear();
});
it("seeds the task title from the first row's issue info when title is empty", () => {
seedIssueInfo(ISSUE_URL_1456, 1456, ISSUE_TITLE_1456);
const { result } = renderHook(() => useDialogFormState(true, "ws-1", null));
act(() => {
result.current.setUseRemote(true);
});
const key = result.current.remoteRepos[0]?.key;
act(() => {
result.current.updateRemoteRepo(key!, { url: ISSUE_URL_1456 });
});
expect(result.current.taskName).toBe(ISSUE_TITLE_1456);
expect(result.current.hasTitle).toBe(true);
});
it("does NOT overwrite a title the user typed themselves", () => {
seedIssueInfo(ISSUE_URL_1456, 1456, ISSUE_TITLE_1456);
const { result } = renderHook(() => useDialogFormState(true, "ws-1", null));
act(() => {
result.current.setTaskName(USER_TYPED_TITLE);
result.current.setUseRemote(true);
});
const key = result.current.remoteRepos[0]?.key;
act(() => {
result.current.updateRemoteRepo(key!, { url: ISSUE_URL_1456 });
});
expect(result.current.taskName).toBe(USER_TYPED_TITLE);
});
it("does NOT re-apply autofill after the user clears the title", () => {
seedIssueInfo(ISSUE_URL_1456, 1456, ISSUE_TITLE_1456);
const { result } = renderHook(() => useDialogFormState(true, "ws-1", null));
act(() => {
result.current.setUseRemote(true);
});
const key = result.current.remoteRepos[0]?.key;
act(() => {
result.current.updateRemoteRepo(key!, { url: ISSUE_URL_1456 });
});
expect(result.current.taskName).toBe(ISSUE_TITLE_1456);
act(() => {
result.current.setTaskName("");
});
expect(result.current.taskName).toBe("");
});
it("re-applies autofill when the user switches to a different issue URL", () => {
const newIssueURL = "https://github.com/acme/site/issues/1457";
seedIssueInfo(ISSUE_URL_1456, 1456, ISSUE_TITLE_1456);
seedIssueInfo(newIssueURL, 1457, "Issue #1457: Add issue URL paste");
const { result } = renderHook(() => useDialogFormState(true, "ws-1", null));
act(() => {
result.current.setUseRemote(true);
});
const key = result.current.remoteRepos[0]?.key;
act(() => {
result.current.updateRemoteRepo(key!, { url: ISSUE_URL_1456 });
});
expect(result.current.taskName).toBe(ISSUE_TITLE_1456);
act(() => {
result.current.setTaskName("");
});
expect(result.current.taskName).toBe("");
act(() => {
result.current.updateRemoteRepo(key!, { url: newIssueURL });
});
expect(result.current.taskName).toBe("Issue #1457: Add issue URL paste");
});
});
@@ -396,7 +396,7 @@ export function useDialogFormState(
// Title autofill: the first remote-repo row's PR info seeds the task title
// when the user hasn't typed one. Keyed only off row 0 — adding more PR
// rows later never overwrites the title.
useTitleAutofillFromFirstRowPRInfo({
useTitleAutofillFromFirstRowGitHubInfo({
open: open && ghUrl.useRemote,
firstRowUrl: remoteRepos.remoteRepos[0]?.url ?? "",
prInfoByUrl,
@@ -428,19 +428,19 @@ export function useDialogFormState(
}
/**
* Seeds the task title from the first Remote row's PR info (when present)
* Seeds the task title from the first Remote row's GitHub URL info (when present)
* the first time it arrives, and only when the user hasn't typed a title.
* Subsequent PR-info changes for the same URL don't overwrite — the
* lastAutoFilledRef tracks our own writes so re-pasting a different PR URL
* still works while a user-edited title is preserved.
* Subsequent info changes for the same URL don't overwrite — the
* lastAutoFilledRef tracks our own writes so re-pasting a different GitHub
* URL still works while a user-edited title is preserved.
*/
/** Sentinel stored in `lastAutoFilledRef` once the user clears an
* auto-filled title. Distinguishable from any real suggested title (which
* always begins with `"PR #"`) so the effect can refuse to re-apply autofill
* for the current URL until the URL itself changes. */
* comes from the GitHub info loader) so the effect can refuse to re-apply
* autofill for the current URL until the URL itself changes. */
const USER_CLEARED_SENTINEL = "\0cleared";
function useTitleAutofillFromFirstRowPRInfo(args: {
function useTitleAutofillFromFirstRowGitHubInfo(args: {
open: boolean;
firstRowUrl: string;
prInfoByUrl: ReturnType<typeof usePRInfoByURL>;
+21
View File
@@ -41,6 +41,23 @@ export type MockPR = {
requested_reviewers?: Array<{ login: string; type: string }>;
};
export type MockIssue = {
number: number;
title: string;
body?: string;
url?: string;
html_url?: string;
state?: string;
author_login?: string;
repo_owner: string;
repo_name: string;
labels?: string[];
assignees?: string[];
created_at?: string;
updated_at?: string;
closed_at?: string | null;
};
export type MockOrg = {
login: string;
avatar_url?: string;
@@ -902,6 +919,10 @@ export class ApiClient {
await this.request("POST", "/api/v1/github/mock/prs", { prs });
}
async mockGitHubAddIssues(issues: MockIssue[]): Promise<void> {
await this.request("POST", "/api/v1/github/mock/issues", { issues });
}
async mockGitHubAddOrgs(orgs: MockOrg[]): Promise<void> {
await this.request("POST", "/api/v1/github/mock/orgs", { orgs });
}
@@ -96,6 +96,16 @@ async function pasteUrlInChip(testPage: Page, url: string, chipIndex = 0): Promi
await pasteInput.press("Enter");
}
async function expectPopoverFitsDialog(testPage: Page): Promise<void> {
const dialogBox = await testPage.getByTestId("create-task-dialog").boundingBox();
const popoverBox = await testPage.getByTestId("remote-repo-popover-content").boundingBox();
expect(dialogBox).not.toBeNull();
expect(popoverBox).not.toBeNull();
expect(popoverBox!.y + popoverBox!.height).toBeLessThanOrEqual(
dialogBox!.y + dialogBox!.height + 1,
);
}
test.describe("Task creation from Remote tab (chip picker)", () => {
test.describe.configure({ retries: 1 });
@@ -200,6 +210,45 @@ test.describe("Task creation from Remote tab (chip picker)", () => {
expect(repoRow.provider_name).toBe("paste-repo");
});
test("pasted GitHub issue URL fills the title and keeps the picker inside the dialog", async ({
testPage,
apiClient,
}) => {
test.setTimeout(60_000);
await apiClient.mockGitHubAddBranches("issue-owner", "issue-repo", [{ name: "main" }]);
await apiClient.mockGitHubAddIssues([
{
number: 1456,
title: "Fix remote repo picker clipping",
body: "The picker overlaps the dialog footer.",
state: "open",
author_login: "mock-user",
repo_owner: "issue-owner",
repo_name: "issue-repo",
html_url: "https://github.com/issue-owner/issue-repo/issues/1456",
},
]);
const kanban = new KanbanPage(testPage);
await openCreateDialog(testPage, kanban);
await clickRemoteMode(testPage);
await testPage.getByTestId("remote-repo-chip-trigger").first().click();
await expectPopoverFitsDialog(testPage);
const pasteInput = testPage.getByTestId("remote-paste-url-input").last();
await pasteInput.fill("https://github.com/issue-owner/issue-repo/issues/1456");
await pasteInput.press("Enter");
await expect(testPage.getByTestId("task-title-input")).toHaveValue(
"Issue #1456: Fix remote repo picker clipping",
{ timeout: 10_000 },
);
await expect(testPage.getByTestId("remote-repo-chip-trigger").first()).toContainText(
"issues/1456",
{ timeout: 5_000 },
);
});
test("scenario 3: three mixed rows (picker + paste-repo + paste-PR)", async ({
testPage,
apiClient,
@@ -0,0 +1,55 @@
import { test, expect } from "../../fixtures/test-base";
import type { Page } from "@playwright/test";
import { MobileKanbanPage } from "../../pages/mobile-kanban-page";
async function openRemotePicker(testPage: Page): Promise<void> {
const mobile = new MobileKanbanPage(testPage);
await mobile.goto();
await mobile.mobileFab.click();
await expect(testPage.getByTestId("create-task-dialog")).toBeVisible();
await testPage.getByTestId("source-mode-remote").click();
await testPage.getByTestId("remote-repo-chip-trigger").first().click();
}
async function expectPopoverFitsViewport(testPage: Page): Promise<void> {
const viewport = testPage.viewportSize();
const box = await testPage.getByTestId("remote-repo-popover-content").boundingBox();
expect(viewport).not.toBeNull();
expect(box).not.toBeNull();
expect(box!.x).toBeGreaterThanOrEqual(0);
expect(box!.x + box!.width).toBeLessThanOrEqual(viewport!.width);
expect(box!.y + box!.height).toBeLessThanOrEqual(viewport!.height);
}
test.describe("Create task Remote repo picker on mobile", () => {
test.beforeEach(async ({ apiClient }) => {
await apiClient.mockGitHubReset();
});
test("pastes a GitHub issue URL without clipping the picker", async ({ testPage, apiClient }) => {
await apiClient.mockGitHubAddBranches("issue-owner", "issue-repo", [{ name: "main" }]);
await apiClient.mockGitHubAddIssues([
{
number: 1456,
title: "Fix remote repo picker clipping",
body: "The picker overlaps the dialog footer.",
state: "open",
author_login: "mock-user",
repo_owner: "issue-owner",
repo_name: "issue-repo",
html_url: "https://github.com/issue-owner/issue-repo/issues/1456",
},
]);
await openRemotePicker(testPage);
await expectPopoverFitsViewport(testPage);
const pasteInput = testPage.getByTestId("remote-paste-url-input").last();
await pasteInput.fill("https://github.com/issue-owner/issue-repo/issues/1456");
await pasteInput.press("Enter");
await expect(testPage.getByTestId("task-title-input")).toHaveValue(
"Issue #1456: Fix remote repo picker clipping",
{ timeout: 10_000 },
);
});
});
@@ -245,6 +245,8 @@ test.describe("@-mention autocomplete: adversarial QA", () => {
await kanban.createTaskButton.first().click();
await expect(testPage.getByTestId("create-task-dialog")).toBeVisible();
// Use scratch mode so submit does not depend on a pre-selected repository.
await testPage.getByTestId("source-mode-scratch").click();
await testPage.getByTestId("task-title-input").fill("qa-submit-task");
const textarea = testPage.getByTestId("task-description-input");
await textarea.click();
@@ -3,7 +3,7 @@ import type { SeedData } from "../../fixtures/test-base";
import type { ApiClient } from "../../helpers/api-client";
import { KanbanPage } from "../../pages/kanban-page";
import { SessionPage } from "../../pages/session-page";
import type { Page } from "@playwright/test";
import { TimeoutError, type Page } from "@playwright/test";
// ---------------------------------------------------------------------------
// Helpers shared across TUI passthrough tests
@@ -108,8 +108,13 @@ test.describe("Terminal agent (TUI passthrough)", () => {
const session = await openTaskSession(testPage, "TUI Test Task");
// Loading overlay is visible while the WebSocket connects
await session.waitForPassthroughLoading();
// The loading overlay is transient; on fast runs it may never be observable.
try {
await session.waitForPassthroughLoading();
} catch (error) {
// Keep tolerance for "didn't appear in time", but preserve real failures.
if (!(error instanceof TimeoutError)) throw error;
}
// Once connected, loading overlay disappears and terminal content is visible
await session.waitForPassthroughLoaded();
@@ -171,6 +171,23 @@ describe("useBranchesByURL — failure & invalidation", () => {
expect(fetchRepoBranchesMock).toHaveBeenCalledTimes(1);
});
it("accepts an issue URL and fetches branches against the underlying repo", async () => {
fetchRepoBranchesMock.mockImplementation((owner: string, repo: string) => {
expect(owner).toBe("acme");
expect(repo).toBe("site");
return Promise.resolve({ branches: [{ name: "main" }, { name: "fix/issue" }] });
});
const { result } = renderHook(() => useBranchesByURL());
const issueURL = "https://github.com/acme/site/issues/1456";
act(() => {
result.current.ensure(issueURL);
});
await waitFor(() => expect(result.current.branches(issueURL)).toHaveLength(2));
expect(fetchRepoBranchesMock).toHaveBeenCalledTimes(1);
});
it("clear(url) lets the next ensure() refetch a successfully loaded URL", async () => {
fetchRepoBranchesMock.mockResolvedValue({ branches: [{ name: "main" }] });
@@ -164,10 +164,9 @@ export function useBranchesByURL(): UseBranchesByURLResult {
const url = rawUrl.trim();
if (!url) return;
if (inFlightRef.current.has(url) || loadedRef.current.has(url)) return;
// Accept both plain repo URLs (`github.com/owner/repo`) and PR URLs
// (`github.com/owner/repo/pull/123`) — branches are listed against the
// repo in both cases, so we extract just `{ owner, repo }` and ignore
// the PR number. Using the repo-only parser here used to reject PR URLs
// Accept plain repo URLs plus PR/issue URLs — branches are listed against
// the repo in every case, so we extract just `{ owner, repo }` and ignore
// the item number. Using the repo-only parser here used to reject PR URLs
// outright, leaving the branch picker permanently empty when the user
// pasted a PR link into the Remote tab.
const parsed = parseGitHubAnyUrl(url);
@@ -2,22 +2,26 @@ import { afterEach, describe, expect, it, vi } from "vitest";
import { act, cleanup, renderHook, waitFor } from "@testing-library/react";
const fetchPRInfoMock = vi.fn();
const fetchIssueInfoMock = vi.fn();
vi.mock("@/lib/api/domains/github-api", () => ({
fetchPRInfo: (...args: unknown[]) => fetchPRInfoMock(...args),
fetchIssueInfo: (...args: unknown[]) => fetchIssueInfoMock(...args),
}));
// Import after mocks so the hook picks up the mocked module.
import { usePRInfoByURL, parseGitHubPrUrl } from "./use-pr-info-by-url";
import { usePRInfoByURL, parseGitHubIssueUrl, parseGitHubPrUrl } from "./use-pr-info-by-url";
afterEach(() => {
cleanup();
fetchPRInfoMock.mockReset();
fetchIssueInfoMock.mockReset();
vi.useRealTimers();
});
const PR_URL_A = "https://github.com/acme/site/pull/42";
const PR_URL_B = "https://github.com/acme/api/pull/7";
const ISSUE_URL_A = "https://github.com/acme/site/issues/1456";
const REPO_URL = "https://github.com/acme/site";
function makePR(overrides: { number: number; title?: string; head?: string; base?: string }) {
@@ -37,6 +41,25 @@ function makePR(overrides: { number: number; title?: string; head?: string; base
};
}
function makeIssue(overrides: { number: number; title?: string; body?: string }) {
return {
number: overrides.number,
title: overrides.title ?? "Test issue",
body: overrides.body ?? "Issue body",
url: "",
html_url: "",
state: "open" as const,
author_login: "octocat",
repo_owner: "acme",
repo_name: "site",
labels: [],
assignees: [],
created_at: "",
updated_at: "",
closed_at: null,
};
}
describe("parseGitHubPrUrl", () => {
it("returns owner/repo/prNumber for a canonical PR URL", () => {
expect(parseGitHubPrUrl(PR_URL_A)).toEqual({
@@ -59,6 +82,31 @@ describe("parseGitHubPrUrl", () => {
});
});
describe("parseGitHubIssueUrl", () => {
it("returns owner/repo/issueNumber for a canonical issue URL", () => {
expect(parseGitHubIssueUrl(ISSUE_URL_A)).toEqual({
owner: "acme",
repo: "site",
issueNumber: 1456,
});
});
it("tolerates trailing path/hash", () => {
expect(parseGitHubIssueUrl(`${ISSUE_URL_A}#issuecomment-1`)).toEqual({
owner: "acme",
repo: "site",
issueNumber: 1456,
});
});
it("returns null for PR URLs, plain repo URLs, invalid input, and empty input", () => {
expect(parseGitHubIssueUrl(PR_URL_A)).toBeNull();
expect(parseGitHubIssueUrl(REPO_URL)).toBeNull();
expect(parseGitHubIssueUrl("not a url")).toBeNull();
expect(parseGitHubIssueUrl("")).toBeNull();
});
});
describe("usePRInfoByURL", () => {
it("fetches PR info once per unique PR URL when ensure() is called", async () => {
fetchPRInfoMock.mockImplementation((_o: string, _r: string, n: number) => {
@@ -129,7 +177,9 @@ describe("usePRInfoByURL", () => {
await waitFor(() => expect(result.current.loading(PR_URL_A)).toBe(false));
expect(result.current.info(PR_URL_A)).toBeDefined();
});
});
describe("usePRInfoByURL — non-PR and issue URLs", () => {
it("no-ops (no fetch, no cached info) for a non-PR repo URL", async () => {
const { result } = renderHook(() => usePRInfoByURL());
@@ -149,6 +199,40 @@ describe("usePRInfoByURL", () => {
expect(fetchPRInfoMock).not.toHaveBeenCalled();
});
it("clear() is a no-op after ensure() records a non-GitHub URL as loaded", () => {
const url = "https://example.com/not-github";
const { result } = renderHook(() => usePRInfoByURL());
act(() => {
result.current.ensure(url);
result.current.clear(url);
});
expect(fetchPRInfoMock).not.toHaveBeenCalled();
expect(fetchIssueInfoMock).not.toHaveBeenCalled();
expect(result.current.info(url)).toBeUndefined();
expect(result.current.loading(url)).toBe(false);
});
it("fetches issue info and exposes a suggested title for GitHub issue URLs", async () => {
fetchIssueInfoMock.mockResolvedValue(makeIssue({ number: 1456, title: "Fix remote picker" }));
const { result } = renderHook(() => usePRInfoByURL());
act(() => {
result.current.ensure(ISSUE_URL_A);
});
await waitFor(() => expect(result.current.info(ISSUE_URL_A)).toBeDefined());
expect(fetchIssueInfoMock).toHaveBeenCalledTimes(1);
expect(fetchIssueInfoMock).toHaveBeenCalledWith("acme", "site", 1456, expect.any(Object));
expect(fetchPRInfoMock).not.toHaveBeenCalled();
expect(result.current.info(ISSUE_URL_A)).toMatchObject({
issueNumber: 1456,
suggestedTitle: "Issue #1456: Fix remote picker",
});
});
it("ignores ensure() with empty string", () => {
const { result } = renderHook(() => usePRInfoByURL());
act(() => {
@@ -1,7 +1,7 @@
"use client";
import { useCallback, useEffect, useRef, useState } from "react";
import { fetchPRInfo } from "@/lib/api/domains/github-api";
import { fetchIssueInfo, fetchPRInfo } from "@/lib/api/domains/github-api";
import { parseGitHubRepoUrl } from "@/lib/github/parse-url";
/**
@@ -27,9 +27,10 @@ import { parseGitHubRepoUrl } from "@/lib/github/parse-url";
*/
export type PRInfo = {
prHeadBranch: string;
prBaseBranch: string;
prNumber: number;
prHeadBranch?: string;
prBaseBranch?: string;
prNumber?: number;
issueNumber?: number;
suggestedTitle: string;
};
@@ -60,14 +61,30 @@ export function parseGitHubPrUrl(
return { owner: prMatch[1], repo: prMatch[2], prNumber: parseInt(prMatch[3], 10) };
}
/** Parse a GitHub URL as either a PR URL or a plain repo URL. Re-exported
/** Parse a GitHub URL and return the owner/repo/issueNumber when it's an
* issue URL. Returns null for PR URLs, plain repos, invalid input. */
export function parseGitHubIssueUrl(
url: string,
): { owner: string; repo: string; issueNumber: number } | null {
const trimmed = url.trim();
if (!trimmed) return null;
const issueMatch = trimmed.match(
/^(?:https?:\/\/)?(?:www\.)?github\.com\/([A-Za-z0-9_.-]+)\/([A-Za-z0-9_.-]+)\/issues\/(\d+)(?:[/?#].*)?$/,
);
if (!issueMatch) return null;
return { owner: issueMatch[1], repo: issueMatch[2], issueNumber: parseInt(issueMatch[3], 10) };
}
/** Parse a GitHub URL as a PR URL, issue URL, or plain repo URL. Re-exported
* so the legacy `parseGitHubUrl` shape (used elsewhere in the dialog code)
* has a single canonical implementation. */
export function parseGitHubAnyUrl(
url: string,
): { owner: string; repo: string; prNumber?: number } | null {
): { owner: string; repo: string; prNumber?: number; issueNumber?: number } | null {
const pr = parseGitHubPrUrl(url);
if (pr) return pr;
const issue = parseGitHubIssueUrl(url);
if (issue) return issue;
return parseGitHubRepoUrl(url);
}
@@ -84,6 +101,15 @@ type Refs = {
type SetState = React.Dispatch<React.SetStateAction<Record<string, URLState>>>;
type SuccessArgs<T> = {
refs: Refs;
setState: SetState;
url: string;
seq: number;
value: T;
buildInfo: (value: T) => PRInfo;
};
/** Marks the entry as loading=true (preserving any prior info) and bumps the
* per-URL sequence counter. */
function initRequest(
@@ -103,24 +129,13 @@ function initRequest(
return { seq, signal: controller.signal };
}
/** Writes the successful PR info when the request is still current. */
function handleSuccess(
refs: Refs,
setState: SetState,
url: string,
seq: number,
pr: Awaited<ReturnType<typeof fetchPRInfo>>,
): void {
/** Writes successful GitHub URL info when the request is still current. */
function handleSuccess<T>(args: SuccessArgs<T>): void {
const { refs, setState, url, seq, value, buildInfo } = args;
if (!refs.mountedRef.current) return;
if (refs.seqRef.current.get(url) !== seq) return;
refs.loadedRef.current.add(url);
const info: PRInfo = {
prHeadBranch: pr.head_branch,
prBaseBranch: pr.base_branch,
prNumber: pr.number,
suggestedTitle: `PR #${pr.number}: ${pr.title}`,
};
setState((prev) => ({ ...prev, [url]: { info, loading: false } }));
setState((prev) => ({ ...prev, [url]: { info: buildInfo(value), loading: false } }));
}
/** Marks loaded on failure (we don't want to retry in a tight loop) and
@@ -142,6 +157,57 @@ function finalizeRequest(refs: Refs, url: string, seq: number): void {
refs.abortersRef.current.delete(url);
}
function runGitHubInfoRequest(args: {
refs: Refs;
setState: SetState;
url: string;
seq: number;
signal: AbortSignal;
pr: NonNullable<ReturnType<typeof parseGitHubPrUrl>> | null;
issue: ReturnType<typeof parseGitHubIssueUrl>;
}): void {
const { refs, setState, url, seq, signal, pr, issue } = args;
let request: Promise<void>;
if (pr) {
request = fetchPRInfo(pr.owner, pr.repo, pr.prNumber, { init: { signal } }).then((res) =>
handleSuccess({
refs,
setState,
url,
seq,
value: res,
buildInfo: (value) => ({
prHeadBranch: value.head_branch,
prBaseBranch: value.base_branch,
prNumber: value.number,
suggestedTitle: `PR #${value.number}: ${value.title}`,
}),
}),
);
} else if (issue) {
request = fetchIssueInfo(issue.owner, issue.repo, issue.issueNumber, {
init: { signal },
}).then((res) =>
handleSuccess({
refs,
setState,
url,
seq,
value: res,
buildInfo: (value) => ({
issueNumber: value.number,
suggestedTitle: `Issue #${value.number}: ${value.title}`,
}),
}),
);
} else {
return;
}
request
.catch(() => handleFailure(refs, setState, url, seq))
.finally(() => finalizeRequest(refs, url, seq));
}
export function usePRInfoByURL(): UsePRInfoByURLResult {
const [state, setState] = useState<Record<string, URLState>>({});
const inFlightRef = useRef<Set<string>>(new Set());
@@ -184,8 +250,9 @@ export function usePRInfoByURL(): UsePRInfoByURLResult {
const url = rawUrl.trim();
if (!url) return;
if (inFlightRef.current.has(url) || loadedRef.current.has(url)) return;
const parsed = parseGitHubPrUrl(url);
if (!parsed) {
const pr = parseGitHubPrUrl(url);
const issue = pr ? null : parseGitHubIssueUrl(url);
if (!pr && !issue) {
// Non-PR URLs (plain repo, invalid) are recorded as "loaded with no
// info" so subsequent ensure() calls for the same URL no-op instead
// of re-parsing on every call.
@@ -194,10 +261,7 @@ export function usePRInfoByURL(): UsePRInfoByURLResult {
}
const refs = refsRef.current;
const { seq, signal } = initRequest(refs, setState, url);
fetchPRInfo(parsed.owner, parsed.repo, parsed.prNumber, { init: { signal } })
.then((pr) => handleSuccess(refs, setState, url, seq, pr))
.catch(() => handleFailure(refs, setState, url, seq))
.finally(() => finalizeRequest(refs, url, seq));
runGitHubInfoRequest({ refs, setState, url, seq, signal, pr, issue });
}, []);
const info = useCallback(
@@ -6,6 +6,7 @@ vi.mock("@/lib/config", () => ({
import {
fetchAccessibleRepos,
fetchIssueInfo,
getTaskCIAutomationOptions,
GitHubUnavailableError,
updateTaskCIAutomationOptions,
@@ -113,6 +114,20 @@ describe("fetchAccessibleRepos — URL & parsing", () => {
});
});
describe("fetchIssueInfo", () => {
it("builds the encoded issue info endpoint and forwards options", async () => {
fetchSpy.mockResolvedValueOnce(jsonResponse({ number: 1456, title: "Fix picker" }));
await fetchIssueInfo("acme org", "site/repo", 1456, { cache: "no-store" });
const call = fetchSpy.mock.calls.at(-1);
expect(String(call?.[0])).toBe(
"http://api.test/api/v1/github/issues/acme%20org/site%2Frepo/1456/info",
);
expect(call?.[1]).toMatchObject({ cache: "no-store" });
});
});
describe("fetchAccessibleRepos — errors & signal", () => {
it("throws GitHubUnavailableError on 503 with code: github_not_configured", async () => {
fetchSpy.mockResolvedValueOnce(
+14
View File
@@ -19,6 +19,7 @@ import type {
CreateIssueWatchRequest,
UpdateIssueWatchRequest,
TriggerIssueResponse,
GitHubIssue,
SearchPRsResponse,
SearchIssuesResponse,
GitHubPRStatus,
@@ -397,6 +398,19 @@ export async function fetchPRInfo(
);
}
// Issue info (lightweight)
export async function fetchIssueInfo(
owner: string,
repo: string,
number: number,
options?: ApiRequestOptions,
) {
return fetchJson<GitHubIssue>(
`/api/v1/github/issues/${encodeURIComponent(owner)}/${encodeURIComponent(repo)}/${number}/info`,
options,
);
}
// Remote repo branches
export async function fetchRepoBranches(owner: string, repo: string, options?: ApiRequestOptions) {
return fetchJson<{ branches: { name: string }[] }>(