diff --git a/packages/pickled-core/tests/test_llm_sanitize.py b/packages/pickled-core/tests/test_llm_sanitize.py index 7f52d74..00c946d 100644 --- a/packages/pickled-core/tests/test_llm_sanitize.py +++ b/packages/pickled-core/tests/test_llm_sanitize.py @@ -246,6 +246,7 @@ def count_tokens(self, messages: list[Message], model: str) -> int: with ( patch("pickled_iac.drafter.iac_binary", return_value="terraform"), + patch("pickled_iac.drafter.iac_format", return_value="terraform"), patch( "pickled_iac.drafter.validate", return_value=ValidateResult(valid=True, diagnostics=()), diff --git a/packages/pickled-iac/src/pickled_iac/drafter.py b/packages/pickled-iac/src/pickled_iac/drafter.py index bb3aca6..741c8ff 100644 --- a/packages/pickled-iac/src/pickled_iac/drafter.py +++ b/packages/pickled-iac/src/pickled_iac/drafter.py @@ -9,7 +9,7 @@ from pickled_core.llm.sanitize import strip_markdown_fence from pickled_core.llm.turns import complete_prompt -from pickled_iac.oracle import iac_binary, validate +from pickled_iac.oracle import iac_binary, iac_format, validate from pickled_iac.types import IaCArtifact @@ -30,8 +30,8 @@ def draft_module( provider: str = "aws", ) -> IaCArtifact: """Draft, validate in a temp dir, and return an IaCArtifact.""" - binary = iac_binary() - fmt: str = "opentofu" if binary == "opentofu" else "terraform" + iac_binary() + fmt = iac_format() last_error = "" for _attempt in range(3): @@ -56,7 +56,7 @@ def draft_module( (root / "main.tf").write_text(hcl, encoding="utf-8") result = validate(root) if result.valid: - return IaCArtifact(content=hcl, format=fmt, path=None) # type: ignore[arg-type] + return IaCArtifact(content=hcl, format=fmt, path=None) last_error = "; ".join(result.diagnostics) or "validation failed" msg = f"failed to draft valid Terraform after 3 attempts: {last_error}" diff --git a/packages/pickled-iac/src/pickled_iac/oracle.py b/packages/pickled-iac/src/pickled_iac/oracle.py index 8d02aac..1a9fc82 100644 --- a/packages/pickled-iac/src/pickled_iac/oracle.py +++ b/packages/pickled-iac/src/pickled_iac/oracle.py @@ -12,17 +12,22 @@ from pickled_iac.types import IaCToolMissingError, PlanResult, ValidateResult -_IAC_BIN: Literal["terraform", "opentofu"] | None +# The executable name to invoke via subprocess. OpenTofu's binary is ``tofu`` +# (see https://opentofu.org/docs/intro/install/); ``opentofu`` is the format +# label only, never an installed binary, so we MUST keep these two separate +# or every drafter / validate / plan call fails with FileNotFoundError on +# OpenTofu-only hosts. +_IAC_BIN: Literal["terraform", "tofu"] | None if shutil.which("terraform"): _IAC_BIN = "terraform" elif shutil.which("tofu"): - _IAC_BIN = "opentofu" + _IAC_BIN = "tofu" else: _IAC_BIN = None -def iac_binary() -> Literal["terraform", "opentofu"]: - """Return the detected IaC CLI binary name.""" +def iac_binary() -> Literal["terraform", "tofu"]: + """Return the IaC CLI executable name to pass to :func:`subprocess.run`.""" if _IAC_BIN is None: raise IaCToolMissingError( "neither 'terraform' nor 'tofu' found on PATH; " @@ -31,6 +36,11 @@ def iac_binary() -> Literal["terraform", "opentofu"]: return _IAC_BIN +def iac_format() -> Literal["terraform", "opentofu"]: + """Return the human-facing format label (``terraform`` or ``opentofu``).""" + return "opentofu" if iac_binary() == "tofu" else "terraform" + + def _run(cmd: list[str], *, cwd: Path) -> subprocess.CompletedProcess[str]: return subprocess.run( cmd, @@ -42,7 +52,7 @@ def _run(cmd: list[str], *, cwd: Path) -> subprocess.CompletedProcess[str]: ) -def _init_if_needed(tf_dir: Path, binary: Literal["terraform", "opentofu"]) -> None: +def _init_if_needed(tf_dir: Path, binary: Literal["terraform", "tofu"]) -> None: if (tf_dir / ".terraform").exists(): return init = _run([binary, "init", "-input=false", "-backend=false"], cwd=tf_dir) @@ -57,7 +67,7 @@ def validate(tf_dir: Path) -> ValidateResult: binary = iac_binary() _init_if_needed(tf_dir, binary) proc = _run([binary, "validate", "-json"], cwd=tf_dir) - fmt: Literal["terraform", "opentofu"] = "opentofu" if binary == "opentofu" else "terraform" + fmt = iac_format() if proc.returncode != 0 and not proc.stdout.strip(): err = (proc.stderr or "validate failed").strip() return ValidateResult(valid=False, diagnostics=[err], format=fmt) @@ -94,7 +104,7 @@ def plan(tf_dir: Path, out_file: Path) -> PlanResult: msg = f"{binary} show failed: {err}" raise RuntimeError(msg) plan_json = json.loads(show_proc.stdout or "{}") - fmt: Literal["terraform", "opentofu"] = "opentofu" if binary == "opentofu" else "terraform" + fmt = iac_format() return PlanResult(plan_json=plan_json, plan_file=out_file, format=fmt) @@ -157,6 +167,7 @@ def plan_json_from_dict(plan_data: dict[str, Any]) -> dict[str, Any]: __all__ = [ "UnsafeTerraformFilenameError", "iac_binary", + "iac_format", "plan", "plan_json_from_dict", "validate", diff --git a/packages/pickled-iac/tests/test_drafter.py b/packages/pickled-iac/tests/test_drafter.py index bc2d041..8a386f7 100644 --- a/packages/pickled-iac/tests/test_drafter.py +++ b/packages/pickled-iac/tests/test_drafter.py @@ -51,7 +51,20 @@ def test_drafter_returns_artifact_when_validate_passes() -> None: with ( patch("pickled_iac.drafter.validate", return_value=ValidateResult(valid=True)), patch("pickled_iac.drafter.iac_binary", return_value="terraform"), + patch("pickled_iac.drafter.iac_format", return_value="terraform"), ): artifact = IaCDrafter(fake).draft_module("Need a bucket", provider="aws") assert "aws_s3_bucket" in artifact.content assert artifact.format == "terraform" + + +def test_drafter_uses_opentofu_label_when_only_tofu_on_path() -> None: + """OpenTofu's binary is ``tofu``; format label stays ``opentofu``.""" + fake = FakeLLMClient(_VALID_TF) + with ( + patch("pickled_iac.drafter.validate", return_value=ValidateResult(valid=True)), + patch("pickled_iac.drafter.iac_binary", return_value="tofu"), + patch("pickled_iac.drafter.iac_format", return_value="opentofu"), + ): + artifact = IaCDrafter(fake).draft_module("Need a bucket", provider="aws") + assert artifact.format == "opentofu" diff --git a/packages/pickled-iac/tests/test_oracle_binary.py b/packages/pickled-iac/tests/test_oracle_binary.py new file mode 100644 index 0000000..baa74a4 --- /dev/null +++ b/packages/pickled-iac/tests/test_oracle_binary.py @@ -0,0 +1,101 @@ +"""Regression tests for the binary-name vs format-label split. + +Historically :func:`iac_binary` returned ``"opentofu"`` when only ``tofu`` +was on ``PATH``; that string was then handed straight to +:func:`subprocess.run`, which raised +``FileNotFoundError: [Errno 2] No such file or directory: 'opentofu'`` +because the OpenTofu CLI ships as ``tofu``, never ``opentofu``. Every +``pickled-iac draft`` invocation and every ``validate_terraform_dir`` MCP +call was broken on OpenTofu-only hosts. The two concerns must stay split: +``iac_binary()`` returns the *executable* name; ``iac_format()`` returns the +human-facing *format label*. + +These tests deliberately do not require ``terraform`` or ``tofu`` to be on +``PATH``; they exercise the resolution logic in isolation via patching. +""" + +from __future__ import annotations + +import subprocess +import sys +from pathlib import Path +from unittest.mock import patch + +import pickled_iac.oracle as oracle_mod +import pytest +from pickled_iac.oracle import iac_binary, iac_format, validate +from pickled_iac.types import IaCToolMissingError + + +def test_iac_binary_returns_tofu_when_only_tofu_installed() -> None: + with patch.object(oracle_mod, "_IAC_BIN", "tofu"): + assert iac_binary() == "tofu" + assert iac_format() == "opentofu" + + +def test_iac_binary_returns_terraform_when_terraform_installed() -> None: + with patch.object(oracle_mod, "_IAC_BIN", "terraform"): + assert iac_binary() == "terraform" + assert iac_format() == "terraform" + + +def test_iac_binary_raises_when_neither_installed() -> None: + with patch.object(oracle_mod, "_IAC_BIN", None), pytest.raises(IaCToolMissingError): + iac_binary() + + +def test_validate_uses_tofu_executable_not_opentofu(tmp_path: Path) -> None: + """End-to-end: validate() must spawn ``tofu`` (not ``opentofu``) on tofu hosts.""" + bin_dir = tmp_path / "bin" + bin_dir.mkdir() + fake_tofu = bin_dir / "tofu" + fake_tofu.write_text( + f"#!{sys.executable}\n" + "import sys\n" + "if len(sys.argv) > 1 and sys.argv[1] == 'validate':\n" + " print('{\"valid\": true, \"diagnostics\": []}')\n" + "sys.exit(0)\n", + encoding="utf-8", + ) + fake_tofu.chmod(0o755) + + cfg_dir = tmp_path / "cfg" + cfg_dir.mkdir() + (cfg_dir / "main.tf").write_text("# empty\n", encoding="utf-8") + + # Prepend our fake bin dir so PATH lookup hits ``tofu`` first, but keep the + # standard PATH segments so the python shebang resolves. + new_path = f"{bin_dir}:{Path(sys.executable).parent}:/usr/bin:/bin" + + with ( + patch.object(oracle_mod, "_IAC_BIN", "tofu"), + patch.dict("os.environ", {"PATH": new_path}, clear=False), + ): + # Sanity-check the failure mode the fix prevents: subprocess.run on the + # bogus name "opentofu" must raise FileNotFoundError (no such binary). + with pytest.raises(FileNotFoundError): + subprocess.run(["opentofu", "validate"], check=False) # noqa: S603,S607 + + result = validate(cfg_dir) + assert result.valid is True + assert result.format == "opentofu" + + +def test_oracle_module_uses_tofu_as_actual_binary_name() -> None: + """``_IAC_BIN`` must hold the executable name, not the format label. + + Source-level guard so a future refactor cannot silently re-introduce the + ``opentofu``-as-binary bug while leaving the runtime branch unreached on + terraform-only CI. + """ + src = Path(oracle_mod.__file__).read_text(encoding="utf-8") + assert '_IAC_BIN = "tofu"' in src, "OpenTofu binary must be referenced as 'tofu'" + assert '_IAC_BIN = "opentofu"' not in src, ( + "Found stale '_IAC_BIN = \"opentofu\"' assignment — subprocess.run " + "would raise FileNotFoundError because no 'opentofu' executable exists " + "(OpenTofu installs as 'tofu')." + ) + + +if __name__ == "__main__": + sys.exit(pytest.main([__file__, "-v"]))