Skip to content

fix(agent): only attach PR URLs from gh pr create#1964

Merged
adboio merged 1 commit intomainfrom
adam/gate-pr-url-detection-on-gh-pr-create
May 5, 2026
Merged

fix(agent): only attach PR URLs from gh pr create#1964
adboio merged 1 commit intomainfrom
adam/gate-pr-url-detection-on-gh-pr-create

Conversation

@adboio
Copy link
Copy Markdown
Contributor

@adboio adboio commented Apr 30, 2026

Summary

  • Gate PR-URL attachment on the originating bash command containing gh pr create, eliminating false positives from gh pr view, gh search prs, gh pr list, etc.
  • Extract the detection logic into a shared pr-url-detector helper, replacing two byte-identical implementations in the desktop service and the cloud agent-server.
  • Thread the bash command through _meta.claudeCode.bashCommand in the SDK→ACP converter so consumers don't have to correlate tool_calltool_call_update.

Why

Today any GitHub PR URL printed by a bash tool gets latched onto TaskRun.output.pr_url. That fires downstream Slack notifications and PR-status UI off URLs the agent merely referenced (e.g. when looking up a historical PR). A real instance of this misfire produced four "Pull request opened" Slack messages from a single gh pr view call.

The most robust signal that a URL is a PR the agent just created is: the command was gh pr create. git push only ever prints pull/new/<branch>, which doesn't match the URL regex; gh pr edit/merge/view/list/search operate on existing PRs.

Test plan

  • `pnpm --filter @posthog/agent test` — 13 new helper tests + 4 new agent-server regression cases pass (317 / 317)
  • `pnpm --filter code test` — desktop suite green (982 / 982)
  • `pnpm --filter @posthog/agent typecheck` and `pnpm --filter code typecheck` clean
  • `pnpm lint` clean
  • Manual end-to-end smoke: with `pnpm dev`, ask the agent to look up an existing PR via `gh pr view ` and confirm `pr_url` is not attached and no Slack notification fires; then ask it to create a PR via `gh pr create` and confirm the existing flow still works.

Generated-By: PostHog Code
Task-Id: 302a50dd-3e6e-4f4c-abbf-4ca0e0f9250d

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Apr 30, 2026

Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
packages/agent/src/pr-url-detector.test.ts:7-137
**Prefer parameterised tests**

All 13 test cases follow the exact same shape — vary `toolName`, `bashCommand`, `toolResponse`/`content`, and expect a URL or `null`. Per the project's simplicity rules, this should be a single `it.each()` table rather than 13 separate `it()` blocks.

```typescript
it.each([
  ["gh pr create (string response)", "Bash", 'gh pr create --title "x"', `${PR_URL}\n`, undefined, PR_URL],
  ["gh pr create (object stdout)",   "Bash", "gh pr create --fill",      { stdout: `${PR_URL}\n` }, undefined, PR_URL],
  ["gh pr view ignored",             "Bash", `gh pr view ${PR_URL}`,      { stdout: PR_URL },        undefined, null],
  // … remaining rows
])(
  "%s",
  (_desc, toolName, bashCommand, toolResponse, content, expected) => {
    expect(extractCreatedPrUrl({ toolName, bashCommand, toolResponse, content }))
      .toBe(expected);
  },
);
```

This also makes it trivial to add new cases without boilerplate.

Reviews (1): Last reviewed commit: "fix(agent): only attach PR URLs from gh ..." | Re-trigger Greptile

Comment thread packages/agent/src/pr-url-detector.test.ts Outdated
@adboio adboio requested review from a team and joshsny April 30, 2026 22:04
Copy link
Copy Markdown
Contributor

@joshsny joshsny left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thank youuuu

Copy link
Copy Markdown
Contributor Author

adboio commented May 1, 2026

will merge after a bit more testing 🫡

Today any GitHub PR URL appearing in a bash tool's output gets attached
to TaskRun.output.pr_url, which over-catches: gh pr view, gh search prs,
gh pr list, and similar lookups all latch unrelated PRs onto the run and
trigger downstream effects (Slack notifications, UI PR-status rendering).

Gate the existing detection on the originating bash command containing
gh pr create. The bash command is threaded onto _meta.claudeCode in
sdk-to-acp.ts so consumers don't have to correlate tool_call to
tool_call_update messages. The detection regex/text extraction is now
shared between the desktop service and the cloud agent-server via a new
pr-url-detector helper, replacing two byte-identical implementations.

Generated-By: PostHog Code
Task-Id: 302a50dd-3e6e-4f4c-abbf-4ca0e0f9250d
@adboio adboio force-pushed the adam/gate-pr-url-detection-on-gh-pr-create branch from 7edf69a to dd91980 Compare May 5, 2026 20:39
Copy link
Copy Markdown
Contributor Author

adboio commented May 5, 2026

This stack of pull requests is managed by Graphite. Learn more about stacking.

@adboio adboio merged commit fb9ab4c into main May 5, 2026
16 checks passed
Copy link
Copy Markdown
Contributor Author

adboio commented May 5, 2026

Merge activity

@adboio adboio deleted the adam/gate-pr-url-detection-on-gh-pr-create branch May 5, 2026 20:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants