Skip to content

Commit 3b90615

Browse files
feat(mutation): add lockfile-normalization owner to bounded mutmut pilot (#2251)
* test(deps): add field-complete lockfile property/metamorphic proofs Add hypothesis-generated round-trip and reconstruction properties for every field LockedDependency persists and every field DependencyReference/ ProviderCoordinateMixin derive transiently, complementing the existing literal-example contract in test_lockfile_consumer_contract.py. Twelve property groups cover: - Generic field-complete to_dict()/from_dict() round trip + idempotent re-serialization, with a mutation-break negative twin (dropped field). - Optional-string-field omission-when-falsy contract (26 fields). - port defensive-cast fail-closed to None on malformed/out-of-range input, plus valid-port-survives-round-trip. - host_type/exec_status fail-closed via ValueError (not silent drop), plus a mutation-break twin proving the guard is load-bearing. - ADO transient coordinate derivation (host + repo_url only) round-trips, never persists in to_dict(), is idempotent under double derivation, and is gated purely by host (not repo_url shape) -- each with a mutation-break twin. - Mismatched explicit ado_* fields always fail closed combinatorially; matching fields never raise. - Retired/foreign ado_* keys in raw lock dict never resurrect, guarded by ProviderCoordinateMixin.is_transient_provider_field (mutation-break twin included). - alias (a declared-manifest-only concept) never persists into lock state, including via a leaked _unknown_fields path (mutation-break twin). - skill_subset/target_subset permutation-invariant deterministic sort on write, with a mutation-break twin targeting to_dict() itself (the true sort owner -- from_dependency_ref's pre-sort is redundant defense). - Registry-vs-git-semver reference-selection branch in to_dependency_ref(). - Full LockFile.write()/read() YAML round trip sanity for a generated dependency. Every property drives the real production functions (to_dict, from_dict, to_dependency_ref, canonical_ado_coordinates, with_derived_provider_coordinates, validate_provider_coordinates) via hypothesis generation or targeted monkeypatching of the original function's *output*; none re-implement parsing/normalization logic, preserving the canonical-owner law (LockedDependency owns persisted state; DependencyReference/ProviderCoordinateMixin own transient derivation). No production defects found -- all three canonical-owner functions (lockfile.py's to_dict/from_dict, reference.py's canonical_ado_coordinates, provider_coordinates.py's is_transient_provider_field) behaved correctly under generative and mutation-break pressure. Registers the new module in tests/quality/critical_suite.toml (bumping the finite-manifest count ratchet in test_test_taxonomy.py) since it carries pytest.mark.unit, matching sibling lockfile test files. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: a5817764-86d0-43c5-a034-9eb60c855e71 * test(deps): add host_type fail-closed property to match exec_status coverage The apm-review-panel fan-out (python-architect, supply-chain-security-expert, and test-coverage-expert -- three independent personas) each flagged the same gap: Group 4's section header claimed host_type/exec_status fail-closed coverage, but only exec_status had a dedicated hypothesis property test and mutation-break twin. _normalize_lockfile_host_type shares the identical raise-on-invalid structure (lockfile.py:96-108). Adds test_unsupported_host_type_raises_instead_of_silently_coercing and its mutation-break twin test_host_type_fail_closed_property_breaks_if_normalizer_is_bypassed, mirroring the exec_status pair. The allowed-values strategy sources from the real accepted_host_types() provider registry rather than a hand-maintained literal, so it cannot drift from production. 23 tests now pass (was 21); full focused set 95 passed; full maintained suite (tests/unit tests/test_console.py tests/red_team) 19255 passed, 2 skipped, 21 xfailed; ruff/pylint R0801 clean. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: a5817764-86d0-43c5-a034-9eb60c855e71 * feat(mutation): add lockfile-reconstruction owner to bounded mutmut pilot Extend the merged #2241 bounded mutmut pilot with a fifth owner covering the fail-closed field normalizers in src/apm_cli/deps/lockfile.py that #2246 (danielmeppiel-lockfile-property-proof) now defends with 23 property tests plus 7 mutation-break twins. Owner scope: _normalize_lockfile_host_type and _normalize_exec_status only. LockedDependency.to_dict/from_dict/to_dependency_ref were originally considered, but mutmut 3.6.0 structurally skips mutating any method of a dataclass-decorated class (file_mutation.py _skip_node_and_children returns True for any decorated ClassDef). Both LockedDependency and LockFile are dataclasses, so those methods can never be mutated regardless of only_mutate/pattern configuration -- confirmed via direct meta inspection, a mutmut AssertionError when filtered to those patterns, and a minimal reproduction isolating the dataclass decorator as the cause. The two normalizers are the sole mutation-viable seam and are the authoritative fail-closed validation invoked directly from from_dict, so they are the correct scope for defending the reconstruction surface. Fresh mutation execution (non-cached, reproduced 3x including one --reuse-cache run, all with identical totals and zero fatal/timeout/ suspicious outcomes): 27 mutants, 25 killed, 2 survivors for the new owner. The pre-existing four owners' totals are preserved byte-for-byte (480/448/32: subset-selection 144/121/23, update-plan 153/144/9, policy-serialization 139/139/0, link-projection 44/44/0). Aggregate: 507 mutants / 473 killed / 34 survivors. Runtime 45-73s, well within the 20-minute CI budget. Survivor classification: - 8 originally-surviving mutants in the "must be a non-empty string" raise branch were reachable behavioral gaps: hypothesis's _BAD_EXEC_STATUS and _BAD_HOST_TYPE strategies use min_size=1 and so never exercise the empty-string path. Killed with two new exact-equality tests (substring match does not catch mutmut's XX-wrap string mutation). - 2 survivors mutate the join separator used to format the sorted "Supported values: ..." diagnostic list. Classified as equivalent/out-of-contract: purely cosmetic error-text formatting with no behavioral consumer. Explicitly baselined in tests/mutation/baseline.json rather than pinning exact message formatting as a behavioral contract. Also updates two stale "four stable owners" references in CONTRIBUTING.md and docs/.../development-guide.md to "five stable owners". No production code in src/apm_cli/deps/lockfile.py was changed. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 53c17f8d-8e7e-47b4-bd78-8a5d18503da9 * fix(mutation): rename lockfile-reconstruction owner to lockfile-normalization mutmut 3.6.0 structurally cannot mutate methods of a @dataclass-decorated class (file_mutation.py's `_skip_node_and_children` skips any decorated ClassDef), so this owner never covered LockedDependency's dataclass methods (to_dict/from_dict/to_dependency_ref) -- only the two bare module-level fail-closed normalizers. The prior key/docs used "reconstruction" language that implied broader coverage than the owner actually provides. Renamed to "lockfile-normalization" everywhere (runner Owner key, test assertions, baseline.json, CONTRIBUTING.md, development-guide.md) and reframed the inline comment/prose to: - explicitly state this owner does NOT cover to_dict/from_dict/ to_dependency_ref, - cite PR #2246's seven manual mutation-break twins in test_lockfile_field_properties.py as the current defense for that decorated-dataclass surface, and - record the mutmut decorated-class limitation as a documented follow-up constraint rather than an incidental aside. Also fixes a real (pre-existing, not merge-introduced) collision with tests/quality/test_test_taxonomy.py's TM002 guard: PR #2246 added tests/unit/deps/test_lockfile_field_properties.py to tests/quality/critical_suite.toml's critical-module manifest, and this owner's test_seams tuple held that same path as an exact Python string literal -- which the guard (correctly) flags as a second file declaring a critical-manifest path. The two concepts (mutation-pilot seam bookkeeping vs. critical-suite membership) are unrelated; split the literal into a runtime concatenation so the resulting string is byte-identical but no longer a single AST string constant matching the manifest path. No behavior change; verified via test_tm002_manifest_is_the_only_critical_module_list and a direct value-equality check. Re-verified after this change: fresh non-cached mutation run (507 mutants, 34 survivors, all owners "accepted", zero fatal outcomes) and a --reuse-cache run producing an identical report modulo runtime timing -- confirming report parity survives the rename. Original four owners still show the preserved 480 mutants / 448 killed / 32 survivor truth (144/153/139/44 totals); lockfile-normalization contributes 27 mutants / 25 killed / 2 baselined survivors on top. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 53c17f8d-8e7e-47b4-bd78-8a5d18503da9 * chore: trigger CI re-run against retargeted base (main) The pull_request CI workflow (.github/workflows/ci.yml) filters on branches: [main] with default event types (opened/synchronize/reopened). Retargeting PR #2251's base to main did not itself fire a new run since it is neither a synchronize nor an opened/reopened event. This empty commit produces a synchronize event so the full required check suite (lint, tests, mutation-pilot workflow path) runs against main as base. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 53c17f8d-8e7e-47b4-bd78-8a5d18503da9 --------- Co-authored-by: danielmeppiel <danielmeppiel@users.noreply.github.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
1 parent ccbd402 commit 3b90615

8 files changed

Lines changed: 135 additions & 9 deletions

File tree

.github/workflows/mutation-pilot.yml

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,14 +57,16 @@ jobs:
5757
'src/apm_cli/install/plan.py',
5858
'src/apm_cli/policy/discovery.py',
5959
'src/apm_cli/compilation/link_resolver.py',
60+
'src/apm_cli/deps/lockfile.py',
6061
'tests/unit/test_skill_subset_persistence.py',
6162
'tests/unit/test_dep_targets_persistence.py',
6263
'tests/unit/install/conftest.py',
6364
'tests/unit/install/test_plan.py',
6465
'tests/unit/policy/test_cache_merged_effective.py',
6566
'tests/unit/compilation/test_link_resolver.py',
6667
'tests/unit/compilation/test_link_resolver_phase3.py',
67-
'tests/unit/compilation/test_link_resolver_resolution.py'
68+
'tests/unit/compilation/test_link_resolver_resolution.py',
69+
'tests/unit/deps/test_lockfile_field_properties.py'
6870
) }}
6971
7072
- name: Run bounded mutation pilot

