-
Notifications
You must be signed in to change notification settings - Fork 35
feat(skills): add skills support to advanced agents with sandboxed cli tool #989
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
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 |
|---|---|---|
|
|
@@ -87,12 +87,16 @@ def create_advanced_agent( | |
| response_format: ResponseFormat[Any] | None = None, | ||
| memory: Sequence[str] = (), | ||
| middleware: Sequence[AgentMiddleware[Any, Any]] = (), | ||
| skills: Sequence[str] | None = None, | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is a logic-changing public change, but the pr does not bump the package version. main is already at 0.16.2 while this branch still has 0.16.1. rebase and bump to the next version in pyproject.toml before merge.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Okay will do this before I merge. |
||
| ) -> CompiledStateGraph[Any, Any, Any, Any]: | ||
| """Create a deepagents agent with planning, filesystem, and sub-agent tools. | ||
|
|
||
| ``memory`` is a list of file paths loaded via deepagents' ``MemoryMiddleware``: | ||
| each is read from ``backend`` and injected into the system prompt every turn, | ||
| and the model maintains them with ``edit_file``. Empty disables the middleware. | ||
|
|
||
| ``skills`` is a list of skill source paths for deepagents' ``SkillsMiddleware``; | ||
| ``None`` or empty disables it (mirroring ``_create_deep_agent``'s contract). | ||
| """ | ||
| return _create_deep_agent( | ||
| model=model, | ||
|
|
@@ -103,6 +107,7 @@ def create_advanced_agent( | |
| response_format=response_format, | ||
| memory=list(memory) or None, | ||
| middleware=list(middleware), | ||
| skills=list(skills) if skills else None, | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this tool is still not wired into the advanced agent. with skills enabled, we only forward the skill paths and leave tools unchanged, so the agent has no way to call uipath_cli. let s add it here for the filesystem-backed skills path and cover the final tool list in a test.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We have this tool behind feature flag and we wired it when creating advanced agent graph for low-code agents if skills are enabled instead of wiring it to advanced agent. The scope for now is to attach the tool for low-code agents. |
||
| ) | ||
|
mariadhakalUipath marked this conversation as resolved.
|
||
|
|
||
|
|
||
|
|
@@ -115,6 +120,7 @@ def create_advanced_agent_graph( | |
| input_schema: type[BaseModel] | None, | ||
| output_schema: type[BaseModel], | ||
| build_user_message: Callable[[dict[str, Any]], str], | ||
| skills: Sequence[str] | None = None, | ||
| ) -> StateGraph[Any, Any, Any, Any]: | ||
| """Wrap the advanced agent in a parent graph that maps typed I/O to/from messages. | ||
|
|
||
|
|
@@ -153,6 +159,7 @@ def create_advanced_agent_graph( | |
| if runtime_system_prompt_key is not None | ||
| else [] | ||
| ), | ||
| skills=skills, | ||
| ) | ||
|
|
||
| wrapper_state = create_state_with_input(input_schema) | ||
|
|
@@ -215,6 +222,7 @@ def create_conversational_advanced_agent_graph( | |
| tools: Sequence[BaseTool], | ||
| system_prompt: str, | ||
| backend: BackendProtocol | BackendFactory | None, | ||
| skills: Sequence[str] | None = None, | ||
| ) -> StateGraph[Any, Any, Any, Any]: | ||
| """Wrap the advanced agent in a parent graph that speaks the conversational contract. | ||
|
|
||
|
|
@@ -237,6 +245,7 @@ def create_conversational_advanced_agent_graph( | |
| system_prompt=system_prompt, | ||
| backend=backend, | ||
| memory=memory_sources, | ||
| skills=skills, | ||
| ) | ||
|
|
||
| class ConversationalAdvancedAgentOutput(BaseModel): | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,6 @@ | ||
| """Internal Tool creation and management for LowCode agents.""" | ||
|
mariadhakalUipath marked this conversation as resolved.
|
||
|
|
||
| from .internal_tool_factory import create_internal_tool | ||
| from .uipath_cli_tool import create_uipath_cli_tool | ||
|
|
||
| __all__ = ["create_internal_tool"] | ||
| __all__ = ["create_internal_tool", "create_uipath_cli_tool"] | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,251 @@ | ||
| """Sandboxed ``uip`` CLI runner injected into advanced agents. | ||
|
|
||
| Unlike the resource-selected internal tools built by ``create_internal_tool``, this | ||
| tool is injected programmatically by the agent graph builder when UiPath skills are | ||
| active — the agent invokes it to run the ``uip`` commands a skill prescribes. It runs | ||
| exactly one ``uip`` invocation per call. | ||
|
mariadhakalUipath marked this conversation as resolved.
|
||
|
|
||
| """ | ||
|
|
||
| import asyncio | ||
| import logging | ||
| import os | ||
| import shlex | ||
| import shutil | ||
| from pathlib import Path | ||
| from typing import Any | ||
|
|
||
| from langchain_core.tools import StructuredTool | ||
| from pydantic import BaseModel, Field | ||
| from uipath.eval.mocks import mockable | ||
| from uipath.runtime import Workspace | ||
|
|
||
| from uipath_langchain.agent.tools.structured_tool_with_argument_properties import ( | ||
| StructuredToolWithArgumentProperties, | ||
| ) | ||
|
|
||
| logger = logging.getLogger("uipath") | ||
|
|
||
| _TOOL_NAME = "uipath_cli" | ||
|
|
||
| _TOOL_DESCRIPTION = ( | ||
| "Run a single UiPath `uip` command in the agent workspace; returns " | ||
| "exit_code/stdout/stderr. Only the uip/uipath binaries, one command per call; " | ||
| "shell chaining is refused. A negative exit_code means the command was refused " | ||
| "before it ran and stderr explains why. Use `subdir` to target a scaffolded " | ||
| "project folder." | ||
| ) | ||
|
|
||
| _ALLOWED_BINARIES: tuple[str, ...] = ("uip", "uipath") | ||
|
|
||
| _REJECTED_EXIT_CODE = -1000 | ||
|
|
||
| _COMMAND_TIMEOUT_SECONDS = 600 | ||
|
mariadhakalUipath marked this conversation as resolved.
|
||
|
|
||
| _SHELL_OPERATORS: frozenset[str] = frozenset({"&&", "||", ";", "|"}) | ||
|
|
||
| _COMMAND_EXAMPLES: tuple[str, ...] = ( | ||
| "solution pack", | ||
| "solution publish", | ||
| "codedagent init", | ||
| ) | ||
|
|
||
|
|
||
| def _parse_uip_command(command: str) -> list[str]: | ||
| """Validate a single ``uip`` command and return its argv (binary excluded). | ||
|
|
||
| Args: | ||
| command: The command to run, e.g. ``"pack"`` or ``"solution publish"``. | ||
| An explicit ``uip``/``uipath`` prefix is tolerated and stripped. | ||
|
|
||
| Returns: | ||
| The argument tokens to pass after the resolved binary. | ||
|
|
||
| Raises: | ||
| ValueError: If the command is empty or chains multiple commands via a shell | ||
| operator. | ||
| """ | ||
| try: | ||
| lexer = shlex.shlex(command, posix=True) | ||
| lexer.whitespace_split = True | ||
| lexer.commenters = "" | ||
| lexer.escape = "" | ||
| tokens = list(lexer) | ||
| except ValueError as exc: | ||
| raise ValueError(f"Could not parse command: {exc}") from exc | ||
|
Copilot marked this conversation as resolved.
Copilot marked this conversation as resolved.
|
||
|
|
||
| if not tokens: | ||
| raise ValueError("No command provided.") | ||
|
|
||
| if tokens[0] in _ALLOWED_BINARIES: | ||
| tokens = tokens[1:] | ||
|
mariadhakalUipath marked this conversation as resolved.
|
||
| if not tokens: | ||
| raise ValueError("No `uip` subcommand provided.") | ||
|
|
||
| for token in tokens: | ||
| if token in _SHELL_OPERATORS: | ||
| raise ValueError( | ||
| f"Shell operator '{token}' is not allowed; run one command at a time." | ||
| ) | ||
|
|
||
| return tokens | ||
|
|
||
|
|
||
| class UiPathCliInput(BaseModel): | ||
| """Input schema for the ``uipath_cli`` tool.""" | ||
|
|
||
| command: str = Field( | ||
| description=( | ||
| "A single `uip` command without the binary prefix, e.g. `solution pack`, " | ||
| "`solution publish`. One command only; no shell operators." | ||
| ), | ||
| examples=list(_COMMAND_EXAMPLES), | ||
| ) | ||
| subdir: str = Field( | ||
| default="", | ||
| description="Workspace-relative directory to run in; defaults to the root.", | ||
| ) | ||
|
|
||
|
|
||
| class UiPathCliOutput(BaseModel): | ||
| """Output schema for the ``uipath_cli`` tool.""" | ||
|
|
||
| command: str = Field(description="The argv actually executed (or attempted).") | ||
| exit_code: int = Field( | ||
| description=( | ||
| f"Process exit code, or {_REJECTED_EXIT_CODE} when the command was " | ||
| "rejected before running (see stderr for the reason)." | ||
| ) | ||
| ) | ||
| stdout: str = Field(description="Captured standard output.") | ||
| stderr: str = Field(description="Captured standard error, or the failure reason.") | ||
|
|
||
|
|
||
| def _rejected(command: str, reason: str) -> dict[str, Any]: | ||
| """Build a 'rejected before running' result carrying the sentinel and reason.""" | ||
| logger.warning("uipath_cli rejected command %r: %s", command, reason) | ||
| return UiPathCliOutput( | ||
| command=command, | ||
| exit_code=_REJECTED_EXIT_CODE, | ||
| stdout="", | ||
| stderr=reason, | ||
| ).model_dump() | ||
|
|
||
|
|
||
| def _resolve_run_dir_within_workspace(workspace_root: Path, subdir: str) -> Path | None: | ||
| """Resolve ``subdir`` under the workspace, or ``None`` if it escapes. | ||
|
|
||
| ``.resolve()`` collapses ``..`` and follows symlinks, so absolute paths, parent | ||
| traversal, and symlink escapes are all rejected. | ||
| """ | ||
| run_dir = (workspace_root / subdir).resolve() | ||
| if run_dir == workspace_root or workspace_root in run_dir.parents: | ||
| return run_dir | ||
| return None | ||
|
|
||
|
|
||
| async def _run_uip_subprocess( | ||
| binary: str, args: list[str], run_dir: Path, echoed: str | ||
| ) -> dict[str, Any]: | ||
| """Run the resolved ``uip`` command in ``run_dir`` and map it to output.""" | ||
| logger.info("uipath_cli running %s (cwd=%s)", echoed, run_dir) | ||
| try: | ||
| proc = await asyncio.create_subprocess_exec( | ||
| binary, | ||
| *args, | ||
| cwd=run_dir, | ||
| stdin=asyncio.subprocess.DEVNULL, | ||
| stdout=asyncio.subprocess.PIPE, | ||
| stderr=asyncio.subprocess.PIPE, | ||
| ) | ||
| except OSError as exc: | ||
| return _rejected(echoed, str(exc)) | ||
|
|
||
| try: | ||
| stdout, stderr = await asyncio.wait_for( | ||
| proc.communicate(), timeout=_COMMAND_TIMEOUT_SECONDS | ||
| ) | ||
| except asyncio.TimeoutError: | ||
| try: | ||
| proc.kill() | ||
| except ProcessLookupError: | ||
| pass | ||
| await proc.wait() | ||
| return _rejected( | ||
| echoed, f"Command timed out after {_COMMAND_TIMEOUT_SECONDS}s." | ||
| ) | ||
|
|
||
| logger.info("uipath_cli command %s exited with code %s", echoed, proc.returncode) | ||
| exit_code = proc.returncode if proc.returncode is not None else _REJECTED_EXIT_CODE | ||
| return UiPathCliOutput( | ||
| command=echoed, | ||
| exit_code=exit_code, | ||
| stdout=stdout.decode(errors="replace"), | ||
| stderr=stderr.decode(errors="replace"), | ||
| ).model_dump() | ||
|
|
||
|
|
||
| def create_uipath_cli_tool(workspace: Workspace) -> StructuredTool: | ||
| """Create the sandboxed ``uipath_cli`` tool bound to ``workspace``. | ||
|
|
||
| The returned tool runs one ``uip`` command per call inside the workspace | ||
| (optionally under ``subdir``) and returns a :class:`UiPathCliOutput` dict. It is | ||
| injected by the agent graph builder rather than selected as a resource, and | ||
| enforces that only the ``uip``/``uipath`` binaries run, one command at a time. | ||
|
|
||
| Args: | ||
| workspace: The agent's workspace; commands run within its directory. Must be a | ||
| real on-disk workspace (inject only for a ``FilesystemBackend``). | ||
|
|
||
| Returns: | ||
| A ``StructuredTool`` named ``uipath_cli``. | ||
| """ | ||
|
|
||
| async def run_uipath_command(command: str, subdir: str = "") -> dict[str, Any]: | ||
|
mariadhakalUipath marked this conversation as resolved.
|
||
| try: | ||
| args = _parse_uip_command(command) | ||
| except ValueError as exc: | ||
| return _rejected(command.strip() or "uip", str(exc)) | ||
|
|
||
| @mockable( | ||
| name=_TOOL_NAME, | ||
| description=_TOOL_DESCRIPTION, | ||
| input_schema=UiPathCliInput.model_json_schema(), | ||
| output_schema=UiPathCliOutput.model_json_schema(), | ||
| example_calls=[], | ||
| ) | ||
| async def _execute(**_tool_kwargs: Any) -> dict[str, Any]: | ||
| binary = shutil.which("uip") or shutil.which("uipath") | ||
| if binary is None: | ||
| return _rejected( | ||
| shlex.join(["uip", *args]), | ||
| "No `uip` or `uipath` binary found on PATH.", | ||
| ) | ||
| echoed = shlex.join([os.path.basename(binary), *args]) | ||
|
|
||
| run_dir = _resolve_run_dir_within_workspace( | ||
| workspace.path.resolve(), subdir | ||
| ) | ||
| if run_dir is None: | ||
| return _rejected( | ||
| echoed, f"Invalid subdir '{subdir}': escapes the workspace." | ||
| ) | ||
|
|
||
| return await _run_uip_subprocess(binary, args, run_dir, echoed) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this still executes the cli arguments directly, without the cli owning and enforcing a safe mode. the parser currently accepts commands such as solution delete and admin tenants delete, so resolving the previous thread did not address the version-skew problem. let s add the safety flag in the cli and always append it here before shipping this tool.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is really good point we were thinking about this, the reason I don't have it implemented is:
|
||
|
|
||
| return await _execute(command=command, subdir=subdir) | ||
|
|
||
| return StructuredToolWithArgumentProperties( | ||
| name=_TOOL_NAME, | ||
| description=_TOOL_DESCRIPTION, | ||
| args_schema=UiPathCliInput, | ||
| coroutine=run_uipath_command, | ||
| output_type=UiPathCliOutput, | ||
| argument_properties={}, | ||
| metadata={ | ||
| "tool_type": _TOOL_NAME, | ||
| "display_name": _TOOL_NAME, | ||
| "args_schema": UiPathCliInput, | ||
| "output_schema": UiPathCliOutput, | ||
| }, | ||
| ) | ||
Uh oh!
There was an error while loading. Please reload this page.