Skip to content

fix(claude-channel): use opening cursor for first-run boundary - #268

Draft
bniladridas wants to merge 1 commit into
Vycari:mainfrom
bniladridas:issue-255-head-boundary
Draft

bniladridas wants to merge 1 commit into
Vycari:mainfrom
bniladridas:issue-255-head-boundary

Conversation

@bniladridas

Copy link
Copy Markdown
Contributor

Human overview

On a start=head connection, the opening cursor now defines the first-run boundary: events above it are live and skip the local created_at cutoff. Replay from a saved position still uses the cutoff, and the boundary is reset on every reconnect so old missed messages are not pushed as live. Refs #255. Server-side atomicity of the head remains tracked by #267, and this stays draft until maintainers confirm it.

Human required (optional)

AI reviewer

Details for agents: what the diff does, invariants, verification, checklist

What the diff does. In src/pepper_cli/claude_channel.py, ChannelWorker stores the opening cursor of a start=head connection in live_from. _handle_frame records it only while head_pending is true, before the first event. _handle_event now receives the SSE frame id and treats an event with event_id > live_from as live, bypassing the created_at < cutoff test for both message classification and the unknown-kind resync path. _follow resets live_from at the start of every connection.

In tests/test_claude_channel.py, four tests cover an hour-behind message after head 30, a message after head 0, genuine replay under --start beginning, and a reconnect where the second connection has no opening cursor and replays an old event as skipped history.

Invariants touched. The boundary is the opening head of this connection, not the saved resume position. A reconnect may replay messages missed while offline; reusing the first connection's head would mark those as live, so the reset is required. --start beginning and servers that ignore start=head never set live_from, so their existing cutoff behavior is unchanged. A cursor frame is still not a boundary in any case other than the first frame of a pending head start; a cursor that arrives before an event on a resume remains a skip marker.

Alternatives rejected. The HTTP Date correction was rolled back in #261 because the header can come from an intermediary and need not share the clock that stamps created_at. Using the saved resume cursor as the boundary was rejected because events after it can be old messages missed while the channel was down. Waiting for #267 before any client change was rejected because the existing protocol already exposes the opening cursor and the remaining exposure is documented.

Failure posture. If the opening cursor is not the true head, or the server can drop events between capturing it and subscribing, events at or below the real head could be classified as live. That does not corrupt state by itself, but it can push old history or suppress a genuine live message under clock skew. The per-connection reset limits the blast radius to one connection. #267 tracks the server guarantee.

Migration notes. No schema, ordering, or backfill change. Rollback is reverting this commit; clients then return to the local cutoff for all first-run events, which is the behavior documented in #261.

Where to look hardest. The head_pending guard in _handle_frame: only the first cursor of a connection may set live_from. Moving that assignment outside the guard would turn a resume skip marker into a live boundary. The zero-head case is covered because the comparison is event_id > live_from, not truthiness. The unknown-kind path bypasses the cutoff as well, so a post-head unknown event triggers resync even when its timestamp is behind.

Verification. uv run pytest tests/test_claude_channel.py -q: 136 passed. The four focused boundary tests: 4 passed. uv run pytest -n auto --cov --cov-report=json:coverage-summary.json: 3364 passed, 2 skipped, total coverage 95.70 against the 88 floor. uv run ruff format --check, uv run ruff check, and uv run ty check src/pepper_cli/: clean. git diff --check: clean. No manual CLI or UI run: the change is event classification inside the worker, and the tests drive that worker with a fake SSE transport. The 2 skips belong to the existing suite; this change adds none and I did not investigate them.

Deferred work. Server-side atomicity, ordering, and the cross-endpoint head sequence are in #267. Older servers that ignore start=head, and --start beginning, still rely on the local cutoff; the documented exposure remains in #255. The rollback and design note remain in #261.

Checklist (repo-specific)

  • uv run ruff format --check and uv run ruff check clean (no new lint ignores). Ran both; passed.
  • uv run ty check src/pepper_cli/ clean. Ran; passed.
  • uv run pytest passes; coverage floor in .claude/maintainerd.json not lowered. 3364 passed, 2 skipped; coverage 95.70 against 88.
  • Server-contract changes: /api/v1 usage stays additive-only, # MIRROR: comments updated, unknown fields and frames still tolerated. No server-contract or mirrored-model change; the existing start=head cursor is used as is.
  • No token or credential is logged, printed, or put in a fixture. The diff touches no credential path.
  • README and the bundled SKILL.md updated if commands or output changed. No command, flag, or output shape changed.
  • Help: help/cli.md updated for a new or changed command, or not user-facing because ___. Not user-facing: internal first-run event classification only.

On a start=head connection, the opening cursor already names the feed head, so events above it are now classified as live and skip the local created_at cutoff, while --start beginning, a server that sends no opening cursor, and replay from a saved position keep the existing cutoff. The boundary is reset for every connection so a reconnect does not treat old missed messages as live. Four tests cover a skewed timestamp after the head, a zero head, genuine replay, and the reconnect case. Full suite 3364 passed, 2 skipped; coverage 95.70 against the 88 floor; ruff format, ruff check and ty clean. Refs Vycari#255. Refs Vycari#267.

Signed-off-by: Niladri Das <125604915+bniladridas@users.noreply.github.com>

This branch has not been deployed

No deployments
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