Skip to content

feat: auto-close issues when linked PR merges#4425

Open
mv50000 wants to merge 1 commit intopaperclipai:masterfrom
mv50000:sec-52/auto-close-on-pr-merge
Open

feat: auto-close issues when linked PR merges#4425
mv50000 wants to merge 1 commit intopaperclipai:masterfrom
mv50000:sec-52/auto-close-on-pr-merge

Conversation

@mv50000
Copy link
Copy Markdown

@mv50000 mv50000 commented Apr 24, 2026

Summary

  • Adds server-side GitHub webhook handler at POST /api/github/webhooks that auto-closes in_review issues when their linked PR merges
  • Two matching strategies: (1) parse issue identifiers (SEC-52, OLL-89) from PR title/body, (2) lookup issue_work_products by PR URL
  • HMAC signature verification via GITHUB_WEBHOOK_SECRET env var
  • Activity log entries for audit trail

How it works

  1. GitHub sends pull_request.closed webhook with merged: true
  2. Handler verifies HMAC-SHA256 signature
  3. Extracts {PREFIX}-{NUMBER} identifiers from PR title and body
  4. Queries issue_work_products table for PRs linked by URL
  5. Transitions matched in_review issues to done
  6. Posts auto-close comment: "Auto-closed: PR #X merged in org/repo"

Test plan

  • 8 unit tests covering signature verification, event filtering, identifier extraction, and auto-close behavior
  • Manual: configure GITHUB_WEBHOOK_SECRET, create test webhook in GitHub repo settings
  • Verify non-PR events return { ignored: true }
  • Verify merged PR with matching identifier auto-closes in_review issue
  • Verify work product URL match also triggers auto-close

Closes SEC-52

🤖 Generated with Claude Code

Add server-side GitHub webhook handler at POST /api/github/webhooks that
listens for pull_request.closed events with merged=true. When a PR merges,
the handler extracts issue identifiers (e.g. SEC-52, OLL-89) from the PR
title and body, and also checks issue_work_products for linked PRs by URL.
Any matched issues in in_review status are automatically transitioned to
done with an auto-close comment.

Closes SEC-52

Co-Authored-By: Paperclip <noreply@paperclip.ing>
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Apr 24, 2026

Greptile Summary

This PR adds a GitHub webhook handler at POST /api/github/webhooks that auto-closes in_review issues when their linked PR merges, using HMAC-SHA256 signature verification and two matching strategies (issue identifier extraction and issue_work_products URL lookup). The implementation is solid — rawBody is already populated globally by the existing express.json verify callback, the boardMutationGuard and actorMiddleware do not interfere with unauthenticated webhook requests, and the 8 unit tests cover the core paths well.

The PR description is missing the required sections from the PR template defined in CONTRIBUTING.md: a Thinking Path, What Changed, Verification, Risks, and Model Used sections are all absent. Please fill out the PR template fully before merging.

Confidence Score: 4/5

Functionally safe to merge, but blocked on CONTRIBUTING.md requirement to fill out the full PR template (Thinking Path, Model Used, etc.).

All inline findings are P2 (duplicated close logic, no per-issue error isolation). The implementation is correct. Score is 4 rather than 5 solely because the PR template sections required by CONTRIBUTING.md are missing, which is an explicit merge requirement per the contribution guidelines.

server/src/routes/github-webhooks.ts — duplicated close logic and missing per-issue error handling.

Important Files Changed

Filename Overview
server/src/routes/github-webhooks.ts New webhook handler with correct HMAC-SHA256 signature verification and two-strategy issue matching; close logic is duplicated and not error-isolated per issue.
server/src/tests/github-webhook-routes.test.ts 8 unit tests covering signature verification, event filtering, identifier extraction, and the no-match case; coverage is solid for the core paths.
server/src/app.ts Registers the new webhook router; rawBody is already populated globally by the existing express.json verify callback, so signature verification will work correctly.
server/src/routes/index.ts Exports the new githubWebhookRoutes factory alongside existing route exports; no issues.
.env.example Adds commented-out GITHUB_WEBHOOK_SECRET entry with helpful context comment; no issues.
Prompt To Fix All With AI
This is a comment left during a code review.
Path: server/src/routes/github-webhooks.ts
Line: 80-101

Comment:
**Duplicate close-issue logic**

The update + `addComment` + `logActivity` block is copy-pasted verbatim for both matching strategies (lines 80–101 and lines 126–149). Extracting it into a shared helper would reduce the maintenance surface and the risk of the two paths diverging silently in the future.

