Add wait_for_status_change MCP tool for event-driven host waits

[james-in-a-box]

Closes #1932

The SDLC skill's Phase 3 / Phase S5 monitor loop polls
get_status(task_id, wait=25) on a pure time-based sleep.
Every cycle the orchestrator returns a full snapshot
regardless of whether anything changed, wasting tokens during
quiet phases (long test runs, idle BRC consensus) and delaying
reactions to OVERSEER_ALERT, phase transitions, and HITL
gates by up to a full poll interval. The server primitives for
event-triggered wait already exist (#1919); this PR is the
host-side counterpart.

1. New MCP tool wait_for_status_change(task_id, wait=25, since=<cursor>) alongside the untouched get_status.
   Wires both the EventBus (for PHASE_STARTED,
   PHASE_COMPLETED, PIPELINE_COMPLETED, PIPELINE_FAILED,
   PIPELINE_CANCELLED, DECISION_CREATED) and
   message_store.get_messages with
   wait_for_types=['OVERSEER_ALERT', 'CONSENSUS_CONFIRMED', 'CONSENSUS_NACK', 'CONSENSUS_RE_REVIEW']. On any event
   returns the full status envelope plus changed: true, trigger, event_type|messages, cursor. On 25 s timeout
   returns {changed: false, no_change: true, current_phase, status, phase_elapsed_seconds, concurrent.consensus, cursor}. DECISION_RESOLVED is explicitly excluded from
   the allowlist so the host does not self-wake after
   provide_input.
2. New HTTP route GET /api/v1/pipelines/<id>/status/wait
   in orchestrator/routes/pipelines.py implementing the
   composite wait via queue.Queue(maxsize=16) + daemon
   thread for the message_store.get_messages call +
   wildcard EventBus handler. Matches the existing
   /messages/wait shape. Tracked by a new
   egg_inflight_host_waits prometheus gauge so operators
   can dashboard host-side load independently from
   sandbox-side long polls. _apply_delphi_filter applied to
   any returned messages so the new route inherits the
   reviewer-redaction contract.
3. EventBus sequence: int field added to the Event
   dataclass (orchestrator/events.py) with a per-EventBus
   monotonic counter populated under the existing _lock.
   The MCP cursor is an opaque compound string
   msg:<redis_stream_id>|evt:<sequence> that the server
   parses into its message-bus and EventBus halves
   independently, closing the same-event-seen-twice race
   (egg-orch message wait-loop returns already-seen messages immediately instead of blocking for new events #1925) for both sources.
4. Waitress default raised 16 → 24 in
   orchestrator/env_config.py::DEFAULT_WAITRESS_THREADS to
   absorb the new host-side wait load (each call holds one
   Waitress worker + one daemon thread for up to 25 s). The
   refuse-to-boot floor stays at 4.
5. SDLC skill updates in skills/sdlc/SKILL.md — §Phase 3
   step 1, §Phase S5 step 1, the surrounding "Important"
   notes, the §Consensus / §HITL / §Pipeline Details /
   §Long-Running Phase Detection / §Troubleshooting
   sections, and the §MCP Tools Reference — switch from
   get_status(task_id, wait=25) to
   wait_for_status_change(task_id, wait=25, since=<prior_cursor>) on every poll after the first.
   The minimal timeout envelope ships concurrent.consensus
   so dashboard consensus never drifts by more than one
   wake cycle. A worked example shows both envelope shapes
   side-by-side to pin the structural branching.
6. Double-sleep regression prevention — the existing
   _apply_get_status_wait in
   orchestrator/mcp_server.py:50-67 stays keyed on
   tool_name == 'get_status' (do NOT generalize). A new
   regression test pins this so a future author cannot
   silently introduce a 25 s async wrapper sleep on top of
   the 25 s server-side wait.
7. Docs + release note — new "Host-Side Waits" §7 in
   docs/reference/agent-wait-patterns.md and release note
   at docs/releases/wait-for-status-change.md.

The 25 s cap is the existing GET_STATUS_MAX_WAIT constant
so raising it (if Claude Code lifts the streamable-HTTP
tool-call timeout upstream, anthropics/claude-code#20335) is
a one-line change. Existing get_status consumers are
unaffected — the refactor that extracts
_build_status_snapshot is pure extraction and a
snapshot-diff test confirms behaviour preservation. Python
SDK MCP surface parity (#1920) is declined for v1 — the
SDLC skill is the only consumer today and in-sandbox agents
already use egg-orch message wait-loop. The 60 s liveness
floor from the issue body is satisfied aspirationally — the
25 s per-call cap plus immediate loop re-entry bounds the
aggregate quiet interval under the floor by construction.

Test Plan

• Automated: new
  orchestrator/tests/test_pipelines_status_wait_route.py
  parametrised over backend=in_memory and backend=redis,
  covering (a) EventBus wake, (b) message-bus wake, (c)
  simultaneous fire (winner/loser), (d) timeout, (e) since
  cursor replay avoidance, (f) DECISION_RESOLVED
  exclusion, (g) malformed-cursor 400, (h) unknown
  pipeline_id 404, (i) daemon-thread lame-duck release
  within wait+epsilon with metric decrement on route
  return, (j) queue.Full drop-with-WARNING. Extended
  test_mcp_tools.py covers envelope construction for both
  branches and dispatcher wiring; existing
  _handle_get_status cases pass unchanged because the
  refactor is pure extraction. Extended test_mcp_server.py
  adds the double-sleep regression (pinning
  _apply_get_status_wait to get_status only). New
  test_events_event_sequence.py covers the sequence
  counter under concurrent publishes. New
  integration_tests/test_host_wait_end_to_end.py drives
  the full MCP → route → event-bus flow against a real
  orchestrator with simulated OVERSEER_ALERT,
  DECISION_CREATED, PHASE_STARTED, and a cursor round-trip
  closing the R2 race window. make lint and
  make test-unit (orchestrator) must pass.
• Manual: (1) Run the SDLC skill against a local
  orchestrator, confirm dashboard renders on {changed: true} and reuses cached snapshot on {no_change: true}
  while still refreshing elapsed time and consensus state.
  (2) Send OVERSEER_ALERT via egg-orch message send,
  confirm host wakes within < 1 s. (3) Force a phase
  transition via egg-contract advance-phase, confirm host
  wakes promptly. (4) Resolve a HITL decision via
  provide_input, confirm the subsequent wait does NOT
  return on DECISION_RESOLVED (no self-wake). (5)
  curl /metrics | grep egg_inflight_host_waits during an
  active session — gauge reflects the in-flight wait and
  decrements on return. (6) Close the Claude Code client
  mid-wait, confirm the EventBus handler is unsubscribed
  within 1 s and the metric decrements. (7)
  curl /api/v1/pipelines/<id>/status/wait?since=garbage
  returns 400 with a descriptive error.

Manual Steps

Pre-merge: none beyond the manual verification above.

Post-merge: operators should watch egg_inflight_host_waits
in Grafana after deploy. The raised Waitress default (16 →
24) accommodates the new load, but if
egg_inflight_long_polls + egg_inflight_host_waits
approaches 24 under steady state, raise the budget further.
The lame-duck daemon-thread window (up to 25 s per
event-wake) is expected and bounded; if operators observe
persistent thread growth beyond the wait cap, trigger the
follow-up tracked in the release note's "Future work". No
schema migration; no breaking changes — the new tool is
additive and Event.sequence is an additive field.

Pipeline Context

Pipeline: issue-1932
Issue: #1932

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

Authored-by: egg

[james-in-a-box]

Addressed all blocking and non-blocking feedback in commit 45f28d2:

Blocking — Documentation Correctness (items 1–3):

• Item 1 (impossible trigger/event_type): Split the single Path A example into two clearly labeled sub-examples — one for trigger: "event" (with event_type: "phase.started") and one for trigger: "message" (with messages: [...]). Applied to SKILL.md Phase 3, Phase S5, and agent-wait-patterns.md §7.1.
• Item 2 (Python constant names vs wire values): Replaced all event_type examples with wire-format values (phase.started, decision.created, pipeline.completed). Updated the §7.1 response-fields table description accordingly.
• Item 3 (§7.7 get_status().cursor): Replaced the contradictory last_cursor = last_status.cursor line with the documented bootstrap protocol — wait_for_status_change(task_id, wait=25) with no since, capturing the cursor from its response.

Blocking — Code (item 4):

• Item 4 (_message_store_tip_id O(N)): Added get_latest_id(pipeline_id) to both MessageStore (O(1) tail read of the in-memory list) and RedisMessageStore (XREVRANGE … COUNT 1). _message_store_tip_id now delegates to this single-call method, eliminating the double-fetch and the 10,000-message load.

Non-blocking (items 5–8):

• Item 5 (Delphi no-op): Updated the comment to clarify that the filter is a no-op for role=None (host caller), plumbed for future role support. Also updated the §7.1 table messages row description.
• Item 6 (duplicate cursor row): Removed the duplicate row in §7.1.
• Item 7 (inter-call EventBus race): Acknowledged — this is a known limitation documented in tests and tracked in the release note's Future Work.
• Item 8 (redundant int() casts): Removed all three: int(event.sequence), int(tip_evt_seq) on both message-wake and timeout paths.

All 4494 orchestrator tests pass, linters clean.

— Authored by egg

[egg-reviewer]

No agent-mode design concerns in the delta.

The new commit (45f28d2) addresses all four blocking items from the code review. From the agent-mode perspective, the documentation fixes are the most relevant changes:

• Split Path A examples (items 1–2): SKILL.md Phase 3 and Phase S5 now show separate trigger: "event" and trigger: "message" sub-examples with correct wire-format values (phase.started, decision.created, etc.). This eliminates the risk of the LLM building branching logic that checks for event_type == "OVERSEER_ALERT" on the event path — a silent miss that would have broken overseer alert detection.
• Bootstrap protocol (item 3): The §7.7 worked example now correctly bootstraps the cursor via wait_for_status_change(task_id, wait=25) with no since, matching the contract documented in SKILL.md.
• O(N) tip-id fix (item 4): Infrastructure-only, no agent-facing impact. get_latest_id() is a clean O(1) replacement.

The non-blocking cleanups (duplicate cursor row, redundant int() casts, Delphi filter comment) are all design-neutral.

Minor observation: §7.7's pseudocode still uses "DECISION_CREATED" (Python constant name) rather than the wire value "decision.created". This is in the reference docs (not SKILL.md), and it's clearly labeled as pseudocode, so it's non-actionable — but noting it for consistency if the docs get a polish pass.

— Authored by egg

[egg-reviewer]

Re-review: wait_for_status_change MCP tool (delta from 9352984)

Reviewed the 5 files changed in commit 45f28d2. The core code changes — get_latest_id delegation, redundant int() removal, Delphi comment update — are correct and clean. The documentation fixes for the split Path A envelopes and bootstrap cursor are well done.

Requesting changes for one remaining documentation correctness issue from the prior review cycle.

---

Blocking — Incomplete fix of prior review item 2

agent-wait-patterns.md §7.7 pseudocode still uses Python constant names

Line 721:

if resp.event_type == "DECISION_CREATED":
    handle_hitl(last_status.pending_decisions)

The wire value is "decision.created" (EventType.DECISION_CREATED.value — see events.py:79). The route returns event.event_type.value at pipelines.py:2681, so resp.event_type will contain "decision.created", never "DECISION_CREATED". This comparison would silently skip HITL handling.

This is the same class of bug as prior review item 2 — the JSON examples were fixed, but this pseudocode comparison was missed. A developer copying this pattern gets a silent no-match on HITL decision events.

Similarly, line 719 references TERMINAL_STATES without defining it. If the reader infers its values from the §7.2 table (which lists PIPELINE_COMPLETED, PIPELINE_FAILED, PIPELINE_CANCELLED), they'd use the Python constant names, producing the same never-match bug for terminal state detection.

Fix: Use wire-format values:

TERMINAL_STATES = {"pipeline.completed", "pipeline.failed", "pipeline.cancelled"}
...
if resp.event_type == "decision.created":
    handle_hitl(last_status.pending_decisions)

---

Non-blocking

1. No unit tests for get_latest_id

Both MessageStore.get_latest_id and RedisMessageStore.get_latest_id are new public methods with zero direct test coverage (orchestrator/tests/test_message_store.py and orchestrator/tests/test_redis_message_store.py have no hits for get_latest_id). The method is exercised indirectly through the route tests, but edge cases — empty pipeline, nonexistent pipeline ID, concurrent append during read — aren't directly covered. Consider adding a small parametrized test to each store's test file.

2. Redis get_latest_id deserializes a full Message unnecessarily

redis_message_store.py:343 calls _message_from_redis(stream_id, fields) — which parses JSON metadata, ISO timestamps, and all other fields — just to return msg.id. Since msg.id comes from fields.get("id") (line 88 of _message_from_redis), the method could extract it directly:

msg_id = fields.get(b"id") or fields.get("id", b"")
if isinstance(msg_id, bytes):
    msg_id = msg_id.decode("utf-8")
return msg_id or None

This avoids JSON parsing and timestamp deserialization on every event-wake and timeout return (the hot path). Minor, but it's a free win.

3. §7.2 allowlist table creates a wire-value trap

The §7.2 table (lines 530-541) lists EventBus triggers using Python constant names (PHASE_STARTED, DECISION_CREATED, etc.) without noting that the wire values differ (phase.started, decision.created). Since the §7.7 pseudocode references this table for branching logic, a reader naturally uses the table's names in comparisons — which is exactly the bug at line 721. Adding a footnote like "EventBus wire values use dotted lowercase (e.g. phase.started); see §7.1 response fields" would close the gap.

---

Verification of prior review items

All four blocking items from the prior review are addressed:

• Item 1 (impossible trigger/event_type combination): Fixed — Path A is now split into two clearly labeled sub-examples in both SKILL.md and agent-wait-patterns.md. Correct.
• Item 2 (Python constant names vs wire values): Partially fixed — JSON examples corrected, but §7.7 pseudocode missed (see blocking issue above).
• Item 3 (§7.7 get_status().cursor): Fixed — bootstrap now uses wait_for_status_change(task_id, wait=25) with no since. Correct.
• Item 4 (_message_store_tip_id O(N)): Fixed — get_latest_id added to both stores, O(1) in both. Correct.

Non-blocking items 5-8 are all addressed: Delphi comment updated (item 5), duplicate cursor row removed (item 6), redundant int() casts removed (item 8). Item 7 (inter-call EventBus race) acknowledged as known limitation.

— Authored by egg

[james-in-a-box]

Contract Verification — Re-review (incremental)

Previous review: commit 9352984762a210ef202097605ce537828070b9ba
Current HEAD: commit 45f28d248527bdde154131db505a2981725f2d42

Delta since last review

One commit since previous review — "Address review feedback: fix doc examples, O(N) tip-id, minor cleanups". Changes in 5 files:

File	Change
docs/reference/agent-wait-patterns.md	Split Path A example into event/message sub-examples; replaced Python constants (PHASE_STARTED) with wire values (phase.started); fixed §7.7 worked example cursor bootstrap (removed invalid get_status().cursor access); removed duplicate cursor row in §7.1 table
orchestrator/message_store.py	Added get_latest_id() — O(1) tail read under lock
orchestrator/redis_message_store.py	Added get_latest_id() — O(1) via XREVRANGE COUNT 1
orchestrator/routes/pipelines.py	Replaced O(N) _message_store_tip_id (fetched up to 10,000 messages to read [-1].id) with get_latest_id(); removed redundant int() casts on event.sequence/current_sequence() (both already int); clarified Delphi filter comment
skills/sdlc/SKILL.md	Same Path A split and wire-value fixes as agent-wait-patterns.md

Verification

All changes comply with the contract. Specifically:

1. Doc accuracy improvements — The Path A split correctly separates event-bus wake (trigger: "event", event_type field) from message-bus wake (trigger: "message", messages array). The prior combined example showed an impossible combination (trigger: "event" with event_type: "OVERSEER_ALERT" — OVERSEER_ALERT is a message-bus type). This is a correctness fix that strengthens task-3-1, task-3-2, and task-4-6 compliance.

2. Wire values over Python constants — Replacing PHASE_STARTED/OVERSEER_ALERT with phase.started/OVERSEER_ALERT (message type) in the event_type field descriptions matches the actual EventType wire format. Prevents LLMs from using Python constant names in event_type comparisons.

3. O(N) → O(1) tip-id fix — _message_store_tip_id previously called get_messages(limit=10_000) and read msgs[-1].id. The new get_latest_id() is O(1) for both in-memory (list tail under lock) and Redis (XREVRANGE COUNT 1). This is a performance fix that doesn't change the route's observable behavior.

4. Redundant int() cast removal — event.sequence is int (dataclass field at events.py:111), current_sequence() returns int (typed at events.py:338), and _build_status_wait_cursor accepts event_tip_seq: int. Casts were no-ops.

5. §7.7 worked example fix — The prior example accessed get_status().cursor, but get_status does not return a cursor. The fix bootstraps the cursor via wait_for_status_change(task_id, wait=25) with no since parameter. Matches the documented cursor protocol.

Regression check

No previously verified criteria are broken by these changes:

• All 45 route/event/integration tests pass
• All 163 MCP tool tests pass
• All 31 CLI tests pass
• Doc examples now match the actual wire format and API behavior

Contract state note

The contract JSON (.egg-state/contracts/issue-1932.json) shows all tasks as pending with no linked commits. The implementation is present in the PR but the contract was not updated via egg-contract complete-task during implementation. The orchestrator is currently unreachable, so egg-contract verify-criterion cannot be run. There are no top-level acceptance_criteria entries to verify (the array is empty). All per-task acceptance criteria have been independently verified against the code.

Verdict

Approve — All acceptance criteria across all 4 phases (18 tasks) are met. The incremental changes since the last review are correctness improvements that strengthen contract compliance. No regressions found.

— Authored by egg

[james-in-a-box]

Addressed all blocking and non-blocking feedback in commit 8fb261c:

Blocking — §7.7 pseudocode wire values:

Fixed the pseudocode to use wire-format values throughout:

• TERMINAL_STATES is now defined inline as {"pipeline.completed", "pipeline.failed", "pipeline.cancelled"} instead of referencing undefined Python constants
• "DECISION_CREATED" comparison replaced with "decision.created"

Non-blocking #1 — get_latest_id unit tests:

Added TestGetLatestId classes to both test files:

• test_message_store.py: 5 tests (empty pipeline, single message, most recent, pipeline isolation, concurrent add during read)
• test_redis_message_store.py: 4 tests (empty pipeline, single message, most recent, pipeline isolation)

Non-blocking #2 — Redis get_latest_id deserialization:

Replaced the _message_from_redis(stream_id, fields) call with direct field extraction: fields.get(b"id"). Avoids JSON metadata parsing and ISO timestamp deserialization on the hot path.

Non-blocking #3 — §7.2 wire-value footnote:

Added wire values to each EventBus row in the §7.2 table and a blockquote footnote explaining that wire values use dotted lowercase format, with a cross-reference to §7.1 response fields.

All 4503 orchestrator tests pass, linters clean.

— Authored by egg

[egg-reviewer]

No agent-mode design concerns in the delta.

Commit 8fb261c addresses all feedback from the previous review cycle (45f28d2). Verified the fixes:

• §7.7 pseudocode wire values (prior blocking item): TERMINAL_STATES now correctly defined as {"pipeline.completed", "pipeline.failed", "pipeline.cancelled"} and "decision.created" used in the comparison. No more silent-miss risk from Python constant names.
• §7.2 wire-value footnote: Each EventBus row in the trigger table now includes the wire value, with a blockquote clarifying the dotted lowercase format and cross-referencing §7.1. Closes the documentation trap that led to the §7.7 bug.
• Redis get_latest_id optimization: Direct fields.get(b"id") extraction instead of full _message_from_redis deserialization. Clean hot-path improvement.
• get_latest_id test coverage: Both stores have direct tests covering empty, single, most-recent, and pipeline-isolation cases.

The overall design remains well-aligned: wait_for_status_change reduces token waste during quiet phases, event filtering and DECISION_RESOLVED exclusion are server-enforced, and the SKILL.md documentation provides API contract knowledge rather than rigid procedures.

— Authored by egg

[egg-reviewer]

Re-review: wait_for_status_change MCP tool (delta from 45f28d2)

Reviewed the 4 files changed in commit 8fb261c. All blocking and non-blocking feedback from the prior review cycle is properly addressed.

Verification of prior review items

Blocking — §7.7 pseudocode wire values: Fixed. TERMINAL_STATES is now defined inline with wire-format values (pipeline.completed, pipeline.failed, pipeline.cancelled). The DECISION_CREATED comparison is replaced with decision.created. No Python constant names remain in any code/comparison context in either agent-wait-patterns.md or SKILL.md.

Non-blocking #1 — get_latest_id unit tests: Added. TestGetLatestId in test_message_store.py (5 tests: empty pipeline, single message, most recent, pipeline isolation, concurrent add during read) and test_redis_message_store.py (4 tests: empty pipeline, single message, most recent, pipeline isolation). The concurrent test in the in-memory store is a good addition — verifies the lock protects against torn reads.

Non-blocking #2 — Redis get_latest_id deserialization: Fixed. Replaced _message_from_redis() with direct field extraction via fields.get(b"id"). Correctly handles the decode_responses=False Redis client configuration (fields come back as bytes). The or fields.get("id", b"") fallback covers clients that return string keys. The msg_id or None return handles empty-id edge cases correctly.

Non-blocking #3 — §7.2 wire-value footnote: Added. Each EventBus row in the §7.2 table now includes the wire value, and a blockquote footnote explains the naming convention with a cross-reference to §7.1.

New code scrutiny

No issues found in the delta:

• The get_latest_id implementations are O(1) in both stores (list tail under lock for in-memory, XREVRANGE COUNT 1 for Redis). Both correctly return None for empty/nonexistent pipelines.
• The _message_store_tip_id function correctly delegates to get_latest_id, eliminating the prior O(N) double-fetch.
• Test coverage is appropriate — edge cases (empty pipeline, pipeline isolation, concurrent access) are directly tested rather than relying solely on indirect coverage through route tests.

— Authored by egg

[james-in-a-box]

Contract Re-verification — Incremental Review

Delta: 45f28d2 → 8fb261c (1 commit)

Changes Reviewed

File	Change	Verdict
docs/reference/agent-wait-patterns.md	Wire-value footnotes in §7.2 table; §7.7 pseudocode uses wire values (decision.created, pipeline.completed)	Correct — addresses blocking items from prior review
orchestrator/redis_message_store.py	get_latest_id() optimized: reads id from Redis hash directly instead of full _message_from_redis() deserialization	Correct — _message_to_redis() stores "id": msg.id at line 50; fields.get(b"id") safely handles both bytes/str keys
orchestrator/tests/test_message_store.py	+5 TestGetLatestId cases (empty, single, most-recent, isolation, concurrent)	All pass
orchestrator/tests/test_redis_message_store.py	+4 TestGetLatestId cases (empty, single, most-recent, isolation)	All pass

Regression Check

All previously-verified test suites continue to pass:

• test_pipelines_status_wait_route.py: 32/32 passed (both backends)
• test_events_event_sequence.py: 7/7 passed
• test_mcp_tools.py (wait/snapshot classes): 7/7 passed
• test_host_wait_integration.py: 6/6 passed
• ruff check + ruff format: clean on all delta files

Contract Compliance

No contract violations found. The delta:

1. Does not break any previously verified acceptance criteria
2. Addresses review feedback from the prior cycle (wire-value fixes, get_latest_id optimization and tests)
3. Strengthens task-4-1 coverage by adding get_latest_id unit tests for both in-memory and Redis backends

Note on Contract State

The orchestrator is unreachable so egg-contract verify-criterion cannot be invoked. The contract JSON shows all tasks as pending with no top-level acceptance criteria (empty array) — a known artifact of the YAML quoting issue fixed in f7480fd. Task-level acceptance criteria embedded in each task definition were used for verification.

— Authored by egg

[james-in-a-box]

egg is addressing review feedback...

[james-in-a-box]

All automated review feedback has been addressed across 4 review cycles (commits 9352984, 45f28d2, 8fb261c). All three review bots approved on the latest commit (8fb261c):

• egg-reviewer[bot] (review): APPROVED
• egg-reviewer[bot] (agent-mode-design): APPROVED
• contract-verification: Approved (no violations)

No remaining actionable feedback from authorized reviewers. Ready for human review.

— Authored by egg

[james-in-a-box]

egg feedback addressed. View run logs

15 previous review(s) hidden.