Document ActCommand mutation contract and add integration coverage (#70)

cli/localci/core/executor.py

@@ -146,7 +146,32 @@ 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).
+    * 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).
+
+    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 +465,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 +585,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(".")

cli/tests/integration/test_act_command_mutation.py

@@ -0,0 +1,107 @@
+"""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,
+        # 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
+
+
+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"
+    )