Webhook: assignee-change pivot creates duplicate implement dispatches #92

New issue

Closed

opened 2026-04-19 17:05:35 +00:00 by claude-desktop · 0 comments

claude-desktop commented

2026-04-19 17:05:35 +00:00

Collaborator

User story

As the operator, I want to re-trigger implement on an already-assigned issue without the previous-assignee's worker also picking up a stale dispatch — so that one ticket produces one PR, not two.

What happened (concrete)

Operating pattern for re-dispatch: re-assign the issue to a different agent then back to the intended one ("assignee-pivot"). Forgejo fires two issues.assigned events — one per step — and the webhook dispatches both:

charles/claude-hooks#73 assigned to dev → dev's worker enqueues implement.
Seconds later, reassigned to boss → boss's worker enqueues implement.

Both agents run to completion, both push a branch, both open a PR — e.g. today:

#73: dev opened #88 (merged), boss opened #89 (dupe, had to be closed manually).
#87: dev opened #90 (rejected, had to be closed manually), boss opened #91 (the keeper).

This happened three separate times today. Pattern is reliable and has now cost non-trivial amounts of operator cleanup + agent compute.

Why today's workaround exists

It exists because the webhook only re-fires an implement dispatch on a fresh issues.assigned event. Re-posting the same assignee doesn't refire (Forgejo de-dupes). Re-opening the issue fires issues.reopened — not routed. Label-flipping only works for area:design. So operators pivot to trigger a new event.

Proposed shape

Pick one of:

A — honour the current assignee, not the event's

handleIssueAssigned(issue, agentFromEvent) today trusts the webhook's payload. Change it to refetch issue.assignees after dedupe and only dispatch if the current assignee still matches the event's agent. Intermediate pivots clear before the second assignment actually fires (or the first firing gets skipped because the current assignee is no longer dev). Race-prone but small.

B — short-lived dedupe on `(repo, issue, agent)` assignment

Inside webhook-handlers.ts, remember the last-dispatched (repo, issue, agent) tuple for ~30 s. If the same tuple fires again within that window, drop it silently. If a different agent fires in the same window, drop the first-in-flight task's worker-queue entry (if still queued) and let the new one run. More robust than A; one extra Map entry plus a timer.

C — explicit re-dispatch endpoint

POST /redispatch {repo, issue, agent, skill?} that bypasses the webhook. Operators use this instead of the pivot trick. Clean conceptually, but adds an endpoint and changes the operator's muscle memory.

Recommend B: it fixes the observed bug without needing operator behaviour change, and the dedupe window is small enough that a real back-to-back reassignment (operator changes their mind twice) still lands the second agent.

Acceptance criteria

Core

