From 5710822dd113492511907fbdef2bcae21160b31e Mon Sep 17 00:00:00 2001 From: Christopher Kevin Date: Tue, 29 Sep 2026 17:23:58 -0700 Subject: [PATCH 1/7] fix: use tool structured output for OpenAI Opus 5 TP4 Signed-off-by: Christopher Kevin --- src/skillspector/llm_analyzer_base.py | 16 +- src/skillspector/llm_utils.py | 29 +++- .../nodes/analyzers/mcp_tool_poisoning.py | 14 ++ tests/test_mcp_tool_poisoning.py | 147 ++++++++++++++++++ tests/unit/test_llm_utils.py | 20 +++ 5 files changed, 219 insertions(+), 7 deletions(-) diff --git a/src/skillspector/llm_analyzer_base.py b/src/skillspector/llm_analyzer_base.py index ec9c9b4e9..028f2053c 100644 --- a/src/skillspector/llm_analyzer_base.py +++ b/src/skillspector/llm_analyzer_base.py @@ -934,7 +934,7 @@ def __init__( max_retries=native_retries, ) self._structured_llm = ( - bind_structured_output(self._llm, self.response_schema, model) + self._bind_structured_output(self._llm, self.response_schema) if self.response_schema else None ) @@ -945,6 +945,18 @@ def __init__( chat_model=self._llm, ) + def _structured_output_preference(self, llm: object) -> str | None: + """Return an analyzer-specific binding preference, if any.""" + return None + + def _bind_structured_output(self, llm: object, schema: type) -> object: + return bind_structured_output( + llm, + schema, + self.model, + preferred_method=self._structured_output_preference(llm), + ) + def _remaining_timeout(self) -> float | None: if callable(self._timeout): remaining = self._timeout() @@ -989,7 +1001,7 @@ def _model_for_call(self) -> tuple[object, object | None]: chat_model_controls(llm), ) structured = ( - bind_structured_output(llm, self.response_schema, self.model) + self._bind_structured_output(llm, self.response_schema) if self.response_schema else None ) diff --git a/src/skillspector/llm_utils.py b/src/skillspector/llm_utils.py index f2a9da738..cd227570c 100644 --- a/src/skillspector/llm_utils.py +++ b/src/skillspector/llm_utils.py @@ -373,14 +373,20 @@ def set_timeout(self, timeout: float | None) -> None: STRUCTURED_OUTPUT_METHODS = ("function_calling", "json_schema") -def structured_output_kwargs(model: str, provider: object | None = None) -> dict[str, str]: +def structured_output_kwargs( + model: str, + provider: object | None = None, + *, + preferred_method: str | None = None, +) -> dict[str, str]: """Keyword arguments for ``with_structured_output`` when binding a schema for *model*. ``SKILLSPECTOR_STRUCTURED_OUTPUT_METHOD`` wins, then the active provider's - ``structured_output_method(model)`` hint, else LangChain's default (no kwargs). + ``structured_output_method(model)`` hint, then an analyzer preference, else + LangChain's default (no kwargs). Raises: - ValueError: when the environment override is not a known method. + ValueError: when the environment override or preference is not a known method. """ override = os.environ.get("SKILLSPECTOR_STRUCTURED_OUTPUT_METHOD", "").strip().lower() if override: @@ -394,11 +400,24 @@ def structured_output_kwargs(model: str, provider: object | None = None) -> dict provider = get_active_provider() hint = getattr(provider, "structured_output_method", None) method = hint(model) if callable(hint) else None + if method: + return {"method": method} + if preferred_method is not None and preferred_method not in STRUCTURED_OUTPUT_METHODS: + raise ValueError( + "preferred structured output method must be one of " + f"{', '.join(STRUCTURED_OUTPUT_METHODS)}; got {preferred_method!r}" + ) + method = preferred_method return {"method": method} if method else {} def bind_structured_output( - llm: object, schema: type, model: str, provider: object | None = None + llm: object, + schema: type, + model: str, + provider: object | None = None, + *, + preferred_method: str | None = None, ) -> object: """``llm.with_structured_output(schema)`` with the method *model* needs. @@ -408,7 +427,7 @@ def bind_structured_output( :class:`StructuredOutputParseError`, which the analyzers retry like any other malformed structured response. """ - kwargs = structured_output_kwargs(model, provider) + kwargs = structured_output_kwargs(model, provider, preferred_method=preferred_method) structured = llm.with_structured_output(schema, **kwargs) # type: ignore[attr-defined] if kwargs or getattr(llm, "supports_tool_choice_values", None) != ("auto",): return structured diff --git a/src/skillspector/nodes/analyzers/mcp_tool_poisoning.py b/src/skillspector/nodes/analyzers/mcp_tool_poisoning.py index 63e86b6ee..8fc679de5 100644 --- a/src/skillspector/nodes/analyzers/mcp_tool_poisoning.py +++ b/src/skillspector/nodes/analyzers/mcp_tool_poisoning.py @@ -26,6 +26,7 @@ from dataclasses import dataclass, field from typing import cast +from langchain_openai.chat_models.base import BaseChatOpenAI from pydantic import BaseModel, Field, field_validator from skillspector.inference_usage import InferenceUsageRecord @@ -45,6 +46,7 @@ append_output_language_instruction, estimate_tokens, ) +from skillspector.llm_utils import chat_model_provider_name from skillspector.model_info import get_max_input_tokens from skillspector.models import Finding, compute_match_fingerprint from skillspector.nodes.analyzers.static_runner import MAX_FINDINGS_PER_ANALYZER @@ -902,6 +904,18 @@ class _TP4Analyzer(LLMAnalyzerBase): response_schema = _TP4AnalysisResult + def _structured_output_preference(self, llm: object) -> str | None: + # The OpenAI-compatible Opus 5 gateway sometimes wraps json_schema + # output in a {"json": ...} object instead of the requested schema. + # Tool calling returns the exact TP4 schema on the same gateway. + if ( + isinstance(llm, BaseChatOpenAI) + and chat_model_provider_name(llm) == "openai" + and self.model == "azure/anthropic/claude-opus-5" + ): + return "function_calling" + return None + def __init__( self, model: str, diff --git a/tests/test_mcp_tool_poisoning.py b/tests/test_mcp_tool_poisoning.py index 0c417003d..5a040bd52 100644 --- a/tests/test_mcp_tool_poisoning.py +++ b/tests/test_mcp_tool_poisoning.py @@ -1578,6 +1578,85 @@ def test_no_applicable_markdown_keeps_clean_status(self, monkeypatch: pytest.Mon class TestTP4Fallbacks: + def test_opus5_openai_tp4_binds_tool_output_for_initial_and_refreshed_models( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + from langchain_openai import ChatOpenAI + + from skillspector import llm_analyzer_base + from skillspector.llm_utils import register_chat_model_provider + + monkeypatch.delenv("SKILLSPECTOR_STRUCTURED_OUTPUT_METHOD", raising=False) + bindings: list[dict[str, object]] = [] + original = ChatOpenAI.with_structured_output + + def record_binding(self, schema, **kwargs): + bindings.append(kwargs) + return original(self, schema, **kwargs) + + monkeypatch.setattr(ChatOpenAI, "with_structured_output", record_binding) + + def openai_model(**kwargs): + llm = ChatOpenAI( + model=kwargs["model"], api_key="sk-test", base_url="https://example.invalid/v1" + ) + register_chat_model_provider(llm, "openai") + return llm + + monkeypatch.setattr(llm_analyzer_base, "get_chat_model", openai_model) + analyzer = mcp_tool_poisoning._TP4Analyzer( + model="azure/anthropic/claude-opus-5", timeout=lambda: 30.0 + ) + monkeypatch.setattr(llm_analyzer_base, "_retarget_request_timeout", lambda *_args: False) + analyzer._model_for_call() + + assert bindings == [ + {"method": "function_calling"}, + {"method": "function_calling"}, + ] + + # The gateway-specific workaround does not change other OpenAI models. + mcp_tool_poisoning._TP4Analyzer(model="azure/anthropic/claude-sonnet-4-6") + assert bindings[-1] == {} + + # An explicit user override retains precedence over the workaround. + monkeypatch.setenv("SKILLSPECTOR_STRUCTURED_OUTPUT_METHOD", "json_schema") + mcp_tool_poisoning._TP4Analyzer(model="azure/anthropic/claude-opus-5") + assert bindings[-1] == {"method": "json_schema"} + + def test_opus5_tp4_does_not_force_tool_output_on_other_providers( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + from langchain_openai import ChatOpenAI + + from skillspector import llm_analyzer_base + from skillspector.llm_utils import register_chat_model_provider + + monkeypatch.delenv("SKILLSPECTOR_STRUCTURED_OUTPUT_METHOD", raising=False) + bindings: list[dict[str, object]] = [] + original = ChatOpenAI.with_structured_output + + def record_binding(self, schema, **kwargs): + bindings.append(kwargs) + return original(self, schema, **kwargs) + + def compatible_model(**kwargs): + llm = ChatOpenAI( + model=kwargs["model"], api_key="sk-test", base_url="https://example.invalid/v1" + ) + register_chat_model_provider(llm, "openai_compatible") + return llm + + monkeypatch.setattr(ChatOpenAI, "with_structured_output", record_binding) + monkeypatch.setattr(llm_analyzer_base, "get_chat_model", compatible_model) + + analyzer = mcp_tool_poisoning._TP4Analyzer( + model="azure/anthropic/claude-opus-5", timeout=lambda: 30.0 + ) + monkeypatch.setattr(llm_analyzer_base, "_retarget_request_timeout", lambda *_args: False) + analyzer._model_for_call() + assert bindings == [{}, {}] + def test_configured_output_language_is_included(self, monkeypatch: pytest.MonkeyPatch) -> None: monkeypatch.setenv("SKILLSPECTOR_OUTPUT_LANGUAGE", "German") _mock_tp4_structured_llm(monkeypatch, [{"is_mismatch": False}]) @@ -1625,6 +1704,74 @@ def test_persistently_malformed_response_returns_empty(self, monkeypatch: pytest ) assert result["analyzer_status_events"][0]["status"] == "degraded" + def test_wrapped_json_schema_response_stays_incomplete( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + state = _make_state("mcp_mismatched_skill", use_llm=True) + monkeypatch.setattr("skillspector.llm_analyzer_base.time.sleep", lambda _delay: None) + structured_llm = _mock_tp4_structured_llm( + monkeypatch, + [{"json": {"is_mismatch": False}} for _ in range(4)], + ) + + result = node(state) + + assert structured_llm.calls == 4 + assert result["analyzer_status_events"][0]["status"] == "degraded" + assert any( + event.get("reason_code") is LedgerReason.LLM_STRUCTURED_RESPONSE_INVALID + for event in result["inspection_ledger"] + ) + + def test_tool_refusal_stays_incomplete(self, monkeypatch: pytest.MonkeyPatch) -> None: + state = _make_state("mcp_mismatched_skill", use_llm=True) + structured_llm = _mock_tp4_structured_llm(monkeypatch, [None]) + + result = node(state) + + assert structured_llm.calls == 1 + assert result["analyzer_status_events"][0]["status"] == "failed" + assert any( + event["outcome"] is LedgerOutcome.FAILED for event in result["inspection_ledger"] + ) + + def test_opus5_function_calling_refusal_stays_incomplete( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + from langchain_core.messages import AIMessage + from langchain_core.outputs import ChatGeneration, ChatResult + from langchain_openai import ChatOpenAI + + from skillspector import llm_analyzer_base + from skillspector.llm_utils import register_chat_model_provider + + monkeypatch.delenv("SKILLSPECTOR_STRUCTURED_OUTPUT_METHOD", raising=False) + + def openai_model(**kwargs): + llm = ChatOpenAI( + model=kwargs["model"], api_key="sk-test", base_url="https://example.invalid/v1" + ) + register_chat_model_provider(llm, "openai") + return llm + + monkeypatch.setattr(llm_analyzer_base, "get_chat_model", openai_model) + monkeypatch.setattr( + ChatOpenAI, + "_generate", + lambda *_args, **_kwargs: ChatResult( + generations=[ChatGeneration(message=AIMessage(content="refused"))] + ), + ) + state = _make_state("mcp_mismatched_skill", use_llm=True) + state["model_config"] = {"default": "azure/anthropic/claude-opus-5"} + + result = node(state) + + assert result["analyzer_status_events"][0]["status"] == "failed" + assert any( + event["outcome"] is LedgerOutcome.FAILED for event in result["inspection_ledger"] + ) + def test_malformed_response_is_retried(self, monkeypatch: pytest.MonkeyPatch): state = _make_state("mcp_mismatched_skill", use_llm=True) sleep = MagicMock() diff --git a/tests/unit/test_llm_utils.py b/tests/unit/test_llm_utils.py index a9a1ea569..e7e38bcb3 100644 --- a/tests/unit/test_llm_utils.py +++ b/tests/unit/test_llm_utils.py @@ -794,6 +794,26 @@ def test_provider_hint_selects_json_schema(self, monkeypatch: pytest.MonkeyPatch } assert structured_output_kwargs("other", provider=_HintingProvider()) == {} + def test_analyzer_preference_applies_only_without_explicit_configuration( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + monkeypatch.delenv("SKILLSPECTOR_STRUCTURED_OUTPUT_METHOD", raising=False) + assert structured_output_kwargs( + "other", provider=_PlainProvider(), preferred_method="function_calling" + ) == {"method": "function_calling"} + assert structured_output_kwargs( + "needs-json-1", provider=_HintingProvider(), preferred_method="function_calling" + ) == {"method": "json_schema"} + monkeypatch.setenv("SKILLSPECTOR_STRUCTURED_OUTPUT_METHOD", "json_schema") + assert structured_output_kwargs( + "other", provider=_PlainProvider(), preferred_method="function_calling" + ) == {"method": "json_schema"} + + def test_unknown_analyzer_preference_is_rejected(self, monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.delenv("SKILLSPECTOR_STRUCTURED_OUTPUT_METHOD", raising=False) + with pytest.raises(ValueError, match="preferred structured output method"): + structured_output_kwargs("other", provider=_PlainProvider(), preferred_method="xml") + def test_env_override_wins_over_the_hint(self, monkeypatch: pytest.MonkeyPatch) -> None: monkeypatch.setenv("SKILLSPECTOR_STRUCTURED_OUTPUT_METHOD", "function_calling") assert structured_output_kwargs("needs-json-1", provider=_HintingProvider()) == { From 07326e52fa4087a60bb6e4ab1f0b02d79b9200cc Mon Sep 17 00:00:00 2001 From: Christopher Kevin <256191862+chrisknvidia@users.noreply.github.com> Date: Mon, 5 Oct 2026 00:04:33 -0700 Subject: [PATCH 2/7] fix: harden TP4 tool preference and structured response retries Signed-off-by: Christopher Kevin <256191862+chrisknvidia@users.noreply.github.com> --- README.md | 13 +- src/skillspector/llm_utils.py | 42 +++- .../nodes/analyzers/mcp_tool_poisoning.py | 34 ++- tests/test_mcp_tool_poisoning.py | 203 ++++++++++------ tests/unit/test_llm_utils.py | 51 +++- tests/unit/test_tp4_openai_http.py | 218 ++++++++++++++++++ 6 files changed, 471 insertions(+), 90 deletions(-) create mode 100644 tests/unit/test_tp4_openai_http.py diff --git a/README.md b/README.md index 68d5dfc54..b573f7031 100644 --- a/README.md +++ b/README.md @@ -244,8 +244,9 @@ inference gateways. | `gemini_cli` | _(none — uses local CLI auth)_ | local `gemini` binary | local Gemini runtime fallback, or `SKILLSPECTOR_MODEL` | | `opencode_cli` | _(none — uses local CLI auth)_ | local `opencode` 1.18.32 binary | local OpenCode runtime fallback, or `SKILLSPECTOR_MODEL` | -Structured output is requested through LangChain's `with_structured_output`, -whose default forces a tool call. Some models reject a forced tool call with +Structured output is requested through LangChain's `with_structured_output`. +The default is client-specific: `ChatOpenAI` uses `json_schema`, while other +clients may use tool calling. Some models reject a forced tool call with HTTP 400 (`tool_choice: type "tool" and "any" are not supported for this model`). The `anthropic` and `anthropic_proxy` providers route those models (`claude-fable-5-1`, `claude-mythos-5-1`, or any registry entry with @@ -256,8 +257,12 @@ prose answer; it recognises the model from the model ID, a geo/global inference-profile ID, or a foundation-model / inference-profile ARN. An application-inference-profile ARN hides the model, so add that ARN to the registry (`SKILLSPECTOR_MODEL_REGISTRY`) with `tool_choice: auto`. -`SKILLSPECTOR_STRUCTURED_OUTPUT_METHOD=json_schema|function_calling` -overrides the method for any provider. +The method precedence is `SKILLSPECTOR_STRUCTURED_OUTPUT_METHOD`, the provider's +method hint, an analyzer preference, then the client default. TP4 prefers +`function_calling` only for provider `openai` and the exact model label +`azure/anthropic/claude-opus-5`, with reasoning/thinking controls unset; dated +or suffixed labels and other providers keep their existing behavior. An explicit +environment override or provider hint still wins when reasoning is configured. ```bash # Stock OpenAI diff --git a/src/skillspector/llm_utils.py b/src/skillspector/llm_utils.py index cd227570c..de29f6844 100644 --- a/src/skillspector/llm_utils.py +++ b/src/skillspector/llm_utils.py @@ -44,8 +44,9 @@ from collections.abc import Coroutine from typing import Any, NoReturn +from langchain_core.exceptions import OutputParserException from langchain_core.language_models.chat_models import BaseChatModel -from langchain_core.runnables import Runnable, RunnableLambda +from langchain_core.runnables import Runnable, RunnableConfig, RunnableLambda from skillspector.inference_usage import ( InferenceUsageCollector, @@ -388,6 +389,11 @@ def structured_output_kwargs( Raises: ValueError: when the environment override or preference is not a known method. """ + if preferred_method is not None and preferred_method not in STRUCTURED_OUTPUT_METHODS: + raise ValueError( + "preferred structured output method must be one of " + f"{', '.join(STRUCTURED_OUTPUT_METHODS)}; got {preferred_method!r}" + ) override = os.environ.get("SKILLSPECTOR_STRUCTURED_OUTPUT_METHOD", "").strip().lower() if override: if override not in STRUCTURED_OUTPUT_METHODS: @@ -402,11 +408,6 @@ def structured_output_kwargs( method = hint(model) if callable(hint) else None if method: return {"method": method} - if preferred_method is not None and preferred_method not in STRUCTURED_OUTPUT_METHODS: - raise ValueError( - "preferred structured output method must be one of " - f"{', '.join(STRUCTURED_OUTPUT_METHODS)}; got {preferred_method!r}" - ) method = preferred_method return {"method": method} if method else {} @@ -429,6 +430,10 @@ def bind_structured_output( """ kwargs = structured_output_kwargs(model, provider, preferred_method=preferred_method) structured = llm.with_structured_output(schema, **kwargs) # type: ignore[attr-defined] + if kwargs.get("method") == "function_calling" and not isinstance( + structured, _StructuredAgentCLIModel + ): + return _validate_tool_call_result(structured, schema) if kwargs or getattr(llm, "supports_tool_choice_values", None) != ("auto",): return structured return _require_tool_call(structured, schema) @@ -446,6 +451,13 @@ def _require_tool_call(structured: Runnable, schema: type) -> Runnable: def _ask(prompt: str) -> str: return f"{prompt}\n\n{_TOOL_CALL_INSTRUCTION.format(tool=tool)}" + return RunnableLambda(_ask) | _validate_tool_call_result(structured, schema) + + +def _validate_tool_call_result(structured: Runnable, schema: type) -> Runnable: + """Validate forced/auto tool results without changing the bound model prompt.""" + tool = schema.__name__ if isinstance(schema, type) else "response" + def _check(result: object) -> object: if result is None: raise StructuredOutputParseError( @@ -453,7 +465,23 @@ def _check(result: object) -> object: ) return result - return RunnableLambda(_ask) | structured | RunnableLambda(_check) + def _invoke(prompt: Any, config: RunnableConfig) -> object: + try: + return _check(structured.invoke(prompt, config=config)) + except OutputParserException as exc: + raise StructuredOutputParseError( + f"model returned invalid {tool} tool arguments" + ) from exc + + async def _ainvoke(prompt: Any, config: RunnableConfig) -> object: + try: + return _check(await structured.ainvoke(prompt, config=config)) + except OutputParserException as exc: + raise StructuredOutputParseError( + f"model returned invalid {tool} tool arguments" + ) from exc + + return RunnableLambda(_invoke, _ainvoke) def get_chat_model( diff --git a/src/skillspector/nodes/analyzers/mcp_tool_poisoning.py b/src/skillspector/nodes/analyzers/mcp_tool_poisoning.py index 8fc679de5..001b51987 100644 --- a/src/skillspector/nodes/analyzers/mcp_tool_poisoning.py +++ b/src/skillspector/nodes/analyzers/mcp_tool_poisoning.py @@ -22,14 +22,14 @@ import re import time import unicodedata -from collections.abc import Callable, Iterator +from collections.abc import Callable, Iterator, Mapping from dataclasses import dataclass, field from typing import cast from langchain_openai.chat_models.base import BaseChatOpenAI from pydantic import BaseModel, Field, field_validator -from skillspector.inference_usage import InferenceUsageRecord +from skillspector.inference_usage import InferenceUsageRecord, chat_model_controls from skillspector.inspection_ledger import ( InspectionLedgerEvent, LedgerOutcome, @@ -56,6 +56,7 @@ padding_run_match_fingerprint, ) from skillspector.providers import get_active_provider +from skillspector.providers.chat_models import resolve_reasoning_effort from skillspector.state import ( AnalyzerNodeResponse, LLMCallRecord, @@ -880,6 +881,28 @@ def _check_tp3( {"python", "javascript", "typescript", "shell", "ruby", "go", "rust"} ) +# Track the exact-label gateway workaround in the public PR: +# https://github.com/NVIDIA/SkillSpector/pull/691 +# Dated/suffixed aliases are deliberately excluded. Only the openai route has +# reproduced wrapper evidence; other providers retain their own method hints. +_TP4_TOOL_OUTPUT_MODELS = frozenset({"azure/anthropic/claude-opus-5"}) + + +def _tp4_reasoning_configured(llm: BaseChatOpenAI) -> bool: + """Avoid an automatic forced tool choice when reasoning controls are set.""" + if resolve_reasoning_effort() is not None: + return True + if chat_model_controls(llm).get("reasoning_effort") is not None: + return True + if any(getattr(llm, name, None) is not None for name in ("reasoning_effort", "reasoning")): + return True + for controls in (llm.model_kwargs, llm.extra_body): + if isinstance(controls, Mapping) and any( + controls.get(name) is not None for name in ("reasoning_effort", "reasoning", "thinking") + ): + return True + return False + class _TP4AnalysisResult(BaseModel): """Validated response from the description-behavior mismatch check.""" @@ -907,11 +930,14 @@ class _TP4Analyzer(LLMAnalyzerBase): def _structured_output_preference(self, llm: object) -> str | None: # The OpenAI-compatible Opus 5 gateway sometimes wraps json_schema # output in a {"json": ...} object instead of the requested schema. - # Tool calling returns the exact TP4 schema on the same gateway. + # Tool calling returns the exact TP4 schema on the same gateway, but + # forcing a tool may conflict with configured reasoning/thinking. + # Explicit environment/provider method choices still take precedence. if ( isinstance(llm, BaseChatOpenAI) and chat_model_provider_name(llm) == "openai" - and self.model == "azure/anthropic/claude-opus-5" + and self.model in _TP4_TOOL_OUTPUT_MODELS + and not _tp4_reasoning_configured(llm) ): return "function_calling" return None diff --git a/tests/test_mcp_tool_poisoning.py b/tests/test_mcp_tool_poisoning.py index 5a040bd52..87aae4501 100644 --- a/tests/test_mcp_tool_poisoning.py +++ b/tests/test_mcp_tool_poisoning.py @@ -21,6 +21,7 @@ import json import re from pathlib import Path +from types import SimpleNamespace from unittest.mock import MagicMock import pytest @@ -235,6 +236,46 @@ def _mock_tp4_structured_llm( return structured_llm +@pytest.fixture +def patch_openai_chat_model(monkeypatch: pytest.MonkeyPatch): + """Use the real ChatOpenAI binder/parser with a recorded model factory.""" + from langchain_openai import ChatOpenAI + + from skillspector import llm_analyzer_base, llm_utils + + monkeypatch.delenv("SKILLSPECTOR_STRUCTURED_OUTPUT_METHOD", raising=False) + monkeypatch.delenv("SKILLSPECTOR_REASONING_EFFORT", raising=False) + bindings: list[dict[str, object]] = [] + original = ChatOpenAI.with_structured_output + + def record_binding(self, schema, **kwargs): + bindings.append(kwargs) + return original(self, schema, **kwargs) + + monkeypatch.setattr(ChatOpenAI, "with_structured_output", record_binding) + + def configure(provider="openai", *, controls=None, provider_hint=None): + def factory(**kwargs): + llm = ChatOpenAI( + model=kwargs["model"], + api_key="sk-test", + base_url="https://example.invalid/v1", + **(controls or {}), + ) + llm_utils.register_chat_model_provider(llm, provider) + return llm + + monkeypatch.setattr(llm_analyzer_base, "get_chat_model", factory) + monkeypatch.setattr( + llm_utils, + "get_active_provider", + lambda: SimpleNamespace(structured_output_method=lambda _model: provider_hint), + ) + return bindings + + return configure + + # Alias used by node import at module level node = mcp_tool_poisoning.node @@ -1579,77 +1620,84 @@ def test_no_applicable_markdown_keeps_clean_status(self, monkeypatch: pytest.Mon class TestTP4Fallbacks: def test_opus5_openai_tp4_binds_tool_output_for_initial_and_refreshed_models( - self, monkeypatch: pytest.MonkeyPatch + self, monkeypatch: pytest.MonkeyPatch, patch_openai_chat_model ) -> None: - from langchain_openai import ChatOpenAI - from skillspector import llm_analyzer_base - from skillspector.llm_utils import register_chat_model_provider - - monkeypatch.delenv("SKILLSPECTOR_STRUCTURED_OUTPUT_METHOD", raising=False) - bindings: list[dict[str, object]] = [] - original = ChatOpenAI.with_structured_output - - def record_binding(self, schema, **kwargs): - bindings.append(kwargs) - return original(self, schema, **kwargs) - - monkeypatch.setattr(ChatOpenAI, "with_structured_output", record_binding) - - def openai_model(**kwargs): - llm = ChatOpenAI( - model=kwargs["model"], api_key="sk-test", base_url="https://example.invalid/v1" - ) - register_chat_model_provider(llm, "openai") - return llm - monkeypatch.setattr(llm_analyzer_base, "get_chat_model", openai_model) + bindings = patch_openai_chat_model() analyzer = mcp_tool_poisoning._TP4Analyzer( model="azure/anthropic/claude-opus-5", timeout=lambda: 30.0 ) monkeypatch.setattr(llm_analyzer_base, "_retarget_request_timeout", lambda *_args: False) analyzer._model_for_call() + assert bindings == [{"method": "function_calling"}, {"method": "function_calling"}] - assert bindings == [ - {"method": "function_calling"}, - {"method": "function_calling"}, - ] - - # The gateway-specific workaround does not change other OpenAI models. - mcp_tool_poisoning._TP4Analyzer(model="azure/anthropic/claude-sonnet-4-6") - assert bindings[-1] == {} - - # An explicit user override retains precedence over the workaround. monkeypatch.setenv("SKILLSPECTOR_STRUCTURED_OUTPUT_METHOD", "json_schema") mcp_tool_poisoning._TP4Analyzer(model="azure/anthropic/claude-opus-5") assert bindings[-1] == {"method": "json_schema"} + @pytest.mark.parametrize( + "provider", ["openai_compatible", "nv_inference", "azure_openai", "nv_build"] + ) def test_opus5_tp4_does_not_force_tool_output_on_other_providers( - self, monkeypatch: pytest.MonkeyPatch + self, monkeypatch: pytest.MonkeyPatch, patch_openai_chat_model, provider ) -> None: - from langchain_openai import ChatOpenAI - from skillspector import llm_analyzer_base - from skillspector.llm_utils import register_chat_model_provider - monkeypatch.delenv("SKILLSPECTOR_STRUCTURED_OUTPUT_METHOD", raising=False) - bindings: list[dict[str, object]] = [] - original = ChatOpenAI.with_structured_output + bindings = patch_openai_chat_model(provider) + analyzer = mcp_tool_poisoning._TP4Analyzer( + model="azure/anthropic/claude-opus-5", timeout=lambda: 30.0 + ) + monkeypatch.setattr(llm_analyzer_base, "_retarget_request_timeout", lambda *_args: False) + analyzer._model_for_call() + assert bindings == [{}, {}] - def record_binding(self, schema, **kwargs): - bindings.append(kwargs) - return original(self, schema, **kwargs) + @pytest.mark.parametrize( + "model", + [ + "azure/anthropic/claude-sonnet-4-6", + "azure/anthropic/claude-opus-5-20260901", + "azure/anthropic/claude-opus-5:latest", + "claude-opus-5", + "AZURE/ANTHROPIC/CLAUDE-OPUS-5", + ], + ) + def test_opus5_tp4_exact_label_excludes_aliases(self, patch_openai_chat_model, model): + bindings = patch_openai_chat_model() + mcp_tool_poisoning._TP4Analyzer(model=model) + assert bindings == [{}] + + @pytest.mark.parametrize("effort", ["low", "medium", "high", "none"]) + def test_reasoning_env_skips_preference_on_initial_and_refreshed_models( + self, monkeypatch, patch_openai_chat_model, effort + ): + from skillspector import llm_analyzer_base - def compatible_model(**kwargs): - llm = ChatOpenAI( - model=kwargs["model"], api_key="sk-test", base_url="https://example.invalid/v1" - ) - register_chat_model_provider(llm, "openai_compatible") - return llm + bindings = patch_openai_chat_model() + monkeypatch.setenv("SKILLSPECTOR_REASONING_EFFORT", effort) + analyzer = mcp_tool_poisoning._TP4Analyzer( + model="azure/anthropic/claude-opus-5", timeout=lambda: 30.0 + ) + monkeypatch.setattr(llm_analyzer_base, "_retarget_request_timeout", lambda *_args: False) + analyzer._model_for_call() + assert bindings == [{}, {}] - monkeypatch.setattr(ChatOpenAI, "with_structured_output", record_binding) - monkeypatch.setattr(llm_analyzer_base, "get_chat_model", compatible_model) + @pytest.mark.parametrize( + "controls", + [ + {"reasoning_effort": "high"}, + {"reasoning": {"effort": "medium"}}, + {"model_kwargs": {"thinking": {"type": "enabled", "budget_tokens": 1024}}}, + {"extra_body": {"thinking": {"type": "enabled", "budget_tokens": 1024}}}, + {"extra_body": {"reasoning_effort": "high"}}, + ], + ) + def test_model_reasoning_controls_skip_preference_on_construction_and_refresh( + self, monkeypatch, patch_openai_chat_model, controls + ): + from skillspector import llm_analyzer_base + bindings = patch_openai_chat_model(controls=controls) analyzer = mcp_tool_poisoning._TP4Analyzer( model="azure/anthropic/claude-opus-5", timeout=lambda: 30.0 ) @@ -1657,6 +1705,25 @@ def compatible_model(**kwargs): analyzer._model_for_call() assert bindings == [{}, {}] + @pytest.mark.parametrize( + "override,hint,expected", + [ + ("function_calling", "json_schema", "function_calling"), + ("json_schema", "function_calling", "json_schema"), + (None, "function_calling", "function_calling"), + (None, "json_schema", "json_schema"), + ], + ) + def test_explicit_method_choices_win_with_reasoning( + self, monkeypatch, patch_openai_chat_model, override, hint, expected + ): + bindings = patch_openai_chat_model(provider_hint=hint) + monkeypatch.setenv("SKILLSPECTOR_REASONING_EFFORT", "high") + if override is not None: + monkeypatch.setenv("SKILLSPECTOR_STRUCTURED_OUTPUT_METHOD", override) + mcp_tool_poisoning._TP4Analyzer(model="azure/anthropic/claude-opus-5") + assert bindings == [{"method": expected}] + def test_configured_output_language_is_included(self, monkeypatch: pytest.MonkeyPatch) -> None: monkeypatch.setenv("SKILLSPECTOR_OUTPUT_LANGUAGE", "German") _mock_tp4_structured_llm(monkeypatch, [{"is_mismatch": False}]) @@ -1736,40 +1803,30 @@ def test_tool_refusal_stays_incomplete(self, monkeypatch: pytest.MonkeyPatch) -> ) def test_opus5_function_calling_refusal_stays_incomplete( - self, monkeypatch: pytest.MonkeyPatch + self, monkeypatch: pytest.MonkeyPatch, patch_openai_chat_model ) -> None: from langchain_core.messages import AIMessage from langchain_core.outputs import ChatGeneration, ChatResult from langchain_openai import ChatOpenAI - from skillspector import llm_analyzer_base - from skillspector.llm_utils import register_chat_model_provider - - monkeypatch.delenv("SKILLSPECTOR_STRUCTURED_OUTPUT_METHOD", raising=False) + patch_openai_chat_model() + calls = [] - def openai_model(**kwargs): - llm = ChatOpenAI( - model=kwargs["model"], api_key="sk-test", base_url="https://example.invalid/v1" - ) - register_chat_model_provider(llm, "openai") - return llm + def refuse(*_args, **_kwargs): + calls.append(1) + return ChatResult(generations=[ChatGeneration(message=AIMessage(content="refused"))]) - monkeypatch.setattr(llm_analyzer_base, "get_chat_model", openai_model) - monkeypatch.setattr( - ChatOpenAI, - "_generate", - lambda *_args, **_kwargs: ChatResult( - generations=[ChatGeneration(message=AIMessage(content="refused"))] - ), - ) + monkeypatch.setattr(ChatOpenAI, "_generate", refuse) + monkeypatch.setattr("skillspector.llm_analyzer_base.time.sleep", lambda _delay: None) state = _make_state("mcp_mismatched_skill", use_llm=True) state["model_config"] = {"default": "azure/anthropic/claude-opus-5"} - result = node(state) - - assert result["analyzer_status_events"][0]["status"] == "failed" + assert len(calls) == 4 + assert result["analyzer_status_events"][0]["status"] == "degraded" assert any( - event["outcome"] is LedgerOutcome.FAILED for event in result["inspection_ledger"] + event.get("reason_code") is LedgerReason.LLM_STRUCTURED_RESPONSE_INVALID + and event.get("error_class") == "ValidationError" + for event in result["inspection_ledger"] ) def test_malformed_response_is_retried(self, monkeypatch: pytest.MonkeyPatch): diff --git a/tests/unit/test_llm_utils.py b/tests/unit/test_llm_utils.py index e7e38bcb3..0d52041c4 100644 --- a/tests/unit/test_llm_utils.py +++ b/tests/unit/test_llm_utils.py @@ -809,10 +809,57 @@ def test_analyzer_preference_applies_only_without_explicit_configuration( "other", provider=_PlainProvider(), preferred_method="function_calling" ) == {"method": "json_schema"} - def test_unknown_analyzer_preference_is_rejected(self, monkeypatch: pytest.MonkeyPatch) -> None: + @pytest.mark.parametrize("override", [None, "json_schema", "function_calling"]) + @pytest.mark.parametrize("provider", [_PlainProvider(), _HintingProvider()]) + def test_unknown_analyzer_preference_is_rejected( + self, monkeypatch: pytest.MonkeyPatch, override, provider + ) -> None: monkeypatch.delenv("SKILLSPECTOR_STRUCTURED_OUTPUT_METHOD", raising=False) + if override is not None: + monkeypatch.setenv("SKILLSPECTOR_STRUCTURED_OUTPUT_METHOD", override) with pytest.raises(ValueError, match="preferred structured output method"): - structured_output_kwargs("other", provider=_PlainProvider(), preferred_method="xml") + structured_output_kwargs("needs-json-1", provider=provider, preferred_method="xml") + + def test_selected_function_calling_rejects_none_without_changing_prompt(self, monkeypatch): + from langchain_core.runnables import RunnableLambda + + monkeypatch.setenv("SKILLSPECTOR_STRUCTURED_OUTPUT_METHOD", "function_calling") + prompts = [] + + class Model: + def with_structured_output(self, schema, **kwargs): + assert kwargs == {"method": "function_calling"} + + def answer(prompt): + prompts.append(prompt) + return None + + return RunnableLambda(answer) + + chain = bind_structured_output(Model(), dict, "m", provider=_PlainProvider()) + with pytest.raises(StructuredOutputParseError): + chain.invoke("unchanged prompt") + assert prompts == ["unchanged prompt"] + + def test_function_calling_preserves_cli_adapter_usage_hooks(self, monkeypatch): + monkeypatch.setenv("SKILLSPECTOR_STRUCTURED_OUTPUT_METHOD", "function_calling") + + class Schema(BaseModel): + verdict: str + + provider = MagicMock() + provider.complete.return_value = '{"verdict": "safe"}' + adapter = AgentCLIChatModel(provider, "m", 1024) + structured = bind_structured_output(adapter, Schema, "m", provider=_PlainProvider()) + collector = InferenceUsageCollector( + node="test", + request_kind="structured_output", + provider="claude_cli", + requested_model="m", + ) + result = structured.invoke_with_usage("test", collector) + assert result == Schema(verdict="safe") + assert collector.response_received def test_env_override_wins_over_the_hint(self, monkeypatch: pytest.MonkeyPatch) -> None: monkeypatch.setenv("SKILLSPECTOR_STRUCTURED_OUTPUT_METHOD", "function_calling") diff --git a/tests/unit/test_tp4_openai_http.py b/tests/unit/test_tp4_openai_http.py new file mode 100644 index 000000000..ee479e39a --- /dev/null +++ b/tests/unit/test_tp4_openai_http.py @@ -0,0 +1,218 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +"""Real SDK HTTP transport, parser, retry, graph and public-report contracts.""" + +from __future__ import annotations + +import json +import threading +from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer +from pathlib import Path + +import pytest + +from skillspector import llm_analyzer_base +from skillspector.inspection_ledger import LedgerReason +from skillspector.nodes.analyzers import mcp_tool_poisoning as tp +from skillspector.providers import reset_provider, use_provider +from skillspector.providers.openai import OpenAIProvider +from skillspector.sarif_models import validate_sarif_report + +_MODEL = "azure/anthropic/claude-opus-5" + + +@pytest.fixture +def openai_http_endpoint(monkeypatch): + requests = [] + behavior = {"mode": "clean", "tp4_calls": 0} + + class Handler(BaseHTTPRequestHandler): + def log_message(self, *_args): + pass + + def do_POST(self): + body = json.loads(self.rfile.read(int(self.headers["Content-Length"]))) + requests.append(body) + tools = body.get("tools", []) + is_tp4 = bool(tools and tools[0]["function"]["name"] == "_TP4AnalysisResult") + if not is_tp4: + response_format = body.get("response_format", {}) + is_tp4 = response_format.get("json_schema", {}).get("name") == "_TP4AnalysisResult" + message = {"role": "assistant", "content": json.dumps({"findings": []})} + finish_reason = "stop" + if is_tp4: + behavior["tp4_calls"] += 1 + mode = behavior["mode"] + if mode == "recover" and behavior["tp4_calls"] > 1: + mode = "clean" + if mode in {"refusal", "recover"}: + message = {"role": "assistant", "content": "Cannot provide this assessment."} + else: + arguments = { + "is_mismatch": mode == "mismatch", + "confidence": 7.0 if mode == "out-of-range" else 0.95, + "declared_purpose_summary": "Format values", + "actual_behavior_summary": "Changes persistent state", + "mismatched_capabilities": ["persistence"] if mode == "mismatch" else [], + "explanation": "The returned assessment is intentionally controlled.", + } + encoded = "not valid JSON" if mode == "malformed" else json.dumps(arguments) + if tools: + message = { + "role": "assistant", + "content": None, + "tool_calls": [ + { + "id": "call_tp4", + "type": "function", + "function": { + "name": "_TP4AnalysisResult", + "arguments": encoded, + }, + } + ], + } + finish_reason = "tool_calls" + else: + message = {"role": "assistant", "content": encoded} + payload = { + "id": "chatcmpl-loopback", + "object": "chat.completion", + "created": 1, + "model": body["model"], + "choices": [{"index": 0, "message": message, "finish_reason": finish_reason}], + "usage": {"prompt_tokens": 12, "completion_tokens": 8, "total_tokens": 20}, + } + encoded = json.dumps(payload).encode() + self.send_response(200) + self.send_header("Content-Type", "application/json") + self.send_header("Content-Length", str(len(encoded))) + self.end_headers() + self.wfile.write(encoded) + + server = ThreadingHTTPServer(("127.0.0.1", 0), Handler) + thread = threading.Thread(target=server.serve_forever, daemon=True) + thread.start() + monkeypatch.setenv("OPENAI_API_KEY", "sk-loopback-test") + monkeypatch.setenv("OPENAI_BASE_URL", f"http://127.0.0.1:{server.server_port}/v1") + monkeypatch.setenv("LANGSMITH_TRACING", "false") + monkeypatch.delenv("SKILLSPECTOR_STRUCTURED_OUTPUT_METHOD", raising=False) + monkeypatch.delenv("SKILLSPECTOR_REASONING_EFFORT", raising=False) + token = use_provider(OpenAIProvider()) + try: + yield requests, behavior + finally: + reset_provider(token) + server.shutdown() + server.server_close() + thread.join(timeout=5) + + +@pytest.mark.parametrize( + "mode,attempts,complete", + [ + ("clean", 1, True), + ("mismatch", 1, True), + ("recover", 2, True), + ("refusal", 4, False), + ("malformed", 4, False), + ("out-of-range", 4, False), + ], +) +@pytest.mark.parametrize("refresh", [False, True]) +def test_tp4_real_http_retries_and_deadline_refresh( + monkeypatch, openai_http_endpoint, mode, attempts, complete, refresh +): + requests, behavior = openai_http_endpoint + behavior["mode"] = mode + if refresh: + monkeypatch.setattr(llm_analyzer_base, "_retarget_request_timeout", lambda *_args: False) + analyzer = tp._TP4Analyzer(model=_MODEL, timeout=(lambda: 30.0) if refresh else 30.0) + outcome = analyzer.run_batches_detailed( + [tp.Batch(file_path="format.py", content="Assess the declared formatting behavior.")] + ) + assert behavior["tp4_calls"] == attempts + assert bool(outcome.successful) is complete + assert bool(outcome.failures) is not complete + for request in requests: + assert request["tool_choice"] == { + "type": "function", + "function": {"name": "_TP4AnalysisResult"}, + } + assert "response_format" not in request + assert "Report your result by calling" not in request["messages"][0]["content"] + if not complete: + assert outcome.failures[0].reason is LedgerReason.LLM_STRUCTURED_RESPONSE_INVALID + assert analyzer.response_received + assert sum(record["total_tokens"] for record in analyzer.inference_usage) == 20 * attempts + + +def test_reasoning_effort_uses_existing_json_schema_route_over_real_http( + monkeypatch, openai_http_endpoint +): + requests, _behavior = openai_http_endpoint + monkeypatch.setenv("SKILLSPECTOR_REASONING_EFFORT", "high") + monkeypatch.setattr(llm_analyzer_base, "_retarget_request_timeout", lambda *_args: False) + analyzer = tp._TP4Analyzer(model=_MODEL, timeout=lambda: 30.0) + outcome = analyzer.run_batches_detailed([tp.Batch(file_path="format.py", content="Assess")]) + assert outcome.successful and not outcome.failures + assert requests[0]["reasoning_effort"] == "high" + assert requests[0]["response_format"]["type"] == "json_schema" + assert "tool_choice" not in requests[0] + + +@pytest.mark.parametrize("output_format", ["json", "sarif"]) +@pytest.mark.parametrize("mode", ["clean", "mismatch", "refusal", "malformed"]) +def test_full_scanner_graph_reports_tp4_http_results( + tmp_path: Path, openai_http_endpoint, output_format, mode +): + from skillspector.graph import create_graph + + requests, behavior = openai_http_endpoint + behavior["mode"] = mode + (tmp_path / "SKILL.md").write_text( + "---\nname: formatting-helper\ndescription: Format supplied values.\n---\n" + "# Formatting Helper\n", + encoding="utf-8", + ) + (tmp_path / "format.py").write_text( + "def format_value(value):\n return str(value)\n", encoding="utf-8" + ) + result = create_graph().invoke( + { + "input_path": str(tmp_path), + "output_format": output_format, + "use_llm": True, + "llm_requested": True, + "model_config": {"default": _MODEL}, + } + ) + tp4_status = next( + event + for event in result["analyzer_status_events"] + if event["analyzer_id"] == tp.ANALYZER_ID + ) + assert tp4_status["status"] == ("completed" if mode in {"clean", "mismatch"} else "degraded") + assert behavior["tp4_calls"] == (1 if mode in {"clean", "mismatch"} else 4) + findings = [f for f in result["findings"] if f.rule_id == "TP4"] + assert bool(findings) is (mode == "mismatch") + if findings: + assert findings[0].file == "format.py" + assert findings[0].match_fingerprint + report = json.loads(result["report_body"]) + complete = mode in {"clean", "mismatch"} + if output_format == "json": + assert report["execution_successful"] is True + assert report["analysis_completeness"]["is_complete"] is complete + assert bool([issue for issue in report["issues"] if issue["id"] == "TP4"]) is ( + mode == "mismatch" + ) + else: + validate_sarif_report(report) + run = report["runs"][0] + assert run["invocations"][0]["properties"]["analysisCompleteness"]["isComplete"] is complete + assert bool([item for item in run["results"] if item["ruleId"] == "TP4"]) is ( + mode == "mismatch" + ) + assert requests From 280176cbc0a0c1f09ad5091e87216e6edea68b61 Mon Sep 17 00:00:00 2001 From: Christopher Kevin <256191862+chrisknvidia@users.noreply.github.com> Date: Mon, 5 Oct 2026 00:05:21 -0700 Subject: [PATCH 3/7] test: assert TP4 manifest and code evidence locations Signed-off-by: Christopher Kevin <256191862+chrisknvidia@users.noreply.github.com> --- tests/unit/test_tp4_openai_http.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/unit/test_tp4_openai_http.py b/tests/unit/test_tp4_openai_http.py index ee479e39a..e5ecb625c 100644 --- a/tests/unit/test_tp4_openai_http.py +++ b/tests/unit/test_tp4_openai_http.py @@ -198,7 +198,8 @@ def test_full_scanner_graph_reports_tp4_http_results( findings = [f for f in result["findings"] if f.rule_id == "TP4"] assert bool(findings) is (mode == "mismatch") if findings: - assert findings[0].file == "format.py" + assert findings[0].file == "SKILL.md" + assert findings[0].evidence["code_path"] == "format.py" assert findings[0].match_fingerprint report = json.loads(result["report_body"]) complete = mode in {"clean", "mismatch"} From c324455d157720f5542052f6b4b9a32509d3fc0f Mon Sep 17 00:00:00 2001 From: Christopher Kevin <256191862+chrisknvidia@users.noreply.github.com> Date: Mon, 5 Oct 2026 00:06:09 -0700 Subject: [PATCH 4/7] test: verify async TP4 HTTP result validation Signed-off-by: Christopher Kevin <256191862+chrisknvidia@users.noreply.github.com> --- tests/unit/test_tp4_openai_http.py | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/tests/unit/test_tp4_openai_http.py b/tests/unit/test_tp4_openai_http.py index e5ecb625c..a84e7161d 100644 --- a/tests/unit/test_tp4_openai_http.py +++ b/tests/unit/test_tp4_openai_http.py @@ -162,6 +162,33 @@ def test_reasoning_effort_uses_existing_json_schema_route_over_real_http( assert "tool_choice" not in requests[0] +@pytest.mark.asyncio +@pytest.mark.parametrize( + "mode,attempts,complete", + [ + ("clean", 1, True), + ("refusal", 4, False), + ("malformed", 4, False), + ], +) +async def test_async_real_http_tool_results_use_same_validation( + openai_http_endpoint, mode, attempts, complete +): + requests, behavior = openai_http_endpoint + behavior["mode"] = mode + analyzer = tp._TP4Analyzer(model=_MODEL, timeout=30.0) + outcome = await analyzer.arun_batches_detailed( + [tp.Batch(file_path="format.py", content="Assess the declared formatting behavior.")] + ) + assert behavior["tp4_calls"] == attempts + assert bool(outcome.successful) is complete + assert bool(outcome.failures) is not complete + if not complete: + assert outcome.failures[0].reason is LedgerReason.LLM_STRUCTURED_RESPONSE_INVALID + assert analyzer.response_received + assert all("response_format" not in request for request in requests) + + @pytest.mark.parametrize("output_format", ["json", "sarif"]) @pytest.mark.parametrize("mode", ["clean", "mismatch", "refusal", "malformed"]) def test_full_scanner_graph_reports_tp4_http_results( From 799eaf2e86bb73a50b3875528d00f157ac974ef8 Mon Sep 17 00:00:00 2001 From: Christopher Kevin <256191862+chrisknvidia@users.noreply.github.com> Date: Mon, 5 Oct 2026 00:08:19 -0700 Subject: [PATCH 5/7] test: verify serialized TP4 identity and code ranges Signed-off-by: Christopher Kevin <256191862+chrisknvidia@users.noreply.github.com> --- tests/unit/test_tp4_openai_http.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/unit/test_tp4_openai_http.py b/tests/unit/test_tp4_openai_http.py index a84e7161d..93774e8a4 100644 --- a/tests/unit/test_tp4_openai_http.py +++ b/tests/unit/test_tp4_openai_http.py @@ -227,7 +227,9 @@ def test_full_scanner_graph_reports_tp4_http_results( if findings: assert findings[0].file == "SKILL.md" assert findings[0].evidence["code_path"] == "format.py" - assert findings[0].match_fingerprint + assert findings[0].evidence["code_start_line"] == 1 + assert findings[0].evidence["code_end_line"] == 2 + assert findings[0].fingerprint() report = json.loads(result["report_body"]) complete = mode in {"clean", "mismatch"} if output_format == "json": From 0f029a42b87805695c3e933e0764f9bfcb02ee62 Mon Sep 17 00:00:00 2001 From: Christopher Kevin <256191862+chrisknvidia@users.noreply.github.com> Date: Mon, 5 Oct 2026 00:09:58 -0700 Subject: [PATCH 6/7] test: verify TP4 findings retain their owning ledger event Signed-off-by: Christopher Kevin <256191862+chrisknvidia@users.noreply.github.com> --- tests/unit/test_tp4_openai_http.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/unit/test_tp4_openai_http.py b/tests/unit/test_tp4_openai_http.py index 93774e8a4..6083fda32 100644 --- a/tests/unit/test_tp4_openai_http.py +++ b/tests/unit/test_tp4_openai_http.py @@ -229,7 +229,11 @@ def test_full_scanner_graph_reports_tp4_http_results( assert findings[0].evidence["code_path"] == "format.py" assert findings[0].evidence["code_start_line"] == 1 assert findings[0].evidence["code_end_line"] == 2 - assert findings[0].fingerprint() + assert any( + findings[0].finding_id in event.get("emitted_finding_ids", []) + for event in result["inspection_ledger"] + if event["analyzer_id"] == tp.ANALYZER_ID and event["path"] == "format.py" + ) report = json.loads(result["report_body"]) complete = mode in {"clean", "mismatch"} if output_format == "json": From d9940d77af30cf3c9942ab832ec27cda33566f7c Mon Sep 17 00:00:00 2001 From: Christopher Kevin Date: Mon, 5 Oct 2026 13:54:51 -0700 Subject: [PATCH 7/7] fix: reject ambiguous structured tool responses Signed-off-by: Christopher Kevin --- src/skillspector/llm_utils.py | 66 +++++++++++++--- tests/test_mcp_tool_poisoning.py | 12 ++- tests/unit/test_tp4_openai_http.py | 121 ++++++++++++++++++++++++----- 3 files changed, 170 insertions(+), 29 deletions(-) diff --git a/src/skillspector/llm_utils.py b/src/skillspector/llm_utils.py index d96efddf4..5335a9eab 100644 --- a/src/skillspector/llm_utils.py +++ b/src/skillspector/llm_utils.py @@ -48,6 +48,7 @@ from langchain_core.exceptions import OutputParserException from langchain_core.language_models.chat_models import BaseChatModel from langchain_core.runnables import Runnable, RunnableConfig, RunnableLambda +from langchain_core.utils.function_calling import convert_to_openai_tool from skillspector.inference_usage import ( InferenceUsageCollector, @@ -437,13 +438,22 @@ def bind_structured_output( which the analyzers retry like any other malformed structured response. """ kwargs = structured_output_kwargs(model, provider, preferred_method=preferred_method) - structured = llm.with_structured_output(schema, **kwargs) # type: ignore[attr-defined] - if _binds_unforced_tool_call(llm, kwargs.get("method")): - return _require_tool_call(structured, schema) + unforced = _binds_unforced_tool_call(llm, kwargs.get("method")) + include_raw = isinstance(llm, BaseChatModel) and ( + unforced or kwargs.get("method") == "function_calling" + ) + binding_kwargs: dict[str, Any] = dict(kwargs) + if include_raw: + # Native parsers may silently keep only the first tool call. Retain the + # raw message so validation can reject extra or invalid calls instead. + binding_kwargs["include_raw"] = True + structured = llm.with_structured_output(schema, **binding_kwargs) # type: ignore[attr-defined] + if unforced: + return _require_tool_call(structured, schema, include_raw=include_raw) if kwargs.get("method") == "function_calling" and not isinstance( structured, _StructuredAgentCLIModel ): - return _validate_tool_call_result(structured, schema) + return _validate_tool_call_result(structured, schema, include_raw=include_raw) return structured @@ -470,21 +480,59 @@ def _binds_unforced_tool_call(llm: object, method: str | None) -> bool: ) -def _require_tool_call(structured: Runnable, schema: type) -> Runnable: +def _require_tool_call( + structured: Runnable, schema: type, *, include_raw: bool = False +) -> Runnable: """Ask for the tool call in the prompt and fail closed when it does not happen.""" - tool = schema.__name__ if isinstance(schema, type) else "response" + tool = ( + convert_to_openai_tool(schema)["function"]["name"] + if include_raw + else schema.__name__ + if isinstance(schema, type) + else "response" + ) def _ask(prompt: str) -> str: return f"{prompt}\n\n{_TOOL_CALL_INSTRUCTION.format(tool=tool)}" - return RunnableLambda(_ask) | _validate_tool_call_result(structured, schema) + return RunnableLambda(_ask) | _validate_tool_call_result( + structured, schema, include_raw=include_raw + ) -def _validate_tool_call_result(structured: Runnable, schema: type) -> Runnable: +def _validate_tool_call_result( + structured: Runnable, schema: type, *, include_raw: bool = False +) -> Runnable: """Validate forced/auto tool results without changing the bound model prompt.""" - tool = schema.__name__ if isinstance(schema, type) else "response" + tool = ( + convert_to_openai_tool(schema)["function"]["name"] + if include_raw + else schema.__name__ + if isinstance(schema, type) + else "response" + ) def _check(result: object) -> object: + if include_raw: + if not isinstance(result, dict): + raise StructuredOutputParseError(f"model returned invalid {tool} tool output") + raw = result.get("raw") + calls = getattr(raw, "tool_calls", None) + if ( + not isinstance(calls, list) + or len(calls) != 1 + or calls[0].get("name") != tool + or getattr(raw, "invalid_tool_calls", None) + ): + raise StructuredOutputParseError( + f"model must return exactly one valid {tool} tool call" + ) + error = result.get("parsing_error") + if error is not None: + raise StructuredOutputParseError( + f"model returned invalid {tool} tool arguments" + ) from error + result = result.get("parsed") if result is None: raise StructuredOutputParseError( f"model answered in prose instead of calling the {tool} tool" diff --git a/tests/test_mcp_tool_poisoning.py b/tests/test_mcp_tool_poisoning.py index dad6bd890..23932b54d 100644 --- a/tests/test_mcp_tool_poisoning.py +++ b/tests/test_mcp_tool_poisoning.py @@ -1667,7 +1667,10 @@ def test_opus5_openai_tp4_binds_tool_output_for_initial_and_refreshed_models( ) monkeypatch.setattr(llm_analyzer_base, "_retarget_request_timeout", lambda *_args: False) analyzer._model_for_call() - assert bindings == [{"method": "function_calling"}, {"method": "function_calling"}] + assert bindings == [ + {"method": "function_calling", "include_raw": True}, + {"method": "function_calling", "include_raw": True}, + ] monkeypatch.setenv("SKILLSPECTOR_STRUCTURED_OUTPUT_METHOD", "json_schema") mcp_tool_poisoning._TP4Analyzer(model="azure/anthropic/claude-opus-5") @@ -1759,7 +1762,12 @@ def test_explicit_method_choices_win_with_reasoning( if override is not None: monkeypatch.setenv("SKILLSPECTOR_STRUCTURED_OUTPUT_METHOD", override) mcp_tool_poisoning._TP4Analyzer(model="azure/anthropic/claude-opus-5") - assert bindings == [{"method": expected}] + assert bindings == [ + { + "method": expected, + **({"include_raw": True} if expected == "function_calling" else {}), + } + ] def test_configured_output_language_is_included(self, monkeypatch: pytest.MonkeyPatch) -> None: monkeypatch.setenv("SKILLSPECTOR_OUTPUT_LANGUAGE", "German") diff --git a/tests/unit/test_tp4_openai_http.py b/tests/unit/test_tp4_openai_http.py index 6083fda32..043c6fcb7 100644 --- a/tests/unit/test_tp4_openai_http.py +++ b/tests/unit/test_tp4_openai_http.py @@ -25,7 +25,7 @@ @pytest.fixture def openai_http_endpoint(monkeypatch): requests = [] - behavior = {"mode": "clean", "tp4_calls": 0} + behavior = {"mode": "clean", "tp4_calls": 0, "tool_name": "_TP4AnalysisResult"} class Handler(BaseHTTPRequestHandler): def log_message(self, *_args): @@ -35,17 +35,20 @@ def do_POST(self): body = json.loads(self.rfile.read(int(self.headers["Content-Length"]))) requests.append(body) tools = body.get("tools", []) - is_tp4 = bool(tools and tools[0]["function"]["name"] == "_TP4AnalysisResult") + tool_name = behavior["tool_name"] + is_tp4 = bool(tools and tools[0]["function"]["name"] == tool_name) if not is_tp4: response_format = body.get("response_format", {}) - is_tp4 = response_format.get("json_schema", {}).get("name") == "_TP4AnalysisResult" + is_tp4 = response_format.get("json_schema", {}).get("name") == tool_name message = {"role": "assistant", "content": json.dumps({"findings": []})} finish_reason = "stop" if is_tp4: behavior["tp4_calls"] += 1 mode = behavior["mode"] - if mode == "recover" and behavior["tp4_calls"] > 1: + if mode in {"recover", "recover-multiple"} and behavior["tp4_calls"] > 1: mode = "clean" + if mode == "recover-multiple": + mode = "multiple-mismatch" if mode in {"refusal", "recover"}: message = {"role": "assistant", "content": "Cannot provide this assessment."} else: @@ -59,19 +62,44 @@ def do_POST(self): } encoded = "not valid JSON" if mode == "malformed" else json.dumps(arguments) if tools: - message = { - "role": "assistant", - "content": None, - "tool_calls": [ + tool_calls = [ + { + "id": "call_tp4", + "type": "function", + "function": { + "name": tool_name, + "arguments": encoded, + }, + } + ] + if mode.startswith("multiple-"): + second_arguments = dict(arguments) + if mode == "multiple-mismatch": + second_arguments.update( + is_mismatch=True, mismatched_capabilities=["persistence"] + ) + tool_calls.append( { - "id": "call_tp4", + "id": "call_tp4_second", "type": "function", "function": { - "name": "_TP4AnalysisResult", - "arguments": encoded, + "name": ( + "UnexpectedAssessment" + if mode == "multiple-unknown" + else tool_name + ), + "arguments": ( + "not valid JSON" + if mode == "multiple-malformed" + else json.dumps(second_arguments) + ), }, } - ], + ) + message = { + "role": "assistant", + "content": None, + "tool_calls": tool_calls, } finish_reason = "tool_calls" else: @@ -96,6 +124,7 @@ def do_POST(self): thread.start() monkeypatch.setenv("OPENAI_API_KEY", "sk-loopback-test") monkeypatch.setenv("OPENAI_BASE_URL", f"http://127.0.0.1:{server.server_port}/v1") + monkeypatch.setenv("SKILLSPECTOR_MODEL", _MODEL) monkeypatch.setenv("LANGSMITH_TRACING", "false") monkeypatch.delenv("SKILLSPECTOR_STRUCTURED_OUTPUT_METHOD", raising=False) monkeypatch.delenv("SKILLSPECTOR_REASONING_EFFORT", raising=False) @@ -118,6 +147,11 @@ def do_POST(self): ("refusal", 4, False), ("malformed", 4, False), ("out-of-range", 4, False), + ("multiple-clean", 4, False), + ("multiple-mismatch", 4, False), + ("multiple-unknown", 4, False), + ("multiple-malformed", 4, False), + ("recover-multiple", 2, True), ], ) @pytest.mark.parametrize("refresh", [False, True]) @@ -140,6 +174,7 @@ def test_tp4_real_http_retries_and_deadline_refresh( "type": "function", "function": {"name": "_TP4AnalysisResult"}, } + assert request["parallel_tool_calls"] is False assert "response_format" not in request assert "Report your result by calling" not in request["messages"][0]["content"] if not complete: @@ -162,13 +197,41 @@ def test_reasoning_effort_uses_existing_json_schema_route_over_real_http( assert "tool_choice" not in requests[0] +def test_real_http_validation_uses_schema_title_as_tool_name(openai_http_endpoint): + requests, behavior = openai_http_endpoint + behavior["tool_name"] = "GatewayTP4Assessment" + + class TitledTP4Assessment(tp._TP4AnalysisResult): + model_config = {"title": "GatewayTP4Assessment"} + + class TitledTP4Analyzer(tp._TP4Analyzer): + response_schema = TitledTP4Assessment + + analyzer = TitledTP4Analyzer(model=_MODEL, timeout=30.0) + outcome = analyzer.run_batches_detailed([tp.Batch(file_path="format.py", content="Assess")]) + + assert behavior["tp4_calls"] == 1 + assert outcome.successful and not outcome.failures + assert isinstance(outcome.successful[0][1][0], TitledTP4Assessment) + assert requests[0]["tool_choice"] == { + "type": "function", + "function": {"name": "GatewayTP4Assessment"}, + } + + @pytest.mark.asyncio @pytest.mark.parametrize( "mode,attempts,complete", [ ("clean", 1, True), + ("mismatch", 1, True), ("refusal", 4, False), ("malformed", 4, False), + ("multiple-clean", 4, False), + ("multiple-mismatch", 4, False), + ("multiple-unknown", 4, False), + ("multiple-malformed", 4, False), + ("recover-multiple", 2, True), ], ) async def test_async_real_http_tool_results_use_same_validation( @@ -190,9 +253,22 @@ async def test_async_real_http_tool_results_use_same_validation( @pytest.mark.parametrize("output_format", ["json", "sarif"]) -@pytest.mark.parametrize("mode", ["clean", "mismatch", "refusal", "malformed"]) +@pytest.mark.parametrize( + "mode,attempts,complete", + [ + ("clean", 1, True), + ("mismatch", 1, True), + ("refusal", 4, False), + ("malformed", 4, False), + ("multiple-clean", 4, False), + ("multiple-mismatch", 4, False), + ("multiple-unknown", 4, False), + ("multiple-malformed", 4, False), + ("recover-multiple", 2, True), + ], +) def test_full_scanner_graph_reports_tp4_http_results( - tmp_path: Path, openai_http_endpoint, output_format, mode + tmp_path: Path, openai_http_endpoint, output_format, mode, attempts, complete ): from skillspector.graph import create_graph @@ -220,8 +296,8 @@ def test_full_scanner_graph_reports_tp4_http_results( for event in result["analyzer_status_events"] if event["analyzer_id"] == tp.ANALYZER_ID ) - assert tp4_status["status"] == ("completed" if mode in {"clean", "mismatch"} else "degraded") - assert behavior["tp4_calls"] == (1 if mode in {"clean", "mismatch"} else 4) + assert tp4_status["status"] == ("completed" if complete else "degraded") + assert behavior["tp4_calls"] == attempts findings = [f for f in result["findings"] if f.rule_id == "TP4"] assert bool(findings) is (mode == "mismatch") if findings: @@ -235,7 +311,6 @@ def test_full_scanner_graph_reports_tp4_http_results( if event["analyzer_id"] == tp.ANALYZER_ID and event["path"] == "format.py" ) report = json.loads(result["report_body"]) - complete = mode in {"clean", "mismatch"} if output_format == "json": assert report["execution_successful"] is True assert report["analysis_completeness"]["is_complete"] is complete @@ -249,4 +324,14 @@ def test_full_scanner_graph_reports_tp4_http_results( assert bool([item for item in run["results"] if item["ruleId"] == "TP4"]) is ( mode == "mismatch" ) - assert requests + tp4_requests = [ + request + for request in requests + if request.get("tools") and request["tools"][0]["function"]["name"] == "_TP4AnalysisResult" + ] + assert len(tp4_requests) == attempts + assert all(request["model"] == _MODEL for request in tp4_requests) + assert all( + request["tool_choice"] == {"type": "function", "function": {"name": "_TP4AnalysisResult"}} + for request in tp4_requests + )