one_d4: give the claim/lease protocol a ClaimRef, and drop a dead parameter

[aaylward]

Follow-up to #1438. -Wunused-parameter produced 55 annotations there; this is what two of them were actually pointing at.

The signal was in the distribution, not the count

Of the 25 annotated parameters in the worker's tests: id ×8, owner ×7, lease ×4, everything else ×6. Three quarters of them are the same two values, and not because the fakes are lazy — because both queue interfaces ask for them on every method:

ClaimNext(owner, lease)
Heartbeat(id, owner, lease)
Progress (id, owner, games_indexed)
Complete (id, owner, games_indexed)
Fail     (id, owner, message)
HandBack (id, owner)
Release  (id, owner)

ClaimNext hands you a claim; every call after it re-states which claim you meant, as two loose string_views. Nothing types the pairing — the fencing is a runtime SQL check, per call — so passing them in the wrong order compiles silently. ReanalysisQueue has the identical shape, so the same six signatures exist twice.

The type already existed: Poller::Claim is {IndexJob job; std::string owner;}, exactly the pair. It just was not the queue's currency.

ClaimRef

struct ClaimRef {
  std::string_view id;
  std::string_view owner;
};

Both queues take it in place of the two parameters, and Claim/ReanalysisClaim grow a ref() so a holder never spells the pair out again — queue_.Complete(claim.ref(), report->games_indexed).

The missing default member initializers are deliberate. With them, {.id = x} would compile and fence against an empty owner. Without them, #1438's -Wextra makes that a compile error, so a designated initializer has to name both. The two flags compose into something neither gives alone; the header says so, since it otherwise reads like an oversight.

Direct call sites use designated initializers ({.id = Id(1), .owner = "worker-2"}), which is where the swap protection actually lands — a positional {a, b} would still be order-dependent.

What it does to the fakes

An override that ignores the claim now takes one unnamed parameter instead of two annotated ones:

