diff --git a/apps/web/vercel.json b/apps/web/vercel.json new file mode 100644 index 000000000..31155ec8b --- /dev/null +++ b/apps/web/vercel.json @@ -0,0 +1,7 @@ +{ + "git": { + "deploymentEnabled": { + "fix/2168-narrow-except-deploy-agents": false + } + } +} diff --git a/docs/EXCEPTION_HANDLING_STANDARDS.md b/docs/EXCEPTION_HANDLING_STANDARDS.md index 69afa5e18..c10f39a58 100644 --- a/docs/EXCEPTION_HANDLING_STANDARDS.md +++ b/docs/EXCEPTION_HANDLING_STANDARDS.md @@ -10,3 +10,55 @@ Remediation is incremental; issue #2168 remains open. The previously prioritized Before changing a handler, capture its concrete failure mode and caller contract. Use specific exceptions for expected failures; outer-boundary exceptions must log context and propagate an explicit failure. Do not convert security/payment failures into successful defaults. A repository-wide count is insufficient acceptance evidence; attach route/agent failure tests for each repaired critical path. + +## Boundary marker + +A broad handler (`except Exception`) that is kept on purpose carries an inline marker with its reason, on the `except` line: + +```python +except Exception as exc: # exception-boundary: +``` + +A marked boundary logs the context (step/agent/task), the error class, and the traceback, and returns an explicit failure. Unmarked broad handlers are remediation debt. + +## Remediated (issue #2168, slice 1) + +| File | Before | After | Tests | +| --- | --- | --- | --- | +| `src/agents/mcp_tools/deployment_tool.py` | 4 broad handlers (entry point + git / GitHub repo / Vercel steps); owner-lookup `RuntimeError` swallowed by the GitHub step; entry point printed to stderr | Steps catch only `OSError` and `subprocess.SubprocessError` (incl. `TimeoutExpired`); owner lookup raises `GitHubIdentityError(RuntimeError)`, the only exception caught around it; every failed step logs stage, error class and a token-redacted traceback; one marked boundary at the entry point logs via `logger` and returns `status: "error"` with `error_class` | `tests/unit/test_exception_boundaries_2168.py` | +| `src/youtube_extension/services/agents/adapters/agent_orchestrator.py` | 5 broad handlers | Shared SQL store setup catches only `SQLAlchemyError`, `ImportError` (missing driver) and `OSError` (file); anything else raises. Four marked boundaries (create agent, `execute_task`, `execute_single`, message delivery) log agent/task, error class and traceback; return values unchanged | `tests/unit/test_exception_boundaries_2168.py` | + +The test file's AST guard pins the broad-handler count (1 and 4) and requires every one +to carry the marker with a reason. It recognizes tuple catches, multiline catches, +bare handlers and `except*` handlers; line-oriented patterns miss these forms. + +## Remaining work + +The counts below are historical line-pattern measurements from 2026-11-09 and can +undercount tuple, multiline and `except*` handlers. Recompute broad-handler totals +with AST nodes when assessing remaining work: + +```python +import ast +from pathlib import Path + +for path in Path("src").rglob("*.py"): + source = path.read_text() + for node in ast.walk(ast.parse(source)): + if not isinstance(node, ast.ExceptHandler): + continue + caught = [] if node.type is None else ast.walk(node.type) + broad = node.type is None or any( + isinstance(t, ast.Name) and t.id in {"Exception", "BaseException"} + for t in caught + ) + if broad and "# exception-boundary:" not in source.splitlines()[node.lineno - 1]: + print(f"{path}:{node.lineno}") +``` + +- `src/agents/` (all modules): 125 unmarked broad handlers. +- `src/youtube_extension/services/agents/adapters/` (other adapters): 12 unmarked broad handlers. +- `src/youtube_extension/services/ai/gemini_service.py`: 22 broad handlers. +- `src/utils/notebooklm_ingest.py`: 5 bare `except:` (10 broad handlers in total). +- `apps/backend/main.py` caption fetch: one `except Exception` around `_fetch_captions`; it already fails closed (returns `success: False`, no fabricated text) but should be narrowed to the provider/transport errors and timeout, or marked as a boundary with logging. +- Ruff `BLE001` (blind except) is not enabled yet; turn it on per module as each one is remediated, starting with the two files above. diff --git a/src/agents/mcp_tools/deployment_tool.py b/src/agents/mcp_tools/deployment_tool.py index 523ef3573..c0b2de771 100644 --- a/src/agents/mcp_tools/deployment_tool.py +++ b/src/agents/mcp_tools/deployment_tool.py @@ -7,11 +7,22 @@ import logging import os import subprocess +import traceback from pathlib import Path from typing import Any, Optional logger = logging.getLogger(__name__) +# Failures a deployment step is expected to hit: the CLI is missing or the +# filesystem refuses (OSError), or the subprocess times out / errors +# (subprocess.SubprocessError, which includes TimeoutExpired). Anything else +# is a bug and propagates to the single boundary in deploy_to_github_and_vercel. +STEP_ERRORS: tuple[type[BaseException], ...] = (OSError, subprocess.SubprocessError) + + +class GitHubIdentityError(RuntimeError): + """The GitHub owner for a created repo could not be determined.""" + class DeploymentMCPTool: """ @@ -34,6 +45,32 @@ async def close(self): """Clean up resources""" pass + def _redact(self, text: str) -> str: + """Remove configured tokens from text bound for logs or results.""" + for secret in (self.github_token, self.vercel_token): + if secret: + # Subprocess exceptions render argv with repr(), escaping e.g. + # backslashes and newlines before the traceback reaches here. + for representation in sorted({secret, repr(secret)[1:-1]}, key=len, reverse=True): + text = text.replace(representation, "***") + return text + + def _log_step_failure(self, step: str, exc: BaseException) -> None: + """Log a failed step with its name, error class and (redacted) traceback. + + The traceback is formatted and redacted here rather than passed as + exc_info, because subprocess exceptions embed the full command line, + which carries the Vercel token. + """ + tb = self._redact("".join(traceback.format_exception(type(exc), exc, exc.__traceback__))) + logger.error( + "Deployment step failed: stage=%s error_class=%s\n%s", + step, + type(exc).__name__, + tb, + extra={"stage": step, "error_class": type(exc).__name__, "traceback": tb}, + ) + async def deploy_to_github_and_vercel( self, project_path: str, @@ -55,6 +92,7 @@ async def deploy_to_github_and_vercel( Returns: Dict with deployment URLs and status """ + results: dict[str, Any] = {} try: logger.info(f"🚀 Deploying: {project_path}") @@ -97,7 +135,7 @@ async def deploy_to_github_and_vercel( # Step 1: Initialize git if needed git_init_result = await self._initialize_git(project_dir) if not git_init_result["success"]: - results["errors"].append({"stage": "git_init", "message": git_init_result["error"]}) + results["errors"].append(self._stage_error("git_init", git_init_result)) results["status"] = "failed" return results @@ -114,10 +152,7 @@ async def deploy_to_github_and_vercel( results["github_url"] = github_result["repo_url"] logger.info(f"✅ GitHub: {results['github_url']}") else: - results["errors"].append({ - "stage": "github", - "message": github_result["error"] - }) + results["errors"].append(self._stage_error("github", github_result)) else: results["errors"].append({ "stage": "github", @@ -137,10 +172,7 @@ async def deploy_to_github_and_vercel( results["vercel_url"] = vercel_result["deployment_url"] logger.info(f"✅ Vercel: {results['vercel_url']}") else: - results["errors"].append({ - "stage": "vercel", - "message": vercel_result["error"] - }) + results["errors"].append(self._stage_error("vercel", vercel_result)) else: if not self.vercel_token: results["errors"].append({ @@ -149,23 +181,32 @@ async def deploy_to_github_and_vercel( }) # Determine overall status - if results["github_deployed"] or results["vercel_deployed"]: + # A completed GitHub artifact is a partial receipt, not proof that + # the full handoff succeeded when another requested stage failed. + if not results["errors"] and (results["github_deployed"] or results["vercel_deployed"]): results["status"] = "success" else: results["status"] = "failed" return results - except Exception as e: - logger.error(f"Deployment error: {e}") - import traceback - traceback.print_exc() + except Exception as exc: # exception-boundary: MCP tool entry point; the agent network needs a result dict, never a raise + self._log_step_failure("deploy_entrypoint", exc) return { + **results, "status": "error", - "error": str(e), + "error": self._redact(str(exc)) or type(exc).__name__, + "error_class": type(exc).__name__, "project_path": project_path } + @staticmethod + def _stage_error(stage: str, step_result: dict[str, Any]) -> dict[str, Any]: + entry = {"stage": stage, "message": step_result["error"]} + if step_result.get("error_class"): + entry["error_class"] = step_result["error_class"] + return entry + async def _initialize_git(self, project_dir: Path) -> dict[str, Any]: """Initialize git repository if not already initialized""" try: @@ -187,7 +228,7 @@ async def _initialize_git(self, project_dir: Path) -> dict[str, Any]: ) if result.returncode != 0: - return {"success": False, "error": result.stderr} + return {"success": False, "error": self._redact(result.stderr)} # Create .gitignore if doesn't exist gitignore = project_dir / ".gitignore" @@ -230,10 +271,12 @@ async def _initialize_git(self, project_dir: Path) -> dict[str, Any]: logger.info("✅ Git initialized") return {"success": True} - except subprocess.TimeoutExpired: - return {"success": False, "error": "Git initialization timed out"} - except Exception as e: - return {"success": False, "error": str(e)} + except subprocess.TimeoutExpired as exc: + self._log_step_failure("git_init", exc) + return {"success": False, "error": "Git initialization timed out", "error_class": type(exc).__name__} + except STEP_ERRORS as exc: + self._log_step_failure("git_init", exc) + return {"success": False, "error": f"Git initialization failed: {type(exc).__name__}", "error_class": type(exc).__name__} async def _create_github_repo( self, @@ -270,7 +313,7 @@ async def _create_github_repo( ) if result.returncode != 0: - error_msg = result.stderr or result.stdout + error_msg = self._redact(result.stderr or result.stdout) return {"success": False, "error": error_msg} # Extract repo URL from output @@ -286,19 +329,34 @@ async def _create_github_repo( break if not repo_url: - # Construct URL from repo name - username = self._get_github_username() - repo_url = f"https://github.com/{github_org or username}/{repo_name}" + # Construct URL from repo name. The org, when given, is the owner; + # otherwise the authenticated user is, and if that cannot be + # looked up the URL is unknown - never guessed. + if github_org: + owner = github_org + else: + try: + owner = self._get_github_username() + except GitHubIdentityError as exc: + self._log_step_failure("github_identity", exc) + return { + "success": False, + "error": "GitHub owner lookup failed; repo URL could not be verified", + "error_class": type(exc).__name__, + } + repo_url = f"https://github.com/{owner}/{repo_name}" return { "success": True, "repo_url": repo_url } - except subprocess.TimeoutExpired: - return {"success": False, "error": "GitHub repo creation timed out"} - except Exception as e: - return {"success": False, "error": str(e)} + except subprocess.TimeoutExpired as exc: + self._log_step_failure("github", exc) + return {"success": False, "error": "GitHub repo creation timed out", "error_class": type(exc).__name__} + except STEP_ERRORS as exc: + self._log_step_failure("github", exc) + return {"success": False, "error": f"GitHub repo creation failed: {type(exc).__name__}", "error_class": type(exc).__name__} async def _deploy_to_vercel( self, @@ -331,7 +389,7 @@ async def _deploy_to_vercel( ) if result.returncode != 0: - error_msg = result.stderr or result.stdout + error_msg = self._redact(result.stderr or result.stdout) return {"success": False, "error": error_msg} # Extract deployment URL from output @@ -357,10 +415,12 @@ async def _deploy_to_vercel( "deployment_url": deployment_url } - except subprocess.TimeoutExpired: - return {"success": False, "error": "Vercel deployment timed out (>5min)"} - except Exception as e: - return {"success": False, "error": str(e)} + except subprocess.TimeoutExpired as exc: + self._log_step_failure("vercel", exc) + return {"success": False, "error": "Vercel deployment timed out (>5min)", "error_class": type(exc).__name__} + except STEP_ERRORS as exc: + self._log_step_failure("vercel", exc) + return {"success": False, "error": f"Vercel deployment failed: {type(exc).__name__}", "error_class": type(exc).__name__} def _get_github_username(self) -> str: """Get GitHub username from gh CLI""" @@ -376,9 +436,9 @@ def _get_github_username(self) -> str: return result.stdout.strip() except (subprocess.TimeoutExpired, OSError) as exc: logger.error("GitHub identity lookup failed", extra={"stage": "github_identity", "error_class": type(exc).__name__}) - raise RuntimeError("GitHub identity lookup failed") from exc + raise GitHubIdentityError("GitHub identity lookup failed") from exc logger.error("GitHub identity lookup failed", extra={"stage": "github_identity", "returncode": result.returncode}) - raise RuntimeError("GitHub identity lookup failed") + raise GitHubIdentityError("GitHub identity lookup failed") # Singleton instance diff --git a/src/youtube_extension/services/agents/adapters/agent_orchestrator.py b/src/youtube_extension/services/agents/adapters/agent_orchestrator.py index 0c53c89b1..ecde3f790 100644 --- a/src/youtube_extension/services/agents/adapters/agent_orchestrator.py +++ b/src/youtube_extension/services/agents/adapters/agent_orchestrator.py @@ -16,6 +16,9 @@ from datetime import datetime from typing import Any, Optional +from sqlalchemy.engine import make_url +from sqlalchemy.exc import ArgumentError, SQLAlchemyError + from ...shared_sql_state import SharedSQLStateStore from ..base_agent import AgentRequest, AgentResult, BaseAgent from ..registry import get as get_agent_class @@ -72,11 +75,26 @@ def __init__(self, *, database_url: Optional[str] = None): ) if shared_database_url: try: + try: + make_url(shared_database_url) + except ValueError as exc: + # URL parsing uses plain ValueError for malformed ports. + # Store-construction ValueErrors still surface as bugs. + raise ArgumentError("Invalid shared-state database URL") from exc self._shared_state = SharedSQLStateStore( database_url=shared_database_url ) - except Exception as exc: - self.logger.warning("Shared A2A SQL state unavailable: %s", exc) + except (SQLAlchemyError, ImportError, OSError) as exc: + # Expected setup failures only: database/URL errors, a missing + # DB driver, or an unwritable SQLite path. Anything else is a + # bug and must surface instead of silently dropping shared state. + self.logger.warning( + "Shared A2A SQL state unavailable: %s (error_class=%s)", + exc, + type(exc).__name__, + extra={"error_class": type(exc).__name__}, + exc_info=True, + ) self._shared_state = None else: self._shared_state = None @@ -152,8 +170,15 @@ async def get_agent( agent = agent_class() self._agents[name] = agent return agent - except Exception as e: - self.logger.error(f"Failed to instantiate agent {name}: {e}") + except Exception as e: # exception-boundary: third-party agent constructors can raise anything; get_agent contract is None on failure + self.logger.error( + "Failed to instantiate agent %s: %s (error_class=%s)", + name, + e, + type(e).__name__, + extra={"agent": name, "error_class": type(e).__name__}, + exc_info=True, + ) return None # Fallback to registry @@ -270,8 +295,15 @@ async def execute_task( return orchestration_result - except Exception as e: - self.logger.error(f"Task execution failed: {e}", exc_info=True) + except Exception as e: # exception-boundary: orchestration entry point; callers expect an OrchestrationResult with success=False, never a raise + self.logger.error( + "Task execution failed: task_type=%s error_class=%s: %s", + task_type, + type(e).__name__, + e, + extra={"task_type": task_type, "error_class": type(e).__name__}, + exc_info=True, + ) return OrchestrationResult( success=False, errors=[f"Orchestration failed: {str(e)}"], @@ -416,8 +448,15 @@ async def execute_single( "; ".join(result.logs) or "Agent execution failed" ) return {"error": error_msg, "output": result.output} - except Exception as e: - self.logger.error("execute_single failed for %s: %s", agent_type, e) + except Exception as e: # exception-boundary: agent dispatch entry point; agent.run() is third-party code and callers expect an error dict + self.logger.error( + "execute_single failed for %s: %s (error_class=%s)", + agent_type, + e, + type(e).__name__, + extra={"agent": agent_type, "error_class": type(e).__name__}, + exc_info=True, + ) self._record_a2a_message( A2AContextMessage( sender="orchestrator", @@ -504,8 +543,15 @@ async def send_a2a_message( if agent and hasattr(agent, "receive_context"): try: await agent.receive_context(content) - except Exception as e: - self.logger.warning("Agent %s failed to receive context: %s", recipient, e) + except Exception as e: # exception-boundary: context delivery is best-effort; the message is already recorded and a recipient's bug must not fail the sender + self.logger.warning( + "Agent %s failed to receive context: %s (error_class=%s)", + recipient, + e, + type(e).__name__, + extra={"agent": recipient, "error_class": type(e).__name__}, + exc_info=True, + ) return msg diff --git a/tests/unit/test_exception_boundaries_2168.py b/tests/unit/test_exception_boundaries_2168.py new file mode 100644 index 000000000..12b1b9aa8 --- /dev/null +++ b/tests/unit/test_exception_boundaries_2168.py @@ -0,0 +1,347 @@ +"""Issue #2168: narrowed exception handling in deploy handoff and agent orchestrator. + +Every failure here is simulated (patched subprocess / store / agents): no +subprocess, network or database is touched. +""" + +from __future__ import annotations + +import ast +import asyncio +import importlib.util +import logging +import os +import subprocess +from pathlib import Path +from types import SimpleNamespace +from unittest.mock import AsyncMock, MagicMock, patch + +import pytest +from sqlalchemy.exc import ArgumentError, OperationalError + +with patch.dict(os.environ): + os.environ.pop("DATABASE_URL", None) + os.environ.pop("EVENTRELAY_SHARED_STATE_DATABASE_URL", None) + from youtube_extension.services.agents.adapters import ( + agent_orchestrator as orch_mod, + ) + from youtube_extension.services.agents.adapters.agent_orchestrator import ( + AgentOrchestrator, + OrchestrationResult, + ) + from youtube_extension.services.agents.base_agent import BaseAgent + +REPO_ROOT = Path(__file__).parents[2] +DEPLOY_PATH = REPO_ROOT / "src/agents/mcp_tools/deployment_tool.py" +ORCH_PATH = REPO_ROOT / "src/youtube_extension/services/agents/adapters/agent_orchestrator.py" + +_spec = importlib.util.spec_from_file_location("deployment_tool_2168", DEPLOY_PATH) +deploy_mod = importlib.util.module_from_spec(_spec) +_spec.loader.exec_module(deploy_mod) + +DEPLOY_LOGGER = deploy_mod.logger.name +SECRET = "vercel-secret-token-2168" + + +@pytest.fixture(autouse=True) +def isolate_shared_state_environment(monkeypatch): + monkeypatch.delenv("DATABASE_URL", raising=False) + monkeypatch.delenv("EVENTRELAY_SHARED_STATE_DATABASE_URL", raising=False) + + +def _tool(github_token=None, vercel_token=None): + instance = object.__new__(deploy_mod.DeploymentMCPTool) + instance.github_token = github_token + instance.vercel_token = vercel_token + return instance + + +def _step_records(caplog, stage): + return [r for r in caplog.records if getattr(r, "stage", None) == stage and getattr(r, "error_class", None)] + + +# --------------------------------------------------------------------------- +# deployment_tool.py +# --------------------------------------------------------------------------- + + +def test_git_init_missing_cli_is_logged_with_stage_class_and_traceback(tmp_path, caplog): + caplog.set_level(logging.ERROR, logger=DEPLOY_LOGGER) + with patch.object(deploy_mod.subprocess, "run", side_effect=FileNotFoundError("git")): + result = asyncio.run(_tool()._initialize_git(tmp_path)) + assert result["success"] is False + assert result["error_class"] == "FileNotFoundError" + (record,) = _step_records(caplog, "git_init") + assert record.error_class == "FileNotFoundError" + assert "Traceback (most recent call last)" in record.traceback + + +def test_git_init_timeout_is_logged(tmp_path, caplog): + caplog.set_level(logging.ERROR, logger=DEPLOY_LOGGER) + with patch.object(deploy_mod.subprocess, "run", side_effect=subprocess.TimeoutExpired("git", 30)): + result = asyncio.run(_tool()._initialize_git(tmp_path)) + assert result == {"success": False, "error": "Git initialization timed out", "error_class": "TimeoutExpired"} + assert _step_records(caplog, "git_init")[0].error_class == "TimeoutExpired" + + +def test_unexpected_error_in_git_step_raises(tmp_path): + with patch.object(deploy_mod.subprocess, "run", side_effect=ValueError("bug")), pytest.raises(ValueError): + asyncio.run(_tool()._initialize_git(tmp_path)) + + +def test_github_step_subprocess_error_is_logged(tmp_path, caplog): + caplog.set_level(logging.ERROR, logger=DEPLOY_LOGGER) + with patch.object(deploy_mod.subprocess, "run", side_effect=subprocess.SubprocessError("boom")): + result = asyncio.run(_tool(github_token="gh-token")._create_github_repo(tmp_path, "demo", None)) + assert result["success"] is False + assert result["error_class"] == "SubprocessError" + assert _step_records(caplog, "github")[0].error_class == "SubprocessError" + + +@pytest.mark.parametrize("secret", [SECRET, "abc\\def", "abc\ndef", "abc'\"def"]) +def test_vercel_timeout_is_logged_without_leaking_token(tmp_path, caplog, secret): + caplog.set_level(logging.ERROR, logger=DEPLOY_LOGGER) + timeout = subprocess.TimeoutExpired(["vercel", "--prod", "--yes", "--token", secret], 300) + with patch.object(deploy_mod.subprocess, "run", side_effect=timeout): + result = asyncio.run(_tool(vercel_token=secret)._deploy_to_vercel(tmp_path, "demo", None)) + assert result["success"] is False + assert result["error_class"] == "TimeoutExpired" + (record,) = _step_records(caplog, "vercel") + assert "Traceback" in record.traceback + assert secret not in caplog.text + assert repr(secret)[1:-1] not in caplog.text + assert secret not in str(result) + + +def test_entrypoint_preserves_completed_github_handoff_on_unexpected_vercel_failure(tmp_path): + tool = _tool(github_token="gh-token", vercel_token=SECRET) + with ( + patch.object(tool, "_initialize_git", new=AsyncMock(return_value={"success": True})), + patch.object(tool, "_create_github_repo", new=AsyncMock(return_value={"success": True, "repo_url": "https://github.com/example/completed"})), + patch.object(tool, "_deploy_to_vercel", new=AsyncMock(side_effect=ValueError("unexpected"))), + ): + result = asyncio.run(tool.deploy_to_github_and_vercel(str(tmp_path))) + assert result["status"] == "error" + assert result["github_deployed"] is True + assert result["github_url"] == "https://github.com/example/completed" + assert result["vercel_deployed"] is False + assert result["error_class"] == "ValueError" + + +def test_expected_vercel_failure_keeps_github_receipt_but_fails_handoff(tmp_path): + tool = _tool(github_token="gh-token", vercel_token=SECRET) + with ( + patch.object(tool, "_initialize_git", new=AsyncMock(return_value={"success": True})), + patch.object(tool, "_create_github_repo", new=AsyncMock(return_value={"success": True, "repo_url": "https://github.com/example/completed"})), + patch.object(tool, "_deploy_to_vercel", new=AsyncMock(return_value={"success": False, "error": "deployment rejected"})), + ): + result = asyncio.run(tool.deploy_to_github_and_vercel(str(tmp_path))) + assert result["status"] == "failed" + assert result["github_deployed"] is True + assert result["github_url"] == "https://github.com/example/completed" + assert result["vercel_deployed"] is False + assert result["errors"] == [{"stage": "vercel", "message": "deployment rejected"}] + + +@pytest.mark.parametrize("step", ["git", "github", "vercel"]) +@pytest.mark.parametrize("stream", ["stderr", "stdout"]) +def test_expected_cli_failure_redacts_configured_tokens(tmp_path, step, stream): + tool = _tool(github_token="github-secret", vercel_token=SECRET) + error = f"Rejected {tool.github_token} and {tool.vercel_token}" + failed = SimpleNamespace(returncode=1, stderr=error if stream == "stderr" else "", stdout=error if stream == "stdout" else "") + calls = { + "git": lambda: tool._initialize_git(tmp_path), + "github": lambda: tool._create_github_repo(tmp_path, "demo", None), + "vercel": lambda: tool._deploy_to_vercel(tmp_path, "demo", None), + } + with patch.object(deploy_mod.subprocess, "run", return_value=failed): + result = asyncio.run(calls[step]()) + assert result["success"] is False + assert SECRET not in str(result) and tool.github_token not in str(result) + + +def test_owner_lookup_failure_is_caught_and_reported(tmp_path, caplog): + caplog.set_level(logging.ERROR, logger=DEPLOY_LOGGER) + created = SimpleNamespace(returncode=0, stdout="created", stderr="") + tool = _tool(github_token="gh-token") + with patch.object(deploy_mod.subprocess, "run", return_value=created), patch.object( + tool, "_get_github_username", side_effect=deploy_mod.GitHubIdentityError("GitHub identity lookup failed") + ): + result = asyncio.run(tool._create_github_repo(tmp_path, "demo", None)) + assert result["success"] is False + assert result["error_class"] == "GitHubIdentityError" + assert "repo_url" not in result + assert _step_records(caplog, "github_identity") + + +def test_owner_lookup_unexpected_error_raises(tmp_path): + created = SimpleNamespace(returncode=0, stdout="created", stderr="") + tool = _tool(github_token="gh-token") + with patch.object(deploy_mod.subprocess, "run", return_value=created), patch.object( + tool, "_get_github_username", side_effect=KeyError("bug") + ), pytest.raises(KeyError): + asyncio.run(tool._create_github_repo(tmp_path, "demo", None)) + + +def test_identity_error_is_runtime_error_subclass(): + assert issubclass(deploy_mod.GitHubIdentityError, RuntimeError) + with patch.object(deploy_mod.subprocess, "run", side_effect=OSError("gh")), pytest.raises( + deploy_mod.GitHubIdentityError + ): + _tool(github_token="gh-token")._get_github_username() + + +def test_entrypoint_boundary_returns_error_class_and_logs_not_stderr(tmp_path, caplog, capsys): + caplog.set_level(logging.ERROR, logger=DEPLOY_LOGGER) + with patch.object(deploy_mod.subprocess, "run", side_effect=ValueError("unexpected")): + result = asyncio.run(_tool().deploy_to_github_and_vercel(str(tmp_path))) + assert result["status"] == "error" + assert result["error_class"] == "ValueError" + (record,) = _step_records(caplog, "deploy_entrypoint") + assert "Traceback" in record.traceback + assert capsys.readouterr().err == "" + + +def test_failed_step_error_class_reaches_deploy_result(tmp_path): + (tmp_path / ".git").mkdir() # git init already done + with patch.object(deploy_mod.subprocess, "run", side_effect=FileNotFoundError("gh")): + result = asyncio.run(_tool(github_token="gh-token").deploy_to_github_and_vercel(str(tmp_path))) + assert result["status"] == "failed" + github_errors = [e for e in result["errors"] if e["stage"] == "github"] + assert github_errors[0]["error_class"] == "FileNotFoundError" + + +# --------------------------------------------------------------------------- +# agent_orchestrator.py +# --------------------------------------------------------------------------- + + +@pytest.mark.parametrize( + "failure", + [ + OperationalError("connect", {}, Exception("refused")), + ArgumentError("bad url"), + ModuleNotFoundError("No module named 'psycopg'"), + PermissionError("read-only path"), + ], + ids=["db", "bad-url", "missing-driver", "file"], +) +def test_shared_store_expected_failures_degrade_with_log(failure, caplog): + caplog.set_level(logging.WARNING, logger="agent_orchestrator") + with patch.object(orch_mod, "SharedSQLStateStore", side_effect=failure): + orch = AgentOrchestrator(database_url="sqlite:///unused.db") + assert orch._shared_state is None + (record,) = [r for r in caplog.records if getattr(r, "error_class", None)] + assert record.error_class == type(failure).__name__ + assert record.exc_info is not None + + +def test_shared_store_unexpected_failure_raises(): + with patch.object(orch_mod, "SharedSQLStateStore", side_effect=ValueError("bug")), pytest.raises(ValueError): + AgentOrchestrator(database_url="sqlite:///unused.db") + + +def test_shared_store_malformed_port_degrades_before_store_construction(caplog): + with patch.object(orch_mod, "SharedSQLStateStore") as store: + orch = AgentOrchestrator(database_url="postgresql://u:p@db:PORT/app") + assert orch._shared_state is None + store.assert_not_called() + assert "ArgumentError" in caplog.text + + +async def test_create_agent_boundary_logs_agent_class_traceback(caplog): + caplog.set_level(logging.ERROR, logger="agent_orchestrator") + + class Exploding: + def __init__(self, config=None): + raise ValueError("ctor failed") + + orch = AgentOrchestrator() + orch.register_agent_type("exploding", Exploding) + assert await orch.get_agent("exploding") is None + (record,) = [r for r in caplog.records if getattr(r, "agent", None) == "exploding"] + assert record.error_class == "ValueError" + assert record.exc_info is not None + + +async def test_execute_task_boundary_returns_failure_and_logs(caplog): + caplog.set_level(logging.ERROR, logger="agent_orchestrator") + agent = MagicMock(spec=BaseAgent) + agent.name = "video_master" + agent.run = MagicMock(side_effect=ValueError("sync crash")) + orch = AgentOrchestrator() + orch._agents["video_master"] = agent + result = await orch.execute_task("content_generation", {}) + assert isinstance(result, OrchestrationResult) + assert result.success is False + assert result.errors == ["Orchestration failed: sync crash"] + (record,) = [r for r in caplog.records if getattr(r, "task_type", None) == "content_generation"] + assert record.error_class == "ValueError" + assert record.exc_info is not None + + +async def test_execute_single_boundary_returns_error_dict_and_logs(caplog): + caplog.set_level(logging.ERROR, logger="agent_orchestrator") + agent = MagicMock(spec=BaseAgent) + agent.name = "worker" + agent.run = AsyncMock(side_effect=RuntimeError("run crashed")) + orch = AgentOrchestrator() + orch._agents["worker"] = agent + assert await orch.execute_single("worker", {"event": 1}) == {"error": "run crashed"} + (record,) = [r for r in caplog.records if getattr(r, "agent", None) == "worker"] + assert record.error_class == "RuntimeError" + assert record.exc_info is not None + + +async def test_message_delivery_boundary_keeps_message_and_logs(caplog): + caplog.set_level(logging.WARNING, logger="agent_orchestrator") + recipient = MagicMock() + recipient.receive_context = AsyncMock(side_effect=KeyError("missing")) + orch = AgentOrchestrator() + orch._agents["receiver"] = recipient + msg = await orch.send_a2a_message("sender", "receiver", {"k": "v"}) + assert msg.recipient == "receiver" + assert orch.get_a2a_log(conversation_id=msg.conversation_id) + (record,) = [r for r in caplog.records if getattr(r, "agent", None) == "receiver"] + assert record.error_class == "KeyError" + assert record.exc_info is not None + + +# --------------------------------------------------------------------------- +# Guard: no new unmarked broad handlers in the two narrowed files +# --------------------------------------------------------------------------- + +_MARKER = "# exception-boundary:" + + +def _broad_handlers(source): + for node in ast.walk(ast.parse(source)): + if not isinstance(node, ast.ExceptHandler): + continue + caught = [] if node.type is None else list(ast.walk(node.type)) + if node.type is None or any(isinstance(t, ast.Name) and t.id in {"Exception", "BaseException"} for t in caught): + yield node + + +@pytest.mark.parametrize("source", [ + "try:\n pass\nexcept (ValueError, Exception):\n pass", + "try:\n pass\nexcept (\n ValueError,\n BaseException\n):\n pass", + "try:\n pass\nexcept* Exception:\n pass", + "try:\n pass\nexcept:\n pass", +]) +def test_broad_handler_guard_recognizes_all_syntaxes(source): + assert len(list(_broad_handlers(source))) == 1 + + +@pytest.mark.parametrize("path,expected", [(DEPLOY_PATH, 1), (ORCH_PATH, 4)], ids=["deploy", "orchestrator"]) +def test_broad_handlers_are_counted_and_marked(path, expected): + source = path.read_text() + lines = source.splitlines() + broad = [lines[node.lineno - 1] for node in _broad_handlers(source)] + assert len(broad) == expected, broad + unmarked = [line for line in broad if _MARKER not in line] + assert unmarked == [] + markers = [line for line in path.read_text().splitlines() if _MARKER in line] + assert len(markers) == expected + for line in markers: + assert len(line.split(_MARKER, 1)[1].strip()) > 10, f"boundary needs a reason: {line}" diff --git a/vercel.json b/vercel.json index d77369556..32b495d9e 100644 --- a/vercel.json +++ b/vercel.json @@ -7,7 +7,10 @@ { "source": "/(.*)", "has": [ - { "type": "host", "value": "event-relay-web.vercel.app" } + { + "type": "host", + "value": "event-relay-web.vercel.app" + } ], "destination": "https://uvai.io/$1", "permanent": true @@ -15,10 +18,18 @@ { "source": "/(.*)", "has": [ - { "type": "host", "value": "v0-uvai.vercel.app" } + { + "type": "host", + "value": "v0-uvai.vercel.app" + } ], "destination": "https://uvai.io/$1", "permanent": true } - ] + ], + "git": { + "deploymentEnabled": { + "fix/2168-narrow-except-deploy-agents": false + } + } }