fix(git): record every PR created for a local task - #3840
Conversation
Local tasks only ever accumulated the PR on the currently checked-out branch, as a side effect of polling getPrStatus during revalidation. A task that opened a second PR on a new branch lost the first one, so the header showed a single PR with no "Other PRs" submenu. Capture the PR at creation time instead: the Create-PR saga now accumulates the created URL through a new accumulatePrUrl seam (CreatePrHost -> GitWorkspaceLookup -> IGitPrStatus -> TaskPrStatusService), independent of which branch is later checked out. Also stop the local agent PR-attribution path from dropping a genuinely-created PR when `gh api user` resolves no login, matching agent-server. Generated-By: PostHog Code Task-Id: 2973f063-1e37-44ea-8d96-3e22a28e10a7
|
Merging to
After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here |
|
React Doctor found no issues in the changed files. 🎉 Reviewed by React Doctor for commit |
PR overviewAll previously flagged issues have been addressed. No open security concerns remain on this pull request. Security reviewNo open security issues remain on this pull request. Fixed/addressed: 1 · PR risk: 0/10 |
Prompt To Fix All With AIFix the following 3 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 3
packages/workspace-server/src/services/git/task-pr-status.ts:74-76
**PR creation order is lost**
When two PRs are created for the same task before either details lookup finishes, their URLs are appended in lookup-completion order rather than creation order. Since the UI treats `prUrls[0]` as primary, the second-created PR is displayed as the primary badge when its lookup resolves first.
### Issue 2 of 3
packages/workspace-server/src/services/agent/agent.ts:2231
**Missing login bypasses attribution**
When `gh api user` transiently fails, any PR created within the five-minute window and present in agent output passes this guard without an author or repository match. This attaches an unrelated recent PR to the task and links the current branch as though the PR were created during this run, producing incorrect PR badge and branch state.
### Issue 3 of 3
apps/code/src/main/di/container.ts:587-591
**PR persistence is fire-and-forget**
When the app shuts down after PR creation while `getPrDetailsByUrl` is still pending, this discarded promise reaches the closed database and rejects without being handled. The create-PR flow has already reported success, but the new PR URL is never persisted and disappears from the task after restart.
Reviews (1): Last reviewed commit: "fix(git): record every PR created for a ..." | Re-trigger Greptile |
| const details = await this.gitService | ||
| .getPrDetailsByUrl(prUrl) | ||
| .catch(() => null); |
There was a problem hiding this comment.
When two PRs are created for the same task before either details lookup finishes, their URLs are appended in lookup-completion order rather than creation order. Since the UI treats prUrls[0] as primary, the second-created PR is displayed as the primary badge when its lookup resolves first.
Knowledge Base Used: workspace-server
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/workspace-server/src/services/git/task-pr-status.ts
Line: 74-76
Comment:
**PR creation order is lost**
When two PRs are created for the same task before either details lookup finishes, their URLs are appended in lookup-completion order rather than creation order. Since the UI treats `prUrls[0]` as primary, the second-created PR is displayed as the primary badge when its lookup resolves first.
**Knowledge Base Used:** [workspace-server](https://app.greptile.com/posthog-org-19734/-/custom-context/knowledge-base/posthog/code/-/docs/workspace-server.md)
How can I resolve this? If you propose a fix, please make it concise.| ]); | ||
| if (!wasCreatedRecently(attribution.createdAt, Date.now())) return; | ||
| if (!wasCreatedByLogin(attribution.author, ghLogin)) return; | ||
| if (ghLogin && !wasCreatedByLogin(attribution.author, ghLogin)) return; |
There was a problem hiding this comment.
Missing login bypasses attribution
When gh api user transiently fails, any PR created within the five-minute window and present in agent output passes this guard without an author or repository match. This attaches an unrelated recent PR to the task and links the current branch as though the PR were created during this run, producing incorrect PR badge and branch state.
Rule Used: When implementing new features, ensure that owners... (source)
Learned From
PostHog/posthog#31236
Knowledge Base Used: workspace-server
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/workspace-server/src/services/agent/agent.ts
Line: 2231
Comment:
**Missing login bypasses attribution**
When `gh api user` transiently fails, any PR created within the five-minute window and present in agent output passes this guard without an author or repository match. This attaches an unrelated recent PR to the task and links the current branch as though the PR were created during this run, producing incorrect PR badge and branch state.
**Rule Used:** When implementing new features, ensure that owners... ([source](https://app.greptile.com/posthog-org-19734/-/custom-context?memory=9655b466-451a-401a-9ba0-5bf3e7b7f9f8))
**Learned From**
[PostHog/posthog#31236](https://github.com/PostHog/posthog/pull/31236)
**Knowledge Base Used:** [workspace-server](https://app.greptile.com/posthog-org-19734/-/custom-context/knowledge-base/posthog/code/-/docs/workspace-server.md)
How can I resolve this? If you propose a fix, please make it concise.| accumulatePrUrl: (taskId, prUrl) => { | ||
| void ctx | ||
| .get<IGitPrStatus>(GIT_PR_STATUS_PROVIDER) | ||
| .accumulatePrUrl(taskId, prUrl); | ||
| }, |
There was a problem hiding this comment.
PR persistence is fire-and-forget
When the app shuts down after PR creation while getPrDetailsByUrl is still pending, this discarded promise reaches the closed database and rejects without being handled. The create-PR flow has already reported success, but the new PR URL is never persisted and disappears from the task after restart.
Rule Used: Always wrap asynchronous calls that may fail, such... (source)
Learned From
PostHog/posthog#32098
Knowledge Base Used:
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/code/src/main/di/container.ts
Line: 587-591
Comment:
**PR persistence is fire-and-forget**
When the app shuts down after PR creation while `getPrDetailsByUrl` is still pending, this discarded promise reaches the closed database and rejects without being handled. The create-PR flow has already reported success, but the new PR URL is never persisted and disappears from the task after restart.
**Rule Used:** Always wrap asynchronous calls that may fail, such... ([source](https://app.greptile.com/posthog-org-19734/-/custom-context?memory=df81f021-48c3-45e4-981f-7348133f5f7a))
**Learned From**
[PostHog/posthog#32098](https://github.com/PostHog/posthog/pull/32098)
**Knowledge Base Used:**
- [PostHog Code Desktop App (`apps/code`)](https://app.greptile.com/posthog-org-19734/-/custom-context/knowledge-base/posthog/code/-/docs/apps-code-desktop.md)
- [workspace-server](https://app.greptile.com/posthog-org-19734/-/custom-context/knowledge-base/posthog/code/-/docs/workspace-server.md)
How can I resolve this? If you propose a fix, please make it concise.Co-authored-by: veria-ai[bot] <224490171+veria-ai[bot]@users.noreply.github.com>
…l-task-multiple-prs
…:posthog/code into posthog-code/fix-local-task-multiple-prs
Restores `if (ghLogin && !wasCreatedByLogin(...))`, undoing the veria-ai suggestion (a4e5cf8) that dropped the `ghLogin &&` condition. The unconditional form is exactly what broke bot/signal PR attribution in #3227: when `gh api user` resolves no login (App tokens, impersonated sessions), wasCreatedByLogin returns false and every genuinely run-created PR is discarded. Mirrors the cloud path fixed in #3302. Generated-By: PostHog Code Task-Id: 2973f063-1e37-44ea-8d96-3e22a28e10a7
Problem
On a local (desktop worktree) task that produced two PRs, the task header shows only one PR badge (the latest / current branch) with no "Other PRs" submenu. Multi‑PR‑per‑task support exists (#3227) via a
pr_urlsarray, but for local tasks nothing recorded a PR at creation time.pr_urlswas only populated as a side effect of pollinggetPrStatuson the currently checked‑out branch during revalidation. So if the agent opens PR #1 on branch A, then continues and opens PR #2 on branch B before any revalidation fires while A is current, PR #1 is never recorded — only PR #2 survives, and there's nothing to populate the submenu.Fix
Event‑driven capture (primary). The Create‑PR saga now accumulates the created PR URL the moment the PR is made, independent of which branch is later checked out — mirroring how cloud already works. New
accumulatePrUrlseam:CreatePrHost→GitWorkspaceLookup→IGitPrStatus→TaskPrStatusService.accumulatePrUrl(doesupdatePrCache(..., accumulate: true)+ emitstaskPrInfoChanged), wired in the composition root. No schema change — thepr_urlscolumn already exists. Array order is preserved, so promote‑to‑primary and dedupe still hold.Local agent hardening.
attachPrIfCreatedThisRunenforced the author match unconditionally, so a genuinely agent‑created PR was silently dropped whenevergh api userreturned no login. Now the author check applies only when a login actually resolved (matchingagent-server.ts); the 5‑minute recency gate still scopes attribution.Tests
TaskPrStatusService.accumulatePrUrlsuite: first PR sets primary +pr_urls=[A]; second distinct PR yields[A,B]; re‑accumulating A is idempotent; no‑op when the task has no workspace row.accumulatePrUrlis called on success.resolveTaskPrUrlsregression case for two accumulated local PRs surfacing an "Other PRs" entry.Verification
@posthog/core,@posthog/workspace-server,@posthog/host-router,apps/code— clean.task-pr-status(12) — pass.noRestrictedImportsinpackages/core; no new host‑boundary violations.Created with PostHog Code