Skip to content

Commit 367de99

Browse files
committed
fix(goals): keep the lifecycle projection honest at its boundaries
Review found two defects in the derived projection. Closeout ignored milestone reachability: a Goal whose declared acceptance marker was still unreached, with no open agent work, was reported as closing with a next transition of closed. Running out of open work is not the same as having reached acceptance, so an unreached milestone now keeps the Goal in qualifying with a milestone_unreached reason. An existing work-lane constraint also outranks this projection's own reading of remaining work. The compact label helper claimed public safety without providing it: it only collapsed whitespace and truncated, so a private absolute path in a run history reference reached the projection verbatim. It now reuses the shared redaction rule and drops a value that still matches a private-text or provider token shape. Both are covered by negative cases, and each case kills its mutant: removing the closeout guard or the redaction rule fails the smoke. Signed-off-by: song <liusongstep@gmail.com>
1 parent 56f8507 commit 367de99

2 files changed

Lines changed: 141 additions & 5 deletions

File tree

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

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
PHASE_QUALIFYING,
2525
PHASE_STARTING,
2626
PHASE_WAITING_OWNER,
27+
_compact_text,
2728
build_goal_artifact_lifecycle_projection,
2829
)
2930

@@ -214,6 +215,93 @@ def assert_projection_is_pure_and_reads_no_state() -> None:
214215
assert first["lifecycle_phase"] == PHASE_QUALIFYING, first
215216

216217

218+
def assert_unreached_milestone_blocks_closeout() -> None:
219+
"""An unclaimed-acceptance Goal must not be told to close."""
220+
221+
projection = build_goal_artifact_lifecycle_projection(
222+
goal_id=GOAL_ID,
223+
goal={
224+
"id": GOAL_ID,
225+
"status": "active",
226+
"acceptance": {"milestones": ["baseline_pass"]},
227+
},
228+
user_todo_summary={"gate_open_items": []},
229+
agent_todo_summary={"open_count": 0},
230+
run_history={"latest_runs": []},
231+
)
232+
assert projection["lifecycle_phase"] == PHASE_QUALIFYING, projection
233+
transitions = projection["next_transitions"]
234+
assert transitions, projection
235+
assert transitions[0]["target_phase"] != PHASE_CLOSED, projection
236+
assert transitions[0]["reason_codes"] == ["milestone_unreached"], projection
237+
238+
239+
def assert_reached_milestones_still_allow_closeout() -> None:
240+
"""The same inputs with evidence present do reach the closing phase."""
241+
242+
projection = build_goal_artifact_lifecycle_projection(
243+
goal_id=GOAL_ID,
244+
goal={
245+
"id": GOAL_ID,
246+
"status": "active",
247+
"acceptance": {"milestones": ["primary_goal_outcome"]},
248+
},
249+
user_todo_summary={"gate_open_items": []},
250+
agent_todo_summary={"open_count": 0},
251+
run_history={
252+
"latest_runs": [
253+
{
254+
"delivery_outcome": "primary_goal_outcome",
255+
"delivery_batch_scale": "multi_surface",
256+
}
257+
]
258+
},
259+
)
260+
assert projection["lifecycle_phase"] == PHASE_CLOSING, projection
261+
assert projection["next_transitions"][0]["target_phase"] == PHASE_CLOSED, projection
262+
263+
264+
def assert_private_values_are_redacted_or_dropped() -> None:
265+
"""A run-history reference is free text; private values must not survive."""
266+
267+
projection = build_goal_artifact_lifecycle_projection(
268+
goal_id=GOAL_ID,
269+
goal={"id": GOAL_ID, "status": "active"},
270+
agent_todo_summary={"open_count": 1},
271+
run_history={
272+
"latest_runs": [
273+
{
274+
"delivery_outcome": "primary_goal_outcome",
275+
"delivery_batch_scale": "multi_surface",
276+
"evidence_ref": "/Users/private-owner/.ssh/id_rsa",
277+
"recommended_action": (
278+
"publish ghp_ABCDEFGHIJKLMNOPQRSTUVWXYZ012345 and "
279+
"see /private/var/folders/secret/notes.md"
280+
),
281+
}
282+
]
283+
},
284+
)
285+
rendered = json.dumps(projection, ensure_ascii=False)
286+
for leaked in (
287+
"/Users/private-owner",
288+
"/private/var/folders",
289+
"ghp_ABCDEFGHIJKLMNOPQRSTUVWXYZ012345",
290+
):
291+
assert leaked not in rendered, (leaked, rendered)
292+
293+
# Pin the redaction rule itself: a benign local path must be replaced, not
294+
# merely dropped, so the boundary still holds when a caller later renders a
295+
# label this projection chose to keep.
296+
assert _compact_text("/Users/private-owner/notes.md") == (
297+
"<local-path-redacted>"
298+
), _compact_text("/Users/private-owner/notes.md")
299+
assert _compact_text("token ghp_ABCDEFGHIJKLMNOPQRSTUVWXYZ012345") is None
300+
assert _compact_text("/private/var/folders/x/secret.md") == (
301+
"<local-path-redacted>"
302+
)
303+
304+
217305
def main() -> int:
218306
assert_starting_phase_without_work()
219307
assert_declared_milestone_stays_unreached_while_gapped()
@@ -222,6 +310,9 @@ def main() -> int:
222310
assert_evidence_guard_is_required_and_owned_by_the_agent()
223311
assert_closing_then_closed_phase()
224312
assert_projection_is_pure_and_reads_no_state()
313+
assert_unreached_milestone_blocks_closeout()
314+
assert_reached_milestones_still_allow_closeout()
315+
assert_private_values_are_redacted_or_dropped()
225316
print("goal-artifact-lifecycle-projection-smoke ok")
226317
return 0
227318

