fix!: raise on an invalid idempotency key instead of running the action unprotected

docs/agents.md

@@ -179,7 +179,7 @@ result = await coordinator.coordinate(
 
 | Method | Returns | Notes |
 |---|---|---|
-| `AsyncIdempotencyCoordinator.coordinate(operation, idempotency_key, ttl_seconds, adapter, action, /, *args, idempotency_fingerprint=None, **kwargs)` | `T` | never raises for storage or decode trouble; raises `IdempotencyInProgressError` when the key is in flight and `in_flight` says so, `IdempotencyKeyReuseError` when the record's fingerprint differs from `idempotency_fingerprint` |
+| `AsyncIdempotencyCoordinator.coordinate(operation, idempotency_key, ttl_seconds, adapter, action, /, *args, idempotency_fingerprint=None, **kwargs)` | `T` | never raises for storage or decode trouble; raises `IdempotencyValidationError` when the operation or a non-`None` key is invalid, `IdempotencyInProgressError` when the key is in flight and `in_flight` says so, `IdempotencyKeyReuseError` when the record's fingerprint differs from `idempotency_fingerprint` |
 | `IdempotencyDomainService.create_record(operation, idempotency_key, result, *, ttl_minutes=None, fingerprint=None)` | `IdempotencyRecord` | raises `IdempotencyInvalidTTLError`, `IdempotencyValidationError` |
 | `IdempotencyDomainService.create_pending_record(operation, idempotency_key, *, lease_seconds, fingerprint=None)` | `IdempotencyRecord` | the reservation; not held to the TTL bounds; raises `IdempotencyValidationError` |
 | `IdempotencyDomainService.validate_record(record)` | `None` | raises `IdempotencyRecordExpiredError`; the coordinator calls it on every record it reads |
@@ -312,8 +312,12 @@ fields existed is read as enabled, `"wait"` and 30 seconds.
    keyword arguments, and from the positional arguments when the parameter can be passed
    that way. Keyword-only is still the shape to write: nothing can land in it by position,
    and the call site has to name it.
-4. **A falsy key means no idempotency.** `None` and `""` both short-circuit straight to the
-   action, in the decorator and in `coordinate()`.
+4. **Only `None` means no idempotency.** `None` short-circuits straight to the action, in
+   the decorator and in `coordinate()`. Any other key, `""` included, is checked together
+   with the operation before anything is read or run (rule 12); an invalid one raises
+   `IdempotencyValidationError`, 400 in HTTP terms, and the action does not run. Pass `None`,
+   not `""`, when a request carries no key. `enabled=False` skips the check along with
+   everything else.
 5. **A coordinator the decorator cannot find means no idempotency.** It looks for
    `infra_param` by name in `kwargs` then as an attribute of the first positional argument,
    then for an `AsyncIdempotencyCoordinator` by type in `kwargs`, in `args`, and in the
@@ -348,8 +352,10 @@ fields existed is read as enabled, `"wait"` and 30 seconds.
     `IdempotencyInvalidTTLError`, which the coordinator catches: the operation is simply not
     cached, and the caller gets its result anyway.
 12. **Neither identifier may contain a colon**, both are stripped of surrounding whitespace,
-    and the lengths are 100 for `operation` and 255 for `idempotency_key`. The Redis
-    repository re-validates on every call, so an over-long key raises there too.
+    and the lengths are 100 for `operation` and 255 for `idempotency_key`; an empty or
+    whitespace-only one is invalid. `coordinate()` and the decorator raise
+    `IdempotencyValidationError` for either before the action runs, and the Redis repository
+    re-validates on every call.
 13. **`PydanticResultAdapter` cannot represent an absent result.** `encode` raises
     `IdempotencyValidationError` when handed `None` rather than storing a `null` it could
     never decode again; the coordinator counts that as `record_validation_error`, logs it,
@@ -532,11 +538,12 @@ All derive from `IdempotencyError`, which is exported alongside them.
 | `IdempotencyInProgressError` | `(operation, key)` | another call with the same key is still running its action. `coordinate()` and the decorator raise it at once with `in_flight="raise"`, and after a whole lease of waiting with `"wait"`. Carries `.operation` and `.key` |
 | `IdempotencyKeyReuseError` | `(operation, key, stored_fingerprint, fingerprint)` | the record under the key was made for a different request. `coordinate()` and the decorator raise it before the action runs, from a completed record and from a pending one. Carries all four |
 | `IdempotencyStorageError` | `(message, operation=None, original_error=None)` | the backend failed. Carries `.operation` and `.original_error` |
-| `IdempotencyValidationError` | `(message, errors=None)` | an identifier or a result failed validation. `.errors` holds the Pydantic error list when there is one |
+| `IdempotencyValidationError` | `(message, errors=None)` | an identifier or a result failed validation. `coordinate()` and the decorator raise it for an invalid operation or key before the action runs. `.errors` holds the Pydantic error list when there is one |
 | `IdempotencyInvalidTTLError` | `(ttl_seconds, min_ttl, max_ttl)` | the TTL is outside the domain service's range. Carries all three |
 
-Through `coordinate()` and the decorator only `IdempotencyInProgressError` and
-`IdempotencyKeyReuseError` reach the caller — both are about the request, not about storage:
+Through `coordinate()` and the decorator only `IdempotencyValidationError` for the
+identifiers, `IdempotencyInProgressError` and `IdempotencyKeyReuseError` reach the caller —
+all three are about the request, not about storage:
 the collision is resolved into a wait or the winner's result, and every other one is logged,
 counted and swallowed. The rest are the contract of the repository and the domain
 service, which is where you meet them if you drive those directly.

docs/api_reference.md

@@ -56,7 +56,7 @@ The flow: reserve the key, run the action, store the result; replay a stored res
   - `in_flight_lease_seconds` (int, default: 30): How long a reservation is held before it counts as abandoned, and the longest a waiting caller waits. Must be at least 1 and longer than the action can take.
   - *Note*: The constructor raises `TypeError` when the repository has no `replace` and `in_flight` is not `"run"`.
 - **Methods**:
-  - `coordinate(operation, idempotency_key, ttl_seconds, adapter, action, /, *args, idempotency_fingerprint=None, **kwargs)`: Runs the flow and returns the action's result type. Everything after the five positional-only arguments goes to the action except `idempotency_fingerprint`, the fingerprint of the request, which is stored with the record and compared on a hit. Never raises for storage or decode trouble; raises `IdempotencyInProgressError` when the key is in flight and `in_flight` says so, and `IdempotencyKeyReuseError` when the record's fingerprint differs from the caller's.
+  - `coordinate(operation, idempotency_key, ttl_seconds, adapter, action, /, *args, idempotency_fingerprint=None, **kwargs)`: Runs the flow and returns the action's result type. Everything after the five positional-only arguments goes to the action except `idempotency_fingerprint`, the fingerprint of the request, which is stored with the record and compared on a hit. A `None` key runs the action and nothing else. Never raises for storage or decode trouble; raises `IdempotencyValidationError` when the operation or a non-`None` key (the empty string included) is invalid, before the action runs, `IdempotencyInProgressError` when the key is in flight and `in_flight` says so, and `IdempotencyKeyReuseError` when the record's fingerprint differs from the caller's.
 
 ### async_idempotent
 The same flow as a decorator: it finds the key in the call's arguments and the coordinator in the call's arguments or on `self`, then delegates to `coordinate()`.
@@ -119,6 +119,6 @@ Redis implementation of the repository protocol.
 - **`IdempotencyInProgressError(operation, key)`**: Raised by `coordinate()` and the decorator when another call with the same key is still running its action.
 - **`IdempotencyKeyReuseError(operation, key, stored_fingerprint, fingerprint)`**: Raised by `coordinate()` and the decorator when the record under the key was made for a request with a different fingerprint.
 - **`IdempotencyStorageError(message, operation, original_error)`**: Raised on storage failure.
-- **`IdempotencyValidationError(message, errors=None)`**: Raised for invalid input (e.g. empty key, too long string).
+- **`IdempotencyValidationError(message, errors=None)`**: Raised for invalid input (e.g. empty key, too long string), by `coordinate()` and the decorator before the action runs as well.
   - `errors` (list, optional): Detailed Pydantic validation errors.
 - **`IdempotencyInvalidTTLError(ttl_seconds, min_ttl, max_ttl)`**: Raised for invalid TTL.

docs/user_guide.md

@@ -456,6 +456,12 @@ The same `idempotency_key` can be used for different operations (e.g., `user.cre
 - Both are stripped of leading/trailing whitespace.
 - Max lengths: operation (100), key (255).
 
+`coordinate()` and the decorator check both before the action runs. An invalid key — empty,
+whitespace-only, with a colon or too long — raises `IdempotencyValidationError` instead of
+running the action unprotected, so a malformed `Idempotency-Key` header becomes a 400 rather
+than a request whose retries are silently not deduplicated. Only `None` means "no key":
+pass `None`, not `""`, when the request carries none.
+
 ### Error Handling
 
 The library defines several exceptions to handle various idempotency scenarios:
@@ -465,7 +471,7 @@ The library defines several exceptions to handle various idempotency scenarios:
 - **`IdempotencyKeyReuseError`**: Raised by `coordinator.coordinate()` and the decorator when the record under the key was made for a request with a different fingerprint. Carries both fingerprints. Map it to HTTP 422.
 - **`IdempotencyRecordExpiredError`**: Raised by `service.validate_record()` if the record exists but its TTL has passed.
 - **`IdempotencyInvalidTTLError`**: Raised by `service.create_record()` if the requested TTL is outside the allowed range (configured in `IdempotencyDomainService`).
-- **`IdempotencyValidationError`**: Raised by `service.create_record()` if validation of `operation` or `idempotency_key` fails (e.g., empty string or too long).
+- **`IdempotencyValidationError`**: Raised by `coordinator.coordinate()` and the decorator before the action runs, and by `service.create_record()`, if validation of `operation` or `idempotency_key` fails (e.g., empty string or too long). Map it to HTTP 400.
 - **`IdempotencyStorageError`**: Raised for critical infrastructure failures like Redis connection errors. This allows you to distinguish between a cache miss and a storage backend failure.
 - **`IdempotencyError`**: Base exception for all library errors. Also raised for data corruption during deserialization.
 

idempotency_kit/core/decorators/aio/idempotent.py

@@ -91,7 +91,7 @@ def decorator(func: Callable[..., Awaitable[T]]) -> Callable[..., Awaitable[T]]:
         async def wrapper(*args: Any, **kwargs: Any) -> T:
             # 1. Resolve idempotency key
             idempotency_key = _resolve_key(args, kwargs)
-            if not idempotency_key:
+            if idempotency_key is None:
                 return await func(*args, **kwargs)
 
             # 2. Resolve coordinator

idempotency_kit/core/services/aio/coordinator.py

@@ -6,6 +6,8 @@
 from enum import Enum, auto
 from typing import Any, Generic, TypeVar
 
+from pydantic import ValidationError
+
 from idempotency_kit.core.constants import (
     DEFAULT_IN_FLIGHT_LEASE_SECONDS,
     DEFAULT_IN_FLIGHT_MODE,
@@ -20,7 +22,7 @@
     IdempotencyRecordExpiredError,
     IdempotencyValidationError,
 )
-from idempotency_kit.core.models.entities import IdempotencyRecord
+from idempotency_kit.core.models.entities import IdempotencyIdentifiers, IdempotencyRecord
 from idempotency_kit.core.protocols.adapter import ResultAdapter
 from idempotency_kit.core.protocols.aio.repository import AsyncIdempotencyRepository
 from idempotency_kit.core.protocols.metrics import IdempotencyMetricsProtocol, NoOpIdempotencyMetrics
@@ -122,7 +124,12 @@ async def coordinate(
         the record, and a record under the same key with a different fingerprint is a key
         reuse. ``None`` on either side means no comparison, which is the key-only behaviour.
 
-        With ``enabled=False`` the action is simply run: nothing is read, nothing is
+        A ``None`` key means no idempotency: the action is simply run. Any other key,
+        the empty string included, has to be a valid identifier, and so does the operation;
+        an invalid one raises ``IdempotencyValidationError`` before the action runs, since
+        running it unprotected would leave the client believing its retries are safe.
+
+        With ``enabled=False`` the action is simply run: nothing is validated, read or
         written, and no metric is recorded.
 
         Storage and decode trouble never raise: the coordinator degrades to running the
@@ -131,9 +138,14 @@ async def coordinate(
         ``"wait"`` and a whole lease has passed; and ``IdempotencyKeyReuseError``, when the
         record under the key was made for a different request.
         """
-        if not self._enabled or not idempotency_key:
+        if not self._enabled or idempotency_key is None:
             return await action(*args, **kwargs)
 
+        try:
+            IdempotencyIdentifiers(operation=operation, idempotency_key=idempotency_key)
+        except ValidationError as e:
+            raise IdempotencyValidationError(str(e), errors=e.errors()) from e
+
         if self._in_flight == "run":
             return await self._coordinate_unreserved(
                 operation, idempotency_key, ttl_seconds, adapter, action, idempotency_fingerprint, *args, **kwargs

tests/unit/core/test_coordinator.py

@@ -4,11 +4,15 @@
 from unittest.mock import AsyncMock, MagicMock
 
 import pytest
+from fakeredis import FakeAsyncRedis as AsyncRedisClient
 
+from idempotency_kit.core.adapters import JsonResultAdapter
+from idempotency_kit.core.constants import InFlightMode
 from idempotency_kit.core.exceptions import IdempotencyKeyCollisionError, IdempotencyValidationError
 from idempotency_kit.core.models.entities import IdempotencyRecord
 from idempotency_kit.core.services.aio.coordinator import AsyncIdempotencyCoordinator
 from idempotency_kit.core.services.domain import IdempotencyDomainService
+from idempotency_kit.infra.storage.redis.aio import RedisAsyncIdempotencyRepository
 
 
 @pytest.mark.asyncio
@@ -358,3 +362,61 @@ async def test__coordinator__disabled__runs_the_action_without_touching_storage(
     mock_repo.get.assert_not_called()
     mock_repo.save.assert_not_called()
     metrics.record_miss.assert_not_called()
+
+
+@pytest.mark.asyncio
+@pytest.mark.parametrize("in_flight", ["wait", "raise", "run"])
+@pytest.mark.parametrize(
+    "idempotency_key", ["", "order:42", "x" * 256, "   "], ids=["empty", "colon", "too_long", "whitespace"]
+)
+async def test__coordinator__invalid_key__raises_validation_error_on_every_retry_and_never_runs_the_action(
+    redis_repository: RedisAsyncIdempotencyRepository,
+    fake_redis: AsyncRedisClient,
+    idempotency_key: str,
+    in_flight: InFlightMode,
+) -> None:
+    """Issue #51: a key the store cannot hold used to run the action unprotected, so every retry repeated it."""
+    # Arrange
+    coordinator = AsyncIdempotencyCoordinator(
+        repository=redis_repository, domain_service=IdempotencyDomainService(), in_flight=in_flight
+    )
+    action = AsyncMock(return_value={"charge_id": "ch_1"})
+
+    # Act & Assert
+    for _ in range(2):
+        with pytest.raises(IdempotencyValidationError, match="idempotency_key"):
+            await coordinator.coordinate("payment.charge", idempotency_key, 3600, JsonResultAdapter(), action)
+    action.assert_not_called()
+    assert await fake_redis.keys("*") == []
+
+
+@pytest.mark.asyncio
+async def test__coordinator__invalid_operation__raises_validation_error_before_the_action(
+    redis_repository: RedisAsyncIdempotencyRepository,
+) -> None:
+    """The operation is half of the storage key and is held to the same rules."""
+    # Arrange
+    coordinator = AsyncIdempotencyCoordinator(repository=redis_repository, domain_service=IdempotencyDomainService())
+    action = AsyncMock()
+
+    # Act & Assert
+    with pytest.raises(IdempotencyValidationError, match="operation"):
+        await coordinator.coordinate("payment:charge", "order-42", 3600, JsonResultAdapter(), action)
+    action.assert_not_called()
+
+
+@pytest.mark.asyncio
+async def test__coordinator__disabled_with_an_invalid_key__runs_the_action_without_validating(
+    mock_repo: AsyncMock, mock_domain_service: MagicMock, mock_adapter: MagicMock
+) -> None:
+    """enabled=False stays a pass-through: the switch does not start refusing requests."""
+    # Arrange
+    coordinator = AsyncIdempotencyCoordinator(repository=mock_repo, domain_service=mock_domain_service, enabled=False)
+    action = AsyncMock(return_value="ran")
+
+    # Act
+    result = await coordinator.coordinate("op", "", 60, mock_adapter, action)
+
+    # Assert
+    assert result == "ran"
+    action.assert_awaited_once()

tests/unit/core/test_idempotent.py

@@ -112,6 +112,27 @@ async def my_func(idempotency_key: str | None) -> str:
     mock_coordinator.coordinate.assert_not_called()
 
 
+@pytest.mark.asyncio
+async def test__decorator__empty_key__goes_to_the_coordinator(
+    mock_coordinator: MagicMock, mock_adapter: MagicMock
+) -> None:
+    """Only ``None`` means no key; an empty one is the coordinator's to refuse."""
+    # Arrange
+    mock_coordinator.coordinate.return_value = "coordinated"
+
+    @async_idempotent(operation="test.op", adapter=mock_adapter)
+    async def my_func(idempotency_key: str | None, coord: AsyncIdempotencyCoordinator) -> str:
+        return "direct"
+
+    # Act
+    result = await my_func(idempotency_key="", coord=mock_coordinator)
+
+    # Assert
+    assert result == "coordinated"
+    mock_coordinator.coordinate.assert_awaited_once()
+    assert mock_coordinator.coordinate.call_args.args[1] == ""
+
+
 @pytest.mark.asyncio
 async def test__decorator__no_coordinator__executes_directly(mock_adapter: MagicMock) -> None:
     """Test that missing coordinator causes direct function execution."""