Issue #1897: event-driven BRC wait primitives + agent heartbeats

[james-in-a-box]

During the issue-1762-membump pipeline run on 2026-04-22, agents
were observed using sleep-and-poll heuristics (for i in 1..10; do egg-orch message poll --wait 60; done, sleep 300 && …) that
blocked actionable BRC messages from being consumed for 5–10
minutes at a time. The root cause is two-fold: (a) the BRC
preamble's "Keep polling …" wording at
orchestrator/routes/pipelines.py:6231 (producer) and :6292
(reviewer) reads like an action the agent itself must orchestrate,
and (b) the 60-second cap on message poll --wait at
orchestrator/routes/messages.py:165 forces some outer loop,
which LLMs then improvise with idioms from training data. There is
also no typed wait primitive, no per-agent state heartbeat, and the
QUESTION message type sits unused in production (but is actively
advertised in the reviewer preamble at
orchestrator/routes/pipelines.py:6342-6346).

This PR implements the full Option C scope confirmed at the
refine phase gate. The work is organised into nine phases inside
one PR so reviewers can step through commit-by-commit:

1. Backend primitives (orchestrator/message_store.py,
   redis_message_store.py) — typed XREAD BLOCK with
   message_type filter, condition-variable blocking on the
   in-memory store with safe clear() semantics (RISK-5),
   removal of the silent non-blocking fallback at
   routes/messages.py:181-184.
2. HTTP route + CLI + env cap — new GET /messages/wait
   endpoint and egg-orch message wait --for TYPE [--from ROLE] [--timeout N] with a deterministic exit-code
   contract (0=matched, 1=timeout, 2=transient, 3=permanent,
   RISK-9); an EGG_MESSAGE_POLL_MAX_WAIT env var (default
   60s) housed in a new orchestrator/env_config.py module;
   a wait-loop convenience subcommand that loops forever and
   exits only on terminal match OR permanent error (RISK-9,
   reviewer_plan blocker 6); a startup log warning when the
   cap exceeds 90s naming the gateway's Squid directives
   (RISK-4).
3. HEARTBEAT type + HealthMonitor wiring + rate limit —
   additive HEARTBEAT enum member with structured state field
   (WORKING | WAITING_ON_ROLE | PROPOSED | IDLE) in metadata;
   metadata-schema validation in routes/messages.py;
   HealthMonitor's MESSAGE_SENT subscription resets
   last_heartbeat when message_type == 'HEARTBEAT' (RISK-2);
   server-side rate limit EGG_HEARTBEAT_RATE_LIMIT default
   20/min, 429 on exceed (architect TD-3). Legacy
   PROGRESS-heartbeat path retained behind a TODO.
4. Waitress thread pool sizing for long polls — raise the
   Waitress thread count via a new EGG_ORCH_WAITRESS_THREADS
   env var (default 16, refuse-to-boot below 4) wired into
   orchestrator/cli.py:290; export
   egg_inflight_long_polls via
   orchestrator/metrics.py (RISK-3). Gunicorn migration is
   explicitly out of scope and filed as a follow-up issue;
   /api/v1/health at routes/health.py:34-77 is already
   off the message-store path so no new /healthz endpoint
   is needed (reviewer_plan blocker 2).
5. consensus_wrapper SSE rewrite —
   orchestrator/consensus_wrapper.py:322-351 shell sleep
   loop replaced with curl --no-buffer SSE against the
   existing /api/v1/pipelines/<id>/stream endpoint at
   routes/pipelines.py:11772 parsing event: consensus.reached (RISK-6). SIGTERM cleanly closes the
   socket (RISK-7). Falls back to current shell sleep loop
   if SSE endpoint unavailable.
6. Agent prompt audit — producer + reviewer "STAY ALIVE"
   blocks in orchestrator/routes/pipelines.py:6231,6292
   rewritten to teach the canonical egg-orch message wait-loop --for CONSENSUS_CONFIRMED --for CONSENSUS_RE_REVIEW --for OVERSEER_ALERT one-liner
   (which loops forever, per TASK-2-4) and the explicit
   Don'ts (no for-loops, no sleep N); same line updated in
   sandbox/agent-config/rules/mission.md:152.
7. QUESTION removal (safe commit order) — staged across
   five commits: (a) edit reviewer prompt at
   pipelines.py:6342-6346 (replace QUESTION example with
   a forward-pointer to follow-up issue), (b) drop QUESTION
   from BRC_HISTORY_TYPES at pipelines.py:5037-5052, (c)
   update test fixtures in test_brc_history.py,
   test_concurrent_integration.py,
   test_checkpoint_inter_agent.py,
   test_checkpoint_cli_inter_agent.py,
   test_brc_cli_args.py, (d) drop "QUESTION" from
   cmd_message_send argparse choices at
   sandbox/egg_lib/orch_cli.py:1862 (NEW, reviewer_plan
   blocker 5), (e) remove the enum member. Each commit keeps
   the test suite green (RISK-1, RISK-10).
8. Tests — concurrent-integration smoke test for sub-2s
   reaction time, regression test for PR Fix #1889 + #1890: bridge contract decisions at phase_gate; harden plan HITL gate #1896's
   _existing_confirmed_for_role dedup (HITL Q1 follow-up),
   and a deliberately-misconfigured-cap test that asserts the
   expected 504 (RISK-4 named failure mode).
9. Docs — docs/reference/agent-wait-patterns.md (new)
   documenting the canonical idiom, the four anti-patterns
   quoted from Agent wait heuristics: replace sleep/poll loops with event-driven BRC message stream consumption #1897, the exit-code contract, the HEARTBEAT
   schema (state + waiting_on + since), and the explicit
   EGG_MESSAGE_POLL_MAX_WAIT ↔ gateway-Squid-directive-
   via-image-rebuild coupling block; a "How to wait" section
   added to docs/guides/concurrent-execution.md.

Impact. Agents react to BRC messages within seconds rather
than minutes; bus chatter from for i in 1..N; do consensus confirmed loops (already mitigated by PR #1896) is now also
impossible by construction once Phase 6's prompts ship; the
overseer reads HEARTBEAT directly so Tier-1 alarms no longer
falsely trip on agents that adopt the new heartbeat type
(RISK-2); local-dev runs without Redis exhibit the same
blocking semantics as production (decision-4); the Waitress
thread pool is sized to absorb the new long-poll volume
without saturating short-request threads (RISK-3); and the
agent prompt teaches a single one-liner idiom so LLMs have
zero degrees of freedom in how they wait.

Test Plan

• Automated unit tests (orchestrator/tests/):
  • test_message_store.py — Phase 1: condition-variable
    blocking, type filter, env-cap respected, clear()
    wakes blocked threads within 100ms, blocked threads
    return empty list when their pipeline disappears.
    Phase 3: HEARTBEAT metadata schema round-trip;
    WAITING_ON_ROLE without waiting_on raises ValueError;
    HEARTBEAT rate limit (TASK-3-4) returns 429 after
    20/min.
  • test_redis_message_store.py — Phase 1: XREAD BLOCK
    with type filter (skipped if no Redis); inner-loop cap
    of 100 enforced; correct timeout when only unwanted
    types arrive.
  • test_messages.py — Phase 2: GET /messages/wait
    route (success/timeout/missing-for/role-filter);
    Phase 1 env-cap test (EGG_MESSAGE_POLL_MAX_WAIT=120
    clamps wait=180 to 120, default still 60); Phase 3
    server-side HEARTBEAT metadata validation (400 on
    malformed). (Note: the test file is test_messages.py,
    not test_messages_route.py — verified.)
  • test_pipeline_prompts.py — Phase 6: producer +
    reviewer lifecycle assertions updated to lock in the
    new STAY ALIVE wording at lines 6231/6292, the
    canonical wait-loop idiom, the explicit Don'ts. Phase
    7: assert QUESTION example is no longer in the reviewer
    preamble at lines 6342-6346 (with forward pointer to
    follow-up issue) and BRC_HISTORY_TYPES at 5037-5052
    no longer contains QUESTION.
  • test_consensus_wrapper.py — Phase 5: wrapper unblocks
    within 100ms of a consensus.reached SSE event on the
    /stream endpoint; SIGTERM during a 60s wait produces
    exit 0 within 2s; total wait budget ≤
    MAX_READY_POLLS * EGG_MESSAGE_POLL_INTERVAL (the bash
    template variable at consensus_wrapper.py:304,
    sourced from MAX_READY_POLL_CYCLES = 10 at
    consensus_wrapper.py:38); SSE fallback to shell sleep
    loop when endpoint 5xxes; explicit test subscribes to
    /stream and asserts the literal SSE event-name is
    consensus.reached (reviewer_plan blocker 4 hardening).
  • test_health_monitor.py — Phase 3: HEARTBEAT
    subscription resets last_heartbeat (HEARTBEAT-only
    path produces no heartbeat_timeout alert; PROGRESS-only
    path still works; emitting neither still alerts).
  • test_brc_history.py — Phase 7: QUESTION assertions
    updated to use STATUS; BRC_HISTORY_TYPES assertion
    updated.
  • test_app_startup.py — NEW, created in Phase 2: env
    var startup log (WARN above 90s). Phase 4: refuse-to-
    boot when EGG_ORCH_WAITRESS_THREADS < 4; default 16;
    startup log names the effective thread count.
  • test_health_routes.py — Phase 4: assert the existing
    /api/v1/health handler does NOT touch the message
    store (this is a regression test confirming the
    reviewer_plan blocker 2 finding holds).
  • test_metrics.py — Phase 4: ten concurrent waits raise
    egg_inflight_long_polls to 10, finishing them returns
    it to 0.
  • test_signals.py — Phase 3: valid HEARTBEAT emit and
    dedup (TASK-3-2); Phase 8: _existing_confirmed_for_role
    dedup regression test (TASK-8-2).
• Automated CLI tests (sandbox/tests/):
  • test_orch_cli_message_wait.py — Phase 2: every exit
    code (0/1/2/3) hit explicitly; --for repeatable;
    --from tester filter; --for missing produces exit 3
    (argparse misuse per contract).
  • test_orch_cli_message_wait_loop.py — Phase 2: wait-loop
    loops forever on timeout (exit-1 → continue); exits 0
    on terminal CONSENSUS_CONFIRMED-final match; exits 1 on
    exit-3 permanent; transient (exit-2) triggers backoff
    sleep (≤ 2s in test mode).
  • test_orch_cli_heartbeat.py — Phase 3: emit/dedup;
    invalid state rejected; WAITING_ON_ROLE without
    --waiting-on rejected; rate limit 429 surfaces as
    exit-3.
  • test_consensus_wrapper_sigterm.py — Phase 5: spawn
    wrapper, send SIGTERM, assert exit ≤ grace period.
• Automated integration tests (orchestrator/tests/):
  • test_concurrent_integration.py::test_event_driven_consensus_wait
    — Phase 8: spawn synthetic two-agent pipeline using
    EGG_MESSAGE_STORE_BACKEND=memory in-process via
    the Flask test client, assert consumer agent unblocks
    within 2s of a CONSENSUS_CONFIRMED write (reviewer_plan
    non-blocking clarification on TASK-8-3 harness).
  • test_concurrent_integration.py::test_consensus_confirmed_dedup_regression
    — Phase 8: ten back-to-back egg-orch consensus confirmed calls produce exactly one bus message (HITL
    Q1). (Actually placed in test_signals.py next to the
    handler under test — see TASK-8-2 file list.)
  • test_concurrent_integration.py::test_misconfigured_cap_504
    — Phase 8: boot orchestrator as a subprocess with
    EGG_MESSAGE_POLL_MAX_WAIT=120 AND a separate pytest
    httpbin / pytest-proxy harness simulating the gateway
    Squid read_timeout, assert the resulting 504 is
    named (RISK-4). (Subprocess + proxy harness per
    reviewer_plan non-blocking clarification.)
• Manual:
  1. Reviewer runs make test — all suites green.
  2. Reviewer runs make orchestrator-up then triggers a single
     /sdlc pipeline on a tiny issue and tail -f an agent
     container; confirm the agent log shows
     egg-orch message wait-loop (not for i in … or sleep N).
  3. Reviewer greps the resulting .egg-state/brc-history/
     transcript for ^sleep [0-9] and for i in shell idioms
     in agent commands; expect zero hits.
  4. Reviewer confirms that under
     EGG_MESSAGE_STORE_BACKEND=memory the wait actually
     blocks (time egg-orch message wait --for HEARTBEAT --timeout 5 against an empty pipeline; expect
     ~5.0s real, not ~0.0s).
  5. Reviewer confirms HEARTBEAT does not double-count: with
     only HEARTBEAT messages flowing (no PROGRESS-heartbeat),
     the orchestrator does NOT emit heartbeat_timeout alerts
     after the configured threshold.
  6. Reviewer confirms the author performed the pre-merge
     deliberate-regression sanity checks listed in
     manual_steps (these are noted in the PR description
     per reviewer_plan non-blocking suggestion).

Manual Steps

Pre-merge:
(i) Author runs the deliberate-regression sanity checks
locally before opening the PR for review:
1. Revert TASK-1-1's cv.notify_all() in
add_message, run TASK-8-1's
test_event_driven_consensus_wait, confirm it
fails with timeout (proves blocking is real,
not a tautology).
2. Revert the dedup logic at
routes/signals.py:1241-1294, run TASK-8-2's
test_consensus_confirmed_dedup_regression,
confirm it fails with N=10 messages.
Then re-apply both reverts and confirm the test
suite is green again. Document the result in the
PR description (per reviewer_plan non-blocking
suggestion — this lets a reviewer verify the
author actually did it).
(ii) Phases must be COMMITTED in the order: 1 → 2, 2 → 6,
4 → 6, 6 → 7 (phases 3, 5, 8, 9 are independent).
In particular, Phase 6 (prompt edits) must precede
Phase 7 (QUESTION removal) so the prompt no longer
advertises QUESTION before the type is gone; and
Phase 4 (Waitress sizing) must precede Phase 6
(prompts) so the thread pool is ready when agents
start using the new wait primitive in volume.
Post-merge:
(a) Operators who want to raise EGG_MESSAGE_POLL_MAX_WAIT
above 60s must raise the gateway image's Squid
read_timeout and request_timeout directives in
lockstep. These directives are baked into the gateway
image via squid.conf; raising them requires a
gateway image rebuild (NOT a k8s ConfigMap edit, per
reviewer_plan blocker 3 fact-check).
(b) File a follow-up issue for a structured REQUEST/REPLY
peer-Q&A subsystem (replaces the deleted QUESTION
affordance) and swap the placeholder URL in
orchestrator/routes/pipelines.py (TASK-7-1) with
the actual issue URL.
(c) File a follow-up issue to deprecate the legacy
PROGRESS-heartbeat path in
orchestrator/health_monitor.py:248-257 once
HEARTBEAT adoption is 100%.
(d) File a follow-up issue for the Gunicorn migration
(Phase 4 of this PR uses Waitress; Gunicorn/gevent
migration is explicitly out of scope for #1897).

Pipeline Context

Pipeline: issue-1897
Issue: #1897

Per-phase BRC transcripts: refine, plan, implement.

Authored-by: egg

[james-in-a-box]

egg contract-verification completed. View run logs

24 previous review(s) hidden.

[egg-reviewer]

Re-Review: Issue #1897 — event-driven BRC wait primitives + agent heartbeats

Reviewed the delta from 497328a to 5562c8c (single commit: .PHONY declaration + timeout bump). Also performed a full-PR scan for issues missed in prior rounds.

---

Delta since last review

Commit 5562c8c is correct:

• smoketest-long-poll added to .PHONY list — matches all other test targets
• --timeout=60 bumped to --timeout=90 — prevents test_wait_capped_at_sixty (which blocks for the full 60s cap) from racing the pytest timeout

All prior blocking fixes (B1, B2, B3, T1, T2, NB1, NB2, NB7, D1, QUESTION fixtures, re-review NB1/NB2) verified intact.

---

Blocking

B1. agent-wait-patterns.md CLI examples show a command that doesn't exist
docs/reference/agent-wait-patterns.md:289-298

The heartbeat CLI examples show:

egg-orch heartbeat WORKING
egg-orch heartbeat WAITING_ON_ROLE --waiting-on coder

The actual CLI is egg-orch message heartbeat --state WORKING — heartbeat is a subcommand of message, not a top-level command, and state is a --state flag, not a positional argument (orch_cli.py:2197-2213). An agent or operator copying these examples will get an unrecognized-command error.

Fix:

egg-orch message heartbeat --state WORKING
egg-orch message heartbeat --state WAITING_ON_ROLE --waiting-on coder
egg-orch message heartbeat --state PROPOSED
egg-orch message heartbeat --state IDLE

B2. agent-wait-patterns.md HEARTBEAT schema contradicts implementation
docs/reference/agent-wait-patterns.md:225-248

The doc says "The structured payload lives in metadata" and shows:

{
  "message_type": "HEARTBEAT",
  "body": "(optional)",
  "metadata": {
    "state": "WORKING",
    "waiting_on": "coder",
    "since": "2026-04-23T06:29:00Z"
  }
}

The implementation (orch_cli.py:1245-1260) sends a flat body:

data: dict[str, Any] = {
    "from_role": role,
    "state": args.state,
}

The code comment at line 1249 explicitly says: "The earlier nested metadata form was dead bytes on the wire (the server never read it)." The doc also references metadata.waiting_on in prose (line 243: "Constructing a WAITING_ON_ROLE heartbeat without metadata.waiting_on raises ValueError at the dataclass layer") — there is no dataclass, and the validation is in the route handler against the flat body.

Fix: Update the JSON example and field table to show {from_role, state, waiting_on?, since?} (the flat body the server actually validates).

B3. --json flag on wait-loop produces broken stdout
sandbox/egg_lib/orch_cli.py:1193,1136-1137,2194

_add_json_flag(msg_wait_loop) (line 2194) adds --json to the wait-loop parser. Each iteration calls cmd_message_wait(args) (line 1193), which unconditionally prints JSON when args.json is set (line 1136-1137):

if args.json:
    print_json(result)

On timeout iterations (rc=1), the full timeout response is printed to stdout before the loop continues. A wait-loop that times out 5 times before matching produces 6 concatenated JSON objects on stdout — invalid for any downstream JSON parser.

Fix: Either (a) remove _add_json_flag from the wait-loop parser and document that --json is not supported on wait-loop, or (b) suppress JSON output in cmd_message_wait when matched is False:

if args.json and matched:
    print_json(result)

---

Non-Blocking

NB1. rc not declared local in consensus wrapper shell function
orchestrator/consensus_wrapper.py:342,446

The check_confirmed_and_wait function declares local poll_interval wait_count sse_url (line 342) but uses rc=$? (line 446) without including rc in the local list. This creates a global variable that persists beyond the function scope. Not a runtime bug in this specific usage (rc is always assigned before read), but inconsistent with the function's own convention.

NB2. Reviewer POLL prompt wording is slightly contradictory
orchestrator/routes/pipelines.py:6307-6312

The POLL step says "Do NOT poll in a shell for loop" while also saying "timeout (re-issue the wait)". An LLM must re-issue the command on timeout, which requires some form of repetition. The distinction (tool re-invocation vs shell for-loop) is meaningful but the wording could trip up agents. The STAY ALIVE step correctly uses wait-loop which avoids this ambiguity. Consider rewriting POLL to say "If it times out (exit 1), run the same command again — do NOT wrap it in a shell for-loop."

NB3. wait_messages endpoint forces 1s minimum timeout
orchestrator/routes/messages.py:394-397

When timeout=0, the endpoint forces timeout=1, blocking for at least 1 second. The comment says "A wait endpoint with no timeout is a bug." This is a reasonable design choice, but it's undocumented — an agent that wants an instant non-blocking check on the wait endpoint (with type filtering) will be silently delayed. The poll endpoint exists for non-blocking checks but doesn't support type filtering. Worth documenting in agent-wait-patterns.md that timeout=0 is not instant.

— Authored by egg

[james-in-a-box]

egg review completed. View run logs

25 previous review(s) hidden.

[james-in-a-box]

egg is addressing review feedback...

[james-in-a-box]

Pushed commit e60d0e6 addressing the latest review feedback.

Blocking (all fixed)

• B1: Fixed CLI examples in agent-wait-patterns.md — egg-orch heartbeat WORKING → egg-orch message heartbeat --state WORKING across all four examples.
• B2: Rewrote HEARTBEAT schema section in agent-wait-patterns.md to document the flat body ({from_role, state, waiting_on?, since?, body?}) that the server actually validates. Removed references to the non-existent nested metadata envelope and dataclass layer.
• B3: Removed _add_json_flag from the wait-loop parser. The loop calls cmd_message_wait repeatedly, and each timeout iteration printed a JSON object to stdout, producing concatenated invalid JSON. args.json is now explicitly set to False in cmd_message_wait_loop. Single-shot egg-orch message wait --json still works.

Non-blocking (fixed NB1)

• NB1: Added rc to the local declaration in check_confirmed_and_wait for consistency with the function's own convention.

Non-blocking (not fixed)

• NB2 (POLL prompt wording): The current wording is functional — the STAY ALIVE step's wait-loop idiom is unambiguous and is what agents actually use. The POLL step wording is a secondary fallback reference. Leaving as-is to avoid expanding the diff.
• NB3 (timeout=0 forces 1s): Agreed this is worth documenting but it's a documentation-only item. The poll endpoint covers the non-blocking use case. Deferring to a follow-up.

Verification: make lint clean, 31 sandbox CLI tests passed, 73 consensus wrapper tests passed, 302 pipeline prompt tests passed, 51 message route tests passed.

— Authored by egg

[james-in-a-box]

egg feedback addressed. View run logs

25 previous review(s) hidden.

[james-in-a-box]

Automated feedback loop has reached the maximum of 5 rounds. Human review is needed to make further progress on this PR.

[egg-reviewer]

Re-reviewed the delta (commit e60d0e6). No agent-mode design concerns.

All three blocking items from my prior review are resolved:

• B1/B2: Agent-facing heartbeat docs now match the actual CLI and server contract — agents following these examples will succeed.
• B3: --json correctly removed from wait-loop, preventing broken concatenated JSON output.

The two deferred non-blocking items (NB2 POLL wording, NB3 timeout=0 docs) are reasonable deferrals with no agent-mode design impact.

— Authored by egg

[james-in-a-box]

egg agent-mode-design completed. View run logs

25 previous review(s) hidden.

[james-in-a-box]

Contract Verification Re-Review (incremental) — PR #1919

Previous review commit: 5562c8c8481c635c6c25a7fd2938777899119ee2
Current HEAD: e60d0e6abd826d5c457789a0863574f02e1c24f6

Delta

Single new commit e60d0e6 — addresses three blocking items (B1, B2, B3) and one non-blocking item (NB1) from the code review at 5562c8c.

Files changed (3):

• docs/reference/agent-wait-patterns.md — B1 + B2 fixes
• orchestrator/consensus_wrapper.py — NB1 fix
• sandbox/egg_lib/orch_cli.py — B3 fix

Change-by-change verification

B1 fix (docs CLI examples): The heartbeat CLI examples now show egg-orch message heartbeat --state WORKING instead of the non-existent egg-orch heartbeat WORKING. Verified against the actual argparse setup at orch_cli.py:2206-2234 — heartbeat is a subcommand of message, and state is a --state flag. All four examples (WORKING, WAITING_ON_ROLE, PROPOSED, IDLE) corrected. Verified.

B2 fix (docs schema contradicts implementation): The "Metadata schema" section renamed to "Body schema". JSON example now shows the flat body {from_role, state, waiting_on?, since?, body?} that the server validates at routes/messages.py:460-535, not the non-existent nested metadata envelope. Field table updated with from_role (required) and body (optional). Validation prose corrected to reference waiting_on instead of metadata.waiting_on. Verified against both the route handler and the CLI sender at orch_cli.py:1245-1260. Verified.

B3 fix (--json on wait-loop): Two-layer fix: (a) _add_json_flag(msg_wait_loop) removed from the argparse setup at line 2199, replaced with a comment explaining why; (b) defense-in-depth args.json = False added at the top of cmd_message_wait_loop (line 1191) to prevent JSON output even if the attribute is set through other means. The inner cmd_message_wait call checks args.json at line 1136 — without this fix, each timeout iteration would print a JSON object to stdout, producing concatenated invalid JSON. Single-shot egg-orch message wait --json remains available. Test helper _make_loop_args() at test_message_wait_cli.py:264 confirms json=False is the expected state. Verified.

NB1 fix (local rc declaration): rc added to the local declaration in the check_confirmed_and_wait() bash function at consensus_wrapper.py:342. The variable is assigned at line 446 (rc=$?) and branched on at lines 447/450. Previously it leaked to global scope. Consistent with the function's own convention of declaring all variables local. Verified.

Contract compliance

All changes are correctness and documentation fixes that strengthen previously verified contract items:

• TASK-9-1 (agent-wait-patterns reference doc): B1/B2 fixes make the docs accurate. Previously the docs would have caused agents to fail with command-not-found errors.
• TASK-2-4 (wait-loop subcommand): B3 fix prevents broken JSON stdout. The wait-loop contract (exit 0 on terminal match, continue on timeout) is preserved.
• TASK-5-1 (consensus_wrapper event-driven wait): NB1 is a shell hygiene fix within the fallback polling path. No behavioral change.

No regressions to previously verified criteria. Tests confirmed passing by feedback reviewer (lint clean, 31 sandbox CLI tests, 73 consensus wrapper tests, 302 pipeline prompt tests, 51 message route tests).

Non-blocking items deferred (acceptable)

• NB2 (POLL prompt wording): The wait-loop idiom in STAY ALIVE is unambiguous and is the primary path agents use. Reasonable deferral.
• NB3 (timeout=0 forces 1s): Documentation-only item. The poll endpoint covers non-blocking checks. Reasonable deferral.

Verdict

No contract violations. All blocking review items resolved correctly. Previously verified acceptance criteria remain intact. The delta improves documentation accuracy and CLI correctness.

— Authored by egg

[egg-reviewer]

Re-Review: Issue #1897 — event-driven BRC wait primitives + agent heartbeats

Reviewed the delta from 5562c8c to e60d0e6 (single commit: "Address review feedback: fix docs, wait-loop --json, local rc"). Also performed a deep scan of the in-memory message store, Redis message store, and env_config module for correctness issues that may have been missed in prior rounds.

---

Verification of Previous Feedback

All three blocking issues and the non-blocking issue from the last review are correctly resolved:

Issue	Status	Notes
B1 (CLI examples in agent-wait-patterns.md)	Fixed	All four examples now use egg-orch message heartbeat --state ... (agent-wait-patterns.md:290-299). Grepped all .md and .py files — zero stale egg-orch heartbeat (without message) references remain in non-.egg-state/ files.
B2 (HEARTBEAT schema contradicts implementation)	Fixed	Schema section (agent-wait-patterns.md:225-248) now documents the flat body {from_role, state, waiting_on?, since?, body?} with no metadata envelope. Field table matches server-side validation in routes/messages.py:460-473 and CLI assembly in orch_cli.py:1256-1265. One minor note: doc says since is "ISO-8601 string" but the server accepts any string without format validation — not blocking, just slightly aspirational documentation.
B3 (--json on wait-loop produces broken stdout)	Fixed	_add_json_flag removed from wait-loop parser (orch_cli.py:2199-2202 comment explains why). cmd_message_wait_loop forces args.json = False at line 1191 as a defensive guard for any code path that might construct args manually. The approach works because Python's argparse Namespace allows dynamic attribute assignment.
NB1 (rc not declared local in consensus wrapper)	Fixed	rc added to local declaration at consensus_wrapper.py:342.

---

Deep Scan Results

Performed additional scrutiny on modules not deeply examined in prior rounds:

• orchestrator/message_store.py (in-memory store): Condition-variable implementation is correct. Single RLock with consistent lock ordering, no deadlock risk. clear() correctly wakes blocked threads before popping the CV. The fast path and blocking loop both execute under the lock, preventing TOCTOU races between checking for messages and blocking on the CV.

• orchestrator/redis_message_store.py (Redis store): The wait_for_types inner-loop is sound — matching messages are returned immediately (line 293-294), so the 100-iteration cap only fires after 100 consecutive iterations with zero type-matched messages. No data loss. Timeout arithmetic is correct; remaining <= 0 check at line 277 prevents negative block durations.

• orchestrator/env_config.py: All three env-var helpers parse correctly. get_waitress_threads() catches val < 4 which subsumes zero and negative values. Startup warning at >90s names the Squid coupling correctly.

---

No New Blocking Issues

The delta is clean. The four fixes are correctly implemented, internally consistent, and do not introduce regressions. All prior blocking fixes (B1-heartbeat exit-code, B2-tight-loop on transient, B3-Makefile lifecycle-secret, T1-T2, QUESTION fixtures, NB1-NB2-NB7, D1) remain intact.

— Authored by egg

[james-in-a-box]

Automated feedback loop has reached the maximum of 5 rounds. Human review is needed to make further progress on this PR.

[james-in-a-box]

Automated feedback loop has reached the maximum of 5 rounds. Human review is needed to make further progress on this PR.

[james-in-a-box]

egg contract-verification completed. View run logs

25 previous review(s) hidden.

[james-in-a-box]

egg review completed. View run logs

25 previous review(s) hidden.