fix: conservative success detection in GoldenPathRunner (#374)

codeframe/cli/app.py

@@ -2078,6 +2078,9 @@ def work_start(
                         latest_log = max(debug_logs, key=lambda p: p.stat().st_mtime)
                         console.print(f"\n[dim]Debug log written to: {latest_log}[/dim]")
 
+                if state.status in (AgentStatus.BLOCKED, AgentStatus.FAILED):
+                    raise typer.Exit(1)
+
             except ValueError as e:
                 console.print(f"[red]Error:[/red] {e}")
                 raise typer.Exit(1)

tests/cli/test_work_exit_codes.py

@@ -0,0 +1,82 @@
+"""Tests that `cf work start --execute` returns non-zero exit codes for BLOCKED/FAILED.
+
+Issue #374: CLI was returning exit code 0 even when the agent state was
+BLOCKED or FAILED, causing false positives in test automation.
+"""
+
+from dataclasses import dataclass, field
+from unittest.mock import patch
+
+import pytest
+from typer.testing import CliRunner
+
+from codeframe.cli.app import app
+from codeframe.core.agent import AgentStatus
+from codeframe.core import tasks
+from codeframe.core.state_machine import TaskStatus
+from codeframe.core.workspace import create_or_load_workspace
+
+pytestmark = pytest.mark.v2
+
+runner = CliRunner()
+
+
+@dataclass
+class _FakeAgentState:
+    status: AgentStatus
+    blocker: object = None
+    step_results: list = field(default_factory=list)
+
+
+@pytest.fixture
+def workspace_with_ready_task(tmp_path, monkeypatch):
+    """Workspace with one READY task, API key set."""
+    repo = tmp_path / "repo"
+    repo.mkdir()
+    monkeypatch.setenv("ANTHROPIC_API_KEY", "sk-ant-test-fake")
+
+    ws = create_or_load_workspace(repo)
+    task = tasks.create(ws, title="Test task", description="A test task",
+                        status=TaskStatus.READY)
+
+    return repo, task.id[:8]
+
+
+class TestWorkStartExitCodes:
+    """Verify CLI exit codes match agent execution outcomes."""
+
+    def test_completed_returns_exit_zero(self, workspace_with_ready_task):
+        repo, tid = workspace_with_ready_task
+        fake_state = _FakeAgentState(status=AgentStatus.COMPLETED)
+
+        with patch("codeframe.core.runtime.execute_agent", return_value=fake_state):
+            result = runner.invoke(
+                app, ["work", "start", tid, "--execute", "-w", str(repo)]
+            )
+
+        assert result.exit_code == 0, f"Expected 0 for COMPLETED: {result.output}"
+        assert "completed successfully" in result.output.lower()
+
+    def test_failed_returns_exit_one(self, workspace_with_ready_task):
+        repo, tid = workspace_with_ready_task
+        fake_state = _FakeAgentState(status=AgentStatus.FAILED)
+
+        with patch("codeframe.core.runtime.execute_agent", return_value=fake_state):
+            result = runner.invoke(
+                app, ["work", "start", tid, "--execute", "-w", str(repo)]
+            )
+
+        assert result.exit_code == 1, f"Expected 1 for FAILED: {result.output}"
+        assert "failed" in result.output.lower()
+
+    def test_blocked_returns_exit_one(self, workspace_with_ready_task):
+        repo, tid = workspace_with_ready_task
+        fake_state = _FakeAgentState(status=AgentStatus.BLOCKED)
+
+        with patch("codeframe.core.runtime.execute_agent", return_value=fake_state):
+            result = runner.invoke(
+                app, ["work", "start", tid, "--execute", "-w", str(repo)]
+            )
+
+        assert result.exit_code == 1, f"Expected 1 for BLOCKED: {result.output}"
+        assert "blocked" in result.output.lower()

tests/e2e/cli/golden_path_runner.py

@@ -315,8 +315,9 @@ def run_execute_task(self, task_id: str, task_title: str) -> TaskExecutionResult
     def _detect_success(self, exit_code: int, output: str) -> bool:
         """Determine actual success from CLI output patterns.
 
-        The CLI currently returns exit code 0 even for failed tasks,
-        so we inspect the output for success/failure markers.
+        Uses conservative detection: only returns True when an explicit
+        success pattern is found AND no failure patterns are present.
+        Returns False when output is ambiguous (no patterns matched).
         """
         if exit_code != 0:
             return False
@@ -331,15 +332,17 @@ def _detect_success(self, exit_code: int, output: str) -> bool:
             "Task completed successfully",
         ]
 
-        for pattern in success_patterns:
-            if pattern in output:
-                return True
+        # Check failure patterns first — failure takes precedence
         for pattern in failure_patterns:
             if pattern in output:
                 return False
 
-        # If no clear signal, fall back to exit code
-        return exit_code == 0
+        for pattern in success_patterns:
+            if pattern in output:
+                return True
+
+        # No clear signal — conservative default: treat as failure
+        return False
 
     def _extract_iterations(self, output: str) -> Optional[int]:
         """Extract iteration count from agent output."""

tests/e2e/cli/test_detect_success.py

@@ -0,0 +1,65 @@
+"""Unit tests for GoldenPathRunner._detect_success().
+
+Validates that the success detection logic uses conservative defaults:
+- Explicit success patterns → True
+- Explicit failure patterns → False
+- No patterns matched → False (not True!)
+- Non-zero exit code → always False
+"""
+
+import pytest
+
+from tests.e2e.cli.golden_path_runner import GoldenPathRunner
+
+pytestmark = pytest.mark.v2
+
+
+@pytest.fixture
+def runner(tmp_path):
+    """Create a GoldenPathRunner instance for testing."""
+    return GoldenPathRunner(project_path=tmp_path, engine="react")
+
+
+class TestDetectSuccess:
+    """Tests for _detect_success() conservative detection logic."""
+
+    def test_explicit_success_pattern_returns_true(self, runner):
+        output = "Some output\nTask completed successfully!\nDone."
+        assert runner._detect_success(exit_code=0, output=output) is True
+
+    def test_explicit_failure_pattern_returns_false(self, runner):
+        output = "Task execution failed\nSome error occurred"
+        assert runner._detect_success(exit_code=0, output=output) is False
+
+    def test_api_key_missing_returns_false(self, runner):
+        output = "ANTHROPIC_API_KEY environment variable is required"
+        assert runner._detect_success(exit_code=0, output=output) is False
+
+    def test_error_pattern_returns_false(self, runner):
+        output = "Error: something went wrong"
+        assert runner._detect_success(exit_code=0, output=output) is False
+
+    def test_blocked_pattern_returns_false(self, runner):
+        output = "Task blocked - needs human input"
+        assert runner._detect_success(exit_code=0, output=output) is False
+
+    def test_empty_output_exit_zero_returns_false(self, runner):
+        """The core bug: empty output should NOT be treated as success."""
+        assert runner._detect_success(exit_code=0, output="") is False
+
+    def test_no_patterns_exit_zero_returns_false(self, runner):
+        """Output with no matching patterns should be failure (conservative)."""
+        output = "Some random output that matches nothing"
+        assert runner._detect_success(exit_code=0, output=output) is False
+
+    def test_nonzero_exit_code_always_returns_false(self, runner):
+        output = "Task completed successfully!"
+        assert runner._detect_success(exit_code=1, output=output) is False
+
+    def test_nonzero_exit_code_with_no_output(self, runner):
+        assert runner._detect_success(exit_code=1, output="") is False
+
+    def test_mixed_success_and_failure_returns_false(self, runner):
+        """When both success and failure patterns present, failure wins."""
+        output = "Task completed successfully!\nBut then Error: crash"
+        assert runner._detect_success(exit_code=0, output=output) is False