-
Notifications
You must be signed in to change notification settings - Fork 12
feat: support sourceDescriptions in operationId/operationPath #73
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
c96335f
a4f46ac
fe22044
7691a5b
66ba2e9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -12,6 +12,8 @@ | |||||||||||||||||
| import jsonpointer | ||||||||||||||||||
|
|
||||||||||||||||||
| from arazzo_runner.auth.models import SecurityOption, SecurityRequirement | ||||||||||||||||||
| from arazzo_runner.evaluator import ExpressionEvaluator | ||||||||||||||||||
| from arazzo_runner.models import ExecutionState | ||||||||||||||||||
|
|
||||||||||||||||||
| # Configure logging | ||||||||||||||||||
| logger = logging.getLogger("arazzo-runner.executor") | ||||||||||||||||||
|
|
@@ -39,6 +41,47 @@ def find_by_id(self, operation_id: str) -> dict | None: | |||||||||||||||||
| Returns: | ||||||||||||||||||
| Dictionary with operation details or None if not found | ||||||||||||||||||
| """ | ||||||||||||||||||
| # Special handling: support references in Arazzo specs like | ||||||||||||||||||
| # $sourceDescriptions.<source_name>.<operationId> | ||||||||||||||||||
| if isinstance(operation_id, str) and operation_id.startswith("$sourceDescriptions."): | ||||||||||||||||||
| # parse into three parts: prefix, source_name, operation_name | ||||||||||||||||||
| parts = operation_id.split(".", 2) | ||||||||||||||||||
| if len(parts) == 3: | ||||||||||||||||||
| _, source_name, target_op = parts | ||||||||||||||||||
| source_desc = self.source_descriptions.get(source_name) | ||||||||||||||||||
| if source_desc: | ||||||||||||||||||
| paths = source_desc.get("paths", {}) | ||||||||||||||||||
| for path, path_item in paths.items(): | ||||||||||||||||||
| for method, operation in path_item.items(): | ||||||||||||||||||
| if ( | ||||||||||||||||||
| method in ["get", "post", "put", "delete", "patch"] | ||||||||||||||||||
| and operation.get("operationId") == target_op | ||||||||||||||||||
| ): | ||||||||||||||||||
| try: | ||||||||||||||||||
| servers = source_desc.get("servers") | ||||||||||||||||||
| if not servers or not isinstance(servers, list): | ||||||||||||||||||
| raise ValueError( | ||||||||||||||||||
| "Missing or invalid 'servers' list in OpenAPI spec." | ||||||||||||||||||
| ) | ||||||||||||||||||
| base_url = servers[0].get("url") | ||||||||||||||||||
| if not base_url or not isinstance(base_url, str): | ||||||||||||||||||
| raise ValueError( | ||||||||||||||||||
| "Missing or invalid 'url' in the first server object." | ||||||||||||||||||
| ) | ||||||||||||||||||
| except (IndexError, ValueError) as e: | ||||||||||||||||||
| raise ValueError( | ||||||||||||||||||
| f"Could not determine base URL from OpenAPI spec servers: {e}" | ||||||||||||||||||
| ) from e | ||||||||||||||||||
|
|
||||||||||||||||||
| return { | ||||||||||||||||||
| "source": source_name, | ||||||||||||||||||
| "path": path, | ||||||||||||||||||
| "method": method, | ||||||||||||||||||
| "url": base_url + path, | ||||||||||||||||||
| "operation": operation, | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| # Default: search all source descriptions for an operation with matching operationId | ||||||||||||||||||
| for source_name, source_desc in self.source_descriptions.items(): | ||||||||||||||||||
| # Search through paths and operations | ||||||||||||||||||
| paths = source_desc.get("paths", {}) | ||||||||||||||||||
|
|
@@ -311,6 +354,9 @@ def _extract_path_method_with_regex( | |||||||||||||||||
|
|
||||||||||||||||||
| # Decode the path (replace ~1 with / and ~0 with ~) | ||||||||||||||||||
| decoded_path = encoded_path.replace("~1", "/").replace("~0", "~") | ||||||||||||||||||
| # Normalize leading slashes: decoded_path may start with multiple slashes | ||||||||||||||||||
| # because encoded_path often begins with a leading '/'. Ensure a single leading slash. | ||||||||||||||||||
| decoded_path = "/" + decoded_path.lstrip("/") | ||||||||||||||||||
| logger.debug(f"Decoded path: {decoded_path}") | ||||||||||||||||||
|
|
||||||||||||||||||
| # Try to find the operation in the source description | ||||||||||||||||||
|
|
@@ -749,11 +795,58 @@ def get_operations_for_workflow(self, workflow: dict) -> list[dict]: | |||||||||||||||||
| if op_info: | ||||||||||||||||||
| operations.append(op_info) | ||||||||||||||||||
| elif "operationPath" in step: | ||||||||||||||||||
| # operationPath format: <source>#<json_pointer> | ||||||||||||||||||
| match = re.match(r"([^#]+)#(.+)", step["operationPath"]) | ||||||||||||||||||
| if match: | ||||||||||||||||||
| source_url, json_pointer = match.groups() | ||||||||||||||||||
| op_info = self.find_by_path(source_url, json_pointer) | ||||||||||||||||||
| if op_info: | ||||||||||||||||||
| operations.append(op_info) | ||||||||||||||||||
| # operationPath may be <source>#<json_pointer> or a runtime expression | ||||||||||||||||||
| # referencing a sourceDescription. We evaluate any braced expressions | ||||||||||||||||||
| # using the shared ExpressionEvaluator and then map the resolved | ||||||||||||||||||
| # left-hand value to a source name (by name or by matching the | ||||||||||||||||||
| # source's declared `url`) before delegating to find_by_path. | ||||||||||||||||||
| op_path = step["operationPath"] | ||||||||||||||||||
| if not isinstance(op_path, str): | ||||||||||||||||||
| continue | ||||||||||||||||||
|
|
||||||||||||||||||
| left, json_pointer = (op_path.split("#", 1) + [""])[0:2] | ||||||||||||||||||
| resolved_left = left.strip() | ||||||||||||||||||
|
|
||||||||||||||||||
| # Evaluate embedded braced runtime expressions using central evaluator | ||||||||||||||||||
| if "{" in resolved_left and "}" in resolved_left: | ||||||||||||||||||
| eval_state = ExecutionState(workflow_id="__internal__") | ||||||||||||||||||
|
|
||||||||||||||||||
| def _eval_braced(m: re.Match) -> str: | ||||||||||||||||||
| expr = m.group(1) | ||||||||||||||||||
|
Comment on lines
+812
to
+815
|
||||||||||||||||||
| eval_state = ExecutionState(workflow_id="__internal__") | |
| def _eval_braced(m: re.Match) -> str: | |
| expr = m.group(1) | |
| workflow_id = workflow.get("id", "__internal__") | |
| eval_state = ExecutionState(workflow_id=workflow_id) | |
| def _eval_braced(m: re.Match) -> str: |
Copilot
AI
Aug 28, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The URL matching logic is overly permissive and could lead to false positives. Using substring matching (in operator) on both directions could match unintended sources. For example, if one source has URL "api.com" and another has "myapi.com", both would match "api.com". Consider using more precise matching logic such as exact matches or proper URL parsing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This path normalization logic is duplicated from existing code and could cause unexpected behavior. The comment mentions "multiple slashes" but the logic strips all leading slashes then adds one back, which could incorrectly modify paths that legitimately start with multiple slashes. Consider extracting this to a dedicated path normalization utility function to ensure consistent behavior across the codebase.