diff --git a/packages/junior-dashboard/src/client/components/ConversationSummary.tsx b/packages/junior-dashboard/src/client/components/ConversationSummary.tsx index ecca4fa85..1cfa1e29b 100644 --- a/packages/junior-dashboard/src/client/components/ConversationSummary.tsx +++ b/packages/junior-dashboard/src/client/components/ConversationSummary.tsx @@ -8,13 +8,17 @@ import { slackLocationLabel, } from "../format"; import type { Conversation } from "../types"; +import { cn } from "../styles"; /** Render the shared conversation title and identity. */ export function ConversationSummary(props: { conversation: Conversation }) { return (
-
- {conversationDisplayTitle(props.conversation)} +
+
+ {conversationDisplayTitle(props.conversation)} +
+
@@ -23,6 +27,38 @@ export function ConversationSummary(props: { conversation: Conversation }) { ); } +function PullRequestBadge(props: { conversation: Conversation }) { + const pullRequest = props.conversation.pullRequest; + if (!pullRequest) return null; + const label = + pullRequest.status === "draft" + ? "PR draft" + : pullRequest.status === "open" + ? "PR ready" + : "PR merged"; + return ( + event.stopPropagation()} + onKeyDown={(event) => event.stopPropagation()} + rel="noreferrer" + target="_blank" + > + {label} + + ); +} + function ConversationIdentity(props: { conversation: Conversation }) { const email = props.conversation.actorIdentity?.email?.trim(); const owner = conversationActorLabel(props.conversation); diff --git a/packages/junior-dashboard/src/client/format.ts b/packages/junior-dashboard/src/client/format.ts index 5e3dfe253..fb8882f5f 100644 --- a/packages/junior-dashboard/src/client/format.ts +++ b/packages/junior-dashboard/src/client/format.ts @@ -856,6 +856,7 @@ export function buildConversations( lastProgressAt: summary.lastProgressAt, lastSeenAt: summary.lastSeenAt, locationId: summary.locationId, + pullRequest: summary.pullRequest, actorIdentity: summary.actorIdentity, sentryTraceUrl: summary.sentryTraceUrl, sourceUrl: summary.sourceUrl, diff --git a/packages/junior-dashboard/src/client/types.ts b/packages/junior-dashboard/src/client/types.ts index b0b682144..b7be1c1f8 100644 --- a/packages/junior-dashboard/src/client/types.ts +++ b/packages/junior-dashboard/src/client/types.ts @@ -138,6 +138,7 @@ export type Conversation = { lastProgressAt: string; lastSeenAt: string; locationId?: string; + pullRequest?: ConversationSummaryReport["pullRequest"]; actorIdentity?: ConversationSummaryReport["actorIdentity"]; sentryTraceUrl?: string; sourceUrl?: string; diff --git a/packages/junior-github/src/pull-request-outcomes/store.ts b/packages/junior-github/src/pull-request-outcomes/store.ts index 51669166c..ffdb8a4f3 100644 --- a/packages/junior-github/src/pull-request-outcomes/store.ts +++ b/packages/junior-github/src/pull-request-outcomes/store.ts @@ -14,6 +14,7 @@ const githubPullRequestOutcomeInputSchema = z candidateOwned: z.boolean(), closedAt: z.date().optional(), commitComposition: githubPullRequestCommitCompositionSchema.optional(), + draft: z.boolean(), mergedAt: z.date().optional(), number: z.number().int().positive(), openedAt: z.date(), diff --git a/packages/junior-github/src/webhooks/handler.ts b/packages/junior-github/src/webhooks/handler.ts index eb2aa91a7..97de62107 100644 --- a/packages/junior-github/src/webhooks/handler.ts +++ b/packages/junior-github/src/webhooks/handler.ts @@ -141,28 +141,56 @@ export function createGitHubWebhookRoute(args: { eventName === "pull_request" ? normalizeGitHubPullRequestLinkedIssues({ body, botEmail }) : undefined; - if (pullRequestOutcome) { - const recordedOutcome = await recordGitHubPullRequestOutcome( - args.db, - pullRequestOutcome, + const recordedOutcome = pullRequestOutcome + ? await recordGitHubPullRequestOutcome(args.db, pullRequestOutcome) + : undefined; + if (issueOutcome) { + await recordGitHubIssueOutcome(args.db, issueOutcome); + } + const recordedIssueConversations = issueConversations + ? await recordGitHubIssueConversations(args.db, issueConversations) + : false; + const recordedPullRequestConversations = pullRequestConversations + ? await recordGitHubPullRequestConversations( + args.db, + pullRequestConversations, + ) + : false; + const recordedPullRequestLinkedIssues = pullRequestLinkedIssues + ? await recordGitHubPullRequestLinkedIssues( + args.db, + pullRequestLinkedIssues, + ) + : false; + if (pullRequestOutcome && recordedOutcome?.applied) { + const conversationIds = [ + ...new Set([ + ...recordedOutcome.conversationIds, + ...(recordedPullRequestConversations && pullRequestConversations + ? pullRequestConversations.conversationIds + : []), + ]), + ]; + const status = + pullRequestOutcome.state === "merged" + ? "merged" + : pullRequestOutcome.state === "closed_unmerged" + ? "closed" + : pullRequestOutcome.draft + ? "draft" + : "open"; + await Promise.all( + conversationIds.map((conversationId) => + args.annotations.forConversation(conversationId).upsert({ + kind: "resource_link", + key: `${pullRequestOutcome.repositoryFullName.toLowerCase()}#${pullRequestOutcome.number}`, + label: `${pullRequestOutcome.repositoryFullName}#${pullRequestOutcome.number}`, + url: `https://github.com/${pullRequestOutcome.repositoryFullName}/pull/${pullRequestOutcome.number}`, + status, + }), + ), ); - if (recordedOutcome.applied && pullRequestOutcome.state !== "open") { - const status = - pullRequestOutcome.state === "merged" ? "merged" : "closed"; - await Promise.all( - recordedOutcome.conversationIds.map((conversationId) => - args.annotations.forConversation(conversationId).upsert({ - kind: "resource_link", - key: `${pullRequestOutcome.repositoryFullName.toLowerCase()}#${pullRequestOutcome.number}`, - label: `${pullRequestOutcome.repositoryFullName}#${pullRequestOutcome.number}`, - url: `https://github.com/${pullRequestOutcome.repositoryFullName}/pull/${pullRequestOutcome.number}`, - status, - }), - ), - ); - } if ( - recordedOutcome.applied && !recordedOutcome.commitComposition && pullRequestOutcome.state === "merged" && args.classifyPullRequestCommits @@ -189,24 +217,6 @@ export function createGitHubWebhookRoute(args: { } } } - if (issueOutcome) { - await recordGitHubIssueOutcome(args.db, issueOutcome); - } - const recordedIssueConversations = issueConversations - ? await recordGitHubIssueConversations(args.db, issueConversations) - : false; - const recordedPullRequestConversations = pullRequestConversations - ? await recordGitHubPullRequestConversations( - args.db, - pullRequestConversations, - ) - : false; - const recordedPullRequestLinkedIssues = pullRequestLinkedIssues - ? await recordGitHubPullRequestLinkedIssues( - args.db, - pullRequestLinkedIssues, - ) - : false; const failingChecks = eventName === "check_suite" && args.loadFailingChecks diff --git a/packages/junior-github/src/webhooks/pull-request-outcome.ts b/packages/junior-github/src/webhooks/pull-request-outcome.ts index ecfbbc2bc..17247cbbe 100644 --- a/packages/junior-github/src/webhooks/pull-request-outcome.ts +++ b/packages/junior-github/src/webhooks/pull-request-outcome.ts @@ -13,12 +13,19 @@ import { botLoginFromEmail } from "./ownership.js"; const canonicalPullRequestOutcomeSchema = z .object({ - action: z.enum(["opened", "closed", "reopened"]), + action: z.enum([ + "opened", + "closed", + "reopened", + "ready_for_review", + "converted_to_draft", + ]), pull_request: z .object({ body: z.string().nullable().optional(), closed_at: z.string().nullable().optional(), created_at: z.string(), + draft: z.boolean().default(false), id: z.number().int().positive(), merged: z.boolean(), merged_at: z.string().nullable().optional(), @@ -38,12 +45,19 @@ const canonicalPullRequestOutcomeSchema = z const pullRequestOutcomeSchema = z .object({ - action: z.enum(["opened", "closed", "reopened"]), + action: z.enum([ + "opened", + "closed", + "reopened", + "ready_for_review", + "converted_to_draft", + ]), pull_request: z .object({ body: z.string().nullable().optional(), closed_at: z.string().nullable().optional(), created_at: z.string(), + draft: z.boolean().default(false), id: z.number().int().positive(), merged: z.boolean(), merged_at: z.string().nullable().optional(), @@ -67,6 +81,7 @@ const pullRequestOutcomeSchema = z body: provider.pull_request.body, closed_at: provider.pull_request.closed_at, created_at: provider.pull_request.created_at, + draft: provider.pull_request.draft, id: provider.pull_request.id, merged: provider.pull_request.merged, merged_at: provider.pull_request.merged_at, @@ -140,7 +155,13 @@ export function normalizeGitHubPullRequestOutcome(args: { const lifecycle = pullRequestLifecycleActionSchema.safeParse(args.body); if ( !lifecycle.success || - !["opened", "closed", "reopened"].includes(lifecycle.data.action) + ![ + "opened", + "closed", + "reopened", + "ready_for_review", + "converted_to_draft", + ].includes(lifecycle.data.action) ) { return undefined; } @@ -183,6 +204,7 @@ export function normalizeGitHubPullRequestOutcome(args: { return { candidateOwned, closedAt, + draft: pullRequest.draft, mergedAt, number: pullRequest.number, openedAt, diff --git a/packages/junior/src/api/conversations/list.ts b/packages/junior/src/api/conversations/list.ts index e9782d358..c1f65aa57 100644 --- a/packages/junior/src/api/conversations/list.ts +++ b/packages/junior/src/api/conversations/list.ts @@ -18,6 +18,7 @@ import { conversationFeedSchema } from "../schema/conversation"; import type { ConversationFeed } from "../schema/conversation"; import { readRootConversationMetricsFromSql } from "./usage"; import { readConversationAuxiliaryCostsFromSql } from "./auxiliary-costs"; +import { listLatestConversationPullRequests } from "@/chat/plugins/annotations"; const CONVERSATION_FEED_LIMIT = 50; @@ -218,6 +219,7 @@ export async function readConversationFeedFromSql( accessByConversation, auxiliaryCostsByRoot, metricsByRoot, + pullRequestByConversation, teamDomainByTeamId, ] = await Promise.all([ readConversationAccessFromSql(db, conversationIds, options.viewer), @@ -225,6 +227,7 @@ export async function readConversationFeedFromSql( includeDescendants: true, }), readRootConversationMetricsFromSql(db, conversationIds), + listLatestConversationPullRequests(db, conversationIds), resolveSlackTeamDomains( conversations.flatMap((conversation) => conversation.sessionSource?.platform === "slack" @@ -242,6 +245,7 @@ export async function readConversationFeedFromSql( access: accessByConversation.get(conversation.conversationId), auxiliaryCosts: auxiliaryCostsByRoot.get(conversation.conversationId), durationMs: metrics?.durationMs ?? row.conversation.durationMs, + pullRequest: pullRequestByConversation.get(conversation.conversationId), teamDomainByTeamId, ...(row.destination?.visibility === "public" ? { locationId: row.destination.id } diff --git a/packages/junior/src/api/conversations/projection.ts b/packages/junior/src/api/conversations/projection.ts index bccedf8a9..a58a877e7 100644 --- a/packages/junior/src/api/conversations/projection.ts +++ b/packages/junior/src/api/conversations/projection.ts @@ -212,6 +212,7 @@ export function conversationSummaryFromStoredConversation(args: { conversation: ConversationProjectionSource; durationMs: number; locationId?: string; + pullRequest?: ConversationSummaryReport["pullRequest"]; teamDomainByTeamId?: ReadonlyMap; usage?: ConversationUsage; }): ConversationSummaryReport { @@ -265,6 +266,9 @@ export function conversationSummaryFromStoredConversation(args: { ...(args.auxiliaryCosts ? { auxiliaryCosts: args.auxiliaryCosts } : {}), ...(usage ? { cumulativeUsage: usage } : {}), ...(actorIdentity ? { actorIdentity } : {}), + ...(canViewPrivateContent && args.pullRequest + ? { pullRequest: args.pullRequest } + : {}), ...(sourceUrl ? { sourceUrl } : {}), ...(conversation.archivedAtMs ? { archivedAt: new Date(conversation.archivedAtMs).toISOString() } diff --git a/packages/junior/src/api/schema/conversation.ts b/packages/junior/src/api/schema/conversation.ts index c156a7e4a..9fa858a8c 100644 --- a/packages/junior/src/api/schema/conversation.ts +++ b/packages/junior/src/api/schema/conversation.ts @@ -202,6 +202,14 @@ export const conversationSummaryReportSchema = z channelName: z.string().optional(), channelNameRedacted: z.boolean().optional(), locationId: z.string().optional(), + pullRequest: z + .object({ + label: z.string(), + status: z.enum(["draft", "open", "merged"]), + url: z.string().url(), + }) + .strict() + .optional(), sentryTraceUrl: z.string().optional(), sourceUrl: z.string().url().optional(), traceId: z.string().optional(), diff --git a/packages/junior/src/chat/plugins/annotations.ts b/packages/junior/src/chat/plugins/annotations.ts index d921b6734..5416e15ae 100644 --- a/packages/junior/src/chat/plugins/annotations.ts +++ b/packages/junior/src/chat/plugins/annotations.ts @@ -1,4 +1,4 @@ -import { and, desc, eq } from "drizzle-orm"; +import { and, desc, eq, inArray } from "drizzle-orm"; import { conversationAnnotationInputSchema, type ConversationAnnotation, @@ -58,6 +58,54 @@ export function createPluginAnnotations(args: { }, }; } +export type ConversationPullRequest = { + label: string; + status: "draft" | "open" | "merged"; + url: string; +}; + +/** Return the newest GitHub pull request annotation for each conversation. */ +export async function listLatestConversationPullRequests( + db: JuniorDatabase, + conversationIds: readonly string[], +): Promise> { + if (conversationIds.length === 0) return new Map(); + const rows = await db + .select() + .from(juniorConversationAnnotations) + .where( + and( + inArray(juniorConversationAnnotations.conversationId, conversationIds), + eq(juniorConversationAnnotations.plugin, "github"), + eq(juniorConversationAnnotations.kind, "resource_link"), + ), + ) + .orderBy( + desc(juniorConversationAnnotations.createdAt), + desc(juniorConversationAnnotations.key), + ); + const latest = new Map(); + for (const row of rows) { + if (latest.has(row.conversationId)) continue; + const parsed = conversationAnnotationInputSchema.safeParse(row.annotation); + if ( + !parsed.success || + parsed.data.kind !== "resource_link" || + !parsed.data.url.startsWith("https://github.com/") || + !parsed.data.url.includes("/pull/") || + !["draft", "open", "merged"].includes(parsed.data.status ?? "") + ) { + continue; + } + latest.set(row.conversationId, { + label: parsed.data.label, + status: parsed.data.status as ConversationPullRequest["status"], + url: parsed.data.url, + }); + } + return latest; +} + export async function listConversationAnnotations( db: JuniorDatabase, conversationId: string, diff --git a/packages/junior/tests/integration/api/conversations/list.test.ts b/packages/junior/tests/integration/api/conversations/list.test.ts index 0ac0bc19b..2ed5da373 100644 --- a/packages/junior/tests/integration/api/conversations/list.test.ts +++ b/packages/junior/tests/integration/api/conversations/list.test.ts @@ -9,8 +9,10 @@ import { } from "@/api/conversations/list"; import { apiErrorSchema, conversationFeedSchema } from "@/api/schema"; import { migrateSchema } from "@/chat/conversations/sql/migrations"; +import { createPluginAnnotations } from "@/chat/plugins/annotations"; import { createSqlStore } from "@/chat/conversations/sql/store"; import { + juniorConversationAnnotations, juniorConversationEvents, juniorConversations, juniorDestinations, @@ -42,6 +44,150 @@ describe("conversation list API", () => { } }); + test("returns the newest linked pull request", async () => { + const fixture = createConfiguredJuniorSqlFixture(); + const store = createSqlStore(fixture.sql); + const conversationId = "slack:C123:pull-request"; + try { + await migrateSchema(fixture.sql); + await store.recordActivity({ + conversationId, + destination: { + platform: "slack", + teamId: "T123", + channelId: "C123", + }, + nowMs: 1_000, + source: "slack", + visibility: "public", + }); + const annotations = createPluginAnnotations({ + conversationId, + db: fixture.sql.db(), + plugin: "github", + }); + await annotations.upsert({ + kind: "resource_link", + key: "getsentry/junior#100", + label: "getsentry/junior#100", + status: "merged", + url: "https://github.com/getsentry/junior/pull/100", + }); + await fixture.sql + .db() + .update(juniorConversationAnnotations) + .set({ createdAt: new Date(1_000) }) + .where(eq(juniorConversationAnnotations.key, "getsentry/junior#100")); + await annotations.upsert({ + kind: "resource_link", + key: "getsentry/junior#101", + label: "getsentry/junior#101", + status: "draft", + url: "https://github.com/getsentry/junior/pull/101", + }); + + await expect(readConversationFeedFromSql()).resolves.toMatchObject({ + conversations: [ + expect.objectContaining({ + conversationId, + pullRequest: { + label: "getsentry/junior#101", + status: "draft", + url: "https://github.com/getsentry/junior/pull/101", + }, + }), + ], + }); + } finally { + await fixture.close(); + } + }); + + test("hides linked pull requests without private-content access", async () => { + const fixture = createConfiguredJuniorSqlFixture(); + const store = createSqlStore(fixture.sql); + const conversationId = "slack:C-private:pull-request"; + try { + await migrateSchema(fixture.sql); + await store.recordActivity({ + actor: { + email: "participant@example.com", + platform: "slack", + slackUserId: "U-participant", + teamId: "TPRIVATE", + }, + conversationId, + destination: { + platform: "slack", + teamId: "TPRIVATE", + channelId: "CPRIVATE", + }, + nowMs: 1_000, + source: "slack", + visibility: "private", + }); + const annotations = createPluginAnnotations({ + conversationId, + db: fixture.sql.db(), + plugin: "github", + }); + await annotations.upsert({ + kind: "resource_link", + key: "getsentry/junior#1081", + label: "getsentry/junior#1081", + status: "open", + url: "https://github.com/getsentry/junior/pull/1081", + }); + + const anonymous = await readConversationFeedFromSql(); + expect( + anonymous.conversations.find( + (conversation) => conversation.conversationId === conversationId, + ), + ).toMatchObject({ + conversationId, + }); + expect( + anonymous.conversations.find( + (conversation) => conversation.conversationId === conversationId, + ), + ).not.toHaveProperty("pullRequest"); + + const linkedUser = await fixture.sql + .db() + .select({ id: juniorUsers.id }) + .from(juniorUsers) + .where( + eq(juniorUsers.primaryEmailNormalized, "participant@example.com"), + ) + .limit(1); + const linkedUserId = linkedUser[0]?.id; + expect(linkedUserId).toBeDefined(); + + await expect( + readConversationFeedFromSql({ + viewer: { + ...testViewer("participant@example.com"), + id: linkedUserId!, + }, + }), + ).resolves.toMatchObject({ + conversations: [ + expect.objectContaining({ + conversationId, + pullRequest: { + label: "getsentry/junior#1081", + status: "open", + url: "https://github.com/getsentry/junior/pull/1081", + }, + }), + ], + }); + } finally { + await fixture.close(); + } + }); + test("returns a Slack source link for a viewable conversation", async () => { const fixture = createConfiguredJuniorSqlFixture(); const store = createSqlStore(fixture.sql);