Problem
ActCommand.build() (cli/localci/core/executor.py:202-282) is pure — it only reads self to assemble an argv list. The surprising side effect lives in the executor: JobExecutor._execute_process() mutates the caller's ActCommand instance in place, writing back act_cmd.env_file and act_cmd._executor_owned_env_file = True (executor.py:600-601) and act_cmd.secret_file / act_cmd._executor_owned_secret_file = True (executor.py:620-621). run() then depends on that mutation for cleanup in its finally block via _cleanup_temp_files(act_cmd) (executor.py:528-529, 734-755). This mutation contract — "pass an ActCommand to the executor and it will populate temp-file fields and ownership flags on your object" — is undocumented: the ActCommand docstring (executor.py:146-150) says nothing about it, and the _executor_owned_* fields are repr=False (executor.py:178-179), making the behavior invisible to callers. Reusing one ActCommand across executor calls, or building it expecting purity, is a latent footgun.
Acceptance Criteria
Implementation Notes
- Primary file:
cli/localci/core/executor.py. The mutation sites are _execute_process lines 600-601 (env file) and 620-621 (secret file); cleanup is _cleanup_temp_files (lines 734-755), invoked from run()'s finally (line 529).
- Note on the finding's phrasing: the eval referred to this as a "build" side effect, but
ActCommand.build() is actually pure — the receiver-state mutation is performed by JobExecutor._execute_process. Ground the docs/tests in the real symbols above.
- Existing unit coverage already asserts the mutation with mocked subprocesses (
cli/tests/test_executor.py:731-793 for env/secret materialization; :833-855 for executor-owned cleanup). This issue adds the missing integration coverage (real act/subprocess) and, importantly, the written contract — reuse the integration marker declared in pyproject.toml:40-42 and the fixtures in cli/tests/integration/.
- Gotcha: integration tests require
act + Docker and are excluded from the default run (norecursedirs = ["integration"], pyproject.toml:43); follow the pattern in cli/tests/integration/test_act_subprocess.py and gate appropriately so CI without Docker still passes.
References
- Eval finding: Test 7 (interface-contract) — "ActCommand mutation contract is side-effectful"; Test 31 (testing) — "No integration test coverage for mutation behavior"
- Related files:
cli/localci/core/executor.py, cli/tests/test_executor.py, cli/tests/integration/test_act_subprocess.py, cli/pyproject.toml
Problem
ActCommand.build()(cli/localci/core/executor.py:202-282) is pure — it only readsselfto assemble an argv list. The surprising side effect lives in the executor:JobExecutor._execute_process()mutates the caller'sActCommandinstance in place, writing backact_cmd.env_fileandact_cmd._executor_owned_env_file = True(executor.py:600-601) andact_cmd.secret_file/act_cmd._executor_owned_secret_file = True(executor.py:620-621).run()then depends on that mutation for cleanup in itsfinallyblock via_cleanup_temp_files(act_cmd)(executor.py:528-529, 734-755). This mutation contract — "pass an ActCommand to the executor and it will populate temp-file fields and ownership flags on your object" — is undocumented: theActCommanddocstring (executor.py:146-150) says nothing about it, and the_executor_owned_*fields arerepr=False(executor.py:178-179), making the behavior invisible to callers. Reusing oneActCommandacross executor calls, or building it expecting purity, is a latent footgun.Acceptance Criteria
ActCommanddocstring (executor.py:146-150) documents thatenv_file,secret_file, and the_executor_owned_*flags are populated as side effects byJobExecutor, and that an instance is single-use per executionJobExecutor.run()/_execute_process()docstrings state that they mutate the passed-inActCommandand own the resulting temp files (cleanup happens inrun()'sfinally)integration, undercli/tests/integration/) exercises the real end-to-end path and asserts the receiver was mutated (temp files created during the run, ownership flags set, and files removed after cleanup)ActCommandis reused or shared across concurrent executions (documented as unsupported, or made safe)Implementation Notes
cli/localci/core/executor.py. The mutation sites are_execute_processlines 600-601 (env file) and 620-621 (secret file); cleanup is_cleanup_temp_files(lines 734-755), invoked fromrun()'sfinally(line 529).ActCommand.build()is actually pure — the receiver-state mutation is performed byJobExecutor._execute_process. Ground the docs/tests in the real symbols above.cli/tests/test_executor.py:731-793for env/secret materialization;:833-855for executor-owned cleanup). This issue adds the missing integration coverage (realact/subprocess) and, importantly, the written contract — reuse theintegrationmarker declared inpyproject.toml:40-42and the fixtures incli/tests/integration/.act+ Docker and are excluded from the default run (norecursedirs = ["integration"], pyproject.toml:43); follow the pattern incli/tests/integration/test_act_subprocess.pyand gate appropriately so CI without Docker still passes.References
cli/localci/core/executor.py,cli/tests/test_executor.py,cli/tests/integration/test_act_subprocess.py,cli/pyproject.toml