From 08d41734c1d4a1d9f3202d0aeda68567146c5c57 Mon Sep 17 00:00:00 2001 From: Niladri Das <125604915+bniladridas@users.noreply.github.com> Date: Sat, 14 Nov 2026 12:51:28 +0530 Subject: [PATCH 1/5] fix(claude): anchor the first-run cutoff to the server clock The first-run cutoff compared event times to the local process start, so a local clock more than 60 seconds ahead of the server could drop a live event as replayed history. Read the stream response's HTTP Date header, carry it on FeedFrame.server_time, and replace the local cutoff on the first frame. A missing or unreadable Date header keeps the local fallback. Add regression tests for a clock-ahead unrecognised event, a clock-ahead live message, an old replay that is still suppressed, and Date header parsing and fallback. Fixes #255. Refs #122. Signed-off-by: Niladri Das <125604915+bniladridas@users.noreply.github.com> --- src/pepper_cli/api_client.py | 39 +++++++++++++++--- src/pepper_cli/claude_channel.py | 5 +++ tests/test_api_client_groups.py | 35 ++++++++++++++++ tests/test_claude_channel.py | 69 +++++++++++++++++++++++++++++++- 4 files changed, 141 insertions(+), 7 deletions(-) diff --git a/src/pepper_cli/api_client.py b/src/pepper_cli/api_client.py index 5b1eba2..0cf4379 100644 --- a/src/pepper_cli/api_client.py +++ b/src/pepper_cli/api_client.py @@ -496,11 +496,16 @@ class FeedFrame: ``id`` is the frame's SSE ``id:``: for an event frame the event id, for a ``cursor`` frame the cursor, ``None`` on ``error`` and ``scope``. Either of the first two is a valid ``Last-Event-ID`` to resume after. + + ``server_time`` is the server's clock when the stream opened, read from + the response's HTTP ``Date`` header, or ``None`` when the server did not + send one. It is the same on every frame of a connection. """ kind: str id: int | None payload: dict[str, Any] + server_time: dt.datetime | None = None # MIRROR: src/pepper/gateway/channels/groups_service/agent_feed.py::scope_frame @@ -3434,8 +3439,9 @@ def _stream_feed( resp.read() _raise_for_error(resp) cut = _DeadlineCut(resp, deadline) + server_time = _feed_server_time(resp.headers.get("Date")) try: - for frame in _iter_feed_frames(resp.iter_lines()): + for frame in _iter_feed_frames(resp.iter_lines(), server_time=server_time): yield frame # Whether or not the cut could be armed, a stream never outlasts its # window by more than the frame in hand. @@ -5263,13 +5269,29 @@ def _iter_sse_events(lines: Iterator[str]) -> Iterator[StreamEvent]: yield event -def _iter_feed_frames(lines: Iterator[str]) -> Iterator[FeedFrame]: +def _feed_server_time(value: str | None) -> dt.datetime | None: + """The server's clock from an HTTP ``Date`` header, or ``None`` when absent or unreadable.""" + if not value: + return None + try: + parsed = email.utils.parsedate_to_datetime(value) + except (TypeError, ValueError): + return None + if parsed.tzinfo is None: + parsed = parsed.replace(tzinfo=dt.UTC) + return parsed.astimezone(dt.UTC) + + +def _iter_feed_frames( + lines: Iterator[str], *, server_time: dt.datetime | None = None +) -> Iterator[FeedFrame]: r"""Parse a line-iterator into :class:`FeedFrame`s. Unlike :func:`_iter_sse_events` this keeps the ``id:`` field, because the feed's id *is* the resume cursor. A frame whose ``data:`` is not a JSON object, or whose ``id:`` is not an integer, is dropped rather than fatal; - a comment line (``: ping``) becomes a ``"comment"`` frame. + a comment line (``: ping``) becomes a ``"comment"`` frame. ``server_time`` + is copied onto every frame. """ event_type: str | None = None event_id: int | None = None @@ -5284,7 +5306,9 @@ def build() -> FeedFrame | None: return None if not isinstance(payload, dict): return None - return FeedFrame(kind=event_type or "message", id=event_id, payload=payload) + return FeedFrame( + kind=event_type or "message", id=event_id, payload=payload, server_time=server_time + ) for line in lines: if not line: @@ -5297,7 +5321,12 @@ def build() -> FeedFrame | None: # ``: connected`` / ``: ping``: proof the connection is alive. The # text is kept so a caller can tell an accepted connection from one # that has stayed healthy long enough to send a heartbeat. - yield FeedFrame(kind="comment", id=None, payload={"comment": line[1:].strip()}) + yield FeedFrame( + kind="comment", + id=None, + payload={"comment": line[1:].strip()}, + server_time=server_time, + ) continue field_name, _, value = line.partition(":") value = value.removeprefix(" ") diff --git a/src/pepper_cli/claude_channel.py b/src/pepper_cli/claude_channel.py index 2e117f2..ed8b72f 100644 --- a/src/pepper_cli/claude_channel.py +++ b/src/pepper_cli/claude_channel.py @@ -428,6 +428,8 @@ def __init__( # first run, or the messages posted while the channel was down would be summarised away. self.first_run = not resumed and stored is None and cursor == 0 self.cutoff = config.started_at - BACKLOG_SLACK + # The stream's HTTP Date header, when present, replaces the local clock. + self.cutoff_from_server = False self.self_id = config.self_id self.bound: api_client.WatchScope | None = None self.scope: api_client.WatchScope | None = None @@ -826,6 +828,9 @@ def _handle_frame(self, frame: api_client.FeedFrame) -> str | None: if _watch_frame_shows_health(frame): self.delay = _WATCH_BACKOFF_START self.failures = 0 + if not self.cutoff_from_server and frame.server_time is not None: + self.cutoff = frame.server_time - BACKLOG_SLACK + self.cutoff_from_server = True if frame.kind == "error": return self._frame_problem(frame) if frame.kind == "comment": diff --git a/tests/test_api_client_groups.py b/tests/test_api_client_groups.py index fecf668..9dd2dfe 100644 --- a/tests/test_api_client_groups.py +++ b/tests/test_api_client_groups.py @@ -10,6 +10,8 @@ from __future__ import annotations +from datetime import UTC, datetime + import httpx import pytest @@ -1028,3 +1030,36 @@ def handler(request: httpx.Request) -> httpx.Response: assert seen[2].url.path == "/api/v1/groups/r1/topics/t1/events/stream" assert all("last-event-id" not in r.headers for r in seen[:3]) assert seen[3].headers["last-event-id"] == "4" + + +def test_the_stream_carries_the_servers_date_on_every_frame() -> None: + def handler(request: httpx.Request) -> httpx.Response: + return httpx.Response( + 200, + text=": connected\n\n: ping\n\n", + headers={ + "content-type": "text/event-stream", + "date": "Sat, 14 Nov 2026 06:20:00 GMT", + }, + ) + + with _agent_client(handler) as api: + frames = list(api.watch_agent_events(since=0)) + + expected = datetime(2026, 11, 14, 6, 20, tzinfo=UTC) + assert [frame.server_time for frame in frames] == [expected, expected] + + +@pytest.mark.parametrize("date_header", [None, "not a date"]) +def test_the_stream_tolerates_a_missing_or_unreadable_date(date_header) -> None: + headers = {"content-type": "text/event-stream"} + if date_header is not None: + headers["date"] = date_header + + def handler(request: httpx.Request) -> httpx.Response: + return httpx.Response(200, text=": connected\n\n", headers=headers) + + with _agent_client(handler) as api: + frames = list(api.watch_agent_events(since=0)) + + assert frames[0].server_time is None diff --git a/tests/test_claude_channel.py b/tests/test_claude_channel.py index ad7032f..bbb5465 100644 --- a/tests/test_claude_channel.py +++ b/tests/test_claude_channel.py @@ -10,6 +10,7 @@ from __future__ import annotations +import email.utils import functools import json import queue @@ -183,9 +184,15 @@ class _Fake: """Plays the Pepper server: the rooms list, topics, a script of stream answers (an SSE body, or an HTTP status), message reads, posts and reactions. Records every request.""" - def __init__(self, *streams: str | int, features: tuple[str, ...] = ("watch_scope",)) -> None: + def __init__( + self, + *streams: str | int, + features: tuple[str, ...] = ("watch_scope",), + server_time: datetime | None = None, + ) -> None: self.streams = list(streams) self.features = features + self.server_time = server_time self.requests: list[httpx.Request] = [] self.messages: dict[str, dict] = {_MESSAGE: _message_json()} self.topics = {_TOPIC: "Dinner", _OTHER_TOPIC: "Chores"} @@ -274,7 +281,10 @@ def __call__(self, request: httpx.Request) -> httpx.Response: item = self.streams.pop(0) if isinstance(item, int): return httpx.Response(item, json={"detail": "No such group."}) - return httpx.Response(200, text=item, headers={"content-type": "text/event-stream"}) + headers = {"content-type": "text/event-stream"} + if self.server_time is not None: + headers["date"] = email.utils.format_datetime(self.server_time, usegmt=True) + return httpx.Response(200, text=item, headers=headers) if path.endswith("/messages") and request.method == "GET": if self.unreachable: raise httpx.ConnectError("refused") @@ -906,6 +916,61 @@ def test_a_first_run_replay_of_old_room_events_owes_no_resync(monkeypatch): assert result.kinds() == ["backlog"] +def test_a_first_run_replay_is_still_suppressed_with_the_server_clock(monkeypatch): + server_now = _now() + old = server_now - timedelta(days=3) + fake = _Fake( + _CONNECTED + _scope() + _event(7, kind="some.new.kind", created=old) + _PING, + server_time=server_now, + ) + + result = run_channel(monkeypatch, fake, ["--room", _ROOM]) + + assert result.notifications == [] + + +def test_a_first_run_cutoff_uses_the_server_clock_when_the_local_clock_is_ahead(monkeypatch): + server_now = _now() + ahead = server_now + timedelta(hours=2) + + class _AheadDatetime(datetime): + @classmethod + def now(cls, tz=None): + return ahead if tz is None else ahead.astimezone(tz) + + monkeypatch.setattr(claude_channel, "datetime", _AheadDatetime) + live = server_now + timedelta(seconds=30) + fake = _Fake( + _CONNECTED + _scope() + _event(7, kind="some.new.kind", created=live) + _PING, + server_time=server_now, + ) + + result = run_channel(monkeypatch, fake, ["--room", _ROOM]) + + assert result.kinds() == ["resync"] + + +def test_a_first_run_live_message_is_not_backlog_when_the_local_clock_is_ahead(monkeypatch): + server_now = _now() + ahead = server_now + timedelta(hours=2) + + class _AheadDatetime(datetime): + @classmethod + def now(cls, tz=None): + return ahead if tz is None else ahead.astimezone(tz) + + monkeypatch.setattr(claude_channel, "datetime", _AheadDatetime) + live = server_now + timedelta(seconds=30) + fake = _Fake( + _CONNECTED + _scope() + _event(7, message=_MESSAGE, created=live) + _PING, + server_time=server_now, + ) + + result = run_channel(monkeypatch, fake, ["--room", _ROOM]) + + assert result.kinds() == ["message"] + + def test_an_old_unknown_event_after_a_restart_still_owes_a_resync(monkeypatch, tmp_path): cursor_file = _scoped_cursor_file(tmp_path / "cursor", 3) old = _now() - timedelta(days=3) From 6802fdce52fc79b46f440faa63ad0729a76c6160 Mon Sep 17 00:00:00 2001 From: Niladri Das <125604915+bniladridas@users.noreply.github.com> Date: Sun, 15 Nov 2026 12:28:02 +0530 Subject: [PATCH 2/5] fix(claude): guard the server date and keep its cutoff to first runs The review findings all came from treating the HTTP Date as a replacement for the process-start cutoff: the UTC conversion ran outside the unreadable-date guard, the cutoff moved on resumed sessions too, and using connection time could skip a message posted while the process was starting. The whole parse and conversion are now inside the guard with OverflowError handled, the cutoff only moves on a first run, and it shifts started_at by the measured server/local offset instead of jumping to connect time. Three regression tests cover the overflow, a slow first-run connect, and a resumed session with a message 55 seconds before start; all three fail on the previous head with the source hunks reverted, and the full suite passes at 3351 with coverage 96 against the 88 floor. Signed-off-by: Niladri Das <125604915+bniladridas@users.noreply.github.com> --- src/pepper_cli/api_client.py | 10 ++++--- src/pepper_cli/claude_channel.py | 10 +++++-- tests/test_api_client_groups.py | 5 +++- tests/test_claude_channel.py | 47 ++++++++++++++++++++++++++++++++ 4 files changed, 64 insertions(+), 8 deletions(-) diff --git a/src/pepper_cli/api_client.py b/src/pepper_cli/api_client.py index 0cf4379..f92722f 100644 --- a/src/pepper_cli/api_client.py +++ b/src/pepper_cli/api_client.py @@ -5275,11 +5275,13 @@ def _feed_server_time(value: str | None) -> dt.datetime | None: return None try: parsed = email.utils.parsedate_to_datetime(value) - except (TypeError, ValueError): + if parsed.tzinfo is None: + parsed = parsed.replace(tzinfo=dt.UTC) + return parsed.astimezone(dt.UTC) + except (TypeError, ValueError, OverflowError): + # A parseable date can still overflow on conversion, for example a west-of-UTC + # offset at the top of the datetime range; that is unreadable, not fatal. return None - if parsed.tzinfo is None: - parsed = parsed.replace(tzinfo=dt.UTC) - return parsed.astimezone(dt.UTC) def _iter_feed_frames( diff --git a/src/pepper_cli/claude_channel.py b/src/pepper_cli/claude_channel.py index ed8b72f..b7611ab 100644 --- a/src/pepper_cli/claude_channel.py +++ b/src/pepper_cli/claude_channel.py @@ -428,7 +428,7 @@ def __init__( # first run, or the messages posted while the channel was down would be summarised away. self.first_run = not resumed and stored is None and cursor == 0 self.cutoff = config.started_at - BACKLOG_SLACK - # The stream's HTTP Date header, when present, replaces the local clock. + # A first run's HTTP Date header shifts this cutoff by the server/local clock offset. self.cutoff_from_server = False self.self_id = config.self_id self.bound: api_client.WatchScope | None = None @@ -828,8 +828,12 @@ def _handle_frame(self, frame: api_client.FeedFrame) -> str | None: if _watch_frame_shows_health(frame): self.delay = _WATCH_BACKOFF_START self.failures = 0 - if not self.cutoff_from_server and frame.server_time is not None: - self.cutoff = frame.server_time - BACKLOG_SLACK + if self.first_run and not self.cutoff_from_server and frame.server_time is not None: + # The Date header times the connection, not process start, so apply the measured + # clock offset to the local start rather than moving the boundary to connect time. + # A message posted while this process was starting stays after the cutoff. + offset = frame.server_time - datetime.now(UTC) + self.cutoff = self.config.started_at + offset - BACKLOG_SLACK self.cutoff_from_server = True if frame.kind == "error": return self._frame_problem(frame) diff --git a/tests/test_api_client_groups.py b/tests/test_api_client_groups.py index 9dd2dfe..aeed05a 100644 --- a/tests/test_api_client_groups.py +++ b/tests/test_api_client_groups.py @@ -1050,7 +1050,10 @@ def handler(request: httpx.Request) -> httpx.Response: assert [frame.server_time for frame in frames] == [expected, expected] -@pytest.mark.parametrize("date_header", [None, "not a date"]) +@pytest.mark.parametrize( + "date_header", + [None, "not a date", "Fri, 31 Dec 9999 23:59:59 -0500"], +) def test_the_stream_tolerates_a_missing_or_unreadable_date(date_header) -> None: headers = {"content-type": "text/event-stream"} if date_header is not None: diff --git a/tests/test_claude_channel.py b/tests/test_claude_channel.py index bbb5465..8af3ac7 100644 --- a/tests/test_claude_channel.py +++ b/tests/test_claude_channel.py @@ -85,6 +85,16 @@ def _now() -> datetime: return datetime.now(UTC) +class _MovableClock(datetime): + """A datetime whose ``now()`` a test can move, to time a slow connect.""" + + current = datetime(2026, 11, 14, 12, 0, tzinfo=UTC) + + @classmethod + def now(cls, tz=None): + return cls.current if tz is None else cls.current.astimezone(tz) + + def _frame(event: str, data: dict, event_id: int | None = None) -> str: head = f"id: {event_id}\n" if event_id is not None else "" return f"{head}event: {event}\ndata: {json.dumps(data, separators=(',', ':'))}\n\n" @@ -971,6 +981,43 @@ def now(cls, tz=None): assert result.kinds() == ["message"] +def test_a_message_posted_while_a_first_run_was_connecting_is_still_pushed(monkeypatch): + started = datetime(2026, 11, 14, 12, 0, tzinfo=UTC) + _MovableClock.current = started + monkeypatch.setattr(claude_channel, "datetime", _MovableClock) + fake = _Fake( + _CONNECTED + + _scope() + + _event(7, message=_MESSAGE, created=started + timedelta(seconds=10)) + + _PING, + server_time=started + timedelta(seconds=90), + ) + fake.on_stream = lambda: setattr(_MovableClock, "current", started + timedelta(seconds=90)) + + result = run_channel(monkeypatch, fake, ["--room", _ROOM]) + + assert result.kinds() == ["message"] + + +def test_a_resumed_cutoff_does_not_move_to_the_connect_time(monkeypatch, tmp_path): + started = datetime(2026, 11, 14, 12, 0, tzinfo=UTC) + _MovableClock.current = started + monkeypatch.setattr(claude_channel, "datetime", _MovableClock) + cursor_file = _scoped_cursor_file(tmp_path / "cursor", 3) + fake = _Fake( + _CONNECTED + + _scope() + + _event(4, message=_MESSAGE, created=started - timedelta(seconds=55)) + + _PING, + server_time=started + timedelta(seconds=10), + ) + fake.on_stream = lambda: setattr(_MovableClock, "current", started + timedelta(seconds=10)) + + result = run_channel(monkeypatch, fake, ["--room", _ROOM, "--cursor-file", str(cursor_file)]) + + assert result.kinds() == ["message"] + + def test_an_old_unknown_event_after_a_restart_still_owes_a_resync(monkeypatch, tmp_path): cursor_file = _scoped_cursor_file(tmp_path / "cursor", 3) old = _now() - timedelta(days=3) From 23475dccc78a91f82bd66f95ac710b68e9cc023d Mon Sep 17 00:00:00 2001 From: Niladri Das <125604915+bniladridas@users.noreply.github.com> Date: Sun, 15 Nov 2026 12:45:42 +0530 Subject: [PATCH 3/5] fix(claude): ignore an implausible server-clock offset The round 2 review found the offset had no bounds: a year 9999 Date header moved the cutoff into the future and silently skipped every live message, while a header near year 1 overflowed the cutoff arithmetic. `_server_cutoff` now rejects any Date whose measured offset from the local clock is more than a day, leaving the local cutoff in place, which is the same fallback as an unreadable header. Two tests cover it: the helper returns None for year 1 and year 9999 and still applies a two hour offset, and an end-to-end first run with a year 9999 Date still pushes its live message. Both fail with the source hunk reverted. Full suite 3353 passed, 2 skipped, coverage 96 against the 88 floor. Signed-off-by: Niladri Das <125604915+bniladridas@users.noreply.github.com> --- src/pepper_cli/claude_channel.py | 22 ++++++++++++++++++---- tests/test_claude_channel.py | 25 +++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 4 deletions(-) diff --git a/src/pepper_cli/claude_channel.py b/src/pepper_cli/claude_channel.py index b7611ab..7fc3b1d 100644 --- a/src/pepper_cli/claude_channel.py +++ b/src/pepper_cli/claude_channel.py @@ -114,6 +114,9 @@ #: An event older than the process start minus this is a pre-session backlog (design §7.4). BACKLOG_SLACK = timedelta(seconds=60) +#: A Date header this far from the local clock cannot be trusted; the local cutoff stands. +MAX_CLOCK_OFFSET = timedelta(days=1) + #: How long ``serve`` waits for the worker's exit code once ``ended`` is out and stdin has closed. VERDICT_WAIT_SECONDS = 5.0 #: How long the process stays up after pushing ``ended``, so the event is read before the tools go. @@ -376,6 +379,15 @@ def _parse_time(value: object) -> datetime | None: return parsed if parsed.tzinfo is not None else parsed.replace(tzinfo=UTC) +def _server_cutoff(started_at: datetime, server_time: datetime) -> datetime | None: + """The first-run cutoff shifted by the server/local offset, or ``None`` when the Date header + is too far from the local clock to trust (a broken or hostile header, not skew).""" + offset = server_time - datetime.now(UTC) + if abs(offset) > MAX_CLOCK_OFFSET: + return None + return started_at + offset - BACKLOG_SLACK + + def _is_known_silent(kind: str) -> bool: return kind in _SILENT_KINDS or kind.startswith("member.") @@ -831,10 +843,12 @@ def _handle_frame(self, frame: api_client.FeedFrame) -> str | None: if self.first_run and not self.cutoff_from_server and frame.server_time is not None: # The Date header times the connection, not process start, so apply the measured # clock offset to the local start rather than moving the boundary to connect time. - # A message posted while this process was starting stays after the cutoff. - offset = frame.server_time - datetime.now(UTC) - self.cutoff = self.config.started_at + offset - BACKLOG_SLACK - self.cutoff_from_server = True + # A message posted while this process was starting stays after the cutoff. An offset + # too large to be real is ignored, leaving the local cutoff in place. + cutoff = _server_cutoff(self.config.started_at, frame.server_time) + if cutoff is not None: + self.cutoff = cutoff + self.cutoff_from_server = True if frame.kind == "error": return self._frame_problem(frame) if frame.kind == "comment": diff --git a/tests/test_claude_channel.py b/tests/test_claude_channel.py index 8af3ac7..cdb42af 100644 --- a/tests/test_claude_channel.py +++ b/tests/test_claude_channel.py @@ -999,6 +999,31 @@ def test_a_message_posted_while_a_first_run_was_connecting_is_still_pushed(monke assert result.kinds() == ["message"] +def test_a_date_far_from_the_local_clock_does_not_move_the_cutoff(): + started = datetime(2026, 11, 14, 12, 0, tzinfo=UTC) + + for server_time in ( + datetime(1, 1, 1, tzinfo=UTC), + datetime(9999, 12, 31, 23, 59, 59, tzinfo=UTC), + ): + assert claude_channel._server_cutoff(started, server_time) is None + + applied = claude_channel._server_cutoff(started, datetime.now(UTC) + timedelta(hours=2)) + assert applied is not None and applied > started + + +def test_a_far_future_date_does_not_swallow_a_first_run_live_message(monkeypatch): + live = _now() + fake = _Fake( + _CONNECTED + _scope() + _event(7, message=_MESSAGE, created=live) + _PING, + server_time=datetime(9999, 12, 31, 23, 59, 59, tzinfo=UTC), + ) + + result = run_channel(monkeypatch, fake, ["--room", _ROOM]) + + assert result.kinds() == ["message"] + + def test_a_resumed_cutoff_does_not_move_to_the_connect_time(monkeypatch, tmp_path): started = datetime(2026, 11, 14, 12, 0, tzinfo=UTC) _MovableClock.current = started From 42d1879b94859ca00e01bca2bb680275a7846e58 Mon Sep 17 00:00:00 2001 From: Niladri Das <125604915+bniladridas@users.noreply.github.com> Date: Sun, 15 Nov 2026 13:06:46 +0530 Subject: [PATCH 4/5] fix(claude): only let the server clock move the cutoff earlier The round 3 review found that the one day bound still accepted a Date up to 20 hours ahead, which moved the first-run cutoff into the future and hid live messages. The HTTP Date can come from an intermediary and nothing guarantees it shares the clock that stamps created_at, so a positive offset is now clipped to zero: the Date may move the cutoff earlier, which fixes a local clock ahead of the server, but never later. Two exact cutoff tests replace the direction only assertion, and an end-to-end first run with a Date 20 hours ahead of a live message fails on 23475dc and passes now. Full suite 3356 passed, 2 skipped; ruff and ty clean. Signed-off-by: Niladri Das <125604915+bniladridas@users.noreply.github.com> --- src/pepper_cli/claude_channel.py | 5 ++++- tests/test_claude_channel.py | 33 ++++++++++++++++++++++++++++++-- 2 files changed, 35 insertions(+), 3 deletions(-) diff --git a/src/pepper_cli/claude_channel.py b/src/pepper_cli/claude_channel.py index 7fc3b1d..96ac79c 100644 --- a/src/pepper_cli/claude_channel.py +++ b/src/pepper_cli/claude_channel.py @@ -385,7 +385,10 @@ def _server_cutoff(started_at: datetime, server_time: datetime) -> datetime | No offset = server_time - datetime.now(UTC) if abs(offset) > MAX_CLOCK_OFFSET: return None - return started_at + offset - BACKLOG_SLACK + # The HTTP Date can come from an intermediary, and nothing guarantees it shares the clock + # that stamps created_at. Apply it only when it moves the cutoff earlier: a later boundary + # can hide a live message, an earlier one can only show more history. + return started_at + min(offset, timedelta(0)) - BACKLOG_SLACK def _is_known_silent(kind: str) -> bool: diff --git a/tests/test_claude_channel.py b/tests/test_claude_channel.py index cdb42af..fa9baba 100644 --- a/tests/test_claude_channel.py +++ b/tests/test_claude_channel.py @@ -1008,8 +1008,25 @@ def test_a_date_far_from_the_local_clock_does_not_move_the_cutoff(): ): assert claude_channel._server_cutoff(started, server_time) is None - applied = claude_channel._server_cutoff(started, datetime.now(UTC) + timedelta(hours=2)) - assert applied is not None and applied > started + +def test_the_cutoff_keeps_the_local_boundary_when_the_date_is_ahead(monkeypatch): + started = datetime(2026, 11, 14, 12, 0, tzinfo=UTC) + _MovableClock.current = started + monkeypatch.setattr(claude_channel, "datetime", _MovableClock) + + cutoff = claude_channel._server_cutoff(started, started + timedelta(hours=2)) + + assert cutoff == started - claude_channel.BACKLOG_SLACK + + +def test_the_cutoff_moves_earlier_by_exactly_the_measured_offset(monkeypatch): + started = datetime(2026, 11, 14, 12, 0, tzinfo=UTC) + _MovableClock.current = started + monkeypatch.setattr(claude_channel, "datetime", _MovableClock) + + cutoff = claude_channel._server_cutoff(started, started - timedelta(hours=2)) + + assert cutoff == started - timedelta(hours=2) - claude_channel.BACKLOG_SLACK def test_a_far_future_date_does_not_swallow_a_first_run_live_message(monkeypatch): @@ -1024,6 +1041,18 @@ def test_a_far_future_date_does_not_swallow_a_first_run_live_message(monkeypatch assert result.kinds() == ["message"] +def test_a_date_ahead_of_the_local_clock_does_not_hide_a_first_run_live_message(monkeypatch): + live = _now() + fake = _Fake( + _CONNECTED + _scope() + _event(7, message=_MESSAGE, created=live) + _PING, + server_time=live + timedelta(hours=20), + ) + + result = run_channel(monkeypatch, fake, ["--room", _ROOM]) + + assert result.kinds() == ["message"] + + def test_a_resumed_cutoff_does_not_move_to_the_connect_time(monkeypatch, tmp_path): started = datetime(2026, 11, 14, 12, 0, tzinfo=UTC) _MovableClock.current = started From 335f036be851dec2bb64f204bcfda848d123cf14 Mon Sep 17 00:00:00 2001 From: Niladri Das <125604915+bniladridas@users.noreply.github.com> Date: Sun, 15 Nov 2026 13:24:22 +0530 Subject: [PATCH 5/5] fix(claude): stop using the HTTP Date for the first-run cutoff MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The HTTP Date can come from an intermediary and need not share the clock that stamps created_at. Allowing it to move the first-run cutoff fails in one direction or the other: a Date ahead hides live events, and a Date behind pushes retained history as live. The Date parsing, the frame field, the cutoff helper, and the clock-based tests are removed. The design note in §7.4 records the local cutoff limitation and the server-side timestamp or head-position fix. Refs #255. Signed-off-by: Niladri Das <125604915+bniladridas@users.noreply.github.com> --- planning/claude-code-channel.md | 11 ++ src/pepper_cli/api_client.py | 41 +------- src/pepper_cli/claude_channel.py | 26 ----- tests/test_api_client_groups.py | 38 ------- tests/test_claude_channel.py | 170 +------------------------------ 5 files changed, 18 insertions(+), 268 deletions(-) diff --git a/planning/claude-code-channel.md b/planning/claude-code-channel.md index 7fb3a32..7a3a90a 100644 --- a/planning/claude-code-channel.md +++ b/planning/claude-code-channel.md @@ -348,6 +348,17 @@ put that backlog into context on every Claude Code restart. Decided (§14 C): position" and gives no count, since the retained window is not a backlog this agent owes. Its wording is first-run safe ("if you keep checkpoints, run your catch-up"), and `resync` uses the same wording. +- The cutoff stays local: `started_at` minus `BACKLOG_SLACK`. The API has no server clock that + shares a basis with `created_at`. The HTTP `Date` header is generated at the HTTP layer and can + be rewritten by an intermediary, so it cannot safely move the boundary in either direction: a + Date ahead hides live events, and a Date behind pushes retained history as live. If the local + clock is more than the slack ahead of the message clock during a first-run replay (an older + server that ignores `start=head`, or an explicit `--start beginning`), a live event in that + window can be summarised as first-run backlog instead, and an unrecognised event can be + consumed as history. The first ping ends first-run suppression, so the exposure is the replay + window. A server-provided timestamp generated by the same clock as `created_at`, or the feed + head position on a beginning stream so history can be classified by event id without clocks, + would remove the exposure (#255). - The replayed window on a first run also holds old room events of kinds this channel does not push (`group.renamed`, `app.installed`, and so on). With no earlier position none of them was missed, so an unrecognised event older than the process start time minus 60 seconds owes no diff --git a/src/pepper_cli/api_client.py b/src/pepper_cli/api_client.py index f92722f..5b1eba2 100644 --- a/src/pepper_cli/api_client.py +++ b/src/pepper_cli/api_client.py @@ -496,16 +496,11 @@ class FeedFrame: ``id`` is the frame's SSE ``id:``: for an event frame the event id, for a ``cursor`` frame the cursor, ``None`` on ``error`` and ``scope``. Either of the first two is a valid ``Last-Event-ID`` to resume after. - - ``server_time`` is the server's clock when the stream opened, read from - the response's HTTP ``Date`` header, or ``None`` when the server did not - send one. It is the same on every frame of a connection. """ kind: str id: int | None payload: dict[str, Any] - server_time: dt.datetime | None = None # MIRROR: src/pepper/gateway/channels/groups_service/agent_feed.py::scope_frame @@ -3439,9 +3434,8 @@ def _stream_feed( resp.read() _raise_for_error(resp) cut = _DeadlineCut(resp, deadline) - server_time = _feed_server_time(resp.headers.get("Date")) try: - for frame in _iter_feed_frames(resp.iter_lines(), server_time=server_time): + for frame in _iter_feed_frames(resp.iter_lines()): yield frame # Whether or not the cut could be armed, a stream never outlasts its # window by more than the frame in hand. @@ -5269,31 +5263,13 @@ def _iter_sse_events(lines: Iterator[str]) -> Iterator[StreamEvent]: yield event -def _feed_server_time(value: str | None) -> dt.datetime | None: - """The server's clock from an HTTP ``Date`` header, or ``None`` when absent or unreadable.""" - if not value: - return None - try: - parsed = email.utils.parsedate_to_datetime(value) - if parsed.tzinfo is None: - parsed = parsed.replace(tzinfo=dt.UTC) - return parsed.astimezone(dt.UTC) - except (TypeError, ValueError, OverflowError): - # A parseable date can still overflow on conversion, for example a west-of-UTC - # offset at the top of the datetime range; that is unreadable, not fatal. - return None - - -def _iter_feed_frames( - lines: Iterator[str], *, server_time: dt.datetime | None = None -) -> Iterator[FeedFrame]: +def _iter_feed_frames(lines: Iterator[str]) -> Iterator[FeedFrame]: r"""Parse a line-iterator into :class:`FeedFrame`s. Unlike :func:`_iter_sse_events` this keeps the ``id:`` field, because the feed's id *is* the resume cursor. A frame whose ``data:`` is not a JSON object, or whose ``id:`` is not an integer, is dropped rather than fatal; - a comment line (``: ping``) becomes a ``"comment"`` frame. ``server_time`` - is copied onto every frame. + a comment line (``: ping``) becomes a ``"comment"`` frame. """ event_type: str | None = None event_id: int | None = None @@ -5308,9 +5284,7 @@ def build() -> FeedFrame | None: return None if not isinstance(payload, dict): return None - return FeedFrame( - kind=event_type or "message", id=event_id, payload=payload, server_time=server_time - ) + return FeedFrame(kind=event_type or "message", id=event_id, payload=payload) for line in lines: if not line: @@ -5323,12 +5297,7 @@ def build() -> FeedFrame | None: # ``: connected`` / ``: ping``: proof the connection is alive. The # text is kept so a caller can tell an accepted connection from one # that has stayed healthy long enough to send a heartbeat. - yield FeedFrame( - kind="comment", - id=None, - payload={"comment": line[1:].strip()}, - server_time=server_time, - ) + yield FeedFrame(kind="comment", id=None, payload={"comment": line[1:].strip()}) continue field_name, _, value = line.partition(":") value = value.removeprefix(" ") diff --git a/src/pepper_cli/claude_channel.py b/src/pepper_cli/claude_channel.py index 96ac79c..2e117f2 100644 --- a/src/pepper_cli/claude_channel.py +++ b/src/pepper_cli/claude_channel.py @@ -114,9 +114,6 @@ #: An event older than the process start minus this is a pre-session backlog (design §7.4). BACKLOG_SLACK = timedelta(seconds=60) -#: A Date header this far from the local clock cannot be trusted; the local cutoff stands. -MAX_CLOCK_OFFSET = timedelta(days=1) - #: How long ``serve`` waits for the worker's exit code once ``ended`` is out and stdin has closed. VERDICT_WAIT_SECONDS = 5.0 #: How long the process stays up after pushing ``ended``, so the event is read before the tools go. @@ -379,18 +376,6 @@ def _parse_time(value: object) -> datetime | None: return parsed if parsed.tzinfo is not None else parsed.replace(tzinfo=UTC) -def _server_cutoff(started_at: datetime, server_time: datetime) -> datetime | None: - """The first-run cutoff shifted by the server/local offset, or ``None`` when the Date header - is too far from the local clock to trust (a broken or hostile header, not skew).""" - offset = server_time - datetime.now(UTC) - if abs(offset) > MAX_CLOCK_OFFSET: - return None - # The HTTP Date can come from an intermediary, and nothing guarantees it shares the clock - # that stamps created_at. Apply it only when it moves the cutoff earlier: a later boundary - # can hide a live message, an earlier one can only show more history. - return started_at + min(offset, timedelta(0)) - BACKLOG_SLACK - - def _is_known_silent(kind: str) -> bool: return kind in _SILENT_KINDS or kind.startswith("member.") @@ -443,8 +428,6 @@ def __init__( # first run, or the messages posted while the channel was down would be summarised away. self.first_run = not resumed and stored is None and cursor == 0 self.cutoff = config.started_at - BACKLOG_SLACK - # A first run's HTTP Date header shifts this cutoff by the server/local clock offset. - self.cutoff_from_server = False self.self_id = config.self_id self.bound: api_client.WatchScope | None = None self.scope: api_client.WatchScope | None = None @@ -843,15 +826,6 @@ def _handle_frame(self, frame: api_client.FeedFrame) -> str | None: if _watch_frame_shows_health(frame): self.delay = _WATCH_BACKOFF_START self.failures = 0 - if self.first_run and not self.cutoff_from_server and frame.server_time is not None: - # The Date header times the connection, not process start, so apply the measured - # clock offset to the local start rather than moving the boundary to connect time. - # A message posted while this process was starting stays after the cutoff. An offset - # too large to be real is ignored, leaving the local cutoff in place. - cutoff = _server_cutoff(self.config.started_at, frame.server_time) - if cutoff is not None: - self.cutoff = cutoff - self.cutoff_from_server = True if frame.kind == "error": return self._frame_problem(frame) if frame.kind == "comment": diff --git a/tests/test_api_client_groups.py b/tests/test_api_client_groups.py index aeed05a..fecf668 100644 --- a/tests/test_api_client_groups.py +++ b/tests/test_api_client_groups.py @@ -10,8 +10,6 @@ from __future__ import annotations -from datetime import UTC, datetime - import httpx import pytest @@ -1030,39 +1028,3 @@ def handler(request: httpx.Request) -> httpx.Response: assert seen[2].url.path == "/api/v1/groups/r1/topics/t1/events/stream" assert all("last-event-id" not in r.headers for r in seen[:3]) assert seen[3].headers["last-event-id"] == "4" - - -def test_the_stream_carries_the_servers_date_on_every_frame() -> None: - def handler(request: httpx.Request) -> httpx.Response: - return httpx.Response( - 200, - text=": connected\n\n: ping\n\n", - headers={ - "content-type": "text/event-stream", - "date": "Sat, 14 Nov 2026 06:20:00 GMT", - }, - ) - - with _agent_client(handler) as api: - frames = list(api.watch_agent_events(since=0)) - - expected = datetime(2026, 11, 14, 6, 20, tzinfo=UTC) - assert [frame.server_time for frame in frames] == [expected, expected] - - -@pytest.mark.parametrize( - "date_header", - [None, "not a date", "Fri, 31 Dec 9999 23:59:59 -0500"], -) -def test_the_stream_tolerates_a_missing_or_unreadable_date(date_header) -> None: - headers = {"content-type": "text/event-stream"} - if date_header is not None: - headers["date"] = date_header - - def handler(request: httpx.Request) -> httpx.Response: - return httpx.Response(200, text=": connected\n\n", headers=headers) - - with _agent_client(handler) as api: - frames = list(api.watch_agent_events(since=0)) - - assert frames[0].server_time is None diff --git a/tests/test_claude_channel.py b/tests/test_claude_channel.py index fa9baba..ad7032f 100644 --- a/tests/test_claude_channel.py +++ b/tests/test_claude_channel.py @@ -10,7 +10,6 @@ from __future__ import annotations -import email.utils import functools import json import queue @@ -85,16 +84,6 @@ def _now() -> datetime: return datetime.now(UTC) -class _MovableClock(datetime): - """A datetime whose ``now()`` a test can move, to time a slow connect.""" - - current = datetime(2026, 11, 14, 12, 0, tzinfo=UTC) - - @classmethod - def now(cls, tz=None): - return cls.current if tz is None else cls.current.astimezone(tz) - - def _frame(event: str, data: dict, event_id: int | None = None) -> str: head = f"id: {event_id}\n" if event_id is not None else "" return f"{head}event: {event}\ndata: {json.dumps(data, separators=(',', ':'))}\n\n" @@ -194,15 +183,9 @@ class _Fake: """Plays the Pepper server: the rooms list, topics, a script of stream answers (an SSE body, or an HTTP status), message reads, posts and reactions. Records every request.""" - def __init__( - self, - *streams: str | int, - features: tuple[str, ...] = ("watch_scope",), - server_time: datetime | None = None, - ) -> None: + def __init__(self, *streams: str | int, features: tuple[str, ...] = ("watch_scope",)) -> None: self.streams = list(streams) self.features = features - self.server_time = server_time self.requests: list[httpx.Request] = [] self.messages: dict[str, dict] = {_MESSAGE: _message_json()} self.topics = {_TOPIC: "Dinner", _OTHER_TOPIC: "Chores"} @@ -291,10 +274,7 @@ def __call__(self, request: httpx.Request) -> httpx.Response: item = self.streams.pop(0) if isinstance(item, int): return httpx.Response(item, json={"detail": "No such group."}) - headers = {"content-type": "text/event-stream"} - if self.server_time is not None: - headers["date"] = email.utils.format_datetime(self.server_time, usegmt=True) - return httpx.Response(200, text=item, headers=headers) + return httpx.Response(200, text=item, headers={"content-type": "text/event-stream"}) if path.endswith("/messages") and request.method == "GET": if self.unreachable: raise httpx.ConnectError("refused") @@ -926,152 +906,6 @@ def test_a_first_run_replay_of_old_room_events_owes_no_resync(monkeypatch): assert result.kinds() == ["backlog"] -def test_a_first_run_replay_is_still_suppressed_with_the_server_clock(monkeypatch): - server_now = _now() - old = server_now - timedelta(days=3) - fake = _Fake( - _CONNECTED + _scope() + _event(7, kind="some.new.kind", created=old) + _PING, - server_time=server_now, - ) - - result = run_channel(monkeypatch, fake, ["--room", _ROOM]) - - assert result.notifications == [] - - -def test_a_first_run_cutoff_uses_the_server_clock_when_the_local_clock_is_ahead(monkeypatch): - server_now = _now() - ahead = server_now + timedelta(hours=2) - - class _AheadDatetime(datetime): - @classmethod - def now(cls, tz=None): - return ahead if tz is None else ahead.astimezone(tz) - - monkeypatch.setattr(claude_channel, "datetime", _AheadDatetime) - live = server_now + timedelta(seconds=30) - fake = _Fake( - _CONNECTED + _scope() + _event(7, kind="some.new.kind", created=live) + _PING, - server_time=server_now, - ) - - result = run_channel(monkeypatch, fake, ["--room", _ROOM]) - - assert result.kinds() == ["resync"] - - -def test_a_first_run_live_message_is_not_backlog_when_the_local_clock_is_ahead(monkeypatch): - server_now = _now() - ahead = server_now + timedelta(hours=2) - - class _AheadDatetime(datetime): - @classmethod - def now(cls, tz=None): - return ahead if tz is None else ahead.astimezone(tz) - - monkeypatch.setattr(claude_channel, "datetime", _AheadDatetime) - live = server_now + timedelta(seconds=30) - fake = _Fake( - _CONNECTED + _scope() + _event(7, message=_MESSAGE, created=live) + _PING, - server_time=server_now, - ) - - result = run_channel(monkeypatch, fake, ["--room", _ROOM]) - - assert result.kinds() == ["message"] - - -def test_a_message_posted_while_a_first_run_was_connecting_is_still_pushed(monkeypatch): - started = datetime(2026, 11, 14, 12, 0, tzinfo=UTC) - _MovableClock.current = started - monkeypatch.setattr(claude_channel, "datetime", _MovableClock) - fake = _Fake( - _CONNECTED - + _scope() - + _event(7, message=_MESSAGE, created=started + timedelta(seconds=10)) - + _PING, - server_time=started + timedelta(seconds=90), - ) - fake.on_stream = lambda: setattr(_MovableClock, "current", started + timedelta(seconds=90)) - - result = run_channel(monkeypatch, fake, ["--room", _ROOM]) - - assert result.kinds() == ["message"] - - -def test_a_date_far_from_the_local_clock_does_not_move_the_cutoff(): - started = datetime(2026, 11, 14, 12, 0, tzinfo=UTC) - - for server_time in ( - datetime(1, 1, 1, tzinfo=UTC), - datetime(9999, 12, 31, 23, 59, 59, tzinfo=UTC), - ): - assert claude_channel._server_cutoff(started, server_time) is None - - -def test_the_cutoff_keeps_the_local_boundary_when_the_date_is_ahead(monkeypatch): - started = datetime(2026, 11, 14, 12, 0, tzinfo=UTC) - _MovableClock.current = started - monkeypatch.setattr(claude_channel, "datetime", _MovableClock) - - cutoff = claude_channel._server_cutoff(started, started + timedelta(hours=2)) - - assert cutoff == started - claude_channel.BACKLOG_SLACK - - -def test_the_cutoff_moves_earlier_by_exactly_the_measured_offset(monkeypatch): - started = datetime(2026, 11, 14, 12, 0, tzinfo=UTC) - _MovableClock.current = started - monkeypatch.setattr(claude_channel, "datetime", _MovableClock) - - cutoff = claude_channel._server_cutoff(started, started - timedelta(hours=2)) - - assert cutoff == started - timedelta(hours=2) - claude_channel.BACKLOG_SLACK - - -def test_a_far_future_date_does_not_swallow_a_first_run_live_message(monkeypatch): - live = _now() - fake = _Fake( - _CONNECTED + _scope() + _event(7, message=_MESSAGE, created=live) + _PING, - server_time=datetime(9999, 12, 31, 23, 59, 59, tzinfo=UTC), - ) - - result = run_channel(monkeypatch, fake, ["--room", _ROOM]) - - assert result.kinds() == ["message"] - - -def test_a_date_ahead_of_the_local_clock_does_not_hide_a_first_run_live_message(monkeypatch): - live = _now() - fake = _Fake( - _CONNECTED + _scope() + _event(7, message=_MESSAGE, created=live) + _PING, - server_time=live + timedelta(hours=20), - ) - - result = run_channel(monkeypatch, fake, ["--room", _ROOM]) - - assert result.kinds() == ["message"] - - -def test_a_resumed_cutoff_does_not_move_to_the_connect_time(monkeypatch, tmp_path): - started = datetime(2026, 11, 14, 12, 0, tzinfo=UTC) - _MovableClock.current = started - monkeypatch.setattr(claude_channel, "datetime", _MovableClock) - cursor_file = _scoped_cursor_file(tmp_path / "cursor", 3) - fake = _Fake( - _CONNECTED - + _scope() - + _event(4, message=_MESSAGE, created=started - timedelta(seconds=55)) - + _PING, - server_time=started + timedelta(seconds=10), - ) - fake.on_stream = lambda: setattr(_MovableClock, "current", started + timedelta(seconds=10)) - - result = run_channel(monkeypatch, fake, ["--room", _ROOM, "--cursor-file", str(cursor_file)]) - - assert result.kinds() == ["message"] - - def test_an_old_unknown_event_after_a_restart_still_owes_a_resync(monkeypatch, tmp_path): cursor_file = _scoped_cursor_file(tmp_path / "cursor", 3) old = _now() - timedelta(days=3)