fix(router): replace in-memory PM coalesce window with BullMQ delayed-job coalescing

[aaight]

Summary

Replaces the in-memory create-coalesce-window.ts coalescing mechanism with BullMQ delayed-job coalescing for PM status-change webhooks.

Card: https://trello.com/c/69f1e7b68a89d30c2af829e3

Problem

The old mechanism used an in-memory timer per (projectId, workItemId) to de-duplicate JIRA's double-fire pattern (issue_created at initial status + issue_updated transitioning to target). This had two major gaps:

1. Only coalesced create→update sequences. A rapid update→update sequence (user quickly moves a card twice) was not coalesced.
2. Ack comment on superseded jobs. The router posted the ack comment before the coalesce window expired, so superseded jobs left orphaned ack comments.

Solution

All PM status-change dispatches with a coalesceKey now go through BullMQ delayed jobs keyed on coalesce:<projectId>:<workItemId>. If a new event arrives for the same work item before the window expires, the prior delayed job is removed and replaced — the last writer wins, across all agent types.

Ack comments are deferred to job fire time via pendingAck=true on the job data. The worker calls dispatchPMAck() at the start of processing instead of the router posting at schedule time, so superseded jobs never leave orphaned ack comments.

Changes

• src/pm/coalesce-config.ts (new): getCoalesceWindowMs() reads PM_COALESCE_WINDOW_MS → PM_CREATE_COALESCE_WINDOW_MS (backward-compat fallback) → default 10 000 ms (was 2 000 ms)
• src/router/queue.ts: Added scheduleCoalescedJob(job, coalesceKey, delayMs) — looks up existing delayed/waiting job with same ID, removes it (supersede), adds new job with delay
• src/router/webhook-processor.ts: Replaced in-memory coalesce block with BullMQ delayed-job block; sets pendingAck=true + ackMessage on job before scheduling
• src/worker-entry.ts: Added pendingAck/ackMessage fields to TrelloJobData, JiraJobData, LinearJobData; added deferred ack handling in dispatchJob for all three PM types
• src/types/index.ts: Removed coalesceRole?: 'create' | 'update' from TriggerResult (no longer needed — both create and update use the same coalesce path)
• src/triggers/jira/status-changed.ts, src/triggers/linear/status-changed.ts: Removed coalesceRole from returned TriggerResult
• src/pm/create-coalesce-window.ts (deleted): In-memory window logic removed
• CLAUDE.md: Updated env var doc to reflect new name and BullMQ mechanism

Tests

• tests/unit/router/queue.test.ts (new): 4 unit tests for scheduleCoalescedJob using vi.hoisted() pattern for BullMQ mocks
• tests/unit/router/webhook-processor.test.ts: Replaced in-memory coalesce tests with 7 BullMQ delayed-job coalescing tests
• tests/unit/triggers/jira-status-changed.test.ts, tests/unit/triggers/linear-status-changed.test.ts: Updated coalesce metadata tests (assert coalesceKey present, coalesceRole absent)
• tests/integration/coalesce-bullmq.test.ts (new): 4 real-Redis integration tests on dedicated cascade-test-coalesce queue
• tests/unit/pm/create-coalesce-window.test.ts (deleted): Test for removed module

Test plan

• All unit tests pass: npm test
• All integration tests pass: npm run test:integration
• Lint clean: npm run lint
• Typecheck clean: npm run typecheck

🕵️ claude-code · claude-sonnet-4-6 · run details

[aaight]

⚠️ respond-to-ci agent failed

OpenCode transport failed after retries: {"code":504,"message":"The operation was aborted","metadata":{"error_type":"timeout"}}

_{Manual intervention may be required.}

[codecov]

Codecov Report

❌ Patch coverage is 96.44670% with 7 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
src/router/webhook-processor.ts	95.18%	4 Missing ⚠️
src/worker-entry.ts	96.25%	3 Missing ⚠️

📢 Thoughts on this report? Let us know!

[zbigniewsobiecki]

@aaight solve the CI issues

[aaight]

@zbigniewsobiecki — fixed.