Dedupe map in webhook-handlers.ts keyed by ${repo}#${issue}@${agent}; TTL 30 s; entry stamp is the last-dispatched timestamp.
handleIssueAssigned rejects a dispatch if the tuple is already in the map within TTL.
Same-repo-issue-different-agent within TTL: cancel the still-queued task from the first agent (if not yet running), then dispatch the new one. If first agent already running, log and let both run (today's failure mode, preserved as the explicit non-handled case).
Existing behaviour for a single clean assignment preserved.

Tests

webhook-handlers.test.ts: two issues.assigned events for the same (repo, issue, agent) within 30 s → exactly one dispatch.
webhook-handlers.test.ts: issues.assigned → dev, issues.assigned → boss within 30 s, dev's task still queued → dev's task is removed, boss dispatched.
webhook-handlers.test.ts: issues.assigned → dev, issues.assigned → boss within 30 s, dev's task already running → both run (logged).

Dashboard visibility

Not required. The existing Workers card on the dashboard already surfaces queue depth per agent; the dedupe kicks in before the queue, so there's nothing new to show.

Out of scope

Automatic PR deduplication: if somehow two PRs DO end up opened for the same issue, that's an upstream concern — Forgejo lets both coexist. Leave as operator cleanup.
Rewriting the webhook router: today's assignee-based path has plenty of tests; this is a guard on top, not a rewrite.
The assignee-pivot trick itself: operators will keep using it until /redispatch exists; this ticket's scope is making the pivot safe.

References

Pattern observed: #73 → PR #88 + #89 (today), #87 → PR #90 + #91 (today), #79 → boss dupe earlier today.
src/webhook-handlers.ts:handleIssueAssigned — where the guard lands.
src/webhook.ts:172 — comment already anticipates this class of double-firing for label routes; extend to assignee routes.

Dependencies

Blocked by: nothing.
Blocks: clean multi-agent e2e smoke runs.
Branch off: main.

## User story As the **operator**, I want to re-trigger `implement` on an already-assigned issue without the previous-assignee's worker also picking up a stale dispatch — so that one ticket produces one PR, not two. ## What happened (concrete) Operating pattern for re-dispatch: re-assign the issue to a different agent then back to the intended one ("assignee-pivot"). Forgejo fires two `issues.assigned` events — one per step — and the webhook dispatches both: 1. `charles/claude-hooks#73` assigned to `dev` → dev's worker enqueues `implement`. 2. Seconds later, reassigned to `boss` → boss's worker enqueues `implement`. Both agents run to completion, both push a branch, both open a PR — e.g. today: - **#73**: dev opened #88 (merged), boss opened #89 (dupe, had to be closed manually). - **#87**: dev opened #90 (rejected, had to be closed manually), boss opened #91 (the keeper). This happened three separate times today. Pattern is reliable and has now cost non-trivial amounts of operator cleanup + agent compute. ## Why today's workaround exists It exists because the webhook only re-fires an implement dispatch on a fresh `issues.assigned` event. Re-posting the same assignee doesn't refire (Forgejo de-dupes). Re-opening the issue fires `issues.reopened` — not routed. Label-flipping only works for `area:design`. So operators pivot to trigger a new event. ## Proposed shape Pick one of: ### A — honour the **current** assignee, not the event's `handleIssueAssigned(issue, agentFromEvent)` today trusts the webhook's payload. Change it to refetch `issue.assignees` after dedupe and only dispatch if the *current* assignee still matches the event's `agent`. Intermediate pivots clear before the second assignment actually fires (or the first firing gets skipped because the current assignee is no longer `dev`). Race-prone but small. ### B — short-lived dedupe on `(repo, issue, agent)` assignment Inside `webhook-handlers.ts`, remember the last-dispatched `(repo, issue, agent)` tuple for ~30 s. If the same tuple fires again within that window, drop it silently. If a *different* agent fires in the same window, drop the first-in-flight task's worker-queue entry (if still queued) and let the new one run. More robust than A; one extra Map entry plus a timer. ### C — explicit re-dispatch endpoint `POST /redispatch {repo, issue, agent, skill?}` that bypasses the webhook. Operators use this instead of the pivot trick. Clean conceptually, but adds an endpoint and changes the operator's muscle memory. Recommend **B**: it fixes the observed bug without needing operator behaviour change, and the dedupe window is small enough that a real back-to-back reassignment (operator changes their mind twice) still lands the second agent. ## Acceptance criteria ### Core - [ ] Dedupe map in `webhook-handlers.ts` keyed by `${repo}#${issue}@${agent}`; TTL 30 s; entry stamp is the last-dispatched timestamp. - [ ] `handleIssueAssigned` rejects a dispatch if the tuple is already in the map within TTL. - [ ] Same-repo-issue-different-agent within TTL: cancel the still-queued task from the first agent (if not yet running), then dispatch the new one. If first agent already running, log and let both run (today's failure mode, preserved as the explicit non-handled case). - [ ] Existing behaviour for a single clean assignment preserved. ### Tests - [ ] `webhook-handlers.test.ts`: two `issues.assigned` events for the same `(repo, issue, agent)` within 30 s → exactly one dispatch. - [ ] `webhook-handlers.test.ts`: `issues.assigned → dev`, `issues.assigned → boss` within 30 s, dev's task still queued → dev's task is removed, boss dispatched. - [ ] `webhook-handlers.test.ts`: `issues.assigned → dev`, `issues.assigned → boss` within 30 s, dev's task already running → both run (logged). ### Dashboard visibility - [ ] Not required. The existing Workers card on the dashboard already surfaces queue depth per agent; the dedupe kicks in before the queue, so there's nothing new to show. ## Out of scope - **Automatic PR deduplication**: if somehow two PRs DO end up opened for the same issue, that's an upstream concern — Forgejo lets both coexist. Leave as operator cleanup. - **Rewriting the webhook router**: today's assignee-based path has plenty of tests; this is a guard on top, not a rewrite. - **The assignee-pivot trick itself**: operators will keep using it until `/redispatch` exists; this ticket's scope is making the pivot safe. ## References - Pattern observed: #73 → PR #88 + #89 (today), #87 → PR #90 + #91 (today), #79 → boss dupe earlier today. - `src/webhook-handlers.ts:handleIssueAssigned` — where the guard lands. - `src/webhook.ts:172` — comment already anticipates this class of double-firing for label routes; extend to assignee routes. ## Dependencies - **Blocked by:** nothing. - **Blocks:** clean multi-agent e2e smoke runs. - **Branch off:** `main`.

claude-desktop added the

area:webhook

type:user-story

labels

2026-04-19 17:05:38 +00:00