fix(engines): invoke opencode run and require file changes (#913)

codeframe/core/adapters/opencode.py

@@ -10,39 +10,69 @@
 class OpenCodeAdapter(SubprocessAdapter):
     """Adapter that delegates code execution to OpenCode CLI.
 
-    Invokes ``opencode`` with ``--non-interactive`` flag for headless execution.
-    The prompt is piped via stdin.
+    Runs ``opencode run <message>`` — the CLI's headless entry point. The
+    previous invocation was ``opencode --non-interactive`` with the prompt on
+    stdin, and **no such flag exists**: verified against opencode 1.18.7,
+    ``--non-interactive`` is absent from the option list and passing it simply
+    starts the TUI, so the delegated run did no work at all (#913).
 
-    Requires OpenCode to be installed:
-    https://github.com/opencode-ai/opencode
+    Requires OpenCode to be installed: https://github.com/sst/opencode
     """
 
-    def __init__(self) -> None:
-        super().__init__(binary="opencode", cli_args=["--non-interactive"])
+    def __init__(self, auto_approve: bool = False) -> None:
+        """Initialize the OpenCode adapter.
+
+        Args:
+            auto_approve: Pass ``--auto``, which opencode documents as
+                "auto-approve permissions that are not explicitly denied
+                (dangerous!)". **Off by default.** Verified against opencode
+                1.18.7 that a plain ``opencode run <message>`` writes files
+                headlessly under the default permission config, so the flag is
+                not needed to make the engine work — and turning it on would
+                auto-approve arbitrary actions for a prompt derived from
+                repository content, the exposure #905–#907 exist to close.
+
+                Where an operator's opencode config *does* deny writes, the run
+                produces no file changes and ``require_file_changes`` below turns
+                that into a loud failure rather than a silent false completion.
+        """
+        cli_args = ["run"]
+        if auto_approve:
+            cli_args.append("--auto")
+
+        super().__init__(
+            binary="opencode",
+            cli_args=cli_args,
+            # A coding agent that exits 0 having written nothing is a false
+            # completion: gates then run on an unchanged tree and the task can be
+            # marked DONE with no code. Same guard the claude-code adapter got
+            # in #739/#819.
+            require_file_changes=True,
+        )
+        self._auto_approve = auto_approve
 
     @property
     def name(self) -> str:  # noqa: D102
         return "opencode"
 
     def build_command(self, prompt: str, workspace_path: Path) -> list[str]:
-        """Build opencode CLI command.
+        """Build the opencode CLI command.
 
         Args:
-            prompt: The task prompt (sent via stdin, not in the command).
+            prompt: The task prompt — a positional argument to ``run``, not
+                stdin. ``opencode run`` declares ``message`` as a positional
+                array; it does not read the prompt from stdin.
             workspace_path: Workspace root (cwd is set by the base class).
 
         Returns:
             Command list for subprocess.Popen.
         """
-        return [self._binary_path, *self._cli_args]
+        return [self._binary_path, *self._cli_args, prompt]
 
     def get_stdin(self, prompt: str) -> str | None:
-        """Send prompt via stdin.
-
-        Args:
-            prompt: The task prompt to pipe into the opencode process.
+        """No stdin: the prompt travels as an argument to ``run``.
 
         Returns:
-            The prompt string.
+            None — sending it twice would duplicate the instruction.
         """
-        return prompt
+        return None

tests/core/adapters/test_opencode.py

@@ -14,8 +14,17 @@ class TestOpenCodeAdapter:
 
     @pytest.fixture(autouse=True)
     def _no_git(self):
-        """Prevent _detect_modified_files from calling real git."""
-        with patch.object(OpenCodeAdapter, "_detect_modified_files", return_value=[]):
+        """Prevent _detect_modified_files from calling real git.
+
+        Returns a modified file, i.e. a run that actually did work. With
+        ``require_file_changes=True`` (#913) an empty list means "wrote
+        nothing", which is now a *failure* — so an empty default would make
+        every test here exercise the false-completion guard rather than the
+        behaviour it names. The no-work case has its own test below.
+        """
+        with patch.object(
+            OpenCodeAdapter, "_detect_modified_files", return_value=["main.py"]
+        ), patch.object(OpenCodeAdapter, "_git_head", return_value="abc123"):
             yield
 
     def test_name(self) -> None:
@@ -33,17 +42,46 @@ def test_raises_if_opencode_not_installed(self) -> None:
             with pytest.raises(EnvironmentError, match="not found on PATH"):
                 OpenCodeAdapter()
 
-    def test_build_command_includes_non_interactive(self) -> None:
+    def test_build_command_uses_the_run_subcommand(self) -> None:
+        """`--non-interactive` does not exist; it silently starts the TUI (#913)."""
         with patch("shutil.which", return_value="/usr/bin/opencode"):
             adapter = OpenCodeAdapter()
             cmd = adapter.build_command("prompt", Path("/tmp"))
-            assert cmd[0] == "/usr/bin/opencode"
-            assert "--non-interactive" in cmd
 
-    def test_sends_prompt_via_stdin(self) -> None:
+        assert cmd == ["/usr/bin/opencode", "run", "prompt"]
+        assert "--non-interactive" not in cmd
+
+    def test_the_prompt_is_an_argument_not_stdin(self) -> None:
+        """`opencode run` declares `message` as a positional; it reads no stdin.
+
+        Returning the prompt here as well would send the instruction twice.
+        """
+        with patch("shutil.which", return_value="/usr/bin/opencode"):
+            adapter = OpenCodeAdapter()
+
+        assert adapter.get_stdin("my prompt") is None
+        assert adapter.build_command("my prompt", Path("/tmp"))[-1] == "my prompt"
+
+    def test_a_zero_work_run_is_not_reported_completed(self) -> None:
+        """Exit 0 with nothing written is a false completion: gates would then
+        run on an unchanged tree and the task could be marked DONE with no code
+        (#739's class, which the claude-code adapter was already patched for)."""
         with patch("shutil.which", return_value="/usr/bin/opencode"):
             adapter = OpenCodeAdapter()
-            assert adapter.get_stdin("my prompt") == "my prompt"
+
+        assert adapter._require_file_changes is True
+
+    def test_auto_approve_is_off_by_default(self) -> None:
+        """opencode documents --auto as "(dangerous!)" — it auto-approves
+        anything not explicitly denied, for a prompt derived from repository
+        content. Verified against 1.18.7 that plain `run` writes files, so the
+        flag is not needed to make the engine work."""
+        with patch("shutil.which", return_value="/usr/bin/opencode"):
+            default = OpenCodeAdapter()
+            opted_in = OpenCodeAdapter(auto_approve=True)
+
+        assert "--auto" not in default.build_command("p", Path("/tmp"))
+        assert "--auto" in opted_in.build_command("p", Path("/tmp"))
 
     def test_successful_execution(self) -> None:
         with patch("shutil.which", return_value="/usr/bin/opencode"):
@@ -101,3 +139,51 @@ def test_event_callback_receives_output_lines(self) -> None:
         assert len(events) == 2
         assert events[0].data["line"] == "line one"
         assert events[1].data["line"] == "line two"
+
+
+class TestZeroWorkIsNotCompleted:
+    """The false-completion guard, exercised through `run` (#913)."""
+
+    @pytest.fixture(autouse=True)
+    def _stable_head(self):
+        """HEAD unchanged, so only the modified-file signal decides."""
+        with patch.object(OpenCodeAdapter, "_git_head", return_value="abc123"):
+            yield
+
+    def _adapter(self) -> OpenCodeAdapter:
+        with patch("shutil.which", return_value="/usr/bin/opencode"):
+            return OpenCodeAdapter()
+
+    def _exit_zero_process(self) -> MagicMock:
+        proc = MagicMock()
+        proc.stdout = iter(["I reviewed the code and everything looks fine.\n"])
+        proc.stderr = MagicMock()
+        proc.stderr.read.return_value = ""
+        proc.stdin = MagicMock()
+        proc.returncode = 0
+        proc.wait.return_value = None
+        return proc
+
+    def test_exit_zero_writing_nothing_is_not_completed(self) -> None:
+        """The reported failure mode: the CLI exits 0 having done no work, gates
+        then run on an unchanged tree, and the task can be marked DONE with no
+        code written."""
+        adapter = self._adapter()
+
+        with patch.object(OpenCodeAdapter, "_detect_modified_files", return_value=[]):
+            with patch("subprocess.Popen", return_value=self._exit_zero_process()):
+                result = adapter.run("task-1", "implement feature", Path("/tmp/repo"))
+
+        assert result.status != "completed", result.status
+
+    def test_exit_zero_with_changes_is_completed(self) -> None:
+        """The guard must not fail runs that genuinely did the work."""
+        adapter = self._adapter()
+
+        with patch.object(
+            OpenCodeAdapter, "_detect_modified_files", return_value=["main.py"]
+        ):
+            with patch("subprocess.Popen", return_value=self._exit_zero_process()):
+                result = adapter.run("task-1", "implement feature", Path("/tmp/repo"))
+
+        assert result.status == "completed", result.status

tests/core/adapters/test_opencode_smoke_913.py

@@ -0,0 +1,143 @@
+"""Binary-gated smoke test: the OpenCode adapter against the real CLI (#913).
+
+The adapter shipped invoking ``opencode --non-interactive`` with the prompt on
+stdin. **No such flag exists.** Verified against opencode 1.18.7: it is absent
+from the option list, and passing it starts the TUI — so the delegated run did
+no work, exited non-zero on the help path, and unit tests built entirely from
+mocks could not tell.
+
+That is the gap this file closes. Every assertion here runs the adapter's own
+``build_command`` output against the installed binary, so a future change to
+the invocation that unit tests would happily mock is caught here instead.
+
+Skipped when ``opencode`` is not installed, so CI without the binary stays
+green — but it is **launch-gating**, not deferred: the engine is one of the
+shipped multi-model options.
+"""
+
+from __future__ import annotations
+
+import shutil
+import subprocess
+from pathlib import Path
+
+import pytest
+
+from codeframe.core.adapters.opencode import OpenCodeAdapter
+
+pytestmark = [
+    pytest.mark.v2,
+    pytest.mark.skipif(
+        shutil.which("opencode") is None, reason="opencode CLI not installed"
+    ),
+]
+
+#: Long enough for a real model round-trip, short enough to fail a hang loudly.
+_TIMEOUT_S = 240
+
+
+@pytest.fixture
+def repo(tmp_path: Path) -> Path:
+    workspace = tmp_path / "repo"
+    workspace.mkdir()
+    subprocess.run(["git", "init", "-q"], cwd=workspace, check=True)
+    return workspace
+
+
+def test_the_run_subcommand_exists_and_non_interactive_does_not() -> None:
+    """The assumption that produced the bug, checked against the binary.
+
+    Asserted on the CLI's own help rather than on our code, so this fails if a
+    future opencode release moves the headless entry point out from under us.
+    """
+    proc = subprocess.run(
+        ["opencode", "--help"], capture_output=True, text=True, timeout=60
+    )
+    # opencode prints help on stderr, not stdout.
+    help_text = proc.stdout + proc.stderr
+
+    assert "opencode run" in help_text, "the headless entry point moved"
+    assert "--non-interactive" not in help_text, (
+        "--non-interactive now exists; revisit the adapter's invocation"
+    )
+
+
+def test_the_adapter_command_actually_writes_a_file(repo: Path) -> None:
+    """Runs exactly what the adapter builds, and asserts on the file it produces.
+
+    Not "exit 0" — the old invocation could exit 0 and do nothing, which is the
+    whole false-completion class here.
+    """
+    adapter = OpenCodeAdapter()
+    cmd = adapter.build_command(
+        "Create a file named smoke.txt containing exactly the word: works", repo
+    )
+
+    try:
+        proc = subprocess.run(
+            cmd, cwd=repo, capture_output=True, text=True, timeout=_TIMEOUT_S
+        )
+    except subprocess.TimeoutExpired:
+        pytest.skip(f"opencode did not complete within {_TIMEOUT_S}s")
+
+    produced = repo / "smoke.txt"
+    if not produced.exists() and "smoke.txt" in proc.stdout:
+        # opencode reported writing the file but it is not in the workspace —
+        # it resolves its project directory from the *parent* process, ignoring
+        # the subprocess cwd every other adapter relies on. Tracked as
+        # #1007 [P0.27]; distinct from this issue's contract.
+        pytest.xfail("opencode ignored cwd and wrote outside the workspace")
+
+    assert produced.exists(), (
+        f"the adapter's command wrote nothing.\n"
+        f"cmd={cmd}\nstdout={proc.stdout[-2000:]}\nstderr={proc.stderr[-2000:]}"
+    )
+    assert "works" in produced.read_text()
+
+
+def test_the_old_invocation_does_no_work(repo: Path) -> None:
+    """Pins the actual bug, so nobody restores the flag believing it worked.
+
+    ``opencode --non-interactive`` prints the banner/usage and writes nothing.
+    """
+    proc = subprocess.run(
+        ["opencode", "--non-interactive"],
+        cwd=repo,
+        input="Create a file named old.txt containing: works",
+        capture_output=True,
+        text=True,
+        timeout=60,
+    )
+
+    assert not (repo / "old.txt").exists(), (
+        "the old invocation wrote a file; the premise of #913 no longer holds"
+    )
+    assert proc.returncode != 0 or "Commands:" in proc.stdout
+
+
+def test_a_run_that_writes_nothing_is_not_reported_completed(repo: Path) -> None:
+    """End to end through `adapter.run`, in a real git repo, with a prompt that
+    asks for no file changes — the guard must catch it."""
+    adapter = OpenCodeAdapter()
+
+    try:
+        result = adapter.run(
+            "task-smoke",
+            "Reply with the single word ACKNOWLEDGED. Do not create, edit or "
+            "delete any files.",
+            repo,
+        )
+    except subprocess.TimeoutExpired:
+        pytest.skip("opencode did not complete in time")
+
+    changed = subprocess.run(
+        ["git", "status", "--porcelain"], cwd=repo, capture_output=True, text=True
+    ).stdout.strip()
+
+    if changed:
+        pytest.skip(f"the model wrote files anyway: {changed!r}")
+
+    assert result.status != "completed", (
+        f"a run that wrote nothing was reported {result.status!r} — gates would "
+        f"then pass on an unchanged tree"
+    )