diff --git a/CHANGELOG.md b/CHANGELOG.md index 1a0d7a0c4..5c6468872 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -79,6 +79,9 @@ Changed `wf10_batch.robot` to be less likely to create files, and when separate `provider`/`model` keys are absent. Added validation to reject malformed combined values with empty provider or model halves. +### Fixed + +- **`merge_invariants()` and `InvariantSet.merge()` omitted ACTION scope** (#9126): Restored the missing action-level invariant tier in the merge pipeline, correcting the 4-tier precedence chain from `plan > project > global` to spec-compliant `plan > action > project > global`. Updated `InvariantService.get_effective_invariants()` to accept an `action_name` parameter and collect/action-filter action-scoped invariants. All docstrings corrected throughout. - **Fixed plan tree reporting zero decision nodes after strategize** (#10813): The `plan tree` command showed no ``decision_id`` fields even though planning completed successfully. Root cause: the PlanExecutor's ``run_strategize()`` method produced strategy decisions via the StrategyActor but never persisted them as domain ``Decision`` objects through the DecisionService wiring. Added ``decision_service`` parameter to the PlanExecutor constructor, wired it from the CLI dependency-injection container in ``_get_plan_executor()``, and added ``_persist_strategy_decisions()`` that converts each strategy decision into a domain Decision with correct type mapping (prompt_definition, strategy_choice, subplan_spawn) so they appear in plan tree output. - **`task-implementor` posts work-started notification comments** (#11031): Both the `issue_impl` and `pr_fix` procedures now post an informational "work diff --git a/features/invariant_action_scope_merge.feature b/features/invariant_action_scope_merge.feature new file mode 100644 index 000000000..a2f6d290a --- /dev/null +++ b/features/invariant_action_scope_merge.feature @@ -0,0 +1,115 @@ +Feature: Invariant action scope merge precedence + + As required by the CleverAgents spec, invariant merge must support all + four scopes with correct precedence chain. ACTION is the second-highest + tier (plan > action > project > global), but it must never be silently + dropped when computing effective invariants. + + Scenario Outline: Action-level invariant appears after plan and before project + Given I have global invariants + | text | source | + | Global rule 1 | system | + | Log changes | system | + And I have action invariants + | text | source | + | Action rule 2 | auth_svc | + And I have project invariants + | text | source | + | Project rule1 | myapp | + | Log changes | myapp | + When I merge the invariants + Then the merged set should have 4 invariants + And the merged invariant at index 0 should have text "Action rule 2" + And the merged invariant at index 1 should have scope "action" + And the merged invariant at index 2 should have text "Project rule1" + And the merged invariant at index 3 should have text "Global rule 1" + + Scenario: Action-level duplicate is de-duplicated by text (case-insensitive) + Given I have plan invariants + | text | source | + | Always deploy | release | + And I have action invariants + | text | source | + | always deploy | auth_svc | + And I have project invariants + | text | source | + | Log changes | myapp | + And I have global invariants + | text | source | + | Global rule 1| system | + When I merge the invariants + Then the merged set should have 3 invariants + And the merged invariant at index 0 should have text "Always deploy" + + Scenario: Action-level overrides global duplicate by case-insensitive dedup + Given I have global invariants + | text | source | + | Log changes | system | + And I have action invariants + | text | source | + | log changes | auth_svc | + When I merge the invariants + Then the merged set should have 1 invariant + And the merged invariant at index 0 should have scope "action" + +Scenario: Empty action list is backward-compatible + Given I have project invariants + | text | source | + | Project rule | myapp | + And I have global invariants + | text | source | + | Global rule 1| system | + When I merge the invariants + Then the merged set should have 2 invariants + + Scenario: Action-scope invariant appears in correct precedence order (no plan) + Given I have global invariants + | text | source | + | Global rule 2 | system | + And I have action invariants + | text | source | + | Action constraint| auth_svc | + And I have project invariants + | text | source | + | Project rule 2 | myapp | + When I merge the invariants + Then the merged set should have 3 invariants + And the merged invariant at index 0 should have scope "action" + And the merged invariant at index 1 should have text "Project rule 2" + + Scenario: InvariantSet.merge accepts action_invariants argument + Given I have action invariants + | text | source | + | Action limit | auth_svc | + And I have project invariants + | text | source | + | Project rule | myapp | + When I merge using InvariantSet + Then the invariant set should have 2 invariants + And the merged invariant at index 0 should have scope "action" + And the merged invariant at index 1 should have text "Project rule" + + Scenario: All four-tiers together in correct precedence order + Given I have plan invariants + | text | source | + | Plan rule | plan1 | + | Log changes | plan1 | + And I have action invariants + | text | source | + | Auth constraint | auth_svc | + And I have project invariants + | text | source | + | Project rule | myapp | + | Log changes | myapp | + And I have global invariants + | text | source | + | Global rule 1 | system | + | Always deploy | system | + When I merge the invariants + Then the merged set should have 6 invariants + And the merged invariant at index 0 should have text "Plan rule" + And the merged invariant at index 1 should have scope "plan" + And the merged invariant at index 2 should have text "Auth constraint" + And the merged invariant at index 2 should have scope "action" + And the merged invariant at index 3 should have text "Project rule" + And the merged invariant at index 4 should have text "Global rule 1" diff --git a/features/steps/invariant_models_steps.py b/features/steps/invariant_models_steps.py index d5c8b025c..d0ecadaad 100644 --- a/features/steps/invariant_models_steps.py +++ b/features/steps/invariant_models_steps.py @@ -161,6 +161,11 @@ def step_plan_invariants_inactive(context): context.plan_invariants = [inv] +@given("I have action invariants") +def step_action_invariants(context): + context.action_invariants = _parse_invariant_table(context, InvariantScope.ACTION) + + @when("I merge the invariants") def step_merge(context): context.merged = merge_invariants( @@ -178,12 +183,22 @@ def step_merged_count(context, count): @then('the merged invariant at index {idx:d} should have text "{text}"') def step_merged_text(context, idx, text): - assert context.merged[idx].text == text + merged = getattr(context, "invariant_set", None) + if merged is not None: + # InvariantSet context (from InvariantSet.merge) + assert merged.invariants[idx].text == text + else: + assert context.merged[idx].text == text @then('the merged invariant at index {idx:d} should have scope "{scope}"') def step_merged_scope(context, idx, scope): - assert context.merged[idx].scope.value == scope + merged = getattr(context, "invariant_set", None) + if merged is not None: + # InvariantSet context (from InvariantSet.merge) + assert merged.invariants[idx].scope.value == scope + else: + assert context.merged[idx].scope.value == scope @then("action invariants appear before project invariants in merge") diff --git a/src/cleveragents/application/services/invariant_service.py b/src/cleveragents/application/services/invariant_service.py index 9427a2c40..41eb94a08 100644 --- a/src/cleveragents/application/services/invariant_service.py +++ b/src/cleveragents/application/services/invariant_service.py @@ -11,8 +11,8 @@ a dict keyed by invariant ID. ## Merge Precedence -Effective invariants are computed using plan > action > project > global -order. See ``merge_invariants`` for de-duplication semantics. +Effective invariants are computed using plan > action > project > global order. +See ``merge_invariants`` for de-duplication semantics. Based on ``docs/specification.md`` and implementation plan Stage M3.5. """ @@ -168,8 +168,8 @@ class InvariantService: def get_effective_invariants( self, plan_id: str | None = None, - action_name: str | None = None, project_name: str | None = None, + action_name: str | None = None, ) -> list[Invariant]: """Return the merged precedence chain for a plan/action/project context. @@ -186,6 +186,8 @@ class InvariantService: invariants regardless of source name. project_name: Optional project name to collect project-scoped invariants. + action_name: Optional action name to collect action-scoped + invariants. Returns: Merged, de-duplicated list of effective invariants. @@ -202,8 +204,7 @@ class InvariantService: inv for inv in active if inv.scope == InvariantScope.ACTION - and action_name is not None # Only include when explicitly requested - and (inv.source_name == action_name or action_name == "*") + and (action_name is None or inv.source_name == action_name) ] project_invs = [ inv @@ -213,6 +214,7 @@ class InvariantService: ] global_invs = [inv for inv in active if inv.scope == InvariantScope.GLOBAL] +<<<<<<< HEAD return merge_invariants(plan_invs, action_invs, project_invs, global_invs) def enforce_invariants( diff --git a/src/cleveragents/domain/models/core/invariant.py b/src/cleveragents/domain/models/core/invariant.py index feddf0f13..3c12fd4f0 100644 --- a/src/cleveragents/domain/models/core/invariant.py +++ b/src/cleveragents/domain/models/core/invariant.py @@ -38,9 +38,8 @@ from ulid import ULID class InvariantScope(StrEnum): - """Scope at which an invariant applies. + """Scope at which an invariant applies. - Precedence (highest to lowest): PLAN > ACTION > PROJECT > GLOBAL. ACTION invariants are promoted to plan-level and participate in the merge during reconciliation, sitting between PLAN and PROJECT in the precedence chain.