refactor(test-writing): correct unapplicable rules and scope fix remediation

README.md

@@ -69,7 +69,7 @@ If you previously installed `gh-tooling` from this marketplace, Claude Code drop
 
 ### test-writing
 
-Generates and validates PHPUnit unit tests for Shopware 6. Analyzes source classes, detects the test category (DTO, Service, Flow/Event, DAL, Exception), generates tests, reviews them against 44 Shopware-specific rules, and iterates fixes until they pass. Also supports team-based consensus review running as a multi-agent Claude Code Workflow (experimental) — one read-only reviewer for unit, integration, and migration tests, with cross-cutting coverage and integration-to-unit placement flags.
+Generates and validates PHPUnit unit tests for Shopware 6. Analyzes source classes, detects the test category (DTO, Service, Flow/Event, DAL, Exception), generates tests, reviews them against the 43 rules of the Shopware unit-test catalog, and iterates fixes until they pass. Also supports team-based consensus review running as a multi-agent Claude Code Workflow (experimental) — one read-only reviewer for unit, integration, and migration tests, with cross-cutting coverage and integration-to-unit placement flags.
 
 ```bash
 /plugin install test-writing@shopware-ai-coding-tools

plugin-tests/test-writing/build_rule_package.bats

@@ -265,7 +265,7 @@ _scoped_render() {
     # Inspect only rule-header lines (# ID — Title); bodies never start that way.
     run grep -E '^# [A-Z]+-[0-9]+ ' "${pkg}"
     assert_success
-    assert_line --partial "CONV-007"     # class-structure
+    assert_line --partial "CONV-005"     # class-structure
     assert_line --partial "DESIGN-004"   # class-bodies
     refute_line --partial "CONV-001"     # method — outside the requested set
 }
@@ -302,8 +302,8 @@ _scoped_render() {
 
     run grep -E '^# [A-Z]+-[0-9]+ ' "${pkg}"
     assert_success
-    refute_line --partial "CONV-007"     # scoped-review=exclude
     refute_line --partial "CONV-005"     # scoped-review=exclude
+    refute_line --partial "CONV-017"     # scoped-review=exclude
     assert_line --partial "CONV-001"     # scoped-review=include — kept
 }
 
@@ -438,10 +438,10 @@ _catalog_ids() {
     pkg="$(_pkg_path)"
 
     # Every convention rule declares test-types: all, so the composed migration
-    # catalog carries all sixteen — named, so a rule that silently stops
+    # catalog carries all fifteen — named, so a rule that silently stops
     # reaching migration tests fails here rather than passing under a count.
     assert_equal "$(_catalog_ids "${pkg}" CONV)" "$(printf '%s\n' \
-        CONV-001 CONV-002 CONV-003 CONV-004 CONV-005 CONV-006 CONV-007 CONV-008 \
+        CONV-001 CONV-002 CONV-003 CONV-004 CONV-005 CONV-006 CONV-008 \
         CONV-009 CONV-010 CONV-011 CONV-012 CONV-013 CONV-014 CONV-016 \
         CONV-017)"
 

plugin-tests/test-writing/review_unit.bats

@@ -27,7 +27,7 @@ _assert_indexed_review_unit() {
 }
 
 @test "_build_rule_index parses a class-structure rule" {
-    _assert_indexed_review_unit CONV-007 class-structure
+    _assert_indexed_review_unit CONV-005 class-structure
 }
 
 @test "_build_rule_index parses a class-bodies rule" {
@@ -84,15 +84,14 @@ _assert_indexed_review_unit() {
     run _filter_rules "" "" "" "" "" "" "class-bodies"
     assert_success
     refute_line "CONV-001"    # method
-    refute_line "CONV-007"    # class-structure (proves exact match, not prefix)
+    refute_line "CONV-005"    # class-structure (proves exact match, not prefix)
 }
 
 @test "review_unit filter composes with group filter" {
     _build_rule_index "${RULES_DIR}"
     run _filter_rules "convention" "" "" "" "" "" "class-structure"
     assert_success
     assert_line "CONV-005"
-    assert_line "CONV-007"
     refute_line "MIGRATION-008"    # class-structure but not in the convention group
 }
 
@@ -102,7 +101,7 @@ _assert_indexed_review_unit() {
 
 @test "get_rules exposes review-unit in the per-rule metadata header" {
     _build_rule_index "${RULES_DIR}"
-    run tool_get_rules '{"ids":"CONV-007"}'
+    run tool_get_rules '{"ids":"CONV-005"}'
     assert_success
     assert_output --partial "Review unit: class-structure"
 }
@@ -218,7 +217,7 @@ _assert_indexed_scoped_review() {
 }
 
 @test "_build_rule_index parses a scoped-review=exclude rule" {
-    _assert_indexed_scoped_review CONV-007 exclude
+    _assert_indexed_scoped_review CONV-005 exclude
 }
 
 @test "_build_rule_index parses a scoped-review=include rule" {
@@ -253,7 +252,7 @@ _assert_indexed_scoped_review() {
     scoped="$(_filter_rules "" "" "" "" "" "true" "" | sort)"
     full="$(_filter_rules "" "" "" "" "" "" "" | sort)"
     excluded="$(comm -23 <(printf '%s\n' "${full}") <(printf '%s\n' "${scoped}"))"
-    assert_equal "${excluded}" "$(printf 'CONV-005\nCONV-007\nCONV-017\nDESIGN-006\nINTEGRATION-008')"
+    assert_equal "${excluded}" "$(printf 'CONV-005\nCONV-017\nDESIGN-006\nINTEGRATION-008')"
 }
 
 @test "scoped_review=true keeps scoped-review=include rules in the result" {
@@ -270,7 +269,7 @@ _assert_indexed_scoped_review() {
     run _filter_rules "" "" "" "" "" "" ""
     assert_success
     assert_line "CONV-005"
-    assert_line "CONV-007"
+    assert_line "CONV-017"
 }
 
 # ============================================================================
@@ -279,7 +278,7 @@ _assert_indexed_scoped_review() {
 
 @test "get_rules exposes scoped-review=exclude in the per-rule metadata header" {
     _build_rule_index "${RULES_DIR}"
-    run tool_get_rules '{"ids":"CONV-007"}'
+    run tool_get_rules '{"ids":"CONV-005"}'
     assert_success
     assert_output --partial "Scoped review: exclude"
 }

plugins/test-writing/.claude-plugin/plugin.json

@@ -1,6 +1,6 @@
 {
   "name": "test-writing",
-  "version": "5.1.1",
+  "version": "5.2.0",
   "description": "Generate and validate PHPUnit tests for Shopware 6. Unit tests: analyzes source classes to detect category (DTO, Service, Flow/Event, DAL, Exception) and applies category-specific templates. Migration tests: analyzes SQL operations to generate pattern-appropriate migration tests with 8 migration-specific rules. Integration tests: generates wired-up tests via phpunit-integration-test-generation for controller, message-handler, indexer, DAL-flow, and multi-service patterns (defers to unit generation when the SUT is unit-shape); 8 quality rules plus a placement smoke check via phpunit-integration-test-reviewing; a separate user-invoked phpunit-integration-to-unit-migrating skill audits placement with 8 deep-reasoning rules and migrates load-bearing-free tests to the unit suite. Workflow-based team review: wave-orchestrated agents coordinated through a shared blackboard (no agent-to-agent messaging), with adversarial red team and defense rounds. Bundles test-rules MCP server for Shopware compliance rules. Orchestrator runs inline fix loop (max 4 iterations) with oscillation detection. Optional dev-tooling plugin enables PHPStan/PHPUnit/ECS validation in the fix loop.",
   "author": {
     "name": "Shopware Labs"

plugins/test-writing/AGENTS.md

@@ -40,12 +40,12 @@ plugins/test-writing/
 │   ├── test-reviewer.md
 │   └── test-adversary.md
 ├── rules/
-│   ├── convention/CONV-{001..014,016,017}.md
+│   ├── convention/CONV-{001..006,008..014,016,017}.md
 │   ├── design/DESIGN-{001..010}.md
 │   ├── isolation/ISOLATION-{001..006}.md
 │   ├── provider/PROVIDER-{001..005}.md
 │   ├── unit/UNIT-{001,003,004,007..010}.md
-│   ├── migration/MIGRATION-{001..009}.md
+│   ├── migration/MIGRATION-{001..006,008,009}.md
 │   ├── integration/INTEGRATION-{001..008}.md
 │   └── placement/PLACEMENT-{001..008}.md
 ├── mcp-server-test-rules/

plugins/test-writing/CHANGELOG.md

@@ -5,6 +5,27 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## [5.2.0] - 2026-09-05
+
+### Removed
+- **CONV-007 (test class member ordering) and MIGRATION-007 (assertSame over assertEquals in migration tests).** Both duplicate deterministic gates verified active in the target repository: php-cs-fixer's `ordered_class_elements` runs over `tests/` and its default group order covers all six of CONV-007's member slots (the `phpunit` group places setUp/tearDown ahead of test methods, and a `public static` provider sorts before private helpers), and phpstan-phpunit's `AssertEqualsIsDiscouragedRule` is active on every analyzed path including `tests/migration`. Same pattern as the CONV-015/UNIT-002 removals in 5.0.0. The unit-review catalog drops from 44 rules to 43.
+
+### Fixed
+- **UNIT-004 no longer prescribes remediations the PHPStan gate rejects.** A shared `createMock()` double whose sibling methods set expectations is exempt from the drop-`expects()` remedy (`NoCreateMockWithoutExpectationsRule` requires every method touching such a double to carry an expectation), UNIT-004 and UNIT-003 defer to that rule where it already reports a double, a cross-method `never()`/`once()` reachability pairing counts as the assertion that pins which branch ran, and the remediation notes quote the Shopware guideline's two stub-conversion gotchas (`->with()` exists on mocks only; a fixture-helper parameter takes `Foo&Stub`, not `MockObject`) that pass PHPUnit and fail only in PHPStan.
+- **CONV-014 no longer relocates position-load-bearing assertions.** An assertion between two actions is intentional when moving it past the next action changes the observed value — reported at most as `consider`, with no relocation suggested — and the rule's own Fix example no longer moves an `assertCount` past a call that could mutate the counted collection.
+- **DESIGN-003 detection compares assertions, not bodies.** Merging is safe only when neither test asserts the distinguishing detail, and a fold into an existing provider names that provider and respects its documented scope.
+- **DESIGN-004 keeps boundary inputs whose distinctness the implementation deliberately erases** — the equivalence between them is itself the contract under test.
+- **CONV-011 suppresses annotations that mechanically expand the method name**, decided by a deterministic camelCase-split comparison run before the finding is emitted.
+- **The migration reviewing skill's `warnings` example matches CONV-014's semantics**: reorder-only, with empty `deleted_methods`/`removed_assertions`.
+
+### Changed
+- **CONV-005 is a `consider` rule with a single-category exemption.** A class deliberately holding one half of a split (for example only the error cases, with the happy paths in a sibling class) is no longer told to grow the categories it does not contain.
+- **UNIT-001 findings name the enforcing mechanism** — return type, PHPStan level, constructor contract, or a named rule such as `NoCreateMockWithoutExpectationsRule` — so a partially wrong triviality premise is checkable before any assertion is removed.
+- **CONV-017 reconciles with the guideline's `setUp()` stance.** State reused across test methods stays in `setUp()`; the identity/accumulated-setup exemption applies only when the property is read more than once inside its single referencing method and those reads must observe the same instance.
+
+### Added
+- **Remediation invariant** in the three reviewing skills' output requirements and the four team-review prompt builders (reviewer, reconcile, red team, defense): a suggested fix never changes what an existing assertion pins and never introduces an assertion as a means of satisfying a structural, layout, naming, or style constraint; coverage-gap rules (MIGRATION-008, DESIGN-006, INTEGRATION-007) carry new assertions as the finding's explicit deliverable; deletions ride `removed_assertions`; re-expressing the same pinned fact (CONV-012's `assertSame` conversion) stays legal.
+
 ## [5.1.1] - 2026-09-05
 
 ### Fixed

plugins/test-writing/README.md

@@ -17,7 +17,7 @@ Generate and validate PHPUnit unit tests for Shopware 6. Automatically analyzes
 - **MCP Rule Server**: Dynamic rule discovery with `mcp__plugin_test-writing_test-rules__get_rules` for context-efficient reviews
 - **Team-Based Consensus Review**: The single Workflow-based reviewer for unit, integration, and migration tests over one mixed manifest — `test_type` (resolved by path) routes each file to its rule catalog, per-type reviewing sub-skill, decomposition track, and adversary lenses. 3 independent reviewers per unit and K independent per-file adversaries (one per lens — tautology / weak-assertion / missed-coverage — each reading a single file). Oversized test classes are decomposed by rule track — method-shards plus a whole-class or body-free structural-digest track — so large files no longer overflow the context window, and large changesets are partitioned into review shards that run as a **campaign of sequential workflow launches** with every stage result persisted to disk, so an interrupted campaign resumes from where it stopped. Stages: independent review + peer reconciliation per shard (consensus), a whole-changeset signals run (cross-file consistency, adoption), and a cost-gated adversarial run (red team, defense, hard-capped arbitration). Deterministic cross-cutting SUT-coverage map and informational integration-to-unit placement flags computed at merge. Findings carry a method-primary locator, a per-finding branch-scope flag (`branch_touched`) on diff runs, a source-change escalation when a fix cannot be made in the test alone, and deletion accounting (`deleted_methods`, `removed_assertions`) naming what a remediation removes. A finding is identified as `rule_id|method`, so reviewers describing one defect differently pool their votes instead of fragmenting into contested singletons. 2-of-3 majority consensus per track, and a review unit that comes back with fewer than two live reviewer stances fails its shard rather than reporting a clean pass. Strictly read-only (see [Team Review](#team-review) below)
 - **Migration Test Generation**: Analyzes migration source classes (SQL operations, updateDestructive logic) to generate pattern-appropriate migration tests
-- **Migration Test Reviewing**: 9 migration-specific rules covering idempotency, cleanup, assertion patterns, and Shopware conventions
+- **Migration Test Reviewing**: 8 migration-specific rules covering idempotency, cleanup, assertion patterns, and Shopware conventions
 - **Integration Test Generation**: Analyzes source classes to detect supported integration patterns (controller/route, message-handler, indexer, DAL-flow, multi-service) and generates `IntegrationTestBehaviour`-based tests. Defers to unit test generation when the SUT is unit-shape
 - **Integration Test Reviewing**: 8 integration-specific rules covering integration-base usage, real-collaborator policy, transactional cleanup, determinism, independence, and a placement smoke check
 - **Integration-to-Unit Migration**: User-invoked audit workflow that walks 8 placement-reasoning rules per test, buckets into migrate/split/keep/delete, and applies one of 6 codified refactoring patterns. Separate skill, never auto-invoked
@@ -137,7 +137,6 @@ The generator analyzes the migration's SQL operations and selects appropriate te
 | MIGRATION-004 | Test-created tables/data not cleaned up                                                        |
 | MIGRATION-005 | Multiple SQL in single try/catch, or catching Exception instead of Throwable                   |
 | MIGRATION-006 | String interpolation for table/column names in SQL                                             |
-| MIGRATION-007 | assertEquals used instead of assertSame                                                        |
 | MIGRATION-008 | Missing testGetCreationTimestamp method                                                        |
 | MIGRATION-009 | setUp/tearDown mutates DB state                                                                |
 
@@ -270,7 +269,6 @@ Rules are organized by group and enforce level.
 | DESIGN-004    | Test redundancy (unjustified cases or methods covering same path)                                                     |
 | CONV-006      | TestDox phrasing doesn't follow guidelines                                                                            |
 | UNIT-003      | Over-mocking (should use StaticEntityRepository or real impl)                                                         |
-| CONV-007      | Test class structure order incorrect                                                                                  |
 | CONV-008      | Exception expectation set after throwing call                                                                         |
 | ISOLATION-001 | Shared mutable state between tests (FIRST: Independent)                                                               |
 | ISOLATION-002 | Non-deterministic inputs without mocking (FIRST: Repeatable)                                                          |
@@ -295,7 +293,6 @@ Rules are organized by group and enforce level.
 | DESIGN-006    | Unbalanced coverage distribution (< 20% edge+error cases)                                                      |
 | CONV-014      | Unclear AAA structure (assertions interspersed with setup)                                                     |
 | ISOLATION-004 | Opaque test data identifiers (UUID hex strings instead of descriptive strings like `'product-id'`)             |
-| CONV-005      | Test method ordering doesn't follow pattern                                                                    |
 | PROVIDER-003  | Data provider uses `return []` instead of `yield`/`iterable`                                                   |
 | CONV-017      | Single-use test property (assigned in `setUp()`, used in only one test method — inline it)                     |
 | CONV-016      | `Test` prefix on non-test helper class (reserve `Test` for classes extending `TestCase`; use `Stub*`, `Fake*`) |
@@ -313,6 +310,7 @@ Rules are organized by group and enforce level.
 | DESIGN-008    | Potential preservation value in redundant test (regression/bug documentation)                                          |
 | ISOLATION-006 | Consider real fixture files for file I/O testing                                                                       |
 | DESIGN-009    | Duplicated inline Arrange code (identical construction in multiple test methods; extract to setUp() or private helper) |
+| CONV-005      | Test method ordering doesn't follow pattern                                                                            |
 
 ### Migration Rules (Must-Fix)
 
@@ -324,7 +322,6 @@ Rules are organized by group and enforce level.
 | MIGRATION-004 | Test-created tables/data not cleaned up                                                        |
 | MIGRATION-005 | Multiple SQL in single try/catch, or catching Exception instead of Throwable                   |
 | MIGRATION-006 | String interpolation for table/column names in SQL                                             |
-| MIGRATION-007 | assertEquals used instead of assertSame                                                        |
 | MIGRATION-008 | Missing testGetCreationTimestamp method                                                        |
 | MIGRATION-009 | setUp/tearDown mutates DB state                                                                |
 
@@ -517,12 +514,12 @@ Reference files provide detailed guidance:
 ### Rule Files
 
 Individual rule files are in `rules/` organized by group:
-- `rules/convention/` — PHPUnit and Shopware coding conventions (CONV-001 through CONV-017, less CONV-015)
+- `rules/convention/` — PHPUnit and Shopware coding conventions (CONV-001..006, CONV-008..014, CONV-016, CONV-017)
 - `rules/design/` — Test design principles (DESIGN-001 through DESIGN-010)
 - `rules/isolation/` — Test independence and isolation (ISOLATION-001 through ISOLATION-006)
 - `rules/provider/` — Data provider patterns (PROVIDER-001 through PROVIDER-005)
 - `rules/unit/` — Unit test-specific rules (UNIT-001 through UNIT-010, less UNIT-002)
-- `rules/migration/` — Migration test rules (MIGRATION-001 through MIGRATION-009)
+- `rules/migration/` — Migration test rules (MIGRATION-001..006, MIGRATION-008, MIGRATION-009)
 - `rules/integration/` — Integration test rules (INTEGRATION-001 through INTEGRATION-008)
 - `rules/placement/` — Placement reasoning prompts (PLACEMENT-001 through PLACEMENT-008)