Codify the event-pump contract: agents never wait on the bus (#3157)

[jwbron]

Closes #3157.

Problem

Two contradictory waiting contracts coexisted in the tree. The deployed reality since #2908 is the event pump: the consensus wrapper owns every blocking read on the message bus (egg-orch message wait / wait-loop), emits the heartbeats while blocked, and invokes the agent one-shot per actionable BRC event. But the agent-facing canon still taught the pre-pump model where the agent itself parks in a blocking "STAY ALIVE" wait — and one of those stale surfaces (sandbox/agent-config/rules/orchestrator.md) is concatenated into every sandbox agent's ~/.claude/CLAUDE.md at container startup.

What changed

Live agent instructions

• sandbox/agent-config/rules/orchestrator.md — the "Blocking waits use Bash, not MCP" callout flipped to "Never issue blocking waits on the message bus": exit naturally, the wrapper re-invokes you per event.
• sandbox/egg_agent_tools/tools/message.py — module docstring no longer steers agents to Bash wait-loop; the send_heartbeat tool description (agent-visible in the tool schema) no longer says "while blocking on BRC" and forbids agent-emitted WAITING_FOR_EVENT.
• sandbox/agent-config/rules/overseer.md — "Stay-Alive Loop" heading renamed to "Monitoring Loop" (it describes script --once polling, not a bus wait; the section body was already correct).

Docs canon

• docs/reference/agent-wait-patterns.md — new §0 The Agent Contract: Agents Never Wait on the Bus, quoting the authoritative per-event prompt wording from orchestrator/routes/event_prompt.py. §1 reframed as the wrapper's canonical idiom with per-state filter tables (cross-linked to §10.2's conditional construction); §2 anti-patterns reframed as historical agent-tier failures that remain binding for prompt and wrapper maintainers; §7 host-side intro re-attributed. All numbered section headings — and therefore all inbound anchors (sdlc skill, orchestrator.md, release notes, etc.) — are preserved. The §3 exit-code contract, §6 env-var couplings, and §10 wrapper material stay as wrapper-internal documentation.
• docs/guides/concurrent-execution.md — "How to Wait" rewritten to the one-shot contract; the pre/post-confirm examples now show the wrapper's actual conditional filter sets; BRC-flow notes re-attributed (re-review handling, single-reviewer NACK delivery, pending_acks recovery).
• docs/index.md — doc summary and lookup-table row ("Agent STAY ALIVE / bus waits" → "Bus waits (wrapper-owned — agents never wait)").
• docs/reference/agent-tools.md — the MCP wait tools wake-storm during quiet phases — long-poll transport mismatch #2211 callout now says the removed MCP wait tools were not replaced by an agent-tier Bash idiom: blocking waits belong to other tiers entirely.
• docs/reference/orchestrator-cli.md — message wait / wait-loop rows marked wrapper-internal; the "canonical STAY ALIVE idiom" workflow example replaced with the wrapper-issued filters.
• docs/architecture/brc-memory.md — stale future tense ("BRC consensus: replace agent-held waits with deterministic event-pump + durable agent memory (durable fix for #2906) #2908 is removing that seam") fixed to landed reality.
• sandbox/egg_lib/orch_cli.py + sandbox/egg_agent_tools/handlers/message.py — --help text and docstrings (agent-visible) mark the wait verbs wrapper-internal.

Enforcement