-  absl::StatusOr<bool> Complete([[maybe_unused]] std::string_view id,
-                                [[maybe_unused]] std::string_view owner,
-                                int games_indexed) override {
+  absl::StatusOr<bool> Complete(ClaimRef, int games_indexed) override {

Repo-wide [[maybe_unused]] count falls from 56 to 41, and the diff is net -22 lines across 19 files despite adding a header.

The dead parameter

HubHandler::StageGameOverLocked(room_id, room, game_id, outbox) took a room and that room's id, and used only the room. Deleted rather than annotated — this one was -Wunused-parameter finding genuine dead weight, which is the case for keeping the check on.

Scope: this is not #1417

#1417 wants the claim/lease SQL shared, and argues conformance-test-first because "refactoring first means refactoring under no test that would notice if it changed behavior." That ordering applies here too, so I checked before touching anything rather than after: pg_queue_test already has CompleteIsFencedOnOwnership, ProgressIsFencedOnOwnershipToo, FailIsFencedOnOwnershipToo, HandBackAndReleaseAreFencedToo, plus the lease-expiry pair. The fencing is covered; a swap would be caught.

This change is signature-only — no SQL text, no predicate, no SET clause moves. #1417's conformance suite and LeasedTable are untouched and still wanted; ClaimRef is what its parameter lists would be built on.

Verified

The gated suites actually ran. PG_TEST_DB_URL and GOLF_HUB_TEST_DB_URL are unset by default and those suites GTEST_SKIP silently, so a green local run proves nothing about the SQL — which is the entire surface this refactor touches. I brought up a local Postgres 16 and confirmed the counts rather than the exit code:

suite	ran	skipped
pg_queue_test	22	0
reanalysis_queue_test	16	0
game_state_jsonb_test	3	0

bazel test //domains/... //bazel/... with both URLs supplied: 218 tests pass, zero diagnostics from first-party files.

Mutation-checked. Swapping the pairing — ref() returning {.id = owner, .owner = job.id} — compiles (both are string-ish, which is the point) and fails poller_test. So the pairing is defended by a test, not just by the type.

The only build failure is @raylib//:raylib_cmake, which needs libxrandr headers this sandbox lacks; unrelated and CI builds it.

Review panel

Not run. Flagging that rather than letting the section header imply otherwise — the evidence here is the mutation check and the gated suites above.

🤖 Generated with Claude Code

https://claude.ai/code/session_01ST7XLGWstRE4CfUF84ijKD

---

Generated by Claude Code

[cloudflare-workers-and-pages]

Deploying with    Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status	Name	Latest Commit	Preview URL	Updated (UTC)
✅ Deployment successful! View logs	1d4-web	792b70a	Commit Preview URL Branch Preview URL	Aug 23 2026, 03:41 PM

[cloudflare-workers-and-pages]

Deploying with    Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status	Name	Latest Commit	Preview URL	Updated (UTC)
✅ Deployment successful! View logs	iili	792b70a	Commit Preview URL Branch Preview URL	Aug 23 2026, 03:42 PM

[cursor]

Review panel

Four read-only agents, four lenses (correctness/control flow; SQL/data/resource safety; tests/docs/CI; altitude). Each hunted then tried to refute. Aggregation re-checked the survivors against the head (f013bf04). CI on that commit is green, including build-and-test and the three sanitizers.

Surviving findings: none. Land as-is.

Verified clean

• Signature-only. SQL text in pg_queue.cc / reanalysis_queue.cc is unchanged vs main; binds are claim.id / claim.owner into the same $1/$2 slots. Fence predicates (owner_id, status, lease_expires_at > NOW()) untouched.
• Pairing. Claim::ref() / ReanalysisClaim::ref() emit {.id = job.id, .owner = owner} — same order the old positional args used. Production call sites use claim.ref() or designated initializers; no positional {a, b} ClaimRefs.
• Lifetimes. ClaimRef is non-owning for the call only. Pg* copies into std::string for Exec; QueueLease / PassLease own id_/owner_ as std::string and rebuild designated refs for Heartbeat/Progress (renewer thread needs owned storage).
• Control flow. Poller / reanalysis poller branch order unchanged; only argument packaging moved. StageGameOverLocked dropped an already-[[maybe_unused]] room_id; ceremony body identical; FinalizeGameLocked still uses room_id for StageRoomStateLocked.
• Tests. poller_test FakeQueue records claim.id on terminals and claim.owner on every fenced write; ExpectEveryWriteFencedOnTheClaim pins the pairing — matches the author’s swapped-ref() mutation claim. PG fencing suites updated to designated inits, coverage preserved. claim_ref BUILD wiring is enough under current layering.
• Altitude. Right layer for the silent (id, owner) swap hazard the unused-param distribution pointed at. Complementary to one_d4: share the claim/lease protocol between queues — starting with a conformance test, not a SQL mixin #1417 (SQL/protocol DRY), not a substitute. No-NSDMI + -Wextra is documented and rides the same domains/ -Werror build: no warnings in first-party C++ (#1408) #1438 locked in. Folding the golf_hub dead param into the same warning-driven PR is fine under the agreement.

Soft notes (not blockers; fold or ignore)

1. PassThrough in reanalysis_poller_test.cc rebuilds ClaimRef fieldwise ({.id = claim.id, .owner = claim.owner}) while SharedQueue in index_pool_test.cc already forwards claim through. Pure noise; forwarding would match.

2. reanalysis_poller_test FakeQueue still ignores the claim — pre-existing (it ignored both string_views before). Pairing coverage for the reanalysis path lives in the PG suite + Claim::ref() shape, not this fake. No new gap opened.

No inline comments. Approve from here.

---

Review panel via Cursor

[cursor]

Recheck (792b70ad)

Against the prior soft notes. Diff re-read; CI on this head is green (build-and-test + sanitizers). PR already merged as 558425fe.

• add scala 3 build support #1 PassThrough fieldwise rebuild: verified fixed. All six forwards now pass claim through (reanalysis_poller_test.cc:84–97), matching SharedQueue.
• improve scala support #2 FakeQueue ignores ClaimRef: left as attested — pre-existing; pairing still pinned by poller_test + PG fencing suites. No new gap.

Nothing else opened. Still LGTM.

---

Recheck via Cursor