Skip to content
Closed
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
25 changes: 24 additions & 1 deletion api/oss/src/apis/fastapi/triggers/models.py
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
from typing import Any, Dict, List, Optional

from pydantic import BaseModel, Field
from pydantic import BaseModel, Field, field_validator

from oss.src.core.shared.dtos import Windowing
from oss.src.core.triggers.dtos import (
TriggerCatalogEvent,
TriggerCatalogEventDetails,
TriggerCatalogIntegration,
TriggerCapabilitiesResult,
TriggerCatalogProvider,
TriggerProviderKind,
TriggerConnection,
TriggerConnectionCreate,
TriggerDelivery,
Expand Down Expand Up @@ -63,6 +65,27 @@ class TriggerCatalogEventsResponse(BaseModel):
events: List[TriggerCatalogEvent] = Field(default_factory=list)


class TriggerDiscoveryQuery(BaseModel):
"""Request body for ``POST /triggers/discover``."""

use_cases: List[str]
provider: str = TriggerProviderKind.COMPOSIO.value
limit_alternatives: int = Field(default=3, ge=0)

@field_validator("use_cases", mode="before")
@classmethod
def _require_use_cases(cls, value: Any) -> List[str]:
if not isinstance(value, list):
raise ValueError("use_cases must be a list of non-empty fragments")
items = [str(v).strip() for v in value if str(v).strip()]
if not items:
raise ValueError("use_cases must contain at least one non-empty fragment")
return items
Comment on lines +71 to +83

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Keep the discovery request schema strict.

provider accepts any string, and _require_use_cases() stringifies arbitrary list items (123, {}) instead of rejecting them. That turns malformed payloads into meaningless searches instead of a 422.

Suggested fix
 class TriggerDiscoveryQuery(BaseModel):
     """Request body for ``POST /triggers/discover``."""
 
     use_cases: List[str]
-    provider: str = TriggerProviderKind.COMPOSIO.value
+    provider: TriggerProviderKind = TriggerProviderKind.COMPOSIO
     limit_alternatives: int = Field(default=3, ge=0)
 
     `@field_validator`("use_cases", mode="before")
     `@classmethod`
     def _require_use_cases(cls, value: Any) -> List[str]:
         if not isinstance(value, list):
             raise ValueError("use_cases must be a list of non-empty fragments")
-        items = [str(v).strip() for v in value if str(v).strip()]
+        if any(not isinstance(v, str) for v in value):
+            raise ValueError("use_cases must be a list of strings")
+        items = [v.strip() for v in value if v.strip()]
         if not items:
             raise ValueError("use_cases must contain at least one non-empty fragment")
         return items
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
use_cases: List[str]
provider: str = TriggerProviderKind.COMPOSIO.value
limit_alternatives: int = Field(default=3, ge=0)
@field_validator("use_cases", mode="before")
@classmethod
def _require_use_cases(cls, value: Any) -> List[str]:
if not isinstance(value, list):
raise ValueError("use_cases must be a list of non-empty fragments")
items = [str(v).strip() for v in value if str(v).strip()]
if not items:
raise ValueError("use_cases must contain at least one non-empty fragment")
return items
use_cases: List[str]
provider: TriggerProviderKind = TriggerProviderKind.COMPOSIO
limit_alternatives: int = Field(default=3, ge=0)
`@field_validator`("use_cases", mode="before")
`@classmethod`
def _require_use_cases(cls, value: Any) -> List[str]:
if not isinstance(value, list):
raise ValueError("use_cases must be a list of non-empty fragments")
if any(not isinstance(v, str) for v in value):
raise ValueError("use_cases must be a list of strings")
items = [v.strip() for v in value if v.strip()]
if not items:
raise ValueError("use_cases must contain at least one non-empty fragment")
return items



TriggerDiscoveryResponse = TriggerCapabilitiesResult

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Wrap /discover in a normal API response envelope.

Aliasing the response to the core DTO makes this endpoint the odd one out in the router: the rest return {count, ...} envelopes, but this one exposes the core result directly. Please define an explicit API response model here and wrap the service DTO at the route boundary. As per coding guidelines, "Define explicit request and response models in models.py, include count plus payload in response envelopes."

Source: Coding guidelines



# ---------------------------------------------------------------------------
# Trigger Connections
#
Expand Down
33 changes: 33 additions & 0 deletions api/oss/src/apis/fastapi/triggers/router.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@
TriggerDeliveriesResponse,
TriggerDeliveryQueryRequest,
TriggerDeliveryResponse,
TriggerDiscoveryQuery,
TriggerDiscoveryResponse,
TriggerEventAck,
TriggerScheduleCreateRequest,
TriggerScheduleEditRequest,
Expand Down Expand Up @@ -173,6 +175,14 @@ def __init__(
response_model=TriggerCatalogEventResponse,
response_model_exclude_none=True,
)
self.router.add_api_route(
"/discover",
self.discover_triggers,
methods=["POST"],
operation_id="discover_triggers",
response_model=TriggerDiscoveryResponse,
response_model_exclude_none=True,
)

# --- Trigger Connections ---
# Shared `gateway_connections` rows; independent surface from tools.
Expand Down Expand Up @@ -945,6 +955,29 @@ async def get_event(

return response

@intercept_exceptions()
@handle_adapter_exceptions()
async def discover_triggers(
self,
request: Request,
*,
body: TriggerDiscoveryQuery,
) -> TriggerDiscoveryResponse:
has_permission = await check_action_access(
user_uid=request.state.user_id,
project_id=request.state.project_id,
permission=Permission.VIEW_TRIGGERS,
)
if not has_permission:
raise FORBIDDEN_EXCEPTION

return await self.triggers_service.discover_triggers(
project_id=UUID(request.state.project_id),
use_cases=body.use_cases,
provider_key=body.provider,
limit_alternatives=body.limit_alternatives,
)

# -----------------------------------------------------------------------
# Trigger Subscriptions
# -----------------------------------------------------------------------
Expand Down
72 changes: 71 additions & 1 deletion api/oss/src/core/triggers/dtos.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
from datetime import datetime
from enum import Enum
from typing import Any, Dict, List, Optional, Union
from typing import Any, Dict, List, Literal, Optional, Union
from uuid import UUID

from pydantic import BaseModel, Field
Expand Down Expand Up @@ -101,6 +101,76 @@ class TriggerCatalogEventsPage(BaseModel):
total: int = 0


# ---------------------------------------------------------------------------
# Trigger discovery (find_triggers)
# ---------------------------------------------------------------------------


class TriggerDiscoveryConnectionState(str, Enum):
READY = "ready"
NEEDS_AUTH = "needs_auth"
NEEDS_INPUT = "needs_input"


class DiscoveredTriggerEvent(BaseModel):
type: Literal["trigger"] = "trigger"
provider: str = TriggerProviderKind.COMPOSIO.value
integration: str
event_key: str
trigger_config: Optional[Dict[str, Any]] = None
payload: Optional[Dict[str, Any]] = None
description: Optional[str] = None
provider_event: str


class DiscoveredTriggerAlternative(BaseModel):
integration: str
event_key: str
description: Optional[str] = None
provider_event: str


class TriggerCapabilityConnection(BaseModel):
state: TriggerDiscoveryConnectionState
id: Optional[UUID] = None
slug: Optional[str] = None


class TriggerConnectAffordance(BaseModel):
endpoint: str = "POST /triggers/connections/"
body: Dict[str, Any]


class TriggerConnectionRequirement(BaseModel):
integration: str
state: TriggerDiscoveryConnectionState
id: Optional[UUID] = None
slug: Optional[str] = None
connect: Optional[TriggerConnectAffordance] = None


class TriggerCapability(BaseModel):
use_case: str
integration: Optional[str] = None
event: Optional[DiscoveredTriggerEvent] = None
alternatives: List[DiscoveredTriggerAlternative] = Field(default_factory=list)
connection: Optional[TriggerCapabilityConnection] = None
note: Optional[str] = None


class TriggerDiscoveryGuidance(BaseModel):
plan_steps: List[str] = Field(default_factory=list)
pitfalls: List[str] = Field(default_factory=list)


class TriggerCapabilitiesResult(BaseModel):
capabilities: List[TriggerCapability] = Field(default_factory=list)
connections: List[TriggerConnectionRequirement] = Field(default_factory=list)
guidance: TriggerDiscoveryGuidance = Field(default_factory=TriggerDiscoveryGuidance)
ready: bool = False
notes: List[str] = Field(default_factory=list)


# ---------------------------------------------------------------------------
# Trigger Connections — shared `gateway_connections` rows, inherited here so the
# triggers router/models never reference the generic gateway DTOs directly.
Expand Down
Loading
Loading