Deepen executor: introduce ScriptRunner seam and collapse execute/executeParallel duplication

[kindermax]

Problem Statement

Developers working on the executor cannot write unit tests for any of its core behaviour — dependency ordering, environment variable layering, init-script semantics, checksum persistence, after-script execution — without spawning a real shell process. The only executor tests that exist cover error-chain helpers and a single env map conversion. At the same time, the two execution paths (execute and executeParallel) duplicate four of their five phases verbatim, so any change to the execution flow (a new phase, a new error path) must be applied twice and kept in sync by hand.

Solution

Introduce a ScriptRunner seam that sits between executor orchestration and os/exec, making the executor fully testable without real shell invocations. Simultaneously collapse execute() and executeParallel() into a single executeFlow that accepts a runner function, so the two paths share one implementation of init, depends, persist, and after-script. As a small companion change, replace the initCalled bool guard with sync.Once to make the once-only init guarantee visible in the type rather than hidden behind a raw boolean.

User Stories

1. As a contributor, I want to write a test that verifies Execute() runs cfg.Init exactly once across multiple calls, so that I can confirm this guarantee without reading the implementation.
2. As a contributor, I want to write a test that verifies executeDepends runs dependencies in declaration order, so that I can catch regressions in dependency sequencing.
3. As a contributor, I want to write a test that verifies a dependency failure produces a DependencyError with the correct chain, so that I can confirm error-chain assembly without a real shell.
4. As a contributor, I want to write a test that verifies all LETS_CHECKSUM_* env vars are present in the script invocation, so that I can confirm env layering without running a shell script.
5. As a contributor, I want to write a test that verifies LETS_CHECKSUM_CHANGED reflects a changed checksum, so that I can confirm the comparison logic without file I/O.
6. As a contributor, I want to write a test that verifies the after script is called even when a command script fails, so that I can confirm cleanup semantics without real shell execution.
7. As a contributor, I want to write a test that verifies the after script error does not override the main command's exit code, so that I can pin the documented behaviour.
8. As a contributor, I want to write a test that verifies parallel commands both execute (regardless of order), so that I can confirm executeParallel dispatches all scripts.
9. As a contributor, I want to write a test that verifies a parallel command failure causes Execute() to return an error, so that I can confirm error propagation through errgroup.
10. As a contributor, I want to write a test that verifies persistChecksum is not called when a command script fails, so that I can confirm checksums are only written on success.
11. As a contributor, I want to inject a controlled ScriptRunner that returns a specific error, so that I can test error-handling paths without creating real failing shell scripts.
12. As a contributor, I want to inject a RecordingRunner that captures every script invocation, so that I can assert on invocation order and env contents in a single test.
13. As a contributor, I want NewExecutor to accept a ScriptRunner, so that production code uses the real shell runner and tests use a fake.
14. As a contributor, I want execute() and executeParallel() to share one implementation of init, depends, persist, and after-script, so that a fix to any phase applies everywhere automatically.
15. As a contributor, I want the once-only init guarantee expressed as sync.Once rather than a raw boolean, so that I can understand the contract without reading the method body.
16. As a maintainer, I want the executor to be testable without Docker or a real shell, so that the unit test suite runs quickly in any environment.
17. As a maintainer, I want the env-layer ordering tested through pure helper functions, so that I can pin which layer wins without a full Execute() invocation.

Implementation Decisions

• Introduce a ScriptRunner abstraction (a function type func(command *config.Command, script string, env []string, dir string) error or a single-method interface — the exact shape to be decided during implementation) that Executor holds and calls instead of directly invoking os/exec.
• NewExecutor gains a runner ScriptRunner parameter. Callers in production pass a ShellRunner constructed from the current newOsCommand + Run() logic; tests pass a RecordingRunner or a stub that returns controlled errors.
• Extract executeFlow(ctx *Context, runScripts func() error) error that contains: initCmd, executeDepends, the provided runScripts closure, persistChecksum, and executeAfterScript. Both execute and executeParallel become thin callers that supply either a sequential or a parallel runScripts closure.
• Replace Executor.initCalled bool with sync.Once. The once.Do wraps the init script invocation. No behaviour change; the guarantee becomes type-level.
• The public interface Execute(*Context) error, NewExecutorCtx, ChildExecutorCtx, DependencyError, and ExecuteError are not changed. This is a purely internal restructuring.
• setupEnv and the env helper functions in env.go are not restructured in this change (deferred to a separate effort).
• The ScriptRunner seam is the recommended injection point for a future dry-run mode; this PRD does not implement dry-run.

Testing Decisions

A good test drives Executor.Execute() with a constructed config.Config and config.Command, injects a RecordingRunner or error-returning stub, and asserts on the returned error and the recorded invocations. Tests should not assert on which internal method was called or in what order methods on the executor are dispatched — only on observable outputs: the error returned, the error chain shape, and the scripts/envs that reached the runner.

Modules to test:

• Executor.Execute() — the primary seam. Cover: init runs once, depends ordering, sequential script ordering, parallel script dispatch, after-script on failure and success, checksum persist only on success, error chain shape.
• getChecksumEnvMap and getChangedChecksumEnvMap in env.go — pure functions, table-driven tests without any executor setup.
• isChecksumChanged — pure, cover the three cases (no persisted checksum, checksum unchanged, checksum changed).

Prior art: internal/executor/dependency_error_test.go — package-internal tests (package executor), standard testing package, no third-party assertion library. New tests should follow the same style.

Out of Scope

• Extracting setupEnv into a standalone EnvResolver module (Candidate 3 from the architecture review — deferred).
• Context cancellation propagation to sequential execution (the ctx.ctx field is currently only used in errgroup.WithContext; expanding this is a separate concern).
• Dry-run mode (the ScriptRunner seam enables it, but implementing it is not part of this PRD).
• Changes to public-facing executor behaviour, CLI flags, or lets.yaml syntax.

Further Notes

This work was identified during an architecture review of internal/executor/ on 2026-06-13. The two Strong candidates from that review (collapse execute/executeParallel duplication; introduce OS-process seam) are bundled here because they touch the same methods and are cleaner to do together. Candidates 3 (EnvResolver) and 4 (sync.Once) from the review are either deferred (3) or included as a minor companion change (4).

The existing Bats integration tests in tests/ provide end-to-end coverage of the CLI surface and are unaffected by this refactoring.