diff --git a/compose2pod/emit.py b/compose2pod/emit.py index 989cf75..73c3144 100644 --- a/compose2pod/emit.py +++ b/compose2pod/emit.py @@ -146,10 +146,28 @@ class EmitOptions: allow_exit_codes: list[int] +@dataclasses.dataclass(frozen=True, slots=True, kw_only=True) +class PlannedScript: + """The rendered pod script and the run-time variables it expands. + + Both are products of a single `_plan` traversal, so they cannot drift. + """ + + script: str + variables: list[str] + + def _render(tokens: list[Token]) -> str: return " ".join(to_shell(token.value) if isinstance(token, _Expand) else shlex.quote(token) for token in tokens) +def _collect_vars(tokens: list[Token], names: set[str]) -> None: + """Add the run-time variables any `_Expand` tokens expand to `names`.""" + for token in tokens: + if isinstance(token, _Expand): + names.update(variable_names(token.value)) + + def _run_tokens(name: str, services: dict[str, Any], options: EmitOptions, hosts: list[str]) -> list[Token]: svc = services[name] tokens = run_flags(name, svc, options.pod, hosts, options.project_dir) @@ -187,11 +205,14 @@ def _emit_target(lines: list[str], run_tokens: list[Token], options: EmitOptions lines.append("esac") -def emit_script(compose: dict[str, Any], options: EmitOptions) -> str: - """Render the full pod test script for `target` and its dependency closure.""" - if not POD_NAME_PATTERN.fullmatch(options.pod): - msg = f"invalid pod name {options.pod!r}" - raise UnsupportedComposeError(msg) +def _plan(compose: dict[str, Any], options: EmitOptions) -> PlannedScript: + """Walk the target's dependency closure once, building the script and its variables. + + Every token source is visited a single time and feeds both outputs: each + service's `_run_tokens` and the `pod_create_flags` render into lines *and* + have their `_Expand` variables collected, so the script and the variable + list cannot disagree about what the script expands at run time. + """ services = compose["services"] hosts = hostnames(services) order = startup_order(services, options.target) @@ -201,6 +222,7 @@ def emit_script(compose: dict[str, Any], options: EmitOptions) -> str: for dep, condition in depends_on(svc).items() if condition == "service_completed_successfully" } + names: set[str] = set() lines = [_SCRIPT_HEADER] teardown = f"podman pod rm -f {shlex.quote(options.pod)} >/dev/null 2>&1 || true" @@ -209,11 +231,13 @@ def emit_script(compose: dict[str, Any], options: EmitOptions) -> str: teardown += f"; {store_teardown}" lines.append(f"trap '{teardown}' EXIT") pod_flags = pod_create_flags(services, order) + _collect_vars(pod_flags, names) pod_create = f"podman pod create --name {shlex.quote(options.pod)}" if pod_flags: pod_create += " " + _render(pod_flags) lines.append(pod_create) lines.extend(stores.create_lines(compose, order, options.pod, options.project_dir)) + names |= stores.referenced_variables(compose, order, options.project_dir) waited: set[str] = set() for name in order: for dep, condition in depends_on(services[name]).items(): @@ -223,32 +247,30 @@ def emit_script(compose: dict[str, Any], options: EmitOptions) -> str: lines.append(f"wait_healthy {shlex.quote(f'{options.pod}-{dep}')} {attempts} {interval}") waited.add(dep) run_tokens = _run_tokens(name, services, options, hosts) + _collect_vars(run_tokens, names) if name == options.target: _emit_target(lines, run_tokens, options) elif name in completion_gated: lines.append(f"podman run --rm {_render(run_tokens)}") else: lines.append(f"podman run -d {_render(run_tokens)}") - return "\n".join(lines) + "\n" + return PlannedScript(script="\n".join(lines) + "\n", variables=sorted(names)) + + +def emit_script(compose: dict[str, Any], options: EmitOptions) -> str: + """Render the full pod test script for `target` and its dependency closure.""" + if not POD_NAME_PATTERN.fullmatch(options.pod): + msg = f"invalid pod name {options.pod!r}" + raise UnsupportedComposeError(msg) + return _plan(compose, options).script def referenced_variables(compose: dict[str, Any], options: EmitOptions) -> list[str]: """Variable names the emitted script expands at run time (sorted, unique). - Reuses the same token construction as `emit_script`, so it counts exactly - the values that pass through `to_shell` -- never a `${VAR}` in a field that - is emitted literally, and never the `--command` override (a literal CLI value). + Projects the same `_plan` traversal `emit_script` renders from, so it counts + exactly the values that pass through `to_shell` -- never a `${VAR}` in a + field emitted literally, and never the `--command` override (a literal CLI + value). """ - services = compose["services"] - hosts = hostnames(services) - order = startup_order(services, options.target) - names: set[str] = set() - for name in order: - for token in _run_tokens(name, services, options, hosts): - if isinstance(token, _Expand): - names.update(variable_names(token.value)) - for token in pod_create_flags(services, order): - if isinstance(token, _Expand): - names.update(variable_names(token.value)) - names |= stores.referenced_variables(compose, order, options.project_dir) - return sorted(names) + return _plan(compose, options).variables diff --git a/planning/changes/2026-07-12.03-one-script-plan.md b/planning/changes/2026-07-12.03-one-script-plan.md new file mode 100644 index 0000000..ad613de --- /dev/null +++ b/planning/changes/2026-07-12.03-one-script-plan.md @@ -0,0 +1,101 @@ +--- +summary: Build the pod-script plan once behind an internal _plan seam so emit_script and referenced_variables both project from a single traversal and cannot drift. +--- + +# Design: One script plan, two readers + +## Summary + +`emit_script` (renders the closure to a script string) and +`referenced_variables` (collects the `$VAR`s the script expands at run time) +independently recompute `services`/`hosts`/`order` and walk the same token +sources. Introduce an internal `_plan(compose, options) -> PlannedScript` that +does the single traversal; both public functions become thin projections +(`.script`, `.variables`). Behavior-preserving: byte-identical script and an +identical referenced-variable list. + +## Motivation + +emit.py:190-232 (`emit_script`) and emit.py:235-254 (`referenced_variables`) +each open with `services = compose["services"]; hosts = hostnames(...); +order = startup_order(...)` and then walk the same three sources: `_run_tokens` +per service, `pod_create_flags`, and the store side. They must agree — the +variables note must count exactly the `$VAR`s the script actually expands — but +nothing enforces it. There is **no active drift today**; it holds only because +the sole `$VAR`-bearing sources happen to be those three (teardown, +`wait_healthy`, `podman cp`, and the `case` gate carry no runtime variables). A +future var-bearing construct added to one reader and not the other would +silently make the stderr note wrong. This is a locality/fragility fix, not a +bug fix. + +## Design + +Introduce a single internal traversal and make the two readers project from it: + +```python +@dataclasses.dataclass(frozen=True, slots=True, kw_only=True) +class PlannedScript: + script: str # finished: joined lines + trailing newline + variables: list[str] # finished: sorted, unique + +def _plan(compose: dict[str, Any], options: EmitOptions) -> PlannedScript: ... + +def emit_script(compose, options) -> str: + if not POD_NAME_PATTERN.fullmatch(options.pod): + raise UnsupportedComposeError(...) + return _plan(compose, options).script + +def referenced_variables(compose, options) -> list[str]: + return _plan(compose, options).variables +``` + +`_plan` walks `order` **once**, calling `_run_tokens(name, ...)` a single time +per service and using that one token list for *both* the rendered line (via the +existing `_emit_target` / `--rm` / `-d` dispatch) and the `_Expand` var +extraction. `pod_create_flags` and the store calls (`stores.create_lines`, +`stores.referenced_variables`) likewise run once, in the same pass. `_plan` +does the final join and sort so both `PlannedScript` fields are finished +products. + +**Interface & callers.** `emit_script` keeps its exact public signature (it is +in `__init__.__all__`); `referenced_variables` keeps its signature (it is +package-internal — only cli.py and tests use it). cli.py and every test are +unchanged. The single traversal *logic* lives in `_plan`, so drift is +structurally impossible even though cli calls both wrappers (the run-once CLI +computes the tiny plan twice — negligible). + +**Validation placement.** The pod-name guard stays in `emit_script`, not +`_plan`, so `referenced_variables` keeps its current no-validation behavior +(the pod name appears only in literal token text, never in an `_Expand` var). + +## Non-goals + +- Not unifying the store-internal render/vars split (`stores.create_lines` vs + `stores.referenced_variables`) — the same pattern one level down, recorded in + `deferred.md`. Keeps this change emit-internal and off the just-shipped store + interface. +- Not a typed compose model — orthogonal, and this is an emit-*internal* seam, + not the public `validate -> emit` seam frozen by + `decisions/2026-07-10-reject-parse-dont-validate.md`. + +## Testing + +- `just test-ci` at 100% line coverage; existing `TestEmitScript` (golden script + assertions) and `TestReferencedVariables` are the safety net and stay green + unchanged. +- Golden check: the emitted script and the referenced-variable list for a + secrets+configs compose are byte/element-identical before and after. +- `just lint-ci` clean. + +## Risk + +- **`referenced_variables` now builds the full plan** (low / low): on a + *malformed* input a direct call could raise where the old, narrower traversal + did not — e.g. a colon-less `--artifact` reaches `_emit_target`'s + `split(":", 1)` and raises `ValueError`, whereas the old `referenced_variables` + returned `[]`. This is unreachable via the only non-test caller: in cli.py, + `emit_script` runs before `referenced_variables` and hits the *same* + `_emit_target` split, so a malformed artifact already raised there first — cli + behavior is unchanged. `referenced_variables` is otherwise package-internal and + every test passes valid input. Mitigation: the golden + var-list equality check + on valid input, and this recorded, intended consequence. diff --git a/planning/deferred.md b/planning/deferred.md index 4c22f35..234026b 100644 --- a/planning/deferred.md +++ b/planning/deferred.md @@ -2,4 +2,16 @@ Real-but-unscheduled items, each with a revisit trigger. -_None currently._ +## Unify the store render/vars seam + +`stores.create_lines` (rendered lines) and `stores.referenced_variables` (the +vars those lines expand) are two functions that must agree — the same +"two readers, one source" pattern that `2026-07-12.03` fixes at the emit level, +one level down inside `stores.py`. Folding them into one per-line +`(text, vars)` producer would make store-side drift unrepresentable too. + +**Revisit trigger:** a third reader of the store create-lines appears, or a +drift bug surfaces between the two store functions (a `$VAR` a create line +expands that `referenced_variables` fails to report). Left out of +`2026-07-12.03` to keep that change emit-internal and avoid re-touching the +just-shipped store interface.