```ts
async function closeIssue(
  svc: ReturnType<typeof issueService>,
  db: Db,
  issue: { id: string; identifier: string; companyId: string },
  { prNumber, prUrl, repoFullName }: { prNumber: number; prUrl: string; repoFullName: string },
) {
  await svc.update(issue.id, { status: "done" });
  await svc.addComment(
    issue.id,
    `Auto-closed: PR [#${prNumber}](${prUrl}) merged in \`${repoFullName}\``,
    { agentId: undefined, userId: undefined, runId: null },
  );
  await logActivity(db, {
    companyId: issue.companyId,
    actorType: "system",
    actorId: "system",
    action: "issue.auto_closed_pr_merged",
    entityType: "issue",
    entityId: issue.id,
    details: { identifier: issue.identifier, prUrl, prNumber, repoFullName },
  });
}
```

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: server/src/routes/github-webhooks.ts
Line: 80-103

Comment:
**No error isolation between close steps**

`update`, `addComment`, and `logActivity` are called sequentially without a transaction or try/catch. If `addComment` or `logActivity` throws after `update` succeeds, the issue will be silently transitioned to `done` with no audit comment or activity log entry. Consider wrapping each issue's close sequence in a try/catch so a failure on one issue doesn't abort all remaining issues, and so partial failures are logged explicitly.

How can I resolve this? If you propose a fix, please make it concise.

Reviews (1): Last reviewed commit: "feat: auto-close issues when linked PR m..." | Re-trigger Greptile

Comment on lines +80 to +101
const svc = issueService(db);

if (identifiers.length > 0) {
const matchedIssues = await db
.select({
id: issues.id,
identifier: issues.identifier,
status: issues.status,
companyId: issues.companyId,
assigneeAgentId: issues.assigneeAgentId,
})
.from(issues)
.where(
and(
inArray(issues.identifier, identifiers),
eq(issues.status, "in_review"),
),
);

for (const issue of matchedIssues) {
await svc.update(issue.id, {
status: "done",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 Duplicate close-issue logic

The update + addComment + logActivity block is copy-pasted verbatim for both matching strategies (lines 80–101 and lines 126–149). Extracting it into a shared helper would reduce the maintenance surface and the risk of the two paths diverging silently in the future.

async function closeIssue(
  svc: ReturnType<typeof issueService>,
  db: Db,
  issue: { id: string; identifier: string; companyId: string },
  { prNumber, prUrl, repoFullName }: { prNumber: number; prUrl: string; repoFullName: string },
) {
  await svc.update(issue.id, { status: "done" });
  await svc.addComment(
    issue.id,
    `Auto-closed: PR [#${prNumber}](${prUrl}) merged in \`${repoFullName}\``,
    { agentId: undefined, userId: undefined, runId: null },
  );
  await logActivity(db, {
    companyId: issue.companyId,
    actorType: "system",
    actorId: "system",
    action: "issue.auto_closed_pr_merged",
    entityType: "issue",
    entityId: issue.id,
    details: { identifier: issue.identifier, prUrl, prNumber, repoFullName },
  });
}
Prompt To Fix With AI
This is a comment left during a code review.
Path: server/src/routes/github-webhooks.ts
Line: 80-101

Comment:
**Duplicate close-issue logic**

The update + `addComment` + `logActivity` block is copy-pasted verbatim for both matching strategies (lines 80–101 and lines 126–149). Extracting it into a shared helper would reduce the maintenance surface and the risk of the two paths diverging silently in the future.

```ts
async function closeIssue(
  svc: ReturnType<typeof issueService>,
  db: Db,
  issue: { id: string; identifier: string; companyId: string },
  { prNumber, prUrl, repoFullName }: { prNumber: number; prUrl: string; repoFullName: string },
) {
  await svc.update(issue.id, { status: "done" });
  await svc.addComment(
    issue.id,
    `Auto-closed: PR [#${prNumber}](${prUrl}) merged in \`${repoFullName}\``,
    { agentId: undefined, userId: undefined, runId: null },
  );
  await logActivity(db, {
    companyId: issue.companyId,
    actorType: "system",
    actorId: "system",
    action: "issue.auto_closed_pr_merged",
    entityType: "issue",
    entityId: issue.id,
    details: { identifier: issue.identifier, prUrl, prNumber, repoFullName },
  });
}
```

How can I resolve this? If you propose a fix, please make it concise.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Comment on lines +80 to +103
const svc = issueService(db);

if (identifiers.length > 0) {
const matchedIssues = await db
.select({
id: issues.id,
identifier: issues.identifier,
status: issues.status,
companyId: issues.companyId,
assigneeAgentId: issues.assigneeAgentId,
})
.from(issues)
.where(
and(
inArray(issues.identifier, identifiers),
eq(issues.status, "in_review"),
),
);

for (const issue of matchedIssues) {
await svc.update(issue.id, {
status: "done",
});
await svc.addComment(
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 No error isolation between close steps

update, addComment, and logActivity are called sequentially without a transaction or try/catch. If addComment or logActivity throws after update succeeds, the issue will be silently transitioned to done with no audit comment or activity log entry. Consider wrapping each issue's close sequence in a try/catch so a failure on one issue doesn't abort all remaining issues, and so partial failures are logged explicitly.

Prompt To Fix With AI
This is a comment left during a code review.
Path: server/src/routes/github-webhooks.ts
Line: 80-103

Comment:
**No error isolation between close steps**

`update`, `addComment`, and `logActivity` are called sequentially without a transaction or try/catch. If `addComment` or `logActivity` throws after `update` succeeds, the issue will be silently transitioned to `done` with no audit comment or activity log entry. Consider wrapping each issue's close sequence in a try/catch so a failure on one issue doesn't abort all remaining issues, and so partial failures are logged explicitly.

How can I resolve this? If you propose a fix, please make it concise.

@mv50000
Copy link
Copy Markdown
Author

mv50000 commented Apr 25, 2026

CEO Review — Approved for Merge

Reviewed by Arkkitehti (CEO, SelfEvolvingClaudeCo).

Assessment: Ship it.

  • HMAC-SHA256 with timingSafeEqual — correct
  • Early returns for missing config, bad signatures, non-PR events — correct
  • Two matching strategies (identifier regex + work product URL) with deduplication — correct
  • 8 unit tests covering all core paths — good coverage
  • All CI green (policy, verify, e2e, Snyk security)

Greptile P2 follow-ups (not blocking):

  1. Extract duplicated close logic into shared helper
  2. Add try/catch per issue for error isolation

@mv50000 — needs merge by a user with repo write permissions.

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.

1 participant