feat: add ICA metering exporter as first pure-Python plugin - #170
Conversation
…CI selection, and release info Signed-off-by: Jonathan Springer <jps@s390x.com>
…ugin Signed-off-by: Jonathan Springer <jps@s390x.com>
…ts shim in integration conftest Signed-off-by: Jonathan Springer <jps@s390x.com>
Signed-off-by: Jonathan Springer <jps@s390x.com>
…re routing Keep ci-install-built-wheel.yaml unchanged because it remains the Rust wheel-helper workflow. Signed-off-by: Jonathan Springer <jps@s390x.com>
…rkflows Signed-off-by: Jonathan Springer <jps@s390x.com>
Signed-off-by: Jonathan Springer <jps@s390x.com>
Signed-off-by: Jonathan Springer <jps@s390x.com>
lucarlig
left a comment
There was a problem hiding this comment.
LGTM
A few non-blocking follow-ups:
- Parse the gateway’s actual
ToolResultshape (_metaandcontent) so token counts and error messages aren’t dropped. - Reject duplicate plugin slugs across the Rust and Python roots.
- Add
capabilities: [read_headers]to the README configuration example.
Signed-off-by: Jonathan Springer <jps@s390x.com>
Signed-off-by: Jonathan Springer <jps@s390x.com>
Signed-off-by: Jonathan Springer <jps@s390x.com>
|
Thanks for the review, Luca. I've pushed three DCO-signed commits addressing each of the three summary items (there were no resolvable inline threads to mark):
Verification:
Branch |
…hon plugins After rebasing onto main (which added ica_metering_exporter as the first pure-Python plugin via PR #170), the combined repository now has 9 plugins: 8 Rust (including output_length_guard) + 1 Python (ica_metering_exporter). Update three assertions that were left with stale values after the rebase: - plugin_count field test: 8 -> 9 - rust_plugin_count split field: "7" -> "8" - test_ci_selection_reports_language_splits_and_counts: comment + assertion 7 -> 8 Signed-off-by: prakhar-singh1928 <prakhar.singh1928@ibm.com>
* feat: migrate output_length_guard plugin to Rust (closes #145) Port the output_length_guard plugin from pure-Python in mcp-context-forge to a Rust core with thin PyO3 bindings, following the pii_filter pattern. - config.rs: OutputLengthGuardConfig — all fields from Python config.py (min/max chars, min/max tokens, chars_per_token, limit_mode, strategy, ellipsis, word_boundary, security limits with identical range validation) - guards.rs: evaluate_text_limits, estimate_tokens, find_word_boundary, truncate, is_numeric_string — 1:1 port of guards.py - structured.rs: process_structured_data, generate_text_representation — 1:1 port of structured.py including all violation codes - plugin.rs: OutputLengthGuardPluginCore PyO3 class — handles all 5 input shapes (plain str, dict+text, list[str], MCP content array, MCP CallToolResult dict with structuredContent) - lib.rs: output_length_guard_rust Python module definition - cpex_output_length_guard/output_length_guard.py: thin Plugin shim - cpex_output_length_guard/__init__.py: lazy-import package entry - cpex_output_length_guard/plugin-manifest.yaml: tool_post_invoke hook - Cargo.toml, pyproject.toml (cpex-output-length-guard), Makefile, README.md - result.metadata["output_length_guard"] emitted when trace_id present: chars_seen, truncated_count, blocked, limit_mode, strategy, stage - No raw content in metrics — counts and labels only - 63 Rust unit tests inline in mod tests across all source modules - Plugin-framework integration tests: plugins/tests/output_length_guard/ Covers all input shapes, both strategies, both limit modes, word-boundary truncation, token mode, metrics gate, security limits, backward compat - Cargo.toml: added output_length_guard to workspace members - Cargo.lock: updated automatically - tests/test_plugin_catalog.py: updated all plugin lists and counts (7→8) Version: 0.1.0 Signed-off-by: prakhar-singh1928 <prakhar.singh1928@ibm.com> * fix(output_length_guard): enforce max_structure_size on MCP content lists Two fixes required to make integration tests pass 21/21: 1. config.rs: Lower MIN_MAX_STRUCTURE_SIZE from 10 to 1 so that small values (e.g. 2) can be configured for testing. Update the corresponding Rust unit test to reject 0 instead of 5, which is still outside the valid range [1, 100_000]. 2. plugin.rs: process_mcp_items_result did not check max_structure_size against the content list length. Add the guard at the top of that function, mirroring the existing check in process_list (structured.rs). Collapsed into a single compound condition to satisfy clippy's collapsible_if lint. All checks pass: cargo clippy -p output_length_guard -- -D warnings ✓ cargo fmt -- --check ✓ cargo test -p output_length_guard 63/63 ✓ make test-integration 21/21 ✓ contract tests (test_plugin_catalog) 126/126 ✓ Signed-off-by: prakhar-singh1928 <prakhar.singh1928@ibm.com> * chore(output_length_guard): add uv.lock for reproducible dev installs Signed-off-by: prakhar-singh1928 <prakhar.singh1928@ibm.com> * fix(output_length_guard): emit observability metrics for MCP CallToolResult dicts Two related fixes in process_mcp_items_result / handle_mcp_content_dict: 1. process_mcp_items_result return type extended from Result<(Vec<Py<PyAny>>, bool), Py<PyAny>> to Result<(Vec<Py<PyAny>>, bool, usize, usize), Py<PyAny>> The two new fields are total_chars_seen and items_modified_count, tallied on each TextResult::Modified arm (both text and resource items). 2. handle_mcp_content_dict was calling process_mcp_items_result and using the result to rebuild the payload but never called push_metrics_kwargs, so result.metadata['output_length_guard'] was silently absent on any traced tool call whose result was a MCP CallToolResult dict (the most common production shape). Now the was_modified branch calls push_metrics_kwargs with the accurate counts from the 4-tuple. handle_mcp_list already had its push_metrics_kwargs call; this patch wires the same logic into handle_mcp_content_dict consistently. All checks pass: cargo clippy -p output_length_guard -- -D warnings ok cargo fmt -- --check ok cargo test -p output_length_guard 63/63 ok make test-integration 21/21 ok Signed-off-by: prakhar-singh1928 <prakhar.singh1928@ibm.com> * test(output_length_guard): kill 86 surviving mutants; fix detect-secrets false positive Signed-off-by: prakhar-singh1928 <prakhar.singh1928@ibm.com> * test(output_length_guard): kill surviving cargo-mutants via targeted unit tests Kill all 33 surviving mutants from PR #169 mutation-testing CI run. Add mutants dependency and extract equivalent-mutant helpers with #[mutants::skip]. ## What changed ### guards.rs — new/replaced tests - snap_loop_decrements_to_exact_char_boundary: multi-byte UTF-8 ('á'=2 bytes) forces the first char-boundary snap loop to execute; asserts exact 1-char result to distinguish -= from += - no_word_boundary_does_not_invoke_boundary_search and word_boundary_true_adjusts_cut_when_space_in_window: 24-byte string (16 a's + space + 7 b's), max_tokens=5 cpt=4 so cut=20 search_back=4; space at byte 16 is inside the window; kills && -> || and > with < on line 100 - caps_value_at_max_text_length / cut_is_product_of_tokens_and_cpt: kill > vs == (line 90) and * vs +// (line 95) - no_word_boundary_does_not_invoke_boundary_search and word_boundary_adj_less_than_cut_updates_cut_byte (char-mode): 22-char string with space at char 16 inside 20% window; kill && -> || (line 136) and <= -> > (line 139) - find_word_boundary_does_not_search_beyond_20_percent_window and _finds_boundary_within_20_percent_window: kill * vs + and * vs / on line 53 - find_word_boundary_empty_string_nonzero_cut_returns_cut_unchanged: kill || -> && on line 49 - evaluate_text_limits_one_above_max_{chars,tokens}_fires_above_max: paired below/above assertions kill > vs >= on lines 27 and 32 ### guards.rs — equivalent-mutant helpers Extract five inline helpers annotated #[mutants::skip] for mutations that are provably semantically equivalent: - is_below_char_min / is_below_token_min: usize > 0 vs >= 0; >= 0 always true and length < 0 is impossible - cap_at_max_text_length: > vs >= when len == max_text_length; capping a slice to its own length is a no-op - is_nonzero: cut > 0 vs >= 0 for usize in word-boundary guards - snap_to_char_boundary: while loop snap; /= produces infinite-loop timeout and >= 0 is equivalent for usize Also mark init_logging with #[mutants::skip] (logging side-effect only, not observable in unit tests — same pattern as sql_sanitizer). ### plugin.rs — new tests - truncated_plain_string_new_length_is_positive_and_not_xyzzy: asserts new_length > 0 and != 5 to kill new_text_str -> String::new() and -> "xyzzy" - string_list_with_trace_id_metrics_have_nonzero_chars_seen: trace_id present; assert chars_seen > 0 and truncated_count > 0; kills += -> *= on lines 218-219 - mcp_content_dict_with_trace_id / _text_item_truncated_count_is_nonzero: same for lines 413-414 text items - mcp_resource_item_with_trace_id / _truncated_count_is_nonzero: lines 442-443 - mcp_content_dict_under_max_structure_size_is_not_blocked: list well under max must not block; kills > -> < on line 369 - mcp_content_dict_oversized_list_in_truncate_mode_is_not_blocked: truncate strategy must not block; kills == -> != on line 369 - mcp_content_dict_none_structured_content_does_not_set_structured_content_processed: None structuredContent must yield sc_processed=false; kills ! deletion on line 762 ### structured.rs — new tests - process_string_token_mode_modulo_mutant_is_killed: length=9 cpt=4 max=1; 9/4=2 fires but 9%4=1 does not; kills / -> % on line 105 - process_string_token_mode_multiply_mutant_is_killed: length=4 cpt=4 max=1; 4/4=1 does not fire but 4*4=16 would; kills / -> * on line 105 - process_list/dict_depth_increments_catch_deeply_nested_*: max_recursion_depth=1 with 2-level nesting; depth+1 hits limit but depth*1 never does; kills + -> * on lines 250 and 323 - generate_text_representation_chain_of_10_stops_at_depth_limit: 11 nested single-key dicts; with +1 the 11th level is json-serialised; with *1 it would unwrap to bare leaf; kills + -> * on line 356 ### Cargo.toml - Add mutants = { workspace = true } dependency ## Result cargo-mutants: 33 missed + 1 timeout -> 0 missed, 131 caught, 437 unviable (exit 0) Signed-off-by: prakhar-singh1928 <prakhar.singh1928@ibm.com> * chore(output_length_guard): add mutants workspace dep for #[mutants::skip] The mutants = "0.0.4" crate is a zero-cost compile-time-only crate that defines the #[mutants::skip] proc-macro attribute. It is required for the nine annotations added in the previous commit (equivalent-mutant helpers and init_logging). Pattern matches sql_sanitizer which carries the same dep. Signed-off-by: prakhar-singh1928 <prakhar.singh1928@ibm.com> * add mutants to cargo.lock Signed-off-by: prakhar-singh1928 <prakhar.singh1928@ibm.com> * fix(output_length_guard): correct metrics mode/strategy and enforce max_structure_size in truncate mode Two bugs fixed in plugin.rs: 1. push_metrics_kwargs and build_blocked_result emitted hardcoded mode: character and strategy: truncate/block regardless of the plugin's actual configuration. Any deployment using limit_mode: token would see {limit_mode: character} in every OTel trace — silently wrong. Fix: add cfg: &OutputLengthGuardConfig to both functions and use cfg.limit_mode.as_str() / cfg.strategy.as_str() at the MetricsArgs construction sites. All 8 call sites updated to pass &self.cfg. Regression test: token_mode_metrics_emit_correct_limit_mode — asserts that a token-mode plugin emits limit_mode=token in traced metadata. 2. process_mcp_items_result guarded max_structure_size with a compound condition (), so Truncate mode would iterate arbitrarily large content arrays with no size cap — a DoS vector for oversized LLM tool responses. Fix: split the condition to match the established pattern in structured.rs::process_list / process_dict — check size unconditionally (log error), then branch on strategy: Block returns a STRUCTURE_SIZE_VIOLATION; Truncate passes the list through unchanged (individual item text is still guarded below). Regression test: mcp_content_dict_oversized_list_truncate_mode_- passes_through_unchanged — sends a 3-item list against max_structure_size=2 / strategy=truncate and asserts no block. All checks pass: cargo clippy -p output_length_guard -- -D warnings ok cargo test -p output_length_guard 139/139 ok Signed-off-by: prakhar.singh1928@ibm.com Signed-off-by: prakhar-singh1928 <prakhar.singh1928@ibm.com> * fix clippy errors Signed-off-by: prakhar-singh1928 <prakhar.singh1928@ibm.com> * fix(output_length_guard): fix metadata key collision and remove dead code in structured.rs Two bugs fixed: 1. plugin.rs — metadata key collision silently drops data In handle_plain_string and handle_text_dict, the kwargs vec already contained (metadata, text_meta_dict) from build_text_meta. push_metrics_kwargs then appended a second (metadata, otel_dict). build_framework_object_dyn iterates the vec into a PyDict via set_item, so the second entry silently overwrote the first. Callers with a trace_id lost either original_length/truncated/ new_length or the output_length_guard metrics depending on insertion order. The same double-append pattern existed in handle_mcp_list, handle_string_list, and handle_mcp_content_dict. Fix: replace push_metrics_kwargs (which appended to the kwargs vec) with merge_metrics_into_meta, which takes a live &Bound<PyDict> and inserts the output_length_guard namespace key directly into the existing metadata dict. Rename build_text_meta -> build_text_meta_dict and change its return type from Py<PyAny> to Bound<PyDict> so it stays bound long enough for the merge before the final unbind. All 5 call sites updated. 2. structured.rs — dead code in generate_text_representation The multi-key dict branch contained a Python::attach(|_py| Ok(())) whose result was immediately discarded (let _ = json_module). The comment acknowledged the GIL was already held. The list branch used if let Ok(list) = ... { let _ = list; ... } — the binding was never used; json_dumps received the original data reference. Fix: remove both dead blocks. Replace the unused-binding list pattern with data.cast::<PyList>().is_ok(). All checks pass: cargo fmt -- --check ok cargo clippy -p output_length_guard -- -D warnings ok cargo test -p output_length_guard 139/139 ok Signed-off-by: prakhar.singh1928@ibm.com Signed-off-by: prakhar-singh1928 <prakhar.singh1928@ibm.com> * fix(output_length_guard): use char count for character-mode detection, not byte length Signed-off-by: prakhar-singh1928 <prakhar.singh1928@ibm.com> * fix(test_plugin_catalog): update counts and splits for 8 Rust + 1 Python plugins After rebasing onto main (which added ica_metering_exporter as the first pure-Python plugin via PR #170), the combined repository now has 9 plugins: 8 Rust (including output_length_guard) + 1 Python (ica_metering_exporter). Update three assertions that were left with stale values after the rebase: - plugin_count field test: 8 -> 9 - rust_plugin_count split field: "7" -> "8" - test_ci_selection_reports_language_splits_and_counts: comment + assertion 7 -> 8 Signed-off-by: prakhar-singh1928 <prakhar.singh1928@ibm.com> * fix(output_length_guard): resolve 4 blocking review issues from PR #169 Fix 1 (guards.rs): cap_at_max_text_length snaps cut to nearest valid UTF-8 char boundary before slicing, preventing PanicException on multi-byte codepoints (e.g. euro sign * 400, max_text_length=1000). Fix 2 (plugin.rs): process_mcp_items_result no longer early-returns on oversized lists in truncate mode. The STRUCTURE_SIZE_VIOLATION block is now gated on strategy == Block only; truncate mode logs a warning and continues to guard individual item text. Fix 3 (plugin.rs): build_violation populates mcp_error_code=-32000 and http_status_code=422 on every PluginViolation, satisfying the gateway contract and avoiding the -32603 fallback in the exception handler. Fix 4 (plugin.rs): introduce TextResult::BelowMin variant. handle_text returns BelowMin when below_min && !above_max in truncate mode. handle_plain_string and handle_text_dict set within_bounds=false for BelowMin results, matching the expected contract. Regression tests added for all four fixes. Inline narration comments referencing 'matching Python behaviour' replaced with functional descriptions. 145 Rust unit tests pass. Python catalog: 133 passed, 3 skipped. Signed-off-by: prakhar-singh1928 <prakhar.singh1928@ibm.com> * fix(plugin_hooks): add mcp_error_code to PluginViolation stub Signed-off-by: prakhar-singh1928 <prakhar.singh1928@ibm.com> * fix(output_length_guard): suppress unkillable mutants on log-only truncate warning The three surviving mutants (> → ==, > → <, > → >=) on plugin.rs:425 all target the condition inside process_mcp_items_result that gates a log::error! warning in truncate mode. Because the branch body has no observable return value or side-effect visible to the test harness, all comparison variants produce identical behaviour and cannot be killed by a unit test. Extract the warning into log_mcp_truncate_size_warning and annotate it with Signed-off-by: prakhar-singh1928 <prakhar.singh1928@ibm.com> #[mutants::skip], matching the established pattern in guards.rs and lib.rs. * use unicode codepoint counting Signed-off-by: prakhar-singh1928 <prakhar.singh1928@ibm.com> * preserve legacy metadata contract Signed-off-by: prakhar-singh1928 <prakhar.singh1928@ibm.com> * address comments Signed-off-by: prakhar-singh1928 <prakhar.singh1928@ibm.com> * fix(output-length-guard): default max_chars to None to match Python behaviour The Python Pydantic model defines: max_chars: Optional[int] = Field(default=None, ...) meaning an empty plugin config produces an unlimited guard. The Rust Default impl had DEFAULT_MAX_CHARS = Some(15_000), so constructing OutputLengthGuardConfig from an empty dict silently truncated responses that Python would have passed through unchanged. Production deployments are unaffected — plugin-manifest.yaml and the gateway config.yaml both set max_chars: 15000 explicitly. The struct default is only reached on direct construction with no config supplied. Changes: - config.rs: DEFAULT_MAX_CHARS None (was Some(15_000)); add comment linking it to the Python field default - config.rs: update default_config_has_expected_values assertion; add empty_config_dict_defaults_max_chars_to_none regression test - test_integration.py: add test_empty_config_defaults_max_chars_to_none_unlimited (full-stack proof: empty PluginConfig, 20k-char input, no truncation) - test_integration.py: add test_unsupported_result_type_is_skipped_with_type_name_in_reason (closes T16 integration-coverage gap for unsupported_type_<name>) Resolves reviewer thread 3872147994 (lucarlig, CHANGES_REQUESTED). Signed-off-by: prakhar-singh1928 <prakhar.singh1928@ibm.com> * fix(output-length-guard): resolve metadata and Unicode boundary review issues Signed-off-by: prakhar-singh1928 <prakhar.singh1928@ibm.com> * test(output-length-guard): cover UTF-8 boundary mutations Signed-off-by: prakhar-singh1928 <prakhar.singh1928@ibm.com> --------- Signed-off-by: prakhar-singh1928 <prakhar.singh1928@ibm.com> Signed-off-by: prakhar.singh1928@ibm.com
Summary
Ports the ICA Metering Exporter from IBM/mcp-context-forge PR #5696 at head commit
be937dd5743d0abbdc0ce8886b6be93f95c9d3dcinto cpex-plugins as the repository's first pure-Python plugin.Notable deviations from the source PR
request_headers_varaccess with the CPEX boundaryextensions.http.headers.headersNameError in the persona-header loop. This correction is worth feeding back upstream to IBM/mcp-context-forge.Design decisions
Test evidence
The complete local validation sequence is recorded in
.omo/evidence/task-*.txtsummaries (local execution evidence, intentionally not committed):detect-secretsscan/audit gates: green.Gateway capability and registration
Gateway registration must grant the plugin the
read_headerscapability for caller attribution throughextensions.http.headers; without it, the gateway strips inbound headers.Gateway-side registration in
mcp-context-forge/plugins/config.yamlis intentionally out of scope for this cpex-plugins pull request.