feat(llm-d): move ext_proc compatibility into AI#334
Conversation
a996b46 to
3aa5dfd
Compare
5e23fbb to
fe71c83
Compare
1f27982 to
5377f78
Compare
There was a problem hiding this comment.
Pull request overview
Moves llm-d ext_proc compatibility into the AI repository behind an opt-in feature.
Changes:
- Adds the AI-owned compatibility crate and vendored protobuf definitions.
- Feature-gates runtime registration and container integration.
- Adds simulator-backed tests and integration documentation.
Reviewed changes
Copilot reviewed 27 out of 29 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
Cargo.lock |
Locks new gRPC/protobuf dependencies. |
Cargo.toml |
Adds workspace crates and dependencies. |
Containerfile |
Includes the optional crate during builds. |
apis/src/lib.rs |
Updates test context initialization. |
docs/developing/llmd-integration-testing.md |
Documents support and testing. |
filters/src/agentic/a2a/mod.rs |
Normalizes comment typography. |
filters/src/lib.rs |
Updates test context initialization. |
integrations/llmd/ext-proc/Cargo.toml |
Defines the compatibility crate. |
integrations/llmd/ext-proc/NOTICE |
Records vendored-code licenses. |
integrations/llmd/ext-proc/build.rs |
Compiles vendored protobufs. |
integrations/llmd/ext-proc/proto/envoy_common.proto |
Vendors Envoy common types. |
integrations/llmd/ext-proc/proto/ext_proc.proto |
Vendors the external-processing protocol. |
integrations/llmd/ext-proc/proto/google/protobuf/any.proto |
Vendors Any. |
integrations/llmd/ext-proc/proto/google/protobuf/duration.proto |
Vendors Duration. |
integrations/llmd/ext-proc/proto/google/protobuf/struct.proto |
Vendors structured values. |
integrations/llmd/ext-proc/proto/google/protobuf/wrappers.proto |
Vendors wrapper types. |
integrations/llmd/ext-proc/src/callout.rs |
Implements single-message callouts. |
integrations/llmd/ext-proc/src/duplex.rs |
Implements duplex exchange state. |
integrations/llmd/ext-proc/src/lib.rs |
Implements configuration and filter hooks. |
integrations/llmd/ext-proc/src/mutations.rs |
Converts and applies mutations. |
integrations/llmd/ext-proc/src/proto.rs |
Exposes generated protocol modules. |
integrations/llmd/ext-proc/src/tests.rs |
Adds extensive unit coverage. |
server/Cargo.toml |
Adds the feature and optional dependency. |
server/src/lib.rs |
Registers the feature-gated filter. |
tests/environment/Cargo.toml |
Defines environment tests. |
tests/environment/tests/suite/main.rs |
Declares the test suite. |
tests/environment/tests/suite/sim_backend.rs |
Tests simulator-backed routing. |
tests/utils/src/net/mod.rs |
Exports simulator helpers. |
tests/utils/src/net/simulator.rs |
Manages simulator containers. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
praxis-bot
left a comment
There was a problem hiding this comment.
Review: feat(llm-d): move ext_proc compatibility into AI
Well-engineered transfer of the llm-d ext_proc compatibility layer into the AI repository. The crate design, state machine, test coverage, and security posture are all strong.
Findings
| Severity | Count |
|---|---|
| Critical | 0 |
| Large | 0 |
| Medium | 2 |
Architecture
The duplex exchange state machine (duplex.rs) is the heart of this crate and is well-structured: six orthogonal state domains, transactional validation (local copy before commit), and clean separation between computation (compute_send_transition) and mutation (apply_send_transition). The BootstrapState enum with SyncWrapper for the non-Sync Process future is a solid approach to satisfying filter state constraints.
The feature-gated registration pattern in server/src/lib.rs is clean -- default builds pay zero cost.
Correctness
- Config validation is thorough:
deny_unknown_fields, range checks on timeouts and status codes, and clear "not yet supported" rejections for unimplemented Envoy features. - Pseudo-header and
Hostprotection in mutations prevents ext_proc from rewriting authority or method. - Body coalescing has overflow-checked size accumulation with a 64 MiB cap.
- The full-duplex
on_request_bodyEOS path correctly sends an empty terminal body marker rather than re-sending accumulated data -- this relies onStreamBufferhaving already forwarded each chunk incrementally. - The
ImmediateResponsestatus code handling correctly falls back to 500 for out-of-range values.
Test Coverage
~8,800 lines of unit tests covering config parsing, validation, proto conversions, all four HeaderAppendAction variants, exchange state machine ordering, timeout overrides, and proto-to-JSON conversion. Six environment integration tests prove the full request path through ext_proc + endpoint_selector to the llm-d-inference-sim container, including spoofed-header rejection and processor failure handling. Strong coverage.
Security
- Pseudo-header and Host mutations are correctly blocked in both request and response directions.
- Timeout overrides are clamped to
max_message_timeoutwith strict protobuf duration validation (rejects negative, zero, sub-millisecond). - The
simulator_spoofed_destination_header_ignoredtest proves client-supplied routing headers cannot override processor decisions. - Body mutation size is bounded with checked arithmetic.
Minor Observations (not blocking)
- The
filters/src/agentic/a2a/mod.rschanges (em-dash to hyphen,O(n^2)notation) are unrelated editorial cleanups. Not harmful but they add noise to the diff -- a separate commit would keep history cleaner. - The struct field reordering in
apis/src/lib.rsandfilters/src/lib.rstest utilities appears to track upstreamHttpFilterContextfield ordering changes. Fine, just noting the coupling.
Move the llm-d ext_proc compatibility layer into the AI repository under integrations/llmd/ext-proc and gate runtime registration behind the llmd-ext-proc feature. The crate is publish=false and scoped to llm-d's current EPP protocol rather than general-purpose Envoy ext_proc support. Environment tests cover the mock EPP to endpoint_selector to llm-d-inference-sim path. Use the published Praxis 0.4.1 crates now that the required support APIs are available from crates.io. Signed-off-by: Brent Salisbury <bsalisbu@redhat.com>
Signed-off-by: Brent Salisbury <bsalisbu@redhat.com>
Signed-off-by: Brent Salisbury <bsalisbu@redhat.com>
Signed-off-by: Brent Salisbury <bsalisbu@redhat.com>
727353c to
e75a9b0
Compare
Summary
Moves the llm-d
ext_proccompatibility layer into the AI repository underintegrations/llmd/ext-proc/.This replaces the abandoned Praxis-side crate publication approach. See the comments from @shaneutt in praxis-proxy/praxis#780 for details. I am pushing a draft as I need it for llm-d integration work in grid so this may be temporary if Shane has other plans for gating ext_proc exposure.
The crate is AI-owned,
publish = false, and scoped to llm-d’s current EPP protocol rather than general-purpose Envoyext_procsupport.Runtime registration is gated behind the
praxis-ai-proxy/llmd-ext-procfeature. Defaultpraxis-ai-proxybuilds do not link, register, or expose theext_procfilter.What changed
praxis-ai-llmd-ext-procatintegrations/llmd/ext-proc/llmd-ext-procfeature topraxis-ai-proxyllm-d-inference-simValidation
cargo check --workspacecargo check --features praxis-ai-proxy/llmd-ext-proccargo clippy --workspace --all-targets -- -D warningscargo clippy --workspace --all-targets --features praxis-ai-proxy/llmd-ext-proc -- -D warningscargo test --workspace --exclude praxis-tests-environmentcargo test -p praxis-tests-environment --features llmd-ext-proc -- --test-threads=1RUSTDOCFLAGS="-D warnings" cargo doc --workspace --no-deps --document-private-itemsmake containergit diff --check