From 3d777a43db72a6b2108617b49c208d17c4a4b8fe Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Sun, 1 Nov 2026 02:35:43 -0500 Subject: [PATCH 1/2] Allow goal decisions after progress in the same turn Fixes #3830. Preserve the original reporting-turn identity and existing state/input checks; native terminal settlement stays with the attempt ledger. --- src/agent_comms/goal_actions.py | 8 +-- tests/test_goal_resume.py | 86 +++++++++++++++++++++------------ 2 files changed, 60 insertions(+), 34 deletions(-) diff --git a/src/agent_comms/goal_actions.py b/src/agent_comms/goal_actions.py index ade8073cb0..6420e5a80e 100644 --- a/src/agent_comms/goal_actions.py +++ b/src/agent_comms/goal_actions.py @@ -238,12 +238,14 @@ def before_transition(self, goal: Goal, ctx: GoalActionContext) -> None: class GoalReportAction(TransitionGoalAction): - """A model's report consumes this goal's slot; recovery commands do not.""" + """Keep the original reporting turn while progress and decisions evolve. + + Native terminal settlement belongs to TurnGoalAccount and the attempt ledger, + not to how many times this turn updates its goal. + """ def change(self, goal: Goal, ctx: GoalActionContext) -> Goal: if ctx.model_report: - if goal.reports_turn(ctx.reported_turn): - raise ValueError("This goal was already reported in this turn.") goal = replace(goal, reported_turn=ctx.reported_turn) return super().change(goal, ctx) diff --git a/tests/test_goal_resume.py b/tests/test_goal_resume.py index db3145706f..2c187254f1 100644 --- a/tests/test_goal_resume.py +++ b/tests/test_goal_resume.py @@ -26,6 +26,7 @@ from agent_comms.goal_attempts import GoalAttemptStore from agent_comms.goal_generation import BlockedGeneration, CancelledGeneration, CompletedGeneration from agent_comms.goals import Goal, NoGoal +from agent_comms.goal_states import ActiveGoal, BlockedGoal, CompletedGoal from agent_comms.lifecycle_capability import Active from agent_comms.reported_turn import ReportedTurn from agent_comms.threads import Thread @@ -63,27 +64,43 @@ def test_same_id_resume_from_paused(comms, monkeypatch): assert comms.registry.require("owner").goal.id == started["id"] -def test_second_goal_report_in_one_turn_is_rejected(comms, monkeypatch, tmp_path): - monkeypatch.setenv("AGENT_COMMS_THREAD", "owner") - comms.registry.declare( - Thread('owner', frozenset(), Checkout(tmp_path), process_identity=ProcessIdentity.capture(os.getpid())) - ) +@pytest.mark.parametrize("status,extra,expected", [ + ("active", {}, ActiveGoal()), + ("standby", {"wait_for": ["dependency"]}, ActiveGoal()), + ("blocked", {}, BlockedGoal("another step")), + ("completed", {}, CompletedGoal()), +]) +def test_later_progress_or_decision_keeps_the_original_turn(comms, tmp_path, status, extra, expected): + for name in ("owner", "dependency"): + comms.registry.declare(Thread( + name, frozenset(), Checkout(tmp_path), + process_identity=ProcessIdentity.capture(os.getpid()), + )) goal = invoke_tool(comms, "comms_set_goal", {"text": "finish this"})["goal"] - comms.agents.begin_turn("owner", TurnId('d7f9429f60abc2787fc3e9717054ea41')) - - first = invoke_tool( - comms, - "comms_goal", - {"goal_id": goal["id"], "status": "active", "progress": "one step"}, - ) - with pytest.raises(ValueError): - invoke_tool( - comms, - "comms_goal", - {"goal_id": goal["id"], "status": "active", "progress": "another step"}, - ) - assert first["goal"]["progress"] == "one step" - assert comms.registry.require("owner").goal.progress == "one step" + goal_id = FieldCodec.decode(GoalId, goal["id"]) + turn = TurnId('d7f9429f60abc2787fc3e9717054ea41') + comms.agents.begin_turn("owner", turn) + generation = comms.goals.open_goal_store().require_generation(goal_id) + report = tmp_path / "report.md" + report.write_text("Delivered report.\n") + first = invoke_tool(comms, "comms_goal", { + "goal_id": goal["id"], "status": "active", "progress": "one step", + })["goal"] + result = invoke_tool(comms, "comms_goal", { + "goal_id": goal["id"], "status": status, "progress": "another step", **extra, + "claims": [{"kind": "report", "source": { + "kind": "file", "path": str(report), "sha256": sha256(report.read_bytes()).hexdigest(), + }}], + })["goal"] + saved = wire(comms.root).registry.require("owner").goal + assert result["state"] == FieldCodec.encode(expected) + assert saved.progress == result["progress"] == "another step" + assert saved.id == goal_id and saved.revision == first["revision"] + 1 + assert saved.reports_turn(ReportedTurn(turn)) + assert comms.registry.require("owner").turn_record.read().reported_turn == ReportedTurn(turn) + assert tuple(target.incarnation.name for target in comms.goals.waits.resolve(saved).targets) == tuple( + extra.get("wait_for", ())) + assert comms.goals.open_goal_store().require_generation(goal_id) == generation @pytest.mark.parametrize("first_status", ["active", "completed"]) @@ -117,13 +134,13 @@ def test_each_new_goal_can_report_in_same_turn(comms, monkeypatch, tmp_path, fir {"goal_id": replacement["id"], "status": "active", "progress": "second"}, ) reopened = wire(comms.root) - with pytest.raises(ValueError, match="already reported"): - invoke_tool( - reopened, - "comms_goal", - {"goal_id": replacement["id"], "status": "active", "progress": "duplicate"}, - ) - assert reopened.registry.require("owner").goal.progress == "second" + invoke_tool( + reopened, + "comms_goal", + {"goal_id": replacement["id"], "status": "active", "progress": "after reopen"}, + ) + assert reopened.registry.require("owner").goal.progress == "after reopen" + assert reopened.registry.require("owner").goal.reports_turn(ReportedTurn(turn)) comms.goals.update_goal( "owner", ClearGoalAction(expect=GoalPrecondition(goal_id=OneGoal(replacement_goal_id))) @@ -182,6 +199,9 @@ def test_authorized_retry_does_not_consume_standby_report(comms, monkeypatch, tm "goal_id": started["id"], "status": "retry", "progress": "cause fixed", }) assert comms.registry.require("owner").goal.reported_turn == before + invoke_tool(comms, "comms_goal", { + "goal_id": started["id"], "status": "active", "progress": "recovery inspected", + }) result = invoke_tool(comms, "comms_goal", { "goal_id": started["id"], "status": "standby", "wait_for": ["dependency"], "progress": "waiting for dependency", @@ -189,10 +209,14 @@ def test_authorized_retry_does_not_consume_standby_report(comms, monkeypatch, tm assert result["goal"]["progress"] == "waiting for dependency" reopened = wire(comms.root) assert reopened.registry.require("owner").goal.reports_turn(ReportedTurn(turn)) - with pytest.raises(ValueError, match="already reported"): - invoke_tool(reopened, "comms_goal", { - "goal_id": started["id"], "status": "active", "progress": "duplicate", - }) + assert tuple(target.incarnation.name for target in reopened.goals.waits.resolve( + reopened.registry.require("owner").goal).targets) == ("dependency",) + invoke_tool(reopened, "comms_goal", { + "goal_id": started["id"], "status": "active", "progress": "dependency resolved", + }) + assert reopened.registry.require("owner").goal.progress == "dependency resolved" + assert reopened.registry.require("owner").goal.reports_turn(ReportedTurn(turn)) + assert not reopened.goals.waits.resolve(reopened.registry.require("owner").goal).targets def test_model_tool_cannot_resume_blocked_uncertain_goal(comms, monkeypatch): From 0244662a9445474bf3672045e294ddc04bd4cd80 Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Sun, 1 Nov 2026 03:01:27 -0500 Subject: [PATCH 2/2] Retain the original released owner in repeated recovery checks --- tests/test_goal_attempts.py | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/tests/test_goal_attempts.py b/tests/test_goal_attempts.py index 4c647fce46..785f1ec85b 100644 --- a/tests/test_goal_attempts.py +++ b/tests/test_goal_attempts.py @@ -469,15 +469,17 @@ def test_stop_recovers_claim_after_owner_exits_without_turn_cleanup(comms, claim assert FailedTurnEvidence.count(conn) == 0 assert stopped.goal == claimed_owner.goal stopped.goal.require().require_active() - # Repeated recovery is a no-op; it never creates another ready generation. - RecoveryMonitorCapability.settle_stopped_owners(comms.owners, (stopped,)) + # Repeated recovery retains the original released process, as stop/restart do; + # the post-stop registry row has already replaced that binding with NoProcess. + RecoveryMonitorCapability.settle_stopped_owners(comms.owners, (claimed_owner,)) assert store.require_generation(goal_id) == successor - # Even without native coordination, a foreign release must be rejected. + # Neither a cleared process binding nor a foreign incarnation is this release. substituted = replace(claimed_owner, created_at=RecordedCreationTime(claimed_owner.created_at + 1)) - with pytest.raises(RecoveryBlocked): - RecoveryMonitorCapability.settle_stopped_owners(comms.owners, (substituted,)) - assert store.require_generation(goal_id) == successor + for mismatched_owner in (stopped, substituted): + with pytest.raises(RecoveryBlocked): + RecoveryMonitorCapability.settle_stopped_owners(comms.owners, (mismatched_owner,)) + assert store.require_generation(goal_id) == successor def test_stop_preserves_already_verified_goal_progress(comms, claimed_owner):