Skip to content
Merged
23 changes: 22 additions & 1 deletion docs/architecture/sdlc-pipeline.md
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ The contract is a JSON document tracking the complete state of an issue through

```json
{
"schemaVersion": "1.0",
"schemaVersion": "1.1",
"issue": { "number": 133, "title": "...", "url": "..." },
"current_phase": "implement",
"slices": [{
Expand Down Expand Up @@ -116,6 +116,27 @@ The contract is a JSON document tracking the complete state of an issue through
> existing imports. See [Slice-DAG Implement Phase](slice-dag.md) for
> the full design.

> **Schema 1.1 (#2548)**: `schemaVersion` was bumped from `1.0` to `1.1`
> to track the addition of four optional `pr.context_*` fields on
> `PRMetadata` (`context_title`, `context_description`, `context_branch`,
> `context_pr_number`) used by the dedicated context-PR mechanism. The
> bump is purely additive — pre-1.1 contracts load transparently via a
> Pydantic `model_validator(mode="after")` migration that stamps
> `schemaVersion = "1.1"` on every load when the on-disk value is exactly
> `"1.0"`; the migration is silent (no audit-log entry) and idempotent.
> `context_title` / `context_description` are planner-emitted optional
> framing for the strategic-plan PR; `context_branch` /
> `context_pr_number` are populated by the orchestrator after the context
> branch is created and the context PR is opened.
>
> **As of slice-1 (#2548 part 1)**, only the schema fields and the
> planner-prompt advertisement are wired. The orchestrator
> branch-creation and PR-opening hooks land in #2548 slices 3-4 — until
> those slices merge, the four `pr.context_*` fields are
> forward-compatibly inert: planners may emit `context_title` /
> `context_description` and the values flow into `PRMetadata`, but
> nothing acts on them yet.

## HITL (Human-in-the-Loop) Mechanism

For detailed HITL workflow documentation, see [HITL Decisions](../hitl-decisions.md).
Expand Down
9 changes: 8 additions & 1 deletion docs/guides/sdlc-pipeline.md
Original file line number Diff line number Diff line change
Expand Up @@ -444,7 +444,7 @@ The local orchestrator handles concurrent contract updates through `orchestrator

```json
{
"schemaVersion": "1.0",
"schemaVersion": "1.1",
"issue": {
"number": 123,
"title": "Add feature X",
Expand Down Expand Up @@ -483,6 +483,13 @@ The local orchestrator handles concurrent contract updates through `orchestrator
}
```

> **Schema 1.1 (#2548)**: The default `schemaVersion` is now `"1.1"`, which
> additively introduces four optional `pr.context_*` fields
> (`context_title`, `context_description`, `context_branch`,
> `context_pr_number`). Pre-1.1 contract JSON loads cleanly — a Pydantic
> `model_validator` silently promotes `"1.0"` to `"1.1"` on load and the
> bumped value is persisted on the next save.

### Role-Based Field Ownership

The `shared/egg_contracts/roles.py` module defines field ownership:
Expand Down
25 changes: 25 additions & 0 deletions docs/templates/plan.md
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,14 @@ pr:
manual_steps: |
Pre-merge: [any required steps before merging, e.g. migrations, config changes]
Post-merge: [any required steps after merging, e.g. deployments]
# Optional context-PR framing (#2548); omit to reuse pr.title / pr.description.
# context_title: |-
# Strategic plan for #<issue> — refine/plan analysis + BRC history
# context_description: |-
# Carries the refine analysis, the plan, the BRC consensus
# history that approved each, and the agent transcripts —
# so reviewers approaching the slice stack can see the strategic
# narrative on a PR that targets the configured base branch.
phases:
- id: 1
name: |-
Expand Down Expand Up @@ -125,6 +133,23 @@ phases:
> the task's files — see [Agent Roles Reference](../reference/agent-roles.md#role-aware-task-assignment)
> for the file-to-role mapping. Tasks without a `role` default to the coder.

> **Context-PR framing (#2548)**: `pr.context_title` and `pr.context_description`
> are *optional* keys planners may emit to give the dedicated context PR a
> different framing from the slice PRs (e.g. "Strategic plan for #N" vs the
> slice's "Implement …"). When omitted the orchestrator falls back to
> `pr.title` / `pr.description`. Two sibling fields — `pr.context_branch` and
> `pr.context_pr_number` — exist on the contract but are populated by the
> orchestrator after the context branch is created and the context PR is
> opened; planners must NOT emit them.
>
> **As of slice-1 (#2548 part 1)**, only the schema fields and this
> planner-prompt guidance are wired. The orchestrator branch-creation
> and PR-opening hooks land in #2548 slices 3-4 — until those slices
> merge, any `context_title` / `context_description` a planner emits
> flows through the parser into `PRMetadata` but nothing acts on it
> yet, so emitting them now is forward-compatibly safe but does not
> change the rendered PR.

> **Slices vs. phases (#2137)**: The plan parser accepts either `slices:`
> (canonical, post-#2137) or `phases:` (legacy alias) at the top of the
> `# yaml-tasks` block. New plans should emit `slices:` so they ingest as
Expand Down
69 changes: 69 additions & 0 deletions orchestrator/routes/pipelines.py
Original file line number Diff line number Diff line change
Expand Up @@ -8990,6 +8990,42 @@ def _auto_create_pr(
" components as a result.",
]