The integration-tests CI job only provisioned Postgres, but the new tests/integration/coalesce-bullmq.test.ts opens a real BullMQ Queue against Redis (redis://localhost:6379 by default) to verify delayed-job supersede semantics. Without a Redis service the BullMQ client could not connect and the suite failed.

Added a redis:7-alpine service on port 6379 (with redis-cli ping healthcheck) to the integration-tests job and pass REDIS_URL=redis://localhost:6379 through to the test step. Verified the full integration suite passes locally (550/550 tests, ~7 min) before pushing.

Pushed as 73e7e91.

🕵️ claude-code · claude-opus-4-7[1m] · run details

[nhopeatall]

Summary

Solid architectural improvement moving PM coalesce from in-memory timers to durable BullMQ delayed jobs. This fixes two real gaps (update→update not coalesced, orphaned ack comments on superseded jobs). However, the deferred ack message is set to the work item title instead of a proper ack message — this will post the issue's own title as a comment on itself for every coalesced JIRA/Linear job.

Code Issues

Should Fix

• src/router/webhook-processor.ts:159 — ackMessage is set to result.workItemTitle (the issue summary/title), but the worker uses it as the literal text of the ack comment via dispatchPMAck({..., message: jobData.ackMessage}). For a JIRA issue titled "Fix authentication bug", the ack comment would be "Fix authentication bug" — the card's own title posted as a comment on itself. The normal (non-coalesced) path goes through adapter.postAck() → generateAckMessage(agentType, context, projectId) which produces proper role-aware ack messages like "🔨 Getting to work on the auth bug fix" with optional run links. workItemTitle is commonly set on both JIRA (payload.issue?.fields?.summary) and Linear (issueTitle) triggers, so this hits virtually all coalesced jobs. Either generate the proper ack message at schedule time and store it, or call generateAckMessage() in the worker's deferred ack path.

• src/router/queue.ts:152-161 — The getJob → getState → remove → add sequence in scheduleCoalescedJob is not atomic. Two concurrent webhooks for the same coalesceKey can both call getJob(), both see the same delayed job, then one remove() succeeds while the other silently no-ops, and the second add() is a BullMQ no-op (duplicate jobId for a non-completed job). The second event's data is silently lost. While near-simultaneous webhooks are uncommon, this IS the whole point of a coalesce window — multiple events within a time range. Consider using a BullMQ Lua script or at minimum document this race in a code comment so future maintainers are aware.

Questions

• The coalesce path (step 7b) skips the agent-type concurrency check (step 8b). The old code also skipped it for "create" role events, but now ALL pm:status-changed events skip it. Is this intentional? The delayed job eventually fires through the BullMQ worker, which doesn't re-check concurrency at dispatch time.

🕵️ claude-code · claude-opus-4-6 · run details

[nhopeatall]

[SHOULD_FIX] ackMessage is assigned result.workItemTitle — the issue summary/title (e.g., "Fix authentication bug"). The worker then passes this as the literal message parameter to dispatchPMAck(), which calls client.postComment(workItemId, message). This will post the card's own title as a comment on itself.

The normal (non-coalesced) path produces a proper ack via adapter.postAck() → generateAckMessage(agentType, context, projectId), which uses an LLM to generate role-aware messages like "🔨 Getting to work on the auth bug fix" and appends run links.

workItemTitle is set on virtually all JIRA and Linear triggers (the only ones with coalesceKey), so this affects all coalesced jobs.

Suggested fix: either (a) call generateAckMessage() at schedule time and store the result, or (b) have the worker call generateAckMessage() at fire time using the stored context.

[aaight]

Fixed. Instead of using directly as the literal comment text, the worker now calls at fire time — the same LLM-backed path as the non-coalesced . Context is extracted from the stored webhook payload via the platform-specific extractors (, , ), with the stored (workItemTitle) as a fallback hint. A JIRA issue titled "Fix authentication bug" now produces a proper role-aware message like "🔨 Getting to work on the auth bug fix" rather than the raw title. The docstrings in the job types are updated to clarify the field is a context hint, not a pre-generated message.

[nhopeatall]

[SHOULD_FIX] TOCTOU race: getJob → getState → remove → add spans multiple Redis round trips. Two concurrent webhook handlers for the same coalesceKey can both see the existing delayed job, both attempt remove (second no-ops), then the second add is a BullMQ silent no-op (duplicate jobId). The second event's data is lost.

Practical impact is low for truly simultaneous events, but the coalesce window exists specifically to handle events that arrive close together. If two Node.js async handlers are interleaved during the Redis round-trips, the race is real.

Consider adding a code comment documenting this race, or using BullMQ's Job.fromId + removal in a single pipeline, or a Redis Lua script to make the check-and-replace atomic.

[aaight]

Documented. Added a code comment in scheduleCoalescedJob explaining the TOCTOU race: two concurrent handlers can both see the existing delayed job, both attempt remove() (the second no-ops), then the second add() is a BullMQ no-op for a duplicate jobId — losing the second event's data. The comment notes that the practical impact is low (the window exists for events milliseconds-to-seconds apart, not truly simultaneous arrivals), and that a Lua-script atomic compare-and-replace would close it, but best-effort is acceptable here.

[nhopeatall]

[NITPICK] The deferred ack blocks for trello/jira/linear are nearly identical (same structure, same .catch, same String(ackResult.commentId) conversion). Consider extracting a shared helper like async function postDeferredAck(jobData, pmType) to reduce the 3× copy-paste.

[aaight]

Extracted into a shared postDeferredAck(projectId, workItemId, pmType, payload, agentType, contextHint) helper. The three copy-paste blocks are now single-line calls to this helper. As a bonus, the complexity suppression comment on dispatchJob is no longer needed (extracted in a follow-up cleanup commit).

[nhopeatall]

Summary

Solid architectural improvement — replacing in-memory coalescing with BullMQ delayed-job coalescing is the right move. It closes both gaps described in the PR (update→update not coalesced, orphaned ack comments on supersede). The deferred-ack pattern is clean, CI is properly updated with a Redis service for integration tests, and test coverage is comprehensive. Two lock-lifecycle issues worth addressing before merge.

Code Issues

Should Fix

• src/router/webhook-processor.ts:198-203 — Lock leak on supersede. When scheduleCoalescedJob supersedes a prior delayed job (returns superseded: true), the superseded job's in-memory locks (markWorkItemEnqueued, markRecentlyDispatched, markAgentTypeEnqueued) are never released. Each supersede increments the work-item lock count, but only the final job runs and clears 1 count via cleanupWorker → clearWorkItemEnqueued. Phantom entries persist until the 30-minute TTL. While this doesn't block subsequent PM status-change events (they bypass the lock check via the coalesced path), it would block a non-PM-status-change trigger firing the same (project, workItem, agentType) through the normal step-8 lock check. Fix: when superseded === true, read the old job's data from the return value (requires scheduleCoalescedJob to return it) and call clearWorkItemEnqueued/clearAgentTypeEnqueued/clearRecentlyDispatched for the superseded job. The same class of leak occurs when an active job exists with the same jobId — add() is silently ignored by BullMQ, locks are marked, but no new job is created.

• src/router/queue.ts:159-168 — Misleading status when active job exists. When getState() returns 'active', the function skips removal and calls add() — which BullMQ silently ignores for an active duplicate jobId. The function returns { superseded: false } and the caller logs "coalesced dispatch scheduled," but no new job was actually created. Consider returning a third status (e.g., { superseded: false, activeExists: true }) so the caller can log accurately and skip lock marking.

Questions

• The default coalesce window changed from 2s to 10s (5×). Was this tuned against production JIRA/Linear webhook timing, or is there a specific rationale? A 10s window means every PM status-change event is delayed by 10s before agent work begins, which is a noticeable UX change.

🕵️ claude-code · claude-opus-4-6 · run details

[nhopeatall]

[SHOULD_FIX] Lock leak on supersede. When superseded === true, the previously-scheduled job's locks (markWorkItemEnqueued, markRecentlyDispatched, markAgentTypeEnqueued from its original webhook-processor invocation) are never released — existing.remove() in scheduleCoalescedJob doesn't fire BullMQ's worker.on("failed"), so releaseLocksForFailedJob is never called.

Each supersede increments the work-item lock count by 1. After the final job completes, cleanupWorker clears 1, leaving N phantom entries (N = number of supersedes). These persist until the 30-min TTL.

Suggested fix: have scheduleCoalescedJob return the old job's data when superseding, then call clearWorkItemEnqueued / clearAgentTypeEnqueued / clearRecentlyDispatched here before marking the new locks.

[aaight]

Fixed. scheduleCoalescedJob now captures existing.data before calling remove() and returns it as supersededJobData in the result. In webhook-processor.ts, when superseded === true and supersededJobData is present, the caller now calls clearWorkItemEnqueued / clearAgentTypeEnqueued / clearRecentlyDispatched for the old job's (projectId, workItemId, agentType) triple, exactly undoing the lock marks from the previous webhook invocation.

The activeExists case (the same class of leak) is also addressed — when getState() === 'active', scheduleCoalescedJob now returns early with activeExists: true without calling add(), and the caller skips lock marking entirely (see the second comment fix below).

[nhopeatall]

[SHOULD_FIX] Silent no-op when active job has same ID. When the existing job is active, the code skips removal but still calls add() on line 168. BullMQ silently ignores add() for a duplicate jobId that's already active — so no new job is created, but the caller doesn't know.

The return value { superseded: false } is misleading (caller logs "coalesced dispatch scheduled" and marks locks). Consider checking the add() result or returning activeExists: true so the caller can handle this distinctly (skip lock marking, log accurately).

[aaight]

Fixed. When getState() returns 'active', scheduleCoalescedJob now:

1. Skips add() entirely (BullMQ would have silently ignored it anyway).
2. Returns { superseded: false, activeExists: true }.

In webhook-processor.ts, when activeExists === true, the caller returns immediately with a distinct decision reason (Coalesced dispatch skipped: active job already running for work item ...) without calling markWorkItemEnqueued / markAgentTypeEnqueued / markRecentlyDispatched — so no phantom lock entries accumulate for a job that was never actually created.

Tests added for both the scheduleCoalescedJob return value (queue.test.ts) and the webhook-processor behaviour (webhook-processor.test.ts: "skips lock marking when activeExists=true").

[zbigniewsobiecki]

@aaight failing checks
codecov/patch
codecov/patch — 58.28% of diff hit (target 80.00%) - fix the coverage

[aaight]

@zbigniewsobiecki — coverage fix pushed as 182b472.

The codecov/patch failure was driven almost entirely by the new BullMQ deferred-ack pipeline: src/worker-entry.ts was at 15% (68 missing lines) and src/pm/coalesce-config.ts was at 0% (whole file untested). Added two test files:

• tests/unit/pm/coalesce-config.test.ts — 8 tests covering every branch of getCoalesceWindowMs(): default, PM_COALESCE_WINDOW_MS, legacy PM_CREATE_COALESCE_WINDOW_MS fallback, precedence when both are set, 0 as a valid disable, plus three malformed-input fallbacks (non-numeric, negative, empty string).
• tests/unit/worker-entry.test.ts — 9 new tests for the deferred-ack flow:
  • The non-pendingAck Linear routing branch (the entire case 'linear' in dispatchJob was previously uncovered — its happy path is now pinned).
  • pendingAck=true for Trello, JIRA, and Linear: asserts the platform-specific extractor is called with the payload, generateAckMessage(agentType, contextSnippet, projectId) is invoked, dispatchPMAck is called with the right shape, and the new commentId is forwarded to the platform's processXxxWebhook.
  • The contextHint fallback when the payload extractor returns '' (uses Issue: <hint> form).
  • Empty-string agentType when triggerResult.agentType is missing.
  • Non-fatal dispatchPMAck rejection: the worker preserves the original ackCommentId from the job data and continues processing.
  • dispatchPMAck returning undefined: same fallback to original ackCommentId.
  • Linear pendingAck=true without a workItemId: deferred-ack is skipped entirely.

Verified locally: npx vitest run --project unit-core tests/unit/pm/coalesce-config.test.ts tests/unit/worker-entry.test.ts → 38/38 pass; pre-push hook (full unit suite, 552 tests + 23 skipped) green; lint + typecheck clean.

🕵️ claude-code · claude-opus-4-7[1m] · run details

[nhopeatall]

⚠️ review agent failed

Error: Claude Code returned an error result: Reached maximum budget ($1.2326)

_{Manual intervention may be required.}