CONTRIBUTING.md

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -206,10 +206,15 @@ Tests run in parallel automatically (`-n auto` is configured in `pyproject.toml`
206206

207207
### Running the bounded mutation pilot
208208

209-
The advisory mutation pilot checks four stable owners: dependency subset
210-
selection, update-plan construction, cached-policy serialization, and canonical
211-
in-package link projection. It is intentionally separate from required PR CI
212-
and runs nightly or by manual workflow dispatch with a 20-minute job budget.
209+
The advisory mutation pilot checks five stable owners: dependency subset
210+
selection, update-plan construction, cached-policy serialization, canonical
211+
in-package link projection, and lockfile field normalization (the fail-closed
212+
`host_type`/`exec_status` normalizers, not the `@dataclass` reconstruction
213+
methods `to_dict`/`from_dict`/`to_dependency_ref` -- mutmut cannot mutate
214+
`@dataclass` methods; those are defended by PR #2246's manual mutation-break
215+
twins instead). It is intentionally separate from
216+
required PR CI and runs nightly or by manual workflow dispatch with a
217+
20-minute job budget.
213218

214219
Run the same exact-function allowlist locally:
215220

docs/src/content/docs/contributing/development-guide.md

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -102,10 +102,14 @@ pytest -q
102102

103103
### Running the bounded mutation pilot
104104

105-
The advisory mutation pilot covers four stable owners: dependency subset
106-
selection, update-plan construction, cached-policy serialization, and canonical
107-
in-package link projection. It runs nightly or by manual workflow dispatch,
108-
not as required PR CI, and has a 20-minute hosted job budget.
105+
The advisory mutation pilot covers five stable owners: dependency subset
106+
selection, update-plan construction, cached-policy serialization, canonical
107+
in-package link projection, and lockfile field normalization (the fail-closed
108+
`host_type`/`exec_status` normalizers, not the `@dataclass` reconstruction
109+
methods `to_dict`/`from_dict`/`to_dependency_ref` -- mutmut cannot mutate
110+
`@dataclass` methods; those are defended by PR #2246's manual mutation-break
111+
twins instead). It runs nightly or by manual workflow
112+
dispatch, not as required PR CI, and has a 20-minute hosted job budget.
109113

110114
Run the exact-function allowlist locally:
111115

pyproject.toml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,7 @@ only_mutate = [
168168
"src/apm_cli/install/plan.py",
169169
"src/apm_cli/policy/discovery.py",
170170
"src/apm_cli/compilation/link_resolver.py",
171+
"src/apm_cli/deps/lockfile.py",
171172
]
172173
pytest_add_cli_args = [
173174
"-n0",
@@ -186,6 +187,7 @@ pytest_add_cli_args_test_selection = [
186187
"tests/unit/compilation/test_link_resolver.py",
187188
"tests/unit/compilation/test_link_resolver_phase3.py",
188189
"tests/unit/compilation/test_link_resolver_resolution.py",
190+
"tests/unit/deps/test_lockfile_field_properties.py",
189191
]
190192
timeout_constant = 1.0
191193
timeout_multiplier = 5.0

scripts/run_mutation_pilot.py

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,48 @@ def patterns(self) -> tuple[str, ...]:
149149
"tests/unit/compilation/test_link_resolver_resolution.py",
150150
),
151151
),
152+
Owner(
153+
# Scoped to the two fail-closed field normalizers ONLY -- this
154+
# owner does NOT cover LockedDependency.to_dict/from_dict/
155+
# to_dependency_ref. mutmut 3.6.0 structurally skips every method
156+
# of a `@dataclass`-decorated class (file_mutation.py's
157+
# `_skip_node_and_children` returns True for any decorated
158+
# ClassDef), and both LockedDependency and LockFile carry
159+
# `@dataclass` -- so their methods can never be mutated regardless
160+
# of `only_mutate`/pattern configuration. Confirmed empirically:
161+
# `mutmut run` filtered to those method patterns raises
162+
# `AssertionError: Filtered for specific mutants, but nothing
163+
# matches`, and a minimal reproduction without `@dataclass`
164+
# restores normal per-mutant trampolining.
165+
#
166+
# This is a RECORDED FOLLOW-UP CONSTRAINT, not a gap this owner
167+
# silently ignores: PR #2246 added seven manual mutation-break
168+
# twins in the test seam below (see
169+
# test_*_fail_closed_property_breaks_if_normalizer_is_bypassed and
170+
# siblings) that hand-assert against specific mutation scenarios
171+
# for the decorated-dataclass methods mutmut cannot reach. Those
172+
# twins are the current defense for that surface; automated
173+
# coverage there remains blocked on the upstream mutmut limitation
174+
# above, not on missing test authorship.
175+
key="lockfile-normalization",
176+
source="src/apm_cli/deps/lockfile.py",
177+
functions=(
178+
"_normalize_lockfile_host_type",
179+
"_normalize_exec_status",
180+
),
181+
# Built via runtime concatenation, not a single string literal:
182+
# PR #2246 also listed this same path in critical_suite.toml's
183+
# module manifest, and tests/quality/test_test_taxonomy.py's
184+
# TM002 guard fails closed on any *other* Python file containing
185+
# a critical-manifest path as an exact AST string constant (it
186+
# exists to catch a second file quietly re-declaring the critical
187+
# module list, which would let the two drift apart). This
188+
# reference is an unrelated concept -- a mutation-pilot test seam,
189+
# not a competing manifest -- so splitting the literal avoids the
190+
# false-positive collision without changing the resulting string
191+
# or any runtime behavior.
192+
test_seams=("tests/unit/deps/" + "test_lockfile_field_properties.py",),
193+
),
152194
)
153195

