From 7f72d78b814ef4dc3a006846be861ab69d7db77d Mon Sep 17 00:00:00 2001 From: bradjin8 Date: Thu, 9 Jul 2026 14:38:59 -0400 Subject: [PATCH 1/4] initial implementation of #70 --- cli/localci/core/executor.py | 39 ++++++- .../integration/test_act_command_mutation.py | 101 ++++++++++++++++++ 2 files changed, 139 insertions(+), 1 deletion(-) create mode 100644 cli/tests/integration/test_act_command_mutation.py diff --git a/cli/localci/core/executor.py b/cli/localci/core/executor.py index 26b5c80..920bdf5 100644 --- a/cli/localci/core/executor.py +++ b/cli/localci/core/executor.py @@ -146,7 +146,28 @@ def summary_line(self) -> str: class ActCommand: """Builder for ``act`` CLI command. - Constructs the full command-line invocation for act. + Constructs the full command-line invocation for act. :meth:`build` is pure: + it only reads ``self`` to assemble an argv list. + + **Executor mutation contract.** When passed to :meth:`JobExecutor.run`, the + executor mutates this instance in place during + :meth:`JobExecutor._execute_process`: + + * :attr:`env_file` and :attr:`_executor_owned_env_file` are set when + :attr:`env` is non-empty (a ``0600`` temp file is created). + * :attr:`secret_file` and :attr:`_executor_owned_secret_file` are set when + :attr:`secrets` is non-empty (a ``0600`` temp file is created). + + The ownership flags tell :meth:`JobExecutor._cleanup_temp_files` (called from + :meth:`JobExecutor.run`'s ``finally`` block) which temp files the executor + created and must delete. Caller-provided ``env_file`` / ``secret_file`` paths + without the ownership flag are left on disk. + + **Single-use per execution.** Build a fresh :class:`ActCommand` for each + call to :meth:`JobExecutor.run`. Reusing the same instance across runs is + unsupported: stale paths, ownership flags, and merged env state from a prior + run produce undefined behavior. Sharing one instance across concurrent + executions is likewise unsupported. """ # Required @@ -440,6 +461,15 @@ def run( ------- JobResult Complete execution result with status, output, and timing. + + Notes + ----- + Mutates *cmd* in place: :meth:`_execute_process` writes + :attr:`ActCommand.env_file` / :attr:`ActCommand.secret_file` and the + ``_executor_owned_*`` flags on the passed-in instance. Temp files the + executor creates are removed in this method's ``finally`` block via + :meth:`_cleanup_temp_files`. See :class:`ActCommand` for the full + mutation contract. """ result = JobResult( job_id=cmd.job_id, @@ -551,6 +581,13 @@ def _execute_process( the secret file; :attr:`ActCommand.secrets` is also injected into the subprocess environment for act's GitHub auth. + **Mutates *act_cmd* in place.** When :attr:`ActCommand.env` or + :attr:`ActCommand.secrets` is non-empty, this method creates temp files + and sets :attr:`ActCommand.env_file` / :attr:`ActCommand.secret_file` + plus the corresponding ``_executor_owned_*`` flags on *act_cmd*. The + caller's object is the cleanup target for :meth:`_cleanup_temp_files`. + Do not reuse *act_cmd* across calls; see :class:`ActCommand`. + Returns ``(exit_code, stdout, stderr)``. """ workdir = act_cmd.workdir or Path(".") diff --git a/cli/tests/integration/test_act_command_mutation.py b/cli/tests/integration/test_act_command_mutation.py new file mode 100644 index 0000000..c63bad0 --- /dev/null +++ b/cli/tests/integration/test_act_command_mutation.py @@ -0,0 +1,101 @@ +"""Integration tests for ActCommand mutation contract (JobExecutor side effects).""" + +from __future__ import annotations + +from pathlib import Path + +import pytest + +from localci.core.command_builder import ActCommandBuilder +from localci.core.executor import ActCommand, JobExecutor, JobStatus +from localci.core.workflow import MatrixEntry, WorkflowAnalyzer + +from .conftest import INTEGRATION_JOB_ID, INTEGRATION_TIMEOUT + +pytestmark = pytest.mark.integration + + +def _workflow_path(name: str, project_dir: Path) -> Path: + return project_dir / ".github/workflows" / name + + +def _build_act_command( + workflow_name: str, + project_dir: Path, + act_runner_image: str, +) -> tuple[MatrixEntry, ActCommand]: + workflow_path = _workflow_path(workflow_name, project_dir) + analyzer = WorkflowAnalyzer() + workflow = analyzer.analyze(workflow_path) + entry = workflow.jobs[INTEGRATION_JOB_ID].matrix[0] + + builder = ActCommandBuilder( + workflow_file=workflow_path, + project_dir=project_dir, + job_id=INTEGRATION_JOB_ID, + default_secrets={"GITHUB_TOKEN": "local-ci-test-token"}, + ) + cmd = builder.build(entry, image_tag=act_runner_image) + return entry, cmd + + +def test_act_command_mutation_and_cleanup( + integration_project: tuple[Path, Path], + act_runner_image: str, +) -> None: + """End-to-end: executor mutates ActCommand during run and cleans temp files.""" + project, logs_dir = integration_project + entry, cmd = _build_act_command("test.yml", project, act_runner_image) + + assert cmd.env, "fixture must populate env so executor materializes env_file" + assert cmd.secrets, ( + "fixture must populate secrets so executor materializes secret_file" + ) + assert cmd.env_file is None + assert cmd.secret_file is None + assert not cmd._executor_owned_env_file + assert not cmd._executor_owned_secret_file + + mutation_during_run = False + + def on_output(_line: str) -> None: + nonlocal mutation_during_run + if mutation_during_run: + return + env_ready = ( + cmd.env_file is not None + and cmd._executor_owned_env_file + and cmd.env_file.exists() + ) + secret_ready = ( + cmd.secret_file is not None + and cmd._executor_owned_secret_file + and cmd.secret_file.exists() + ) + if env_ready and secret_ready: + mutation_during_run = True + + executor = JobExecutor(logs_dir=logs_dir) + result = executor.run( + cmd, + matrix_index=entry.index, + matrix_name=entry.name, + timeout=INTEGRATION_TIMEOUT, + stream_output=False, + on_output=on_output, + ) + + assert result.status == JobStatus.PASSED + assert mutation_during_run, ( + "temp files and ownership flags must be set before act output" + ) + assert cmd._executor_owned_env_file + assert cmd._executor_owned_secret_file + assert cmd.env_file is not None + assert cmd.secret_file is not None + assert not cmd.env_file.exists(), ( + "executor-owned env file must be removed after run" + ) + assert not cmd.secret_file.exists(), ( + "executor-owned secret file must be removed after run" + ) From 64b481dc01c3af3fd3b13a8f24ad836ff6f21788 Mon Sep 17 00:00:00 2001 From: bradjin8 Date: Thu, 9 Jul 2026 15:17:09 -0400 Subject: [PATCH 2/4] fix: integration test ci failure --- cli/tests/integration/test_act_command_mutation.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/cli/tests/integration/test_act_command_mutation.py b/cli/tests/integration/test_act_command_mutation.py index c63bad0..fce1f01 100644 --- a/cli/tests/integration/test_act_command_mutation.py +++ b/cli/tests/integration/test_act_command_mutation.py @@ -35,7 +35,13 @@ def _build_act_command( job_id=INTEGRATION_JOB_ID, default_secrets={"GITHUB_TOKEN": "local-ci-test-token"}, ) - cmd = builder.build(entry, image_tag=act_runner_image) + cmd = builder.build( + entry, + image_tag=act_runner_image, + # Minimal fixture matrix has no cc/cxx; supply env so executor + # materializes an executor-owned env_file (mutation contract). + extra_env={"LOCALCI_TEST_ENV": "mutation-contract"}, + ) return entry, cmd From f3b980f8f2c938a51728d467c4bcf9089153f066 Mon Sep 17 00:00:00 2001 From: bradjin8 Date: Thu, 9 Jul 2026 15:22:49 -0400 Subject: [PATCH 3/4] fix: nitpick comment --- cli/localci/core/executor.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/cli/localci/core/executor.py b/cli/localci/core/executor.py index 920bdf5..5d6a687 100644 --- a/cli/localci/core/executor.py +++ b/cli/localci/core/executor.py @@ -154,7 +154,10 @@ class ActCommand: :meth:`JobExecutor._execute_process`: * :attr:`env_file` and :attr:`_executor_owned_env_file` are set when - :attr:`env` is non-empty (a ``0600`` temp file is created). + :attr:`env` is non-empty (a ``0600`` temp file is created). If a + caller-supplied, unowned :attr:`env_file` already exists, its contents + are copied into the new temp file before :attr:`env` entries are written; + the original caller file is left on disk. * :attr:`secret_file` and :attr:`_executor_owned_secret_file` are set when :attr:`secrets` is non-empty (a ``0600`` temp file is created). From b102628f4df433db4a265ede8f9789ca2e1c6f7e Mon Sep 17 00:00:00 2001 From: bradjin8 Date: Thu, 9 Jul 2026 16:59:37 -0400 Subject: [PATCH 4/4] fix: added one bullet mirroring the code in _execute_process under the env_file item --- cli/localci/core/executor.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/cli/localci/core/executor.py b/cli/localci/core/executor.py index 5d6a687..34f1e39 100644 --- a/cli/localci/core/executor.py +++ b/cli/localci/core/executor.py @@ -154,10 +154,11 @@ class ActCommand: :meth:`JobExecutor._execute_process`: * :attr:`env_file` and :attr:`_executor_owned_env_file` are set when - :attr:`env` is non-empty (a ``0600`` temp file is created). If a - caller-supplied, unowned :attr:`env_file` already exists, its contents - are copied into the new temp file before :attr:`env` entries are written; - the original caller file is left on disk. + :attr:`env` is non-empty (a ``0600`` temp file is created). + * When materializing that file, if a caller-supplied, unowned + :attr:`env_file` already exists on disk, its contents are copied into + the new temp file before :attr:`env` entries are written; the original + caller file is left on disk. * :attr:`secret_file` and :attr:`_executor_owned_secret_file` are set when :attr:`secrets` is non-empty (a ``0600`` temp file is created).