Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions agents/IMPLEMENTATION.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@

## Recently Completed (June 2026)

- **Graceful transient connection-error handling (Jul 13)**: A lost API connection (laptop asleep / no internet) during generation used to be swallowed — a single blip could mark a phase `completed` that never actually ran. Now: (1) `model_routing.classify_error` recognizes `AgentErrorType.CONNECTION_ERROR` via a narrow pattern list (`unable to connect to api`, `socket connection was closed`, `connection refused/reset`, `server disconnected`, …), checked before the 4xx/5xx guard. (2) `execute_all_phases` raises `WorkspaceAbortedError` on a `CONNECTION_ERROR` phase **before** the checkpoint write, so a lost phase is never falsely recorded (a later `retry_generation` re-runs it). **Retry itself is left to the Claude SDK default** (no custom wait-loop / config knobs). Surfacing an all-workspace failure up through the orchestrator (so a total connection loss ends in FAILED rather than "complete") and failure-notification were both intentionally descoped from this PR. Tests: `test_classify_error.py::TestConnectionError`, `test_execute_all_phases_connection_error.py` (backend 2011 passed / 36 skipped). Pre-existing, untouched mypy errors remain in `app/state/workspace_models.py:73`, `app/services/openrouter_api.py:21`, `app/services/model_catalog.py:80`.

- **Quickstart P10Y repository lookup pagination (Jul 8)**: Fixed Quickstart initialization for P10Y organizations with more than one repository page. `create_generation_session_repos.py` now searches generated repos with the qualified GitHub owner/prefix (for example `org/specflow-workspace`) and uses a shared paginated P10Y repository listing helper for ID resolution, re-fetch connection discovery, status checks, and status polling. Tests: `uv run pytest test/scripts/test_create_generation_session_repos.py -q` from `backend` (25 passed).

- **TUI workspace notification dedupe (Jul 1)**: Fixed workspace drill-in causing repeated desktop "Workspace phase progressed" notifications every poll during KB init. `MilestoneTracker` now emits workspace progress only when `last_completed_phase` increases, so label-only differences between session-list and `/status` payloads stay silent. `WorkspaceMessagesScreen` feeds its `/status` poll into the shared per-run tracker, and the app-wide active-session watcher excludes the workspace screen's generation to avoid mixed payload sources. Tests: `test_tui_poller.py::TestMilestoneTracker::test_kb_init_phase_label_change_without_progress_is_silent`, `test_tui_app.py::TestWorkspaceDrillIn::test_workspace_screen_excludes_app_level_session_watcher`; broader TUI check: `uv run pytest tests/test_tui_app.py tests/test_tui_poller.py -v` from `mcp_server` (78 passed).
Expand Down
1 change: 1 addition & 0 deletions backend/app/schemas/agent.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
class AgentErrorType(str, Enum):
TOOL_CALL_FAILURE = "tool_call_failure"
MODEL_ROUTING_FAILURE = "model_routing_failure"
CONNECTION_ERROR = "connection_error"


@dataclass
Expand Down
6 changes: 6 additions & 0 deletions backend/app/services/claude_code.py
Original file line number Diff line number Diff line change
Expand Up @@ -1270,6 +1270,12 @@ async def execute_all_phases(
f"(model={workspace.model}). Aborting workspace — other workspaces continue. "
f"Error: {phase_result.result}"
)
if error_type == AgentErrorType.CONNECTION_ERROR:
raise WorkspaceAbortedError(
f"[{workspace_name}] Phase {phase_num} lost connection to the API "
f"(model={workspace.model}). Aborting workspace — other workspaces continue. "
f"Error: {phase_result.result}"
)
consecutive_errors += 1
logger.error(
f"[{workspace_name}] Phase {phase_num} reported is_error=True "
Expand Down
15 changes: 15 additions & 0 deletions backend/app/services/model_routing.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,26 @@
"empty response",
]

# Transient network/connection failures (laptop asleep, lost internet) — retryable, no HTTP status.
_CONNECTION_ERROR_PATTERNS = [
"unable to connect to api",
"connectionrefused",
"connection refused",
"socket connection was closed",
"connection error",
"connection reset",
"server disconnected",
"network is unreachable",
]

def classify_error(error_msg: str, api_error_status: Optional[int] = None) -> Optional[AgentErrorType]:
"""Classify an agent error for targeted logging and telemetry."""
msg_lower = error_msg.lower()
if any(p.lower() in msg_lower for p in _TOOL_CALL_ERROR_PATTERNS):
return AgentErrorType.TOOL_CALL_FAILURE
# Connection failure checked before the 4xx/5xx guard (there is no HTTP status) and before routing.
if any(p in msg_lower for p in _CONNECTION_ERROR_PATTERNS):
return AgentErrorType.CONNECTION_ERROR
# 4xx/5xx = definitive HTTP error, not a routing failure. Routing failures return HTTP 200
# with a malformed body, so api_error_status=200 (or None) must NOT be excluded here.
if api_error_status is not None and api_error_status >= 400:
Expand Down
38 changes: 37 additions & 1 deletion backend/test/test_classify_error.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,9 @@ def test_empty_response_case_insensitive(self):
assert classify_error("EMPTY RESPONSE received") == AgentErrorType.MODEL_ROUTING_FAILURE

def test_does_not_match_unrelated_errors(self):
assert classify_error("connection refused") is None
# "rate limit" and generic "timeout" are intentionally NOT connection errors (out of
# scope for the transient-connection retry). "connection refused" now classifies as
# CONNECTION_ERROR — see TestConnectionError.
assert classify_error("rate limit exceeded") is None
assert classify_error("timeout after 30s") is None

Expand All @@ -58,6 +60,40 @@ def test_tool_call_takes_priority_over_routing(self):
assert classify_error(msg) == AgentErrorType.TOOL_CALL_FAILURE


