docs: Update branch/slice env docs for pipeline work-ref (#2399/#2403/#2410) [doc-updater] - #2426
Conversation
Update docs to reflect three changes from a505cfa: - Pipeline tip is now at <branch>/work (e.g. egg/issue-N/work) so slice integration branches can coexist as siblings under egg/issue-N/; update branch examples in concurrent-execution.md, orchestrator.md, and slice-dag.md. - Add EGG_SLICE_ID to the concurrent-execution env injection table; per-slice agents receive this to route CONSENSUS_* signals to the correct per-slice tracker. - Add slice_id parameter example to the agent restart REST API snippet in orchestrator-cli.md.
This comment has been minimized.
This comment has been minimized.
![egg-reviewer[bot]](https://avatars.githubusercontent.com/u/8340608?s=60&v=4){kind=small}
There was a problem hiding this comment.
All three documentation claims are faithful to the implementation that landed in #2402 / #2403 / #2410.
Verified
egg/issue-N→egg/issue-N/workmirrors_ensure_pipeline_work_ref()inorchestrator/routes/pipelines.py:682-726and the BABYSIT skip at line 1454. The slice integration branches built off_slice_namespace_root(line 729) confirm the sibling layout the new prose describes.- The new
EGG_SLICE_IDrow inconcurrent-execution.mdmatcheskubernetes_spawner.py:760-761(if slice_id is not None: environment["EGG_SLICE_ID"] = slice_id) and the_PROTECTED_ENV_KEYSentry at line 104. "Absent for pipeline-level agents" is correct — there is no fallback assignment. The "BRC handlers forward this" claim lines up with_maybe_attach_slice_idinsandbox/egg_agent_tools/handlers/brc.py:26-41, which uses both the requestslice_idandEGG_SLICE_IDfrom env, and the validation againstslice-<N>matches the doc's value example. - The slice-scoped restart curl example matches
orchestrator/routes/pipelines.py:2262-2338: query param wins over body, both go throughextract_slice_id, and the docstring already documents both forms.
Non-blocking observation
docs/architecture/slice-dag.md:313 keeps the parenthetical "per-role suffix /{role}/work for babysit-pr staging" intact while updating the surrounding line. Per concurrent_executor.py:209-262 and the test_concurrent_executor_staging_branch.py assertions, the actual babysit-pr staging shape is egg/babysit-pr/{pr}/{short-sha}/{role} — there is no /work suffix on per-role staging branches. The /{role}/work shape was the pre-v6 slice-mode layout collapsed in coder v6 (commit 97de106, per the test docstring), and it was never the babysit shape. Pre-existing inaccuracy that this PR didn't introduce, but since the PR is editing this exact line and the rule applies, worth a follow-up: drop or correct that parenthetical so readers don't search the codebase for a non-existent shape.
— Authored by egg
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Reviewer noted the parenthetical on the pipeline-mode row referenced a
non-existent `/{role}/work` suffix for babysit-pr staging. The actual
shape produced by `ConcurrentPhaseExecutor.get_worktree_branch`
(orchestrator/concurrent_executor.py:257) is
`egg/babysit-pr/{pr}/{short-sha}/{role}` — no `/work` suffix. Per the
test docstring in `test_concurrent_executor_staging_branch.py`, the
`/{role}/work` shape was the pre-v6 slice-mode layout (collapsed in
97de106) and was never the babysit shape.
Authored-by: egg
Review ResponseBoth bot reviews approved. One non-blocking observation from the Disposition
— Authored by egg |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
No agent-mode design concerns. The delta since my prior review (commit bb47b84) is a one-line correction to the babysit-pr staging branch format in docs/architecture/slice-dag.md — pure documentation accuracy fix with no agent prompt, tool surface, or behavior changes.
— Authored by egg
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Re-review at bb47b84 — the prior non-blocking observation has been addressed.
Verified fix
docs/architecture/slice-dag.md:313 now reads babysit-pr staging uses egg/babysit-pr/{pr}/{short-sha}/{role} — this matches the source of truth:
orchestrator/concurrent_executor.py:257returnsf"egg/babysit-pr/{pr_number}/{short_sha}/{role.value}".- The docstring on
get_worktree_branch(concurrent_executor.py:225) names the same shape. orchestrator/tests/test_slice_branch_naming.py:94corroborates the commit message: the/{role}/worksuffix the old text referenced was the pre-v6 slice-mode layout collapsed by coder v6 (97de106), not a babysit shape.
No new issues in the delta. The other claims in the original PR (<branch>/work tip, EGG_SLICE_ID env injection, slice-scoped restart curl example) were already verified against routes/pipelines.py, kubernetes_spawner.py, and sandbox/egg_agent_tools/handlers/brc.py in the prior review and are unchanged in this commit.
— Authored by egg
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
…i.md
Kept HEAD's babysit-pr branch shape correction (egg/babysit-pr/{pr}/{short-sha}/{role})
in slice-dag.md per bb47b84 — verified against orchestrator/concurrent_executor.py:257.
Took main's wording for the slice_id query-param comment in orchestrator-cli.md
(equivalent meaning, established by #2425).
Conflict Resolution SummaryResolved merge conflicts with
Please review: The — Authored by egg |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
No agent-mode design concerns. The delta since my prior review at bb47b84 is the merge from main resolving conflicts with #2425 — the surviving changes are confined to docs/architecture/orchestrator.md (new live-pod-guard bullet), docs/architecture/slice-dag.md (slice integration prose + out-of-scope section), and docs/reference/orchestrator-cli.md (one wording tweak on the slice-scoped restart example). All documentation; no agent prompt, tool-surface, or behavior changes to evaluate.
— Authored by egg
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Re-review at 1f18a95 — the delta since my prior review at bb47b84 is entirely a merge-from-main resolving conflicts with parallel doc PRs #2423 (live-pod-guard) and #2425 (slice-dag / orchestrator-cli rewording). The conflict resolution is the only PR-authored work in the delta, and it's the right call.
Verified resolutions
-
docs/architecture/orchestrator.md:47— auto-merged; the new live-pod-guard bullet matchesorchestrator/startup_reconciliation.py:34-216(label keyegg.pipeline.idviaLABEL_PIPELINE_ID, fail-safe-on-error skip leaving the pipeline RUNNING, zero-live-pods fallback to stale-record detection). Accurate. -
docs/architecture/slice-dag.md:313— the merge correctly kept HEAD'segg/babysit-pr/{pr}/{short-sha}/{role}shape rather than main's/{role}/worksuffix.orchestrator/concurrent_executor.py:257returnsf"egg/babysit-pr/{pr_number}/{short_sha}/{role.value}"— main's#2425wording was wrong, this PR's correction is right. Taking the upstream wording here would have re-introduced the inaccuracy I flagged in my previous review. -
docs/architecture/slice-dag.md:333-374— the new prose aboutEGG_SLICE_IDbeing read by BRC handlers and attached to everyCONSENSUS_*payload matchessandbox/egg_agent_tools/handlers/brc.py:_maybe_attach_slice_id(line 26) readsreq["slice_id"] or get_slice_id()and is invoked from every BRC verb (lines 444, 529, 586, 634, 754). The composite-key claim —peer_consensus._tracker_keyreturning{pipeline_id}/{slice_id}— matchesorchestrator/peer_consensus.py:1834-1856. Accurate. -
docs/architecture/slice-dag.md:685-695— out-of-scope section correctly removesrestart_agentwithslice_id(landed in #2399/#2410) while keepingrestart_slice,get_slice_status,list_slicesdeferred to #2199. The new "Note:" line accurately describes the current REST endpoint shape. -
docs/reference/orchestrator-cli.md:211— wording-only change to align with #2425's phrasing; semantically equivalent to the prior text. No accuracy concern.
No new issues in the delta. Documentation-only, all claims still backed by source.
— Authored by egg
This comment has been minimized.
This comment has been minimized.
| egg is addressing review feedback... |
Review ResponseTwo bot reviews at the current head ( Disposition
The earlier non-blocking observation about the — Authored by egg |
|
egg feedback addressed. View run logs 10 previous review(s) hidden. |
1 participant
docs: Update branch naming and slice env vars for pipeline namespace changes
Update documentation to reflect changes from a505cfa (Fix #2399: push pipeline tip to /work so slice refs coexist):
egg/issue-N→egg/issue-N/work): The orchestrator now pushes the pipeline tip one level deeper so slice integration branches (egg/issue-N/slice-M) can coexist as siblings without a git "directory file conflict". Updated examples inconcurrent-execution.md(branch model intro + per-agent worktree section),orchestrator.md(shared branch bullet), andslice-dag.md(mode table).EGG_SLICE_IDenv var: Per-slice agents receive this new env var so BRC handlers routeCONSENSUS_*signals to the correct per-slice tracker. Added to the environment injection table inconcurrent-execution.md.slice_idquery parameter. Added an example toorchestrator-cli.md.Triggered by: #2402
Authored-by: egg