Add "why" context and Slack thread link to agent-created PRs#2534
Conversation
The cloud agent now adds a brief **Why** section to every PR body it prepares — one or two sentences capturing the reason the user asked for the change — and, when the task originated from a Slack thread, a link to that thread. The Slack thread URL is read from the task run state (`slack_thread_url`) and threaded through buildSessionSystemPrompt -> buildCloudSystemPrompt so it can be embedded directly in the instruction. Covers both the repository auto-publish path and the no-repository clone-and-PR path. Generated-By: PostHog Code Task-Id: dcc0de67-21e6-4a7c-9656-b16d9383c508
|
React Doctor found no issues in the changed files. 🎉 Reviewed by React Doctor for commit |
Prompt To Fix All With AIFix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 1
packages/agent/src/server/agent-server.test.ts:1467-1502
**Duplicate prompt construction across tests 1 and 3**
Both "instructs adding a brief Why…" (line 1467) and "falls back to a conditional thread hint…" (line 1492) call `createServer()` with `POSTHOG_CODE_INTERACTION_ORIGIN = "slack"` and `buildCloudSystemPrompt()` with no arguments — they construct the exact same prompt and then each assert a different substring of it. Per the team's OnceAndOnlyOnce principle, these should be merged into a single test (or a parameterised suite) that asserts all expected strings against the same prompt, rather than building it twice. The team also prefers parameterised tests, which would work well here given the four cases share a setup/teardown skeleton.
Reviews (1): Last reviewed commit: "Add "why" context and Slack thread link ..." | Re-trigger Greptile |
Add a brevity instruction to the cloud PR-creation prompt: summarize only the most important changes rather than enumerating every change. Applied to both the repository auto-publish path and the no-repository clone-and-PR path. Generated-By: PostHog Code Task-Id: dcc0de67-21e6-4a7c-9656-b16d9383c508
The brevity instruction is a static string, so a variable added indirection without benefit. Inline it at both PR-creation call sites. Generated-By: PostHog Code Task-Id: dcc0de67-21e6-4a7c-9656-b16d9383c508
Addresses Greptile review: tests that built the identical Slack/no-arg prompt now build it once and assert all expectations together (why context, brevity, and the thread-link fallback). Same for the no-repository path. Also wrap env mutation in try/finally so a failed assertion can't leak POSTHOG_CODE_INTERACTION_ORIGIN into later tests. Generated-By: PostHog Code Task-Id: dcc0de67-21e6-4a7c-9656-b16d9383c508
Prompt To Fix All With AIFix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 1
packages/agent/src/server/agent-server.test.ts:1479-1487
Duplicate prompt construction — no-repository path. Both `"instructs keeping the PR description brief on the no-repository path"` (line 1479) and `"adds Why but omits the thread hint for non-Slack no-repository runs"` (line 1528) call `createServer({ repositoryPath: undefined })` and `buildCloudSystemPrompt()` with no arguments, building the exact same prompt twice. Per OnceAndOnlyOnce, merge them into a single test (or a parameterised row alongside the existing cases) that asserts all expected substrings at once.
```suggestion
it("instructs keeping the PR description brief on the no-repository path", () => {
delete process.env.POSTHOG_CODE_INTERACTION_ORIGIN;
const s = createServer({ repositoryPath: undefined });
const prompt = (
s as unknown as TestableServer
).buildCloudSystemPrompt();
expect(prompt).toContain("open a draft pull request");
expect(prompt).toContain("Keep the PR description brief");
expect(prompt).toContain("**Why**");
expect(prompt).not.toContain("Slack thread");
});
```
Reviews (2): Last reviewed commit: "Consolidate PR-body prompt tests to avoi..." | Re-trigger Greptile |
| const threadLinkInstruction = slackThreadUrl | ||
| ? ` Since this task started from a Slack thread, also link it: ${slackThreadUrl}.` | ||
| : isSlack | ||
| ? " If the task started from a Slack thread, link to that thread." |
There was a problem hiding this comment.
i think we should remove this fallback, assuming the agent won't actually have a way to find the slack URL when it's not provided ?
There was a problem hiding this comment.
Good call — removed the fallback in 3448fd0. Now the thread link is only added when a concrete slack_thread_url is injected (read from the task run state); when it is not available we simply omit it rather than telling the agent to link a thread it has no way to locate.
Per Adam's review: when no concrete slack_thread_url is available the agent has no reliable way to find the thread, so instructing it to "link to that thread" isn't actionable and risks a fabricated link. Only add the thread link when a real URL is injected. Generated-By: PostHog Code Task-Id: dcc0de67-21e6-4a7c-9656-b16d9383c508
Problem
When the PostHog Slack app opens a PR, reviewers only see the diff — not why the change was requested, with no path back to the conversation. PR descriptions also tended to over-explain every change.
Why: Tim asked for agent-created PRs to carry the context behind the request plus a link to the originating Slack thread, and to keep the description brief.
Slack thread: https://posthog.slack.com/archives/C09G8Q32R6F/p1780952041801889
Changes
Updated the cloud PR-creation prompt in
packages/agent/src/server/agent-server.tsso the agent now:slack_thread_url).Applied to both PR-creation paths (repository auto-publish and no-repository clone-and-PR).
How did you test this?
pnpm --filter agent typecheckandbiome check— clean.pnpm exec vitest run src/server/agent-server.test.ts— all 57 pass (env matching CI). Added 6 tests covering the Why, embedded/fallback thread link, brevity, and the no-repository path.Automatic notifications