Return flag tokens instead of mutating a list in place

compose2pod/emit.py

@@ -47,8 +47,9 @@ def entrypoint_tokens(svc: dict[str, Any]) -> list[Token]:
     return [Expand(value=str(token)) for token in entrypoint]
 
 
-def _add_health_flags(flags: list[Token], healthcheck: dict[str, Any]) -> None:
-    """Add healthcheck flags to the flags list."""
+def _health_flags(healthcheck: dict[str, Any]) -> list[Token]:
+    """Healthcheck flag tokens."""
+    flags: list[Token] = []
     cmd = health_cmd(healthcheck.get("test"))
     if cmd is not None:
         flags += ["--health-cmd", Expand(value=cmd)]
@@ -58,10 +59,12 @@ def _add_health_flags(flags: list[Token], healthcheck: dict[str, Any]) -> None:
             flags += ["--health-start-period", str(healthcheck["start_period"])]
         if "retries" in healthcheck:
             flags += ["--health-retries", str(healthcheck["retries"])]
+    return flags
 
 
-def _add_env_flags(flags: list[Token], svc: dict[str, Any], project_dir: str) -> None:
-    """Add -e and --env-file flags to the flags list."""
+def _env_flags(svc: dict[str, Any], project_dir: str) -> list[Token]:
+    """-e and --env-file flag tokens."""
+    flags: list[Token] = []
     # A null environment value means "pass KEY through from the host" (bare `-e KEY`).
     for pair in key_value_pairs(svc.get("environment") or {}):
         flags += ["-e", Expand(value=str(pair))]
@@ -70,10 +73,12 @@ def _add_env_flags(flags: list[Token], svc: dict[str, Any], project_dir: str) ->
         env_files = [env_files]
     for env_file in env_files:
         flags += ["--env-file", Expand(value=str(Path(project_dir, env_file)))]
+    return flags
 
 
-def _add_volume_flags(flags: list[Token], svc: dict[str, Any], project_dir: str) -> None:
-    """Add -v and --tmpfs flags to the flags list."""
+def _volume_flags(svc: dict[str, Any], project_dir: str) -> list[Token]:
+    """-v and --tmpfs flag tokens."""
+    flags: list[Token] = []
     for volume in svc.get("volumes") or []:
         if ":" not in volume:
             # Anonymous volume: a bare container path, no host source to translate.
@@ -91,14 +96,15 @@ def _add_volume_flags(flags: list[Token], svc: dict[str, Any], project_dir: str)
         tmpfs = [tmpfs]
     for mount in tmpfs:
         flags += ["--tmpfs", Expand(value=mount)]
+    return flags
 
 
 def run_flags(name: str, svc: dict[str, Any], pod: str, project_dir: str) -> list[Token]:
     """Flag tokens (unquoted) for `podman run` of one service."""
     flags: list[Token] = ["--pod", pod, "--name", f"{pod}-{name}"]
-    _add_env_flags(flags, svc, project_dir)
-    _add_volume_flags(flags, svc, project_dir)
-    _add_health_flags(flags, svc.get("healthcheck") or {})
+    flags += _env_flags(svc, project_dir)
+    flags += _volume_flags(svc, project_dir)
+    flags += _health_flags(svc.get("healthcheck") or {})
     for key, spec in SERVICE_KEYS.items():
         if key in svc:
             flags += spec.emit(svc[key])

planning/changes/2026-07-13.09-uniform-flag-composition.md

@@ -0,0 +1,88 @@
+---
+summary: Convert emit.py's three mutate-in-place flag helpers (_add_env_flags, _add_volume_flags, _add_health_flags) to return list[Token], so all six flag sources composed by run_flags share the one convention every other flag source in the package already follows.
+---
+
+# Change: Uniform flag composition in run_flags
+
+**Lane:** lightweight — ≲30 LOC net, 1 file, no new file, no public-API
+change, no new test (behavior-preserving; existing tests are the safety net).
+
+## Goal
+
+`run_flags` (`emit.py`) composes six flag sources in ten lines using two
+different conventions: three helpers mutate a list passed in
+(`_add_env_flags`, `_add_volume_flags`, `_add_health_flags`, all `-> None`),
+while three return a list that gets concatenated (`spec.emit(...)`,
+`stores.flags(...)`, `deploy_resource_flags(...)`). Make all six uniform by
+converting the three mutators to return `list[Token]`.
+
+The return convention is the established one everywhere else in the package —
+`KeySpec.emit`, `stores.flags`, `resources.deploy_resource_flags`, and all
+three of `pod.py`'s helpers (`_dns_flags`, `_sysctl_flags`, `_add_host_flags`,
+composed by `pod_create_flags` as `a + b + c`). These three `emit.py` helpers
+are the only exceptions in the package, and they sit directly beside three
+return-style calls in the same function, making the inconsistency maximally
+visible.
+
+Behavior-preserving: no bug is closed and no risk is removed. This is a
+consistency fix, done because it is cheap and leaves one convention rather
+than two.
+
+## Approach
+
+Rename and re-shape the three helpers, keeping every body unchanged except for
+accumulating into a local list and returning it:
+
+```python
+def _health_flags(healthcheck: dict[str, Any]) -> list[Token]:
+    flags: list[Token] = []
+    ...                       # body unchanged
+    return flags
+```
+
+`run_flags` then reads uniformly:
+
+```python
+def run_flags(name: str, svc: dict[str, Any], pod: str, project_dir: str) -> list[Token]:
+    flags: list[Token] = ["--pod", pod, "--name", f"{pod}-{name}"]
+    flags += _env_flags(svc, project_dir)
+    flags += _volume_flags(svc, project_dir)
+    flags += _health_flags(svc.get("healthcheck") or {})
+    for key, spec in SERVICE_KEYS.items():
+        if key in svc:
+            flags += spec.emit(svc[key])
+    flags += stores.flags(svc, pod)
+    flags += deploy_resource_flags(svc)
+    return flags
+```
+
+Emitted flag order is unchanged — each helper's tokens still land in the same
+position, since `+=` at the same call site appends the same tokens the
+in-place mutation did.
+
+The helpers are renamed to `_env_flags`/`_volume_flags`/`_health_flags`, since
+the `_add_*` prefix described the side effect that is going away. This matches
+`pod.py`'s `_dns_flags`/`_sysctl_flags`; note `pod.py` also has an
+`_add_host_flags` that returns tokens while keeping the prefix, so the package
+is not uniform on this — the naming here follows the majority, and no rename
+outside this file is in scope.
+
+## Files
+
+- `compose2pod/emit.py` — three helpers converted from
+  `(flags, ...) -> None` to `(...) -> list[Token]` and renamed; `run_flags`
+  updated to concatenate them.
+
+No test changes: all three helpers are private with one caller each, reached
+only through `run_flags`, which existing tests cover thoroughly
+(`TestRunFlags`, plus the integration suite executing real generated scripts).
+
+## Verification
+
+- [ ] No failing-test-first step — this is a behavior-preserving refactor with
+      no new behavior to assert. The existing suite is the safety net: it must
+      stay green, unchanged, at 100% coverage.
+- [ ] Apply the change.
+- [ ] `just test-ci` — 407 tests green (unchanged count), 100% line coverage.
+- [ ] `just lint-ci` — clean.
+- [ ] `just check-planning` — OK.