[issue-2777][slice-2/4] Scaffold + PR-phase deletions (slice-1b)

gateway/gateway.py

@@ -1102,15 +1102,11 @@ def config_reload() -> Response:
 # secret gates ``/api/v1/sessions/create``), so this exemption is not reachable
 # from a sandboxed agent's session token.
 _SLICE_INTEGRATION_BRANCH_RE = re.compile(r"^egg/[A-Za-z0-9][A-Za-z0-9_-]*/(?:slice|phase)-\d+$")
-# Context-branch shape for the synthetic-session exemption (#2548).
-# Matches ``egg/<base>/context`` — the doc-only branch that carries
-# refine/plan analysis docs and BRC consensus history so the strategic
-# narrative reaches ``main``.  The orchestrator creates this branch via the
-# same synthetic-session push path as slice integration branches; the
-# pipeline-session push block from #2028 must therefore exempt it the same
-# way.  Same trust model as ``_SLICE_INTEGRATION_BRANCH_RE``: only
-# launcher-gated synthetic sessions can ever opt into the exemption.
-_CONTEXT_BRANCH_RE = re.compile(r"^egg/[A-Za-z0-9][A-Za-z0-9_-]*/context$")
+# NOTE: ``_CONTEXT_BRANCH_RE`` (the synthetic-session exemption for
+# ``egg/<base>/context`` from #2548) was removed in #2777 (cq-2 / cq-4).
+# The dedicated context branch is gone; the context PR now opens on
+# ``egg/<id>/work → main`` directly and that branch already lives on
+# the pipeline-session push-allow list.
 
 
 @app.route("/api/v1/git/push", methods=["POST"])
@@ -1332,24 +1328,12 @@ def git_push() -> tuple[Response, int] | Response:
     # ``/api/v1/sessions/create`` endpoint is gated by ``require_launcher_auth``),
     # so a sandboxed agent's session token cannot reach this branch.
     #
-    # Context-branch creation (#2548) follows the same pattern: the
-    # orchestrator creates ``egg/<base>/context`` from the pipeline's base
-    # branch via a synthetic-session push and then commits the refine/plan
-    # artifacts onto it.  Both shapes share the exemption — the synthetic-
-    # session check below is the load-bearing trust gate; the regex match
-    # only narrows which branches the exemption covers.
-    # ``is_slice_integration_push`` is a legacy variable name kept for the
-    # downstream audit-trail filter at the second event below; it now
-    # covers both slice-integration AND context-branch synthetic pushes.
-    # The branch-shape distinction is captured by ``is_context_push`` so
-    # the second exemption event below can emit a precise ``exempt_type``
-    # ("context_branch" vs "slice_integration_branch") and SIEM pipelines
-    # keying on ``exempt_type`` can tell them apart (#2548 review).
+    # The legacy ``egg/<base>/context`` context-branch exemption (#2548) was
+    # removed in #2777 (cq-2 / cq-4): the dedicated context branch is gone
+    # and the context PR now opens on ``egg/<id>/work → main`` directly,
+    # which is already covered by the pipeline-session push-allow list.
     is_slice_integration_push = False
-    is_context_push = False
-    if not is_infrastructure_push and (
-        _SLICE_INTEGRATION_BRANCH_RE.match(branch) or _CONTEXT_BRANCH_RE.match(branch)
-    ):
+    if not is_infrastructure_push and _SLICE_INTEGRATION_BRANCH_RE.match(branch):
         # ``Session.synthetic`` is a ``bool`` (default ``False``); only an
         # orchestrator-issued session can carry ``synthetic=True`` because
         # ``/api/v1/sessions/create`` is gated on the launcher secret.  Use
@@ -1360,7 +1344,6 @@ def git_push() -> tuple[Response, int] | Response:
         if hasattr(g, "session") and getattr(g.session, "synthetic", False) is True:
             is_slice_integration_push = True
             is_infrastructure_push = True
-            is_context_push = bool(_CONTEXT_BRANCH_RE.match(branch))
             audit_log(
                 "push_slice_integration_exempt",
                 "git_push",
@@ -1371,10 +1354,7 @@ def git_push() -> tuple[Response, int] | Response:
                     "refspec": refspec,
                     "branch": branch,
                     "reason": (
-                        "Synthetic-session context branch push — "
-                        "orchestrator infrastructure (#2548)"
-                        if is_context_push
-                        else "Synthetic-session slice integration branch push — "
+                        "Synthetic-session slice integration branch push — "
                         "orchestrator infrastructure (#2368)"
                     ),
                 },
