fix(db): tolerate not-yet-ready members in Patroni cluster discovery

[Evrard-Nil]

Symptom

Adding a new postgres replica triggered this in both cloud-api and chat-api (shared database crate):

ERROR database::patroni_discovery: Failed to refresh cluster state:
Failed to parse cluster info: error decoding response body

Exact root cause (confirmed empirically)

Patroni lists a node as a /cluster member as soon as it registers in the DCS — before it publishes its conn_url. So a replica mid-creation (state creating replica, or an uninitialized node) appears without host/port/api_url. Those fields are required on ClusterMember, so a strict Vec<ClusterMember> parse fails the entire response.

Reproduced against the real struct in a unit test:

PARSE ERROR: missing field `host` at line 3 column 84

One half-registered member poisons the whole cluster view → discovery refresh errors for every consumer until the new node finishes registering (and startup discovery would fail outright during that window). This is the same class as the prior lag:"unknown" poison — different field.

Fix

Parse members individually (deserialize_members) and drop any that don't fully deserialize — a member with no connection info isn't a usable leader/replica target anyway, and the next refresh picks it up once it finishes registering. ClusterMember stays rigid, so cluster_manager is untouched (every surviving member still has host/port). Same philosophy as the existing deserialize_lag resilience.

Tests

• test_initializing_member_does_not_poison_parse — leader + one creating replica (no host/port) → parses, bad member dropped.
• test_full_cluster_with_initializing_replica_parses — real staging /cluster shape + an initializing replica → 3 ready members survive, leader discoverable.
• All existing patroni_discovery tests still pass (6 passed; 0 failed).

Validation

Verified in dev (deterministic repro against the real struct). End-to-end validation: deploy this, then add a staging postgres replica and confirm discovery stays healthy through the member's initialization window. Unblocks deploying new postgres instances.

[gemini-code-assist]

Code Review

This pull request introduces a custom deserializer, deserialize_members, for Patroni cluster members to handle half-registered nodes gracefully. Instead of failing the entire /cluster response parsing when a member is missing connection info, it drops the incomplete member and logs a warning. Unit tests were also added to verify this behavior. The review feedback suggests optimizing the deserialization loop by extracting the member's name before calling serde_json::from_value, thereby avoiding cloning the serde_json::Value on every iteration and allowing ownership transfer.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

[gemini-code-assist]

By extracting the member's name before deserialization, we can avoid cloning the entire serde_json::Value on every iteration. serde_json::from_value can then take ownership of the Value directly, which allows it to reuse the internal allocations (like strings) when constructing the ClusterMember struct.

Suggested change

	for value in raw {
	match serde_json::from_value::<ClusterMember>(value.clone()) {
	Ok(member) => members.push(member),
	Err(e) => {
	let name = value
	.get("name")
	.and_then(serde_json::Value::as_str)
	.unwrap_or("<unknown>");
	warn!("Skipping not-yet-ready cluster member {name}: {e}");
	}
	}
	}
	for value in raw {
	let name = value
	.get("name")
	.and_then(serde_json::Value::as_str)
	.map(String::from);
	match serde_json::from_value::<ClusterMember>(value) {
	Ok(member) => members.push(member),
	Err(e) => {
	let name = name.as_deref().unwrap_or("<unknown>");
	warn!("Skipping not-yet-ready cluster member {name}: {e}");
	}
	}
	}

[claude]

Review — PR #794: tolerate not-yet-ready Patroni members

Focused, well-tested fix for a real production failure. The root-cause writeup is empirically grounded and the approach mirrors the existing deserialize_lag resilience pattern, keeping ClusterMember rigid so downstream cluster_manager stays untouched. No existing review threads to build on.

Correctness verified:

• deserialize_members parses into Vec<Value> then deserializes each member independently, dropping failures with a warn! — exactly the right granularity.
• Downstream is safe: update_cluster_state (patroni_discovery.rs:180) filters surviving members by state and handles an empty/leaderless result, so dropping half-registered nodes can't break selection.
• Privacy: the warn! logs only member name (infra hostname) + serde error — no customer data. serde_json "missing field" errors don't echo values. ✅
• Backward compatibility preserved — members was already a required field; behavior only relaxes per-member strictness.

Minor (non-blocking) nits:

• test_full_cluster_with_initializing_replica_parses doc comment says "The 4 ready members must still parse" but the fixture has 3 ready members (a, b, leader) + 1 initializing (newrep), and the assertion correctly checks len == 3. Comment should say 3.
• value.clone() in the parse loop (line 67) clones each JSON value so value survives for the error-path name lookup. Negligible given cluster sizes, but if you want to avoid it you could lift the name extraction before the from_value call and consume value. Not worth changing.

No critical issues. Logic, error paths, and tests all hold up.

✅ (approved)

[PierreLeGuen]

The fix isolates member-level parse failures so a single not-yet-ready node can't poison the whole /cluster topology response, mirroring the existing deserialize_lag strategy. Small, correct, and covered by two new regression tests.

Non-blocking follow-ups:

• The warn! for dropped members logs only member name + serde error, no customer data (patroni_discovery.rs:60-79) — confirmed safe; downstream consumers in cluster_manager.rs only see fully-deserialized members.
• Doc comment in test_full_cluster_with_initializing_replica_parses says "4 ready members" but the fixture has 3 (the assertion correctly checks len == 3) — cosmetic.
• cargo fmt --check fails on two changed assertions in patroni_discovery.rs, and CI Lint is red for the same diff — worth a quick cargo fmt before merge.

Checks: cargo test -p database patroni_discovery (6/6 passed, incl. 2 new regression tests) and cargo clippy -p database --lib clean; cargo fmt --check / CI Lint failed on formatting.