From 7ffd1aeeb608dc0af4082ca202cf30672e492f31 Mon Sep 17 00:00:00 2001
From: James Wiesebron <jameswiesebron@khanacademy.org>
Date: Wed, 6 May 2026 18:31:02 -0700
Subject: [PATCH 1/4] Fix #2508: add `.github-staging/` convention for
 agent-proposed CI changes
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Producer agents (coder, documenter, tester) are blocked from `.github/`
to preserve the branch-protection invariant. When a plan tasks an agent
with editing CI workflows or CODEOWNERS, the agent silently fails and
the slice ends with the file unchanged.

Introduce a convention: agents stage proposed `.github/` changes under
top-level `.github-staging/`, mirroring the `.github/` structure. The
existing `.github/` blocked prefix matches via `startswith(".github/")`,
which doesn't match `.github-staging/...`, so the coder's catch-all
allowlist already reaches the staging dir without any pattern change.

`_build_pr_body` now scans `.github-staging/` and, when non-empty,
emits an auto-generated entry under `## Manual Steps` listing each
staged file with explicit `git mv` instructions for the human reviewer.
The planner prompt and the coder file-boundary section both document
the convention so agents know to use the staging dir when their task
requires `.github/` changes.

Layer-2 (GitHub `workflow` auth scope) is out of scope here — once a
human moves files locally and pushes, their normal user auth covers it.
---
 .../tests/test_agent_restrictions_patterns.py |  31 +++++
 orchestrator/routes/pipelines.py              | 118 +++++++++++++++++-
 orchestrator/tests/test_auto_pr.py            |  75 +++++++++++
 orchestrator/tests/test_pipeline_prompts.py   |  11 ++
 shared/egg_restrictions/patterns.py           |   9 +-
 5 files changed, 241 insertions(+), 3 deletions(-)

diff --git a/gateway/tests/test_agent_restrictions_patterns.py b/gateway/tests/test_agent_restrictions_patterns.py
index a6c2ef6ace..2dd798204c 100644
--- a/gateway/tests/test_agent_restrictions_patterns.py
+++ b/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
 
diff --git a/orchestrator/routes/pipelines.py b/orchestrator/routes/pipelines.py
index ded73731c5..fd7b9a1e2a 100644
--- a/orchestrator/routes/pipelines.py
+++ b/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)
 
diff --git a/orchestrator/tests/test_auto_pr.py b/orchestrator/tests/test_auto_pr.py
index 51a53f012d..0ef37d7874 100644
--- a/orchestrator/tests/test_auto_pr.py
+++ b/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."""
 
diff --git a/orchestrator/tests/test_pipeline_prompts.py b/orchestrator/tests/test_pipeline_prompts.py
index c0de9e3596..4c8a9523d6 100644
--- a/orchestrator/tests/test_pipeline_prompts.py
+++ b/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."""
diff --git a/shared/egg_restrictions/patterns.py b/shared/egg_restrictions/patterns.py
index b1de38c6fe..2c0b7ee730 100644
--- a/shared/egg_restrictions/patterns.py
+++ b/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=[

From 093299ccf58a636aac3a656d25bd4218f726f8e4 Mon Sep 17 00:00:00 2001
From: James Wiesebron <jameswiesebron@khanacademy.org>
Date: Wed, 6 May 2026 18:40:11 -0700
Subject: [PATCH 2/4] Update .github/ push-denial hint to point at
 .github-staging/

The existing hint told blocked agents to "ask a human reviewer to merge
changes here", which dead-ended any agent that ignored the prompt
guidance. Rewrite it to point at the staging-dir convention so a 403
becomes a self-correcting nudge: the agent restages the change at
`.github-staging/<same path>`, the gateway accepts the push, and the
PR builder emits the manual step for the human reviewer.

Updates the parametrized hint test to match the new substring.
---
 shared/egg_restrictions/hints.py            | 12 +++++++++++-
 shared/tests/test_egg_restrictions_hints.py |  6 ++++--
 2 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/shared/egg_restrictions/hints.py b/shared/egg_restrictions/hints.py
index 9c728dc01b..a0bf2d8925 100644
--- a/shared/egg_restrictions/hints.py
+++ b/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/",
diff --git a/shared/tests/test_egg_restrictions_hints.py b/shared/tests/test_egg_restrictions_hints.py
index d256605f47..4f7a8ae83d 100644
--- a/shared/tests/test_egg_restrictions_hints.py
+++ b/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"),

From 01a9f65ae17ea9b5d2a8260a7001895ca1954862 Mon Sep 17 00:00:00 2001
From: "egg-reviewer[bot]"
 <261018737+egg-reviewer[bot]@users.noreply.github.com>
Date: Thu, 7 May 2026 02:15:14 +0000
Subject: [PATCH 3/4] Block .github/ at the role-pattern layer + harden staging
 step
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Address review feedback on PR #2514 (egg-reviewer[bot]):

- Add .github/ to blocked_write for CODER/TESTER/DOCUMENTER/AUTOFIXER/
  CONFLICT_RESOLVER role definitions (agent_roles.py) and to
  blocked_patterns for DOCUMENTER/AUTOFIXER/CONFLICT_RESOLVER patterns
  (patterns.py). The prior PR's prose claim that 'every producer role
  is blocked from writing under .github/' was empirically false:
  documenter could rewrite .github/PULL_REQUEST_TEMPLATE.md via
  **/*.md, autofixer could rewrite .github/workflows/ci.yml via
  **/*.yml, and conflict_resolver could commit a workflow conflict
  resolution directly. The blocks make the assertion true.

- Add .github-staging/ to coder allowed_write so the rendered Allowed
  list (and the gateway-enforced patterns) agree on the staging path.

- Drop staged symlinks in _build_github_staging_manual_step (Path.is_file
  follows symlinks, so a malicious .github-staging/evil.yml -> /etc/passwd
  would have survived into the manual-step file list).

- Reword step 3 of the manual step: git doesn't track empty dirs, so
  there's no separate 'remove the empty staging dir' commit. Folded the
  cleanup into step 2 as a local rm -rf hint, and renumbered step 4
  (commit + push) into step 3.

- Document the .gitignore interaction in the role-restrictions prompt
  prose so planners flag it.

- Strengthen test_includes_github_staging_convention with assertions on
  the actionable guidance (role assignment, concrete staging path
  example, .gitignore note) — bare-string assertions also pass when a
  refactor drops the directive content.

- Add regression tests for the new .github/ blocks on documenter,
  autofixer, conflict_resolver, plus symlink-filter coverage in the PR
  builder.
---
 .../tests/test_agent_restrictions_patterns.py | 42 +++++++++++++++++
 orchestrator/routes/pipelines.py              | 25 ++++++++--
 orchestrator/tests/test_auto_pr.py            | 47 +++++++++++++++++++
 orchestrator/tests/test_pipeline_prompts.py   | 12 +++++
 shared/egg_contracts/agent_roles.py           | 32 +++++++++++++
 shared/egg_restrictions/patterns.py           | 18 +++++++
 6 files changed, 172 insertions(+), 4 deletions(-)

diff --git a/gateway/tests/test_agent_restrictions_patterns.py b/gateway/tests/test_agent_restrictions_patterns.py
index 2dd798204c..39a47f840d 100644
--- a/gateway/tests/test_agent_restrictions_patterns.py
+++ b/gateway/tests/test_agent_restrictions_patterns.py
@@ -646,6 +646,23 @@ def test_cannot_write_contracts(self, pattern):
     def test_can_write_agent_outputs(self, pattern):
         assert pattern.can_write(".egg-state/agent-outputs/doc-out.json") is True
 
+    def test_cannot_write_github_dir_md(self, pattern):
+        """Documenter is blocked from markdown under `.github/` (issue #2508).
+
+        Without the `.github/` block, the documenter's ``**/*.md``
+        allow would let it rewrite ``.github/PULL_REQUEST_TEMPLATE.md``,
+        ``.github/ISSUE_TEMPLATE.md``, etc. — bypassing the
+        branch-protection invariant the staging convention preserves.
+        """
+        assert pattern.can_write(".github/PULL_REQUEST_TEMPLATE.md") is False
+        assert pattern.can_write(".github/ISSUE_TEMPLATE.md") is False
+
+    def test_cannot_write_github_dir_codeowners(self, pattern):
+        """Even non-markdown under `.github/` is blocked — the documenter
+        couldn't reach CODEOWNERS via its allowlist anyway, but the
+        block makes the invariant explicit."""
+        assert pattern.can_write(".github/CODEOWNERS") is False
+
 
 class TestArchitectRole:
     """Verify architect agent can only write drafts and agent-outputs."""
@@ -833,6 +850,18 @@ def test_cannot_write_contracts(self, pattern):
     def test_can_write_agent_outputs(self, pattern):
         assert pattern.can_write(".egg-state/agent-outputs/autofixer-out.json") is True
 
+    def test_cannot_write_github_workflows(self, pattern):
+        """Autofixer is blocked from `.github/` (issue #2508).
+
+        Without this block, the autofixer's ``**/*.yml`` allow would
+        let it rewrite ``.github/workflows/ci.yml`` directly when
+        applying a YAML lint fix — bypassing the branch-protection
+        invariant the staging convention preserves.
+        """
+        assert pattern.can_write(".github/workflows/ci.yml") is False
+        assert pattern.can_write(".github/workflows/test.yaml") is False
+        assert pattern.can_write(".github/dependabot.yml") is False
+
 
 class TestConflictResolverRole:
     """Verify conflict resolver agent can/cannot write expected files."""
@@ -912,6 +941,19 @@ def test_cannot_write_reviews(self, pattern):
     def test_can_write_agent_outputs(self, pattern):
         assert pattern.can_write(".egg-state/agent-outputs/resolver-out.json") is True
 
+    def test_cannot_write_github_dir(self, pattern):
+        """Conflict resolver is blocked from `.github/` (issue #2508).
+
+        Without this block, resolving a merge conflict that touches
+        ``.github/workflows/*.yml`` or ``.github/CODEOWNERS`` would
+        commit the resolution directly into `.github/` and push it,
+        bypassing the branch-protection invariant the staging
+        convention preserves.
+        """
+        assert pattern.can_write(".github/workflows/ci.yml") is False
+        assert pattern.can_write(".github/CODEOWNERS") is False
+        assert pattern.can_write(".github/PULL_REQUEST_TEMPLATE.md") is False
+
 
 class TestAllRolesRegistered:
     """Verify all defined roles have patterns in the registry."""
diff --git a/orchestrator/routes/pipelines.py b/orchestrator/routes/pipelines.py
index fd7b9a1e2a..b5c9ae17f6 100644
--- a/orchestrator/routes/pipelines.py
+++ b/orchestrator/routes/pipelines.py
@@ -5577,7 +5577,10 @@ def _build_role_restrictions_section() -> str:
         "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`."
+        "`files_affected`. `.github-staging/` must remain tracked by "
+        "git (do not add it to `.gitignore`); otherwise the staged "
+        "files won't be in the PR commit and the reviewer's `git mv` "
+        "will fail."
     )
     lines.append("")
 
@@ -8504,6 +8507,16 @@ def _build_github_staging_manual_step(worktree_repo_path: Path) -> str:
 
     staged_paths: list[str] = []
     for path in sorted(staging_dir.rglob("*")):
+        # Skip symlinks: ``Path.is_file()`` follows them, so without this
+        # guard a staged ``.github-staging/evil.yml`` → ``/etc/passwd``
+        # would be surfaced in the manual-step file list, the reviewer's
+        # ``git mv`` would preserve it, and ``.github/evil.yml`` would
+        # land in the repo as a symlink. The reviewer's only mitigation
+        # would be the diff (where a symlink shows as a small mode
+        # change that's easy to skim past). Drop staged symlinks here so
+        # the helper is the choke point.
+        if path.is_symlink():
+            continue
         if not path.is_file():
             continue
         try:
@@ -8539,9 +8552,13 @@ def _build_github_staging_manual_step(worktree_repo_path: Path) -> str:
             "   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).",
+            "   After the `git mv`, `.github-staging/` is no longer tracked "
+            "by git (git doesn't track empty directories). Run "
+            "`rm -rf .github-staging` locally if you want to clear any "
+            "leftover empty subdirectories from your worktree.",
+            "3. Commit the move and 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)
diff --git a/orchestrator/tests/test_auto_pr.py b/orchestrator/tests/test_auto_pr.py
index 0ef37d7874..adf7266d87 100644
--- a/orchestrator/tests/test_auto_pr.py
+++ b/orchestrator/tests/test_auto_pr.py
@@ -478,6 +478,53 @@ def test_ignores_subdirectories_with_no_files(self, tmp_path):
 
         assert "Move staged" not in body
 
