From 74d0e3afcf7508b0cfc5e2714d6f1d879e10d8f9 Mon Sep 17 00:00:00 2001 From: Soju06 Date: Fri, 14 Aug 2026 10:42:45 +0000 Subject: [PATCH 01/11] fix(cache): restore a pending bump whose write is cancelled or raises MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit _flush_pending_bumps clears each pending marker before awaiting its write — deliberately, so a request_bump() arriving mid-write re-queues rather than being coalesced into the version already being written. But it only restored the marker when bump() returned False. A write that was cancelled or raised left the namespace neither written nor pending. stop() cancels the polling task by design, so this silently dropped whatever namespace the poll loop was mid-write on at shutdown: a mutation that had already committed never reached peer replicas, leaving their caches stale until an unrelated later bump. It affects every namespace on the bus. Found while investigating #1354; independent of that fix. Co-Authored-By: Darafei Praliaskouski Co-Authored-By: Claude Fable 5 --- app/core/cache/invalidation.py | 10 +++- .../test_cache_invalidation_bus.py | 49 +++++++++++++++++++ 2 files changed, 58 insertions(+), 1 deletion(-) diff --git a/app/core/cache/invalidation.py b/app/core/cache/invalidation.py index 587108ea29..4748260fd3 100644 --- a/app/core/cache/invalidation.py +++ b/app/core/cache/invalidation.py @@ -284,8 +284,16 @@ async def _flush_pending_bumps(self) -> None: # later bump instead of being coalesced into the version already # being written. self._pending_bumps.discard(namespace) - if not await self.bump(namespace): + try: + if not await self.bump(namespace): + self._pending_bumps.add(namespace) + except BaseException: + # Includes CancelledError, which ``stop()`` raises in this task + # by design: without restoring the marker the namespace whose + # write was in flight is neither written nor pending, so a + # mutation that already committed never reaches peer replicas. self._pending_bumps.add(namespace) + raise async def _poll_once(self) -> bool: """Flush pending bumps and reconcile observed versions once. diff --git a/tests/integration/test_cache_invalidation_bus.py b/tests/integration/test_cache_invalidation_bus.py index fd38a4998f..2c2bbdf43b 100644 --- a/tests/integration/test_cache_invalidation_bus.py +++ b/tests/integration/test_cache_invalidation_bus.py @@ -386,6 +386,55 @@ def test_namespace_log_labels_cover_all_namespaces() -> None: } +@pytest.mark.asyncio +async def test_pending_bump_survives_a_cancelled_flush(db_setup, monkeypatch) -> None: + """The marker is cleared before the write is awaited, so a cancelled write + must restore it. stop() cancels the polling task by design, so without the + restore a mutation that already committed is neither written nor pending + and never reaches peer replicas.""" + namespace = "test_flush_cancelled" + started = asyncio.Event() + + async def never_finishes(ns: str) -> bool: + started.set() + await asyncio.Event().wait() + return True + + poller = CacheInvalidationPoller(SessionLocal) + monkeypatch.setattr(poller, "bump", never_finishes) + poller.request_bump(namespace) + + flush_task = asyncio.create_task(poller._flush_pending_bumps()) + await asyncio.wait_for(started.wait(), timeout=2.0) + assert namespace not in poller._pending_bumps, "marker is cleared before the write, by design" + + flush_task.cancel() + with pytest.raises(asyncio.CancelledError): + await flush_task + + assert namespace in poller._pending_bumps + assert await _namespace_version(namespace) is None + + +@pytest.mark.asyncio +async def test_pending_bump_survives_a_raising_flush(db_setup, monkeypatch) -> None: + """Same contract for a write that raises rather than returning False.""" + namespace = "test_flush_raised" + + async def raises(ns: str) -> bool: + raise RuntimeError("driver exploded") + + poller = CacheInvalidationPoller(SessionLocal) + monkeypatch.setattr(poller, "bump", raises) + poller.request_bump(namespace) + + with pytest.raises(RuntimeError, match="driver exploded"): + await poller._flush_pending_bumps() + + assert namespace in poller._pending_bumps + assert await _namespace_version(namespace) is None + + @pytest.mark.asyncio async def test_pending_coalesced_bump_flushes_after_recovery(db_setup) -> None: namespace = "test_pending_flush" From 2bf3e44b8662c41f4339210d6e9f6027cac4df9a Mon Sep 17 00:00:00 2001 From: Soju06 Date: Fri, 14 Aug 2026 11:31:17 +0000 Subject: [PATCH 02/11] docs(openspec): scope the change to the aborted-write fix MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The invalidation-bus spec already requires coalesced namespaces to remain pending until a bump succeeds; an aborted write silently dropping one violated that. Record the clarification rather than inventing a new shutdown-delivery contract — 'a lost bump still converges within the fallback TTL' is the documented bound and stays so. Co-Authored-By: Claude Fable 5 --- .../proposal.md | 21 +++++++++ .../specs/query-caching/spec.md | 43 +++++++++++++++++++ .../tasks.md | 12 ++++++ 3 files changed, 76 insertions(+) create mode 100644 openspec/changes/keep-aborted-invalidation-bumps-pending/proposal.md create mode 100644 openspec/changes/keep-aborted-invalidation-bumps-pending/specs/query-caching/spec.md create mode 100644 openspec/changes/keep-aborted-invalidation-bumps-pending/tasks.md diff --git a/openspec/changes/keep-aborted-invalidation-bumps-pending/proposal.md b/openspec/changes/keep-aborted-invalidation-bumps-pending/proposal.md new file mode 100644 index 0000000000..4e08ffec0e --- /dev/null +++ b/openspec/changes/keep-aborted-invalidation-bumps-pending/proposal.md @@ -0,0 +1,21 @@ +## Why + +The invalidation-bus spec already requires that "coalesced (`request_bump`) namespaces MUST remain pending and be retried on subsequent poll cycles until a bump succeeds". The implementation violated it for one case. + +`_flush_pending_bumps` clears each namespace's pending marker before awaiting its write — deliberately, so a `request_bump()` arriving mid-write re-queues instead of being coalesced into the version already being written. But it restored the marker only when `bump()` returned `False`. A write that was **cancelled** or **raised** left the namespace neither written nor pending, with nothing logged and no retry holding it. Since `_run` swallows poll exceptions and keeps cycling, a raising write silently lost its namespace during ordinary operation. + +## What Changes + +- Restore the pending marker when the bump write is cancelled or raises, so the required retry actually happens. `except BaseException` rather than `Exception`, because `CancelledError` is the case that matters. + +Process shutdown is deliberately out of scope: `stop()` cancels the polling task, so a bump queued at that moment has no cycle left to drain it. That is already the documented contract — "a lost bump still converges within the fallback TTL" — and guaranteeing delivery against an unresponsive database at shutdown is a separate concern with its own bounding and task-ownership design. + +## Capabilities + +### New Capabilities + +None. + +### Modified Capabilities + +- `query-caching`: state explicitly that an aborted (not merely failed) write keeps its namespace queued. diff --git a/openspec/changes/keep-aborted-invalidation-bumps-pending/specs/query-caching/spec.md b/openspec/changes/keep-aborted-invalidation-bumps-pending/specs/query-caching/spec.md new file mode 100644 index 0000000000..c6329e1540 --- /dev/null +++ b/openspec/changes/keep-aborted-invalidation-bumps-pending/specs/query-caching/spec.md @@ -0,0 +1,43 @@ +## MODIFIED Requirements + +### Requirement: Cache invalidation bumps and polling are resilient and observable +`bump()` MUST retry transient write failures (including SQLite "database is locked") with a short backoff; on final failure it MUST log at ERROR with the namespace, increment `codex_lb_cache_invalidation_bump_failures_total{namespace}`, and MUST NOT fail the originating mutation. Coalesced (`request_bump`) namespaces MUST remain pending and be retried on subsequent poll cycles until a bump succeeds — including when the write is aborted rather than merely failing, since the pending marker is cleared before the write is awaited, an aborted write MUST restore it — and a `request_bump` arriving while a flush for the same namespace is already awaiting its bump MUST be preserved and produce a later bump. When any invalidation callback for a namespace fails, the poller MUST NOT acknowledge the observed version and MUST re-run that namespace's callbacks on subsequent poll cycles until they succeed. The poller MUST escalate consecutive poll failures above debug level after a bounded count (WARNING after 3, ERROR after 10) and increment `codex_lb_cache_invalidation_poll_failures_total`. + +#### Scenario: Bump failure under database lock is observable and does not fail the mutation + +- **GIVEN** the database rejects cache-invalidation writes with a lock error for longer than the retry budget +- **WHEN** a mutation attempts a durable namespace bump +- **THEN** the mutation itself still succeeds +- **AND** an ERROR log naming the namespace is emitted and the bump-failure counter increments + +#### Scenario: Pending coalesced namespace flushes on the next successful cycle + +- **GIVEN** a coalesced `request_bump` namespace failed to flush during a poll cycle +- **WHEN** the database becomes writable again +- **THEN** the next poll cycle flushes the pending namespace and increments its version + +#### Scenario: Bump requested during an in-flight flush produces a later bump + +- **GIVEN** a coalesced flush is awaiting the bump write for a namespace +- **WHEN** another mutation commits and requests a bump for the same namespace before the flush completes +- **THEN** the namespace is re-queued and flushed again on a subsequent cycle, incrementing the version beyond the in-flight bump + +#### Scenario: Failed invalidation callback keeps the version unacknowledged and is retried + +- **GIVEN** a replica observes an `account_routing` version bump +- **AND** its routing snapshot refresh fails with a transient database error +- **WHEN** the poll cycle completes +- **THEN** the replica does not record the new version as seen +- **AND** the refresh is retried on subsequent poll cycles until it succeeds + +#### Scenario: Consecutive poll failures escalate above debug + +- **GIVEN** a replica's poller cannot read the `cache_invalidation` table +- **WHEN** three consecutive polls fail +- **THEN** a WARNING is logged and the poll-failure counter increments + +#### Scenario: An aborted bump write keeps its namespace queued + +- **GIVEN** a coalesced flush has cleared a namespace's pending marker and is awaiting its bump write +- **WHEN** that write is cancelled, or raises +- **THEN** the namespace is restored to the pending set for a later cycle, and no version is written diff --git a/openspec/changes/keep-aborted-invalidation-bumps-pending/tasks.md b/openspec/changes/keep-aborted-invalidation-bumps-pending/tasks.md new file mode 100644 index 0000000000..4966d88913 --- /dev/null +++ b/openspec/changes/keep-aborted-invalidation-bumps-pending/tasks.md @@ -0,0 +1,12 @@ +## 1. Fix + +- [x] 1.1 Restore the pending namespace in `_flush_pending_bumps` when the bump write is cancelled or raises, then re-raise + +## 2. Tests + +- [x] 2.1 A cancelled write restores the marker, and the marker is cleared before the write (locking in the intended coalescing) +- [x] 2.2 A raising write restores the marker + +## 3. Spec + +- [x] 3.1 Make "remains pending" explicitly cover an aborted write, not only a failed one From 13d3e9593eaeaaf4eb39b60b5d66d06878c1cc9d Mon Sep 17 00:00:00 2001 From: Soju06 Date: Fri, 14 Aug 2026 11:38:29 +0000 Subject: [PATCH 03/11] docs(openspec): state the ambiguous-abort preference MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A cancellation can reach the bump write after the database accepted its commit. The restore is unconditional there by design: a redundant bump only re-runs peers' idempotent invalidation callbacks, while dropping an unconfirmed write leaves them stale until the fallback TTL. The bus already tolerates extra increments — a request_bump arriving mid-flush produces one deliberately. Co-Authored-By: Claude Fable 5 --- app/core/cache/invalidation.py | 4 ++++ .../keep-aborted-invalidation-bumps-pending/proposal.md | 2 ++ .../specs/query-caching/spec.md | 9 ++++++++- 3 files changed, 14 insertions(+), 1 deletion(-) diff --git a/app/core/cache/invalidation.py b/app/core/cache/invalidation.py index 4748260fd3..04a09ebbcf 100644 --- a/app/core/cache/invalidation.py +++ b/app/core/cache/invalidation.py @@ -292,6 +292,10 @@ async def _flush_pending_bumps(self) -> None: # by design: without restoring the marker the namespace whose # write was in flight is neither written nor pending, so a # mutation that already committed never reaches peer replicas. + # Restored even when the abort is ambiguous (cancellation + # arriving after the database accepted the commit): a redundant + # bump only re-runs peers' idempotent callbacks, while dropping + # an unconfirmed write leaves them stale until the fallback TTL. self._pending_bumps.add(namespace) raise diff --git a/openspec/changes/keep-aborted-invalidation-bumps-pending/proposal.md b/openspec/changes/keep-aborted-invalidation-bumps-pending/proposal.md index 4e08ffec0e..1e4209284d 100644 --- a/openspec/changes/keep-aborted-invalidation-bumps-pending/proposal.md +++ b/openspec/changes/keep-aborted-invalidation-bumps-pending/proposal.md @@ -8,6 +8,8 @@ The invalidation-bus spec already requires that "coalesced (`request_bump`) name - Restore the pending marker when the bump write is cancelled or raises, so the required retry actually happens. `except BaseException` rather than `Exception`, because `CancelledError` is the case that matters. +The restore is unconditional even when the abort's outcome is ambiguous (cancellation arriving after the database accepted the commit): a redundant bump only re-runs peers' idempotent invalidation callbacks, while dropping an unconfirmed write leaves them stale until the fallback TTL. The bus already tolerates extra version increments — `request_bump` arriving mid-flush deliberately produces one. + Process shutdown is deliberately out of scope: `stop()` cancels the polling task, so a bump queued at that moment has no cycle left to drain it. That is already the documented contract — "a lost bump still converges within the fallback TTL" — and guaranteeing delivery against an unresponsive database at shutdown is a separate concern with its own bounding and task-ownership design. ## Capabilities diff --git a/openspec/changes/keep-aborted-invalidation-bumps-pending/specs/query-caching/spec.md b/openspec/changes/keep-aborted-invalidation-bumps-pending/specs/query-caching/spec.md index c6329e1540..927111ad78 100644 --- a/openspec/changes/keep-aborted-invalidation-bumps-pending/specs/query-caching/spec.md +++ b/openspec/changes/keep-aborted-invalidation-bumps-pending/specs/query-caching/spec.md @@ -1,7 +1,7 @@ ## MODIFIED Requirements ### Requirement: Cache invalidation bumps and polling are resilient and observable -`bump()` MUST retry transient write failures (including SQLite "database is locked") with a short backoff; on final failure it MUST log at ERROR with the namespace, increment `codex_lb_cache_invalidation_bump_failures_total{namespace}`, and MUST NOT fail the originating mutation. Coalesced (`request_bump`) namespaces MUST remain pending and be retried on subsequent poll cycles until a bump succeeds — including when the write is aborted rather than merely failing, since the pending marker is cleared before the write is awaited, an aborted write MUST restore it — and a `request_bump` arriving while a flush for the same namespace is already awaiting its bump MUST be preserved and produce a later bump. When any invalidation callback for a namespace fails, the poller MUST NOT acknowledge the observed version and MUST re-run that namespace's callbacks on subsequent poll cycles until they succeed. The poller MUST escalate consecutive poll failures above debug level after a bounded count (WARNING after 3, ERROR after 10) and increment `codex_lb_cache_invalidation_poll_failures_total`. +`bump()` MUST retry transient write failures (including SQLite "database is locked") with a short backoff; on final failure it MUST log at ERROR with the namespace, increment `codex_lb_cache_invalidation_bump_failures_total{namespace}`, and MUST NOT fail the originating mutation. Coalesced (`request_bump`) namespaces MUST remain pending and be retried on subsequent poll cycles until a bump succeeds — including when the write is aborted rather than merely failing, since the pending marker is cleared before the write is awaited, an aborted write MUST restore it. An abort whose outcome is ambiguous — cancellation reaching the commit after the database accepted it — MUST also restore the namespace: a redundant bump only re-runs idempotent invalidation callbacks on peers, whereas dropping an unconfirmed write would leave them stale until the fallback TTL — and a `request_bump` arriving while a flush for the same namespace is already awaiting its bump MUST be preserved and produce a later bump. When any invalidation callback for a namespace fails, the poller MUST NOT acknowledge the observed version and MUST re-run that namespace's callbacks on subsequent poll cycles until they succeed. The poller MUST escalate consecutive poll failures above debug level after a bounded count (WARNING after 3, ERROR after 10) and increment `codex_lb_cache_invalidation_poll_failures_total`. #### Scenario: Bump failure under database lock is observable and does not fail the mutation @@ -41,3 +41,10 @@ - **GIVEN** a coalesced flush has cleared a namespace's pending marker and is awaiting its bump write - **WHEN** that write is cancelled, or raises - **THEN** the namespace is restored to the pending set for a later cycle, and no version is written + +#### Scenario: An ambiguous abort prefers a redundant bump over a lost one + +- **GIVEN** cancellation reaches a bump write after the database accepted its commit but before completion is reported +- **WHEN** the namespace is restored and flushed on a later cycle +- **THEN** the extra version increment only re-runs peers' invalidation callbacks, which is safe +- **AND** the namespace is not dropped on the chance that the write already landed From d953c568de7533e69eecdb7049babf5730f0ce86 Mon Sep 17 00:00:00 2001 From: Soju06 Date: Fri, 14 Aug 2026 11:44:14 +0000 Subject: [PATCH 04/11] docs(openspec): qualify the no-version assertion for the unambiguous abort The aborted-write scenario asserted no version is written, which the ambiguous-abort scenario immediately below it contradicts. Scope it to a cancellation arriving before the database accepts the commit. Co-Authored-By: Claude Fable 5 --- .../specs/query-caching/spec.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openspec/changes/keep-aborted-invalidation-bumps-pending/specs/query-caching/spec.md b/openspec/changes/keep-aborted-invalidation-bumps-pending/specs/query-caching/spec.md index 927111ad78..8cb6a03c2a 100644 --- a/openspec/changes/keep-aborted-invalidation-bumps-pending/specs/query-caching/spec.md +++ b/openspec/changes/keep-aborted-invalidation-bumps-pending/specs/query-caching/spec.md @@ -39,7 +39,7 @@ #### Scenario: An aborted bump write keeps its namespace queued - **GIVEN** a coalesced flush has cleared a namespace's pending marker and is awaiting its bump write -- **WHEN** that write is cancelled, or raises +- **WHEN** that write is cancelled before the database accepts its commit, or raises - **THEN** the namespace is restored to the pending set for a later cycle, and no version is written #### Scenario: An ambiguous abort prefers a redundant bump over a lost one From 79c47e84bd344ab775cf7baec6919fd468edc5a9 Mon Sep 17 00:00:00 2001 From: Soju06 Date: Fri, 14 Aug 2026 11:50:18 +0000 Subject: [PATCH 05/11] docs(openspec): scope both abort branches by commit acceptance The qualifier only covered cancellation, but a driver can raise after the server accepted COMMIT too. Both scenarios now split on whether the database accepted the commit rather than on how the write aborted. Co-Authored-By: Claude Fable 5 --- app/core/cache/invalidation.py | 4 ++-- .../keep-aborted-invalidation-bumps-pending/proposal.md | 2 +- .../specs/query-caching/spec.md | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/app/core/cache/invalidation.py b/app/core/cache/invalidation.py index 04a09ebbcf..7b2dd4fd8d 100644 --- a/app/core/cache/invalidation.py +++ b/app/core/cache/invalidation.py @@ -292,8 +292,8 @@ async def _flush_pending_bumps(self) -> None: # by design: without restoring the marker the namespace whose # write was in flight is neither written nor pending, so a # mutation that already committed never reaches peer replicas. - # Restored even when the abort is ambiguous (cancellation - # arriving after the database accepted the commit): a redundant + # Restored even when the abort is ambiguous (cancellation or + # a driver error after the database accepted the commit): a redundant # bump only re-runs peers' idempotent callbacks, while dropping # an unconfirmed write leaves them stale until the fallback TTL. self._pending_bumps.add(namespace) diff --git a/openspec/changes/keep-aborted-invalidation-bumps-pending/proposal.md b/openspec/changes/keep-aborted-invalidation-bumps-pending/proposal.md index 1e4209284d..de2dd5ea0d 100644 --- a/openspec/changes/keep-aborted-invalidation-bumps-pending/proposal.md +++ b/openspec/changes/keep-aborted-invalidation-bumps-pending/proposal.md @@ -8,7 +8,7 @@ The invalidation-bus spec already requires that "coalesced (`request_bump`) name - Restore the pending marker when the bump write is cancelled or raises, so the required retry actually happens. `except BaseException` rather than `Exception`, because `CancelledError` is the case that matters. -The restore is unconditional even when the abort's outcome is ambiguous (cancellation arriving after the database accepted the commit): a redundant bump only re-runs peers' idempotent invalidation callbacks, while dropping an unconfirmed write leaves them stale until the fallback TTL. The bus already tolerates extra version increments — `request_bump` arriving mid-flush deliberately produces one. +The restore is unconditional even when the abort's outcome is ambiguous (cancellation or a driver error arriving after the database accepted the commit): a redundant bump only re-runs peers' idempotent invalidation callbacks, while dropping an unconfirmed write leaves them stale until the fallback TTL. The bus already tolerates extra version increments — `request_bump` arriving mid-flush deliberately produces one. Process shutdown is deliberately out of scope: `stop()` cancels the polling task, so a bump queued at that moment has no cycle left to drain it. That is already the documented contract — "a lost bump still converges within the fallback TTL" — and guaranteeing delivery against an unresponsive database at shutdown is a separate concern with its own bounding and task-ownership design. diff --git a/openspec/changes/keep-aborted-invalidation-bumps-pending/specs/query-caching/spec.md b/openspec/changes/keep-aborted-invalidation-bumps-pending/specs/query-caching/spec.md index 8cb6a03c2a..b52e0e4d8d 100644 --- a/openspec/changes/keep-aborted-invalidation-bumps-pending/specs/query-caching/spec.md +++ b/openspec/changes/keep-aborted-invalidation-bumps-pending/specs/query-caching/spec.md @@ -39,12 +39,12 @@ #### Scenario: An aborted bump write keeps its namespace queued - **GIVEN** a coalesced flush has cleared a namespace's pending marker and is awaiting its bump write -- **WHEN** that write is cancelled before the database accepts its commit, or raises +- **WHEN** that write aborts — cancelled or raised — before the database accepts its commit - **THEN** the namespace is restored to the pending set for a later cycle, and no version is written #### Scenario: An ambiguous abort prefers a redundant bump over a lost one -- **GIVEN** cancellation reaches a bump write after the database accepted its commit but before completion is reported +- **GIVEN** a bump write aborts — cancelled, or the driver raises — after the database accepted its commit but before completion is reported - **WHEN** the namespace is restored and flushed on a later cycle - **THEN** the extra version increment only re-runs peers' invalidation callbacks, which is safe - **AND** the namespace is not dropped on the chance that the write already landed From 16a4cd69fe3c0470777501c40e26e98dc49f35fa Mon Sep 17 00:00:00 2001 From: Soju06 Date: Fri, 14 Aug 2026 11:56:56 +0000 Subject: [PATCH 06/11] test(cache): prove the poller retries an aborted bump The unit-level tests asserted the marker is restored; the product behavior is that the background poller then writes the version. Cover it end to end: first write raises, the running poller retries, the version lands. Also moves the ambiguous-abort rationale out of the normative requirement into the proposal, per the spec/context split, and trims the code comment to what is true now that shutdown delivery is out of scope. Co-Authored-By: Claude Fable 5 --- app/core/cache/invalidation.py | 13 +++---- .../proposal.md | 4 +++ .../specs/query-caching/spec.md | 6 ++-- .../tasks.md | 3 +- .../test_cache_invalidation_bus.py | 34 +++++++++++++++++++ 5 files changed, 48 insertions(+), 12 deletions(-) diff --git a/app/core/cache/invalidation.py b/app/core/cache/invalidation.py index 7b2dd4fd8d..2a9b5c3260 100644 --- a/app/core/cache/invalidation.py +++ b/app/core/cache/invalidation.py @@ -288,14 +288,11 @@ async def _flush_pending_bumps(self) -> None: if not await self.bump(namespace): self._pending_bumps.add(namespace) except BaseException: - # Includes CancelledError, which ``stop()`` raises in this task - # by design: without restoring the marker the namespace whose - # write was in flight is neither written nor pending, so a - # mutation that already committed never reaches peer replicas. - # Restored even when the abort is ambiguous (cancellation or - # a driver error after the database accepted the commit): a redundant - # bump only re-runs peers' idempotent callbacks, while dropping - # an unconfirmed write leaves them stale until the fallback TTL. + # The marker is cleared before the write, so an aborted write + # (cancelled or raised) would otherwise leave the namespace + # neither written nor pending, breaking the required retry. + # Restored even when the abort is ambiguous — a redundant bump + # only re-runs peers' idempotent callbacks. self._pending_bumps.add(namespace) raise diff --git a/openspec/changes/keep-aborted-invalidation-bumps-pending/proposal.md b/openspec/changes/keep-aborted-invalidation-bumps-pending/proposal.md index de2dd5ea0d..3c1b02f867 100644 --- a/openspec/changes/keep-aborted-invalidation-bumps-pending/proposal.md +++ b/openspec/changes/keep-aborted-invalidation-bumps-pending/proposal.md @@ -12,6 +12,10 @@ The restore is unconditional even when the abort's outcome is ambiguous (cancell Process shutdown is deliberately out of scope: `stop()` cancels the polling task, so a bump queued at that moment has no cycle left to drain it. That is already the documented contract — "a lost bump still converges within the fallback TTL" — and guaranteeing delivery against an unresponsive database at shutdown is a separate concern with its own bounding and task-ownership design. +## Why the ambiguous case still restores + +A cancellation or driver error can arrive after the database accepted the commit, so the restore can produce a redundant bump. That is the deliberate trade: a redundant bump only re-runs peers' idempotent invalidation callbacks, while dropping an unconfirmed write leaves them stale until the fallback TTL. The bus already tolerates extra increments — a `request_bump` arriving mid-flush produces one by design. + ## Capabilities ### New Capabilities diff --git a/openspec/changes/keep-aborted-invalidation-bumps-pending/specs/query-caching/spec.md b/openspec/changes/keep-aborted-invalidation-bumps-pending/specs/query-caching/spec.md index b52e0e4d8d..4a28a72202 100644 --- a/openspec/changes/keep-aborted-invalidation-bumps-pending/specs/query-caching/spec.md +++ b/openspec/changes/keep-aborted-invalidation-bumps-pending/specs/query-caching/spec.md @@ -1,7 +1,7 @@ ## MODIFIED Requirements ### Requirement: Cache invalidation bumps and polling are resilient and observable -`bump()` MUST retry transient write failures (including SQLite "database is locked") with a short backoff; on final failure it MUST log at ERROR with the namespace, increment `codex_lb_cache_invalidation_bump_failures_total{namespace}`, and MUST NOT fail the originating mutation. Coalesced (`request_bump`) namespaces MUST remain pending and be retried on subsequent poll cycles until a bump succeeds — including when the write is aborted rather than merely failing, since the pending marker is cleared before the write is awaited, an aborted write MUST restore it. An abort whose outcome is ambiguous — cancellation reaching the commit after the database accepted it — MUST also restore the namespace: a redundant bump only re-runs idempotent invalidation callbacks on peers, whereas dropping an unconfirmed write would leave them stale until the fallback TTL — and a `request_bump` arriving while a flush for the same namespace is already awaiting its bump MUST be preserved and produce a later bump. When any invalidation callback for a namespace fails, the poller MUST NOT acknowledge the observed version and MUST re-run that namespace's callbacks on subsequent poll cycles until they succeed. The poller MUST escalate consecutive poll failures above debug level after a bounded count (WARNING after 3, ERROR after 10) and increment `codex_lb_cache_invalidation_poll_failures_total`. +`bump()` MUST retry transient write failures (including SQLite "database is locked") with a short backoff; on final failure it MUST log at ERROR with the namespace, increment `codex_lb_cache_invalidation_bump_failures_total{namespace}`, and MUST NOT fail the originating mutation. Coalesced (`request_bump`) namespaces MUST remain pending and be retried on subsequent poll cycles until a bump succeeds — including when the write aborts rather than merely failing; an aborted write MUST restore the pending marker regardless of whether the database had already accepted its commit — cancellation reaching the commit after the database accepted it — MUST also restore the namespace: a redundant bump only re-runs idempotent invalidation callbacks on peers, whereas dropping an unconfirmed write would leave them stale until the fallback TTL — and a `request_bump` arriving while a flush for the same namespace is already awaiting its bump MUST be preserved and produce a later bump. When any invalidation callback for a namespace fails, the poller MUST NOT acknowledge the observed version and MUST re-run that namespace's callbacks on subsequent poll cycles until they succeed. The poller MUST escalate consecutive poll failures above debug level after a bounded count (WARNING after 3, ERROR after 10) and increment `codex_lb_cache_invalidation_poll_failures_total`. #### Scenario: Bump failure under database lock is observable and does not fail the mutation @@ -46,5 +46,5 @@ - **GIVEN** a bump write aborts — cancelled, or the driver raises — after the database accepted its commit but before completion is reported - **WHEN** the namespace is restored and flushed on a later cycle -- **THEN** the extra version increment only re-runs peers' invalidation callbacks, which is safe -- **AND** the namespace is not dropped on the chance that the write already landed +- **THEN** the namespace is still restored and bumped on a later cycle +- **AND** the resulting redundant version increment is accepted rather than dropping the namespace diff --git a/openspec/changes/keep-aborted-invalidation-bumps-pending/tasks.md b/openspec/changes/keep-aborted-invalidation-bumps-pending/tasks.md index 4966d88913..5b7df78afd 100644 --- a/openspec/changes/keep-aborted-invalidation-bumps-pending/tasks.md +++ b/openspec/changes/keep-aborted-invalidation-bumps-pending/tasks.md @@ -6,7 +6,8 @@ - [x] 2.1 A cancelled write restores the marker, and the marker is cleared before the write (locking in the intended coalescing) - [x] 2.2 A raising write restores the marker +- [x] 2.3 End-to-end: the running poller retries the aborted namespace and writes its version ## 3. Spec -- [x] 3.1 Make "remains pending" explicitly cover an aborted write, not only a failed one +- [x] 3.1 Make "remains pending" explicitly cover an aborted write, not only a failed one, keeping the requirement normative and the rationale in the proposal diff --git a/tests/integration/test_cache_invalidation_bus.py b/tests/integration/test_cache_invalidation_bus.py index 2c2bbdf43b..799237051f 100644 --- a/tests/integration/test_cache_invalidation_bus.py +++ b/tests/integration/test_cache_invalidation_bus.py @@ -842,3 +842,37 @@ async def test_inflight_poll_does_not_clobber_concurrent_local_bump(db_setup) -> await source._poll_once() assert source_calls == [] assert source._known_versions.get(NAMESPACE_RESET_CREDITS) == 2 + + +@pytest.mark.asyncio +async def test_aborted_bump_is_retried_by_the_running_poller(db_setup, monkeypatch) -> None: + """End-to-end: the point of restoring the marker is that the background + poller actually retries. Without the restore the first raise loses the + namespace and no later cycle ever writes its version.""" + namespace = "test_abort_retried_by_poller" + attempts = 0 + real_bump = CacheInvalidationPoller.bump + + poller = CacheInvalidationPoller(SessionLocal, poll_interval_seconds=0.01) + + async def failing_then_real(ns: str) -> bool: + nonlocal attempts + attempts += 1 + if attempts == 1: + raise RuntimeError("driver exploded") + return await real_bump(poller, ns) + + monkeypatch.setattr(poller, "bump", failing_then_real) + poller.request_bump(namespace) + await poller.start() + try: + for _ in range(200): + if await _namespace_version(namespace) is not None: + break + await asyncio.sleep(0.01) + finally: + await poller.stop() + + assert attempts >= 2, "the poller must retry the aborted namespace" + assert await _namespace_version(namespace) == 1 + assert namespace not in poller._pending_bumps From 48e32f2126382e3ece06e184e99bb591a229260d Mon Sep 17 00:00:00 2001 From: Soju06 Date: Fri, 14 Aug 2026 12:03:14 +0000 Subject: [PATCH 07/11] docs(openspec): finish removing rationale from the requirement My previous edit left the original clause fragment in place, so the rationale I said I had moved was still there, duplicated. The requirement now states only the testable outcome; the reasoning lives in the proposal. Co-Authored-By: Claude Fable 5 --- .../specs/query-caching/spec.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/openspec/changes/keep-aborted-invalidation-bumps-pending/specs/query-caching/spec.md b/openspec/changes/keep-aborted-invalidation-bumps-pending/specs/query-caching/spec.md index 4a28a72202..66ea8c0036 100644 --- a/openspec/changes/keep-aborted-invalidation-bumps-pending/specs/query-caching/spec.md +++ b/openspec/changes/keep-aborted-invalidation-bumps-pending/specs/query-caching/spec.md @@ -1,7 +1,7 @@ ## MODIFIED Requirements ### Requirement: Cache invalidation bumps and polling are resilient and observable -`bump()` MUST retry transient write failures (including SQLite "database is locked") with a short backoff; on final failure it MUST log at ERROR with the namespace, increment `codex_lb_cache_invalidation_bump_failures_total{namespace}`, and MUST NOT fail the originating mutation. Coalesced (`request_bump`) namespaces MUST remain pending and be retried on subsequent poll cycles until a bump succeeds — including when the write aborts rather than merely failing; an aborted write MUST restore the pending marker regardless of whether the database had already accepted its commit — cancellation reaching the commit after the database accepted it — MUST also restore the namespace: a redundant bump only re-runs idempotent invalidation callbacks on peers, whereas dropping an unconfirmed write would leave them stale until the fallback TTL — and a `request_bump` arriving while a flush for the same namespace is already awaiting its bump MUST be preserved and produce a later bump. When any invalidation callback for a namespace fails, the poller MUST NOT acknowledge the observed version and MUST re-run that namespace's callbacks on subsequent poll cycles until they succeed. The poller MUST escalate consecutive poll failures above debug level after a bounded count (WARNING after 3, ERROR after 10) and increment `codex_lb_cache_invalidation_poll_failures_total`. +`bump()` MUST retry transient write failures (including SQLite "database is locked") with a short backoff; on final failure it MUST log at ERROR with the namespace, increment `codex_lb_cache_invalidation_bump_failures_total{namespace}`, and MUST NOT fail the originating mutation. Coalesced (`request_bump`) namespaces MUST remain pending and be retried on subsequent poll cycles until a bump succeeds, including when the write aborts rather than merely failing: an aborted write MUST restore the pending marker regardless of whether the database had already accepted its commit. A `request_bump` arriving while a flush for the same namespace is already awaiting its bump MUST be preserved and produce a later bump. When any invalidation callback for a namespace fails, the poller MUST NOT acknowledge the observed version and MUST re-run that namespace's callbacks on subsequent poll cycles until they succeed. The poller MUST escalate consecutive poll failures above debug level after a bounded count (WARNING after 3, ERROR after 10) and increment `codex_lb_cache_invalidation_poll_failures_total`. #### Scenario: Bump failure under database lock is observable and does not fail the mutation @@ -42,9 +42,9 @@ - **WHEN** that write aborts — cancelled or raised — before the database accepts its commit - **THEN** the namespace is restored to the pending set for a later cycle, and no version is written -#### Scenario: An ambiguous abort prefers a redundant bump over a lost one +#### Scenario: An abort after the commit was accepted still restores the namespace - **GIVEN** a bump write aborts — cancelled, or the driver raises — after the database accepted its commit but before completion is reported -- **WHEN** the namespace is restored and flushed on a later cycle -- **THEN** the namespace is still restored and bumped on a later cycle -- **AND** the resulting redundant version increment is accepted rather than dropping the namespace +- **WHEN** the abort is handled +- **THEN** the namespace is restored to the pending set and bumped on a later cycle +- **AND** the resulting duplicate version increment is accepted From 4095d19e78217c54e274538b745090014cd559c3 Mon Sep 17 00:00:00 2001 From: Soju06 Date: Fri, 14 Aug 2026 13:06:33 +0000 Subject: [PATCH 08/11] test(cache): drop the false shutdown-delivery rationale from the docstring MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The restore matters where a later cycle exists to retry; at process stop no cycle remains either way — shutdown delivery is explicitly out of scope and the restore there only keeps the pending set honest. Co-Authored-By: Claude Fable 5 --- tests/integration/test_cache_invalidation_bus.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tests/integration/test_cache_invalidation_bus.py b/tests/integration/test_cache_invalidation_bus.py index 799237051f..faaef42029 100644 --- a/tests/integration/test_cache_invalidation_bus.py +++ b/tests/integration/test_cache_invalidation_bus.py @@ -389,9 +389,10 @@ def test_namespace_log_labels_cover_all_namespaces() -> None: @pytest.mark.asyncio async def test_pending_bump_survives_a_cancelled_flush(db_setup, monkeypatch) -> None: """The marker is cleared before the write is awaited, so a cancelled write - must restore it. stop() cancels the polling task by design, so without the - restore a mutation that already committed is neither written nor pending - and never reaches peer replicas.""" + must restore it — otherwise the namespace is neither written nor pending + and no later cycle can retry it. (At process stop no cycle remains either + way; shutdown delivery is explicitly out of scope, and the restore there + only keeps the pending set honest.)""" namespace = "test_flush_cancelled" started = asyncio.Event() From 4249001368aec9f78d7e55186c2639d4b8fb5c0a Mon Sep 17 00:00:00 2001 From: Soju06 Date: Fri, 14 Aug 2026 13:16:59 +0000 Subject: [PATCH 09/11] fix(cache): keep flushing other namespaces after an abnormal raise MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit bump() reports normal failure by returning False, so a raise is abnormal — but re-raising aborted the sorted flush loop, and a persistently raising namespace that sorts first would starve every namespace after it on every cycle. Cancellation still restores and re-raises (teardown must abort); an abnormal raise now restores, logs at warning, and continues. Co-Authored-By: Claude Fable 5 --- app/core/cache/invalidation.py | 22 +++++++++--- .../specs/query-caching/spec.md | 9 ++++- .../tasks.md | 4 +-- .../test_cache_invalidation_bus.py | 34 ++++++++++++------- 4 files changed, 49 insertions(+), 20 deletions(-) diff --git a/app/core/cache/invalidation.py b/app/core/cache/invalidation.py index 2a9b5c3260..9e93be5c34 100644 --- a/app/core/cache/invalidation.py +++ b/app/core/cache/invalidation.py @@ -287,14 +287,26 @@ async def _flush_pending_bumps(self) -> None: try: if not await self.bump(namespace): self._pending_bumps.add(namespace) - except BaseException: + except asyncio.CancelledError: # The marker is cleared before the write, so an aborted write - # (cancelled or raised) would otherwise leave the namespace - # neither written nor pending, breaking the required retry. - # Restored even when the abort is ambiguous — a redundant bump - # only re-runs peers' idempotent callbacks. + # would otherwise leave the namespace neither written nor + # pending, breaking the required retry. Restored even when the + # abort is ambiguous — a redundant bump only re-runs peers' + # idempotent callbacks. self._pending_bumps.add(namespace) raise + except Exception: + # ``bump()`` reports normal failure by returning False, so a + # raise is abnormal — but re-raising would abort the flush and, + # since the loop is sorted, a persistently raising namespace + # would starve every namespace sorting after it on every cycle. + # Restore it and keep flushing the rest. + self._pending_bumps.add(namespace) + logger.warning( + "cache_invalidation flush bump raised for namespace %s; kept pending", + _NAMESPACE_LOG_LABELS.get(namespace, "unknown"), + exc_info=True, + ) async def _poll_once(self) -> bool: """Flush pending bumps and reconcile observed versions once. diff --git a/openspec/changes/keep-aborted-invalidation-bumps-pending/specs/query-caching/spec.md b/openspec/changes/keep-aborted-invalidation-bumps-pending/specs/query-caching/spec.md index 66ea8c0036..885ddae521 100644 --- a/openspec/changes/keep-aborted-invalidation-bumps-pending/specs/query-caching/spec.md +++ b/openspec/changes/keep-aborted-invalidation-bumps-pending/specs/query-caching/spec.md @@ -1,7 +1,7 @@ ## MODIFIED Requirements ### Requirement: Cache invalidation bumps and polling are resilient and observable -`bump()` MUST retry transient write failures (including SQLite "database is locked") with a short backoff; on final failure it MUST log at ERROR with the namespace, increment `codex_lb_cache_invalidation_bump_failures_total{namespace}`, and MUST NOT fail the originating mutation. Coalesced (`request_bump`) namespaces MUST remain pending and be retried on subsequent poll cycles until a bump succeeds, including when the write aborts rather than merely failing: an aborted write MUST restore the pending marker regardless of whether the database had already accepted its commit. A `request_bump` arriving while a flush for the same namespace is already awaiting its bump MUST be preserved and produce a later bump. When any invalidation callback for a namespace fails, the poller MUST NOT acknowledge the observed version and MUST re-run that namespace's callbacks on subsequent poll cycles until they succeed. The poller MUST escalate consecutive poll failures above debug level after a bounded count (WARNING after 3, ERROR after 10) and increment `codex_lb_cache_invalidation_poll_failures_total`. +`bump()` MUST retry transient write failures (including SQLite "database is locked") with a short backoff; on final failure it MUST log at ERROR with the namespace, increment `codex_lb_cache_invalidation_bump_failures_total{namespace}`, and MUST NOT fail the originating mutation. Coalesced (`request_bump`) namespaces MUST remain pending and be retried on subsequent poll cycles until a bump succeeds, including when the write aborts rather than merely failing: an aborted write MUST restore the pending marker regardless of whether the database had already accepted its commit. A write that raises MUST NOT prevent the remaining pending namespaces from flushing in the same cycle. A `request_bump` arriving while a flush for the same namespace is already awaiting its bump MUST be preserved and produce a later bump. When any invalidation callback for a namespace fails, the poller MUST NOT acknowledge the observed version and MUST re-run that namespace's callbacks on subsequent poll cycles until they succeed. The poller MUST escalate consecutive poll failures above debug level after a bounded count (WARNING after 3, ERROR after 10) and increment `codex_lb_cache_invalidation_poll_failures_total`. #### Scenario: Bump failure under database lock is observable and does not fail the mutation @@ -42,6 +42,13 @@ - **WHEN** that write aborts — cancelled or raised — before the database accepts its commit - **THEN** the namespace is restored to the pending set for a later cycle, and no version is written +#### Scenario: A raising namespace does not starve the others + +- **GIVEN** two pending namespaces where the first (in sort order) raises on every bump attempt +- **WHEN** a flush cycle runs +- **THEN** the raising namespace stays pending with no version written +- **AND** the other namespace is bumped in that same cycle + #### Scenario: An abort after the commit was accepted still restores the namespace - **GIVEN** a bump write aborts — cancelled, or the driver raises — after the database accepted its commit but before completion is reported diff --git a/openspec/changes/keep-aborted-invalidation-bumps-pending/tasks.md b/openspec/changes/keep-aborted-invalidation-bumps-pending/tasks.md index 5b7df78afd..285fbf9efc 100644 --- a/openspec/changes/keep-aborted-invalidation-bumps-pending/tasks.md +++ b/openspec/changes/keep-aborted-invalidation-bumps-pending/tasks.md @@ -1,11 +1,11 @@ ## 1. Fix -- [x] 1.1 Restore the pending namespace in `_flush_pending_bumps` when the bump write is cancelled or raises, then re-raise +- [x] 1.1 Restore the pending namespace in `_flush_pending_bumps` when the bump write aborts: cancellation restores and re-raises; an abnormal raise restores, logs, and continues with the remaining namespaces so a persistently raising namespace cannot starve the ones sorting after it ## 2. Tests - [x] 2.1 A cancelled write restores the marker, and the marker is cleared before the write (locking in the intended coalescing) -- [x] 2.2 A raising write restores the marker +- [x] 2.2 A raising write restores the marker and does not block later namespaces from flushing - [x] 2.3 End-to-end: the running poller retries the aborted namespace and writes its version ## 3. Spec diff --git a/tests/integration/test_cache_invalidation_bus.py b/tests/integration/test_cache_invalidation_bus.py index faaef42029..d48a9e746a 100644 --- a/tests/integration/test_cache_invalidation_bus.py +++ b/tests/integration/test_cache_invalidation_bus.py @@ -418,22 +418,32 @@ async def never_finishes(ns: str) -> bool: @pytest.mark.asyncio -async def test_pending_bump_survives_a_raising_flush(db_setup, monkeypatch) -> None: - """Same contract for a write that raises rather than returning False.""" - namespace = "test_flush_raised" - - async def raises(ns: str) -> bool: - raise RuntimeError("driver exploded") +async def test_pending_bump_survives_a_raising_flush_and_does_not_starve_others(db_setup, monkeypatch) -> None: + """A raise is abnormal (bump() reports failure by returning False), so it + must not abort the flush: the loop is sorted, and a persistently raising + namespace sorting first would otherwise starve every namespace after it + on every cycle. The raiser stays pending; the rest still land.""" + raising_namespace = "test_flush_raised_a" + healthy_namespace = "test_flush_raised_b" + real_bump = CacheInvalidationPoller.bump poller = CacheInvalidationPoller(SessionLocal) - monkeypatch.setattr(poller, "bump", raises) - poller.request_bump(namespace) - with pytest.raises(RuntimeError, match="driver exploded"): - await poller._flush_pending_bumps() + async def raising_for_one(ns: str) -> bool: + if ns == raising_namespace: + raise RuntimeError("driver exploded") + return await real_bump(poller, ns) + + monkeypatch.setattr(poller, "bump", raising_for_one) + poller.request_bump(raising_namespace) + poller.request_bump(healthy_namespace) - assert namespace in poller._pending_bumps - assert await _namespace_version(namespace) is None + await poller._flush_pending_bumps() + + assert raising_namespace in poller._pending_bumps + assert await _namespace_version(raising_namespace) is None + assert healthy_namespace not in poller._pending_bumps + assert await _namespace_version(healthy_namespace) == 1 @pytest.mark.asyncio From b1677023e4a4ad6a1e8d18c506cbb58ff27dd7ba Mon Sep 17 00:00:00 2001 From: Soju06 Date: Fri, 14 Aug 2026 13:25:02 +0000 Subject: [PATCH 10/11] docs(openspec): sync the proposal with the split abort handling Co-Authored-By: Claude Fable 5 --- .../changes/keep-aborted-invalidation-bumps-pending/proposal.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openspec/changes/keep-aborted-invalidation-bumps-pending/proposal.md b/openspec/changes/keep-aborted-invalidation-bumps-pending/proposal.md index 3c1b02f867..f4e85b9530 100644 --- a/openspec/changes/keep-aborted-invalidation-bumps-pending/proposal.md +++ b/openspec/changes/keep-aborted-invalidation-bumps-pending/proposal.md @@ -6,7 +6,7 @@ The invalidation-bus spec already requires that "coalesced (`request_bump`) name ## What Changes -- Restore the pending marker when the bump write is cancelled or raises, so the required retry actually happens. `except BaseException` rather than `Exception`, because `CancelledError` is the case that matters. +- Restore the pending marker when the bump write aborts, so the required retry actually happens. The two abort kinds are handled differently: `CancelledError` restores and re-raises (task teardown must abort the flush), while an ordinary `Exception` — abnormal, since `bump()` reports failure by returning `False` — restores, logs at warning, and continues, so a persistently raising namespace cannot starve the namespaces sorting after it. The restore is unconditional even when the abort's outcome is ambiguous (cancellation or a driver error arriving after the database accepted the commit): a redundant bump only re-runs peers' idempotent invalidation callbacks, while dropping an unconfirmed write leaves them stale until the fallback TTL. The bus already tolerates extra version increments — `request_bump` arriving mid-flush deliberately produces one. From abfea30aa978cdb7d87bee139b30ec50268397a3 Mon Sep 17 00:00:00 2001 From: Soju06 Date: Fri, 14 Aug 2026 13:31:13 +0000 Subject: [PATCH 11/11] test(cache): wait for the retry to settle before stopping the poller The committed row can become visible while bump() is still in its shielded session cleanup; stopping the poller at that instant cancels the retry mid-flight. Gate the stop on an event set only after bump() fully returns. Co-Authored-By: Claude Fable 5 --- tests/integration/test_cache_invalidation_bus.py | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/tests/integration/test_cache_invalidation_bus.py b/tests/integration/test_cache_invalidation_bus.py index d48a9e746a..199289b31c 100644 --- a/tests/integration/test_cache_invalidation_bus.py +++ b/tests/integration/test_cache_invalidation_bus.py @@ -866,21 +866,25 @@ async def test_aborted_bump_is_retried_by_the_running_poller(db_setup, monkeypat poller = CacheInvalidationPoller(SessionLocal, poll_interval_seconds=0.01) + retry_settled = asyncio.Event() + async def failing_then_real(ns: str) -> bool: nonlocal attempts attempts += 1 if attempts == 1: raise RuntimeError("driver exploded") - return await real_bump(poller, ns) + result = await real_bump(poller, ns) + # Signal only after bump() fully returns: the committed row can become + # visible while the shielded session cleanup is still running, and + # stopping the poller at that instant would cancel the retry mid-flight. + retry_settled.set() + return result monkeypatch.setattr(poller, "bump", failing_then_real) poller.request_bump(namespace) await poller.start() try: - for _ in range(200): - if await _namespace_version(namespace) is not None: - break - await asyncio.sleep(0.01) + await asyncio.wait_for(retry_settled.wait(), timeout=5.0) finally: await poller.stop()