From 05bb901cf9654f9ec01d4c7d258e3fa130eeaa54 Mon Sep 17 00:00:00 2001 From: Nathaniel Ramm Date: Sat, 18 Apr 2026 20:14:44 +1000 Subject: [PATCH 1/8] docs(specs): add setattr-bypass fix design Restores canonical pydantic v2 assignment semantics on MountainAshBaseSettings (validate_assignment=True), removes the redundant update_settings_from_dict re-application in __init__, and converts seven meta-field writes to object.__setattr__. Addresses the setattr-bypass-limitation backlog item promoted from mountainash-data. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../2026-04-18-setattr-bypass-fix-design.md | 256 ++++++++++++++++++ 1 file changed, 256 insertions(+) create mode 100644 docs/superpowers/specs/2026-04-18-setattr-bypass-fix-design.md diff --git a/docs/superpowers/specs/2026-04-18-setattr-bypass-fix-design.md b/docs/superpowers/specs/2026-04-18-setattr-bypass-fix-design.md new file mode 100644 index 0000000..f812479 --- /dev/null +++ b/docs/superpowers/specs/2026-04-18-setattr-bypass-fix-design.md @@ -0,0 +1,256 @@ +# Setattr Bypass Fix — Restore Canonical Pydantic Assignment Semantics + +**Date:** 2026-04-18 +**Status:** Design — awaiting user review +**Related backlog:** `mountainash-central/01.principles/mountainash-data/f.backlog/setattr-bypass-limitation.md` +**Related prior spec:** `docs/superpowers/specs/2026-04-16-profiles-promotion-design.md` + +## Problem + +`MountainAshBaseSettings.update_settings_from_dict` uses raw `setattr(self, name, value)` +after pydantic's validation has already run. Because `MountainAshBaseSettings.model_config` +has `validate_assignment` disabled (commented out), these setattrs bypass the declared-type +contract: + +- `SecretStr` fields end up holding plain `str` at runtime. +- Enum-typed fields hold raw strings rather than enum members. +- `AfterValidator` transforms that return normalized values are overwritten with the + un-transformed raw input. + +The bypass forces per-class `__setattr__` overrides (`PySparkMode` in `mountainash-data`) +and per-field defensive unwrap guards (`ConnectionProfile._default_driver_kwargs`). +As the descriptor/registry pattern is promoted to `mountainash-settings` for reuse by +`mountainash-utils-files`, `mountainash-utils-secrets`, and `mountainash-acrds-core`, +every new domain inherits the workaround burden. + +## Root Cause Analysis + +`MountainAshBaseSettings.__init__` at `src/mountainash_settings/settings/base_settings.py:83-110` +performs the following sequence: + +1. `super().__init__(..., **valid_attribute_kwargs)` — full pydantic validation runs; fields are + wrapped/coerced/transformed correctly. +2. `self.update_settings_from_dict(settings_dict=valid_attribute_kwargs)` — + **the same kwargs** are raw-setattr'd back over the validated values, discarding + the validation work. +3. Seven `setattr(self, "SETTINGS_*", ...)` calls for meta-field bookkeeping. + +Pydantic v2's canonical contract is that `setattr` obeys declared types *only* when +`model_config["validate_assignment"] = True`. MountainAsh has this flag deliberately +off, and the `update_settings_from_dict` re-application amplifies the consequence: +every constructor call overwrites validated state with raw state. + +### Callsite audit for `update_settings_from_dict` + +Three live callsites in `mountainash-settings`: + +| Callsite | Purpose | Status | +|---|---|---| +| `base_settings.py:98` (inside `__init__`) | Re-apply kwargs that `super().__init__` just applied with validation. | **Redundant.** Its only non-redundant side-effect is the `SETTINGS_SOURCE_KWARGS` stash at line 258. | +| `settings_manager.py:47` | Apply runtime override kwargs to `model_copy()` of a cached instance. | **Legitimate.** Runtime-override flow — exactly where setattr bypass silently corrupts types. | +| `settings_parameters.py:365` (`apply_runtime_overrides`) | Same `model_copy()` + override kwargs pattern. | **Legitimate.** Parallel flow to the above. | + +## Goal + +Restore canonical pydantic v2 assignment semantics so declared field types (enums, +`SecretStr`, `AfterValidator` transforms) are honoured on all post-construction +mutations — both direct `setattr` and `update_settings_from_dict`. + +## Non-Goals (Out of Scope) + +- Removing `PySparkMode.__setattr__` override in `mountainash-data`. +- Removing `ConnectionProfile._default_driver_kwargs` `SecretStr` unwrap guard. +- Removing adapter `str(enum_value)` defensive code. +- Any change to `ProfileDescriptor` / `ParameterSpec` / `DescriptorProfile` public API. + +These are captured by a linked follow-up item (tracked separately in +`mountainash-data/f.backlog/`) once `mountainash-settings` is released and consumers bump. + +## Design + +Three coordinated changes, all within `mountainash-settings`. + +### Change A — Enable canonical assignment validation + +`src/mountainash_settings/settings/base_settings.py:16-23` + +```python +model_config = SettingsConfigDict( + extra="ignore", + validate_default=False, + arbitrary_types_allowed=True, + validate_assignment=True, # was commented out +) +``` + +**Effect:** every `setattr(instance, name, value)` runs the field's full validator +pipeline — enum coercion, `SecretStr` wrapping, `AfterValidator` transforms. +This is pydantic v2's documented canonical behaviour. + +### Change B — Remove redundant re-application in `__init__` + +`src/mountainash_settings/settings/base_settings.py:83-110` + +**Before:** + +```python +super().__init__(..., **valid_attribute_kwargs) # validated +... +self.update_settings_from_dict(settings_dict=valid_attribute_kwargs) # overwrites raw +setattr(self, "SETTINGS_CLASS", ...) +... # six more setattrs +``` + +**After:** + +```python +super().__init__(..., **valid_attribute_kwargs) # validated +... +# Bookkeeping only — no re-application of already-validated kwargs. +# object.__setattr__ documents intent: bypass validation for +# harness meta-fields that are not user config. +object.__setattr__(self, "SETTINGS_SOURCE_KWARGS", valid_attribute_kwargs) +object.__setattr__(self, "SETTINGS_CLASS", ...) +... # six more object.__setattr__ calls +``` + +**Rationale for `object.__setattr__` on the seven meta-field writes:** +With `validate_assignment=True`, every plain `setattr` validates. The meta-fields +(`SETTINGS_CLASS`, `SETTINGS_CLASS_NAME`, `SETTINGS_SOURCE_ENV_PREFIX`, +`SETTINGS_SOURCE_ENV_FILES`, `SETTINGS_SOURCE_YAML_FILES`, `SETTINGS_SOURCE_TOML_FILES`, +`SETTINGS_SOURCE_JSON_FILES`, `SETTINGS_SOURCE_SECRETS_DIR`) are permissively typed and +carry bookkeeping semantics, not user configuration. Explicit `object.__setattr__` +documents intentional bypass and avoids needless validation on a hot path. + +### Change C — Leave `update_settings_from_dict` as-is + +With `validate_assignment=True`, the method's existing `setattr` loop validates +automatically. Callsites 2 and 3 (runtime-override flows) correctly coerce enums, +wrap `SecretStr`, and apply transforms with zero code change. This is the systemic +payoff of going root-canonical: the runtime-override path becomes correct for free. + +## Risk Surface + +### Category 1 — Internal setattrs in `MountainAshBaseSettings.__init__` + +Seven writes converted to `object.__setattr__` by Change B → exempt by construction. + +### Category 2 — Downstream `setattr` in subclasses / callers + +Every plain `setattr` on a `MountainAshBaseSettings` instance now validates: + +- **Type-compatible value written** → validation passes, transforms fire, + behaviour becomes more correct. Intended outcome. +- **Type-incompatible value written** → `ValidationError` raises. That's a latent + bug the bypass was previously masking. + +**Audit scope within `mountainash-settings`:** grep for `setattr` / `__setattr__` +in `src/` and `tests/`. Confirm each site either (a) targets a meta-field now +using `object.__setattr__` via Change B, or (b) writes a type-compatible value. + +`DescriptorProfile.post_init` at `src/mountainash_settings/profiles/profile.py:147` +already uses `object.__setattr__` — unchanged, correct. + +### Category 3 — Existing test failures + +`tests/test_base_settings_coverage.py` has `update_settings_from_dict` tests +(lines 312-367, 541, 608). Expected outcomes after Changes A+B: + +- Tests writing type-compatible values → pass, possibly with adjusted assertion + shape (e.g. `SecretStr` vs raw `str`). +- Tests writing type-incompatible values → raise `ValidationError`. + +Per the user's global test-integrity rule, each failure is triaged explicitly: +either the test asserted the broken contract (rewrite to the validated contract) +or the implementation has a genuine bug (fix the implementation). **No test is +skipped, disabled, or silently modified.** Decisions escalate to the user. + +### Category 4 — `DescriptorProfile` subclasses + +`DescriptorProfile.__pydantic_init_subclass__` at `profile.py:42-90` already wires +`AfterValidator` from `ParameterSpec.validator`. With `validate_assignment=True`, +those validators fire on assignment too — exactly what the backlog item wanted. +No code change required in this file. This is where the systemic payoff lands +across the four target domains. + +## Testing Strategy + +### New tests (added to `tests/test_base_settings_coverage.py`) + +A new class `TestCanonicalAssignmentSemantics` covering: + +1. **SecretStr round-trip on assignment.** Construct a settings class with a + `SecretStr` field. `setattr` a raw string. Assert stored value is + `isinstance(..., SecretStr)` and `.get_secret_value()` returns the raw string. + +2. **Enum coercion on assignment.** Settings class with a `StrEnum` field. + `setattr` the raw string value. Assert stored value `is` the enum member + (identity, not equality — `PySparkMode`'s workaround was specifically about + identity). + +3. **`AfterValidator` transform on assignment.** Settings class with + `Annotated[str, AfterValidator(str.upper)]`. `setattr` a lowercase string. + Assert stored value is uppercased. + +4. **`update_settings_from_dict` validates.** Same three primitives above, routed + through `update_settings_from_dict`. Confirms `SettingsManager` and + `apply_runtime_overrides` flows are fixed. + +5. **`DescriptorProfile` integration.** Minimal `DescriptorProfile` subclass with + `ParameterSpec(secret=True)`, `ParameterSpec(type=SomeStrEnum)`, and + `ParameterSpec(validator=lambda s: s.upper())`. Assert correct behaviour on + both construction and post-construction `setattr`. + +6. **Meta-field bookkeeping bypass.** Assert `SETTINGS_SOURCE_KWARGS` and the + other six meta-fields still write as dicts/lists via `object.__setattr__` + in `__init__` and are not coerced (guards against someone "tidying up" by + removing the bypass). + +### Regression guard + +```python +def test_validate_assignment_is_enabled(): + """Regression guard — canonical assignment validation must stay on.""" + assert MountainAshBaseSettings.model_config.get("validate_assignment") is True +``` + +### Existing tests + +Run `hatch run test:test` after Changes A+B; triage each failure per Category 3 +above. + +## Implementation Order + +One commit per step for clean bisect: + +1. Add new tests from `TestCanonicalAssignmentSemantics` as `xfail` / skipped + (proves contract pre-change). +2. **Change A:** flip `validate_assignment=True`. +3. **Change B:** convert seven `__init__` meta-field writes to `object.__setattr__`; + remove redundant `update_settings_from_dict(valid_attribute_kwargs)` call; + lift `SETTINGS_SOURCE_KWARGS` stash inline. +4. Flip new tests to passing (remove `xfail`). Triage existing-test failures. +5. Add regression guard test. + +## Release Notes Entry + +> **Behaviour change:** `MountainAshBaseSettings` now validates on assignment +> (`validate_assignment=True`). Values written via `setattr` after construction — +> including via `update_settings_from_dict`, `SettingsManager` runtime overrides, +> and subclass custom setters — now run the declared field validator pipeline. +> `SecretStr` fields wrap raw strings automatically; enum fields coerce raw +> values to enum members; `AfterValidator` transforms apply on assignment as +> well as construction. Subclasses that previously relied on `setattr` bypassing +> validation must switch to `object.__setattr__` for intentional bypass, or fix +> the declared type. CalVer micro bump recommended. + +## Follow-up (Tracked Separately) + +After `mountainash-settings` is released and consumers bump: + +- Remove `PySparkMode.__setattr__` override in `mountainash-data`. +- Remove `ConnectionProfile._default_driver_kwargs` `SecretStr` unwrap guard + (keep the transform-at-emit step; drop only the defensive unwrap). +- Audit adapter `str(enum_value)` defensive code; simplify where enum instances + are now guaranteed. +- Close `mountainash-central/01.principles/mountainash-data/f.backlog/setattr-bypass-limitation.md`. From b6e74428e740381c4f35ada43f1685b9f3672b13 Mon Sep 17 00:00:00 2001 From: Nathaniel Ramm Date: Sat, 18 Apr 2026 20:19:14 +1000 Subject: [PATCH 2/8] docs(plans): add setattr-bypass fix implementation plan MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four-task TDD plan: xfail tests → validate_assignment flip → __init__ refactor → regression guard. Plus a final verify + PR-open task targeting develop per the three-tier flow. Spec: docs/superpowers/specs/2026-04-18-setattr-bypass-fix-design.md Co-Authored-By: Claude Opus 4.7 (1M context) --- .../plans/2026-04-18-setattr-bypass-fix.md | 616 ++++++++++++++++++ 1 file changed, 616 insertions(+) create mode 100644 docs/superpowers/plans/2026-04-18-setattr-bypass-fix.md diff --git a/docs/superpowers/plans/2026-04-18-setattr-bypass-fix.md b/docs/superpowers/plans/2026-04-18-setattr-bypass-fix.md new file mode 100644 index 0000000..4a40101 --- /dev/null +++ b/docs/superpowers/plans/2026-04-18-setattr-bypass-fix.md @@ -0,0 +1,616 @@ +# Setattr Bypass Fix Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Restore canonical pydantic v2 assignment semantics on `MountainAshBaseSettings` so declared field types (enums, `SecretStr`, `AfterValidator` transforms) are honoured on every post-construction mutation. + +**Architecture:** Flip `model_config["validate_assignment"] = True`, remove the redundant `update_settings_from_dict` re-application inside `__init__`, and convert the seven meta-field writes in `__init__` to `object.__setattr__` so they stay exempt from the newly-enabled validation. + +**Tech Stack:** Python 3.12 · pydantic 2.9 · pydantic-settings 2.6 · hatch · pytest · UPath + +**Spec:** `docs/superpowers/specs/2026-04-18-setattr-bypass-fix-design.md` + +--- + +## File Structure + +**Modified files:** +- `src/mountainash_settings/settings/base_settings.py` — config flag + `__init__` cleanup +- `tests/test_base_settings_coverage.py` — add `TestCanonicalAssignmentSemantics` and regression-guard test + +**Unchanged but relevant (do not edit):** +- `src/mountainash_settings/profiles/profile.py` — `DescriptorProfile` already wires `AfterValidator` and uses `object.__setattr__` for template writes; no edit needed. +- `src/mountainash_settings/settings_cache/settings_manager.py:47` — uses `update_settings_from_dict`; gains correctness automatically. +- `src/mountainash_settings/settings_parameters/settings_parameters.py:365` — same. + +**Branch:** work continues on `feat/profiles-promotion` (current branch). Target PR base: `develop`. + +--- + +## Task 1: Add canonical-assignment tests as xfail + +**Files:** +- Modify: `tests/test_base_settings_coverage.py` — append a new test class at the end of the file (after `TestPostInitHook`, line 608-ish). + +Rationale: these tests codify the contract we want. Add them as `xfail` first so they don't block commits; flip to expected-pass after Change A lands in Task 2. + +- [ ] **Step 1: Read the current end of `test_base_settings_coverage.py`** + +```bash +wc -l tests/test_base_settings_coverage.py +``` + +Expected: a line count (~610). Note the number — we'll append after the last class. + +- [ ] **Step 2: Append the new test class** + +Add this at the end of `tests/test_base_settings_coverage.py`: + +```python + + +# --------------------------------------------------------------------------- +# Canonical pydantic assignment semantics +# --------------------------------------------------------------------------- +# See docs/superpowers/specs/2026-04-18-setattr-bypass-fix-design.md +# Tests codify the contract restored by enabling validate_assignment=True. +# --------------------------------------------------------------------------- + +from enum import StrEnum +from typing import Annotated + +from pydantic import AfterValidator, Field, SecretStr + +from mountainash_settings import MountainAshBaseSettings +from mountainash_settings.auth import NoAuth +from mountainash_settings.profiles import ( + DescriptorProfile, + ParameterSpec, + ProfileDescriptor, +) + + +class _Mode(StrEnum): + FULL = "full" + INCREMENTAL = "incremental" + + +class _SecretSettings(MountainAshBaseSettings): + PASSWORD: SecretStr = Field(default=SecretStr("")) + + +class _EnumSettings(MountainAshBaseSettings): + MODE: _Mode = Field(default=_Mode.FULL) + + +class _TransformSettings(MountainAshBaseSettings): + NAME: Annotated[str, AfterValidator(str.upper)] = Field(default="") + + +class _SampleProfile(DescriptorProfile): + __descriptor__ = ProfileDescriptor( + name="sample", + provider_type="sample", + parameters=[ + ParameterSpec(name="TOKEN", type=str, tier="core", secret=True), + ParameterSpec(name="MODE", type=_Mode, tier="core", + default=_Mode.FULL), + ParameterSpec(name="LABEL", type=str, tier="core", default="x", + validator=str.upper), + ], + auth_modes=[NoAuth], + ) + + +class TestCanonicalAssignmentSemantics: + """Validate_assignment=True restores pydantic's declared-type contract.""" + + @pytest.mark.unit + @pytest.mark.xfail(reason="Enabled by Change A in Task 2", strict=True) + def test_secretstr_wraps_on_direct_setattr(self): + s = _SecretSettings() + s.PASSWORD = "plain" + assert isinstance(s.PASSWORD, SecretStr) + assert s.PASSWORD.get_secret_value() == "plain" + + @pytest.mark.unit + @pytest.mark.xfail(reason="Enabled by Change A in Task 2", strict=True) + def test_enum_coerces_on_direct_setattr(self): + s = _EnumSettings() + s.MODE = "incremental" + assert s.MODE is _Mode.INCREMENTAL + + @pytest.mark.unit + @pytest.mark.xfail(reason="Enabled by Change A in Task 2", strict=True) + def test_aftervalidator_transforms_on_direct_setattr(self): + s = _TransformSettings() + s.NAME = "lower" + assert s.NAME == "LOWER" + + @pytest.mark.unit + @pytest.mark.xfail(reason="Enabled by Change A in Task 2", strict=True) + def test_update_settings_from_dict_wraps_secretstr(self): + s = _SecretSettings() + s.update_settings_from_dict({"PASSWORD": "plain"}) + assert isinstance(s.PASSWORD, SecretStr) + assert s.PASSWORD.get_secret_value() == "plain" + + @pytest.mark.unit + @pytest.mark.xfail(reason="Enabled by Change A in Task 2", strict=True) + def test_update_settings_from_dict_coerces_enum(self): + s = _EnumSettings() + s.update_settings_from_dict({"MODE": "incremental"}) + assert s.MODE is _Mode.INCREMENTAL + + @pytest.mark.unit + @pytest.mark.xfail(reason="Enabled by Change A in Task 2", strict=True) + def test_update_settings_from_dict_applies_transform(self): + s = _TransformSettings() + s.update_settings_from_dict({"NAME": "lower"}) + assert s.NAME == "LOWER" + + @pytest.mark.unit + @pytest.mark.xfail(reason="Enabled by Change A in Task 2", strict=True) + def test_descriptor_profile_secret_on_setattr(self): + p = _SampleProfile(TOKEN="raw", auth=NoAuth()) + p.TOKEN = "new" + assert isinstance(p.TOKEN, SecretStr) + assert p.TOKEN.get_secret_value() == "new" + + @pytest.mark.unit + @pytest.mark.xfail(reason="Enabled by Change A in Task 2", strict=True) + def test_descriptor_profile_enum_on_setattr(self): + p = _SampleProfile(TOKEN="raw", auth=NoAuth()) + p.MODE = "incremental" + assert p.MODE is _Mode.INCREMENTAL + + @pytest.mark.unit + @pytest.mark.xfail(reason="Enabled by Change A in Task 2", strict=True) + def test_descriptor_profile_validator_transform_on_setattr(self): + p = _SampleProfile(TOKEN="raw", auth=NoAuth()) + p.LABEL = "lower" + assert p.LABEL == "LOWER" +``` + +- [ ] **Step 3: Run the new tests to confirm they xfail (not error)** + +```bash +hatch run test:test tests/test_base_settings_coverage.py::TestCanonicalAssignmentSemantics -v +``` + +Expected: all 9 tests reported as XFAIL (expected failure). If any errors with `ImportError` or class-definition failure, fix imports/class definitions before continuing — the tests must *run* (and fail at the assertion) for `xfail` to be meaningful. A test that errors during collection is not a valid `xfail`. + +- [ ] **Step 4: Run the full existing suite to confirm no regression** + +```bash +hatch run test:test +``` + +Expected: previous baseline green plus 9 new XFAIL lines. No new FAIL or ERROR. + +- [ ] **Step 5: Commit** + +```bash +git add tests/test_base_settings_coverage.py +git commit -m "$(cat <<'EOF' +test(base_settings): add canonical assignment semantics tests (xfail) + +Codifies the contract to be restored by enabling +validate_assignment=True on MountainAshBaseSettings. Covers +SecretStr wrapping, StrEnum coercion, AfterValidator transform +on direct setattr, update_settings_from_dict, and the +DescriptorProfile integration path. + +Marked xfail(strict=True) — will flip to expected-pass once +Change A lands. + +Spec: docs/superpowers/specs/2026-04-18-setattr-bypass-fix-design.md +EOF +)" +``` + +--- + +## Task 2: Change A — enable `validate_assignment=True` + +**Files:** +- Modify: `src/mountainash_settings/settings/base_settings.py:16-23` + +- [ ] **Step 1: Flip the config flag** + +Replace the `model_config` block at lines 16-23: + +```python + model_config = SettingsConfigDict( + extra="ignore", + validate_default=False, + arbitrary_types_allowed=True, + # validate_assignment=True, + # validate_assignment=False, + + ) +``` + +with: + +```python + model_config = SettingsConfigDict( + extra="ignore", + validate_default=False, + arbitrary_types_allowed=True, + validate_assignment=True, + + ) +``` + +- [ ] **Step 2: Run the canonical-assignment tests — they should now fail xfail (XPASS)** + +```bash +hatch run test:test tests/test_base_settings_coverage.py::TestCanonicalAssignmentSemantics -v +``` + +Expected: **all 9 tests FAIL** because `xfail(strict=True)` converts unexpected passes to failures. The output will show `XPASS(strict)` — that's the signal Change A worked. + +- [ ] **Step 3: Run the full suite to surface existing-test impact** + +```bash +hatch run test:test 2>&1 | tee /tmp/phase2-suite.log +``` + +Expected: a mix of outcomes: +- New `XPASS(strict)` lines (treated as failures) — fixed in Task 3 Step 1. +- Previously-passing tests may now fail if they asserted raw-string post-setattr shape. + +- [ ] **Step 4: Triage failures** + +For each failing test that is NOT one of our new `TestCanonicalAssignmentSemantics` cases: + +1. Read the failure and the test source. +2. Classify: + - **(a) Test asserted the broken contract** (e.g. `assert isinstance(s.PASSWORD, str) and not isinstance(s.PASSWORD, SecretStr)`) → this test codified the bypass bug. Report to user with the classification; await direction. + - **(b) Implementation has a latent bug exposed by validation** → report to user; await direction. + +Per `~/.claude/CLAUDE.md` test-integrity rule: **do not skip, xfail, or silently rewrite any existing test.** Surface each failure to the user before making changes. + +If there are zero existing-test failures (the ideal case), proceed to Step 5. + +- [ ] **Step 5: Remove `xfail` markers from `TestCanonicalAssignmentSemantics`** + +In `tests/test_base_settings_coverage.py`, delete every line that matches: + +```python + @pytest.mark.xfail(reason="Enabled by Change A in Task 2", strict=True) +``` + +Nine occurrences — one per test method in `TestCanonicalAssignmentSemantics`. Leave the `@pytest.mark.unit` decorators intact. + +- [ ] **Step 6: Re-run and confirm green** + +```bash +hatch run test:test tests/test_base_settings_coverage.py::TestCanonicalAssignmentSemantics -v +hatch run test:test +``` + +Expected: all 9 canonical-assignment tests PASS. Full suite green (or at parity with Step 4's triaged baseline). + +- [ ] **Step 7: Commit** + +```bash +git add src/mountainash_settings/settings/base_settings.py tests/test_base_settings_coverage.py +git commit -m "$(cat <<'EOF' +feat(base_settings): enable validate_assignment for canonical semantics + +Sets model_config["validate_assignment"] = True on +MountainAshBaseSettings. Every setattr on an instance (including +via update_settings_from_dict, SettingsManager runtime overrides, +and apply_runtime_overrides) now runs the field's declared +validator pipeline — enum coercion, SecretStr wrapping, +AfterValidator transforms. + +Addresses the setattr-bypass-limitation backlog item. Removes +the bypass mechanism that forced PySparkMode.__setattr__ and +ConnectionProfile SecretStr defensive code in consumer packages. + +Flips the canonical-assignment test class out of xfail. + +Spec: docs/superpowers/specs/2026-04-18-setattr-bypass-fix-design.md +EOF +)" +``` + +--- + +## Task 3: Change B — remove redundant re-application in `__init__` + +**Files:** +- Modify: `src/mountainash_settings/settings/base_settings.py:97-107` + +**Why this task is separate from Task 2:** Change A alone fixes the canonical contract. Change B is about removing dead code (the redundant re-application) and documenting intentional validation bypass for harness meta-fields. Keeping them as separate commits makes bisect clean. + +- [ ] **Step 1: Write a test asserting internal bookkeeping still stashes meta-fields** + +Append to `TestCanonicalAssignmentSemantics` in `tests/test_base_settings_coverage.py`: + +```python + + @pytest.mark.unit + def test_meta_field_bookkeeping_still_works(self): + """Change B refactors __init__ meta-field writes to + object.__setattr__. Confirm the bookkeeping values still land.""" + from fixtures.settings_classes import TestSettings + s = TestSettings(TEST_VAL_1="x", TEST_VAL_2="y") + assert s.SETTINGS_CLASS is TestSettings + assert s.SETTINGS_CLASS_NAME == "TestSettings" + assert s.SETTINGS_SOURCE_KWARGS == {"TEST_VAL_1": "x", "TEST_VAL_2": "y"} +``` + +- [ ] **Step 2: Run the new test — it should pass on current code (baseline)** + +```bash +hatch run test:test tests/test_base_settings_coverage.py::TestCanonicalAssignmentSemantics::test_meta_field_bookkeeping_still_works -v +``` + +Expected: PASS. This is the guard: if Change B breaks meta-field bookkeeping, this test flips red. + +- [ ] **Step 3: Apply Change B in `base_settings.py`** + +Replace lines 97-107 (the section from the `#Update all vals from valid kwargs` comment through the last `setattr`): + +```python + #Update all vals from valid kwargs + self.update_settings_from_dict(settings_dict=valid_attribute_kwargs) + + setattr(self, "SETTINGS_CLASS", local_settings_params.settings_class or MountainAshBaseSettings) + setattr(self, "SETTINGS_CLASS_NAME", local_settings_params.settings_class.__name__ if local_settings_params.settings_class else "MountainAshBaseSettings") + setattr(self, "SETTINGS_SOURCE_ENV_PREFIX", local_settings_params.env_prefix) + setattr(self, "SETTINGS_SOURCE_ENV_FILES", obj_config_files.env_files) + setattr(self, "SETTINGS_SOURCE_YAML_FILES", obj_config_files.yaml_files) + setattr(self, "SETTINGS_SOURCE_TOML_FILES", obj_config_files.toml_files) + setattr(self, "SETTINGS_SOURCE_JSON_FILES", obj_config_files.json_files) + setattr(self, "SETTINGS_SOURCE_SECRETS_DIR", local_settings_params.secrets_dir) +``` + +with: + +```python + # Meta-field bookkeeping only. super().__init__ above already applied + # valid_attribute_kwargs under full validation — re-applying them via + # update_settings_from_dict would overwrite validated values with raw + # input (see setattr-bypass-limitation spec, 2026-04-18). + # + # object.__setattr__ is intentional: these fields are harness + # bookkeeping, not user config, and with validate_assignment=True on + # model_config we want to skip revalidation on them explicitly. + object.__setattr__(self, "SETTINGS_SOURCE_KWARGS", valid_attribute_kwargs) + object.__setattr__(self, "SETTINGS_CLASS", local_settings_params.settings_class or MountainAshBaseSettings) + object.__setattr__(self, "SETTINGS_CLASS_NAME", local_settings_params.settings_class.__name__ if local_settings_params.settings_class else "MountainAshBaseSettings") + object.__setattr__(self, "SETTINGS_SOURCE_ENV_PREFIX", local_settings_params.env_prefix) + object.__setattr__(self, "SETTINGS_SOURCE_ENV_FILES", obj_config_files.env_files) + object.__setattr__(self, "SETTINGS_SOURCE_YAML_FILES", obj_config_files.yaml_files) + object.__setattr__(self, "SETTINGS_SOURCE_TOML_FILES", obj_config_files.toml_files) + object.__setattr__(self, "SETTINGS_SOURCE_JSON_FILES", obj_config_files.json_files) + object.__setattr__(self, "SETTINGS_SOURCE_SECRETS_DIR", local_settings_params.secrets_dir) +``` + +Note: `SETTINGS_SOURCE_KWARGS` is now stashed inline (first `object.__setattr__`) — previously it was set by `update_settings_from_dict` at line 258. `update_settings_from_dict` still stashes it at line 258 for callsites 2 & 3 (`SettingsManager.get_settings_object`, `apply_runtime_overrides`) — that line is **unchanged**. + +- [ ] **Step 4: Run the meta-field bookkeeping test** + +```bash +hatch run test:test tests/test_base_settings_coverage.py::TestCanonicalAssignmentSemantics::test_meta_field_bookkeeping_still_works -v +``` + +Expected: PASS. + +- [ ] **Step 5: Run the full `TestUpdateSettingsFromDict` class** + +```bash +hatch run test:test tests/test_base_settings_coverage.py::TestUpdateSettingsFromDict -v +``` + +Expected: all existing tests still PASS. `update_settings_from_dict` itself is unchanged; only the redundant call inside `__init__` was removed. + +- [ ] **Step 6: Run the full suite** + +```bash +hatch run test:test +``` + +Expected: green, at parity with end of Task 2. + +- [ ] **Step 7: Commit** + +```bash +git add src/mountainash_settings/settings/base_settings.py tests/test_base_settings_coverage.py +git commit -m "$(cat <<'EOF' +refactor(base_settings): remove redundant kwargs re-application in __init__ + +super().__init__(**valid_attribute_kwargs) already applies kwargs +under full pydantic validation. The subsequent +update_settings_from_dict(valid_attribute_kwargs) call was +overwriting the validated values with raw input — the original +source of the setattr-bypass-limitation. + +Replaces the call plus seven meta-field setattrs with explicit +object.__setattr__ writes. Meta-fields are harness bookkeeping, +not user config: bypassing validation is intentional and now +explicit. SETTINGS_SOURCE_KWARGS is stashed inline here; +update_settings_from_dict still stashes it for the +SettingsManager and apply_runtime_overrides callsites. + +Adds a guard test for meta-field bookkeeping. + +Spec: docs/superpowers/specs/2026-04-18-setattr-bypass-fix-design.md +EOF +)" +``` + +--- + +## Task 4: Regression guard + +**Files:** +- Modify: `tests/test_base_settings_coverage.py` + +- [ ] **Step 1: Add a regression guard test** + +Append to `TestCanonicalAssignmentSemantics` in `tests/test_base_settings_coverage.py`: + +```python + + @pytest.mark.unit + def test_validate_assignment_is_enabled(self): + """Regression guard — canonical assignment validation must stay on. + + If this assertion fires, someone disabled validate_assignment on + MountainAshBaseSettings. Do not 'fix' by deleting this test. + See docs/superpowers/specs/2026-04-18-setattr-bypass-fix-design.md + """ + assert MountainAshBaseSettings.model_config.get("validate_assignment") is True +``` + +- [ ] **Step 2: Run the guard** + +```bash +hatch run test:test tests/test_base_settings_coverage.py::TestCanonicalAssignmentSemantics::test_validate_assignment_is_enabled -v +``` + +Expected: PASS. + +- [ ] **Step 3: Run the full suite one last time** + +```bash +hatch run test:test +``` + +Expected: green. + +- [ ] **Step 4: Lint the modified files** + +```bash +hatch run ruff:check src/mountainash_settings/settings/base_settings.py tests/test_base_settings_coverage.py +``` + +Expected: no complaints, or run `hatch run ruff:fix` on the same paths and re-run. + +- [ ] **Step 5: Commit** + +```bash +git add tests/test_base_settings_coverage.py +git commit -m "$(cat <<'EOF' +test(base_settings): add regression guard for validate_assignment + +Explicit assertion that model_config["validate_assignment"] is True +on MountainAshBaseSettings. Fails loudly if a future change turns +off the canonical assignment contract. + +Spec: docs/superpowers/specs/2026-04-18-setattr-bypass-fix-design.md +EOF +)" +``` + +--- + +## Task 5: Final verification + PR + +- [ ] **Step 1: Confirm the full test suite is green** + +```bash +hatch run test:test +``` + +Expected: all tests pass, including the 11 new ones in `TestCanonicalAssignmentSemantics` (9 canonical + 1 bookkeeping guard + 1 regression guard). + +- [ ] **Step 2: Confirm ruff is clean for the whole project** + +```bash +hatch run ruff:check +``` + +Expected: clean. Fix any complaints or run `hatch run ruff:fix`, then re-run and recommit with `style:` prefix. + +- [ ] **Step 3: Confirm mypy is clean** + +```bash +hatch run mypy:check +``` + +Expected: clean (or at parity with pre-change baseline). Triage any new errors with the user before proceeding. + +- [ ] **Step 4: Verify the commits look right** + +```bash +git log --oneline feat/profiles-promotion ^origin/develop +``` + +Expected: four new commits on top of the spec commit — `test: xfail tests`, `feat: validate_assignment`, `refactor: __init__`, `test: regression guard`. Plus the design-doc commit. + +- [ ] **Step 5: Push the branch** + +```bash +git push -u origin feat/profiles-promotion +``` + +- [ ] **Step 6: Open the PR targeting `develop`** + +Per the mountainash-io three-tier flow, feature branches PR into `develop`, not `main`. + +```bash +gh pr create --base develop --title "fix(base_settings): restore canonical pydantic assignment semantics" --body "$(cat <<'EOF' +## Summary + +- Enables `validate_assignment=True` on `MountainAshBaseSettings` so declared field types (enums, `SecretStr`, `AfterValidator` transforms) are honoured on every post-construction mutation. +- Removes the redundant `update_settings_from_dict` re-application in `__init__` that was overwriting validated fields with raw kwargs. +- Converts seven meta-field writes in `__init__` to explicit `object.__setattr__` to document intentional bypass for harness bookkeeping. + +## Why + +Addresses the `setattr-bypass-limitation` backlog item: +`mountainash-central/01.principles/mountainash-data/f.backlog/setattr-bypass-limitation.md` + +Root cause was `__init__` applying kwargs twice: once via `super().__init__(**kwargs)` (with validation) and then again via `update_settings_from_dict` (raw setattr, no validation) — discarding the validated values. Combined with `validate_assignment` being disabled, this forced per-class `__setattr__` overrides (e.g. `PySparkMode`) and defensive unwrap guards (e.g. `ConnectionProfile._default_driver_kwargs`) across consumer packages. + +## Test plan + +- [ ] `hatch run test:test` green +- [ ] `hatch run ruff:check` clean +- [ ] `hatch run mypy:check` clean +- [ ] New `TestCanonicalAssignmentSemantics` class (11 tests) passes +- [ ] Existing `TestUpdateSettingsFromDict` still passes +- [ ] Downstream packages (`mountainash-data`) still build against this branch + +## Spec & Design + +`docs/superpowers/specs/2026-04-18-setattr-bypass-fix-design.md` + +## Follow-up (separate PRs) + +After this is released and consumers bump: + +- Remove `PySparkMode.__setattr__` override in `mountainash-data` +- Remove `ConnectionProfile._default_driver_kwargs` SecretStr unwrap guard +- Simplify adapter `str(enum_value)` defensive code +- Close the backlog item + +🤖 Generated with [Claude Code](https://claude.com/claude-code) +EOF +)" +``` + +--- + +## Self-Review + +**Spec coverage:** +- Change A (`validate_assignment=True`) → Task 2. +- Change B (remove redundant call + convert meta-field setattrs) → Task 3. +- Change C (leave `update_settings_from_dict` as-is) → confirmed unchanged in Task 3 Step 5. +- Test strategy items (SecretStr / enum / AfterValidator / update_settings_from_dict / DescriptorProfile / meta-field bookkeeping / regression guard) → Tasks 1, 3, 4. +- Release-notes entry → captured in the PR body in Task 5 Step 6. +- Risk category 2 triage (existing-test failures) → Task 2 Step 4. + +**Placeholder scan:** no TBD/TODO; every step has concrete code or commands. Triage step in Task 2 Step 4 references specific classification rules, not a vague "handle failures." + +**Type consistency:** `_Mode`, `_SecretSettings`, `_EnumSettings`, `_TransformSettings`, `_SampleProfile` are referenced consistently across Task 1 tests. `ParameterSpec`, `ProfileDescriptor`, `NoAuth`, `DescriptorProfile` import paths match the current package exports (`src/mountainash_settings/__init__.py` and `src/mountainash_settings/profiles/__init__.py`, verified against `feat/profiles-promotion` HEAD). + +**Gap found & fixed:** original draft had no explicit ruff/mypy verification; added to Task 5 Steps 2–3. From 5933a027be244e3daa9e620dfc3dd82689c5cdef Mon Sep 17 00:00:00 2001 From: Nathaniel Ramm Date: Sat, 18 Apr 2026 20:26:02 +1000 Subject: [PATCH 3/8] test(base_settings): add canonical assignment semantics tests (xfail) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codifies the contract to be restored by enabling validate_assignment=True on MountainAshBaseSettings. Covers SecretStr wrapping, StrEnum coercion, AfterValidator transform on direct setattr, update_settings_from_dict, and the DescriptorProfile integration path. Marked xfail(strict=True) — will flip to expected-pass once Change A lands. Spec: docs/superpowers/specs/2026-04-18-setattr-bypass-fix-design.md --- tests/test_base_settings_coverage.py | 136 +++++++++++++++++++++++++++ 1 file changed, 136 insertions(+) diff --git a/tests/test_base_settings_coverage.py b/tests/test_base_settings_coverage.py index 73dff32..b69dc7e 100644 --- a/tests/test_base_settings_coverage.py +++ b/tests/test_base_settings_coverage.py @@ -625,3 +625,139 @@ def test_extract_parameters_idempotent(self): # Should produce equivalent parameters assert extracted1.settings_class == extracted2.settings_class assert extracted1.kwargs == extracted2.kwargs + + +# --------------------------------------------------------------------------- +# Canonical pydantic assignment semantics +# --------------------------------------------------------------------------- +# See docs/superpowers/specs/2026-04-18-setattr-bypass-fix-design.md +# Tests codify the contract restored by enabling validate_assignment=True. +# --------------------------------------------------------------------------- + +from enum import Enum +from typing import Annotated + +from pydantic import AfterValidator, Field, SecretStr + +from mountainash_settings import MountainAshBaseSettings +from mountainash_settings.auth import NoAuth +from mountainash_settings.profiles import ( + DescriptorProfile, + ParameterSpec, + ProfileDescriptor, +) + + +class _Mode(str, Enum): + FULL = "full" + INCREMENTAL = "incremental" + + +class _SecretSettings(MountainAshBaseSettings): + PASSWORD: SecretStr = Field(default=SecretStr("")) + + +class _EnumSettings(MountainAshBaseSettings): + MODE: _Mode = Field(default=_Mode.FULL) + + +class _TransformSettings(MountainAshBaseSettings): + NAME: Annotated[str, AfterValidator(str.upper)] = Field(default="") + + +class _SampleProfile(DescriptorProfile): + __descriptor__ = ProfileDescriptor( + name="sample", + provider_type="sample", + parameters=[ + ParameterSpec(name="TOKEN", type=str, tier="core", secret=True), + ParameterSpec(name="MODE", type=_Mode, tier="core", + default=_Mode.FULL), + ParameterSpec(name="LABEL", type=str, tier="core", default="x", + validator=str.upper), + ], + auth_modes=[NoAuth], + ) + + +class TestCanonicalAssignmentSemantics: + """Validate_assignment=True restores pydantic's declared-type contract.""" + + @pytest.mark.unit + @pytest.mark.xfail(reason="Enabled by Change A in Task 2", strict=True) + def test_secretstr_wraps_on_direct_setattr(self): + s = _SecretSettings() + # Raw-str assignment is the scenario under test: + # validate_assignment=True should wrap it into SecretStr at runtime. + s.PASSWORD = "plain" # type: ignore[assignment] + assert isinstance(s.PASSWORD, SecretStr) + assert s.PASSWORD.get_secret_value() == "plain" + + @pytest.mark.unit + @pytest.mark.xfail(reason="Enabled by Change A in Task 2", strict=True) + def test_enum_coerces_on_direct_setattr(self): + s = _EnumSettings() + # Raw-str assignment is the scenario under test: + # validate_assignment=True should coerce it to _Mode at runtime. + s.MODE = "incremental" # type: ignore[assignment] + assert s.MODE is _Mode.INCREMENTAL + + @pytest.mark.unit + @pytest.mark.xfail(reason="Enabled by Change A in Task 2", strict=True) + def test_aftervalidator_transforms_on_direct_setattr(self): + s = _TransformSettings() + s.NAME = "lower" + assert s.NAME == "LOWER" + + @pytest.mark.unit + @pytest.mark.xfail(reason="Enabled by Change A in Task 2", strict=True) + def test_update_settings_from_dict_wraps_secretstr(self): + s = _SecretSettings() + s.update_settings_from_dict({"PASSWORD": "plain"}) + assert isinstance(s.PASSWORD, SecretStr) + assert s.PASSWORD.get_secret_value() == "plain" + + @pytest.mark.unit + @pytest.mark.xfail(reason="Enabled by Change A in Task 2", strict=True) + def test_update_settings_from_dict_coerces_enum(self): + s = _EnumSettings() + s.update_settings_from_dict({"MODE": "incremental"}) + assert s.MODE is _Mode.INCREMENTAL + + @pytest.mark.unit + @pytest.mark.xfail(reason="Enabled by Change A in Task 2", strict=True) + def test_update_settings_from_dict_applies_transform(self): + s = _TransformSettings() + s.update_settings_from_dict({"NAME": "lower"}) + assert s.NAME == "LOWER" + + @pytest.mark.unit + @pytest.mark.xfail(reason="Enabled by Change A in Task 2", strict=True) + def test_descriptor_profile_secret_on_setattr(self): + # Fields (TOKEN/MODE/LABEL) are installed at runtime by + # DescriptorProfile.__pydantic_init_subclass__; pyright has no + # static view of them. + p = _SampleProfile(TOKEN="raw", auth=NoAuth()) # type: ignore[call-arg] + p.TOKEN = "new" # type: ignore[attr-defined] + assert isinstance(p.TOKEN, SecretStr) # type: ignore[attr-defined] + assert p.TOKEN.get_secret_value() == "new" # type: ignore[attr-defined] + + @pytest.mark.unit + @pytest.mark.xfail(reason="Enabled by Change A in Task 2", strict=True) + def test_descriptor_profile_enum_on_setattr(self): + # Fields (TOKEN/MODE/LABEL) are installed at runtime by + # DescriptorProfile.__pydantic_init_subclass__; pyright has no + # static view of them. + p = _SampleProfile(TOKEN="raw", auth=NoAuth()) # type: ignore[call-arg] + p.MODE = "incremental" # type: ignore[attr-defined] + assert p.MODE is _Mode.INCREMENTAL # type: ignore[attr-defined] + + @pytest.mark.unit + @pytest.mark.xfail(reason="Enabled by Change A in Task 2", strict=True) + def test_descriptor_profile_validator_transform_on_setattr(self): + # Fields (TOKEN/MODE/LABEL) are installed at runtime by + # DescriptorProfile.__pydantic_init_subclass__; pyright has no + # static view of them. + p = _SampleProfile(TOKEN="raw", auth=NoAuth()) # type: ignore[call-arg] + p.LABEL = "lower" # type: ignore[attr-defined] + assert p.LABEL == "LOWER" # type: ignore[attr-defined] From 81c998c12f13e5cdeb35aba9422f1e997c5cc134 Mon Sep 17 00:00:00 2001 From: Nathaniel Ramm Date: Sat, 18 Apr 2026 20:37:40 +1000 Subject: [PATCH 4/8] feat(base_settings): enable validate_assignment for canonical semantics MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sets model_config["validate_assignment"] = True on MountainAshBaseSettings. Every setattr on an instance (including via update_settings_from_dict, SettingsManager runtime overrides, and apply_runtime_overrides) now runs the field's declared validator pipeline — enum coercion, SecretStr wrapping, AfterValidator transforms. Addresses the setattr-bypass-limitation backlog item. Removes the bypass mechanism that forced PySparkMode.__setattr__ and ConnectionProfile SecretStr defensive code in consumer packages. Flips the canonical-assignment test class out of xfail. Spec: docs/superpowers/specs/2026-04-18-setattr-bypass-fix-design.md --- src/mountainash_settings/settings/base_settings.py | 3 +-- tests/test_base_settings_coverage.py | 9 --------- 2 files changed, 1 insertion(+), 11 deletions(-) diff --git a/src/mountainash_settings/settings/base_settings.py b/src/mountainash_settings/settings/base_settings.py index a10c373..479b45f 100644 --- a/src/mountainash_settings/settings/base_settings.py +++ b/src/mountainash_settings/settings/base_settings.py @@ -17,8 +17,7 @@ class MountainAshBaseSettings(BaseSettings): extra="ignore", validate_default=False, arbitrary_types_allowed=True, - # validate_assignment=True, - # validate_assignment=False, + validate_assignment=True, ) diff --git a/tests/test_base_settings_coverage.py b/tests/test_base_settings_coverage.py index b69dc7e..8cad973 100644 --- a/tests/test_base_settings_coverage.py +++ b/tests/test_base_settings_coverage.py @@ -684,7 +684,6 @@ class TestCanonicalAssignmentSemantics: """Validate_assignment=True restores pydantic's declared-type contract.""" @pytest.mark.unit - @pytest.mark.xfail(reason="Enabled by Change A in Task 2", strict=True) def test_secretstr_wraps_on_direct_setattr(self): s = _SecretSettings() # Raw-str assignment is the scenario under test: @@ -694,7 +693,6 @@ def test_secretstr_wraps_on_direct_setattr(self): assert s.PASSWORD.get_secret_value() == "plain" @pytest.mark.unit - @pytest.mark.xfail(reason="Enabled by Change A in Task 2", strict=True) def test_enum_coerces_on_direct_setattr(self): s = _EnumSettings() # Raw-str assignment is the scenario under test: @@ -703,14 +701,12 @@ def test_enum_coerces_on_direct_setattr(self): assert s.MODE is _Mode.INCREMENTAL @pytest.mark.unit - @pytest.mark.xfail(reason="Enabled by Change A in Task 2", strict=True) def test_aftervalidator_transforms_on_direct_setattr(self): s = _TransformSettings() s.NAME = "lower" assert s.NAME == "LOWER" @pytest.mark.unit - @pytest.mark.xfail(reason="Enabled by Change A in Task 2", strict=True) def test_update_settings_from_dict_wraps_secretstr(self): s = _SecretSettings() s.update_settings_from_dict({"PASSWORD": "plain"}) @@ -718,21 +714,18 @@ def test_update_settings_from_dict_wraps_secretstr(self): assert s.PASSWORD.get_secret_value() == "plain" @pytest.mark.unit - @pytest.mark.xfail(reason="Enabled by Change A in Task 2", strict=True) def test_update_settings_from_dict_coerces_enum(self): s = _EnumSettings() s.update_settings_from_dict({"MODE": "incremental"}) assert s.MODE is _Mode.INCREMENTAL @pytest.mark.unit - @pytest.mark.xfail(reason="Enabled by Change A in Task 2", strict=True) def test_update_settings_from_dict_applies_transform(self): s = _TransformSettings() s.update_settings_from_dict({"NAME": "lower"}) assert s.NAME == "LOWER" @pytest.mark.unit - @pytest.mark.xfail(reason="Enabled by Change A in Task 2", strict=True) def test_descriptor_profile_secret_on_setattr(self): # Fields (TOKEN/MODE/LABEL) are installed at runtime by # DescriptorProfile.__pydantic_init_subclass__; pyright has no @@ -743,7 +736,6 @@ def test_descriptor_profile_secret_on_setattr(self): assert p.TOKEN.get_secret_value() == "new" # type: ignore[attr-defined] @pytest.mark.unit - @pytest.mark.xfail(reason="Enabled by Change A in Task 2", strict=True) def test_descriptor_profile_enum_on_setattr(self): # Fields (TOKEN/MODE/LABEL) are installed at runtime by # DescriptorProfile.__pydantic_init_subclass__; pyright has no @@ -753,7 +745,6 @@ def test_descriptor_profile_enum_on_setattr(self): assert p.MODE is _Mode.INCREMENTAL # type: ignore[attr-defined] @pytest.mark.unit - @pytest.mark.xfail(reason="Enabled by Change A in Task 2", strict=True) def test_descriptor_profile_validator_transform_on_setattr(self): # Fields (TOKEN/MODE/LABEL) are installed at runtime by # DescriptorProfile.__pydantic_init_subclass__; pyright has no From f638d7f95f20afa73bf30527971f8ceb57222ebc Mon Sep 17 00:00:00 2001 From: Nathaniel Ramm Date: Sat, 18 Apr 2026 20:46:31 +1000 Subject: [PATCH 5/8] refactor(base_settings): remove redundant kwargs re-application in __init__ MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit super().__init__(**valid_attribute_kwargs) already applies kwargs under full pydantic validation. The subsequent update_settings_from_dict(valid_attribute_kwargs) call was overwriting the validated values with raw input — the original source of the setattr-bypass-limitation. Replaces the call plus seven meta-field setattrs with explicit object.__setattr__ writes. Meta-fields are harness bookkeeping, not user config: bypassing validation is intentional and now explicit. SETTINGS_SOURCE_KWARGS is stashed inline here; update_settings_from_dict still stashes it for the SettingsManager and apply_runtime_overrides callsites. Adds a guard test for meta-field bookkeeping. Spec: docs/superpowers/specs/2026-04-18-setattr-bypass-fix-design.md --- .../settings/base_settings.py | 28 +++++++++++-------- tests/test_base_settings_coverage.py | 10 +++++++ 2 files changed, 27 insertions(+), 11 deletions(-) diff --git a/src/mountainash_settings/settings/base_settings.py b/src/mountainash_settings/settings/base_settings.py index 479b45f..7b2638e 100644 --- a/src/mountainash_settings/settings/base_settings.py +++ b/src/mountainash_settings/settings/base_settings.py @@ -93,17 +93,23 @@ def __init__(self, ) - #Update all vals from valid kwargs - self.update_settings_from_dict(settings_dict=valid_attribute_kwargs) - - setattr(self, "SETTINGS_CLASS", local_settings_params.settings_class or MountainAshBaseSettings) - setattr(self, "SETTINGS_CLASS_NAME", local_settings_params.settings_class.__name__ if local_settings_params.settings_class else "MountainAshBaseSettings") - setattr(self, "SETTINGS_SOURCE_ENV_PREFIX", local_settings_params.env_prefix) - setattr(self, "SETTINGS_SOURCE_ENV_FILES", obj_config_files.env_files) - setattr(self, "SETTINGS_SOURCE_YAML_FILES", obj_config_files.yaml_files) - setattr(self, "SETTINGS_SOURCE_TOML_FILES", obj_config_files.toml_files) - setattr(self, "SETTINGS_SOURCE_JSON_FILES", obj_config_files.json_files) - setattr(self, "SETTINGS_SOURCE_SECRETS_DIR", local_settings_params.secrets_dir) + # Meta-field bookkeeping only. super().__init__ above already applied + # valid_attribute_kwargs under full validation — re-applying them via + # update_settings_from_dict would overwrite validated values with raw + # input (see setattr-bypass-limitation spec, 2026-04-18). + # + # object.__setattr__ is intentional: these fields are harness + # bookkeeping, not user config, and with validate_assignment=True on + # model_config we want to skip revalidation on them explicitly. + object.__setattr__(self, "SETTINGS_SOURCE_KWARGS", valid_attribute_kwargs) + object.__setattr__(self, "SETTINGS_CLASS", local_settings_params.settings_class or MountainAshBaseSettings) + object.__setattr__(self, "SETTINGS_CLASS_NAME", local_settings_params.settings_class.__name__ if local_settings_params.settings_class else "MountainAshBaseSettings") + object.__setattr__(self, "SETTINGS_SOURCE_ENV_PREFIX", local_settings_params.env_prefix) + object.__setattr__(self, "SETTINGS_SOURCE_ENV_FILES", obj_config_files.env_files) + object.__setattr__(self, "SETTINGS_SOURCE_YAML_FILES", obj_config_files.yaml_files) + object.__setattr__(self, "SETTINGS_SOURCE_TOML_FILES", obj_config_files.toml_files) + object.__setattr__(self, "SETTINGS_SOURCE_JSON_FILES", obj_config_files.json_files) + object.__setattr__(self, "SETTINGS_SOURCE_SECRETS_DIR", local_settings_params.secrets_dir) # Initialise templated variables self.post_init() diff --git a/tests/test_base_settings_coverage.py b/tests/test_base_settings_coverage.py index 8cad973..1c27391 100644 --- a/tests/test_base_settings_coverage.py +++ b/tests/test_base_settings_coverage.py @@ -752,3 +752,13 @@ def test_descriptor_profile_validator_transform_on_setattr(self): p = _SampleProfile(TOKEN="raw", auth=NoAuth()) # type: ignore[call-arg] p.LABEL = "lower" # type: ignore[attr-defined] assert p.LABEL == "LOWER" # type: ignore[attr-defined] + + @pytest.mark.unit + def test_meta_field_bookkeeping_still_works(self): + """Change B refactors __init__ meta-field writes to + object.__setattr__. Confirm the bookkeeping values still land.""" + from fixtures.settings_classes import TestSettings + s = TestSettings(TEST_VAL_1="x", TEST_VAL_2="y") + assert s.SETTINGS_CLASS is TestSettings + assert s.SETTINGS_CLASS_NAME == "TestSettings" + assert s.SETTINGS_SOURCE_KWARGS == {"TEST_VAL_1": "x", "TEST_VAL_2": "y"} From 800f75c92dfc18de3b000409c10712918727e15f Mon Sep 17 00:00:00 2001 From: Nathaniel Ramm Date: Sat, 18 Apr 2026 20:52:27 +1000 Subject: [PATCH 6/8] test(base_settings): add regression guard for validate_assignment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Explicit assertion that model_config["validate_assignment"] is True on MountainAshBaseSettings. Fails loudly if a future change turns off the canonical assignment contract. Also: - Extends the Task-3 meta-field bookkeeping guard to cover a DescriptorProfile subclass, since descriptor profiles share the same __init__ path. - Clarifies the __init__ comment block: exact spec filename, and explicit note that object.__setattr__ bypasses __pydantic_fields_set__ tracking (intentional, matches pre-Task-2 behaviour — meta-fields are bookkeeping, not model state). Spec: docs/superpowers/specs/2026-04-18-setattr-bypass-fix-design.md --- .../settings/base_settings.py | 15 +++++++- tests/test_base_settings_coverage.py | 36 ++++++++++++++++++- 2 files changed, 49 insertions(+), 2 deletions(-) diff --git a/src/mountainash_settings/settings/base_settings.py b/src/mountainash_settings/settings/base_settings.py index 7b2638e..a57e0b0 100644 --- a/src/mountainash_settings/settings/base_settings.py +++ b/src/mountainash_settings/settings/base_settings.py @@ -12,6 +12,15 @@ T = TypeVar('T', BaseSettings, 'MountainAshBaseSettings') class MountainAshBaseSettings(BaseSettings): + """Base settings class with template support, multi-format config files, + and smart caching. + + Assignments to declared fields after construction are revalidated via + pydantic's field-validator pipeline — ``SecretStr`` wrapping, enum + coercion, and ``AfterValidator`` transforms all run on every ``setattr``. + This is canonical pydantic v2 behaviour; see + ``docs/superpowers/specs/2026-04-18-setattr-bypass-fix-design.md``. + """ model_config = SettingsConfigDict( extra="ignore", @@ -96,11 +105,15 @@ def __init__(self, # Meta-field bookkeeping only. super().__init__ above already applied # valid_attribute_kwargs under full validation — re-applying them via # update_settings_from_dict would overwrite validated values with raw - # input (see setattr-bypass-limitation spec, 2026-04-18). + # input (see 2026-04-18-setattr-bypass-fix-design.md). # # object.__setattr__ is intentional: these fields are harness # bookkeeping, not user config, and with validate_assignment=True on # model_config we want to skip revalidation on them explicitly. + # Note: this also bypasses __pydantic_fields_set__ tracking, so these + # meta-fields do not appear in model_fields_set and are dropped by + # model_dump(exclude_unset=True). That matches the pre-Task-2 + # behaviour and is intentional — bookkeeping is not model state. object.__setattr__(self, "SETTINGS_SOURCE_KWARGS", valid_attribute_kwargs) object.__setattr__(self, "SETTINGS_CLASS", local_settings_params.settings_class or MountainAshBaseSettings) object.__setattr__(self, "SETTINGS_CLASS_NAME", local_settings_params.settings_class.__name__ if local_settings_params.settings_class else "MountainAshBaseSettings") diff --git a/tests/test_base_settings_coverage.py b/tests/test_base_settings_coverage.py index 1c27391..70e1044 100644 --- a/tests/test_base_settings_coverage.py +++ b/tests/test_base_settings_coverage.py @@ -756,9 +756,43 @@ def test_descriptor_profile_validator_transform_on_setattr(self): @pytest.mark.unit def test_meta_field_bookkeeping_still_works(self): """Change B refactors __init__ meta-field writes to - object.__setattr__. Confirm the bookkeeping values still land.""" + object.__setattr__. Confirm the bookkeeping values still land on + both direct MountainAshBaseSettings subclasses and + DescriptorProfile subclasses (which inherit the __init__ path).""" from fixtures.settings_classes import TestSettings s = TestSettings(TEST_VAL_1="x", TEST_VAL_2="y") assert s.SETTINGS_CLASS is TestSettings assert s.SETTINGS_CLASS_NAME == "TestSettings" assert s.SETTINGS_SOURCE_KWARGS == {"TEST_VAL_1": "x", "TEST_VAL_2": "y"} + + # DescriptorProfile subclasses inherit MountainAshBaseSettings.__init__, + # so the same meta-field bookkeeping must land on them too. + # (SETTINGS_SOURCE_KWARGS is not asserted here — profile construction + # passes `auth` and SecretStr-wrapped fields, producing a post-validation + # kwargs shape that differs from the raw dict. The CLASS/CLASS_NAME + # assertions are sufficient witnesses that the __init__ path ran.) + p = _SampleProfile(TOKEN="raw", auth=NoAuth()) # type: ignore[call-arg] + assert p.SETTINGS_CLASS is _SampleProfile + assert p.SETTINGS_CLASS_NAME == "_SampleProfile" + + @pytest.mark.unit + def test_validate_assignment_is_enabled(self): + """Regression guard — canonical assignment validation must stay on. + + If this assertion fires, someone disabled validate_assignment on + MountainAshBaseSettings. Do not 'fix' by deleting this test. + See docs/superpowers/specs/2026-04-18-setattr-bypass-fix-design.md + """ + assert MountainAshBaseSettings.model_config.get("validate_assignment") is True + + @pytest.mark.unit + def test_validate_assignment_is_enabled_on_descriptor_profile(self): + """Regression guard — DescriptorProfile must not override + model_config in a way that drops validate_assignment. + + Pydantic's model_config is a class attribute, so a subclass that + redeclares it fully shadows the parent. validate_assignment=True + must be carried forward explicitly (or the subclass must leave + model_config alone and inherit via MRO). + """ + assert DescriptorProfile.model_config.get("validate_assignment") is True From 18de5ad445697ee88abe8425db9c8dc0113fae2b Mon Sep 17 00:00:00 2001 From: Nathaniel Ramm Date: Sat, 18 Apr 2026 23:38:19 +1000 Subject: [PATCH 7/8] chore(tooling): pin project venv + add pyright config MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - hatch.toml: set path = ".venv" under [envs.default] so the project env is non-ephemeral and predictable (per repo convention). - pyrightconfig.json: new — pins venvPath/venv to ".venv", includes src + tests, basic type-checking mode, keyed to Python 3.10. - .gitignore: ignore local .hiivmind/ (user-specific workspace symlinks to hiivmind-pulse-gh config outside the repo). Co-Authored-By: Claude Opus 4.7 (1M context) --- .gitignore | 3 +++ hatch.toml | 1 + pyrightconfig.json | 54 ++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 58 insertions(+) create mode 100644 pyrightconfig.json diff --git a/.gitignore b/.gitignore index 495a987..522a3d8 100644 --- a/.gitignore +++ b/.gitignore @@ -175,3 +175,6 @@ htmlcov/ #testing artifacts junit.* coverage.* + +# Local hiivmind workspace symlinks (user-specific) +.hiivmind/ diff --git a/hatch.toml b/hatch.toml index d3d69b5..6f8b2fc 100644 --- a/hatch.toml +++ b/hatch.toml @@ -28,6 +28,7 @@ export-requirements = "hatch dep show requirements > ./requirements.txt" #================ [envs.default] installer = "uv" +path = ".venv" dependencies = [] #================ diff --git a/pyrightconfig.json b/pyrightconfig.json new file mode 100644 index 0000000..e0c6a17 --- /dev/null +++ b/pyrightconfig.json @@ -0,0 +1,54 @@ +{ + "venvPath": ".", + "venv": ".venv", + + "include": ["src", "tests"], + "exclude": [ + "**/__pycache__", + "**/.venv", + "**/build", + "**/dist", + "**/node_modules", + "docs/superpowers", + ], + "extraPaths": ["src"], + + "pythonVersion": "3.12", + "pythonPlatform": "Linux", + + "typeCheckingMode": "basic", + "useLibraryCodeForTypes": true, + + "reportMissingImports": "error", + "reportUndefinedVariable": "error", + "reportInvalidTypeForm": "error", + "reportAssignmentType": "warning", + "reportReturnType": "warning", + "reportArgumentType": "warning", + "reportCallIssue": "warning", + "reportAttributeAccessIssue": "warning", + "reportOptionalMemberAccess": "warning", + "reportOptionalSubscript": "warning", + "reportOperatorIssue": "warning", + "reportIndexIssue": "warning", + + "reportUnusedImport": "none", + "reportUnusedVariable": "none", + "reportUnusedFunction": "none", + "reportUnusedClass": "none", + "reportPrivateImportUsage": "none", + "reportImportCycles": "none", + + "reportIncompatibleMethodOverride": "warning", + "reportIncompatibleVariableOverride": "warning", + "reportGeneralTypeIssues": "warning", + + "executionEnvironments": [ + { + "root": "tests", + "reportPrivateUsage": "none", + "reportMissingTypeStubs": "none", + "reportUntypedFunctionDecorator": "none", + }, + ], +} From cd21af4849fc1278347b18cbb93d4c21be72409f Mon Sep 17 00:00:00 2001 From: Nathaniel Ramm Date: Sat, 18 Apr 2026 23:42:18 +1000 Subject: [PATCH 8/8] release: v26.4.1 First production release from the 26.4.0 RC line. Headlines: - Restores canonical pydantic v2 assignment semantics on MountainAshBaseSettings (validate_assignment=True). SecretStr wrapping, enum coercion, and AfterValidator transforms now run on every post-construction setattr. - Removes the redundant update_settings_from_dict re-application from __init__; seven meta-field writes converted to explicit object.__setattr__ for harness bookkeeping. - Adds regression guards for both MountainAshBaseSettings and DescriptorProfile. - Pins project venv path in hatch.toml and ships pyrightconfig.json. Closes the setattr-bypass-limitation backlog item. Downstream consumer cleanup (PySparkMode.__setattr__, ConnectionProfile SecretStr unwrap guard, adapter str(enum) defensive code) tracked as follow-up. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/mountainash_settings/__version__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mountainash_settings/__version__.py b/src/mountainash_settings/__version__.py index e233240..c4c67b1 100644 --- a/src/mountainash_settings/__version__.py +++ b/src/mountainash_settings/__version__.py @@ -1 +1 @@ -__version__="26.4.0" \ No newline at end of file +__version__="26.4.1" \ No newline at end of file