[LFXV2-2190, LFXV2-2193] fix: lazy-bind weekly-brief KV buckets and fix query-service version param

[andrest50]

Summary

• Replaces the one-shot best-effort startup loop for weekly-brief KV buckets with a GetOrBindKVStore method that binds on first request access under a RWMutex
• Pods that started before the buckets were provisioned now self-heal on the first incoming request instead of returning 503 for their entire lifetime
• Removes the need for a manual rolling restart to recover from the startup race condition
• Adds missing version=1 query parameter to all three m2m source clients (meetings, mailing lists, votes); the query-service requires this param and was returning HTTP 400, causing NATS to exhaust MaxDeliver:3 and orphan every brief permanently in generating state

How it works

GetOrBindKVStore uses a double-checked locking pattern: read-lock fast path for the common case (bucket already bound), write-lock slow path for first-access binding. Once bound, subsequent requests take the fast path. The throttle bucket in GetGroupWeeklyBriefForWindow retains its best-effort semantics — a bind failure there is still silently skipped since throttle data is advisory.

Tickets

LFXV2-2190 — committee-service: GET weekly-briefs/current 503s "bucket not initialized" though bucket exists + writes persist (read-path bug)

LFXV2-2193 — committee-service: weekly brief stuck in "generating" state; all source fetches returning 400 due to missing version=1 query param

🤖 Generated with Claude Code

[coderabbitai]

Walkthrough

The NATS client now synchronizes lazy KV-store binding. Weekly-brief storage uses this resolver for reads and writes. Mailing-list and meeting queries add v=1; vote queries add version=1.

Changes

NATS KV binding

Layer / File(s)	Summary
Synchronized lazy KV resolution internal/infrastructure/nats/client.go	NATSClient protects cached KV stores with an RWMutex. GetOrBindKVStore checks, binds, caches, and returns buckets. Binding failures return ServiceUnavailable. Weekly-brief buckets are no longer initialized during startup.
Weekly-brief storage integration internal/infrastructure/nats/group_weekly_brief_storage.go	Weekly-brief and throttle operations obtain buckets through GetOrBindKVStore. Required-operation errors are returned. Advisory throttle-read errors remain ignored.

M2M API versioning

Layer / File(s)	Summary
Versioned M2M resource queries internal/infrastructure/m2m/mailing_list_source.go, internal/infrastructure/m2m/meeting_source.go, internal/infrastructure/m2m/vote_source.go	Mailing-list and meeting requests include v=1. Vote requests include version=1.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant WeeklyBriefStorage
  participant NATSClient
  participant JetStream
  WeeklyBriefStorage->>NATSClient: GetOrBindKVStore(bucketName)
  NATSClient->>NATSClient: Check cached bucket under read lock
  NATSClient->>JetStream: Bind missing bucket
  JetStream-->>NATSClient: Return KeyValue bucket
  NATSClient-->>WeeklyBriefStorage: Return bucket or ServiceUnavailable

Loading

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly summarizes both main changes: lazy binding for weekly-brief KV buckets and the query-service version parameter fix.
Description check	✅ Passed	The description directly explains the lazy KV binding changes, self-healing behavior, throttle semantics, and query parameter fixes.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feat/LFXV2-2190-lazy-bind-weekly-brief-kv-buckets

---

_{Comment @coderabbitai help to get the list of available commands.}

[Copilot]

Pull request overview

Adds lazy binding for weekly-brief NATS KV buckets so pods recover without restarting.

Changes:

• Adds mutex-protected first-access KV binding.
• Routes weekly-brief storage operations through the new binding method.
• Removes best-effort startup binding.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
internal/infrastructure/nats/client.go	Implements lazy KV binding and removes startup binding.
internal/infrastructure/nats/group_weekly_brief_storage.go	Uses lazy binding for weekly-brief buckets.

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@internal/infrastructure/nats/client.go`:
- Around line 88-117: Update GetOrBindKVStore so the potentially blocking
js.KeyValue call runs without holding c.kvMu. Recheck the cache before binding,
perform the bind outside the global lock, then reacquire the lock to store the
handle while preserving an already-cached handle if another request won the
race; use per-bucket coordination only if duplicate binds must be prevented.
- Around line 91-115: Synchronize every access to c.kvStore consistently across
GetOrBindKVStore, KeyValueStore, and the storage methods that read it directly.
Protect all lookups and writes with kvMu, or route them through a shared
synchronized accessor, while preserving the existing caching and binding
behavior.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 7c43db14-e434-4689-9a43-8a750bac04ad

📥 Commits

Reviewing files that changed from the base of the PR and between 2231bc5 and 8d5f8ba.

📒 Files selected for processing (2)

• internal/infrastructure/nats/client.go
• internal/infrastructure/nats/group_weekly_brief_storage.go

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Suppressed comments (1)

internal/infrastructure/nats/client.go:115

• This mutates kvStore while most storage paths still read that same map directly without kvMu (for example, storage.go:40 and storage.go:114). A first weekly-brief bind concurrent with any ordinary committee request can therefore race and trigger concurrent map read and map write. Keep lazy handles in a separate mutex-protected map, or route every access to kvStore through the same lock before writing it at runtime.

	c.kvStore[bucketName] = kv

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

Suppressed comments (3)

internal/infrastructure/m2m/meeting_source.go:100

• This required parameter is missing from MeetingSource’s documented request at lines 40–41, which still shows /query/resources?type=.... That stale example documents the same invalid request shape this change fixes; please add version=1 to it so the adapter contract matches the request sent here.

	q.Set("version", "1")

internal/infrastructure/m2m/mailing_list_source.go:87

• This required parameter is missing from MailingListSource’s documented request at lines 36–37, which still shows /query/resources?type=.... That stale example documents the same invalid request shape this change fixes; please add version=1 so the adapter contract matches the request sent here.

	q.Set("version", "1")

internal/infrastructure/m2m/vote_source.go:87

• This required parameter is missing from VoteSource’s documented request at lines 36–37, which still shows /query/resources?type=.... That stale example documents the same invalid request shape this change fixes; please add version=1 so the adapter contract matches the request sent here.

	q.Set("version", "1")

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

Suppressed comments (4)

internal/infrastructure/nats/client.go:98

• The lazy-bind and synchronization path has no regression coverage; the only weekly-brief NATS test covers key formatting. Add a co-located test using an in-process JetStream server that verifies a failed lookup succeeds after the bucket is created and that concurrent first accesses are race-free. Otherwise the startup-race fix and its cache locking can regress while the suite remains green.

func (c *NATSClient) GetOrBindKVStore(ctx context.Context, bucketName string) (jetstream.KeyValue, error) {

internal/infrastructure/m2m/meeting_source.go:100

• The existing TestMeetingSource_M2MTokenUsed_NotCallerBearer exercises this request but never asserts the required API-version query parameter. Extend its httptest handler (or add a table case) to require r.URL.Query().Get("v") == "1"; without that assertion, removing this line recreates the reported HTTP 400 failure while tests still pass.

	q.Set("v", "1")

internal/infrastructure/m2m/vote_source.go:87

• No test exercises the vote source request, so the newly required v=1 contract is unprotected. Add a co-located httptest-based method test that rejects requests unless r.URL.Query().Get("v") == "1"; otherwise this exact HTTP 400 regression can return unnoticed.

	q.Set("v", "1")

internal/infrastructure/m2m/mailing_list_source.go:87

• No test exercises the mailing-list source request, so the newly required v=1 contract is unprotected. Add a co-located httptest-based method test that rejects requests unless r.URL.Query().Get("v") == "1"; otherwise this exact HTTP 400 regression can return unnoticed.

	q.Set("v", "1")