fix(client): reject at_timestamp=0 instead of silently unpinning the read

at_timestamp is a plain proto3 scalar with no field presence, so zero is
not serialised and the server reads the field as absent. A caller asking
to read as of the epoch therefore got a current read: the request was
accepted, the pin was dropped on the wire, and live data came back for a
time-travel query. The same package already asserts that half of it, in
test_absent_by_default, which pins an unpinned request to timestamp zero.

Validation now requires a positive integer and says why, and the public
docstring records that zero is how the wire says "no pin" and so cannot
also ask for one.

Carries a regression test for the zero case; it fails against the previous
validation, which accepted anything non-negative.

Author: polaz

coordinode/coordinode/client.py

@@ -296,7 +296,8 @@ async def cypher(
           The timestamp has to fall inside the MVCC retention window; older snapshots are
           collected and the server answers UNAVAILABLE. It cannot be combined with a
           non-zero ``after_index``: waiting for a new write and reading a fixed past are
-          opposite requests, and the pair is rejected.
+          opposite requests, and the pair is rejected. Zero is rejected too: it is how the
+          wire says "no pin", so it cannot also ask for one.
         """
         from coordinode._proto.coordinode.v1.query.cypher_pb2 import (  # type: ignore[import]
             ExecuteCypherRequest,
@@ -1116,8 +1117,14 @@ def _make_read_concern(level: str | None, after_index: int | None, at_timestamp:
             raise ValueError(f"after_index must be a non-negative integer, got {after_index!r}")
         kwargs["after_index"] = after_index
     if at_timestamp is not None:
-        if not isinstance(at_timestamp, int) or isinstance(at_timestamp, bool) or at_timestamp < 0:
-            raise ValueError(f"at_timestamp must be a non-negative integer, got {at_timestamp!r}")
+        # Positive, not merely non-negative: the field is a plain proto3 scalar
+        # with no presence, so zero is not put on the wire and the server reads
+        # it as "no pin". A request to read as of the epoch would come back as
+        # a current read, which is the one answer time travel must not give.
+        if not isinstance(at_timestamp, int) or isinstance(at_timestamp, bool) or at_timestamp < 1:
+            raise ValueError(
+                f"at_timestamp must be a positive integer (microseconds since the epoch), got {at_timestamp!r}"
+            )
         # A fence waits for the log to reach an index; a pin reads a fixed
         # point in the past. The server calls the pair mutually exclusive and
         # answers INVALID_ARGUMENT, so say so here rather than a round trip

tests/unit/test_consistency_helpers.py

@@ -76,9 +76,22 @@ def test_allows_a_zero_fence_with_a_pin(self) -> None:
 
     @pytest.mark.parametrize("bad", [-1, True, "42", 1.5])
     def test_rejects_non_negative_integers(self, bad: object) -> None:
-        with pytest.raises(ValueError, match="at_timestamp must be a non-negative integer"):
+        with pytest.raises(ValueError, match="at_timestamp must be a positive integer"):
             _make_read_concern(None, None, bad)  # type: ignore[arg-type]
 
+    def test_rejects_zero_because_the_wire_cannot_carry_it(self) -> None:
+        """Zero is how the wire says "no pin", so it cannot also mean a pin.
+
+        `at_timestamp` is a plain proto3 scalar with no field presence, so
+        zero is not serialised and the server reads the field as absent. A
+        caller asking to read as of the epoch would silently get a current
+        read instead, which is the one answer a time-travel query must never
+        return. `test_absent_by_default` pins the other half of this: an
+        unpinned request is exactly the one whose timestamp is zero.
+        """
+        with pytest.raises(ValueError, match="at_timestamp must be a positive integer"):
+            _make_read_concern(None, None, 0)
+
     def test_defaults_the_level_to_snapshot(self) -> None:
         """A pinned read is a snapshot read, and the server enforces that.