fix(goals): require an acceptance verdict before recommending closed

Builds on the recorded-gap fix rather than replacing it: `outcome_gap`
staying an unreached marker is kept, and so is naming the acceptance
sources the bounded observation could not read.

Two gaps remained in that reading.

An empty `missing_sources` was treated as an acceptance verdict.  It only
means the observation read every source it knows about; the projection
still reports `acceptance_assessed=False` and a coverage that is
`partial` or `unavailable`, never `complete`.  So a Goal with both an
attention item and agent vision present produced a bare `next: closed`
with no disclosure at all -- the same defect as the reported one, moved
to a fully observed input.  `_acceptance_supports_closeout` now reads the
verdict fields directly instead of inferring one from silence.

Annotating the reason codes did not undo the recommendation.  The
reported defect is an actionable wrong step shown to an operator, and
`target_phase: closed` remained that step even with `acceptance_unverified`
attached.  Closing stays reachable as the todo-completion reading, but
without a verdict the step stays inside closing and asks for the
acceptance the existing owner has not given.

Declared acceptance markers are removed.  No producer writes
`goal.acceptance.milestones` or `goal.milestones` anywhere in the
repository, so the reader was unreachable in production while carrying
the only guard able to hold back a closeout, and the fixtures exercising
it could not show that a user declaration reaches the readout.  Add it
back together with the producer that writes it.

Signed-off-by: song <liusongstep@gmail.com>

Author: songoow

examples/control_plane/goal-artifact-lifecycle-projection-smoke.py

@@ -65,28 +65,6 @@ def assert_starting_phase_without_work() -> None:
     assert_no_public_leak(projection)
 
 
-def assert_declared_milestone_stays_unreached_while_gapped() -> None:
-    """A declared marker with an open acceptance gap is not reached."""
-
-    projection = build_goal_artifact_lifecycle_projection(
-        goal_id=GOAL_ID,
-        goal={
-            "id": GOAL_ID,
-            "status": "active",
-            "acceptance": {"milestones": ["environment_ready", "baseline_pass"]},
-        },
-        agent_todo_summary={"open_count": 2},
-        run_history={"latest_runs": []},
-        acceptance_gaps=[
-            {"kind": "vision_acceptance_gap", "agent_id": "agent-a"},
-        ],
-    )
-    reached = {item["id"]: item["reached"] for item in projection["milestones"]}
-    assert reached == {"environment_ready": False, "baseline_pass": False}, projection
-    assert projection["lifecycle_phase"] == PHASE_QUALIFYING, projection
-    assert_no_public_leak(projection)
-
-
 def assert_outcome_gap_is_material_but_not_reached() -> None:
     """A recorded gap is retained history, never a reached marker.
 
@@ -230,7 +208,8 @@ def assert_closing_then_closed_phase() -> None:
     )
     assert closing["lifecycle_phase"] == PHASE_CLOSING, closing
     transition = closing["next_transitions"][0]
-    assert transition["target_phase"] == PHASE_CLOSED, closing
+    assert transition["target_phase"] == PHASE_CLOSING, closing
+    assert transition["target_phase"] != PHASE_CLOSED, closing
     # Closing is the todo-completion reading, not an acceptance verdict. The
     # acceptance observation could not read agent vision here, so the closeout
     # step names that source instead of implying a verified acceptance.
@@ -265,37 +244,17 @@ def assert_projection_is_pure_and_reads_no_state() -> None:
     assert first["lifecycle_phase"] == PHASE_QUALIFYING, first
 
 
-def assert_unreached_milestone_blocks_closeout() -> None:
-    """An unclaimed-acceptance Goal must not be told to close."""
-
-    projection = build_goal_artifact_lifecycle_projection(
-        goal_id=GOAL_ID,
-        goal={
-            "id": GOAL_ID,
-            "status": "active",
-            "acceptance": {"milestones": ["baseline_pass"]},
-        },
-        user_todo_summary={"gate_open_items": []},
-        agent_todo_summary={"open_count": 0},
-        run_history={"latest_runs": []},
-    )
-    assert projection["lifecycle_phase"] == PHASE_QUALIFYING, projection
-    transitions = projection["next_transitions"]
-    assert transitions, projection
-    assert transitions[0]["target_phase"] != PHASE_CLOSED, projection
-    assert transitions[0]["reason_codes"] == ["milestone_unreached"], projection
-
+def assert_progress_evidence_alone_does_not_authorize_closeout() -> None:
+    """Evidence reaches the closing phase; only a verdict recommends closed.
 
