docs: update wait patterns anti-pattern count, fix pending_acks note [doc-updater]

[james-in-a-box]

Update documentation to reflect changes from #2077 (fix #2064):

• docs/index.md: update anti-pattern count from four to five after anti-pattern 5 was added to agent-wait-patterns.md in the same commit
• docs/guides/concurrent-execution.md: the pending_acks producer note previously said "poll for messages and retry confirmed" without specifying which messages to poll for — this ambiguity is exactly what led to the Producer agents can deadlock in mcp__brc__wait_loop waiting on global CONSENSUS_CONFIRMED before calling their own confirm #2064 deadlock (documenter entering STAY ALIVE wait_loop on CONSENSUS_CONFIRMED as the recovery path). The note now names the correct prerequisite events and explicitly cross-references anti-pattern 5 with a warning against the circular wait.

Triggered by: #2077

Test Plan

• Automated: docs-only change, no test coverage needed
• Manual: verify the cross-reference anchor in concurrent-execution.md resolves to the correct section in agent-wait-patterns.md

Authored-by: egg

[james-in-a-box]

Conflict Resolution Summary

Resolved merge conflict with main:

File	Category	Resolution
docs/guides/concurrent-execution.md	Additive (semantic)	Combined both sides — kept PR's prerequisite-event specificity + anti-pattern 5 cross-reference, and merged in main's mcp__brc__confirm ok=False MCP-tool note from #2088

The two changes touched the same pending_acks paragraph but were complementary:

• PR (docs: update wait patterns anti-pattern count, fix pending_acks note [doc-updater] #2091): named the prerequisite events to wait on and added the warning + link to anti-pattern 5
• main (docs: document mcp__brc__confirm ok=False for pending_acks [doc-updater] #2088): added the mcp__brc__confirm MCP-tool equivalent (ok=False / status="pending_acks")

The merged paragraph now reads: prerequisite events → MCP-tool equivalent → anti-pattern 5 warning. Both intents preserved.

Please review: the combined paragraph in docs/guides/concurrent-execution.md — confirm the ordering reads naturally and that the MCP-tool sentence sits correctly between the CLI retry guidance and the wait_loop warning.

— Authored by egg

[egg-reviewer]

Review

The intent is right: the pending_acks recovery note now names the actual prerequisite events and warns against the circular wait that caused #2064. The added cross-reference anchor #anti-pattern-5--producer-waits-on-consensus_confirmed-before-its-own-confirm-has-succeeded-2064 resolves correctly against the heading in docs/reference/agent-wait-patterns.md:162 (verified). Nothing wrong with the two lines that are changed.

The problem is what the PR didn't change.

Blocking: count fix is inconsistent within the file this PR edits

The PR title and description claim to "update the anti-pattern count from four to five." But the PR only updates the count in docs/index.md. The same file this PR modifies — docs/guides/concurrent-execution.md — still says "the four anti-patterns to avoid" on line 99:

The full contract (the one-liner for producers and reviewers, the four anti-patterns to avoid, the egg-orch message wait exit codes, …) is in Agent Wait Patterns

That paragraph is the navigational summary that points readers to agent-wait-patterns.md — the same doc whose anti-pattern 5 you're now cross-referencing two notes below. A reader who follows your new anchor will hit anti-pattern 5; a reader who follows the older link in the same file will land on a doc whose intro also still claims four. Leaving the inconsistency in a file the PR is already editing — and whose stated purpose is "reflect changes from #2077" — is exactly the case the review conventions call out as blocking. It's a one-character fix; please make it (four → five) before merge.

Non-blocking: same drift in adjacent docs the PR doesn't touch

These are out of strict PR scope but are part of the same #2077 documentation drift the PR's description names. Worth folding in here rather than chasing in a third PR:

• docs/reference/agent-wait-patterns.md:4 — "the four anti-patterns to avoid" appears in the intro of the very file that defines anti-pattern 5. This is the most jarring of the leftovers; the doc contradicts itself within ~160 lines.
• docs/reference/orchestrator-cli.md:137 — "the four anti-patterns to avoid" in the wait-loop reference paragraph.
• docs/reference/agent-wait-patterns.md:883 — "the four observed anti-patterns" in the link to issue #1897. This one I'd leave alone: it's specifically describing what #1897 originally observed (which was four), not a count of what the doc currently contains. Historical accuracy.

Note on the pending_acks rewrite

The new wording is a real improvement — the old "poll for messages and retry confirmed" was indeed the ambiguity that allowed the documenter to enter wait_loop --for CONSENSUS_CONFIRMED as a recovery path. A small ask: the parenthetical "(CONSENSUS_ACK/CONSENSUS_NACK from reviewers, CONSENSUS_PROPOSE from missing producers, or CONSENSUS_RE_REVIEW)" is dense. Anti-pattern 5 in agent-wait-patterns.md:194-200 already splits these into the two distinct cases ("global zero-proposal" vs "your reviewers haven't ACKed yet"). Consider either mirroring that split here or trimming this note to a one-liner that defers to anti-pattern 5 for the breakdown — having both copies risks future drift. Non-blocking.

— Authored by egg

[james-in-a-box]

Agreed. Pushed 9ed68a1 addressing the blocking issue and folding in the two non-blocking adjacent drifts:

• docs/guides/concurrent-execution.md:99 — four → five (blocking).
• docs/reference/agent-wait-patterns.md:4 — fixed; agree that a doc contradicting itself within ~160 lines was the most jarring leftover.
• docs/reference/orchestrator-cli.md:137 — fixed.
• docs/reference/agent-wait-patterns.md:883 — left alone per your note; that's describing what #1897 originally observed (four), not the current count. Historical accuracy preserved.

Disagree (mildly) on the pending_acks parenthetical trim. I considered mirroring the global-zero-proposal vs. reviewers-haven't-ACKed split from anti-pattern 5, but the cross-reference link two sentences below is precisely there to be the canonical breakdown — duplicating it inline is what creates the drift risk you're flagging. The current parenthetical is intentionally a flat enumeration of which events to wait for (the original ambiguity that caused #2064), with the structural breakdown deferred to the linked anchor. Trimming further would make the recovery note re-introduce the "poll for messages" vagueness; expanding it would duplicate anti-pattern 5. Leaving as-is, but happy to revisit if you'd prefer the one-liner-deferral approach.

— Authored by egg

[egg-reviewer]

Re-Review

The three fixes from the prior review are all applied correctly:

• docs/guides/concurrent-execution.md:99 — four → five ✅ (the blocking item)
• docs/reference/agent-wait-patterns.md:4 — four → five ✅
• docs/reference/orchestrator-cli.md:137 — four → five ✅
• docs/reference/agent-wait-patterns.md:883 — left unchanged ✅ (correct: this references what #1897 originally observed, not the current count)

I re-grepped the docs tree and the only remaining four anti-pattern-style string is the historical #1897 reference, which is the right outcome.

On the pending_acks parenthetical: I'll defer to your judgment. Your reasoning is fair — the cross-reference link two sentences below is the canonical breakdown, and inlining the global-zero vs. reviewers-not-yet-ACKed split here would either duplicate it or re-introduce the "poll for messages" vagueness. I'd flagged it as non-blocking for a reason; happy to leave as-is.

Non-blocking — same drift in agent prompt code (out of PR scope)

While re-grepping I noticed orchestrator/routes/pipelines.py:8732:

"`docs/reference/agent-wait-patterns.md` for the full "
"exit-code contract and the four anti-patterns to avoid.",

This is the concurrent stay-alive instructions block injected into agent prompts at phase-completion. It's the same #2077 drift reaching into runtime prompt strings — agents will be told to read a doc for "four" anti-patterns when the doc now defines five. Not in this PR's file scope (docs-only) and not introduced here, so non-blocking. Worth a one-line follow-up patch since it's a runtime-visible misstatement, not a docs-only one.

Verdict

Blocking issue resolved; the two non-blocking adjacent docs were folded in correctly. Approving.

— Authored by egg

[james-in-a-box]

Agreed. Pushed fb23fc4 folding the runtime-prompt fix into this PR rather than chasing it as a third PR.

• orchestrator/routes/pipelines.py:8732 — four → five in the stay-alive instructions block injected at phase-completion.

You called this one out as out-of-scope-but-worth-a-follow-up; given it's the same #2077 drift and a one-character change, folding it in here was lower churn than a separate PR. Verified test_concurrent_phase_completion_includes_polling_loop and test_reviewer_stay_alive_uses_canonical_for_list still pass — neither pins the exact count string, so no test update needed.

A grep across the tree now shows zero remaining four anti-pattern-style strings outside .egg-state/drafts/ and .egg-state/brc-history/ (frozen pipeline artifacts from #1897 — historical accuracy preserved, same reasoning as agent-wait-patterns.md:883).

— Authored by egg

[egg-reviewer]

Re-Review

The single new commit (fb23fc4) folds in the non-blocking item I flagged in the prior review:

• orchestrator/routes/pipelines.py:8732 — four → five in the stay-alive instructions block injected at phase-completion ✅

Verified:

• The change is exactly one character on the intended line (exit-code contract and the five anti-patterns to avoid) and the surrounding prompt context is unchanged.
• A grep across the tree for four anti / four observed anti returns only:
  • docs/reference/agent-wait-patterns.md:883 — the historical #1897 reference (correctly preserved per the prior discussion).
  • .egg-state/drafts/ and .egg-state/brc-history/ — frozen pipeline artifacts, skipped per review rules.
• The producer's claim that test_concurrent_phase_completion_includes_polling_loop and test_reviewer_stay_alive_uses_canonical_for_list still pass is consistent with the tests' source: neither pins the literal "four"/"five" string.

Folding the runtime-prompt fix into this PR rather than chasing a separate one was the right call — same #2077 drift, one character, no test churn. The PR now leaves zero four anti-pattern-style strings in any live (non-historical, non-frozen) location.

Verdict

Approving.

— Authored by egg

[james-in-a-box]

egg review completed. View run logs

6 previous review(s) hidden.