Fix #2508: stage agent-proposed .github/ changes for human review

gateway/tests/test_agent_restrictions_patterns.py

@@ -250,6 +250,23 @@ def test_cannot_write_contracts(self, pattern):
     def test_can_write_agent_outputs(self, pattern):
         assert pattern.can_write(".egg-state/agent-outputs/coder-out.json") is True
 
+    def test_cannot_write_github_dir(self, pattern):
+        """Coder is blocked from `.github/` (branch-protection invariant)."""
+        assert pattern.can_write(".github/workflows/ci.yml") is False
+        assert pattern.can_write(".github/CODEOWNERS") is False
+
+    def test_can_write_github_staging_dir(self, pattern):
+        """Coder can write to `.github-staging/` (issue #2508 staging convention).
+
+        The `.github/` blocked prefix is matched via ``startswith(".github/")``,
+        which doesn't match `.github-staging/...`, so the catch-all
+        ``**`` allowlist reaches it. The PR builder turns staged files
+        into a manual step asking the human reviewer to move them
+        into `.github/` before merge.
+        """
+        assert pattern.can_write(".github-staging/workflows/ci.yml") is True
+        assert pattern.can_write(".github-staging/CODEOWNERS") is True
+
     def test_can_write_python_version(self, pattern):
         """Coder can write .python-version (common project config)."""
         assert pattern.can_write(".python-version") is True
@@ -377,6 +394,20 @@ def test_blocks_github_codeowners(self, pattern):
         """.github/ is blocked (branch-protection invariant)."""
         assert pattern.can_write(".github/CODEOWNERS") is False
 
+    def test_allows_github_staging_workflow(self, pattern):
+        """`.github-staging/` is the staging-dir convention from issue #2508.
+
+        The `.github/` blocked prefix is matched via ``startswith(".github/")``,
+        which does not match `.github-staging/...` — so the catch-all
+        ``**`` allowlist reaches the staging dir. This regression-locks
+        that behavior so the convention keeps working.
+        """
+        assert pattern.can_write(".github-staging/workflows/ci.yml") is True
+
+    def test_allows_github_staging_codeowners(self, pattern):
+        """Staging-dir convention covers extensionless files like CODEOWNERS."""
+        assert pattern.can_write(".github-staging/CODEOWNERS") is True
+
     def test_allows_license(self, pattern):
         assert pattern.can_write("LICENSE") is True
 

orchestrator/routes/pipelines.py

@@ -5561,6 +5561,26 @@ def _build_role_restrictions_section() -> str:
     )
     lines.append("")
 
+    # Staging-dir convention for `.github/` (issue #2508).
+    lines.append("### `.github/` changes — use the `.github-staging/` convention")
+    lines.append("")
+    lines.append(
+        "Every producer role is blocked from writing under `.github/` "
+        "(CI workflows, CODEOWNERS, dependabot config) — this is a "
+        "branch-protection invariant, not a planner mistake. Tasks that "
+        "need to modify those files must instead write the proposed "
+        "end-state to top-level `.github-staging/`, mirroring the "
+        "`.github/` structure (e.g. a proposed change to "
+        "`.github/workflows/ci.yml` is staged at "
+        "`.github-staging/workflows/ci.yml`). The orchestrator's PR "
+        "builder auto-detects `.github-staging/` and emits a manual "
+        "step asking the human reviewer to move the staged files into "
+        "place before merge. Assign such tasks to `role: coder` and "
+        "make the staging path explicit in the task's "
+        "`files_affected`."
+    )
+    lines.append("")
+
     return "\n".join(lines)
 
 
@@ -8464,6 +8484,69 @@ def _pr_metadata_from_plan_draft(
     )
 
 
