Port PR-closed cleanup chain to pr-closed.yml flow #1093

Closed
opened 2026-05-10 23:30:40 +00:00 by claude-desktop · 0 comments
Collaborator

As an operator, I want PR-closed cleanup (branch cleanup, dependency row drop, pr.merged trigger publication) to live in a YAML flow, so that the imperative handlePullRequestClosed call in webhook.ts:334 can be deleted and PR-close side effects become reviewable as a flow.

Context

apps/server/src/http/webhook.ts:333-335 still calls handlePullRequestClosed directly because the YAML port doesn't cover all three side effects:

  1. Branch cleanup (cleanupBranch — agent worktree + remote branch deletion)
  2. pr_dependencies row drop (DB cleanup for dependency graph)
  3. pr.merged internal trigger publication (drives pr-merged.yml rebase cascade)

flows/defaults/pr-merged.yml already covers the cascade fan-out, but only fires AFTER step 3 publishes the trigger. Steps 1 + 2 have no YAML coverage.

Acceptance criteria

Ops

  • cleanup_branch op — wraps cleanupBranch; inputs repo, branch_ref
  • drop_pr_dependencies op — wraps DB cleanup helper; inputs repo, pr_number
  • publish_pr_merged op — wraps triggerBus.publish({kind:"pr",action:"merged",...}); inputs repo, pr_number; only fires when event.pr.merged === true

Flow

  • flows/defaults/pr-closed.yml on pull_request: [closed]:
    1. cleanup_branch
    2. drop_pr_dependencies
    3. publish_pr_merged (gated by if: event.pr.merged)

Tests

  • Unit tests for each op
  • Flow integration test for pr-closed.yml (merged vs not-merged paths — verify pr.merged only publishes on merged PR)
  • Verify pr-merged.yml still fires after the switch (regression test for the cascade)

Cleanup

  • Delete handlePullRequestClosed call from webhook.ts:333-335
  • Delete handlePullRequestClosed from event-handlers.ts
  • Delete its tests

Out of scope

  • CI events port — separate ticket (#1092)
  • Slash commands — separate ticket
  • legacy* executor hooks — separate ticket

References

  • apps/server/src/http/webhook.ts:326-335 (imperative call comment)
  • apps/server/src/domain/workflow/event-handlers.ts:680 (handlePullRequestClosed impl)
  • flows/defaults/pr-merged.yml (existing cascade flow that depends on the internal trigger)
  • Root-cause analysis in this conversation
As an operator, I want PR-closed cleanup (branch cleanup, dependency row drop, `pr.merged` trigger publication) to live in a YAML flow, so that the imperative `handlePullRequestClosed` call in `webhook.ts:334` can be deleted and PR-close side effects become reviewable as a flow. ## Context `apps/server/src/http/webhook.ts:333-335` still calls `handlePullRequestClosed` directly because the YAML port doesn't cover all three side effects: 1. Branch cleanup (`cleanupBranch` — agent worktree + remote branch deletion) 2. `pr_dependencies` row drop (DB cleanup for dependency graph) 3. `pr.merged` internal trigger publication (drives `pr-merged.yml` rebase cascade) `flows/defaults/pr-merged.yml` already covers the cascade fan-out, but only fires AFTER step 3 publishes the trigger. Steps 1 + 2 have no YAML coverage. ## Acceptance criteria ### Ops - [ ] `cleanup_branch` op — wraps `cleanupBranch`; inputs `repo, branch_ref` - [ ] `drop_pr_dependencies` op — wraps DB cleanup helper; inputs `repo, pr_number` - [ ] `publish_pr_merged` op — wraps `triggerBus.publish({kind:"pr",action:"merged",...})`; inputs `repo, pr_number`; only fires when `event.pr.merged === true` ### Flow - [ ] `flows/defaults/pr-closed.yml` on `pull_request: [closed]`: 1. `cleanup_branch` 2. `drop_pr_dependencies` 3. `publish_pr_merged` (gated by `if: event.pr.merged`) ### Tests - [ ] Unit tests for each op - [ ] Flow integration test for `pr-closed.yml` (merged vs not-merged paths — verify `pr.merged` only publishes on merged PR) - [ ] Verify `pr-merged.yml` still fires after the switch (regression test for the cascade) ### Cleanup - [ ] Delete `handlePullRequestClosed` call from `webhook.ts:333-335` - [ ] Delete `handlePullRequestClosed` from `event-handlers.ts` - [ ] Delete its tests ## Out of scope - CI events port — separate ticket (#1092) - Slash commands — separate ticket - `legacy*` executor hooks — separate ticket ## References - `apps/server/src/http/webhook.ts:326-335` (imperative call comment) - `apps/server/src/domain/workflow/event-handlers.ts:680` (`handlePullRequestClosed` impl) - `flows/defaults/pr-merged.yml` (existing cascade flow that depends on the internal trigger) - Root-cause analysis in this conversation
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
charles/claude-hooks#1093
No description provided.