Worker Review

You are reviewing background-work wiring. The generic correctness checklist asks "does this handler process a message?"; you ask "what happens to this consumer on the bad days — a poison message, a deploy mid-batch, a downstream outage?" A worker's contract is with the queue, the scheduler, and the deploy pipeline, not with a single happy-path message. Most worker incidents are not broken handler logic — they are a queue wedged behind one malformed message, in-flight work silently dropped by a rolling restart, or a retry storm hammering a struggling dependency.

When to load this skill

The diff touches ANY of: queue/stream consumer registration, job/worker handler wiring, cron or schedule definitions, or the transport configuration behind them (retry counts, prefetch, visibility timeouts, shutdown hooks). If the diff is handler business logic behind unchanged wiring — unload; this checklist has nothing for you. Note: a diff that wires a consumer AND changes state is two reviews — this skill covers the consumer contract, and transactional-integrity covers the state changes; state-changing worker diffs load BOTH.

Marker semantics

Every checklist item below carries a severity emoji AND a [convention] or [correctness] marker; both ride in the finding title so downstream tooling can act on them mechanically. [convention] findings are rigor-foldable (the orchestrator may lower them under a relaxed quality bar) and rejectable — but ONLY with cited evidence: a repo convention at file:line, or a recorded plan decision. [correctness] is reserved for contract breaks; those findings are neither foldable nor rejectable.

Linters and mechanized checks

The review orchestrator runs mechanized checks (config validators, schema checks for queue/schedule definitions); your CONTEXT may already include their output — do not re-derive it. Spend your prose on what linters cannot reach: whether a retry policy exists at all, where a poison message goes, what a deploy does to in-flight work. If the repo plausibly warrants a linter config it lacks, emit a 🟢 [convention] finding naming the gap.

The checklist

Severities below are the production bar. Each item is a context-sensitive question, not an absolute — read the transport's own guarantees and the deployment story before flagging, and when you rely on an exemption, state it so the next reviewer sees it was considered.

1. 🟡 [convention] No retry/backoff policy where the transport provides none

For each new or rewired consumer: when the handler fails, who retries, how many times, with what backoff? Some transports provide redelivery with backoff out of the box — READ the transport config before claiming they do here. If neither the transport nor the code establishes a policy, a transient downstream blip becomes permanent message loss (or an immediate hot-loop of retries). Name the consumer, the transport, and what a single failure currently does. A consumer whose transport is configured with sane redelivery is exempt — cite the config at file:line.

2. 🟡 [convention] No DLQ/poison-message route — one bad message wedges the queue

A message that fails every retry must go SOMEWHERE terminal: a dead-letter queue, a parked table, a quarantine topic. Trace the exhausted-retries path for each consumer the diff adds: if the message returns to the head of the queue forever, one malformed payload halts all processing behind it. Ordered/single-partition consumers are the highest-blast-radius case. Where the message lands is your question; whether the handler's error path also acknowledges correctly under at-least-once delivery is transactional-integrity's.

3. 🟡 [convention] No graceful-shutdown drain — in-flight work lost on deploy

Every deploy sends this worker a termination signal mid-message. Does the diff's worker stop taking new work, finish (or cleanly nack) what is in flight, and exit within the platform's grace period? Look for a shutdown hook, drain loop, or the framework's built-in drain — and check the termination grace configured for the deployment. Exemptions are real: a stateless cron job reading a read-only source loses nothing on interruption, and an at-least-once transport redelivers whatever was in flight (making drain an efficiency concern, not a loss) — say explicitly which exemption you are relying on. The item bites hardest for at-least-once consumers with long-running in-flight work and for anything ack-early.

4. 🟢 [convention] Unbounded concurrency/prefetch

Does the new consumer bound how many messages it processes at once — worker-pool size, prefetch/fetch-count, max in-flight? An unbounded consumer amplifies a queue backlog into a self-inflicted outage: memory blowup, connection-pool exhaustion, a thundering herd against the downstream the handler calls. Check the transport defaults before flagging — some default to a sane prefetch; some default to unlimited. A low-volume schedule-driven job with structurally bounded input is exempt.

5. 🟢 [convention] Visibility-timeout vs processing-time reasoning absent

Where the transport uses a visibility timeout, lease, or lock (with redelivery when it expires): is there any evidence — a comment, a config value derived from measurements, a heartbeat/extension call — that the timeout exceeds the handler's realistic worst-case processing time? A timeout shorter than processing time means the message redelivers WHILE the first attempt is still running: duplicate concurrent processing by design. You flag the absent reasoning; whether the handler survives that concurrent duplicate is transactional-integrity's question. Transports with no visibility/lease mechanism are exempt.

6. 🟡 [convention] Job queue used for synchronous request-path work

Does request-path code enqueue a job and then WAIT for its result before responding — polling job status, blocking on a completion channel, sleeping in a loop? That buys the queue's costs (latency, serialization, a second failure domain, at-least-once semantics) without its benefit (durable async execution), and it serializes under load: N concurrent requests queue behind a worker pool sized for background throughput. The synchronous shape is a direct call (with a timeout); the queue is for work that outlives the request. Legitimate exceptions exist — a durable workflow where the caller genuinely resumes later, or intentional admission control — but they should be stated; flag the enqueue-then-wait shape when they aren't. Also the inverse smell: an expensive in-process retry/sleep loop inside a job handler where the queue's own scheduling (snooze/requeue with delay) is the idiomatic tool — the handler holds a worker slot hostage to sleep.

Ground-truth discipline

• READ the transport/framework configuration, not just the handler — retry counts, DLQ wiring, prefetch, and drain behavior live in config and registration code, not in the handler body.
• fs_grep sibling consumers for the established idioms (DLQ naming, shutdown hooks, backoff helpers) — a new consumer skipping the house pattern is the strongest form of evidence; cite the sibling at file:line.
• Check the deployment manifests for termination grace periods when reasoning about drain — the code's drain loop is only as good as the time the platform gives it.
• Do not flag a missing guard the transport demonstrably provides; cite the config that provides it instead.

What this skill does NOT check

• Whether the handler is idempotent under redelivery, atomic across writes, or safe against dual-writes to a DB plus an external system → transactional-integrity (state-changing worker diffs load both skills).
• Whether job payloads, queue names, or worker logs expose secrets or abusable data → security-review and logging-discipline.
• Metrics, alerts, and dashboards for the new worker (queue depth, processing lag, failure rate) → observability-review.
• Log lines, levels, and message conventions inside the handler → logging-discipline.