+    def test_drops_symlinks_from_staged_paths(self, tmp_path):
+        """Symlinks under `.github-staging/` are filtered out (issue #2508).
+
+        ``Path.is_file()`` follows symlinks, so without an explicit
+        ``is_symlink()`` guard a malicious staged file pointing at
+        ``/etc/passwd`` would survive into the manual-step file list,
+        the reviewer's `git mv` would preserve it, and `.github/...`
+        would land in the repo as a symlink. This test regression-locks
+        the guard so the helper stays the choke point.
+        """
+        pipeline = _make_pipeline()
+        staging = tmp_path / ".github-staging" / "workflows"
+        staging.mkdir(parents=True)
+        # Real file alongside a symlink — only the real file should
+        # appear in the rendered step.
+        (staging / "ci.yml").write_text("name: ci\n")
+        target = tmp_path / "outside-target.yml"
+        target.write_text("name: outside\n")
+        (staging / "evil-symlink.yml").symlink_to(target)
+
+        _title, body, _ = _build_pr_body(pipeline, tmp_path)
+
+        assert "Move staged `.github/` changes" in body
+        assert "`.github-staging/workflows/ci.yml`" in body
+        # The symlink must NOT be surfaced — it would pass `is_file()`
+        # but the guard above drops it before the rel-path is recorded.
+        assert "evil-symlink.yml" not in body
+
+    def test_drops_step_when_only_symlinks_staged(self, tmp_path):
+        """When `.github-staging/` contains only symlinks, no step is emitted.
+
+        Mirrors :meth:`test_no_step_when_staging_dir_empty` for the
+        symlink-only case — the symlink-filter must not leave the
+        helper in a state where it emits a header with an empty file
+        list.
+        """
+        pipeline = _make_pipeline()
+        staging = tmp_path / ".github-staging"
+        staging.mkdir()
+        target = tmp_path / "outside-target.yml"
+        target.write_text("name: outside\n")
+        (staging / "only-symlink.yml").symlink_to(target)
+
+        _title, body, _ = _build_pr_body(pipeline, tmp_path)
+
+        assert "Move staged" not in body
+
 
 class TestAutoCreatePr:
     """Tests for _auto_create_pr."""
diff --git a/orchestrator/tests/test_pipeline_prompts.py b/orchestrator/tests/test_pipeline_prompts.py
index 4c8a9523d6..96c0ff3680 100644
--- a/orchestrator/tests/test_pipeline_prompts.py
+++ b/orchestrator/tests/test_pipeline_prompts.py
@@ -3445,10 +3445,22 @@ def test_includes_github_staging_convention(self):
         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.
