Remove the in-memory message store backend (#3159)

[jwbron]

Closes #3159.

Problem

#2662 / PR #3153 pinned every deployment to EGG_MESSAGE_STORE_BACKEND=redis, so the in-memory MessageStore no longer ran anywhere — but the backend, the auto/memory selection modes, the #3077 slice-6 auto→memory fail-loud scaffolding, and ~1,500 lines of in-memory unit tests all still existed. Two store implementations means every behavioral contract (blocking reads, full-history cursor fallback, filters) maintained twice and free to drift, plus the #3076 restart-loss risk if the in-memory store were ever selected again by accident.

Changes

Redis-only store. orchestrator/message_store.py shrinks to the shared types (Message, MessageType, GetMessagesMeta, HEARTBEAT_STATES, coerce_deprecated_message_type) and the get_message_store() singleton accessor — the typing seam stays where every consumer already imports it; RedisMessageStore keeps its own module. The ~370-line in-memory class (including the blocking-Condition machinery added to mirror XREAD BLOCK) is deleted.

Fail-loud selection. This deliberately supersedes the #3077 HITL Q3 freeze, per the issue: EGG_MESSAGE_STORE_BACKEND unset or "redis" selects Redis (there is nothing else the variable could mean); the removed multi-backend-era values "memory"/"auto" and any unknown value raise at creation with a pointer to #3159, and an unreachable Redis raises instead of falling back. The k8s manifest keeps the explicit redis pin as deployed documentation (still pinned by the integration test).

Fallback machinery removed. MEMORY_FALLBACK_MARKER, is_memory_fallback_degraded(), _reset_memory_fallback_state_for_test(), and the degraded components.message_store entry in /api/v1/health exist only because auto→memory was possible — all gone, along with their per-test reset fixtures. The TASK-4-3 probe-isolation regression lock (health must not call into the store) is retained.

Tests migrate to fakeredis. orchestrator/tests/conftest.py installs a session-scoped fakeredis-backed creator behind get_message_store(), replacing the silent auto→memory fallback the unit tier used to land on (reset_message_store() mid-test re-creates against fakeredis too). test_message_store.py is rewritten around the type surface + redis-only creation semantics; in-memory behavioral tests are dropped where test_redis_message_store.py already pins the same contract, and the plural-from_roles/slice filter matrix — previously unit-pinned only against the in-memory store — is ported to the redis unit file. Direct MessageStore() constructions become fakeredis-backed RedisMessageStore; MagicMock(spec=MessageStore) sweeps to spec=RedisMessageStore; the dual-backend parametrizations (test_pipelines_status_wait_route.py, the #2640 regression tier) drop their in-memory arm. The /status/wait blocking-read handshake that used to poke store._cond re-lands as an instrumented _resolve_tip_stream_id snapshot event.

Docs/manifests. concurrent-execution.md, coordination-state.md (wipe-semantics table row retired, slice-6 history preserved), STRUCTURE.md, and the k8s manifest comments now describe the redis-only world; integration_tests/test_message_store_backend.py pins that /api/v1/health carries no message_store component.

Testing

• orchestrator/tests: 6,674 passed, 1 failed — test_state_store_wedge_propagation.py::test_probe_skipped_when_request_context_missing, verified failing identically on pristine main on this host (environmental, pre-existing).
• tests/: 7,257 passed (2 pre-existing reap-script host failures, known from #3115).
• integration_tests/regression message-bus + BRC gap-audit tiers: 39 passed.
• make lint clean.

Out of scope

Message-routing test coverage on top of the store (#2640/#2661) and the third-party-image supply-chain decision (#3154), per the issue.

[egg-reviewer]

No agent-mode design concerns.

This is an infrastructure refactor (removing the in-memory MessageStore backend in favor of Redis-only) touching the message store, health routes, k8s manifests, tests, and docs. It doesn't touch prompt construction, model selection, LLM invocation paths, or any agent input/output surface — so none of the agent-mode design guidelines apply. No direct Anthropic API calls, pinned model identifiers, human-facing structured-output requirements, pre-fetching, or post-processing pipelines were introduced.

— Authored by egg

[egg-reviewer]

Review: Remove the in-memory message store backend (#3159)

I traced the full changeset — production selection logic, the deleted in-memory class and its consumers, the fakeredis test seam, and every mechanical sweep — and verified the cross-module wiring rather than reviewing files in isolation. This is a clean, well-scoped deletion. No blocking issues. Details below, then two minor non-blocking notes.

What I verified

No dangling references to deleted symbols. Grepped all production code for MessageStore (the class), is_memory_fallback_degraded, MEMORY_FALLBACK_MARKER, _reset_memory_fallback_state_for_test, and MESSAGE_STORE_AUTO_FALLBACK_TO_MEMORY — zero hits outside docstrings and .egg-state/ artifacts. Every production importer of message_store pulls only retained symbols (Message, MessageType, GetMessagesMeta, coerce_deprecated_message_type, get_message_store). The _reset_message_store_fallback autouse fixtures and their local import message_store were removed together, so no orphaned F401 imports remain (consistent with the clean make lint claim, and tests/+orchestrator/tests/ show no residual message_store references in the three health/wedge files).

Fail-loud selection is real, not just documented. _create_message_store raises RuntimeError (with the #3159 pointer) on any backend value other than redis/unset, before importing the redis module. get_redis_message_store issues client.ping() and raises ConnectionError on an unreachable Redis — so the "no fallback" claim holds end-to-end. The k8s comment's "failed creation leaves the singleton unset → lazy retry" is also accurate: the exception propagates out of _create_message_store() before _message_store/_redis_store are assigned, so the next get_message_store() retries cleanly.

No deployed behavior change. k8s/base/orchestrator-deployment.yaml already pinned EGG_MESSAGE_STORE_BACKEND=redis (#2662), and the new integration test pins both the manifest value and the absence of the components.message_store health entry. The only behavioral change is for an unset env var (now redis/fail-loud instead of auto/memory-fallback) — exactly the #3076 risk surface the issue targets.

Tests exercise the production path. The session-scoped conftest fixture rebinds _create_message_store to build a real RedisMessageStore over fakeredis.FakeRedis(), so the unit tier runs the actual store class (XADD/XREAD/XRANGE), not a stub. The TestBackendCreation tests capture the genuine pre-patch _create_message_store at import time and mock only get_redis_message_store, correctly covering unset→redis, explicit redis, removed memory/auto→raise, unknown→raise, and connection-failure-propagates. The ported from_roles/slice filter matrix in test_redis_message_store.py and the _resolve_tip_stream_id handshake in test_host_wait_integration.py both drive real RedisMessageStore methods (confirmed both exist). The MagicMock(spec=MessageStore) → spec=RedisMessageStore sweep is valid — RedisMessageStore exposes the full interface (add_message, get_messages, get_messages_with_meta, get_latest_id, get_status, clear). No self-seeding goldens; the one renamed test (test_in_memory_fallback_without_wait → test_wait_returns_immediately_when_messages_exist) has a body that matches its new name. The pre-existing _FakeMessageStore doubles for reconstruct_tracker_from_messages retain real-store coverage via test_brc_gap_audit.py (now fakeredis-backed).

Non-blocking suggestions

1. test_unset_env_selects_redis isn't hermetic w.r.t. EGG_MESSAGE_STORE_BACKEND. It sets REDIS_HOST/PORT/DB but never clears the backend var, relying on the ambient pytest env having it unset. If a runner ever exports EGG_MESSAGE_STORE_BACKEND=memory/auto, the test errors on the unmocked raise rather than asserting the unset→redis contract it's named for. Add monkeypatch.delenv("EGG_MESSAGE_STORE_BACKEND", raising=False) to make the "unset" precondition explicit.

2. (trivia, not introduced here) redis_message_store.py:102 uses the parenthesis-free except json.JSONDecodeError, TypeError: (PEP 758, valid only on the repo's required python>=3.14). It's correct and catches both types — I verified — but it reads like the Python-2 except E, name: form to anyone scanning quickly. Since the PR is already rewriting this module's header, wrapping it as except (json.JSONDecodeError, TypeError): would remove the double-take. Entirely optional.

Nice cleanup — deleting ~1,500 lines of a second behavioral contract that could only drift is the right call, and the migration kept the real coverage.

— Authored by egg

[james-in-a-box]

egg is addressing review feedback...

[james-in-a-box]

Review feedback dispositions

Both reviews (egg-reviewer[bot]) approved the PR with two non-blocking suggestions. Per-item disposition below. No code change was pushed — both items resolve to "no change" for the reasons given.

1. test_unset_env_selects_redis hermeticity (monkeypatch.delenv) — disagree (already present)
The suggested guard is already in the reviewed commit: orchestrator/tests/test_message_store.py:91 calls monkeypatch.delenv("EGG_MESSAGE_STORE_BACKEND", raising=False) as the first statement of the test, before the REDIS_HOST/PORT/DB setenvs. The "unset" precondition is therefore already explicit and the test is hermetic w.r.t. the backend var. The review premise ("never clears the backend var") doesn't hold for commit a4a5f93, so there is nothing to add.

2. Parenthesize except (json.JSONDecodeError, TypeError): in redis_message_store.py:102 — disagree (formatter reverts it)
I applied the parenthesized form and ran make lint. ruff format (which make lint enforces, and CI runs on HEAD) rewrites it straight back to the parenthesis-free PEP 758 form on this repo's required python>=3.14 — reproduced twice: each make lint-fix reports "1 file reformatted" and restores except json.JSONDecodeError, TypeError:. The parenthesized variant cannot survive CI's format check, so the existing form is the canonical one here. As the reviewer noted, the current code is correct and catches both types; this was flagged as a readability double-take ("entirely optional", "trivia, not introduced here"), and the formatter's canonical choice wins.

— Authored by egg

[james-in-a-box]

egg feedback addressed. View run logs

4 previous review(s) hidden.