)]}'
{"watcher/decision_engine/sync.py":[{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"749cd1a707de5ce07cf84708c8fc3592cb773533","unresolved":false,"context_lines":[{"line_number":442,"context_line":"                        action_plan.id"},{"line_number":443,"context_line":"                    ].state \u003d objects.action_plan.State.CANCELLED"},{"line_number":444,"context_line":""},{"line_number":445,"context_line":"    def _find_stale_action_plans_due_to_goal_for_terminated_audits(self):"},{"line_number":446,"context_line":"        # There may be non-terminated ActionPlans for terminated Audits created"},{"line_number":447,"context_line":"        # from the updated goal. We need to cancel them."},{"line_number":448,"context_line":"        for goal_id, synced_goal in self.goal_mapping.items():"}],"source_content_type":"text/x-python","patch_set":1,"id":"8c56db7e_a7e709d8","line":445,"updated":"2026-11-12 16:52:12.000000000","message":"_find_stale_action_plans_due_to_goal_for_terminated_audits is the only path that cancels leftover non-terminal action plans whose audits are already terminated when a goal is updated. No test exercises it in isolation: in test_end2end_sync_goals_with_modified_goal_and_strategy every action plan it cancels (ap_term_recommended, ap_term_pending, ap_term_ongoing, ap_term_cancelling, ap_on_stale_audit, ap_both) also references strategy2/3/4, which sync always recreates when their goal changes, so _find_stale_action_plans_due_to_strategy already cancels them; in test_end2end_sync_goals_with_removed_goal_and_strategy the method is never invoked because goal_mapping stays empty when a goal is removed rather than modified. Deleting the method, or inverting/typo-ing its outer \u0027state__notin\u0027 audit filter, would leave every assertion in the suite passing.\n\n**Severity**: WARNING | **Confidence**: 0.85\n\n**Impact**: A future regression that silently disables this path (removed method, wrong goal_id filter, or inverted outer state filter) leaves stale RECOMMENDED/PENDING action plans of SUCCEEDED/FAILED audits uncancelled on goal changes, quietly reintroducing the leftover-stale-plan behavior fixed here for bug #2174051, with no CI signal. This path is also the only one that catches action plans whose strategy_id points to a strategy generation superseded by an earlier sync, so such plans would linger indefinitely referencing soft-deleted strategies.\n\n**Suggestion**:\nIn test_end2end_sync_goals_with_modified_goal_and_strategy, add a RECOMMENDED action plan attached to a terminal audit (e.g., audit_goal_succeeded) whose strategy_id references an unmodified strategy (strategy1) so neither the modified-strategy path nor the stale-audit path can cancel it, and assert it appears in modified_action_plans with state CANCELLED; also assert a terminal action plan on the same audit remains untouched.","commit_id":"6dd0132b0f573dffc18d3a63ba78f14a41e42468"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"96bd77be3f02f51871e2edb2e18ffa7cb22cb726","unresolved":false,"context_lines":[{"line_number":442,"context_line":"                        action_plan.id"},{"line_number":443,"context_line":"                    ].state \u003d objects.action_plan.State.CANCELLED"},{"line_number":444,"context_line":""},{"line_number":445,"context_line":"    def _find_stale_action_plans_due_to_goal_for_terminated_audits(self):"},{"line_number":446,"context_line":"        # There may be non-terminated ActionPlans for terminated Audits created"},{"line_number":447,"context_line":"        # from the updated goal. We need to cancel them."},{"line_number":448,"context_line":"        for goal_id, synced_goal in self.goal_mapping.items():"}],"source_content_type":"text/x-python","patch_set":1,"id":"7e37c1c8_5170d377","line":445,"in_reply_to":"8c56db7e_a7e709d8","updated":"2026-11-13 08:46:18.000000000","message":"The case you mentioned is not possible as strategy1 is attached to goal1 which is unmodified. I just realized an unmodified strategy attached to an modified goal will actually be mark as stale and be modified to update the goal.uuid, so actually that case will be handled by the _find_stale_action_plans_due_to_strategy method. _find_stale_action_plans_due_to_goal_for_terminated_audits is not needed.","commit_id":"6dd0132b0f573dffc18d3a63ba78f14a41e42468"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"54da5abb6e082f8e4ae7e3eef31ee2c22de966bf","unresolved":false,"context_lines":[{"line_number":423,"context_line":"        # because its related Audit has been set to stale as it was in"},{"line_number":424,"context_line":"        # non-terminal state when the strategy or goal was updated."},{"line_number":425,"context_line":"        for audit_id, synced_audit in self.stale_audits_map.items():"},{"line_number":426,"context_line":"            filters \u003d {"},{"line_number":427,"context_line":"                \"audit_id\": audit_id,"},{"line_number":428,"context_line":"                \u0027state__in\u0027: objects.action_plan.State.NON_TERMINAL_STATES,"},{"line_number":429,"context_line":"            }"}],"source_content_type":"text/x-python","patch_set":2,"id":"c3aad47b_9489e10d","line":426,"updated":"2026-11-13 09:28:55.000000000","message":"The new state__in filter in _find_stale_action_plans_due_to_audit restricts cancellation to non-terminal action plans, but no test exercises this path in isolation. In both end-to-end sync tests, every non-terminal action plan whose audit is in stale_audits_map also references a modified or removed strategy, so _find_stale_action_plans_due_to_strategy or _soft_delete_removed_strategies cancels it first. The comment on ap_on_stale_audit claims it covers the audit path, but that plan\u0027s strategy (strategy2) is also modified, so the strategy pass cancels it regardless.\n\n**Severity**: SUGGESTION | **Confidence**: 0.85\n\n**Impact**: The exact defect class this change fixes (stale non-terminal action plans left runnable) could silently regress on this path, for example by referencing the wrong State constant in one of the seven near-identical filter dicts, and CI would not detect it, weakening the protection the change is meant to add for bug 2174051.\n\n**Recommendation**:\nAdd a scenario isolating the audit path in test_end2end_sync_goals_with_modified_goal_and_strategy: create a PENDING audit tied to the modified goal (goal2) but the unchanged strategy (strategy1), plus a RECOMMENDED action plan referencing that audit, run sync(), and assert the plan state is CANCELLED. Because its strategy is unchanged, only the due-to-audit pass can cancel it, and a RECOMMENDED state also pins the correct State constant.","commit_id":"82f6e19e195a6730594a8db40165f5a7ac0d090a"}]}