# Shared context-PR framing guidance injected into planner prompts (#2548).
# The planner may optionally emit ``pr.context_title`` / ``pr.context_description``
# to give the dedicated context PR a different framing from the slice PRs;
# falls back to ``pr.title`` / ``pr.description`` when omitted. The
# orchestrator-populated fields ``pr.context_branch`` and
# ``pr.context_pr_number`` are intentionally excluded — those are runtime
# values written by the orchestrator after the context branch is created
# and the context PR is opened, and the planner must NOT emit them.
_PR_CONTEXT_GUIDANCE = [
"**Optional context-PR framing (#2548)**: the orchestrator opens a "
"dedicated *context PR* at the root of the slice stack carrying the "
"refine/plan analysis docs and BRC consensus history. You MAY emit "
"`pr.context_title` and `pr.context_description` to frame this "
'context PR differently from the slice PRs (e.g. "Strategic plan '
'for #N" vs the slice\'s "Implement …"). Both keys are optional — '
"omit them and the orchestrator falls back to `pr.title` / "
"`pr.description`. Do NOT emit `pr.context_branch` or "
"`pr.context_pr_number`: those are populated by the orchestrator "
"after the context branch is created and the PR is opened.",
]

# Example YAML lines documenting the optional context-PR keys. Indented to
# match the surrounding ``pr:`` block (`` context_title:`` lines up with
# `` description:``). Both lines are commented-out hints because they are
# optional — emitting them is encouraged when the framing should differ.
_PR_CONTEXT_YAML_EXAMPLE_LINES = [
" # Optional context-PR framing (#2548); omit to reuse pr.title / pr.description.",
" # context_title: |-",
" # Strategic plan for #<issue> — refine/plan analysis + BRC history",
" # context_description: |-",
" # Carries the refine analysis, the plan, the BRC consensus",
" # history that approved each, and the agent transcripts —",
" # so reviewers approaching the slice stack can see the strategic",
" # narrative on a PR that targets the configured base branch.",
]

