Skip to content

test(redis): make the fake server's AUTH refusal deterministic (LAB-8767) - #569

Merged
27Bslash6 merged 1 commit into
mainfrom
lab-8767-deterministic-wrongpass-refusal
Oct 10, 2026
Merged

27Bslash6 merged 1 commit into
mainfrom
lab-8767-deterministic-wrongpass-refusal

Conversation

@27Bslash6

Copy link
Copy Markdown
Contributor

_FakeRedis.refuse_from_now() now makes the backend's next command re-authenticate on every run. The backend's redis-py pool drops its connections before the command runs, so the command opens a new connection and its AUTH meets WRONGPASS, whatever the kernel has delivered by then.

The flake

refuse_from_now() shut down the server's side of each open connection and relied on the client noticing. redis-py's pool reconnects a pooled connection only if can_read() already sees it closed when the pool hands it out, and a pool connection gets no retry. The client sees the server's close only once the kernel delivers it. So a command sent before then went out on the old connection and failed with a ConnectionError (a reset, or EOF) instead of reaching AUTH.

A provider backend's init ping leaves exactly such a connection in its pool, so only provider-backend rows were exposed:

  • Every provider-backend row of test_an_interrupt_while_classifying_a_failure_clears_the_failures_frames_too, and every wrongpass row of test_provider_backend_failure_reaches_no_frame_holding_the_password, failed its AuthenticationError precondition.
  • The wrongpass row of test_a_cancel_during_a_failing_lock_attempt_reaches_no_frame_holding_the_password failed with a transient ConnectionError instead of the cancel.
  • The provider-backend-wrongpass row of test_a_failed_write_is_freed_without_the_cyclic_gc passed, but as a dropped case: the server never sent WRONGPASS.

RedisBackend rows and the provider's init ping open their first connection inside the operation, so they never raced.

The fix

All of it is in tests/unit/backends/test_redis_error_frames.py. Nothing under src/ changes.

  • refuse_from_now(backend) sets the server refusing, then calls connection_pool.disconnect() on the backend's redis-py client, when the backend holds one. disconnect() clears each connection's socket before closing it, so the pool's next connect() opens a new socket and sends AUTH.
  • It then waits for the server to see every connection it accepted close, and fails the test if one stays open. An open connection is still authenticated, and a wrongpass case would quietly run as dropped. This replaces the server-side shutdown and its OSError rule, which guarded the same thing.
  • Each caller passes what it built. No assertion changes: the AuthenticationError precondition and the _assert_cleared(...) call after it are as they were, and _FAILURES still maps wrongpass to redis.AuthenticationError.

Verification

  • Forced interleaving. A throwaway pytest plugin made the pool's staleness check (the can_read() call in get_connection) see nothing, as it does when the server's close has not reached the client yet. On main that fails exactly the rows listed above, and the gc row passes with no WRONGPASS sent. On this branch the whole file passes, and every wrongpass and interrupt-classifying row gets its WRONGPASS.
  • Under load. Repeated serial runs of uv run pytest tests/unit/backends/test_redis_error_frames.py -q -k "interrupt or signal", and of the whole file, pass under sustained CPU and loopback-traffic load.
  • The guard bites. Passing None instead of the backend in the gc test fails its provider-backend row with "a connection authenticated before the refusal is still open".
  • uv run pytest tests/unit -m "not slow" passes, and ruff and basedpyright are clean.

refuse_from_now() shut down the server's side of each open connection, but
redis-py's pool reconnects a pooled connection only if it already sees it
closed, and the client sees the server's close only once the kernel delivers
it. A command sent before then failed on the old connection with a reset or
EOF instead of reaching AUTH, so a provider-backend wrongpass row failed its
AuthenticationError precondition, or quietly ran as a dropped case.

The backend's redis-py pool now drops its connections before the command
runs, so the command always opens a new connection and its AUTH is refused.
refuse_from_now() then waits for the server to see every connection it
accepted close, and fails if one stays open.
@coderabbitai

coderabbitai Bot commented Oct 10, 2026

Copy link
Copy Markdown

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 52 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration
  • Configuration used: Repository: cachekit-io/cachekit-py/.coderabbit.yaml
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: 20885efd-20b7-460f-9f4d-d8bf5cedafb9

📥 Commits

Reviewing files that changed from the base of the PR and between d2dc83a and 77ba780.


📒 Files selected for processing (1)
  • tests/unit/backends/test_redis_error_frames.py


  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@kodus-27b

kodus-27b Bot commented Oct 10, 2026 •

Copy link
Copy Markdown

Code Review Completed! 🔥

The code review was successfully completed based on your current configurations.

Kody Guide: Usage and Configuration
Interacting with Kody
  • Request a Review: Ask Kody to review your PR manually by adding a comment with the `@kody start-review` command at the root of your PR.

  • Provide Feedback: Help Kody learn and improve by reacting to its comments with a 👍 for helpful suggestions or a 👎 if improvements are needed.

Providing Context (Files & MCPs)

Add these hints in your PR description (or a comment) to unlock deeper checks:

  • Ticket / Acceptance Criteria: `Refs: ABC-123` (Linear/Jira/Asana/ClickUp/Trello) or a direct ticket link.
  • Bugfix Validation: a Sentry/Datadog/Bugsnag event link (or paste the stack trace/error message).
  • Endpoint Risk: mention the route (e.g., `POST /api/payments`) or controller/action name.
  • Attach a repo file as context: use an explicit marker like `@file:docs/guide.mdx#L10-L50` (replace with your real path).
  • API Contract Docs: include `@file:openapi.yaml` or `@file:swagger.json` when changing routes/schemas.
  • Definition of Done / Standards: include `@file:DOD.md` or `@file:CONTRIBUTING.md` if your repo has them.
  • Design System Source of Truth: include `@file:ui/index.ts` (replace with your DS entrypoint path).
  • Feature Flags: include the flag key/name and `@file:flags.ts` / `@file:config.json` (and optionally the PostHog flag name).
  • Edge/CDN Rules: link the Cloudflare rule/zone or describe the intended redirect/header behavior.
  • Attach an MCP tool output: use `@mcp<provider|tool>` (replace with an installed MCP provider + tool, e.g., `@mcp<sentry|events.search>`).
Current Kody Configuration
Review Options

The following review options are enabled or disabled:

Options Enabled
Bug ✅
Performance ✅
Security ✅
Business Logic ✅

Access your configuration settings here.

@27Bslash6
27Bslash6 merged commit cf73c28 into main Oct 10, 2026
37 checks passed
@27Bslash6
27Bslash6 deleted the lab-8767-deterministic-wrongpass-refusal branch October 10, 2026 02:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant