Skip to content

Commit 2ecc6b6

Browse files
authored
feat(git): allow multiple PRs per task with an Other PRs submenu (#3227)
1 parent a9514ed commit 2ecc6b6

43 files changed

Lines changed: 2481 additions & 150 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

packages/agent/src/agent.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { buildPrOutput, mergePrUrls, readPrUrls } from "@posthog/shared";
12
import {
23
createAcpConnection,
34
type InProcessAcpConnection,
@@ -167,8 +168,13 @@ export class Agent {
167168
throw error;
168169
}
169170

171+
const freshOutput = await this.posthogAPI
172+
.getTaskRun(taskId, this.taskRunId)
173+
.then((run) => run.output)
174+
.catch(() => null);
175+
const urls = mergePrUrls(readPrUrls(freshOutput), [prUrl]);
170176
const updates: TaskRunUpdate = {
171-
output: { pr_url: prUrl },
177+
output: buildPrOutput(freshOutput, urls),
172178
};
173179
if (branchName) {
174180
updates.branch = branchName;

packages/agent/src/pr-url-detector.test.ts

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,10 @@
11
import { describe, expect, it } from "vitest";
2-
import { findPrUrl, wasCreatedRecently } from "./pr-url-detector";
2+
import {
3+
findPrUrl,
4+
findPrUrls,
5+
wasCreatedByLogin,
6+
wasCreatedRecently,
7+
} from "./pr-url-detector";
38

49
const PR_URL = "https://github.com/PostHog/posthog.com/pull/17764";
510

@@ -33,6 +38,38 @@ describe("findPrUrl", () => {
3338
});
3439
});
3540

41+
describe("findPrUrls", () => {
42+
const OTHER = "https://github.com/PostHog/posthog/pull/99";
43+
44+
it("finds every PR URL in one chunk, in order", () => {
45+
expect(findPrUrls(`Opened ${PR_URL} and ${OTHER} today`)).toEqual([
46+
PR_URL,
47+
OTHER,
48+
]);
49+
});
50+
51+
it("dedupes repeated mentions of the same PR", () => {
52+
expect(findPrUrls(`${PR_URL} again: ${PR_URL}`)).toEqual([PR_URL]);
53+
});
54+
55+
it("returns an empty array when there is no PR URL", () => {
56+
expect(findPrUrls("nothing here")).toEqual([]);
57+
});
58+
});
59+
60+
describe("wasCreatedByLogin", () => {
61+
it.each([
62+
["run-owner", "run-owner", true],
63+
["Run-Owner", "run-owner", true],
64+
["someone-else", "run-owner", false],
65+
[null, "run-owner", false],
66+
["run-owner", null, false],
67+
["", "", false],
68+
] as const)("author=%s login=%s -> %s", (author, login, expected) => {
69+
expect(wasCreatedByLogin(author, login)).toBe(expected);
70+
});
71+
});
72+
3673
describe("wasCreatedRecently", () => {
3774
const now = new Date("2026-06-18T17:00:00Z").getTime();
3875
const maxAge = 15 * 60 * 1000;

packages/agent/src/pr-url-detector.ts

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,24 @@
1-
const PR_URL_REGEX = /https:\/\/github\.com\/[^/\s"]+\/[^/\s"]+\/pull\/\d+/;
1+
const PR_URL_REGEX = /https:\/\/github\.com\/[^/\s"]+\/[^/\s"]+\/pull\/\d+/g;
22

33
// A fixed window (not "since run start") so a PR the agent merely views on a
44
// long run is too old to be mistaken for one it just created.
55
export const PR_CREATION_RECENCY_MS = 5 * 60 * 1000;
66

77
export function findPrUrl(text: string): string | null {
8-
return text.match(PR_URL_REGEX)?.[0] ?? null;
8+
return findPrUrls(text)[0] ?? null;
9+
}
10+
11+
export function findPrUrls(text: string): string[] {
12+
return [...new Set(text.match(PR_URL_REGEX) ?? [])];
13+
}
14+
15+
// Fails closed on missing/invalid input so we never attribute on uncertainty.
16+
export function wasCreatedByLogin(
17+
author: string | null | undefined,
18+
login: string | null | undefined,
19+
): boolean {
20+
if (!author || !login) return false;
21+
return author.toLowerCase() === login.toLowerCase();
922
}
1023

1124
// Fails closed on missing/invalid input so we never attribute on uncertainty.

packages/agent/src/server/agent-server.test.ts

Lines changed: 57 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1864,18 +1864,45 @@ describe("AgentServer HTTP Mode", () => {
18641864
p: JwtPayload,
18651865
u: Record<string, unknown> | undefined,
18661866
): void;
1867-
fetchPrCreatedAt(url: string): Promise<string | null>;
1867+
fetchPrAttribution(
1868+
url: string,
1869+
): Promise<{ createdAt: string | null; author: string | null }>;
1870+
fetchGhLogin(): Promise<string | null>;
18681871
detectedPrUrl: string | null;
1869-
posthogAPI: { updateTaskRun: ReturnType<typeof vi.fn> };
1872+
posthogAPI: {
1873+
getTaskRun: ReturnType<typeof vi.fn>;
1874+
updateTaskRun: ReturnType<typeof vi.fn>;
1875+
};
18701876
};
18711877

18721878
const justNow = () => new Date().toISOString();
18731879
const longAgo = "2020-01-01T00:00:00Z";
1880+
const GH_LOGIN = "run-owner";
18741881

1875-
const setup = (prCreatedAt: string | null): PrTestServer => {
1882+
const setup = (
1883+
prCreatedAt: string | null,
1884+
prAuthor: string | null = GH_LOGIN,
1885+
): PrTestServer => {
18761886
const s = createServer() as unknown as PrTestServer;
1877-
s.fetchPrCreatedAt = vi.fn(async () => prCreatedAt);
1878-
s.posthogAPI = { updateTaskRun: vi.fn(async () => ({})) };
1887+
s.fetchPrAttribution = vi.fn(async () => ({
1888+
createdAt: prCreatedAt,
1889+
author: prAuthor,
1890+
}));
1891+
s.fetchGhLogin = vi.fn(async () => GH_LOGIN);
1892+
let storedOutput: Record<string, unknown> | null = null;
1893+
s.posthogAPI = {
1894+
getTaskRun: vi.fn(async () => ({ output: storedOutput })),
1895+
updateTaskRun: vi.fn(
1896+
async (
1897+
_taskId: string,
1898+
_runId: string,
1899+
updates: { output: Record<string, unknown> },
1900+
) => {
1901+
storedOutput = updates.output;
1902+
return {};
1903+
},
1904+
),
1905+
};
18791906
return s;
18801907
};
18811908

@@ -1886,7 +1913,7 @@ describe("AgentServer HTTP Mode", () => {
18861913
s.maybeAttachCreatedPr(payload, terminalUpdate(PR_URL));
18871914
await flush();
18881915
expect(s.posthogAPI.updateTaskRun).toHaveBeenCalledWith("t", "r", {
1889-
output: { pr_url: PR_URL },
1916+
output: { pr_url: PR_URL, pr_urls: [PR_URL] },
18901917
});
18911918
expect(s.detectedPrUrl).toBe(PR_URL);
18921919
});
@@ -1903,7 +1930,7 @@ describe("AgentServer HTTP Mode", () => {
19031930
const s = setup(justNow());
19041931
s.maybeAttachCreatedPr(payload, { sessionUpdate: "agent_thought_chunk" });
19051932
await flush();
1906-
expect(s.fetchPrCreatedAt).not.toHaveBeenCalled();
1933+
expect(s.fetchPrAttribution).not.toHaveBeenCalled();
19071934
expect(s.posthogAPI.updateTaskRun).not.toHaveBeenCalled();
19081935
});
19091936

@@ -1913,20 +1940,19 @@ describe("AgentServer HTTP Mode", () => {
19131940
s.maybeAttachCreatedPr(payload, terminalUpdate(PR_URL));
19141941
s.maybeAttachCreatedPr(payload, terminalUpdate(PR_URL));
19151942
await flush();
1916-
expect(s.fetchPrCreatedAt).toHaveBeenCalledTimes(1);
1943+
expect(s.fetchPrAttribution).toHaveBeenCalledTimes(1);
19171944
expect(s.posthogAPI.updateTaskRun).toHaveBeenCalledTimes(1);
19181945
});
19191946

1920-
it("attributes the most recent PR when a run opens several, in detection order", async () => {
1921-
// output.pr_url holds one value; the latest PR the run created is the useful one.
1947+
it("accumulates every PR a run opens, keeping the first as primary", async () => {
19221948
const s = setup(justNow());
19231949
const second = "https://github.com/PostHog/posthog.com/pull/17765";
19241950
s.maybeAttachCreatedPr(payload, terminalUpdate(PR_URL));
19251951
s.maybeAttachCreatedPr(payload, terminalUpdate(second));
19261952
await flush();
19271953
expect(s.posthogAPI.updateTaskRun).toHaveBeenCalledTimes(2);
19281954
expect(s.posthogAPI.updateTaskRun).toHaveBeenLastCalledWith("t", "r", {
1929-
output: { pr_url: second },
1955+
output: { pr_url: PR_URL, pr_urls: [PR_URL, second] },
19301956
});
19311957
expect(s.detectedPrUrl).toBe(second);
19321958
});
@@ -1935,15 +1961,32 @@ describe("AgentServer HTTP Mode", () => {
19351961
const viewed = "https://github.com/PostHog/posthog.com/pull/1";
19361962
// The created PR reads as recent; the later, merely-viewed PR reads as old.
19371963
const s = setup(justNow());
1938-
s.fetchPrCreatedAt = vi.fn(async (url: string) =>
1939-
url === PR_URL ? justNow() : longAgo,
1940-
);
1964+
s.fetchPrAttribution = vi.fn(async (url: string) => ({
1965+
createdAt: url === PR_URL ? justNow() : longAgo,
1966+
author: GH_LOGIN,
1967+
}));
19411968
s.maybeAttachCreatedPr(payload, terminalUpdate(PR_URL));
19421969
s.maybeAttachCreatedPr(payload, terminalUpdate(viewed));
19431970
await flush();
19441971
expect(s.detectedPrUrl).toBe(PR_URL);
19451972
expect(s.posthogAPI.updateTaskRun).toHaveBeenCalledTimes(1);
19461973
});
1974+
1975+
it("does not attribute a fresh PR authored by someone else (merely viewed)", async () => {
1976+
const s = setup(justNow(), "someone-else");
1977+
s.maybeAttachCreatedPr(payload, terminalUpdate(PR_URL));
1978+
await flush();
1979+
expect(s.posthogAPI.updateTaskRun).not.toHaveBeenCalled();
1980+
expect(s.detectedPrUrl).toBeNull();
1981+
});
1982+
1983+
it("fails closed when the run's GitHub identity cannot be resolved", async () => {
1984+
const s = setup(justNow());
1985+
s.fetchGhLogin = vi.fn(async () => null);
1986+
s.maybeAttachCreatedPr(payload, terminalUpdate(PR_URL));
1987+
await flush();
1988+
expect(s.posthogAPI.updateTaskRun).not.toHaveBeenCalled();
1989+
});
19471990
});
19481991

19491992
describe("buildCloudSystemPrompt", () => {

packages/agent/src/server/agent-server.ts

Lines changed: 74 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,12 @@ import {
1616
import { type ServerType, serve } from "@hono/node-server";
1717
import { execGh } from "@posthog/git/gh";
1818
import { getCurrentBranch } from "@posthog/git/queries";
19-
import type { Adapter } from "@posthog/shared";
19+
import {
20+
type Adapter,
21+
buildPrOutput,
22+
mergePrUrls,
23+
readPrUrls,
24+
} from "@posthog/shared";
2025
import { unzipSync } from "fflate";
2126
import { Hono } from "hono";
2227
import { z } from "zod";
@@ -45,7 +50,11 @@ import type { PermissionMode } from "../execution-mode";
4550
import { DEFAULT_CODEX_MODEL, fetchGatewayModels } from "../gateway-models";
4651
import { HandoffCheckpointTracker } from "../handoff-checkpoint";
4752
import { PostHogAPIClient } from "../posthog-api";
48-
import { findPrUrl, wasCreatedRecently } from "../pr-url-detector";
53+
import {
54+
findPrUrls,
55+
wasCreatedByLogin,
56+
wasCreatedRecently,
57+
} from "../pr-url-detector";
4958
import {
5059
formatConversationForResume,
5160
type ResumeState,
@@ -3438,13 +3447,14 @@ ${signedCommitInstructions}${prLinkInstructions}${shellEfficiencyInstructions}
34383447
update: Record<string, unknown> | undefined,
34393448
): void {
34403449
if (!update) return;
3441-
const prUrl = findPrUrl(JSON.stringify(update));
3442-
if (!prUrl || this.evaluatedPrUrls.has(prUrl)) return;
3443-
this.evaluatedPrUrls.add(prUrl);
3444-
// Chain so attributions run in detection order; later PRs overwrite earlier ones.
3445-
this.prAttributionChain = this.prAttributionChain
3446-
.catch(() => {})
3447-
.then(() => this.attachPrIfCreatedThisRun(payload, prUrl));
3450+
for (const prUrl of findPrUrls(JSON.stringify(update))) {
3451+
if (this.evaluatedPrUrls.has(prUrl)) continue;
3452+
this.evaluatedPrUrls.add(prUrl);
3453+
// Chain so attributions run in detection order; later PRs append after earlier ones.
3454+
this.prAttributionChain = this.prAttributionChain
3455+
.catch(() => {})
3456+
.then(() => this.attachPrIfCreatedThisRun(payload, prUrl));
3457+
}
34483458
}
34493459

34503460
private async attachPrIfCreatedThisRun(
@@ -3454,9 +3464,13 @@ ${signedCommitInstructions}${prLinkInstructions}${shellEfficiencyInstructions}
34543464
// Already the attributed PR (e.g. seeded from a Slack notification, or re-detected).
34553465
if (prUrl === this.detectedPrUrl) return;
34563466

3457-
let createdAt: string | null;
3467+
let attribution: { createdAt: string | null; author: string | null };
3468+
let ghLogin: string | null;
34583469
try {
3459-
createdAt = await this.fetchPrCreatedAt(prUrl);
3470+
[attribution, ghLogin] = await Promise.all([
3471+
this.fetchPrAttribution(prUrl),
3472+
this.fetchGhLogin(),
3473+
]);
34603474
} catch (err) {
34613475
this.logger.debug("PR attribution lookup failed", {
34623476
runId: payload.run_id,
@@ -3466,14 +3480,21 @@ ${signedCommitInstructions}${prLinkInstructions}${shellEfficiencyInstructions}
34663480
return;
34673481
}
34683482

3469-
// Only attribute PRs created during this run, not ones the agent merely viewed.
3470-
if (!wasCreatedRecently(createdAt, Date.now())) return;
3483+
// Only attribute PRs created during this run by this run's own GitHub
3484+
// identity — not ones the agent merely viewed.
3485+
if (!wasCreatedRecently(attribution.createdAt, Date.now())) return;
3486+
if (!wasCreatedByLogin(attribution.author, ghLogin)) return;
34713487

34723488
this.detectedPrUrl = prUrl;
34733489

34743490
try {
3491+
const freshOutput = await this.posthogAPI
3492+
.getTaskRun(payload.task_id, payload.run_id)
3493+
.then((run) => run.output)
3494+
.catch(() => null);
3495+
const urls = mergePrUrls(readPrUrls(freshOutput), [prUrl]);
34753496
await this.posthogAPI.updateTaskRun(payload.task_id, payload.run_id, {
3476-
output: { pr_url: prUrl },
3497+
output: buildPrOutput(freshOutput, urls),
34773498
});
34783499
this.logger.debug("Attributed created PR to task run", {
34793500
taskId: payload.task_id,
@@ -3490,21 +3511,50 @@ ${signedCommitInstructions}${prLinkInstructions}${shellEfficiencyInstructions}
34903511
}
34913512
}
34923513

3493-
private async fetchPrCreatedAt(prUrl: string): Promise<string | null> {
3494-
const res = await execGh(["pr", "view", prUrl, "--json", "createdAt"], {
3495-
cwd: this.config.repositoryPath,
3496-
timeoutMs: 10_000,
3497-
});
3498-
if (res.exitCode !== 0) return null;
3514+
private async fetchPrAttribution(
3515+
prUrl: string,
3516+
): Promise<{ createdAt: string | null; author: string | null }> {
3517+
const res = await execGh(
3518+
["pr", "view", prUrl, "--json", "createdAt,author"],
3519+
{
3520+
cwd: this.config.repositoryPath,
3521+
timeoutMs: 10_000,
3522+
},
3523+
);
3524+
if (res.exitCode !== 0) return { createdAt: null, author: null };
34993525
try {
3500-
return (
3501-
(JSON.parse(res.stdout) as { createdAt?: string }).createdAt ?? null
3502-
);
3526+
const data = JSON.parse(res.stdout) as {
3527+
createdAt?: string;
3528+
author?: { login?: string };
3529+
};
3530+
return {
3531+
createdAt: data.createdAt ?? null,
3532+
author: data.author?.login ?? null,
3533+
};
35033534
} catch {
3504-
return null;
3535+
return { createdAt: null, author: null };
35053536
}
35063537
}
35073538

3539+
private ghLoginPromise: Promise<string | null> | null = null;
3540+
3541+
private fetchGhLogin(): Promise<string | null> {
3542+
this.ghLoginPromise ??= execGh(["api", "user", "--jq", ".login"], {
3543+
cwd: this.config.repositoryPath,
3544+
timeoutMs: 10_000,
3545+
})
3546+
.then((res) => {
3547+
const login = res.exitCode === 0 ? res.stdout.trim() : "";
3548+
if (!login) this.ghLoginPromise = null;
3549+
return login || null;
3550+
})
3551+
.catch(() => {
3552+
this.ghLoginPromise = null;
3553+
return null;
3554+
});
3555+
return this.ghLoginPromise;
3556+
}
3557+
35083558
private async cleanupSession({
35093559
completeEventStream = false,
35103560
}: {

packages/core/src/git-interaction/gitInteractionService.test.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -276,6 +276,7 @@ describe("GitInteractionService.runCreatePr", () => {
276276
expect(effects.attachPrUrlToTask).toHaveBeenCalledWith(
277277
"t",
278278
"https://example.test/pr/1",
279+
undefined,
279280
);
280281
if (result.outcome === "success") {
281282
expect(result.linkedBranchName).toBe("feature-x");

0 commit comments

Comments
 (0)