@@ -1389,12 +1369,6 @@ def git_push() -> tuple[Response, int] | Response:
         if is_infrastructure_push or is_ckpt_repo:
             if is_ckpt_repo:
                 exempt_type = "checkpoint_repo"
-            elif is_context_push:
-                # Distinct ``exempt_type`` for context-branch pushes so
-                # SIEM pipelines that filter by the generic
-                # ``push_infrastructure_exempt`` event can tell them
-                # apart from slice-integration pushes (#2548 review).
-                exempt_type = "context_branch"
             elif is_slice_integration_push:
                 exempt_type = "slice_integration_branch"
             else:

gateway/phase_filter.py

@@ -523,16 +523,13 @@ def _get_default_permissions(self) -> dict[PipelinePhase, PhasePermissions]:
                 ],
                 exit_requires="reviewer",
             ),
-            PipelinePhase.PR: PhasePermissions(
-                allowed_operations=[
-                    Operation(OperationType.GH, "pr create*", "Create PRs"),
-                    Operation(OperationType.GH, "pr edit *", "Edit PRs"),
-                    Operation(OperationType.GIT, "push *", "Push code"),
-                    Operation(OperationType.EGG_CONTRACT, "show *", "View contract state"),
-                ],
-                blocked_operations=[],
-                exit_requires="human",
-            ),
+            # The PR phase was hard-removed in #2777 (cq-4 / TASK-2-2);
+            # no PhasePermissions row is registered for it. The
+            # orchestrator's ``GatewayClient.create_pr`` now registers
+            # its synthetic session WITHOUT a phase value, hitting the
+            # gh_pr_create handler's explicit "No phase set - allow by
+            # default" branch (``gateway.py:3685``), so the carve-out no
+            # longer needs an entry here.
         }
 
     def _get_default_file_restrictions(self) -> list[FileRestriction]:
@@ -639,10 +636,10 @@ def _get_default_phase_file_restrictions(
                     "checkpoints, agent anchors, and reviews only"
                 ),
             ),
-            PipelinePhase.PR: PhaseFileRestriction(
-                allowed_patterns=["*"],
-                description="PR phase can push everything",
-            ),
+            # The PR phase was hard-removed in #2777 (cq-4 / TASK-2-2);
+            # no PhaseFileRestriction row remains. See the matching
+            # PhasePermissions deletion in ``_get_default_permissions``
+            # for the synthetic-session carve-out rationale.
         }
 
     def get_file_restrictions(self) -> list[FileRestriction]:

gateway/phase_transition.py

@@ -50,8 +50,10 @@ class TransitionRole(StrEnum):
     PipelinePhase.REFINE: [PipelinePhase.PLAN],
     PipelinePhase.PLAN: [PipelinePhase.IMPLEMENT, PipelinePhase.APPLY],
     PipelinePhase.APPLY: [PipelinePhase.IMPLEMENT],
-    PipelinePhase.IMPLEMENT: [PipelinePhase.PR],
-    PipelinePhase.PR: [],  # Terminal state - no automatic transitions
+    # IMPLEMENT is now terminal — the PR phase was removed in #2777
+    # (cq-4). Callers requesting ``target='pr'`` are default-denied at
+    # the transition validator.
+    PipelinePhase.IMPLEMENT: [],
 }
 
 

gateway/tests/test_gateway.py

