refactor: collapse extends' duplicate list helper onto keys.py

compose2pod/extends.py

@@ -3,7 +3,7 @@
 from typing import Any
 
 from compose2pod.exceptions import UnsupportedComposeError
-from compose2pod.keys import SERVICE_KEYS, pairs_to_mapping
+from compose2pod.keys import SERVICE_KEYS, concat_list, pairs_to_mapping
 
 
 # Merge policy for keys with a SERVICE_KEYS KeySpec comes from spec.merge (see
@@ -39,7 +39,14 @@ def _extends_target(name: str, ext: Any) -> str:  # noqa: ANN401 - Compose value
     return service
 
 
-def _as_mapping(key: str, name: str, value: Any) -> dict[str, Any]:  # noqa: ANN401 - Compose values are untyped
+def _as_mapping(name: str, key: str, value: Any) -> dict[str, Any]:  # noqa: ANN401 - Compose values are untyped
+    """Normalize a structural mapping-merge key's value to a mapping.
+
+    Deliberately stricter than `keys.pairs_to_mapping`: list form is accepted
+    only for `environment` and `depends_on`, the two keys Compose actually
+    defines a list form for. `extra_hosts`/`healthcheck` in list form on a
+    merged side are refused as an incompatible form rather than coerced.
+    """
     if isinstance(value, dict):
         return value
     if isinstance(value, list):
@@ -55,15 +62,6 @@ def _as_mapping(key: str, name: str, value: Any) -> dict[str, Any]:  # noqa: ANN
     raise UnsupportedComposeError(msg)
 
 
-def _as_list(key: str, name: str, value: Any) -> list[Any]:  # noqa: ANN401 - Compose values are untyped
-    if isinstance(value, list):
-        return list(value)
-    if isinstance(value, str):
-        return [value]
-    msg = f"service {name!r}: cannot merge {key!r} across incompatible forms"
-    raise UnsupportedComposeError(msg)
-
-
 def _merge(base: dict[str, Any], local: dict[str, Any], name: str) -> dict[str, Any]:
     """Merge `local` onto `base` per key category: mapping-merge, sequence-concat, else override."""
     merged: dict[str, Any] = dict(base)