+
+        Asserts the actionable guidance in addition to the bare strings, so
+        a future refactor that drops the role-assignment instruction or the
+        concrete staging-path example is caught here.
         """
         section = _build_role_restrictions_section()
+        # Bare strings — both also occur elsewhere in the section.
         assert ".github-staging/" in section
         assert ".github/" in section
+        # Actionable guidance: the role assignment, a concrete staging
+        # path example, and the `.gitignore` warning. These are the
+        # things a planner needs to act on; a refactor that loses them
+        # would render the section informational rather than directive.
+        assert "role: coder" in section
+        assert ".github-staging/workflows/ci.yml" in section
+        assert ".gitignore" in section
 
 
 class TestTaskPlannerRoleRestrictions:
diff --git a/shared/egg_contracts/agent_roles.py b/shared/egg_contracts/agent_roles.py
index d8b6332885..535671d630 100644
--- a/shared/egg_contracts/agent_roles.py
+++ b/shared/egg_contracts/agent_roles.py
@@ -206,11 +206,22 @@ def depends_on(self, other: AgentRole) -> bool:
             "**/*.yaml",
             "**/*.json",
             ".egg-state/agent-outputs/",  # For handoff data
+            # Issue #2508: staging dir for proposed `.github/` changes.
+            # `.github/` itself is blocked below; the agent stages the
+            # proposed end-state here and the PR builder emits a manual
+            # step asking the human reviewer to move the files into
+            # `.github/` before merge.
+            ".github-staging/",
         ],
         blocked_write=[
             "docs/",  # Documenter handles docs
             "**/README.md",  # Documenter handles READMEs
             ".egg-state/contracts/",  # Contracts are managed by API
+            # Issue #2508: branch-protection invariant — agents cannot
+            # push to `.github/` directly. Use `.github-staging/` (above)
+            # to propose CI workflow / CODEOWNERS changes for human
+            # review.
+            ".github/",
         ],
     ),
     produces_outputs=["changed_files", "commits"],
@@ -271,6 +282,11 @@ def depends_on(self, other: AgentRole) -> bool:
             "**/README.md",
             "**/*.md",
             ".egg-state/contracts/",
+            # Issue #2508: branch-protection invariant — even a
+            # tester-applied auto-fix to `.github/workflows/*.yml` must
+            # go through the `.github-staging/` convention so a human
+            # reviewer moves it in before merge.
+            ".github/",
         ],
     ),
     produces_outputs=["test_files", "coverage_report", "gaps_found", "check_results"],
@@ -306,6 +322,12 @@ def depends_on(self, other: AgentRole) -> bool:
             "**/*.java",
             "tests/",  # Cannot modify tests
             ".egg-state/contracts/",
+            # Issue #2508: branch-protection invariant — even markdown
+            # files under `.github/` (PULL_REQUEST_TEMPLATE.md,
+            # ISSUE_TEMPLATE.md, etc.) must go through the
+            # `.github-staging/` convention so a human reviewer moves
+            # them in before merge.
+            ".github/",
         ],
     ),
     can_run_in_parallel=True,  # Can run in parallel with tester
@@ -794,6 +816,11 @@ def depends_on(self, other: AgentRole) -> bool:
             "docs/",
             "**/*.md",
             ".egg-state/contracts/",
+            # Issue #2508: branch-protection invariant — even an
+            # auto-fix to `.github/workflows/*.yml` must go through the
+            # `.github-staging/` convention so a human reviewer moves
+            # it in before merge.
+            ".github/",
         ],
     ),
     produces_outputs=["autofix_report", "fixed_files"],
@@ -857,6 +884,11 @@ def depends_on(self, other: AgentRole) -> bool:
             ".egg-state/pipelines/",
             ".egg-state/reviews/",
             ".egg-state/oversight/",
+            # Issue #2508: branch-protection invariant — when resolving
+            # a conflict that touches a workflow / CODEOWNERS file, the
+            # resolution must go through the `.github-staging/`
+            # convention so a human reviewer moves it in before merge.
+            ".github/",
         ],
     ),
     produces_outputs=["conflict_report", "resolved_files"],
diff --git a/shared/egg_restrictions/patterns.py b/shared/egg_restrictions/patterns.py
index 2c0b7ee730..47ebe05211 100644
--- a/shared/egg_restrictions/patterns.py
+++ b/shared/egg_restrictions/patterns.py
@@ -254,6 +254,13 @@ def _normalize_path(file_path: str) -> str:
         "**/test/",
         # Contracts
         ".egg-state/contracts/",
+        # Issue #2508: branch-protection invariant — even markdown
+        # files under `.github/` (PULL_REQUEST_TEMPLATE.md,
+        # ISSUE_TEMPLATE.md, etc.) must go through the
+        # `.github-staging/` convention so a human reviewer moves them
+        # in before merge. Without this block, `**/*.md` would let the
+        # documenter rewrite `.github/PULL_REQUEST_TEMPLATE.md`.
+        ".github/",
     ],
 )
 
@@ -506,6 +513,12 @@ def _normalize_path(file_path: str) -> str:
         "**/*.md",
         # Contracts
         ".egg-state/contracts/",
+        # Issue #2508: branch-protection invariant — even an autofixer
+        # YAML lint fix to `.github/workflows/*.yml` must go through
+        # the `.github-staging/` convention so a human reviewer moves
+        # it in before merge. Without this block, `**/*.yml` would let
+        # the autofixer rewrite workflows directly.
+        ".github/",
     ],
 )
 
@@ -576,6 +589,11 @@ def _normalize_path(file_path: str) -> str:
         ".egg-state/pipelines/",
         ".egg-state/reviews/",
         ".egg-state/oversight/",
+        # Issue #2508: branch-protection invariant — when resolving a
+        # merge conflict that touches a workflow / CODEOWNERS file,
+        # the resolution must go through the `.github-staging/`
+        # convention so a human reviewer moves it in before merge.
+        ".github/",
     ],
 )
 

From 0025a559c06cb452bfa39ba2be0371e208cb4459 Mon Sep 17 00:00:00 2001
From: "egg-reviewer[bot]"
 <261018737+egg-reviewer[bot]@users.noreply.github.com>
Date: Thu, 7 May 2026 02:53:37 +0000
Subject: [PATCH 4/4] Guard .github-staging dir-as-symlink + fix stale comments
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Address re-review feedback on PR #2514:

- _build_github_staging_manual_step: also drop the step when
  .github-staging itself is a symlink. Path.is_dir() follows
  symlinks, so without this guard a malicious .github-staging ->
  /etc would let rglob enumerate host files into the manual-step
  file list. Adds a regression test mirroring the per-entry
  symlink guard.

- _build_file_boundary_section: reword the staging-dir comment
  so it doesn't claim other producer roles can't write the
  staged files. The .github/ block uses startswith, so
  autofixer / conflict_resolver allowlists can technically reach
  .github-staging/ — they just aren't expected to plan new
  .github/ work, so the planning-time guidance only needs to
  surface for coder.

- test_includes_github_staging_convention: split assertions
  into bare-string (ambiguous) vs prose-pinning (unambiguous)
  groups in the comment, so the test's coverage claims match
  reality (role: coder appears in the role-assignment paragraph
  too, not just the staging-dir prose).
---
 orchestrator/routes/pipelines.py            | 18 +++++++++++++---
 orchestrator/tests/test_auto_pr.py          | 23 +++++++++++++++++++++
 orchestrator/tests/test_pipeline_prompts.py | 16 +++++++++-----
 3 files changed, 49 insertions(+), 8 deletions(-)

diff --git a/orchestrator/routes/pipelines.py b/orchestrator/routes/pipelines.py
index b5c9ae17f6..ead3af2ba0 100644
--- a/orchestrator/routes/pipelines.py
+++ b/orchestrator/routes/pipelines.py
@@ -8502,6 +8502,14 @@ def _build_github_staging_manual_step(worktree_repo_path: Path) -> str:
     Returns an empty string when `.github-staging/` is absent or empty.
     """
     staging_dir = worktree_repo_path / ".github-staging"
