Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/architecture/orchestrator.md
Original file line number Diff line number Diff line change
Expand Up @@ -457,7 +457,7 @@ The orchestrator calls `ensure_egg_state_dirs()` before spawning containers to c

See `orchestrator/routes/pipelines.py` for implementation details.

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 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](../guides/concurrent-execution.md#reviewer-worktree-sync) for details.
4. **Wrapper-driven 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. The BRC preamble (`_build_brc_preamble()`) still emits a belt-and-braces SYNC instruction in the spawn prompt, but the event-pump discards spawn prompts between events (#3033), so the wrapper bash is the reliable sync layer. See [Concurrent Execution: Reviewer Worktree Sync](../guides/concurrent-execution.md#reviewer-worktree-sync) for details.

This architecture ensures the orchestrator reads artifacts from the correct isolated workspace rather than the main repository, preventing cross-contamination between pipelines.

Expand Down
18 changes: 8 additions & 10 deletions docs/guides/concurrent-execution.md
Original file line number Diff line number Diff line change
Expand Up @@ -360,7 +360,7 @@ Each agent tracks two state machines (producer and reviewer) independently:
### BRC Protocol Flow

1. **Propose**: Producer completes work, commits and pushes to the remote branch, then sends `CONSENSUS_PROPOSE` with a summary, artifact list, and the pushed commit SHA (`--commit-sha`). The orchestrator rejects proposals whose commit SHA is confirmed absent from the branch (verification failures due to network errors are non-blocking).
2. **Review**: Assigned reviewers discover proposals via polling. Before a reviewer has submitted their own evaluation, the Delphi filter delivers a **redacted** version of the `CONSENSUS_PROPOSE` message (`body` cleared, `metadata.payload` stripped except `version` and `commit_sha`, `metadata.delphi_redacted=True`). This notifies the reviewer that a proposal exists without exposing the producer's self-assessment. Reviewers must **sync their worktree** before reviewing (`git fetch origin && git merge origin/{branch} --no-edit`) to pull in the producer's pushed commits. After reviewing the git artifacts and submitting `CONSENSUS_ACK` or `CONSENSUS_NACK`, subsequent polls return the full unredacted message.
2. **Review**: Assigned reviewers discover proposals via polling. Before a reviewer has submitted their own evaluation, the Delphi filter delivers a **redacted** version of the `CONSENSUS_PROPOSE` message (`body` cleared, `metadata.payload` stripped except `version` and `commit_sha`, `metadata.delphi_redacted=True`). This notifies the reviewer that a proposal exists without exposing the producer's self-assessment. The consensus wrapper automatically merges the producer's proposed commit into the reviewer's worktree (`sync_to_proposals`, #3076) before the `ack`/`nack` invocation, so reviewers see the latest code without a manual fetch+merge step. After reviewing the git artifacts and submitting `CONSENSUS_ACK` or `CONSENSUS_NACK`, subsequent polls return the full unredacted message.
3. **Converge**: When all critical reviewers ACK, the producer sends `CONSENSUS_CONFIRMED`. When all agents are confirmed, the phase advances. Reviewers also call `CONSENSUS_CONFIRMED` after completing all reviews; the protocol enforces multiple guards in both the producer and reviewer confirmation paths (see [Action Guards](#action-guards) and [Deadlock Prevention Guards](#deadlock-prevention-guards) below).
4. **Re-propose**: If a NACK is received, the producer addresses the feedback and re-proposes (with `changed_artifacts` to scope re-evaluation). Flip-flop cycles are capped at `max_flip_flops` (default: 3). If any reviewer had already confirmed on a prior proposal version, they automatically receive a `CONSENSUS_RE_REVIEW` message and are un-confirmed so they re-enter the review loop — preventing a deadlock where a stale-confirmed reviewer can never see the new proposal.

Expand Down Expand Up @@ -633,8 +633,10 @@ egg-orch consensus propose --push \
# --files-changed, --tests-run, --tasks are optional but recommended for traceability.
# Argv `--summary "…"` still works but writes a deprecation warning to stderr — see #2741.

# Reviewer: sync worktree before reviewing (fetch producer's commits)
git fetch origin && git merge origin/egg/feature-x --no-edit
# Reviewer worktree sync is handled automatically by the consensus wrapper's
# sync_to_proposals step (#3076) before ack/nack invocations. Manual fetch+merge
# is only needed in non-event-pump contexts or for debugging.
# git fetch origin && git merge origin/egg/feature-x --no-edit

# Reviewer: ACK after reviewing
# --reason is required and must be ≥50 chars. Your --reason IS your review — include full analysis.
Expand Down Expand Up @@ -1139,15 +1141,11 @@ The documenter's docs/markdown scope is mutually exclusive from the others, so i

### Reviewer Worktree Sync

Per-agent worktrees are created at phase start from the team's branch — the pipeline branch for refine/plan, the slice integration branch for each implement slice. When a producer pushes commits and proposes, the reviewer's worktree does not automatically have those commits. To address this, the BRC preamble instructs reviewers to sync their worktree before reviewing:
Per-agent worktrees are created at phase start from the team's branch — the pipeline branch for refine/plan, the slice integration branch for each implement slice. When a producer pushes commits and proposes, the reviewer's worktree does not automatically have those commits.

```bash
git fetch origin && git merge origin/{branch} --no-edit
```

This explicit fetch+merge step ensures reviewers see the latest code (including the producer's pushed commits) when they start reviewing. Without it, reviewers would evaluate stale code and miss issues that appear in the actual changeset.
The consensus wrapper's `sync_to_proposals` step (#3076) handles this automatically: before every `ack` or `nack` invocation the wrapper extracts each pending producer's `proposal_commit_sha` from the event payload, validates the SHA as hex, and attempts a `git merge --no-edit` of each SHA into the reviewer's worktree. The BRC preamble (`_build_brc_preamble()`) still emits a SYNC instruction in the spawn prompt for reviewers — both layers coexist — but the event-pump discards spawn prompts between events (#3033), so the wrapper bash is the layer that actually runs every cycle. If the merge fails (conflict or unresolvable SHA), the wrapper aborts it and logs the fallback — the reviewer can still read the producer's artifacts via `git show <proposal_commit_sha>:<path>` from the shared object store.

The same sync instruction is included for dual-role agents (e.g., `tester`) in their producer ORIENT step, so they also have up-to-date code before beginning work.
The producer `propose` arm intentionally skips the sync: a producer's own commits are already on HEAD, and merging peer proposal commits onto a producer turn would cause dual-role bleed-through.

**Reviewer diff command:** Reviewers use `git diff origin/{base_branch}...HEAD` (three-dot merge-base syntax) to see the full changeset against the base branch, rather than an arbitrary truncated window. The `base_branch` is resolved from `pipeline.base_branch` or the repository's default branch. This matches the context available to PR review bots, which see the complete PR diff.

Expand Down
2 changes: 1 addition & 1 deletion docs/reference/agent-roles.md
Original file line number Diff line number Diff line change
Expand Up @@ -278,7 +278,7 @@ tests, and its mandate is two-fold: **(1) comprehensive regression coverage**
- Test file commits on the worktree branch
- `.egg-state/agent-outputs/{identifier}-tester-output.json` — Handoff data (includes lint/type-check results and gaps found)

**Directed coordination**: The coder authors and pushes its own tests, so the tester no longer receives a test-file HANDOFF from the coder — it reads the coder's tests off the branch after the coder proposes (`git fetch origin && git merge origin/<branch> --no-edit`) and hardens them in place. The tester still *sends* `HANDOFF` messages to the coder for things outside the tester's write scope — when a CI fix or `.github/` change is uncovered during testing (e.g. a workflow needs a new step to run a regression), the tester describes the required end-state in the HANDOFF body and the coder stages it under `.github-staging/`. See [Directed Coordination](../guides/concurrent-execution.md#directed-coordination).
**Directed coordination**: The coder authors and pushes its own tests, so the tester no longer receives a test-file HANDOFF from the coder — it reads the coder's tests off the branch after the coder proposes and hardens them in place. On the tester's *reviewer* arm the consensus wrapper's `sync_to_proposals` step (#3076) has already merged the coder's `proposal_commit_sha` into the worktree before `ack`/`nack`, so no manual fetch+merge is needed. On the tester's *producer* arm (ORIENT before its own propose) the wrapper deliberately skips sync to avoid dual-role bleed-through, so the manual `git fetch origin && git merge origin/<branch> --no-edit` still applies there to pick up any post-phase-start commits before hardening tests. The tester still *sends* `HANDOFF` messages to the coder for things outside the tester's write scope — when a CI fix or `.github/` change is uncovered during testing (e.g. a workflow needs a new step to run a regression), the tester describes the required end-state in the HANDOFF body and the coder stages it under `.github-staging/`. See [Directed Coordination](../guides/concurrent-execution.md#directed-coordination).

**Prompt context**: Summarized background, coder handoff data, task list.

Expand Down
25 changes: 15 additions & 10 deletions docs/reference/orchestrator-cli.md
Original file line number Diff line number Diff line change
Expand Up @@ -558,14 +558,19 @@ egg-orch brc resolve-obligation \
--commit-sha $(git rev-parse HEAD) \
--note-file .egg-state/agent-outputs/obligation-resolved.md

# brc read-peer-artifact — paginated read over .egg-state/brc-history/<id>-<phase>.json
# files. Used by reviewers (and the prompt composer in slice-3) to reconstruct
# peer-to-peer review history without hand-grepping JSON off disk. --phase is required;
# --peer-role / --producer-role narrows by sender; --message-type can be repeated to
# filter by CONSENSUS_* / STATUS / HANDOFF / etc. --limit defaults to 50, max 500;
# --cursor is the opaque token returned by the previous call. Slice-scoped reads
# (phase=implement + EGG_SLICE_ID set) merge in the per-pipeline unattributed file by
# default; pass --no-include-unattributed to read only the per-slice file.
# brc read-peer-artifact — dual-source BRC transcript read: the orchestrator's
# live /brc-transcript route (in-flight phase, message store) merged with the
# on-disk .egg-state/brc-history/<id>-<phase>.json files (completed phases).
# Used by reviewers (and the prompt composer in slice-3) to reconstruct peer-to-
# peer review history without hand-grepping JSON off disk. --phase is required;
# --peer-role / --producer-role narrows by sender; --message-type can be repeated
# to filter by CONSENSUS_* / STATUS / HANDOFF / etc. --limit defaults to 50,
# max 500; --cursor is the opaque token returned by the previous call. Slice-
# scoped reads (phase=implement + EGG_SLICE_ID set) merge in the per-pipeline
# unattributed file by default; pass --no-include-unattributed to read only the
# per-slice file. The response includes a `live` bool (true iff the live route
# was reachable and returned a usable record list — empty lists still count as
# reachable) and an optional `hint` when both sources are empty.
egg-orch brc read-peer-artifact --phase implement --peer-role coder \
--message-type CONSENSUS_PROPOSE --message-type CONSENSUS_ACK --limit 100
```
Expand All @@ -576,9 +581,9 @@ egg-orch brc read-peer-artifact --phase implement --peer-role coder \
| `brc get-state` | `mcp__brc__get_state` | Full BRC consensus state. `--verbose` includes the full pipeline-status payload. |
| `brc list-blocking` | `mcp__brc__list_blocking` | Roles currently blocking consensus. Newline-delimited by default; `--json` returns the array. |
| `brc resolve-obligation` | `mcp__brc__resolve_obligation` | Mark a reviewer's conditional-ACK obligation satisfied in-cycle (#2338). |
| `brc read-peer-artifact` | `mcp__brc__read_peer_artifact` | Paginated read over the local `.egg-state/brc-history/<id>-<phase>.json` log. |
| `brc read-peer-artifact` | `mcp__brc__read_peer_artifact` | Dual-source BRC transcript read (#3076): merges the orchestrator's live `/brc-transcript` route (in-flight phase) with the on-disk `.egg-state/brc-history/<id>-<phase>.json` files (completed phases). Records are deduped by message id and sorted by timestamp. |

Four of the five subcommands — `next-action`, `get-state`, `list-blocking`, `resolve-obligation` — honour `EGG_ORCHESTRATOR_URL` and `EGG_LIFECYCLE_SECRET` and run against the gateway routes the MCP tools use. `brc read-peer-artifact` is the odd one out: it reads `.egg-state/brc-history/<identifier>-<phase>.json` files from local disk, with no HTTP transport. It consumes `EGG_PIPELINE_ID` / `EGG_ISSUE_NUMBER` (to resolve the identifier) and `EGG_SLICE_ID` (to pick the per-slice partition for `phase == "implement"`) directly from the environment; `EGG_ORCHESTRATOR_URL` and `EGG_LIFECYCLE_SECRET` do not apply, so missing-secret failures against `read-peer-artifact` are misdiagnosed if you trace them through the HTTP layer. The `brc` surface is additive to the existing `consensus` surface — every prior subcommand under `consensus` keeps working unchanged; the split only reflects that `consensus` is verb-by-state-change (proposes / acks / withdraws) and `brc` is verb-by-read-or-derive (derive-next, list-blocking, read history, resolve obligation).
All five subcommands honour `EGG_ORCHESTRATOR_URL` and `EGG_LIFECYCLE_SECRET` for their HTTP requests. `brc read-peer-artifact` additionally consumes `EGG_PIPELINE_ID` / `EGG_ISSUE_NUMBER` (to resolve the identifier) and `EGG_SLICE_ID` (to pick the per-slice partition for `phase == "implement"`). The live source (`/api/v1/pipelines/{id}/brc-transcript`) serves the in-flight phase's records from the message store, which is cleared on phase transitions — so for completed phases the on-disk files are the authoritative source. A live-route failure (older orchestrator, network error) degrades gracefully to disk-only and sets `live=false` in the response. The `brc` surface is additive to the existing `consensus` surface — every prior subcommand under `consensus` keeps working unchanged; the split only reflects that `consensus` is verb-by-state-change (proposes / acks / withdraws) and `brc` is verb-by-read-or-derive (derive-next, list-blocking, read history, resolve obligation).

## Context PR Surfaces ([#2777](https://github.com/jwbron/egg/issues/2777))

Expand Down
6 changes: 5 additions & 1 deletion sandbox/egg_agent_tools/handlers/brc.py
Original file line number Diff line number Diff line change
Expand Up @@ -1043,7 +1043,11 @@ def brc_read_peer_artifact(req: dict[str, Any]) -> dict[str, Any]:

Records are deduplicated by message ``id`` and sorted by timestamp.
A live-route failure degrades gracefully to disk-only with a
``hint`` saying the live source was unavailable.
``hint`` saying the live source was unavailable. The response's
``live`` flag reports route *reachability* (i.e. the live endpoint
returned a usable record list — empty lists still count as
reachable), not whether the live source contributed records to the
merged output.

No CLI counterpart (decision-8).

Expand Down
Loading