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
16 changes: 16 additions & 0 deletions docs/architecture/sdlc-pipeline.md
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,22 @@ the agent reads to recover the task:
> links are branch-qualified absolute URLs
> (`https://github.com/<repo>/blob/<work-branch>/...`) because GitHub
> resolves relative links in PR bodies against the default branch.
>
> **Body reflow + stack cross-links (#3122).** Prose fields
> (`pr.description` / `pr.test_plan` / `pr.manual_steps`, and the
> per-slice `goal` lead on slice PRs) are treated as markdown source,
> not preformatted text: `egg_contracts.markdown.unwrap_soft_breaks`
> joins the ~75-char YAML block-scalar hard wraps back into paragraphs
> (lists, headings, tables, code fences, and explicit hard breaks are
> preserved) so GitHub renders prose instead of a choppy column. After
> each slice PR opens, the run loop records its number/URL on the
> contract slice (`Slice.pr_number` / `Slice.pr_url`) and re-composes +
> pushes the context-PR body via the gateway's `gh pr edit` REST route,
> so the slice table gains a `— #N` link to each slice PR as the stack
> materialises. Pipeline-generated PR bodies are **machine-owned**: the
> refresh fully regenerates the body and clobbers manual edits. The
> refresh is best-effort/cosmetic — failures log and never fail the
> slice.

### Schema v1.1 → v1.2 migration note (#2777)

Expand Down
13 changes: 12 additions & 1 deletion docs/architecture/slice-dag.md
Original file line number Diff line number Diff line change
Expand Up @@ -539,7 +539,18 @@ every slice PR is purely slice-scoped.
criteria behind a `<details>` fold) → `## Stack` (position, base
PR, base branch). Program-level test plan, manual steps and
pre-merge obligations live on the up-front context PR (#2777), not
on any slice PR.
on any slice PR. Prose fields (`goal`, inlined program narrative)
have their YAML block-scalar hard wraps joined back into paragraphs
before rendering (`unwrap_soft_breaks`, #3122).
- **Reverse linkage (#3122).** After a slice PR opens, the run loop
parses the PR number from the returned URL, persists it on the
contract slice (`Slice.pr_number` / `Slice.pr_url` — also on the
idempotent already-open path, so resumes recover the linkage), and
refreshes the machine-owned context-PR body so its slice table links
the new PR (`— #N`). The refresh routes through
`GatewayClient.update_pr_body` (synthetic session →
`/api/v1/gh/pr/edit`, same seam as `rebase_onto`'s base retarget)
and is strictly best-effort: failures log and never fail the slice.
- **No `context_pr_number` — should not occur under #2777.** Because
the context PR is opened up-front, hard-required and idempotent at
the plan→implement boundary, every slice PR sees a populated
Expand Down
3 changes: 2 additions & 1 deletion docs/guides/sdlc-pipeline.md
Original file line number Diff line number Diff line change
Expand Up @@ -871,7 +871,7 @@ Template sections:
- [TASK-1-2] Add role validation — Acceptance: Unauthorized mutations rejected
```

**PR Metadata**: The plan should include a `pr:` section in the YAML appendix with a title, description, test plan, and manual steps for the pull request. The `test_plan` field is required — describe both automated test coverage and manual verification steps. The `manual_steps` field lists any pre- or post-merge actions (migrations, config changes, deployments); use an empty string if none. The context PR's body renders all of these: the description verbatim, then `## Test Plan` / `## Manual Steps` sections, then a generated `## Pipeline context` footer linking the originating issue, the slice table, and the analysis/plan drafts and BRC transcripts committed on the work branch (#3115). Each slice's `goal` field is likewise rendered verbatim as that slice's PR-body lead paragraph, so write it as a reviewer-facing summary. If not provided, the orchestrator falls back to the issue title (or a generic stub) and opens the PR as a **draft** with a warning banner in the body that lists any parse errors from the plan draft, so reviewers cannot silently merge a PR whose planner metadata is missing (see #1975).
**PR Metadata**: The plan should include a `pr:` section in the YAML appendix with a title, description, test plan, and manual steps for the pull request. The `test_plan` field is required — describe both automated test coverage and manual verification steps. The `manual_steps` field lists any pre- or post-merge actions (migrations, config changes, deployments); use an empty string if none. The context PR's body renders all of these: the description, then `## Test Plan` / `## Manual Steps` sections, then a generated `## Pipeline context` footer linking the originating issue, the slice table, and the analysis/plan drafts and BRC transcripts committed on the work branch (#3115). Each slice's `goal` field is likewise rendered as that slice's PR-body lead paragraph, so write it as a reviewer-facing summary. All of these prose fields are treated as markdown source, not preformatted text: single newlines inside paragraphs (YAML block-scalar hard wraps) are joined back into prose before rendering, while lists, headings, code fences, and blank-line paragraph breaks are preserved (#3122) — so wrap freely when authoring. If not provided, the orchestrator falls back to the issue title (or a generic stub) and opens the PR as a **draft** with a warning banner in the body that lists any parse errors from the plan draft, so reviewers cannot silently merge a PR whose planner metadata is missing (see #1975).

### Phase Completion Comments

Expand Down Expand Up @@ -991,6 +991,7 @@ Default checks for each phase are defined in `shared/egg_contracts/phase_default
**Context PR (opened at implement-phase start, #2777):**
- The terminal "PR phase" was **deleted** as a separate pipeline stage; `IMPLEMENT` is now terminal. The context PR (`egg/<pipeline_id>/work → main`) is opened by the orchestrator at the plan→implement boundary via `_open_context_pr_at_implement_start` — no separate phase or agent is spawned for this. The open is hard-required and idempotent (`GatewayClient.lookup_open_pr` pre-flight with a server-side `gh pr list --head … --base … --limit 1` filter). Per-slice PRs are opened inline by `create_slice_pr`, which uses the same `GatewayClient.lookup_open_pr` primitive (#2777 cq-8 / #2934 unified both PR-idempotency paths onto it).
- The PR title is sourced from `contract.pr.title` (populated by the plan agent); the body is composed by `_compose_context_pr_body` (#3115) from `contract.pr.description`, rendered `## Test Plan` / `## Manual Steps` sections, and a generated `## Pipeline context` footer (pipeline id, originating issue, slice table, branch-qualified absolute-URL links to the refine analysis draft / plan draft, and the per-phase BRC transcripts committed on the work branch). The `## Pipeline context` header is suppressed when only the bare pipeline-id line would be emitted. The BRC-history transcript line is rendered inline inside the footer (the helper calls `_build_brc_history_link_line` while building the footer body) rather than appended as a separate paragraph; see [Concurrent Execution — BRC History Link in PR Body](concurrent-execution.md#brc-history-link-in-pr-body) for the link-line shape.
- **Stack cross-links + machine-owned body (#3122)**: after each slice PR opens, the run loop persists its number/URL on the contract slice (`Slice.pr_number` / `Slice.pr_url`) and re-composes + pushes the context-PR body through the gateway (`GatewayClient.update_pr_body` → `/api/v1/gh/pr/edit`), so the footer's slice table links each slice PR (`— #N`) as the stack materialises. The context-PR body is machine-owned: each refresh fully regenerates it, clobbering manual edits. The refresh is best-effort — a failure logs a warning and never fails the slice.
- **Draft preservation**: Pipeline-specific draft files (`.egg-state/drafts/{id}-analysis.md`, `.egg-state/drafts/{id}-plan.md`) are **preserved** on the PR branch as artifacts of the pipeline's reasoning. Reviewers can compare the planned approach against the shipped code, and post-hoc debugging has the analysis and plan available as a baseline (#1713).
- If the up-front context-PR open fails (after the idempotent pre-flight returns no existing PR and `gh pr create` itself fails), the pipeline is marked **FAILED** immediately — there is no terminal back-stop because the open is hard-required up-front.

Expand Down
137 changes: 132 additions & 5 deletions orchestrator/gateway_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import subprocess
import sys
import time
import uuid
from collections.abc import Callable
from dataclasses import dataclass
from datetime import datetime, timedelta
Expand Down Expand Up @@ -50,6 +51,15 @@ def get_logger(name: str, **kwargs) -> logging.Logger: # type: ignore[misc]
GATEWAY_CONTAINER_NAME = "egg-gateway"
GATEWAY_PORT = 9848 # noqa: EGG002

try:
from egg_contracts.markdown import unwrap_soft_breaks
except ImportError:
# Fallback: render prose verbatim when egg_contracts is unavailable
# (matches the egg_logging/egg_config degradation above).
def unwrap_soft_breaks(text: str | None) -> str: # type: ignore[misc]
return text or ""


logger = get_logger("orchestrator.gateway_client")

T = TypeVar("T")
Expand Down Expand Up @@ -1830,9 +1840,12 @@ def create_slice_pr(
# description just below (the blurb would duplicate its first
# sentence).
inline_program_narrative = has_program_title and not has_base_pr
lead = (slice_goal or "").strip()
# Soft-break unwrapping (#3122): the goal / description reach us
# as YAML block scalars hard-wrapped at ~75 chars, and GitHub
# renders every newline in a PR body as a line break.
lead = unwrap_soft_breaks(slice_goal).strip()
if not lead and program_description and not inline_program_narrative:
lead = _first_sentence(program_description)
lead = _first_sentence(unwrap_soft_breaks(program_description))
if lead:
body_lines.append(lead)
body_lines.append("")
Expand All @@ -1851,7 +1864,7 @@ def create_slice_pr(
# ``/work``. The stack is structurally unmergeable in this
# state — fixing the body here is a UX backstop, not a fix.
if program_description and program_description.strip():
body_lines.append(program_description.strip())
body_lines.append(unwrap_soft_breaks(program_description).strip())
body_lines.append("")

_append_diff_summary_section(body_lines, diffstat, commit_subjects)
Expand All @@ -1861,12 +1874,12 @@ def create_slice_pr(
if program_test_plan and program_test_plan.strip():
body_lines.append("## Test Plan")
body_lines.append("")
body_lines.append(program_test_plan.strip())
body_lines.append(unwrap_soft_breaks(program_test_plan).strip())
body_lines.append("")
if program_manual_steps and program_manual_steps.strip():
body_lines.append("## Manual Steps")
body_lines.append("")
body_lines.append(program_manual_steps.strip())
body_lines.append(unwrap_soft_breaks(program_manual_steps).strip())
body_lines.append("")

# ``## Stack`` block — parent PR + base PR + position. Replaces
Expand Down Expand Up @@ -1926,6 +1939,120 @@ def create_slice_pr(
draft=draft,
)

def update_pr_body(
self,
pipeline_id: str,
repo: str,
*,
pr_number: int,
body: str,
issue_number: int | None = None,
agent_role: str | None = None,
mode: Literal["public", "private"] = "public",
) -> bool:
"""Replace an existing PR's body via the gateway (#3122).

Routes through the per-agent ``/api/v1/gh/pr/edit`` endpoint
(``gh api repos/<repo>/pulls/<n> -X PATCH -f body=...``) under a
synthetic phase-less session, exactly like :meth:`create_pr` /
:meth:`rebase_onto` — no new privileged orchestrator endpoint.
The gateway's PR-ownership policy still applies, which is the
desired bound: the orchestrator only rewrites PRs the egg bot
user authored.

Sole production caller is the run loop's context-PR refresh:
after a slice PR opens, the context PR body is recomposed with a
link to it. Pipeline-generated PR bodies are machine-owned —
each call fully replaces the body, clobbering manual edits.

Returns ``True`` on success, ``False`` on any failure. Unlike
:meth:`create_pr` this method does NOT propagate errors: a body
refresh is cosmetic, and no caller should fail a slice over it.
"""
if (
not repo
or pr_number is None
or isinstance(pr_number, bool)
or not isinstance(pr_number, int)
or pr_number < 1
):
logger.warning(
"update_pr_body: invalid repo/pr_number",
pipeline_id=pipeline_id,
repo=repo,
pr_number=pr_number,
)
return False

# The gateway's ``gh pr edit`` rejects an empty payload (#3431
# in gateway/gateway.py) — short-circuit before burning a
# synthetic-session create+delete round-trip on a guaranteed
# 400.
if not body:
logger.warning(
"update_pr_body: empty body",
pipeline_id=pipeline_id,
repo=repo,
pr_number=pr_number,
)
return False

# Suffix the container id with a short random tag so two
# concurrent refreshes for the same pipeline (two slices in
# the same wave finishing within ms of each other) don't share
# a session-table key in the gateway. Matches the
# per-slice-id uniqueness create_pr / rebase_onto already get
# for free.
temp_container_id = f"{pipeline_id}-pr-body-update-{uuid.uuid4().hex[:8]}"
session_token: str | None = None
try:
session = self.register_session(
container_id=temp_container_id,
container_ip=self.self_ip,
mode=mode,
pipeline_id=pipeline_id,
repos=[repo],
issue_number=issue_number,
agent_role=agent_role,
synthetic=True,
)
session_token = session.session_token

self._make_request(
"/api/v1/gh/pr/edit",
method="POST",
data={
"repo": repo,
"pr_number": int(pr_number),
"body": body,
},
bearer_token=session_token,
)
logger.info(
"Updated PR body via gateway",
pipeline_id=pipeline_id,
repo=repo,
pr_number=pr_number,
)
return True
except Exception as exc: # noqa: BLE001
# Session registration + single gh-pr-edit HTTP call.
# Catches GatewayError and OSError (DNS / socket).
logger.warning(
"update_pr_body: gateway request failed",
pipeline_id=pipeline_id,
repo=repo,
pr_number=pr_number,
error=str(exc),
)
return False
finally:
if session_token:
try:
self.delete_session(session_token)
except Exception:
pass

def rebase_onto(
self,
pipeline_id: str,
Expand Down
Loading
Loading