-def assert_reached_milestones_still_allow_closeout() -> None:
-    """The same inputs with evidence present do reach the closing phase."""
+    The canonical progress outcomes prove the Goal advanced. They do not prove
+    the declared acceptance was assessed, and the acceptance owner reports it
+    was not, so the step stays inside closing.
+    """
 
     projection = build_goal_artifact_lifecycle_projection(
         goal_id=GOAL_ID,
-        goal={
-            "id": GOAL_ID,
-            "status": "active",
-            "acceptance": {"milestones": ["primary_goal_outcome"]},
-        },
+        goal={"id": GOAL_ID, "status": "active"},
         user_todo_summary={"gate_open_items": []},
         agent_todo_summary={"open_count": 0},
         run_history={
@@ -307,8 +266,13 @@ def assert_reached_milestones_still_allow_closeout() -> None:
             ]
         },
     )
+    reached = {item["id"]: item["reached"] for item in projection["milestones"]}
+    assert reached == {"primary_goal_outcome": True}, projection
     assert projection["lifecycle_phase"] == PHASE_CLOSING, projection
-    assert projection["next_transitions"][0]["target_phase"] == PHASE_CLOSED, projection
+    transition = projection["next_transitions"][0]
+    assert transition["target_phase"] == PHASE_CLOSING, projection
+    assert transition["target_phase"] != PHASE_CLOSED, projection
+    assert "acceptance_unverified" in transition["reason_codes"], projection
 
 
 def assert_private_values_are_redacted_or_dropped() -> None:
@@ -414,7 +378,6 @@ def assert_status_collection_attaches_a_readable_readout() -> None:
                 {
                     "id": GOAL_ID,
                     "status": "active",
-                    "acceptance": {"milestones": ["baseline_pass"]},
                 }
             ]
         },
@@ -513,16 +476,14 @@ def assert_control_plane_imports_no_presentation_module() -> None:
 
 def main() -> int:
     assert_starting_phase_without_work()
-    assert_declared_milestone_stays_unreached_while_gapped()
     assert_outcome_gap_is_material_but_not_reached()
     assert_gap_only_evidence_blocks_closeout()
     assert_evidence_milestone_reached_from_run_history()
     assert_open_owner_gate_blocks_the_next_transition()
     assert_evidence_guard_is_required_and_owned_by_the_agent()
     assert_closing_then_closed_phase()
     assert_projection_is_pure_and_reads_no_state()
-    assert_unreached_milestone_blocks_closeout()
-    assert_reached_milestones_still_allow_closeout()
+    assert_progress_evidence_alone_does_not_authorize_closeout()
     assert_private_values_are_redacted_or_dropped()
     assert_batch_scale_never_promotes_an_outcome()
     assert_status_collection_attaches_a_readable_readout()

loopx/control_plane/goals/artifact_lifecycle.py

@@ -85,40 +85,15 @@ def _is_closed(goal: dict[str, Any]) -> bool:
     return _goal_status(goal) in _TERMINAL_GOAL_STATUSES
 
 
-def _declared_milestones(goal: dict[str, Any]) -> list[dict[str, Any]]:
-    """Read Goal-declared acceptance markers, if the Goal records any."""
-
-    acceptance = _mapping(goal.get("acceptance"))
-    raw = _list(acceptance.get("milestones")) or _list(goal.get("milestones"))
-    milestones: list[dict[str, Any]] = []
-    for index, item in enumerate(raw):
-        record = _mapping(item)
-        if not isinstance(item, str) and not record:
-            continue
-        milestone_id = (
-            _compact_text(item, limit=120) if isinstance(item, str)
-            else _compact_text(record.get("id") or record.get("milestone_id"), limit=120)
-        )
-        if not milestone_id:
-            milestone_id = f"milestone_{index + 1}"
-        label = (
-            None if isinstance(item, str)
-            else _compact_text(record.get("label") or record.get("summary"))
-        )
-        milestones.append(
-            {
-                "id": milestone_id,
-                "label": label or milestone_id,
-                "reached": False,
-                "reached_evidence_refs": [],
-                "source": "declared",
-            }
-        )
-    return milestones
-
-
 def _evidence_milestones(run_history: dict[str, Any]) -> list[dict[str, Any]]:
-    """Fall back to material evidence the run history already recorded."""
+    """Markers this readout can evidence from material run history.
+
+    Goal-declared acceptance markers are deliberately not read: no authoring
+    or collection path records `goal.acceptance.milestones` or
+    `goal.milestones`, so reading them promised a readout that no user
+    declaration could reach, while carrying the only guard able to hold back a
+    closeout.  Add them back together with the producer that writes them.
+    """
 
     milestones: list[dict[str, Any]] = []
     seen: set[str] = set()
@@ -159,23 +134,6 @@ def _evidence_milestones(run_history: dict[str, Any]) -> list[dict[str, Any]]:
     return milestones
 
 
-def _milestones(
-    goal: dict[str, Any],
-    run_history: dict[str, Any],
-) -> list[dict[str, Any]]:
-    declared = _declared_milestones(goal)
-    evidence = _evidence_milestones(run_history)
-    if not declared:
-        return evidence
-    # A declared marker is a claim about what the Goal intends to reach, not
-    # proof that it did. It counts as reached only when evidence already
-    # records that outcome.
-    reached_outcomes = {item["id"] for item in evidence if item["reached"]}
-    for milestone in declared:
-        milestone["reached"] = milestone["id"] in reached_outcomes
-    return declared + [item for item in evidence if item["id"] not in {m["id"] for m in declared}]
-
-
 def _guards(
     observation: dict[str, Any],
     acceptance_gaps: list[Any],
@@ -217,6 +175,26 @@ def _guards(
     return guards
 
 
+def _acceptance_supports_closeout(observation: dict[str, Any]) -> bool:
+    """Whether the acceptance owner has actually assessed the declared acceptance.
+
+    An empty `missing_sources` only means the bounded observation read every
+    source it knows about, never that acceptance was verified: this projection
+    reports `acceptance_assessed=False` and a coverage that is `partial` or
+    `unavailable`.  Treating "nothing left to name" as a verdict is what let a
+    fully observed Goal be recommended for closeout with no acceptance behind
+    it, so the verdict fields are read directly.
+    """
+
+    if not observation:
+        return False
+    if observation.get("acceptance_assessed") is not True:
+        return False
+    if observation.get("coverage") != "complete":
+        return False
+    return not _list(observation.get("missing_sources"))
+
+
 def _unobserved_acceptance_sources(observation: dict[str, Any]) -> list[str]:
     """Name the acceptance sources the bounded observation could not read.
 
