fix(firestore): preserve atomic membership reads

[ReubenBond]

Firestore ReadAllAsync could return a torn membership view when a concurrent update landed while the emulator was streaming RunQuery responses. This surfaced as the table version and membership row status coming from different committed states in ConcurrentReadAll_ReturnsOnlyAtomicCommittedViews.

Execute full membership reads inside a Firestore transaction so every streamed row and the version document are bound to one serializable snapshot. If a concurrent write changes the transaction's read set, the Firestore SDK retries the read rather than returning a mixed view.

The protocol-level coverage now verifies the transaction selector, commit association, retry behavior, failure propagation, and cancellation behavior for full-table reads.

Microsoft Reviewers: Open in CodeFlow

[Copilot]

Copilot review overview

🟡 Changes recommended

The updated ReadAll retry test no longer asserts that returned ETags match the final committed snapshot, reducing coverage for a key part of the atomic-view contract.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1

Open (1)

• Restore ETag assertions after transaction retry · New

What changed in this PR

This PR fixes a Firestore membership-table consistency issue where ReadAllAsync could observe a torn view (rows and cluster version from different commits) when RunQuery streaming overlapped with concurrent updates, by executing full membership reads inside a Firestore transaction to bind the read to a single serializable snapshot.

Changes:

• Run FirestoreMembershipTable.ReadAllAsync inside a Firestore transaction to guarantee atomic, snapshot-consistent full-table reads (with SDK retry on read-set changes).
• Update the protocol-level tests to assert transactional query selectors, commit/transaction correlation, and retry behavior during streamed changes and error scenarios.

File	Description
src/​Google/​Orleans.Clustering.Firestore/​FirestoreMembershipTable.cs	Wraps ReadAllAsync in a Firestore transaction so version + rows are read from one serializable snapshot (with automatic retry on contention).
test/​Extensions/​Orleans.Clustering.Firestore.Tests/​FirestoreMembershipHeartbeatTests.cs	Adjusts protocol-level assertions to validate transactional reads/commits and retry behavior for ReadAllAsync while keeping ReadRowAsync non-transactional.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Copilot review overview

🔵 Needs a closer look

It changes the runtime consistency mechanism for cluster membership reads (introducing transactional reads and retry semantics), which warrants final human review despite strong targeted test updates.

Review effort: Lite
Findings: None

Resolved since last review (1)

• Restore ETag assertions after transaction retry

[github-actions]

Code coverage

Metric	Pull request
Lines	82.91% (115,337 / 139,117)
Branches	72.17% (33,454 / 46,353)

Report-only conclusion: current-main baseline stale.

The newest successful coverage run tested a68e9ed, not current main 8bc9fd2.

Coverage combines every CI test matrix job, including providers, CodeGen, .NET 8/10, Linux, Windows, and macOS, using canonical physical source and branch identities.

The comparison remains report-only while normal line and branch variance is calibrated.

Coverage details

• Pull request commit: b82036b101
• View the pull request test run and coverage artifacts
• View the trusted coverage reporting run