‎loopx/control_plane/goals/artifact_lifecycle.py‎

Lines changed: 50 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,12 @@
1818

1919
from __future__ import annotations
2020

21+
import re
2122
from typing import Any
2223

24+
from ...presentation.public_safety import redact_public_text
25+
from ...public_safe_text import find_private_text_match
26+
2327
GOAL_ARTIFACT_LIFECYCLE_PROJECTION_SCHEMA_VERSION = (
2428
"goal_artifact_lifecycle_projection_v0"
2529
)
@@ -37,19 +41,31 @@
3741

3842
_TERMINAL_GOAL_STATUSES = {"closed", "retired", "archived", "done", "complete"}
3943

44+
# Provider token shapes the shared private-text rules do not cover. A run
45+
# history reference is free text, so a leaked token there must never reach a
46+
# public projection just because the shared corpus did not list its prefix.
47+
_TOKEN_SHAPES = re.compile(
48+
r"\b(?:gh[pousr]_[A-Za-z0-9]{16,}|sk-[A-Za-z0-9_-]{16,}|AKIA[0-9A-Z]{16})\b"
49+
)
50+
4051
# A material run outcome that a Goal's own acceptance can rest on.
4152
_MATERIAL_OUTCOMES = {"primary_goal_outcome", "outcome_progress", "multi_surface"}
4253

4354

4455
def _compact_text(value: Any, *, limit: int = 240) -> str | None:
45-
"""Bound one public-safe label; never carry a raw body or path."""
56+
"""Bound and redact one label; never carry a raw body, path or credential."""
4657

4758
if not isinstance(value, str):
4859
return None
49-
collapsed = " ".join(value.split())
60+
collapsed = redact_public_text(value, limit=limit)
5061
if not collapsed:
5162
return None
52-
return collapsed[:limit]
63+
# The shared sanitizer covers local paths; this projection additionally
64+
# refuses a value that still matches a private-text or credential shape
65+
# rather than publishing a partly-redacted fragment.
66+
if find_private_text_match(collapsed) or _TOKEN_SHAPES.search(collapsed):
67+
return None
68+
return collapsed
5369

5470

5571
def _mapping(value: Any) -> dict[str, Any]:
@@ -207,7 +223,10 @@ def _lifecycle_phase(
207223
total_open = open_count if isinstance(open_count, int) and not isinstance(open_count, bool) else 0
208224
if not milestones and total_open == 0:
209225
return PHASE_STARTING
210-
if total_open == 0:
226+
# An unclaimed-acceptance Goal is never closing: running out of open agent
227+
# work is not the same as having reached the declared acceptance markers.
228+
unreached = any(milestone["reached"] is not True for milestone in milestones)
229+
if total_open == 0 and not unreached:
211230
return PHASE_CLOSING
212231
return PHASE_QUALIFYING
213232

@@ -217,6 +236,7 @@ def _next_transitions(
217236
*,
218237
phase: str,
219238
guards: list[dict[str, Any]],
239+
milestones: list[dict[str, Any]],
220240
work_lane: dict[str, Any],
221241
) -> list[dict[str, Any]]:
222242
"""Reuse the existing lane/frontier derivation instead of a second machine."""
@@ -238,6 +258,27 @@ def _next_transitions(
238258
"reason_codes": ["guard_open"],
239259
}
240260
]
261+
# An existing work-lane constraint outranks this projection's own reading
262+
# of open work: the lane owner decides what runs next.
263+
if lane and phase != PHASE_CLOSING:
264+
return [
265+
{
266+
"target_phase": PHASE_QUALIFYING,
267+
"precondition": obligation or "advance the selected lane",
268+
"reason_codes": ["work_lane_selected"],
269+
}
270+
]
271+
unreached = [
272+
milestone["id"] for milestone in milestones if milestone["reached"] is not True
273+
]
274+
if unreached:
275+
return [
276+
{
277+
"target_phase": PHASE_QUALIFYING,
278+
"precondition": "reach the declared acceptance milestones with evidence",
279+
"reason_codes": ["milestone_unreached"],
280+
}
281+
]
241282
if phase == PHASE_CLOSING:
242283
return [
243284
{
@@ -293,7 +334,11 @@ def build_goal_artifact_lifecycle_projection(
293334
"milestones": milestones,
294335
"guards": guards,
295336
"next_transitions": _next_transitions(
296-
goal_record, phase=phase, guards=guards, work_lane=lane
337+
goal_record,
338+
phase=phase,
339+
guards=guards,
340+
milestones=milestones,
341+
work_lane=lane,
297342
),
298343
}
299344

0 commit comments

Comments
 (0)