+def _build_github_staging_manual_step(worktree_repo_path: Path) -> str:
+    """Render the auto manual-step for `.github-staging/` files (issue #2508).
+
+    Producer agents (coder, etc.) cannot push to `.github/` because the
+    gateway blocks the path as a branch-protection invariant.  When a
+    plan calls for CI workflow or CODEOWNERS changes, the agent instead
+    writes the proposed end-state to top-level `.github-staging/`,
+    mirroring the `.github/` structure.  This helper scans that
+    directory and returns a markdown step the human reviewer must
+    complete before merge: review the staged files, move them into
+    `.github/`, delete the staging dir, and push the resulting commit.
+
+    Returns an empty string when `.github-staging/` is absent or empty.
+    """
+    staging_dir = worktree_repo_path / ".github-staging"
+    if not staging_dir.is_dir():
+        return ""
+
+    staged_paths: list[str] = []
+    for path in sorted(staging_dir.rglob("*")):
+        if not path.is_file():
+            continue
+        try:
+            rel = path.relative_to(worktree_repo_path).as_posix()
+        except ValueError:
+            continue
+        staged_paths.append(rel)
+
+    if not staged_paths:
+        return ""
+
+    lines = [
+        "### Move staged `.github/` changes (auto-generated, issue #2508)",
+        "",
+        "This PR includes proposed `.github/` changes under `.github-staging/`. "
+        "Agent roles cannot push to `.github/` directly (CI workflow / CODEOWNERS "
+        "branch-protection invariant), so the agent staged the proposed "
+        "end-state for human review.",
+        "",
+        "Staged files:",
+    ]
+    for rel in staged_paths:
+        lines.append(f"- `{rel}`")
+    lines.extend(
+        [
+            "",
+            "Before merging:",
+            "",
+            "1. Review each staged file for correctness — these are proposed "
+            "CI / repo-config changes that bypass the agent's normal sandbox.",
+            "2. Move each file from `.github-staging/<path>` to `.github/<path>`. For example:",
+            "   ```",
+            "   mkdir -p .github/workflows",
+            "   git mv .github-staging/workflows/test-e2e.yml .github/workflows/test-e2e.yml",
+            "   ```",
+            "3. Remove the empty `.github-staging/` directory and commit the move.",
+            "4. Push from a context with the GitHub `workflow` scope (a normal "
+            "user push works; the bot token may not — see issue #2508 layer 2).",
+        ]
+    )
+    return "\n".join(lines)
+
+
 def _build_pr_body(
     pipeline: Pipeline,
     worktree_repo_path: Path,
@@ -8598,9 +8681,22 @@ def _build_pr_body(
     else:
         body_parts.append("## Test Plan\n\n_No test plan provided by the planner._")
 
-    # Manual steps section (only if there are steps)
+    # Auto-generated step for `.github-staging/` (issue #2508): the
+    # gateway blocks every producer role from pushing to `.github/`,
+    # so agents drop proposed CI workflow / CODEOWNERS changes into
+    # top-level `.github-staging/` instead. Detect them here and
+    # surface a step the human reviewer must complete before merge.
+    github_staging_step = _build_github_staging_manual_step(worktree_repo_path)
+
+    # Manual steps section: planner-supplied steps and the staging-dir
+    # auto-step are rendered together so reviewers see one block.
+    manual_step_chunks: list[str] = []
     if pr_manual_steps:
-        body_parts.append(f"## Manual Steps\n\n{pr_manual_steps}")
+        manual_step_chunks.append(pr_manual_steps)
+    if github_staging_step:
+        manual_step_chunks.append(github_staging_step)
+    if manual_step_chunks:
+        body_parts.append("## Manual Steps\n\n" + "\n\n".join(manual_step_chunks))
 
     # Add pipeline context section
     if pipeline.id or pipeline.issue_number:
@@ -10248,6 +10344,24 @@ def _build_file_boundary_section(role_value: str) -> str:
         lines.append("**Allowed:** " + ", ".join(f"`{p}`" for p in fa.allowed_write))
     if fa.blocked_write:
         lines.append("**Blocked:** " + ", ".join(f"`{p}`" for p in fa.blocked_write))
+
+    # `.github/` staging-dir convention (issue #2508). Surfaced for the
+    # coder role specifically because it's the only producer whose
+    # catch-all allowlist reaches `.github-staging/`; other roles can't
+    # write the staged files even via the convention.
+    if role_value == "coder":
+        lines.append("")
+        lines.append(
+            "**`.github/` changes**: `.github/` is blocked above. If your "
+            "task requires modifying CI workflows, CODEOWNERS, dependabot "
+            "config, or anything else under `.github/`, write the proposed "
+            "end-state to top-level `.github-staging/` instead, mirroring "
+            "the `.github/` structure (e.g. stage "
+            "`.github/workflows/test-e2e.yml` as "
+            "`.github-staging/workflows/test-e2e.yml`). The PR builder "
+            "auto-emits a manual step asking the human reviewer to move "
+            "the staged files into place before merge — see issue #2508."
+        )
     lines.append("")
     return "\n".join(lines)
 

orchestrator/tests/test_auto_pr.py

@@ -404,6 +404,81 @@ def test_banner_notes_missing_draft(self, tmp_path):
         assert ".egg-state/drafts/42-plan.md" in body
 
 
+class TestBuildPrBodyGithubStaging:
+    """Tests for the `.github-staging/` auto-step in _build_pr_body (issue #2508).
+
+    Producer agents are blocked from `.github/` by role patterns. The
+    convention introduced in #2508 has agents stage proposed `.github/`
+    changes under top-level `.github-staging/`; the PR builder detects
+    them and emits a manual step asking the human reviewer to move the
+    files into `.github/` before merge.
+    """
+
+    def test_no_step_when_staging_dir_absent(self, tmp_path):
+        """No manual step when `.github-staging/` does not exist."""
+        pipeline = _make_pipeline()
+
+        _title, body, _ = _build_pr_body(pipeline, tmp_path)
+
+        assert ".github-staging" not in body
+        assert "Move staged" not in body
+
+    def test_no_step_when_staging_dir_empty(self, tmp_path):
+        """No manual step when `.github-staging/` exists but contains no files."""
+        pipeline = _make_pipeline()
+        (tmp_path / ".github-staging").mkdir()
+
+        _title, body, _ = _build_pr_body(pipeline, tmp_path)
+
+        assert "Move staged" not in body
+
+    def test_emits_step_when_staging_dir_has_files(self, tmp_path):
+        """Manual step lists each staged file and tells reviewer to move them."""
+        pipeline = _make_pipeline()
+        staging = tmp_path / ".github-staging"
+        (staging / "workflows").mkdir(parents=True)
+        (staging / "workflows" / "test-e2e.yml").write_text("name: e2e\n")
+        (staging / "CODEOWNERS").write_text("* @team\n")
+
+        _title, body, _ = _build_pr_body(pipeline, tmp_path)
+
+        assert "## Manual Steps" in body
+        assert "Move staged `.github/` changes" in body
+        assert "`.github-staging/workflows/test-e2e.yml`" in body
+        assert "`.github-staging/CODEOWNERS`" in body
+        assert "git mv" in body
+
+    def test_step_merged_with_planner_manual_steps(self, tmp_path):
+        """Planner-supplied manual_steps and the auto step share one section."""
+        pipeline = _make_pipeline()
+        contract_dir = tmp_path / ".egg-state" / "contracts"
+        contract_dir.mkdir(parents=True)
+        contract = _make_contract_json()
+        contract["pr"]["manual_steps"] = "Pre-merge: run db migration."
+        (contract_dir / "42.json").write_text(json.dumps(contract))
+        staging = tmp_path / ".github-staging" / "workflows"
+        staging.mkdir(parents=True)
+        (staging / "ci.yml").write_text("name: ci\n")
+
+        _title, body, _ = _build_pr_body(pipeline, tmp_path)
+
+        # Both the planner step and the auto step appear under one
+        # `## Manual Steps` heading (only one heading in the body).
+        assert body.count("## Manual Steps") == 1
+        assert "Pre-merge: run db migration." in body
+        assert "Move staged `.github/` changes" in body
+        assert "`.github-staging/workflows/ci.yml`" in body
+
+    def test_ignores_subdirectories_with_no_files(self, tmp_path):
+        """Empty subdirectories under `.github-staging/` don't emit the step."""
+        pipeline = _make_pipeline()
+        (tmp_path / ".github-staging" / "workflows").mkdir(parents=True)
+
+        _title, body, _ = _build_pr_body(pipeline, tmp_path)
+
+        assert "Move staged" not in body
+
+
 class TestAutoCreatePr:
     """Tests for _auto_create_pr."""
 

orchestrator/tests/test_pipeline_prompts.py

@@ -3439,6 +3439,17 @@ def test_includes_header(self):
         section = _build_role_restrictions_section()
         assert "## Execution Role File Restrictions" in section
 
+    def test_includes_github_staging_convention(self):
+        """Section documents the `.github-staging/` staging convention (issue #2508).
+
+        Without this guidance, the planner schedules `.github/`-touching tasks
+        that no producer role can actually push, leading to silent
+        slice-failure as observed on the issue-2474 pipeline.
+        """
+        section = _build_role_restrictions_section()
+        assert ".github-staging/" in section
+        assert ".github/" in section
+
 
 class TestTaskPlannerRoleRestrictions:
     """Tests for task_planner prompt including role restrictions."""

shared/egg_restrictions/hints.py

@@ -42,7 +42,17 @@
     ),
     (
         ".github/",
-        "These paths are owned by infrastructure; ask a human reviewer to merge changes here.",
+        # Issue #2508: producer roles cannot push to `.github/` directly,
+        # but they CAN stage proposed changes under top-level
+        # `.github-staging/` mirroring the `.github/` structure. The PR
+        # builder auto-emits a manual step asking the human reviewer to
+        # move the staged files into `.github/` before merge. Point
+        # agents at the staging path so a 403 here becomes a
+        # self-correcting nudge rather than a dead-end.
+        "Stage your proposed change at `.github-staging/<same path>` "
+        "instead — the PR builder will surface a manual step asking "
+        "the human reviewer to move the staged files into `.github/` "
+        "before merge. See issue #2508.",
     ),
     (
         "docs/",

shared/egg_restrictions/patterns.py

@@ -147,7 +147,14 @@ def _normalize_path(file_path: str) -> str:
         "**/*.spec.jsx",
         "**/conftest.py",
         # Defense-in-depth: CI workflows and CODEOWNERS — preserves the
-        # branch-protection invariant.
+        # branch-protection invariant. Agents that need to propose
+        # `.github/` changes (CI workflow edits, CODEOWNERS rotation)
+        # write the proposed end-state to top-level `.github-staging/`
+        # mirroring the `.github/` structure; the prefix-match below
+        # leaves `.github-staging/` allowed via the `**` allowlist, and
+        # `_build_pr_body` auto-emits a manual step for the human
+        # reviewer to move the files into `.github/` before merge
+        # (issue #2508).
         ".github/",
     ],
     block_exempt_patterns=[

shared/tests/test_egg_restrictions_hints.py

@@ -24,8 +24,10 @@ def test_unmatched_path_returns_none() -> None:
         (".egg-state/agent-anchors/coder.json", "mcp__sdlc__update_anchor"),
         (".egg-state/drafts/plan.md", "different agent role"),
         (".egg-state/reviews/code.md", "different agent role"),
-        (".github/workflows/ci.yml", "owned by infrastructure"),
-        (".github/CODEOWNERS", "owned by infrastructure"),
+        # Issue #2508: hint points agents at the `.github-staging/`
+        # convention so a 403 here becomes self-correcting.
+        (".github/workflows/ci.yml", ".github-staging/"),
+        (".github/CODEOWNERS", ".github-staging/"),
         ("docs/index.md", "documenter role"),
         ("README.md", "documenter role"),
         ("orchestrator/foo/README.md", "documenter role"),