Add compare-and-set update for DCR credentials

[tgrunnagle]

Summary

DCRCredentialStore can write a DCR row only if it is absent (StoreDCRCredentialsIfAbsent) or only if it is present (UpdateDCRCredentialsIfPresent). It cannot overwrite a row only if the row still holds the value the caller read. Consumers that coordinate DCR re-registration across replicas need that guarantee. Suppose a lock holder passes its ownership check and then stalls past its lease (a GC pause or a frozen process). In the meantime another replica registers and persists a newer client. When the stalled holder resumes, its UpdateDCRCredentialsIfPresent overwrites that newer row. Consumers can't close this gap themselves: the lock key and the row key hash to different Cluster slots, and the DCR key format and stored encoding are unexported.

• Add UpdateDCRCredentialsIfUnchanged(ctx, creds, expected) to DCRCredentialStore. It overwrites the row at creds.Key only if the stored row still equals expected. It returns ErrNotFound (wrapped) when the row is absent and the new ErrDCRCredentialsChanged sentinel when the row differs. In both cases nothing is written.
• Input validation (validateDCRCompareAndSet) requires a non-nil expected whose Key matches creds.Key. This stops a caller from gating a write to one row on the contents of another.
• Redis: the compare and the write run in a WATCH/MULTI transaction on the single row key, the same pattern StoreDCRCredentialsIfAbsent uses. If EXEC aborts because of a concurrent touch, the operation retries up to maxDCRClaimRetries times. The TTL follows the same ClientSecretExpiresAt rules as UpdateDCRCredentialsIfPresent. The stored-form conversion moved out of marshalDCRCredentialsForStore into newStoredDCRCredentials, so the compare reuses it. This part is a refactor with no behavior change.
• Memory: the compare and the write both run while holding s.mu. Time fields are compared with time.Time.Equal.
• Regenerate the storage mock.

Closes #6757

Type of change

• Bug fix
• New feature
• Refactoring (no behavior change)
• Dependency update
• Documentation
• Other (describe):

Test plan

• Unit tests (task test)
• E2E tests (task test-e2e)
• Linting (task lint-fix)
• Manual testing (describe below)

New unit tests for both the memory and Redis (miniredis) backends cover these cases:

• an unchanged row is overwritten
• a changed row is refused and left intact
• an absent row returns not-found and no row is created
• invalid input: nil creds, nil expected, and mismatched keys
• a concurrent race in which N writers share one expected value: exactly one wins and the others get ErrDCRCredentialsChanged

Redis-only tests also cover the one-second stored time precision, the TTL rules, and connection failure.

Ran the DCR integration tests against a real Redis Sentinel cluster in Docker with go test -race -tags integration -run TestIntegration_DCRCredentials ./pkg/authserver/storage/. They passed, including the new TestIntegration_DCRCredentials_UpdateIfUnchanged.

Changes

File	Change
pkg/authserver/storage/types.go	Interface method, ErrDCRCredentialsChanged, validateDCRCompareAndSet
pkg/authserver/storage/redis.go	WATCH/MULTI implementation; newStoredDCRCredentials extracted from marshalDCRCredentialsForStore
pkg/authserver/storage/memory.go	Mutex-guarded implementation and dcrCredentialsEqual
pkg/authserver/storage/mocks/mock_storage.go	Regenerated mock
pkg/authserver/storage/*_test.go	Unit, concurrency and Sentinel integration tests

Does this introduce a user-facing change?

No. This is a new storage API for internal consumers. Any out-of-tree implementation of DCRCredentialStore must add the new method.

Special notes for reviewers

• Whole-value comparison, not etag/version. The issue allowed either design. Comparing whole values needed no change to the storage format. Redis compares decoded stored forms, not raw JSON bytes. Field ordering and rows written by older encoders therefore don't affect the result, and a value returned by GetDCRCredentials always matches its row even though times are stored at one-second precision. The trade-off: a decorator that transforms fields (for example, encryption) must pass the stored form of expected, meaning what the inner GetDCRCredentials returned. The interface doc comment says so.
• WATCH/MULTI instead of a Lua script. The issue suggested Lua. WATCH/MULTI on one key gives the same single-key atomicity and matches the existing StoreDCRCredentialsIfAbsent code path.
• Redis Cluster mode has no test, because the repo has no Cluster harness. Cluster safety rests on the operation touching only one key, the same reasoning that applies to StoreDCRCredentialsIfAbsent. Standalone mode is covered by the miniredis tests and Sentinel mode by the integration tests.

🤖 Generated with Claude Code

[tgrunnagle]

One consumer-side note on the ErrNotFound outcome of UpdateDCRCredentialsIfUnchanged.

A caller typically reads a row, decides to replace it, and then calls this method with expected set to the value it read. Between that read and the update, the row can disappear: a row with ClientSecretExpiresAt set carries a backend TTL, so it can simply expire. The method then returns ErrNotFound, which is correct, but that outcome means "the row I meant to replace is gone and the slot is free", not "someone else won the race".

The right recovery is to fall back to the create path, StoreDCRCredentialsIfAbsent (SET NX). That way the first writer still wins, and a caller that loses that step adopts the stored row. If the caller instead treats ErrNotFound as a hard failure, it throws away a registration it has already performed. If it treats ErrNotFound like ErrDCRCredentialsChanged, it re-reads, finds nothing, and has nothing to adopt.

It may be worth one sentence in the interface doc comment, next to the existing ErrNotFound description, saying this outcome means the row vanished after the caller's read and pointing callers to StoreDCRCredentialsIfAbsent.

[tgrunnagle]

Multi-Agent Consensus Review

Agents consulted: concurrency-reviewer, api-reviewer, test-reviewer

Consensus Summary

#	Finding	Consensus	Severity	Action
1	Whole-value contract is hard for randomized-encryption decorators	7/10	MEDIUM	Discuss
2	Retry, corrupt-row and expired-but-present paths untested	8/10	MEDIUM	Fix
3	No per-field comparison test (times, ClientSecretExpiresAt)	7/10	MEDIUM	Fix
4	Retry exhaustion is an undocumented third outcome	8/10	LOW	Fix
5	Struct == silently misses a future time.Time field	8/10	LOW	Fix
6	maxDCRClaimRetries comment is stale	7/10	LOW	Fix

Overall

This adds a single-key compare-and-set to DCRCredentialStore. The memory backend compares and writes under s.mu. The Redis backend compares decoded stored forms in a WATCH/MULTI on one key. That closes the stalled-lock-holder window from #6757 and keeps the operation Cluster-safe. The approach is sound, and the newStoredDCRCredentials extraction preserves existing behavior.

The findings are about contract clarity and test depth rather than correctness. The main design question is how an encrypting decorator can supply the "stored form" of expected when the cipher is randomized. The main test gaps are the retry, exhaustion and corrupt-row paths, and field-by-field coverage of the comparison.

Documentation

The comment on maxDCRClaimRetries (redis.go:42) still describes only StoreDCRCredentialsIfAbsent.

---

Generated with Claude Code

[codecov]

Codecov Report

❌ Patch coverage is 97.33333% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 79.32%. Comparing base (35cfadf) to head (1700085).

Files with missing lines	Patch %	Lines
pkg/authserver/storage/redis.go	96.07%	2 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #6758   +/-   ##
=======================================
  Coverage   79.31%   79.32%           
=======================================
  Files         802      802           
  Lines       81372    81430   +58     
=======================================
+ Hits        64544    64592   +48     
- Misses      16823    16833   +10     
  Partials        5        5           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.