diff --git a/cmd/e2e-server/main.go b/cmd/e2e-server/main.go index 10c4c1adf..98b551041 100644 --- a/cmd/e2e-server/main.go +++ b/cmd/e2e-server/main.go @@ -1131,6 +1131,7 @@ func buildAppState( caps: platform.Capabilities{ ReadRepositories: true, ReadIssues: true, + AssigneeMutation: true, }, repos: []platform.Repository{{ Ref: platform.RepoRef{ diff --git a/frontend/src/lib/components/detail/UserPicker.test.ts b/frontend/src/lib/components/detail/UserPicker.test.ts index e11cc7e45..642733d2b 100644 --- a/frontend/src/lib/components/detail/UserPicker.test.ts +++ b/frontend/src/lib/components/detail/UserPicker.test.ts @@ -55,6 +55,48 @@ describe("UserPicker", () => { expect(screen.getAllByRole("menuitemcheckbox")).toHaveLength(2); }); + it("renders synced profile images when an avatar URL is available", () => { + render(UserPicker, { + props: { + title: "Edit reviewers", + candidates: ["alice", "manual-user"], + selected: [], + avatarUrlForUser: (username) => (username === "alice" ? "https://github.example/alice.png?size=40" : ""), + ontoggle: vi.fn(), + onclose: vi.fn(), + }, + }); + + const aliceRow = screen.getByRole("menuitemcheckbox", { name: /alice/i }); + const aliceAvatar = aliceRow.querySelector("img.user-picker__avatar"); + expect(aliceAvatar?.getAttribute("src")).toBe("https://github.example/alice.png?size=40"); + expect(aliceAvatar?.getAttribute("alt")).toBe(""); + + expect(screen.getByRole("menuitemcheckbox", { name: /manual-user/i }).textContent).toContain("M"); + }); + + it("falls back to initials when a profile image fails to load", async () => { + render(UserPicker, { + props: { + title: "Edit reviewers", + candidates: ["roborev-ci"], + selected: [], + avatarUrlForUser: () => "https://github.example/roborev-ci.png?size=40", + ontoggle: vi.fn(), + onclose: vi.fn(), + }, + }); + + const row = screen.getByRole("menuitemcheckbox", { name: /roborev-ci/i }); + const avatar = row.querySelector("img.user-picker__avatar"); + expect(avatar).toBeTruthy(); + + await fireEvent.error(avatar as HTMLImageElement); + + expect(row.querySelector("img.user-picker__avatar")).toBeNull(); + expect(row.textContent).toContain("R"); + }); + it("notifies query changes so callers can fetch matching candidates", async () => { const onQuery = vi.fn(); render(UserPicker, { diff --git a/frontend/tests/e2e-full/assignees-reviewers.spec.ts b/frontend/tests/e2e-full/assignees-reviewers.spec.ts index 0531c43f4..f372539e3 100644 --- a/frontend/tests/e2e-full/assignees-reviewers.spec.ts +++ b/frontend/tests/e2e-full/assignees-reviewers.spec.ts @@ -1,10 +1,26 @@ -import { expect, test } from "@playwright/test"; -import { startIsolatedE2EServer, type IsolatedE2EServer } from "./support/e2eServer"; +import { expect, test, type Page } from "@playwright/test"; +import { startIsolatedE2EServer, startIsolatedE2EServerWithOptions, type IsolatedE2EServer } from "./support/e2eServer"; + +const transparentPixelPng = Buffer.from( + "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mNk+M9QDwADhgGAWjR9awAAAABJRU5ErkJggg==", + "base64", +); + +async function stubGitHubAvatarRequests(page: Page) { + await page.route("https://github.com/*.png?size=40", async (route) => { + await route.fulfill({ + status: 200, + contentType: "image/png", + body: transparentPixelPng, + }); + }); +} test.describe("assignee and reviewer editing", () => { test("pull detail edits assignees and persists across reload", async ({ page }) => { let isolatedServer: IsolatedE2EServer | null = null; try { + await stubGitHubAvatarRequests(page); isolatedServer = await startIsolatedE2EServer(); const baseURL = isolatedServer.info.base_url; @@ -15,6 +31,9 @@ test.describe("assignee and reviewer editing", () => { await page.getByRole("button", { name: "Edit assignees" }).click(); await expect(page.getByRole("dialog", { name: "Edit assignees" })).toBeVisible(); await expect(page.getByRole("menuitemcheckbox", { name: /alice/i })).toHaveAttribute("aria-checked", "true"); + await expect( + page.getByRole("menuitemcheckbox", { name: /alice/i }).locator("img.user-picker__avatar"), + ).toHaveAttribute("src", "https://github.com/alice.png?size=40"); await expect(page.getByRole("menuitemcheckbox", { name: /bob/i })).toHaveAttribute("aria-checked", "false"); // The picker is a compact dropdown: start-aligned under its @@ -50,6 +69,7 @@ test.describe("assignee and reviewer editing", () => { test("pull detail requests and removes reviewers", async ({ page }) => { let isolatedServer: IsolatedE2EServer | null = null; try { + await stubGitHubAvatarRequests(page); isolatedServer = await startIsolatedE2EServer(); const baseURL = isolatedServer.info.base_url; @@ -60,6 +80,9 @@ test.describe("assignee and reviewer editing", () => { await page.getByRole("button", { name: "Edit reviewers" }).click(); await expect(page.getByRole("dialog", { name: "Edit reviewers" })).toBeVisible(); await expect(page.getByRole("menuitemcheckbox", { name: /carol/i })).toHaveAttribute("aria-checked", "true"); + await expect( + page.getByRole("menuitemcheckbox", { name: /carol/i }).locator("img.user-picker__avatar"), + ).toHaveAttribute("src", "https://github.com/carol.png?size=40"); // Removing the only requested reviewer issues a PUT with an empty set. const removeResponse = page.waitForResponse( @@ -88,6 +111,7 @@ test.describe("assignee and reviewer editing", () => { test("pull detail searches candidates server-side and assigns a typed username", async ({ page }) => { let isolatedServer: IsolatedE2EServer | null = null; try { + await stubGitHubAvatarRequests(page); isolatedServer = await startIsolatedE2EServer(); const baseURL = isolatedServer.info.base_url; @@ -95,6 +119,9 @@ test.describe("assignee and reviewer editing", () => { await expect(page.locator(".pull-detail")).toBeVisible(); await page.getByRole("button", { name: "Edit assignees" }).click(); await expect(page.getByRole("dialog", { name: "Edit assignees" })).toBeVisible(); + await expect( + page.getByRole("menuitemcheckbox", { name: /alice/i }).locator("img.user-picker__avatar"), + ).toHaveAttribute("src", "https://github.com/alice.png?size=40"); // Typing requeries the autocomplete endpoint with the filter so // candidates beyond the first page stay reachable. @@ -273,4 +300,25 @@ test.describe("assignee and reviewer editing", () => { await isolatedServer?.stop(); } }); + + test("non-GitHub issue picker falls back to initials without provider avatar data", async ({ page }) => { + let isolatedServer: IsolatedE2EServer | null = null; + try { + isolatedServer = await startIsolatedE2EServerWithOptions({ providerCollision: true }); + const baseURL = isolatedServer.info.base_url; + + await page.goto(`${baseURL}/host/github.com/issues/gitea/acme/widgets/901`); + await expect(page.locator(".issue-detail")).toBeVisible(); + await page.getByRole("button", { name: "Edit assignees" }).click(); + await expect(page.getByRole("dialog", { name: "Edit assignees" })).toBeVisible(); + + const ginaRow = page.getByRole("menuitemcheckbox", { name: /gina/i }); + await expect(ginaRow).toHaveAttribute("aria-checked", "false"); + await expect(ginaRow.locator("img.user-picker__avatar")).toHaveCount(0); + await expect(ginaRow.locator(".user-picker__avatar")).toHaveText("G"); + await expect(page.getByRole("menuitemcheckbox", { name: /alice/i })).toHaveCount(0); + } finally { + await isolatedServer?.stop(); + } + }); }); diff --git a/internal/db/queries.go b/internal/db/queries.go index df85e7e6a..44aafa61b 100644 --- a/internal/db/queries.go +++ b/internal/db/queries.go @@ -141,6 +141,14 @@ func canonicalRepoLookupIdentifier(host, owner, name string) (string, string, st strings.ToLower(strings.TrimSpace(name)) } +func canonicalRepoPlatform(platform string) string { + platform = strings.ToLower(strings.TrimSpace(platform)) + if platform == "" { + return "github" + } + return platform +} + func canonicalRepoPathKey(path string) string { parts := strings.Split(strings.Trim(path, "/ "), "/") kept := parts[:0] @@ -217,10 +225,7 @@ func GitHubRepoIdentity(host, owner, name string) RepoIdentity { } func canonicalRepoIdentity(identity RepoIdentity) RepoIdentity { - identity.Platform = strings.ToLower(strings.TrimSpace(identity.Platform)) - if identity.Platform == "" { - identity.Platform = "github" - } + identity.Platform = canonicalRepoPlatform(identity.Platform) identity.PlatformHost = strings.ToLower(strings.TrimSpace(identity.PlatformHost)) if identity.PlatformHost == "" && identity.Platform == "github" { identity.PlatformHost = "github.com" @@ -3931,9 +3936,10 @@ func (d *DB) ListIssueEvents(ctx context.Context, issueID int64) ([]IssueEvent, // ListCommentAutocompleteUsers returns repo-scoped username suggestions for comment mentions. func (d *DB) ListCommentAutocompleteUsers( ctx context.Context, - platformHost, owner, name, query string, + platform, platformHost, owner, name, query string, limit int, ) ([]string, error) { + platform = canonicalRepoPlatform(platform) platformHost, owner, name = canonicalRepoLookupIdentifier(platformHost, owner, name) if limit <= 0 { limit = 10 @@ -3946,7 +3952,7 @@ func (d *DB) ListCommentAutocompleteUsers( WITH repo AS ( SELECT id FROM middleman_repos - WHERE platform_host = ? AND owner_key = ? AND name_key = ? + WHERE platform = ? AND platform_host = ? AND owner_key = ? AND name_key = ? ), candidates AS ( SELECT mr.author AS login, mr.last_activity_at AS last_seen FROM middleman_merge_requests mr @@ -3979,7 +3985,7 @@ func (d *DB) ListCommentAutocompleteUsers( last_seen DESC, login ASC LIMIT ?`, - platformHost, owner, name, + platform, platformHost, owner, name, query, containsQuery, query, prefixQuery, limit, @@ -4006,9 +4012,10 @@ func (d *DB) ListCommentAutocompleteUsers( // ListCommentAutocompleteReferences returns repo-scoped item reference suggestions. func (d *DB) ListCommentAutocompleteReferences( ctx context.Context, - platformHost, owner, name, query, itemKind string, + platform, platformHost, owner, name, query, itemKind string, limit int, ) ([]CommentAutocompleteReference, error) { + platform = canonicalRepoPlatform(platform) platformHost, owner, name = canonicalRepoLookupIdentifier(platformHost, owner, name) if limit <= 0 { limit = 10 @@ -4022,7 +4029,7 @@ func (d *DB) ListCommentAutocompleteReferences( WITH repo AS ( SELECT id FROM middleman_repos - WHERE platform_host = ? AND owner_key = ? AND name_key = ? + WHERE platform = ? AND platform_host = ? AND owner_key = ? AND name_key = ? ), candidates AS ( SELECT 'pull' AS kind, mr.number, mr.title, mr.state, mr.last_activity_at FROM middleman_merge_requests mr @@ -4046,7 +4053,7 @@ func (d *DB) ListCommentAutocompleteReferences( last_activity_at DESC, number DESC LIMIT ?`, - platformHost, owner, name, + platform, platformHost, owner, name, itemKind, itemKind, query, numberPrefix, titleQuery, query, numberPrefix, diff --git a/internal/db/queries_test.go b/internal/db/queries_test.go index 0b42c16c2..1d1dc86b6 100644 --- a/internal/db/queries_test.go +++ b/internal/db/queries_test.go @@ -957,11 +957,15 @@ func TestProviderCanonicalReadPathsUseLookupKeys(t *testing.T) { require.NotNil(refreshedIssue) assert.NotNil(refreshedIssue.DetailFetchedAt) - users, err := d.ListCommentAutocompleteUsers(ctx, "gitlab.example.com", "group/subgroup", "projectname", "auth", 10) + users, err := d.ListCommentAutocompleteUsers( + ctx, "gitlab", "gitlab.example.com", "group/subgroup", "projectname", "auth", 10, + ) require.NoError(err) assert.Equal([]string{"author"}, users) - refs, err := d.ListCommentAutocompleteReferences(ctx, "gitlab.example.com", "group/subgroup", "projectname", "GitLab", "", 10) + refs, err := d.ListCommentAutocompleteReferences( + ctx, "gitlab", "gitlab.example.com", "group/subgroup", "projectname", "GitLab", "", 10, + ) require.NoError(err) require.Len(refs, 2) assert.Equal([]int{8, 7}, []int{refs[0].Number, refs[1].Number}) @@ -3904,15 +3908,56 @@ func TestListCommentAutocompleteUsers(t *testing.T) { DedupeKey: "issue-comment-1", }})) - users, err := d.ListCommentAutocompleteUsers(ctx, "github.com", "acme", "widget", "al", 10) + users, err := d.ListCommentAutocompleteUsers(ctx, "github", "github.com", "acme", "widget", "al", 10) require.NoError(err) assert.Equal([]string{"alice", "albert", "alex"}, users) - users, err = d.ListCommentAutocompleteUsers(ctx, "github.com", "acme", "widget", "bert", 10) + users, err = d.ListCommentAutocompleteUsers(ctx, "github", "github.com", "acme", "widget", "bert", 10) require.NoError(err) assert.Equal([]string{"albert"}, users) } +func TestListCommentAutocompleteUsersScopesByProvider(t *testing.T) { + assert := Assert.New(t) + require := require.New(t) + d := openTestDB(t) + ctx := t.Context() + base := baseTime() + + githubRepoID := insertTestRepo(t, d, "acme", "widget") + giteaRepoID, err := d.UpsertRepo(ctx, RepoIdentity{ + Platform: "gitea", + PlatformHost: "github.com", + Owner: "acme", + Name: "widget", + RepoPath: "acme/widget", + }) + require.NoError(err) + + insertTestIssueWithOptions(t, d, testIssue( + githubRepoID, + 1, + withIssueTitle("GitHub collision issue"), + withIssueAuthor("alice"), + withIssueActivity(base.Add(time.Hour)), + )) + insertTestIssueWithOptions(t, d, testIssue( + giteaRepoID, + 901, + withIssueTitle("Gitea collision issue"), + withIssueAuthor("gina"), + withIssueActivity(base.Add(2*time.Hour)), + )) + + users, err := d.ListCommentAutocompleteUsers(ctx, "gitea", "github.com", "acme", "widget", "", 10) + require.NoError(err) + assert.Equal([]string{"gina"}, users) + + users, err = d.ListCommentAutocompleteUsers(ctx, "github", "github.com", "acme", "widget", "", 10) + require.NoError(err) + assert.Equal([]string{"alice"}, users) +} + func TestListCommentAutocompleteReferences(t *testing.T) { assert := Assert.New(t) require := require.New(t) @@ -3926,32 +3971,75 @@ func TestListCommentAutocompleteReferences(t *testing.T) { insertTestIssue(t, d, repoID, 17, "Mention bug", base.Add(2*time.Hour)) insertTestIssue(t, d, repoID, 101, "Numbered item", base.Add(time.Hour)) - refs, err := d.ListCommentAutocompleteReferences(ctx, "github.com", "acme", "widget", "1", "", 10) + refs, err := d.ListCommentAutocompleteReferences(ctx, "github", "github.com", "acme", "widget", "1", "", 10) require.NoError(err) require.Len(refs, 3) assert.Equal(CommentAutocompleteReference{Kind: "pull", Number: 12, Title: "Polish mentions", State: "open"}, refs[0]) assert.Equal(CommentAutocompleteReference{Kind: "issue", Number: 17, Title: "Mention bug", State: "open"}, refs[1]) assert.Equal(CommentAutocompleteReference{Kind: "issue", Number: 101, Title: "Numbered item", State: "open"}, refs[2]) - refs, err = d.ListCommentAutocompleteReferences(ctx, "github.com", "acme", "widget", "doc", "", 10) + refs, err = d.ListCommentAutocompleteReferences(ctx, "github", "github.com", "acme", "widget", "doc", "", 10) require.NoError(err) require.Len(refs, 1) assert.Equal(CommentAutocompleteReference{Kind: "pull", Number: 3, Title: "Add docs", State: "open"}, refs[0]) - refs, err = d.ListCommentAutocompleteReferences(ctx, "github.com", "acme", "widget", "1", "issue", 10) + refs, err = d.ListCommentAutocompleteReferences(ctx, "github", "github.com", "acme", "widget", "1", "issue", 10) require.NoError(err) assert.Equal([]CommentAutocompleteReference{ {Kind: "issue", Number: 17, Title: "Mention bug", State: "open"}, {Kind: "issue", Number: 101, Title: "Numbered item", State: "open"}, }, refs) - refs, err = d.ListCommentAutocompleteReferences(ctx, "github.com", "acme", "widget", "1", "pull", 10) + refs, err = d.ListCommentAutocompleteReferences(ctx, "github", "github.com", "acme", "widget", "1", "pull", 10) require.NoError(err) assert.Equal([]CommentAutocompleteReference{ {Kind: "pull", Number: 12, Title: "Polish mentions", State: "open"}, }, refs) } +func TestListCommentAutocompleteReferencesScopesByProvider(t *testing.T) { + assert := Assert.New(t) + require := require.New(t) + d := openTestDB(t) + ctx := t.Context() + base := baseTime() + + githubRepoID := insertTestRepo(t, d, "acme", "widget") + giteaRepoID, err := d.UpsertRepo(ctx, RepoIdentity{ + Platform: "gitea", + PlatformHost: "github.com", + Owner: "acme", + Name: "widget", + RepoPath: "acme/widget", + }) + require.NoError(err) + + insertTestIssueWithOptions(t, d, testIssue( + githubRepoID, + 1, + withIssueTitle("Provider collision issue"), + withIssueActivity(base.Add(time.Hour)), + )) + insertTestIssueWithOptions(t, d, testIssue( + giteaRepoID, + 901, + withIssueTitle("Provider collision issue"), + withIssueActivity(base.Add(2*time.Hour)), + )) + + refs, err := d.ListCommentAutocompleteReferences(ctx, "gitea", "github.com", "acme", "widget", "collision", "", 10) + require.NoError(err) + assert.Equal([]CommentAutocompleteReference{ + {Kind: "issue", Number: 901, Title: "Provider collision issue", State: "open"}, + }, refs) + + refs, err = d.ListCommentAutocompleteReferences(ctx, "github", "github.com", "acme", "widget", "collision", "", 10) + require.NoError(err) + assert.Equal([]CommentAutocompleteReference{ + {Kind: "issue", Number: 1, Title: "Provider collision issue", State: "open"}, + }, refs) +} + func TestWorktreeLinksCascadeOnMRDelete(t *testing.T) { require := require.New(t) d := openTestDB(t) diff --git a/internal/platform/gitlab/client.go b/internal/platform/gitlab/client.go index a499ae3b0..20d1bc452 100644 --- a/internal/platform/gitlab/client.go +++ b/internal/platform/gitlab/client.go @@ -28,6 +28,7 @@ type clientOptions struct { baseURL string foregroundTimeout time.Duration rateTracker *ratelimit.RateTracker + disableRetries bool } type Client struct { @@ -84,6 +85,12 @@ func WithRateTracker(rateTracker *ratelimit.RateTracker) ClientOption { } } +func WithoutRetriesForTesting() ClientOption { + return func(opts *clientOptions) { + opts.disableRetries = true + } +} + func NewClient(host string, source tokenauth.Source, options ...ClientOption) (*Client, error) { opts := clientOptions{ baseURL: "https://" + strings.TrimRight(host, "/") + "/api/v4", @@ -94,6 +101,9 @@ func NewClient(host string, source tokenauth.Source, options ...ClientOption) (* } clientOptions := []gitlab.ClientOptionFunc{gitlab.WithBaseURL(opts.baseURL)} + if opts.disableRetries { + clientOptions = append(clientOptions, gitlab.WithoutRetries()) + } baseTransport := http.DefaultTransport if opts.rateTracker != nil { baseTransport = &rateTrackingTransport{ diff --git a/internal/platform/gitlab/client_test.go b/internal/platform/gitlab/client_test.go index e7b9607ba..7059ab2ed 100644 --- a/internal/platform/gitlab/client_test.go +++ b/internal/platform/gitlab/client_test.go @@ -66,6 +66,27 @@ func TestClientLooksUpProjectByRawPathAndUsesNumericIDAfterLookup(t *testing.T) }, paths) } +func TestClientTestHelperDisablesRetries(t *testing.T) { + assert := assert.New(t) + require := require.New(t) + requests := 0 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + requests++ + http.Error(w, "rate limited", http.StatusTooManyRequests) + })) + defer server.Close() + + client := newTestClient(t, server.URL) + _, err := client.GetRepository(context.Background(), platform.RepoRef{ + Platform: platform.KindGitLab, + Host: "gitlab.example.com", + RepoPath: "group/project", + }) + + require.Error(err) + assert.Equal(1, requests) +} + func TestClientListOpenMergeRequestsPopulatesForkHeadRepoCloneURL(t *testing.T) { assert := assert.New(t) require := require.New(t) @@ -717,7 +738,10 @@ func TestSelfHostedBaseURLConstruction(t *testing.T) { func newTestClient(t *testing.T, serverURL string, opts ...ClientOption) *Client { t.Helper() - allOpts := append([]ClientOption{WithBaseURLForTesting(serverURL + "/api/v4")}, opts...) + allOpts := append([]ClientOption{ + WithBaseURLForTesting(serverURL + "/api/v4"), + WithoutRetriesForTesting(), + }, opts...) client, err := NewClient("gitlab.example.com", testTokenSource("token"), allOpts...) require.NoError(t, err) return client diff --git a/internal/server/api_test.go b/internal/server/api_test.go index 00a6d7285..f055ffba3 100644 --- a/internal/server/api_test.go +++ b/internal/server/api_test.go @@ -6270,6 +6270,138 @@ func TestAPICommentAutocompleteUsesRepoPlatformHost(t *testing.T) { assert.Equal([]db.CommentAutocompleteReference{{Kind: "pull", Number: 12, Title: "Polish mentions", State: "open"}}, body.References) } +func TestAPICommentAutocompleteReferencesScopesByProvider(t *testing.T) { + assert := Assert.New(t) + require := require.New(t) + srv, database := setupTestServer(t) + ctx := t.Context() + now := time.Now().UTC().Truncate(time.Second) + + githubRepoID, err := database.UpsertRepo(ctx, db.RepoIdentity{ + Platform: "github", + PlatformHost: "github.com", + Owner: "acme", + Name: "widget", + RepoPath: "acme/widget", + }) + require.NoError(err) + giteaRepoID, err := database.UpsertRepo(ctx, db.RepoIdentity{ + Platform: "gitea", + PlatformHost: "github.com", + Owner: "acme", + Name: "widget", + RepoPath: "acme/widget", + }) + require.NoError(err) + + _, err = database.UpsertIssue(ctx, &db.Issue{ + RepoID: githubRepoID, + PlatformID: 17001, + Number: 1, + URL: "https://github.com/acme/widget/issues/1", + Title: "Provider collision issue", + Author: "alice", + State: "open", + CreatedAt: now.Add(-2 * time.Hour), + UpdatedAt: now.Add(-2 * time.Hour), + LastActivityAt: now.Add(-2 * time.Hour), + }) + require.NoError(err) + _, err = database.UpsertIssue(ctx, &db.Issue{ + RepoID: giteaRepoID, + PlatformID: 17901, + Number: 901, + URL: "https://github.com/acme/widget/issues/901", + Title: "Provider collision issue", + Author: "gina", + State: "open", + CreatedAt: now.Add(-time.Hour), + UpdatedAt: now.Add(-time.Hour), + LastActivityAt: now.Add(-time.Hour), + }) + require.NoError(err) + + req := httptest.NewRequest(http.MethodGet, "/api/v1/host/github.com/repo/gitea/acme/widget/comment-autocomplete?trigger=%23&q=collision&limit=10", nil) + rr := httptest.NewRecorder() + srv.ServeHTTP(rr, req) + require.Equal(http.StatusOK, rr.Code, rr.Body.String()) + + var body commentAutocompleteResponse + require.NoError(json.NewDecoder(rr.Body).Decode(&body)) + assert.Equal([]db.CommentAutocompleteReference{ + {Kind: "issue", Number: 901, Title: "Provider collision issue", State: "open"}, + }, body.References) + assert.Empty(body.Users) +} + +func TestAPICommentAutocompleteGitLabMergeRequestReferencesScopesByProvider(t *testing.T) { + assert := Assert.New(t) + require := require.New(t) + srv, database := setupTestServer(t) + ctx := t.Context() + now := time.Now().UTC().Truncate(time.Second) + + giteaRepoID, err := database.UpsertRepo(ctx, db.RepoIdentity{ + Platform: "gitea", + PlatformHost: "gitlab.example.com", + Owner: "acme", + Name: "widget", + RepoPath: "acme/widget", + }) + require.NoError(err) + gitlabRepoID, err := database.UpsertRepo(ctx, db.RepoIdentity{ + Platform: "gitlab", + PlatformHost: "gitlab.example.com", + Owner: "acme", + Name: "widget", + RepoPath: "acme/widget", + }) + require.NoError(err) + + _, err = database.UpsertMergeRequest(ctx, &db.MergeRequest{ + RepoID: giteaRepoID, + PlatformID: 12001, + Number: 1, + URL: "https://gitlab.example.com/acme/widget/pulls/1", + Title: "Provider collision merge request", + Author: "gina", + State: "open", + HeadBranch: "feature-gitea", + BaseBranch: "main", + CreatedAt: now.Add(-2 * time.Hour), + UpdatedAt: now.Add(-2 * time.Hour), + LastActivityAt: now.Add(-2 * time.Hour), + }) + require.NoError(err) + _, err = database.UpsertMergeRequest(ctx, &db.MergeRequest{ + RepoID: gitlabRepoID, + PlatformID: 12901, + Number: 901, + URL: "https://gitlab.example.com/acme/widget/-/merge_requests/901", + Title: "Provider collision merge request", + Author: "glenda", + State: "open", + HeadBranch: "feature-gitlab", + BaseBranch: "main", + CreatedAt: now.Add(-time.Hour), + UpdatedAt: now.Add(-time.Hour), + LastActivityAt: now.Add(-time.Hour), + }) + require.NoError(err) + + req := httptest.NewRequest(http.MethodGet, "/api/v1/host/gitlab.example.com/repo/gitlab/acme/widget/comment-autocomplete?trigger=!&q=collision&limit=10", nil) + rr := httptest.NewRecorder() + srv.ServeHTTP(rr, req) + require.Equal(http.StatusOK, rr.Code, rr.Body.String()) + + var body commentAutocompleteResponse + require.NoError(json.NewDecoder(rr.Body).Decode(&body)) + assert.Equal([]db.CommentAutocompleteReference{ + {Kind: "pull", Number: 901, Title: "Provider collision merge request", State: "open"}, + }, body.References) + assert.Empty(body.Users) +} + func TestAPISyncStatus(t *testing.T) { require := require.New(t) @@ -24140,20 +24272,11 @@ func TestWorkspaceRuntimePlainShellTerminalWebSocketE2E(t *testing.T) { require.Contains(got.String(), "echo:ping") } -// TestWorkspaceRuntimeShellTerminalDeliversExitFrameE2E pins the -// websocket "exited" text frame contract. The frontend's ShellDrawer -// only fires onExit when this frame arrives; without it the drawer -// doesn't auto-close on shell exit and the user is stranded on a dead -// session (TerminalPane reconnect-loops on a still-listed-but- -// output-dead session, which looks like a hang). -// -// Uses the "pty-close-on-input-then-sleep" helper on the pty-owner backend to -// deterministically open the race window where drainOutput's PTY EOF precedes -// watchSession's cmd.Wait return by hundreds of milliseconds. Without the -// bridge fix (always send exit frame on outputDone), this test fails because -// the 100ms timeout fires before attachment.Done — exactly the systemd- -// run-wrapped shell case the user hit. Real tmux does not expose that signal: -// while the pane process is still sleeping, the attach client remains open. +// TestWorkspaceRuntimePlainShellTerminalDeliversExitFrameE2E pins the +// websocket "exited" text frame contract through the real runtime stack. The +// frontend's ShellDrawer only fires onExit when this frame arrives; without it +// the drawer doesn't auto-close on shell exit and the user is stranded on a +// dead session. func TestWorkspaceRuntimePlainShellTerminalDeliversExitFrameE2E(t *testing.T) { if runtime.GOOS == "windows" { t.Skip("test fixture uses /bin/sh") @@ -24164,7 +24287,9 @@ func TestWorkspaceRuntimePlainShellTerminalDeliversExitFrameE2E(t *testing.T) { dir := t.TempDir() ptyOwnerDir := filepath.Join(dir, "pty-owner") cfg := &config.Config{ - Tmux: config.Tmux{Command: []string{filepath.Join(dir, "missing-tmux")}}, + Tmux: config.Tmux{ + Command: []string{filepath.Join(dir, "missing-tmux")}, + }, Shell: config.Shell{ Command: serverRuntimeHelperCommand("pty-close-on-input-then-sleep"), }, @@ -24172,13 +24297,15 @@ func TestWorkspaceRuntimePlainShellTerminalDeliversExitFrameE2E(t *testing.T) { fixture := setupWorkspaceServerFixtureWithOptions( t, cfg, ptyOwnerServerOptions(ptyOwnerDir), ) + client := fixture.client + srv := fixture.server ctx := context.Background() - ws := createReadyWorkspace(t, ctx, fixture.client) + ws := createReadyWorkspace(t, ctx, client) - shell := launchPlainShellRuntimeSession(t, ctx, fixture.client, ws.Id) + shell := launchPlainShellRuntimeSession(t, ctx, client, ws.Id) cleanupPtyOwnerWorkspace(t, ptyOwnerDir, shell.Key) - ts := httptest.NewServer(fixture.server) + ts := httptest.NewServer(srv) t.Cleanup(ts.Close) wsURL := "ws" + strings.TrimPrefix(ts.URL, "http") + "/ws/v1/workspaces/" + ws.Id + @@ -24186,20 +24313,13 @@ func TestWorkspaceRuntimePlainShellTerminalDeliversExitFrameE2E(t *testing.T) { conn := dialWebSocketForTest(t, ctx, wsURL, "shell") defer conn.Close(websocket.StatusNormalClosure, "done") - // Trigger after attach so the session cannot naturally exit before - // the websocket connects. The helper then closes its PTY end and - // sleeps long enough before process exit that a regression which gates - // the exit frame on cmd.Wait would push delivery well past our - // promptness budget. The bridge's outputDone path must deliver the - // frame within a few hundred ms of PTY EOF; cmd.Wait can only return - // when the helper finishes its sleep, so the gap between "right" and - // "wrong" is the helper's sleep duration. Helper sleeps 2 s; we allow - // up to 800 ms for the PTY-EOF path even on slow CI. + // Trigger after attach so the session cannot naturally exit before the + // websocket connects. The focused bridge test below covers the fast + // outputDone-before-Done race directly; this e2e keeps the real runtime + // contract pinned without depending on backend-specific PTY EOF timing. require.NoError(conn.Write(ctx, websocket.MessageBinary, []byte("close\n"))) readCtx, cancel := context.WithTimeout(ctx, 5*time.Second) defer cancel() - const exitFrameBudget = 800 * time.Millisecond - attachStart := time.Now() for { typ, data, readErr := conn.Read(readCtx) if readErr != nil { @@ -24211,14 +24331,6 @@ func TestWorkspaceRuntimePlainShellTerminalDeliversExitFrameE2E(t *testing.T) { if typ != websocket.MessageText { continue } - elapsed := time.Since(attachStart) - require.Lessf(elapsed, exitFrameBudget, - "exit frame took %s after attach; bridge appears "+ - "gated on cmd.Wait rather than PTY EOF (helper "+ - "sleeps 2 s, so a regression would clock in "+ - "around there)", - elapsed, - ) var msg struct { Type string `json:"type"` Code int `json:"code"` @@ -24229,13 +24341,82 @@ func TestWorkspaceRuntimePlainShellTerminalDeliversExitFrameE2E(t *testing.T) { // reads the snapshot) or -1 (writeRuntimeExit read snapshot // before watchSession populated ExitCode). Both are "the // session ended" signals the frontend treats identically; - // pinning a specific value would be timing-dependent. Reject + // pinning a specific value would be timing-dependent. require.NotEqual(0, msg.Code, "non-success exit must report non-zero (or -1)") return } } +// TestBridgeRuntimeAttachmentOutputClosedEmitsExitFrameBeforeDone pins the +// bridge branch for wrappers where PTY EOF reaches the subscriber before +// cmd.Wait marks the session done. That is the race the real ShellDrawer cares +// about, but it is cleaner to drive it with controlled channels than to depend +// on backend-specific PTY behavior in an e2e helper. +func TestBridgeRuntimeAttachmentOutputClosedEmitsExitFrameBeforeDone(t *testing.T) { + require := require.New(t) + closedOutput := make(chan []byte) + close(closedOutput) + stillWaiting := make(chan struct{}) + exitCode := 7 + attach := localruntime.NewAttachmentForTesting( + localruntime.AttachmentForTestingOptions{ + Output: closedOutput, + Done: stillWaiting, + Info: func() localruntime.SessionInfo { + return localruntime.SessionInfo{ExitCode: &exitCode} + }, + SessionOutputClosed: func() bool { return true }, + }, + ) + + bridgeReturn := make(chan bool, 1) + acceptErr := make(chan error, 1) + srv := httptest.NewServer(http.HandlerFunc( + func(w http.ResponseWriter, r *http.Request) { + conn, err := websocket.Accept(w, r, &websocket.AcceptOptions{ + InsecureSkipVerify: true, + }) + if err != nil { + acceptErr <- err + return + } + exited := bridgeRuntimeAttachment(r.Context(), conn, attach) + bridgeReturn <- exited + conn.Close(websocket.StatusNormalClosure, "test done") + }, + )) + t.Cleanup(srv.Close) + + ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) + defer cancel() + wsURL := "ws" + strings.TrimPrefix(srv.URL, "http") + conn, _, err := websocket.Dial(ctx, wsURL, nil) + require.NoError(err) + defer conn.Close(websocket.StatusNormalClosure, "done") + + typ, data, err := conn.Read(ctx) + require.NoError(err) + require.Equal(websocket.MessageText, typ) + var msg struct { + Type string `json:"type"` + Code int `json:"code"` + } + require.NoError(json.Unmarshal(data, &msg)) + require.Equal("exited", msg.Type) + require.Equal(7, msg.Code) + + select { + case exited := <-bridgeReturn: + require.True(exited, + "bridge must report exited when session output closed") + case err := <-acceptErr: + require.NoError(err, "websocket accept failed") + case <-time.After(2 * time.Second): + require.Fail("bridge did not return") + } +} + // TestWorkspaceRuntimePlainShellAfterExitStartsFreshE2E pins the user-visible // no-tmux behavior that launching a shell after the terminal has reported exit // returns a fresh ptyowner-backed session, never the dead shell record the diff --git a/internal/server/huma_routes.go b/internal/server/huma_routes.go index 34537478d..2c70e97b5 100644 --- a/internal/server/huma_routes.go +++ b/internal/server/huma_routes.go @@ -2642,6 +2642,7 @@ func (s *Server) getCommentAutocomplete( case "@": users, err := s.db.ListCommentAutocompleteUsers( ctx, + repo.Platform, repo.PlatformHost, input.Owner, input.Name, @@ -2669,6 +2670,7 @@ func (s *Server) getCommentAutocomplete( } references, err := s.db.ListCommentAutocompleteReferences( ctx, + repo.Platform, repo.PlatformHost, input.Owner, input.Name, diff --git a/packages/ui/src/components/detail/IssueDetail.svelte b/packages/ui/src/components/detail/IssueDetail.svelte index 8df7866bd..b7acd604d 100644 --- a/packages/ui/src/components/detail/IssueDetail.svelte +++ b/packages/ui/src/components/detail/IssueDetail.svelte @@ -1,6 +1,6 @@