From b93360f673bc26c669072727d004100c4e96c381 Mon Sep 17 00:00:00 2001 From: jonahgabriel Date: Wed, 12 Aug 2026 17:30:01 -0400 Subject: [PATCH 1/5] fix(OMN-15959): delete zombie handler_llm_cli_subprocess.py from node_llm_inference_effect OMN-13250 (Phase D, Done 2026-07-15) claimed node_llm_inference_effect had been stripped to HTTP-only and handler_cross_cli_invoker.py deleted. Neither was true: handler_llm_cli_subprocess.py was still live with all three CLI-shell-out operations (inference.gemini_cli/claude_cli/opencode_cli) and 40 passing tests. OMN-13215 had already killed the architecture this handler served (omnimarket routing_tiers.yaml:27 -- "shelled codex-cli/cli_agents tiers were REMOVED"); the handler was the stump. - Delete handler_llm_cli_subprocess.py, its 3 operations from node_llm_inference_effect/contract.yaml, and its 40 tests (3 files). - Remove the dead validation_exemptions.yaml entry for the deleted file. - Fix a stale hostile-reviewer.yml doc-comment that named this handler as the review-pairing transport; omniintelligence.review_pairing.cli_review never actually imported it (grep-verified zero hits in omniintelligence). - Update docstrings in handler_coding_agent_invoke.py, handler_wiring.py, and enum_cli_backend_status.py that referenced the now-deleted module. - Repoint 2 dead-file references in compliance/contract-sync tests and genericize 2 test fixtures that used the deleted handler's symbol/op names as illustrative examples (test_wiring.py), preserving coverage of the general "shared handler, multiple operations" wiring behavior they guard (OMN-9461) without naming the deleted class. Net -1396/+53 lines, zero new surface. The sanctioned coding-agent CLI shell-out surface is node_coding_agent_invoke_effect (descriptor.agent_invocation: true) -- and only that node, in exactly one place in omnibase_infra now. --- .github/workflows/hostile-reviewer.yml | 12 +- .../coding_agent/enum_cli_backend_status.py | 11 +- .../handlers/handler_coding_agent_invoke.py | 9 +- .../node_llm_inference_effect/contract.yaml | 42 +- .../handlers/handler_llm_cli_subprocess.py | 316 --------- .../runtime/auto_wiring/handler_wiring.py | 14 +- .../validation/validation_exemptions.yaml | 6 - .../test_handler_autowiring_compliance.py | 8 - ..._handler_autowiring_compliance_followup.py | 18 +- .../scripts/test_validate_pr_contract_sync.py | 2 +- .../test_handler_llm_cli_subprocess.py | 163 ----- ...dler_llm_cli_subprocess_claude_opencode.py | 195 ------ ...st_handler_llm_cli_subprocess_execution.py | 633 ------------------ tests/unit/runtime/auto_wiring/test_wiring.py | 20 +- 14 files changed, 53 insertions(+), 1396 deletions(-) delete mode 100644 src/omnibase_infra/nodes/node_llm_inference_effect/handlers/handler_llm_cli_subprocess.py delete mode 100644 tests/unit/nodes/node_llm_inference_effect/handlers/test_handler_llm_cli_subprocess.py delete mode 100644 tests/unit/nodes/node_llm_inference_effect/handlers/test_handler_llm_cli_subprocess_claude_opencode.py delete mode 100644 tests/unit/nodes/node_llm_inference_effect/handlers/test_handler_llm_cli_subprocess_execution.py diff --git a/.github/workflows/hostile-reviewer.yml b/.github/workflows/hostile-reviewer.yml index 1ce2b8ddf4..a58149454e 100644 --- a/.github/workflows/hostile-reviewer.yml +++ b/.github/workflows/hostile-reviewer.yml @@ -5,9 +5,13 @@ # # Adversarial multi-model code review gate on every pull request (OMN-8468). # -# Invokes omniintelligence.review_pairing.cli_review (node-based path via -# HandlerLlmCliSubprocess / adapter_ai_reviewer) with the two currently-live -# local models (qwen3-review on the 5090, qwen3-review-b on the 4090 -- +# Invokes omniintelligence.review_pairing.cli_review directly (a standalone +# module calling model endpoints over HTTP; it does not import or route +# through any omnibase_infra handler -- OMN-15959 corrected this stale +# doc-comment, which named a since-deleted HandlerLlmCliSubprocess class that +# omniintelligence.review_pairing.cli_review never actually imported) with +# the two currently-live local models (qwen3-review on the 5090, +# qwen3-review-b on the 4090 -- # OMN-14176; deepseek-r1 no longer serves on the local fleet). Codex is # excluded from the initial pilot -- add --model codex once non-interactive # OAuth auth is confirmed on the omnibase-ci runner (OMN-8524). @@ -407,7 +411,7 @@ jobs: '| `blocked` | CRITICAL findings found | **Yes** |', '| `degraded` | All models unavailable (infra) | No (pilot) |', '', - '_Powered by [omniintelligence.review_pairing.cli_review](https://github.com/OmniNode-ai/omniintelligence) — node-based adversarial review via HandlerLlmCliSubprocess (OMN-8468/OMN-8524)_', + '_Powered by [omniintelligence.review_pairing.cli_review](https://github.com/OmniNode-ai/omniintelligence) — multi-model adversarial review (OMN-8468/OMN-8524)_', ].join('\n'); const { data: comments } = await github.rest.issues.listComments({ diff --git a/src/omnibase_infra/models/coding_agent/enum_cli_backend_status.py b/src/omnibase_infra/models/coding_agent/enum_cli_backend_status.py index 9122e198c4..088299a0ea 100644 --- a/src/omnibase_infra/models/coding_agent/enum_cli_backend_status.py +++ b/src/omnibase_infra/models/coding_agent/enum_cli_backend_status.py @@ -2,11 +2,12 @@ # SPDX-License-Identifier: MIT """Structured CLI subprocess failure classes for coding-agent invocations. -Mirrors the failure vocabulary in -``node_llm_inference_effect.handlers.handler_llm_cli_subprocess.EnumCliBackendStatus``. -Phase D (OMN-13250) collapses the duplicate by relocating the inference handler; -until then the coding-agent surface owns its own copy so the invoke effect does -not import a sibling node's private handler module. +OMN-15959: the sibling copy of this vocabulary in +``node_llm_inference_effect.handlers.handler_llm_cli_subprocess`` was deleted +(OMN-13250's Phase D DoD claimed that collapse had already happened; it had +not — the handler was still live with 40 tests). This module is now the sole +owner of the CLI-backend failure vocabulary; the coding-agent surface does not +import a sibling node's private handler module. """ from __future__ import annotations diff --git a/src/omnibase_infra/nodes/node_coding_agent_invoke_effect/handlers/handler_coding_agent_invoke.py b/src/omnibase_infra/nodes/node_coding_agent_invoke_effect/handlers/handler_coding_agent_invoke.py index 576a688324..1baeac17b6 100644 --- a/src/omnibase_infra/nodes/node_coding_agent_invoke_effect/handlers/handler_coding_agent_invoke.py +++ b/src/omnibase_infra/nodes/node_coding_agent_invoke_effect/handlers/handler_coding_agent_invoke.py @@ -2,10 +2,11 @@ # SPDX-License-Identifier: MIT """Coding-agent invoke EFFECT handler (OMN-13247, plan §5.1 / §5.7). -The ONLY I/O node in the workflow. Ported from -``node_llm_inference_effect.handlers.handler_llm_cli_subprocess`` and upgraded -from read-only single-shot inference to a workspace-write agentic invocation with -git-derived diff capture: +The ONLY I/O node in the workflow. Originally ported from +``node_llm_inference_effect.handlers.handler_llm_cli_subprocess`` (deleted +under OMN-15959 — that handler's shell-out architecture was superseded by this +node, not merely duplicated) and upgraded from read-only single-shot inference +to a workspace-write agentic invocation with git-derived diff capture: - ``shutil.which`` availability gate (UNAVAILABLE if the CLI is missing). - structured failure classes (UNAVAILABLE / TIMEOUT / SUBPROCESS_ERROR / diff --git a/src/omnibase_infra/nodes/node_llm_inference_effect/contract.yaml b/src/omnibase_infra/nodes/node_llm_inference_effect/contract.yaml index b56e5a2067..a37ad747fa 100644 --- a/src/omnibase_infra/nodes/node_llm_inference_effect/contract.yaml +++ b/src/omnibase_infra/nodes/node_llm_inference_effect/contract.yaml @@ -1,27 +1,24 @@ # SPDX-FileCopyrightText: 2025 OmniNode.ai Inc. # SPDX-License-Identifier: MIT # OMN-8735: handler constructor compliance refactor (2026-04-15) -# OMN-8735 follow-up: HandlerLlmCliSubprocess null-guard compliance (2026-04-16) # OMN-12816: additive extra_body passthrough — provider-specific request-body # fields (e.g. chat_template_kwargs.enable_thinking) merged in _build_payload # without overriding declared fields (2026-06-07) -# OMN-13158: HandlerLlmCliSubprocess resolves OAuth/subscription CLI backends -# from request.model (claude-cli, gemini-cli, opencode-cli). -# OMN-13215: the codex-cli shell-execution path was REMOVED. The delegation -# ceiling executes over the canonical HTTP inference path; no codex subprocess -# operation or backend remains. # OMN-13204: HandlerLlmOpenaiCompatible Protocol Conformance Note corrected to # the real protocol I/O types — ProtocolHandler.execute(ModelProtocolRequest) # -> ModelProtocolResponse and ProtocolMessageHandler.handle(ModelEventEnvelope) # -> ModelHandlerOutput — replacing the deleted ModelOnexEnvelope reference # (B5 residual of OMN-13196; deploy-gate runtime-path docstring edit, behavior # unchanged). (2026-06-17) -# OMN-14804: HandlerLlmCliSubprocess now exposes the canonical async handle() -# dispatch entrypoint for gemini/claude/opencode CLI operations; the retained -# execute_cli_inference accessor delegates to the same owned core. (2026-07-19) # OMN-15115: HandlerLlmOpenaiCompatible accepts per-request max_retries and the # OpenAI-compatible request model permits timeout_seconds up to 1800s for slow # local endpoints measured below the old 600s ceiling. (2026-07-25) +# OMN-15959: HandlerLlmCliSubprocess and its inference.{gemini,claude,opencode}_cli +# shell-out operations were DELETED. OMN-13250 (Phase D) claimed this handler +# was already stripped to HTTP-only; it was not — the file, its 3 operations, +# and 40 tests were still live. The sanctioned CLI shell-out surface is +# node_coding_agent_invoke_effect (descriptor.agent_invocation: true) only. +# (2026-08-12) # Copyright (c) 2026 OmniNode Team # # ONEX Node Contract @@ -36,8 +33,8 @@ contract_name: "node_llm_inference_effect" node_name: "node_llm_inference_effect" contract_version: major: 1 - minor: 4 - patch: 4 + minor: 5 + patch: 0 node_version: major: 0 minor: 1 @@ -87,27 +84,6 @@ handler_routing: module: "omnibase_infra.nodes.node_llm_inference_effect.handlers.handler_llm_openai_compatible" handler_type: "HTTP" description: "Execute LLM inference via OpenAI-compatible API (vLLM, OpenAI, GLM/Z.AI, etc.) with per-request timeout_seconds up to 1800s and max_retries passthrough." - # Gemini CLI Subprocess Handler - - operation: "inference.gemini_cli" - handler: - name: "HandlerLlmCliSubprocess" - module: "omnibase_infra.nodes.node_llm_inference_effect.handlers.handler_llm_cli_subprocess" - description: "Execute LLM inference via Gemini CLI subprocess (gemini -p)" - # OMN-13215: the Codex CLI subprocess operation (inference.codex_cli) was - # REMOVED. The delegation ceiling executes over the canonical HTTP inference - # path (HandlerLlmOpenaiCompatible) — no codex shell-out remains. - # Claude CLI Subprocess Handler (OMN-10137) - - operation: "inference.claude_cli" - handler: - name: "HandlerLlmCliSubprocess" - module: "omnibase_infra.nodes.node_llm_inference_effect.handlers.handler_llm_cli_subprocess" - description: "Execute LLM inference via Claude CLI subprocess (claude -p --output-format json)" - # opencode CLI Subprocess Handler (OMN-10137) - - operation: "inference.opencode_cli" - handler: - name: "HandlerLlmCliSubprocess" - module: "omnibase_infra.nodes.node_llm_inference_effect.handlers.handler_llm_cli_subprocess" - description: "Execute LLM inference via opencode CLI subprocess (opencode run --format json --pure)" execution_mode: "single" max_timeout_seconds: 1800 # ============================================================================= @@ -224,7 +200,7 @@ metadata: author: "OmniNode Team" license: "MIT" created: "2026-02-13" - updated: "2026-07-19" + updated: "2026-08-12" tags: - effect - llm diff --git a/src/omnibase_infra/nodes/node_llm_inference_effect/handlers/handler_llm_cli_subprocess.py b/src/omnibase_infra/nodes/node_llm_inference_effect/handlers/handler_llm_cli_subprocess.py deleted file mode 100644 index eb3449232b..0000000000 --- a/src/omnibase_infra/nodes/node_llm_inference_effect/handlers/handler_llm_cli_subprocess.py +++ /dev/null @@ -1,316 +0,0 @@ -# SPDX-FileCopyrightText: 2025 OmniNode.ai Inc. -# SPDX-License-Identifier: MIT - -"""CLI subprocess handler for LLM inference via Gemini / Claude / opencode CLI. - -Generalizes the proven subprocess dispatch pattern from the hostile reviewer -aggregator (aggregate_reviews.py) into an ONEX handler that accepts -ModelLlmInferenceRequest and returns ModelLlmInferenceResponse. - -CLI subprocess handlers distinguish these failure classes: -- UNAVAILABLE: CLI binary not found on PATH -- INVALID_REQUEST: empty prompt or malformed input -- TIMEOUT: subprocess exceeded deadline -- SUBPROCESS_ERROR: non-zero exit code with stderr -- EMPTY_RESPONSE: process succeeded but stdout was empty -- SUCCESS: valid response returned - -Related: - - OMN-7106: Add Gemini CLI and Codex CLI as subprocess LLM handlers - - OMN-7103: Node-Based LLM Delegation Workflow - - OMN-13215: codex-cli REMOVED — the delegation ceiling executes over the - canonical HTTP inference path; no codex subprocess config remains here. -""" - -from __future__ import annotations - -import logging -import shutil -import subprocess -import time -from datetime import UTC -from enum import Enum -from uuid import uuid4 - -from omnibase_core.models.dispatch.model_handler_output import ModelHandlerOutput -from omnibase_infra.enums import ( - EnumHandlerType, - EnumHandlerTypeCategory, - EnumInfraTransportType, -) -from omnibase_infra.errors import InfraUnavailableError, ModelInfraErrorContext -from omnibase_infra.models.llm.model_llm_inference_request import ( - ModelLlmInferenceRequest, -) -from omnibase_infra.models.llm.model_llm_inference_response import ( - ModelLlmInferenceResponse, -) - -logger = logging.getLogger(__name__) - - -class EnumCliBackendStatus(str, Enum): - """Structured failure classes for CLI subprocess handlers.""" - - SUCCESS = "success" - UNAVAILABLE = "unavailable" - INVALID_REQUEST = "invalid_request" - TIMEOUT = "timeout" - SUBPROCESS_ERROR = "subprocess_error" - EMPTY_RESPONSE = "empty_response" - - -# OMN-13215: the ``codex-cli`` model config was REMOVED. The delegation ceiling -# executes over the canonical HTTP inference path — no codex shell-out remains. -_CLI_CONFIG_BY_MODEL: dict[str, tuple[str, list[str]]] = { - "gemini-cli": ("gemini", ["-p"]), - "claude-cli": ( - "claude", - [ - "-p", - "--output-format", - "text", - "--permission-mode", - "dontAsk", - "--no-session-persistence", - ], - ), - "opencode-cli": ("opencode", ["run", "--format", "json", "--pure"]), -} - - -class HandlerLlmCliSubprocess: - """Dispatch LLM inference to a CLI tool (gemini, codex) via subprocess. - - This handler spawns the CLI in headless/non-interactive mode, - passes the user prompt, captures stdout as the response, and wraps it - in a ModelLlmInferenceResponse. - - ``handle`` is the canonical def-B dispatch entrypoint the auto-wired runtime - binds; it returns the response as a ``ModelHandlerOutput`` effect event or - fails fast on a non-SUCCESS backend status. ``execute_cli_inference`` exposes - the same owned core as the ``(response, status, detail)`` triple, preserving - the structured ``EnumCliBackendStatus`` classification for fallback routing and - metrics callers. - - Example: - >>> handler = HandlerLlmCliSubprocess(cli="gemini", cli_args=["-p"]) - >>> output = await handler.handle(request) # canonical dispatch entrypoint - >>> response, status, detail = handler.execute_cli_inference(request) - >>> if status == EnumCliBackendStatus.SUCCESS: - ... print(response.generated_text) - """ - - def __init__( - self, - cli: str | None = None, - cli_args: list[str] | None = None, - timeout: int = 120, - ) -> None: - self._cli = cli - self._cli_args = cli_args or ["-p"] - self._timeout = timeout - - @property - def cli_name(self) -> str | None: - """Return the CLI binary name, or None if not configured.""" - return self._cli - - @property - def handler_type(self) -> EnumHandlerType: - """Architectural role classification.""" - return EnumHandlerType.INFRA_HANDLER - - @property - def handler_category(self) -> EnumHandlerTypeCategory: - """Behavioral classification.""" - return EnumHandlerTypeCategory.EFFECT - - async def handle( - self, - request: ModelLlmInferenceRequest, - ) -> ModelHandlerOutput[None]: - """Canonical dispatch entrypoint (def B) for the auto-wired runtime. - - OMN-14804 (child of OMN-14510 missing-handle burn-down): before this - method existed, ``HandlerLlmCliSubprocess`` was contract-declared on three - CLI operations (``inference.gemini_cli`` / ``inference.claude_cli`` / - ``inference.opencode_cli``) yet exposed only ``execute_cli_inference()``. - Auto-wiring's ``_make_dispatch_callback`` then bound ``_missing_handle``, - which raised ``ModelOnexError`` on the FIRST dispatch while the contract - validated, the node booted, and CI stayed green. - - ``handle`` owns the runtime dispatch contract for this EFFECT node: it runs - CLI-subprocess inference through the owned ``_run_cli_inference`` core and - returns the produced ``ModelLlmInferenceResponse`` as the node's output - event, or fails fast with ``InfraUnavailableError`` when the CLI backend - cannot produce a usable response (mirroring the fail-fast transport error in - the OMN-14489 reference ``HandlerA2ATask.submit``). The structured - ``(response, status, detail)`` triple — carrying the ``EnumCliBackendStatus`` - classification for fallback-routing callers — stays available via - ``execute_cli_inference``; both public methods consume the same core. - """ - response, status, detail = self._run_cli_inference(request) - if response is None or status is not EnumCliBackendStatus.SUCCESS: - raise InfraUnavailableError( - f"CLI inference produced no usable response [{status.value}]: {detail}", - context=ModelInfraErrorContext.with_correlation( - correlation_id=request.correlation_id, - transport_type=EnumInfraTransportType.RUNTIME, - operation="cli_inference", - ), - ) - return ModelHandlerOutput.for_effect( - input_envelope_id=uuid4(), - correlation_id=response.correlation_id, - handler_id=type(self).__name__, - events=(response,), - processing_time_ms=response.latency_ms, - ) - - def execute_cli_inference( - self, - request: ModelLlmInferenceRequest, - ) -> tuple[ModelLlmInferenceResponse | None, EnumCliBackendStatus, str]: - """Structured-status view over the owned CLI-subprocess inference core. - - Returns the ``(response, status, detail)`` triple so fallback-routing and - metrics callers can branch on the ``EnumCliBackendStatus`` classification. - ``handle`` is the canonical auto-wired dispatch entrypoint; this method and - ``handle`` both delegate to the same ``_run_cli_inference`` core, so their - behavior can never diverge. - - Returns: - Tuple of (response, status, detail). Status is always set even - on failure, preserving structured failure information. - """ - return self._run_cli_inference(request) - - def _run_cli_inference( - self, - request: ModelLlmInferenceRequest, - ) -> tuple[ModelLlmInferenceResponse | None, EnumCliBackendStatus, str]: - """Owned CLI-subprocess inference core (spawn + structured classification). - - Resolves the CLI backend from the handler config or the request model, - spawns the subprocess, and classifies the outcome into one of the - ``EnumCliBackendStatus`` failure classes or SUCCESS. Consumed by both - ``handle`` (dispatch entrypoint) and ``execute_cli_inference`` - (structured-status accessor). - """ - cli = self._cli - cli_args = self._cli_args - if cli is None: - resolved = _CLI_CONFIG_BY_MODEL.get(request.model) - if resolved is not None: - cli, cli_args = resolved - - if cli is None: - return ( - None, - EnumCliBackendStatus.UNAVAILABLE, - "cli not configured (no CLI binary specified)", - ) - - if not shutil.which(cli): - return ( - None, - EnumCliBackendStatus.UNAVAILABLE, - f"{cli} not found on PATH", - ) - - # Extract last user message as prompt - prompt = "" - for msg in reversed(request.messages): - if msg.role == "user": - prompt = msg.content or "" - break - - if not prompt: - return ( - None, - EnumCliBackendStatus.INVALID_REQUEST, - "no user message in request", - ) - - try: - start = time.monotonic() - result = subprocess.run( - [cli, *cli_args, prompt], - capture_output=True, - text=True, - timeout=self._timeout, - check=False, - ) - latency_ms = (time.monotonic() - start) * 1000 - - if result.returncode != 0: - stderr_preview = result.stderr[:200] if result.stderr else "(no stderr)" - logger.debug("%s exited %d: %s", cli, result.returncode, stderr_preview) - return ( - None, - EnumCliBackendStatus.SUBPROCESS_ERROR, - f"exit {result.returncode}: {stderr_preview}", - ) - - content = result.stdout.strip() - if not content: - return ( - None, - EnumCliBackendStatus.EMPTY_RESPONSE, - "stdout empty after successful exit", - ) - - # Build response with all required fields - from datetime import datetime - - from omnibase_infra.enums import ( - EnumLlmFinishReason, - EnumLlmOperationType, - ) - from omnibase_infra.models.llm.model_llm_usage import ModelLlmUsage - from omnibase_infra.models.model_backend_result import ModelBackendResult - - # Rough token estimate: ~1.3 tokens per word - prompt_tokens = int(len(prompt.split()) * 1.3) - completion_tokens = int(len(content.split()) * 1.3) - - response = ModelLlmInferenceResponse( - status="success", - generated_text=content, - model_used=f"{cli}-cli", - operation_type=EnumLlmOperationType.CHAT_COMPLETION, - finish_reason=EnumLlmFinishReason.STOP, - usage=ModelLlmUsage( - tokens_input=prompt_tokens, - tokens_output=completion_tokens, - tokens_total=prompt_tokens + completion_tokens, - ), - latency_ms=latency_ms, - backend_result=ModelBackendResult(success=True, duration_ms=latency_ms), - correlation_id=getattr(request, "correlation_id", uuid4()), - execution_id=uuid4(), - timestamp=datetime.now(tz=UTC), - ) - - logger.info( - "%s-cli: completed in %.0fms (~%d tokens)", - cli, - latency_ms, - completion_tokens, - ) - - return (response, EnumCliBackendStatus.SUCCESS, "") - - except subprocess.TimeoutExpired: - return ( - None, - EnumCliBackendStatus.TIMEOUT, - f"{cli} exceeded {self._timeout}s deadline", - ) - - -__all__ = [ - "EnumCliBackendStatus", - "HandlerLlmCliSubprocess", -] diff --git a/src/omnibase_infra/runtime/auto_wiring/handler_wiring.py b/src/omnibase_infra/runtime/auto_wiring/handler_wiring.py index 14bd1f1117..ce36805e30 100644 --- a/src/omnibase_infra/runtime/auto_wiring/handler_wiring.py +++ b/src/omnibase_infra/runtime/auto_wiring/handler_wiring.py @@ -4862,11 +4862,11 @@ def _derive_route_id( that share a common segment (OMN-8735). When two routing entries reference the same handler class for different - operations (e.g. ``HandlerLlmCliSubprocess`` for both ``inference.gemini_cli`` - and ``inference.codex_cli``) and subscribe to the same topic, the - ``handler + topic`` pair alone produces a collision. The handler entry key - includes the sanitized operation suffix when present, guaranteeing each - entry gets a distinct route ID (OMN-9461 / OMN-10447). + operations (e.g. one handler bound to both ``inference.variant_a`` and + ``inference.variant_b`` on the same contract) and subscribe to the same + topic, the ``handler + topic`` pair alone produces a collision. The + handler entry key includes the sanitized operation suffix when present, + guaranteeing each entry gets a distinct route ID (OMN-9461 / OMN-10447). """ safe_topic = re.sub(r"[.\-]", "_", topic) return f"route.auto.{contract_name}.{handler_key}.{safe_topic}" @@ -4876,8 +4876,8 @@ def _derive_dispatcher_id(contract_name: str, handler_key: str) -> str: """Derive a dispatcher ID from contract name and handler entry key. When two routing entries in the same contract reference the same handler - class (e.g. ``HandlerLlmCliSubprocess`` wired for both ``inference.gemini_cli`` - and ``inference.codex_cli``), the plain handler name alone produces a + class (e.g. one handler wired for both ``inference.variant_a`` and + ``inference.variant_b``), the plain handler name alone produces a collision. The entry key includes the sanitized operation suffix and keeps dispatcher IDs distinct (OMN-9461 / OMN-10447). """ diff --git a/src/omnibase_infra/validation/validation_exemptions.yaml b/src/omnibase_infra/validation/validation_exemptions.yaml index 82c678e436..1edebabbfc 100644 --- a/src/omnibase_infra/validation/validation_exemptions.yaml +++ b/src/omnibase_infra/validation/validation_exemptions.yaml @@ -2690,12 +2690,6 @@ pattern_exemptions: 'handle' is the canonical ONEX handler method name (definition B, OMN-14355/OMN-14407) that the shared runtime adapter binds. This node was refactored from the legacy scripts/check_release_identity.py gate into the canonical CONTRACT+NODE+HANDLER shape, which requires the core method be named 'handle'. ticket: OMN-14471 - - file_pattern: 'handler_llm_cli_subprocess\.py' - violation_pattern: "Function name 'handle' is too generic" - reason: > - 'handle' is the canonical ONEX def-B dispatch entrypoint name that auto-wiring's _make_dispatch_callback binds; a handler exposing neither handle nor handle_async is bound to _missing_handle and raises ModelOnexError on every dispatch (OMN-14510 missing-handle burn-down). HandlerLlmCliSubprocess was regenerated to def-B under OMN-14804 to add this entrypoint, matching the ProtocolMessageHandler convention used by every other node handler on node_llm_inference_effect. - - ticket: OMN-14804 - file_pattern: 'handler_auth_gate\.py' violation_pattern: "Function name 'handle' is too generic" reason: > diff --git a/tests/integration/handlers/test_handler_autowiring_compliance.py b/tests/integration/handlers/test_handler_autowiring_compliance.py index f835aa6ab5..9263685798 100644 --- a/tests/integration/handlers/test_handler_autowiring_compliance.py +++ b/tests/integration/handlers/test_handler_autowiring_compliance.py @@ -122,14 +122,6 @@ def test_handler_scope_manifest_write_complete_no_args(self) -> None: handler = HandlerScopeManifestWriteComplete() assert handler is not None - def test_handler_llm_cli_subprocess_no_args(self) -> None: - from omnibase_infra.nodes.node_llm_inference_effect.handlers.handler_llm_cli_subprocess import ( - HandlerLlmCliSubprocess, - ) - - handler = HandlerLlmCliSubprocess() - assert handler is not None - def test_handler_runtime_tick_no_args(self) -> None: from omnibase_infra.nodes.node_registration_orchestrator.handlers.handler_runtime_tick import ( HandlerRuntimeTick, diff --git a/tests/integration/test_handler_autowiring_compliance_followup.py b/tests/integration/test_handler_autowiring_compliance_followup.py index 20bb8e1071..c535bbbd1e 100644 --- a/tests/integration/test_handler_autowiring_compliance_followup.py +++ b/tests/integration/test_handler_autowiring_compliance_followup.py @@ -2,11 +2,15 @@ # SPDX-License-Identifier: MIT """Integration tests for OMN-8735 follow-up auto-wiring constructor compliance. -Verifies that HandlerLlmCliSubprocess and HandlerRuntimeTick can be instantiated -with no constructor arguments, as required by the strict auto-wiring framework. +Verifies that HandlerRuntimeTick can be instantiated with no constructor +arguments, as required by the strict auto-wiring framework. -These two handlers were missed in the initial OMN-8735 pass and are fixed in +This handler was missed in the initial OMN-8735 pass and is fixed in the follow-up PR (omnibase_infra#1325). + +OMN-15959: the sibling ``test_handler_llm_cli_subprocess_no_args`` case that +used to live in this class was removed along with the deleted +HandlerLlmCliSubprocess handler. """ from __future__ import annotations @@ -19,14 +23,6 @@ class TestHandlerAutowiringComplianceFollowup: """Verify OMN-8735 follow-up: missed handlers instantiate with no constructor arguments.""" - def test_handler_llm_cli_subprocess_no_args(self) -> None: - from omnibase_infra.nodes.node_llm_inference_effect.handlers.handler_llm_cli_subprocess import ( - HandlerLlmCliSubprocess, - ) - - handler = HandlerLlmCliSubprocess() - assert handler is not None - def test_handler_runtime_tick_no_args(self) -> None: from omnibase_infra.nodes.node_registration_orchestrator.handlers.handler_runtime_tick import ( HandlerRuntimeTick, diff --git a/tests/scripts/test_validate_pr_contract_sync.py b/tests/scripts/test_validate_pr_contract_sync.py index 73c530a645..9fcab62666 100644 --- a/tests/scripts/test_validate_pr_contract_sync.py +++ b/tests/scripts/test_validate_pr_contract_sync.py @@ -84,7 +84,7 @@ def test_node_handler_test_change_without_contract_change_passes(): """Unit tests under tests/unit/nodes are not production handler changes.""" result = run_script( [ - "tests/unit/nodes/node_llm_inference_effect/handlers/test_handler_llm_cli_subprocess_execution.py", + "tests/unit/nodes/node_llm_inference_effect/handlers/test_handler_llm_openai_compatible.py", ] ) assert result.returncode == 0, ( diff --git a/tests/unit/nodes/node_llm_inference_effect/handlers/test_handler_llm_cli_subprocess.py b/tests/unit/nodes/node_llm_inference_effect/handlers/test_handler_llm_cli_subprocess.py deleted file mode 100644 index 3596f9d88c..0000000000 --- a/tests/unit/nodes/node_llm_inference_effect/handlers/test_handler_llm_cli_subprocess.py +++ /dev/null @@ -1,163 +0,0 @@ -# SPDX-FileCopyrightText: 2025 OmniNode.ai Inc. -# SPDX-License-Identifier: MIT -"""Dispatch-entrypoint (def-B) tests for HandlerLlmCliSubprocess. - -OMN-8735 follow-up: auto-wiring compliance — no-args construction. -OMN-14804 (child of OMN-14510 missing-handle burn-down): HandlerLlmCliSubprocess - was contract-declared on ``node_llm_inference_effect`` for three CLI operations - (``inference.gemini_cli`` / ``inference.claude_cli`` / ``inference.opencode_cli``) - yet exposed ONLY its business method ``execute_cli_inference()`` — NEITHER - ``handle`` NOR ``handle_async``. Auto-wiring's ``_make_dispatch_callback`` binds - ``_missing_handle`` in that case, which raises ``ModelOnexError`` on the FIRST - real dispatch while the contract validates, the node boots, and CI stays green. - - These tests drive the REAL production dispatch callback over the REAL handler - class (no fake handler, no patched entrypoint). Before OMN-14804 they FAIL for - the missing-handle reason; they pass only once a genuine def-B ``handle`` exists. -""" - -from __future__ import annotations - -import subprocess -from unittest.mock import patch -from uuid import uuid4 - -import pytest - -from omnibase_core.models.dispatch.model_handler_output import ModelHandlerOutput -from omnibase_core.models.events.model_event_envelope import ModelEventEnvelope -from omnibase_infra.enums import EnumHandlerType, EnumHandlerTypeCategory -from omnibase_infra.models.llm.model_llm_inference_request import ( - ModelLlmInferenceRequest, -) -from omnibase_infra.models.llm.model_llm_inference_response import ( - ModelLlmInferenceResponse, -) -from omnibase_infra.models.llm.model_llm_message import ModelLlmMessage -from omnibase_infra.nodes.node_llm_inference_effect.handlers.handler_llm_cli_subprocess import ( - HandlerLlmCliSubprocess, -) -from omnibase_infra.runtime.auto_wiring.handler_wiring import _make_dispatch_callback - - -@pytest.mark.unit -def test_handler_llm_cli_subprocess_constructs_with_no_args() -> None: - """HandlerLlmCliSubprocess must construct with no arguments for auto-wiring.""" - handler = HandlerLlmCliSubprocess() - assert handler.handler_type == EnumHandlerType.INFRA_HANDLER - assert handler.handler_category == EnumHandlerTypeCategory.EFFECT - - -@pytest.mark.unit -def test_handler_exposes_a_dispatch_entrypoint() -> None: - """The bare invariant: auto-wiring can only bind ``handle`` / ``handle_async``. - - RED against the pre-OMN-14804 handler, which had only ``execute_cli_inference()``. - """ - assert callable(getattr(HandlerLlmCliSubprocess, "handle", None)) or callable( - getattr(HandlerLlmCliSubprocess, "handle_async", None) - ), ( - "HandlerLlmCliSubprocess exposes neither handle() nor handle_async(); " - "auto-wiring binds _missing_handle and every dispatch raises ModelOnexError." - ) - - -@pytest.mark.unit -@pytest.mark.asyncio -async def test_real_dispatch_callback_runs_cli_and_emits_response() -> None: - """LOAD-BEARING: a request dispatched through the REAL auto-wiring callback - reaches the handler and yields a ``ModelLlmInferenceResponse`` output event. - - The contract declares these CLI operations with NO ``event_model`` (they route - via ``operation_match``), so this exercises the untyped def-B arm (OMN-14716): - ``_make_dispatch_callback`` signature-introspects ``handle(request: ModelX)`` and - coerces the envelope payload into ``ModelLlmInferenceRequest`` at the adapter - boundary. Against the entrypoint-less handler this raises ``ModelOnexError`` - (``_missing_handle``) instead of returning a result — that raise IS the bug. - """ - callback = _make_dispatch_callback(HandlerLlmCliSubprocess()) - cid = uuid4() - request = ModelLlmInferenceRequest( - base_url="http://localhost:1", - model="gemini-cli", - messages=(ModelLlmMessage(role="user", content="What is 2+2?"),), - correlation_id=cid, - ) - envelope: ModelEventEnvelope[object] = ModelEventEnvelope( - payload=request, - correlation_id=cid, - event_type="ModelLlmInferenceRequest", - ) - completed = subprocess.CompletedProcess( - args=["gemini", "-p", "What is 2+2?"], - returncode=0, - stdout="Four\n", - stderr="", - ) - - with ( - patch("shutil.which", return_value="/usr/bin/gemini"), - patch("subprocess.run", return_value=completed), - ): - result = await callback(envelope) - - assert result is not None, "Dispatch produced no result — the handler never ran." - assert len(result.output_events) == 1, ( - "Expected exactly one ModelLlmInferenceResponse output event from the def-B " - f"CLI-subprocess handler. Got {result.output_events!r}" - ) - event = result.output_events[0] - assert isinstance(event, ModelLlmInferenceResponse) - assert event.generated_text == "Four" # stdout stripped - assert event.correlation_id == cid - - -@pytest.mark.unit -@pytest.mark.asyncio -async def test_dispatch_handler_receives_typed_request_not_raw_envelope() -> None: - """Seam check: the def-B ``handle`` receives a validated ModelLlmInferenceRequest, - NOT a raw envelope, on the untyped (operation_match / no event_model) arm. - - Before OMN-14716 the untyped arm handed the handler the raw ModelEventEnvelope, - which a def-B handler breaks on at first attribute access. This guards that the - ``ModelLlmInferenceRequest`` annotation is runtime-resolvable so the coercion - fires (it would silently no-op if the model were import-guarded behind - ``TYPE_CHECKING`` and the annotation failed ``eval_str`` resolution). - """ - seen: list[object] = [] - handler = HandlerLlmCliSubprocess() - original = handler.handle - - async def _spy(request: ModelLlmInferenceRequest) -> ModelHandlerOutput[None]: - seen.append(request) - return await original(request) - - handler.handle = _spy # type: ignore[method-assign] - callback = _make_dispatch_callback(handler) - cid = uuid4() - request = ModelLlmInferenceRequest( - base_url="http://localhost:1", - model="gemini-cli", - messages=(ModelLlmMessage(role="user", content="ping"),), - correlation_id=cid, - ) - envelope: ModelEventEnvelope[object] = ModelEventEnvelope( - payload=request, - correlation_id=cid, - event_type="ModelLlmInferenceRequest", - ) - completed = subprocess.CompletedProcess( - args=["gemini", "-p", "ping"], returncode=0, stdout="pong\n", stderr="" - ) - - with ( - patch("shutil.which", return_value="/usr/bin/gemini"), - patch("subprocess.run", return_value=completed), - ): - await callback(envelope) - - assert len(seen) == 1, "Handler was not invoked exactly once." - assert isinstance(seen[0], ModelLlmInferenceRequest), ( - "def-B handle received a raw envelope instead of a validated " - f"ModelLlmInferenceRequest: {type(seen[0]).__name__}." - ) diff --git a/tests/unit/nodes/node_llm_inference_effect/handlers/test_handler_llm_cli_subprocess_claude_opencode.py b/tests/unit/nodes/node_llm_inference_effect/handlers/test_handler_llm_cli_subprocess_claude_opencode.py deleted file mode 100644 index 33ebc21c5c..0000000000 --- a/tests/unit/nodes/node_llm_inference_effect/handlers/test_handler_llm_cli_subprocess_claude_opencode.py +++ /dev/null @@ -1,195 +0,0 @@ -# SPDX-FileCopyrightText: 2025 OmniNode.ai Inc. -# SPDX-License-Identifier: MIT -"""Unit tests for OAuth/headless CLI backend registration in omnibase_infra. - -OMN-10137: Register claude and opencode as CLI subprocess backends. -OMN-13158: Register Codex as the primary headless CLI subprocess backend. -""" - -from __future__ import annotations - -import subprocess -from pathlib import Path -from unittest.mock import patch - -import pytest -import yaml - - -@pytest.mark.unit -def test_claude_handler_is_registered_in_contract() -> None: - contract_path = ( - Path(__file__).parents[5] - / "src/omnibase_infra/nodes/node_llm_inference_effect/contract.yaml" - ) - data = yaml.safe_load(contract_path.read_text()) - ops = [h["operation"] for h in data["handler_routing"]["handlers"]] - assert "inference.claude_cli" in ops - - -@pytest.mark.unit -def test_opencode_handler_is_registered_in_contract() -> None: - contract_path = ( - Path(__file__).parents[5] - / "src/omnibase_infra/nodes/node_llm_inference_effect/contract.yaml" - ) - data = yaml.safe_load(contract_path.read_text()) - ops = [h["operation"] for h in data["handler_routing"]["handlers"]] - assert "inference.opencode_cli" in ops - - -@pytest.mark.unit -def test_claude_cli_subprocess_unavailable_when_binary_missing() -> None: - from omnibase_infra.models.llm.model_llm_inference_request import ( - ModelLlmInferenceRequest, - ) - from omnibase_infra.models.llm.model_llm_message import ModelLlmMessage - from omnibase_infra.nodes.node_llm_inference_effect.handlers.handler_llm_cli_subprocess import ( - EnumCliBackendStatus, - HandlerLlmCliSubprocess, - ) - - handler = HandlerLlmCliSubprocess( - cli="claude", cli_args=["-p", "--output-format", "json"] - ) - req = ModelLlmInferenceRequest( - base_url="http://localhost:1", - messages=[ModelLlmMessage(role="user", content="hi")], - model="claude-cli", - ) - with patch("shutil.which", return_value=None): - _, status, _ = handler.execute_cli_inference(req) - assert status == EnumCliBackendStatus.UNAVAILABLE - - -@pytest.mark.unit -def test_codex_cli_model_no_longer_resolves_a_subprocess_config() -> None: - """OMN-13215: the codex-cli shell-out config was removed. - - A request whose model is ``codex-cli`` must no longer resolve a CLI config — - the handler reports UNAVAILABLE (cli not configured) instead of spawning the - codex binary. The delegation ceiling runs over the canonical HTTP path. - """ - from omnibase_infra.models.llm.model_llm_inference_request import ( - ModelLlmInferenceRequest, - ) - from omnibase_infra.models.llm.model_llm_message import ModelLlmMessage - from omnibase_infra.nodes.node_llm_inference_effect.handlers.handler_llm_cli_subprocess import ( - _CLI_CONFIG_BY_MODEL, - EnumCliBackendStatus, - HandlerLlmCliSubprocess, - ) - - # The codex model has no CLI config mapping anymore. - assert "codex-cli" not in _CLI_CONFIG_BY_MODEL - - handler = HandlerLlmCliSubprocess() - req = ModelLlmInferenceRequest( - base_url="http://localhost:1", - messages=[ModelLlmMessage(role="user", content="hi")], - model="codex-cli", - ) - # No subprocess is spawned: with no resolved CLI, the handler fails closed. - with patch("subprocess.run") as mock_run: - _, status, detail = handler.execute_cli_inference(req) - assert status == EnumCliBackendStatus.UNAVAILABLE - assert "cli not configured" in detail - mock_run.assert_not_called() - - -@pytest.mark.unit -def test_claude_cli_subprocess_resolves_headless_oauth_argv_from_request_model() -> ( - None -): - from omnibase_infra.models.llm.model_llm_inference_request import ( - ModelLlmInferenceRequest, - ) - from omnibase_infra.models.llm.model_llm_message import ModelLlmMessage - from omnibase_infra.nodes.node_llm_inference_effect.handlers.handler_llm_cli_subprocess import ( - EnumCliBackendStatus, - HandlerLlmCliSubprocess, - ) - - handler = HandlerLlmCliSubprocess() - req = ModelLlmInferenceRequest( - base_url="http://localhost:1", - messages=[ModelLlmMessage(role="user", content="hi")], - model="claude-cli", - ) - completed = subprocess.CompletedProcess( - args=["claude"], - returncode=0, - stdout="claude response\n", - stderr="", - ) - with ( - patch("shutil.which", return_value="/opt/homebrew/bin/claude"), - patch("subprocess.run", return_value=completed) as mock_run, - ): - response, status, detail = handler.execute_cli_inference(req) - - assert status == EnumCliBackendStatus.SUCCESS - assert detail == "" - assert response is not None - assert response.generated_text == "claude response" - assert mock_run.call_args.args[0] == [ - "claude", - "-p", - "--output-format", - "text", - "--permission-mode", - "dontAsk", - "--no-session-persistence", - "hi", - ] - - -@pytest.mark.unit -def test_opencode_cli_subprocess_unavailable_when_binary_missing() -> None: - from omnibase_infra.models.llm.model_llm_inference_request import ( - ModelLlmInferenceRequest, - ) - from omnibase_infra.models.llm.model_llm_message import ModelLlmMessage - from omnibase_infra.nodes.node_llm_inference_effect.handlers.handler_llm_cli_subprocess import ( - EnumCliBackendStatus, - HandlerLlmCliSubprocess, - ) - - handler = HandlerLlmCliSubprocess( - cli="opencode", cli_args=["run", "--format", "json", "--pure"] - ) - req = ModelLlmInferenceRequest( - base_url="http://localhost:1", - messages=[ModelLlmMessage(role="user", content="hi")], - model="opencode-cli", - ) - with patch("shutil.which", return_value=None): - _, status, _ = handler.execute_cli_inference(req) - assert status == EnumCliBackendStatus.UNAVAILABLE - - -@pytest.mark.unit -def test_existing_gemini_cli_entry_still_present() -> None: - contract_path = ( - Path(__file__).parents[5] - / "src/omnibase_infra/nodes/node_llm_inference_effect/contract.yaml" - ) - data = yaml.safe_load(contract_path.read_text()) - ops = [h["operation"] for h in data["handler_routing"]["handlers"]] - assert "inference.gemini_cli" in ops - - -@pytest.mark.unit -def test_codex_cli_operation_removed_from_contract() -> None: - """OMN-13215: the inference.codex_cli operation was removed from the contract. - - The codex shell-out is gone; the ceiling executes over the canonical HTTP - inference path (HandlerLlmOpenaiCompatible). - """ - contract_path = ( - Path(__file__).parents[5] - / "src/omnibase_infra/nodes/node_llm_inference_effect/contract.yaml" - ) - data = yaml.safe_load(contract_path.read_text()) - ops = [h["operation"] for h in data["handler_routing"]["handlers"]] - assert "inference.codex_cli" not in ops diff --git a/tests/unit/nodes/node_llm_inference_effect/handlers/test_handler_llm_cli_subprocess_execution.py b/tests/unit/nodes/node_llm_inference_effect/handlers/test_handler_llm_cli_subprocess_execution.py deleted file mode 100644 index 4fc8afe903..0000000000 --- a/tests/unit/nodes/node_llm_inference_effect/handlers/test_handler_llm_cli_subprocess_execution.py +++ /dev/null @@ -1,633 +0,0 @@ -# SPDX-FileCopyrightText: 2025 OmniNode.ai Inc. -# SPDX-License-Identifier: MIT -"""Unit tests for HandlerLlmCliSubprocess.execute_cli_inference. - -OMN-13743: Raise coverage of handler_llm_cli_subprocess.py. - -Covers execute_cli_inference branches (previously uncovered): -- UNAVAILABLE when self._cli is None (no CLI binary specified) -- UNAVAILABLE when shutil.which returns None for an explicitly-set binary -- INVALID_REQUEST when no user-role message in request.messages -- INVALID_REQUEST when user message exists but content is None / empty -- SUBPROCESS_ERROR when subprocess.run returns non-zero exit code -- SUBPROCESS_ERROR stderr truncation and empty-stderr placeholder -- EMPTY_RESPONSE when stdout is empty after a successful exit -- SUCCESS with fully-constructed ModelLlmInferenceResponse -- SUCCESS token estimates scale with word count -- SUCCESS uses last user message when multiple messages present -- TIMEOUT when subprocess.TimeoutExpired is raised -""" - -from __future__ import annotations - -import subprocess -from unittest.mock import MagicMock, patch -from uuid import UUID, uuid4 - -import pytest - -from omnibase_infra.models.llm.model_llm_inference_request import ( - ModelLlmInferenceRequest, -) -from omnibase_infra.models.llm.model_llm_message import ModelLlmMessage -from omnibase_infra.nodes.node_llm_inference_effect.handlers.handler_llm_cli_subprocess import ( - EnumCliBackendStatus, - HandlerLlmCliSubprocess, -) - -# --------------------------------------------------------------------------- -# Helpers -# --------------------------------------------------------------------------- - - -def _req( - model: str = "gemini-cli", - user_content: str = "hello", - messages: tuple[ModelLlmMessage, ...] | None = None, - correlation_id: UUID | None = None, -) -> ModelLlmInferenceRequest: - """Build a minimal CHAT_COMPLETION request.""" - if messages is None: - messages = (ModelLlmMessage(role="user", content=user_content),) - kwargs: dict = { - "base_url": "http://localhost:1", - "model": model, - "messages": messages, - } - if correlation_id is not None: - kwargs["correlation_id"] = correlation_id - return ModelLlmInferenceRequest(**kwargs) - - -def _ok( - stdout: str = "the answer\n", returncode: int = 0, stderr: str = "" -) -> subprocess.CompletedProcess: # type: ignore[type-arg] - return subprocess.CompletedProcess( - args=["gemini", "-p", "hello"], - returncode=returncode, - stdout=stdout, - stderr=stderr, - ) - - -# --------------------------------------------------------------------------- -# UNAVAILABLE: cli is None -# --------------------------------------------------------------------------- - - -@pytest.mark.unit -def test_unavailable_when_cli_is_none() -> None: - """UNAVAILABLE when cli=None and the request model has no CLI mapping.""" - handler = HandlerLlmCliSubprocess() # cli=None (default) - req = _req(model="unmapped-cli") - - with patch("subprocess.run") as mock_run: - response, status, detail = handler.execute_cli_inference(req) - - assert response is None - assert status == EnumCliBackendStatus.UNAVAILABLE - assert "cli not configured" in detail - # subprocess.run must never be reached - mock_run.assert_not_called() - - -@pytest.mark.unit -def test_unavailable_detail_mentions_no_binary() -> None: - """UNAVAILABLE detail message is descriptive.""" - handler = HandlerLlmCliSubprocess() - req = _req(model="unmapped-cli") - _, _, detail = handler.execute_cli_inference(req) - assert detail != "" - - -# --------------------------------------------------------------------------- -# UNAVAILABLE: binary not found on PATH -# --------------------------------------------------------------------------- - - -@pytest.mark.unit -def test_unavailable_when_which_returns_none() -> None: - """UNAVAILABLE when shutil.which cannot find the CLI binary.""" - handler = HandlerLlmCliSubprocess(cli="gemini", cli_args=["-p"]) - req = _req() - - with ( - patch("shutil.which", return_value=None), - patch("subprocess.run") as mock_run, - ): - response, status, detail = handler.execute_cli_inference(req) - - assert response is None - assert status == EnumCliBackendStatus.UNAVAILABLE - assert "gemini" in detail - assert "PATH" in detail - mock_run.assert_not_called() - - -@pytest.mark.unit -def test_unavailable_detail_contains_binary_name() -> None: - """UNAVAILABLE detail names the missing binary.""" - handler = HandlerLlmCliSubprocess(cli="my-llm-tool", cli_args=["--prompt"]) - req = _req() - - with patch("shutil.which", return_value=None): - _, _, detail = handler.execute_cli_inference(req) - - assert "my-llm-tool" in detail - - -# --------------------------------------------------------------------------- -# INVALID_REQUEST: no user-role message / empty content -# --------------------------------------------------------------------------- - - -@pytest.mark.unit -def test_invalid_request_when_no_user_role_message() -> None: - """INVALID_REQUEST when messages contain no role='user' entry.""" - handler = HandlerLlmCliSubprocess(cli="gemini", cli_args=["-p"]) - - # Use a mock to bypass Pydantic validation and inject a non-user message - mock_req = MagicMock(spec=ModelLlmInferenceRequest) - mock_req.model = "gemini-cli" - assistant_msg = MagicMock() - assistant_msg.role = "assistant" - assistant_msg.content = "I am the model" - mock_req.messages = [assistant_msg] - - with ( - patch("shutil.which", return_value="/usr/bin/gemini"), - patch("subprocess.run") as mock_run, - ): - response, status, detail = handler.execute_cli_inference(mock_req) - - assert response is None - assert status == EnumCliBackendStatus.INVALID_REQUEST - assert "no user message" in detail - mock_run.assert_not_called() - - -@pytest.mark.unit -def test_invalid_request_when_user_message_content_is_none() -> None: - """INVALID_REQUEST when user message exists but content is None.""" - handler = HandlerLlmCliSubprocess(cli="gemini", cli_args=["-p"]) - - mock_req = MagicMock(spec=ModelLlmInferenceRequest) - mock_req.model = "gemini-cli" - user_msg = MagicMock() - user_msg.role = "user" - user_msg.content = None - mock_req.messages = [user_msg] - - with ( - patch("shutil.which", return_value="/usr/bin/gemini"), - patch("subprocess.run") as mock_run, - ): - response, status, _detail = handler.execute_cli_inference(mock_req) - - assert response is None - assert status == EnumCliBackendStatus.INVALID_REQUEST - mock_run.assert_not_called() - - -@pytest.mark.unit -def test_invalid_request_when_user_message_content_is_empty_string() -> None: - """INVALID_REQUEST when user message content is an empty string.""" - handler = HandlerLlmCliSubprocess(cli="gemini", cli_args=["-p"]) - - mock_req = MagicMock(spec=ModelLlmInferenceRequest) - mock_req.model = "gemini-cli" - user_msg = MagicMock() - user_msg.role = "user" - user_msg.content = "" - mock_req.messages = [user_msg] - - with ( - patch("shutil.which", return_value="/usr/bin/gemini"), - patch("subprocess.run") as mock_run, - ): - response, status, _detail = handler.execute_cli_inference(mock_req) - - assert response is None - assert status == EnumCliBackendStatus.INVALID_REQUEST - mock_run.assert_not_called() - - -# --------------------------------------------------------------------------- -# SUBPROCESS_ERROR: non-zero exit code -# --------------------------------------------------------------------------- - - -@pytest.mark.unit -def test_subprocess_error_on_nonzero_exit() -> None: - """SUBPROCESS_ERROR when subprocess exits with code != 0.""" - handler = HandlerLlmCliSubprocess(cli="gemini", cli_args=["-p"]) - req = _req() - - failed = _ok(returncode=1, stdout="", stderr="authentication error: bad token") - - with ( - patch("shutil.which", return_value="/usr/bin/gemini"), - patch("subprocess.run", return_value=failed), - ): - response, status, detail = handler.execute_cli_inference(req) - - assert response is None - assert status == EnumCliBackendStatus.SUBPROCESS_ERROR - assert "exit 1" in detail - assert "authentication error" in detail - - -@pytest.mark.unit -def test_subprocess_error_exit_code_appears_in_detail() -> None: - """Non-zero exit code value is embedded in the SUBPROCESS_ERROR detail.""" - handler = HandlerLlmCliSubprocess(cli="gemini", cli_args=["-p"]) - req = _req() - - with ( - patch("shutil.which", return_value="/usr/bin/gemini"), - patch("subprocess.run", return_value=_ok(returncode=42, stderr="oops")), - ): - _, status, detail = handler.execute_cli_inference(req) - - assert status == EnumCliBackendStatus.SUBPROCESS_ERROR - assert "42" in detail - - -@pytest.mark.unit -def test_subprocess_error_empty_stderr_shows_placeholder() -> None: - """When stderr is empty, SUBPROCESS_ERROR detail contains '(no stderr)'.""" - handler = HandlerLlmCliSubprocess(cli="gemini", cli_args=["-p"]) - req = _req() - - with ( - patch("shutil.which", return_value="/usr/bin/gemini"), - patch("subprocess.run", return_value=_ok(returncode=1, stderr="")), - ): - _, status, detail = handler.execute_cli_inference(req) - - assert status == EnumCliBackendStatus.SUBPROCESS_ERROR - assert "(no stderr)" in detail - - -@pytest.mark.unit -def test_subprocess_error_stderr_truncated_to_200_chars() -> None: - """SUBPROCESS_ERROR detail truncates stderr preview to 200 characters.""" - handler = HandlerLlmCliSubprocess(cli="gemini", cli_args=["-p"]) - req = _req() - - long_stderr = "X" * 500 - - with ( - patch("shutil.which", return_value="/usr/bin/gemini"), - patch("subprocess.run", return_value=_ok(returncode=1, stderr=long_stderr)), - ): - _, status, detail = handler.execute_cli_inference(req) - - assert status == EnumCliBackendStatus.SUBPROCESS_ERROR - # The 500-char stderr should be truncated to <=200 in the detail - assert long_stderr not in detail # full string not present - assert "X" * 200 in detail # first 200 chars are present - - -# --------------------------------------------------------------------------- -# EMPTY_RESPONSE: exit 0, stdout empty -# --------------------------------------------------------------------------- - - -@pytest.mark.unit -def test_empty_response_when_stdout_is_empty_string() -> None: - """EMPTY_RESPONSE returned when subprocess exits 0 but stdout is empty.""" - handler = HandlerLlmCliSubprocess(cli="gemini", cli_args=["-p"]) - req = _req() - - with ( - patch("shutil.which", return_value="/usr/bin/gemini"), - patch("subprocess.run", return_value=_ok(stdout="")), - ): - response, status, detail = handler.execute_cli_inference(req) - - assert response is None - assert status == EnumCliBackendStatus.EMPTY_RESPONSE - assert "empty" in detail - - -@pytest.mark.unit -def test_empty_response_when_stdout_is_whitespace_only() -> None: - """EMPTY_RESPONSE when stdout contains only whitespace (strip() → empty).""" - handler = HandlerLlmCliSubprocess(cli="gemini", cli_args=["-p"]) - req = _req() - - with ( - patch("shutil.which", return_value="/usr/bin/gemini"), - patch("subprocess.run", return_value=_ok(stdout=" \n\t ")), - ): - response, status, _detail = handler.execute_cli_inference(req) - - assert response is None - assert status == EnumCliBackendStatus.EMPTY_RESPONSE - - -# --------------------------------------------------------------------------- -# SUCCESS: fully-constructed ModelLlmInferenceResponse -# --------------------------------------------------------------------------- - - -@pytest.mark.unit -def test_success_returns_response_with_generated_text() -> None: - """SUCCESS returns ModelLlmInferenceResponse with stripped stdout as text.""" - from omnibase_infra.models.llm.model_llm_inference_response import ( - ModelLlmInferenceResponse, - ) - - handler = HandlerLlmCliSubprocess(cli="gemini", cli_args=["-p"]) - req = _req(user_content="What is 2+2?") - - with ( - patch("shutil.which", return_value="/usr/bin/gemini"), - patch("subprocess.run", return_value=_ok(stdout=" Four \n")), - ): - response, status, detail = handler.execute_cli_inference(req) - - assert status == EnumCliBackendStatus.SUCCESS - assert detail == "" - assert isinstance(response, ModelLlmInferenceResponse) - assert response.generated_text == "Four" # stripped - - -@pytest.mark.unit -def test_success_model_used_reflects_cli_binary() -> None: - """SUCCESS response model_used encodes the CLI binary name.""" - handler = HandlerLlmCliSubprocess(cli="gemini", cli_args=["-p"]) - req = _req() - - with ( - patch("shutil.which", return_value="/usr/bin/gemini"), - patch("subprocess.run", return_value=_ok()), - ): - response, status, _ = handler.execute_cli_inference(req) - - assert status == EnumCliBackendStatus.SUCCESS - assert response is not None - assert "gemini" in response.model_used - - -@pytest.mark.unit -def test_success_operation_type_and_finish_reason() -> None: - """SUCCESS response has CHAT_COMPLETION operation and STOP finish reason.""" - from omnibase_infra.enums import EnumLlmFinishReason, EnumLlmOperationType - - handler = HandlerLlmCliSubprocess(cli="gemini", cli_args=["-p"]) - req = _req() - - with ( - patch("shutil.which", return_value="/usr/bin/gemini"), - patch("subprocess.run", return_value=_ok()), - ): - response, status, _ = handler.execute_cli_inference(req) - - assert status == EnumCliBackendStatus.SUCCESS - assert response is not None - assert response.operation_type == EnumLlmOperationType.CHAT_COMPLETION - assert response.finish_reason == EnumLlmFinishReason.STOP - - -@pytest.mark.unit -def test_success_usage_fields_are_positive_integers() -> None: - """SUCCESS response token estimates are positive and internally consistent.""" - handler = HandlerLlmCliSubprocess(cli="gemini", cli_args=["-p"]) - req = _req(user_content="Explain quantum entanglement") - - with ( - patch("shutil.which", return_value="/usr/bin/gemini"), - patch( - "subprocess.run", - return_value=_ok(stdout="It is a quantum phenomenon\n"), - ), - ): - response, _, _ = handler.execute_cli_inference(req) - - assert response is not None - assert response.usage.tokens_input > 0 - assert response.usage.tokens_output > 0 - assert response.usage.tokens_total == ( - response.usage.tokens_input + response.usage.tokens_output - ) - - -@pytest.mark.unit -def test_success_latency_ms_is_non_negative() -> None: - """SUCCESS response latency_ms field is >= 0.""" - handler = HandlerLlmCliSubprocess(cli="gemini", cli_args=["-p"]) - req = _req() - - with ( - patch("shutil.which", return_value="/usr/bin/gemini"), - patch("subprocess.run", return_value=_ok()), - ): - response, _, _ = handler.execute_cli_inference(req) - - assert response is not None - assert response.latency_ms >= 0.0 - - -@pytest.mark.unit -def test_success_backend_result_reports_success() -> None: - """SUCCESS response backend_result.success is True.""" - handler = HandlerLlmCliSubprocess(cli="gemini", cli_args=["-p"]) - req = _req() - - with ( - patch("shutil.which", return_value="/usr/bin/gemini"), - patch("subprocess.run", return_value=_ok()), - ): - response, _, _ = handler.execute_cli_inference(req) - - assert response is not None - assert response.backend_result.success is True - - -@pytest.mark.unit -def test_success_correlation_id_propagated_from_request() -> None: - """SUCCESS response correlation_id matches the request's correlation_id.""" - caller_cid = uuid4() - handler = HandlerLlmCliSubprocess(cli="gemini", cli_args=["-p"]) - req = _req(correlation_id=caller_cid) - - with ( - patch("shutil.which", return_value="/usr/bin/gemini"), - patch("subprocess.run", return_value=_ok()), - ): - response, _, _ = handler.execute_cli_inference(req) - - assert response is not None - assert response.correlation_id == caller_cid - - -@pytest.mark.unit -def test_success_execution_id_is_fresh_uuid() -> None: - """SUCCESS response execution_id is a valid UUID (auto-generated per call).""" - handler = HandlerLlmCliSubprocess(cli="gemini", cli_args=["-p"]) - req = _req() - - with ( - patch("shutil.which", return_value="/usr/bin/gemini"), - patch("subprocess.run", return_value=_ok()), - ): - response, _, _ = handler.execute_cli_inference(req) - - assert response is not None - assert isinstance(response.execution_id, UUID) - - -@pytest.mark.unit -def test_success_timestamp_is_timezone_aware() -> None: - """SUCCESS response timestamp is timezone-aware.""" - handler = HandlerLlmCliSubprocess(cli="gemini", cli_args=["-p"]) - req = _req() - - with ( - patch("shutil.which", return_value="/usr/bin/gemini"), - patch("subprocess.run", return_value=_ok()), - ): - response, _, _ = handler.execute_cli_inference(req) - - assert response is not None - assert response.timestamp.tzinfo is not None - - -@pytest.mark.unit -def test_success_uses_last_user_message_as_prompt() -> None: - """When multiple user messages exist, the LAST one is used as the prompt.""" - handler = HandlerLlmCliSubprocess(cli="gemini", cli_args=["-p"]) - req = _req( - messages=( - ModelLlmMessage(role="user", content="first message"), - ModelLlmMessage(role="user", content="last message"), - ) - ) - - with ( - patch("shutil.which", return_value="/usr/bin/gemini"), - patch("subprocess.run", return_value=_ok()) as mock_run, - ): - _, status, _ = handler.execute_cli_inference(req) - - assert status == EnumCliBackendStatus.SUCCESS - call_argv = mock_run.call_args.args[0] - assert call_argv[-1] == "last message" - - -@pytest.mark.unit -def test_success_token_estimates_scale_with_length() -> None: - """Longer prompts and responses produce higher token estimates.""" - handler = HandlerLlmCliSubprocess(cli="gemini", cli_args=["-p"]) - - short_req = _req(user_content="hi") - long_req = _req( - user_content="Please give me a very comprehensive and detailed explanation of all aspects of quantum mechanics" - ) - - short_out = "OK\n" - long_out = " ".join(["word"] * 40) + "\n" - - with ( - patch("shutil.which", return_value="/usr/bin/gemini"), - patch("subprocess.run", return_value=_ok(stdout=short_out)), - ): - short_resp, _, _ = handler.execute_cli_inference(short_req) - - with ( - patch("shutil.which", return_value="/usr/bin/gemini"), - patch("subprocess.run", return_value=_ok(stdout=long_out)), - ): - long_resp, _, _ = handler.execute_cli_inference(long_req) - - assert short_resp is not None and long_resp is not None - assert long_resp.usage.tokens_input > short_resp.usage.tokens_input - assert long_resp.usage.tokens_output > short_resp.usage.tokens_output - - -@pytest.mark.unit -def test_success_cli_args_forwarded_to_subprocess() -> None: - """CLI args configured on the handler are forwarded verbatim to subprocess.run.""" - custom_args = ["--mode", "headless", "--no-color"] - handler = HandlerLlmCliSubprocess(cli="gemini", cli_args=custom_args) - req = _req(user_content="ping") - - with ( - patch("shutil.which", return_value="/usr/bin/gemini"), - patch("subprocess.run", return_value=_ok()) as mock_run, - ): - handler.execute_cli_inference(req) - - call_argv = mock_run.call_args.args[0] - # argv: [binary, *cli_args, prompt] - assert call_argv[0] == "gemini" - assert call_argv[1:4] == custom_args - assert call_argv[-1] == "ping" - - -# --------------------------------------------------------------------------- -# TIMEOUT: subprocess.TimeoutExpired -# --------------------------------------------------------------------------- - - -@pytest.mark.unit -def test_timeout_returned_on_timeout_expired() -> None: - """TIMEOUT returned when subprocess.run raises TimeoutExpired.""" - handler = HandlerLlmCliSubprocess(cli="gemini", cli_args=["-p"], timeout=5) - req = _req() - - with ( - patch("shutil.which", return_value="/usr/bin/gemini"), - patch( - "subprocess.run", - side_effect=subprocess.TimeoutExpired(cmd="gemini", timeout=5), - ), - ): - response, status, detail = handler.execute_cli_inference(req) - - assert response is None - assert status == EnumCliBackendStatus.TIMEOUT - assert "gemini" in detail - assert "5" in detail - - -@pytest.mark.unit -def test_timeout_detail_contains_configured_deadline() -> None: - """TIMEOUT detail message mentions the configured timeout value.""" - timeout_val = 77 - handler = HandlerLlmCliSubprocess( - cli="gemini", cli_args=["-p"], timeout=timeout_val - ) - req = _req() - - with ( - patch("shutil.which", return_value="/usr/bin/gemini"), - patch( - "subprocess.run", - side_effect=subprocess.TimeoutExpired(cmd="gemini", timeout=timeout_val), - ), - ): - _, status, detail = handler.execute_cli_inference(req) - - assert status == EnumCliBackendStatus.TIMEOUT - assert str(timeout_val) in detail - - -@pytest.mark.unit -def test_timeout_response_is_none() -> None: - """TIMEOUT tuple has None as the response element.""" - handler = HandlerLlmCliSubprocess(cli="gemini", cli_args=["-p"], timeout=1) - req = _req() - - with ( - patch("shutil.which", return_value="/usr/bin/gemini"), - patch( - "subprocess.run", - side_effect=subprocess.TimeoutExpired(cmd="gemini", timeout=1), - ), - ): - response, _, _ = handler.execute_cli_inference(req) - - assert response is None diff --git a/tests/unit/runtime/auto_wiring/test_wiring.py b/tests/unit/runtime/auto_wiring/test_wiring.py index bc7f435cef..8b74d6bc2a 100644 --- a/tests/unit/runtime/auto_wiring/test_wiring.py +++ b/tests/unit/runtime/auto_wiring/test_wiring.py @@ -182,7 +182,7 @@ def test_route_id(self) -> None: def test_route_id_with_operation(self) -> None: """OMN-9461: operation suffix disambiguates repeated handler references.""" handler_key = _derive_handler_entry_key( - self._entry("HandlerLlmCliSubprocess", "inference.gemini_cli") + self._entry("HandlerRoutingExample", "inference.example_op") ) assert ( @@ -191,7 +191,7 @@ def test_route_id_with_operation(self) -> None: handler_key, "onex.cmd.omnibase-infra.llm-inference-request.v1", ) - == "route.auto.my_node.HandlerLlmCliSubprocess.inference_gemini_cli_fb462661.onex_cmd_omnibase_infra_llm_inference_request_v1" + == "route.auto.my_node.HandlerRoutingExample.inference_example_op_d1904b8c.onex_cmd_omnibase_infra_llm_inference_request_v1" ) def test_route_id_without_operation_unchanged(self) -> None: @@ -214,7 +214,7 @@ def test_dispatcher_id(self) -> None: def test_dispatcher_id_with_operation(self) -> None: """OMN-9461: operation suffix disambiguates repeated handler references.""" handler_key = _derive_handler_entry_key( - self._entry("HandlerLlmCliSubprocess", "inference.gemini_cli") + self._entry("HandlerRoutingExample", "inference.example_op") ) assert ( @@ -222,7 +222,7 @@ def test_dispatcher_id_with_operation(self) -> None: "node_llm_inference_effect", handler_key, ) - == "dispatcher.auto.node_llm_inference_effect.HandlerLlmCliSubprocess.inference_gemini_cli_fb462661" + == "dispatcher.auto.node_llm_inference_effect.HandlerRoutingExample.inference_example_op_d1904b8c" ) def test_dispatcher_id_collision_safe(self) -> None: @@ -964,9 +964,9 @@ async def test_repeated_handler_same_class_different_operations(self) -> None: """OMN-9461: two entries referencing the same handler class for different operations must wire without duplicate dispatcher or route ID collisions. - Mirrors the real node_llm_inference_effect contract which lists - HandlerLlmCliSubprocess for both ``inference.gemini_cli`` and - ``inference.claude_cli``. + Synthetic regression fixture: a shared handler class bound to two + distinct operations on the same contract (the general shape this + guards, independent of any specific live contract). """ from omnibase_infra.runtime.message_dispatch_engine import ( MessageDispatchEngine, @@ -977,12 +977,12 @@ async def test_repeated_handler_same_class_different_operations(self) -> None: ModelHandlerRoutingEntry( handler=ModelHandlerRef(name="HandlerShared", module="fake.module"), event_model=None, - operation="inference.gemini_cli", + operation="inference.variant_a", ), ModelHandlerRoutingEntry( handler=ModelHandlerRef(name="HandlerShared", module="fake.module"), event_model=None, - operation="inference.claude_cli", + operation="inference.variant_b", ), ) handler_routing = ModelHandlerRouting( @@ -990,7 +990,7 @@ async def test_repeated_handler_same_class_different_operations(self) -> None: handlers=handler_entries, ) contract = _make_contract( - name="node_llm_inference_effect", + name="node_test_shared_handler", subscribe_topics=("onex.cmd.omnibase-infra.llm-inference-request.v1",), handler_routing=handler_routing, ) From 98532a648b662a00bb713f70638f037bead75de8 Mon Sep 17 00:00:00 2001 From: jonahgabriel Date: Wed, 12 Aug 2026 18:12:32 -0400 Subject: [PATCH 2/5] chore(OMN-15959): trigger fresh CI evaluation with Evidence-Source in PR body From 8886c50ee227e200a401434783f37cca41caf148 Mon Sep 17 00:00:00 2001 From: jonahgabriel Date: Thu, 13 Aug 2026 08:14:17 -0400 Subject: [PATCH 3/5] fix(OMN-15959): remove stale infra-node-allowlist.txt entry for deleted handler Handler_llm_cli_subprocess.py was deleted in this PR but its allowlist entry in scripts/ci/infra-node-allowlist.txt was left behind, tripping the "Reject unallowlisted infra node handlers" gate (Infra Node Handler Ownership check) with a stale-entry error. --- scripts/ci/infra-node-allowlist.txt | 1 - 1 file changed, 1 deletion(-) diff --git a/scripts/ci/infra-node-allowlist.txt b/scripts/ci/infra-node-allowlist.txt index 989e9e3b01..67ae8d0cea 100644 --- a/scripts/ci/infra-node-allowlist.txt +++ b/scripts/ci/infra-node-allowlist.txt @@ -127,7 +127,6 @@ src/omnibase_infra/nodes/node_llm_inference_effect/handlers/bifrost/model_bifros src/omnibase_infra/nodes/node_llm_inference_effect/handlers/bifrost/model_bifrost_request.py # approved existing infra handler path; new business-domain handlers belong in omnimarket. src/omnibase_infra/nodes/node_llm_inference_effect/handlers/bifrost/model_bifrost_response.py # approved existing infra handler path; new business-domain handlers belong in omnimarket. src/omnibase_infra/nodes/node_llm_inference_effect/handlers/bifrost/model_bifrost_routing_rule.py # approved existing infra handler path; new business-domain handlers belong in omnimarket. -src/omnibase_infra/nodes/node_llm_inference_effect/handlers/handler_llm_cli_subprocess.py # approved existing infra handler path; new business-domain handlers belong in omnimarket. src/omnibase_infra/nodes/node_llm_inference_effect/handlers/handler_llm_inference_command.py # infra-owned runtime command adapter for the LLM inference effect; business routing remains outside this handler. src/omnibase_infra/nodes/node_llm_inference_effect/handlers/handler_llm_openai_compatible.py # approved existing infra handler path; new business-domain handlers belong in omnimarket. src/omnibase_infra/nodes/node_merge_gate_effect/handlers/__init__.py # approved existing infra handler path; new business-domain handlers belong in omnimarket. From 4ee39bb38ebdf366ed47bccc4302686ce8c873ef Mon Sep 17 00:00:00 2001 From: jonahgabriel Date: Tue, 18 Aug 2026 06:44:20 -0400 Subject: [PATCH 4/5] fix(OMN-15959): regenerate dispatch-parity fixture + bump coding-agent contract Two follow-ups to the handler_llm_cli_subprocess.py deletion: 1. baseline-selection-v2.json pinned the pre-deletion selection for the llm-inference-request topic, which still listed the 3 deleted CLI dispatchers (gemini_cli/claude_cli/opencode_cli) alongside the surviving openai_compatible + inference_command handlers. Regenerated via the documented harness (`uv run python -m tests.fixtures.dispatch_parity.harness --out ...`). Diff verified probe-by-probe: only the 2 llm-inference-request probes changed, only the 3 deleted dispatcher_ids removed from each selection tuple -- registered_dispatchers 114->111, registered_routes 124->121 (exactly -3, matching the deletion), zero other probes touched. tests/integration/runtime/test_dispatch_selection_parity.py: 10/10 pass. 2. Wave C contract-sync gate (OMN-8915) flagged this PR's docstring update to handler_coding_agent_invoke.py (references the deleted handler by name) as a handler change with no matching contract.yaml touch. Bumped node_coding_agent_invoke_effect's contract_version/node_version patch (1.0.1 -> 1.0.2) -- docstring-only, no behavioral or schema change. Evidence-Ticket: OMN-15959 Evidence-Source: OCC#6402 --- .../contract.yaml | 4 +- .../baseline-selection-v2.json | 93 ++----------------- 2 files changed, 8 insertions(+), 89 deletions(-) diff --git a/src/omnibase_infra/nodes/node_coding_agent_invoke_effect/contract.yaml b/src/omnibase_infra/nodes/node_coding_agent_invoke_effect/contract.yaml index de686dcba4..970127f7c7 100644 --- a/src/omnibase_infra/nodes/node_coding_agent_invoke_effect/contract.yaml +++ b/src/omnibase_infra/nodes/node_coding_agent_invoke_effect/contract.yaml @@ -37,9 +37,9 @@ # transport-envelope-shaped output events before publish, preserving the # contract-declared terminal event semantics without node-local envelope code. name: node_coding_agent_invoke_effect -contract_version: {major: 1, minor: 0, patch: 1} +contract_version: {major: 1, minor: 0, patch: 2} node_type: EFFECT_GENERIC -node_version: {major: 1, minor: 0, patch: 1} +node_version: {major: 1, minor: 0, patch: 2} description: > Coding-agent invoke effect. Runs the claude/codex CLI subprocess in the target workspace and returns a system-derived ModelCodingAgentResult: status, exit code, git-derived files_changed + diff (captured AFTER the subprocess exits, never trusted from agent stdout), diff_hash, starting_head_sha, structured error_class (UNAVAILABLE/TIMEOUT/SUBPROCESS_ERROR/EMPTY_RESPONSE), timed_out, and duration. The handler uses the canonical def-B runtime shape: the contract event_model payload is validated by the shared runtime adapter before handle(command), and the bare ModelCodingAgentResult return is wrapped for EFFECT publication by the runtime. shutil.which gates availability; the subprocess spawns in its own process group and a timeout kills the whole group. The subprocess + git I/O are injected seams so this node is the single sanctioned model-CLI shell-out surface. agent-reported output/usage are advisory only. diff --git a/tests/fixtures/dispatch_parity/baseline-selection-v2.json b/tests/fixtures/dispatch_parity/baseline-selection-v2.json index 97ff8a9136..13f0197553 100644 --- a/tests/fixtures/dispatch_parity/baseline-selection-v2.json +++ b/tests/fixtures/dispatch_parity/baseline-selection-v2.json @@ -2296,51 +2296,6 @@ "package": "omnibase_infra", "type_scoped": false }, - "dispatcher.auto.node_llm_inference_effect.HandlerLlmCliSubprocess.inference_claude_cli_419d77e2": { - "category": "command", - "contract_name": "node_llm_inference_effect", - "event_model_module": null, - "event_model_name": null, - "handler_id": "dispatcher.auto.node_llm_inference_effect.HandlerLlmCliSubprocess.inference_claude_cli_419d77e2", - "handler_module": "omnibase_infra.nodes.node_llm_inference_effect.handlers.handler_llm_cli_subprocess", - "handler_name": "HandlerLlmCliSubprocess", - "message_types": [ - "omnibase-infra.llm-inference-request", - "onex.cmd.omnibase-infra.llm-inference-request.v1" - ], - "package": "omnibase_infra", - "type_scoped": false - }, - "dispatcher.auto.node_llm_inference_effect.HandlerLlmCliSubprocess.inference_gemini_cli_fb462661": { - "category": "command", - "contract_name": "node_llm_inference_effect", - "event_model_module": null, - "event_model_name": null, - "handler_id": "dispatcher.auto.node_llm_inference_effect.HandlerLlmCliSubprocess.inference_gemini_cli_fb462661", - "handler_module": "omnibase_infra.nodes.node_llm_inference_effect.handlers.handler_llm_cli_subprocess", - "handler_name": "HandlerLlmCliSubprocess", - "message_types": [ - "omnibase-infra.llm-inference-request", - "onex.cmd.omnibase-infra.llm-inference-request.v1" - ], - "package": "omnibase_infra", - "type_scoped": false - }, - "dispatcher.auto.node_llm_inference_effect.HandlerLlmCliSubprocess.inference_opencode_cli_a90c169f": { - "category": "command", - "contract_name": "node_llm_inference_effect", - "event_model_module": null, - "event_model_name": null, - "handler_id": "dispatcher.auto.node_llm_inference_effect.HandlerLlmCliSubprocess.inference_opencode_cli_a90c169f", - "handler_module": "omnibase_infra.nodes.node_llm_inference_effect.handlers.handler_llm_cli_subprocess", - "handler_name": "HandlerLlmCliSubprocess", - "message_types": [ - "omnibase-infra.llm-inference-request", - "onex.cmd.omnibase-infra.llm-inference-request.v1" - ], - "package": "omnibase_infra", - "type_scoped": false - }, "dispatcher.auto.node_llm_inference_effect.HandlerLlmInferenceCommand.inference_command_cb168568": { "category": "command", "contract_name": "node_llm_inference_effect", @@ -3237,8 +3192,8 @@ "discovery_errors": 0, "distinct_subscribe_topics": 108, "probe_count": 1046, - "registered_dispatchers": 114, - "registered_routes": 124, + "registered_dispatchers": 111, + "registered_routes": 121, "type_scoped_dispatchers": 69 }, "duplicate_winners": [], @@ -3279,10 +3234,10 @@ "omnimemory" ], "fixture_version": "v2", - "generated_at_utc": "2026-08-13T00:51:20.726285+00:00", + "generated_at_utc": "2026-08-18T10:39:18.121771+00:00", "installed_package_versions": { "omnibase-core": "0.46.8", - "omnibase-infra": "0.38.4", + "omnibase-infra": "0.38.7", "omniclaude": null, "omniintelligence": null, "omnimarket": null, @@ -3556,10 +3511,7 @@ }, "selection": { "dispatcher_ids": [ - "dispatcher.auto.node_llm_inference_effect.HandlerLlmOpenaiCompatible.inference_openai_compatible_2587ad5b", - "dispatcher.auto.node_llm_inference_effect.HandlerLlmCliSubprocess.inference_gemini_cli_fb462661", - "dispatcher.auto.node_llm_inference_effect.HandlerLlmCliSubprocess.inference_claude_cli_419d77e2", - "dispatcher.auto.node_llm_inference_effect.HandlerLlmCliSubprocess.inference_opencode_cli_a90c169f" + "dispatcher.auto.node_llm_inference_effect.HandlerLlmOpenaiCompatible.inference_openai_compatible_2587ad5b" ], "dlq_topic": null, "message_category": "command", @@ -20494,10 +20446,7 @@ "selection": { "dispatcher_ids": [ "dispatcher.auto.node_llm_inference_effect.HandlerLlmInferenceCommand.inference_command_cb168568", - "dispatcher.auto.node_llm_inference_effect.HandlerLlmOpenaiCompatible.inference_openai_compatible_2587ad5b", - "dispatcher.auto.node_llm_inference_effect.HandlerLlmCliSubprocess.inference_gemini_cli_fb462661", - "dispatcher.auto.node_llm_inference_effect.HandlerLlmCliSubprocess.inference_claude_cli_419d77e2", - "dispatcher.auto.node_llm_inference_effect.HandlerLlmCliSubprocess.inference_opencode_cli_a90c169f" + "dispatcher.auto.node_llm_inference_effect.HandlerLlmOpenaiCompatible.inference_openai_compatible_2587ad5b" ], "dlq_topic": null, "message_category": "command", @@ -23395,36 +23344,6 @@ "topic_pattern": "*.cmd.omnibase-infra.llm-embedding-request.*", "type_scoped": false }, - { - "contract_name": "node_llm_inference_effect", - "dispatcher_id": "dispatcher.auto.node_llm_inference_effect.HandlerLlmCliSubprocess.inference_claude_cli_419d77e2", - "handler_id": "dispatcher.auto.node_llm_inference_effect.HandlerLlmCliSubprocess.inference_claude_cli_419d77e2", - "message_category": "command", - "route_id": "route.auto.node_llm_inference_effect.HandlerLlmCliSubprocess.inference_claude_cli_419d77e2.onex_cmd_omnibase_infra_llm_inference_request_v1", - "topic": "onex.cmd.omnibase-infra.llm-inference-request.v1", - "topic_pattern": "*.cmd.omnibase-infra.llm-inference-request.*", - "type_scoped": false - }, - { - "contract_name": "node_llm_inference_effect", - "dispatcher_id": "dispatcher.auto.node_llm_inference_effect.HandlerLlmCliSubprocess.inference_gemini_cli_fb462661", - "handler_id": "dispatcher.auto.node_llm_inference_effect.HandlerLlmCliSubprocess.inference_gemini_cli_fb462661", - "message_category": "command", - "route_id": "route.auto.node_llm_inference_effect.HandlerLlmCliSubprocess.inference_gemini_cli_fb462661.onex_cmd_omnibase_infra_llm_inference_request_v1", - "topic": "onex.cmd.omnibase-infra.llm-inference-request.v1", - "topic_pattern": "*.cmd.omnibase-infra.llm-inference-request.*", - "type_scoped": false - }, - { - "contract_name": "node_llm_inference_effect", - "dispatcher_id": "dispatcher.auto.node_llm_inference_effect.HandlerLlmCliSubprocess.inference_opencode_cli_a90c169f", - "handler_id": "dispatcher.auto.node_llm_inference_effect.HandlerLlmCliSubprocess.inference_opencode_cli_a90c169f", - "message_category": "command", - "route_id": "route.auto.node_llm_inference_effect.HandlerLlmCliSubprocess.inference_opencode_cli_a90c169f.onex_cmd_omnibase_infra_llm_inference_request_v1", - "topic": "onex.cmd.omnibase-infra.llm-inference-request.v1", - "topic_pattern": "*.cmd.omnibase-infra.llm-inference-request.*", - "type_scoped": false - }, { "contract_name": "node_llm_inference_effect", "dispatcher_id": "dispatcher.auto.node_llm_inference_effect.HandlerLlmInferenceCommand.inference_command_cb168568", From caf628022c21c9ecf227f1c63911b58d62e2a4b5 Mon Sep 17 00:00:00 2001 From: jonahgabriel Date: Sat, 22 Aug 2026 19:21:54 -0400 Subject: [PATCH 5/5] fix(OMN-15959): update stale version/date pins for node_llm_inference_effect This PR bumps node_llm_inference_effect's contract_version 1.4.4 -> 1.5.0 and metadata.updated -> 2026-08-12 (operation-surface removal from deleting the 3 CLI ops). Two downstream integration tests hardcode the exact prior values as staleness guards and needed updating to match: tests/integration/test_omn12816_main_promotion_coverage.py (contract_version pin) and tests/integration/test_omn_6659_type_suppression_contract_touch.py (metadata.updated pin, node_llm_inference_effect entry only -- the other 4 nodes in that dict are untouched by this PR). Both verified passing locally after the fix. --- tests/integration/test_omn12816_main_promotion_coverage.py | 2 +- .../test_omn_6659_type_suppression_contract_touch.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/integration/test_omn12816_main_promotion_coverage.py b/tests/integration/test_omn12816_main_promotion_coverage.py index 19ce50d5d2..585c989557 100644 --- a/tests/integration/test_omn12816_main_promotion_coverage.py +++ b/tests/integration/test_omn12816_main_promotion_coverage.py @@ -18,5 +18,5 @@ def test_omn12816_llm_inference_contract_declares_extra_body() -> None: contract: dict[str, Any] = yaml.safe_load(contract_path.read_text(encoding="utf-8")) - assert contract["contract_version"] == {"major": 1, "minor": 4, "patch": 4} + assert contract["contract_version"] == {"major": 1, "minor": 5, "patch": 0} assert "extra_body" in contract["input_model"]["description"] diff --git a/tests/integration/test_omn_6659_type_suppression_contract_touch.py b/tests/integration/test_omn_6659_type_suppression_contract_touch.py index 333f7a516c..d616484517 100644 --- a/tests/integration/test_omn_6659_type_suppression_contract_touch.py +++ b/tests/integration/test_omn_6659_type_suppression_contract_touch.py @@ -20,7 +20,7 @@ _EXPECTED_UPDATED_DATES = { "node_artifact_change_detector_effect": "2026-05-22", "node_impact_analyzer_compute": "2026-07-20", - "node_llm_inference_effect": "2026-07-19", + "node_llm_inference_effect": "2026-08-12", "node_runtime_error_triage_effect": "2026-05-22", "node_session_state_effect": "2026-05-22", }