# YAML safety guidance for planner prompts. Plain (unquoted) scalars break
# when they contain ``: `` sequences — e.g. "Add `sequence: int = 0` field"
# parses as a nested mapping and raises ScannerError. Block scalars (``|-``)
Expand Down Expand Up @@ -9266,6 +9302,8 @@ def _build_phase_prompt(
"",
*_PR_DESCRIPTION_GUIDANCE,
"",
*_PR_CONTEXT_GUIDANCE,
"",
"End your document with a fenced YAML block like this:",
"",
"````",
Expand All @@ -9281,6 +9319,7 @@ def _build_phase_prompt(
" manual_steps: |",
" Pre-merge: any required steps before merging",
" Post-merge: any required steps after merging",
*_PR_CONTEXT_YAML_EXAMPLE_LINES,
"phases:",
" - id: 1",
" name: |-",
Expand Down Expand Up @@ -11071,6 +11110,8 @@ def _build_agent_prompt(
"",
*_PR_DESCRIPTION_GUIDANCE,
"",
*_PR_CONTEXT_GUIDANCE,
"",
"End your document with a fenced YAML block like this:",
"",
"````",
Expand All @@ -11086,6 +11127,7 @@ def _build_agent_prompt(
" manual_steps: |",
" Pre-merge: any required steps before merging",
" Post-merge: any required steps after merging",
*_PR_CONTEXT_YAML_EXAMPLE_LINES,
"phases:",
" - id: 1",
" name: |-",
Expand Down Expand Up @@ -14876,11 +14918,38 @@ def _populate_contract_from_plan(
if result.pr_title:
from egg_contracts.models import PRMetadata

# #2548 — preserve orchestrator-populated runtime fields on
# ``PRMetadata`` across re-populates. The planner-emitted
# ``context_title`` / ``context_description`` still flow in
# fresh from the parsed plan; the fields below are populated
# by orchestrator code paths (gateway primitives, the
# conditional-ACK gate at ``complete_phase``) and would
# otherwise be silently dropped when this safety-net
# populator re-runs (e.g. on a ``start_phase=implement``
# re-entry where ``deferred_actions`` was already populated
# during implement-phase close).
#
# ``deferred_actions`` is the merge-blocking *Pre-merge
# Obligations* handoff written by ``decisions.py`` after a
# conditional-ACK gate resolves; losing it here erases the
# reviewer's only durable handoff for git-mv / migration /
# cross-repo flips. See test
# ``test_populate_contract_from_plan_preserves_deferred_actions``.
preserved_branch = contract.pr.context_branch if contract.pr is not None else None
preserved_pr_number = contract.pr.context_pr_number if contract.pr is not None else None
preserved_deferred_actions = (
list(contract.pr.deferred_actions) if contract.pr is not None else []
)
contract.pr = PRMetadata(
title=result.pr_title,
description=result.pr_description or "",
test_plan=result.pr_test_plan or "",
manual_steps=result.pr_manual_steps or "",
context_title=result.pr_context_title,
context_description=result.pr_context_description,
context_branch=preserved_branch,
context_pr_number=preserved_pr_number,
deferred_actions=preserved_deferred_actions,
)
changed = True

Expand Down
69 changes: 69 additions & 0 deletions orchestrator/tests/test_short_flow_contract_population.py
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,75 @@ def test_no_plan_draft_is_noop(self, tmp_path: Path):
contract = load_contract(pipeline_id, tmp_path)
assert len(contract.phases) == 0 # Still empty

def test_populate_contract_from_plan_preserves_deferred_actions(self, tmp_path: Path):
"""A re-populate must preserve runtime-only ``PRMetadata`` fields.

Regression for the slice-1 review in PR #2555: the populator
rebuilds ``contract.pr`` wholesale from the plan, and a prior
version preserved ``context_branch`` / ``context_pr_number``
but silently wiped ``deferred_actions`` — the merge-blocking
Pre-merge Obligations handoff written by the conditional-ACK
gate at ``decisions.py:complete_phase``. The
``start_phase=implement`` re-entry path can hit this populator
after ``deferred_actions`` is already populated; losing it
erases the only durable handoff for git-mv / migration /
cross-repo flips.

Setup: create a contract, populate ``contract.pr`` once from
the plan, then mutate ``contract.pr.deferred_actions`` and
``contract.pr.context_branch`` / ``context_pr_number`` to
simulate runtime-populated state, save, and re-run the
populator. Assert the runtime fields survive while the
planner-emitted fields are refreshed from the plan.
"""
from egg_contracts.loader import create_contract, load_contract, save_contract
from egg_contracts.models import DeferredAction
from routes.pipelines import _populate_contract_from_plan

pipeline_id = "pipeline-deferred-preserve"

create_contract(pipeline_id=pipeline_id, title="Test", repo_root=tmp_path)

drafts_dir = tmp_path / ".egg-state" / "drafts"
drafts_dir.mkdir(parents=True, exist_ok=True)
(drafts_dir / f"{pipeline_id}-plan.md").write_text(SAMPLE_PLAN)

# First populate — establishes ``contract.pr`` from the plan.
_populate_contract_from_plan(tmp_path, pipeline_id, "local")

# Simulate runtime-populated state: a conditional-ACK gate
# resolved at ``complete_phase`` and stamped a deferred action,
# plus the orchestrator opened the context PR and stamped the
# branch / PR-number.
contract = load_contract(pipeline_id, tmp_path)
assert contract.pr is not None
contract.pr.deferred_actions = [
DeferredAction(
reviewer="reviewer_code",
condition="must rename foo → bar before merge",
resolved_in_diff="",
)
]
contract.pr.context_branch = "egg/pipeline-deferred-preserve/context"
contract.pr.context_pr_number = 7777
save_contract(contract, tmp_path)

# Re-run the populator (e.g. start_phase=implement re-entry).
_populate_contract_from_plan(tmp_path, pipeline_id, "local")

# All three runtime-populated fields must survive the re-build.
contract_after = load_contract(pipeline_id, tmp_path)
assert contract_after.pr is not None
assert len(contract_after.pr.deferred_actions) == 1
assert (
contract_after.pr.deferred_actions[0].condition == "must rename foo → bar before merge"
)
assert contract_after.pr.deferred_actions[0].reviewer == "reviewer_code"
assert contract_after.pr.context_branch == "egg/pipeline-deferred-preserve/context"
assert contract_after.pr.context_pr_number == 7777
# And the planner-emitted fields are still refreshed from the plan.
assert contract_after.pr.title == "Add retry logic to API client"


class TestEnsureStatefilesRestoresPRMetadata:
"""_ensure_statefiles_on_branch re-populates PR metadata from plan draft.
Expand Down
9 changes: 9 additions & 0 deletions scripts/file-size-allowlist.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -50,3 +50,12 @@ files:
issue: "2248"
orchestrator/kubernetes_spawner.py:
issue: "2248"
# On the egg/issue-2548/work merge target, slice-1's
# extract_pr_context_metadata_from_yaml + ParseResult.pr_context_*
# plumbing (#2548) stacks on top of #2527's validate_task_role_alignment
# additions, pushing the file to ~1,530 lines. The slice-1 branch alone
# is 1,388 lines (under the 1,500-line hard cap), but the merged
# work-branch state breaches the cap. Allowlisting under #2548 so the
# BRC implement-phase lint passes; decompose under #2569.
shared/egg_contracts/plan_parser.py:
issue: "2548"
Loading
Loading