diff --git a/desloppify/app/commands/review/importing/plan_sync.py b/desloppify/app/commands/review/importing/plan_sync.py index bf71e3fb6..49fcdbbc0 100644 --- a/desloppify/app/commands/review/importing/plan_sync.py +++ b/desloppify/app/commands/review/importing/plan_sync.py @@ -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, ) @@ -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, diff --git a/desloppify/app/commands/scan/plan_reconcile.py b/desloppify/app/commands/scan/plan_reconcile.py index 630194ce2..ecbab0358 100644 --- a/desloppify/app/commands/scan/plan_reconcile.py +++ b/desloppify/app/commands/scan/plan_reconcile.py @@ -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, ) @@ -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) @@ -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 @@ -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 diff --git a/desloppify/engine/_plan/sync/workflow.py b/desloppify/engine/_plan/sync/workflow.py index 480a5553a..efa0f061f 100644 --- a/desloppify/engine/_plan/sync/workflow.py +++ b/desloppify/engine/_plan/sync/workflow.py @@ -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 @@ -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() @@ -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) @@ -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", diff --git a/desloppify/tests/commands/review/test_review_importing_support_direct.py b/desloppify/tests/commands/review/test_review_importing_support_direct.py index 618ce8570..f116a19e7 100644 --- a/desloppify/tests/commands/review/test_review_importing_support_direct.py +++ b/desloppify/tests/commands/review/test_review_importing_support_direct.py @@ -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] = {} diff --git a/desloppify/tests/commands/scan/test_plan_reconcile.py b/desloppify/tests/commands/scan/test_plan_reconcile.py index a3ab7b5d7..f4975d65b 100644 --- a/desloppify/tests/commands/scan/test_plan_reconcile.py +++ b/desloppify/tests/commands/scan/test_plan_reconcile.py @@ -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"] = { @@ -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 # --------------------------------------------------------------------------- diff --git a/desloppify/tests/commands/scan/test_plan_reconcile_postflight_and_reconcile.py b/desloppify/tests/commands/scan/test_plan_reconcile_postflight_and_reconcile.py index 9960a3813..2408a3174 100644 --- a/desloppify/tests/commands/scan/test_plan_reconcile_postflight_and_reconcile.py +++ b/desloppify/tests/commands/scan/test_plan_reconcile_postflight_and_reconcile.py @@ -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( @@ -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() @@ -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" diff --git a/desloppify/tests/plan/test_reconcile_pipeline.py b/desloppify/tests/plan/test_reconcile_pipeline.py index db37507ba..c4bc2db1f 100644 --- a/desloppify/tests/plan/test_reconcile_pipeline.py +++ b/desloppify/tests/plan/test_reconcile_pipeline.py @@ -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, @@ -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