Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions desloppify/app/commands/review/importing/plan_sync.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
)
from desloppify.engine._plan.sync.workflow_gates import sync_import_scores_needed
from desloppify.engine._plan.sync.workflow import (
clear_communicate_score_sentinel,
clear_create_plan_sentinel,
clear_score_communicated_sentinel,
)
Expand Down Expand Up @@ -251,6 +252,7 @@ def _apply_import_plan_transitions(
if trusted:
clear_score_communicated_sentinel(plan)
clear_create_plan_sentinel(plan)
clear_communicate_score_sentinel(plan)
if sync_inputs.covered_ids:
mark_subjective_review_completed(
plan,
Expand Down
10 changes: 10 additions & 0 deletions desloppify/app/commands/scan/plan_reconcile.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
from desloppify.engine._plan.sync.dimensions import current_unscored_ids
from desloppify.engine._plan.sync.context import is_mid_cycle
from desloppify.engine._plan.sync.workflow import (
clear_communicate_score_sentinel,
clear_create_plan_sentinel,
clear_score_communicated_sentinel,
)
Expand All @@ -52,6 +53,8 @@ def _reset_cycle_for_force_rescan(plan: dict[str, object]) -> bool:
order.remove(item)
clear_score_communicated_sentinel(plan)
clear_create_plan_sentinel(plan)
clear_communicate_score_sentinel(plan)
plan.pop("scan_count_at_plan_start", None)
meta = plan.get("epic_triage_meta", {})
if isinstance(meta, dict):
meta.pop("triage_recommended", None)
Expand Down Expand Up @@ -108,6 +111,7 @@ def _seed_plan_start_scores(plan: dict[str, object], state: state_mod.StateModel
}
clear_score_communicated_sentinel(plan)
clear_create_plan_sentinel(plan)
clear_communicate_score_sentinel(plan)
plan["scan_count_at_plan_start"] = int(state.get("scan_count", 0) or 0)
return True

Expand Down Expand Up @@ -155,6 +159,12 @@ def _clear_plan_start_scores_if_queue_empty(
plan["plan_start_scores"] = {}
clear_score_communicated_sentinel(plan)
clear_create_plan_sentinel(plan)
# NOTE: do NOT clear communicate_score_resolved_this_cycle here.
# This sentinel must survive until the next cycle starts (when
# _seed_plan_start_scores runs), otherwise the post-queue-drain rescan
# re-injects workflow::communicate-score because both guards
# (previous_plan_start_scores and communicate_score_resolved_this_cycle)
# have been cleared before reconcile_plan runs on the next scan.
return True


Expand Down
18 changes: 18 additions & 0 deletions desloppify/engine/_plan/sync/workflow.py
Original file line number Diff line number Diff line change
Expand Up @@ -313,6 +313,16 @@ def clear_create_plan_sentinel(plan: PlanModel) -> None:
plan.pop("create_plan_resolved_this_cycle", None)


def clear_communicate_score_sentinel(plan: PlanModel) -> None:
"""Clear the ``communicate_score_resolved_this_cycle`` sentinel.

Call this at the same cycle-boundary points as the other workflow
sentinels so that ``sync_communicate_score_needed`` can re-inject
``workflow::communicate-score`` in the next cycle.
"""
plan.pop("communicate_score_resolved_this_cycle", None)


_EMPTY = QueueSyncResult


Expand Down Expand Up @@ -485,6 +495,12 @@ def sync_communicate_score_needed(
# at injection time and cleared at cycle boundaries.
if "previous_plan_start_scores" in plan:
return _EMPTY()
# Already resolved this cycle — sentinel is set when injected and
# cleared at cycle boundaries (force-rescan, score seeding, queue
# drain, trusted import). This prevents re-injection after the
# rescan that follows a resolve clears previous_plan_start_scores.
if plan.get("communicate_score_resolved_this_cycle"):
return _EMPTY()
if not _subjective_review_current_for_cycle(plan, state, policy=policy):
return _EMPTY()

Expand All @@ -494,6 +510,7 @@ def sync_communicate_score_needed(
# to rebaseline) so mid-cycle scans don't re-inject.
if not plan.get("previous_plan_start_scores"):
plan["previous_plan_start_scores"] = {}
plan["communicate_score_resolved_this_cycle"] = True
return _inject(plan, WORKFLOW_COMMUNICATE_SCORE_ID)


Expand All @@ -520,6 +537,7 @@ def _rebaseline_plan_start_scores(
__all__ = [
"PendingImportScoresMeta",
"ScoreSnapshot",
"clear_communicate_score_sentinel",
"clear_create_plan_sentinel",
"clear_score_communicated_sentinel",
"import_scores_meta_matches",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -416,6 +416,30 @@ def fake_reconcile(_plan, _state, target_strict):
]


def test_sync_plan_after_import_trusted_internal_clears_workflow_cycle_sentinels(
monkeypatch,
) -> None:
plan: dict = {
"queue_order": [],
"previous_plan_start_scores": {"strict": 70.0},
"create_plan_resolved_this_cycle": True,
"communicate_score_resolved_this_cycle": True,
}

_patch_basic_plan_sync_runtime(monkeypatch, plan=plan)

plan_sync_mod.sync_plan_after_import(
state={},
diff={"new": 0, "reopened": 0, "auto_resolved": 0},
assessment_mode="trusted_internal",
request=_sync_request(import_payload={"assessments": {"Naming Quality": 82}}),
)

assert "previous_plan_start_scores" not in plan
assert "create_plan_resolved_this_cycle" not in plan
assert "communicate_score_resolved_this_cycle" not in plan


def test_sync_plan_after_import_reuses_plan_aware_policy(monkeypatch) -> None:
plan: dict = {"queue_order": []}
seen: dict[str, object] = {}
Expand Down
31 changes: 31 additions & 0 deletions desloppify/tests/commands/scan/test_plan_reconcile.py
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,21 @@ def test_seeds_when_plan_start_scores_empty(self):
"objective": 88.0, "verified": 80.0,
}

def test_seeding_clears_workflow_cycle_sentinels(self):
plan = empty_plan()
plan["previous_plan_start_scores"] = {"strict": 70.0}
plan["create_plan_resolved_this_cycle"] = True
plan["communicate_score_resolved_this_cycle"] = True
state = _make_state(
strict_score=85.0, overall_score=90.0,
objective_score=88.0, verified_strict_score=80.0,
)

assert reconcile_mod._seed_plan_start_scores(plan, state) is True
assert "previous_plan_start_scores" not in plan
assert "create_plan_resolved_this_cycle" not in plan
assert "communicate_score_resolved_this_cycle" not in plan

def test_does_not_reseed_when_scores_exist(self):
plan = empty_plan()
plan["plan_start_scores"] = {
Expand Down Expand Up @@ -164,6 +179,22 @@ def test_returns_false_when_existing_is_non_dict(self):
assert reconcile_mod._seed_plan_start_scores(plan, state) is False


class TestResetCycleForForceRescan:

def test_force_rescan_clears_workflow_cycle_sentinels(self):
plan = empty_plan()
plan["queue_order"] = ["workflow::communicate-score", "workflow::create-plan"]
plan["plan_start_scores"] = {"strict": 80.0}
plan["previous_plan_start_scores"] = {"strict": 70.0}
plan["create_plan_resolved_this_cycle"] = True
plan["communicate_score_resolved_this_cycle"] = True

assert reconcile_mod._reset_cycle_for_force_rescan(plan) is True
assert "previous_plan_start_scores" not in plan
assert "create_plan_resolved_this_cycle" not in plan
assert "communicate_score_resolved_this_cycle" not in plan


# ---------------------------------------------------------------------------
# Tests: _apply_plan_reconciliation
# ---------------------------------------------------------------------------
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,9 @@ def test_clears_and_copies_to_state(self, monkeypatch):
"strict": 80.0, "overall": 85.0,
"objective": 82.0, "verified": 78.0,
}
plan["previous_plan_start_scores"] = {"strict": 70.0}
plan["create_plan_resolved_this_cycle"] = True
plan["communicate_score_resolved_this_cycle"] = True
state = _make_state()

monkeypatch.setattr(
Expand All @@ -41,6 +44,12 @@ def test_clears_and_copies_to_state(self, monkeypatch):
assert result is True
assert plan["plan_start_scores"] == {}
assert state["_plan_start_scores_for_reveal"]["strict"] == 80.0
assert "previous_plan_start_scores" not in plan
assert "create_plan_resolved_this_cycle" not in plan
# communicate_score_resolved_this_cycle must survive queue-drain so
# the post-drain rescan doesn't re-inject workflow::communicate-score.
# It is cleared by _seed_plan_start_scores at the next cycle start.
assert plan.get("communicate_score_resolved_this_cycle") is True

def test_does_not_clear_when_queue_has_items(self, monkeypatch):
plan = empty_plan()
Expand Down Expand Up @@ -388,3 +397,70 @@ def test_multiple_issues_superseded_at_once(self, monkeypatch):
assert "a" not in saved[0]["queue_order"]
assert "b" not in saved[0]["queue_order"]
assert "c" in saved[0]["queue_order"]

# ---------------------------------------------------------------------------
# Regression: communicate-score re-injection after resolve
# ---------------------------------------------------------------------------


def test_communicate_score_not_reinjected_after_resolve(monkeypatch):
"""communicate_score_resolved_this_cycle survives queue-drain so post-drain rescan doesn't re-inject.

Scenario: agent resolves workflow::communicate-score, then rescans twice.
The sentinel must block re-injection on both subsequent scans until a new
cycle starts (_seed_plan_start_scores clears it).
"""
from desloppify.engine._plan.constants import WORKFLOW_COMMUNICATE_SCORE_ID

# State after agent resolves communicate-score:
# - communicate_score_resolved_this_cycle is True (set at injection time)
# - previous_plan_start_scores is set (set at injection time)
# - plan_start_scores has the rebaselined score
# - queue_order is empty (communicate-score was purged on resolve)
plan = empty_plan()
plan["plan_start_scores"] = {
"strict": 90.0,
"overall": 90.0,
"objective": 90.0,
"verified": 30.0,
}
plan["previous_plan_start_scores"] = {"strict": 30.0}
plan["communicate_score_resolved_this_cycle"] = True

state = _make_state(
strict_score=90.0,
overall_score=90.0,
objective_score=90.0,
verified_strict_score=30.0,
)

saved: list[dict] = []

def _capture_save(p, _path=None):
import copy

saved.append(copy.deepcopy(p))

monkeypatch.setattr(reconcile_mod, "load_plan", lambda _path=None: plan)
monkeypatch.setattr(reconcile_mod, "save_plan", _capture_save)

# Scan 1 (post-resolve rescan): queue drains, plan_start_scores cleared,
# but communicate_score_resolved_this_cycle must survive.
reconcile_mod.reconcile_plan_post_scan(_runtime(state=state))
assert saved, "plan should have been saved"
scan1_plan = saved[-1]
assert WORKFLOW_COMMUNICATE_SCORE_ID not in scan1_plan.get(
"queue_order", []
), "communicate-score must not be re-injected on scan 1"
assert (
scan1_plan.get("communicate_score_resolved_this_cycle") is True
), "sentinel must survive queue-drain so scan 2 is also protected"

# Scan 2: sentinel still present — must still block re-injection.
saved.clear()
reconcile_mod.reconcile_plan_post_scan(_runtime(state=state))
if saved:
scan2_plan = saved[-1]
assert WORKFLOW_COMMUNICATE_SCORE_ID not in scan2_plan.get(
"queue_order", []
), "communicate-score must not be re-injected on scan 2"
14 changes: 13 additions & 1 deletion desloppify/tests/plan/test_reconcile_pipeline.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
)
from desloppify.engine._plan.schema import empty_plan
from desloppify.engine._plan.sync import live_planned_queue_empty, reconcile_plan
from desloppify.engine._plan.sync.workflow import clear_score_communicated_sentinel
from desloppify.engine._plan.sync.workflow import clear_communicate_score_sentinel, clear_score_communicated_sentinel
from desloppify.engine._work_queue.snapshot import (
PHASE_ASSESSMENT_POSTFLIGHT,
PHASE_EXECUTE,
Expand Down Expand Up @@ -467,6 +467,18 @@ def test_clear_score_communicated_sentinel() -> None:
assert "previous_plan_start_scores" not in plan


def test_clear_communicate_score_sentinel() -> None:
"""Helper removes the resolved-this-cycle sentinel; missing key is a no-op."""
plan = empty_plan()
plan["communicate_score_resolved_this_cycle"] = True

clear_communicate_score_sentinel(plan)
assert "communicate_score_resolved_this_cycle" not in plan

clear_communicate_score_sentinel(plan)
assert "communicate_score_resolved_this_cycle" not in plan


def test_sentinel_blocks_communicate_score_reinjection() -> None:
"""When the sentinel is set, communicate-score does not re-inject."""
from desloppify.engine._plan.sync.workflow import sync_communicate_score_needed
Expand Down
Loading