diff --git a/packages/agent/src/agent.ts b/packages/agent/src/agent.ts index 28d26d627c..69ae25c34c 100644 --- a/packages/agent/src/agent.ts +++ b/packages/agent/src/agent.ts @@ -1,3 +1,4 @@ +import { buildPrOutput, mergePrUrls, readPrUrls } from "@posthog/shared"; import { createAcpConnection, type InProcessAcpConnection, @@ -167,8 +168,13 @@ export class Agent { throw error; } + const freshOutput = await this.posthogAPI + .getTaskRun(taskId, this.taskRunId) + .then((run) => run.output) + .catch(() => null); + const urls = mergePrUrls(readPrUrls(freshOutput), [prUrl]); const updates: TaskRunUpdate = { - output: { pr_url: prUrl }, + output: buildPrOutput(freshOutput, urls), }; if (branchName) { updates.branch = branchName; diff --git a/packages/agent/src/pr-url-detector.test.ts b/packages/agent/src/pr-url-detector.test.ts index 9478ea733a..90ca498f45 100644 --- a/packages/agent/src/pr-url-detector.test.ts +++ b/packages/agent/src/pr-url-detector.test.ts @@ -1,5 +1,10 @@ import { describe, expect, it } from "vitest"; -import { findPrUrl, wasCreatedRecently } from "./pr-url-detector"; +import { + findPrUrl, + findPrUrls, + wasCreatedByLogin, + wasCreatedRecently, +} from "./pr-url-detector"; const PR_URL = "https://github.com/PostHog/posthog.com/pull/17764"; @@ -33,6 +38,38 @@ describe("findPrUrl", () => { }); }); +describe("findPrUrls", () => { + const OTHER = "https://github.com/PostHog/posthog/pull/99"; + + it("finds every PR URL in one chunk, in order", () => { + expect(findPrUrls(`Opened ${PR_URL} and ${OTHER} today`)).toEqual([ + PR_URL, + OTHER, + ]); + }); + + it("dedupes repeated mentions of the same PR", () => { + expect(findPrUrls(`${PR_URL} again: ${PR_URL}`)).toEqual([PR_URL]); + }); + + it("returns an empty array when there is no PR URL", () => { + expect(findPrUrls("nothing here")).toEqual([]); + }); +}); + +describe("wasCreatedByLogin", () => { + it.each([ + ["run-owner", "run-owner", true], + ["Run-Owner", "run-owner", true], + ["someone-else", "run-owner", false], + [null, "run-owner", false], + ["run-owner", null, false], + ["", "", false], + ] as const)("author=%s login=%s -> %s", (author, login, expected) => { + expect(wasCreatedByLogin(author, login)).toBe(expected); + }); +}); + describe("wasCreatedRecently", () => { const now = new Date("2026-06-18T17:00:00Z").getTime(); const maxAge = 15 * 60 * 1000; diff --git a/packages/agent/src/pr-url-detector.ts b/packages/agent/src/pr-url-detector.ts index af052a8465..621c1d415a 100644 --- a/packages/agent/src/pr-url-detector.ts +++ b/packages/agent/src/pr-url-detector.ts @@ -1,11 +1,24 @@ -const PR_URL_REGEX = /https:\/\/github\.com\/[^/\s"]+\/[^/\s"]+\/pull\/\d+/; +const PR_URL_REGEX = /https:\/\/github\.com\/[^/\s"]+\/[^/\s"]+\/pull\/\d+/g; // A fixed window (not "since run start") so a PR the agent merely views on a // long run is too old to be mistaken for one it just created. export const PR_CREATION_RECENCY_MS = 5 * 60 * 1000; export function findPrUrl(text: string): string | null { - return text.match(PR_URL_REGEX)?.[0] ?? null; + return findPrUrls(text)[0] ?? null; +} + +export function findPrUrls(text: string): string[] { + return [...new Set(text.match(PR_URL_REGEX) ?? [])]; +} + +// Fails closed on missing/invalid input so we never attribute on uncertainty. +export function wasCreatedByLogin( + author: string | null | undefined, + login: string | null | undefined, +): boolean { + if (!author || !login) return false; + return author.toLowerCase() === login.toLowerCase(); } // Fails closed on missing/invalid input so we never attribute on uncertainty. diff --git a/packages/agent/src/server/agent-server.test.ts b/packages/agent/src/server/agent-server.test.ts index ca321c7ea7..426a700443 100644 --- a/packages/agent/src/server/agent-server.test.ts +++ b/packages/agent/src/server/agent-server.test.ts @@ -1864,18 +1864,45 @@ describe("AgentServer HTTP Mode", () => { p: JwtPayload, u: Record | undefined, ): void; - fetchPrCreatedAt(url: string): Promise; + fetchPrAttribution( + url: string, + ): Promise<{ createdAt: string | null; author: string | null }>; + fetchGhLogin(): Promise; detectedPrUrl: string | null; - posthogAPI: { updateTaskRun: ReturnType }; + posthogAPI: { + getTaskRun: ReturnType; + updateTaskRun: ReturnType; + }; }; const justNow = () => new Date().toISOString(); const longAgo = "2020-01-01T00:00:00Z"; + const GH_LOGIN = "run-owner"; - const setup = (prCreatedAt: string | null): PrTestServer => { + const setup = ( + prCreatedAt: string | null, + prAuthor: string | null = GH_LOGIN, + ): PrTestServer => { const s = createServer() as unknown as PrTestServer; - s.fetchPrCreatedAt = vi.fn(async () => prCreatedAt); - s.posthogAPI = { updateTaskRun: vi.fn(async () => ({})) }; + s.fetchPrAttribution = vi.fn(async () => ({ + createdAt: prCreatedAt, + author: prAuthor, + })); + s.fetchGhLogin = vi.fn(async () => GH_LOGIN); + let storedOutput: Record | null = null; + s.posthogAPI = { + getTaskRun: vi.fn(async () => ({ output: storedOutput })), + updateTaskRun: vi.fn( + async ( + _taskId: string, + _runId: string, + updates: { output: Record }, + ) => { + storedOutput = updates.output; + return {}; + }, + ), + }; return s; }; @@ -1886,7 +1913,7 @@ describe("AgentServer HTTP Mode", () => { s.maybeAttachCreatedPr(payload, terminalUpdate(PR_URL)); await flush(); expect(s.posthogAPI.updateTaskRun).toHaveBeenCalledWith("t", "r", { - output: { pr_url: PR_URL }, + output: { pr_url: PR_URL, pr_urls: [PR_URL] }, }); expect(s.detectedPrUrl).toBe(PR_URL); }); @@ -1903,7 +1930,7 @@ describe("AgentServer HTTP Mode", () => { const s = setup(justNow()); s.maybeAttachCreatedPr(payload, { sessionUpdate: "agent_thought_chunk" }); await flush(); - expect(s.fetchPrCreatedAt).not.toHaveBeenCalled(); + expect(s.fetchPrAttribution).not.toHaveBeenCalled(); expect(s.posthogAPI.updateTaskRun).not.toHaveBeenCalled(); }); @@ -1913,12 +1940,11 @@ describe("AgentServer HTTP Mode", () => { s.maybeAttachCreatedPr(payload, terminalUpdate(PR_URL)); s.maybeAttachCreatedPr(payload, terminalUpdate(PR_URL)); await flush(); - expect(s.fetchPrCreatedAt).toHaveBeenCalledTimes(1); + expect(s.fetchPrAttribution).toHaveBeenCalledTimes(1); expect(s.posthogAPI.updateTaskRun).toHaveBeenCalledTimes(1); }); - it("attributes the most recent PR when a run opens several, in detection order", async () => { - // output.pr_url holds one value; the latest PR the run created is the useful one. + it("accumulates every PR a run opens, keeping the first as primary", async () => { const s = setup(justNow()); const second = "https://github.com/PostHog/posthog.com/pull/17765"; s.maybeAttachCreatedPr(payload, terminalUpdate(PR_URL)); @@ -1926,7 +1952,7 @@ describe("AgentServer HTTP Mode", () => { await flush(); expect(s.posthogAPI.updateTaskRun).toHaveBeenCalledTimes(2); expect(s.posthogAPI.updateTaskRun).toHaveBeenLastCalledWith("t", "r", { - output: { pr_url: second }, + output: { pr_url: PR_URL, pr_urls: [PR_URL, second] }, }); expect(s.detectedPrUrl).toBe(second); }); @@ -1935,15 +1961,32 @@ describe("AgentServer HTTP Mode", () => { const viewed = "https://github.com/PostHog/posthog.com/pull/1"; // The created PR reads as recent; the later, merely-viewed PR reads as old. const s = setup(justNow()); - s.fetchPrCreatedAt = vi.fn(async (url: string) => - url === PR_URL ? justNow() : longAgo, - ); + s.fetchPrAttribution = vi.fn(async (url: string) => ({ + createdAt: url === PR_URL ? justNow() : longAgo, + author: GH_LOGIN, + })); s.maybeAttachCreatedPr(payload, terminalUpdate(PR_URL)); s.maybeAttachCreatedPr(payload, terminalUpdate(viewed)); await flush(); expect(s.detectedPrUrl).toBe(PR_URL); expect(s.posthogAPI.updateTaskRun).toHaveBeenCalledTimes(1); }); + + it("does not attribute a fresh PR authored by someone else (merely viewed)", async () => { + const s = setup(justNow(), "someone-else"); + s.maybeAttachCreatedPr(payload, terminalUpdate(PR_URL)); + await flush(); + expect(s.posthogAPI.updateTaskRun).not.toHaveBeenCalled(); + expect(s.detectedPrUrl).toBeNull(); + }); + + it("fails closed when the run's GitHub identity cannot be resolved", async () => { + const s = setup(justNow()); + s.fetchGhLogin = vi.fn(async () => null); + s.maybeAttachCreatedPr(payload, terminalUpdate(PR_URL)); + await flush(); + expect(s.posthogAPI.updateTaskRun).not.toHaveBeenCalled(); + }); }); describe("buildCloudSystemPrompt", () => { diff --git a/packages/agent/src/server/agent-server.ts b/packages/agent/src/server/agent-server.ts index 7202875935..78b8621350 100644 --- a/packages/agent/src/server/agent-server.ts +++ b/packages/agent/src/server/agent-server.ts @@ -16,7 +16,12 @@ import { import { type ServerType, serve } from "@hono/node-server"; import { execGh } from "@posthog/git/gh"; import { getCurrentBranch } from "@posthog/git/queries"; -import type { Adapter } from "@posthog/shared"; +import { + type Adapter, + buildPrOutput, + mergePrUrls, + readPrUrls, +} from "@posthog/shared"; import { unzipSync } from "fflate"; import { Hono } from "hono"; import { z } from "zod"; @@ -45,7 +50,11 @@ import type { PermissionMode } from "../execution-mode"; import { DEFAULT_CODEX_MODEL, fetchGatewayModels } from "../gateway-models"; import { HandoffCheckpointTracker } from "../handoff-checkpoint"; import { PostHogAPIClient } from "../posthog-api"; -import { findPrUrl, wasCreatedRecently } from "../pr-url-detector"; +import { + findPrUrls, + wasCreatedByLogin, + wasCreatedRecently, +} from "../pr-url-detector"; import { formatConversationForResume, type ResumeState, @@ -3355,13 +3364,14 @@ ${signedCommitInstructions}${prLinkInstructions}${shellEfficiencyInstructions} update: Record | undefined, ): void { if (!update) return; - const prUrl = findPrUrl(JSON.stringify(update)); - if (!prUrl || this.evaluatedPrUrls.has(prUrl)) return; - this.evaluatedPrUrls.add(prUrl); - // Chain so attributions run in detection order; later PRs overwrite earlier ones. - this.prAttributionChain = this.prAttributionChain - .catch(() => {}) - .then(() => this.attachPrIfCreatedThisRun(payload, prUrl)); + for (const prUrl of findPrUrls(JSON.stringify(update))) { + if (this.evaluatedPrUrls.has(prUrl)) continue; + this.evaluatedPrUrls.add(prUrl); + // Chain so attributions run in detection order; later PRs append after earlier ones. + this.prAttributionChain = this.prAttributionChain + .catch(() => {}) + .then(() => this.attachPrIfCreatedThisRun(payload, prUrl)); + } } private async attachPrIfCreatedThisRun( @@ -3371,9 +3381,13 @@ ${signedCommitInstructions}${prLinkInstructions}${shellEfficiencyInstructions} // Already the attributed PR (e.g. seeded from a Slack notification, or re-detected). if (prUrl === this.detectedPrUrl) return; - let createdAt: string | null; + let attribution: { createdAt: string | null; author: string | null }; + let ghLogin: string | null; try { - createdAt = await this.fetchPrCreatedAt(prUrl); + [attribution, ghLogin] = await Promise.all([ + this.fetchPrAttribution(prUrl), + this.fetchGhLogin(), + ]); } catch (err) { this.logger.debug("PR attribution lookup failed", { runId: payload.run_id, @@ -3383,14 +3397,21 @@ ${signedCommitInstructions}${prLinkInstructions}${shellEfficiencyInstructions} return; } - // Only attribute PRs created during this run, not ones the agent merely viewed. - if (!wasCreatedRecently(createdAt, Date.now())) return; + // Only attribute PRs created during this run by this run's own GitHub + // identity — not ones the agent merely viewed. + if (!wasCreatedRecently(attribution.createdAt, Date.now())) return; + if (!wasCreatedByLogin(attribution.author, ghLogin)) return; this.detectedPrUrl = prUrl; try { + const freshOutput = await this.posthogAPI + .getTaskRun(payload.task_id, payload.run_id) + .then((run) => run.output) + .catch(() => null); + const urls = mergePrUrls(readPrUrls(freshOutput), [prUrl]); await this.posthogAPI.updateTaskRun(payload.task_id, payload.run_id, { - output: { pr_url: prUrl }, + output: buildPrOutput(freshOutput, urls), }); this.logger.debug("Attributed created PR to task run", { taskId: payload.task_id, @@ -3407,21 +3428,50 @@ ${signedCommitInstructions}${prLinkInstructions}${shellEfficiencyInstructions} } } - private async fetchPrCreatedAt(prUrl: string): Promise { - const res = await execGh(["pr", "view", prUrl, "--json", "createdAt"], { - cwd: this.config.repositoryPath, - timeoutMs: 10_000, - }); - if (res.exitCode !== 0) return null; + private async fetchPrAttribution( + prUrl: string, + ): Promise<{ createdAt: string | null; author: string | null }> { + const res = await execGh( + ["pr", "view", prUrl, "--json", "createdAt,author"], + { + cwd: this.config.repositoryPath, + timeoutMs: 10_000, + }, + ); + if (res.exitCode !== 0) return { createdAt: null, author: null }; try { - return ( - (JSON.parse(res.stdout) as { createdAt?: string }).createdAt ?? null - ); + const data = JSON.parse(res.stdout) as { + createdAt?: string; + author?: { login?: string }; + }; + return { + createdAt: data.createdAt ?? null, + author: data.author?.login ?? null, + }; } catch { - return null; + return { createdAt: null, author: null }; } } + private ghLoginPromise: Promise | null = null; + + private fetchGhLogin(): Promise { + this.ghLoginPromise ??= execGh(["api", "user", "--jq", ".login"], { + cwd: this.config.repositoryPath, + timeoutMs: 10_000, + }) + .then((res) => { + const login = res.exitCode === 0 ? res.stdout.trim() : ""; + if (!login) this.ghLoginPromise = null; + return login || null; + }) + .catch(() => { + this.ghLoginPromise = null; + return null; + }); + return this.ghLoginPromise; + } + private async cleanupSession({ completeEventStream = false, }: { diff --git a/packages/core/src/git-interaction/gitInteractionService.test.ts b/packages/core/src/git-interaction/gitInteractionService.test.ts index f3436a2d4f..8cbba71945 100644 --- a/packages/core/src/git-interaction/gitInteractionService.test.ts +++ b/packages/core/src/git-interaction/gitInteractionService.test.ts @@ -276,6 +276,7 @@ describe("GitInteractionService.runCreatePr", () => { expect(effects.attachPrUrlToTask).toHaveBeenCalledWith( "t", "https://example.test/pr/1", + undefined, ); if (result.outcome === "success") { expect(result.linkedBranchName).toBe("feature-x"); diff --git a/packages/core/src/git-interaction/gitInteractionService.ts b/packages/core/src/git-interaction/gitInteractionService.ts index fe86571ab1..cc4ba46595 100644 --- a/packages/core/src/git-interaction/gitInteractionService.ts +++ b/packages/core/src/git-interaction/gitInteractionService.ts @@ -82,7 +82,7 @@ export interface GitInteractionEffects { markFirstPrShipped(): void; celebrate(): void; openExternalUrl(url: string): void; - attachPrUrlToTask(taskId: string, prUrl: string): void; + attachPrUrlToTask(taskId: string, prUrl: string, prTitle?: string): void; getConversationContext(taskId: string): string | undefined; logError(message: string, error: unknown): void; logWarn(message: string, context: Record): void; @@ -385,7 +385,11 @@ export class GitInteractionService { if (result.prUrl) { this.effects.openExternalUrl(result.prUrl); - this.effects.attachPrUrlToTask(input.taskId, result.prUrl); + this.effects.attachPrUrlToTask( + input.taskId, + result.prUrl, + input.prTitle.trim() || undefined, + ); } return { diff --git a/packages/core/src/git-pr/git-pr.ts b/packages/core/src/git-pr/git-pr.ts index 8291590cf6..6c06bfd0d8 100644 --- a/packages/core/src/git-pr/git-pr.ts +++ b/packages/core/src/git-pr/git-pr.ts @@ -233,6 +233,40 @@ ${truncatedDiff || "(no diff available)"}${contextSection}`; }; } + async generatePrShortSummary( + conversationContext?: string, + prTitle?: string, + ): Promise<{ summary: string }> { + if (!conversationContext && !prTitle) return { summary: "" }; + + const system = `You generate ultra-short labels for pull requests. Given context about a PR, output a label of 15-20 characters that captures what the PR does. + +Rules: +- 15-20 characters total, never more than 24 +- Plain words, no punctuation, no quotes, no trailing period +- Imperative mood ("Fix login loop" not "Fixed login loop") +- Output only the label, nothing else`; + + const parts: string[] = []; + if (prTitle) parts.push(`PR title: ${prTitle}`); + if (conversationContext) { + parts.push(`Conversation context:\n${conversationContext}`); + } + + const response = await this.llm.prompt( + [{ role: "user", content: parts.join("\n\n") }], + { + system, + maxTokens: 30, + model: HELPER_GATEWAY_MODEL, + posthogProperties: { $ai_span_name: "pr_short_summary" }, + }, + ); + + const summary = response.content.trim().replace(/^["']|["']$/g, ""); + return { summary: summary.length > 24 ? summary.slice(0, 24) : summary }; + } + /** * Orchestrate branch -> commit -> push -> PR creation as a saga. Host git/gh * operations come through `host`; commit-message and PR-description generation diff --git a/packages/core/src/git/router-schemas.ts b/packages/core/src/git/router-schemas.ts index 54c94a490e..81c275f171 100644 --- a/packages/core/src/git/router-schemas.ts +++ b/packages/core/src/git/router-schemas.ts @@ -384,6 +384,7 @@ export const getPrDetailsByUrlOutput = z.object({ merged: z.boolean(), draft: z.boolean(), headRefName: z.string().nullable(), + title: z.string().nullable(), }); export type PrDetailsByUrlOutput = z.infer; @@ -496,6 +497,15 @@ export const generatePrTitleAndBodyOutput = z.object({ body: z.string(), }); +export const generatePrShortSummaryInput = z.object({ + conversationContext: z.string().optional(), + prTitle: z.string().optional(), +}); + +export const generatePrShortSummaryOutput = z.object({ + summary: z.string(), +}); + export const gitStateSnapshotSchema = z.object({ changedFiles: z.array(changedFileSchema).optional(), diffStats: diffStatsSchema.optional(), diff --git a/packages/core/src/sidebar/buildSidebarData.ts b/packages/core/src/sidebar/buildSidebarData.ts index 43edfc52b2..aea6828e27 100644 --- a/packages/core/src/sidebar/buildSidebarData.ts +++ b/packages/core/src/sidebar/buildSidebarData.ts @@ -1,3 +1,4 @@ +import { readPrUrls } from "@posthog/shared"; import type { Task, TaskRunStatus } from "@posthog/shared/domain-types"; import { getRepositoryInfo } from "./groupTasks"; import type { TaskData } from "./sidebarData.types"; @@ -150,9 +151,9 @@ export function deriveTaskData( taskLastViewedAt != null && lastActivityAt > taskLastViewedAt; const cloudPrUrl = - typeof task.latest_run?.output?.pr_url === "string" - ? task.latest_run.output.pr_url - : ((session?.cloudOutput?.pr_url as string | undefined) ?? null); + readPrUrls(task.latest_run?.output)[0] ?? + readPrUrls(session?.cloudOutput)[0] ?? + null; const originProduct = task.origin_product ?? diff --git a/packages/host-router/src/ports/git-pr-status.ts b/packages/host-router/src/ports/git-pr-status.ts index d6b0c14e31..eadfaed4b5 100644 --- a/packages/host-router/src/ports/git-pr-status.ts +++ b/packages/host-router/src/ports/git-pr-status.ts @@ -13,4 +13,5 @@ export interface IGitPrStatus { cloudPrUrl: string | null, ): Promise; getCachedPrUrl(taskId: string): CachedPrUrlOutput; + setPrimaryPrUrl(taskId: string, prUrl: string): void; } diff --git a/packages/host-router/src/routers/git.router.ts b/packages/host-router/src/routers/git.router.ts index 79ffa22dfb..219b00790a 100644 --- a/packages/host-router/src/routers/git.router.ts +++ b/packages/host-router/src/routers/git.router.ts @@ -29,6 +29,8 @@ import { discardFileChangesOutput, generateCommitMessageInput, generateCommitMessageOutput, + generatePrShortSummaryInput, + generatePrShortSummaryOutput, generatePrTitleAndBodyInput, generatePrTitleAndBodyOutput, getAllBranchesInput, @@ -525,6 +527,7 @@ export const gitRouter = router({ merged: false, draft: false, headRefName: null, + title: null, } ); }), @@ -656,6 +659,16 @@ export const gitRouter = router({ ), ), + generatePrShortSummary: publicProcedure + .input(generatePrShortSummaryInput) + .output(generatePrShortSummaryOutput) + .mutation(({ ctx, input }) => + getGitPrService(ctx.container).generatePrShortSummary( + input.conversationContext, + input.prTitle, + ), + ), + searchGithubRefs: publicProcedure .input(searchGithubRefsInput) .output(searchGithubRefsOutput) diff --git a/packages/host-router/src/routers/workspace.router.ts b/packages/host-router/src/routers/workspace.router.ts index f87dda1e7d..93ddadb574 100644 --- a/packages/host-router/src/routers/workspace.router.ts +++ b/packages/host-router/src/routers/workspace.router.ts @@ -34,6 +34,7 @@ import { markViewedInput, reconcileCloudWorkspacesInput, reconcileCloudWorkspacesOutput, + setPrimaryPrUrlInput, taskPrStatusInput, taskPrStatusOutput, togglePinInput, @@ -240,6 +241,12 @@ export const workspaceRouter = router({ getGitService(ctx.container).getCachedPrUrl(input.taskId), ), + setPrimaryPrUrl: publicProcedure + .input(setPrimaryPrUrlInput) + .mutation(({ ctx, input }) => + getGitService(ctx.container).setPrimaryPrUrl(input.taskId, input.prUrl), + ), + onError: subscribe(WorkspaceServiceEvent.Error), onWarning: subscribe(WorkspaceServiceEvent.Warning), onPromoted: subscribe(WorkspaceServiceEvent.Promoted), diff --git a/packages/shared/src/index.ts b/packages/shared/src/index.ts index fa824dae13..af9b5e9b34 100644 --- a/packages/shared/src/index.ts +++ b/packages/shared/src/index.ts @@ -139,6 +139,13 @@ export { pathToFileUri, toRelativePath, } from "./path"; +export { + buildPrOutput, + mergePrUrls, + promotePrUrl, + readPrSummaries, + readPrUrls, +} from "./pr-urls"; export { type CloudRegion, formatRegionBadge, diff --git a/packages/shared/src/pr-urls.test.ts b/packages/shared/src/pr-urls.test.ts new file mode 100644 index 0000000000..922c308cc5 --- /dev/null +++ b/packages/shared/src/pr-urls.test.ts @@ -0,0 +1,140 @@ +import { describe, expect, it } from "vitest"; +import { + buildPrOutput, + mergePrUrls, + promotePrUrl, + readPrSummaries, + readPrUrls, +} from "./pr-urls"; + +const A = "https://github.com/posthog/posthog/pull/1"; +const B = "https://github.com/posthog/posthog/pull/2"; +const C = "https://github.com/other/repo/pull/3"; + +describe("readPrUrls", () => { + it.each([ + ["null output", null, []], + ["undefined output", undefined, []], + ["empty output", {}, []], + ["legacy pr_url only", { pr_url: A }, [A]], + ["pr_urls only", { pr_urls: [A, B] }, [A, B]], + ["consistent pr_url and pr_urls", { pr_url: A, pr_urls: [A, B] }, [A, B]], + [ + "old-writer pr_url diverging from pr_urls appends at end", + { pr_url: C, pr_urls: [A, B] }, + [A, B, C], + ], + ["empty string pr_url ignored", { pr_url: "" }, []], + [ + "non-string junk filtered from pr_urls", + { pr_urls: [A, 42, null, "", B] }, + [A, B], + ], + ["duplicates collapsed", { pr_urls: [A, B, A] }, [A, B]], + ["non-array pr_urls with pr_url", { pr_url: A, pr_urls: "junk" }, [A]], + ])("%s", (_name, output, expected) => { + expect(readPrUrls(output as Record | null)).toEqual( + expected, + ); + }); +}); + +describe("mergePrUrls", () => { + it.each([ + ["no lists", [], []], + ["single list", [[A, B]], [A, B]], + [ + "earlier list wins on order", + [ + [A, B], + [C, A], + ], + [A, B, C], + ], + ["dedupes across lists", [[A], [A], [B]], [A, B]], + ["empty lists ignored", [[], [A], []], [A]], + ])("%s", (_name, lists, expected) => { + expect(mergePrUrls(...(lists as string[][]))).toEqual(expected); + }); +}); + +describe("promotePrUrl", () => { + it.each([ + ["moves an existing url to the front", [A, B, C], B, [B, A, C]], + ["keeps an already-primary url in place", [A, B], A, [A, B]], + ["adds a missing url at the front", [A, B], C, [C, A, B]], + ["works on an empty list", [], A, [A]], + ])("%s", (_name, urls, url, expected) => { + expect(promotePrUrl(urls, url)).toEqual(expected); + }); +}); + +describe("readPrSummaries", () => { + it.each([ + ["null output", null, {}], + ["missing key", {}, {}], + ["non-object pr_summaries", { pr_summaries: "junk" }, {}], + ["array pr_summaries", { pr_summaries: [A] }, {}], + [ + "keeps string entries, drops junk and empties", + { pr_summaries: { [A]: "Fix login loop", [B]: 42, [C]: "" } }, + { [A]: "Fix login loop" }, + ], + ])("%s", (_name, output, expected) => { + expect(readPrSummaries(output as Record | null)).toEqual( + expected, + ); + }); +}); + +describe("buildPrOutput", () => { + it("sets pr_url to the first entry and pr_urls to the full list", () => { + expect(buildPrOutput({}, [A, B])).toEqual({ pr_url: A, pr_urls: [A, B] }); + }); + + it("preserves foreign keys", () => { + expect(buildPrOutput({ commit_sha: "abc", pr_url: B }, [A, B])).toEqual({ + commit_sha: "abc", + pr_url: A, + pr_urls: [A, B], + }); + }); + + it("drops stale pr keys when the list is empty", () => { + expect(buildPrOutput({ commit_sha: "abc", pr_url: A }, [])).toEqual({ + commit_sha: "abc", + }); + }); + + it("dedupes and filters the provided list", () => { + expect(buildPrOutput(null, [A, "", A, B])).toEqual({ + pr_url: A, + pr_urls: [A, B], + }); + }); + + it("merges new summaries over existing ones", () => { + const existing = { pr_summaries: { [A]: "Old label" } }; + expect(buildPrOutput(existing, [A, B], { [B]: "Fix login loop" })).toEqual({ + pr_url: A, + pr_urls: [A, B], + pr_summaries: { [A]: "Old label", [B]: "Fix login loop" }, + }); + }); + + it("drops summaries for urls no longer in the list", () => { + const existing = { pr_summaries: { [A]: "Old label", [C]: "Stale" } }; + expect(buildPrOutput(existing, [A])).toEqual({ + pr_url: A, + pr_urls: [A], + pr_summaries: { [A]: "Old label" }, + }); + }); + + it("omits pr_summaries entirely when none apply", () => { + expect(buildPrOutput({ pr_summaries: { [C]: "Stale" } }, [A])).toEqual({ + pr_url: A, + pr_urls: [A], + }); + }); +}); diff --git a/packages/shared/src/pr-urls.ts b/packages/shared/src/pr-urls.ts new file mode 100644 index 0000000000..cd7a4d0ea4 --- /dev/null +++ b/packages/shared/src/pr-urls.ts @@ -0,0 +1,77 @@ +function dedupeNonEmpty(urls: readonly unknown[]): string[] { + const seen = new Set(); + const result: string[] = []; + for (const url of urls) { + if (typeof url !== "string" || url.length === 0 || seen.has(url)) continue; + seen.add(url); + result.push(url); + } + return result; +} + +export function readPrUrls( + output: Record | null | undefined, +): string[] { + if (!output) return []; + const listed = Array.isArray(output.pr_urls) + ? dedupeNonEmpty(output.pr_urls) + : []; + const single = output.pr_url; + if (typeof single === "string" && single.length > 0) { + if (listed.length === 0) return [single]; + if (!listed.includes(single)) listed.push(single); + } + return listed; +} + +export function mergePrUrls( + ...lists: ReadonlyArray +): string[] { + return dedupeNonEmpty(lists.flat()); +} + +export function promotePrUrl(urls: readonly string[], url: string): string[] { + return dedupeNonEmpty([url, ...urls]); +} + +export function readPrSummaries( + output: Record | null | undefined, +): Record { + const raw = output?.pr_summaries; + if (!raw || typeof raw !== "object" || Array.isArray(raw)) return {}; + const result: Record = {}; + for (const [url, summary] of Object.entries(raw)) { + if (typeof summary === "string" && summary.length > 0) { + result[url] = summary; + } + } + return result; +} + +export function buildPrOutput( + existing: Record | null | undefined, + urls: readonly string[], + summaries?: Record, +): Record { + const clean = dedupeNonEmpty(urls); + const { + pr_url: _prUrl, + pr_urls: _prUrls, + pr_summaries: _prSummaries, + ...rest + } = existing ?? {}; + if (clean.length === 0) return rest; + + const merged = { ...readPrSummaries(existing), ...summaries }; + const kept: Record = {}; + for (const url of clean) { + if (merged[url]) kept[url] = merged[url]; + } + + return { + ...rest, + pr_url: clean[0], + pr_urls: clean, + ...(Object.keys(kept).length > 0 ? { pr_summaries: kept } : {}), + }; +} diff --git a/packages/ui/src/features/git-interaction/cloudPrUrl.test.ts b/packages/ui/src/features/git-interaction/cloudPrUrl.test.ts index d65a745cf9..18ce673f4d 100644 --- a/packages/ui/src/features/git-interaction/cloudPrUrl.test.ts +++ b/packages/ui/src/features/git-interaction/cloudPrUrl.test.ts @@ -1,7 +1,7 @@ import type { Task } from "@posthog/shared/domain-types"; import type { AgentSession } from "@posthog/ui/features/sessions/sessionStore"; import { describe, expect, it } from "vitest"; -import { resolveCloudPrUrl } from "./cloudPrUrl"; +import { resolveCloudPrUrl, resolveCloudPrUrls } from "./cloudPrUrl"; function makeTask(prUrl?: unknown): Task { return { @@ -57,4 +57,67 @@ describe("resolveCloudPrUrl", () => { "https://github.com/org/repo/pull/3", ); }); + + it("returns the first entry of pr_urls as the primary", () => { + const task = { + id: "task-1", + latest_run: { + output: { + pr_url: "https://github.com/org/repo/pull/1", + pr_urls: [ + "https://github.com/org/repo/pull/1", + "https://github.com/org/repo/pull/2", + ], + }, + }, + } as unknown as Task; + expect(resolveCloudPrUrl(task, undefined)).toBe( + "https://github.com/org/repo/pull/1", + ); + }); +}); + +describe("resolveCloudPrUrls", () => { + it("returns an empty list when both sources are undefined", () => { + expect(resolveCloudPrUrls(undefined, undefined)).toEqual([]); + }); + + it("unions task and session URLs with task order winning", () => { + const task = { + id: "task-1", + latest_run: { + output: { + pr_url: "https://github.com/org/repo/pull/1", + pr_urls: [ + "https://github.com/org/repo/pull/1", + "https://github.com/org/repo/pull/2", + ], + }, + }, + } as unknown as Task; + const session = { + cloudOutput: { pr_url: "https://github.com/org/repo/pull/3" }, + } as unknown as AgentSession; + expect(resolveCloudPrUrls(task, session)).toEqual([ + "https://github.com/org/repo/pull/1", + "https://github.com/org/repo/pull/2", + "https://github.com/org/repo/pull/3", + ]); + }); + + it("appends a diverging legacy pr_url after the listed ones", () => { + const task = { + id: "task-1", + latest_run: { + output: { + pr_url: "https://github.com/org/repo/pull/9", + pr_urls: ["https://github.com/org/repo/pull/1"], + }, + }, + } as unknown as Task; + expect(resolveCloudPrUrls(task, undefined)).toEqual([ + "https://github.com/org/repo/pull/1", + "https://github.com/org/repo/pull/9", + ]); + }); }); diff --git a/packages/ui/src/features/git-interaction/cloudPrUrl.ts b/packages/ui/src/features/git-interaction/cloudPrUrl.ts index 11e659a921..a6a942e6d1 100644 --- a/packages/ui/src/features/git-interaction/cloudPrUrl.ts +++ b/packages/ui/src/features/git-interaction/cloudPrUrl.ts @@ -1,19 +1,30 @@ +import { mergePrUrls, readPrSummaries, readPrUrls } from "@posthog/shared"; import type { Task } from "@posthog/shared/domain-types"; import type { AgentSession } from "@posthog/ui/features/sessions/sessionStore"; -/** - * Extracts the PR URL from a task and/or session. The URL can arrive via the - * persisted TaskRun output or the live session's cloudOutput (pushed over SSE - * while the run is active), so both sources are consulted. - */ +export function resolveCloudPrUrls( + task: Task | undefined, + session: AgentSession | undefined, +): string[] { + return mergePrUrls( + readPrUrls(task?.latest_run?.output), + readPrUrls(session?.cloudOutput), + ); +} + +export function resolveCloudPrSummaries( + task: Task | undefined, + session: AgentSession | undefined, +): Record { + return { + ...readPrSummaries(session?.cloudOutput), + ...readPrSummaries(task?.latest_run?.output), + }; +} + export function resolveCloudPrUrl( task: Task | undefined, session: AgentSession | undefined, ): string | null { - const taskPrUrl = task?.latest_run?.output?.pr_url; - const sessionPrUrl = session?.cloudOutput?.pr_url; - - if (typeof taskPrUrl === "string" && taskPrUrl) return taskPrUrl; - if (typeof sessionPrUrl === "string" && sessionPrUrl) return sessionPrUrl; - return null; + return resolveCloudPrUrls(task, session)[0] ?? null; } diff --git a/packages/ui/src/features/git-interaction/components/TaskActionsMenu.tsx b/packages/ui/src/features/git-interaction/components/TaskActionsMenu.tsx index 945bdc605d..768dc20550 100644 --- a/packages/ui/src/features/git-interaction/components/TaskActionsMenu.tsx +++ b/packages/ui/src/features/git-interaction/components/TaskActionsMenu.tsx @@ -9,6 +9,7 @@ import { GitPullRequest, } from "@phosphor-icons/react"; import { getPrVisualConfig } from "@posthog/core/git-interaction/prStatus"; +import { parseGithubUrl } from "@posthog/git/utils"; import { ButtonGroup, DropdownMenuContent, @@ -24,15 +25,22 @@ import { ChevronDown } from "lucide-react"; import { Tooltip } from "../../../primitives/Tooltip"; import { toast } from "../../../primitives/toast"; import { useLocalRepoPath } from "../../workspace/useLocalRepoPath"; -import { getPrActionIcon } from "../prIcon"; +import { getPrActionIcon, getPrVisualIcon } from "../prIcon"; +import { useCloudPrSummaries, useCloudPrUrls } from "../useCloudPrUrl"; import { type GitMenuAction, type GitMenuActionId, useGitInteraction, } from "../useGitInteraction"; import { usePrActions } from "../usePrActions"; -import { usePrDetails } from "../usePrDetails"; -import { useTaskPrUrl } from "../useTaskPrUrl"; +import { + type PrStateDetails, + usePrDetails, + usePrDetailsMap, +} from "../usePrDetails"; +import { usePrSummaryBackfill } from "../usePrSummaryBackfill"; +import { useSetPrimaryPr } from "../useSetPrimaryPr"; +import { useTaskPrUrls } from "../useTaskPrUrl"; import { CreatePrDialog } from "./CreatePrDialog"; import { GitBranchDialog, @@ -77,7 +85,12 @@ export function TaskActionsMenu({ taskId, isCloud }: TaskActionsMenuProps) { actions: gitActions, } = useGitInteraction(taskId, isCloud ? undefined : localRepoPath); - const prUrl = useTaskPrUrl(taskId, isCloud); + const { primaryUrl: prUrl, otherUrls } = useTaskPrUrls(taskId, isCloud); + const cloudPrUrls = useCloudPrUrls(taskId); + const prSummaries = useCloudPrSummaries(taskId); + const { mutate: setPrimaryPr } = useSetPrimaryPr(taskId); + usePrSummaryBackfill(taskId, cloudPrUrls, otherUrls.length > 0, prSummaries); + const otherPrDetails = usePrDetailsMap(otherUrls); const { meta: { state: prState, merged, draft, headRefName }, @@ -110,10 +123,17 @@ export function TaskActionsMenu({ taskId, isCloud }: TaskActionsMenuProps) { merged={merged} draft={draft} branchName={headRefName} + otherPrs={buildOtherPrItems( + pr.url, + otherUrls, + prSummaries, + otherPrDetails, + )} isPrPending={isPrActionPending} gitItems={gitItems} onGitSelect={gitActions.openAction} onPrSelect={executePrAction} + onOtherPrSelect={setPrimaryPr} /> ) : ( | null; +} + +function buildOtherPrItems( + primaryUrl: string, + otherUrls: string[], + summaries: Record, + details: Record, +): OtherPrItem[] { + const primary = parseGithubUrl(primaryUrl); + return otherUrls.map((url) => { + const parsed = parseGithubUrl(url); + const sameRepo = + !!parsed && + !!primary && + parsed.owner.toLowerCase() === primary.owner.toLowerCase() && + parsed.repo.toLowerCase() === primary.repo.toLowerCase(); + const detail = details[url]; + return { + url, + label: parsed?.kind === "pr" ? `#${parsed.number}` : url, + summary: summaries[url] ?? null, + repoLabel: parsed && !sameRepo ? `${parsed.owner}/${parsed.repo}` : null, + visual: detail + ? getPrVisualConfig(detail.state, detail.merged, detail.draft) + : null, + }; + }); +} + interface PrBadgeControlProps { prUrl: string; prState: string; merged: boolean; draft: boolean; branchName: string | null; + otherPrs: OtherPrItem[]; isPrPending: boolean; gitItems: GitMenuAction[]; onGitSelect: (id: GitMenuActionId) => void; onPrSelect: (action: PrActionType) => void; + onOtherPrSelect: (url: string) => void; } function PrBadgeControl({ @@ -224,15 +281,17 @@ function PrBadgeControl({ merged, draft, branchName, + otherPrs, isPrPending, gitItems, onGitSelect, onPrSelect, + onOtherPrSelect, }: PrBadgeControlProps) { const config = getPrVisualConfig(prState, merged, draft); const lifecycleItems = config.actions; const hasMenuItems = gitItems.length + lifecycleItems.length > 0; - const hasDropdown = hasMenuItems || !!branchName; + const hasDropdown = hasMenuItems || !!branchName || otherPrs.length > 0; const copyBranchName = async () => { if (!branchName) return; @@ -296,9 +355,49 @@ function PrBadgeControl({ ))} - {branchName && ( + {otherPrs.length > 0 && ( <> {hasMenuItems && } + + + + + Other PRs + + + + {otherPrs.map((otherPr) => ( + onOtherPrSelect(otherPr.url)} + > + + + + {otherPr.label} + {otherPr.summary && {otherPr.summary}} + {otherPr.visual && ( + + {" "} + · {otherPr.visual.label} + + )} + {otherPr.repoLabel && ( + · {otherPr.repoLabel} + )} + + + + ))} + + + + )} + {branchName && ( + <> + {(hasMenuItems || otherPrs.length > 0) && ( + + )} @@ -314,6 +413,14 @@ function PrBadgeControl({ ); } +function OtherPrStateIcon({ visual }: { visual: OtherPrItem["visual"] }) { + if (!visual) return ; + const StateIcon = getPrVisualIcon(visual.icon); + return ( + + ); +} + // --- Trigger when no PR: solid primary git action + git dropdown --- interface GitActionControlProps { diff --git a/packages/ui/src/features/git-interaction/gitInteractionAdapter.ts b/packages/ui/src/features/git-interaction/gitInteractionAdapter.ts index f98faabd04..48e63d1ffc 100644 --- a/packages/ui/src/features/git-interaction/gitInteractionAdapter.ts +++ b/packages/ui/src/features/git-interaction/gitInteractionAdapter.ts @@ -7,7 +7,13 @@ import { HOST_TRPC_CLIENT, type HostTrpcClient, } from "@posthog/host-router/client"; -import { ANALYTICS_EVENTS } from "@posthog/shared"; +import { + ANALYTICS_EVENTS, + buildPrOutput, + mergePrUrls, + promotePrUrl, + readPrUrls, +} from "@posthog/shared"; import { getAuthenticatedClient } from "@posthog/ui/features/auth/authClientImperative"; import { useOnboardingStore } from "@posthog/ui/features/onboarding/onboardingStore"; import { useSessionStore } from "@posthog/ui/features/sessions/sessionStore"; @@ -60,16 +66,128 @@ function getConversationContext(taskId: string): string | undefined { return state.sessions[taskRunId]?.conversationSummary; } -function attachPrUrlToTask(taskId: string, prUrl: string): void { - const taskRunId = useSessionStore.getState().taskIdIndex[taskId]; +function attachPrUrlToTask( + taskId: string, + prUrl: string, + prTitle?: string, +): void { + const state = useSessionStore.getState(); + const taskRunId = state.taskIdIndex[taskId]; if (!taskRunId) return; - void getAuthenticatedClient().then((client) => { + const sessionUrls = readPrUrls(state.sessions[taskRunId]?.cloudOutput); + const conversationContext = getConversationContext(taskId); + void getAuthenticatedClient().then(async (client) => { if (!client) return; - client - .updateTaskRun(taskId, taskRunId, { output: { pr_url: prUrl } }) - .catch((err) => - log.warn("Failed to attach PR URL to task", { taskId, prUrl, err }), - ); + try { + const [freshOutput, summary] = await Promise.all([ + client + .getTaskRun(taskId, taskRunId) + .then((run) => run.output) + .catch(() => null), + conversationContext || prTitle + ? host() + .git.generatePrShortSummary.mutate({ + conversationContext, + prTitle, + }) + .then((r) => r.summary || null) + .catch(() => null) + : Promise.resolve(null), + ]); + const urls = mergePrUrls(readPrUrls(freshOutput), sessionUrls, [prUrl]); + await client.updateTaskRun(taskId, taskRunId, { + output: buildPrOutput( + freshOutput, + urls, + summary ? { [prUrl]: summary } : undefined, + ), + }); + } catch (err) { + log.warn("Failed to attach PR URL to task", { taskId, prUrl, err }); + } + }); +} + +const summaryBackfillAttempts = new Set(); + +export async function backfillPrSummaries( + taskId: string, + urls: string[], + existingSummaries: Record, +): Promise { + const taskRunId = useSessionStore.getState().taskIdIndex[taskId]; + if (!taskRunId) return false; + const missing = urls.filter((url) => { + const key = `${taskRunId}|${url}`; + if (existingSummaries[url] || summaryBackfillAttempts.has(key)) { + return false; + } + summaryBackfillAttempts.add(key); + return true; + }); + if (missing.length === 0) return false; + const conversationContext = getConversationContext(taskId); + const client = await getAuthenticatedClient(); + if (!client) return false; + try { + const generated = await Promise.all( + missing.map(async (url) => { + const title = await host() + .git.getPrDetailsByUrl.query({ prUrl: url }) + .then((details) => details.title ?? undefined) + .catch(() => undefined); + if (!conversationContext && !title) return null; + const summary = await host() + .git.generatePrShortSummary.mutate({ + conversationContext, + prTitle: title, + }) + .then((r) => r.summary || null) + .catch(() => null); + return summary ? ([url, summary] as const) : null; + }), + ); + const summaries = Object.fromEntries( + generated.filter((entry) => entry !== null), + ); + if (Object.keys(summaries).length === 0) return false; + const freshOutput = await client + .getTaskRun(taskId, taskRunId) + .then((run) => run.output) + .catch(() => null); + const cloudUrls = readPrUrls(freshOutput); + if (cloudUrls.length === 0) return false; + await client.updateTaskRun(taskId, taskRunId, { + output: buildPrOutput(freshOutput, cloudUrls, summaries), + }); + return true; + } catch (err) { + log.warn("Failed to backfill PR summaries", { taskId, err }); + return false; + } +} + +export async function promoteTaskPrUrl( + taskId: string, + prUrl: string, +): Promise { + host() + .workspace.setPrimaryPrUrl.mutate({ taskId, prUrl }) + .catch((err) => + log.warn("Failed to promote PR locally", { taskId, prUrl, err }), + ); + + const taskRunId = useSessionStore.getState().taskIdIndex[taskId]; + if (!taskRunId) return; + const client = await getAuthenticatedClient(); + if (!client) return; + const freshOutput = await client + .getTaskRun(taskId, taskRunId) + .then((run) => run.output) + .catch(() => null); + const urls = promotePrUrl(readPrUrls(freshOutput), prUrl); + await client.updateTaskRun(taskId, taskRunId, { + output: buildPrOutput(freshOutput, urls), }); } diff --git a/packages/ui/src/features/git-interaction/useCloudPrUrl.ts b/packages/ui/src/features/git-interaction/useCloudPrUrl.ts index 8bf705cf7b..eb6de92938 100644 --- a/packages/ui/src/features/git-interaction/useCloudPrUrl.ts +++ b/packages/ui/src/features/git-interaction/useCloudPrUrl.ts @@ -1,13 +1,28 @@ import { useSessionForTask } from "../sessions/useSession"; import { useTasks } from "../tasks/useTasks"; -import { resolveCloudPrUrl } from "./cloudPrUrl"; +import { + resolveCloudPrSummaries, + resolveCloudPrUrl, + resolveCloudPrUrls, +} from "./cloudPrUrl"; export { resolveCloudPrUrl }; /** Hook wrapper for components that don't already have the task/session. */ export function useCloudPrUrl(taskId: string): string | null { + return useCloudPrUrls(taskId)[0] ?? null; +} + +export function useCloudPrUrls(taskId: string): string[] { + const { data: tasks = [] } = useTasks(); + const task = tasks.find((t) => t.id === taskId); + const session = useSessionForTask(taskId); + return resolveCloudPrUrls(task, session); +} + +export function useCloudPrSummaries(taskId: string): Record { const { data: tasks = [] } = useTasks(); const task = tasks.find((t) => t.id === taskId); const session = useSessionForTask(taskId); - return resolveCloudPrUrl(task, session); + return resolveCloudPrSummaries(task, session); } diff --git a/packages/ui/src/features/git-interaction/usePrActions.ts b/packages/ui/src/features/git-interaction/usePrActions.ts index 2b2932dc35..dcc16e9acd 100644 --- a/packages/ui/src/features/git-interaction/usePrActions.ts +++ b/packages/ui/src/features/git-interaction/usePrActions.ts @@ -24,6 +24,7 @@ export function usePrActions(prUrl: string | null) { (prev) => ({ ...getOptimisticPrState(variables.action), headRefName: prev?.headRefName ?? null, + title: prev?.title ?? null, }), ); // The inbox Pulls list reads PR status from the batched diff --git a/packages/ui/src/features/git-interaction/usePrDetails.ts b/packages/ui/src/features/git-interaction/usePrDetails.ts index edf5a171d7..657b027290 100644 --- a/packages/ui/src/features/git-interaction/usePrDetails.ts +++ b/packages/ui/src/features/git-interaction/usePrDetails.ts @@ -1,6 +1,6 @@ import { useHostTRPC } from "@posthog/host-router/react"; import type { PrReviewThread } from "@posthog/shared"; -import { useQuery } from "@tanstack/react-query"; +import { useQueries, useQuery } from "@tanstack/react-query"; import { useMemo } from "react"; import type { PrCommentThread } from "../code-review/prCommentAnnotations"; @@ -22,6 +22,39 @@ function threadsToMap(threads: PrReviewThread[]): Map { return map; } +export interface PrStateDetails { + state: string; + merged: boolean; + draft: boolean; +} + +/** + * Fetch lifecycle state for a set of PRs at once (the "Other PRs" submenu). + * Also serves as a prefetch: it warms the same `getPrDetailsByUrl` cache + * `usePrDetails` reads, so promoting one of these PRs renders its badge with + * the correct state instantly. + */ +export function usePrDetailsMap( + prUrls: string[], +): Record { + const trpc = useHostTRPC(); + return useQueries({ + queries: prUrls.map((prUrl) => ({ + ...trpc.git.getPrDetailsByUrl.queryOptions({ prUrl }), + staleTime: 60_000, + retry: 1, + })), + combine: (results) => + Object.fromEntries( + results.flatMap((result, i) => + result.data && result.data.state !== "unknown" + ? [[prUrls[i], result.data]] + : [], + ), + ), + }); +} + export function usePrDetails( prUrl: string | null, options?: UsePrDetailsOptions, diff --git a/packages/ui/src/features/git-interaction/usePrSummaryBackfill.ts b/packages/ui/src/features/git-interaction/usePrSummaryBackfill.ts new file mode 100644 index 0000000000..4761752d1c --- /dev/null +++ b/packages/ui/src/features/git-interaction/usePrSummaryBackfill.ts @@ -0,0 +1,28 @@ +import { useQueryClient } from "@tanstack/react-query"; +import { useEffect, useRef } from "react"; +import { taskKeys } from "../tasks/taskKeys"; +import { backfillPrSummaries } from "./gitInteractionAdapter"; + +export function usePrSummaryBackfill( + taskId: string, + cloudUrls: string[], + hasOtherPrs: boolean, + summaries: Record, +): void { + const queryClient = useQueryClient(); + const summariesRef = useRef(summaries); + summariesRef.current = summaries; + const urlsKey = cloudUrls.join("\n"); + useEffect(() => { + if (!hasOtherPrs || !urlsKey) return; + void backfillPrSummaries( + taskId, + urlsKey.split("\n"), + summariesRef.current, + ).then((wrote) => { + if (wrote) { + void queryClient.invalidateQueries({ queryKey: taskKeys.lists() }); + } + }); + }, [taskId, urlsKey, hasOtherPrs, queryClient]); +} diff --git a/packages/ui/src/features/git-interaction/useSetPrimaryPr.ts b/packages/ui/src/features/git-interaction/useSetPrimaryPr.ts new file mode 100644 index 0000000000..b7e5b747a9 --- /dev/null +++ b/packages/ui/src/features/git-interaction/useSetPrimaryPr.ts @@ -0,0 +1,97 @@ +import { useHostTRPC } from "@posthog/host-router/react"; +import { + buildPrOutput, + promotePrUrl, + readPrSummaries, + readPrUrls, +} from "@posthog/shared"; +import type { Task } from "@posthog/shared/domain-types"; +import { useMutation, useQueryClient } from "@tanstack/react-query"; +import { toast } from "../../primitives/toast"; +import { sessionStoreSetters, useSessionStore } from "../sessions/sessionStore"; +import { taskKeys } from "../tasks/taskKeys"; +import { promoteTaskPrUrl } from "./gitInteractionAdapter"; + +function promoteOutput( + output: Record | null | undefined, + prUrl: string, +): Record { + return buildPrOutput( + output, + promotePrUrl(readPrUrls(output), prUrl), + readPrSummaries(output), + ); +} + +export function useSetPrimaryPr(taskId: string) { + const queryClient = useQueryClient(); + const trpc = useHostTRPC(); + return useMutation({ + mutationFn: (prUrl: string) => promoteTaskPrUrl(taskId, prUrl), + onMutate: async (prUrl) => { + const cachedKey = trpc.workspace.getCachedPrUrl.queryKey({ taskId }); + await Promise.all([ + queryClient.cancelQueries({ queryKey: taskKeys.lists() }), + queryClient.cancelQueries({ queryKey: cachedKey }), + ]); + + const previousLists = queryClient.getQueriesData({ + queryKey: taskKeys.lists(), + }); + queryClient.setQueriesData( + { queryKey: taskKeys.lists() }, + (tasks) => + tasks?.map((task) => + task.id === taskId && task.latest_run + ? { + ...task, + latest_run: { + ...task.latest_run, + output: promoteOutput(task.latest_run.output, prUrl), + }, + } + : task, + ), + ); + + const previousCached = queryClient.getQueryData(cachedKey); + queryClient.setQueryData(cachedKey, (prev) => + prev + ? { ...prev, prUrl, prUrls: promotePrUrl(prev.prUrls, prUrl) } + : prev, + ); + + const state = useSessionStore.getState(); + const taskRunId = state.taskIdIndex[taskId]; + const previousOutput = taskRunId + ? state.sessions[taskRunId]?.cloudOutput + : undefined; + if (taskRunId && previousOutput) { + sessionStoreSetters.updateCloudStatus(taskRunId, { + output: promoteOutput(previousOutput, prUrl), + }); + } + + return { previousLists, previousCached, taskRunId, previousOutput }; + }, + onError: (_err, _prUrl, context) => { + for (const [key, data] of context?.previousLists ?? []) { + queryClient.setQueryData(key, data); + } + if (context) { + queryClient.setQueryData( + trpc.workspace.getCachedPrUrl.queryKey({ taskId }), + context.previousCached, + ); + if (context.taskRunId && context.previousOutput) { + sessionStoreSetters.updateCloudStatus(context.taskRunId, { + output: context.previousOutput, + }); + } + } + toast.error("Couldn't change primary PR"); + }, + onSuccess: () => + queryClient.invalidateQueries({ queryKey: taskKeys.lists() }), + }); +} diff --git a/packages/ui/src/features/git-interaction/useTaskPrUrl.ts b/packages/ui/src/features/git-interaction/useTaskPrUrl.ts index f924e19837..056cd5eef4 100644 --- a/packages/ui/src/features/git-interaction/useTaskPrUrl.ts +++ b/packages/ui/src/features/git-interaction/useTaskPrUrl.ts @@ -2,16 +2,17 @@ import { useHostTRPC } from "@posthog/host-router/react"; import { useQuery } from "@tanstack/react-query"; import { useLocalRepoPath } from "../workspace/useLocalRepoPath"; import { useWorkspace } from "../workspace/useWorkspace"; -import { useCloudPrUrl } from "./useCloudPrUrl"; +import { useCloudPrUrls } from "./useCloudPrUrl"; import { useLinkedBranchPrUrl } from "./useLinkedBranchPrUrl"; +import { resolveTaskPrUrls, type TaskPrUrls } from "./utils/resolveTaskPrUrls"; /** - * Resolves the PR URL for a task across all task kinds: - * - cloud: the cloud run's `pr_url` + * Resolves the PR URLs for a task across all task kinds: + * - cloud: the cloud run's accumulated `pr_urls` (first-created first) * - local: the linked-branch lookup, falling back to `getPrStatus` on the - * active repo path + * active repo path, plus every PR cached for the task over its lifetime * - * On task switch we prefer the cached PR URL from the workspaces table so the + * On task switch we prefer the cached PR URLs from the workspaces table so the * value is available synchronously — the live `gh` lookups still run and * supersede the cache as their values arrive. * @@ -19,8 +20,8 @@ import { useLinkedBranchPrUrl } from "./useLinkedBranchPrUrl"; * header (`CommandCenterPRButton`) so they always agree on what PR a task * points at. */ -export function useTaskPrUrl(taskId: string, isCloud: boolean): string | null { - const cloudPrUrl = useCloudPrUrl(taskId); +export function useTaskPrUrls(taskId: string, isCloud: boolean): TaskPrUrls { + const cloudUrls = useCloudPrUrls(taskId); const workspace = useWorkspace(taskId); const linkedPrUrl = useLinkedBranchPrUrl({ linkedBranch: workspace?.linkedBranch ?? null, @@ -45,6 +46,21 @@ export function useTaskPrUrl(taskId: string, isCloud: boolean): string | null { placeholderData: (prev) => prev, }); - if (isCloud) return cloudPrUrl; - return linkedPrUrl ?? prStatus?.prUrl ?? cached?.prUrl ?? null; + if (isCloud) { + return resolveTaskPrUrls({ + cloudUrls, + cachedUrls: [], + currentBranchUrl: null, + }); + } + + return resolveTaskPrUrls({ + cloudUrls, + cachedUrls: cached?.prUrls ?? [], + currentBranchUrl: linkedPrUrl ?? prStatus?.prUrl ?? cached?.prUrl ?? null, + }); +} + +export function useTaskPrUrl(taskId: string, isCloud: boolean): string | null { + return useTaskPrUrls(taskId, isCloud).primaryUrl; } diff --git a/packages/ui/src/features/git-interaction/utils/resolveTaskPrUrls.test.ts b/packages/ui/src/features/git-interaction/utils/resolveTaskPrUrls.test.ts new file mode 100644 index 0000000000..72f0e049ca --- /dev/null +++ b/packages/ui/src/features/git-interaction/utils/resolveTaskPrUrls.test.ts @@ -0,0 +1,52 @@ +import { describe, expect, it } from "vitest"; +import { resolveTaskPrUrls } from "./resolveTaskPrUrls"; + +const PR_1 = "https://github.com/org/repo/pull/1"; +const PR_2 = "https://github.com/org/repo/pull/2"; +const PR_3 = "https://github.com/other/repo/pull/3"; + +describe("resolveTaskPrUrls", () => { + it.each([ + [ + "no sources", + { cloudUrls: [], cachedUrls: [], currentBranchUrl: null }, + { primaryUrl: null, otherUrls: [] }, + ], + [ + "cloud first entry is primary", + { cloudUrls: [PR_1, PR_2], cachedUrls: [], currentBranchUrl: null }, + { primaryUrl: PR_1, otherUrls: [PR_2] }, + ], + [ + "cached order wins over current branch PR (promotion sticks)", + { cloudUrls: [], cachedUrls: [PR_1], currentBranchUrl: PR_2 }, + { primaryUrl: PR_1, otherUrls: [PR_2] }, + ], + [ + "cached list is the fallback primary", + { cloudUrls: [], cachedUrls: [PR_1, PR_2], currentBranchUrl: null }, + { primaryUrl: PR_1, otherUrls: [PR_2] }, + ], + [ + "current branch PR is the last-resort primary", + { cloudUrls: [], cachedUrls: [], currentBranchUrl: PR_2 }, + { primaryUrl: PR_2, otherUrls: [] }, + ], + [ + "primary is excluded from others across sources", + { cloudUrls: [PR_1], cachedUrls: [PR_1, PR_2], currentBranchUrl: PR_1 }, + { primaryUrl: PR_1, otherUrls: [PR_2] }, + ], + [ + "dedupes across sources preserving cloud order", + { + cloudUrls: [PR_1, PR_2], + cachedUrls: [PR_2, PR_3], + currentBranchUrl: PR_3, + }, + { primaryUrl: PR_1, otherUrls: [PR_2, PR_3] }, + ], + ])("%s", (_name, input, expected) => { + expect(resolveTaskPrUrls(input)).toEqual(expected); + }); +}); diff --git a/packages/ui/src/features/git-interaction/utils/resolveTaskPrUrls.ts b/packages/ui/src/features/git-interaction/utils/resolveTaskPrUrls.ts new file mode 100644 index 0000000000..7e6a3e74c7 --- /dev/null +++ b/packages/ui/src/features/git-interaction/utils/resolveTaskPrUrls.ts @@ -0,0 +1,26 @@ +import { mergePrUrls } from "@posthog/shared"; + +export interface TaskPrUrls { + primaryUrl: string | null; + otherUrls: string[]; +} + +export interface ResolveTaskPrUrlsInput { + cloudUrls: string[]; + cachedUrls: string[]; + currentBranchUrl: string | null; +} + +export function resolveTaskPrUrls({ + cloudUrls, + cachedUrls, + currentBranchUrl, +}: ResolveTaskPrUrlsInput): TaskPrUrls { + const primaryUrl = cloudUrls[0] ?? cachedUrls[0] ?? currentBranchUrl ?? null; + const otherUrls = mergePrUrls( + cloudUrls, + cachedUrls, + currentBranchUrl ? [currentBranchUrl] : [], + ).filter((url) => url !== primaryUrl); + return { primaryUrl, otherUrls }; +} diff --git a/packages/ui/src/features/workspace/workspace-events.contribution.test.ts b/packages/ui/src/features/workspace/workspace-events.contribution.test.ts index f0df80bd9b..ce3e6d020b 100644 --- a/packages/ui/src/features/workspace/workspace-events.contribution.test.ts +++ b/packages/ui/src/features/workspace/workspace-events.contribution.test.ts @@ -105,6 +105,7 @@ describe("WorkspaceEventsContribution", () => { client.handlers.onTaskPrInfoChanged({ taskId: "task-1", prUrl: "https://github.com/o/r/pull/1", + prUrls: ["https://github.com/o/r/pull/1"], prState: "open", }); @@ -138,7 +139,10 @@ describe("WorkspaceEventsContribution", () => { ["workspace", "getCachedPrUrl"], { input: { taskId: "task-1" }, type: "query" }, ], - { prUrl: "https://github.com/o/r/pull/1" }, + { + prUrl: "https://github.com/o/r/pull/1", + prUrls: ["https://github.com/o/r/pull/1"], + }, ); }); }); diff --git a/packages/ui/src/features/workspace/workspace-events.contribution.ts b/packages/ui/src/features/workspace/workspace-events.contribution.ts index de4280d022..c273cba24a 100644 --- a/packages/ui/src/features/workspace/workspace-events.contribution.ts +++ b/packages/ui/src/features/workspace/workspace-events.contribution.ts @@ -64,7 +64,7 @@ export class WorkspaceEventsContribution implements Contribution { queryClient: this.queryClient, }); this.hostClient.workspace.onTaskPrInfoChanged.subscribe(undefined, { - onData: ({ taskId, prUrl, prState }) => { + onData: ({ taskId, prUrl, prUrls, prState }) => { this.queryClient.setQueriesData<{ prState: typeof prState; hasDiff: boolean; @@ -83,7 +83,7 @@ export class WorkspaceEventsContribution implements Contribution { ); this.queryClient.setQueryData( options.workspace.getCachedPrUrl.queryKey({ taskId }), - { prUrl }, + { prUrl, prUrls: prUrls ?? [] }, ); }, }); diff --git a/packages/workspace-server/src/db/migrations/0018_add_pr_urls.sql b/packages/workspace-server/src/db/migrations/0018_add_pr_urls.sql new file mode 100644 index 0000000000..8b2391b37b --- /dev/null +++ b/packages/workspace-server/src/db/migrations/0018_add_pr_urls.sql @@ -0,0 +1 @@ +ALTER TABLE `workspaces` ADD `pr_urls` text DEFAULT '[]' NOT NULL; \ No newline at end of file diff --git a/packages/workspace-server/src/db/migrations/meta/0018_snapshot.json b/packages/workspace-server/src/db/migrations/meta/0018_snapshot.json new file mode 100644 index 0000000000..6fd42cd1cf --- /dev/null +++ b/packages/workspace-server/src/db/migrations/meta/0018_snapshot.json @@ -0,0 +1,1016 @@ +{ + "version": "6", + "dialect": "sqlite", + "id": "44f02595-2145-4a3a-a2f0-751791193102", + "prevId": "60bfe0e6-f17c-481a-ae23-482b07bf6e1d", + "tables": { + "archives": { + "name": "archives", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "workspace_id": { + "name": "workspace_id", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "branch_name": { + "name": "branch_name", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "checkpoint_id": { + "name": "checkpoint_id", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "archived_at": { + "name": "archived_at", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "created_at": { + "name": "created_at", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "(CURRENT_TIMESTAMP)" + }, + "updated_at": { + "name": "updated_at", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "(CURRENT_TIMESTAMP)" + } + }, + "indexes": { + "archives_workspaceId_unique": { + "name": "archives_workspaceId_unique", + "columns": ["workspace_id"], + "isUnique": true + } + }, + "foreignKeys": { + "archives_workspace_id_workspaces_id_fk": { + "name": "archives_workspace_id_workspaces_id_fk", + "tableFrom": "archives", + "tableTo": "workspaces", + "columnsFrom": ["workspace_id"], + "columnsTo": ["id"], + "onDelete": "cascade", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "auth_org_project_preferences": { + "name": "auth_org_project_preferences", + "columns": { + "account_key": { + "name": "account_key", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "cloud_region": { + "name": "cloud_region", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "org_id": { + "name": "org_id", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "last_selected_project_id": { + "name": "last_selected_project_id", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "created_at": { + "name": "created_at", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "(CURRENT_TIMESTAMP)" + }, + "updated_at": { + "name": "updated_at", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "(CURRENT_TIMESTAMP)" + } + }, + "indexes": { + "auth_org_project_account_region_org_idx": { + "name": "auth_org_project_account_region_org_idx", + "columns": ["account_key", "cloud_region", "org_id"], + "isUnique": false + } + }, + "foreignKeys": {}, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "auth_preferences": { + "name": "auth_preferences", + "columns": { + "account_key": { + "name": "account_key", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "cloud_region": { + "name": "cloud_region", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "last_selected_project_id": { + "name": "last_selected_project_id", + "type": "integer", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "last_selected_org_id": { + "name": "last_selected_org_id", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "created_at": { + "name": "created_at", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "(CURRENT_TIMESTAMP)" + }, + "updated_at": { + "name": "updated_at", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "(CURRENT_TIMESTAMP)" + } + }, + "indexes": { + "auth_preferences_account_region_idx": { + "name": "auth_preferences_account_region_idx", + "columns": ["account_key", "cloud_region"], + "isUnique": false + } + }, + "foreignKeys": {}, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "auth_sessions": { + "name": "auth_sessions", + "columns": { + "id": { + "name": "id", + "type": "integer", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "refresh_token_encrypted": { + "name": "refresh_token_encrypted", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "cloud_region": { + "name": "cloud_region", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "selected_project_id": { + "name": "selected_project_id", + "type": "integer", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "scope_version": { + "name": "scope_version", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "created_at": { + "name": "created_at", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "(CURRENT_TIMESTAMP)" + }, + "updated_at": { + "name": "updated_at", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "(CURRENT_TIMESTAMP)" + } + }, + "indexes": {}, + "foreignKeys": {}, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "autoresearch_runs": { + "name": "autoresearch_runs", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "task_id": { + "name": "task_id", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "ended_at": { + "name": "ended_at", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "data": { + "name": "data", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "created_at": { + "name": "created_at", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "(CURRENT_TIMESTAMP)" + }, + "updated_at": { + "name": "updated_at", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "(CURRENT_TIMESTAMP)" + } + }, + "indexes": { + "autoresearch_runs_task_id_idx": { + "name": "autoresearch_runs_task_id_idx", + "columns": ["task_id"], + "isUnique": false + } + }, + "foreignKeys": {}, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "browser_tabs": { + "name": "browser_tabs", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "window_id": { + "name": "window_id", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "dashboard_id": { + "name": "dashboard_id", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "task_id": { + "name": "task_id", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "channel_id": { + "name": "channel_id", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "channel_section": { + "name": "channel_section", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "position": { + "name": "position", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "scroll_state": { + "name": "scroll_state", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "created_at": { + "name": "created_at", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "last_active_at": { + "name": "last_active_at", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + } + }, + "indexes": { + "browser_tabs_window_idx": { + "name": "browser_tabs_window_idx", + "columns": ["window_id"], + "isUnique": false + } + }, + "foreignKeys": { + "browser_tabs_window_id_browser_windows_id_fk": { + "name": "browser_tabs_window_id_browser_windows_id_fk", + "tableFrom": "browser_tabs", + "tableTo": "browser_windows", + "columnsFrom": ["window_id"], + "columnsTo": ["id"], + "onDelete": "cascade", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "browser_windows": { + "name": "browser_windows", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "is_primary": { + "name": "is_primary", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": false + }, + "bounds": { + "name": "bounds", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "active_tab_id": { + "name": "active_tab_id", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "position": { + "name": "position", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": 0 + }, + "created_at": { + "name": "created_at", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "updated_at": { + "name": "updated_at", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + } + }, + "indexes": {}, + "foreignKeys": {}, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "claude_session_imports": { + "name": "claude_session_imports", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "source_session_id": { + "name": "source_session_id", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "imported_session_id": { + "name": "imported_session_id", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "task_id": { + "name": "task_id", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "repo_path": { + "name": "repo_path", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "source_mtime_ms": { + "name": "source_mtime_ms", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "source_size_bytes": { + "name": "source_size_bytes", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "source_last_entry_uuid": { + "name": "source_last_entry_uuid", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "created_at": { + "name": "created_at", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "(CURRENT_TIMESTAMP)" + } + }, + "indexes": { + "claude_session_imports_importedSessionId_unique": { + "name": "claude_session_imports_importedSessionId_unique", + "columns": ["imported_session_id"], + "isUnique": true + }, + "claude_session_imports_source_idx": { + "name": "claude_session_imports_source_idx", + "columns": ["source_session_id"], + "isUnique": false + }, + "claude_session_imports_task_idx": { + "name": "claude_session_imports_task_idx", + "columns": ["task_id"], + "isUnique": false + } + }, + "foreignKeys": {}, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "default_additional_directories": { + "name": "default_additional_directories", + "columns": { + "path": { + "name": "path", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "created_at": { + "name": "created_at", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "(CURRENT_TIMESTAMP)" + } + }, + "indexes": {}, + "foreignKeys": {}, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "repositories": { + "name": "repositories", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "path": { + "name": "path", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "remote_url": { + "name": "remote_url", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "last_accessed_at": { + "name": "last_accessed_at", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "created_at": { + "name": "created_at", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "(CURRENT_TIMESTAMP)" + }, + "updated_at": { + "name": "updated_at", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "(CURRENT_TIMESTAMP)" + } + }, + "indexes": { + "repositories_path_unique": { + "name": "repositories_path_unique", + "columns": ["path"], + "isUnique": true + } + }, + "foreignKeys": {}, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "suspensions": { + "name": "suspensions", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "workspace_id": { + "name": "workspace_id", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "branch_name": { + "name": "branch_name", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "checkpoint_id": { + "name": "checkpoint_id", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "suspended_at": { + "name": "suspended_at", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "reason": { + "name": "reason", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "created_at": { + "name": "created_at", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "(CURRENT_TIMESTAMP)" + }, + "updated_at": { + "name": "updated_at", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "(CURRENT_TIMESTAMP)" + } + }, + "indexes": { + "suspensions_workspaceId_unique": { + "name": "suspensions_workspaceId_unique", + "columns": ["workspace_id"], + "isUnique": true + } + }, + "foreignKeys": { + "suspensions_workspace_id_workspaces_id_fk": { + "name": "suspensions_workspace_id_workspaces_id_fk", + "tableFrom": "suspensions", + "tableTo": "workspaces", + "columnsFrom": ["workspace_id"], + "columnsTo": ["id"], + "onDelete": "cascade", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "task_metadata": { + "name": "task_metadata", + "columns": { + "task_id": { + "name": "task_id", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "pinned_at": { + "name": "pinned_at", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "last_viewed_at": { + "name": "last_viewed_at", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "last_activity_at": { + "name": "last_activity_at", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "archived_at": { + "name": "archived_at", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "created_at": { + "name": "created_at", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "(CURRENT_TIMESTAMP)" + }, + "updated_at": { + "name": "updated_at", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "(CURRENT_TIMESTAMP)" + } + }, + "indexes": {}, + "foreignKeys": {}, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "workspaces": { + "name": "workspaces", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "task_id": { + "name": "task_id", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "repository_id": { + "name": "repository_id", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "mode": { + "name": "mode", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "linked_branch": { + "name": "linked_branch", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "pinned_at": { + "name": "pinned_at", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "last_viewed_at": { + "name": "last_viewed_at", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "last_activity_at": { + "name": "last_activity_at", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "additional_directories": { + "name": "additional_directories", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "'[]'" + }, + "pr_url": { + "name": "pr_url", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "pr_state": { + "name": "pr_state", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "pr_urls": { + "name": "pr_urls", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "'[]'" + }, + "created_at": { + "name": "created_at", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "(CURRENT_TIMESTAMP)" + }, + "updated_at": { + "name": "updated_at", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "(CURRENT_TIMESTAMP)" + } + }, + "indexes": { + "workspaces_taskId_unique": { + "name": "workspaces_taskId_unique", + "columns": ["task_id"], + "isUnique": true + }, + "workspaces_repository_id_idx": { + "name": "workspaces_repository_id_idx", + "columns": ["repository_id"], + "isUnique": false + } + }, + "foreignKeys": { + "workspaces_repository_id_repositories_id_fk": { + "name": "workspaces_repository_id_repositories_id_fk", + "tableFrom": "workspaces", + "tableTo": "repositories", + "columnsFrom": ["repository_id"], + "columnsTo": ["id"], + "onDelete": "set null", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "worktrees": { + "name": "worktrees", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "workspace_id": { + "name": "workspace_id", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "name": { + "name": "name", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "path": { + "name": "path", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "created_at": { + "name": "created_at", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "(CURRENT_TIMESTAMP)" + }, + "updated_at": { + "name": "updated_at", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "(CURRENT_TIMESTAMP)" + } + }, + "indexes": { + "worktrees_workspaceId_unique": { + "name": "worktrees_workspaceId_unique", + "columns": ["workspace_id"], + "isUnique": true + } + }, + "foreignKeys": { + "worktrees_workspace_id_workspaces_id_fk": { + "name": "worktrees_workspace_id_workspaces_id_fk", + "tableFrom": "worktrees", + "tableTo": "workspaces", + "columnsFrom": ["workspace_id"], + "columnsTo": ["id"], + "onDelete": "cascade", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + } + }, + "views": {}, + "enums": {}, + "_meta": { + "schemas": {}, + "tables": {}, + "columns": {} + }, + "internal": { + "indexes": {} + } +} diff --git a/packages/workspace-server/src/db/migrations/meta/_journal.json b/packages/workspace-server/src/db/migrations/meta/_journal.json index b740cd84f5..ec1a4276da 100644 --- a/packages/workspace-server/src/db/migrations/meta/_journal.json +++ b/packages/workspace-server/src/db/migrations/meta/_journal.json @@ -127,6 +127,13 @@ "when": 1783005202636, "tag": "0017_nice_bloodaxe", "breakpoints": true + }, + { + "idx": 18, + "version": "6", + "when": 1783430845937, + "tag": "0018_add_pr_urls", + "breakpoints": true } ] } diff --git a/packages/workspace-server/src/db/repositories/repositories.test.ts b/packages/workspace-server/src/db/repositories/repositories.test.ts index 4f667aea67..e8cb47a1f9 100644 --- a/packages/workspace-server/src/db/repositories/repositories.test.ts +++ b/packages/workspace-server/src/db/repositories/repositories.test.ts @@ -59,6 +59,106 @@ describe("RepositoryRepository round-trip", () => { }); }); +describe("WorkspaceRepository PR cache accumulation", () => { + const PR_1 = "https://github.com/acme/repo/pull/1"; + const PR_2 = "https://github.com/acme/repo/pull/2"; + + beforeEach(() => { + workspaces.create({ taskId: "task-1", repositoryId: null, mode: "local" }); + }); + + it("appends each new PR URL while keeping first-created order", () => { + workspaces.updatePrCache("task-1", { + prUrl: PR_1, + prState: "open", + accumulate: true, + }); + workspaces.updatePrCache("task-1", { + prUrl: PR_2, + prState: "open", + accumulate: true, + }); + + expect(workspaces.getPrUrls("task-1")).toEqual([PR_1, PR_2]); + expect(workspaces.findByTaskId("task-1")?.prUrl).toBe(PR_2); + }); + + it("does not duplicate an already-seen PR URL", () => { + workspaces.updatePrCache("task-1", { + prUrl: PR_1, + prState: "open", + accumulate: true, + }); + workspaces.updatePrCache("task-1", { + prUrl: PR_1, + prState: "merged", + accumulate: true, + }); + + expect(workspaces.getPrUrls("task-1")).toEqual([PR_1]); + }); + + it("keeps accumulated URLs when the current PR clears", () => { + workspaces.updatePrCache("task-1", { + prUrl: PR_1, + prState: "open", + accumulate: true, + }); + workspaces.updatePrCache("task-1", { + prUrl: null, + prState: null, + accumulate: false, + }); + + expect(workspaces.getPrUrls("task-1")).toEqual([PR_1]); + expect(workspaces.findByTaskId("task-1")?.prUrl).toBeNull(); + }); + + it("reads an untouched row as an empty list", () => { + expect(workspaces.getPrUrls("task-1")).toEqual([]); + }); + + it("does not accumulate a non-attributable PR, but still shows it as current", () => { + workspaces.updatePrCache("task-1", { + prUrl: PR_1, + prState: "open", + accumulate: false, + }); + + expect(workspaces.getPrUrls("task-1")).toEqual([]); + expect(workspaces.findByTaskId("task-1")?.prUrl).toBe(PR_1); + }); + + it("promotePrUrl moves the chosen URL to the front", () => { + workspaces.updatePrCache("task-1", { + prUrl: PR_1, + prState: "open", + accumulate: true, + }); + workspaces.updatePrCache("task-1", { + prUrl: PR_2, + prState: "open", + accumulate: true, + }); + + workspaces.promotePrUrl("task-1", PR_2); + + expect(workspaces.getPrUrls("task-1")).toEqual([PR_2, PR_1]); + }); + + it("promotePrUrl adds an unseen URL at the front", () => { + workspaces.updatePrCache("task-1", { + prUrl: PR_1, + prState: "open", + accumulate: true, + }); + + workspaces.promotePrUrl("task-1", PR_2); + + expect(workspaces.getPrUrls("task-1")).toEqual([PR_2, PR_1]); + }); +}); + describe("repository → workspace → worktree round-trip", () => { it("persists the full ownership chain across repositories", () => { const repository = repositories.create({ path: "/repos/twig" }); diff --git a/packages/workspace-server/src/db/repositories/workspace-repository.mock.ts b/packages/workspace-server/src/db/repositories/workspace-repository.mock.ts index 25493aacde..9d99bc3b2a 100644 --- a/packages/workspace-server/src/db/repositories/workspace-repository.mock.ts +++ b/packages/workspace-server/src/db/repositories/workspace-repository.mock.ts @@ -1,7 +1,8 @@ +import { mergePrUrls, promotePrUrl } from "@posthog/shared"; import { type CreateWorkspaceData, type IWorkspaceRepository, - parseDirectories, + parseStringArray, type Workspace, } from "./workspace-repository"; @@ -27,7 +28,7 @@ export function createMockWorkspaceRepository(): MockWorkspaceRepository { ) => { const w = findLiveByTaskId(taskId); if (!w) return; - const next = update(parseDirectories(w.additionalDirectories)); + const next = update(parseStringArray(w.additionalDirectories)); if (next === null) return; workspaces.set(w.id, { ...w, @@ -66,6 +67,7 @@ export function createMockWorkspaceRepository(): MockWorkspaceRepository { additionalDirectories: "[]", prUrl: null, prState: null, + prUrls: "[]", createdAt: now, updatedAt: now, }; @@ -88,6 +90,7 @@ export function createMockWorkspaceRepository(): MockWorkspaceRepository { additionalDirectories: "[]", prUrl: null, prState: null, + prUrls: "[]", createdAt: now, updatedAt: now, }; @@ -134,7 +137,7 @@ export function createMockWorkspaceRepository(): MockWorkspaceRepository { }); }, getAdditionalDirectories: (taskId) => - parseDirectories(findLiveByTaskId(taskId)?.additionalDirectories), + parseStringArray(findLiveByTaskId(taskId)?.additionalDirectories), addAdditionalDirectory: (taskId, path) => { updateDirectoriesForTask(taskId, (current) => current.includes(path) ? null : [...current, path], @@ -148,14 +151,30 @@ export function createMockWorkspaceRepository(): MockWorkspaceRepository { updatePrCache: (taskId, update) => { const w = findLiveByTaskId(taskId); if (!w) return; + const existing = parseStringArray(w.prUrls); + const prUrls = + update.prUrl && update.accumulate + ? mergePrUrls(existing, [update.prUrl]) + : existing; const now = new Date().toISOString(); workspaces.set(w.id, { ...w, prUrl: update.prUrl, prState: update.prState, + prUrls: JSON.stringify(prUrls), updatedAt: now, }); }, + getPrUrls: (taskId) => parseStringArray(findLiveByTaskId(taskId)?.prUrls), + promotePrUrl: (taskId, prUrl) => { + const w = findLiveByTaskId(taskId); + if (!w) return; + workspaces.set(w.id, { + ...w, + prUrls: JSON.stringify(promotePrUrl(parseStringArray(w.prUrls), prUrl)), + updatedAt: new Date().toISOString(), + }); + }, deleteAll: () => { workspaces.clear(); taskIndex.clear(); diff --git a/packages/workspace-server/src/db/repositories/workspace-repository.ts b/packages/workspace-server/src/db/repositories/workspace-repository.ts index 74f024e218..fd9190b74a 100644 --- a/packages/workspace-server/src/db/repositories/workspace-repository.ts +++ b/packages/workspace-server/src/db/repositories/workspace-repository.ts @@ -1,4 +1,4 @@ -import type { WorkspaceMode } from "@posthog/shared"; +import { mergePrUrls, promotePrUrl, type WorkspaceMode } from "@posthog/shared"; import { eq, isNotNull } from "drizzle-orm"; import { inject, injectable } from "inversify"; import { DATABASE_SERVICE } from "../identifiers"; @@ -20,6 +20,7 @@ export interface CreateWorkspaceData { export interface PrCacheUpdate { prUrl: string | null; prState: CachedPrState | null; + accumulate: boolean; } export interface IWorkspaceRepository { @@ -46,10 +47,12 @@ export interface IWorkspaceRepository { addAdditionalDirectory(taskId: string, path: string): void; removeAdditionalDirectory(taskId: string, path: string): void; updatePrCache(taskId: string, update: PrCacheUpdate): void; + getPrUrls(taskId: string): string[]; + promotePrUrl(taskId: string, prUrl: string): void; deleteAll(): void; } -export function parseDirectories(value: string | null | undefined): string[] { +export function parseStringArray(value: string | null | undefined): string[] { if (!value) return []; try { const parsed = JSON.parse(value); @@ -199,7 +202,7 @@ export class WorkspaceRepository implements IWorkspaceRepository { getAdditionalDirectories(taskId: string): string[] { const workspace = this.findByTaskId(taskId); - return parseDirectories(workspace?.additionalDirectories); + return parseStringArray(workspace?.additionalDirectories); } private updateDirectories( @@ -232,17 +235,36 @@ export class WorkspaceRepository implements IWorkspaceRepository { } updatePrCache(taskId: string, update: PrCacheUpdate): void { + const existing = parseStringArray(this.findByTaskId(taskId)?.prUrls); + const prUrls = + update.prUrl && update.accumulate + ? mergePrUrls(existing, [update.prUrl]) + : existing; this.db .update(workspaces) .set({ prUrl: update.prUrl, prState: update.prState, + prUrls: JSON.stringify(prUrls), updatedAt: now(), }) .where(byTaskId(taskId)) .run(); } + getPrUrls(taskId: string): string[] { + return parseStringArray(this.findByTaskId(taskId)?.prUrls); + } + + promotePrUrl(taskId: string, prUrl: string): void { + const prUrls = promotePrUrl(this.getPrUrls(taskId), prUrl); + this.db + .update(workspaces) + .set({ prUrls: JSON.stringify(prUrls), updatedAt: now() }) + .where(byTaskId(taskId)) + .run(); + } + deleteAll(): void { this.db.delete(workspaces).run(); } diff --git a/packages/workspace-server/src/db/schema.ts b/packages/workspace-server/src/db/schema.ts index 5949bf74a7..11081cf0a8 100644 --- a/packages/workspace-server/src/db/schema.ts +++ b/packages/workspace-server/src/db/schema.ts @@ -37,6 +37,7 @@ export const workspaces = sqliteTable( prUrl: text(), /** Cached PR state — values match the `SidebarPrState` union (open/merged/closed/draft). */ prState: text({ enum: ["open", "merged", "closed", "draft"] }), + prUrls: text().notNull().default("[]"), createdAt: createdAt(), updatedAt: updatedAt(), }, diff --git a/packages/workspace-server/src/services/agent/agent.ts b/packages/workspace-server/src/services/agent/agent.ts index 77ff3dea7a..e675e9a5e4 100644 --- a/packages/workspace-server/src/services/agent/agent.ts +++ b/packages/workspace-server/src/services/agent/agent.ts @@ -38,7 +38,11 @@ import { isOpenAIModel, } from "@posthog/agent/gateway-models"; import { getLlmGatewayUrl } from "@posthog/agent/posthog-api"; -import { findPrUrl, wasCreatedRecently } from "@posthog/agent/pr-url-detector"; +import { + findPrUrls, + wasCreatedByLogin, + wasCreatedRecently, +} from "@posthog/agent/pr-url-detector"; import type * as AgentTypes from "@posthog/agent/types"; import { execGh } from "@posthog/git/gh"; import { getCurrentBranch } from "@posthog/git/queries"; @@ -317,9 +321,11 @@ interface ManagedSession { mcpToolApprovals: McpToolApprovals; /** Maps tool keys to their installation for backend approval updates */ toolInstallations: McpToolInstallations; - // Reset per session. `evaluatedPrUrls` dedupes the GitHub lookup per URL. - prAttributed: boolean; + // Reset per session. `evaluatedPrUrls` dedupes the GitHub lookup per URL; + // `prAttachChain` serializes attach writes so concurrent fetch-merge-patch + // cycles can't drop each other's URLs from the accumulated list. evaluatedPrUrls: Set; + prAttachChain: Promise; } /** Get the agent session ID from a managed session, throwing if not set. */ @@ -1090,8 +1096,8 @@ If a repository IS genuinely required, attach one in this priority order: inFlightMcpToolCalls: new Map(), mcpToolApprovals: toolApprovals, toolInstallations, - prAttributed: false, evaluatedPrUrls: new Set(), + prAttachChain: Promise.resolve(), }; this.sessions.set(taskRunId, session); @@ -2052,11 +2058,14 @@ For git operations while detached: session: ManagedSession | undefined, update: unknown, ): void { - if (!session || session.prAttributed) return; - const prUrl = findPrUrl(JSON.stringify(update)); - if (!prUrl || session.evaluatedPrUrls.has(prUrl)) return; - session.evaluatedPrUrls.add(prUrl); - void this.attachPrIfCreatedThisRun(taskRunId, session, prUrl); + if (!session) return; + for (const prUrl of findPrUrls(JSON.stringify(update))) { + if (session.evaluatedPrUrls.has(prUrl)) continue; + session.evaluatedPrUrls.add(prUrl); + session.prAttachChain = session.prAttachChain + .catch(() => {}) + .then(() => this.attachPrIfCreatedThisRun(taskRunId, session, prUrl)); + } } private async attachPrIfCreatedThisRun( @@ -2064,33 +2073,30 @@ For git operations while detached: session: ManagedSession, prUrl: string, ): Promise { - if (session.prAttributed) return; - - const createdAt = await this.fetchPrCreatedAt(session.repoPath, prUrl); - if (!wasCreatedRecently(createdAt, Date.now())) return; - // Re-check after the await: another URL may have attributed while we waited. - if (session.prAttributed) return; + const [attribution, ghLogin] = await Promise.all([ + this.fetchPrAttribution(session.repoPath, prUrl), + this.fetchGhLogin(session.repoPath), + ]); + if (!wasCreatedRecently(attribution.createdAt, Date.now())) return; + if (!wasCreatedByLogin(attribution.author, ghLogin)) return; - session.prAttributed = true; this.log.info("Detected PR URL created during run", { taskRunId, prUrl }); - session.agent - .attachPullRequestToTask(session.taskId, prUrl) - .then(() => { - this.log.info("PR URL attached to task", { - taskRunId, - taskId: session.taskId, - prUrl, - }); - }) - .catch((err) => { - this.log.error("Failed to attach PR URL to task", { - taskRunId, - taskId: session.taskId, - prUrl, - error: err, - }); + try { + await session.agent.attachPullRequestToTask(session.taskId, prUrl); + this.log.info("PR URL attached to task", { + taskRunId, + taskId: session.taskId, + prUrl, }); + } catch (err) { + this.log.error("Failed to attach PR URL to task", { + taskRunId, + taskId: session.taskId, + prUrl, + error: err, + }); + } // The user-initiated PR-creation flow links the current branch to the // workspace atomically (see GitService.createPr). PRs created via bash — @@ -2105,26 +2111,53 @@ For git operations while detached: }); } - /** PR `createdAt` (ISO) via the GitHub CLI, or null if it can't be resolved. */ - private async fetchPrCreatedAt( + /** PR `createdAt` (ISO) and author login via the GitHub CLI; nulls if unresolvable. */ + private async fetchPrAttribution( cwd: string, prUrl: string, - ): Promise { + ): Promise<{ createdAt: string | null; author: string | null }> { try { - const res = await execGh(["pr", "view", prUrl, "--json", "createdAt"], { - cwd, - timeoutMs: 10_000, - }); - if (res.exitCode !== 0) return null; - return ( - (JSON.parse(res.stdout) as { createdAt?: string }).createdAt ?? null + const res = await execGh( + ["pr", "view", prUrl, "--json", "createdAt,author"], + { + cwd, + timeoutMs: 10_000, + }, ); + if (res.exitCode !== 0) return { createdAt: null, author: null }; + const data = JSON.parse(res.stdout) as { + createdAt?: string; + author?: { login?: string }; + }; + return { + createdAt: data.createdAt ?? null, + author: data.author?.login ?? null, + }; } catch (err) { - this.log.debug("Failed to resolve PR createdAt", { prUrl, error: err }); - return null; + this.log.debug("Failed to resolve PR attribution", { prUrl, error: err }); + return { createdAt: null, author: null }; } } + private ghLoginPromise: Promise | null = null; + + private fetchGhLogin(cwd: string): Promise { + this.ghLoginPromise ??= execGh(["api", "user", "--jq", ".login"], { + cwd, + timeoutMs: 10_000, + }) + .then((res) => { + const login = res.exitCode === 0 ? res.stdout.trim() : ""; + if (!login) this.ghLoginPromise = null; + return login || null; + }) + .catch(() => { + this.ghLoginPromise = null; + return null; + }); + return this.ghLoginPromise; + } + /** * Track agent file activity for branch association observability. */ diff --git a/packages/workspace-server/src/services/git/schemas.ts b/packages/workspace-server/src/services/git/schemas.ts index 66941df7eb..11cb2fa718 100644 --- a/packages/workspace-server/src/services/git/schemas.ts +++ b/packages/workspace-server/src/services/git/schemas.ts @@ -309,6 +309,7 @@ export const getPrDetailsByUrlOutput = z.object({ merged: z.boolean(), draft: z.boolean(), headRefName: z.string().nullable(), + title: z.string().nullable(), }); export type PrDetailsByUrlOutput = z.infer; diff --git a/packages/workspace-server/src/services/git/service.ts b/packages/workspace-server/src/services/git/service.ts index 7fcfd86b02..24e41f0125 100644 --- a/packages/workspace-server/src/services/git/service.ts +++ b/packages/workspace-server/src/services/git/service.ts @@ -971,7 +971,7 @@ export class GitService extends TypedEventEmitter { "api", `repos/${pr.owner}/${pr.repo}/pulls/${pr.number}`, "--jq", - "{state,merged,draft,headRefName: .head.ref}", + "{state,merged,draft,headRefName: .head.ref,title}", ]); if (result.exitCode !== 0) { @@ -983,6 +983,7 @@ export class GitService extends TypedEventEmitter { merged: boolean; draft: boolean; headRefName: string | null; + title: string | null; }; return data; diff --git a/packages/workspace-server/src/services/git/task-pr-status.test.ts b/packages/workspace-server/src/services/git/task-pr-status.test.ts index a6213837e6..b2492f7442 100644 --- a/packages/workspace-server/src/services/git/task-pr-status.test.ts +++ b/packages/workspace-server/src/services/git/task-pr-status.test.ts @@ -68,6 +68,7 @@ describe("TaskPrStatusService revalidation PR detection", () => { let workspaceRepo: { findByTaskId: ReturnType; updatePrCache: ReturnType; + getPrUrls: ReturnType; }; beforeEach(() => { @@ -83,6 +84,9 @@ describe("TaskPrStatusService revalidation PR detection", () => { workspaceRepo = { findByTaskId: vi.fn().mockReturnValue({ prUrl: null, prState: null }), updatePrCache: vi.fn(), + getPrUrls: vi + .fn() + .mockReturnValue(["https://github.com/acme/repo/pull/7"]), }; service = new TaskPrStatusService( gitService as unknown as GitService, @@ -109,9 +113,35 @@ describe("TaskPrStatusService revalidation PR detection", () => { expectedCache: { prUrl: "https://github.com/acme/repo/pull/7", prState: "open", + accumulate: false, }, expectedEmit: { prUrl: "https://github.com/acme/repo/pull/7", + prUrls: ["https://github.com/acme/repo/pull/7"], + prState: "open", + }, + }, + { + name: "accumulates a PR detected on a task's dedicated worktree", + taskId: "task-wt", + workspace: { mode: "worktree", worktreePath: "/wt", folderPath: null }, + prStatus: { + prExists: true, + prState: "open", + prUrl: "https://github.com/acme/repo/pull/7", + isDraft: false, + }, + diffStats: { filesChanged: 0 }, + expectedRepoPath: "/wt", + expectDiffStatsCalled: true, + expectedCache: { + prUrl: "https://github.com/acme/repo/pull/7", + prState: "open", + accumulate: true, + }, + expectedEmit: { + prUrl: "https://github.com/acme/repo/pull/7", + prUrls: ["https://github.com/acme/repo/pull/7"], prState: "open", }, }, @@ -123,7 +153,7 @@ describe("TaskPrStatusService revalidation PR detection", () => { diffStats: { filesChanged: 0 }, expectedRepoPath: "/repo", expectDiffStatsCalled: false, - expectedCache: { prUrl: null, prState: null }, + expectedCache: { prUrl: null, prState: null, accumulate: false }, expectedEmit: null, }, { @@ -134,7 +164,7 @@ describe("TaskPrStatusService revalidation PR detection", () => { diffStats: { filesChanged: 3 }, expectedRepoPath: "/wt", expectDiffStatsCalled: true, - expectedCache: { prUrl: null, prState: null }, + expectedCache: { prUrl: null, prState: null, accumulate: false }, expectedEmit: null, }, ])( @@ -183,3 +213,32 @@ describe("TaskPrStatusService revalidation PR detection", () => { }, ); }); + +describe("TaskPrStatusService.setPrimaryPrUrl", () => { + it("emits the promoted url as prUrl even though the row column is stale", () => { + const PR_OLD = "https://github.com/acme/repo/pull/1"; + const PR_NEW = "https://github.com/acme/repo/pull/2"; + const gitService = {} as unknown as GitService; + const workspaceService = { emit: vi.fn() }; + const workspaceRepo = { + promotePrUrl: vi.fn(), + findByTaskId: vi.fn().mockReturnValue({ prUrl: PR_OLD, prState: "open" }), + getPrUrls: vi.fn().mockReturnValue([PR_NEW, PR_OLD]), + }; + const service = new TaskPrStatusService( + gitService, + workspaceRepo as unknown as IWorkspaceRepository, + workspaceService as unknown as WorkspaceService, + ); + + service.setPrimaryPrUrl("task-1", PR_NEW); + + expect(workspaceRepo.promotePrUrl).toHaveBeenCalledWith("task-1", PR_NEW); + expect(workspaceService.emit).toHaveBeenCalledWith("taskPrInfoChanged", { + taskId: "task-1", + prUrl: PR_NEW, + prUrls: [PR_NEW, PR_OLD], + prState: "open", + }); + }); +}); diff --git a/packages/workspace-server/src/services/git/task-pr-status.ts b/packages/workspace-server/src/services/git/task-pr-status.ts index 77c39ab78f..09c98723b6 100644 --- a/packages/workspace-server/src/services/git/task-pr-status.ts +++ b/packages/workspace-server/src/services/git/task-pr-status.ts @@ -42,7 +42,21 @@ export class TaskPrStatusService { getCachedPrUrl(taskId: string): CachedPrUrlOutput { const row = this.workspaceRepo.findByTaskId(taskId); - return { prUrl: row?.prUrl ?? null }; + return { + prUrl: row?.prUrl ?? null, + prUrls: this.workspaceRepo.getPrUrls(taskId), + }; + } + + setPrimaryPrUrl(taskId: string, prUrl: string): void { + this.workspaceRepo.promotePrUrl(taskId, prUrl); + const row = this.workspaceRepo.findByTaskId(taskId); + this.workspaceService.emit("taskPrInfoChanged", { + taskId, + prUrl, + prUrls: this.workspaceRepo.getPrUrls(taskId), + prState: row?.prState ?? null, + }); } private async computeWorktreeHasDiff(taskId: string): Promise { @@ -84,6 +98,7 @@ export class TaskPrStatusService { this.workspaceRepo.updatePrCache(taskId, { prUrl: fresh.prUrl, prState: fresh.prState, + accumulate: fresh.attributable, }); if (cachedPrUrl === fresh.prUrl && cachedPrState === fresh.prState) { @@ -93,6 +108,7 @@ export class TaskPrStatusService { this.workspaceService.emit("taskPrInfoChanged", { taskId, prUrl: fresh.prUrl, + prUrls: this.workspaceRepo.getPrUrls(taskId), prState: fresh.prState, }); }) @@ -112,9 +128,17 @@ export class TaskPrStatusService { prUrl: string | null; prState: SidebarPrState; hasDiff: boolean; + attributable: boolean; }> { const workspace = await this.workspaceService.getWorkspace(taskId); - if (!workspace) return { prUrl: null, prState: null, hasDiff: false }; + if (!workspace) { + return { + prUrl: null, + prState: null, + hasDiff: false, + attributable: false, + }; + } const { mode, worktreePath, folderPath, linkedBranch } = workspace; const isCloud = mode === "cloud"; @@ -127,15 +151,33 @@ export class TaskPrStatusService { prUrl: cloudPrUrl, prState: mapPrState(details.state, details.merged, details.draft), hasDiff: false, + attributable: true, }; } - return { prUrl: cloudPrUrl, prState: null, hasDiff: false }; + return { + prUrl: cloudPrUrl, + prState: null, + hasDiff: false, + attributable: true, + }; } - if (isCloud) return { prUrl: null, prState: null, hasDiff: false }; + if (isCloud) { + return { + prUrl: null, + prState: null, + hasDiff: false, + attributable: false, + }; + } if (repoPath && !fs.existsSync(repoPath)) { - return { prUrl: null, prState: null, hasDiff: false }; + return { + prUrl: null, + prState: null, + hasDiff: false, + attributable: false, + }; } if (linkedBranch && repoPath) { @@ -150,10 +192,16 @@ export class TaskPrStatusService { prUrl, prState: mapPrState(details.state, details.merged, details.draft), hasDiff: false, + attributable: true, }; } } - return { prUrl: null, prState: null, hasDiff: false }; + return { + prUrl: null, + prState: null, + hasDiff: false, + attributable: false, + }; } if (repoPath) { @@ -167,6 +215,7 @@ export class TaskPrStatusService { prStatus.isDraft ?? false, ), hasDiff: false, + attributable: !!worktreePath, }; } @@ -181,10 +230,10 @@ export class TaskPrStatusService { (diffStats?.filesChanged ?? 0) > 0 || (syncStatus?.aheadOfDefault ?? 0) > 0; - return { prUrl: null, prState: null, hasDiff }; + return { prUrl: null, prState: null, hasDiff, attributable: false }; } } - return { prUrl: null, prState: null, hasDiff: false }; + return { prUrl: null, prState: null, hasDiff: false, attributable: false }; } } diff --git a/packages/workspace-server/src/services/workspace/schemas.ts b/packages/workspace-server/src/services/workspace/schemas.ts index 8042e57417..13da3cd1dc 100644 --- a/packages/workspace-server/src/services/workspace/schemas.ts +++ b/packages/workspace-server/src/services/workspace/schemas.ts @@ -128,6 +128,7 @@ export const linkedBranchChangedPayload = z.object({ export const taskPrInfoChangedPayload = z.object({ taskId: z.string(), prUrl: z.string().nullable(), + prUrls: z.array(z.string()).optional(), prState: z.enum(["merged", "open", "draft", "closed"]).nullable(), }); @@ -277,6 +278,12 @@ export const cachedPrUrlInput = z.object({ export const cachedPrUrlOutput = z.object({ prUrl: z.string().nullable(), + prUrls: z.array(z.string()), +}); + +export const setPrimaryPrUrlInput = z.object({ + taskId: z.string(), + prUrl: z.string(), }); export const sidebarPrStateSchema = z