@@ -4341,7 +4341,14 @@ def test_session_phase_update_validates_phase(self, client, launcher_auth_header
         assert "invalid" in data["message"].lower()
 
     def test_session_phase_update_success(self, client, launcher_auth_headers, tmp_path):
-        """Session phase update succeeds with valid parameters."""
+        """Session phase update succeeds with valid parameters.
+
+        Pre-#2777 slice-2 this used ``"pr"`` as the destination phase
+        (the legacy PR phase). The PR phase was deleted by cq-4 /
+        TASK-2-2; we drive the same code path with the surviving
+        ``IMPLEMENT`` value here (any valid post-slice-2 phase
+        works).
+        """
         from session_manager import SessionManager
 
         # Create a real session manager with temp file
@@ -4350,28 +4357,33 @@ def test_session_phase_update_success(self, client, launcher_auth_headers, tmp_p
             container_id="test-container",
             container_ip="172.18.0.5",
             mode="private",
-            phase="implement",
+            phase="plan",
         )
 
         with patch.object(gateway, "get_session_manager", return_value=manager):
             response = client.patch(
                 f"/api/v1/sessions/{token}/phase",
                 headers=launcher_auth_headers,
-                data=json.dumps({"phase": "pr"}),
+                data=json.dumps({"phase": "implement"}),
                 content_type="application/json",
             )
 
             assert response.status_code == 200
             data = json.loads(response.data)
             assert data["success"] is True
-            assert data["data"]["phase"] == "pr"
+            assert data["data"]["phase"] == "implement"
 
             # Verify session was updated
             session = manager.get_session(token)
-            assert session.phase == "pr"
+            assert session.phase == "implement"
 
     def test_session_phase_update_session_not_found(self, client, launcher_auth_headers):
-        """Session phase update returns 404 for unknown session."""
+        """Session phase update returns 404 for unknown session.
+
+        See ``test_session_phase_update_success`` for the post-#2777
+        slice-2 substitution of ``"implement"`` for the deleted
+        ``"pr"`` phase value.
+        """
         with patch.object(gateway, "get_session_manager") as mock_get_manager:
             mock_manager = MagicMock()
             mock_manager.update_phase.return_value = False
@@ -4380,7 +4392,7 @@ def test_session_phase_update_session_not_found(self, client, launcher_auth_head
             response = client.patch(
                 "/api/v1/sessions/unknown-token/phase",
                 headers=launcher_auth_headers,
-                data=json.dumps({"phase": "pr"}),
+                data=json.dumps({"phase": "implement"}),
                 content_type="application/json",
             )
 

gateway/tests/test_phase_api.py

@@ -330,17 +330,22 @@ def test_advance_phase_unauthorized(self, client, mock_contract):
         assert "cannot exit" in data["message"].lower() or "denied" in data["message"].lower()
 
     def test_advance_phase_terminal_state(self, client, auth_headers):
-        """Cannot advance from PR phase (terminal)."""
+        """Cannot advance from IMPLEMENT phase (terminal post-#2777 slice-2).
+
+        Pre-slice-2 the terminal phase was PR. Slice-2 deletes the PR
+        phase; IMPLEMENT is now terminal. The endpoint must reject the
+        advance attempt with a clear "terminal" message.
+        """
         from egg_contracts.models import Contract, IssueInfo, PipelinePhase
 
         terminal_contract = Contract(
-            schemaVersion="1.0",
+            schemaVersion="1.2",
             issue=IssueInfo(
                 number=123,
                 title="Test Issue",
                 url="https://github.com/test/repo/issues/123",
             ),
-            current_phase=PipelinePhase.PR,
+            current_phase=PipelinePhase.IMPLEMENT,
         )
 
         with patch("phase_api.load_contract", return_value=terminal_contract):
@@ -353,7 +358,40 @@ def test_advance_phase_terminal_state(self, client, auth_headers):
         assert response.status_code == 400
         data = response.get_json()
         assert data["success"] is False
-        assert "terminal" in data["message"].lower()
+        assert "terminal" in data["message"].lower(), (
+            f"Expected 'terminal' in rejection message for IMPLEMENT; got: {data!r}"
+        )
+
+    def test_advance_phase_target_pr_default_denied(self, client, auth_headers):
+        """``advance_phase target='pr'`` must default-deny (#2777 slice-2 AC-4c).
+
+        Even from a non-terminal phase (e.g. IMPLEMENT pre-slice-2 was
+        IMPLEMENT→PR), explicitly targeting the deleted ``'pr'`` phase
+        must be rejected with a 400.
+        """
+        from egg_contracts.models import Contract, IssueInfo, PipelinePhase
+
+        implement_contract = Contract(
+            schemaVersion="1.2",
+            issue=IssueInfo(
+                number=123,
+                title="Test Issue",
+                url="https://github.com/test/repo/issues/123",
+            ),
+            current_phase=PipelinePhase.IMPLEMENT,
+        )
+
+        with patch("phase_api.load_contract", return_value=implement_contract):
+            response = client.post(
+                "/api/v1/phase/advance",
+                headers=auth_headers,
+                json={"issue_number": 123, "target": "pr"},
+            )
+
+        assert response.status_code == 400, (
+            f"advance_phase target='pr' must default-deny; got "
+            f"{response.status_code}: {response.data!r}"
+        )
 
     def test_advance_phase_missing_issue(self, client, auth_headers):
         """Advance phase without issue number."""
@@ -576,20 +614,33 @@ def test_allowed_repo_path_accepted(self, client, auth_headers, mock_contract):
 class TestReviewerPhaseTransitionIntegration:
     """Integration tests for reviewer phase transitions.
 
-    These tests use real contract mutations (not mocked) to verify
-    that reviewer can actually advance from implement to PR phase.
+    Pre-#2777 slice-2 this class verified the reviewer-driven
+    ``IMPLEMENT → PR`` advance with real contract mutation. Slice-2
+    deletes the PR phase entirely (cq-4 / TASK-2-2); IMPLEMENT is the
+    terminal phase and any advance attempt from IMPLEMENT must be
+    rejected. The replacement test below pins that contract: a real
+    on-disk IMPLEMENT contract advance-attempt returns 400 and the
+    contract's ``current_phase`` is unchanged.
     """
 
-    def test_reviewer_can_advance_implement_to_pr(self, client):
-        """Reviewer can advance from implement to PR phase with real mutation."""
+    def test_reviewer_cannot_advance_from_implement_post_slice_2(self, client):
+        """Real-mutation regression: IMPLEMENT advance is terminal-rejected.
+
+        Drives the same integration-shape path as the deleted
+        ``test_reviewer_can_advance_implement_to_pr``: real contract
+        on disk, real reviewer session, real ``/api/v1/phase/advance``
+        call. The expected response is a terminal-state rejection and
+        the on-disk contract must still be at IMPLEMENT after the call.
+        """
         import tempfile
 
-        from egg_contracts import save_contract
+        from egg_contracts import load_contract, save_contract
         from egg_contracts.models import Contract, IssueInfo, PipelinePhase
 
-        # Create a contract in implement phase
+        # Create a contract in implement phase (the new terminal phase
+        # under slice-2; schema bumped to 1.2 to match the new default).
         contract = Contract(
-            schemaVersion="1.0",
+            schemaVersion="1.2",
             issue=IssueInfo(
                 number=999,
                 title="Test Issue",
@@ -598,12 +649,10 @@ def test_reviewer_can_advance_implement_to_pr(self, client):
             current_phase=PipelinePhase.IMPLEMENT,
         )
 
-        # Save to temp directory
         with tempfile.TemporaryDirectory() as tmpdir:
             tmppath = Path(tmpdir)
             save_contract(contract, tmppath)
 
-            # Create a session with reviewer role
             mock_session = MagicMock()
             mock_session.mode = "public"
             mock_session.container_id = "test-container"
@@ -625,7 +674,6 @@ def test_reviewer_can_advance_implement_to_pr(self, client):
 
             current_session_manager = sys.modules.get("session_manager", session_manager)
 
-            # Patch the allowed paths to include our temp directory
             with (
                 patch.object(
                     current_session_manager,
@@ -649,16 +697,18 @@ def test_reviewer_can_advance_implement_to_pr(self, client):
                     },
                 )
 
-            assert response.status_code == 200, (
-                f"Expected 200, got {response.status_code}: {response.get_json()}"
+            assert response.status_code == 400, (
+                f"After slice-2, IMPLEMENT is terminal; advance must "
+                f"return 400. Got {response.status_code}: {response.get_json()}"
             )
             data = response.get_json()
-            assert data["success"] is True
-            assert data["data"]["from_phase"] == "implement"
-            assert data["data"]["to_phase"] == "pr"
-
-            # Verify the contract was actually updated
-            from egg_contracts import load_contract
+            assert data["success"] is False
+            assert "terminal" in data["message"].lower(), (
+                f"Expected 'terminal' in rejection message; got: {data!r}"
+            )
 
-            updated_contract = load_contract(999, tmppath)
-            assert updated_contract.current_phase == PipelinePhase.PR
+            # The on-disk contract must be unchanged.
+            unchanged = load_contract(999, tmppath)
+            assert unchanged.current_phase == PipelinePhase.IMPLEMENT, (
+                f"Rejected advance must not mutate on-disk phase; got {unchanged.current_phase!r}"
+            )