refactor(library): migrate vendor actions to canonical HTTP clients - #2212
Merged
Pouyanpi merged 35 commits intoJul 30, 2026
Merged
Conversation
github-actions
Bot
added
status: needs triage
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
This was referenced Jul 23, 2026
Pouyanpi
force-pushed
the
pouyanpi/rail-library-stack-13-http-action-migrations
branch
2 times, most recently
from
July 23, 2026 16:21
1f8c466 to
9852187
Compare
Pouyanpi
force-pushed
the
pouyanpi/rail-library-stack-13-http-action-migrations
branch
from
July 23, 2026 16:28
9852187 to
736a206
Compare
Pouyanpi
added
status: triaged
Pouyanpi
marked this pull request as ready for review
July 23, 2026 17:16
Contributor
Greptile SummaryThis PR migrates 10 vendor guard integrations (AI Defense, AutoAlign, CrowdStrike AIDR, Fiddler, Pangea, Patronus AI, PolicyAI, Prompt Security, Trend Micro, F5) from per-integration HTTP sessions (
|
| Filename | Overview |
|---|---|
| nemoguardrails/http/types.py | Adds HTTPTLSConfig frozen dataclass with verify, ca_bundle, client_certificate, and client_key fields; post_init validates cert/key pairing but silently allows ca_bundle with verify=False. |
| nemoguardrails/http/transport.py | Wires HTTPTLSConfig into HttpxHTTPClient; validates TLS cannot be combined with an injected client and correctly derives verify/cert from the config dataclass. |
| nemoguardrails/library/f5/actions.py | Removes bespoke aiohttp retry loop; delegates retry and Retry-After handling to RetryPolicy with clamp_retry_after=True; fail-open paths and 429 detection preserved correctly. |
| nemoguardrails/library/clavata/request.py | Migrates ClavataClient._make_request from aiohttp to http_call; RetryingHTTPClient wraps injected or factory clients; except ClavataPluginError: raise correctly preserves the ClavataPluginAPIRateLimitError type. |
| nemoguardrails/library/autoalign/actions.py | Extracts _autoalign_post helper; autoalign_infer guards against empty NDJSON lines but autoalign_groundedness_infer does not, creating an inconsistency that could surface a JSONDecodeError on interior blank lines. |
| nemoguardrails/library/hf_classifier/backends.py | Replaces custom aiohttp/httpx sessions with _RemoteBackend._post backed by http_call and HTTPTLSConfig; _build_ssl_config duplicates the cert/key pairing check already in HTTPTLSConfig.post_init. |
| nemoguardrails/library/pangea/actions.py | Migrates pangea_ai_guard from a managed httpx context to http_call with raise_for_status=False; broad except Exception preserves original fail-open semantics; text_guard_response is always defined before use. |
| nemoguardrails/library/crowdstrike_aidr/actions.py | Uses try/except HTTPStatusError/else pattern cleanly; fallback is pre-built before the request so the function always returns a valid outcome; guard_response is only accessed in the else branch where it is guaranteed to be assigned. |
| tests/http/test_library_boundary.py | New AST-based conformance test enforces that no file under nemoguardrails/library/ imports aiohttp, httpx, requests, or urllib3 directly; effective guard against future regressions. |
| tests/http_utils.py | Extracts shared RecordedHTTPResponses into a single utility that asserts bidirectional URL matching, replacing the previously fragmented per-file copies. |
Sequence Diagram
sequenceDiagram
participant Action as Vendor Action
participant http_call as http_call()
participant Resolve as _resolve_http_client()
participant Injected as Injected HTTPClient
participant Factory as factory()
participant Retrying as RetryingHTTPClient
participant Transport as HttpxHTTPClient
Action->>http_call: http_call(client?, method, url, factory)
http_call->>Resolve: _resolve_http_client(client, factory)
alt client provided
Resolve-->>Injected: yield injected (not closed)
else client is None
Resolve->>Factory: factory()
Factory->>Transport: "HttpxHTTPClient(tls=HTTPTLSConfig)"
Factory->>Retrying: RetryingHTTPClient(transport, RetryPolicy)
Resolve-->>Retrying: yield owned (closed on exit)
end
http_call->>Retrying: request(method, url)
Retrying->>Transport: POST with retry on 429
Transport-->>Retrying: HTTPResponse
Retrying-->>http_call: HTTPResponse
http_call-->>Action: HTTPResponse
Reviews (9): Last reviewed commit: "docs(library): document provider action ..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Pouyanpi
force-pushed
the
pouyanpi/rail-library-stack-13-http-action-migrations
branch
3 times, most recently
from
July 24, 2026 15:53
bf8c358 to
9c0ae71
Compare
11 tasks
Pouyanpi
force-pushed
the
pouyanpi/rail-library-stack-13-http-action-migrations
branch
from
July 29, 2026 08:13
9c0ae71 to
60b4af2
Compare
Signed-off-by: Pouyanpi <13303554+Pouyanpi@users.noreply.github.com>
Signed-off-by: Pouyanpi <13303554+Pouyanpi@users.noreply.github.com>
Signed-off-by: Pouyanpi <13303554+Pouyanpi@users.noreply.github.com>
Signed-off-by: Pouyanpi <13303554+Pouyanpi@users.noreply.github.com>
Signed-off-by: Pouyanpi <13303554+Pouyanpi@users.noreply.github.com>
Signed-off-by: Pouyanpi <13303554+Pouyanpi@users.noreply.github.com>
Signed-off-by: Pouyanpi <13303554+Pouyanpi@users.noreply.github.com>
Signed-off-by: Pouyanpi <13303554+Pouyanpi@users.noreply.github.com>
Signed-off-by: Pouyanpi <13303554+Pouyanpi@users.noreply.github.com>
Signed-off-by: Pouyanpi <13303554+Pouyanpi@users.noreply.github.com>
Signed-off-by: Pouyanpi <13303554+Pouyanpi@users.noreply.github.com>
Signed-off-by: Pouyanpi <13303554+Pouyanpi@users.noreply.github.com>
Signed-off-by: Pouyanpi <13303554+Pouyanpi@users.noreply.github.com>
Signed-off-by: Pouyanpi <13303554+Pouyanpi@users.noreply.github.com>
Signed-off-by: Pouyanpi <13303554+Pouyanpi@users.noreply.github.com>
Signed-off-by: Pouyanpi <13303554+Pouyanpi@users.noreply.github.com>
Signed-off-by: Pouyanpi <13303554+Pouyanpi@users.noreply.github.com>
Pouyanpi
force-pushed
the
pouyanpi/rail-library-stack-13-http-action-migrations
branch
from
July 30, 2026 07:56
73a2977 to
b02780f
Compare
Contributor
📝 WalkthroughWalkthroughThe PR adds shared TLS configuration and injectable HTTP clients, migrates library integrations from direct HTTP libraries to the canonical HTTP abstraction, centralizes retry and failure handling, and expands recording-based tests for transport behavior and request contracts. ChangesHTTP foundation
Guardrail integrations
Clavata and Fiddler
Remote classifiers and vendor actions
Testing infrastructure
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (5)
nemoguardrails/library/crowdstrike_aidr/actions.py (1)
137-163: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueCollapse the two identical fallback branches.
Both handlers build the same
GuardChatCompletionsResultand return the same outcome; only the log message differs. Merging them removes the duplication and also lets you narrow the blindexcept Exceptionthat Ruff flags (BLE001) to the decode/validation errors you actually expect (HTTPClientError,ValidationError).♻️ Suggested shape
- except HTTPStatusError as e: - log.error("HTTP status error from CrowdStrike AIDR API: %s", e) - return _crowdstrike_aidr_outcome( - GuardChatCompletionsResult( - guard_output={"messages": messages}, - blocked=False, - transformed=False, - bot_message=bot_message, - user_message=user_message, - ), - mode, - ) - except Exception as e: - log.error("Error calling CrowdStrike AIDR API: %s", e) + except (HTTPStatusError, HTTPClientError, ValidationError) as e: + log.error("CrowdStrike AIDR API call failed (%s): %s", type(e).__name__, e) return _crowdstrike_aidr_outcome( GuardChatCompletionsResult( guard_output={"messages": messages}, blocked=False, transformed=False, bot_message=bot_message, user_message=user_message, ), mode, )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@nemoguardrails/library/crowdstrike_aidr/actions.py` around lines 137 - 163, Consolidate the duplicate fallback returns in the response handling around GuardChatCompletionsResponse into one shared branch, while retaining distinct logging as needed. Replace the broad `except Exception` with handlers for the expected `HTTPClientError` and `ValidationError` cases, alongside `HTTPStatusError`, and ensure all these failures return the same `_crowdstrike_aidr_outcome` built from `GuardChatCompletionsResult`.Source: Linters/SAST tools
nemoguardrails/library/autoalign/actions.py (1)
171-180: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueExtract the repeated POST + status-check into one helper.
The same
http_call(..., raise_for_status=False)plusif response.status_code != 200: raise ValueError(...)block is duplicated three times; a small_autoalign_post(http_client, request_url, headers, request_body)returning the response keeps the failure message in one place.Also applies to: 206-215, 249-258
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@nemoguardrails/library/autoalign/actions.py` around lines 171 - 180, Extract the duplicated POST request and non-200 status validation from the AutoAlign action flows into a shared _autoalign_post(http_client, request_url, headers, request_body) helper. Have the helper call http_call with the existing arguments, raise the same ValueError including status code and response text, and return the successful response; replace all three duplicated blocks, including those around the referenced ranges, with calls to this helper.nemoguardrails/library/f5/actions.py (1)
92-96: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDocument the new
http_clientparameter.The Args section still lists only
textandconfig, so the injectable-client contract (caller-owned, wrapped with the F5 retry policy) is undocumented.As per coding guidelines: "Update documentation when changing user-visible behavior, public APIs, configuration syntax, examples, or installation requirements."📝 Proposed docstring addition
Args: text: The text to scan. config: The active RailsConfig; used to resolve ``rails.config.f5``. + http_client: Optional caller-owned client; it is wrapped with the F5 + retry policy and is not closed by this action.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@nemoguardrails/library/f5/actions.py` around lines 92 - 96, Update the docstring for f5_guardrails_scan to document the http_client parameter in its Args section, including that callers may provide an owned HTTP client and that it is used with the F5 retry policy; retain the existing text and config documentation.Source: Coding guidelines
tests/test_clavata_models.py (1)
278-320: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider moving the transport/retry tests out of
TestCreateJobResponseand sharing the retry-patch setup.These new tests exercise
ClavataClienttransport behavior, notCreateJobResponseparsing, and the_retrying_http_clientzero-sleep monkeypatch is duplicated verbatim in two tests. A dedicated class plus a fixture for the patched retry client would keep intent clear.Also applies to: 385-397
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_clavata_models.py` around lines 278 - 320, Move the transport/retry tests, including test_clavata_client_uses_shared_retry_policy and the related test at the indicated later section, out of TestCreateJobResponse into a dedicated ClavataClient transport/retry test class. Add a shared fixture for the zero-sleep ClavataClient._retrying_http_client monkeypatch and reuse it in both tests, preserving their existing assertions and behavior.nemoguardrails/library/clavata/request.py (1)
173-197: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReturn
ClosableHTTPClientfrom the retry helpers.
http_call(..., factory=...)requiresCallable[[], ClosableHTTPClient], but both_retrying_http_clientand_create_http_clientare typed asHTTPClient. Narrow those return types here, or inline theRetryingHTTPClientconstruction, so the factory signature matches.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@nemoguardrails/library/clavata/request.py` around lines 173 - 197, Update _retrying_http_client and _create_http_client to return ClosableHTTPClient, ensuring RetryingHTTPClient satisfies that type and the factory passed to http_call matches Callable[[], ClosableHTTPClient]. Preserve the existing retry policy and client construction behavior.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@nemoguardrails/library/ai_defense/actions.py`:
- Around line 124-137: Validate that the value returned by response.json() is a
mapping before the existing "is_safe" handling in the AI Defense action. Treat
lists, strings, numbers, and other non-object JSON bodies as malformed, log the
appropriate fail_open/fail_closed warning, and set is_blocked according to that
policy; continue using the existing field validation for object responses.
In `@nemoguardrails/library/trend_micro/actions.py`:
- Around line 135-156: The fail-open handling does not cover outbound request
failures in both integrations. In nemoguardrails/library/trend_micro/actions.py
lines 135-156, move http_call into the try block and widen the exception
handling to cover transport, response decoding, and GuardResult validation
failures while preserving the allow fallback. In
nemoguardrails/library/pangea/actions.py lines 124-151, move http_call into the
existing try block so transport failures reach the current _pangea_outcome
fallback.
In `@tests/http/test_runtime.py`:
- Around line 35-38: Update the test around the factory.call_args assertions to
first verify that the factory was invoked exactly once, then inspect its kwargs
and retain the existing timeout, limits, and follow_redirects assertions.
---
Nitpick comments:
In `@nemoguardrails/library/autoalign/actions.py`:
- Around line 171-180: Extract the duplicated POST request and non-200 status
validation from the AutoAlign action flows into a shared
_autoalign_post(http_client, request_url, headers, request_body) helper. Have
the helper call http_call with the existing arguments, raise the same ValueError
including status code and response text, and return the successful response;
replace all three duplicated blocks, including those around the referenced
ranges, with calls to this helper.
In `@nemoguardrails/library/clavata/request.py`:
- Around line 173-197: Update _retrying_http_client and _create_http_client to
return ClosableHTTPClient, ensuring RetryingHTTPClient satisfies that type and
the factory passed to http_call matches Callable[[], ClosableHTTPClient].
Preserve the existing retry policy and client construction behavior.
In `@nemoguardrails/library/crowdstrike_aidr/actions.py`:
- Around line 137-163: Consolidate the duplicate fallback returns in the
response handling around GuardChatCompletionsResponse into one shared branch,
while retaining distinct logging as needed. Replace the broad `except Exception`
with handlers for the expected `HTTPClientError` and `ValidationError` cases,
alongside `HTTPStatusError`, and ensure all these failures return the same
`_crowdstrike_aidr_outcome` built from `GuardChatCompletionsResult`.
In `@nemoguardrails/library/f5/actions.py`:
- Around line 92-96: Update the docstring for f5_guardrails_scan to document the
http_client parameter in its Args section, including that callers may provide an
owned HTTP client and that it is used with the F5 retry policy; retain the
existing text and config documentation.
In `@tests/test_clavata_models.py`:
- Around line 278-320: Move the transport/retry tests, including
test_clavata_client_uses_shared_retry_policy and the related test at the
indicated later section, out of TestCreateJobResponse into a dedicated
ClavataClient transport/retry test class. Add a shared fixture for the
zero-sleep ClavataClient._retrying_http_client monkeypatch and reuse it in both
tests, preserving their existing assertions and behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 0cd009ab-0ab9-424e-8547-b32de9443b83
📒 Files selected for processing (39)
nemoguardrails/http/__init__.pynemoguardrails/http/runtime.pynemoguardrails/http/transport.pynemoguardrails/http/types.pynemoguardrails/library/ai_defense/actions.pynemoguardrails/library/autoalign/actions.pynemoguardrails/library/clavata/actions.pynemoguardrails/library/clavata/errs.pynemoguardrails/library/clavata/request.pynemoguardrails/library/clavata/utils.pynemoguardrails/library/crowdstrike_aidr/actions.pynemoguardrails/library/f5/actions.pynemoguardrails/library/fiddler/actions.pynemoguardrails/library/hf_classifier/actions.pynemoguardrails/library/hf_classifier/backends.pynemoguardrails/library/pangea/actions.pynemoguardrails/library/patronusai/actions.pynemoguardrails/library/policyai/actions.pynemoguardrails/library/prompt_security/actions.pynemoguardrails/library/trend_micro/actions.pynemoguardrails/testing/http.pytests/http/test_contract.pytests/http/test_library_boundary.pytests/http/test_runtime.pytests/http/test_testing.pytests/http/test_transport.pytests/http_utils.pytests/test_ai_defense.pytests/test_autoalign.pytests/test_clavata.pytests/test_clavata_models.pytests/test_clavata_utils.pytests/test_f5_guardrails.pytests/test_fiddler_rails.pytests/test_hf_classifier.pytests/test_pangea_ai_guard.pytests/test_patronus_evaluate_api.pytests/test_policyai_rail.pytests/test_prompt_security.py
💤 Files with no reviewable changes (3)
- nemoguardrails/library/clavata/errs.py
- nemoguardrails/library/clavata/utils.py
- tests/test_clavata_utils.py
Collaborator
Author
|
Thank you @tgasser-nv for the review. Improved patch coverage to 100% and fixed those kwargs slops. |
Signed-off-by: Pouyanpi <13303554+Pouyanpi@users.noreply.github.com>
Signed-off-by: Pouyanpi <13303554+Pouyanpi@users.noreply.github.com>
Signed-off-by: Pouyanpi <13303554+Pouyanpi@users.noreply.github.com>
Signed-off-by: Pouyanpi <13303554+Pouyanpi@users.noreply.github.com>
Signed-off-by: Pouyanpi <13303554+Pouyanpi@users.noreply.github.com>
Signed-off-by: Pouyanpi <13303554+Pouyanpi@users.noreply.github.com>
Signed-off-by: Pouyanpi <13303554+Pouyanpi@users.noreply.github.com>
Pouyanpi
deleted the
pouyanpi/rail-library-stack-13-http-action-migrations
branch
July 30, 2026 08:48
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Migrate built-in library integrations to the canonical outbound HTTP client.
This combines the former vendor-action and specialized-client stack layers.
The migrated vendor actions cover AI Defense, AutoAlign, CrowdStrike AIDR,
Fiddler, Pangea, Patronus AI, PolicyAI, Prompt Security, Trend Micro, and F5.
They receive the managed client through action-parameter injection and use
http_callfor request execution.The Hugging Face classifier and Clavata integrations retain their specialized
retry and TLS requirements. A transport-neutral
HTTPTLSConfigsupports customCA bundles and paired client certificate and key paths without leaking HTTPX
types into integrations.
Integration-owned POST retry policies remain explicit. Injected clients remain
caller-owned, while fallback factories return fully composed clients that
http_callcloses deterministically.F5 preserves its bounded
Retry-Afterbehavior. Shared recorded-response testsverify registered endpoints in both directions, and an AST-based conformance
test prevents library integrations from importing HTTPX, aiohttp, Requests, or
urllib3 directly.
Impact
boundary.
Stack
pouyanpi/rail-library-stack-9-http-contractdeveloppouyanpi/rail-library-stack-10-http-instrumentationpouyanpi/rail-library-stack-11-http-observabilitypouyanpi/rail-library-stack-12-http-request-migrationspouyanpi/rail-library-stack-13-http-action-migrationsRelated Issue(s)
Verification
make test TEST="tests/http tests/test_ai_defense.py tests/test_f5_guardrails.py tests/test_fiddler_rails.py tests/test_pangea_ai_guard.py tests/test_patronus_evaluate_api.py tests/test_policyai_rail.py"— 197 passed, 4 skipped.AI Assistance
Codex assisted with implementation analysis, validation, stack restructuring,
and this draft. Check the disclosure box after human review.
Checklist
Summary by CodeRabbit