class TestConnectionError:
"""Transient network/connection failures — retryable, HTTP-status-independent."""

def test_unable_to_connect_to_api(self):
# The exact string the SDK emits when the laptop has no internet.
assert classify_error("API Error: Unable to connect to API (ConnectionRefused)") == AgentErrorType.CONNECTION_ERROR

def test_socket_connection_closed(self):
assert classify_error("API Error: The socket connection was closed unexpectedly.") == AgentErrorType.CONNECTION_ERROR

def test_connection_refused(self):
assert classify_error("connection refused") == AgentErrorType.CONNECTION_ERROR

def test_connection_reset(self):
assert classify_error("Connection reset by peer") == AgentErrorType.CONNECTION_ERROR

def test_server_disconnected(self):
assert classify_error("Server disconnected without sending a response.") == AgentErrorType.CONNECTION_ERROR

def test_case_insensitive(self):
assert classify_error("UNABLE TO CONNECT TO API") == AgentErrorType.CONNECTION_ERROR

def test_connection_wins_over_spurious_status_in_message(self):
# A stray number must not demote a real connection error to unclassified.
msg = "Unable to connect to API (ConnectionRefused) after 500ms"
inferred = extract_http_status_from_message(msg)
assert inferred == 500 # extractor would pick this up...
assert classify_error(msg, api_error_status=inferred) == AgentErrorType.CONNECTION_ERROR # ...but connection still wins

def test_tool_call_takes_priority_over_connection(self):
# tool-call incompatibility must abort immediately, never be retried as transient.
assert classify_error("tool_use failed: connection error") == AgentErrorType.TOOL_CALL_FAILURE


class TestApiErrorStatusExclusion:
def test_known_http_error_not_classified_as_routing_failure(self):
msg = "API Error: upstream overloaded"
Expand Down
111 changes: 111 additions & 0 deletions backend/test/test_execute_all_phases_connection_error.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
"""
execute_all_phases: a lost API connection (surfaced after the SDK's own retries are
exhausted) must abort the workspace BEFORE the checkpoint write, so the phase is never
falsely recorded as completed — a later retry_generation must re-run it, not skip it.
"""

from __future__ import annotations

from pathlib import Path
from unittest.mock import AsyncMock, Mock, patch

import pytest

from app.schemas.agent import AgentResult
from app.schemas.planning import PhaseInfo, PlanningResult
from app.schemas.specification import GenerateAppRequest
from app.services.claude_code import WorkspaceAbortedError, execute_all_phases


def _mock_workspace(tmp_path: Path) -> Mock:
ws = Mock()
ws.workspace_path = tmp_path
ws.get_isolated_root = Mock(return_value=str(tmp_path))
ws.model = "m"
ws.provider = "openrouter"
return ws


def _mock_manager() -> Mock:
mgr = Mock()
mgr.settings = Mock()
mgr.settings.ROSETTA_MCP_ENABLED = False
mgr.commit_and_push_outstanding = AsyncMock(return_value=True)
return mgr


def _planning_one_phase() -> PlanningResult:
return PlanningResult(
phase_count=1,
phases=[PhaseInfo(number=1, name="Backend", description="API", estimated_commits=1)],
plan_file_path="specflow/plan.md",
)


@pytest.mark.asyncio
async def test_connection_error_aborts_without_checkpoint(tmp_path: Path) -> None:
"""A connection-lost phase raises WorkspaceAbortedError and never writes the checkpoint."""
planning = _planning_one_phase()
mock_ws = _mock_workspace(tmp_path)
mock_mgr = _mock_manager()
request = GenerateAppRequest(spec_path="specs", outputs_dir="specflow", generation_id="e1")

svc = Mock()
svc.update_workspace_phase = AsyncMock()
svc.update_deployment_workspace_phase = AsyncMock()

async def fake_phase_fn(**kwargs):
return AgentResult(
result="API Error: Unable to connect to API (ConnectionRefused)",
session_id=None,
is_error=True,
)

with patch("app.services.claude_code.phase_agent_fn", side_effect=fake_phase_fn), \
patch("app.services.claude_code.is_skip_mode_enabled", return_value=False):
with pytest.raises(WorkspaceAbortedError) as exc_info:
await execute_all_phases(
planning_data=planning,
workspace=mock_ws,
manager=mock_mgr,
request=request,
logger=Mock(),
generation_session_service=svc,
workspace_id="ws-1",
)

assert "lost connection" in str(exc_info.value).lower()
# The linchpin assertion: the errored phase was NOT checkpointed as completed.
svc.update_workspace_phase.assert_not_called()
svc.update_deployment_workspace_phase.assert_not_called()


@pytest.mark.asyncio
async def test_connection_error_aborts_on_first_occurrence(tmp_path: Path) -> None:
"""Unlike a generic error (tolerated up to 2), a single connection loss aborts immediately."""
planning = _planning_one_phase()
request = GenerateAppRequest(spec_path="specs", outputs_dir="specflow", generation_id="e1")

calls = {"n": 0}

async def fake_phase_fn(**kwargs):
calls["n"] += 1
return AgentResult(
result="API Error: The socket connection was closed unexpectedly.",
session_id=None,
is_error=True,
)

with patch("app.services.claude_code.phase_agent_fn", side_effect=fake_phase_fn), \
patch("app.services.claude_code.is_skip_mode_enabled", return_value=False):
with pytest.raises(WorkspaceAbortedError):
await execute_all_phases(
planning_data=planning,
workspace=_mock_workspace(tmp_path),
manager=_mock_manager(),
request=request,
logger=Mock(),
)

# Aborted after the very first errored phase — no second attempt.
assert calls["n"] == 1