From 593942428e7ffbd562cdecdb4d925bee09175379 Mon Sep 17 00:00:00 2001 From: Eduard Dumitru Date: Mon, 7 Sep 2026 11:14:57 +0200 Subject: [PATCH 01/18] feat: stream pooled job logs to the handler over uipath-ipc The pooled `uipath server` now grabs the handler's IIpcLogSink callback during Register (via the injected Message) and points uipath-runtime's process-global log sink at it, so a pooled job's logs stream back over the existing pipe. Best-effort and version-guarded: an older uipath-runtime without the pooled sink API is a no-op and jobs keep the file path. Register(self, message) gains the reach-back handle; the sink forwards each SendLog onto the server loop from the job's worker thread. Bump 2.14.10 -> 2.14.11. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../uipath/src/uipath/_cli/cli_server_ipc.py | 84 +++++++++++++++- packages/uipath/tests/cli/test_server_ipc.py | 96 ++++++++++++++++++- 2 files changed, 174 insertions(+), 6 deletions(-) diff --git a/packages/uipath/src/uipath/_cli/cli_server_ipc.py b/packages/uipath/src/uipath/_cli/cli_server_ipc.py index d099f1e23..3342a4a30 100644 --- a/packages/uipath/src/uipath/_cli/cli_server_ipc.py +++ b/packages/uipath/src/uipath/_cli/cli_server_ipc.py @@ -1,9 +1,23 @@ +import asyncio from abc import ABC, abstractmethod +from concurrent.futures import Future from dataclasses import dataclass, field +from typing import TYPE_CHECKING from ._server_core import COMMANDS, _run_command_isolated, _state, parse_args from ._utils._console import ConsoleLogger +if TYPE_CHECKING: + from uipath_ipc import Message +else: + # Optional dependency: only present when the uipath-ipc channel is served (uipath[ipc]). + # The Register annotation stays a string forward-ref so this placeholder is never subscripted + # at import; it is resolved (to the real Message) only during dispatch, which needs uipath-ipc. + try: + from uipath_ipc import Message + except ImportError: # pragma: no cover - no IPC means Register is never dispatched + Message = object + console = ConsoleLogger() @@ -42,8 +56,14 @@ class IPythonRuntimeServer(ABC): """Contract the job executor calls over uipath-ipc.""" @abstractmethod - async def Register(self) -> bool: - """Prove the connection is up. No-op until there is something to register.""" + async def Register(self, message: "Message[None]") -> bool: + """Prove the connection is up, and grab the caller's log-sink callback. + + The injected ``message`` is a reach-back handle (it carries no wire + argument): ``message.client.get_callback`` reaches the handler's + ``IIpcLogSink`` over this same pipe, which pooled jobs stream their logs + into. + """ @abstractmethod async def RunJob(self, request: PythonServerRunRequest) -> PythonServerRunJobResult: @@ -57,7 +77,8 @@ async def StopJob(self, request: PythonServerStopJobRequest) -> bool: class PythonRuntimeService(IPythonRuntimeServer): """``IPythonRuntimeServer`` implementation backed by run/debug/eval.""" - async def Register(self) -> bool: + async def Register(self, message: "Message[None]") -> bool: + _wire_pooled_log_sink(message) console.info("Runtime client registered.") return True @@ -96,6 +117,49 @@ async def StopJob(self, request: PythonServerStopJobRequest) -> bool: return True +def _drain(future: "Future[object]") -> None: + # Retrieve (and discard) a forwarded log's result so a failed one-way send never surfaces. + try: + future.exception() + except BaseException: + pass + + +def _wire_pooled_log_sink(message: "Message[None]") -> None: + """Point the runtime's process-global log sink at this connection's IIpcLogSink callback. + + Pooled jobs run in this same process (``_run_command_isolated`` → a worker thread), so their + logging can reach back to the handler over the pipe the handler dialed in on. The runtime raises + each record to a process-global sink; here we make that sink forward to the handler's callback. + + Best-effort: log streaming is an add-on to the Register handshake, so any failure (an older + uipath-runtime without the pooled sink API, or a peer hosting no callback) leaves jobs on their + file+watcher path and never fails registration. + """ + # A Message built without a caller handle has nothing to reach back to. + client = message.client + if client is None: + return + try: + from uipath.runtime.jobapi import ( # type: ignore[import-untyped] + IIpcLogSink, + set_pooled_log_sink, + ) + except ImportError: + return # runtime predates the pooled sink — nothing to wire; jobs keep the file path + + sink_proxy = client.get_callback(IIpcLogSink) + loop = asyncio.get_running_loop() + + def _forward(job_id: str, log: object) -> None: + # Runs on the job's worker thread: hand the one-way SendLog to the server loop and return at + # once. A dropped log (pipe down) must never surface into the job, so failures are swallowed. + future = asyncio.run_coroutine_threadsafe(sink_proxy.SendLog(job_id, log), loop) + future.add_done_callback(_drain) + + set_pooled_log_sink(_forward) + + async def start_ipc_server(pipe_name: str) -> None: """Serve the Python runtime over a uipath-ipc named pipe until it is closed.""" try: @@ -114,5 +178,15 @@ async def start_ipc_server(pipe_name: str) -> None: request_timeout=None, # jobs are long-running; no server-side timeout ) console.success(f"IPC server listening on pipe '{pipe_name}'") - async with server: - await server.serve_forever() + try: + async with server: + await server.serve_forever() + finally: + # Drop the process-global sink so it can't outlive this loop (matters if the server is + # ever restarted in-process, e.g. in tests). + try: + from uipath.runtime.jobapi import set_pooled_log_sink + + set_pooled_log_sink(None) + except ImportError: + pass diff --git a/packages/uipath/tests/cli/test_server_ipc.py b/packages/uipath/tests/cli/test_server_ipc.py index 801920dd2..3f5a3a708 100644 --- a/packages/uipath/tests/cli/test_server_ipc.py +++ b/packages/uipath/tests/cli/test_server_ipc.py @@ -17,6 +17,7 @@ import sys import threading import time +import types from typing import Any, Awaitable, Callable import click @@ -24,6 +25,7 @@ from uipath_ipc import ( IpcClient, IpcServer, + Message, NamedPipeClientTransport, NamedPipeServerTransport, ) @@ -31,6 +33,7 @@ from uipath._cli import _server_core from uipath._cli.cli_server import ( IPythonRuntimeServer, + PythonRuntimeService, PythonServerRunJobResult, start_ipc_server, ) @@ -284,7 +287,7 @@ def test_all_wire_fields_arrive_intact(self): received: list[Any] = [] class SpyService(IPythonRuntimeServer): - async def Register(self) -> bool: + async def Register(self, message: Any) -> bool: return True async def RunJob(self, request: Any) -> PythonServerRunJobResult: @@ -329,3 +332,94 @@ async def drive(proxy: Any) -> None: assert stop_request.JobKey == job_key assert stop_request.ResumeVersion == 5 assert stop_request.ForceStop is True + + +class TestPooledLogSink: + """``Register`` wires the runtime's process-global log sink to the caller's callback. + + This is the pooled path: the handler dials in and hosts an ``IIpcLogSink`` callback; the server + grabs it off the injected ``Message`` and points the runtime's sink at it, so a pooled job's logs + reach back over the same pipe. The runtime side is exercised in uipath-runtime's own tests; here + we lock in the seam (get_callback → set_pooled_log_sink → forward on the loop). + """ + + @staticmethod + def _fake_runtime_jobapi(monkeypatch: Any) -> dict[str, Any]: + """Install a stand-in ``uipath.runtime.jobapi`` and record what the server wires into it.""" + captured: dict[str, Any] = {} + + class IIpcLogSink: # matches the contract the server asks get_callback for + pass + + def set_pooled_log_sink(sink: Any) -> None: + captured["sink"] = sink + + module = types.ModuleType("uipath.runtime.jobapi") + module.IIpcLogSink = IIpcLogSink # type: ignore[attr-defined] + module.set_pooled_log_sink = set_pooled_log_sink # type: ignore[attr-defined] + monkeypatch.setitem(sys.modules, "uipath.runtime.jobapi", module) + captured["IIpcLogSink"] = IIpcLogSink + return captured + + def test_register_forwards_logs_to_the_caller_callback(self, monkeypatch): + captured = self._fake_runtime_jobapi(monkeypatch) + sent: list[tuple[str, Any]] = [] + + class _Callback: + async def SendLog(self, job_id: str, log: Any) -> None: + sent.append((job_id, log)) + + class _Client: + def __init__(self) -> None: + self.asked_for: Any = None + + def get_callback(self, contract: Any) -> Any: + self.asked_for = contract + return _Callback() + + client = _Client() + + async def scenario() -> None: + ok = await PythonRuntimeService().Register(Message(client=client)) + assert ok is True + # It asked the caller for exactly the IIpcLogSink callback... + assert client.asked_for is captured["IIpcLogSink"] + # ...and registered a forwarder. Driving it (as the runtime would, off a worker thread) + # schedules SendLog on this loop, tagged with the job id. + sink = captured["sink"] + log = {"Message": "hello", "LogLevel": 2} + sink("job-key-42", log) + await asyncio.sleep(0.05) + assert sent == [("job-key-42", log)] + + asyncio.run(scenario()) + + def test_register_is_graceful_when_runtime_lacks_pooled_sink(self, monkeypatch): + # An older uipath-runtime has no jobapi module: importing it raises, and Register must still + # succeed (the job simply keeps its file+watcher log path). + monkeypatch.setitem(sys.modules, "uipath.runtime.jobapi", None) + + class _Client: + def get_callback( + self, contract: Any + ) -> Any: # pragma: no cover - never reached + raise AssertionError( + "must not reach get_callback without the runtime API" + ) + + async def scenario() -> None: + assert ( + await PythonRuntimeService().Register(Message(client=_Client())) is True + ) + + asyncio.run(scenario()) + + def test_register_without_a_caller_handle_is_a_noop(self, monkeypatch): + # A Message with no client (defensive; real dispatch always injects one) wires nothing. + captured = self._fake_runtime_jobapi(monkeypatch) + + async def scenario() -> None: + assert await PythonRuntimeService().Register(Message()) is True + + asyncio.run(scenario()) + assert "sink" not in captured From 6fc05ef923beeb2c835815c98401f75e9f538611 Mon Sep 17 00:00:00 2001 From: Eduard Dumitru Date: Tue, 8 Sep 2026 14:02:53 +0200 Subject: [PATCH 02/18] feat(cli): stream job logs and result to the handler over uipath-ipc uipath-python owns the uipath-ipc contract and connection (_job_api.py) and points uipath-runtime's in-memory sinks at it. The contract is IJobInvocationCommonApi (SendLog + SetResult), the runtime-agnostic base the handler hosts; the JS-only telemetry channel lives on a .NET superset Python doesn't use, so Python keys on the common base directly. Pooled: `uipath server` grabs the handler's IJobInvocationCommonApi callback at Register and installs the sinks per job. Non-pooled: `uipath run --handler-ipc-pipe` dials the handler's per-job server. The result envelope travels inline; output arguments go off-heap via a file pointer. Review fixes: - run the non-pooled handler connection on its own loop/thread so the result sink's blocking ack can't deadlock the job's own event loop - install/clear the process-global sinks inside the job core's lock so concurrently-dispatched RunJobs can't corrupt each other's routing - log (not silently swallow) an IPC result-delivery failure - always disconnect the handler IPC on the run's exit path (async context manager) - register the default runtime factory in start_ipc_server - pin the DTO wire key sets and the log-level mapping in tests Co-Authored-By: Claude Opus 4.8 (1M context) --- packages/uipath/src/uipath/_cli/_job_api.py | 289 ++++++++++++++++++ .../uipath/src/uipath/_cli/_server_core.py | 22 +- packages/uipath/src/uipath/_cli/cli_run.py | 17 +- .../uipath/src/uipath/_cli/cli_server_ipc.py | 116 ++++--- packages/uipath/tests/cli/test_job_api.py | 284 +++++++++++++++++ packages/uipath/tests/cli/test_server_ipc.py | 150 +++++---- 6 files changed, 741 insertions(+), 137 deletions(-) create mode 100644 packages/uipath/src/uipath/_cli/_job_api.py create mode 100644 packages/uipath/tests/cli/test_job_api.py diff --git a/packages/uipath/src/uipath/_cli/_job_api.py b/packages/uipath/src/uipath/_cli/_job_api.py new file mode 100644 index 000000000..21666c1f7 --- /dev/null +++ b/packages/uipath/src/uipath/_cli/_job_api.py @@ -0,0 +1,289 @@ +"""The job-invocation IPC contract and the glue that routes a job's logs and result over it.""" + +from __future__ import annotations + +import asyncio +import contextlib +import logging +import sys +import threading +from abc import ABC, abstractmethod +from collections.abc import AsyncIterator +from concurrent.futures import Future +from dataclasses import dataclass +from enum import IntEnum +from typing import Any + +logger = logging.getLogger(__name__) + +_SET_RESULT_TIMEOUT_S = 30.0 + + +class LogLevel(IntEnum): + """Log-level wire values.""" + + TRACE = 0 + DEBUG = 1 + INFORMATION = 2 + WARNING = 3 + ERROR = 4 + CRITICAL = 5 + NONE = 6 + + +class ExecutorJobStatus(IntEnum): + """Job-status wire values.""" + + RUNNING = 1 + FAULTED = 2 + SUCCESSFUL = 3 + STOPPED = 4 + SUSPENDED = 5 + + +@dataclass +class JobLogDto: + """A log entry; field names are the wire keys (do not rename).""" + + Message: str = "" + LogLevel: int = LogLevel.INFORMATION.value + + +@dataclass +class JobExecutorError: + """A result error; field names are the wire keys (do not rename).""" + + Code: str | None = None + Title: str | None = None + Detail: str | None = None + Category: str | None = None + Status: int | None = None + + +@dataclass +class JobResultDto: + """The final result; field names are the wire keys (do not rename).""" + + id: str = "" + status: int = ExecutorJobStatus.SUCCESSFUL.value + outputArguments: Any = None + outputArgumentsFilePath: str | None = None + info: str | None = None + error: JobExecutorError | None = None + + +class IJobInvocationCommonApi(ABC): + """The job-invocation contract: logs + the final result. The class name is the endpoint key.""" + + @abstractmethod + async def SendLog(self, jobId: str, log: JobLogDto) -> None: + """Forward one log entry.""" + + @abstractmethod + async def SetResult(self, jobId: str, result: JobResultDto) -> bool: + """Submit the final result.""" + + +def _to_log_level(levelno: int) -> int: + if levelno >= logging.CRITICAL: + return LogLevel.CRITICAL + if levelno >= logging.ERROR: + return LogLevel.ERROR + if levelno >= logging.WARNING: + return LogLevel.WARNING + if levelno >= logging.INFO: + return LogLevel.INFORMATION + if levelno >= logging.DEBUG: + return LogLevel.DEBUG + return LogLevel.TRACE + + +_EXECUTOR_STATUS: dict[str, int] = { + "successful": ExecutorJobStatus.SUCCESSFUL.value, + "faulted": ExecutorJobStatus.FAULTED.value, + "suspended": ExecutorJobStatus.SUSPENDED.value, +} + + +def _to_result_dto( + job_id: str, result: Any, output_arguments_file_path: str +) -> JobResultDto: + error = None + if result is not None and getattr(result, "error", None) is not None: + category = result.error.category + error = JobExecutorError( + Code=result.error.code, + Title=result.error.title, + Detail=result.error.detail, + Category=getattr(category, "value", category), + Status=result.error.status, + ) + raw_status = getattr(result, "status", None) + status_key = str(getattr(raw_status, "value", raw_status) or "successful").lower() + return JobResultDto( + id=job_id, + status=_EXECUTOR_STATUS.get(status_key, ExecutorJobStatus.SUCCESSFUL.value), + outputArgumentsFilePath=output_arguments_file_path, + error=error, + ) + + +def _drain(future: "Future[object]") -> None: + try: + future.exception() + except BaseException: + pass + + +class _IpcLogHandler(logging.Handler): + """Forwards each log record to the callback.""" + + def __init__( + self, job_id: str, callback: Any, loop: asyncio.AbstractEventLoop + ) -> None: + super().__init__() + self._job_id = job_id + self._callback = callback + self._loop = loop + + def emit(self, record: logging.LogRecord) -> None: + try: + message = self.format(record) + dto = JobLogDto(Message=message, LogLevel=_to_log_level(record.levelno)) + future = asyncio.run_coroutine_threadsafe( + self._callback.SendLog(self._job_id, dto), self._loop + ) + future.add_done_callback(_drain) + except Exception: + self.handleError(record) + + +def install_runtime_sinks( + job_id: str, callback: Any, loop: asyncio.AbstractEventLoop +) -> None: + """Install the log + result sinks, forwarding to ``callback`` on ``loop``. + + ``loop`` must run on a different thread than the one the sinks are invoked on, or the result ack + deadlocks. No-op if the sinks aren't available. + """ + try: + from uipath.runtime.output_sinks import ( # type: ignore[import-untyped] + set_log_handler, + set_result_sink, + ) + except ImportError: + return + + handler = _IpcLogHandler(job_id, callback, loop) + handler.setFormatter(logging.Formatter("%(message)s")) + + def _result_sink(result: Any, output_arguments_file_path: str) -> None: + dto = _to_result_dto(job_id, result, output_arguments_file_path) + try: + future = asyncio.run_coroutine_threadsafe( + callback.SetResult(job_id, dto), loop + ) + future.result(timeout=_SET_RESULT_TIMEOUT_S) + except Exception: + # Best-effort — a dropped result must be logged, not swallowed. + logger.exception("Failed to deliver job result over IPC (SetResult)") + + set_log_handler(handler) + set_result_sink(_result_sink) + + +def clear_runtime_sinks() -> None: + """Clear the installed sinks.""" + try: + from uipath.runtime.output_sinks import set_log_handler, set_result_sink + except ImportError: + return + set_log_handler(None) + set_result_sink(None) + + +def _new_ipc_event_loop() -> asyncio.AbstractEventLoop: + """A fresh event loop for the connection's thread (Proactor on Windows, required for named pipes).""" + if sys.platform == "win32": + return asyncio.ProactorEventLoop() + return asyncio.new_event_loop() + + +class _HandlerIpcConnection: + """An IPC client on its own loop/thread, so the synchronous result ack can't deadlock the job's loop.""" + + def __init__( + self, + client: Any, + loop: asyncio.AbstractEventLoop, + thread: threading.Thread, + ) -> None: + self._client = client + self._loop = loop + self._thread = thread + + def _shutdown(self) -> None: + """Close the client and stop its loop/thread.""" + try: + asyncio.run_coroutine_threadsafe(self._client.aclose(), self._loop).result( + timeout=_SET_RESULT_TIMEOUT_S + ) + except Exception: + pass + self._loop.call_soon_threadsafe(self._loop.stop) + self._thread.join(timeout=_SET_RESULT_TIMEOUT_S) + self._loop.close() + + +def connect_handler_ipc(pipe: str, job_id: str) -> _HandlerIpcConnection: + """Dial ``pipe`` on a dedicated loop/thread and install the sinks (see ``_HandlerIpcConnection``).""" + try: + from uipath_ipc import IpcClient, NamedPipeClientTransport + except ImportError as e: + raise RuntimeError( + "--handler-ipc-pipe requires the 'uipath-ipc' package. Install it (pip install 'uipath[ipc]')." + ) from e + + loop = _new_ipc_event_loop() + thread = threading.Thread( + target=loop.run_forever, name="uipath-handler-ipc", daemon=True + ) + thread.start() + + async def _build() -> Any: + client = IpcClient(transport=NamedPipeClientTransport(pipe)) + proxy = client.get_proxy(IJobInvocationCommonApi) # type: ignore[type-abstract] + return client, proxy + + try: + client, proxy = asyncio.run_coroutine_threadsafe(_build(), loop).result( + timeout=_SET_RESULT_TIMEOUT_S + ) + except BaseException: + # On failure, don't leak the loop/thread. + loop.call_soon_threadsafe(loop.stop) + thread.join(timeout=_SET_RESULT_TIMEOUT_S) + loop.close() + raise + # Install in the caller's context, not on the IPC thread: the sinks are contextvars, resolved in + # the job's own context at teardown. The ack still runs on `loop` (a separate thread) — no deadlock. + install_runtime_sinks(job_id, proxy, loop) + return _HandlerIpcConnection(client, loop, thread) + + +async def disconnect_handler_ipc(conn: _HandlerIpcConnection) -> None: + """Clear the sinks and tear down the connection.""" + clear_runtime_sinks() + # Off the caller's loop so joining the thread doesn't block it. + await asyncio.to_thread(conn._shutdown) + + +@contextlib.asynccontextmanager +async def handler_ipc_connection(pipe: str | None, job_id: str) -> AsyncIterator[Any]: + """Connect (if ``pipe`` is set) and always disconnect on exit; yields the connection or None.""" + conn = connect_handler_ipc(pipe, job_id) if pipe else None + try: + yield conn + finally: + if conn is not None: + await disconnect_handler_ipc(conn) diff --git a/packages/uipath/src/uipath/_cli/_server_core.py b/packages/uipath/src/uipath/_cli/_server_core.py index c426126ff..25d9614c5 100644 --- a/packages/uipath/src/uipath/_cli/_server_core.py +++ b/packages/uipath/src/uipath/_cli/_server_core.py @@ -3,7 +3,7 @@ import asyncio import os import shlex -from typing import Any +from typing import Any, Callable from .cli_debug import debug from .cli_eval import eval @@ -50,8 +50,14 @@ async def _run_command_isolated( args: list[str], env_vars: dict[str, str], working_dir: str | None, + on_run_start: Callable[[], None] | None = None, + on_run_end: Callable[[], None] | None = None, ) -> dict[str, Any]: - """Run one command with per-job env/cwd isolation (the shared job core).""" + """Run one command with per-job env/cwd isolation (the shared job core). + + ``on_run_start`` / ``on_run_end`` run INSIDE the serialization lock, so any per-job process-global + state is visible only while this job runs. + """ if _state.lock is None or _state.baseline_env is None: raise RuntimeError("Server state not initialized") @@ -79,9 +85,15 @@ async def _run_command_isolated( "ClientError": True, } - result_value = await asyncio.to_thread( - cmd.main, args, standalone_mode=False - ) + if on_run_start is not None: + on_run_start() + try: + result_value = await asyncio.to_thread( + cmd.main, args, standalone_mode=False + ) + finally: + if on_run_end is not None: + on_run_end() return { "ExitCode": 0, "Error": None, diff --git a/packages/uipath/src/uipath/_cli/cli_run.py b/packages/uipath/src/uipath/_cli/cli_run.py index ffd89be9c..a07b1a607 100644 --- a/packages/uipath/src/uipath/_cli/cli_run.py +++ b/packages/uipath/src/uipath/_cli/cli_run.py @@ -116,6 +116,12 @@ def get_usage_help(self) -> list[str]: default=None, help="Simulation config as a JSON object (same schema as simulation.json)", ) +@click.option( + "--handler-ipc-pipe", + required=False, + default=None, + help="Named pipe to stream this job's logs and result over uipath-ipc instead of writing them to files.", +) @track_command("run") def run( entrypoint: str | None, @@ -130,6 +136,7 @@ def run( debug_port: int, keep_state_file: bool, simulation: str | None, + handler_ipc_pipe: str | None, ) -> None: """Execute the project.""" input_file = file or input_file @@ -213,8 +220,14 @@ async def execute() -> None: JsonLinesFileExporter(ctx.trace_file) ) - async with ResourceOverwritesContext( - lambda: read_resource_overwrites_from_file(ctx.runtime_dir) + # If a pipe was given, install the sinks around the run (always torn down); else a no-op. + from ._job_api import handler_ipc_connection + + async with ( + handler_ipc_connection(handler_ipc_pipe, ctx.job_id or ""), + ResourceOverwritesContext( + lambda: read_resource_overwrites_from_file(ctx.runtime_dir) + ), ): with ExecutionSourceContext(ctx.execution_source), ctx: base_runtime: UiPathRuntimeProtocol | None = None diff --git a/packages/uipath/src/uipath/_cli/cli_server_ipc.py b/packages/uipath/src/uipath/_cli/cli_server_ipc.py index 3342a4a30..8e0442200 100644 --- a/packages/uipath/src/uipath/_cli/cli_server_ipc.py +++ b/packages/uipath/src/uipath/_cli/cli_server_ipc.py @@ -1,8 +1,7 @@ import asyncio from abc import ABC, abstractmethod -from concurrent.futures import Future from dataclasses import dataclass, field -from typing import TYPE_CHECKING +from typing import TYPE_CHECKING, Any from ._server_core import COMMANDS, _run_command_isolated, _state, parse_args from ._utils._console import ConsoleLogger @@ -10,9 +9,8 @@ if TYPE_CHECKING: from uipath_ipc import Message else: - # Optional dependency: only present when the uipath-ipc channel is served (uipath[ipc]). - # The Register annotation stays a string forward-ref so this placeholder is never subscripted - # at import; it is resolved (to the real Message) only during dispatch, which needs uipath-ipc. + # Optional dependency (uipath[ipc]): the Register annotation stays a string forward-ref so this + # placeholder is never subscripted at import — resolved to the real Message only at dispatch. try: from uipath_ipc import Message except ImportError: # pragma: no cover - no IPC means Register is never dispatched @@ -37,6 +35,8 @@ class PythonServerRunRequest: Args: str | list[str] | None = None WorkingDirectory: str | None = None EnvironmentVariables: dict[str, str] = field(default_factory=dict) + # Per-job opt-in (default False so it stays off unless explicitly set). + StreamOutputOverIpc: bool = False @dataclass @@ -57,13 +57,7 @@ class IPythonRuntimeServer(ABC): @abstractmethod async def Register(self, message: "Message[None]") -> bool: - """Prove the connection is up, and grab the caller's log-sink callback. - - The injected ``message`` is a reach-back handle (it carries no wire - argument): ``message.client.get_callback`` reaches the handler's - ``IIpcLogSink`` over this same pipe, which pooled jobs stream their logs - into. - """ + """Prove the connection is up and grab the caller's callback via ``message.client.get_callback``.""" @abstractmethod async def RunJob(self, request: PythonServerRunRequest) -> PythonServerRunJobResult: @@ -77,8 +71,23 @@ async def StopJob(self, request: PythonServerStopJobRequest) -> bool: class PythonRuntimeService(IPythonRuntimeServer): """``IPythonRuntimeServer`` implementation backed by run/debug/eval.""" + def __init__(self) -> None: + # The caller's callback, grabbed at Register (None until then). + self._callback: Any = None + self._loop: "asyncio.AbstractEventLoop | None" = None + async def Register(self, message: "Message[None]") -> bool: - _wire_pooled_log_sink(message) + client = message.client + if client is not None: + try: + from ._job_api import IJobInvocationCommonApi + + self._callback = client.get_callback(IJobInvocationCommonApi) # type: ignore[type-abstract] + self._loop = asyncio.get_running_loop() + except Exception: + self._callback = ( + None # older runtime / no callback: jobs keep the file path + ) console.info("Runtime client registered.") return True @@ -101,9 +110,27 @@ async def RunJob(self, request: PythonServerRunRequest) -> PythonServerRunJobRes f"Running job {_run_id(request.JobKey, request.ResumeVersion)}: {command_name} {args}" ) + # Only when opted in and a callback exists. Install/clear the sinks INSIDE the job core's lock + # (via the hooks) so they're bound only while THIS job runs. + callback, loop = self._callback, self._loop + on_run_start: "Any" = None + on_run_end: "Any" = None + if request.StreamOutputOverIpc and callback is not None and loop is not None: + from ._job_api import clear_runtime_sinks, install_runtime_sinks + + job_key = request.JobKey + on_run_start = lambda: install_runtime_sinks(job_key, callback, loop) # noqa: E731 + on_run_end = clear_runtime_sinks + result = await _run_command_isolated( - cmd, args, request.EnvironmentVariables, request.WorkingDirectory + cmd, + args, + request.EnvironmentVariables, + request.WorkingDirectory, + on_run_start=on_run_start, + on_run_end=on_run_end, ) + # IPC contract (PythonServerRunJobResult) carries only ExitCode + Error. return PythonServerRunJobResult( ExitCode=result["ExitCode"], Error=result["Error"] @@ -117,49 +144,6 @@ async def StopJob(self, request: PythonServerStopJobRequest) -> bool: return True -def _drain(future: "Future[object]") -> None: - # Retrieve (and discard) a forwarded log's result so a failed one-way send never surfaces. - try: - future.exception() - except BaseException: - pass - - -def _wire_pooled_log_sink(message: "Message[None]") -> None: - """Point the runtime's process-global log sink at this connection's IIpcLogSink callback. - - Pooled jobs run in this same process (``_run_command_isolated`` → a worker thread), so their - logging can reach back to the handler over the pipe the handler dialed in on. The runtime raises - each record to a process-global sink; here we make that sink forward to the handler's callback. - - Best-effort: log streaming is an add-on to the Register handshake, so any failure (an older - uipath-runtime without the pooled sink API, or a peer hosting no callback) leaves jobs on their - file+watcher path and never fails registration. - """ - # A Message built without a caller handle has nothing to reach back to. - client = message.client - if client is None: - return - try: - from uipath.runtime.jobapi import ( # type: ignore[import-untyped] - IIpcLogSink, - set_pooled_log_sink, - ) - except ImportError: - return # runtime predates the pooled sink — nothing to wire; jobs keep the file path - - sink_proxy = client.get_callback(IIpcLogSink) - loop = asyncio.get_running_loop() - - def _forward(job_id: str, log: object) -> None: - # Runs on the job's worker thread: hand the one-way SendLog to the server loop and return at - # once. A dropped log (pipe down) must never surface into the job, so failures are swallowed. - future = asyncio.run_coroutine_threadsafe(sink_proxy.SendLog(job_id, log), loop) - future.add_done_callback(_drain) - - set_pooled_log_sink(_forward) - - async def start_ipc_server(pipe_name: str) -> None: """Serve the Python runtime over a uipath-ipc named pipe until it is closed.""" try: @@ -172,6 +156,12 @@ async def start_ipc_server(pipe_name: str) -> None: ) from e _state.init() + + # Register the default runtime factory (idempotent) so the server works when started outside the CLI. + from uipath._cli import _ensure_runtime_initialized + + _ensure_runtime_initialized() + server = IpcServer( transport=NamedPipeServerTransport(pipe_name), services={IPythonRuntimeServer: PythonRuntimeService()}, @@ -182,11 +172,7 @@ async def start_ipc_server(pipe_name: str) -> None: async with server: await server.serve_forever() finally: - # Drop the process-global sink so it can't outlive this loop (matters if the server is - # ever restarted in-process, e.g. in tests). - try: - from uipath.runtime.jobapi import set_pooled_log_sink - - set_pooled_log_sink(None) - except ImportError: - pass + # Drop the sinks so they can't outlive this loop (matters on in-process restart). + from ._job_api import clear_runtime_sinks + + clear_runtime_sinks() diff --git a/packages/uipath/tests/cli/test_job_api.py b/packages/uipath/tests/cli/test_job_api.py new file mode 100644 index 000000000..0317f0337 --- /dev/null +++ b/packages/uipath/tests/cli/test_job_api.py @@ -0,0 +1,284 @@ +"""The uipath-python job-api glue: result mapping and the runtime-sink installer. + +The runtime side (``uipath.runtime.output_sinks``) is faked here so these tests exercise the wiring +in isolation — that install points a log handler + result sink at the callback, that the handler +forwards SendLog, and that the result sink maps the runtime result and calls SetResult. +""" + +import asyncio +import logging +import os +import sys +import threading +import types +from typing import Any + +import pytest + +from uipath._cli import _job_api + + +def test_to_result_dto_maps_status_error_and_path(): + class _Category: + value = "User" + + class _Error: + code = "BOOM" + title = "It broke" + detail = "stack" + category = _Category() + status = None + + class _Result: + status = "faulted" + error = _Error() + + dto = _job_api._to_result_dto("job-1", _Result(), "out.args") + + assert dto.id == "job-1" + assert dto.status == _job_api.ExecutorJobStatus.FAULTED.value + assert dto.outputArgumentsFilePath == "out.args" + assert dto.outputArguments is None + assert dto.error is not None + assert (dto.error.Code, dto.error.Category) == ("BOOM", "User") + + +def test_to_result_dto_defaults_to_successful_without_error(): + class _Result: + status = "successful" + error = None + + dto = _job_api._to_result_dto("j", _Result(), "p.args") + assert dto.status == _job_api.ExecutorJobStatus.SUCCESSFUL.value + assert dto.error is None + + +def test_to_log_level_maps_python_levels_to_wire_values(): + assert _job_api._to_log_level(logging.CRITICAL) == _job_api.LogLevel.CRITICAL + assert _job_api._to_log_level(logging.ERROR) == _job_api.LogLevel.ERROR + assert _job_api._to_log_level(logging.WARNING) == _job_api.LogLevel.WARNING + assert _job_api._to_log_level(logging.INFO) == _job_api.LogLevel.INFORMATION + assert _job_api._to_log_level(logging.DEBUG) == _job_api.LogLevel.DEBUG + assert _job_api._to_log_level(logging.NOTSET) == _job_api.LogLevel.TRACE + # A level between two named severities rounds down to the lower one. + assert _job_api._to_log_level(logging.WARNING + 5) == _job_api.LogLevel.WARNING + + +def test_dto_wire_key_sets_are_pinned(): + """Pin each DTO's on-wire JSON keys so an accidental rename is caught on this side. + + Guards our half of the wire contract: JobResultDto is camelCase, JobLogDto / JobExecutorError + are PascalCase. + """ + serialization = pytest.importorskip("uipath_ipc.wire.serialization") + to_wire = serialization.to_wire + + result_keys = set( + to_wire(_job_api.JobResultDto(id="j", outputArgumentsFilePath="p.args")) + ) + assert result_keys == { + "id", + "status", + "outputArguments", + "outputArgumentsFilePath", + "info", + "error", + } + assert set(to_wire(_job_api.JobLogDto(Message="m"))) == {"Message", "LogLevel"} + assert set(to_wire(_job_api.JobExecutorError(Code="c"))) == { + "Code", + "Title", + "Detail", + "Category", + "Status", + } + + +def _fake_output_sinks(monkeypatch) -> dict[str, Any]: + captured: dict[str, Any] = {} + module = types.ModuleType("uipath.runtime.output_sinks") + module.set_log_handler = lambda h: captured.__setitem__("handler", h) # type: ignore[attr-defined] + module.set_result_sink = lambda s: captured.__setitem__("sink", s) # type: ignore[attr-defined] + monkeypatch.setitem(sys.modules, "uipath.runtime.output_sinks", module) + return captured + + +def test_install_wires_log_handler_and_result_sink(monkeypatch): + captured = _fake_output_sinks(monkeypatch) + logs: list[tuple[str, Any]] = [] + results: list[tuple[str, Any]] = [] + + class _Callback: + async def SendLog(self, jid: str, dto: Any) -> None: + logs.append((jid, dto)) + + async def SetResult(self, jid: str, dto: Any) -> bool: + results.append((jid, dto)) + return True + + async def scenario() -> None: + loop = asyncio.get_running_loop() + _job_api.install_runtime_sinks("job-7", _Callback(), loop) + + # The log handler forwards each record as SendLog, tagged with the job id. + handler = captured["handler"] + handler.emit( + logging.LogRecord("n", logging.WARNING, "p", 1, "hi %s", ("there",), None) + ) + await asyncio.sleep(0.05) + assert logs[0][0] == "job-7" + assert logs[0][1].Message == "hi there" + assert logs[0][1].LogLevel == _job_api.LogLevel.WARNING.value + + # The result sink maps the result and calls SetResult, off a worker thread, for the ack. + class _Result: + status = "successful" + error = None + + sink = captured["sink"] + await asyncio.to_thread(sink, _Result(), "out.args") + assert results[0][0] == "job-7" + assert results[0][1].outputArgumentsFilePath == "out.args" + + asyncio.run(scenario()) + + +def test_install_is_a_noop_without_the_runtime(monkeypatch): + # An older uipath-runtime has no output_sinks module: install/clear must not raise. + monkeypatch.setitem(sys.modules, "uipath.runtime.output_sinks", None) + _job_api.install_runtime_sinks("j", object(), asyncio.new_event_loop()) + _job_api.clear_runtime_sinks() + + +def test_connect_installs_sinks_and_disconnect_clears(monkeypatch): + pytest.importorskip("uipath_ipc") + captured = _fake_output_sinks(monkeypatch) + + async def scenario() -> None: + # A named-pipe client connects lazily, so no server is needed to build it. + client = _job_api.connect_handler_ipc("some-pipe", "job-1") + assert captured["handler"] is not None + assert captured["sink"] is not None + + await _job_api.disconnect_handler_ipc(client) + assert captured["handler"] is None + assert captured["sink"] is None + + asyncio.run(scenario()) + + +def test_connect_without_uipath_ipc_raises(monkeypatch): + monkeypatch.setitem(sys.modules, "uipath_ipc", None) + with pytest.raises(RuntimeError, match="uipath-ipc"): + _job_api.connect_handler_ipc("pipe", "job-1") + + +_jobapi_pipe_counter = 0 + + +def _unique_jobapi_pipe() -> str: + global _jobapi_pipe_counter + _jobapi_pipe_counter += 1 + return f"uipath-jobapi-test-{os.getpid()}-{_jobapi_pipe_counter}" + + +def _serve_jobapi_in_background(pipe: str, api: Any): + """Host ``api`` as IJobInvocationCommonApi on ``pipe`` in a daemon thread; return a stop() callable. + + The server runs on its OWN loop/thread so it can keep accepting while the test thread is + blocked inside the (synchronous) result sink — the whole point of the regression below. + """ + from uipath_ipc import IpcServer, NamedPipeServerTransport + + loop = asyncio.new_event_loop() + ready = threading.Event() + holder: dict[str, Any] = {} + + async def _serve() -> None: + server = IpcServer( + transport=NamedPipeServerTransport(pipe), + services={_job_api.IJobInvocationCommonApi: api}, + request_timeout=None, + ) + holder["server"] = server + async with server: # __aenter__ binds the listener + ready.set() + await server.serve_forever() + + def _run() -> None: + asyncio.set_event_loop(loop) + try: + loop.run_until_complete(_serve()) + except Exception: + pass + finally: + loop.close() + + thread = threading.Thread(target=_run, name="jobapi-test-server", daemon=True) + thread.start() + if not ready.wait(timeout=10): + raise TimeoutError(f"job-api test server on {pipe!r} did not start") + + def stop() -> None: + server = holder.get("server") + if server is not None: + try: + asyncio.run_coroutine_threadsafe(server.aclose(), loop).result( + timeout=10 + ) + except Exception: + pass + thread.join(timeout=10) + + return stop + + +def test_result_sink_delivers_when_invoked_on_the_caller_loop_thread(monkeypatch): + """Regression for the non-pooled deadlock. + + Under ``uipath run`` the runtime invokes the result sink synchronously on the job's own asyncio + loop thread, and the sink blocks for the handler's ack. If that ack were scheduled onto the same + loop it could never run — 30s timeout, dropped result. ``connect_handler_ipc`` isolates the + connection on its own loop, so the ack still completes. Here we drive the real sink on the + caller's loop thread against a real in-proc server and assert SetResult actually arrived. + """ + pytest.importorskip("uipath_ipc") + captured = _fake_output_sinks(monkeypatch) + received: dict[str, Any] = {} + + class _Api(_job_api.IJobInvocationCommonApi): + async def SendLog(self, jobId: str, log: Any) -> None: + received.setdefault("logs", []).append((jobId, log)) + + async def SetResult(self, jobId: str, result: Any) -> bool: + # The server deserializes against the contract's typed signature, so result arrives as a + # real JobResultDto (this also exercises the wire round-trip of the DTO). + received["result"] = (jobId, result) + return True + + class _Result: + status = "successful" + error = None + + pipe = _unique_jobapi_pipe() + stop = _serve_jobapi_in_background(pipe, _Api()) + try: + + async def scenario() -> None: + conn = _job_api.connect_handler_ipc(pipe, "job-77") + # Call the sink ON this loop's thread, exactly as the runtime's __exit__ does. Before the + # fix this deadlocked the loop the ack was scheduled on; now it completes. + captured["sink"](_Result(), "out.args") + await _job_api.disconnect_handler_ipc(conn) + + asyncio.run(scenario()) + finally: + stop() + + assert "result" in received, ( + "SetResult never arrived — the result sink deadlocked/timed out" + ) + job_id, dto = received["result"] + assert job_id == "job-77" + assert dto.outputArgumentsFilePath == "out.args" + assert dto.status == _job_api.ExecutorJobStatus.SUCCESSFUL.value diff --git a/packages/uipath/tests/cli/test_server_ipc.py b/packages/uipath/tests/cli/test_server_ipc.py index 3f5a3a708..6a6dc25ad 100644 --- a/packages/uipath/tests/cli/test_server_ipc.py +++ b/packages/uipath/tests/cli/test_server_ipc.py @@ -17,7 +17,6 @@ import sys import threading import time -import types from typing import Any, Awaitable, Callable import click @@ -334,40 +333,11 @@ async def drive(proxy: Any) -> None: assert stop_request.ForceStop is True -class TestPooledLogSink: - """``Register`` wires the runtime's process-global log sink to the caller's callback. +class TestPooledSinks: + """Register grabs the handler's callback; RunJob installs the runtime sinks around each job.""" - This is the pooled path: the handler dials in and hosts an ``IIpcLogSink`` callback; the server - grabs it off the injected ``Message`` and points the runtime's sink at it, so a pooled job's logs - reach back over the same pipe. The runtime side is exercised in uipath-runtime's own tests; here - we lock in the seam (get_callback → set_pooled_log_sink → forward on the loop). - """ - - @staticmethod - def _fake_runtime_jobapi(monkeypatch: Any) -> dict[str, Any]: - """Install a stand-in ``uipath.runtime.jobapi`` and record what the server wires into it.""" - captured: dict[str, Any] = {} - - class IIpcLogSink: # matches the contract the server asks get_callback for - pass - - def set_pooled_log_sink(sink: Any) -> None: - captured["sink"] = sink - - module = types.ModuleType("uipath.runtime.jobapi") - module.IIpcLogSink = IIpcLogSink # type: ignore[attr-defined] - module.set_pooled_log_sink = set_pooled_log_sink # type: ignore[attr-defined] - monkeypatch.setitem(sys.modules, "uipath.runtime.jobapi", module) - captured["IIpcLogSink"] = IIpcLogSink - return captured - - def test_register_forwards_logs_to_the_caller_callback(self, monkeypatch): - captured = self._fake_runtime_jobapi(monkeypatch) - sent: list[tuple[str, Any]] = [] - - class _Callback: - async def SendLog(self, job_id: str, log: Any) -> None: - sent.append((job_id, log)) + def test_register_grabs_the_callback(self): + from uipath._cli import _job_api class _Client: def __init__(self) -> None: @@ -375,51 +345,101 @@ def __init__(self) -> None: def get_callback(self, contract: Any) -> Any: self.asked_for = contract - return _Callback() + return "CALLBACK" client = _Client() + service = PythonRuntimeService() async def scenario() -> None: - ok = await PythonRuntimeService().Register(Message(client=client)) - assert ok is True - # It asked the caller for exactly the IIpcLogSink callback... - assert client.asked_for is captured["IIpcLogSink"] - # ...and registered a forwarder. Driving it (as the runtime would, off a worker thread) - # schedules SendLog on this loop, tagged with the job id. - sink = captured["sink"] - log = {"Message": "hello", "LogLevel": 2} - sink("job-key-42", log) - await asyncio.sleep(0.05) - assert sent == [("job-key-42", log)] + assert await service.Register(Message(client=client)) is True asyncio.run(scenario()) + assert client.asked_for is _job_api.IJobInvocationCommonApi + assert service._callback == "CALLBACK" + assert service._loop is not None - def test_register_is_graceful_when_runtime_lacks_pooled_sink(self, monkeypatch): - # An older uipath-runtime has no jobapi module: importing it raises, and Register must still - # succeed (the job simply keeps its file+watcher log path). - monkeypatch.setitem(sys.modules, "uipath.runtime.jobapi", None) + def test_register_without_a_client_is_a_noop(self): + service = PythonRuntimeService() - class _Client: - def get_callback( - self, contract: Any - ) -> Any: # pragma: no cover - never reached - raise AssertionError( - "must not reach get_callback without the runtime API" - ) + async def scenario() -> None: + assert await service.Register(Message()) is True + + asyncio.run(scenario()) + assert service._callback is None + + def test_runjob_installs_then_clears_the_sinks(self, monkeypatch): + from uipath._cli import _job_api, cli_server_ipc + from uipath._cli.cli_server_ipc import PythonServerRunRequest + + events: list[tuple[Any, ...]] = [] + monkeypatch.setattr( + _job_api, + "install_runtime_sinks", + lambda jid, cb, loop: events.append(("install", jid, cb)), + ) + monkeypatch.setattr( + _job_api, "clear_runtime_sinks", lambda: events.append(("clear",)) + ) + + async def _fake_run(cmd, args, env, wd, on_run_start=None, on_run_end=None): + # The real core runs the hooks inside its lock, around the job; mirror that here. + if on_run_start: + on_run_start() + events.append(("run",)) + if on_run_end: + on_run_end() + return {"ExitCode": 0, "Error": None} + + monkeypatch.setattr(cli_server_ipc, "_run_command_isolated", _fake_run) + + service = PythonRuntimeService() + service._callback = "CALLBACK" + service._loop = asyncio.new_event_loop() + request = PythonServerRunRequest( + JobKey="job-9", Command="run", Args=[], StreamOutputOverIpc=True + ) async def scenario() -> None: - assert ( - await PythonRuntimeService().Register(Message(client=_Client())) is True - ) + await service.RunJob(request) asyncio.run(scenario()) + service._loop.close() + assert events == [("install", "job-9", "CALLBACK"), ("run",), ("clear",)] + + def test_runjob_skips_sinks_when_not_opted_in(self, monkeypatch): + from uipath._cli import _job_api, cli_server_ipc + from uipath._cli.cli_server_ipc import PythonServerRunRequest + + events: list[tuple[Any, ...]] = [] + monkeypatch.setattr( + _job_api, + "install_runtime_sinks", + lambda jid, cb, loop: events.append(("install",)), + ) + monkeypatch.setattr( + _job_api, "clear_runtime_sinks", lambda: events.append(("clear",)) + ) + + async def _fake_run(cmd, args, env, wd, on_run_start=None, on_run_end=None): + # The real core runs the hooks inside its lock, around the job; mirror that here. + if on_run_start: + on_run_start() + events.append(("run",)) + if on_run_end: + on_run_end() + return {"ExitCode": 0, "Error": None} + + monkeypatch.setattr(cli_server_ipc, "_run_command_isolated", _fake_run) - def test_register_without_a_caller_handle_is_a_noop(self, monkeypatch): - # A Message with no client (defensive; real dispatch always injects one) wires nothing. - captured = self._fake_runtime_jobapi(monkeypatch) + service = PythonRuntimeService() + service._callback = "CALLBACK" + service._loop = asyncio.new_event_loop() + # StreamOutputOverIpc defaults False -> the handler did not opt this job in. + request = PythonServerRunRequest(JobKey="job-9", Command="run", Args=[]) async def scenario() -> None: - assert await PythonRuntimeService().Register(Message()) is True + await service.RunJob(request) asyncio.run(scenario()) - assert "sink" not in captured + service._loop.close() + assert events == [("run",)] # no install / clear From 1efc68a1f0e2ee9ef282b225a33a19cbb305e686 Mon Sep 17 00:00:00 2001 From: Eduard Dumitru Date: Thu, 10 Sep 2026 12:04:25 +0200 Subject: [PATCH 03/18] build(deps): require uipath-runtime>=0.13.5 for the output sinks API The IPC log/result sinks this CLI installs live in uipath.runtime.output_sinks, which first shipped in uipath-runtime 0.13.5. The old >=0.13.1 floor (and a lockfile pinned to 0.13.1) would resolve a runtime without that module, silently no-opping the feature via the ImportError fallback. Bump the floor and relock to 0.13.5. Co-Authored-By: Claude Opus 4.8 (1M context) --- packages/uipath/pyproject.toml | 4 ++-- packages/uipath/uv.lock | 10 +++++----- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/packages/uipath/pyproject.toml b/packages/uipath/pyproject.toml index 6122cd06b..e31f9d05c 100644 --- a/packages/uipath/pyproject.toml +++ b/packages/uipath/pyproject.toml @@ -1,12 +1,12 @@ [project] name = "uipath" -version = "2.14.16" +version = "2.14.17" description = "Python SDK and CLI for UiPath Platform, enabling programmatic interaction with automation services, process management, and deployment tools." readme = { file = "README.md", content-type = "text/markdown" } requires-python = ">=3.11" dependencies = [ "uipath-core>=0.5.30, <0.6.0", - "uipath-runtime>=0.13.1, <0.14.0", + "uipath-runtime>=0.13.5, <0.14.0", "uipath-platform>=0.2.27, <0.3.0", "click>=8.3.3, <9.0.0", "httpx>=0.28.1", diff --git a/packages/uipath/uv.lock b/packages/uipath/uv.lock index 98877ec72..4483eaf38 100644 --- a/packages/uipath/uv.lock +++ b/packages/uipath/uv.lock @@ -2599,7 +2599,7 @@ wheels = [ [[package]] name = "uipath" -version = "2.14.16" +version = "2.14.17" source = { editable = "." } dependencies = [ { name = "applicationinsights" }, @@ -2683,7 +2683,7 @@ requires-dist = [ { name = "uipath-core", editable = "../uipath-core" }, { name = "uipath-ipc", marker = "extra == 'ipc'", specifier = ">=2.5.1,<2.6.0" }, { name = "uipath-platform", editable = "../uipath-platform" }, - { name = "uipath-runtime", specifier = ">=0.13.1,<0.14.0" }, + { name = "uipath-runtime", specifier = ">=0.13.5,<0.14.0" }, ] provides-extras = ["ipc"] @@ -2802,16 +2802,16 @@ dev = [ [[package]] name = "uipath-runtime" -version = "0.13.1" +version = "0.13.5" source = { registry = "https://pypi.org/simple" } dependencies = [ { name = "chardet" }, { name = "uipath-core" }, { name = "vadersentiment" }, ] -sdist = { url = "https://files.pythonhosted.org/packages/15/42/837240fd4cb2824ee5899cd2e06352c617645077efcecff7081a7292464a/uipath_runtime-0.13.1.tar.gz", hash = "sha256:4fd51d95e8b3c8554de8148239425499bbb7ec8687ad93a38f5107c6cdc0d7dd", size = 246257, upload-time = "2026-08-12T12:48:20.386Z" } +sdist = { url = "https://files.pythonhosted.org/packages/c8/8f/14ff9ccf152b6d97c0881f6b40d3e01037045ee52c467ed00dbb2319a0cb/uipath_runtime-0.13.5.tar.gz", hash = "sha256:923e4514f3d868f3711211fee6e2dd39f76c6aaf7354fec9e80cdc5e6fad5d80", size = 250415, upload-time = "2026-09-10T09:32:53.126Z" } wheels = [ - { url = "https://files.pythonhosted.org/packages/84/de/94e1ebcd7f25ee7620cb712f376cd84349d5d4b599bdbc6298556310598f/uipath_runtime-0.13.1-py3-none-any.whl", hash = "sha256:365134dfdc87050636b9027b839bbd93636ee56bd463bb38c1e48b7e19e848dd", size = 96876, upload-time = "2026-08-12T12:48:19.014Z" }, + { url = "https://files.pythonhosted.org/packages/95/88/b851e75b3c70bbedde1e24d5b5f38035863ed3d50e8facb75ac7d2db40b8/uipath_runtime-0.13.5-py3-none-any.whl", hash = "sha256:d5eb7d578689f67d5dd631d28a092954c9a8797e130e16fe46e2895e4b0656ac", size = 98513, upload-time = "2026-09-10T09:32:51.404Z" }, ] [[package]] From 896cd8eef272baab2130ab9b13c8a3c66f2767d3 Mon Sep 17 00:00:00 2001 From: Eduard Dumitru Date: Fri, 11 Sep 2026 12:53:32 +0200 Subject: [PATCH 04/18] test(cli): exercise the real UiPathRuntimeStatus in result-status mapping MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The _to_result_dto tests set the fake result's status to bare strings ("faulted"/"successful"), but the runtime hands a UiPathRuntimeStatus (str, Enum) member, where str(member) is "UiPathRuntimeStatus.FAULTED", not "faulted". Only the getattr(status, "value", status) extraction normalizes it — and the bare-string fakes never took that path, so dropping it would report a faulted/suspended job as SUCCESSFUL while every test stayed green. Use real UiPathRuntimeStatus members in the fakes and add the SUSPENDED case (previously untested), so the mapping is guarded against that regression. --- packages/uipath/tests/cli/test_job_api.py | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/packages/uipath/tests/cli/test_job_api.py b/packages/uipath/tests/cli/test_job_api.py index 0317f0337..7110babd9 100644 --- a/packages/uipath/tests/cli/test_job_api.py +++ b/packages/uipath/tests/cli/test_job_api.py @@ -16,6 +16,7 @@ import pytest from uipath._cli import _job_api +from uipath.runtime.result import UiPathRuntimeStatus def test_to_result_dto_maps_status_error_and_path(): @@ -30,7 +31,7 @@ class _Error: status = None class _Result: - status = "faulted" + status = UiPathRuntimeStatus.FAULTED error = _Error() dto = _job_api._to_result_dto("job-1", _Result(), "out.args") @@ -45,7 +46,7 @@ class _Result: def test_to_result_dto_defaults_to_successful_without_error(): class _Result: - status = "successful" + status = UiPathRuntimeStatus.SUCCESSFUL error = None dto = _job_api._to_result_dto("j", _Result(), "p.args") @@ -53,6 +54,15 @@ class _Result: assert dto.error is None +def test_to_result_dto_maps_suspended(): + class _Result: + status = UiPathRuntimeStatus.SUSPENDED + error = None + + dto = _job_api._to_result_dto("j", _Result(), "p.args") + assert dto.status == _job_api.ExecutorJobStatus.SUSPENDED.value + + def test_to_log_level_maps_python_levels_to_wire_values(): assert _job_api._to_log_level(logging.CRITICAL) == _job_api.LogLevel.CRITICAL assert _job_api._to_log_level(logging.ERROR) == _job_api.LogLevel.ERROR @@ -132,7 +142,7 @@ async def scenario() -> None: # The result sink maps the result and calls SetResult, off a worker thread, for the ack. class _Result: - status = "successful" + status = UiPathRuntimeStatus.SUCCESSFUL error = None sink = captured["sink"] @@ -257,7 +267,7 @@ async def SetResult(self, jobId: str, result: Any) -> bool: return True class _Result: - status = "successful" + status = UiPathRuntimeStatus.SUCCESSFUL error = None pipe = _unique_jobapi_pipe() From f95ce219ce66e104a1031fd83052438d591362b0 Mon Sep 17 00:00:00 2001 From: Eduard Dumitru Date: Fri, 11 Sep 2026 14:32:11 +0200 Subject: [PATCH 05/18] test(cli): cover the best-effort swallow paths for logs, result, and Register Nothing exercised the failure guarantees the design forbids violating: a SendLog forward that fails must not escape emit(), a SetResult failure must be logged and not raised (else the job faults and output.json is clobbered), and a get_callback that raises at Register must leave the callback None rather than break registration. Add a test for each. Verified by mutation: making any of the three except blocks re-raise fails the matching test. --- packages/uipath/tests/cli/test_job_api.py | 47 ++++++++++++++++++++ packages/uipath/tests/cli/test_server_ipc.py | 15 +++++++ 2 files changed, 62 insertions(+) diff --git a/packages/uipath/tests/cli/test_job_api.py b/packages/uipath/tests/cli/test_job_api.py index 7110babd9..3638014f5 100644 --- a/packages/uipath/tests/cli/test_job_api.py +++ b/packages/uipath/tests/cli/test_job_api.py @@ -153,6 +153,53 @@ class _Result: asyncio.run(scenario()) +def test_log_forwarding_failure_does_not_escape_emit(monkeypatch): + # A forward failure must not raise out of emit() — the logging framework calls it, so an escape + # would break the job's own logging. emit routes to handleError instead. (SendLog raising + # synchronously stands in for any forward failure.) + captured = _fake_output_sinks(monkeypatch) + + class _Callback: + def SendLog(self, jid: str, dto: Any) -> None: + raise RuntimeError("pipe is gone") + + loop = asyncio.new_event_loop() + try: + _job_api.install_runtime_sinks("job-8", _Callback(), loop) + handler = captured["handler"] + handled: list[Any] = [] + monkeypatch.setattr(handler, "handleError", handled.append) + handler.emit(logging.LogRecord("n", logging.INFO, "p", 1, "hi", (), None)) + assert len(handled) == 1 + finally: + loop.close() + + +def test_result_delivery_failure_is_swallowed_and_logged(monkeypatch, caplog): + # A SetResult failure is a side channel: it must be logged, not raised — an escape would fault + # the job and clobber the already-written output.json. + captured = _fake_output_sinks(monkeypatch) + + class _Callback: + async def SetResult(self, jid: str, dto: Any) -> bool: + raise RuntimeError("handler said no") + + class _Result: + status = UiPathRuntimeStatus.SUCCESSFUL + error = None + + async def scenario() -> None: + loop = asyncio.get_running_loop() + _job_api.install_runtime_sinks("job-9", _Callback(), loop) + sink = captured["sink"] + # The runtime calls the sink synchronously on a worker thread; nothing may escape it. + await asyncio.to_thread(sink, _Result(), "out.args") + + with caplog.at_level(logging.ERROR, logger="uipath._cli._job_api"): + asyncio.run(scenario()) + assert "Failed to deliver job result over IPC" in caplog.text + + def test_install_is_a_noop_without_the_runtime(monkeypatch): # An older uipath-runtime has no output_sinks module: install/clear must not raise. monkeypatch.setitem(sys.modules, "uipath.runtime.output_sinks", None) diff --git a/packages/uipath/tests/cli/test_server_ipc.py b/packages/uipath/tests/cli/test_server_ipc.py index 6a6dc25ad..09c179bef 100644 --- a/packages/uipath/tests/cli/test_server_ipc.py +++ b/packages/uipath/tests/cli/test_server_ipc.py @@ -367,6 +367,21 @@ async def scenario() -> None: asyncio.run(scenario()) assert service._callback is None + def test_register_swallows_a_failing_get_callback(self): + # A peer that can't hand back a callback must not break registration; the job just keeps + # to the file path (callback stays None). + class _Client: + def get_callback(self, contract: Any) -> Any: + raise RuntimeError("no callback available") + + service = PythonRuntimeService() + + async def scenario() -> None: + assert await service.Register(Message(client=_Client())) is True + + asyncio.run(scenario()) + assert service._callback is None + def test_runjob_installs_then_clears_the_sinks(self, monkeypatch): from uipath._cli import _job_api, cli_server_ipc from uipath._cli.cli_server_ipc import PythonServerRunRequest From d36f236103ed00d013fcdad7c81b26b60db4b394 Mon Sep 17 00:00:00 2001 From: Eduard Dumitru Date: Fri, 11 Sep 2026 14:50:07 +0200 Subject: [PATCH 06/18] test(cli): pin the pooled sink hooks against the real job core The IPC pooled-sink tests fake _run_command_isolated, so the hook-placement guarantees were only asserted against the fake. Drive real on_run_start/on_run_end spies through the real core and pin: on_run_end fires in the finally even when the job raises (SystemExit/Exception), on_run_start runs only after env/cwd are applied, and neither hook runs on the bad-working-dir early return. Verified by mutation: relocating either hook fails these tests. --- .../uipath/tests/cli/test_server_job_core.py | 69 +++++++++++++++++++ 1 file changed, 69 insertions(+) diff --git a/packages/uipath/tests/cli/test_server_job_core.py b/packages/uipath/tests/cli/test_server_job_core.py index 1722d0c5b..955099d2a 100644 --- a/packages/uipath/tests/cli/test_server_job_core.py +++ b/packages/uipath/tests/cli/test_server_job_core.py @@ -68,6 +68,75 @@ async def test_reports_unexpected_exception(restore_state: Any) -> None: assert "boom" in result["Error"] +# The pooled channel installs a per-job sink via on_run_start and clears it via +# on_run_end. The IPC layer's own tests fake this core, so the placement guarantees +# below (clear-on-raise, install-after-cwd, no hooks on the bad-cwd early return) +# are pinned here against the real core. + + +@pytest.mark.parametrize("boom", [SystemExit(2), ValueError("kaboom")]) +async def test_on_run_end_fires_even_when_the_job_raises( + restore_state: Any, boom: BaseException +) -> None: + _init(restore_state) + events: list[str] = [] + cmd = Mock() + cmd.main.side_effect = boom + await _server_core._run_command_isolated( + cmd, + [], + {}, + None, + on_run_start=lambda: events.append("start"), + on_run_end=lambda: events.append("end"), + ) + # on_run_end is in a finally, so a raising job still clears its sink before the + # next job runs — otherwise a stale sink would cross-wire the next job. + assert events == ["start", "end"] + + +async def test_on_run_start_sees_the_jobs_cwd_and_env( + restore_state: Any, tmp_path: Any +) -> None: + _init(restore_state) + seen: dict[str, Any] = {} + cmd = Mock() + + def _start() -> None: + seen["cwd"] = os.getcwd() + seen["env"] = os.environ.get("JOB_VAR") + + await _server_core._run_command_isolated( + cmd, + [], + {"JOB_VAR": "42"}, + str(tmp_path), + on_run_start=_start, + on_run_end=lambda: None, + ) + # The hook runs after env/cwd are applied, so it sees exactly what the job sees. + assert os.path.samefile(seen["cwd"], str(tmp_path)) + assert seen["env"] == "42" + + +async def test_hooks_do_not_run_on_bad_working_dir( + restore_state: Any, tmp_path: Any +) -> None: + _init(restore_state) + events: list[str] = [] + result = await _server_core._run_command_isolated( + Mock(), + [], + {}, + str(tmp_path / "does-not-exist"), + on_run_start=lambda: events.append("start"), + on_run_end=lambda: events.append("end"), + ) + assert result["ClientError"] is True + # The early return sits above on_run_start: no install means no clear owed. + assert events == [] + + # parse_args accepts what every caller sends: the .NET peer sends a single # string (shlex-split), HTTP dicts / tests may send a pre-split list, or None. From 3c7117287cf3b31a4a55f49de42554e3de2a03df Mon Sep 17 00:00:00 2001 From: Eduard Dumitru Date: Mon, 14 Sep 2026 10:07:50 +0200 Subject: [PATCH 07/18] fix(cli): derive the pooled IPC callback per request instead of at Register MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Register captured the caller's callback on the singleton service, but message.client is the per-accept connection and hdens only calls Register from Initialize(). After a pipe reconnect the stored callback proxied a closed connection, and after a Python server restart it was None — so every opted-in job for the rest of the pod's life either streamed into a dead connection or skipped streaming. With execution.log no longer watched for those jobs, their logs vanished with no signal. RunJob now takes the injected Message and derives the callback plus the running loop from that request's own connection; Register is a readiness probe and the service holds no connection state. Also: - Fail loud when a job is opted in but no callback is available, rather than running it with file sinks nobody reads. - Drop clear_runtime_sinks() from start_ipc_server's finally: uipath-ipc dispatches each request in its own task, so the sinks are per-request contextvars and clearing them from the server task was a no-op. - Remove a type: ignore that the uipath-runtime 0.13.5 bump made unused. - Capture the result-delivery log on the module logger directly; through caplog it was flaky, since other tests in the suite mutate propagation. --- packages/uipath/src/uipath/_cli/_job_api.py | 2 +- .../uipath/src/uipath/_cli/cli_server_ipc.py | 56 ++++++------- packages/uipath/tests/cli/test_job_api.py | 43 +++++++--- packages/uipath/tests/cli/test_server_ipc.py | 83 ++++++++++--------- 4 files changed, 99 insertions(+), 85 deletions(-) diff --git a/packages/uipath/src/uipath/_cli/_job_api.py b/packages/uipath/src/uipath/_cli/_job_api.py index 21666c1f7..888837328 100644 --- a/packages/uipath/src/uipath/_cli/_job_api.py +++ b/packages/uipath/src/uipath/_cli/_job_api.py @@ -167,7 +167,7 @@ def install_runtime_sinks( deadlocks. No-op if the sinks aren't available. """ try: - from uipath.runtime.output_sinks import ( # type: ignore[import-untyped] + from uipath.runtime.output_sinks import ( set_log_handler, set_result_sink, ) diff --git a/packages/uipath/src/uipath/_cli/cli_server_ipc.py b/packages/uipath/src/uipath/_cli/cli_server_ipc.py index 8e0442200..7f6b613a3 100644 --- a/packages/uipath/src/uipath/_cli/cli_server_ipc.py +++ b/packages/uipath/src/uipath/_cli/cli_server_ipc.py @@ -71,27 +71,13 @@ async def StopJob(self, request: PythonServerStopJobRequest) -> bool: class PythonRuntimeService(IPythonRuntimeServer): """``IPythonRuntimeServer`` implementation backed by run/debug/eval.""" - def __init__(self) -> None: - # The caller's callback, grabbed at Register (None until then). - self._callback: Any = None - self._loop: "asyncio.AbstractEventLoop | None" = None - async def Register(self, message: "Message[None]") -> bool: - client = message.client - if client is not None: - try: - from ._job_api import IJobInvocationCommonApi - - self._callback = client.get_callback(IJobInvocationCommonApi) # type: ignore[type-abstract] - self._loop = asyncio.get_running_loop() - except Exception: - self._callback = ( - None # older runtime / no callback: jobs keep the file path - ) console.info("Runtime client registered.") return True - async def RunJob(self, request: PythonServerRunRequest) -> PythonServerRunJobResult: + async def RunJob( + self, request: PythonServerRunRequest, *, message: "Message[None] | None" = None + ) -> PythonServerRunJobResult: command_name = request.Command if not isinstance(command_name, str) or not command_name: return PythonServerRunJobResult( @@ -110,14 +96,30 @@ async def RunJob(self, request: PythonServerRunRequest) -> PythonServerRunJobRes f"Running job {_run_id(request.JobKey, request.ResumeVersion)}: {command_name} {args}" ) - # Only when opted in and a callback exists. Install/clear the sinks INSIDE the job core's lock - # (via the hooks) so they're bound only while THIS job runs. - callback, loop = self._callback, self._loop on_run_start: "Any" = None on_run_end: "Any" = None - if request.StreamOutputOverIpc and callback is not None and loop is not None: - from ._job_api import clear_runtime_sinks, install_runtime_sinks + if request.StreamOutputOverIpc: + # Derived per request, never stored: a captured callback goes stale on reconnect/restart. + from ._job_api import ( + IJobInvocationCommonApi, + clear_runtime_sinks, + install_runtime_sinks, + ) + callback = None + client = message.client if message is not None else None + if client is not None: + try: + callback = client.get_callback(IJobInvocationCommonApi) # type: ignore[type-abstract] + except Exception: + callback = None + if callback is None: + return PythonServerRunJobResult( + ExitCode=1, + Error="StreamOutputOverIpc was requested but no IPC callback is available", + ) + + loop = asyncio.get_running_loop() job_key = request.JobKey on_run_start = lambda: install_runtime_sinks(job_key, callback, loop) # noqa: E731 on_run_end = clear_runtime_sinks @@ -168,11 +170,5 @@ async def start_ipc_server(pipe_name: str) -> None: request_timeout=None, # jobs are long-running; no server-side timeout ) console.success(f"IPC server listening on pipe '{pipe_name}'") - try: - async with server: - await server.serve_forever() - finally: - # Drop the sinks so they can't outlive this loop (matters on in-process restart). - from ._job_api import clear_runtime_sinks - - clear_runtime_sinks() + async with server: + await server.serve_forever() diff --git a/packages/uipath/tests/cli/test_job_api.py b/packages/uipath/tests/cli/test_job_api.py index 3638014f5..4b2db55d4 100644 --- a/packages/uipath/tests/cli/test_job_api.py +++ b/packages/uipath/tests/cli/test_job_api.py @@ -154,9 +154,6 @@ class _Result: def test_log_forwarding_failure_does_not_escape_emit(monkeypatch): - # A forward failure must not raise out of emit() — the logging framework calls it, so an escape - # would break the job's own logging. emit routes to handleError instead. (SendLog raising - # synchronously stands in for any forward failure.) captured = _fake_output_sinks(monkeypatch) class _Callback: @@ -175,9 +172,7 @@ def SendLog(self, jid: str, dto: Any) -> None: loop.close() -def test_result_delivery_failure_is_swallowed_and_logged(monkeypatch, caplog): - # A SetResult failure is a side channel: it must be logged, not raised — an escape would fault - # the job and clobber the already-written output.json. +def test_result_delivery_failure_is_swallowed_and_logged(monkeypatch): captured = _fake_output_sinks(monkeypatch) class _Callback: @@ -188,16 +183,38 @@ class _Result: status = UiPathRuntimeStatus.SUCCESSFUL error = None - async def scenario() -> None: - loop = asyncio.get_running_loop() + # Capture on the module logger directly, not via caplog: other tests in the suite mutate root + # handlers / propagate, which would silently drop the record from caplog. + records: list[logging.LogRecord] = [] + + class _Capture(logging.Handler): + def emit(self, record: logging.LogRecord) -> None: + records.append(record) + + api_logger = logging.getLogger("uipath._cli._job_api") + handler = _Capture() + api_logger.addHandler(handler) + prior_level = api_logger.level + api_logger.setLevel(logging.ERROR) + + # Ack loop on its own thread so the cross-thread wait is deterministic. + loop = asyncio.new_event_loop() + thread = threading.Thread(target=loop.run_forever, daemon=True) + thread.start() + try: _job_api.install_runtime_sinks("job-9", _Callback(), loop) sink = captured["sink"] - # The runtime calls the sink synchronously on a worker thread; nothing may escape it. - await asyncio.to_thread(sink, _Result(), "out.args") + sink(_Result(), "out.args") # must not raise + finally: + loop.call_soon_threadsafe(loop.stop) + thread.join(timeout=5) + loop.close() + api_logger.removeHandler(handler) + api_logger.setLevel(prior_level) - with caplog.at_level(logging.ERROR, logger="uipath._cli._job_api"): - asyncio.run(scenario()) - assert "Failed to deliver job result over IPC" in caplog.text + assert any( + "Failed to deliver job result over IPC" in r.getMessage() for r in records + ) def test_install_is_a_noop_without_the_runtime(monkeypatch): diff --git a/packages/uipath/tests/cli/test_server_ipc.py b/packages/uipath/tests/cli/test_server_ipc.py index 09c179bef..cc3844f52 100644 --- a/packages/uipath/tests/cli/test_server_ipc.py +++ b/packages/uipath/tests/cli/test_server_ipc.py @@ -334,29 +334,15 @@ async def drive(proxy: Any) -> None: class TestPooledSinks: - """Register grabs the handler's callback; RunJob installs the runtime sinks around each job.""" + """Register is a readiness probe; RunJob derives the callback per request and installs sinks.""" - def test_register_grabs_the_callback(self): - from uipath._cli import _job_api - - class _Client: - def __init__(self) -> None: - self.asked_for: Any = None - - def get_callback(self, contract: Any) -> Any: - self.asked_for = contract - return "CALLBACK" - - client = _Client() + def test_register_is_a_readiness_probe(self): service = PythonRuntimeService() async def scenario() -> None: - assert await service.Register(Message(client=client)) is True + assert await service.Register(Message(client=object())) is True asyncio.run(scenario()) - assert client.asked_for is _job_api.IJobInvocationCommonApi - assert service._callback == "CALLBACK" - assert service._loop is not None def test_register_without_a_client_is_a_noop(self): service = PythonRuntimeService() @@ -365,24 +351,44 @@ async def scenario() -> None: assert await service.Register(Message()) is True asyncio.run(scenario()) - assert service._callback is None - def test_register_swallows_a_failing_get_callback(self): - # A peer that can't hand back a callback must not break registration; the job just keeps - # to the file path (callback stays None). - class _Client: - def get_callback(self, contract: Any) -> Any: - raise RuntimeError("no callback available") + @pytest.mark.parametrize("mode", ["no-client", "get_callback-raises"]) + def test_runjob_fails_loud_when_opted_in_but_no_callback(self, monkeypatch, mode): + from uipath._cli import cli_server_ipc + from uipath._cli.cli_server_ipc import PythonServerRunRequest + + ran: list[bool] = [] + + async def _fake_run(*args: Any, **kwargs: Any) -> dict[str, Any]: + ran.append(True) + return {"ExitCode": 0, "Error": None} + + monkeypatch.setattr(cli_server_ipc, "_run_command_isolated", _fake_run) + + if mode == "no-client": + message = Message() + else: + + class _Client: + def get_callback(self, contract: Any) -> Any: + raise RuntimeError("no callback available") + + message = Message(client=_Client()) service = PythonRuntimeService() + request = PythonServerRunRequest( + JobKey="job-9", Command="run", Args=[], StreamOutputOverIpc=True + ) - async def scenario() -> None: - assert await service.Register(Message(client=_Client())) is True + async def scenario() -> Any: + return await service.RunJob(request, message=message) - asyncio.run(scenario()) - assert service._callback is None + result = asyncio.run(scenario()) + assert result.ExitCode == 1 + assert "callback" in (result.Error or "").lower() + assert ran == [] - def test_runjob_installs_then_clears_the_sinks(self, monkeypatch): + def test_runjob_installs_the_sinks_from_the_request_callback(self, monkeypatch): from uipath._cli import _job_api, cli_server_ipc from uipath._cli.cli_server_ipc import PythonServerRunRequest @@ -397,7 +403,6 @@ def test_runjob_installs_then_clears_the_sinks(self, monkeypatch): ) async def _fake_run(cmd, args, env, wd, on_run_start=None, on_run_end=None): - # The real core runs the hooks inside its lock, around the job; mirror that here. if on_run_start: on_run_start() events.append(("run",)) @@ -407,18 +412,19 @@ async def _fake_run(cmd, args, env, wd, on_run_start=None, on_run_end=None): monkeypatch.setattr(cli_server_ipc, "_run_command_isolated", _fake_run) + class _Client: + def get_callback(self, contract: Any) -> Any: + return "CALLBACK" + service = PythonRuntimeService() - service._callback = "CALLBACK" - service._loop = asyncio.new_event_loop() request = PythonServerRunRequest( JobKey="job-9", Command="run", Args=[], StreamOutputOverIpc=True ) async def scenario() -> None: - await service.RunJob(request) + await service.RunJob(request, message=Message(client=_Client())) asyncio.run(scenario()) - service._loop.close() assert events == [("install", "job-9", "CALLBACK"), ("run",), ("clear",)] def test_runjob_skips_sinks_when_not_opted_in(self, monkeypatch): @@ -436,7 +442,6 @@ def test_runjob_skips_sinks_when_not_opted_in(self, monkeypatch): ) async def _fake_run(cmd, args, env, wd, on_run_start=None, on_run_end=None): - # The real core runs the hooks inside its lock, around the job; mirror that here. if on_run_start: on_run_start() events.append(("run",)) @@ -447,14 +452,10 @@ async def _fake_run(cmd, args, env, wd, on_run_start=None, on_run_end=None): monkeypatch.setattr(cli_server_ipc, "_run_command_isolated", _fake_run) service = PythonRuntimeService() - service._callback = "CALLBACK" - service._loop = asyncio.new_event_loop() - # StreamOutputOverIpc defaults False -> the handler did not opt this job in. request = PythonServerRunRequest(JobKey="job-9", Command="run", Args=[]) async def scenario() -> None: - await service.RunJob(request) + await service.RunJob(request, message=Message(client=None)) asyncio.run(scenario()) - service._loop.close() - assert events == [("run",)] # no install / clear + assert events == [("run",)] From f27a921ca4e8ef26f5c3333aef95e8c779ac701d Mon Sep 17 00:00:00 2001 From: Eduard Dumitru Date: Mon, 14 Sep 2026 10:28:30 +0200 Subject: [PATCH 08/18] fix(cli): keep the IPC transport's own logs off the IPC channel MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The runtime interceptor attaches the job log handler to the root logger, so uipath_ipc's own diagnostics ("dropping malformed frame", "receive loop failed", ...) became JobLogDto entries in the customer's job output — and each one scheduled another SendLog on the very transport whose failure it was reporting. Route records from uipath_ipc.* and this module to the original stderr instead of the IPC channel, so they stay visible in the pod without riding the channel they describe. Also correct the Register docstring: it still claimed to grab the caller's callback, which moved to RunJob. --- packages/uipath/src/uipath/_cli/_job_api.py | 17 ++++++++ .../uipath/src/uipath/_cli/cli_server_ipc.py | 2 +- packages/uipath/tests/cli/test_job_api.py | 39 +++++++++++++++++++ 3 files changed, 57 insertions(+), 1 deletion(-) diff --git a/packages/uipath/src/uipath/_cli/_job_api.py b/packages/uipath/src/uipath/_cli/_job_api.py index 888837328..79210f4e6 100644 --- a/packages/uipath/src/uipath/_cli/_job_api.py +++ b/packages/uipath/src/uipath/_cli/_job_api.py @@ -18,6 +18,9 @@ _SET_RESULT_TIMEOUT_S = 30.0 +# Loggers whose records must never ride the IPC channel: the transport itself and this module. +_NO_IPC_LOGGERS = ("uipath_ipc", __name__) + class LogLevel(IntEnum): """Log-level wire values.""" @@ -135,6 +138,17 @@ def _drain(future: "Future[object]") -> None: pass +def _to_original_stderr(handler: logging.Handler, record: logging.LogRecord) -> None: + stream = sys.__stderr__ + if stream is None: + return + try: + stream.write(handler.format(record) + "\n") + stream.flush() + except Exception: + pass + + class _IpcLogHandler(logging.Handler): """Forwards each log record to the callback.""" @@ -147,6 +161,9 @@ def __init__( self._loop = loop def emit(self, record: logging.LogRecord) -> None: + if record.name.startswith(_NO_IPC_LOGGERS): + _to_original_stderr(self, record) + return try: message = self.format(record) dto = JobLogDto(Message=message, LogLevel=_to_log_level(record.levelno)) diff --git a/packages/uipath/src/uipath/_cli/cli_server_ipc.py b/packages/uipath/src/uipath/_cli/cli_server_ipc.py index 7f6b613a3..0e5a8e8e7 100644 --- a/packages/uipath/src/uipath/_cli/cli_server_ipc.py +++ b/packages/uipath/src/uipath/_cli/cli_server_ipc.py @@ -57,7 +57,7 @@ class IPythonRuntimeServer(ABC): @abstractmethod async def Register(self, message: "Message[None]") -> bool: - """Prove the connection is up and grab the caller's callback via ``message.client.get_callback``.""" + """Prove the connection is up.""" @abstractmethod async def RunJob(self, request: PythonServerRunRequest) -> PythonServerRunJobResult: diff --git a/packages/uipath/tests/cli/test_job_api.py b/packages/uipath/tests/cli/test_job_api.py index 4b2db55d4..34facf408 100644 --- a/packages/uipath/tests/cli/test_job_api.py +++ b/packages/uipath/tests/cli/test_job_api.py @@ -6,6 +6,7 @@ """ import asyncio +import io import logging import os import sys @@ -172,6 +173,44 @@ def SendLog(self, jid: str, dto: Any) -> None: loop.close() +def test_transport_and_self_logs_never_ride_the_ipc_channel(monkeypatch): + captured = _fake_output_sinks(monkeypatch) + sent: list[Any] = [] + forwarded = threading.Event() + + class _Callback: + async def SendLog(self, jid: str, dto: Any) -> None: + sent.append((jid, dto)) + forwarded.set() + + buf = io.StringIO() + monkeypatch.setattr(sys, "__stderr__", buf) + + loop = asyncio.new_event_loop() + thread = threading.Thread(target=loop.run_forever, daemon=True) + thread.start() + try: + _job_api.install_runtime_sinks("job-6", _Callback(), loop) + handler = captured["handler"] + for name in ("uipath_ipc.client.connection", _job_api.__name__): + handler.emit( + logging.LogRecord( + name, logging.WARNING, "p", 1, "internal chatter", (), None + ) + ) + handler.emit( + logging.LogRecord("job.logger", logging.INFO, "p", 1, "job line", (), None) + ) + forwarded.wait(timeout=5) + finally: + loop.call_soon_threadsafe(loop.stop) + thread.join(timeout=5) + loop.close() + + assert [dto.Message for _, dto in sent] == ["job line"] + assert buf.getvalue().count("internal chatter") == 2 + + def test_result_delivery_failure_is_swallowed_and_logged(monkeypatch): captured = _fake_output_sinks(monkeypatch) From b1b3e4dec0807618eeb0951a8cf1c04445d8d0d3 Mon Sep 17 00:00:00 2001 From: Eduard Dumitru Date: Mon, 14 Sep 2026 10:43:32 +0200 Subject: [PATCH 09/18] fix(cli): drain in-flight log sends before teardown SendLog futures were fire-and-forget, so the tail of a job's log could be lost: non-pooled, aclose() failed whatever was still queued; pooled, the RunJob response was written straight after the job, letting the peer unregister it before the last frames arrived. Track pending sends on the handler and drain them, bounded, before teardown. The two paths need different drains: non-pooled blocks inside _shutdown (it already runs on a worker thread, and the sink loop has its own), while pooled must await in RunJob, because on_run_end runs on the very loop the sends need in order to complete. --- packages/uipath/src/uipath/_cli/_job_api.py | 45 ++++++++++++++++--- .../uipath/src/uipath/_cli/cli_server_ipc.py | 13 +++++- packages/uipath/tests/cli/test_job_api.py | 26 +++++++++++ packages/uipath/tests/cli/test_server_ipc.py | 40 +++++++++++++++++ 4 files changed, 118 insertions(+), 6 deletions(-) diff --git a/packages/uipath/src/uipath/_cli/_job_api.py b/packages/uipath/src/uipath/_cli/_job_api.py index 79210f4e6..7db1f539c 100644 --- a/packages/uipath/src/uipath/_cli/_job_api.py +++ b/packages/uipath/src/uipath/_cli/_job_api.py @@ -10,6 +10,7 @@ from abc import ABC, abstractmethod from collections.abc import AsyncIterator from concurrent.futures import Future +from concurrent.futures import wait as _futures_wait from dataclasses import dataclass from enum import IntEnum from typing import Any @@ -17,6 +18,7 @@ logger = logging.getLogger(__name__) _SET_RESULT_TIMEOUT_S = 30.0 +_LOG_FLUSH_TIMEOUT_S = 5.0 # Loggers whose records must never ride the IPC channel: the transport itself and this module. _NO_IPC_LOGGERS = ("uipath_ipc", __name__) @@ -159,6 +161,8 @@ def __init__( self._job_id = job_id self._callback = callback self._loop = loop + self._pending: set[Future[object]] = set() + self._pending_lock = threading.Lock() def emit(self, record: logging.LogRecord) -> None: if record.name.startswith(_NO_IPC_LOGGERS): @@ -170,14 +174,38 @@ def emit(self, record: logging.LogRecord) -> None: future = asyncio.run_coroutine_threadsafe( self._callback.SendLog(self._job_id, dto), self._loop ) - future.add_done_callback(_drain) + with self._pending_lock: + self._pending.add(future) + future.add_done_callback(self._settled) except Exception: self.handleError(record) + def _settled(self, future: "Future[object]") -> None: + with self._pending_lock: + self._pending.discard(future) + _drain(future) + + def _snapshot(self) -> "list[Future[object]]": + with self._pending_lock: + return list(self._pending) + + def flush_pending(self, timeout: float = _LOG_FLUSH_TIMEOUT_S) -> None: + # Blocks; only safe off the sink loop's own thread (see _HandlerIpcConnection._shutdown). + pending = self._snapshot() + if pending: + _futures_wait(pending, timeout=timeout) + + async def aflush_pending(self, timeout: float = _LOG_FLUSH_TIMEOUT_S) -> None: + # Awaits instead of blocking, so it is safe when the caller shares the sink's loop. + pending = self._snapshot() + if not pending: + return + await asyncio.wait([asyncio.wrap_future(f) for f in pending], timeout=timeout) + def install_runtime_sinks( job_id: str, callback: Any, loop: asyncio.AbstractEventLoop -) -> None: +) -> "_IpcLogHandler | None": """Install the log + result sinks, forwarding to ``callback`` on ``loop``. ``loop`` must run on a different thread than the one the sinks are invoked on, or the result ack @@ -189,7 +217,7 @@ def install_runtime_sinks( set_result_sink, ) except ImportError: - return + return None handler = _IpcLogHandler(job_id, callback, loop) handler.setFormatter(logging.Formatter("%(message)s")) @@ -207,6 +235,7 @@ def _result_sink(result: Any, output_arguments_file_path: str) -> None: set_log_handler(handler) set_result_sink(_result_sink) + return handler def clear_runtime_sinks() -> None: @@ -234,13 +263,19 @@ def __init__( client: Any, loop: asyncio.AbstractEventLoop, thread: threading.Thread, + log_handler: "_IpcLogHandler | None" = None, ) -> None: self._client = client self._loop = loop self._thread = thread + self._log_handler = log_handler def _shutdown(self) -> None: """Close the client and stop its loop/thread.""" + # Runs on a worker thread, so blocking here is safe: the sink loop has its own thread and + # keeps draining. aclose() would fail whatever is still queued, losing the log tail. + if self._log_handler is not None: + self._log_handler.flush_pending() try: asyncio.run_coroutine_threadsafe(self._client.aclose(), self._loop).result( timeout=_SET_RESULT_TIMEOUT_S @@ -284,8 +319,8 @@ async def _build() -> Any: raise # Install in the caller's context, not on the IPC thread: the sinks are contextvars, resolved in # the job's own context at teardown. The ack still runs on `loop` (a separate thread) — no deadlock. - install_runtime_sinks(job_id, proxy, loop) - return _HandlerIpcConnection(client, loop, thread) + handler = install_runtime_sinks(job_id, proxy, loop) + return _HandlerIpcConnection(client, loop, thread, handler) async def disconnect_handler_ipc(conn: _HandlerIpcConnection) -> None: diff --git a/packages/uipath/src/uipath/_cli/cli_server_ipc.py b/packages/uipath/src/uipath/_cli/cli_server_ipc.py index 0e5a8e8e7..e6fff09c6 100644 --- a/packages/uipath/src/uipath/_cli/cli_server_ipc.py +++ b/packages/uipath/src/uipath/_cli/cli_server_ipc.py @@ -98,6 +98,7 @@ async def RunJob( on_run_start: "Any" = None on_run_end: "Any" = None + installed: "list[Any]" = [] if request.StreamOutputOverIpc: # Derived per request, never stored: a captured callback goes stale on reconnect/restart. from ._job_api import ( @@ -121,7 +122,11 @@ async def RunJob( loop = asyncio.get_running_loop() job_key = request.JobKey - on_run_start = lambda: install_runtime_sinks(job_key, callback, loop) # noqa: E731 + + def _install() -> None: + installed.append(install_runtime_sinks(job_key, callback, loop)) + + on_run_start = _install on_run_end = clear_runtime_sinks result = await _run_command_isolated( @@ -133,6 +138,12 @@ async def RunJob( on_run_end=on_run_end, ) + # Drain in-flight sends by awaiting: they share this loop, and the peer may unregister the + # job the moment this response lands. + for handler in installed: + if handler is not None: + await handler.aflush_pending() + # IPC contract (PythonServerRunJobResult) carries only ExitCode + Error. return PythonServerRunJobResult( ExitCode=result["ExitCode"], Error=result["Error"] diff --git a/packages/uipath/tests/cli/test_job_api.py b/packages/uipath/tests/cli/test_job_api.py index 34facf408..a0ce080d1 100644 --- a/packages/uipath/tests/cli/test_job_api.py +++ b/packages/uipath/tests/cli/test_job_api.py @@ -211,6 +211,32 @@ async def SendLog(self, jid: str, dto: Any) -> None: assert buf.getvalue().count("internal chatter") == 2 +def test_pending_log_sends_are_flushed_before_teardown(monkeypatch): + _fake_output_sinks(monkeypatch) + landed: list[str] = [] + + class _Callback: + async def SendLog(self, jid: str, dto: Any) -> None: + await asyncio.sleep(0.2) + landed.append(dto.Message) + + class _Client: + async def aclose(self) -> None: + return None + + loop = asyncio.new_event_loop() + thread = threading.Thread(target=loop.run_forever, daemon=True) + thread.start() + handler = _job_api.install_runtime_sinks("job-7", _Callback(), loop) + assert handler is not None + handler.emit(logging.LogRecord("j", logging.INFO, "p", 1, "tail line", (), None)) + + conn = _job_api._HandlerIpcConnection(_Client(), loop, thread, handler) + conn._shutdown() + + assert landed == ["tail line"] + + def test_result_delivery_failure_is_swallowed_and_logged(monkeypatch): captured = _fake_output_sinks(monkeypatch) diff --git a/packages/uipath/tests/cli/test_server_ipc.py b/packages/uipath/tests/cli/test_server_ipc.py index cc3844f52..078b48c21 100644 --- a/packages/uipath/tests/cli/test_server_ipc.py +++ b/packages/uipath/tests/cli/test_server_ipc.py @@ -427,6 +427,46 @@ async def scenario() -> None: asyncio.run(scenario()) assert events == [("install", "job-9", "CALLBACK"), ("run",), ("clear",)] + def test_runjob_drains_pending_sends_before_returning(self, monkeypatch): + from uipath._cli import _job_api, cli_server_ipc + from uipath._cli.cli_server_ipc import PythonServerRunRequest + + events: list[str] = [] + + class _Handler: + async def aflush_pending(self, *a: Any, **k: Any) -> None: + events.append("flush") + + monkeypatch.setattr( + _job_api, "install_runtime_sinks", lambda jid, cb, loop: _Handler() + ) + monkeypatch.setattr(_job_api, "clear_runtime_sinks", lambda: None) + + async def _fake_run(cmd, args, env, wd, on_run_start=None, on_run_end=None): + if on_run_start: + on_run_start() + events.append("run") + if on_run_end: + on_run_end() + return {"ExitCode": 0, "Error": None} + + monkeypatch.setattr(cli_server_ipc, "_run_command_isolated", _fake_run) + + class _Client: + def get_callback(self, contract: Any) -> Any: + return "CALLBACK" + + service = PythonRuntimeService() + request = PythonServerRunRequest( + JobKey="job-7", Command="run", Args=[], StreamOutputOverIpc=True + ) + + async def scenario() -> None: + await service.RunJob(request, message=Message(client=_Client())) + + asyncio.run(scenario()) + assert events == ["run", "flush"] + def test_runjob_skips_sinks_when_not_opted_in(self, monkeypatch): from uipath._cli import _job_api, cli_server_ipc from uipath._cli.cli_server_ipc import PythonServerRunRequest From aaa1140088bb8fce3e2d039479a34b298bf3f602 Mon Sep 17 00:00:00 2001 From: Eduard Dumitru Date: Mon, 14 Sep 2026 10:48:30 +0200 Subject: [PATCH 10/18] fix(cli): surface a rejected result and make IPC teardown non-raising Two teardown-path defects on the job-api channel. SetResult's return value was discarded, but the handler returns false when it cannot route the job (unknown job key, or the executor threw). A rejected result was therefore treated as delivered and silently lost; it is now reported. The report reaches the original stderr rather than the channel that just rejected it. loop.close() also ran unconditionally after a timeout-bounded join. A wedged IPC thread leaves the loop running, so close() raised: inside the context manager's finally that masked the job's real exception (or turned a success into exit 1), and on the connect-failure path it pre-empted the bare raise, masking the original error. Teardown now lives in one non-raising helper used by both paths, which only closes the loop once its thread is gone. --- packages/uipath/src/uipath/_cli/_job_api.py | 31 +++++++--- packages/uipath/tests/cli/test_job_api.py | 65 +++++++++++++++++++++ 2 files changed, 89 insertions(+), 7 deletions(-) diff --git a/packages/uipath/src/uipath/_cli/_job_api.py b/packages/uipath/src/uipath/_cli/_job_api.py index 7db1f539c..f7684c0f5 100644 --- a/packages/uipath/src/uipath/_cli/_job_api.py +++ b/packages/uipath/src/uipath/_cli/_job_api.py @@ -228,7 +228,10 @@ def _result_sink(result: Any, output_arguments_file_path: str) -> None: future = asyncio.run_coroutine_threadsafe( callback.SetResult(job_id, dto), loop ) - future.result(timeout=_SET_RESULT_TIMEOUT_S) + if future.result(timeout=_SET_RESULT_TIMEOUT_S) is False: + logger.error( + "The handler rejected the job result (SetResult returned false)" + ) except Exception: # Best-effort — a dropped result must be logged, not swallowed. logger.exception("Failed to deliver job result over IPC (SetResult)") @@ -248,6 +251,24 @@ def clear_runtime_sinks() -> None: set_result_sink(None) +def _stop_loop_thread( + loop: asyncio.AbstractEventLoop, thread: threading.Thread, timeout: float +) -> None: + # Teardown must never raise: a wedged thread leaves the loop running, and close() would then + # raise and mask the job's own outcome (or the error that got us here). + try: + loop.call_soon_threadsafe(loop.stop) + except Exception: + pass + thread.join(timeout=timeout) + if thread.is_alive(): + return + try: + loop.close() + except Exception: + pass + + def _new_ipc_event_loop() -> asyncio.AbstractEventLoop: """A fresh event loop for the connection's thread (Proactor on Windows, required for named pipes).""" if sys.platform == "win32": @@ -282,9 +303,7 @@ def _shutdown(self) -> None: ) except Exception: pass - self._loop.call_soon_threadsafe(self._loop.stop) - self._thread.join(timeout=_SET_RESULT_TIMEOUT_S) - self._loop.close() + _stop_loop_thread(self._loop, self._thread, _SET_RESULT_TIMEOUT_S) def connect_handler_ipc(pipe: str, job_id: str) -> _HandlerIpcConnection: @@ -313,9 +332,7 @@ async def _build() -> Any: ) except BaseException: # On failure, don't leak the loop/thread. - loop.call_soon_threadsafe(loop.stop) - thread.join(timeout=_SET_RESULT_TIMEOUT_S) - loop.close() + _stop_loop_thread(loop, thread, _SET_RESULT_TIMEOUT_S) raise # Install in the caller's context, not on the IPC thread: the sinks are contextvars, resolved in # the job's own context at teardown. The ack still runs on `loop` (a separate thread) — no deadlock. diff --git a/packages/uipath/tests/cli/test_job_api.py b/packages/uipath/tests/cli/test_job_api.py index a0ce080d1..7552cd69d 100644 --- a/packages/uipath/tests/cli/test_job_api.py +++ b/packages/uipath/tests/cli/test_job_api.py @@ -211,6 +211,71 @@ async def SendLog(self, jid: str, dto: Any) -> None: assert buf.getvalue().count("internal chatter") == 2 +def test_rejected_result_is_reported(monkeypatch): + captured = _fake_output_sinks(monkeypatch) + + class _Callback: + async def SetResult(self, jid: str, dto: Any) -> bool: + return False + + class _Result: + status = UiPathRuntimeStatus.SUCCESSFUL + error = None + + records: list[logging.LogRecord] = [] + + class _Capture(logging.Handler): + def emit(self, record: logging.LogRecord) -> None: + records.append(record) + + api_logger = logging.getLogger("uipath._cli._job_api") + handler = _Capture() + api_logger.addHandler(handler) + prior_level = api_logger.level + api_logger.setLevel(logging.ERROR) + + loop = asyncio.new_event_loop() + thread = threading.Thread(target=loop.run_forever, daemon=True) + thread.start() + try: + _job_api.install_runtime_sinks("job-8", _Callback(), loop) + captured["sink"](_Result(), "out.args") + finally: + loop.call_soon_threadsafe(loop.stop) + thread.join(timeout=5) + loop.close() + api_logger.removeHandler(handler) + api_logger.setLevel(prior_level) + + assert any("rejected" in r.getMessage().lower() for r in records) + + +def test_teardown_does_not_raise_when_the_loop_thread_is_wedged(): + loop = asyncio.new_event_loop() + started = threading.Event() + release = threading.Event() + + def _run() -> None: + asyncio.set_event_loop(loop) + + async def _block() -> None: + started.set() + release.wait(timeout=10) + + loop.run_until_complete(_block()) + + thread = threading.Thread(target=_run, daemon=True) + thread.start() + assert started.wait(timeout=5) + + _job_api._stop_loop_thread(loop, thread, 0.2) + assert thread.is_alive() + + release.set() + thread.join(timeout=5) + loop.close() + + def test_pending_log_sends_are_flushed_before_teardown(monkeypatch): _fake_output_sinks(monkeypatch) landed: list[str] = [] From 99b9a03afe3a6ff3d0084ac7a0d8c570e2eb300c Mon Sep 17 00:00:00 2001 From: Eduard Dumitru Date: Mon, 14 Sep 2026 10:55:21 +0200 Subject: [PATCH 11/18] fix(cli): fail loudly when the handler IPC pipe is unreachable NamedPipeClientTransport dials on first use, so an unreachable pipe (a TMPDIR mismatch, a peer not yet bound, a typo) was not noticed at connect. By then the interceptor has already suppressed execution.log for the job, so every log line went nowhere while `uipath run` still exited 0 reporting a successful execution. Dial the transport eagerly and raise with the pipe name if it cannot be reached. Losing the job is better than running it with its entire log silently discarded. Also give IpcClient a request timeout: against a peer that accepts the connection and then stalls, each log line otherwise became an uncancellable task parked on the write lock for the life of the job. connect_handler_ipc is therefore no longer lazy - it now requires a reachable pipe, and its test stands up a real server instead of relying on the old behaviour. --- packages/uipath/src/uipath/_cli/_job_api.py | 13 +++++- packages/uipath/tests/cli/test_job_api.py | 51 +++++++++++++++++---- 2 files changed, 54 insertions(+), 10 deletions(-) diff --git a/packages/uipath/src/uipath/_cli/_job_api.py b/packages/uipath/src/uipath/_cli/_job_api.py index f7684c0f5..556aa441a 100644 --- a/packages/uipath/src/uipath/_cli/_job_api.py +++ b/packages/uipath/src/uipath/_cli/_job_api.py @@ -19,6 +19,7 @@ _SET_RESULT_TIMEOUT_S = 30.0 _LOG_FLUSH_TIMEOUT_S = 5.0 +_IPC_REQUEST_TIMEOUT_S = 30.0 # Loggers whose records must never ride the IPC channel: the transport itself and this module. _NO_IPC_LOGGERS = ("uipath_ipc", __name__) @@ -322,7 +323,17 @@ def connect_handler_ipc(pipe: str, job_id: str) -> _HandlerIpcConnection: thread.start() async def _build() -> Any: - client = IpcClient(transport=NamedPipeClientTransport(pipe)) + transport = NamedPipeClientTransport(pipe) + # The transport dials lazily, and the interceptor has already suppressed execution.log by + # the time the first record is sent: an unreachable pipe would lose every line in silence. + try: + _, writer = await transport.connect() + except Exception as e: + raise RuntimeError( + f"Could not reach the handler IPC pipe {pipe!r}: {e}" + ) from e + writer.close() + client = IpcClient(transport=transport, request_timeout=_IPC_REQUEST_TIMEOUT_S) proxy = client.get_proxy(IJobInvocationCommonApi) # type: ignore[type-abstract] return client, proxy diff --git a/packages/uipath/tests/cli/test_job_api.py b/packages/uipath/tests/cli/test_job_api.py index 7552cd69d..d427f13a2 100644 --- a/packages/uipath/tests/cli/test_job_api.py +++ b/packages/uipath/tests/cli/test_job_api.py @@ -358,17 +358,50 @@ def test_connect_installs_sinks_and_disconnect_clears(monkeypatch): pytest.importorskip("uipath_ipc") captured = _fake_output_sinks(monkeypatch) - async def scenario() -> None: - # A named-pipe client connects lazily, so no server is needed to build it. - client = _job_api.connect_handler_ipc("some-pipe", "job-1") - assert captured["handler"] is not None - assert captured["sink"] is not None + class _Api(_job_api.IJobInvocationCommonApi): + async def SendLog(self, jobId: str, log: Any) -> None: + return None - await _job_api.disconnect_handler_ipc(client) - assert captured["handler"] is None - assert captured["sink"] is None + async def SetResult(self, jobId: str, result: Any) -> bool: + return True - asyncio.run(scenario()) + pipe = _unique_jobapi_pipe() + stop = _serve_jobapi_in_background(pipe, _Api()) + try: + + async def scenario() -> None: + conn = _job_api.connect_handler_ipc(pipe, "job-1") + assert captured["handler"] is not None + assert captured["sink"] is not None + + await _job_api.disconnect_handler_ipc(conn) + assert captured["handler"] is None + assert captured["sink"] is None + + asyncio.run(scenario()) + finally: + stop() + + +def test_connect_fails_loudly_when_the_pipe_is_unreachable(monkeypatch): + pytest.importorskip("uipath_ipc") + _fake_output_sinks(monkeypatch) + + def live() -> int: + return len( + [ + t + for t in threading.enumerate() + if t.name == "uipath-handler-ipc" and t.is_alive() + ] + ) + + before = live() + + with pytest.raises(RuntimeError, match="Could not reach the handler IPC pipe"): + _job_api.connect_handler_ipc("uipath-jobapi-does-not-exist-12345", "job-1") + + assert live() == before def test_connect_without_uipath_ipc_raises(monkeypatch): From 34c65bf718fb310f51f2bda5beb58d1017c21d3c Mon Sep 17 00:00:00 2001 From: Eduard Dumitru Date: Mon, 14 Sep 2026 11:53:48 +0200 Subject: [PATCH 12/18] fix(cli): refuse to stream job output without a real job id The peer types the job id as a Guid and rejects anything it cannot parse as one. Both lanes accepted whatever they were handed: the CLI turned a missing UIPATH_JOB_KEY into "", and the pooled server passed an unset JobKey straight through. Since requesting IPC suppresses the file sinks, every SendLog and SetResult then failed on the wire and the run produced no logs anywhere while still exiting 0. Validate the shape in both places and fail fast instead. --- packages/uipath/src/uipath/_cli/_job_api.py | 24 +++++- packages/uipath/src/uipath/_cli/cli_run.py | 2 +- .../uipath/src/uipath/_cli/cli_server_ipc.py | 7 ++ packages/uipath/tests/cli/test_job_api.py | 38 +++++++-- packages/uipath/tests/cli/test_server_ipc.py | 85 +++++++++++++++++-- 5 files changed, 140 insertions(+), 16 deletions(-) diff --git a/packages/uipath/src/uipath/_cli/_job_api.py b/packages/uipath/src/uipath/_cli/_job_api.py index 556aa441a..9023a52c3 100644 --- a/packages/uipath/src/uipath/_cli/_job_api.py +++ b/packages/uipath/src/uipath/_cli/_job_api.py @@ -7,13 +7,14 @@ import logging import sys import threading +import uuid from abc import ABC, abstractmethod from collections.abc import AsyncIterator from concurrent.futures import Future from concurrent.futures import wait as _futures_wait from dataclasses import dataclass from enum import IntEnum -from typing import Any +from typing import Any, TypeGuard logger = logging.getLogger(__name__) @@ -307,8 +308,23 @@ def _shutdown(self) -> None: _stop_loop_thread(self._loop, self._thread, _SET_RESULT_TIMEOUT_S) -def connect_handler_ipc(pipe: str, job_id: str) -> _HandlerIpcConnection: +def is_wire_job_id(job_id: str | None) -> TypeGuard[str]: + """The peer types the job id as a Guid, and rejects anything it can't parse as one.""" + try: + uuid.UUID(str(job_id)) + except ValueError: + return False + return True + + +def connect_handler_ipc(pipe: str, job_id: str | None) -> _HandlerIpcConnection: """Dial ``pipe`` on a dedicated loop/thread and install the sinks (see ``_HandlerIpcConnection``).""" + if not is_wire_job_id(job_id): + # Every call would fail on the wire, and the file sinks are already suppressed by now. + raise RuntimeError( + f"--handler-ipc-pipe needs UIPATH_JOB_KEY to be a job id; got {job_id!r}." + ) + try: from uipath_ipc import IpcClient, NamedPipeClientTransport except ImportError as e: @@ -359,7 +375,9 @@ async def disconnect_handler_ipc(conn: _HandlerIpcConnection) -> None: @contextlib.asynccontextmanager -async def handler_ipc_connection(pipe: str | None, job_id: str) -> AsyncIterator[Any]: +async def handler_ipc_connection( + pipe: str | None, job_id: str | None +) -> AsyncIterator[Any]: """Connect (if ``pipe`` is set) and always disconnect on exit; yields the connection or None.""" conn = connect_handler_ipc(pipe, job_id) if pipe else None try: diff --git a/packages/uipath/src/uipath/_cli/cli_run.py b/packages/uipath/src/uipath/_cli/cli_run.py index a07b1a607..3e9c4904f 100644 --- a/packages/uipath/src/uipath/_cli/cli_run.py +++ b/packages/uipath/src/uipath/_cli/cli_run.py @@ -224,7 +224,7 @@ async def execute() -> None: from ._job_api import handler_ipc_connection async with ( - handler_ipc_connection(handler_ipc_pipe, ctx.job_id or ""), + handler_ipc_connection(handler_ipc_pipe, ctx.job_id), ResourceOverwritesContext( lambda: read_resource_overwrites_from_file(ctx.runtime_dir) ), diff --git a/packages/uipath/src/uipath/_cli/cli_server_ipc.py b/packages/uipath/src/uipath/_cli/cli_server_ipc.py index e6fff09c6..b5c2c927b 100644 --- a/packages/uipath/src/uipath/_cli/cli_server_ipc.py +++ b/packages/uipath/src/uipath/_cli/cli_server_ipc.py @@ -105,8 +105,15 @@ async def RunJob( IJobInvocationCommonApi, clear_runtime_sinks, install_runtime_sinks, + is_wire_job_id, ) + if not is_wire_job_id(request.JobKey): + return PythonServerRunJobResult( + ExitCode=1, + Error=f"StreamOutputOverIpc needs a 'JobKey' that is a job id; got {request.JobKey!r}", + ) + callback = None client = message.client if message is not None else None if client is not None: diff --git a/packages/uipath/tests/cli/test_job_api.py b/packages/uipath/tests/cli/test_job_api.py index d427f13a2..6208a211a 100644 --- a/packages/uipath/tests/cli/test_job_api.py +++ b/packages/uipath/tests/cli/test_job_api.py @@ -19,6 +19,9 @@ from uipath._cli import _job_api from uipath.runtime.result import UiPathRuntimeStatus +JOB_ID = "3f2504e0-4f89-11d3-9a0c-0305e82c3301" +JOB_ID_2 = "9a7b1c22-0e54-4f7d-8c31-6b2f0a5d9e10" + def test_to_result_dto_maps_status_error_and_path(): class _Category: @@ -370,7 +373,7 @@ async def SetResult(self, jobId: str, result: Any) -> bool: try: async def scenario() -> None: - conn = _job_api.connect_handler_ipc(pipe, "job-1") + conn = _job_api.connect_handler_ipc(pipe, JOB_ID) assert captured["handler"] is not None assert captured["sink"] is not None @@ -399,7 +402,7 @@ def live() -> int: before = live() with pytest.raises(RuntimeError, match="Could not reach the handler IPC pipe"): - _job_api.connect_handler_ipc("uipath-jobapi-does-not-exist-12345", "job-1") + _job_api.connect_handler_ipc("uipath-jobapi-does-not-exist-12345", JOB_ID) assert live() == before @@ -407,7 +410,32 @@ def live() -> int: def test_connect_without_uipath_ipc_raises(monkeypatch): monkeypatch.setitem(sys.modules, "uipath_ipc", None) with pytest.raises(RuntimeError, match="uipath-ipc"): - _job_api.connect_handler_ipc("pipe", "job-1") + _job_api.connect_handler_ipc("pipe", JOB_ID) + + +def _live_ipc_threads() -> int: + return len( + [ + t + for t in threading.enumerate() + if t.name == "uipath-handler-ipc" and t.is_alive() + ] + ) + + +@pytest.mark.parametrize("job_id", [None, "", " ", "job-1", "not-a-guid"]) +def test_connect_without_a_real_job_id_fails_fast(job_id): + before = _live_ipc_threads() + with pytest.raises(RuntimeError, match="UIPATH_JOB_KEY"): + _job_api.connect_handler_ipc("pipe", job_id) + assert _live_ipc_threads() == before + + +@pytest.mark.parametrize("job_id", [None, "", "job-1"]) +async def test_handler_ipc_connection_without_a_real_job_id_fails_fast(job_id): + with pytest.raises(RuntimeError, match="UIPATH_JOB_KEY"): + async with _job_api.handler_ipc_connection("pipe", job_id): + pass _jobapi_pipe_counter = 0 @@ -502,7 +530,7 @@ class _Result: try: async def scenario() -> None: - conn = _job_api.connect_handler_ipc(pipe, "job-77") + conn = _job_api.connect_handler_ipc(pipe, JOB_ID_2) # Call the sink ON this loop's thread, exactly as the runtime's __exit__ does. Before the # fix this deadlocked the loop the ack was scheduled on; now it completes. captured["sink"](_Result(), "out.args") @@ -516,6 +544,6 @@ async def scenario() -> None: "SetResult never arrived — the result sink deadlocked/timed out" ) job_id, dto = received["result"] - assert job_id == "job-77" + assert job_id == JOB_ID_2 assert dto.outputArgumentsFilePath == "out.args" assert dto.status == _job_api.ExecutorJobStatus.SUCCESSFUL.value diff --git a/packages/uipath/tests/cli/test_server_ipc.py b/packages/uipath/tests/cli/test_server_ipc.py index 078b48c21..f85748a22 100644 --- a/packages/uipath/tests/cli/test_server_ipc.py +++ b/packages/uipath/tests/cli/test_server_ipc.py @@ -17,7 +17,7 @@ import sys import threading import time -from typing import Any, Awaitable, Callable +from typing import Any, Awaitable, Callable, cast import click import pytest @@ -126,6 +126,9 @@ def test_start_ipc_server_fails_fast_without_uipath_ipc(monkeypatch): asyncio.run(coro) +JOB_ID = "3f2504e0-4f89-11d3-9a0c-0305e82c3301" + + class TestIpcServer: @pytest.fixture def pipe(self): @@ -340,7 +343,7 @@ def test_register_is_a_readiness_probe(self): service = PythonRuntimeService() async def scenario() -> None: - assert await service.Register(Message(client=object())) is True + assert await service.Register(Message(client=cast(Any, object()))) is True asyncio.run(scenario()) @@ -365,6 +368,7 @@ async def _fake_run(*args: Any, **kwargs: Any) -> dict[str, Any]: monkeypatch.setattr(cli_server_ipc, "_run_command_isolated", _fake_run) + message: "Message[None]" if mode == "no-client": message = Message() else: @@ -373,11 +377,11 @@ class _Client: def get_callback(self, contract: Any) -> Any: raise RuntimeError("no callback available") - message = Message(client=_Client()) + message = Message(client=cast(Any, _Client())) service = PythonRuntimeService() request = PythonServerRunRequest( - JobKey="job-9", Command="run", Args=[], StreamOutputOverIpc=True + JobKey=JOB_ID, Command="run", Args=[], StreamOutputOverIpc=True ) async def scenario() -> Any: @@ -388,6 +392,73 @@ async def scenario() -> Any: assert "callback" in (result.Error or "").lower() assert ran == [] + @pytest.mark.parametrize( + "job_key", ["", "job-9", "00000000-0000-0000-0000-00000000000"] + ) + def test_runjob_without_a_real_job_key_fails_loudly(self, monkeypatch, job_key): + from uipath._cli import _job_api, cli_server_ipc + from uipath._cli.cli_server_ipc import ( + PythonRuntimeService, + PythonServerRunRequest, + ) + + events: list[Any] = [] + monkeypatch.setattr( + _job_api, + "install_runtime_sinks", + lambda jid, cb, loop: events.append(("install", jid)), + ) + + async def _fake_run(*args: Any, **kwargs: Any) -> dict[str, Any]: + events.append("run") + return {"ExitCode": 0, "Error": None} + + monkeypatch.setattr(cli_server_ipc, "_run_command_isolated", _fake_run) + + class _Client: + def get_callback(self, contract: Any) -> Any: + return "CALLBACK" + + service = PythonRuntimeService() + request = PythonServerRunRequest( + JobKey=job_key, Command="run", Args=[], StreamOutputOverIpc=True + ) + + async def scenario() -> Any: + return await service.RunJob( + request, message=Message(client=cast(Any, _Client())) + ) + + result = asyncio.run(scenario()) + assert result.ExitCode == 1 + assert "JobKey" in (result.Error or "") + assert events == [] + + def test_runjob_without_streaming_does_not_need_a_job_key(self, monkeypatch): + from uipath._cli import cli_server_ipc + from uipath._cli.cli_server_ipc import ( + PythonRuntimeService, + PythonServerRunRequest, + ) + + ran: list[bool] = [] + + async def _fake_run(*args: Any, **kwargs: Any) -> dict[str, Any]: + ran.append(True) + return {"ExitCode": 0, "Error": None} + + monkeypatch.setattr(cli_server_ipc, "_run_command_isolated", _fake_run) + + service = PythonRuntimeService() + request = PythonServerRunRequest(JobKey="", Command="run", Args=[]) + + async def scenario() -> Any: + return await service.RunJob(request) + + result = asyncio.run(scenario()) + assert result.ExitCode == 0 + assert ran == [True] + def test_runjob_installs_the_sinks_from_the_request_callback(self, monkeypatch): from uipath._cli import _job_api, cli_server_ipc from uipath._cli.cli_server_ipc import PythonServerRunRequest @@ -418,14 +489,14 @@ def get_callback(self, contract: Any) -> Any: service = PythonRuntimeService() request = PythonServerRunRequest( - JobKey="job-9", Command="run", Args=[], StreamOutputOverIpc=True + JobKey=JOB_ID, Command="run", Args=[], StreamOutputOverIpc=True ) async def scenario() -> None: await service.RunJob(request, message=Message(client=_Client())) asyncio.run(scenario()) - assert events == [("install", "job-9", "CALLBACK"), ("run",), ("clear",)] + assert events == [("install", JOB_ID, "CALLBACK"), ("run",), ("clear",)] def test_runjob_drains_pending_sends_before_returning(self, monkeypatch): from uipath._cli import _job_api, cli_server_ipc @@ -458,7 +529,7 @@ def get_callback(self, contract: Any) -> Any: service = PythonRuntimeService() request = PythonServerRunRequest( - JobKey="job-7", Command="run", Args=[], StreamOutputOverIpc=True + JobKey=JOB_ID, Command="run", Args=[], StreamOutputOverIpc=True ) async def scenario() -> None: From c6ad1655d00c4039feb5f50ad6c3d5743741d726 Mon Sep 17 00:00:00 2001 From: Eduard Dumitru Date: Mon, 14 Sep 2026 12:38:23 +0200 Subject: [PATCH 13/18] fix(cli): refuse the handler IPC pipe when a plugin takes over the run A "run" middleware that returns should_continue=False runs the job itself and skips the block that opens the pipe, so the caller -- which stopped watching the log file because it was promised a pipe -- got no logs, no result, and "Successful execution.". Refuse the flag in that case instead. Both checks now test the pipe for presence rather than truthiness, so an empty name cannot slip past them either. --- packages/uipath/src/uipath/_cli/_job_api.py | 2 +- packages/uipath/src/uipath/_cli/cli_run.py | 8 +++ packages/uipath/tests/cli/test_run.py | 71 +++++++++++++++++++++ 3 files changed, 80 insertions(+), 1 deletion(-) diff --git a/packages/uipath/src/uipath/_cli/_job_api.py b/packages/uipath/src/uipath/_cli/_job_api.py index 9023a52c3..5ab1e7fb8 100644 --- a/packages/uipath/src/uipath/_cli/_job_api.py +++ b/packages/uipath/src/uipath/_cli/_job_api.py @@ -379,7 +379,7 @@ async def handler_ipc_connection( pipe: str | None, job_id: str | None ) -> AsyncIterator[Any]: """Connect (if ``pipe`` is set) and always disconnect on exit; yields the connection or None.""" - conn = connect_handler_ipc(pipe, job_id) if pipe else None + conn = connect_handler_ipc(pipe, job_id) if pipe is not None else None try: yield conn finally: diff --git a/packages/uipath/src/uipath/_cli/cli_run.py b/packages/uipath/src/uipath/_cli/cli_run.py index 3e9c4904f..f5d7daa59 100644 --- a/packages/uipath/src/uipath/_cli/cli_run.py +++ b/packages/uipath/src/uipath/_cli/cli_run.py @@ -171,6 +171,14 @@ def run( console.error(result.error_message) return + if not result.should_continue and handler_ipc_pipe is not None: + # A plugin ran the job its own way, so nothing reached the pipe the caller is waiting on. + console.error( + "--handler-ipc-pipe was requested, but a plugin took over the run and cannot " + "stream the logs or the result over it." + ) + return + if result.should_continue: try: diff --git a/packages/uipath/tests/cli/test_run.py b/packages/uipath/tests/cli/test_run.py index aa182c7c5..df15d6d24 100644 --- a/packages/uipath/tests/cli/test_run.py +++ b/packages/uipath/tests/cli/test_run.py @@ -178,6 +178,77 @@ def test_run_input_file_success( assert result.exit_code == 0 assert "Successful execution." in result.output + class TestHandlerIpcPipe: + """``--handler-ipc-pipe`` promises the caller logs + result over a pipe.""" + + @staticmethod + def _taken_over(): + return patch( + "uipath._cli.cli_run.Middlewares.next", + return_value=MiddlewareResult( + should_continue=False, + info_message="Execution succeeded", + error_message=None, + should_include_stacktrace=False, + ), + ) + + @pytest.mark.parametrize("pipe", ["some-pipe", ""]) + def test_taken_over_run_cannot_honour_the_pipe( + self, runner: CliRunner, temp_dir: str, entrypoint: str, pipe: str + ): + with runner.isolated_filesystem(temp_dir=temp_dir), self._taken_over(): + result = runner.invoke( + cli, ["run", entrypoint, "--handler-ipc-pipe", pipe] + ) + assert result.exit_code == 1 + assert "a plugin took over the run" in result.output + assert "Successful execution." not in result.output + + def test_taken_over_run_without_the_pipe_is_untouched( + self, runner: CliRunner, temp_dir: str, entrypoint: str + ): + with runner.isolated_filesystem(temp_dir=temp_dir), self._taken_over(): + result = runner.invoke(cli, ["run", entrypoint]) + assert result.exit_code == 0 + assert "Successful execution." in result.output + + def test_a_run_nobody_claimed_reaches_the_pipe( + self, runner: CliRunner, temp_dir: str + ): + opened: list[str] = [] + + @asynccontextmanager + async def _fake_connection(pipe, job_id): + opened.append(pipe) + yield None + + with runner.isolated_filesystem(temp_dir=temp_dir): + with ( + patch( + "uipath._cli.cli_run.Middlewares.next", + return_value=_middleware_continue(), + ), + patch( + "uipath._cli.cli_run.UiPathRuntimeFactoryRegistry.get", + return_value=_make_mock_factory(["my_agent"]), + ), + patch( + "uipath._cli.cli_run.ResourceOverwritesContext", + side_effect=_mock_resource_overwrites_context, + ), + patch( + "uipath._cli._job_api.handler_ipc_connection", _fake_connection + ), + ): + result = runner.invoke( + cli, ["run", "--handler-ipc-pipe", "some-pipe"] + ) + + assert opened == ["some-pipe"], f"output: {result.output!r}" + assert result.exit_code == 0 + assert "took over the run" not in result.output + class TestMiddleware: def test_autodiscover_entrypoint(self, runner: CliRunner, temp_dir: str): """When exactly one entrypoint exists, it is auto-resolved.""" From 860a8183724b2ebe0550aea49ddcfeebf7ee2fae Mon Sep 17 00:00:00 2001 From: Eduard Dumitru Date: Mon, 14 Sep 2026 14:24:50 +0200 Subject: [PATCH 14/18] style(cli): keep only the comments that record something non-obvious Nineteen comments down to seven one-liners. What is left is the blocking/awaiting asymmetry between the two lanes, why teardown must not raise, why the connect is eager, where the contextvar sinks have to be installed, why the Message placeholder survives import, and why the callback is never cached. The rest restated the code or repeated an error message. --- packages/uipath/src/uipath/_cli/_job_api.py | 19 ++++--------------- packages/uipath/src/uipath/_cli/cli_run.py | 2 -- .../uipath/src/uipath/_cli/cli_server_ipc.py | 10 +++------- 3 files changed, 7 insertions(+), 24 deletions(-) diff --git a/packages/uipath/src/uipath/_cli/_job_api.py b/packages/uipath/src/uipath/_cli/_job_api.py index 5ab1e7fb8..8214e9e1a 100644 --- a/packages/uipath/src/uipath/_cli/_job_api.py +++ b/packages/uipath/src/uipath/_cli/_job_api.py @@ -22,7 +22,6 @@ _LOG_FLUSH_TIMEOUT_S = 5.0 _IPC_REQUEST_TIMEOUT_S = 30.0 -# Loggers whose records must never ride the IPC channel: the transport itself and this module. _NO_IPC_LOGGERS = ("uipath_ipc", __name__) @@ -192,13 +191,12 @@ def _snapshot(self) -> "list[Future[object]]": return list(self._pending) def flush_pending(self, timeout: float = _LOG_FLUSH_TIMEOUT_S) -> None: - # Blocks; only safe off the sink loop's own thread (see _HandlerIpcConnection._shutdown). + # Blocks; only safe off the sink loop's own thread. pending = self._snapshot() if pending: _futures_wait(pending, timeout=timeout) async def aflush_pending(self, timeout: float = _LOG_FLUSH_TIMEOUT_S) -> None: - # Awaits instead of blocking, so it is safe when the caller shares the sink's loop. pending = self._snapshot() if not pending: return @@ -235,7 +233,6 @@ def _result_sink(result: Any, output_arguments_file_path: str) -> None: "The handler rejected the job result (SetResult returned false)" ) except Exception: - # Best-effort — a dropped result must be logged, not swallowed. logger.exception("Failed to deliver job result over IPC (SetResult)") set_log_handler(handler) @@ -256,8 +253,7 @@ def clear_runtime_sinks() -> None: def _stop_loop_thread( loop: asyncio.AbstractEventLoop, thread: threading.Thread, timeout: float ) -> None: - # Teardown must never raise: a wedged thread leaves the loop running, and close() would then - # raise and mask the job's own outcome (or the error that got us here). + # Never raise here: a wedged thread leaves the loop running, and close() would mask the job's outcome. try: loop.call_soon_threadsafe(loop.stop) except Exception: @@ -295,8 +291,6 @@ def __init__( def _shutdown(self) -> None: """Close the client and stop its loop/thread.""" - # Runs on a worker thread, so blocking here is safe: the sink loop has its own thread and - # keeps draining. aclose() would fail whatever is still queued, losing the log tail. if self._log_handler is not None: self._log_handler.flush_pending() try: @@ -320,7 +314,6 @@ def is_wire_job_id(job_id: str | None) -> TypeGuard[str]: def connect_handler_ipc(pipe: str, job_id: str | None) -> _HandlerIpcConnection: """Dial ``pipe`` on a dedicated loop/thread and install the sinks (see ``_HandlerIpcConnection``).""" if not is_wire_job_id(job_id): - # Every call would fail on the wire, and the file sinks are already suppressed by now. raise RuntimeError( f"--handler-ipc-pipe needs UIPATH_JOB_KEY to be a job id; got {job_id!r}." ) @@ -340,8 +333,7 @@ def connect_handler_ipc(pipe: str, job_id: str | None) -> _HandlerIpcConnection: async def _build() -> Any: transport = NamedPipeClientTransport(pipe) - # The transport dials lazily, and the interceptor has already suppressed execution.log by - # the time the first record is sent: an unreachable pipe would lose every line in silence. + # Dial now: execution.log is already suppressed, so a dead pipe would lose every line in silence. try: _, writer = await transport.connect() except Exception as e: @@ -358,11 +350,9 @@ async def _build() -> Any: timeout=_SET_RESULT_TIMEOUT_S ) except BaseException: - # On failure, don't leak the loop/thread. _stop_loop_thread(loop, thread, _SET_RESULT_TIMEOUT_S) raise - # Install in the caller's context, not on the IPC thread: the sinks are contextvars, resolved in - # the job's own context at teardown. The ack still runs on `loop` (a separate thread) — no deadlock. + # Install in the caller's context: the sinks are contextvars, read in the job's own context at teardown. handler = install_runtime_sinks(job_id, proxy, loop) return _HandlerIpcConnection(client, loop, thread, handler) @@ -370,7 +360,6 @@ async def _build() -> Any: async def disconnect_handler_ipc(conn: _HandlerIpcConnection) -> None: """Clear the sinks and tear down the connection.""" clear_runtime_sinks() - # Off the caller's loop so joining the thread doesn't block it. await asyncio.to_thread(conn._shutdown) diff --git a/packages/uipath/src/uipath/_cli/cli_run.py b/packages/uipath/src/uipath/_cli/cli_run.py index f5d7daa59..8ab150ab5 100644 --- a/packages/uipath/src/uipath/_cli/cli_run.py +++ b/packages/uipath/src/uipath/_cli/cli_run.py @@ -172,7 +172,6 @@ def run( return if not result.should_continue and handler_ipc_pipe is not None: - # A plugin ran the job its own way, so nothing reached the pipe the caller is waiting on. console.error( "--handler-ipc-pipe was requested, but a plugin took over the run and cannot " "stream the logs or the result over it." @@ -228,7 +227,6 @@ async def execute() -> None: JsonLinesFileExporter(ctx.trace_file) ) - # If a pipe was given, install the sinks around the run (always torn down); else a no-op. from ._job_api import handler_ipc_connection async with ( diff --git a/packages/uipath/src/uipath/_cli/cli_server_ipc.py b/packages/uipath/src/uipath/_cli/cli_server_ipc.py index b5c2c927b..6cc41da8f 100644 --- a/packages/uipath/src/uipath/_cli/cli_server_ipc.py +++ b/packages/uipath/src/uipath/_cli/cli_server_ipc.py @@ -9,8 +9,7 @@ if TYPE_CHECKING: from uipath_ipc import Message else: - # Optional dependency (uipath[ipc]): the Register annotation stays a string forward-ref so this - # placeholder is never subscripted at import — resolved to the real Message only at dispatch. + # Optional dep: the annotation stays a string forward-ref, so this placeholder is never subscripted. try: from uipath_ipc import Message except ImportError: # pragma: no cover - no IPC means Register is never dispatched @@ -35,7 +34,6 @@ class PythonServerRunRequest: Args: str | list[str] | None = None WorkingDirectory: str | None = None EnvironmentVariables: dict[str, str] = field(default_factory=dict) - # Per-job opt-in (default False so it stays off unless explicitly set). StreamOutputOverIpc: bool = False @@ -100,7 +98,7 @@ async def RunJob( on_run_end: "Any" = None installed: "list[Any]" = [] if request.StreamOutputOverIpc: - # Derived per request, never stored: a captured callback goes stale on reconnect/restart. + # Never stored: a captured callback goes stale on reconnect/restart. from ._job_api import ( IJobInvocationCommonApi, clear_runtime_sinks, @@ -145,8 +143,7 @@ def _install() -> None: on_run_end=on_run_end, ) - # Drain in-flight sends by awaiting: they share this loop, and the peer may unregister the - # job the moment this response lands. + # Await, don't block: these share this loop, and the peer may unregister the job once this returns. for handler in installed: if handler is not None: await handler.aflush_pending() @@ -177,7 +174,6 @@ async def start_ipc_server(pipe_name: str) -> None: _state.init() - # Register the default runtime factory (idempotent) so the server works when started outside the CLI. from uipath._cli import _ensure_runtime_initialized _ensure_runtime_initialized() From 47ba232df4e80b386a46e18dd5a30c3ae0707656 Mon Sep 17 00:00:00 2001 From: Eduard Dumitru Date: Mon, 14 Sep 2026 14:25:00 +0200 Subject: [PATCH 15/18] fix(cli): declare Message on the RunJob contract so the pooled lane works The dispatcher builds its plan from the contract, not the implementation, so declaring `message` only on PythonRuntimeService left it uninjected: every pooled job that opted into streaming was rejected with "no IPC callback is available" before it ran. Every pooled test passed a message by hand, which hid it -- the new test drives a real pipe with a client that hosts the callback. --- .../uipath/src/uipath/_cli/cli_server_ipc.py | 9 ++- packages/uipath/tests/cli/test_server_ipc.py | 61 ++++++++++++++++++- 2 files changed, 67 insertions(+), 3 deletions(-) diff --git a/packages/uipath/src/uipath/_cli/cli_server_ipc.py b/packages/uipath/src/uipath/_cli/cli_server_ipc.py index 6cc41da8f..30e8aaea4 100644 --- a/packages/uipath/src/uipath/_cli/cli_server_ipc.py +++ b/packages/uipath/src/uipath/_cli/cli_server_ipc.py @@ -58,8 +58,13 @@ async def Register(self, message: "Message[None]") -> bool: """Prove the connection is up.""" @abstractmethod - async def RunJob(self, request: PythonServerRunRequest) -> PythonServerRunJobResult: - """Run a job → PythonServerRunJobResult(ExitCode, Error).""" + async def RunJob( + self, request: PythonServerRunRequest, *, message: "Message[None] | None" = None + ) -> PythonServerRunJobResult: + """Run a job → PythonServerRunJobResult(ExitCode, Error). + + ``message`` is injected by the dispatcher, which reads this contract — not the impl. + """ @abstractmethod async def StopJob(self, request: PythonServerStopJobRequest) -> bool: diff --git a/packages/uipath/tests/cli/test_server_ipc.py b/packages/uipath/tests/cli/test_server_ipc.py index f85748a22..0462d4fb6 100644 --- a/packages/uipath/tests/cli/test_server_ipc.py +++ b/packages/uipath/tests/cli/test_server_ipc.py @@ -30,6 +30,7 @@ ) from uipath._cli import _server_core +from uipath._cli._job_api import IJobInvocationCommonApi from uipath._cli.cli_server import ( IPythonRuntimeServer, PythonRuntimeService, @@ -292,7 +293,9 @@ class SpyService(IPythonRuntimeServer): async def Register(self, message: Any) -> bool: return True - async def RunJob(self, request: Any) -> PythonServerRunJobResult: + async def RunJob( + self, request: Any, *, message: Any = None + ) -> PythonServerRunJobResult: received.append(request) return PythonServerRunJobResult(ExitCode=0) @@ -459,6 +462,62 @@ async def scenario() -> Any: assert result.ExitCode == 0 assert ran == [True] + def test_pooled_streaming_works_over_a_real_pipe(self, monkeypatch): + """No hand-fed ``message``: the dispatcher must inject it from the contract.""" + from uipath._cli import _job_api, cli_server_ipc + + installed: list[Any] = [] + monkeypatch.setattr( + _job_api, + "install_runtime_sinks", + lambda jid, cb, loop: installed.append(jid), + ) + + async def _fake_run(cmd, args, env, wd, on_run_start=None, on_run_end=None): + if on_run_start: + on_run_start() + if on_run_end: + on_run_end() + return {"ExitCode": 0, "Error": None} + + monkeypatch.setattr(cli_server_ipc, "_run_command_isolated", _fake_run) + + class _Callback: + async def SendLog(self, job_id: str, log: Any) -> None: + return None + + async def SetResult(self, job_id: str, result: Any) -> bool: + return True + + pipe = _unique_pipe() + _serve_in_background(pipe) + + async def scenario() -> Any: + client = IpcClient( + transport=NamedPipeClientTransport(pipe), + callbacks={IJobInvocationCommonApi: _Callback()}, + ) + try: + proxy = client.get_proxy(IPythonRuntimeServer) # type: ignore[type-abstract] + return await proxy.RunJob( + cast( + Any, + { + "JobKey": JOB_ID, + "Command": "run", + "Args": [], + "StreamOutputOverIpc": True, + }, + ) + ) + finally: + await client.aclose() + + result = asyncio.run(scenario()) + assert result.Error is None, result.Error + assert result.ExitCode == 0 + assert installed == [JOB_ID] + def test_runjob_installs_the_sinks_from_the_request_callback(self, monkeypatch): from uipath._cli import _job_api, cli_server_ipc from uipath._cli.cli_server_ipc import PythonServerRunRequest From 367fc2374e2fc616f49d688d37ddfc9910de6d0c Mon Sep 17 00:00:00 2001 From: Eduard Dumitru Date: Mon, 14 Sep 2026 14:25:01 +0200 Subject: [PATCH 16/18] fix(cli): report a broken log channel instead of dropping every send SendLog failures landed in a future whose exception was retrieved and thrown away, so a handler that went away mid-job produced an empty log stream and exit 0 -- the failure this channel exists to prevent. Report the first one to the real stderr, then stay quiet so a dead pipe cannot spam. --- packages/uipath/src/uipath/_cli/_job_api.py | 32 ++++++++++++++------- packages/uipath/tests/cli/test_job_api.py | 32 +++++++++++++++++++++ 2 files changed, 54 insertions(+), 10 deletions(-) diff --git a/packages/uipath/src/uipath/_cli/_job_api.py b/packages/uipath/src/uipath/_cli/_job_api.py index 8214e9e1a..cd8f690b6 100644 --- a/packages/uipath/src/uipath/_cli/_job_api.py +++ b/packages/uipath/src/uipath/_cli/_job_api.py @@ -134,24 +134,21 @@ def _to_result_dto( ) -def _drain(future: "Future[object]") -> None: - try: - future.exception() - except BaseException: - pass - - -def _to_original_stderr(handler: logging.Handler, record: logging.LogRecord) -> None: +def _write_original_stderr(text: str) -> None: stream = sys.__stderr__ if stream is None: return try: - stream.write(handler.format(record) + "\n") + stream.write(text + "\n") stream.flush() except Exception: pass +def _to_original_stderr(handler: logging.Handler, record: logging.LogRecord) -> None: + _write_original_stderr(handler.format(record)) + + class _IpcLogHandler(logging.Handler): """Forwards each log record to the callback.""" @@ -164,6 +161,7 @@ def __init__( self._loop = loop self._pending: set[Future[object]] = set() self._pending_lock = threading.Lock() + self._reported_failure = False def emit(self, record: logging.LogRecord) -> None: if record.name.startswith(_NO_IPC_LOGGERS): @@ -184,7 +182,21 @@ def emit(self, record: logging.LogRecord) -> None: def _settled(self, future: "Future[object]") -> None: with self._pending_lock: self._pending.discard(future) - _drain(future) + try: + error: BaseException | None = future.exception() + except BaseException as e: # cancelled + error = e + if error is None: + return + with self._pending_lock: + first = not self._reported_failure + self._reported_failure = True + if first: + # Once per handler: a broken channel fails every later send too. + _write_original_stderr( + f"uipath: job logs are no longer reaching the handler ({error!r}); " + "further log-delivery errors for this job are suppressed." + ) def _snapshot(self) -> "list[Future[object]]": with self._pending_lock: diff --git a/packages/uipath/tests/cli/test_job_api.py b/packages/uipath/tests/cli/test_job_api.py index 6208a211a..2eff4478c 100644 --- a/packages/uipath/tests/cli/test_job_api.py +++ b/packages/uipath/tests/cli/test_job_api.py @@ -413,6 +413,38 @@ def test_connect_without_uipath_ipc_raises(monkeypatch): _job_api.connect_handler_ipc("pipe", JOB_ID) +def test_a_broken_log_channel_is_reported_once_to_stderr(monkeypatch): + _fake_output_sinks(monkeypatch) + + buf = io.StringIO() + monkeypatch.setattr(sys, "__stderr__", buf) + + class _Callback: + async def SendLog(self, job_id, log): + raise RuntimeError("pipe is gone") + + loop = asyncio.new_event_loop() + thread = threading.Thread(target=loop.run_forever, daemon=True) + thread.start() + try: + handler = _job_api.install_runtime_sinks(JOB_ID, _Callback(), loop) + assert handler is not None + for i in range(4): + handler.emit( + logging.LogRecord("job", logging.INFO, __file__, 1, f"m{i}", None, None) + ) + handler.flush_pending(timeout=5.0) + finally: + loop.call_soon_threadsafe(loop.stop) + thread.join(timeout=5.0) + loop.close() + + out = buf.getvalue() + assert "no longer reaching the handler" in out, out + assert "pipe is gone" in out + assert out.count("no longer reaching the handler") == 1, out + + def _live_ipc_threads() -> int: return len( [ From 19fd3e2c05c3d1d5f6381de69a35f4d145012dea Mon Sep 17 00:00:00 2001 From: Eduard Dumitru Date: Tue, 15 Sep 2026 10:52:47 +0200 Subject: [PATCH 17/18] fix(cli): address the review follow-ups on the IPC job-api Refuse an all-zeros job id: Guid's default parses as a UUID, so it passed the shape check and then routed nowhere, the peer dropping every log on a debug line nobody reads. The test meant to cover it used a 35-character string, so it had only ever exercised "malformed". Hide --handler-ipc-pipe: the job executor sets it, a person never does, yet it was listed in every user's --help and in the generated CLI docs. Drop the "no IPC callback available" guard down to what is knowable. The dispatcher always injects a Message carrying the connection and get_callback merely wraps it, so the guard could not fire over IPC while advertising a wire-level problem it was incapable of seeing. Stop claiming the serialization lock bounds per-job global state: cancelling the RunJob task releases it while the job is still running on a thread cancellation cannot reach. On the tests, four that did not pin what they claimed: - The sink helper swapped the runtime's ContextVars for a dict, recording THAT something was installed but not WHERE. Installing from a foreign thread -- the one thing the code forbids -- kept all 42 green; it now fails two. - The non-pooled test proved the pipe was dialled, not that the job ran inside it. Hoisting the connection out of the run, or passing None for the job id, both passed; both now fail. - No test asserted a log line crossing a real pipe, only the result half. - The mapped error checked Code and Category alone, so swapping Title and Detail passed -- a stack trace would surface as the error's title. Plus coverage for aflush_pending and for a cancelled send, whose future raises rather than returns, and a note that the suspended mapping is defensive: the runtime delivers SUCCESSFUL and FAULTED alone and the peer resolves suspend from output.json. --- packages/uipath/src/uipath/_cli/_job_api.py | 14 +- .../uipath/src/uipath/_cli/_server_core.py | 5 +- packages/uipath/src/uipath/_cli/cli_run.py | 1 + .../uipath/src/uipath/_cli/cli_server_ipc.py | 15 +- packages/uipath/tests/cli/test_job_api.py | 136 ++++++++++++++---- packages/uipath/tests/cli/test_run.py | 51 +++++-- packages/uipath/tests/cli/test_server_ipc.py | 18 +-- 7 files changed, 173 insertions(+), 67 deletions(-) diff --git a/packages/uipath/src/uipath/_cli/_job_api.py b/packages/uipath/src/uipath/_cli/_job_api.py index cd8f690b6..7b0d63445 100644 --- a/packages/uipath/src/uipath/_cli/_job_api.py +++ b/packages/uipath/src/uipath/_cli/_job_api.py @@ -107,6 +107,8 @@ def _to_log_level(levelno: int) -> int: _EXECUTOR_STATUS: dict[str, int] = { "successful": ExecutorJobStatus.SUCCESSFUL.value, "faulted": ExecutorJobStatus.FAULTED.value, + # Defensive only: the runtime delivers a result for SUCCESSFUL and FAULTED alone, and the peer + # resolves a suspended job from output.json (resume triggers never cross the wire). "suspended": ExecutorJobStatus.SUSPENDED.value, } @@ -168,8 +170,9 @@ def emit(self, record: logging.LogRecord) -> None: _to_original_stderr(self, record) return try: - message = self.format(record) - dto = JobLogDto(Message=message, LogLevel=_to_log_level(record.levelno)) + dto = JobLogDto( + Message=self.format(record), LogLevel=_to_log_level(record.levelno) + ) future = asyncio.run_coroutine_threadsafe( self._callback.SendLog(self._job_id, dto), self._loop ) @@ -315,12 +318,13 @@ def _shutdown(self) -> None: def is_wire_job_id(job_id: str | None) -> TypeGuard[str]: - """The peer types the job id as a Guid, and rejects anything it can't parse as one.""" + """The peer types the job id as a Guid, and routes nothing for one it can't match.""" try: - uuid.UUID(str(job_id)) + parsed = uuid.UUID(str(job_id)) except ValueError: return False - return True + # All-zeros is Guid's default, so it is the one unusable value a caller reaches by omission. + return parsed.int != 0 def connect_handler_ipc(pipe: str, job_id: str | None) -> _HandlerIpcConnection: diff --git a/packages/uipath/src/uipath/_cli/_server_core.py b/packages/uipath/src/uipath/_cli/_server_core.py index 25d9614c5..1b70b6cb5 100644 --- a/packages/uipath/src/uipath/_cli/_server_core.py +++ b/packages/uipath/src/uipath/_cli/_server_core.py @@ -55,8 +55,9 @@ async def _run_command_isolated( ) -> dict[str, Any]: """Run one command with per-job env/cwd isolation (the shared job core). - ``on_run_start`` / ``on_run_end`` run INSIDE the serialization lock, so any per-job process-global - state is visible only while this job runs. + ``on_run_start`` / ``on_run_end`` run inside the serialization lock. That orders them against + the next job's start, but claims nothing once this task is cancelled: the job runs on a thread + that cancellation cannot reach, so it outlives the lock and the globals move under it. """ if _state.lock is None or _state.baseline_env is None: raise RuntimeError("Server state not initialized") diff --git a/packages/uipath/src/uipath/_cli/cli_run.py b/packages/uipath/src/uipath/_cli/cli_run.py index 8ab150ab5..b94dd85ba 100644 --- a/packages/uipath/src/uipath/_cli/cli_run.py +++ b/packages/uipath/src/uipath/_cli/cli_run.py @@ -120,6 +120,7 @@ def get_usage_help(self) -> list[str]: "--handler-ipc-pipe", required=False, default=None, + hidden=True, # set by the job executor, never by a person help="Named pipe to stream this job's logs and result over uipath-ipc instead of writing them to files.", ) @track_command("run") diff --git a/packages/uipath/src/uipath/_cli/cli_server_ipc.py b/packages/uipath/src/uipath/_cli/cli_server_ipc.py index 30e8aaea4..6ef227885 100644 --- a/packages/uipath/src/uipath/_cli/cli_server_ipc.py +++ b/packages/uipath/src/uipath/_cli/cli_server_ipc.py @@ -117,19 +117,16 @@ async def RunJob( Error=f"StreamOutputOverIpc needs a 'JobKey' that is a job id; got {request.JobKey!r}", ) - callback = None - client = message.client if message is not None else None - if client is not None: - try: - callback = client.get_callback(IJobInvocationCommonApi) # type: ignore[type-abstract] - except Exception: - callback = None - if callback is None: + if message is None or message.client is None: return PythonServerRunJobResult( ExitCode=1, - Error="StreamOutputOverIpc was requested but no IPC callback is available", + Error="StreamOutputOverIpc is only available when RunJob is invoked over IPC", ) + # get_callback only wraps the connection, so this cannot tell us whether the peer + # actually hosts the contract; a peer that doesn't shows up as a failing send. + callback = message.client.get_callback(IJobInvocationCommonApi) # type: ignore[type-abstract] + loop = asyncio.get_running_loop() job_key = request.JobKey diff --git a/packages/uipath/tests/cli/test_job_api.py b/packages/uipath/tests/cli/test_job_api.py index 2eff4478c..f31ca76a7 100644 --- a/packages/uipath/tests/cli/test_job_api.py +++ b/packages/uipath/tests/cli/test_job_api.py @@ -11,7 +11,8 @@ import os import sys import threading -import types +from concurrent.futures import Future +from contextvars import ContextVar from typing import Any import pytest @@ -32,7 +33,7 @@ class _Error: title = "It broke" detail = "stack" category = _Category() - status = None + status = 404 class _Result: status = UiPathRuntimeStatus.FAULTED @@ -45,7 +46,15 @@ class _Result: assert dto.outputArgumentsFilePath == "out.args" assert dto.outputArguments is None assert dto.error is not None - assert (dto.error.Code, dto.error.Category) == ("BOOM", "User") + # Every field, so a swapped Title/Detail (a stack trace shown as the error's title in the + # job's failure record) cannot pass. + assert ( + dto.error.Code, + dto.error.Title, + dto.error.Detail, + dto.error.Category, + dto.error.Status, + ) == ("BOOM", "It broke", "stack", "User", 404) def test_to_result_dto_defaults_to_successful_without_error(): @@ -59,6 +68,10 @@ class _Result: def test_to_result_dto_maps_suspended(): + """Mapping only. Suspend never reaches SetResult -- the runtime delivers SUCCESSFUL/FAULTED + alone and the peer resolves a suspended job from output.json. Do not read this as the + suspend path being carried over IPC.""" + class _Result: status = UiPathRuntimeStatus.SUSPENDED error = None @@ -108,17 +121,36 @@ def test_dto_wire_key_sets_are_pinned(): } -def _fake_output_sinks(monkeypatch) -> dict[str, Any]: - captured: dict[str, Any] = {} - module = types.ModuleType("uipath.runtime.output_sinks") - module.set_log_handler = lambda h: captured.__setitem__("handler", h) # type: ignore[attr-defined] - module.set_result_sink = lambda s: captured.__setitem__("sink", s) # type: ignore[attr-defined] - monkeypatch.setitem(sys.modules, "uipath.runtime.output_sinks", module) - return captured +def _isolated_output_sinks(monkeypatch) -> Any: + """Read the sinks back through the REAL ContextVars. + + A dict would record that something was installed but not *where*: installing from the wrong + thread or task would keep every test green while the job, looking the value up in its own + context, found nothing. Fresh ContextVars per test keep that observable and stop one test's + sinks leaking into the next. + """ + from uipath.runtime import output_sinks + + monkeypatch.setattr( + output_sinks, "_log_handler", ContextVar("test_log_handler", default=None) + ) + monkeypatch.setattr( + output_sinks, "_result_sink", ContextVar("test_result_sink", default=None) + ) + + class _Installed: + def __getitem__(self, key: str) -> Any: + if key == "handler": + return output_sinks.get_log_handler() + if key == "sink": + return output_sinks.get_result_sink() + raise KeyError(key) + + return _Installed() def test_install_wires_log_handler_and_result_sink(monkeypatch): - captured = _fake_output_sinks(monkeypatch) + captured = _isolated_output_sinks(monkeypatch) logs: list[tuple[str, Any]] = [] results: list[tuple[str, Any]] = [] @@ -134,12 +166,12 @@ async def scenario() -> None: loop = asyncio.get_running_loop() _job_api.install_runtime_sinks("job-7", _Callback(), loop) - # The log handler forwards each record as SendLog, tagged with the job id. + # The log handler batches records and forwards them tagged with the job id. handler = captured["handler"] handler.emit( logging.LogRecord("n", logging.WARNING, "p", 1, "hi %s", ("there",), None) ) - await asyncio.sleep(0.05) + await handler.aflush_pending() assert logs[0][0] == "job-7" assert logs[0][1].Message == "hi there" assert logs[0][1].LogLevel == _job_api.LogLevel.WARNING.value @@ -157,27 +189,30 @@ class _Result: asyncio.run(scenario()) -def test_log_forwarding_failure_does_not_escape_emit(monkeypatch): - captured = _fake_output_sinks(monkeypatch) +def test_an_unformattable_record_does_not_escape_emit(monkeypatch): + """A mid-flight send failure is covered by the broken-channel test; this is the sync path.""" + captured = _isolated_output_sinks(monkeypatch) class _Callback: - def SendLog(self, jid: str, dto: Any) -> None: - raise RuntimeError("pipe is gone") + async def SendLog(self, jid: str, log: Any) -> None: + return None loop = asyncio.new_event_loop() try: - _job_api.install_runtime_sinks("job-8", _Callback(), loop) + _job_api.install_runtime_sinks(JOB_ID, _Callback(), loop) handler = captured["handler"] handled: list[Any] = [] monkeypatch.setattr(handler, "handleError", handled.append) - handler.emit(logging.LogRecord("n", logging.INFO, "p", 1, "hi", (), None)) + # %d against a str: formatting raises inside emit. + handler.emit(logging.LogRecord("n", logging.INFO, "p", 1, "%d", ("x",), None)) assert len(handled) == 1 + assert handler._snapshot() == [] finally: loop.close() def test_transport_and_self_logs_never_ride_the_ipc_channel(monkeypatch): - captured = _fake_output_sinks(monkeypatch) + captured = _isolated_output_sinks(monkeypatch) sent: list[Any] = [] forwarded = threading.Event() @@ -215,7 +250,7 @@ async def SendLog(self, jid: str, dto: Any) -> None: def test_rejected_result_is_reported(monkeypatch): - captured = _fake_output_sinks(monkeypatch) + captured = _isolated_output_sinks(monkeypatch) class _Callback: async def SetResult(self, jid: str, dto: Any) -> bool: @@ -280,7 +315,7 @@ async def _block() -> None: def test_pending_log_sends_are_flushed_before_teardown(monkeypatch): - _fake_output_sinks(monkeypatch) + _isolated_output_sinks(monkeypatch) landed: list[str] = [] class _Callback: @@ -306,7 +341,7 @@ async def aclose(self) -> None: def test_result_delivery_failure_is_swallowed_and_logged(monkeypatch): - captured = _fake_output_sinks(monkeypatch) + captured = _isolated_output_sinks(monkeypatch) class _Callback: async def SetResult(self, jid: str, dto: Any) -> bool: @@ -359,7 +394,7 @@ def test_install_is_a_noop_without_the_runtime(monkeypatch): def test_connect_installs_sinks_and_disconnect_clears(monkeypatch): pytest.importorskip("uipath_ipc") - captured = _fake_output_sinks(monkeypatch) + captured = _isolated_output_sinks(monkeypatch) class _Api(_job_api.IJobInvocationCommonApi): async def SendLog(self, jobId: str, log: Any) -> None: @@ -388,7 +423,7 @@ async def scenario() -> None: def test_connect_fails_loudly_when_the_pipe_is_unreachable(monkeypatch): pytest.importorskip("uipath_ipc") - _fake_output_sinks(monkeypatch) + _isolated_output_sinks(monkeypatch) def live() -> int: return len( @@ -414,13 +449,13 @@ def test_connect_without_uipath_ipc_raises(monkeypatch): def test_a_broken_log_channel_is_reported_once_to_stderr(monkeypatch): - _fake_output_sinks(monkeypatch) + _isolated_output_sinks(monkeypatch) buf = io.StringIO() monkeypatch.setattr(sys, "__stderr__", buf) class _Callback: - async def SendLog(self, job_id, log): + async def SendLog(self, job_id: str, log: Any) -> None: raise RuntimeError("pipe is gone") loop = asyncio.new_event_loop() @@ -445,6 +480,25 @@ async def SendLog(self, job_id, log): assert out.count("no longer reaching the handler") == 1, out +def test_a_cancelled_send_is_reported_like_any_other_failure(monkeypatch): + """`future.exception()` RAISES for a cancelled future, so this arm needs its own case.""" + _isolated_output_sinks(monkeypatch) + buf = io.StringIO() + monkeypatch.setattr(sys, "__stderr__", buf) + + loop = asyncio.new_event_loop() + try: + handler = _job_api._IpcLogHandler(JOB_ID, object(), loop) + cancelled: Future[object] = Future() + assert cancelled.cancel() + + handler._settled(cancelled) + finally: + loop.close() + + assert "no longer reaching the handler" in buf.getvalue() + + def _live_ipc_threads() -> int: return len( [ @@ -455,7 +509,18 @@ def _live_ipc_threads() -> int: ) -@pytest.mark.parametrize("job_id", [None, "", " ", "job-1", "not-a-guid"]) +@pytest.mark.parametrize( + "job_id", + [ + None, + "", + " ", + "job-1", + "not-a-guid", + "00000000-0000-0000-0000-000000000000", # Guid.Empty: parses, routes nowhere + "00000000-0000-0000-0000-00000000000", # a digit short: malformed, not empty + ], +) def test_connect_without_a_real_job_id_fails_fast(job_id): before = _live_ipc_threads() with pytest.raises(RuntimeError, match="UIPATH_JOB_KEY"): @@ -540,7 +605,7 @@ def test_result_sink_delivers_when_invoked_on_the_caller_loop_thread(monkeypatch caller's loop thread against a real in-proc server and assert SetResult actually arrived. """ pytest.importorskip("uipath_ipc") - captured = _fake_output_sinks(monkeypatch) + captured = _isolated_output_sinks(monkeypatch) received: dict[str, Any] = {} class _Api(_job_api.IJobInvocationCommonApi): @@ -563,6 +628,12 @@ class _Result: async def scenario() -> None: conn = _job_api.connect_handler_ipc(pipe, JOB_ID_2) + # A log line has to survive the round trip too, not just the result. + captured["handler"].emit( + logging.LogRecord( + "job", logging.WARNING, "p", 1, "over %s", ("ipc",), None + ) + ) # Call the sink ON this loop's thread, exactly as the runtime's __exit__ does. Before the # fix this deadlocked the loop the ack was scheduled on; now it completes. captured["sink"](_Result(), "out.args") @@ -572,6 +643,13 @@ async def scenario() -> None: finally: stop() + # F: the log half of the contract, asserted on the wire rather than against a fake. + assert "logs" in received, "SendLog never arrived over the pipe" + log_job_id, entry = received["logs"][0] + assert log_job_id == JOB_ID_2 + assert entry.Message == "over ipc" + assert entry.LogLevel == _job_api.LogLevel.WARNING.value + assert "result" in received, ( "SetResult never arrived — the result sink deadlocked/timed out" ) diff --git a/packages/uipath/tests/cli/test_run.py b/packages/uipath/tests/cli/test_run.py index df15d6d24..6167aab39 100644 --- a/packages/uipath/tests/cli/test_run.py +++ b/packages/uipath/tests/cli/test_run.py @@ -213,15 +213,46 @@ def test_taken_over_run_without_the_pipe_is_untouched( assert result.exit_code == 0 assert "Successful execution." in result.output - def test_a_run_nobody_claimed_reaches_the_pipe( - self, runner: CliRunner, temp_dir: str + def test_the_flag_is_hidden_but_still_accepted(self, runner: CliRunner): + """Handler-only: it must stay out of --help and the published docs, yet keep working.""" + assert ( + "handler-ipc-pipe" not in runner.invoke(cli, ["run", "--help"]).output + ) + # Hiding an option must not un-declare it. + assert ( + "No such option" + not in runner.invoke( + cli, ["run", "x", "--handler-ipc-pipe", "some-pipe"] + ).output + ) + + def test_the_job_runs_inside_the_open_connection( + self, runner: CliRunner, temp_dir: str, monkeypatch ): - opened: list[str] = [] + """Dialling the pipe is not enough: the run has to happen while it is open.""" + job_key = "3f2504e0-4f89-11d3-9a0c-0305e82c3301" + monkeypatch.setenv("UIPATH_JOB_KEY", job_key) + + events: list[str] = [] + seen: list[tuple[str, str]] = [] @asynccontextmanager async def _fake_connection(pipe, job_id): - opened.append(pipe) - yield None + seen.append((pipe, job_id)) + events.append("open") + try: + yield None + finally: + events.append("close") + + factory = _make_mock_factory(["my_agent"]) + runtime = factory.new_runtime.return_value + + async def _execute(*a, **k): + events.append("run") + return Mock(status="SUCCESSFUL") + + runtime.execute = AsyncMock(side_effect=_execute) with runner.isolated_filesystem(temp_dir=temp_dir): with ( @@ -231,7 +262,7 @@ async def _fake_connection(pipe, job_id): ), patch( "uipath._cli.cli_run.UiPathRuntimeFactoryRegistry.get", - return_value=_make_mock_factory(["my_agent"]), + return_value=factory, ), patch( "uipath._cli.cli_run.ResourceOverwritesContext", @@ -245,11 +276,13 @@ async def _fake_connection(pipe, job_id): cli, ["run", "--handler-ipc-pipe", "some-pipe"] ) - assert opened == ["some-pipe"], f"output: {result.output!r}" - assert result.exit_code == 0 + assert result.exit_code == 0, f"output: {result.output!r}" + # The whole point: the connection must still be open when the job runs. + assert events == ["open", "run", "close"], events + # And it must be told which job, not None. + assert seen == [("some-pipe", job_key)] assert "took over the run" not in result.output - class TestMiddleware: def test_autodiscover_entrypoint(self, runner: CliRunner, temp_dir: str): """When exactly one entrypoint exists, it is auto-resolved.""" with runner.isolated_filesystem(temp_dir=temp_dir): diff --git a/packages/uipath/tests/cli/test_server_ipc.py b/packages/uipath/tests/cli/test_server_ipc.py index 0462d4fb6..1fc04c19a 100644 --- a/packages/uipath/tests/cli/test_server_ipc.py +++ b/packages/uipath/tests/cli/test_server_ipc.py @@ -358,8 +358,8 @@ async def scenario() -> None: asyncio.run(scenario()) - @pytest.mark.parametrize("mode", ["no-client", "get_callback-raises"]) - def test_runjob_fails_loud_when_opted_in_but_no_callback(self, monkeypatch, mode): + def test_runjob_fails_loud_when_not_invoked_over_ipc(self, monkeypatch): + """Only reachable by a direct call: the dispatcher always injects a Message with a client.""" from uipath._cli import cli_server_ipc from uipath._cli.cli_server_ipc import PythonServerRunRequest @@ -372,15 +372,7 @@ async def _fake_run(*args: Any, **kwargs: Any) -> dict[str, Any]: monkeypatch.setattr(cli_server_ipc, "_run_command_isolated", _fake_run) message: "Message[None]" - if mode == "no-client": - message = Message() - else: - - class _Client: - def get_callback(self, contract: Any) -> Any: - raise RuntimeError("no callback available") - - message = Message(client=cast(Any, _Client())) + message = Message() service = PythonRuntimeService() request = PythonServerRunRequest( @@ -392,11 +384,11 @@ async def scenario() -> Any: result = asyncio.run(scenario()) assert result.ExitCode == 1 - assert "callback" in (result.Error or "").lower() + assert "over IPC" in (result.Error or "") assert ran == [] @pytest.mark.parametrize( - "job_key", ["", "job-9", "00000000-0000-0000-0000-00000000000"] + "job_key", ["", "job-9", "00000000-0000-0000-0000-000000000000"] ) def test_runjob_without_a_real_job_key_fails_loudly(self, monkeypatch, job_key): from uipath._cli import _job_api, cli_server_ipc From f880ae06c607d5a8c4dd81eaef38394fb168fcc5 Mon Sep 17 00:00:00 2001 From: Eduard Dumitru Date: Tue, 15 Sep 2026 13:07:03 +0200 Subject: [PATCH 18/18] fix(cli): tighten the IPC job-api after a third review pass Refuse to stream when the runtime has no output sinks. install_runtime_sinks swallowed the ImportError and returned None, and neither lane noticed: the connection came back live with no log handler and the job reported ExitCode 0, while the peer had already stopped watching execution.log on our promise. The whole job's output vanished and everything reported success. Teardown still tolerates an older runtime; only install refuses. Match the peer's frame cap. uipath-ipc caps an inbound frame at 2 MB, but every .NET server on this contract already accepts 30 -- PythonJobApiServer for the per-job pipe, IpcServerDefaults for the JS lane. Python was the only endpoint still at the default. Give the timeouts room. 30s for a call and 5s to drain in-flight sends were picked with nothing behind them. They are ceilings, not delays -- a healthy peer acks in under a millisecond -- so they only decide how long to hang on when the peer is slow. Nothing throttles emit and production p99 is 7,500 log lines a job (worst seen 58,694), so a backed-up peer can have thousands in flight when the job ends, and five seconds truncates exactly the tail worth having. 120s for all three. Three tests did not pin what they claimed: - An earlier commit deleted the line `class TestMiddleware:`, reparenting five unrelated middleware tests under TestHandlerIpcPipe, so `-k TestMiddleware` selected nothing. - The non-pooled deadlock regression passed WITH the deadlock, in 30.9s instead of 0.19s: every assertion sat after `await disconnect_handler_ipc`, so the sink burned its timeout, gave up, and the queued ack completed during that await. Assert before any await, and bound the call. - The pooled real-pipe test patched install_runtime_sinks away, so the callback it obtained was never used -- pointing get_callback at another contract kept the suite, mypy and ruff clean while every pooled send went to an endpoint the peer does not host. Install for real and assert a line arrives. Also: drive the public context manager rather than connect/disconnect, since cli_run only ever uses that; pin that the sinks are live when the runtime context snapshots them; and clear what the batching revert left behind. --- packages/uipath/src/uipath/_cli/_job_api.py | 30 ++++++++--- .../uipath/src/uipath/_cli/cli_server_ipc.py | 3 ++ packages/uipath/tests/cli/test_job_api.py | 54 ++++++++++++++----- packages/uipath/tests/cli/test_run.py | 29 ++++++++++ packages/uipath/tests/cli/test_server_ipc.py | 35 +++++++++--- 5 files changed, 126 insertions(+), 25 deletions(-) diff --git a/packages/uipath/src/uipath/_cli/_job_api.py b/packages/uipath/src/uipath/_cli/_job_api.py index 7b0d63445..1db129062 100644 --- a/packages/uipath/src/uipath/_cli/_job_api.py +++ b/packages/uipath/src/uipath/_cli/_job_api.py @@ -18,9 +18,16 @@ logger = logging.getLogger(__name__) -_SET_RESULT_TIMEOUT_S = 30.0 -_LOG_FLUSH_TIMEOUT_S = 5.0 -_IPC_REQUEST_TIMEOUT_S = 30.0 +# Generous on purpose, and a ceiling rather than a delay: a healthy peer acks in under a millisecond, +# so these only elapse when something is already wrong. Nothing throttles emit, so a chatty job can +# leave thousands of sends in flight at teardown -- a peer that is merely slow, not dead, still needs +# time to drain them, and the tail is the part of a log worth having. +_SET_RESULT_TIMEOUT_S = 120.0 +_LOG_FLUSH_TIMEOUT_S = 120.0 +_IPC_REQUEST_TIMEOUT_S = 120.0 +# Inbound frame cap. uipath-ipc defaults to 2 MB, but every .NET server on this contract already +# accepts 30 (PythonJobApiServer, IpcServerDefaults), so matching keeps the pair symmetric. +_MAX_MESSAGE_BYTES = 30 * 1024 * 1024 _NO_IPC_LOGGERS = ("uipath_ipc", __name__) @@ -224,15 +231,20 @@ def install_runtime_sinks( """Install the log + result sinks, forwarding to ``callback`` on ``loop``. ``loop`` must run on a different thread than the one the sinks are invoked on, or the result ack - deadlocks. No-op if the sinks aren't available. + deadlocks. Raises if this runtime has no sinks to install into. """ try: from uipath.runtime.output_sinks import ( set_log_handler, set_result_sink, ) - except ImportError: - return None + except ImportError as e: + # The caller has already stopped watching execution.log on our promise, so falling back to + # it would lose the whole job's output in silence. + raise RuntimeError( + "This uipath-runtime has no output sinks, so job logs cannot be streamed over IPC. " + "Install uipath-runtime>=0.13.5." + ) from e handler = _IpcLogHandler(job_id, callback, loop) handler.setFormatter(logging.Formatter("%(message)s")) @@ -357,7 +369,11 @@ async def _build() -> Any: f"Could not reach the handler IPC pipe {pipe!r}: {e}" ) from e writer.close() - client = IpcClient(transport=transport, request_timeout=_IPC_REQUEST_TIMEOUT_S) + client = IpcClient( + transport=transport, + request_timeout=_IPC_REQUEST_TIMEOUT_S, + max_message_size=_MAX_MESSAGE_BYTES, + ) proxy = client.get_proxy(IJobInvocationCommonApi) # type: ignore[type-abstract] return client, proxy diff --git a/packages/uipath/src/uipath/_cli/cli_server_ipc.py b/packages/uipath/src/uipath/_cli/cli_server_ipc.py index 6ef227885..cd489bd17 100644 --- a/packages/uipath/src/uipath/_cli/cli_server_ipc.py +++ b/packages/uipath/src/uipath/_cli/cli_server_ipc.py @@ -180,10 +180,13 @@ async def start_ipc_server(pipe_name: str) -> None: _ensure_runtime_initialized() + from ._job_api import _MAX_MESSAGE_BYTES + server = IpcServer( transport=NamedPipeServerTransport(pipe_name), services={IPythonRuntimeServer: PythonRuntimeService()}, request_timeout=None, # jobs are long-running; no server-side timeout + max_message_size=_MAX_MESSAGE_BYTES, ) console.success(f"IPC server listening on pipe '{pipe_name}'") async with server: diff --git a/packages/uipath/tests/cli/test_job_api.py b/packages/uipath/tests/cli/test_job_api.py index f31ca76a7..0e6918818 100644 --- a/packages/uipath/tests/cli/test_job_api.py +++ b/packages/uipath/tests/cli/test_job_api.py @@ -1,8 +1,8 @@ """The uipath-python job-api glue: result mapping and the runtime-sink installer. -The runtime side (``uipath.runtime.output_sinks``) is faked here so these tests exercise the wiring -in isolation — that install points a log handler + result sink at the callback, that the handler -forwards SendLog, and that the result sink maps the runtime result and calls SetResult. +The runtime's real ``output_sinks`` ContextVars are used, with a fresh pair per test: a dict would +record that a sink was installed but not in which context, and the context is the thing that decides +whether the job ever sees it. """ import asyncio @@ -11,6 +11,7 @@ import os import sys import threading +import time from concurrent.futures import Future from contextvars import ContextVar from typing import Any @@ -166,7 +167,7 @@ async def scenario() -> None: loop = asyncio.get_running_loop() _job_api.install_runtime_sinks("job-7", _Callback(), loop) - # The log handler batches records and forwards them tagged with the job id. + # The log handler forwards each record, tagged with the job id. handler = captured["handler"] handler.emit( logging.LogRecord("n", logging.WARNING, "p", 1, "hi %s", ("there",), None) @@ -385,10 +386,9 @@ def emit(self, record: logging.LogRecord) -> None: ) -def test_install_is_a_noop_without_the_runtime(monkeypatch): - # An older uipath-runtime has no output_sinks module: install/clear must not raise. +def test_clear_is_a_noop_without_the_runtime(monkeypatch): + # Teardown still has to work on an older runtime -- only install refuses (see the test below). monkeypatch.setitem(sys.modules, "uipath.runtime.output_sinks", None) - _job_api.install_runtime_sinks("j", object(), asyncio.new_event_loop()) _job_api.clear_runtime_sinks() @@ -408,11 +408,12 @@ async def SetResult(self, jobId: str, result: Any) -> bool: try: async def scenario() -> None: - conn = _job_api.connect_handler_ipc(pipe, JOB_ID) - assert captured["handler"] is not None - assert captured["sink"] is not None + # The context manager, not the two halves: cli_run.py only ever uses this, and a + # wiring mistake inside it would leave every assertion below untouched. + async with _job_api.handler_ipc_connection(pipe, JOB_ID): + assert captured["handler"] is not None + assert captured["sink"] is not None - await _job_api.disconnect_handler_ipc(conn) assert captured["handler"] is None assert captured["sink"] is None @@ -499,6 +500,22 @@ def test_a_cancelled_send_is_reported_like_any_other_failure(monkeypatch): assert "no longer reaching the handler" in buf.getvalue() +def test_missing_output_sinks_fails_loudly(monkeypatch): + """Silently returning None would lose the job: the peer has already stopped watching the file.""" + monkeypatch.setitem(sys.modules, "uipath.runtime.output_sinks", None) + + with pytest.raises(RuntimeError, match="uipath-runtime"): + _job_api.install_runtime_sinks(JOB_ID, object(), asyncio.new_event_loop()) + + +def test_the_frame_cap_matches_the_dotnet_peer(): + """Both .NET servers on this contract accept 30 MB; uipath-ipc would otherwise default to 2.""" + from uipath_ipc.wire import MAX_PAYLOAD_BYTES + + assert _job_api._MAX_MESSAGE_BYTES == 30 * 1024 * 1024 + assert _job_api._MAX_MESSAGE_BYTES > MAX_PAYLOAD_BYTES + + def _live_ipc_threads() -> int: return len( [ @@ -636,14 +653,27 @@ async def scenario() -> None: ) # Call the sink ON this loop's thread, exactly as the runtime's __exit__ does. Before the # fix this deadlocked the loop the ack was scheduled on; now it completes. + started = time.monotonic() captured["sink"](_Result(), "out.args") + elapsed = time.monotonic() - started + + # Assert BEFORE any await. Deadlocked, the sink burns _SET_RESULT_TIMEOUT_S and gives + # up; the queued ack then completes during the await below and the result shows up + # anyway, so anything asserted after an await passes either way. + assert "result" in received, ( + "SetResult did not complete on the caller's loop thread" + ) + assert elapsed < 5, ( + f"the sink blocked for {elapsed:.1f}s — the ack could not run" + ) + await _job_api.disconnect_handler_ipc(conn) asyncio.run(scenario()) finally: stop() - # F: the log half of the contract, asserted on the wire rather than against a fake. + # The log half of the contract, asserted on the wire rather than against a fake. assert "logs" in received, "SendLog never arrived over the pipe" log_job_id, entry = received["logs"][0] assert log_job_id == JOB_ID_2 diff --git a/packages/uipath/tests/cli/test_run.py b/packages/uipath/tests/cli/test_run.py index 6167aab39..a09b4ac33 100644 --- a/packages/uipath/tests/cli/test_run.py +++ b/packages/uipath/tests/cli/test_run.py @@ -1,5 +1,6 @@ # type: ignore import json +import logging import os from contextlib import asynccontextmanager from unittest.mock import AsyncMock, Mock, patch @@ -235,16 +236,38 @@ def test_the_job_runs_inside_the_open_connection( events: list[str] = [] seen: list[tuple[str, str]] = [] + snapshots: list[object] = [] + + from uipath.runtime import context as runtime_context + from uipath.runtime import output_sinks + + sentinel = logging.Handler() @asynccontextmanager async def _fake_connection(pipe, job_id): seen.append((pipe, job_id)) events.append("open") + # Install for real: the ordering that matters is against the runtime context's + # one-shot snapshot, not against runtime.execute. + output_sinks.set_log_handler(sentinel) try: yield None finally: + output_sinks.set_log_handler(None) events.append("close") + # UiPathRuntimeContext.__enter__ reads the sink exactly once; record what it saw. + real_get = runtime_context.get_log_handler + + def _spy_get_log_handler(): + value = real_get() + snapshots.append(value) + return value + + monkeypatch.setattr( + runtime_context, "get_log_handler", _spy_get_log_handler + ) + factory = _make_mock_factory(["my_agent"]) runtime = factory.new_runtime.return_value @@ -281,8 +304,14 @@ async def _execute(*a, **k): assert events == ["open", "run", "close"], events # And it must be told which job, not None. assert seen == [("some-pipe", job_key)] + # The connection must be open BEFORE the runtime context snapshots the sinks: move it + # inside and __enter__ would capture None, losing every log line and the result. + assert snapshots == [sentinel], ( + "the runtime context did not see the installed log sink" + ) assert "took over the run" not in result.output + class TestMiddleware: def test_autodiscover_entrypoint(self, runner: CliRunner, temp_dir: str): """When exactly one entrypoint exists, it is auto-resolved.""" with runner.isolated_filesystem(temp_dir=temp_dir): diff --git a/packages/uipath/tests/cli/test_server_ipc.py b/packages/uipath/tests/cli/test_server_ipc.py index 1fc04c19a..711e0fd58 100644 --- a/packages/uipath/tests/cli/test_server_ipc.py +++ b/packages/uipath/tests/cli/test_server_ipc.py @@ -13,6 +13,7 @@ import asyncio import json +import logging import os import sys import threading @@ -459,24 +460,41 @@ def test_pooled_streaming_works_over_a_real_pipe(self, monkeypatch): from uipath._cli import _job_api, cli_server_ipc installed: list[Any] = [] - monkeypatch.setattr( - _job_api, - "install_runtime_sinks", - lambda jid, cb, loop: installed.append(jid), - ) + real_install = _job_api.install_runtime_sinks + + def _record(jid, cb, loop): + installed.append(jid) + # Install for real: patching this away would leave the callback unused, so nothing + # would notice if RunJob asked the peer for the wrong contract. + return real_install(jid, cb, loop) + + monkeypatch.setattr(_job_api, "install_runtime_sinks", _record) async def _fake_run(cmd, args, env, wd, on_run_start=None, on_run_end=None): if on_run_start: on_run_start() + # Emit through the installed sink, so a log line actually crosses the pipe. + from uipath.runtime import output_sinks + + handler = cast(Any, output_sinks.get_log_handler()) + assert handler is not None, "the sinks were not installed for this job" + handler.emit( + logging.LogRecord( + "job", logging.INFO, "p", 1, "pooled %s", ("line",), None + ) + ) + handler.flush_pending(timeout=10.0) if on_run_end: on_run_end() return {"ExitCode": 0, "Error": None} monkeypatch.setattr(cli_server_ipc, "_run_command_isolated", _fake_run) + delivered: list[tuple[str, Any]] = [] + class _Callback: async def SendLog(self, job_id: str, log: Any) -> None: - return None + delivered.append((job_id, log)) async def SetResult(self, job_id: str, result: Any) -> bool: return True @@ -509,6 +527,11 @@ async def scenario() -> Any: assert result.Error is None, result.Error assert result.ExitCode == 0 assert installed == [JOB_ID] + # The contract passed to get_callback IS the endpoint key on the wire: ask for the wrong + # one and every send is addressed to something the peer does not host. + assert len(delivered) == 1, "no log line crossed the pooled callback" + assert delivered[0][0] == JOB_ID + assert delivered[0][1].Message == "pooled line" def test_runjob_installs_the_sinks_from_the_request_callback(self, monkeypatch): from uipath._cli import _job_api, cli_server_ipc