Task 3 report — RedisStore (durable, HA-safe jobs.Store)

Status: DONE. go build, go vet, go test ./..., and go test -race ./internal/jobs/... all green.

Files

• internal/jobs/redisstore.go (new) — RedisStore implementing the full jobs.Store interface + NewRedis(rdb).
• internal/jobs/redisstore_test.go (new) — miniredis-backed tests (mirrors of the LocalStore behaviors + Redis-specific concurrency/durability).

No existing files modified (additive).

Redis key layout

Key	Type	Contents
job:{id}	HASH	full job record — every field of Job, one hash field each
job:{id}:payload	STRING	raw submit bytes (kept out of the hash; may be large)
job:{parent}:children	SET	child ids for fan-out (Task 8)
jobs:ready:{class}	LIST	per-class queue; LPUSH at head, claim from tail → FIFO
jobs:processing	LIST	acks-late in-flight list (RPOPLPUSH target)
idem:{tenant}:{key}	STRING	(tenant, idem-key) → job id; SETNX is the atomic dedup point

Timestamps are stored as Unix-millisecond decimal strings ("" for a zero time). Rationale: ms is well inside Lua's double (2^53) range, so the reclaim script compares heartbeats numerically without precision loss — nanoseconds (~1.75e18) would exceed 2^53 and corrupt the Before(cutoff) boundary. Ms is far finer than any visibility timeout. Claim order is the ready-list FIFO (not CreatedAt), so sub-ms CreatedAt ties never affect fairness.

TTLs (consts, documented): defaultIdemTTL = 24h on the idem key (dedup window / result-retention floor); defaultJobTTL = 24h on the job hash + payload + children. Both are safely longer than GPU_QUEUE_WAIT_MS (30 min) + processing, so an in-flight job never expires under it. Configurable via the RedisStore.idemTTL/jobTTL fields.

Atomicity approach

