docs: update brc read-peer-artifact and reviewer sync docs [doc-updater]

[james-in-a-box]

Update documentation to reflect changes from 99c8122 (feat(brc): live-backed read_peer_artifact + wrapper sync-to-proposal, #3076/#3083):

• orchestrator-cli.md: brc read-peer-artifact was documented as a disk-only read with "no HTTP transport." After #3076 it merges the orchestrator's live /brc-transcript route (in-flight phase from the message store) with the on-disk .egg-state/brc-history/ files (completed phases). Updated the comment block, table entry, and explanatory paragraph accordingly.
• concurrent-execution.md: The "Reviewer Worktree Sync" section described the BRC preamble instructing reviewers to manually run git fetch && git merge before reviewing. After #3076, the consensus wrapper's sync_to_proposals bash function handles this automatically before ack/nack invocations (deterministic wrapper bash replacing prompt prose, which the event-pump discards per #3033). Updated the protocol flow step 2, the Reviewer Worktree Sync section, and the shell example block.

Triggered by: #3083

Authored-by: egg

[egg-reviewer]

Summary

This is a documentation-only PR updating two docs to reflect the implementation changes in #3076 / #3083:

• docs/reference/orchestrator-cli.md — brc read-peer-artifact is no longer disk-only; it merges the live /brc-transcript route with on-disk history.
• docs/guides/concurrent-execution.md — reviewer worktree sync is now wrapper-driven (sync_to_proposals), not prompt-driven.

I verified the doc claims against the implementation:

• orchestrator/consensus_wrapper.py:485-537 — sync_to_proposals() matches the description (hex-validated SHA extraction, fail-soft merge with abort on conflict, logging the fallback). The propose-arm skip is correctly gated at line 771-773.
• orchestrator/routes/messages.py:415-499 — the /brc-transcript route exists with the documented query params and Delphi redaction.
• sandbox/egg_agent_tools/handlers/brc.py:1028-1335 — brc_read_peer_artifact performs the dual-source merge, dedup by message id, returns live bool and conditional hint.
• sandbox/egg_agent_tools/handlers/_gateway.py:269-303 — orchestrator_request consumes EGG_ORCHESTRATOR_URL / EGG_LIFECYCLE_SECRET, confirming the claim "all five subcommands honour" those env vars.

The new prose is otherwise accurate.

Blocking

1. docs/architecture/orchestrator.md:460 still describes the old prompt-driven sync mechanism

The PR updates the "Reviewer Worktree Sync" section in docs/guides/concurrent-execution.md to describe the new wrapper-driven sync_to_proposals. But docs/architecture/orchestrator.md carries the same content (item 4 under "Worktree state synchronization") and was missed:

Agent-initiated sync (on review): During concurrent phases, each agent's worktree is frozen at the phase-start SHA. When a producer pushes commits and proposes via CONSENSUS_PROPOSE, reviewer worktrees do not automatically receive those commits. To address this, the BRC preamble (_build_brc_preamble()) instructs reviewers to sync their worktree before reviewing: git fetch origin && git merge origin/{branch} --no-edit. This prompt-level approach avoids orchestrator-side worktree manipulation while ensuring reviewers evaluate up-to-date code. See Concurrent Execution: Reviewer Worktree Sync for details.

Critically, this section even links to the section the PR rewrote. After this PR merges, the architecture doc says the BRC preamble's prompt prose is the mechanism, the cross-referenced concurrent-execution guide says the wrapper bash is the mechanism. Same topic, two contradicting authoritative sources.

The PR explicitly says it is updating reviewer-sync docs, and this is the same content in the same scope. Per the review rule on pre-existing inconsistencies in code the PR modifies, this should be fixed in the same PR.

Suggested replacement for item 4:

Agent-initiated sync (on review): During concurrent phases, each agent's worktree is frozen at the phase-start SHA. When a producer pushes commits and proposes via CONSENSUS_PROPOSE, reviewer worktrees do not automatically receive those commits. To address this, the consensus wrapper's sync_to_proposals step (#3076, orchestrator/consensus_wrapper.py:485) runs before every ack/nack invocation: it extracts each pending producer's proposal_commit_sha from the event payload, hex-validates the SHA, and attempts a git merge --no-edit of each SHA into the reviewer's worktree, aborting cleanly on conflict so the reviewer falls back to git show <sha>:<path> reads. The producer propose arm intentionally skips the sync to avoid dual-role bleed-through. See Concurrent Execution: Reviewer Worktree Sync for details.

Non-blocking

2. "replacing prompt prose" is misleading — the BRC preamble still emits SYNC

docs/guides/concurrent-execution.md:1146 says:

This is deterministic bash in the wrapper rather than prose in the spawn prompt (which the event-pump discards per #3033), so the sync is reliable under the event-pump model.

The implication is that the prompt prose was removed. But _build_brc_preamble() still emits step 3 SYNC for reviewers (orchestrator/routes/pipelines.py:306-309) telling them to run git fetch origin && git merge .... The two layers coexist: the prompt instruction is retained but is effectively belt-and-braces over the wrapper's deterministic sync.

This is a wording precision issue, not a correctness bug. Consider clarifying — for example: "This is deterministic bash in the wrapper rather than relying on the spawn prompt's SYNC instruction (which the event-pump discards between events per #3033)" — to acknowledge that both layers exist and explain why only the wrapper layer is reliable.

3. docs/reference/agent-roles.md:281 still describes the manual fetch+merge

The "Directed coordination" paragraph describes the tester reading the coder's tests off the branch after the coder proposes via git fetch origin && git merge origin/<branch> --no-edit. After #3076 this is no longer required for the tester's reviewer arm (the wrapper has already synced before ack/nack), but it is still required for the tester's producer arm where it hardens the tests (the wrapper deliberately skips sync on propose). The sentence is still technically accurate but no longer reflects the typical flow. Worth a quick note that the reviewer-arm sync is now wrapper-handled.

4. live flag semantics are slightly imprecise (pre-existing in handler docstring)

docs/reference/orchestrator-cli.md:572 and the table entry at line 583 both describe live as "true iff the live route contributed [records]". The actual implementation (sandbox/egg_agent_tools/handlers/brc.py:1016-1025) returns live=True whenever the route returned a usable response, even when the records list is empty — i.e. it reports route reachability, not whether records were contributed.

This wording is consistent with the handler's own docstring, so the PR is not introducing the imprecision. Worth a follow-up to align all three sources on "live route was reachable" rather than "live route contributed".

— Authored by egg

[james-in-a-box]

Thanks for the careful review. All four items addressed in 9f72afb.

1. docs/architecture/orchestrator.md:460 describes the old prompt-driven sync — fixed-in-PR (commit 9f72afb)
Replaced item 4 of "Worktree state synchronization" with the wrapper-driven sync_to_proposals description you suggested, keeping the cross-link to concurrent-execution.md#reviewer-worktree-sync. I added one extra sentence beyond your suggested replacement noting the BRC preamble still emits a belt-and-braces SYNC instruction — that ties this section to fix #2 below so the two docs stay consistent on the "both layers coexist" framing.

2. "replacing prompt prose" wording in concurrent-execution.md:1146 is misleading — fixed-in-PR (commit 9f72afb)
You're right: _build_brc_preamble() still emits step 3 SYNC for reviewers (orchestrator/routes/pipelines.py:306-309). Rewrote the sentence to explicitly say both layers coexist and explain that the wrapper bash is the reliable layer because the event-pump discards spawn prompts between events (#3033), not because the prose was removed.

3. docs/reference/agent-roles.md:281 still describes manual fetch+merge — fixed-in-PR (commit 9f72afb)
Updated the tester's "Directed coordination" paragraph to split the two arms: the reviewer-arm sync is now wrapper-handled (sync_to_proposals has already merged the coder's proposal_commit_sha before ack/nack), and the manual git fetch && git merge still applies on the producer-arm ORIENT step where the wrapper deliberately skips sync to avoid dual-role bleed-through.

4. live flag semantics are imprecise — fixed-in-PR (commit 9f72afb)
Defaulting to in-PR per the contract (you flagged this as "worth a follow-up" — a soft suggestion, not an explicit defer request). Aligned all three sources on "live route reachability" rather than "live route contributed records":

• orchestrator-cli.md comment block (~line 571): live is true iff the live route was reachable and returned a usable record list — empty lists still count as reachable.
• sandbox/egg_agent_tools/handlers/brc.py docstring (~line 1044): added an explicit clarification that live reports route reachability, not contribution, matching the actual return path at line 1025 (return [...], True whenever the route returned a list).
• The table entry at line 583 already reads correctly ("Records are deduped by message id and sorted by timestamp" — no contribution claim), so no edit was needed there.

make lint is green on the changed files; the three pre-existing mypy errors in shared/egg_agent/client.py are unrelated to this PR.

— Authored by egg

[egg-reviewer]

No agent-mode design concerns. This is a docs-only PR reflecting the wrapper-driven sync change in #3076. The underlying shift it documents — replacing prompt-level "git fetch && git merge" instructions with deterministic wrapper bash that runs before every ack/nack — is actually aligned with the "sandbox/infrastructure is the constraint" principle from agent-mode-design.md: prompt instructions are advisory, wrapper bash runs reliably every cycle.

— Authored by egg

[egg-reviewer]

Re-review of delta ab4cff49..9f72afba

All four items from the prior review are correctly addressed in commit 9f72afb. I verified each against the implementation.

Verification

1. docs/architecture/orchestrator.md:460 — wrapper-driven sync (fixed)
Item 4 of "Worktree state synchronization" now matches the implementation: sync_to_proposals at orchestrator/consensus_wrapper.py:485, hex-validated SHA extraction at line 503 (regex [0-9a-fA-F]{7,64}), git merge --no-edit at line 529, git merge --abort on conflict at line 532, and propose-arm skip gated at lines 770-773. The added belt-and-braces sentence ties in with fix #2 and keeps the two doc surfaces consistent.

2. docs/guides/concurrent-execution.md:1146 — "both layers coexist" framing (fixed)
Rewritten to explicitly acknowledge that _build_brc_preamble() still emits a SYNC instruction. Confirmed against orchestrator/routes/pipelines.py:12144-12145, 12757, 12794 where the preamble still emits git fetch origin && git merge origin/{branch} --no-edit for reviewers. The new wording correctly attributes reliability to the wrapper's bash because of the event-pump's spawn-prompt discard (#3033), not to prose removal.

3. docs/reference/agent-roles.md:281 — tester two-arm split (fixed)
The Directed-coordination paragraph now distinguishes the reviewer-arm (wrapper-synced via sync_to_proposals) from the producer-arm ORIENT (still uses manual git fetch origin && git merge origin/<branch> --no-edit because the wrapper skips sync on propose to avoid dual-role bleed-through). This matches the gate at consensus_wrapper.py:770-773.

4. live flag semantics (fixed)

• docs/reference/orchestrator-cli.md:571-573 now reads "true iff the live route was reachable and returned a usable record list — empty lists still count as reachable".
• sandbox/egg_agent_tools/handlers/brc.py:1046-1050 docstring adds the matching clarification ("reports route reachability … not whether the live source contributed records").

This is consistent with _fetch_live_brc_transcript (brc.py:990-1025): the only path that returns live=True is line 1025 (return [r for r in records if isinstance(r, dict)], True), reached whenever the route returned a list-typed records field — even if empty. All three failure paths (no pipeline id, GatewayError, non-list records) return False. The table entry at line 584 was already correctly free of the "contributed" claim, so no edit there.

No new issues

The delta is a tightly scoped follow-up — five files, +10/-5 lines — and introduces no new problems. The "live route was reachable and returned a usable record list" phrasing is repeated nearly verbatim across the CLI doc, the brc_read_peer_artifact docstring, and _fetch_live_brc_transcript's own docstring, which is the right outcome for a wording-precision fix.

— Authored by egg

[james-in-a-box]

egg review completed. View run logs

4 previous review(s) hidden.