fix(redis): keep the reap sweep alive when a claimed payload is gone

[claudear]

The error

Sentry CLOUD-3R40 — RuntimeException: Queue delivery payload is missing, 100 events, thrown from Broker\Redis::reap() under maintain() on the Swoole adapter's maintenance tick.

at Utopia\Queue\Broker\Redis->reap  (src/Queue/Broker/Redis.php:457)
at Utopia\Queue\Broker\Redis->maintain (src/Queue/Broker/Redis.php:268)
at Utopia\Queue\Adapter->maintain (src/Queue/Adapter.php:226)
   (src/Queue/Adapter/Swoole.php:357)

Root cause

reap() read the ownership record and then the payload as two separate commands, and treated "owner present, payload gone" as impossible — "Only legacy payloads expire while processing" — by throwing. Two ordinary situations produce exactly that state:

• A concurrent settle. settle.lua deletes the owner key and the job key in one atomic call. A commit landing between this sweep's GET owner and GET job leaves the sweep holding a token for a delivery that no longer exists. On a busy queue a sweep scans up to 2000 claims, two round trips each, while workers commit throughout.
• Eviction. claim.lua stores the job key without a TTL, so under an allkeys-* maxmemory policy a claimed payload can be evicted while its owner key survives.

The throw did more than log noise: it aborted the sweep, so every claim behind the broken one went unrecovered, and where the state persisted, that queue's reaping never made progress again.

The fix

A payload-less claim cannot be requeued — there is no payload to put back. It is now parked on the dead list like an exhausted claim, through the same reclaim script the rest of the sweep uses, so the decision stays atomic:

• ownership must still match, so a delivery another consumer already settled is left alone (the race above becomes a no-op, counted as retained);
• a live heartbeat still wins, so a worker actually holding the delivery keeps it;
• reclaim.lua only insists on the payload key when it is replaying a payload — the case where its absence means someone else got there first. Parking needs no payload.

Legacy claims (no ownership record) keep their existing behaviour: dropped from the processing list.

Tests

Written first, against real Redis; all three reproduced the exact exception at Redis.php:457 before the fix.

• testReapClearsAnOwnedClaimWhosePayloadIsGone — the claim is cleared, parked on the dead list, its ownership record removed and the processing counter settled.
• testReapKeepsSweepingPastAClaimWhosePayloadIsGone — the stranded claim behind the broken one is still recovered.
• testAHeartbeatedClaimWithoutItsPayloadIsLeftAlone — ownership plus a live heartbeat means the sweep does not take the delivery from its worker.

Full unit suite (143) and e2e suite (149, KEDA excluded — no kind cluster locally) pass against the compose services. PHPStan reports no new errors.

🤖 Generated with Claude Code

[github-actions]

Thanks for contributing! This repository is a read-only mirror; development for this library happens in packages/queue in the utopia-php monorepo. Please open this pull request there instead.

[greptile-apps]

Confidence Score: 4/5

The production change appears behaviorally sound, but the explicit repository testing requirement must be satisfied before merging.

Findings

1.  Tests Mirror Redis Internals ▶

Fix with agent prompt

### Issue 1
tests/Queue/E2E/Adapter/RedisBrokerRecoveryTest.php:105-106
These tests hard-code and mutate the Redis `.claims`, `.jobs`, `.owners`, and `.stats` key layout, then assert those same storage details. This violates the repository directive to test observable behavior rather than mirror source implementation or configuration. A harmless key-layout refactor would break these tests even if broker behavior remained correct. Replace the duplicated key construction and internal-state checks with behavior-oriented fault injection and broker-visible recovery outcomes. The repository requirement must be satisfied before merging; the same pattern also appears at lines 114–115, 124–125, and 141.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Summary

This PR keeps Redis claim reaping operational when an owned delivery loses its payload. It classifies the claim as unrecoverable, atomically parks its PID on the dead list when ownership remains valid and no heartbeat exists, and adds recovery coverage.

• Preserves live, heartbeated claims even when their payload key is absent.
• Allows dead parking without requiring the missing job key while retaining the payload check for normal requeueing.
• Verifies that a missing payload no longer prevents later claims in the sweep from being recovered.
• The new tests are excessively coupled to Redis key-layout internals and violate the repository’s explicit testing rule.

_{Reviews (1) · Last reviewed commit: "fix(redis): keep the reap sweep alive wh..."}

[greptile-apps]

Tests Mirror Redis Internals

These tests hard-code and mutate the Redis .claims, .jobs, .owners, and .stats key layout, then assert those same storage details. This violates the repository directive to test observable behavior rather than mirror source implementation or configuration. A harmless key-layout refactor would break these tests even if broker behavior remained correct. Replace the duplicated key construction and internal-state checks with behavior-oriented fault injection and broker-visible recovery outcomes. The repository requirement must be satisfied before merging; the same pattern also appears at lines 114–115, 124–125, and 141.

Context Used: Call out and harshly judge implementation-coupled tests. We don't mirror source code, configuration, or version pins in assertions. We test observable behavior; use linters for syntax and schema checks. (source)

Prompt To Fix With AI

This is a comment left during a code review.
Path: tests/Queue/E2E/Adapter/RedisBrokerRecoveryTest.php
Line: 105-106

Comment:
**Tests Mirror Redis Internals**

These tests hard-code and mutate the Redis `.claims`, `.jobs`, `.owners`, and `.stats` key layout, then assert those same storage details. This violates the repository directive to test observable behavior rather than mirror source implementation or configuration. A harmless key-layout refactor would break these tests even if broker behavior remained correct. Replace the duplicated key construction and internal-state checks with behavior-oriented fault injection and broker-visible recovery outcomes. The repository requirement must be satisfied before merging; the same pattern also appears at lines 114–115, 124–125, and 141.

**Context Used:** Call out and harshly judge implementation-coupled tests. We don't mirror source code, configuration, or version pins in assertions. We test observable behavior; use linters for syntax and schema checks. ([source](https://app.greptile.com/review/custom-context?memory=instruction-0))

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!