+    # Drop the whole step when ``.github-staging`` itself is a symlink:
+    # ``Path.is_dir()`` follows symlinks, so without this guard a
+    # ``.github-staging -> /etc`` (or any other host path) would let
+    # ``rglob`` enumerate the link target's files into the manual-step
+    # file list, polluting the PR body with arbitrary host-filesystem
+    # paths. Mirrors the per-entry symlink guard below.
+    if staging_dir.is_symlink():
+        return ""
     if not staging_dir.is_dir():
         return ""
 
@@ -10363,9 +10371,13 @@ def _build_file_boundary_section(role_value: str) -> str:
         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.
+    # coder role specifically because it's the producer that's expected
+    # to initiate `.github/` work. The role-pattern check
+    # (``startswith(".github/")``) doesn't match `.github-staging/`, so
+    # autofixer / conflict_resolver allowlists technically reach the
+    # staging path too — but those roles are reactive and aren't asked
+    # to plan new `.github/` changes, so the convention's planning-time
+    # guidance only needs to land for coder.
     if role_value == "coder":
         lines.append("")
         lines.append(
diff --git a/orchestrator/tests/test_auto_pr.py b/orchestrator/tests/test_auto_pr.py
index adf7266d87..21a54b9973 100644
--- a/orchestrator/tests/test_auto_pr.py
+++ b/orchestrator/tests/test_auto_pr.py
@@ -525,6 +525,29 @@ def test_drops_step_when_only_symlinks_staged(self, tmp_path):
 
         assert "Move staged" not in body
 
+    def test_drops_step_when_staging_dir_is_symlink(self, tmp_path):
+        """When `.github-staging` itself is a symlink, no step is emitted.
+
+        ``Path.is_dir()`` follows symlinks, and the per-entry
+        ``is_symlink()`` guard only checks leaf components — so without
+        a guard on the staging dir itself, a malicious
+        ``.github-staging -> /etc`` would let ``rglob`` enumerate host
+        files into the manual-step file list. Regression-locks the
+        directory-as-symlink guard added alongside the per-entry one.
+        """
+        pipeline = _make_pipeline()
+        # Real directory with a regular file the rglob would otherwise pick up.
+        real_target = tmp_path / "real-target"
+        real_target.mkdir()
+        (real_target / "evil.yml").write_text("name: evil\n")
+        # `.github-staging` itself is a symlink to that directory.
+        (tmp_path / ".github-staging").symlink_to(real_target)
+
+        _title, body, _ = _build_pr_body(pipeline, tmp_path)
+
+        assert "Move staged" not in body
+        assert "evil.yml" not in body
+
 
 class TestAutoCreatePr:
     """Tests for _auto_create_pr."""
diff --git a/orchestrator/tests/test_pipeline_prompts.py b/orchestrator/tests/test_pipeline_prompts.py
index 96c0ff3680..4187de2dbe 100644
--- a/orchestrator/tests/test_pipeline_prompts.py
+++ b/orchestrator/tests/test_pipeline_prompts.py
@@ -3451,14 +3451,20 @@ def test_includes_github_staging_convention(self):
         concrete staging-path example is caught here.
         """
         section = _build_role_restrictions_section()
-        # Bare strings — both also occur elsewhere in the section.
+        # Bare-string assertions — these tokens also occur in the
+        # surrounding role-restrictions section (`.github/` is rendered
+        # in the plan-phase blocked-write list, `.github-staging/` is
+        # mentioned in the role-assignment paragraph, and `role: coder`
+        # appears in the role-assignment instruction). They confirm the
+        # tokens haven't been deleted from the section entirely but
+        # don't pin the staging-dir prose specifically.
         assert ".github-staging/" in section
         assert ".github/" in section
-        # Actionable guidance: the role assignment, a concrete staging
-        # path example, and the `.gitignore` warning. These are the
-        # things a planner needs to act on; a refactor that loses them
-        # would render the section informational rather than directive.
         assert "role: coder" in section
+        # Prose-pinning assertions: the concrete staging-path example
+        # and the `.gitignore` warning only appear in the staging-dir
+        # subsection. A refactor that drops the staging-dir prose will
+        # break here even if the bare-string tokens above survive.
         assert ".github-staging/workflows/ci.yml" in section
         assert ".gitignore" in section