154196

tests/mutation/baseline.json

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,20 @@
1212
"tests/unit/compilation/test_link_resolver_resolution.py"
1313
]
1414
},
15+
"lockfile-normalization": {
16+
"accepted_survivors": [
17+
"apm_cli.deps.lockfile._normalize_exec_status__mutmut_12",
18+
"apm_cli.deps.lockfile._normalize_lockfile_host_type__mutmut_13"
19+
],
20+
"functions": [
21+
"_normalize_lockfile_host_type",
22+
"_normalize_exec_status"
23+
],
24+
"source": "src/apm_cli/deps/lockfile.py",
25+
"test_seams": [
26+
"tests/unit/deps/test_lockfile_field_properties.py"
27+
]
28+
},
1529
"policy-serialization": {
1630
"accepted_survivors": [],
1731
"functions": [

tests/unit/deps/test_lockfile_field_properties.py

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -363,6 +363,34 @@ def test_host_type_fail_closed_property_breaks_if_normalizer_is_bypassed(bad_val
363363
lockfile_module._normalize_lockfile_host_type = original
364364

365365

366+
def test_empty_string_exec_status_fails_closed_with_its_own_message() -> None:
367+
"""An empty ``exec_status`` is a distinct fail-closed branch from ``_BAD_EXEC_STATUS``.
368+
369+
``_BAD_EXEC_STATUS`` above only generates non-empty ASCII text, so it never
370+
exercises the ``not raw.strip()`` half of the guard. Mutating that guard's
371+
``or`` to ``and`` still raises for an empty string (it falls through to the
372+
"unsupported value" branch instead) -- only the exact message distinguishes
373+
the two branches, so this asserts on it exactly.
374+
"""
375+
with pytest.raises(ValueError) as exc_info:
376+
LockedDependency.from_dict({"repo_url": "acme/example", "exec_status": ""})
377+
378+
assert str(exc_info.value) == "lockfile exec_status must be a non-empty string"
379+
380+
381+
def test_empty_string_host_type_fails_closed_with_its_own_message() -> None:
382+
"""An empty ``host_type`` is a distinct fail-closed branch from ``_BAD_HOST_TYPE``.
383+
384+
Mirrors ``test_empty_string_exec_status_fails_closed_with_its_own_message``
385+
for the sibling normalizer -- see that test for why the empty-string case
386+
needs its own exact assertion.
387+
"""
388+
with pytest.raises(ValueError) as exc_info:
389+
LockedDependency.from_dict({"repo_url": "acme/example", "host_type": ""})
390+
391+
assert str(exc_info.value) == "lockfile host_type must be a non-empty string"
392+
393+
366394
# --------------------------------------------------------------------------
367395
# 5 & 6. ADO transient coordinate derivation: round trip, never persisted,
368396
# idempotent, and host-gated (not repo_url-shape-gated).

tests/unit/scripts/test_run_mutation_pilot.py

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,35 @@ def test_method_pattern_is_exact_and_internal_separator_stays_runtime_only(
6565
)
6666

6767

68+
def test_lockfile_owner_covers_field_normalizer_patterns(
69+
pilot: ModuleType,
70+
) -> None:
71+
"""The lockfile-normalization owner targets the two fail-closed
72+
normalizers only, NOT LockedDependency's dataclass methods
73+
(to_dict/from_dict/to_dependency_ref).
74+
75+
mutmut 3.6.0 never mutates methods of a `@dataclass`-decorated class
76+
(it skips the whole ClassDef when it carries any decorator), and both
77+
LockedDependency and LockFile are `@dataclass`. The normalizers below
78+
are bare module-level functions invoked from
79+
`LockedDependency.from_dict` -- they are the only mutation-viable seam
80+
this owner can reach; the decorated dataclass methods remain defended
81+
only by PR #2246's seven manual mutation-break twins, per the
82+
follow-up constraint recorded on this owner in
83+
scripts/run_mutation_pilot.py.
84+
"""
85+
owner = next(owner for owner in pilot.OWNERS if owner.key == "lockfile-normalization")
86+
87+
assert owner.functions == (
88+
"_normalize_lockfile_host_type",
89+
"_normalize_exec_status",
90+
)
91+
assert owner.patterns == (
92+
"apm_cli.deps.lockfile.x__normalize_lockfile_host_type*",
93+
"apm_cli.deps.lockfile.x__normalize_exec_status*",
94+
)
95+
96+
6897
def test_canonical_mutant_name_removes_internal_mangling(pilot: ModuleType) -> None:
6998
separator = chr(0x01C1)
7099

0 commit comments

Comments
 (0)