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

codeframe/core/adapters/opencode.py

@@ -6,43 +6,100 @@
 
 from codeframe.core.adapters.subprocess_adapter import SubprocessAdapter
 
+#: Linux caps a *single* argv entry at MAX_ARG_STRLEN — 32 pages, 128 KiB —
+#: independently of the much larger total ARG_MAX. CodeFrame's context packager
+#: budgets 100K tokens (~400 KB of prompt), so a large task prompt passed as a
+#: positional raises OSError(E2BIG) before opencode ever starts. Verified:
+#: ``subprocess.run(["/bin/true", "x" * 200_000])`` → "Argument list too long".
+_MAX_ARG_BYTES = 128 * 1024
+
 
 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, timeout_s: int | None = None) -> None:
+        """Initialize the OpenCode adapter.
+
+        Args:
+            timeout_s: Max execution time, forwarded to ``SubprocessAdapter``
+                (same knob as ``KilocodeAdapter``). None keeps the 30-minute
+                default; tests bound it far lower so an opencode hang fails in
+                seconds rather than stalling the suite.
+            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,
+            timeout_s=timeout_s,
+            # 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"
 
+    @staticmethod
+    def _prompt_exceeds_argv(prompt: str) -> bool:
+        """True when the prompt is too large to survive as a single argv entry."""
+        return len(prompt.encode("utf-8")) >= _MAX_ARG_BYTES
+
     def build_command(self, prompt: str, workspace_path: Path) -> list[str]:
-        """Build opencode CLI command.
+        """Build the opencode CLI command.
+
+        ``opencode run`` declares ``message`` as a positional array, and that is
+        the form verified end-to-end against the CLI. An oversized prompt cannot
+        go that way (see ``_MAX_ARG_BYTES``), so it is omitted from argv and sent
+        on stdin instead — ``opencode run`` with no positional reads the message
+        from stdin, confirmed by its own ``prompt_submit`` log carrying the piped
+        text verbatim.
 
         Args:
-            prompt: The task prompt (sent via stdin, not in the command).
+            prompt: The task prompt.
             workspace_path: Workspace root (cwd is set by the base class).
 
         Returns:
             Command list for subprocess.Popen.
         """
-        return [self._binary_path, *self._cli_args]
+        cmd = [self._binary_path, *self._cli_args]
+        if not self._prompt_exceeds_argv(prompt):
+            cmd.append(prompt)
+        return cmd
 
     def get_stdin(self, prompt: str) -> str | None:
-        """Send prompt via stdin.
-
-        Args:
-            prompt: The task prompt to pipe into the opencode process.
+        """The prompt, but only when it did not fit in argv.
 
         Returns:
-            The prompt string.
+            None for a normal prompt — it is already a positional argument, and
+            sending it twice would duplicate the instruction. The prompt itself
+            when it was too large for argv.
         """
-        return prompt
+        return prompt if self._prompt_exceeds_argv(prompt) else 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,74 @@ 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.
+
+        Returning the prompt from get_stdin as well would send it 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_an_oversized_prompt_moves_to_stdin(self) -> None:
+        """Linux caps one argv entry at 128 KiB, well under the 100K-token budget.
+
+        Passing such a prompt positionally raises OSError(E2BIG) before opencode
+        starts — every large-prompt run would fail. Verified: `/bin/true` with a
+        200 KB argument raises "Argument list too long".
+        """
+        big = "x" * 200_000
+        with patch("shutil.which", return_value="/usr/bin/opencode"):
+            adapter = OpenCodeAdapter()
+
+        cmd = adapter.build_command(big, Path("/tmp"))
+        assert cmd == ["/usr/bin/opencode", "run"], "oversized prompt must leave argv"
+        assert adapter.get_stdin(big) == big, "…and must still reach opencode"
+
+        # The real limit, not just a big number: this is executable as argv.
+        assert all(len(a.encode()) < 128 * 1024 for a in cmd)
+
+    def test_the_prompt_is_never_sent_twice(self) -> None:
+        """Whichever transport is chosen, exactly one of them carries the prompt."""
+        with patch("shutil.which", return_value="/usr/bin/opencode"):
+            adapter = OpenCodeAdapter()
+
+        for prompt in ("small", "x" * 200_000):
+            in_argv = prompt in adapter.build_command(prompt, Path("/tmp"))
+            in_stdin = adapter.get_stdin(prompt) is not None
+            assert in_argv != in_stdin, f"prompt of {len(prompt)} bytes sent {in_argv + in_stdin}x"
+
+    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 +167,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