From 890cca9e8901622dd295b990255064c8beb970e5 Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Sat, 31 Oct 2026 17:01:47 -0400 Subject: [PATCH] Preserve declared goal refusals through owner RPC --- src/agent_comms/errors.py | 9 ++++++++- src/agent_comms/goals.py | 28 +++++++++++----------------- src/agent_comms/ui_model/goal.py | 6 ------ tests/test_held_problem_orders.py | 24 +++++++++++++++++++++++- tests/test_report_goal_evidence.py | 18 ++++++++++++++++-- 5 files changed, 58 insertions(+), 27 deletions(-) diff --git a/src/agent_comms/errors.py b/src/agent_comms/errors.py index dfcec62727..1e581fa2ad 100644 --- a/src/agent_comms/errors.py +++ b/src/agent_comms/errors.py @@ -4,7 +4,7 @@ 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 @@ class RelationViolationError(ValueError, DrainUnavailable): """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.""" diff --git a/src/agent_comms/goals.py b/src/agent_comms/goals.py index 69c39a8c37..bd28ce28da 100644 --- a/src/agent_comms/goals.py +++ b/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,7 +265,7 @@ 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" @@ -273,7 +273,7 @@ class GoalChangedError(RejectedEnforcementError): 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,7 +501,7 @@ 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): @@ -509,7 +509,7 @@ def require_order(self, goal: Goal, goals: Goals, owner: ThreadIncarnation) -> N 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() diff --git a/src/agent_comms/ui_model/goal.py b/src/agent_comms/ui_model/goal.py index aea6e72475..0d99bee89a 100644 --- a/src/agent_comms/ui_model/goal.py +++ b/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 diff --git a/tests/test_held_problem_orders.py b/tests/test_held_problem_orders.py index c6b60499e2..01ca4b93a8 100644 --- a/tests/test_held_problem_orders.py +++ b/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 diff --git a/tests/test_report_goal_evidence.py b/tests/test_report_goal_evidence.py index 285e844052..ccdf674450 100644 --- a/tests/test_report_goal_evidence.py +++ b/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)