Add circuit breaker support to RedisPool

[premtsd-code]

Summary

• allow RedisPool to accept an optional circuit breaker
• fail open with fallback values when breaker-wrapped Redis pool operations fail
• add coverage for unavailable pool behavior

Tests

• php -l src/Abuse/Adapters/TimeLimit/RedisPool.php
• php -l tests/Abuse/RedisPoolCircuitBreakerTest.php
• vendor\bin\phpunit.bat --filter RedisPoolCircuitBreakerTest
• composer lint
• composer format
• composer check
• composer validate --strict --no-check-publish
• git diff --check

[greptile-apps]

Greptile Summary

This PR adds an optional CircuitBreaker parameter to RedisPool, wrapping all four pool->use() call sites in a guard() helper that returns safe fallback values (0, null, or []) when the circuit is open, implementing a fail-open strategy so Redis unavailability doesn't block abuse checks.

• guard() delegates to pool->use() directly when no breaker is provided, preserving existing behaviour, and uses breaker->call(open: fallback, close: pool->use()) otherwise.
• A new test RedisPoolCircuitBreakerTest exercises the fail-open path with a pool whose factory always throws, but it constructs new CircuitBreaker() with the default threshold: 3; a single check() call generates only one circuit-breaker failure, so the circuit stays CLOSED, the exception propagates, and the test assertion is never reached.

Confidence Score: 3/5

The production implementation is reasonable, but the only test covering the new circuit-breaker path does not actually exercise it and will throw rather than pass.

The guard() helper and fallback wiring in RedisPool.php are logically sound, but the test that was added to validate this feature is broken: it relies on the circuit opening before the assertion runs, which cannot happen with the default failure threshold of 3 and a single check() call. Merging leaves the new code path effectively untested.

tests/Abuse/RedisPoolCircuitBreakerTest.php needs the CircuitBreaker constructed with threshold: 1 (or the breaker pre-tripped) before the assertion will behave as intended.

Important Files Changed

Filename	Overview
src/Abuse/Adapters/TimeLimit/RedisPool.php	Adds optional CircuitBreaker parameter and a private guard() helper that wraps all four pool->use() call sites with fallback values; the count-increment in hit() still runs unconditionally when the circuit is open, which can inflate the per-request cached count without a Redis write.
tests/Abuse/RedisPoolCircuitBreakerTest.php	New test for circuit-breaker fail-open behavior, but uses new CircuitBreaker() with default threshold 3; a single check() call generates only one circuit-breaker failure which is below the threshold, so the pool exception propagates before the assertion is ever reached.
composer.json	Adds utopia-php/circuit-breaker 0.3.* as a production dependency alongside the existing utopia-php/pools dependency.
composer.lock	Lock file updated to include the resolved utopia-php/circuit-breaker 0.3.x package and its dependencies.

Prompt To Fix All With AI

Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 2
tests/Abuse/RedisPoolCircuitBreakerTest.php:29-33
**Test uses a breaker that won't open in time**

`new CircuitBreaker()` defaults to `threshold: 3`, meaning the circuit stays CLOSED for the first two failures and re-throws the exception on each. `$abuse->check()` triggers exactly one `guard()` call (via `count()`), so the pool throws once, the circuit records one failure (1 < 3), and the exception propagates through `count()` → `check()` → the test instead of returning `false`. The assertion is never reached.

The test should construct the breaker with `threshold: 1` so the circuit opens on the very first failed pool call and the fallback (`0`) is returned immediately.

### Issue 2 of 2
src/Abuse/Adapters/TimeLimit/RedisPool.php:84-101
**Count incremented unconditionally when circuit-open write is skipped**

When the circuit is open, `guard()` returns `null` without writing to Redis, but `$this->count = ($this->count ?? 0) + 1` still runs on line 101. Within the same request this inflates the cached count to 1 (matching the limit), which would make a second `check()` call on the same adapter instance report abuse (`count >= limit`) even though nothing was recorded in Redis. This is a latent inconsistency: if callers reuse the same adapter for more than one check per request cycle, the second call will always report abuse when the circuit is open, regardless of actual Redis state.

_{Reviews (1): Last reviewed commit: "Add circuit breaker support to RedisPool" | Re-trigger Greptile}

[greptile-apps]

Test uses a breaker that won't open in time

new CircuitBreaker() defaults to threshold: 3, meaning the circuit stays CLOSED for the first two failures and re-throws the exception on each. $abuse->check() triggers exactly one guard() call (via count()), so the pool throws once, the circuit records one failure (1 < 3), and the exception propagates through count() → check() → the test instead of returning false. The assertion is never reached.

The test should construct the breaker with threshold: 1 so the circuit opens on the very first failed pool call and the fallback (0) is returned immediately.

Prompt To Fix With AI

This is a comment left during a code review.
Path: tests/Abuse/RedisPoolCircuitBreakerTest.php
Line: 29-33

Comment:
**Test uses a breaker that won't open in time**

`new CircuitBreaker()` defaults to `threshold: 3`, meaning the circuit stays CLOSED for the first two failures and re-throws the exception on each. `$abuse->check()` triggers exactly one `guard()` call (via `count()`), so the pool throws once, the circuit records one failure (1 < 3), and the exception propagates through `count()` → `check()` → the test instead of returning `false`. The assertion is never reached.

The test should construct the breaker with `threshold: 1` so the circuit opens on the very first failed pool call and the fallback (`0`) is returned immediately.

How can I resolve this? If you propose a fix, please make it concise.

[greptile-apps]

Count incremented unconditionally when circuit-open write is skipped

When the circuit is open, guard() returns null without writing to Redis, but $this->count = ($this->count ?? 0) + 1 still runs on line 101. Within the same request this inflates the cached count to 1 (matching the limit), which would make a second check() call on the same adapter instance report abuse (count >= limit) even though nothing was recorded in Redis. This is a latent inconsistency: if callers reuse the same adapter for more than one check per request cycle, the second call will always report abuse when the circuit is open, regardless of actual Redis state.

Prompt To Fix With AI

This is a comment left during a code review.
Path: src/Abuse/Adapters/TimeLimit/RedisPool.php
Line: 84-101

Comment:
**Count incremented unconditionally when circuit-open write is skipped**

When the circuit is open, `guard()` returns `null` without writing to Redis, but `$this->count = ($this->count ?? 0) + 1` still runs on line 101. Within the same request this inflates the cached count to 1 (matching the limit), which would make a second `check()` call on the same adapter instance report abuse (`count >= limit`) even though nothing was recorded in Redis. This is a latent inconsistency: if callers reuse the same adapter for more than one check per request cycle, the second call will always report abuse when the circuit is open, regardless of actual Redis state.

How can I resolve this? If you propose a fix, please make it concise.