Keep goal policy refusals typed across owner RPC

src/agent_comms/errors.py

@@ -1,10 +1,10 @@
 """Errors: declaration and persistence owners."""
 
 from __future__ import annotations
 
 from typing import TYPE_CHECKING, ClassVar
 
-from agent_comms.coordination_errors import DrainUnavailable
+from agent_comms.coordination_errors import DrainUnavailable, Refused
 from agent_comms_hooks import ToolRefusedError
 
 
@@ -57,6 +57,13 @@
     """A required relation between declarations cannot be proved."""
 
 
+class GoalRefused(Refused, RelationViolationError):
+    """A goal relation refused the action; preserve that refusal across owner RPC."""
+
+    def __init__(self, message: str) -> None:
+        super().__init__(self.INVALID_PARAMS, message)
+
+
 class UnclaimedError(RelationViolationError):
     """The requested bus sequence has no committed claim transition."""
 

src/agent_comms/goals.py

@@ -28,7 +28,7 @@
 )
 from .context_segments.wake_values import NoWakeGoalContext, WakeGoalContext, WakeGoalValue
 from .nominal.declared_family import DeclaredFamily
-from .errors import RejectedEnforcementError, RelationViolationError
+from .errors import GoalRefused, RejectedEnforcementError, RelationViolationError
 from .nominal.field_codec import (
     FieldCodec,
     GoalId,
@@ -265,15 +265,15 @@ def __str__(self) -> str:
         return f"goal {self.revision.id}@{self.revision.revision}"
 
 
-class GoalChangedError(RejectedEnforcementError):
+class GoalChangedError(GoalRefused, RejectedEnforcementError):
     """An observed goal changed before its checked action could run."""
 
     reason: ClassVar[str] = "goal changed"
 
     def __init__(self, thread: Thread, expected: GoalCheckpoint) -> None:
         self.thread = thread
         self.expected = expected
-        super().__init__()
+        super().__init__(str(self))
 
     def __str__(self) -> str:
         return (
@@ -501,15 +501,15 @@ class ProblemGoalOrder(GoalOrder):
     text: ClassVar[str] = "Complete the assigned problems."
 
     def require_completion(self, goal: Goal, goals: Goals, owner: ThreadIncarnation) -> None:
-        raise ValueError("Complete each problem with its checked delivered commit.")
+        raise GoalRefused("Complete each problem with its checked delivered commit.")
 
     def require_order(self, goal: Goal, goals: Goals, owner: ThreadIncarnation) -> None:
         if not goals.todos.for_goal(owner, goal.id):
             raise ValueError("The problem Goal has no assigned Todos.")
 
     def require_replacement(self, goal: Goal, goals: Goals, owner: ThreadIncarnation) -> None:
         if goals.todos.holds_pending(owner):
-            raise ValueError("Finish or release the assigned problems before replacing this Goal.")
+            raise GoalRefused("Finish or release the assigned problems before replacing this Goal.")
 
     def todos(self, goals: Goals, owner: ThreadIncarnation, goal_id: GoalId) -> tuple[Todo, ...]:
         return goals.todos.for_goal(owner, goal_id)
@@ -521,15 +521,15 @@ def goal_text(self, goal: Goal) -> str:
         return self.text
 
     def require_edit(self) -> None:
-        raise ValueError("A problem Goal's text is derived from its assigned problems.")
+        raise GoalRefused("A problem Goal's text is derived from its assigned problems.")
 
     def progress(self, goal: Goal, todos: Sequence[Todo]) -> str:
         """Each problem's order state and latest verdict, as Core records them on its Todo."""
         return "\n".join(todo.progress_line() for todo in todos)
 
     def require_model_block(self) -> None:
-        raise ValueError("A problem goal is not blocked by text: ask the commander to block the "
-                         "problem's Todo, or standby with wait_for naming the decider.")
+        raise GoalRefused("A problem goal is not blocked by text: ask the commander to block the "
+                          "problem's Todo, or standby with wait_for naming the decider.")
 
 
 @dataclass(frozen=True)
@@ -651,7 +651,7 @@ def require_active(self) -> Goal:
 
     def require_checkpoint(self, checkpoint: GoalRevision) -> Goal:
         if not PresentGoalCheckpoint(checkpoint).matches(self):
-            raise ValueError("The goal changed before this action; the action was not applied.")
+            raise GoalRefused("The goal changed before this action; the action was not applied.")
         return self.require()
 
     def matching(self, goal_id: GoalId) -> GoalDeclaration:
@@ -737,9 +737,6 @@ def standby(self, execution: GoalExecution) -> bool:
     def heading(self, execution: GoalExecution) -> str:
         return "No current goal"
 
-    def action_failure(self, error: Exception) -> str:
-        return "This goal has been cleared."
-
 
 @dataclass(frozen=True)
 class Goal(RecordMember, GoalDeclaration):
@@ -814,7 +811,7 @@ def peer_change_values(self) -> GoalPeerChange:
 
     def check_expected(self, current: GoalDeclaration) -> None:
         if self != current:
-            raise ValueError("Goal changed during resume; refresh its state.")
+            raise GoalRefused("Goal changed during resume; refresh its state.")
 
     def accepts_following_handoff(self) -> bool:
         return bool(self.state.items_with(Active, self))
@@ -835,9 +832,6 @@ def standby(self, execution: GoalExecution) -> bool:
     def heading(self, execution: GoalExecution) -> str:
         return execution.heading(self.revision)
 
-    def action_failure(self, error: Exception) -> str:
-        return str(error)
-
     unavailable_heading: ClassVar[str] = "Goal state unavailable · last confirmed snapshot"
 
     def continued_by(self, current: GoalDeclaration) -> GoalDeclaration:
@@ -853,7 +847,7 @@ def require_id(self, goal_id: GoalId) -> Goal:
 
     def require_state(self, expected: GoalState) -> None:
         if self.state != expected:
-            raise ValueError(self.state.activity_refusal())
+            raise GoalRefused(self.state.activity_refusal())
 
     def matching(self, goal_id: GoalId) -> GoalDeclaration:
         return self if self.id == goal_id else NoGoal()

src/agent_comms/ui_model/goal.py

@@ -33,9 +33,6 @@ def details_heading(self, execution: GoalExecution) -> str:
     @abstractmethod
     def standby(self, execution: GoalExecution) -> bool: ...
 
-    @abstractmethod
-    def action_failure(self, error: Exception) -> str: ...
-
 
 @dataclass(frozen=True)
 class GoalUnavailable(GoalDisplay):
@@ -52,8 +49,5 @@ def snapshot(self) -> GoalDeclaration:
     def standby(self, execution: GoalExecution) -> bool:
         return False
 
-    def action_failure(self, error: Exception) -> str:
-        return str(error)
-
     def heading(self, execution: GoalExecution) -> str:
         return self.last_confirmed.unavailable_heading

tests/test_held_problem_orders.py

@@ -14,7 +14,11 @@
 from agent_comms.nominal.field_codec import FieldCodec, GoalId, OrderId
 from agent_comms.installer.practice import PracticeCopy
 from agent_comms.locked_store import RootFile
-from agent_comms.goals import CountCheckResult, OrderCheck
+from agent_comms.coordination_errors import Refused
+from agent_comms.errors import GoalRefused
+from agent_comms.goal_actions import ClearGoalAction, SetGoalAction
+from agent_comms.goals import CountCheckResult, OrderCheck, ProblemGoalOrder
+from agent_comms.owner_protocol import OwnerFailure, OwnerLine
 from agent_comms.recorded_time import RecordedEventTime
 from agent_comms.todos import (
     DeliveredProblemOrder, MergedListedPullRequest, OrderMessage, PullRequestCommit,
@@ -64,6 +68,24 @@ def held_commander(pr_commander, cited_source):
     assert source.comms.goals.todos.get(todo.id) == original
 
 
+def test_held_goal_replacement_is_a_preserved_refusal_without_writes(held_commander):
+    ctx = held_commander
+    comms = ctx.comms
+    todo = comms.goals.todos.get("P1")
+    goal = comms.goals.update_goal("worker", SetGoalAction(
+        text=ProblemGoalOrder.text, order=ProblemGoalOrder(), goal_id=todo.held().generation,
+    ))
+    for action in (ClearGoalAction(), SetGoalAction(text="Replace the held work")):
+        with pytest.raises(GoalRefused) as denied:
+            comms.goals.update_goal("worker", action)
+        with pytest.raises(Refused) as reply:
+            OwnerLine.read(OwnerFailure.refused(denied.value).wire()).reply()
+        assert reply.value.code == denied.value.code == Refused.INVALID_PARAMS
+        assert str(reply.value) == str(denied.value)
+        assert comms.registry.require("worker").goal == goal
+        assert comms.goals.todos.get("P1") == todo
+
+
 def link_pull_request(ctx, pid, number):
     """The order's recorded PR link, as `order pr` writes it."""
     store = ctx.comms.goals.todos

tests/test_report_goal_evidence.py

@@ -1,10 +1,14 @@
 """Report completion verifies original content before changing the real goal."""
 
 import hashlib
+from functools import partial
 
 import pytest
 
+from agent_comms.coordination_errors import Refused
+from agent_comms.errors import GoalRefused
 from agent_comms.nominal.field_codec import FieldCodec
+from agent_comms.owner_protocol import OwnerFailure, OwnerLine
 from agent_comms.goal_actions import SetGoalAction
 from agent_comms.goals import ProblemGoalOrder
 from agent_comms.progress_claims import ProgressClaim, ReportClaim
@@ -67,5 +71,15 @@ def test_report_is_not_implementation_order_completion(comms, tmp_path, report_g
     path.write_text("Delivered analysis, not an implemented fix")
     claim = ReportClaim(FileProvenance(str(path), hashlib.sha256(path.read_bytes()).hexdigest()))
     ProgressClaim.require_all((claim,), comms, tmp_path)
-    with pytest.raises(ValueError, match="checked delivered commit"):
-        ProblemGoalOrder().require_completion(report_goal, comms.goals, comms.registry.require("editor").incarnation)
+    policy = ProblemGoalOrder()
+    for operation in (
+        partial(policy.require_completion, report_goal, comms.goals, comms.registry.require("editor").incarnation),
+        policy.require_edit,
+        policy.require_model_block,
+    ):
+        with pytest.raises(GoalRefused) as denied:
+            operation()
+        with pytest.raises(Refused) as reply:
+            OwnerLine.read(OwnerFailure.refused(denied.value).wire()).reply()
+        assert reply.value.code == denied.value.code == Refused.INVALID_PARAMS
+        assert str(reply.value) == str(denied.value)