diff --git a/conductor/directives/adapt_test_mocks_to_production_api_change/meta.md b/conductor/directives/adapt_test_mocks_to_production_api_change/meta.md new file mode 100644 index 00000000..fb92d7f5 --- /dev/null +++ b/conductor/directives/adapt_test_mocks_to_production_api_change/meta.md @@ -0,0 +1,7 @@ +# adapt_test_mocks_to_production_api_change + +## v1 + +**Why this iteration:** Lifted verbatim from `docs/superpowers/plans/2026-06-05-regression-fixes.md` Task 1 (the `C_LBL` theme-caused regression fix; the explicit "Per AGENTS.md: DO NOT CREATE MOCK PATCHES..." citation). +**Source:** `docs/superpowers/plans/2026-06-05-regression-fixes.md` §A.Theme (Task 1, Step 1.3; "Fix: Adapt the test...") +**Lifted:** 2026-07-03 scavenge sweep batch 3/5: docs/superpowers/plans/ diff --git a/conductor/directives/adapt_test_mocks_to_production_api_change/v1.md b/conductor/directives/adapt_test_mocks_to_production_api_change/v1.md new file mode 100644 index 00000000..1bc9d317 --- /dev/null +++ b/conductor/directives/adapt_test_mocks_to_production_api_change/v1.md @@ -0,0 +1,11 @@ +# Adapt tests when production code changes its public API; never patch around new APIs with mock stubs + +When a refactor changes a production function signature or shape (e.g. a module-level `C_LBL: ImVec4` becomes a `def C_LBL() -> ImVec4` callable), the test must be adapted to the new contract, not bent to keep working with old mocks. + +**The wrong move:** Add a wrapping mock that converts the new return type to the old one, just so the existing `assert_any_call` / `to_dict()` keeps using the old shape. This silently masks the real public surface and propagates the wrong contract to anyone who reads the test as documentation. + +**The right move:** Update the test's mock setup so the test exercises the new public API end-to-end. Example from the theme refactor regression: the test originally patched `src.gui_2.imgui` and `src.imgui_scopes.imgui`, but `theme.get_color('text_disabled')` now resolves through `src.theme_2.imgui`. The fix was to add `patch("src.theme_2.imgui", new=mock_imgui)` to the context manager chain, not to wrap `C_LBL()` in another layer of mocks. + +**General rule:** when production code changes, the test must mock all transitively-imported modules that the production code's new code path now reaches. Re-walk the call graph; do not patch the same surface as before and add an `unittest.mock.patch("module.old_name", new=old_value)` shim. + +References: AGENTS.md "DO NOT CREATE MOCK PATCHES TO PSEUDO API CALLS OR HOOKS BECAUSE THE APP SOURCE WAS CHANGED. ADAPT TESTS PROPERLY."; `docs/superpowers/plans/2026-06-05-regression-fixes.md` §A.Theme (Task 1, Step 1.3). diff --git a/conductor/directives/controller_property_delegation_no_dual_state/meta.md b/conductor/directives/controller_property_delegation_no_dual_state/meta.md new file mode 100644 index 00000000..dcf8fbec --- /dev/null +++ b/conductor/directives/controller_property_delegation_no_dual_state/meta.md @@ -0,0 +1,7 @@ +# controller_property_delegation_no_dual_state + +## v1 + +**Why this iteration:** Lifted verbatim from `docs/superpowers/plans/2026-06-05-live-gui-state-sync.md` (Tasks 1-5: the property-pair pattern that fixed the App/Controller dual-state bug; tests in `tests/test_app_controller_state_sync.py`). +**Source:** `docs/superpowers/plans/2026-06-05-live-gui-state-sync.md` (Tasks 1-5; Architecture section) +**Lifted:** 2026-07-03 scavenge sweep batch 3/5: docs/superpowers/plans/ diff --git a/conductor/directives/controller_property_delegation_no_dual_state/v1.md b/conductor/directives/controller_property_delegation_no_dual_state/v1.md new file mode 100644 index 00000000..cade91ad --- /dev/null +++ b/conductor/directives/controller_property_delegation_no_dual_state/v1.md @@ -0,0 +1,29 @@ +# App must not duplicate Controller state; expose Controller fields as @property + @X.setter pairs + +## Pattern: Controller is the single source of truth; App is a thin facade + +When the App class holds its own copies of fields that the Controller also manages, two writers (the App's getter and the Controller's setter) silently diverge. Tests that read `app.X` after calling `set_value('X', v)` see stale values; save/load flows capture the wrong half. + +**The fix is property delegation:** add `@property` + `@X.setter` pairs on the App class so the getter reads `self.controller.X` and the setter writes `self.controller.X`. Both the legacy App-only fields (no Controller counterpart, kept as plain attributes) and the dual-state fields are kept distinct. + +```python + @property + def ui_ai_input(self) -> str: + return self.controller.ui_ai_input + + @ui_ai_input.setter + def ui_ai_input(self, value: str) -> None: + self.controller.ui_ai_input = value +``` + +**In-place mutation of mutable fields works** because the property returns the same reference the Controller owns. `app.show_windows['X'] = True` mutates the Controller's dict; `app.show_windows = new_dict` goes through the setter. + +**App-only fields stay as plain attributes.** Don't promote fields the Controller doesn't manage to properties. Validate by listing: `getattr(type(app), 'X', None)` should return `None` for App-only fields and a `property` instance for Controller-owned fields. + +**Regression test pattern:** +- Instantiate App via `App.__new__(App)` to skip `__init__`; attach a Controller (`app.controller = AppController()`). +- Set via Controller, read via App; set via App, read via Controller. Assert equality in both directions. +- For dict fields, test both replacement (`=` assignment) and in-place (`[key]=`) semantics. +- For App-only fields, assert `not hasattr(type(app), 'X')` so a future property leak is caught. + +References: docs/superpowers/plans/2026-06-05-live-gui-state-sync.md (Tasks 1-5). diff --git a/conductor/directives/enforce_no_real_toml_in_tests/v1.md b/conductor/directives/enforce_no_real_toml_in_tests/v1.md new file mode 100644 index 00000000..6ff406cf --- /dev/null +++ b/conductor/directives/enforce_no_real_toml_in_tests/v1.md @@ -0,0 +1,24 @@ +# Tests must not read or write real TOML files; route through `enforce_no_real_toml` autouse fixture + +Tests must never read or write the user's real `manual_slop.toml`, `config.toml`, `credentials.toml`, `presets.toml`, `personas.toml`, `tool_presets.toml`, `workspace_profiles.toml` from `cwd`. Doing so silently mutates the user's live config and gives the test unreproducible state across machines. + +**Enforcement is two-layered:** + +1. **Static audit** (CI gate): `scripts/check_test_toml_paths.py` greps every `tests/test_*.py` file for direct `./.toml`, `Path(".toml")`, `open(".toml")` references. Excludes `tests/artifacts/`, `tests/logs/`, `__pycache__/`, `snapshots/`. Exits 0 on clean; 1 on any violation. +2. **Runtime guard** (autouse fixture): `enforce_no_real_toml` in `tests/conftest.py` snapshots every real TOML in `cwd` before the test, removes them, restores them after. Tests that need TOML data must use `tmp_path` + `monkeypatch` to access it. + +**Pattern for sandboxed tests:** + +```python +def test_load_presets(tmp_path, monkeypatch): + from src import paths + presets_path = tmp_path / "presets.toml" + presets_path.write_text("[presets]\ndefault = {}\n") + monkeypatch.setattr(paths, "get_global_presets_path", lambda: presets_path) + data = tomllib.loads(presets_path.read_text()) + assert "default" in data +``` + +**Migration rule:** when a test is found using a real TOML, migrate it to `tmp_path` + `monkeypatch` (or reuse the existing `isolate_workspace` fixture). Do not add an exception list to the audit; fix the test, not the gate. + +References: `docs/superpowers/plans/2026-06-02-test-consolidation.md` (Tasks 1-2; the `TOML_BASENAMES` set + the `enforce_no_real_toml` fixture).