@@ -318,17 +296,26 @@ def _next_transitions(
         # acceptance verdict. When the acceptance owner could not read some of
         # its sources, the closeout step says so instead of implying a verified
         # acceptance, and the reader keeps the decision.
+        if _acceptance_supports_closeout(observation):
+            return [
+                {
+                    "target_phase": PHASE_CLOSED,
+                    "precondition": "record the terminal no-follow-up outcome",
+                    "reason_codes": ["no_open_agent_work"],
+                }
+            ]
+        # Without that verdict the step stays inside closing: recommending a
+        # terminal outcome is the actionable error, and annotating the reason
+        # codes does not undo it.  Name the unread sources when there are any.
         unobserved = _unobserved_acceptance_sources(observation)
-        precondition = "record the terminal no-follow-up outcome"
-        reason_codes = ["no_open_agent_work"]
+        precondition = "verify the declared acceptance with its existing owner"
         if unobserved:
             precondition += "; this readout could not observe " + ", ".join(unobserved)
-            reason_codes.append("acceptance_unverified")
         return [
             {
-                "target_phase": PHASE_CLOSED,
+                "target_phase": PHASE_CLOSING,
                 "precondition": precondition,
-                "reason_codes": reason_codes,
+                "reason_codes": ["no_open_agent_work", "acceptance_unverified"],
             }
         ]
     return []
@@ -376,7 +363,7 @@ def build_goal_artifact_lifecycle_projection(
     gaps = [gap for gap in _list(acceptance_gaps) if _mapping(gap)]
     if acceptance_gaps is None:
         gaps = _list(observation.get("acceptance_gaps"))
-    milestones = _milestones(goal_record, history)
+    milestones = _evidence_milestones(history)
     guards = _guards(observation, gaps, agent_id=agent_id)
     phase = _lifecycle_phase(
         goal_record, guards=guards, milestones=milestones, agent_summary=agent_summary,

tests/control_plane/test_goal_acceptance_observation.py

@@ -330,13 +330,14 @@ def test_lifecycle_public_safety_covers_all_emitted_text():
     ]
     for value in unsafe_values:
         projection = build_goal_artifact_lifecycle_projection(
-            goal_id="demo", goal={"acceptance": {"milestones": [{"id": "baseline", "label": value}]}},
+            goal_id="demo", goal={},
             agent_id=value, acceptance_gaps=[{"kind": "gap"}],
             run_history={"latest_runs": [{"delivery_outcome": "outcome_progress", "recommended_action": value, "evidence_ref": value}]},
         )
         validate_public_safe_value(projection)
-        assert projection["milestones"][0]["label"] == "baseline"
-        evidence = projection["milestones"][1]
+        # An unsafe label and locator are both dropped, so the marker falls
+        # back to its canonical outcome id and publishes no evidence ref.
+        evidence = projection["milestones"][0]
         assert evidence["label"] == "outcome_progress"
         assert evidence["reached_evidence_refs"] == []
         assert projection["guards"][0]["agent_id"] is None
@@ -433,8 +434,58 @@ def test_lifecycle_closeout_names_unobserved_acceptance_sources():
         agent_todo_summary={"open_count": 0},
         run_history={"latest_runs": [{"delivery_outcome": "outcome_progress"}]},
     )
+    # Closing is the todo-completion reading. Without an acceptance verdict the
+    # step stays inside closing and names what was not observed, rather than
+    # recommending the terminal outcome with a caveat attached.
     assert projection["lifecycle_phase"] == "closing"
     transition = projection["next_transitions"][0]
-    assert transition["target_phase"] == "closed"
+    assert transition["target_phase"] == "closing"
     assert transition["reason_codes"] == ["no_open_agent_work", "acceptance_unverified"]
     assert transition["precondition"].endswith("this readout could not observe agent_vision")
+
+
+def test_lifecycle_fully_observed_goal_still_needs_an_acceptance_verdict():
+    """An empty `missing_sources` is not an acceptance verdict.
+
+    With an attention item and agent vision both present the observation has
+    nothing left to name, but it still reports `acceptance_assessed=False` and
+    a `partial` coverage. Reading "nothing missing" as "acceptance verified"
+    would recommend the terminal outcome with no acceptance behind it and no
+    disclosure attached.
+    """
+
+    from loopx.control_plane.goals.acceptance_observation import (
+        build_goal_acceptance_observation,
+    )
+    from loopx.control_plane.goals.artifact_lifecycle import (
+        build_goal_artifact_lifecycle_projection,
+    )
+
+    runs = [{
+        "delivery_outcome": "outcome_progress",
+        "agent_id": "agent-a",
+        "agent_vision": {"agent_id": "agent-a", "acceptance_met": True},
+    }]
+    attention = {
+        "goal_id": "demo",
+        "user_todos": {"gate_open_items": []},
+        "agent_todos": {"open_count": 0},
+    }
+    observation = build_goal_acceptance_observation(
+        {"id": "demo", "status": "active", "latest_runs": runs}, attention
+    )
+    assert observation["missing_sources"] == []
+    assert observation["acceptance_assessed"] is False
+    assert observation["coverage"] != "complete"
+
+    projection = build_goal_artifact_lifecycle_projection(
+        goal_id="demo", goal={"id": "demo", "status": "active"},
+        user_todo_summary={"gate_open_items": []},
+        agent_todo_summary={"open_count": 0},
+        run_history={"latest_runs": runs},
+        attention_item=attention,
+    )
+    transition = projection["next_transitions"][0]
+    assert transition["target_phase"] == "closing"
+    assert transition["reason_codes"] == ["no_open_agent_work", "acceptance_unverified"]
+    assert "could not observe" not in transition["precondition"]