[F101] enforce PR-open state gate on gateway open_pr (parity with HTTP path)

roboco/foundation/policy/lifecycle.py

@@ -951,6 +951,46 @@ PRECONDITION_NON_TERMINAL = Precondition(
 )
 
 
+# The set of states from which a PR may be opened — the lifecycle-owned canon.
+# The HTTP PR-create path (GitService._assert_pr_create_allowed) and the gateway
+# ``open_pr`` intent must agree on this, so it lives here (the policy layer) as
+# the single source and the service derives its str set from it. A PR opens
+# during active dev (in_progress / verifying), the doc phase
+# (awaiting_documentation), QA review (awaiting_qa), or a rework cycle
+# (needs_revision) — never from claim/pause/block/terminal, which the HTTP path
+# already blocked but the gateway ``open_pr`` (composes=() → no source-status
+# gate) historically did not (F101).
+PR_OPEN_STATES: frozenset[Status] = frozenset(
+    {
+        Status.IN_PROGRESS,
+        Status.VERIFYING,
+        Status.AWAITING_QA,
+        Status.AWAITING_DOCUMENTATION,
+        Status.NEEDS_REVISION,
+    }
+)
+
+
+def _p_pr_open_state(task: Any, _agent: Any, _ctx: Any) -> bool:
+    """True iff the task is in a PR-open-eligible state (see ``PR_OPEN_STATES``)."""
+    status = getattr(task, "status", None)
+    value = status.value if isinstance(status, Status) else str(status)
+    return value in {s.value for s in PR_OPEN_STATES}
+
+
+PRECONDITION_PR_OPEN_STATE = Precondition(
+    key="pr_open_state",
+    check=_p_pr_open_state,
+    remediate=(
+        "open_pr is only valid during active dev states "
+        "(in_progress / verifying / awaiting_qa / awaiting_documentation / "
+        "needs_revision); move the task into one of those first"
+    ),
+    missing_token="pr_open_state",
+    rejection_kind="invalid_state",
+)
+
+
 _INTENT_VERBS: dict[str, IntentSpec] = {
     # Phase 1: developer verbs
     "give_me_work": IntentSpec(
@@ -1022,6 +1062,7 @@ _INTENT_VERBS: dict[str, IntentSpec] = {
         composes=(),
         extra_preconditions=(
             PRECONDITION_OWNERSHIP,
+            PRECONDITION_PR_OPEN_STATE,
             PRECONDITION_COMMITS,
             PRECONDITION_NO_PR,
         ),

roboco/services/git.py

@@ -9,6 +9,7 @@ from __future__ import annotations
 
 import asyncio
 import base64
+import contextlib
 import json
 import os
 import re
@@ -47,6 +48,7 @@ from roboco.exceptions import (
     GitTimeoutError,
     MergeConflictError,
 )
+from roboco.foundation.policy import lifecycle
 from roboco.models.base import AgentRole, TaskStatus
 from roboco.services.base import (
     BaseService,
@@ -141,12 +143,10 @@ def _remove_stale_git_locks(workspace: Path) -> None:
         return
     try:
         for lock in git_dir.rglob("*.lock"):
-            try:
+            # A lock a real process just grabbed, or a permission issue —
+            # leave it. The TTL/next-op path is the backstop.
+            with contextlib.suppress(OSError):
                 lock.unlink()
-            except OSError:
-                # A lock a real process just grabbed, or a permission issue —
-                # leave it. The TTL/next-op path is the backstop.
-                pass
     except OSError:
         return
 
@@ -2509,14 +2509,11 @@ class GitService(BaseService):
             "updated_fields": updated,
         }
 
+    # PR-open-eligible states — the lifecycle policy owns the canon
+    # (``PR_OPEN_STATES``); the HTTP path derives its str set from it so the
+    # gateway ``open_pr`` spec gate and this HTTP gate can never drift.
     _PR_OPEN_STATES: ClassVar[frozenset[str]] = frozenset(
-        {
-            TaskStatus.IN_PROGRESS.value,
-            TaskStatus.VERIFYING.value,
-            TaskStatus.AWAITING_QA.value,
-            TaskStatus.AWAITING_DOCUMENTATION.value,
-            TaskStatus.NEEDS_REVISION.value,
-        }
+        s.value for s in lifecycle.PR_OPEN_STATES
     )
 
     @staticmethod

tests/foundation/test_lifecycle_spec.py

@@ -566,6 +566,112 @@ def test_can_invoke_intent_developer_open_pr_no_commits_tracing_gap() -> None:
     assert "commits>=1" in d.missing
 
 
+# --------------------------------------------------------------------------- #
+# F101: open_pr must enforce the PR-open state gate (parity with the HTTP path)
+# --------------------------------------------------------------------------- #
+
+
+def _owned_task(**overrides: Any) -> SimpleNamespace:
+    """A task owned by ``actor`` with commits and no prior PR — only the state
+    gate can fail, isolating the PR-open-state precondition."""
+    actor = overrides.pop("actor_id", uuid4())
+    return _stub_task(
+        assigned_to=actor,
+        commits=["abc123"],
+        pr_number=None,
+        **overrides,
+    )
+
+
+def test_open_pr_rejected_on_claimed_task() -> None:
+    """F101: ``open_pr`` has ``composes=()`` so the spec gate applied NO
+    source-status check — a dev could open a PR from ``claimed`` (before
+    ``in_progress``), skipping the active-dev state the HTTP path's
+    ``_assert_pr_create_allowed`` enforces. The state gate now rejects it."""
+    actor = uuid4()
+    d = spec.can_invoke_intent(
+        spec.Role.DEVELOPER,
+        "open_pr",
+        _owned_task(status="claimed", actor_id=actor),
+        context=spec.Context(actor_id=actor),
+    )
+    assert d.allowed is False
+    assert d.rejection_kind == "invalid_state"
+
+
+def test_open_pr_rejected_on_paused_task() -> None:
+    actor = uuid4()
+    d = spec.can_invoke_intent(
+        spec.Role.DEVELOPER,
+        "open_pr",
+        _owned_task(status="paused", actor_id=actor),
+        context=spec.Context(actor_id=actor),
+    )
+    assert d.allowed is False
+    assert d.rejection_kind == "invalid_state"
+
+
+def test_open_pr_rejected_on_blocked_task() -> None:
+    actor = uuid4()
+    d = spec.can_invoke_intent(
+        spec.Role.DEVELOPER,
+        "open_pr",
+        _owned_task(status="blocked", actor_id=actor),
+        context=spec.Context(actor_id=actor),
+    )
+    assert d.allowed is False
+    assert d.rejection_kind == "invalid_state"
+
+
+def test_open_pr_rejected_on_completed_task() -> None:
+    """A completed task is terminal — opening a PR on it is nonsensical."""
+    actor = uuid4()
+    d = spec.can_invoke_intent(
+        spec.Role.DEVELOPER,
+        "open_pr",
+        _owned_task(status="completed", actor_id=actor),
+        context=spec.Context(actor_id=actor),
+    )
+    assert d.allowed is False
+    assert d.rejection_kind == "invalid_state"
+
+
+@pytest.mark.parametrize(
+    "status",
+    [
+        "in_progress",
+        "verifying",
+        "awaiting_qa",
+        "awaiting_documentation",
+        "needs_revision",
+    ],
+)
+def test_open_pr_allowed_in_pr_open_states(status: str) -> None:
+    """Regression guard: every PR-open-eligible state still lets the owner open
+    a PR — the new state gate must not over-restrict the legitimate path."""
+    actor = uuid4()
+    d = spec.can_invoke_intent(
+        spec.Role.DEVELOPER,
+        "open_pr",
+        _owned_task(status=status, actor_id=actor),
+        context=spec.Context(actor_id=actor),
+    )
+    assert d.allowed is True, f"open_pr should be allowed from {status}"
+
+
+def test_open_pr_state_gate_takes_priority_over_unowned() -> None:
+    """A non-owner in a wrong state: ownership (not_authorized) is checked
+    before state, mirroring the HTTP path's assignee-first ordering."""
+    d = spec.can_invoke_intent(
+        spec.Role.DEVELOPER,
+        "open_pr",
+        _owned_task(status="claimed", actor_id=uuid4()),
+        context=spec.Context(actor_id=uuid4()),  # different actor -> not owner
+    )
+    assert d.allowed is False
+    assert d.rejection_kind == "not_authorized"
+
+
 def test_escalate_up_rejected_on_completed_task() -> None:
     """F043: a PM must not resurrect a COMPLETED task via escalate_up.