Deepen executor env setup: extract EnvResolver with testable layer ordering

[kindermax]

Problem Statement

Developers working on executor environment setup cannot write tests for the env-composition logic without running a full command execution. setupEnv() is a shallow module whose interface — eight sources pulled from config.Config, config.Command, and three computed maps — is nearly as complex as its implementation. The layer-ordering rule (which env source wins when two define the same key) is entirely invisible in tests; it exists only inside setupEnv()'s body, embedded in orchestration code that requires a live os/exec.Cmd to invoke. Any layering bug — a checksum var being silently overridden by a user env, for instance — cannot be caught by a unit test today.

Solution

Extract the env-composition logic into a standalone EnvResolver module: a pure function (or small type) that accepts explicit inputs and returns a composed env slice and an error. The eight input sources become clearly named parameters; the ordering becomes an internal implementation detail that can be asserted on directly in table-driven tests. The executor calls the resolver and assigns the result, with no change to observable CLI behaviour.

User Stories

1. As a contributor, I want to write a table-driven test that asserts command env overrides global env, so that I can pin layer-ordering precedence without running a shell.
2. As a contributor, I want to write a test that asserts CLI options (docopt) override command env vars, so that I can confirm the highest-priority layer wins.
3. As a contributor, I want to write a test that asserts LETS_CHECKSUM is present in the resolved env list when a checksum is defined, so that I can confirm checksum injection without file I/O.
4. As a contributor, I want to write a test that asserts LETS_CHECKSUM_CHANGED=true when no persisted checksum exists, so that I can confirm the "brand new checksum" case.
5. As a contributor, I want to write a test that asserts LETS_CHECKSUM_CHANGED=false when the checksum is unchanged, so that I can confirm the stable-checksum case.
6. As a contributor, I want to write a test that asserts LETS_CHECKSUM_<NAME>_CHANGED uses the normalised (uppercased, hyphen-to-underscore) key name, so that I can pin the key format.
7. As a contributor, I want to write a test that asserts LETS_COMMAND_NAME is set to the command's name, so that I can confirm builtin vars are present in every resolved env.
8. As a contributor, I want to write a test that asserts LETS_COMMAND_WORK_DIR reflects the effective working directory, so that I can confirm the builtin is derived after dir resolution.
9. As a contributor, I want to write a test that asserts a named checksum env key does not collide with the default LETS_CHECKSUM key, so that I can confirm the special-case handling.
10. As a contributor, I want to write a test that asserts an error from command.GetEnv() is surfaced by the resolver, so that I can confirm error propagation without an executor.
11. As a contributor, I want a test that asserts the resolver returns a deterministic env list for a given set of inputs, so that I can use it as a regression anchor when changing layer logic.
12. As a contributor, I want the layer-ordering to be documented by the test names themselves, so that a new contributor understands precedence by reading the test file.
13. As a maintainer, I want setupEnv complexity to live in one testable place, so that adding a new env layer requires one code change and one test, not a hunt through orchestration code.
14. As a maintainer, I want env-composition bugs to be catchable without Docker or a running shell, so that the unit suite can serve as a first-pass regression gate.
15. As a contributor, I want the executor to call the resolver with clearly named arguments, so that I can understand which config and command fields contribute to the env without tracing through side effects.

Implementation Decisions

• Extract a pure resolver function (signature: ResolveEnv(cfg *config.Config, cmd *config.Command, shell, dir string) ([]string, error)) that encapsulates the full env-composition pipeline: builtin vars, global env, command env, docopt options, CLI options, checksum vars, and changed-checksum vars.
• The layer ordering (later layers shadow earlier ones) becomes an internal detail of the resolver implementation, not visible to the executor.
• setupEnv() is replaced by a call to the resolver; the osCmd.Env assignment stays in the executor body.
• The resolver does not write to os/exec.Cmd or any other receiver — it is a pure data transformation. This is the key deepening: the interface shrinks to four typed inputs and one output.
• The existing helper functions getChecksumEnvMap, getChangedChecksumEnvMap, isChecksumChanged, and convertEnvMapToList remain as internal helpers called by the resolver. They are not part of the resolver's interface.
• normalizeEnvKey remains an internal helper.
• No changes to config.Config, config.Command, or any public executor types (Execute, NewExecutor, Context, DependencyError, ExecuteError).
• This change is independent of issue Deepen executor: introduce ScriptRunner seam and collapse execute/executeParallel duplication #355 (ScriptRunner seam / executeFlow collapse) and can be done in either order.

Testing Decisions

A good test for the resolver passes explicit config.Config and config.Command values constructed in the test, calls the resolver, and asserts on specific entries in the returned slice. Tests should not assert on the slice length or the full slice contents — only on the presence, absence, and value of specific keys — so that adding a new builtin var does not break existing tests.

Modules to test:

• ResolveEnv (the extracted resolver) — primary target. Cover: each builtin var, layer precedence (command env beats global env, CLI option beats command env, checksum vars present, changed-checksum flag), error propagation from GetEnv.
• getChecksumEnvMap — already partially covered by the architecture review; add table-driven tests for the default key special case and normalised named-checksum keys.
• isChecksumChanged — three cases: no persisted checksum (true), same checksum (false), different checksum (true).
• getChangedChecksumEnvMap — integration of isChecksumChanged into the env map, including the default key special case.

Prior art: internal/executor/env_test.go for the helper-function style; internal/executor/dependency_error_test.go for the package-internal, standard-testing, table-driven pattern. New tests should follow the same conventions.

Out of Scope

• Changing the layer-ordering rule itself (this PRD only makes existing ordering testable).
• Exposing ResolveEnv as part of any public package API.
• Adding new env sources or new builtin vars.
• The ScriptRunner seam and executeFlow collapse (tracked separately in issue Deepen executor: introduce ScriptRunner seam and collapse execute/executeParallel duplication #355).
• Context cancellation propagation to sequential execution.

Further Notes

This work was identified as Candidate 3 ("Deepen env setup into an EnvResolver module") during an architecture review of internal/executor/ on 2026-06-13. It was explicitly deferred from issue #355 to keep that change focused on the execution-flow restructuring. The two changes are independent and can land in either order.

The mass-diagram from the architecture review captures the shallowness cleanly: setupEnv()'s interface (8 input sources) is nearly as wide as its implementation. The resolver's interface (4 named inputs → 1 output) is the target shape.