@@ -72,9 +70,9 @@ def _merge(base: dict[str, Any], local: dict[str, Any], name: str) -> dict[str,
         if key in base and spec is not None and spec.merge is not None:
             merged[key] = spec.merge(name, key, base[key], local_val)
         elif key in base and key in _STRUCTURAL_MERGE_KEYS:
-            merged[key] = {**_as_mapping(key, name, base[key]), **_as_mapping(key, name, local_val)}
+            merged[key] = {**_as_mapping(name, key, base[key]), **_as_mapping(name, key, local_val)}
         elif key in base and key in _STRUCTURAL_CONCAT_KEYS:
-            merged[key] = _as_list(key, name, base[key]) + _as_list(key, name, local_val)
+            merged[key] = concat_list(name, key, base[key], local_val)
         else:
             merged[key] = local_val
     return merged

compose2pod/keys.py

@@ -147,7 +147,7 @@ def validate_map(name: str, key: str, value: Any) -> None:  # noqa: ANN401 - Com
     raise UnsupportedComposeError(msg)
 
 
-def _as_list(name: str, key: str, value: Any) -> list[Any]:  # noqa: ANN401 - Compose values are untyped YAML/JSON
+def as_list(name: str, key: str, value: Any) -> list[Any]:  # noqa: ANN401 - Compose values are untyped YAML/JSON
     """Normalize list-or-scalar-string form to a list, for merging across extends."""
     if isinstance(value, list):
         return list(value)
@@ -157,9 +157,9 @@ def _as_list(name: str, key: str, value: Any) -> list[Any]:  # noqa: ANN401 - Co
     raise UnsupportedComposeError(msg)
 
 
-def _concat_list(name: str, key: str, base: Any, local: Any) -> list[Any]:  # noqa: ANN401 - Compose values are untyped YAML/JSON
+def concat_list(name: str, key: str, base: Any, local: Any) -> list[Any]:  # noqa: ANN401 - Compose values are untyped YAML/JSON
     """Merge policy for list-shaped keys: concatenate base then local."""
-    return _as_list(name, key, base) + _as_list(name, key, local)
+    return as_list(name, key, base) + as_list(name, key, local)
 
 
 def pairs_to_mapping(name: str, key: str, value: Any) -> dict[str, Any]:  # noqa: ANN401 - Compose values are untyped YAML/JSON
@@ -212,7 +212,7 @@ def emit(value: Any) -> list[Token]:  # noqa: ANN401 - Compose values are untype
             tokens += [flag, Expand(value=str(item))]
         return tokens
 
-    return KeySpec(validate=_validate_list, emit=emit, merge=_concat_list)
+    return KeySpec(validate=_validate_list, emit=emit, merge=concat_list)
 
 
 def _map(flag: str) -> KeySpec:

planning/changes/2026-07-14.03-extends-keys-dedup.md

@@ -0,0 +1,89 @@
+---
+summary: extends.py drops its private copy of keys.py's list-normalizing helper and its inline concat, using the promoted keys.concat_list instead; the two byte-identical helpers with swapped parameters are gone.
+---
+
+# Design: Collapse extends' duplicate merge helpers onto keys.py
+
+## Summary
+
+`extends.py` carries a private `_as_list` that is byte-identical to `keys.py`'s
+`_as_list` — same body, same error message — **with the first two parameters
+swapped**. Delete the copy, promote the `keys.py` original to the module's
+public primitive set, and have `extends` use it.
+
+## Motivation
+
+The two functions today:
+
+```python
+# compose2pod/extends.py
+def _as_list(key: str, name: str, value: Any) -> list[Any]:
+    if isinstance(value, list):
+        return list(value)
+    if isinstance(value, str):
+        return [value]
+    msg = f"service {name!r}: cannot merge {key!r} across incompatible forms"
+    raise UnsupportedComposeError(msg)
+
+# compose2pod/keys.py
+def _as_list(name: str, key: str, value: Any) -> list[Any]:
+    ...identical body, identical message...
+```
+
+Same name, same behavior, opposite parameter order. Each call site is correct
+only because it happens to match its own module's local signature — nothing
+catches a mix-up but the error message coming out with `name` and `key`
+transposed, which no test asserts on. This is a latent footgun sitting in the
+`extends` merge path, and `extends` runs *ahead of the gate*
+(`cli.py` calls `resolve_extends()` before `validate()`), which is precisely
+where this codebase has already been bitten.
+
+`extends._merge` also open-codes the same two merge policies `keys.py` already
+names: `_as_list(base) + _as_list(local)` is `keys._concat_list`.
+
+## Design
+
+`keys.py` is already the home of the cross-module primitives — `key_value_pairs`,
+`pairs_to_mapping`, `validate_map`, `require_string_keys`, `extra_host_pairs`,
+`is_number` — all on the same `(name, key, value)` signature
+(`2026-07-13.07-public-keys-primitives`). Two more join them:
+
+- `_as_list` → **`as_list(name, key, value)`**
+- `_concat_list` → **`concat_list(name, key, base, local)`**
+
+`extends.py` then deletes its `_as_list` and calls `concat_list` for its
+sequence-concatenate keys. One definition, one parameter order.
+
+**`extends._as_mapping` stays.** It is *not* duplication: it is deliberately
+stricter than `keys.pairs_to_mapping`, accepting list form only for
+`environment` and `depends_on` and refusing it for `extra_hosts`/`healthcheck`
+rather than coercing. Collapsing it onto `pairs_to_mapping` would silently start
+coercing list-form `extra_hosts` on a merged side — a behavior change, not a
+dedup. Its parameter order is corrected to `(name, key, value)` to match every
+other helper, removing the second half of the footgun.
+
+No structural-key registry (`decisions/2026-07-12-reject-structural-key-registry.md`
+stands): this moves two helpers, it does not build a dispatch table.
+
+## Non-goals
+
+- **No behavior change.** Every accepted document still merges identically and
+  every rejected one still raises the same message. This is a pure refactor; if
+  a test needs changing, the refactor is wrong.
+- Not unifying structural-key merge policy — the asymmetry where a registry key
+  (`labels`) coerces list form on merge while a structural key (`extra_hosts`)
+  refuses it is real, but it is a *policy* question, not a duplication one, and
+  it stays deferred behind `decisions/2026-07-12`'s revisit trigger.
+
+## Testing
+
+`just test-ci` at 100%, unchanged. The existing `tests/test_extends.py` merge
+suite is the regression net: a pure refactor must leave all of it green without
+edits. Add one test pinning the error message's `name`/`key` order, so a future
+transposition fails loudly rather than silently producing a garbled message.
+
+## Risk
+
+- **A silent parameter transposition during the edit** — exactly the bug being
+  removed. Mitigated by the message-order test above and by the existing merge
+  suite, which covers both the concat and the incompatible-form paths.

tests/test_extends.py

@@ -305,3 +305,32 @@ def test_non_string_key_merged_into_environment_across_extends_passes_through(se
         }
         merged = resolve_extends(doc)
         assert merged["services"]["app"]["environment"] == {"A": "1", 3: "x"}
+
+
+class TestMergeErrorMessageOrder:
+    """The service name and the key must not be transposed in a merge error."""
+
+    def test_structural_concat_incompatible_form_names_service_then_key(self) -> None:
+        # 'volumes' is a structural concat key: it goes through extends' own
+        # list-normalizing path, not a KeySpec.merge. A swapped (name, key) would
+        # render "service 'volumes': cannot merge 'app'" and go unnoticed.
+        doc = {
+            "services": {
+                "base": {"image": "x", "volumes": ["./a:/a"]},
+                "app": {"extends": {"service": "base"}, "volumes": {"not": "a list"}},
+            }
+        }
+        with pytest.raises(UnsupportedComposeError) as excinfo:
+            resolve_extends(doc)
+        assert str(excinfo.value) == "service 'app': cannot merge 'volumes' across incompatible forms"
+
+    def test_structural_concat_merges_scalar_and_list_forms(self) -> None:
+        # The normalize-then-concat path itself: a scalar string on one side.
+        doc = {
+            "services": {
+                "base": {"image": "x", "env_file": "base.env"},
+                "app": {"extends": {"service": "base"}, "env_file": ["local.env"]},
+            }
+        }
+        merged = resolve_extends(doc)
+        assert merged["services"]["app"]["env_file"] == ["base.env", "local.env"]

tests/test_keys.py

@@ -5,10 +5,10 @@
     SERVICE_KEYS,
     STRUCTURAL_KEYS,
     Expand,
-    _concat_list,
     _merge_map,
     _validate_list,
     _validate_ulimits,
+    concat_list,
     pairs_to_mapping,
     require_string_keys,
     validate_map,
@@ -167,19 +167,19 @@ class TestMergeCallables:
 
     extends.py (Task 2) will call these through SERVICE_KEYS[key].merge, but
     that wiring doesn't exist yet — these tests exercise every branch of
-    _concat_list/_as_list/_merge_map/pairs_to_mapping on their own so Task 1
+    concat_list/as_list/_merge_map/pairs_to_mapping on their own so Task 1
     is fully covered without depending on Task 2.
     """
 
     def test_concat_list_merges_list_forms(self) -> None:
-        assert _concat_list("web", "cap_add", ["NET_ADMIN"], ["SYS_TIME"]) == ["NET_ADMIN", "SYS_TIME"]
+        assert concat_list("web", "cap_add", ["NET_ADMIN"], ["SYS_TIME"]) == ["NET_ADMIN", "SYS_TIME"]
 
     def test_concat_list_normalizes_scalar_string_form(self) -> None:
-        assert _concat_list("web", "cap_add", "NET_ADMIN", ["SYS_TIME"]) == ["NET_ADMIN", "SYS_TIME"]
+        assert concat_list("web", "cap_add", "NET_ADMIN", ["SYS_TIME"]) == ["NET_ADMIN", "SYS_TIME"]
 
     def test_concat_list_refuses_incompatible_form(self) -> None:
         with pytest.raises(UnsupportedComposeError, match="cannot merge 'cap_add' across incompatible forms"):
-            _concat_list("web", "cap_add", ["NET_ADMIN"], {"bad": "shape"})
+            concat_list("web", "cap_add", ["NET_ADMIN"], {"bad": "shape"})
 
     def test_merge_map_merges_dict_forms(self) -> None:
         assert _merge_map("web", "labels", {"team": "core"}, {"tier": "web"}) == {"team": "core", "tier": "web"}