diff --git a/desloppify/app/commands/plan/override/misc.py b/desloppify/app/commands/plan/override/misc.py index b2724efb4..f1ef7a38a 100644 --- a/desloppify/app/commands/plan/override/misc.py +++ b/desloppify/app/commands/plan/override/misc.py @@ -7,18 +7,19 @@ from desloppify.app.commands.helpers.command_runtime import command_runtime from desloppify.app.commands.helpers.state import require_issue_inventory, state_path +from desloppify.app.commands.helpers.transition_messages import emit_transition_message from desloppify.app.commands.plan.shared.patterns import resolve_ids_from_patterns -from .io import ( - _plan_file_for_state, - save_plan_state_transactional, -) from desloppify.base.config import target_strict_score_from_config from desloppify.base.output.terminal import colorize -from desloppify.engine.plan_state import ( - load_plan, - purge_uncommitted_ids, - save_plan, +from desloppify.engine._plan.refresh_lifecycle import ( + invalidate_postflight_scan, ) +from desloppify.engine._plan.sync import reconcile_plan +from desloppify.engine._plan.triage.protection import ( + clear_protected_triage_artifacts, + protected_review_issue_ids, +) +from desloppify.engine._state.resolution import resolve_issues from desloppify.engine.plan_ops import ( annotate_issue, append_log_entry, @@ -26,14 +27,18 @@ describe_issue, set_focus, ) -from desloppify.app.commands.helpers.transition_messages import emit_transition_message -from desloppify.engine._plan.refresh_lifecycle import ( - invalidate_postflight_scan, +from desloppify.engine.plan_state import ( + load_plan, + purge_uncommitted_ids, + save_plan, ) -from desloppify.engine._plan.sync import reconcile_plan -from desloppify.engine._state.resolution import resolve_issues from desloppify.state_io import load_state +from .io import ( + _plan_file_for_state, + save_plan_state_transactional, +) + def cmd_plan_describe(args: argparse.Namespace) -> None: """Set augmented description on issues.""" @@ -111,12 +116,16 @@ def cmd_plan_reopen(args: argparse.Namespace) -> None: return plan = load_plan(plan_file) + protected_ids = protected_review_issue_ids(plan) + clear_protected_triage_artifacts(plan, state_data) purge_uncommitted_ids(plan, reopened) skipped = plan.get("skipped", {}) count = 0 order = set(plan.get("queue_order", [])) for fid in reopened: + if fid in protected_ids: + continue if fid in skipped: skipped.pop(fid) count += 1 @@ -127,7 +136,12 @@ def cmd_plan_reopen(args: argparse.Namespace) -> None: append_log_entry(plan, "reopen", issue_ids=reopened, actor="user") transition_phase: str | None = None - if invalidate_postflight_scan(plan, issue_ids=reopened, state=state_data): + executable_reopened = [fid for fid in reopened if fid not in protected_ids] + if executable_reopened and invalidate_postflight_scan( + plan, + issue_ids=executable_reopened, + state=state_data, + ): result = reconcile_plan( plan, state_data, diff --git a/desloppify/app/commands/plan/override/resolve_cmd.py b/desloppify/app/commands/plan/override/resolve_cmd.py index 6dddeac4b..b4b6fd13e 100644 --- a/desloppify/app/commands/plan/override/resolve_cmd.py +++ b/desloppify/app/commands/plan/override/resolve_cmd.py @@ -3,7 +3,6 @@ from __future__ import annotations import argparse -import logging from desloppify.app.commands.helpers.attestation import ( show_attestation_requirement, @@ -12,16 +11,15 @@ validate_note_length, ) from desloppify.app.commands.helpers.command_runtime import command_runtime +from desloppify.app.commands.plan.shared.patterns import resolve_ids_from_patterns from desloppify.app.commands.resolve.cmd import cmd_resolve from desloppify.base.exception_sets import PLAN_LOAD_EXCEPTIONS -from desloppify.base.output.fallbacks import log_best_effort_failure from desloppify.base.output.terminal import colorize +from desloppify.engine._plan.triage.protection import protected_review_issue_ids from desloppify.engine._work_queue.core import ATTEST_EXAMPLE from desloppify.engine.plan_state import ( load_plan, - save_plan, ) -from desloppify.engine.plan_ops import append_log_entry from .resolve_helpers import ( check_cluster_guard, @@ -29,8 +27,6 @@ ) from .resolve_workflow import resolve_workflow_patterns -logger = logging.getLogger(__name__) - def cmd_plan_resolve(args: argparse.Namespace) -> None: """Mark issues as fixed and delegate to resolve command UX.""" @@ -75,27 +71,22 @@ def cmd_plan_resolve(args: argparse.Namespace) -> None: plan = load_plan() if check_cluster_guard(patterns, plan, state): return + protected_ids = sorted( + set(resolve_ids_from_patterns(state, patterns, plan=plan, status_filter="all")) + & protected_review_issue_ids(plan) + ) + if protected_ids: + print( + colorize( + " Cannot resolve protected review item(s): " + + ", ".join(protected_ids), + "red", + ) + ) + return except PLAN_LOAD_EXCEPTIONS: plan = None - try: - if plan is None: - plan = load_plan() - clusters = plan.get("clusters", {}) - cluster_name = next((pattern for pattern in patterns if pattern in clusters), None) - append_log_entry( - plan, - "done", - issue_ids=patterns, - cluster_name=cluster_name, - actor="user", - note=note, - ) - save_plan(plan) - except PLAN_LOAD_EXCEPTIONS as exc: - log_best_effort_failure(logger, "append plan resolve log entry", exc) - print(colorize(f" Note: unable to append plan resolve log entry ({exc}).", "dim")) - resolve_args = argparse.Namespace( status="fixed", patterns=patterns, diff --git a/desloppify/app/commands/plan/override/resolve_workflow.py b/desloppify/app/commands/plan/override/resolve_workflow.py index 152a2c501..a4e88cfda 100644 --- a/desloppify/app/commands/plan/override/resolve_workflow.py +++ b/desloppify/app/commands/plan/override/resolve_workflow.py @@ -9,26 +9,16 @@ from desloppify import state as state_mod from desloppify.app.commands.helpers.state import state_path +from desloppify.app.commands.helpers.transition_messages import emit_transition_message from desloppify.app.commands.plan.triage.review_coverage import ( has_open_review_issues, ) -from desloppify.app.commands.helpers.transition_messages import emit_transition_message -from desloppify.base.config import target_strict_score_from_config -from .resolve_helpers import blocked_triage_stages from desloppify.app.commands.plan.triage.stage_queue import ( has_triage_in_queue, inject_triage_stages, ) +from desloppify.base.config import target_strict_score_from_config from desloppify.base.output.terminal import colorize -from desloppify.engine.plan_state import ( - load_plan, - save_plan, -) -from desloppify.engine.plan_ops import ( - append_log_entry, - auto_complete_steps, - purge_ids, -) from desloppify.engine._plan.constants import ( WORKFLOW_CREATE_PLAN_ID, WORKFLOW_SCORE_CHECKPOINT_ID, @@ -42,13 +32,24 @@ maybe_append_entered_planning, maybe_append_execution_drain, ) - -_logger = logging.getLogger(__name__) +from desloppify.engine.plan_ops import ( + append_log_entry, + auto_complete_steps, + purge_ids, +) +from desloppify.engine.plan_state import ( + load_plan, + save_plan, +) from desloppify.engine.plan_triage import ( triage_manual_stage_command, triage_runner_commands, ) +from .resolve_helpers import blocked_triage_stages + +_logger = logging.getLogger(__name__) + WORKFLOW_GATE_IDS = frozenset({WORKFLOW_SCORE_CHECKPOINT_ID, WORKFLOW_CREATE_PLAN_ID}) _WORKFLOW_PLAN_JUST_RESOLVED_KEY = "workflow_plan_just_resolved" @@ -375,7 +376,9 @@ def _reconcile_if_queue_drained( return resolved_state_path = state_path(args) state_data = state_mod.load_state(resolved_state_path) - if WORKFLOW_CREATE_PLAN_ID in synthetic_ids and has_open_review_issues(state_data): + if WORKFLOW_CREATE_PLAN_ID in synthetic_ids and has_open_review_issues( + state_data, plan + ): plan.setdefault("refresh_state", {})[_WORKFLOW_PLAN_JUST_RESOLVED_KEY] = True result = reconcile_plan( plan, diff --git a/desloppify/app/commands/plan/override/skip.py b/desloppify/app/commands/plan/override/skip.py index ad1f8affd..09e70bd2e 100644 --- a/desloppify/app/commands/plan/override/skip.py +++ b/desloppify/app/commands/plan/override/skip.py @@ -14,20 +14,17 @@ ) from desloppify.app.commands.helpers.command_runtime import command_runtime from desloppify.app.commands.helpers.state import require_issue_inventory -from .io import ( - _plan_file_for_state, - save_plan_state_transactional, -) +from desloppify.app.commands.helpers.transition_messages import emit_transition_message from desloppify.app.commands.plan.shared.patterns import resolve_ids_from_patterns from desloppify.base.config import target_strict_score_from_config from desloppify.base.exception_sets import CommandError from desloppify.base.output.terminal import colorize from desloppify.base.output.user_message import print_user_message -from desloppify.app.commands.helpers.transition_messages import emit_transition_message from desloppify.engine._plan.refresh_lifecycle import ( invalidate_postflight_scan, ) from desloppify.engine._plan.sync import reconcile_plan +from desloppify.engine._plan.triage.protection import protected_review_issue_ids from desloppify.engine.plan_ops import ( SKIP_KIND_LABELS, append_log_entry, @@ -44,6 +41,11 @@ save_plan, ) +from .io import ( + _plan_file_for_state, + save_plan_state_transactional, +) + logger = logging.getLogger(__name__) _BULK_SKIP_THRESHOLD = 5 @@ -224,6 +226,16 @@ def cmd_plan_skip(args: argparse.Namespace) -> None: if not issue_ids: print(colorize(" No matching issues found.", "yellow")) return + protected_ids = sorted(set(issue_ids) & protected_review_issue_ids(plan)) + if protected_ids: + print( + colorize( + " Cannot skip protected review item(s): " + + ", ".join(protected_ids), + "red", + ) + ) + return _warn_or_block_bulk_skip(issue_ids, confirm=bool(getattr(args, "confirm", False))) diff --git a/desloppify/app/commands/plan/triage/completion_flow.py b/desloppify/app/commands/plan/triage/completion_flow.py index fe91c304d..9ae21cb64 100644 --- a/desloppify/app/commands/plan/triage/completion_flow.py +++ b/desloppify/app/commands/plan/triage/completion_flow.py @@ -8,17 +8,19 @@ from typing import Any from desloppify.base.output.terminal import colorize -from desloppify.engine._plan.refresh_lifecycle import current_lifecycle_phase -from desloppify.engine._state.progression import ( - append_progression_event, - build_triage_complete_event, -) from desloppify.engine._plan.constants import ( WORKFLOW_CREATE_PLAN_ID, WORKFLOW_SCORE_CHECKPOINT_ID, ) -from desloppify.engine._plan.policy.stale import review_issue_snapshot_hash -from desloppify.engine._plan.refresh_lifecycle import mark_postflight_scan_completed +from desloppify.engine._plan.policy.stale import triage_review_issue_snapshot_hash +from desloppify.engine._plan.refresh_lifecycle import ( + current_lifecycle_phase, + mark_postflight_scan_completed, +) +from desloppify.engine._state.progression import ( + append_progression_event, + build_triage_complete_event, +) from desloppify.engine.plan_ops import purge_ids from desloppify.engine.plan_state import Cluster, PlanModel from desloppify.engine.plan_triage import TRIAGE_IDS @@ -79,7 +81,7 @@ def _sync_completion_meta( ) -> tuple[dict[str, Any], str]: meta = ensure_triage_meta(plan) if state.get("last_scan"): - meta["issue_snapshot_hash"] = review_issue_snapshot_hash(state) + meta["issue_snapshot_hash"] = triage_review_issue_snapshot_hash(plan, state) elif not meta.get("issue_snapshot_hash"): meta.pop("issue_snapshot_hash", None) diff --git a/desloppify/app/commands/plan/triage/confirmations/basic.py b/desloppify/app/commands/plan/triage/confirmations/basic.py index 9c8fe77b8..9e053ab20 100644 --- a/desloppify/app/commands/plan/triage/confirmations/basic.py +++ b/desloppify/app/commands/plan/triage/confirmations/basic.py @@ -6,14 +6,18 @@ from desloppify.base.output.terminal import colorize from desloppify.base.output.user_message import print_user_message +from desloppify.engine._plan.triage.protection import ( + clear_protected_triage_artifacts, + protected_review_issue_ids_from_meta, +) +from ..services import TriageServices, default_triage_services +from ..stages.records import TriageStages from .shared import ( StageConfirmationRequest, ensure_stage_is_confirmable, finalize_stage_confirmation, ) -from ..services import TriageServices, default_triage_services -from ..stages.records import TriageStages # Observe verdicts that trigger auto-skip on confirmation _AUTO_SKIP_VERDICTS = frozenset({"false positive", "exaggerated"}) @@ -150,15 +154,19 @@ def _apply_observe_auto_skips( """ from desloppify.state_io import utc_now + clear_protected_triage_artifacts(plan) dispositions = meta.get("issue_dispositions", {}) if not dispositions: return 0 skipped = plan.setdefault("skipped", {}) queue_order = plan.get("queue_order", []) + protected_ids = protected_review_issue_ids_from_meta(meta) count = 0 for issue_id, disp in dispositions.items(): + if issue_id in protected_ids: + continue verdict = disp.get("verdict", "") if verdict not in _AUTO_SKIP_VERDICTS: continue @@ -198,8 +206,10 @@ def _undo_observe_auto_skips(plan: dict, meta: dict) -> int: Returns the number of entries un-skipped. """ + clear_protected_triage_artifacts(plan) dispositions = meta.get("issue_dispositions", {}) skipped = plan.get("skipped", {}) + protected_ids = protected_review_issue_ids_from_meta(meta) count = 0 # Find all entries with decision_source == "observe_auto" @@ -208,6 +218,8 @@ def _undo_observe_auto_skips(plan: dict, meta: dict) -> int: if disp.get("decision_source") == "observe_auto" } for issue_id in auto_skipped_ids: + if issue_id in protected_ids: + continue entry = skipped.get(issue_id) if entry and entry.get("kind") == "triage_observe_auto": del skipped[issue_id] diff --git a/desloppify/app/commands/plan/triage/confirmations/enrich.py b/desloppify/app/commands/plan/triage/confirmations/enrich.py index 40b315013..2bf5e63cf 100644 --- a/desloppify/app/commands/plan/triage/confirmations/enrich.py +++ b/desloppify/app/commands/plan/triage/confirmations/enrich.py @@ -7,20 +7,26 @@ from desloppify.base.output.terminal import colorize from desloppify.base.output.user_message import print_user_message -from .basic import MIN_ATTESTATION_LEN, validate_attestation -from .shared import ( - StageConfirmationRequest, - ensure_stage_is_confirmable, - finalize_stage_confirmation, -) from ..services import TriageServices, default_triage_services -from ..stages.helpers import scoped_manual_clusters_with_issues -from ..review_coverage import active_triage_issue_ids +from ..stages.helpers import ( + active_triage_issue_scope, + scoped_manual_clusters_with_issues, +) from ..validation.enrich_quality import ( EnrichQualityIssue as _ConfirmationCheckIssue, +) +from ..validation.enrich_quality import ( EnrichQualityReport as _ConfirmationCheckReport, +) +from ..validation.enrich_quality import ( evaluate_enrich_quality, ) +from .basic import MIN_ATTESTATION_LEN, validate_attestation +from .shared import ( + StageConfirmationRequest, + ensure_stage_is_confirmable, + finalize_stage_confirmation, +) def _print_confirmation_failure( @@ -179,7 +185,7 @@ def confirm_enrich( checks = _collect_enrich_level_confirmation_checks( plan, include_stale_issue_ref_warning=True, - triage_issue_ids=active_triage_issue_ids(plan, state) or None, + triage_issue_ids=active_triage_issue_scope(plan, state), ) print(colorize(" Stage: ENRICH — Make steps executor-ready (detail, refs)", "bold")) @@ -232,7 +238,7 @@ def confirm_sense_check( checks = _collect_enrich_level_confirmation_checks( plan, include_stale_issue_ref_warning=False, - triage_issue_ids=active_triage_issue_ids(plan, state) or None, + triage_issue_ids=active_triage_issue_scope(plan, state), ) print(colorize(" Stage: SENSE-CHECK — Verify accuracy & cross-cluster deps", "bold")) diff --git a/desloppify/app/commands/plan/triage/confirmations/organize.py b/desloppify/app/commands/plan/triage/confirmations/organize.py index 6ee5dc1a3..da0624871 100644 --- a/desloppify/app/commands/plan/triage/confirmations/organize.py +++ b/desloppify/app/commands/plan/triage/confirmations/organize.py @@ -7,18 +7,12 @@ from desloppify.base.output.terminal import colorize from desloppify.base.output.user_message import print_user_message -from .basic import MIN_ATTESTATION_LEN, validate_attestation -from .shared import ( - StageConfirmationRequest, - ensure_stage_is_confirmable, - finalize_stage_confirmation, -) -from ..display.dashboard import show_plan_summary from ..completion_flow import count_log_activity_since +from ..display.dashboard import show_plan_summary from ..review_coverage import ( cluster_issue_ids, - open_review_ids_from_state, triage_coverage, + triage_open_review_ids_from_state, ) from ..services import TriageServices, default_triage_services from ..validation.enrich_checks import ( @@ -26,6 +20,12 @@ _clusters_with_directory_scatter, _clusters_with_high_step_ratio, ) +from .basic import MIN_ATTESTATION_LEN, validate_attestation +from .shared import ( + StageConfirmationRequest, + ensure_stage_is_confirmable, + finalize_stage_confirmation, +) def _require_enriched_clusters(plan: dict, state: dict | None = None) -> bool: @@ -167,7 +167,10 @@ def confirm_organize( all_clusters = plan.get("clusters", {}) _print_orphaned_cluster_notes(all_clusters) - organized, total, _ = triage_coverage(plan, open_review_ids=open_review_ids_from_state(state)) + organized, total, _ = triage_coverage( + plan, + open_review_ids=triage_open_review_ids_from_state(plan, state), + ) if not finalize_stage_confirmation( plan=plan, stages=stages, diff --git a/desloppify/app/commands/plan/triage/display/layout.py b/desloppify/app/commands/plan/triage/display/layout.py index eff987074..d6a1acad3 100644 --- a/desloppify/app/commands/plan/triage/display/layout.py +++ b/desloppify/app/commands/plan/triage/display/layout.py @@ -5,8 +5,8 @@ from collections import defaultdict from desloppify.app.commands.helpers.issue_id_display import short_issue_id +from desloppify.base.output.terminal import colorize from desloppify.engine._plan.constants import is_synthetic_id -from desloppify.engine.plan_triage import TriageSnapshot from desloppify.engine.plan_triage import ( TRIAGE_CMD_CLUSTER_ADD, TRIAGE_CMD_CLUSTER_CREATE, @@ -21,20 +21,20 @@ TRIAGE_CMD_RUN_STAGES_CLAUDE, TRIAGE_CMD_RUN_STAGES_CODEX, TRIAGE_CMD_RUN_STAGES_ROVODEV, - triage_runner_commands, TRIAGE_CMD_STRATEGIZE, + TriageSnapshot, + triage_runner_commands, ) -from desloppify.base.output.terminal import colorize -from .primitives import print_stage_progress from ..review_coverage import ( cluster_issue_ids, find_cluster_for, manual_clusters_with_issues, - open_review_ids_from_state, triage_coverage, + triage_open_review_ids_from_state, ) from ..stages.helpers import unenriched_clusters +from .primitives import print_stage_progress def _print_runner_paths( @@ -337,7 +337,10 @@ def show_plan_summary(plan: dict, state: dict) -> None: cluster_name = find_cluster_for(fid, active) print(f" {i + 1}. [{detector}] {summary}{f' ({cluster_name})' if cluster_name else ''}") - organized, total, _ = triage_coverage(plan, open_review_ids=open_review_ids_from_state(state)) + organized, total, _ = triage_coverage( + plan, + open_review_ids=triage_open_review_ids_from_state(plan, state), + ) pct = int(organized / total * 100) if total else 0 print(colorize(f"\n Coverage: {organized}/{total} in clusters ({pct}%)", "bold")) diff --git a/desloppify/app/commands/plan/triage/review_coverage.py b/desloppify/app/commands/plan/triage/review_coverage.py index f9cf97820..f5b28b0e2 100644 --- a/desloppify/app/commands/plan/triage/review_coverage.py +++ b/desloppify/app/commands/plan/triage/review_coverage.py @@ -3,20 +3,36 @@ from __future__ import annotations from desloppify.app.commands.plan.shared.cluster_membership import cluster_issue_ids +from desloppify.engine._plan.policy.stale import open_review_ids, triage_open_review_ids from desloppify.engine._plan.triage.lifecycle import ensure_active_triage_issue_ids +from desloppify.engine._plan.triage.protection import protected_review_issue_ids from desloppify.engine._state.schema import StateModel from desloppify.engine.plan_state import Cluster, PlanModel from desloppify.engine.plan_triage import ( active_triage_issue_ids as _active_triage_issue_ids, +) +from desloppify.engine.plan_triage import ( coverage_open_ids as _coverage_open_ids, +) +from desloppify.engine.plan_triage import ( find_cluster_for as _find_cluster_for, +) +from desloppify.engine.plan_triage import ( live_active_triage_issue_ids as _live_active_triage_issue_ids, +) +from desloppify.engine.plan_triage import ( manual_clusters_with_issues as _manual_clusters_with_issues, +) +from desloppify.engine.plan_triage import ( plan_review_ids as _plan_review_ids, +) +from desloppify.engine.plan_triage import ( triage_coverage as _triage_coverage, +) +from desloppify.engine.plan_triage import ( undispositioned_triage_issue_ids as _undispositioned_triage_issue_ids, ) -from desloppify.engine._plan.policy.stale import open_review_ids + from .plan_state_access import ensure_triage_meta _ACTIVE_TRIAGE_ISSUE_IDS_KEY = "active_triage_issue_ids" @@ -29,9 +45,20 @@ def open_review_ids_from_state(state: StateModel) -> set[str]: return open_review_ids(state) -def has_open_review_issues(state: StateModel | dict | None) -> bool: - """Return True when any open review issues exist.""" - return bool(open_review_ids_from_state(state or {})) +def triage_open_review_ids_from_state(plan: PlanModel, state: StateModel) -> set[str]: + """Return live review IDs that remain in automated triage scope.""" + return open_review_ids_from_state(state) - protected_review_issue_ids(plan) + + +def has_open_review_issues( + state: StateModel | dict | None, + plan: PlanModel | None = None, +) -> bool: + """Return True when any review issues remain eligible for triage work.""" + state_data = state or {} + if plan is None: + return bool(open_review_ids_from_state(state_data)) + return bool(triage_open_review_ids(plan, state_data)) def plan_review_ids(plan: PlanModel) -> list[str]: @@ -123,6 +150,7 @@ def find_cluster_for(fid: str, clusters: dict[str, Cluster]) -> str | None: "open_review_ids_from_state", "plan_review_ids", "sync_undispositioned_triage_meta", + "triage_open_review_ids_from_state", "triage_coverage", "undispositioned_triage_issue_ids", ] diff --git a/desloppify/app/commands/plan/triage/runner/orchestrator_codex_pipeline.py b/desloppify/app/commands/plan/triage/runner/orchestrator_codex_pipeline.py index 4151670a1..a72dcd93c 100644 --- a/desloppify/app/commands/plan/triage/runner/orchestrator_codex_pipeline.py +++ b/desloppify/app/commands/plan/triage/runner/orchestrator_codex_pipeline.py @@ -19,9 +19,10 @@ from desloppify.base.exception_sets import CommandError from desloppify.base.output.terminal import colorize -from ..stage_queue import has_triage_in_queue, inject_triage_stages from ..lifecycle import TriageLifecycleDeps, ensure_triage_started from ..services import TriageServices, default_triage_services +from ..stage_queue import has_triage_in_queue, inject_triage_stages +from ..stages.helpers import value_check_targets from ..validation.reflect_accounting import ( analyze_reflect_issue_accounting, validate_reflect_accounting, @@ -44,12 +45,23 @@ DEFAULT_STAGE_HANDLERS, StageExecutionDependencies, StageHandler, +) +from .orchestrator_codex_pipeline_execution import ( execute_stage as execute_stage_impl, +) +from .orchestrator_codex_pipeline_execution import ( read_stage_output as read_stage_output_impl, ) from .orchestrator_common import STAGES, run_stamp from .stage_prompts import build_stage_prompt -from ..stages.helpers import value_check_targets +from .stage_runner_override import ( # re-exported for backwards compat + active_runner_name, + active_stage_runner, # noqa: F401 + clear_stage_runner_override, # noqa: F401 + set_stage_runner_override, # noqa: F401 + stage_runner_override, +) + _STAGE_HANDLERS: dict[str, StageHandler] = DEFAULT_STAGE_HANDLERS # Module-level override for the per-stage runner. The default (``None``) @@ -57,13 +69,6 @@ # :mod:`rovodev_pipeline` swap this for the rovodev stage runner during # the lifetime of one ``run_codex_pipeline`` call so that the existing # pipeline can drive any subprocess backend without further refactoring. -from .stage_runner_override import ( # re-exported for backwards compat - active_runner_name, - active_stage_runner, - clear_stage_runner_override, - set_stage_runner_override, - stage_runner_override, -) _analyze_reflect_issue_accounting = analyze_reflect_issue_accounting _validate_reflect_issue_accounting = validate_reflect_accounting @@ -183,11 +188,7 @@ def _run_stage_sequence( si = pipeline_context.services.collect_triage_input(plan, pipeline_context.state) if stage == "sense-check": si.value_check_targets = value_check_targets(plan, pipeline_context.state) - setattr( - pipeline_context.args, - "sense_check_value_targets", - list(si.value_check_targets), - ) + pipeline_context.args.sense_check_value_targets = list(si.value_check_targets) last_triage_input = si execution_result = execute_stage_impl( StageRunContext( @@ -369,7 +370,13 @@ def run_codex_pipeline( ), ) if getattr(start_outcome, "status", None) == "blocked": - return + reason = str(getattr(start_outcome, "reason", "unknown")).strip() or "unknown" + raise CommandError( + "Triage runner blocked before executing any stage " + f"({reason}). Inspect `desloppify plan triage` and resolve or confirm " + "the recorded stage before rerunning; use an attested restart only when intentional.", + exit_code=1, + ) plan = resolved_services.load_plan() stamp = run_stamp() @@ -404,7 +411,7 @@ def run_codex_pipeline( output_dir=output_dir, logs_dir=logs_dir, run_log_path=run_log_path, - cli_command=str(cli_helper), + cli_command=shlex.quote(str(cli_helper)), append_run_log=append_run_log, ) diff --git a/desloppify/app/commands/plan/triage/runner/orchestrator_codex_pipeline_execution.py b/desloppify/app/commands/plan/triage/runner/orchestrator_codex_pipeline_execution.py index 9e9286b52..dab7f4388 100644 --- a/desloppify/app/commands/plan/triage/runner/orchestrator_codex_pipeline_execution.py +++ b/desloppify/app/commands/plan/triage/runner/orchestrator_codex_pipeline_execution.py @@ -42,7 +42,7 @@ class StageHandler: """Per-stage execution/record hooks for the codex triage pipeline.""" run_parallel: Callable[[StageRunContext], TriageStageRunResult] | None = None - record_report: Callable[[str, argparse.Namespace, TriageServices], None] | None = None + record_report: Callable[[str, argparse.Namespace, TriageServices], bool | None] | None = None prompt_mode: PromptMode = "output_only" @@ -80,7 +80,7 @@ def _record_reflect_report( report: str, args: argparse.Namespace, services: TriageServices, -) -> None: +) -> bool: from ..stages.commands import cmd_stage_reflect record_args = argparse.Namespace( @@ -88,7 +88,7 @@ def _record_reflect_report( report=report, state=getattr(args, "state", None), ) - cmd_stage_reflect(record_args, services=services) + return cmd_stage_reflect(record_args, services=services) def _record_sense_check_report( @@ -646,7 +646,19 @@ def _record_stage_report_if_needed( append_run_log=context.append_run_log, ) - handler.record_report(report, context.args, context.services) + record_result = handler.record_report(report, context.args, context.services) + if record_result is False: + return _failure_result( + stage=stage, + elapsed=elapsed, + error="reflect_report_rejected", + append_run_log=context.append_run_log, + log_event="stage-record-rejected", + printed_message=( + f" Stage {stage}: report was rejected by record-time validation. " + "See the validation output above." + ), + ) plan_after_record = context.services.load_plan() if not stage_report_recorded(plan_after_record, stage): return _failure_result( diff --git a/desloppify/app/commands/plan/triage/runner/stage_validation.py b/desloppify/app/commands/plan/triage/runner/stage_validation.py index 06ee0c8e9..3734f2518 100644 --- a/desloppify/app/commands/plan/triage/runner/stage_validation.py +++ b/desloppify/app/commands/plan/triage/runner/stage_validation.py @@ -7,27 +7,20 @@ from desloppify.engine.plan_triage import TriageInput +from ..completion_flow import count_log_activity_since +from ..observe_batches import observe_dimension_breakdown +from ..review_coverage import ( + cluster_issue_ids, + triage_open_review_ids_from_state, +) from ..stages.evidence_parsing import ( parse_observe_evidence, + parse_value_check_decision_ledger, validate_observe_evidence, validate_reflect_skip_evidence, validate_report_has_file_paths, validate_report_references_clusters, ) -from ..validation.enrich_quality import evaluate_enrich_quality -from ..validation.completion_policy import evaluate_completion_readiness -from ..validation.enrich_checks import ( - _cluster_file_overlaps, - _clusters_with_directory_scatter, - _clusters_with_high_step_ratio, -) -from ..completion_flow import count_log_activity_since -from ..observe_batches import observe_dimension_breakdown -from ..review_coverage import ( - active_triage_issue_ids, - cluster_issue_ids, - open_review_ids_from_state, -) from ..stages.helpers import ( active_triage_issue_scope, scoped_manual_clusters_with_issues, @@ -35,7 +28,13 @@ unenriched_clusters, value_check_targets, ) -from ..stages.evidence_parsing import parse_value_check_decision_ledger +from ..validation.completion_policy import evaluate_completion_readiness +from ..validation.enrich_checks import ( + _cluster_file_overlaps, + _clusters_with_directory_scatter, + _clusters_with_high_step_ratio, +) +from ..validation.enrich_quality import evaluate_enrich_quality @dataclass(frozen=True) @@ -185,7 +184,11 @@ def _validate_organize_stage(plan: dict, state: dict, stages: dict) -> tuple[boo if "organize" not in stages: return False, "Organize stage not recorded." triage_scope = active_triage_issue_scope(plan, state) - open_review_ids = open_review_ids_from_state(state) if triage_scope is None else triage_scope + open_review_ids = ( + triage_open_review_ids_from_state(plan, state) + if triage_scope is None + else triage_scope + ) manual = scoped_manual_clusters_with_issues(plan, state) if not open_review_ids and not manual: report = stages["organize"].get("report", "") @@ -238,7 +241,7 @@ def _validate_enrich_stage( plan, repo_root, phase_label="enrich", - triage_issue_ids=active_triage_issue_ids(plan, state) or None, + triage_issue_ids=active_triage_issue_scope(plan, state), ) if failures: return False, failures[0].message @@ -260,9 +263,13 @@ def _validate_sense_check_stage( if len(report) < 100: return False, f"Sense-check report too short ({len(report)} chars, need 100+)." manual_clusters = scoped_manual_clusters_with_issues(plan, state) - triage_issue_ids = active_triage_issue_ids(plan, state) or None triage_scope = active_triage_issue_scope(plan, state) - open_review_ids = open_review_ids_from_state(state) if triage_scope is None else triage_scope + triage_issue_ids = triage_scope + open_review_ids = ( + triage_open_review_ids_from_state(plan, state) + if triage_scope is None + else triage_scope + ) if not open_review_ids and not manual_clusters: return True, "" failures = run_enrich_quality_checks( diff --git a/desloppify/app/commands/plan/triage/stage_queue.py b/desloppify/app/commands/plan/triage/stage_queue.py index 6385dabe3..0bb2faa71 100644 --- a/desloppify/app/commands/plan/triage/stage_queue.py +++ b/desloppify/app/commands/plan/triage/stage_queue.py @@ -6,13 +6,14 @@ from desloppify.base.output.terminal import colorize from desloppify.engine._plan.triage.lifecycle import ( - clear_triage_stage_skips, has_triage_in_queue, inject_triage_stages, ) +from desloppify.engine._plan.triage.protection import ( + protected_review_issue_ids_from_meta, +) from desloppify.engine.plan_ops import purge_ids from desloppify.engine.plan_state import PlanModel -from desloppify.engine.plan_triage import TRIAGE_STAGE_IDS STAGE_ORDER = ["strategize", "observe", "reflect", "organize", "enrich", "sense-check"] @@ -32,6 +33,9 @@ def cascade_clear_dispositions(meta: dict[str, Any], from_stage: str) -> None: dispositions = meta.get("issue_dispositions") if not dispositions: return + protected_ids = protected_review_issue_ids_from_meta(meta) + for issue_id in protected_ids: + dispositions.pop(issue_id, None) if from_stage == "observe": meta["issue_dispositions"] = {} elif from_stage == "reflect": diff --git a/desloppify/app/commands/plan/triage/stages/completion.py b/desloppify/app/commands/plan/triage/stages/completion.py index a2e8c6f0d..4ea55bf04 100644 --- a/desloppify/app/commands/plan/triage/stages/completion.py +++ b/desloppify/app/commands/plan/triage/stages/completion.py @@ -7,8 +7,15 @@ from desloppify.base.output.terminal import colorize from desloppify.base.output.user_message import print_user_message -from .records import record_confirm_existing_completion -from .rendering import _print_complete_summary +from ..completion_flow import apply_completion +from ..review_coverage import ( + manual_clusters_with_issues, + sync_undispositioned_triage_meta, + triage_coverage, + triage_open_review_ids_from_state, +) +from ..services import TriageServices, default_triage_services +from ..stage_queue import has_triage_in_queue from ..validation.completion_policy import ( _completion_strategy_valid, _confirm_existing_stages_valid, @@ -29,16 +36,9 @@ _require_sense_check_stage_for_complete, ) from ..validation.enrich_checks import _underspecified_steps -from ..completion_flow import apply_completion -from ..review_coverage import ( - manual_clusters_with_issues, - open_review_ids_from_state, - sync_undispositioned_triage_meta, - triage_coverage, -) -from ..stage_queue import has_triage_in_queue -from ..services import TriageServices, default_triage_services from .helpers import active_triage_issue_scope, triage_scoped_plan +from .records import record_confirm_existing_completion +from .rendering import _print_complete_summary def _print_completion_coverage_warning(*, organized: int, total: int) -> None: @@ -171,7 +171,11 @@ def _cmd_triage_complete( state = resolved_services.command_runtime(args).state triage_scope = active_triage_issue_scope(plan, state) - review_ids = open_review_ids_from_state(state) if triage_scope is None else triage_scope + review_ids = ( + triage_open_review_ids_from_state(plan, state) + if triage_scope is None + else triage_scope + ) # Organize gate if not _require_organize_stage_for_complete(plan=plan, meta=meta, stages=stages): diff --git a/desloppify/app/commands/plan/triage/stages/enrich.py b/desloppify/app/commands/plan/triage/stages/enrich.py index 496bb4cb4..2a0b7684c 100644 --- a/desloppify/app/commands/plan/triage/stages/enrich.py +++ b/desloppify/app/commands/plan/triage/stages/enrich.py @@ -11,8 +11,10 @@ from desloppify.base.output.user_message import print_user_message from desloppify.engine.plan_triage import compute_triage_progress -from .records import record_enrich_stage, resolve_reusable_report -from ..validation.enrich_quality import evaluate_enrich_quality +from ..completion_flow import count_log_activity_since +from ..review_coverage import triage_open_review_ids_from_state +from ..services import TriageServices, default_triage_services +from ..stage_queue import has_triage_in_queue, print_cascade_clear_feedback from ..validation.enrich_checks import ( _enrich_report_or_error, _require_organize_stage_for_enrich, @@ -20,13 +22,9 @@ _steps_without_effort, _underspecified_steps, ) -from ..completion_flow import count_log_activity_since -from ..review_coverage import ( - active_triage_issue_ids, - open_review_ids_from_state, -) -from ..stage_queue import has_triage_in_queue, print_cascade_clear_feedback -from ..services import TriageServices, default_triage_services +from ..validation.enrich_quality import evaluate_enrich_quality +from .helpers import active_triage_issue_scope +from .records import record_enrich_stage, resolve_reusable_report ColorizeFn = Callable[[str, str], str] @@ -120,7 +118,7 @@ def _require_cluster_update_activity( return True activity = deps.count_log_activity_since(plan, organize_ts) update_ops = activity.get("cluster_update", 0) - if update_ops != 0 or not open_review_ids_from_state(state): + if update_ops != 0 or not triage_open_review_ids_from_state(plan, state): return True if attestation and len(attestation.strip()) >= 40: print( @@ -268,7 +266,7 @@ def run_stage_enrich( if get_project_root is None: from desloppify.base.discovery.paths import get_project_root - triage_ids = active_triage_issue_ids(plan, state) or None + triage_ids = active_triage_issue_scope(plan, state) quality_report = evaluate_enrich_quality( plan, get_project_root(), diff --git a/desloppify/app/commands/plan/triage/stages/helpers.py b/desloppify/app/commands/plan/triage/stages/helpers.py index bf1860d8f..ccc6dcd54 100644 --- a/desloppify/app/commands/plan/triage/stages/helpers.py +++ b/desloppify/app/commands/plan/triage/stages/helpers.py @@ -4,7 +4,10 @@ from desloppify.base.output.terminal import colorize from desloppify.engine._plan.constants import is_synthetic_id -from desloppify.engine._state.issue_semantics import is_review_work_item, is_triage_finding +from desloppify.engine._plan.triage.protection import protected_review_issue_ids +from desloppify.engine._state.issue_semantics import ( + is_review_work_item, +) from desloppify.engine.plan_triage import TRIAGE_IDS from ..review_coverage import ( @@ -61,8 +64,19 @@ def active_triage_issue_scope( `None` means "do not scope" for legacy/non-triage flows. An empty set means a frozen triage run exists but none of its issues are live. """ + meta = plan.get("epic_triage_meta", {}) + raw_scope = meta.get("active_triage_issue_ids") if isinstance(meta, dict) else None + has_explicit_scope = isinstance(raw_scope, list) + has_explicit_issue = has_explicit_scope and any( + isinstance(issue_id, str) and issue_id.strip() for issue_id in raw_scope + ) + if has_explicit_scope and not has_explicit_issue: + return set() + frozen = active_triage_issue_ids(plan, state) if not frozen: + if has_explicit_scope: + return set() return None if state is None: return frozen @@ -153,6 +167,7 @@ def unclustered_review_issues(plan: dict, state: dict | None = None) -> list[str skipped_ids = { fid for fid in (plan.get("skipped", {}) or {}).keys() if isinstance(fid, str) } + protected_ids = protected_review_issue_ids(plan) if state is not None: # Only count review-type issues for ledger purposes — mechanical @@ -162,16 +177,17 @@ def unclustered_review_issues(plan: dict, state: dict | None = None) -> list[str fid for fid, finding in (state.get("work_items") or state.get("issues", {})).items() if finding.get("status") == "open" and is_review_work_item(finding) + and fid not in protected_ids ] - frozen_ids = (plan.get("epic_triage_meta", {}) or {}).get("active_triage_issue_ids") - if isinstance(frozen_ids, list) and frozen_ids: - frozen_id_set = live_active_triage_issue_ids(plan, state) - review_ids = [fid for fid in review_ids if fid in frozen_id_set] + triage_scope = active_triage_issue_scope(plan, state) + if triage_scope is not None: + review_ids = [fid for fid in review_ids if fid in triage_scope] else: review_ids = [ fid for fid in plan.get("queue_order", []) if not is_synthetic_id(fid) and (fid.startswith("review::") or fid.startswith("concerns::")) + and fid not in protected_ids ] return [ diff --git a/desloppify/app/commands/plan/triage/stages/observe.py b/desloppify/app/commands/plan/triage/stages/observe.py index 04bf4081b..4587f9ca7 100644 --- a/desloppify/app/commands/plan/triage/stages/observe.py +++ b/desloppify/app/commands/plan/triage/stages/observe.py @@ -6,16 +6,20 @@ from desloppify.base.output.terminal import colorize from desloppify.base.output.user_message import print_user_message +from desloppify.engine._plan.triage.protection import ( + clear_protected_triage_artifacts, + protected_review_issue_ids, +) +from ..lifecycle import TriageLifecycleDeps, ensure_triage_started +from ..observe_batches import observe_dimension_breakdown +from ..services import TriageServices, default_triage_services from ..stage_queue import ( cascade_clear_dispositions, has_triage_in_queue, inject_triage_stages, print_cascade_clear_feedback, ) -from ..lifecycle import TriageLifecycleDeps, ensure_triage_started -from ..observe_batches import observe_dimension_breakdown -from ..services import TriageServices, default_triage_services from .flow_helpers import validate_stage_report_length from .records import record_observe_stage, resolve_reusable_report from .rendering import _print_observe_report_requirement @@ -53,6 +57,8 @@ def cmd_stage_observe( return meta = plan.setdefault("epic_triage_meta", {}) + protected_ids = protected_review_issue_ids(plan) + clear_protected_triage_artifacts(plan, state) stages = meta.setdefault("triage_stages", {}) existing_stage = stages.get("observe") @@ -142,7 +148,7 @@ def cmd_stage_observe( dispositions: dict[str, dict] = {} for entry in evidence.entries: full_id = resolve_short_hash_to_full_id(entry.issue_hash, valid_ids) - if full_id: + if full_id and full_id not in protected_ids: dispositions[full_id] = { "verdict": entry.verdict, "verdict_reasoning": entry.verdict_reasoning, diff --git a/desloppify/app/commands/plan/triage/stages/organize.py b/desloppify/app/commands/plan/triage/stages/organize.py index 676580868..be23ba6c4 100644 --- a/desloppify/app/commands/plan/triage/stages/organize.py +++ b/desloppify/app/commands/plan/triage/stages/organize.py @@ -6,15 +6,11 @@ from desloppify.base.output.terminal import colorize -from ..display.dashboard import print_organize_result from ..completion_flow import count_log_activity_since -from ..review_coverage import open_review_ids_from_state -from ..stage_queue import has_triage_in_queue +from ..display.dashboard import print_organize_result +from ..review_coverage import triage_open_review_ids_from_state from ..services import TriageServices, default_triage_services -from ..validation.stage_policy import ( - ReflectAutoConfirmDeps, - auto_confirm_reflect_for_organize, -) +from ..stage_queue import has_triage_in_queue from ..validation.organize_policy import ( _clusters_enriched_or_error, _manual_clusters_or_error, @@ -23,7 +19,11 @@ _validate_organize_against_ledger_or_error, validate_backlog_promotions_executed, ) -from ..validation.stage_policy import require_prerequisite +from ..validation.stage_policy import ( + ReflectAutoConfirmDeps, + auto_confirm_reflect_for_organize, + require_prerequisite, +) from .records import record_organize_stage @@ -110,7 +110,7 @@ def _validate_organize_submission( is_reuse: bool, services: TriageServices, ) -> tuple[list[str], str] | None: - open_review_ids = open_review_ids_from_state(state) + open_review_ids = triage_open_review_ids_from_state(plan, state) triage_input = services.collect_triage_input(plan, state) if not auto_confirm_reflect_for_organize( args=args, @@ -233,7 +233,7 @@ def _cmd_stage_organize( runtime = resolved_services.command_runtime(args) state = runtime.state - open_review_ids = open_review_ids_from_state(state) + open_review_ids = triage_open_review_ids_from_state(plan, state) validated = _validate_organize_submission( args=args, diff --git a/desloppify/app/commands/plan/triage/stages/reflect.py b/desloppify/app/commands/plan/triage/stages/reflect.py index 0c76cb405..f044a229b 100644 --- a/desloppify/app/commands/plan/triage/stages/reflect.py +++ b/desloppify/app/commands/plan/triage/stages/reflect.py @@ -5,11 +5,19 @@ import argparse from desloppify.base.output.terminal import colorize +from desloppify.engine._plan.triage.protection import ( + clear_protected_triage_artifacts, + protected_review_issue_ids, +) from desloppify.state_io import utc_now from ..display.dashboard import print_reflect_result -from ..stage_queue import cascade_clear_dispositions, cascade_clear_later_confirmations, has_triage_in_queue from ..services import TriageServices, default_triage_services +from ..stage_queue import ( + cascade_clear_dispositions, + cascade_clear_later_confirmations, + has_triage_in_queue, +) from ..validation.reflect_accounting import ( BacklogDecision, ReflectDisposition, @@ -198,6 +206,8 @@ def _persist_reflect_stage( services: TriageServices, ) -> tuple[dict, list[str]]: stages = meta.setdefault("triage_stages", {}) + clear_protected_triage_artifacts(plan) + protected_ids = protected_review_issue_ids(plan) # On fresh reflect run, cascade-clear reflect decisions from dispositions if not is_reuse: @@ -213,11 +223,16 @@ def _persist_reflect_stage( "duplicate_issue_ids": duplicate_ids, "recurring_dims": recurring_dims, } - if disposition_ledger: - reflect_stage["disposition_ledger"] = [d.to_dict() for d in disposition_ledger] + scoped_ledger = [ + disposition + for disposition in disposition_ledger + if disposition.issue_id not in protected_ids + ] + if scoped_ledger: + reflect_stage["disposition_ledger"] = [d.to_dict() for d in scoped_ledger] # Write reflect decisions to the disposition map dispositions = meta.setdefault("issue_dispositions", {}) - for d in disposition_ledger: + for d in scoped_ledger: entry = dispositions.setdefault(d.issue_id, {}) decision = "skip" if d.decision == "permanent_skip" else d.decision entry["decision"] = decision @@ -248,8 +263,8 @@ def _cmd_stage_reflect( args: argparse.Namespace, *, services: TriageServices | None = None, -) -> None: - """Record the REFLECT stage: compare current issues against completed work.""" +) -> bool: + """Record the REFLECT stage and report whether the submission was accepted.""" report: str | None = getattr(args, "report", None) attestation: str | None = getattr(args, "attestation", None) @@ -260,7 +275,7 @@ def _cmd_stage_reflect( if not has_triage_in_queue(plan): print(colorize(" No planning stages in the queue — nothing to reflect on.", "yellow")) - return + return False meta = plan.get("epic_triage_meta", {}) stages = meta.get("triage_stages", {}) @@ -269,7 +284,7 @@ def _cmd_stage_reflect( report, is_reuse = resolve_reusable_report(report, existing_stage) if not report: _print_reflect_report_requirement() - return + return False submission = _validate_reflect_submission( report=report, @@ -280,7 +295,7 @@ def _cmd_stage_reflect( services=resolved_services, ) if submission is None: - return + return False ( triage_input, issue_count, recurring, recurring_dims, cited_ids, missing_ids, duplicate_ids, disposition_ledger, @@ -312,15 +327,16 @@ def _cmd_stage_reflect( cleared=cleared, stages=stages, ) + return True def cmd_stage_reflect( args: argparse.Namespace, *, services: TriageServices | None = None, -) -> None: +) -> bool: """Public entrypoint for reflect stage recording.""" - _cmd_stage_reflect(args, services=services) + return _cmd_stage_reflect(args, services=services) __all__ = ["_cmd_stage_reflect", "cmd_stage_reflect"] diff --git a/desloppify/app/commands/plan/triage/stages/sense_check.py b/desloppify/app/commands/plan/triage/stages/sense_check.py index 7a4c40856..cbb4bc23a 100644 --- a/desloppify/app/commands/plan/triage/stages/sense_check.py +++ b/desloppify/app/commands/plan/triage/stages/sense_check.py @@ -9,9 +9,9 @@ from desloppify.base.output.terminal import colorize -from .records import record_sense_check_stage, resolve_reusable_report -from .helpers import value_check_targets -from ..validation.enrich_quality import evaluate_enrich_quality +from ..review_coverage import triage_open_review_ids_from_state +from ..services import TriageServices, default_triage_services +from ..stage_queue import has_triage_in_queue, print_cascade_clear_feedback from ..validation.enrich_checks import ( _steps_missing_issue_refs, _steps_with_bad_paths, @@ -19,10 +19,14 @@ _steps_without_effort, _underspecified_steps, ) -from ..review_coverage import active_triage_issue_ids, open_review_ids_from_state -from ..stage_queue import has_triage_in_queue, print_cascade_clear_feedback -from ..services import TriageServices, default_triage_services +from ..validation.enrich_quality import evaluate_enrich_quality from .enrich import ColorizeFn +from .helpers import ( + active_triage_issue_scope, + scoped_manual_clusters_with_issues, + value_check_targets, +) +from .records import record_sense_check_stage, resolve_reusable_report @dataclass(frozen=True) @@ -66,7 +70,7 @@ def _sense_check_quality_problems( from desloppify.base.discovery.paths import get_project_root repo_root = get_project_root() - triage_ids = active_triage_issue_ids(plan, state) or None + triage_ids = active_triage_issue_scope(plan, state) quality_report = evaluate_enrich_quality( plan, repo_root, @@ -118,13 +122,18 @@ def _sense_check_evidence_failures( validate_report_has_file_paths, validate_report_references_clusters, ) - from ..review_coverage import manual_clusters_with_issues failures: list[object] = [] - if open_review_ids_from_state(state): + triage_scope = active_triage_issue_scope(plan, state) + open_review_ids = ( + triage_open_review_ids_from_state(plan, state) + if triage_scope is None + else triage_scope + ) + if open_review_ids: failures.extend(validate_report_has_file_paths(report) or []) - cluster_names = manual_clusters_with_issues(plan) + cluster_names = scoped_manual_clusters_with_issues(plan, state) if cluster_names: failures.extend(validate_report_references_clusters(report, cluster_names) or []) diff --git a/desloppify/app/commands/plan/triage/stages/strategize.py b/desloppify/app/commands/plan/triage/stages/strategize.py index 671fc21c7..be8a64f09 100644 --- a/desloppify/app/commands/plan/triage/stages/strategize.py +++ b/desloppify/app/commands/plan/triage/stages/strategize.py @@ -188,6 +188,7 @@ def cmd_stage_strategize( ) -> None: """Record the STRATEGIZE stage: big-picture cross-cycle analysis.""" report: str | None = getattr(args, "report", None) + attestation: str | None = getattr(args, "attestation", None) resolved_services = services or default_triage_services() runtime = resolved_services.command_runtime(args) @@ -199,6 +200,7 @@ def cmd_stage_strategize( plan, services=resolved_services, state=state, + attestation=attestation, start_message=" Planning mode auto-started (7 stages queued).", deps=TriageLifecycleDeps( has_triage_in_queue=has_triage_in_queue, diff --git a/desloppify/app/commands/plan/triage/validation/completion_policy.py b/desloppify/app/commands/plan/triage/validation/completion_policy.py index 341dc3c30..3a154ae9b 100644 --- a/desloppify/app/commands/plan/triage/validation/completion_policy.py +++ b/desloppify/app/commands/plan/triage/validation/completion_policy.py @@ -4,15 +4,14 @@ from dataclasses import dataclass -from desloppify.engine.plan_triage import TRIAGE_CMD_ORGANIZE from desloppify.base.output.terminal import colorize -from desloppify.engine.plan_triage import extract_issue_citations +from desloppify.engine.plan_triage import TRIAGE_CMD_ORGANIZE, extract_issue_citations from ..display.dashboard import show_plan_summary from ..review_coverage import ( cluster_issue_ids, - open_review_ids_from_state, triage_coverage, + triage_open_review_ids_from_state, ) from ..stages.helpers import ( active_triage_issue_scope, @@ -142,7 +141,9 @@ def evaluate_completion_readiness( triage_scope = active_triage_issue_scope(plan, state) in_scope_open_ids = ( - open_review_ids_from_state(state) if state is not None and triage_scope is None else (triage_scope or set()) + triage_open_review_ids_from_state(plan, state) + if state is not None and triage_scope is None + else (triage_scope or set()) ) if state is not None and not in_scope_open_ids: return CompletionReadiness(ok=True) diff --git a/desloppify/app/commands/plan/triage/validation/enrich_quality.py b/desloppify/app/commands/plan/triage/validation/enrich_quality.py index 2ae706399..d4b00c890 100644 --- a/desloppify/app/commands/plan/triage/validation/enrich_quality.py +++ b/desloppify/app/commands/plan/triage/validation/enrich_quality.py @@ -6,6 +6,7 @@ from pathlib import Path from typing import Literal +from ..review_coverage import cluster_issue_ids from .enrich_checks import ( _steps_missing_issue_refs, _steps_referencing_skipped_issues, @@ -14,7 +15,6 @@ _steps_without_effort, _underspecified_steps, ) -from ..review_coverage import cluster_issue_ids Severity = Literal["failure", "warning"] @@ -85,10 +85,13 @@ def _active_cluster_names(plan: dict, triage_issue_ids: set[str]) -> set[str]: def _scoped_plan(plan: dict, triage_issue_ids: set[str] | None) -> dict: """Narrow plan to only clusters relevant to the current triage cycle. - Returns the original plan unchanged when no triage scoping is needed. + ``None`` means no triage scoping is needed. An empty set is an explicitly + frozen empty scope and therefore excludes historical clusters. """ - if not triage_issue_ids: + if triage_issue_ids is None: return plan + if not triage_issue_ids: + return {**plan, "clusters": {}} active = _active_cluster_names(plan, triage_issue_ids) return { **plan, diff --git a/desloppify/app/commands/plan/triage/validation/organize_policy.py b/desloppify/app/commands/plan/triage/validation/organize_policy.py index 844edf514..eb3451a10 100644 --- a/desloppify/app/commands/plan/triage/validation/organize_policy.py +++ b/desloppify/app/commands/plan/triage/validation/organize_policy.py @@ -7,6 +7,7 @@ from desloppify.base.output.terminal import colorize from desloppify.engine._plan.cluster_semantics import cluster_is_active +from desloppify.engine._plan.triage.protection import protected_review_issue_ids from ..review_coverage import cluster_issue_ids, manual_clusters_with_issues from ..stages.helpers import unclustered_review_issues, unenriched_clusters @@ -296,17 +297,28 @@ def validate_backlog_promotions_executed( return [] # Check which promoted clusters actually got promoted (are in queue_order - # or have execution_status set to active) + # or have execution_status set to active). clusters = plan.get("clusters", {}) + queue_order = plan.get("queue_order") + queued_ids = { + issue_id + for issue_id in queue_order + if isinstance(issue_id, str) + } if isinstance(queue_order, list) else set() + protected_ids = protected_review_issue_ids(plan) warnings: list[str] = [] for decision in promote_decisions: cluster = clusters.get(decision.cluster_name) if cluster is None: continue - # A promoted cluster should have been activated. + member_ids = set(cluster_issue_ids(cluster)) - protected_ids + queued_members = bool(member_ids) and member_ids.issubset(queued_ids) + # `plan promote` expands a cluster into its member IDs in queue_order + # without necessarily setting its execution status. An empty cluster + # must not pass the queue-membership check via vacuous truth. # Note: "in_progress" is a cluster *lifecycle* status (pending→in_progress→completed), # not an execution status. The old check accepted it here by mistake. - if not cluster_is_active(cluster): + if not (cluster_is_active(cluster) or queued_members): warnings.append( f"Reflect requested promoting {decision.cluster_name} " f"but it was not promoted during organize." diff --git a/desloppify/app/commands/resolve/apply.py b/desloppify/app/commands/resolve/apply.py index f3e1b3c5d..d35724284 100644 --- a/desloppify/app/commands/resolve/apply.py +++ b/desloppify/app/commands/resolve/apply.py @@ -9,6 +9,7 @@ from desloppify.app.commands.helpers.query import ( write_query_best_effort as _write_query_best_effort, ) +from desloppify.engine._plan.triage.protection import protected_review_issue_ids from .plan_load import ResolvePlanAccess, load_resolve_plan_access from .selection import ResolveQueryContext @@ -46,11 +47,18 @@ def _resolve_all_patterns( plan_access: ResolvePlanAccess | None = None, ) -> list[str]: all_resolved: list[str] = [] + resolved_plan_access = plan_access or load_resolve_plan_access() + plan = resolved_plan_access.usable_plan( + behavior="Protected review holds are unavailable until plan loading succeeds.", + ) + protected_ids = protected_review_issue_ids(plan) for pattern in args.patterns: # Check if pattern is a cluster name — expand to member IDs - cluster_ids = _try_expand_cluster(pattern, plan_access=plan_access) + cluster_ids = _try_expand_cluster(pattern, plan_access=resolved_plan_access) if cluster_ids: for fid in cluster_ids: + if fid in protected_ids: + continue resolved = state_mod.resolve_issues( state, fid, @@ -61,6 +69,8 @@ def _resolve_all_patterns( all_resolved.extend(resolved) continue + if pattern in protected_ids: + continue resolved = state_mod.resolve_issues( state, pattern, diff --git a/desloppify/app/commands/resolve/queue_guard.py b/desloppify/app/commands/resolve/queue_guard.py index 788fd526f..923f9210a 100644 --- a/desloppify/app/commands/resolve/queue_guard.py +++ b/desloppify/app/commands/resolve/queue_guard.py @@ -4,12 +4,13 @@ import logging +from desloppify.base.config import load_config from desloppify.base.output.terminal import colorize from desloppify.engine._work_queue.context import ( queue_context, ) -from desloppify.engine._work_queue.plan_order import collapse_clusters from desloppify.engine._work_queue.core import QueueBuildOptions +from desloppify.engine._work_queue.plan_order import collapse_clusters from desloppify.engine.planning.queue_policy import build_execution_queue from .plan_load import ResolvePlanAccess, load_resolve_plan_access @@ -128,12 +129,16 @@ def _check_queue_order_guard( if not queue_order: return False - ctx = queue_context(state, plan=plan) + # Resolve with the same configured threshold as ``desloppify next``. + # Otherwise a project target below the default 95 can make resolve see + # synthetic re-review work while next correctly presents its planned packet. + ctx = queue_context(state, plan=plan, config=load_config()) result = build_work_queue( state, options=QueueBuildOptions( count=None, include_subjective=True, + subjective_threshold=ctx.target_strict, context=ctx, ), ) @@ -155,7 +160,7 @@ def _check_queue_order_guard( return False resolved_issue_ids = {issue_id for issue_id in resolved_ids if issue_id in issues} - if resolved_issue_ids == resolved_ids: + if resolved_issue_ids: planned_front_ids = _front_planned_issue_ids( queue_order, issues=issues, diff --git a/desloppify/app/commands/review/batch/execution_results.py b/desloppify/app/commands/review/batch/execution_results.py index 8f8bd8693..0db531a8f 100644 --- a/desloppify/app/commands/review/batch/execution_results.py +++ b/desloppify/app/commands/review/batch/execution_results.py @@ -143,23 +143,34 @@ def merge_and_write_results( merged_assessment_dims + merged_issue_dims ) review_scope["imported_dimensions"] = merged_imported_dims - missing_after_import = print_import_dimension_coverage_notice( + missing_scored_dimensions = print_import_dimension_coverage_notice( assessed_dims=merged_assessment_dims, scored_dims=scored_dimensions, scan_path=scan_path, colorize_fn=colorize_fn, ) + selected_assessment_dimensions = normalize_dimension_list(packet_dimensions) + imported_assessment_dimensions = set(merged_assessment_dims) + missing_selected_dimensions = [ + dimension + for dimension in selected_assessment_dimensions + if dimension not in imported_assessment_dimensions + ] merged["assessment_coverage"] = { "scored_dimensions": scored_dimensions, - "selected_dimensions": packet_dimensions, + "selected_dimensions": selected_assessment_dimensions, "imported_dimensions": merged_assessment_dims, - "missing_dimensions": missing_after_import, + # The global gap remains useful review telemetry, but does not make a + # complete explicit subset review untrustworthy. + "missing_dimensions": missing_scored_dimensions, + "missing_selected_dimensions": missing_selected_dimensions, } merged_path = run_dir / "holistic_issues_merged.json" safe_write_text_fn(merged_path, json.dumps(merged, indent=2) + "\n") print(colorize_fn(f"\n Merged outputs: {merged_path}", "bold")) print_review_quality(quality, colorize_fn=colorize_fn) - return merged_path, missing_after_import + # The trusted-import gate must only enforce the selected packet contract. + return merged_path, missing_selected_dimensions def import_and_finalize( diff --git a/desloppify/app/commands/review/importing/cmd.py b/desloppify/app/commands/review/importing/cmd.py index 903a7e4c2..faf234983 100644 --- a/desloppify/app/commands/review/importing/cmd.py +++ b/desloppify/app/commands/review/importing/cmd.py @@ -6,10 +6,10 @@ from pathlib import Path from types import SimpleNamespace +from desloppify.app.commands.scan.artifacts import emit_scorecard_badge from desloppify.app.commands.scan.reporting import ( dimensions as reporting_dimensions_mod, ) -from desloppify.app.commands.scan.artifacts import emit_scorecard_badge from desloppify.base.exception_sets import CommandError, PacketValidationError from desloppify.base.output.terminal import colorize from desloppify.engine._plan.constants import WORKFLOW_IMPORT_SCORES_ID @@ -22,19 +22,21 @@ import_scores_meta_matches, pending_import_scores_meta, ) +from desloppify.engine._plan.triage.protection import protected_review_issue_ids from desloppify.engine._state.persistence import load_state, save_state from desloppify.engine._state.schema import utc_now from desloppify.intelligence import integrity as subjective_integrity_mod -from desloppify.intelligence.review.importing.holistic import import_holistic_issues from desloppify.intelligence.review.importing.contracts_models import ( AssessmentImportPolicyModel, ) +from desloppify.intelligence.review.importing.holistic import import_holistic_issues from desloppify.state_score_snapshot import score_snapshot from ..assessment_integrity import ( bind_scorecard_subjective_at_target, subjective_at_target_dimensions, ) +from ..state_payloads import append_assessment_import_audit from .flags import ( ReviewImportConfig, build_import_load_config, @@ -47,14 +49,13 @@ print_assessment_policy_notice, print_import_load_errors, ) -from ..state_payloads import append_assessment_import_audit -from .policy import assessment_policy_model_from_payload from .parse import ( ImportPayloadLoadError, load_import_issues_data, resolve_override_context, ) from .plan_sync import PlanImportSyncRequest, sync_plan_after_import +from .policy import assessment_policy_model_from_payload from .results import report_review_import_outcome _SCORECARD_SUBJECTIVE_AT_TARGET = bind_scorecard_subjective_at_target( @@ -267,6 +268,16 @@ def _has_refreshable_scorecard_context(state: dict) -> bool: return False +def _protected_review_issue_ids_for_import(state_file) -> set[str]: + """Return user-held review IDs that an import must never auto-resolve.""" + if state_file is None: + return set() + plan_path = plan_path_for_state(Path(state_file)) + if not has_living_plan(plan_path): + return set() + return protected_review_issue_ids(load_plan(plan_path)) + + def _refresh_scorecard_after_import( *, state: dict, @@ -326,7 +337,12 @@ def do_import( prev = score_snapshot(state) working_state = _build_working_state(state, state_file) - diff = import_holistic_issues(issues_data, working_state, lang.name) + diff = import_holistic_issues( + issues_data, + working_state, + lang.name, + preserve_open_issue_ids=_protected_review_issue_ids_for_import(state_file), + ) label = "Holistic review" provisional_count = _apply_assessment_policy( working_state=working_state, diff --git a/desloppify/app/commands/runner/codex_batch.py b/desloppify/app/commands/runner/codex_batch.py index 6fb754ab2..844238f3e 100644 --- a/desloppify/app/commands/runner/codex_batch.py +++ b/desloppify/app/commands/runner/codex_batch.py @@ -16,7 +16,9 @@ resolve_retry_config, run_batch_attempt, ) -from desloppify.app.commands.review.runner_process_impl.io import extract_payload_from_log +from desloppify.app.commands.review.runner_process_impl.io import ( + extract_payload_from_log, +) from desloppify.app.commands.review.runner_process_impl.types import ( CodexBatchRunnerDeps, FollowupScanDeps, @@ -193,6 +195,22 @@ def run_codex_batch( log_sections=log_sections, ) if success_code is not None: + if success_code == 1 and attempt < config.max_attempts: + delay = config.retry_backoff_seconds * (2 ** (attempt - 1)) + log_sections.append( + "Runner exited 0 but output validation failed; " + f"retrying in {delay:.1f}s (attempt {attempt + 1}/{config.max_attempts})." + ) + try: + if delay > 0: + deps.sleep_fn(delay) + except (OSError, RuntimeError, ValueError, TypeError) as exc: + log_sections.append( + f"Retry delay hook failed: {exc} — aborting remaining retries." + ) + deps.safe_write_text_fn(log_file, "\n\n".join(log_sections)) + return 1 + continue return success_code failure_code = handle_failed_attempt( result=result, diff --git a/desloppify/engine/_plan/auto_cluster.py b/desloppify/engine/_plan/auto_cluster.py index ea8b1685a..b342a4a49 100644 --- a/desloppify/engine/_plan/auto_cluster.py +++ b/desloppify/engine/_plan/auto_cluster.py @@ -3,16 +3,24 @@ from __future__ import annotations from desloppify.base.config import DEFAULT_TARGET_STRICT_SCORE -from desloppify.engine._plan.cluster_semantics import cluster_is_active -from desloppify.engine._plan.constants import AUTO_PREFIX from desloppify.engine._plan.auto_cluster_sync import ( prune_stale_clusters as _prune_stale_clusters, +) +from desloppify.engine._plan.auto_cluster_sync import ( sync_issue_clusters as _sync_issue_clusters, +) +from desloppify.engine._plan.auto_cluster_sync import ( sync_subjective_clusters as _sync_subjective_clusters, ) -from desloppify.engine._plan.schema import PlanModel, ensure_plan_defaults +from desloppify.engine._plan.cluster_semantics import cluster_is_active +from desloppify.engine._plan.constants import AUTO_PREFIX from desloppify.engine._plan.policy.subjective import SubjectiveVisibility +from desloppify.engine._plan.schema import PlanModel, ensure_plan_defaults from desloppify.engine._plan.sync.context import is_mid_cycle +from desloppify.engine._plan.triage.protection import ( + clear_protected_triage_artifacts, + protected_review_issue_ids, +) from desloppify.engine._state.schema import StateModel, utc_now # --------------------------------------------------------------------------- @@ -32,10 +40,16 @@ def _clear_missing_cluster_override( return 1 -def _canonical_cluster_membership(clusters: dict) -> dict[str, str]: +def _canonical_cluster_membership( + clusters: dict, + *, + protected_issue_ids: set[str], +) -> dict[str, str]: canonical: dict[str, str] = {} for name, cluster in clusters.items(): for issue_id in cluster.get("issue_ids", []): + if issue_id in protected_issue_ids: + continue if issue_id not in canonical or not cluster.get("auto"): canonical[issue_id] = name return canonical @@ -99,7 +113,10 @@ def _repair_ghost_cluster_refs(plan: PlanModel, now: str) -> int: for override in overrides.values(): repaired += _clear_missing_cluster_override(override, clusters, now) - canonical = _canonical_cluster_membership(clusters) + canonical = _canonical_cluster_membership( + clusters, + protected_issue_ids=protected_review_issue_ids(plan), + ) for issue_id, cluster_name in canonical.items(): repaired += _sync_override_to_canonical_cluster(issue_id, cluster_name, overrides, now) @@ -162,13 +179,19 @@ def _evictable_auto_cluster_issue_ids(plan: PlanModel) -> set[str]: active_ids: set[str] = set() inactive_ids: set[str] = set() + protected_ids = protected_review_issue_ids(plan) for cluster in plan.get("clusters", {}).values(): if not isinstance(cluster, dict): continue ids = { issue_id for issue_id in cluster.get("issue_ids", []) - if isinstance(issue_id, str) and issue_id and not is_synthetic_id(issue_id) + if ( + isinstance(issue_id, str) + and issue_id + and issue_id not in protected_ids + and not is_synthetic_id(issue_id) + ) } if not cluster.get("auto"): active_ids |= ids @@ -184,6 +207,7 @@ def _sync_active_auto_cluster_queue_membership(plan: PlanModel) -> int: order: list[str] = plan.get("queue_order", []) skipped = set(plan.get("skipped", {}).keys()) existing = set(order) + protected_ids = protected_review_issue_ids(plan) changes = 0 # Evict queue_order entries from non-active auto-clusters. @@ -205,6 +229,7 @@ def _sync_active_auto_cluster_queue_membership(plan: PlanModel) -> int: or not issue_id or issue_id in skipped or issue_id in existing + or issue_id in protected_ids ): continue order.append(issue_id) @@ -225,6 +250,7 @@ def auto_cluster_issues( Returns count of changes made (clusters created, updated, or deleted). """ ensure_plan_defaults(plan) + clear_protected_triage_artifacts(plan, state) if is_mid_cycle(plan): return 0 diff --git a/desloppify/engine/_plan/auto_cluster_sync_issue.py b/desloppify/engine/_plan/auto_cluster_sync_issue.py index a794b1766..8ce46b527 100644 --- a/desloppify/engine/_plan/auto_cluster_sync_issue.py +++ b/desloppify/engine/_plan/auto_cluster_sync_issue.py @@ -7,9 +7,9 @@ from desloppify.base.registry import DETECTORS from desloppify.engine._plan.cluster_semantics import ( + EXECUTION_POLICY_EPHEMERAL_AUTOPROMOTE, EXECUTION_STATUS_ACTIVE, EXECUTION_STATUS_REVIEW, - EXECUTION_POLICY_EPHEMERAL_AUTOPROMOTE, infer_cluster_execution_policy, normalize_cluster_semantics, ) @@ -25,6 +25,7 @@ from desloppify.engine._plan.cluster_strategy import ( grouping_key as _grouping_key, ) +from desloppify.engine._plan.triage.protection import protected_review_issue_ids _MIN_CLUSTER_SIZE = 2 @@ -65,8 +66,10 @@ def _group_clusterable_issues( issues: dict, *, manual_member_ids: set[str], + protected_issue_ids: set[str] | None = None, ) -> tuple[dict[str, list[str]], dict[str, dict]]: """Group open, non-suppressed, non-manual issues by detector/subtype key.""" + protected_ids = protected_issue_ids or set() groups: dict[str, list[str]] = defaultdict(list) issue_data: dict[str, dict] = {} for fid, issue in issues.items(): @@ -76,6 +79,8 @@ def _group_clusterable_issues( continue if fid in manual_member_ids: continue + if fid in protected_ids: + continue detector = issue.get("detector", "") meta = DETECTORS.get(detector) @@ -214,7 +219,9 @@ def sync_issue_clusters( changes = 0 groups, issue_data = _group_clusterable_issues( - issues, manual_member_ids=_manual_member_ids(clusters) + issues, + manual_member_ids=_manual_member_ids(clusters), + protected_issue_ids=protected_review_issue_ids(plan), ) for key, member_ids in groups.items(): diff --git a/desloppify/engine/_plan/operations/cluster.py b/desloppify/engine/_plan/operations/cluster.py index 2dd22e3e0..0c8432b27 100644 --- a/desloppify/engine/_plan/operations/cluster.py +++ b/desloppify/engine/_plan/operations/cluster.py @@ -4,12 +4,13 @@ from desloppify.engine._plan.cluster_semantics import ( ACTION_TYPE_MANUAL_FIX, - EXECUTION_STATUS_ACTIVE, EXECUTION_POLICY_PLANNED_ONLY, + EXECUTION_STATUS_ACTIVE, ) from desloppify.engine._plan.operations.lifecycle import clear_focus_if_cluster_empty from desloppify.engine._plan.operations.queue import move_items from desloppify.engine._plan.schema import Cluster, PlanModel, ensure_plan_defaults +from desloppify.engine._plan.triage.protection import protected_review_issue_ids from desloppify.engine._state.schema import utc_now @@ -27,6 +28,8 @@ def _upsert_cluster_override( cluster_name: str, timestamp: str, ) -> None: + if issue_id in protected_review_issue_ids(plan): + return overrides = plan["overrides"] if issue_id not in overrides: overrides[issue_id] = {"issue_id": issue_id, "created_at": timestamp} @@ -108,6 +111,7 @@ def add_to_cluster( member_ids: list[str] = cluster["issue_ids"] queue_order: list[str] = plan.get("queue_order", []) is_manual = not cluster.get("auto") + protected_ids = protected_review_issue_ids(plan) count = 0 now = utc_now() for fid in issue_ids: @@ -115,7 +119,7 @@ def add_to_cluster( member_ids.append(fid) count += 1 # Ensure manual cluster members are in the queue - if is_manual and fid not in queue_order: + if is_manual and fid not in protected_ids and fid not in queue_order: queue_order.append(fid) _upsert_cluster_override( plan, diff --git a/desloppify/engine/_plan/operations/queue.py b/desloppify/engine/_plan/operations/queue.py index a45ab0ea8..1d04378c2 100644 --- a/desloppify/engine/_plan/operations/queue.py +++ b/desloppify/engine/_plan/operations/queue.py @@ -4,6 +4,7 @@ from desloppify.engine._plan.promoted_ids import add_promoted_ids from desloppify.engine._plan.schema import PlanModel, SkipEntry, ensure_plan_defaults +from desloppify.engine._plan.triage.protection import protected_review_issue_ids def _remove_id_from_lists(plan: PlanModel, issue_id: str) -> None: @@ -104,7 +105,10 @@ def move_items( # Triage stage IDs are workflow-managed and cannot be manually reordered. from desloppify.engine._plan.constants import TRIAGE_IDS - issue_ids = [fid for fid in issue_ids if fid not in TRIAGE_IDS] + protected_ids = protected_review_issue_ids(plan) + issue_ids = [ + fid for fid in issue_ids if fid not in TRIAGE_IDS and fid not in protected_ids + ] if not issue_ids: return 0 diff --git a/desloppify/engine/_plan/operations/skip.py b/desloppify/engine/_plan/operations/skip.py index 581f6baf0..8a89b722f 100644 --- a/desloppify/engine/_plan/operations/skip.py +++ b/desloppify/engine/_plan/operations/skip.py @@ -9,6 +9,10 @@ from desloppify.engine._plan.promoted_ids import prune_promoted_ids from desloppify.engine._plan.schema import PlanModel, SkipEntry, ensure_plan_defaults from desloppify.engine._plan.skip_policy import skip_kind_needs_state_reopen +from desloppify.engine._plan.triage.protection import ( + clear_protected_triage_artifacts, + protected_review_issue_ids, +) from desloppify.engine._state.schema import utc_now @@ -75,6 +79,9 @@ def skip_items( legacy_kwargs=legacy_kwargs, ) ensure_plan_defaults(plan) + clear_protected_triage_artifacts(plan) + protected_ids = protected_review_issue_ids(plan) + issue_ids = [issue_id for issue_id in issue_ids if issue_id not in protected_ids] now = utc_now() count = 0 skipped: dict[str, SkipEntry] = plan["skipped"] @@ -123,11 +130,16 @@ def unskip_items( ``include_protected=True``). """ ensure_plan_defaults(plan) + clear_protected_triage_artifacts(plan) + protected_ids = protected_review_issue_ids(plan) count = 0 need_reopen: list[str] = [] protected_kept: list[str] = [] skipped: dict[str, SkipEntry] = plan["skipped"] for fid in issue_ids: + if fid in protected_ids: + protected_kept.append(fid) + continue entry = skipped.get(fid) if entry is None: continue @@ -151,9 +163,13 @@ def resurface_stale_skips( Returns list of resurfaced issue IDs. """ ensure_plan_defaults(plan) + clear_protected_triage_artifacts(plan) + protected_ids = protected_review_issue_ids(plan) skipped: dict[str, SkipEntry] = plan["skipped"] resurfaced: list[str] = [] for fid in list(skipped): + if fid in protected_ids: + continue entry = skipped[fid] if entry.get("kind") != "temporary": continue @@ -176,6 +192,7 @@ def backlog_items(plan: PlanModel, issue_ids: list[str]) -> list[str]: Returns the IDs that were actually removed. """ ensure_plan_defaults(plan) + clear_protected_triage_artifacts(plan) removed: list[str] = [] for fid in issue_ids: if fid in plan["skipped"]: diff --git a/desloppify/engine/_plan/policy/stale.py b/desloppify/engine/_plan/policy/stale.py index a6dd32b74..ca2a75304 100644 --- a/desloppify/engine/_plan/policy/stale.py +++ b/desloppify/engine/_plan/policy/stale.py @@ -5,10 +5,12 @@ import hashlib from desloppify.base.config import DEFAULT_TARGET_STRICT_SCORE +from desloppify.engine._plan.triage.protection import protected_review_issue_ids from desloppify.engine._state.schema import StateModel from desloppify.engine._work_queue.helpers import slugify from desloppify.engine.planning.scorecard_projection import all_subjective_entries + def open_review_ids(state: StateModel) -> set[str]: """Return IDs of open review/concerns issues from state. @@ -25,6 +27,15 @@ def open_review_ids(state: StateModel) -> set[str]: } +def triage_open_review_ids(plan: dict, state: StateModel) -> set[str]: + """Return open review IDs that are in automated triage scope. + + Explicitly protected IDs remain open in state. They are excluded here so + triage automation cannot re-inject or disposition a user-owned hold. + """ + return open_review_ids(state) - protected_review_issue_ids(plan) + + def open_mechanical_count(state: StateModel) -> int: """Return the count of open mechanical defects from state.""" from desloppify.engine._state.issue_semantics import is_objective_finding @@ -158,13 +169,21 @@ def review_issue_snapshot_hash(state: StateModel) -> str: return hashlib.sha256("|".join(review_ids).encode()).hexdigest()[:16] +def triage_review_issue_snapshot_hash(plan: dict, state: StateModel) -> str: + """Hash only the review IDs that automated triage is allowed to process.""" + review_ids = sorted(triage_open_review_ids(plan, state)) + if not review_ids: + return "" + return hashlib.sha256("|".join(review_ids).encode()).hexdigest()[:16] + + def compute_new_issue_ids(plan: dict, state: StateModel) -> set[str]: """Return open review/concerns IDs that appeared since the last triage.""" meta = plan.get("epic_triage_meta", {}) triaged = set(meta.get("triaged_ids", [])) active = set(meta.get("active_triage_issue_ids", [])) known = triaged | active - return open_review_ids(state) - known if known else set() + return triage_open_review_ids(plan, state) - known if known else set() def is_triage_stale( @@ -193,7 +212,7 @@ def is_triage_stale( known = triaged_ids | active_ids # Any new review issue → stale - if open_review_ids(state) - known: + if triage_open_review_ids(plan, state) - known: return True # Check mechanical growth threshold @@ -219,4 +238,6 @@ def is_triage_stale( "open_mechanical_count", "open_review_ids", "review_issue_snapshot_hash", + "triage_open_review_ids", + "triage_review_issue_snapshot_hash", ] diff --git a/desloppify/engine/_plan/promoted_ids.py b/desloppify/engine/_plan/promoted_ids.py index d99e4589b..309ab8474 100644 --- a/desloppify/engine/_plan/promoted_ids.py +++ b/desloppify/engine/_plan/promoted_ids.py @@ -4,6 +4,8 @@ from typing import Any +from desloppify.engine._plan.triage.protection import protected_review_issue_ids + def _promoted_ids(plan: dict[str, Any]) -> list[str]: promoted = plan.get("promoted_ids") @@ -18,8 +20,9 @@ def add_promoted_ids(plan: dict[str, Any], issue_ids: list[str]) -> None: """Append issue IDs to promoted_ids preserving existing order.""" promoted = _promoted_ids(plan) existing = set(promoted) + protected_ids = protected_review_issue_ids(plan) for issue_id in issue_ids: - if issue_id in existing: + if issue_id in existing or issue_id in protected_ids: continue promoted.append(issue_id) existing.add(issue_id) diff --git a/desloppify/engine/_plan/scan_issue_reconcile.py b/desloppify/engine/_plan/scan_issue_reconcile.py index f791c3e72..099e9e954 100644 --- a/desloppify/engine/_plan/scan_issue_reconcile.py +++ b/desloppify/engine/_plan/scan_issue_reconcile.py @@ -6,8 +6,11 @@ from datetime import UTC, datetime, timedelta from desloppify.engine._plan.annotations import get_issue_note +from desloppify.engine._plan.cluster_semantics import ( + EXECUTION_STATUS_DONE, + cluster_is_active, +) from desloppify.engine._plan.constants import SYNTHETIC_PREFIXES -from desloppify.engine._plan.cluster_semantics import EXECUTION_STATUS_DONE, cluster_is_active from desloppify.engine._plan.operations.lifecycle import clear_focus_if_cluster_empty from desloppify.engine._plan.operations.meta import append_log_entry from desloppify.engine._plan.operations.skip import resurface_stale_skips @@ -19,6 +22,7 @@ ensure_plan_defaults, ) from desloppify.engine._plan.skip_policy import skip_kind_state_status +from desloppify.engine._plan.triage.protection import clear_protected_triage_artifacts from desloppify.engine._state.schema import StateModel, ensure_state_defaults, utc_now SUPERSEDED_TTL_DAYS = 90 @@ -378,6 +382,9 @@ def reconcile_plan_after_scan( """ ensure_plan_defaults(plan) ensure_state_defaults(state) + # A protected ID is a hold, not a skip. Sanitize before skipped entries + # are synchronized into non-open state on this scan boundary. + clear_protected_triage_artifacts(plan, state) result = ReconcileResult() now = utc_now() now_dt = datetime.now(UTC) diff --git a/desloppify/engine/_plan/schema/__init__.py b/desloppify/engine/_plan/schema/__init__.py index 146afaa92..74025dd87 100644 --- a/desloppify/engine/_plan/schema/__init__.py +++ b/desloppify/engine/_plan/schema/__init__.py @@ -208,6 +208,7 @@ class EpicTriageMeta(TypedDict, total=False): triaged_ids: list[str] active_triage_issue_ids: list[str] + protected_review_issue_ids: list[str] dismissed_ids: list[str] undispositioned_issue_ids: list[str] undispositioned_issue_count: int @@ -335,27 +336,20 @@ def executable_objective_ids( ) -> set[str]: """Return objective IDs eligible for execution. - Before the plan tracks any queue work at all, all objective IDs are - implicitly executable. Once *any* queue items exist — including synthetic - review/workflow/triage items — execution becomes queue-driven and only - objective IDs explicitly present in ``plan["queue_order"]`` remain - eligible for ``next``. + Before the plan tracks substantive objective queue work, all objective IDs + are implicitly executable. Synthetic review, workflow, triage, and + strategy IDs coordinate lifecycle work but must not hide objective backlog. + Once a substantive queue item exists, only objective IDs explicitly + present in ``plan["queue_order"]`` remain eligible for ``next``. """ if not isinstance(plan, dict): return set(all_objective_ids) skipped_ids = set(plan.get("skipped", {}).keys()) - queued_ids = { - issue_id - for issue_id in plan.get("queue_order", []) - if isinstance(issue_id, str) - and issue_id - and issue_id not in skipped_ids - } live_queue_ids = live_planned_queue_ids(plan) queued_objective_ids = all_objective_ids & live_queue_ids if queued_objective_ids: return queued_objective_ids - if not queued_ids: + if not live_queue_ids: return set(all_objective_ids) - skipped_ids return set() diff --git a/desloppify/engine/_plan/sync/pipeline.py b/desloppify/engine/_plan/sync/pipeline.py index 9eaf59f1c..9406f435f 100644 --- a/desloppify/engine/_plan/sync/pipeline.py +++ b/desloppify/engine/_plan/sync/pipeline.py @@ -4,22 +4,20 @@ from dataclasses import dataclass -from desloppify.state_scoring import score_snapshot from desloppify.engine._plan.auto_cluster import auto_cluster_issues from desloppify.engine._plan.constants import ( PRE_REVIEW_WORKFLOW_IDS, WORKFLOW_COMMUNICATE_SCORE_ID, WORKFLOW_CREATE_PLAN_ID, WORKFLOW_DEFERRED_DISPOSITION_ID, - WORKFLOW_IMPORT_SCORES_ID, WORKFLOW_RUN_SCAN_ID, WORKFLOW_SCORE_CHECKPOINT_ID, QueueSyncResult, is_synthetic_id, ) from desloppify.engine._plan.operations.meta import append_log_entry +from desloppify.engine._plan.policy.stale import triage_open_review_ids from desloppify.engine._plan.policy.subjective import compute_subjective_visibility -from desloppify.engine._plan.policy.stale import open_review_ids from desloppify.engine._plan.refresh_lifecycle import ( _set_lifecycle_phase, derive_display_phase, @@ -28,12 +26,14 @@ from desloppify.engine._plan.sync.dimensions import sync_subjective_dimensions from desloppify.engine._plan.sync.phase_cleanup import prune_synthetic_for_phase from desloppify.engine._plan.sync.triage import sync_triage_needed -from desloppify.engine._plan.triage.snapshot import build_triage_snapshot from desloppify.engine._plan.sync.workflow import ( ScoreSnapshot, sync_communicate_score_needed, sync_create_plan_needed, ) +from desloppify.engine._plan.triage.protection import clear_protected_triage_artifacts +from desloppify.engine._plan.triage.snapshot import build_triage_snapshot +from desloppify.state_scoring import score_snapshot _SCAN_PHASE_WORKFLOW_IDS = { WORKFLOW_DEFERRED_DISPOSITION_ID, @@ -156,7 +156,7 @@ def _resolve_reconcile_display_phase( if item not in (plan.get("skipped") or {}) ) has_review_postflight = triage_gated_review or ( - not has_real_work and bool(open_review_ids(state)) + not has_real_work and bool(triage_open_review_ids(plan, state)) ) return derive_display_phase( @@ -227,6 +227,10 @@ def reconcile_plan( """Run the shared boundary reconciliation pipeline.""" result = ReconcileResult() + # Apply user-owned holds before queue, lifecycle, or auto-cluster + # reconciliation can turn them back into executable work. + clear_protected_triage_artifacts(plan, state) + # Migration cleanup: prune stale subjective items from queue_order # left by the old mid-cycle re-injection bug. With boundary-only sync # they won't be re-added, so they just block phase resolution. diff --git a/desloppify/engine/_plan/sync/review_import.py b/desloppify/engine/_plan/sync/review_import.py index 47835f85a..df680062a 100644 --- a/desloppify/engine/_plan/sync/review_import.py +++ b/desloppify/engine/_plan/sync/review_import.py @@ -11,6 +11,10 @@ compute_open_issue_ids, sync_triage_needed, ) +from desloppify.engine._plan.triage.protection import ( + clear_protected_triage_artifacts, + protected_review_issue_ids, +) from desloppify.engine._state.issue_semantics import is_triage_finding from desloppify.engine._state.schema import StateModel @@ -49,7 +53,8 @@ def _review_issue_ids_for_import_sync( """ if _has_triage_baseline(plan): return compute_new_issue_ids(plan, state) - return set(open_review_ids) if open_review_ids is not None else compute_open_issue_ids(state) + current_ids = set(open_review_ids) if open_review_ids is not None else compute_open_issue_ids(state) + return current_ids - protected_review_issue_ids(plan) def _is_review_queue_id(issue_id: str, state: StateModel) -> bool: @@ -162,6 +167,7 @@ def sync_plan_after_review_import( changes are required. """ ensure_plan_defaults(plan) + clear_protected_triage_artifacts(plan, state) open_review_ids = compute_open_issue_ids(state) stale_pruned_from_queue = _prune_stale_review_ids_from_plan( plan, diff --git a/desloppify/engine/_plan/sync/triage.py b/desloppify/engine/_plan/sync/triage.py index 817dcc5ed..9eb85b2e6 100644 --- a/desloppify/engine/_plan/sync/triage.py +++ b/desloppify/engine/_plan/sync/triage.py @@ -2,7 +2,6 @@ from __future__ import annotations -from desloppify.engine._plan.policy import stale as stale_policy_mod from desloppify.engine._plan.constants import ( TRIAGE_IDS, TRIAGE_STAGE_IDS, @@ -11,12 +10,14 @@ normalize_queue_workflow_and_triage_prefix, recorded_unconfirmed_triage_stage_names, ) -from desloppify.engine._plan.schema import PlanModel, ensure_plan_defaults +from desloppify.engine._plan.policy import stale as stale_policy_mod from desloppify.engine._plan.policy.subjective import SubjectiveVisibility +from desloppify.engine._plan.schema import PlanModel, ensure_plan_defaults from desloppify.engine._plan.triage.lifecycle import ( ensure_active_triage_issue_ids, inject_triage_stages, ) +from desloppify.engine._plan.triage.protection import clear_protected_triage_artifacts from desloppify.engine._state.schema import StateModel from .defer_policy import ( @@ -39,6 +40,7 @@ def _new_review_ids_since_triage( + plan: PlanModel, state: StateModel, meta: dict, ) -> set[str]: @@ -46,7 +48,8 @@ def _new_review_ids_since_triage( triaged_ids = set(meta.get("triaged_ids", [])) active_ids = set(meta.get("active_triage_issue_ids", [])) known_ids = triaged_ids | active_ids - return stale_policy_mod.open_review_ids(state) - known_ids if known_ids else set() + review_ids = stale_policy_mod.triage_open_review_ids(plan, state) + return review_ids - known_ids if known_ids else set() def _baseline_triage_issue_ids(meta: dict) -> set[str]: @@ -157,14 +160,14 @@ def _prune_stale_present_stages( result = QueueSyncResult() if not last_hash or confirmed or recorded_unconfirmed: return result - if not _baseline_triage_issue_ids(meta) and stale_policy_mod.open_review_ids(state): + if not _baseline_triage_issue_ids(meta) and stale_policy_mod.triage_open_review_ids(plan, state): return result - new_since_triage = _new_review_ids_since_triage(state, meta) + new_since_triage = _new_review_ids_since_triage(plan, state, meta) if new_since_triage: return result _prune_all_triage_stages(order) _clear_triage_defer_tracking(meta) - current_hash = stale_policy_mod.review_issue_snapshot_hash(state) + current_hash = stale_policy_mod.triage_review_issue_snapshot_hash(plan, state) if current_hash: meta["issue_snapshot_hash"] = current_hash plan["epic_triage_meta"] = meta @@ -185,10 +188,10 @@ def _backfill_partial_triage_snapshot( ) if not has_completed_stage or meta.get("triaged_ids") or last_hash: return - current_review = sorted(stale_policy_mod.open_review_ids(state)) + current_review = sorted(stale_policy_mod.triage_open_review_ids(plan, state)) if current_review: meta["triaged_ids"] = current_review - meta["issue_snapshot_hash"] = stale_policy_mod.review_issue_snapshot_hash(state) + meta["issue_snapshot_hash"] = stale_policy_mod.triage_review_issue_snapshot_hash(plan, state) plan["epic_triage_meta"] = meta @@ -265,9 +268,9 @@ def _sync_hash_change( policy: SubjectiveVisibility | None, current_hash: str, ) -> QueueSyncResult: - new_since_triage = _new_review_ids_since_triage(state, meta) + new_since_triage = _new_review_ids_since_triage(plan, state, meta) if not new_since_triage and not _baseline_triage_issue_ids(meta): - new_since_triage = stale_policy_mod.open_review_ids(state) + new_since_triage = stale_policy_mod.triage_open_review_ids(plan, state) if new_since_triage: return _defer_or_inject_triage( plan=plan, @@ -345,6 +348,7 @@ def sync_triage_needed( is needed since the user is working through the plan. """ ensure_plan_defaults(plan) + clear_protected_triage_artifacts(plan, state) result = QueueSyncResult() order: list[str] = plan["queue_order"] meta = plan.get("epic_triage_meta", {}) @@ -355,7 +359,7 @@ def sync_triage_needed( recorded_unconfirmed = recorded_unconfirmed_triage_stage_names(meta) if _consume_workflow_plan_resolution_marker(plan, meta): - if stale_policy_mod.open_review_ids(state): + if stale_policy_mod.triage_open_review_ids(plan, state): ensure_active_triage_issue_ids(plan, state) _mark_triage_ready(plan, meta) injected = inject_triage_stages(plan) @@ -365,7 +369,7 @@ def sync_triage_needed( # Check if any triage stage is already in queue already_present = any(sid in order for sid in TRIAGE_IDS) - current_hash = stale_policy_mod.review_issue_snapshot_hash(state) + current_hash = stale_policy_mod.triage_review_issue_snapshot_hash(plan, state) last_hash = meta.get("issue_snapshot_hash", "") if already_present: diff --git a/desloppify/engine/_plan/triage/apply.py b/desloppify/engine/_plan/triage/apply.py index d7e6abdc8..ec4f99695 100644 --- a/desloppify/engine/_plan/triage/apply.py +++ b/desloppify/engine/_plan/triage/apply.py @@ -5,7 +5,7 @@ from dataclasses import dataclass from desloppify.engine._plan.cluster_semantics import EXECUTION_STATUS_ACTIVE -from desloppify.engine._plan.policy.stale import review_issue_snapshot_hash +from desloppify.engine._plan.policy.stale import triage_review_issue_snapshot_hash from desloppify.engine._plan.schema import ( EPIC_PREFIX, Cluster, @@ -13,6 +13,10 @@ ensure_plan_defaults, ) from desloppify.engine._plan.skip_policy import skip_kind_state_status +from desloppify.engine._plan.triage.protection import ( + clear_protected_triage_artifacts, + protected_review_issue_ids, +) from desloppify.engine._state.issue_semantics import is_triage_finding from desloppify.engine._state.schema import StateModel, ensure_state_defaults, utc_now @@ -120,10 +124,36 @@ def _upsert_triage_clusters( triage: TriageResult, now: str, version: int, + protected_issue_ids: set[str], ) -> tuple[int, int]: created = 0 updated = 0 for epic_data in sorted(triage.clusters, key=_epic_sort_key): + filtered_issue_ids = [ + issue_id + for issue_id in epic_data.get("issue_ids", []) + if issue_id not in protected_issue_ids + ] + filtered_dismissed = [ + issue_id + for issue_id in epic_data.get("dismissed", []) + if issue_id not in protected_issue_ids + ] + if ( + not filtered_issue_ids + and not filtered_dismissed + and (epic_data.get("issue_ids") or epic_data.get("dismissed")) + ): + continue + if ( + filtered_issue_ids != epic_data.get("issue_ids", []) + or filtered_dismissed != epic_data.get("dismissed", []) + ): + epic_data = { + **epic_data, + "issue_ids": filtered_issue_ids, + "dismissed": filtered_dismissed, + } raw_name = epic_data["name"] epic_name = _normalized_epic_name(raw_name) existing = clusters.get(epic_name) @@ -146,13 +176,14 @@ def _reorder_queue_by_dependency( order: list[str], triage: TriageResult, dismissed_ids: list[str], + protected_issue_ids: set[str], ) -> None: epic_issue_ids: set[str] = set() epic_ordered_ids: list[str] = [] dismissed_set = set(dismissed_ids) for epic_data in sorted(triage.clusters, key=_epic_sort_key): for fid in epic_data["issue_ids"]: - if fid in epic_issue_ids or fid in dismissed_set: + if fid in protected_issue_ids or fid in epic_issue_ids or fid in dismissed_set: continue epic_issue_ids.add(fid) epic_ordered_ids.append(fid) @@ -173,15 +204,23 @@ def _set_triage_meta( dismissed_ids: list[str], trigger: str, ) -> None: - current_hash = review_issue_snapshot_hash(state) + existing_meta = plan.get("epic_triage_meta", {}) + protected_config = ( + existing_meta.get("protected_review_issue_ids") + if isinstance(existing_meta, dict) + else None + ) + current_hash = triage_review_issue_snapshot_hash(plan, state) + protected_ids = protected_review_issue_ids(plan) open_review_ids = sorted( - fid - for fid, issue in (state.get("work_items") or state.get("issues", {})).items() + issue_id + for issue_id, issue in (state.get("work_items") or state.get("issues", {})).items() if issue.get("status") == "open" and is_triage_finding(issue) + and issue_id not in protected_ids ) - plan["epic_triage_meta"] = { + meta = { "triaged_ids": open_review_ids, "last_run": now, "version": version, @@ -190,6 +229,9 @@ def _set_triage_meta( "strategy_summary": triage.strategy_summary, "trigger": trigger, } + if isinstance(protected_config, list): + meta["protected_review_issue_ids"] = protected_config + plan["epic_triage_meta"] = meta def _apply_auto_cluster_decisions( @@ -200,6 +242,7 @@ def _apply_auto_cluster_decisions( now: str, version: int, result: TriageMutationResult, + protected_issue_ids: set[str], ) -> None: """Process auto_cluster_decisions from the triage result. @@ -222,7 +265,11 @@ def _apply_auto_cluster_decisions( existing_in_order = set(order) new_ids = [ fid for fid in issue_ids - if isinstance(fid, str) and fid not in existing_in_order + if ( + isinstance(fid, str) + and fid not in existing_in_order + and fid not in protected_issue_ids + ) ] # Determine insertion position based on priority hint priority = (decision.priority or "").lower().strip() @@ -301,6 +348,7 @@ def apply_triage_to_plan( 4. Updates epic_triage_meta with snapshot hash """ ensure_plan_defaults(plan) + clear_protected_triage_artifacts(plan, state) ensure_state_defaults(state) now = utc_now() result = TriageMutationResult() @@ -310,6 +358,7 @@ def apply_triage_to_plan( skipped: dict = plan["skipped"] order: list[str] = plan["queue_order"] meta = plan.get("epic_triage_meta", {}) + protected_ids = protected_review_issue_ids(plan) version = int(meta.get("version", 0)) + 1 result.triage_version = version @@ -318,6 +367,7 @@ def apply_triage_to_plan( triage=triage, now=now, version=version, + protected_issue_ids=protected_ids, ) result.epics_created += created result.epics_updated += updated @@ -329,6 +379,7 @@ def apply_triage_to_plan( now=now, version=version, scan_count=int(state.get("scan_count", 0)), + protected_issue_ids=protected_ids, ) result.issues_dismissed += dismiss_count @@ -345,6 +396,7 @@ def apply_triage_to_plan( order=order, triage=triage, dismissed_ids=dismissed_ids, + protected_issue_ids=protected_ids, ) # Process auto-cluster decisions (backward-compatible: no-op if empty) @@ -356,6 +408,7 @@ def apply_triage_to_plan( now=now, version=version, result=result, + protected_issue_ids=protected_ids, ) _set_triage_meta( diff --git a/desloppify/engine/_plan/triage/dismiss.py b/desloppify/engine/_plan/triage/dismiss.py index 2803158d3..4118e83ea 100644 --- a/desloppify/engine/_plan/triage/dismiss.py +++ b/desloppify/engine/_plan/triage/dismiss.py @@ -33,12 +33,16 @@ def dismiss_triage_issues( now: str, version: int, scan_count: int, + protected_issue_ids: set[str] | None = None, ) -> tuple[list[str], int]: """Move triage-dismissed issues out of queue and into skipped metadata.""" + protected_ids = protected_issue_ids or set() dismissed_ids: list[str] = [] dismiss_count = 0 for dismissed in triage.dismissed_issues: issue_id = dismissed.issue_id + if issue_id in protected_ids: + continue dismissed_ids.append(issue_id) if issue_id in order: order.remove(issue_id) @@ -53,7 +57,7 @@ def dismiss_triage_issues( for epic_data in triage.clusters: for issue_id in epic_data.get("dismissed", []): - if issue_id in dismissed_ids or issue_id not in order: + if issue_id in protected_ids or issue_id in dismissed_ids or issue_id not in order: continue order.remove(issue_id) dismissed_ids.append(issue_id) diff --git a/desloppify/engine/_plan/triage/lifecycle.py b/desloppify/engine/_plan/triage/lifecycle.py index b690c63d9..983b28af0 100644 --- a/desloppify/engine/_plan/triage/lifecycle.py +++ b/desloppify/engine/_plan/triage/lifecycle.py @@ -13,6 +13,7 @@ TRIAGE_STAGE_IDS, normalize_queue_workflow_and_triage_prefix, ) +from desloppify.engine._plan.triage.protection import clear_protected_triage_artifacts from desloppify.engine._plan.triage.snapshot import coverage_open_ids from desloppify.engine._state.schema import StateModel from desloppify.engine.plan_state import EpicTriageMeta, PlanModel, SkipEntry @@ -81,6 +82,7 @@ def inject_triage_stages(plan: PlanModel) -> list[str]: def ensure_active_triage_issue_ids(plan: PlanModel, state: StateModel) -> list[str]: """Freeze the current triage issue set for validation across stage reruns.""" + clear_protected_triage_artifacts(plan, state) meta = ensure_triage_meta(plan) active_ids = sorted(coverage_open_ids(plan, state)) meta[_ACTIVE_TRIAGE_ISSUE_IDS_KEY] = active_ids diff --git a/desloppify/engine/_plan/triage/prompt.py b/desloppify/engine/_plan/triage/prompt.py index b016ebe2e..5abeaf481 100644 --- a/desloppify/engine/_plan/triage/prompt.py +++ b/desloppify/engine/_plan/triage/prompt.py @@ -7,12 +7,13 @@ from desloppify.engine._plan.cluster_semantics import cluster_autofix_hint from desloppify.engine._plan.schema import ( - Cluster, EPIC_PREFIX, + Cluster, PlanModel, ensure_plan_defaults, triage_clusters, ) +from desloppify.engine._plan.triage.protection import protected_review_issue_ids from desloppify.engine._plan.triage.snapshot import build_triage_snapshot from desloppify.engine._state.issue_semantics import is_triage_finding from desloppify.engine._state.schema import StateModel @@ -212,20 +213,47 @@ def collect_triage_input(plan: PlanModel, state: StateModel) -> TriageInput: ensure_plan_defaults(plan) issues = (state.get("work_items") or state.get("issues", {})) meta = plan.get("epic_triage_meta", {}) - epics = triage_clusters(plan) + protected_ids = protected_review_issue_ids(plan) + epics = { + name: { + **cluster, + "issue_ids": [ + issue_id + for issue_id in cluster.get("issue_ids", []) + if isinstance(issue_id, str) and issue_id not in protected_ids + ], + } + for name, cluster in triage_clusters(plan).items() + } auto_clusters = { - name: cluster + name: { + **cluster, + "issue_ids": [ + issue_id + for issue_id in cluster.get("issue_ids", []) + if isinstance(issue_id, str) and issue_id not in protected_ids + ], + } for name, cluster in plan.get("clusters", {}).items() if cluster.get("auto") and not name.startswith(EPIC_PREFIX) } open_review, open_mechanical = _split_open_issue_buckets(issues) + open_review = { + issue_id: issue + for issue_id, issue in open_review.items() + if issue_id not in protected_ids + } snapshot = build_triage_snapshot(plan, state) - triaged_ids = set(meta.get("triaged_ids", [])) + triaged_ids = set(meta.get("triaged_ids", [])) - protected_ids current_review_ids = set(open_review.keys()) new_since = set(snapshot.new_since_triage_ids) resolved_since = triaged_ids - current_review_ids - previously_dismissed = list(meta.get("dismissed_ids", [])) + previously_dismissed = [ + issue_id + for issue_id in meta.get("dismissed_ids", []) + if issue_id not in protected_ids + ] version = int(meta.get("version", 0)) + 1 # Resolved issue objects (for REFLECT stage) diff --git a/desloppify/engine/_plan/triage/protection.py b/desloppify/engine/_plan/triage/protection.py new file mode 100644 index 000000000..ff94be39c --- /dev/null +++ b/desloppify/engine/_plan/triage/protection.py @@ -0,0 +1,148 @@ +"""User-owned review issue exclusions for triage automation. + +Protected review IDs remain open in state and may remain in user-owned plan +clusters. They are deliberately outside automated triage scope; protection is +not a skip, resolution, or disposition. +""" + +from __future__ import annotations + +from collections.abc import Mapping, MutableMapping + +from desloppify.engine._plan.skip_policy import SYSTEM_SKIP_KINDS + +PROTECTED_REVIEW_ISSUE_IDS_KEY = "protected_review_issue_ids" + + +def protected_review_issue_ids_from_meta(meta: Mapping[str, object] | None) -> set[str]: + """Return normalized user-protected review IDs from triage metadata.""" + if not isinstance(meta, Mapping): + return set() + raw_ids = meta.get(PROTECTED_REVIEW_ISSUE_IDS_KEY) + if not isinstance(raw_ids, list): + return set() + return { + issue_id.strip() + for issue_id in raw_ids + if isinstance(issue_id, str) and issue_id.strip() + } + + +def protected_review_issue_ids(plan: Mapping[str, object] | None) -> set[str]: + """Return explicit review IDs that automated triage must leave untouched.""" + if not isinstance(plan, Mapping): + return set() + meta = plan.get("epic_triage_meta") + return protected_review_issue_ids_from_meta(meta if isinstance(meta, Mapping) else None) + + +def clear_protected_triage_artifacts( + plan: MutableMapping[str, object], + state: MutableMapping[str, object] | None = None, +) -> None: + """Remove automated plan artifacts for protected review IDs. + + Protection is an exclusion from triage, not a disposition. When a user + protects an ID that was previously touched by automation, clear those + automation-owned plan entries instead of retaining or rewriting them. + + This is deliberately a sanitation boundary rather than a skip operation: + protected IDs must not remain in execution containers that can make them + visible to ``next``. Manual cluster membership is retained, but automated + cluster membership and all queue/override references are removed. + """ + protected_ids = protected_review_issue_ids(plan) + if not protected_ids: + return + + automated_ids: set[str] = set() + meta = plan.get("epic_triage_meta") + if isinstance(meta, MutableMapping): + dispositions = meta.get("issue_dispositions") + if isinstance(dispositions, MutableMapping): + for issue_id in protected_ids: + entry = dispositions.get(issue_id) + if isinstance(entry, Mapping) and entry.get("decision_source") == "observe_auto": + automated_ids.add(issue_id) + dispositions.pop(issue_id, None) + for key in ("active_triage_issue_ids", "undispositioned_issue_ids", "dismissed_ids"): + issue_ids = meta.get(key) + if isinstance(issue_ids, list): + meta[key] = [ + issue_id + for issue_id in issue_ids + if issue_id not in protected_ids + ] + + root_dispositions = plan.get("issue_dispositions") + if isinstance(root_dispositions, MutableMapping): + for issue_id in protected_ids: + entry = root_dispositions.get(issue_id) + if isinstance(entry, Mapping) and entry.get("decision_source") == "observe_auto": + automated_ids.add(issue_id) + root_dispositions.pop(issue_id, None) + + skipped = plan.get("skipped") + if isinstance(skipped, MutableMapping): + for issue_id in protected_ids: + entry = skipped.get(issue_id) + if isinstance(entry, Mapping) and entry.get("kind") in SYSTEM_SKIP_KINDS: + automated_ids.add(issue_id) + skipped.pop(issue_id, None) + + queue_order = plan.get("queue_order") + if isinstance(queue_order, list): + queue_order[:] = [ + issue_id for issue_id in queue_order if issue_id not in protected_ids + ] + + promoted_ids = plan.get("promoted_ids") + if isinstance(promoted_ids, list): + promoted_ids[:] = [ + issue_id for issue_id in promoted_ids if issue_id not in protected_ids + ] + + overrides = plan.get("overrides") + if isinstance(overrides, MutableMapping): + for issue_id in protected_ids: + overrides.pop(issue_id, None) + + clusters = plan.get("clusters") + if isinstance(clusters, Mapping): + for cluster in clusters.values(): + if not isinstance(cluster, MutableMapping) or not cluster.get("auto"): + continue + issue_ids = cluster.get("issue_ids") + if not isinstance(issue_ids, list): + continue + cluster["issue_ids"] = [ + issue_id for issue_id in issue_ids if issue_id not in protected_ids + ] + + active_cluster = plan.get("active_cluster") + active = clusters.get(active_cluster) if isinstance(active_cluster, str) else None + if isinstance(active, Mapping) and active.get("auto") and not active.get("issue_ids"): + plan["active_cluster"] = None + + if state is None: + return + for issue_map in (state.get("work_items"), state.get("issues")): + if not isinstance(issue_map, MutableMapping): + continue + for issue_id in protected_ids: + issue = issue_map.get(issue_id) + if not isinstance(issue, MutableMapping): + continue + status = issue.get("status") + if status == "triaged_out" or ( + issue_id in automated_ids and status == "false_positive" + ): + issue["status"] = "open" + + +__all__ = [ + "PROTECTED_REVIEW_ISSUE_IDS_KEY", + "clear_protected_triage_artifacts", + "protected_review_issue_ids", + "protected_review_issue_ids_from_meta", +] diff --git a/desloppify/engine/_plan/triage/snapshot.py b/desloppify/engine/_plan/triage/snapshot.py index a11589f46..ae7aee257 100644 --- a/desloppify/engine/_plan/triage/snapshot.py +++ b/desloppify/engine/_plan/triage/snapshot.py @@ -6,12 +6,15 @@ from desloppify.engine._plan.cluster_membership import cluster_issue_ids from desloppify.engine._plan.constants import TRIAGE_IDS, is_synthetic_id -from desloppify.engine._plan.policy.stale import open_review_ids +from desloppify.engine._plan.policy.stale import triage_open_review_ids from desloppify.engine._plan.schema import Cluster, PlanModel -from desloppify.engine._plan.triage.playbook import TriageProgress, compute_triage_progress +from desloppify.engine._plan.triage.playbook import ( + TriageProgress, + compute_triage_progress, +) +from desloppify.engine._plan.triage.protection import protected_review_issue_ids from desloppify.engine._state.schema import StateModel - _cluster_issue_ids = cluster_issue_ids @@ -45,22 +48,24 @@ def plan_review_ids(plan: PlanModel) -> list[str]: def coverage_open_ids(plan: PlanModel, state: StateModel) -> set[str]: """Return the frozen or live open review IDs covered by this triage run.""" meta = plan.get("epic_triage_meta", {}) + protected_ids = protected_review_issue_ids(plan) active_ids = _normalized_issue_id_list(meta.get("active_triage_issue_ids")) if active_ids: - return set(active_ids) + return set(active_ids) - protected_ids has_completed_scan = bool(state.get("last_scan")) - review_ids = open_review_ids(state) + review_ids = triage_open_review_ids(plan, state) if not has_completed_scan and not review_ids: - return set(plan_review_ids(plan)) + return set(plan_review_ids(plan)) - protected_ids return review_ids def active_triage_issue_ids(plan: PlanModel, state: StateModel | None = None) -> set[str]: """Return the frozen review issue set for the current triage run.""" meta = plan.get("epic_triage_meta", {}) + protected_ids = protected_review_issue_ids(plan) active_ids = _normalized_issue_id_list(meta.get("active_triage_issue_ids")) if active_ids: - return set(active_ids) + return set(active_ids) - protected_ids if state is None: return set() return coverage_open_ids(plan, state) @@ -68,7 +73,7 @@ def active_triage_issue_ids(plan: PlanModel, state: StateModel | None = None) -> def _explicit_active_triage_issue_ids(plan: PlanModel) -> set[str]: meta = plan.get("epic_triage_meta", {}) - return set(_normalized_issue_id_list(meta.get("active_triage_issue_ids"))) + return set(_normalized_issue_id_list(meta.get("active_triage_issue_ids"))) - protected_review_issue_ids(plan) def live_active_triage_issue_ids(plan: PlanModel, state: StateModel | None = None) -> set[str]: @@ -76,7 +81,7 @@ def live_active_triage_issue_ids(plan: PlanModel, state: StateModel | None = Non frozen_ids = active_triage_issue_ids(plan, state) if state is None or not frozen_ids: return frozen_ids - return frozen_ids & open_review_ids(state) + return frozen_ids & triage_open_review_ids(plan, state) def undispositioned_triage_issue_ids(plan: PlanModel, state: StateModel | None = None) -> list[str]: @@ -156,9 +161,10 @@ class TriageSnapshot: def build_triage_snapshot(plan: PlanModel, state: StateModel) -> TriageSnapshot: """Build a canonical triage snapshot from plan and state.""" meta = plan.get("epic_triage_meta", {}) - triaged_ids = set(_normalized_issue_id_list(meta.get("triaged_ids"))) - frozen_ids = _explicit_active_triage_issue_ids(plan) & open_review_ids(state) - live_open = open_review_ids(state) + protected_ids = protected_review_issue_ids(plan) + triaged_ids = set(_normalized_issue_id_list(meta.get("triaged_ids"))) - protected_ids + frozen_ids = _explicit_active_triage_issue_ids(plan) & triage_open_review_ids(plan, state) + live_open = triage_open_review_ids(plan, state) known_ids = triaged_ids | frozen_ids new_since = live_open - known_ids if known_ids else set() in_scope_ids = coverage_open_ids(plan, state) diff --git a/desloppify/engine/_work_queue/selection.py b/desloppify/engine/_work_queue/selection.py index f8fb6c1c1..6889cbe49 100644 --- a/desloppify/engine/_work_queue/selection.py +++ b/desloppify/engine/_work_queue/selection.py @@ -2,14 +2,15 @@ from __future__ import annotations +from desloppify.engine._plan.triage.protection import protected_review_issue_ids +from desloppify.engine._state.issue_semantics import is_review_work_item +from desloppify.engine._state.schema import StateModel from desloppify.engine._work_queue.helpers import scope_matches from desloppify.engine._work_queue.inputs import gather_subjective_items from desloppify.engine._work_queue.models import QueueBuildOptions, QueueVisibility from desloppify.engine._work_queue.ranking import build_issue_items from desloppify.engine._work_queue.snapshot import build_queue_snapshot from desloppify.engine._work_queue.types import WorkQueueItem -from desloppify.engine._state.issue_semantics import is_review_work_item -from desloppify.engine._state.schema import StateModel def select_queue_items( @@ -44,6 +45,9 @@ def select_queue_items( ) if opts.include_subjective and status == "all": items += gather_subjective_items(state, opts, threshold) + protected_ids = protected_review_issue_ids(plan) + if protected_ids: + items = [item for item in items if item.get("id", "") not in protected_ids] return items diff --git a/desloppify/engine/_work_queue/snapshot.py b/desloppify/engine/_work_queue/snapshot.py index 04984cded..99f65d8d0 100644 --- a/desloppify/engine/_work_queue/snapshot.py +++ b/desloppify/engine/_work_queue/snapshot.py @@ -14,10 +14,6 @@ WORKFLOW_DEFERRED_DISPOSITION_ID, WORKFLOW_RUN_SCAN_ID, ) -from desloppify.engine._plan.schema import ( - executable_objective_ids as _executable_objective_ids, - live_planned_queue_ids as _live_planned_queue_ids, -) from desloppify.engine._plan.refresh_lifecycle import ( LIFECYCLE_PHASE_ASSESSMENT_POSTFLIGHT, LIFECYCLE_PHASE_EXECUTE, @@ -29,13 +25,19 @@ current_lifecycle_phase, derive_display_phase, ) +from desloppify.engine._plan.schema import ( + executable_objective_ids as _executable_objective_ids, +) +from desloppify.engine._plan.schema import ( + live_planned_queue_ids as _live_planned_queue_ids, +) +from desloppify.engine._plan.triage.protection import protected_review_issue_ids from desloppify.engine._plan.triage.snapshot import build_triage_snapshot from desloppify.engine._state.filtering import path_scoped_issues from desloppify.engine._state.issue_semantics import ( counts_toward_objective_backlog, is_assessment_request, is_review_work_item, - is_triage_finding, ) from desloppify.engine._state.schema import StateModel from desloppify.engine._work_queue.ranking import build_issue_items @@ -118,8 +120,21 @@ def _is_objective_item(item: WorkQueueItem, *, skipped_ids: set[str]) -> bool: ) -def _review_issue_items(items: Iterable[WorkQueueItem]) -> list[WorkQueueItem]: - return [item for item in items if is_triage_finding(item)] +def _review_issue_items( + items: Iterable[WorkQueueItem], + plan: dict | None = None, +) -> list[WorkQueueItem]: + """Return review findings visible to automation and queue views. + + A protected review ID stays open in state for user-owned follow-up, but + is intentionally absent from the automated work queue. + """ + protected_ids = protected_review_issue_ids(plan) + return [ + item + for item in items + if is_review_work_item(item) and item.get("id", "") not in protected_ids + ] def _assessment_request_items(items: Iterable[WorkQueueItem]) -> list[WorkQueueItem]: @@ -442,7 +457,7 @@ def _build_item_partitions( if item.get("id", "") in executable_objective_ids ] - review_issue_items = _review_issue_items(all_issue_items) + review_issue_items = _review_issue_items(all_issue_items, effective_plan) assessment_request_items_list = _assessment_request_items(all_issue_items) executable_review_items = _executable_review_issue_items( effective_plan, diff --git a/desloppify/intelligence/review/__init__.py b/desloppify/intelligence/review/__init__.py index 9e8ab4316..f7ff3959c 100644 --- a/desloppify/intelligence/review/__init__.py +++ b/desloppify/intelligence/review/__init__.py @@ -16,8 +16,6 @@ from typing import Any from desloppify.engine._state.schema import StateModel, utc_now -from desloppify.intelligence.review.importing.contracts_types import ReviewImportPayload - from desloppify.intelligence.integrity import ( is_holistic_subjective_issue, is_subjective_review_open, @@ -40,6 +38,8 @@ get_lang_guidance, ) from desloppify.intelligence.review.dimensions.selection import resolve_dimensions +from desloppify.intelligence.review.importing.contracts_types import ReviewImportPayload +from desloppify.intelligence.review.personas import PERSONAS, Persona, assign_personas from desloppify.intelligence.review.policy import ( DimensionPolicy, append_custom_dimensions, @@ -55,8 +55,9 @@ prepare_holistic_review, prepare_review, ) -from desloppify.intelligence.review.personas import PERSONAS, Persona, assign_personas -from desloppify.intelligence.review.prepare_batches_builders import build_investigation_batches +from desloppify.intelligence.review.prepare_batches_builders import ( + build_investigation_batches, +) from desloppify.intelligence.review.remediation import generate_remediation_plan from desloppify.intelligence.review.selection import ( LOW_VALUE_NAMES, @@ -94,6 +95,7 @@ def import_holistic_issues( lang_name: str, *, project_root: Path | str | None = None, + preserve_open_issue_ids: set[str] | None = None, utc_now_fn=utc_now, ) -> dict[str, Any]: """Lazy wrapper to avoid import cycles during package initialization.""" @@ -106,6 +108,7 @@ def import_holistic_issues( state, lang_name, project_root=project_root, + preserve_open_issue_ids=preserve_open_issue_ids, utc_now_fn=utc_now_fn, ) diff --git a/desloppify/intelligence/review/importing/holistic.py b/desloppify/intelligence/review/importing/holistic.py index decc57f9b..f52f6a305 100644 --- a/desloppify/intelligence/review/importing/holistic.py +++ b/desloppify/intelligence/review/importing/holistic.py @@ -5,9 +5,9 @@ from pathlib import Path from typing import Any -from desloppify.engine.concerns import cleanup_stale_dismissals, generate_concerns from desloppify.engine._state.merge import MergeScanOptions, merge_scan from desloppify.engine._state.schema import StateModel, utc_now +from desloppify.engine.concerns import cleanup_stale_dismissals, generate_concerns from desloppify.engine.scoring import HOLISTIC_POTENTIAL from desloppify.intelligence.review.dimensions import normalize_dimension_name from desloppify.intelligence.review.dimensions.data import load_dimensions_for_lang @@ -58,6 +58,7 @@ def import_holistic_issues( lang_name: str, *, project_root: Path | str | None = None, + preserve_open_issue_ids: set[str] | None = None, utc_now_fn=utc_now, ) -> dict[str, Any]: """Import holistic (codebase-wide) issues into state.""" @@ -166,6 +167,7 @@ def import_holistic_issues( utc_now_fn, imported_dimensions=imported_dimensions, full_sweep_included=scope_full_sweep, + preserve_open_issue_ids=preserve_open_issue_ids, ) if skipped: diff --git a/desloppify/intelligence/review/importing/holistic_issue_flow.py b/desloppify/intelligence/review/importing/holistic_issue_flow.py index afd40d132..28e6cd09c 100644 --- a/desloppify/intelligence/review/importing/holistic_issue_flow.py +++ b/desloppify/intelligence/review/importing/holistic_issue_flow.py @@ -206,8 +206,14 @@ def auto_resolve_stale_holistic( *, imported_dimensions: set[str] | None = None, full_sweep_included: bool | None = None, + preserve_open_issue_ids: set[str] | None = None, ) -> None: """Auto-resolve open holistic issues not present in the latest import.""" + preserved_ids = { + issue_id.strip() + for issue_id in (preserve_open_issue_ids or set()) + if isinstance(issue_id, str) and issue_id.strip() + } scope_dimensions = { normalize_dimension_name(dim) for dim in (imported_dimensions or set()) @@ -218,6 +224,8 @@ def auto_resolve_stale_holistic( return def _should_resolve(issue: Issue) -> bool: + if str(issue.get("id", "")) in preserved_ids: + return False if issue.get("detector") not in ("review", "concerns"): return False detail = issue.get("detail") diff --git a/desloppify/tests/commands/plan/test_plan_override_transactions.py b/desloppify/tests/commands/plan/test_plan_override_transactions.py index c8e7b22c5..944744751 100644 --- a/desloppify/tests/commands/plan/test_plan_override_transactions.py +++ b/desloppify/tests/commands/plan/test_plan_override_transactions.py @@ -7,13 +7,15 @@ import pytest +import desloppify.engine._plan.persistence as plan_persistence from desloppify import state as state_mod from desloppify.app.commands.helpers.command_runtime import CommandRuntime from desloppify.app.commands.plan.override import io as override_io +from desloppify.app.commands.plan.override import resolve_cmd as override_resolve from desloppify.app.commands.plan.override import skip as override_skip from desloppify.base.exception_sets import CommandError -from desloppify.engine.plan_state import empty_plan, load_plan, save_plan from desloppify.engine.plan_ops import skip_items +from desloppify.engine.plan_state import empty_plan, load_plan, save_plan _ATTEST = "I have actually reviewed this and I am not gaming the score." @@ -133,3 +135,57 @@ def _boom(*_args, **_kwargs): assert state_after["issues"][issue_id]["status"] == "open" assert plan_after.get("queue_order", []) == [issue_id] assert plan_after.get("skipped", {}) == {} + + +def test_cmd_plan_resolve_out_of_order_keeps_state_and_plan_unchanged( + tmp_path, + monkeypatch: pytest.MonkeyPatch, + capsys, +) -> None: + state_file = tmp_path / "state.json" + plan_file = tmp_path / "plan.json" + + state, first_id = _seed_state() + second = state_mod.make_issue( + "review", + "src/example.py", + "second-sample-issue", + tier=1, + confidence="high", + summary="second sample", + ) + second_id = second["id"] + state["work_items"][second_id] = second + plan = _seed_plan(first_id) + plan["queue_order"] = [first_id, second_id] + state_mod.save_state(state, state_file) + save_plan(plan, plan_file) + before_plan = load_plan(plan_file) + monkeypatch.setattr(plan_persistence, "PLAN_FILE", plan_file) + + runtime = CommandRuntime( + config={}, + state=copy.deepcopy(state), + state_path=state_file, + ) + override_resolve.cmd_plan_resolve( + argparse.Namespace( + runtime=runtime, + patterns=[second_id], + note="Verified the requested item but it is not at the front of the plan queue", + attest=_ATTEST, + confirm=False, + force_resolve=False, + state=state_file, + lang=None, + path=".", + exclude=None, + ) + ) + + assert "Queue order violation" in capsys.readouterr().out + state_after = state_mod.load_state(state_file) + plan_after = load_plan(plan_file) + assert state_after["work_items"][second_id]["status"] == "open" + assert plan_after["queue_order"] == before_plan["queue_order"] + assert plan_after["execution_log"] == before_plan["execution_log"] diff --git a/desloppify/tests/commands/plan/test_plan_overrides_direct.py b/desloppify/tests/commands/plan/test_plan_overrides_direct.py index d1b5e4c05..b4392e5bd 100644 --- a/desloppify/tests/commands/plan/test_plan_overrides_direct.py +++ b/desloppify/tests/commands/plan/test_plan_overrides_direct.py @@ -101,7 +101,6 @@ def test_override_resolve_cmd_confirm_allows_small_cluster(monkeypatch) -> None: } plan = {"clusters": {"small": {"issue_ids": ["i1", "i2"]}}} delegated: list[argparse.Namespace] = [] - log_entries: list[dict] = [] monkeypatch.setattr( override_resolve_cmd_mod, @@ -109,12 +108,6 @@ def test_override_resolve_cmd_confirm_allows_small_cluster(monkeypatch) -> None: lambda _args: SimpleNamespace(state=state), ) monkeypatch.setattr(override_resolve_cmd_mod, "load_plan", lambda: plan) - monkeypatch.setattr( - override_resolve_cmd_mod, - "append_log_entry", - lambda *_args, **kwargs: log_entries.append(kwargs), - ) - monkeypatch.setattr(override_resolve_cmd_mod, "save_plan", lambda _plan: None) monkeypatch.setattr(override_resolve_cmd_mod, "cmd_resolve", delegated.append) override_resolve_cmd_mod.cmd_plan_resolve( @@ -135,7 +128,6 @@ def test_override_resolve_cmd_confirm_allows_small_cluster(monkeypatch) -> None: assert delegated[0].patterns == ["small"] assert delegated[0].status == "fixed" assert delegated[0].attest.startswith("I have actually resolved the small cluster") - assert log_entries[0]["cluster_name"] == "small" def test_override_resolve_cmd_confirm_requires_note(capsys) -> None: @@ -155,6 +147,81 @@ def test_override_resolve_cmd_confirm_requires_note(capsys) -> None: assert "--confirm requires --note" in out +def test_override_resolve_cmd_rejects_protected_review_id(monkeypatch, capsys) -> None: + state = { + "issues": { + "held": {"id": "held", "status": "open", "detector": "review"}, + } + } + plan = {"epic_triage_meta": {"protected_review_issue_ids": ["held"]}} + delegated: list[argparse.Namespace] = [] + + monkeypatch.setattr( + override_resolve_cmd_mod, + "command_runtime", + lambda _args: SimpleNamespace(state=state), + ) + monkeypatch.setattr(override_resolve_cmd_mod, "load_plan", lambda: plan) + monkeypatch.setattr(override_resolve_cmd_mod, "cmd_resolve", delegated.append) + + override_resolve_cmd_mod.cmd_plan_resolve( + argparse.Namespace( + patterns=["held"], + attest=None, + note="Kept this review finding out of the automated triage queue", + confirm=True, + force_resolve=False, + state=None, + lang=None, + path=".", + exclude=None, + ) + ) + + assert delegated == [] + assert "Cannot resolve protected review item" in capsys.readouterr().out + + +def test_override_skip_cmd_rejects_protected_review_id(monkeypatch, capsys) -> None: + state = { + "scan_metadata": {"source": "scan"}, + "last_scan": "2026-07-31T00:00:00+00:00", + "issues": { + "held": {"id": "held", "status": "open", "detector": "review"}, + } + } + plan = {"epic_triage_meta": {"protected_review_issue_ids": ["held"]}} + state_transitions: list[str] = [] + + monkeypatch.setattr( + override_skip_mod, + "command_runtime", + lambda _args: SimpleNamespace(state=state, state_path=Path("state.json")), + ) + monkeypatch.setattr(override_skip_mod, "load_plan", lambda _path=None: plan) + monkeypatch.setattr( + override_skip_mod, + "_apply_state_skip_resolution", + lambda **_kwargs: state_transitions.append("resolved"), + ) + + override_skip_mod.cmd_plan_skip( + argparse.Namespace( + patterns=["held"], + reason="later", + review_after=None, + permanent=False, + false_positive=False, + note=None, + attest=None, + confirm=False, + ) + ) + + assert state_transitions == [] + assert "Cannot skip protected review item" in capsys.readouterr().out + + def test_override_resolve_cmd_handles_synthetic_only_resolution( monkeypatch, capsys ) -> None: @@ -354,7 +421,9 @@ def test_resolve_workflow_patterns_reconciles_when_create_plan_drains_queue( resolve_workflow_mod, "live_planned_queue_empty", lambda _plan: True ) monkeypatch.setattr( - resolve_workflow_mod, "has_open_review_issues", lambda _state: True + resolve_workflow_mod, + "has_open_review_issues", + lambda _state, _plan=None: True, ) monkeypatch.setattr( resolve_workflow_mod, @@ -1000,6 +1069,44 @@ def test_cmd_plan_reopen_reconciles_after_invalidation(monkeypatch) -> None: assert ("emit", "execute") in seen +def test_cmd_plan_reopen_keeps_protected_review_hold_out_of_queue(monkeypatch) -> None: + state_data = { + "config": {}, + "work_items": {"held": {"id": "held", "status": "fixed", "detector": "review"}}, + } + plan = { + "queue_order": [], + "skipped": {}, + "epic_triage_meta": {"protected_review_issue_ids": ["held"]}, + } + + monkeypatch.setattr( + override_misc_mod, "state_path", lambda _args: Path("state.json") + ) + monkeypatch.setattr(override_misc_mod, "load_state", lambda _path: state_data) + monkeypatch.setattr( + override_misc_mod, "_plan_file_for_state", lambda _path: Path("plan.json") + ) + monkeypatch.setattr(override_misc_mod, "load_plan", lambda _path=None: plan) + monkeypatch.setattr( + override_misc_mod, + "resolve_issues", + lambda state, _pattern, _status: state["work_items"]["held"].update(status="open") or ["held"], + ) + monkeypatch.setattr( + override_misc_mod, "purge_uncommitted_ids", lambda *_a, **_k: None + ) + monkeypatch.setattr(override_misc_mod, "append_log_entry", lambda *_a, **_k: None) + monkeypatch.setattr( + override_misc_mod, "save_plan_state_transactional", lambda **_k: None + ) + + override_misc_mod.cmd_plan_reopen(argparse.Namespace(patterns=["held"])) + + assert state_data["work_items"]["held"]["status"] == "open" + assert plan["queue_order"] == [] + + def test_cmd_plan_skip_invalid_permanent_skip_exits_nonzero(monkeypatch) -> None: runtime = SimpleNamespace( state={"last_scan": "2026-03-01T00:00:00+00:00", "scan_count": 2, "issues": {}}, diff --git a/desloppify/tests/commands/plan/test_strategist.py b/desloppify/tests/commands/plan/test_strategist.py index 604681d87..09fa9b8cc 100644 --- a/desloppify/tests/commands/plan/test_strategist.py +++ b/desloppify/tests/commands/plan/test_strategist.py @@ -78,6 +78,59 @@ def test_cmd_stage_strategize_persists_briefing_and_auto_confirms(monkeypatch, c assert "auto-confirmed" in capsys.readouterr().out +def test_cmd_stage_strategize_forwards_auto_start_attestation(monkeypatch) -> None: + plan = { + "queue_order": [], + "epic_triage_meta": {"triage_stages": {}}, + "execution_log": [], + "commit_log": [], + } + state = {"scan_count": 1, "scan_history": [], "dimension_scores": {}, "work_items": {}} + captured: dict[str, object] = {} + + def start_triage(_plan, **kwargs): + captured["attestation"] = kwargs["attestation"] + plan["queue_order"] = list(TRIAGE_STAGE_IDS) + return SimpleNamespace(status="started") + + monkeypatch.setattr(strategize_mod, "ensure_triage_started", start_triage) + monkeypatch.setattr(strategize_mod, "load_progression", lambda: []) + monkeypatch.setattr( + strategize_mod, + "collect_strategist_input", + lambda *_args, **_kwargs: SimpleNamespace( + rework_loops=[], + score_trajectory=SimpleNamespace(trend="stable"), + debt_trajectory=SimpleNamespace(trend="stable"), + ), + ) + + strategize_mod.cmd_stage_strategize( + argparse.Namespace( + attestation="I reviewed the objective backlog before starting triage.", + report=( + '{"score_trend":"stable","debt_trend":"stable",' + '"executive_summary":"' + + ("x" * 120) + + '","observe_guidance":"' + + ("y" * 60) + + '","reflect_guidance":"' + + ("z" * 60) + + '","organize_guidance":"' + + ("o" * 60) + + '","sense_check_guidance":"' + + ("s" * 60) + + '","focus_dimensions":[{"name":"naming"}]}' + ), + ), + services=_services(plan, state), + ) + + assert captured == { + "attestation": "I reviewed the objective backlog before starting triage." + } + + def test_observe_is_blocked_until_strategize_is_recorded(capsys) -> None: plan = {"queue_order": list(TRIAGE_STAGE_IDS), "epic_triage_meta": {"triage_stages": {}}, "execution_log": [], "commit_log": []} state = {"work_items": {}} diff --git a/desloppify/tests/commands/plan/test_triage_coverage.py b/desloppify/tests/commands/plan/test_triage_coverage.py index 4fd199289..6a33f2103 100644 --- a/desloppify/tests/commands/plan/test_triage_coverage.py +++ b/desloppify/tests/commands/plan/test_triage_coverage.py @@ -2,9 +2,12 @@ from __future__ import annotations -from desloppify.app.commands.plan.triage.review_coverage import triage_coverage -from desloppify.engine._plan.schema import empty_plan +from desloppify.app.commands.plan.triage.review_coverage import ( + has_open_review_issues, + triage_coverage, +) from desloppify.engine._plan.constants import TRIAGE_STAGE_IDS +from desloppify.engine._plan.schema import empty_plan def _plan_with_queue(*issue_ids: str, clustered: list[str] | None = None) -> dict: @@ -28,6 +31,13 @@ def _review_ids(*ids: str) -> set[str]: class TestTriageCoverage: + def test_workflow_review_gate_ignores_protected_only_review_ids(self): + plan = empty_plan() + plan["epic_triage_meta"] = {"protected_review_issue_ids": ["held"]} + state = {"issues": {"held": {"status": "open", "detector": "review"}}} + + assert has_open_review_issues(state, plan) is False + def test_coverage_excludes_non_review_items(self): """Non-review queue items (mechanical issues) don't inflate total.""" plan = _plan_with_queue( diff --git a/desloppify/tests/commands/plan/test_triage_split_modules_direct.py b/desloppify/tests/commands/plan/test_triage_split_modules_direct.py index c96df51b5..185cb777d 100644 --- a/desloppify/tests/commands/plan/test_triage_split_modules_direct.py +++ b/desloppify/tests/commands/plan/test_triage_split_modules_direct.py @@ -23,14 +23,18 @@ import desloppify.app.commands.plan.triage.runner.orchestrator_codex_pipeline_execution as orchestrator_pipeline_execution_mod import desloppify.app.commands.plan.triage.runner.orchestrator_codex_sense as orchestrator_sense_mod import desloppify.app.commands.plan.triage.runner.orchestrator_common as orchestrator_common_mod +import desloppify.app.commands.plan.triage.stages.commands as stage_commands_mod +import desloppify.app.commands.plan.triage.stages.helpers as stage_helpers_mod import desloppify.app.commands.plan.triage.stages.organize as organize_stage_mod import desloppify.app.commands.plan.triage.validation.completion_policy as completion_policy_mod import desloppify.app.commands.plan.triage.validation.completion_stages as completion_stages_mod import desloppify.app.commands.plan.triage.validation.enrich_checks as enrich_checks_mod +import desloppify.app.commands.plan.triage.validation.organize_policy as organize_policy_mod from desloppify.app.commands.plan.triage.runner.orchestrator_codex_pipeline_execution import ( StageExecutionResult, ) from desloppify.base.exception_sets import CommandError +from desloppify.engine._plan.schema import empty_plan from desloppify.engine.plan_triage import build_triage_snapshot @@ -61,6 +65,61 @@ def _make_stage_context( return orchestrator_pipeline_context_mod.StageRunContext(**defaults) +def test_active_triage_scope_stays_empty_when_protected_items_exhaust_it() -> None: + plan = {"epic_triage_meta": {"active_triage_issue_ids": []}} + + assert stage_helpers_mod.active_triage_issue_scope(plan, {"last_scan": "now"}) == set() + + +def test_active_triage_scope_does_not_absorb_new_reviews_after_empty_freeze() -> None: + plan = {"epic_triage_meta": {"active_triage_issue_ids": []}} + state = { + "last_scan": "now", + "work_items": {"review::new.py::late": {"detector": "review", "status": "open"}}, + } + + assert stage_helpers_mod.active_triage_issue_scope(plan, state) == set() + assert stage_helpers_mod.unclustered_review_issues(plan, state) == [] + + +def test_enrich_quality_empty_triage_scope_ignores_historical_clusters(tmp_path: Path) -> None: + import desloppify.app.commands.plan.triage.validation.enrich_quality as enrich_quality_mod + + plan = { + "clusters": { + "historical": { + "auto": False, + "issue_ids": ["review::old.py::historic"], + "action_steps": [{"title": "Historic step"}], + } + } + } + kwargs = { + "phase_label": "enrich", + "bad_paths_severity": "warning", + "missing_effort_severity": "warning", + "include_missing_issue_refs": False, + "include_vague_detail": False, + "stale_issue_refs_severity": None, + } + + scoped = enrich_quality_mod.evaluate_enrich_quality( + plan, + tmp_path, + triage_issue_ids=set(), + **kwargs, + ) + unscoped = enrich_quality_mod.evaluate_enrich_quality( + plan, + tmp_path, + triage_issue_ids=None, + **kwargs, + ) + + assert scoped.failures == [] + assert unscoped.failure("underspecified").total == 1 + + def test_completion_policy_helpers_cover_success_and_fail_paths(monkeypatch, capsys) -> None: monkeypatch.setattr( completion_policy_mod, @@ -70,8 +129,8 @@ def test_completion_policy_helpers_cover_success_and_fail_paths(monkeypatch, cap monkeypatch.setattr(completion_policy_mod, "active_triage_issue_scope", lambda _plan, _state=None: None) monkeypatch.setattr( completion_policy_mod, - "open_review_ids_from_state", - lambda _state: {"review::a.py::id1"}, + "triage_open_review_ids_from_state", + lambda _plan, _state: {"review::a.py::id1"}, ) monkeypatch.setattr(completion_policy_mod, "triage_coverage", lambda _plan, open_review_ids: (1, 1, [])) monkeypatch.setattr(completion_policy_mod, "unenriched_clusters", lambda _plan, _state=None: []) @@ -132,6 +191,21 @@ def test_completion_policy_helpers_cover_success_and_fail_paths(monkeypatch, cap assert "Strategy too short" in out +def test_completion_gate_ignores_only_protected_open_review_ids() -> None: + plan = empty_plan() + plan["epic_triage_meta"] = {"protected_review_issue_ids": ["review::held"]} + state = { + "issues": { + "review::held": {"status": "open", "detector": "review"}, + }, + } + + readiness = completion_policy_mod.evaluate_completion_readiness(plan, state) + + assert readiness.ok is True + assert readiness.open_review_ids == frozenset() + + def test_runner_validate_completion_uses_shared_completion_boundary(monkeypatch, tmp_path: Path) -> None: import desloppify.app.commands.plan.triage.runner.stage_validation as stage_validation_mod @@ -275,7 +349,11 @@ def test_validate_organize_submission_passes_state_to_enrichment_gate(monkeypatc captured: dict[str, object] = {} state = {"issues": {"review::closed-only": {"status": "closed", "detector": "review"}}} - monkeypatch.setattr(organize_stage_mod, "open_review_ids_from_state", lambda _state: set()) + monkeypatch.setattr( + organize_stage_mod, + "triage_open_review_ids_from_state", + lambda _plan, _state: set(), + ) monkeypatch.setattr( organize_stage_mod, "auto_confirm_reflect_for_organize", lambda **_kwargs: True ) @@ -329,6 +407,45 @@ def _capture_enriched(plan, actual_state): assert captured["state"] is state +def test_validate_backlog_promotions_accepts_active_or_queued_members() -> None: + plan = { + "queue_order": ["review::ready"], + "epic_triage_meta": {"protected_review_issue_ids": ["review::held"]}, + "clusters": { + "auto/active-empty": { + "issue_ids": [], + "execution_status": "active", + }, + "auto/queued-members": { + "issue_ids": ["review::ready", "review::held"], + "execution_status": "review", + }, + "auto/empty-review": { + "issue_ids": [], + "execution_status": "review", + }, + }, + } + stages = { + "reflect": { + "backlog_decisions": [ + {"cluster_name": "auto/active-empty", "decision": "promote"}, + {"cluster_name": "auto/queued-members", "decision": "promote"}, + {"cluster_name": "auto/empty-review", "decision": "promote"}, + ] + } + } + + warnings = organize_policy_mod.validate_backlog_promotions_executed( + plan=plan, + stages=stages, + ) + + assert warnings == [ + "Reflect requested promoting auto/empty-review but it was not promoted during organize." + ] + + def test_confirm_organize_passes_state_to_enrichment_gate(monkeypatch) -> None: captured: dict[str, object] = {} state = {"issues": {"review::closed-only": {"status": "closed", "detector": "review"}}} @@ -1850,6 +1967,58 @@ def test_execute_stage_fails_when_handler_does_not_persist_stage(monkeypatch, tm assert result.payload["error"] == "stage_not_recorded" +def test_record_reflect_report_forwards_rejection(monkeypatch) -> None: + monkeypatch.setattr(stage_commands_mod, "cmd_stage_reflect", lambda *_a, **_k: False) + + accepted = orchestrator_pipeline_execution_mod._record_reflect_report( + "reflect report", + argparse.Namespace(state=None), + SimpleNamespace(), + ) + + assert accepted is False + + +def test_execute_stage_reports_reflect_record_validation_rejection(monkeypatch, tmp_path: Path) -> None: + monkeypatch.setattr(orchestrator_pipeline_mod, "build_stage_prompt", lambda *a, **k: "prompt") + monkeypatch.setattr( + orchestrator_pipeline_mod, + "run_triage_stage", + lambda **_kwargs: codex_runner_mod.TriageStageRunResult(exit_code=0), + ) + monkeypatch.setitem( + orchestrator_pipeline_mod._STAGE_HANDLERS, + "reflect", + orchestrator_pipeline_mod.StageHandler(record_report=lambda *_a, **_k: False), + ) + + run_log: list[str] = [] + services = SimpleNamespace(load_plan=lambda: {"epic_triage_meta": {"triage_stages": {}}}) + result = orchestrator_pipeline_execution_mod.execute_stage( + _make_stage_context( + tmp_path, + stage="reflect", + services=services, + plan={"epic_triage_meta": {"triage_stages": {"observe": {"report": "ok"}}}}, + triage_input=SimpleNamespace(open_issues={}), + prior_reports={"observe": "ok"}, + append_run_log=run_log.append, + ), + handlers=orchestrator_pipeline_mod._STAGE_HANDLERS, + dependencies=orchestrator_pipeline_mod.StageExecutionDependencies( + build_stage_prompt=lambda *_a, **_k: "prompt", + run_triage_stage=lambda **_kwargs: codex_runner_mod.TriageStageRunResult(exit_code=0), + read_stage_output=lambda _path: "x" * 120, + analyze_reflect_issue_accounting=orchestrator_pipeline_mod._analyze_reflect_issue_accounting, + validate_reflect_issue_accounting=orchestrator_pipeline_mod._validate_reflect_issue_accounting, + ), + ) + + assert result.status == "failed" + assert result.payload["error"] == "reflect_report_rejected" + assert any("stage-record-rejected stage=reflect" in line for line in run_log) + + def test_run_codex_pipeline_raises_on_stage_failure(monkeypatch, tmp_path: Path) -> None: monkeypatch.setattr(orchestrator_pipeline_mod, "get_project_root", lambda: tmp_path) monkeypatch.setattr(orchestrator_pipeline_mod, "run_stamp", lambda: "20260309_151500") @@ -1885,3 +2054,71 @@ def test_run_codex_pipeline_raises_on_stage_failure(monkeypatch, tmp_path: Path) assert excinfo.value.exit_code == 1 assert "triage stage failed: organize" in excinfo.value.message + + +def test_run_codex_pipeline_fails_when_triage_start_is_blocked(monkeypatch, tmp_path: Path) -> None: + """A startup guard must not look like a successful no-artifact runner pass.""" + monkeypatch.setattr(orchestrator_pipeline_mod, "get_project_root", lambda: tmp_path) + monkeypatch.setattr( + orchestrator_pipeline_mod, + "ensure_triage_started", + lambda *_args, **_kwargs: triage_lifecycle_mod.TriageStartOutcome( + status="blocked", + reason="unfinished_triage_stage_records", + ), + ) + monkeypatch.setattr( + orchestrator_pipeline_mod, + "_run_stage_sequence", + lambda *_args, **_kwargs: pytest.fail("blocked startup must not launch a stage"), + ) + + services = SimpleNamespace( + load_plan=lambda: {"epic_triage_meta": {"triage_stages": {}}}, + command_runtime=lambda _args: SimpleNamespace(state={}), + ) + + with pytest.raises(CommandError) as excinfo: + orchestrator_pipeline_mod.run_codex_pipeline( + argparse.Namespace(stage_timeout_seconds=30, dry_run=False, state=None), + stages_to_run=["reflect"], + services=services, + ) + + assert excinfo.value.exit_code == 1 + assert "unfinished_triage_stage_records" in excinfo.value.message + assert "before executing any stage" in excinfo.value.message.lower() + assert not (tmp_path / ".desloppify" / "triage_runs").exists() + + +def test_run_codex_pipeline_quotes_cli_helper_path_with_spaces(monkeypatch, tmp_path: Path) -> None: + repo_root = tmp_path / "repo with spaces" + repo_root.mkdir() + captured: dict[str, object] = {} + + monkeypatch.setattr(orchestrator_pipeline_mod, "get_project_root", lambda: repo_root) + monkeypatch.setattr(orchestrator_pipeline_mod, "run_stamp", lambda: "20260309_151500") + + def fail_after_capturing_context(context, *_args, **_kwargs): + captured["cli_command"] = context.cli_command + return StageExecutionResult(status="failed", payload={"status": "failed", "error": "boom"}) + + monkeypatch.setattr(orchestrator_pipeline_mod, "execute_stage_impl", fail_after_capturing_context) + + services = SimpleNamespace( + load_plan=lambda: {"epic_triage_meta": {"triage_stages": {}}}, + command_runtime=lambda _args: SimpleNamespace(state={}), + collect_triage_input=lambda _plan, _state: SimpleNamespace(open_issues={}, resolved_issues={}), + ) + monkeypatch.setattr(orchestrator_pipeline_mod, "default_triage_services", lambda: services) + monkeypatch.setattr(orchestrator_pipeline_mod, "ensure_triage_started", lambda *_a, **_k: None) + + with pytest.raises(CommandError): + orchestrator_pipeline_mod.run_codex_pipeline( + argparse.Namespace(stage_timeout_seconds=30, dry_run=False, state=None), + stages_to_run=["organize"], + services=services, + ) + + helper = repo_root / ".desloppify" / "triage_runs" / "20260309_151500" / "run_desloppify.sh" + assert captured["cli_command"] == f"'{helper}'" diff --git a/desloppify/tests/commands/plan/test_triage_stage_flow_observe_reflect_organize_direct.py b/desloppify/tests/commands/plan/test_triage_stage_flow_observe_reflect_organize_direct.py index 35b1f211d..25fa67d58 100644 --- a/desloppify/tests/commands/plan/test_triage_stage_flow_observe_reflect_organize_direct.py +++ b/desloppify/tests/commands/plan/test_triage_stage_flow_observe_reflect_organize_direct.py @@ -99,11 +99,40 @@ def test_reflect_exits_when_triage_not_in_queue(monkeypatch, capsys) -> None: services, _saved, _logs = _services(plan) monkeypatch.setattr(reflect_mod, "has_triage_in_queue", lambda _plan: False) - reflect_mod._cmd_stage_reflect(_args(report="r" * 120), services=services) + accepted = reflect_mod._cmd_stage_reflect(_args(report="r" * 120), services=services) out = capsys.readouterr().out + assert accepted is False assert "No planning stages in the queue" in out +def test_reflect_rejects_humanized_recurring_dimension_name(monkeypatch) -> None: + plan = { + "epic_triage_meta": { + "triage_stages": { + "observe": { + "report": "x" * 120, + "confirmed_at": "2026-03-09T00:00:00Z", + } + } + } + } + services, _saved, _logs = _services(plan, open_issues={"review::type_safety::aabbccdd": {}}) + services.detect_recurring_patterns = lambda _open, _resolved: { + "type_safety": {"open": ["review::type_safety::aabbccdd"], "resolved": []} + } + monkeypatch.setattr(reflect_mod, "has_triage_in_queue", lambda _plan: True) + monkeypatch.setattr(reflect_mod, "auto_confirm_observe_if_attested", lambda **_kwargs: True) + monkeypatch.setattr(reflect_mod, "validate_stage_report_length", lambda **_kwargs: True) + + accepted = reflect_mod._cmd_stage_reflect( + _args(report="Type safety remains important, but this does not use the validator key."), + services=services, + ) + + assert accepted is False + assert "reflect" not in plan["epic_triage_meta"]["triage_stages"] + + def test_organize_exits_when_reflect_requirement_not_met(monkeypatch) -> None: plan = {"epic_triage_meta": {"triage_stages": {}}, "clusters": {}} services, _saved, _logs = _services(plan, state={"issues": {}}) @@ -180,7 +209,7 @@ def test_reflect_rejects_incomplete_issue_accounting(monkeypatch, capsys) -> Non services, _saved, _logs = _services(plan, open_issues=open_issues) monkeypatch.setattr(reflect_mod, "has_triage_in_queue", lambda _plan: True) - reflect_mod._cmd_stage_reflect( + accepted = reflect_mod._cmd_stage_reflect( _args( report=( "Cluster alpha will handle aaaabbbb in src/a.ts after reviewing the current " @@ -191,6 +220,7 @@ def test_reflect_rejects_incomplete_issue_accounting(monkeypatch, capsys) -> Non ) out = capsys.readouterr().out + assert accepted is False assert "account for every open review issue exactly once" in out assert "reflect" not in plan["epic_triage_meta"]["triage_stages"] @@ -226,7 +256,7 @@ def test_reflect_preserves_observe_auto_disposition_during_fresh_persist( monkeypatch.setattr(reflect_mod, "validate_stage_report_length", lambda **_kwargs: True) monkeypatch.setattr(reflect_mod, "_validate_recurring_dimension_mentions", lambda **_kwargs: True) - reflect_mod._cmd_stage_reflect( + accepted = reflect_mod._cmd_stage_reflect( _args( report=( "## Coverage Ledger\n" @@ -239,6 +269,7 @@ def test_reflect_preserves_observe_auto_disposition_during_fresh_persist( services=services, ) + assert accepted is True dispositions = plan["epic_triage_meta"]["issue_dispositions"] assert dispositions["review::complexity::aaaa1111"]["decision"] == "cluster" assert dispositions["review::complexity::aaaa1111"]["target"] == "cluster-alpha" diff --git a/desloppify/tests/commands/plan/test_workflow_gates.py b/desloppify/tests/commands/plan/test_workflow_gates.py index 5184a8c3f..0900e6b35 100644 --- a/desloppify/tests/commands/plan/test_workflow_gates.py +++ b/desloppify/tests/commands/plan/test_workflow_gates.py @@ -16,12 +16,11 @@ import desloppify.app.commands.plan.override.misc as misc_mod import desloppify.app.commands.plan.override.resolve_cmd as resolve_mod import desloppify.app.commands.plan.override.resolve_workflow as resolve_workflow_mod -from desloppify.engine._plan.schema import empty_plan from desloppify.engine._plan.constants import ( WORKFLOW_CREATE_PLAN_ID, WORKFLOW_SCORE_CHECKPOINT_ID, ) - +from desloppify.engine._plan.schema import empty_plan # --------------------------------------------------------------------------- # Helpers @@ -101,7 +100,12 @@ def _mock_plan_io(monkeypatch, plan): monkeypatch.setattr(resolve_workflow_mod, "load_plan", lambda *a, **kw: plan) monkeypatch.setattr(misc_mod, "load_plan", lambda *a, **kw: plan) saved = [] - monkeypatch.setattr(resolve_mod, "save_plan", lambda p, *a, **kw: saved.append(p)) + monkeypatch.setattr( + resolve_mod, + "save_plan", + lambda p, *a, **kw: saved.append(p), + raising=False, + ) monkeypatch.setattr(resolve_workflow_mod, "save_plan", lambda p, *a, **kw: saved.append(p)) monkeypatch.setattr(misc_mod, "save_plan", lambda p, *a, **kw: saved.append(p)) return saved diff --git a/desloppify/tests/commands/review/test_review_batch_execution_helpers_direct.py b/desloppify/tests/commands/review/test_review_batch_execution_helpers_direct.py index 8b88b306a..fbf575808 100644 --- a/desloppify/tests/commands/review/test_review_batch_execution_helpers_direct.py +++ b/desloppify/tests/commands/review/test_review_batch_execution_helpers_direct.py @@ -203,11 +203,18 @@ def test_merge_and_finalize_helpers(tmp_path: Path, monkeypatch) -> None: colorize_fn=lambda text, _tone=None: text, ) assert merged_path.exists() - assert missing == ["missing_dim"] + assert missing == [] merged_payload = json.loads(merged_path.read_text()) assert merged_payload["review_scope"]["reviewed_files_count"] == 2 assert merged_payload["provenance"]["trusted"] is True assert merged_payload["review_quality"]["overall"] == 0.8 + assert merged_payload["assessment_coverage"] == { + "scored_dimensions": ["design_coherence", "type_safety"], + "selected_dimensions": ["design_coherence"], + "imported_dimensions": ["design_coherence"], + "missing_dimensions": ["missing_dim"], + "missing_selected_dimensions": [], + } logs: list[str] = [] args = SimpleNamespace(scan_after_import=True, path=".") @@ -228,6 +235,45 @@ def test_merge_and_finalize_helpers(tmp_path: Path, monkeypatch) -> None: assert any("run-finished" in line for line in logs) +def test_merge_reports_missing_selected_assessments_for_trusted_import( + tmp_path: Path, + monkeypatch, +) -> None: + """A selected dimension without an assessment must still block import.""" + monkeypatch.setattr(results_mod, "print_review_quality", lambda *_args, **_kwargs: None) + + merged_path, missing = results_mod.merge_and_write_results( + merge_batch_results_fn=lambda _batch_results: { + "assessments": {}, + "issues": [], + }, + build_import_provenance_fn=lambda **_kwargs: {"trusted": True}, + batch_results=[{"dummy": True}], + batches=[{"name": "design_coherence"}], + successful_indexes=[0], + packet={"dimensions": ["design_coherence"], "total_files": 10}, + packet_dimensions=["design_coherence"], + scored_dimensions=["design_coherence", "type_safety"], + scan_path=".", + runner="codex", + prompt_packet_path=tmp_path / "packet.json", + stamp="r1", + run_dir=tmp_path / "run", + safe_write_text_fn=_safe_write_text, + colorize_fn=lambda text, _tone=None: text, + ) + + assert missing == ["design_coherence"] + merged_payload = json.loads(merged_path.read_text()) + assert merged_payload["assessment_coverage"]["missing_dimensions"] == [ + "design_coherence", + "type_safety", + ] + assert merged_payload["assessment_coverage"]["missing_selected_dimensions"] == [ + "design_coherence" + ] + + def test_import_and_finalize_raises_when_followup_scan_fails(tmp_path: Path) -> None: merged_path = tmp_path / "merged.json" merged_path.write_text("{}") 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 9fb5dea7c..2bfde321b 100644 --- a/desloppify/tests/commands/review/test_review_importing_support_direct.py +++ b/desloppify/tests/commands/review/test_review_importing_support_direct.py @@ -13,8 +13,11 @@ import desloppify.app.commands.review.importing.plan_sync as plan_sync_mod import desloppify.app.commands.review.importing.results as results_mod import desloppify.engine._plan.constants as plan_constants_mod -from desloppify.engine._state.progression import append_progression_event, load_progression import desloppify.intelligence.review.importing.holistic as holistic_import_mod +from desloppify.engine._state.progression import ( + append_progression_event, + load_progression, +) from desloppify.state import empty_state as build_empty_state @@ -121,6 +124,23 @@ def test_sync_plan_after_import_no_living_plan(monkeypatch) -> None: ) +def test_protected_review_issue_ids_for_import_reads_living_plan(monkeypatch, tmp_path) -> None: + plan_path = tmp_path / "plan.json" + monkeypatch.setattr(import_cmd_mod, "plan_path_for_state", lambda _path: plan_path) + monkeypatch.setattr(import_cmd_mod, "has_living_plan", lambda _path: True) + monkeypatch.setattr( + import_cmd_mod, + "load_plan", + lambda _path: { + "epic_triage_meta": {"protected_review_issue_ids": ["review::held"]} + }, + ) + + assert import_cmd_mod._protected_review_issue_ids_for_import( + tmp_path / "state.json" + ) == {"review::held"} + + def test_sync_plan_after_import_marks_subjective_review_complete_for_current_scan( monkeypatch: pytest.MonkeyPatch, ) -> None: diff --git a/desloppify/tests/commands/test_queue_order_guard.py b/desloppify/tests/commands/test_queue_order_guard.py index 5ca38898b..e76a6add9 100644 --- a/desloppify/tests/commands/test_queue_order_guard.py +++ b/desloppify/tests/commands/test_queue_order_guard.py @@ -388,3 +388,110 @@ def test_guard_allows_front_planned_review_ids_even_if_synthetic_items_render_fi blocked = _check_queue_order_guard(state, ["review::a", "review::b"], "fixed") assert blocked is False + + +def test_guard_allows_front_planned_cluster_when_synthetic_items_render_first( + tmp_path, monkeypatch, +): + """A front planned cluster must not be blocked by an unplanned synthetic row.""" + state = { + "issues": { + "review::a": { + "id": "review::a", + "status": "open", + "detector": "review", + "file": "a.py", + "tier": 1, + "confidence": "high", + "summary": "Review issue a", + }, + "review::b": { + "id": "review::b", + "status": "open", + "detector": "review", + "file": "b.py", + "tier": 1, + "confidence": "high", + "summary": "Review issue b", + }, + }, + "scan_count": 5, + } + _setup_plan( + tmp_path, + monkeypatch, + ["review::a", "review::b"], + clusters={ + "planned-review": { + "name": "planned-review", + "auto": False, + "issue_ids": ["review::a", "review::b"], + }, + }, + ) + + import desloppify.app.commands.resolve.queue_guard as queue_guard_mod + + monkeypatch.setattr( + queue_guard_mod, + "build_work_queue", + lambda *_args, **_kwargs: { + "items": [ + {"id": "subjective::contract_coherence", "kind": "subjective_dimension"}, + ] + }, + ) + + blocked = _check_queue_order_guard(state, ["planned-review"], "fixed") + assert blocked is False + + +def test_guard_uses_configured_target_to_match_next_packet( + tmp_path, monkeypatch, +): + """A lower project target must not make resolve invent a re-review gate.""" + state = _state_with_issues("preceding", "review::a", "review::b") + _setup_plan( + tmp_path, + monkeypatch, + ["preceding", "review::a", "review::b"], + clusters={ + "planned-review": { + "name": "planned-review", + "auto": False, + "issue_ids": ["review::a", "review::b"], + }, + }, + ) + + import desloppify.app.commands.resolve.queue_guard as queue_guard_mod + + observed_targets = [] + + def queue_for_target(_state, *, options): + target = (options.context.target_strict, options.subjective_threshold) + observed_targets.append(target) + if target == (85.0, 85.0): + return { + "items": [ + {"id": "review::a", "kind": "issue"}, + {"id": "review::b", "kind": "issue"}, + ] + } + return { + "items": [ + {"id": "subjective::contract_coherence", "kind": "subjective_dimension"}, + ] + } + + monkeypatch.setattr( + queue_guard_mod, + "load_config", + lambda: {"target_strict_score": 85}, + ) + monkeypatch.setattr(queue_guard_mod, "build_work_queue", queue_for_target) + + blocked = _check_queue_order_guard(state, ["planned-review"], "fixed") + + assert blocked is False + assert observed_targets == [(85.0, 85.0)] diff --git a/desloppify/tests/commands/test_runner_modules_direct.py b/desloppify/tests/commands/test_runner_modules_direct.py index 083885ca8..3e885516c 100644 --- a/desloppify/tests/commands/test_runner_modules_direct.py +++ b/desloppify/tests/commands/test_runner_modules_direct.py @@ -7,6 +7,7 @@ import desloppify.app.commands.runner.codex_batch as codex_batch_mod import desloppify.app.commands.runner.run_logs as run_logs_mod +from desloppify.app.commands.review.runner_process_impl.types import _ExecutionResult def test_wrap_cmd_c_collapses_arguments_into_single_string() -> None: @@ -174,7 +175,7 @@ def test_codex_batch_command_uses_sanitized_reasoning_effort(monkeypatch, tmp_pa assert any(c.endswith("codex") or "codex" in c for c in command[:3]) assert "exec" in command assert "--ephemeral" in command - assert f'model_reasoning_effort="high"' in command + assert 'model_reasoning_effort="high"' in command assert str(tmp_path) in command monkeypatch.setenv("DESLOPPIFY_CODEX_REASONING_EFFORT", "invalid") @@ -183,7 +184,7 @@ def test_codex_batch_command_uses_sanitized_reasoning_effort(monkeypatch, tmp_pa repo_root=tmp_path, output_file=tmp_path / "out.json", ) - assert f'model_reasoning_effort="low"' in command + assert 'model_reasoning_effort="low"' in command def test_codex_batch_command_uses_sandbox_env_override(monkeypatch, tmp_path: Path) -> None: @@ -260,6 +261,54 @@ def fake_run_batch_attempt(**kwargs): assert sleeps == [0.25] +def test_run_codex_batch_retries_exit_zero_with_invalid_output(monkeypatch, tmp_path: Path) -> None: + """Exit-zero output validation failures consume the configured retry budget.""" + attempts: list[int] = [] + sleeps: list[float] = [] + output_file = tmp_path / "out.json" + log_file = tmp_path / "batch.log" + + def fake_run_batch_attempt(**kwargs): + attempt = kwargs["attempt"] + attempts.append(attempt) + output_file.write_text( + '{"assessments": {}' if attempt == 1 else '{"assessments": {}}', + encoding="utf-8", + ) + return ( + f"ATTEMPT {attempt}/2", + _ExecutionResult(code=0, stdout_text="", stderr_text=""), + ) + + monkeypatch.setattr(codex_batch_mod, "run_batch_attempt", fake_run_batch_attempt) + code = codex_batch_mod.run_codex_batch( + prompt="prompt", + repo_root=tmp_path, + output_file=output_file, + log_file=log_file, + deps=SimpleNamespace( + timeout_seconds=10, + max_retries=1, + retry_backoff_seconds=0.25, + live_log_interval_seconds=0.1, + stall_after_output_seconds=5, + use_popen_runner=False, + subprocess_popen=None, + validate_output_fn=None, + output_validation_grace_seconds=0.0, + output_validation_poll_seconds=0.01, + sleep_fn=sleeps.append, + safe_write_text_fn=lambda path, text: path.write_text(text, encoding="utf-8"), + ), + codex_batch_command_fn=lambda **_kwargs: ["codex", "exec"], + ) + + assert code == 0 + assert attempts == [1, 2] + assert sleeps == [0.25] + assert "output validation failed; retrying" in log_file.read_text(encoding="utf-8") + + def test_run_followup_scan_handles_force_bypass_timeout_and_oserror( capsys, tmp_path: Path, diff --git a/desloppify/tests/intelligence/test_review_import_prepare_split_direct.py b/desloppify/tests/intelligence/test_review_import_prepare_split_direct.py index 088f6b52e..8872bec7f 100644 --- a/desloppify/tests/intelligence/test_review_import_prepare_split_direct.py +++ b/desloppify/tests/intelligence/test_review_import_prepare_split_direct.py @@ -10,9 +10,9 @@ import desloppify.intelligence.review.importing.holistic_issue_flow as issue_flow_mod import desloppify.intelligence.review.importing.resolution as resolution_mod import desloppify.intelligence.review.importing.state_helpers as state_helpers_mod -import desloppify.intelligence.review.prepare_batches_core as prepare_batches_core_mod import desloppify.intelligence.review.prepare_batches_collectors_quality as collectors_quality_mod import desloppify.intelligence.review.prepare_batches_collectors_structure as collectors_structure_mod +import desloppify.intelligence.review.prepare_batches_core as prepare_batches_core_mod import desloppify.intelligence.review.prepare_holistic_batches as holistic_batches_mod import desloppify.intelligence.review.prepare_holistic_orchestration as orchestration_mod import desloppify.intelligence.review.prepare_holistic_payload_parts as payload_parts_mod @@ -171,6 +171,42 @@ def test_issue_flow_build_collect_and_auto_resolve_paths(monkeypatch) -> None: assert state["work_items"]["review::old"]["status"] == "fixed" +def test_auto_resolve_stale_holistic_preserves_explicit_review_holds() -> None: + state = { + "issues": { + "review::held": { + "id": "review::held", + "detector": "review", + "status": "open", + "detail": {"holistic": True, "dimension": "naming_quality"}, + }, + "review::ordinary": { + "id": "review::ordinary", + "detector": "review", + "status": "open", + "detail": {"holistic": True, "dimension": "naming_quality"}, + }, + } + } + diff = {"auto_resolved": 0} + + issue_flow_mod.auto_resolve_stale_holistic( + state, + new_ids=set(), + diff=diff, + utc_now_fn=lambda: "2026-03-09T00:00:00+00:00", + imported_dimensions={"naming_quality"}, + full_sweep_included=False, + preserve_open_issue_ids={"review::held"}, + ) + + assert diff["auto_resolved"] == 1 + assert state["work_items"]["review::held"]["status"] == "open" + assert "resolved_at" not in state["work_items"]["review::held"] + assert "resolution_attestation" not in state["work_items"]["review::held"] + assert state["work_items"]["review::ordinary"]["status"] == "fixed" + + def test_resolution_and_state_helper_utilities() -> None: state: dict = { "issues": { diff --git a/desloppify/tests/plan/test_auto_cluster.py b/desloppify/tests/plan/test_auto_cluster.py index 084892a76..043eed341 100644 --- a/desloppify/tests/plan/test_auto_cluster.py +++ b/desloppify/tests/plan/test_auto_cluster.py @@ -31,6 +31,7 @@ filter_cluster_focus, ) from desloppify.engine._work_queue.ranking import item_sort_key +from desloppify.engine._work_queue.snapshot import build_queue_snapshot # --------------------------------------------------------------------------- # Helpers @@ -77,6 +78,49 @@ def test_grouping_key_review(): assert key == "review::abstraction_fitness" +def test_auto_cluster_excludes_protected_review_ids_from_membership_and_queue(): + plan = empty_plan() + plan["epic_triage_meta"] = {"protected_review_issue_ids": ["r1", "r2"]} + state = _state_with( + _issue( + "r1", + "review", + detail={"dimension": "abstraction_fitness"}, + ), + _issue( + "r2", + "review", + detail={"dimension": "abstraction_fitness"}, + ), + ) + + auto_cluster_issues(plan, state) + + assert "auto/review-abstraction_fitness" not in plan["clusters"] + assert "r1" not in plan["queue_order"] + assert "r2" not in plan["queue_order"] + assert "r1" not in plan["overrides"] + assert "r2" not in plan["overrides"] + + +def test_queue_snapshot_does_not_surface_protected_review_ids(): + plan = empty_plan() + plan["epic_triage_meta"] = { + "protected_review_issue_ids": ["held"], + "triaged_ids": ["live"], + } + state = _state_with( + _issue("held", "review", detail={"dimension": "naming"}), + _issue("live", "review", detail={"dimension": "naming"}), + _issue("mechanical", "smells"), + ) + + snapshot = build_queue_snapshot(state, plan=plan) + + assert {item["id"] for item in snapshot.all_postflight_review_items} == {"live"} + assert snapshot.objective_execution_count == 0 + + def test_grouping_key_judgment_required_returns_none(): from desloppify.base.registry import DETECTORS # needs_judgment=True detectors return None (flow through review, not auto-task) diff --git a/desloppify/tests/plan/test_epic_triage.py b/desloppify/tests/plan/test_epic_triage.py index cc4b97574..f25cc3dde 100644 --- a/desloppify/tests/plan/test_epic_triage.py +++ b/desloppify/tests/plan/test_epic_triage.py @@ -3,13 +3,7 @@ from __future__ import annotations from desloppify.engine._plan.constants import TRIAGE_STAGE_IDS -from desloppify.engine._plan.triage.core import ( - DismissedIssue, - TriageResult, - apply_triage_to_plan, - collect_triage_input, - parse_triage_result, -) +from desloppify.engine._plan.policy.stale import review_issue_snapshot_hash from desloppify.engine._plan.schema import ( EPIC_PREFIX, VALID_EPIC_DIRECTIONS, @@ -17,11 +11,17 @@ empty_plan, ensure_plan_defaults, ) -from desloppify.engine._plan.policy.stale import review_issue_snapshot_hash from desloppify.engine._plan.sync.triage import ( is_triage_stale, sync_triage_needed, ) +from desloppify.engine._plan.triage.core import ( + DismissedIssue, + TriageResult, + apply_triage_to_plan, + collect_triage_input, + parse_triage_result, +) from desloppify.engine._work_queue.synthetic import build_triage_stage_items # --------------------------------------------------------------------------- @@ -668,6 +668,41 @@ def test_tracks_new_since_last(self): assert si.new_since_last == {"r2"} assert si.resolved_since_last == set() + def test_excludes_explicitly_protected_review_ids_from_prompt_input(self): + plan = empty_plan() + plan["epic_triage_meta"] = {"protected_review_issue_ids": ["r1"]} + plan["clusters"]["epic/test"] = { + "name": "epic/test", + "thesis": "test", + "direction": "delete", + "issue_ids": ["r1", "r2"], + "auto": True, + "cluster_key": "epic::epic/test", + } + plan["clusters"]["auto/review"] = { + "name": "auto/review", + "auto": True, + "issue_ids": ["r1", "u1"], + } + state = _state_with_review_issues("r1", "r2") + + si = collect_triage_input(plan, state) + + assert set(si.review_issues) == {"r2"} + assert si.existing_clusters["epic/test"]["issue_ids"] == ["r2"] + assert si.auto_clusters["auto/review"]["issue_ids"] == ["u1"] + + +class TestProtectedReviewIssueQueueScope: + def test_sync_does_not_inject_triage_for_only_protected_review_ids(self): + plan = empty_plan() + plan["epic_triage_meta"] = {"protected_review_issue_ids": ["r1"]} + + result = sync_triage_needed(plan, _state_with_review_issues("r1")) + + assert result.injected == [] + assert not any(stage_id in plan["queue_order"] for stage_id in TRIAGE_STAGE_IDS) + # --------------------------------------------------------------------------- # Parse triage result tests diff --git a/desloppify/tests/plan/test_epic_triage_apply.py b/desloppify/tests/plan/test_epic_triage_apply.py index 94a3b4af1..679c8d983 100644 --- a/desloppify/tests/plan/test_epic_triage_apply.py +++ b/desloppify/tests/plan/test_epic_triage_apply.py @@ -2,13 +2,13 @@ from __future__ import annotations +from desloppify.engine._plan.policy.stale import review_issue_snapshot_hash +from desloppify.engine._plan.schema import empty_plan from desloppify.engine._plan.triage.apply import ( TriageMutationResult, apply_triage_to_plan, ) from desloppify.engine._plan.triage.prompt import DismissedIssue, TriageResult -from desloppify.engine._plan.schema import empty_plan -from desloppify.engine._plan.policy.stale import review_issue_snapshot_hash # --------------------------------------------------------------------------- # Helpers @@ -514,6 +514,60 @@ def test_triaged_ids_recorded(self): assert sorted(plan["epic_triage_meta"]["triaged_ids"]) == ["r1", "r2"] + def test_apply_reopens_and_removes_all_automated_protected_artifacts(self): + """A protected hold cleans old automation before direct triage apply.""" + plan = empty_plan() + plan["queue_order"] = ["held", "live"] + plan["promoted_ids"] = ["held"] + plan["overrides"]["held"] = {"issue_id": "held", "cluster": "auto/review"} + plan["skipped"]["held"] = { + "issue_id": "held", + "kind": "triage_observe_auto", + } + plan["issue_dispositions"] = { + "held": {"decision_source": "observe_auto", "decision": "skip"}, + } + plan["clusters"] = { + "auto/review": { + "name": "auto/review", + "auto": True, + "issue_ids": ["held", "live"], + }, + "source-verified-review-dispositions": { + "name": "source-verified-review-dispositions", + "auto": False, + "issue_ids": ["held"], + }, + } + plan["epic_triage_meta"] = { + "protected_review_issue_ids": ["held"], + "active_triage_issue_ids": ["held", "live"], + "issue_dispositions": { + "held": {"decision_source": "observe_auto", "decision": "skip"}, + }, + } + state = _state_with_review_issues("held", "live") + state["issues"]["held"]["status"] = "false_positive" + + apply_triage_to_plan( + plan, + state, + _triage_with_epics(_epic("live-work", ["held", "live"])), + ) + + assert state["issues"]["held"]["status"] == "open" + assert "held" not in plan["skipped"] + assert "held" not in plan["issue_dispositions"] + assert "held" not in plan["queue_order"] + assert "held" not in plan["promoted_ids"] + assert "held" not in plan["overrides"] + assert "held" not in plan["clusters"]["auto/review"]["issue_ids"] + assert plan["clusters"]["source-verified-review-dispositions"]["issue_ids"] == [ + "held" + ] + assert plan["clusters"]["epic/live-work"]["issue_ids"] == ["live"] + assert "held" not in plan["epic_triage_meta"].get("issue_dispositions", {}) + def test_dismissed_ids_recorded(self): plan = empty_plan() plan["queue_order"] = ["r1", "r2"] @@ -605,4 +659,3 @@ def test_only_open_defect_issues_in_triaged_ids(self): # --------------------------------------------------------------------------- # Edge cases # --------------------------------------------------------------------------- - diff --git a/desloppify/tests/plan/test_reconcile.py b/desloppify/tests/plan/test_reconcile.py index 742ff50b0..89e92f3d8 100644 --- a/desloppify/tests/plan/test_reconcile.py +++ b/desloppify/tests/plan/test_reconcile.py @@ -146,6 +146,24 @@ def test_reconcile_no_log_when_no_changes(): assert len(reconcile_entries) == 0 +def test_reconcile_clears_protected_skip_before_status_sync(): + """A scan boundary must not turn a protected hold into triaged_out.""" + plan = empty_plan() + plan["skipped"]["held"] = { + "issue_id": "held", + "kind": "triaged_out", + } + plan["epic_triage_meta"] = { + "protected_review_issue_ids": ["held"], + } + state = _state_with_issues("held") + + reconcile_plan_after_scan(plan, state) + + assert state["issues"]["held"]["status"] == "open" + assert "held" not in plan["skipped"] + + def test_reconcile_prunes_existing_superseded_references(): """Already-superseded IDs should not linger in queue_order or clusters.""" plan = _plan_with_queue("a", "b") diff --git a/desloppify/tests/plan/test_reconcile_pipeline.py b/desloppify/tests/plan/test_reconcile_pipeline.py index d5afb53d2..d93dd52b0 100644 --- a/desloppify/tests/plan/test_reconcile_pipeline.py +++ b/desloppify/tests/plan/test_reconcile_pipeline.py @@ -30,7 +30,7 @@ LIFECYCLE_PHASE_TRIAGE_POSTFLIGHT, LIFECYCLE_PHASE_WORKFLOW_POSTFLIGHT, ) -from desloppify.engine._plan.schema import empty_plan +from desloppify.engine._plan.schema import empty_plan, executable_objective_ids from desloppify.engine._plan.sync import live_planned_queue_empty, reconcile_plan from desloppify.engine._plan.sync.pipeline import ( ReconcileResult, @@ -125,6 +125,20 @@ def test_live_planned_queue_empty_ignores_synthetic_items() -> None: assert live_planned_queue_empty(plan) is True +@pytest.mark.parametrize( + "synthetic_id", + ["subjective::design_coherence", "strategy::review-refresh"], +) +def test_executable_objective_ids_ignore_synthetic_only_queue( + synthetic_id: str, +) -> None: + plan = empty_plan() + plan["queue_order"] = [synthetic_id] + objective_ids = {"smells::src/a.py::complexity", "unused::src/b.py::symbol"} + + assert executable_objective_ids(objective_ids, plan) == objective_ids + + def test_live_planned_queue_empty_ignores_skipped_items() -> None: plan = empty_plan() plan["queue_order"] = ["unused::a"] diff --git a/desloppify/tests/plan/test_skip.py b/desloppify/tests/plan/test_skip.py index f62e9ca8c..1d4e9dcc2 100644 --- a/desloppify/tests/plan/test_skip.py +++ b/desloppify/tests/plan/test_skip.py @@ -58,6 +58,17 @@ def test_skip_temporary(): assert plan["skipped"]["b"]["kind"] == "temporary" +def test_skip_rejects_explicitly_protected_review_id(): + plan = _plan_with_queue("held") + plan["epic_triage_meta"] = {"protected_review_issue_ids": ["held"]} + + count = skip_items(plan, ["held"], kind="temporary") + + assert count == 0 + assert "held" not in plan["skipped"] + assert "held" not in plan["queue_order"] + + def test_skip_permanent(): plan = _plan_with_queue("a", "b") count = skip_items( diff --git a/desloppify/tests/plan/test_triage_snapshot_direct.py b/desloppify/tests/plan/test_triage_snapshot_direct.py index 62e394e58..2a83474ff 100644 --- a/desloppify/tests/plan/test_triage_snapshot_direct.py +++ b/desloppify/tests/plan/test_triage_snapshot_direct.py @@ -3,6 +3,7 @@ from __future__ import annotations import desloppify.engine._plan.triage.snapshot as snapshot_mod +from desloppify.engine._plan.triage.lifecycle import ensure_active_triage_issue_ids def test_normalized_issue_id_list_filters_invalid_values() -> None: @@ -47,6 +48,23 @@ def test_coverage_open_ids_falls_back_to_queue_order_before_first_scan() -> None } +def test_protected_review_ids_are_excluded_from_coverage_and_freeze() -> None: + plan = { + "epic_triage_meta": { + "protected_review_issue_ids": ["review::held", "", "review::held"], + } + } + issues = { + "review::held": {"status": "open", "detector": "review"}, + "review::live": {"status": "open", "detector": "review"}, + } + state = {"issues": issues, "work_items": issues, "last_scan": "now"} + + assert ensure_active_triage_issue_ids(plan, state) == ["review::live"] + assert plan["epic_triage_meta"]["active_triage_issue_ids"] == ["review::live"] + assert snapshot_mod.coverage_open_ids(plan, state) == {"review::live"} + + def test_manual_clusters_with_issues_and_find_cluster_for_ignore_auto_clusters() -> None: plan = { "clusters": { diff --git a/desloppify/tests/plan/test_unified_disposition_map.py b/desloppify/tests/plan/test_unified_disposition_map.py index 3b04e5b4a..f85a27576 100644 --- a/desloppify/tests/plan/test_unified_disposition_map.py +++ b/desloppify/tests/plan/test_unified_disposition_map.py @@ -2,15 +2,14 @@ from __future__ import annotations - -from desloppify.app.commands.plan.triage.stage_queue import cascade_clear_dispositions -from desloppify.app.commands.plan.triage.stages.evidence_parsing import ( - resolve_short_hash_to_full_id, -) from desloppify.app.commands.plan.triage.confirmations.basic import ( + _AUTO_SKIP_VERDICTS, _apply_observe_auto_skips, _undo_observe_auto_skips, - _AUTO_SKIP_VERDICTS, +) +from desloppify.app.commands.plan.triage.stage_queue import cascade_clear_dispositions +from desloppify.app.commands.plan.triage.stages.evidence_parsing import ( + resolve_short_hash_to_full_id, ) from desloppify.app.commands.plan.triage.validation.organize_policy import ( validate_organize_against_dispositions, @@ -25,7 +24,6 @@ skip_kind_state_status, ) - # --------------------------------------------------------------------------- # Schema: IssueDisposition exists and is well-typed # --------------------------------------------------------------------------- @@ -131,6 +129,20 @@ def test_other_stage_is_noop(self): cascade_clear_dispositions(meta, "organize") assert meta["issue_dispositions"]["id1"]["verdict"] == "genuine" + def test_observe_clears_protected_disposition_entries(self): + held = {"verdict": "genuine", "decision": "cluster", "target": "hold"} + meta = { + "protected_review_issue_ids": ["held"], + "issue_dispositions": { + "held": held, + "other": {"verdict": "genuine"}, + }, + } + + cascade_clear_dispositions(meta, "observe") + + assert meta["issue_dispositions"] == {} + # --------------------------------------------------------------------------- # Auto-skip on observe confirmation diff --git a/desloppify/tests/review/context/test_holistic_review.py b/desloppify/tests/review/context/test_holistic_review.py index 5c1894bfd..aecd70caf 100644 --- a/desloppify/tests/review/context/test_holistic_review.py +++ b/desloppify/tests/review/context/test_holistic_review.py @@ -38,7 +38,9 @@ from desloppify.intelligence.review.prepare_batches_builders import ( build_investigation_batches as _build_investigation_batches, ) -from desloppify.intelligence.review.prepare_batches_builders import filter_batches_to_dimensions +from desloppify.intelligence.review.prepare_batches_builders import ( + filter_batches_to_dimensions, +) from desloppify.state import empty_state, path_scoped_issues @@ -700,6 +702,36 @@ def test_holistic_cache_updated(self): assert rc["holistic"]["issue_count"] == 1 assert "reviewed_at" in rc["holistic"] + def test_public_facade_preserves_explicit_open_issue_ids(self): + state = empty_state() + issues_data = [ + { + "dimension": "naming_quality", + "identifier": "held_issue", + "summary": "Legacy runtime loader reads like a predicate", + "confidence": "high", + "related_files": ["src/runtime.py"], + "evidence": ["The loader name does not communicate its import side effect."], + "suggestion": "Name the loader for its runtime-loading responsibility.", + } + ] + + _call_import_holistic_issues(issues_data, state, "python") + held_id = next(iter(state["work_items"])) + + diff = _call_import_holistic_issues( + { + "issues": [], + "review_scope": {"full_sweep_included": True}, + }, + state, + "python", + preserve_open_issue_ids={held_id}, + ) + + assert diff["auto_resolved"] == 0 + assert state["work_items"][held_id]["status"] == "open" + def test_reviewed_files_refreshes_per_file_cache(self, tmp_path): state = empty_state() module_path = tmp_path / "pkg" / "module.py" @@ -1662,4 +1694,3 @@ def test_fallback_batch_created_for_missing_dimension(self): assert filtered[0]["name"] == "low_level_elegance" assert filtered[0]["dimensions"] == ["low_level_elegance"] assert "files_to_read" not in filtered[0] -