Problem Statement
Executor uses a raw initCalled bool field to ensure the global init script runs exactly once across multiple Execute() calls. This guarantee is invisible from the type: callers reading the struct see a boolean with no indication of why it exists or what it protects. The guard is also not safe for concurrent Execute() calls — two goroutines racing on the first call could both see initCalled == false and run init twice. The current calling pattern is single-threaded, so this hasn't caused a bug, but the guarantee is fragile and silent.
Solution
Replace the initCalled bool field with a sync.Once. The once-only init semantics are encoded in the type, documented by the standard library, and concurrency-safe without any additional locking. No observable behaviour changes; the guarantee becomes explicit at the cost of zero complexity.
User Stories
- As a contributor, I want to understand the init-script guarantee by reading the
Executor struct definition, so that I don't have to trace through the Execute() body to find the if !e.initCalled guard.
- As a contributor, I want the init-script guarantee to be concurrency-safe by construction, so that a future change that calls
Execute() from multiple goroutines does not silently break the guarantee.
- As a contributor, I want to write a test that calls
Execute() twice and verifies the init script ran exactly once, so that I can pin the once-only semantics as a regression anchor.
- As a contributor, I want the init logic to not require resetting state between test cases, so that I can construct a fresh
Executor per test without worrying about leaked boolean state.
- As a maintainer, I want the init-script guarantee to be self-documenting, so that a code reviewer can confirm correctness without knowing the convention around
initCalled.
- As a maintainer, I want
Executor to have no exported or unexported boolean fields that silently alter the behaviour of Execute(), so that the executor's state model is simple and auditable.
- As a contributor, I want the removal of the boolean guard to be a no-op refactor with a clear mechanical mapping (
if !e.initCalled { e.initCalled = true; ... } → e.once.Do(...)), so that I can review the change with confidence that no behaviour was altered.
Implementation Decisions
Testing Decisions
A good test constructs an Executor with a non-empty cfg.Init, calls Execute() twice with valid contexts, and asserts that the init script was invoked exactly once. The test should drive this through Executor.Execute() — the highest available seam — rather than inspecting the once field directly.
Once issue #355 lands, a RecordingRunner can count init invocations precisely. Before #355 lands, a lightweight script-counting stub (a closure over a counter) suffices for the same assertion.
Modules to test:
Executor.Execute() — verify init runs once on first call and is skipped on subsequent calls.
Executor.Execute() — verify that an Executor with an empty cfg.Init never invokes the init path (the sync.Once consumes silently or the guard inside Do skips it).
Prior art: internal/executor/dependency_error_test.go — package-internal, standard testing package, no third-party assertion library. New tests follow the same style.
Out of Scope
Further Notes
This was surfaced as Candidate 4 ("Make the init-script guarantee explicit") during an architecture review of internal/executor/ on 2026-06-13. It was included as a companion change in issue #355 but is extracted here as a standalone issue so it can be reviewed and merged independently — it is the smallest possible self-contained improvement in the executor package.
Problem Statement
Executoruses a rawinitCalled boolfield to ensure the globalinitscript runs exactly once across multipleExecute()calls. This guarantee is invisible from the type: callers reading the struct see a boolean with no indication of why it exists or what it protects. The guard is also not safe for concurrentExecute()calls — two goroutines racing on the first call could both seeinitCalled == falseand run init twice. The current calling pattern is single-threaded, so this hasn't caused a bug, but the guarantee is fragile and silent.Solution
Replace the
initCalled boolfield with async.Once. The once-only init semantics are encoded in the type, documented by the standard library, and concurrency-safe without any additional locking. No observable behaviour changes; the guarantee becomes explicit at the cost of zero complexity.User Stories
Executorstruct definition, so that I don't have to trace through theExecute()body to find theif !e.initCalledguard.Execute()from multiple goroutines does not silently break the guarantee.Execute()twice and verifies the init script ran exactly once, so that I can pin the once-only semantics as a regression anchor.Executorper test without worrying about leaked boolean state.initCalled.Executorto have no exported or unexported boolean fields that silently alter the behaviour ofExecute(), so that the executor's state model is simple and auditable.if !e.initCalled { e.initCalled = true; ... }→e.once.Do(...)), so that I can review the change with confidence that no behaviour was altered.Implementation Decisions
initCalled boolfield onExecutorwith async.Oncefield.if e.cfg.Init != "" && !e.initCalled { e.initCalled = true; ... }block inExecute()becomese.once.Do(func() { if e.cfg.Init != "" { ... } })— or equivalently, theInit != ""guard moves inside theDoclosure so theOnceis only consumed when init is actually configured.NewExecutor,Execute,Context, or any error types.sync.Onceis already in the Go standard library; no new dependencies.Testing Decisions
A good test constructs an
Executorwith a non-emptycfg.Init, callsExecute()twice with valid contexts, and asserts that the init script was invoked exactly once. The test should drive this throughExecutor.Execute()— the highest available seam — rather than inspecting theoncefield directly.Once issue #355 lands, a
RecordingRunnercan count init invocations precisely. Before #355 lands, a lightweight script-counting stub (a closure over a counter) suffices for the same assertion.Modules to test:
Executor.Execute()— verify init runs once on first call and is skipped on subsequent calls.Executor.Execute()— verify that anExecutorwith an emptycfg.Initnever invokes the init path (thesync.Onceconsumes silently or the guard insideDoskips it).Prior art:
internal/executor/dependency_error_test.go— package-internal, standardtestingpackage, no third-party assertion library. New tests follow the same style.Out of Scope
cfg.Init(when it runs, what it receives, how errors are handled).Executorsafe for concurrentExecute()calls beyond the init-script guarantee — full concurrency safety of the executor is a separate, larger concern.ScriptRunnerseam (issue Deepen executor: introduce ScriptRunner seam and collapse execute/executeParallel duplication #355) andEnvResolver(issue Deepen executor env setup: extract EnvResolver with testable layer ordering #356).Further Notes
This was surfaced as Candidate 4 ("Make the init-script guarantee explicit") during an architecture review of
internal/executor/on 2026-06-13. It was included as a companion change in issue #355 but is extracted here as a standalone issue so it can be reviewed and merged independently — it is the smallest possible self-contained improvement in the executor package.