Fix #1925: wait-loop defaults to stream tip so it blocks for NEW events

[jwbron]

Summary

Fixes #1925. egg-orch message wait-loop was returning already-seen messages immediately because the /messages/wait endpoint's cursor-less path scanned from stream ID 0-0, so XREAD/XRANGE returned the first matching event in the stream instead of blocking for a new one.

• /messages/wait now passes from_tip=True when no since_id is supplied. The store snaps the starting cursor to the stream tip at call entry so only events added after the call can unblock the wait.
  • Redis backend: start_id = "$" (XREAD's native "tip at call time" sentinel).
  • In-memory backend: snapshot len(messages) under the lock on entry.
• Explicit since_id disables from_tip so callers that need zero-drop cursor-passing (race-safety across a send→wait boundary) can opt in.
• Agent prompts + docs updated: the three STAY ALIVE preambles in routes/pipelines.py and docs/reference/agent-wait-patterns.md now call out the new-events-only default and describe the --since <id> escape hatch.

Root cause

Traced in orchestrator/redis_message_store.py:195 (no since_id → start_id = \"0-0\") and orchestrator/message_store.py:248-257 (fast path filters with since_id=None → matches full history). Each wait-loop invocation re-scanned history and matched the first still-matching event, so repeated calls kept returning the same stale CONSENSUS_CONFIRMED. The existing test at test_messages.py:1054 encoded this broken behaviour as "expected"; it's been rewritten to exercise the correct event-driven semantics.

Test plan

• orchestrator/tests/test_messages.py::TestWaitEndpoint — existing tests rewritten to inject the match via a background thread (new semantics). Added test_wait_ignores_pre_existing_messages (egg-orch message wait-loop returns already-seen messages immediately instead of blocking for new events #1925 regression) and test_wait_honors_explicit_since_id (cursor-passing opt-in).
• orchestrator/tests/test_message_store.py::TestFromTipSemantics — pre-existing ignored, post-call unblocks, wait=0 degrades safely, explicit since_id wins.
• orchestrator/tests/test_redis_message_store.py::TestRedisFromTipSemantics — same, with fakeredis; verifies $ vs 0-0 start_id selection.
• test_concurrent_integration.py producer/reviewer stay-alive assertions still pass (canonical --for lists unchanged).
• Observed: a full pipeline run where the documenter hits stay-alive — confirm the first wait-loop blocks on the bus instead of returning instantly.

Related

• Issue egg-orch message wait-loop returns already-seen messages immediately instead of blocking for new events #1925
• PR Issue #1897: event-driven BRC wait primitives + agent heartbeats #1919 (landed the primitive; this is the semantic fix-up)
• Issue Agent wait heuristics: replace sleep/poll loops with event-driven BRC message stream consumption #1897 (original design)

[egg-reviewer]

Solid fix for #1925. The root cause analysis is correct — cursor-less waits scanning from 0-0 / index 0 caused repeated calls to re-match stale events instead of blocking for new ones. The fix is well-designed: from_tip snaps the starting cursor to the stream tip at call entry in both backends, and the preconditions (from_tip AND NOT since_id AND wait > 0) correctly protect the edge cases.

What I verified

• In-memory backend: tip_index captured under self._lock in the fast-path block, then used stably in the blocking loop's _filter closure. Race-free because add_message must acquire the same lock, and cv.wait() releases/reacquires it correctly.
• Redis backend: start_id = "$" only enters the XREAD path (which accepts $), never the XRANGE path (which doesn't). Cursor advancement after the first XREAD correctly replaces $ with a concrete last_sid, so subsequent iterations use real stream IDs.
• wait=0 safety: use_tip is False when wait=0 in both backends. This avoids the footgun where $ + XRANGE would fail and where from_tip + non-blocking would always return empty.
• since_id override: since_id being set disables from_tip in both backends, preserving the cursor-passing escape hatch for zero-drop semantics.
• Endpoint wiring: from_tip=since_id is None at routes/messages.py:422 correctly derives the flag from the query parameter presence.
• Prompt updates: All three stay-alive preambles (two in _build_brc_preamble, one in _build_agent_prompt) are updated consistently.
• Test coverage: Strong — both backends have dedicated TestFromTipSemantics / TestRedisFromTipSemantics classes; endpoint tests rewritten to inject messages via background threads (correct for the new event-driven contract); dedicated regression test (test_wait_ignores_pre_existing_messages) and cursor-opt-in test (test_wait_honors_explicit_since_id).

Non-blocking suggestions

1. Redis from_tip docstring omits wait > 0 precondition (redis_message_store.py:192-193)

The in-memory backend's docstring correctly documents all three conditions:

from_tip: If True AND ``since_id`` is not set AND ``wait > 0``, snap ...

But the Redis docstring says only:

from_tip: If True AND ``since_id`` is not set, start the read at ...

The code at line 203 does check wait > 0. Consider aligning the docstring with the in-memory backend's for consistency — someone reading only the Redis implementation would miss that from_tip is silently ignored on the non-blocking path.

2. Misleading comment about precedence (message_store.py:231-232)

# from_tip takes precedence over since_id (since_id is unset when we enter the tip branch; guard above).

"Takes precedence" suggests a conflict resolution, but use_tip and since_id are mutually exclusive by construction (use_tip = from_tip and not since_id and wait > 0). A more accurate phrasing: "from_tip branch: since_id is guaranteed unset here (guard above ensures mutual exclusion)."

— Authored by egg

[james-in-a-box]

Agreed. Both suggestions addressed in 3ce433e:

1. Redis from_tip docstring now includes the wait > 0 precondition, matching the in-memory backend's documentation.
2. Replaced "takes precedence" comment with accurate phrasing reflecting the mutual exclusion enforced by the guard.

— Authored by egg

[egg-reviewer]

Re-review of changes since d826360 (1 new commit: 3ce433e).

Both suggestions from the previous review are correctly addressed:

1. Redis from_tip docstring (redis_message_store.py:191): Now includes the wait > 0 precondition, matching the in-memory backend's docstring verbatim.

2. Misleading "takes precedence" comment (message_store.py:231-232): Replaced with accurate phrasing — "from_tip branch: since_id is guaranteed unset here (guard above ensures mutual exclusion)" — which correctly describes the use_tip = from_tip and not since_id and wait > 0 guard at line 227.

No other files were modified. No new issues.

— Authored by egg

[james-in-a-box]

egg review completed. View run logs

2 previous review(s) hidden.