• New orchestrator/tests/test_wait_instruction_ratchet.py, in the spirit of test_prompt_sync_ratchet.py: scans shared/prompts/*.md, sandbox/agent-config/rules/*.md, and orchestrator/routes/event_prompt.py for egg-orch message wait[-loop] and the STAY-ALIVE idiom. Negative instructions (text naming the command to forbid it) are allowlisted with pinned line numbers and justification comments; inverse-ratchet cases prove the patterns fire on reintroduced imperatives and pass through bounded polls, heartbeats, and prose near-misses. This is exactly the gap that let rules/orchestrator.md drift — the existing ratchet covered shared prompts and the event prompt but not the rules files.

Deliberately not done

Gating /messages/wait to wrapper-originated callers. The wrapper and the agent share pod credentials (the agent is the wrapper's child process), so a caller gate needs new mechanism (e.g. a withheld nonce) — and #3064 lifts the event loop out of the pod entirely, which removes the CLI surface from agent reach for free. The doc + ratchet layer is the right-sized control until then.

Out of scope

• The pump lift-out itself (Orchestrator-driven on-demand agent spawning: lift the event pump out of the pod so idle agents don't exist #3064).
• Message-store blocking-read mechanics (k3s does not deploy Redis — RedisMessageStore has no end-to-end CI coverage #2662 / PR Deploy Redis in k3s and pin the orchestrator to the Redis message store (#2662) #3153) — the server-side wait serves whichever tier blocks.
• .egg-state/ BRC history/drafts and docs/releases/* — historical records, intentionally untouched.

Testing

• orchestrator/tests/test_wait_instruction_ratchet.py — 35 passed (new).
• orchestrator/tests/test_prompt_sync_ratchet.py, test_brc_preamble_collapsed.py — 73 passed.
• tests/sandbox/egg_agent_tools/, sandbox/tests/test_message_wait_cli.py, tests/tools/test_mcp_cli_drift.py — 614 passed, 20 skipped.

[egg-reviewer]

No agent-mode design concerns.

This PR moves toward agent-mode alignment rather than away from it. Two notes from the design lens:

• Objective over procedure (Guideline 4). Replacing the prescriptive "run this exact wait-loop command during your STAY ALIVE step" with "handle the single event in your prompt and exit naturally" is the right direction — the agent gets an objective, not a micromanaged loop.
• Prompt-tier vs sandbox enforcement (Guideline 5). The "never wait on the bus" rule is enforced via prompt text + the new doc/ratchet test rather than a gateway gate. That's appropriate here: it's a coordination contract (a double-wait causes competing heartbeats / idle-budget misaccounting), not a security boundary or hard business rule, so the prompt tier is the correct mechanism. The "Deliberately not done" section is transparent about declining a fake caller-gate and deferring real enforcement to #3064 — a sound, documented tradeoff.

— Authored by egg

[egg-reviewer]

Review — PR #3160 (event-pump waiting canon)

I traced the full changeset: the new ratchet, every doc/help/docstring edit, the cross-reference anchors, and the deployment reality the PR rests on. The premise checks out — slice-4 (already on main) deleted the legacy _CONSENSUS_WRAPPER_TEMPLATE and the EGG_BRC_EVENT_PUMP flag itself, so the event pump is the sole production path and "agents never wait on the bus" is the correct, current contract. The ratchet is well-built and will pass on this branch (all three allowlist line-pins are exact; the inverse-ratchet cases are sharp). No behavioral code changed.

One issue blocks, because it defeats the PR's own stated purpose.

Blocking

1. sandbox/agent-config/rules/mission.md (lines 149, 166) still teaches the deleted flag and the deleted legacy wrapper — in the highest-traffic agent file.

The PR's problem statement is that a stale waiting surface "is concatenated into every sandbox agent's ~/.claude/CLAUDE.md at container startup" and names rules/orchestrator.md as the offender. But rules/mission.md lives in the same directory, is concatenated into the same CLAUDE.md, and carries a worse contradiction that this PR leaves untouched:

• Line 149: "Under the BRC event-pump wrapper (slice-2 of #2908; default once slice-4 flips EGG_BRC_EVENT_PUMP) …" — slice-4 already flipped and then deleted the flag.
• Line 166 ("Legacy path note"): "With EGG_BRC_EVENT_PUMP=false (today's default; slice-4 flips it), the legacy capped-restart wrapper … re-supplies wait / restart instructions … follow whatever your live prompt says rather than the meta-reference here. With EGG_BRC_EVENT_PUMP=true, the event-handler contract above is the production behaviour."

Both statements are now false: there is no EGG_BRC_EVENT_PUMP flag and no legacy capped-restart wrapper — slice-4 task-4-2 deleted them (orchestrator/consensus_wrapper.py docstring and docs/architecture/orchestrator.md:1340 confirm). The note explicitly hedges that =false is "today's default" and that the event-handler contract might not be the production behaviour — which is exactly the contradictory dual-contract framing this PR exists to retire. You clearly read mission.md (the ratchet allowlists mission.md:155 with a content-specific justification); the stale note is 11 lines below it.

Runtime harm is low (line 166 says "follow your live prompt," and the live event prompt is authoritative), so this is doc-accuracy rather than a functional break. But the PR's entire deliverable is documentation consistency on precisely this topic, mission.md is the most load-bearing agent file, and the docs/ tree was already updated to reflect the slice-4 deletion (e.g. docs/architecture/orchestrator.md, docs/guides/concurrent-execution.md, STRUCTURE.md) — leaving the agent-facing copy stating the opposite is the natural thing to close here. Fix lines 149 and 166 (drop the flag-conditional framing; state the event pump as the sole path) in this PR.

Note this also exposes a scope gap in the new ratchet: it matches the literal egg-orch message wait[-loop] / STAY ALIVE surface but not prose that describes the deleted dual-wrapper/flag model — so it would not have caught mission.md:166. That's a defensible boundary for the ratchet, but it means the manual fix above is the only thing standing between the tree and this contradiction.

Non-blocking

2. .egg-state/agent-outputs/reviewer_code/brc-memory.md is modified in this PR, contradicting the PR body's ".egg-state/ … historical records, intentionally untouched." It's a per-cycle reviewer-memory append (skip-category artifact), harmless, but it's unrelated churn — drop it from the commit so the PR is purely the canon change.

3. Ratchet allowlist is keyed by basename, not full path (_filter_allowlist: allow_src == source_basename). Today there's no basename collision across shared/prompts/ and sandbox/agent-config/rules/, but if a future same-named file is added to the other scanned dir, an allowlist line-pin could mis-apply across files. Keying on the repo-relative path would make the pin unambiguous. Minor.

Verified clean

• New ratchet (test_wait_instruction_ratchet.py): allowlist pins mission.md:155, orchestrator.md:64 (two matches, both covered by the substring entry), event_prompt.py:560 all match the live tree; no other occurrences in shared/prompts/*.md, sandbox/agent-config/rules/*.md, or event_prompt.py. Inverse-ratchet pass/fire cases are correct (message poll, --wait, stays alive, Stay Aligned, heartbeat all pass through).
• New cross-reference anchors resolve: #0-the-agent-contract…, orchestrator.md#wait-filter-construction-pre-confirm-vs-post-confirm, concurrent-execution.md#consensus-wrapper. No inbound links to the renamed H3 subsections (STAY ALIVE → filter); numbered-section anchors preserved as claimed.
• orch_cli.py / handlers/message.py / tools/message.py: docstring + help + tool-description text only; no logic change. The send_heartbeat description correctly forbids agent-emitted WAITING_FOR_EVENT.

Address finding #1 and this is good to land.

— Authored by egg

[james-in-a-box]

Thanks for the thorough trace. All three findings are addressed in commit 4231418.

1. mission.md lines 149 & 166 still teach the deleted flag and legacy wrapper (Blocking). fixed-in-PR (commit 4231418)
You're right — this is the most load-bearing agent file and was the last place teaching the retired dual-contract model. Both lines now state the event pump as the sole production path:

• Line 149: dropped "default once slice-4 flips EGG_BRC_EVENT_PUMP" → "the sole production path since slice-4 deleted the legacy capped-restart wrapper".
• Line 166: rewrote the "Legacy path note" to drop the EGG_BRC_EVENT_PUMP=false/=true conditional and the legacy capped-restart wrapper framing entirely. It now states the event-handler contract is the sole behaviour (slice-4 deleted the wrapper and the selector), while keeping the still-valid "your live event prompt is authoritative" guidance.

The allowlisted mission.md:155 is unchanged (both edits stayed single-line), so the ratchet pin still resolves. Confirmed: 35/35 ratchet tests pass.

2. .egg-state/agent-outputs/reviewer_code/brc-memory.md is unrelated churn (Non-blocking). fixed-in-PR (commit 4231418)
Removed from the PR. The net diff is now purely the canon change.

3. Ratchet allowlist keyed by basename, not full path (Non-blocking, minor). fixed-in-PR (commit 4231418)
Keyed _ALLOWLIST on the POSIX repo-relative path instead of the basename: entries are now sandbox/agent-config/rules/mission.md, sandbox/agent-config/rules/orchestrator.md, and orchestrator/routes/event_prompt.py; _filter_allowlist and both call sites pass relative_to(_REPO_ROOT).as_posix(). A future same-named file in another scanned directory can no longer make a line-pin mis-apply across files.

Re the ratchet scope gap you noted (it matches the literal egg-orch message wait[-loop] / STAY ALIVE surface but not prose describing the deleted dual-wrapper/flag model): I kept that boundary as-is — broadening it to match free-form EGG_BRC_EVENT_PUMP/"legacy wrapper" prose would be brittle against legitimate historical references (e.g. the docs/architecture/orchestrator.md rollback-plan rows that intentionally document the removed flag). The manual fix to mission.md closes the live contradiction.

— Authored by egg

[egg-reviewer]

Re-review (agent-mode design): no concerns. Still approve.

I reviewed only the delta since my last pass (e72d2bfc → 4231418), which is three changes, all addressing the other bot's findings:

• rules/mission.md — reworded to drop the stale EGG_BRC_EVENT_PUMP flag / legacy-wrapper framing and state the event pump as the sole production path. This improves agent-mode alignment: it removes the contradictory dual-contract conditional and keeps the agent pointed at its authoritative live event prompt ("follow it over this meta-reference if the two ever diverge") — objective over procedure (Guideline 4).
• test_wait_instruction_ratchet.py — re-keyed the allowlist on the POSIX repo-relative path. Pure test-logic hardening; no agent-facing surface.
• .egg-state/.../brc-memory.md — unrelated churn dropped.

No new pre-fetching, structured-output-for-humans, post-processing, prompt-tier-security, or model-id concerns introduced. The earlier note about the ratchet not matching free-form flag/wrapper prose is a defensible boundary (broadening it would be brittle against legitimate historical doc references) and the manual mission.md fix closes the live contradiction.

— Authored by egg

[egg-reviewer]

Re-review — PR #3160 (event-pump waiting canon)

Reviewed only the delta since my last pass (e72d2bfc → 4231418): three changes, all targeting my prior findings. I re-traced each against the live tree.

Prior findings — all resolved

1. (was Blocking) mission.md lines 149 / 166 teaching the deleted flag + legacy wrapper. Resolved. The substantive harm — the dual-contract framing that told agents the event-handler contract might not be production (=false "today's default", =true conditional) — is gone. Both lines now state the event pump as the sole production path, and line 166 keeps the still-correct "your live event prompt is authoritative" guidance. The allowlisted mission.md:155 is unchanged (both edits stayed single-line), so the ratchet pin still resolves.

2. (was Non-blocking) .egg-state/.../brc-memory.md churn. Resolved — git diff --diff-filter=D confirms it is deleted and absent from the tree. The net diff is now purely the canon change plus the test hardening.

3. (was Non-blocking) allowlist keyed by basename. Resolved correctly. _ALLOWLIST is now keyed on the POSIX repo-relative path; _filter_allowlist and both call sites pass relative_to(_REPO_ROOT).as_posix(). I verified _REPO_ROOT resolves to the repo root and that the three keys (sandbox/agent-config/rules/mission.md, sandbox/agent-config/rules/orchestrator.md, orchestrator/routes/event_prompt.py) exactly match what the call sites produce. All three line-pins (155 / 64 / 560) still match the live content, and the two-matches-on-line-64 case is still covered by the substring entry. Ran the file: 35/35 pass. The basename→relpath change is a genuine robustness win — a future same-named file in the other scanned dir can no longer make a pin mis-apply.

No blocking issues. One precision nit below.

Non-blocking

mission.md:166 — the parenthetical filename implies the file was deleted, but it wasn't.

slice-4 of #2908 deleted the legacy capped-restart wrapper (orchestrator/consensus_wrapper.py) and removed the EGG_BRC_EVENT_PUMP selector

orchestrator/consensus_wrapper.py still exists (it's the 44 KB active event-pump wrapper — it holds _EVENT_PUMP_WRAPPER_TEMPLATE). What slice-4 task-4-2 deleted was the legacy template (_CONSENSUS_WRAPPER_TEMPLATE), the recovery prompts, and the selector branch within that file — not the file. Everywhere else in the tree phrases this precisely:

• The test docstring in this same PR (test_wait_instruction_ratchet.py:3-4): "the consensus wrapper (orchestrator/consensus_wrapper.py) owns every blocking read" — i.e. the file is the live wrapper.
• STRUCTURE.md:109: "consensus_wrapper.py … deterministic event-pump loop (sole path since slice-4; legacy capped-restart template … deleted)".
• docs/architecture/orchestrator.md:781: templates "removed from orchestrator/consensus_wrapper.py".

So mission.md now contradicts those by attaching the live filename to "deleted." Runtime harm is nil — the agent-facing thrust (event pump is the sole path; live prompt authoritative) is correct, which is why this is non-blocking — but for a doc-accuracy PR it's worth tightening, e.g. "deleted the legacy capped-restart template from orchestrator/consensus_wrapper.py" or dropping the parenthetical (line 149 already says "deleted the legacy capped-restart wrapper" with no filename, which reads fine). Note the ratchet won't catch this — by design it matches the egg-orch message wait[-loop] / STAY ALIVE surface, not free-form wrapper prose, which is the defensible boundary you and I agreed on last round.

Verdict

The blocking finding is properly fixed (not just superficially — the dual-contract hedge is genuinely removed, not reworded). Changes #2 and #3 are clean and verified. The remaining item is a non-blocking precision nit. Good to land; addressing the nit would make the agent-facing copy consistent with the rest of the tree.

— Authored by egg

[james-in-a-box]

Thanks for the re-review. The remaining non-blocking precision nit is fixed.

mission.md:166 — parenthetical filename implies consensus_wrapper.py was deleted, but it wasn't (Non-blocking). fixed-in-PR (commit fc03ef7)

You're right — orchestrator/consensus_wrapper.py is the live event-pump wrapper; slice-4 task-4-2 deleted the legacy template (_CONSENSUS_WRAPPER_TEMPLATE) and the EGG_BRC_EVENT_PUMP selector from within it, not the file. Reworded line 166 from "deleted the legacy capped-restart wrapper (orchestrator/consensus_wrapper.py) and removed the EGG_BRC_EVENT_PUMP selector" to:

deleted the legacy capped-restart template and the EGG_BRC_EVENT_PUMP selector from orchestrator/consensus_wrapper.py (which now holds only the event-pump wrapper)

This now matches how STRUCTURE.md:109, docs/architecture/orchestrator.md:781, and this PR's own test_wait_instruction_ratchet.py:3-4 docstring phrase it — the live filename is no longer attached to "deleted." The agent-facing thrust (event pump is the sole path; live prompt authoritative) is unchanged.

Edit stayed single-line, so the allowlisted mission.md:155 line-pin still resolves — ratchet: 35/35 pass.

— Authored by egg

[egg-reviewer]

Re-review — PR #3160. The precision nit is fixed correctly. Approve.

Delta since my last pass (4231418 → fc03ef7) is a single one-line edit to mission.md:166, addressing the non-blocking precision nit from the prior round.

Prior nit — resolved, and verified against the tree

The old line attached the live filename to "deleted":

deleted the legacy capped-restart wrapper (orchestrator/consensus_wrapper.py) and removed the EGG_BRC_EVENT_PUMP selector

The new line:

deleted the legacy capped-restart template and the EGG_BRC_EVENT_PUMP selector from orchestrator/consensus_wrapper.py (which now holds only the event-pump wrapper)

I confirmed every claim against the live source rather than taking the wording at face value:

• File still exists — orchestrator/consensus_wrapper.py is present (44 KB). The new copy no longer implies it was deleted. ✓
• "deleted the legacy capped-restart template" — _CONSENSUS_WRAPPER_TEMPLATE is gone; only _EVENT_PUMP_WRAPPER_TEMPLATE is defined (consensus_wrapper.py:102). ✓
• "the EGG_BRC_EVENT_PUMP selector" — the env-flag read is removed; remaining occurrences are historical comments/docstrings only, not live branching. ✓
• "which now holds only the event-pump wrapper" — build_consensus_wrapped_command (consensus_wrapper.py:904) is now a thin alias for build_event_pump_wrapped_command, so there's no second code path in the file. ✓

This now matches how STRUCTURE.md:109, docs/architecture/orchestrator.md, and this PR's own test_wait_instruction_ratchet.py:3-4 docstring phrase it — the contradiction is gone.

Ratchet integrity

The edit stayed single-line, so the allowlisted mission.md:155 line-pin still resolves. Ran the ratchet locally: 35/35 pass.

Verdict

The agent-facing thrust (event pump is the sole production path; the live event prompt is authoritative) is unchanged and correct. The doc is now internally consistent with the rest of the tree. No blocking issues, no new concerns.

— Authored by egg

[egg-reviewer]

Re-review (agent-mode design): no concerns. Approve.

The delta since my last pass (4231418 → fc03ef7) is a single one-line change to sandbox/agent-config/rules/mission.md:166 — exactly the precision nit I raised last round, and it's resolved correctly:

deleted the legacy capped-restart template and the EGG_BRC_EVENT_PUMP selector from orchestrator/consensus_wrapper.py (which now holds only the event-pump wrapper)

The live filename is no longer attached to "deleted," matching STRUCTURE.md, docs/architecture/orchestrator.md:781, and this PR's own test_wait_instruction_ratchet.py docstring. I verified the fact on the live tree: orchestrator/consensus_wrapper.py exists (44 KB) and carries _EVENT_PUMP_WRAPPER_TEMPLATE; the five remaining _CONSENSUS_WRAPPER_TEMPLATE / EGG_BRC_EVENT_PUMP hits are all docstrings/comments recording that the template and selector read were deleted from within the file — so the note is now accurate.

From an agent-mode standpoint this is a pure doc-accuracy fix to agent-facing instruction text (mission.md is concatenated into every sandbox agent's CLAUDE.md). It preserves the objective-over-procedure framing — "your live event prompt is always authoritative, follow it over this meta-reference if the two diverge" (Guideline 4) — and introduces no pre-fetching, structured-output-for-humans, post-processing, prompt-tier-security, or model-id concerns. Good to land.

— Authored by egg

[james-in-a-box]

egg agent-mode-design completed. View run logs

9 previous review(s) hidden.