• Dedup (Create): write our hash+payload first, then SETNX idem:{tenant}:{key} = id. The SETNX loser deletes its orphan hash/payload and loads the winner's already-complete record — so no reader ever sees a half-written job. Winner enqueues on jobs:ready:{class} and (if ParentID) SADDs to the parent's children set. If the idem key points at a vanished (expired) job, the key is repointed at the fresh record and we fall through to a new create (mirrors LocalStore's "indexed job gone → create").
• ClaimNext (claim-once): a single Lua script does RPOPLPUSH ready→processing and flips the hash to processing + sets StartedAt/Heartbeat + HINCRBY attempts 1, atomically. RPOPLPUSH atomicity alone guarantees at-most-once handoff; folding the stamp into the same script closes the window where a just-claimed id would look "fresh but unstamped" to a concurrent reclaim. Genuinely class-scoped (per-call = one class), converging with LocalStore's advisory priority once the manager (Task 7) loops classes interactive→system→batch. Non-blocking RPOPLPUSH is used (the manager polls; brief-directed, and miniredis BRPOPLPUSH is limited).
• ReclaimStale: a single Lua script scans jobs:processing; per id still processing with heartbeat < cutoff (strict), it LREMs (guard: LREM > 0 makes the re-queue idempotent so two replicas can't double-requeue) then LPUSHes it back onto its class ready list with status queued. Attempts already bumped at claim.
• UpdateStatus: optimistic WATCH/MULTI retry loop (the mutator is arbitrary Go, so it can't run in Lua) — a concurrent write aborts EXEC (redis.TxFailedErr) and we reload+retry (bounded at 50).
• Heartbeat / SetResult: tiny EXISTS-guarded Lua so a plain HSET can't resurrect an expired/unknown job; return 0 → ErrNotFound.
• Get / Payload / ListChildren: HGETALL (empty map → ErrNotFound); Payload existence-checks the hash then GETs the payload string; ListChildren SMEMBERS → per-id Get, sorted by (CreatedAt, ID).

miniredis limitations

Probed all commands before relying on them (a throwaway test): variadic HSET, SetNX, RPOPLPUSH/HINCRBY/HSET/LREM/LRANGE/HGET/EXISTS inside EVAL, EVAL returning false → redis.Nil, and WATCH/MULTI/TxPipelined optimistic transactions — all supported by miniredis v2.38.0. No feature had to be skipped or faked; no blocking BRPOPLPUSH was used. Tests use miniredis.RunT(t) exactly like redispool_test.go (no external Redis on this box).

RED → GREEN

RED: tests written first — the package failed to compile (RedisStore/NewRedis undefined). GREEN: after implementing redisstore.go, all tests pass. The concurrency tests are load-bearing: TestRedisCreateDedupAtomic (16 concurrent Creates → exactly 1 created, all ids identical, only 1 claimable) would fail without the SETNX dedup; TestRedisClaimOnce (8 claimers over 20 jobs → each claimed exactly once, total == 20) would fail without atomic RPOPLPUSH. All 11 Redis tests pass 3× each under -race.

Tests (all miniredis, namespace-safe t3-* ids/tenants)

Mirrors of LocalStore: CreateDedup, ClaimNextPerClass, ClaimFIFOWithinClass, ReclaimStale, ReclaimSkipsFreshHeartbeat, ListChildren, SetResultAndGet, GetUnknown (+ Payload/Heartbeat/SetResult/UpdateStatus unknown). Redis-specific: CreateDedupAtomic, ClaimOnce, RestartDurability (a fresh RedisStore on the same miniredis sees record + payload + processing status + timestamps + attempts).

Concerns / notes for later tasks

• ClaimNext swallows non-redis.Nil errors as "nothing to claim" (the (*Job, bool) signature has no error return). If Redis is down the manager (Task 7) sees empty claims — acceptable given the fail-open posture in redispool.go, but Task 7 should not treat "no claim" as "queue empty" for backpressure without a health signal.
• No Delete/GC of terminal jobs beyond TTL — fine for P2; a sweeper can come later if 24h retention proves too coarse.

Note: this file previously held a stale report from an unrelated earlier "Task 3" (LocalPool); it was overwritten intentionally.

---

Review fix (2026-07-06): EXISTS-guard claim/reclaim against expired-hash auto-vivify

Task-3 review (.superpowers/sdd/task-3-review.md, verdict "Needs fixes") found 1 Important + 2 Minor. All three addressed on branch feat/gateway-p2-jobs-ocr.

Important — claim/reclaim EXISTS-guard (redisstore.go)

The jobs:ready:{class} and jobs:processing lists have no TTL, but job:{id} carries an absolute 24h jobTTL from creation. So an id can outlive its hash. The old claimScript did RPOPLPUSH then an unconditional HSET, which Redis auto-vivifies into a blank hash — ClaimNext then returned a "processing" job with empty tenant/model/doc_hash and Payload() returned ([]byte{}, nil) instead of ErrNotFound, silently feeding the Task 7 manager an empty, unattributed OCR call.

• claimScript is now a loop: after each RPOPLPUSH it checks EXISTS job:{id}. On a hit it stamps + HINCRBY + returns the id (unchanged behavior). On a miss it LREMs the orphan back out of jobs:processing (so it can't linger) and keeps popping; an empty ready list returns false. Still a single atomic EVAL, still at-most-once, and a live job sitting behind expired ids is now still claimed rather than stranded.
• reclaimScript gets the symmetric guard: a leading EXISTS == 0 branch LREMs the expired-hash orphan out of jobs:processing and never re-queues it (no blank-hash resurrection onto a ready list). The live-and-stale path keeps its existing LREM > 0 idempotency guard unchanged.
• Atomicity-summary doc comment updated to describe both guards.

Minor — unchecked fire-and-forget Redis errors (redisstore.go)

Del (dedup-loser orphan cleanup) and Expire (children-set TTL) now capture .Err() into _ = with a comment documenting the deliberate best-effort semantics (a failure is harmless: the orphan is never enqueued so it just expires at jobTTL; a missed children Expire just lives to jobTTL off the last SAdd). No logging dependency introduced.

Minor — no active reaper for queued-past-GPU_QUEUE_WAIT_MS

Acknowledged as a Task 7 handoff item (unchanged from the original report's known-gap note); not a Task 3 code change.

Tests (TDD)

Added TestRedisClaimSkipsExpiredHash (an expired-hash id ahead of a live job in the same class list → claim skips the orphan, returns the live job, Get/Payload on the expired id stay ErrNotFound, no blank hash) and TestRedisClaimAllExpiredReportsNoClaim (a list of only expired ids drains to a clean no-claim, no blank records). RED: both failed against the old bare-HSET claimScript (claimed "" want live ...; ClaimNext claimed from a list of only expired ids). GREEN: both pass with the guard. All prior 11 Redis + 6 LocalStore tests still pass.

go build ./..., go vet ./..., go test -race -count=1 ./internal/jobs/... all green (19 test funcs pass).