worker: stop awaiting each claim batch, so concurrency limits and /health mean something

[jerelvelarde]

From the review of #224. These are the items that PR deliberately left out, and the first one is load-bearing for two others.

The batch await makes concurrency accounting dead code

poll() fully awaits Promise.allSettled over each claimed batch, and schedulePoll only runs after the poll resolves. So activeJobs.size and activeJobsByType are always 0 when availableSlots and typeLimit - activeForType are computed. Both expressions are unreachable: mutating each to ignore in-flight work passes the whole suite.

It does not over-fetch — the serialization accidentally bounds it — but per-type concurrency limits never bind, which is not what concurrencyByType looks like it does.

Which is also why /health is still wrong (#182, #184)

Because poll() blocks for the whole batch duration, lastPollTime stalls for as long as the longest job. buildHealthResponse compares it against STALE_POLL_MS = 60_000 with no active-job exemption, against a 120s AI_RESPONSE timeout. So a healthy worker mid-job returns 503 "stalled" and Railway restarts it — killing the job it was in the middle of.

Two ways out, and they should be decided together:

1. Stop awaiting the batch (fixes the accounting too), or
2. Exempt active jobs from the stale-poll check.

(1) is the better fix and makes (2) unnecessary. This is the reason /health has now been deferred three times; it should be one PR with the batch change.

An unregistered job type is terminally destroyed

FAILED with completedAt and no attempt increment — and job-cleanup.ts only deletes COMPLETED and DEAD_LETTER, so the rows accumulate forever. #224 added lockedAt: null, claimToken: null to that write, which means reclaim can't rescue them either.

Reachable only via claimAndProcessJobs, and production sets concurrencyByType so it takes the type-filtered path. Latent today; live the moment concurrencyByType is emptied or a worker ships without the full handler set.

A no-op release back to PENDING with no attempt consumed is the safe failure. Data-destroying path, so it wants its own change.

The timeout doesn't cancel the handler, and the drain has no deadline

runWithTimeout stops waiting for the handler; the handler keeps running. And AI_RESPONSE at 120s against Railway's ~30s SIGTERM grace means the graceful drain resolves well after SIGKILL already landed.

So the real shutdown path is still crash-abandonment plus reclaim. #224's fence makes that safe at the row level — it is not cancellation, and shouldn't be read as such. Threading an AbortSignal through JobHandlerContext is the actual fix.

Handlers with external side effects need idempotency keys

A database fence can't un-send an email. In an interleaving where a live claim is reclaimed, the first execution's external effects have already happened. lockUntil (#224) makes that much rarer, not impossible.

• AI_RESPONSE — covered by feat(queue): one AI answer per ticket, arbitrated by the database #191's Message.ticketId_responseKey and the responseState machine.
• ESCALATION, HUBSPOT_SYNC, TRACKER_SYNC — not covered, and fix(queue): fence job claims and drain in-flight work on shutdown #224 is what introduces automatic re-execution.

Smaller

• void currentPoll.finally(cb) builds a derived promise with no rejection handler. poll() can't reject today, so latent — but if a throw ever escapes it's an unhandled rejection, and stop()'s await this.pollPromise would propagate out so $disconnect() never runs.
• job-cleanup.ts never deletes FAILED rows at all, which is what makes the accumulation above unbounded.

[jerelvelarde]

A review round on #224 turned up a consequence of this that I think raises its priority: the batch-await does not just make the accounting cosmetic, it starves a fixed tail of job types indefinitely. Verified by execution against the real Worker, not read off the code.

apps/worker/src/index.ts sets maxConcurrency: 10 with per-type limits summing to 14. claimJobsByType iterates this.handlers in insertion order and decrements remainingGlobalSlots by every claimed job — after Promise.allSettled has already awaited them, so nothing actually occupies a slot by then. Under a broad backlog the budget is exhausted partway down the list.

A run giving every type a full backlog claimed:

AI_RESPONSE, ESCALATION, SLA_CHECK, ONBOARDING_DIGEST, ACCOUNT_SCORING, HUBSPOT_SYNC

and never reached TRACKER_SYNC, JOB_CLEANUP, GITHUB_REACTION_POLL or PENDING_RESPONSE_SWEEP.

Because processedCount > 0 schedules the next poll at nextPollDelay = 0, the next poll makes the identical choice from the identical state. That is indefinite starvation of the same tail, not delay — and it is deterministic, so it will always be those four.

PENDING_RESPONSE_SWEEP is the handler that rescues AI responses stranded in PENDING. Worth weighing against the current Discord response complaints: under sustained load the recovery mechanism is the thing that stops running.

Two more from the same round, same root cause:

• Head-of-line blocking across types. await Promise.allSettled sits inside the per-type loop, so type N+1 is not claimed until type N's batch fully drains. With AI_RESPONSE registered first at a 120s timeout, one slow AI response delays ESCALATION — the path that exists for when things are going wrong — by up to two minutes. Measured: an AI_RESPONSE handler sleeping 5s meant the ESCALATION claim query was not issued at all until it finished.
• The graceful-shutdown drain in stop() is dead code. Every claim path awaits its batch before the poll promise resolves, and stop() awaits pollPromise first, so activeJobs.size is always 0 by the time the drain check runs. Replacing the whole block with a throw leaves all 53 tests passing. shutdownResolve, the shuttingDown branch in processJob's finally, and the Waiting for N active jobs to complete... log are all unreachable — that log will never print in production. (Overlaps worker: shutdown() does not actually drain in-flight jobs #181.)

No existing test would catch any of this: every handler in worker-concurrency.test.ts is instantaneous, including the one named processes different types in separate pools, which only proves both types get claimed eventually.

Suggested shape when this is picked up: claim across all types first, then process everything in one Promise.allSettled, applying the global budget when building typesWithCapacity rather than after each claim — and rotate the iteration start across polls so the budget cannot always run out at the same place.